| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAMuHMdVPSDdjGa=AF_9g_RMSv2iv862WVcrmAgvPay+ceNrzgQ@mail.gmail.com>
Date: Tue, 27 Feb 2024 17:34:21 +0100
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: Yoshinori Sato <ysato@...rs.sourceforge.jp>
Cc: linux-sh@...r.kernel.org, Damien Le Moal <dlemoal@...nel.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>, Conor Dooley <conor+dt@...nel.org>,
Geert Uytterhoeven <geert+renesas@...der.be>, Michael Turquette <mturquette@...libre.com>,
Stephen Boyd <sboyd@...nel.org>, Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
David Airlie <airlied@...il.com>, Daniel Vetter <daniel@...ll.ch>,
Thomas Gleixner <tglx@...utronix.de>, Lorenzo Pieralisi <lpieralisi@...nel.org>,
Krzysztof Wilczyński <kw@...ux.com>,
Bjorn Helgaas <bhelgaas@...gle.com>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Jiri Slaby <jirislaby@...nel.org>, Magnus Damm <magnus.damm@...il.com>,
Daniel Lezcano <daniel.lezcano@...aro.org>, Rich Felker <dalias@...c.org>,
John Paul Adrian Glaubitz <glaubitz@...sik.fu-berlin.de>, Lee Jones <lee@...nel.org>,
Helge Deller <deller@....de>, Heiko Stuebner <heiko@...ech.de>,
Jernej Skrabec <jernej.skrabec@...il.com>, Chris Morgan <macromorgan@...mail.com>,
Yang Xiwen <forbidden405@...mail.com>, Sebastian Reichel <sre@...nel.org>,
Linus Walleij <linus.walleij@...aro.org>, Randy Dunlap <rdunlap@...radead.org>,
Arnd Bergmann <arnd@...db.de>, Vlastimil Babka <vbabka@...e.cz>, Hyeonggon Yoo <42.hyeyoo@...il.com>,
David Rientjes <rientjes@...gle.com>, Baoquan He <bhe@...hat.com>,
Andrew Morton <akpm@...ux-foundation.org>, Guenter Roeck <linux@...ck-us.net>,
Stephen Rothwell <sfr@...b.auug.org.au>, Azeem Shaikh <azeemshaikh38@...il.com>,
Javier Martinez Canillas <javierm@...hat.com>, Max Filippov <jcmvbkbc@...il.com>,
Palmer Dabbelt <palmer@...osinc.com>, Bin Meng <bmeng@...ylab.org>,
Jonathan Corbet <corbet@....net>, Jacky Huang <ychuang3@...oton.com>,
Lukas Bulwahn <lukas.bulwahn@...il.com>, Biju Das <biju.das.jz@...renesas.com>,
Uwe Kleine-König <u.kleine-koenig@...gutronix.de>,
Sam Ravnborg <sam@...nborg.org>, Sergey Shtylyov <s.shtylyov@....ru>,
Michael Karcher <kernel@...rcher.dialup.fu-berlin.de>,
Laurent Pinchart <laurent.pinchart+renesas@...asonboard.com>, linux-ide@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-renesas-soc@...r.kernel.org, linux-clk@...r.kernel.org,
dri-devel@...ts.freedesktop.org, linux-pci@...r.kernel.org,
linux-serial@...r.kernel.org, linux-fbdev@...r.kernel.org
Subject: Re: [DO NOT MERGE v6 15/37] clk: renesas: Add SH7750/7751 CPG Driver
Hi Sato-san,
On Tue, Jan 9, 2024 at 9:24 AM Yoshinori Sato
<ysato@...rs.sourceforge.jp> wrote:
> Renesas SH7750 and SH7751 series CPG driver.
> This driver supported frequency control and clock gating.
>
> Signed-off-by: Yoshinori Sato <ysato@...rs.sourceforge.jp>
Thanks for your patch!
> --- a/drivers/clk/renesas/Kconfig
> +++ b/drivers/clk/renesas/Kconfig
> @@ -193,6 +196,10 @@ config CLK_SH73A0
> select CLK_RENESAS_CPG_MSTP
> select CLK_RENESAS_DIV6
>
> +config CLK_SH7750
> + bool "SH7750/7751 family clock support" if COMPILE_TEST
> + help
> + This is a driver for SH7750 / SH7751 CPG.
This is a duplicate of the below. Please drop it.
>
> # Family
> config CLK_RCAR_CPG_LIB
> @@ -223,6 +230,11 @@ config CLK_RZG2L
> bool "Renesas RZ/{G2L,G2UL,G3S,V2L} family clock support" if COMPILE_TEST
> select RESET_CONTROLLER
>
> +config CLK_SH7750
> + bool "Renesas SH7750/7751 family clock support" if COMPILE_TEST
> + help
> + This is a driver for SH7750 / SH7751 CPG.
> +
> # Generic
> config CLK_RENESAS_CPG_MSSR
> bool "CPG/MSSR clock support" if COMPILE_TEST
> --- /dev/null
> +++ b/drivers/clk/renesas/clk-sh7750.c
> +static int register_pll(struct device_node *node, struct cpg_priv *cpg)
> +{
> + const char *clk_name = node->name;
> + const char *parent_name;
> + struct clk_init_data init = {
> + .name = PLLOUT,
> + .ops = &pll_ops,
> + .flags = 0,
> + .num_parents = 1,
> + };
> + int ret;
> +
> + parent_name = of_clk_get_parent_name(node, 0);
> + init.parent_names = &parent_name;
> + cpg->hw.init = &init;
> +
> + ret = of_clk_hw_register(node, &cpg->hw);
> + if (ret < 0) {
> + pr_err("%s: failed to register %s pll clock (%d)\n",
> + __func__, clk_name, ret);
> + return ret;
> + }
> + if (ret < 0)
> + pr_err("%s: failed to add provider %s (%d)\n",
> + __func__, clk_name, ret);
Bogus check and error message.
> + return ret;
> +}
> +static int register_div(struct device_node *node, struct cpg_priv *cpg)
> +{
> + static const char * const divout[] = {
> + "fck", "bck", "ick",
> + };
> + static const char * const stbcrout[] = {
> + "sci_clk", "rtc_clk", "tmu012_clk", /* STBCR */
> + "scif_clk", "dmac_clk", /* STBCR */
> + "ubc_clk", "sq_clk", /* STBCR2 */
> + };
> + static const char * const clkstpout[] = {
> + "intc_clk", "tmu34_clk", "pcic_clk", /* CLKSTP00 */
> + };
> +
> + unsigned int i;
> + int ret;
> + struct clk_hw_onecell_data *data;
> + struct clk_hw *reg_hw;
> + int num_clk = ARRAY_SIZE(divout) + ARRAY_SIZE(stbcrout) + ARRAY_SIZE(clkstpout);
> +
> + data = kzalloc(struct_size(data, hws, num_clk + 1), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + num_clk = 0;
> + for (i = 0; i < ARRAY_SIZE(divout); i++) {
> + reg_hw = __clk_hw_register_divider(NULL, node, divout[i],
> + PLLOUT, NULL, NULL,
> + 0, cpg->frqcr, i * 3, 3,
> + CLK_DIVIDER_REG_16BIT,
> + (i == 0) ? pdiv_table : div_table,
> + &cpg->clklock);
> + if (IS_ERR(reg_hw)) {
> + ret = PTR_ERR(reg_hw);
> + goto error;
> + }
> + data->hws[num_clk++] = reg_hw;
> + }
> + for (i = 0; i < ARRAY_SIZE(stbcrout); i++) {
> + u32 off = (i < 5) ? STBCR : STBCR2;
> +
> + if (i >= 5 && !(cpg->feat & MSTP_CR2))
> + break;
Alternatively, you could set the maximum loop counter upfront
n = cpg->feat & MSTP_CR2 ? ARRAY_SIZE(stbcrout) : 5;
for (i = 0; i < n; i++) ...
> + reg_hw = __clk_hw_register_gate(NULL, node, stbcrout[i],
> + divout[0], NULL, NULL,
> + 0, cpg->frqcr + off, i % 5,
> + CLK_GATE_REG_8BIT | CLK_GATE_SET_TO_DISABLE,
> + &cpg->clklock);
> + if (IS_ERR(reg_hw)) {
> + ret = PTR_ERR(reg_hw);
> + goto error;
> + }
> + data->hws[num_clk++] = reg_hw;
> + }
> + if (cpg->feat & MSTP_CLKSTP) {
> + for (i = 0; i < ARRAY_SIZE(clkstpout); i++) {
> + if (i == 2 && !(cpg->feat & MSTP_CSTP2))
> + continue;
Set maximum loop counter upfront?
> + reg_hw = clk_hw_register_clkstp(node, clkstpout[i],
> + divout[0], cpg->clkstp00,
> + i, &cpg->clklock);
> + if (IS_ERR(reg_hw)) {
> + ret = PTR_ERR(reg_hw);
> + goto error;
> + }
> + data->hws[num_clk++] = reg_hw;
> + }
> + }
> + data->num = num_clk;
> + ret = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, data);
> + if (ret < 0)
> + goto error;
> + return 0;
> +
> +error:
> + pr_err("%pOF: failed to register clock (%d)\n",
> + node, ret);
> + for (num_clk--; num_clk >= 0; num_clk--)
> + kfree(data->hws[num_clk]);
> + kfree(data);
> + return ret;
> +}
> +
> +static struct cpg_priv *sh7750_cpg_setup(struct device_node *node, u32 feat)
> +{
> + unsigned int num_parents;
> + u32 mode;
> + struct cpg_priv *cpg;
> + int ret = 0;
> +
> + num_parents = of_clk_get_parent_count(node);
> + if (num_parents < 1) {
> + pr_err("%s: no parent found", node->name);
> + return ERR_PTR(-ENODEV);
> + }
Do you need num_parents?
> +
> + of_property_read_u32_index(node, "renesas,mode", 0, &mode);
mode may be used uninitialized, if "renesas,mode" is missing.
> + if (mode >= 7) {
> + pr_err("%s: Invalid clock mode setting (%u)\n",
> + node->name, mode);
> + return ERR_PTR(-EINVAL);
> + }
> +
> + cpg = kzalloc(sizeof(struct cpg_priv), GFP_KERNEL);
> + if (!cpg)
> + return ERR_PTR(-ENOMEM);
> +
> + cpg->frqcr = of_iomap(node, 0);
> + if (cpg->frqcr == NULL) {
> + pr_err("%pOF: failed to map divide register", node);
> + ret = -ENODEV;
> + goto cpg_free;
> + }
> +
> + if (feat & MSTP_CLKSTP) {
> + cpg->clkstp00 = of_iomap(node, 1);
> + if (cpg->clkstp00 == NULL) {
> + pr_err("%pOF: failed to map clkstp00 register", node);
> + ret = -ENODEV;
> + goto unmap_frqcr;
> + }
> + }
> + cpg->feat = feat;
> + cpg->mode = mode;
> +
> + ret = register_pll(node, cpg);
> + if (ret < 0)
> + goto unmap_clkstp00;
> +
> + ret = register_div(node, cpg);
> + if (ret < 0)
> + goto unmap_clkstp00;
> +
Perhaps "cpg_data = cpg;" here, and return an error code instead? ...
> + return cpg;
> +
> +unmap_clkstp00:
> + iounmap(cpg->clkstp00);
> +unmap_frqcr:
> + iounmap(cpg->frqcr);
> +cpg_free:
> + kfree(cpg);
> + return ERR_PTR(ret);
> +}
> +
> +static void __init sh7750_cpg_init(struct device_node *node)
> +{
> + cpg_data = sh7750_cpg_setup(node, cpg_feature[CPG_SH7750]);
> + if (IS_ERR(cpg_data))
> + cpg_data = NULL;
.. then all cpg_data handling can be removed here...
> +}
> +static int sh7750_cpg_probe(struct platform_device *pdev)
> +{
> + u32 feature;
> +
> + if (cpg_data)
> + return 0;
> + feature = *(u32 *)of_device_get_match_data(&pdev->dev);
> + cpg_data = sh7750_cpg_setup(pdev->dev.of_node, feature);
> + if (IS_ERR(cpg_data))
> + return PTR_ERR(cpg_data);
> + return 0;
.. and this can be simplified to
return sh7750_cpg_setup(...);
> +}
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68korg
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Powered by blists - more mailing lists