[PATCH] mux: mmio: Do not use syscon helper to build regmap

[PATCH] mux: mmio: Do not use syscon helper to build regmap

[Andrew Davis]

The syscon helper device_node_to_regmap() is used to fetch a regmap
registered to a device node. It also currently creates this regmap
if the node did not already have a regmap associated with it. This
should only be used on "syscon" nodes. This driver is not such a
device and instead uses device_node_to_regmap() on its own node as
a hacky way to create a regmap for itself.

This will not work going forward and so we should create our regmap
the normal way by defining our regmap_config, fetching our memory
resource, then using the normal regmap_init_mmio() function.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 drivers/mux/mmio.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/mux/mmio.c b/drivers/mux/mmio.c
index 30a952c34365f..278efdc480777 100644
--- a/drivers/mux/mmio.c
+++ b/drivers/mux/mmio.c
@@ -33,6 +33,12 @@ static const struct of_device_id mux_mmio_dt_ids[] = {
 };
 MODULE_DEVICE_TABLE(of, mux_mmio_dt_ids);
 
+static const struct regmap_config mux_mmio_regmap_cfg = {
+	.reg_bits = 32,
+	.val_bits = 32,
+	.reg_stride = 4,
+};
+
 static int mux_mmio_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -40,6 +46,7 @@ static int mux_mmio_probe(struct platform_device *pdev)
 	struct regmap_field **fields;
 	struct mux_chip *mux_chip;
 	struct regmap *regmap;
+	void __iomem *base;
 	int num_fields;
 	int ret;
 	int i;
@@ -47,7 +54,11 @@ static int mux_mmio_probe(struct platform_device *pdev)
 	if (of_device_is_compatible(np, "mmio-mux")) {
 		regmap = syscon_node_to_regmap(np->parent);
 	} else {
-		regmap = device_node_to_regmap(np);
+		base = devm_platform_ioremap_resource(pdev, 0);
+		if (IS_ERR(base))
+			regmap = ERR_PTR(-ENODEV);
+		else
+			regmap = regmap_init_mmio(dev, base, &mux_mmio_regmap_cfg);
 		/* Fallback to checking the parent node on "real" errors. */
 		if (IS_ERR(regmap) && regmap != ERR_PTR(-EPROBE_DEFER)) {
 			regmap = dev_get_regmap(dev->parent, NULL);
-- 
2.39.2

Re: [PATCH] mux: mmio: Do not use syscon helper to build regmap

[Nishanth Menon]

On 12:20-20250123, Andrew Davis wrote:
> The syscon helper device_node_to_regmap() is used to fetch a regmap
> registered to a device node. It also currently creates this regmap
> if the node did not already have a regmap associated with it. This
> should only be used on "syscon" nodes. This driver is not such a
> device and instead uses device_node_to_regmap() on its own node as
> a hacky way to create a regmap for itself.
> 
> This will not work going forward and so we should create our regmap
> the normal way by defining our regmap_config, fetching our memory
> resource, then using the normal regmap_init_mmio() function.
> 
> Signed-off-by: Andrew Davis <afd@ti.com>
> ---
Tested-by: Nishanth Menon <nm@ti.com>

Greg, Peter,

This is part of the fixes TI K3 platforms boot issues reported in
https://lore.kernel.org/all/b2413460-ec8b-4c77-99b8-4c32b262439a@ti.com/
on the latest linus master v6.13-5001-gd0d106a2bd21 + linux
next-20250123

Total set of patches tested with:
https://lore.kernel.org/all/20250119182121.3956546-1-vaishnav.a@ti.com/
https://lore.kernel.org/r/20250123181726.597144-1-afd@ti.com
https://lore.kernel.org/r/20250123181913.597304-1-afd@ti.com
https://lore.kernel.org/r/20250123182059.597491-1-afd@ti.com
https://lore.kernel.org/r/20250123182234.597665-1-afd@ti.com

Could we get this routed to master as fixes asap please to get a sane 6.14?

Cc Rob and Lee

>  drivers/mux/mmio.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mux/mmio.c b/drivers/mux/mmio.c
> index 30a952c34365f..278efdc480777 100644
> --- a/drivers/mux/mmio.c
> +++ b/drivers/mux/mmio.c
> @@ -33,6 +33,12 @@ static const struct of_device_id mux_mmio_dt_ids[] = {
>  };
>  MODULE_DEVICE_TABLE(of, mux_mmio_dt_ids);
>  
> +static const struct regmap_config mux_mmio_regmap_cfg = {
> +	.reg_bits = 32,
> +	.val_bits = 32,
> +	.reg_stride = 4,
> +};
> +
>  static int mux_mmio_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -40,6 +46,7 @@ static int mux_mmio_probe(struct platform_device *pdev)
>  	struct regmap_field **fields;
>  	struct mux_chip *mux_chip;
>  	struct regmap *regmap;
> +	void __iomem *base;
>  	int num_fields;
>  	int ret;
>  	int i;
> @@ -47,7 +54,11 @@ static int mux_mmio_probe(struct platform_device *pdev)
>  	if (of_device_is_compatible(np, "mmio-mux")) {
>  		regmap = syscon_node_to_regmap(np->parent);
>  	} else {
> -		regmap = device_node_to_regmap(np);
> +		base = devm_platform_ioremap_resource(pdev, 0);
> +		if (IS_ERR(base))
> +			regmap = ERR_PTR(-ENODEV);
> +		else
> +			regmap = regmap_init_mmio(dev, base, &mux_mmio_regmap_cfg);
>  		/* Fallback to checking the parent node on "real" errors. */
>  		if (IS_ERR(regmap) && regmap != ERR_PTR(-EPROBE_DEFER)) {
>  			regmap = dev_get_regmap(dev->parent, NULL);
> -- 
> 2.39.2
> 

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

Re: [PATCH] mux: mmio: Do not use syscon helper to build regmap

[Greg Kroah-Hartman]

On Thu, Jan 23, 2025 at 02:04:12PM -0600, Nishanth Menon wrote:
> On 12:20-20250123, Andrew Davis wrote:
> > The syscon helper device_node_to_regmap() is used to fetch a regmap
> > registered to a device node. It also currently creates this regmap
> > if the node did not already have a regmap associated with it. This
> > should only be used on "syscon" nodes. This driver is not such a
> > device and instead uses device_node_to_regmap() on its own node as
> > a hacky way to create a regmap for itself.
> > 
> > This will not work going forward and so we should create our regmap
> > the normal way by defining our regmap_config, fetching our memory
> > resource, then using the normal regmap_init_mmio() function.
> > 
> > Signed-off-by: Andrew Davis <afd@ti.com>
> > ---
> Tested-by: Nishanth Menon <nm@ti.com>
> 
> Greg, Peter,
> 
> This is part of the fixes TI K3 platforms boot issues reported in
> https://lore.kernel.org/all/b2413460-ec8b-4c77-99b8-4c32b262439a@ti.com/
> on the latest linus master v6.13-5001-gd0d106a2bd21 + linux
> next-20250123
> 
> Total set of patches tested with:
> https://lore.kernel.org/all/20250119182121.3956546-1-vaishnav.a@ti.com/
> https://lore.kernel.org/r/20250123181726.597144-1-afd@ti.com
> https://lore.kernel.org/r/20250123181913.597304-1-afd@ti.com
> https://lore.kernel.org/r/20250123182059.597491-1-afd@ti.com
> https://lore.kernel.org/r/20250123182234.597665-1-afd@ti.com
> 
> Could we get this routed to master as fixes asap please to get a sane 6.14?

Our trees are of course closed right now due to the merge window, you
know that :)

So we can take things after -rc1 is out, and route bugfixes for
6.14-final and new stuff for 6.15-rc1, like always.  It's hard to see
that this is just a bugfix, but if it is, it needs a Fixes: tag at the
very least, right?

thanks,

greg k-h

Re: [PATCH] mux: mmio: Do not use syscon helper to build regmap

[Nishanth Menon]

On 06:26-20250124, Greg Kroah-Hartman wrote:
[...]
> > Greg, Peter,
> > 
> > This is part of the fixes TI K3 platforms boot issues reported in
> > https://lore.kernel.org/all/b2413460-ec8b-4c77-99b8-4c32b262439a@ti.com/
> > on the latest linus master v6.13-5001-gd0d106a2bd21 + linux
> > next-20250123
> > 
> > Total set of patches tested with:
> > https://lore.kernel.org/all/20250119182121.3956546-1-vaishnav.a@ti.com/
> > https://lore.kernel.org/r/20250123181726.597144-1-afd@ti.com
> > https://lore.kernel.org/r/20250123181913.597304-1-afd@ti.com
> > https://lore.kernel.org/r/20250123182059.597491-1-afd@ti.com
> > https://lore.kernel.org/r/20250123182234.597665-1-afd@ti.com
> > 
> > Could we get this routed to master as fixes asap please to get a sane 6.14?
> 
> Our trees are of course closed right now due to the merge window, you
> know that :)

Yes, of course.

> 
> So we can take things after -rc1 is out, and route bugfixes for
> 6.14-final and new stuff for 6.15-rc1, like always.  It's hard to see
> that this is just a bugfix, but if it is, it needs a Fixes: tag at the
> very least, right?

I am not entirely sure which commit to use for fixes.

On one hand, we can use
https://lore.kernel.org/all/b2413460-ec8b-4c77-99b8-4c32b262439a@ti.com/
as the cause of the Fixes. Since, the change in behavior of the subsystem late
in the merge cycle caused this side effect, but, we could argue that
this patch fixes the subsystem for it's intended behavior and
unfortunately exposed issues in users of the syscon subsystem depending
on the "wrong" behavior.

OR

The code introduced long back which depended on the older syscon
behavior. but, then, we can argue that it was working fine at that
point.. and if we cherry-pick the fixup for stable, it is probably
inaccurate for the syscon behavior in older kernels?

Lee, Rob: Could you chime in with your suggestion?

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

Re: [PATCH] mux: mmio: Do not use syscon helper to build regmap

[Nishanth Menon]

On 06:26-20250124, Greg Kroah-Hartman wrote:
> On Thu, Jan 23, 2025 at 02:04:12PM -0600, Nishanth Menon wrote:
> > On 12:20-20250123, Andrew Davis wrote:
> > > The syscon helper device_node_to_regmap() is used to fetch a regmap
> > > registered to a device node. It also currently creates this regmap
> > > if the node did not already have a regmap associated with it. This
> > > should only be used on "syscon" nodes. This driver is not such a
> > > device and instead uses device_node_to_regmap() on its own node as
> > > a hacky way to create a regmap for itself.
> > > 
> > > This will not work going forward and so we should create our regmap
> > > the normal way by defining our regmap_config, fetching our memory
> > > resource, then using the normal regmap_init_mmio() function.
> > > 
> > > Signed-off-by: Andrew Davis <afd@ti.com>
> > > ---
> > Tested-by: Nishanth Menon <nm@ti.com>
> > 
> > Greg, Peter,
> > 
> > This is part of the fixes TI K3 platforms boot issues reported in
> > https://lore.kernel.org/all/b2413460-ec8b-4c77-99b8-4c32b262439a@ti.com/
> > on the latest linus master v6.13-5001-gd0d106a2bd21 + linux
> > next-20250123
> > 
> > Total set of patches tested with:
> > https://lore.kernel.org/all/20250119182121.3956546-1-vaishnav.a@ti.com/
> > https://lore.kernel.org/r/20250123181726.597144-1-afd@ti.com
> > https://lore.kernel.org/r/20250123181913.597304-1-afd@ti.com
> > https://lore.kernel.org/r/20250123182059.597491-1-afd@ti.com
> > https://lore.kernel.org/r/20250123182234.597665-1-afd@ti.com
> > 
> > Could we get this routed to master as fixes asap please to get a sane 6.14?
> 
> Our trees are of course closed right now due to the merge window, you
> know that :)
> 
> So we can take things after -rc1 is out, and route bugfixes for
> 6.14-final and new stuff for 6.15-rc1, like always.  It's hard to see
> that this is just a bugfix, but if it is, it needs a Fixes: tag at the
> very least, right?

Just following up after a bit of discussion on IRC.. (Thanks Rob)

I think getting this patch in current window is less urgent for given
Rob has posted a restoration of behavior[1]. This patch is still
good to be merged based on [2], but probably queued as appropriate.

[1] https://lore.kernel.org/all/20250124191644.2309790-1-robh@kernel.org/
[2] https://lore.kernel.org/all/CAL_JsqKVydDqmr-gyn1wt=1PO_p1NMze+1D+4qXWxGiGVLV9EA@mail.gmail.com/

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

Re: [PATCH] mux: mmio: Do not use syscon helper to build regmap

[Krzysztof Kozlowski]

On Thu, 23 Jan 2025 12:20:59 -0600, Andrew Davis wrote:
> The syscon helper device_node_to_regmap() is used to fetch a regmap
> registered to a device node. It also currently creates this regmap
> if the node did not already have a regmap associated with it. This
> should only be used on "syscon" nodes. This driver is not such a
> device and instead uses device_node_to_regmap() on its own node as
> a hacky way to create a regmap for itself.
> 
> [...]

Applied, thanks!

[1/1] mux: mmio: Do not use syscon helper to build regmap
      https://git.kernel.org/krzk/linux/c/61de83fd8256e185588670d3cf0bccc3e913819c

Best regards,
-- 
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>