| Message ID | 20170414010445.21727-2-javier@osg.samsung.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
On Thu, Apr 13, 2017 at 10:04:25PM -0300, Javier Martinez Canillas wrote: > The at24 driver allows to register I2C EEPROM chips using different vendor > and devices, but the I2C subsystem does not take the vendor into account > when matching using the I2C table since it only has device entries. > > But when matching using an OF table, both the vendor and device has to be > taken into account so the driver defines only a set of compatible strings > using the "atmel" vendor as a generic fallback for compatible I2C devices. > > Document in the Device Tree binding document that this manufacturer should > be used as the generic fallback. > > Suggested-by: Wolfram Sang <wsa@the-dreams.de> > Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com> > > --- > > Changes in v3: None > Changes in v2: None > > Documentation/devicetree/bindings/eeprom/eeprom.txt | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/eeprom/eeprom.txt b/Documentation/devicetree/bindings/eeprom/eeprom.txt > index 5696eb508e95..d0395f14e2b3 100644 > --- a/Documentation/devicetree/bindings/eeprom/eeprom.txt > +++ b/Documentation/devicetree/bindings/eeprom/eeprom.txt > @@ -17,7 +17,8 @@ Required properties: > "renesas,r1ex24002" > > If there is no specific driver for <manufacturer>, a generic > - driver based on <type> is selected. Possible types are: > + driver based on <type> and manufacturer "atmel" is selected. > + Possible types are: This isn't quite right. What the driver does isn't really relevant to the binding. These types with no vendor are used as the compatible string, so we have to allow them. But it should be clear that no vendor is deprecated. Ironically, it is a lot of Atmel boards that do this. We should also explicitly list what are valid manufacturers. We also have "at" as a vendor prefix which perhaps we should explicitly say is deprecated. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Apr 19, 2017 at 6:27 PM, Rob Herring <robh@kernel.org> wrote: > On Thu, Apr 13, 2017 at 10:04:25PM -0300, Javier Martinez Canillas wrote: >> The at24 driver allows to register I2C EEPROM chips using different vendor >> and devices, but the I2C subsystem does not take the vendor into account >> when matching using the I2C table since it only has device entries. >> >> But when matching using an OF table, both the vendor and device has to be >> taken into account so the driver defines only a set of compatible strings >> using the "atmel" vendor as a generic fallback for compatible I2C devices. >> >> Document in the Device Tree binding document that this manufacturer should >> be used as the generic fallback. >> >> Suggested-by: Wolfram Sang <wsa@the-dreams.de> >> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com> >> >> --- >> >> Changes in v3: None >> Changes in v2: None >> >> Documentation/devicetree/bindings/eeprom/eeprom.txt | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/Documentation/devicetree/bindings/eeprom/eeprom.txt b/Documentation/devicetree/bindings/eeprom/eeprom.txt >> index 5696eb508e95..d0395f14e2b3 100644 >> --- a/Documentation/devicetree/bindings/eeprom/eeprom.txt >> +++ b/Documentation/devicetree/bindings/eeprom/eeprom.txt >> @@ -17,7 +17,8 @@ Required properties: >> "renesas,r1ex24002" >> >> If there is no specific driver for <manufacturer>, a generic >> - driver based on <type> is selected. Possible types are: >> + driver based on <type> and manufacturer "atmel" is selected. >> + Possible types are: > > This isn't quite right. What the driver does isn't really relevant to > the binding. > > These types with no vendor are used as the compatible string, so we have > to allow them. But it should be clear that no vendor is deprecated. > Ironically, it is a lot of Atmel boards that do this. > > We should also explicitly list what are valid manufacturers. We also > have "at" as a vendor prefix which perhaps we should explicitly say is > deprecated. I should perhaps look at the rest of the series before replying.. Based on that, the only comment that applies is listing the manufacturers that are valid. From a DT perspective, I should not have to know what the OS driver supports. If the device is compatible with atmel, then that is required. If not, then the specific manufacturer's compatible alone is enough and the OS has to match to that. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello Rob, Thanks for the feedback and sorry for the delayed response (I've been moving to a new country so didn't have too much time to answer emails). On Thu, Apr 20, 2017 at 1:35 AM, Rob Herring <robh@kernel.org> wrote: > On Wed, Apr 19, 2017 at 6:27 PM, Rob Herring <robh@kernel.org> wrote: >> On Thu, Apr 13, 2017 at 10:04:25PM -0300, Javier Martinez Canillas wrote: [snip] >>> >>> If there is no specific driver for <manufacturer>, a generic >>> - driver based on <type> is selected. Possible types are: >>> + driver based on <type> and manufacturer "atmel" is selected. >>> + Possible types are: >> >> This isn't quite right. What the driver does isn't really relevant to >> the binding. >> >> These types with no vendor are used as the compatible string, so we have >> to allow them. But it should be clear that no vendor is deprecated. >> Ironically, it is a lot of Atmel boards that do this. >> >> We should also explicitly list what are valid manufacturers. We also >> have "at" as a vendor prefix which perhaps we should explicitly say is >> deprecated. > > I should perhaps look at the rest of the series before replying.. > > Based on that, the only comment that applies is listing the > manufacturers that are valid. From a DT perspective, I should not have > to know what the OS driver supports. If the device is compatible with > atmel, then that is required. If not, then the specific manufacturer's > compatible alone is enough and the OS has to match to that. > Got it, I'll re-spin the series probably this week adding that information to the DT binding doc. > Rob Best regards, Javier -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/eeprom/eeprom.txt b/Documentation/devicetree/bindings/eeprom/eeprom.txt index 5696eb508e95..d0395f14e2b3 100644 --- a/Documentation/devicetree/bindings/eeprom/eeprom.txt +++ b/Documentation/devicetree/bindings/eeprom/eeprom.txt @@ -17,7 +17,8 @@ Required properties: "renesas,r1ex24002" If there is no specific driver for <manufacturer>, a generic - driver based on <type> is selected. Possible types are: + driver based on <type> and manufacturer "atmel" is selected. + Possible types are: "24c00", "24c01", "24c02", "24c04", "24c08", "24c16", "24c32", "24c64", "24c128", "24c256", "24c512", "24c1024", "spd"
The at24 driver allows to register I2C EEPROM chips using different vendor and devices, but the I2C subsystem does not take the vendor into account when matching using the I2C table since it only has device entries. But when matching using an OF table, both the vendor and device has to be taken into account so the driver defines only a set of compatible strings using the "atmel" vendor as a generic fallback for compatible I2C devices. Document in the Device Tree binding document that this manufacturer should be used as the generic fallback. Suggested-by: Wolfram Sang <wsa@the-dreams.de> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com> --- Changes in v3: None Changes in v2: None Documentation/devicetree/bindings/eeprom/eeprom.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)