diff mbox

[v2,1/5] ARM: Broadcom: Unconditionally build arch/arm/mach-bcm

Message ID 20130726171153.GN29916@titan.lakedaemon.net (mailing list archive)
State New, archived
Headers show

Commit Message

Jason Cooper July 26, 2013, 5:11 p.m. UTC
Christian,

On Fri, Jul 26, 2013 at 08:55:42AM -0700, Christian Daudt wrote:
> [resend in plain text]
> On 13-07-26 08:29 AM, Jason Cooper wrote:
> >On Fri, Jul 26, 2013 at 04:56:40PM +0200, Domenico Andreoli wrote:
> >>From: Domenico Andreoli<domenico.andreoli@linux.com>
> >>
> >>arch/arm/mach-bcm contains a plurality of Broadcom SoCs, each configured
> >>separately. As a matter of flexibility and maintenance, it needs to be
> >>always included in the build.
> >
> >So if I'm building mach-kirkwood, I _have_ to build Broadcom?  What is
> >the *specific* problem you're encountering that this solves?
> 
>  No it won't, as the Makefile inside mach-bcm will only pull in
> files based on ARCH_ settings. This move is so that a number
> different Broadcom SoCs can co-exist inside the mach-bcm directory.


Why wouldn't this work?

---->8-------

Comments

Christian Daudt July 26, 2013, 5:17 p.m. UTC | #1
On 13-07-26 10:11 AM, Jason Cooper wrote:
> Christian,
>
> On Fri, Jul 26, 2013 at 08:55:42AM -0700, Christian Daudt wrote:
>> [resend in plain text]
>> On 13-07-26 08:29 AM, Jason Cooper wrote:
>>> On Fri, Jul 26, 2013 at 04:56:40PM +0200, Domenico Andreoli wrote:
>>>> From: Domenico Andreoli<domenico.andreoli@linux.com>
>>>>
>>>> arch/arm/mach-bcm contains a plurality of Broadcom SoCs, each configured
>>>> separately. As a matter of flexibility and maintenance, it needs to be
>>>> always included in the build.
>>> So if I'm building mach-kirkwood, I _have_ to build Broadcom?  What is
>>> the *specific* problem you're encountering that this solves?
>>   No it won't, as the Makefile inside mach-bcm will only pull in
>> files based on ARCH_ settings. This move is so that a number
>> different Broadcom SoCs can co-exist inside the mach-bcm directory.
>
> Why wouldn't this work?
>
> ---->8-------
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index ba412e0..97b6aff 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -366,6 +366,12 @@ config ARCH_AT91
>   	  This enables support for systems based on Atmel
>   	  AT91RM9200 and AT91SAM9* processors.
>   
> +config ARCH_BCM
> +	bool "Broadcom family SoCs"
> +	help
> +	  This enables support for systems based on the Broadcom
> +	  bcm4760 and bcm281XX series SoCs.
> +
>   config ARCH_CLPS711X
>   	bool "Cirrus Logic CLPS711x/EP721x/EP731x-based"
>   	select ARCH_REQUIRE_GPIOLIB
> diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig
> index f112895..4b1f9db 100644
> --- a/arch/arm/mach-bcm/Kconfig
> +++ b/arch/arm/mach-bcm/Kconfig
> @@ -1,5 +1,9 @@
> -config ARCH_BCM
> -	bool "Broadcom SoC" if ARCH_MULTI_V7
> +if ARCH_BCM
> +
> +menu "Broadcom SoC Implementations"
> +
> +config MACH_BCM281XX
> +	bool "BCM281XX SoCs" if ARCH_MULTI_V7
>   	depends on MMU
>   	select ARCH_REQUIRE_GPIOLIB
>   	select ARM_ERRATA_754322
> @@ -17,3 +21,7 @@ config ARCH_BCM
>   	  It currently supports the 'BCM281XX' family, which includes
>   	  BCM11130, BCM11140, BCM11351, BCM28145 and
>   	  BCM28155 variants.
> +
> +endmenu
> +
> +endif
> diff --git a/arch/arm/mach-bcm/Makefile b/arch/arm/mach-bcm/Makefile
> index 6adb6aec..e3f8f27 100644
> --- a/arch/arm/mach-bcm/Makefile
> +++ b/arch/arm/mach-bcm/Makefile
> @@ -10,6 +10,6 @@
>   # of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>   # GNU General Public License for more details.
>   
> -obj-$(CONFIG_ARCH_BCM)		:= board_bcm.o bcm_kona_smc.o bcm_kona_smc_asm.o
> +obj-$(CONFIG_MACH_BCM281XX)	:= board_bcm.o bcm_kona_smc.o bcm_kona_smc_asm.o
>   plus_sec := $(call as-instr,.arch_extension sec,+sec)
>   AFLAGS_bcm_kona_smc_asm.o	:=-Wa,-march=armv7-a$(plus_sec)
>
Because ARCH_BCM is meant to be used for a number of SoC families. We've 
started upstreaming one (the BCM281XX) but have 2 more internally that 
we're working towards upstreaming. And in the future our plan is to keep 
the Broadcom Mobile SoCs all building under this single ARCH_BCM 
configuration as multiplatform code building a single zImage for them.
The intent of mach-bcm on the other hand is to aggregate future SoC 
chips beyond ARCH_BCM (which is mobile SoC focused) to include other 
Broadcom ARM based SoCs. And there are 2 in the wings at the moment 
making their way into mainline as patches. The idea here being that with 
the new multiplatform code, the mach- dirs contain so little code that 
it makes sense to aggregate a bunch of SoCs from the same company in 
there (and we are the guinea pig on this one).

  Thanks,
    csd
Jason Cooper July 26, 2013, 5:33 p.m. UTC | #2
On Fri, Jul 26, 2013 at 10:17:33AM -0700, Christian Daudt wrote:
> On 13-07-26 10:11 AM, Jason Cooper wrote:
> >Christian,
> >
> >On Fri, Jul 26, 2013 at 08:55:42AM -0700, Christian Daudt wrote:
> >>[resend in plain text]
> >>On 13-07-26 08:29 AM, Jason Cooper wrote:
> >>>On Fri, Jul 26, 2013 at 04:56:40PM +0200, Domenico Andreoli wrote:
> >>>>From: Domenico Andreoli<domenico.andreoli@linux.com>
> >>>>
> >>>>arch/arm/mach-bcm contains a plurality of Broadcom SoCs, each configured
> >>>>separately. As a matter of flexibility and maintenance, it needs to be
> >>>>always included in the build.
> >>>So if I'm building mach-kirkwood, I _have_ to build Broadcom?  What is
> >>>the *specific* problem you're encountering that this solves?
> >>  No it won't, as the Makefile inside mach-bcm will only pull in
> >>files based on ARCH_ settings. This move is so that a number
> >>different Broadcom SoCs can co-exist inside the mach-bcm directory.
> >
> >Why wouldn't this work?
> >
> >---->8-------
> >diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> >index ba412e0..97b6aff 100644
> >--- a/arch/arm/Kconfig
> >+++ b/arch/arm/Kconfig
> >@@ -366,6 +366,12 @@ config ARCH_AT91
> >  	  This enables support for systems based on Atmel
> >  	  AT91RM9200 and AT91SAM9* processors.
> >+config ARCH_BCM
> >+	bool "Broadcom family SoCs"
> >+	help
> >+	  This enables support for systems based on the Broadcom
> >+	  bcm4760 and bcm281XX series SoCs.
> >+
> >  config ARCH_CLPS711X
> >  	bool "Cirrus Logic CLPS711x/EP721x/EP731x-based"
> >  	select ARCH_REQUIRE_GPIOLIB
> >diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig
> >index f112895..4b1f9db 100644
> >--- a/arch/arm/mach-bcm/Kconfig
> >+++ b/arch/arm/mach-bcm/Kconfig
> >@@ -1,5 +1,9 @@
> >-config ARCH_BCM
> >-	bool "Broadcom SoC" if ARCH_MULTI_V7
> >+if ARCH_BCM
> >+
> >+menu "Broadcom SoC Implementations"
> >+
> >+config MACH_BCM281XX
> >+	bool "BCM281XX SoCs" if ARCH_MULTI_V7
> >  	depends on MMU
> >  	select ARCH_REQUIRE_GPIOLIB
> >  	select ARM_ERRATA_754322
> >@@ -17,3 +21,7 @@ config ARCH_BCM
> >  	  It currently supports the 'BCM281XX' family, which includes
> >  	  BCM11130, BCM11140, BCM11351, BCM28145 and
> >  	  BCM28155 variants.
> >+
> >+endmenu
> >+
> >+endif
> >diff --git a/arch/arm/mach-bcm/Makefile b/arch/arm/mach-bcm/Makefile
> >index 6adb6aec..e3f8f27 100644
> >--- a/arch/arm/mach-bcm/Makefile
> >+++ b/arch/arm/mach-bcm/Makefile
> >@@ -10,6 +10,6 @@
> >  # of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> >  # GNU General Public License for more details.
> >-obj-$(CONFIG_ARCH_BCM)		:= board_bcm.o bcm_kona_smc.o bcm_kona_smc_asm.o
> >+obj-$(CONFIG_MACH_BCM281XX)	:= board_bcm.o bcm_kona_smc.o bcm_kona_smc_asm.o
> >  plus_sec := $(call as-instr,.arch_extension sec,+sec)
> >  AFLAGS_bcm_kona_smc_asm.o	:=-Wa,-march=armv7-a$(plus_sec)
> >
> Because ARCH_BCM is meant to be used for a number of SoC families.

As is ARCH_MVEBU for Marvell.  As kirkwood, dove, orion5x and mv78xx0
complete their conversion to DT and are multiplatform capable, they will
be migrated to mach-mvebu.  As is, it already contains two flavors of
their Armada SoCs.  mach-mvebu/Kconfig does things a little differently
than what I proposed above.  But the end result is the same because the
mach-mvebu/Kconfig file gets sourced, just like mach-bcm/Kconfig.

> We've started upstreaming one (the BCM281XX) but have 2 more
> internally that we're working towards upstreaming. And in the future
> our plan is to keep the Broadcom Mobile SoCs all building under this
> single ARCH_BCM configuration as multiplatform code building a
> single zImage for them.
> The intent of mach-bcm on the other hand is to aggregate future SoC
> chips beyond ARCH_BCM (which is mobile SoC focused) to include other
> Broadcom ARM based SoCs. And there are 2 in the wings at the moment
> making their way into mainline as patches. The idea here being that
> with the new multiplatform code, the mach- dirs contain so little
> code that it makes sense to aggregate a bunch of SoCs from the same
> company in there (and we are the guinea pig on this one).

So are we ;-)

thx,

Jason.
Russell King - ARM Linux July 26, 2013, 6:47 p.m. UTC | #3
On Fri, Jul 26, 2013 at 10:17:33AM -0700, Christian Daudt wrote:
> Because ARCH_BCM is meant to be used for a number of SoC families. We've  
> started upstreaming one (the BCM281XX) but have 2 more internally that  
> we're working towards upstreaming. And in the future our plan is to keep  
> the Broadcom Mobile SoCs all building under this single ARCH_BCM  
> configuration as multiplatform code building a single zImage for them.

1. We're moving to a single zImage for everything.  Not just Broadcom.
There's no need for Broadcom stuff to be treated any differently.
Participate in the single zImage project and you will get that benefit
without these games.

2. "library" mach-* support code doesn't go into another mach-* directory.
That's why we have the plat-* stuff.  Please follow this well established
model.  plat-* directories are selected by CONFIG_PLAT_* symbols, so
you would need CONFIG_PLAT_BCM.

Please don't persue your own solutions to problems which were solved years
ago and have established solutions already in place.
Christian Daudt July 26, 2013, 7:09 p.m. UTC | #4
On 13-07-26 11:47 AM, Russell King - ARM Linux wrote:
> On Fri, Jul 26, 2013 at 10:17:33AM -0700, Christian Daudt wrote:
>> Because ARCH_BCM is meant to be used for a number of SoC families. We've
>> started upstreaming one (the BCM281XX) but have 2 more internally that
>> we're working towards upstreaming. And in the future our plan is to keep
>> the Broadcom Mobile SoCs all building under this single ARCH_BCM
>> configuration as multiplatform code building a single zImage for them.
> 1. We're moving to a single zImage for everything.  Not just Broadcom.
> There's no need for Broadcom stuff to be treated any differently.
> Participate in the single zImage project and you will get that benefit
> without these games.
I don't follow what is the game being played. The Broadcom mobile team 
is planning on building all future chips out of a single ARCH_ config 
(which we called ARCH_BCM), and others need a separate ARCH_ to build 
(other) specific SoCs for different families. Our ARCH_BCM chip was one 
of the first upstreamed with multiplatform enabled (multi_v7_defconfig 
already selects ARCH_BCM). So right now there are the following 3:
ARCH_BCM -> BCM281XX now + a few others to follow
ARCH_BCM5301X -> BCM530X family
ARCH_BCM4760 -> BCM4760 family.

and these would all reside in mach-bcm. The only problem I see with the 
above is that ARCH_BCM would probably have been more aptly named 
ARCH_MOBILE_BCM as the SoCs showing up under that ARCH are all from the 
mobile team @ Broadcom. And if that is what is confusing people, I'm ok 
with changing that.

>
> 2. "library" mach-* support code doesn't go into another mach-* directory.
> That's why we have the plat-* stuff.  Please follow this well established
> model.  plat-* directories are selected by CONFIG_PLAT_* symbols, so
> you would need CONFIG_PLAT_BCM.
>
> Please don't persue your own solutions to problems which were solved years
> ago and have established solutions already in place.
Maybe I missed something but with the migration to multiplatform (and 
the minimalist size of the ensuing arch-specific files) my understanding 
was that there is no need for PLAT going forward, that all socs from a 
single company can just co-exist under the same mach- directory. If you 
look at mach-bcm at present there are 2 .c files + 1 .S file. With the 
cleanups we are doing to support > 1 family under ARCH_BCM, that number 
is going to grow by 2-4 source files. Adding the 2 other ARCHes above 
will add another handful to a grand total of < 10 source files. None of 
these are 'library' files in the current sense (i.e. each subset of 
these files will only be used for a single ARCH_ config option), so 
there is nothing to move to a plat- directory.
  As an alternative to what is currently being done, I guess could go 
back to the 1-mach-directory-per-arch style so we'd have:
mach-bcm -> ARCH_BCM
mach-bcm5301x -> ARCH_BCM530X
mach-bcm4760 -> ARCH_BCM4760
but each of these dirs will have 2-3 source files. Which is what I heard 
a while back we wanted to start avoiding (the explosition of mach- dirs 
with next-to-nothing in each). Are you proposing we revert to this model ?

  thanks,
    csd
Russell King - ARM Linux July 26, 2013, 7:39 p.m. UTC | #5
On Fri, Jul 26, 2013 at 12:09:03PM -0700, Christian Daudt wrote:
> On 13-07-26 11:47 AM, Russell King - ARM Linux wrote:
>> On Fri, Jul 26, 2013 at 10:17:33AM -0700, Christian Daudt wrote:
>>> Because ARCH_BCM is meant to be used for a number of SoC families. We've
>>> started upstreaming one (the BCM281XX) but have 2 more internally that
>>> we're working towards upstreaming. And in the future our plan is to keep
>>> the Broadcom Mobile SoCs all building under this single ARCH_BCM
>>> configuration as multiplatform code building a single zImage for them.
>> 1. We're moving to a single zImage for everything.  Not just Broadcom.
>> There's no need for Broadcom stuff to be treated any differently.
>> Participate in the single zImage project and you will get that benefit
>> without these games.
> I don't follow what is the game being played.

I don't really mind what you do, but when you start talking about having
any ARM build always descend into mach-bcm, then something is very wrong
with how you're going about it - you don't see anyone else needing that
in the tree, not even OMAP.

I don't really care if your intention is to decend into mach-bcm and then
do nothing - that's not the point.  The point is we shouldn't even
decend into mach-bcm if there's nothing to be built there.
Domenico Andreoli July 26, 2013, 10:28 p.m. UTC | #6
On Fri, Jul 26, 2013 at 08:39:08PM +0100, Russell King - ARM Linux wrote:
> On Fri, Jul 26, 2013 at 12:09:03PM -0700, Christian Daudt wrote:
> > On 13-07-26 11:47 AM, Russell King - ARM Linux wrote:
> >> On Fri, Jul 26, 2013 at 10:17:33AM -0700, Christian Daudt wrote:
> >>> Because ARCH_BCM is meant to be used for a number of SoC families. We've
> >>> started upstreaming one (the BCM281XX) but have 2 more internally that
> >>> we're working towards upstreaming. And in the future our plan is to keep
> >>> the Broadcom Mobile SoCs all building under this single ARCH_BCM
> >>> configuration as multiplatform code building a single zImage for them.
> >> 1. We're moving to a single zImage for everything.  Not just Broadcom.
> >> There's no need for Broadcom stuff to be treated any differently.
> >> Participate in the single zImage project and you will get that benefit
> >> without these games.
> > I don't follow what is the game being played.
> 
> I don't really mind what you do, but when you start talking about having
> any ARM build always descend into mach-bcm, then something is very wrong
> with how you're going about it - you don't see anyone else needing that
> in the tree, not even OMAP.

That was a Great Idea(tm) of mine that obviously needed some rebuttal.
No insult was implied for those not having had it in solving the very
same problem.

BTW I asked this very same question [1] and got _zero_ feedback so I
thought it was worth a patch.

> I don't really care if your intention is to decend into mach-bcm and then
> do nothing - that's not the point.  The point is we shouldn't even
> decend into mach-bcm if there's nothing to be built there.

I've got the point but didn't fully understand why would be so wrong to
walk one more subdir even if nothing is going to be built.

thanks,
Domenico

[1] http://permalink.gmane.org/gmane.linux.ports.arm.kernel/253500
Russell King - ARM Linux July 26, 2013, 10:38 p.m. UTC | #7
On Sat, Jul 27, 2013 at 12:28:09AM +0200, Domenico Andreoli wrote:
> I've got the point but didn't fully understand why would be so wrong to
> walk one more subdir even if nothing is going to be built.

Consider what would happen if we decended the 61 mach- subdirectories
on every build run when only one was really required.  Even though
kbuild tries to be fast, there's still a non-zero amount of time
involved with finding out there's nothing to be done - and you
will still end up generating a built-in.o file even though there's
no other objects, which will then also be included in the final
link - again, adding to the work which has to be done.
Domenico Andreoli July 26, 2013, 11:30 p.m. UTC | #8
On Fri, Jul 26, 2013 at 11:38:29PM +0100, Russell King - ARM Linux wrote:
> On Sat, Jul 27, 2013 at 12:28:09AM +0200, Domenico Andreoli wrote:
> > I've got the point but didn't fully understand why would be so wrong to
> > walk one more subdir even if nothing is going to be built.
> 
> Consider what would happen if we decended the 61 mach- subdirectories
> on every build run when only one was really required.  Even though
> kbuild tries to be fast, there's still a non-zero amount of time
> involved with finding out there's nothing to be done - and you
> will still end up generating a built-in.o file even though there's
> no other objects, which will then also be included in the final
> link - again, adding to the work which has to be done.

crystal clear. thanks.

Domenico
diff mbox

Patch

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index ba412e0..97b6aff 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -366,6 +366,12 @@  config ARCH_AT91
 	  This enables support for systems based on Atmel
 	  AT91RM9200 and AT91SAM9* processors.
 
+config ARCH_BCM
+	bool "Broadcom family SoCs"
+	help
+	  This enables support for systems based on the Broadcom
+	  bcm4760 and bcm281XX series SoCs.
+
 config ARCH_CLPS711X
 	bool "Cirrus Logic CLPS711x/EP721x/EP731x-based"
 	select ARCH_REQUIRE_GPIOLIB
diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig
index f112895..4b1f9db 100644
--- a/arch/arm/mach-bcm/Kconfig
+++ b/arch/arm/mach-bcm/Kconfig
@@ -1,5 +1,9 @@ 
-config ARCH_BCM
-	bool "Broadcom SoC" if ARCH_MULTI_V7
+if ARCH_BCM
+
+menu "Broadcom SoC Implementations"
+
+config MACH_BCM281XX
+	bool "BCM281XX SoCs" if ARCH_MULTI_V7
 	depends on MMU
 	select ARCH_REQUIRE_GPIOLIB
 	select ARM_ERRATA_754322
@@ -17,3 +21,7 @@  config ARCH_BCM
 	  It currently supports the 'BCM281XX' family, which includes
 	  BCM11130, BCM11140, BCM11351, BCM28145 and
 	  BCM28155 variants.
+
+endmenu
+
+endif
diff --git a/arch/arm/mach-bcm/Makefile b/arch/arm/mach-bcm/Makefile
index 6adb6aec..e3f8f27 100644
--- a/arch/arm/mach-bcm/Makefile
+++ b/arch/arm/mach-bcm/Makefile
@@ -10,6 +10,6 @@ 
 # of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 # GNU General Public License for more details.
 
-obj-$(CONFIG_ARCH_BCM)		:= board_bcm.o bcm_kona_smc.o bcm_kona_smc_asm.o
+obj-$(CONFIG_MACH_BCM281XX)	:= board_bcm.o bcm_kona_smc.o bcm_kona_smc_asm.o
 plus_sec := $(call as-instr,.arch_extension sec,+sec)
 AFLAGS_bcm_kona_smc_asm.o	:=-Wa,-march=armv7-a$(plus_sec)