2011-04-06 20:35:49

by Alexander Holler

[permalink] [raw]
Subject: [PATCH 0/2] ARM: Unify setup for Marvell SheevaPlugs and Seagate DockStars

Hello,

it seems the time is right to submit these 2 patches.

The first patch merges the setup for Seagate DockStars into the setup
for Marvell SheevaPlugs and the second one removes the machine type for
DockStars at all.

Removing the machine type for DockStar shouldn't be a big problem. Support
for them is already broken in mainline U-Boot since 2 versions, so changing the
stuff there is already needed and it shouldn't be a problem to use the same
machine type as used for SheevaPlugs there.

And if someone really can't change his bootloader to use the same machine type
for DockStars as for SheevaPlugs, he still could revert the second patch.

Regards,

Alexander Holler


2011-04-06 20:35:56

by Alexander Holler

[permalink] [raw]
Subject: [PATCH 1/2] ARM: Differentiate SheevaPlugs and DockStars on the basis of the memory size.

It is possible to differentiate a SheevaPlugs and DockStars on the basis
of the memory size.

This makes it possible to unify both setup files.

Afterwards it is impossible for the userland to detect the exact board
through uname, but we don't care. If necessary the difference could be
still be detected by the memory size (e.g. with free), or through the
available leds in /sys/class/leds.

Signed-off-by: Alexander Holler <[email protected]>
---
arch/arm/mach-kirkwood/Makefile | 2 +-
arch/arm/mach-kirkwood/dockstar-setup.c | 110 -----------------------------
arch/arm/mach-kirkwood/sheevaplug-setup.c | 39 ++++++++++
3 files changed, 40 insertions(+), 111 deletions(-)
delete mode 100644 arch/arm/mach-kirkwood/dockstar-setup.c

diff --git a/arch/arm/mach-kirkwood/Makefile b/arch/arm/mach-kirkwood/Makefile
index 5dcaa81..f06bc3d 100644
--- a/arch/arm/mach-kirkwood/Makefile
+++ b/arch/arm/mach-kirkwood/Makefile
@@ -7,7 +7,7 @@ obj-$(CONFIG_MACH_MV88F6281GTW_GE) += mv88f6281gtw_ge-setup.o
obj-$(CONFIG_MACH_SHEEVAPLUG) += sheevaplug-setup.o
obj-$(CONFIG_MACH_ESATA_SHEEVAPLUG) += sheevaplug-setup.o
obj-$(CONFIG_MACH_GURUPLUG) += guruplug-setup.o
-obj-$(CONFIG_MACH_DOCKSTAR) += dockstar-setup.o
+obj-$(CONFIG_MACH_DOCKSTAR) += sheevaplug-setup.o
obj-$(CONFIG_MACH_TS219) += ts219-setup.o tsx1x-common.o
obj-$(CONFIG_MACH_TS41X) += ts41x-setup.o tsx1x-common.o
obj-$(CONFIG_MACH_OPENRD) += openrd-setup.o
diff --git a/arch/arm/mach-kirkwood/dockstar-setup.c b/arch/arm/mach-kirkwood/dockstar-setup.c
deleted file mode 100644
index 433ea36..0000000
--- a/arch/arm/mach-kirkwood/dockstar-setup.c
+++ /dev/null
@@ -1,110 +0,0 @@
-/*
- * arch/arm/mach-kirkwood/dockstar-setup.c
- *
- * Seagate FreeAgent DockStar Setup
- *
- * This file is licensed under the terms of the GNU General Public
- * License version 2. This program is licensed "as is" without any
- * warranty of any kind, whether express or implied.
- */
-
-#include <linux/kernel.h>
-#include <linux/init.h>
-#include <linux/platform_device.h>
-#include <linux/ata_platform.h>
-#include <linux/mtd/partitions.h>
-#include <linux/mv643xx_eth.h>
-#include <linux/gpio.h>
-#include <linux/leds.h>
-#include <asm/mach-types.h>
-#include <asm/mach/arch.h>
-#include <mach/kirkwood.h>
-#include <plat/mvsdio.h>
-#include "common.h"
-#include "mpp.h"
-
-static struct mtd_partition dockstar_nand_parts[] = {
- {
- .name = "u-boot",
- .offset = 0,
- .size = SZ_1M
- }, {
- .name = "uImage",
- .offset = MTDPART_OFS_NXTBLK,
- .size = SZ_4M
- }, {
- .name = "root",
- .offset = MTDPART_OFS_NXTBLK,
- .size = MTDPART_SIZ_FULL
- },
-};
-
-static struct mv643xx_eth_platform_data dockstar_ge00_data = {
- .phy_addr = MV643XX_ETH_PHY_ADDR(0),
-};
-
-static struct gpio_led dockstar_led_pins[] = {
- {
- .name = "dockstar:green:health",
- .default_trigger = "default-on",
- .gpio = 46,
- .active_low = 1,
- },
- {
- .name = "dockstar:orange:misc",
- .default_trigger = "none",
- .gpio = 47,
- .active_low = 1,
- },
-};
-
-static struct gpio_led_platform_data dockstar_led_data = {
- .leds = dockstar_led_pins,
- .num_leds = ARRAY_SIZE(dockstar_led_pins),
-};
-
-static struct platform_device dockstar_leds = {
- .name = "leds-gpio",
- .id = -1,
- .dev = {
- .platform_data = &dockstar_led_data,
- }
-};
-
-static unsigned int dockstar_mpp_config[] __initdata = {
- MPP29_GPIO, /* USB Power Enable */
- MPP46_GPIO, /* LED green */
- MPP47_GPIO, /* LED orange */
- 0
-};
-
-static void __init dockstar_init(void)
-{
- /*
- * Basic setup. Needs to be called early.
- */
- kirkwood_init();
-
- /* setup gpio pin select */
- kirkwood_mpp_conf(dockstar_mpp_config);
-
- kirkwood_uart0_init();
- kirkwood_nand_init(ARRAY_AND_SIZE(dockstar_nand_parts), 25);
-
- if (gpio_request(29, "USB Power Enable") != 0 ||
- gpio_direction_output(29, 1) != 0)
- printk(KERN_ERR "can't set up GPIO 29 (USB Power Enable)\n");
- kirkwood_ehci_init();
-
- kirkwood_ge00_init(&dockstar_ge00_data);
-
- platform_device_register(&dockstar_leds);
-}
-
-MACHINE_START(DOCKSTAR, "Seagate FreeAgent DockStar")
- .boot_params = 0x00000100,
- .init_machine = dockstar_init,
- .map_io = kirkwood_map_io,
- .init_irq = kirkwood_init_irq,
- .timer = &kirkwood_timer,
-MACHINE_END
diff --git a/arch/arm/mach-kirkwood/sheevaplug-setup.c b/arch/arm/mach-kirkwood/sheevaplug-setup.c
index d2eec35..d1593db 100644
--- a/arch/arm/mach-kirkwood/sheevaplug-setup.c
+++ b/arch/arm/mach-kirkwood/sheevaplug-setup.c
@@ -18,6 +18,7 @@
#include <linux/leds.h>
#include <asm/mach-types.h>
#include <asm/mach/arch.h>
+#include <asm/setup.h>
#include <mach/kirkwood.h>
#include <plat/mvsdio.h>
#include "common.h"
@@ -65,6 +66,21 @@ static struct gpio_led sheevaplug_led_pins[] = {
},
};

+static struct gpio_led dockstar_led_pins[] = {
+ {
+ .name = "dockstar:green:health",
+ .default_trigger = "default-on",
+ .gpio = 46,
+ .active_low = 1,
+ },
+ {
+ .name = "dockstar:orange:misc",
+ .default_trigger = "none",
+ .gpio = 47,
+ .active_low = 1,
+ },
+};
+
static struct gpio_led_platform_data sheevaplug_led_data = {
.leds = sheevaplug_led_pins,
.num_leds = ARRAY_SIZE(sheevaplug_led_pins),
@@ -92,6 +108,13 @@ static unsigned int sheeva_esata_mpp_config[] __initdata = {
0
};

+static unsigned int dockstar_mpp_config[] __initdata = {
+ MPP29_GPIO, /* USB Power Enable */
+ MPP46_GPIO, /* LED green */
+ MPP47_GPIO, /* LED orange */
+ 0
+};
+
static void __init sheevaplug_init(void)
{
/*
@@ -102,6 +125,12 @@ static void __init sheevaplug_init(void)
/* setup gpio pin select */
if (machine_is_sheeva_esata())
kirkwood_mpp_conf(sheeva_esata_mpp_config);
+ else if ((meminfo.nr_banks == 1) && (meminfo.bank[0].size == SZ_128M)) {
+ pr_info("Assuming machine is a DockStar (memory size is 128MB)\n");
+ kirkwood_mpp_conf(dockstar_mpp_config);
+ sheevaplug_led_data.leds = dockstar_led_pins;
+ sheevaplug_led_data.num_leds = ARRAY_SIZE(dockstar_led_pins);
+ }
else
kirkwood_mpp_conf(sheevaplug_mpp_config);

@@ -148,3 +177,13 @@ MACHINE_START(ESATA_SHEEVAPLUG, "Marvell eSATA SheevaPlug Reference Board")
.timer = &kirkwood_timer,
MACHINE_END
#endif
+
+#ifdef CONFIG_MACH_DOCKSTAR
+MACHINE_START(DOCKSTAR, "Seagate FreeAgent DockStar")
+ .boot_params = 0x00000100,
+ .init_machine = sheevaplug_init,
+ .map_io = kirkwood_map_io,
+ .init_irq = kirkwood_init_irq,
+ .timer = &kirkwood_timer,
+MACHINE_END
+#endif
--
1.7.2.2

2011-04-06 20:35:55

by Alexander Holler

[permalink] [raw]
Subject: [PATCH 2/2] ARM: Remove machine type dockstar (use sheevaplug instead)

There is no need to have a different machine type for Seagate DockStars.
Remove it.

Be aware, this patch requires a change in the bootloader to feed the
same machine type to the kernel as used for Marvell SheevaPlugs.

Signed-off-by: Alexander Holler <[email protected]>
---
arch/arm/mach-kirkwood/Kconfig | 10 ++--------
arch/arm/mach-kirkwood/Makefile | 1 -
arch/arm/mach-kirkwood/sheevaplug-setup.c | 10 ----------
3 files changed, 2 insertions(+), 19 deletions(-)

diff --git a/arch/arm/mach-kirkwood/Kconfig b/arch/arm/mach-kirkwood/Kconfig
index 7fc603b..9dafb58 100644
--- a/arch/arm/mach-kirkwood/Kconfig
+++ b/arch/arm/mach-kirkwood/Kconfig
@@ -27,10 +27,10 @@ config MACH_MV88F6281GTW_GE
Marvell 88F6281 GTW GE Board.

config MACH_SHEEVAPLUG
- bool "Marvell SheevaPlug Reference Board"
+ bool "Marvell SheevaPlug Reference Board or Seagate DockStar"
help
Say 'Y' here if you want your kernel to support the
- Marvell SheevaPlug Reference Board.
+ Marvell SheevaPlug Reference Board or the Seagate DockStar.

config MACH_ESATA_SHEEVAPLUG
bool "Marvell eSATA SheevaPlug Reference Board"
@@ -58,12 +58,6 @@ config MACH_TS41X
QNAP TS-410, TS-410U, TS-419P, TS-419P+ and TS-419U Turbo
NAS devices.

-config MACH_DOCKSTAR
- bool "Seagate FreeAgent DockStar"
- help
- Say 'Y' here if you want your kernel to support the
- Seagate FreeAgent DockStar.
-
config MACH_OPENRD
bool

diff --git a/arch/arm/mach-kirkwood/Makefile b/arch/arm/mach-kirkwood/Makefile
index f06bc3d..48c3c83 100644
--- a/arch/arm/mach-kirkwood/Makefile
+++ b/arch/arm/mach-kirkwood/Makefile
@@ -7,7 +7,6 @@ obj-$(CONFIG_MACH_MV88F6281GTW_GE) += mv88f6281gtw_ge-setup.o
obj-$(CONFIG_MACH_SHEEVAPLUG) += sheevaplug-setup.o
obj-$(CONFIG_MACH_ESATA_SHEEVAPLUG) += sheevaplug-setup.o
obj-$(CONFIG_MACH_GURUPLUG) += guruplug-setup.o
-obj-$(CONFIG_MACH_DOCKSTAR) += sheevaplug-setup.o
obj-$(CONFIG_MACH_TS219) += ts219-setup.o tsx1x-common.o
obj-$(CONFIG_MACH_TS41X) += ts41x-setup.o tsx1x-common.o
obj-$(CONFIG_MACH_OPENRD) += openrd-setup.o
diff --git a/arch/arm/mach-kirkwood/sheevaplug-setup.c b/arch/arm/mach-kirkwood/sheevaplug-setup.c
index d1593db..f548aa9 100644
--- a/arch/arm/mach-kirkwood/sheevaplug-setup.c
+++ b/arch/arm/mach-kirkwood/sheevaplug-setup.c
@@ -177,13 +177,3 @@ MACHINE_START(ESATA_SHEEVAPLUG, "Marvell eSATA SheevaPlug Reference Board")
.timer = &kirkwood_timer,
MACHINE_END
#endif
-
-#ifdef CONFIG_MACH_DOCKSTAR
-MACHINE_START(DOCKSTAR, "Seagate FreeAgent DockStar")
- .boot_params = 0x00000100,
- .init_machine = sheevaplug_init,
- .map_io = kirkwood_map_io,
- .init_irq = kirkwood_init_irq,
- .timer = &kirkwood_timer,
-MACHINE_END
-#endif
--
1.7.2.2

2011-04-06 21:02:55

by Nicolas Pitre

[permalink] [raw]
Subject: Re: [PATCH 2/2] ARM: Remove machine type dockstar (use sheevaplug instead)

On Wed, 6 Apr 2011, Alexander Holler wrote:

> There is no need to have a different machine type for Seagate DockStars.
> Remove it.
>
> Be aware, this patch requires a change in the bootloader to feed the
> same machine type to the kernel as used for Marvell SheevaPlugs.

This is therefore not a good idea to do this.

If the SheevaPlug setup code can be reused as is for DockStars, then it
is best to simply add an additional machine record along with the
SheevaPlug one.


Nicolas

2011-04-06 21:44:11

by Alexander Holler

[permalink] [raw]
Subject: Re: [PATCH 2/2] ARM: Remove machine type dockstar (use sheevaplug instead)

On 06.04.2011 23:02, Nicolas Pitre wrote:
> On Wed, 6 Apr 2011, Alexander Holler wrote:
>
>> There is no need to have a different machine type for Seagate DockStars.
>> Remove it.
>>
>> Be aware, this patch requires a change in the bootloader to feed the
>> same machine type to the kernel as used for Marvell SheevaPlugs.
>
> This is therefore not a good idea to do this.

The original bootloader isn't affected at all, only already modified
ones which are using the machine type dockstar. And I don't think this
are that many because that machine type was only supported for one or
two revisions of U-Boot.

Regards,

Alexander Holler

2011-04-06 21:44:45

by Nicolas Pitre

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: Differentiate SheevaPlugs and DockStars on the basis of the memory size.

On Wed, 6 Apr 2011, Alexander Holler wrote:

> It is possible to differentiate a SheevaPlugs and DockStars on the basis
> of the memory size.
>
> This makes it possible to unify both setup files.

No no no !!! This is an abomination!

We are not going to reduce the amount of code under arch/arm/ with such
fragile hacks. This would create an even worse maintenance problem the
day either of those devices is released with more RAM or whatever.


Nicolas

2011-04-06 22:45:56

by Alexander Holler

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: Differentiate SheevaPlugs and DockStars on the basis of the memory size.

On 06.04.2011 23:44, Nicolas Pitre wrote:
> On Wed, 6 Apr 2011, Alexander Holler wrote:
>
>> It is possible to differentiate a SheevaPlugs and DockStars on the basis
>> of the memory size.
>>
>> This makes it possible to unify both setup files.
>
> No no no !!! This is an abomination!
>
> We are not going to reduce the amount of code under arch/arm/ with such
> fragile hacks. This would create an even worse maintenance problem the
> day either of those devices is released with more RAM or whatever.

DockStars are already obsolet and vanilla Linux was never supported by
the manufacturer (Seagate). And they never used the machine type in
question.

So I don't think this in any way a fragile hack.

Anyway, it was just a suggestion.

Regards,

Alexander

2011-04-06 23:01:09

by Nicolas Pitre

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: Differentiate SheevaPlugs and DockStars on the basis of the memory size.

On Thu, 7 Apr 2011, Alexander Holler wrote:

> On 06.04.2011 23:44, Nicolas Pitre wrote:
> > On Wed, 6 Apr 2011, Alexander Holler wrote:
> >
> > > It is possible to differentiate a SheevaPlugs and DockStars on the basis
> > > of the memory size.
> > >
> > > This makes it possible to unify both setup files.
> >
> > No no no !!! This is an abomination!
> >
> > We are not going to reduce the amount of code under arch/arm/ with such
> > fragile hacks. This would create an even worse maintenance problem the
> > day either of those devices is released with more RAM or whatever.
>
> DockStars are already obsolet and vanilla Linux was never supported by the
> manufacturer (Seagate). And they never used the machine type in question.

Is the code for DockStar in mainline actually useful? If no then we may
simply delete it.

> So I don't think this in any way a fragile hack.

Determining a machine type based on its amount of RAM is fragile, ugly
and sets up a bad example for even more hacky tricks like this to crop
up. If someone is experimenting with his SheevaPlug by giving different
memory information in the kernel cmdline e.g. to create memory holes in
order to test some memory allocator changes, then the kernel may think
that it is not running on a SheevaPlug but a DockStar, and the resulting
behavior will certainly be unexpected.

> Anyway, it was just a suggestion.

Better luck next time.


Nicolas

2011-04-06 23:22:40

by Alexander Holler

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: Differentiate SheevaPlugs and DockStars on the basis of the memory size.

On 07.04.2011 01:01, Nicolas Pitre wrote:
> On Thu, 7 Apr 2011, Alexander Holler wrote:
>
>> On 06.04.2011 23:44, Nicolas Pitre wrote:
>>> On Wed, 6 Apr 2011, Alexander Holler wrote:
>>>
>>>> It is possible to differentiate a SheevaPlugs and DockStars on the basis
>>>> of the memory size.
>>>>
>>>> This makes it possible to unify both setup files.
>>>
>>> No no no !!! This is an abomination!
>>>
>>> We are not going to reduce the amount of code under arch/arm/ with such
>>> fragile hacks. This would create an even worse maintenance problem the
>>> day either of those devices is released with more RAM or whatever.
>>
>> DockStars are already obsolet and vanilla Linux was never supported by the
>> manufacturer (Seagate). And they never used the machine type in question.
>
> Is the code for DockStar in mainline actually useful? If no then we may
> simply delete it.

I don't know, I can't speak for others.

>> So I don't think this in any way a fragile hack.
>
> Determining a machine type based on its amount of RAM is fragile, ugly
> and sets up a bad example for even more hacky tricks like this to crop
> up. If someone is experimenting with his SheevaPlug by giving different
> memory information in the kernel cmdline e.g. to create memory holes in
> order to test some memory allocator changes, then the kernel may think
> that it is not running on a SheevaPlug but a DockStar, and the resulting
> behavior will certainly be unexpected.

People doing such should now what they do. And the pr_info() in the
"abomination" should be enough to give those experimenters a hint (if it
is missing or new).

>> Anyway, it was just a suggestion.
>
> Better luck next time.

Sorry, I don't feel the need to waste my time prodcuing patches to get
them called "abominations". Which means my willingness to post further
patches just got below zero.

Regards,

Alexander

2011-04-06 23:23:33

by Nico Erfurth

[permalink] [raw]
Subject: Re: [PATCH 0/2] ARM: Unify setup for Marvell SheevaPlugs and Seagate DockStars

Alexander Holler wrote:

> The first patch merges the setup for Seagate DockStars into the setup
> for Marvell SheevaPlugs and the second one removes the machine type for
> DockStars at all.

That looks fine so far.

> Removing the machine type for DockStar shouldn't be a big problem. Support
> for them is already broken in mainline U-Boot since 2 versions, so changing the
> stuff there is already needed and it shouldn't be a problem to use the same
> machine type as used for SheevaPlugs there.

This sounds like very bad reasoning to me. The Dockstar has a machine-id
assigned, if the bootloader is broken, people should either use a
working version, or fix the current one.

Also, using the memory-size to differ between these two machines sounds
like something thats doomed to fail in the future.

Nico

2011-04-07 00:10:21

by Eric Cooper

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: Differentiate SheevaPlugs and DockStars on the basis of the memory size.

On Wed, Apr 06, 2011 at 07:01:07PM -0400, Nicolas Pitre wrote:
> Is the code for DockStar in mainline actually useful?

Yes, because it sets up the correct LEDs.

--
Eric Cooper e c c @ c m u . e d u

2011-04-07 02:55:32

by Nicolas Pitre

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: Differentiate SheevaPlugs and DockStars on the basis of the memory size.

On Thu, 7 Apr 2011, Alexander Holler wrote:

> > > So I don't think this in any way a fragile hack.
> >
> > Determining a machine type based on its amount of RAM is fragile, ugly
> > and sets up a bad example for even more hacky tricks like this to crop
> > up. If someone is experimenting with his SheevaPlug by giving different
> > memory information in the kernel cmdline e.g. to create memory holes in
> > order to test some memory allocator changes, then the kernel may think
> > that it is not running on a SheevaPlug but a DockStar, and the resulting
> > behavior will certainly be unexpected.
>
> People doing such should now what they do. And the pr_info() in the
> "abomination" should be enough to give those experimenters a hint (if it is
> missing or new).

They shouldn't have to. We have a well defined system to identify
machines/platforms with a unique number already. It is completely
unambiguous. What you're proposing is to break that system.

The proper way to go about this rationalization is to move that stuff
over to device tree.

> > > Anyway, it was just a suggestion.
> >
> > Better luck next time.
>
> Sorry, I don't feel the need to waste my time prodcuing patches to get them
> called "abominations". Which means my willingness to post further patches just
> got below zero.

I'm sorry if you feel like that. You shouldn't take this too personal.
It is not like if I qualified _you_ as a moron and a pariah (yes this
happened to me last week from our master penguin himself).


Nicolas

2011-04-07 09:08:28

by Alexander Clouter

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: Differentiate SheevaPlugs and DockStars on the basis of the memory size.

In gmane.linux.kernel Alexander Holler <[email protected]> wrote:
>
> Sorry, I don't feel the need to waste my time prodcuing patches to get
> them called "abominations". Which means my willingness to post further
> patches just got below zero.
>
In a posting from Nico before you got offended it was explained to you
*why* it was a Bad Idea(tm) and that that sort of thing simply does not
work. As a game, is it really a situation with *zero* probability of
occuring that someone might produce a kirkwood/Sheeva flavoured board
with only 128MB RAM?

As well as a willingness to 'produce' patches (which to be frank
*anyone* can do[1])...the hard part is producing *good* patches. To
produce good patches you need to read and understand the wisdom you get
back from the mailing lists you post to. If you do not understand the
reasoning, ask.

This is why you see [PATCHv${BIGNUM}] so often in a number of mailing
lists. If you are not willing to accept *everyone*, including yourself,
writes crap code...well the value of your patches falls below zero.

Cheers

[1] just look at the abomination Marvell released for their 'orion5x'
chipset originally

--
Alexander Clouter
.sigmonster says: But Captain -- the engines can't take this much longer!

2011-04-07 09:21:04

by Alexander Holler

[permalink] [raw]
Subject: Re: [PATCH 0/2] ARM: Unify setup for Marvell SheevaPlugs and Seagate DockStars

Am 07.04.2011 00:50, schrieb Nico Erfurth:
> Alexander Holler wrote:
>
>> The first patch merges the setup for Seagate DockStars into the setup
>> for Marvell SheevaPlugs and the second one removes the machine type for
>> DockStars at all.
>
> That looks fine so far.
>
>> Removing the machine type for DockStar shouldn't be a big problem. Support
>> for them is already broken in mainline U-Boot since 2 versions, so changing the
>> stuff there is already needed and it shouldn't be a problem to use the same
>> machine type as used for SheevaPlugs there.
>
> This sounds like very bad reasoning to me. The Dockstar has a machine-id
> assigned, if the bootloader is broken, people should either use a
> working version, or fix the current one.
>
> Also, using the memory-size to differ between these two machines sounds
> like something thats doomed to fail in the future.

I wonder how many people believe that either there will be another
DockStar with the same HW and GPIOs for the LEDs but more memory (and
still without sata) or that there will be another SheevaPlug with just
128MB RAM or that someone could have a reason to change the memory
layout using a mem= parameter.

For me all that is pretty unlikely.

Anyway, if someone wants, he could just use my patch 1/2 and replace
patch 2/2 with a patch which changes the one line which checks the
memory layout to be one bank with 128MB to machine_is_dockstar().

Regards,

Alexander

2011-04-07 09:37:49

by Nico Erfurth

[permalink] [raw]
Subject: Re: [PATCH 0/2] ARM: Unify setup for Marvell SheevaPlugs and Seagate DockStars

Alexander Holler wrote:

> I wonder how many people believe that either there will be another
> DockStar with the same HW and GPIOs for the LEDs but more memory (and
> still without sata) or that there will be another SheevaPlug with just
> 128MB RAM or that someone could have a reason to change the memory
> layout using a mem= parameter.
>
> For me all that is pretty unlikely.

As Nicolas stated it's not just about "Oh, thats totally unlikely to
happen!". It is about maintainable code, if somebody looks at it in 3
years they should not think "WTF?!?!". Using machine ids and the
generated macros helps to keep the code clean and readable.

Nico

2011-04-07 09:44:10

by Alexander Holler

[permalink] [raw]
Subject: Re: [PATCH 0/2] ARM: Unify setup for Marvell SheevaPlugs and Seagate DockStars

Am 07.04.2011 11:37, schrieb Nico Erfurth:
> Alexander Holler wrote:
>
>> I wonder how many people believe that either there will be another
>> DockStar with the same HW and GPIOs for the LEDs but more memory (and
>> still without sata) or that there will be another SheevaPlug with just
>> 128MB RAM or that someone could have a reason to change the memory
>> layout using a mem= parameter.
>>
>> For me all that is pretty unlikely.
>
> As Nicolas stated it's not just about "Oh, thats totally unlikely to
> happen!". It is about maintainable code, if somebody looks at it in 3
> years they should not think "WTF?!?!". Using machine ids and the
> generated macros helps to keep the code clean and readable.

Sorry, I can't agree. For me some unique hardware identifier is more
reasonable, than some machine id which comes from outerspace.

And in no way I see any argument for that "clean and readable", at least
not in the patch I posted.

Anyway, I leave this discussion and wish all a nice day.

Regards,

Alexander

2011-04-07 17:39:31

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH 0/2] ARM: Unify setup for Marvell SheevaPlugs and Seagate DockStars

On Thu, Apr 07, 2011 at 11:44:02AM +0200, Alexander Holler wrote:
> Am 07.04.2011 11:37, schrieb Nico Erfurth:
>> Alexander Holler wrote:
>>
>>> I wonder how many people believe that either there will be another
>>> DockStar with the same HW and GPIOs for the LEDs but more memory (and
>>> still without sata) or that there will be another SheevaPlug with just
>>> 128MB RAM or that someone could have a reason to change the memory
>>> layout using a mem= parameter.
>>>
>>> For me all that is pretty unlikely.
>>
>> As Nicolas stated it's not just about "Oh, thats totally unlikely to
>> happen!". It is about maintainable code, if somebody looks at it in 3
>> years they should not think "WTF?!?!". Using machine ids and the
>> generated macros helps to keep the code clean and readable.
>
> Sorry, I can't agree. For me some unique hardware identifier is more
> reasonable, than some machine id which comes from outerspace.

I agree 100% with Nicolas - using memory size is far from obvious and
is not clean and understandable. Using memory size as a way of detecting
the machine type is far worse than just mere "silly".

We have an established API and convention in the kernel which you claim
is "from outerspace". It's not "from outerspace" but a designed API to
allow platforms to live together in the same kernel image. So I find
your arguments totally unreasonable.

I fully support Nicolas in rejecting your patches outright on this point
alone.

2011-04-07 21:31:19

by Alexander Holler

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: Differentiate SheevaPlugs and DockStars on the basis of the memory size.

Am 07.04.2011 10:55, schrieb Alexander Clouter:
> In gmane.linux.kernel Alexander Holler<[email protected]> wrote:
>>
>> Sorry, I don't feel the need to waste my time prodcuing patches to get
>> them called "abominations". Which means my willingness to post further
>> patches just got below zero.
>>
> In a posting from Nico before you got offended it was explained to you
> *why* it was a Bad Idea(tm) and that that sort of thing simply does not
> work. As a game, is it really a situation with *zero* probability of
> occuring that someone might produce a kirkwood/Sheeva flavoured board
> with only 128MB RAM?
>
> As well as a willingness to 'produce' patches (which to be frank
> *anyone* can do[1])...the hard part is producing *good* patches. To
> produce good patches you need to read and understand the wisdom you get
> back from the mailing lists you post to. If you do not understand the
> reasoning, ask.
>
> This is why you see [PATCHv${BIGNUM}] so often in a number of mailing
> lists. If you are not willing to accept *everyone*, including yourself,
> writes crap code...well the value of your patches falls below zero.

Requiring a machine ID and the needed stuff to handle that for a board
which just is using two GPIOs different than another board is why the
ARM tree exploded. And requiring a machine ID just to follow the rule
that a machine ID is much better than some pretty unique hw feature is
in my humble opinion senseless and that the resulting code is more
readable and better to maintain is a myth.

Sorry that I wanted to help there. Will not try it again.

Regards,

Alexander

2011-04-07 21:45:00

by Alexander Clouter

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: Differentiate SheevaPlugs and DockStars on the basis of the memory size.

* Alexander Holler <[email protected]> [2011-04-07 23:31:08+0200]:
>
> Requiring a machine ID and the needed stuff to handle that for a
> board which just is using two GPIOs different than another board is
> why the ARM tree exploded. And requiring a machine ID just to follow
> the rule that a machine ID is much better than some pretty unique hw
> feature is in my humble opinion senseless and that the resulting code
> is more readable and better to maintain is a myth.
>
It's something that most people agree with you on too. However, I am
guessing it's around as no one else *at the time* of it's conception
could think of a better way.

Now this is why there is a lot of work being done on device-tree.

> Sorry that I wanted to help there. Will not try it again.
>
Yeah, as obviously no tries to help anyone improve around here:

http://groups.google.com/group/openrd/browse_frm/thread/7ba48f2d7916a88f/2b89c877c4f39098?tvc=1&fwc=1
http://lists.infradead.org/pipermail/linux-arm-kernel/2010-August/022855.html
http://lists.infradead.org/pipermail/linux-arm-kernel/2010-August/024072.html

http://git.kernel.org/?p=linux/kernel/git/nico/orion.git;a=commit;h=f2ac38dc

Regards

--
Alexander Clouter
.sigmonster says: Graduate life: It's not just a job. It's an indenture.

2011-04-07 22:00:15

by Alexander Holler

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: Differentiate SheevaPlugs and DockStars on the basis of the memory size.

Am 07.04.2011 23:44, schrieb Alexander Clouter:
> * Alexander Holler<[email protected]> [2011-04-07 23:31:08+0200]:
>>
>> Requiring a machine ID and the needed stuff to handle that for a
>> board which just is using two GPIOs different than another board is
>> why the ARM tree exploded. And requiring a machine ID just to follow
>> the rule that a machine ID is much better than some pretty unique hw
>> feature is in my humble opinion senseless and that the resulting code
>> is more readable and better to maintain is a myth.
>>
> It's something that most people agree with you on too. However, I am
> guessing it's around as no one else *at the time* of it's conception
> could think of a better way.
>
> Now this is why there is a lot of work being done on device-tree.
>
>> Sorry that I wanted to help there. Will not try it again.
>>
> Yeah, as obviously no tries to help anyone improve around here:

That is future, I've tried to help with the existing stuff. Maybe I
should have suggested just to remove all boards now which never will get
support for DT. Linux might get happy.

And than I get called that I'm writing an abomination, which would have
been nice with just the change of one line.

Sorry guys, this isn't what motivates people, at least not without the
needed compensations to tolerate such manners.

Regards,

Alexander

2011-04-07 22:09:08

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: Differentiate SheevaPlugs and DockStars on the basis of the memory size.

On Thu, Apr 07, 2011 at 11:31:08PM +0200, Alexander Holler wrote:
> Am 07.04.2011 10:55, schrieb Alexander Clouter:
>> In gmane.linux.kernel Alexander Holler<[email protected]> wrote:
>>>
>>> Sorry, I don't feel the need to waste my time prodcuing patches to get
>>> them called "abominations". Which means my willingness to post further
>>> patches just got below zero.
>>>
>> In a posting from Nico before you got offended it was explained to you
>> *why* it was a Bad Idea(tm) and that that sort of thing simply does not
>> work. As a game, is it really a situation with *zero* probability of
>> occuring that someone might produce a kirkwood/Sheeva flavoured board
>> with only 128MB RAM?
>>
>> As well as a willingness to 'produce' patches (which to be frank
>> *anyone* can do[1])...the hard part is producing *good* patches. To
>> produce good patches you need to read and understand the wisdom you get
>> back from the mailing lists you post to. If you do not understand the
>> reasoning, ask.
>>
>> This is why you see [PATCHv${BIGNUM}] so often in a number of mailing
>> lists. If you are not willing to accept *everyone*, including yourself,
>> writes crap code...well the value of your patches falls below zero.
>
> Requiring a machine ID and the needed stuff to handle that for a board
> which just is using two GPIOs different than another board is why the
> ARM tree exploded.

You can not be any more wrong than that.

The reason the ARM tree exploded is because of the compartmentalized
sub-community structure, where the vast majority of (eg) OMAP development
is done independently of the (eg) Samsung development.

Consequently, there's no attempt to consolidate code between the SoCs,
even for basic stuff like 32-bit up-counting timers. We've ended up with
_nine_ implementations of clocksources all doing the same thing in that
respect.

That's got precisely zilch to do with machine IDs, and your attempt to
blame the bloat on machine IDs just shows how misinformed you are.

2011-04-07 23:04:10

by Alexander Holler

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: Differentiate SheevaPlugs and DockStars on the basis of the memory size.

Am 08.04.2011 00:08, schrieb Russell King - ARM Linux:

>> Requiring a machine ID and the needed stuff to handle that for a board
>> which just is using two GPIOs different than another board is why the
>> ARM tree exploded.
>
> You can not be any more wrong than that.
>
> The reason the ARM tree exploded is because of the compartmentalized
> sub-community structure, where the vast majority of (eg) OMAP development
> is done independently of the (eg) Samsung development.
>
> Consequently, there's no attempt to consolidate code between the SoCs,
> even for basic stuff like 32-bit up-counting timers. We've ended up with
> _nine_ implementations of clocksources all doing the same thing in that
> respect.
>
> That's got precisely zilch to do with machine IDs, and your attempt to
> blame the bloat on machine IDs just shows how misinformed you are.

I had a look at what's going on in the OMAP linux world for more than a
year now and I think I've seen a lot of the stuff you are referring to.

And I think one of the reasons that the mess happened is the same I've
got trapped in. Why should anyone try to submit patches if he must fear
to get caught in some senseless endless discussion about one line.

E.g. requiring people to use NULL than 0 or that stupid discussion now
about the simple patch I've posted. I'm writing whole (readable) C++
applications (not crap!) in less time than what's is needed to submit
and discuss a small patch for some silly hw.

So for me it's fully understandable why companies don't try to work with
kernel people at first. They try to develop innovativ products they can
sell, and it doesn't help if their developers would have to fear that
they get called stupid, crap and abonimation writing aliens for things
like the small patch I've posted, which in turn requires them to defend
them against those people doing so (which ends in such stupid
discussions like this).

It's one thing to say such to someone you know and work with, but it's a
totally different thing to say such to someone you almost know nothing
about. And not everybody who hasn't the name of a big company in his
email address is a moron.

Sorry, I'm getting sick having to defend me here against people who like
to call others crap and abonimation writing ones just because they have
maintainer status or whatever.

Regards,

Alexander

2011-04-08 03:39:08

by Alexander Holler

[permalink] [raw]
Subject: Re: [PATCH 0/2] ARM: Unify setup for Marvell SheevaPlugs and Seagate DockStars

Am 07.04.2011 19:39, schrieb Russell King - ARM Linux:

> We have an established API and convention in the kernel which you claim
> is "from outerspace". It's not "from outerspace" but a designed API to
> allow platforms to live together in the same kernel image. So I find
> your arguments totally unreasonable.

I've claimed that the specific machine ID is from outerspace not the
API. This one (ID) got invented just because of a different usage of two
GPIOs (and not even by the manufacturerer). And I still think using the
(fixed) memory size here is totally reasonable. There is nothing else to
differentiate these HWs, it's extremely unlikely that this difference
will get proved wrong, and if that would really happen, than one could
still add some code to fix this specific problem. _Always_ dealing with
"what happens when" is (imho) fruitless and no solution or API can deal
with everything.

> I fully support Nicolas in rejecting your patches outright on this point
> alone.

Which I fully understand. And I wouldn't have started to defend me, if
that would have happened in a more civil language.

But because I don't want to get involved in more discussions with people
who are missing the needed manners (at least how I learned them) to talk
with other people without offending them (not you), I better try to stay
away posting patches.

Maybe I'm getting too old and lacking the needed enthusiasm needed to
discuss such stuff (and with everyone who thinks he must enter this
discussion too, again, not you). ;)

I wish you all a nice day,

Alexander

2011-04-08 07:44:56

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: Differentiate SheevaPlugs and DockStars on the basis of the memory size.

On Fri, Apr 08, 2011 at 01:04:01AM +0200, Alexander Holler wrote:
> Am 08.04.2011 00:08, schrieb Russell King - ARM Linux:
>
>>> Requiring a machine ID and the needed stuff to handle that for a board
>>> which just is using two GPIOs different than another board is why the
>>> ARM tree exploded.
>>
>> You can not be any more wrong than that.
>>
>> The reason the ARM tree exploded is because of the compartmentalized
>> sub-community structure, where the vast majority of (eg) OMAP development
>> is done independently of the (eg) Samsung development.
>>
>> Consequently, there's no attempt to consolidate code between the SoCs,
>> even for basic stuff like 32-bit up-counting timers. We've ended up with
>> _nine_ implementations of clocksources all doing the same thing in that
>> respect.
>>
>> That's got precisely zilch to do with machine IDs, and your attempt to
>> blame the bloat on machine IDs just shows how misinformed you are.
>
> I had a look at what's going on in the OMAP linux world for more than a
> year now and I think I've seen a lot of the stuff you are referring to.
>
> And I think one of the reasons that the mess happened is the same I've
> got trapped in. Why should anyone try to submit patches if he must fear
> to get caught in some senseless endless discussion about one line.

Because matching against the memory size is _technically_ the wrong thing
to do, as you've already been told.

2011-04-08 09:34:06

by Alexander Holler

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: Differentiate SheevaPlugs and DockStars on the basis of the memory size.

Hello,

Am 08.04.2011 09:24, schrieb Russell King - ARM Linux:
>
>> And I think one of the reasons that the mess happened is the same I've
>> got trapped in. Why should anyone try to submit patches if he must fear
>> to get caught in some senseless endless discussion about one line.
>>
> Because matching against the memory size is _technically_ the wrong thing
> to do, as you've already been told.
>
Together with the assumption that I'm using a dice to write code.

Thanks,

Alexander

2011-04-09 08:29:36

by Bernd Petrovitsch

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: Differentiate SheevaPlugs and DockStars on the basis of the memory size.

On Fre, 2011-04-08 at 01:04 +0200, Alexander Holler wrote:
[...]
> I had a look at what's going on in the OMAP linux world for more than a
> year now and I think I've seen a lot of the stuff you are referring to.

Others were far longer in in several of these "Linux worlds" ....

> And I think one of the reasons that the mess happened is the same I've
> got trapped in. Why should anyone try to submit patches if he must fear
> to get caught in some senseless endless discussion about one line.

Welcome to the world where software is actually really maintained - and
not longer in the easy world of "fork it, hack it up to ship it and be
done with it - except of bugs actually reported by customers who care
enough to get the bug report that far" like it is not uncommon in the
embedded world.

> E.g. requiring people to use NULL than 0 or that stupid discussion now

You obviously where never forced to bugfix with source where you can
identify the writer of each be the style.

> about the simple patch I've posted. I'm writing whole (readable) C++

"readable" is quite subjective. And just because it is readable by you
doesn't imply it is readable for many others. Especially if you actually
wrote it .....

> applications (not crap!) in less time than what's is needed to submit
> and discuss a small patch for some silly hw.

That's precisely the point: Writing the source is the easiest job.
Maintaining it over years and keeping it clear is hard.

> So for me it's fully understandable why companies don't try to work with
> kernel people at first. They try to develop innovativ products they can
> sell, and it doesn't help if their developers would have to fear that

"Innovative products which actually sell" are neither necessary nor
sufficient to produce maintainable code.
IMHO it it quite the opposite - these companies just want to get their
products built as quickly as possible to minimize the time to market and
get it sold (as in "second place, first looser") and be done with it
(except the sales part of course).
And the vast majority of the software on these products is actually not
really maintained - they have just bugs fixed if they crop up and some
user works hard enough to get the bug report actually to someone who
might fix it.
Existing software is just patched to implement the "new features" far
enough to be usable on that one product - ignoring the big picture of
the upstream with all the features and possibilities.
For the next release/version/product, the old code is plain simply
forked and everyone moves on.

[...]
> It's one thing to say such to someone you know and work with, but it's a
> totally different thing to say such to someone you almost know nothing
> about. And not everybody who hasn't the name of a big company in his
> email address is a moron.

You have to read more mails on the LKML and you will see, that the
domain name in the mail address help in any direction - neither to avoid
being flamed nor that a patch goes faster in.
Of course, there *are* people out there where it looks like that a
well-known domain name helps - but most of them started to hack on the
kernel and (successfully) submit patches with other domains. Think about
it ...

> Sorry, I'm getting sick having to defend me here against people who like
> to call others crap and abonimation writing ones just because they have
> maintainer status or whatever.

The main reason was a pure technical one. Think about it ....

Bernd
--
Bernd Petrovitsch Email : [email protected]
LUGA : http://www.luga.at

2011-04-10 18:14:36

by Alexander Holler

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: Differentiate SheevaPlugs and DockStars on the basis of the memory size.

Hello,

On 09.04.2011 10:29, Bernd Petrovitsch wrote:

Thanks for your helpfull tipps, but I'm writing software since about
30a, and I get paid for that since about 25a.

How long to you that?

If anyone else wants to enter this discussion, sorry, but I will not
answer any more emails here.

Alexander

2011-04-10 19:29:42

by Alexander Holler

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: Differentiate SheevaPlugs and DockStars on the basis of the memory size.

On 10.04.2011 20:14, Alexander Holler wrote:
> Hello,
>
> On 09.04.2011 10:29, Bernd Petrovitsch wrote:
>
> Thanks for your helpfull tipps, but I'm writing software since about
> 30a, and I get paid for that since about 25a.

And I'm usingLinux only since I've switched to it from FreeBSD 2.2.

So if anyone else thinks he must send me helpfull insights about free,
open and maintained software, sorry, I've got enough hints in last days
and I will need at least some month to learn to follow them. So please
don't send me more of them. I promise too that I will not send any more
patches until I've learned to follow all the good sugestions.

Thanks to all,

Alexander