[1/6] dt-bindings: mmc: mmci: Add st,stm32mp25-sdmmc2 compatible

Message ID 20230615092001.1213132-2-yann.gautier@foss.st.com
State New
Headers
Series Update MMCI driver for STM32MP25 |

Commit Message

Yann Gautier June 15, 2023, 9:19 a.m. UTC
  For STM32MP25, we'll need to distinguish how is managed the delay block.
This is done through a new comptible dedicated for this SoC, as the
delay block registers are located in SYSCFG peripheral.

Signed-off-by: Yann Gautier <yann.gautier@foss.st.com>
---
 Documentation/devicetree/bindings/mmc/arm,pl18x.yaml | 6 ++++++
 1 file changed, 6 insertions(+)
  

Comments

Ulf Hansson June 15, 2023, 1:20 p.m. UTC | #1
On Thu, 15 Jun 2023 at 11:20, Yann Gautier <yann.gautier@foss.st.com> wrote:
>
> For STM32MP25, we'll need to distinguish how is managed the delay block.
> This is done through a new comptible dedicated for this SoC, as the
> delay block registers are located in SYSCFG peripheral.
>
> Signed-off-by: Yann Gautier <yann.gautier@foss.st.com>
> ---
>  Documentation/devicetree/bindings/mmc/arm,pl18x.yaml | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mmc/arm,pl18x.yaml b/Documentation/devicetree/bindings/mmc/arm,pl18x.yaml
> index 1c96da04f0e53..e47b3418b6c77 100644
> --- a/Documentation/devicetree/bindings/mmc/arm,pl18x.yaml
> +++ b/Documentation/devicetree/bindings/mmc/arm,pl18x.yaml
> @@ -59,6 +59,12 @@ properties:
>            - const: st,stm32-sdmmc2
>            - const: arm,pl18x
>            - const: arm,primecell
> +      - description: Entry for STMicroelectronics variant of PL18x for
> +          STM32MP25. This dedicated compatible is used by bootloaders.

What does this last sentence mean? Can we drop it?

> +        items:
> +          - const: st,stm32mp25-sdmmc2
> +          - const: arm,pl18x
> +          - const: arm,primecell
>
>    clocks:
>      description: One or two clocks, the "apb_pclk" and the "MCLK"
> --
> 2.25.1
>

Kind regards
Uffe
  
Yann Gautier June 15, 2023, 3:16 p.m. UTC | #2
On 6/15/23 15:20, Ulf Hansson wrote:
> On Thu, 15 Jun 2023 at 11:20, Yann Gautier <yann.gautier@foss.st.com> wrote:
>>
>> For STM32MP25, we'll need to distinguish how is managed the delay block.
>> This is done through a new comptible dedicated for this SoC, as the
>> delay block registers are located in SYSCFG peripheral.
>>
>> Signed-off-by: Yann Gautier <yann.gautier@foss.st.com>
>> ---
>>   Documentation/devicetree/bindings/mmc/arm,pl18x.yaml | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/arm,pl18x.yaml b/Documentation/devicetree/bindings/mmc/arm,pl18x.yaml
>> index 1c96da04f0e53..e47b3418b6c77 100644
>> --- a/Documentation/devicetree/bindings/mmc/arm,pl18x.yaml
>> +++ b/Documentation/devicetree/bindings/mmc/arm,pl18x.yaml
>> @@ -59,6 +59,12 @@ properties:
>>             - const: st,stm32-sdmmc2
>>             - const: arm,pl18x
>>             - const: arm,primecell
>> +      - description: Entry for STMicroelectronics variant of PL18x for
>> +          STM32MP25. This dedicated compatible is used by bootloaders.
> 
> What does this last sentence mean? Can we drop it?

Hi Ulf,

I just copied (and added "for STM32MP25") what was done for STM32MP1x:
       - description: Entry for STMicroelectronics variant of PL18x.
           This dedicated compatible is used by bootloaders.
         items:
           - const: st,stm32-sdmmc2
           - const: arm,pl18x
           - const: arm,primecell
       - description: Entry for STMicroelectronics variant of PL18x for
           STM32MP25. This dedicated compatible is used by bootloaders.
         items:
           - const: st,stm32mp25-sdmmc2
           - const: arm,pl18x
           - const: arm,primecell


Should I remove (or adapt) both descriptions?


Best regards,
Yann

> 
>> +        items:
>> +          - const: st,stm32mp25-sdmmc2
>> +          - const: arm,pl18x
>> +          - const: arm,primecell
>>
>>     clocks:
>>       description: One or two clocks, the "apb_pclk" and the "MCLK"
>> --
>> 2.25.1
>>
> 
> Kind regards
> Uffe
  
Yann Gautier June 15, 2023, 3:19 p.m. UTC | #3
On 6/15/23 17:16, Yann Gautier wrote:
> On 6/15/23 15:20, Ulf Hansson wrote:
>> On Thu, 15 Jun 2023 at 11:20, Yann Gautier <yann.gautier@foss.st.com> 
>> wrote:
>>>
>>> For STM32MP25, we'll need to distinguish how is managed the delay block.
>>> This is done through a new comptible dedicated for this SoC, as the
>>> delay block registers are located in SYSCFG peripheral.
>>>
>>> Signed-off-by: Yann Gautier <yann.gautier@foss.st.com>
>>> ---
>>>   Documentation/devicetree/bindings/mmc/arm,pl18x.yaml | 6 ++++++
>>>   1 file changed, 6 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/mmc/arm,pl18x.yaml 
>>> b/Documentation/devicetree/bindings/mmc/arm,pl18x.yaml
>>> index 1c96da04f0e53..e47b3418b6c77 100644
>>> --- a/Documentation/devicetree/bindings/mmc/arm,pl18x.yaml
>>> +++ b/Documentation/devicetree/bindings/mmc/arm,pl18x.yaml
>>> @@ -59,6 +59,12 @@ properties:
>>>             - const: st,stm32-sdmmc2
>>>             - const: arm,pl18x
>>>             - const: arm,primecell
>>> +      - description: Entry for STMicroelectronics variant of PL18x for
>>> +          STM32MP25. This dedicated compatible is used by bootloaders.
>>
>> What does this last sentence mean? Can we drop it?
> 
> Hi Ulf,
> 
> I just copied (and added "for STM32MP25") what was done for STM32MP1x:
>        - description: Entry for STMicroelectronics variant of PL18x.
>            This dedicated compatible is used by bootloaders.
>          items:
>            - const: st,stm32-sdmmc2
>            - const: arm,pl18x
>            - const: arm,primecell
>        - description: Entry for STMicroelectronics variant of PL18x for
>            STM32MP25. This dedicated compatible is used by bootloaders.
>          items:
>            - const: st,stm32mp25-sdmmc2
>            - const: arm,pl18x
>            - const: arm,primecell
> 
> 
> Should I remove (or adapt) both descriptions?
> 
> 
> Best regards,
> Yann
> 

At the time the patch was done it was really just used by bootloaders.
But as it is now used in the driver for delay block, I should remove the 
second sentence.


Yann

>>
>>> +        items:
>>> +          - const: st,stm32mp25-sdmmc2
>>> +          - const: arm,pl18x
>>> +          - const: arm,primecell
>>>
>>>     clocks:
>>>       description: One or two clocks, the "apb_pclk" and the "MCLK"
>>> -- 
>>> 2.25.1
>>>
>>
>> Kind regards
>> Uffe
>
  
Linus Walleij June 15, 2023, 6:51 p.m. UTC | #4
On Thu, Jun 15, 2023 at 5:19 PM Yann Gautier <yann.gautier@foss.st.com> wrote:

> >        - description: Entry for STMicroelectronics variant of PL18x.
> >            This dedicated compatible is used by bootloaders.
(...)
> >        - description: Entry for STMicroelectronics variant of PL18x for
> >            STM32MP25. This dedicated compatible is used by bootloaders.
(...)
> > Should I remove (or adapt) both descriptions?
> >
> >
>
> At the time the patch was done it was really just used by bootloaders.
> But as it is now used in the driver for delay block, I should remove the
> second sentence.

Remove both.

After "This dedicated compatible is used by bootloaders" there is
an implicit "in the SDK provided by ST Microelectronics", and that
is of no concern for DT bindings, which are (well, in theory) used by
e.g. BSD or other operating systems and who knows what they will
use and not, we don't put Linux specifics in there, neither Boot
loader specifics nor ST SDK specifics.

At least that is the little bureaucratic ambition we have.

Yours,
Linus Walleij
  
Krzysztof Kozlowski June 17, 2023, 8:04 a.m. UTC | #5
On 15/06/2023 11:19, Yann Gautier wrote:
> For STM32MP25, we'll need to distinguish how is managed the delay block.
> This is done through a new comptible dedicated for this SoC, as the
> delay block registers are located in SYSCFG peripheral.
> 
> Signed-off-by: Yann Gautier <yann.gautier@foss.st.com>
> ---
>  Documentation/devicetree/bindings/mmc/arm,pl18x.yaml | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/arm,pl18x.yaml b/Documentation/devicetree/bindings/mmc/arm,pl18x.yaml
> index 1c96da04f0e53..e47b3418b6c77 100644
> --- a/Documentation/devicetree/bindings/mmc/arm,pl18x.yaml
> +++ b/Documentation/devicetree/bindings/mmc/arm,pl18x.yaml
> @@ -59,6 +59,12 @@ properties:
>            - const: st,stm32-sdmmc2
>            - const: arm,pl18x
>            - const: arm,primecell
> +      - description: Entry for STMicroelectronics variant of PL18x for
> +          STM32MP25. This dedicated compatible is used by bootloaders.
> +        items:
> +          - const: st,stm32mp25-sdmmc2

Except what's said, this looks like can be part of previous entry via enum.

Best regards,
Krzysztof
  
Yann Gautier June 19, 2023, 7:29 a.m. UTC | #6
On 6/15/23 20:51, Linus Walleij wrote:
> On Thu, Jun 15, 2023 at 5:19 PM Yann Gautier <yann.gautier@foss.st.com> wrote:
> 
>>>         - description: Entry for STMicroelectronics variant of PL18x.
>>>             This dedicated compatible is used by bootloaders.
> (...)
>>>         - description: Entry for STMicroelectronics variant of PL18x for
>>>             STM32MP25. This dedicated compatible is used by bootloaders.
> (...)
>>> Should I remove (or adapt) both descriptions?
>>>
>>>
>>
>> At the time the patch was done it was really just used by bootloaders.
>> But as it is now used in the driver for delay block, I should remove the
>> second sentence.
> 
> Remove both.
> 
> After "This dedicated compatible is used by bootloaders" there is
> an implicit "in the SDK provided by ST Microelectronics", and that
> is of no concern for DT bindings, which are (well, in theory) used by
> e.g. BSD or other operating systems and who knows what they will
> use and not, we don't put Linux specifics in there, neither Boot
> loader specifics nor ST SDK specifics.
> 
> At least that is the little bureaucratic ambition we have.
> 
> Yours,
> Linus Walleij

Hi,

Thanks for all the reviews.
I'll update this patch in the v2, removing bootloader line and using enum.

Ulf, should I send the new series now, or do you prefer to review the 
whole series before?


Yann
  
Ulf Hansson June 19, 2023, 11:01 a.m. UTC | #7
On Mon, 19 Jun 2023 at 09:29, Yann Gautier <yann.gautier@foss.st.com> wrote:
>
> On 6/15/23 20:51, Linus Walleij wrote:
> > On Thu, Jun 15, 2023 at 5:19 PM Yann Gautier <yann.gautier@foss.st.com> wrote:
> >
> >>>         - description: Entry for STMicroelectronics variant of PL18x.
> >>>             This dedicated compatible is used by bootloaders.
> > (...)
> >>>         - description: Entry for STMicroelectronics variant of PL18x for
> >>>             STM32MP25. This dedicated compatible is used by bootloaders.
> > (...)
> >>> Should I remove (or adapt) both descriptions?
> >>>
> >>>
> >>
> >> At the time the patch was done it was really just used by bootloaders.
> >> But as it is now used in the driver for delay block, I should remove the
> >> second sentence.
> >
> > Remove both.
> >
> > After "This dedicated compatible is used by bootloaders" there is
> > an implicit "in the SDK provided by ST Microelectronics", and that
> > is of no concern for DT bindings, which are (well, in theory) used by
> > e.g. BSD or other operating systems and who knows what they will
> > use and not, we don't put Linux specifics in there, neither Boot
> > loader specifics nor ST SDK specifics.
> >
> > At least that is the little bureaucratic ambition we have.
> >
> > Yours,
> > Linus Walleij
>
> Hi,
>
> Thanks for all the reviews.
> I'll update this patch in the v2, removing bootloader line and using enum.
>
> Ulf, should I send the new series now, or do you prefer to review the
> whole series before?

Actually I have already looked through the series and it looks good to
me! Please submit a new version so we can get this queued up for v6.5.

Kind regards
Uffe
  

Patch

diff --git a/Documentation/devicetree/bindings/mmc/arm,pl18x.yaml b/Documentation/devicetree/bindings/mmc/arm,pl18x.yaml
index 1c96da04f0e53..e47b3418b6c77 100644
--- a/Documentation/devicetree/bindings/mmc/arm,pl18x.yaml
+++ b/Documentation/devicetree/bindings/mmc/arm,pl18x.yaml
@@ -59,6 +59,12 @@  properties:
           - const: st,stm32-sdmmc2
           - const: arm,pl18x
           - const: arm,primecell
+      - description: Entry for STMicroelectronics variant of PL18x for
+          STM32MP25. This dedicated compatible is used by bootloaders.
+        items:
+          - const: st,stm32mp25-sdmmc2
+          - const: arm,pl18x
+          - const: arm,primecell
 
   clocks:
     description: One or two clocks, the "apb_pclk" and the "MCLK"