Re: [PATCH v3] mfd: core: Support auxiliary device

[Raag Jadav <raag.jadav@intel.com>]

On Sat, Apr 19, 2025 at 06:28:28PM +0300, Andy Shevchenko wrote:
> On Fri, Apr 18, 2025 at 06:04:33PM +0530, Raag Jadav wrote:
> > Extend MFD subsystem to support auxiliary child device. This is useful
> > for MFD usecases where parent device is on a discoverable bus and doesn't
> > fit into the platform device criteria. Purpose of this implementation is
> > to provide discoverable MFDs just enough infrastructure to register
> > independent child devices with their own memory and interrupt resources
> > without abusing the platform device.
> > 
> > Current support is limited to just PCI type MFDs, but this can be further
> > extended to support other types like USB in the future.
> 
> ...
> 
> >  config MFD_CORE
> >  	tristate
> > +	select AUXILIARY_BUS
> >  	select IRQ_DOMAIN
> 
> >  	default n
> 
> Can be dropped. It's the default 'default' and with user invisible
> configuration, it makes even less sense.

Sure.

> > +/*
> > + * Common structure between MFD parent and auxiliary child device.
> > + * To be used by leaf drivers to access child device resources.
> > + */
> > +struct mfd_aux_device {
> > +	struct auxiliary_device auxdev;
> > +	struct resource	mem;
> > +	struct resource	irq;
> > +	/* Place holder for other types */
> > +	struct resource	ext;
> 
> I don't like it. It's no-scalable solution, just make it VLA. Each resource
> already has type in case one needs to extract an exact one, also this won't
> work in case of the two resources for memory or IRQ needed.

Agree, but is it worth introducing a whole new set of get_resource() and
friends? I'm not sure if the maintainers will be okay with it.

> > +/* TODO: Convert the platform device abusers and remove this flag */
> > +#define MFD_AUX_TYPE	BIT(31)
> 
> Yeah, LKP wants this to be signed... Or 'id' to be unsigned.

I'll revert it back to INT_MIN, a specific number would rather become more
questionable.

Raag