[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <KYAH0S.DN4CQO1UCNM72@somainline.org>
Date: Mon, 04 Sep 2023 22:44:44 +0200
From: Martin Botka <martin.botka@...ainline.org>
To: Andre Przywara <andre.przywara@....com>
Cc: Mark Rutland <mark.rutland@....com>,
Lorenzo Pieralisi <lpieralisi@...nel.org>,
Sudeep Holla <sudeep.holla@....com>,
"Rafael J. Wysocki" <rafael@...nel.org>,
Viresh Kumar <viresh.kumar@...aro.org>,
Yangtao Li <tiny.windzz@...il.com>,
Chen-Yu Tsai <wens@...e.org>,
Jernej Skrabec <jernej.skrabec@...il.com>,
Samuel Holland <samuel@...lland.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
linux-pm@...r.kernel.org, linux-sunxi@...ts.linux.dev,
devicetree@...r.kernel.org, Alan Ma <tech@...u3d.com>,
Luke Harrison <bttuniversity@...u3d.com>,
Marijn Suijten <marijn.suijten@...ainline.org>,
AngeloGioacchino Del Regno
<angelogioacchino.delregno@...ainline.org>,
Konrad Dybcio <konrad.dybcio@...ainline.org>,
Rogerio Goncalves <rogerlz@...il.com>,
Martin Botka <martin@...u3d.com>
Subject: Re: [PATCH 4/6] cpufreq: sun50i: Add H616 support
On Mon, Sep 4 2023 at 09:41:08 PM +01:00:00, Andre Przywara
<andre.przywara@....com> wrote:
> On Mon, 04 Sep 2023 17:57:04 +0200
> Martin Botka <martin.botka@...ainline.org> wrote:
>
> Hi,
>
>> AllWinner H616 SoC has few revisions that support different list
>> of uV and frequencies.
>>
>> Some revisions have the same NVMEM value and thus we have to check
>> the SoC revision from SMCCC to differentiate between them.
>
> So this patch is a bit hard to read, as it combines two things: the
> refactoring and the actual H616 bits. Can you please split this up,
> with a first patch just introducing struct sunxi_cpufreq_soc_data and
> moving the existing code into the separate xlate function, and all the
> other required changes? Then having a second patch adding the H616
> bits on top? This makes review easier, as the first patch should not
> change any behaviour, and the second patch just focuses on the new
> H616
> bits.
Yea it is. I will split them up in V2.
>
> Cheers,
> Andre
>
>>
>> Signed-off-by: Martin Botka <martin.botka@...ainline.org>
>> ---
>> drivers/cpufreq/sun50i-cpufreq-nvmem.c | 149
>> ++++++++++++++++++++++++++++-----
>> 1 file changed, 126 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/cpufreq/sun50i-cpufreq-nvmem.c
>> b/drivers/cpufreq/sun50i-cpufreq-nvmem.c
>> index 4321d7bbe769..19c126fb081e 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>
>> @@ -23,20 +24,94 @@
>> #define NVMEM_MASK 0x7
>> #define NVMEM_SHIFT 5
>>
>> +struct sunxi_cpufreq_soc_data {
>> + int (*efuse_xlate)(u32 *versions, u32 *efuse, char *name, size_t
>> len);
>> + u8 ver_freq_limit;
>> +};
>> +
>> static struct platform_device *cpufreq_dt_pdev,
>> *sun50i_cpufreq_pdev;
>>
>> +static int sun50i_h616_efuse_xlate(u32 *versions, u32 *efuse, char
>> *name, size_t len)
>> +{
>> + int value = 0;
>> + u32 speedgrade = 0;
>> + u32 i;
>> + int ver_bits = arm_smccc_get_soc_id_revision();
>> +
>> + if (len > 4) {
>> + pr_err("Invalid nvmem cell length\n");
>> + return -EINVAL;
>> + }
>> +
>> + for (i = 0; i < len; i++)
>> + speedgrade |= (efuse[i] << (i * 8));
>> +
>> + switch (speedgrade) {
>> + case 0x2000:
>> + value = 0;
>> + break;
>> + case 0x2400:
>> + case 0x7400:
>> + case 0x2c00:
>> + case 0x7c00:
>> + if (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:
>> + default:
>> + value = 0;
>> + }
>> + *versions = (1 << value);
>> + snprintf(name, MAX_NAME_LEN, "speed%d", value);
>> + return 0;
>> +}
>> +
>> +static int sun50i_h6_efuse_xlate(u32 *versions, u32 *efuse, char
>> *name, size_t len)
>> +{
>> + int efuse_value = (*efuse >> 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;
>> +
>> + snprintf(name, MAX_NAME_LEN, "speed%d", *versions);
>> + return 0;
>> +}
>> +
>> /**
>> * sun50i_cpufreq_get_efuse() - Determine speed grade from efuse
>> value
>> + * @soc_data: Struct containing soc specific data & functions
>> * @versions: Set to the value parsed from efuse
>> + * @name: Set to the name of speed
>> *
>> * Returns 0 if success.
>> */
>> -static int sun50i_cpufreq_get_efuse(u32 *versions)
>> +static int sun50i_cpufreq_get_efuse(const struct
>> sunxi_cpufreq_soc_data *soc_data,
>> + u32 *versions, char *name)
>> {
>> struct nvmem_cell *speedbin_nvmem;
>> struct device_node *np;
>> struct device *cpu_dev;
>> - u32 *speedbin, efuse_value;
>> + u32 *speedbin;
>> size_t len;
>> int ret;
>>
>> @@ -48,9 +123,9 @@ 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) {
>> + if (of_device_is_compatible(np,
>> "allwinner,sun50i-h6-operating-points")) {
>> + } else if (of_device_is_compatible(np,
>> "allwinner,sun50i-h616-operating-points")) {
>> + } else {
>> of_node_put(np);
>> return -ENOENT;
>> }
>> @@ -66,17 +141,9 @@ static int sun50i_cpufreq_get_efuse(u32
>> *versions)
>> 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 = soc_data->efuse_xlate(versions, speedbin, name, len);
>> + if (ret)
>> + return ret;
>>
>> kfree(speedbin);
>> return 0;
>> @@ -84,25 +151,30 @@ static int sun50i_cpufreq_get_efuse(u32
>> *versions)
>>
>> static int sun50i_cpufreq_nvmem_probe(struct platform_device *pdev)
>> {
>> + const struct of_device_id *match;
>> + const struct sunxi_cpufreq_soc_data *soc_data;
>> int *opp_tokens;
>> char name[MAX_NAME_LEN];
>> unsigned int cpu;
>> - u32 speed = 0;
>> + u32 version = 0;
>> int ret;
>>
>> + match = dev_get_platdata(&pdev->dev);
>> + if (!match)
>> + return -EINVAL;
>> + soc_data = match->data;
>> +
>> opp_tokens = kcalloc(num_possible_cpus(), sizeof(*opp_tokens),
>> GFP_KERNEL);
>> if (!opp_tokens)
>> return -ENOMEM;
>>
>> - ret = sun50i_cpufreq_get_efuse(&speed);
>> + ret = sun50i_cpufreq_get_efuse(match->data, &version, name);
>> if (ret) {
>> kfree(opp_tokens);
>> return ret;
>> }
>>
>> - snprintf(name, MAX_NAME_LEN, "speed%d", speed);
>> -
>> for_each_possible_cpu(cpu) {
>> struct device *cpu_dev = get_cpu_device(cpu);
>>
>> @@ -117,6 +189,16 @@ static int sun50i_cpufreq_nvmem_probe(struct
>> platform_device *pdev)
>> pr_err("Failed to set prop name\n");
>> goto free_opp;
>> }
>> +
>> + if (soc_data->ver_freq_limit) {
>> + opp_tokens[cpu] = dev_pm_opp_set_supported_hw(cpu_dev,
>> + &version, 1);
>> + if (opp_tokens[cpu] < 0) {
>> + ret = opp_tokens[cpu];
>> + pr_err("Failed to set hw\n");
>> + goto free_opp;
>> + }
>> + }
>> }
>>
>> cpufreq_dt_pdev = platform_device_register_simple("cpufreq-dt",
>> -1,
>> @@ -132,6 +214,8 @@ static int sun50i_cpufreq_nvmem_probe(struct
>> platform_device *pdev)
>> free_opp:
>> for_each_possible_cpu(cpu)
>> dev_pm_opp_put_prop_name(opp_tokens[cpu]);
>> + if (soc_data->ver_freq_limit)
>> + dev_pm_opp_put_supported_hw(opp_tokens[cpu]);
>> kfree(opp_tokens);
>>
>> return ret;
>> @@ -140,12 +224,21 @@ static int sun50i_cpufreq_nvmem_probe(struct
>> platform_device *pdev)
>> static int sun50i_cpufreq_nvmem_remove(struct platform_device
>> *pdev)
>> {
>> int *opp_tokens = platform_get_drvdata(pdev);
>> + const struct of_device_id *match;
>> + const struct sunxi_cpufreq_soc_data *soc_data;
>> unsigned int cpu;
>>
>> + match = dev_get_platdata(&pdev->dev);
>> + if (!match)
>> + return -EINVAL;
>> + soc_data = match->data;
>> +
>> platform_device_unregister(cpufreq_dt_pdev);
>>
>> for_each_possible_cpu(cpu)
>> dev_pm_opp_put_prop_name(opp_tokens[cpu]);
>> + if (soc_data->ver_freq_limit)
>> + dev_pm_opp_put_supported_hw(opp_tokens[cpu]);
>>
>> kfree(opp_tokens);
>>
>> @@ -160,8 +253,18 @@ static struct platform_driver
>> sun50i_cpufreq_driver = {
>> },
>> };
>>
>> +static const struct sunxi_cpufreq_soc_data sun50i_h616_data = {
>> + .efuse_xlate = sun50i_h616_efuse_xlate,
>> + .ver_freq_limit = true,
>> +};
>> +
>> +static const struct sunxi_cpufreq_soc_data sun50i_h6_data = {
>> + .efuse_xlate = sun50i_h6_efuse_xlate,
>> +};
>> +
>> static const struct of_device_id sun50i_cpufreq_match_list[] = {
>> - { .compatible = "allwinner,sun50i-h6" },
>> + { .compatible = "allwinner,sun50i-h6", .data = &sun50i_h6_data },
>> + { .compatible = "allwinner,sun50i-h616", .data =
>> &sun50i_h616_data },
>> {}
>> };
>> MODULE_DEVICE_TABLE(of, sun50i_cpufreq_match_list);
>> @@ -197,8 +300,8 @@ static int __init sun50i_cpufreq_init(void)
>> return ret;
>>
>> sun50i_cpufreq_pdev =
>> - platform_device_register_simple("sun50i-cpufreq-nvmem",
>> - -1, NULL, 0);
>> + platform_device_register_data(NULL, "sun50i-cpufreq-nvmem",
>> + -1, match, sizeof(*match));
>> ret = PTR_ERR_OR_ZERO(sun50i_cpufreq_pdev);
>> if (ret == 0)
>> return 0;
>>
>
Powered by blists - more mailing lists