gpio-mcp23s08 driver with multiple chips

gpio-mcp23s08 driver with multiple chips

[Nathan Williams]

Hi,

I'm using the GPIO driver gpio-mcp23s08 for two MCP23S17 chips that use
the same SPI chip select (and different addresses).

My device tree contains:

&spi0 {
	status = "okay";
	bus-num = <0>;
	num-cs = <2>;
	cs-gpios =
		<&portc 2 0>,
		<&portc 3 0>;

	gpio at 1 {
	        compatible = "microchip,mcp23s17";
		gpio-controller;
		#gpio-cells = <2>;
		microchip,spi-present-mask = <0x03>;
		reg = <1>;
		spi-max-frequency = <1000000>;
	};
};

Both gpiochips are registered and I can see them in /sys/class/gpio as
gpiochip395 and gpiochip411.

My problem is that I can't figure out which gpiochip is which. They both
have the same label, ngpio and share the same of_node.

Setting some GPIOs and probing the hardware helped me determine that
gpiochip411 was the chip with addr = 0 and gpiochip395 was the chip with
addr = 1. However I presume there is no guarantee on what base number is
allocated to each chip?

In fact, if I repeatedly unload and reload the driver each time the base
numbers decrease until I eventually get the following warning:

[ 3376.068639] gpiochip_find_base: cannot find free range
[ 3376.073766] gpiochip_add_data: GPIOs 0..15 (mcp23s17) failed to register
[ 3376.080474] ------------[ cut here ]------------
[ 3376.085105] WARNING: CPU: 1 PID: 807 at drivers/base/core.c:251 device_release+0x8c/0x90
[ 3376.093159] Device 'gpiochip29' does not have a release() function, it is broken and must be fixed.
[ 3376.102174] Modules linked in: gpio_mcp23s08(+) [last unloaded: gpio_mcp23s08]
[ 3376.109420] CPU: 1 PID: 807 Comm: modprobe Not tainted 4.7.0-rc2 #10
[ 3376.115744] Hardware name: Altera SOCFPGA
[ 3376.119766] [<c010f780>] (unwind_backtrace) from [<c010b864>] (show_stack+0x10/0x14)
[ 3376.127487] [<c010b864>] (show_stack) from [<c02c20f8>] (dump_stack+0x8c/0xa0)
[ 3376.134685] [<c02c20f8>] (dump_stack) from [<c011c434>] (__warn+0xec/0x104)
[ 3376.141619] [<c011c434>] (__warn) from [<c011c484>] (warn_slowpath_fmt+0x38/0x48)
[ 3376.149074] [<c011c484>] (warn_slowpath_fmt) from [<c0324798>] (device_release+0x8c/0x90)
[ 3376.157223] [<c0324798>] (device_release) from [<c02c40e0>] (kobject_put+0xb4/0xec)
[ 3376.164858] [<c02c40e0>] (kobject_put) from [<bf04ef74>] (mcp23s08_probe+0x2c0/0x318 [gpio_mcp23s08])
[ 3376.174055] [<bf04ef74>] (mcp23s08_probe [gpio_mcp23s08]) from [<c035b7c8>] (spi_drv_probe+0x7c/0xa8)
[ 3376.183245] [<c035b7c8>] (spi_drv_probe) from [<c0328be0>] (driver_probe_device+0x224/0x2bc)
[ 3376.191653] [<c0328be0>] (driver_probe_device) from [<c0328d30>] (__driver_attach+0xb8/0xbc)
[ 3376.200057] [<c0328d30>] (__driver_attach) from [<c0326f18>] (bus_for_each_dev+0x68/0x9c)
[ 3376.208203] [<c0326f18>] (bus_for_each_dev) from [<c0328068>] (bus_add_driver+0x1a4/0x21c)
[ 3376.216437] [<c0328068>] (bus_add_driver) from [<c0329694>] (driver_register+0x78/0xf8)
[ 3376.224414] [<c0329694>] (driver_register) from [<bf052014>] (init_module+0x14/0x50 [gpio_mcp23s08])
[ 3376.233513] [<bf052014>] (init_module [gpio_mcp23s08]) from [<c0101780>] (do_one_initcall+0x40/0x174)
[ 3376.242698] [<c0101780>] (do_one_initcall) from [<c01c9c50>] (do_init_module+0x64/0x394)
[ 3376.250763] [<c01c9c50>] (do_init_module) from [<c018ceb4>] (load_module+0x1a60/0x206c)
[ 3376.258736] [<c018ceb4>] (load_module) from [<c018d60c>] (SyS_init_module+0x14c/0x15c)
[ 3376.266624] [<c018d60c>] (SyS_init_module) from [<c0107680>] (ret_fast_syscall+0x0/0x3c)
[ 3376.274700] ---[ end trace 3a4ba228e00948b3 ]---
[ 3376.279316] mcp23s08: probe of spi0.1 failed with error -28

Ultimately I'd like to be able to use gpio-line-names in the device tree
to name each of the GPIOs I'll be using in user space. To support
multiple chips on the same SPI chip select I came up with this patch:

--- a/drivers/gpio/gpio-mcp23s08.c
+++ b/drivers/gpio/gpio-mcp23s08.c
@@ -552,6 +552,7 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
 {
 	int status;
 	bool mirror = false;
+	struct device_node *np;
 
 	mutex_init(&mcp->lock);
 
@@ -566,7 +567,24 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
 	mcp->chip.dbg_show = mcp23s08_dbg_show;
 #ifdef CONFIG_OF
 	mcp->chip.of_gpio_n_cells = 2;
-	mcp->chip.of_node = dev->of_node;
+	for_each_child_of_node(dev->of_node, np) {
+		const __be32 *reg;
+		int len;
+		int a_cells, s_cells;
+
+		reg = of_get_property(np, "reg", &len);
+		if (!reg)
+			continue;
+
+		a_cells = of_n_addr_cells(np);
+		s_cells = of_n_size_cells(np);
+
+		if (of_read_number(reg, a_cells) == cs) {
+			mcp->chip.of_node = np;
+		}
+	}
+	if (!mcp->chip.of_node)
+		mcp->chip.of_node = dev->of_node;
 #endif
 
 	switch (type) {

This allows me to use something like this in my device tree:

	gpio at 1 {
	        compatible = "microchip,mcp23s17";
		gpio-controller;
		#gpio-cells = <2>;
		microchip,spi-present-mask = <0x03>;
		reg = <1>;
		spi-max-frequency = <1000000>;

		chip at 0 {
			reg = <0>;
			gpio-line-names =
				"RLY_CTRL3", /* GPA0 */
				...

		chip at 1 {
			reg = <1>;
			gpio-line-names =
				"USBA_PWR", /* GPA0 */
				...

Which displays the names correctly with lsgpio, however I'm still stuck
on how to figure out which base number to use when I want to export one
of these GPIOs.

Regards,
Nathan