[PATCH] net: ethernet: remove unneeded dependency of mvneta and update help text

[PATCH] net: ethernet: remove unneeded dependency of mvneta and update help text

[Thomas Petazzoni]

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
+	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
-- 
1.8.3.2

Re: [PATCH] net: ethernet: remove unneeded dependency of mvneta and update help text

[Jason Cooper]

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.

Re: [PATCH] net: ethernet: remove unneeded dependency of mvneta and update help text

[Thomas Petazzoni]

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
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH] net: ethernet: remove unneeded dependency of mvneta and update help text

[Jason Cooper]

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.

Re: [PATCH] net: ethernet: remove unneeded dependency of mvneta and update help text

[Thomas Petazzoni]

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
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH] net: ethernet: remove unneeded dependency of mvneta and update help text

[Andrew Lunn]

> 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

Re: [PATCH] net: ethernet: remove unneeded dependency of mvneta and update help text

[Thomas Petazzoni]

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
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH] net: ethernet: remove unneeded dependency of mvneta and update help text

[Ezequiel Garcia]

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.
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [PATCH] net: ethernet: remove unneeded dependency of mvneta and update help text

[Thomas Petazzoni]

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
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH] net: ethernet: remove unneeded dependency of mvneta and update help text

[Ezequiel Garcia]

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.
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [PATCH] net: ethernet: remove unneeded dependency of mvneta and update help text

[Jisheng Zhang]

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

Re: [PATCH] net: ethernet: remove unneeded dependency of mvneta and update help text

[Thomas Petazzoni]

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
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com