[RFC PATCH] brcmfmac: add 43751 SDIO ids and initialization

[Ulf Hansson]

On Thu, 9 Mar 2023 at 13:13, Marc Gonzalez <marc.w.gonzalez at free.fr> 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.

> >
> > 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?

>
>         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.

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".

>
> Confused again...
>
> Regards
>

Kind regards
Uffe