Messages in this thread | | | Date | Thu, 10 Jun 2021 12:20:10 +0200 | From | Oleksij Rempel <> | Subject | Re: [PATCH net-next v3 2/9] net: dsa: microchip: ksz8795: add phylink support |
| |
On Thu, May 27, 2021 at 01:13:04AM +0300, Vladimir Oltean wrote: > On Wed, May 26, 2021 at 06:30:30AM +0200, Oleksij Rempel wrote: > > From: Michael Grzeschik <m.grzeschik@pengutronix.de> > > > > This patch adds the phylink support to the ksz8795 driver to provide > > configuration exceptions on quirky KSZ8863 and KSZ8873 ports. > > > > Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de> > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > > --- > > drivers/net/dsa/microchip/ksz8795.c | 59 +++++++++++++++++++++++++++++ > > 1 file changed, 59 insertions(+) > > > > diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c > > index ba065003623f..cf81ae87544d 100644 > > --- a/drivers/net/dsa/microchip/ksz8795.c > > +++ b/drivers/net/dsa/microchip/ksz8795.c > > @@ -18,6 +18,7 @@ > > #include <linux/micrel_phy.h> > > #include <net/dsa.h> > > #include <net/switchdev.h> > > +#include <linux/phylink.h> > > > > #include "ksz_common.h" > > #include "ksz8795_reg.h" > > @@ -1420,11 +1421,69 @@ static int ksz8_setup(struct dsa_switch *ds) > > return 0; > > } > > > > +static void ksz8_validate(struct dsa_switch *ds, int port, > > + unsigned long *supported, > > + struct phylink_link_state *state) > > +{ > > + __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, }; > > + struct ksz_device *dev = ds->priv; > > + > > + if (port == dev->cpu_port) { > > + if (state->interface != PHY_INTERFACE_MODE_RMII && > > + state->interface != PHY_INTERFACE_MODE_MII && > > + state->interface != PHY_INTERFACE_MODE_NA) > > + goto unsupported; > > + } else if (port > dev->port_cnt) { > > + bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS); > > + dev_err(ds->dev, "Unsupported port: %i\n", port); > > + return; > > Is this possible or do we just like to invent things to check? > Unless I'm missing something, ksz8_switch_init() does: > > dev->ds->num_ports = dev->port_cnt; > > and dsa_port_phylink_validate() does: > > ds->ops->phylink_validate(ds, dp->index, supported, state); > > where dp->index is set to @port by dsa_port_touch() in this loop: > > for (port = 0; port < ds->num_ports; port++) { > dp = dsa_port_touch(ds, port); > if (!dp) > return -ENOMEM; > } > > So, if 0 <= dp->index < ds->num_ports == dev->port_cnt, what is the point?
good point
Regards, Oleksij -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
| |