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);