Re: [RFC PATCH 3/8] ASoC: SDCA: expose class helpers with hw_ops for non-DisCo platforms

[Charles Keepax]

On Thu, Jul 23, 2026 at 12:42:13AM +0100, Srinivas Kandagatla wrote:
> On ARM/DT platforms without ACPI/DisCo, sdca_lookup_functions() is a
> no-op and num_functions stays 0.  Introduce struct sdca_class_hw_ops
> with hw_init and get_function_data callbacks so codec drivers can
> supply pre-populated sdca_function_data and toggle supplies/reset at
> probe time.
> 
> Convert the class SoundWire probe into an exported library helper
> sdca_class_probe(sdw, hw_ops) and export sdca_class_read_prop() and
> sdca_class_pm_ops.  Codec-specific SoundWire drivers can register their
> own sdw_driver and call these helpers from their probe.  The built-in
> class_sdw_driver stays for generic SDCA parts that need no per-device
> quirks (hw_ops = NULL).
> 
> For non-DisCo boots, the class helper injects the driver-supplied
> function descriptors into sdca_device_data so sdca_dev_register_functions()
> can create the auxiliary devices, and the auxiliary function driver uses
> get_function_data() as a fallback source of per-entity data when the
> firmware node is absent.
> 
> Assisted-by: Claude:claude-opus-4-7
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxxxxxxxx>
> ---
> -static int class_sdw_probe(struct sdw_slave *sdw, const struct sdw_device_id *id)
> +/**
> + * sdca_class_probe - SDCA class SoundWire slave probe helper
> + * @sdw: SoundWire slave
> + * @hw_ops: optional device-specific hw_ops (may be NULL for pure-generic
> + *          SDCA parts that need no quirks)
> + *
> + * Exported so codec-specific SoundWire drivers can call this from their
> + * own sdw_driver.probe.  For codecs with quirks, pass the codec's
> + * sdca_class_hw_ops so hw_init, DT function injection, and PDE hooks
> + * are wired up.  For pure-generic SDCA parts (used by the built-in
> + * class_sdw_driver in this file), pass NULL.
> + */
> +int sdca_class_probe(struct sdw_slave *sdw,
> +		     const struct sdca_class_hw_ops *hw_ops)

If we end up going down this route quite tempted to pass the
hw_ops through the driver_data in the sdw_device_id. But not
totally certain on that.

>  {
>  	struct device *dev = &sdw->dev;
> +	struct sdca_device_data *data = &sdw->sdca_data;
>  	struct regmap_config *dev_config;
>  	struct sdca_class_drv *drv;
>  	int ret;
> @@ -156,11 +179,47 @@ static int class_sdw_probe(struct sdw_slave *sdw, const struct sdw_device_id *id
>  
>  	drv->dev = dev;
>  	drv->sdw = sdw;
> +	drv->hw_ops = hw_ops;
>  	mutex_init(&drv->regmap_lock);
>  	mutex_init(&drv->init_lock);
>  
>  	dev_set_drvdata(drv->dev, drv);
>  
> +	/*
> +	 * On ARM platforms without ACPI/DisCo tables, sdca_lookup_functions()
> +	 * is a no-op and num_functions stays 0. Inject the function descriptors
> +	 * from the device-specific static data so sdca_dev_register_functions()
> +	 * can create the auxiliary devices.
> +	 */
> +	if (data->num_functions == 0 && hw_ops && hw_ops->get_function_data) {
> +		struct sdca_function_data *fdata;
> +		unsigned int num = 0;
> +		unsigned int i;
> +
> +		fdata = hw_ops->get_function_data(&num);
> +		if (!fdata || num == 0 || num > SDCA_MAX_FUNCTION_COUNT)
> +			return -EINVAL;
> +
> +		for (i = 0; i < num; i++) {
> +			if (!fdata[i].desc)
> +				return -EINVAL;
> +			data->function[i].type = fdata[i].desc->type;
> +			data->function[i].adr  = fdata[i].desc->adr;
> +			data->function[i].name = fdata[i].desc->name;
> +			data->function[i].node = NULL;

Probably simpler to just populate this directly from your drivers
probe function, seems odd to call this in both the class and the
function drivers, limits how it has to work.

> -	ret = sdca_parse_function(dev, core->sdw, drv->function);
> -	if (ret)
> -		return ret;
> +	if (drv->function->desc->node) {
> +		ret = sdca_parse_function(dev, core->sdw, drv->function);
> +		if (ret)
> +			return ret;
> +	} else if (core->hw_ops && core->hw_ops->get_function_data) {
> +		/*
> +		 * No DisCo/ACPI firmware node available (e.g. DT/ARM platform).
> +		 * Use pre-populated static function data supplied by the
> +		 * device-specific hw_ops instead of sdca_parse_function().
> +		 * The callback returns an array of function_data entries;
> +		 * pick the one matching this auxdev's function type.
> +		 */
> +		struct sdca_function_data *fdata;
> +		unsigned int num = 0;
> +		unsigned int i;
> +
> +		fdata = core->hw_ops->get_function_data(&num);
> +		if (!fdata || num == 0)
> +			return -EINVAL;
> +
> +		for (i = 0; i < num; i++) {
> +			if (fdata[i].desc &&
> +			    fdata[i].desc->type == sdev->function.desc->type) {
> +				*drv->function = fdata[i];
> +				drv->function->desc = sdev->function.desc;
> +				break;
> +			}
> +		}

Would be much nicer to have the callback directly populate the
function data, similar to how sdca_parse_function works.

Thanks,
Charles