[PATCH 0/3] 98dx3236 i2c related fixes

[PATCH 0/3] 98dx3236 i2c related fixes

[Chris Packham]

I noticed these while adding support for i2c recovery for a couple of our
boards. They date back to when I initially added support for the 98dx3236. They
probably haven't been causing a problem because the HW defaults are correct and
unless you attempt to use the specific pinctrl functions there won't be a
problem.

Chris Packham (3):
  pinctrl: mvebu: Fix i2c sda definition for 98DX3236
  ARM: dts: Remove non-existent i2c1 from 98dx3236
  ARM: dts: Add i2c0 pinctrl information for 98dx3236

 arch/arm/boot/dts/armada-xp-98dx3236.dtsi | 12 +++++++-----
 drivers/pinctrl/mvebu/pinctrl-armada-xp.c |  2 +-
 2 files changed, 8 insertions(+), 6 deletions(-)

-- 
2.28.0

[PATCH 1/3] pinctrl: mvebu: Fix i2c sda definition for 98DX3236

[Chris Packham]

Per the datasheet the i2c functions use MPP_Sel=0x1. They are documented
as using MPP_Sel=0x4 as well but mixing 0x1 and 0x4 is clearly wrong. On
the board tested 0x4 resulted in a non-functioning i2c bus so stick with
0x1 which works.

Fixes: d7ae8f8dee7f ("pinctrl: mvebu: pinctrl driver for 98DX3236 SoC")
Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
 drivers/pinctrl/mvebu/pinctrl-armada-xp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-xp.c b/drivers/pinctrl/mvebu/pinctrl-armada-xp.c
index a767a05fa3a0..48e2a6c56a83 100644
--- a/drivers/pinctrl/mvebu/pinctrl-armada-xp.c
+++ b/drivers/pinctrl/mvebu/pinctrl-armada-xp.c
@@ -414,7 +414,7 @@ static struct mvebu_mpp_mode mv98dx3236_mpp_modes[] = {
 		 MPP_VAR_FUNCTION(0x1, "i2c0", "sck",        V_98DX3236_PLUS)),
 	MPP_MODE(15,
 		 MPP_VAR_FUNCTION(0x0, "gpio", NULL,         V_98DX3236_PLUS),
-		 MPP_VAR_FUNCTION(0x4, "i2c0", "sda",        V_98DX3236_PLUS)),
+		 MPP_VAR_FUNCTION(0x1, "i2c0", "sda",        V_98DX3236_PLUS)),
 	MPP_MODE(16,
 		 MPP_VAR_FUNCTION(0x0, "gpo", NULL,          V_98DX3236_PLUS),
 		 MPP_VAR_FUNCTION(0x4, "dev", "oe",          V_98DX3236_PLUS)),
-- 
2.28.0

Re: [PATCH 1/3] pinctrl: mvebu: Fix i2c sda definition for 98DX3236

[Andrew Lunn]

On Mon, Sep 07, 2020 at 02:41:47PM +1200, Chris Packham wrote:
> Per the datasheet the i2c functions use MPP_Sel=0x1. They are documented
> as using MPP_Sel=0x4 as well but mixing 0x1 and 0x4 is clearly wrong. On
> the board tested 0x4 resulted in a non-functioning i2c bus so stick with
> 0x1 which works.
> 
> Fixes: d7ae8f8dee7f ("pinctrl: mvebu: pinctrl driver for 98DX3236 SoC")
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH 2/3] ARM: dts: Remove non-existent i2c1 from 98dx3236

[Chris Packham]

The switches with integrated CPUs have only got a single i2c controller.
The incorrectly gained one when they were split from the Armada-XP.

Fixes: 43e28ba87708 ("ARM: dts: Use armada-370-xp as a base for armada-xp-98dx3236")
Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
 arch/arm/boot/dts/armada-xp-98dx3236.dtsi | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/arch/arm/boot/dts/armada-xp-98dx3236.dtsi b/arch/arm/boot/dts/armada-xp-98dx3236.dtsi
index 654648b05c7c..aeccedd12574 100644
--- a/arch/arm/boot/dts/armada-xp-98dx3236.dtsi
+++ b/arch/arm/boot/dts/armada-xp-98dx3236.dtsi
@@ -266,11 +266,6 @@ &i2c0 {
 	reg = <0x11000 0x100>;
 };
 
-&i2c1 {
-	compatible = "marvell,mv78230-i2c", "marvell,mv64xxx-i2c";
-	reg = <0x11100 0x100>;
-};
-
 &mpic {
 	reg = <0x20a00 0x2d0>, <0x21070 0x58>;
 };
-- 
2.28.0

Re: [PATCH 2/3] ARM: dts: Remove non-existent i2c1 from 98dx3236

[Andrew Lunn]

On Mon, Sep 07, 2020 at 02:41:48PM +1200, Chris Packham wrote:
> The switches with integrated CPUs have only got a single i2c controller.
> The incorrectly gained one when they were split from the Armada-XP.
> 
> Fixes: 43e28ba87708 ("ARM: dts: Use armada-370-xp as a base for armada-xp-98dx3236")
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH 2/3] ARM: dts: Remove non-existent i2c1 from 98dx3236

[Chris Packham]

On 8/09/20 3:45 am, Andrew Lunn wrote:
> On Mon, Sep 07, 2020 at 02:41:48PM +1200, Chris Packham wrote:
>> The switches with integrated CPUs have only got a single i2c controller.
>> The incorrectly gained one when they were split from the Armada-XP.
Someone pointed out a small grammo instead of "The incorrectly" it 
should be "They incorrectly". Is it worth me sending a v2 just to fix that?
>>
>> Fixes: 43e28ba87708 ("ARM: dts: Use armada-370-xp as a base for armada-xp-98dx3236")
>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
>
>      Andrew

Re: [PATCH 2/3] ARM: dts: Remove non-existent i2c1 from 98dx3236

[Andrew Lunn]

On Mon, Sep 07, 2020 at 09:04:48PM +0000, Chris Packham wrote:
> 
> On 8/09/20 3:45 am, Andrew Lunn wrote:
> > On Mon, Sep 07, 2020 at 02:41:48PM +1200, Chris Packham wrote:
> >> The switches with integrated CPUs have only got a single i2c controller.
> >> The incorrectly gained one when they were split from the Armada-XP.
> Someone pointed out a small grammo instead of "The incorrectly" it 
> should be "They incorrectly". Is it worth me sending a v2 just to fix that?

You are asking somebody who is dyslexic, and often fails to notice
things like this, particularly when written by me :-)

Up to you.

  Andrew

Re: [PATCH 2/3] ARM: dts: Remove non-existent i2c1 from 98dx3236

[Chris Packham]

On 8/09/20 9:07 am, Andrew Lunn wrote:
> On Mon, Sep 07, 2020 at 09:04:48PM +0000, Chris Packham wrote:
>> On 8/09/20 3:45 am, Andrew Lunn wrote:
>>> On Mon, Sep 07, 2020 at 02:41:48PM +1200, Chris Packham wrote:
>>>> The switches with integrated CPUs have only got a single i2c controller.
>>>> The incorrectly gained one when they were split from the Armada-XP.
>> Someone pointed out a small grammo instead of "The incorrectly" it
>> should be "They incorrectly". Is it worth me sending a v2 just to fix that?
> You are asking somebody who is dyslexic, and often fails to notice
> things like this, particularly when written by me :-)
>
> Up to you.
I'll send a v2 fixing the grammo and including your r-by. If the series 
has already been picked up then no great loss.

[PATCH 3/3] ARM: dts: Add i2c0 pinctrl information for 98dx3236

[Chris Packham]

Add pinctrl information for the 98dx3236 (and variants). There is only
one choice for i2c0 MPP14 and MPP15.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
 arch/arm/boot/dts/armada-xp-98dx3236.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/boot/dts/armada-xp-98dx3236.dtsi b/arch/arm/boot/dts/armada-xp-98dx3236.dtsi
index aeccedd12574..38a052a0312d 100644
--- a/arch/arm/boot/dts/armada-xp-98dx3236.dtsi
+++ b/arch/arm/boot/dts/armada-xp-98dx3236.dtsi
@@ -264,6 +264,8 @@ refclk: oscillator {
 &i2c0 {
 	compatible = "marvell,mv78230-i2c", "marvell,mv64xxx-i2c";
 	reg = <0x11000 0x100>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c0_pins>;
 };
 
 &mpic {
@@ -319,6 +321,11 @@ spi0_pins: spi0-pins {
 			       "mpp2", "mpp3";
 		marvell,function = "spi0";
 	};
+
+	i2c0_pins: i2c-pins-0 {
+		marvell,pins = "mpp14", "mpp15";
+		marvell,function = "i2c0";
+	};
 };
 
 &spi0 {
-- 
2.28.0

Re: [PATCH 3/3] ARM: dts: Add i2c0 pinctrl information for 98dx3236

[Andrew Lunn]

On Mon, Sep 07, 2020 at 02:41:49PM +1200, Chris Packham wrote:
> Add pinctrl information for the 98dx3236 (and variants). There is only
> one choice for i2c0 MPP14 and MPP15.
> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew