| Message ID | 20220530104257.21485-4-arun.ramadoss@microchip.com (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: dsa: microchip: common spi probe for the ksz series switches | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/subject_prefix | success | Link |
| netdev/cover_letter | success | Series has a cover letter |
| netdev/patch_count | success | Link |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/cc_maintainers | success | CCed 12 of 12 maintainers |
| netdev/build_clang | success | Errors and warnings before: 0 this patch: 0 |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/check_selftest | success | No net selftest shell script |
| netdev/verify_fixes | success | No Fixes tag |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 135 lines checked |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
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@microchip.com> > --- It would be easier to review if you would split the phy_read/phy_write change from the get_tag_protocol change. > 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/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c > index 7d3c8f6908b6..4fb96e53487e 100644 > --- a/drivers/net/dsa/microchip/ksz9477.c > +++ b/drivers/net/dsa/microchip/ksz9477.c > @@ -276,21 +276,8 @@ static void ksz9477_port_init_cnt(struct ksz_device *dev, int port) > mutex_unlock(&mib->cnt_mutex); > } > > -static enum dsa_tag_protocol ksz9477_get_tag_protocol(struct dsa_switch *ds, > - int port, > - enum dsa_tag_protocol mp) > +static void ksz9477_r_phy(struct ksz_device *dev, u16 addr, u16 reg, u16 *data) > { > - enum dsa_tag_protocol proto = DSA_TAG_PROTO_KSZ9477; > - struct ksz_device *dev = ds->priv; > - > - if (dev->features & IS_9893) > - proto = DSA_TAG_PROTO_KSZ9893; > - return proto; > -} > - > -static int ksz9477_phy_read16(struct dsa_switch *ds, int addr, int reg) > -{ > - struct ksz_device *dev = ds->priv; > u16 val = 0xffff; > > /* No real PHY after this. Simulate the PHY. > @@ -335,24 +322,20 @@ static int ksz9477_phy_read16(struct dsa_switch *ds, int addr, int reg) > ksz_pread16(dev, addr, 0x100 + (reg << 1), &val); > } > > - return val; > + *data = val; > } > > -static int ksz9477_phy_write16(struct dsa_switch *ds, int addr, int reg, > - u16 val) > +static void ksz9477_w_phy(struct ksz_device *dev, u16 addr, u16 reg, u16 val) > { > - struct ksz_device *dev = ds->priv; > - > /* No real PHY after this. */ > if (addr >= dev->phy_port_cnt) > - return 0; > + return; > > /* No gigabit support. Do not write to this register. */ > if (!(dev->features & GBIT_SUPPORT) && reg == MII_CTRL1000) > - return 0; > - ksz_pwrite16(dev, addr, 0x100 + (reg << 1), val); > + return; > > - return 0; > + ksz_pwrite16(dev, addr, 0x100 + (reg << 1), val); > } > > static void ksz9477_cfg_port_member(struct ksz_device *dev, int port, > @@ -1326,10 +1309,10 @@ static int ksz9477_setup(struct dsa_switch *ds) > } > > static const struct dsa_switch_ops ksz9477_switch_ops = { > - .get_tag_protocol = ksz9477_get_tag_protocol, > + .get_tag_protocol = ksz_get_tag_protocol, > .setup = ksz9477_setup, > - .phy_read = ksz9477_phy_read16, > - .phy_write = ksz9477_phy_write16, > + .phy_read = ksz_phy_read16, > + .phy_write = ksz_phy_write16, > .phylink_mac_link_down = ksz_mac_link_down, > .phylink_get_caps = ksz9477_get_caps, > .port_enable = ksz_enable_port, > @@ -1417,6 +1400,8 @@ static const struct ksz_dev_ops ksz9477_dev_ops = { > .cfg_port_member = ksz9477_cfg_port_member, > .flush_dyn_mac_table = ksz9477_flush_dyn_mac_table, > .port_setup = ksz9477_port_setup, > + .r_phy = ksz9477_r_phy, > + .w_phy = ksz9477_w_phy, > .r_mib_cnt = ksz9477_r_mib_cnt, > .r_mib_pkt = ksz9477_r_mib_pkt, > .r_mib_stat64 = ksz_r_mib_stats64, > 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. > + > + 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 >
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@microchip.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 > > > >
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/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c index 7d3c8f6908b6..4fb96e53487e 100644 --- a/drivers/net/dsa/microchip/ksz9477.c +++ b/drivers/net/dsa/microchip/ksz9477.c @@ -276,21 +276,8 @@ static void ksz9477_port_init_cnt(struct ksz_device *dev, int port) mutex_unlock(&mib->cnt_mutex); } -static enum dsa_tag_protocol ksz9477_get_tag_protocol(struct dsa_switch *ds, - int port, - enum dsa_tag_protocol mp) +static void ksz9477_r_phy(struct ksz_device *dev, u16 addr, u16 reg, u16 *data) { - enum dsa_tag_protocol proto = DSA_TAG_PROTO_KSZ9477; - struct ksz_device *dev = ds->priv; - - if (dev->features & IS_9893) - proto = DSA_TAG_PROTO_KSZ9893; - return proto; -} - -static int ksz9477_phy_read16(struct dsa_switch *ds, int addr, int reg) -{ - struct ksz_device *dev = ds->priv; u16 val = 0xffff; /* No real PHY after this. Simulate the PHY. @@ -335,24 +322,20 @@ static int ksz9477_phy_read16(struct dsa_switch *ds, int addr, int reg) ksz_pread16(dev, addr, 0x100 + (reg << 1), &val); } - return val; + *data = val; } -static int ksz9477_phy_write16(struct dsa_switch *ds, int addr, int reg, - u16 val) +static void ksz9477_w_phy(struct ksz_device *dev, u16 addr, u16 reg, u16 val) { - struct ksz_device *dev = ds->priv; - /* No real PHY after this. */ if (addr >= dev->phy_port_cnt) - return 0; + return; /* No gigabit support. Do not write to this register. */ if (!(dev->features & GBIT_SUPPORT) && reg == MII_CTRL1000) - return 0; - ksz_pwrite16(dev, addr, 0x100 + (reg << 1), val); + return; - return 0; + ksz_pwrite16(dev, addr, 0x100 + (reg << 1), val); } static void ksz9477_cfg_port_member(struct ksz_device *dev, int port, @@ -1326,10 +1309,10 @@ static int ksz9477_setup(struct dsa_switch *ds) } static const struct dsa_switch_ops ksz9477_switch_ops = { - .get_tag_protocol = ksz9477_get_tag_protocol, + .get_tag_protocol = ksz_get_tag_protocol, .setup = ksz9477_setup, - .phy_read = ksz9477_phy_read16, - .phy_write = ksz9477_phy_write16, + .phy_read = ksz_phy_read16, + .phy_write = ksz_phy_write16, .phylink_mac_link_down = ksz_mac_link_down, .phylink_get_caps = ksz9477_get_caps, .port_enable = ksz_enable_port, @@ -1417,6 +1400,8 @@ static const struct ksz_dev_ops ksz9477_dev_ops = { .cfg_port_member = ksz9477_cfg_port_member, .flush_dyn_mac_table = ksz9477_flush_dyn_mac_table, .port_setup = ksz9477_port_setup, + .r_phy = ksz9477_r_phy, + .w_phy = ksz9477_w_phy, .r_mib_cnt = ksz9477_r_mib_cnt, .r_mib_pkt = ksz9477_r_mib_pkt, .r_mib_stat64 = ksz_r_mib_stats64, 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; + + 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 */
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@microchip.com> --- 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(-)