All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Heiko Stübner" <heiko@sntech.de>
To: linux-rockchip@lists.infradead.org, Hao Zhang <hao.zhang@coolkit.cn>
Cc: devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	conor+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org,
	robh+dt@kernel.org, linux-kernel@vger.kernel.org,
	"hao.zhang" <hao.zhang@coolkit.cn>
Subject: Re: [PATCH 1/1] ARM: dts: rockchip: Wifi improvements for Sonoff iHost
Date: Sun, 27 Apr 2025 13:14:58 +0200	[thread overview]
Message-ID: <1929240.tdWV9SEqCh@diego> (raw)
In-Reply-To: <20250427065013.99871-2-hao.zhang@coolkit.cn>

Hi,

Am Sonntag, 27. April 2025, 08:50:13 Mitteleuropäische Sommerzeit schrieb Hao Zhang:
> From: "hao.zhang" <hao.zhang@coolkit.cn>
> 
> After some Sonoff-iHosts have been running for a long time, 
> the WiFi module will run abnormally.
> 
> Adjust the pmu_io_domains and sdio properties 
> to solve the WiFi module operation abnormality.

"adjust the ... properties", really sounds like hacking around some issue.

> diff --git a/arch/arm/boot/dts/rockchip/rv1126-sonoff-ihost.dtsi b/arch/arm/boot/dts/rockchip/rv1126-sonoff-ihost.dtsi
> index 9a87dc0d5f66..3c0371103015 100644
> --- a/arch/arm/boot/dts/rockchip/rv1126-sonoff-ihost.dtsi
> +++ b/arch/arm/boot/dts/rockchip/rv1126-sonoff-ihost.dtsi
> @@ -323,15 +323,15 @@ wifi_enable_h: wifi-enable-h {
>  };
>  
>  &pmu_io_domains {
> -	pmuio0-supply = <&vcc1v8_pmu>;
> +	pmuio0-supply = <&vcc3v3_sys>;
>  	pmuio1-supply = <&vcc3v3_sys>;
>  	vccio1-supply = <&vcc_1v8>;
>  	vccio2-supply = <&vccio_sd>;
> -	vccio3-supply = <&vcc3v3_sd>;
> -	vccio4-supply = <&vcc_dovdd>;
> -	vccio5-supply = <&vcc_1v8>;
> -	vccio6-supply = <&vcc_1v8>;
> -	vccio7-supply = <&vcc_dovdd>;
> +	vccio3-supply = <&vcc_3v3>;
> +	vccio4-supply = <&vcc_3v3>;
> +	vccio5-supply = <&vcc_3v3>;
> +	vccio6-supply = <&vcc_3v3>;
> +	vccio7-supply = <&vcc_1v8>;
>  	status = "okay";
>  };

First of all, this would be two patches.  If the io-domains do not follow
the schematics, fixing this is one patch, but for such a big change
I do expect actual references to the devices' schematics for that.

This is even more important, as you're switching some supplies
between sources of different voltages


> @@ -342,18 +342,15 @@ &saradc {
>  
>  &sdio {
>  	bus-width = <4>;
> -	cap-sd-highspeed;
>  	cap-sdio-irq;
>  	keep-power-in-suspend;
> -	max-frequency = <50000000>;
> +	max-frequency = <25000000>;
>  	mmc-pwrseq = <&sdio_pwrseq>;
> +	supports-sdio;
>  	non-removable;
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&sdmmc1_clk &sdmmc1_cmd &sdmmc1_bus4>;
>  	rockchip,default-sample-phase = <90>;
> -	sd-uhs-sdr50;
> -	vmmc-supply = <&vcc3v3_sd>;
> -	vqmmc-supply = <&vcc_1v8>;
>  	status = "okay";
>  };

and here it looks like you're more or less randomly adding and removing
properties until it worked "for you".

Especially removing the supply-regulators does not really make sense.
If you see instabilities, the main contenders would be max-frequency and
sd-uhs-sdr50 as culprits.

Similarly, supports-sdio is not even a valid property, so neither the
devicetree spec does allow it, nor does the kernel handle it at all.


Heiko




WARNING: multiple messages have this Message-ID (diff)
From: "Heiko Stübner" <heiko@sntech.de>
To: linux-rockchip@lists.infradead.org, Hao Zhang <hao.zhang@coolkit.cn>
Cc: devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	conor+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org,
	robh+dt@kernel.org, linux-kernel@vger.kernel.org,
	"hao.zhang" <hao.zhang@coolkit.cn>
Subject: Re: [PATCH 1/1] ARM: dts: rockchip: Wifi improvements for Sonoff iHost
Date: Sun, 27 Apr 2025 13:14:58 +0200	[thread overview]
Message-ID: <1929240.tdWV9SEqCh@diego> (raw)
In-Reply-To: <20250427065013.99871-2-hao.zhang@coolkit.cn>

Hi,

Am Sonntag, 27. April 2025, 08:50:13 Mitteleuropäische Sommerzeit schrieb Hao Zhang:
> From: "hao.zhang" <hao.zhang@coolkit.cn>
> 
> After some Sonoff-iHosts have been running for a long time, 
> the WiFi module will run abnormally.
> 
> Adjust the pmu_io_domains and sdio properties 
> to solve the WiFi module operation abnormality.

"adjust the ... properties", really sounds like hacking around some issue.

> diff --git a/arch/arm/boot/dts/rockchip/rv1126-sonoff-ihost.dtsi b/arch/arm/boot/dts/rockchip/rv1126-sonoff-ihost.dtsi
> index 9a87dc0d5f66..3c0371103015 100644
> --- a/arch/arm/boot/dts/rockchip/rv1126-sonoff-ihost.dtsi
> +++ b/arch/arm/boot/dts/rockchip/rv1126-sonoff-ihost.dtsi
> @@ -323,15 +323,15 @@ wifi_enable_h: wifi-enable-h {
>  };
>  
>  &pmu_io_domains {
> -	pmuio0-supply = <&vcc1v8_pmu>;
> +	pmuio0-supply = <&vcc3v3_sys>;
>  	pmuio1-supply = <&vcc3v3_sys>;
>  	vccio1-supply = <&vcc_1v8>;
>  	vccio2-supply = <&vccio_sd>;
> -	vccio3-supply = <&vcc3v3_sd>;
> -	vccio4-supply = <&vcc_dovdd>;
> -	vccio5-supply = <&vcc_1v8>;
> -	vccio6-supply = <&vcc_1v8>;
> -	vccio7-supply = <&vcc_dovdd>;
> +	vccio3-supply = <&vcc_3v3>;
> +	vccio4-supply = <&vcc_3v3>;
> +	vccio5-supply = <&vcc_3v3>;
> +	vccio6-supply = <&vcc_3v3>;
> +	vccio7-supply = <&vcc_1v8>;
>  	status = "okay";
>  };

First of all, this would be two patches.  If the io-domains do not follow
the schematics, fixing this is one patch, but for such a big change
I do expect actual references to the devices' schematics for that.

This is even more important, as you're switching some supplies
between sources of different voltages


> @@ -342,18 +342,15 @@ &saradc {
>  
>  &sdio {
>  	bus-width = <4>;
> -	cap-sd-highspeed;
>  	cap-sdio-irq;
>  	keep-power-in-suspend;
> -	max-frequency = <50000000>;
> +	max-frequency = <25000000>;
>  	mmc-pwrseq = <&sdio_pwrseq>;
> +	supports-sdio;
>  	non-removable;
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&sdmmc1_clk &sdmmc1_cmd &sdmmc1_bus4>;
>  	rockchip,default-sample-phase = <90>;
> -	sd-uhs-sdr50;
> -	vmmc-supply = <&vcc3v3_sd>;
> -	vqmmc-supply = <&vcc_1v8>;
>  	status = "okay";
>  };

and here it looks like you're more or less randomly adding and removing
properties until it worked "for you".

Especially removing the supply-regulators does not really make sense.
If you see instabilities, the main contenders would be max-frequency and
sd-uhs-sdr50 as culprits.

Similarly, supports-sdio is not even a valid property, so neither the
devicetree spec does allow it, nor does the kernel handle it at all.


Heiko



_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

  reply	other threads:[~2025-04-27 11:18 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-27  6:50 ARM: dts: rockchip: Wifi improvements for Sonoff iHost Hao Zhang
2025-04-27  6:50 ` Hao Zhang
2025-04-27  6:50 ` [PATCH 1/1] " Hao Zhang
2025-04-27  6:50   ` Hao Zhang
2025-04-27 11:14   ` Heiko Stübner [this message]
2025-04-27 11:14     ` Heiko Stübner
  -- strict thread matches above, loose matches on Subject: below --
2025-04-30  0:23 kernel test robot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1929240.tdWV9SEqCh@diego \
    --to=heiko@sntech.de \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=hao.zhang@coolkit.cn \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=robh+dt@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.