linux-kernel - Re: [PATCH] phy: ti: gmii-sel: Allow parent to not be syscon 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] [day] [month] [year] [list]

Message-ID: <0d28eb47-a564-2155-875b-5c8dc8aa806e@ti.com>
Date:   Mon, 10 Jul 2023 09:45:30 -0500
From:   Andrew Davis <afd@...com>
To:     Roger Quadros <rogerq@...nel.org>, Vinod Koul <vkoul@...nel.org>,
        Kishon Vijay Abraham I <kishon@...nel.org>,
        Siddharth Vadapalli <s-vadapalli@...com>
CC:     <linux-phy@...ts.infradead.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] phy: ti: gmii-sel: Allow parent to not be syscon node

On 5/16/23 1:33 PM, Roger Quadros wrote:
> Hi Andrew,
> 
> On 15/05/2023 22:59, Andrew Davis wrote:
>> If the parent node is not a syscon type, then fallback and check
>> if we can get a regmap from our own node. This no longer forces
>> us to make the parent of this node a syscon node when that might
>> not be appropriate.
> 
> Trying to understand the motive for this and if it is better to
> introduce a "syscon = <&syscon_node>" property instead which
> makes it fool proof for all cases.
> 

My motivation is to reduce our overuse of syscon nodes, IMHO syscon
is almost always a broken design in DT and goes against the standard
usage.

Some drivers like this one force us to make the parent node a syscon
device, even what that is not needed otherwise (the register space is
standalone and the standard DT "reg" property can be used to describe
the device register space).

Using "syscon = <&syscon_node>" could be a useful option for devices
when syscon is actually needed. But I think that should only be used
when the whole node itself cannot be made a child of the syscon node,
making it a child when we can is better for DT organization vs. having
a bunch of top-level nodes that point around to their register spaces
with phandles.

Andrew

>>
>> Signed-off-by: Andrew Davis <afd@...com>
>> ---
>>   drivers/phy/ti/phy-gmii-sel.c | 9 ++++++---
>>   1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/phy/ti/phy-gmii-sel.c b/drivers/phy/ti/phy-gmii-sel.c
>> index 8c667819c39a..1e67ed9a5cf6 100644
>> --- a/drivers/phy/ti/phy-gmii-sel.c
>> +++ b/drivers/phy/ti/phy-gmii-sel.c
>> @@ -435,9 +435,12 @@ static int phy_gmii_sel_probe(struct platform_device *pdev)
>>   
>>   	priv->regmap = syscon_node_to_regmap(node->parent);
>>   	if (IS_ERR(priv->regmap)) {
>> -		ret = PTR_ERR(priv->regmap);
>> -		dev_err(dev, "Failed to get syscon %d\n", ret);
>> -		return ret;
>> +		priv->regmap = device_node_to_regmap(node);
>> +		if (IS_ERR(priv->regmap)) {
>> +			ret = PTR_ERR(priv->regmap);
>> +			dev_err(dev, "Failed to get syscon %d\n", ret);
>> +			return ret;
>> +		}
>>   	}
>>   
>>   	ret = phy_gmii_sel_init_ports(priv);
>