| Message ID | 20220325000745.1708610-4-chris.packham@alliedtelesis.co.nz (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mmc: xenon: Convert to JSON schema | expand |
On 25/03/2022 01:07, Chris Packham wrote: > The armada-37xx SoC dtsi includes this as a compatible string. Add it to > the dt-binding. > > Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> > --- > > Notes: > Changes in v3: > - new > > .../devicetree/bindings/mmc/marvell,xenon-sdhci.yaml | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.yaml b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.yaml > index 326ac3fa36b5..776bed5046fa 100644 > --- a/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.yaml > +++ b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.yaml > @@ -31,6 +31,10 @@ properties: > - const: marvell,armada-ap807-sdhci > - const: marvell,armada-ap806-sdhci > > + - items: > + - const: marvell,armada-3700-sdhci > + - const: marvell,sdhci-xenon Do you know of any usages of marvell,armada-3700-sdhci alone (without sdhci-xenon)? If not, it should be removed from the enum before (the one added in your patch #2). It does not look correct to have it both as standalone compatible and one compatible with sdhci-xenon. Either it is compatible with sdhci-xenon or not. :) I suggested before to make this change here as separate patch, but I think I missed the fact that it is simple correction of armada-3700-sdhci compatible. Such simple corrections can be done within same patch as conversion, with explanation in commit msg (which was missing in your v1). To avoid unnecessary patch changes you could squash it with v1 and include this in the commit msg (actually extend it to say that you are correcting the 3700-sdhci compatible), or create patch #2 that way: + oneOf: + - enum: + - marvell,armada-cp110-sdhci + - marvell,armada-ap806-sdhci + - items: + - const: marvell,armada-ap807-sdhci + - const: marvell,armada-ap806-sdhci + - items: + - marvell,armada-3700-sdhci so now you will only add xenon here. But then it is so small patch that with explanation we usually include it in conversion. Best regards, Krzysztof
On 26/03/22 01:04, Krzysztof Kozlowski wrote: > On 25/03/2022 01:07, Chris Packham wrote: >> The armada-37xx SoC dtsi includes this as a compatible string. Add it to >> the dt-binding. >> >> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> >> --- >> >> Notes: >> Changes in v3: >> - new >> >> .../devicetree/bindings/mmc/marvell,xenon-sdhci.yaml | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.yaml b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.yaml >> index 326ac3fa36b5..776bed5046fa 100644 >> --- a/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.yaml >> +++ b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.yaml >> @@ -31,6 +31,10 @@ properties: >> - const: marvell,armada-ap807-sdhci >> - const: marvell,armada-ap806-sdhci >> >> + - items: >> + - const: marvell,armada-3700-sdhci >> + - const: marvell,sdhci-xenon > Do you know of any usages of marvell,armada-3700-sdhci alone (without > sdhci-xenon)? If not, it should be removed from the enum before (the one > added in your patch #2). It does not look correct to have it both as > standalone compatible and one compatible with sdhci-xenon. Either it is > compatible with sdhci-xenon or not. :) The in-tree uses all have both. I'll remove it from the enum. > > I suggested before to make this change here as separate patch, but I > think I missed the fact that it is simple correction of > armada-3700-sdhci compatible. Such simple corrections can be done within > same patch as conversion, with explanation in commit msg (which was > missing in your v1). > > To avoid unnecessary patch changes you could squash it with v1 and > include this in the commit msg (actually extend it to say that you are > correcting the 3700-sdhci compatible), or create patch #2 that way: > > + oneOf: > + - enum: > + - marvell,armada-cp110-sdhci > + - marvell,armada-ap806-sdhci > + - items: > + - const: marvell,armada-ap807-sdhci > + - const: marvell,armada-ap806-sdhci > + - items: > + - marvell,armada-3700-sdhci > > so now you will only add xenon here. But then it is so small patch that > with explanation we usually include it in conversion. OK will squash and add an explanation for v4. > > Best regards, > Krzysztof
diff --git a/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.yaml b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.yaml index 326ac3fa36b5..776bed5046fa 100644 --- a/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.yaml +++ b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.yaml @@ -31,6 +31,10 @@ properties: - const: marvell,armada-ap807-sdhci - const: marvell,armada-ap806-sdhci + - items: + - const: marvell,armada-3700-sdhci + - const: marvell,sdhci-xenon + reg: minItems: 1 maxItems: 2
The armada-37xx SoC dtsi includes this as a compatible string. Add it to the dt-binding. Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> --- Notes: Changes in v3: - new .../devicetree/bindings/mmc/marvell,xenon-sdhci.yaml | 4 ++++ 1 file changed, 4 insertions(+)