linux-kernel - Re: [PATCH v5 1/2] driver core: auxiliary bus: Introduce auxiliary device resource management

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <aCsLoWtIdfR5a2YS@smile.fi.intel.com>
Date: Mon, 19 May 2025 13:44:49 +0300
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: Raag Jadav <raag.jadav@...el.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 v5 1/2] driver core: auxiliary bus: Introduce auxiliary
 device resource management

On Fri, May 16, 2025 at 10:20:02PM +0300, Raag Jadav wrote:
> On Thu, May 15, 2025 at 04:06:47PM +0300, Andy Shevchenko wrote:
> > On Thu, May 15, 2025 at 03:52:38PM +0300, Raag Jadav wrote:
> > > On Wed, May 14, 2025 at 03:36:49PM +0300, Andy Shevchenko wrote:
> > > > On Wed, May 14, 2025 at 05:54:31PM +0530, Raag Jadav wrote:

...

> > > > > +int auxiliary_get_irq_optional(struct auxiliary_device *auxdev, unsigned int num)
> > > > > +{
> > > > > +	struct resource *r;
> > > > > +	int ret = -ENXIO;
> > > > > +
> > > > > +	r = auxiliary_get_resource(auxdev, IORESOURCE_IRQ, num);
> > > > > +	if (!r)
> > > > > +		goto out;
> > > > > +
> > > > > +	/*
> > > > > +	 * The resources may pass trigger flags to the irqs that need to be
> > > > > +	 * set up. It so happens that the trigger flags for IORESOURCE_BITS
> > > > > +	 * correspond 1-to-1 to the IRQF_TRIGGER* settings.
> > > > > +	 */
> > > > > +	if (r->flags & IORESOURCE_BITS) {
> > > > > +		struct irq_data *irqd;
> > > > > +
> > > > > +		irqd = irq_get_irq_data(r->start);
> > > > > +		if (!irqd)
> > > > > +			goto out;
> > > > > +		irqd_set_trigger_type(irqd, r->flags & IORESOURCE_BITS);
> > > > > +	}
> > > > > +
> > > > > +	ret = r->start;
> > > > > +	if (WARN(!ret, "0 is an invalid IRQ number\n"))
> > > > > +		ret = -EINVAL;
> > > > > +out:
> > > > > +	return ret;
> > > > > +}
> > > > 
> > > > Please, do not inherit the issues that the respective platform device API has.
> > > > And after all, why do you need this? What's wrong with plain fwnode_irq_get()?
> > > 
> > > Can you please elaborate? Are we expecting fwnode to be supported by auxiliary
> > > device?
> > 
> > Platform IRQ getter is legacy for the board files, but it has support for fwnode.
> > Why do you need to inherit all that legacy? What's the point?
> 
> This is just to abstract get_resource(IRQ) which has been carved up by the
> parent device. And since this is an auxiliary child device, I'm not sure if
> we have a firmware to work with.

To make get_resource() work, someone has to add those resources to the list.
The question is, why do we need this for AUX devices? Are you expecting
several IRQs to be dedicated for several devices (no sharing)? If now, why
is the fwnode version of IRQ getter not enough?

> Please correct me if I've misunderstood your question.

For the memory and port resources it might be indeed needed to have a split.
But then, since it's a lot of the copy from platform code, I would expect
the common library for both rather than reinventing the wheel. To achieve
that one might need to abstract away from the certain device container when
handling resources (no platform_device nor auxiliary_device). Would that
approach work?

-- 
With Best Regards,
Andy Shevchenko