All of lore.kernel.org
 help / color / mirror / Atom feed
From: gregory.clement@bootlin.com (Gregory CLEMENT)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/2] ARM: dts: armada388-clearfog: increase speed of i2c0 to 400kHz
Date: Mon, 05 Mar 2018 17:22:53 +0100	[thread overview]
Message-ID: <878tb65w6a.fsf@bootlin.com> (raw)
In-Reply-To: <20180305160929.GG9418@n2100.armlinux.org.uk> (Russell King's message of "Mon, 5 Mar 2018 16:09:30 +0000")

Hi Russell King,
 
 On lun., mars 05 2018, Russell King - ARM Linux <linux@armlinux.org.uk> wrote:

> On Mon, Mar 05, 2018 at 04:48:54PM +0100, Gregory CLEMENT wrote:
>> Hi Russell,
>>  
>>  On mar., f?vr. 27 2018, Russell King <rmk+kernel@armlinux.org.uk> wrote:
>> 
>> > All the devices on I2C0 support fast mode, so increase the bus speed
>> > to match.
>> 
>> I agree with this part. I didn't pay attention to it in the initial
>> submission but now I've checked it, and indeed according to the
>> datasheet the MCP3021 support "standard (100 kHz) and Fast (400 kHz) I2C
>> Compatible modes"
>> 
>> > The Armada 388 is known to have a timing issue when in
>> > standard mode, which we believe causes the ficticious device at 0x64
>> > to appear.
>> 
>> This errata "FE-8471889 I2C Repeated Start is not supported in standard
>> mode (up to 100K)" was supposed to be fixed with the commit c1d15b68aab8
>> ("i2c: mv64xxx: Fix timing issue on Armada XP (errata FE-8471889)"). And
>> it was used on the Armada 38x SoCs since fbffee74986c ("ARM: dts: Fix I2C
>> repeated start issue on Armada-38x") in 4.14.
>
> The commit pre-dates 4.14, and the commit message was correct at the time
> it was committed (Nov 29 2016).  I don't know whether its still the case
> that the ficticious device still appears, but increasing the bus speed is
> still a sensible thing to do if the devices all support it.

I agree to apply it. I was concerned by the fact that the i2c fix might
not work. I've just tested it and by using the "marvell,mv78230-a0-i2c"
there was no more ficticious device at 100KHz, so I am happy with it.

In order to make know that now the Standard mode is working on Armada
38x, I will just add a the following comment to your commit log:

[gregory.clement at bootlin.com: Note that since the commit fbffee74986c
("ARM: dts: Fix I2C repeated start issue on Armada-38x") in 4.14, the
timing issue is managed for the Armada 38x SoCs.]

Thanks,

Gregory

>
>> So I think that now even at 100kHz the ficticious device should not
>> appear any more. If it is still the case then there might be an issue
>> with the fix in the 2c driver.
>> 
>> Gregory
>> 
>> >
>> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
>> > ---
>> >  arch/arm/boot/dts/armada-388-clearfog.dtsi | 7 ++-----
>> >  1 file changed, 2 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/arch/arm/boot/dts/armada-388-clearfog.dtsi b/arch/arm/boot/dts/armada-388-clearfog.dtsi
>> > index 68acfc968706..0e3b1f140e6d 100644
>> > --- a/arch/arm/boot/dts/armada-388-clearfog.dtsi
>> > +++ b/arch/arm/boot/dts/armada-388-clearfog.dtsi
>> > @@ -143,8 +143,7 @@
>> >  };
>> >  
>> >  &i2c0 {
>> > -	/* Is there anything on this? */
>> > -	clock-frequency = <100000>;
>> > +	clock-frequency = <400000>;
>> >  	pinctrl-0 = <&i2c0_pins>;
>> >  	pinctrl-names = "default";
>> >  	status = "okay";
>> > @@ -239,13 +238,11 @@
>> >  		};
>> >  	};
>> >  
>> > -	/* The MCP3021 is 100kHz clock only */
>> > +	/* The MCP3021 supports standard and fast modes */
>> >  	mikrobus_adc: mcp3021 at 4c {
>> >  		compatible = "microchip,mcp3021";
>> >  		reg = <0x4c>;
>> >  	};
>> > -
>> > -	/* Also something at 0x64 */
>> >  };
>> >  
>> >  &i2c1 {
>> > -- 
>> > 2.7.4
>> >
>> 
>> -- 
>> Gregory Clement, Bootlin (formerly Free Electrons)
>> Embedded Linux and Kernel engineering
>> http://bootlin.com
>
> -- 
> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
> According to speedtest.net: 8.21Mbps down 510kbps up

-- 
Gregory Clement, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com

WARNING: multiple messages have this Message-ID (diff)
From: Gregory CLEMENT <gregory.clement@bootlin.com>
To: Russell King - ARM Linux <linux@armlinux.org.uk>
Cc: Mark Rutland <mark.rutland@arm.com>, Andrew Lunn <andrew@lunn.ch>,
	Jason Cooper <jason@lakedaemon.net>,
	devicetree@vger.kernel.org, Rob Herring <robh+dt@kernel.org>,
	Gregory Clement <gregory.clement@free-electrons.com>,
	linux-arm-kernel@lists.infradead.org,
	Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Subject: Re: [PATCH 1/2] ARM: dts: armada388-clearfog: increase speed of i2c0 to 400kHz
Date: Mon, 05 Mar 2018 17:22:53 +0100	[thread overview]
Message-ID: <878tb65w6a.fsf@bootlin.com> (raw)
In-Reply-To: <20180305160929.GG9418@n2100.armlinux.org.uk> (Russell King's message of "Mon, 5 Mar 2018 16:09:30 +0000")

Hi Russell King,
 
 On lun., mars 05 2018, Russell King - ARM Linux <linux@armlinux.org.uk> wrote:

> On Mon, Mar 05, 2018 at 04:48:54PM +0100, Gregory CLEMENT wrote:
>> Hi Russell,
>>  
>>  On mar., févr. 27 2018, Russell King <rmk+kernel@armlinux.org.uk> wrote:
>> 
>> > All the devices on I2C0 support fast mode, so increase the bus speed
>> > to match.
>> 
>> I agree with this part. I didn't pay attention to it in the initial
>> submission but now I've checked it, and indeed according to the
>> datasheet the MCP3021 support "standard (100 kHz) and Fast (400 kHz) I2C
>> Compatible modes"
>> 
>> > The Armada 388 is known to have a timing issue when in
>> > standard mode, which we believe causes the ficticious device at 0x64
>> > to appear.
>> 
>> This errata "FE-8471889 I2C Repeated Start is not supported in standard
>> mode (up to 100K)" was supposed to be fixed with the commit c1d15b68aab8
>> ("i2c: mv64xxx: Fix timing issue on Armada XP (errata FE-8471889)"). And
>> it was used on the Armada 38x SoCs since fbffee74986c ("ARM: dts: Fix I2C
>> repeated start issue on Armada-38x") in 4.14.
>
> The commit pre-dates 4.14, and the commit message was correct at the time
> it was committed (Nov 29 2016).  I don't know whether its still the case
> that the ficticious device still appears, but increasing the bus speed is
> still a sensible thing to do if the devices all support it.

I agree to apply it. I was concerned by the fact that the i2c fix might
not work. I've just tested it and by using the "marvell,mv78230-a0-i2c"
there was no more ficticious device at 100KHz, so I am happy with it.

In order to make know that now the Standard mode is working on Armada
38x, I will just add a the following comment to your commit log:

[gregory.clement@bootlin.com: Note that since the commit fbffee74986c
("ARM: dts: Fix I2C repeated start issue on Armada-38x") in 4.14, the
timing issue is managed for the Armada 38x SoCs.]

Thanks,

Gregory

>
>> So I think that now even at 100kHz the ficticious device should not
>> appear any more. If it is still the case then there might be an issue
>> with the fix in the 2c driver.
>> 
>> Gregory
>> 
>> >
>> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
>> > ---
>> >  arch/arm/boot/dts/armada-388-clearfog.dtsi | 7 ++-----
>> >  1 file changed, 2 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/arch/arm/boot/dts/armada-388-clearfog.dtsi b/arch/arm/boot/dts/armada-388-clearfog.dtsi
>> > index 68acfc968706..0e3b1f140e6d 100644
>> > --- a/arch/arm/boot/dts/armada-388-clearfog.dtsi
>> > +++ b/arch/arm/boot/dts/armada-388-clearfog.dtsi
>> > @@ -143,8 +143,7 @@
>> >  };
>> >  
>> >  &i2c0 {
>> > -	/* Is there anything on this? */
>> > -	clock-frequency = <100000>;
>> > +	clock-frequency = <400000>;
>> >  	pinctrl-0 = <&i2c0_pins>;
>> >  	pinctrl-names = "default";
>> >  	status = "okay";
>> > @@ -239,13 +238,11 @@
>> >  		};
>> >  	};
>> >  
>> > -	/* The MCP3021 is 100kHz clock only */
>> > +	/* The MCP3021 supports standard and fast modes */
>> >  	mikrobus_adc: mcp3021@4c {
>> >  		compatible = "microchip,mcp3021";
>> >  		reg = <0x4c>;
>> >  	};
>> > -
>> > -	/* Also something at 0x64 */
>> >  };
>> >  
>> >  &i2c1 {
>> > -- 
>> > 2.7.4
>> >
>> 
>> -- 
>> Gregory Clement, Bootlin (formerly Free Electrons)
>> Embedded Linux and Kernel engineering
>> http://bootlin.com
>
> -- 
> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
> According to speedtest.net: 8.21Mbps down 510kbps up

-- 
Gregory Clement, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2018-03-05 16:22 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-27 16:01 [PATCH 0/2] SolidRun Clearfog SFP support Russell King - ARM Linux
2018-02-27 16:01 ` Russell King - ARM Linux
2018-02-27 16:01 ` [PATCH 1/2] ARM: dts: armada388-clearfog: increase speed of i2c0 to 400kHz Russell King
2018-02-27 16:01   ` Russell King
2018-03-05 15:48   ` Gregory CLEMENT
2018-03-05 15:48     ` Gregory CLEMENT
2018-03-05 16:09     ` Russell King - ARM Linux
2018-03-05 16:09       ` Russell King - ARM Linux
2018-03-05 16:22       ` Gregory CLEMENT [this message]
2018-03-05 16:22         ` Gregory CLEMENT
2018-02-27 16:02 ` [PATCH 2/2] ARM: dts: armada388-clearfog: add SFP module support Russell King
2018-02-27 16:02   ` Russell King
2018-03-02 21:56   ` Andrew Lunn
2018-03-02 21:56     ` Andrew Lunn
2018-03-02 22:02     ` Russell King - ARM Linux
2018-03-02 22:02       ` Russell King - ARM Linux
2018-03-02 22:04       ` Andrew Lunn
2018-03-02 22:04         ` Andrew Lunn
2018-03-04 12:13   ` Baruch Siach
2018-03-04 12:13     ` Baruch Siach
2018-03-04 12:49     ` Russell King - ARM Linux
2018-03-04 12:49       ` Russell King - ARM Linux
2018-03-04 13:19       ` Baruch Siach
2018-03-04 13:19         ` Baruch Siach
2018-03-19 21:41   ` Gregory CLEMENT
2018-03-19 21:41     ` Gregory CLEMENT

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=878tb65w6a.fsf@bootlin.com \
    --to=gregory.clement@bootlin.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.