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@xxxxxxxxx>
>> Signed-off-by: Florian Fainelli <f.fainelli@xxxxxxxxx>
>> ---
>> 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
