| 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
| ||
|
Date: Tue, 14 Jun 2022 07:10:33 +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 02/15] net: dsa: microchip: move switch
chip_id detection to ksz_common
Hi Vladimir,
Thanks for the comment.
On Mon, 2022-06-13 at 12:18 +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:44PM +0530, Arun Ramadoss wrote:
> > KSZ87xx and KSZ88xx have chip_id representation at reg location 0.
> > And
> > KSZ9477 compatible switch and LAN937x switch have same chip_id
> > detection
> > at location 0x01 and 0x02. To have the common switch detect
> > functionality for ksz switches, ksz_switch_detect function is
> > introduced.
> >
> > Signed-off-by: Arun Ramadoss <arun.ramadoss@...rochip.com>
> > ---
> > drivers/net/dsa/microchip/ksz8795.c | 46 ---------------
> > drivers/net/dsa/microchip/ksz8795_reg.h | 13 -----
> > drivers/net/dsa/microchip/ksz9477.c | 21 -------
> > drivers/net/dsa/microchip/ksz9477_reg.h | 1 -
> > drivers/net/dsa/microchip/ksz_common.c | 78
> > +++++++++++++++++++++++--
> > drivers/net/dsa/microchip/ksz_common.h | 19 +++++-
> > 6 files changed, 92 insertions(+), 86 deletions(-)
> >
> > diff --git a/drivers/net/dsa/microchip/ksz8795.c
> > b/drivers/net/dsa/microchip/ksz8795.c
> > index 12a599d5e61a..927db57d02db 100644
> > --- a/drivers/net/dsa/microchip/ksz8795.c
> > +++ b/drivers/net/dsa/microchip/ksz8795.c
> > @@ -1424,51 +1424,6 @@ static u32 ksz8_get_port_addr(int port, int
> > offset)
> > return PORT_CTRL_ADDR(port, offset);
> > }
> >
> > -static int ksz8_switch_detect(struct ksz_device *dev)
> > -{
> > - u8 id1, id2;
> > - u16 id16;
> > - int ret;
> > -
> > - /* read chip id */
> > - ret = ksz_read16(dev, REG_CHIP_ID0, &id16);
> > - if (ret)
> > - return ret;
> > -
> > - id1 = id16 >> 8;
> > - id2 = id16 & SW_CHIP_ID_M;
> > -
> > - switch (id1) {
> > - case KSZ87_FAMILY_ID:
> > - if ((id2 != CHIP_ID_94 && id2 != CHIP_ID_95))
> > - return -ENODEV;
> > -
> > - if (id2 == CHIP_ID_95) {
> > - u8 val;
> > -
> > - id2 = 0x95;
> > - ksz_read8(dev, REG_PORT_STATUS_0, &val);
>
> Could you replace all remaining occurrences of REG_PORT_STATUS_0 and
> PORT_FIBER_MODE with KSZ8_PORT_STATUS_0 and KSZ8_PORT_FIBER_MODE?
> It would be good to not have multiple definitions for the same thing.
Ok. I will update the macro unique for KSZ8 Switches.
> > - if (val & PORT_FIBER_MODE)
> > - id2 = 0x65;
> > - } else if (id2 == CHIP_ID_94) {
> > - id2 = 0x94;
> > - }
> > - break;
> > - case KSZ88_FAMILY_ID:
> > - if (id2 != CHIP_ID_63)
> > - return -ENODEV;
> > - break;
> > - default:
> > - dev_err(dev->dev, "invalid family id: %d\n", id1);
> > - return -ENODEV;
> > - }
> > - id16 &= ~0xff;
> > - id16 |= id2;
> > - dev->chip_id = id16;
> > -
> > - return 0;
> > -}
> > -
> >
> > diff --git a/drivers/net/dsa/microchip/ksz8795_reg.h
> > b/drivers/net/dsa/microchip/ksz8795_reg.h
> > index 4109433b6b6c..50cdc2a09f5a 100644
> > --- a/drivers/net/dsa/microchip/ksz8795_reg.h
> > +++ b/drivers/net/dsa/microchip/ksz8795_reg.h
> > @@ -14,23 +14,10 @@
> > #define KS_PRIO_M 0x3
> > #define KS_PRIO_S 2
> >
> > -#define REG_CHIP_ID0 0x00
> > -
> > -#define KSZ87_FAMILY_ID 0x87
> > -#define KSZ88_FAMILY_ID 0x88
> > -
> > -#define REG_CHIP_ID1 0x01
> > -
> > -#define SW_CHIP_ID_M 0xF0
> > -#define SW_CHIP_ID_S 4
> > #define SW_REVISION_M 0x0E
> > #define SW_REVISION_S 1
> > #define SW_START 0x01
> >
> > -#define CHIP_ID_94 0x60
> > -#define CHIP_ID_95 0x90
> > -#define CHIP_ID_63 0x30
> > -
> > #define KSZ8863_REG_SW_RESET 0x43
> >
> > #define KSZ8863_GLOBAL_SOFTWARE_RESET BIT(4)
> > diff --git a/drivers/net/dsa/microchip/ksz9477.c
> > b/drivers/net/dsa/microchip/ksz9477.c
> > index 7afc06681c02..7d3c8f6908b6 100644
> > --- a/drivers/net/dsa/microchip/ksz9477.c
> > +++ b/drivers/net/dsa/microchip/ksz9477.c
> > @@ -1360,23 +1360,6 @@ static u32 ksz9477_get_port_addr(int port,
> > int offset)
> > return PORT_CTRL_ADDR(port, offset);
> > }
> >
> >
> > diff --git a/drivers/net/dsa/microchip/ksz9477_reg.h
> > b/drivers/net/dsa/microchip/ksz9477_reg.h
> > index 7a2c8d4767af..077e35ab11b5 100644
> > --- a/drivers/net/dsa/microchip/ksz9477_reg.h
> > +++ b/drivers/net/dsa/microchip/ksz9477_reg.h
> > @@ -25,7 +25,6 @@
> >
> > #define REG_CHIP_ID2__1 0x0002
> >
> > -#define CHIP_ID_63 0x63
> > #define CHIP_ID_66 0x66
> > #define CHIP_ID_67 0x67
> > #define CHIP_ID_77 0x77
> > diff --git a/drivers/net/dsa/microchip/ksz_common.c
> > b/drivers/net/dsa/microchip/ksz_common.c
> > index 9ca8c8d7740f..9057cdb5971c 100644
> > --- a/drivers/net/dsa/microchip/ksz_common.c
> > +++ b/drivers/net/dsa/microchip/ksz_common.c
> > @@ -930,6 +930,72 @@ void ksz_port_stp_state_set(struct dsa_switch
> > *ds, int port,
> > }
> > EXPORT_SYMBOL_GPL(ksz_port_stp_state_set);
> >
> >
> > diff --git a/drivers/net/dsa/microchip/ksz_common.h
> > b/drivers/net/dsa/microchip/ksz_common.h
> > index 8500eaedad67..d16c095cdefb 100644
> > --- a/drivers/net/dsa/microchip/ksz_common.h
> > +++ b/drivers/net/dsa/microchip/ksz_common.h
> > @@ -90,6 +90,7 @@ struct ksz_device {
> >
> > /* chip specific data */
> > u32 chip_id;
> > + u8 chip_rev;
> > int cpu_port; /* port connected to CPU */
> > int phy_port_cnt;
> > phy_interface_t compat_interface;
> > @@ -182,7 +183,6 @@ struct ksz_dev_ops {
> > void (*freeze_mib)(struct ksz_device *dev, int port, bool
> > freeze);
> > void (*port_init_cnt)(struct ksz_device *dev, int port);
> > int (*shutdown)(struct ksz_device *dev);
> > - int (*detect)(struct ksz_device *dev);
> > int (*init)(struct ksz_device *dev);
> > void (*exit)(struct ksz_device *dev);
> > };
> > @@ -353,6 +353,23 @@ static inline void ksz_regmap_unlock(void
> > *__mtx)
> > #define PORT_RX_ENABLE BIT(1)
> > #define PORT_LEARN_DISABLE BIT(0)
> >
> > +/* Switch ID Defines */
> > +#define REG_CHIP_ID0 0x00
> > +
> > +#define SW_FAMILY_ID_M GENMASK(15, 8)
> > +#define KSZ87_FAMILY_ID 0x87
> > +#define KSZ88_FAMILY_ID 0x88
> > +
> > +#define KSZ8_PORT_STATUS_0 0x08
> > +#define KSZ8_PORT_FIBER_MODE BIT(7)
> > +
> > +#define SW_CHIP_ID_M GENMASK(7, 4)
> > +#define CHIP_ID_94 0x6
> > +#define CHIP_ID_95 0x9
>
> KSZ87_CHIP_ID_xxx maybe?
Ok. I will update.
>
> > +#define CHIP_ID_63 0x3
>
> And KSZ88_CHIP_ID_63.
I will rename it this.
>
> > +
> > +#define SW_REV_ID_M GENMASK(7, 4)
> > +
> > /* Regmap tables generation */
> > #define KSZ_SPI_OP_RD 3
> > #define KSZ_SPI_OP_WR 2
> > --
> > 2.36.1
> >
>
>
Powered by blists - more mailing lists