RE: [RFC PATCH 2/4] clk: renesas: rzg2l-cpg: Add support to stack the resets instead of indexing

[Biju Das]

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@xxxxxxxxxxxxxx>
> ---
>  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