| Message ID | 1351090434-30499-4-git-send-email-andrew@lunn.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Thanks for doing all this. Some typos to fix, I've commented below but I thought it might be easier to push a version for you to steal. It's here: git://github.com/lentinj/linux.git v3.7-rc2-pinctrl https://raw.github.com/lentinj/linux/v3.7-rc2-pinctrl/arch/arm/boot/dts/kirkwood-dnskw.dtsi Tested on a DNS-320, not a DNS-325 yet. Similar to lsxl_init, the custom GPIO registrations fail:- dnskw: failed to configure power-off GPIO dnskw: Failed to register dnskw:power:sata0 dnskw: Failed to register dnskw:power:sata1 dnskw: Failed to register dnskw:power:recover So I guess they will need a new home somewhere. However most things (fan, buttons, SATA detect/power via sysfs, power via sysfs) work. The key thing that doesn't is LEDs. Registration looks reasonable: Registered led device: dns320:blue:power kirkwood-pinctrl f1010000.pinctrl: request pin 43 (PIN43) for mvebu-gpio:43 Registered led device: dns320:blue:usb kirkwood-pinctrl f1010000.pinctrl: request pin 28 (PIN28) for mvebu-gpio:28 Registered led device: dns320:orange:l_hdd kirkwood-pinctrl f1010000.pinctrl: request pin 27 (PIN27) for mvebu-gpio:27 Registered led device: dns320:orange:r_hdd kirkwood-pinctrl f1010000.pinctrl: request pin 35 (PIN35) for mvebu-gpio:35 Registered led device: dns320:orange:usb However setting brightness on/off does the following: cat /sys/class/leds/dns320\:blue\:power/trigger dns320:blue:power - No effect, LED continues to blink as bootloader configures it dns320:orange:l_hdd - Works fine dns320:orange:r_hdd - Works fine dns320:orange:usb - Turns on, turning off locks NAS hard dns320:blue:usb - Turns on, turning off locks NAS hard Any ideas? Cheers, On Wed, 24 Oct 2012, Andrew Lunn wrote: > Signed-off-by: Andrew Lunn <andrew@lunn.ch> > --- > arch/arm/boot/dts/kirkwood-dnskw.dtsi | 136 +++++++++++++++++++++++++++++++++ > arch/arm/mach-kirkwood/board-dnskw.c | 37 --------- > 2 files changed, 136 insertions(+), 37 deletions(-) > > diff --git a/arch/arm/boot/dts/kirkwood-dnskw.dtsi b/arch/arm/boot/dts/kirkwood-dnskw.dtsi > index 9b32d02..5d8cf93 100644 > --- a/arch/arm/boot/dts/kirkwood-dnskw.dtsi > +++ b/arch/arm/boot/dts/kirkwood-dnskw.dtsi > @@ -36,6 +36,142 @@ > }; > > ocp@f1000000 { > + pinctrl: pinctrl@10000 { > + compatible = "marvell,88f6281-pinctrl"; > + reg = <0x10000 0x20>; > + > + pinctrl-0 = < &pmx_uart1 &pmx_sata1 Need a &pmx_sata0 too (see below). > + &pmx_gpio_24 &pmx_gpio_25 > + &pmx_led_power &pmx_led_power Shouldn't be repeated, I'm guessing. > + &pmx_led_red_right_hdd > + &pmx_led_red_left_hdd > + &pmx_led_red_usb_325 > + &pmx_gpio_30 &pmx_gpio_31 > + &pmx_gpio_32 &pmx_gpio_33 > + &pmx_button_power > + &pmx_led_red_usb_320 > + &pmx_power_off &pmx_power_back_on > + &pmx_power_sata0 &pmx_power_sata1 > + &pmx_present_sata0 &pmx_present_sata1 > + &pmx_led_white_usb &pmx_fan_tacho > + &pmx_fan_high_speed &pmx_fan_low_speed > + &pmx_button_unmount &pmx_button_reset > + &pmx_temp_alarm >; > + pinctrl-names = "default"; > + > + pmx_uart1: pmx-uart1 { > + marvell,pins = "mpp13", "mpp14"; > + marvell,function = "uart1"; > + }; > + pmx_sata1: pmx-sata1 { > + marvell,pins = "mpp4", "mpp20", "mpp22"; mpp4 is for the NAND. I'm guessing mpp22 should be mpp21, but this should have the "sata0" function. On a related note, the NAND pins aren't registered. I probably should have done this before, although if the bootloader didn't set up the NAND pins then it'd probably not get this far :) > + marvell,function = "sata1"; > + }; > + pmx_gpio_24: pmx-gpio-24 { > + marvell,pins = "mpp24"; > + marvell,function = "gpio"; > + }; > + pmx_gpio_25: pmx-gpio-25 { > + marvell,pins = "mpp25"; > + marvell,function = "gpio"; > + }; These don't actually do anything (there's no more NAS functions left to find now), so could they just be left out? They didn't serve any purpose before other than making it easy to see where the gaps were. > + pmx_led_power: pmx-led-power { > + marvell,pins = "mpp26"; > + marvell,function = "gpio"; > + }; > + pmx_led_red_right_hdd: pmx-led-red-right-hdd { > + marvell,pins = "mpp27"; > + marvell,function = "gpio"; > + }; > + pmx_led_red_left_hdd: pmx-led-red-left-hdd { > + marvell,pins = "mpp28"; > + marvell,function = "gpio"; > + }; > + pmx_led_red_usb_325: pmx-led-red-usb-325 { > + marvell,pins = "mpp29"; > + marvell,function = "gpio"; > + }; > + pmx_gpio_30: pmx-gpio-30 { > + marvell,pins = "mpp30"; > + marvell,function = "gpio"; > + }; > + pmx_gpio_31: pmx-gpio-31 { > + marvell,pins = "mpp31"; > + marvell,function = "gpio"; > + }; > + pmx_gpio_32: pmx-gpio-32 { > + marvell,pins = "mpp32"; > + marvell,function = "gpio"; > + }; > + pmx_gpio_33: pmx-gpio-33 { > + marvell,pins = "mpp33"; > + marvell,function = "gpio"; > + }; Should be "gpo", but could just leave out all 4. > + pmx_button_power: pmx-button-power { > + marvell,pins = "mpp34"; > + marvell,function = "gpio"; > + }; > + pmx_led_red_usb_320: pmx-led-red-usb-320 { > + marvell,pins = "mpp35"; > + marvell,function = "gpio"; > + }; > + pmx_power_off: pmx-power-off { > + marvell,pins = "mpp36"; > + marvell,function = "gpio"; > + }; > + pmx_power_back_on: pmx-power-back-on { > + marvell,pins = "mpp37"; > + marvell,function = "gpio"; > + }; > + pmx_gpio_38: pmx-gpio-38 { > + marvell,pins = "mpp38"; > + marvell,function = "gpio"; > + }; This one wasn't registered above, but not a used pin. > + pmx_power_sata0: pmx-power-sata0 { > + marvell,pins = "mpp39"; > + marvell,function = "gpio"; > + }; > + pmx_power_sata1: pmx-power-sata1 { > + marvell,pins = "mpp40"; > + marvell,function = "gpio"; > + }; > + pmx_present_sata0: pmx-present-sata0 { > + marvell,pins = "mpp41"; > + marvell,function = "gpio"; > + }; > + pmx_present_sata1: pmx-present-sata1 { > + marvell,pins = "mpp42"; > + marvell,function = "gpio"; > + }; > + pmx_led_white_usb: pmx-led-white-usb { > + marvell,pins = "mpp43"; > + marvell,function = "gpio"; > + }; > + pmx_fan_tacho: pmx-fan-tacho { > + marvell,pins = "mpp44"; > + marvell,function = "gpio"; > + }; > + pmx_fan_high_speed: pmx-fan-high-speed { > + marvell,pins = "mpp45"; > + marvell,function = "gpio"; > + }; > + pmx_fan_low_speed: pmx-fan-low-speed { > + marvell,pins = "mpp46"; > + marvell,function = "gpio"; > + }; > + pmx_button_unmount: pmx-button-unmount { > + marvell,pins = "mpp47"; > + marvell,function = "gpio"; > + }; > + pmx_button_reset: pmx-button-reset { > + marvell,pins = "mpp48"; > + marvell,function = "gpio"; > + }; > + pmx_temp_alarm: pmx-temp-alarm { > + marvell,pins = "mpp49"; > + marvell,function = "gpio"; > + }; > + }; > sata@80000 { > status = "okay"; > nr-ports = <2>; > diff --git a/arch/arm/mach-kirkwood/board-dnskw.c b/arch/arm/mach-kirkwood/board-dnskw.c > index 43d16d6..ed93c09 100644 > --- a/arch/arm/mach-kirkwood/board-dnskw.c > +++ b/arch/arm/mach-kirkwood/board-dnskw.c > @@ -17,46 +17,11 @@ > #include <linux/mv643xx_eth.h> > #include <linux/gpio.h> > #include "common.h" > -#include "mpp.h" > > static struct mv643xx_eth_platform_data dnskw_ge00_data = { > .phy_addr = MV643XX_ETH_PHY_ADDR(8), > }; > > -static unsigned int dnskw_mpp_config[] __initdata = { > - MPP13_UART1_TXD, /* Custom ... */ > - MPP14_UART1_RXD, /* ... Controller (DNS-320 only) */ > - MPP20_SATA1_ACTn, /* LED: White Right HDD */ > - MPP21_SATA0_ACTn, /* LED: White Left HDD */ > - MPP24_GPIO, > - MPP25_GPIO, > - MPP26_GPIO, /* LED: Power */ > - MPP27_GPIO, /* LED: Red Right HDD */ > - MPP28_GPIO, /* LED: Red Left HDD */ > - MPP29_GPIO, /* LED: Red USB (DNS-325 only) */ > - MPP30_GPIO, > - MPP31_GPIO, > - MPP32_GPIO, > - MPP33_GPO, > - MPP34_GPIO, /* Button: Front power */ > - MPP35_GPIO, /* LED: Red USB (DNS-320 only) */ > - MPP36_GPIO, /* Power: Turn off board */ > - MPP37_GPIO, /* Power: Turn back on after power failure */ > - MPP38_GPIO, > - MPP39_GPIO, /* Power: SATA0 */ > - MPP40_GPIO, /* Power: SATA1 */ > - MPP41_GPIO, /* SATA0 present */ > - MPP42_GPIO, /* SATA1 present */ > - MPP43_GPIO, /* LED: White USB */ > - MPP44_GPIO, /* Fan: Tachometer Pin */ > - MPP45_GPIO, /* Fan: high speed */ > - MPP46_GPIO, /* Fan: low speed */ > - MPP47_GPIO, /* Button: Back unmount */ > - MPP48_GPIO, /* Button: Back reset */ > - MPP49_GPIO, /* Temp Alarm (DNS-325) Pin of U5 (DNS-320) */ > - 0 > -}; > - > static void dnskw_power_off(void) > { > gpio_set_value(36, 1); > @@ -76,8 +41,6 @@ static void __init dnskw_gpio_register(unsigned gpio, char *name, int def) > > void __init dnskw_init(void) > { > - kirkwood_mpp_conf(dnskw_mpp_config); > - > kirkwood_ehci_init(); > kirkwood_ge00_init(&dnskw_ge00_data); > >
On Thu, Oct 25, 2012 at 11:58:37PM +0100, Jamie Lentin wrote: > Thanks for doing all this. Some typos to fix, I've commented below > but I thought it might be easier to push a version for you to steal. > It's here: > > git://github.com/lentinj/linux.git v3.7-rc2-pinctrl > https://raw.github.com/lentinj/linux/v3.7-rc2-pinctrl/arch/arm/boot/dts/kirkwood-dnskw.dtsi Thanks. I will probably squash the three patches into my original, and add a Signed-off-by: if that is O.K. for you. > > Tested on a DNS-320, not a DNS-325 yet. > > Similar to lsxl_init, the custom GPIO registrations fail:- > > dnskw: failed to configure power-off GPIO > dnskw: Failed to register dnskw:power:sata0 > dnskw: Failed to register dnskw:power:sata1 > dnskw: Failed to register dnskw:power:recover > > So I guess they will need a new home somewhere. I hope to look at this problem this weekend. Maybe a gpio regulator could be a solution, or loading the pinctrl stuff earlier. We will see. > However most things (fan, buttons, SATA detect/power via sysfs, > power via sysfs) work. The key thing that doesn't is LEDs. > Registration looks reasonable: > > Registered led device: dns320:blue:power > kirkwood-pinctrl f1010000.pinctrl: request pin 43 (PIN43) for mvebu-gpio:43 > Registered led device: dns320:blue:usb > kirkwood-pinctrl f1010000.pinctrl: request pin 28 (PIN28) for mvebu-gpio:28 > Registered led device: dns320:orange:l_hdd > kirkwood-pinctrl f1010000.pinctrl: request pin 27 (PIN27) for mvebu-gpio:27 > Registered led device: dns320:orange:r_hdd > kirkwood-pinctrl f1010000.pinctrl: request pin 35 (PIN35) for mvebu-gpio:35 > Registered led device: dns320:orange:usb > > However setting brightness on/off does the following: > cat /sys/class/leds/dns320\:blue\:power/trigger > > dns320:blue:power - No effect, LED continues to blink as bootloader > configures it > dns320:orange:l_hdd - Works fine > dns320:orange:r_hdd - Works fine > dns320:orange:usb - Turns on, turning off locks NAS hard > dns320:blue:usb - Turns on, turning off locks NAS hard > > Any ideas? First thing that comes to mind, is it registering them for the correct GPIO controller. I think with the new setup, pinctrl and gpio are closely linked, so maybe, its modifying pins on the wrong controller. Bit of a long shot.... > > ocp@f1000000 { > >+ pinctrl: pinctrl@10000 { > >+ compatible = "marvell,88f6281-pinctrl"; > >+ reg = <0x10000 0x20>; > >+ > >+ pinctrl-0 = < &pmx_uart1 &pmx_sata1 > > Need a &pmx_sata0 too (see below). I just turned the existing MPP setup into pinctrl. Things like SATA, SPI pins, etc, i left alone if they were not configured in the old C code. I've no problems adding them here. > > >+ &pmx_gpio_24 &pmx_gpio_25 > >+ &pmx_led_power &pmx_led_power > > Shouldn't be repeated, I'm guessing. > > >+ &pmx_led_red_right_hdd > >+ &pmx_led_red_left_hdd > >+ &pmx_led_red_usb_325 > >+ &pmx_gpio_30 &pmx_gpio_31 > >+ &pmx_gpio_32 &pmx_gpio_33 > >+ &pmx_button_power > >+ &pmx_led_red_usb_320 > >+ &pmx_power_off &pmx_power_back_on > >+ &pmx_power_sata0 &pmx_power_sata1 > >+ &pmx_present_sata0 &pmx_present_sata1 > >+ &pmx_led_white_usb &pmx_fan_tacho > >+ &pmx_fan_high_speed &pmx_fan_low_speed > >+ &pmx_button_unmount &pmx_button_reset > >+ &pmx_temp_alarm >; > >+ pinctrl-names = "default"; > >+ > >+ pmx_uart1: pmx-uart1 { > >+ marvell,pins = "mpp13", "mpp14"; > >+ marvell,function = "uart1"; > >+ }; > >+ pmx_sata1: pmx-sata1 { > >+ marvell,pins = "mpp4", "mpp20", "mpp22"; > > mpp4 is for the NAND. I'm guessing mpp22 should be mpp21, but this > should have the "sata0" function. MPP_MODE(4, MPP_VAR_FUNCTION(0x0, "gpio", NULL, V(1, 1, 1, 1, 1)), MPP_VAR_FUNCTION(0x1, "nand", "io6", V(1, 1, 1, 1, 1)), MPP_VAR_FUNCTION(0x2, "uart0", "rxd", V(1, 1, 1, 1, 1)), MPP_VAR_FUNCTION(0x5, "sata1", "act", V(0, 0, 1, 1, 1)), MPP_VAR_FUNCTION(0xb, "lcd", "hsync", V(0, 0, 0, 0, 1)), MPP_VAR_FUNCTION(0xd, "ptp", "clk", V(1, 1, 1, 1, 0))), 4 can be both NAND and SATA. It looks like NAND has to use pins mpp0-mpp5,mpp18-mmp19, they are not available anywhere else. SATA1 is duplicated, so we have to be careful to get the right pins. Maybe boot the old kernel and look these lines: [ 16.187814] initial MPP regs: 01112222 43303311 55550000 00000000 00000000 00000000 00000000 [ 16.187833] final MPP regs: 01552222 03303311 55550000 00000000 00000000 00000000 00000000 The first line is how uboot setup the MPP pins. The second is after the init function was called. Andrew
On Fri, 26 Oct 2012, Andrew Lunn wrote: > On Thu, Oct 25, 2012 at 11:58:37PM +0100, Jamie Lentin wrote: >> Thanks for doing all this. Some typos to fix, I've commented below >> but I thought it might be easier to push a version for you to steal. >> It's here: >> >> git://github.com/lentinj/linux.git v3.7-rc2-pinctrl >> https://raw.github.com/lentinj/linux/v3.7-rc2-pinctrl/arch/arm/boot/dts/kirkwood-dnskw.dtsi > > Thanks. I will probably squash the three patches into my original, and > add a Signed-off-by: if that is O.K. for you. > Sounds good. >> >> Tested on a DNS-320, not a DNS-325 yet. >> >> Similar to lsxl_init, the custom GPIO registrations fail:- >> >> dnskw: failed to configure power-off GPIO >> dnskw: Failed to register dnskw:power:sata0 >> dnskw: Failed to register dnskw:power:sata1 >> dnskw: Failed to register dnskw:power:recover >> >> So I guess they will need a new home somewhere. > > I hope to look at this problem this weekend. Maybe a gpio regulator > could be a solution, or loading the pinctrl stuff earlier. We will > see. > I did look at using the gpio-regulator stuff a while back, and decided it wasn't quite the right shape. Although I can't remember why, and it might have changed since. The power-off GPIO registration could happen in dnskw_power_off instead, or the attempt at a gpio-poweroff driver could be resurrected (I'd forgotten about it until now). I could accept dnskw:power:recover is a weirdo configuration option that should be set in the bootloader / userland rather than the kernel supporting it. Although it would be nice if the kernel registered it's purpose. Maybe GPIO pins could be exported by adding sub-nodes to the GPIO chip, if that's not too hackish? >> However most things (fan, buttons, SATA detect/power via sysfs, >> power via sysfs) work. The key thing that doesn't is LEDs. >> Registration looks reasonable: >> >> Registered led device: dns320:blue:power >> kirkwood-pinctrl f1010000.pinctrl: request pin 43 (PIN43) for mvebu-gpio:43 >> Registered led device: dns320:blue:usb >> kirkwood-pinctrl f1010000.pinctrl: request pin 28 (PIN28) for mvebu-gpio:28 >> Registered led device: dns320:orange:l_hdd >> kirkwood-pinctrl f1010000.pinctrl: request pin 27 (PIN27) for mvebu-gpio:27 >> Registered led device: dns320:orange:r_hdd >> kirkwood-pinctrl f1010000.pinctrl: request pin 35 (PIN35) for mvebu-gpio:35 >> Registered led device: dns320:orange:usb >> >> However setting brightness on/off does the following: >> cat /sys/class/leds/dns320\:blue\:power/trigger >> >> dns320:blue:power - No effect, LED continues to blink as bootloader >> configures it >> dns320:orange:l_hdd - Works fine >> dns320:orange:r_hdd - Works fine >> dns320:orange:usb - Turns on, turning off locks NAS hard >> dns320:blue:usb - Turns on, turning off locks NAS hard >> >> Any ideas? > First thing that comes to mind, is it registering them for the correct > GPIO controller. I think with the new setup, pinctrl and gpio are > closely linked, so maybe, its modifying pins on the wrong controller. > Bit of a long shot.... > I did wonder that, but then why would turning the LEDs on work fine? I wonder if both pins are being toggled or something. I'll investigate further and report back. The two that cause the NAS to lock up are the only ones on &gpio1 though. >>> ocp@f1000000 { >>> + pinctrl: pinctrl@10000 { >>> + compatible = "marvell,88f6281-pinctrl"; >>> + reg = <0x10000 0x20>; >>> + >>> + pinctrl-0 = < &pmx_uart1 &pmx_sata1 >> >> Need a &pmx_sata0 too (see below). > > I just turned the existing MPP setup into pinctrl. Things like SATA, > SPI pins, etc, i left alone if they were not configured in the old C > code. I've no problems adding them here. > >> >>> + &pmx_gpio_24 &pmx_gpio_25 >>> + &pmx_led_power &pmx_led_power >> >> Shouldn't be repeated, I'm guessing. >> >>> + &pmx_led_red_right_hdd >>> + &pmx_led_red_left_hdd >>> + &pmx_led_red_usb_325 >>> + &pmx_gpio_30 &pmx_gpio_31 >>> + &pmx_gpio_32 &pmx_gpio_33 >>> + &pmx_button_power >>> + &pmx_led_red_usb_320 >>> + &pmx_power_off &pmx_power_back_on >>> + &pmx_power_sata0 &pmx_power_sata1 >>> + &pmx_present_sata0 &pmx_present_sata1 >>> + &pmx_led_white_usb &pmx_fan_tacho >>> + &pmx_fan_high_speed &pmx_fan_low_speed >>> + &pmx_button_unmount &pmx_button_reset >>> + &pmx_temp_alarm >; >>> + pinctrl-names = "default"; >>> + >>> + pmx_uart1: pmx-uart1 { >>> + marvell,pins = "mpp13", "mpp14"; >>> + marvell,function = "uart1"; >>> + }; >>> + pmx_sata1: pmx-sata1 { >>> + marvell,pins = "mpp4", "mpp20", "mpp22"; >> >> mpp4 is for the NAND. I'm guessing mpp22 should be mpp21, but this >> should have the "sata0" function. > > MPP_MODE(4, > MPP_VAR_FUNCTION(0x0, "gpio", NULL, V(1, 1, 1, 1, 1)), > MPP_VAR_FUNCTION(0x1, "nand", "io6", V(1, 1, 1, 1, 1)), > MPP_VAR_FUNCTION(0x2, "uart0", "rxd", V(1, 1, 1, 1, 1)), > MPP_VAR_FUNCTION(0x5, "sata1", "act", V(0, 0, 1, 1, 1)), > MPP_VAR_FUNCTION(0xb, "lcd", "hsync", V(0, 0, 0, 0, 1)), > MPP_VAR_FUNCTION(0xd, "ptp", "clk", V(1, 1, 1, 1, 0))), > > 4 can be both NAND and SATA. It looks like NAND has to use pins > mpp0-mpp5,mpp18-mmp19, they are not available anywhere else. SATA1 is > duplicated, so we have to be careful to get the right pins. > The sata0 and sata1 activity leds are definitely MPP20 and MPP21---I've set them up as GPIO leds before successfully. > Maybe boot the old kernel and look these lines: > > [ 16.187814] initial MPP regs: 01112222 43303311 55550000 00000000 00000000 00000000 00000000 > [ 16.187833] final MPP regs: 01552222 03303311 55550000 00000000 00000000 00000000 00000000 > > The first line is how uboot setup the MPP pins. The second is after > the init function was called. initial MPP regs: 01111111 03303311 00551100 00000000 00000000 00000000 00000000 final MPP regs: 01111111 03303311 00551100 00000000 00000000 00000000 00000000 Although the initial MPP regs is also set by a recompiled u-boot with ~identical MPP setup code. > > Andrew >
> I did look at using the gpio-regulator stuff a while back, and > decided it wasn't quite the right shape. Although I can't remember > why, and it might have changed since. O.K. I will look at it this weekend. > The power-off GPIO registration could happen in dnskw_power_off > instead, or the attempt at a gpio-poweroff driver could be > resurrected (I'd forgotten about it until now). I dusted the code off last weekend, but ran out of time. I wanted to make it also support the pxa use-case. > I could accept dnskw:power:recover is a weirdo configuration option > that should be set in the bootloader / userland rather than the > kernel supporting it. Although it would be nice if the kernel > registered it's purpose. Maybe GPIO pins could be exported by adding > sub-nodes to the GPIO chip, if that's not too hackish? We need some sort of solution. It is quite common to use GPIO this way, to enable power, etc. So either we need some form of gpio regulator, or the ability to set gpio defaults as you said in the gpio DT node, or fix the ordering so the init function can set them up. > >First thing that comes to mind, is it registering them for the correct > >GPIO controller. I think with the new setup, pinctrl and gpio are > >closely linked, so maybe, its modifying pins on the wrong controller. > >Bit of a long shot.... > > > > I did wonder that, but then why would turning the LEDs on work fine? > I wonder if both pins are being toggled or something. I'll > investigate further and report back. The two that cause the NAS to > lock up are the only ones on &gpio1 though. What would they map to on gpio0? NAND? > The sata0 and sata1 activity leds are definitely MPP20 and > MPP21---I've set them up as GPIO leds before successfully. O.K. > >[ 16.187814] initial MPP regs: 01112222 43303311 55550000 00000000 00000000 00000000 00000000 > >[ 16.187833] final MPP regs: 01552222 03303311 55550000 00000000 00000000 00000000 00000000 > > > >The first line is how uboot setup the MPP pins. The second is after > >the init function was called. > > initial MPP regs: 01111111 03303311 00551100 00000000 00000000 00000000 00000000 > final MPP regs: 01111111 03303311 00551100 00000000 00000000 00000000 00000000 > > Although the initial MPP regs is also set by a recompiled u-boot > with ~identical MPP setup code. It might be useful to generate the same sort of dumps with the new driver, even if its code just hacked in for testing. Andrew
On Fri, 26 Oct 2012, Andrew Lunn wrote: >> I did look at using the gpio-regulator stuff a while back, and >> decided it wasn't quite the right shape. Although I can't remember >> why, and it might have changed since. > > O.K. I will look at it this weekend. > >> The power-off GPIO registration could happen in dnskw_power_off >> instead, or the attempt at a gpio-poweroff driver could be >> resurrected (I'd forgotten about it until now). > > I dusted the code off last weekend, but ran out of time. I wanted to > make it also support the pxa use-case. Okay, I'm unlikely to get a chance to have a play with it for a week or so but if I do I'll let you know. >> I could accept dnskw:power:recover is a weirdo configuration option >> that should be set in the bootloader / userland rather than the >> kernel supporting it. Although it would be nice if the kernel >> registered it's purpose. Maybe GPIO pins could be exported by adding >> sub-nodes to the GPIO chip, if that's not too hackish? > > We need some sort of solution. It is quite common to use GPIO this > way, to enable power, etc. So either we need some form of gpio > regulator, or the ability to set gpio defaults as you said in the gpio > DT node, or fix the ordering so the init function can set them up. Setting the defaults in the DT gpio node would get my vote, as it pretty much removes board-dnskw.c, and there'd be easy sysfs files to turn the pins on/off (maybe someone out there doesn't want their NAS to turn back on after a power failure...). >>> First thing that comes to mind, is it registering them for the correct >>> GPIO controller. I think with the new setup, pinctrl and gpio are >>> closely linked, so maybe, its modifying pins on the wrong controller. >>> Bit of a long shot.... >>> >> >> I did wonder that, but then why would turning the LEDs on work fine? >> I wonder if both pins are being toggled or something. I'll >> investigate further and report back. The two that cause the NAS to >> lock up are the only ones on &gpio1 though. > > What would they map to on gpio0? NAND? > 3 & 11, so NAND and UART0 RX. But it's not just that the console is dead, I can't ping the NAS any more either. However the power LED is still blinking via. hardware GPIO blink. >> The sata0 and sata1 activity leds are definitely MPP20 and >> MPP21---I've set them up as GPIO leds before successfully. > > O.K. > >>> [ 16.187814] initial MPP regs: 01112222 43303311 55550000 00000000 00000000 00000000 00000000 >>> [ 16.187833] final MPP regs: 01552222 03303311 55550000 00000000 00000000 00000000 00000000 >>> >>> The first line is how uboot setup the MPP pins. The second is after >>> the init function was called. >> >> initial MPP regs: 01111111 03303311 00551100 00000000 00000000 00000000 00000000 >> final MPP regs: 01111111 03303311 00551100 00000000 00000000 00000000 00000000 >> >> Although the initial MPP regs is also set by a recompiled u-boot >> with ~identical MPP setup code. > > It might be useful to generate the same sort of dumps with the new > driver, even if its code just hacked in for testing. I'd been looking at the debugfs output if you hadn't spotted it: root@rocoto:~# cat /debug/pinctrl/f1010000.pinctrl/pinmux-pins Pinmux settings per pin Format: pin (name): mux_owner gpio_owner hog? pin 0 (PIN0): f1010000.pinctrl (GPIO UNCLAIMED) (HOG) function nand group mpp0 pin 1 (PIN1): f1010000.pinctrl (GPIO UNCLAIMED) (HOG) function nand group mpp1 pin 2 (PIN2): f1010000.pinctrl (GPIO UNCLAIMED) (HOG) function nand group mpp2 . . . Although it doesn't say what the initial MPP setup was. > > Andrew >
diff --git a/arch/arm/boot/dts/kirkwood-dnskw.dtsi b/arch/arm/boot/dts/kirkwood-dnskw.dtsi index 9b32d02..5d8cf93 100644 --- a/arch/arm/boot/dts/kirkwood-dnskw.dtsi +++ b/arch/arm/boot/dts/kirkwood-dnskw.dtsi @@ -36,6 +36,142 @@ }; ocp@f1000000 { + pinctrl: pinctrl@10000 { + compatible = "marvell,88f6281-pinctrl"; + reg = <0x10000 0x20>; + + pinctrl-0 = < &pmx_uart1 &pmx_sata1 + &pmx_gpio_24 &pmx_gpio_25 + &pmx_led_power &pmx_led_power + &pmx_led_red_right_hdd + &pmx_led_red_left_hdd + &pmx_led_red_usb_325 + &pmx_gpio_30 &pmx_gpio_31 + &pmx_gpio_32 &pmx_gpio_33 + &pmx_button_power + &pmx_led_red_usb_320 + &pmx_power_off &pmx_power_back_on + &pmx_power_sata0 &pmx_power_sata1 + &pmx_present_sata0 &pmx_present_sata1 + &pmx_led_white_usb &pmx_fan_tacho + &pmx_fan_high_speed &pmx_fan_low_speed + &pmx_button_unmount &pmx_button_reset + &pmx_temp_alarm >; + pinctrl-names = "default"; + + pmx_uart1: pmx-uart1 { + marvell,pins = "mpp13", "mpp14"; + marvell,function = "uart1"; + }; + pmx_sata1: pmx-sata1 { + marvell,pins = "mpp4", "mpp20", "mpp22"; + marvell,function = "sata1"; + }; + pmx_gpio_24: pmx-gpio-24 { + marvell,pins = "mpp24"; + marvell,function = "gpio"; + }; + pmx_gpio_25: pmx-gpio-25 { + marvell,pins = "mpp25"; + marvell,function = "gpio"; + }; + pmx_led_power: pmx-led-power { + marvell,pins = "mpp26"; + marvell,function = "gpio"; + }; + pmx_led_red_right_hdd: pmx-led-red-right-hdd { + marvell,pins = "mpp27"; + marvell,function = "gpio"; + }; + pmx_led_red_left_hdd: pmx-led-red-left-hdd { + marvell,pins = "mpp28"; + marvell,function = "gpio"; + }; + pmx_led_red_usb_325: pmx-led-red-usb-325 { + marvell,pins = "mpp29"; + marvell,function = "gpio"; + }; + pmx_gpio_30: pmx-gpio-30 { + marvell,pins = "mpp30"; + marvell,function = "gpio"; + }; + pmx_gpio_31: pmx-gpio-31 { + marvell,pins = "mpp31"; + marvell,function = "gpio"; + }; + pmx_gpio_32: pmx-gpio-32 { + marvell,pins = "mpp32"; + marvell,function = "gpio"; + }; + pmx_gpio_33: pmx-gpio-33 { + marvell,pins = "mpp33"; + marvell,function = "gpio"; + }; + pmx_button_power: pmx-button-power { + marvell,pins = "mpp34"; + marvell,function = "gpio"; + }; + pmx_led_red_usb_320: pmx-led-red-usb-320 { + marvell,pins = "mpp35"; + marvell,function = "gpio"; + }; + pmx_power_off: pmx-power-off { + marvell,pins = "mpp36"; + marvell,function = "gpio"; + }; + pmx_power_back_on: pmx-power-back-on { + marvell,pins = "mpp37"; + marvell,function = "gpio"; + }; + pmx_gpio_38: pmx-gpio-38 { + marvell,pins = "mpp38"; + marvell,function = "gpio"; + }; + pmx_power_sata0: pmx-power-sata0 { + marvell,pins = "mpp39"; + marvell,function = "gpio"; + }; + pmx_power_sata1: pmx-power-sata1 { + marvell,pins = "mpp40"; + marvell,function = "gpio"; + }; + pmx_present_sata0: pmx-present-sata0 { + marvell,pins = "mpp41"; + marvell,function = "gpio"; + }; + pmx_present_sata1: pmx-present-sata1 { + marvell,pins = "mpp42"; + marvell,function = "gpio"; + }; + pmx_led_white_usb: pmx-led-white-usb { + marvell,pins = "mpp43"; + marvell,function = "gpio"; + }; + pmx_fan_tacho: pmx-fan-tacho { + marvell,pins = "mpp44"; + marvell,function = "gpio"; + }; + pmx_fan_high_speed: pmx-fan-high-speed { + marvell,pins = "mpp45"; + marvell,function = "gpio"; + }; + pmx_fan_low_speed: pmx-fan-low-speed { + marvell,pins = "mpp46"; + marvell,function = "gpio"; + }; + pmx_button_unmount: pmx-button-unmount { + marvell,pins = "mpp47"; + marvell,function = "gpio"; + }; + pmx_button_reset: pmx-button-reset { + marvell,pins = "mpp48"; + marvell,function = "gpio"; + }; + pmx_temp_alarm: pmx-temp-alarm { + marvell,pins = "mpp49"; + marvell,function = "gpio"; + }; + }; sata@80000 { status = "okay"; nr-ports = <2>; diff --git a/arch/arm/mach-kirkwood/board-dnskw.c b/arch/arm/mach-kirkwood/board-dnskw.c index 43d16d6..ed93c09 100644 --- a/arch/arm/mach-kirkwood/board-dnskw.c +++ b/arch/arm/mach-kirkwood/board-dnskw.c @@ -17,46 +17,11 @@ #include <linux/mv643xx_eth.h> #include <linux/gpio.h> #include "common.h" -#include "mpp.h" static struct mv643xx_eth_platform_data dnskw_ge00_data = { .phy_addr = MV643XX_ETH_PHY_ADDR(8), }; -static unsigned int dnskw_mpp_config[] __initdata = { - MPP13_UART1_TXD, /* Custom ... */ - MPP14_UART1_RXD, /* ... Controller (DNS-320 only) */ - MPP20_SATA1_ACTn, /* LED: White Right HDD */ - MPP21_SATA0_ACTn, /* LED: White Left HDD */ - MPP24_GPIO, - MPP25_GPIO, - MPP26_GPIO, /* LED: Power */ - MPP27_GPIO, /* LED: Red Right HDD */ - MPP28_GPIO, /* LED: Red Left HDD */ - MPP29_GPIO, /* LED: Red USB (DNS-325 only) */ - MPP30_GPIO, - MPP31_GPIO, - MPP32_GPIO, - MPP33_GPO, - MPP34_GPIO, /* Button: Front power */ - MPP35_GPIO, /* LED: Red USB (DNS-320 only) */ - MPP36_GPIO, /* Power: Turn off board */ - MPP37_GPIO, /* Power: Turn back on after power failure */ - MPP38_GPIO, - MPP39_GPIO, /* Power: SATA0 */ - MPP40_GPIO, /* Power: SATA1 */ - MPP41_GPIO, /* SATA0 present */ - MPP42_GPIO, /* SATA1 present */ - MPP43_GPIO, /* LED: White USB */ - MPP44_GPIO, /* Fan: Tachometer Pin */ - MPP45_GPIO, /* Fan: high speed */ - MPP46_GPIO, /* Fan: low speed */ - MPP47_GPIO, /* Button: Back unmount */ - MPP48_GPIO, /* Button: Back reset */ - MPP49_GPIO, /* Temp Alarm (DNS-325) Pin of U5 (DNS-320) */ - 0 -}; - static void dnskw_power_off(void) { gpio_set_value(36, 1); @@ -76,8 +41,6 @@ static void __init dnskw_gpio_register(unsigned gpio, char *name, int def) void __init dnskw_init(void) { - kirkwood_mpp_conf(dnskw_mpp_config); - kirkwood_ehci_init(); kirkwood_ge00_init(&dnskw_ge00_data);
Signed-off-by: Andrew Lunn <andrew@lunn.ch> --- arch/arm/boot/dts/kirkwood-dnskw.dtsi | 136 +++++++++++++++++++++++++++++++++ arch/arm/mach-kirkwood/board-dnskw.c | 37 --------- 2 files changed, 136 insertions(+), 37 deletions(-)