On Mon, Aug 07, 2023 at 08:29:21AM +0200, Krzysztof Kozlowski wrote: > On 05/08/2023 05:14, Drew Fustini wrote: > > Add compatible value for the T-Head TH1520 dwcmshc controller and > > thead,io-fixed-1v8 and thead,pull-up properties. > > > > Signed-off-by: Drew Fustini <dfustini@xxxxxxxxxxxx> > > --- > > Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml | 9 +++++++++ > > 1 file changed, 9 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml > > index a43eb837f8da..57602c345cab 100644 > > --- a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml > > +++ b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml > > @@ -19,6 +19,7 @@ properties: > > - rockchip,rk3568-dwcmshc > > - rockchip,rk3588-dwcmshc > > - snps,dwcmshc-sdhci > > + - thead,th1520-dwcmshc > > > > reg: > > maxItems: 1 > > @@ -60,6 +61,14 @@ properties: > > description: Specify the number of delay for tx sampling. > > $ref: /schemas/types.yaml#/definitions/uint8 > > > > + thead,io-fixed-1v8: > > + description: SoC PHY pad is fixed 1.8V > > + type: boolean > > Isn't this duplicating existing properties for MMC modes with 1.8 V? Thank you for reviewing. Yes, now that you mention it, I do see those properties now in mmc-controller.yaml. It seems like the existing mmc-ddr-1_8v property would be appropriate. > > > + > > + thead,pull-up: > > + description: True if pull-up, false if pull-down > > This explains me nothing. No clue what you are pulling and why do you > need it. Pin pulls should be done via pin controller, not MMC. Good point that my description is not helpful. The pull-up property determines whether certain phy registers are written to. I need to try to can get documentation on the phy so that I can better understand the details of the pull-up configuration in the phy registers. > > Anyway you should have here allOf:if:then (move the allOf: from top to > behind "required:") which will disallow these properties for other variants. I noticed that nvidia,tegra20-sdhci.yaml has several lines related to pull-up/down configuration: 218 - if: 219 properties: 220 compatible: 221 contains: 222 const: nvidia,tegra210-sdhci 223 then: 224 properties: 225 pinctrl-names: 226 oneOf: 227 - items: 228 - const: sdmmc-3v3 229 description: pad configuration for 3.3 V 230 - const: sdmmc-1v8 231 description: pad configuration for 1.8 V 232 - const: sdmmc-3v3-drv 233 description: pull-up/down configuration for 3.3 V 234 - const: sdmmc-1v8-drv 235 description: pull-up/down configuration for 1.8 V 236 - items: 237 - const: sdmmc-3v3-drv 238 description: pull-up/down configuration for 3.3 V 239 - const: sdmmc-1v8-drv 240 description: pull-up/down configuration for 1.8 V 241 - items: 242 - const: sdmmc-1v8-drv 243 description: pull-up/down configuration for 1.8 V Do you think creating something like that would be a good approach? Thank you, Drew