On 30.08.22 10:29, Marco Felsch wrote: > On 22-08-30, Ahmad Fatoum wrote: >> Hello Marco, >> >> On 30.08.22 10:10, Marco Felsch wrote: >>> Hi Ahamd, >>> >>> On 22-08-30, Ahmad Fatoum wrote: >>>> Now with i.MX8M feature controller driver support available, have the >>>> OCOTP provide feature control on the i.MX8MM to ensure the kernel DT >>>> does not cause Linux to access the VPU and its power domains, >>>> when barebox knows them to be unavailable. >>>> >>>> This is needed because the upstream kernel imx8mm.dtsi only >>>> describes the full-featured SoC, which can lead to hangs when >>>> instantiating drivers for hardware that's unavailable in a >>>> less-featureful variant of the SoC. >>>> >>>> Signed-off-by: Ahmad Fatoum <a.fatoum@xxxxxxxxxxxxxx> >>>> --- >>>> v1 was RFC patch 10/10 of: >>>> https://lore.barebox.org/barebox/20220818051955.2088238-11-a.fatoum@xxxxxxxxxxxxxx/T/#u >>>> >>>> Patches 01-08 are still applicable, this replaces the approach in v1 >>>> with a standalone feature controller with having the OCOTP as feature >>>> controller, like is done for i.MX8MN in patch 08/10 of above referenced >>>> series. >>>> --- >>>> arch/arm/dts/imx8mm.dtsi | 52 ++++++++++++++++++++++++++++++++++++++++ >>>> 1 file changed, 52 insertions(+) >>>> >>>> diff --git a/arch/arm/dts/imx8mm.dtsi b/arch/arm/dts/imx8mm.dtsi >>>> index cdf212820594..1e81d03d6b84 100644 >>>> --- a/arch/arm/dts/imx8mm.dtsi >>>> +++ b/arch/arm/dts/imx8mm.dtsi >>>> @@ -1,10 +1,18 @@ >>>> // SPDX-License-Identifier: (GPL-2.0+ OR MIT) >>>> + >>>> +#include <dt-bindings/features/imx8m.h> >>>> + >>>> / { >>>> aliases { >>>> gpr.reboot_mode = &reboot_mode_gpr; >>>> }; >>>> }; >>>> >>>> +feat: &ocotp { >>>> + #feature-cells = <1>; >>>> + barebox,feature-controller; >>>> +}; >>> >>> Why not just appending the node like: >>> >>> / { >>> aliases { >>> gpr.reboot_mode = &reboot_mode_gpr; >>> }; >>> >>> feat: ocotp { >>> #feature-cells = <1>; >>> barebox,feature-controller; >>> }; >>> }; >> >> Yours adds a new /ocotp node while my patch gives the existing >> node pointed at by &ocotp an additional label and extends it. > > This should extend the ocotp node as well or would it be a new node > due to the new label? To me it locked very strange, therefore I asked. I > never noticed that: "new_label: &old_label {}" is even possible. It would be a new node, because the ocotp isn't at top level, but instead at /soc@0/bus@30000000/efuse@30350000. Instead of using the full path, I used the &ocotp label and instead of using &ocotp everywhere, I add an additional &feat alias to better convey that the ocotp acts as a feature controller. I intend to upstream this and will likely just use the ocotp label directly then, but having the &feat label here for now allows easily trying out other providers as mentioned in my previous mail. Cheers, Ahmad > > Regards, > Marco > >> I prefer the additional label, because it gives us flexibility >> in future if upstream decides that there should be a dedicated >> feature controller. In that case we would only need to move >> the label instead of touching all references. see RFC patch 10/10 >> referenced above. >> >> Cheers, >> Ahmad >> >>> >>> Regards, >>> Marco >>> >>>> + >>>> &pgc_otg1 { >>>> barebox,allow-dummy; >>>> }; >>>> @@ -24,3 +32,47 @@ >>>> mode-serial = <0x00000010>, <0x40000000>; >>>> }; >>>> }; >>>> + >>>> +&A53_1 { >>>> + barebox,feature-gates = <&feat IMX8M_FEAT_CPU_DUAL>; >>>> +}; >>>> + >>>> +&A53_2 { >>>> + barebox,feature-gates = <&feat IMX8M_FEAT_CPU_QUAD>; >>>> +}; >>>> + >>>> +&A53_3 { >>>> + barebox,feature-gates = <&feat IMX8M_FEAT_CPU_QUAD>; >>>> +}; >>>> + >>>> +&gpc { >>>> + barebox,feature-gates = <&feat 0>; >>>> +}; >>>> + >>>> +&vpu_g1 { >>>> + barebox,feature-gates = <&feat IMX8M_FEAT_VPU>; >>>> +}; >>>> + >>>> +&vpu_g2 { >>>> + barebox,feature-gates = <&feat IMX8M_FEAT_VPU>; >>>> +}; >>>> + >>>> +&vpu_blk_ctrl { >>>> + barebox,feature-gates = <&feat IMX8M_FEAT_VPU>; >>>> +}; >>>> + >>>> +&pgc_vpumix { >>>> + barebox,feature-gates = <&feat IMX8M_FEAT_VPU>; >>>> +}; >>>> + >>>> +&pgc_vpu_g1 { >>>> + barebox,feature-gates = <&feat IMX8M_FEAT_VPU>; >>>> +}; >>>> + >>>> +&pgc_vpu_g2 { >>>> + barebox,feature-gates = <&feat IMX8M_FEAT_VPU>; >>>> +}; >>>> + >>>> +&pgc_vpu_h1 { >>>> + barebox,feature-gates = <&feat IMX8M_FEAT_VPU>; >>>> +}; >>>> -- >>>> 2.30.2 >>>> >>>> >>>> >>> >> >> >> -- >> Pengutronix e.K. | | >> Steuerwalder Str. 21 | http://www.pengutronix.de/ | >> 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | >> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | >> > -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
