Hi Biju,
Thanks for your patch!
On Thu, Nov 11, 2021 at 12:54 PM Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> This patch adds support for watchdog reset selection.
Please explain what this patch really does, and why it is needed,
instead of repeating the one-line summary.
> Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> --- a/drivers/clk/renesas/r9a07g044-cpg.c
> +++ b/drivers/clk/renesas/r9a07g044-cpg.c
> @@ -295,7 +296,28 @@ static const unsigned int r9a07g044_crit_mod_clks[] __initconst = {
> MOD_CLK_BASE + R9A07G044_DMAC_ACLK,
> };
>
> +#define CPG_WDTRST_SEL 0xb14
> +#define CPG_WDTRST_SEL_WDTRSTSEL(n) BIT(n)
> +
> +#define CPG_WDTRST_SEL_WDTRST (CPG_WDTRST_SEL_WDTRSTSEL(0) | \
> + CPG_WDTRST_SEL_WDTRSTSEL(1) | \
> + CPG_WDTRST_SEL_WDTRSTSEL(2))
You might as well use BIT() directly. Or GENMASK().
> +
> +int r9a07g044_wdt_rst_setect(void __iomem *base)
> +{
> + writel((CPG_WDTRST_SEL_WDTRST << 16) | CPG_WDTRST_SEL_WDTRST,
> + base + CPG_WDTRST_SEL);
> +
> + return 0;
> +}
> +
> +static const struct rzg2l_cpg_soc_operations r9a07g044_cpg_ops = {
> + .wdt_rst_setect = r9a07g044_wdt_rst_setect,
As you use a function pointer, I assume different SoCs need different
handling, and you can't just store e.g. a bitmask of bits to set in info?
> +};
> +
> const struct rzg2l_cpg_info r9a07g044_cpg_info = {
> + .ops = &r9a07g044_cpg_ops,
> +
> /* Core Clocks */
> .core_clks = r9a07g044_core_clks,
> .num_core_clks = ARRAY_SIZE(r9a07g044_core_clks),
> diff --git a/drivers/clk/renesas/rzg2l-cpg.c b/drivers/clk/renesas/rzg2l-cpg.c
> index a77cb47b75e7..f9dfee14a33e 100644
> --- a/drivers/clk/renesas/rzg2l-cpg.c
> +++ b/drivers/clk/renesas/rzg2l-cpg.c
> @@ -932,6 +932,12 @@ static int __init rzg2l_cpg_probe(struct platform_device *pdev)
> if (error)
> return error;
>
> + if (info->ops && info->ops->wdt_rst_setect) {
> + error = info->ops->wdt_rst_setect(priv->base);
> + if (error)
> + return error;
> + }
> +
> return 0;
> }
>
> diff --git a/drivers/clk/renesas/rzg2l-cpg.h b/drivers/clk/renesas/rzg2l-cpg.h
> index 484c7cee2629..e1b1497002ed 100644
> --- a/drivers/clk/renesas/rzg2l-cpg.h
> +++ b/drivers/clk/renesas/rzg2l-cpg.h
> @@ -156,9 +156,20 @@ struct rzg2l_reset {
> .bit = (_bit) \
> }
>
> +/**
> + * struct rzg2l_cpg_soc_operations - SoC-specific CPG Operations
> + *
> + * @wdt_rst_setect: WDT reset selection
> + */
> +struct rzg2l_cpg_soc_operations {
> + int (*wdt_rst_setect)(void __iomem *base); /* Platform specific WDT reset selection */
Do you plan to add more operations?
> +};
> +
> /**
> * struct rzg2l_cpg_info - SoC-specific CPG Description
> *
> + * @ops: SoC-specific CPG Operations
> + *
> * @core_clks: Array of Core Clock definitions
> * @num_core_clks: Number of entries in core_clks[]
> * @last_dt_core_clk: ID of the last Core Clock exported to DT
> @@ -176,6 +187,9 @@ struct rzg2l_reset {
> * @num_crit_mod_clks: Number of entries in crit_mod_clks[]
> */
> struct rzg2l_cpg_info {
> + /* CPG Operations */
> + const struct rzg2l_cpg_soc_operations *ops;
> +
> /* Core Clocks */
> const struct cpg_core_clk *core_clks;
> unsigned int num_core_clks;
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
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
