linux-kernel - Re: [PATCH] mux: mmio: use reg property when parent device is not a syscon

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] [day] [month] [year] [list]

Message-ID: <68457fec-f947-553f-af51-8c15c4199145@ti.com>
Date:   Tue, 16 May 2023 15:37:42 -0500
From:   Andrew Davis <afd@...com>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        Peter Rosin <peda@...ntia.se>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Nishanth Menon <nm@...com>,
        Vignesh Raghavendra <vigneshr@...com>
CC:     <devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] mux: mmio: use reg property when parent device is not a
 syscon

On 5/16/23 1:44 PM, Krzysztof Kozlowski wrote:
> On 16/05/2023 20:37, Krzysztof Kozlowski wrote:
>> On 15/05/2023 21:19, Andrew Davis wrote:
>>> The DT binding for the reg-mux compatible states it can be used when the
>>> "parent device of mux controller is not syscon device". It also allows
>>> for a reg property. When the parent device is indeed not a syscon device,
>>> nor is it a regmap provider, we should fallback to using that reg
>>> property to identify the address space to use for this mux.
>>>
>>> Signed-off-by: Andrew Davis <afd@...com>
>>> ---
>>>   drivers/mux/mmio.c | 9 ++++++---
>>>   1 file changed, 6 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/mux/mmio.c b/drivers/mux/mmio.c
>>> index 44a7a0e885b8..42e00b9fd0a9 100644
>>> --- a/drivers/mux/mmio.c
>>> +++ b/drivers/mux/mmio.c
>>> @@ -44,10 +44,13 @@ static int mux_mmio_probe(struct platform_device *pdev)
>>>   	int ret;
>>>   	int i;
>>>   
>>> -	if (of_device_is_compatible(np, "mmio-mux"))
>>> +	if (of_device_is_compatible(np, "mmio-mux")) {
>>>   		regmap = syscon_node_to_regmap(np->parent);
>>> -	else
>>> -		regmap = dev_get_regmap(dev->parent, NULL) ?: ERR_PTR(-ENODEV);
>>> +	} else {
>>> +		regmap = dev_get_regmap(dev->parent, NULL);
>>> +		if (!regmap)
>>> +			regmap = device_node_to_regmap(np) ?: ERR_PTR(-ENODEV);
>>
>> Unless I miss something obvious, the original code is simply bogus and
>> wrong. I would like to give here Rb tag... but maybe I miss something
>> obvious. Why mux cannot be a device with MMIO itself? Binding allows it
>> which would be perfectly proper description of hardware.
> 
> OK, I see now original binding and it did not allow 'reg' property, thus
> binding was matching driver. Commit 9b358af7c818 ("dt-bindings: mux:
> Convert mux controller bindings to schema") introduced reg, which is
> reasonable.
> 
> The driver change is reasonable as well - we should not need syscon
> parent... however, I think the code should be different. If reg is
> present you should use it first. If reg is missing, use parent. This
> solves the case when node with reg will be put inside something else
> which has regmap, but should not be used for reg-mux.
> 

Makes sense to me. I didn't do that to begin with to keep the
functionality the same, but I don't see any users of this with both
"reg" and a parent with regmap today. Will flip the logic for v2.

Thanks,
Andrew

> Best regards,
> Krzysztof
>