| Message ID | 1551224265-9304-4-git-send-email-Tristram.Ha@microchip.com |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | net: dsa: microchip: add KSZ9893 switch support | expand |
Hi Tristram,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on net-next/master]
url: https://github.com/0day-ci/linux/commits/Tristram-Ha-microchip-com/net-dsa-microchip-add-KSZ9893-switch-support/20190227-141333
config: i386-randconfig-x009-201908 (attached as .config)
compiler: gcc-8 (Debian 8.2.0-20) 8.2.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
drivers/net/dsa/microchip/ksz9477.c: In function 'ksz9477_switch_detect':
>> drivers/net/dsa/microchip/ksz9477.c:1516:8: error: implicit declaration of function 'of_modalias_node'; did you mean 'of_match_node'? [-Werror=implicit-function-declaration]
if (!of_modalias_node(dev->dev->of_node, name, sizeof(name))) {
^~~~~~~~~~~~~~~~
of_match_node
cc1: some warnings being treated as errors
vim +1516 drivers/net/dsa/microchip/ksz9477.c
1433
1434 static int ksz9477_switch_detect(struct ksz_device *dev)
1435 {
1436 int chip = -1;
1437 u8 data8;
1438 u8 id_hi;
1439 u8 id_lo;
1440 u32 id32;
1441 int ret;
1442
1443 /* turn off SPI DO Edge select */
1444 ret = ksz_read8(dev, REG_SW_GLOBAL_SERIAL_CTRL_0, &data8);
1445 if (ret)
1446 return ret;
1447
1448 data8 &= ~SPI_AUTO_EDGE_DETECTION;
1449 ret = ksz_write8(dev, REG_SW_GLOBAL_SERIAL_CTRL_0, data8);
1450 if (ret)
1451 return ret;
1452
1453 /* read chip id */
1454 ret = ksz_read32(dev, REG_CHIP_ID0__1, &id32);
1455 if (ret)
1456 return ret;
1457 ret = ksz_read8(dev, REG_GLOBAL_OPTIONS, &data8);
1458 if (ret)
1459 return ret;
1460
1461 /* Number of ports can be reduced depending on chip. */
1462 dev->mib_port_cnt = TOTAL_PORT_NUM;
1463 dev->phy_port_cnt = 5;
1464
1465 /* Default capability is gigabit capable. */
1466 dev->features = GBIT_SUPPORT;
1467
1468 id_hi = (u8)(id32 >> 16);
1469 id_lo = (u8)(id32 >> 8);
1470 if ((id_lo & 0xf) == 3) {
1471 /* Chip is from KSZ9893 design. */
1472 dev->features |= IS_9893;
1473
1474 /* Chip does not support gigabit. */
1475 if (data8 & SW_QW_ABLE)
1476 dev->features &= ~GBIT_SUPPORT;
1477 dev->mib_port_cnt = 3;
1478 dev->phy_port_cnt = 2;
1479 if (!(data8 & SW_AVB_ABLE))
1480 chip = KSZ9893_SW_CHIP;
1481 else if (data8 & SW_QW_ABLE)
1482 chip = KSZ8563_SW_CHIP;
1483 else
1484 chip = KSZ9563_SW_CHIP;
1485 } else {
1486 /* Chip uses new XMII register definitions. */
1487 dev->features |= NEW_XMII;
1488
1489 /* Chip does not support gigabit. */
1490 if (!(data8 & SW_GIGABIT_ABLE))
1491 dev->features &= ~GBIT_SUPPORT;
1492 if ((id_lo & 0xf) == 6)
1493 dev->mib_port_cnt = 6;
1494 if (id_hi == FAMILY_ID_94)
1495 chip = KSZ9477_SW_CHIP;
1496 else if (id_hi == FAMILY_ID_98 && id_lo == CHIP_ID_97)
1497 chip = KSZ9897_SW_CHIP;
1498 else if (id_hi == FAMILY_ID_98 && id_lo == CHIP_ID_96)
1499 chip = KSZ9896_SW_CHIP;
1500 else if (id_hi == FAMILY_ID_95 && id_lo == CHIP_ID_67)
1501 chip = KSZ9567_SW_CHIP;
1502 else if (id_hi == FAMILY_ID_85 && id_lo == CHIP_ID_67)
1503 chip = KSZ8567_SW_CHIP;
1504 if (id_lo == CHIP_ID_67) {
1505 id_hi = FAMILY_ID_98;
1506 id_lo = CHIP_ID_97;
1507 } else if (id_lo == CHIP_ID_66) {
1508 id_hi = FAMILY_ID_98;
1509 id_lo = CHIP_ID_96;
1510 }
1511 }
1512 if (dev->dev->of_node) {
1513 char name[80];
1514
1515 /* KSZ8565 chip can only be set through device tree. */
> 1516 if (!of_modalias_node(dev->dev->of_node, name, sizeof(name))) {
1517 if (!strcmp(name, "ksz8565")) {
1518 chip = KSZ8565_SW_CHIP;
1519 id_hi = FAMILY_ID_98;
1520 id_lo = 0x95;
1521 }
1522 }
1523 }
1524
1525 /* Change chip id to known ones so it can be matched against them. */
1526 id32 = (id_hi << 16) | (id_lo << 8);
1527
1528 dev->chip_id = id32;
1529
1530 /* Update switch device name to matched chip. */
1531 if (chip >= 0)
1532 dev->name = ksz9477_chip_names[chip];
1533
1534 return 0;
1535 }
1536
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
> + if (dev->dev->of_node) { > + char name[80]; > + > + /* KSZ8565 chip can only be set through device tree. */ > + if (!of_modalias_node(dev->dev->of_node, name, sizeof(name))) { > + if (!strcmp(name, "ksz8565")) { > + chip = KSZ8565_SW_CHIP; > + id_hi = FAMILY_ID_98; > + id_lo = 0x95; > + } > + } of_device_is_compatible() seems like a better call use use. Andrew
On Tue, Feb 26, 2019 at 03:37:45PM -0800, Tristram.Ha@microchip.com wrote: > From: Tristram Ha <Tristram.Ha@microchip.com> > > Add other switches in KSZ9477 family. > > KSZ9896 is a switch with 6 ports; the last one is typically used to > connect to MAC. > KSZ9567 is same as KSZ9897 but with 1588 PTP capability. > KSZ8567 is same as KSZ9567 but without gigabit capability. > KSZ9563 is same as KSZ9893 but with 1588 PTP capability. > KSZ8563 is same as KSZ9563 but without gigabit capability. > KSZ8565 is a switch with 5 ports; however, port 7 has to be used to > connect to MAC. This chip can only be set through device tree. > > Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com> > --- > drivers/net/dsa/microchip/ksz9477.c | 93 ++++++++++++++++++++++++++++++++- > drivers/net/dsa/microchip/ksz9477_spi.c | 3 ++ > drivers/net/dsa/microchip/ksz_common.c | 4 ++ > 3 files changed, 98 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c > index 3bb548a..81e7c2f 100644 > --- a/drivers/net/dsa/microchip/ksz9477.c > +++ b/drivers/net/dsa/microchip/ksz9477.c > @@ -1264,6 +1264,32 @@ static void ksz9477_port_setup(struct ksz_device *dev, int port, bool cpu_port) > ksz_pread16(dev, port, REG_PORT_PHY_INT_ENABLE, &data16); > } > > +#define KSZ_CHIP_NAME_SIZE 18 > + > +static char *ksz9477_chip_names[KSZ_CHIP_NAME_SIZE] = { > + "Microchip KSZ9477", This looks wrong. You are defining an array of 18 elements, not an array of pointers to 18 byte chars. > + "Microchip KSZ9897", > + "Microchip KSZ9896", > + "Microchip KSZ9567", > + "Microchip KSZ8567", > + "Microchip KSZ8565", > + "Microchip KSZ9893", > + "Microchip KSZ9563", > + "Microchip KSZ8563", > +}; > + > +enum { > + KSZ9477_SW_CHIP, > + KSZ9897_SW_CHIP, > + KSZ9896_SW_CHIP, > + KSZ9567_SW_CHIP, > + KSZ8567_SW_CHIP, > + KSZ8565_SW_CHIP, > + KSZ9893_SW_CHIP, > + KSZ9563_SW_CHIP, > + KSZ8563_SW_CHIP, > +}; There should be a one-to-one mapping between this enum and the array above. You can make this clear using Designated Initializers. > + > static void ksz9477_config_cpu_port(struct dsa_switch *ds) > { > struct ksz_device *dev = ds->priv; > @@ -1314,7 +1340,8 @@ static void ksz9477_config_cpu_port(struct dsa_switch *ds) > p->vid_member = (1 << i); > p->member = dev->port_mask; > ksz9477_port_stp_state_set(ds, i, BR_STATE_DISABLED); > - p->on = 1; > + if (!dsa_is_unused_port(ds, i)) > + p->on = 1; > if (i < dev->phy_port_cnt) > p->phy = 1; > if (dev->chip_id == 0x00947700 && i == 6) { > @@ -1406,6 +1433,7 @@ static u32 ksz9477_get_port_addr(int port, int offset) > > static int ksz9477_switch_detect(struct ksz_device *dev) > { > + int chip = -1; > u8 data8; > u8 id_hi; > u8 id_lo; > @@ -1448,6 +1476,12 @@ static int ksz9477_switch_detect(struct ksz_device *dev) > dev->features &= ~GBIT_SUPPORT; > dev->mib_port_cnt = 3; > dev->phy_port_cnt = 2; > + if (!(data8 & SW_AVB_ABLE)) > + chip = KSZ9893_SW_CHIP; > + else if (data8 & SW_QW_ABLE) > + chip = KSZ8563_SW_CHIP; > + else > + chip = KSZ9563_SW_CHIP; > } else { > /* Chip uses new XMII register definitions. */ > dev->features |= NEW_XMII; > @@ -1455,6 +1489,37 @@ static int ksz9477_switch_detect(struct ksz_device *dev) > /* Chip does not support gigabit. */ > if (!(data8 & SW_GIGABIT_ABLE)) > dev->features &= ~GBIT_SUPPORT; > + if ((id_lo & 0xf) == 6) > + dev->mib_port_cnt = 6; > + if (id_hi == FAMILY_ID_94) > + chip = KSZ9477_SW_CHIP; > + else if (id_hi == FAMILY_ID_98 && id_lo == CHIP_ID_97) > + chip = KSZ9897_SW_CHIP; > + else if (id_hi == FAMILY_ID_98 && id_lo == CHIP_ID_96) > + chip = KSZ9896_SW_CHIP; > + else if (id_hi == FAMILY_ID_95 && id_lo == CHIP_ID_67) > + chip = KSZ9567_SW_CHIP; > + else if (id_hi == FAMILY_ID_85 && id_lo == CHIP_ID_67) > + chip = KSZ8567_SW_CHIP; > + if (id_lo == CHIP_ID_67) { > + id_hi = FAMILY_ID_98; > + id_lo = CHIP_ID_97; > + } else if (id_lo == CHIP_ID_66) { > + id_hi = FAMILY_ID_98; > + id_lo = CHIP_ID_96; > + } Maybe add a mask to ksz_chip_data table, so you can mask id_low, id_high and then compare against chip_id? > @@ -1495,6 +1564,15 @@ struct ksz_chip_data { > .port_cnt = 7, /* total physical port count */ > }, > { > + .chip_id = 0x00989600, > + .dev_name = "KSZ9896", > + .num_vlans = 4096, > + .num_alus = 4096, > + .num_statics = 16, > + .cpu_ports = 0x3F, /* can be configured as cpu port */ > + .port_cnt = 6, /* total port count */ > + }, Andrew
diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c index 3bb548a..81e7c2f 100644 --- a/drivers/net/dsa/microchip/ksz9477.c +++ b/drivers/net/dsa/microchip/ksz9477.c @@ -1264,6 +1264,32 @@ static void ksz9477_port_setup(struct ksz_device *dev, int port, bool cpu_port) ksz_pread16(dev, port, REG_PORT_PHY_INT_ENABLE, &data16); } +#define KSZ_CHIP_NAME_SIZE 18 + +static char *ksz9477_chip_names[KSZ_CHIP_NAME_SIZE] = { + "Microchip KSZ9477", + "Microchip KSZ9897", + "Microchip KSZ9896", + "Microchip KSZ9567", + "Microchip KSZ8567", + "Microchip KSZ8565", + "Microchip KSZ9893", + "Microchip KSZ9563", + "Microchip KSZ8563", +}; + +enum { + KSZ9477_SW_CHIP, + KSZ9897_SW_CHIP, + KSZ9896_SW_CHIP, + KSZ9567_SW_CHIP, + KSZ8567_SW_CHIP, + KSZ8565_SW_CHIP, + KSZ9893_SW_CHIP, + KSZ9563_SW_CHIP, + KSZ8563_SW_CHIP, +}; + static void ksz9477_config_cpu_port(struct dsa_switch *ds) { struct ksz_device *dev = ds->priv; @@ -1314,7 +1340,8 @@ static void ksz9477_config_cpu_port(struct dsa_switch *ds) p->vid_member = (1 << i); p->member = dev->port_mask; ksz9477_port_stp_state_set(ds, i, BR_STATE_DISABLED); - p->on = 1; + if (!dsa_is_unused_port(ds, i)) + p->on = 1; if (i < dev->phy_port_cnt) p->phy = 1; if (dev->chip_id == 0x00947700 && i == 6) { @@ -1406,6 +1433,7 @@ static u32 ksz9477_get_port_addr(int port, int offset) static int ksz9477_switch_detect(struct ksz_device *dev) { + int chip = -1; u8 data8; u8 id_hi; u8 id_lo; @@ -1448,6 +1476,12 @@ static int ksz9477_switch_detect(struct ksz_device *dev) dev->features &= ~GBIT_SUPPORT; dev->mib_port_cnt = 3; dev->phy_port_cnt = 2; + if (!(data8 & SW_AVB_ABLE)) + chip = KSZ9893_SW_CHIP; + else if (data8 & SW_QW_ABLE) + chip = KSZ8563_SW_CHIP; + else + chip = KSZ9563_SW_CHIP; } else { /* Chip uses new XMII register definitions. */ dev->features |= NEW_XMII; @@ -1455,6 +1489,37 @@ static int ksz9477_switch_detect(struct ksz_device *dev) /* Chip does not support gigabit. */ if (!(data8 & SW_GIGABIT_ABLE)) dev->features &= ~GBIT_SUPPORT; + if ((id_lo & 0xf) == 6) + dev->mib_port_cnt = 6; + if (id_hi == FAMILY_ID_94) + chip = KSZ9477_SW_CHIP; + else if (id_hi == FAMILY_ID_98 && id_lo == CHIP_ID_97) + chip = KSZ9897_SW_CHIP; + else if (id_hi == FAMILY_ID_98 && id_lo == CHIP_ID_96) + chip = KSZ9896_SW_CHIP; + else if (id_hi == FAMILY_ID_95 && id_lo == CHIP_ID_67) + chip = KSZ9567_SW_CHIP; + else if (id_hi == FAMILY_ID_85 && id_lo == CHIP_ID_67) + chip = KSZ8567_SW_CHIP; + if (id_lo == CHIP_ID_67) { + id_hi = FAMILY_ID_98; + id_lo = CHIP_ID_97; + } else if (id_lo == CHIP_ID_66) { + id_hi = FAMILY_ID_98; + id_lo = CHIP_ID_96; + } + } + if (dev->dev->of_node) { + char name[80]; + + /* KSZ8565 chip can only be set through device tree. */ + if (!of_modalias_node(dev->dev->of_node, name, sizeof(name))) { + if (!strcmp(name, "ksz8565")) { + chip = KSZ8565_SW_CHIP; + id_hi = FAMILY_ID_98; + id_lo = 0x95; + } + } } /* Change chip id to known ones so it can be matched against them. */ @@ -1462,6 +1527,10 @@ static int ksz9477_switch_detect(struct ksz_device *dev) dev->chip_id = id32; + /* Update switch device name to matched chip. */ + if (chip >= 0) + dev->name = ksz9477_chip_names[chip]; + return 0; } @@ -1495,6 +1564,15 @@ struct ksz_chip_data { .port_cnt = 7, /* total physical port count */ }, { + .chip_id = 0x00989600, + .dev_name = "KSZ9896", + .num_vlans = 4096, + .num_alus = 4096, + .num_statics = 16, + .cpu_ports = 0x3F, /* can be configured as cpu port */ + .port_cnt = 6, /* total port count */ + }, + { .chip_id = 0x00989300, .dev_name = "KSZ9893", .num_vlans = 4096, @@ -1503,6 +1581,15 @@ struct ksz_chip_data { .cpu_ports = 0x07, /* can be configured as cpu port */ .port_cnt = 3, /* total port count */ }, + { + .chip_id = 0x00989500, + .dev_name = "KSZ8565", + .num_vlans = 4096, + .num_alus = 4096, + .num_statics = 16, + .cpu_ports = 0x4F, /* can be configured as cpu port */ + .port_cnt = 7, /* total port count */ + }, }; static int ksz9477_switch_init(struct ksz_device *dev) @@ -1515,7 +1602,8 @@ static int ksz9477_switch_init(struct ksz_device *dev) const struct ksz_chip_data *chip = &ksz9477_switch_chips[i]; if (dev->chip_id == chip->chip_id) { - dev->name = chip->dev_name; + if (!dev->name) + dev->name = chip->dev_name; dev->num_vlans = chip->num_vlans; dev->num_alus = chip->num_alus; dev->num_statics = chip->num_statics; @@ -1531,6 +1619,7 @@ static int ksz9477_switch_init(struct ksz_device *dev) return -ENODEV; dev->port_mask = (1 << dev->port_cnt) - 1; + dev->port_mask &= dev->cpu_ports; dev->reg_mib_cnt = SWITCH_COUNTER_NUM; dev->mib_cnt = TOTAL_SWITCH_COUNTER_NUM; diff --git a/drivers/net/dsa/microchip/ksz9477_spi.c b/drivers/net/dsa/microchip/ksz9477_spi.c index 7517862..878dd64 100644 --- a/drivers/net/dsa/microchip/ksz9477_spi.c +++ b/drivers/net/dsa/microchip/ksz9477_spi.c @@ -155,6 +155,9 @@ static void ksz9477_spi_shutdown(struct spi_device *spi) static const struct of_device_id ksz9477_dt_ids[] = { { .compatible = "microchip,ksz9477" }, { .compatible = "microchip,ksz9897" }, + { .compatible = "microchip,ksz9896" }, + { .compatible = "microchip,ksz9567" }, + { .compatible = "microchip,ksz8565" }, { .compatible = "microchip,ksz9893" }, { .compatible = "microchip,ksz9563" }, {}, diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c index 39dace8..826e046 100644 --- a/drivers/net/dsa/microchip/ksz_common.c +++ b/drivers/net/dsa/microchip/ksz_common.c @@ -84,6 +84,10 @@ static void ksz_mib_read_work(struct work_struct *work) for (i = 0; i < dev->mib_port_cnt; i++) { p = &dev->ports[i]; + + /* Port is not being used. */ + if (!p->on) + continue; mib = &p->mib; mutex_lock(&mib->cnt_mutex);