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

Date:   Tue, 16 May 2023 20:44:56 +0200
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Andrew Davis <afd@...com>, 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 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.

Best regards,
Krzysztof