> On 16 Mar 2023, at 8:32 pm, Marc Gonzalez <marc.w.gonzalez@xxxxxxx> wrote:
>
> On 16/03/2023 12:59, Ulf Hansson wrote:
>
>> If the problem is regulator specific, this would be the correct thing to do.
>>
>> Although, if the problem is pwrseq specific, like that we need a delay
>> after enabling the clock and asserting the GPIO enable pin for the
>> WiFi chip, then we have the "post-power-on-delay-ms" of the pwrseq
>> node to play with instead.
>
> Heiner, Jerome, Uffe, Amlogic DT maintainers,
>
> Perhaps tweaking the driver for every user is too intrusive.
>
> Another solution would be to factorize all boards that implement
> the reference design SDIO-based RF (WiFi + BT using brcm).
>
> Then we can add post-power-on-delay-ms = 20 in sdio_pwrseq.
>
> NB: there's vddio_ao1v8 vs vddao_1v8 in sei510.
> Is my addition OK?
In LibreELEC we use a fairly minimal kernel defconfig; things are booting fast
compared to most desktop distro configs. I see occasional wireless issues with
cheap STB boards where SDIO speeds need to be reduced or UHS modes need to be
removed to keep things stable, but I don’t see SDIO probe issues with any of
the 30+ boards I do random testing with. The only SDIO drama in recent times
have been Heiner's IRQ changes (which are now resolved).
Are you using a real SEI510 board? ..I ask because we get lots of users with
random no-name Android boxes who try lots of dtb files until they come across
something that works; and there is a recurring trend where users have been
using SEI510 or SEI620 with poor results, and odd issues magically go away
once they are persuaded to use something else. So I have the impression those
specific device-trees aren’t well suited to typical reference design clones.
FWIW, I’ve also had thoughts about factoring aspects of WiFi support into a
few common dtsi files as there are increasing numbers of boxes/boards that
use Realtek or Qualcomm chips in circulation (due to post-covid chip supply
and pricing troubles). I would leave the sd_mmc_a node in the board files
since this is common (and in my experience works the same everywhere) while
moving the BT bits that need ‘compatible’ and GPIO details and the SDIO ID
poke and compatible (which isn’t really needed) to dtsi files. My goal would
be to make the copy/paste assembly of a new Android box dts even easier than
it already is.
If you don’t have an SEI510 board I have a hunch you’d be better off with
another device-tree as the source/base for your board. If you’re able to
share the schematics (privately if preferred) I’d be interested in seeing
what other dtb's could be used; and perhaps getting you to boot a LibreELEC
test image on your board to see whether my codebase gives same or different
results?
Christian
> RFC prototype:
>
> diff --git a/arch/arm64/boot/dts/amlogic/meson-g12a-radxa-zero.dts b/arch/arm64/boot/dts/amlogic/meson-g12a-radxa-zero.dts
> index e3bb6df42ff3e..42b5dcf358912 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-g12a-radxa-zero.dts
> +++ b/arch/arm64/boot/dts/amlogic/meson-g12a-radxa-zero.dts
> @@ -6,6 +6,7 @@
> /dts-v1/;
>
> #include "meson-g12a.dtsi"
> +#include "meson-g12a-ref-design-brcm-rf.dtsi"
> #include <dt-bindings/gpio/meson-g12a-gpio.h>
> #include <dt-bindings/sound/meson-g12a-tohdmitx.h>
>
> @@ -53,13 +54,6 @@ emmc_pwrseq: emmc-pwrseq {
> reset-gpios = <&gpio BOOT_12 GPIO_ACTIVE_LOW>;
> };
>
> - sdio_pwrseq: sdio-pwrseq {
> - compatible = "mmc-pwrseq-simple";
> - reset-gpios = <&gpio GPIOX_6 GPIO_ACTIVE_LOW>;
> - clocks = <&wifi32k>;
> - clock-names = "ext_clock";
> - };
> -
> ao_5v: regulator-ao_5v {
> compatible = "regulator-fixed";
> regulator-name = "AO_5V";
> @@ -182,13 +176,6 @@ codec {
> };
> };
> };
> -
> - wifi32k: wifi32k {
> - compatible = "pwm-clock";
> - #clock-cells = <0>;
> - clock-frequency = <32768>;
> - pwms = <&pwm_ef 0 30518 0>; /* PWM_E at 32.768KHz */
> - };
> };
>
> &arb {
> @@ -299,37 +286,6 @@ &saradc {
> vref-supply = <&vddao_1v8>;
> };
>
> -/* SDIO */
> -&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>;
> -
> - vmmc-supply = <&vddao_3v3>;
> - vqmmc-supply = <&vddao_1v8>;
> -
> - brcmf: wifi@1 {
> - reg = <1>;
> - compatible = "brcm,bcm4329-fmac";
> - };
> -};
> -
> /* SD card */
> &sd_emmc_b {
> status = "okay";
> diff --git a/arch/arm64/boot/dts/amlogic/meson-g12a-ref-design-brcm-rf.dtsi b/arch/arm64/boot/dts/amlogic/meson-g12a-ref-design-brcm-rf.dtsi
> new file mode 100644
> index 0000000000000..e462324596964
> --- /dev/null
> +++ b/arch/arm64/boot/dts/amlogic/meson-g12a-ref-design-brcm-rf.dtsi
> @@ -0,0 +1,53 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2018 Amlogic, Inc. All rights reserved.
> + */
> +
> +#include <dt-bindings/gpio/meson-g12a-gpio.h>
> +
> +/ {
> + 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 */
> + };
> +};
> +
> +/* SDIO */
> +&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>;
> +
> + vmmc-supply = <&vddao_3v3>;
> + vqmmc-supply = <&vddao_1v8>;
> +
> + brcmf: wifi@1 {
> + reg = <1>;
> + compatible = "brcm,bcm4329-fmac";
> + };
> +};
> diff --git a/arch/arm64/boot/dts/amlogic/meson-g12a-sei510.dts b/arch/arm64/boot/dts/amlogic/meson-g12a-sei510.dts
> index 23b790c6469d3..e12aeb956b7d7 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-g12a-sei510.dts
> +++ b/arch/arm64/boot/dts/amlogic/meson-g12a-sei510.dts
> @@ -6,6 +6,7 @@
> /dts-v1/;
>
> #include "meson-g12a.dtsi"
> +#include "meson-g12a-ref-design-brcm-rf.dtsi"
> #include <dt-bindings/gpio/gpio.h>
> #include <dt-bindings/input/input.h>
> #include <dt-bindings/gpio/meson-g12a-gpio.h>
> @@ -96,6 +97,15 @@ emmc_1v8: regulator-emmc_1v8 {
> regulator-always-on;
> };
>
> + vddao_1v8: regulator-vddao_1v8 {
> + compatible = "regulator-fixed";
> + regulator-name = "VDDAO_1V8";
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + vin-supply = <&vddao_3v3>;
> + regulator-always-on;
> + };
> +
> vddao_3v3: regulator-vddao_3v3 {
> compatible = "regulator-fixed";
> regulator-name = "VDDAO_3V3";
> @@ -143,20 +153,6 @@ vddio_ao1v8: regulator-vddio_ao1v8 {
> regulator-always-on;
> };
>
> - 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 */
> - };
> -
> sound {
> compatible = "amlogic,axg-sound-card";
> model = "SEI510";
> @@ -375,37 +371,6 @@ &saradc {
> vref-supply = <&vddio_ao1v8>;
> };
>
> -/* SDIO */
> -&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>;
> -
> - vmmc-supply = <&vddao_3v3>;
> - vqmmc-supply = <&vddio_ao1v8>;
> -
> - brcmf: wifi@1 {
> - reg = <1>;
> - compatible = "brcm,bcm4329-fmac";
> - };
> -};
> -
> /* SD card */
> &sd_emmc_b {
> status = "okay";
> diff --git a/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts b/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts
> index b2bb94981838f..68a8876386115 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts
> +++ b/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts
> @@ -6,6 +6,7 @@
> /dts-v1/;
>
> #include "meson-g12a.dtsi"
> +#include "meson-g12a-ref-design-brcm-rf.dtsi"
> #include <dt-bindings/gpio/gpio.h>
> #include <dt-bindings/gpio/meson-g12a-gpio.h>
> #include <dt-bindings/sound/meson-g12a-tohdmitx.h>
> @@ -60,13 +61,6 @@ emmc_pwrseq: emmc-pwrseq {
> reset-gpios = <&gpio BOOT_12 GPIO_ACTIVE_LOW>;
> };
>
> - sdio_pwrseq: sdio-pwrseq {
> - compatible = "mmc-pwrseq-simple";
> - reset-gpios = <&gpio GPIOX_6 GPIO_ACTIVE_LOW>;
> - clocks = <&wifi32k>;
> - clock-names = "ext_clock";
> - };
> -
> flash_1v8: regulator-flash_1v8 {
> compatible = "regulator-fixed";
> regulator-name = "FLASH_1V8";
> @@ -226,13 +220,6 @@ codec {
> };
> };
> };
> -
> - wifi32k: wifi32k {
> - compatible = "pwm-clock";
> - #clock-cells = <0>;
> - clock-frequency = <32768>;
> - pwms = <&pwm_ef 0 30518 0>; /* PWM_E at 32.768KHz */
> - };
> };
>
> &arb {
> @@ -391,37 +378,6 @@ &usb {
> dr_mode = "host";
> };
>
> -/* SDIO */
> -&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>;
> -
> - vmmc-supply = <&vddao_3v3>;
> - vqmmc-supply = <&vddao_1v8>;
> -
> - brcmf: wifi@1 {
> - reg = <1>;
> - compatible = "brcm,bcm4329-fmac";
> - };
> -};
> -
> /* SD card */
> &sd_emmc_b {
> status = "okay";
>
>
> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic@xxxxxxxxxxxxxxxxxxx
> http://lists.infradead.org/mailman/listinfo/linux-amlogic
