ARM: Dove: allow PCI to be disabled
PATCH 1/1 ARM: Dove: allow PCI to be disabled
Allow PCI support for Dove to be disabled. Some platforms do not have anything connected to the PCIe ports, so requiring PCI support to be built into the kernel just wastes space, and presents a lot more config options than are necessary.
However, select USB_ARCH_HAS_EHCI so that we can still have EHCI support.
--- arch/arm/Kconfig | 3 ++- arch/arm/mach-dove/Makefile | 3 ++- arch/arm/mach-dove/common.h | 4 ++++ 3 files changed, 8 insertions(+), 2 deletions(-)
arch/arm/Kconfig +2 -1
@@ -536,14 +536,15 @@ config ARCH_IXP4XX config ARCH_DOVE bool "Marvell Dove" select CPU_V7 - select PCI select ARCH_REQUIRE_GPIOLIB select GENERIC_CLOCKEVENTS + select MIGHT_HAVE_PCI select NEED_MACH_IO_H select PLAT_ORION select CLKDEV_LOOKUP select NEED_MACH_MEMORY_H select COMMON_CLK + select USB_ARCH_HAS_EHCI help Support for the Marvell Dove SoC 88AP510
arch/arm/mach-dove/Makefile +2 -1
@@ -1,4 +1,5 @@ -obj-y += common.o addr-map.o irq.o pcie.o mpp.o clock.o dump_cp15_regs.o +obj-y += common.o addr-map.o irq.o mpp.o clock.o dump_cp15_regs.o +obj-$(CONFIG_PCI) += pcie.o obj-$(CONFIG_MACH_DOVE_DB) += dove-db-setup.o obj-$(CONFIG_MACH_CM_A510) += cm-a510.o
arch/arm/mach-dove/common.h +4 -0
@@ -29,7 +29,11 @@ void dove_setup_cpu_mbus(void);void dove_ge00_init (struct mv643xx_eth_platform_data *eth_data);void dove_hwmon_init (void );void dove_sata_init (struct mv_sata_platform_data *sata_data); +#ifdef CONFIG_PCI void dove_pcie_init (int init_port0,int init_port1); +#else +static inline void dove_pcie_init (int init_port0,int init_port1) { } +#endif void dove_ehci0_init (void );void dove_ehci1_init (void );void dove_uart0_init (void );
Signature · 1 line
1.7.4.4
Andrew Lunn
maintainerPATCH 1/1 ARM: Dove: allow PCI to be disabled
Quoted 7 lines · Russell King
Allow PCI support for Dove to be disabled. Some platforms do not have anything connected to the PCIe ports, so requiring PCI support to be built into the kernel just wastes space, and presents a lot more config options than are necessary.
However, select
USB_ARCH_HAS_EHCIso that we can still have EHCI support.
Hi Russel
I have a patch in GregKH usb-next which also ensures USB_ARCH_HAS_EHCI is selected.
So we might not need your select as well, although it does not harm.
Andrew
PATCH 1/1 ARM: Dove: allow PCI to be disabled
Quoted 9 lines · Andrew Lunn
Quoted 7 lines · Russell King
Allow PCI support for Dove to be disabled. Some platforms do not have anything connected to the PCIe ports, so requiring PCI support to be built into the kernel just wastes space, and presents a lot more config options than are necessary.
However, select
USB_ARCH_HAS_EHCIso that we can still have EHCI support.
Quoted 8 lines
Hi Russel
I have a patch in GregKH usb-next which also ensures
USB_ARCH_HAS_EHCIis selected.So we might not need your select as well, although it does not harm.
That depends on how this patch is handled. If it gets into mainline via a separate path to GregKH's USB tree, then it must be kept together as otherwise it _will_ cause regressions - namely EHCI becomes impossible to select when PCI=n. That in turn makes the CuBox (and probably other Dove platforms) a rather useless display-only platform...
PATCH 1/1 ARM: Dove: allow PCI to be disabled
Quoted 17 lines · Russell King - ARM Linux
Quoted 16 lines · Andrew Lunn
Quoted 7 lines · Russell King
Allow PCI support for Dove to be disabled. Some platforms do not have anything connected to the PCIe ports, so requiring PCI support to be built into the kernel just wastes space, and presents a lot more config options than are necessary.
However, select
USB_ARCH_HAS_EHCIso that we can still have EHCI support.Hi Russel
I have a patch in GregKH usb-next which also ensures
USB_ARCH_HAS_EHCIis selected.
Quoted 7 lines
Quoted 1 line
So we might not need your select as well, although it does not harm.
That depends on how this patch is handled. If it gets into mainline via a separate path to GregKH's USB tree, then it must be kept together as otherwise it _will_ cause regressions - namely EHCI becomes impossible to select when PCI=n. That in turn makes the CuBox (and probably other Dove platforms) a rather useless di
[message trucated, I need to working on file locking for my email auto-flagging script... :-/]
greg just pulled Andrew's patch into his usb tree within the past few days. Russell, would you prefer to submit this to greg's usb tree, or resend without USB_ARCH_HAS_EHCI?
thx,
Jason.
PATCH 1/1 ARM: Dove: allow PCI to be disabled
Quoted 25 lines · Jason Cooper
Quoted 17 lines · Russell King - ARM Linux
Quoted 16 lines · Andrew Lunn
Quoted 7 lines · Russell King
Allow PCI support for Dove to be disabled. Some platforms do not have anything connected to the PCIe ports, so requiring PCI support to be built into the kernel just wastes space, and presents a lot more config options than are necessary.
However, select
USB_ARCH_HAS_EHCIso that we can still have EHCI support.Hi Russel
I have a patch in GregKH usb-next which also ensures
USB_ARCH_HAS_EHCIis selected.Quoted 7 lines
Quoted 1 line
So we might not need your select as well, although it does not harm.
That depends on how this patch is handled. If it gets into mainline via a separate path to GregKH's USB tree, then it must be kept together as otherwise it _will_ cause regressions - namely EHCI becomes impossible to select when PCI=n. That in turn makes the CuBox (and probably other Dove platforms) a rather useless di
Quoted 6 lines
[message trucated, I need to working on file locking for my email auto-flagging script... :-/]
greg just pulled Andrew's patch into his usb tree within the past few days. Russell, would you prefer to submit this to greg's usb tree, or resend without
USB_ARCH_HAS_EHCI?
I thought I'd already answered the latter point.
As I said, it breaks the patch to remove that select, so it can not be removed. Removing it will create regressions - can I be any more clear about that. So no, I will not remove that select statement because I can't without _breaking_ my Dove builds here.
I don't think submitting this patch to Greg's USB tree is also appropriate either, it's clearly ARM specific in nature, or even PCI specific. It isn't a USB patch. Neither is a patch changing a USB setting in arch/arm/Kconfig. If any tree, these patches should be going through arm-soc.
PATCH 1/1 ARM: Dove: allow PCI to be disabled
Quoted 33 lines · Russell King - ARM Linux
Quoted 25 lines · Jason Cooper
Quoted 17 lines · Russell King - ARM Linux
Quoted 16 lines · Andrew Lunn
Quoted 7 lines · Russell King
Allow PCI support for Dove to be disabled. Some platforms do not have anything connected to the PCIe ports, so requiring PCI support to be built into the kernel just wastes space, and presents a lot more config options than are necessary.
However, select
USB_ARCH_HAS_EHCIso that we can still have EHCI support.Hi Russel
I have a patch in GregKH usb-next which also ensures
USB_ARCH_HAS_EHCIis selected.Quoted 7 lines
Quoted 1 line
So we might not need your select as well, although it does not harm.
That depends on how this patch is handled. If it gets into mainline via a separate path to GregKH's USB tree, then it must be kept together as otherwise it _will_ cause regressions - namely EHCI becomes impossible to select when PCI=n. That in turn makes the CuBox (and probably other Dove platforms) a rather useless di
Quoted 6 lines
[message trucated, I need to working on file locking for my email auto-flagging script... :-/]
greg just pulled Andrew's patch into his usb tree within the past few days. Russell, would you prefer to submit this to greg's usb tree, or resend without
USB_ARCH_HAS_EHCI?
Quoted 6 lines
I thought I'd already answered the latter point.
As I said, it breaks the patch to remove that select, so it can not be removed. Removing it will create regressions - can I be any more clear about that. So no, I will not remove that select statement because I can't without _breaking_ my Dove builds here.
My apologies. I was simply trying to achieve the cleanest solution. Andrew's patch selects USB_ARCH_HAS_EHCI whenever PLAT_ORION is enabled, of which dove is one. Purely from the pov of the code, this makes the select redundant.
iiuc, the regression you are refering to would occur iff this patch (without the select) makes it into mainline _before_ Andrew's patch in Greg's tree. As I haven't pulled this patch yet, I can add a dependency in the tag message listing the patch in gregkh's usb tree. Arnd and Olof would heed this and make sure this patch comes after Andrews. This should prevent the regression.
If the above is true, then indeed, the select is not necessary. Is there something else I'm missing?
Quoted 3 lines
I don't think submitting this patch to Greg's USB tree is also appropriate either, it's clearly ARM specific in nature, or even PCI specific. It isn't a USB patch.
Agreed, please chalk that up to new maintainer crack-smoking. :-)
Quoted 2 lines
Neither is a patch changing a USB setting in
arch/arm/Kconfig. If any tree, these patches should be going through arm-soc.
Agreed.
thx,
Jason.
PATCH 1/1 ARM: Dove: allow PCI to be disabled
Quoted 4 lines · Jason Cooper
iiuc, the regression you are refering to would occur iff this patch (without the select) makes it into mainline _before_ Andrew's patch in Greg's tree. As I haven't pulled this patch yet, I can add a dependency in the tag message listing the patch in gregkh's usb tree.
For the regression to occur, you need to land on a commit in the tree where this patch (without the select) is in the history but not the patch in Greg's tree.
The commit history remains a series of independent branches even after trees are merged into Linus' tree - there is _no_ linearisation of the history in git.
This means when anyone comes to use git bisect, they _can_ land on any commit in any branch, which means if this patch and the other patch take two different routes, and this patch does not have the select, you _will_ hit the regression.
Let me illustrate:
---A----B----C----D---...---E----M
`--G----H----I----J---...---K-'
Let's say A-E is Greg's tree, and D is the commit you are talking about. Let's say G-K is Arnd's tree, and H is my patch without the 'select' statement. Let's also say that '...' in each case includes a significant number of commits.
If, through a git bisect (or manual commit selection) you end up trying to build at any commit I to K, you will not be builting a tree with D in, but you will be building a tree with H in. You end up with EHCI being disabled in your configuration.
What this also means is that when building Arnd's tree without Greg's tree, you will also end up with EHCI disabled in those configurations (because you also don't have commit D.)
See? There is no such thing as "lets make sure tree X gets merged before tree Y because commit A depends on commit B". That "solution" doesn't fix any dependency between the two commits - it merely gives the impression that it does because you're looking only at the result.
That kind of solution is broken thinking.
Quoted 3 lines
Arnd and
Olof would heed this and make sure this patch comes after Andrews. This should prevent the regression.
As you can see from the above, tree order does not solve these issues. It gives the impression from the final result that it does, but it doesn't. It breaks at least our most powerful regression debug tool - git bisect.
PATCH 1/1 ARM: Dove: allow PCI to be disabled
Quoted 6 lines · Russell King - ARM Linux
Quoted 4 lines · Jason Cooper
iiuc, the regression you are refering to would occur iff this patch (without the select) makes it into mainline _before_ Andrew's patch in Greg's tree. As I haven't pulled this patch yet, I can add a dependency in the tag message listing the patch in gregkh's usb tree.
Quoted 7 lines
For the regression to occur, you need to land on a commit in the tree where this patch (without the select) is in the history but not the patch in Greg's tree.
The commit history remains a series of independent branches even after trees are merged into Linus' tree - there is _no_ linearisation of the history in git.
@Jason:
The dependency tracking done in the arm-soc tree performed by Arnd and Olof is merely a way for you to _base_ your branch on top of other people's branch before they pull it from you. For example, you may decide to build your branch on top of Greg's to get some prerequisite commits for your work. In that case the goal for the arm-soc maintainers is to avoid the situation where a pull of the arm-soc tree into mainline would also pull greg's stuff without his asking for it.
Nicolas
PATCH 1/1 ARM: Dove: allow PCI to be disabled
In reply to Russell King - ARM LinuxQuoted 6 lines · Russell King - ARM Linux
Quoted 4 lines · Jason Cooper
iiuc, the regression you are refering to would occur iff this patch (without the select) makes it into mainline _before_ Andrew's patch in Greg's tree. As I haven't pulled this patch yet, I can add a dependency in the tag message listing the patch in gregkh's usb tree.
Quoted 7 lines
For the regression to occur, you need to land on a commit in the tree where this patch (without the select) is in the history but not the patch in Greg's tree.
The commit history remains a series of independent branches even after trees are merged into Linus' tree - there is _no_ linearisation of the history in git.
<ding> (light comes on). Thank you! Somewhere in the time I've been working with git I developed an incorrect assumption. While embarrassing, I'm glad I asked for clarification.
Anytime I view the merging of branches in my head, I always see gitg's "all branches" view. Since patches are sorted over time, I assumed they became related that way once they were merged. It sounds silly as I type it out loud. :-)
It all makes sense now.
Thanks for taking the time to explain it so clearly,
Jason.
PATCH 1/1 ARM: Dove: allow PCI to be disabled
In reply to Jason CooperQuoted 41 lines · Jason Cooper
Quoted 33 lines · Russell King - ARM Linux
Quoted 25 lines · Jason Cooper
Quoted 17 lines · Russell King - ARM Linux
Quoted 16 lines · Andrew Lunn
Quoted 7 lines · Russell King
Allow PCI support for Dove to be disabled. Some platforms do not have anything connected to the PCIe ports, so requiring PCI support to be built into the kernel just wastes space, and presents a lot more config options than are necessary.
However, select
USB_ARCH_HAS_EHCIso that we can still have EHCI support.Hi Russel
I have a patch in GregKH usb-next which also ensures
USB_ARCH_HAS_EHCIis selected.Quoted 7 lines
Quoted 1 line
So we might not need your select as well, although it does not harm.
That depends on how this patch is handled. If it gets into mainline via a separate path to GregKH's USB tree, then it must be kept together as otherwise it _will_ cause regressions - namely EHCI becomes impossible to select when PCI=n. That in turn makes the CuBox (and probably other Dove platforms) a rather useless di
Quoted 6 lines
[message trucated, I need to working on file locking for my email auto-flagging script... :-/]
greg just pulled Andrew's patch into his usb tree within the past few days. Russell, would you prefer to submit this to greg's usb tree, or resend without
USB_ARCH_HAS_EHCI?Quoted 6 lines
I thought I'd already answered the latter point.
As I said, it breaks the patch to remove that select, so it can not be removed. Removing it will create regressions - can I be any more clear about that. So no, I will not remove that select statement because I can't without _breaking_ my Dove builds here.
Quoted 4 lines
My apologies. I was simply trying to achieve the cleanest solution. Andrew's patch selects
USB_ARCH_HAS_EHCIwheneverPLAT_ORIONis enabled, of which dove is one. Purely from the pov of the code, this makes the select redundant.
Are we trying to split hairs here?
At best, the select will be redundant and that is trivially cleaned up later. At worst, you'll have a trivial git merge conflict to solve (and that happens all the time).
So the simplest thing to do is to keep the select there to prevent build issues and remember that perfection is the enemy of the good.
Nicolas
PATCH 1/1 ARM: Dove: allow PCI to be disabled
Quoted 2 lines · Nicolas Pitre
So the simplest thing to do is to keep the select there to prevent build issues and remember that perfection is the enemy of the good.
Yes, I have to remind myself of that a lot. To the point I've considered getting a tattoo of it somewhere. :-)
You are right. I was primarily trying to drill down on something I didn't understand, which is how leaving out that line could cause a regression. The patch is fine, of course, I'll pull it in in my next round. Anything else in this thread will be purely me attempting to make sure I fully understand git and the merge process wrt to causing regressions.
thx,
Jason.