thesofproject / linux

Linux kernel source tree
Other
91 stars 133 forks source link

SoundWire/ASoC/ALSA: fix usages of device_get_named_child_node() #4931

Closed plbossart closed 7 months ago

plbossart commented 7 months ago

We don't seem to be following the documentation for all access to hierarchical _DSD properties for audio.

plbossart commented 7 months ago

@andy-shev FYI, if you have time to review.

andy-shev commented 7 months ago

So, it's mostly technical conversion, the problem is clear, the solution as well. I'm not going to check everything, esp. you already got a response, but on brief view looks reasonable.

vijendarmukunda commented 7 months ago

In sdw_slave_read_prop() function, below condition is not addressed. nval = hweight32(prop->source_ports); prop->src_dpn_prop = devm_kcalloc(&slave->dev, nval, sizeof(*prop->src_dpn_prop), GFP_KERNEL); if (!prop->src_dpn_prop) return -ENOMEM;

Second condition check:

prop->sink_dpn_prop = devm_kcalloc(&slave->dev, nval, sizeof(*prop->sink_dpn_prop), GFP_KERNEL); if (!prop->sink_dpn_prop) return -ENOMEM;

As per our understanding, above one must be valid cases, where we need for put.

plbossart commented 7 months ago

In sdw_master_read_amd_prop() function , for below check , fwnode_handle_put(link) addition is missing.

link = device_get_named_child_node(bus->dev, name); if (!link) { dev_err(bus->dev, "Manager node %s not found\n", name); return -EIO; }

no if the get failed there's no need for a put.

vijendarmukunda commented 7 months ago

In sdw_master_read_amd_prop() function , for below check , fwnode_handle_put(link) addition is missing. link = device_get_named_child_node(bus->dev, name); if (!link) { dev_err(bus->dev, "Manager node %s not found\n", name); return -EIO; }

no if the get failed there's no need for a put.

Agreed.

plbossart commented 7 months ago

In sdw_slave_read_prop() function, below condition is not addressed. nval = hweight32(prop->source_ports); prop->src_dpn_prop = devm_kcalloc(&slave->dev, nval, sizeof(*prop->src_dpn_prop), GFP_KERNEL); if (!prop->src_dpn_prop) return -ENOMEM;

Second condition check:

prop->sink_dpn_prop = devm_kcalloc(&slave->dev, nval, sizeof(*prop->sink_dpn_prop), GFP_KERNEL); if (!prop->sink_dpn_prop) return -ENOMEM;

Yes this one has a set of misses, will fix. Thanks for the review.

plbossart commented 7 months ago

In sdw_slave_read_prop() function, below condition is not addressed. nval = hweight32(prop->source_ports); prop->src_dpn_prop = devm_kcalloc(&slave->dev, nval, sizeof(*prop->src_dpn_prop), GFP_KERNEL); if (!prop->src_dpn_prop) return -ENOMEM;

Humm, after re-checking this is fine: the "port" is released above. Please double-check.

Second condition check:

prop->sink_dpn_prop = devm_kcalloc(&slave->dev, nval, sizeof(*prop->sink_dpn_prop), GFP_KERNEL); if (!prop->sink_dpn_prop) return -ENOMEM;

Same, for src and sink ports all the references are released in sdw_slave_read_dpn()

So no change for these two comments.

vijendarmukunda commented 7 months ago

sdw_slave_read_dpn

Correct. Port references are released in sdw_slave_read_dpn (). No need to add put for these cases.