Messages in this thread | | | Subject | Re: [PATCH 2/3] reset: mchp: sparx5: add switch reset driver | From | Steen Hegelund <> | Date | Thu, 14 Jan 2021 09:30:50 +0100 |
| |
On Thu, 2021-01-14 at 00:23 +0100, Andrew Lunn wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you > know the content is safe > > > +static int sparx5_switch_reset(struct reset_controller_dev *rcdev, > > + unsigned long id) > > +{ > > + struct mchp_reset_context *ctx = > > + container_of(rcdev, struct mchp_reset_context, > > reset_ctrl); > > + u32 val; > > + > > + /* Make sure the core is PROTECTED from reset */ > > + regmap_update_bits(ctx->cpu_ctrl, PROTECT_REG, PROTECT_BIT, > > PROTECT_BIT); > > + > > + dev_info(ctx->dev, "soft reset of switchcore\n"); > > dev_dbg()?
I will remove that. > > > + > > + /* Start soft reset */ > > + regmap_write(ctx->gcb_ctrl, SOFT_RESET_REG, SOFT_RESET_BIT); > > + > > + /* Wait for soft reset done */ > > + return read_poll_timeout(sparx5_read_soft_rst, val, > > + (val & SOFT_RESET_BIT) == 0, > > + 1, 100, false, > > + ctx); > > +} > > > +static int mchp_sparx5_reset_config(struct platform_device *pdev, > > + struct mchp_reset_context *ctx) > > +{ > > + struct device_node *dn = pdev->dev.of_node; > > + struct regmap *cpu_ctrl, *gcb_ctrl; > > + struct device_node *syscon_np; > > + int err; > > + > > + syscon_np = of_parse_phandle(dn, "syscons", 0); > > + if (!syscon_np) > > + return -ENODEV; > > + cpu_ctrl = syscon_node_to_regmap(syscon_np); > > + if (IS_ERR(cpu_ctrl)) > > + goto err_cpu; > > + of_node_put(syscon_np); > > + > > + syscon_np = of_parse_phandle(dn, "syscons", 1); > > + if (!syscon_np) > > + return -ENODEV; > > + gcb_ctrl = syscon_node_to_regmap(syscon_np); > > + if (IS_ERR(gcb_ctrl)) > > + goto err_gcb; > > + of_node_put(syscon_np); > > + > > + ctx->cpu_ctrl = cpu_ctrl; > > + ctx->gcb_ctrl = gcb_ctrl; > > + > > + ctx->reset_ctrl.owner = THIS_MODULE; > > + ctx->reset_ctrl.nr_resets = 1; > > + ctx->reset_ctrl.ops = &sparx5_reset_ops; > > + ctx->reset_ctrl.of_node = dn; > > + > > + err = devm_reset_controller_register(&pdev->dev, &ctx- > > >reset_ctrl); > > + if (err) > > + dev_err(&pdev->dev, "could not register reset > > controller\n"); > > + pr_info("%s:%d\n", __func__, __LINE__); > > + return err; > > +err_cpu: > > + of_node_put(syscon_np); > > + dev_err(&pdev->dev, "No cpu syscon map\n"); > > + return PTR_ERR(cpu_ctrl); > > +err_gcb: > > + of_node_put(syscon_np); > > + dev_err(&pdev->dev, "No gcb syscon map\n"); > > + return PTR_ERR(gcb_ctrl); > > It would be normal to put the dev_err() before the goto, set err = > PTR_ERR() and then goto out;
OK. I will change that.
> > > > +} > > + > > +static int mchp_sparx5_reset_probe(struct platform_device *pdev) > > +{ > > + struct device *dev = &pdev->dev; > > + struct mchp_reset_context *ctx; > > + > > + pr_info("%s:%d\n", __func__, __LINE__); > > More left over debug.
Yes. That will have to go.
> > > + ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL); > > + if (!ctx) > > + return -ENOMEM; > > + ctx->dev = dev; > > + return mchp_sparx5_reset_config(pdev, ctx); > > +} > > + > > +static const struct of_device_id mchp_sparx5_reset_of_match[] = { > > + { > > + .compatible = "microchip,sparx5-switch-reset", > > + }, > > + { /*sentinel*/ } > > +}; > > > +static int __init mchp_sparx5_reset_init(void) > > +{ > > + return platform_driver_register(&mchp_sparx5_reset_driver); > > +} > > + > > +postcore_initcall(mchp_sparx5_reset_init); > > Does it actually need to be postcore? The users of the reset should > look for -EPROBE_DEFER and try again later. And this then becomes > just > a normal driver.
I tried using that, but the SGPIO driver bailed out after 3 DEFER attempts, so that is why I changed it to use the postcore_initcall. Maybe it is because the SGPIO driver is a builtin_platform_driver?
> > Andrew
| |