Messages in this thread | | | Subject | Re: [PATCH 1/2] mux:adgs1408/1409: New driver for Analog Devices ADGS1408/1409 mux … | From | Peter Rosin <> | Date | Sat, 14 Jul 2018 14:04:08 +0200 |
| |
Hi!
Thanks for your patches!
Please add a space after "mux:" in the subject. And the second part should generally be the driver name and the text should not start with a capital letter. So, I'd like the subject to be:
[...] mux: adgs1408: new driver for Analog Devices ADGS1408/1409 mux
On 2018-07-13 14:27, Mircea Caprioru wrote: > This patch adds basic support for Analog Device ADGS1408/09 SPI mux > controller. > > The device is probed and set to a disabled state. It uses the new mux > controller framework. > > Signed-off-by: Mircea Caprioru <mircea.caprioru@analog.com> > --- > MAINTAINERS | 7 +++ > drivers/mux/Kconfig | 12 ++++ > drivers/mux/Makefile | 2 + > drivers/mux/adgs140x.c | 132 +++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 153 insertions(+) > create mode 100644 drivers/mux/adgs140x.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index 192d7f73fd01..7aa68f38ea4b 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -810,6 +810,13 @@ L: linux-media@vger.kernel.org > S: Maintained > F: drivers/media/i2c/ad9389b* > > +ANALOG DEVICES INC ADGS1408 DRIVER > +M: Mircea Caprioru <mircea.caprioru@analog.com> > +W: http://wiki.analog.com/ > +W: http://ez.analog.com/community/linux-device-drivers > +S: Supported > +F: drivers/mux/adgs140x.c > + > ANALOG DEVICES INC ADV7180 DRIVER > M: Lars-Peter Clausen <lars@metafoo.de> > L: linux-media@vger.kernel.org > diff --git a/drivers/mux/Kconfig b/drivers/mux/Kconfig > index 6241678e99af..87b3fda56d8f 100644 > --- a/drivers/mux/Kconfig > +++ b/drivers/mux/Kconfig > @@ -48,4 +48,16 @@ config MUX_MMIO > To compile the driver as a module, choose M here: the module will > be called mux-mmio. > > +config MUX_ADGS140X
Please name the driver after one of the chips, i.e. without wildcard. And keep the Kconfig entries sorted.
> + tristate "Analog Devices ADGS1408/ADGS1409 Multiplexers" > + depends on SPI > + help > + ADGS1408 and ADGS1409 4 independent single-pole/single-throw > + switches.
That's wrong. ADGS1408 has one SP8T, ADGS1409 has two SPQT (or one DPQT).
> + > + The driver suports driving each switch independently.
supports
> + > + To compile the driver as a module, choose M here: the module will > + be called mux-ads140x.
mux-adgs1408
> + > endmenu > diff --git a/drivers/mux/Makefile b/drivers/mux/Makefile > index c3d883955fd5..236e7738462a 100644 > --- a/drivers/mux/Makefile > +++ b/drivers/mux/Makefile > @@ -5,10 +5,12 @@ > > mux-core-objs := core.o > mux-adg792a-objs := adg792a.o > +mux-adgs140x-objs := adgs140x.o
s/adgs140x/adgs1408/
Make this change throughout, of course. And this comment also applies to the bindings patch.
> mux-gpio-objs := gpio.o > mux-mmio-objs := mmio.o > > obj-$(CONFIG_MULTIPLEXER) += mux-core.o > obj-$(CONFIG_MUX_ADG792A) += mux-adg792a.o > +obj-$(CONFIG_MUX_ADGS140X) += mux-adgs140x.o > obj-$(CONFIG_MUX_GPIO) += mux-gpio.o > obj-$(CONFIG_MUX_MMIO) += mux-mmio.o > diff --git a/drivers/mux/adgs140x.c b/drivers/mux/adgs140x.c > new file mode 100644 > index 000000000000..13dd95acca20 > --- /dev/null > +++ b/drivers/mux/adgs140x.c > @@ -0,0 +1,132 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * ADG1408 SPI MUX driver > + * > + * Copyright 2018 Analog Devices Inc. > + *
Remove the above empty line.
> + */ > + > +#include <linux/err.h> > +#include <linux/of.h> > +#include <linux/module.h> > +#include <linux/mux/driver.h> > +#include <linux/spi/spi.h> > +#include <linux/property.h>
Keep the #includes sorted.
> + > +#define ADGS1408_SW_DATA (0x01) > +#define ADGS1408_REG_READ(reg) ((reg) | 0x80) > +#define ADGS1408_DISABLE (0x00) > +#define ADGS1408_MUX(state) (((state) << 1) | 1) > + > +static int adgs140x_spi_reg_write(struct spi_device *spi, > + u8 reg_addr, u8 reg_data) > +{ > + u8 tx_buf[2]; > + > + tx_buf[0] = reg_addr; > + tx_buf[1] = reg_data; > + > + return spi_write_then_read(spi, tx_buf, sizeof(tx_buf), NULL, 0);
return spi_write(spi, tx_buf, sizeof(tx_buf));
> +} > + > +static int adgs140x_set(struct mux_control *mux, int state) > +{ > + struct spi_device *spi = to_spi_device(mux->chip->dev.parent); > + u8 reg; > + > + if (state == MUX_IDLE_DISCONNECT) > + reg = ADGS1408_DISABLE; > + else > + reg = ADGS1408_MUX(state); > + > + return adgs140x_spi_reg_write(spi, ADGS1408_SW_DATA, reg); > +} > + > +static const struct mux_control_ops adgs140x_ops = { > + .set = adgs140x_set, > +}; > + > +static int adgs140x_probe(struct spi_device *spi) > +{ > + struct device *dev = &spi->dev; > + struct mux_chip *mux_chip; > + u32 idle_state[2]; > + u32 cells; > + int ret; > + int i; > + > + ret = device_property_read_u32(dev, "#mux-control-cells", &cells); > + if (ret < 0) > + return ret; > + > + if (cells >= 2) > + return -EINVAL;
cells should always be zero for ADGS1408.
> + > + mux_chip = devm_mux_chip_alloc(dev, cells ? 2 : 1, 0); > + if (IS_ERR(mux_chip)) > + return PTR_ERR(mux_chip); > + > + mux_chip->ops = &adgs140x_ops; > + > + ret = adgs140x_spi_reg_write(spi, ADGS1408_SW_DATA, ADGS1408_DISABLE); > + if (ret < 0) > + return ret; > + > + ret = device_property_read_u32_array(dev, "idle-state", > + idle_state, > + mux_chip->controllers); > + if (ret < 0) { > + idle_state[0] = MUX_IDLE_AS_IS; > + idle_state[1] = MUX_IDLE_AS_IS; > + } > + > + for (i = 0; i < mux_chip->controllers; ++i) { > + struct mux_control *mux = &mux_chip->mux[i]; > + > + if (mux_chip->controllers == 1) > + mux->states = 8;
8 states is never right for ADGS1409.
> + else > + mux->states = 4;
4 states is never right for ADGS1408.
> + > + switch (idle_state[i]) { > + case MUX_IDLE_DISCONNECT: > + case MUX_IDLE_AS_IS: > + case 0 ... 8:
Hmmm, I realize that you have adapted this 0 ... 8 from the 0 ... 4 in the adg795a driver, but now I fail to see why it isn't 0 ... 3 there. I think the 4 is a left-over from an older encoding of the special idle states.
So, for ADGS1408, 0 ... 7 are valid, and for ADGS1409 it's 0 ... 3.
Cheers, Peter
> + mux->idle_state = idle_state[i]; > + break; > + default: > + dev_err(dev, "invalid idle-state %d\n", idle_state[i]); > + return -EINVAL; > + } > + } > + > + return devm_mux_chip_register(dev, mux_chip); > +} > + > +static const struct spi_device_id adgs140x_id[] = { > + { .name = "adgs1408", }, > + { .name = "adgs1409", }, > + { } > +}; > +MODULE_DEVICE_TABLE(spi, adgs140x_id); > + > +static const struct of_device_id adgs140x_of_match[] = { > + { .compatible = "adi,adgs1408", }, > + { .compatible = "adi,adgs1409", }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, adgs140x_of_match); > + > +static struct spi_driver adgs140x_driver = { > + .driver = { > + .name = "adgs1408", > + .of_match_table = of_match_ptr(adgs140x_of_match), > + }, > + .probe = adgs140x_probe, > + .id_table = adgs140x_id, > +}; > +module_spi_driver(adgs140x_driver); > + > +MODULE_AUTHOR("Mircea Caprioru <mircea.caprioru@analog.com>"); > +MODULE_DESCRIPTION("Analog Devices ADGS140x MUX driver"); > +MODULE_LICENSE("GPL v2"); >
| |