| Message ID | 20191105151202.4709-1-freifunk@adrianschmutzler.de |
|---|---|
| State | Rejected |
| Delegated to: | Adrian Schmutzler |
| Headers | show |
| Series | [OpenWrt-Devel,v2] ath79: use gpio_hog instead of gpio-export | expand |
Thanks Adrian for taking this up again! Birger On 5 November 2019 16:12:02 CET, Adrian Schmutzler <freifunk@adrianschmutzler.de> wrote: >From: Birger Koblitz <mail@birger-koblitz.de> > >The gpio-export functionality is a hack for missing kernel >functionality, which was rejected in upstream kernel long time ago, >for details see this email >http://lists.infradead.org/pipermail/openwrt-devel/2019-February/015772.html, >discussion in PR#1366 or >https://github.com/openwrt/openwrt/pull/1814#issuecomment-462942022. > >This patch converts all DTS files with settings that normally do >not need user interaction, e.g. power for external USB ports, to >gpio_hog. The only remaining gpio-export will be in >qca9558_openmesh_om5p-ac-v2.dts > >Signed-off-by: Birger Koblitz <mail@birger-koblitz.de> >[rebased, renamed nodes, do not change openmesh, removed redundant >status=okay, updated commit message] >Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de> > >--- > >This is a resubmission of the patch initially created by Birger >Koblitz. It should be limited to conversion that do not require >the user setting the value. >--- > .../dts/ar7161_buffalo_wzr-hp-ag300h.dts | 19 ++++++------ > .../ath79/dts/ar7241_tplink_tl-mr3x20.dtsi | 15 +++++----- > .../ath79/dts/ar7241_tplink_tl-wr842n-v1.dts | 19 +++++------- > .../ath79/dts/ar7242_buffalo_wzr-bhr.dtsi | 20 ++++++------- > .../dts/ar7242_buffalo_wzr-hp-g302h-a1a0.dts | 20 ++++++------- > .../ath79/dts/ar9341_tplink_tl-wr842n-v2.dts | 19 ++++++------ > .../ath79/dts/qca9558_devolo_dvl1750e.dts | 15 +++++----- > .../ath79/dts/qca9558_tplink_archer-c7.dtsi | 30 +++++++++---------- > .../dts/qca9558_tplink_tl-wdr4900-v2.dts | 28 +++++++++-------- > .../ath79/dts/qca9558_tplink_tl-wr1043nd.dtsi | 20 +++++-------- > .../ath79/dts/qca9561_tplink_archer-c5x.dtsi | 30 +++++++++---------- > .../ath79/dts/qca9563_dlink_dir-859-a1.dts | 20 +++++-------- > .../ath79/dts/qca9563_tplink_archer-c7-v4.dts | 30 +++++++++---------- > .../dts/qca9563_tplink_archer-x7-v5.dtsi | 17 +++++------ > .../dts/qca9563_tplink_tl-wr1043nd-v4.dts | 16 +++++----- > 15 files changed, 145 insertions(+), 173 deletions(-) > >diff --git a/target/linux/ath79/dts/ar7161_buffalo_wzr-hp-ag300h.dts >b/target/linux/ath79/dts/ar7161_buffalo_wzr-hp-ag300h.dts >index f51bc0f771..23f1845876 100644 >--- a/target/linux/ath79/dts/ar7161_buffalo_wzr-hp-ag300h.dts >+++ b/target/linux/ath79/dts/ar7161_buffalo_wzr-hp-ag300h.dts >@@ -120,16 +120,6 @@ > }; > }; > >- gpio-export { >- compatible = "gpio-export"; >- >- gpio_usb_power { >- gpio-export,name = "buffalo:power:usb"; >- gpio-export,output = <1>; >- gpios = <&gpio 2 GPIO_ACTIVE_HIGH>; >- }; >- }; >- > flash { > compatible = "mtd-concat"; > >@@ -172,6 +162,15 @@ > }; > }; > >+&gpio { >+ usb_power { >+ gpio-hog; >+ line-name = "buffalo:power:usb"; >+ gpios = <2 GPIO_ACTIVE_HIGH>; >+ output-high; >+ }; >+}; >+ > &usb_phy { > status = "okay"; > }; >diff --git a/target/linux/ath79/dts/ar7241_tplink_tl-mr3x20.dtsi >b/target/linux/ath79/dts/ar7241_tplink_tl-mr3x20.dtsi >index 04403637b6..333ee17ceb 100644 >--- a/target/linux/ath79/dts/ar7241_tplink_tl-mr3x20.dtsi >+++ b/target/linux/ath79/dts/ar7241_tplink_tl-mr3x20.dtsi >@@ -3,15 +3,14 @@ > #include "ar7241_tplink.dtsi" > > / { >- gpio-export { >- compatible = "gpio-export"; >- #size-cells = <0>; >+}; > >- gpio_usb_power { >- gpio-export,name = "tp-link:power:usb"; >- gpio-export,output = <1>; >- gpios = <&gpio 6 GPIO_ACTIVE_HIGH>; >- }; >+&gpio { >+ usb_power { >+ gpio-hog; >+ line-name = "tp-link:power:usb"; >+ gpios = <6 GPIO_ACTIVE_HIGH>; >+ output-high; > }; > }; > >diff --git a/target/linux/ath79/dts/ar7241_tplink_tl-wr842n-v1.dts >b/target/linux/ath79/dts/ar7241_tplink_tl-wr842n-v1.dts >index 162b5f2838..ee468df244 100644 >--- a/target/linux/ath79/dts/ar7241_tplink_tl-wr842n-v1.dts >+++ b/target/linux/ath79/dts/ar7241_tplink_tl-wr842n-v1.dts >@@ -66,15 +66,16 @@ > linux,default-trigger = "phy0tpt"; > }; > }; >+}; > >- gpio-export { >- compatible = "gpio-export"; >+&gpio { >+ status = "okay"; > >- gpio_usb_power { >- gpio-export,name = "tp-link:power:usb"; >- gpio-export,output = <1>; >- gpios = <&gpio 6 GPIO_ACTIVE_HIGH>; >- }; >+ usb_power { >+ gpio-hog; >+ line-name = "tp-link:power:usb"; >+ gpios = <6 GPIO_ACTIVE_HIGH>; >+ output-high; > }; > }; > >@@ -155,10 +156,6 @@ > mtd-mac-address-increment = <1>; > }; > >-&gpio { >- status = "okay"; >-}; >- > &uart { > status = "okay"; > }; >diff --git a/target/linux/ath79/dts/ar7242_buffalo_wzr-bhr.dtsi >b/target/linux/ath79/dts/ar7242_buffalo_wzr-bhr.dtsi >index 3b5a4dd13d..d7632faf5c 100644 >--- a/target/linux/ath79/dts/ar7242_buffalo_wzr-bhr.dtsi >+++ b/target/linux/ath79/dts/ar7242_buffalo_wzr-bhr.dtsi >@@ -57,17 +57,6 @@ > }; > }; > >- gpio-export { >- compatible = "gpio-export"; >- #size-cells = <0>; >- >- gpio_usb_power { >- gpio-export,name = "buffalo:usb-power"; >- gpio-export,output = <1>; >- gpios = <&gpio 16 GPIO_ACTIVE_HIGH>; >- }; >- }; >- > virtual_flash { > compatible = "mtd-concat"; > devices = <&flash0 &flash1>; >@@ -109,6 +98,15 @@ > }; > }; > >+&gpio { >+ usb_power { >+ gpio-hog; >+ line-name = "buffalo:usb-power"; >+ gpios = <16 GPIO_ACTIVE_HIGH>; >+ output-high; >+ }; >+}; >+ > &spi { > status = "okay"; > cs-gpios = <0>, <0>; >diff --git >a/target/linux/ath79/dts/ar7242_buffalo_wzr-hp-g302h-a1a0.dts >b/target/linux/ath79/dts/ar7242_buffalo_wzr-hp-g302h-a1a0.dts >index 8ac4df2194..2b30b7830b 100644 >--- a/target/linux/ath79/dts/ar7242_buffalo_wzr-hp-g302h-a1a0.dts >+++ b/target/linux/ath79/dts/ar7242_buffalo_wzr-hp-g302h-a1a0.dts >@@ -109,17 +109,6 @@ > }; > }; > >- gpio-export { >- compatible = "gpio-export"; >- #size-cells = <0>; >- >- gpio_usb_power { >- gpio-export,name = "buffalo:usb-power"; >- gpio-export,output = <1>; >- gpios = <&gpio 13 GPIO_ACTIVE_HIGH>; >- }; >- }; >- > virtual_flash { > compatible = "mtd-concat"; > devices = <&flash0 &flash1>; >@@ -161,6 +150,15 @@ > }; > }; > >+&gpio { >+ usb_power { >+ gpio-hog; >+ line-name = "buffalo:usb-power"; >+ gpios = <13 GPIO_ACTIVE_HIGH>; >+ output-high; >+ }; >+}; >+ > &spi { > status = "okay"; > cs-gpios = <0>, <0>; >diff --git a/target/linux/ath79/dts/ar9341_tplink_tl-wr842n-v2.dts >b/target/linux/ath79/dts/ar9341_tplink_tl-wr842n-v2.dts >index 86a316b518..1df0e27cb2 100644 >--- a/target/linux/ath79/dts/ar9341_tplink_tl-wr842n-v2.dts >+++ b/target/linux/ath79/dts/ar9341_tplink_tl-wr842n-v2.dts >@@ -6,16 +6,6 @@ > / { > model = "TP-Link TL-WR842N/ND v2"; > compatible = "tplink,tl-wr842n-v2", "qca,ar9341"; >- >- gpio-export { >- compatible = "gpio-export"; >- >- gpio_usb_power { >- gpio-export,name = "tp-link:power:usb"; >- gpio-export,output = <1>; >- gpios = <&gpio 4 GPIO_ACTIVE_HIGH>; >- }; >- }; > }; > > &keys { >@@ -36,6 +26,15 @@ > }; > }; > >+&gpio { >+ usb_power { >+ gpio-hog; >+ line-name = "tp-link:power:usb"; >+ gpios = <4 GPIO_ACTIVE_HIGH>; >+ output-high; >+ }; >+}; >+ > &spi { > num-cs = <1>; > >diff --git a/target/linux/ath79/dts/qca9558_devolo_dvl1750e.dts >b/target/linux/ath79/dts/qca9558_devolo_dvl1750e.dts >index 3d25004c40..e790cf0df4 100644 >--- a/target/linux/ath79/dts/qca9558_devolo_dvl1750e.dts >+++ b/target/linux/ath79/dts/qca9558_devolo_dvl1750e.dts >@@ -53,15 +53,14 @@ > compatible = "gpio-beeper"; > gpios = <&gpio 4 GPIO_ACTIVE_HIGH>; > }; >+}; > >- gpio_export { >- compatible = "gpio-export"; >- >- gpio_usb_power { >- gpio-export,name = "devolo:power:usb"; >- gpio-export,output = <1>; >- gpios = <&gpio 11 GPIO_ACTIVE_HIGH>; >- }; >+&gpio { >+ usb_power { >+ gpio-hog; >+ line-name = "devolo:power:usb"; >+ gpios = <11 GPIO_ACTIVE_HIGH>; >+ output-high; > }; > }; > >diff --git a/target/linux/ath79/dts/qca9558_tplink_archer-c7.dtsi >b/target/linux/ath79/dts/qca9558_tplink_archer-c7.dtsi >index c77613dad6..fb38f35dc1 100644 >--- a/target/linux/ath79/dts/qca9558_tplink_archer-c7.dtsi >+++ b/target/linux/ath79/dts/qca9558_tplink_archer-c7.dtsi >@@ -64,22 +64,6 @@ > debounce-interval = <60>; > }; > }; >- >- gpio-export { >- compatible = "gpio-export"; >- >- gpio_usb1_power { >- gpio-export,name = "tp-link:power:usb1"; >- gpio-export,output = <1>; >- gpios = <&gpio 21 GPIO_ACTIVE_HIGH>; >- }; >- >- gpio_usb2_power { >- gpio-export,name = "tp-link:power:usb2"; >- gpio-export,output = <1>; >- gpios = <&gpio 22 GPIO_ACTIVE_HIGH>; >- }; >- }; > }; > > &pcie1 { >@@ -92,6 +76,20 @@ > > &gpio { > status = "okay"; >+ >+ usb1_power { >+ gpio-hog; >+ line-name = "tp-link:power:usb1"; >+ gpios = <21 GPIO_ACTIVE_HIGH>; >+ output-high; >+ }; >+ >+ usb2_power { >+ gpio-hog; >+ line-name = "tp-link:power:usb2"; >+ gpios = <22 GPIO_ACTIVE_HIGH>; >+ output-high; >+ }; > }; > > &usb_phy0 { >diff --git a/target/linux/ath79/dts/qca9558_tplink_tl-wdr4900-v2.dts >b/target/linux/ath79/dts/qca9558_tplink_tl-wdr4900-v2.dts >index 77166b8bfe..470b742578 100644 >--- a/target/linux/ath79/dts/qca9558_tplink_tl-wdr4900-v2.dts >+++ b/target/linux/ath79/dts/qca9558_tplink_tl-wdr4900-v2.dts >@@ -57,7 +57,7 @@ > }; > }; > >- ath9k-leds { >+ ath9k-leds { > compatible = "gpio-leds"; > > wlan5g { >@@ -77,21 +77,23 @@ > debounce-interval = <60>; > }; > }; >+}; > >- gpio-export { >- compatible = "gpio-export"; >+&gpio { >+ status = "okay"; > >- gpio_usb1_power { >- gpio-export,name = "tp-link:power:usb1"; >- gpio-export,output = <1>; >- gpios = <&gpio 21 GPIO_ACTIVE_HIGH>; >- }; >+ usb1_power { >+ gpio-hog; >+ line-name = "tp-link:power:usb1"; >+ gpios = <21 GPIO_ACTIVE_HIGH>; >+ output-high; >+ }; > >- gpio_usb2_power { >- gpio-export,name = "tp-link:power:usb2"; >- gpio-export,output = <1>; >- gpios = <&gpio 22 GPIO_ACTIVE_HIGH>; >- }; >+ usb2_power { >+ gpio-hog; >+ line-name = "tp-link:power:usb2"; >+ gpios = <22 GPIO_ACTIVE_HIGH>; >+ output-high; > }; > }; > >diff --git a/target/linux/ath79/dts/qca9558_tplink_tl-wr1043nd.dtsi >b/target/linux/ath79/dts/qca9558_tplink_tl-wr1043nd.dtsi >index 1092250f02..f6d34ab3ff 100644 >--- a/target/linux/ath79/dts/qca9558_tplink_tl-wr1043nd.dtsi >+++ b/target/linux/ath79/dts/qca9558_tplink_tl-wr1043nd.dtsi >@@ -64,16 +64,16 @@ > debounce-interval = <60>; > }; > }; >+}; > >- gpio-export { >- compatible = "gpio-export"; >- #size-cells = <0>; >+&gpio { >+ status = "okay"; > >- gpio_usb_power { >- gpio-export,name = "tp-link:power:usb"; >- gpio-export,output = <1>; >- gpios = <&gpio 21 GPIO_ACTIVE_HIGH>; >- }; >+ usb_power { >+ gpio-hog; >+ line-name = "tp-link:power:usb"; >+ gpios = <21 GPIO_ACTIVE_HIGH>; >+ output-high; > }; > }; > >@@ -81,10 +81,6 @@ > status = "okay"; > }; > >-&gpio { >- status = "okay"; >-}; >- > &usb_phy0 { > status = "okay"; > }; >diff --git a/target/linux/ath79/dts/qca9561_tplink_archer-c5x.dtsi >b/target/linux/ath79/dts/qca9561_tplink_archer-c5x.dtsi >index ddf92108b5..53329f2268 100644 >--- a/target/linux/ath79/dts/qca9561_tplink_archer-c5x.dtsi >+++ b/target/linux/ath79/dts/qca9561_tplink_archer-c5x.dtsi >@@ -103,22 +103,6 @@ > gpios = <&gpio 21 GPIO_ACTIVE_LOW>; > }; > }; >- >- gpio-export { >- compatible = "gpio-export"; >- >- gpio_shift_register_oe { >- gpio-export,name = "tp-link:oe:sr"; >- gpio-export,output = <0>; >- gpios = <&gpio 16 GPIO_ACTIVE_HIGH>; >- }; >- >- gpio_shift_register_reset { >- gpio-export,name = "tp-link:reset:sr"; >- gpio-export,output = <1>; >- gpios = <&gpio 19 GPIO_ACTIVE_HIGH>; >- }; >- }; > }; > > &uart { >@@ -127,6 +111,20 @@ > > &gpio { > status = "okay"; >+ >+ shift_register_oe { >+ gpio-hog; >+ line-name = "tp-link:oe:sr"; >+ gpios = <16 GPIO_ACTIVE_HIGH>; >+ output-low; >+ }; >+ >+ shift_register_reset { >+ gpio-hog; >+ line-name = "tp-link:reset:sr"; >+ gpios = <19 GPIO_ACTIVE_HIGH>; >+ output-high; >+ }; > }; > > &pcie { >diff --git a/target/linux/ath79/dts/qca9563_dlink_dir-859-a1.dts >b/target/linux/ath79/dts/qca9563_dlink_dir-859-a1.dts >index a17d9f263a..63baa376c6 100644 >--- a/target/linux/ath79/dts/qca9563_dlink_dir-859-a1.dts >+++ b/target/linux/ath79/dts/qca9563_dlink_dir-859-a1.dts >@@ -61,16 +61,16 @@ > debounce-interval = <60>; > }; > }; >+}; > >- gpio-export { >- compatible = "gpio-export"; >- #size-cells = <0>; >+&gpio { >+ status = "okay"; > >- gpio_switch_reset { >- gpio-export,name = "dir-859-a1:reset:switch"; >- gpio-export,output = <1>; >- gpios = <&gpio 11 GPIO_ACTIVE_HIGH>; >- }; >+ switch_reset { >+ gpio-hog; >+ line-name = "dir-859-a1:reset:switch"; >+ gpios = <11 GPIO_ACTIVE_HIGH>; >+ output-high; > }; > }; > >@@ -78,10 +78,6 @@ > status = "okay"; > }; > >-&gpio { >- status = "okay"; >-}; >- > &pcie { > status = "okay"; > }; >diff --git a/target/linux/ath79/dts/qca9563_tplink_archer-c7-v4.dts >b/target/linux/ath79/dts/qca9563_tplink_archer-c7-v4.dts >index 470a8e6bf9..66083aa7f9 100644 >--- a/target/linux/ath79/dts/qca9563_tplink_archer-c7-v4.dts >+++ b/target/linux/ath79/dts/qca9563_tplink_archer-c7-v4.dts >@@ -42,22 +42,6 @@ > }; > }; > >- gpio-export { >- compatible = "gpio-export"; >- >- gpio_shift_register_oe { >- gpio-export,name = "tp-link:oe:sr"; >- gpio-export,output = <0>; >- gpios = <&gpio 1 GPIO_ACTIVE_LOW>; // 74HC595 /OE (Output Enable) >- }; >- >- gpio_shift_register_reset { >- gpio-export,name = "tp-link:reset:sr"; >- gpio-export,output = <1>; >- gpios = <&gpio 21 GPIO_ACTIVE_LOW>; // 74HC595 /SRCLR (Serial >Clear) >- }; >- }; >- > leds { > compatible = "gpio-leds"; > >@@ -158,6 +142,20 @@ > > &gpio { > status = "okay"; >+ >+ shift_register_oe { // 74HC595 /OE >+ gpio-hog; >+ line-name = "tp-link:oe:sr"; >+ gpios = <1 GPIO_ACTIVE_LOW>; >+ output-low; >+ }; >+ >+ shift_register_reset { // 74HC595 /SRCLR >+ gpio-hog; >+ line-name = "tp-link:reset:sr"; >+ gpios = <21 GPIO_ACTIVE_LOW>; >+ output-high; >+ }; > }; > > &usb_phy0 { >diff --git a/target/linux/ath79/dts/qca9563_tplink_archer-x7-v5.dtsi >b/target/linux/ath79/dts/qca9563_tplink_archer-x7-v5.dtsi >index 2e66e0f03e..64115c4248 100644 >--- a/target/linux/ath79/dts/qca9563_tplink_archer-x7-v5.dtsi >+++ b/target/linux/ath79/dts/qca9563_tplink_archer-x7-v5.dtsi >@@ -93,16 +93,6 @@ > debounce-interval = <60>; > }; > }; >- >- gpio-export { >- compatible = "gpio-export"; >- >- gpio_usb_power { >- gpio-export,name = "tp-link:power:usb"; >- gpio-export,output = <1>; >- gpios = <&gpio 19 GPIO_ACTIVE_HIGH>; >- }; >- }; > }; > > &pcie { >@@ -115,6 +105,13 @@ > > &gpio { > status = "okay"; >+ >+ usb_power { >+ gpio-hog; >+ line-name = "tp-link:power:usb"; >+ gpios = <19 GPIO_ACTIVE_HIGH>; >+ output-high; >+ }; > }; > > &usb_phy0 { >diff --git a/target/linux/ath79/dts/qca9563_tplink_tl-wr1043nd-v4.dts >b/target/linux/ath79/dts/qca9563_tplink_tl-wr1043nd-v4.dts >index 07a7409886..45ee0c21c7 100644 >--- a/target/linux/ath79/dts/qca9563_tplink_tl-wr1043nd-v4.dts >+++ b/target/linux/ath79/dts/qca9563_tplink_tl-wr1043nd-v4.dts >@@ -6,16 +6,14 @@ > / { > compatible = "tplink,tl-wr1043nd-v4", "qca,qca9563"; > model = "TP-Link TL-WR1043ND v4"; >+}; > >- gpio-export { >- compatible = "gpio-export"; >- #size-cells = <0>; >- >- gpio_usb_power { >- gpio-export,name = "tp-link:power:usb"; >- gpio-export,output = <1>; >- gpios = <&gpio 8 GPIO_ACTIVE_HIGH>; >- }; >+&gpio { >+ usb_power { >+ gpio-hog; >+ line-name = "tp-link:power:usb"; >+ gpios = <8 GPIO_ACTIVE_HIGH>; >+ output-high; > }; > }; > >-- >2.20.1 > > >_______________________________________________ >openwrt-devel mailing list >openwrt-devel@lists.openwrt.org >https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Adrian Schmutzler <freifunk@adrianschmutzler.de> writes: > This patch converts all DTS files with settings that normally do > not need user interaction, e.g. power for external USB ports, to > gpio_hog. Wouldn't it be better to map these as fixed regulators? Then you could eventually link them to the connected ports/controllers, and allow them to be automatically turned off when not needed. Bjørn
Hi, > -----Original Message----- > From: openwrt-devel [mailto:openwrt-devel-bounces@lists.openwrt.org] On > Behalf Of Bjørn Mork > Sent: Dienstag, 5. November 2019 17:38 > To: Adrian Schmutzler <freifunk@adrianschmutzler.de> > Cc: openwrt-devel@lists.openwrt.org; Birger Koblitz <mail@birger-koblitz.de> > Subject: Re: [OpenWrt-Devel] [PATCH v2] ath79: use gpio_hog instead of gpio- > export > > Adrian Schmutzler <freifunk@adrianschmutzler.de> writes: > > > This patch converts all DTS files with settings that normally do > > not need user interaction, e.g. power for external USB ports, to > > gpio_hog. > > Wouldn't it be better to map these as fixed regulators? Then you could > eventually link them to the connected ports/controllers, and allow them > to be automatically turned off when not needed. I'm no expert for this stuff. But, based on the discussion here, the opposite has been identified as superior solution (discussing nand subtarget): https://github.com/openwrt/openwrt/pull/2184#discussion_r342136635 Despite, I remember a comment that fixed regulators won't be available on all SOCs... Best Adrian > > > Bjørn > > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
"Adrian Schmutzler" <mail@adrianschmutzler.de> writes: > But, based on the discussion here, the opposite has been identified as > superior solution (discussing nand subtarget): > https://github.com/openwrt/openwrt/pull/2184#discussion_r342136635 That's missing the point. Regulators are superior if there is controlling driver. E.g. https://www.kernel.org/doc/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt See arch/arm/boot/dts/armada-385-linksys.dtsi for a nice, OpenWrt relevant, example using this with a fixed regulator. If you don't link anything to the regulator, then I agree that you might as well use gpio-hog. But I still don't see how you can call that a superior solution. It doesn't suck more or less. I thought the ath79 conversion was all about using devicetree to document the boards ;-) Bjørn
Hi, > -----Original Message----- > From: Bjørn Mork [mailto:bjorn@mork.no] > Sent: Dienstag, 5. November 2019 20:01 > To: Adrian Schmutzler <mail@adrianschmutzler.de> > Cc: openwrt-devel@lists.openwrt.org; 'Birger Koblitz' <mail@birger-koblitz.de> > Subject: Re: [OpenWrt-Devel] [PATCH v2] ath79: use gpio_hog instead of gpio- > export > > "Adrian Schmutzler" <mail@adrianschmutzler.de> writes: > > > But, based on the discussion here, the opposite has been identified as > > superior solution (discussing nand subtarget): > > https://github.com/openwrt/openwrt/pull/2184#discussion_r342136635 > > That's missing the point. Regulators are superior if there is > controlling driver. E.g. > https://www.kernel.org/doc/Documentation/devicetree/bindings/usb/usb-nop- > xceiv.txt > > See arch/arm/boot/dts/armada-385-linksys.dtsi for a nice, OpenWrt > relevant, example using this with a fixed regulator. > > If you don't link anything to the regulator, then I agree that you might > as well use gpio-hog. But I still don't see how you can call that a > superior solution. It doesn't suck more or less. I thought the ath79 > conversion was all about using devicetree to document the boards ;-) Maybe "superior" was wrong here, as in the linked discussion the argument was that both are comparable, but gpio-hog requires less space. For the content of this patch, I just want to get rid of gpio-export. So, for the moment I think the gpio-hog is an improvement over gpio-export. Based on that, I'd be happy if someone would be willing to transition to an even better solution. :-) No offense, but I aim at the low-hanging fruit for this one. Best Adrian
"Adrian Schmutzler" <mail@adrianschmutzler.de> writes:
> So, for the moment I think the gpio-hog is an improvement over gpio-export.
Agreed 100%
Bjørn
Hi, > -----Original Message----- > From: openwrt-devel [mailto:openwrt-devel-bounces@lists.openwrt.org] > On Behalf Of Adrian Schmutzler > Sent: Dienstag, 5. November 2019 16:12 > To: openwrt-devel@lists.openwrt.org > Cc: Birger Koblitz <mail@birger-koblitz.de> > Subject: [OpenWrt-Devel] [PATCH v2] ath79: use gpio_hog instead of gpio- > export > > From: Birger Koblitz <mail@birger-koblitz.de> > > The gpio-export functionality is a hack for missing kernel functionality, which > was rejected in upstream kernel long time ago, for details see this email > http://lists.infradead.org/pipermail/openwrt-devel/2019- > February/015772.html, > discussion in PR#1366 or > https://github.com/openwrt/openwrt/pull/1814#issuecomment-462942022. > > This patch converts all DTS files with settings that normally do not need user > interaction, e.g. power for external USB ports, to gpio_hog. The only > remaining gpio-export will be in qca9558_openmesh_om5p-ac-v2.dts > I've just had a look at the openmesh_om5p-ac-v2, and it seems as if the gpio-exports there are just voltage changes for a power amplifier. To me this looks like those can be dealt with by a gpio-hog, too: /* power amplifier high power, 4.2V at RFFM4203/4503 instead of 3.3 */ ath79_gpio_function_enable(QCA955X_GPIO_FUNC_JTAG_DISABLE); ath79_gpio_output_select(OM5PACV2_GPIO_PA_DCDC, QCA955X_GPIO_OUT_GPIO); ath79_gpio_output_select(OM5PACV2_GPIO_PA_HIGH, QCA955X_GPIO_OUT_GPIO); gpio_request_one(OM5PACV2_GPIO_PA_DCDC, GPIOF_OUT_INIT_HIGH, "PA DC/DC"); gpio_request_one(OM5PACV2_GPIO_PA_HIGH, GPIOF_OUT_INIT_HIGH, "PA HIGH"); https://github.com/openwrt/openwrt/blob/master/target/linux/ar71xx/files/arch/mips/ath79/mach-om5pacv2.c Thus, I will also convert that device without an explicit resend of the patch unless there is protest about it. Best Adrian
BTW, being able to toggle on and off USB power is essential in some cases. Can this be done with hog? Thanks! E > Il giorno 5 nov 2019, alle ore 17:44, Adrian Schmutzler <mail@adrianschmutzler.de> ha scritto: > > Hi, > >> -----Original Message----- >> From: openwrt-devel [mailto:openwrt-devel-bounces@lists.openwrt.org] On >> Behalf Of Bjørn Mork >> Sent: Dienstag, 5. November 2019 17:38 >> To: Adrian Schmutzler <freifunk@adrianschmutzler.de> >> Cc: openwrt-devel@lists.openwrt.org; Birger Koblitz <mail@birger-koblitz.de> >> Subject: Re: [OpenWrt-Devel] [PATCH v2] ath79: use gpio_hog instead of gpio- >> export >> >> Adrian Schmutzler <freifunk@adrianschmutzler.de> writes: >> >>> This patch converts all DTS files with settings that normally do >>> not need user interaction, e.g. power for external USB ports, to >>> gpio_hog. >> >> Wouldn't it be better to map these as fixed regulators? Then you could >> eventually link them to the connected ports/controllers, and allow them >> to be automatically turned off when not needed. > > I'm no expert for this stuff. > > But, based on the discussion here, the opposite has been identified as superior solution (discussing nand subtarget): > https://github.com/openwrt/openwrt/pull/2184#discussion_r342136635 > > Despite, I remember a comment that fixed regulators won't be available on all SOCs... > > Best > > Adrian > > >> >> >> Bjørn >> >> _______________________________________________ >> openwrt-devel mailing list >> openwrt-devel@lists.openwrt.org >> https://lists.openwrt.org/mailman/listinfo/openwrt-devel > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Hi, > -----Original Message----- > From: Enrico Mioso [mailto:mrkiko.rs@gmail.com] > Sent: Dienstag, 5. November 2019 23:07 > To: Adrian Schmutzler <mail@adrianschmutzler.de> > Cc: Bjørn Mork <bjorn@mork.no>; openwrt-devel@lists.openwrt.org; Birger > Koblitz <mail@birger-koblitz.de> > Subject: Re: [OpenWrt-Devel] [PATCH v2] ath79: use gpio_hog instead of > gpio-export > > BTW, being able to toggle on and off USB power is essential in some cases. > Can this be done with hog? Thanks! > E with hogs, you cannot enable/disable. This is the reason why this topic got stuck in the first place. On ath79, it seems that usb_power is only used for external USB ports so far. So I felt that the discussion led about toggling USB on ramips would not apply here, and that choosing gpio_hogs over a user-space solution would be preferable for this case. With any new device support for the last 6 months, USB ports were set up with gpio_hogs IIRC. Best Adrian
Hi Adrian, On 05.11.2019 22:19, mail@adrianschmutzler.de wrote: > Hi, > >> -----Original Message----- >> From: openwrt-devel [mailto:openwrt-devel-bounces@lists.openwrt.org] >> On Behalf Of Adrian Schmutzler >> Sent: Dienstag, 5. November 2019 16:12 >> To: openwrt-devel@lists.openwrt.org >> Cc: Birger Koblitz <mail@birger-koblitz.de> >> Subject: [OpenWrt-Devel] [PATCH v2] ath79: use gpio_hog instead of gpio- >> export >> >> From: Birger Koblitz <mail@birger-koblitz.de> >> >> The gpio-export functionality is a hack for missing kernel functionality, which >> was rejected in upstream kernel long time ago, for details see this email >> http://lists.infradead.org/pipermail/openwrt-devel/2019- >> February/015772.html, >> discussion in PR#1366 or >> https://github.com/openwrt/openwrt/pull/1814#issuecomment-462942022. >> >> This patch converts all DTS files with settings that normally do not need user >> interaction, e.g. power for external USB ports, to gpio_hog. The only >> remaining gpio-export will be in qca9558_openmesh_om5p-ac-v2.dts >> > > I've just had a look at the openmesh_om5p-ac-v2, and it seems as if the gpio-exports there are just voltage changes for a power amplifier. To me this looks like those can be dealt with by a gpio-hog, too: What if someone would like to lower TX power on this board? With gpio-hog that wouldn't be possible anymore. I would personally consider that change as a user experience limitation or even a regression. I had this discussion many, many... many times before with Mathias (and I believe we still agree to disagree on this topic). Until there is a dedicated driver for such features controlled by GPIOs (lets say, SIM switching, driving relays, enabling higher power output in ext PA, etc.), switching from gpio-export to gpio-hog only limits user control on the hardware they own and in fact doesn't get us closer to something which could make the gpio-export finally go away (libgpiod). I'm always on the end user side here. If the hardware is capable of switching something with GPIO, user should have a way to make use of that. Even if current solution was rejected in upstream.
Hello Adrian, and all.
thank you for your work, again.
So, the TP-Link TL-MR6400 uses USB power for this. But in general, where there is a port and you can plug a {3,4}G Gadget, the need of "resetting" the device via USB power removal arises.
I know I am being pedantic here, and I am sorry for that, butjust wanted to point this out.
I think there are other devices using USB power for this purpose, even tough I don't know if they are supported by OpenWRt or not.
Here I am not proposing to hold your patch, just wondering what we can do about this use-case. I think it's an important use case BTW.
Thanks again,
Enrico
On Tue, 5 Nov 2019, mail@adrianschmutzler.de wrote:
> Date: Tue, 5 Nov 2019 23:11:07
> From: mail@adrianschmutzler.de
> To: Enrico Mioso <mrkiko.rs@gmail.com>
> Cc: Bjørn Mork <bjorn@mork.no>, openwrt-devel@lists.openwrt.org,
> Birger Koblitz <mail@birger-koblitz.de>
> Subject: RE: [OpenWrt-Devel] [PATCH v2] ath79: use gpio_hog instead of
> gpio-export
>
> Hi,
>
>> -----Original Message-----
>> From: Enrico Mioso [mailto:mrkiko.rs@gmail.com]
>> Sent: Dienstag, 5. November 2019 23:07
>> To: Adrian Schmutzler <mail@adrianschmutzler.de>
>> Cc: Bjørn Mork <bjorn@mork.no>; openwrt-devel@lists.openwrt.org; Birger
>> Koblitz <mail@birger-koblitz.de>
>> Subject: Re: [OpenWrt-Devel] [PATCH v2] ath79: use gpio_hog instead of
>> gpio-export
>>
>> BTW, being able to toggle on and off USB power is essential in some cases.
>> Can this be done with hog? Thanks!
>> E
>
> with hogs, you cannot enable/disable. This is the reason why this topic got stuck in the first place.
>
> On ath79, it seems that usb_power is only used for external USB ports so far. So I felt that the discussion led about toggling USB on ramips would not apply here, and that choosing gpio_hogs over a user-space solution would be preferable for this case. With any new device support for the last 6 months, USB ports were set up with gpio_hogs IIRC.
>
> Best
>
> Adrian
>
Hi, TL;DR: 1. We should find an agreement that can be used coherently at least for new device support submissions. 2. Everyone (and particular committers) feel invited to add your view. > > I've just had a look at the openmesh_om5p-ac-v2, and it seems as if the > gpio-exports there are just voltage changes for a power amplifier. To me this > looks like those can be dealt with by a gpio-hog, too: > > What if someone would like to lower TX power on this board? With gpio-hog > that wouldn't be possible anymore. I would personally consider that change > as a user experience limitation or even a regression. Well, normally I'd say to lower the TX power you would use the user-space txpower setting and not change voltages of an amplifier. Nevertheless, I'm not religious about the gpio-hogs, I just want to get the gpio-exports off the table. If the majority thinks everything should rely on 03_gpio_switches, I will happily implement it (though this might be a problem for people updating into it.). > > I had this discussion many, many... many times before with Mathias (and I > believe we still agree to disagree on this topic). Until there is a dedicated > driver for such features controlled by GPIOs (lets say, SIM switching, driving > relays, enabling higher power output in ext PA, etc.), switching from gpio- > export to gpio-hog only limits user control on the hardware they own and in > fact doesn't get us closer to something which could make the gpio-export > finally go away (libgpiod). Yes, I read the old discussion before I asked for closing it. Despite my desire to get rid of the "old" gpio-exports, note that we currently do not accept gpio-exports for new device support (for several months already). So there is no "keep gpio-exports until we have something better", unless we start accepting it for submissions again. > > I'm always on the end user side here. If the hardware is capable of switching > something with GPIO, user should have a way to make use of that. Even if > current solution was rejected in upstream. So, that would mean that we use 03_gpio_switches from now on, at least for new submissions. This would be a change of what mostly has been done so far (reviewers suggesting to use gpio_hog). The big majority of what we deal with is USB power. I've read Enrico's arguments, but I'm not really convinced that resetting an USB device by toggling its power really is a feature, and not just a workaround for a faulty USB device. That's why I personally can well live with having USB power for external ports set by hogs, and anything else converted to user space switches. But I also admit that you (Piotr) are right that this is a reduction of user power over the device (I suspect it would be the same with the fixed regulator?). At the end, I'm fine with gpio_hogs, 03_gpio_switches or a mixed solution, but I think it's time to have a decision on that topic, instead of determining the correct solution based on who is reviewing a patch. So please, share your views on this topic, so we might extract a path to go ahead. Best Adrian
On 11/5/19 11:01 AM, Bjørn Mork wrote: > "Adrian Schmutzler" <mail@adrianschmutzler.de> writes: > >> But, based on the discussion here, the opposite has been identified as >> superior solution (discussing nand subtarget): >> https://github.com/openwrt/openwrt/pull/2184#discussion_r342136635 > That's missing the point. Regulators are superior if there is > controlling driver. E.g. > https://www.kernel.org/doc/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt > > See arch/arm/boot/dts/armada-385-linksys.dtsi for a nice, OpenWrt > relevant, example using this with a fixed regulator. > > If you don't link anything to the regulator, then I agree that you might > as well use gpio-hog. But I still don't see how you can call that a > superior solution. It doesn't suck more or less. I thought the ath79 > conversion was all about using devicetree to document the boards ;-) > I agree that if the driver can control the regulator and do something useful with it (such as dropping it to hard-reset the USB devices), there are advantages. However, my recent work on the ath79-nand kernel shows that adding CONFIG_REGULATOR=y CONFIG_REGULATOR_FIXED_VOLTAGE=y results in an increase in kernel size of ~14 kB: 1,952,020 with regulator-fixed 1,937,164 with gpio-hog At least for the ath79-nand target, things are getting tight for a 2 MB kernel limit, with only a handful of boards and their "unique" hardware aspects accounted for yet. Jeff
Hi Adrian, On 06.11.2019 00:14, mail@adrianschmutzler.de wrote: > Hi, > > TL;DR: > 1. We should find an agreement that can be used coherently at least for new device support submissions. I believe the major issue here is that there is no 'in place' replacement for 'gpio-export' (or I'm just not aware of it). > 2. Everyone (and particular committers) feel invited to add your view. > >> > I've just had a look at the openmesh_om5p-ac-v2, and it seems as if the >> gpio-exports there are just voltage changes for a power amplifier. To me this >> looks like those can be dealt with by a gpio-hog, too: >> >> What if someone would like to lower TX power on this board? With gpio-hog >> that wouldn't be possible anymore. I would personally consider that change >> as a user experience limitation or even a regression. > > Well, normally I'd say to lower the TX power you would use the user-space txpower setting and not change voltages of an amplifier. It seems this one has two levels of amplification, depending on the supply voltage (3.3V vs. up to 5V). And based on the GPIO names and comments in code, this can be controlled with GPIOs. Another question here is 'should we allow user to change such settings?'. I'm not that strict and I believe user is owner of the hardware and should be able to do with it whatever is possible but I know there are developers with different opinions on the subject. > Nevertheless, I'm not religious about the gpio-hogs, I just want to get the gpio-exports off the table. If the majority thinks everything should rely on 03_gpio_switches, I will happily implement it (though this might be a problem for people updating into it.). Are there any other reasons to get rid of 'gpio-export' _now_, other than the fact upstream rejected this approach? Wouldn't it make more sense to spend time now on implementing future-proof solution and switch to it when it's ready? >> I had this discussion many, many... many times before with Mathias (and I >> believe we still agree to disagree on this topic). Until there is a dedicated >> driver for such features controlled by GPIOs (lets say, SIM switching, driving >> relays, enabling higher power output in ext PA, etc.), switching from gpio- >> export to gpio-hog only limits user control on the hardware they own and in >> fact doesn't get us closer to something which could make the gpio-export >> finally go away (libgpiod). > > Yes, I read the old discussion before I asked for closing it. Despite my desire to get rid of the "old" gpio-exports, note that we currently do not accept gpio-exports for new device support (for several months already). So there is no "keep gpio-exports until we have something better", unless we start accepting it for submissions again. Even if there is no alternative? We should really make such decision more transparent, public and give users and downstream projects time to adopt for the change. >> I'm always on the end user side here. If the hardware is capable of switching >> something with GPIO, user should have a way to make use of that. Even if >> current solution was rejected in upstream. > > So, that would mean that we use 03_gpio_switches from now on, at least for new submissions. This would be a change of what mostly has been done so far (reviewers suggesting to use gpio_hog). '03_gpio_switches' doesn't handle inputs. Of course, it has advantages, like the fact it makes the GPIO setup uci-based but on the other hand... it does its job fairly late during bootup. In some cases, you might want to, for example, enable power for 3/4G modem as early as possible, to give it time to register in network. Anyway, under the hood, it's the same approach, export named GPIO using _deprecated_ sysfs. Excluding uci and place in boot time where it happens, the difference is where the GPIOs are defined, DTS vs. user-space scripts. > The big majority of what we deal with is USB power. I've read Enrico's arguments, but I'm not really convinced that resetting an USB device by toggling its power really is a feature, and not just a workaround for a faulty USB device. That's why I personally can well live with having USB power for external ports set by hogs, and anything else converted to user space switches. But I also admit that you (Piotr) are right that this is a reduction of user power over the device (I suspect it would be the same with the fixed regulator?). I suppose you don't have much experience with USB based 3/4G modules :) Faulty or not, workaround or just user's need, people are using them and we shouldn't make it hard for them. I definitely wouldn't want OpenWrt to become a platform where user is... just a user of 'rented' hardware, like many vendors are trying to achieve. In case of VBUS I'm pretty sure 'regulator' is a better approach than 'gpio-hog'. At least in that case there is a way to disable VBUS by unloading host driver (see for example 'mt7621_xiaomi_mir3g.dts' under ramips). But still, there are corner cases (based on a real device) - this won't work if device has two USB ports (under the same HUB on single bus), with separated GPIO-controlled VBUS supply. > At the end, I'm fine with gpio_hogs, 03_gpio_switches or a mixed solution, but I think it's time to have a decision on that topic, instead of determining the correct solution based on who is reviewing a patch. I believe it's time to think about future-proof solution first. > So please, share your views on this topic, so we might extract a path to go ahead. IMHO, 'gpio-hog' should replace 'gpio-export' where there is no other, more suitable solution (dedicated driver, like 'regulator') but not before we can offer users replacement for deprecated sysfs approach. Thus, personally I would prefer to start discussion about our 'version' of libgpiod instead. That's my view.
<mail@adrianschmutzler.de> writes: > I'm not really convinced that resetting an USB device by toggling its > power really is a feature, and not just a workaround for a faulty USB > device. A workaround for a faulty USB device *is* a feature :-) This feature is very important to at least one group of users, as Enrico points out. Most 3G/4G modem firmware is unreliable and unstable, and will crash from time to time. We can't fix that. Most of the time, the modem will reboot and everything is fine by itself. But sometimes it gets stuck in a state where we need to toggle power to get it running again. This is a major problem if the modem is connected to an OpenWrt router in a remote location. Being able to control USB port power toggling in software is essential for such systems. It does not matter whether the modem is internally or externally connected, if the OpenWrt system itself is difficult to access. Bjørn
<mail@adrianschmutzler.de> writes: > Well, normally I'd say to lower the TX power you would use the > user-space txpower setting and not change voltages of an amplifier. This choice will affect S/N. You'll probably limit the usable txpower range this way. Please use the features of the hardware. Don't hide them. Sure, all hardware supporting code will use some flash. So what? Are you going to support the hardware or not? If you aren't, then I don't think there is any value in OpenWrt. Bjørn
Hi, > Wouldn't it make more sense to spend time now on implementing > future-proof solution and switch to it when it's ready? Obviously, yes. But for the meantime, I'd like to have a less-arbitrary status quo. > I believe the major issue here is that there is no 'in place' > replacement for 'gpio-export' (or I'm just not aware of it). > [...] > > Are there any other reasons to get rid of 'gpio-export' _now_, other > than the fact upstream rejected this approach? > [...] > > '03_gpio_switches' doesn't handle inputs. > > Of course, it has advantages, like the fact it makes the GPIO setup > uci-based but on the other hand... it does its job fairly late during > bootup. In some cases, you might want to, for example, enable power for > 3/4G modem as early as possible, to give it time to register in network. > > Anyway, under the hood, it's the same approach, export named GPIO using > _deprecated_ sysfs. Excluding uci and place in boot time where it > happens, the difference is where the GPIOs are defined, DTS vs. > user-space scripts. > So, both 03_gpio_switches and gpio-hogs provide less functionality than gpio-exports with no striking benefit. From that point of view we should actually allow gpio-exports in device support submissions again, and actually discourage gpio_hogs for the status quo ... (and it would be better to convert hogs to exports and not the other way around ...) Best Adrian
Hi Adrian, Sorry for a late reply. On 06.11.2019 16:47, Adrian Schmutzler wrote: > Hi, > >> Wouldn't it make more sense to spend time now on implementing >> future-proof solution and switch to it when it's ready? > > Obviously, yes. But for the meantime, I'd like to have a less-arbitrary status quo. For me, in that case, I would leave decision to the author of support _and_ reviewer/committer. >> I believe the major issue here is that there is no 'in place' >> replacement for 'gpio-export' (or I'm just not aware of it). >> > [...] >> >> Are there any other reasons to get rid of 'gpio-export' _now_, other >> than the fact upstream rejected this approach? >> > [...] >> >> '03_gpio_switches' doesn't handle inputs. >> >> Of course, it has advantages, like the fact it makes the GPIO setup >> uci-based but on the other hand... it does its job fairly late during >> bootup. In some cases, you might want to, for example, enable power for >> 3/4G modem as early as possible, to give it time to register in network. >> >> Anyway, under the hood, it's the same approach, export named GPIO using >> _deprecated_ sysfs. Excluding uci and place in boot time where it >> happens, the difference is where the GPIOs are defined, DTS vs. >> user-space scripts. >> > > So, both 03_gpio_switches and gpio-hogs provide less functionality than gpio-exports with no striking benefit. From that point of view we should actually allow gpio-exports in device support submissions again, and actually discourage gpio_hogs for the status quo ... (and it would be better to convert hogs to exports and not the other way around ...) Someone could say that 'gpio-hog' is the accepted solution in upstream and the 'gpio-export' is the rejected one so we need to get rid of it ASAP. Personally, I don't see now any good reason to convert everything back to 'gpio-export'. I would be just more pragmatic when reviewing and accepting boards support - if the author thinks that it would be better (look at it from usability point of view) to have user-space control on a specific GPIO line, I wouldn't ask him to use 'gpio-hog'. For me, also the uci approach is fine if there is no need to setup GPIO before the whole boot process finishes. Still, in some cases maybe 'fixed-regulator' would fit even better than discussed solution. IIRC, at least in case of the USB, there is still a way to have control on the VBUS if 'fixed-regulator' is used (unload the driver, power goes off, load it back, power goes on). I just don't think it makes sense to look for a consensus now on something which for sure has to go away/change in, I hope close future.
Hi Piotr, > -----Original Message----- > From: openwrt-devel [mailto:openwrt-devel-bounces@lists.openwrt.org] > On Behalf Of Piotr Dymacz > Sent: Samstag, 16. November 2019 15:51 > To: Adrian Schmutzler <mail@adrianschmutzler.de>; openwrt- > devel@lists.openwrt.org > Cc: bjorn@mork.no; 'Enrico Mioso' <mrkiko.rs@gmail.com>; 'Mathias Kresin' > <dev@kresin.me>; 'Birger Koblitz' <mail@birger-koblitz.de> > Subject: Re: [OpenWrt-Devel] [PATCH v2] ath79: use gpio_hog instead of > gpio-export > > Hi Adrian, > > Sorry for a late reply. > > On 06.11.2019 16:47, Adrian Schmutzler wrote: > > Hi, > > > >> Wouldn't it make more sense to spend time now on implementing > >> future-proof solution and switch to it when it's ready? > > > > Obviously, yes. But for the meantime, I'd like to have a less-arbitrary status > quo. > > For me, in that case, I would leave decision to the author of support _and_ > reviewer/committer. > > >> I believe the major issue here is that there is no 'in place' > >> replacement for 'gpio-export' (or I'm just not aware of it). > >> > > [...] > >> > >> Are there any other reasons to get rid of 'gpio-export' _now_, other > >> than the fact upstream rejected this approach? > >> > > [...] > >> > >> '03_gpio_switches' doesn't handle inputs. > >> > >> Of course, it has advantages, like the fact it makes the GPIO setup > >> uci-based but on the other hand... it does its job fairly late during > >> bootup. In some cases, you might want to, for example, enable power > >> for 3/4G modem as early as possible, to give it time to register in network. > >> > >> Anyway, under the hood, it's the same approach, export named GPIO > >> using _deprecated_ sysfs. Excluding uci and place in boot time where > >> it happens, the difference is where the GPIOs are defined, DTS vs. > >> user-space scripts. > >> > > > > So, both 03_gpio_switches and gpio-hogs provide less functionality > > than gpio-exports with no striking benefit. From that point of view we > > should actually allow gpio-exports in device support submissions > > again, and actually discourage gpio_hogs for the status quo ... (and > > it would be better to convert hogs to exports and not the other way > > around ...) > > Someone could say that 'gpio-hog' is the accepted solution in upstream and > the 'gpio-export' is the rejected one so we need to get rid of it ASAP. > > Personally, I don't see now any good reason to convert everything back to > 'gpio-export'. I would be just more pragmatic when reviewing and accepting > boards support - if the author thinks that it would be better (look at it from > usability point of view) to have user-space control on a specific GPIO line, I > wouldn't ask him to use 'gpio-hog'. For me, also the uci approach is fine if > there is no need to setup GPIO before the whole boot process finishes. > > Still, in some cases maybe 'fixed-regulator' would fit even better than > discussed solution. IIRC, at least in case of the USB, there is still a way to have > control on the VBUS if 'fixed-regulator' is used (unload the driver, power > goes off, load it back, power goes on). > > I just don't think it makes sense to look for a consensus now on something > which for sure has to go away/change in, I hope close future. okay, I accept that. I've marked this particular patch with "Rejected" already a week ago I think, but since it is consensus that replacement of gpio-export by gpio-hog reduces functionality, I will also mark the other patches attempting this conversion as "Rejected". Best Adrian > > -- > Cheers, > Piotr > > > > > Best > > > > Adrian > > > > > > _______________________________________________ > > openwrt-devel mailing list > > openwrt-devel@lists.openwrt.org > > https://lists.openwrt.org/mailman/listinfo/openwrt-devel > > > > > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
diff --git a/target/linux/ath79/dts/ar7161_buffalo_wzr-hp-ag300h.dts b/target/linux/ath79/dts/ar7161_buffalo_wzr-hp-ag300h.dts index f51bc0f771..23f1845876 100644 --- a/target/linux/ath79/dts/ar7161_buffalo_wzr-hp-ag300h.dts +++ b/target/linux/ath79/dts/ar7161_buffalo_wzr-hp-ag300h.dts @@ -120,16 +120,6 @@ }; }; - gpio-export { - compatible = "gpio-export"; - - gpio_usb_power { - gpio-export,name = "buffalo:power:usb"; - gpio-export,output = <1>; - gpios = <&gpio 2 GPIO_ACTIVE_HIGH>; - }; - }; - flash { compatible = "mtd-concat"; @@ -172,6 +162,15 @@ }; }; +&gpio { + usb_power { + gpio-hog; + line-name = "buffalo:power:usb"; + gpios = <2 GPIO_ACTIVE_HIGH>; + output-high; + }; +}; + &usb_phy { status = "okay"; }; diff --git a/target/linux/ath79/dts/ar7241_tplink_tl-mr3x20.dtsi b/target/linux/ath79/dts/ar7241_tplink_tl-mr3x20.dtsi index 04403637b6..333ee17ceb 100644 --- a/target/linux/ath79/dts/ar7241_tplink_tl-mr3x20.dtsi +++ b/target/linux/ath79/dts/ar7241_tplink_tl-mr3x20.dtsi @@ -3,15 +3,14 @@ #include "ar7241_tplink.dtsi" / { - gpio-export { - compatible = "gpio-export"; - #size-cells = <0>; +}; - gpio_usb_power { - gpio-export,name = "tp-link:power:usb"; - gpio-export,output = <1>; - gpios = <&gpio 6 GPIO_ACTIVE_HIGH>; - }; +&gpio { + usb_power { + gpio-hog; + line-name = "tp-link:power:usb"; + gpios = <6 GPIO_ACTIVE_HIGH>; + output-high; }; }; diff --git a/target/linux/ath79/dts/ar7241_tplink_tl-wr842n-v1.dts b/target/linux/ath79/dts/ar7241_tplink_tl-wr842n-v1.dts index 162b5f2838..ee468df244 100644 --- a/target/linux/ath79/dts/ar7241_tplink_tl-wr842n-v1.dts +++ b/target/linux/ath79/dts/ar7241_tplink_tl-wr842n-v1.dts @@ -66,15 +66,16 @@ linux,default-trigger = "phy0tpt"; }; }; +}; - gpio-export { - compatible = "gpio-export"; +&gpio { + status = "okay"; - gpio_usb_power { - gpio-export,name = "tp-link:power:usb"; - gpio-export,output = <1>; - gpios = <&gpio 6 GPIO_ACTIVE_HIGH>; - }; + usb_power { + gpio-hog; + line-name = "tp-link:power:usb"; + gpios = <6 GPIO_ACTIVE_HIGH>; + output-high; }; }; @@ -155,10 +156,6 @@ mtd-mac-address-increment = <1>; }; -&gpio { - status = "okay"; -}; - &uart { status = "okay"; }; diff --git a/target/linux/ath79/dts/ar7242_buffalo_wzr-bhr.dtsi b/target/linux/ath79/dts/ar7242_buffalo_wzr-bhr.dtsi index 3b5a4dd13d..d7632faf5c 100644 --- a/target/linux/ath79/dts/ar7242_buffalo_wzr-bhr.dtsi +++ b/target/linux/ath79/dts/ar7242_buffalo_wzr-bhr.dtsi @@ -57,17 +57,6 @@ }; }; - gpio-export { - compatible = "gpio-export"; - #size-cells = <0>; - - gpio_usb_power { - gpio-export,name = "buffalo:usb-power"; - gpio-export,output = <1>; - gpios = <&gpio 16 GPIO_ACTIVE_HIGH>; - }; - }; - virtual_flash { compatible = "mtd-concat"; devices = <&flash0 &flash1>; @@ -109,6 +98,15 @@ }; }; +&gpio { + usb_power { + gpio-hog; + line-name = "buffalo:usb-power"; + gpios = <16 GPIO_ACTIVE_HIGH>; + output-high; + }; +}; + &spi { status = "okay"; cs-gpios = <0>, <0>; diff --git a/target/linux/ath79/dts/ar7242_buffalo_wzr-hp-g302h-a1a0.dts b/target/linux/ath79/dts/ar7242_buffalo_wzr-hp-g302h-a1a0.dts index 8ac4df2194..2b30b7830b 100644 --- a/target/linux/ath79/dts/ar7242_buffalo_wzr-hp-g302h-a1a0.dts +++ b/target/linux/ath79/dts/ar7242_buffalo_wzr-hp-g302h-a1a0.dts @@ -109,17 +109,6 @@ }; }; - gpio-export { - compatible = "gpio-export"; - #size-cells = <0>; - - gpio_usb_power { - gpio-export,name = "buffalo:usb-power"; - gpio-export,output = <1>; - gpios = <&gpio 13 GPIO_ACTIVE_HIGH>; - }; - }; - virtual_flash { compatible = "mtd-concat"; devices = <&flash0 &flash1>; @@ -161,6 +150,15 @@ }; }; +&gpio { + usb_power { + gpio-hog; + line-name = "buffalo:usb-power"; + gpios = <13 GPIO_ACTIVE_HIGH>; + output-high; + }; +}; + &spi { status = "okay"; cs-gpios = <0>, <0>; diff --git a/target/linux/ath79/dts/ar9341_tplink_tl-wr842n-v2.dts b/target/linux/ath79/dts/ar9341_tplink_tl-wr842n-v2.dts index 86a316b518..1df0e27cb2 100644 --- a/target/linux/ath79/dts/ar9341_tplink_tl-wr842n-v2.dts +++ b/target/linux/ath79/dts/ar9341_tplink_tl-wr842n-v2.dts @@ -6,16 +6,6 @@ / { model = "TP-Link TL-WR842N/ND v2"; compatible = "tplink,tl-wr842n-v2", "qca,ar9341"; - - gpio-export { - compatible = "gpio-export"; - - gpio_usb_power { - gpio-export,name = "tp-link:power:usb"; - gpio-export,output = <1>; - gpios = <&gpio 4 GPIO_ACTIVE_HIGH>; - }; - }; }; &keys { @@ -36,6 +26,15 @@ }; }; +&gpio { + usb_power { + gpio-hog; + line-name = "tp-link:power:usb"; + gpios = <4 GPIO_ACTIVE_HIGH>; + output-high; + }; +}; + &spi { num-cs = <1>; diff --git a/target/linux/ath79/dts/qca9558_devolo_dvl1750e.dts b/target/linux/ath79/dts/qca9558_devolo_dvl1750e.dts index 3d25004c40..e790cf0df4 100644 --- a/target/linux/ath79/dts/qca9558_devolo_dvl1750e.dts +++ b/target/linux/ath79/dts/qca9558_devolo_dvl1750e.dts @@ -53,15 +53,14 @@ compatible = "gpio-beeper"; gpios = <&gpio 4 GPIO_ACTIVE_HIGH>; }; +}; - gpio_export { - compatible = "gpio-export"; - - gpio_usb_power { - gpio-export,name = "devolo:power:usb"; - gpio-export,output = <1>; - gpios = <&gpio 11 GPIO_ACTIVE_HIGH>; - }; +&gpio { + usb_power { + gpio-hog; + line-name = "devolo:power:usb"; + gpios = <11 GPIO_ACTIVE_HIGH>; + output-high; }; }; diff --git a/target/linux/ath79/dts/qca9558_tplink_archer-c7.dtsi b/target/linux/ath79/dts/qca9558_tplink_archer-c7.dtsi index c77613dad6..fb38f35dc1 100644 --- a/target/linux/ath79/dts/qca9558_tplink_archer-c7.dtsi +++ b/target/linux/ath79/dts/qca9558_tplink_archer-c7.dtsi @@ -64,22 +64,6 @@ debounce-interval = <60>; }; }; - - gpio-export { - compatible = "gpio-export"; - - gpio_usb1_power { - gpio-export,name = "tp-link:power:usb1"; - gpio-export,output = <1>; - gpios = <&gpio 21 GPIO_ACTIVE_HIGH>; - }; - - gpio_usb2_power { - gpio-export,name = "tp-link:power:usb2"; - gpio-export,output = <1>; - gpios = <&gpio 22 GPIO_ACTIVE_HIGH>; - }; - }; }; &pcie1 { @@ -92,6 +76,20 @@ &gpio { status = "okay"; + + usb1_power { + gpio-hog; + line-name = "tp-link:power:usb1"; + gpios = <21 GPIO_ACTIVE_HIGH>; + output-high; + }; + + usb2_power { + gpio-hog; + line-name = "tp-link:power:usb2"; + gpios = <22 GPIO_ACTIVE_HIGH>; + output-high; + }; }; &usb_phy0 { diff --git a/target/linux/ath79/dts/qca9558_tplink_tl-wdr4900-v2.dts b/target/linux/ath79/dts/qca9558_tplink_tl-wdr4900-v2.dts index 77166b8bfe..470b742578 100644 --- a/target/linux/ath79/dts/qca9558_tplink_tl-wdr4900-v2.dts +++ b/target/linux/ath79/dts/qca9558_tplink_tl-wdr4900-v2.dts @@ -57,7 +57,7 @@ }; }; - ath9k-leds { + ath9k-leds { compatible = "gpio-leds"; wlan5g { @@ -77,21 +77,23 @@ debounce-interval = <60>; }; }; +}; - gpio-export { - compatible = "gpio-export"; +&gpio { + status = "okay"; - gpio_usb1_power { - gpio-export,name = "tp-link:power:usb1"; - gpio-export,output = <1>; - gpios = <&gpio 21 GPIO_ACTIVE_HIGH>; - }; + usb1_power { + gpio-hog; + line-name = "tp-link:power:usb1"; + gpios = <21 GPIO_ACTIVE_HIGH>; + output-high; + }; - gpio_usb2_power { - gpio-export,name = "tp-link:power:usb2"; - gpio-export,output = <1>; - gpios = <&gpio 22 GPIO_ACTIVE_HIGH>; - }; + usb2_power { + gpio-hog; + line-name = "tp-link:power:usb2"; + gpios = <22 GPIO_ACTIVE_HIGH>; + output-high; }; }; diff --git a/target/linux/ath79/dts/qca9558_tplink_tl-wr1043nd.dtsi b/target/linux/ath79/dts/qca9558_tplink_tl-wr1043nd.dtsi index 1092250f02..f6d34ab3ff 100644 --- a/target/linux/ath79/dts/qca9558_tplink_tl-wr1043nd.dtsi +++ b/target/linux/ath79/dts/qca9558_tplink_tl-wr1043nd.dtsi @@ -64,16 +64,16 @@ debounce-interval = <60>; }; }; +}; - gpio-export { - compatible = "gpio-export"; - #size-cells = <0>; +&gpio { + status = "okay"; - gpio_usb_power { - gpio-export,name = "tp-link:power:usb"; - gpio-export,output = <1>; - gpios = <&gpio 21 GPIO_ACTIVE_HIGH>; - }; + usb_power { + gpio-hog; + line-name = "tp-link:power:usb"; + gpios = <21 GPIO_ACTIVE_HIGH>; + output-high; }; }; @@ -81,10 +81,6 @@ status = "okay"; }; -&gpio { - status = "okay"; -}; - &usb_phy0 { status = "okay"; }; diff --git a/target/linux/ath79/dts/qca9561_tplink_archer-c5x.dtsi b/target/linux/ath79/dts/qca9561_tplink_archer-c5x.dtsi index ddf92108b5..53329f2268 100644 --- a/target/linux/ath79/dts/qca9561_tplink_archer-c5x.dtsi +++ b/target/linux/ath79/dts/qca9561_tplink_archer-c5x.dtsi @@ -103,22 +103,6 @@ gpios = <&gpio 21 GPIO_ACTIVE_LOW>; }; }; - - gpio-export { - compatible = "gpio-export"; - - gpio_shift_register_oe { - gpio-export,name = "tp-link:oe:sr"; - gpio-export,output = <0>; - gpios = <&gpio 16 GPIO_ACTIVE_HIGH>; - }; - - gpio_shift_register_reset { - gpio-export,name = "tp-link:reset:sr"; - gpio-export,output = <1>; - gpios = <&gpio 19 GPIO_ACTIVE_HIGH>; - }; - }; }; &uart { @@ -127,6 +111,20 @@ &gpio { status = "okay"; + + shift_register_oe { + gpio-hog; + line-name = "tp-link:oe:sr"; + gpios = <16 GPIO_ACTIVE_HIGH>; + output-low; + }; + + shift_register_reset { + gpio-hog; + line-name = "tp-link:reset:sr"; + gpios = <19 GPIO_ACTIVE_HIGH>; + output-high; + }; }; &pcie { diff --git a/target/linux/ath79/dts/qca9563_dlink_dir-859-a1.dts b/target/linux/ath79/dts/qca9563_dlink_dir-859-a1.dts index a17d9f263a..63baa376c6 100644 --- a/target/linux/ath79/dts/qca9563_dlink_dir-859-a1.dts +++ b/target/linux/ath79/dts/qca9563_dlink_dir-859-a1.dts @@ -61,16 +61,16 @@ debounce-interval = <60>; }; }; +}; - gpio-export { - compatible = "gpio-export"; - #size-cells = <0>; +&gpio { + status = "okay"; - gpio_switch_reset { - gpio-export,name = "dir-859-a1:reset:switch"; - gpio-export,output = <1>; - gpios = <&gpio 11 GPIO_ACTIVE_HIGH>; - }; + switch_reset { + gpio-hog; + line-name = "dir-859-a1:reset:switch"; + gpios = <11 GPIO_ACTIVE_HIGH>; + output-high; }; }; @@ -78,10 +78,6 @@ status = "okay"; }; -&gpio { - status = "okay"; -}; - &pcie { status = "okay"; }; diff --git a/target/linux/ath79/dts/qca9563_tplink_archer-c7-v4.dts b/target/linux/ath79/dts/qca9563_tplink_archer-c7-v4.dts index 470a8e6bf9..66083aa7f9 100644 --- a/target/linux/ath79/dts/qca9563_tplink_archer-c7-v4.dts +++ b/target/linux/ath79/dts/qca9563_tplink_archer-c7-v4.dts @@ -42,22 +42,6 @@ }; }; - gpio-export { - compatible = "gpio-export"; - - gpio_shift_register_oe { - gpio-export,name = "tp-link:oe:sr"; - gpio-export,output = <0>; - gpios = <&gpio 1 GPIO_ACTIVE_LOW>; // 74HC595 /OE (Output Enable) - }; - - gpio_shift_register_reset { - gpio-export,name = "tp-link:reset:sr"; - gpio-export,output = <1>; - gpios = <&gpio 21 GPIO_ACTIVE_LOW>; // 74HC595 /SRCLR (Serial Clear) - }; - }; - leds { compatible = "gpio-leds"; @@ -158,6 +142,20 @@ &gpio { status = "okay"; + + shift_register_oe { // 74HC595 /OE + gpio-hog; + line-name = "tp-link:oe:sr"; + gpios = <1 GPIO_ACTIVE_LOW>; + output-low; + }; + + shift_register_reset { // 74HC595 /SRCLR + gpio-hog; + line-name = "tp-link:reset:sr"; + gpios = <21 GPIO_ACTIVE_LOW>; + output-high; + }; }; &usb_phy0 { diff --git a/target/linux/ath79/dts/qca9563_tplink_archer-x7-v5.dtsi b/target/linux/ath79/dts/qca9563_tplink_archer-x7-v5.dtsi index 2e66e0f03e..64115c4248 100644 --- a/target/linux/ath79/dts/qca9563_tplink_archer-x7-v5.dtsi +++ b/target/linux/ath79/dts/qca9563_tplink_archer-x7-v5.dtsi @@ -93,16 +93,6 @@ debounce-interval = <60>; }; }; - - gpio-export { - compatible = "gpio-export"; - - gpio_usb_power { - gpio-export,name = "tp-link:power:usb"; - gpio-export,output = <1>; - gpios = <&gpio 19 GPIO_ACTIVE_HIGH>; - }; - }; }; &pcie { @@ -115,6 +105,13 @@ &gpio { status = "okay"; + + usb_power { + gpio-hog; + line-name = "tp-link:power:usb"; + gpios = <19 GPIO_ACTIVE_HIGH>; + output-high; + }; }; &usb_phy0 { diff --git a/target/linux/ath79/dts/qca9563_tplink_tl-wr1043nd-v4.dts b/target/linux/ath79/dts/qca9563_tplink_tl-wr1043nd-v4.dts index 07a7409886..45ee0c21c7 100644 --- a/target/linux/ath79/dts/qca9563_tplink_tl-wr1043nd-v4.dts +++ b/target/linux/ath79/dts/qca9563_tplink_tl-wr1043nd-v4.dts @@ -6,16 +6,14 @@ / { compatible = "tplink,tl-wr1043nd-v4", "qca,qca9563"; model = "TP-Link TL-WR1043ND v4"; +}; - gpio-export { - compatible = "gpio-export"; - #size-cells = <0>; - - gpio_usb_power { - gpio-export,name = "tp-link:power:usb"; - gpio-export,output = <1>; - gpios = <&gpio 8 GPIO_ACTIVE_HIGH>; - }; +&gpio { + usb_power { + gpio-hog; + line-name = "tp-link:power:usb"; + gpios = <8 GPIO_ACTIVE_HIGH>; + output-high; }; };