Re: [PATCH] ARM: dts: BCM5301X: Fix pin controller node

[Florian Fainelli <f.fainelli@gmail.com>]

On 8/19/20 1:48 PM, Christian Lamparter wrote:
> On 2020-08-19 06:23, Florian Fainelli wrote:
>> The pin controller resources start at 0xc0 from the CRU base which is at
>> 0x100 from th DMU base, for a final address of 0x1800_c1c0, whereas we
>> are currently off by 0x100. The resource size of the CRU is also
>> incorrect and should end at 0x248 bytes from 0x100 which is the start
>> address. Finally, the compatibility strings defined for the
>> pin-controller node should reflect the SoC being used.
>>
>> Fixes: 9994241ac97c ("ARM: dts: BCM5301X: Describe Northstar pins mux
>> controller")
>> Reported-by: Christian Lamparter <chunkeey@gmail.com>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>> Christian, can you test this as a preliminary patch for your Cisco
>> Meraki MR32 series? Thanks!
> 
> Hm, it looks like this is more complicated than this. We should have
> looked at pinctrl-ns.c's ns_pinctrl_probe() [0] before calling it.
> 
> |    ns_pinctrl->regmap = syscon_node_to_regmap(of_get_parent(np));
> |    if (IS_ERR(ns_pinctrl->regmap)) {
> |        int err = PTR_ERR(ns_pinctrl->regmap);
> |
> |        dev_err(dev, "Failed to map pinctrl regs: %d\n", err);
> |
> |        return err;
> |    }
> |
> |    if (of_property_read_u32(np, "offset", &ns_pinctrl->offset)) {
> |        dev_err(dev, "Failed to get register offset\n");
> |        return -ENOENT;
> |    }
> 
> So, the ns_pinctrl_probe() takes the address of the parent node (cru)
> and then looks for a "offset" property to add to this (which is missing
> in the bcm5301x.dtsi [1]).
> 
> Thing is, for this to work, the parent-node should be a "simple-mfd" (so
> a regmap is created for the reg), right? This would also mean that the
> "reg" property in the pin-controller node is just cosmetic.
> 
> I guess the reason why this sort-of-works for me is because I'm using
> this MR32 with OpenWrt (Rafał Miłecki is probably using it too ;) ).
> 
> (Note: We should not forget to update the binding-documentation as well!)
> 
> BTW: I'll reply my findings for the i2c issue with the MR32 in the other
> mail.

Rafal, has this driver ever worked to begin with? None of this should be
necessary, we should just be using a simple platform device resource here.
-- 
Florian

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