| Message ID | 20240505164549.65644-2-linux@fw-web.de (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | Add Bananapi R3 Mini | expand |
On 05/05/2024 18:45, Frank Wunderlich wrote: > From: Frank Wunderlich <frank-w@public-files.de> > > Add led trigger implemented with config-symbol LEDS_TRIGGER_NETDEV to > binding. > > Signed-off-by: Frank Wunderlich <frank-w@public-files.de> > --- > Documentation/devicetree/bindings/leds/common.yaml | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/Documentation/devicetree/bindings/leds/common.yaml b/Documentation/devicetree/bindings/leds/common.yaml > index 8a3c2398b10c..bf9a101e4d42 100644 > --- a/Documentation/devicetree/bindings/leds/common.yaml > +++ b/Documentation/devicetree/bindings/leds/common.yaml > @@ -113,6 +113,8 @@ properties: > # LED indicates NAND memory activity (deprecated), > # in new implementations use "mtd" > - nand-disk > + # LED indicates network activity > + - netdev "dev" is redundant (there is no flash-dev or usb-host-dev). Two network interfaces are already provided, so your commit msg must provide rationale why this is not enough and why this is useful/needed. Best regards, Krzysztof
On Mon, May 06, 2024 at 10:18:09AM +0200, Krzysztof Kozlowski wrote: > On 05/05/2024 18:45, Frank Wunderlich wrote: > > From: Frank Wunderlich <frank-w@public-files.de> > > > > Add led trigger implemented with config-symbol LEDS_TRIGGER_NETDEV to > > binding. > > > > Signed-off-by: Frank Wunderlich <frank-w@public-files.de> > > --- > > Documentation/devicetree/bindings/leds/common.yaml | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/leds/common.yaml b/Documentation/devicetree/bindings/leds/common.yaml > > index 8a3c2398b10c..bf9a101e4d42 100644 > > --- a/Documentation/devicetree/bindings/leds/common.yaml > > +++ b/Documentation/devicetree/bindings/leds/common.yaml > > @@ -113,6 +113,8 @@ properties: > > # LED indicates NAND memory activity (deprecated), > > # in new implementations use "mtd" > > - nand-disk > > + # LED indicates network activity > > + - netdev > > "dev" is redundant (there is no flash-dev or usb-host-dev). Two network > interfaces are already provided, so your commit msg must provide > rationale why this is not enough and why this is useful/needed. Also note that using 'netdev' assigned via DT via linux,default-trigger currently doesn't work well. This is because the assignment of the trigger from DT happens when the PHY is being attached initially, and that's **before** the network device is registered with Linux. As a result, LED event offloading is never used if done in this way. I know that bindings and implementation are two different independent things, but yet I believe that adding bindings for a feature which doesn't really work would be misleading.
Hi > Gesendet: Montag, 06. Mai 2024 um 10:18 Uhr > Von: "Krzysztof Kozlowski" <krzysztof.kozlowski@linaro.org> > An: "Frank Wunderlich" <linux@fw-web.de>, "Rob Herring" <robh@kernel.org>, "Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>, "Conor Dooley" <conor+dt@kernel.org>, "Michael Turquette" <mturquette@baylibre.com>, "Stephen Boyd" <sboyd@kernel.org>, "Pavel Machek" <pavel@ucw.cz>, "Lee Jones" <lee@kernel.org>, "David S. Miller" <davem@davemloft.net>, "Eric Dumazet" <edumazet@google.com>, "Jakub Kicinski" <kuba@kernel.org>, "Paolo Abeni" <pabeni@redhat.com>, "Matthias Brugger" <matthias.bgg@gmail.com>, "AngeloGioacchino Del Regno" <angelogioacchino.delregno@collabora.com> > Cc: "Frank Wunderlich" <frank-w@public-files.de>, "Eric Woudstra" <ericwouds@gmail.com>, "Tianling Shen" <cnsztl@immortalwrt.org>, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org, linux-leds@vger.kernel.org, netdev@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org > Betreff: Re: [RFC v1 1/5] dt-bindings: leds: add led trigger netdev > > On 05/05/2024 18:45, Frank Wunderlich wrote: > > From: Frank Wunderlich <frank-w@public-files.de> > > > > Add led trigger implemented with config-symbol LEDS_TRIGGER_NETDEV to > > binding. > > > > Signed-off-by: Frank Wunderlich <frank-w@public-files.de> > > --- > > Documentation/devicetree/bindings/leds/common.yaml | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/leds/common.yaml b/Documentation/devicetree/bindings/leds/common.yaml > > index 8a3c2398b10c..bf9a101e4d42 100644 > > --- a/Documentation/devicetree/bindings/leds/common.yaml > > +++ b/Documentation/devicetree/bindings/leds/common.yaml > > @@ -113,6 +113,8 @@ properties: > > # LED indicates NAND memory activity (deprecated), > > # in new implementations use "mtd" > > - nand-disk > > + # LED indicates network activity > > + - netdev > > "dev" is redundant (there is no flash-dev or usb-host-dev). Two network > interfaces are already provided, so your commit msg must provide > rationale why this is not enough and why this is useful/needed. i only see 1 network binding...and this is labeled/described with wlan and phy # LED is triggered by WLAN activity - pattern: "^phy[0-9]+tx$" which second do you mean? btw. usb + disk has 3 trigger and "netdev" is already used in some dts, so i thought adding the binding is a good idea arch/arm/boot/dts/rockchip/rk3128-xpi-3128.dts:107: * linux,default-trigger = "netdev"; arch/arm/boot/dts/nxp/imx/imx53-m53menlo.dts:52: linux,default-trigger = "netdev"; arch/arm/boot/dts/intel/ixp/intel-ixp42x-dlink-dsm-g600.dts:51: linux,default-trigger = "netdev"; arch/arm/boot/dts/intel/ixp/intel-ixp42x-iomega-nas100d.dts:39: linux,default-trigger = "netdev"; arch/arm/boot/dts/ti/omap/am5729-beagleboneai.dts:138: linux,default-trigger = "netdev"; arch/arm/boot/dts/ti/omap/am335x-netcan-plus-1xx.dts:27: linux,default-trigger = "netdev"; arch/mips/boot/dts/ralink/gardena_smart_gateway_mt7688.dts:107: linux,default-trigger = "netdev"; arch/mips/boot/dts/ralink/gardena_smart_gateway_mt7688.dts:113: linux,default-trigger = "netdev"; first one has it as comment that not yet in binding and with not that it needs to be set in userspace like Daniel stated in the dts patch (does not work for phys)...so i drop this patch and the property in the dts. > Best regards, > Krzysztof regards Frank
On 06/05/2024 19:10, Frank Wunderlich wrote: > Hi > >> Gesendet: Montag, 06. Mai 2024 um 10:18 Uhr >> Von: "Krzysztof Kozlowski" <krzysztof.kozlowski@linaro.org> >> An: "Frank Wunderlich" <linux@fw-web.de>, "Rob Herring" <robh@kernel.org>, "Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>, "Conor Dooley" <conor+dt@kernel.org>, "Michael Turquette" <mturquette@baylibre.com>, "Stephen Boyd" <sboyd@kernel.org>, "Pavel Machek" <pavel@ucw.cz>, "Lee Jones" <lee@kernel.org>, "David S. Miller" <davem@davemloft.net>, "Eric Dumazet" <edumazet@google.com>, "Jakub Kicinski" <kuba@kernel.org>, "Paolo Abeni" <pabeni@redhat.com>, "Matthias Brugger" <matthias.bgg@gmail.com>, "AngeloGioacchino Del Regno" <angelogioacchino.delregno@collabora.com> >> Cc: "Frank Wunderlich" <frank-w@public-files.de>, "Eric Woudstra" <ericwouds@gmail.com>, "Tianling Shen" <cnsztl@immortalwrt.org>, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org, linux-leds@vger.kernel.org, netdev@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org >> Betreff: Re: [RFC v1 1/5] dt-bindings: leds: add led trigger netdev >> >> On 05/05/2024 18:45, Frank Wunderlich wrote: >>> From: Frank Wunderlich <frank-w@public-files.de> >>> >>> Add led trigger implemented with config-symbol LEDS_TRIGGER_NETDEV to >>> binding. >>> >>> Signed-off-by: Frank Wunderlich <frank-w@public-files.de> >>> --- >>> Documentation/devicetree/bindings/leds/common.yaml | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/leds/common.yaml b/Documentation/devicetree/bindings/leds/common.yaml >>> index 8a3c2398b10c..bf9a101e4d42 100644 >>> --- a/Documentation/devicetree/bindings/leds/common.yaml >>> +++ b/Documentation/devicetree/bindings/leds/common.yaml >>> @@ -113,6 +113,8 @@ properties: >>> # LED indicates NAND memory activity (deprecated), >>> # in new implementations use "mtd" >>> - nand-disk >>> + # LED indicates network activity >>> + - netdev >> >> "dev" is redundant (there is no flash-dev or usb-host-dev). Two network >> interfaces are already provided, so your commit msg must provide >> rationale why this is not enough and why this is useful/needed. > > i only see 1 network binding...and this is labeled/described with wlan and phy > > # LED is triggered by WLAN activity > - pattern: "^phy[0-9]+tx$" > > which second do you mean? > > btw. usb + disk has 3 trigger and "netdev" is already used in some dts, so i thought adding the binding is a good idea > > arch/arm/boot/dts/rockchip/rk3128-xpi-3128.dts:107: * linux,default-trigger = "netdev"; > arch/arm/boot/dts/nxp/imx/imx53-m53menlo.dts:52: linux,default-trigger = "netdev"; > arch/arm/boot/dts/intel/ixp/intel-ixp42x-dlink-dsm-g600.dts:51: linux,default-trigger = "netdev"; > arch/arm/boot/dts/intel/ixp/intel-ixp42x-iomega-nas100d.dts:39: linux,default-trigger = "netdev"; > arch/arm/boot/dts/ti/omap/am5729-beagleboneai.dts:138: linux,default-trigger = "netdev"; > arch/arm/boot/dts/ti/omap/am335x-netcan-plus-1xx.dts:27: linux,default-trigger = "netdev"; > arch/mips/boot/dts/ralink/gardena_smart_gateway_mt7688.dts:107: linux,default-trigger = "netdev"; > arch/mips/boot/dts/ralink/gardena_smart_gateway_mt7688.dts:113: linux,default-trigger = "netdev"; > Then please check previous discussions: https://lore.kernel.org/all/20230217230346.GA2217008-robh@kernel.org/ Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/leds/common.yaml b/Documentation/devicetree/bindings/leds/common.yaml index 8a3c2398b10c..bf9a101e4d42 100644 --- a/Documentation/devicetree/bindings/leds/common.yaml +++ b/Documentation/devicetree/bindings/leds/common.yaml @@ -113,6 +113,8 @@ properties: # LED indicates NAND memory activity (deprecated), # in new implementations use "mtd" - nand-disk + # LED indicates network activity + - netdev # No trigger assigned to the LED. This is the default mode # if trigger is absent - none