diff mbox series

[v1,6/6] ARM: BCM5301X: Add DT for Meraki MR32

Message ID 5e9435aabe663fecd76ecf7775bbfd2d60021b16.1597768760.git.chunkeey@gmail.com (mailing list archive)
State New, archived
Headers show
Series [v1,1/6] dt-bindings: ARM: add bindings for the Meraki MR32 | expand

Commit Message

Christian Lamparter Aug. 18, 2020, 4:39 p.m. UTC
This patch adds support for Cisco Meraki MR32.
The unit was donated by Chris Blake. Thank you!

SoC:    Broadcom BCM53016A1 (1 GHz, 2 cores)
RAM:    128 MiB
NAND:   128 MiB Spansion S34ML01G2 (~114 MiB useable)
ETH:    1GBit Ethernet Port - PoE
WIFI1:  Broadcom BCM43520 an+ac (2x2:2 - id: 0x4352)
WIFI2:  Broadcom BCM43520 bgn (2x2:2 - id: 0x4352)
WIFI3:  Broadcom BCM43428 abgn (1x1:1 - id: 43428)

BLE:    Broadcom BCM20732 (ttyS1)
LEDS:   1 x Programmable RGB Status LED (driven by a PWM)
        1 x White LED (GPIO)
        1 x Orange LED Fault Indicator (GPIO)
        2 x LAN Activity / Speed LEDs (On the RJ45 Port)
BUTTON: one Reset button
MISC:   AT24C64 8KiB EEPROM (i2c - stores Ethernet MAC)
        ina219 hardware monitor (i2c)
        Kensington Lock

SERIAL:
	WARNING: The serial port needs a TTL/RS-232 3V3 level converter!
        The Serial setting is 115200-8-N-1. The board has a populated
        right angle 1x4 0.1" pinheader.
        The pinout is: VCC, RX, TX, GND.

Odd stuff:
	- uart0 clock frequency is 62.5 MHz.
	- The LEDs are labeled as SYS-LED1 through SYS-LED3
	  because of the silkscreen on the PCB.
	- the original u-boot has been compiled with most functions
	  and commands disabled. The u-boot env isn't setup properly
	  either and as a result, the bcm47xxpart probing is not
	  working. Hence, the nand partitions are specified through a
	  "fixed-partition" binding.
	- The "WICED SMART(TM)" Bluetooth LE 4.0 BCM20732 chip is
	  connected to uart2 of the SoC. The BCM20732 does not
	  provide a HCI. So the linux' bluetooth stack is useless.
	  The mock-up node with the compatible binding and
	  enable-gpios property is provided solely as documentation.

Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
---
 arch/arm/boot/dts/Makefile                 |   1 +
 arch/arm/boot/dts/bcm53016-meraki-mr32.dts | 191 +++++++++++++++++++++
 2 files changed, 192 insertions(+)
 create mode 100644 arch/arm/boot/dts/bcm53016-meraki-mr32.dts

Comments

Scott Branden Aug. 18, 2020, 6:19 p.m. UTC | #1
Hi Christian,

On 2020-08-18 9:39 a.m., Christian Lamparter wrote:
> This patch adds support for Cisco Meraki MR32.
> The unit was donated by Chris Blake. Thank you!
>
> SoC:    Broadcom BCM53016A1 (1 GHz, 2 cores)
> RAM:    128 MiB
> NAND:   128 MiB Spansion S34ML01G2 (~114 MiB useable)
> ETH:    1GBit Ethernet Port - PoE
> WIFI1:  Broadcom BCM43520 an+ac (2x2:2 - id: 0x4352)
> WIFI2:  Broadcom BCM43520 bgn (2x2:2 - id: 0x4352)
> WIFI3:  Broadcom BCM43428 abgn (1x1:1 - id: 43428)
>
> BLE:    Broadcom BCM20732 (ttyS1)
> LEDS:   1 x Programmable RGB Status LED (driven by a PWM)
>         1 x White LED (GPIO)
>         1 x Orange LED Fault Indicator (GPIO)
>         2 x LAN Activity / Speed LEDs (On the RJ45 Port)
> BUTTON: one Reset button
> MISC:   AT24C64 8KiB EEPROM (i2c - stores Ethernet MAC)
>         ina219 hardware monitor (i2c)
>         Kensington Lock
>
> SERIAL:
> 	WARNING: The serial port needs a TTL/RS-232 3V3 level converter!
>         The Serial setting is 115200-8-N-1. The board has a populated
>         right angle 1x4 0.1" pinheader.
>         The pinout is: VCC, RX, TX, GND.
>
> Odd stuff:
> 	- uart0 clock frequency is 62.5 MHz.
> 	- The LEDs are labeled as SYS-LED1 through SYS-LED3
> 	  because of the silkscreen on the PCB.
> 	- the original u-boot has been compiled with most functions
> 	  and commands disabled. The u-boot env isn't setup properly
> 	  either and as a result, the bcm47xxpart probing is not
> 	  working. Hence, the nand partitions are specified through a
> 	  "fixed-partition" binding.
> 	- The "WICED SMART(TM)" Bluetooth LE 4.0 BCM20732 chip is
> 	  connected to uart2 of the SoC. The BCM20732 does not
> 	  provide a HCI. So the linux' bluetooth stack is useless.
> 	  The mock-up node with the compatible binding and
> 	  enable-gpios property is provided solely as documentation.
>
> Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
> ---
>  arch/arm/boot/dts/Makefile                 |   1 +
>  arch/arm/boot/dts/bcm53016-meraki-mr32.dts | 191 +++++++++++++++++++++
>  2 files changed, 192 insertions(+)
>  create mode 100644 arch/arm/boot/dts/bcm53016-meraki-mr32.dts
>
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index e6a1cac0bfc7..b0756d62772b 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -126,6 +126,7 @@ dtb-$(CONFIG_ARCH_BCM_5301X) += \
>  	bcm47094-luxul-xwr-3100.dtb \
>  	bcm47094-luxul-xwr-3150-v1.dtb \
>  	bcm47094-netgear-r8500.dtb \
> +	bcm53016-meraki-mr32.dtb \
>  	bcm47094-phicomm-k3.dtb \
alpha order would be here.
>  	bcm94708.dtb \
>  	bcm94709.dtb \
> diff --git a/arch/arm/boot/dts/bcm53016-meraki-mr32.dts b/arch/arm/boot/dts/bcm53016-meraki-mr32.dts
> new file mode 100644
> index 000000000000..816fe8cd492d
> --- /dev/null
> +++ b/arch/arm/boot/dts/bcm53016-meraki-mr32.dts
> @@ -0,0 +1,191 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
> +/*
> + * Broadcom BCM470X / BCM5301X ARM platform code.
> + * DTS for Meraki MR32 / Codename: Espresso
> + *
> + * Copyright (C) 2018-2020 Christian Lamparter <chunkeey@gmail.com>
> + */
> +
> +/dts-v1/;
> +
> +#include "bcm4708.dtsi"
> +#include "bcm5301x-nand-cs0-bch8.dtsi"
> +#include <dt-bindings/leds/common.h>
> +
> +/ {
> +	compatible = "meraki,mr32", "brcm,brcm53016", "brcm,bcm4708";
> +	model = "Meraki MR32";
> +
> +	chosen {
> +		bootargs = " console=ttyS0,115200n8 earlycon";
> +	};
> +
> +	memory {
> +		reg = <0x00000000 0x08000000>;
> +	};
> +
> +	aliases {
> +		serial1 = &uart2;
> +	};
> +
> +	leds {
> +		compatible = "gpio-leds";
> +
> +		sysled3 {
> +			function = LED_FUNCTION_FAULT;
> +			color = <LED_COLOR_ID_AMBER>;
> +			gpios = <&chipcommon 18 GPIO_ACTIVE_LOW>;
> +			panic-indicator;
> +		};
> +		sysled2 {
> +			function = LED_FUNCTION_INDICATOR;
> +			color = <LED_COLOR_ID_WHITE>;
> +			gpios = <&chipcommon 19 GPIO_ACTIVE_HIGH>;
> +		};
> +	};
> +
> +	i2c-gpio {
> +		compatible = "i2c-gpio";
> +		sda-gpios = <&chipcommon 5 GPIO_ACTIVE_HIGH>;
> +		scl-gpios = <&chipcommon 4 GPIO_ACTIVE_HIGH>;
> +		i2c-gpio,delay-us = <10>; /* close to 100 kHz */
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		current_sense: ina219@45 {
> +			compatible = "ti,ina219";
> +			reg = <0x45>;
> +			shunt-resistor = <60000>; /* = 60 mOhms */
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +		};
> +
> +		eeprom: eeprom@50 {
> +			compatible = "atmel,24c64";
> +			reg = <0x50>;
> +			pagesize = <32>;
> +			read-only;
> +		};
> +	};
> +
> +	gpio-keys {
> +		compatible = "gpio-keys";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		restart {
> +			label = "Reset";
> +			linux,code = <KEY_RESTART>;
> +			gpios = <&chipcommon 21 GPIO_ACTIVE_LOW>;
> +		};
> +	};
> +
> +	pwm-leds {
> +		compatible = "pwm-leds";
> +
> +		red {
> +			/* SYS-LED 1 - Tricolor */
> +			function = LED_FUNCTION_INDICATOR;
> +			color = <LED_COLOR_ID_RED>;
> +			pwms = <&pwm 0 50000 0>;
> +			max-brightness = <255>;
> +		};
> +
> +		green {
> +			/* SYS-LED 1 - Tricolor */
> +			function = LED_FUNCTION_POWER;
> +			color = <LED_COLOR_ID_GREEN>;
> +			pwms = <&pwm 1 50000 0>;
> +			max-brightness = <255>;
> +		};
> +
> +		blue {
> +			/* SYS-LED 1 - Tricolor */
> +			function = LED_FUNCTION_INDICATOR;
> +			color = <LED_COLOR_ID_BLUE>;
> +			pwms = <&pwm 2 50000 0>;
> +			max-brightness = <255>;
> +		};
> +	};
> +
> +};
> +
> +&uart0 {
> +	clock-frequency = <62500000>;
> +	/delete-property/ clocks;
> +};
> +
> +&uart1 {
> +	status = "disabled";
> +};
> +
> +&uart2 {
> +	status = "okay";
> +	/*
> +	 * bluetooth-le {
> +	 *	compatible = "brcm,bcm20732";
> +	 *	enable-gpios = <&chipcommon 20 GPIO_ACTIVE_HIGH>;
> +	 *};
> +	 */
> +};
> +
> +&gmac1 {
> +	status = "disabled";
> +};
> +&gmac2 {
> +	status = "disabled";
> +};
> +&gmac3 {
> +	status = "disabled";
> +};
> +
> +&pwm {
> +	status = "okay";
> +};
> +
> +&nandcs {
> +	nand-ecc-algo = "hw";
> +
> +	partitions {
> +		/*
> +		 * The partition autodetection does not work for this devi
> +		 * It will only detect the "nvram" partition with an incorrect size.
> +		 *	[    1.721667] 1 bcm47xxpart partitions found on MTD device brcmnand.0
> +		 *	[    1.727962] Creating 1 MTD partitions on "brcmnand.0":
> +		 *	[    1.733117] 0x000000400000-0x000008000000 : "nvram"
> +		 */
> +
If autodetection doesn't work..
the partition info should be passed in via the bootloader rather than the DT?
> +		compatible = "fixed-partitions";
> +		#address-cells = <0x1>;
> +		#size-cells = <0x1>;
> +
> +		partition0@0 {
> +			label = "u-boot";
> +			reg = <0x0 0x100000>;
> +			read-only;
> +		};
> +
> +		partition1@100000 {
> +			label = "bootkernel1";
> +			reg = <0x100000 0x300000>;
> +			read-only;
> +		};
> +
> +		partition2@400000 {
> +			label = "nvram";
> +			reg = <0x400000 0x100000>;
> +			read-only;
> +		};
> +
> +		partition3@500000 {
> +			label = "bootkernel2";
> +			reg = <0x500000 0x300000>;
> +			read-only;
> +		};
> +
> +		partition4@800000 {
> +			label = "ubi";
> +			reg = <0x800000 0x7780000>;
> +		};
> +	};
> +};
Christian Lamparter Aug. 18, 2020, 7:19 p.m. UTC | #2
Hi Scott,

On 2020-08-18 20:19, Scott Branden wrote:
> On 2020-08-18 9:39 a.m., Christian Lamparter wrote:
>> This patch adds support for Cisco Meraki MR32.
[...]
>> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
>> index e6a1cac0bfc7..b0756d62772b 100644
>> --- a/arch/arm/boot/dts/Makefile
>> +++ b/arch/arm/boot/dts/Makefile
>> @@ -126,6 +126,7 @@ dtb-$(CONFIG_ARCH_BCM_5301X) += \
>>   	bcm47094-luxul-xwr-3100.dtb \
>>   	bcm47094-luxul-xwr-3150-v1.dtb \
>>   	bcm47094-netgear-r8500.dtb \
>> +	bcm53016-meraki-mr32.dtb \
>>   	bcm47094-phicomm-k3.dtb \
> alpha order would be here.

Yes. That's true. I should have posted this patch sooner ;).

>>   	bcm94708.dtb \ >>   	bcm94709.dtb \
>> diff --git a/arch/arm/boot/dts/bcm53016-meraki-mr32.dts b/arch/arm/boot/dts/bcm53016-meraki-mr32.dts
>> new file mode 100644
>> index 000000000000..816fe8cd492d
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/bcm53016-meraki-mr32.dts
>> @@ -0,0 +1,191 @@
 >> [...]
>> +
>> +&nandcs {
>> +	nand-ecc-algo = "hw";
>> +
>> +	partitions {
>> +		/*
>> +		 * The partition autodetection does not work for this devi
>> +		 * It will only detect the "nvram" partition with an incorrect size.
>> +		 *	[    1.721667] 1 bcm47xxpart partitions found on MTD device brcmnand.0
>> +		 *	[    1.727962] Creating 1 MTD partitions on "brcmnand.0":
>> +		 *	[    1.733117] 0x000000400000-0x000008000000 : "nvram"
>> +		 */
>> +
> If autodetection doesn't work..
> the partition info should be passed in via the bootloader rather than the DT?

No, Meraki embedded the partition table in the now "old-way" in their 
Device-Tree file too :).

Thanks to Chris Blake's GPL inquiry, I can just provide a link to the 
nand-node (with the partition child nodes) of the original 
"espresso.dts" [0]. (The espresso.dts file is licensed GPL2 - see 
boilerplate.)

|	nand {
|		#address-cells = <1>;
|		#size-cells = <1>;
|		compatible = "iproc,nand-iproc";
|
|		partition@0 {
|			label = "U-boot";
|			reg = <0x00000000 0x100000>;
|		};
|
|		partition@1 {
|			label = "bootkernel1";
|			reg = <0x100000 0x300000>;
|		};
|		partition@2 {
|			label = "senao_nvram";
|			reg = <0x400000 0x100000>;
|		};
|		[...]

>> +		compatible = "fixed-partitions";
>> +		#address-cells = <0x1>;
>> +		#size-cells = <0x1>;
>> +
>> +		partition0@0 {
>> +			label = "u-boot";
>> +			reg = <0x0 0x100000>;
>> +			read-only;
>> +		};
>> +
>> +		partition1@100000 {
>> +			label = "bootkernel1";
>> +			reg = <0x100000 0x300000>;
>> +			read-only;
>> +		};
>> +
>> +		partition2@400000 {
>> +			label = "nvram";
>> +			reg = <0x400000 0x100000>;
>> +			read-only;
>> +		};
>> +
>> +		partition3@500000 {
>> +			label = "bootkernel2";
>> +			reg = <0x500000 0x300000>;
>> +			read-only;
>> +		};
>> +
>> +		partition4@800000 {
>> +			label = "ubi";
>> +			reg = <0x800000 0x7780000>;
>> +		};
>> +	};
>> +};

Thank you for the fast review. I'll stick around and post v2 when
everyone had a good look.

Cheers,
Christian


[0] 
<https://github.com/riptidewave93/meraki-linux/blob/r24-linux-3.4-20170427/arch/arm/boot/dts/espresso.dts#L99>
Florian Fainelli Aug. 18, 2020, 8 p.m. UTC | #3
On 8/18/2020 9:39 AM, Christian Lamparter wrote:
> This patch adds support for Cisco Meraki MR32.
> The unit was donated by Chris Blake. Thank you!
> 
> SoC:    Broadcom BCM53016A1 (1 GHz, 2 cores)
> RAM:    128 MiB
> NAND:   128 MiB Spansion S34ML01G2 (~114 MiB useable)
> ETH:    1GBit Ethernet Port - PoE
> WIFI1:  Broadcom BCM43520 an+ac (2x2:2 - id: 0x4352)
> WIFI2:  Broadcom BCM43520 bgn (2x2:2 - id: 0x4352)
> WIFI3:  Broadcom BCM43428 abgn (1x1:1 - id: 43428)
> 
> BLE:    Broadcom BCM20732 (ttyS1)
> LEDS:   1 x Programmable RGB Status LED (driven by a PWM)
>          1 x White LED (GPIO)
>          1 x Orange LED Fault Indicator (GPIO)
>          2 x LAN Activity / Speed LEDs (On the RJ45 Port)
> BUTTON: one Reset button
> MISC:   AT24C64 8KiB EEPROM (i2c - stores Ethernet MAC)
>          ina219 hardware monitor (i2c)
>          Kensington Lock
> 
> SERIAL:
> 	WARNING: The serial port needs a TTL/RS-232 3V3 level converter!
>          The Serial setting is 115200-8-N-1. The board has a populated
>          right angle 1x4 0.1" pinheader.
>          The pinout is: VCC, RX, TX, GND.
> 
> Odd stuff:
> 	- uart0 clock frequency is 62.5 MHz.
> 	- The LEDs are labeled as SYS-LED1 through SYS-LED3
> 	  because of the silkscreen on the PCB.
> 	- the original u-boot has been compiled with most functions
> 	  and commands disabled. The u-boot env isn't setup properly
> 	  either and as a result, the bcm47xxpart probing is not
> 	  working. Hence, the nand partitions are specified through a
> 	  "fixed-partition" binding.
> 	- The "WICED SMART(TM)" Bluetooth LE 4.0 BCM20732 chip is
> 	  connected to uart2 of the SoC. The BCM20732 does not
> 	  provide a HCI. So the linux' bluetooth stack is useless.
> 	  The mock-up node with the compatible binding and
> 	  enable-gpios property is provided solely as documentation.
> 
> Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
> ---
>   arch/arm/boot/dts/Makefile                 |   1 +
>   arch/arm/boot/dts/bcm53016-meraki-mr32.dts | 191 +++++++++++++++++++++
>   2 files changed, 192 insertions(+)
>   create mode 100644 arch/arm/boot/dts/bcm53016-meraki-mr32.dts
> 
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index e6a1cac0bfc7..b0756d62772b 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -126,6 +126,7 @@ dtb-$(CONFIG_ARCH_BCM_5301X) += \
>   	bcm47094-luxul-xwr-3100.dtb \
>   	bcm47094-luxul-xwr-3150-v1.dtb \
>   	bcm47094-netgear-r8500.dtb \
> +	bcm53016-meraki-mr32.dtb \
>   	bcm47094-phicomm-k3.dtb \
>   	bcm94708.dtb \
>   	bcm94709.dtb \
> diff --git a/arch/arm/boot/dts/bcm53016-meraki-mr32.dts b/arch/arm/boot/dts/bcm53016-meraki-mr32.dts
> new file mode 100644
> index 000000000000..816fe8cd492d
> --- /dev/null
> +++ b/arch/arm/boot/dts/bcm53016-meraki-mr32.dts
> @@ -0,0 +1,191 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
> +/*
> + * Broadcom BCM470X / BCM5301X ARM platform code.
> + * DTS for Meraki MR32 / Codename: Espresso
> + *
> + * Copyright (C) 2018-2020 Christian Lamparter <chunkeey@gmail.com>
> + */
> +
> +/dts-v1/;
> +
> +#include "bcm4708.dtsi"
> +#include "bcm5301x-nand-cs0-bch8.dtsi"
> +#include <dt-bindings/leds/common.h>
> +
> +/ {
> +	compatible = "meraki,mr32", "brcm,brcm53016", "brcm,bcm4708";
> +	model = "Meraki MR32";
> +
> +	chosen {
> +		bootargs = " console=ttyS0,115200n8 earlycon";
> +	};
> +
> +	memory {
> +		reg = <0x00000000 0x08000000>;

Should not this include a device_type = "memory" property? Also, it 
should be memory@0 to avoid DTC warnings.

[snip]

> +
> +&gmac1 {
> +	status = "disabled";
> +};
> +&gmac2 {
> +	status = "disabled";
> +};
> +&gmac3 {
> +	status = "disabled";
> +};

We should probably make the various gmac nodes disabled by default at 
some point.
Florian Fainelli Aug. 18, 2020, 8:11 p.m. UTC | #4
On 8/18/2020 9:39 AM, Christian Lamparter wrote:
[snip]
> +	i2c-gpio {
> +		compatible = "i2c-gpio";
> +		sda-gpios = <&chipcommon 5 GPIO_ACTIVE_HIGH>;
> +		scl-gpios = <&chipcommon 4 GPIO_ACTIVE_HIGH>;
> +		i2c-gpio,delay-us = <10>; /* close to 100 kHz */
> +		#address-cells = <1>;
> +		#size-cells = <0>;

Can you try using the hardware controller here instead of bit banging 
i2c over gpios? GPIOs 4 and 5 are the default pins for I2C.
Christian Lamparter Aug. 18, 2020, 10:52 p.m. UTC | #5
Hello Florian,

On 2020-08-18 22:11, Florian Fainelli wrote:
> 
> 
> On 8/18/2020 9:39 AM, Christian Lamparter wrote:
> [snip]
>> +    i2c-gpio {
>> +        compatible = "i2c-gpio";
>> +        sda-gpios = <&chipcommon 5 GPIO_ACTIVE_HIGH>;
>> +        scl-gpios = <&chipcommon 4 GPIO_ACTIVE_HIGH>;
>> +        i2c-gpio,delay-us = <10>; /* close to 100 kHz */
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
> 
> Can you try using the hardware controller here instead of bit banging 
> i2c over gpios? GPIOs 4 and 5 are the default pins for I2C.

Ok. I gave this a try. What I did was:

I removed the i2c-gpio node and went with this in the mr32.dts:

+&i2c0 {
+       status = "okay";
+
+	clock-frequency = <100000>; /* also tried 400KHz */
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinmux_i2c>; /* editted bcm5301x.dtsi for this */
+
+       cur_mon: ina2xx@45 {
+               compatible = "ti,ina219";
+               reg = <0x45>;
+               shunt-resistor = <60000>; /* = 60 mOhms */
+       };
+
+       meraki_eeprom: at24@50 {
+               compatible = "atmel,24c64";
+               reg = <0x50>;
+               pagesize = <32>;
+               read-only;
+       };
+};

I enabled CONFIG_I2C_BCM_IPROC=y and built the whole thing and moved it 
to the AP.

During boot, I now get:

|[    1.039174] bcm-iproc-i2c 18009000.i2c: bus set to 100000 Hz
|[    8.918419] i2c /dev entries driver
[...] (The i2c-bcm-iproc now causes a long "wait" during boot)
|[   59.385497] bcm-iproc-i2c 18009000.i2c: transaction timed out
|[   59.391272] ina2xx 0-0045: error configuring the device: -110
|[  110.585517] bcm-iproc-i2c 18009000.i2c: transaction timed out

Is there a special magic needed to get this working with bcm5301x's 
existing i2c0 node?

Cheers,
Christian

Here are some dumps from debugfs' pinctrl and the bcm5301x.dtsi patch

--- /sys/kernel/debug/pinctrl/pinctrl-handles ---
Requested pin control handlers their pinmux maps:
device: 18009000.i2c current state: default
   state: default
     type: MUX_GROUP controller pinctrl-ns group: i2c_grp (1) function: 
i2c (1)


--- /sys/kernel/debug/pinctrl/1800c100.cru:pinctrl/pinmux-pins ---
Pinmux settings per pin
Format: pin (name): mux_owner gpio_owner hog?
pin 0 (spi_clk): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 1 (spi_ss): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 2 (spi_mosi): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 3 (spi_miso): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 4 (i2c_scl): 18009000.i2c (GPIO UNCLAIMED) function i2c group i2c_grp
pin 5 (i2c_sda): 18009000.i2c (GPIO UNCLAIMED) function i2c group i2c_grp
pin 8 (pwm0): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 9 (pwm1): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 10 (pwm2): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 11 (pwm3): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 12 (uart1_rx): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 13 (uart1_tx): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 14 (uart1_cts): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 15 (uart1_rts): (MUX UNCLAIMED) (GPIO UNCLAIMED)
---

---
diff --git a/arch/arm/boot/dts/bcm5301x.dtsi 
b/arch/arm/boot/dts/bcm5301x.dtsi
index f7bd1587e285..9bb97deb7331 100644
--- a/arch/arm/boot/dts/bcm5301x.dtsi
+++ b/arch/arm/boot/dts/bcm5301x.dtsi
@@ -438,7 +438,7 @@ spi-pins {
  					function = "spi";
  				};

-				i2c {
+				pinmux_i2c: i2c {
  					groups = "i2c_grp";
  					function = "i2c";
  				};
Ray Jui Aug. 19, 2020, 12:07 a.m. UTC | #6
On 8/18/2020 3:52 PM, Christian Lamparter wrote:
> Hello Florian,
> 
> On 2020-08-18 22:11, Florian Fainelli wrote:
>>
>>
>> On 8/18/2020 9:39 AM, Christian Lamparter wrote:
>> [snip]
>>> +    i2c-gpio {
>>> +        compatible = "i2c-gpio";
>>> +        sda-gpios = <&chipcommon 5 GPIO_ACTIVE_HIGH>;
>>> +        scl-gpios = <&chipcommon 4 GPIO_ACTIVE_HIGH>;
>>> +        i2c-gpio,delay-us = <10>; /* close to 100 kHz */
>>> +        #address-cells = <1>;
>>> +        #size-cells = <0>;
>>
>> Can you try using the hardware controller here instead of bit banging
>> i2c over gpios? GPIOs 4 and 5 are the default pins for I2C.
> 
> Ok. I gave this a try. What I did was:
> 
> I removed the i2c-gpio node and went with this in the mr32.dts:
> 
> +&i2c0 {
> +       status = "okay";
> +
> +    clock-frequency = <100000>; /* also tried 400KHz */
> +    pinctrl-names = "default";
> +    pinctrl-0 = <&pinmux_i2c>; /* editted bcm5301x.dtsi for this */
> +
> +       cur_mon: ina2xx@45 {
> +               compatible = "ti,ina219";
> +               reg = <0x45>;
> +               shunt-resistor = <60000>; /* = 60 mOhms */
> +       };
> +
> +       meraki_eeprom: at24@50 {
> +               compatible = "atmel,24c64";
> +               reg = <0x50>;
> +               pagesize = <32>;
> +               read-only;
> +       };
> +};
> 
> I enabled CONFIG_I2C_BCM_IPROC=y and built the whole thing and moved it
> to the AP.
> 
> During boot, I now get:
> 
> |[    1.039174] bcm-iproc-i2c 18009000.i2c: bus set to 100000 Hz
> |[    8.918419] i2c /dev entries driver
> [...] (The i2c-bcm-iproc now causes a long "wait" during boot)
> |[   59.385497] bcm-iproc-i2c 18009000.i2c: transaction timed out
> |[   59.391272] ina2xx 0-0045: error configuring the device: -110
> |[  110.585517] bcm-iproc-i2c 18009000.i2c: transaction timed out
> 

The long wait is probably caused by waiting for the I2C transaction to
time out, which it eventually did.

> Is there a special magic needed to get this working with bcm5301x's
> existing i2c0 node?
> 

Two things to check: 1) if pinmux is configured properly; 2) if
interrupt line is configured properly.

After the long wait and errot, can you check cat /proc/interrupts to see
if there's any I2C interrupt fired?

> Cheers,
> Christian
> 
> Here are some dumps from debugfs' pinctrl and the bcm5301x.dtsi patch
> 
> --- /sys/kernel/debug/pinctrl/pinctrl-handles ---
> Requested pin control handlers their pinmux maps:
> device: 18009000.i2c current state: default
>   state: default
>     type: MUX_GROUP controller pinctrl-ns group: i2c_grp (1) function:
> i2c (1)
> 
> 
> --- /sys/kernel/debug/pinctrl/1800c100.cru:pinctrl/pinmux-pins ---
> Pinmux settings per pin
> Format: pin (name): mux_owner gpio_owner hog?
> pin 0 (spi_clk): (MUX UNCLAIMED) (GPIO UNCLAIMED)
> pin 1 (spi_ss): (MUX UNCLAIMED) (GPIO UNCLAIMED)
> pin 2 (spi_mosi): (MUX UNCLAIMED) (GPIO UNCLAIMED)
> pin 3 (spi_miso): (MUX UNCLAIMED) (GPIO UNCLAIMED)
> pin 4 (i2c_scl): 18009000.i2c (GPIO UNCLAIMED) function i2c group i2c_grp
> pin 5 (i2c_sda): 18009000.i2c (GPIO UNCLAIMED) function i2c group i2c_grp

This looks right at the framework level, but in case someone who coded
the pinmux driver screwed up the table mapping, I would suggest using
devmem to dump out the actual readings of the pinmux registers to be
100% certain.

> pin 8 (pwm0): (MUX UNCLAIMED) (GPIO UNCLAIMED)
> pin 9 (pwm1): (MUX UNCLAIMED) (GPIO UNCLAIMED)
> pin 10 (pwm2): (MUX UNCLAIMED) (GPIO UNCLAIMED)
> pin 11 (pwm3): (MUX UNCLAIMED) (GPIO UNCLAIMED)
> pin 12 (uart1_rx): (MUX UNCLAIMED) (GPIO UNCLAIMED)
> pin 13 (uart1_tx): (MUX UNCLAIMED) (GPIO UNCLAIMED)
> pin 14 (uart1_cts): (MUX UNCLAIMED) (GPIO UNCLAIMED)
> pin 15 (uart1_rts): (MUX UNCLAIMED) (GPIO UNCLAIMED)
> ---
> 
> ---
> diff --git a/arch/arm/boot/dts/bcm5301x.dtsi
> b/arch/arm/boot/dts/bcm5301x.dtsi
> index f7bd1587e285..9bb97deb7331 100644
> --- a/arch/arm/boot/dts/bcm5301x.dtsi
> +++ b/arch/arm/boot/dts/bcm5301x.dtsi
> @@ -438,7 +438,7 @@ spi-pins {
>                      function = "spi";
>                  };
> 
> -                i2c {
> +                pinmux_i2c: i2c {
>                      groups = "i2c_grp";
>                      function = "i2c";
>                  };
>
Christian Lamparter Aug. 19, 2020, 2:46 a.m. UTC | #7
On 2020-08-19 02:07, Ray Jui wrote:
> 
> 
> On 8/18/2020 3:52 PM, Christian Lamparter wrote:
>> Hello Florian,
>>
>> On 2020-08-18 22:11, Florian Fainelli wrote:
>>>
>>>
>>> On 8/18/2020 9:39 AM, Christian Lamparter wrote:
>>> [snip]
>>>> +    i2c-gpio {
>>>> +        compatible = "i2c-gpio";
>>>> +        sda-gpios = <&chipcommon 5 GPIO_ACTIVE_HIGH>;
>>>> +        scl-gpios = <&chipcommon 4 GPIO_ACTIVE_HIGH>;
>>>> +        i2c-gpio,delay-us = <10>; /* close to 100 kHz */
>>>> +        #address-cells = <1>;
>>>> +        #size-cells = <0>;
>>>
>>> Can you try using the hardware controller here instead of bit banging
>>> i2c over gpios? GPIOs 4 and 5 are the default pins for I2C.
>>
>> Ok. I gave this a try. What I did was:
>>
>> I removed the i2c-gpio node and went with this in the mr32.dts:
>>
>> +&i2c0 {
>> +       status = "okay";
>> +
>> +    clock-frequency = <100000>; /* also tried 400KHz */
>> +    pinctrl-names = "default";
>> +    pinctrl-0 = <&pinmux_i2c>; /* editted bcm5301x.dtsi for this */
>> +
>> +       cur_mon: ina2xx@45 {
>> +               compatible = "ti,ina219";
>> +               reg = <0x45>;
>> +               shunt-resistor = <60000>; /* = 60 mOhms */
>> +       };
>> +
>> +       meraki_eeprom: at24@50 {
>> +               compatible = "atmel,24c64";
>> +               reg = <0x50>;
>> +               pagesize = <32>;
>> +               read-only;
>> +       };
>> +};
>>
>> I enabled CONFIG_I2C_BCM_IPROC=y and built the whole thing and moved it
>> to the AP.
>>
>> During boot, I now get:
>>
>> |[    1.039174] bcm-iproc-i2c 18009000.i2c: bus set to 100000 Hz
>> |[    8.918419] i2c /dev entries driver
>> [...] (The i2c-bcm-iproc now causes a long "wait" during boot)
>> |[   59.385497] bcm-iproc-i2c 18009000.i2c: transaction timed out
>> |[   59.391272] ina2xx 0-0045: error configuring the device: -110
>> |[  110.585517] bcm-iproc-i2c 18009000.i2c: transaction timed out
>>
> 
> The long wait is probably caused by waiting for the I2C transaction to
> time out, which it eventually did.

Yes.

 >
> 
>> Is there a special magic needed to get this working with bcm5301x's
>> existing i2c0 node?
>>
> 
> Two things to check: 1) if pinmux is configured properly; 
Hm, any tips for testing this? The /sys/kernel/debug/pinctrl is 
populated correctly from what I can tell.

What I don't know is if the DTS in bcm5301x.dtsi.

|        dmu@1800c000 {
|                compatible = "simple-bus";
|                ranges = <0 0x1800c000 0x1000>;
|                #address-cells = <1>;
|                #size-cells = <1>;
|
|                cru@100 {
|                        compatible = "simple-bus";
|                        reg = <0x100 0x1a4>;
|                        ranges;
|                        #address-cells = <1>;
|                        #size-cells = <1>;
|
|                        pin-controller@1c0 {
|                                compatible = "brcm,bcm4708-pinmux";
|                                reg = <0x1c0 0x24>;

Based on my understanding of the DT, the pinctrl register should be at 
0x1800c2c0 (that would be outside of the 0x1a4 size though?!). This is
because of 0x1800c000 (dmu base) + 0x100 (cru reg) + 0x1c0 
(pin-controller reg). If so, here are devmem's output of said region 
(read value is after the "=")

devmem 0x1800c2c0 = 0x0
devmem 0x1800c2c4 = 0x001D2003
devmem 0x1800c2c8 = 0x00000284
devmem 0x1800c2cc = 0x00000284
devmem 0x1800c2d0 = 0x00000285
devmem 0x1800c2d4 = 0x00000284
devmem 0x1800c2d8 = 0x00000284
devmem 0x1800c2dc = 0x00000284
devmem 0x1800c2e0 = 0x00000284
devmem 0x1800c2e4 = 0x00000284

Just in case, I've also checked 0x1800c1c0.
This is because I stumbled over the "Example" in the binding 
Documentation under 
Documentation/devicetree/bindings/pinctrl/brcm,bcm4708-pinmux.txt. 
Because this uses a "offset = <0xc0>"
property and seems to be based of the cru@100 node?!

devmem 0x1800c1c0 = 0x00140037
devmem 0x1800c1c4 = 0x0
devmem 0x1800c1c8 = 0x00FFFFFF
devmem 0x1800c1cc = 0x00FFFFFF
devmem 0x1800c1d0 = 0x0
devmem 0x1800c1d4 = 0x0
devmem 0x1800c1d8 = 0x0
devmem 0x1800c1dc = 0x0
devmem 0x1800c1e0 = 0x0
devmem 0x1800c1e4 = 0x00000283

(I also saw the compatible, in my case it should be 
"brcm,bcm53012-pinmux". Can we get a pinmux: label for the pin-controller?)

Can someone please look up the correct address of the pinctrl register,
or confirm, that this pinctrl for the NorthStar is working as expected?

> 2) if
> interrupt line is configured properly.
 >
> 
> After the long wait and errot, can you check cat /proc/interrupts to see
> if there's any I2C interrupt fired?
No, I don't see any with the combinations listed above. The Interrupt 
count remained at "0" the whole time for the i2c. (From what I can tell, 
the i2c-bcm-iproc also supports polling. But it didn't help either)

  23:          0          0     GIC-0 153 Level     18009000.i2c
  Err:          0

Cheers,
Christian

>> Here are some dumps from debugfs' pinctrl and the bcm5301x.dtsi patch
>>
>> --- /sys/kernel/debug/pinctrl/pinctrl-handles ---
>> Requested pin control handlers their pinmux maps:
>> device: 18009000.i2c current state: default
>>    state: default
>>      type: MUX_GROUP controller pinctrl-ns group: i2c_grp (1) function:
>> i2c (1)
>>
>>
>> --- /sys/kernel/debug/pinctrl/1800c100.cru:pinctrl/pinmux-pins ---
>> Pinmux settings per pin
>> Format: pin (name): mux_owner gpio_owner hog?
>> pin 0 (spi_clk): (MUX UNCLAIMED) (GPIO UNCLAIMED)
>> pin 1 (spi_ss): (MUX UNCLAIMED) (GPIO UNCLAIMED)
>> pin 2 (spi_mosi): (MUX UNCLAIMED) (GPIO UNCLAIMED)
>> pin 3 (spi_miso): (MUX UNCLAIMED) (GPIO UNCLAIMED)
>> pin 4 (i2c_scl): 18009000.i2c (GPIO UNCLAIMED) function i2c group i2c_grp
>> pin 5 (i2c_sda): 18009000.i2c (GPIO UNCLAIMED) function i2c group i2c_grp
> 
> This looks right at the framework level, but in case someone who coded
> the pinmux driver screwed up the table mapping, I would suggest using
> devmem to dump out the actual readings of the pinmux registers to be
> 100% certain.
> 
>> pin 8 (pwm0): (MUX UNCLAIMED) (GPIO UNCLAIMED)
>> pin 9 (pwm1): (MUX UNCLAIMED) (GPIO UNCLAIMED)
>> pin 10 (pwm2): (MUX UNCLAIMED) (GPIO UNCLAIMED)
>> pin 11 (pwm3): (MUX UNCLAIMED) (GPIO UNCLAIMED)
>> pin 12 (uart1_rx): (MUX UNCLAIMED) (GPIO UNCLAIMED)
>> pin 13 (uart1_tx): (MUX UNCLAIMED) (GPIO UNCLAIMED)
>> pin 14 (uart1_cts): (MUX UNCLAIMED) (GPIO UNCLAIMED)
>> pin 15 (uart1_rts): (MUX UNCLAIMED) (GPIO UNCLAIMED)
>> ---
>>
>> ---
>> diff --git a/arch/arm/boot/dts/bcm5301x.dtsi
>> b/arch/arm/boot/dts/bcm5301x.dtsi
>> index f7bd1587e285..9bb97deb7331 100644
>> --- a/arch/arm/boot/dts/bcm5301x.dtsi
>> +++ b/arch/arm/boot/dts/bcm5301x.dtsi
>> @@ -438,7 +438,7 @@ spi-pins {
>>                       function = "spi";
>>                   };
>>
>> -                i2c {
>> +                pinmux_i2c: i2c {
>>                       groups = "i2c_grp";
>>                       function = "i2c";
>>                   };
>>
Florian Fainelli Aug. 19, 2020, 3:34 a.m. UTC | #8
On 8/18/2020 7:46 PM, Christian Lamparter wrote:
> On 2020-08-19 02:07, Ray Jui wrote:
>>
>>
>> On 8/18/2020 3:52 PM, Christian Lamparter wrote:
>>> Hello Florian,
>>>
>>> On 2020-08-18 22:11, Florian Fainelli wrote:
>>>>
>>>>
>>>> On 8/18/2020 9:39 AM, Christian Lamparter wrote:
>>>> [snip]
>>>>> +    i2c-gpio {
>>>>> +        compatible = "i2c-gpio";
>>>>> +        sda-gpios = <&chipcommon 5 GPIO_ACTIVE_HIGH>;
>>>>> +        scl-gpios = <&chipcommon 4 GPIO_ACTIVE_HIGH>;
>>>>> +        i2c-gpio,delay-us = <10>; /* close to 100 kHz */
>>>>> +        #address-cells = <1>;
>>>>> +        #size-cells = <0>;
>>>>
>>>> Can you try using the hardware controller here instead of bit banging
>>>> i2c over gpios? GPIOs 4 and 5 are the default pins for I2C.
>>>
>>> Ok. I gave this a try. What I did was:
>>>
>>> I removed the i2c-gpio node and went with this in the mr32.dts:
>>>
>>> +&i2c0 {
>>> +       status = "okay";
>>> +
>>> +    clock-frequency = <100000>; /* also tried 400KHz */
>>> +    pinctrl-names = "default";
>>> +    pinctrl-0 = <&pinmux_i2c>; /* editted bcm5301x.dtsi for this */
>>> +
>>> +       cur_mon: ina2xx@45 {
>>> +               compatible = "ti,ina219";
>>> +               reg = <0x45>;
>>> +               shunt-resistor = <60000>; /* = 60 mOhms */
>>> +       };
>>> +
>>> +       meraki_eeprom: at24@50 {
>>> +               compatible = "atmel,24c64";
>>> +               reg = <0x50>;
>>> +               pagesize = <32>;
>>> +               read-only;
>>> +       };
>>> +};
>>>
>>> I enabled CONFIG_I2C_BCM_IPROC=y and built the whole thing and moved it
>>> to the AP.
>>>
>>> During boot, I now get:
>>>
>>> |[    1.039174] bcm-iproc-i2c 18009000.i2c: bus set to 100000 Hz
>>> |[    8.918419] i2c /dev entries driver
>>> [...] (The i2c-bcm-iproc now causes a long "wait" during boot)
>>> |[   59.385497] bcm-iproc-i2c 18009000.i2c: transaction timed out
>>> |[   59.391272] ina2xx 0-0045: error configuring the device: -110
>>> |[  110.585517] bcm-iproc-i2c 18009000.i2c: transaction timed out
>>>
>>
>> The long wait is probably caused by waiting for the I2C transaction to
>> time out, which it eventually did.
> 
> Yes.
> 
>  >
>>
>>> Is there a special magic needed to get this working with bcm5301x's
>>> existing i2c0 node?
>>>
>>
>> Two things to check: 1) if pinmux is configured properly; 
> Hm, any tips for testing this? The /sys/kernel/debug/pinctrl is 
> populated correctly from what I can tell.
> 
> What I don't know is if the DTS in bcm5301x.dtsi.
> 
> |        dmu@1800c000 {
> |                compatible = "simple-bus";
> |                ranges = <0 0x1800c000 0x1000>;
> |                #address-cells = <1>;
> |                #size-cells = <1>;
> |
> |                cru@100 {
> |                        compatible = "simple-bus";
> |                        reg = <0x100 0x1a4>;
> |                        ranges;
> |                        #address-cells = <1>;
> |                        #size-cells = <1>;
> |
> |                        pin-controller@1c0 {
> |                                compatible = "brcm,bcm4708-pinmux";
> |                                reg = <0x1c0 0x24>;
> 
> Based on my understanding of the DT, the pinctrl register should be at 
> 0x1800c2c0 (that would be outside of the 0x1a4 size though?!). This is
> because of 0x1800c000 (dmu base) + 0x100 (cru reg) + 0x1c0 
> (pin-controller reg). If so, here are devmem's output of said region 
> (read value is after the "=")

There is a translation problem here, we are off by 0x100 (need to check 
the bcm4708 data sheet though quite positive it applies there, too), the 
following would be more correct:

diff --git a/arch/arm/boot/dts/bcm5301x.dtsi 
b/arch/arm/boot/dts/bcm5301x.dtsi
index 2d9b4dd05830..d73e5151ce51 100644
--- a/arch/arm/boot/dts/bcm5301x.dtsi
+++ b/arch/arm/boot/dts/bcm5301x.dtsi
@@ -407,9 +407,9 @@
                         #address-cells = <1>;
                         #size-cells = <1>;

-                       pin-controller@1c0 {
-                               compatible = "brcm,bcm4708-pinmux";
-                               reg = <0x1c0 0x24>;
+                       pin-controller@c0 {
+                               compatible = "brcm,bcm53012-pinmux";
+                               reg = <0xc0 0x24>;
                                 reg-names = "cru_gpio_control";

                                 spi-pins {

> 
> devmem 0x1800c2c0 = 0x0
> devmem 0x1800c2c4 = 0x001D2003
> devmem 0x1800c2c8 = 0x00000284
> devmem 0x1800c2cc = 0x00000284
> devmem 0x1800c2d0 = 0x00000285
> devmem 0x1800c2d4 = 0x00000284
> devmem 0x1800c2d8 = 0x00000284
> devmem 0x1800c2dc = 0x00000284
> devmem 0x1800c2e0 = 0x00000284
> devmem 0x1800c2e4 = 0x00000284
> 
> Just in case, I've also checked 0x1800c1c0.
> This is because I stumbled over the "Example" in the binding 
> Documentation under 
> Documentation/devicetree/bindings/pinctrl/brcm,bcm4708-pinmux.txt. 
> Because this uses a "offset = <0xc0>"
> property and seems to be based of the cru@100 node?!
> 
> devmem 0x1800c1c0 = 0x00140037

So here we can see that bits 4 and 5 are set to 1, which means that they 
are still configured as GPIOs, and not as pins for the desired functions 
which explains the timeout.
Ray Jui Aug. 19, 2020, 3:24 p.m. UTC | #9
On 8/18/2020 8:34 PM, Florian Fainelli wrote:
> 
> 
> On 8/18/2020 7:46 PM, Christian Lamparter wrote:
>> On 2020-08-19 02:07, Ray Jui wrote:
>>>
>>>
>>> On 8/18/2020 3:52 PM, Christian Lamparter wrote:
>>>> Hello Florian,
>>>>
>>>> On 2020-08-18 22:11, Florian Fainelli wrote:
>>>>>
>>>>>
>>>>> On 8/18/2020 9:39 AM, Christian Lamparter wrote:
>>>>> [snip]
>>>>>> +    i2c-gpio {
>>>>>> +        compatible = "i2c-gpio";
>>>>>> +        sda-gpios = <&chipcommon 5 GPIO_ACTIVE_HIGH>;
>>>>>> +        scl-gpios = <&chipcommon 4 GPIO_ACTIVE_HIGH>;
>>>>>> +        i2c-gpio,delay-us = <10>; /* close to 100 kHz */
>>>>>> +        #address-cells = <1>;
>>>>>> +        #size-cells = <0>;
>>>>>
>>>>> Can you try using the hardware controller here instead of bit banging
>>>>> i2c over gpios? GPIOs 4 and 5 are the default pins for I2C.
>>>>
>>>> Ok. I gave this a try. What I did was:
>>>>
>>>> I removed the i2c-gpio node and went with this in the mr32.dts:
>>>>
>>>> +&i2c0 {
>>>> +       status = "okay";
>>>> +
>>>> +    clock-frequency = <100000>; /* also tried 400KHz */
>>>> +    pinctrl-names = "default";
>>>> +    pinctrl-0 = <&pinmux_i2c>; /* editted bcm5301x.dtsi for this */
>>>> +
>>>> +       cur_mon: ina2xx@45 {
>>>> +               compatible = "ti,ina219";
>>>> +               reg = <0x45>;
>>>> +               shunt-resistor = <60000>; /* = 60 mOhms */
>>>> +       };
>>>> +
>>>> +       meraki_eeprom: at24@50 {
>>>> +               compatible = "atmel,24c64";
>>>> +               reg = <0x50>;
>>>> +               pagesize = <32>;
>>>> +               read-only;
>>>> +       };
>>>> +};
>>>>
>>>> I enabled CONFIG_I2C_BCM_IPROC=y and built the whole thing and moved it
>>>> to the AP.
>>>>
>>>> During boot, I now get:
>>>>
>>>> |[    1.039174] bcm-iproc-i2c 18009000.i2c: bus set to 100000 Hz
>>>> |[    8.918419] i2c /dev entries driver
>>>> [...] (The i2c-bcm-iproc now causes a long "wait" during boot)
>>>> |[   59.385497] bcm-iproc-i2c 18009000.i2c: transaction timed out
>>>> |[   59.391272] ina2xx 0-0045: error configuring the device: -110
>>>> |[  110.585517] bcm-iproc-i2c 18009000.i2c: transaction timed out
>>>>
>>>
>>> The long wait is probably caused by waiting for the I2C transaction to
>>> time out, which it eventually did.
>>
>> Yes.
>>
>>  >
>>>
>>>> Is there a special magic needed to get this working with bcm5301x's
>>>> existing i2c0 node?
>>>>
>>>
>>> Two things to check: 1) if pinmux is configured properly; 
>> Hm, any tips for testing this? The /sys/kernel/debug/pinctrl is
>> populated correctly from what I can tell.
>>
>> What I don't know is if the DTS in bcm5301x.dtsi.
>>
>> |        dmu@1800c000 {
>> |                compatible = "simple-bus";
>> |                ranges = <0 0x1800c000 0x1000>;
>> |                #address-cells = <1>;
>> |                #size-cells = <1>;
>> |
>> |                cru@100 {
>> |                        compatible = "simple-bus";
>> |                        reg = <0x100 0x1a4>;
>> |                        ranges;
>> |                        #address-cells = <1>;
>> |                        #size-cells = <1>;
>> |
>> |                        pin-controller@1c0 {
>> |                                compatible = "brcm,bcm4708-pinmux";
>> |                                reg = <0x1c0 0x24>;
>>
>> Based on my understanding of the DT, the pinctrl register should be at
>> 0x1800c2c0 (that would be outside of the 0x1a4 size though?!). This is
>> because of 0x1800c000 (dmu base) + 0x100 (cru reg) + 0x1c0
>> (pin-controller reg). If so, here are devmem's output of said region
>> (read value is after the "=")
> 
> There is a translation problem here, we are off by 0x100 (need to check
> the bcm4708 data sheet though quite positive it applies there, too), the
> following would be more correct:
> 

It looks like someone made a mistake when creating this file (and
obviously these pinmux configurations were not tested when that someone
upstreamed this).

It looks like 0x100 is done at the CDRU node and got double added again
in the pin-controller node under the CDRU node.

> diff --git a/arch/arm/boot/dts/bcm5301x.dtsi
> b/arch/arm/boot/dts/bcm5301x.dtsi
> index 2d9b4dd05830..d73e5151ce51 100644
> --- a/arch/arm/boot/dts/bcm5301x.dtsi
> +++ b/arch/arm/boot/dts/bcm5301x.dtsi
> @@ -407,9 +407,9 @@
>                         #address-cells = <1>;
>                         #size-cells = <1>;
> 
> -                       pin-controller@1c0 {
> -                               compatible = "brcm,bcm4708-pinmux";
> -                               reg = <0x1c0 0x24>;
> +                       pin-controller@c0 {
> +                               compatible = "brcm,bcm53012-pinmux";
> +                               reg = <0xc0 0x24>;
>                                 reg-names = "cru_gpio_control";
> 
>                                 spi-pins {
> 
>>
>> devmem 0x1800c2c0 = 0x0
>> devmem 0x1800c2c4 = 0x001D2003
>> devmem 0x1800c2c8 = 0x00000284
>> devmem 0x1800c2cc = 0x00000284
>> devmem 0x1800c2d0 = 0x00000285
>> devmem 0x1800c2d4 = 0x00000284
>> devmem 0x1800c2d8 = 0x00000284
>> devmem 0x1800c2dc = 0x00000284
>> devmem 0x1800c2e0 = 0x00000284
>> devmem 0x1800c2e4 = 0x00000284
>>
>> Just in case, I've also checked 0x1800c1c0.
>> This is because I stumbled over the "Example" in the binding
>> Documentation under
>> Documentation/devicetree/bindings/pinctrl/brcm,bcm4708-pinmux.txt.
>> Because this uses a "offset = <0xc0>"
>> property and seems to be based of the cru@100 node?!
>>
>> devmem 0x1800c1c0 = 0x00140037
> 
> So here we can see that bits 4 and 5 are set to 1, which means that they
> are still configured as GPIOs, and not as pins for the desired functions
> which explains the timeout.

Yes, that makes sense. Thanks, Florian!
Christian Lamparter Aug. 19, 2020, 9:26 p.m. UTC | #10
On 2020-08-19 17:24, Ray Jui wrote:
> On 8/18/2020 8:34 PM, Florian Fainelli wrote:
>> On 8/18/2020 7:46 PM, Christian Lamparter wrote:
>>> On 2020-08-19 02:07, Ray Jui wrote:
>>>> On 8/18/2020 3:52 PM, Christian Lamparter wrote:
>>>>> Hello Florian,
>>>>>
>>>>> On 2020-08-18 22:11, Florian Fainelli wrote:
>>>>>>
>>>>>>
>>>>>> On 8/18/2020 9:39 AM, Christian Lamparter wrote:
>>>>>> [snip]
>>>>>>> +    i2c-gpio {
>>>>>>> +        compatible = "i2c-gpio";
>>>>>>> +        sda-gpios = <&chipcommon 5 GPIO_ACTIVE_HIGH>;
>>>>>>> +        scl-gpios = <&chipcommon 4 GPIO_ACTIVE_HIGH>;
>>>>>>> +        i2c-gpio,delay-us = <10>; /* close to 100 kHz */
>>>>>>> +        #address-cells = <1>;
>>>>>>> +        #size-cells = <0>;
>>>>>>
>>>>>> Can you try using the hardware controller here instead of bit banging
>>>>>> i2c over gpios? GPIOs 4 and 5 are the default pins for I2C.
>>>>>
>>>>> Ok. I gave this a try. What I did was:
>>>>>
>>>>> I removed the i2c-gpio node and went with this in the mr32.dts:
>>>>>
>>>>> +&i2c0 {
>>>>> +       status = "okay";
>>>>> +
>>>>> +    clock-frequency = <100000>; /* also tried 400KHz */
>>>>> +    pinctrl-names = "default";
>>>>> +    pinctrl-0 = <&pinmux_i2c>; /* editted bcm5301x.dtsi for this */
>>>>> +
>>>>> +       cur_mon: ina2xx@45 {
>>>>> +               compatible = "ti,ina219";
>>>>> +               reg = <0x45>;
>>>>> +               shunt-resistor = <60000>; /* = 60 mOhms */
>>>>> +       };
>>>>> +
>>>>> +       meraki_eeprom: at24@50 {
>>>>> +               compatible = "atmel,24c64";
>>>>> +               reg = <0x50>;
>>>>> +               pagesize = <32>;
>>>>> +               read-only;
>>>>> +       };
>>>>> +};
>>>>>
>>>>> I enabled CONFIG_I2C_BCM_IPROC=y and built the whole thing and moved it
>>>>> to the AP.
>>>>>
>>>>> During boot, I now get:
>>>>>
>>>>> |[    1.039174] bcm-iproc-i2c 18009000.i2c: bus set to 100000 Hz
>>>>> |[    8.918419] i2c /dev entries driver
>>>>> [...] (The i2c-bcm-iproc now causes a long "wait" during boot)
>>>>> |[   59.385497] bcm-iproc-i2c 18009000.i2c: transaction timed out
>>>>> |[   59.391272] ina2xx 0-0045: error configuring the device: -110
>>>>> |[  110.585517] bcm-iproc-i2c 18009000.i2c: transaction timed out
>>>>>
>>>>
>>>> The long wait is probably caused by waiting for the I2C transaction to
>>>> time out, which it eventually did.
>>>
>>> Yes.
>>>
>>>   >
>>>>
>>>>> Is there a special magic needed to get this working with bcm5301x's
>>>>> existing i2c0 node?
>>>>>
>>>>
>>>> Two things to check: 1) if pinmux is configured properly;
>>> Hm, any tips for testing this? The /sys/kernel/debug/pinctrl is
>>> populated correctly from what I can tell.
>>>
>>> What I don't know is if the DTS in bcm5301x.dtsi.
>>>
>>> |        dmu@1800c000 {
>>> |                compatible = "simple-bus";
>>> |                ranges = <0 0x1800c000 0x1000>;
>>> |                #address-cells = <1>;
>>> |                #size-cells = <1>;
>>> |
>>> |                cru@100 {
>>> |                        compatible = "simple-bus";
>>> |                        reg = <0x100 0x1a4>;
>>> |                        ranges;
>>> |                        #address-cells = <1>;
>>> |                        #size-cells = <1>;
>>> |
>>> |                        pin-controller@1c0 {
>>> |                                compatible = "brcm,bcm4708-pinmux";
>>> |                                reg = <0x1c0 0x24>;
>>>
>>> Based on my understanding of the DT, the pinctrl register should be at
>>> 0x1800c2c0 (that would be outside of the 0x1a4 size though?!). This is
>>> because of 0x1800c000 (dmu base) + 0x100 (cru reg) + 0x1c0
>>> (pin-controller reg). If so, here are devmem's output of said region
>>> (read value is after the "=")
>>
>> There is a translation problem here, we are off by 0x100 (need to check
>> the bcm4708 data sheet though quite positive it applies there, too), the
>> following would be more correct:
>>
> 
> It looks like someone made a mistake when creating this file (and
> obviously these pinmux configurations were not tested when that someone
> upstreamed this).
> 
> It looks like 0x100 is done at the CDRU node and got double added again
> in the pin-controller node under the CDRU node.
> 
>> diff --git a/arch/arm/boot/dts/bcm5301x.dtsi
>> b/arch/arm/boot/dts/bcm5301x.dtsi
>> index 2d9b4dd05830..d73e5151ce51 100644
>> --- a/arch/arm/boot/dts/bcm5301x.dtsi
>> +++ b/arch/arm/boot/dts/bcm5301x.dtsi
>> @@ -407,9 +407,9 @@
>>                          #address-cells = <1>;
>>                          #size-cells = <1>;
>>
>> -                       pin-controller@1c0 {
>> -                               compatible = "brcm,bcm4708-pinmux";
>> -                               reg = <0x1c0 0x24>;
>> +                       pin-controller@c0 {
>> +                               compatible = "brcm,bcm53012-pinmux";
>> +                               reg = <0xc0 0x24>;
>>                                  reg-names = "cru_gpio_control";
>>
>>                                  spi-pins {
>>
>>>
>>> devmem 0x1800c2c0 = 0x0
>>> devmem 0x1800c2c4 = 0x001D2003
>>> devmem 0x1800c2c8 = 0x00000284
>>> devmem 0x1800c2cc = 0x00000284
>>> devmem 0x1800c2d0 = 0x00000285
>>> devmem 0x1800c2d4 = 0x00000284
>>> devmem 0x1800c2d8 = 0x00000284
>>> devmem 0x1800c2dc = 0x00000284
>>> devmem 0x1800c2e0 = 0x00000284
>>> devmem 0x1800c2e4 = 0x00000284
>>>
>>> Just in case, I've also checked 0x1800c1c0.
>>> This is because I stumbled over the "Example" in the binding
>>> Documentation under
>>> Documentation/devicetree/bindings/pinctrl/brcm,bcm4708-pinmux.txt.
>>> Because this uses a "offset = <0xc0>"
>>> property and seems to be based of the cru@100 node?!
>>>
>>> devmem 0x1800c1c0 = 0x00140037
>>
>> So here we can see that bits 4 and 5 are set to 1, which means that they
>> are still configured as GPIOs, and not as pins for the desired functions
>> which explains the timeout.
> 

I've made "sure" that pinctrl-ns got the correct 0x1800c1c0 as the 
address and added a pr_info into the ns_pinctrl_set_mux() to verify
its operation:

[    1.034270] ns_pinctrl_read offset:0xc0 tmp:0x40037 unset:0x30 
write:0x40007
(This means that it read 0x40037 from 0x1800c1c0, did a & ~0x30 and
wrote 0x40007 to the address)
[    1.040199] bcm-iproc-i2c 18009000.i2c: bus set to 100000 Hz
...
[  110.598980] bcm-iproc-i2c 18009000.i2c: transaction timed out

Sadly no go. The i2c didn't budge. It still times out for both i2c chips. :(

I've separately confirmed with devmem 0x1800c1c0 in userspace that
it now reads 0x00140007 (So Bit 4 and 5) are cleared.

/proc/interrupts says there where no interrupts.
  23:          0          0     GIC-0 153 Level     18009000.i2c

thanks to devmem, I can peek at the i2c registers and used the driver
[0] to make some sense of it.

devmem 0x18009000 (CFG_OFFSET) tells me 0x400F0C00
(So the Device is enabled as CFG_EN_SHIFT is set)

devmem 0x18009030 (M_CMD_OFFSET) is 0x00000E00

devmem 0x18009038 (IE_OFFSET) is 0x0
devmem 0x1800903c (IS_OFFSET) is 0x10000000 (IS_M_START_BUSY_SHIFT)

devmem 0x18009040 (TX_OFFSET) is 0x0
devmem 0x18009044 (RX_OFFSET) is 0x0

but it doesn't look like it's working.

Would it be okay to leave the bit-banged i2c-gpio for now?

Cheers,
Christian

[0] 
<https://elixir.bootlin.com/linux/v5.9-rc1/source/drivers/i2c/busses/i2c-bcm-iproc.c>
diff mbox series

Patch

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index e6a1cac0bfc7..b0756d62772b 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -126,6 +126,7 @@  dtb-$(CONFIG_ARCH_BCM_5301X) += \
 	bcm47094-luxul-xwr-3100.dtb \
 	bcm47094-luxul-xwr-3150-v1.dtb \
 	bcm47094-netgear-r8500.dtb \
+	bcm53016-meraki-mr32.dtb \
 	bcm47094-phicomm-k3.dtb \
 	bcm94708.dtb \
 	bcm94709.dtb \
diff --git a/arch/arm/boot/dts/bcm53016-meraki-mr32.dts b/arch/arm/boot/dts/bcm53016-meraki-mr32.dts
new file mode 100644
index 000000000000..816fe8cd492d
--- /dev/null
+++ b/arch/arm/boot/dts/bcm53016-meraki-mr32.dts
@@ -0,0 +1,191 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
+/*
+ * Broadcom BCM470X / BCM5301X ARM platform code.
+ * DTS for Meraki MR32 / Codename: Espresso
+ *
+ * Copyright (C) 2018-2020 Christian Lamparter <chunkeey@gmail.com>
+ */
+
+/dts-v1/;
+
+#include "bcm4708.dtsi"
+#include "bcm5301x-nand-cs0-bch8.dtsi"
+#include <dt-bindings/leds/common.h>
+
+/ {
+	compatible = "meraki,mr32", "brcm,brcm53016", "brcm,bcm4708";
+	model = "Meraki MR32";
+
+	chosen {
+		bootargs = " console=ttyS0,115200n8 earlycon";
+	};
+
+	memory {
+		reg = <0x00000000 0x08000000>;
+	};
+
+	aliases {
+		serial1 = &uart2;
+	};
+
+	leds {
+		compatible = "gpio-leds";
+
+		sysled3 {
+			function = LED_FUNCTION_FAULT;
+			color = <LED_COLOR_ID_AMBER>;
+			gpios = <&chipcommon 18 GPIO_ACTIVE_LOW>;
+			panic-indicator;
+		};
+		sysled2 {
+			function = LED_FUNCTION_INDICATOR;
+			color = <LED_COLOR_ID_WHITE>;
+			gpios = <&chipcommon 19 GPIO_ACTIVE_HIGH>;
+		};
+	};
+
+	i2c-gpio {
+		compatible = "i2c-gpio";
+		sda-gpios = <&chipcommon 5 GPIO_ACTIVE_HIGH>;
+		scl-gpios = <&chipcommon 4 GPIO_ACTIVE_HIGH>;
+		i2c-gpio,delay-us = <10>; /* close to 100 kHz */
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		current_sense: ina219@45 {
+			compatible = "ti,ina219";
+			reg = <0x45>;
+			shunt-resistor = <60000>; /* = 60 mOhms */
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+
+		eeprom: eeprom@50 {
+			compatible = "atmel,24c64";
+			reg = <0x50>;
+			pagesize = <32>;
+			read-only;
+		};
+	};
+
+	gpio-keys {
+		compatible = "gpio-keys";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		restart {
+			label = "Reset";
+			linux,code = <KEY_RESTART>;
+			gpios = <&chipcommon 21 GPIO_ACTIVE_LOW>;
+		};
+	};
+
+	pwm-leds {
+		compatible = "pwm-leds";
+
+		red {
+			/* SYS-LED 1 - Tricolor */
+			function = LED_FUNCTION_INDICATOR;
+			color = <LED_COLOR_ID_RED>;
+			pwms = <&pwm 0 50000 0>;
+			max-brightness = <255>;
+		};
+
+		green {
+			/* SYS-LED 1 - Tricolor */
+			function = LED_FUNCTION_POWER;
+			color = <LED_COLOR_ID_GREEN>;
+			pwms = <&pwm 1 50000 0>;
+			max-brightness = <255>;
+		};
+
+		blue {
+			/* SYS-LED 1 - Tricolor */
+			function = LED_FUNCTION_INDICATOR;
+			color = <LED_COLOR_ID_BLUE>;
+			pwms = <&pwm 2 50000 0>;
+			max-brightness = <255>;
+		};
+	};
+
+};
+
+&uart0 {
+	clock-frequency = <62500000>;
+	/delete-property/ clocks;
+};
+
+&uart1 {
+	status = "disabled";
+};
+
+&uart2 {
+	status = "okay";
+	/*
+	 * bluetooth-le {
+	 *	compatible = "brcm,bcm20732";
+	 *	enable-gpios = <&chipcommon 20 GPIO_ACTIVE_HIGH>;
+	 *};
+	 */
+};
+
+&gmac1 {
+	status = "disabled";
+};
+&gmac2 {
+	status = "disabled";
+};
+&gmac3 {
+	status = "disabled";
+};
+
+&pwm {
+	status = "okay";
+};
+
+&nandcs {
+	nand-ecc-algo = "hw";
+
+	partitions {
+		/*
+		 * The partition autodetection does not work for this device.
+		 * It will only detect the "nvram" partition with an incorrect size.
+		 *	[    1.721667] 1 bcm47xxpart partitions found on MTD device brcmnand.0
+		 *	[    1.727962] Creating 1 MTD partitions on "brcmnand.0":
+		 *	[    1.733117] 0x000000400000-0x000008000000 : "nvram"
+		 */
+
+		compatible = "fixed-partitions";
+		#address-cells = <0x1>;
+		#size-cells = <0x1>;
+
+		partition0@0 {
+			label = "u-boot";
+			reg = <0x0 0x100000>;
+			read-only;
+		};
+
+		partition1@100000 {
+			label = "bootkernel1";
+			reg = <0x100000 0x300000>;
+			read-only;
+		};
+
+		partition2@400000 {
+			label = "nvram";
+			reg = <0x400000 0x100000>;
+			read-only;
+		};
+
+		partition3@500000 {
+			label = "bootkernel2";
+			reg = <0x500000 0x300000>;
+			read-only;
+		};
+
+		partition4@800000 {
+			label = "ubi";
+			reg = <0x800000 0x7780000>;
+		};
+	};
+};