All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Rafał Miłecki" <zajec5@gmail.com>
To: "Lisa Hill" <f11o1iomh3dmt141n@icloud.com>,
	"Rafał Miłecki" <rafal@milecki.pl>,
	"Florian Fainelli" <f.fainelli@gmail.com>,
	"Hauke Mehrtens" <hauke@hauke-m.de>
Cc: Rob Herring <robh+dt@kernel.org>,
	linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
	bcm-kernel-feedback-list@broadcom.com
Subject: Re: [PATCH] ARM: dts: BCM5301X: Add USB GPIO and missing LEDs on Netgear R6300v2
Date: Fri, 10 Dec 2021 08:39:37 +0100	[thread overview]
Message-ID: <cc83723a-cf88-66bc-9ab9-101355f3801f@gmail.com> (raw)
In-Reply-To: <74E33696-D05A-4009-8C40-927A4DBDD4EF@icloud.com>

On 10.12.2021 07:53, Lisa Hill wrote:
> Set GPIO controlling power for both USB ports properly and add two WAN LEDs and a USB LED missing for the Netgear R6300v2.
> 
> Signed-off-by: Lisa Hill <f11o1iomh3dmt141n@icloud.com>

Nice work!

One more comment on sending patches: please configure your e-mail client
to send text only (text/plain) without extra HTML version (text/html).

If you can't configure your e-mail easily you can also try
"git send-email" command line tool.

See:
https://www.kernel.org/doc/html/v5.15/process/submitting-patches.html#no-mime-no-links-no-compression-no-attachments-just-plain-text
https://www.kernel.org/doc/html/v5.15/process/email-clients.html

Also see comments below.


> ---
>   .../arm/boot/dts/bcm4708-netgear-r6300-v2.dts | 27 ++++++++++++++++---
>   1 file changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/bcm4708-netgear-r6300-v2.dts b/arch/arm/boot/dts/bcm4708-netgear-r6300-v2.dts
> index 4c60eda296d973..5810c95ab239ae 100644
> --- a/arch/arm/boot/dts/bcm4708-netgear-r6300-v2.dts
> +++ b/arch/arm/boot/dts/bcm4708-netgear-r6300-v2.dts
> @@ -34,20 +34,33 @@
>   			linux,default-trigger = "default-on";
>   		};
>   
> -		power0 {
> +		power-green {
>   			label = "bcm53xx:green:power";
>   			gpios = <&chipcommon 2 GPIO_ACTIVE_LOW>;
> +			linux,default-trigger = "default-on";
>   		};
>   
> -		power1 {
> +		power-amber {
>   			label = "bcm53xx:amber:power";
>   			gpios = <&chipcommon 3 GPIO_ACTIVE_LOW>;
> -			linux,default-trigger = "default-on";
>   		};
>   
>   		usb {
>   			label = "bcm53xx:blue:usb";
>   			gpios = <&chipcommon 8 GPIO_ACTIVE_LOW>;
> +			trigger-sources = <&ohci_port1>, <&ehci_port1>, <&xhci_port1>;
> +			linux,default-trigger = "usbport";
> +		};
> +		

No tabs in empty separator line please.


> +		wan-green {
> +			label = "bcm53xx:green:wan";
> +			gpios = <&chipcommon 10 GPIO_ACTIVE_LOW>;
> +			linux,default-trigger = "default-on";

I think it's a rather bad idea to blindly set WAN LED on. It should be
up to operating system to decide under what conditions turn it on.
I think you should drop that linux,default-trigger.


> +		};
> +		

No tabs in empty separator line please.

> +		wan-amber {
> +			label = "bcm53xx:amber:wan";
> +			gpios = <&chipcommon 10 GPIO_ACTIVE_HIGH>;
>   		};

It's unlikely both WAN LEDs (green & amber) are controlled by the same
GPIO (10). Please verify that.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: "Rafał Miłecki" <zajec5@gmail.com>
To: "Lisa Hill" <f11o1iomh3dmt141n@icloud.com>,
	"Rafał Miłecki" <rafal@milecki.pl>,
	"Florian Fainelli" <f.fainelli@gmail.com>,
	"Hauke Mehrtens" <hauke@hauke-m.de>
Cc: Rob Herring <robh+dt@kernel.org>,
	linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
	bcm-kernel-feedback-list@broadcom.com
Subject: Re: [PATCH] ARM: dts: BCM5301X: Add USB GPIO and missing LEDs on Netgear R6300v2
Date: Fri, 10 Dec 2021 08:39:37 +0100	[thread overview]
Message-ID: <cc83723a-cf88-66bc-9ab9-101355f3801f@gmail.com> (raw)
In-Reply-To: <74E33696-D05A-4009-8C40-927A4DBDD4EF@icloud.com>

On 10.12.2021 07:53, Lisa Hill wrote:
> Set GPIO controlling power for both USB ports properly and add two WAN LEDs and a USB LED missing for the Netgear R6300v2.
> 
> Signed-off-by: Lisa Hill <f11o1iomh3dmt141n@icloud.com>

Nice work!

One more comment on sending patches: please configure your e-mail client
to send text only (text/plain) without extra HTML version (text/html).

If you can't configure your e-mail easily you can also try
"git send-email" command line tool.

See:
https://www.kernel.org/doc/html/v5.15/process/submitting-patches.html#no-mime-no-links-no-compression-no-attachments-just-plain-text
https://www.kernel.org/doc/html/v5.15/process/email-clients.html

Also see comments below.


> ---
>   .../arm/boot/dts/bcm4708-netgear-r6300-v2.dts | 27 ++++++++++++++++---
>   1 file changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/bcm4708-netgear-r6300-v2.dts b/arch/arm/boot/dts/bcm4708-netgear-r6300-v2.dts
> index 4c60eda296d973..5810c95ab239ae 100644
> --- a/arch/arm/boot/dts/bcm4708-netgear-r6300-v2.dts
> +++ b/arch/arm/boot/dts/bcm4708-netgear-r6300-v2.dts
> @@ -34,20 +34,33 @@
>   			linux,default-trigger = "default-on";
>   		};
>   
> -		power0 {
> +		power-green {
>   			label = "bcm53xx:green:power";
>   			gpios = <&chipcommon 2 GPIO_ACTIVE_LOW>;
> +			linux,default-trigger = "default-on";
>   		};
>   
> -		power1 {
> +		power-amber {
>   			label = "bcm53xx:amber:power";
>   			gpios = <&chipcommon 3 GPIO_ACTIVE_LOW>;
> -			linux,default-trigger = "default-on";
>   		};
>   
>   		usb {
>   			label = "bcm53xx:blue:usb";
>   			gpios = <&chipcommon 8 GPIO_ACTIVE_LOW>;
> +			trigger-sources = <&ohci_port1>, <&ehci_port1>, <&xhci_port1>;
> +			linux,default-trigger = "usbport";
> +		};
> +		

No tabs in empty separator line please.


> +		wan-green {
> +			label = "bcm53xx:green:wan";
> +			gpios = <&chipcommon 10 GPIO_ACTIVE_LOW>;
> +			linux,default-trigger = "default-on";

I think it's a rather bad idea to blindly set WAN LED on. It should be
up to operating system to decide under what conditions turn it on.
I think you should drop that linux,default-trigger.


> +		};
> +		

No tabs in empty separator line please.

> +		wan-amber {
> +			label = "bcm53xx:amber:wan";
> +			gpios = <&chipcommon 10 GPIO_ACTIVE_HIGH>;
>   		};

It's unlikely both WAN LEDs (green & amber) are controlled by the same
GPIO (10). Please verify that.

  reply	other threads:[~2021-12-10  7:41 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-10  6:53 [PATCH] ARM: dts: BCM5301X: Add USB GPIO and missing LEDs on Netgear R6300v2 Lisa Hill
2021-12-10  6:53 ` Lisa Hill
2021-12-10  7:39 ` Rafał Miłecki [this message]
2021-12-10  7:39   ` Rafał Miłecki
     [not found]   ` <C624909D-1513-4256-BCC1-50C7B43402CC@icloud.com>
2021-12-10  9:31     ` Rafał Miłecki
2021-12-10  9:31       ` Rafał Miłecki
  -- strict thread matches above, loose matches on Subject: below --
2021-12-11  8:53 Lisa Hill
2021-12-11  8:53 ` Lisa Hill
2021-12-16 21:56 ` kernel test robot
2021-12-16 21:56   ` kernel test robot
2021-12-16 21:56   ` 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=cc83723a-cf88-66bc-9ab9-101355f3801f@gmail.com \
    --to=zajec5@gmail.com \
    --cc=bcm-kernel-feedback-list@broadcom.com \
    --cc=devicetree@vger.kernel.org \
    --cc=f.fainelli@gmail.com \
    --cc=f11o1iomh3dmt141n@icloud.com \
    --cc=hauke@hauke-m.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=rafal@milecki.pl \
    --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.