linux-kernel - Re: [RFC v3 06/16] clk: starfive: Add JH8100 System clock generator driver

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <deb23094f40df7df9e7330e95af4e64d.sboyd@kernel.org>
Date: Thu, 11 Apr 2024 00:45:43 -0700
From: Stephen Boyd <sboyd@...nel.org>
To: Sia Jee Heng <jeeheng.sia@...rfivetech.com>, aou@...s.berkeley.edu, conor@...nel.org, emil.renner.berthing@...onical.com, hal.feng@...rfivetech.com, kernel@...il.dk, krzysztof.kozlowski+dt@...aro.org, mturquette@...libre.com, p.zabel@...gutronix.de, palmer@...belt.com, paul.walmsley@...ive.com, robh+dt@...nel.org, xingyu.wu@...rfivetech.com
Cc: linux-riscv@...ts.infradead.org, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, linux-clk@...r.kernel.org, jeeheng.sia@...rfivetech.com, leyfoon.tan@...rfivetech.com
Subject: Re: [RFC v3 06/16] clk: starfive: Add JH8100 System clock generator driver

Quoting Sia Jee Heng (2024-01-10 05:31:18)
> diff --git a/drivers/clk/starfive/clk-starfive-jh8100-sys.c b/drivers/clk/starfive/clk-starfive-jh8100-sys.c
> new file mode 100644
> index 000000000000..6d7e750dce82
> --- /dev/null
> +++ b/drivers/clk/starfive/clk-starfive-jh8100-sys.c
> @@ -0,0 +1,415 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * StarFive JH8100 System Clock Driver
> + *
> + * Copyright (C) 2023 StarFive Technology Co., Ltd.
> + *
> + * Author: Jee Heng Sia <jeeheng.sia@...rfivetech.com>
> + *
> + */
> +
> +#include <linux/clk.h>

Drop this unused include.

> +#include <linux/auxiliary_bus.h>
> +#include <linux/clk-provider.h>
> +#include <linux/init.h>
> +#include <linux/io.h>

Is this include used in this file?

> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +#include <soc/starfive/reset-starfive-common.h>
> +
> +#include <dt-bindings/clock/starfive,jh8100-crg.h>
> +
> +#include "clk-starfive-jh8100.h"
> +
> +#define JH8100_SYSCLK_NUM_CLKS                 (JH8100_SYSCLK_NNE_ICG_EN + 1)
> +
[...]
> +
> +static void jh8100_reset_adev_release(struct device *dev)
> +{
> +       struct auxiliary_device *adev = to_auxiliary_dev(dev);
> +       struct starfive_reset_adev *rdev = to_starfive_reset_adev(adev);
> +
> +       kfree(rdev);
> +}
> +
> +int jh8100_reset_controller_register(struct starfive_clk_priv *priv,

Just pass 'dev' and 'base' instead.

> +                                    const char *adev_name,
> +                                    u32 adev_id)
> +{
> +       struct starfive_reset_adev *rdev;
> +       struct auxiliary_device *adev;
> +       int ret;
> +
> +       rdev = kzalloc(sizeof(*rdev), GFP_KERNEL);
> +       if (!rdev)
> +               return -ENOMEM;
> +
> +       rdev->base = priv->base;
> +
> +       adev = &rdev->adev;
> +       adev->name = adev_name;
> +       adev->dev.parent = priv->dev;
> +       adev->dev.release = jh8100_reset_adev_release;
> +       adev->id = adev_id;
> +
> +       ret = auxiliary_device_init(adev);
> +       if (ret)
> +               return ret;
> +
> +       ret = auxiliary_device_add(adev);
> +       if (ret) {
> +               auxiliary_device_uninit(adev);
> +               return ret;
> +       }
> +
> +       return devm_add_action_or_reset(priv->dev,
> +                                       jh8100_reset_unregister_adev, adev);
> +}
> +EXPORT_SYMBOL_GPL(jh8100_reset_controller_register);

Move this to drivers/reset/ please.

> +
> +static int __init jh8100_syscrg_probe(struct platform_device *pdev)
> +{
> +       struct starfive_clk_priv *priv;
> +       unsigned int idx;
> +       int ret;
> +
> +       priv = devm_kzalloc(&pdev->dev,
> +                           struct_size(priv, reg, JH8100_SYSCLK_NUM_CLKS),
> +                           GFP_KERNEL);
> +       if (!priv)
> +               return -ENOMEM;
> +
> +       spin_lock_init(&priv->rmw_lock);
> +       priv->dev = &pdev->dev;
> +       priv->base = devm_platform_ioremap_resource(pdev, 0);
> +       if (IS_ERR(priv->base))
> +               return PTR_ERR(priv->base);
[...]
> +
> +       ret = devm_of_clk_add_hw_provider(&pdev->dev, jh8100_sysclk_get, priv);
> +       if (ret)
> +               return ret;
> +
> +       return jh8100_reset_controller_register(priv, "rst-sys", 0);
> +}
> +
> +static const struct of_device_id jh8100_syscrg_match[] = {
> +       { .compatible = "starfive,jh8100-syscrg" },
> +       { /* sentinel */ }
> +};
> +
> +static struct platform_driver jh8100_syscrg_driver = {
> +       .driver = {
> +               .name = "clk-starfive-jh8100-sys",
> +               .of_match_table = jh8100_syscrg_match,
> +               .suppress_bind_attrs = true,
> +       },
> +};
> +builtin_platform_driver_probe(jh8100_syscrg_driver, jh8100_syscrg_probe);
> diff --git a/drivers/clk/starfive/clk-starfive-jh8100.h b/drivers/clk/starfive/clk-starfive-jh8100.h
> new file mode 100644
> index 000000000000..9b69a56fe5fc
> --- /dev/null
> +++ b/drivers/clk/starfive/clk-starfive-jh8100.h
> @@ -0,0 +1,11 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __CLK_STARFIVE_JH8100_H
> +#define __CLK_STARFIVE_JH8100_H
> +
> +#include "clk-starfive-common.h"

Drop this include.

> +
> +int jh8100_reset_controller_register(struct starfive_clk_priv *priv,

Forward declare starfive_clk_priv instead.

> +                                    const char *adev_name,
> +                                    u32 adev_id);

Why is this header here at all?