[RFC PATCH] brcmfmac: add 43751 SDIO ids and initialization
Marc Gonzalez
marc.w.gonzalez at free.fr
Thu Mar 9 08:45:12 PST 2023
On 09/03/2023 15:29, Ulf Hansson wrote:
> On Thu, 9 Mar 2023 at 13:13, Marc Gonzalez wrote:
>
>> On 09/03/2023 11:16, Marc Gonzalez wrote:
>>
>>> On 06/03/2023 11:24, Marc Gonzalez wrote:
>>>
>>>> # cat /sys/bus/sdio/devices/mmc2:0001:1/uevent
>>>> OF_NAME=wifi
>>>> OF_FULLNAME=/soc/sd at ffe03000/wifi at 1
>>>> OF_COMPATIBLE_0=brcm,bcm4329-fmac
>>>> OF_COMPATIBLE_N=1
>>>> SDIO_CLASS=00
>>>> SDIO_ID=02D0:AAE7
>>>> SDIO_REVISION=0.0
>>>> MODALIAS=sdio:c00v02D0dAAE7
>>>>
>>>> NB: 0xaae7 = 43751
>>>
>>> I have run into another issue.
>>>
>>> The WiFi device (and the mmc2 bus it sits on) don't show up at all
>>> in the kernel log *unless* I add lots of debug output, such as with
>>> #define DEBUG in drivers/base/dd.c
>>>
>>> I think this points to some kind of race condition?
>>>
>>> Neil suggested that maybe the host probes the mmc2 bus "too soon",
>>> when the WiFi device is still powering up, which makes the entire
>>> probe fail.
>
> Ideally, the WiFi device/driver should not need to be initialized to
> allow the SDIO card to be detected properly. Looks like there is
> something fishy going on.
Something fishy indeed ;)
>>> This patch appears to solve the symptom:
>>>
>>> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
>>> index 6e5ea0213b477..999b3843c0d0b 100644
>>> --- a/drivers/mmc/host/meson-gx-mmc.c
>>> +++ b/drivers/mmc/host/meson-gx-mmc.c
>>> @@ -1400,7 +1400,7 @@ static struct platform_driver meson_mmc_driver = {
>>> .remove = meson_mmc_remove,
>>> .driver = {
>>> .name = DRIVER_NAME,
>>> - .probe_type = PROBE_PREFER_ASYNCHRONOUS,
>>> + .probe_type = PROBE_FORCE_SYNCHRONOUS,
>>> .of_match_table = meson_mmc_of_match,
>>> },
>>> };
>>>
>>> But this might just be delaying the probe enough for the device
>>> to become ready?
>>
>> FWIW, the relevant device tree nodes are:
>>
>> /* decompiled DTS */
>>
>> sd at ffe03000 {
>> compatible = "amlogic,meson-axg-mmc";
>> reg = <0x0 0xffe03000 0x0 0x800>;
>> interrupts = <0x0 0xbd 0x4>;
>> status = "okay";
>> clocks = <0x2 0x21 0x2 0x3c 0x2 0x2>;
>> clock-names = "core", "clkin0", "clkin1";
>> resets = <0x5 0x2c>;
>> amlogic,dram-access-quirk;
>> pinctrl-0 = <0x2c>;
>> pinctrl-1 = <0x2d>;
>> pinctrl-names = "default", "clk-gate";
>> #address-cells = <0x1>;
>> #size-cells = <0x0>;
>> bus-width = <0x4>;
>> cap-sd-highspeed;
>> sd-uhs-sdr50;
>> max-frequency = <0x5f5e100>;
>> non-removable;
>> disable-wp;
>> keep-power-in-suspend;
>> mmc-pwrseq = <0x2e>;
>> vmmc-supply = <0x2b>;
>> vqmmc-supply = <0x21>;
>>
>> wifi at 1 {
>> reg = <0x1>;
>> compatible = "brcm,bcm4329-fmac";
>> };
>> };
>>
>>
>> /* original DTS */
>>
>> sd_emmc_a: sd at ffe03000 {
>> compatible = "amlogic,meson-axg-mmc";
>> reg = <0x0 0xffe03000 0x0 0x800>;
>> interrupts = <GIC_SPI 189 IRQ_TYPE_LEVEL_HIGH>;
>> status = "disabled";
>> clocks = <&clkc CLKID_SD_EMMC_A>,
>> <&clkc CLKID_SD_EMMC_A_CLK0>,
>> <&clkc CLKID_FCLK_DIV2>;
>> clock-names = "core", "clkin0", "clkin1";
>> resets = <&reset RESET_SD_EMMC_A>;
>> };
>>
>> &sd_emmc_a {
>> status = "okay";
>> pinctrl-0 = <&sdio_pins>;
>> pinctrl-1 = <&sdio_clk_gate_pins>;
>> pinctrl-names = "default", "clk-gate";
>> #address-cells = <1>;
>> #size-cells = <0>;
>>
>> bus-width = <4>;
>> cap-sd-highspeed;
>> sd-uhs-sdr50;
>> max-frequency = <100000000>;
>>
>> non-removable;
>> disable-wp;
>>
>> /* WiFi firmware requires power to be kept while in suspend */
>> keep-power-in-suspend;
>>
>> mmc-pwrseq = <&sdio_pwrseq>;
>
> This one is particularly interesting. Can you share the content of the
> sdio_pwrseq node too?
sdio_pwrseq: sdio-pwrseq {
compatible = "mmc-pwrseq-simple";
reset-gpios = <&gpio GPIOX_6 GPIO_ACTIVE_LOW>;
clocks = <&wifi32k>;
clock-names = "ext_clock";
};
wifi32k: wifi32k {
compatible = "pwm-clock";
#clock-cells = <0>;
clock-frequency = <32768>;
pwms = <&pwm_ef 0 30518 0>; /* PWM_E at 32.768KHz */
};
pwm_ef: pwm at 19000 {
compatible = "amlogic,meson-g12a-ee-pwm";
reg = <0x0 0x19000 0x0 0x20>;
#pwm-cells = <3>;
status = "disabled";
};
&pwm_ef {
status = "okay";
pinctrl-0 = <&pwm_e_pins>;
pinctrl-names = "default";
clocks = <&xtal>;
clock-names = "clkin0";
};
pwm_e_pins: pwm-e {
mux {
groups = "pwm_e";
function = "pwm_e";
bias-disable;
};
};
>> vmmc-supply = <&vddao_3v3>;
>> vqmmc-supply = <&vddio_ao1v8>;
>>
>> brcmf: wifi at 1 {
>> reg = <1>;
>> compatible = "brcm,bcm4329-fmac";
>> };
>> };
>>
>> With an asynchronous probe, meson_mmc_probe() always succeeds,
>> yet the WiFi card is not detected later on, even if I sleep
>> 1-2 seconds in meson_mmc_probe().
>>
>> [ 0.879756] YO: meson_mmc_probe: ffe03000.sd
>> [ 0.914320] YO: meson_mmc_probe: ffe03000.sd ALL OK
>> [ 1.199170] YO: meson_mmc_probe: ffe07000.mmc
>> [ 1.232734] YO: meson_mmc_probe: ffe07000.mmc ALL OK
>
> To narrow down the problem, I would start by preventing the WiFi
> driver from being insmoded. To make sure it doesn't affect the SDIO
> card detection process.
So far, I always embedded all device drivers in the kernel (=y)
You are suggesting to build the WiFi driver as a module, correct?
> The point is, the SDIO card should be detected properly, no matter
> whether there is a corresponding SDIO func driver (WiFi driver)
> available for it. For a detected SDIO/eMMC/SD card, mmc_add_card()
> prints a message about the card in the log during initialization. It
> could look like the below print, for example:
>
> "mmc2: new ultra high speed SDR104 SDIO card at address 0001".
When everything works, I get:
mmc2: new ultra high speed SDR50 SDIO card at address 0001
When it doesn't, I don't get that line.
I'm trying to debug with printk's in
mmc_rescan_try_freq, mmc_rescan, mmc_attach_sdio, meson_mmc_probe
The problem is that adding "too many" printks makes the
probe work! (Debugging this issue is nerve-racking.)
Thanks, I will test your suggestions!
Regards
More information about the linux-amlogic
mailing list