| Message ID | 20240326114743.712167-1-andre.przywara@arm.com |
|---|---|
| Headers | show |
| Series | cpufreq: sun50i: Add Allwinner H616 support | expand |
On Tue, Mar 26, 2024 at 11:47:36AM +0000, Andre Przywara wrote: > From: Martin Botka <martin.botka@somainline.org> > > The "SoC ID revision" as provided via the SMCCC SOCID interface can be > valuable information for drivers, when certain functionality depends > on a die revision, for instance. > One example is the sun50i-cpufreq-nvmem driver, which needs this > information to determine the speed bin of the SoC. > > Export the arm_smccc_get_soc_id_revision() function so that it can be > called by any driver. > Assuming sun50i cpufreq driver can be built as module, Acked-by: Sudeep Holla <sudeep.holla@arm.com>
Hi Andre, On 3/26/24 06:47, Andre Przywara wrote: > From: Martin Botka <martin.botka@somainline.org> > > The Allwinner H616/H618 SoCs have different OPP tables per SoC version > and die revision. The SoC version is stored in NVMEM, as before, though > encoded differently. The die revision is in a different register, in the > SRAM controller. Firmware already exports that value in a standardised > way, through the SMCCC SoCID mechanism. We need both values, as some chips > have the same SoC version, but they don't support the same frequencies and > they get differentiated by the die revision. > > Add the new compatible string and tie the new translation function to > it. This mechanism not only covers the original H616 SoC, but also its > very close sibling SoCs H618 and H700, so add them to the list as well. > > Signed-off-by: Martin Botka <martin.botka@somainline.org> > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- > drivers/cpufreq/sun50i-cpufreq-nvmem.c | 61 ++++++++++++++++++++++++++ > 1 file changed, 61 insertions(+) > > diff --git a/drivers/cpufreq/sun50i-cpufreq-nvmem.c b/drivers/cpufreq/sun50i-cpufreq-nvmem.c > index bd170611c7906..f9e9fc340f848 100644 > --- a/drivers/cpufreq/sun50i-cpufreq-nvmem.c > +++ b/drivers/cpufreq/sun50i-cpufreq-nvmem.c > @@ -10,6 +10,7 @@ > > #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > > +#include <linux/arm-smccc.h> > #include <linux/cpu.h> > #include <linux/module.h> > #include <linux/nvmem-consumer.h> > @@ -46,14 +47,71 @@ static u32 sun50i_h6_efuse_xlate(u32 speedbin) > return 0; > } > > +/* > + * Judging by the OPP tables in the vendor BSP, the quality order of the > + * returned speedbin index is 4 -> 0/2 -> 3 -> 1, from worst to best. > + * 0 and 2 seem identical from the OPP tables' point of view. > + */ > +static u32 sun50i_h616_efuse_xlate(u32 speedbin) > +{ > + int ver_bits = arm_smccc_get_soc_id_revision(); This needs a Kconfig dependency on ARM_SMCCC_SOC_ID. Regards, Samuel > + u32 value = 0; > + > + switch (speedbin & 0xffff) { > + case 0x2000: > + value = 0; > + break; > + case 0x2400: > + case 0x7400: > + case 0x2c00: > + case 0x7c00: > + if (ver_bits != SMCCC_RET_NOT_SUPPORTED && ver_bits <= 1) { > + /* ic version A/B */ > + value = 1; > + } else { > + /* ic version C and later version */ > + value = 2; > + } > + break; > + case 0x5000: > + case 0x5400: > + case 0x6000: > + value = 3; > + break; > + case 0x5c00: > + value = 4; > + break; > + case 0x5d00: > + value = 0; > + break; > + case 0x6c00: > + value = 5; > + break; > + default: > + pr_warn("sun50i-cpufreq-nvmem: unknown speed bin 0x%x, using default bin 0\n", > + speedbin & 0xffff); > + value = 0; > + break; > + } > + > + return value; > +} > + > static struct sunxi_cpufreq_data sun50i_h6_cpufreq_data = { > .efuse_xlate = sun50i_h6_efuse_xlate, > }; > > +static struct sunxi_cpufreq_data sun50i_h616_cpufreq_data = { > + .efuse_xlate = sun50i_h616_efuse_xlate, > +}; > + > static const struct of_device_id cpu_opp_match_list[] = { > { .compatible = "allwinner,sun50i-h6-operating-points", > .data = &sun50i_h6_cpufreq_data, > }, > + { .compatible = "allwinner,sun50i-h616-operating-points", > + .data = &sun50i_h616_cpufreq_data, > + }, > {} > }; > > @@ -230,6 +288,9 @@ static struct platform_driver sun50i_cpufreq_driver = { > > static const struct of_device_id sun50i_cpufreq_match_list[] = { > { .compatible = "allwinner,sun50i-h6" }, > + { .compatible = "allwinner,sun50i-h616" }, > + { .compatible = "allwinner,sun50i-h618" }, > + { .compatible = "allwinner,sun50i-h700" }, > {} > }; > MODULE_DEVICE_TABLE(of, sun50i_cpufreq_match_list);
On Tue, 26 Mar 2024 22:46:27 -0500 Samuel Holland <samuel@sholland.org> wrote: Hi Samuel, > On 3/26/24 06:47, Andre Przywara wrote: > > From: Martin Botka <martin.botka@somainline.org> > > > > The Allwinner H616/H618 SoCs have different OPP tables per SoC version > > and die revision. The SoC version is stored in NVMEM, as before, though > > encoded differently. The die revision is in a different register, in the > > SRAM controller. Firmware already exports that value in a standardised > > way, through the SMCCC SoCID mechanism. We need both values, as some chips > > have the same SoC version, but they don't support the same frequencies and > > they get differentiated by the die revision. > > > > Add the new compatible string and tie the new translation function to > > it. This mechanism not only covers the original H616 SoC, but also its > > very close sibling SoCs H618 and H700, so add them to the list as well. > > > > Signed-off-by: Martin Botka <martin.botka@somainline.org> > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > > --- > > drivers/cpufreq/sun50i-cpufreq-nvmem.c | 61 ++++++++++++++++++++++++++ > > 1 file changed, 61 insertions(+) > > > > diff --git a/drivers/cpufreq/sun50i-cpufreq-nvmem.c b/drivers/cpufreq/sun50i-cpufreq-nvmem.c > > index bd170611c7906..f9e9fc340f848 100644 > > --- a/drivers/cpufreq/sun50i-cpufreq-nvmem.c > > +++ b/drivers/cpufreq/sun50i-cpufreq-nvmem.c > > @@ -10,6 +10,7 @@ > > > > #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > > > > +#include <linux/arm-smccc.h> > > #include <linux/cpu.h> > > #include <linux/module.h> > > #include <linux/nvmem-consumer.h> > > @@ -46,14 +47,71 @@ static u32 sun50i_h6_efuse_xlate(u32 speedbin) > > return 0; > > } > > > > +/* > > + * Judging by the OPP tables in the vendor BSP, the quality order of the > > + * returned speedbin index is 4 -> 0/2 -> 3 -> 1, from worst to best. > > + * 0 and 2 seem identical from the OPP tables' point of view. > > + */ > > +static u32 sun50i_h616_efuse_xlate(u32 speedbin) > > +{ > > + int ver_bits = arm_smccc_get_soc_id_revision(); > > This needs a Kconfig dependency on ARM_SMCCC_SOC_ID. That was my first impulse as well, but it's actually not true: ARM_SMCCC_SOC_ID just protects the sysfs export code, not this function here. That does just depend on HAVE_ARM_SMCCC_DISCOVERY, which gets selected by ARM_GIC_V3, which gets selected by CONFIG_ARM64. So the arm64 kernel is safe. Now apart from ARM(32) (where the situation seems a bit more complex) I just realise that this would torpedo Brandon's D1 efforts, so I need to add this change I played with to provide an alternative: static int get_soc_id_revision(void) { #ifdef CONFIG_HAVE_ARM_SMCCC_DISCOVERY return arm_smccc_get_soc_id_revision(); #else /* Return the value for the worse die revision, to be safe. */ return 2; #endif } Does that look acceptable, despite the #ifdef? Cheers, Andre > > Regards, > Samuel > > > + u32 value = 0; > > + > > + switch (speedbin & 0xffff) { > > + case 0x2000: > > + value = 0; > > + break; > > + case 0x2400: > > + case 0x7400: > > + case 0x2c00: > > + case 0x7c00: > > + if (ver_bits != SMCCC_RET_NOT_SUPPORTED && > > ver_bits <= 1) { > > + /* ic version A/B */ > > + value = 1; > > + } else { > > + /* ic version C and later version */ > > + value = 2; > > + } > > + break; > > + case 0x5000: > > + case 0x5400: > > + case 0x6000: > > + value = 3; > > + break; > > + case 0x5c00: > > + value = 4; > > + break; > > + case 0x5d00: > > + value = 0; > > + break; > > + case 0x6c00: > > + value = 5; > > + break; > > + default: > > + pr_warn("sun50i-cpufreq-nvmem: unknown speed bin > > 0x%x, using default bin 0\n", > > + speedbin & 0xffff); > > + value = 0; > > + break; > > + } > > + > > + return value; > > +} > > + > > static struct sunxi_cpufreq_data sun50i_h6_cpufreq_data = { > > .efuse_xlate = sun50i_h6_efuse_xlate, > > }; > > > > +static struct sunxi_cpufreq_data sun50i_h616_cpufreq_data = { > > + .efuse_xlate = sun50i_h616_efuse_xlate, > > +}; > > + > > static const struct of_device_id cpu_opp_match_list[] = { > > { .compatible = "allwinner,sun50i-h6-operating-points", > > .data = &sun50i_h6_cpufreq_data, > > }, > > + { .compatible = "allwinner,sun50i-h616-operating-points", > > + .data = &sun50i_h616_cpufreq_data, > > + }, > > {} > > }; > > > > @@ -230,6 +288,9 @@ static struct platform_driver > > sun50i_cpufreq_driver = { > > static const struct of_device_id sun50i_cpufreq_match_list[] = { > > { .compatible = "allwinner,sun50i-h6" }, > > + { .compatible = "allwinner,sun50i-h616" }, > > + { .compatible = "allwinner,sun50i-h618" }, > > + { .compatible = "allwinner,sun50i-h700" }, > > {} > > }; > > MODULE_DEVICE_TABLE(of, sun50i_cpufreq_match_list); > >
On Wed, Mar 27, 2024 at 11:46:08AM +0000, Andre Przywara wrote: > On Tue, 26 Mar 2024 22:46:27 -0500 > Samuel Holland <samuel@sholland.org> wrote: > > Hi Samuel, > > > On 3/26/24 06:47, Andre Przywara wrote: > > > From: Martin Botka <martin.botka@somainline.org> > > > > > > The Allwinner H616/H618 SoCs have different OPP tables per SoC version > > > and die revision. The SoC version is stored in NVMEM, as before, though > > > encoded differently. The die revision is in a different register, in the > > > SRAM controller. Firmware already exports that value in a standardised > > > way, through the SMCCC SoCID mechanism. We need both values, as some chips > > > have the same SoC version, but they don't support the same frequencies and > > > they get differentiated by the die revision. > > > > > > Add the new compatible string and tie the new translation function to > > > it. This mechanism not only covers the original H616 SoC, but also its > > > very close sibling SoCs H618 and H700, so add them to the list as well. > > > > > > Signed-off-by: Martin Botka <martin.botka@somainline.org> > > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > > > --- > > > drivers/cpufreq/sun50i-cpufreq-nvmem.c | 61 ++++++++++++++++++++++++++ > > > 1 file changed, 61 insertions(+) > > > > > > diff --git a/drivers/cpufreq/sun50i-cpufreq-nvmem.c b/drivers/cpufreq/sun50i-cpufreq-nvmem.c > > > index bd170611c7906..f9e9fc340f848 100644 > > > --- a/drivers/cpufreq/sun50i-cpufreq-nvmem.c > > > +++ b/drivers/cpufreq/sun50i-cpufreq-nvmem.c > > > @@ -10,6 +10,7 @@ > > > > > > #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > > > > > > +#include <linux/arm-smccc.h> > > > #include <linux/cpu.h> > > > #include <linux/module.h> > > > #include <linux/nvmem-consumer.h> > > > @@ -46,14 +47,71 @@ static u32 sun50i_h6_efuse_xlate(u32 speedbin) > > > return 0; > > > } > > > > > > +/* > > > + * Judging by the OPP tables in the vendor BSP, the quality order of the > > > + * returned speedbin index is 4 -> 0/2 -> 3 -> 1, from worst to best. > > > + * 0 and 2 seem identical from the OPP tables' point of view. > > > + */ > > > +static u32 sun50i_h616_efuse_xlate(u32 speedbin) > > > +{ > > > + int ver_bits = arm_smccc_get_soc_id_revision(); > > > > This needs a Kconfig dependency on ARM_SMCCC_SOC_ID. > > That was my first impulse as well, but it's actually not true: > ARM_SMCCC_SOC_ID just protects the sysfs export code, not this function > here. That does just depend on HAVE_ARM_SMCCC_DISCOVERY, which gets > selected by ARM_GIC_V3, which gets selected by CONFIG_ARM64. So the > arm64 kernel is safe. It is safe to add the dependency explicitly so that if GICV3 decides to drop it, this won't be affected. Thoughts ? > Now apart from ARM(32) (where the situation seems a bit more complex) I > just realise that this would torpedo Brandon's D1 efforts, so I need to > add this change I played with to provide an alternative: > > static int get_soc_id_revision(void) > { > #ifdef CONFIG_HAVE_ARM_SMCCC_DISCOVERY > return arm_smccc_get_soc_id_revision(); > #else > /* Return the value for the worse die revision, to be safe. */ > return 2; > #endif > } > > Does that look acceptable, despite the #ifdef? > if(IS_ENABLED(CONFIG_HAVE_ARM_SMCCC_DISCOVERY)) instead ?
On Wed, Mar 27, 2024 at 11:58:12AM +0000, Sudeep Holla wrote: > On Wed, Mar 27, 2024 at 11:46:08AM +0000, Andre Przywara wrote: > > On Tue, 26 Mar 2024 22:46:27 -0500 > > Samuel Holland <samuel@sholland.org> wrote: > > > > Hi Samuel, > > > > > On 3/26/24 06:47, Andre Przywara wrote: > > > > From: Martin Botka <martin.botka@somainline.org> > > > > > > > > The Allwinner H616/H618 SoCs have different OPP tables per SoC version > > > > and die revision. The SoC version is stored in NVMEM, as before, though > > > > encoded differently. The die revision is in a different register, in the > > > > SRAM controller. Firmware already exports that value in a standardised > > > > way, through the SMCCC SoCID mechanism. We need both values, as some chips > > > > have the same SoC version, but they don't support the same frequencies and > > > > they get differentiated by the die revision. > > > > > > > > Add the new compatible string and tie the new translation function to > > > > it. This mechanism not only covers the original H616 SoC, but also its > > > > very close sibling SoCs H618 and H700, so add them to the list as well. > > > > > > > > Signed-off-by: Martin Botka <martin.botka@somainline.org> > > > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > > > > --- > > > > drivers/cpufreq/sun50i-cpufreq-nvmem.c | 61 ++++++++++++++++++++++++++ > > > > 1 file changed, 61 insertions(+) > > > > > > > > diff --git a/drivers/cpufreq/sun50i-cpufreq-nvmem.c b/drivers/cpufreq/sun50i-cpufreq-nvmem.c > > > > index bd170611c7906..f9e9fc340f848 100644 > > > > --- a/drivers/cpufreq/sun50i-cpufreq-nvmem.c > > > > +++ b/drivers/cpufreq/sun50i-cpufreq-nvmem.c > > > > @@ -10,6 +10,7 @@ > > > > > > > > #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > > > > > > > > +#include <linux/arm-smccc.h> > > > > #include <linux/cpu.h> > > > > #include <linux/module.h> > > > > #include <linux/nvmem-consumer.h> > > > > @@ -46,14 +47,71 @@ static u32 sun50i_h6_efuse_xlate(u32 speedbin) > > > > return 0; > > > > } > > > > > > > > +/* > > > > + * Judging by the OPP tables in the vendor BSP, the quality order of the > > > > + * returned speedbin index is 4 -> 0/2 -> 3 -> 1, from worst to best. > > > > + * 0 and 2 seem identical from the OPP tables' point of view. > > > > + */ > > > > +static u32 sun50i_h616_efuse_xlate(u32 speedbin) > > > > +{ > > > > + int ver_bits = arm_smccc_get_soc_id_revision(); > > > > > > This needs a Kconfig dependency on ARM_SMCCC_SOC_ID. > > > > That was my first impulse as well, but it's actually not true: > > ARM_SMCCC_SOC_ID just protects the sysfs export code, not this function > > here. That does just depend on HAVE_ARM_SMCCC_DISCOVERY, which gets > > selected by ARM_GIC_V3, which gets selected by CONFIG_ARM64. So the > > arm64 kernel is safe. > > It is safe to add the dependency explicitly so that if GICV3 decides to drop > it, this won't be affected. Thoughts ? Ignore this as this will block the ARM(32) build of the driver which I suppose is needed as well. -- Regards, Sudeep
On Wed, 27 Mar 2024 11:58:12 +0000 Sudeep Holla <sudeep.holla@arm.com> wrote: Hi Sudeep, > On Wed, Mar 27, 2024 at 11:46:08AM +0000, Andre Przywara wrote: > > On Tue, 26 Mar 2024 22:46:27 -0500 > > Samuel Holland <samuel@sholland.org> wrote: > > > > Hi Samuel, > > > > > On 3/26/24 06:47, Andre Przywara wrote: > > > > From: Martin Botka <martin.botka@somainline.org> > > > > > > > > The Allwinner H616/H618 SoCs have different OPP tables per SoC version > > > > and die revision. The SoC version is stored in NVMEM, as before, though > > > > encoded differently. The die revision is in a different register, in the > > > > SRAM controller. Firmware already exports that value in a standardised > > > > way, through the SMCCC SoCID mechanism. We need both values, as some chips > > > > have the same SoC version, but they don't support the same frequencies and > > > > they get differentiated by the die revision. > > > > > > > > Add the new compatible string and tie the new translation function to > > > > it. This mechanism not only covers the original H616 SoC, but also its > > > > very close sibling SoCs H618 and H700, so add them to the list as well. > > > > > > > > Signed-off-by: Martin Botka <martin.botka@somainline.org> > > > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > > > > --- > > > > drivers/cpufreq/sun50i-cpufreq-nvmem.c | 61 ++++++++++++++++++++++++++ > > > > 1 file changed, 61 insertions(+) > > > > > > > > diff --git a/drivers/cpufreq/sun50i-cpufreq-nvmem.c b/drivers/cpufreq/sun50i-cpufreq-nvmem.c > > > > index bd170611c7906..f9e9fc340f848 100644 > > > > --- a/drivers/cpufreq/sun50i-cpufreq-nvmem.c > > > > +++ b/drivers/cpufreq/sun50i-cpufreq-nvmem.c > > > > @@ -10,6 +10,7 @@ > > > > > > > > #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > > > > > > > > +#include <linux/arm-smccc.h> > > > > #include <linux/cpu.h> > > > > #include <linux/module.h> > > > > #include <linux/nvmem-consumer.h> > > > > @@ -46,14 +47,71 @@ static u32 sun50i_h6_efuse_xlate(u32 speedbin) > > > > return 0; > > > > } > > > > > > > > +/* > > > > + * Judging by the OPP tables in the vendor BSP, the quality order of the > > > > + * returned speedbin index is 4 -> 0/2 -> 3 -> 1, from worst to best. > > > > + * 0 and 2 seem identical from the OPP tables' point of view. > > > > + */ > > > > +static u32 sun50i_h616_efuse_xlate(u32 speedbin) > > > > +{ > > > > + int ver_bits = arm_smccc_get_soc_id_revision(); > > > > > > This needs a Kconfig dependency on ARM_SMCCC_SOC_ID. > > > > That was my first impulse as well, but it's actually not true: > > ARM_SMCCC_SOC_ID just protects the sysfs export code, not this function > > here. That does just depend on HAVE_ARM_SMCCC_DISCOVERY, which gets > > selected by ARM_GIC_V3, which gets selected by CONFIG_ARM64. So the > > arm64 kernel is safe. > > It is safe to add the dependency explicitly so that if GICV3 decides to drop > it, this won't be affected. Thoughts ? That would be one possibility, but since there are patches on the list[1] that use this driver for the Allwinner D1 with RISC-V cores, this would need to be revisited then anyway. > > Now apart from ARM(32) (where the situation seems a bit more complex) I > > just realise that this would torpedo Brandon's D1 efforts, so I need to > > add this change I played with to provide an alternative: > > > > static int get_soc_id_revision(void) > > { > > #ifdef CONFIG_HAVE_ARM_SMCCC_DISCOVERY > > return arm_smccc_get_soc_id_revision(); > > #else > > /* Return the value for the worse die revision, to be safe. */ > > return 2; > > #endif > > } > > > > Does that look acceptable, despite the #ifdef? > > > > if(IS_ENABLED(CONFIG_HAVE_ARM_SMCCC_DISCOVERY)) instead ? Well, but this would rely on at least the prototype to be visible, right? And then on the toolchain to optimise away the call, so that the symbol doesn't even appear in the object file? So would this work for the RISC-V case? Cheers, Andre [1] https://lore.kernel.org/linux-sunxi/20231218110543.64044-1-fusibrandon13@gmail.com/T/#t
Dne torek, 26. marec 2024 ob 12:47:37 CET je Andre Przywara napisal(a): > From: Martin Botka <martin.botka@somainline.org> > > The AllWinner H616 SoC will use the (extended) H6 OPP driver, so add > them to the cpufreq-dt blocklist, to not create the device twice. > This also affects the closely related sibling SoCs H618 and H700. > > Signed-off-by: Martin Botka <martin.botka@somainline.org> > Signed-off-by: Andre Przywara <andre.przywara@arm.com> Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com> Best regards, Jernej > --- > drivers/cpufreq/cpufreq-dt-platdev.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c > index b993a498084bc..86d8baa816795 100644 > --- a/drivers/cpufreq/cpufreq-dt-platdev.c > +++ b/drivers/cpufreq/cpufreq-dt-platdev.c > @@ -104,6 +104,9 @@ static const struct of_device_id allowlist[] __initconst = { > */ > static const struct of_device_id blocklist[] __initconst = { > { .compatible = "allwinner,sun50i-h6", }, > + { .compatible = "allwinner,sun50i-h616", }, > + { .compatible = "allwinner,sun50i-h618", }, > + { .compatible = "allwinner,sun50i-h700", }, > > { .compatible = "apple,arm-platform", }, > >
Dne torek, 26. marec 2024 ob 12:47:39 CET je Andre Przywara napisal(a): > From: Brandon Cheo Fusi <fusibrandon13@gmail.com> > > Make converting the speed bin value into a speed grade generic and > determined by a platform specific callback. Also change the prototypes > involved to encode the speed bin directly in the return value. > > This allows to extend the driver more easily to support more SoCs. > > Signed-off-by: Brandon Cheo Fusi <fusibrandon13@gmail.com> > [Andre: merge output into return value] > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- > drivers/cpufreq/sun50i-cpufreq-nvmem.c | 74 +++++++++++++++++--------- > 1 file changed, 49 insertions(+), 25 deletions(-) > > diff --git a/drivers/cpufreq/sun50i-cpufreq-nvmem.c b/drivers/cpufreq/sun50i-cpufreq-nvmem.c > index 32a9c88f8ff6d..7b44f3b13e7d2 100644 > --- a/drivers/cpufreq/sun50i-cpufreq-nvmem.c > +++ b/drivers/cpufreq/sun50i-cpufreq-nvmem.c > @@ -25,19 +25,52 @@ > > static struct platform_device *cpufreq_dt_pdev, *sun50i_cpufreq_pdev; > > +struct sunxi_cpufreq_data { > + u32 (*efuse_xlate)(u32 speedbin); > +}; > + > +static u32 sun50i_h6_efuse_xlate(u32 speedbin) > +{ > + u32 efuse_value; > + > + efuse_value = (speedbin >> NVMEM_SHIFT) & NVMEM_MASK; > + > + /* > + * We treat unexpected efuse values as if the SoC was from > + * the slowest bin. Expected efuse values are 1-3, slowest > + * to fastest. > + */ > + if (efuse_value >= 1 && efuse_value <= 3) > + return efuse_value - 1; > + else > + return 0; > +} > + > +static struct sunxi_cpufreq_data sun50i_h6_cpufreq_data = { > + .efuse_xlate = sun50i_h6_efuse_xlate, > +}; > + > +static const struct of_device_id cpu_opp_match_list[] = { > + { .compatible = "allwinner,sun50i-h6-operating-points", > + .data = &sun50i_h6_cpufreq_data, > + }, > + {} > +}; > + > /** > * sun50i_cpufreq_get_efuse() - Determine speed grade from efuse value > - * @versions: Set to the value parsed from efuse > * > - * Returns 0 if success. > + * Returns non-negative speed bin index on success, a negative error > + * value otherwise. > */ > -static int sun50i_cpufreq_get_efuse(u32 *versions) > +static int sun50i_cpufreq_get_efuse(void) > { > struct nvmem_cell *speedbin_nvmem; > struct device_node *np; > struct device *cpu_dev; > - u32 *speedbin, efuse_value; > - size_t len; > + const struct of_device_id *match; > + const struct sunxi_cpufreq_data *opp_data; > + u32 *speedbin; nit: reverse christmas tree Other than that, Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com> Best regards, Jernej > int ret; > > cpu_dev = get_cpu_device(0); > @@ -48,12 +81,12 @@ static int sun50i_cpufreq_get_efuse(u32 *versions) > if (!np) > return -ENOENT; > > - ret = of_device_is_compatible(np, > - "allwinner,sun50i-h6-operating-points"); > - if (!ret) { > + match = of_match_node(cpu_opp_match_list, np); > + if (!match) { > of_node_put(np); > return -ENOENT; > } > + opp_data = match->data; > > speedbin_nvmem = of_nvmem_cell_get(np, NULL); > of_node_put(np); > @@ -61,25 +94,16 @@ static int sun50i_cpufreq_get_efuse(u32 *versions) > return dev_err_probe(cpu_dev, PTR_ERR(speedbin_nvmem), > "Could not get nvmem cell\n"); > > - speedbin = nvmem_cell_read(speedbin_nvmem, &len); > + speedbin = nvmem_cell_read(speedbin_nvmem, NULL); > nvmem_cell_put(speedbin_nvmem); > if (IS_ERR(speedbin)) > return PTR_ERR(speedbin); > > - efuse_value = (*speedbin >> NVMEM_SHIFT) & NVMEM_MASK; > - > - /* > - * We treat unexpected efuse values as if the SoC was from > - * the slowest bin. Expected efuse values are 1-3, slowest > - * to fastest. > - */ > - if (efuse_value >= 1 && efuse_value <= 3) > - *versions = efuse_value - 1; > - else > - *versions = 0; > + ret = opp_data->efuse_xlate(*speedbin); > > kfree(speedbin); > - return 0; > + > + return ret; > }; > > static int sun50i_cpufreq_nvmem_probe(struct platform_device *pdev) > @@ -87,7 +111,7 @@ static int sun50i_cpufreq_nvmem_probe(struct platform_device *pdev) > int *opp_tokens; > char name[MAX_NAME_LEN]; > unsigned int cpu; > - u32 speed = 0; > + int speed; > int ret; > > opp_tokens = kcalloc(num_possible_cpus(), sizeof(*opp_tokens), > @@ -95,10 +119,10 @@ static int sun50i_cpufreq_nvmem_probe(struct platform_device *pdev) > if (!opp_tokens) > return -ENOMEM; > > - ret = sun50i_cpufreq_get_efuse(&speed); > - if (ret) { > + speed = sun50i_cpufreq_get_efuse(); > + if (speed < 0) { > kfree(opp_tokens); > - return ret; > + return speed; > } > > snprintf(name, MAX_NAME_LEN, "speed%d", speed); >
Dne torek, 26. marec 2024 ob 12:47:40 CET je Andre Przywara napisal(a): > The opp_supported_hw DT property allows the DT to specify a mask of chip > revisions that a certain OPP is eligible for. This allows for easy > limiting of maximum frequencies, for instance. > > Add support for that in the sun50i-cpufreq-nvmem driver. We support both > the existing opp-microvolt suffix properties as well as the > opp-supported-hw property, the generic code figures out which is needed > automatically. > However if none of the DT OPP nodes contain an opp-supported-hw > property, the core code will ignore all OPPs and the driver will fail > probing. So check the DT's eligibility first before using that feature. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com> Best regards, Jernej
Dne torek, 26. marec 2024 ob 12:47:42 CET je Andre Przywara napisal(a): > From: Martin Botka <martin.botka@somainline.org> > > Add an Operating Performance Points table for the CPU cores to enable > Dynamic Voltage & Frequency Scaling (DVFS) on the H616. > The values were taken from the BSP sources. The (newer) H700 chips we > have seen seem to use a separate speed bin, its OPP values were taken > from a live system and added to the mix. > > Also add the needed cpu_speed_grade nvmem cell and the cooling cells > properties, to enable passive cooling. > > Signed-off-by: Martin Botka <martin.botka@somainline.org> > [Andre: rework to minimise opp-microvolt properties] > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- > .../dts/allwinner/sun50i-h616-cpu-opp.dtsi | 125 ++++++++++++++++++ > .../arm64/boot/dts/allwinner/sun50i-h616.dtsi | 8 ++ > 2 files changed, 133 insertions(+) > create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h616-cpu-opp.dtsi > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h616-cpu-opp.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h616-cpu-opp.dtsi > new file mode 100644 > index 0000000000000..6073fdf672592 > --- /dev/null > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h616-cpu-opp.dtsi > @@ -0,0 +1,125 @@ > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) > +// Copyright (C) 2023 Martin Botka <martin@somainline.org> > + > +/ { > + cpu_opp_table: opp-table-cpu { > + compatible = "allwinner,sun50i-h616-operating-points"; > + nvmem-cells = <&cpu_speed_grade>; > + opp-shared; > + > + opp-480000000 { > + opp-hz = /bits/ 64 <480000000>; > + opp-microvolt = <900000>; Ideally triplet of voltages should be specified, to support PMIC-less boards, but that's unlikely to happen with these SoCs. Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com> Best regards, Jernej > + clock-latency-ns = <244144>; /* 8 32k periods */ > + opp-supported-hw = <0x3f>; > + }; > + > + opp-600000000 { > + opp-hz = /bits/ 64 <600000000>; > + opp-microvolt = <900000>; > + clock-latency-ns = <244144>; /* 8 32k periods */ > + opp-supported-hw = <0x12>; > + }; > + > + opp-720000000 { > + opp-hz = /bits/ 64 <720000000>; > + opp-microvolt = <900000>; > + clock-latency-ns = <244144>; /* 8 32k periods */ > + opp-supported-hw = <0x2d>; > + }; > + > + opp-792000000 { > + opp-hz = /bits/ 64 <792000000>; > + opp-microvolt-speed1 = <900000>; > + opp-microvolt-speed4 = <940000>; > + clock-latency-ns = <244144>; /* 8 32k periods */ > + opp-supported-hw = <0x12>; > + }; > + > + opp-936000000 { > + opp-hz = /bits/ 64 <936000000>; > + opp-microvolt = <900000>; > + clock-latency-ns = <244144>; /* 8 32k periods */ > + opp-supported-hw = <0x0d>; > + }; > + > + opp-1008000000 { > + opp-hz = /bits/ 64 <1008000000>; > + opp-microvolt-speed0 = <950000>; > + opp-microvolt-speed1 = <940000>; > + opp-microvolt-speed2 = <950000>; > + opp-microvolt-speed3 = <950000>; > + opp-microvolt-speed4 = <1020000>; > + clock-latency-ns = <244144>; /* 8 32k periods */ > + opp-supported-hw = <0x1f>; > + }; > + > + opp-10320000 { > + opp-hz = /bits/ 64 <1032000000>; > + opp-microvolt = <900000>; > + clock-latency-ns = <244144>; /* 8 32k periods */ > + opp-supported-hw = <0x20>; > + }; > + > + opp-1104000000 { > + opp-hz = /bits/ 64 <1104000000>; > + opp-microvolt-speed0 = <1000000>; > + opp-microvolt-speed2 = <1000000>; > + opp-microvolt-speed3 = <1000000>; > + opp-microvolt-speed5 = <950000>; > + clock-latency-ns = <244144>; /* 8 32k periods */ > + opp-supported-hw = <0x2d>; > + }; > + > + opp-1200000000 { > + opp-hz = /bits/ 64 <1200000000>; > + opp-microvolt-speed0 = <1050000>; > + opp-microvolt-speed1 = <1020000>; > + opp-microvolt-speed2 = <1050000>; > + opp-microvolt-speed3 = <1050000>; > + opp-microvolt-speed4 = <1100000>; > + opp-microvolt-speed5 = <1020000>; > + clock-latency-ns = <244144>; /* 8 32k periods */ > + opp-supported-hw = <0x3f>; > + }; > + > + opp-1320000000 { > + opp-hz = /bits/ 64 <1320000000>; > + opp-microvolt = <1100000>; > + clock-latency-ns = <244144>; /* 8 32k periods */ > + opp-supported-hw = <0x1d>; > + }; > + > + opp-1416000000 { > + opp-hz = /bits/ 64 <1416000000>; > + opp-microvolt = <1100000>; > + clock-latency-ns = <244144>; /* 8 32k periods */ > + opp-supported-hw = <0x2d>; > + }; > + > + opp-1512000000 { > + opp-hz = /bits/ 64 <1512000000>; > + opp-microvolt-speed1 = <1100000>; > + opp-microvolt-speed3 = <1100000>; > + opp-microvolt-speed5 = <1160000>; > + clock-latency-ns = <244144>; /* 8 32k periods */ > + opp-supported-hw = <0x2a>; > + }; > + }; > +}; > + > +&cpu0 { > + operating-points-v2 = <&cpu_opp_table>; > +}; > + > +&cpu1 { > + operating-points-v2 = <&cpu_opp_table>; > +}; > + > +&cpu2 { > + operating-points-v2 = <&cpu_opp_table>; > +}; > + > +&cpu3 { > + operating-points-v2 = <&cpu_opp_table>; > +}; > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi > index b2e85e52d1a12..c0fa466fa9f07 100644 > --- a/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi > @@ -26,6 +26,7 @@ cpu0: cpu@0 { > reg = <0>; > enable-method = "psci"; > clocks = <&ccu CLK_CPUX>; > + #cooling-cells = <2>; > }; > > cpu1: cpu@1 { > @@ -34,6 +35,7 @@ cpu1: cpu@1 { > reg = <1>; > enable-method = "psci"; > clocks = <&ccu CLK_CPUX>; > + #cooling-cells = <2>; > }; > > cpu2: cpu@2 { > @@ -42,6 +44,7 @@ cpu2: cpu@2 { > reg = <2>; > enable-method = "psci"; > clocks = <&ccu CLK_CPUX>; > + #cooling-cells = <2>; > }; > > cpu3: cpu@3 { > @@ -50,6 +53,7 @@ cpu3: cpu@3 { > reg = <3>; > enable-method = "psci"; > clocks = <&ccu CLK_CPUX>; > + #cooling-cells = <2>; > }; > }; > > @@ -156,6 +160,10 @@ sid: efuse@3006000 { > ths_calibration: thermal-sensor-calibration@14 { > reg = <0x14 0x8>; > }; > + > + cpu_speed_grade: cpu-speed-grade@0 { > + reg = <0x0 2>; > + }; > }; > > watchdog: watchdog@30090a0 { >
Dne torek, 26. marec 2024 ob 12:47:43 CET je Andre Przywara napisal(a): > With the DT bindings now describing the format of the CPU OPP tables, we > can include the OPP table in each board's .dts file, and specify the CPU > power supply. > This allows to enable DVFS, and get up to 50% of performance benefit in > the highest OPP, or up to 60% power savings in the lowest OPP, compared > to the fixed 1GHz @ 1.0V OPP we are running in by default at the moment. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com> Best regards, Jernej