| Message ID | 1351090434-30499-10-git-send-email-andrew@lunn.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Andrew, On Wed, 24 Oct 2012 16:53:54 +0200, Andrew Lunn wrote: > Signed-off-by: Andrew Lunn <andrew@lunn.ch> > --- > arch/arm/boot/dts/kirkwood-iomega_ix2_200.dts | 90 +++++++++++++++++++++++++ > arch/arm/mach-kirkwood/board-iomega_ix2_200.c | 24 ------- > 2 files changed, 90 insertions(+), 24 deletions(-) > > diff --git a/arch/arm/boot/dts/kirkwood-iomega_ix2_200.dts b/arch/arm/boot/dts/kirkwood-iomega_ix2_200.dts > index 865aeec..d8fa8e8 100644 > --- a/arch/arm/boot/dts/kirkwood-iomega_ix2_200.dts > +++ b/arch/arm/boot/dts/kirkwood-iomega_ix2_200.dts > @@ -16,6 +16,96 @@ > }; > > ocp@f1000000 { > + pinctrl: pinctrl@10000 { > + compatible = "marvell,88f6281-pinctrl"; > + reg = <0x10000 0x20>; This definition (compatible + reg) should go in kirkwood.dtsi. The pinctrl unit should be declared at the SoC level. Ditto for all other patches. > + pinctrl-0 = < &pmx_button_reset &pmx_button_power > + &pmx_led_backup &pmx_led_power > + &pmx_button_otb &pmx_led_rebuild > + &pmx_led_health > + &pmx_led_sata_brt_ctrl_1 > + &pmx_led_sata_brt_ctrl_2 > + &pmx_led_backup_brt_ctrl_1 > + &pmx_led_backup_brt_ctrl_2 > + &pmx_led_power_brt_ctrl_1 > + &pmx_led_power_brt_ctrl_2 > + &pmx_led_health_brt_ctrl_1 > + &pmx_led_health_brt_ctrl_2 > + &pmx_led_rebuild_brt_ctrl_1 > + &pmx_led_rebuild_brt_ctrl_2 >; > + pinctrl-names = "default"; > + > + pmx_button_reset: pmx-button-reset { > + marvell,pins = "mpp12"; > + marvell,function = "gpio"; > + }; > + pmx_button_power: pmx-button-power { > + marvell,pins = "mpp14"; > + marvell,function = "gpio"; > + }; > + pmx_led_backup: pmx-led-backup { > + marvell,pins = "mpp15"; > + marvell,function = "gpio"; > + }; > + pmx_led_power: pmx-led-power { > + marvell,pins = "mpp16"; > + marvell,function = "gpio"; > + }; > + pmx_button_otb: pmx-button-otb { > + marvell,pins = "mpp35"; > + marvell,function = "gpio"; > + }; > + pmx_led_rebuild: pmx-led-rebuild { > + marvell,pins = "mpp36"; > + marvell,function = "gpio"; > + }; > + pmx_led_health: pmx-led_health { > + marvell,pins = "mpp37"; > + marvell,function = "gpio"; > + }; > + pmx_led_sata_brt_ctrl_1: pmx-led-sata-brt-ctrl-1 { > + marvell,pins = "mpp38"; > + marvell,function = "gpio"; > + }; > + pmx_led_sata_brt_ctrl_2: pmx-led-sata-brt-ctrl-2 { > + marvell,pins = "mpp39"; > + marvell,function = "gpio"; > + }; > + pmx_led_backup_brt_ctrl_1: pmx-led-backup-brt-ctrl-1 { > + marvell,pins = "mpp40"; > + marvell,function = "gpio"; > + }; > + pmx_led_backup_brt_ctrl_2: pmx-led-backup-brt-ctrl-2 { > + marvell,pins = "mpp41"; > + marvell,function = "gpio"; > + }; > + pmx_led_power_brt_ctrl_1: pmx-led-power-brt-ctrl-1 { > + marvell,pins = "mpp42"; > + marvell,function = "gpio"; > + }; > + pmx_led_power_brt_ctrl_2: pmx-led-power-brt-ctrl-2 { > + marvell,pins = "mpp43"; > + marvell,function = "gpio"; > + }; > + pmx_led_health_brt_ctrl_1: pmx-led-health-brt-ctrl-1 { > + marvell,pins = "mpp44"; > + marvell,function = "gpio"; > + }; > + pmx_led_health_brt_ctrl_2: pmx-led-health-brt-ctrl-2 { > + marvell,pins = "mpp45"; > + marvell,function = "gpio"; > + }; > + pmx_led_rebuild_brt_ctrl_1: pmx-led-rebuild-brt-ctrl-1 { > + marvell,pins = "mpp44"; > + marvell,function = "gpio"; > + }; > + pmx_led_rebuild_brt_ctrl_2: pmx-led-rebuild-brt-ctrl-2 { > + marvell,pins = "mpp45"; > + marvell,function = "gpio"; > + }; This is not a strong comment, but I am not sure it is really useful to define one pinmux configuration for each pin. You can do something like: pmx_buttons: pmx-buttons { marvell,pins = "mpp45", "mpp46", "mpp47", "mpp48"; marvell,function = "gpio"; }; I think all the pins used for buttons or for LEDs kind of make sense to be muxed together. And then, when you declare the LEDs in the DT, you can do: leds { compatible = "gpio-leds"; pinctrl-names = "default"; pinctrl-0 = <&led_pins>; red_led { label = "red_led"; gpios = <&gpio1 17 1>; default-state = "off"; }; yellow_led { label = "yellow_led"; gpios = <&gpio1 19 1>; default-state = "off"; }; green_led { label = "green_led"; gpios = <&gpio1 21 1>; default-state = "off"; linux,default-trigger = "heartbeat"; }; }; Where "led_pins" is a pinmux configuration that includes three pins. Best regards, Thomas
On Wed, Oct 24, 2012 at 10:04:29PM +0200, Thomas Petazzoni wrote: > Andrew, > > On Wed, 24 Oct 2012 16:53:54 +0200, Andrew Lunn wrote: > > Signed-off-by: Andrew Lunn <andrew@lunn.ch> > > --- > > arch/arm/boot/dts/kirkwood-iomega_ix2_200.dts | 90 +++++++++++++++++++++++++ > > arch/arm/mach-kirkwood/board-iomega_ix2_200.c | 24 ------- > > 2 files changed, 90 insertions(+), 24 deletions(-) > > > > diff --git a/arch/arm/boot/dts/kirkwood-iomega_ix2_200.dts b/arch/arm/boot/dts/kirkwood-iomega_ix2_200.dts > > index 865aeec..d8fa8e8 100644 > > --- a/arch/arm/boot/dts/kirkwood-iomega_ix2_200.dts > > +++ b/arch/arm/boot/dts/kirkwood-iomega_ix2_200.dts > > @@ -16,6 +16,96 @@ > > }; > > > > ocp@f1000000 { > > + pinctrl: pinctrl@10000 { > > + compatible = "marvell,88f6281-pinctrl"; > > + reg = <0x10000 0x20>; > > This definition (compatible + reg) should go in kirkwood.dtsi. The > pinctrl unit should be declared at the SoC level. Ditto for all other > patches. The reg maybe. Compatibility not. We need to tell pinctrl which of the 5 different variants of kirkwood this particular kirkwood is. See drivers/pinctrl/pinctrl-kirkwood.c enum kirkwood_variant { VARIANT_MV88F6180 = V(1, 0, 0, 0, 0), VARIANT_MV88F6190 = V(0, 1, 0, 0, 0), VARIANT_MV88F6192 = V(0, 0, 1, 0, 0), VARIANT_MV88F6281 = V(0, 0, 0, 1, 0), VARIANT_MV88F6282 = V(0, 0, 0, 0, 1), }; static struct of_device_id kirkwood_pinctrl_of_match[] __devinitdata = { { .compatible = "marvell,88f6180-pinctrl", .data = &mv88f6180_info }, { .compatible = "marvell,88f6190-pinctrl", .data = &mv88f6190_info }, { .compatible = "marvell,88f6192-pinctrl", .data = &mv88f6192_info }, { .compatible = "marvell,88f6281-pinctrl", .data = &mv88f6281_info }, { .compatible = "marvell,88f6282-pinctrl", .data = &mv88f6282_info }, { } }; What SoC is mounted on a board is a property of the board.... Andrew
Andrew, On Wed, 24 Oct 2012 22:20:10 +0200, Andrew Lunn wrote: > The reg maybe. Compatibility not. We need to tell pinctrl which of the > 5 different variants of kirkwood this particular kirkwood is. See Agreed. > drivers/pinctrl/pinctrl-kirkwood.c > > enum kirkwood_variant { > VARIANT_MV88F6180 = V(1, 0, 0, 0, 0), > VARIANT_MV88F6190 = V(0, 1, 0, 0, 0), > VARIANT_MV88F6192 = V(0, 0, 1, 0, 0), > VARIANT_MV88F6281 = V(0, 0, 0, 1, 0), > VARIANT_MV88F6282 = V(0, 0, 0, 0, 1), > }; > > static struct of_device_id kirkwood_pinctrl_of_match[] __devinitdata = { > { .compatible = "marvell,88f6180-pinctrl", .data = &mv88f6180_info }, > { .compatible = "marvell,88f6190-pinctrl", .data = &mv88f6190_info }, > { .compatible = "marvell,88f6192-pinctrl", .data = &mv88f6192_info }, > { .compatible = "marvell,88f6281-pinctrl", .data = &mv88f6281_info }, > { .compatible = "marvell,88f6282-pinctrl", .data = &mv88f6282_info }, > { } > }; > > What SoC is mounted on a board is a property of the board.... The way we solved that on Armada XP is that we have a common armada-xp.dtsi file with definitions common to all SoCs in the family. Then, we have sub .dtsi files named armada-xp-mv78230.dtsi, armada-xp-mv78260.dtsi and armada-xp-mv78460.dtsi that handle the differences between specific SoCs in the family. For example, the differences are: different compatible string for the pinctrl driver, different number of CPUs, different number of pins and therefore different number of GPIOs, different number of PCIe ports, etc. But besides those differences, most units have a lot of common definitions, that we factor out in armada-xp.dtsi. So we have the following hierarchy: + armada-xp.dtsi + armada-xp-mv78230.dtsi + someboard-here.dts + armada-xp-mv78260.dtsi + openblocks-ax3-4.dts + armada-xp-mv78460.dtsi + armada-xp-db.dts Best regards, Thomas
diff --git a/arch/arm/boot/dts/kirkwood-iomega_ix2_200.dts b/arch/arm/boot/dts/kirkwood-iomega_ix2_200.dts index 865aeec..d8fa8e8 100644 --- a/arch/arm/boot/dts/kirkwood-iomega_ix2_200.dts +++ b/arch/arm/boot/dts/kirkwood-iomega_ix2_200.dts @@ -16,6 +16,96 @@ }; ocp@f1000000 { + pinctrl: pinctrl@10000 { + compatible = "marvell,88f6281-pinctrl"; + reg = <0x10000 0x20>; + + pinctrl-0 = < &pmx_button_reset &pmx_button_power + &pmx_led_backup &pmx_led_power + &pmx_button_otb &pmx_led_rebuild + &pmx_led_health + &pmx_led_sata_brt_ctrl_1 + &pmx_led_sata_brt_ctrl_2 + &pmx_led_backup_brt_ctrl_1 + &pmx_led_backup_brt_ctrl_2 + &pmx_led_power_brt_ctrl_1 + &pmx_led_power_brt_ctrl_2 + &pmx_led_health_brt_ctrl_1 + &pmx_led_health_brt_ctrl_2 + &pmx_led_rebuild_brt_ctrl_1 + &pmx_led_rebuild_brt_ctrl_2 >; + pinctrl-names = "default"; + + pmx_button_reset: pmx-button-reset { + marvell,pins = "mpp12"; + marvell,function = "gpio"; + }; + pmx_button_power: pmx-button-power { + marvell,pins = "mpp14"; + marvell,function = "gpio"; + }; + pmx_led_backup: pmx-led-backup { + marvell,pins = "mpp15"; + marvell,function = "gpio"; + }; + pmx_led_power: pmx-led-power { + marvell,pins = "mpp16"; + marvell,function = "gpio"; + }; + pmx_button_otb: pmx-button-otb { + marvell,pins = "mpp35"; + marvell,function = "gpio"; + }; + pmx_led_rebuild: pmx-led-rebuild { + marvell,pins = "mpp36"; + marvell,function = "gpio"; + }; + pmx_led_health: pmx-led_health { + marvell,pins = "mpp37"; + marvell,function = "gpio"; + }; + pmx_led_sata_brt_ctrl_1: pmx-led-sata-brt-ctrl-1 { + marvell,pins = "mpp38"; + marvell,function = "gpio"; + }; + pmx_led_sata_brt_ctrl_2: pmx-led-sata-brt-ctrl-2 { + marvell,pins = "mpp39"; + marvell,function = "gpio"; + }; + pmx_led_backup_brt_ctrl_1: pmx-led-backup-brt-ctrl-1 { + marvell,pins = "mpp40"; + marvell,function = "gpio"; + }; + pmx_led_backup_brt_ctrl_2: pmx-led-backup-brt-ctrl-2 { + marvell,pins = "mpp41"; + marvell,function = "gpio"; + }; + pmx_led_power_brt_ctrl_1: pmx-led-power-brt-ctrl-1 { + marvell,pins = "mpp42"; + marvell,function = "gpio"; + }; + pmx_led_power_brt_ctrl_2: pmx-led-power-brt-ctrl-2 { + marvell,pins = "mpp43"; + marvell,function = "gpio"; + }; + pmx_led_health_brt_ctrl_1: pmx-led-health-brt-ctrl-1 { + marvell,pins = "mpp44"; + marvell,function = "gpio"; + }; + pmx_led_health_brt_ctrl_2: pmx-led-health-brt-ctrl-2 { + marvell,pins = "mpp45"; + marvell,function = "gpio"; + }; + pmx_led_rebuild_brt_ctrl_1: pmx-led-rebuild-brt-ctrl-1 { + marvell,pins = "mpp44"; + marvell,function = "gpio"; + }; + pmx_led_rebuild_brt_ctrl_2: pmx-led-rebuild-brt-ctrl-2 { + marvell,pins = "mpp45"; + marvell,function = "gpio"; + }; + + }; i2c@11000 { status = "okay"; diff --git a/arch/arm/mach-kirkwood/board-iomega_ix2_200.c b/arch/arm/mach-kirkwood/board-iomega_ix2_200.c index 158fb97..c144712 100644 --- a/arch/arm/mach-kirkwood/board-iomega_ix2_200.c +++ b/arch/arm/mach-kirkwood/board-iomega_ix2_200.c @@ -15,7 +15,6 @@ #include <linux/ethtool.h> #include <mach/kirkwood.h> #include "common.h" -#include "mpp.h" static struct mv643xx_eth_platform_data iomega_ix2_200_ge00_data = { .phy_addr = MV643XX_ETH_PHY_NONE, @@ -23,34 +22,11 @@ static struct mv643xx_eth_platform_data iomega_ix2_200_ge00_data = { .duplex = DUPLEX_FULL, }; -static unsigned int iomega_ix2_200_mpp_config[] __initdata = { - MPP12_GPIO, /* Reset Button */ - MPP14_GPIO, /* Power Button */ - MPP15_GPIO, /* Backup LED (blue) */ - MPP16_GPIO, /* Power LED (white) */ - MPP35_GPIO, /* OTB Button */ - MPP36_GPIO, /* Rebuild LED (white) */ - MPP37_GPIO, /* Health LED (red) */ - MPP38_GPIO, /* SATA LED brightness control 1 */ - MPP39_GPIO, /* SATA LED brightness control 2 */ - MPP40_GPIO, /* Backup LED brightness control 1 */ - MPP41_GPIO, /* Backup LED brightness control 2 */ - MPP42_GPIO, /* Power LED brightness control 1 */ - MPP43_GPIO, /* Power LED brightness control 2 */ - MPP44_GPIO, /* Health LED brightness control 1 */ - MPP45_GPIO, /* Health LED brightness control 2 */ - MPP46_GPIO, /* Rebuild LED brightness control 1 */ - MPP47_GPIO, /* Rebuild LED brightness control 2 */ - 0 -}; - void __init iomega_ix2_200_init(void) { /* * Basic setup. Needs to be called early. */ - kirkwood_mpp_conf(iomega_ix2_200_mpp_config); - kirkwood_ehci_init(); kirkwood_ge01_init(&iomega_ix2_200_ge00_data);
Signed-off-by: Andrew Lunn <andrew@lunn.ch> --- arch/arm/boot/dts/kirkwood-iomega_ix2_200.dts | 90 +++++++++++++++++++++++++ arch/arm/mach-kirkwood/board-iomega_ix2_200.c | 24 ------- 2 files changed, 90 insertions(+), 24 deletions(-)