Date:   Wed, 19 Jul 2023 06:43:47 +0000
From:   Biju Das <biju.das.jz@...renesas.com>
To:     Dmitry Torokhov <dmitry.torokhov@...il.com>
CC:     Mark Brown <broonie@...nel.org>,
        Mike Looijmans <mike.looijmans@...ic.nl>,
        Andreas Helbech Kleist <andreaskleist@...il.com>,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>,
        "linux-input@...r.kernel.org" <linux-input@...r.kernel.org>,
        Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@...renesas.com>,
        "linux-renesas-soc@...r.kernel.org" 
        <linux-renesas-soc@...r.kernel.org>,
        Wolfram Sang <wsa+renesas@...g-engineering.com>,
        Andy Shevchenko <andy.shevchenko@...il.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v2 1/2] Input: exc3000 - Simplify probe()

Hi Dmitry Torokhov,

Thanks for the feedback.

> Subject: Re: [PATCH v2 1/2] Input: exc3000 - Simplify probe()
> 
> On Mon, Jul 17, 2023 at 06:45:27PM +0000, Biju Das wrote:
> > Hi Dmitry,
> >
> > > Subject: Re: [PATCH v2 1/2] Input: exc3000 - Simplify probe()
> > >
> > > On Mon, Jul 17, 2023 at 07:15:50PM +0100, Mark Brown wrote:
> > > > On Mon, Jul 17, 2023 at 04:35:02PM +0000, Biju Das wrote:
> > > >
> > > > > The .device_get_match_data callbacks are missing for I2C and SPI
> > > > > bus
> > > subsystems.
> > > > > Can you please throw some lights on this?
> > > >
> > > > It's the first time I've ever heard of that callback, I don't know
> > > > why whoever added it wouldn't have done those buses in particular
> > > > or if it just didn't happen.  Try adding it and if it works send
> the patches?
> > >
> > > I think there is a disconnect. Right now device_get_match_data
> > > callbacks are part of fwnode_operations. I was proposing to add
> > > another optional device_get_match_data callback to 'struct bus_type'
> > > to allow individual buses control how match data is handled, before
> > > (or after) jumping into the fwnode-backed device_get_match_data
> callbacks.
> >
> > That is what implemented here [1] and [2] right?

[1] https://elixir.bootlin.com/linux/v6.5-rc2/source/drivers/spi/spi.c#L364

[2] https://elixir.bootlin.com/linux/v6.5-rc2/source/drivers/i2c/i2c-core-base.c#L117

> >
> >
> > First it check for fwnode-backed device_get_match_data callbacks and
> > Fallback is bus-type based match.
> >
> > Looks like you are proposing to unify [1] and [2] and you want the
> > logic to be other way around. ie, first bus-type match, then
> > fwnode-backed callbacks?
> >
> 
> I do not have a strong preference for the ordering, i.e. I think it is
> perfectly fine to do the generic fwnode-based lookup and if there is no
> match have bus method called as a fallback, 

That involves a bit of work.

const void *device_get_match_data(const struct device *dev);

const struct i2c_device_id *i2c_match_id(const struct i2c_device_id *id,
					 const struct i2c_client *client);

const struct spi_device_id *spi_get_device_id(const struct spi_device *sdev);

Basically, the bus-client driver(such as exc3000) needs to pass struct device
and device_get_match_data after generic fwnode-based lookup,
needs to find the bus type based on struct device and call a new generic 
void* bus_get_match_data(void*) callback, so that each bus interface
can do a match.

I am not sure, is this proposal acceptable to wider people??


> but I do not want driver
> writers to learn about multiple <bus-prefix>_get_match_data()
> implementations, I would prefer if they could call
> device_get_match_data() and the right thing happened in all cases.

The driver is bus specific. So I don't know, why you want to
be it generic. If it is i2c client, like other I2C api call the bus-subsystem api for match_data. Similarly, if it is spi client, do the same.

Cheers,
Biju

Powered by blists - more mailing lists