Default machine include placements

Default machine include placements

[Ben Dooks]

Currently in the s3c/s5p familt we're seeing quite a number of the
same things being repeated for items like entry-macro.S, hardware.h
and so on.

The first question is about adding include/mach directories to
eitehr plat-s5p or plat-samsung to mop up the files that keep
getting repeated (since the plat-* directories are processed after
the machine directory includes the mach-xxx are still free to overide
these as necessary)

The second question is whether some of these files should have defaults
in arch/arm/include? I think this might be less useful as build failures
for new ports ensure that at-least these files are thought about

-- 
Ben

Q:      What's a light-year?
A:      One-third less calories than a regular year.

Default machine include placements

[Russell King - ARM Linux]

On Mon, Jan 25, 2010 at 04:02:56AM +0000, Ben Dooks wrote:
> Currently in the s3c/s5p familt we're seeing quite a number of the
> same things being repeated for items like entry-macro.S, hardware.h
> and so on.
> 
> The first question is about adding include/mach directories to
> eitehr plat-s5p or plat-samsung to mop up the files that keep
> getting repeated (since the plat-* directories are processed after
> the machine directory includes the mach-xxx are still free to overide
> these as necessary)
> 
> The second question is whether some of these files should have defaults
> in arch/arm/include? I think this might be less useful as build failures
> for new ports ensure that at-least these files are thought about

No - doing this means it's harder to find out what's going on.  Rather
than being able to look in arch/arm/mach-*/include for the relevant
mach header file and know you've got the right one, you have to instead
consider whether the one you're using is the one found in
arch/arm/mach-*/include/mach, arch/arm/plat-*/include/mach or
arch/arm/include/mach.

Having multiple places where include files can live is a nightmare; you
only have to look at glibc to know that - where you have to search the
entire source looking for the header file you want.

Default machine include placements

[Ben Dooks]

On Mon, Jan 25, 2010 at 10:05:47AM +0000, Russell King - ARM Linux wrote:
> On Mon, Jan 25, 2010 at 04:02:56AM +0000, Ben Dooks wrote:
> > Currently in the s3c/s5p familt we're seeing quite a number of the
> > same things being repeated for items like entry-macro.S, hardware.h
> > and so on.
> > 
> > The first question is about adding include/mach directories to
> > eitehr plat-s5p or plat-samsung to mop up the files that keep
> > getting repeated (since the plat-* directories are processed after
> > the machine directory includes the mach-xxx are still free to overide
> > these as necessary)
> > 
> > The second question is whether some of these files should have defaults
> > in arch/arm/include? I think this might be less useful as build failures
> > for new ports ensure that at-least these files are thought about
> 
> No - doing this means it's harder to find out what's going on.  Rather
> than being able to look in arch/arm/mach-*/include for the relevant
> mach header file and know you've got the right one, you have to instead
> consider whether the one you're using is the one found in
> arch/arm/mach-*/include/mach, arch/arm/plat-*/include/mach or
> arch/arm/include/mach.
> 
> Having multiple places where include files can live is a nightmare; you
> only have to look at glibc to know that - where you have to search the
> entire source looking for the header file you want.

Ok, but this is going to make it really interesting once we sort out
multiple machine building on the s5p series like the s3c24xx series
currently has. Is there any suggestion about what to do then when
we may end up including many arch/arm/mach-*/include/mach?

-- 
Ben

Q:      What's a light-year?
A:      One-third less calories than a regular year.

Default machine include placements

[Daniel Silverstone]

On Mon, Jan 25, 2010 at 10:05:47AM +0000, Russell King - ARM Linux wrote:
> > The first question is about adding include/mach directories to
> > eitehr plat-s5p or plat-samsung to mop up the files that keep
> > getting repeated (since the plat-* directories are processed after
> > the machine directory includes the mach-xxx are still free to overide
> > these as necessary)
> > 
> > The second question is whether some of these files should have defaults
> > in arch/arm/include? I think this might be less useful as build failures
> > for new ports ensure that at-least these files are thought about
> 
> No - doing this means it's harder to find out what's going on.  Rather
> than being able to look in arch/arm/mach-*/include for the relevant
> mach header file and know you've got the right one, you have to instead
> consider whether the one you're using is the one found in
> arch/arm/mach-*/include/mach, arch/arm/plat-*/include/mach or
> arch/arm/include/mach.
> 
> Having multiple places where include files can live is a nightmare; you
> only have to look at glibc to know that - where you have to search the
> entire source looking for the header file you want.

As a suggestion; how about one of symbolic links; or headers of the form:

/* arch/arm/mach-s5pc100/include/mach/foo.h
 *
 */

#include <plat-samsung/include/mach/generic-foo.h>

?

D.

-- 
Daniel Silverstone                              http://www.simtec.co.uk/

Default machine include placements

[Ben Dooks]

On Mon, Jan 25, 2010 at 10:32:16AM +0000, Daniel Silverstone wrote:
> On Mon, Jan 25, 2010 at 10:05:47AM +0000, Russell King - ARM Linux wrote:
> > > The first question is about adding include/mach directories to
> > > eitehr plat-s5p or plat-samsung to mop up the files that keep
> > > getting repeated (since the plat-* directories are processed after
> > > the machine directory includes the mach-xxx are still free to overide
> > > these as necessary)
> > > 
> > > The second question is whether some of these files should have defaults
> > > in arch/arm/include? I think this might be less useful as build failures
> > > for new ports ensure that at-least these files are thought about
> > 
> > No - doing this means it's harder to find out what's going on.  Rather
> > than being able to look in arch/arm/mach-*/include for the relevant
> > mach header file and know you've got the right one, you have to instead
> > consider whether the one you're using is the one found in
> > arch/arm/mach-*/include/mach, arch/arm/plat-*/include/mach or
> > arch/arm/include/mach.
> > 
> > Having multiple places where include files can live is a nightmare; you
> > only have to look at glibc to know that - where you have to search the
> > entire source looking for the header file you want.
> 
> As a suggestion; how about one of symbolic links; or headers of the form:
> 
> /* arch/arm/mach-s5pc100/include/mach/foo.h
>  *
>  */
> 
> #include <plat-samsung/include/mach/generic-foo.h>

We do a bit of #include <plat/xxx-base.h> already for some things.

The 'empty' headers always seme to end up catching more copyright
statement than the space they end up saving.

Not sure how git deals with symlinks, and I think this will just leave
us a mess on systems that don't really understand symlinks.

-- 
Ben

Q:      What's a light-year?
A:      One-third less calories than a regular year.

Default machine include placements

[Daniel Silverstone]

On Mon, Jan 25, 2010 at 10:44:25AM +0000, Ben Dooks wrote:
> We do a bit of #include <plat/xxx-base.h> already for some things.
> The 'empty' headers always seme to end up catching more copyright
> statement than the space they end up saving.

I guess it depends if you're aiming for space savings or "only one place to fix
a bug" in that case.

> Not sure how git deals with symlinks, and I think this will just leave
> us a mess on systems that don't really understand symlinks.

Does the kernel even build on systems which don't properly support symbolic
links?  I thought we had more problems with caseless filesystems anyway?

Regards,

Daniel.

-- 
Daniel Silverstone                              http://www.simtec.co.uk/

Default machine include placements

[Ben Dooks]

On Mon, Jan 25, 2010 at 10:49:59AM +0000, Daniel Silverstone wrote:
> On Mon, Jan 25, 2010 at 10:44:25AM +0000, Ben Dooks wrote:
> > We do a bit of #include <plat/xxx-base.h> already for some things.
> > The 'empty' headers always seme to end up catching more copyright
> > statement than the space they end up saving.
> 
> I guess it depends if you're aiming for space savings or "only one place to fix
> a bug" in that case.

The big one is bug repetition, which happened with the s3c64xx clksrc
code. The internal Samsung trees ended up with 4 similar copies of this
code, each with its own unique problems. Whilst this problem is quite
easy to fix (and it is now fixed in the current kernel) the other problem
is the size of submission for each new CPU support.

One solution is just to make mach-s5p and stick all s5p implementations
in there, but this could end up with one quite big directory or a large
number of sub directiories springing up for CPU specific support.
 
-- 
Ben

Q:      What's a light-year?
A:      One-third less calories than a regular year.

Default machine include placements

[Russell King - ARM Linux]

On Mon, Jan 25, 2010 at 10:55:03AM +0000, Ben Dooks wrote:
> On Mon, Jan 25, 2010 at 10:49:59AM +0000, Daniel Silverstone wrote:
> > On Mon, Jan 25, 2010 at 10:44:25AM +0000, Ben Dooks wrote:
> > > We do a bit of #include <plat/xxx-base.h> already for some things.
> > > The 'empty' headers always seme to end up catching more copyright
> > > statement than the space they end up saving.
> > 
> > I guess it depends if you're aiming for space savings or "only one place to fix
> > a bug" in that case.
> 
> The big one is bug repetition, which happened with the s3c64xx clksrc
> code. The internal Samsung trees ended up with 4 similar copies of this
> code, each with its own unique problems. Whilst this problem is quite
> easy to fix (and it is now fixed in the current kernel) the other problem
> is the size of submission for each new CPU support.

You're talking about .c files here, not header files.

> One solution is just to make mach-s5p and stick all s5p implementations
> in there, but this could end up with one quite big directory or a large
> number of sub directiories springing up for CPU specific support.

CPU or SoC?  Please don't confuse the two terms - they're entirely
different things.  There's no need what so ever to separate out CPU
specific support because the kernel already does that for you.

SoC specific is what I think you mean.

I personally think that the proliferation of mach-s* directories for
Samsung is becoming a problem in itself - some of these directories
contain next to nothing, and others are duplications with minimal
changes.

For example, the differences between s3c6400 and s3c6410 are minimal,
yet they have most of the contained non-board code duplicated.  Also,
the mach-s3c2442 directory seems to only support one board - GTA02,
and mach-s3c2443 seems to be a similar story.

You're worried about a mach-*/ directory becoming too big, but you
don't seem to have noticed that the arch/arm directory has 15 (!)
Samsung directories.

Default machine include placements

[Ben Dooks]

On Mon, Jan 25, 2010 at 11:19:40AM +0000, Russell King - ARM Linux wrote:
> On Mon, Jan 25, 2010 at 10:55:03AM +0000, Ben Dooks wrote:
> > On Mon, Jan 25, 2010 at 10:49:59AM +0000, Daniel Silverstone wrote:
> > > On Mon, Jan 25, 2010 at 10:44:25AM +0000, Ben Dooks wrote:
> > > > We do a bit of #include <plat/xxx-base.h> already for some things.
> > > > The 'empty' headers always seme to end up catching more copyright
> > > > statement than the space they end up saving.
> > > 
> > > I guess it depends if you're aiming for space savings or "only one place to fix
> > > a bug" in that case.
> > 
> > The big one is bug repetition, which happened with the s3c64xx clksrc
> > code. The internal Samsung trees ended up with 4 similar copies of this
> > code, each with its own unique problems. Whilst this problem is quite
> > easy to fix (and it is now fixed in the current kernel) the other problem
> > is the size of submission for each new CPU support.
> 
> You're talking about .c files here, not header files.

Sorry, I'm trying to apply the same principles to all files being
created during the development process.

> > One solution is just to make mach-s5p and stick all s5p implementations
> > in there, but this could end up with one quite big directory or a large
> > number of sub directiories springing up for CPU specific support.
> 
> CPU or SoC?  Please don't confuse the two terms - they're entirely
> different things.  There's no need what so ever to separate out CPU
> specific support because the kernel already does that for you.

Sorry, it has been the end of a long day for me. SoCs are the problem
here and it is unfortuante whilst the core of the Samsung support is
pretty similar, each SoC or couple of SoCs is not the same as their
neigbours.
 
> SoC specific is what I think you mean.
> 
> I personally think that the proliferation of mach-s* directories for
> Samsung is becoming a problem in itself - some of these directories
> contain next to nothing, and others are duplications with minimal
> changes.
> 
> For example, the differences between s3c6400 and s3c6410 are minimal,
> yet they have most of the contained non-board code duplicated.

I would disagree on that, most of the common s3c6400 and s3c6410 is
contained in plat-s3c64xx with the machine directories containing the
cpu sepcific initialisation bits, such as the changes in the ARM CLKDIV
the VIC popuilation and the extra devices that the s3c6410 have.

It does however mean that mach-s3c6400 and mach-s3c6410 are not heavily
populated with machines, especially as the intended new Openmoko device
never appeared.

>  Also,
> the mach-s3c2442 directory seems to only support one board - GTA02,
> and mach-s3c2443 seems to be a similar story.

Unfortunately there seems to be very few people interested in actually
submitting s3c2443 machines. The s3c2442 ended up in a pile of iPAQs
but again there doesn't seem to be a lot of interest in getting anything
submitted (I don't belive that S3C2442 actually made it into anything
other than the large OEM chanell and thus hasn't been seen in anything
other than iPAQ type devices - although I could be wrong)
 
> You're worried about a mach-*/ directory becoming too big, but you
> don't seem to have noticed that the arch/arm directory has 15 (!)
> Samsung directories.

If someone would like to suggest alternate layouts, then please let
me know what people think. It was hoped that there would be many
people that would be submitting machines for these SoCs and that
each directory would overflow with support. This is unfrortunately
not the case. 

-- 
Ben

Q:      What's a light-year?
A:      One-third less calories than a regular year.

Default machine include placements

[Russell King - ARM Linux]

On Mon, Jan 25, 2010 at 11:53:44AM +0000, Ben Dooks wrote:
> > I personally think that the proliferation of mach-s* directories for
> > Samsung is becoming a problem in itself - some of these directories
> > contain next to nothing, and others are duplications with minimal
> > changes.
> > 
> > For example, the differences between s3c6400 and s3c6410 are minimal,
> > yet they have most of the contained non-board code duplicated.
> 
> I would disagree on that, most of the common s3c6400 and s3c6410 is
> contained in plat-s3c64xx with the machine directories containing the
> cpu sepcific initialisation bits, such as the changes in the ARM CLKDIV
> the VIC popuilation and the extra devices that the s3c6410 have.

You may disagree, but my comment was based upon diffing the two
directories, and comparing mach-s3c6400/s3c6400.c with mach-s3c6410/cpu.c:

First, comparing the common files between the two directories:

-char *s3c6400_hsmmc_clksrcs[4] = {
+char *s3c6410_hsmmc_clksrcs[4] = {
...
-void s3c6400_setup_sdhci_cfg_card(struct platform_device *dev,
-                                 void __iomem *r,
-                                 struct mmc_ios *ios,
-                                 struct mmc_card *card)
+
+void s3c6410_setup_sdhci0_cfg_card(struct platform_device *dev,
+                                   void __iomem *r,
+                                   struct mmc_ios *ios,
+                                   struct mmc_card *card)
...
+       /* don't need to alter anything acording to card-type */
+
+       writel(S3C64XX_SDHCI_CONTROL4_DRIVE_9mA, r + S3C64XX_SDHCI_CONTROL4);
+

And s3c6400.c vs cpu.c, taking account of the different prefixes used
for variables and function names (so s3c6410 -> s3c6400):

+#include <plat/s3c6400.h>
+
+/* Initial IO mappings */
+
+static struct map_desc s3c6400_iodesc[] __initdata = {
+};
+
+/* s3c6400_map_io
+ *
+ * register the standard cpu IO areas
+*/
...
-       /* setup SDHCI */
+       iotable_init(s3c6400_iodesc, ARRAY_SIZE(s3c6400_iodesc));

+       /* initialise device information early */
...
+       s3c_i2c1_setname("s3c2440-i2c");
...
-       s3c6400_register_clocks(S3C6400_CLKDIV0_ARM_MASK);
+       s3c6400_register_clocks(S3C6410_CLKDIV0_ARM_MASK);
...
-       /* VIC0 does not have IRQS 5..7,
-        * VIC1 is fully populated. */
-       s3c64xx_init_irq(~0 & ~(0xf << 5), ~0);
+       /* VIC0 is missing IRQ7, VIC1 is fully populated. */
+       s3c64xx_init_irq(~0 & ~(1 << 7), ~0);
...
-       printk("S3C6400: Initialising architecture\n");
+       printk("S3C6410: Initialising architecture\n");

Default machine include placements

[Ben Dooks]

On Mon, Jan 25, 2010 at 12:04:54PM +0000, Russell King - ARM Linux wrote:
> On Mon, Jan 25, 2010 at 11:53:44AM +0000, Ben Dooks wrote:
> > > I personally think that the proliferation of mach-s* directories for
> > > Samsung is becoming a problem in itself - some of these directories
> > > contain next to nothing, and others are duplications with minimal
> > > changes.
> > > 
> > > For example, the differences between s3c6400 and s3c6410 are minimal,
> > > yet they have most of the contained non-board code duplicated.
> > 
> > I would disagree on that, most of the common s3c6400 and s3c6410 is
> > contained in plat-s3c64xx with the machine directories containing the
> > cpu sepcific initialisation bits, such as the changes in the ARM CLKDIV
> > the VIC popuilation and the extra devices that the s3c6410 have.
> 
> You may disagree, but my comment was based upon diffing the two
> directories, and comparing mach-s3c6400/s3c6400.c with mach-s3c6410/cpu.c:
> 
> First, comparing the common files between the two directories:
> 
> -char *s3c6400_hsmmc_clksrcs[4] = {
> +char *s3c6410_hsmmc_clksrcs[4] = {
> ...
> -void s3c6400_setup_sdhci_cfg_card(struct platform_device *dev,
> -                                 void __iomem *r,
> -                                 struct mmc_ios *ios,
> -                                 struct mmc_card *card)
> +
> +void s3c6410_setup_sdhci0_cfg_card(struct platform_device *dev,
> +                                   void __iomem *r,
> +                                   struct mmc_ios *ios,
> +                                   struct mmc_card *card)
> ...
> +       /* don't need to alter anything acording to card-type */
> +
> +       writel(S3C64XX_SDHCI_CONTROL4_DRIVE_9mA, r + S3C64XX_SDHCI_CONTROL4);
> +
> 
> And s3c6400.c vs cpu.c, taking account of the different prefixes used
> for variables and function names (so s3c6410 -> s3c6400):
> 
> +#include <plat/s3c6400.h>
> +
> +/* Initial IO mappings */
> +
> +static struct map_desc s3c6400_iodesc[] __initdata = {
> +};
> +
> +/* s3c6400_map_io
> + *
> + * register the standard cpu IO areas
> +*/
> ...
> -       /* setup SDHCI */
> +       iotable_init(s3c6400_iodesc, ARRAY_SIZE(s3c6400_iodesc));

we should have probably culled the cpiu specific iotables
 
> +       /* initialise device information early */
> ...
> +       s3c_i2c1_setname("s3c2440-i2c");
> ...
> -       s3c6400_register_clocks(S3C6400_CLKDIV0_ARM_MASK);
> +       s3c6400_register_clocks(S3C6410_CLKDIV0_ARM_MASK);

yeah, the s3c6410 armdiv clock divider is larger.

> ...
> -       /* VIC0 does not have IRQS 5..7,
> -        * VIC1 is fully populated. */
> -       s3c64xx_init_irq(~0 & ~(0xf << 5), ~0);
> +       /* VIC0 is missing IRQ7, VIC1 is fully populated. */
> +       s3c64xx_init_irq(~0 & ~(1 << 7), ~0);
> ...
> -       printk("S3C6400: Initialising architecture\n");
> +       printk("S3C6410: Initialising architecture\n");
> 
> >From all that, the only material differences are:
> 
> 1. the VIC initialization
> 2. the sdhci drive strength
> 3. the presence of a second i2c port
> 4. the CLKDIV0_ARM_MASK
> 
> The rest of the non-board support code is identical in those two
> directories.
> 
> > It does however mean that mach-s3c6400 and mach-s3c6410 are not heavily
> > populated with machines, especially as the intended new Openmoko device
> > never appeared.
> 
> Given the above, and the "mass population" hasn't happened, maybe part
> of the solution to the duplicated header file problem is to start
> combining some of these directories?

For plat-s3c64xx/mach-s3c6400/mach-s3c6410 how about moving the
plat-s3c64xx in with the machine definitions, as it is quite clear
now that it is unlikely to have any friends to join it (dev work is
on s5p and onwards).

Either arch/arm/mach-s3c64xx and have all s3c6400 and s3c6410 machines
in there, or keep the two mach- directories... I think i'd prefer to
see the two directories culled into one.

For the s3c24xx case, it probably is the best case to merges s3c2440 and
s3c2442 into mach-s3c244x as they are pretty similar.

s3c2443 is on list of things to expand, it is a different beast to the
predecessors.

I'll try making patches for this tomorrow and see what people think?

-- 
Ben

Q:      What's a light-year?
A:      One-third less calories than a regular year.

Default machine include placements

[Russell King - ARM Linux]

On Mon, Jan 25, 2010 at 01:54:45PM +0000, Ben Dooks wrote:
> On Mon, Jan 25, 2010 at 12:04:54PM +0000, Russell King - ARM Linux wrote:
> > Given the above, and the "mass population" hasn't happened, maybe part
> > of the solution to the duplicated header file problem is to start
> > combining some of these directories?
> 
> For plat-s3c64xx/mach-s3c6400/mach-s3c6410 how about moving the
> plat-s3c64xx in with the machine definitions, as it is quite clear
> now that it is unlikely to have any friends to join it (dev work is
> on s5p and onwards).
> 
> Either arch/arm/mach-s3c64xx and have all s3c6400 and s3c6410 machines
> in there, or keep the two mach- directories... I think i'd prefer to
> see the two directories culled into one.

I'd prefer that too.

> For the s3c24xx case, it probably is the best case to merges s3c2440 and
> s3c2442 into mach-s3c244x as they are pretty similar.
> 
> s3c2443 is on list of things to expand, it is a different beast to the
> predecessors.

Sounds like a reason to keep it separate for the time being.

> I'll try making patches for this tomorrow and see what people think?

I'm not sure where mach-s3c24a0 fits in to this stuff - the only thing
it seems to contain is a load of include files.

With these changes, it's going to take it from 10 mach-s[35]* directories
down to 8.  Not sure if it makes much material difference to mach/*.h
includes though.

Default machine include placements

[Ben Dooks]

On Mon, Jan 25, 2010 at 02:59:35PM +0000, Russell King - ARM Linux wrote:
> On Mon, Jan 25, 2010 at 01:54:45PM +0000, Ben Dooks wrote:
> > On Mon, Jan 25, 2010 at 12:04:54PM +0000, Russell King - ARM Linux wrote:
> > > Given the above, and the "mass population" hasn't happened, maybe part
> > > of the solution to the duplicated header file problem is to start
> > > combining some of these directories?
> > 
> > For plat-s3c64xx/mach-s3c6400/mach-s3c6410 how about moving the
> > plat-s3c64xx in with the machine definitions, as it is quite clear
> > now that it is unlikely to have any friends to join it (dev work is
> > on s5p and onwards).
> > 
> > Either arch/arm/mach-s3c64xx and have all s3c6400 and s3c6410 machines
> > in there, or keep the two mach- directories... I think i'd prefer to
> > see the two directories culled into one.
> 
> I'd prefer that too.

Ok, thanks.
 
> > For the s3c24xx case, it probably is the best case to merges s3c2440 and
> > s3c2442 into mach-s3c244x as they are pretty similar.
> > 
> > s3c2443 is on list of things to expand, it is a different beast to the
> > predecessors.
> 
> Sounds like a reason to keep it separate for the time being.
> 
> > I'll try making patches for this tomorrow and see what people think?
> 
> I'm not sure where mach-s3c24a0 fits in to this stuff - the only thing
> it seems to contain is a load of include files.
> 
> With these changes, it's going to take it from 10 mach-s[35]* directories
> down to 8.  Not sure if it makes much material difference to mach/*.h
> includes though.

Should be an improvement, we've already got V210 and C110 into the
same dir (really just packaging options) and I'll look at culling
the C100 and plat-c11x as this line looks like it ended with the
C100 and the newer devices are differrent beasts.
 
-- 
Ben

Q:      What's a light-year?
A:      One-third less calories than a regular year.

Default machine include placements

[Ben Dooks]

On Mon, Jan 25, 2010 at 12:04:54PM +0000, Russell King - ARM Linux wrote:
> On Mon, Jan 25, 2010 at 11:53:44AM +0000, Ben Dooks wrote:
> > > I personally think that the proliferation of mach-s* directories for
> > > Samsung is becoming a problem in itself - some of these directories
> > > contain next to nothing, and others are duplications with minimal
> > > changes.
> > > 
> > > For example, the differences between s3c6400 and s3c6410 are minimal,
> > > yet they have most of the contained non-board code duplicated.
> > 
> > I would disagree on that, most of the common s3c6400 and s3c6410 is
> > contained in plat-s3c64xx with the machine directories containing the
> > cpu sepcific initialisation bits, such as the changes in the ARM CLKDIV
> > the VIC popuilation and the extra devices that the s3c6410 have.
> 
> You may disagree, but my comment was based upon diffing the two
> directories, and comparing mach-s3c6400/s3c6400.c with mach-s3c6410/cpu.c:

Sorry, I did think that I had tried hard to eliminate the common code
from the two SoCs... looking at it after the event it seems that there
is still a bit of work to do.
 
This unfrotunately doesn't really solve the solution fully for the
plat-s5p case just yet, as these SoCs may be similar but have a few
problems with respect to the peripheral placement and some of the
more interestesting configurations.

It may be we just accept the compromise of repeated header files and
deal with this once we've sorted out exactly what we want to do.

-- 
Ben

Q:      What's a light-year?
A:      One-third less calories than a regular year.

Default machine include placements

[Mark Brown]

On Mon, Jan 25, 2010 at 10:44:25AM +0000, Ben Dooks wrote:

> Not sure how git deals with symlinks, and I think this will just leave
> us a mess on systems that don't really understand symlinks.

Given that the kernel already includes a number of files distinguished
only by case I'm not sure that that is such a concern - the kernel is
already causing troubles for older filesystems anyway.

Default machine include placements

[Ben Dooks]

On Mon, Jan 25, 2010 at 04:02:56AM +0000, Ben Dooks wrote:
> Currently in the s3c/s5p familt we're seeing quite a number of the
> same things being repeated for items like entry-macro.S, hardware.h
> and so on.
> 
> The first question is about adding include/mach directories to
> eitehr plat-s5p or plat-samsung to mop up the files that keep
> getting repeated (since the plat-* directories are processed after
> the machine directory includes the mach-xxx are still free to overide
> these as necessary)

As context, this is part of the work that I am currently doing with
Samsung to improve the mainlining of the newer SoC series. Part of this
is to try and cut down on the amount of repetition that gets done by
some of the porting teams. This is generally to stop things like the
replication of the serial drivers or ending up with complete new copies
of the SDHCI core simply because they could.

-- 
Ben

Q:      What's a light-year?
A:      One-third less calories than a regular year.

Default machine include placements

[Russell King - ARM Linux]

On Mon, Jan 25, 2010 at 10:49:56AM +0000, Ben Dooks wrote:
> On Mon, Jan 25, 2010 at 04:02:56AM +0000, Ben Dooks wrote:
> > Currently in the s3c/s5p familt we're seeing quite a number of the
> > same things being repeated for items like entry-macro.S, hardware.h
> > and so on.
> > 
> > The first question is about adding include/mach directories to
> > eitehr plat-s5p or plat-samsung to mop up the files that keep
> > getting repeated (since the plat-* directories are processed after
> > the machine directory includes the mach-xxx are still free to overide
> > these as necessary)
> 
> As context, this is part of the work that I am currently doing with
> Samsung to improve the mainlining of the newer SoC series. Part of this
> is to try and cut down on the amount of repetition that gets done by
> some of the porting teams. This is generally to stop things like the
> replication of the serial drivers or ending up with complete new copies
> of the SDHCI core simply because they could.

Changing the policy wrt include files isn't going to stop drivers
and core code being copy-n-edited.

The copy-n-edit is a culture thing - it's something that people
needlessly do because that's how they've worked in the past and don't
know any better.  The solution to that is education, not changing the
way we layout files in the kernel tree.

Default machine include placements

[Ben Dooks]

On Mon, Jan 25, 2010 at 11:01:16AM +0000, Russell King - ARM Linux wrote:
> On Mon, Jan 25, 2010 at 10:49:56AM +0000, Ben Dooks wrote:
> > On Mon, Jan 25, 2010 at 04:02:56AM +0000, Ben Dooks wrote:
> > > Currently in the s3c/s5p familt we're seeing quite a number of the
> > > same things being repeated for items like entry-macro.S, hardware.h
> > > and so on.
> > > 
> > > The first question is about adding include/mach directories to
> > > eitehr plat-s5p or plat-samsung to mop up the files that keep
> > > getting repeated (since the plat-* directories are processed after
> > > the machine directory includes the mach-xxx are still free to overide
> > > these as necessary)
> > 
> > As context, this is part of the work that I am currently doing with
> > Samsung to improve the mainlining of the newer SoC series. Part of this
> > is to try and cut down on the amount of repetition that gets done by
> > some of the porting teams. This is generally to stop things like the
> > replication of the serial drivers or ending up with complete new copies
> > of the SDHCI core simply because they could.
> 
> Changing the policy wrt include files isn't going to stop drivers
> and core code being copy-n-edited.
> 
> The copy-n-edit is a culture thing - it's something that people
> needlessly do because that's how they've worked in the past and don't
> know any better.  The solution to that is education, not changing the
> way we layout files in the kernel tree.

This is an unfortunate side-effect of the education process, having
looked ast te s5p6440 code that is already queued for merging and the
s5pv210 code that is currently in review, there are a number of header
files that are common, for example:

<mach/hardware.h> which is 18 lines long has three lines different:

linux at linux-laptop:~/bjd.git$ diff -u arch/arm/mach-s5pv210/include/mach/hardware.h  arch/arm/mach-s5p6440/include/mach/hardware.h 
--- arch/arm/mach-s5pv210/include/mach/hardware.h       2010-01-25 20:31:01.000000000 +0900
+++ arch/arm/mach-s5p6440/include/mach/hardware.h       2010-01-22 10:50:22.000000000 +0900
@@ -1,9 +1,9 @@
-/* linux/arch/arm/mach-s5pv210/include/mach/hardware.h
+/* linux/arch/arm/mach-s5p6440/include/mach/hardware.h
  *
- * Copyright (c) 2010 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2009 Samsung Electronics Co., Ltd.
  *             http://www.samsung.com/
  *
- * S5PV210 - Hardware support
+ * S5P6440 - Hardware support
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as

The same is with <mach/system.h> where for 26 lines and have the same
three differences due to copyright date and placemeent.

With the map files, it isn't so bad as each chip does have differences
in the phyiscal addresses of hte peripheral blocks (yay) and thus we
can easily abstract the same data into a platform include with a unique
name.

As long as we have a clear policy then it makes it easier, my main worry
is that I've spent quite a lot of time trying to ensure that things are
not being duplicated and then have to turn around and say 'but not here'
and then end up having to try and ensure that the whole process does not
end up failing.

As a last comment, is a header file such as <mach/entry-macro.S> allowed
to be simply a copyright statement followed by including a platform
include file, as so:

/*
 * arch/arm/mach-foo/include/mach/entry-macro.S
 *
 * Copyright 2010 FooCorp
 *
 * Licensed under GPLv25B
*/

#include <plat/entry-macro-common-foocorp.S>

-- 
Ben

Q:      What's a light-year?
A:      One-third less calories than a regular year.