| Message ID | 1392719391-8851-1-git-send-email-thomas.petazzoni@free-electrons.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Feb 18, 2014 at 11:29:51AM +0100, Thomas Petazzoni wrote: > With the introduction of the support for Armada 375 and Armada 38x, > the hidden Kconfig option MACH_ARMADA_370_XP is being renamed to > MACH_MVEBU_V7. Therefore, the dependency that was used for the mvneta > driver can no longer work. > > However, such a dependency is not really necessary: there is no point > in preventing this driver from being built in other situations, just > like the mv643xx_eth driver. As a consequence, this commit removes the > unnecessary Kconfig dependency. > > In addition to this, it takes this opportunity to adjust the > description and help text to indicate that the driver can is also used > for Armada 38x. Note that Armada 375 cannot use this driver as it has > a completely different networking unit, which will require a separate > driver. > > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > --- > drivers/net/ethernet/marvell/Kconfig | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/ethernet/marvell/Kconfig b/drivers/net/ethernet/marvell/Kconfig > index 6300fd2..97b91f7 100644 > --- a/drivers/net/ethernet/marvell/Kconfig > +++ b/drivers/net/ethernet/marvell/Kconfig > @@ -43,12 +43,11 @@ config MVMDIO > This driver is used by the MV643XX_ETH and MVNETA drivers. > > config MVNETA > - tristate "Marvell Armada 370/XP network interface support" > - depends on MACH_ARMADA_370_XP Have you build-tested this on the usual fail scenarios? eg x86_64, powerpc, s390, allno, allyes, allmod, etc? I think you might be opening up pandora's box here without needing to. :) thx, Jason.
Dear Jason Cooper, On Tue, 18 Feb 2014 07:29:52 -0500, Jason Cooper wrote: > > config MVNETA > > - tristate "Marvell Armada 370/XP network interface support" > > - depends on MACH_ARMADA_370_XP > > Have you build-tested this on the usual fail scenarios? eg x86_64, > powerpc, s390, allno, allyes, allmod, etc? > > I think you might be opening up pandora's box here without needing to. > :) No, I haven't tested all the build scenarios of course. If you feel that this is too dangerous, I can resend a patch that replaces 'depends on ARMADA_370_XP' by 'depends on PLAT_ORION'. Would this be OK for you? Thanks, Thomas
On Tue, Feb 18, 2014 at 01:45:32PM +0100, Thomas Petazzoni wrote: > Dear Jason Cooper, > > On Tue, 18 Feb 2014 07:29:52 -0500, Jason Cooper wrote: > > > > config MVNETA > > > - tristate "Marvell Armada 370/XP network interface support" > > > - depends on MACH_ARMADA_370_XP > > > > Have you build-tested this on the usual fail scenarios? eg x86_64, > > powerpc, s390, allno, allyes, allmod, etc? > > > > I think you might be opening up pandora's box here without needing to. > > :) > > No, I haven't tested all the build scenarios of course. If you feel > that this is too dangerous, I can resend a patch that replaces 'depends > on ARMADA_370_XP' by 'depends on PLAT_ORION'. Would this be OK for you? I'm fine either way. I'm definitely a "What's in the box?" kind of guy. But I do like being prepared. The real question is: Do you think this IP block will ever be found on anything other than Marvell Armada boards? If so, stick with this version and build test the crap out of it. Otherwise, let's save opening the box for another day. thx, Jason.
Dear Jason Cooper, On Tue, 18 Feb 2014 07:52:06 -0500, Jason Cooper wrote: > > No, I haven't tested all the build scenarios of course. If you feel > > that this is too dangerous, I can resend a patch that replaces 'depends > > on ARMADA_370_XP' by 'depends on PLAT_ORION'. Would this be OK for you? > > I'm fine either way. I'm definitely a "What's in the box?" kind of guy. > But I do like being prepared. And it's all in your honour! > The real question is: Do you think this IP block will ever be found on > anything other than Marvell Armada boards? If so, stick with this > version and build test the crap out of it. Otherwise, let's save > opening the box for another day. At this point, I don't expect to see this IP in any other SOCs than Marvell Armada EBU, so I'll resend with a "depends on PLAT_ORION", as I agree it is much safer in terms of build problems. Thomas
> At this point, I don't expect to see this IP in any other SOCs than > Marvell Armada EBU, so I'll resend with a "depends on PLAT_ORION", as I > agree it is much safer in terms of build problems. Hi Thomas PLAT_ORION is a bit of an odd thing now. For me, PLAT_ORION means arch/arm/plat-orion. But as far as i know, 370/XP does not actually use anything from arch/arm/plat-orion. When kirkwood moves into mach-mvebu, it also will not use any code from it, and i suspect dove is the same. So maybe in a few cycles, when only mach-orion5x is left, we can merge arch/arm/plat-orion into arch/arm/mach-orion5x and PLAT_ORION goes away? Or do we want to define that PLAT_ORION means any system which can make use of mvebu drivers? Andrew
Dear Andrew Lunn, On Tue, 18 Feb 2014 14:58:09 +0100, Andrew Lunn wrote: > PLAT_ORION is a bit of an odd thing now. > > For me, PLAT_ORION means arch/arm/plat-orion. No, it is PLAT_ORION_LEGACY which means arch/arm/plat-orion. > But as far as i know, 370/XP does not actually use anything from > arch/arm/plat-orion. When kirkwood moves into mach-mvebu, it also will > not use any code from it, and i suspect dove is the same. Interestingly, we have -I$(srctree)/arch/arm/plat-orion/include in mach-mvebu/Makefile. Might be something to revisit later on. > So maybe in a few cycles, when only mach-orion5x is left, we can merge > arch/arm/plat-orion into arch/arm/mach-orion5x and PLAT_ORION goes > away? We also have mach-mv78xx0 to worry about, unless we decide to remove support for it entirely. And even if plat-orion is merged into mach-orion5x, we will keep PLAT_ORION as a way to indicate that the platform needs to use a certain number of drivers (see below). > Or do we want to define that PLAT_ORION means any system which can > make use of mvebu drivers? This is what it means today. The MV_XOR driver is under PLAT_ORION, the gpio driver is under PLAT_ORION, the Device Bus driver is under PLAT_ORION, the I2C driver is under PLAT_ORION, the pinctrl driver as well, and so on and so on. At the very end of the clean up, when even Orion5x support will be merged in mach-mvebu/, then we can certainly replace these dependencies by a "depends on ARCH_MVEBU". But for the time being, PLAT_ORION seems like the right common denominator for all these platforms. Best regards, Thomas
On Tue, Feb 18, 2014 at 03:10:27PM +0100, Thomas Petazzoni wrote: [..] > > At the very end of the clean up, when even Orion5x support will be > merged in mach-mvebu/, then we can certainly replace these dependencies > by a "depends on ARCH_MVEBU". But for the time being, PLAT_ORION seems > like the right common denominator for all these platforms. > Last time we talked about this with Sebastian and Andrew it was decided that the right choice is: MACH_KIRKWOOD || MACH_DOVE || MACH_ARMADA_370_XP Which would become MACH_MVEBU. Of course, this is near the nitpick boundary, so AFAIC you can leave PLAT_ORION as in v2, if you feel like.
Dear Ezequiel Garcia, On Tue, 18 Feb 2014 12:45:12 -0300, Ezequiel Garcia wrote: > > At the very end of the clean up, when even Orion5x support will be > > merged in mach-mvebu/, then we can certainly replace these dependencies > > by a "depends on ARCH_MVEBU". But for the time being, PLAT_ORION seems > > like the right common denominator for all these platforms. > > > > Last time we talked about this with Sebastian and Andrew it was decided > that the right choice is: > > MACH_KIRKWOOD || MACH_DOVE || MACH_ARMADA_370_XP And why not Orion5x and MV78xx0 ? > > Which would become MACH_MVEBU. > > Of course, this is near the nitpick boundary, so AFAIC you can leave > PLAT_ORION as in v2, if you feel like. As I suggested previously, PLAT_ORION is what *today* controls the visibility of all Marvell EBU drivers. Please grep for PLAT_ORION in your kernel tree. So why on earth would we invent something completely different for mvneta? When tomorrow ARCH_MVEBU will be used for all mvebu platforms, then indeed PLAT_ORION can be changed to ARCH_MVEBU. Your suggestion above to use MACH_ARMADA_370_XP is *precisely* what this patch is removing, because MACH_ARMADA_370_XP is being removed from arch/arm/mach-mvebu/. Thomas
On Tue, Feb 18, 2014 at 04:51:28PM +0100, Thomas Petazzoni wrote: > On Tue, 18 Feb 2014 12:45:12 -0300, Ezequiel Garcia wrote: > > > > At the very end of the clean up, when even Orion5x support will be > > > merged in mach-mvebu/, then we can certainly replace these dependencies > > > by a "depends on ARCH_MVEBU". But for the time being, PLAT_ORION seems > > > like the right common denominator for all these platforms. > > > > > > > Last time we talked about this with Sebastian and Andrew it was decided > > that the right choice is: > > > > MACH_KIRKWOOD || MACH_DOVE || MACH_ARMADA_370_XP > > And why not Orion5x and MV78xx0 ? > Ouch, I confused s/MARCH/ARCH. What I meant to say is that instead of PLAT_ORION, it was agreed to use: ARCH_ORION5X || ARCH_KIRKWOOD || ARCH_DOVE || ARCH_MVEBU See here for the previous discussion: http://www.spinics.net/lists/devicetree/msg19091.html We discussed the issue on its own thread: http://www.spinics.net/lists/devicetree/msg19159.html > > > > Which would become MACH_MVEBU. > > > > Of course, this is near the nitpick boundary, so AFAIC you can leave > > PLAT_ORION as in v2, if you feel like. > > As I suggested previously, PLAT_ORION is what *today* controls the > visibility of all Marvell EBU drivers. Please grep for PLAT_ORION in > your kernel tree. Again, it has been agreed that those are all wrong and should all be replaced by proper ARCH_whatever. Then, we'll probably be able to stop selecting PLAT_ORION from ARCH_MVEBU. > So why on earth would we invent something completely > different for mvneta? > I don't have a strong opinion, it just feels wrong to have a different rule each time. If you look at the watchdog patches, you'll notice we've changed it to depend on ARCH_whatever. So now you propose to go with PLAT_ORION. I'm fine with either of them; but I think we should decide between one of them and stick to it, instead of each of us having its own rule.
Dear Thomas, On Tue, 18 Feb 2014 05:13:03 -0800 Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: > Dear Jason Cooper, > > On Tue, 18 Feb 2014 07:52:06 -0500, Jason Cooper wrote: > > > > No, I haven't tested all the build scenarios of course. If you feel > > > that this is too dangerous, I can resend a patch that replaces 'depends > > > on ARMADA_370_XP' by 'depends on PLAT_ORION'. Would this be OK for you? > > > > I'm fine either way. I'm definitely a "What's in the box?" kind of guy. > > But I do like being prepared. > > And it's all in your honour! > > > The real question is: Do you think this IP block will ever be found on > > anything other than Marvell Armada boards? If so, stick with this > > version and build test the crap out of it. Otherwise, let's save > > opening the box for another day. > > At this point, I don't expect to see this IP in any other SOCs than > Marvell Armada EBU, so I'll resend with a "depends on PLAT_ORION", as I This IP is used in Marvell Berlin SoCs too ;) But Berlin SoC doesn't have a mbus concept. Currently, we just hack the mvneta to configure the mbus window. Thanks, Jisheng
Dear Jisheng Zhang, On Wed, 19 Feb 2014 10:10:43 +0800, Jisheng Zhang wrote: > > At this point, I don't expect to see this IP in any other SOCs than > > Marvell Armada EBU, so I'll resend with a "depends on PLAT_ORION", as I > > This IP is used in Marvell Berlin SoCs too ;) Ah good to know! Then when this IP will be enabled on Marvell Berlin SoCs, we can extend the "depends on PLAT_ORION" to "depends on PLAT_ORION || ARCH_BERLIN". > But Berlin SoC doesn't have a mbus concept. Currently, we just hack the > mvneta to configure the mbus window. We can simply introduce a separate compatible string, and make the driver behave differently depending on which compatible string was used to probe the device. Best regards, Thomas
diff --git a/drivers/net/ethernet/marvell/Kconfig b/drivers/net/ethernet/marvell/Kconfig index 6300fd2..97b91f7 100644 --- a/drivers/net/ethernet/marvell/Kconfig +++ b/drivers/net/ethernet/marvell/Kconfig @@ -43,12 +43,11 @@ config MVMDIO This driver is used by the MV643XX_ETH and MVNETA drivers. config MVNETA - tristate "Marvell Armada 370/XP network interface support" - depends on MACH_ARMADA_370_XP + tristate "Marvell Armada 370/38x/XP network interface support" select MVMDIO ---help--- This driver supports the network interface units in the - Marvell ARMADA XP and ARMADA 370 SoC family. + Marvell ARMADA XP, ARMADA 370 and Armada 38x SoC family. Note that this driver is distinct from the mv643xx_eth driver, which should be used for the older Marvell SoCs
With the introduction of the support for Armada 375 and Armada 38x, the hidden Kconfig option MACH_ARMADA_370_XP is being renamed to MACH_MVEBU_V7. Therefore, the dependency that was used for the mvneta driver can no longer work. However, such a dependency is not really necessary: there is no point in preventing this driver from being built in other situations, just like the mv643xx_eth driver. As a consequence, this commit removes the unnecessary Kconfig dependency. In addition to this, it takes this opportunity to adjust the description and help text to indicate that the driver can is also used for Armada 38x. Note that Armada 375 cannot use this driver as it has a completely different networking unit, which will require a separate driver. Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> --- drivers/net/ethernet/marvell/Kconfig | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)