[PATCH] arm64: dts: rockchip: add rfkill node for M.2 E wifi on orangepi-5-plus

[PATCH] arm64: dts: rockchip: add rfkill node for M.2 E wifi on orangepi-5-plus

[Florian Klink]

This follows the same logic as 82d40b141a4c7ab66, which did the same
for rock-5b.

The GPIO pin connecting the WiFi enable signal inside the M.2 Key E slot
is the same as there.

This can be validated in
arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dtsi from Armbian
rk-5.10-rkr4 kernel, which contains a `wifi_disable` node with that same
GPIO pin.

Signed-off-by: Florian Klink <flokli@flokli.de>
Tested-by: Florian Klink <flokli@flokli.de>
---
 arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts b/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts
index e74871491ef5..c3a6812cc93a 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts
@@ -105,6 +105,13 @@ led {
 		};
 	};
 
+	rfkill {
+		compatible = "rfkill-gpio";
+		label = "rfkill-pcie-wlan";
+		radio-type = "wlan";
+		shutdown-gpios = <&gpio0 RK_PC4 GPIO_ACTIVE_HIGH>;
+	};
+
 	sound {
 		compatible = "simple-audio-card";
 		pinctrl-names = "default";
-- 
2.45.2

Re: [PATCH] arm64: dts: rockchip: add rfkill node for M.2 E wifi on orangepi-5-plus

[Dragan Simic]

Hello Florian,

On 2024-08-07 18:20, Florian Klink wrote:
> This follows the same logic as 82d40b141a4c7ab66, which did the same
> for rock-5b.

Please use the standard format for referencing commits, i.e. 
82d40b141a4c
("arm64: dts: rockchip: add rfkill node for M.2 Key E WiFi on rock-5b").

> The GPIO pin connecting the WiFi enable signal inside the M.2 Key E 
> slot
> is the same as there.
> 
> This can be validated in
> arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dtsi from Armbian
> rk-5.10-rkr4 kernel, which contains a `wifi_disable` node with that 
> same
> GPIO pin.

This might be used as a reference, but please provide a permanent link
to the actual source code in the referenced downstream repository.

> Signed-off-by: Florian Klink <flokli@flokli.de>
> Tested-by: Florian Klink <flokli@flokli.de>
> ---
>  arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts
> b/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts
> index e74871491ef5..c3a6812cc93a 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts
> @@ -105,6 +105,13 @@ led {
>  		};
>  	};
> 
> +	rfkill {
> +		compatible = "rfkill-gpio";
> +		label = "rfkill-pcie-wlan";
> +		radio-type = "wlan";
> +		shutdown-gpios = <&gpio0 RK_PC4 GPIO_ACTIVE_HIGH>;
> +	};
> +
>  	sound {
>  		compatible = "simple-audio-card";
>  		pinctrl-names = "default";

[PATCH v2] arm64: dts: rockchip: add rfkill node for M.2 E wifi on orangepi-5-plus

[Florian Klink]

This follows the same logic as 82d40b141a4c ("arm64: dts: rockchip: add
rfkill node for M.2 Key E WiFi on rock-5b").

On the orangepi-5-plus, there's also a GPIO pin connecting the WiFi
enable signal inside the M.2 Key E slot.

The exact GPIO PIN can be validated in the Armbian rk-5.10-rkr4 kernel
rk3588-orangepi-5-plus.dtsi file [1], which contains a `wifi_disable`
node referencing RK_PC4 on &gpio0.

Signed-off-by: Florian Klink <flokli@flokli.de>
Tested-by: Florian Klink <flokli@flokli.de>
Link: https://github.com/armbian/linux-rockchip/blob/9fbe23c9da24f236c6009f42d3f02c1ffb84c169/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts [1]
---
 arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts b/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts
index e74871491ef5..c3a6812cc93a 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts
@@ -105,6 +105,13 @@ led {
 		};
 	};
 
+	rfkill {
+		compatible = "rfkill-gpio";
+		label = "rfkill-pcie-wlan";
+		radio-type = "wlan";
+		shutdown-gpios = <&gpio0 RK_PC4 GPIO_ACTIVE_HIGH>;
+	};
+
 	sound {
 		compatible = "simple-audio-card";
 		pinctrl-names = "default";
-- 
2.45.2

Re: [PATCH v2] arm64: dts: rockchip: add rfkill node for M.2 E wifi on orangepi-5-plus

[Dragan Simic]

On 2024-08-07 19:00, Florian Klink wrote:
> This follows the same logic as 82d40b141a4c ("arm64: dts: rockchip: add
> rfkill node for M.2 Key E WiFi on rock-5b").
> 
> On the orangepi-5-plus, there's also a GPIO pin connecting the WiFi
> enable signal inside the M.2 Key E slot.
> 
> The exact GPIO PIN can be validated in the Armbian rk-5.10-rkr4 kernel
> rk3588-orangepi-5-plus.dtsi file [1], which contains a `wifi_disable`
> node referencing RK_PC4 on &gpio0.
> 
> Signed-off-by: Florian Klink <flokli@flokli.de>
> Tested-by: Florian Klink <flokli@flokli.de>
> Link:
> https://github.com/armbian/linux-rockchip/blob/9fbe23c9da24f236c6009f42d3f02c1ffb84c169/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts
> [1]

Unfortunately, this isn't how the "Link: ..." tag is to be used, or how
a reference is to be provided.  Please see the patch submission linked
below for a correct example of providing links as references.

https://lore.kernel.org/linux-rockchip/4449f7d4eead787308300e2d1d37b88c9d1446b2.1717308862.git.dsimic@manjaro.org/T/#u

> ---
>  arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts
> b/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts
> index e74871491ef5..c3a6812cc93a 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts
> @@ -105,6 +105,13 @@ led {
>  		};
>  	};
> 
> +	rfkill {
> +		compatible = "rfkill-gpio";
> +		label = "rfkill-pcie-wlan";
> +		radio-type = "wlan";
> +		shutdown-gpios = <&gpio0 RK_PC4 GPIO_ACTIVE_HIGH>;
> +	};
> +
>  	sound {
>  		compatible = "simple-audio-card";
>  		pinctrl-names = "default";

Re: [PATCH v2] arm64: dts: rockchip: add rfkill node for M.2 E wifi on orangepi-5-plus

[Heiko Stübner]

Hi Florian,

Am Mittwoch, 7. August 2024, 19:15:03 CEST schrieb Dragan Simic:
> On 2024-08-07 19:00, Florian Klink wrote:
> > This follows the same logic as 82d40b141a4c ("arm64: dts: rockchip: add
> > rfkill node for M.2 Key E WiFi on rock-5b").
> > 
> > On the orangepi-5-plus, there's also a GPIO pin connecting the WiFi
> > enable signal inside the M.2 Key E slot.
> > 
> > The exact GPIO PIN can be validated in the Armbian rk-5.10-rkr4 kernel
> > rk3588-orangepi-5-plus.dtsi file [1], which contains a `wifi_disable`
> > node referencing RK_PC4 on &gpio0.
> > 
> > Signed-off-by: Florian Klink <flokli@flokli.de>
> > Tested-by: Florian Klink <flokli@flokli.de>
> > Link:
> > https://github.com/armbian/linux-rockchip/blob/9fbe23c9da24f236c6009f42d3f02c1ffb84c169/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts
> > [1]
> 
> Unfortunately, this isn't how the "Link: ..." tag is to be used, or how
> a reference is to be provided.  Please see the patch submission linked
> below for a correct example of providing links as references.
> 
> https://lore.kernel.org/linux-rockchip/4449f7d4eead787308300e2d1d37b88c9d1446b2.1717308862.git.dsimic@manjaro.org/T/#u

please also don't post v2 patches as replies to v1.
Instead start a new mail thread please.

A lot of tooling cannot really find the correct version in such
multiversion threads.

Heiko

Re: [PATCH v2] arm64: dts: rockchip: add rfkill node for M.2 E wifi on orangepi-5-plus

[Florian Klink]

Hey,

On Wed, Aug 07, 2024 at 07:17:49PM GMT, Heiko Stübner wrote:
>Hi Florian,
>
>Am Mittwoch, 7. August 2024, 19:15:03 CEST schrieb Dragan Simic:
>> On 2024-08-07 19:00, Florian Klink wrote:
>> > This follows the same logic as 82d40b141a4c ("arm64: dts: rockchip: add
>> > rfkill node for M.2 Key E WiFi on rock-5b").
>> >
>> > On the orangepi-5-plus, there's also a GPIO pin connecting the WiFi
>> > enable signal inside the M.2 Key E slot.
>> >
>> > The exact GPIO PIN can be validated in the Armbian rk-5.10-rkr4 kernel
>> > rk3588-orangepi-5-plus.dtsi file [1], which contains a `wifi_disable`
>> > node referencing RK_PC4 on &gpio0.
>> >
>> > Signed-off-by: Florian Klink <flokli@flokli.de>
>> > Tested-by: Florian Klink <flokli@flokli.de>
>> > Link:
>> > https://github.com/armbian/linux-rockchip/blob/9fbe23c9da24f236c6009f42d3f02c1ffb84c169/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts
>> > [1]
>>
>> Unfortunately, this isn't how the "Link: ..." tag is to be used, or how
>> a reference is to be provided.  Please see the patch submission linked
>> below for a correct example of providing links as references.
>>
>> https://lore.kernel.org/linux-rockchip/4449f7d4eead787308300e2d1d37b88c9d1446b2.1717308862.git.dsimic@manjaro.org/T/#u
>
>please also don't post v2 patches as replies to v1.
>Instead start a new mail thread please.
>
>A lot of tooling cannot really find the correct version in such
>multiversion threads.

sorry for the noise. I sent a v3, addressing the requested changes, as a
new thread.

Somewhat offtopic for this patch, but it'd be great if
process/submitting-patches.html could include:

  - A mention of the kernel quotation style for commit ids and subjects
	 and how to produce them
  - A styleguide for how to link to references
  - An active discouragement from using --in-reply-to for v2 (which
	 differs from what `git send-email` proposes).

Florian

Re: [PATCH v2] arm64: dts: rockchip: add rfkill node for M.2 E wifi on orangepi-5-plus

[Dragan Simic]

On 2024-08-07 19:28, Florian Klink wrote:
> On Wed, Aug 07, 2024 at 07:17:49PM GMT, Heiko Stübner wrote:
>> Am Mittwoch, 7. August 2024, 19:15:03 CEST schrieb Dragan Simic:
>>> On 2024-08-07 19:00, Florian Klink wrote:
>>> > This follows the same logic as 82d40b141a4c ("arm64: dts: rockchip: add
>>> > rfkill node for M.2 Key E WiFi on rock-5b").
>>> >
>>> > On the orangepi-5-plus, there's also a GPIO pin connecting the WiFi
>>> > enable signal inside the M.2 Key E slot.
>>> >
>>> > The exact GPIO PIN can be validated in the Armbian rk-5.10-rkr4 kernel
>>> > rk3588-orangepi-5-plus.dtsi file [1], which contains a `wifi_disable`
>>> > node referencing RK_PC4 on &gpio0.
>>> >
>>> > Signed-off-by: Florian Klink <flokli@flokli.de>
>>> > Tested-by: Florian Klink <flokli@flokli.de>
>>> > Link:
>>> > https://github.com/armbian/linux-rockchip/blob/9fbe23c9da24f236c6009f42d3f02c1ffb84c169/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts
>>> > [1]
>>> 
>>> Unfortunately, this isn't how the "Link: ..." tag is to be used, or 
>>> how
>>> a reference is to be provided.  Please see the patch submission 
>>> linked
>>> below for a correct example of providing links as references.
>>> 
>>> https://lore.kernel.org/linux-rockchip/4449f7d4eead787308300e2d1d37b88c9d1446b2.1717308862.git.dsimic@manjaro.org/T/#u
>> 
>> please also don't post v2 patches as replies to v1.
>> Instead start a new mail thread please.
>> 
>> A lot of tooling cannot really find the correct version in such
>> multiversion threads.
> 
> sorry for the noise. I sent a v3, addressing the requested changes, as 
> a
> new thread.
> 
> Somewhat offtopic for this patch, but it'd be great if
> process/submitting-patches.html could include:
> 
>  - A mention of the kernel quotation style for commit ids and subjects
> 	 and how to produce them
>  - A styleguide for how to link to references
>  - An active discouragement from using --in-reply-to for v2 (which
> 	 differs from what `git send-email` proposes).

I'd suggest that you go ahead and whip up a new patch that adds those
specific instructions to the documentation.

Re: [PATCH v2] arm64: dts: rockchip: add rfkill node for M.2 E wifi on orangepi-5-plus

[Dragan Simic]

On 2024-08-07 19:00, Florian Klink wrote:
> This follows the same logic as 82d40b141a4c ("arm64: dts: rockchip: add
> rfkill node for M.2 Key E WiFi on rock-5b").
> 
> On the orangepi-5-plus, there's also a GPIO pin connecting the WiFi
> enable signal inside the M.2 Key E slot.
> 
> The exact GPIO PIN can be validated in the Armbian rk-5.10-rkr4 kernel
> rk3588-orangepi-5-plus.dtsi file [1], which contains a `wifi_disable`
> node referencing RK_PC4 on &gpio0.
> 
> Signed-off-by: Florian Klink <flokli@flokli.de>
> Tested-by: Florian Klink <flokli@flokli.de>

I forgot to mention that providing a Tested-by tag is redundant when
there's already a Signed-off-by tag, because the latter already implies
the former.

> Link:
> https://github.com/armbian/linux-rockchip/blob/9fbe23c9da24f236c6009f42d3f02c1ffb84c169/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts
> [1]
> ---
>  arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts
> b/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts
> index e74871491ef5..c3a6812cc93a 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts
> @@ -105,6 +105,13 @@ led {
>  		};
>  	};
> 
> +	rfkill {
> +		compatible = "rfkill-gpio";
> +		label = "rfkill-pcie-wlan";
> +		radio-type = "wlan";
> +		shutdown-gpios = <&gpio0 RK_PC4 GPIO_ACTIVE_HIGH>;
> +	};
> +
>  	sound {
>  		compatible = "simple-audio-card";
>  		pinctrl-names = "default";

Re: [PATCH v2] arm64: dts: rockchip: add rfkill node for M.2 E wifi on orangepi-5-plus

[Florian Klink]

On Wed, Aug 07, 2024 at 07:24:27PM GMT, Dragan Simic wrote:
>On 2024-08-07 19:00, Florian Klink wrote:
>>This follows the same logic as 82d40b141a4c ("arm64: dts: rockchip: add
>>rfkill node for M.2 Key E WiFi on rock-5b").
>>
>>On the orangepi-5-plus, there's also a GPIO pin connecting the WiFi
>>enable signal inside the M.2 Key E slot.
>>
>>The exact GPIO PIN can be validated in the Armbian rk-5.10-rkr4 kernel
>>rk3588-orangepi-5-plus.dtsi file [1], which contains a `wifi_disable`
>>node referencing RK_PC4 on &gpio0.
>>
>>Signed-off-by: Florian Klink <flokli@flokli.de>
>>Tested-by: Florian Klink <flokli@flokli.de>
>
>I forgot to mention that providing a Tested-by tag is redundant when
>there's already a Signed-off-by tag, because the latter already implies
>the former.

This came after I sent the v3. Generally I wish people would test things
- though too often it's not. I explicitly tested this to work (with a
wifi module added to that slot being unblock-able afterwards), and
wanted to point that out, thus adding the Tested-by.

DCO 1.1 doesn't say anything about Tested-by, it's mostly legalese about
being allowed to send out the patch, and understanding the consequences
regarding licensing. It doesn't require the person adding their
Signed-Off-By to have tested it.

Florian

Re: [PATCH v2] arm64: dts: rockchip: add rfkill node for M.2 E wifi on orangepi-5-plus

[Heiko Stübner]

Am Mittwoch, 7. August 2024, 20:14:24 CEST schrieb Florian Klink:
> On Wed, Aug 07, 2024 at 07:24:27PM GMT, Dragan Simic wrote:
> >On 2024-08-07 19:00, Florian Klink wrote:
> >>This follows the same logic as 82d40b141a4c ("arm64: dts: rockchip: add
> >>rfkill node for M.2 Key E WiFi on rock-5b").
> >>
> >>On the orangepi-5-plus, there's also a GPIO pin connecting the WiFi
> >>enable signal inside the M.2 Key E slot.
> >>
> >>The exact GPIO PIN can be validated in the Armbian rk-5.10-rkr4 kernel
> >>rk3588-orangepi-5-plus.dtsi file [1], which contains a `wifi_disable`
> >>node referencing RK_PC4 on &gpio0.
> >>
> >>Signed-off-by: Florian Klink <flokli@flokli.de>
> >>Tested-by: Florian Klink <flokli@flokli.de>
> >
> >I forgot to mention that providing a Tested-by tag is redundant when
> >there's already a Signed-off-by tag, because the latter already implies
> >the former.
> 
> This came after I sent the v3. Generally I wish people would test things
> - though too often it's not. I explicitly tested this to work (with a
> wifi module added to that slot being unblock-able afterwards), and
> wanted to point that out, thus adding the Tested-by.
> 
> DCO 1.1 doesn't say anything about Tested-by, it's mostly legalese about
> being allowed to send out the patch, and understanding the consequences
> regarding licensing. It doesn't require the person adding their
> Signed-Off-By to have tested it.

While the DCO may not say it, everyone else will simply require it though ;-) .

Aka no maintainer will apply a patch without the submitter having tested
their change. This is just implicitly expected.

Like if it comes to light later that the change was not tested before
submission that creates quite a trust-issue between submitter and
maintainer on future submissions.


Heiko

Re: [PATCH v2] arm64: dts: rockchip: add rfkill node for M.2 E wifi on orangepi-5-plus

[Dragan Simic]

On 2024-08-07 20:14, Florian Klink wrote:
> On Wed, Aug 07, 2024 at 07:24:27PM GMT, Dragan Simic wrote:
>> On 2024-08-07 19:00, Florian Klink wrote:
>>> This follows the same logic as 82d40b141a4c ("arm64: dts: rockchip: 
>>> add
>>> rfkill node for M.2 Key E WiFi on rock-5b").
>>> 
>>> On the orangepi-5-plus, there's also a GPIO pin connecting the WiFi
>>> enable signal inside the M.2 Key E slot.
>>> 
>>> The exact GPIO PIN can be validated in the Armbian rk-5.10-rkr4 
>>> kernel
>>> rk3588-orangepi-5-plus.dtsi file [1], which contains a `wifi_disable`
>>> node referencing RK_PC4 on &gpio0.
>>> 
>>> Signed-off-by: Florian Klink <flokli@flokli.de>
>>> Tested-by: Florian Klink <flokli@flokli.de>
>> 
>> I forgot to mention that providing a Tested-by tag is redundant when
>> there's already a Signed-off-by tag, because the latter already 
>> implies
>> the former.
> 
> This came after I sent the v3. Generally I wish people would test 
> things
> - though too often it's not. I explicitly tested this to work (with a
> wifi module added to that slot being unblock-able afterwards), and
> wanted to point that out, thus adding the Tested-by.

In general, some time should be allowed between sending consecutive
versions of the same patch, so people can provide their feedback.

When it comes to testing the submitted patches, please note that signing
off a patch implies that the signer has already, to the best of their
abilities, made sure that the patch works as described and expected.

With all that in mind, please allow me to repeat that a Tested-by tag
should not be provided from the same person that the Signed-off-by tag
is already coming from.  It's simply redundant.

> DCO 1.1 doesn't say anything about Tested-by, it's mostly legalese 
> about
> being allowed to send out the patch, and understanding the consequences
> regarding licensing. It doesn't require the person adding their
> Signed-Off-By to have tested it.

Well, not all rules are to be followed blindly, and some documentation
perhaps needs updating or expanding to be more precise.  On top of that,
having something absent from the documentation doesn't necessarily mean
that some additional rules don't apply.  It's many times simply about
applying common sense.

Re: [PATCH v2] arm64: dts: rockchip: add rfkill node for M.2 E wifi on orangepi-5-plus

[Alexey Charkov]

On Wednesday, August 7, 2024 9:32:51 PM GMT+3 Dragan Simic wrote:
> On 2024-08-07 20:14, Florian Klink wrote:
> > On Wed, Aug 07, 2024 at 07:24:27PM GMT, Dragan Simic wrote:
> >> On 2024-08-07 19:00, Florian Klink wrote:
> >>> This follows the same logic as 82d40b141a4c ("arm64: dts: rockchip:
> >>> add
> >>> rfkill node for M.2 Key E WiFi on rock-5b").
> >>> 
> >>> On the orangepi-5-plus, there's also a GPIO pin connecting the WiFi
> >>> enable signal inside the M.2 Key E slot.
> >>> 
> >>> The exact GPIO PIN can be validated in the Armbian rk-5.10-rkr4
> >>> kernel
> >>> rk3588-orangepi-5-plus.dtsi file [1], which contains a `wifi_disable`
> >>> node referencing RK_PC4 on &gpio0.
> >>> 
> >>> Signed-off-by: Florian Klink <flokli@flokli.de>
> >>> Tested-by: Florian Klink <flokli@flokli.de>
> >> 
> >> I forgot to mention that providing a Tested-by tag is redundant when
> >> there's already a Signed-off-by tag, because the latter already
> >> implies
> >> the former.
> > 
> > This came after I sent the v3. Generally I wish people would test
> > things
> > - though too often it's not. I explicitly tested this to work (with a
> > wifi module added to that slot being unblock-able afterwards), and
> > wanted to point that out, thus adding the Tested-by.
> 
> In general, some time should be allowed between sending consecutive
> versions of the same patch, so people can provide their feedback.
> 
> When it comes to testing the submitted patches, please note that signing
> off a patch implies that the signer has already, to the best of their
> abilities, made sure that the patch works as described and expected.
> 
> With all that in mind, please allow me to repeat that a Tested-by tag
> should not be provided from the same person that the Signed-off-by tag
> is already coming from.  It's simply redundant.

Just two cents: perhaps dropping the tag and expanding the commit message a 
bit could be the best of both worlds. Just state that you tested it with such 
and such module, observing such and such results. That would also help if for 
example another user tries a different module and that fails due to some 
quirks: it's easier to debug a potential issue when one knows a working 
configuration to compare a non-working one against.

Best regards,
Alexey

Re: [PATCH v2] arm64: dts: rockchip: add rfkill node for M.2 E wifi on orangepi-5-plus

[Dragan Simic]

Hello Alexey,

On 2024-08-07 23:12, Alexey Charkov wrote:
> On Wednesday, August 7, 2024 9:32:51 PM GMT+3 Dragan Simic wrote:
>> On 2024-08-07 20:14, Florian Klink wrote:
>> > On Wed, Aug 07, 2024 at 07:24:27PM GMT, Dragan Simic wrote:
>> >> On 2024-08-07 19:00, Florian Klink wrote:
>> >>> This follows the same logic as 82d40b141a4c ("arm64: dts: rockchip:
>> >>> add
>> >>> rfkill node for M.2 Key E WiFi on rock-5b").
>> >>>
>> >>> On the orangepi-5-plus, there's also a GPIO pin connecting the WiFi
>> >>> enable signal inside the M.2 Key E slot.
>> >>>
>> >>> The exact GPIO PIN can be validated in the Armbian rk-5.10-rkr4
>> >>> kernel
>> >>> rk3588-orangepi-5-plus.dtsi file [1], which contains a `wifi_disable`
>> >>> node referencing RK_PC4 on &gpio0.
>> >>>
>> >>> Signed-off-by: Florian Klink <flokli@flokli.de>
>> >>> Tested-by: Florian Klink <flokli@flokli.de>
>> >>
>> >> I forgot to mention that providing a Tested-by tag is redundant when
>> >> there's already a Signed-off-by tag, because the latter already
>> >> implies
>> >> the former.
>> >
>> > This came after I sent the v3. Generally I wish people would test
>> > things
>> > - though too often it's not. I explicitly tested this to work (with a
>> > wifi module added to that slot being unblock-able afterwards), and
>> > wanted to point that out, thus adding the Tested-by.
>> 
>> In general, some time should be allowed between sending consecutive
>> versions of the same patch, so people can provide their feedback.
>> 
>> When it comes to testing the submitted patches, please note that 
>> signing
>> off a patch implies that the signer has already, to the best of their
>> abilities, made sure that the patch works as described and expected.
>> 
>> With all that in mind, please allow me to repeat that a Tested-by tag
>> should not be provided from the same person that the Signed-off-by tag
>> is already coming from.  It's simply redundant.
> 
> Just two cents: perhaps dropping the tag and expanding the commit 
> message a
> bit could be the best of both worlds. Just state that you tested it 
> with such
> and such module, observing such and such results. That would also help 
> if for
> example another user tries a different module and that fails due to 
> some
> quirks: it's easier to debug a potential issue when one knows a working
> configuration to compare a non-working one against.

Totally agreed.  Providing as much detail of the performed testing
as possible in the patch description is always a good thing.

Re: [PATCH v2] arm64: dts: rockchip: add rfkill node for M.2 E wifi on orangepi-5-plus

[Florian Klink]

On Wed, Aug 07, 2024 at 11:30:40PM GMT, Dragan Simic wrote:
>Hello Alexey,
>
>On 2024-08-07 23:12, Alexey Charkov wrote:
>>On Wednesday, August 7, 2024 9:32:51 PM GMT+3 Dragan Simic wrote:
>>>On 2024-08-07 20:14, Florian Klink wrote:
>>>> On Wed, Aug 07, 2024 at 07:24:27PM GMT, Dragan Simic wrote:
>>>>> On 2024-08-07 19:00, Florian Klink wrote:
>>>>>> This follows the same logic as 82d40b141a4c ("arm64: dts: rockchip:
>>>>>> add
>>>>>> rfkill node for M.2 Key E WiFi on rock-5b").
>>>>>>
>>>>>> On the orangepi-5-plus, there's also a GPIO pin connecting the WiFi
>>>>>> enable signal inside the M.2 Key E slot.
>>>>>>
>>>>>> The exact GPIO PIN can be validated in the Armbian rk-5.10-rkr4
>>>>>> kernel
>>>>>> rk3588-orangepi-5-plus.dtsi file [1], which contains a `wifi_disable`
>>>>>> node referencing RK_PC4 on &gpio0.
>>>>>>
>>>>>> Signed-off-by: Florian Klink <flokli@flokli.de>
>>>>>> Tested-by: Florian Klink <flokli@flokli.de>
>>>>>
>>>>> I forgot to mention that providing a Tested-by tag is redundant when
>>>>> there's already a Signed-off-by tag, because the latter already
>>>>> implies
>>>>> the former.
>>>>
>>>> This came after I sent the v3. Generally I wish people would test
>>>> things
>>>> - though too often it's not. I explicitly tested this to work (with a
>>>> wifi module added to that slot being unblock-able afterwards), and
>>>> wanted to point that out, thus adding the Tested-by.
>>>
>>>In general, some time should be allowed between sending consecutive
>>>versions of the same patch, so people can provide their feedback.
>>>
>>>When it comes to testing the submitted patches, please note that 
>>>signing
>>>off a patch implies that the signer has already, to the best of their
>>>abilities, made sure that the patch works as described and expected.
>>>
>>>With all that in mind, please allow me to repeat that a Tested-by tag
>>>should not be provided from the same person that the Signed-off-by tag
>>>is already coming from.  It's simply redundant.
>>
>>Just two cents: perhaps dropping the tag and expanding the commit 
>>message a
>>bit could be the best of both worlds. Just state that you tested it 
>>with such
>>and such module, observing such and such results. That would also 
>>help if for
>>example another user tries a different module and that fails due to 
>>some
>>quirks: it's easier to debug a potential issue when one knows a working
>>configuration to compare a non-working one against.
>
>Totally agreed.  Providing as much detail of the performed testing
>as possible in the patch description is always a good thing.

Just sent out a v4 including more information about my testing, and
dropping the explicit Tested-By tag.

Thanks,
Florian