[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <2fabead977bee651800790f6b0d6323ffdc372c5.camel@pengutronix.de>
Date: Mon, 10 Nov 2025 12:34:16 +0100
From: Philipp Zabel <p.zabel@...gutronix.de>
To: Conor Dooley <conor@...nel.org>, claudiu.beznea@...on.dev
Cc: Conor Dooley <conor.dooley@...rochip.com>, Daire McNamara
<daire.mcnamara@...rochip.com>, pierre-henry.moussay@...rochip.com,
valentina.fernandezalanis@...rochip.com, Michael Turquette
<mturquette@...libre.com>, Stephen Boyd <sboyd@...nel.org>, Rob Herring
<robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
linux-riscv@...ts.infradead.org, linux-clk@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v7 1/5] reset: mpfs: add non-auxiliary bus probing
On Mo, 2025-11-10 at 11:23 +0000, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@...rochip.com>
>
> While the auxiliary bus was a nice bandaid, and meant that re-writing
> the representation of the clock regions in devicetree was not required,
> it has run its course. The "mss_top_sysreg" region that contains the
> clock and reset regions, also contains pinctrl and an interrupt
> controller, so the time has come rewrite the devicetree and probe the
> reset controller from an mfd devicetree node, rather than implement
> those drivers using the auxiliary bus. Wanting to avoid propagating this
> naive/incorrect description of the hardware to the new pic64gx SoC is a
> major motivating factor here.
>
> Signed-off-by: Conor Dooley <conor.dooley@...rochip.com>
> ---
> v7:
> - move entirely to regmap
> - use clear/set instead of update
>
> v6:
> - depend on MFD_SYSCON
> - return regmap_update_bits() result directly instead of an additional
> return 0
>
> v4:
> - Only use driver specific lock for non-regmap writes
>
> v2:
> - Implement the request to use regmap_update_bits(). I found that I then
> hated the read/write helpers since they were just bloat, so I ripped
> them out. I replaced the regular spin_lock_irqsave() stuff with a
> guard(spinlock_irqsave), since that's a simpler way of handling the two
> different paths through such a trivial pair of functions.
> ---
> drivers/clk/microchip/clk-mpfs.c | 4 +-
> drivers/reset/Kconfig | 1 +
> drivers/reset/reset-mpfs.c | 92 +++++++++++++++++++-------------
> include/soc/microchip/mpfs.h | 3 +-
> 4 files changed, 61 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/clk/microchip/clk-mpfs.c b/drivers/clk/microchip/clk-mpfs.c
> index 484893e68b67..ee58304913ef 100644
> --- a/drivers/clk/microchip/clk-mpfs.c
> +++ b/drivers/clk/microchip/clk-mpfs.c
> @@ -38,7 +38,7 @@ static const struct regmap_config mpfs_clk_regmap_config = {
> .reg_stride = 4,
> .val_bits = 32,
> .val_format_endian = REGMAP_ENDIAN_LITTLE,
> - .max_register = REG_SUBBLK_CLOCK_CR,
> + .max_register = REG_SUBBLK_RESET_CR,
> };
>
> /*
> @@ -502,7 +502,7 @@ static inline int mpfs_clk_old_format_probe(struct mpfs_clock_data *clk_data,
> if (IS_ERR(clk_data->regmap))
> return PTR_ERR(clk_data->regmap);
>
> - return mpfs_reset_controller_register(dev, clk_data->base + REG_SUBBLK_RESET_CR);
> + return mpfs_reset_controller_register(dev, clk_data->regmap);
> }
>
> static int mpfs_clk_probe(struct platform_device *pdev)
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index 78b7078478d4..0ec4b7cd08d6 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -200,6 +200,7 @@ config RESET_PISTACHIO
> config RESET_POLARFIRE_SOC
> bool "Microchip PolarFire SoC (MPFS) Reset Driver"
> depends on MCHP_CLK_MPFS
> + depends on MFD_SYSCON
> select AUXILIARY_BUS
> default MCHP_CLK_MPFS
> help
> diff --git a/drivers/reset/reset-mpfs.c b/drivers/reset/reset-mpfs.c
> index f6fa10e03ea8..d00212450990 100644
> --- a/drivers/reset/reset-mpfs.c
> +++ b/drivers/reset/reset-mpfs.c
> @@ -7,13 +7,16 @@
> *
> */
> #include <linux/auxiliary_bus.h>
> +#include <linux/cleanup.h>
Not used anymore.
[...]
> @@ -176,12 +196,12 @@ static const struct auxiliary_device_id mpfs_reset_ids[] = {
> };
> MODULE_DEVICE_TABLE(auxiliary, mpfs_reset_ids);
>
> -static struct auxiliary_driver mpfs_reset_driver = {
> - .probe = mpfs_reset_probe,
> +static struct auxiliary_driver mpfs_reset_aux_driver = {
> + .probe = mpfs_reset_adev_probe,
> .id_table = mpfs_reset_ids,
> };
>
> -module_auxiliary_driver(mpfs_reset_driver);
> +module_auxiliary_driver(mpfs_reset_aux_driver);
>
> MODULE_DESCRIPTION("Microchip PolarFire SoC Reset Driver");
> MODULE_AUTHOR("Conor Dooley <conor.dooley@...rochip.com>");
> diff --git a/include/soc/microchip/mpfs.h b/include/soc/microchip/mpfs.h
> index 0bd67e10b704..ec04c98a8b63 100644
> --- a/include/soc/microchip/mpfs.h
> +++ b/include/soc/microchip/mpfs.h
> @@ -14,6 +14,7 @@
>
> #include <linux/types.h>
> #include <linux/of_device.h>
> +#include <linux/regmap.h>
You don't have to #include <linux/regmap.h> here, a forward declaration
struct regmap;
would suffice.
> struct mpfs_sys_controller;
>
> @@ -44,7 +45,7 @@ struct mtd_info *mpfs_sys_controller_get_flash(struct mpfs_sys_controller *mpfs_
>
> #if IS_ENABLED(CONFIG_MCHP_CLK_MPFS)
> #if IS_ENABLED(CONFIG_RESET_POLARFIRE_SOC)
> -int mpfs_reset_controller_register(struct device *clk_dev, void __iomem *base);
> +int mpfs_reset_controller_register(struct device *clk_dev, struct regmap *map);
> #else
> static inline int mpfs_reset_controller_register(struct device *clk_dev, void __iomem *base) { return 0; }
> #endif /* if IS_ENABLED(CONFIG_RESET_POLARFIRE_SOC) */
With the superfluous cleanup include fixed.
Reviewed-by: Philipp Zabel <p.zabel@...gutronix.de>
and
Acked-by: Philipp Zabel <p.zabel@...gutronix.de>
to be merged with the reset of the series.
regards
Philipp
Powered by blists - more mailing lists