linux-kernel - Re: [PATCH 09/13] dt-bindings: phy: Add documentation for Airoha AN7581 USB PHY

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: <67d95979.050a0220.166852.25e6@mx.google.com>
Date: Tue, 18 Mar 2025 12:31:00 +0100
From: Christian Marangi <ansuelsmth@...il.com>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: Michael Turquette <mturquette@...libre.com>,
	Stephen Boyd <sboyd@...nel.org>, Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>, Lee Jones <lee@...nel.org>,
	Vinod Koul <vkoul@...nel.org>,
	Kishon Vijay Abraham I <kishon@...nel.org>,
	Matthias Brugger <matthias.bgg@...il.com>,
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Lorenzo Bianconi <lorenzo@...nel.org>,
	Daniel Danzberger <dd@...edd.com>, Arnd Bergmann <arnd@...db.de>,
	Linus Walleij <linus.walleij@...aro.org>,
	Nikita Shubin <nikita.shubin@...uefel.me>,
	Guo Ren <guoren@...nel.org>, Yangyu Chen <cyy@...self.name>,
	Ben Hutchings <ben@...adent.org.uk>, Felix Fietkau <nbd@....name>,
	linux-clk@...r.kernel.org, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	linux-phy@...ts.infradead.org, linux-mediatek@...ts.infradead.org,
	linux-usb@...r.kernel.org, upstream@...oha.com
Subject: Re: [PATCH 09/13] dt-bindings: phy: Add documentation for Airoha
 AN7581 USB PHY

On Sun, Mar 16, 2025 at 06:01:02PM +0100, Krzysztof Kozlowski wrote:
> On 11/03/2025 19:51, Christian Marangi wrote:
> >>
> >>> +    $ref: /schemas/types.yaml#/definitions/uint32
> >>> +    enum: [0, 1]
> >>
> >> I don't understand why do you need index property here (which are
> >> usually not allowed).
> >>
> > 
> > Eh... As said in the description this is really to differentiate the 2
> > different physical port...
> > 
> > Each port have a dedicated oscillator for calibration and these
> > calibration are identified by an offset (all placed one after another in
> > a separate register space).
> 
> So different oscillators? Then describe the oscillator and its differences.
> 
> Different programing model? Different compatible.
> 
> Other difference? Depending what is the difference.
> 
> But there is no such thing as "different port ID" based on your
> description above. You just claimed that they are different, but you do
> not put that difference to hardware description. Instead you encode that
> difference in the drivers and it should be opposite. The DTS, so the
> hardware description, should tell you the difference. And I am sorry,
> but in 99% of cases "I am the first phy" and "I am the second" is not
> the actual difference we are interested in.
>

Ok to make it as clear as possible. (hope I don't contraddict with the
previous statement) (actually yes the "separate register space statement
was wrong and sorry for the confusion")

- 2 USB port
- USB 2.0 needs to be calibrated with an oscillator
- Each USB port have his own dedicated oscillator somewhere in the HW
- Each USB port have at the same offset a register to SELECT the
  oscillator. This register refer to the same oscillator selection in
  the HW.
  
  Example:
  reg 0x1fac0100 MASK 27:26 can be set to source out of oscillator 0,1,2
  reg 0x1fae0100 MASK 27:26 can be set to source out of oscillator 0,1,2

  Both register refer to the same oscillators in hardware (hence each
  port should select the correct one)
  Selecting oscillator 0 for both USB port is problematic.

With this in mind is it ok if I describe this with something like

airoha,usb2-monitor-clk-sel = <AIROHA_USB2_MONCLK_SEL0>;

and some dt-bindings include

#define AIROHA_USB2_MONCLK_SEL0 0
#define AIROHA_USB2_MONCLK_SEL1 1
#define AIROHA_USB2_MONCLK_SEL2 2
#define AIROHA_USB2_MONCLK_SEL3 3

Or an enum of string like "osc0", "osc1", "osc2"...?

> > 
> > Oscillator 0 for physical port 0
> > Oscillator 1 for physcial port 1
> > 
> > And model this is a bit problematic without an additional property, any
> > hint for this?
> > 
> Best regards,
> Krzysztof

-- 
	Ansuel