| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <cb39cfd69441955a40a0b9cd08b2595771cca5cf.camel@microchip.com>
Date: Wed, 15 Jun 2022 06:36:37 +0000
From: <Arun.Ramadoss@...rochip.com>
To: <olteanv@...il.com>
CC: <andrew@...n.ch>, <linux-kernel@...r.kernel.org>,
<UNGLinuxDriver@...rochip.com>, <vivien.didelot@...il.com>,
<linux@...linux.org.uk>, <f.fainelli@...il.com>, <kuba@...nel.org>,
<edumazet@...gle.com>, <pabeni@...hat.com>,
<netdev@...r.kernel.org>, <Woojung.Huh@...rochip.com>,
<davem@...emloft.net>
Subject: Re: [RFC Patch net-next v2 03/15] net: dsa: microchip: move
tag_protocol & phy read/write to ksz_common
On Mon, 2022-06-13 at 12:22 +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:45PM +0530, Arun Ramadoss wrote:
> > This patch move the dsa hook get_tag_protocol to ksz_common file.
> > And
> > the tag_protocol is returned based on the dev->chip_id.
> > ksz8795 and ksz9477 implementation on phy read/write hooks are
> > different. This patch modifies the ksz9477 implementation same as
> > ksz8795 by updating the ksz9477_dev_ops structure.
> >
> > Signed-off-by: Arun Ramadoss <arun.ramadoss@...rochip.com>
> > ---
>
> It would be easier to review if you would split the
> phy_read/phy_write
> change from the get_tag_protocol change.
In the RFC v1, these two are in separate patch. But patchwork errored
like maximum patch in the series should be 15. So I merged them into
one. Is it ok if the patch size increased beyond 15 or should I split
this patch series into two.
>
> > drivers/net/dsa/microchip/ksz8795.c | 13 +--------
> > drivers/net/dsa/microchip/ksz9477.c | 37 ++++++++------------
> > ------
> > drivers/net/dsa/microchip/ksz_common.c | 24 +++++++++++++++++
> > drivers/net/dsa/microchip/ksz_common.h | 2 ++
> > 4 files changed, 38 insertions(+), 38 deletions(-)
> >
> > diff --git a/drivers/net/dsa/microchip/ksz8795.c
> > b/drivers/net/dsa/microchip/ksz8795.c
> > index 927db57d02db..6e5f665fa1f6 100644
> > --- a/drivers/net/dsa/microchip/ksz8795.c
> > +++ b/drivers/net/dsa/microchip/ksz8795.c
> > @@ -898,17 +898,6 @@ static void ksz8_w_phy(struct ksz_device *dev,
> > u16 phy, u16 reg, u16 val)
> > }
> > }
> >
> > -static enum dsa_tag_protocol ksz8_get_tag_protocol(struct
> > dsa_switch *ds,
> > - int port,
> > - enum
> > dsa_tag_protocol mp)
> > -{
> > - struct ksz_device *dev = ds->priv;
> > -
> > - /* ksz88x3 uses the same tag schema as KSZ9893 */
> > - return ksz_is_ksz88x3(dev) ?
> > - DSA_TAG_PROTO_KSZ9893 : DSA_TAG_PROTO_KSZ8795;
> > -}
> > -
> > static u32 ksz8_sw_get_phy_flags(struct dsa_switch *ds, int port)
> > {
> > /* Silicon Errata Sheet (DS80000830A):
> > @@ -1394,7 +1383,7 @@ static void ksz8_get_caps(struct dsa_switch
> > *ds, int port,
> > }
> >
> > static const struct dsa_switch_ops ksz8_switch_ops = {
> > - .get_tag_protocol = ksz8_get_tag_protocol,
> > + .get_tag_protocol = ksz_get_tag_protocol,
> > .get_phy_flags = ksz8_sw_get_phy_flags,
> > .setup = ksz8_setup,
> > .phy_read = ksz_phy_read16,
> >
> > diff --git a/drivers/net/dsa/microchip/ksz_common.c
> > b/drivers/net/dsa/microchip/ksz_common.c
> > index 9057cdb5971c..a43b01c2e67f 100644
> > --- a/drivers/net/dsa/microchip/ksz_common.c
> > +++ b/drivers/net/dsa/microchip/ksz_common.c
> > @@ -930,6 +930,30 @@ void ksz_port_stp_state_set(struct dsa_switch
> > *ds, int port,
> > }
> EXPORT_SYMBOL_GPL(ksz_port_stp_state_set);
> >
> > +enum dsa_tag_protocol ksz_get_tag_protocol(struct dsa_switch *ds,
> > + int port, enum
> > dsa_tag_protocol mp)
> > +{
> > + struct ksz_device *dev = ds->priv;
> > + enum dsa_tag_protocol proto;
>
> Please choose a default value in case the dev->chip_id does not take
> any
> of the branches, or at least do something to not return uninitialized
> variables from the stack.
Ok. I will update the default value.
> > +
> > + if (dev->chip_id == KSZ8795_CHIP_ID ||
> > + dev->chip_id == KSZ8794_CHIP_ID ||
> > + dev->chip_id == KSZ8765_CHIP_ID)
> > + proto = DSA_TAG_PROTO_KSZ8795;
> > +
> > + if (dev->chip_id == KSZ8830_CHIP_ID ||
> > + dev->chip_id == KSZ9893_CHIP_ID)
> > + proto = DSA_TAG_PROTO_KSZ9893;
> > +
> > + if (dev->chip_id == KSZ9477_CHIP_ID ||
> > + dev->chip_id == KSZ9897_CHIP_ID ||
> > + dev->chip_id == KSZ9567_CHIP_ID)
> > + proto = DSA_TAG_PROTO_KSZ9477;
> > +
> > + return proto;
> > +}
> > +EXPORT_SYMBOL_GPL(ksz_get_tag_protocol);
> > +
> > static int ksz_switch_detect(struct ksz_device *dev)
> > {
> > u8 id1, id2;
> > diff --git a/drivers/net/dsa/microchip/ksz_common.h
> > b/drivers/net/dsa/microchip/ksz_common.h
> > index d16c095cdefb..f253f3f22386 100644
> > --- a/drivers/net/dsa/microchip/ksz_common.h
> > +++ b/drivers/net/dsa/microchip/ksz_common.h
> > @@ -231,6 +231,8 @@ int ksz_port_mdb_del(struct dsa_switch *ds, int
> > port,
> > int ksz_enable_port(struct dsa_switch *ds, int port, struct
> > phy_device *phy);
> > void ksz_get_strings(struct dsa_switch *ds, int port,
> > u32 stringset, uint8_t *buf);
> > +enum dsa_tag_protocol ksz_get_tag_protocol(struct dsa_switch *ds,
> > + int port, enum
> > dsa_tag_protocol mp);
> >
> > /* Common register access functions */
> >
> > --
> > 2.36.1
> >
>
>
Powered by blists - more mailing lists