On 06/01/2023 17:48, Piyush Malgujar wrote: > Hi Krzysztof, > > Thank you the review comments. > > On Mon, Dec 19, 2022 at 04:40:35PM +0100, Krzysztof Kozlowski wrote: >> On 19/12/2022 15:24, Piyush Malgujar wrote: >>> From: Jayanthi Annadurai <jannadurai@xxxxxxxxxxx> >>> >> >> Subject: use final prefix matching the file, so "cdns,sdhci:" >> >>> Add support for SD6 controller support >> >> Full stop. >> >>> >>> Signed-off-by: Jayanthi Annadurai <jannadurai@xxxxxxxxxxx> >>> Signed-off-by: Piyush Malgujar <pmalgujar@xxxxxxxxxxx> >>> --- >>> .../devicetree/bindings/mmc/cdns,sdhci.yaml | 33 +++++++++++++++++-- >>> 1 file changed, 31 insertions(+), 2 deletions(-) >>> >>> diff --git a/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml b/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml >>> index 8b1a0fdcb5e3e2e8b87d8d7678e37f3dad447fc1..2043e78ccd5f708a01e87fd96ec410418fcd539f 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@xxxxxxxxxxxxx> >>> @@ -19,6 +19,7 @@ properties: >>> - microchip,mpfs-sd4hc >>> - socionext,uniphier-sd4hc >>> - const: cdns,sd4hc >>> + - const: cdns,sd6hc >> >> Does not look like you tested the DTS against bindings. Please run `make >> dtbs_check` (see Documentation/devicetree/bindings/writing-schema.rst >> for instructions). >> >> ... because it does not really make sense. Why do you require SD6HC as >> fallback? I think you meant enum. >> > > Yes, that's correct. I will change it to enum. > >>> >>> reg: >>> maxItems: 1 >>> @@ -111,6 +112,34 @@ properties: >>> minimum: 0 >>> maximum: 0x7f >>> >>> + cdns,iocell_input_delay: >> >> No underscores. Use proper units in name suffix: >> https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/property-units.yaml >> >> >>> + description: Delay in ps across the input IO cells >>> + $ref: "/schemas/types.yaml#/definitions/uint32" >> >> Ditto... and so on - all of the fields. >> >>> + >>> + cdns,iocell_output_delay: >>> + description: Delay in ps across the output IO cells >>> + $ref: "/schemas/types.yaml#/definitions/uint32" >>> + >>> + cdns,delay_element: >>> + description: Delay element in ps used for calculating phy timings >>> + $ref: "/schemas/types.yaml#/definitions/uint32" >>> + >>> + cdns,read_dqs_cmd_delay: >>> + description: Command delay used in HS200 tuning >>> + $ref: "/schemas/types.yaml#/definitions/uint32" >>> + >>> + cdns,tune_val_start: >>> + description: Staring value of data delay used in HS200 tuning >>> + $ref: "/schemas/types.yaml#/definitions/uint32" >>> + >>> + cdns,tune_val_step: >>> + description: Incremental value of data delay used in HS200 tuning >>> + $ref: "/schemas/types.yaml#/definitions/uint32" >>> + >>> + cdns,max_tune_iter: >>> + description: Maximum number of iterations to complete the HS200 tuning process >>> + $ref: "/schemas/types.yaml#/definitions/uint32" >> >> Why these three are properties of DT? >> > > These tuning parameters are added here so to make them custom configurable for different > boards. I understand why do you wanted to add them, but I am asking why these are suitable for DT? DT describes hardware, so what is here specific to hardware which requires DT property? Best regards, Krzysztof
