Patchwork [v4,3/3] powerpc: doc/dts-bindings: update doc of FSL I2C bindings

login
register
mail settings
Submitter Wolfgang Grandegger
Date Jan. 28, 2010, 1:25 p.m.
Message ID <1264685141-26391-4-git-send-email-wg@grandegger.com>
Download mbox | patch
Permalink /patch/43859/
State Superseded
Headers show

Comments

Wolfgang Grandegger - Jan. 28, 2010, 1:25 p.m.
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>
---
 Documentation/powerpc/dts-bindings/fsl/i2c.txt |   30 +++++++++++++++++++----
 1 files changed, 24 insertions(+), 6 deletions(-)
Wolfram Sang - Jan. 29, 2010, 4:26 p.m.
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>
Grant Likely - Feb. 9, 2010, 5:50 p.m.
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
>
Wolfgang Grandegger - Feb. 9, 2010, 6:57 p.m.
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 - Feb. 9, 2010, 7:23 p.m.
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.
Grant Likely - Feb. 9, 2010, 10:15 p.m.
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.

Patch

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>;
 	};
-