| Message ID | 1264685141-26391-4-git-send-email-wg@grandegger.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Thu, Jan 28, 2010 at 02:25:41PM +0100, Wolfgang Grandegger wrote: > From: Wolfgang Grandegger <wg@denx.de> > > This patch adds the MPC5121 to the list of supported devices, > enhances the doc of the "clock-frequency" property and removes > the obsolete "cell-index" property from the example nodes. > Furthermore and example for the MPC5121 has been added. > > Signed-off-by: Wolfgang Grandegger <wg@denx.de> Reviewed-by: Wolfram Sang <w.sang@pengutronix.de>
On Thu, Jan 28, 2010 at 6:25 AM, Wolfgang Grandegger <wg@grandegger.com> wrote: > From: Wolfgang Grandegger <wg@denx.de> > > This patch adds the MPC5121 to the list of supported devices, > enhances the doc of the "clock-frequency" property and removes > the obsolete "cell-index" property from the example nodes. > Furthermore and example for the MPC5121 has been added. > > Signed-off-by: Wolfgang Grandegger <wg@denx.de> Thanks Wolfgang. Comments below. > --- > Documentation/powerpc/dts-bindings/fsl/i2c.txt | 30 +++++++++++++++++++---- > 1 files changed, 24 insertions(+), 6 deletions(-) > > diff --git a/Documentation/powerpc/dts-bindings/fsl/i2c.txt b/Documentation/powerpc/dts-bindings/fsl/i2c.txt > index b6d2e21..2f62dae 100644 > --- a/Documentation/powerpc/dts-bindings/fsl/i2c.txt > +++ b/Documentation/powerpc/dts-bindings/fsl/i2c.txt > @@ -9,8 +9,9 @@ Recommended properties : > > - compatible : compatibility list with 2 entries, the first should > be "fsl,CHIP-i2c" where CHIP is the name of a compatible processor, > - e.g. mpc8313, mpc8543, mpc8544, mpc5200 or mpc5200b. The second one > - should be "fsl-i2c". > + e.g. mpc8313, mpc8543, mpc8544, mpc5121, mpc5200 or mpc5200b. The > + second one should be "fsl-i2c". For the mpc5121, an additional node > + "fsl,mpc5121-i2c-ctrl" is required as shown in the example below. While you're editing this line; drop the requirement for the second value to be 'fsl-i2c'. We don't use it anymore, and only preserve it for backwards compatibility with old trees. > - interrupts : <a b> where a is the interrupt number and b is a > field that represents an encoding of the sense and level > information for the interrupt. This should be encoded based on > @@ -20,29 +21,46 @@ Recommended properties : > services interrupts for this device. > - fsl,preserve-clocking : boolean; if defined, the clock settings > from the bootloader are preserved (not touched). > - - clock-frequency : desired I2C bus clock frequency in Hz. > + - clock-frequency : desired I2C bus clock frequency in Hz. If this > + property and "fsl,preserve-clocking" is not defined, a safe fixed > + clock divider value is used (resulting in a small clock frequency). Nah, leave this as is. Don't make it sound like omitting both properties is a valid option. The driver may (and should!) handle the situation gracefully, but that fact does not need to be documented. > > Examples : > > + /* MPC5121 based board */ > + i2c@1740 { > + #address-cells = <1>; > + #size-cells = <0>; > + compatible = "fsl,mpc5121-i2c", "fsl-i2c"; > + reg = <0x1740 0x20>; > + interrupts = <11 0x8>; > + interrupt-parent = <&ipic>; > + clock-frequency = <100000>; > + }; > + > + i2ccontrol@1760 { > + compatible = "fsl,mpc5121-i2c-ctrl"; > + reg = <0x1760 0x8>; > + }; > + > + /* MPC5200B based board */ > i2c@3d00 { > #address-cells = <1>; > #size-cells = <0>; > compatible = "fsl,mpc5200b-i2c","fsl,mpc5200-i2c","fsl-i2c"; > - cell-index = <0>; > reg = <0x3d00 0x40>; > interrupts = <2 15 0>; > interrupt-parent = <&mpc5200_pic>; > fsl,preserve-clocking; > }; > > + /* MPC8544 base board */ > i2c@3100 { > #address-cells = <1>; > #size-cells = <0>; > - cell-index = <1>; > compatible = "fsl,mpc8544-i2c", "fsl-i2c"; > reg = <0x3100 0x100>; > interrupts = <43 2>; > interrupt-parent = <&mpic>; > clock-frequency = <400000>; > }; > - > -- > 1.6.2.5 > > _______________________________________________ > devicetree-discuss mailing list > devicetree-discuss@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/devicetree-discuss >
Hi Grant, Grant Likely wrote: > On Thu, Jan 28, 2010 at 6:25 AM, Wolfgang Grandegger <wg@grandegger.com> wrote: >> From: Wolfgang Grandegger <wg@denx.de> >> >> This patch adds the MPC5121 to the list of supported devices, >> enhances the doc of the "clock-frequency" property and removes >> the obsolete "cell-index" property from the example nodes. >> Furthermore and example for the MPC5121 has been added. >> >> Signed-off-by: Wolfgang Grandegger <wg@denx.de> > > Thanks Wolfgang. Comments below. > >> --- >> Documentation/powerpc/dts-bindings/fsl/i2c.txt | 30 +++++++++++++++++++---- >> 1 files changed, 24 insertions(+), 6 deletions(-) >> >> diff --git a/Documentation/powerpc/dts-bindings/fsl/i2c.txt b/Documentation/powerpc/dts-bindings/fsl/i2c.txt >> index b6d2e21..2f62dae 100644 >> --- a/Documentation/powerpc/dts-bindings/fsl/i2c.txt >> +++ b/Documentation/powerpc/dts-bindings/fsl/i2c.txt >> @@ -9,8 +9,9 @@ Recommended properties : >> >> - compatible : compatibility list with 2 entries, the first should >> be "fsl,CHIP-i2c" where CHIP is the name of a compatible processor, >> - e.g. mpc8313, mpc8543, mpc8544, mpc5200 or mpc5200b. The second one >> - should be "fsl-i2c". >> + e.g. mpc8313, mpc8543, mpc8544, mpc5121, mpc5200 or mpc5200b. The >> + second one should be "fsl-i2c". For the mpc5121, an additional node >> + "fsl,mpc5121-i2c-ctrl" is required as shown in the example below. > > While you're editing this line; drop the requirement for the second > value to be 'fsl-i2c'. We don't use it anymore, and only preserve it > for backwards compatibility with old trees. OK. >> - interrupts : <a b> where a is the interrupt number and b is a >> field that represents an encoding of the sense and level >> information for the interrupt. This should be encoded based on >> @@ -20,29 +21,46 @@ Recommended properties : >> services interrupts for this device. >> - fsl,preserve-clocking : boolean; if defined, the clock settings >> from the bootloader are preserved (not touched). >> - - clock-frequency : desired I2C bus clock frequency in Hz. >> + - clock-frequency : desired I2C bus clock frequency in Hz. If this >> + property and "fsl,preserve-clocking" is not defined, a safe fixed >> + clock divider value is used (resulting in a small clock frequency). > > Nah, leave this as is. Don't make it sound like omitting both > properties is a valid option. The driver may (and should!) handle the > situation gracefully, but that fact does not need to be documented. The safe value is not a good choice, indeed. Then it will also change MPC_I2C_CLOCK_SAFE to MPC_I2C_CLOCK_LEGACY in i2c-mpc.c. I will also fix the other issues you commented on. Thanks, Wolfgang.
Wolfgang Grandegger wrote: > Hi Grant, > > Grant Likely wrote: >> On Thu, Jan 28, 2010 at 6:25 AM, Wolfgang Grandegger <wg@grandegger.com> wrote: >>> From: Wolfgang Grandegger <wg@denx.de> >>> >>> This patch adds the MPC5121 to the list of supported devices, >>> enhances the doc of the "clock-frequency" property and removes >>> the obsolete "cell-index" property from the example nodes. >>> Furthermore and example for the MPC5121 has been added. >>> >>> Signed-off-by: Wolfgang Grandegger <wg@denx.de> >> Thanks Wolfgang. Comments below. >> >>> --- >>> Documentation/powerpc/dts-bindings/fsl/i2c.txt | 30 +++++++++++++++++++---- >>> 1 files changed, 24 insertions(+), 6 deletions(-) >>> >>> diff --git a/Documentation/powerpc/dts-bindings/fsl/i2c.txt b/Documentation/powerpc/dts-bindings/fsl/i2c.txt >>> index b6d2e21..2f62dae 100644 >>> --- a/Documentation/powerpc/dts-bindings/fsl/i2c.txt >>> +++ b/Documentation/powerpc/dts-bindings/fsl/i2c.txt >>> @@ -9,8 +9,9 @@ Recommended properties : >>> >>> - compatible : compatibility list with 2 entries, the first should >>> be "fsl,CHIP-i2c" where CHIP is the name of a compatible processor, >>> - e.g. mpc8313, mpc8543, mpc8544, mpc5200 or mpc5200b. The second one >>> - should be "fsl-i2c". >>> + e.g. mpc8313, mpc8543, mpc8544, mpc5121, mpc5200 or mpc5200b. The >>> + second one should be "fsl-i2c". For the mpc5121, an additional node >>> + "fsl,mpc5121-i2c-ctrl" is required as shown in the example below. >> While you're editing this line; drop the requirement for the second >> value to be 'fsl-i2c'. We don't use it anymore, and only preserve it >> for backwards compatibility with old trees. > > OK. > >>> - interrupts : <a b> where a is the interrupt number and b is a >>> field that represents an encoding of the sense and level >>> information for the interrupt. This should be encoded based on >>> @@ -20,29 +21,46 @@ Recommended properties : >>> services interrupts for this device. >>> - fsl,preserve-clocking : boolean; if defined, the clock settings >>> from the bootloader are preserved (not touched). >>> - - clock-frequency : desired I2C bus clock frequency in Hz. >>> + - clock-frequency : desired I2C bus clock frequency in Hz. If this >>> + property and "fsl,preserve-clocking" is not defined, a safe fixed >>> + clock divider value is used (resulting in a small clock frequency). >> Nah, leave this as is. Don't make it sound like omitting both >> properties is a valid option. The driver may (and should!) handle the >> situation gracefully, but that fact does not need to be documented. > > The safe value is not a good choice, indeed. Then it will also change > MPC_I2C_CLOCK_SAFE to MPC_I2C_CLOCK_LEGACY in i2c-mpc.c. > > I will also fix the other issues you commented on. And I will also remove the "device_type" line from: ------------- Required properties : - device_type : Should be "i2c" - reg : Offset and length of the register set for the device ------------- Wolfgang.
On Tue, Feb 9, 2010 at 12:23 PM, Wolfgang Grandegger <wg@grandegger.com> wrote: > Wolfgang Grandegger wrote: >> Hi Grant, >> >> Grant Likely wrote: >>> On Thu, Jan 28, 2010 at 6:25 AM, Wolfgang Grandegger <wg@grandegger.com> wrote: >>>> From: Wolfgang Grandegger <wg@denx.de> >>>> >>>> This patch adds the MPC5121 to the list of supported devices, >>>> enhances the doc of the "clock-frequency" property and removes >>>> the obsolete "cell-index" property from the example nodes. >>>> Furthermore and example for the MPC5121 has been added. >>>> >>>> Signed-off-by: Wolfgang Grandegger <wg@denx.de> >>> Thanks Wolfgang. Comments below. >>> >>>> --- >>>> Documentation/powerpc/dts-bindings/fsl/i2c.txt | 30 +++++++++++++++++++---- >>>> 1 files changed, 24 insertions(+), 6 deletions(-) >>>> >>>> diff --git a/Documentation/powerpc/dts-bindings/fsl/i2c.txt b/Documentation/powerpc/dts-bindings/fsl/i2c.txt >>>> index b6d2e21..2f62dae 100644 >>>> --- a/Documentation/powerpc/dts-bindings/fsl/i2c.txt >>>> +++ b/Documentation/powerpc/dts-bindings/fsl/i2c.txt >>>> @@ -9,8 +9,9 @@ Recommended properties : >>>> >>>> - compatible : compatibility list with 2 entries, the first should >>>> be "fsl,CHIP-i2c" where CHIP is the name of a compatible processor, >>>> - e.g. mpc8313, mpc8543, mpc8544, mpc5200 or mpc5200b. The second one >>>> - should be "fsl-i2c". >>>> + e.g. mpc8313, mpc8543, mpc8544, mpc5121, mpc5200 or mpc5200b. The >>>> + second one should be "fsl-i2c". For the mpc5121, an additional node >>>> + "fsl,mpc5121-i2c-ctrl" is required as shown in the example below. >>> While you're editing this line; drop the requirement for the second >>> value to be 'fsl-i2c'. We don't use it anymore, and only preserve it >>> for backwards compatibility with old trees. >> >> OK. >> >>>> - interrupts : <a b> where a is the interrupt number and b is a >>>> field that represents an encoding of the sense and level >>>> information for the interrupt. This should be encoded based on >>>> @@ -20,29 +21,46 @@ Recommended properties : >>>> services interrupts for this device. >>>> - fsl,preserve-clocking : boolean; if defined, the clock settings >>>> from the bootloader are preserved (not touched). >>>> - - clock-frequency : desired I2C bus clock frequency in Hz. >>>> + - clock-frequency : desired I2C bus clock frequency in Hz. If this >>>> + property and "fsl,preserve-clocking" is not defined, a safe fixed >>>> + clock divider value is used (resulting in a small clock frequency). >>> Nah, leave this as is. Don't make it sound like omitting both >>> properties is a valid option. The driver may (and should!) handle the >>> situation gracefully, but that fact does not need to be documented. >> >> The safe value is not a good choice, indeed. Then it will also change >> MPC_I2C_CLOCK_SAFE to MPC_I2C_CLOCK_LEGACY in i2c-mpc.c. >> >> I will also fix the other issues you commented on. > > And I will also remove the "device_type" line from: > > ------------- > Required properties : > > - device_type : Should be "i2c" > - reg : Offset and length of the register set for the device Yes, please. Hmmm. compatible should also be in the 'required properties' section. Thanks, g.
diff --git a/Documentation/powerpc/dts-bindings/fsl/i2c.txt b/Documentation/powerpc/dts-bindings/fsl/i2c.txt index b6d2e21..2f62dae 100644 --- a/Documentation/powerpc/dts-bindings/fsl/i2c.txt +++ b/Documentation/powerpc/dts-bindings/fsl/i2c.txt @@ -9,8 +9,9 @@ Recommended properties : - compatible : compatibility list with 2 entries, the first should be "fsl,CHIP-i2c" where CHIP is the name of a compatible processor, - e.g. mpc8313, mpc8543, mpc8544, mpc5200 or mpc5200b. The second one - should be "fsl-i2c". + e.g. mpc8313, mpc8543, mpc8544, mpc5121, mpc5200 or mpc5200b. The + second one should be "fsl-i2c". For the mpc5121, an additional node + "fsl,mpc5121-i2c-ctrl" is required as shown in the example below. - interrupts : <a b> where a is the interrupt number and b is a field that represents an encoding of the sense and level information for the interrupt. This should be encoded based on @@ -20,29 +21,46 @@ Recommended properties : services interrupts for this device. - fsl,preserve-clocking : boolean; if defined, the clock settings from the bootloader are preserved (not touched). - - clock-frequency : desired I2C bus clock frequency in Hz. + - clock-frequency : desired I2C bus clock frequency in Hz. If this + property and "fsl,preserve-clocking" is not defined, a safe fixed + clock divider value is used (resulting in a small clock frequency). Examples : + /* MPC5121 based board */ + i2c@1740 { + #address-cells = <1>; + #size-cells = <0>; + compatible = "fsl,mpc5121-i2c", "fsl-i2c"; + reg = <0x1740 0x20>; + interrupts = <11 0x8>; + interrupt-parent = <&ipic>; + clock-frequency = <100000>; + }; + + i2ccontrol@1760 { + compatible = "fsl,mpc5121-i2c-ctrl"; + reg = <0x1760 0x8>; + }; + + /* MPC5200B based board */ i2c@3d00 { #address-cells = <1>; #size-cells = <0>; compatible = "fsl,mpc5200b-i2c","fsl,mpc5200-i2c","fsl-i2c"; - cell-index = <0>; reg = <0x3d00 0x40>; interrupts = <2 15 0>; interrupt-parent = <&mpc5200_pic>; fsl,preserve-clocking; }; + /* MPC8544 base board */ i2c@3100 { #address-cells = <1>; #size-cells = <0>; - cell-index = <1>; compatible = "fsl,mpc8544-i2c", "fsl-i2c"; reg = <0x3100 0x100>; interrupts = <43 2>; interrupt-parent = <&mpic>; clock-frequency = <400000>; }; -