Re: [PATCH 07/15] ASoC: SDCA: Rely less on the ASoC component in IRQ handling

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

On Tue, Sep 09, 2025 at 09:42:20AM +0800, Liao, Bard wrote:
> On 9/8/2025 10:43 PM, Charles Keepax wrote:
> > On Mon, Sep 08, 2025 at 01:56:57PM +0100, Charles Keepax wrote:
> >> On Mon, Sep 08, 2025 at 06:27:40PM +0800, Liao, Bard wrote:
> >>> On 9/5/2025 10:31 PM, Charles Keepax wrote:
> >>>> -int sdca_irq_data_populate(struct snd_soc_component *component,
> >>>> +int sdca_irq_data_populate(struct device *dev, struct regmap *regmap,
> >>>> +			   struct snd_soc_component *component,
> >>>>  			   struct sdca_function_data *function,
> >>>>  			   struct sdca_entity *entity,
> >>>>  			   struct sdca_control *control,
> >>>>  			   struct sdca_interrupt *interrupt)
> >>>>  {
> >>>> -	struct device *dev = component->dev;
> >>>
> >>> Previously, we assume 'component' will never be null.
> >>>
> >>>>  	const char *name;
> >>>>  
> >>>> +	if (!dev && component)
> >>>
> >>> But now we test 'component'. If we don't assume 'component' is not null
> >>> any more, we could test 'component' at the very beginning of this function.
> >>
> >> Good spot, that was missed when moving from an older version of
> >> the code. I will get that fixed up.
> > 
> > Apologies I had slightly misinterpretted this, and thought you
> > meant we were dereferencing component before testing it in
> > sdca_irq_data_populate, but we arn't.
> 
> No, my question is about whether component can be NULL.

The expectation would be component can not be NULL in
sdca_irq_populate(), but it could be NULL inside
sdca_irq_data_populate(), if called from somewhere else.

> Understood. I had the question because the change below.
> 
> +	interrupt->dev = dev;
> +	if (!regmap && component)
> +		interrupt->function_regmap = component->regmap;
> +	else
> +		interrupt->function_regmap = regmap;
> 
> If both regmap and component are null, the interrupt->function_regmap
> will be NULL. Is it valid?

Ok... yeah I see. I guess it depends on the IRQ handler that
ultimately gets registered. In most cases I would expect the
handler to need a regmap, however, we don't really validate
any of the data that is purely for the handler. For example we
don't validate function/entity/control either, we check dev for
NULL because it is used for devm allocation of the name string,
not because it is data for the handler. I think doing it this
way saves us making assumptions, once we start doing so we start
to constraint what can be done through the API.

Thanks,
Charles