diff mbox

ARM: mvebu: GPIO reset on GlobalScale Mirabox

Message ID 1427319230-2500-1-git-send-email-leigh@solinno.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Leigh Brown March 25, 2015, 9:33 p.m. UTC
Add support for the system reset button on the GlobalScale Mirabox.
The reset button is connected via GPIO so add a gpio-keys entry for
it in the device tree file.  Patch is based on the implementation
in armada-370-netgear-rn102.dts but using the GPIO pin specified in
the original patch by Ezequiel Garcia from 2013.

Tested with the triggerhappy daemon.

Signed-off-by: Leigh Brown <leigh@solinno.co.uk>
---
 arch/arm/boot/dts/armada-370-mirabox.dts | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

Comments

Gregory CLEMENT March 25, 2015, 10:16 p.m. UTC | #1
Hi Leigh,

On 25/03/2015 22:33, Leigh Brown wrote:
> Add support for the system reset button on the GlobalScale Mirabox.
> The reset button is connected via GPIO so add a gpio-keys entry for
> it in the device tree file.  Patch is based on the implementation
> in armada-370-netgear-rn102.dts but using the GPIO pin specified in
> the original patch by Ezequiel Garcia from 2013.
> 

Thanks for your contribution, however the reset button is not always
connected to this GPIO. At least on the Mirabox I have the reset button
triggers an hardware reset. My concern is that for some board the dts
representation would be wrong.

I am not really against this patch, first because it is possible that the
board I got was an exception, but as we don't have any feedback from GlobalScale,
it is hard to figure out. Then, I am not sure that exposing this reset button
when it was not trigger by software was a problem. I have to test it and also
think about it.

I would like to have the opinion of other people about it.


Thanks,

Gregory



> Tested with the triggerhappy daemon.
> 
> Signed-off-by: Leigh Brown <leigh@solinno.co.uk>
> ---
>  arch/arm/boot/dts/armada-370-mirabox.dts | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/armada-370-mirabox.dts b/arch/arm/boot/dts/armada-370-mirabox.dts
> index e1b0eb6..41f453d 100644
> --- a/arch/arm/boot/dts/armada-370-mirabox.dts
> +++ b/arch/arm/boot/dts/armada-370-mirabox.dts
> @@ -9,6 +9,7 @@
>   */
>  
>  /dts-v1/;
> +#include <dt-bindings/input/input.h>
>  #include <dt-bindings/gpio/gpio.h>
>  #include "armada-370.dtsi"
>  
> @@ -78,6 +79,18 @@
>  				};
>  			};
>  
> +			gpio-keys {
> +				compatible = "gpio-keys";
> +				pinctrl-0 = <&reset_button_pin>;
> +				pinctrl-names = "default";
> +
> +				reset-button {
> +					label = "Reset Button";
> +					linux,code = <KEY_RESTART>;
> +					gpios = <&gpio1 26 GPIO_ACTIVE_LOW>;
> +				};
> +			};
> +
>  			mdio {
>  				pinctrl-0 = <&mdio_pins>;
>  				pinctrl-names = "default";
> @@ -168,5 +181,10 @@
>  		marvell,pins = "mpp64", "mpp65";
>  		marvell,function = "gpio";
>  	};
> +
> +	reset_button_pin: reset-button-pin {
> +		marvell,pins = "mpp58";
> +		marvell,function = "gpio";
> +	};
>  };
>  
>
Andrew Lunn March 25, 2015, 10:36 p.m. UTC | #2
> Thanks for your contribution, however the reset button is not always
> connected to this GPIO. At least on the Mirabox I have the reset button
> triggers an hardware reset. My concern is that for some board the dts
> representation would be wrong.

When looking at WiFi issues, it became clear there are two different
wifi designs. I would not be too surprised if this reset button
changed at the same time.

Gregory, Leigh, is your wifi on the SDIO bus, or the USB bus?

The mirabox i have has an sdio wifi device. But i cannot easily test
the reset button, i blew the 5v power rail, so all USB is dead, and my
rootfs was on mmc, which is implemented via USB :-(

       Andrew
Leigh Brown March 25, 2015, 11 p.m. UTC | #3
Hi Gregory,

On 2015-03-25 22:16, Gregory CLEMENT wrote:
> On 25/03/2015 22:33, Leigh Brown wrote:
>> Add support for the system reset button on the GlobalScale Mirabox.
>> The reset button is connected via GPIO so add a gpio-keys entry for
>> it in the device tree file.  Patch is based on the implementation
>> in armada-370-netgear-rn102.dts but using the GPIO pin specified in
>> the original patch by Ezequiel Garcia from 2013.
> 
> Thanks for your contribution, however the reset button is not always
> connected to this GPIO. At least on the Mirabox I have the reset button
> triggers an hardware reset. My concern is that for some board the dts
> representation would be wrong.

I wish I had your Mirabox, because having the reset button on a GPIO is
almost useless :-(

However, I have what I have so it would be nice to find a way to support
it.

Regards,

Leigh.
Gregory CLEMENT March 25, 2015, 11:10 p.m. UTC | #4
Hi,

On 26/03/2015 00:16, Leigh Brown wrote:
> Hi Andrew,
> 
> On 2015-03-25 22:36, Andrew Lunn wrote:
>>> Thanks for your contribution, however the reset button is not always
>>> connected to this GPIO. At least on the Mirabox I have the reset 
>>> button
>>> triggers an hardware reset. My concern is that for some board the dts
>>> representation would be wrong.
>>
>> When looking at WiFi issues, it became clear there are two different
>> wifi designs. I would not be too surprised if this reset button
>> changed at the same time.
>>
>> Gregory, Leigh, is your wifi on the SDIO bus, or the USB bus?
>>
>> The mirabox i have has an sdio wifi device. But i cannot easily test
>> the reset button, i blew the 5v power rail, so all USB is dead, and my
>> rootfs was on mmc, which is implemented via USB :-(
> 
> The WiFi on my Mirabox is on the SDIO bus.
> 
> [   29.408420] mwifiex_sdio mmc0:0001:1: WLAN FW already running! Skip 
> FW dnld
> [   29.408430] mwifiex_sdio mmc0:0001:1: WLAN FW is active
> [   29.511297] mwifiex_sdio mmc0:0001:1: driver_version = mwifiex 1.0 
> (14.66.35.p52)

I think it is the same for me. I brought my Mirabox at ELC, but it is in
my room and just after ELC I will go to the airport. If I find a plug
there, I will boot it and I will confirm you, else we will have to wait
until Friday morning CET.


Thanks,

Gregory


> 
> Regards,
> 
> Leigh.
>
Leigh Brown March 25, 2015, 11:16 p.m. UTC | #5
Hi Andrew,

On 2015-03-25 22:36, Andrew Lunn wrote:
>> Thanks for your contribution, however the reset button is not always
>> connected to this GPIO. At least on the Mirabox I have the reset 
>> button
>> triggers an hardware reset. My concern is that for some board the dts
>> representation would be wrong.
> 
> When looking at WiFi issues, it became clear there are two different
> wifi designs. I would not be too surprised if this reset button
> changed at the same time.
> 
> Gregory, Leigh, is your wifi on the SDIO bus, or the USB bus?
> 
> The mirabox i have has an sdio wifi device. But i cannot easily test
> the reset button, i blew the 5v power rail, so all USB is dead, and my
> rootfs was on mmc, which is implemented via USB :-(

The WiFi on my Mirabox is on the SDIO bus.

[   29.408420] mwifiex_sdio mmc0:0001:1: WLAN FW already running! Skip 
FW dnld
[   29.408430] mwifiex_sdio mmc0:0001:1: WLAN FW is active
[   29.511297] mwifiex_sdio mmc0:0001:1: driver_version = mwifiex 1.0 
(14.66.35.p52)

Regards,

Leigh.
Gregory CLEMENT March 26, 2015, 5:16 p.m. UTC | #6
On 26/03/2015 00:10, Gregory CLEMENT wrote:
> Hi,
> 
> On 26/03/2015 00:16, Leigh Brown wrote:
>> Hi Andrew,
>>
>> On 2015-03-25 22:36, Andrew Lunn wrote:
>>>> Thanks for your contribution, however the reset button is not always
>>>> connected to this GPIO. At least on the Mirabox I have the reset 
>>>> button
>>>> triggers an hardware reset. My concern is that for some board the dts
>>>> representation would be wrong.
>>>
>>> When looking at WiFi issues, it became clear there are two different
>>> wifi designs. I would not be too surprised if this reset button
>>> changed at the same time.
>>>
>>> Gregory, Leigh, is your wifi on the SDIO bus, or the USB bus?
>>>
>>> The mirabox i have has an sdio wifi device. But i cannot easily test
>>> the reset button, i blew the 5v power rail, so all USB is dead, and my
>>> rootfs was on mmc, which is implemented via USB :-(
>>
>> The WiFi on my Mirabox is on the SDIO bus.
>>
>> [   29.408420] mwifiex_sdio mmc0:0001:1: WLAN FW already running! Skip 
>> FW dnld
>> [   29.408430] mwifiex_sdio mmc0:0001:1: WLAN FW is active
>> [   29.511297] mwifiex_sdio mmc0:0001:1: driver_version = mwifiex 1.0 
>> (14.66.35.p52)
> 
> I think it is the same for me. I brought my Mirabox at ELC, but it is in
> my room and just after ELC I will go to the airport. If I find a plug
> there, I will boot it and I will confirm you, else we will have to wait
> until Friday morning CET.

So I checked and I confirm that the Wifi is on the SDIO bus.

Going back to how representing the reset button. We have several option:

1. Modifying the file armada-370-mirabox.dts as done in this patch.

2. Introduce a armada-370-mirabox.dtsi file and adding two new dts file using this
one for each variation of the Mirabox

3. Using the Transaction Device Tree + Overlays framwork and creating a device tree
fragment.


I already commented about the 1.

For 2., it seems overkill for me and we need to be able to make difference with the
different flavor of the board, which doesn't seem possible now.

The 3. could be a good solution, but there is some pending question as where to store
this DT fragment and as for 2., how to recognize the board.


Thanks,

Gregory


> 
> 
> Thanks,
> 
> Gregory
> 
> 
>>
>> Regards,
>>
>> Leigh.
>>
> 
>
diff mbox

Patch

diff --git a/arch/arm/boot/dts/armada-370-mirabox.dts b/arch/arm/boot/dts/armada-370-mirabox.dts
index e1b0eb6..41f453d 100644
--- a/arch/arm/boot/dts/armada-370-mirabox.dts
+++ b/arch/arm/boot/dts/armada-370-mirabox.dts
@@ -9,6 +9,7 @@ 
  */
 
 /dts-v1/;
+#include <dt-bindings/input/input.h>
 #include <dt-bindings/gpio/gpio.h>
 #include "armada-370.dtsi"
 
@@ -78,6 +79,18 @@ 
 				};
 			};
 
+			gpio-keys {
+				compatible = "gpio-keys";
+				pinctrl-0 = <&reset_button_pin>;
+				pinctrl-names = "default";
+
+				reset-button {
+					label = "Reset Button";
+					linux,code = <KEY_RESTART>;
+					gpios = <&gpio1 26 GPIO_ACTIVE_LOW>;
+				};
+			};
+
 			mdio {
 				pinctrl-0 = <&mdio_pins>;
 				pinctrl-names = "default";
@@ -168,5 +181,10 @@ 
 		marvell,pins = "mpp64", "mpp65";
 		marvell,function = "gpio";
 	};
+
+	reset_button_pin: reset-button-pin {
+		marvell,pins = "mpp58";
+		marvell,function = "gpio";
+	};
 };