Date:   Tue, 28 Feb 2023 09:46:48 -0600
From:   Rob Herring <robh@kernel.org>
To:     Piyush Malgujar <pmalgujar@marvell.com>
Cc:     linux-mmc@vger.kernel.org, linux-kernel@vger.kernel.org,
        adrian.hunter@intel.com, ulf.hansson@linaro.org,
        krzysztof.kozlowski+dt@linaro.org, yamada.masahiro@socionext.com,
        devicetree@vger.kernel.org, jannadurai@marvell.com,
        cchavva@marvell.com
Subject: Re: [PATCH v3 5/6] dt-bindings: mmc: sdhci-cadence: SD6 support
Message-ID: <20230228154648.GA3298015-robh@kernel.org>
References: <20230227183151.27912-1-pmalgujar@marvell.com>
 <20230227183151.27912-6-pmalgujar@marvell.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230227183151.27912-6-pmalgujar@marvell.com>
Precedence: bulk

On Mon, Feb 27, 2023 at 10:31:50AM -0800, Piyush Malgujar wrote:
> From: Jayanthi Annadurai <jannadurai@marvell.com>
> 
> Add support for SD6 controller support.

On what h/w?

> 
> Signed-off-by: Jayanthi Annadurai <jannadurai@marvell.com>
> Signed-off-by: Piyush Malgujar <pmalgujar@marvell.com>
> ---
>  .../devicetree/bindings/mmc/cdns,sdhci.yaml   | 24 +++++++++++++++++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml b/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
> index 8b1a0fdcb5e3e2e8b87d8d7678e37f3dad447fc1..0dba17c4f17f82c8ae68e46225ed72418e8361ff 100644
> --- a/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
> +++ b/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
> @@ -4,7 +4,7 @@
>  $id: http://devicetree.org/schemas/mmc/cdns,sdhci.yaml#
>  $schema: http://devicetree.org/meta-schemas/core.yaml#
>  
> -title: Cadence SD/SDIO/eMMC Host Controller (SD4HC)
> +title: Cadence SD/SDIO/eMMC Host Controller (SD4HC, SD6HC)
>  
>  maintainers:
>    - Masahiro Yamada <yamada.masahiro@socionext.com>
> @@ -18,7 +18,9 @@ properties:
>        - enum:
>            - microchip,mpfs-sd4hc
>            - socionext,uniphier-sd4hc
> -      - const: cdns,sd4hc
> +      - enum:
> +          - cdns,sd4hc
> +          - cdns,sd6hc

Other than FPGA implementations IP vendor compatible strings are pretty 
much useless. Define a compatible for your h/w.

>  
>    reg:
>      maxItems: 1
> @@ -111,6 +113,24 @@ properties:
>      minimum: 0
>      maximum: 0x7f
>  
> +  cdns,iocell-input-delay-ps:
> +    description: Delay in ps across the input IO cells
> +
> +  cdns,iocell-output-delay-ps:
> +    description: Delay in ps across the output IO cells
> +
> +  cdns,delay-element-ps:
> +    description: Delay element in ps used for calculating phy timings
> +
> +  cdns,read-dqs-cmd-delay-ps:
> +    description: Command delay used in HS200 tuning
> +
> +  cdns,tune-val-start-ps:
> +    description: Staring value of data delay used in HS200 tuning
> +
> +  cdns,tune-val-step-ps:
> +    description: Incremental value of data delay used in HS200 tuning

Wouldn't any controller implementation need these possibly? IIRC, we 
have some common properties for this. If not, survey what we do have and 
come up with something common. Or you can imply all this from the h/w 
specific compatible you are going to add.

Rob