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
