* [PATCH RFC net-next] net: dsa: microchip: add KSZ9477 I2C driver @ 2018-12-18 1:39 Tristram.Ha 2018-12-18 1:40 ` Tristram.Ha 2018-12-18 8:06 ` Dan Carpenter 0 siblings, 2 replies; 10+ messages in thread From: Tristram.Ha @ 2018-12-18 1:39 UTC (permalink / raw) To: Sergio Paracuellos, Andrew Lunn, Florian Fainelli, Pavel Machek, Marek Vasut, Dan Carpenter Cc: Tristram Ha, vivien.didelot, UNGLinuxDriver, netdev From: Tristram Ha <Tristram.Ha@microchip.com> This patch adds KSZ9477 I2C driver support. I know a patch for KSZ9477 I2C driver was already submitted. There is a minor problem though. The structure and code of the I2C driver should match those in the SPI driver. The only difference is the register access. Accordingly the file ksz_i2c.h is created so that some shared I2C functions can be used by other switch drivers. Tristram Ha (1): net: dsa: microchip: add KSZ9477 I2C driver drivers/net/dsa/microchip/Kconfig | 6 + drivers/net/dsa/microchip/Makefile | 1 + drivers/net/dsa/microchip/ksz9477_i2c.c | 194 ++++++++++++++++++++++++++++++++ drivers/net/dsa/microchip/ksz_i2c.h | 69 ++++++++++++ 4 files changed, 270 insertions(+) create mode 100644 drivers/net/dsa/microchip/ksz9477_i2c.c create mode 100644 drivers/net/dsa/microchip/ksz_i2c.h -- 1.9.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH RFC net-next] net: dsa: microchip: add KSZ9477 I2C driver 2018-12-18 1:39 [PATCH RFC net-next] net: dsa: microchip: add KSZ9477 I2C driver Tristram.Ha @ 2018-12-18 1:40 ` Tristram.Ha 2018-12-18 2:55 ` Woojung.Huh ` (2 more replies) 2018-12-18 8:06 ` Dan Carpenter 1 sibling, 3 replies; 10+ messages in thread From: Tristram.Ha @ 2018-12-18 1:40 UTC (permalink / raw) To: Sergio Paracuellos, Andrew Lunn, Florian Fainelli, Pavel Machek, Marek Vasut, Dan Carpenter Cc: Tristram Ha, vivien.didelot, UNGLinuxDriver, netdev From: Tristram Ha <Tristram.Ha@microchip.com> Add KSZ9477 I2C driver support. The code ksz9477.c and ksz_common.c are used together to generate the I2C driver. Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com> --- drivers/net/dsa/microchip/Kconfig | 6 + drivers/net/dsa/microchip/Makefile | 1 + drivers/net/dsa/microchip/ksz9477_i2c.c | 194 ++++++++++++++++++++++++++++++++ drivers/net/dsa/microchip/ksz_i2c.h | 69 ++++++++++++ 4 files changed, 270 insertions(+) create mode 100644 drivers/net/dsa/microchip/ksz9477_i2c.c create mode 100644 drivers/net/dsa/microchip/ksz_i2c.h diff --git a/drivers/net/dsa/microchip/Kconfig b/drivers/net/dsa/microchip/Kconfig index bea29fd..fd94441 100644 --- a/drivers/net/dsa/microchip/Kconfig +++ b/drivers/net/dsa/microchip/Kconfig @@ -9,6 +9,12 @@ menuconfig NET_DSA_MICROCHIP_KSZ9477 help This driver adds support for Microchip KSZ9477 switch chips. +config NET_DSA_MICROCHIP_KSZ9477_I2C + tristate "KSZ9477 series I2C connected switch driver" + depends on NET_DSA_MICROCHIP_KSZ9477 && I2C + help + Select to enable support for registering switches configured through I2C. + config NET_DSA_MICROCHIP_KSZ9477_SPI tristate "KSZ9477 series SPI connected switch driver" depends on NET_DSA_MICROCHIP_KSZ9477 && SPI diff --git a/drivers/net/dsa/microchip/Makefile b/drivers/net/dsa/microchip/Makefile index 3142c18..dbcc5db 100644 --- a/drivers/net/dsa/microchip/Makefile +++ b/drivers/net/dsa/microchip/Makefile @@ -1,3 +1,4 @@ obj-$(CONFIG_NET_DSA_MICROCHIP_KSZ_COMMON) += ksz_common.o obj-$(CONFIG_NET_DSA_MICROCHIP_KSZ9477) += ksz9477.o +obj-$(CONFIG_NET_DSA_MICROCHIP_KSZ9477_I2C) += ksz9477_i2c.o obj-$(CONFIG_NET_DSA_MICROCHIP_KSZ9477_SPI) += ksz9477_spi.o diff --git a/drivers/net/dsa/microchip/ksz9477_i2c.c b/drivers/net/dsa/microchip/ksz9477_i2c.c new file mode 100644 index 0000000..63d2f57 --- /dev/null +++ b/drivers/net/dsa/microchip/ksz9477_i2c.c @@ -0,0 +1,194 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Microchip KSZ9477 series register access through I2C + * + * Copyright (C) 2018 Microchip Technology Inc. + */ + +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/i2c.h> + +#include "ksz_priv.h" +#include "ksz_i2c.h" + +/* Enough to read all switch port registers. */ +#define I2C_TX_BUF_LEN 0x100 + +static int ksz9477_i2c_read_reg(struct i2c_client *i2c, u32 reg, u8 *val, + unsigned int len) +{ + struct i2c_msg msg[2]; + int ret = 0; + + val[0] = (u8)(reg >> 8); + val[1] = (u8)reg; + + msg[0].addr = i2c->addr; + msg[0].flags = 0; + msg[0].len = 2; + msg[0].buf = val; + + msg[1].addr = i2c->addr; + msg[1].flags = I2C_M_RD; + msg[1].len = len; + msg[1].buf = &val[2]; + + if (i2c_transfer(i2c->adapter, msg, 2) != 2) + ret = -ENODEV; + return ret; +} + +static int ksz9477_i2c_write_reg(struct i2c_client *i2c, u32 reg, u8 *val, + unsigned int len) +{ + struct i2c_msg msg; + int ret = 0; + + val[0] = (u8)(reg >> 8); + val[1] = (u8)reg; + + msg.addr = i2c->addr; + msg.flags = 0; + msg.len = 2 + len; + msg.buf = val; + + if (i2c_transfer(i2c->adapter, &msg, 1) != 1) + ret = -ENODEV; + return ret; +} + +static int ksz_i2c_read(struct ksz_device *dev, u32 reg, u8 *data, + unsigned int len) +{ + struct i2c_client *i2c = dev->priv; + int ret; + + ret = ksz9477_i2c_read_reg(i2c, reg, dev->txbuf, len); + if (!ret) + memcpy(data, &dev->txbuf[2], len); + return ret; +} + +static int ksz_i2c_write(struct ksz_device *dev, u32 reg, void *data, + unsigned int len) +{ + struct i2c_client *i2c = dev->priv; + + if (len > I2C_TX_BUF_LEN) + len = I2C_TX_BUF_LEN; + memcpy(&dev->txbuf[2], data, len); + return ksz9477_i2c_write_reg(i2c, reg, dev->txbuf, len); +} + +static int ksz_i2c_read24(struct ksz_device *dev, u32 reg, u32 *val) +{ + int ret; + + *val = 0; + ret = ksz_i2c_read(dev, reg, (u8 *)val, 3); + if (!ret) { + *val = be32_to_cpu(*val); + /* convert to 24bit */ + *val >>= 8; + } + + return ret; +} + +static int ksz_i2c_write24(struct ksz_device *dev, u32 reg, u32 value) +{ + /* make it to big endian 24bit from MSB */ + value <<= 8; + value = cpu_to_be32(value); + return ksz_i2c_write(dev, reg, &value, 3); +} + +static const struct ksz_io_ops ksz9477_i2c_ops = { + .read8 = ksz_i2c_read8, + .read16 = ksz_i2c_read16, + .read24 = ksz_i2c_read24, + .read32 = ksz_i2c_read32, + .write8 = ksz_i2c_write8, + .write16 = ksz_i2c_write16, + .write24 = ksz_i2c_write24, + .write32 = ksz_i2c_write32, + .get = ksz_i2c_get, + .set = ksz_i2c_set, +}; + +static int ksz9477_i2c_probe(struct i2c_client *i2c, + const struct i2c_device_id *i2c_id) +{ + struct ksz_device *dev; + int ret; + + dev = ksz_switch_alloc(&i2c->dev, &ksz9477_i2c_ops, i2c); + if (!dev) + return -ENOMEM; + + if (i2c->dev.platform_data) + dev->pdata = i2c->dev.platform_data; + + dev->txbuf = devm_kzalloc(dev->dev, 2 + I2C_TX_BUF_LEN, GFP_KERNEL); + + ret = ksz9477_switch_register(dev); + + /* Main DSA driver may not be started yet. */ + if (ret) + return ret; + + i2c_set_clientdata(i2c, dev); + + return 0; +} + +static int ksz9477_i2c_remove(struct i2c_client *i2c) +{ + struct ksz_device *dev = i2c_get_clientdata(i2c); + + if (dev) + ksz_switch_remove(dev); + + return 0; +} + +static void ksz9477_i2c_shutdown(struct i2c_client *i2c) +{ + struct ksz_device *dev = i2c_get_clientdata(i2c); + + if (dev && dev->dev_ops->shutdown) + dev->dev_ops->shutdown(dev); +} + +static const struct i2c_device_id ksz9477_i2c_id[] = { + { "ksz9477-switch", 0 }, + {}, +}; + +MODULE_DEVICE_TABLE(i2c, ksz9477_i2c_id); + +static const struct of_device_id ksz9477_dt_ids[] = { + { .compatible = "microchip,ksz9477" }, + { .compatible = "microchip,ksz9897" }, + {}, +}; +MODULE_DEVICE_TABLE(of, ksz9477_dt_ids); + +static struct i2c_driver ksz9477_i2c_driver = { + .driver = { + .name = "ksz9477-switch", + .owner = THIS_MODULE, + .of_match_table = of_match_ptr(ksz9477_dt_ids), + }, + .probe = ksz9477_i2c_probe, + .remove = ksz9477_i2c_remove, + .shutdown = ksz9477_i2c_shutdown, + .id_table = ksz9477_i2c_id, +}; + +module_i2c_driver(ksz9477_i2c_driver); + +MODULE_AUTHOR("Woojung Huh <Woojung.Huh@microchip.com>"); +MODULE_DESCRIPTION("Microchip KSZ9477 Series Switch I2C access Driver"); +MODULE_LICENSE("GPL"); diff --git a/drivers/net/dsa/microchip/ksz_i2c.h b/drivers/net/dsa/microchip/ksz_i2c.h new file mode 100644 index 0000000..b9af0a8 --- /dev/null +++ b/drivers/net/dsa/microchip/ksz_i2c.h @@ -0,0 +1,69 @@ +/* SPDX-License-Identifier: GPL-2.0 + * Microchip KSZ series I2C access common header + * + * Copyright (C) 2018 Microchip Technology Inc. + * Tristram Ha <Tristram.Ha@microchip.com> + */ + +#ifndef __KSZ_I2C_H +#define __KSZ_I2C_H + +/* Chip dependent I2C access */ +static int ksz_i2c_read(struct ksz_device *dev, u32 reg, u8 *data, + unsigned int len); +static int ksz_i2c_write(struct ksz_device *dev, u32 reg, void *data, + unsigned int len); + +static int ksz_i2c_read8(struct ksz_device *dev, u32 reg, u8 *val) +{ + return ksz_i2c_read(dev, reg, val, 1); +} + +static int ksz_i2c_read16(struct ksz_device *dev, u32 reg, u16 *val) +{ + int ret = ksz_i2c_read(dev, reg, (u8 *)val, 2); + + if (!ret) + *val = be16_to_cpu(*val); + + return ret; +} + +static int ksz_i2c_read32(struct ksz_device *dev, u32 reg, u32 *val) +{ + int ret = ksz_i2c_read(dev, reg, (u8 *)val, 4); + + if (!ret) + *val = be32_to_cpu(*val); + + return ret; +} + +static int ksz_i2c_write8(struct ksz_device *dev, u32 reg, u8 value) +{ + return ksz_i2c_write(dev, reg, &value, 1); +} + +static int ksz_i2c_write16(struct ksz_device *dev, u32 reg, u16 value) +{ + value = cpu_to_be16(value); + return ksz_i2c_write(dev, reg, &value, 2); +} + +static int ksz_i2c_write32(struct ksz_device *dev, u32 reg, u32 value) +{ + value = cpu_to_be32(value); + return ksz_i2c_write(dev, reg, &value, 4); +} + +static int ksz_i2c_get(struct ksz_device *dev, u32 reg, void *data, size_t len) +{ + return ksz_i2c_read(dev, reg, data, len); +} + +static int ksz_i2c_set(struct ksz_device *dev, u32 reg, void *data, size_t len) +{ + return ksz_i2c_write(dev, reg, data, len); +} + +#endif -- 1.9.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* RE: [PATCH RFC net-next] net: dsa: microchip: add KSZ9477 I2C driver 2018-12-18 1:40 ` Tristram.Ha @ 2018-12-18 2:55 ` Woojung.Huh 2018-12-18 4:16 ` Tristram.Ha 2018-12-18 6:24 ` Sergio Paracuellos 2 siblings, 0 replies; 10+ messages in thread From: Woojung.Huh @ 2018-12-18 2:55 UTC (permalink / raw) To: Tristram.Ha, sergio.paracuellos, andrew, f.fainelli, pavel, marex, dan.carpenter Cc: vivien.didelot, UNGLinuxDriver, netdev Tristram, > +++ b/drivers/net/dsa/microchip/ksz9477_i2c.c .. > +MODULE_AUTHOR("Woojung Huh <Woojung.Huh@microchip.com>"); Believe this should be your name. Thanks. Woojung ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH RFC net-next] net: dsa: microchip: add KSZ9477 I2C driver 2018-12-18 1:40 ` Tristram.Ha 2018-12-18 2:55 ` Woojung.Huh @ 2018-12-18 4:16 ` Tristram.Ha 2018-12-18 6:30 ` Sergio Paracuellos 2018-12-18 6:24 ` Sergio Paracuellos 2 siblings, 1 reply; 10+ messages in thread From: Tristram.Ha @ 2018-12-18 4:16 UTC (permalink / raw) To: sergio.paracuellos Cc: vivien.didelot, UNGLinuxDriver, netdev, dan.carpenter, marex, f.fainelli, andrew, pavel Sorry about the patch. I know you were using the code from the new SPI and old I2C drivers to come up with your patch. You can incorporate the changes and test the driver and re-submit the patch if you want. Your i2c_probe function displays an error message when ksz9477_switch_register is not successful. It is likely the error code is from the dsa_register_switch function when the core DSA driver is not loaded yet. All the register access functions in ksz_io_ops structure will be called by the driver code. The length should always be non-zero. The set and get functions can be invoked by the standard kernel register access API, which is called in user space. The functions that handle this API make sure the length is non-zero before continuing. For switches with simple register set the get function can dump all registers in one call. I am not sure this register access API is allowed anymore as this may create a security hole in the kernel, but it helps greatly during development and testing as the driver is quite opaque to display the hardware state when something is wrong. A little out-of-topic is the modalias now returns a different string rather than "i2c:ksz9477." This is done with the "cat /sys/bus/i2c/devices/0-005f/modalias" command. The string looks legit but it is difficult for regular users to get anything from it. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH RFC net-next] net: dsa: microchip: add KSZ9477 I2C driver 2018-12-18 4:16 ` Tristram.Ha @ 2018-12-18 6:30 ` Sergio Paracuellos 0 siblings, 0 replies; 10+ messages in thread From: Sergio Paracuellos @ 2018-12-18 6:30 UTC (permalink / raw) To: Tristram.Ha Cc: vivien.didelot, UNGLinuxDriver, netdev, Dan Carpenter, marex, Florian Fainelli, Andrew Lunn, pavel Hi Tristam, On Tue, Dec 18, 2018 at 5:16 AM <Tristram.Ha@microchip.com> wrote: > > Sorry about the patch. I know you were using the code from the new SPI and old I2C drivers to come up with your patch. You can incorporate the changes and test the driver and re-submit the patch if you want. For me is ok to just use the driver you have just submitted, there is no need to me to resend anything, I think. I only wanted an i2c driver for this switch and because there wasn't one I just sent my patches :-). > > Your i2c_probe function displays an error message when ksz9477_switch_register is not successful. It is likely the error code is from the dsa_register_switch function when the core DSA driver is not loaded yet. > > All the register access functions in ksz_io_ops structure will be called by the driver code. The length should always be non-zero. The set and get functions can be invoked by the standard kernel register access API, which is called in user space. The functions that handle this API make sure the length is non-zero before continuing. For switches with simple register set the get function can dump all registers in one call. > > I am not sure this register access API is allowed anymore as this may create a security hole in the kernel, but it helps greatly during development and testing as the driver is quite opaque to display the hardware state when something is wrong. > > A little out-of-topic is the modalias now returns a different string rather than "i2c:ksz9477." This is done with the "cat /sys/bus/i2c/devices/0-005f/modalias" command. The string looks legit but it is difficult for regular users to get anything from it. > Thanks for clarification. Best regards, Sergio Paracuellos ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH RFC net-next] net: dsa: microchip: add KSZ9477 I2C driver 2018-12-18 1:40 ` Tristram.Ha 2018-12-18 2:55 ` Woojung.Huh 2018-12-18 4:16 ` Tristram.Ha @ 2018-12-18 6:24 ` Sergio Paracuellos 2018-12-18 10:01 ` Andrew Lunn 2 siblings, 1 reply; 10+ messages in thread From: Sergio Paracuellos @ 2018-12-18 6:24 UTC (permalink / raw) To: Tristram.Ha Cc: Andrew Lunn, Florian Fainelli, Pavel Machek, Marek Vasut, Dan Carpenter, vivien.didelot, UNGLinuxDriver, netdev Hi Tristram, Two minor comments. On Tue, Dec 18, 2018 at 2:40 AM <Tristram.Ha@microchip.com> wrote: > > From: Tristram Ha <Tristram.Ha@microchip.com> > > Add KSZ9477 I2C driver support. The code ksz9477.c and ksz_common.c are > used together to generate the I2C driver. > > Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com> > --- > drivers/net/dsa/microchip/Kconfig | 6 + > drivers/net/dsa/microchip/Makefile | 1 + > drivers/net/dsa/microchip/ksz9477_i2c.c | 194 ++++++++++++++++++++++++++++++++ > drivers/net/dsa/microchip/ksz_i2c.h | 69 ++++++++++++ > 4 files changed, 270 insertions(+) > create mode 100644 drivers/net/dsa/microchip/ksz9477_i2c.c > create mode 100644 drivers/net/dsa/microchip/ksz_i2c.h > > diff --git a/drivers/net/dsa/microchip/Kconfig b/drivers/net/dsa/microchip/Kconfig > index bea29fd..fd94441 100644 > --- a/drivers/net/dsa/microchip/Kconfig > +++ b/drivers/net/dsa/microchip/Kconfig > @@ -9,6 +9,12 @@ menuconfig NET_DSA_MICROCHIP_KSZ9477 > help > This driver adds support for Microchip KSZ9477 switch chips. > > +config NET_DSA_MICROCHIP_KSZ9477_I2C > + tristate "KSZ9477 series I2C connected switch driver" > + depends on NET_DSA_MICROCHIP_KSZ9477 && I2C > + help > + Select to enable support for registering switches configured through I2C. > + > config NET_DSA_MICROCHIP_KSZ9477_SPI > tristate "KSZ9477 series SPI connected switch driver" > depends on NET_DSA_MICROCHIP_KSZ9477 && SPI > diff --git a/drivers/net/dsa/microchip/Makefile b/drivers/net/dsa/microchip/Makefile > index 3142c18..dbcc5db 100644 > --- a/drivers/net/dsa/microchip/Makefile > +++ b/drivers/net/dsa/microchip/Makefile > @@ -1,3 +1,4 @@ > obj-$(CONFIG_NET_DSA_MICROCHIP_KSZ_COMMON) += ksz_common.o > obj-$(CONFIG_NET_DSA_MICROCHIP_KSZ9477) += ksz9477.o > +obj-$(CONFIG_NET_DSA_MICROCHIP_KSZ9477_I2C) += ksz9477_i2c.o > obj-$(CONFIG_NET_DSA_MICROCHIP_KSZ9477_SPI) += ksz9477_spi.o > diff --git a/drivers/net/dsa/microchip/ksz9477_i2c.c b/drivers/net/dsa/microchip/ksz9477_i2c.c > new file mode 100644 > index 0000000..63d2f57 > --- /dev/null > +++ b/drivers/net/dsa/microchip/ksz9477_i2c.c > @@ -0,0 +1,194 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Microchip KSZ9477 series register access through I2C > + * > + * Copyright (C) 2018 Microchip Technology Inc. > + */ > + > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/i2c.h> > + > +#include "ksz_priv.h" > +#include "ksz_i2c.h" > + > +/* Enough to read all switch port registers. */ > +#define I2C_TX_BUF_LEN 0x100 > + > +static int ksz9477_i2c_read_reg(struct i2c_client *i2c, u32 reg, u8 *val, > + unsigned int len) > +{ > + struct i2c_msg msg[2]; > + int ret = 0; > + > + val[0] = (u8)(reg >> 8); > + val[1] = (u8)reg; > + > + msg[0].addr = i2c->addr; > + msg[0].flags = 0; > + msg[0].len = 2; > + msg[0].buf = val; > + > + msg[1].addr = i2c->addr; > + msg[1].flags = I2C_M_RD; > + msg[1].len = len; > + msg[1].buf = &val[2]; > + > + if (i2c_transfer(i2c->adapter, msg, 2) != 2) > + ret = -ENODEV; Should this be -EREMOTEIO instead? > + return ret; > +} > + > +static int ksz9477_i2c_write_reg(struct i2c_client *i2c, u32 reg, u8 *val, > + unsigned int len) > +{ > + struct i2c_msg msg; > + int ret = 0; > + > + val[0] = (u8)(reg >> 8); > + val[1] = (u8)reg; > + > + msg.addr = i2c->addr; > + msg.flags = 0; > + msg.len = 2 + len; > + msg.buf = val; > + > + if (i2c_transfer(i2c->adapter, &msg, 1) != 1) > + ret = -ENODEV; Should this be -EREMOTEIO instead? > + return ret; > +} > + > +static int ksz_i2c_read(struct ksz_device *dev, u32 reg, u8 *data, > + unsigned int len) > +{ > + struct i2c_client *i2c = dev->priv; > + int ret; > + > + ret = ksz9477_i2c_read_reg(i2c, reg, dev->txbuf, len); > + if (!ret) > + memcpy(data, &dev->txbuf[2], len); > + return ret; > +} > + > +static int ksz_i2c_write(struct ksz_device *dev, u32 reg, void *data, > + unsigned int len) > +{ > + struct i2c_client *i2c = dev->priv; > + > + if (len > I2C_TX_BUF_LEN) > + len = I2C_TX_BUF_LEN; > + memcpy(&dev->txbuf[2], data, len); > + return ksz9477_i2c_write_reg(i2c, reg, dev->txbuf, len); > +} > + > +static int ksz_i2c_read24(struct ksz_device *dev, u32 reg, u32 *val) > +{ > + int ret; > + > + *val = 0; > + ret = ksz_i2c_read(dev, reg, (u8 *)val, 3); > + if (!ret) { > + *val = be32_to_cpu(*val); > + /* convert to 24bit */ > + *val >>= 8; > + } > + > + return ret; > +} > + > +static int ksz_i2c_write24(struct ksz_device *dev, u32 reg, u32 value) > +{ > + /* make it to big endian 24bit from MSB */ > + value <<= 8; > + value = cpu_to_be32(value); > + return ksz_i2c_write(dev, reg, &value, 3); > +} > + > +static const struct ksz_io_ops ksz9477_i2c_ops = { > + .read8 = ksz_i2c_read8, > + .read16 = ksz_i2c_read16, > + .read24 = ksz_i2c_read24, > + .read32 = ksz_i2c_read32, > + .write8 = ksz_i2c_write8, > + .write16 = ksz_i2c_write16, > + .write24 = ksz_i2c_write24, > + .write32 = ksz_i2c_write32, > + .get = ksz_i2c_get, > + .set = ksz_i2c_set, > +}; > + > +static int ksz9477_i2c_probe(struct i2c_client *i2c, > + const struct i2c_device_id *i2c_id) > +{ > + struct ksz_device *dev; > + int ret; > + > + dev = ksz_switch_alloc(&i2c->dev, &ksz9477_i2c_ops, i2c); > + if (!dev) > + return -ENOMEM; > + > + if (i2c->dev.platform_data) > + dev->pdata = i2c->dev.platform_data; > + > + dev->txbuf = devm_kzalloc(dev->dev, 2 + I2C_TX_BUF_LEN, GFP_KERNEL); > + > + ret = ksz9477_switch_register(dev); > + > + /* Main DSA driver may not be started yet. */ > + if (ret) > + return ret; > + > + i2c_set_clientdata(i2c, dev); > + > + return 0; > +} > + > +static int ksz9477_i2c_remove(struct i2c_client *i2c) > +{ > + struct ksz_device *dev = i2c_get_clientdata(i2c); > + > + if (dev) > + ksz_switch_remove(dev); > + > + return 0; > +} > + > +static void ksz9477_i2c_shutdown(struct i2c_client *i2c) > +{ > + struct ksz_device *dev = i2c_get_clientdata(i2c); > + > + if (dev && dev->dev_ops->shutdown) > + dev->dev_ops->shutdown(dev); > +} > + > +static const struct i2c_device_id ksz9477_i2c_id[] = { > + { "ksz9477-switch", 0 }, > + {}, > +}; > + > +MODULE_DEVICE_TABLE(i2c, ksz9477_i2c_id); > + > +static const struct of_device_id ksz9477_dt_ids[] = { > + { .compatible = "microchip,ksz9477" }, > + { .compatible = "microchip,ksz9897" }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, ksz9477_dt_ids); > + > +static struct i2c_driver ksz9477_i2c_driver = { > + .driver = { > + .name = "ksz9477-switch", > + .owner = THIS_MODULE, > + .of_match_table = of_match_ptr(ksz9477_dt_ids), > + }, > + .probe = ksz9477_i2c_probe, > + .remove = ksz9477_i2c_remove, > + .shutdown = ksz9477_i2c_shutdown, > + .id_table = ksz9477_i2c_id, > +}; > + > +module_i2c_driver(ksz9477_i2c_driver); > + > +MODULE_AUTHOR("Woojung Huh <Woojung.Huh@microchip.com>"); > +MODULE_DESCRIPTION("Microchip KSZ9477 Series Switch I2C access Driver"); > +MODULE_LICENSE("GPL"); > diff --git a/drivers/net/dsa/microchip/ksz_i2c.h b/drivers/net/dsa/microchip/ksz_i2c.h > new file mode 100644 > index 0000000..b9af0a8 > --- /dev/null > +++ b/drivers/net/dsa/microchip/ksz_i2c.h > @@ -0,0 +1,69 @@ > +/* SPDX-License-Identifier: GPL-2.0 > + * Microchip KSZ series I2C access common header > + * > + * Copyright (C) 2018 Microchip Technology Inc. > + * Tristram Ha <Tristram.Ha@microchip.com> > + */ > + > +#ifndef __KSZ_I2C_H > +#define __KSZ_I2C_H > + > +/* Chip dependent I2C access */ > +static int ksz_i2c_read(struct ksz_device *dev, u32 reg, u8 *data, > + unsigned int len); > +static int ksz_i2c_write(struct ksz_device *dev, u32 reg, void *data, > + unsigned int len); > + > +static int ksz_i2c_read8(struct ksz_device *dev, u32 reg, u8 *val) > +{ > + return ksz_i2c_read(dev, reg, val, 1); > +} > + > +static int ksz_i2c_read16(struct ksz_device *dev, u32 reg, u16 *val) > +{ > + int ret = ksz_i2c_read(dev, reg, (u8 *)val, 2); > + > + if (!ret) > + *val = be16_to_cpu(*val); > + > + return ret; > +} > + > +static int ksz_i2c_read32(struct ksz_device *dev, u32 reg, u32 *val) > +{ > + int ret = ksz_i2c_read(dev, reg, (u8 *)val, 4); > + > + if (!ret) > + *val = be32_to_cpu(*val); > + > + return ret; > +} > + > +static int ksz_i2c_write8(struct ksz_device *dev, u32 reg, u8 value) > +{ > + return ksz_i2c_write(dev, reg, &value, 1); > +} > + > +static int ksz_i2c_write16(struct ksz_device *dev, u32 reg, u16 value) > +{ > + value = cpu_to_be16(value); > + return ksz_i2c_write(dev, reg, &value, 2); > +} > + > +static int ksz_i2c_write32(struct ksz_device *dev, u32 reg, u32 value) > +{ > + value = cpu_to_be32(value); > + return ksz_i2c_write(dev, reg, &value, 4); > +} > + > +static int ksz_i2c_get(struct ksz_device *dev, u32 reg, void *data, size_t len) > +{ > + return ksz_i2c_read(dev, reg, data, len); > +} > + > +static int ksz_i2c_set(struct ksz_device *dev, u32 reg, void *data, size_t len) > +{ > + return ksz_i2c_write(dev, reg, data, len); > +} > + > +#endif > -- > 1.9.1 > Best regards, Sergio Paracuellos ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH RFC net-next] net: dsa: microchip: add KSZ9477 I2C driver 2018-12-18 6:24 ` Sergio Paracuellos @ 2018-12-18 10:01 ` Andrew Lunn 2018-12-18 14:49 ` Marek Vasut 0 siblings, 1 reply; 10+ messages in thread From: Andrew Lunn @ 2018-12-18 10:01 UTC (permalink / raw) To: Sergio Paracuellos Cc: Tristram.Ha, Florian Fainelli, Pavel Machek, Marek Vasut, Dan Carpenter, vivien.didelot, UNGLinuxDriver, netdev On Tue, Dec 18, 2018 at 07:24:52AM +0100, Sergio Paracuellos wrote: > > +static int ksz9477_i2c_read_reg(struct i2c_client *i2c, u32 reg, u8 *val, > > + unsigned int len) > > +{ > > + struct i2c_msg msg[2]; > > + int ret = 0; > > + > > + val[0] = (u8)(reg >> 8); > > + val[1] = (u8)reg; > > + > > + msg[0].addr = i2c->addr; > > + msg[0].flags = 0; > > + msg[0].len = 2; > > + msg[0].buf = val; > > + > > + msg[1].addr = i2c->addr; > > + msg[1].flags = I2C_M_RD; > > + msg[1].len = len; > > + msg[1].buf = &val[2]; > > + > > + if (i2c_transfer(i2c->adapter, msg, 2) != 2) > > + ret = -ENODEV; > > Should this be -EREMOTEIO instead? It should actually be whatever error code i2c_transfer returns. > > +static int ksz9477_i2c_write_reg(struct i2c_client *i2c, u32 reg, u8 *val, > > + unsigned int len) > > +{ > > + struct i2c_msg msg; > > + int ret = 0; > > + > > + val[0] = (u8)(reg >> 8); > > + val[1] = (u8)reg; > > + > > + msg.addr = i2c->addr; > > + msg.flags = 0; > > + msg.len = 2 + len; > > + msg.buf = val; > > + > > + if (i2c_transfer(i2c->adapter, &msg, 1) != 1) > > + ret = -ENODEV; > > > Should this be -EREMOTEIO instead? And the same here. > > +MODULE_AUTHOR("Woojung Huh <Woojung.Huh@microchip.com>"); > > +MODULE_DESCRIPTION("Microchip KSZ9477 Series Switch I2C access Driver"); > > +MODULE_LICENSE("GPL"); > > diff --git a/drivers/net/dsa/microchip/ksz_i2c.h b/drivers/net/dsa/microchip/ksz_i2c.h > > new file mode 100644 > > index 0000000..b9af0a8 > > --- /dev/null > > +++ b/drivers/net/dsa/microchip/ksz_i2c.h > > @@ -0,0 +1,69 @@ > > +/* SPDX-License-Identifier: GPL-2.0 This is i think inconsistent. MODULE_LICENSE("GPL") means GPL 2 and at your choice, future versions. Your SPDX is for v2 only. Andrew ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH RFC net-next] net: dsa: microchip: add KSZ9477 I2C driver 2018-12-18 10:01 ` Andrew Lunn @ 2018-12-18 14:49 ` Marek Vasut 0 siblings, 0 replies; 10+ messages in thread From: Marek Vasut @ 2018-12-18 14:49 UTC (permalink / raw) To: Andrew Lunn, Sergio Paracuellos Cc: Tristram.Ha, Florian Fainelli, Pavel Machek, Dan Carpenter, vivien.didelot, UNGLinuxDriver, netdev On 12/18/2018 11:01 AM, Andrew Lunn wrote: > On Tue, Dec 18, 2018 at 07:24:52AM +0100, Sergio Paracuellos wrote: >>> +static int ksz9477_i2c_read_reg(struct i2c_client *i2c, u32 reg, u8 *val, >>> + unsigned int len) >>> +{ >>> + struct i2c_msg msg[2]; >>> + int ret = 0; >>> + >>> + val[0] = (u8)(reg >> 8); >>> + val[1] = (u8)reg; >>> + >>> + msg[0].addr = i2c->addr; >>> + msg[0].flags = 0; >>> + msg[0].len = 2; >>> + msg[0].buf = val; >>> + >>> + msg[1].addr = i2c->addr; >>> + msg[1].flags = I2C_M_RD; >>> + msg[1].len = len; >>> + msg[1].buf = &val[2]; >>> + >>> + if (i2c_transfer(i2c->adapter, msg, 2) != 2) >>> + ret = -ENODEV; >> >> Should this be -EREMOTEIO instead? > > It should actually be whatever error code i2c_transfer returns. Before we plunge into coding yet another I2C-and-SPI layer, can we use regmap instead ? -- Best regards, Marek Vasut ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH RFC net-next] net: dsa: microchip: add KSZ9477 I2C driver 2018-12-18 1:39 [PATCH RFC net-next] net: dsa: microchip: add KSZ9477 I2C driver Tristram.Ha 2018-12-18 1:40 ` Tristram.Ha @ 2018-12-18 8:06 ` Dan Carpenter 2018-12-18 8:54 ` Sergio Paracuellos 1 sibling, 1 reply; 10+ messages in thread From: Dan Carpenter @ 2018-12-18 8:06 UTC (permalink / raw) To: Tristram.Ha Cc: Sergio Paracuellos, Andrew Lunn, Florian Fainelli, Pavel Machek, Marek Vasut, vivien.didelot, UNGLinuxDriver, netdev Do we still need Sergio's device tree patch? regards, dan carpenter ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH RFC net-next] net: dsa: microchip: add KSZ9477 I2C driver 2018-12-18 8:06 ` Dan Carpenter @ 2018-12-18 8:54 ` Sergio Paracuellos 0 siblings, 0 replies; 10+ messages in thread From: Sergio Paracuellos @ 2018-12-18 8:54 UTC (permalink / raw) To: Dan Carpenter Cc: Tristram.Ha, Andrew Lunn, Florian Fainelli, Pavel Machek, Marek Vasut, vivien.didelot, UNGLinuxDriver, netdev On Tue, Dec 18, 2018 at 9:06 AM Dan Carpenter <dan.carpenter@oracle.com> wrote: > > Do we still need Sergio's device tree patch? I think is useful. If not at least add a clarification that the reg "5f" for the i2c is the one to use. > > regards, > dan carpenter > Thanks, Sergio Paracuellos ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-12-18 14:49 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-12-18 1:39 [PATCH RFC net-next] net: dsa: microchip: add KSZ9477 I2C driver Tristram.Ha 2018-12-18 1:40 ` Tristram.Ha 2018-12-18 2:55 ` Woojung.Huh 2018-12-18 4:16 ` Tristram.Ha 2018-12-18 6:30 ` Sergio Paracuellos 2018-12-18 6:24 ` Sergio Paracuellos 2018-12-18 10:01 ` Andrew Lunn 2018-12-18 14:49 ` Marek Vasut 2018-12-18 8:06 ` Dan Carpenter 2018-12-18 8:54 ` Sergio Paracuellos
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).