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

[Ray Jui]

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 at 45 {
> +               compatible = "ti,ina219";
> +               reg = <0x45>;
> +               shunt-resistor = <60000>; /* = 60 mOhms */
> +       };
> +
> +       meraki_eeprom: at24 at 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";
>                  };
>