[PATCH v2 00/15] iio: adc: rzg2l_adc: Add support for RZ/G3S

[PATCH v2 00/15] iio: adc: rzg2l_adc: Add support for RZ/G3S

[Claudiu]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Hi,

This series adds ADC support for the Renesas RZ/G3S SoC.

Series is organized as follows:
- patch 01/15:		adds clocks, reset and power domain support
			for ADC
- patches 02-07/15:	cleanup patches to ease the addition of RZ/G3S
			support
- patches 08/15:	enables runtime PM autosuspend support
- patches 09-13/15:	add RZ/G3S support, including suspend-to-RAM
			functionality
- patches 14-15/15:	add device tree support

Merge strategy, if any:
- patch 01/15 can go through the Renesas tree
- patches 02-13/15 can go through the IIO tree
- patch 14-15/15 can go through the Renesas tree

Thank you,
Claudiu Beznea

Changes in v2:
- added patch "iio: adc: rzg2l_adc: Convert dev_err() to dev_err_probe()"
  as requested in the review process
- addressed review comments
- collected tags
- each patch includes a detailed description of its changes


Claudiu Beznea (15):
  clk: renesas: r9a08g045: Add clocks, resets and power domain support
    for the ADC IP
  iio: adc: rzg2l_adc: Convert dev_err() to dev_err_probe()
  iio: adc: rzg2l_adc: Use devres helpers to request pre-deasserted
    reset controls
  iio: adc: rzg2l_adc: Simplify the runtime PM code
  iio: adc: rzg2l_adc: Switch to RUNTIME_PM_OPS() and pm_ptr()
  iio: adc: rzg2l_adc: Use read_poll_timeout()
  iio: adc: rzg2l_adc: Simplify the locking scheme in
    rzg2l_adc_read_raw()
  iio: adc: rzg2l_adc: Enable runtime PM autosuspend support
  iio: adc: rzg2l_adc: Prepare for the addition of RZ/G3S support
  iio: adc: rzg2l_adc: Add support for channel 8
  iio: adc: rzg2l_adc: Add suspend/resume support
  dt-bindings: iio: adc: renesas,rzg2l-adc: Document RZ/G3S SoC
  iio: adc: rzg2l_adc: Add support for Renesas RZ/G3S
  arm64: dts: renesas: r9a08g045: Add ADC node
  arm64: dts: renesas: rzg3s-smarc-som: Enable ADC

 .../bindings/iio/adc/renesas,rzg2l-adc.yaml   |  37 +-
 arch/arm64/boot/dts/renesas/r9a08g045.dtsi    |  53 +++
 .../boot/dts/renesas/rzg3s-smarc-som.dtsi     |   4 +
 drivers/clk/renesas/r9a08g045-cpg.c           |   7 +
 drivers/iio/adc/rzg2l_adc.c                   | 423 ++++++++++--------
 5 files changed, 316 insertions(+), 208 deletions(-)

-- 
2.39.2

[PATCH v2 01/15] clk: renesas: r9a08g045: Add clocks, resets and power domain support for the ADC IP

[Claudiu]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Add clocks, resets and power domains for ADC IP available on the Renesas
RZ/G3S SoC.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v2:
- rebased on top of the latest r9a08g045-cpg version

 drivers/clk/renesas/r9a08g045-cpg.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/clk/renesas/r9a08g045-cpg.c b/drivers/clk/renesas/r9a08g045-cpg.c
index 559afc417c6c..0e7e3bf05b52 100644
--- a/drivers/clk/renesas/r9a08g045-cpg.c
+++ b/drivers/clk/renesas/r9a08g045-cpg.c
@@ -187,6 +187,7 @@ static const struct cpg_core_clk r9a08g045_core_clks[] __initconst = {
 	DEF_FIXED("OSC", R9A08G045_OSCCLK, CLK_EXTAL, 1, 1),
 	DEF_FIXED("OSC2", R9A08G045_OSCCLK2, CLK_EXTAL, 1, 3),
 	DEF_FIXED("HP", R9A08G045_CLK_HP, CLK_PLL6, 1, 2),
+	DEF_FIXED("TSU", R9A08G045_CLK_TSU, CLK_PLL2_DIV2, 1, 8),
 };
 
 static const struct rzg2l_mod_clk r9a08g045_mod_clks[] = {
@@ -238,6 +239,8 @@ static const struct rzg2l_mod_clk r9a08g045_mod_clks[] = {
 	DEF_MOD("scif4_clk_pck",	R9A08G045_SCIF4_CLK_PCK, R9A08G045_CLK_P0, 0x584, 4),
 	DEF_MOD("scif5_clk_pck",	R9A08G045_SCIF5_CLK_PCK, R9A08G045_CLK_P0, 0x584, 5),
 	DEF_MOD("gpio_hclk",		R9A08G045_GPIO_HCLK, R9A08G045_OSCCLK, 0x598, 0),
+	DEF_MOD("adc_adclk",		R9A08G045_ADC_ADCLK, R9A08G045_CLK_TSU, 0x5a8, 0),
+	DEF_MOD("adc_pclk",		R9A08G045_ADC_PCLK, R9A08G045_CLK_TSU, 0x5a8, 1),
 	DEF_MOD("vbat_bclk",		R9A08G045_VBAT_BCLK, R9A08G045_OSCCLK, 0x614, 0),
 };
 
@@ -274,6 +277,8 @@ static const struct rzg2l_reset r9a08g045_resets[] = {
 	DEF_RST(R9A08G045_GPIO_RSTN, 0x898, 0),
 	DEF_RST(R9A08G045_GPIO_PORT_RESETN, 0x898, 1),
 	DEF_RST(R9A08G045_GPIO_SPARE_RESETN, 0x898, 2),
+	DEF_RST(R9A08G045_ADC_PRESETN, 0x8a8, 0),
+	DEF_RST(R9A08G045_ADC_ADRST_N, 0x8a8, 1),
 	DEF_RST(R9A08G045_VBAT_BRESETN, 0x914, 0),
 };
 
@@ -346,6 +351,8 @@ static const struct rzg2l_cpg_pm_domain_init_data r9a08g045_pm_domains[] = {
 				DEF_REG_CONF(CPG_BUS_MCPU2_MSTOP, BIT(5)), 0),
 	DEF_PD("scif5",		R9A08G045_PD_SCIF5,
 				DEF_REG_CONF(CPG_BUS_MCPU3_MSTOP, BIT(4)), 0),
+	DEF_PD("adc",		R9A08G045_PD_ADC,
+				DEF_REG_CONF(CPG_BUS_MCPU2_MSTOP, BIT(14)), 0),
 	DEF_PD("vbat",		R9A08G045_PD_VBAT,
 				DEF_REG_CONF(CPG_BUS_MCPU3_MSTOP, BIT(8)),
 				GENPD_FLAG_ALWAYS_ON),
-- 
2.39.2

Re: [PATCH v2 01/15] clk: renesas: r9a08g045: Add clocks, resets and power domain support for the ADC IP

[Geert Uytterhoeven]

On Fri, Dec 6, 2024 at 12:13 PM Claudiu <claudiu.beznea@tuxon.dev> wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> Add clocks, resets and power domains for ADC IP available on the Renesas
> RZ/G3S SoC.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
>
> Changes in v2:
> - rebased on top of the latest r9a08g045-cpg version

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-clk for v6.14.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

Re: [PATCH v2 01/15] clk: renesas: r9a08g045: Add clocks, resets and power domain support for the ADC IP

[Lad, Prabhakar]

On Fri, Dec 6, 2024 at 11:14 AM Claudiu <claudiu.beznea@tuxon.dev> wrote:
>
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> Add clocks, resets and power domains for ADC IP available on the Renesas
> RZ/G3S SoC.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
>
> Changes in v2:
> - rebased on top of the latest r9a08g045-cpg version
>
>  drivers/clk/renesas/r9a08g045-cpg.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Cheers,
Prabhakar

> diff --git a/drivers/clk/renesas/r9a08g045-cpg.c b/drivers/clk/renesas/r9a08g045-cpg.c
> index 559afc417c6c..0e7e3bf05b52 100644
> --- a/drivers/clk/renesas/r9a08g045-cpg.c
> +++ b/drivers/clk/renesas/r9a08g045-cpg.c
> @@ -187,6 +187,7 @@ static const struct cpg_core_clk r9a08g045_core_clks[] __initconst = {
>         DEF_FIXED("OSC", R9A08G045_OSCCLK, CLK_EXTAL, 1, 1),
>         DEF_FIXED("OSC2", R9A08G045_OSCCLK2, CLK_EXTAL, 1, 3),
>         DEF_FIXED("HP", R9A08G045_CLK_HP, CLK_PLL6, 1, 2),
> +       DEF_FIXED("TSU", R9A08G045_CLK_TSU, CLK_PLL2_DIV2, 1, 8),
>  };
>
>  static const struct rzg2l_mod_clk r9a08g045_mod_clks[] = {
> @@ -238,6 +239,8 @@ static const struct rzg2l_mod_clk r9a08g045_mod_clks[] = {
>         DEF_MOD("scif4_clk_pck",        R9A08G045_SCIF4_CLK_PCK, R9A08G045_CLK_P0, 0x584, 4),
>         DEF_MOD("scif5_clk_pck",        R9A08G045_SCIF5_CLK_PCK, R9A08G045_CLK_P0, 0x584, 5),
>         DEF_MOD("gpio_hclk",            R9A08G045_GPIO_HCLK, R9A08G045_OSCCLK, 0x598, 0),
> +       DEF_MOD("adc_adclk",            R9A08G045_ADC_ADCLK, R9A08G045_CLK_TSU, 0x5a8, 0),
> +       DEF_MOD("adc_pclk",             R9A08G045_ADC_PCLK, R9A08G045_CLK_TSU, 0x5a8, 1),
>         DEF_MOD("vbat_bclk",            R9A08G045_VBAT_BCLK, R9A08G045_OSCCLK, 0x614, 0),
>  };
>
> @@ -274,6 +277,8 @@ static const struct rzg2l_reset r9a08g045_resets[] = {
>         DEF_RST(R9A08G045_GPIO_RSTN, 0x898, 0),
>         DEF_RST(R9A08G045_GPIO_PORT_RESETN, 0x898, 1),
>         DEF_RST(R9A08G045_GPIO_SPARE_RESETN, 0x898, 2),
> +       DEF_RST(R9A08G045_ADC_PRESETN, 0x8a8, 0),
> +       DEF_RST(R9A08G045_ADC_ADRST_N, 0x8a8, 1),
>         DEF_RST(R9A08G045_VBAT_BRESETN, 0x914, 0),
>  };
>
> @@ -346,6 +351,8 @@ static const struct rzg2l_cpg_pm_domain_init_data r9a08g045_pm_domains[] = {
>                                 DEF_REG_CONF(CPG_BUS_MCPU2_MSTOP, BIT(5)), 0),
>         DEF_PD("scif5",         R9A08G045_PD_SCIF5,
>                                 DEF_REG_CONF(CPG_BUS_MCPU3_MSTOP, BIT(4)), 0),
> +       DEF_PD("adc",           R9A08G045_PD_ADC,
> +                               DEF_REG_CONF(CPG_BUS_MCPU2_MSTOP, BIT(14)), 0),
>         DEF_PD("vbat",          R9A08G045_PD_VBAT,
>                                 DEF_REG_CONF(CPG_BUS_MCPU3_MSTOP, BIT(8)),
>                                 GENPD_FLAG_ALWAYS_ON),
> --
> 2.39.2
>
>

[PATCH v2 02/15] iio: adc: rzg2l_adc: Convert dev_err() to dev_err_probe()

[Claudiu]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Convert all occurrences of dev_err() in the probe path to dev_err_probe().
This improves readability and simplifies the code.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v2:
- none, this patch is new

 drivers/iio/adc/rzg2l_adc.c | 64 +++++++++++++------------------------
 1 file changed, 22 insertions(+), 42 deletions(-)

diff --git a/drivers/iio/adc/rzg2l_adc.c b/drivers/iio/adc/rzg2l_adc.c
index cd3a7e46ea53..8a804f81c04b 100644
--- a/drivers/iio/adc/rzg2l_adc.c
+++ b/drivers/iio/adc/rzg2l_adc.c
@@ -313,15 +313,11 @@ static int rzg2l_adc_parse_properties(struct platform_device *pdev, struct rzg2l
 		return -ENOMEM;
 
 	num_channels = device_get_child_node_count(&pdev->dev);
-	if (!num_channels) {
-		dev_err(&pdev->dev, "no channel children\n");
-		return -ENODEV;
-	}
+	if (!num_channels)
+		return dev_err_probe(&pdev->dev, -ENODEV, "no channel children\n");
 
-	if (num_channels > RZG2L_ADC_MAX_CHANNELS) {
-		dev_err(&pdev->dev, "num of channel children out of range\n");
-		return -EINVAL;
-	}
+	if (num_channels > RZG2L_ADC_MAX_CHANNELS)
+		return dev_err_probe(&pdev->dev, -EINVAL, "num of channel children out of range\n");
 
 	chan_array = devm_kcalloc(&pdev->dev, num_channels, sizeof(*chan_array),
 				  GFP_KERNEL);
@@ -445,62 +441,46 @@ static int rzg2l_adc_probe(struct platform_device *pdev)
 		return PTR_ERR(adc->base);
 
 	adc->pclk = devm_clk_get(dev, "pclk");
-	if (IS_ERR(adc->pclk)) {
-		dev_err(dev, "Failed to get pclk");
-		return PTR_ERR(adc->pclk);
-	}
+	if (IS_ERR(adc->pclk))
+		return dev_err_probe(dev, PTR_ERR(adc->pclk), "Failed to get pclk");
 
 	adc->adclk = devm_clk_get(dev, "adclk");
-	if (IS_ERR(adc->adclk)) {
-		dev_err(dev, "Failed to get adclk");
-		return PTR_ERR(adc->adclk);
-	}
+	if (IS_ERR(adc->adclk))
+		return dev_err_probe(dev, PTR_ERR(adc->adclk), "Failed to get adclk");
 
 	adc->adrstn = devm_reset_control_get_exclusive(dev, "adrst-n");
-	if (IS_ERR(adc->adrstn)) {
-		dev_err(dev, "failed to get adrstn\n");
-		return PTR_ERR(adc->adrstn);
-	}
+	if (IS_ERR(adc->adrstn))
+		return dev_err_probe(dev, PTR_ERR(adc->adrstn), "failed to get adrstn\n");
 
 	adc->presetn = devm_reset_control_get_exclusive(dev, "presetn");
-	if (IS_ERR(adc->presetn)) {
-		dev_err(dev, "failed to get presetn\n");
-		return PTR_ERR(adc->presetn);
-	}
+	if (IS_ERR(adc->presetn))
+		return dev_err_probe(dev, PTR_ERR(adc->presetn), "failed to get presetn\n");
 
 	ret = reset_control_deassert(adc->adrstn);
-	if (ret) {
-		dev_err(&pdev->dev, "failed to deassert adrstn pin, %d\n", ret);
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "failed to deassert adrstn pin, %d\n", ret);
 
 	ret = devm_add_action_or_reset(&pdev->dev,
 				       rzg2l_adc_reset_assert, adc->adrstn);
 	if (ret) {
-		dev_err(&pdev->dev, "failed to register adrstn assert devm action, %d\n",
-			ret);
-		return ret;
+		return dev_err_probe(&pdev->dev, ret,
+				     "failed to register adrstn assert devm action, %d\n", ret);
 	}
 
 	ret = reset_control_deassert(adc->presetn);
-	if (ret) {
-		dev_err(&pdev->dev, "failed to deassert presetn pin, %d\n", ret);
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "failed to deassert presetn pin, %d\n", ret);
 
 	ret = devm_add_action_or_reset(&pdev->dev,
 				       rzg2l_adc_reset_assert, adc->presetn);
 	if (ret) {
-		dev_err(&pdev->dev, "failed to register presetn assert devm action, %d\n",
-			ret);
-		return ret;
+		return dev_err_probe(&pdev->dev, ret,
+				     "failed to register presetn assert devm action, %d\n", ret);
 	}
 
 	ret = rzg2l_adc_hw_init(adc);
-	if (ret) {
-		dev_err(&pdev->dev, "failed to initialize ADC HW, %d\n", ret);
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "failed to initialize ADC HW, %d\n", ret);
 
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0)
-- 
2.39.2

Re: [PATCH v2 02/15] iio: adc: rzg2l_adc: Convert dev_err() to dev_err_probe()

[Jonathan Cameron]

On Fri,  6 Dec 2024 13:13:24 +0200
Claudiu <claudiu.beznea@tuxon.dev> wrote:

> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> Convert all occurrences of dev_err() in the probe path to dev_err_probe().
> This improves readability and simplifies the code.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Hi Claudiu,

> +	if (IS_ERR(adc->presetn))
> +		return dev_err_probe(dev, PTR_ERR(adc->presetn), "failed to get presetn\n");
>  
>  	ret = reset_control_deassert(adc->adrstn);
> -	if (ret) {
> -		dev_err(&pdev->dev, "failed to deassert adrstn pin, %d\n", ret);
> -		return ret;
> -	}
> +	if (ret)
> +		return dev_err_probe(&pdev->dev, ret, "failed to deassert adrstn pin, %d\n", ret);

Take a closer look at the implementation of dev_err_probe().
It already prints the return code (where appropriate) and in a pretty text form
which is easier to read.  So we should not print it again.

I'd also prefer wrapping some of the longer lines in here a little earlier.  For IIO
I still prefer to stay under 80 chars or only a little over it where it doesn't
hurt readability.

Jonathan

Re: [PATCH v2 02/15] iio: adc: rzg2l_adc: Convert dev_err() to dev_err_probe()

[Lad, Prabhakar]

On Fri, Dec 6, 2024 at 11:14 AM Claudiu <claudiu.beznea@tuxon.dev> wrote:
>
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> Convert all occurrences of dev_err() in the probe path to dev_err_probe().
> This improves readability and simplifies the code.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
>
> Changes in v2:
> - none, this patch is new
>
>  drivers/iio/adc/rzg2l_adc.c | 64 +++++++++++++------------------------
>  1 file changed, 22 insertions(+), 42 deletions(-)
>
With comments fixed from Jonathan,

Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Cheers,
Prabhakar

> diff --git a/drivers/iio/adc/rzg2l_adc.c b/drivers/iio/adc/rzg2l_adc.c
> index cd3a7e46ea53..8a804f81c04b 100644
> --- a/drivers/iio/adc/rzg2l_adc.c
> +++ b/drivers/iio/adc/rzg2l_adc.c
> @@ -313,15 +313,11 @@ static int rzg2l_adc_parse_properties(struct platform_device *pdev, struct rzg2l
>                 return -ENOMEM;
>
>         num_channels = device_get_child_node_count(&pdev->dev);
> -       if (!num_channels) {
> -               dev_err(&pdev->dev, "no channel children\n");
> -               return -ENODEV;
> -       }
> +       if (!num_channels)
> +               return dev_err_probe(&pdev->dev, -ENODEV, "no channel children\n");
>
> -       if (num_channels > RZG2L_ADC_MAX_CHANNELS) {
> -               dev_err(&pdev->dev, "num of channel children out of range\n");
> -               return -EINVAL;
> -       }
> +       if (num_channels > RZG2L_ADC_MAX_CHANNELS)
> +               return dev_err_probe(&pdev->dev, -EINVAL, "num of channel children out of range\n");
>
>         chan_array = devm_kcalloc(&pdev->dev, num_channels, sizeof(*chan_array),
>                                   GFP_KERNEL);
> @@ -445,62 +441,46 @@ static int rzg2l_adc_probe(struct platform_device *pdev)
>                 return PTR_ERR(adc->base);
>
>         adc->pclk = devm_clk_get(dev, "pclk");
> -       if (IS_ERR(adc->pclk)) {
> -               dev_err(dev, "Failed to get pclk");
> -               return PTR_ERR(adc->pclk);
> -       }
> +       if (IS_ERR(adc->pclk))
> +               return dev_err_probe(dev, PTR_ERR(adc->pclk), "Failed to get pclk");
>
>         adc->adclk = devm_clk_get(dev, "adclk");
> -       if (IS_ERR(adc->adclk)) {
> -               dev_err(dev, "Failed to get adclk");
> -               return PTR_ERR(adc->adclk);
> -       }
> +       if (IS_ERR(adc->adclk))
> +               return dev_err_probe(dev, PTR_ERR(adc->adclk), "Failed to get adclk");
>
>         adc->adrstn = devm_reset_control_get_exclusive(dev, "adrst-n");
> -       if (IS_ERR(adc->adrstn)) {
> -               dev_err(dev, "failed to get adrstn\n");
> -               return PTR_ERR(adc->adrstn);
> -       }
> +       if (IS_ERR(adc->adrstn))
> +               return dev_err_probe(dev, PTR_ERR(adc->adrstn), "failed to get adrstn\n");
>
>         adc->presetn = devm_reset_control_get_exclusive(dev, "presetn");
> -       if (IS_ERR(adc->presetn)) {
> -               dev_err(dev, "failed to get presetn\n");
> -               return PTR_ERR(adc->presetn);
> -       }
> +       if (IS_ERR(adc->presetn))
> +               return dev_err_probe(dev, PTR_ERR(adc->presetn), "failed to get presetn\n");
>
>         ret = reset_control_deassert(adc->adrstn);
> -       if (ret) {
> -               dev_err(&pdev->dev, "failed to deassert adrstn pin, %d\n", ret);
> -               return ret;
> -       }
> +       if (ret)
> +               return dev_err_probe(&pdev->dev, ret, "failed to deassert adrstn pin, %d\n", ret);
>
>         ret = devm_add_action_or_reset(&pdev->dev,
>                                        rzg2l_adc_reset_assert, adc->adrstn);
>         if (ret) {
> -               dev_err(&pdev->dev, "failed to register adrstn assert devm action, %d\n",
> -                       ret);
> -               return ret;
> +               return dev_err_probe(&pdev->dev, ret,
> +                                    "failed to register adrstn assert devm action, %d\n", ret);
>         }
>
>         ret = reset_control_deassert(adc->presetn);
> -       if (ret) {
> -               dev_err(&pdev->dev, "failed to deassert presetn pin, %d\n", ret);
> -               return ret;
> -       }
> +       if (ret)
> +               return dev_err_probe(&pdev->dev, ret, "failed to deassert presetn pin, %d\n", ret);
>
>         ret = devm_add_action_or_reset(&pdev->dev,
>                                        rzg2l_adc_reset_assert, adc->presetn);
>         if (ret) {
> -               dev_err(&pdev->dev, "failed to register presetn assert devm action, %d\n",
> -                       ret);
> -               return ret;
> +               return dev_err_probe(&pdev->dev, ret,
> +                                    "failed to register presetn assert devm action, %d\n", ret);
>         }
>
>         ret = rzg2l_adc_hw_init(adc);
> -       if (ret) {
> -               dev_err(&pdev->dev, "failed to initialize ADC HW, %d\n", ret);
> -               return ret;
> -       }
> +       if (ret)
> +               return dev_err_probe(&pdev->dev, ret, "failed to initialize ADC HW, %d\n", ret);
>
>         irq = platform_get_irq(pdev, 0);
>         if (irq < 0)
> --
> 2.39.2
>
>

[PATCH v2 03/15] iio: adc: rzg2l_adc: Use devres helpers to request pre-deasserted reset controls

[Claudiu]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Starting with commit d872bed85036 ("reset: Add devres helpers to request
pre-deasserted reset controls"), devres helpers are available to simplify
the process of requesting pre-deasserted reset controls. Update the
rzg2l_adc driver to utilize these helpers, reducing complexity in this
way.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v2:
- rebased on top of patch 2/15 from this version
- used "failed to get/deassert" failure messages

 drivers/iio/adc/rzg2l_adc.c | 37 ++++++-------------------------------
 1 file changed, 6 insertions(+), 31 deletions(-)

diff --git a/drivers/iio/adc/rzg2l_adc.c b/drivers/iio/adc/rzg2l_adc.c
index 8a804f81c04b..c0c13e99aa92 100644
--- a/drivers/iio/adc/rzg2l_adc.c
+++ b/drivers/iio/adc/rzg2l_adc.c
@@ -411,11 +411,6 @@ static void rzg2l_adc_pm_runtime_set_suspended(void *data)
 	pm_runtime_set_suspended(dev->parent);
 }
 
-static void rzg2l_adc_reset_assert(void *data)
-{
-	reset_control_assert(data);
-}
-
 static int rzg2l_adc_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -448,34 +443,14 @@ static int rzg2l_adc_probe(struct platform_device *pdev)
 	if (IS_ERR(adc->adclk))
 		return dev_err_probe(dev, PTR_ERR(adc->adclk), "Failed to get adclk");
 
-	adc->adrstn = devm_reset_control_get_exclusive(dev, "adrst-n");
+	adc->adrstn = devm_reset_control_get_exclusive_deasserted(dev, "adrst-n");
 	if (IS_ERR(adc->adrstn))
-		return dev_err_probe(dev, PTR_ERR(adc->adrstn), "failed to get adrstn\n");
-
-	adc->presetn = devm_reset_control_get_exclusive(dev, "presetn");
-	if (IS_ERR(adc->presetn))
-		return dev_err_probe(dev, PTR_ERR(adc->presetn), "failed to get presetn\n");
-
-	ret = reset_control_deassert(adc->adrstn);
-	if (ret)
-		return dev_err_probe(&pdev->dev, ret, "failed to deassert adrstn pin, %d\n", ret);
-
-	ret = devm_add_action_or_reset(&pdev->dev,
-				       rzg2l_adc_reset_assert, adc->adrstn);
-	if (ret) {
-		return dev_err_probe(&pdev->dev, ret,
-				     "failed to register adrstn assert devm action, %d\n", ret);
-	}
+		return dev_err_probe(dev, PTR_ERR(adc->adrstn), "failed to get/deassert adrst-n\n");
 
-	ret = reset_control_deassert(adc->presetn);
-	if (ret)
-		return dev_err_probe(&pdev->dev, ret, "failed to deassert presetn pin, %d\n", ret);
-
-	ret = devm_add_action_or_reset(&pdev->dev,
-				       rzg2l_adc_reset_assert, adc->presetn);
-	if (ret) {
-		return dev_err_probe(&pdev->dev, ret,
-				     "failed to register presetn assert devm action, %d\n", ret);
+	adc->presetn = devm_reset_control_get_exclusive_deasserted(dev, "presetn");
+	if (IS_ERR(adc->presetn)) {
+		return dev_err_probe(dev, PTR_ERR(adc->presetn),
+				     "failed to get/deassert presetn\n");
 	}
 
 	ret = rzg2l_adc_hw_init(adc);
-- 
2.39.2

Re: [PATCH v2 03/15] iio: adc: rzg2l_adc: Use devres helpers to request pre-deasserted reset controls

[Jonathan Cameron]

On Fri,  6 Dec 2024 13:13:25 +0200
Claudiu <claudiu.beznea@tuxon.dev> wrote:

> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> Starting with commit d872bed85036 ("reset: Add devres helpers to request
> pre-deasserted reset controls"), devres helpers are available to simplify
> the process of requesting pre-deasserted reset controls. Update the
> rzg2l_adc driver to utilize these helpers, reducing complexity in this
> way.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
> 
> Changes in v2:
> - rebased on top of patch 2/15 from this version
> - used "failed to get/deassert" failure messages
> 
>  drivers/iio/adc/rzg2l_adc.c | 37 ++++++-------------------------------
>  1 file changed, 6 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/iio/adc/rzg2l_adc.c b/drivers/iio/adc/rzg2l_adc.c
> index 8a804f81c04b..c0c13e99aa92 100644
> --- a/drivers/iio/adc/rzg2l_adc.c
> +++ b/drivers/iio/adc/rzg2l_adc.c
> @@ -411,11 +411,6 @@ static void rzg2l_adc_pm_runtime_set_suspended(void *data)
>  	pm_runtime_set_suspended(dev->parent);
>  }
>  
> -static void rzg2l_adc_reset_assert(void *data)
> -{
> -	reset_control_assert(data);
> -}
> -
>  static int rzg2l_adc_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -448,34 +443,14 @@ static int rzg2l_adc_probe(struct platform_device *pdev)
>  	if (IS_ERR(adc->adclk))
>  		return dev_err_probe(dev, PTR_ERR(adc->adclk), "Failed to get adclk");
>  
> -	adc->adrstn = devm_reset_control_get_exclusive(dev, "adrst-n");
> +	adc->adrstn = devm_reset_control_get_exclusive_deasserted(dev, "adrst-n");
>  	if (IS_ERR(adc->adrstn))
> -		return dev_err_probe(dev, PTR_ERR(adc->adrstn), "failed to get adrstn\n");
> -
> -	adc->presetn = devm_reset_control_get_exclusive(dev, "presetn");
> -	if (IS_ERR(adc->presetn))
> -		return dev_err_probe(dev, PTR_ERR(adc->presetn), "failed to get presetn\n");
> -
> -	ret = reset_control_deassert(adc->adrstn);
> -	if (ret)
> -		return dev_err_probe(&pdev->dev, ret, "failed to deassert adrstn pin, %d\n", ret);
> -
> -	ret = devm_add_action_or_reset(&pdev->dev,
> -				       rzg2l_adc_reset_assert, adc->adrstn);
> -	if (ret) {
Huh. Missed this in previous. These brackets shouldn't be there.
> -		return dev_err_probe(&pdev->dev, ret,
> -				     "failed to register adrstn assert devm action, %d\n", ret);
> -	}
> +		return dev_err_probe(dev, PTR_ERR(adc->adrstn), "failed to get/deassert adrst-n\n");
>  
> -	ret = reset_control_deassert(adc->presetn);
> -	if (ret)
> -		return dev_err_probe(&pdev->dev, ret, "failed to deassert presetn pin, %d\n", ret);
> -
> -	ret = devm_add_action_or_reset(&pdev->dev,
> -				       rzg2l_adc_reset_assert, adc->presetn);
> -	if (ret) {
> -		return dev_err_probe(&pdev->dev, ret,
> -				     "failed to register presetn assert devm action, %d\n", ret);
> +	adc->presetn = devm_reset_control_get_exclusive_deasserted(dev, "presetn");
> +	if (IS_ERR(adc->presetn)) {
Adding bracket here both makes limited sense and messes up the diff. I dropped them.

Jonathan

> +		return dev_err_probe(dev, PTR_ERR(adc->presetn),
> +				     "failed to get/deassert presetn\n");
>  	}
>  
>  	ret = rzg2l_adc_hw_init(adc);

Re: [PATCH v2 03/15] iio: adc: rzg2l_adc: Use devres helpers to request pre-deasserted reset controls

[Lad, Prabhakar]

On Fri, Dec 6, 2024 at 11:14 AM Claudiu <claudiu.beznea@tuxon.dev> wrote:
>
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> Starting with commit d872bed85036 ("reset: Add devres helpers to request
> pre-deasserted reset controls"), devres helpers are available to simplify
> the process of requesting pre-deasserted reset controls. Update the
> rzg2l_adc driver to utilize these helpers, reducing complexity in this
> way.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
>
> Changes in v2:
> - rebased on top of patch 2/15 from this version
> - used "failed to get/deassert" failure messages
>
>  drivers/iio/adc/rzg2l_adc.c | 37 ++++++-------------------------------
>  1 file changed, 6 insertions(+), 31 deletions(-)
>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Cheers,
Prabhakar

> diff --git a/drivers/iio/adc/rzg2l_adc.c b/drivers/iio/adc/rzg2l_adc.c
> index 8a804f81c04b..c0c13e99aa92 100644
> --- a/drivers/iio/adc/rzg2l_adc.c
> +++ b/drivers/iio/adc/rzg2l_adc.c
> @@ -411,11 +411,6 @@ static void rzg2l_adc_pm_runtime_set_suspended(void *data)
>         pm_runtime_set_suspended(dev->parent);
>  }
>
> -static void rzg2l_adc_reset_assert(void *data)
> -{
> -       reset_control_assert(data);
> -}
> -
>  static int rzg2l_adc_probe(struct platform_device *pdev)
>  {
>         struct device *dev = &pdev->dev;
> @@ -448,34 +443,14 @@ static int rzg2l_adc_probe(struct platform_device *pdev)
>         if (IS_ERR(adc->adclk))
>                 return dev_err_probe(dev, PTR_ERR(adc->adclk), "Failed to get adclk");
>
> -       adc->adrstn = devm_reset_control_get_exclusive(dev, "adrst-n");
> +       adc->adrstn = devm_reset_control_get_exclusive_deasserted(dev, "adrst-n");
>         if (IS_ERR(adc->adrstn))
> -               return dev_err_probe(dev, PTR_ERR(adc->adrstn), "failed to get adrstn\n");
> -
> -       adc->presetn = devm_reset_control_get_exclusive(dev, "presetn");
> -       if (IS_ERR(adc->presetn))
> -               return dev_err_probe(dev, PTR_ERR(adc->presetn), "failed to get presetn\n");
> -
> -       ret = reset_control_deassert(adc->adrstn);
> -       if (ret)
> -               return dev_err_probe(&pdev->dev, ret, "failed to deassert adrstn pin, %d\n", ret);
> -
> -       ret = devm_add_action_or_reset(&pdev->dev,
> -                                      rzg2l_adc_reset_assert, adc->adrstn);
> -       if (ret) {
> -               return dev_err_probe(&pdev->dev, ret,
> -                                    "failed to register adrstn assert devm action, %d\n", ret);
> -       }
> +               return dev_err_probe(dev, PTR_ERR(adc->adrstn), "failed to get/deassert adrst-n\n");
>
> -       ret = reset_control_deassert(adc->presetn);
> -       if (ret)
> -               return dev_err_probe(&pdev->dev, ret, "failed to deassert presetn pin, %d\n", ret);
> -
> -       ret = devm_add_action_or_reset(&pdev->dev,
> -                                      rzg2l_adc_reset_assert, adc->presetn);
> -       if (ret) {
> -               return dev_err_probe(&pdev->dev, ret,
> -                                    "failed to register presetn assert devm action, %d\n", ret);
> +       adc->presetn = devm_reset_control_get_exclusive_deasserted(dev, "presetn");
> +       if (IS_ERR(adc->presetn)) {
> +               return dev_err_probe(dev, PTR_ERR(adc->presetn),
> +                                    "failed to get/deassert presetn\n");
>         }
>
>         ret = rzg2l_adc_hw_init(adc);
> --
> 2.39.2
>
>

[PATCH v2 04/15] iio: adc: rzg2l_adc: Simplify the runtime PM code

[Claudiu]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

All Renesas SoCs using the rzg2l_adc driver manage ADC clocks through PM
domains. Calling pm_runtime_{resume_and_get, put_sync}() implicitly sets
the state of the clocks. As a result, the code in the rzg2l_adc driver that
explicitly manages ADC clocks can be removed, leading to simpler and
cleaner implementation.

Additionally, replace the use of rzg2l_adc_set_power() with direct PM
runtime API calls to further simplify and clean up the code.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v2:
- rebased on top of patch 02/15 from this version

 drivers/iio/adc/rzg2l_adc.c | 96 ++++++++-----------------------------
 1 file changed, 20 insertions(+), 76 deletions(-)

diff --git a/drivers/iio/adc/rzg2l_adc.c b/drivers/iio/adc/rzg2l_adc.c
index c0c13e99aa92..780cb927eab1 100644
--- a/drivers/iio/adc/rzg2l_adc.c
+++ b/drivers/iio/adc/rzg2l_adc.c
@@ -8,7 +8,6 @@
  */
 
 #include <linux/bitfield.h>
-#include <linux/clk.h>
 #include <linux/completion.h>
 #include <linux/delay.h>
 #include <linux/iio/iio.h>
@@ -69,8 +68,6 @@ struct rzg2l_adc_data {
 
 struct rzg2l_adc {
 	void __iomem *base;
-	struct clk *pclk;
-	struct clk *adclk;
 	struct reset_control *presetn;
 	struct reset_control *adrstn;
 	struct completion completion;
@@ -188,29 +185,18 @@ static int rzg2l_adc_conversion_setup(struct rzg2l_adc *adc, u8 ch)
 	return 0;
 }
 
-static int rzg2l_adc_set_power(struct iio_dev *indio_dev, bool on)
-{
-	struct device *dev = indio_dev->dev.parent;
-
-	if (on)
-		return pm_runtime_resume_and_get(dev);
-
-	return pm_runtime_put_sync(dev);
-}
-
 static int rzg2l_adc_conversion(struct iio_dev *indio_dev, struct rzg2l_adc *adc, u8 ch)
 {
+	struct device *dev = indio_dev->dev.parent;
 	int ret;
 
-	ret = rzg2l_adc_set_power(indio_dev, true);
+	ret = pm_runtime_resume_and_get(dev);
 	if (ret)
 		return ret;
 
 	ret = rzg2l_adc_conversion_setup(adc, ch);
-	if (ret) {
-		rzg2l_adc_set_power(indio_dev, false);
-		return ret;
-	}
+	if (ret)
+		goto rpm_put;
 
 	reinit_completion(&adc->completion);
 
@@ -219,12 +205,14 @@ static int rzg2l_adc_conversion(struct iio_dev *indio_dev, struct rzg2l_adc *adc
 	if (!wait_for_completion_timeout(&adc->completion, RZG2L_ADC_TIMEOUT)) {
 		rzg2l_adc_writel(adc, RZG2L_ADINT,
 				 rzg2l_adc_readl(adc, RZG2L_ADINT) & ~RZG2L_ADINT_INTEN_MASK);
-		rzg2l_adc_start_stop(adc, false);
-		rzg2l_adc_set_power(indio_dev, false);
-		return -ETIMEDOUT;
+		ret = -ETIMEDOUT;
 	}
 
-	return rzg2l_adc_set_power(indio_dev, false);
+	rzg2l_adc_start_stop(adc, false);
+
+rpm_put:
+	pm_runtime_put_sync(dev);
+	return ret;
 }
 
 static int rzg2l_adc_read_raw(struct iio_dev *indio_dev,
@@ -348,13 +336,13 @@ static int rzg2l_adc_parse_properties(struct platform_device *pdev, struct rzg2l
 	return 0;
 }
 
-static int rzg2l_adc_hw_init(struct rzg2l_adc *adc)
+static int rzg2l_adc_hw_init(struct device *dev, struct rzg2l_adc *adc)
 {
 	int timeout = 5;
 	u32 reg;
 	int ret;
 
-	ret = clk_prepare_enable(adc->pclk);
+	ret = pm_runtime_resume_and_get(dev);
 	if (ret)
 		return ret;
 
@@ -392,25 +380,10 @@ static int rzg2l_adc_hw_init(struct rzg2l_adc *adc)
 	rzg2l_adc_writel(adc, RZG2L_ADM(3), reg);
 
 exit_hw_init:
-	clk_disable_unprepare(adc->pclk);
-
+	pm_runtime_put_sync(dev);
 	return ret;
 }
 
-static void rzg2l_adc_pm_runtime_disable(void *data)
-{
-	struct device *dev = data;
-
-	pm_runtime_disable(dev->parent);
-}
-
-static void rzg2l_adc_pm_runtime_set_suspended(void *data)
-{
-	struct device *dev = data;
-
-	pm_runtime_set_suspended(dev->parent);
-}
-
 static int rzg2l_adc_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -435,14 +408,6 @@ static int rzg2l_adc_probe(struct platform_device *pdev)
 	if (IS_ERR(adc->base))
 		return PTR_ERR(adc->base);
 
-	adc->pclk = devm_clk_get(dev, "pclk");
-	if (IS_ERR(adc->pclk))
-		return dev_err_probe(dev, PTR_ERR(adc->pclk), "Failed to get pclk");
-
-	adc->adclk = devm_clk_get(dev, "adclk");
-	if (IS_ERR(adc->adclk))
-		return dev_err_probe(dev, PTR_ERR(adc->adclk), "Failed to get adclk");
-
 	adc->adrstn = devm_reset_control_get_exclusive_deasserted(dev, "adrst-n");
 	if (IS_ERR(adc->adrstn))
 		return dev_err_probe(dev, PTR_ERR(adc->adrstn), "failed to get/deassert adrst-n\n");
@@ -453,7 +418,13 @@ static int rzg2l_adc_probe(struct platform_device *pdev)
 				     "failed to get/deassert presetn\n");
 	}
 
-	ret = rzg2l_adc_hw_init(adc);
+	ret = devm_pm_runtime_enable(dev);
+	if (ret)
+		return ret;
+
+	platform_set_drvdata(pdev, indio_dev);
+
+	ret = rzg2l_adc_hw_init(dev, adc);
 	if (ret)
 		return dev_err_probe(&pdev->dev, ret, "failed to initialize ADC HW, %d\n", ret);
 
@@ -468,26 +439,12 @@ static int rzg2l_adc_probe(struct platform_device *pdev)
 
 	init_completion(&adc->completion);
 
-	platform_set_drvdata(pdev, indio_dev);
-
 	indio_dev->name = DRIVER_NAME;
 	indio_dev->info = &rzg2l_adc_iio_info;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->channels = adc->data->channels;
 	indio_dev->num_channels = adc->data->num_channels;
 
-	pm_runtime_set_suspended(dev);
-	ret = devm_add_action_or_reset(&pdev->dev,
-				       rzg2l_adc_pm_runtime_set_suspended, &indio_dev->dev);
-	if (ret)
-		return ret;
-
-	pm_runtime_enable(dev);
-	ret = devm_add_action_or_reset(&pdev->dev,
-				       rzg2l_adc_pm_runtime_disable, &indio_dev->dev);
-	if (ret)
-		return ret;
-
 	return devm_iio_device_register(dev, indio_dev);
 }
 
@@ -503,8 +460,6 @@ static int __maybe_unused rzg2l_adc_pm_runtime_suspend(struct device *dev)
 	struct rzg2l_adc *adc = iio_priv(indio_dev);
 
 	rzg2l_adc_pwr(adc, false);
-	clk_disable_unprepare(adc->adclk);
-	clk_disable_unprepare(adc->pclk);
 
 	return 0;
 }
@@ -513,17 +468,6 @@ static int __maybe_unused rzg2l_adc_pm_runtime_resume(struct device *dev)
 {
 	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct rzg2l_adc *adc = iio_priv(indio_dev);
-	int ret;
-
-	ret = clk_prepare_enable(adc->pclk);
-	if (ret)
-		return ret;
-
-	ret = clk_prepare_enable(adc->adclk);
-	if (ret) {
-		clk_disable_unprepare(adc->pclk);
-		return ret;
-	}
 
 	rzg2l_adc_pwr(adc, true);
 
-- 
2.39.2

Re: [PATCH v2 04/15] iio: adc: rzg2l_adc: Simplify the runtime PM code

[Lad, Prabhakar]

On Fri, Dec 6, 2024 at 11:14 AM Claudiu <claudiu.beznea@tuxon.dev> wrote:
>
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> All Renesas SoCs using the rzg2l_adc driver manage ADC clocks through PM
> domains. Calling pm_runtime_{resume_and_get, put_sync}() implicitly sets
> the state of the clocks. As a result, the code in the rzg2l_adc driver that
> explicitly manages ADC clocks can be removed, leading to simpler and
> cleaner implementation.
>
> Additionally, replace the use of rzg2l_adc_set_power() with direct PM
> runtime API calls to further simplify and clean up the code.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
>
> Changes in v2:
> - rebased on top of patch 02/15 from this version
>
>  drivers/iio/adc/rzg2l_adc.c | 96 ++++++++-----------------------------
>  1 file changed, 20 insertions(+), 76 deletions(-)
>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Cheers,
Prabhakar

> diff --git a/drivers/iio/adc/rzg2l_adc.c b/drivers/iio/adc/rzg2l_adc.c
> index c0c13e99aa92..780cb927eab1 100644
> --- a/drivers/iio/adc/rzg2l_adc.c
> +++ b/drivers/iio/adc/rzg2l_adc.c
> @@ -8,7 +8,6 @@
>   */
>
>  #include <linux/bitfield.h>
> -#include <linux/clk.h>
>  #include <linux/completion.h>
>  #include <linux/delay.h>
>  #include <linux/iio/iio.h>
> @@ -69,8 +68,6 @@ struct rzg2l_adc_data {
>
>  struct rzg2l_adc {
>         void __iomem *base;
> -       struct clk *pclk;
> -       struct clk *adclk;
>         struct reset_control *presetn;
>         struct reset_control *adrstn;
>         struct completion completion;
> @@ -188,29 +185,18 @@ static int rzg2l_adc_conversion_setup(struct rzg2l_adc *adc, u8 ch)
>         return 0;
>  }
>
> -static int rzg2l_adc_set_power(struct iio_dev *indio_dev, bool on)
> -{
> -       struct device *dev = indio_dev->dev.parent;
> -
> -       if (on)
> -               return pm_runtime_resume_and_get(dev);
> -
> -       return pm_runtime_put_sync(dev);
> -}
> -
>  static int rzg2l_adc_conversion(struct iio_dev *indio_dev, struct rzg2l_adc *adc, u8 ch)
>  {
> +       struct device *dev = indio_dev->dev.parent;
>         int ret;
>
> -       ret = rzg2l_adc_set_power(indio_dev, true);
> +       ret = pm_runtime_resume_and_get(dev);
>         if (ret)
>                 return ret;
>
>         ret = rzg2l_adc_conversion_setup(adc, ch);
> -       if (ret) {
> -               rzg2l_adc_set_power(indio_dev, false);
> -               return ret;
> -       }
> +       if (ret)
> +               goto rpm_put;
>
>         reinit_completion(&adc->completion);
>
> @@ -219,12 +205,14 @@ static int rzg2l_adc_conversion(struct iio_dev *indio_dev, struct rzg2l_adc *adc
>         if (!wait_for_completion_timeout(&adc->completion, RZG2L_ADC_TIMEOUT)) {
>                 rzg2l_adc_writel(adc, RZG2L_ADINT,
>                                  rzg2l_adc_readl(adc, RZG2L_ADINT) & ~RZG2L_ADINT_INTEN_MASK);
> -               rzg2l_adc_start_stop(adc, false);
> -               rzg2l_adc_set_power(indio_dev, false);
> -               return -ETIMEDOUT;
> +               ret = -ETIMEDOUT;
>         }
>
> -       return rzg2l_adc_set_power(indio_dev, false);
> +       rzg2l_adc_start_stop(adc, false);
> +
> +rpm_put:
> +       pm_runtime_put_sync(dev);
> +       return ret;
>  }
>
>  static int rzg2l_adc_read_raw(struct iio_dev *indio_dev,
> @@ -348,13 +336,13 @@ static int rzg2l_adc_parse_properties(struct platform_device *pdev, struct rzg2l
>         return 0;
>  }
>
> -static int rzg2l_adc_hw_init(struct rzg2l_adc *adc)
> +static int rzg2l_adc_hw_init(struct device *dev, struct rzg2l_adc *adc)
>  {
>         int timeout = 5;
>         u32 reg;
>         int ret;
>
> -       ret = clk_prepare_enable(adc->pclk);
> +       ret = pm_runtime_resume_and_get(dev);
>         if (ret)
>                 return ret;
>
> @@ -392,25 +380,10 @@ static int rzg2l_adc_hw_init(struct rzg2l_adc *adc)
>         rzg2l_adc_writel(adc, RZG2L_ADM(3), reg);
>
>  exit_hw_init:
> -       clk_disable_unprepare(adc->pclk);
> -
> +       pm_runtime_put_sync(dev);
>         return ret;
>  }
>
> -static void rzg2l_adc_pm_runtime_disable(void *data)
> -{
> -       struct device *dev = data;
> -
> -       pm_runtime_disable(dev->parent);
> -}
> -
> -static void rzg2l_adc_pm_runtime_set_suspended(void *data)
> -{
> -       struct device *dev = data;
> -
> -       pm_runtime_set_suspended(dev->parent);
> -}
> -
>  static int rzg2l_adc_probe(struct platform_device *pdev)
>  {
>         struct device *dev = &pdev->dev;
> @@ -435,14 +408,6 @@ static int rzg2l_adc_probe(struct platform_device *pdev)
>         if (IS_ERR(adc->base))
>                 return PTR_ERR(adc->base);
>
> -       adc->pclk = devm_clk_get(dev, "pclk");
> -       if (IS_ERR(adc->pclk))
> -               return dev_err_probe(dev, PTR_ERR(adc->pclk), "Failed to get pclk");
> -
> -       adc->adclk = devm_clk_get(dev, "adclk");
> -       if (IS_ERR(adc->adclk))
> -               return dev_err_probe(dev, PTR_ERR(adc->adclk), "Failed to get adclk");
> -
>         adc->adrstn = devm_reset_control_get_exclusive_deasserted(dev, "adrst-n");
>         if (IS_ERR(adc->adrstn))
>                 return dev_err_probe(dev, PTR_ERR(adc->adrstn), "failed to get/deassert adrst-n\n");
> @@ -453,7 +418,13 @@ static int rzg2l_adc_probe(struct platform_device *pdev)
>                                      "failed to get/deassert presetn\n");
>         }
>
> -       ret = rzg2l_adc_hw_init(adc);
> +       ret = devm_pm_runtime_enable(dev);
> +       if (ret)
> +               return ret;
> +
> +       platform_set_drvdata(pdev, indio_dev);
> +
> +       ret = rzg2l_adc_hw_init(dev, adc);
>         if (ret)
>                 return dev_err_probe(&pdev->dev, ret, "failed to initialize ADC HW, %d\n", ret);
>
> @@ -468,26 +439,12 @@ static int rzg2l_adc_probe(struct platform_device *pdev)
>
>         init_completion(&adc->completion);
>
> -       platform_set_drvdata(pdev, indio_dev);
> -
>         indio_dev->name = DRIVER_NAME;
>         indio_dev->info = &rzg2l_adc_iio_info;
>         indio_dev->modes = INDIO_DIRECT_MODE;
>         indio_dev->channels = adc->data->channels;
>         indio_dev->num_channels = adc->data->num_channels;
>
> -       pm_runtime_set_suspended(dev);
> -       ret = devm_add_action_or_reset(&pdev->dev,
> -                                      rzg2l_adc_pm_runtime_set_suspended, &indio_dev->dev);
> -       if (ret)
> -               return ret;
> -
> -       pm_runtime_enable(dev);
> -       ret = devm_add_action_or_reset(&pdev->dev,
> -                                      rzg2l_adc_pm_runtime_disable, &indio_dev->dev);
> -       if (ret)
> -               return ret;
> -
>         return devm_iio_device_register(dev, indio_dev);
>  }
>
> @@ -503,8 +460,6 @@ static int __maybe_unused rzg2l_adc_pm_runtime_suspend(struct device *dev)
>         struct rzg2l_adc *adc = iio_priv(indio_dev);
>
>         rzg2l_adc_pwr(adc, false);
> -       clk_disable_unprepare(adc->adclk);
> -       clk_disable_unprepare(adc->pclk);
>
>         return 0;
>  }
> @@ -513,17 +468,6 @@ static int __maybe_unused rzg2l_adc_pm_runtime_resume(struct device *dev)
>  {
>         struct iio_dev *indio_dev = dev_get_drvdata(dev);
>         struct rzg2l_adc *adc = iio_priv(indio_dev);
> -       int ret;
> -
> -       ret = clk_prepare_enable(adc->pclk);
> -       if (ret)
> -               return ret;
> -
> -       ret = clk_prepare_enable(adc->adclk);
> -       if (ret) {
> -               clk_disable_unprepare(adc->pclk);
> -               return ret;
> -       }
>
>         rzg2l_adc_pwr(adc, true);
>
> --
> 2.39.2
>
>

[PATCH v2 05/15] iio: adc: rzg2l_adc: Switch to RUNTIME_PM_OPS() and pm_ptr()

[Claudiu]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

The use of SET_RUNTIME_PM_OPS() is now deprecated and requires
__maybe_unused annotations to avoid warnings about unused functions.
Switching to RUNTIME_PM_OPS() and pm_ptr() eliminates the need for such
annotations because the compiler can directly reference the runtime PM
functions, thereby suppressing the warnings. As a result, the
__maybe_unused markings can be removed.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v2:
- none

 drivers/iio/adc/rzg2l_adc.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/adc/rzg2l_adc.c b/drivers/iio/adc/rzg2l_adc.c
index 780cb927eab1..482da6dcf174 100644
--- a/drivers/iio/adc/rzg2l_adc.c
+++ b/drivers/iio/adc/rzg2l_adc.c
@@ -454,7 +454,7 @@ static const struct of_device_id rzg2l_adc_match[] = {
 };
 MODULE_DEVICE_TABLE(of, rzg2l_adc_match);
 
-static int __maybe_unused rzg2l_adc_pm_runtime_suspend(struct device *dev)
+static int rzg2l_adc_pm_runtime_suspend(struct device *dev)
 {
 	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct rzg2l_adc *adc = iio_priv(indio_dev);
@@ -464,7 +464,7 @@ static int __maybe_unused rzg2l_adc_pm_runtime_suspend(struct device *dev)
 	return 0;
 }
 
-static int __maybe_unused rzg2l_adc_pm_runtime_resume(struct device *dev)
+static int rzg2l_adc_pm_runtime_resume(struct device *dev)
 {
 	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct rzg2l_adc *adc = iio_priv(indio_dev);
@@ -475,9 +475,7 @@ static int __maybe_unused rzg2l_adc_pm_runtime_resume(struct device *dev)
 }
 
 static const struct dev_pm_ops rzg2l_adc_pm_ops = {
-	SET_RUNTIME_PM_OPS(rzg2l_adc_pm_runtime_suspend,
-			   rzg2l_adc_pm_runtime_resume,
-			   NULL)
+	RUNTIME_PM_OPS(rzg2l_adc_pm_runtime_suspend, rzg2l_adc_pm_runtime_resume, NULL)
 };
 
 static struct platform_driver rzg2l_adc_driver = {
@@ -485,7 +483,7 @@ static struct platform_driver rzg2l_adc_driver = {
 	.driver		= {
 		.name		= DRIVER_NAME,
 		.of_match_table = rzg2l_adc_match,
-		.pm		= &rzg2l_adc_pm_ops,
+		.pm		= pm_ptr(&rzg2l_adc_pm_ops),
 	},
 };
 
-- 
2.39.2

Re: [PATCH v2 05/15] iio: adc: rzg2l_adc: Switch to RUNTIME_PM_OPS() and pm_ptr()

[Lad, Prabhakar]

On Fri, Dec 6, 2024 at 11:14 AM Claudiu <claudiu.beznea@tuxon.dev> wrote:
>
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> The use of SET_RUNTIME_PM_OPS() is now deprecated and requires
> __maybe_unused annotations to avoid warnings about unused functions.
> Switching to RUNTIME_PM_OPS() and pm_ptr() eliminates the need for such
> annotations because the compiler can directly reference the runtime PM
> functions, thereby suppressing the warnings. As a result, the
> __maybe_unused markings can be removed.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
>
> Changes in v2:
> - none
>
>  drivers/iio/adc/rzg2l_adc.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Cheers,
Prabhakar

> diff --git a/drivers/iio/adc/rzg2l_adc.c b/drivers/iio/adc/rzg2l_adc.c
> index 780cb927eab1..482da6dcf174 100644
> --- a/drivers/iio/adc/rzg2l_adc.c
> +++ b/drivers/iio/adc/rzg2l_adc.c
> @@ -454,7 +454,7 @@ static const struct of_device_id rzg2l_adc_match[] = {
>  };
>  MODULE_DEVICE_TABLE(of, rzg2l_adc_match);
>
> -static int __maybe_unused rzg2l_adc_pm_runtime_suspend(struct device *dev)
> +static int rzg2l_adc_pm_runtime_suspend(struct device *dev)
>  {
>         struct iio_dev *indio_dev = dev_get_drvdata(dev);
>         struct rzg2l_adc *adc = iio_priv(indio_dev);
> @@ -464,7 +464,7 @@ static int __maybe_unused rzg2l_adc_pm_runtime_suspend(struct device *dev)
>         return 0;
>  }
>
> -static int __maybe_unused rzg2l_adc_pm_runtime_resume(struct device *dev)
> +static int rzg2l_adc_pm_runtime_resume(struct device *dev)
>  {
>         struct iio_dev *indio_dev = dev_get_drvdata(dev);
>         struct rzg2l_adc *adc = iio_priv(indio_dev);
> @@ -475,9 +475,7 @@ static int __maybe_unused rzg2l_adc_pm_runtime_resume(struct device *dev)
>  }
>
>  static const struct dev_pm_ops rzg2l_adc_pm_ops = {
> -       SET_RUNTIME_PM_OPS(rzg2l_adc_pm_runtime_suspend,
> -                          rzg2l_adc_pm_runtime_resume,
> -                          NULL)
> +       RUNTIME_PM_OPS(rzg2l_adc_pm_runtime_suspend, rzg2l_adc_pm_runtime_resume, NULL)
>  };
>
>  static struct platform_driver rzg2l_adc_driver = {
> @@ -485,7 +483,7 @@ static struct platform_driver rzg2l_adc_driver = {
>         .driver         = {
>                 .name           = DRIVER_NAME,
>                 .of_match_table = rzg2l_adc_match,
> -               .pm             = &rzg2l_adc_pm_ops,
> +               .pm             = pm_ptr(&rzg2l_adc_pm_ops),
>         },
>  };
>
> --
> 2.39.2
>
>

[PATCH v2 06/15] iio: adc: rzg2l_adc: Use read_poll_timeout()

[Claudiu]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Replace the driver-specific implementation with the read_poll_timeout()
function. This change simplifies the code and improves maintainability by
leveraging the standardized helper.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v2:
- none

 drivers/iio/adc/rzg2l_adc.c | 29 ++++++++++-------------------
 1 file changed, 10 insertions(+), 19 deletions(-)

diff --git a/drivers/iio/adc/rzg2l_adc.c b/drivers/iio/adc/rzg2l_adc.c
index 482da6dcf174..38d4fb014847 100644
--- a/drivers/iio/adc/rzg2l_adc.c
+++ b/drivers/iio/adc/rzg2l_adc.c
@@ -13,6 +13,7 @@
 #include <linux/iio/iio.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
+#include <linux/iopoll.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
@@ -112,7 +113,7 @@ static void rzg2l_adc_pwr(struct rzg2l_adc *adc, bool on)
 
 static void rzg2l_adc_start_stop(struct rzg2l_adc *adc, bool start)
 {
-	int timeout = 5;
+	int ret;
 	u32 reg;
 
 	reg = rzg2l_adc_readl(adc, RZG2L_ADM(0));
@@ -125,15 +126,10 @@ static void rzg2l_adc_start_stop(struct rzg2l_adc *adc, bool start)
 	if (start)
 		return;
 
-	do {
-		usleep_range(100, 200);
-		reg = rzg2l_adc_readl(adc, RZG2L_ADM(0));
-		timeout--;
-		if (!timeout) {
-			pr_err("%s stopping ADC timed out\n", __func__);
-			break;
-		}
-	} while (((reg & RZG2L_ADM0_ADBSY) || (reg & RZG2L_ADM0_ADCE)));
+	ret = read_poll_timeout(rzg2l_adc_readl, reg, !(reg & (RZG2L_ADM0_ADBSY | RZG2L_ADM0_ADCE)),
+				200, 1000, true, adc, RZG2L_ADM(0));
+	if (ret)
+		pr_err("%s stopping ADC timed out\n", __func__);
 }
 
 static void rzg2l_set_trigger(struct rzg2l_adc *adc)
@@ -338,7 +334,6 @@ static int rzg2l_adc_parse_properties(struct platform_device *pdev, struct rzg2l
 
 static int rzg2l_adc_hw_init(struct device *dev, struct rzg2l_adc *adc)
 {
-	int timeout = 5;
 	u32 reg;
 	int ret;
 
@@ -351,14 +346,10 @@ static int rzg2l_adc_hw_init(struct device *dev, struct rzg2l_adc *adc)
 	reg |= RZG2L_ADM0_SRESB;
 	rzg2l_adc_writel(adc, RZG2L_ADM(0), reg);
 
-	while (!(rzg2l_adc_readl(adc, RZG2L_ADM(0)) & RZG2L_ADM0_SRESB)) {
-		if (!timeout) {
-			ret = -EBUSY;
-			goto exit_hw_init;
-		}
-		timeout--;
-		usleep_range(100, 200);
-	}
+	ret = read_poll_timeout(rzg2l_adc_readl, reg, reg & RZG2L_ADM0_SRESB,
+				200, 1000, false, adc, RZG2L_ADM(0));
+	if (ret)
+		goto exit_hw_init;
 
 	/* Only division by 4 can be set */
 	reg = rzg2l_adc_readl(adc, RZG2L_ADIVC);
-- 
2.39.2

Re: [PATCH v2 06/15] iio: adc: rzg2l_adc: Use read_poll_timeout()

[Lad, Prabhakar]

On Fri, Dec 6, 2024 at 11:15 AM Claudiu <claudiu.beznea@tuxon.dev> wrote:
>
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> Replace the driver-specific implementation with the read_poll_timeout()
> function. This change simplifies the code and improves maintainability by
> leveraging the standardized helper.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
>
> Changes in v2:
> - none
>
>  drivers/iio/adc/rzg2l_adc.c | 29 ++++++++++-------------------
>  1 file changed, 10 insertions(+), 19 deletions(-)
>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Cheers,
Prabhakar

> diff --git a/drivers/iio/adc/rzg2l_adc.c b/drivers/iio/adc/rzg2l_adc.c
> index 482da6dcf174..38d4fb014847 100644
> --- a/drivers/iio/adc/rzg2l_adc.c
> +++ b/drivers/iio/adc/rzg2l_adc.c
> @@ -13,6 +13,7 @@
>  #include <linux/iio/iio.h>
>  #include <linux/interrupt.h>
>  #include <linux/io.h>
> +#include <linux/iopoll.h>
>  #include <linux/mod_devicetable.h>
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
> @@ -112,7 +113,7 @@ static void rzg2l_adc_pwr(struct rzg2l_adc *adc, bool on)
>
>  static void rzg2l_adc_start_stop(struct rzg2l_adc *adc, bool start)
>  {
> -       int timeout = 5;
> +       int ret;
>         u32 reg;
>
>         reg = rzg2l_adc_readl(adc, RZG2L_ADM(0));
> @@ -125,15 +126,10 @@ static void rzg2l_adc_start_stop(struct rzg2l_adc *adc, bool start)
>         if (start)
>                 return;
>
> -       do {
> -               usleep_range(100, 200);
> -               reg = rzg2l_adc_readl(adc, RZG2L_ADM(0));
> -               timeout--;
> -               if (!timeout) {
> -                       pr_err("%s stopping ADC timed out\n", __func__);
> -                       break;
> -               }
> -       } while (((reg & RZG2L_ADM0_ADBSY) || (reg & RZG2L_ADM0_ADCE)));
> +       ret = read_poll_timeout(rzg2l_adc_readl, reg, !(reg & (RZG2L_ADM0_ADBSY | RZG2L_ADM0_ADCE)),
> +                               200, 1000, true, adc, RZG2L_ADM(0));
> +       if (ret)
> +               pr_err("%s stopping ADC timed out\n", __func__);
>  }
>
>  static void rzg2l_set_trigger(struct rzg2l_adc *adc)
> @@ -338,7 +334,6 @@ static int rzg2l_adc_parse_properties(struct platform_device *pdev, struct rzg2l
>
>  static int rzg2l_adc_hw_init(struct device *dev, struct rzg2l_adc *adc)
>  {
> -       int timeout = 5;
>         u32 reg;
>         int ret;
>
> @@ -351,14 +346,10 @@ static int rzg2l_adc_hw_init(struct device *dev, struct rzg2l_adc *adc)
>         reg |= RZG2L_ADM0_SRESB;
>         rzg2l_adc_writel(adc, RZG2L_ADM(0), reg);
>
> -       while (!(rzg2l_adc_readl(adc, RZG2L_ADM(0)) & RZG2L_ADM0_SRESB)) {
> -               if (!timeout) {
> -                       ret = -EBUSY;
> -                       goto exit_hw_init;
> -               }
> -               timeout--;
> -               usleep_range(100, 200);
> -       }
> +       ret = read_poll_timeout(rzg2l_adc_readl, reg, reg & RZG2L_ADM0_SRESB,
> +                               200, 1000, false, adc, RZG2L_ADM(0));
> +       if (ret)
> +               goto exit_hw_init;
>
>         /* Only division by 4 can be set */
>         reg = rzg2l_adc_readl(adc, RZG2L_ADIVC);
> --
> 2.39.2
>
>

[PATCH v2 07/15] iio: adc: rzg2l_adc: Simplify the locking scheme in rzg2l_adc_read_raw()

[Claudiu]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Simplify the locking scheme in rzg2l_adc_read_raw() by using
guard(mutex)().

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v2:
- used guard(mutex)()
- adjusted the patch description

 drivers/iio/adc/rzg2l_adc.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/adc/rzg2l_adc.c b/drivers/iio/adc/rzg2l_adc.c
index 38d4fb014847..953511191eac 100644
--- a/drivers/iio/adc/rzg2l_adc.c
+++ b/drivers/iio/adc/rzg2l_adc.c
@@ -8,6 +8,7 @@
  */
 
 #include <linux/bitfield.h>
+#include <linux/cleanup.h>
 #include <linux/completion.h>
 #include <linux/delay.h>
 #include <linux/iio/iio.h>
@@ -220,21 +221,21 @@ static int rzg2l_adc_read_raw(struct iio_dev *indio_dev,
 	u8 ch;
 
 	switch (mask) {
-	case IIO_CHAN_INFO_RAW:
+	case IIO_CHAN_INFO_RAW: {
 		if (chan->type != IIO_VOLTAGE)
 			return -EINVAL;
 
-		mutex_lock(&adc->lock);
+		guard(mutex)(&adc->lock);
+
 		ch = chan->channel & RZG2L_ADC_CHN_MASK;
 		ret = rzg2l_adc_conversion(indio_dev, adc, ch);
-		if (ret) {
-			mutex_unlock(&adc->lock);
+		if (ret)
 			return ret;
-		}
+
 		*val = adc->last_val[ch];
-		mutex_unlock(&adc->lock);
 
 		return IIO_VAL_INT;
+	}
 
 	default:
 		return -EINVAL;
-- 
2.39.2

Re: [PATCH v2 07/15] iio: adc: rzg2l_adc: Simplify the locking scheme in rzg2l_adc_read_raw()

[Lad, Prabhakar]

On Fri, Dec 6, 2024 at 11:15 AM Claudiu <claudiu.beznea@tuxon.dev> wrote:
>
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> Simplify the locking scheme in rzg2l_adc_read_raw() by using
> guard(mutex)().
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
>
> Changes in v2:
> - used guard(mutex)()
> - adjusted the patch description
>
>  drivers/iio/adc/rzg2l_adc.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Cheers,
Prabhakar

> diff --git a/drivers/iio/adc/rzg2l_adc.c b/drivers/iio/adc/rzg2l_adc.c
> index 38d4fb014847..953511191eac 100644
> --- a/drivers/iio/adc/rzg2l_adc.c
> +++ b/drivers/iio/adc/rzg2l_adc.c
> @@ -8,6 +8,7 @@
>   */
>
>  #include <linux/bitfield.h>
> +#include <linux/cleanup.h>
>  #include <linux/completion.h>
>  #include <linux/delay.h>
>  #include <linux/iio/iio.h>
> @@ -220,21 +221,21 @@ static int rzg2l_adc_read_raw(struct iio_dev *indio_dev,
>         u8 ch;
>
>         switch (mask) {
> -       case IIO_CHAN_INFO_RAW:
> +       case IIO_CHAN_INFO_RAW: {
>                 if (chan->type != IIO_VOLTAGE)
>                         return -EINVAL;
>
> -               mutex_lock(&adc->lock);
> +               guard(mutex)(&adc->lock);
> +
>                 ch = chan->channel & RZG2L_ADC_CHN_MASK;
>                 ret = rzg2l_adc_conversion(indio_dev, adc, ch);
> -               if (ret) {
> -                       mutex_unlock(&adc->lock);
> +               if (ret)
>                         return ret;
> -               }
> +
>                 *val = adc->last_val[ch];
> -               mutex_unlock(&adc->lock);
>
>                 return IIO_VAL_INT;
> +       }
>
>         default:
>                 return -EINVAL;
> --
> 2.39.2
>
>

[PATCH v2 08/15] iio: adc: rzg2l_adc: Enable runtime PM autosuspend support

[Claudiu]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Enable runtime PM autosuspend support for the rzg2l_adc driver. With this
change, consecutive conversion requests will no longer cause the device to
be runtime-enabled/disabled after each request. Instead, the device will
transition based on the delay configured by the user.

This approach reduces the frequency of hardware register access during
runtime PM suspend/resume cycles, thereby saving CPU cycles.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v2:
- used a non-zero default autosusped delay
- adjusted the patch description to reflect that the default autosuspend
  delay has been changed

 drivers/iio/adc/rzg2l_adc.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/adc/rzg2l_adc.c b/drivers/iio/adc/rzg2l_adc.c
index 953511191eac..c3f9f95cdbba 100644
--- a/drivers/iio/adc/rzg2l_adc.c
+++ b/drivers/iio/adc/rzg2l_adc.c
@@ -208,7 +208,8 @@ static int rzg2l_adc_conversion(struct iio_dev *indio_dev, struct rzg2l_adc *adc
 	rzg2l_adc_start_stop(adc, false);
 
 rpm_put:
-	pm_runtime_put_sync(dev);
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_put_autosuspend(dev);
 	return ret;
 }
 
@@ -372,7 +373,8 @@ static int rzg2l_adc_hw_init(struct device *dev, struct rzg2l_adc *adc)
 	rzg2l_adc_writel(adc, RZG2L_ADM(3), reg);
 
 exit_hw_init:
-	pm_runtime_put_sync(dev);
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_put_autosuspend(dev);
 	return ret;
 }
 
@@ -410,6 +412,8 @@ static int rzg2l_adc_probe(struct platform_device *pdev)
 				     "failed to get/deassert presetn\n");
 	}
 
+	pm_runtime_set_autosuspend_delay(dev, 300);
+	pm_runtime_use_autosuspend(dev);
 	ret = devm_pm_runtime_enable(dev);
 	if (ret)
 		return ret;
-- 
2.39.2

Re: [PATCH v2 08/15] iio: adc: rzg2l_adc: Enable runtime PM autosuspend support

[Lad, Prabhakar]

On Fri, Dec 6, 2024 at 11:15 AM Claudiu <claudiu.beznea@tuxon.dev> wrote:
>
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> Enable runtime PM autosuspend support for the rzg2l_adc driver. With this
> change, consecutive conversion requests will no longer cause the device to
> be runtime-enabled/disabled after each request. Instead, the device will
> transition based on the delay configured by the user.
>
> This approach reduces the frequency of hardware register access during
> runtime PM suspend/resume cycles, thereby saving CPU cycles.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
>
> Changes in v2:
> - used a non-zero default autosusped delay
> - adjusted the patch description to reflect that the default autosuspend
>   delay has been changed
>
>  drivers/iio/adc/rzg2l_adc.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Cheers,
Prabhakar

> diff --git a/drivers/iio/adc/rzg2l_adc.c b/drivers/iio/adc/rzg2l_adc.c
> index 953511191eac..c3f9f95cdbba 100644
> --- a/drivers/iio/adc/rzg2l_adc.c
> +++ b/drivers/iio/adc/rzg2l_adc.c
> @@ -208,7 +208,8 @@ static int rzg2l_adc_conversion(struct iio_dev *indio_dev, struct rzg2l_adc *adc
>         rzg2l_adc_start_stop(adc, false);
>
>  rpm_put:
> -       pm_runtime_put_sync(dev);
> +       pm_runtime_mark_last_busy(dev);
> +       pm_runtime_put_autosuspend(dev);
>         return ret;
>  }
>
> @@ -372,7 +373,8 @@ static int rzg2l_adc_hw_init(struct device *dev, struct rzg2l_adc *adc)
>         rzg2l_adc_writel(adc, RZG2L_ADM(3), reg);
>
>  exit_hw_init:
> -       pm_runtime_put_sync(dev);
> +       pm_runtime_mark_last_busy(dev);
> +       pm_runtime_put_autosuspend(dev);
>         return ret;
>  }
>
> @@ -410,6 +412,8 @@ static int rzg2l_adc_probe(struct platform_device *pdev)
>                                      "failed to get/deassert presetn\n");
>         }
>
> +       pm_runtime_set_autosuspend_delay(dev, 300);
> +       pm_runtime_use_autosuspend(dev);
>         ret = devm_pm_runtime_enable(dev);
>         if (ret)
>                 return ret;
> --
> 2.39.2
>
>

[PATCH v2 09/15] iio: adc: rzg2l_adc: Prepare for the addition of RZ/G3S support

[Claudiu]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

The ADC IP available on the RZ/G3S differs slightly from the one found on
the RZ/G2L. The identified differences are as follows:
- different number of channels (one being used for temperature conversion);
  consequently, various registers differ
- different default sampling periods
- the RZ/G3S variant lacks the ADVIC register.

To accommodate these differences, the rzg2l_adc driver has been updated by
introducing the struct rzg2l_adc_hw_params, which encapsulates the
hardware-specific differences between the IP variants. A pointer to an
object of type struct rzg2l_adc_hw_params is embedded in
struct rzg2l_adc_data.

Additionally, the completion member of struct rzg2l_adc_data was relocated
to avoid potential padding, if any.

The code has been adjusted to utilize hardware-specific parameters stored
in the new structure instead of relying on plain macros.

The check of chan->channel in rzg2l_adc_read_raw() function, against the
driver specific mask was removed as the subsystem should have already
been done this before reaching the rzg2l_adc_read_raw() function. Along
with it the local variable ch was dropped as chan->channel could be used
instead.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v2:
- kept the RZG2L_ADC_MAX_CHANNELS as suggested in the review process;
  along with it, last_val[] is now again statically alocated; code
  from v1 around last_val has been adjusted to align with the new
  approach
- dropped ch variable from rzg2l_adc_read_raw() and adjusted the
  patch description to reflect it.

 drivers/iio/adc/rzg2l_adc.c | 87 +++++++++++++++++++++++++------------
 1 file changed, 59 insertions(+), 28 deletions(-)

diff --git a/drivers/iio/adc/rzg2l_adc.c b/drivers/iio/adc/rzg2l_adc.c
index c3f9f95cdbba..6740912f83c5 100644
--- a/drivers/iio/adc/rzg2l_adc.c
+++ b/drivers/iio/adc/rzg2l_adc.c
@@ -33,20 +33,15 @@
 #define RZG2L_ADM1_MS			BIT(2)
 #define RZG2L_ADM1_BS			BIT(4)
 #define RZG2L_ADM1_EGA_MASK		GENMASK(13, 12)
-#define RZG2L_ADM2_CHSEL_MASK		GENMASK(7, 0)
 #define RZG2L_ADM3_ADIL_MASK		GENMASK(31, 24)
 #define RZG2L_ADM3_ADCMP_MASK		GENMASK(23, 16)
-#define RZG2L_ADM3_ADCMP_E		FIELD_PREP(RZG2L_ADM3_ADCMP_MASK, 0xe)
-#define RZG2L_ADM3_ADSMP_MASK		GENMASK(15, 0)
 
 #define RZG2L_ADINT			0x20
-#define RZG2L_ADINT_INTEN_MASK		GENMASK(7, 0)
 #define RZG2L_ADINT_CSEEN		BIT(16)
 #define RZG2L_ADINT_INTS		BIT(31)
 
 #define RZG2L_ADSTS			0x24
 #define RZG2L_ADSTS_CSEST		BIT(16)
-#define RZG2L_ADSTS_INTST_MASK		GENMASK(7, 0)
 
 #define RZG2L_ADIVC			0x28
 #define RZG2L_ADIVC_DIVADC_MASK		GENMASK(8, 0)
@@ -57,12 +52,27 @@
 #define RZG2L_ADCR(n)			(0x30 + ((n) * 0x4))
 #define RZG2L_ADCR_AD_MASK		GENMASK(11, 0)
 
-#define RZG2L_ADSMP_DEFAULT_SAMPLING	0x578
-
 #define RZG2L_ADC_MAX_CHANNELS		8
-#define RZG2L_ADC_CHN_MASK		0x7
 #define RZG2L_ADC_TIMEOUT		usecs_to_jiffies(1 * 4)
 
+/**
+ * struct rzg2l_adc_hw_params - ADC hardware specific parameters
+ * @default_adsmp: default ADC sampling period (see ADM3 register)
+ * @adsmp_mask: ADC sampling period mask (see ADM3 register)
+ * @adint_inten_mask: conversion end interrupt mask (see ADINT register)
+ * @default_adcmp: default ADC cmp (see ADM3 register)
+ * @num_channels: number of supported channels
+ * @adivc: specifies if ADVIC register is available
+ */
+struct rzg2l_adc_hw_params {
+	u16 default_adsmp;
+	u16 adsmp_mask;
+	u16 adint_inten_mask;
+	u8 default_adcmp;
+	u8 num_channels;
+	bool adivc;
+};
+
 struct rzg2l_adc_data {
 	const struct iio_chan_spec *channels;
 	u8 num_channels;
@@ -72,8 +82,9 @@ struct rzg2l_adc {
 	void __iomem *base;
 	struct reset_control *presetn;
 	struct reset_control *adrstn;
-	struct completion completion;
 	const struct rzg2l_adc_data *data;
+	const struct rzg2l_adc_hw_params *hw_params;
+	struct completion completion;
 	struct mutex lock;
 	u16 last_val[RZG2L_ADC_MAX_CHANNELS];
 };
@@ -154,6 +165,7 @@ static void rzg2l_set_trigger(struct rzg2l_adc *adc)
 
 static int rzg2l_adc_conversion_setup(struct rzg2l_adc *adc, u8 ch)
 {
+	const struct rzg2l_adc_hw_params *hw_params = adc->hw_params;
 	u32 reg;
 
 	if (rzg2l_adc_readl(adc, RZG2L_ADM(0)) & RZG2L_ADM0_ADBSY)
@@ -163,7 +175,7 @@ static int rzg2l_adc_conversion_setup(struct rzg2l_adc *adc, u8 ch)
 
 	/* Select analog input channel subjected to conversion. */
 	reg = rzg2l_adc_readl(adc, RZG2L_ADM(2));
-	reg &= ~RZG2L_ADM2_CHSEL_MASK;
+	reg &= ~GENMASK(hw_params->num_channels - 1, 0);
 	reg |= BIT(ch);
 	rzg2l_adc_writel(adc, RZG2L_ADM(2), reg);
 
@@ -175,7 +187,7 @@ static int rzg2l_adc_conversion_setup(struct rzg2l_adc *adc, u8 ch)
 	 */
 	reg = rzg2l_adc_readl(adc, RZG2L_ADINT);
 	reg &= ~RZG2L_ADINT_INTS;
-	reg &= ~RZG2L_ADINT_INTEN_MASK;
+	reg &= ~hw_params->adint_inten_mask;
 	reg |= (RZG2L_ADINT_CSEEN | BIT(ch));
 	rzg2l_adc_writel(adc, RZG2L_ADINT, reg);
 
@@ -184,6 +196,7 @@ static int rzg2l_adc_conversion_setup(struct rzg2l_adc *adc, u8 ch)
 
 static int rzg2l_adc_conversion(struct iio_dev *indio_dev, struct rzg2l_adc *adc, u8 ch)
 {
+	const struct rzg2l_adc_hw_params *hw_params = adc->hw_params;
 	struct device *dev = indio_dev->dev.parent;
 	int ret;
 
@@ -201,7 +214,7 @@ static int rzg2l_adc_conversion(struct iio_dev *indio_dev, struct rzg2l_adc *adc
 
 	if (!wait_for_completion_timeout(&adc->completion, RZG2L_ADC_TIMEOUT)) {
 		rzg2l_adc_writel(adc, RZG2L_ADINT,
-				 rzg2l_adc_readl(adc, RZG2L_ADINT) & ~RZG2L_ADINT_INTEN_MASK);
+				 rzg2l_adc_readl(adc, RZG2L_ADINT) & ~hw_params->adint_inten_mask);
 		ret = -ETIMEDOUT;
 	}
 
@@ -219,7 +232,6 @@ static int rzg2l_adc_read_raw(struct iio_dev *indio_dev,
 {
 	struct rzg2l_adc *adc = iio_priv(indio_dev);
 	int ret;
-	u8 ch;
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW: {
@@ -228,12 +240,11 @@ static int rzg2l_adc_read_raw(struct iio_dev *indio_dev,
 
 		guard(mutex)(&adc->lock);
 
-		ch = chan->channel & RZG2L_ADC_CHN_MASK;
-		ret = rzg2l_adc_conversion(indio_dev, adc, ch);
+		ret = rzg2l_adc_conversion(indio_dev, adc, chan->channel);
 		if (ret)
 			return ret;
 
-		*val = adc->last_val[ch];
+		*val = adc->last_val[chan->channel];
 
 		return IIO_VAL_INT;
 	}
@@ -258,6 +269,7 @@ static const struct iio_info rzg2l_adc_iio_info = {
 static irqreturn_t rzg2l_adc_isr(int irq, void *dev_id)
 {
 	struct rzg2l_adc *adc = dev_id;
+	const struct rzg2l_adc_hw_params *hw_params = adc->hw_params;
 	unsigned long intst;
 	u32 reg;
 	int ch;
@@ -270,11 +282,11 @@ static irqreturn_t rzg2l_adc_isr(int irq, void *dev_id)
 		return IRQ_HANDLED;
 	}
 
-	intst = reg & RZG2L_ADSTS_INTST_MASK;
+	intst = reg & GENMASK(hw_params->num_channels - 1, 0);
 	if (!intst)
 		return IRQ_NONE;
 
-	for_each_set_bit(ch, &intst, RZG2L_ADC_MAX_CHANNELS)
+	for_each_set_bit(ch, &intst, hw_params->num_channels)
 		adc->last_val[ch] = rzg2l_adc_readl(adc, RZG2L_ADCR(ch)) & RZG2L_ADCR_AD_MASK;
 
 	/* clear the channel interrupt */
@@ -287,6 +299,7 @@ static irqreturn_t rzg2l_adc_isr(int irq, void *dev_id)
 
 static int rzg2l_adc_parse_properties(struct platform_device *pdev, struct rzg2l_adc *adc)
 {
+	const struct rzg2l_adc_hw_params *hw_params = adc->hw_params;
 	struct iio_chan_spec *chan_array;
 	struct rzg2l_adc_data *data;
 	unsigned int channel;
@@ -302,7 +315,7 @@ static int rzg2l_adc_parse_properties(struct platform_device *pdev, struct rzg2l
 	if (!num_channels)
 		return dev_err_probe(&pdev->dev, -ENODEV, "no channel children\n");
 
-	if (num_channels > RZG2L_ADC_MAX_CHANNELS)
+	if (num_channels > hw_params->num_channels)
 		return dev_err_probe(&pdev->dev, -EINVAL, "num of channel children out of range\n");
 
 	chan_array = devm_kcalloc(&pdev->dev, num_channels, sizeof(*chan_array),
@@ -316,7 +329,7 @@ static int rzg2l_adc_parse_properties(struct platform_device *pdev, struct rzg2l
 		if (ret)
 			return ret;
 
-		if (channel >= RZG2L_ADC_MAX_CHANNELS)
+		if (channel >= hw_params->num_channels)
 			return -EINVAL;
 
 		chan_array[i].type = IIO_VOLTAGE;
@@ -336,6 +349,7 @@ static int rzg2l_adc_parse_properties(struct platform_device *pdev, struct rzg2l
 
 static int rzg2l_adc_hw_init(struct device *dev, struct rzg2l_adc *adc)
 {
+	const struct rzg2l_adc_hw_params *hw_params = adc->hw_params;
 	u32 reg;
 	int ret;
 
@@ -353,11 +367,13 @@ static int rzg2l_adc_hw_init(struct device *dev, struct rzg2l_adc *adc)
 	if (ret)
 		goto exit_hw_init;
 
-	/* Only division by 4 can be set */
-	reg = rzg2l_adc_readl(adc, RZG2L_ADIVC);
-	reg &= ~RZG2L_ADIVC_DIVADC_MASK;
-	reg |= RZG2L_ADIVC_DIVADC_4;
-	rzg2l_adc_writel(adc, RZG2L_ADIVC, reg);
+	if (hw_params->adivc) {
+		/* Only division by 4 can be set */
+		reg = rzg2l_adc_readl(adc, RZG2L_ADIVC);
+		reg &= ~RZG2L_ADIVC_DIVADC_MASK;
+		reg |= RZG2L_ADIVC_DIVADC_4;
+		rzg2l_adc_writel(adc, RZG2L_ADIVC, reg);
+	}
 
 	/*
 	 * Setup AMD3
@@ -368,8 +384,10 @@ static int rzg2l_adc_hw_init(struct device *dev, struct rzg2l_adc *adc)
 	reg = rzg2l_adc_readl(adc, RZG2L_ADM(3));
 	reg &= ~RZG2L_ADM3_ADIL_MASK;
 	reg &= ~RZG2L_ADM3_ADCMP_MASK;
-	reg &= ~RZG2L_ADM3_ADSMP_MASK;
-	reg |= (RZG2L_ADM3_ADCMP_E | RZG2L_ADSMP_DEFAULT_SAMPLING);
+	reg &= ~hw_params->adsmp_mask;
+	reg |= FIELD_PREP(RZG2L_ADM3_ADCMP_MASK, hw_params->default_adcmp) |
+	       hw_params->default_adsmp;
+
 	rzg2l_adc_writel(adc, RZG2L_ADM(3), reg);
 
 exit_hw_init:
@@ -392,6 +410,10 @@ static int rzg2l_adc_probe(struct platform_device *pdev)
 
 	adc = iio_priv(indio_dev);
 
+	adc->hw_params = device_get_match_data(dev);
+	if (!adc->hw_params || adc->hw_params->num_channels > RZG2L_ADC_MAX_CHANNELS)
+		return -EINVAL;
+
 	ret = rzg2l_adc_parse_properties(pdev, adc);
 	if (ret)
 		return ret;
@@ -444,8 +466,17 @@ static int rzg2l_adc_probe(struct platform_device *pdev)
 	return devm_iio_device_register(dev, indio_dev);
 }
 
+static const struct rzg2l_adc_hw_params rzg2l_hw_params = {
+	.num_channels = 8,
+	.default_adcmp = 0xe,
+	.default_adsmp = 0x578,
+	.adsmp_mask = GENMASK(15, 0),
+	.adint_inten_mask = GENMASK(7, 0),
+	.adivc = true
+};
+
 static const struct of_device_id rzg2l_adc_match[] = {
-	{ .compatible = "renesas,rzg2l-adc",},
+	{ .compatible = "renesas,rzg2l-adc", .data = &rzg2l_hw_params },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, rzg2l_adc_match);
-- 
2.39.2

Re: [PATCH v2 09/15] iio: adc: rzg2l_adc: Prepare for the addition of RZ/G3S support

[Lad, Prabhakar]

On Fri, Dec 6, 2024 at 11:16 AM Claudiu <claudiu.beznea@tuxon.dev> wrote:
>
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> The ADC IP available on the RZ/G3S differs slightly from the one found on
> the RZ/G2L. The identified differences are as follows:
> - different number of channels (one being used for temperature conversion);
>   consequently, various registers differ
> - different default sampling periods
> - the RZ/G3S variant lacks the ADVIC register.
>
> To accommodate these differences, the rzg2l_adc driver has been updated by
> introducing the struct rzg2l_adc_hw_params, which encapsulates the
> hardware-specific differences between the IP variants. A pointer to an
> object of type struct rzg2l_adc_hw_params is embedded in
> struct rzg2l_adc_data.
>
> Additionally, the completion member of struct rzg2l_adc_data was relocated
> to avoid potential padding, if any.
>
> The code has been adjusted to utilize hardware-specific parameters stored
> in the new structure instead of relying on plain macros.
>
> The check of chan->channel in rzg2l_adc_read_raw() function, against the
> driver specific mask was removed as the subsystem should have already
> been done this before reaching the rzg2l_adc_read_raw() function. Along
> with it the local variable ch was dropped as chan->channel could be used
> instead.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
>
> Changes in v2:
> - kept the RZG2L_ADC_MAX_CHANNELS as suggested in the review process;
>   along with it, last_val[] is now again statically alocated; code
>   from v1 around last_val has been adjusted to align with the new
>   approach
> - dropped ch variable from rzg2l_adc_read_raw() and adjusted the
>   patch description to reflect it.
>
>  drivers/iio/adc/rzg2l_adc.c | 87 +++++++++++++++++++++++++------------
>  1 file changed, 59 insertions(+), 28 deletions(-)
>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Cheers,
Prabhakar

> diff --git a/drivers/iio/adc/rzg2l_adc.c b/drivers/iio/adc/rzg2l_adc.c
> index c3f9f95cdbba..6740912f83c5 100644
> --- a/drivers/iio/adc/rzg2l_adc.c
> +++ b/drivers/iio/adc/rzg2l_adc.c
> @@ -33,20 +33,15 @@
>  #define RZG2L_ADM1_MS                  BIT(2)
>  #define RZG2L_ADM1_BS                  BIT(4)
>  #define RZG2L_ADM1_EGA_MASK            GENMASK(13, 12)
> -#define RZG2L_ADM2_CHSEL_MASK          GENMASK(7, 0)
>  #define RZG2L_ADM3_ADIL_MASK           GENMASK(31, 24)
>  #define RZG2L_ADM3_ADCMP_MASK          GENMASK(23, 16)
> -#define RZG2L_ADM3_ADCMP_E             FIELD_PREP(RZG2L_ADM3_ADCMP_MASK, 0xe)
> -#define RZG2L_ADM3_ADSMP_MASK          GENMASK(15, 0)
>
>  #define RZG2L_ADINT                    0x20
> -#define RZG2L_ADINT_INTEN_MASK         GENMASK(7, 0)
>  #define RZG2L_ADINT_CSEEN              BIT(16)
>  #define RZG2L_ADINT_INTS               BIT(31)
>
>  #define RZG2L_ADSTS                    0x24
>  #define RZG2L_ADSTS_CSEST              BIT(16)
> -#define RZG2L_ADSTS_INTST_MASK         GENMASK(7, 0)
>
>  #define RZG2L_ADIVC                    0x28
>  #define RZG2L_ADIVC_DIVADC_MASK                GENMASK(8, 0)
> @@ -57,12 +52,27 @@
>  #define RZG2L_ADCR(n)                  (0x30 + ((n) * 0x4))
>  #define RZG2L_ADCR_AD_MASK             GENMASK(11, 0)
>
> -#define RZG2L_ADSMP_DEFAULT_SAMPLING   0x578
> -
>  #define RZG2L_ADC_MAX_CHANNELS         8
> -#define RZG2L_ADC_CHN_MASK             0x7
>  #define RZG2L_ADC_TIMEOUT              usecs_to_jiffies(1 * 4)
>
> +/**
> + * struct rzg2l_adc_hw_params - ADC hardware specific parameters
> + * @default_adsmp: default ADC sampling period (see ADM3 register)
> + * @adsmp_mask: ADC sampling period mask (see ADM3 register)
> + * @adint_inten_mask: conversion end interrupt mask (see ADINT register)
> + * @default_adcmp: default ADC cmp (see ADM3 register)
> + * @num_channels: number of supported channels
> + * @adivc: specifies if ADVIC register is available
> + */
> +struct rzg2l_adc_hw_params {
> +       u16 default_adsmp;
> +       u16 adsmp_mask;
> +       u16 adint_inten_mask;
> +       u8 default_adcmp;
> +       u8 num_channels;
> +       bool adivc;
> +};
> +
>  struct rzg2l_adc_data {
>         const struct iio_chan_spec *channels;
>         u8 num_channels;
> @@ -72,8 +82,9 @@ struct rzg2l_adc {
>         void __iomem *base;
>         struct reset_control *presetn;
>         struct reset_control *adrstn;
> -       struct completion completion;
>         const struct rzg2l_adc_data *data;
> +       const struct rzg2l_adc_hw_params *hw_params;
> +       struct completion completion;
>         struct mutex lock;
>         u16 last_val[RZG2L_ADC_MAX_CHANNELS];
>  };
> @@ -154,6 +165,7 @@ static void rzg2l_set_trigger(struct rzg2l_adc *adc)
>
>  static int rzg2l_adc_conversion_setup(struct rzg2l_adc *adc, u8 ch)
>  {
> +       const struct rzg2l_adc_hw_params *hw_params = adc->hw_params;
>         u32 reg;
>
>         if (rzg2l_adc_readl(adc, RZG2L_ADM(0)) & RZG2L_ADM0_ADBSY)
> @@ -163,7 +175,7 @@ static int rzg2l_adc_conversion_setup(struct rzg2l_adc *adc, u8 ch)
>
>         /* Select analog input channel subjected to conversion. */
>         reg = rzg2l_adc_readl(adc, RZG2L_ADM(2));
> -       reg &= ~RZG2L_ADM2_CHSEL_MASK;
> +       reg &= ~GENMASK(hw_params->num_channels - 1, 0);
>         reg |= BIT(ch);
>         rzg2l_adc_writel(adc, RZG2L_ADM(2), reg);
>
> @@ -175,7 +187,7 @@ static int rzg2l_adc_conversion_setup(struct rzg2l_adc *adc, u8 ch)
>          */
>         reg = rzg2l_adc_readl(adc, RZG2L_ADINT);
>         reg &= ~RZG2L_ADINT_INTS;
> -       reg &= ~RZG2L_ADINT_INTEN_MASK;
> +       reg &= ~hw_params->adint_inten_mask;
>         reg |= (RZG2L_ADINT_CSEEN | BIT(ch));
>         rzg2l_adc_writel(adc, RZG2L_ADINT, reg);
>
> @@ -184,6 +196,7 @@ static int rzg2l_adc_conversion_setup(struct rzg2l_adc *adc, u8 ch)
>
>  static int rzg2l_adc_conversion(struct iio_dev *indio_dev, struct rzg2l_adc *adc, u8 ch)
>  {
> +       const struct rzg2l_adc_hw_params *hw_params = adc->hw_params;
>         struct device *dev = indio_dev->dev.parent;
>         int ret;
>
> @@ -201,7 +214,7 @@ static int rzg2l_adc_conversion(struct iio_dev *indio_dev, struct rzg2l_adc *adc
>
>         if (!wait_for_completion_timeout(&adc->completion, RZG2L_ADC_TIMEOUT)) {
>                 rzg2l_adc_writel(adc, RZG2L_ADINT,
> -                                rzg2l_adc_readl(adc, RZG2L_ADINT) & ~RZG2L_ADINT_INTEN_MASK);
> +                                rzg2l_adc_readl(adc, RZG2L_ADINT) & ~hw_params->adint_inten_mask);
>                 ret = -ETIMEDOUT;
>         }
>
> @@ -219,7 +232,6 @@ static int rzg2l_adc_read_raw(struct iio_dev *indio_dev,
>  {
>         struct rzg2l_adc *adc = iio_priv(indio_dev);
>         int ret;
> -       u8 ch;
>
>         switch (mask) {
>         case IIO_CHAN_INFO_RAW: {
> @@ -228,12 +240,11 @@ static int rzg2l_adc_read_raw(struct iio_dev *indio_dev,
>
>                 guard(mutex)(&adc->lock);
>
> -               ch = chan->channel & RZG2L_ADC_CHN_MASK;
> -               ret = rzg2l_adc_conversion(indio_dev, adc, ch);
> +               ret = rzg2l_adc_conversion(indio_dev, adc, chan->channel);
>                 if (ret)
>                         return ret;
>
> -               *val = adc->last_val[ch];
> +               *val = adc->last_val[chan->channel];
>
>                 return IIO_VAL_INT;
>         }
> @@ -258,6 +269,7 @@ static const struct iio_info rzg2l_adc_iio_info = {
>  static irqreturn_t rzg2l_adc_isr(int irq, void *dev_id)
>  {
>         struct rzg2l_adc *adc = dev_id;
> +       const struct rzg2l_adc_hw_params *hw_params = adc->hw_params;
>         unsigned long intst;
>         u32 reg;
>         int ch;
> @@ -270,11 +282,11 @@ static irqreturn_t rzg2l_adc_isr(int irq, void *dev_id)
>                 return IRQ_HANDLED;
>         }
>
> -       intst = reg & RZG2L_ADSTS_INTST_MASK;
> +       intst = reg & GENMASK(hw_params->num_channels - 1, 0);
>         if (!intst)
>                 return IRQ_NONE;
>
> -       for_each_set_bit(ch, &intst, RZG2L_ADC_MAX_CHANNELS)
> +       for_each_set_bit(ch, &intst, hw_params->num_channels)
>                 adc->last_val[ch] = rzg2l_adc_readl(adc, RZG2L_ADCR(ch)) & RZG2L_ADCR_AD_MASK;
>
>         /* clear the channel interrupt */
> @@ -287,6 +299,7 @@ static irqreturn_t rzg2l_adc_isr(int irq, void *dev_id)
>
>  static int rzg2l_adc_parse_properties(struct platform_device *pdev, struct rzg2l_adc *adc)
>  {
> +       const struct rzg2l_adc_hw_params *hw_params = adc->hw_params;
>         struct iio_chan_spec *chan_array;
>         struct rzg2l_adc_data *data;
>         unsigned int channel;
> @@ -302,7 +315,7 @@ static int rzg2l_adc_parse_properties(struct platform_device *pdev, struct rzg2l
>         if (!num_channels)
>                 return dev_err_probe(&pdev->dev, -ENODEV, "no channel children\n");
>
> -       if (num_channels > RZG2L_ADC_MAX_CHANNELS)
> +       if (num_channels > hw_params->num_channels)
>                 return dev_err_probe(&pdev->dev, -EINVAL, "num of channel children out of range\n");
>
>         chan_array = devm_kcalloc(&pdev->dev, num_channels, sizeof(*chan_array),
> @@ -316,7 +329,7 @@ static int rzg2l_adc_parse_properties(struct platform_device *pdev, struct rzg2l
>                 if (ret)
>                         return ret;
>
> -               if (channel >= RZG2L_ADC_MAX_CHANNELS)
> +               if (channel >= hw_params->num_channels)
>                         return -EINVAL;
>
>                 chan_array[i].type = IIO_VOLTAGE;
> @@ -336,6 +349,7 @@ static int rzg2l_adc_parse_properties(struct platform_device *pdev, struct rzg2l
>
>  static int rzg2l_adc_hw_init(struct device *dev, struct rzg2l_adc *adc)
>  {
> +       const struct rzg2l_adc_hw_params *hw_params = adc->hw_params;
>         u32 reg;
>         int ret;
>
> @@ -353,11 +367,13 @@ static int rzg2l_adc_hw_init(struct device *dev, struct rzg2l_adc *adc)
>         if (ret)
>                 goto exit_hw_init;
>
> -       /* Only division by 4 can be set */
> -       reg = rzg2l_adc_readl(adc, RZG2L_ADIVC);
> -       reg &= ~RZG2L_ADIVC_DIVADC_MASK;
> -       reg |= RZG2L_ADIVC_DIVADC_4;
> -       rzg2l_adc_writel(adc, RZG2L_ADIVC, reg);
> +       if (hw_params->adivc) {
> +               /* Only division by 4 can be set */
> +               reg = rzg2l_adc_readl(adc, RZG2L_ADIVC);
> +               reg &= ~RZG2L_ADIVC_DIVADC_MASK;
> +               reg |= RZG2L_ADIVC_DIVADC_4;
> +               rzg2l_adc_writel(adc, RZG2L_ADIVC, reg);
> +       }
>
>         /*
>          * Setup AMD3
> @@ -368,8 +384,10 @@ static int rzg2l_adc_hw_init(struct device *dev, struct rzg2l_adc *adc)
>         reg = rzg2l_adc_readl(adc, RZG2L_ADM(3));
>         reg &= ~RZG2L_ADM3_ADIL_MASK;
>         reg &= ~RZG2L_ADM3_ADCMP_MASK;
> -       reg &= ~RZG2L_ADM3_ADSMP_MASK;
> -       reg |= (RZG2L_ADM3_ADCMP_E | RZG2L_ADSMP_DEFAULT_SAMPLING);
> +       reg &= ~hw_params->adsmp_mask;
> +       reg |= FIELD_PREP(RZG2L_ADM3_ADCMP_MASK, hw_params->default_adcmp) |
> +              hw_params->default_adsmp;
> +
>         rzg2l_adc_writel(adc, RZG2L_ADM(3), reg);
>
>  exit_hw_init:
> @@ -392,6 +410,10 @@ static int rzg2l_adc_probe(struct platform_device *pdev)
>
>         adc = iio_priv(indio_dev);
>
> +       adc->hw_params = device_get_match_data(dev);
> +       if (!adc->hw_params || adc->hw_params->num_channels > RZG2L_ADC_MAX_CHANNELS)
> +               return -EINVAL;
> +
>         ret = rzg2l_adc_parse_properties(pdev, adc);
>         if (ret)
>                 return ret;
> @@ -444,8 +466,17 @@ static int rzg2l_adc_probe(struct platform_device *pdev)
>         return devm_iio_device_register(dev, indio_dev);
>  }
>
> +static const struct rzg2l_adc_hw_params rzg2l_hw_params = {
> +       .num_channels = 8,
> +       .default_adcmp = 0xe,
> +       .default_adsmp = 0x578,
> +       .adsmp_mask = GENMASK(15, 0),
> +       .adint_inten_mask = GENMASK(7, 0),
> +       .adivc = true
> +};
> +
>  static const struct of_device_id rzg2l_adc_match[] = {
> -       { .compatible = "renesas,rzg2l-adc",},
> +       { .compatible = "renesas,rzg2l-adc", .data = &rzg2l_hw_params },
>         { /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, rzg2l_adc_match);
> --
> 2.39.2
>
>

[PATCH v2 10/15] iio: adc: rzg2l_adc: Add support for channel 8

[Claudiu]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

The ADC on the Renesas RZ/G3S SoC includes an additional channel (channel
8) dedicated to reading temperature values from the Thermal Sensor Unit
(TSU). There is a direct in-SoC connection between the ADC and TSU IPs.

To read the temperature reported by the TSU, a different sampling rate
(compared to channels 0-7) must be configured in the ADM3 register.

The rzg2l_adc driver has been updated to support reading the TSU
temperature.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v2:
- adjusted the RZG2L_ADC_MAX_CHANNELS
- introduced rzg2l_adc_ch_to_adsmp_index() and used it accordingly
- made the IIO_TEMP channel as raw channel as requested in the
  review process. I also realized having it as scale channel is
  wrong as the ADC doesn't actually report a temperature but a
  raw value that is then converted to a temperature with the
  help of the TSU (Thermal Sensor Unit) driver. Code from the
  TSU driver (not yet published) that reads the TSU sensor through
  the ADC and coverts the raw value to a temperature value is as
  follows:


// ...

#define TSU_READ_STEPS		8

/* Default calibration values, if FUSE values are missing */
#define SW_CALIB0_VAL	1297
#define SW_CALIB1_VAL	751

#define MCELSIUS(temp)		(temp * MILLIDEGREE_PER_DEGREE)

struct rzg3s_thermal_priv {
	void __iomem *base;
	struct device *dev;
	struct thermal_zone_device *tz;
	struct reset_control *rstc;
	struct iio_channel *channel;
	u16 calib0;
	u16 calib1;
};

// ...

static int rzg3s_thermal_get_temp(struct thermal_zone_device *tz, int *temp)
{
	struct rzg3s_thermal_priv *priv = thermal_zone_device_priv(tz);
	struct device *dev = priv->dev;
	u32 ts_code_ave = 0;
	int ret, val;

	ret = pm_runtime_resume_and_get(dev);
	if (ret)
		return ret;

	for (u8 i = 0; i < TSU_READ_STEPS; i++) {
		ret = iio_read_channel_raw(priv->channel, &val);
		if (ret < 0)
			goto rpm_put;
		
		ts_code_ave += val;
		/*
		 * According to HW manual (section 40.4.4 Procedure for Measuring the Temperature)
		 * we need to wait here at leat 3us.
		 */
		usleep_range(5, 10);
	}

	ret = 0;
	ts_code_ave = DIV_ROUND_CLOSEST(ts_code_ave, TSU_READ_STEPS);

	/*
	 * According to HW manual (section 40.4.4 Procedure for Measuring the Temperature)
	 * the formula to compute the temperature is as follows;
	 *
	 * Tj = (ts_code_ave - priv->calib0) * (165 / (priv->calib0 - priv->calib1)) - 40
	 */
	*temp = DIV_ROUND_CLOSEST_ULL(((u64)(ts_code_ave - priv->calib1) * 165),
				      (priv->calib0 - priv->calib1)) - 40;

	/* Round it up to 0.5 degrees Celsius and report it in Mili Celsius. */
	*temp = roundup(MCELSIUS(*temp), 500);

rpm_put:
	pm_runtime_mark_last_busy(dev);
	pm_runtime_put_autosuspend(dev);

	return ret;
}

// ...


 drivers/iio/adc/rzg2l_adc.c | 62 ++++++++++++++++++++++++++-----------
 1 file changed, 44 insertions(+), 18 deletions(-)

diff --git a/drivers/iio/adc/rzg2l_adc.c b/drivers/iio/adc/rzg2l_adc.c
index 6740912f83c5..e8dbc5dfbea1 100644
--- a/drivers/iio/adc/rzg2l_adc.c
+++ b/drivers/iio/adc/rzg2l_adc.c
@@ -52,12 +52,13 @@
 #define RZG2L_ADCR(n)			(0x30 + ((n) * 0x4))
 #define RZG2L_ADCR_AD_MASK		GENMASK(11, 0)
 
-#define RZG2L_ADC_MAX_CHANNELS		8
+#define RZG2L_ADC_MAX_CHANNELS		9
 #define RZG2L_ADC_TIMEOUT		usecs_to_jiffies(1 * 4)
 
 /**
  * struct rzg2l_adc_hw_params - ADC hardware specific parameters
- * @default_adsmp: default ADC sampling period (see ADM3 register)
+ * @default_adsmp: default ADC sampling period (see ADM3 register); index 0 is
+ * used for voltage channels, index 1 is used for temperature channel
  * @adsmp_mask: ADC sampling period mask (see ADM3 register)
  * @adint_inten_mask: conversion end interrupt mask (see ADINT register)
  * @default_adcmp: default ADC cmp (see ADM3 register)
@@ -65,7 +66,7 @@
  * @adivc: specifies if ADVIC register is available
  */
 struct rzg2l_adc_hw_params {
-	u16 default_adsmp;
+	u16 default_adsmp[2];
 	u16 adsmp_mask;
 	u16 adint_inten_mask;
 	u8 default_adcmp;
@@ -89,15 +90,26 @@ struct rzg2l_adc {
 	u16 last_val[RZG2L_ADC_MAX_CHANNELS];
 };
 
-static const char * const rzg2l_adc_channel_name[] = {
-	"adc0",
-	"adc1",
-	"adc2",
-	"adc3",
-	"adc4",
-	"adc5",
-	"adc6",
-	"adc7",
+/**
+ * struct rzg2l_adc_channel - ADC channel descriptor
+ * @name: ADC channel name
+ * @type: ADC channel type
+ */
+struct rzg2l_adc_channel {
+	const char * const name;
+	enum iio_chan_type type;
+};
+
+static const struct rzg2l_adc_channel rzg2l_adc_channels[] = {
+	{ "adc0", IIO_VOLTAGE },
+	{ "adc1", IIO_VOLTAGE },
+	{ "adc2", IIO_VOLTAGE },
+	{ "adc3", IIO_VOLTAGE },
+	{ "adc4", IIO_VOLTAGE },
+	{ "adc5", IIO_VOLTAGE },
+	{ "adc6", IIO_VOLTAGE },
+	{ "adc7", IIO_VOLTAGE },
+	{ "adc8", IIO_TEMP },
 };
 
 static unsigned int rzg2l_adc_readl(struct rzg2l_adc *adc, u32 reg)
@@ -163,9 +175,18 @@ static void rzg2l_set_trigger(struct rzg2l_adc *adc)
 	rzg2l_adc_writel(adc, RZG2L_ADM(1), reg);
 }
 
+static u8 rzg2l_adc_ch_to_adsmp_index(u8 ch)
+{
+	if (rzg2l_adc_channels[ch].type == IIO_VOLTAGE)
+		return 0;
+
+	return 1;
+}
+
 static int rzg2l_adc_conversion_setup(struct rzg2l_adc *adc, u8 ch)
 {
 	const struct rzg2l_adc_hw_params *hw_params = adc->hw_params;
+	u8 index = rzg2l_adc_ch_to_adsmp_index(ch);
 	u32 reg;
 
 	if (rzg2l_adc_readl(adc, RZG2L_ADM(0)) & RZG2L_ADM0_ADBSY)
@@ -179,6 +200,11 @@ static int rzg2l_adc_conversion_setup(struct rzg2l_adc *adc, u8 ch)
 	reg |= BIT(ch);
 	rzg2l_adc_writel(adc, RZG2L_ADM(2), reg);
 
+	reg = rzg2l_adc_readl(adc, RZG2L_ADM(3));
+	reg &= ~hw_params->adsmp_mask;
+	reg |= hw_params->default_adsmp[index];
+	rzg2l_adc_writel(adc, RZG2L_ADM(3), reg);
+
 	/*
 	 * Setup ADINT
 	 * INTS[31] - Select pulse signal
@@ -235,7 +261,7 @@ static int rzg2l_adc_read_raw(struct iio_dev *indio_dev,
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW: {
-		if (chan->type != IIO_VOLTAGE)
+		if (chan->type != IIO_VOLTAGE && chan->type != IIO_TEMP)
 			return -EINVAL;
 
 		guard(mutex)(&adc->lock);
@@ -258,7 +284,7 @@ static int rzg2l_adc_read_label(struct iio_dev *iio_dev,
 				const struct iio_chan_spec *chan,
 				char *label)
 {
-	return sysfs_emit(label, "%s\n", rzg2l_adc_channel_name[chan->channel]);
+	return sysfs_emit(label, "%s\n", rzg2l_adc_channels[chan->channel].name);
 }
 
 static const struct iio_info rzg2l_adc_iio_info = {
@@ -332,11 +358,11 @@ static int rzg2l_adc_parse_properties(struct platform_device *pdev, struct rzg2l
 		if (channel >= hw_params->num_channels)
 			return -EINVAL;
 
-		chan_array[i].type = IIO_VOLTAGE;
+		chan_array[i].type = rzg2l_adc_channels[channel].type;
 		chan_array[i].indexed = 1;
 		chan_array[i].channel = channel;
 		chan_array[i].info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
-		chan_array[i].datasheet_name = rzg2l_adc_channel_name[channel];
+		chan_array[i].datasheet_name = rzg2l_adc_channels[channel].name;
 		i++;
 	}
 
@@ -386,7 +412,7 @@ static int rzg2l_adc_hw_init(struct device *dev, struct rzg2l_adc *adc)
 	reg &= ~RZG2L_ADM3_ADCMP_MASK;
 	reg &= ~hw_params->adsmp_mask;
 	reg |= FIELD_PREP(RZG2L_ADM3_ADCMP_MASK, hw_params->default_adcmp) |
-	       hw_params->default_adsmp;
+	       hw_params->default_adsmp[0];
 
 	rzg2l_adc_writel(adc, RZG2L_ADM(3), reg);
 
@@ -469,7 +495,7 @@ static int rzg2l_adc_probe(struct platform_device *pdev)
 static const struct rzg2l_adc_hw_params rzg2l_hw_params = {
 	.num_channels = 8,
 	.default_adcmp = 0xe,
-	.default_adsmp = 0x578,
+	.default_adsmp = { 0x578 },
 	.adsmp_mask = GENMASK(15, 0),
 	.adint_inten_mask = GENMASK(7, 0),
 	.adivc = true
-- 
2.39.2

Re: [PATCH v2 10/15] iio: adc: rzg2l_adc: Add support for channel 8

[Lad, Prabhakar]

On Fri, Dec 6, 2024 at 11:16 AM Claudiu <claudiu.beznea@tuxon.dev> wrote:
>
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> The ADC on the Renesas RZ/G3S SoC includes an additional channel (channel
> 8) dedicated to reading temperature values from the Thermal Sensor Unit
> (TSU). There is a direct in-SoC connection between the ADC and TSU IPs.
>
> To read the temperature reported by the TSU, a different sampling rate
> (compared to channels 0-7) must be configured in the ADM3 register.
>
> The rzg2l_adc driver has been updated to support reading the TSU
> temperature.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
>
> Changes in v2:
> - adjusted the RZG2L_ADC_MAX_CHANNELS
> - introduced rzg2l_adc_ch_to_adsmp_index() and used it accordingly
> - made the IIO_TEMP channel as raw channel as requested in the
>   review process. I also realized having it as scale channel is
>   wrong as the ADC doesn't actually report a temperature but a
>   raw value that is then converted to a temperature with the
>   help of the TSU (Thermal Sensor Unit) driver. Code from the
>   TSU driver (not yet published) that reads the TSU sensor through
>   the ADC and coverts the raw value to a temperature value is as
>   follows:
>
>
> // ...
>
> #define TSU_READ_STEPS          8
>
> /* Default calibration values, if FUSE values are missing */
> #define SW_CALIB0_VAL   1297
> #define SW_CALIB1_VAL   751
>
> #define MCELSIUS(temp)          (temp * MILLIDEGREE_PER_DEGREE)
>
> struct rzg3s_thermal_priv {
>         void __iomem *base;
>         struct device *dev;
>         struct thermal_zone_device *tz;
>         struct reset_control *rstc;
>         struct iio_channel *channel;
>         u16 calib0;
>         u16 calib1;
> };
>
> // ...
>
> static int rzg3s_thermal_get_temp(struct thermal_zone_device *tz, int *temp)
> {
>         struct rzg3s_thermal_priv *priv = thermal_zone_device_priv(tz);
>         struct device *dev = priv->dev;
>         u32 ts_code_ave = 0;
>         int ret, val;
>
>         ret = pm_runtime_resume_and_get(dev);
>         if (ret)
>                 return ret;
>
>         for (u8 i = 0; i < TSU_READ_STEPS; i++) {
>                 ret = iio_read_channel_raw(priv->channel, &val);
>                 if (ret < 0)
>                         goto rpm_put;
>
>                 ts_code_ave += val;
>                 /*
>                  * According to HW manual (section 40.4.4 Procedure for Measuring the Temperature)
>                  * we need to wait here at leat 3us.
>                  */
>                 usleep_range(5, 10);
>         }
>
>         ret = 0;
>         ts_code_ave = DIV_ROUND_CLOSEST(ts_code_ave, TSU_READ_STEPS);
>
>         /*
>          * According to HW manual (section 40.4.4 Procedure for Measuring the Temperature)
>          * the formula to compute the temperature is as follows;
>          *
>          * Tj = (ts_code_ave - priv->calib0) * (165 / (priv->calib0 - priv->calib1)) - 40
>          */
>         *temp = DIV_ROUND_CLOSEST_ULL(((u64)(ts_code_ave - priv->calib1) * 165),
>                                       (priv->calib0 - priv->calib1)) - 40;
>
>         /* Round it up to 0.5 degrees Celsius and report it in Mili Celsius. */
>         *temp = roundup(MCELSIUS(*temp), 500);
>
> rpm_put:
>         pm_runtime_mark_last_busy(dev);
>         pm_runtime_put_autosuspend(dev);
>
>         return ret;
> }
>
> // ...
>
>
>  drivers/iio/adc/rzg2l_adc.c | 62 ++++++++++++++++++++++++++-----------
>  1 file changed, 44 insertions(+), 18 deletions(-)
>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Cheers,
Prabhakar

> diff --git a/drivers/iio/adc/rzg2l_adc.c b/drivers/iio/adc/rzg2l_adc.c
> index 6740912f83c5..e8dbc5dfbea1 100644
> --- a/drivers/iio/adc/rzg2l_adc.c
> +++ b/drivers/iio/adc/rzg2l_adc.c
> @@ -52,12 +52,13 @@
>  #define RZG2L_ADCR(n)                  (0x30 + ((n) * 0x4))
>  #define RZG2L_ADCR_AD_MASK             GENMASK(11, 0)
>
> -#define RZG2L_ADC_MAX_CHANNELS         8
> +#define RZG2L_ADC_MAX_CHANNELS         9
>  #define RZG2L_ADC_TIMEOUT              usecs_to_jiffies(1 * 4)
>
>  /**
>   * struct rzg2l_adc_hw_params - ADC hardware specific parameters
> - * @default_adsmp: default ADC sampling period (see ADM3 register)
> + * @default_adsmp: default ADC sampling period (see ADM3 register); index 0 is
> + * used for voltage channels, index 1 is used for temperature channel
>   * @adsmp_mask: ADC sampling period mask (see ADM3 register)
>   * @adint_inten_mask: conversion end interrupt mask (see ADINT register)
>   * @default_adcmp: default ADC cmp (see ADM3 register)
> @@ -65,7 +66,7 @@
>   * @adivc: specifies if ADVIC register is available
>   */
>  struct rzg2l_adc_hw_params {
> -       u16 default_adsmp;
> +       u16 default_adsmp[2];
>         u16 adsmp_mask;
>         u16 adint_inten_mask;
>         u8 default_adcmp;
> @@ -89,15 +90,26 @@ struct rzg2l_adc {
>         u16 last_val[RZG2L_ADC_MAX_CHANNELS];
>  };
>
> -static const char * const rzg2l_adc_channel_name[] = {
> -       "adc0",
> -       "adc1",
> -       "adc2",
> -       "adc3",
> -       "adc4",
> -       "adc5",
> -       "adc6",
> -       "adc7",
> +/**
> + * struct rzg2l_adc_channel - ADC channel descriptor
> + * @name: ADC channel name
> + * @type: ADC channel type
> + */
> +struct rzg2l_adc_channel {
> +       const char * const name;
> +       enum iio_chan_type type;
> +};
> +
> +static const struct rzg2l_adc_channel rzg2l_adc_channels[] = {
> +       { "adc0", IIO_VOLTAGE },
> +       { "adc1", IIO_VOLTAGE },
> +       { "adc2", IIO_VOLTAGE },
> +       { "adc3", IIO_VOLTAGE },
> +       { "adc4", IIO_VOLTAGE },
> +       { "adc5", IIO_VOLTAGE },
> +       { "adc6", IIO_VOLTAGE },
> +       { "adc7", IIO_VOLTAGE },
> +       { "adc8", IIO_TEMP },
>  };
>
>  static unsigned int rzg2l_adc_readl(struct rzg2l_adc *adc, u32 reg)
> @@ -163,9 +175,18 @@ static void rzg2l_set_trigger(struct rzg2l_adc *adc)
>         rzg2l_adc_writel(adc, RZG2L_ADM(1), reg);
>  }
>
> +static u8 rzg2l_adc_ch_to_adsmp_index(u8 ch)
> +{
> +       if (rzg2l_adc_channels[ch].type == IIO_VOLTAGE)
> +               return 0;
> +
> +       return 1;
> +}
> +
>  static int rzg2l_adc_conversion_setup(struct rzg2l_adc *adc, u8 ch)
>  {
>         const struct rzg2l_adc_hw_params *hw_params = adc->hw_params;
> +       u8 index = rzg2l_adc_ch_to_adsmp_index(ch);
>         u32 reg;
>
>         if (rzg2l_adc_readl(adc, RZG2L_ADM(0)) & RZG2L_ADM0_ADBSY)
> @@ -179,6 +200,11 @@ static int rzg2l_adc_conversion_setup(struct rzg2l_adc *adc, u8 ch)
>         reg |= BIT(ch);
>         rzg2l_adc_writel(adc, RZG2L_ADM(2), reg);
>
> +       reg = rzg2l_adc_readl(adc, RZG2L_ADM(3));
> +       reg &= ~hw_params->adsmp_mask;
> +       reg |= hw_params->default_adsmp[index];
> +       rzg2l_adc_writel(adc, RZG2L_ADM(3), reg);
> +
>         /*
>          * Setup ADINT
>          * INTS[31] - Select pulse signal
> @@ -235,7 +261,7 @@ static int rzg2l_adc_read_raw(struct iio_dev *indio_dev,
>
>         switch (mask) {
>         case IIO_CHAN_INFO_RAW: {
> -               if (chan->type != IIO_VOLTAGE)
> +               if (chan->type != IIO_VOLTAGE && chan->type != IIO_TEMP)
>                         return -EINVAL;
>
>                 guard(mutex)(&adc->lock);
> @@ -258,7 +284,7 @@ static int rzg2l_adc_read_label(struct iio_dev *iio_dev,
>                                 const struct iio_chan_spec *chan,
>                                 char *label)
>  {
> -       return sysfs_emit(label, "%s\n", rzg2l_adc_channel_name[chan->channel]);
> +       return sysfs_emit(label, "%s\n", rzg2l_adc_channels[chan->channel].name);
>  }
>
>  static const struct iio_info rzg2l_adc_iio_info = {
> @@ -332,11 +358,11 @@ static int rzg2l_adc_parse_properties(struct platform_device *pdev, struct rzg2l
>                 if (channel >= hw_params->num_channels)
>                         return -EINVAL;
>
> -               chan_array[i].type = IIO_VOLTAGE;
> +               chan_array[i].type = rzg2l_adc_channels[channel].type;
>                 chan_array[i].indexed = 1;
>                 chan_array[i].channel = channel;
>                 chan_array[i].info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
> -               chan_array[i].datasheet_name = rzg2l_adc_channel_name[channel];
> +               chan_array[i].datasheet_name = rzg2l_adc_channels[channel].name;
>                 i++;
>         }
>
> @@ -386,7 +412,7 @@ static int rzg2l_adc_hw_init(struct device *dev, struct rzg2l_adc *adc)
>         reg &= ~RZG2L_ADM3_ADCMP_MASK;
>         reg &= ~hw_params->adsmp_mask;
>         reg |= FIELD_PREP(RZG2L_ADM3_ADCMP_MASK, hw_params->default_adcmp) |
> -              hw_params->default_adsmp;
> +              hw_params->default_adsmp[0];
>
>         rzg2l_adc_writel(adc, RZG2L_ADM(3), reg);
>
> @@ -469,7 +495,7 @@ static int rzg2l_adc_probe(struct platform_device *pdev)
>  static const struct rzg2l_adc_hw_params rzg2l_hw_params = {
>         .num_channels = 8,
>         .default_adcmp = 0xe,
> -       .default_adsmp = 0x578,
> +       .default_adsmp = { 0x578 },
>         .adsmp_mask = GENMASK(15, 0),
>         .adint_inten_mask = GENMASK(7, 0),
>         .adivc = true
> --
> 2.39.2
>
>

[PATCH v2 11/15] iio: adc: rzg2l_adc: Add suspend/resume support

[Claudiu]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

The Renesas RZ/G3S SoC features a power-saving mode where power to most of
the SoC components is turned off, including the ADC IP.

Suspend/resume support has been added to the rzg2l_adc driver to restore
functionality after resuming from this power-saving mode. During suspend,
the ADC resets are asserted, and the ADC is powered down. On resume, the
ADC resets are de-asserted, the hardware is re-initialized, and the ADC
power is restored using the runtime PM APIs.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v2:
- none

 drivers/iio/adc/rzg2l_adc.c | 70 +++++++++++++++++++++++++++++++++++++
 1 file changed, 70 insertions(+)

diff --git a/drivers/iio/adc/rzg2l_adc.c b/drivers/iio/adc/rzg2l_adc.c
index e8dbc5dfbea1..2a911269a358 100644
--- a/drivers/iio/adc/rzg2l_adc.c
+++ b/drivers/iio/adc/rzg2l_adc.c
@@ -88,6 +88,7 @@ struct rzg2l_adc {
 	struct completion completion;
 	struct mutex lock;
 	u16 last_val[RZG2L_ADC_MAX_CHANNELS];
+	bool was_rpm_active;
 };
 
 /**
@@ -527,8 +528,77 @@ static int rzg2l_adc_pm_runtime_resume(struct device *dev)
 	return 0;
 }
 
+static int rzg2l_adc_suspend(struct device *dev)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct rzg2l_adc *adc = iio_priv(indio_dev);
+	struct reset_control_bulk_data resets[] = {
+		{ .rstc = adc->presetn },
+		{ .rstc = adc->adrstn },
+	};
+	int ret;
+
+	if (pm_runtime_suspended(dev)) {
+		adc->was_rpm_active = false;
+	} else {
+		ret = pm_runtime_force_suspend(dev);
+		if (ret)
+			return ret;
+		adc->was_rpm_active = true;
+	}
+
+	ret = reset_control_bulk_assert(ARRAY_SIZE(resets), resets);
+	if (ret)
+		goto rpm_restore;
+
+	return 0;
+
+rpm_restore:
+	if (adc->was_rpm_active)
+		pm_runtime_force_resume(dev);
+
+	return ret;
+}
+
+static int rzg2l_adc_resume(struct device *dev)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct rzg2l_adc *adc = iio_priv(indio_dev);
+	struct reset_control_bulk_data resets[] = {
+		{ .rstc = adc->adrstn },
+		{ .rstc = adc->presetn },
+	};
+	int ret;
+
+	ret = reset_control_bulk_deassert(ARRAY_SIZE(resets), resets);
+	if (ret)
+		return ret;
+
+	if (adc->was_rpm_active) {
+		ret = pm_runtime_force_resume(dev);
+		if (ret)
+			goto resets_restore;
+	}
+
+	ret = rzg2l_adc_hw_init(dev, adc);
+	if (ret)
+		goto rpm_restore;
+
+	return 0;
+
+rpm_restore:
+	if (adc->was_rpm_active) {
+		pm_runtime_mark_last_busy(dev);
+		pm_runtime_put_autosuspend(dev);
+	}
+resets_restore:
+	reset_control_bulk_assert(ARRAY_SIZE(resets), resets);
+	return ret;
+}
+
 static const struct dev_pm_ops rzg2l_adc_pm_ops = {
 	RUNTIME_PM_OPS(rzg2l_adc_pm_runtime_suspend, rzg2l_adc_pm_runtime_resume, NULL)
+	SYSTEM_SLEEP_PM_OPS(rzg2l_adc_suspend, rzg2l_adc_resume)
 };
 
 static struct platform_driver rzg2l_adc_driver = {
-- 
2.39.2

Re: [PATCH v2 11/15] iio: adc: rzg2l_adc: Add suspend/resume support

[Lad, Prabhakar]

On Fri, Dec 6, 2024 at 11:16 AM Claudiu <claudiu.beznea@tuxon.dev> wrote:
>
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> The Renesas RZ/G3S SoC features a power-saving mode where power to most of
> the SoC components is turned off, including the ADC IP.
>
> Suspend/resume support has been added to the rzg2l_adc driver to restore
> functionality after resuming from this power-saving mode. During suspend,
> the ADC resets are asserted, and the ADC is powered down. On resume, the
> ADC resets are de-asserted, the hardware is re-initialized, and the ADC
> power is restored using the runtime PM APIs.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
>
> Changes in v2:
> - none
>
>  drivers/iio/adc/rzg2l_adc.c | 70 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 70 insertions(+)
>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Cheers,
Prabhakar

> diff --git a/drivers/iio/adc/rzg2l_adc.c b/drivers/iio/adc/rzg2l_adc.c
> index e8dbc5dfbea1..2a911269a358 100644
> --- a/drivers/iio/adc/rzg2l_adc.c
> +++ b/drivers/iio/adc/rzg2l_adc.c
> @@ -88,6 +88,7 @@ struct rzg2l_adc {
>         struct completion completion;
>         struct mutex lock;
>         u16 last_val[RZG2L_ADC_MAX_CHANNELS];
> +       bool was_rpm_active;
>  };
>
>  /**
> @@ -527,8 +528,77 @@ static int rzg2l_adc_pm_runtime_resume(struct device *dev)
>         return 0;
>  }
>
> +static int rzg2l_adc_suspend(struct device *dev)
> +{
> +       struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +       struct rzg2l_adc *adc = iio_priv(indio_dev);
> +       struct reset_control_bulk_data resets[] = {
> +               { .rstc = adc->presetn },
> +               { .rstc = adc->adrstn },
> +       };
> +       int ret;
> +
> +       if (pm_runtime_suspended(dev)) {
> +               adc->was_rpm_active = false;
> +       } else {
> +               ret = pm_runtime_force_suspend(dev);
> +               if (ret)
> +                       return ret;
> +               adc->was_rpm_active = true;
> +       }
> +
> +       ret = reset_control_bulk_assert(ARRAY_SIZE(resets), resets);
> +       if (ret)
> +               goto rpm_restore;
> +
> +       return 0;
> +
> +rpm_restore:
> +       if (adc->was_rpm_active)
> +               pm_runtime_force_resume(dev);
> +
> +       return ret;
> +}
> +
> +static int rzg2l_adc_resume(struct device *dev)
> +{
> +       struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +       struct rzg2l_adc *adc = iio_priv(indio_dev);
> +       struct reset_control_bulk_data resets[] = {
> +               { .rstc = adc->adrstn },
> +               { .rstc = adc->presetn },
> +       };
> +       int ret;
> +
> +       ret = reset_control_bulk_deassert(ARRAY_SIZE(resets), resets);
> +       if (ret)
> +               return ret;
> +
> +       if (adc->was_rpm_active) {
> +               ret = pm_runtime_force_resume(dev);
> +               if (ret)
> +                       goto resets_restore;
> +       }
> +
> +       ret = rzg2l_adc_hw_init(dev, adc);
> +       if (ret)
> +               goto rpm_restore;
> +
> +       return 0;
> +
> +rpm_restore:
> +       if (adc->was_rpm_active) {
> +               pm_runtime_mark_last_busy(dev);
> +               pm_runtime_put_autosuspend(dev);
> +       }
> +resets_restore:
> +       reset_control_bulk_assert(ARRAY_SIZE(resets), resets);
> +       return ret;
> +}
> +
>  static const struct dev_pm_ops rzg2l_adc_pm_ops = {
>         RUNTIME_PM_OPS(rzg2l_adc_pm_runtime_suspend, rzg2l_adc_pm_runtime_resume, NULL)
> +       SYSTEM_SLEEP_PM_OPS(rzg2l_adc_suspend, rzg2l_adc_resume)
>  };
>
>  static struct platform_driver rzg2l_adc_driver = {
> --
> 2.39.2
>
>

Re: [PATCH v2 11/15] iio: adc: rzg2l_adc: Add suspend/resume support

[Jonathan Cameron]

On Sun, 8 Dec 2024 21:35:50 +0000
"Lad, Prabhakar" <prabhakar.csengg@gmail.com> wrote:

> On Fri, Dec 6, 2024 at 11:16 AM Claudiu <claudiu.beznea@tuxon.dev> wrote:
> >
> > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> >
> > The Renesas RZ/G3S SoC features a power-saving mode where power to most of
> > the SoC components is turned off, including the ADC IP.
> >
> > Suspend/resume support has been added to the rzg2l_adc driver to restore
> > functionality after resuming from this power-saving mode. During suspend,
> > the ADC resets are asserted, and the ADC is powered down. On resume, the
> > ADC resets are de-asserted, the hardware is re-initialized, and the ADC
> > power is restored using the runtime PM APIs.
> >
> > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> > ---
> >
> > Changes in v2:
> > - none
> >
> >  drivers/iio/adc/rzg2l_adc.c | 70 +++++++++++++++++++++++++++++++++++++
> >  1 file changed, 70 insertions(+)
> >  
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Thanks.  I've updated tags.

Jonathan

> 
> Cheers,
> Prabhakar
> 
> > diff --git a/drivers/iio/adc/rzg2l_adc.c b/drivers/iio/adc/rzg2l_adc.c
> > index e8dbc5dfbea1..2a911269a358 100644
> > --- a/drivers/iio/adc/rzg2l_adc.c
> > +++ b/drivers/iio/adc/rzg2l_adc.c
> > @@ -88,6 +88,7 @@ struct rzg2l_adc {
> >         struct completion completion;
> >         struct mutex lock;
> >         u16 last_val[RZG2L_ADC_MAX_CHANNELS];
> > +       bool was_rpm_active;
> >  };
> >
> >  /**
> > @@ -527,8 +528,77 @@ static int rzg2l_adc_pm_runtime_resume(struct device *dev)
> >         return 0;
> >  }
> >
> > +static int rzg2l_adc_suspend(struct device *dev)
> > +{
> > +       struct iio_dev *indio_dev = dev_get_drvdata(dev);
> > +       struct rzg2l_adc *adc = iio_priv(indio_dev);
> > +       struct reset_control_bulk_data resets[] = {
> > +               { .rstc = adc->presetn },
> > +               { .rstc = adc->adrstn },
> > +       };
> > +       int ret;
> > +
> > +       if (pm_runtime_suspended(dev)) {
> > +               adc->was_rpm_active = false;
> > +       } else {
> > +               ret = pm_runtime_force_suspend(dev);
> > +               if (ret)
> > +                       return ret;
> > +               adc->was_rpm_active = true;
> > +       }
> > +
> > +       ret = reset_control_bulk_assert(ARRAY_SIZE(resets), resets);
> > +       if (ret)
> > +               goto rpm_restore;
> > +
> > +       return 0;
> > +
> > +rpm_restore:
> > +       if (adc->was_rpm_active)
> > +               pm_runtime_force_resume(dev);
> > +
> > +       return ret;
> > +}
> > +
> > +static int rzg2l_adc_resume(struct device *dev)
> > +{
> > +       struct iio_dev *indio_dev = dev_get_drvdata(dev);
> > +       struct rzg2l_adc *adc = iio_priv(indio_dev);
> > +       struct reset_control_bulk_data resets[] = {
> > +               { .rstc = adc->adrstn },
> > +               { .rstc = adc->presetn },
> > +       };
> > +       int ret;
> > +
> > +       ret = reset_control_bulk_deassert(ARRAY_SIZE(resets), resets);
> > +       if (ret)
> > +               return ret;
> > +
> > +       if (adc->was_rpm_active) {
> > +               ret = pm_runtime_force_resume(dev);
> > +               if (ret)
> > +                       goto resets_restore;
> > +       }
> > +
> > +       ret = rzg2l_adc_hw_init(dev, adc);
> > +       if (ret)
> > +               goto rpm_restore;
> > +
> > +       return 0;
> > +
> > +rpm_restore:
> > +       if (adc->was_rpm_active) {
> > +               pm_runtime_mark_last_busy(dev);
> > +               pm_runtime_put_autosuspend(dev);
> > +       }
> > +resets_restore:
> > +       reset_control_bulk_assert(ARRAY_SIZE(resets), resets);
> > +       return ret;
> > +}
> > +
> >  static const struct dev_pm_ops rzg2l_adc_pm_ops = {
> >         RUNTIME_PM_OPS(rzg2l_adc_pm_runtime_suspend, rzg2l_adc_pm_runtime_resume, NULL)
> > +       SYSTEM_SLEEP_PM_OPS(rzg2l_adc_suspend, rzg2l_adc_resume)
> >  };
> >
> >  static struct platform_driver rzg2l_adc_driver = {
> > --
> > 2.39.2
> >
> >  

[PATCH v2 12/15] dt-bindings: iio: adc: renesas,rzg2l-adc: Document RZ/G3S SoC

[Claudiu]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Document the ADC IP available on the RZ/G3S SoC. The ADC IP on the RZ/G3S
differs slightly from the one found on the RZ/G2L. The identified
differences are as follows:
- different number of channels (one being used for temperature conversion);
  consequently, various registers differ; the temperature channel
  support was not available for the RZ/G2L variant; the #io-channel-cells
  property was added to be able to request the temperature channel from
  the thermal driver
- different default sampling periods
- the RZ/G3S variant lacks the ADVIC register.

Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v2:
- collected tags

 .../bindings/iio/adc/renesas,rzg2l-adc.yaml   | 37 +++++++++++++------
 1 file changed, 26 insertions(+), 11 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml b/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml
index ba86c7b7d622..40341d541726 100644
--- a/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml
@@ -17,12 +17,15 @@ description: |
 
 properties:
   compatible:
-    items:
-      - enum:
-          - renesas,r9a07g043-adc   # RZ/G2UL and RZ/Five
-          - renesas,r9a07g044-adc   # RZ/G2L
-          - renesas,r9a07g054-adc   # RZ/V2L
-      - const: renesas,rzg2l-adc
+    oneOf:
+      - items:
+          - enum:
+              - renesas,r9a07g043-adc   # RZ/G2UL and RZ/Five
+              - renesas,r9a07g044-adc   # RZ/G2L
+              - renesas,r9a07g054-adc   # RZ/V2L
+          - const: renesas,rzg2l-adc
+      - items:
+          - const: renesas,r9a08g045-adc  # RZ/G3S
 
   reg:
     maxItems: 1
@@ -57,6 +60,9 @@ properties:
   '#size-cells':
     const: 0
 
+  "#io-channel-cells":
+    const: 1
+
 required:
   - compatible
   - reg
@@ -68,7 +74,7 @@ required:
   - reset-names
 
 patternProperties:
-  "^channel@[0-7]$":
+  "^channel@[0-8]$":
     $ref: adc.yaml
     type: object
     description: |
@@ -78,6 +84,8 @@ patternProperties:
       reg:
         description: |
           The channel number.
+        minimum: 0
+        maximum: 8
 
     required:
       - reg
@@ -92,18 +100,25 @@ allOf:
             const: renesas,r9a07g043-adc
     then:
       patternProperties:
-        "^channel@[2-7]$": false
+        "^channel@[2-8]$": false
         "^channel@[0-1]$":
           properties:
             reg:
-              minimum: 0
               maximum: 1
-    else:
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - renesas,r9a07g044-adc
+              - renesas,r9a07g054-adc
+    then:
       patternProperties:
+        "^channel@[8]$": false
         "^channel@[0-7]$":
           properties:
             reg:
-              minimum: 0
               maximum: 7
 
 additionalProperties: false
-- 
2.39.2

[PATCH v2 13/15] iio: adc: rzg2l_adc: Add support for Renesas RZ/G3S

[Claudiu]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Add ADC support for the Renesas RZ/G3S SoC. The key features of this IP
include:
- 9 channels, with one dedicated to reading the temperature reported by the
  Thermal Sensor Unit (TSU)
- A different default ADCMP value, which is written to the ADM3 register.
- Different default sampling rates
- ADM3.ADSMP field is 8 bits wide
- ADINT.INTEN field is 11 bits wide

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v2:
- none

 drivers/iio/adc/rzg2l_adc.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/iio/adc/rzg2l_adc.c b/drivers/iio/adc/rzg2l_adc.c
index 2a911269a358..81904e2c4075 100644
--- a/drivers/iio/adc/rzg2l_adc.c
+++ b/drivers/iio/adc/rzg2l_adc.c
@@ -502,7 +502,16 @@ static const struct rzg2l_adc_hw_params rzg2l_hw_params = {
 	.adivc = true
 };
 
+static const struct rzg2l_adc_hw_params rzg3s_hw_params = {
+	.num_channels = 9,
+	.default_adcmp = 0x1d,
+	.default_adsmp = { 0x7f, 0xff },
+	.adsmp_mask = GENMASK(7, 0),
+	.adint_inten_mask = GENMASK(11, 0),
+};
+
 static const struct of_device_id rzg2l_adc_match[] = {
+	{ .compatible = "renesas,r9a08g045-adc", .data = &rzg3s_hw_params },
 	{ .compatible = "renesas,rzg2l-adc", .data = &rzg2l_hw_params },
 	{ /* sentinel */ }
 };
-- 
2.39.2

Re: [PATCH v2 13/15] iio: adc: rzg2l_adc: Add support for Renesas RZ/G3S

[Jonathan Cameron]

On Fri,  6 Dec 2024 13:13:35 +0200
Claudiu <claudiu.beznea@tuxon.dev> wrote:

> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> Add ADC support for the Renesas RZ/G3S SoC. The key features of this IP
> include:
> - 9 channels, with one dedicated to reading the temperature reported by the
>   Thermal Sensor Unit (TSU)
> - A different default ADCMP value, which is written to the ADM3 register.
> - Different default sampling rates
> - ADM3.ADSMP field is 8 bits wide
> - ADINT.INTEN field is 11 bits wide
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
Hi Claudiu

As my comments were all minor stuff, I have applied this.
However they were the sort of minor changes that result in lots of
fuzz and hand editing when applying so please check the result.
Applied to the testing branch of iio.git.

Thanks,

Jonathan

> ---
> 
> Changes in v2:
> - none
> 
>  drivers/iio/adc/rzg2l_adc.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/iio/adc/rzg2l_adc.c b/drivers/iio/adc/rzg2l_adc.c
> index 2a911269a358..81904e2c4075 100644
> --- a/drivers/iio/adc/rzg2l_adc.c
> +++ b/drivers/iio/adc/rzg2l_adc.c
> @@ -502,7 +502,16 @@ static const struct rzg2l_adc_hw_params rzg2l_hw_params = {
>  	.adivc = true
>  };
>  
> +static const struct rzg2l_adc_hw_params rzg3s_hw_params = {
> +	.num_channels = 9,
> +	.default_adcmp = 0x1d,
> +	.default_adsmp = { 0x7f, 0xff },
> +	.adsmp_mask = GENMASK(7, 0),
> +	.adint_inten_mask = GENMASK(11, 0),
> +};
> +
>  static const struct of_device_id rzg2l_adc_match[] = {
> +	{ .compatible = "renesas,r9a08g045-adc", .data = &rzg3s_hw_params },
>  	{ .compatible = "renesas,rzg2l-adc", .data = &rzg2l_hw_params },
>  	{ /* sentinel */ }
>  };

Re: [PATCH v2 13/15] iio: adc: rzg2l_adc: Add support for Renesas RZ/G3S

[Claudiu Beznea]

Hi, Jonathan,

On 07.12.2024 20:34, Jonathan Cameron wrote:
> On Fri,  6 Dec 2024 13:13:35 +0200
> Claudiu <claudiu.beznea@tuxon.dev> wrote:
> 
>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>
>> Add ADC support for the Renesas RZ/G3S SoC. The key features of this IP
>> include:
>> - 9 channels, with one dedicated to reading the temperature reported by the
>>   Thermal Sensor Unit (TSU)
>> - A different default ADCMP value, which is written to the ADM3 register.
>> - Different default sampling rates
>> - ADM3.ADSMP field is 8 bits wide
>> - ADINT.INTEN field is 11 bits wide
>>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> Hi Claudiu
> 
> As my comments were all minor stuff, I have applied this.
> However they were the sort of minor changes that result in lots of
> fuzz and hand editing when applying so please check the result.
> Applied to the testing branch of iio.git.

I checked and tested testing branch at iio.git. Everything is good.

Thank you for taking care of this,
Claudiu


> 
> Thanks,
> 
> Jonathan

[PATCH v2 14/15] arm64: dts: renesas: r9a08g045: Add ADC node

[Claudiu]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Add the device tree node for the ADC IP available on the Renesas RZ/G3S
SoC.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v2:
- none

 arch/arm64/boot/dts/renesas/r9a08g045.dtsi | 53 ++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r9a08g045.dtsi b/arch/arm64/boot/dts/renesas/r9a08g045.dtsi
index be8a0a768c65..eb57a52d2086 100644
--- a/arch/arm64/boot/dts/renesas/r9a08g045.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a08g045.dtsi
@@ -87,6 +87,59 @@ rtc: rtc@1004ec00 {
 			status = "disabled";
 		};
 
+		adc: adc@10058000 {
+			compatible = "renesas,r9a08g045-adc";
+			reg = <0 0x10058000 0 0x400>;
+			interrupts = <GIC_SPI 312 IRQ_TYPE_EDGE_RISING>;
+			clocks = <&cpg CPG_MOD R9A08G045_ADC_ADCLK>,
+				 <&cpg CPG_MOD R9A08G045_ADC_PCLK>;
+			clock-names = "adclk", "pclk";
+			resets = <&cpg R9A08G045_ADC_PRESETN>,
+				 <&cpg R9A08G045_ADC_ADRST_N>;
+			reset-names = "presetn", "adrst-n";
+			power-domains = <&cpg>;
+			#io-channel-cells = <1>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+
+			channel@0 {
+				reg = <0>;
+			};
+
+			channel@1 {
+				reg = <1>;
+			};
+
+			channel@2 {
+				reg = <2>;
+			};
+
+			channel@3 {
+				reg = <3>;
+			};
+
+			channel@4 {
+				reg = <4>;
+			};
+
+			channel@5 {
+				reg = <5>;
+			};
+
+			channel@6 {
+				reg = <6>;
+			};
+
+			channel@7 {
+				reg = <7>;
+			};
+
+			channel@8 {
+				reg = <8>;
+			};
+		};
+
 		vbattb: clock-controller@1005c000 {
 			compatible = "renesas,r9a08g045-vbattb";
 			reg = <0 0x1005c000 0 0x1000>;
-- 
2.39.2

Re: [PATCH v2 14/15] arm64: dts: renesas: r9a08g045: Add ADC node

[Geert Uytterhoeven]

Hi Claudiu,

On Fri, Dec 6, 2024 at 12:14 PM Claudiu <claudiu.beznea@tuxon.dev> wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> Add the device tree node for the ADC IP available on the Renesas RZ/G3S
> SoC.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Thanks for your patch!

> --- a/arch/arm64/boot/dts/renesas/r9a08g045.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r9a08g045.dtsi
> @@ -87,6 +87,59 @@ rtc: rtc@1004ec00 {
>                         status = "disabled";
>                 };
>
> +               adc: adc@10058000 {
> +                       compatible = "renesas,r9a08g045-adc";
> +                       reg = <0 0x10058000 0 0x400>;

Table 5.1 ("Detailed Address Space") says the size is 4 KiB.

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-devel for v6.14, with the above fixed.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

Re: [PATCH v2 14/15] arm64: dts: renesas: r9a08g045: Add ADC node

[Claudiu Beznea]

Hi, Geert,

On 11.12.2024 15:27, Geert Uytterhoeven wrote:
> Hi Claudiu,
> 
> On Fri, Dec 6, 2024 at 12:14 PM Claudiu <claudiu.beznea@tuxon.dev> wrote:
>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>
>> Add the device tree node for the ADC IP available on the Renesas RZ/G3S
>> SoC.
>>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> Thanks for your patch!
> 
>> --- a/arch/arm64/boot/dts/renesas/r9a08g045.dtsi
>> +++ b/arch/arm64/boot/dts/renesas/r9a08g045.dtsi
>> @@ -87,6 +87,59 @@ rtc: rtc@1004ec00 {
>>                         status = "disabled";
>>                 };
>>
>> +               adc: adc@10058000 {
>> +                       compatible = "renesas,r9a08g045-adc";
>> +                       reg = <0 0x10058000 0 0x400>;
> 
> Table 5.1 ("Detailed Address Space") says the size is 4 KiB.
> 
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> i.e. will queue in renesas-devel for v6.14, with the above fixed.

Thank you!

> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 

[PATCH v2 15/15] arm64: dts: renesas: rzg3s-smarc-som: Enable ADC

[Claudiu]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Enable ADC.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v2:
- none

 arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi b/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi
index 2ed01d391554..57ebdfc858eb 100644
--- a/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi
+++ b/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi
@@ -94,6 +94,10 @@ vcc_sdhi2: regulator2 {
 	};
 };
 
+&adc {
+	status = "okay";
+};
+
 #if SW_CONFIG3 == SW_ON
 &eth0 {
 	pinctrl-0 = <&eth0_pins>;
-- 
2.39.2

Re: [PATCH v2 15/15] arm64: dts: renesas: rzg3s-smarc-som: Enable ADC

[Geert Uytterhoeven]

On Fri, Dec 6, 2024 at 12:14 PM Claudiu <claudiu.beznea@tuxon.dev> wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> Enable ADC.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-devel for v6.14.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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