Re: [PATCH] ARM: zynq: dts: Setting default i2c clock frequency to 400kHz

[Krzysztof Kozlowski]

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@xxxxxxx>
>>>
>>> Setting default i2c clock frequency for Zynq to maximum rate of 400kHz.
>>> Current default value is 100kHz.
>>>
>>> Signed-off-by: Varalaxmi Bingi <varalaxmi.bingi@xxxxxxx>
>>> Signed-off-by: Michal Simek <michal.simek@xxxxxxx>
>>> ---
>>>
>>>   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