From: Maxime Ripard <maxime@cerno.tech> To: Shuosheng Huang <huangshuosheng@allwinnertech.com> Cc: tiny.windzz@gmail.com, rjw@rjwysocki.net, viresh.kumar@linaro.org, wens@csie.org, jernej.skrabec@siol.net, samuel@sholland.org, linux-pm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v4 1/6] cpufreq: sun50i: add efuse_xlate to get efuse version. Date: Tue, 8 Dec 2020 09:21:50 +0100 [thread overview] Message-ID: <20201208082150.jbpdlpcghbj3liqj@gilmour> (raw) In-Reply-To: <20201208071928.2078-1-huangshuosheng@allwinnertech.com> [-- Attachment #1: Type: text/plain, Size: 4872 bytes --] Hi, On Tue, Dec 08, 2020 at 03:19:28PM +0800, Shuosheng Huang wrote: > It's better to use efuse_xlate to extract the differentiated part > regarding different SoC. > > Signed-off-by: Shuosheng Huang <huangshuosheng@allwinnertech.com> Please wait a bit for reviews before sending a new version. You've sent three versions in a day, and I haven't reviewed the first one. Also, please put a changelog in the cover letter, we don't have any way to tell what is the difference between v1, v2, v3 and v4. > --- > drivers/cpufreq/sun50i-cpufreq-nvmem.c | 82 ++++++++++++++++---------- > 1 file changed, 51 insertions(+), 31 deletions(-) > > diff --git a/drivers/cpufreq/sun50i-cpufreq-nvmem.c b/drivers/cpufreq/sun50i-cpufreq-nvmem.c > index 9907a165135b..3c0531938d1a 100644 > --- a/drivers/cpufreq/sun50i-cpufreq-nvmem.c > +++ b/drivers/cpufreq/sun50i-cpufreq-nvmem.c > @@ -19,24 +19,51 @@ > > #define MAX_NAME_LEN 7 > > -#define NVMEM_MASK 0x7 > -#define NVMEM_SHIFT 5 > +#define SUN50I_H6_NVMEM_MASK 0x7 > +#define SUN50I_H6_NVMEM_SHIFT 5 > + > +struct sunxi_cpufreq_soc_data { > + int (*efuse_xlate)(struct nvmem_cell *speedbin_nvmem); > +}; > > static struct platform_device *cpufreq_dt_pdev, *sun50i_cpufreq_pdev; > > +static int sun50i_h6_efuse_xlate(struct nvmem_cell *speedbin_nvmem) > +{ > + size_t len; > + u32 *speedbin; > + u32 efuse_value; > + > + speedbin = nvmem_cell_read(speedbin_nvmem, &len); > + if (IS_ERR(speedbin)) > + return PTR_ERR(speedbin); > + > + efuse_value = (*(u32 *)speedbin >> SUN50I_H6_NVMEM_SHIFT) & > + SUN50I_H6_NVMEM_MASK; > + kfree(speedbin); > + /* > + * 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; > +} > + > /** > * sun50i_cpufreq_get_efuse() - Determine speed grade from efuse value > - * @versions: Set to the value parsed from efuse > + * @soc_data: pointer to sunxi_cpufreq_soc_data context > * > * 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) > { > struct nvmem_cell *speedbin_nvmem; > struct device_node *np; > struct device *cpu_dev; > - u32 *speedbin, efuse_value; > - size_t len; > + int versions; > int ret; > > cpu_dev = get_cpu_device(0); > @@ -63,43 +90,33 @@ static int sun50i_cpufreq_get_efuse(u32 *versions) > return PTR_ERR(speedbin_nvmem); > } > > - speedbin = nvmem_cell_read(speedbin_nvmem, &len); > + versions = soc_data->efuse_xlate(speedbin_nvmem); > 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; > > - kfree(speedbin); > - return 0; > + return versions; > }; > > static int sun50i_cpufreq_nvmem_probe(struct platform_device *pdev) > { > + const struct of_device_id *match; > struct opp_table **opp_tables; > char name[MAX_NAME_LEN]; > unsigned int cpu; > - u32 speed = 0; > + int speed = 0; > int ret; > > + match = dev_get_platdata(&pdev->dev); > + if (!match) > + return -EINVAL; > + > opp_tables = kcalloc(num_possible_cpus(), sizeof(*opp_tables), > GFP_KERNEL); > if (!opp_tables) > return -ENOMEM; > > - ret = sun50i_cpufreq_get_efuse(&speed); > - if (ret) > - return ret; > + speed = sun50i_cpufreq_get_efuse(match->data); > + if (speed < 0) > + return speed; > > snprintf(name, MAX_NAME_LEN, "speed%d", speed); > > @@ -163,8 +180,12 @@ static struct platform_driver sun50i_cpufreq_driver = { > }, > }; > > +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 }, > {} > }; > > @@ -198,9 +219,8 @@ static int __init sun50i_cpufreq_init(void) > if (unlikely(ret < 0)) > return ret; > > - sun50i_cpufreq_pdev = > - platform_device_register_simple("sun50i-cpufreq-nvmem", > - -1, NULL, 0); > + sun50i_cpufreq_pdev = platform_device_register_data(NULL, > + "sun50i-cpufreq-nvmem", -1, match, sizeof(*match)); The alignment here raises a checkpatch warning with --strict Maxime [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --]
WARNING: multiple messages have this Message-ID (diff)
From: Maxime Ripard <maxime@cerno.tech> To: Shuosheng Huang <huangshuosheng@allwinnertech.com> Cc: jernej.skrabec@siol.net, samuel@sholland.org, tiny.windzz@gmail.com, viresh.kumar@linaro.org, linux-pm@vger.kernel.org, rjw@rjwysocki.net, linux-kernel@vger.kernel.org, wens@csie.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v4 1/6] cpufreq: sun50i: add efuse_xlate to get efuse version. Date: Tue, 8 Dec 2020 09:21:50 +0100 [thread overview] Message-ID: <20201208082150.jbpdlpcghbj3liqj@gilmour> (raw) In-Reply-To: <20201208071928.2078-1-huangshuosheng@allwinnertech.com> [-- Attachment #1.1: Type: text/plain, Size: 4872 bytes --] Hi, On Tue, Dec 08, 2020 at 03:19:28PM +0800, Shuosheng Huang wrote: > It's better to use efuse_xlate to extract the differentiated part > regarding different SoC. > > Signed-off-by: Shuosheng Huang <huangshuosheng@allwinnertech.com> Please wait a bit for reviews before sending a new version. You've sent three versions in a day, and I haven't reviewed the first one. Also, please put a changelog in the cover letter, we don't have any way to tell what is the difference between v1, v2, v3 and v4. > --- > drivers/cpufreq/sun50i-cpufreq-nvmem.c | 82 ++++++++++++++++---------- > 1 file changed, 51 insertions(+), 31 deletions(-) > > diff --git a/drivers/cpufreq/sun50i-cpufreq-nvmem.c b/drivers/cpufreq/sun50i-cpufreq-nvmem.c > index 9907a165135b..3c0531938d1a 100644 > --- a/drivers/cpufreq/sun50i-cpufreq-nvmem.c > +++ b/drivers/cpufreq/sun50i-cpufreq-nvmem.c > @@ -19,24 +19,51 @@ > > #define MAX_NAME_LEN 7 > > -#define NVMEM_MASK 0x7 > -#define NVMEM_SHIFT 5 > +#define SUN50I_H6_NVMEM_MASK 0x7 > +#define SUN50I_H6_NVMEM_SHIFT 5 > + > +struct sunxi_cpufreq_soc_data { > + int (*efuse_xlate)(struct nvmem_cell *speedbin_nvmem); > +}; > > static struct platform_device *cpufreq_dt_pdev, *sun50i_cpufreq_pdev; > > +static int sun50i_h6_efuse_xlate(struct nvmem_cell *speedbin_nvmem) > +{ > + size_t len; > + u32 *speedbin; > + u32 efuse_value; > + > + speedbin = nvmem_cell_read(speedbin_nvmem, &len); > + if (IS_ERR(speedbin)) > + return PTR_ERR(speedbin); > + > + efuse_value = (*(u32 *)speedbin >> SUN50I_H6_NVMEM_SHIFT) & > + SUN50I_H6_NVMEM_MASK; > + kfree(speedbin); > + /* > + * 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; > +} > + > /** > * sun50i_cpufreq_get_efuse() - Determine speed grade from efuse value > - * @versions: Set to the value parsed from efuse > + * @soc_data: pointer to sunxi_cpufreq_soc_data context > * > * 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) > { > struct nvmem_cell *speedbin_nvmem; > struct device_node *np; > struct device *cpu_dev; > - u32 *speedbin, efuse_value; > - size_t len; > + int versions; > int ret; > > cpu_dev = get_cpu_device(0); > @@ -63,43 +90,33 @@ static int sun50i_cpufreq_get_efuse(u32 *versions) > return PTR_ERR(speedbin_nvmem); > } > > - speedbin = nvmem_cell_read(speedbin_nvmem, &len); > + versions = soc_data->efuse_xlate(speedbin_nvmem); > 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; > > - kfree(speedbin); > - return 0; > + return versions; > }; > > static int sun50i_cpufreq_nvmem_probe(struct platform_device *pdev) > { > + const struct of_device_id *match; > struct opp_table **opp_tables; > char name[MAX_NAME_LEN]; > unsigned int cpu; > - u32 speed = 0; > + int speed = 0; > int ret; > > + match = dev_get_platdata(&pdev->dev); > + if (!match) > + return -EINVAL; > + > opp_tables = kcalloc(num_possible_cpus(), sizeof(*opp_tables), > GFP_KERNEL); > if (!opp_tables) > return -ENOMEM; > > - ret = sun50i_cpufreq_get_efuse(&speed); > - if (ret) > - return ret; > + speed = sun50i_cpufreq_get_efuse(match->data); > + if (speed < 0) > + return speed; > > snprintf(name, MAX_NAME_LEN, "speed%d", speed); > > @@ -163,8 +180,12 @@ static struct platform_driver sun50i_cpufreq_driver = { > }, > }; > > +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 }, > {} > }; > > @@ -198,9 +219,8 @@ static int __init sun50i_cpufreq_init(void) > if (unlikely(ret < 0)) > return ret; > > - sun50i_cpufreq_pdev = > - platform_device_register_simple("sun50i-cpufreq-nvmem", > - -1, NULL, 0); > + sun50i_cpufreq_pdev = platform_device_register_data(NULL, > + "sun50i-cpufreq-nvmem", -1, match, sizeof(*match)); The alignment here raises a checkpatch warning with --strict Maxime [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] [-- Attachment #2: Type: text/plain, Size: 176 bytes --] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-12-08 8:22 UTC|newest] Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-12-08 7:19 [PATCH v4 1/6] cpufreq: sun50i: add efuse_xlate to get efuse version Shuosheng Huang 2020-12-08 7:19 ` Shuosheng Huang 2020-12-08 7:29 ` Viresh Kumar 2020-12-08 7:29 ` Viresh Kumar 2020-12-08 8:21 ` Maxime Ripard [this message] 2020-12-08 8:21 ` Maxime Ripard 2020-12-14 0:46 ` Samuel Holland 2020-12-14 0:46 ` Samuel Holland
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20201208082150.jbpdlpcghbj3liqj@gilmour \ --to=maxime@cerno.tech \ --cc=huangshuosheng@allwinnertech.com \ --cc=jernej.skrabec@siol.net \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pm@vger.kernel.org \ --cc=rjw@rjwysocki.net \ --cc=samuel@sholland.org \ --cc=tiny.windzz@gmail.com \ --cc=viresh.kumar@linaro.org \ --cc=wens@csie.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.