[U-Boot] Best way of making some drivers common across kirkwood and orion5x SoCs?
Hi,
I am working on adding mainline support for Orion5x family of Marvell SoCs in U-boot, based on the Kirkwood support already present.
I believe these are different enough families to justify adding an 'orion5x' SoC along the existing 'kirkwood' one.
Several kirkwood drivers could actually be reused almost as-is and should thus be shared between both SoC families. For instance, kirkwood_egiga.c and ehci-kirkwood.c would only differ by the number of ports and kirkwood_i2c.c could be reused as-is.
However, these drivers have hard-coded numbers of ports, and 'hard' references to 'kirkwood.h' for their register definitions. Making them cross-SoC would require moving the numbers of ports to some SoC-specific header file, and that this header file *not* be named after a specific SoC.
I've searched for a layer between CPU and Core where cross-SoC code could fit, but I haven't seen one, and I don't think one would be needed.
Instead, I have thought of replacing the #include "kirkwood.h" with a #include "soc.h", where soc.h would exist in both SoC's header files include/asm-arm/arch-kirkwood and include/asm-arm/arch-orion5x. This soc.h file would either include the specific soc header file (kirkwood.h or orion5x.h) or, better yet, be a symlink to it, or better again, replace it.
This could be done in a two-step approach, each step being one commit.
1) introduce "soc.h" by:
- 1a) renaming, symlinking or including kirkwood.h into soc.h and changing kirkwood drivers that #include "kirkwood.h" accordingly;
- 1b) turning all kirkwood-specific namings in these kirkwood drivers into marvell-non-soc-specific ones (remove "KW_" prefixes and such).
(steps 1a and 1b are independent)
2) add orion5x support with its own "soc.h" file.
Would this approach be clean enough to be considered for inclusion in mainline?
Amicalement,
Dear Albert ARIBAUD,
In message 4AEBFF60.90902@free.fr you wrote:
Several kirkwood drivers could actually be reused almost as-is and should thus be shared between both SoC families. For instance, kirkwood_egiga.c and ehci-kirkwood.c would only differ by the number of ports and kirkwood_i2c.c could be reused as-is.
However, these drivers have hard-coded numbers of ports, and 'hard' references to 'kirkwood.h' for their register definitions. Making them cross-SoC would require moving the numbers of ports to some SoC-specific header file, and that this header file *not* be named after a specific SoC.
... or to include the right SoC-specific header(s) only.
Instead, I have thought of replacing the #include "kirkwood.h" with a #include "soc.h", where soc.h would exist in both SoC's header files
"soc.h" is eventually a too generic name.
include/asm-arm/arch-kirkwood and include/asm-arm/arch-orion5x. This soc.h file would either include the specific soc header file (kirkwood.h or orion5x.h) or, better yet, be a symlink to it, or better again, replace it.
Please don't use symlinks.
This could be done in a two-step approach, each step being one commit.
- introduce "soc.h" by:
- 1a) renaming, symlinking or including kirkwood.h into soc.h and
changing kirkwood drivers that #include "kirkwood.h" accordingly;
- 1b) turning all kirkwood-specific namings in these kirkwood drivers
into marvell-non-soc-specific ones (remove "KW_" prefixes and such).
(steps 1a and 1b are independent)
We do things like
#if defined(CONFIG_8xx) #include <asm/8xx_immap.h> ... #elif defined(CONFIG_5xx) #include <asm/5xx_immap.h> #elif defined(CONFIG_MPC5xxx) #include <mpc5xxx.h> #elif defined(CONFIG_MPC512X) #include <asm/immap_512x.h> #elif defined(CONFIG_MPC8220) #include <asm/immap_8220.h> ...
even in pretty central files like "include/common.h". I don't want to suggest that this is the most beautiful solution, though.
- add orion5x support with its own "soc.h" file.
Would this approach be clean enough to be considered for inclusion in mainline?
Yes, except for the "soc.h" name. And no links, please.
Best regards,
Wolfgang Denk
Wolfgang Denk a écrit :
"soc.h" is eventually a too generic name.
I'd actually chosen it "as generic as" the 'cpu.h' file which already exists in the kirkwood arch (and others), but making it more specific is ok, as long as it is not as specific as 'kirkwood' -- see below for an alternate proposal.
Please don't use symlinks.
I've already started with replacing the file rather than symlinking, so that's fine with me.
Would this approach be clean enough to be considered for inclusion
in mainline?
Yes, except for the "soc.h" name. And no links, please.
Would a name such as "marvell.h" be more acceptable? This would be consistent with my intent to replace kirkwood references with marvell ones in all non-kirkwood-specific symbols, e.g. kikwood_egiga_initialize would become marvell_egiga_initialize etc.
Amicalement,
Dear Albert ARIBAUD,
In message 4AEC53DB.6050509@free.fr you wrote:
Would a name such as "marvell.h" be more acceptable? This would be
Hm... I expect there will be little chance to see MAarvell IP blocks in non-Marvell SoCs, so this should work indeed. On the other hand, why not chose a name that works for other CPUs / SoCs as well. [So on second thought maybe your "soc.h" was not that bad a name.]
Let's see what others suggest.
consistent with my intent to replace kirkwood references with marvell ones in all non-kirkwood-specific symbols, e.g. kikwood_egiga_initialize would become marvell_egiga_initialize etc.
Do we need this "kirkwood_" or "marvell_" prefix at all? Where e4lse do we expect to see for example "egiga_" show up?
We do not use "intel_e1000_..." or "realtek_rtl8139_..." etc. either,
Just drop that part. Nobody will miss it.
Best regards,
Wolfgang Denk
Wolfgang Denk a écrit :
Dear Albert ARIBAUD,
In message 4AEC53DB.6050509@free.fr you wrote:
Would a name such as "marvell.h" be more acceptable? This would be
Hm... I expect there will be little chance to see MAarvell IP blocks in non-Marvell SoCs, so this should work indeed. On the other hand, why not chose a name that works for other CPUs / SoCs as well. [So on second thought maybe your "soc.h" was not that bad a name.]
Let's see what others suggest.
While we wait for others' opinion, one quick question: do you prefer me to submit this rework of the Marvell IP/drivers as individual patches, one for each IP/driver, or as a single patch for the whole rework?
Amicalement,
Dear Albert
-----Original Message----- From: u-boot-bounces@lists.denx.de [mailto:u-boot-bounces@lists.denx.de] On Behalf Of Albert ARIBAUD Sent: Sunday, November 01, 2009 6:30 AM To: u-boot@lists.denx.de Subject: Re: [U-Boot] Best way of making some drivers common across kirkwood and orion5x SoCs?
Wolfgang Denk a écrit :
Dear Albert ARIBAUD,
In message 4AEC53DB.6050509@free.fr you wrote:
Would a name such as "marvell.h" be more acceptable? This would be
Hm... I expect there will be little chance to see MAarvell IP blocks in non-Marvell SoCs, so this should work indeed. On the other hand, why not chose a name that works for other CPUs / SoCs as
well. [So on
second thought maybe your "soc.h" was not that bad a name.]
Let's see what others suggest.
I propose the following strategy for this- For orion- let's follow the standard development strategy used for ARM architectures including Kirkwood. Since orion and kirkwood has lot of similarities but when both these will evolve for there entire supports there will be problems resulting code complexity.
So My suggestion is- let's develop orion as separate ARM SOC family as kirkwood. Some of Kirkwood drivers can be directly used with Orion, that can be addressed through makefiles as we used NS16550 Serial support in Kirkwood. SOC specific configuration in the common drivers (for ex. Egiga) can always be addressed through individual <asm/arch/*.h>
While we wait for others' opinion, one quick question: do you prefer me to submit this rework of the Marvell IP/drivers as individual patches, one for each IP/driver, or as a single patch for the whole rework?
It is always better to break down the entire work in the small pieces, it's easier to digest them faster :-)
Regards.. Prafulla . .
Amicalement,
Albert.
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Prafulla Wadaskar a écrit :
Dear Albert
I propose the following strategy for this- For orion- let's follow the standard development strategy used for ARM architectures including Kirkwood. Since orion and kirkwood has lot of similarities but when both these will evolve for there entire supports there will be problems resulting code complexity.
So My suggestion is- let's develop orion as separate ARM SOC family as kirkwood.
This is what I intend to do, so that's fine with me.
Some of Kirkwood drivers can be directly used with Orion, that can be addressed through makefiles as we used NS16550 Serial support in Kirkwood. SOC specific configuration in the common drivers (for ex. Egiga) can always be addressed through individual <asm/arch/*.h>
Agreed on reusing the drivers; again, that is what I intend to do.
Thus the only issue left is that the shared drivers (egiga, ehci, I2C, gpio and, to a lesser extent, serial) #include the kirkwood.h file and use kirkwood-defined symbols (e.g. KW_UART0_BASE) which ties them to the kirkwood soc -- purely syntactically, as these symbols for the most part are not actually kirkwood-specific.
The #include part I can handle with something like
#if defined (CONFIG_KIRKWOOD) #include <asm/arch/kirkwood.h> #elif defined (CONFIG_ORION5X) #include <asm/arch/orion5x.h> #else #error egiga.c needs either CONFIG_KIRKWOOD or CONFIG_ORION5X. #endif
in each driver, so yes, I can live without the 'soc.h' file renaming.
Still, the drivers would be full of 'KWxxx" and "kwxxx" symbols of which many are not kirkwood-specific actually. In order for these drivers to compile with an orion5x SoC, I would have to adopt kirkwood names in the orion5x code, which I don't like as much as I would like fixing the symbols to make them marvell-, not kirkwood-, specific.
Note that such fixing is rather trivial (tens of lines in the source code, basically search and replace) and binary-invariant (I have patches for these already that I can post for review upon request).
It is always better to break down the entire work in the small pieces, it's easier to digest them faster :-)
Fine!
Amicalement,
-----Original Message----- From: Albert ARIBAUD [mailto:albert.aribaud@free.fr] Sent: Monday, November 02, 2009 3:34 PM To: Prafulla Wadaskar Cc: u-boot@lists.denx.de; Tom Subject: Re: [U-Boot] Best way of making some drivers common across kirkwood and orion5x SoCs?
Prafulla Wadaskar a écrit :
Dear Albert
I propose the following strategy for this- For orion- let's follow the standard development strategy used for ARM architectures including Kirkwood. Since orion and kirkwood has lot of similarities but when both these will evolve for there entire supports there will be problems resulting code complexity.
So My suggestion is- let's develop orion as separate ARM SOC family as kirkwood.
This is what I intend to do, so that's fine with me.
Some of Kirkwood drivers can be directly used with Orion,
that can be
addressed through makefiles as we used NS16550 Serial support in Kirkwood. SOC specific configuration in the common drivers (for ex. Egiga) can always be addressed through individual <asm/arch/*.h>
Agreed on reusing the drivers; again, that is what I intend to do.
Thus the only issue left is that the shared drivers (egiga, ehci, I2C, gpio and, to a lesser extent, serial) #include the kirkwood.h file and use kirkwood-defined symbols (e.g. KW_UART0_BASE) which ties them to the kirkwood soc -- purely syntactically, as these symbols for the most part are not actually kirkwood-specific.
The #include part I can handle with something like
#if defined (CONFIG_KIRKWOOD) #include <asm/arch/kirkwood.h> #elif defined (CONFIG_ORION5X) #include <asm/arch/orion5x.h> #else #error egiga.c needs either CONFIG_KIRKWOOD or CONFIG_ORION5X. #endif
This is correct approach.
in each driver, so yes, I can live without the 'soc.h' file renaming.
With above reference, you should be, because it will come from include/asm/arch-<soc>/ that will be different in both the cases.
Still, the drivers would be full of 'KWxxx" and "kwxxx" symbols of which many are not kirkwood-specific actually.
Any way, those are not even orion specific nor Marvell specific. Those are related to the functionality supported by SOCs that may be customized by each SOC
In order for these drivers to compile with an orion5x SoC, I would have to adopt kirkwood names in the
What harm in this?
orion5x code, which I don't like as much as I would like fixing the
If you see kernel code even there you can see kirkwood call from Orion drivers and vice versa.
symbols to make them marvell-, not kirkwood-, specific.
This will not solve the root problem, what about some non marvell SoC have same h/w and want to reuse the code? Do we again change the suffixes? (kirkwood re-used external serial driver adopting external definitions).
I suggest here to adopt kw symbols in Orion. This would make it clear for anybody that kirkwood code is reused by orion. With this kirkwood drives will be untouched.
While doing this reuse activity, if we find something blocking, certainly we should address that, but let's avoid changing identity of drivers.
Regards.. Prafulla . .
Note that such fixing is rather trivial (tens of lines in the source code, basically search and replace) and binary-invariant (I have patches for these already that I can post for review upon request).
It is always better to break down the entire work in the small pieces, it's easier to digest them faster :-)
Fine!
Amicalement,
Albert.
Prafulla Wadaskar a écrit :
Still, the drivers would be full of 'KWxxx" and "kwxxx" symbols of which many are not kirkwood-specific actually.
Any way, those are not even orion specific nor Marvell specific.
Those are related to the functionality supported by SOCs that may be
customized by each SOC
In order for these drivers to compile with an orion5x SoC, I would have to adopt kirkwood names in the
What harm in this?
I would say it harms maintenability and reuseability of the code. If those drivers are neither kirkwood- nor even marvell-specific, then they should not lead readers to believe they are, otherwise people might not realize they can ruse these drivers in their own SoCs.
orion5x code, which I don't like as much as I would like fixing the
If you see kernel code even there you can see kirkwood call from Orion drivers and vice versa.
symbols to make them marvell-, not kirkwood-, specific.
This will not solve the root problem, what about some non marvell SoC have same h/w and want to reuse the code? Do we again change the suffixes? (kirkwood re-used external serial driver adopting external definitions).
Point valid and taken--see my suggestion below.
I suggest here to adopt kw symbols in Orion. This would make it clear for anybody that kirkwood code is reused by orion. With this kirkwood drives will be untouched.
This does not solve the root problem any better than switching to Marvell prefixes, as you rightly point out. I thus suggest removing any Marvell, Kirkwood or Orion prefixes from symbols in these drivers altogether. For instance, egiga symbols would take EGIGA_ as a prefix. Then each SoC (kirkwood, orion5x, any other) header file would provide adequate definitions (#define EGIGA_xxxx yyyy).
While doing this reuse activity, if we find something blocking, certainly we should address that, but let's avoid changing identity of drivers.
I personally believe that having the Orion5x SoC dependent of the kirkwood one is blocking enough from a design standpoint.
Regards.. Prafulla . .
Amicalement,
Hi Albert
-----Original Message----- From: u-boot-bounces@lists.denx.de [mailto:u-boot-bounces@lists.denx.de] On Behalf Of Albert ARIBAUD Sent: Saturday, October 31, 2009 8:42 PM To: u-boot@lists.denx.de Subject: Re: [U-Boot] Best way of making some drivers common across kirkwood and orion5x SoCs?
Wolfgang Denk a écrit :
"soc.h" is eventually a too generic name.
I'd actually chosen it "as generic as" the 'cpu.h' file which already exists in the kirkwood arch (and others), but making it more specific is ok, as long as it is not as specific as 'kirkwood' -- see below for an alternate proposal.
Anything inside "Kirkwood" folder is Kirkwood specific, that applies for cpu.c/h. There is no sense to keep soc.h in SOC specific folders. If we are keeping it in Arch specific folder then it is applicable for all other SOCs too.
Please don't use symlinks.
I've already started with replacing the file rather than symlinking, so that's fine with me.
I fully ack with NOT to use symlinks
Would this approach be clean enough to be considered for
inclusion in mainline?
Yes, except for the "soc.h" name. And no links, please.
Would a name such as "marvell.h" be more acceptable? This would be consistent with my intent to replace kirkwood references with marvell ones in all non-kirkwood-specific symbols, e.g. kikwood_egiga_initialize would become marvell_egiga_initialize etc.
I disagree with this, - marvell.h is again more generic name applicable for all other Marvell IPs that May not be the same on other SOCs unless we call them with specific names. The name kirkwood_egiga.c/h clearly specifies it is for kirkwood. If it can be used as it is, well there are no issues. We can tune it to address Orion issues if any so that kirkwood/orion can be supported though clean abstraction. Otherwise I would always prefer to have separate driver (for ex. Orion_egiga.c/h).
Regards.. Prafulla . .
Amicalement,
Albert.
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Copying Tom I will replay latter
Regards.. Prafulla . .
-----Original Message----- From: Albert ARIBAUD [mailto:albert.aribaud@free.fr] Sent: Saturday, October 31, 2009 2:42 PM To: u-boot@lists.denx.de Cc: Jean-Christophe PLAGNIOL-VILLARD; Prafulla Wadaskar Subject: Best way of making some drivers common across kirkwood and orion5x SoCs?
Hi,
I am working on adding mainline support for Orion5x family of Marvell SoCs in U-boot, based on the Kirkwood support already present.
I believe these are different enough families to justify adding an 'orion5x' SoC along the existing 'kirkwood' one.
Several kirkwood drivers could actually be reused almost as-is and should thus be shared between both SoC families. For instance, kirkwood_egiga.c and ehci-kirkwood.c would only differ by the number of ports and kirkwood_i2c.c could be reused as-is.
However, these drivers have hard-coded numbers of ports, and 'hard' references to 'kirkwood.h' for their register definitions. Making them cross-SoC would require moving the numbers of ports to some SoC-specific header file, and that this header file *not* be named after a specific SoC.
I've searched for a layer between CPU and Core where cross-SoC code could fit, but I haven't seen one, and I don't think one would be needed.
Instead, I have thought of replacing the #include "kirkwood.h" with a #include "soc.h", where soc.h would exist in both SoC's header files include/asm-arm/arch-kirkwood and include/asm-arm/arch-orion5x. This soc.h file would either include the specific soc header file (kirkwood.h or orion5x.h) or, better yet, be a symlink to it, or better again, replace it.
This could be done in a two-step approach, each step being one commit.
- introduce "soc.h" by:
- 1a) renaming, symlinking or including kirkwood.h into soc.h and
changing kirkwood drivers that #include "kirkwood.h" accordingly;
- 1b) turning all kirkwood-specific namings in these kirkwood drivers
into marvell-non-soc-specific ones (remove "KW_" prefixes and such).
(steps 1a and 1b are independent)
- add orion5x support with its own "soc.h" file.
Would this approach be clean enough to be considered for inclusion in mainline?
Amicalement,
Albert.
participants (3)
-
Albert ARIBAUD -
Prafulla Wadaskar -
Wolfgang Denk