[Tone][Loire]Using qcwcn with brcmfmac

Platform: Tone, Loire, Shinano Kernel version: 4.9 and newer Android version: Oreo and newer

Description Hi SODP team, I [backported a recent brcmfmac driver into an older kernel] for the Shinano (MSM8974) platform and I follow the same approach like you do this for Tone or Loire by using the Qualcomm/Atheros qcwcn HAL. @oshmoun explained the rationale here: https://github.com/sonyxperiadev/device-sony-loire/pull/246

This works OK in my case, but many things seem to not work correctly, and the root cause seems to be that the qcwcn HAL (obviously) does not use the right vendor-specific commands.

My sources: Kernel: Derived from Lineage_17.1 Device Tree Changes: shinano-common The full project is described here on XDA.

Example 1: During initialization of the HAL, the HAL tries to gather the supported features via an nl80211 vendor command, using OUI_QCA as the vendor ID (=0x1374). Brcmfmac instead filters commands to match the BROADCOM_OUI (=0x0018) and does not know what "QCA_NL80211_VENDOR_SUBCMD_GET_SUPPORTED_FEATURES" means. Consequently, the kernel answers with an error -95 EOPNOTSUPP which is then translated via mapKernelErrortoWifiHalError into -3 WIFI_ERROR_NOT_SUPPORTED and resulting in a log entry like this: E WifiHAL-AOSP: acquire_supported_features: requestResponse Error:-3

Example 2: When the device detects e.g. a network change it tries to submit a regulatory hint (with a two letter country code) to kernel space (cfg80211), using a custom string via ioctl. Brcmfmac however does not understand this string, instead it expects an nl80211_cmd with NL80211_CMD_REQ_SET_REG. In my case this leads to: E wpa_supplicant: wpa_driver_nl80211_driver_cmd: failed to issue private command: COUNTRY DE

I can partially overcome these issues by modifying the HAL to use the broadcom-specific commands, but how do you address this e.g. for Tone and Loire? Do you have the same issues?

@konradybcio @ix5

@Tom1000 Nice, thanks for digging through those ioctls and finding the failure reason!

It's been a while since Suzu here has last seen a downstream kernel with brcmfmac, and as far as I remember we wat least get the regdom issue on 5GHz. Probably okay to ignore, though I'm surprised it doesn't just use NL80211_CMD_REQ_SET_REG which is a standardized command. The AOSP HAL seems to do the right thing though: https://cs.android.com/android/platform/superproject/+/android-11.0.0_r1:hardware/qcom/wlan/qcwcn/wifi_hal/wificonfig.cpp;l=119-168. However, this also uses a vendor code in the constructor, what's up with that?

Is the custom command perhaps sent through wpa_driver_nl80211_driver_cmd -> wpa_driver_notify_country_change (qcwcn/wpa_supplicant_8_lib/driver_cmd_nl80211.c in the same repository)? I guess that P2P thingy there is just deprecated, old, and wrong...

Perhaps an idea to find and use the same HAL as we did back in the day, ie. from AOSP instead of CAF nor Lineage (@oshmoun we were on AOSP, right?).

Are any of the features relevant or worth shimming? If they're all vendor-specific and not supported anyway you might indeed return 0 features from the kernel or remove the check from userspace. From the logs seen earlier it's fatal though, right? My mind is fuzzy :)

@MarijnS95 Thanks a lot for your feedback.

My recent trials were all with the AOSP HAL, although there is not much difference to the CAF HAL regarding these points.

I'm surprised it doesn't just use NL80211_CMD_REQ_SET_REGwhich is a standardized command

Hmm, I have not found the caller of this function anywhere. There seems to be something coming with Android 11 (Android vendor HAL 1.4), but I miss the piece of the puzzle for Android 10 or earlier. In any case it can't work out-of-the-box with brcmfmac because of the vendor specific commands, at least that is my understanding.

Is the custom command perhaps sent through wpa_driver_nl80211_driver_cmd?

Yes, that seems to come somehow from here. This call then ends up here like you guessed. And yes, I agree, that looks outdated. I started to modify it to make it work with brcmfmac, but that doesn't feel like the right way.

Are any of the features relevant or worth shimming?

I ask myself the same question. There is a lot going on - first, the HAL tries to acquire supported features and things like that. Later on, there are commands for switching Bluetooth Coexistence or Suspend Modes being set through that old interface. In my preliminary tests (+ feedback from two other users) basic features seem to work without such shimming (e.g. scan, connection to APs, 2,4/5GHz transition, hotspot/teathering) but I cannot say much about reliability, power consumption, etc. I have not run any of the "official" wifi test cases.

@Tom1000 Apologies for the late reply - I had queued this up to take a closer look (on Loire) but did not even manage to get close.

It is indeed most likely in the same state: all the basic features work, but things like reliability and power consumption have to be confirmed over time. Anyway, shimming out the features like that seems like the only way forward to resolve these, especially the country code. It surprises me no vendor-agnostic command is used which clearly exists... For the other features, I guess disable them or ignore while reading the logs unless something actually breaks. Testing is all that remains.

However there must be Loire/Tone users around here that my be able to post logs; some of which are most likely already available on the bug_tracker. Won't be a bad idea to collect some of those and compare them to your own just to see how similar the breakage is.

Let me know if there's anything specific I can help out with!

@Tom1000 Perhaps interesting to mention that I have WiFi working on mainline (v5.12-rc7) with very little config in DT and no special userspace binaries besides the usual wpa_supplicant. I hope to publish these soon-ish (could still be months) after cleaning it all up but I can dump the relevant bits here if you're interested.

@MarijnS95 that would be fantastic!

@Tom1000 On the userspace side, we're using this:

A very simple service declaration to start wpa_supplicant with the right driver and control interface (wifi.rc, see PRODUCT_COPY_FILES below):

on zygote-start
    mkdir /data/vendor/wifi 0770 wifi wifi
    mkdir /data/vendor/wifi/wpa 0770 wifi wifi
    mkdir /data/vendor/wifi/wpa/sockets 0770 wifi wifi

service wpa_supplicant /vendor/bin/hw/wpa_supplicant \
                       -Dnl80211 -g@android:wpa_wlan0
    interface android.hardware.wifi.supplicant@1.0::ISupplicant default
    interface android.hardware.wifi.supplicant@1.1::ISupplicant default
    socket wpa_wlan0 dgram 660 wifi wifi
    class main
    disabled
    oneshot

And our wifi.mk file (we've separated configuration in one file per "system" :grimacing:) tying it all together: (Disclaimer: it's still specific to Loire, but will be made more generic as more platforms trickle in on our side. But that makes it concise enough for your Loire/Tone specific case)

WPA_SUPPLICANT_VERSION := VER_0_8_X
BOARD_WPA_SUPPLICANT_DRIVER := NL80211
BOARD_HOSTAPD_DRIVER := NL80211
BOARD_WLAN_DEVICE := qcwcn
BOARD_HOSTAPD_PRIVATE_LIB := lib_driver_cmd_$(BOARD_WLAN_DEVICE)

PRODUCT_PACKAGES += \
    wpa_supplicant \
    wpa_supplicant.conf

# TODO: Might need these later, or are they transitively included? \
    hostapd \
    libwpa_client \
    wificond \
    wifilogd

PRODUCT_COPY_FILES += \
    $(LOCAL_PATH)/rootdir/vendor/etc/init/wifi.rc:$(TARGET_COPY_OUT_VENDOR)/etc/init/wifi.rc

PRODUCT_COPY_FILES += \
    frameworks/native/data/etc/android.hardware.wifi.xml:$(TARGET_COPY_OUT_VENDOR)/etc/permissions/android.hardware.wifi.xml \
    frameworks/native/data/etc/android.hardware.wifi.direct.xml:$(TARGET_COPY_OUT_VENDOR)/etc/permissions/android.hardware.wifi.direct.xml

BCM_FW_SRC_FILE_STA := fw_bcmdhd.bin

# TODO: Driver tries machine-specific txt file first, use that to our advantage
# For suzu, that is `brcm/brcmfmac43455-sdio.sony,suzu-row.txt` with
# `sony,suzu-row` our machine name.
# These files come from https://github.com/sonyxperiadev/vendor-broadcom-wlan
# and SODP. We hope to find and use updated firmware at some point.
PRODUCT_COPY_FILES += \
    vendor/broadcom/wlan/bcmdhd/firmware/bcm43455/$(BCM_FW_SRC_FILE_STA):$(TARGET_COPY_OUT_VENDOR)/firmware/brcm/brcmfmac43455-sdio.bin \
    device/sony/suzu/rootdir/vendor/firmware/bcmdhd.cal:$(TARGET_COPY_OUT_VENDOR)/firmware/brcm/brcmfmac43455-sdio.txt

I'm sure you'll figure out how to work this into the existing SODP :)

Then, on the kernel side we have:

/ {
    bcm43455_pwr_vreg: wlan-regulator {
        compatible = "regulator-fixed";

        regulator-name = "wlan-vreg";
        regulator-min-microvolt = <3300000>;
        regulator-max-microvolt = <3300000>;

        gpio = <&tlmm 48 GPIO_ACTIVE_HIGH>;
        enable-active-high;

        pinctrl-names = "default";
        pinctrl-0 = <&wl_vreg_on_gpio>;
    };

    bcm43455_pwrseq: wifi-pwrseq {
        compatible = "mmc-pwrseq-simple";
        reset-gpios = <&pm8950_gpios 2 GPIO_ACTIVE_LOW>;
        pinctrl-names = "default";
        pinctrl-0 = <&wlan_sleep_pin>;
        post-power-on-delay-ms = <500>;
        power-off-delay-us = <500>;
    };
};

&sdhc_3 {
    status = "ok";

    max-frequency = <100000000>;
    keep-power-in-suspend;
    non-removable;
    wakeup-source;
    no-mmc;
    no-sd;

    pinctrl-names = "default", "sleep";
    pinctrl-0 = <&sdc3_clk_on &sdc3_cmd_on &sdc3_dat_on>;
    pinctrl-1 = <&sdc3_clk_off &sdc3_cmd_off &sdc3_dat_off>;

    vmmc-supply = <&bcm43455_pwr_vreg>;
    mmc-pwrseq = <&bcm43455_pwrseq>;

    brcmf: brcmf@1 {
        compatible = "brcm,bcm4329-fmac";
        reg = <1>;
        interrupt-parent = <&tlmm>;
        interrupts = <45 IRQ_TYPE_LEVEL_LOW>;
        interrupt-names = "host-wake";

        pinctrl-names = "default";
        pinctrl-0 = <&wl_host_wake_n_gpio>;
    };
};

&tlmm {
    wl_host_wake_n_gpio: wl-host-wake-n {
        pins = "gpio45";
        function = "gpio";
        drive-strength = <2>;
        bias-pull-down;
        input-enable;
    };

    wl_vreg_on_gpio: wl-vreg-on-gpio {
        pins = "gpio48";
        function = "gpio";
        drive-strength = <2>;
        bias-disable;
        output-high;
    };
};

The labels are different downstream so you'll have to substitute those, and use native pinctrl nodes instead of redefining for the same GPIOs. Good luck, let me know if anything is missing!

sonyxperiadev / bug_tracker

[Tone][Loire]Using qcwcn with brcmfmac #671