linux-kernel - Re: [PATCH v2 2/7] dt-bindings: clock: renesas,r9a06g032-sysctrl: Add h2mode property

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: <20221115150417.513955a7@bootlin.com>
Date:   Tue, 15 Nov 2022 15:04:17 +0100
From:   Herve Codina <herve.codina@...tlin.com>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Cc:     Geert Uytterhoeven <geert+renesas@...der.be>,
        Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Magnus Damm <magnus.damm@...il.com>,
        Gareth Williams <gareth.williams.jx@...esas.com>,
        linux-renesas-soc@...r.kernel.org, linux-clk@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-usb@...r.kernel.org,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
        Miquel Raynal <miquel.raynal@...tlin.com>
Subject: Re: [PATCH v2 2/7] dt-bindings: clock: renesas,r9a06g032-sysctrl:
 Add h2mode property

Hi Krzysztof,

On Tue, 15 Nov 2022 14:07:52 +0100
Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org> wrote:

> On 15/11/2022 14:05, Krzysztof Kozlowski wrote:
> > On 14/11/2022 12:15, Herve Codina wrote:  
> >> Add the h2mode property to force the USBs mode ie:
> >>  - 2 hosts
> >> or
> >>  - 1 host and 1 device
> >>
> >> Signed-off-by: Herve Codina <herve.codina@...tlin.com>
> >> ---
> >>  .../bindings/clock/renesas,r9a06g032-sysctrl.yaml      | 10 ++++++++++
> >>  1 file changed, 10 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/clock/renesas,r9a06g032-sysctrl.yaml b/Documentation/devicetree/bindings/clock/renesas,r9a06g032-sysctrl.yaml
> >> index 95bf485c6cec..f9e0a58aa4fb 100644
> >> --- a/Documentation/devicetree/bindings/clock/renesas,r9a06g032-sysctrl.yaml
> >> +++ b/Documentation/devicetree/bindings/clock/renesas,r9a06g032-sysctrl.yaml
> >> @@ -39,6 +39,16 @@ properties:
> >>    '#power-domain-cells':
> >>      const: 0
> >>  
> >> +  renesas,h2mode:
> >> +    description: |
> >> +      Configure the USBs mode.
> >> +        - <0> : the USBs are in 1 host and 1 device mode.
> >> +        - <1> : the USBs are in 2 host mode.
> >> +      If the property is not present, the value used is the one already present
> >> +      in the CFG_USB register (from reset or set by the bootloader).
> >> +    $ref: /schemas/types.yaml#/definitions/uint32
> >> +    enum: [0, 1]  
> > 
> > 0/1 are quite cryptic. Why not making it a string which is easy to read
> > and understand? Can be something like "two-hosts" and "one-host". Or
> > anything you find more readable...  
> 
> ...but actually you should rather make it a property of your USB
> controller, not clock controller. You have two controllers and we have a
> generic property for them - dr_mode.
> 
> Best regards,
> Krzysztof
> 

IMHO, this property in the USB controllers does not make sense.
Indeed each controller cannot have a different 'mode'.
Some controllers are USB host only (EHCI and OHCI) and the USBF
controller I worked on is device only.
'h2mode' allows to choose between host or device on one of the USB
but not at the USB controller level.

This property should be handle outside the USB controller nodes.

Currently, this node (declared as a clock node) is in fact a sysctrl
node and can do some configuration not related to clocks.

I agree with you something related to choosing USB Host/Device in
a clock node seems strange.

Some discussion were already opened related to this property and how
to handle it:
  https://lore.kernel.org/all/20221107182642.05a09f2f@bootlin.com/
  https://lore.kernel.org/all/20221107173614.474707d7@bootlin.com/

Regards,
Hervé

-- 
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com