2021-06-28 12:12:31

by Pali Rohár

[permalink] [raw]
Subject: [PATCH] arm64: dts: armada-3720-turris-mox.dts: remove mrvl,i2c-fast-mode

Some SFP modules are not detected when i2c-fast-mode is enabled even when
clock-frequency is already set to 100000. The I2C bus violates the timing
specifications when run in fast mode. So disable fast mode on Turris Mox.

Same change was already applied for uDPU (also Armada 3720 board with SFP)
in commit fe3ec631a77d ("arm64: dts: uDPU: remove i2c-fast-mode").

Fixes: 7109d817db2e ("arm64: dts: marvell: add DTS for Turris Mox")
Signed-off-by: Pali Rohár <[email protected]>
---
arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
index 6bcc319a0161..27ded36a1a13 100644
--- a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
+++ b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
@@ -119,6 +119,7 @@
pinctrl-names = "default";
pinctrl-0 = <&i2c1_pins>;
clock-frequency = <100000>;
+ /delete-property/mrvl,i2c-fast-mode;
status = "okay";

rtc@6f {
--
2.20.1


2021-06-28 13:07:52

by Marek Behún

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: armada-3720-turris-mox.dts: remove mrvl,i2c-fast-mode

On Mon, 28 Jun 2021 14:10:15 +0200
Pali Rohár <[email protected]> wrote:

> + /delete-property/mrvl,i2c-fast-mode;

Can you add an additional space here?
/delete-property/ mrvl,i2c-fast-mode;

Other than that

Reviewed-by: Marek Behún <[email protected]>


BTW the i2c driver should me made to respect the `clock-frequency` DT
property, not the fast-mode nonsense.

The `mrvl,i2c-fast-mode` should be IMO completely removed and whether it
is enabled should be decided by the value of `clock-frequency`.

BTW there are clock generating registers in A3720 I2C controller that
can generate 100 kHz even in fast-mode. The driver does not use them,
though, it leaves them to their default values, which for normal mode
generate ~96 kHz and for fast mode ~258 kHz. Maybe we should implement
configuring these registers and make the driver choose
normal-mode / fast-mode / high-speed mode depending on
`clock-frequency` ?

Marek

2021-06-28 13:11:41

by Marek Behún

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: armada-3720-turris-mox.dts: remove mrvl,i2c-fast-mode

Also please remove ".dts" from commit title, i.e.:
arm64: dts: armada-3720-turris-mox: remove mrvl,i2c-fast-mode

since "dts" is already present in the prefix.

Marek

2021-06-28 15:35:50

by Pali Rohár

[permalink] [raw]
Subject: [PATCH v2] arm64: dts: armada-3720-turris-mox: remove mrvl,i2c-fast-mode

Some SFP modules are not detected when i2c-fast-mode is enabled even when
clock-frequency is already set to 100000. The I2C bus violates the timing
specifications when run in fast mode. So disable fast mode on Turris Mox.

Same change was already applied for uDPU (also Armada 3720 board with SFP)
in commit fe3ec631a77d ("arm64: dts: uDPU: remove i2c-fast-mode").

Fixes: 7109d817db2e ("arm64: dts: marvell: add DTS for Turris Mox")
Signed-off-by: Pali Rohár <[email protected]>
Reviewed-by: Marek Behún <[email protected]>
---
arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
index 6bcc319a0161..85f15f2a4740 100644
--- a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
+++ b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
@@ -119,6 +119,7 @@
pinctrl-names = "default";
pinctrl-0 = <&i2c1_pins>;
clock-frequency = <100000>;
+ /delete-property/ mrvl,i2c-fast-mode;
status = "okay";

rtc@6f {
--
2.20.1

2021-06-28 23:37:54

by Russell King (Oracle)

[permalink] [raw]
Subject: Re: [PATCH v2] arm64: dts: armada-3720-turris-mox: remove mrvl,i2c-fast-mode

On Mon, Jun 28, 2021 at 05:12:29PM +0200, Pali Roh?r wrote:
> Some SFP modules are not detected when i2c-fast-mode is enabled even when
> clock-frequency is already set to 100000. The I2C bus violates the timing
> specifications when run in fast mode. So disable fast mode on Turris Mox.

Yes. SFP module specification is 100kHz max on the I2C bus, which is
often forgotten about until someone runs into a problem.

Acked-by: Russell King (Oracle) <[email protected]>

--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

2021-07-23 13:01:39

by Gregory CLEMENT

[permalink] [raw]
Subject: Re: [PATCH v2] arm64: dts: armada-3720-turris-mox: remove mrvl,i2c-fast-mode

Hi Pali,

> Some SFP modules are not detected when i2c-fast-mode is enabled even when
> clock-frequency is already set to 100000. The I2C bus violates the timing
> specifications when run in fast mode. So disable fast mode on Turris Mox.
>
> Same change was already applied for uDPU (also Armada 3720 board with SFP)
> in commit fe3ec631a77d ("arm64: dts: uDPU: remove i2c-fast-mode").
>
> Fixes: 7109d817db2e ("arm64: dts: marvell: add DTS for Turris Mox")
> Signed-off-by: Pali Rohár <[email protected]>
> Reviewed-by: Marek Behún <[email protected]>

Applied on mvebu/fixes

Thanks,

Gregory

> ---
> arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> index 6bcc319a0161..85f15f2a4740 100644
> --- a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> +++ b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> @@ -119,6 +119,7 @@
> pinctrl-names = "default";
> pinctrl-0 = <&i2c1_pins>;
> clock-frequency = <100000>;
> + /delete-property/ mrvl,i2c-fast-mode;
> status = "okay";
>
> rtc@6f {
> --
> 2.20.1
>

--
Gregory Clement, Bootlin
Embedded Linux and Kernel engineering
http://bootlin.com