Re: [PATCH v3 1/2] dt-bindings: mmc: xenon: add AC5 compatible string

From: Ulf Hansson
Date: Fri Mar 18 2022 - 06:41:56 EST


On Fri, 18 Mar 2022 at 04:41, Chris Packham
<Chris.Packham@xxxxxxxxxxxxxxxxxxx> wrote:
>
>
> On 18/03/22 11:27, Chris Packham wrote:
> >
> > On 17/03/22 23:13, Ulf Hansson wrote:
> >> On Tue, 15 Mar 2022 at 23:05, Chris Packham
> >> <chris.packham@xxxxxxxxxxxxxxxxxxx> wrote:
> >>> Import binding documentation from the Marvell SDK which adds
> >>> marvell,ac5-sdhci compatible string and documents the requirements for
> >>> the for the Xenon SDHCI controller on the 98DX2530.
> >>>
> >>> Signed-off-by: Chris Packham <chris.packham@xxxxxxxxxxxxxxxxxxx>
> >>> Reviewed-by: Andrew Lunn <andrew@xxxxxxx>
> >>> ---
> >>>
> >>> Notes:
> >>> Changes in v3:
> >>> - Split from larger series
> >>> - Add review from Andrew
> >>> Changes in v2:
> >>> - New
> >>>
> >>> .../bindings/mmc/marvell,xenon-sdhci.txt | 52
> >>> +++++++++++++++++++
> >>> 1 file changed, 52 insertions(+)
> >> Would you mind converting these bindings to the new yaml format, as
> >> the first step?
> >>
> >> Up until this point, I have accepted only very small changes to the
> >> legacy txt based bindings, but I am starting to think that it's time
> >> to reject those too. We need all bindings to move to yaml.
> >>
> >> Sorry, if this causes additional churns for you.
> >
> > If it earns me some good karma it'll probably be worth it. Can I put
> > you down as the maintainer in the yaml binding?
>
> I've fired off a patch for converting the binding
>
> https://lore.kernel.org/linux-devicetree/20220318033521.1432767-1-chris.packham@xxxxxxxxxxxxxxxxxxx/T/#u

Thanks, I will have a look!

>
> For this change specifically I might park it. When I looked at the
> actual changes that were being made in the Marvell SDK they're doing
> something weird with dma addresses and of_dma_get_range() which won't
> work . The boards we're making won't have MMC and I don't have the
> desire to help Marvell bring their code up to scratch (at least not for
> a driver I don't need).

I see.

Then you deserve a special thanks for helping out with the DT conversion!

Kind regards
Uffe

>
> >
> >>
> >> Kind regards
> >> Uffe
> >>
> >>> diff --git
> >>> a/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
> >>> b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
> >>> index c51a62d751dc..43df466f0cb3 100644
> >>> --- a/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
> >>> +++ b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
> >>> @@ -14,6 +14,7 @@ Required Properties:
> >>> - "marvell,armada-ap806-sdhci": For controllers on Armada AP806.
> >>> - "marvell,armada-ap807-sdhci": For controllers on Armada AP807.
> >>> - "marvell,armada-cp110-sdhci": For controllers on Armada CP110.
> >>> + - "marvell,ac5-sdhci": For CnM on AC5, AC5X and derived.
> >>>
> >>> - clocks:
> >>> Array of clocks required for SDHC.
> >>> @@ -33,6 +34,13 @@ Required Properties:
> >>> in below.
> >>> Please also check property marvell,pad-type in below.
> >>>
> >>> + * For "marvell,ac5-sdhci", one or two register areas.
> >>> + (reg-names "ctrl" & "decoder").
> >>> + The first one is mandatory for the Xenon IP registers.
> >>> + The second one is for systems where DMA mapping is required and
> >>> is the
> >>> + related address decoder register (the value to configure is
> >>> derived from
> >>> + the parent "dma-ranges").
> >>> +
> >>> * For other compatible strings, one register area for Xenon IP.
> >>>
> >>> Optional Properties:
> >>> @@ -171,3 +179,47 @@ Example:
> >>>
> >>> marvell,pad-type = "sd";
> >>> };
> >>> +
> >>> +
> >>> +- For eMMC with compatible "marvell,ac5-sdhci" with one reg range
> >>> (no dma):
> >>> + sdhci0: sdhci@805c0000 {
> >>> + compatible = "marvell,ac5-sdhci";
> >>> + reg = <0x0 0x805c0000 0x0 0x300>;
> >>> + reg-names = "ctrl", "decoder";
> >>> + interrupts = <GIC_SPI 92 IRQ_TYPE_LEVEL_HIGH>;
> >>> + clocks = <&core_clock>;
> >>> + clock-names = "core";
> >>> + status = "okay";
> >>> + bus-width = <8>;
> >>> + /*marvell,xenon-phy-slow-mode;*/
> >>> + non-removable;
> >>> + mmc-ddr-1_8v;
> >>> + mmc-hs200-1_8v;
> >>> + mmc-hs400-1_8v;
> >>> + };
> >>> +
> >>> +- For eMMC with compatible "marvell,ac5-sdhci" with two reg ranges
> >>> (with dma):
> >>> + mmc_dma: mmc-dma-peripherals@80500000 {
> >>> + compatible = "simple-bus";
> >>> + #address-cells = <0x2>;
> >>> + #size-cells = <0x2>;
> >>> + ranges;
> >>> + dma-ranges = <0x2 0x0 0x2 0x80000000 0x1 0x0>;
> >>> + dma-coherent;
> >>> +
> >>> + sdhci0: sdhci@805c0000 {
> >>> + compatible = "marvell,ac5-sdhci",
> >>> "marvell,armada-ap806-sdhci";
> >>> + reg = <0x0 0x805c0000 0x0 0x300>, <0x0
> >>> 0x80440230 0x0 0x4>;
> >>> + reg-names = "ctrl", "decoder";
> >>> + interrupts = <GIC_SPI 92 IRQ_TYPE_LEVEL_HIGH>;
> >>> + clocks = <&core_clock>;
> >>> + clock-names = "core";
> >>> + status = "okay";
> >>> + bus-width = <8>;
> >>> + /*marvell,xenon-phy-slow-mode;*/
> >>> + non-removable;
> >>> + mmc-ddr-1_8v;
> >>> + mmc-hs200-1_8v;
> >>> + mmc-hs400-1_8v;
> >>> + };
> >>> + };
> >>> --
> >>> 2.35.1
> >>>