Re: [PATCH v2 2/7] soundwire: intel: Allow for dynamic assignment of PDIs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] soundwire: intel: sdw_intel_pdi_config() adds separate layer over DAI ops
--

commit e83fbc567de1bac187beea3bd11c5b0304d25e97
Author: Cezary Rojewski <cezary.rojewski@intel.com>

soundwire: intel: Allow for dynamic assignment of PDIs

This commit introduces sdw_cdns_assign_pdi() and sdw_cdns_release_pdi() for
dynamic PDI assignment, along with sdw_intel_pdi_config() to configure PDIs
separately from assignment.

> diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
> index af65214836b4d..d53fa3d299bc7 100644
> --- a/drivers/soundwire/intel.c
> +++ b/drivers/soundwire/intel.c
> @@ -674,6 +674,22 @@ intel_pdi_alh_configure(struct sdw_intel *sdw, struct sdw_cdns_pdi *pdi)
>  	intel_writel(alh, SDW_ALH_STRMZCFG(pdi->intel_alh_id), conf);
>  }
>  
> +/**
> + * sdw_intel_pdi_config - Configure specified PDI
> + * @bus: SoundWire bus instance
> + * @pdi: PDI instance to configure
> + */
> +void sdw_intel_pdi_config(struct sdw_bus *bus, struct sdw_cdns_pdi *pdi)
> +{
> +	struct sdw_cdns *cdns = bus_to_cdns(bus);
> +	struct sdw_intel *sdw = cdns_to_intel(cdns);
> +
> +	intel_pdi_shim_configure(sdw, pdi);
> +	intel_pdi_alh_configure(sdw, pdi);
> +	sdw_cdns_config_stream(cdns, pdi->ch_count, pdi->dir, pdi);
> +}
> +EXPORT_SYMBOL(sdw_intel_pdi_config);

[Severity: Medium]
As reported by Vinod Koul on v1 [1], exporting sdw_intel_pdi_config() adds a
separate layer in drivers/soundwire/intel.c for external driver-specific DAI
handling while leaving the existing DAI operations in intel_hw_params()
unchanged:

drivers/soundwire/intel.c:intel_hw_params() {
    ...
	pdi = sdw_cdns_alloc_pdi(cdns, &cdns->pcm, ch, dir, dai->id);

	if (!pdi)
		return -EINVAL;

	/* do run-time configurations for SHIM, ALH and PDI/PORT */
	intel_pdi_shim_configure(sdw, pdi);
	intel_pdi_alh_configure(sdw, pdi);
	sdw_cdns_config_stream(cdns, ch, dir, pdi);
    ...
}

Instead of exporting sdw_intel_pdi_config() for external callers while
intel_hw_params() continues to allocate PDIs statically by DAI ID and call
intel_pdi_shim_configure(), intel_pdi_alh_configure(), and
sdw_cdns_config_stream() directly, could the SoundWire Intel driver itself be
updated to handle both startup and hw_params uniformly across SOF and AVS
drivers?

[1] https://lore.kernel.org/r/aulzN9MCl5CJB1Al@parshuram

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103134958.2089136-1-cezary.rojewski@intel.com?part=2