[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <OS0PR01MB592225B5A5E1BA7D250C466786C29@OS0PR01MB5922.jpnprd01.prod.outlook.com>
Date: Thu, 5 May 2022 19:48:18 +0000
From: Biju Das <biju.das.jz@...renesas.com>
To: Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@...renesas.com>,
Geert Uytterhoeven <geert+renesas@...der.be>,
Michael Turquette <mturquette@...libre.com>,
Stephen Boyd <sboyd@...nel.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
"linux-clk@...r.kernel.org" <linux-clk@...r.kernel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>
CC: "linux-renesas-soc@...r.kernel.org"
<linux-renesas-soc@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Prabhakar <prabhakar.csengg@...il.com>,
Phil Edworthy <phil.edworthy@...esas.com>,
Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@...renesas.com>
Subject: RE: [RFC PATCH 2/4] clk: renesas: rzg2l-cpg: Add support to stack the
resets instead of indexing
Hi Lad Prabhakar,
Thanks for the patch.
> Subject: [RFC PATCH 2/4] clk: renesas: rzg2l-cpg: Add support to stack the
> resets instead of indexing
>
> Instead of indexing the resets, stack them and instead create an id member
> in struct rzg2l_reset to store the index. With this approach for every id
> we will have to loop through the resets array to match the id.
>
> This in preparation to add support for Renesas RZ/Five CPG in r9a07g043-
> cpg.c file where the resets array will be split up into three i.e. common
> and two SoC specific.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
> ---
> drivers/clk/renesas/rzg2l-cpg.c | 76 ++++++++++++++++++++++++++-------
> drivers/clk/renesas/rzg2l-cpg.h | 4 +-
> 2 files changed, 63 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/clk/renesas/rzg2l-cpg.c b/drivers/clk/renesas/rzg2l-
> cpg.c index 1ce35f65682b..94fe307ec4c5 100644
> --- a/drivers/clk/renesas/rzg2l-cpg.c
> +++ b/drivers/clk/renesas/rzg2l-cpg.c
> @@ -681,14 +681,37 @@ rzg2l_cpg_register_mod_clk(const struct rzg2l_mod_clk
> *mod,
>
> #define rcdev_to_priv(x) container_of(x, struct rzg2l_cpg_priv,
> rcdev)
>
> +static const struct rzg2l_reset
> +*rzg2l_get_reset_ptr(struct rzg2l_cpg_priv *priv,
> + unsigned long id)
> +
> +{
> + const struct rzg2l_cpg_info *info = priv->info;
> + unsigned int i;
> +
> + for (i = 0; i < priv->num_resets; i++) {
> + if (info->resets[i].id == id)
> + return &info->resets[i];
> + }
Is it not possible to use shared reset like RZ/G2L and RZ/V2L?, which
has optimal memory and performance wise we can avoid bigger loop.
Like adding Last index of RZ/Five as last reset index and
Handle RZ/G2UL specific as invalid reset index in xlate??
> +
> + return NULL;
> +}
> +
> static int rzg2l_cpg_reset(struct reset_controller_dev *rcdev,
> unsigned long id)
> {
> struct rzg2l_cpg_priv *priv = rcdev_to_priv(rcdev);
> - const struct rzg2l_cpg_info *info = priv->info;
> - unsigned int reg = info->resets[id].off;
> - u32 dis = BIT(info->resets[id].bit);
> - u32 we = dis << 16;
> + const struct rzg2l_reset *reset;
> + unsigned int reg;
> + u32 dis, we;
> +
> + reset = rzg2l_get_reset_ptr(priv, id);
> + if (!reset)
> + return -EINVAL;
> +
> + reg = reset->off;
> + dis = BIT(reset->bit);
> + we = dis << 16;
>
> dev_dbg(rcdev->dev, "reset id:%ld offset:0x%x\n", id,
> CLK_RST_R(reg));
>
> @@ -708,9 +731,16 @@ static int rzg2l_cpg_assert(struct
> reset_controller_dev *rcdev,
> unsigned long id)
> {
> struct rzg2l_cpg_priv *priv = rcdev_to_priv(rcdev);
> - const struct rzg2l_cpg_info *info = priv->info;
> - unsigned int reg = info->resets[id].off;
> - u32 value = BIT(info->resets[id].bit) << 16;
> + const struct rzg2l_reset *reset;
> + unsigned int reg;
> + u32 value;
> +
> + reset = rzg2l_get_reset_ptr(priv, id);
> + if (!reset)
> + return -EINVAL;
> +
> + reg = reset->off;
> + value = BIT(reset->bit) << 16;
>
> dev_dbg(rcdev->dev, "assert id:%ld offset:0x%x\n", id,
> CLK_RST_R(reg));
>
> @@ -722,11 +752,17 @@ static int rzg2l_cpg_deassert(struct
> reset_controller_dev *rcdev,
> unsigned long id)
> {
> struct rzg2l_cpg_priv *priv = rcdev_to_priv(rcdev);
> - const struct rzg2l_cpg_info *info = priv->info;
> - unsigned int reg = info->resets[id].off;
> - u32 dis = BIT(info->resets[id].bit);
> - u32 value = (dis << 16) | dis;
> + const struct rzg2l_reset *reset;
> + unsigned int reg;
> + u32 dis, value;
> +
> + reset = rzg2l_get_reset_ptr(priv, id);
> + if (!reset)
> + return -EINVAL;
>
> + reg = reset->off;
> + dis = BIT(reset->bit);
> + value = (dis << 16) | dis;
> dev_dbg(rcdev->dev, "deassert id:%ld offset:0x%x\n", id,
> CLK_RST_R(reg));
>
> @@ -738,9 +774,16 @@ static int rzg2l_cpg_status(struct
> reset_controller_dev *rcdev,
> unsigned long id)
> {
> struct rzg2l_cpg_priv *priv = rcdev_to_priv(rcdev);
> - const struct rzg2l_cpg_info *info = priv->info;
> - unsigned int reg = info->resets[id].off;
> - u32 bitmask = BIT(info->resets[id].bit);
> + const struct rzg2l_reset *reset;
> + unsigned int reg;
> + u32 bitmask;
> +
> + reset = rzg2l_get_reset_ptr(priv, id);
> + if (!reset)
> + return -EINVAL;
> +
> + reg = reset->off;
> + bitmask = BIT(reset->bit);
>
> return !(readl(priv->base + CLK_MRST_R(reg)) & bitmask); } @@ -
> 756,10 +799,11 @@ static int rzg2l_cpg_reset_xlate(struct
> reset_controller_dev *rcdev,
> const struct of_phandle_args *reset_spec) {
> struct rzg2l_cpg_priv *priv = rcdev_to_priv(rcdev);
> - const struct rzg2l_cpg_info *info = priv->info;
> unsigned int id = reset_spec->args[0];
> + const struct rzg2l_reset *reset;
>
> - if (id >= rcdev->nr_resets || !info->resets[id].off) {
> + reset = rzg2l_get_reset_ptr(priv, id);
> + if (!reset) {
> dev_err(rcdev->dev, "Invalid reset index %u\n", id);
> return -EINVAL;
> }
> diff --git a/drivers/clk/renesas/rzg2l-cpg.h b/drivers/clk/renesas/rzg2l-
> cpg.h index 92c88f42ca7f..a99f2ba7868f 100644
> --- a/drivers/clk/renesas/rzg2l-cpg.h
> +++ b/drivers/clk/renesas/rzg2l-cpg.h
> @@ -152,12 +152,14 @@ struct rzg2l_mod_clk {
> * @bit: reset bit
> */
> struct rzg2l_reset {
> + unsigned int id;
Now you are adding 4 bytes to each reset entry in the LUT.
Cheers,
Biju
> u16 off;
> u8 bit;
> };
>
> #define DEF_RST(_id, _off, _bit) \
> - [_id] = { \
> + { \
> + .id = (_id), \
> .off = (_off), \
> .bit = (_bit) \
> }
> --
> 2.25.1
Powered by blists - more mailing lists