Re: [PATCH v2 05/11] ASoC: SDCA: add populate_function hw_op for DT function data

[Charles Keepax <ckeepax@opensource.cirrus.com>]

On Tue, Sep 08, 2026 at 07:00:22PM +0100, Srinivas Kandagatla wrote:
> On 9/8/26 5:25 PM, Charles Keepax wrote:
> > On Mon, Sep 07, 2026 at 09:37:19AM +0100, Srinivas Kandagatla wrote:
> >> -	ret = sdca_parse_function(dev, drv->function);
> >> +	if (drv->function->desc->node) {
> >> +		ret = sdca_parse_function(dev, drv->function);
> >> +	} else if (core->hw_ops && core->hw_ops->populate_function) {
> >> +		ret = core->hw_ops->populate_function(core->sdw, drv->function);
> >> +	} else {
> >> +		dev_err(dev, "no firmware node and no populate_function hook\n");
> >> +		return -ENOENT;
> >> +	}
> > 
> > Is this the right logic here? I feel like if we have a
> > populate_function() callback we should call it regardless of if we
> 
> something like this?
> -------------------->cut<---------------------------------
> if (drv->function->desc->node)
> 	ret = sdca_parse_function(dev, drv->function);
> 
> if (core->hw_ops && core->hw_ops->populate_function)
> 	ret = core->hw_ops->populate_function(core->sdw, drv->function);
> 
> if (ret)
> 	return ret;

I was thinking more:

if (core->hw_ops && core->hw_ops->populate_function) {
	ret = core->hw_ops->populate_function(core->sdw, drv->function);
} else if (drv->function->desc->node) {
	ret = sdca_parse_function(dev, drv->function);
} else {
	dev_err(dev, "no firmware node and no populate_function hook\n");
	return -ENOENT;
}

Although noticing as I do that it is also a little weird the
populate_function() callback takes the soundwire device not the
function device as an argument. I will have a look at the wider
picture on that.

Thanks,
Charles