diff mbox series

ARM: zynq: dts: Setting default i2c clock frequency to 400kHz

Message ID 4dde5d1eb8e4572dae4295a19a4c83002a58e5da.1683035611.git.michal.simek@amd.com
State New
Headers
Series ARM: zynq: dts: Setting default i2c clock frequency to 400kHz |

Commit Message

Michal Simek May 2, 2023, 1:53 p.m. UTC 
  From: Varalaxmi Bingi <varalaxmi.bingi@amd.com>

Setting default i2c clock frequency for Zynq to maximum rate of 400kHz.
Current default value is 100kHz.

Signed-off-by: Varalaxmi Bingi <varalaxmi.bingi@amd.com>
Signed-off-by: Michal Simek <michal.simek@amd.com>
---

 arch/arm/boot/dts/zynq-7000.dtsi | 2 ++
 1 file changed, 2 insertions(+)

Comments

Krzysztof Kozlowski May 2, 2023, 8:13 p.m. UTC | #1 
On 02/05/2023 15:53, Michal Simek wrote:
> From: Varalaxmi Bingi <varalaxmi.bingi@amd.com>
> 
> Setting default i2c clock frequency for Zynq to maximum rate of 400kHz.
> Current default value is 100kHz.
> 
> Signed-off-by: Varalaxmi Bingi <varalaxmi.bingi@amd.com>
> Signed-off-by: Michal Simek <michal.simek@amd.com>
> ---
> 
>  arch/arm/boot/dts/zynq-7000.dtsi | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
> index cd9931f6bcbd..a7db3f3009f2 100644
> --- a/arch/arm/boot/dts/zynq-7000.dtsi
> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
> @@ -149,6 +149,7 @@ i2c0: i2c@e0004000 {
>  			clocks = <&clkc 38>;
>  			interrupt-parent = <&intc>;
>  			interrupts = <0 25 4>;
> +			clock-frequency = <400000>;

I think this should be rather property of the board, because it depends
on I2C devices, right?

Best regards,
Krzysztof
Michal Simek May 3, 2023, 7:27 a.m. UTC | #2 
On 5/2/23 22:13, Krzysztof Kozlowski wrote:
> CAUTION: This message has originated from an External Source. Please use proper judgment and caution when opening attachments, clicking links, or responding to this email.
> 
> 
> On 02/05/2023 15:53, Michal Simek wrote:
>> From: Varalaxmi Bingi <varalaxmi.bingi@amd.com>
>>
>> Setting default i2c clock frequency for Zynq to maximum rate of 400kHz.
>> Current default value is 100kHz.
>>
>> Signed-off-by: Varalaxmi Bingi <varalaxmi.bingi@amd.com>
>> Signed-off-by: Michal Simek <michal.simek@amd.com>
>> ---
>>
>>   arch/arm/boot/dts/zynq-7000.dtsi | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
>> index cd9931f6bcbd..a7db3f3009f2 100644
>> --- a/arch/arm/boot/dts/zynq-7000.dtsi
>> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
>> @@ -149,6 +149,7 @@ i2c0: i2c@e0004000 {
>>                        clocks = <&clkc 38>;
>>                        interrupt-parent = <&intc>;
>>                        interrupts = <0 25 4>;
>> +                     clock-frequency = <400000>;
> 
> I think this should be rather property of the board, because it depends
> on I2C devices, right?

We have 15+ boards with i2c connected and from this count I am aware about only 
one configuration which is not working on 400k.
It is a question if make sense to update all boards (except for 1 and not all in 
kernel yet) to 400k or just say 400k is default and if there is any issue use 
lower frequency.
Obviously this patch is setting up 400k as default and let "problematic" boards 
to setup lower freq.
The same approach was used for modepin description.

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/arch/arm64/boot/dts/xilinx/zynqmp.dtsi?h=v6.3&id=53ba1b2bdaf7f481fdd878e9c18cd0e54081fac9

The next point is that 100k is default in Linux but doesn't mean it is default 
in other projects.

But if you think that it must be board property I think we should setup 100k as 
default in this file too to make it clear.

Thanks,
Michal
Krzysztof Kozlowski May 3, 2023, 7:40 a.m. UTC | #3 
On 03/05/2023 09:27, Michal Simek wrote:
> 
> 
> On 5/2/23 22:13, Krzysztof Kozlowski wrote:
>> CAUTION: This message has originated from an External Source. Please use proper judgment and caution when opening attachments, clicking links, or responding to this email.
>>
>>
>> On 02/05/2023 15:53, Michal Simek wrote:
>>> From: Varalaxmi Bingi <varalaxmi.bingi@amd.com>
>>>
>>> Setting default i2c clock frequency for Zynq to maximum rate of 400kHz.
>>> Current default value is 100kHz.
>>>
>>> Signed-off-by: Varalaxmi Bingi <varalaxmi.bingi@amd.com>
>>> Signed-off-by: Michal Simek <michal.simek@amd.com>
>>> ---
>>>
>>>   arch/arm/boot/dts/zynq-7000.dtsi | 2 ++
>>>   1 file changed, 2 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
>>> index cd9931f6bcbd..a7db3f3009f2 100644
>>> --- a/arch/arm/boot/dts/zynq-7000.dtsi
>>> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
>>> @@ -149,6 +149,7 @@ i2c0: i2c@e0004000 {
>>>                        clocks = <&clkc 38>;
>>>                        interrupt-parent = <&intc>;
>>>                        interrupts = <0 25 4>;
>>> +                     clock-frequency = <400000>;
>>
>> I think this should be rather property of the board, because it depends
>> on I2C devices, right?
> 
> We have 15+ boards with i2c connected and from this count I am aware about only 
> one configuration which is not working on 400k.
> It is a question if make sense to update all boards (except for 1 and not all in 
> kernel yet) to 400k or just say 400k is default and if there is any issue use 
> lower frequency.
> Obviously this patch is setting up 400k as default and let "problematic" boards 
> to setup lower freq.
> The same approach was used for modepin description.
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/arch/arm64/boot/dts/xilinx/zynqmp.dtsi?h=v6.3&id=53ba1b2bdaf7f481fdd878e9c18cd0e54081fac9
> 
> The next point is that 100k is default in Linux but doesn't mean it is default 
> in other projects.
> 
> But if you think that it must be board property I think we should setup 100k as 
> default in this file too to make it clear.

No, if it is conscious choice, it's fine.

Best regards,
Krzysztof
Michal Simek May 3, 2023, 8:02 a.m. UTC | #4 
On 5/3/23 09:40, Krzysztof Kozlowski wrote:
> On 03/05/2023 09:27, Michal Simek wrote:
>>
>>
>> On 5/2/23 22:13, Krzysztof Kozlowski wrote:
>>> CAUTION: This message has originated from an External Source. Please use proper judgment and caution when opening attachments, clicking links, or responding to this email.
>>>
>>>
>>> On 02/05/2023 15:53, Michal Simek wrote:
>>>> From: Varalaxmi Bingi <varalaxmi.bingi@amd.com>
>>>>
>>>> Setting default i2c clock frequency for Zynq to maximum rate of 400kHz.
>>>> Current default value is 100kHz.
>>>>
>>>> Signed-off-by: Varalaxmi Bingi <varalaxmi.bingi@amd.com>
>>>> Signed-off-by: Michal Simek <michal.simek@amd.com>
>>>> ---
>>>>
>>>>    arch/arm/boot/dts/zynq-7000.dtsi | 2 ++
>>>>    1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
>>>> index cd9931f6bcbd..a7db3f3009f2 100644
>>>> --- a/arch/arm/boot/dts/zynq-7000.dtsi
>>>> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
>>>> @@ -149,6 +149,7 @@ i2c0: i2c@e0004000 {
>>>>                         clocks = <&clkc 38>;
>>>>                         interrupt-parent = <&intc>;
>>>>                         interrupts = <0 25 4>;
>>>> +                     clock-frequency = <400000>;
>>>
>>> I think this should be rather property of the board, because it depends
>>> on I2C devices, right?
>>
>> We have 15+ boards with i2c connected and from this count I am aware about only
>> one configuration which is not working on 400k.
>> It is a question if make sense to update all boards (except for 1 and not all in
>> kernel yet) to 400k or just say 400k is default and if there is any issue use
>> lower frequency.
>> Obviously this patch is setting up 400k as default and let "problematic" boards
>> to setup lower freq.
>> The same approach was used for modepin description.
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/arch/arm64/boot/dts/xilinx/zynqmp.dtsi?h=v6.3&id=53ba1b2bdaf7f481fdd878e9c18cd0e54081fac9
>>
>> The next point is that 100k is default in Linux but doesn't mean it is default
>> in other projects.
>>
>> But if you think that it must be board property I think we should setup 100k as
>> default in this file too to make it clear.
> 
> No, if it is conscious choice, it's fine.

Is it Ack then or you want me to extend commit message?

Thanks,
Michal
Krzysztof Kozlowski May 3, 2023, 8:06 a.m. UTC | #5 
On 03/05/2023 10:02, Michal Simek wrote:
> 
> 
> On 5/3/23 09:40, Krzysztof Kozlowski wrote:
>> On 03/05/2023 09:27, Michal Simek wrote:
>>>
>>>
>>> On 5/2/23 22:13, Krzysztof Kozlowski wrote:
>>>> CAUTION: This message has originated from an External Source. Please use proper judgment and caution when opening attachments, clicking links, or responding to this email.
>>>>
>>>>
>>>> On 02/05/2023 15:53, Michal Simek wrote:
>>>>> From: Varalaxmi Bingi <varalaxmi.bingi@amd.com>
>>>>>
>>>>> Setting default i2c clock frequency for Zynq to maximum rate of 400kHz.
>>>>> Current default value is 100kHz.
>>>>>
>>>>> Signed-off-by: Varalaxmi Bingi <varalaxmi.bingi@amd.com>
>>>>> Signed-off-by: Michal Simek <michal.simek@amd.com>
>>>>> ---
>>>>>
>>>>>    arch/arm/boot/dts/zynq-7000.dtsi | 2 ++
>>>>>    1 file changed, 2 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
>>>>> index cd9931f6bcbd..a7db3f3009f2 100644
>>>>> --- a/arch/arm/boot/dts/zynq-7000.dtsi
>>>>> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
>>>>> @@ -149,6 +149,7 @@ i2c0: i2c@e0004000 {
>>>>>                         clocks = <&clkc 38>;
>>>>>                         interrupt-parent = <&intc>;
>>>>>                         interrupts = <0 25 4>;
>>>>> +                     clock-frequency = <400000>;
>>>>
>>>> I think this should be rather property of the board, because it depends
>>>> on I2C devices, right?
>>>
>>> We have 15+ boards with i2c connected and from this count I am aware about only
>>> one configuration which is not working on 400k.
>>> It is a question if make sense to update all boards (except for 1 and not all in
>>> kernel yet) to 400k or just say 400k is default and if there is any issue use
>>> lower frequency.
>>> Obviously this patch is setting up 400k as default and let "problematic" boards
>>> to setup lower freq.
>>> The same approach was used for modepin description.
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/arch/arm64/boot/dts/xilinx/zynqmp.dtsi?h=v6.3&id=53ba1b2bdaf7f481fdd878e9c18cd0e54081fac9
>>>
>>> The next point is that 100k is default in Linux but doesn't mean it is default
>>> in other projects.
>>>
>>> But if you think that it must be board property I think we should setup 100k as
>>> default in this file too to make it clear.
>>
>> No, if it is conscious choice, it's fine.
> 
> Is it Ack then or you want me to extend commit message?

Ack, although you don't need my acks for your platform :)

FWIW:

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof
Michal Simek May 3, 2023, 8:09 a.m. UTC | #6 
On 5/3/23 10:06, Krzysztof Kozlowski wrote:
> On 03/05/2023 10:02, Michal Simek wrote:
>>
>>
>> On 5/3/23 09:40, Krzysztof Kozlowski wrote:
>>> On 03/05/2023 09:27, Michal Simek wrote:
>>>>
>>>>
>>>> On 5/2/23 22:13, Krzysztof Kozlowski wrote:
>>>>> CAUTION: This message has originated from an External Source. Please use proper judgment and caution when opening attachments, clicking links, or responding to this email.
>>>>>
>>>>>
>>>>> On 02/05/2023 15:53, Michal Simek wrote:
>>>>>> From: Varalaxmi Bingi <varalaxmi.bingi@amd.com>
>>>>>>
>>>>>> Setting default i2c clock frequency for Zynq to maximum rate of 400kHz.
>>>>>> Current default value is 100kHz.
>>>>>>
>>>>>> Signed-off-by: Varalaxmi Bingi <varalaxmi.bingi@amd.com>
>>>>>> Signed-off-by: Michal Simek <michal.simek@amd.com>
>>>>>> ---
>>>>>>
>>>>>>     arch/arm/boot/dts/zynq-7000.dtsi | 2 ++
>>>>>>     1 file changed, 2 insertions(+)
>>>>>>
>>>>>> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>> index cd9931f6bcbd..a7db3f3009f2 100644
>>>>>> --- a/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
>>>>>> @@ -149,6 +149,7 @@ i2c0: i2c@e0004000 {
>>>>>>                          clocks = <&clkc 38>;
>>>>>>                          interrupt-parent = <&intc>;
>>>>>>                          interrupts = <0 25 4>;
>>>>>> +                     clock-frequency = <400000>;
>>>>>
>>>>> I think this should be rather property of the board, because it depends
>>>>> on I2C devices, right?
>>>>
>>>> We have 15+ boards with i2c connected and from this count I am aware about only
>>>> one configuration which is not working on 400k.
>>>> It is a question if make sense to update all boards (except for 1 and not all in
>>>> kernel yet) to 400k or just say 400k is default and if there is any issue use
>>>> lower frequency.
>>>> Obviously this patch is setting up 400k as default and let "problematic" boards
>>>> to setup lower freq.
>>>> The same approach was used for modepin description.
>>>>
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/arch/arm64/boot/dts/xilinx/zynqmp.dtsi?h=v6.3&id=53ba1b2bdaf7f481fdd878e9c18cd0e54081fac9
>>>>
>>>> The next point is that 100k is default in Linux but doesn't mean it is default
>>>> in other projects.
>>>>
>>>> But if you think that it must be board property I think we should setup 100k as
>>>> default in this file too to make it clear.
>>>
>>> No, if it is conscious choice, it's fine.
>>
>> Is it Ack then or you want me to extend commit message?
> 
> Ack, although you don't need my acks for your platform :)

you spell that properly. But I can. :-)

> 
> FWIW:
> 
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Thanks,
Michal
Michal Simek May 12, 2023, 11:26 a.m. UTC | #7 
On 5/2/23 15:53, Michal Simek wrote:
> From: Varalaxmi Bingi <varalaxmi.bingi@amd.com>
> 
> Setting default i2c clock frequency for Zynq to maximum rate of 400kHz.
> Current default value is 100kHz.
> 
> Signed-off-by: Varalaxmi Bingi <varalaxmi.bingi@amd.com>
> Signed-off-by: Michal Simek <michal.simek@amd.com>
> ---
> 
>   arch/arm/boot/dts/zynq-7000.dtsi | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
> index cd9931f6bcbd..a7db3f3009f2 100644
> --- a/arch/arm/boot/dts/zynq-7000.dtsi
> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
> @@ -149,6 +149,7 @@ i2c0: i2c@e0004000 {
>   			clocks = <&clkc 38>;
>   			interrupt-parent = <&intc>;
>   			interrupts = <0 25 4>;
> +			clock-frequency = <400000>;
>   			reg = <0xe0004000 0x1000>;
>   			#address-cells = <1>;
>   			#size-cells = <0>;
> @@ -160,6 +161,7 @@ i2c1: i2c@e0005000 {
>   			clocks = <&clkc 39>;
>   			interrupt-parent = <&intc>;
>   			interrupts = <0 48 4>;
> +			clock-frequency = <400000>;
>   			reg = <0xe0005000 0x1000>;
>   			#address-cells = <1>;
>   			#size-cells = <0>;

Applied.
M
diff mbox series

Patch

diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
index cd9931f6bcbd..a7db3f3009f2 100644
--- a/arch/arm/boot/dts/zynq-7000.dtsi
+++ b/arch/arm/boot/dts/zynq-7000.dtsi
@@ -149,6 +149,7 @@  i2c0: i2c@e0004000 {
 			clocks = <&clkc 38>;
 			interrupt-parent = <&intc>;
 			interrupts = <0 25 4>;
+			clock-frequency = <400000>;
 			reg = <0xe0004000 0x1000>;
 			#address-cells = <1>;
 			#size-cells = <0>;
@@ -160,6 +161,7 @@  i2c1: i2c@e0005000 {
 			clocks = <&clkc 39>;
 			interrupt-parent = <&intc>;
 			interrupts = <0 48 4>;
+			clock-frequency = <400000>;
 			reg = <0xe0005000 0x1000>;
 			#address-cells = <1>;
 			#size-cells = <0>;