[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZLsCOj1t4JvG3SEp@google.com>
Date: Fri, 21 Jul 2023 15:10:02 -0700
From: Dmitry Torokhov <dmitry.torokhov@...il.com>
To: Biju Das <biju.das.jz@...renesas.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()
On Wed, Jul 19, 2023 at 06:43:47AM +0000, Biju Das wrote:
> 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.
Yes, something like this (which does not seem that involved to me...):
diff --git a/drivers/base/property.c b/drivers/base/property.c
index 8c40abed7852..cc0bf7bb6f3a 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -1277,7 +1277,13 @@ EXPORT_SYMBOL(fwnode_graph_parse_endpoint);
const void *device_get_match_data(const struct device *dev)
{
- return fwnode_call_ptr_op(dev_fwnode(dev), device_get_match_data, dev);
+ const void *data;
+
+ data = fwnode_call_ptr_op(dev_fwnode(dev), device_get_match_data, dev);
+ if (!data && dev->bus && dev->bus->get_match_data)
+ data = dev->bus->get_match_data(dev);
+
+ return data;
}
EXPORT_SYMBOL_GPL(device_get_match_data);
diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 60746652fd52..5fe47bc491a6 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -114,6 +114,26 @@ const struct i2c_device_id *i2c_match_id(const struct i2c_device_id *id,
}
EXPORT_SYMBOL_GPL(i2c_match_id);
+static const void *i2c_device_get_match_data(const struct device *dev)
+{
+ const struct i2c_client *client = to_i2c_client(dev);
+ const struct i2c_driver *driver;
+ const struct i2c_device_id *match;
+
+ if (!dev->driver)
+ return NULL;
+
+ driver = to_i2c_driver(dev->driver);
+ if (!driver)
+ return NULL;
+
+ match = i2c_match_id(driver->id_table, client);
+ if (!match)
+ return NULL;
+
+ return (const void *)match->driver_data;
+}
+
const void *i2c_get_match_data(const struct i2c_client *client)
{
struct i2c_driver *driver = to_i2c_driver(client->dev.driver);
@@ -695,6 +715,7 @@ struct bus_type i2c_bus_type = {
.probe = i2c_device_probe,
.remove = i2c_device_remove,
.shutdown = i2c_device_shutdown,
+ .get_match_data = i2c_device_get_match_data,
};
EXPORT_SYMBOL_GPL(i2c_bus_type);
diff --git a/include/linux/device/bus.h b/include/linux/device/bus.h
index ae10c4322754..3f2cba28a1af 100644
--- a/include/linux/device/bus.h
+++ b/include/linux/device/bus.h
@@ -102,6 +102,8 @@ struct bus_type {
int (*dma_configure)(struct device *dev);
void (*dma_cleanup)(struct device *dev);
+ const void *(*get_match_data)(const struct device *dev);
+
const struct dev_pm_ops *pm;
const struct iommu_ops *iommu_ops;
Thanks.
--
Dmitry
Powered by blists - more mailing lists