| Message ID | 20250225105329.3037853-1-horatiu.vultur@microchip.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] reset: mchp: sparx5: Fix for lan966x | expand |
Hi Horatiu, On Tue, 25 Feb 2025 11:53:29 +0100 Horatiu Vultur <horatiu.vultur@microchip.com> wrote: > With the blamed commit it seems that lan966x doesn't seem to boot > anymore when the internal CPU is used. > The reason seems to be the usage of the devm_of_iomap, if we replace > this with of_iomap, this seems to fix the issue as we use the same > region also for other devices. > > Fixes: 0426a920d6269c ("reset: mchp: sparx5: Map cpu-syscon locally in case of LAN966x") > Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com> > --- > v1->v2: > - make sure to use iounmap when driver is removed > --- > drivers/reset/reset-microchip-sparx5.c | 40 +++++++++++++++++++------- > 1 file changed, 29 insertions(+), 11 deletions(-) > ... > static struct regmap *mchp_lan966x_syscon_to_regmap(struct device *dev, > - struct device_node *syscon_np) > + struct device_node *syscon_np, > + struct mchp_reset_context *ctx) > { > struct regmap_config regmap_config = mchp_lan966x_syscon_regmap_config; > - resource_size_t size; > - void __iomem *base; > + struct resource res; > > - base = devm_of_iomap(dev, syscon_np, 0, &size); > - if (IS_ERR(base)) > - return ERR_CAST(base); > + if (of_address_to_resource(syscon_np, 0, &res)) > + return ERR_PTR(-ENOMEM); Why not forwarding the error returned by of_address_to_resource() ? Other than that: Reviewed-by: Herve Codina <herve.codina@bootlin.com> Also, I tested the patch successfully on my LAN966x PCI device. Tested-by: Herve Codina <herve.codina@bootlin.com> Best regards, Hervé
On Di, 2025-02-25 at 11:53 +0100, Horatiu Vultur wrote: > With the blamed commit it seems that lan966x doesn't seem to boot > anymore when the internal CPU is used. > The reason seems to be the usage of the devm_of_iomap, if we replace > this with of_iomap, this seems to fix the issue as we use the same > region also for other devices. So the issue is the devm_request_mem_region() called by devm_ioremap_resource()? Wouldn't it be easier to switch to devm_ioremap() instead? regards Philipp
The 02/26/2025 11:27, Philipp Zabel wrote: > > On Di, 2025-02-25 at 11:53 +0100, Horatiu Vultur wrote: > > With the blamed commit it seems that lan966x doesn't seem to boot > > anymore when the internal CPU is used. > > The reason seems to be the usage of the devm_of_iomap, if we replace > > this with of_iomap, this seems to fix the issue as we use the same > > region also for other devices. > > So the issue is the devm_request_mem_region() called by > devm_ioremap_resource()? That is correct. > > Wouldn't it be easier to switch to devm_ioremap() instead? That would be easier and the change set will be also smaller. > > regards > Philipp
diff --git a/drivers/reset/reset-microchip-sparx5.c b/drivers/reset/reset-microchip-sparx5.c index aa5464be7053b..cbe68026adc8d 100644 --- a/drivers/reset/reset-microchip-sparx5.c +++ b/drivers/reset/reset-microchip-sparx5.c @@ -8,6 +8,7 @@ */ #include <linux/mfd/syscon.h> #include <linux/of.h> +#include <linux/of_address.h> #include <linux/module.h> #include <linux/platform_device.h> #include <linux/property.h> @@ -26,6 +27,7 @@ struct mchp_reset_context { struct regmap *gcb_ctrl; struct reset_controller_dev rcdev; const struct reset_props *props; + void __iomem *base; }; static struct regmap_config sparx5_reset_regmap_config = { @@ -69,23 +71,27 @@ static const struct regmap_config mchp_lan966x_syscon_regmap_config = { }; static struct regmap *mchp_lan966x_syscon_to_regmap(struct device *dev, - struct device_node *syscon_np) + struct device_node *syscon_np, + struct mchp_reset_context *ctx) { struct regmap_config regmap_config = mchp_lan966x_syscon_regmap_config; - resource_size_t size; - void __iomem *base; + struct resource res; - base = devm_of_iomap(dev, syscon_np, 0, &size); - if (IS_ERR(base)) - return ERR_CAST(base); + if (of_address_to_resource(syscon_np, 0, &res)) + return ERR_PTR(-ENOMEM); - regmap_config.max_register = size - 4; + ctx->base = of_iomap(syscon_np, 0); + if (!ctx->base) + return ERR_PTR(-ENOMEM); - return devm_regmap_init_mmio(dev, base, ®map_config); + regmap_config.max_register = resource_size(&res) - 4; + + return devm_regmap_init_mmio(dev, ctx->base, ®map_config); } static int mchp_sparx5_map_syscon(struct platform_device *pdev, char *name, - struct regmap **target) + struct regmap **target, + struct mchp_reset_context *ctx) { struct device_node *syscon_np; struct regmap *regmap; @@ -103,7 +109,8 @@ static int mchp_sparx5_map_syscon(struct platform_device *pdev, char *name, * device removal. */ if (of_device_is_compatible(pdev->dev.of_node, "microchip,lan966x-switch-reset")) - regmap = mchp_lan966x_syscon_to_regmap(&pdev->dev, syscon_np); + regmap = mchp_lan966x_syscon_to_regmap(&pdev->dev, syscon_np, + ctx); else regmap = syscon_node_to_regmap(syscon_np); of_node_put(syscon_np); @@ -146,7 +153,7 @@ static int mchp_sparx5_reset_probe(struct platform_device *pdev) if (!ctx) return -ENOMEM; - err = mchp_sparx5_map_syscon(pdev, "cpu-syscon", &ctx->cpu_ctrl); + err = mchp_sparx5_map_syscon(pdev, "cpu-syscon", &ctx->cpu_ctrl, ctx); if (err) return err; err = mchp_sparx5_map_io(pdev, 0, &ctx->gcb_ctrl); @@ -165,9 +172,19 @@ static int mchp_sparx5_reset_probe(struct platform_device *pdev) if (err) return err; + dev_set_drvdata(&pdev->dev, ctx); + return devm_reset_controller_register(&pdev->dev, &ctx->rcdev); } +static void mchp_sparx5_reset_remove(struct platform_device *pdev) +{ + struct mchp_reset_context *ctx = dev_get_drvdata(&pdev->dev); + + if (ctx->base != NULL) + iounmap(ctx->base); +} + static const struct reset_props reset_props_sparx5 = { .protect_reg = 0x84, .protect_bit = BIT(10), @@ -196,6 +213,7 @@ MODULE_DEVICE_TABLE(of, mchp_sparx5_reset_of_match); static struct platform_driver mchp_sparx5_reset_driver = { .probe = mchp_sparx5_reset_probe, + .remove = mchp_sparx5_reset_remove, .driver = { .name = "sparx5-switch-reset", .of_match_table = mchp_sparx5_reset_of_match,
With the blamed commit it seems that lan966x doesn't seem to boot anymore when the internal CPU is used. The reason seems to be the usage of the devm_of_iomap, if we replace this with of_iomap, this seems to fix the issue as we use the same region also for other devices. Fixes: 0426a920d6269c ("reset: mchp: sparx5: Map cpu-syscon locally in case of LAN966x") Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com> --- v1->v2: - make sure to use iounmap when driver is removed --- drivers/reset/reset-microchip-sparx5.c | 40 +++++++++++++++++++------- 1 file changed, 29 insertions(+), 11 deletions(-)