Re: [RFC Patch net-next v2 05/15] net: dsa: microchip: move the port mirror to ksz_common

Arun.Ramadoss
2022-06-15T06:40:46Z

On Mon, 2022-06-13 at 12:28 +0300, Vladimir Oltean wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
>
> On Mon, May 30, 2022 at 04:12:47PM +0530, Arun Ramadoss wrote:
> > This patch updates the common port mirror add/del dsa_switch_ops in
> > ksz_common.c. The individual switches implementation is executed
> > based
> > on the ksz_dev_ops function pointers.
> >
> > Signed-off-by: Arun Ramadoss <arun.ramadoss@microchip.com>
> > ---
>
> Reviewed-by: Vladimir Oltean <olteanv@gmail.com>
>
> > drivers/net/dsa/microchip/ksz8795.c | 13 ++++++-------
> > drivers/net/dsa/microchip/ksz9477.c | 12 ++++++------
> > drivers/net/dsa/microchip/ksz_common.c | 25
> > +++++++++++++++++++++++++
> > drivers/net/dsa/microchip/ksz_common.h | 10 ++++++++++
> > 4 files changed, 47 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/net/dsa/microchip/ksz8795.c
> > b/drivers/net/dsa/microchip/ksz8795.c
> > index 157d69e46793..8657b520b336 100644
> > --- a/drivers/net/dsa/microchip/ksz8795.c
> > +++ b/drivers/net/dsa/microchip/ksz8795.c
> > @@ -1089,12 +1089,10 @@ static int ksz8_port_vlan_del(struct
> > ksz_device *dev, int port,
> > return 0;
> > }
> >
> > diff --git a/drivers/net/dsa/microchip/ksz_common.c
> > b/drivers/net/dsa/microchip/ksz_common.c
> > index a1fef9e4e36c..1ed4cc94795e 100644
> > --- a/drivers/net/dsa/microchip/ksz_common.c
> > +++ b/drivers/net/dsa/microchip/ksz_common.c
> > @@ -994,6 +994,31 @@ int ksz_port_vlan_del(struct dsa_switch *ds,
> > int port,
> > }
> > EXPORT_SYMBOL_GPL(ksz_port_vlan_del);
> >
> > +int ksz_port_mirror_add(struct dsa_switch *ds, int port,
> > + struct dsa_mall_mirror_tc_entry *mirror,
> > + bool ingress, struct netlink_ext_ack *extack)
> > +{
> > + struct ksz_device *dev = ds->priv;
> > + int ret = -EOPNOTSUPP;
> > +
> > + if (dev->dev_ops->mirror_add)
> > + ret = dev->dev_ops->mirror_add(dev, port, mirror,
> > ingress,
> > + extack);
> > +
> > + return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(ksz_port_mirror_add);
>
> Just as a minor style comment, take it or leave it.
>
> If you switch the function pointer presence check, you reduce the
> indentation of the long statement, making it fit a single line, and
> you
> eliminate the need for a "ret" variable:
>
> if (!dev->dev_ops->mirror_add)
> return -EOPNOTSUPP;
>
> return dev->dev_ops->mirror_add(dev, port, mirror, ingress,
> extack);

Yes it reduces the indentation. I will update it for other checks too.

> > +
> > +void ksz_port_mirror_del(struct dsa_switch *ds, int port,
> > + struct dsa_mall_mirror_tc_entry *mirror)
> > +{
> > + struct ksz_device *dev = ds->priv;
> > +
> > + if (dev->dev_ops->mirror_del)
> > + dev->dev_ops->mirror_del(dev, port, mirror);
> > +}
> > +EXPORT_SYMBOL_GPL(ksz_port_mirror_del);
> > +
> > static int ksz_switch_detect(struct ksz_device *dev)
> > {
> > u8 id1, id2;
> >
> >
> > --
> > 2.36.1
> >
>
>