linux-kernel - Re: [PATCH v3] mfd: core: Support auxiliary device

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aASSHasej5Lme0Wc@black.fi.intel.com>
Date: Sun, 20 Apr 2025 09:20:13 +0300
From: Raag Jadav <raag.jadav@...el.com>
To: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc: gregkh@...uxfoundation.org, david.m.ertman@...el.com,
	ira.weiny@...el.com, lee@...nel.org,
	mika.westerberg@...ux.intel.com, heikki.krogerus@...ux.intel.com,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] mfd: core: Support auxiliary device

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