linux-kernel - Re: [PATCH net-next 1/2] dt-bindings: net: pse-pd: Add bindings for Si3474 PSE controller

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]

Message-ID: <0b7afab0-0283-4a52-82bc-0d492f752034@adtran.com>
Date: Fri, 16 May 2025 14:09:25 +0000
From: Piotr Kubik <piotr.kubik@...ran.com>
To: Krzysztof Kozlowski <krzk@...nel.org>, Oleksij Rempel
	<o.rempel@...gutronix.de>, Kory Maincent <kory.maincent@...tlin.com>, Andrew
 Lunn <andrew+netdev@...n.ch>, "David S. Miller" <davem@...emloft.net>, Eric
 Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
	<pabeni@...hat.com>, Rob Herring <robh@...nel.org>, Krzysztof Kozlowski
	<krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net-next 1/2] dt-bindings: net: pse-pd: Add bindings for
 Si3474 PSE controller

On 5/16/25 15:37, Krzysztof Kozlowski wrote:
> On 15/05/2025 17:20, Piotr Kubik wrote:
>> On 5/13/25 10:24, Krzysztof Kozlowski wrote:
>>> On 13/05/2025 00:05, Piotr Kubik wrote:
>>>> +
>>>> +maintainers:
>>>> +  - Piotr Kubik <piotr.kubik@...ran.com>
>>>> +
>>>> +allOf:
>>>> +  - $ref: pse-controller.yaml#
>>>> +
>>>> +properties:
>>>> +  compatible:
>>>> +    enum:
>>>> +      - skyworks,si3474
>>>> +
>>>> +  reg-names:
>>>> +    items:
>>>> +      - const: main
>>>> +      - const: slave
>>>
>>> s/slave/secondary/ (or whatever is there in recommended names in coding
>>> style)
>>>
>>
>> Well I was thinking about it and decided to use 'slave' for at least two reasons:
>> - si3474 datasheet calls the second part of IC (we configure it here) this way
> 
> 
> This could be a reason, but specs are changing over time (see I2C, I3C)
> to include different namings. If this annoys certain government sending
> their executive directives, then even better.
> 

OK, I've read some discussions regarding this issue and decided to rename here and in si3474 code.

> 
>> - description of i2c_new_ancillary_device() calls this device explicitly slave multiple times
> 
> Old driver code should not be an argument. If code changes, which it can
> anytime, are you going to change binding? No, because such change in the
> binding would not be allowed.
> 



>>
>>>> +
>>>> +  reg:
>>>
>>> First reg, then reg-names. Please follow other bindings/examples.
>>>
>>>> +    maxItems: 2
>>>> +
>>>> +  channels:
>>>> +    description: The Si3474 is a single-chip PoE PSE controller managing
>>>> +      8 physical power delivery channels. Internally, it's structured
>>>> +      into two logical "Quads".
>>>> +      Quad 0 Manages physical channels ('ports' in datasheet) 0, 1, 2, 3
>>>> +      Quad 1 Manages physical channels ('ports' in datasheet) 4, 5, 6, 7.
>>>> +      This parameter describes the relationship between the logical and
>>>> +      the physical power channels.
>>>
>>> How exactly this maps here logical and physical channels? You just
>>> listed channels one after another...
>>
>> yes, here in this example it is 1 to 1 simple mapping, but in a real world,
>> depending on hw connections, there is a possibility that 
>> e.g. "pse_pi0" will use "<&phys0_4>, <&phys0_5>" pairset for lan port 3.
>>
> 
> Ack, I see that's actually common for pse-pd. It's fine.
> 
Actually, after some consideration I agreed with you and decided to remove this part 
in v3 as this 'channels' node does not really  describe HW mapping, the pse-pis part does.
v3: https://lore.kernel.org/netdev/ebe9a9f5-db9c-4b82-a5e9-3b108a0c6338@adtran.com/

> 
> Best regards,
> Krzysztof

Thanks
/Piotr