| 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: <MN2PR07MB7006B3FBB7FFEF18EF60EDC5CDB59@MN2PR07MB7006.namprd07.prod.outlook.com>
Date: Mon, 11 Oct 2021 08:36:08 +0000
From: Dhananjay Vilasrao Kangude <dkangude@...ence.com>
To: Rob Herring <robh@...nel.org>
CC: "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Milind Parab <mparab@...ence.com>,
"vigneshr@...com" <vigneshr@...com>,
"kishon@...com" <kishon@...com>
Subject: RE: [PATCH v2] dt-bindings: ufshc: cdns: convert bindings for Cadence
UFS host controller
Hi Rob,
This is with respect to your comment on the right type for property freq-table-hz.
I have tried different combinations but it seems not compatible with property.
The param freq-table-hz have multiple pair of frequencies in <Min Max> format which
needed to decide operating frequencies in the same order of clocks property.
I would appreciate if you provide some suggestion for correct type.
Thanks
Dhananjay
> -----Original Message-----
> From: Dhananjay Vilasrao Kangude
> Sent: Thursday, September 9, 2021 5:38 PM
> To: Rob Herring <robh@...nel.org>
> Cc: devicetree@...r.kernel.org; linux-kernel@...r.kernel.org; Milind Parab
> <mparab@...ence.com>; vigneshr@...com; kishon@...com
> Subject: RE: [PATCH v2] dt-bindings: ufshc: cdns: convert bindings for
> Cadence UFS host controller
>
>
> > On Thu, Sep 02, 2021 at 08:37:54AM +0200, Dhananjay Kangude wrote:
> > > 1.Converted bindings into yaml format for Cadence UFS host
> > > controller 2.Modified reference to cdns,ufshc.txt tp cdns,ufshc.yaml
> > > 3.Removed power,domain property from ti,j721e-ufs.yaml as it is not
> > > required
> >
> > Maybe not required, but should be allowed which is still not the case.
>
> I got it. I will add the power,domain property as an optional in ufs and retain
> ti,j721-ufs.yaml in original form.
> > >
> > > Signed-off-by: Dhananjay Kangude <dkangude@...ence.com>
> > > ---
> > > .../devicetree/bindings/ufs/cdns,ufshc.txt | 32 --------
> > > .../devicetree/bindings/ufs/cdns,ufshc.yaml | 80
> > ++++++++++++++++++++
> > > .../devicetree/bindings/ufs/ti,j721e-ufs.yaml | 3 +-
> > > 3 files changed, 81 insertions(+), 34 deletions(-) delete mode
> > > 100644 Documentation/devicetree/bindings/ufs/cdns,ufshc.txt
> > > create mode 100644
> > > Documentation/devicetree/bindings/ufs/cdns,ufshc.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/ufs/cdns,ufshc.txt
> > > b/Documentation/devicetree/bindings/ufs/cdns,ufshc.txt
> > > deleted file mode 100644
> > > index 02347b0..0000000
> > > --- a/Documentation/devicetree/bindings/ufs/cdns,ufshc.txt
> > > +++ /dev/null
> > > @@ -1,32 +0,0 @@
> > > -* Cadence Universal Flash Storage (UFS) Controller
> > > -
> > > -UFS nodes are defined to describe on-chip UFS host controllers.
> > > -Each UFS controller instance should have its own node.
> > > -Please see the ufshcd-pltfrm.txt for a list of all available properties.
> > > -
> > > -Required properties:
> > > -- compatible : Compatible list, contains one of the following
> controllers:
> > > - "cdns,ufshc" - Generic CDNS HCI,
> > > - "cdns,ufshc-m31-16nm" - CDNS UFS HC + M31 16nm
> > PHY
> > > - complemented with the JEDEC version:
> > > - "jedec,ufs-2.0"
> > > -
> > > -- reg : Address and length of the UFS register set.
> > > -- interrupts : One interrupt mapping.
> > > -- freq-table-hz : Clock frequency table.
> > > - See the ufshcd-pltfrm.txt for details.
> > > -- clocks : List of phandle and clock specifier pairs.
> > > -- clock-names : List of clock input name strings sorted in the same
> > > - order as the clocks property. "core_clk" is mandatory.
> > > - Depending on a type of a PHY,
> > > - the "phy_clk" clock can also be added, if needed.
> > > -
> > > -Example:
> > > - ufs@...30000 {
> > > - compatible = "cdns,ufshc", "jedec,ufs-2.0";
> > > - reg = <0xfd030000 0x10000>;
> > > - interrupts = <0 1 IRQ_TYPE_LEVEL_HIGH>;
> > > - freq-table-hz = <0 0>, <0 0>;
> > > - clocks = <&ufs_core_clk>, <&ufs_phy_clk>;
> > > - clock-names = "core_clk", "phy_clk";
> > > - };
> > > diff --git a/Documentation/devicetree/bindings/ufs/cdns,ufshc.yaml
> > > b/Documentation/devicetree/bindings/ufs/cdns,ufshc.yaml
> > > new file mode 100644
> > > index 0000000..4509ae0
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/ufs/cdns,ufshc.yaml
> > > @@ -0,0 +1,80 @@
> > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2
> > > +---
> > > +$id:
> > > +https://urldefense.com/v3/__http://devicetree.org/schemas/ufs/cdns,
> > > +uf
> > >
> >
> +shc.yaml*__;Iw!!EHscmS1ygiU1lA!VP17yEajmecovQs1IfNiRJklavtRgx16eZXg0
> > b
> > > +G5re3EAQrSfmirkkVpagqs5mE$
> > > +$schema:
> > > +https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core
> > > +.y
> > >
> >
> +aml*__;Iw!!EHscmS1ygiU1lA!VP17yEajmecovQs1IfNiRJklavtRgx16eZXg0bG5r
> > e3
> > > +EAQrSfmirkkVpEHrPgh4$
> > > +
> > > +title: Cadence Universal Flash Storage (UFS) Controller
> > > +
> > > +maintainers:
> > > + - Dhananjay Kangude <dkangude@...ence.com>
> > > +
> > > +description:
> > > + UFS nodes are defined to describe on-chip Cadence UFS host
> controllers.
> > > + Each UFS controller instance should have its own node.
> > > +
> > > +properties:
> > > + compatible:
> > > + oneOf:
> > > + - items:
> > > + - enum:
> > > + - cdns,ufshc
> > > + - cdns,ufshc-m31-16nm
> > > + - const: jedec,ufs-2.0
> > > + - items:
> > > + - const: jedec,ufs-2.0
> > > +
> > > + reg:
> > > + items:
> > > + - description: UFS controller register set
> >
> > Not a useful description. Just 'maxItems: 1'.
>
> I agree. I will change the property details.
> >
> > > +
> > > + interrupts:
> > > + maxItems: 1
> > > +
> > > + clocks:
> > > + minItems: 1
> > > + items:
> > > + - description: Description of core_clk
> > > + - description: Description of phy_clk
> > > + - description: Description of ref_clk
> >
> > 'Description of core_clk'?
> OK. I will change it.
> > ref_clk was not documented. Okay to add, but mention why in the commit
> > msg.
> The ref_clk is used by ufs spec to decide ufs device reference clock and it is
> part of ufs framework.
> Usually SoC vendors fix this value by using this property used by core
> framework.
> The details are already mentioned in
> Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> > > +
> > > + clock-names:
> > > + minItems: 1
> > > + items:
> > > + - const: core_clk
> > > + - const: phy_clk
> > > + - const: ref_clk
> > > +
> > > + freq-table-hz:
> > > + $ref: /schemas/types.yaml#/definitions/uint64-matrix
> >
> > Not the right type.
>
> I have tried with different ref as single array but it seems not compatible.
> The freq-table-hz property need the values in 2D array format.
> for example:
> freq-table-hz = <250000000 250000000>, <19200000 19200000>, <19200000
> 19200000>;
>
> Could you suggest what type suits the requirement. Thanks.
> >
> > > + description:
> > > + Clock frequency table.
> > > + See the ufshcd-pltfrm.txt for details.
> > > +
> > > + dma-coherent: true
> > > +
> > > +required:
> > > + - compatible
> > > + - reg
> > > + - interrupts
> > > + - freq-table-hz
> > > + - clocks
> > > + - clock-names
> > > +
> > > +additionalProperties: false
> > > +unevaluatedProperties: false
> >
> > Don't need both. Just additionalProperties.
>
> I got it. I will change in next version.
> >
> > > +
> > > +examples:
> > > + - |
> > > + #include <dt-bindings/interrupt-controller/irq.h>
> > > + #include <dt-bindings/interrupt-controller/arm-gic.h>
> > > +
> > > + ufs: ufs@...30000 {
> > > + compatible = "cdns,ufshc", "jedec,ufs-2.0";
> > > + reg = <0xfd030000 0x10000>;
> > > + interrupts = <0 1 IRQ_TYPE_LEVEL_HIGH>;
> > > + freq-table-hz = <0 0>;
> > > + clocks = <&ufs_core_clk>;
> > > + clock-names = "core_clk";
> > > + };
> > > +
> > > diff --git a/Documentation/devicetree/bindings/ufs/ti,j721e-ufs.yaml
> > > b/Documentation/devicetree/bindings/ufs/ti,j721e-ufs.yaml
> > > index 4d13e6b..b8f73dd 100644
> > > --- a/Documentation/devicetree/bindings/ufs/ti,j721e-ufs.yaml
> > > +++ b/Documentation/devicetree/bindings/ufs/ti,j721e-ufs.yaml
> > > @@ -50,7 +50,7 @@ patternProperties:
> > > type: object
> > > description: |
> > > Cadence UFS controller node must be the child node. Refer
> > > - Documentation/devicetree/bindings/ufs/cdns,ufshc.txt for binding
> > > + Documentation/devicetree/bindings/ufs/cdns,ufshc.yaml for
> > > + binding
> > > documentation of child node
> > >
> > > additionalProperties: false
> > > @@ -81,7 +81,6 @@ examples:
> > > reg = <0x0 0x4000 0x0 0x10000>;
> > > interrupts = <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>;
> > > freq-table-hz = <19200000 19200000>;
> > > - power-domains = <&k3_pds 277>;
> > > clocks = <&k3_clks 277 1>;
> > > assigned-clocks = <&k3_clks 277 1>;
> > > assigned-clock-parents = <&k3_clks 277 4>;
> > > --
> > > 1.7.1
> > >
> > >
Powered by blists - more mailing lists