Le Thu, 19 May 2022 17:25:10 +0100, "Russell King (Oracle)" <linux@xxxxxxxxxxxxxxx> a écrit : > Hi, > > On Thu, May 19, 2022 at 05:30:59PM +0200, Clément Léger 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> > > Looks much better now, thanks. Only one thing I've spotted is: > > > +static int miic_validate(struct phylink_pcs *pcs, unsigned long *supported, > > + const struct phylink_link_state *state) > > +{ > > + if (state->interface == PHY_INTERFACE_MODE_RGMII || > > + state->interface == PHY_INTERFACE_MODE_RGMII_ID || > > + state->interface == PHY_INTERFACE_MODE_RGMII_TXID || > > + state->interface == PHY_INTERFACE_MODE_RGMII_RXID || > > The above could use: > > if (phy_interface_mode_is_rgmii(state->interface) || Thanks, I did found the one to set the bit for phylink part but not this one. > > Also, as a request to unbind this driver would be disasterous to users, > I think you should set ".suppress_bind_attrs = true" to prevent the > sysfs bind/unbind facility being available. This doesn't completely > solve the problem. Acked. What should I do to make it more robust ? Should I use a refcount per pdev and check that in the remove() callback to avoid removing the pdev if used ? Thanks, -- Clément Léger, Embedded Linux and Kernel engineer at Bootlin https://bootlin.com