Hi Krzysztof, On Sat, Jan 07, 2023 at 02:25:02PM +0100, Krzysztof Kozlowski wrote: > 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? > > We have different values based on emmc devices populated on different boards and these tuning parameters are used to program phy registers accordingly. > Best regards, > Krzysztof Thanks, Piyush>
