Re: [PATCH v2 10/19] ASoC: SDCA: Parse XU Entity properties

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

On Wed, Sep 17, 2025 at 08:58:51PM +0200, Pierre-Louis Bossart wrote:
> 
> > +/**
> > + * enum sdca_xu_reset_machanism - SDCA FDL Resets
> > + */
> > +enum sdca_xu_reset_mechanism {
> > +	SDCA_XU_RESET_FUNCTION				= 0x0,
> > +	SDCA_XU_RESET_DEVICE				= 0x1,
> > +	SDCA_XU_RESET_BUS				= 0x2,
> > +};
> 
> It'd be worth explaining how the last two might work? The
> RESET_BUS is puzzling, if the controller performs a hard reset
> then in theory the device should lose all context. Likewise
> a RESET_DEVICE should cause the SoundWire device to fall
> off the bus and lose context as well, and in the case of a
> multi-function device it could be fun if each function causes
> a device reset. This could end-up in a boot-loop, no?

I mean yeah that one seems problematic to implement to me, but
table 322 in v1.1 defines these three resets. Its up to the
implementation of the device to pick one that works for it.

> > +static int find_sdca_entity_xu(struct device *dev,
> > +			       struct fwnode_handle *entity_node,
> > +			       struct sdca_entity *entity)
> > +{
> > +	struct sdca_entity_xu *xu = &entity->xu;
> > +	u32 tmp;
> > +	int ret;
> > +
> > +	ret = fwnode_property_read_u32(entity_node,
> > +				       "mipi-sdca-RxUMP-ownership-transition-max-delay",
> > +				       &tmp);
> > +	if (!ret)
> > +		xu->max_delay = tmp;
> 
> maybe add a sanity check on the value?
> 
> > +	ret = fwnode_property_read_u32(entity_node, "mipi-sdca-FDL-reset-mechanism",
> > +				       &tmp);
> > +	if (!ret)
> > +		xu->reset_mechanism = tmp;
> 
> same here?

None of the existing delays are sanity checking, so I think I
would vote for leaving this as future work. It would make more
sense to add it to all delays rather than just the subset we are
adding there.

Thanks,
Charles