| Message ID | 20131127082805.20015.82680.sendpatchset@w520 (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> + #address-cells = <1>; > + #size-cells = <0>; > + > + eeprom: 24c128@50 { > + compatible = "at,24c128"; Minor nit: Should be a vendor name, not the name of the driver > + reg = <0x50>; > + }; > + }; > + > };
Hi Magnus, Thank you for the patch. On Wednesday 27 November 2013 17:28:05 Magnus Damm wrote: > From: Magnus Damm <damm@opensource.se> > > Add support for the Genmai I2C bus hooked up to P1_5 and P1_4 using > the i2c-gpio driver. On the bus sits a 24c128 EEPRROM. Is this a temporary workaround until we get a proper I2C controller driver, or is the EEPROM really hooked up to pins that are not wired to a hardware I2C controller ? > Signed-off-by: Magnus Damm <damm@opensource.se> > --- > > arch/arm/boot/dts/r7s72100-genmai-reference.dts | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > > --- 0008/arch/arm/boot/dts/r7s72100-genmai-reference.dts > +++ work/arch/arm/boot/dts/r7s72100-genmai-reference.dts 2013-11-27 > 15:51:31.000000000 +0900 @@ -39,4 +39,22 @@ > gpios = <&port4 11 GPIO_ACTIVE_LOW>; > }; > }; > + > + i2c@0 { > + compatible = "i2c-gpio"; > + gpios = <&port1 5 GPIO_ACTIVE_HIGH /* sda */ > + &port1 4 GPIO_ACTIVE_HIGH /* scl */ > + >; > + i2c-gpio,sda-open-drain; > + i2c-gpio,scl-open-drain; > + i2c-gpio,delay-us = <5>; /* ~100 kHz */ > + #address-cells = <1>; > + #size-cells = <0>; > + > + eeprom: 24c128@50 { > + compatible = "at,24c128"; > + reg = <0x50>; > + }; > + }; > + > };
Hi Laurent, On Wed, Nov 27, 2013 at 8:15 PM, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > Hi Magnus, > > Thank you for the patch. > > On Wednesday 27 November 2013 17:28:05 Magnus Damm wrote: >> From: Magnus Damm <damm@opensource.se> >> >> Add support for the Genmai I2C bus hooked up to P1_5 and P1_4 using >> the i2c-gpio driver. On the bus sits a 24c128 EEPRROM. > > Is this a temporary workaround until we get a proper I2C controller driver, or > is the EEPROM really hooked up to pins that are not wired to a hardware I2C > controller ? This is a stop-gap solution until we have upstream driver support for the I2C master hardware. The pins used for the I2C bus can be configured in GPIO mode or a zillion other pin functions including SCL and SDA. Cheers, / magnus
Hi Magnus, On Thursday 28 November 2013 09:44:31 Magnus Damm wrote: > On Wed, Nov 27, 2013 at 8:15 PM, Laurent Pinchart wrote: > > On Wednesday 27 November 2013 17:28:05 Magnus Damm wrote: > >> From: Magnus Damm <damm@opensource.se> > >> > >> Add support for the Genmai I2C bus hooked up to P1_5 and P1_4 using > >> the i2c-gpio driver. On the bus sits a 24c128 EEPRROM. > > > > Is this a temporary workaround until we get a proper I2C controller > > driver, or is the EEPROM really hooked up to pins that are not wired to a > > hardware I2C controller ? > > This is a stop-gap solution until we have upstream driver support for the > I2C master hardware. OK, no problem. Is there a timeline for that ? > The pins used for the I2C bus can be configured in GPIO mode or a zillion > other pin functions including SCL and SDA.
Hello. On 27-11-2013 15:04, Wolfram Sang wrote: >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + eeprom: 24c128@50 { >> + compatible = "at,24c128"; > Minor nit: Should be a vendor name, not the name of the driver Moreover, the node should be named generically, like "flash@50", not "24c128@50". WBR, Sergei
On Thursday 28 November 2013 21:11:48 Sergei Shtylyov wrote: > On 27-11-2013 15:04, Wolfram Sang wrote: > >> + #address-cells = <1>; > >> + #size-cells = <0>; > >> + > >> + eeprom: 24c128@50 { > >> + compatible = "at,24c128"; > > > > Minor nit: Should be a vendor name, not the name of the driver > > Moreover, the node should be named generically, like "flash@50", not > "24c128@50". Or, given that it's an eeprom, "eeprom@50" ? :-)
Hello. On 28-11-2013 21:14, Laurent Pinchart wrote: >>>> + #address-cells = <1>; >>>> + #size-cells = <0>; >>>> + >>>> + eeprom: 24c128@50 { >>>> + compatible = "at,24c128"; >>> Minor nit: Should be a vendor name, not the name of the driver >> Moreover, the node should be named generically, like "flash@50", not >> "24c128@50". > Or, given that it's an eeprom, "eeprom@50" ? :-) At least ePAPR spec. only has "flash". WBR, Sergei
--- 0008/arch/arm/boot/dts/r7s72100-genmai-reference.dts +++ work/arch/arm/boot/dts/r7s72100-genmai-reference.dts 2013-11-27 15:51:31.000000000 +0900 @@ -39,4 +39,22 @@ gpios = <&port4 11 GPIO_ACTIVE_LOW>; }; }; + + i2c@0 { + compatible = "i2c-gpio"; + gpios = <&port1 5 GPIO_ACTIVE_HIGH /* sda */ + &port1 4 GPIO_ACTIVE_HIGH /* scl */ + >; + i2c-gpio,sda-open-drain; + i2c-gpio,scl-open-drain; + i2c-gpio,delay-us = <5>; /* ~100 kHz */ + #address-cells = <1>; + #size-cells = <0>; + + eeprom: 24c128@50 { + compatible = "at,24c128"; + reg = <0x50>; + }; + }; + };