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

[Florian Fainelli]

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 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.
> 
> 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 at 1800c000 {
> |                compatible = "simple-bus";
> |                ranges = <0 0x1800c000 0x1000>;
> |                #address-cells = <1>;
> |                #size-cells = <1>;
> |
> |                cru at 100 {
> |                        compatible = "simple-bus";
> |                        reg = <0x100 0x1a4>;
> |                        ranges;
> |                        #address-cells = <1>;
> |                        #size-cells = <1>;
> |
> |                        pin-controller at 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 at 1c0 {
-                               compatible = "brcm,bcm4708-pinmux";
-                               reg = <0x1c0 0x24>;
+                       pin-controller at 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 at 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.
-- 
Florian