[BMIPS] "gpio-leds" does not work on BCM63168

For Developers
1

I'm trying to add support for F@st3864 in the BMIPS target and found that "gpio-leds" didnt work on the BCM63168 SOC.

"gpio-leds" or GPIOs with ID>32 dont work on the BMIPS target. These were working on the soon deprecated BCM63xx target.

Here is the relevant dts:

/ {
   < omitted >

	leds {
		compatible = "gpio-leds";

		led@36 {
			gpios = <&gpio 36 GPIO_ACTIVE_LOW>;
			label = "green:wlan";
		};

		led@37 {
			gpios = <&gpio 37 GPIO_ACTIVE_HIGH>;
			label = "green:wlan_pull_up";
			default-state = "on";
		};

		led@38 {
			gpios = <&gpio 38 GPIO_ACTIVE_LOW>;
			label = "amber:wan";
		};

		led@39 {
			gpios = <&gpio 39 GPIO_ACTIVE_LOW>;
			label = "green:wan";
		};
	};
};

This wiki page said that:

When having more than 32 GPIOs they are splitted between 2 gpiochips. The labels in the Linux kernel are:

bcm63xx-gpio.0
bcm63xx-gpio.1

On my OpenWrt build, I get

root@OpenWrt:~# cat /sys/kernel/debug/gpio
gpiochip0: GPIOs 460-511, parent: platform/100000d0.pinctrl, 100000d0.pinctrl, can sleep:
 gpio-492 (                    |reset               ) in  hi ACTIVE LOW
 gpio-493 (                    |wps                 ) in  hi ACTIVE LOW
 gpio-494 (                    |wlan                ) in  hi ACTIVE LOW
 gpio-496 (                    |green:wlan          ) out lo ACTIVE LOW
 gpio-497 (                    |green:wlan_pull_up  ) out lo
 gpio-498 (                    |amber:wan           ) out lo ACTIVE LOW
 gpio-499 (                    |green:wan           ) out lo ACTIVE LOW
root@OpenWrt:~# cat /sys/kernel/debug/pinctrl/100000d0.pinctrl/pinmux-pins
Pinmux settings per pin
Format: pin (name): mux_owner|gpio_owner (strict) hog?
pin 0 (gpio0): device 10001900.led-controller function serial_led_clk group gpio0
pin 1 (gpio1): device 10001900.led-controller function serial_led_data group gpio1
pin 2 (gpio2): UNCLAIMED
pin 3 (gpio3): UNCLAIMED
pin 4 (gpio4): UNCLAIMED
pin 5 (gpio5): UNCLAIMED
pin 6 (gpio6): UNCLAIMED
pin 7 (gpio7): UNCLAIMED
pin 8 (gpio8): device 10001900.led-controller function led group gpio8
pin 9 (gpio9): device 10001900.led-controller function led group gpio9
pin 10 (gpio10): UNCLAIMED
pin 11 (gpio11): UNCLAIMED
pin 12 (gpio12): UNCLAIMED
pin 13 (gpio13): UNCLAIMED
pin 14 (gpio14): UNCLAIMED
pin 15 (gpio15): device 10001900.led-controller function led group gpio15
pin 16 (gpio16): UNCLAIMED
pin 17 (gpio17): UNCLAIMED
pin 18 (gpio18): UNCLAIMED
pin 19 (gpio19): UNCLAIMED
pin 20 (gpio20): device 10001900.led-controller function led group gpio20
pin 21 (gpio21): UNCLAIMED
pin 22 (gpio22): UNCLAIMED
pin 23 (gpio23): UNCLAIMED
pin 24 (gpio24): UNCLAIMED
pin 25 (gpio25): UNCLAIMED
pin 26 (gpio26): UNCLAIMED
pin 27 (gpio27): UNCLAIMED
pin 28 (gpio28): UNCLAIMED
pin 29 (gpio29): UNCLAIMED
pin 30 (gpio30): UNCLAIMED
pin 31 (gpio31): UNCLAIMED
pin 32 (gpio32): UNCLAIMED
pin 33 (gpio33): UNCLAIMED
pin 34 (gpio34): UNCLAIMED
pin 35 (gpio35): UNCLAIMED
pin 36 (gpio36): UNCLAIMED
pin 37 (gpio37): UNCLAIMED
pin 38 (gpio38): UNCLAIMED
pin 39 (gpio39): UNCLAIMED
pin 40 (gpio40): UNCLAIMED
pin 41 (gpio41): UNCLAIMED
pin 42 (gpio42): UNCLAIMED
pin 43 (gpio43): UNCLAIMED
pin 44 (gpio44): UNCLAIMED
pin 45 (gpio45): UNCLAIMED
pin 46 (gpio46): UNCLAIMED
pin 47 (gpio47): UNCLAIMED
pin 48 (gpio48): UNCLAIMED
pin 49 (gpio49): UNCLAIMED
pin 50 (gpio50): UNCLAIMED
pin 51 (gpio51): UNCLAIMED

There is only one gpiochip, which is not consistent with the wiki.

2

I'm working on several BCM63168 devices and I'm seeing a similar thing. The behaviour seems to be:

  • Don't assign the pins in the DT: driving them with gpioset works
  • Assign the pins to the bcm6328-leds node: setting them in sysfs work
  • Assign the pins to the gpio-leds node: some pins work, some don't

Looking at the gpio registers the bits aren't being set by the gpio-leds driver.

For the Actiontec T1200H, pins 9, 22, and 35 work as gpio-leds. There are other leds that I can't find the pin for, so there may be other issues.

root@OpenWrt:/sys/class/leds# echo 1 > led1/brightness 
root@OpenWrt:/sys/class/leds# devmem 0x100000c8 64
0x0000000F40EFFF1F
root@OpenWrt:/sys/class/leds# echo 0 > led1/brightness 
root@OpenWrt:/sys/class/leds# devmem 0x100000c8 64
0x0000000F40EFFF1F

root@OpenWrt:/sys/class/leds# echo 1 > led9/brightness 
root@OpenWrt:/sys/class/leds# devmem 0x100000c8 64
0x0000000F40EFFD1F
root@OpenWrt:/sys/class/leds# echo 0 > led9/brightness 
root@OpenWrt:/sys/class/leds# devmem 0x100000c8 64
0x0000000F40EFFF1F

root@OpenWrt:/sys/class/leds# echo 1 > led20/brightness 
root@OpenWrt:/sys/class/leds# devmem 0x100000c8 64
0x0000000F40EFFF1F
root@OpenWrt:/sys/class/leds# echo 0 > led20/brightness 
root@OpenWrt:/sys/class/leds# devmem 0x100000c8 64
0x0000000F40EFFF1F

root@OpenWrt:/sys/class/leds# echo 1 > led22/brightness 
root@OpenWrt:/sys/class/leds# devmem 0x100000c8 64
0x0000000F40AFFF1F
root@OpenWrt:/sys/class/leds# echo 0 > led22/brightness 
root@OpenWrt:/sys/class/leds# devmem 0x100000c8 64
0x0000000F40EFFF1F

root@OpenWrt:/sys/class/leds# echo 1 > led35/brightness 
root@OpenWrt:/sys/class/leds# devmem 0x100000c8 64
0x0000000740EFFF1F
root@OpenWrt:/sys/class/leds# echo 0 > led35/brightness 
root@OpenWrt:/sys/class/leds# devmem 0x100000c8 64
0x0000000F40EFFF1F

For the SmartRG SR505n, only pin 16 worked with gpio-leds.

root@OpenWrt:/sys/class/leds# echo 1 > led1/brightness 
root@OpenWrt:/sys/class/leds# devmem 0x100000c8 64
0x0000000FFFEEDDFF
root@OpenWrt:/sys/class/leds# echo 0 > led1/brightness 
root@OpenWrt:/sys/class/leds# devmem 0x100000c8 64
0x0000000FFFEEDDFF

root@OpenWrt:/sys/class/leds# echo 1 > led8/brightness 
root@OpenWrt:/sys/class/leds# devmem 0x100000c8 64
0x0000000FFFEEDDFF
root@OpenWrt:/sys/class/leds# echo 0 > led8/brightness 
root@OpenWrt:/sys/class/leds# devmem 0x100000c8 64
0x0000000FFFEEDDFF

root@OpenWrt:/sys/class/leds# echo 1 > led14/brightness 
root@OpenWrt:/sys/class/leds# devmem 0x100000c8 64
0x0000000FFFEEDDFF
root@OpenWrt:/sys/class/leds# echo 0 > led14/brightness 
root@OpenWrt:/sys/class/leds# devmem 0x100000c8 64
0x0000000FFFEEDDFF

root@OpenWrt:/sys/class/leds# echo 1 > led15/brightness 
root@OpenWrt:/sys/class/leds# devmem 0x100000c8 64
0x0000000FFFEEDDFF
root@OpenWrt:/sys/class/leds# echo 0 > led15/brightness 
root@OpenWrt:/sys/class/leds# devmem 0x100000c8 64
0x0000000FFFEEDDFF

root@OpenWrt:/sys/class/leds# echo 1 > led16/brightness 
root@OpenWrt:/sys/class/leds# devmem 0x100000c8 64
0x0000000FFFEEDDFF
root@OpenWrt:/sys/class/leds# echo 0 > led16/brightness 
root@OpenWrt:/sys/class/leds# devmem 0x100000c8 64
0x0000000FFFEFDDFF

root@OpenWrt:/sys/class/leds# echo 1 > led20/brightness 
root@OpenWrt:/sys/class/leds# devmem 0x100000c8 64
0x0000000FFFEFDDFF
root@OpenWrt:/sys/class/leds# echo 0 > led20/brightness 
root@OpenWrt:/sys/class/leds# devmem 0x100000c8 64
0x0000000FFFEFDDFF

root@OpenWrt:/sys/class/leds# echo 1 > led21/brightness 
root@OpenWrt:/sys/class/leds# devmem 0x100000c8 64
0x0000000FFFEFDDFF
root@OpenWrt:/sys/class/leds# echo 0 > led21/brightness 
root@OpenWrt:/sys/class/leds# devmem 0x100000c8 64
0x0000000FFFEFDDFF

For the OvisLink OV804WVA, all the pins worked with gpio-leds.

root@OpenWrt:/sys/class/leds# echo 1 > led8/brightness
root@OpenWrt:/sys/class/leds# devmem 0x100000c8 64
0x0000000FFFFFDEFE
root@OpenWrt:/sys/class/leds# echo 0 > led8/brightness
root@OpenWrt:/sys/class/leds# devmem 0x100000c8 64
0x0000000FFFFFDFFE

root@OpenWrt:/sys/class/leds# echo 1 > led9/brightness
root@OpenWrt:/sys/class/leds# devmem 0x100000c8 64
0x0000000FFFFFDDFE
root@OpenWrt:/sys/class/leds# echo 0 > led9/brightness
root@OpenWrt:/sys/class/leds# devmem 0x100000c8 64
0x0000000FFFFFDFFE

root@OpenWrt:/sys/class/leds# echo 1 > led14/brightness
root@OpenWrt:/sys/class/leds# devmem 0x100000c8 64
0x0000000FFFFF9FFE
root@OpenWrt:/sys/class/leds# echo 0 > led14/brightness
root@OpenWrt:/sys/class/leds# devmem 0x100000c8 64
0x0000000FFFFFDFFE

root@OpenWrt:/sys/class/leds# echo 1 > led18/brightness
root@OpenWrt:/sys/class/leds# devmem 0x100000c8 64
0x0000000FFFFBDFFE
root@OpenWrt:/sys/class/leds# echo 0 > led18/brightness
root@OpenWrt:/sys/class/leds# devmem 0x100000c8 64
0x0000000FFFFFDFFE

root@OpenWrt:/sys/class/leds# echo 1 > led19/brightness
root@OpenWrt:/sys/class/leds# devmem 0x100000c8 64
0x0000000FFFF7DFFE
root@OpenWrt:/sys/class/leds# echo 0 > led19/brightness
root@OpenWrt:/sys/class/leds# devmem 0x100000c8 64
0x0000000FFFFFDFFE

root@OpenWrt:/sys/class/leds# echo 1 > led20/brightness
root@OpenWrt:/sys/class/leds# devmem 0x100000c8 64
0x0000000FFFEFDFFE
root@OpenWrt:/sys/class/leds# echo 0 > led20/brightness
root@OpenWrt:/sys/class/leds# devmem 0x100000c8 64
0x0000000FFFFFDFFE

root@OpenWrt:/sys/class/leds# echo 1 > led21/brightness
root@OpenWrt:/sys/class/leds# devmem 0x100000c8 64
0x0000000FFFDFDFFE
root@OpenWrt:/sys/class/leds# echo 0 > led21/brightness
root@OpenWrt:/sys/class/leds# devmem 0x100000c8 64
0x0000000FFFFFDFFE

root@OpenWrt:/sys/class/leds# echo 1 > led22/brightness
root@OpenWrt:/sys/class/leds# devmem 0x100000c8 64
0x0000000FFFBFDFFE
root@OpenWrt:/sys/class/leds# echo 0 > led22/brightness
root@OpenWrt:/sys/class/leds# devmem 0x100000c8 64
0x0000000FFFFFDFFE

root@OpenWrt:/sys/class/leds# echo 1 > led23/brightness
root@OpenWrt:/sys/class/leds# devmem 0x100000c8 64
0x0000000FFF7FDFFE
root@OpenWrt:/sys/class/leds# echo 0 > led23/brightness
root@OpenWrt:/sys/class/leds# devmem 0x100000c8 64
0x0000000FFFFFDFFE

I'd have to dig deeper to figure out if the driver isn't trying to set the bits, if something else is settings them back, or if the new value isn't accepted. My guess is the bootloader is leaving the lines in a weird state.

I'm going to try and work around this by using bcm6328-leds for pins <24 and gpio-leds for the rest.

3

I would agree with this. There could be a mux on each GPIO pin to determine its role. In this case, the LED pin could be in GPIO output mode, or the LED controller peripheral directly control. In addition, the LED controller also supports a "serial mode", where a 74HC164 shift register is connected to 2 GPIOs (Clock and Data). The LED controller takes over the control of these GPIOs.

I don't know how many LEDs the LED controller supports and how Linux assigns IDs to LEDs when all of the three LED modes are used at the same time.

It is extremely hard to tell as we don't have the datasheet of the SoC. It's time to dive into the Linux source code and insert some printk debug outputs...

4

I think I found something that explains why the behaviour is different on bcm63xx:
board_bcm963xx.c

The platform code clears the mode register so all the pins are explicitly gpios it seems. A heavy handed kludge would be to do that in bcm63268_pinctrl_probe(), but the proper way is probably to use the device tree.

When I add a node such as:

&pinctrl {
	pinctrl_gpioleds: gpioleds {
		function = "gpio";
		pins = "gpio20";
	};
};

I get:
bcm63268-pinctrl 100000d0.pinctrl: invalid function gpio in map table
This makes some sense since pinctrl-bcm63268.c only enumerates the alt modes for pins.

Should it be necessary to explicitly configure pins as gpio in the DT? Should the driver consuming the gpio be the one that sets the pinmux?

5

I've been trying to get this to work but can't get the pins configured for gpio mode. I did a quick patch adding a gpio function group that seems to work. It seems to clear the mode for all the pins which isn't ideal, but it looks like that's what most of the other groups do as well. The alternative is to create a function group for every individual pin which would be a lot of clutter.

If anyone has a better understanding of how this should work and has any suggestions I'd be interested to hear them. Otherwise I'll sit on this for a few days and try to get a pull request up. Patch as follows:

--- a/drivers/pinctrl/bcm/pinctrl-bcm63268.c
+++ b/drivers/pinctrl/bcm/pinctrl-bcm63268.c
@@ -38,6 +38,7 @@ enum bcm63268_pinctrl_reg {
        BCM63268_MODE,
        BCM63268_CTRL,
        BCM63268_BASEMODE,
+       BCM63268_NOREG,
 };
 
 struct bcm63268_function {
@@ -242,6 +243,61 @@ static struct pingroup bcm63268_groups[]
        BCM_PIN_GROUP(vdsl_phy3_grp),
 };
 
+static const char * const gpio_groups[] = {
+       "gpio0",
+       "gpio1",
+       "gpio2",
+       "gpio3",
+       "gpio4",
+       "gpio5",
+       "gpio6",
+       "gpio7",
+       "gpio8",
+       "gpio9",
+       "gpio10",
+       "gpio11",
+       "gpio12",
+       "gpio13",
+       "gpio14",
+       "gpio15",
+       "gpio16",
+       "gpio17",
+       "gpio18",
+       "gpio19",
+       "gpio20",
+       "gpio21",
+       "gpio22",
+       "gpio23",
+       "gpio24",
+       "gpio25",
+       "gpio26",
+       "gpio27",
+       "gpio28",
+       "gpio29",
+       "gpio30",
+       "gpio31",
+       "gpio32",
+       "gpio33",
+       "gpio34",
+       "gpio35",
+       "gpio36",
+       "gpio37",
+       "gpio38",
+       "gpio39",
+       "gpio40",
+       "gpio41",
+       "gpio42",
+       "gpio43",
+       "gpio44",
+       "gpio45",
+       "gpio46",
+       "gpio47",
+       "gpio48",
+       "gpio49",
+       "gpio50",
+       "gpio51",
+};
+
 static const char * const led_groups[] = {
        "gpio0",
        "gpio1",
@@ -427,7 +483,16 @@ static const char * const vdsl_phy_overr
                .mask = val,                            \
        }
 
+#define BCM63268_NOMODE_FUN(n)                 \
+       {                                               \
+               .name = #n,                             \
+               .groups = n##_groups,                   \
+               .num_groups = ARRAY_SIZE(n##_groups),   \
+               .reg = BCM63268_NOREG,                  \
+       }
+
 static const struct bcm63268_function bcm63268_funcs[] = {
+       BCM63268_NOMODE_FUN(gpio),
        BCM63268_LED_FUN(led),
        BCM63268_MODE_FUN(serial_led_clk),
        BCM63268_MODE_FUN(serial_led_data),
@@ -562,6 +627,9 @@ static int bcm63268_pinctrl_set_mux(stru
                mask = f->mask;
                val = f->mask;
                break;
+       case BCM63268_NOREG:
+               /*Do nothing, leave regs as default*/
+               break;
        default:
                WARN_ON(1);
                return -EINVAL;
6

Commit e44daa4 adds the ability to explicitly set pins to gpio mode. Configuring this from the device tree is similar to using bcm6328-leds except the function in the DT pinctrl is gpio:

leds {
		compatible = "gpio-leds";

		pinctrl-names = "default";
		pinctrl-0 = <&pinctrl_gpioleds>;

		led-0 {
			label = "red:internet";
			color = <LED_COLOR_ID_RED>;
			gpios = <&gpio 0 GPIO_ACTIVE_LOW>;
		};

		led_power_green: led-20 {
			function = LED_FUNCTION_POWER;
			color = <LED_COLOR_ID_GREEN>;
			gpios = <&gpio 20 GPIO_ACTIVE_LOW>;
		};
	};

&pinctrl {
	pinctrl_gpioleds: gpioleds {
		pinmux {
			function = "gpio";
			groups = "gpio0",
			"gpio20";
		};
	};
};

I'm seeing leds on at startup for no reason, so the gpio states are probably not in a consistent state either. This is probably easy to work around setting a default-state if no trigger is used.

1 Like
7

Your PR looks good. Previously, it was very hard to tell if a pin is controlled by the led module or used as a GPIO output. Do you have a git repo for the patch? Once I got time, I will try it on my F@st3864 BCM63168 SOC.

8

Add better patch that doesn't need the extra config in the device tree just got put up:

1 Like