Hi Clément, On Thu, May 5, 2022 at 2:33 PM Clément Léger <clement.leger@xxxxxxxxxxx> wrote: > Le Thu, 5 May 2022 09:16:38 +0200, > Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> a écrit : > > On Wed, May 4, 2022 at 11:31 AM Clément Léger <clement.leger@xxxxxxxxxxx> wrote: > > > Add a PCS driver for the MII converter that is present on the Renesas > > > RZ/N1 SoC. This MII converter is reponsible for converting MII to > > > RMII/RGMII or act as a MII pass-trough. Exposing it as a PCS allows to > > > reuse it in both the switch driver and the stmmac driver. Currently, > > > this driver only allows the PCS to be used by the dual Cortex-A7 > > > subsystem since the register locking system is not used. > > > > > > Signed-off-by: Clément Léger <clement.leger@xxxxxxxxxxx> > > > > Thanks for your patch! > > > > > --- /dev/null > > > +++ b/drivers/net/pcs/pcs-rzn1-miic.c > > > > > +static int miic_probe(struct platform_device *pdev) > > > +{ > > > + struct device *dev = &pdev->dev; > > > + struct miic *miic; > > > + u32 mode_cfg; > > > + int ret; > > > + > > > + ret = miic_parse_dt(dev, &mode_cfg); > > > + if (ret < 0) > > > + return -EINVAL; > > > + > > > + miic = devm_kzalloc(dev, sizeof(*miic), GFP_KERNEL); > > > + if (!miic) > > > + return -ENOMEM; > > > + > > > + spin_lock_init(&miic->lock); > > > + miic->dev = dev; > > > + miic->base = devm_platform_ioremap_resource(pdev, 0); > > > + if (!miic->base) > > > + return -EINVAL; > > > + > > > + pm_runtime_enable(dev); > > > + ret = pm_runtime_resume_and_get(dev); > > > + if (ret < 0) > > > > Missing pm_runtime_disable(dev). > > Maybe using devm_pm_runtime_enable() would be sufficient and avoid such > calls. That's an option. Note that that still won't allow you to get rid of the .remove() callback, as you still have to call pm_runtime_put() manually. > > > > > + return ret; > > > + > > > + ret = miic_init_hw(miic, mode_cfg); > > > + if (ret) > > > + goto disable_runtime_pm; > > > + > > > + /* miic_create() relies on that fact that data are attached to the > > > + * platform device to determine if the driver is ready so this needs to > > > + * be the last thing to be done after everything is initialized > > > + * properly. > > > + */ > > > + platform_set_drvdata(pdev, miic); > > > + > > > + return 0; > > > + > > > +disable_runtime_pm: > > > + pm_runtime_put(dev); > > > > Missing pm_runtime_disable(dev). > > > > > + > > > + return ret; > > > +} > > > + > > > +static int miic_remove(struct platform_device *pdev) > > > +{ > > > + pm_runtime_put(&pdev->dev); > > > > Missing pm_runtime_disable(dev). Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds