Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update the MCP23017 overlay to support using multiple devices with repeated calls #3207

Closed
Dloranger opened this issue Sep 5, 2019 · 12 comments

Comments

@Dloranger
Copy link

Describe the bug
Currently the overlay only supports 1 chipset, but the device is capable of having 8 on the bus with only an address change (pin setting) and using a new gpio pin for the int pin.

Can we upgrade this dtbo to be able to use the address field to make each call unique to be able to load from 2-8 chipsets with multiple calls to the dtbo?

This would greatly simplify my development workflow by allowing me to use stock dtbo calls rather than having to recompile custom dtbo regularly.

To reproduce
call the mcp23017 overlay 2x, only the last one is used.

Expected behaviour
call the overlay 2-8x and all 2-8 devices would be loaded as unique devices.

Actual behaviour
Only 1 device gets loaded

System
this is an issue that has persisted due to the implementation of the overlay, not specific to any particular version of code or hardware.

@6by9
Copy link
Contributor

6by9 commented Sep 5, 2019

I suspect the issue is the collision in the name mcp23017_pins to describe the interrupt GPIO.
https://github.com/raspberrypi/linux/blob/rpi-4.19.y/arch/arm/boot/dts/overlays/mcp23017-overlay.dts

The interrupt GPIO is optional, so one could remove it and the mcp23017_pins fragment. Making it an overlay option to remove it would require someone with more DT overlay knowledge than me....

@6by9
Copy link
Contributor

6by9 commented Sep 5, 2019

Having said that, I think that falls out easily.

// Definitions for MCP23017 Gpio Extender from Microchip Semiconductor

/dts-v1/;
/plugin/;

/ {
	compatible = "brcm,bcm2835";

	fragment@0 {
		target = <&i2c1>;
		__overlay__ {
			status = "okay";
		};
	};

	fragment@1 {
		target = <&gpio>;
		__overlay__ {
			mcp23017_pins: mcp23017_pins {
				brcm,pins = <4>;
				brcm,function = <0>;
			};
		};
	};

	fragment@2 {
		target = <&i2c1>;
		__overlay__ {
			#address-cells = <1>;
			#size-cells = <0>;

			mcp23017: mcp@20 {
				compatible = "microchip,mcp23017";
				reg = <0x20>;
				gpio-controller;
				#gpio-cells = <2>;

				status = "okay";
			};
		};
	};

	fragment@3 {
		target = <&mcp23017>;
		__dormant__ {
			compatible = "microchip,mcp23008";
		};
	};

	fragment@4 {
		target = <&mcp23017>;
		__overlay__ {
			#interrupt-cells=<2>;
			interrupt-parent = <&gpio>;
			interrupts = <4 2>;
			interrupt-controller;
			microchip,irq-mirror;
		};
	};

	__overrides__ {
		gpiopin = <&mcp23017_pins>,"brcm,pins:0",
				<&mcp23017>,"interrupts:0";
		addr = <&mcp23017>,"reg:0";
		mcp23008 = <0>,"=3";
		noints = <0>,"!1!4";
	};
};

Save that as mcp23017-overlay.dts
Run dtc -@ -I dts -O dtb -o mcp23017.dtbo mcp23017-overlay.dts
sudo cp mcp23017.dtbo /boot/overlays/
Edit your config.txt lines to read dtoverlay=mcp23017,addr=0x20,noints=1 or similar.
Reboot.
Report back (I don't have any mcp23017's to hand, but it seems to set up the DT nodes that I'd expect).

@pelwell
Copy link
Contributor

pelwell commented Sep 5, 2019

There is a better way - renaming the _pins node using the address:

		__overlay__ {
			mcp23017_pins: mcp23017_pins@20 {
				brcm,pins = <4>;
				brcm,function = <0>;
...
		addr = <&mcp23017>,"reg:0", <&mcp23017_pins>,"reg:0";

@6by9
Copy link
Contributor

6by9 commented Sep 5, 2019

Too much magic seeing as mcp23017_pins doesn't have a reg field! :-S

If tweaking the overlay then adding the option of not having to allocate an interrupt line has some uses, eg if you only want it for lots of outputs.

@pelwell
Copy link
Contributor

pelwell commented Sep 5, 2019

Nah - it's just sufficiently advanced technology.

But yes, if the device can function without an interrupt line then we can make it an option.

6by9 added a commit to 6by9/linux that referenced this issue Sep 5, 2019
In order to allow the overlay to be loaded multiple times the
GPIO node for the interrupt line needs to be unique.
Rename it based on the MCP23017 I2C address

raspberrypi#3207

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
@6by9
Copy link
Contributor

6by9 commented Sep 5, 2019

Well I tried with the commits on https://github.com/6by9/linux/tree/3207.
Renaming the mcp23017_pins node works, but having moved the interrupt block out of the fragment 2 the interrupts node doesn't get updated. Leave that bit for tomorrow.

@ghost
Copy link

ghost commented Sep 5, 2019

But yes, if the device can function without an interrupt line then we can make it an option.

The MCP23017 can certainly function without an interrupt. I've got an Adafruit i2c 16x2 RGB LCD Pi Plate that doesn't even have the two interrupt lines connected (which is a bit annoying as you have to poll the buttons).

@Dloranger
Copy link
Author

Dloranger commented Sep 5, 2019 via email

@pelwell
Copy link
Contributor

pelwell commented Sep 6, 2019

I don't have the time to debug an MCP23017 interrupt problem. You can open a separate issue to record the problem, but don't expect it to get much attention for the foreseeable future.

@6by9
Copy link
Contributor

6by9 commented Sep 6, 2019

I was hooking up an MCP23017 to see what it did and confirm that disabling the interrupts works.

Reading that thread, it looks like it's an issue of trying to claim the interrupt for the interrupt GPIO from the MCP23017, rather than registering for interrupts off the individual pins of the MCP23017.
I also note from the readme on https://github.com/mytechguyri/dtmfdecoder/

The interrupt driven version has been merged into the master branch now that interrupts are functioning correctly.

There's messing setting a pullup on the interrupt GPIO pin though. That should be handled by the dtoverlay (or on the external board).

There is a minor difference between the DT example bindings and our config on the int pin.
Our usage is
interrupts = <4 2>; https://github.com/raspberrypi/linux/blob/rpi-4.19.y/arch/arm/boot/dts/overlays/mcp23017-overlay.dts#L39 (2 would appear to be IRQ_TYPE_EDGE_FALLING)
The example:
interrupts = <17 IRQ_TYPE_LEVEL_LOW>; https://github.com/raspberrypi/linux/blob/rpi-4.19.y/Documentation/devicetree/bindings/pinctrl/pinctrl-mcp23s08.txt#L74
Understanding whether that is an issue or not would be some serious datasheet reading.

pelwell pushed a commit that referenced this issue Sep 6, 2019
In order to allow the overlay to be loaded multiple times the
GPIO node for the interrupt line needs to be unique.
Rename it based on the MCP23017 I2C address

#3207

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
@pelwell
Copy link
Contributor

pelwell commented Sep 6, 2019

Fixed by #3211 .

popcornmix added a commit to raspberrypi/firmware that referenced this issue Sep 6, 2019
kernel: overlays: mcp23017: Add option for not connecting the int GPIO
See: raspberrypi/linux#3207
pelwell pushed a commit that referenced this issue Sep 13, 2019
In order to allow the overlay to be loaded multiple times the
GPIO node for the interrupt line needs to be unique.
Rename it based on the MCP23017 I2C address

#3207

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
pelwell pushed a commit that referenced this issue Sep 13, 2019
In order to allow the overlay to be loaded multiple times the
GPIO node for the interrupt line needs to be unique.
Rename it based on the MCP23017 I2C address

#3207

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
pelwell pushed a commit that referenced this issue Sep 13, 2019
In order to allow the overlay to be loaded multiple times the
GPIO node for the interrupt line needs to be unique.
Rename it based on the MCP23017 I2C address

#3207

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
pelwell pushed a commit that referenced this issue Sep 13, 2019
In order to allow the overlay to be loaded multiple times the
GPIO node for the interrupt line needs to be unique.
Rename it based on the MCP23017 I2C address

#3207

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
popcornmix pushed a commit that referenced this issue Sep 17, 2019
In order to allow the overlay to be loaded multiple times the
GPIO node for the interrupt line needs to be unique.
Rename it based on the MCP23017 I2C address

#3207

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
mike-scott pushed a commit to mike-scott/linux that referenced this issue Sep 17, 2019
…evice

In order to allow the overlay to be loaded multiple times the
GPIO node for the interrupt line needs to be unique.
Rename it based on the MCP23017 I2C address

raspberrypi/linux#3207

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
Signed-off-by: Michael Scott <mike@foundries.io>
pelwell pushed a commit that referenced this issue Sep 19, 2019
In order to allow the overlay to be loaded multiple times the
GPIO node for the interrupt line needs to be unique.
Rename it based on the MCP23017 I2C address

#3207

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
pelwell pushed a commit that referenced this issue Sep 19, 2019
In order to allow the overlay to be loaded multiple times the
GPIO node for the interrupt line needs to be unique.
Rename it based on the MCP23017 I2C address

#3207

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
popcornmix pushed a commit that referenced this issue Oct 4, 2019
In order to allow the overlay to be loaded multiple times the
GPIO node for the interrupt line needs to be unique.
Rename it based on the MCP23017 I2C address

#3207

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
popcornmix pushed a commit that referenced this issue Oct 4, 2019
In order to allow the overlay to be loaded multiple times the
GPIO node for the interrupt line needs to be unique.
Rename it based on the MCP23017 I2C address

#3207

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
popcornmix pushed a commit that referenced this issue Oct 11, 2019
In order to allow the overlay to be loaded multiple times the
GPIO node for the interrupt line needs to be unique.
Rename it based on the MCP23017 I2C address

#3207

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
popcornmix pushed a commit that referenced this issue Oct 11, 2019
In order to allow the overlay to be loaded multiple times the
GPIO node for the interrupt line needs to be unique.
Rename it based on the MCP23017 I2C address

#3207

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
popcornmix pushed a commit that referenced this issue Oct 21, 2019
In order to allow the overlay to be loaded multiple times the
GPIO node for the interrupt line needs to be unique.
Rename it based on the MCP23017 I2C address

#3207

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
jason77-wang pushed a commit to jason77-wang/eoan-rpi-pull that referenced this issue Oct 25, 2019
BugLink: https://bugs.launchpad.net/bugs/1849623

In order to allow the overlay to be loaded multiple times the
GPIO node for the interrupt line needs to be unique.
Rename it based on the MCP23017 I2C address

raspberrypi/linux#3207

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
(cherry picked from commit 9b2cfb8ed3697b5d3c26f496d9838ddc3f16483d
https://github.com/raspberrypi/linux.git rpi-5.3.y)
Signed-off-by: Hui Wang <hui.wang@canonical.com>
pelwell pushed a commit that referenced this issue Oct 29, 2019
In order to allow the overlay to be loaded multiple times the
GPIO node for the interrupt line needs to be unique.
Rename it based on the MCP23017 I2C address

#3207

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
pelwell pushed a commit that referenced this issue Oct 30, 2019
In order to allow the overlay to be loaded multiple times the
GPIO node for the interrupt line needs to be unique.
Rename it based on the MCP23017 I2C address

#3207

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
popcornmix pushed a commit that referenced this issue Nov 1, 2019
In order to allow the overlay to be loaded multiple times the
GPIO node for the interrupt line needs to be unique.
Rename it based on the MCP23017 I2C address

#3207

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
popcornmix pushed a commit that referenced this issue Nov 11, 2019
In order to allow the overlay to be loaded multiple times the
GPIO node for the interrupt line needs to be unique.
Rename it based on the MCP23017 I2C address

#3207

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
popcornmix pushed a commit that referenced this issue Nov 18, 2019
In order to allow the overlay to be loaded multiple times the
GPIO node for the interrupt line needs to be unique.
Rename it based on the MCP23017 I2C address

#3207

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
popcornmix pushed a commit that referenced this issue Nov 22, 2019
In order to allow the overlay to be loaded multiple times the
GPIO node for the interrupt line needs to be unique.
Rename it based on the MCP23017 I2C address

#3207

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
popcornmix pushed a commit that referenced this issue Nov 26, 2019
In order to allow the overlay to be loaded multiple times the
GPIO node for the interrupt line needs to be unique.
Rename it based on the MCP23017 I2C address

#3207

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
popcornmix pushed a commit that referenced this issue Nov 29, 2019
In order to allow the overlay to be loaded multiple times the
GPIO node for the interrupt line needs to be unique.
Rename it based on the MCP23017 I2C address

#3207

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
popcornmix pushed a commit that referenced this issue Dec 9, 2019
In order to allow the overlay to be loaded multiple times the
GPIO node for the interrupt line needs to be unique.
Rename it based on the MCP23017 I2C address

#3207

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
popcornmix pushed a commit that referenced this issue Dec 13, 2019
In order to allow the overlay to be loaded multiple times the
GPIO node for the interrupt line needs to be unique.
Rename it based on the MCP23017 I2C address

#3207

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
@JamesH65
Copy link
Contributor

Closing this issue as questions answered/issue resolved.

popcornmix pushed a commit that referenced this issue Jan 6, 2020
In order to allow the overlay to be loaded multiple times the
GPIO node for the interrupt line needs to be unique.
Rename it based on the MCP23017 I2C address

#3207

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
big-henry pushed a commit to big-henry/mirror_ubuntu-bionic-kernel that referenced this issue Feb 17, 2020
BugLink: https://bugs.launchpad.net/bugs/1849623

In order to allow the overlay to be loaded multiple times the
GPIO node for the interrupt line needs to be unique.
Rename it based on the MCP23017 I2C address

raspberrypi/linux#3207

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
(cherry picked from commit 9b2cfb8ed3697b5d3c26f496d9838ddc3f16483d
https://github.com/raspberrypi/linux.git rpi-5.3.y)
Signed-off-by: Hui Wang <hui.wang@canonical.com>
Signed-off-by: Connor Kuehl <connor.kuehl@canonical.com>
Acked-by: Paolo Pisati <paolo.pisati@canonical.com>
Acked-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com>
Signed-off-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants