* Sharing header files -- arch/arm/mach-generic ?
@ 2012-04-23 19:40 ` Pavel Machek
0 siblings, 0 replies; 6+ messages in thread
From: Pavel Machek @ 2012-04-23 19:40 UTC (permalink / raw)
To: linux-arm-kernel
Hi!
I have new architecture I'd like to merge into the mainline. It has
some code very similar to existing plaforms (for example
real differences to mach-realview/platsmp.c are two lines.)
Is that a problem?
Should mach-generic/platsmp.c be created, so code can be better
shared? Or are the code pieces small enough that this duplication can
be ignored?
headsmp.S is another candidate for sharing. AFAICT we have 4 times
identical code in the tree already:
./mach-exynos/headsmp.S: ldmia r4, {r5, r6}
./plat-versatile/headsmp.S: ldmia r4, {r5, r6}
./mach-ux500/headsmp.S: ldmia r4, {r5, r6}
./mach-msm/headsmp.S: ldmia r4, {r5, r6}
Thoughts?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply [flat|nested] 6+ messages in thread* Sharing header files -- arch/arm/mach-generic ? @ 2012-04-23 19:40 ` Pavel Machek 0 siblings, 0 replies; 6+ messages in thread From: Pavel Machek @ 2012-04-23 19:40 UTC (permalink / raw) To: kernel list, linux-arm-kernel, wd Hi! I have new architecture I'd like to merge into the mainline. It has some code very similar to existing plaforms (for example real differences to mach-realview/platsmp.c are two lines.) Is that a problem? Should mach-generic/platsmp.c be created, so code can be better shared? Or are the code pieces small enough that this duplication can be ignored? headsmp.S is another candidate for sharing. AFAICT we have 4 times identical code in the tree already: ./mach-exynos/headsmp.S: ldmia r4, {r5, r6} ./plat-versatile/headsmp.S: ldmia r4, {r5, r6} ./mach-ux500/headsmp.S: ldmia r4, {r5, r6} ./mach-msm/headsmp.S: ldmia r4, {r5, r6} Thoughts? Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ^ permalink raw reply [flat|nested] 6+ messages in thread
* Sharing header files -- arch/arm/mach-generic ? 2012-04-23 19:40 ` Pavel Machek @ 2012-04-23 19:52 ` Russell King - ARM Linux -1 siblings, 0 replies; 6+ messages in thread From: Russell King - ARM Linux @ 2012-04-23 19:52 UTC (permalink / raw) To: linux-arm-kernel On Mon, Apr 23, 2012 at 09:40:42PM +0200, Pavel Machek wrote: > I have new architecture I'd like to merge into the mainline. It has > some code very similar to existing plaforms (for example > real differences to mach-realview/platsmp.c are two lines.) I suspect there's more differences than just that, because 17 lines in that file are very specific to realview. I doubt that you even need a scu_base_addr() function as you probably know where the SCU is located. So, I bet the reason your example is just two lines different from mach-realview/platsmp.c is because someone copied and modified the file rather than thinking about what's really required. > Should mach-generic/platsmp.c be created, so code can be better > shared? Or are the code pieces small enough that this duplication can > be ignored? It should be ignored. > headsmp.S is another candidate for sharing. AFAICT we have 4 times > identical code in the tree already: > > ./mach-exynos/headsmp.S: ldmia r4, {r5, r6} > ./plat-versatile/headsmp.S: ldmia r4, {r5, r6} > ./mach-ux500/headsmp.S: ldmia r4, {r5, r6} > ./mach-msm/headsmp.S: ldmia r4, {r5, r6} Again, because of standard 'lets copy and modify' behaviour. I'd really like to drop the holding pen from versatile stuff - it's not really required, as we can wake each CPU individually. The only reason it hasn't happened is because the holding pen stuff is also used for holding CPUs in the unplugged state, as realview and versatile express have no way to power manage or reset individual CPUs. Most normal platforms (iow, those fabricated on real SoCs rather than test-chips) have ways to power off the unplugged CPU, and don't need this holding pen. Or they only have one additional CPU, and so don't even need any holding pen. Unfortunately, I don't have the time to hit people over the head to get them to fix their implementations here, so lots of copy-n-modify for the SMP bringup from Realview has happened over the years. It's little surprise though that OMAP (where I've been quite involved with) does not suffer from this... Basically, everything except plat-versatile probably shouldn't be using this holding pen stuff. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Sharing header files -- arch/arm/mach-generic ? @ 2012-04-23 19:52 ` Russell King - ARM Linux 0 siblings, 0 replies; 6+ messages in thread From: Russell King - ARM Linux @ 2012-04-23 19:52 UTC (permalink / raw) To: Pavel Machek; +Cc: kernel list, linux-arm-kernel, wd On Mon, Apr 23, 2012 at 09:40:42PM +0200, Pavel Machek wrote: > I have new architecture I'd like to merge into the mainline. It has > some code very similar to existing plaforms (for example > real differences to mach-realview/platsmp.c are two lines.) I suspect there's more differences than just that, because 17 lines in that file are very specific to realview. I doubt that you even need a scu_base_addr() function as you probably know where the SCU is located. So, I bet the reason your example is just two lines different from mach-realview/platsmp.c is because someone copied and modified the file rather than thinking about what's really required. > Should mach-generic/platsmp.c be created, so code can be better > shared? Or are the code pieces small enough that this duplication can > be ignored? It should be ignored. > headsmp.S is another candidate for sharing. AFAICT we have 4 times > identical code in the tree already: > > ./mach-exynos/headsmp.S: ldmia r4, {r5, r6} > ./plat-versatile/headsmp.S: ldmia r4, {r5, r6} > ./mach-ux500/headsmp.S: ldmia r4, {r5, r6} > ./mach-msm/headsmp.S: ldmia r4, {r5, r6} Again, because of standard 'lets copy and modify' behaviour. I'd really like to drop the holding pen from versatile stuff - it's not really required, as we can wake each CPU individually. The only reason it hasn't happened is because the holding pen stuff is also used for holding CPUs in the unplugged state, as realview and versatile express have no way to power manage or reset individual CPUs. Most normal platforms (iow, those fabricated on real SoCs rather than test-chips) have ways to power off the unplugged CPU, and don't need this holding pen. Or they only have one additional CPU, and so don't even need any holding pen. Unfortunately, I don't have the time to hit people over the head to get them to fix their implementations here, so lots of copy-n-modify for the SMP bringup from Realview has happened over the years. It's little surprise though that OMAP (where I've been quite involved with) does not suffer from this... Basically, everything except plat-versatile probably shouldn't be using this holding pen stuff. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Sharing header files -- arch/arm/mach-generic ? 2012-04-23 19:40 ` Pavel Machek @ 2012-04-23 20:15 ` Arnd Bergmann -1 siblings, 0 replies; 6+ messages in thread From: Arnd Bergmann @ 2012-04-23 20:15 UTC (permalink / raw) To: linux-arm-kernel On Monday 23 April 2012, Pavel Machek wrote: > I have new architecture I'd like to merge into the mainline. It has > some code very similar to existing plaforms (for example > real differences to mach-realview/platsmp.c are two lines.) > > Is that a problem? > > Should mach-generic/platsmp.c be created, so code can be better > shared? Or are the code pieces small enough that this duplication can > be ignored? Hi Pavel, I'd suggest you just post the code with the duplicate files for review at first, and then we can discuss what to do with them. As Russell mentioned, the code might not actually be ideal in the duplicate version so you end up rewriting it, or there might be a different solution that someone finds when we see the patches. Arnd ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Sharing header files -- arch/arm/mach-generic ? @ 2012-04-23 20:15 ` Arnd Bergmann 0 siblings, 0 replies; 6+ messages in thread From: Arnd Bergmann @ 2012-04-23 20:15 UTC (permalink / raw) To: linux-arm-kernel; +Cc: Pavel Machek, kernel list, wd On Monday 23 April 2012, Pavel Machek wrote: > I have new architecture I'd like to merge into the mainline. It has > some code very similar to existing plaforms (for example > real differences to mach-realview/platsmp.c are two lines.) > > Is that a problem? > > Should mach-generic/platsmp.c be created, so code can be better > shared? Or are the code pieces small enough that this duplication can > be ignored? Hi Pavel, I'd suggest you just post the code with the duplicate files for review at first, and then we can discuss what to do with them. As Russell mentioned, the code might not actually be ideal in the duplicate version so you end up rewriting it, or there might be a different solution that someone finds when we see the patches. Arnd ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-04-23 20:15 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-04-23 19:40 Sharing header files -- arch/arm/mach-generic ? Pavel Machek 2012-04-23 19:40 ` Pavel Machek 2012-04-23 19:52 ` Russell King - ARM Linux 2012-04-23 19:52 ` Russell King - ARM Linux 2012-04-23 20:15 ` Arnd Bergmann 2012-04-23 20:15 ` Arnd Bergmann
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.