[PATCHv2 0/2] Introduce mcp230xx support

[PATCHv2 0/2] Introduce mcp230xx support

[Sebastian Reichel]

Hi,

Add driver for mcp230xx devices, which are quite common.

Changes since PATCHv1:
 * add new patch adding dm_i2c_reg_clrset() as suggested
   by Simon Glass
 * Use | instead of + to build device register
   (resulting number is the same)
 * Properly handle GPIOD_IS_OUT_ACTIVE flag
 * Use bool for local variables in mcp230xx_set_flags()
 * Add Reviewed-by from Simon Glass

-- Sebastian

Sebastian Reichel (2):
  i2c: add dm_i2c_reg_clrset
  gpio: mcp230xx: Introduce new driver

 drivers/gpio/Kconfig         |  10 ++
 drivers/gpio/Makefile        |   1 +
 drivers/gpio/mcp230xx_gpio.c | 235 +++++++++++++++++++++++++++++++++++
 drivers/i2c/i2c-uclass.c     |  15 +++
 include/i2c.h                |  14 +++
 test/dm/i2c.c                |  29 +++++
 6 files changed, 304 insertions(+)
 create mode 100644 drivers/gpio/mcp230xx_gpio.c

-- 
2.30.2

[PATCHv2 1/2] i2c: add dm_i2c_reg_clrset

[Sebastian Reichel]

Add function to apply a bitmask to an i2c register, so
that specific bits can be cleared and/or set.

Suggested-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/i2c/i2c-uclass.c | 15 +++++++++++++++
 include/i2c.h            | 14 ++++++++++++++
 test/dm/i2c.c            | 29 +++++++++++++++++++++++++++++
 3 files changed, 58 insertions(+)

diff --git a/drivers/i2c/i2c-uclass.c b/drivers/i2c/i2c-uclass.c
index be56785217c3..8adcdaf86411 100644
--- a/drivers/i2c/i2c-uclass.c
+++ b/drivers/i2c/i2c-uclass.c
@@ -245,6 +245,21 @@ int dm_i2c_reg_write(struct udevice *dev, uint offset, uint value)
 	return dm_i2c_write(dev, offset, &val, 1);
 }
 
+int dm_i2c_reg_clrset(struct udevice *dev, uint offset, u32 clr, u32 set)
+{
+	uint8_t val;
+	int ret;
+
+	ret = dm_i2c_read(dev, offset, &val, 1);
+	if (ret < 0)
+		return ret;
+
+	val &= ~clr;
+	val |= set;
+
+	return dm_i2c_write(dev, offset, &val, 1);
+}
+
 /**
  * i2c_probe_chip() - probe for a chip on a bus
  *
diff --git a/include/i2c.h b/include/i2c.h
index c0fe94c1f333..4fa2222a15ae 100644
--- a/include/i2c.h
+++ b/include/i2c.h
@@ -242,6 +242,20 @@ int dm_i2c_reg_read(struct udevice *dev, uint offset);
  */
 int dm_i2c_reg_write(struct udevice *dev, uint offset, unsigned int val);
 
+/**
+ * dm_i2c_reg_clrset() - Apply bitmask to an I2C register
+ *
+ * Read value, apply bitmask and write modified value back to the
+ * given address in an I2C chip
+ *
+ * @dev:	Device to use for transfer
+ * @offset:	Address for the R/W operation
+ * @clr:	Bitmask of bits that should be cleared
+ * @set:	Bitmask of bits that should be set
+ * @return 0 on success, -ve on error
+ */
+int dm_i2c_reg_clrset(struct udevice *dev, uint offset, u32 clr, u32 set);
+
 /**
  * dm_i2c_xfer() - Transfer messages over I2C
  *
diff --git a/test/dm/i2c.c b/test/dm/i2c.c
index d74f5f9fbc7c..74b209719560 100644
--- a/test/dm/i2c.c
+++ b/test/dm/i2c.c
@@ -304,3 +304,32 @@ static int dm_test_i2c_addr_offset(struct unit_test_state *uts)
 }
 
 DM_TEST(dm_test_i2c_addr_offset, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
+
+static int dm_test_i2c_reg_clrset(struct unit_test_state *uts)
+{
+	struct udevice *eeprom;
+	struct udevice *dev;
+	u8 buf[5];
+
+	ut_assertok(i2c_get_chip_for_busnum(busnum, chip, 1, &dev));
+
+	/* Do a transfer so we can find the emulator */
+	ut_assertok(dm_i2c_read(dev, 0, buf, 5));
+	ut_assertok(uclass_first_device(UCLASS_I2C_EMUL, &eeprom));
+
+	/* Dummy data for the test */
+	ut_assertok(dm_i2c_write(dev, 0, "\xff\x00\xff\x00\x10", 5));
+
+	/* Do some clrset tests */
+	ut_assertok(dm_i2c_reg_clrset(dev, 0, 0xff, 0x10));
+	ut_assertok(dm_i2c_reg_clrset(dev, 1, 0x00, 0x11));
+	ut_assertok(dm_i2c_reg_clrset(dev, 2, 0xed, 0x00));
+	ut_assertok(dm_i2c_reg_clrset(dev, 3, 0xff, 0x13));
+	ut_assertok(dm_i2c_reg_clrset(dev, 4, 0x00, 0x14));
+
+	ut_assertok(dm_i2c_read(dev, 0, buf, 5));
+	ut_asserteq_mem("\x10\x11\x12\x13\x14", buf, sizeof(buf));
+
+	return 0;
+}
+DM_TEST(dm_test_i2c_reg_clrset, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
-- 
2.30.2

[PATCHv2 2/2] gpio: mcp230xx: Introduce new driver

[Sebastian Reichel]

Introduce driver for I2C based MCP230xx GPIO chips, which are
quite common and already well supported by the Linux kernel.

Reviewed-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/gpio/Kconfig         |  10 ++
 drivers/gpio/Makefile        |   1 +
 drivers/gpio/mcp230xx_gpio.c | 235 +++++++++++++++++++++++++++++++++++
 3 files changed, 246 insertions(+)
 create mode 100644 drivers/gpio/mcp230xx_gpio.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index de4dc51d4b48..f93e736639bb 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -179,6 +179,16 @@ config LPC32XX_GPIO
 	help
 	  Support for the LPC32XX GPIO driver.
 
+config MCP230XX_GPIO
+	bool "MCP230XX GPIO driver"
+	depends on DM
+	help
+	  Support for Microchip's MCP230XX I2C connected GPIO devices.
+	  The following chips are supported:
+	   - MCP23008
+	   - MCP23017
+	   - MCP23018
+
 config MSCC_SGPIO
 	bool "Microsemi Serial GPIO driver"
 	depends on DM_GPIO && SOC_VCOREIII
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 8541ba0b0aec..00ffcc753534 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -24,6 +24,7 @@ obj-$(CONFIG_KIRKWOOD_GPIO)	+= kw_gpio.o
 obj-$(CONFIG_KONA_GPIO)	+= kona_gpio.o
 obj-$(CONFIG_MARVELL_GPIO)	+= mvgpio.o
 obj-$(CONFIG_MARVELL_MFP)	+= mvmfp.o
+obj-$(CONFIG_MCP230XX_GPIO)	+= mcp230xx_gpio.o
 obj-$(CONFIG_MXC_GPIO)	+= mxc_gpio.o
 obj-$(CONFIG_MXS_GPIO)	+= mxs_gpio.o
 obj-$(CONFIG_PCA953X)		+= pca953x.o
diff --git a/drivers/gpio/mcp230xx_gpio.c b/drivers/gpio/mcp230xx_gpio.c
new file mode 100644
index 000000000000..9f02fd42b35f
--- /dev/null
+++ b/drivers/gpio/mcp230xx_gpio.c
@@ -0,0 +1,235 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2021, Collabora Ltd.
+ * Copyright (C) 2021, General Electric Company
+ * Author(s): Sebastian Reichel <sebastian.reichel@collabora.com>
+ */
+
+#define LOG_CATEGORY UCLASS_GPIO
+
+#include <common.h>
+#include <errno.h>
+#include <dm.h>
+#include <i2c.h>
+#include <asm/gpio.h>
+#include <dm/device_compat.h>
+#include <dt-bindings/gpio/gpio.h>
+
+enum mcp230xx_type {
+	UNKNOWN = 0,
+	MCP23008,
+	MCP23017,
+	MCP23018,
+};
+
+#define MCP230XX_IODIR 0x00
+#define MCP230XX_GPPU 0x06
+#define MCP230XX_GPIO 0x09
+#define MCP230XX_OLAT 0x0a
+
+#define BANKSIZE 8
+
+static int mcp230xx_read(struct udevice *dev, uint reg, uint offset)
+{
+	struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev);
+	int bank = offset / BANKSIZE;
+	int mask = 1 << (offset % BANKSIZE);
+	int shift = (uc_priv->gpio_count / BANKSIZE) - 1;
+	int ret;
+
+	ret = dm_i2c_reg_read(dev, (reg << shift) | bank);
+	if (ret < 0)
+		return ret;
+
+	return !!(ret & mask);
+}
+
+static int mcp230xx_write(struct udevice *dev, uint reg, uint offset, bool val)
+{
+	struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev);
+	int bank = offset / BANKSIZE;
+	int mask = 1 << (offset % BANKSIZE);
+	int shift = (uc_priv->gpio_count / BANKSIZE) - 1;
+
+	return dm_i2c_reg_clrset(dev, (reg << shift) | bank, mask, val ? mask : 0);
+}
+
+static int mcp230xx_get_value(struct udevice *dev, uint offset)
+{
+	int ret;
+
+	ret = mcp230xx_read(dev, MCP230XX_GPIO, offset);
+	if (ret < 0) {
+		dev_err(dev, "%s error: %d\n", __func__, ret);
+		return ret;
+	}
+
+	return ret;
+}
+
+static int mcp230xx_set_value(struct udevice *dev, uint offset, int val)
+{
+	int ret;
+
+	ret = mcp230xx_write(dev, MCP230XX_GPIO, offset, val);
+	if (ret < 0) {
+		dev_err(dev, "%s error: %d\n", __func__, ret);
+		return ret;
+	}
+
+	return ret;
+}
+
+static int mcp230xx_get_flags(struct udevice *dev, unsigned int offset,
+				  ulong *flags)
+{
+	int direction, pullup;
+
+	pullup = mcp230xx_read(dev, MCP230XX_GPPU, offset);
+	if (pullup < 0) {
+		dev_err(dev, "%s error: %d\n", __func__, pullup);
+		return pullup;
+	}
+
+	direction = mcp230xx_read(dev, MCP230XX_IODIR, offset);
+	if (direction < 0) {
+		dev_err(dev, "%s error: %d\n", __func__, direction);
+		return direction;
+	}
+
+	*flags = direction ? GPIOD_IS_IN : GPIOD_IS_OUT;
+
+	if (pullup)
+		*flags |= GPIOD_PULL_UP;
+
+	return 0;
+}
+
+static int mcp230xx_set_flags(struct udevice *dev, uint offset, ulong flags)
+{
+	bool input = !(flags & (GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE));
+	bool pullup = flags & GPIOD_PULL_UP;
+	ulong supported_mask;
+	int ret;
+
+	/* Note: active-low is ignored (handled by core) */
+	supported_mask = GPIOD_ACTIVE_LOW | GPIOD_MASK_DIR | GPIOD_PULL_UP;
+	if (flags & ~supported_mask) {
+		dev_err(dev, "%s unsupported flag(s): %lx\n", __func__, flags);
+		return -EINVAL;
+	}
+
+	ret = mcp230xx_write(dev, MCP230XX_OLAT, offset, !!(flags & GPIOD_IS_OUT_ACTIVE));
+	if (ret) {
+		dev_err(dev, "%s failed to setup output latch: %d\n", __func__, ret);
+		return ret;
+	}
+
+	ret = mcp230xx_write(dev, MCP230XX_GPPU, offset, pullup);
+	if (ret) {
+		dev_err(dev, "%s failed to setup pull-up: %d\n", __func__, ret);
+		return ret;
+	}
+
+	ret = mcp230xx_write(dev, MCP230XX_IODIR, offset, input);
+	if (ret) {
+		dev_err(dev, "%s failed to setup direction: %d\n", __func__, ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int mcp230xx_direction_input(struct udevice *dev, uint offset)
+{
+	return mcp230xx_set_flags(dev, offset, GPIOD_IS_IN);
+}
+
+static int mcp230xx_direction_output(struct udevice *dev, uint offset, int val)
+{
+	int ret = mcp230xx_set_value(dev, offset, val);
+	if (ret < 0) {
+		dev_err(dev, "%s error: %d\n", __func__, ret);
+		return ret;
+	}
+	return mcp230xx_set_flags(dev, offset, GPIOD_IS_OUT);
+}
+
+static int mcp230xx_get_function(struct udevice *dev, uint offset)
+{
+	int ret;
+
+	ret = mcp230xx_read(dev, MCP230XX_IODIR, offset);
+	if (ret < 0) {
+		dev_err(dev, "%s error: %d\n", __func__, ret);
+		return ret;
+	}
+
+	return ret ? GPIOF_INPUT : GPIOF_OUTPUT;
+}
+
+static const struct dm_gpio_ops mcp230xx_ops = {
+	.direction_input	= mcp230xx_direction_input,
+	.direction_output	= mcp230xx_direction_output,
+	.get_value		= mcp230xx_get_value,
+	.set_value		= mcp230xx_set_value,
+	.get_function		= mcp230xx_get_function,
+	.set_flags		= mcp230xx_set_flags,
+	.get_flags		= mcp230xx_get_flags,
+};
+
+static int mcp230xx_probe(struct udevice *dev)
+{
+	struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev);
+	char name[32], label[8], *str;
+	int addr, gpio_count, size;
+	const u8 *tmp;
+
+	switch (dev_get_driver_data(dev)) {
+	case MCP23008:
+		gpio_count = 8;
+		break;
+	case MCP23017:
+	case MCP23018:
+		gpio_count = 16;
+		break;
+	default:
+		return -ENODEV;
+	}
+
+	addr = dev_read_addr(dev);
+	tmp = dev_read_prop(dev, "label", &size);
+	if (tmp) {
+		memcpy(label, tmp, sizeof(label) - 1);
+		label[sizeof(label) - 1] = '\0';
+		snprintf(name, sizeof(name), "%s@%x_", label, addr);
+	} else {
+		snprintf(name, sizeof(name), "gpio@%x_", addr);
+	}
+
+	str = strdup(name);
+	if (!str)
+		return -ENOMEM;
+
+	uc_priv->bank_name = str;
+	uc_priv->gpio_count = gpio_count;
+
+	dev_dbg(dev, "%s is ready\n", str);
+
+	return 0;
+}
+
+static const struct udevice_id mcp230xx_ids[] = {
+	{ .compatible = "microchip,mcp23008", .data = MCP23008, },
+	{ .compatible = "microchip,mcp23017", .data = MCP23017, },
+	{ .compatible = "microchip,mcp23018", .data = MCP23018, },
+	{ }
+};
+
+U_BOOT_DRIVER(mcp230xx) = {
+	.name		= "mcp230xx",
+	.id		= UCLASS_GPIO,
+	.ops		= &mcp230xx_ops,
+	.probe		= mcp230xx_probe,
+	.of_match	= mcp230xx_ids,
+};
-- 
2.30.2

Re: [PATCHv2 1/2] i2c: add dm_i2c_reg_clrset

[Simon Glass]

On Thu, 15 Jul 2021 at 09:40, Sebastian Reichel
<sebastian.reichel@collabora.com> wrote:
>
> Add function to apply a bitmask to an i2c register, so
> that specific bits can be cleared and/or set.
>
> Suggested-by: Simon Glass <sjg@chromium.org>
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
>  drivers/i2c/i2c-uclass.c | 15 +++++++++++++++
>  include/i2c.h            | 14 ++++++++++++++
>  test/dm/i2c.c            | 29 +++++++++++++++++++++++++++++
>  3 files changed, 58 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCHv2 2/2] gpio: mcp230xx: Introduce new driver

[Simon Glass]

Hi Sebastian,

On Thu, 15 Jul 2021 at 09:40, Sebastian Reichel
<sebastian.reichel@collabora.com> wrote:
>
> Introduce driver for I2C based MCP230xx GPIO chips, which are
> quite common and already well supported by the Linux kernel.
>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
>  drivers/gpio/Kconfig         |  10 ++
>  drivers/gpio/Makefile        |   1 +
>  drivers/gpio/mcp230xx_gpio.c | 235 +++++++++++++++++++++++++++++++++++
>  3 files changed, 246 insertions(+)
>  create mode 100644 drivers/gpio/mcp230xx_gpio.c

Reviewed-by: Simon Glass <sjg@chromium.org>

BTW I see there is no change log on this patch. Patman will do this
for you automatically.


- Simon

Re: [PATCHv2 1/2] i2c: add dm_i2c_reg_clrset

[Heiko Schocher]

Hello Sebstian,

On 15.07.21 17:39, Sebastian Reichel wrote:
> Add function to apply a bitmask to an i2c register, so
> that specific bits can be cleared and/or set.
> 
> Suggested-by: Simon Glass <sjg@chromium.org>
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
>  drivers/i2c/i2c-uclass.c | 15 +++++++++++++++
>  include/i2c.h            | 14 ++++++++++++++
>  test/dm/i2c.c            | 29 +++++++++++++++++++++++++++++
>  3 files changed, 58 insertions(+)

Applied to u-boot-i2c.git master

Thanks!

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs@denx.de

Re: [PATCHv2 2/2] gpio: mcp230xx: Introduce new driver

[Heiko Schocher]

Hello Sebstian,

On 15.07.21 17:40, Sebastian Reichel wrote:
> Introduce driver for I2C based MCP230xx GPIO chips, which are
> quite common and already well supported by the Linux kernel.
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
>  drivers/gpio/Kconfig         |  10 ++
>  drivers/gpio/Makefile        |   1 +
>  drivers/gpio/mcp230xx_gpio.c | 235 +++++++++++++++++++++++++++++++++++
>  3 files changed, 246 insertions(+)
>  create mode 100644 drivers/gpio/mcp230xx_gpio.c

Applied to u-boot-i2c.git master

Thanks!

bye,
Heiko

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs@denx.de