| Message ID | 20250225-hdp-upstream-v1-7-9d049c65330a@foss.st.com |
|---|---|
| State | New |
| Headers | show |
| Series | Introduce HDP support for STM32MP platforms | expand |
On 25/02/2025 09:48, Clément Le Goffic wrote: > Add the hdp devicetree node for stm32mp25 SoC family > > Signed-off-by: Clément Le Goffic <clement.legoffic@foss.st.com> > --- > arch/arm64/boot/dts/st/stm32mp251.dtsi | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/arch/arm64/boot/dts/st/stm32mp251.dtsi b/arch/arm64/boot/dts/st/stm32mp251.dtsi > index f3c6cdfd7008..43aaed4fcf10 100644 > --- a/arch/arm64/boot/dts/st/stm32mp251.dtsi > +++ b/arch/arm64/boot/dts/st/stm32mp251.dtsi > @@ -918,6 +918,13 @@ package_otp@1e8 { > }; > }; > > + hdp: pinctrl@44090000 { > + compatible = "st,stm32mp-hdp"; So here again - you have stm32mp251 SoC, but use entirely different compatible. Same feedback as with previous patchsets from ST. Please take it inside, digest internally and do not repeat same issues. Best regards, Krzysztof
On 25/02/2025 17:09, Clement LE GOFFIC wrote: > On 2/25/25 14:05, Krzysztof Kozlowski wrote: >> On 25/02/2025 09:48, Clément Le Goffic wrote: >>> Add the hdp devicetree node for stm32mp25 SoC family >>> >>> Signed-off-by: Clément Le Goffic <clement.legoffic@foss.st.com> >>> --- >>> arch/arm64/boot/dts/st/stm32mp251.dtsi | 7 +++++++ >>> 1 file changed, 7 insertions(+) >>> >>> diff --git a/arch/arm64/boot/dts/st/stm32mp251.dtsi b/arch/arm64/boot/dts/st/stm32mp251.dtsi >>> index f3c6cdfd7008..43aaed4fcf10 100644 >>> --- a/arch/arm64/boot/dts/st/stm32mp251.dtsi >>> +++ b/arch/arm64/boot/dts/st/stm32mp251.dtsi >>> @@ -918,6 +918,13 @@ package_otp@1e8 { >>> }; >>> }; >>> >>> + hdp: pinctrl@44090000 { >>> + compatible = "st,stm32mp-hdp"; >> >> So here again - you have stm32mp251 SoC, but use entirely different >> compatible. > > Ok so I will use "st,stm32mp15-hdp" This means this is stm32mp15 SoC. I do not see such SoC on list of your SoCs in bindings. What's more, there are no bindings for other SoC components for stm32mp15! Something is here not matching - this change, this DTSI, top level bindings or all of your SoC device/blocks bindings. Best regards, Krzysztof
Hi Krzysztof On 2/26/25 08:23, Krzysztof Kozlowski wrote: > On 25/02/2025 17:09, Clement LE GOFFIC wrote: >> On 2/25/25 14:05, Krzysztof Kozlowski wrote: >>> On 25/02/2025 09:48, Clément Le Goffic wrote: >>>> Add the hdp devicetree node for stm32mp25 SoC family >>>> >>>> Signed-off-by: Clément Le Goffic <clement.legoffic@foss.st.com> >>>> --- >>>> arch/arm64/boot/dts/st/stm32mp251.dtsi | 7 +++++++ >>>> 1 file changed, 7 insertions(+) >>>> >>>> diff --git a/arch/arm64/boot/dts/st/stm32mp251.dtsi b/arch/arm64/boot/dts/st/stm32mp251.dtsi >>>> index f3c6cdfd7008..43aaed4fcf10 100644 >>>> --- a/arch/arm64/boot/dts/st/stm32mp251.dtsi >>>> +++ b/arch/arm64/boot/dts/st/stm32mp251.dtsi >>>> @@ -918,6 +918,13 @@ package_otp@1e8 { >>>> }; >>>> }; >>>> >>>> + hdp: pinctrl@44090000 { >>>> + compatible = "st,stm32mp-hdp"; >>> >>> So here again - you have stm32mp251 SoC, but use entirely different >>> compatible. >> >> Ok so I will use "st,stm32mp15-hdp" > > > This means this is stm32mp15 SoC. I do not see such SoC on list of your > SoCs in bindings. What's more, there are no bindings for other SoC > components for stm32mp15! Yes stm32mp15 is not a "real SoC". I agree that at the beginning of the STM32 story we didn't have a clear rule/view to correctly naming our compatible. We tried to improve the situation to avoid compatible like "st,stm32", "st,stm32mp" or "st,stm32mp1". So we introduced "st,stm32mp13", "st,stm32mp15" or "st,stm32mp25" for new drivers. So yes it represents a SoC family and not a real SoC. We haven't had much negative feedback it. But, if it's not clean to do it in this way, lets define SoC compatible for any new driver. For the HDP case it is: "st,stm32mp157" and used for STM32MP13, STM32MP15 end STM32MP25 SoC families (if driver is the same for all those SoCs). regards Alex > Something is here not matching - this change, this DTSI, top level > bindings or all of your SoC device/blocks bindings. > > > > Best regards, > Krzysztof
On 26/02/2025 10:33, Alexandre TORGUE wrote: >>>>> + hdp: pinctrl@44090000 { >>>>> + compatible = "st,stm32mp-hdp"; >>>> >>>> So here again - you have stm32mp251 SoC, but use entirely different >>>> compatible. >>> >>> Ok so I will use "st,stm32mp15-hdp" >> >> >> This means this is stm32mp15 SoC. I do not see such SoC on list of your >> SoCs in bindings. What's more, there are no bindings for other SoC >> components for stm32mp15! > > Yes stm32mp15 is not a "real SoC". I agree that at the beginning of the > STM32 story we didn't have a clear rule/view to correctly naming our > compatible. We tried to improve the situation to avoid compatible like > "st,stm32", "st,stm32mp" or "st,stm32mp1". So we introduced > "st,stm32mp13", "st,stm32mp15" or "st,stm32mp25" for new drivers. So yes > it represents a SoC family and not a real SoC. We haven't had much > negative feedback it. > > But, if it's not clean to do it in this way, lets define SoC compatible > for any new driver. Compatibles are for hardware. > For the HDP case it is: "st,stm32mp157" and used for STM32MP13, > STM32MP15 end STM32MP25 SoC families (if driver is the same for all > those SoCs). No, it's three compatibles, because you have three SoCs. BTW, writing bindings (and online resources and previous reviews and my talks) are saying that, so we do not ask for anything new here, anything different. At least not new when looking at last 5 years, because 10 years ago many rules were relaxed... Best regards, Krzysztof
On 2/26/25 16:30, Alexandre TORGUE wrote: > > > On 2/26/25 16:08, Krzysztof Kozlowski wrote: >> On 26/02/2025 10:33, Alexandre TORGUE wrote: >>>>>>> + hdp: pinctrl@44090000 { >>>>>>> + compatible = "st,stm32mp-hdp"; >>>>>> >>>>>> So here again - you have stm32mp251 SoC, but use entirely different >>>>>> compatible. >>>>> >>>>> Ok so I will use "st,stm32mp15-hdp" >>>> >>>> >>>> This means this is stm32mp15 SoC. I do not see such SoC on list of your >>>> SoCs in bindings. What's more, there are no bindings for other SoC >>>> components for stm32mp15! >>> >>> Yes stm32mp15 is not a "real SoC". I agree that at the beginning of the >>> STM32 story we didn't have a clear rule/view to correctly naming our >>> compatible. We tried to improve the situation to avoid compatible like >>> "st,stm32", "st,stm32mp" or "st,stm32mp1". So we introduced >>> "st,stm32mp13", "st,stm32mp15" or "st,stm32mp25" for new drivers. So yes >>> it represents a SoC family and not a real SoC. We haven't had much >>> negative feedback it. >>> >>> But, if it's not clean to do it in this way, lets define SoC compatible >>> for any new driver. >> >> Compatibles are for hardware. >> >>> For the HDP case it is: "st,stm32mp157" and used for STM32MP13, >>> STM32MP15 end STM32MP25 SoC families (if driver is the same for all >>> those SoCs). >> >> No, it's three compatibles, because you have three SoCs. BTW, writing >> bindings (and online resources and previous reviews and my talks) are >> saying that, so we do not ask for anything new here, anything different. >> At least not new when looking at last 5 years, because 10 years ago many >> rules were relaxed... > > So adding 3 times the same IP in 3 different SoCs implies to have 3 > different compatibles. So each time we use this same IP in a new SoC, we > have to add a new compatible. My (wrong) understanding was: as we have > the same IP (same hardware) in each SoC we have the same compatible (and > IP integration differences (clocks, interrupts) are handled by DT > properties. Just to complete, reading the Linux kernel doc, as device are same we will use fallbacks like this: MP15: compatible = "st,stm32mp151-hdp"; MP13: compatible = "st,stm32mp131-hdp", "st,stm32mp151-hdp"; MP25: compatible = "st,stm32mp251-hdp", "st,stm32mp151-hdp"; > >> >> >> Best regards, >> Krzysztof
On 26/02/2025 16:30, Alexandre TORGUE wrote: > > > On 2/26/25 16:08, Krzysztof Kozlowski wrote: >> On 26/02/2025 10:33, Alexandre TORGUE wrote: >>>>>>> + hdp: pinctrl@44090000 { >>>>>>> + compatible = "st,stm32mp-hdp"; >>>>>> >>>>>> So here again - you have stm32mp251 SoC, but use entirely different >>>>>> compatible. >>>>> >>>>> Ok so I will use "st,stm32mp15-hdp" >>>> >>>> >>>> This means this is stm32mp15 SoC. I do not see such SoC on list of your >>>> SoCs in bindings. What's more, there are no bindings for other SoC >>>> components for stm32mp15! >>> >>> Yes stm32mp15 is not a "real SoC". I agree that at the beginning of the >>> STM32 story we didn't have a clear rule/view to correctly naming our >>> compatible. We tried to improve the situation to avoid compatible like >>> "st,stm32", "st,stm32mp" or "st,stm32mp1". So we introduced >>> "st,stm32mp13", "st,stm32mp15" or "st,stm32mp25" for new drivers. So yes >>> it represents a SoC family and not a real SoC. We haven't had much >>> negative feedback it. >>> >>> But, if it's not clean to do it in this way, lets define SoC compatible >>> for any new driver. >> >> Compatibles are for hardware. >> >>> For the HDP case it is: "st,stm32mp157" and used for STM32MP13, >>> STM32MP15 end STM32MP25 SoC families (if driver is the same for all >>> those SoCs). >> >> No, it's three compatibles, because you have three SoCs. BTW, writing >> bindings (and online resources and previous reviews and my talks) are >> saying that, so we do not ask for anything new here, anything different. >> At least not new when looking at last 5 years, because 10 years ago many >> rules were relaxed... > > So adding 3 times the same IP in 3 different SoCs implies to have 3 Yes. Always, as requested by writing bindings. > different compatibles. So each time we use this same IP in a new SoC, we > have to add a new compatible. My (wrong) understanding was: as we have Yes, as requested by writing bindings and followed up by all recent platforms having decent/active upstream support. See qcom, nxp, renesas for example. > the same IP (same hardware) in each SoC we have the same compatible (and You do not have same hardware. You have same IP, or almost same because they are almost never same, implemented in different hardware. > IP integration differences (clocks, interrupts) are handled by DT > properties. Which binding doc/guide suggested such way? Countless reviews from DT maintainers were saying opposite. Best regards, Krzysztof
diff --git a/arch/arm64/boot/dts/st/stm32mp251.dtsi b/arch/arm64/boot/dts/st/stm32mp251.dtsi index f3c6cdfd7008..43aaed4fcf10 100644 --- a/arch/arm64/boot/dts/st/stm32mp251.dtsi +++ b/arch/arm64/boot/dts/st/stm32mp251.dtsi @@ -918,6 +918,13 @@ package_otp@1e8 { }; }; + hdp: pinctrl@44090000 { + compatible = "st,stm32mp-hdp"; + reg = <0x44090000 0x400>; + clocks = <&rcc CK_BUS_HDP>; + status = "disabled"; + }; + rcc: clock-controller@44200000 { compatible = "st,stm32mp25-rcc"; reg = <0x44200000 0x10000>;
Add the hdp devicetree node for stm32mp25 SoC family Signed-off-by: Clément Le Goffic <clement.legoffic@foss.st.com> --- arch/arm64/boot/dts/st/stm32mp251.dtsi | 7 +++++++ 1 file changed, 7 insertions(+)