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 <m.grzesc...@pengutronix.de>
>
[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.
> + 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.o
> diff --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 <ker...@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
