Netdev Archive on lore.kernel.org
 help / color / Atom feed
* net: macb: Fix compilation on systems without COMMON_CLK, v2
@ 2019-06-25  8:48 Palmer Dabbelt
  2019-06-25  8:48 ` [PATCH v2 1/2] net: macb: Kconfig: Make MACB depend on COMMON_CLK Palmer Dabbelt
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Palmer Dabbelt @ 2019-06-25  8:48 UTC (permalink / raw)
  To: nicolas.ferre, harinik; +Cc: davem, netdev, linux-kernel

Our patch to add support for the FU540-C000 broke compilation on at
least powerpc allyesconfig, which was found as part of the linux-next
build regression tests.  This must have somehow slipped through the
cracks, as the patch has been reverted in linux-next for a while now.
This patch applies on top of the offending commit, which is the only one
I've even tried it on as I'm not sure how this subsystem makes it to
Linus.

This patch set fixes the issue by adding a dependency of COMMON_CLK to
the MACB Kconfig entry, which avoids the build failure by disabling MACB
on systems where it wouldn't compile.  All known users of MACB have
COMMON_CLK, so this shouldn't cause any issues.  This is a significantly
simpler approach than disabling just the FU540-C000 support.

I've also included a second patch to indicate this is a driver for a
Cadence device that was originally written by an engineer at Atmel.  The
only relation is that I stumbled across it when writing the first patch.

Changes since v1 <20190624061603.1704-1-palmer@sifive.com>:

* Disable MACB on systems without COMMON_CLK, instead of just disabling
  the FU540-C000 support on these systems.
* Update the commit message to reflect the driver was written by Atmel.



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 1/2] net: macb: Kconfig: Make MACB depend on COMMON_CLK
  2019-06-25  8:48 net: macb: Fix compilation on systems without COMMON_CLK, v2 Palmer Dabbelt
@ 2019-06-25  8:48 ` Palmer Dabbelt
  2019-06-25  8:54   ` Nicolas.Ferre
  2019-06-25  8:48 ` [PATCH v2 2/2] net: macb: Kconfig: Rename Atmel to Cadence Palmer Dabbelt
  2019-06-26 21:09 ` net: macb: Fix compilation on systems without COMMON_CLK, v2 David Miller
  2 siblings, 1 reply; 6+ messages in thread
From: Palmer Dabbelt @ 2019-06-25  8:48 UTC (permalink / raw)
  To: nicolas.ferre, harinik; +Cc: davem, netdev, linux-kernel, Palmer Dabbelt

commit c218ad559020 ("macb: Add support for SiFive FU540-C000") added a
dependency on the common clock framework to the macb driver, but didn't
express that dependency in Kconfig.  As a result macb now fails to
compile on systems without COMMON_CLK, which specifically causes a build
failure on powerpc allyesconfig.

This patch adds the dependency, which results in the macb driver no
longer being selectable on systems without the common clock framework.
All known systems that have this device already support the common clock
framework, so this should not cause trouble for any uses.  Supporting
both the FU540-C000 and systems without COMMON_CLK is quite ugly.

I've build tested this on powerpc allyesconfig and RISC-V defconfig
(which selects MACB), but I have not even booted the resulting kernels.

Fixes: c218ad559020 ("macb: Add support for SiFive FU540-C000")
Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
---
 drivers/net/ethernet/cadence/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/cadence/Kconfig b/drivers/net/ethernet/cadence/Kconfig
index 1766697c9c5a..64d8d6ee7739 100644
--- a/drivers/net/ethernet/cadence/Kconfig
+++ b/drivers/net/ethernet/cadence/Kconfig
@@ -21,7 +21,7 @@ if NET_VENDOR_CADENCE
 
 config MACB
 	tristate "Cadence MACB/GEM support"
-	depends on HAS_DMA
+	depends on HAS_DMA && COMMON_CLK
 	select PHYLIB
 	---help---
 	  The Cadence MACB ethernet interface is found on many Atmel AT32 and
@@ -42,7 +42,7 @@ config MACB_USE_HWSTAMP
 
 config MACB_PCI
 	tristate "Cadence PCI MACB/GEM support"
-	depends on MACB && PCI && COMMON_CLK
+	depends on MACB && PCI
 	---help---
 	  This is PCI wrapper for MACB driver.
 
-- 
2.21.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 2/2] net: macb: Kconfig: Rename Atmel to Cadence
  2019-06-25  8:48 net: macb: Fix compilation on systems without COMMON_CLK, v2 Palmer Dabbelt
  2019-06-25  8:48 ` [PATCH v2 1/2] net: macb: Kconfig: Make MACB depend on COMMON_CLK Palmer Dabbelt
@ 2019-06-25  8:48 ` Palmer Dabbelt
  2019-06-25  8:54   ` Nicolas.Ferre
  2019-06-26 21:09 ` net: macb: Fix compilation on systems without COMMON_CLK, v2 David Miller
  2 siblings, 1 reply; 6+ messages in thread
From: Palmer Dabbelt @ 2019-06-25  8:48 UTC (permalink / raw)
  To: nicolas.ferre, harinik; +Cc: davem, netdev, linux-kernel, Palmer Dabbelt

The help text makes it look like NET_VENDOR_CADENCE enables support for
Atmel devices, when in reality it's a driver written by Atmel that
supports Cadence devices.  This may confuse users that have this device
on a non-Atmel SoC.

The fix is just s/Atmel/Cadence/, but I did go and re-wrap the Kconfig
help text as that change caused it to go over 80 characters.

Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
---
 drivers/net/ethernet/cadence/Kconfig | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/cadence/Kconfig b/drivers/net/ethernet/cadence/Kconfig
index 64d8d6ee7739..f4b3bd85dfe3 100644
--- a/drivers/net/ethernet/cadence/Kconfig
+++ b/drivers/net/ethernet/cadence/Kconfig
@@ -1,6 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0-only
 #
-# Atmel device configuration
+# Cadence device configuration
 #
 
 config NET_VENDOR_CADENCE
@@ -13,8 +13,8 @@ config NET_VENDOR_CADENCE
 	  If unsure, say Y.
 
 	  Note that the answer to this question doesn't directly affect the
-	  kernel: saying N will just cause the configurator to skip all
-	  the remaining Atmel network card questions. If you say Y, you will be
+	  kernel: saying N will just cause the configurator to skip all the
+	  remaining Cadence network card questions. If you say Y, you will be
 	  asked for your specific card in the following questions.
 
 if NET_VENDOR_CADENCE
-- 
2.21.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/2] net: macb: Kconfig: Make MACB depend on COMMON_CLK
  2019-06-25  8:48 ` [PATCH v2 1/2] net: macb: Kconfig: Make MACB depend on COMMON_CLK Palmer Dabbelt
@ 2019-06-25  8:54   ` Nicolas.Ferre
  0 siblings, 0 replies; 6+ messages in thread
From: Nicolas.Ferre @ 2019-06-25  8:54 UTC (permalink / raw)
  To: palmer, harinik; +Cc: davem, netdev, linux-kernel

On 25/06/2019 at 10:48, Palmer Dabbelt wrote:
> commit c218ad559020 ("macb: Add support for SiFive FU540-C000") added a
> dependency on the common clock framework to the macb driver, but didn't
> express that dependency in Kconfig.  As a result macb now fails to
> compile on systems without COMMON_CLK, which specifically causes a build
> failure on powerpc allyesconfig.
> 
> This patch adds the dependency, which results in the macb driver no
> longer being selectable on systems without the common clock framework.
> All known systems that have this device already support the common clock
> framework, so this should not cause trouble for any uses.  Supporting
> both the FU540-C000 and systems without COMMON_CLK is quite ugly.
> 
> I've build tested this on powerpc allyesconfig and RISC-V defconfig
> (which selects MACB), but I have not even booted the resulting kernels.
> 
> Fixes: c218ad559020 ("macb: Add support for SiFive FU540-C000")
> Signed-off-by: Palmer Dabbelt <palmer@sifive.com>

Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>

Thanks!

> ---
>   drivers/net/ethernet/cadence/Kconfig | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/Kconfig b/drivers/net/ethernet/cadence/Kconfig
> index 1766697c9c5a..64d8d6ee7739 100644
> --- a/drivers/net/ethernet/cadence/Kconfig
> +++ b/drivers/net/ethernet/cadence/Kconfig
> @@ -21,7 +21,7 @@ if NET_VENDOR_CADENCE
>   
>   config MACB
>   	tristate "Cadence MACB/GEM support"
> -	depends on HAS_DMA
> +	depends on HAS_DMA && COMMON_CLK
>   	select PHYLIB
>   	---help---
>   	  The Cadence MACB ethernet interface is found on many Atmel AT32 and
> @@ -42,7 +42,7 @@ config MACB_USE_HWSTAMP
>   
>   config MACB_PCI
>   	tristate "Cadence PCI MACB/GEM support"
> -	depends on MACB && PCI && COMMON_CLK
> +	depends on MACB && PCI
>   	---help---
>   	  This is PCI wrapper for MACB driver.
>   
> 


-- 
Nicolas Ferre

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 2/2] net: macb: Kconfig: Rename Atmel to Cadence
  2019-06-25  8:48 ` [PATCH v2 2/2] net: macb: Kconfig: Rename Atmel to Cadence Palmer Dabbelt
@ 2019-06-25  8:54   ` Nicolas.Ferre
  0 siblings, 0 replies; 6+ messages in thread
From: Nicolas.Ferre @ 2019-06-25  8:54 UTC (permalink / raw)
  To: palmer, harinik; +Cc: davem, netdev, linux-kernel

On 25/06/2019 at 10:48, Palmer Dabbelt wrote:
> The help text makes it look like NET_VENDOR_CADENCE enables support for
> Atmel devices, when in reality it's a driver written by Atmel that
> supports Cadence devices.  This may confuse users that have this device
> on a non-Atmel SoC.
> 
> The fix is just s/Atmel/Cadence/, but I did go and re-wrap the Kconfig
> help text as that change caused it to go over 80 characters.
> 
> Signed-off-by: Palmer Dabbelt <palmer@sifive.com>

Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>

> ---
>   drivers/net/ethernet/cadence/Kconfig | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/Kconfig b/drivers/net/ethernet/cadence/Kconfig
> index 64d8d6ee7739..f4b3bd85dfe3 100644
> --- a/drivers/net/ethernet/cadence/Kconfig
> +++ b/drivers/net/ethernet/cadence/Kconfig
> @@ -1,6 +1,6 @@
>   # SPDX-License-Identifier: GPL-2.0-only
>   #
> -# Atmel device configuration
> +# Cadence device configuration
>   #
>   
>   config NET_VENDOR_CADENCE
> @@ -13,8 +13,8 @@ config NET_VENDOR_CADENCE
>   	  If unsure, say Y.
>   
>   	  Note that the answer to this question doesn't directly affect the
> -	  kernel: saying N will just cause the configurator to skip all
> -	  the remaining Atmel network card questions. If you say Y, you will be
> +	  kernel: saying N will just cause the configurator to skip all the
> +	  remaining Cadence network card questions. If you say Y, you will be
>   	  asked for your specific card in the following questions.
>   
>   if NET_VENDOR_CADENCE
> 


-- 
Nicolas Ferre

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: net: macb: Fix compilation on systems without COMMON_CLK, v2
  2019-06-25  8:48 net: macb: Fix compilation on systems without COMMON_CLK, v2 Palmer Dabbelt
  2019-06-25  8:48 ` [PATCH v2 1/2] net: macb: Kconfig: Make MACB depend on COMMON_CLK Palmer Dabbelt
  2019-06-25  8:48 ` [PATCH v2 2/2] net: macb: Kconfig: Rename Atmel to Cadence Palmer Dabbelt
@ 2019-06-26 21:09 ` David Miller
  2 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2019-06-26 21:09 UTC (permalink / raw)
  To: palmer; +Cc: nicolas.ferre, harinik, netdev, linux-kernel

From: Palmer Dabbelt <palmer@sifive.com>
Date: Tue, 25 Jun 2019 01:48:26 -0700

> Our patch to add support for the FU540-C000 broke compilation on at
> least powerpc allyesconfig, which was found as part of the linux-next
> build regression tests.  This must have somehow slipped through the
> cracks, as the patch has been reverted in linux-next for a while now.
> This patch applies on top of the offending commit, which is the only one
> I've even tried it on as I'm not sure how this subsystem makes it to
> Linus.
> 
> This patch set fixes the issue by adding a dependency of COMMON_CLK to
> the MACB Kconfig entry, which avoids the build failure by disabling MACB
> on systems where it wouldn't compile.  All known users of MACB have
> COMMON_CLK, so this shouldn't cause any issues.  This is a significantly
> simpler approach than disabling just the FU540-C000 support.
> 
> I've also included a second patch to indicate this is a driver for a
> Cadence device that was originally written by an engineer at Atmel.  The
> only relation is that I stumbled across it when writing the first patch.
> 
> Changes since v1 <20190624061603.1704-1-palmer@sifive.com>:
> 
> * Disable MACB on systems without COMMON_CLK, instead of just disabling
>   the FU540-C000 support on these systems.
> * Update the commit message to reflect the driver was written by Atmel.

Series applied, thanks.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-25  8:48 net: macb: Fix compilation on systems without COMMON_CLK, v2 Palmer Dabbelt
2019-06-25  8:48 ` [PATCH v2 1/2] net: macb: Kconfig: Make MACB depend on COMMON_CLK Palmer Dabbelt
2019-06-25  8:54   ` Nicolas.Ferre
2019-06-25  8:48 ` [PATCH v2 2/2] net: macb: Kconfig: Rename Atmel to Cadence Palmer Dabbelt
2019-06-25  8:54   ` Nicolas.Ferre
2019-06-26 21:09 ` net: macb: Fix compilation on systems without COMMON_CLK, v2 David Miller

Netdev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/netdev/0 netdev/git/0.git
	git clone --mirror https://lore.kernel.org/netdev/1 netdev/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 netdev netdev/ https://lore.kernel.org/netdev \
		netdev@vger.kernel.org netdev@archiver.kernel.org
	public-inbox-index netdev


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.netdev


AGPL code for this site: git clone https://public-inbox.org/ public-inbox