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

[Chris Packham]

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

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).

>
>>
>> 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
>>>