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: <d05161ed-eb30-2d4d-e9bc-4b40e8ae09e7@linaro.org>
Date:   Mon, 23 Jan 2023 20:42:54 +0100
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Piyush Malgujar <pmalgujar@...vell.com>, linux-mmc@...r.kernel.org,
        linux-kernel@...r.kernel.org, adrian.hunter@...el.com,
        ulf.hansson@...aro.org, robh+dt@...nel.org,
        krzysztof.kozlowski+dt@...aro.org, yamada.masahiro@...ionext.com,
        devicetree@...r.kernel.org
Cc:     jannadurai@...vell.com, cchavva@...vell.com
Subject: Re: [PATCH v2 4/5] dt-bindings: mmc: sdhci-cadence: SD6 support

On 23/01/2023 20:27, Piyush Malgujar wrote:
> From: Jayanthi Annadurai <jannadurai@...vell.com>
> 
> Add support for SD6 controller support.

This is a friendly reminder during the review process.

It seems my previous comments were not fully addressed. Maybe my
feedback got lost between the quotes, maybe you just forgot to apply it.
Please go back to the previous discussion and either implement all
requested changes or keep discussing them.

Thank you.

> 
> Signed-off-by: Jayanthi Annadurai <jannadurai@...vell.com>
> Signed-off-by: Piyush Malgujar <pmalgujar@...vell.com>
> ---
>  .../devicetree/bindings/mmc/cdns,sdhci.yaml   | 34 +++++++++++++++++--
>  1 file changed, 32 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml b/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
> index 8b1a0fdcb5e3e2e8b87d8d7678e37f3dad447fc1..26ef2804aa9e17c583adaa906338ec7af8c4990b 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@...ionext.com>
> @@ -18,7 +18,9 @@ properties:
>        - enum:
>            - microchip,mpfs-sd4hc
>            - socionext,uniphier-sd4hc
> -      - const: cdns,sd4hc
> +      - enum:
> +          - cdns,sd4hc
> +          - cdns,sd6hc
>  
>    reg:
>      maxItems: 1
> @@ -111,6 +113,34 @@ properties:
>      minimum: 0
>      maximum: 0x7f
>  
> +  cdns,iocell-input-delay:
> +    description: Delay in ps across the input IO cells

Use proper unit suffix -ps, so ref wont' be needed.

This comment was also ignored.

> +    $ref: "/schemas/types.yaml#/definitions/uint32"
> +
> +  cdns,iocell-output-delay:
> +    description: Delay in ps across the output IO cells

Ditto

> +    $ref: "/schemas/types.yaml#/definitions/uint32"
> +
> +  cdns,delay-element:
> +    description: Delay element in ps used for calculating phy timings

Ditto

> +    $ref: "/schemas/types.yaml#/definitions/uint32"
> +
> +  cdns,read-dqs-cmd-delay:
> +    description: Command delay used in HS200 tuning

What are the units?

> +    $ref: "/schemas/types.yaml#/definitions/uint32"

Drop quotes (everywhere)

> +
> +  cdns,tune-val-start:
> +    description: Staring value of data delay used in HS200 tuning

Same problem - missing units.

> +    $ref: "/schemas/types.yaml#/definitions/uint32"
> +


I don't get why the feedback has to be repeated. It's a bit a waste of
time, isn't it?

Best regards,
Krzysztof

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ