LED's 1...4 supported for Mikrotik RB922 (https://github.com/rogerpueyo/openwrt)

I had a little discussion with dear Roger about this.
He offered to bring it to the forum, but while I was thinking about it, I managed to write the patch earlier.
However, I am sure that it can be done better. :slight_smile:

mikrotik-rb-922-leds-patch
--- build_dir/target-mips_24kc_musl/linux-ath79_mikrotik/ath10k-ct-regular/ath10k-ct-2021-01-11-9fe1df7d/ath10k-5.10/hw.h.orig      2021-05-03 22:35:54.365633776 +0300
+++ build_dir/target-mips_24kc_musl/linux-ath79_mikrotik/ath10k-ct-regular/ath10k-ct-2021-01-11-9fe1df7d/ath10k-5.10/hw.h   2021-05-04 11:19:44.766434317 +0300
@@ -522,6 +522,7 @@
        u32 patch_load_addr;
        int uart_pin;
        int led_pin;
+       int led_pin_rb922[4];
        u32 otp_exe_param;

        /* Type of hw cycle counter wraparound logic, for more info

--- build_dir/target-mips_24kc_musl/linux-ath79_mikrotik/ath10k-ct-regular/ath10k-ct-2021-01-11-9fe1df7d/ath10k-5.10/core.h.orig    2021-05-03 22:35:54.373633794 +0300
+++ build_dir/target-mips_24kc_musl/linux-ath79_mikrotik/ath10k-ct-regular/ath10k-ct-2021-01-11-9fe1df7d/ath10k-5.10/core.h 2021-05-04 12:09:36.824326794 +0300
@@ -1552,10 +1552,10 @@
        } testmode;

        struct {
-               struct gpio_led wifi_led;
-               struct led_classdev cdev;
-               char label[48];
-               u32 gpio_state_pin;
+               struct gpio_led wifi_led, wifi_led2, wifi_led3, wifi_led4, wifi_led5;
+               struct led_classdev cdev, cdev2, cdev3, cdev4, cdev5;
+               char label[48], label2[48], label3[48], label4[48], label5[48];
+               u32 gpio_state_pin, gpio_state_pin2, gpio_state_pin3, gpio_state_pin4, gpio_state_pin5;
        } leds;

        struct {

--- build_dir/target-mips_24kc_musl/linux-ath79_mikrotik/ath10k-ct-regular/ath10k-ct-2021-01-11-9fe1df7d/ath10k-5.10/core.c.orig    2021-05-04 11:21:21.522649356 +0300
+++ build_dir/target-mips_24kc_musl/linux-ath79_mikrotik/ath10k-ct-regular/ath10k-ct-2021-01-11-9fe1df7d/ath10k-5.10/core.c 2021-05-04 11:20:59.526600485 +0300
@@ -70,6 +70,7 @@
                .bus = ATH10K_BUS_PCI,
                .name = "qca988x hw2.0",
                .led_pin = 1,
+               .led_pin_rb922 = { 8, 9, 10, 11,},
                .patch_load_addr = QCA988X_HW_2_0_PATCH_LOAD_ADDR,
                .uart_pin = 7,
                .cc_wraparound_type = ATH10K_HW_CC_WRAP_SHIFTED_ALL,

--- /home/yury/openwrt1/build_dir/target-mips_24kc_musl/linux-ath79_mikrotik/ath10k-ct-regular/ath10k-ct-2021-01-11-9fe1df7d/ath10k-5.10/leds.c.orig    2021-05-03 22:35:54.373633794 +0300
+++ /home/yury/openwrt1/build_dir/target-mips_24kc_musl/linux-ath79_mikrotik/ath10k-ct-regular/ath10k-ct-2021-01-11-9fe1df7d/ath10k-5.10/leds.c 2021-05-04 13:33:18.973779405 +0300
@@ -46,6 +46,90 @@
        return 0;
 }

+static int ath10k_leds_set_brightness_blocking2(struct led_classdev *led_cdev,
+                                               enum led_brightness brightness)
+{
+        struct ath10k *ar = container_of(led_cdev, struct ath10k,
+                                         leds.cdev2);
+        struct gpio_led *led = &ar->leds.wifi_led2;
+
+        mutex_lock(&ar->conf_mutex);
+
+        if (ar->state != ATH10K_STATE_ON)
+                goto out;
+
+        ar->leds.gpio_state_pin2 = (brightness != LED_OFF) ^ led->active_low;
+        ath10k_wmi_gpio_output(ar, led->gpio, ar->leds.gpio_state_pin2);
+
+out:
+        mutex_unlock(&ar->conf_mutex);
+
+        return 0;
+}
+
+static int ath10k_leds_set_brightness_blocking3(struct led_classdev *led_cdev,
+                                               enum led_brightness brightness)
+{
+        struct ath10k *ar = container_of(led_cdev, struct ath10k,
+                                         leds.cdev3);
+        struct gpio_led *led = &ar->leds.wifi_led3;
+
+        mutex_lock(&ar->conf_mutex);
+
+        if (ar->state != ATH10K_STATE_ON)
+                goto out;
+
+        ar->leds.gpio_state_pin3 = (brightness != LED_OFF) ^ led->active_low;
+        ath10k_wmi_gpio_output(ar, led->gpio, ar->leds.gpio_state_pin3);
+
+out:
+        mutex_unlock(&ar->conf_mutex);
+
+        return 0;
+}
+
+static int ath10k_leds_set_brightness_blocking4(struct led_classdev *led_cdev,
+                                               enum led_brightness brightness)
+{
+        struct ath10k *ar = container_of(led_cdev, struct ath10k,
+                                         leds.cdev4);
+        struct gpio_led *led = &ar->leds.wifi_led4;
+
+        mutex_lock(&ar->conf_mutex);
+
+        if (ar->state != ATH10K_STATE_ON)
+                goto out;
+
+        ar->leds.gpio_state_pin4 = (brightness != LED_OFF) ^ led->active_low;
+        ath10k_wmi_gpio_output(ar, led->gpio, ar->leds.gpio_state_pin4);
+
+out:
+        mutex_unlock(&ar->conf_mutex);
+
+        return 0;
+}
+
+static int ath10k_leds_set_brightness_blocking5(struct led_classdev *led_cdev,
+                                               enum led_brightness brightness)
+{
+        struct ath10k *ar = container_of(led_cdev, struct ath10k,
+                                         leds.cdev5);
+        struct gpio_led *led = &ar->leds.wifi_led5;
+
+        mutex_lock(&ar->conf_mutex);
+
+        if (ar->state != ATH10K_STATE_ON)
+                goto out;
+
+        ar->leds.gpio_state_pin5 = (brightness != LED_OFF) ^ led->active_low;
+        ath10k_wmi_gpio_output(ar, led->gpio, ar->leds.gpio_state_pin5);
+
+out:
+        mutex_unlock(&ar->conf_mutex);
+
+        return 0;
+}
+
 int ath10k_leds_start(struct ath10k *ar)
 {
        if (ar->hw_params.led_pin == 0)
@@ -58,9 +142,25 @@
         * QCA9984 and QCA99XX devices so far
         */
        ath10k_wmi_gpio_config(ar, ar->hw_params.led_pin, 0,
-                              WMI_GPIO_PULL_NONE, WMI_GPIO_INTTYPE_DISABLE);
+                               WMI_GPIO_PULL_NONE, WMI_GPIO_INTTYPE_DISABLE);
        ath10k_wmi_gpio_output(ar, ar->hw_params.led_pin, 1);

+       ath10k_wmi_gpio_config(ar, ar->hw_params.led_pin_rb922[0], 0,
+                               WMI_GPIO_PULL_NONE, WMI_GPIO_INTTYPE_DISABLE);
+       ath10k_wmi_gpio_output(ar, ar->hw_params.led_pin_rb922[0], 1);
+
+       ath10k_wmi_gpio_config(ar, ar->hw_params.led_pin_rb922[1], 0,
+                               WMI_GPIO_PULL_NONE, WMI_GPIO_INTTYPE_DISABLE);
+       ath10k_wmi_gpio_output(ar, ar->hw_params.led_pin_rb922[1], 1);
+
+       ath10k_wmi_gpio_config(ar, ar->hw_params.led_pin_rb922[2], 0,
+                               WMI_GPIO_PULL_NONE, WMI_GPIO_INTTYPE_DISABLE);
+       ath10k_wmi_gpio_output(ar, ar->hw_params.led_pin_rb922[2], 1);
+
+       ath10k_wmi_gpio_config(ar, ar->hw_params.led_pin_rb922[3], 0,
+                               WMI_GPIO_PULL_NONE, WMI_GPIO_INTTYPE_DISABLE);
+       ath10k_wmi_gpio_output(ar, ar->hw_params.led_pin_rb922[3], 1);
+
        return 0;
 }

@@ -87,6 +187,66 @@
        if (ret)
                return ret;

+       snprintf(ar->leds.label, sizeof(ar->leds.label), "ath10k-%s-2",
+               wiphy_name(ar->hw->wiphy));
+       ar->leds.wifi_led2.active_low = 1;
+       ar->leds.wifi_led2.gpio = ar->hw_params.led_pin_rb922[0];
+       ar->leds.wifi_led2.name = ar->leds.label;
+       ar->leds.wifi_led2.default_state = LEDS_GPIO_DEFSTATE_KEEP;
+
+       ar->leds.cdev2.name = ar->leds.label;
+       ar->leds.cdev2.brightness_set_blocking = ath10k_leds_set_brightness_blocking2;
+       ar->leds.cdev2.default_trigger = ar->led_default_trigger;
+
+       ret = led_classdev_register(wiphy_dev(ar->hw->wiphy), &ar->leds.cdev2);
+       if (ret)
+               return ret;
+
+       snprintf(ar->leds.label, sizeof(ar->leds.label), "ath10k-%s-3",
+               wiphy_name(ar->hw->wiphy));
+       ar->leds.wifi_led3.active_low = 1;
+       ar->leds.wifi_led3.gpio = ar->hw_params.led_pin_rb922[1];
+       ar->leds.wifi_led3.name = ar->leds.label;
+       ar->leds.wifi_led3.default_state = LEDS_GPIO_DEFSTATE_KEEP;
+
+       ar->leds.cdev3.name = ar->leds.label;
+       ar->leds.cdev3.brightness_set_blocking = ath10k_leds_set_brightness_blocking3;
+       ar->leds.cdev3.default_trigger = ar->led_default_trigger;
+
+       ret = led_classdev_register(wiphy_dev(ar->hw->wiphy), &ar->leds.cdev3);
+       if (ret)
+               return ret;
+
+       snprintf(ar->leds.label, sizeof(ar->leds.label), "ath10k-%s-4",
+               wiphy_name(ar->hw->wiphy));
+       ar->leds.wifi_led4.active_low = 1;
+       ar->leds.wifi_led4.gpio = ar->hw_params.led_pin_rb922[2];
+       ar->leds.wifi_led4.name = ar->leds.label;
+       ar->leds.wifi_led4.default_state = LEDS_GPIO_DEFSTATE_KEEP;
+
+       ar->leds.cdev4.name = ar->leds.label;
+       ar->leds.cdev4.brightness_set_blocking = ath10k_leds_set_brightness_blocking4;
+       ar->leds.cdev4.default_trigger = ar->led_default_trigger;
+
+       ret = led_classdev_register(wiphy_dev(ar->hw->wiphy), &ar->leds.cdev4);
+       if (ret)
+               return ret;
+
+       snprintf(ar->leds.label, sizeof(ar->leds.label), "ath10k-%s-5",
+               wiphy_name(ar->hw->wiphy));
+       ar->leds.wifi_led5.active_low = 1;
+       ar->leds.wifi_led5.gpio = ar->hw_params.led_pin_rb922[3];
+       ar->leds.wifi_led5.name = ar->leds.label;
+       ar->leds.wifi_led5.default_state = LEDS_GPIO_DEFSTATE_KEEP;
+
+       ar->leds.cdev5.name = ar->leds.label;
+       ar->leds.cdev5.brightness_set_blocking = ath10k_leds_set_brightness_blocking5;
+       ar->leds.cdev5.default_trigger = ar->led_default_trigger;
+
+       ret = led_classdev_register(wiphy_dev(ar->hw->wiphy), &ar->leds.cdev5);
+       if (ret)
+               return ret;
+
        return 0;
 }

--- target/linux/ath79/dts/qca9558_mikrotik_routerboard-922uags-5hpacd.dts.orig     2021-05-03 16:38:47.217309263 +0300
+++ target/linux/ath79/dts/qca9558_mikrotik_routerboard-922uags-5hpacd.dts  2021-05-04 12:50:25.825091188 +0300
@@ -14,6 +14,26 @@
                        gpios = <&ath10k 0 GPIO_ACTIVE_LOW>;
                        linux,default-trigger = "phy0tpt";
                };
+               wlan5g2 {
+                       label = "green:wlan5g2";
+                       gpios = <&ath10k 1 GPIO_ACTIVE_LOW>;
+                       linux,default-trigger = "phy0tpt";
+               };
+               wlan5g3 {
+                       label = "green:wlan5g3";
+                       gpios = <&ath10k 2 GPIO_ACTIVE_LOW>;
+                       linux,default-trigger = "phy0tpt";
+               };
+               wlan5g4 {
+                       label = "green:wlan5g4";
+                       gpios = <&ath10k 3 GPIO_ACTIVE_LOW>;
+                       linux,default-trigger = "phy0tpt";
+               };
+               wlan5g5 {
+                       label = "green:wlan5g5";
+                       gpios = <&ath10k 4 GPIO_ACTIVE_LOW>;
+                       linux,default-trigger = "phy0tpt";
+               };
        };

        gpio-export {
@@ -52,7 +72,7 @@
        ath10k: wifi@0,0 {
                compatible = "qcom,ath10k";
                reg = <0 0 0 0 0>;
-               #gpio-cells = <2>;
+               gpio-cells = <5>;
                gpio-controller;
        };
 };
1 Like

Hi,

This is very nice! I'll give it a try this week. :+1:

Ideally, the patch should be sent upstream to the ath10k/ath10k-ct driver. However, I doubt that adding device-specific code for the RB922 would be accepted, neither redundant functions ath10k_leds_set_brightness_blockingX().

The DTS part looks quite good. Does the "gpio-cells = <5>" line change anything? Maybe the "ngpios" property should be used instead: https://www.kernel.org/doc/Documentation/devicetree/bindings/gpio/gpio.txt . Ideally, I think, the ath10k/ath10k-ct driver should be able to use all the GPIOs the QCA9882 chip has, and limit via the "ngpios" property in the DTS how many to actually use.

Congratulations for your finding! :smiley:

Hi @Croco131,

I gave your code a try and, while the patch didn't apply to my codebase, confirmed that the remaining 4 LEDs are on ath10k's GPIO 8 to 11. :+1:

This said, I wonder if you could get in touch with the ath10k/ath10k-ct driver developers, or with the author of the patch that adds the GPIO/LED support. Maybe now, with your findings, the patch can be extended to enable more than just one GPIO/LED.

Cheers!