| 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
| ||
|
Message-ID: <20250517003525.2f6a5005@kmaincent-XPS-13-7390>
Date: Sat, 17 May 2025 00:35:25 +0200
From: Kory Maincent <kory.maincent@...tlin.com>
To: Piotr Kubik <piotr.kubik@...ran.com>
Cc: Krzysztof Kozlowski <krzk@...nel.org>, Oleksij Rempel
<o.rempel@...gutronix.de>, 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: [EXTERNAL]Re: [PATCH net-next 1/2] dt-bindings: net: pse-pd:
Add bindings for Si3474 PSE controller
On Thu, 15 May 2025 15:20:40 +0000
Piotr Kubik <piotr.kubik@...ran.com> 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
> - description of i2c_new_ancillary_device() calls this device explicitly
> slave multiple times
It is better to avoid the usage of such word in new code. Secondary suits well
for replacement.
> >> +
> >> + 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.
But here you should describe the channels of the controller and the channel has
no link to the relationship between logical and physical power channels. This
relationship rather is described in the "pairsets" parameter of PSE PI.
Maybe something like that:
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 defines the 8 physical delivery channels on the controller that
can be referenced by PSE PIs through their "pairsets" property. The actual port
matrix mapping is created when PSE PIs reference these channels in their
pairsets. For 4-pair operation, two channels from the same group (0-3 or 4-7)
must be referenced by a single PSE PI.
Similarly the description I used on the tps23881 is also not correct. I have to
change it.
I didn't look into the datasheet, could we have parameters specific to a
quad? If that the case we maybe should have something like that:
quad0: quad@0 {
reg = <0>;
#address-cells = <1>;
#size-cells = <0>;
phys0: port@0 {
reg = <0>;
};
phys1: port@1 {
reg = <1>;
};
phys2: port@2 {
reg = <2>;
};
phys3: port@3 {
reg = <3>;
};
};
quad@1 {
reg = <1>;
#address-cells = <1>;
#size-cells = <0>;
phys4: port@0 {
reg = <0>;
};
phys5: port@1 {
reg = <1>;
};
phys6: port@2 {
reg = <2>;
};
phys7: port@3 {
reg = <3>;
};
};
};
Regards,
--
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
Powered by blists - more mailing lists