| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <60544429-3eeb-41df-b42c-613da651b4a1@tuxon.dev>
Date: Thu, 23 Oct 2025 07:06:48 +0300
From: Claudiu Beznea <claudiu.beznea@...on.dev>
To: Conor Dooley <conor@...nel.org>
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>,
Philipp Zabel <p.zabel@...gutronix.de>, linux-riscv@...ts.infradead.org,
linux-clk@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 3/9] reset: mpfs: add non-auxiliary bus probing
Hi, Conor,
On 10/13/25 20:45, 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>
> ---
> 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/reset/reset-mpfs.c | 83 ++++++++++++++++++++++++++++++--------
> 1 file changed, 66 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/reset/reset-mpfs.c b/drivers/reset/reset-mpfs.c
> index f6fa10e03ea8..8e5ed4deecf3 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>
> #include <linux/delay.h>
> #include <linux/io.h>
> +#include <linux/mfd/syscon.h>
Should you add a depends on MFD_SYSCON ?
> #include <linux/module.h>
> #include <linux/of.h>
> #include <linux/platform_device.h>
> -#include <linux/slab.h>
> +#include <linux/regmap.h>
> #include <linux/reset-controller.h>
> +#include <linux/slab.h>
> #include <dt-bindings/clock/microchip,mpfs-clock.h>
> #include <soc/microchip/mpfs.h>
>
> @@ -27,11 +30,14 @@
> #define MPFS_SLEEP_MIN_US 100
> #define MPFS_SLEEP_MAX_US 200
>
> +#define REG_SUBBLK_RESET_CR 0x88u
> +
> /* block concurrent access to the soft reset register */
> static DEFINE_SPINLOCK(mpfs_reset_lock);
>
> struct mpfs_reset {
> void __iomem *base;
> + struct regmap *regmap;
> struct reset_controller_dev rcdev;
> };
>
> @@ -46,41 +52,50 @@ static inline struct mpfs_reset *to_mpfs_reset(struct reset_controller_dev *rcde
> static int mpfs_assert(struct reset_controller_dev *rcdev, unsigned long id)
> {
> struct mpfs_reset *rst = to_mpfs_reset(rcdev);
> - unsigned long flags;
> u32 reg;
>
> - spin_lock_irqsave(&mpfs_reset_lock, flags);
> + if (rst->regmap) {
> + regmap_update_bits(rst->regmap, REG_SUBBLK_RESET_CR, BIT(id), BIT(id));
> + return 0;
You can:
return regmap_update_bits();
> + }
> +
> + guard(spinlock_irqsave)(&mpfs_reset_lock);
>
> reg = readl(rst->base);
> reg |= BIT(id);
> writel(reg, rst->base);
>
> - spin_unlock_irqrestore(&mpfs_reset_lock, flags);
> -
> return 0;
> }
>
> static int mpfs_deassert(struct reset_controller_dev *rcdev, unsigned long id)
> {
> struct mpfs_reset *rst = to_mpfs_reset(rcdev);
> - unsigned long flags;
> u32 reg;
>
> - spin_lock_irqsave(&mpfs_reset_lock, flags);
> + if (rst->regmap) {
> + regmap_update_bits(rst->regmap, REG_SUBBLK_RESET_CR, BIT(id), 0);
> + return 0;
> + }
> +
> + guard(spinlock_irqsave)(&mpfs_reset_lock);
>
> reg = readl(rst->base);
> reg &= ~BIT(id);
> writel(reg, rst->base);
>
> - spin_unlock_irqrestore(&mpfs_reset_lock, flags);
> -
> return 0;
> }
>
> static int mpfs_status(struct reset_controller_dev *rcdev, unsigned long id)
> {
> struct mpfs_reset *rst = to_mpfs_reset(rcdev);
> - u32 reg = readl(rst->base);
> + u32 reg;
> +
> + if (rst->regmap)
> + regmap_read(rst->regmap, REG_SUBBLK_RESET_CR, ®);
> + else
> + reg = readl(rst->base);
>
> /*
> * It is safe to return here as MPFS_NUM_RESETS makes sure the sign bit
> @@ -130,11 +145,45 @@ static int mpfs_reset_xlate(struct reset_controller_dev *rcdev,
> return index - MPFS_PERIPH_OFFSET;
> }
>
> -static int mpfs_reset_probe(struct auxiliary_device *adev,
> - const struct auxiliary_device_id *id)
> +static int mpfs_reset_mfd_probe(struct platform_device *pdev)
> {
> - struct device *dev = &adev->dev;
> struct reset_controller_dev *rcdev;
> + struct device *dev = &pdev->dev;
> + struct mpfs_reset *rst;
> +
> + rst = devm_kzalloc(dev, sizeof(*rst), GFP_KERNEL);
> + if (!rst)
> + return -ENOMEM;
> +
> + rcdev = &rst->rcdev;
> + rcdev->dev = dev;
> + rcdev->ops = &mpfs_reset_ops;
> +
> + rcdev->of_node = pdev->dev.parent->of_node;
> + rcdev->of_reset_n_cells = 1;
> + rcdev->of_xlate = mpfs_reset_xlate;
> + rcdev->nr_resets = MPFS_NUM_RESETS;
> +
> + rst->regmap = device_node_to_regmap(pdev->dev.parent->of_node);
> + if (IS_ERR(rst->regmap))
> + dev_err_probe(dev, PTR_ERR(rst->regmap), "Failed to find syscon regmap\n");
> +
> + return devm_reset_controller_register(dev, rcdev);
> +}
> +
> +static struct platform_driver mpfs_reset_mfd_driver = {
> + .probe = mpfs_reset_mfd_probe,
> + .driver = {
> + .name = "mpfs-reset",
> + },
> +};
> +module_platform_driver(mpfs_reset_mfd_driver);
> +
> +static int mpfs_reset_adev_probe(struct auxiliary_device *adev,
> + const struct auxiliary_device_id *id)
> +{
> + struct reset_controller_dev *rcdev;
> + struct device *dev = &adev->dev;
> struct mpfs_reset *rst;
>
> rst = devm_kzalloc(dev, sizeof(*rst), GFP_KERNEL);
> @@ -145,8 +194,8 @@ static int mpfs_reset_probe(struct auxiliary_device *adev,
>
> rcdev = &rst->rcdev;
> rcdev->dev = dev;
> - rcdev->dev->parent = dev->parent;
> rcdev->ops = &mpfs_reset_ops;
> +
> rcdev->of_node = dev->parent->of_node;
> rcdev->of_reset_n_cells = 1;
> rcdev->of_xlate = mpfs_reset_xlate;
> @@ -176,12 +225,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>");
Powered by blists - more mailing lists