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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 19 Aug 2020 13:49:45 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Christian Lamparter <chunkeey@...il.com>,
        linux-arm-kernel@...ts.infradead.org
Cc:     Hauke Mehrtens <hauke@...ke-m.de>,
        Rafał Miłecki <zajec5@...il.com>,
        "maintainer:BROADCOM BCM5301X ARM ARCHITECTURE" 
        <bcm-kernel-feedback-list@...adcom.com>,
        Rob Herring <robh+dt@...nel.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] ARM: dts: BCM5301X: Fix pin controller node

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