LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Rob Herring <robh@kernel.org>
To: Dhananjay Kangude <dkangude@cadence.com>
Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	mparab@cadence.com, vigneshr@ti.com, kishon@ti.com
Subject: Re: [PATCH v2] dt-bindings: ufshc: cdns: convert bindings for Cadence UFS host controller
Date: Tue, 7 Sep 2021 13:54:43 -0500	[thread overview]
Message-ID: <YTe1c7sxlL9HGxv7@robh.at.kernel.org> (raw)
In-Reply-To: <20210902063754.9509-2-dkangude@cadence.com>

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.

> 
> Signed-off-by: Dhananjay Kangude <dkangude@cadence.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@fd030000 {
> -		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: http://devicetree.org/schemas/ufs/cdns,ufshc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Cadence Universal Flash Storage (UFS) Controller
> +
> +maintainers:
> +  - Dhananjay Kangude <dkangude@cadence.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'.

> +
> +  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'?

ref_clk was not documented. Okay to add, but mention why in the commit 
msg.

> +
> +  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.

> +    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.

> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    ufs: ufs@fd030000 {
> +         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
> 
> 

  reply	other threads:[~2021-09-07 18:54 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-26 18:51 [PATCH v1] dt-bindings " Dhananjay Kangude
2021-08-26 18:51 ` [PATCH v1] dt-bindings: ufshc: cdns: convert bindings " Dhananjay Kangude
2021-08-27 15:06   ` Rob Herring
2021-09-02  6:37   ` [PATCH v2] dt-bindings " Dhananjay Kangude
2021-09-02  6:37     ` [PATCH v2] dt-bindings: ufshc: cdns: convert bindings " Dhananjay Kangude
2021-09-07 18:54       ` Rob Herring [this message]
2021-09-09 12:08         ` Dhananjay Vilasrao Kangude
2021-10-11  8:36           ` Dhananjay Vilasrao Kangude

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YTe1c7sxlL9HGxv7@robh.at.kernel.org \
    --to=robh@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dkangude@cadence.com \
    --cc=kishon@ti.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mparab@cadence.com \
    --cc=vigneshr@ti.com \
    --subject='Re: [PATCH v2] dt-bindings: ufshc: cdns: convert bindings for Cadence UFS host controller' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).