| 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: <5e6a9cc0d6c1c6928cafd83fe34c8a545198c3d9.camel@microchip.com>
Date: Thu, 10 Mar 2022 22:57:36 +0530
From: Prasanna Vengateshan <prasanna.vengateshan@...rochip.com>
To: Florian Fainelli <f.fainelli@...il.com>, <robh+dt@...nel.org>
CC: <UNGLinuxDriver@...rochip.com>, <woojung.huh@...rochip.com>,
<hkallweit1@...il.com>, <linux@...linux.org.uk>,
<davem@...emloft.net>, <kuba@...nel.org>,
<linux-kernel@...r.kernel.org>, <vivien.didelot@...il.com>,
<devicetree@...r.kernel.org>, Rob Herring <robh@...nel.org>,
<andrew@...n.ch>, <netdev@...r.kernel.org>, <olteanv@...il.com>
Subject: Re: [PATCH v8 net-next 01/10] dt-bindings: net: dsa: dt bindings
for microchip lan937x
Hi Rob/Florian,
On Wed, 2022-03-02 at 17:11 +0530, Prasanna Vengateshan wrote:
> Hi Rob and Florian,
>
> On Fri, 2022-02-11 at 19:56 -0800, Florian Fainelli wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> > content is safe
> >
> > On 2/9/2022 3:58 AM, Prasanna Vengateshan wrote:
> > > On Mon, 2022-02-07 at 18:53 -0800, Florian Fainelli wrote:
> > > > EXTERNAL EMAIL: Do not click links or open attachments unless you know
> > > > the
> > > > content is safe
> > > >
> > > > On 2/7/2022 9:21 AM, Prasanna Vengateshan wrote:
> > > > > Documentation in .yaml format and updates to the MAINTAINERS
> > > > > Also 'make dt_binding_check' is passed.
> > > > >
> > > > > RGMII internal delay values for the mac is retrieved from
> > > > > rx-internal-delay-ps & tx-internal-delay-ps as per the feedback from
> > > > > v3 patch series.
> > > > > https://lore.kernel.org/netdev/20210802121550.gqgbipqdvp5x76ii@skbuf/
> > > > >
> > > > > It supports only the delay value of 0ns and 2ns.
> > > > >
> > > > > Signed-off-by: Prasanna Vengateshan <
> > > > > prasanna.vengateshan@...rochip.com>
> > > > > Reviewed-by: Rob Herring <robh@...nel.org>
> > > > > ---
> > > > > .../bindings/net/dsa/microchip,lan937x.yaml | 179
> > > > > ++++++++++++++++++
> > > > > MAINTAINERS | 1 +
> > > > > 2 files changed, 180 insertions(+)
> > > > > create mode 100644
> > > > > Documentation/devicetree/bindings/net/dsa/microchip,lan937x.yaml
> > > > >
> > > > > + maxItems: 1
> > > > > +
> > > > > + mdio:
> > > > > + $ref: /schemas/net/mdio.yaml#
> > > > > + unevaluatedProperties: false
> > > >
> > > > This should be moved to dsa.yaml since this is about describing the
> > > > switch's internal MDIO bus controller. This is applicable to any switch,
> > > > really.
> > >
> > > Thanks for your review and feedback. Do you mean that 'mdio' to be added
> > > in
> > > dsa.yaml instead adding here?
> >
> > Yes indeed, since this is a common property of all DSA switches, it can
> > be defined or not depending on whether the switch does have an internal
> > MDIO bus controller or not.
> >
> > >
> > > >
> > > > > +
> > > > > +patternProperties:
> > > > > + "^(ethernet-)?ports$":
> > > > > + patternProperties:
> > > > > + "^(ethernet-)?port@[0-7]+$":
> > > > > + allOf:
> > > > > + - if:
> > > > > + properties:
> > > > > + phy-mode:
> > > > > + contains:
> > > > > + enum:
> > > > > + - rgmii
> > > > > + - rgmii-rxid
> > > > > + - rgmii-txid
> > > > > + - rgmii-id
> > > > > + then:
> > > > > + properties:
> > > > > + rx-internal-delay-ps:
> > > > > + $ref: "#/$defs/internal-delay-ps"
> > > > > + tx-internal-delay-ps:
> > > > > + $ref: "#/$defs/internal-delay-ps"
> > > >
> > > > Likewise, this should actually be changed in ethernet-controller.yaml
> > >
> > > There is *-internal-delay-ps property defined for mac in ethernet-
> > > controller.yaml. Should that be changed like above?
> >
> > It seems to me that these properties override whatever 'phy-mode'
> > property is defined, but in premise you are right that this is largely
> > applicable to RGMII only. I seem to recall that the QCA8K driver had
> > some sort of similar delay being applied even in SGMII mode but I am not
> > sure if we got to the bottom of this.
> >
> > Please make sure that this does not create regressions for other DTS in
> > the tree before going with that change in ethernet-controller.yaml.
> >
>
> I just tried changing rx-internal-delay-ps & tx-internal-delay-ps on
> conditional
> basis like above in the ethernet-controller.yaml and it passed 'make
> dt_binding_check' as well.
>
> It would be like below if existing *-internal-delay-ps are removed from
> ethernet-controller.yaml.
>
> allOf:
> - if:
> properties:
> phy-mode:
> contains:
> enum:
> - rgmii
> - rgmii-rxid
> - rgmii-txid
> - rgmii-id
> then:
> properties:
> rx-internal-delay-ps:
> description:
> RGMII Receive Clock Delay defined in pico seconds.This is
> used for controllers that have configurable RX internal
> delays. If this property is present then the MAC applies
> the RX delay.
> tx-internal-delay-ps:
> description:
> RGMII Transmit Clock Delay defined in pico seconds.This is
> used for controllers that have configurable TX internal
> delays. If this property is present then the MAC applies
> the TX delay.
>
> After the above changes, these two properties descriptions are different
> compare
> to other properties. So i just wanted to know whether i am following the right
> approach or are there any other proposal available? Thanks.
>
> Prasanna V
>
Is the above method looking good? Your feedback will be helpful. Thanks.
Prasanna V
Powered by blists - more mailing lists