* [PATCH v2] i2c: core: make it possible to match a pure device tree driver @ 2013-05-13 20:18 Linus Walleij [not found] ` <1368476301-10495-1-git-send-email-linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> 0 siblings, 1 reply; 18+ messages in thread From: Linus Walleij @ 2013-05-13 20:18 UTC (permalink / raw) To: Wolfram Sang, linux-i2c-u79uwXL29TY76Z2rM5mHXA, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ Cc: Linus Walleij, Rob Herring, Grant Likely This tries to address an issue found when writing an MFD driver for the Nomadik STw481x PMICs: as the platform is using device tree exclusively I want to specify the driver matching like this: static const struct of_device_id stw481x_match[] = { { .compatible = "st,stw4810", }, { .compatible = "st,stw4811", }, {}, }; static struct i2c_driver stw481x_driver = { .driver = { .name = "stw481x", .of_match_table = stw481x_match, }, .probe = stw481x_probe, .remove = stw481x_remove, }; However that turns out not to be possible: the I2C probe code is written so that the probe() call is always passed a match from i2c_match_id() using non-devicetree matches. This is probably why most devices using device tree for I2C clients currently will pass no .of_match_table *at all* but instead just use .id_table from struct i2c_driver to match the device. As you realize that means that the whole idea with compatible strings is discarded, and that is why we find strange device tree I2C device compatible strings like "product" instead of "vendor,product" as you could expect. Let's figure out how to fix this before the mess spreads. This patch will allow probeing devices with only an of_match_table as per above, and will pass NULL as the second argument to the probe() function. If the driver wants to deduce secondary info from the struct of_device_id .data field, it has to call of_match_device() on its own match table in the probe function device tree probe path. If drivers define both an .of_match_table *AND* a i2c_driver .id_table, the .of_match_table will take precedence, just as is done in the i2c_device_match() function in i2c-core.c. I2C devices probed from device tree should subsequently be fixed to handle the case where of_match_table() is used (I think none of them do that today), and platforms should fix their device trees to use compatible strings for I2C devices instead of setting the name to Linux device driver names as is done in multiple cases today. Cc: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org> Cc: Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> Signed-off-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> --- ChangeLog v1->v2: - Use of_match_device() to determine if there is a DT match in the probe code. If there is a match we pass NULL for the id_table match parameter. --- drivers/i2c/i2c-core.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c index 6b63cc7..17cd22a 100644 --- a/drivers/i2c/i2c-core.c +++ b/drivers/i2c/i2c-core.c @@ -240,7 +240,7 @@ static int i2c_device_probe(struct device *dev) return 0; driver = to_i2c_driver(dev->driver); - if (!driver->probe || !driver->id_table) + if (!driver->probe || (!driver->id_table && !dev->driver->of_match_table)) return -ENODEV; client->driver = driver; if (!device_can_wakeup(&client->dev)) @@ -248,7 +248,12 @@ static int i2c_device_probe(struct device *dev) client->flags & I2C_CLIENT_WAKE); dev_dbg(dev, "probe\n"); - status = driver->probe(client, i2c_match_id(driver->id_table, client)); + if (of_match_device(dev->driver->of_match_table, dev)) + /* Device tree matching */ + status = driver->probe(client, NULL); + else + /* Fall back to matching the id_table */ + status = driver->probe(client, i2c_match_id(driver->id_table, client)); if (status) { client->driver = NULL; i2c_set_clientdata(client, NULL); -- 1.8.1.4 ^ permalink raw reply related [flat|nested] 18+ messages in thread
[parent not found: <1368476301-10495-1-git-send-email-linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>]
* Re: [PATCH v2] i2c: core: make it possible to match a pure device tree driver [not found] ` <1368476301-10495-1-git-send-email-linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> @ 2013-05-22 7:56 ` Linus Walleij [not found] ` <CACRpkdae1chj9ZKuQV5MuiaicLUTKr4hZqr8rtjbB3Yjt+U_fg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2013-06-17 15:48 ` Stephen Warren 2013-06-07 21:32 ` Wolfram Sang 2013-06-12 17:09 ` Wolfram Sang 2 siblings, 2 replies; 18+ messages in thread From: Linus Walleij @ 2013-05-22 7:56 UTC (permalink / raw) To: Wolfram Sang, linux-i2c-u79uwXL29TY76Z2rM5mHXA, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ Cc: Linus Walleij, Rob Herring, Grant Likely On Mon, May 13, 2013 at 10:18 PM, Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote: > This tries to address an issue found when writing an MFD driver > for the Nomadik STw481x PMICs: as the platform is using device > tree exclusively I want to specify the driver matching like > this: (...) > --- > ChangeLog v1->v2: > - Use of_match_device() to determine if there is a DT match in > the probe code. If there is a match we pass NULL for the > id_table match parameter. Ping on this. v2 should be doing what Grant suggested... Yours, Linus Walleij ^ permalink raw reply [flat|nested] 18+ messages in thread
[parent not found: <CACRpkdae1chj9ZKuQV5MuiaicLUTKr4hZqr8rtjbB3Yjt+U_fg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH v2] i2c: core: make it possible to match a pure device tree driver [not found] ` <CACRpkdae1chj9ZKuQV5MuiaicLUTKr4hZqr8rtjbB3Yjt+U_fg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2013-05-30 8:00 ` Linus Walleij 0 siblings, 0 replies; 18+ messages in thread From: Linus Walleij @ 2013-05-30 8:00 UTC (permalink / raw) To: Wolfram Sang, linux-i2c-u79uwXL29TY76Z2rM5mHXA, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ Cc: Linus Walleij, Rob Herring, Grant Likely On Wed, May 22, 2013 at 9:56 AM, Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote: > On Mon, May 13, 2013 at 10:18 PM, Linus Walleij > <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote: > >> This tries to address an issue found when writing an MFD driver >> for the Nomadik STw481x PMICs: as the platform is using device >> tree exclusively I want to specify the driver matching like >> this: > (...) >> --- >> ChangeLog v1->v2: >> - Use of_match_device() to determine if there is a DT match in >> the probe code. If there is a match we pass NULL for the >> id_table match parameter. > > Ping on this. > > v2 should be doing what Grant suggested... Ping, nocheinmal. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] i2c: core: make it possible to match a pure device tree driver 2013-05-22 7:56 ` Linus Walleij [not found] ` <CACRpkdae1chj9ZKuQV5MuiaicLUTKr4hZqr8rtjbB3Yjt+U_fg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2013-06-17 15:48 ` Stephen Warren [not found] ` <51BF2FC5.40207-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> 1 sibling, 1 reply; 18+ messages in thread From: Stephen Warren @ 2013-06-17 15:48 UTC (permalink / raw) To: Linus Walleij, Wolfram Sang Cc: linux-i2c, devicetree-discuss, Grant Likely, Rob Herring, linux-next On 05/22/2013 01:56 AM, Linus Walleij wrote: > On Mon, May 13, 2013 at 10:18 PM, Linus Walleij > <linus.walleij@linaro.org> wrote: > >> This tries to address an issue found when writing an MFD driver >> for the Nomadik STw481x PMICs: as the platform is using device >> tree exclusively I want to specify the driver matching like >> this: > (...) >> --- >> ChangeLog v1->v2: >> - Use of_match_device() to determine if there is a DT match in >> the probe code. If there is a match we pass NULL for the >> id_table match parameter. > > Ping on this. > > v2 should be doing what Grant suggested... This has just shown up in next-20130617, and breaks at least the TPS65910 and TPS62360 drivers, since they assume that the id parameter passed to probe is non-NULL. However, now the parameter is NULL since these drivers have both an ID table and an OF match table. I'd like to suggest this patch be reverted an re-introduced immediately after the merge window. That should give enough time for everyone to get a heads-up on fixing any drivers with a similar problem, rather than trying to cram all that in immediately before the merge window. I'd also suggest that this patch should be accompanied by fixes for any similarly broken drivers, based on code inspection. Do people agree? If not, please let us know ASAP so we can post patches to fix this. ^ permalink raw reply [flat|nested] 18+ messages in thread
[parent not found: <51BF2FC5.40207-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>]
* Re: [PATCH v2] i2c: core: make it possible to match a pure device tree driver [not found] ` <51BF2FC5.40207-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> @ 2013-06-17 16:33 ` Linus Walleij [not found] ` <CACRpkdbjy+PcpNkSg4PuuP2Z9RyrnqSW0uchHFHVZ8VYPeoDyA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 18+ messages in thread From: Linus Walleij @ 2013-06-17 16:33 UTC (permalink / raw) To: Stephen Warren Cc: Wolfram Sang, linux-i2c-u79uwXL29TY76Z2rM5mHXA, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Grant Likely, Rob Herring, linux-next-u79uwXL29TY76Z2rM5mHXA On Mon, Jun 17, 2013 at 5:48 PM, Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote: > This has just shown up in next-20130617, and breaks at least the > TPS65910 and TPS62360 drivers, since they assume that the id parameter > passed to probe is non-NULL. However, now the parameter is NULL since > these drivers have both an ID table and an OF match table. So you mean they come in through the DT boot path and assume that parameter is non-null even though they should not make use of it? > I'd like to suggest this patch be reverted an re-introduced immediately > after the merge window. That should give enough time for everyone to get > a heads-up on fixing any drivers with a similar problem, rather than > trying to cram all that in immediately before the merge window. OK that works for me, I'm not in any hurry. Wolfram get to decide how to handle this... Yours, Linus Walleij ^ permalink raw reply [flat|nested] 18+ messages in thread
[parent not found: <CACRpkdbjy+PcpNkSg4PuuP2Z9RyrnqSW0uchHFHVZ8VYPeoDyA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH v2] i2c: core: make it possible to match a pure device tree driver [not found] ` <CACRpkdbjy+PcpNkSg4PuuP2Z9RyrnqSW0uchHFHVZ8VYPeoDyA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2013-06-17 16:49 ` Stephen Warren 2013-06-17 22:15 ` Grant Likely 1 sibling, 0 replies; 18+ messages in thread From: Stephen Warren @ 2013-06-17 16:49 UTC (permalink / raw) To: Linus Walleij Cc: Wolfram Sang, linux-i2c-u79uwXL29TY76Z2rM5mHXA, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Grant Likely, Rob Herring, linux-next-u79uwXL29TY76Z2rM5mHXA On 06/17/2013 10:33 AM, Linus Walleij wrote: > On Mon, Jun 17, 2013 at 5:48 PM, Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote: > >> This has just shown up in next-20130617, and breaks at least the >> TPS65910 and TPS62360 drivers, since they assume that the id parameter >> passed to probe is non-NULL. However, now the parameter is NULL since >> these drivers have both an ID table and an OF match table. > > So you mean they come in through the DT boot path and assume > that parameter is non-null even though they should not make use of > it? Yes. Since this wasn't true/enforced before, it probably wasn't clear that rule existed. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] i2c: core: make it possible to match a pure device tree driver [not found] ` <CACRpkdbjy+PcpNkSg4PuuP2Z9RyrnqSW0uchHFHVZ8VYPeoDyA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2013-06-17 16:49 ` Stephen Warren @ 2013-06-17 22:15 ` Grant Likely [not found] ` <CACxGe6sTq=h9jjqesjvUA4Bweh7zgE4VOLoOTgGPC_D_v+8NtA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2013-06-18 7:33 ` Wolfram Sang 1 sibling, 2 replies; 18+ messages in thread From: Grant Likely @ 2013-06-17 22:15 UTC (permalink / raw) To: Linus Walleij Cc: Stephen Warren, Wolfram Sang, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Rob Herring, linux-next-u79uwXL29TY76Z2rM5mHXA, linux-i2c-u79uwXL29TY76Z2rM5mHXA On Mon, Jun 17, 2013 at 5:33 PM, Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote: > On Mon, Jun 17, 2013 at 5:48 PM, Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote: > >> This has just shown up in next-20130617, and breaks at least the >> TPS65910 and TPS62360 drivers, since they assume that the id parameter >> passed to probe is non-NULL. However, now the parameter is NULL since >> these drivers have both an ID table and an OF match table. > > So you mean they come in through the DT boot path and assume > that parameter is non-null even though they should not make use of > it? > >> I'd like to suggest this patch be reverted an re-introduced immediately >> after the merge window. That should give enough time for everyone to get >> a heads-up on fixing any drivers with a similar problem, rather than >> trying to cram all that in immediately before the merge window. > > OK that works for me, I'm not in any hurry. Deferring by a merge window isn't going to make it any less painful. Do your best to find all the users that need to be changed. Use a coccinelle search perhaps, but I think it should be merged anyway. g. ^ permalink raw reply [flat|nested] 18+ messages in thread
[parent not found: <CACxGe6sTq=h9jjqesjvUA4Bweh7zgE4VOLoOTgGPC_D_v+8NtA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH v2] i2c: core: make it possible to match a pure device tree driver [not found] ` <CACxGe6sTq=h9jjqesjvUA4Bweh7zgE4VOLoOTgGPC_D_v+8NtA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2013-06-17 23:25 ` Stephen Warren 0 siblings, 0 replies; 18+ messages in thread From: Stephen Warren @ 2013-06-17 23:25 UTC (permalink / raw) To: Grant Likely Cc: Linus Walleij, Wolfram Sang, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Rob Herring, linux-next-u79uwXL29TY76Z2rM5mHXA, linux-i2c-u79uwXL29TY76Z2rM5mHXA On 06/17/2013 04:15 PM, Grant Likely wrote: > On Mon, Jun 17, 2013 at 5:33 PM, Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote: >> On Mon, Jun 17, 2013 at 5:48 PM, Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote: >> >>> This has just shown up in next-20130617, and breaks at least the >>> TPS65910 and TPS62360 drivers, since they assume that the id parameter >>> passed to probe is non-NULL. However, now the parameter is NULL since >>> these drivers have both an ID table and an OF match table. >> >> So you mean they come in through the DT boot path and assume >> that parameter is non-null even though they should not make use of >> it? >> >>> I'd like to suggest this patch be reverted an re-introduced immediately >>> after the merge window. That should give enough time for everyone to get >>> a heads-up on fixing any drivers with a similar problem, rather than >>> trying to cram all that in immediately before the merge window. >> >> OK that works for me, I'm not in any hurry. > > Deferring by a merge window isn't going to make it any less painful. It'll give a lot more time for people to be exposed to the change and hence find/fix it linux-next rather than seeing it for the first time in Linus's tree. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] i2c: core: make it possible to match a pure device tree driver 2013-06-17 22:15 ` Grant Likely [not found] ` <CACxGe6sTq=h9jjqesjvUA4Bweh7zgE4VOLoOTgGPC_D_v+8NtA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2013-06-18 7:33 ` Wolfram Sang 2013-06-18 7:44 ` Linus Walleij 1 sibling, 1 reply; 18+ messages in thread From: Wolfram Sang @ 2013-06-18 7:33 UTC (permalink / raw) To: Grant Likely Cc: Linus Walleij, Stephen Warren, devicetree-discuss, Rob Herring, linux-next, linux-i2c [-- Attachment #1: Type: text/plain, Size: 1317 bytes --] On Mon, Jun 17, 2013 at 11:15:30PM +0100, Grant Likely wrote: > On Mon, Jun 17, 2013 at 5:33 PM, Linus Walleij <linus.walleij@linaro.org> wrote: > > On Mon, Jun 17, 2013 at 5:48 PM, Stephen Warren <swarren@wwwdotorg.org> wrote: > > > >> This has just shown up in next-20130617, and breaks at least the > >> TPS65910 and TPS62360 drivers, since they assume that the id parameter > >> passed to probe is non-NULL. However, now the parameter is NULL since > >> these drivers have both an ID table and an OF match table. > > > > So you mean they come in through the DT boot path and assume > > that parameter is non-null even though they should not make use of > > it? > > > >> I'd like to suggest this patch be reverted an re-introduced immediately > >> after the merge window. That should give enough time for everyone to get > >> a heads-up on fixing any drivers with a similar problem, rather than > >> trying to cram all that in immediately before the merge window. > > > > OK that works for me, I'm not in any hurry. > > Deferring by a merge window isn't going to make it any less painful. > Do your best to find all the users that need to be changed. Use a > coccinelle search perhaps, but I think it should be merged anyway. I'll try a bit of my coccinelle-foo today and then decide. [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] i2c: core: make it possible to match a pure device tree driver 2013-06-18 7:33 ` Wolfram Sang @ 2013-06-18 7:44 ` Linus Walleij [not found] ` <CACRpkdakdUVzHM4L5qOJ5Ls8NwOimy197NCUDSP=X28d2fzHPg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 18+ messages in thread From: Linus Walleij @ 2013-06-18 7:44 UTC (permalink / raw) To: Wolfram Sang Cc: Grant Likely, Stephen Warren, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Rob Herring, linux-next-u79uwXL29TY76Z2rM5mHXA, linux-i2c-u79uwXL29TY76Z2rM5mHXA On Tue, Jun 18, 2013 at 9:33 AM, Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> wrote: > On Mon, Jun 17, 2013 at 11:15:30PM +0100, Grant Likely wrote: >> On Mon, Jun 17, 2013 at 5:33 PM, Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote: >> > OK that works for me, I'm not in any hurry. >> >> Deferring by a merge window isn't going to make it any less painful. >> Do your best to find all the users that need to be changed. Use a >> coccinelle search perhaps, but I think it should be merged anyway. > > I'll try a bit of my coccinelle-foo today and then decide. Thanks Wolfram, much appreciated. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 18+ messages in thread
[parent not found: <CACRpkdakdUVzHM4L5qOJ5Ls8NwOimy197NCUDSP=X28d2fzHPg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH v2] i2c: core: make it possible to match a pure device tree driver [not found] ` <CACRpkdakdUVzHM4L5qOJ5Ls8NwOimy197NCUDSP=X28d2fzHPg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2013-06-18 15:55 ` Wolfram Sang 0 siblings, 0 replies; 18+ messages in thread From: Wolfram Sang @ 2013-06-18 15:55 UTC (permalink / raw) To: Linus Walleij Cc: Grant Likely, Stephen Warren, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Rob Herring, linux-next-u79uwXL29TY76Z2rM5mHXA, linux-i2c-u79uwXL29TY76Z2rM5mHXA [-- Attachment #1: Type: text/plain, Size: 1202 bytes --] On Tue, Jun 18, 2013 at 09:44:17AM +0200, Linus Walleij wrote: > On Tue, Jun 18, 2013 at 9:33 AM, Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> wrote: > > On Mon, Jun 17, 2013 at 11:15:30PM +0100, Grant Likely wrote: > >> On Mon, Jun 17, 2013 at 5:33 PM, Linus Walleij <linus.walleij@linaro.org> wrote: > >> > OK that works for me, I'm not in any hurry. > >> > >> Deferring by a merge window isn't going to make it any less painful. > >> Do your best to find all the users that need to be changed. Use a > >> coccinelle search perhaps, but I think it should be merged anyway. > > > > I'll try a bit of my coccinelle-foo today and then decide. > > Thanks Wolfram, much appreciated. I am going to revert that commit. I was thinking back and forth, even playing with the idea to remove the id as a parameter to probe for i2c drivers and let them request the id from the i2c core when needed. But now I found more side-effects. E.g. run-time based instantiation for i2c devices is depending on an id-table. So, for now I keep insisting that an id-table must exist. Looks like DT-only drivers need more thinking and this is too late for 3.11. Regards, Wolfram [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] i2c: core: make it possible to match a pure device tree driver [not found] ` <1368476301-10495-1-git-send-email-linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> 2013-05-22 7:56 ` Linus Walleij @ 2013-06-07 21:32 ` Wolfram Sang 2013-06-10 12:18 ` Linus Walleij 2013-06-12 13:20 ` Grant Likely 2013-06-12 17:09 ` Wolfram Sang 2 siblings, 2 replies; 18+ messages in thread From: Wolfram Sang @ 2013-06-07 21:32 UTC (permalink / raw) To: Linus Walleij Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Rob Herring, Grant Likely [-- Attachment #1: Type: text/plain, Size: 606 bytes --] ... > I2C devices probed from device tree should subsequently be > fixed to handle the case where of_match_table() is > used (I think none of them do that today), and platforms should > fix their device trees to use compatible strings for I2C devices > instead of setting the name to Linux device driver names as is > done in multiple cases today. I guess your solution is the least intrusive one. Still, it could happen that of_match_table is scanned three times (by driver core, i2c layer, and i2c driver) which is IMO an indication to look for a more elegant solution tp find out what really matched? [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] i2c: core: make it possible to match a pure device tree driver 2013-06-07 21:32 ` Wolfram Sang @ 2013-06-10 12:18 ` Linus Walleij [not found] ` <CACRpkdbBsXB+9wN0oshfZB8Xady9G_wu9d8UkgA-bYqQe=8Mpw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2013-06-12 13:20 ` Grant Likely 1 sibling, 1 reply; 18+ messages in thread From: Linus Walleij @ 2013-06-10 12:18 UTC (permalink / raw) To: Wolfram Sang, Grant Likely, Rob Herring, Benjamin Herrenschmidt Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ On Fri, Jun 7, 2013 at 11:32 PM, Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> wrote: > ... >> I2C devices probed from device tree should subsequently be >> fixed to handle the case where of_match_table() is >> used (I think none of them do that today), and platforms should >> fix their device trees to use compatible strings for I2C devices >> instead of setting the name to Linux device driver names as is >> done in multiple cases today. > > I guess your solution is the least intrusive one. Still, it could happen > that of_match_table is scanned three times (by driver core, i2c layer, > and i2c driver) which is IMO an indication to look for a more elegant > solution tp find out what really matched? I think that is a generic problem with the device tree being completely stateless, and rather a comment on the of_match_device() intrinsics being inelegant than on this patch? Do you see it as a blocker for the patch? What happens before this patch is that instead of looping over the of_match_table, the id_table is always iterated to the end also in the DT case, yielding NULL as the last argument to driver->probe() anyway. Maybe the OF people have some comments on this... I cannot really see how it could be any different given the way the FDT works :-/ Yours, Linus Walleij ^ permalink raw reply [flat|nested] 18+ messages in thread
[parent not found: <CACRpkdbBsXB+9wN0oshfZB8Xady9G_wu9d8UkgA-bYqQe=8Mpw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH v2] i2c: core: make it possible to match a pure device tree driver [not found] ` <CACRpkdbBsXB+9wN0oshfZB8Xady9G_wu9d8UkgA-bYqQe=8Mpw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2013-06-10 13:40 ` Wolfram Sang 2013-06-12 13:22 ` Grant Likely 0 siblings, 1 reply; 18+ messages in thread From: Wolfram Sang @ 2013-06-10 13:40 UTC (permalink / raw) To: Linus Walleij Cc: Grant Likely, Rob Herring, Benjamin Herrenschmidt, linux-i2c-u79uwXL29TY76Z2rM5mHXA, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ [-- Attachment #1: Type: text/plain, Size: 1257 bytes --] On Mon, Jun 10, 2013 at 02:18:17PM +0200, Linus Walleij wrote: > On Fri, Jun 7, 2013 at 11:32 PM, Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> wrote: > > ... > >> I2C devices probed from device tree should subsequently be > >> fixed to handle the case where of_match_table() is > >> used (I think none of them do that today), and platforms should > >> fix their device trees to use compatible strings for I2C devices > >> instead of setting the name to Linux device driver names as is > >> done in multiple cases today. > > > > I guess your solution is the least intrusive one. Still, it could happen > > that of_match_table is scanned three times (by driver core, i2c layer, > > and i2c driver) which is IMO an indication to look for a more elegant > > solution tp find out what really matched? > > I think that is a generic problem with the device tree > being completely stateless, and rather a comment on the > of_match_device() intrinsics being inelegant than on this > patch? Yes. > Do you see it as a blocker for the patch? No blocker. Yet, I was hoping for somebody perhaps having a good idea. Platform devices have 'id_entry', for example. Sadly, I don't have the resources to pick up a similar idea. [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] i2c: core: make it possible to match a pure device tree driver 2013-06-10 13:40 ` Wolfram Sang @ 2013-06-12 13:22 ` Grant Likely 0 siblings, 0 replies; 18+ messages in thread From: Grant Likely @ 2013-06-12 13:22 UTC (permalink / raw) To: Wolfram Sang, Linus Walleij Cc: Rob Herring, Benjamin Herrenschmidt, linux-i2c-u79uwXL29TY76Z2rM5mHXA, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ On Mon, 10 Jun 2013 15:40:14 +0200, Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> wrote: > On Mon, Jun 10, 2013 at 02:18:17PM +0200, Linus Walleij wrote: > > On Fri, Jun 7, 2013 at 11:32 PM, Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> wrote: > > > ... > > >> I2C devices probed from device tree should subsequently be > > >> fixed to handle the case where of_match_table() is > > >> used (I think none of them do that today), and platforms should > > >> fix their device trees to use compatible strings for I2C devices > > >> instead of setting the name to Linux device driver names as is > > >> done in multiple cases today. > > > > > > I guess your solution is the least intrusive one. Still, it could happen > > > that of_match_table is scanned three times (by driver core, i2c layer, > > > and i2c driver) which is IMO an indication to look for a more elegant > > > solution tp find out what really matched? > > > > I think that is a generic problem with the device tree > > being completely stateless, and rather a comment on the > > of_match_device() intrinsics being inelegant than on this > > patch? > > Yes. > > > Do you see it as a blocker for the patch? > > No blocker. Yet, I was hoping for somebody perhaps having a good idea. > Platform devices have 'id_entry', for example. Sadly, I don't have the > resources to pick up a similar idea. > I did at one time have a patch that kept the pointer around in the struct device if an of_match_table succeeded, but it caused subtle race conditions that weren't easy to solve. I reverted back to just calling of_match_table() several times because it was easy. g. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] i2c: core: make it possible to match a pure device tree driver 2013-06-07 21:32 ` Wolfram Sang 2013-06-10 12:18 ` Linus Walleij @ 2013-06-12 13:20 ` Grant Likely 2013-06-13 9:02 ` Linus Walleij 1 sibling, 1 reply; 18+ messages in thread From: Grant Likely @ 2013-06-12 13:20 UTC (permalink / raw) To: Wolfram Sang, Linus Walleij Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Rob Herring On Fri, 7 Jun 2013 23:32:42 +0200, Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> wrote: > ... > > I2C devices probed from device tree should subsequently be > > fixed to handle the case where of_match_table() is > > used (I think none of them do that today), and platforms should > > fix their device trees to use compatible strings for I2C devices > > instead of setting the name to Linux device driver names as is > > done in multiple cases today. > > I guess your solution is the least intrusive one. Still, it could happen > that of_match_table is scanned three times (by driver core, i2c layer, > and i2c driver) which is IMO an indication to look for a more elegant > solution tp find out what really matched? It's what we do on platform_devices. It really isn't an expensive operation so I haven't pushed anyone to go optimize it. g. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] i2c: core: make it possible to match a pure device tree driver 2013-06-12 13:20 ` Grant Likely @ 2013-06-13 9:02 ` Linus Walleij 0 siblings, 0 replies; 18+ messages in thread From: Linus Walleij @ 2013-06-13 9:02 UTC (permalink / raw) To: Grant Likely Cc: Wolfram Sang, linux-i2c-u79uwXL29TY76Z2rM5mHXA, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Rob Herring On Wed, Jun 12, 2013 at 3:20 PM, Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote: > On Fri, 7 Jun 2013 23:32:42 +0200, Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> wrote: >> I guess your solution is the least intrusive one. Still, it could happen >> that of_match_table is scanned three times (by driver core, i2c layer, >> and i2c driver) which is IMO an indication to look for a more elegant >> solution tp find out what really matched? > > It's what we do on platform_devices. It really isn't an expensive > operation so I haven't pushed anyone to go optimize it. I tried to think of something and it ended up with ideas like decorating the device tree representation and it was just ... ouch. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] i2c: core: make it possible to match a pure device tree driver [not found] ` <1368476301-10495-1-git-send-email-linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> 2013-05-22 7:56 ` Linus Walleij 2013-06-07 21:32 ` Wolfram Sang @ 2013-06-12 17:09 ` Wolfram Sang 2 siblings, 0 replies; 18+ messages in thread From: Wolfram Sang @ 2013-06-12 17:09 UTC (permalink / raw) To: Linus Walleij Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Rob Herring, Grant Likely [-- Attachment #1: Type: text/plain, Size: 2523 bytes --] On Mon, May 13, 2013 at 10:18:21PM +0200, Linus Walleij wrote: > This tries to address an issue found when writing an MFD driver > for the Nomadik STw481x PMICs: as the platform is using device > tree exclusively I want to specify the driver matching like > this: > > static const struct of_device_id stw481x_match[] = { > { .compatible = "st,stw4810", }, > { .compatible = "st,stw4811", }, > {}, > }; > > static struct i2c_driver stw481x_driver = { > .driver = { > .name = "stw481x", > .of_match_table = stw481x_match, > }, > .probe = stw481x_probe, > .remove = stw481x_remove, > }; > > However that turns out not to be possible: the I2C probe code > is written so that the probe() call is always passed a match > from i2c_match_id() using non-devicetree matches. > > This is probably why most devices using device tree for I2C > clients currently will pass no .of_match_table *at all* but > instead just use .id_table from struct i2c_driver to match > the device. As you realize that means that the whole idea with > compatible strings is discarded, and that is why we find strange > device tree I2C device compatible strings like "product" instead > of "vendor,product" as you could expect. > > Let's figure out how to fix this before the mess spreads. This > patch will allow probeing devices with only an of_match_table > as per above, and will pass NULL as the second argument to the > probe() function. If the driver wants to deduce secondary info > from the struct of_device_id .data field, it has to call > of_match_device() on its own match table in the probe function > device tree probe path. > > If drivers define both an .of_match_table *AND* a i2c_driver > .id_table, the .of_match_table will take precedence, just > as is done in the i2c_device_match() function in i2c-core.c. > > I2C devices probed from device tree should subsequently be > fixed to handle the case where of_match_table() is > used (I think none of them do that today), and platforms should > fix their device trees to use compatible strings for I2C devices > instead of setting the name to Linux device driver names as is > done in multiple cases today. > > Cc: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org> > Cc: Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> > Signed-off-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> Applied to for-next, thanks! Thanks also to Grant for the insight. [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2013-06-18 15:55 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-05-13 20:18 [PATCH v2] i2c: core: make it possible to match a pure device tree driver Linus Walleij [not found] ` <1368476301-10495-1-git-send-email-linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> 2013-05-22 7:56 ` Linus Walleij [not found] ` <CACRpkdae1chj9ZKuQV5MuiaicLUTKr4hZqr8rtjbB3Yjt+U_fg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2013-05-30 8:00 ` Linus Walleij 2013-06-17 15:48 ` Stephen Warren [not found] ` <51BF2FC5.40207-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> 2013-06-17 16:33 ` Linus Walleij [not found] ` <CACRpkdbjy+PcpNkSg4PuuP2Z9RyrnqSW0uchHFHVZ8VYPeoDyA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2013-06-17 16:49 ` Stephen Warren 2013-06-17 22:15 ` Grant Likely [not found] ` <CACxGe6sTq=h9jjqesjvUA4Bweh7zgE4VOLoOTgGPC_D_v+8NtA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2013-06-17 23:25 ` Stephen Warren 2013-06-18 7:33 ` Wolfram Sang 2013-06-18 7:44 ` Linus Walleij [not found] ` <CACRpkdakdUVzHM4L5qOJ5Ls8NwOimy197NCUDSP=X28d2fzHPg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2013-06-18 15:55 ` Wolfram Sang 2013-06-07 21:32 ` Wolfram Sang 2013-06-10 12:18 ` Linus Walleij [not found] ` <CACRpkdbBsXB+9wN0oshfZB8Xady9G_wu9d8UkgA-bYqQe=8Mpw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2013-06-10 13:40 ` Wolfram Sang 2013-06-12 13:22 ` Grant Likely 2013-06-12 13:20 ` Grant Likely 2013-06-13 9:02 ` Linus Walleij 2013-06-12 17:09 ` Wolfram Sang
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.