Message ID | 20210126090120.19900-1-gabriel.fernandez@foss.st.com (mailing list archive) |
---|---|
Headers | show |
Series | Introduce STM32MP1 RCC in secured mode | expand |
On 1/26/21 3:01 AM, gabriel.fernandez@foss.st.com wrote: > From: Gabriel Fernandez <gabriel.fernandez@foss.st.com> > > Platform STM32MP1 can be used in configuration where some clocks and > IP resets can relate as secure resources. > These resources are moved from a RCC clock/reset handle to a SCMI > clock/reset_domain handle. > > The RCC clock driver is now dependent of the SCMI driver, then we have > to manage now the probe defering. > > v1 -> v2: > - fix yamllint warnings. Hi Gabriel, I don't have much clout with the maintainers, but I have to NAK this series after finding major breakage. The problem with series is that it breaks pretty much every board it touches. I have a DK2 here that I'm using for development, which no longer boots with this series applied. The crux of the matter is that this series assumes all boards will boot with an FSBL that implements a very specific SCMI clock tree. This is major ABI breakage for anyone not using TF-A as the first stage bootloader. Anyone using u-boot SPL is screwed. This series imposes a SOC-wide change via the dtsi files. So even boards that you don't intend to convert to SCMI will get broken this way. Adding a -no-scmi file that isn't used anywhere doesn't help things. Here's what I suggest: Generate new dtb files for those boards that you want to convert. So you would get: - stm32mp157c-dk2.dtb # Good old hardware clocks - stm32mp157c-dk2-secure-rcc.dtb # Clocks accessible by scmi. A lot of users use a larger build system where they extract the relevant files. With the scheme I'm proposing you don't break their builds, and you allow SCMI users to have upstream support. This means that you'll have to rethink the DTS and DTSI changes to accomodate both use cases. Thanks, Alex > > Gabriel Fernandez (14): > clk: stm32mp1: merge 'clk-hsi-div' and 'ck_hsi' into one clock > clk: stm32mp1: merge 'ck_hse_rtc' and 'ck_rtc' into one clock > clk: stm32mp1: remove intermediate pll clocks > clk: stm32mp1: convert to module driver > clk: stm32mp1: move RCC reset controller into RCC clock driver > reset: stm32mp1: remove stm32mp1 reset > dt-bindings: clock: add IDs for SCMI clocks on stm32mp15 > dt-bindings: reset: add IDs for SCMI reset domains on stm32mp15 > dt-bindings: reset: add MCU HOLD BOOT ID for SCMI reset domains on > stm32mp15 > clk: stm32mp1: new compatible for secure RCC support > ARM: dts: stm32: define SCMI resources on stm32mp15 > ARM: dts: stm32: move clocks/resets to SCMI resources for stm32mp15 > dt-bindings: clock: stm32mp1 new compatible for secure rcc > ARM: dts: stm32: introduce basic boot include on stm32mp15x board > > .../bindings/clock/st,stm32mp1-rcc.yaml | 6 +- > arch/arm/boot/dts/stm32mp15-no-scmi.dtsi | 158 ++++++ > arch/arm/boot/dts/stm32mp151.dtsi | 127 +++-- > arch/arm/boot/dts/stm32mp153.dtsi | 4 +- > arch/arm/boot/dts/stm32mp157.dtsi | 2 +- > arch/arm/boot/dts/stm32mp15xc.dtsi | 4 +- > drivers/clk/Kconfig | 10 + > drivers/clk/clk-stm32mp1.c | 495 +++++++++++++++--- > drivers/reset/Kconfig | 6 - > drivers/reset/Makefile | 1 - > drivers/reset/reset-stm32mp1.c | 115 ---- > include/dt-bindings/clock/stm32mp1-clks.h | 27 + > include/dt-bindings/reset/stm32mp1-resets.h | 15 + > 13 files changed, 704 insertions(+), 266 deletions(-) > create mode 100644 arch/arm/boot/dts/stm32mp15-no-scmi.dtsi > delete mode 100644 drivers/reset/reset-stm32mp1.c >
Hi ALex > -----Original Message----- > From: Alex G. <mr.nuke.me@gmail.com> > Sent: mardi 9 mars 2021 22:50 > To: Gabriel FERNANDEZ - foss <gabriel.fernandez@foss.st.com>; Michael > Turquette <mturquette@baylibre.com>; Stephen Boyd > <sboyd@kernel.org>; Rob Herring <robh+dt@kernel.org>; Maxime Coquelin > <mcoquelin.stm32@gmail.com>; Alexandre TORGUE > <alexandre.torgue@st.com>; Philipp Zabel <p.zabel@pengutronix.de>; > Etienne CARRIERE <etienne.carriere@st.com>; marex@denx.de; Marek > Vasut <marex@denx.de> > Cc: devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux- > clk@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-stm32@st- > md-mailman.stormreply.com > Subject: Re: [PATCH v2 00/14] Introduce STM32MP1 RCC in secured mode > > On 1/26/21 3:01 AM, gabriel.fernandez@foss.st.com wrote: > > From: Gabriel Fernandez <gabriel.fernandez@foss.st.com> > > > > Platform STM32MP1 can be used in configuration where some clocks and > > IP resets can relate as secure resources. > > These resources are moved from a RCC clock/reset handle to a SCMI > > clock/reset_domain handle. > > > > The RCC clock driver is now dependent of the SCMI driver, then we have > > to manage now the probe defering. > > > > v1 -> v2: > > - fix yamllint warnings. > > Hi Gabriel, > > I don't have much clout with the maintainers, but I have to NAK this series > after finding major breakage. > > The problem with series is that it breaks pretty much every board it touches. > I have a DK2 here that I'm using for development, which no longer boots with > this series applied. > > The crux of the matter is that this series assumes all boards will boot with an > FSBL that implements a very specific SCMI clock tree. This is major ABI > breakage for anyone not using TF-A as the first stage bootloader. Anyone > using u-boot SPL is screwed. > > This series imposes a SOC-wide change via the dtsi files. So even boards that > you don't intend to convert to SCMI will get broken this way. > Adding a -no-scmi file that isn't used anywhere doesn't help things. You are right. We mainly take care about NO ST (DH/...) boards, but not really about current usage Of our stm32 boards. Several options exist: 1- Break the current ABI: as soon as those patches are merged, stm32mp157c-dk2.dtb will impose to use A tf-a for scmi clocks. For people using u-boot spl, the will have to create their own "no-secure" devicetree. 2-As you suggest, create a new "secure" dtb per boards (Not my wish for maintenance perspectives). 3- Keep kernel device tree as they are and applied this secure layer (scmi clocks phandle) thanks to dtbo in U-boot. The third could be the less costly. Thanks Alex > Here's what I suggest: > > Generate new dtb files for those boards that you want to convert. So you > would get: > - stm32mp157c-dk2.dtb # Good old hardware clocks > - stm32mp157c-dk2-secure-rcc.dtb # Clocks accessible by scmi. > > A lot of users use a larger build system where they extract the relevant files. > With the scheme I'm proposing you don't break their builds, and you allow > SCMI users to have upstream support. > > This means that you'll have to rethink the DTS and DTSI changes to > accomodate both use cases. > > Thanks, > Alex > > > > > > > > Gabriel Fernandez (14): > > clk: stm32mp1: merge 'clk-hsi-div' and 'ck_hsi' into one clock > > clk: stm32mp1: merge 'ck_hse_rtc' and 'ck_rtc' into one clock > > clk: stm32mp1: remove intermediate pll clocks > > clk: stm32mp1: convert to module driver > > clk: stm32mp1: move RCC reset controller into RCC clock driver > > reset: stm32mp1: remove stm32mp1 reset > > dt-bindings: clock: add IDs for SCMI clocks on stm32mp15 > > dt-bindings: reset: add IDs for SCMI reset domains on stm32mp15 > > dt-bindings: reset: add MCU HOLD BOOT ID for SCMI reset domains on > > stm32mp15 > > clk: stm32mp1: new compatible for secure RCC support > > ARM: dts: stm32: define SCMI resources on stm32mp15 > > ARM: dts: stm32: move clocks/resets to SCMI resources for stm32mp15 > > dt-bindings: clock: stm32mp1 new compatible for secure rcc > > ARM: dts: stm32: introduce basic boot include on stm32mp15x board > > > > .../bindings/clock/st,stm32mp1-rcc.yaml | 6 +- > > arch/arm/boot/dts/stm32mp15-no-scmi.dtsi | 158 ++++++ > > arch/arm/boot/dts/stm32mp151.dtsi | 127 +++-- > > arch/arm/boot/dts/stm32mp153.dtsi | 4 +- > > arch/arm/boot/dts/stm32mp157.dtsi | 2 +- > > arch/arm/boot/dts/stm32mp15xc.dtsi | 4 +- > > drivers/clk/Kconfig | 10 + > > drivers/clk/clk-stm32mp1.c | 495 +++++++++++++++--- > > drivers/reset/Kconfig | 6 - > > drivers/reset/Makefile | 1 - > > drivers/reset/reset-stm32mp1.c | 115 ---- > > include/dt-bindings/clock/stm32mp1-clks.h | 27 + > > include/dt-bindings/reset/stm32mp1-resets.h | 15 + > > 13 files changed, 704 insertions(+), 266 deletions(-) > > create mode 100644 arch/arm/boot/dts/stm32mp15-no-scmi.dtsi > > delete mode 100644 drivers/reset/reset-stm32mp1.c > >
On 3/11/21 9:08 AM, Alexandre TORGUE wrote: > Hi ALex Hello everyone, [...] >> Subject: Re: [PATCH v2 00/14] Introduce STM32MP1 RCC in secured mode >> >> On 1/26/21 3:01 AM, gabriel.fernandez@foss.st.com wrote: >>> From: Gabriel Fernandez <gabriel.fernandez@foss.st.com> >>> >>> Platform STM32MP1 can be used in configuration where some clocks and >>> IP resets can relate as secure resources. >>> These resources are moved from a RCC clock/reset handle to a SCMI >>> clock/reset_domain handle. >>> >>> The RCC clock driver is now dependent of the SCMI driver, then we have >>> to manage now the probe defering. >>> >>> v1 -> v2: >>> - fix yamllint warnings. >> >> Hi Gabriel, >> >> I don't have much clout with the maintainers, but I have to NAK this series >> after finding major breakage. >> >> The problem with series is that it breaks pretty much every board it touches. >> I have a DK2 here that I'm using for development, which no longer boots with >> this series applied. >> >> The crux of the matter is that this series assumes all boards will boot with an >> FSBL that implements a very specific SCMI clock tree. This is major ABI >> breakage for anyone not using TF-A as the first stage bootloader. Anyone >> using u-boot SPL is screwed. >> >> This series imposes a SOC-wide change via the dtsi files. So even boards that >> you don't intend to convert to SCMI will get broken this way. >> Adding a -no-scmi file that isn't used anywhere doesn't help things. > > You are right. We mainly take care about NO ST (DH/...) boards, but not really about current usage > Of our stm32 boards. Several options exist: Since a lot of people benefit from the good upstream support for the MP1 _and_ keep updating their machines to get the latest fixes, it is very important to keep the current usage working. > 1- Break the current ABI: as soon as those patches are merged, stm32mp157c-dk2.dtb will impose to use > A tf-a for scmi clocks. For people using u-boot spl, the will have to create their own "no-secure" devicetree. NAK, this breaks existing boards and existing setups, e.g. DK2 that does not use ATF. > 2-As you suggest, create a new "secure" dtb per boards (Not my wish for maintenance perspectives). I agree with Alex (G) that the "secure" option should be opt-in. That way existing setups remain working and no extra requirements are imposed on MP1 users. Esp. since as far as I understand this, the "secure" part isn't really about security, but rather about moving clock configuration from Linux to some firmware blob. > 3- Keep kernel device tree as they are and applied this secure layer (scmi clocks phandle) thanks to dtbo in > U-boot. Is this really better than #include "stm32mp15xx-enable-secure-stuff.dtsi" in a board DT ? Because that is how I imagine the opt-in "secure" option could work. > The third could be the less costly. [...]
Hi Marek On 3/11/21 12:43 PM, Marek Vasut wrote: > On 3/11/21 9:08 AM, Alexandre TORGUE wrote: >> Hi ALex > > Hello everyone, > > [...] > >>> Subject: Re: [PATCH v2 00/14] Introduce STM32MP1 RCC in secured mode >>> >>> On 1/26/21 3:01 AM, gabriel.fernandez@foss.st.com wrote: >>>> From: Gabriel Fernandez <gabriel.fernandez@foss.st.com> >>>> >>>> Platform STM32MP1 can be used in configuration where some clocks and >>>> IP resets can relate as secure resources. >>>> These resources are moved from a RCC clock/reset handle to a SCMI >>>> clock/reset_domain handle. >>>> >>>> The RCC clock driver is now dependent of the SCMI driver, then we have >>>> to manage now the probe defering. >>>> >>>> v1 -> v2: >>>> - fix yamllint warnings. >>> >>> Hi Gabriel, >>> >>> I don't have much clout with the maintainers, but I have to NAK this >>> series >>> after finding major breakage. >>> >>> The problem with series is that it breaks pretty much every board it >>> touches. >>> I have a DK2 here that I'm using for development, which no longer >>> boots with >>> this series applied. >>> >>> The crux of the matter is that this series assumes all boards will >>> boot with an >>> FSBL that implements a very specific SCMI clock tree. This is major ABI >>> breakage for anyone not using TF-A as the first stage bootloader. Anyone >>> using u-boot SPL is screwed. >>> >>> This series imposes a SOC-wide change via the dtsi files. So even >>> boards that >>> you don't intend to convert to SCMI will get broken this way. >>> Adding a -no-scmi file that isn't used anywhere doesn't help things. >> >> You are right. We mainly take care about NO ST (DH/...) boards, but >> not really about current usage >> Of our stm32 boards. Several options exist: > > Since a lot of people benefit from the good upstream support for the MP1 > _and_ keep updating their machines to get the latest fixes, it is very > important to keep the current usage working. > >> 1- Break the current ABI: as soon as those patches are merged, >> stm32mp157c-dk2.dtb will impose to use >> A tf-a for scmi clocks. For people using u-boot spl, the will have to >> create their own "no-secure" devicetree. > > NAK, this breaks existing boards and existing setups, e.g. DK2 that does > not use ATF. > >> 2-As you suggest, create a new "secure" dtb per boards (Not my wish >> for maintenance perspectives). > > I agree with Alex (G) that the "secure" option should be opt-in. > That way existing setups remain working and no extra requirements are > imposed on MP1 users. Esp. since as far as I understand this, the > "secure" part isn't really about security, but rather about moving clock > configuration from Linux to some firmware blob. > >> 3- Keep kernel device tree as they are and applied this secure layer >> (scmi clocks phandle) thanks to dtbo in >> U-boot. > > Is this really better than > #include "stm32mp15xx-enable-secure-stuff.dtsi" > in a board DT ? Because that is how I imagine the opt-in "secure" option > could work. The dtbo usage could avoid to add another st board (actually a secure config) in arch/arm/boot/dts. Cheers Alex > >> The third could be the less costly. > > [...]
On 3/11/21 2:15 PM, Alexandre TORGUE wrote: > Hi Marek Hello Alexandre, > On 3/11/21 12:43 PM, Marek Vasut wrote: >> On 3/11/21 9:08 AM, Alexandre TORGUE wrote: >>> Hi ALex >> >> Hello everyone, >> >> [...] >> >>>> Subject: Re: [PATCH v2 00/14] Introduce STM32MP1 RCC in secured mode >>>> >>>> On 1/26/21 3:01 AM, gabriel.fernandez@foss.st.com wrote: >>>>> From: Gabriel Fernandez <gabriel.fernandez@foss.st.com> >>>>> >>>>> Platform STM32MP1 can be used in configuration where some clocks and >>>>> IP resets can relate as secure resources. >>>>> These resources are moved from a RCC clock/reset handle to a SCMI >>>>> clock/reset_domain handle. >>>>> >>>>> The RCC clock driver is now dependent of the SCMI driver, then we have >>>>> to manage now the probe defering. >>>>> >>>>> v1 -> v2: >>>>> - fix yamllint warnings. >>>> >>>> Hi Gabriel, >>>> >>>> I don't have much clout with the maintainers, but I have to NAK this >>>> series >>>> after finding major breakage. >>>> >>>> The problem with series is that it breaks pretty much every board it >>>> touches. >>>> I have a DK2 here that I'm using for development, which no longer >>>> boots with >>>> this series applied. >>>> >>>> The crux of the matter is that this series assumes all boards will >>>> boot with an >>>> FSBL that implements a very specific SCMI clock tree. This is major ABI >>>> breakage for anyone not using TF-A as the first stage bootloader. >>>> Anyone >>>> using u-boot SPL is screwed. >>>> >>>> This series imposes a SOC-wide change via the dtsi files. So even >>>> boards that >>>> you don't intend to convert to SCMI will get broken this way. >>>> Adding a -no-scmi file that isn't used anywhere doesn't help things. >>> >>> You are right. We mainly take care about NO ST (DH/...) boards, but >>> not really about current usage >>> Of our stm32 boards. Several options exist: >> >> Since a lot of people benefit from the good upstream support for the >> MP1 _and_ keep updating their machines to get the latest fixes, it is >> very important to keep the current usage working. >> >>> 1- Break the current ABI: as soon as those patches are merged, >>> stm32mp157c-dk2.dtb will impose to use >>> A tf-a for scmi clocks. For people using u-boot spl, the will have to >>> create their own "no-secure" devicetree. >> >> NAK, this breaks existing boards and existing setups, e.g. DK2 that >> does not use ATF. > >>> 2-As you suggest, create a new "secure" dtb per boards (Not my wish >>> for maintenance perspectives). >> >> I agree with Alex (G) that the "secure" option should be opt-in. >> That way existing setups remain working and no extra requirements are >> imposed on MP1 users. Esp. since as far as I understand this, the >> "secure" part isn't really about security, but rather about moving >> clock configuration from Linux to some firmware blob. >> >>> 3- Keep kernel device tree as they are and applied this secure layer >>> (scmi clocks phandle) thanks to dtbo in >>> U-boot. >> >> Is this really better than >> #include "stm32mp15xx-enable-secure-stuff.dtsi" >> in a board DT ? Because that is how I imagine the opt-in "secure" >> option could work. > > The dtbo usage could avoid to add another st board (actually a secure > config) in arch/arm/boot/dts. It isn't even a board, it is a configuration. Could you detect this secure/non-secure state at runtime, have both clock options in the DT, and handle it accordingly ? That might be even better option.
Hi On 3/11/21 12:43 PM, Marek Vasut wrote: > On 3/11/21 9:08 AM, Alexandre TORGUE wrote: >> Hi ALex > > Hello everyone, > > [...] > >>> Subject: Re: [PATCH v2 00/14] Introduce STM32MP1 RCC in secured mode >>> >>> On 1/26/21 3:01 AM, gabriel.fernandez@foss.st.com wrote: >>>> From: Gabriel Fernandez <gabriel.fernandez@foss.st.com> >>>> >>>> Platform STM32MP1 can be used in configuration where some clocks and >>>> IP resets can relate as secure resources. >>>> These resources are moved from a RCC clock/reset handle to a SCMI >>>> clock/reset_domain handle. >>>> >>>> The RCC clock driver is now dependent of the SCMI driver, then we have >>>> to manage now the probe defering. >>>> >>>> v1 -> v2: >>>> - fix yamllint warnings. >>> >>> Hi Gabriel, >>> >>> I don't have much clout with the maintainers, but I have to NAK this >>> series >>> after finding major breakage. >>> >>> The problem with series is that it breaks pretty much every board it >>> touches. >>> I have a DK2 here that I'm using for development, which no longer >>> boots with >>> this series applied. >>> >>> The crux of the matter is that this series assumes all boards will >>> boot with an >>> FSBL that implements a very specific SCMI clock tree. This is major ABI >>> breakage for anyone not using TF-A as the first stage bootloader. Anyone >>> using u-boot SPL is screwed. >>> >>> This series imposes a SOC-wide change via the dtsi files. So even >>> boards that >>> you don't intend to convert to SCMI will get broken this way. >>> Adding a -no-scmi file that isn't used anywhere doesn't help things. >> >> You are right. We mainly take care about NO ST (DH/...) boards, but >> not really about current usage >> Of our stm32 boards. Several options exist: > > Since a lot of people benefit from the good upstream support for the MP1 > _and_ keep updating their machines to get the latest fixes, it is very > important to keep the current usage working. > >> 1- Break the current ABI: as soon as those patches are merged, >> stm32mp157c-dk2.dtb will impose to use >> A tf-a for scmi clocks. For people using u-boot spl, the will have to >> create their own "no-secure" devicetree. > > NAK, this breaks existing boards and existing setups, e.g. DK2 that does > not use ATF. > >> 2-As you suggest, create a new "secure" dtb per boards (Not my wish >> for maintenance perspectives). > > I agree with Alex (G) that the "secure" option should be opt-in. > That way existing setups remain working and no extra requirements are > imposed on MP1 users. Esp. since as far as I understand this, the > "secure" part isn't really about security, but rather about moving clock > configuration from Linux to some firmware blob. > >> 3- Keep kernel device tree as they are and applied this secure layer >> (scmi clocks phandle) thanks to dtbo in >> U-boot. > > Is this really better than > #include "stm32mp15xx-enable-secure-stuff.dtsi" > in a board DT ? Because that is how I imagine the opt-in "secure" option > could work. > Discussing with Patrick about u-boot, we could use dtbo application thanks to extlinux.conf. BUT it it will not prevent other case (i.e. TF-A which jump directly in kernel@). So the "least worst" solution is to create a new "stm32mp1257c-scmi-dk2 board which will overload clock entries with a scmi phandle (as proposed by Alex). Gabriel, can you wait a bit before sending something about SCMI in dtsi, I would like to align this strategy internally. Marek, Alex: thanks for your inputs. Regards Alex >> The third could be the less costly. > > [...]
Hello, On 11.03.21 15:02, Alexandre TORGUE wrote: > On 3/11/21 12:43 PM, Marek Vasut wrote: >> On 3/11/21 9:08 AM, Alexandre TORGUE wrote: >>> 1- Break the current ABI: as soon as those patches are merged, stm32mp157c-dk2.dtb will impose to use >>> A tf-a for scmi clocks. For people using u-boot spl, the will have to create their own "no-secure" devicetree. >> >> NAK, this breaks existing boards and existing setups, e.g. DK2 that does not use ATF. >> >>> 2-As you suggest, create a new "secure" dtb per boards (Not my wish for maintenance perspectives). >> >> I agree with Alex (G) that the "secure" option should be opt-in. >> That way existing setups remain working and no extra requirements are imposed on MP1 users. Esp. since as far as I understand this, the "secure" part isn't really about security, but rather about moving clock configuration from Linux to some firmware blob. >> >>> 3- Keep kernel device tree as they are and applied this secure layer (scmi clocks phandle) thanks to dtbo in >>> U-boot. >> >> Is this really better than >> #include "stm32mp15xx-enable-secure-stuff.dtsi" >> in a board DT ? Because that is how I imagine the opt-in "secure" option could work. >> > > Discussing with Patrick about u-boot, we could use dtbo application thanks to extlinux.conf. BUT it it will not prevent other case (i.e. TF-A which jump directly in kernel@). So the "least worst" solution is to create a new "stm32mp1257c-scmi-dk2 board which will overload clock entries with a scmi phandle (as proposed by Alex). I raised this issue before with your colleagues. I still believe the correct way would be for the TF-A to pass down either a device tree or an overlay with the actual settings in use, e.g.: - Clocks/Resets done via SCMI - Reserved memory regions If TF-A directly boots Linux, it can apply the overlay itself, otherwise it's passed down to SSBL that applies it before booting Linux. Cheers, Ahmad > > Gabriel, can you wait a bit before sending something about SCMI in dtsi, I would like to align this strategy internally. > > Marek, Alex: thanks for your inputs. > > Regards > Alex > >>> The third could be the less costly. >> >> [...] > _______________________________________________ > Linux-stm32 mailing list > Linux-stm32@st-md-mailman.stormreply.com > https://st-md-mailman.stormreply.com/mailman/listinfo/linux-stm32
Hi Ahmad On 3/11/21 3:41 PM, Ahmad Fatoum wrote: > Hello, > > On 11.03.21 15:02, Alexandre TORGUE wrote: >> On 3/11/21 12:43 PM, Marek Vasut wrote: >>> On 3/11/21 9:08 AM, Alexandre TORGUE wrote: >>>> 1- Break the current ABI: as soon as those patches are merged, stm32mp157c-dk2.dtb will impose to use >>>> A tf-a for scmi clocks. For people using u-boot spl, the will have to create their own "no-secure" devicetree. >>> >>> NAK, this breaks existing boards and existing setups, e.g. DK2 that does not use ATF. >>> >>>> 2-As you suggest, create a new "secure" dtb per boards (Not my wish for maintenance perspectives). >>> >>> I agree with Alex (G) that the "secure" option should be opt-in. >>> That way existing setups remain working and no extra requirements are imposed on MP1 users. Esp. since as far as I understand this, the "secure" part isn't really about security, but rather about moving clock configuration from Linux to some firmware blob. >>> >>>> 3- Keep kernel device tree as they are and applied this secure layer (scmi clocks phandle) thanks to dtbo in >>>> U-boot. >>> >>> Is this really better than >>> #include "stm32mp15xx-enable-secure-stuff.dtsi" >>> in a board DT ? Because that is how I imagine the opt-in "secure" option could work. >>> >> >> Discussing with Patrick about u-boot, we could use dtbo application thanks to extlinux.conf. BUT it it will not prevent other case (i.e. TF-A which jump directly in kernel@). So the "least worst" solution is to create a new "stm32mp1257c-scmi-dk2 board which will overload clock entries with a scmi phandle (as proposed by Alex). > > I raised this issue before with your colleagues. I still believe the correct way > would be for the TF-A to pass down either a device tree or an overlay with the > actual settings in use, e.g.: > > - Clocks/Resets done via SCMI > - Reserved memory regions > > If TF-A directly boots Linux, it can apply the overlay itself, otherwise it's > passed down to SSBL that applies it before booting Linux. Discussing with tf-a and u-boot guys, this solution could imply hard synchronization between tf-a/u-boot. The most simple remains a "secure" dts. regards Alex > Cheers, > Ahmad > >> >> Gabriel, can you wait a bit before sending something about SCMI in dtsi, I would like to align this strategy internally. >> >> Marek, Alex: thanks for your inputs. >> >> Regards >> Alex >> >>>> The third could be the less costly. >>> >>> [...] >> _______________________________________________ >> Linux-stm32 mailing list >> Linux-stm32@st-md-mailman.stormreply.com >> https://st-md-mailman.stormreply.com/mailman/listinfo/linux-stm32 >
Hello, On 11.03.21 16:18, Alexandre TORGUE wrote: >> I raised this issue before with your colleagues. I still believe the correct way >> would be for the TF-A to pass down either a device tree or an overlay with the >> actual settings in use, e.g.: >> >> - Clocks/Resets done via SCMI >> - Reserved memory regions >> >> If TF-A directly boots Linux, it can apply the overlay itself, otherwise it's >> passed down to SSBL that applies it before booting Linux. > Discussing with tf-a and u-boot guys, this solution could imply hard synchronization between tf-a/u-boot. The most simple remains a "secure" dts. OP-TEE can be configured via CFG_EXTERNAL_DTB_OVERLAY to pass along an overlay that describes the reserved memory regions it uses. A similiar approach could work here. The only synchronization you need in Linux is to keep phandles that the overlay can reference. Cheers, Ahmad > > regards > Alex > >> Cheers, >> Ahmad >> >>> >>> Gabriel, can you wait a bit before sending something about SCMI in dtsi, I would like to align this strategy internally. >>> >>> Marek, Alex: thanks for your inputs. >>> >>> Regards >>> Alex >>> >>>>> The third could be the less costly. >>>> >>>> [...] >>> _______________________________________________ >>> Linux-stm32 mailing list >>> Linux-stm32@st-md-mailman.stormreply.com >>> https://st-md-mailman.stormreply.com/mailman/listinfo/linux-stm32 >> >
On 3/11/21 3:41 PM, Ahmad Fatoum wrote: > Hello, Hi, > On 11.03.21 15:02, Alexandre TORGUE wrote: >> On 3/11/21 12:43 PM, Marek Vasut wrote: >>> On 3/11/21 9:08 AM, Alexandre TORGUE wrote: >>>> 1- Break the current ABI: as soon as those patches are merged, stm32mp157c-dk2.dtb will impose to use >>>> A tf-a for scmi clocks. For people using u-boot spl, the will have to create their own "no-secure" devicetree. >>> >>> NAK, this breaks existing boards and existing setups, e.g. DK2 that does not use ATF. >>> >>>> 2-As you suggest, create a new "secure" dtb per boards (Not my wish for maintenance perspectives). >>> >>> I agree with Alex (G) that the "secure" option should be opt-in. >>> That way existing setups remain working and no extra requirements are imposed on MP1 users. Esp. since as far as I understand this, the "secure" part isn't really about security, but rather about moving clock configuration from Linux to some firmware blob. >>> >>>> 3- Keep kernel device tree as they are and applied this secure layer (scmi clocks phandle) thanks to dtbo in >>>> U-boot. >>> >>> Is this really better than >>> #include "stm32mp15xx-enable-secure-stuff.dtsi" >>> in a board DT ? Because that is how I imagine the opt-in "secure" option could work. >>> >> >> Discussing with Patrick about u-boot, we could use dtbo application thanks to extlinux.conf. BUT it it will not prevent other case (i.e. TF-A which jump directly in kernel@). So the "least worst" solution is to create a new "stm32mp1257c-scmi-dk2 board which will overload clock entries with a scmi phandle (as proposed by Alex). > > I raised this issue before with your colleagues. I still believe the correct way > would be for the TF-A to pass down either a device tree or an overlay with the > actual settings in use, e.g.: > > - Clocks/Resets done via SCMI > - Reserved memory regions > > If TF-A directly boots Linux, it can apply the overlay itself, otherwise it's > passed down to SSBL that applies it before booting Linux. That sounds good and it is something e.g. R-Car already does, it merges DT fragment from prior stages at U-Boot level and then passes the result to Linux. So on ST hardware, the same could very well happen and it would work for both non-ATF / ATF / ATF+TEE options.
Hi Guys On 3/11/21 5:11 PM, Marek Vasut wrote: > On 3/11/21 3:41 PM, Ahmad Fatoum wrote: >> Hello, > > Hi, > >> On 11.03.21 15:02, Alexandre TORGUE wrote: >>> On 3/11/21 12:43 PM, Marek Vasut wrote: >>>> On 3/11/21 9:08 AM, Alexandre TORGUE wrote: >>>>> 1- Break the current ABI: as soon as those patches are merged, >>>>> stm32mp157c-dk2.dtb will impose to use >>>>> A tf-a for scmi clocks. For people using u-boot spl, the will have >>>>> to create their own "no-secure" devicetree. >>>> >>>> NAK, this breaks existing boards and existing setups, e.g. DK2 that >>>> does not use ATF. >>>> >>>>> 2-As you suggest, create a new "secure" dtb per boards (Not my wish >>>>> for maintenance perspectives). >>>> >>>> I agree with Alex (G) that the "secure" option should be opt-in. >>>> That way existing setups remain working and no extra requirements >>>> are imposed on MP1 users. Esp. since as far as I understand this, >>>> the "secure" part isn't really about security, but rather about >>>> moving clock configuration from Linux to some firmware blob. >>>> >>>>> 3- Keep kernel device tree as they are and applied this secure >>>>> layer (scmi clocks phandle) thanks to dtbo in >>>>> U-boot. >>>> >>>> Is this really better than >>>> #include "stm32mp15xx-enable-secure-stuff.dtsi" >>>> in a board DT ? Because that is how I imagine the opt-in "secure" >>>> option could work. >>>> >>> >>> Discussing with Patrick about u-boot, we could use dtbo application >>> thanks to extlinux.conf. BUT it it will not prevent other case (i.e. >>> TF-A which jump directly in kernel@). So the "least worst" solution >>> is to create a new "stm32mp1257c-scmi-dk2 board which will overload >>> clock entries with a scmi phandle (as proposed by Alex). >> >> I raised this issue before with your colleagues. I still believe the >> correct way >> would be for the TF-A to pass down either a device tree or an overlay >> with the >> actual settings in use, e.g.: >> >> - Clocks/Resets done via SCMI >> - Reserved memory regions >> >> If TF-A directly boots Linux, it can apply the overlay itself, >> otherwise it's >> passed down to SSBL that applies it before booting Linux. > > That sounds good and it is something e.g. R-Car already does, it merges > DT fragment from prior stages at U-Boot level and then passes the result > to Linux. > > So on ST hardware, the same could very well happen and it would work for > both non-ATF / ATF / ATF+TEE options. Even this solution sounds good but we are currently not able to do it in our TF-A/u-boot so not feasible for the moment. So we have to find a solution for now. Create a new dtb can be this solution. Our internal strategy is to use scmi on our official ST board. It will be a really drawback to include a "no-scmi.dtsi" in DH boards (for example) and to create a stm32mp157c-noscmi-dk2.dts ?
On 3/11/21 12:10 PM, Alexandre TORGUE wrote: > Hi Guys > > On 3/11/21 5:11 PM, Marek Vasut wrote: >> On 3/11/21 3:41 PM, Ahmad Fatoum wrote: >>> Hello, >> >> Hi, >> >>> On 11.03.21 15:02, Alexandre TORGUE wrote: >>>> On 3/11/21 12:43 PM, Marek Vasut wrote: >>>>> On 3/11/21 9:08 AM, Alexandre TORGUE wrote: >>>>>> 1- Break the current ABI: as soon as those patches are merged, >>>>>> stm32mp157c-dk2.dtb will impose to use >>>>>> A tf-a for scmi clocks. For people using u-boot spl, the will have >>>>>> to create their own "no-secure" devicetree. >>>>> >>>>> NAK, this breaks existing boards and existing setups, e.g. DK2 that >>>>> does not use ATF. >>>>> >>>>>> 2-As you suggest, create a new "secure" dtb per boards (Not my >>>>>> wish for maintenance perspectives). >>>>> >>>>> I agree with Alex (G) that the "secure" option should be opt-in. >>>>> That way existing setups remain working and no extra requirements >>>>> are imposed on MP1 users. Esp. since as far as I understand this, >>>>> the "secure" part isn't really about security, but rather about >>>>> moving clock configuration from Linux to some firmware blob. >>>>> >>>>>> 3- Keep kernel device tree as they are and applied this secure >>>>>> layer (scmi clocks phandle) thanks to dtbo in >>>>>> U-boot. >>>>> >>>>> Is this really better than >>>>> #include "stm32mp15xx-enable-secure-stuff.dtsi" >>>>> in a board DT ? Because that is how I imagine the opt-in "secure" >>>>> option could work. >>>>> >>>> >>>> Discussing with Patrick about u-boot, we could use dtbo application >>>> thanks to extlinux.conf. BUT it it will not prevent other case (i.e. >>>> TF-A which jump directly in kernel@). So the "least worst" solution >>>> is to create a new "stm32mp1257c-scmi-dk2 board which will overload >>>> clock entries with a scmi phandle (as proposed by Alex). >>> >>> I raised this issue before with your colleagues. I still believe the >>> correct way >>> would be for the TF-A to pass down either a device tree or an overlay >>> with the >>> actual settings in use, e.g.: >>> >>> - Clocks/Resets done via SCMI >>> - Reserved memory regions >>> >>> If TF-A directly boots Linux, it can apply the overlay itself, >>> otherwise it's >>> passed down to SSBL that applies it before booting Linux. >> >> That sounds good and it is something e.g. R-Car already does, it >> merges DT fragment from prior stages at U-Boot level and then passes >> the result to Linux. >> >> So on ST hardware, the same could very well happen and it would work >> for both non-ATF / ATF / ATF+TEE options. > > Even this solution sounds good but we are currently not able to do it in > our TF-A/u-boot so not feasible for the moment. So we have to find a > solution for now. Create a new dtb can be this solution. Our internal > strategy is to use scmi on our official ST board. It will be a really > drawback to include a "no-scmi.dtsi" in DH boards (for example) and to > create a stm32mp157c-noscmi-dk2.dts ? It could work, as long as all users are reminded to change their build scripts to pick up a "-noscmi.dtb". I suspect that if this were the case we'll see quite a few bug reports saying "stm32mp1 no longer boots with kernel v5.13". I didn't think of this originally, though u-boot already does the DTB patching for OPTEE reserved memory regions. It's not too hard to also patch in the SCMI clocks at boot. In u-boot's case, runtime detection might even be feasible. Alex
On 3/11/21 7:10 PM, Alexandre TORGUE wrote: > Hi Guys > > On 3/11/21 5:11 PM, Marek Vasut wrote: >> On 3/11/21 3:41 PM, Ahmad Fatoum wrote: >>> Hello, >> >> Hi, >> >>> On 11.03.21 15:02, Alexandre TORGUE wrote: >>>> On 3/11/21 12:43 PM, Marek Vasut wrote: >>>>> On 3/11/21 9:08 AM, Alexandre TORGUE wrote: >>>>>> 1- Break the current ABI: as soon as those patches are merged, >>>>>> stm32mp157c-dk2.dtb will impose to use >>>>>> A tf-a for scmi clocks. For people using u-boot spl, the will have >>>>>> to create their own "no-secure" devicetree. >>>>> >>>>> NAK, this breaks existing boards and existing setups, e.g. DK2 that >>>>> does not use ATF. >>>>> >>>>>> 2-As you suggest, create a new "secure" dtb per boards (Not my >>>>>> wish for maintenance perspectives). >>>>> >>>>> I agree with Alex (G) that the "secure" option should be opt-in. >>>>> That way existing setups remain working and no extra requirements >>>>> are imposed on MP1 users. Esp. since as far as I understand this, >>>>> the "secure" part isn't really about security, but rather about >>>>> moving clock configuration from Linux to some firmware blob. >>>>> >>>>>> 3- Keep kernel device tree as they are and applied this secure >>>>>> layer (scmi clocks phandle) thanks to dtbo in >>>>>> U-boot. >>>>> >>>>> Is this really better than >>>>> #include "stm32mp15xx-enable-secure-stuff.dtsi" >>>>> in a board DT ? Because that is how I imagine the opt-in "secure" >>>>> option could work. >>>>> >>>> >>>> Discussing with Patrick about u-boot, we could use dtbo application >>>> thanks to extlinux.conf. BUT it it will not prevent other case (i.e. >>>> TF-A which jump directly in kernel@). So the "least worst" solution >>>> is to create a new "stm32mp1257c-scmi-dk2 board which will overload >>>> clock entries with a scmi phandle (as proposed by Alex). >>> >>> I raised this issue before with your colleagues. I still believe the >>> correct way >>> would be for the TF-A to pass down either a device tree or an overlay >>> with the >>> actual settings in use, e.g.: >>> >>> - Clocks/Resets done via SCMI >>> - Reserved memory regions >>> >>> If TF-A directly boots Linux, it can apply the overlay itself, >>> otherwise it's >>> passed down to SSBL that applies it before booting Linux. >> >> That sounds good and it is something e.g. R-Car already does, it >> merges DT fragment from prior stages at U-Boot level and then passes >> the result to Linux. >> >> So on ST hardware, the same could very well happen and it would work >> for both non-ATF / ATF / ATF+TEE options. > > Even this solution sounds good but we are currently not able to do it in > our TF-A/u-boot so not feasible for the moment. Why not ? U-Boot is perfectly capable of merging prior stage DT and DT loaded from elsewhere. See R-Car3 for example. > So we have to find a > solution for now. Create a new dtb can be this solution. Our internal > strategy is to use scmi on our official ST board. It will be a really > drawback to include a "no-scmi.dtsi" in DH boards (for example) and to > create a stm32mp157c-noscmi-dk2.dts ? I would highly prefer the SCMI to be opt-in, not opt-out. But still, isn't it possible to auto-detect the board configuration in Linux and pick the clock enumeration based on that automatically ?
Hi Marek On 3/11/21 9:09 PM, Marek Vasut wrote: > On 3/11/21 7:10 PM, Alexandre TORGUE wrote: >> Hi Guys >> >> On 3/11/21 5:11 PM, Marek Vasut wrote: >>> On 3/11/21 3:41 PM, Ahmad Fatoum wrote: >>>> Hello, >>> >>> Hi, >>> >>>> On 11.03.21 15:02, Alexandre TORGUE wrote: >>>>> On 3/11/21 12:43 PM, Marek Vasut wrote: >>>>>> On 3/11/21 9:08 AM, Alexandre TORGUE wrote: >>>>>>> 1- Break the current ABI: as soon as those patches are merged, >>>>>>> stm32mp157c-dk2.dtb will impose to use >>>>>>> A tf-a for scmi clocks. For people using u-boot spl, the will >>>>>>> have to create their own "no-secure" devicetree. >>>>>> >>>>>> NAK, this breaks existing boards and existing setups, e.g. DK2 >>>>>> that does not use ATF. >>>>>> >>>>>>> 2-As you suggest, create a new "secure" dtb per boards (Not my >>>>>>> wish for maintenance perspectives). >>>>>> >>>>>> I agree with Alex (G) that the "secure" option should be opt-in. >>>>>> That way existing setups remain working and no extra requirements >>>>>> are imposed on MP1 users. Esp. since as far as I understand this, >>>>>> the "secure" part isn't really about security, but rather about >>>>>> moving clock configuration from Linux to some firmware blob. >>>>>> >>>>>>> 3- Keep kernel device tree as they are and applied this secure >>>>>>> layer (scmi clocks phandle) thanks to dtbo in >>>>>>> U-boot. >>>>>> >>>>>> Is this really better than >>>>>> #include "stm32mp15xx-enable-secure-stuff.dtsi" >>>>>> in a board DT ? Because that is how I imagine the opt-in "secure" >>>>>> option could work. >>>>>> >>>>> >>>>> Discussing with Patrick about u-boot, we could use dtbo application >>>>> thanks to extlinux.conf. BUT it it will not prevent other case >>>>> (i.e. TF-A which jump directly in kernel@). So the "least worst" >>>>> solution is to create a new "stm32mp1257c-scmi-dk2 board which will >>>>> overload clock entries with a scmi phandle (as proposed by Alex). >>>> >>>> I raised this issue before with your colleagues. I still believe the >>>> correct way >>>> would be for the TF-A to pass down either a device tree or an >>>> overlay with the >>>> actual settings in use, e.g.: >>>> >>>> - Clocks/Resets done via SCMI >>>> - Reserved memory regions >>>> >>>> If TF-A directly boots Linux, it can apply the overlay itself, >>>> otherwise it's >>>> passed down to SSBL that applies it before booting Linux. >>> >>> That sounds good and it is something e.g. R-Car already does, it >>> merges DT fragment from prior stages at U-Boot level and then passes >>> the result to Linux. >>> >>> So on ST hardware, the same could very well happen and it would work >>> for both non-ATF / ATF / ATF+TEE options. >> >> Even this solution sounds good but we are currently not able to do it >> in our TF-A/u-boot so not feasible for the moment. > > Why not ? U-Boot is perfectly capable of merging prior stage DT and DT > loaded from elsewhere. See R-Car3 for example. Ok, We will check what it is possible to do by this way before taking a decision. Thanks Alex > >> So we have to find a solution for now. Create a new dtb can be this >> solution. Our internal strategy is to use scmi on our official ST >> board. It will be a really drawback to include a "no-scmi.dtsi" in DH >> boards (for example) and to create a stm32mp157c-noscmi-dk2.dts ? > > I would highly prefer the SCMI to be opt-in, not opt-out. > > But still, isn't it possible to auto-detect the board configuration in > Linux and pick the clock enumeration based on that automatically ?
From: Gabriel Fernandez <gabriel.fernandez@foss.st.com> Platform STM32MP1 can be used in configuration where some clocks and IP resets can relate as secure resources. These resources are moved from a RCC clock/reset handle to a SCMI clock/reset_domain handle. The RCC clock driver is now dependent of the SCMI driver, then we have to manage now the probe defering. v1 -> v2: - fix yamllint warnings. Gabriel Fernandez (14): clk: stm32mp1: merge 'clk-hsi-div' and 'ck_hsi' into one clock clk: stm32mp1: merge 'ck_hse_rtc' and 'ck_rtc' into one clock clk: stm32mp1: remove intermediate pll clocks clk: stm32mp1: convert to module driver clk: stm32mp1: move RCC reset controller into RCC clock driver reset: stm32mp1: remove stm32mp1 reset dt-bindings: clock: add IDs for SCMI clocks on stm32mp15 dt-bindings: reset: add IDs for SCMI reset domains on stm32mp15 dt-bindings: reset: add MCU HOLD BOOT ID for SCMI reset domains on stm32mp15 clk: stm32mp1: new compatible for secure RCC support ARM: dts: stm32: define SCMI resources on stm32mp15 ARM: dts: stm32: move clocks/resets to SCMI resources for stm32mp15 dt-bindings: clock: stm32mp1 new compatible for secure rcc ARM: dts: stm32: introduce basic boot include on stm32mp15x board .../bindings/clock/st,stm32mp1-rcc.yaml | 6 +- arch/arm/boot/dts/stm32mp15-no-scmi.dtsi | 158 ++++++ arch/arm/boot/dts/stm32mp151.dtsi | 127 +++-- arch/arm/boot/dts/stm32mp153.dtsi | 4 +- arch/arm/boot/dts/stm32mp157.dtsi | 2 +- arch/arm/boot/dts/stm32mp15xc.dtsi | 4 +- drivers/clk/Kconfig | 10 + drivers/clk/clk-stm32mp1.c | 495 +++++++++++++++--- drivers/reset/Kconfig | 6 - drivers/reset/Makefile | 1 - drivers/reset/reset-stm32mp1.c | 115 ---- include/dt-bindings/clock/stm32mp1-clks.h | 27 + include/dt-bindings/reset/stm32mp1-resets.h | 15 + 13 files changed, 704 insertions(+), 266 deletions(-) create mode 100644 arch/arm/boot/dts/stm32mp15-no-scmi.dtsi delete mode 100644 drivers/reset/reset-stm32mp1.c