netdev - Re: [PATCHv2 4/4] dt-bindings: net: dsa: add new MT7530 binding to support MT7621

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 for Android: free password hash cracker in your pocket

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

Message-ID: <e9439cb1-43e4-75ed-98f3-26ae3b470342@kernel.org>
Date:   Thu, 17 Jan 2019 15:17:30 +1000
From:   Greg Ungerer <gerg@...nel.org>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     sean.wang@...iatek.com, bjorn@...k.no,
        vivien.didelot@...oirfairelinux.com, f.fainelli@...il.com,
        netdev@...r.kernel.org, rene@...rst.com, john@...ozen.org,
        neil@...wn.name
Subject: Re: [PATCHv2 4/4] dt-bindings: net: dsa: add new MT7530 binding to
 support MT7621

Hi Andrew,

On 17/1/19 2:12 am, Andrew Lunn wrote:
> On Wed, Jan 16, 2019 at 11:14:30PM +1000, Greg Ungerer wrote:
>> Hi Andrew,
>>
>> On 15/1/19 11:18 pm, Andrew Lunn wrote:
>> [snip]
>>> As i said, it is a bit messy. I would probably have a section:
>>>
>>> Required properties
>>>
>>> which lists all common required properties. And then a section
>>>
>>> Required properties mediatek,mt7530
>>>
>>> With those which are required by that device.
>>
>>
>> Ok, I see what you mean.
>> The simplest approach is something like this:
>>
>> diff --git a/Documentation/devicetree/bindings/net/dsa/mt7530.txt b/Documentation/devicetree/bindings/net/dsa/mt7530.txt
>> index aa3527f71fdc..47aa205ee0bd 100644
>> --- a/Documentation/devicetree/bindings/net/dsa/mt7530.txt
>> +++ b/Documentation/devicetree/bindings/net/dsa/mt7530.txt
>> @@ -3,12 +3,16 @@ Mediatek MT7530 Ethernet switch
>>   Required properties:
>> -- compatible: Must be compatible = "mediatek,mt7530";
>> +- compatible: may be compatible = "mediatek,mt7530"
>> +	or compatible = "mediatek,mt7621"
>>   - #address-cells: Must be 1.
>>   - #size-cells: Must be 0.
>>   - mediatek,mcm: Boolean; if defined, indicates that either MT7530 is the part
>>   	on multi-chip module belong to MT7623A has or the remotely standalone
>>   	chip as the function MT7623N reference board provided for.
>> +
>> +If compatible mediatek,mt7530 is set then the following properties are required
>> +
>>   - core-supply: Phandle to the regulator node necessary for the core power.
>>   - io-supply: Phandle to the regulator node necessary for the I/O power.
>>   	See Documentation/devicetree/bindings/regulator/mt6323-regulator.txt
> 
> Yes, that looks good.
> 
>>
>>
>> That leaves the existing if/else with other bindings in place after this.
>>
>> Maybe I am mis-reading the mt7530.c code but I am not entirely sure that
>> "reset-gpios" is actually required for the mediatek,mcm not defined case -
>> it is optional. (And I actually don't define it for my mt7621 case where
>> mediatek,mcm is not defined).
> 
> The gpio seems to be optional in general, any only used for
> device other than mediatek,mcm.
> 
> The reset controller is required for mediatek,mcm, but not others.
> 
> So it looks like the binding document needs a few fixes :-(

Yep.. Thanks for the feedback though. I'll roll a v3.

Regards
Greg