Re: [PATCH v4 09/11] net: dsa: microchip: Add Microchip KSZ8863 SMI based driver support
From: Florian Fainelli <f.fainelli@gmail.com>
Date: 2020-08-03 18:12:36
On 8/2/2020 10:44 PM, Michael Grzeschik wrote:
Add KSZ88X3 driver support. We add support for the KXZ88X3 three port switches using the Microchip SMI Interface. They are supported using the MDIO-Bitbang Interface. Signed-off-by: Michael Grzeschik <redacted>
[snip
+ +config NET_DSA_MICROCHIP_KSZ8863_SMI + tristate "KSZ series SMI connected switch driver" + depends on NET_DSA_MICROCHIP_KSZ8795 + select MDIO_BITBANG + default y
As Randy already identified please remove this.
quoted hunk ↗ jump to hunk
+ help + Select to enable support for registering switches configured through + Microchip SMI. It Supports the KSZ8863 and KSZ8873 Switch.diff --git a/drivers/net/dsa/microchip/Makefile b/drivers/net/dsa/microchip/Makefile index 929caa81e782ed2..2a03b21a3386f5d 100644 --- a/drivers/net/dsa/microchip/Makefile +++ b/drivers/net/dsa/microchip/Makefile@@ -5,3 +5,4 @@ obj-$(CONFIG_NET_DSA_MICROCHIP_KSZ9477_I2C) += ksz9477_i2c.o obj-$(CONFIG_NET_DSA_MICROCHIP_KSZ9477_SPI) += ksz9477_spi.o obj-$(CONFIG_NET_DSA_MICROCHIP_KSZ8795) += ksz8795.o obj-$(CONFIG_NET_DSA_MICROCHIP_KSZ8795_SPI) += ksz8795_spi.o +obj-$(CONFIG_NET_DSA_MICROCHIP_KSZ8863_SMI) += ksz8863_smi.odiff --git a/drivers/net/dsa/microchip/ksz8863_smi.c b/drivers/net/dsa/microchip/ksz8863_smi.c new file mode 100644 index 000000000000000..fd493441d725284 --- /dev/null +++ b/drivers/net/dsa/microchip/ksz8863_smi.c@@ -0,0 +1,204 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Microchip KSZ8863 series register access through SMI + * + * Copyright (C) 2019 Pengutronix, Michael Grzeschik <kernel@pengutronix.de> + */ + +#include "ksz8.h" +#include "ksz_common.h" + +/* Serial Management Interface (SMI) uses the following frame format: + * + * preamble|start|Read/Write| PHY | REG |TA| Data bits | Idle + * |frame| OP code |address |address| | | + * read | 32x1´s | 01 | 00 | 1xRRR | RRRRR |Z0| 00000000DDDDDDDD | Z + * write| 32x1´s | 01 | 00 | 0xRRR | RRRRR |10| xxxxxxxxDDDDDDDD | Z + * + */ + +static int ksz8863_mdio_read(void *ctx, const void *reg_buf, size_t reg_len, + void *val_buf, size_t val_len) +{ + struct ksz_device *dev = (struct ksz_device *)ctx;
There is no need to cast a void pointer, can you also make it a const void *ctx?
+ struct ksz8 *ksz8 = dev->priv;
+ struct mdio_device *mdev = ksz8->priv;
+ u8 reg = *(u8 *)reg_buf;
+ u8 *val = val_buf;
+ int ret = 0;
+ int i;
+
+ mutex_lock_nested(&mdev->bus->mdio_lock, MDIO_MUTEX_NESTED);
+ for (i = 0; i < val_len; i++) {
+ int tmp = reg + i;Humm, how does this work for reg_len >1 with reg being a scalar? Why not just use reg[i]?
+ + ret = __mdiobus_read(mdev->bus, ((tmp & 0xE0) >> 5) | + BIT(4), tmp);
Can we provide defines instead of these numbers?
+ if (ret < 0)
+ goto out;
+
+ val[i] = ret;
+ }
+ ret = 0;
+
+ out:
+ mutex_unlock(&mdev->bus->mdio_lock);
+
+ return ret;
+}
+
+static int ksz8863_mdio_write(void *ctx, const void *data, size_t count)
+{
+ struct ksz_device *dev = (struct ksz_device *)ctx;Likewise, no need to cast here.
+ struct ksz8 *ksz8 = dev->priv; + struct mdio_device *mdev = ksz8->priv; + u8 *val = (u8 *)(data + 4); + u32 reg = *(u32 *)data;
Humm, are you positive this works for all endian configurations? -- Florian