From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751363AbdH3UPt (ORCPT ); Wed, 30 Aug 2017 16:15:49 -0400 Received: from mail-pf0-f169.google.com ([209.85.192.169]:34805 "EHLO mail-pf0-f169.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750761AbdH3UPq (ORCPT ); Wed, 30 Aug 2017 16:15:46 -0400 MIME-Version: 1.0 In-Reply-To: References: <20170615185418.13980-1-javier@dowhile0.org> <20170731153009.xkjknygpmckv2pfp@ninjato> <20170828160121.gu4wf7jqu4ditksc@ninjato> <20170829084831.hsdok3ksi7anxzuc@ninjato> <20170830174228.sx5jgtxxy56zq47t@ninjato> From: Geert Uytterhoeven Date: Wed, 30 Aug 2017 22:15:45 +0200 X-Google-Sender-Auth: K0aYXwOQuGrWPg1kO1kiHFwrbIc Message-ID: Subject: Re: [RESEND PATCH v5 00/16] eeprom: at24: Add OF device ID table To: Javier Martinez Canillas Cc: Wolfram Sang , Linux Kernel , Rob Herring , Florian Larysch , David Lechner , Rob Herring , Andy Shevchenko , Catalin Marinas , =?UTF-8?Q?S=C3=B6ren_Brinkmann?= , Simon Horman , Michal Simek , Dinh Nguyen , Russell King , Will Deacon , "devicetree@vger.kernel.org" , Sekhar Nori , Scott Wood , Benjamin Herrenschmidt , Joachim Eastwood , Mark Rutland , "linux-arm-kernel@lists.infradead.org" , Masahiro Yamada , Michael Ellerman , Santosh Shilimkar , Linux-Renesas , Paul Mackerras , Magnus Damm , linuxppc-dev , =?UTF-8?Q?Uwe_Kleine=2DK=C3=B6nig?= , Linux I2C Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Javier, On Wed, Aug 30, 2017 at 9:57 PM, Javier Martinez Canillas wrote: >> I think we should talk about the same case: Let me repeat what I did: >> >> 1) I added your patch "eeprom: at24: Add OF device ID table" >> 2) I added an EEPROM node to an I2C >> >> + eeprom@50 { >> + compatible = "renesas,24c01"; >> + reg = <0x50>; >> + }; >> >> -> no at24 binding to the device >> >> 3) I revert your patch >> >> -> at24 binding to the device >> > > I've tested this and you are right, it fails... > > The problem is that the patch also changes how the driver obtains the > EEPROM parameters (the magic value in the entry's data field). > > So even when module autoload and device / driver matching works, the > driver probe function fails because if (client->dev.of_node) the > driver attempts to get the entry data using > of_device_get_match_data(), which is obviously wrong since the > compatible string in the dev node isn't present in the OF table. > > The id->driver_data from the I2C table should be used instead since > that's the table that matches in this case. > > One option is to fallback to id->driver_data if > of_device_get_match_data() fails, but that's just an (ugly) > workaround. So I agree with you that the best option is to wait for > the DTS patches to land first. Which means new kernels won't work with old DTBs. Oops... I'm afraid that needs to be fixed. People care about DTB backward compatibility on many platforms. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds From mboxrd@z Thu Jan 1 00:00:00 1970 From: Geert Uytterhoeven Subject: Re: [RESEND PATCH v5 00/16] eeprom: at24: Add OF device ID table Date: Wed, 30 Aug 2017 22:15:45 +0200 Message-ID: References: <20170615185418.13980-1-javier@dowhile0.org> <20170731153009.xkjknygpmckv2pfp@ninjato> <20170828160121.gu4wf7jqu4ditksc@ninjato> <20170829084831.hsdok3ksi7anxzuc@ninjato> <20170830174228.sx5jgtxxy56zq47t@ninjato> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Return-path: In-Reply-To: Sender: linux-i2c-owner@vger.kernel.org To: Javier Martinez Canillas Cc: Wolfram Sang , Linux Kernel , Rob Herring , Florian Larysch , David Lechner , Rob Herring , Andy Shevchenko , Catalin Marinas , =?UTF-8?Q?S=C3=B6ren_Brinkmann?= , Simon Horman , Michal Simek , Dinh Nguyen , Russell King , Will Deacon , "devicetree@vger.kernel.org" , Sekhar Nori , Scott Wood , Benjamin Herrenschmidt , Joachim Eastwood , Mark List-Id: devicetree@vger.kernel.org Hi Javier, On Wed, Aug 30, 2017 at 9:57 PM, Javier Martinez Canillas wrote: >> I think we should talk about the same case: Let me repeat what I did: >> >> 1) I added your patch "eeprom: at24: Add OF device ID table" >> 2) I added an EEPROM node to an I2C >> >> + eeprom@50 { >> + compatible = "renesas,24c01"; >> + reg = <0x50>; >> + }; >> >> -> no at24 binding to the device >> >> 3) I revert your patch >> >> -> at24 binding to the device >> > > I've tested this and you are right, it fails... > > The problem is that the patch also changes how the driver obtains the > EEPROM parameters (the magic value in the entry's data field). > > So even when module autoload and device / driver matching works, the > driver probe function fails because if (client->dev.of_node) the > driver attempts to get the entry data using > of_device_get_match_data(), which is obviously wrong since the > compatible string in the dev node isn't present in the OF table. > > The id->driver_data from the I2C table should be used instead since > that's the table that matches in this case. > > One option is to fallback to id->driver_data if > of_device_get_match_data() fails, but that's just an (ugly) > workaround. So I agree with you that the best option is to wait for > the DTS patches to land first. Which means new kernels won't work with old DTBs. Oops... I'm afraid that needs to be fixed. People care about DTB backward compatibility on many platforms. Gr{oetje,eeting}s, Geert From mboxrd@z Thu Jan 1 00:00:00 1970 From: geert@linux-m68k.org (Geert Uytterhoeven) Date: Wed, 30 Aug 2017 22:15:45 +0200 Subject: [RESEND PATCH v5 00/16] eeprom: at24: Add OF device ID table In-Reply-To: References: <20170615185418.13980-1-javier@dowhile0.org> <20170731153009.xkjknygpmckv2pfp@ninjato> <20170828160121.gu4wf7jqu4ditksc@ninjato> <20170829084831.hsdok3ksi7anxzuc@ninjato> <20170830174228.sx5jgtxxy56zq47t@ninjato> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Javier, On Wed, Aug 30, 2017 at 9:57 PM, Javier Martinez Canillas wrote: >> I think we should talk about the same case: Let me repeat what I did: >> >> 1) I added your patch "eeprom: at24: Add OF device ID table" >> 2) I added an EEPROM node to an I2C >> >> + eeprom at 50 { >> + compatible = "renesas,24c01"; >> + reg = <0x50>; >> + }; >> >> -> no at24 binding to the device >> >> 3) I revert your patch >> >> -> at24 binding to the device >> > > I've tested this and you are right, it fails... > > The problem is that the patch also changes how the driver obtains the > EEPROM parameters (the magic value in the entry's data field). > > So even when module autoload and device / driver matching works, the > driver probe function fails because if (client->dev.of_node) the > driver attempts to get the entry data using > of_device_get_match_data(), which is obviously wrong since the > compatible string in the dev node isn't present in the OF table. > > The id->driver_data from the I2C table should be used instead since > that's the table that matches in this case. > > One option is to fallback to id->driver_data if > of_device_get_match_data() fails, but that's just an (ugly) > workaround. So I agree with you that the best option is to wait for > the DTS patches to land first. Which means new kernels won't work with old DTBs. Oops... I'm afraid that needs to be fixed. People care about DTB backward compatibility on many platforms. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds