ARM: kirkwood: DT board setup for Network Space v2 and parents

This patch adds DT board setup for LaCie Network Space v2 and parents, based the Marvell Kirkwood 6281 SoC. This includes Network Space (Max) v2 and Internet Space v2.

arch/arm/mach-kirkwood/board-dt.c +13 -0

+#define NETSPACE_V2_GPIO_BLUE_LED_SLOW	29
+#define NETSPACE_V2_GPIO_BLUE_LED_CMD	30
+
+static struct ns2_led ns2_led_pins[] = {
+	{
+		.name	= "ns_v2:blue:sata",
+		.cmd	= NETSPACE_V2_GPIO_BLUE_LED_CMD,
+		.slow	= NETSPACE_V2_GPIO_BLUE_LED_SLOW,
+	},
+};
+
+static struct ns2_led_platform_data ns2_leds_data = {
+	.num_leds	= ARRAY_SIZE(ns2_led_pins),

Quoted 3 lines · Simon Guinot

This patch adds DT board setup for LaCie Network Space v2 and parents, based the Marvell Kirkwood 6281 SoC. This includes Network Space (Max) v2 and Internet Space v2.

Hi Simon

At a first look, this looks good. Also nice to see you use the new gpio-fan binding.

Quoted 7 lines

arch/arm/mach-kirkwood/board-dt.c +0 -0

@@ -96,6 +96,11 @@ static void __init kirkwood_dt_init(void)
 	if (of_machine_is_compatible("keymile,km_kirkwood"))
 		km_kirkwood_init();
 

arch/arm/mach-kirkwood/board-dt.c static void __init kirkwood_dt_init(void) · line 105

Quoted 7 lines

arch/arm/mach-kirkwood/board-dt.c +5 -0

+	if (of_machine_is_compatible("lacie,inetspace_v2") ||
+	    of_machine_is_compatible("lacie,netspace_v2") ||
+	    of_machine_is_compatible("lacie,netspace_max_v2"))
+		ns2_init();
+
 	of_platform_populate(NULL, kirkwood_dt_match_table,
 			     kirkwood_auxdata_lookup, NULL);

I'm not a DT policy expert. Could this be one compatibility string for all the boards? Maybe ask on the DT mainline list?

Quoted 18 lines

arch/arm/mach-kirkwood/board-dt.c +17 -0

+#define NETSPACE_V2_GPIO_BLUE_LED_SLOW	29
+#define NETSPACE_V2_GPIO_BLUE_LED_CMD	30
+
+static struct ns2_led ns2_led_pins[] = {
+	{
+		.name	= "ns_v2:blue:sata",
+		.cmd	= NETSPACE_V2_GPIO_BLUE_LED_CMD,
+		.slow	= NETSPACE_V2_GPIO_BLUE_LED_SLOW,
+	},
+};
+
+static struct ns2_led_platform_data ns2_leds_data = {
+	.num_leds	= ARRAY_SIZE(ns2_led_pins),
+	.leds		= ns2_led_pins,
+};
+
+static struct platform_device ns2_leds = {

arch/arm/mach-kirkwood/board-dt.c

Quoted 6 lines

arch/arm/mach-kirkwood/board-dt.c +6 -0

+	.name		= "leds-ns2",
+	.id		= -1,
+	.dev		= {
+		.platform_data	= &ns2_leds_data,
+	},
+};

Have you thought about adding a DT binding for this driver?

It would be nice if you could respin the patches for -rc1, when it comes out. There are changes to Makefile.boot at least required.

Quoted 5 lines · Andrew Lunn

Quoted 3 lines · Simon Guinot

This patch adds DT board setup for LaCie Network Space v2 and parents, based the Marvell Kirkwood 6281 SoC. This includes Network Space (Max) v2 and Internet Space v2.

Quoted 4 lines

Hi Simon

At a first look, this looks good. Also nice to see you use the new gpio-fan binding.

Hi Andrew,

It is nice to have this binding. It works fine.

Quoted 1 line

Have you thought about adding a DT binding for this driver?

Yes. I am on it.

Quoted 2 lines

It would be nice if you could respin the patches for -rc1, when it comes out. There are changes to Makefile.boot at least required.

OK. I will.

Note that this patch is based against the branch kirkwood/boards from the git repository git://git.infradead.org/users/jcooper/linux.git. Is that correct ?

Quoted 6 lines

Quoted 6 lines

arch/arm/mach-kirkwood/board-dt.c +0 -0

@@ -96,6 +96,11 @@ static void __init kirkwood_dt_init(void)
 	if (of_machine_is_compatible("keymile,km_kirkwood"))
 		km_kirkwood_init();
 

Quoted 10 lines

arch/arm/mach-kirkwood/board-dt.c static void __init kirkwood_dt_init(void) · line 105

Quoted 7 lines

arch/arm/mach-kirkwood/board-dt.c +5 -0

+	if (of_machine_is_compatible("lacie,inetspace_v2") ||
+	    of_machine_is_compatible("lacie,netspace_v2") ||
+	    of_machine_is_compatible("lacie,netspace_max_v2"))
+		ns2_init();
+
 	of_platform_populate(NULL, kirkwood_dt_match_table,
 			     kirkwood_auxdata_lookup, NULL);

I'm not a DT policy expert. Could this be one compatibility string for all the boards? Maybe ask on the DT mainline list?

Maybe I could use "lacie,ns2_common" as a compatibility string. But this does not match any existing device. I don't know if it is correct.

Quoted 17 lines

Quoted 6 lines

Quoted 6 lines

arch/arm/mach-kirkwood/board-dt.c +0 -0

@@ -96,6 +96,11 @@ static void __init kirkwood_dt_init(void)
 	if (of_machine_is_compatible("keymile,km_kirkwood"))
 		km_kirkwood_init();
 

Quoted 10 lines

arch/arm/mach-kirkwood/board-dt.c static void __init kirkwood_dt_init(void) · line 105

Quoted 7 lines

arch/arm/mach-kirkwood/board-dt.c +5 -0

+	if (of_machine_is_compatible("lacie,inetspace_v2") ||
+	    of_machine_is_compatible("lacie,netspace_v2") ||
+	    of_machine_is_compatible("lacie,netspace_max_v2"))
+		ns2_init();
+
 	of_platform_populate(NULL, kirkwood_dt_match_table,
 			     kirkwood_auxdata_lookup, NULL);

I'm not a DT policy expert. Could this be one compatibility string for all the boards? Maybe ask on the DT mainline list?

Quoted 2 lines

Maybe I could use "lacie,ns2_common" as a compatibility string. But this does not match any existing device. I don't know if it is correct.

Hi Simon

I did a bit of looking around. For kirkwood, we already have two boards sharing the same compatibility string. kirkwood-dns320.dts and kirkwood-dns325.dts both have dlink,dns-kirkwood and this is what the board-dt.c matches on.

For the tegra20 soc, all boards match on nvidia,tegra20, and that is the only compatibility string in board-dt-tegra20.c.

So i don't see any problem having just one compatibility string here.

The question is, what is the appropriate name. How common is this common C code? Are there ns2 where this C code is not appropriate. One thing to remember is that most of this C code will soon disappear and become DT. All the mpp will be replaced with pinctrl in 3.8. I hope we can get the Ethernet setup in DT as well. You are working on ns2_led, so all the C code will be replaced by DT. So all we are really left with is power off GPIO handling.

So i think the danger of using lacie,ns2_common, and then finding it does not work with some other ns2 device is quite low.

What do you think?

Quoted 22 lines · Andrew Lunn

Quoted 16 lines

Quoted 6 lines

Quoted 6 lines

--- a/arch/arm/mach-kirkwood/board-dt.c +++ b/arch/arm/mach-kirkwood/board-dt.c @@ -96,6 +96,11 @@ static void __init kirkwood_dt_init(void) if (of_machine_is_compatible("keymile,km_kirkwood")) km_kirkwood_init();

Quoted 9 lines

Quoted 5 lines

(quoted diff) +1 -0

+	if (of_machine_is_compatible("lacie,inetspace_v2") || +

of_machine_is_compatible("lacie,netspace_v2") || + of_machine_is_compatible("lacie,netspace_max_v2")) + ns2_init(); + of_platform_populate(NULL, kirkwood_dt_match_table, kirkwood_auxdata_lookup, NULL);

I'm not a DT policy expert. Could this be one compatibility string for all the boards? Maybe ask on the DT mainline list?

Quoted 3 lines

Maybe I could use "lacie,ns2_common" as a compatibility string. But this does not match any existing device. I don't know if it is correct.

Hi Simon

Quoted 7 lines

I did a bit of looking around. For kirkwood, we already have two boards sharing the same compatibility string. kirkwood-dns320.dts and kirkwood-dns325.dts both have dlink,dns-kirkwood and this is what the board-dt.c matches on.

For the tegra20 soc, all boards match on nvidia,tegra20, and that is the only compatibility string in board-dt-tegra20.c.

Quoted 12 lines

So i don't see any problem having just one compatibility string here.

The question is, what is the appropriate name. How common is this common C code? Are there ns2 where this C code is not appropriate. One thing to remember is that most of this C code will soon disappear and become DT. All the mpp will be replaced with pinctrl in 3.8. I hope we can get the Ethernet setup in DT as well. You are working on ns2_led, so all the C code will be replaced by DT. So all we are really left with is power off GPIO handling.

Quoted 2 lines

So i think the danger of using lacie,ns2_common, and then finding it does not work with some other ns2 device is quite low.

Quoted 1 line

What do you think?

The status for this ns2 boards is more or less end-of-life. I mean, this boards are no longer in production. So I think it is safe to consider the code inside board-ns2.c as common and shareable between ns2, is2 and ns2max.

Once this file will be removed, the compatibility checks will be removed as well. But one could easily forget to remove the useless compatibility string "lacie,ns2_common" from the dts...

Quoted 9 lines

Quoted 9 lines

The question is, what is the appropriate name. How common is this common C code? Are there ns2 where this C code is not appropriate. One thing to remember is that most of this C code will soon disappear and become DT. All the mpp will be replaced with pinctrl in 3.8. I hope we can get the Ethernet setup in DT as well. You are working on ns2_led, so all the C code will be replaced by DT. So all we are really left with is power off GPIO handling.

Quoted 2 lines

Quoted 2 lines

So i think the danger of using lacie,ns2_common, and then finding it does not work with some other ns2 device is quite low.

lacie,ns2_common doesn't sound like a HW description, but rather a SW invention. DT should be describing purely the HW. If there's no common HW between these compatible boards (which seems unlikely), then there shouldn't be a shared compatible value.

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

The question is, what is the appropriate name. How common is this common C code? Are there ns2 where this C code is not appropriate. One thing to remember is that most of this C code will soon disappear and become DT. All the mpp will be replaced with pinctrl in 3.8. I hope we can get the Ethernet setup in DT as well. You are working on ns2_led, so all the C code will be replaced by DT. So all we are really left with is power off GPIO handling.

Quoted 7 lines

Quoted 2 lines

Quoted 2 lines

So i think the danger of using lacie,ns2_common, and then finding it does not work with some other ns2 device is quite low.

lacie,ns2_common doesn't sound like a HW description, but rather a SW invention. DT should be describing purely the HW. If there's no common HW between these compatible boards (which seems unlikely), then there shouldn't be a shared compatible value.

Actually, there is common hardware between these boards:

NS2 LED driver
Pinctrl setup
GPIO used for power off
Ethernet configuration.

At the moment, we don't have DT for these, so there is C code. The "lacie,ns2_common" compatibility string would be used to enable this C code for these boards.

Quoted 23 lines · Simon Guinot

Quoted 22 lines · Andrew Lunn

Quoted 16 lines

Quoted 6 lines

Quoted 6 lines

--- a/arch/arm/mach-kirkwood/board-dt.c +++ b/arch/arm/mach-kirkwood/board-dt.c @@ -96,6 +96,11 @@ static void __init kirkwood_dt_init(void) if (of_machine_is_compatible("keymile,km_kirkwood")) km_kirkwood_init();

Quoted 9 lines

Quoted 5 lines

(quoted diff) +1 -0

+	if (of_machine_is_compatible("lacie,inetspace_v2") || +

of_machine_is_compatible("lacie,netspace_v2") || + of_machine_is_compatible("lacie,netspace_max_v2")) + ns2_init(); + of_platform_populate(NULL, kirkwood_dt_match_table, kirkwood_auxdata_lookup, NULL);

I'm not a DT policy expert. Could this be one compatibility string for all the boards? Maybe ask on the DT mainline list?

Quoted 3 lines

Maybe I could use "lacie,ns2_common" as a compatibility string. But this does not match any existing device. I don't know if it is correct.

Hi Simon

Quoted 7 lines

Quoted 7 lines

I did a bit of looking around. For kirkwood, we already have two boards sharing the same compatibility string. kirkwood-dns320.dts and kirkwood-dns325.dts both have dlink,dns-kirkwood and this is what the board-dt.c matches on.

For the tegra20 soc, all boards match on nvidia,tegra20, and that is the only compatibility string in board-dt-tegra20.c.

nvidia,tegra20 is the compatible value for the SoC itself, so naturally any Tegra20-based board has that.

Quoted 12 lines

Quoted 12 lines

So i don't see any problem having just one compatibility string here.

The question is, what is the appropriate name. How common is this common C code? Are there ns2 where this C code is not appropriate. One thing to remember is that most of this C code will soon disappear and become DT. All the mpp will be replaced with pinctrl in 3.8. I hope we can get the Ethernet setup in DT as well. You are working on ns2_led, so all the C code will be replaced by DT. So all we are really left with is power off GPIO handling.

Quoted 2 lines

Quoted 2 lines

So i think the danger of using lacie,ns2_common, and then finding it does not work with some other ns2 device is quite low.

lacie,ns2_common doesn't sound like a HW description, but rather a SW invention. DT should be describing purely the HW. If there's no common HW between these compatible boards (which seems unlikely), then there shouldn't be a shared compatible value.

Quoted 8 lines

Quoted 2 lines

What do you think?

The status for this ns2 boards is more or less end-of-life. I mean, this boards are no longer in production. So I think it is safe to consider the code inside board-ns2.c as common and shareable between ns2, is2 and ns2max.

Quoted 3 lines

Once this file will be removed, the compatibility checks will be removed as well. But one could easily forget to remove the useless compatibility string "lacie,ns2_common" from the dts...

If that value is added to the DT, it should not be removed, and the kernel should continue to support it indefinitely.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Quoted 22 lines · Stephen Warren

Quoted 21 lines · Simon Guinot

Quoted 20 lines · Andrew Lunn

Quoted 20 lines

Quoted 6 lines

Quoted 6 lines

--- a/arch/arm/mach-kirkwood/board-dt.c +++ b/arch/arm/mach-kirkwood/board-dt.c @@ -96,6 +96,11 @@ static void __init kirkwood_dt_init(void) if (of_machine_is_compatible("keymile,km_kirkwood")) km_kirkwood_init();

Quoted 10 lines

Quoted 6 lines

(quoted diff) +2 -0

+	if (of_machine_is_compatible("lacie,inetspace_v2") ||
+ of_machine_is_compatible("lacie,netspace_v2") || + 

of_machine_is_compatible("lacie,netspace_max_v2")) + ns2_init(); + of_platform_populate(NULL, kirkwood_dt_match_table, kirkwood_auxdata_lookup, NULL);

I'm not a DT policy expert. Could this be one compatibility string for all the boards? Maybe ask on the DT mainline list?

Maybe I could use "lacie,ns2_common" as a compatibility string. But this does not match any existing device. I don't know if it is correct.

Quoted 5 lines

Quoted 5 lines

The status for this ns2 boards is more or less end-of-life. I mean, this boards are no longer in production. So I think it is safe to consider the code inside board-ns2.c as common and shareable between ns2, is2 and ns2max.

Quoted 7 lines

Quoted 4 lines

Once this file will be removed, the compatibility checks will be removed as well. But one could easily forget to remove the useless compatibility string "lacie,ns2_common" from the dts...

If that value is added to the DT, it should not be removed, and the kernel should continue to support it indefinitely.

I understand it is a strong requirement. But as I mentioned before, due to the production process, one could easily end up with an embedded DTB not matching exactly the hardware. It could even be worse. A end-of-life device could be replaced by a supposed compatible one. And sometimes the compatibility assumption is false. In a such scenario, maybe you don't want the kernel to support indefinitely a DT statement.