All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] ARM: at91: Fix link breakage when !CONFIG_PHYLIB
@ 2013-05-27 10:48 ` Alexandre Belloni
  0 siblings, 0 replies; 24+ messages in thread
From: Alexandre Belloni @ 2013-05-27 10:48 UTC (permalink / raw)
  To: Nicolas Ferre
  Cc: Jean-Christophe Plagniol-Villard, linux-arm-kernel, linux-kernel,
	Ludovic Desroches, Alexandre Belloni

Fixes:
arch/arm/mach-at91/built-in.o: In function `ksz9021rn_phy_fixup':
:(.text+0x1174): undefined reference to `mdiobus_write'
:(.text+0x1188): undefined reference to `mdiobus_write'
:(.text+0x119c): undefined reference to `mdiobus_write'
:(.text+0x11b0): undefined reference to `mdiobus_write'
arch/arm/mach-at91/built-in.o: In function `sama5_dt_device_init':
:(.init.text+0x1e34): undefined reference to `phy_register_fixup_for_uid'

when CONFIG_PHYLIB is not selected.

Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Acked-by: Ludovic Desroches <ludovic.desroches@atmel.com>
---
Changes in v3:
	added SoB
	added Acked-By

Changes in v2:
	use IS_BUILTIN
	use CONFIG_PHYLIB and not CONFIG_PHY

 arch/arm/mach-at91/board-dt-sama5.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/arch/arm/mach-at91/board-dt-sama5.c b/arch/arm/mach-at91/board-dt-sama5.c
index 705305e..e9ce541 100644
--- a/arch/arm/mach-at91/board-dt-sama5.c
+++ b/arch/arm/mach-at91/board-dt-sama5.c
@@ -47,22 +47,24 @@ static int ksz9021rn_phy_fixup(struct phy_device *phy)
 #define GMII_ERCR	11
 #define GMII_ERDWR	12
 
-	/* Set delay values */
-	value = GMII_RCCPSR | 0x8000;
-	phy_write(phy, GMII_ERCR, value);
-	value = 0xF2F4;
-	phy_write(phy, GMII_ERDWR, value);
-	value = GMII_RRDPSR | 0x8000;
-	phy_write(phy, GMII_ERCR, value);
-	value = 0x2222;
-	phy_write(phy, GMII_ERDWR, value);
+	if (IS_BUILTIN(CONFIG_PHYLIB)) {
+		/* Set delay values */
+		value = GMII_RCCPSR | 0x8000;
+		phy_write(phy, GMII_ERCR, value);
+		value = 0xF2F4;
+		phy_write(phy, GMII_ERDWR, value);
+		value = GMII_RRDPSR | 0x8000;
+		phy_write(phy, GMII_ERCR, value);
+		value = 0x2222;
+		phy_write(phy, GMII_ERDWR, value);
+	}
 
 	return 0;
 }
 
 static void __init sama5_dt_device_init(void)
 {
-	if (of_machine_is_compatible("atmel,sama5d3xcm"))
+	if (of_machine_is_compatible("atmel,sama5d3xcm") && IS_BUILTIN(CONFIG_PHYLIB))
 		phy_register_fixup_for_uid(PHY_ID_KSZ9021, MICREL_PHY_ID_MASK,
 			ksz9021rn_phy_fixup);
 
-- 
1.8.1.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v3] ARM: at91: Fix link breakage when !CONFIG_PHYLIB
@ 2013-05-27 10:48 ` Alexandre Belloni
  0 siblings, 0 replies; 24+ messages in thread
From: Alexandre Belloni @ 2013-05-27 10:48 UTC (permalink / raw)
  To: linux-arm-kernel

Fixes:
arch/arm/mach-at91/built-in.o: In function `ksz9021rn_phy_fixup':
:(.text+0x1174): undefined reference to `mdiobus_write'
:(.text+0x1188): undefined reference to `mdiobus_write'
:(.text+0x119c): undefined reference to `mdiobus_write'
:(.text+0x11b0): undefined reference to `mdiobus_write'
arch/arm/mach-at91/built-in.o: In function `sama5_dt_device_init':
:(.init.text+0x1e34): undefined reference to `phy_register_fixup_for_uid'

when CONFIG_PHYLIB is not selected.

Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Acked-by: Ludovic Desroches <ludovic.desroches@atmel.com>
---
Changes in v3:
	added SoB
	added Acked-By

Changes in v2:
	use IS_BUILTIN
	use CONFIG_PHYLIB and not CONFIG_PHY

 arch/arm/mach-at91/board-dt-sama5.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/arch/arm/mach-at91/board-dt-sama5.c b/arch/arm/mach-at91/board-dt-sama5.c
index 705305e..e9ce541 100644
--- a/arch/arm/mach-at91/board-dt-sama5.c
+++ b/arch/arm/mach-at91/board-dt-sama5.c
@@ -47,22 +47,24 @@ static int ksz9021rn_phy_fixup(struct phy_device *phy)
 #define GMII_ERCR	11
 #define GMII_ERDWR	12
 
-	/* Set delay values */
-	value = GMII_RCCPSR | 0x8000;
-	phy_write(phy, GMII_ERCR, value);
-	value = 0xF2F4;
-	phy_write(phy, GMII_ERDWR, value);
-	value = GMII_RRDPSR | 0x8000;
-	phy_write(phy, GMII_ERCR, value);
-	value = 0x2222;
-	phy_write(phy, GMII_ERDWR, value);
+	if (IS_BUILTIN(CONFIG_PHYLIB)) {
+		/* Set delay values */
+		value = GMII_RCCPSR | 0x8000;
+		phy_write(phy, GMII_ERCR, value);
+		value = 0xF2F4;
+		phy_write(phy, GMII_ERDWR, value);
+		value = GMII_RRDPSR | 0x8000;
+		phy_write(phy, GMII_ERCR, value);
+		value = 0x2222;
+		phy_write(phy, GMII_ERDWR, value);
+	}
 
 	return 0;
 }
 
 static void __init sama5_dt_device_init(void)
 {
-	if (of_machine_is_compatible("atmel,sama5d3xcm"))
+	if (of_machine_is_compatible("atmel,sama5d3xcm") && IS_BUILTIN(CONFIG_PHYLIB))
 		phy_register_fixup_for_uid(PHY_ID_KSZ9021, MICREL_PHY_ID_MASK,
 			ksz9021rn_phy_fixup);
 
-- 
1.8.1.2

^ permalink raw reply related	[flat|nested] 24+ messages in thread
* Re: [PATCH v3] ARM: at91: Fix link breakage when !CONFIG_PHYLIB
  2013-05-27 10:48 ` Alexandre Belloni
@ 2013-05-27 11:26   ` Jean-Christophe PLAGNIOL-VILLARD
  -1 siblings, 0 replies; 24+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-05-27 11:26 UTC (permalink / raw)
  To: Alexandre Belloni
  Cc: Jean-Christophe PLAGNIOL-VILLARD, Nicolas Ferre,
	linux-arm-kernel, linux-kernel, Ludovic Desroches


On May 27, 2013, at 6:48 PM, Alexandre Belloni <alexandre.belloni@free-electrons.com> wrote:

> Fixes:
> arch/arm/mach-at91/built-in.o: In function `ksz9021rn_phy_fixup':
> :(.text+0x1174): undefined reference to `mdiobus_write'
> :(.text+0x1188): undefined reference to `mdiobus_write'
> :(.text+0x119c): undefined reference to `mdiobus_write'
> :(.text+0x11b0): undefined reference to `mdiobus_write'
> arch/arm/mach-at91/built-in.o: In function `sama5_dt_device_init':
> :(.init.text+0x1e34): undefined reference to `phy_register_fixup_for_uid'
> 
> when CONFIG_PHYLIB is not selected.
> 
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> Acked-by: Ludovic Desroches <ludovic.desroches@atmel.com>

no for this change

add an inline version of phy_register_fixup_for_uid

this will do the same for every platform

> ---
> Changes in v3:
> 	added SoB
> 	added Acked-By
> 
> Changes in v2:
> 	use IS_BUILTIN
> 	use CONFIG_PHYLIB and not CONFIG_PHY
> 
> arch/arm/mach-at91/board-dt-sama5.c | 22 ++++++++++++----------
> 1 file changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/arm/mach-at91/board-dt-sama5.c b/arch/arm/mach-at91/board-dt-sama5.c
> index 705305e..e9ce541 100644
> --- a/arch/arm/mach-at91/board-dt-sama5.c
> +++ b/arch/arm/mach-at91/board-dt-sama5.c
> @@ -47,22 +47,24 @@ static int ksz9021rn_phy_fixup(struct phy_device *phy)
> #define GMII_ERCR	11
> #define GMII_ERDWR	12
> 
> -	/* Set delay values */
> -	value = GMII_RCCPSR | 0x8000;
> -	phy_write(phy, GMII_ERCR, value);
> -	value = 0xF2F4;
> -	phy_write(phy, GMII_ERDWR, value);
> -	value = GMII_RRDPSR | 0x8000;
> -	phy_write(phy, GMII_ERCR, value);
> -	value = 0x2222;
> -	phy_write(phy, GMII_ERDWR, value);
> +	if (IS_BUILTIN(CONFIG_PHYLIB)) {
> +		/* Set delay values */
> +		value = GMII_RCCPSR | 0x8000;
> +		phy_write(phy, GMII_ERCR, value);
> +		value = 0xF2F4;
> +		phy_write(phy, GMII_ERDWR, value);
> +		value = GMII_RRDPSR | 0x8000;
> +		phy_write(phy, GMII_ERCR, value);
> +		value = 0x2222;
> +		phy_write(phy, GMII_ERDWR, value);
> +	}
> 
> 	return 0;
> }
> 
> static void __init sama5_dt_device_init(void)
> {
> -	if (of_machine_is_compatible("atmel,sama5d3xcm"))
> +	if (of_machine_is_compatible("atmel,sama5d3xcm") && IS_BUILTIN(CONFIG_PHYLIB))
> 		phy_register_fixup_for_uid(PHY_ID_KSZ9021, MICREL_PHY_ID_MASK,
> 			ksz9021rn_phy_fixup);
> 
> -- 
> 1.8.1.2
> 


^ permalink raw reply	[flat|nested] 24+ messages in thread
* [PATCH v3] ARM: at91: Fix link breakage when !CONFIG_PHYLIB
@ 2013-05-27 11:26   ` Jean-Christophe PLAGNIOL-VILLARD
  0 siblings, 0 replies; 24+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-05-27 11:26 UTC (permalink / raw)
  To: linux-arm-kernel


On May 27, 2013, at 6:48 PM, Alexandre Belloni <alexandre.belloni@free-electrons.com> wrote:

> Fixes:
> arch/arm/mach-at91/built-in.o: In function `ksz9021rn_phy_fixup':
> :(.text+0x1174): undefined reference to `mdiobus_write'
> :(.text+0x1188): undefined reference to `mdiobus_write'
> :(.text+0x119c): undefined reference to `mdiobus_write'
> :(.text+0x11b0): undefined reference to `mdiobus_write'
> arch/arm/mach-at91/built-in.o: In function `sama5_dt_device_init':
> :(.init.text+0x1e34): undefined reference to `phy_register_fixup_for_uid'
> 
> when CONFIG_PHYLIB is not selected.
> 
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> Acked-by: Ludovic Desroches <ludovic.desroches@atmel.com>

no for this change

add an inline version of phy_register_fixup_for_uid

this will do the same for every platform

> ---
> Changes in v3:
> 	added SoB
> 	added Acked-By
> 
> Changes in v2:
> 	use IS_BUILTIN
> 	use CONFIG_PHYLIB and not CONFIG_PHY
> 
> arch/arm/mach-at91/board-dt-sama5.c | 22 ++++++++++++----------
> 1 file changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/arm/mach-at91/board-dt-sama5.c b/arch/arm/mach-at91/board-dt-sama5.c
> index 705305e..e9ce541 100644
> --- a/arch/arm/mach-at91/board-dt-sama5.c
> +++ b/arch/arm/mach-at91/board-dt-sama5.c
> @@ -47,22 +47,24 @@ static int ksz9021rn_phy_fixup(struct phy_device *phy)
> #define GMII_ERCR	11
> #define GMII_ERDWR	12
> 
> -	/* Set delay values */
> -	value = GMII_RCCPSR | 0x8000;
> -	phy_write(phy, GMII_ERCR, value);
> -	value = 0xF2F4;
> -	phy_write(phy, GMII_ERDWR, value);
> -	value = GMII_RRDPSR | 0x8000;
> -	phy_write(phy, GMII_ERCR, value);
> -	value = 0x2222;
> -	phy_write(phy, GMII_ERDWR, value);
> +	if (IS_BUILTIN(CONFIG_PHYLIB)) {
> +		/* Set delay values */
> +		value = GMII_RCCPSR | 0x8000;
> +		phy_write(phy, GMII_ERCR, value);
> +		value = 0xF2F4;
> +		phy_write(phy, GMII_ERDWR, value);
> +		value = GMII_RRDPSR | 0x8000;
> +		phy_write(phy, GMII_ERCR, value);
> +		value = 0x2222;
> +		phy_write(phy, GMII_ERDWR, value);
> +	}
> 
> 	return 0;
> }
> 
> static void __init sama5_dt_device_init(void)
> {
> -	if (of_machine_is_compatible("atmel,sama5d3xcm"))
> +	if (of_machine_is_compatible("atmel,sama5d3xcm") && IS_BUILTIN(CONFIG_PHYLIB))
> 		phy_register_fixup_for_uid(PHY_ID_KSZ9021, MICREL_PHY_ID_MASK,
> 			ksz9021rn_phy_fixup);
> 
> -- 
> 1.8.1.2
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v3] ARM: at91: Fix link breakage when !CONFIG_PHYLIB
  2013-05-27 10:48 ` Alexandre Belloni
@ 2013-05-27 12:05   ` Nicolas Ferre
  -1 siblings, 0 replies; 24+ messages in thread
From: Nicolas Ferre @ 2013-05-27 12:05 UTC (permalink / raw)
  To: Alexandre Belloni, Jean-Christophe Plagniol-Villard
  Cc: linux-arm-kernel, linux-kernel, Ludovic Desroches

On 27/05/2013 12:48, Alexandre Belloni :
> Fixes:
> arch/arm/mach-at91/built-in.o: In function `ksz9021rn_phy_fixup':
> :(.text+0x1174): undefined reference to `mdiobus_write'
> :(.text+0x1188): undefined reference to `mdiobus_write'
> :(.text+0x119c): undefined reference to `mdiobus_write'
> :(.text+0x11b0): undefined reference to `mdiobus_write'
> arch/arm/mach-at91/built-in.o: In function `sama5_dt_device_init':
> :(.init.text+0x1e34): undefined reference to `phy_register_fixup_for_uid'
>
> when CONFIG_PHYLIB is not selected.
>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> Acked-by: Ludovic Desroches <ludovic.desroches@atmel.com>
> ---
> Changes in v3:
> 	added SoB
> 	added Acked-By

A v3 is not needed simply for this. Thanks, anyway!

One question though: why not just add

         select PHYLIB

In Kconfig entry:

  config MACH_SAMA5_DT

?

(note that I like to play with everybody's nerves ;-))

Bye,

> Changes in v2:
> 	use IS_BUILTIN
> 	use CONFIG_PHYLIB and not CONFIG_PHY
>
>   arch/arm/mach-at91/board-dt-sama5.c | 22 ++++++++++++----------
>   1 file changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/arch/arm/mach-at91/board-dt-sama5.c b/arch/arm/mach-at91/board-dt-sama5.c
> index 705305e..e9ce541 100644
> --- a/arch/arm/mach-at91/board-dt-sama5.c
> +++ b/arch/arm/mach-at91/board-dt-sama5.c
> @@ -47,22 +47,24 @@ static int ksz9021rn_phy_fixup(struct phy_device *phy)
>   #define GMII_ERCR	11
>   #define GMII_ERDWR	12
>
> -	/* Set delay values */
> -	value = GMII_RCCPSR | 0x8000;
> -	phy_write(phy, GMII_ERCR, value);
> -	value = 0xF2F4;
> -	phy_write(phy, GMII_ERDWR, value);
> -	value = GMII_RRDPSR | 0x8000;
> -	phy_write(phy, GMII_ERCR, value);
> -	value = 0x2222;
> -	phy_write(phy, GMII_ERDWR, value);
> +	if (IS_BUILTIN(CONFIG_PHYLIB)) {
> +		/* Set delay values */
> +		value = GMII_RCCPSR | 0x8000;
> +		phy_write(phy, GMII_ERCR, value);
> +		value = 0xF2F4;
> +		phy_write(phy, GMII_ERDWR, value);
> +		value = GMII_RRDPSR | 0x8000;
> +		phy_write(phy, GMII_ERCR, value);
> +		value = 0x2222;
> +		phy_write(phy, GMII_ERDWR, value);
> +	}
>
>   	return 0;
>   }
>
>   static void __init sama5_dt_device_init(void)
>   {
> -	if (of_machine_is_compatible("atmel,sama5d3xcm"))
> +	if (of_machine_is_compatible("atmel,sama5d3xcm") && IS_BUILTIN(CONFIG_PHYLIB))
>   		phy_register_fixup_for_uid(PHY_ID_KSZ9021, MICREL_PHY_ID_MASK,
>   			ksz9021rn_phy_fixup);
>
>


-- 
Nicolas Ferre

^ permalink raw reply	[flat|nested] 24+ messages in thread
* [PATCH v3] ARM: at91: Fix link breakage when !CONFIG_PHYLIB
@ 2013-05-27 12:05   ` Nicolas Ferre
  0 siblings, 0 replies; 24+ messages in thread
From: Nicolas Ferre @ 2013-05-27 12:05 UTC (permalink / raw)
  To: linux-arm-kernel

On 27/05/2013 12:48, Alexandre Belloni :
> Fixes:
> arch/arm/mach-at91/built-in.o: In function `ksz9021rn_phy_fixup':
> :(.text+0x1174): undefined reference to `mdiobus_write'
> :(.text+0x1188): undefined reference to `mdiobus_write'
> :(.text+0x119c): undefined reference to `mdiobus_write'
> :(.text+0x11b0): undefined reference to `mdiobus_write'
> arch/arm/mach-at91/built-in.o: In function `sama5_dt_device_init':
> :(.init.text+0x1e34): undefined reference to `phy_register_fixup_for_uid'
>
> when CONFIG_PHYLIB is not selected.
>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> Acked-by: Ludovic Desroches <ludovic.desroches@atmel.com>
> ---
> Changes in v3:
> 	added SoB
> 	added Acked-By

A v3 is not needed simply for this. Thanks, anyway!

One question though: why not just add

         select PHYLIB

In Kconfig entry:

  config MACH_SAMA5_DT

?

(note that I like to play with everybody's nerves ;-))

Bye,

> Changes in v2:
> 	use IS_BUILTIN
> 	use CONFIG_PHYLIB and not CONFIG_PHY
>
>   arch/arm/mach-at91/board-dt-sama5.c | 22 ++++++++++++----------
>   1 file changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/arch/arm/mach-at91/board-dt-sama5.c b/arch/arm/mach-at91/board-dt-sama5.c
> index 705305e..e9ce541 100644
> --- a/arch/arm/mach-at91/board-dt-sama5.c
> +++ b/arch/arm/mach-at91/board-dt-sama5.c
> @@ -47,22 +47,24 @@ static int ksz9021rn_phy_fixup(struct phy_device *phy)
>   #define GMII_ERCR	11
>   #define GMII_ERDWR	12
>
> -	/* Set delay values */
> -	value = GMII_RCCPSR | 0x8000;
> -	phy_write(phy, GMII_ERCR, value);
> -	value = 0xF2F4;
> -	phy_write(phy, GMII_ERDWR, value);
> -	value = GMII_RRDPSR | 0x8000;
> -	phy_write(phy, GMII_ERCR, value);
> -	value = 0x2222;
> -	phy_write(phy, GMII_ERDWR, value);
> +	if (IS_BUILTIN(CONFIG_PHYLIB)) {
> +		/* Set delay values */
> +		value = GMII_RCCPSR | 0x8000;
> +		phy_write(phy, GMII_ERCR, value);
> +		value = 0xF2F4;
> +		phy_write(phy, GMII_ERDWR, value);
> +		value = GMII_RRDPSR | 0x8000;
> +		phy_write(phy, GMII_ERCR, value);
> +		value = 0x2222;
> +		phy_write(phy, GMII_ERDWR, value);
> +	}
>
>   	return 0;
>   }
>
>   static void __init sama5_dt_device_init(void)
>   {
> -	if (of_machine_is_compatible("atmel,sama5d3xcm"))
> +	if (of_machine_is_compatible("atmel,sama5d3xcm") && IS_BUILTIN(CONFIG_PHYLIB))
>   		phy_register_fixup_for_uid(PHY_ID_KSZ9021, MICREL_PHY_ID_MASK,
>   			ksz9021rn_phy_fixup);
>
>


-- 
Nicolas Ferre

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v3] ARM: at91: Fix link breakage when !CONFIG_PHYLIB
  2013-05-27 12:05   ` Nicolas Ferre
@ 2013-05-27 12:42     ` Arnd Bergmann
  -1 siblings, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2013-05-27 12:42 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Nicolas Ferre, Alexandre Belloni,
	Jean-Christophe Plagniol-Villard, Ludovic Desroches,
	linux-kernel

On Monday 27 May 2013 14:05:13 Nicolas Ferre wrote:
> On 27/05/2013 12:48, Alexandre Belloni :
> A v3 is not needed simply for this. Thanks, anyway!
> 
> One question though: why not just add
> 
>          select PHYLIB
> 
> In Kconfig entry:
> 
>   config MACH_SAMA5_DT
> 
> ?
> 
> (note that I like to play with everybody's nerves ;-))

That would make it impossible to build a sama5 kernel without networking
support, which is probably not what you want.

Jean-Christophe's suggestion to add the inline helper is best. Aside from
that, please note that

> > diff --git a/arch/arm/mach-at91/board-dt-sama5.c b/arch/arm/mach-at91/board-dt-sama5.c
> > index 705305e..e9ce541 100644
> > --- a/arch/arm/mach-at91/board-dt-sama5.c
> > +++ b/arch/arm/mach-at91/board-dt-sama5.c
> > @@ -47,22 +47,24 @@ static int ksz9021rn_phy_fixup(struct phy_device *phy)
> >   #define GMII_ERCR	11
> >   #define GMII_ERDWR	12
> >
> > -	/* Set delay values */
> > -	value = GMII_RCCPSR | 0x8000;
> > -	phy_write(phy, GMII_ERCR, value);
> > -	value = 0xF2F4;
> > -	phy_write(phy, GMII_ERDWR, value);
> > -	value = GMII_RRDPSR | 0x8000;
> > -	phy_write(phy, GMII_ERCR, value);
> > -	value = 0x2222;
> > -	phy_write(phy, GMII_ERDWR, value);
> > +	if (IS_BUILTIN(CONFIG_PHYLIB)) {
> > +		/* Set delay values */
> > +		value = GMII_RCCPSR | 0x8000;
> > +		phy_write(phy, GMII_ERCR, value);
> > +		value = 0xF2F4;
> > +		phy_write(phy, GMII_ERDWR, value);
> > +		value = GMII_RRDPSR | 0x8000;
> > +		phy_write(phy, GMII_ERCR, value);
> > +		value = 0x2222;
> > +		phy_write(phy, GMII_ERDWR, value);
> > +	}
> >
> >   	return 0;
> >   }

This part of the patch is not actually needed if you

> >   static void __init sama5_dt_device_init(void)
> >   {
> > -	if (of_machine_is_compatible("atmel,sama5d3xcm"))
> > +	if (of_machine_is_compatible("atmel,sama5d3xcm") && IS_BUILTIN(CONFIG_PHYLIB))
> >   		phy_register_fixup_for_uid(PHY_ID_KSZ9021, MICREL_PHY_ID_MASK,
> >   			ksz9021rn_phy_fixup);

do this change, or the inline helper.

	Arnd

^ permalink raw reply	[flat|nested] 24+ messages in thread
* [PATCH v3] ARM: at91: Fix link breakage when !CONFIG_PHYLIB
@ 2013-05-27 12:42     ` Arnd Bergmann
  0 siblings, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2013-05-27 12:42 UTC (permalink / raw)
  To: linux-arm-kernel

On Monday 27 May 2013 14:05:13 Nicolas Ferre wrote:
> On 27/05/2013 12:48, Alexandre Belloni :
> A v3 is not needed simply for this. Thanks, anyway!
> 
> One question though: why not just add
> 
>          select PHYLIB
> 
> In Kconfig entry:
> 
>   config MACH_SAMA5_DT
> 
> ?
> 
> (note that I like to play with everybody's nerves ;-))

That would make it impossible to build a sama5 kernel without networking
support, which is probably not what you want.

Jean-Christophe's suggestion to add the inline helper is best. Aside from
that, please note that

> > diff --git a/arch/arm/mach-at91/board-dt-sama5.c b/arch/arm/mach-at91/board-dt-sama5.c
> > index 705305e..e9ce541 100644
> > --- a/arch/arm/mach-at91/board-dt-sama5.c
> > +++ b/arch/arm/mach-at91/board-dt-sama5.c
> > @@ -47,22 +47,24 @@ static int ksz9021rn_phy_fixup(struct phy_device *phy)
> >   #define GMII_ERCR	11
> >   #define GMII_ERDWR	12
> >
> > -	/* Set delay values */
> > -	value = GMII_RCCPSR | 0x8000;
> > -	phy_write(phy, GMII_ERCR, value);
> > -	value = 0xF2F4;
> > -	phy_write(phy, GMII_ERDWR, value);
> > -	value = GMII_RRDPSR | 0x8000;
> > -	phy_write(phy, GMII_ERCR, value);
> > -	value = 0x2222;
> > -	phy_write(phy, GMII_ERDWR, value);
> > +	if (IS_BUILTIN(CONFIG_PHYLIB)) {
> > +		/* Set delay values */
> > +		value = GMII_RCCPSR | 0x8000;
> > +		phy_write(phy, GMII_ERCR, value);
> > +		value = 0xF2F4;
> > +		phy_write(phy, GMII_ERDWR, value);
> > +		value = GMII_RRDPSR | 0x8000;
> > +		phy_write(phy, GMII_ERCR, value);
> > +		value = 0x2222;
> > +		phy_write(phy, GMII_ERDWR, value);
> > +	}
> >
> >   	return 0;
> >   }

This part of the patch is not actually needed if you

> >   static void __init sama5_dt_device_init(void)
> >   {
> > -	if (of_machine_is_compatible("atmel,sama5d3xcm"))
> > +	if (of_machine_is_compatible("atmel,sama5d3xcm") && IS_BUILTIN(CONFIG_PHYLIB))
> >   		phy_register_fixup_for_uid(PHY_ID_KSZ9021, MICREL_PHY_ID_MASK,
> >   			ksz9021rn_phy_fixup);

do this change, or the inline helper.

	Arnd

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v3] ARM: at91: Fix link breakage when !CONFIG_PHYLIB
  2013-05-27 12:05   ` Nicolas Ferre
@ 2013-05-27 12:43     ` Jean-Christophe PLAGNIOL-VILLARD
  -1 siblings, 0 replies; 24+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-05-27 12:43 UTC (permalink / raw)
  To: Nicolas Ferre
  Cc: Jean-Christophe PLAGNIOL-VILLARD, Alexandre Belloni,
	linux-arm-kernel, linux-kernel, Ludovic Desroches


On May 27, 2013, at 8:05 PM, Nicolas Ferre <nicolas.ferre@atmel.com> wrote:

> On 27/05/2013 12:48, Alexandre Belloni :
>> Fixes:
>> arch/arm/mach-at91/built-in.o: In function `ksz9021rn_phy_fixup':
>> :(.text+0x1174): undefined reference to `mdiobus_write'
>> :(.text+0x1188): undefined reference to `mdiobus_write'
>> :(.text+0x119c): undefined reference to `mdiobus_write'
>> :(.text+0x11b0): undefined reference to `mdiobus_write'
>> arch/arm/mach-at91/built-in.o: In function `sama5_dt_device_init':
>> :(.init.text+0x1e34): undefined reference to `phy_register_fixup_for_uid'
>> 
>> when CONFIG_PHYLIB is not selected.
>> 
>> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
>> Acked-by: Ludovic Desroches <ludovic.desroches@atmel.com>
>> ---
>> Changes in v3:
>> 	added SoB
>> 	added Acked-By
> 
> A v3 is not needed simply for this. Thanks, anyway!
> 
> One question though: why not just add
> 
>        select PHYLIB
> 
> In Kconfig entry:
> 
> config MACH_SAMA5_DT
> 
> ?
> 
> (note that I like to play with everybody's nerves ;-))
> 

I prefer this than the IS_BUILTIN


> Bye,
> 
>> Changes in v2:
>> 	use IS_BUILTIN
>> 	use CONFIG_PHYLIB and not CONFIG_PHY
>> 
>>  arch/arm/mach-at91/board-dt-sama5.c | 22 ++++++++++++----------
>>  1 file changed, 12 insertions(+), 10 deletions(-)
>> 
>> diff --git a/arch/arm/mach-at91/board-dt-sama5.c b/arch/arm/mach-at91/board-dt-sama5.c
>> index 705305e..e9ce541 100644
>> --- a/arch/arm/mach-at91/board-dt-sama5.c
>> +++ b/arch/arm/mach-at91/board-dt-sama5.c
>> @@ -47,22 +47,24 @@ static int ksz9021rn_phy_fixup(struct phy_device *phy)
>>  #define GMII_ERCR	11
>>  #define GMII_ERDWR	12
>> 
>> -	/* Set delay values */
>> -	value = GMII_RCCPSR | 0x8000;
>> -	phy_write(phy, GMII_ERCR, value);
>> -	value = 0xF2F4;
>> -	phy_write(phy, GMII_ERDWR, value);
>> -	value = GMII_RRDPSR | 0x8000;
>> -	phy_write(phy, GMII_ERCR, value);
>> -	value = 0x2222;
>> -	phy_write(phy, GMII_ERDWR, value);
>> +	if (IS_BUILTIN(CONFIG_PHYLIB)) {
>> +		/* Set delay values */
>> +		value = GMII_RCCPSR | 0x8000;
>> +		phy_write(phy, GMII_ERCR, value);
>> +		value = 0xF2F4;
>> +		phy_write(phy, GMII_ERDWR, value);
>> +		value = GMII_RRDPSR | 0x8000;
>> +		phy_write(phy, GMII_ERCR, value);
>> +		value = 0x2222;
>> +		phy_write(phy, GMII_ERDWR, value);
>> +	}
>> 
>>  	return 0;
>>  }
>> 
>>  static void __init sama5_dt_device_init(void)
>>  {
>> -	if (of_machine_is_compatible("atmel,sama5d3xcm"))
>> +	if (of_machine_is_compatible("atmel,sama5d3xcm") && IS_BUILTIN(CONFIG_PHYLIB))
>>  		phy_register_fixup_for_uid(PHY_ID_KSZ9021, MICREL_PHY_ID_MASK,
>>  			ksz9021rn_phy_fixup);
>> 
>> 
> 
> 
> -- 
> Nicolas Ferre


^ permalink raw reply	[flat|nested] 24+ messages in thread
* [PATCH v3] ARM: at91: Fix link breakage when !CONFIG_PHYLIB
@ 2013-05-27 12:43     ` Jean-Christophe PLAGNIOL-VILLARD
  0 siblings, 0 replies; 24+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-05-27 12:43 UTC (permalink / raw)
  To: linux-arm-kernel


On May 27, 2013, at 8:05 PM, Nicolas Ferre <nicolas.ferre@atmel.com> wrote:

> On 27/05/2013 12:48, Alexandre Belloni :
>> Fixes:
>> arch/arm/mach-at91/built-in.o: In function `ksz9021rn_phy_fixup':
>> :(.text+0x1174): undefined reference to `mdiobus_write'
>> :(.text+0x1188): undefined reference to `mdiobus_write'
>> :(.text+0x119c): undefined reference to `mdiobus_write'
>> :(.text+0x11b0): undefined reference to `mdiobus_write'
>> arch/arm/mach-at91/built-in.o: In function `sama5_dt_device_init':
>> :(.init.text+0x1e34): undefined reference to `phy_register_fixup_for_uid'
>> 
>> when CONFIG_PHYLIB is not selected.
>> 
>> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
>> Acked-by: Ludovic Desroches <ludovic.desroches@atmel.com>
>> ---
>> Changes in v3:
>> 	added SoB
>> 	added Acked-By
> 
> A v3 is not needed simply for this. Thanks, anyway!
> 
> One question though: why not just add
> 
>        select PHYLIB
> 
> In Kconfig entry:
> 
> config MACH_SAMA5_DT
> 
> ?
> 
> (note that I like to play with everybody's nerves ;-))
> 

I prefer this than the IS_BUILTIN


> Bye,
> 
>> Changes in v2:
>> 	use IS_BUILTIN
>> 	use CONFIG_PHYLIB and not CONFIG_PHY
>> 
>>  arch/arm/mach-at91/board-dt-sama5.c | 22 ++++++++++++----------
>>  1 file changed, 12 insertions(+), 10 deletions(-)
>> 
>> diff --git a/arch/arm/mach-at91/board-dt-sama5.c b/arch/arm/mach-at91/board-dt-sama5.c
>> index 705305e..e9ce541 100644
>> --- a/arch/arm/mach-at91/board-dt-sama5.c
>> +++ b/arch/arm/mach-at91/board-dt-sama5.c
>> @@ -47,22 +47,24 @@ static int ksz9021rn_phy_fixup(struct phy_device *phy)
>>  #define GMII_ERCR	11
>>  #define GMII_ERDWR	12
>> 
>> -	/* Set delay values */
>> -	value = GMII_RCCPSR | 0x8000;
>> -	phy_write(phy, GMII_ERCR, value);
>> -	value = 0xF2F4;
>> -	phy_write(phy, GMII_ERDWR, value);
>> -	value = GMII_RRDPSR | 0x8000;
>> -	phy_write(phy, GMII_ERCR, value);
>> -	value = 0x2222;
>> -	phy_write(phy, GMII_ERDWR, value);
>> +	if (IS_BUILTIN(CONFIG_PHYLIB)) {
>> +		/* Set delay values */
>> +		value = GMII_RCCPSR | 0x8000;
>> +		phy_write(phy, GMII_ERCR, value);
>> +		value = 0xF2F4;
>> +		phy_write(phy, GMII_ERDWR, value);
>> +		value = GMII_RRDPSR | 0x8000;
>> +		phy_write(phy, GMII_ERCR, value);
>> +		value = 0x2222;
>> +		phy_write(phy, GMII_ERDWR, value);
>> +	}
>> 
>>  	return 0;
>>  }
>> 
>>  static void __init sama5_dt_device_init(void)
>>  {
>> -	if (of_machine_is_compatible("atmel,sama5d3xcm"))
>> +	if (of_machine_is_compatible("atmel,sama5d3xcm") && IS_BUILTIN(CONFIG_PHYLIB))
>>  		phy_register_fixup_for_uid(PHY_ID_KSZ9021, MICREL_PHY_ID_MASK,
>>  			ksz9021rn_phy_fixup);
>> 
>> 
> 
> 
> -- 
> Nicolas Ferre

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v3] ARM: at91: Fix link breakage when !CONFIG_PHYLIB
  2013-05-27 12:05   ` Nicolas Ferre
@ 2013-05-27 12:46     ` Alexandre Belloni
  -1 siblings, 0 replies; 24+ messages in thread
From: Alexandre Belloni @ 2013-05-27 12:46 UTC (permalink / raw)
  To: Nicolas Ferre
  Cc: Jean-Christophe Plagniol-Villard, linux-arm-kernel, linux-kernel,
	Ludovic Desroches

On 27/05/2013 14:05, Nicolas Ferre wrote:
> On 27/05/2013 12:48, Alexandre Belloni :
>> Fixes:
>> arch/arm/mach-at91/built-in.o: In function `ksz9021rn_phy_fixup':
>> :(.text+0x1174): undefined reference to `mdiobus_write'
>> :(.text+0x1188): undefined reference to `mdiobus_write'
>> :(.text+0x119c): undefined reference to `mdiobus_write'
>> :(.text+0x11b0): undefined reference to `mdiobus_write'
>> arch/arm/mach-at91/built-in.o: In function `sama5_dt_device_init':
>> :(.init.text+0x1e34): undefined reference to
>> `phy_register_fixup_for_uid'
>>
>> when CONFIG_PHYLIB is not selected.
>>
>> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
>> Acked-by: Ludovic Desroches <ludovic.desroches@atmel.com>
>> ---
>> Changes in v3:
>>     added SoB
>>     added Acked-By
>
> A v3 is not needed simply for this. Thanks, anyway!
>

It was not much and I have to stop forgetting my SoB ;)

> One question though: why not just add
>
>         select PHYLIB
>
> In Kconfig entry:
>
>  config MACH_SAMA5_DT
>
> ?
>

That would simply mean that you have to enable NET when using a SAMA5
which is not what every user will want.

Regards,

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


^ permalink raw reply	[flat|nested] 24+ messages in thread
* [PATCH v3] ARM: at91: Fix link breakage when !CONFIG_PHYLIB
@ 2013-05-27 12:46     ` Alexandre Belloni
  0 siblings, 0 replies; 24+ messages in thread
From: Alexandre Belloni @ 2013-05-27 12:46 UTC (permalink / raw)
  To: linux-arm-kernel

On 27/05/2013 14:05, Nicolas Ferre wrote:
> On 27/05/2013 12:48, Alexandre Belloni :
>> Fixes:
>> arch/arm/mach-at91/built-in.o: In function `ksz9021rn_phy_fixup':
>> :(.text+0x1174): undefined reference to `mdiobus_write'
>> :(.text+0x1188): undefined reference to `mdiobus_write'
>> :(.text+0x119c): undefined reference to `mdiobus_write'
>> :(.text+0x11b0): undefined reference to `mdiobus_write'
>> arch/arm/mach-at91/built-in.o: In function `sama5_dt_device_init':
>> :(.init.text+0x1e34): undefined reference to
>> `phy_register_fixup_for_uid'
>>
>> when CONFIG_PHYLIB is not selected.
>>
>> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
>> Acked-by: Ludovic Desroches <ludovic.desroches@atmel.com>
>> ---
>> Changes in v3:
>>     added SoB
>>     added Acked-By
>
> A v3 is not needed simply for this. Thanks, anyway!
>

It was not much and I have to stop forgetting my SoB ;)

> One question though: why not just add
>
>         select PHYLIB
>
> In Kconfig entry:
>
>  config MACH_SAMA5_DT
>
> ?
>

That would simply mean that you have to enable NET when using a SAMA5
which is not what every user will want.

Regards,

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v3] ARM: at91: Fix link breakage when !CONFIG_PHYLIB
  2013-05-27 11:26   ` Jean-Christophe PLAGNIOL-VILLARD
@ 2013-05-27 12:50     ` Alexandre Belloni
  -1 siblings, 0 replies; 24+ messages in thread
From: Alexandre Belloni @ 2013-05-27 12:50 UTC (permalink / raw)
  To: Jean-Christophe PLAGNIOL-VILLARD
  Cc: Nicolas Ferre, linux-arm-kernel, linux-kernel, Ludovic Desroches

On 27/05/2013 13:26, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On May 27, 2013, at 6:48 PM, Alexandre Belloni <alexandre.belloni@free-electrons.com> wrote:
>
>> Fixes:
>> arch/arm/mach-at91/built-in.o: In function `ksz9021rn_phy_fixup':
>> :(.text+0x1174): undefined reference to `mdiobus_write'
>> :(.text+0x1188): undefined reference to `mdiobus_write'
>> :(.text+0x119c): undefined reference to `mdiobus_write'
>> :(.text+0x11b0): undefined reference to `mdiobus_write'
>> arch/arm/mach-at91/built-in.o: In function `sama5_dt_device_init':
>> :(.init.text+0x1e34): undefined reference to `phy_register_fixup_for_uid'
>>
>> when CONFIG_PHYLIB is not selected.
>>
>> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
>> Acked-by: Ludovic Desroches <ludovic.desroches@atmel.com>
> no for this change
>
> add an inline version of phy_register_fixup_for_uid
>
> this will do the same for every platform

I don't actually see the added value of doing that. Using IS_BUILTIN is
readable and ensure that you will definitely be able to link. The
functions will be optimized out and removed by gcc even when you have
CONFIG_PHYLIB=m.

>> ---
>> Changes in v3:
>> 	added SoB
>> 	added Acked-By
>>
>> Changes in v2:
>> 	use IS_BUILTIN
>> 	use CONFIG_PHYLIB and not CONFIG_PHY
>>
>> arch/arm/mach-at91/board-dt-sama5.c | 22 ++++++++++++----------
>> 1 file changed, 12 insertions(+), 10 deletions(-)
>>
>> diff --git a/arch/arm/mach-at91/board-dt-sama5.c b/arch/arm/mach-at91/board-dt-sama5.c
>> index 705305e..e9ce541 100644
>> --- a/arch/arm/mach-at91/board-dt-sama5.c
>> +++ b/arch/arm/mach-at91/board-dt-sama5.c
>> @@ -47,22 +47,24 @@ static int ksz9021rn_phy_fixup(struct phy_device *phy)
>> #define GMII_ERCR	11
>> #define GMII_ERDWR	12
>>
>> -	/* Set delay values */
>> -	value = GMII_RCCPSR | 0x8000;
>> -	phy_write(phy, GMII_ERCR, value);
>> -	value = 0xF2F4;
>> -	phy_write(phy, GMII_ERDWR, value);
>> -	value = GMII_RRDPSR | 0x8000;
>> -	phy_write(phy, GMII_ERCR, value);
>> -	value = 0x2222;
>> -	phy_write(phy, GMII_ERDWR, value);
>> +	if (IS_BUILTIN(CONFIG_PHYLIB)) {
>> +		/* Set delay values */
>> +		value = GMII_RCCPSR | 0x8000;
>> +		phy_write(phy, GMII_ERCR, value);
>> +		value = 0xF2F4;
>> +		phy_write(phy, GMII_ERDWR, value);
>> +		value = GMII_RRDPSR | 0x8000;
>> +		phy_write(phy, GMII_ERCR, value);
>> +		value = 0x2222;
>> +		phy_write(phy, GMII_ERDWR, value);
>> +	}
>>
>> 	return 0;
>> }
>>
>> static void __init sama5_dt_device_init(void)
>> {
>> -	if (of_machine_is_compatible("atmel,sama5d3xcm"))
>> +	if (of_machine_is_compatible("atmel,sama5d3xcm") && IS_BUILTIN(CONFIG_PHYLIB))
>> 		phy_register_fixup_for_uid(PHY_ID_KSZ9021, MICREL_PHY_ID_MASK,
>> 			ksz9021rn_phy_fixup);
>>
>> -- 
>> 1.8.1.2
>>


-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


^ permalink raw reply	[flat|nested] 24+ messages in thread
* [PATCH v3] ARM: at91: Fix link breakage when !CONFIG_PHYLIB
@ 2013-05-27 12:50     ` Alexandre Belloni
  0 siblings, 0 replies; 24+ messages in thread
From: Alexandre Belloni @ 2013-05-27 12:50 UTC (permalink / raw)
  To: linux-arm-kernel

On 27/05/2013 13:26, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On May 27, 2013, at 6:48 PM, Alexandre Belloni <alexandre.belloni@free-electrons.com> wrote:
>
>> Fixes:
>> arch/arm/mach-at91/built-in.o: In function `ksz9021rn_phy_fixup':
>> :(.text+0x1174): undefined reference to `mdiobus_write'
>> :(.text+0x1188): undefined reference to `mdiobus_write'
>> :(.text+0x119c): undefined reference to `mdiobus_write'
>> :(.text+0x11b0): undefined reference to `mdiobus_write'
>> arch/arm/mach-at91/built-in.o: In function `sama5_dt_device_init':
>> :(.init.text+0x1e34): undefined reference to `phy_register_fixup_for_uid'
>>
>> when CONFIG_PHYLIB is not selected.
>>
>> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
>> Acked-by: Ludovic Desroches <ludovic.desroches@atmel.com>
> no for this change
>
> add an inline version of phy_register_fixup_for_uid
>
> this will do the same for every platform

I don't actually see the added value of doing that. Using IS_BUILTIN is
readable and ensure that you will definitely be able to link. The
functions will be optimized out and removed by gcc even when you have
CONFIG_PHYLIB=m.

>> ---
>> Changes in v3:
>> 	added SoB
>> 	added Acked-By
>>
>> Changes in v2:
>> 	use IS_BUILTIN
>> 	use CONFIG_PHYLIB and not CONFIG_PHY
>>
>> arch/arm/mach-at91/board-dt-sama5.c | 22 ++++++++++++----------
>> 1 file changed, 12 insertions(+), 10 deletions(-)
>>
>> diff --git a/arch/arm/mach-at91/board-dt-sama5.c b/arch/arm/mach-at91/board-dt-sama5.c
>> index 705305e..e9ce541 100644
>> --- a/arch/arm/mach-at91/board-dt-sama5.c
>> +++ b/arch/arm/mach-at91/board-dt-sama5.c
>> @@ -47,22 +47,24 @@ static int ksz9021rn_phy_fixup(struct phy_device *phy)
>> #define GMII_ERCR	11
>> #define GMII_ERDWR	12
>>
>> -	/* Set delay values */
>> -	value = GMII_RCCPSR | 0x8000;
>> -	phy_write(phy, GMII_ERCR, value);
>> -	value = 0xF2F4;
>> -	phy_write(phy, GMII_ERDWR, value);
>> -	value = GMII_RRDPSR | 0x8000;
>> -	phy_write(phy, GMII_ERCR, value);
>> -	value = 0x2222;
>> -	phy_write(phy, GMII_ERDWR, value);
>> +	if (IS_BUILTIN(CONFIG_PHYLIB)) {
>> +		/* Set delay values */
>> +		value = GMII_RCCPSR | 0x8000;
>> +		phy_write(phy, GMII_ERCR, value);
>> +		value = 0xF2F4;
>> +		phy_write(phy, GMII_ERDWR, value);
>> +		value = GMII_RRDPSR | 0x8000;
>> +		phy_write(phy, GMII_ERCR, value);
>> +		value = 0x2222;
>> +		phy_write(phy, GMII_ERDWR, value);
>> +	}
>>
>> 	return 0;
>> }
>>
>> static void __init sama5_dt_device_init(void)
>> {
>> -	if (of_machine_is_compatible("atmel,sama5d3xcm"))
>> +	if (of_machine_is_compatible("atmel,sama5d3xcm") && IS_BUILTIN(CONFIG_PHYLIB))
>> 		phy_register_fixup_for_uid(PHY_ID_KSZ9021, MICREL_PHY_ID_MASK,
>> 			ksz9021rn_phy_fixup);
>>
>> -- 
>> 1.8.1.2
>>


-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v3] ARM: at91: Fix link breakage when !CONFIG_PHYLIB
  2013-05-27 12:50     ` Alexandre Belloni
@ 2013-05-27 12:59       ` Jean-Christophe PLAGNIOL-VILLARD
  -1 siblings, 0 replies; 24+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-05-27 12:59 UTC (permalink / raw)
  To: Alexandre Belloni
  Cc: Jean-Christophe PLAGNIOL-VILLARD, Nicolas Ferre,
	linux-arm-kernel, linux-kernel, Ludovic Desroches


On May 27, 2013, at 8:50 PM, Alexandre Belloni <alexandre.belloni@free-electrons.com> wrote:

> On 27/05/2013 13:26, Jean-Christophe PLAGNIOL-VILLARD wrote:
>> On May 27, 2013, at 6:48 PM, Alexandre Belloni <alexandre.belloni@free-electrons.com> wrote:
>> 
>>> Fixes:
>>> arch/arm/mach-at91/built-in.o: In function `ksz9021rn_phy_fixup':
>>> :(.text+0x1174): undefined reference to `mdiobus_write'
>>> :(.text+0x1188): undefined reference to `mdiobus_write'
>>> :(.text+0x119c): undefined reference to `mdiobus_write'
>>> :(.text+0x11b0): undefined reference to `mdiobus_write'
>>> arch/arm/mach-at91/built-in.o: In function `sama5_dt_device_init':
>>> :(.init.text+0x1e34): undefined reference to `phy_register_fixup_for_uid'
>>> 
>>> when CONFIG_PHYLIB is not selected.
>>> 
>>> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
>>> Acked-by: Ludovic Desroches <ludovic.desroches@atmel.com>
>> no for this change
>> 
>> add an inline version of phy_register_fixup_for_uid
>> 
>> this will do the same for every platform
> 
> I don't actually see the added value of doing that. Using IS_BUILTIN is
> readable and ensure that you will definitely be able to link. The
> functions will be optimized out and removed by gcc even when you have
> CONFIG_PHYLIB=m.

no will not as you do ifdef CONFIG_PHYLIB and if CONFIG_PHYLIB=m the macro is defended

we do this all the time in the kernel

I add the IS_BUILTIN & co to be used when a simple inline is not sufficient

Best Regards,
J/
> 
>>> ---
>>> Changes in v3:
>>> 	added SoB
>>> 	added Acked-By
>>> 
>>> Changes in v2:
>>> 	use IS_BUILTIN
>>> 	use CONFIG_PHYLIB and not CONFIG_PHY
>>> 
>>> arch/arm/mach-at91/board-dt-sama5.c | 22 ++++++++++++----------
>>> 1 file changed, 12 insertions(+), 10 deletions(-)
>>> 
>>> diff --git a/arch/arm/mach-at91/board-dt-sama5.c b/arch/arm/mach-at91/board-dt-sama5.c
>>> index 705305e..e9ce541 100644
>>> --- a/arch/arm/mach-at91/board-dt-sama5.c
>>> +++ b/arch/arm/mach-at91/board-dt-sama5.c
>>> @@ -47,22 +47,24 @@ static int ksz9021rn_phy_fixup(struct phy_device *phy)
>>> #define GMII_ERCR	11
>>> #define GMII_ERDWR	12
>>> 
>>> -	/* Set delay values */
>>> -	value = GMII_RCCPSR | 0x8000;
>>> -	phy_write(phy, GMII_ERCR, value);
>>> -	value = 0xF2F4;
>>> -	phy_write(phy, GMII_ERDWR, value);
>>> -	value = GMII_RRDPSR | 0x8000;
>>> -	phy_write(phy, GMII_ERCR, value);
>>> -	value = 0x2222;
>>> -	phy_write(phy, GMII_ERDWR, value);
>>> +	if (IS_BUILTIN(CONFIG_PHYLIB)) {
>>> +		/* Set delay values */
>>> +		value = GMII_RCCPSR | 0x8000;
>>> +		phy_write(phy, GMII_ERCR, value);
>>> +		value = 0xF2F4;
>>> +		phy_write(phy, GMII_ERDWR, value);
>>> +		value = GMII_RRDPSR | 0x8000;
>>> +		phy_write(phy, GMII_ERCR, value);
>>> +		value = 0x2222;
>>> +		phy_write(phy, GMII_ERDWR, value);
>>> +	}
>>> 
>>> 	return 0;
>>> }
>>> 
>>> static void __init sama5_dt_device_init(void)
>>> {
>>> -	if (of_machine_is_compatible("atmel,sama5d3xcm"))
>>> +	if (of_machine_is_compatible("atmel,sama5d3xcm") && IS_BUILTIN(CONFIG_PHYLIB))
>>> 		phy_register_fixup_for_uid(PHY_ID_KSZ9021, MICREL_PHY_ID_MASK,
>>> 			ksz9021rn_phy_fixup);
>>> 
>>> -- 
>>> 1.8.1.2
>>> 
> 
> 
> -- 
> Alexandre Belloni, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com
> 


^ permalink raw reply	[flat|nested] 24+ messages in thread
* [PATCH v3] ARM: at91: Fix link breakage when !CONFIG_PHYLIB
@ 2013-05-27 12:59       ` Jean-Christophe PLAGNIOL-VILLARD
  0 siblings, 0 replies; 24+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-05-27 12:59 UTC (permalink / raw)
  To: linux-arm-kernel


On May 27, 2013, at 8:50 PM, Alexandre Belloni <alexandre.belloni@free-electrons.com> wrote:

> On 27/05/2013 13:26, Jean-Christophe PLAGNIOL-VILLARD wrote:
>> On May 27, 2013, at 6:48 PM, Alexandre Belloni <alexandre.belloni@free-electrons.com> wrote:
>> 
>>> Fixes:
>>> arch/arm/mach-at91/built-in.o: In function `ksz9021rn_phy_fixup':
>>> :(.text+0x1174): undefined reference to `mdiobus_write'
>>> :(.text+0x1188): undefined reference to `mdiobus_write'
>>> :(.text+0x119c): undefined reference to `mdiobus_write'
>>> :(.text+0x11b0): undefined reference to `mdiobus_write'
>>> arch/arm/mach-at91/built-in.o: In function `sama5_dt_device_init':
>>> :(.init.text+0x1e34): undefined reference to `phy_register_fixup_for_uid'
>>> 
>>> when CONFIG_PHYLIB is not selected.
>>> 
>>> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
>>> Acked-by: Ludovic Desroches <ludovic.desroches@atmel.com>
>> no for this change
>> 
>> add an inline version of phy_register_fixup_for_uid
>> 
>> this will do the same for every platform
> 
> I don't actually see the added value of doing that. Using IS_BUILTIN is
> readable and ensure that you will definitely be able to link. The
> functions will be optimized out and removed by gcc even when you have
> CONFIG_PHYLIB=m.

no will not as you do ifdef CONFIG_PHYLIB and if CONFIG_PHYLIB=m the macro is defended

we do this all the time in the kernel

I add the IS_BUILTIN & co to be used when a simple inline is not sufficient

Best Regards,
J/
> 
>>> ---
>>> Changes in v3:
>>> 	added SoB
>>> 	added Acked-By
>>> 
>>> Changes in v2:
>>> 	use IS_BUILTIN
>>> 	use CONFIG_PHYLIB and not CONFIG_PHY
>>> 
>>> arch/arm/mach-at91/board-dt-sama5.c | 22 ++++++++++++----------
>>> 1 file changed, 12 insertions(+), 10 deletions(-)
>>> 
>>> diff --git a/arch/arm/mach-at91/board-dt-sama5.c b/arch/arm/mach-at91/board-dt-sama5.c
>>> index 705305e..e9ce541 100644
>>> --- a/arch/arm/mach-at91/board-dt-sama5.c
>>> +++ b/arch/arm/mach-at91/board-dt-sama5.c
>>> @@ -47,22 +47,24 @@ static int ksz9021rn_phy_fixup(struct phy_device *phy)
>>> #define GMII_ERCR	11
>>> #define GMII_ERDWR	12
>>> 
>>> -	/* Set delay values */
>>> -	value = GMII_RCCPSR | 0x8000;
>>> -	phy_write(phy, GMII_ERCR, value);
>>> -	value = 0xF2F4;
>>> -	phy_write(phy, GMII_ERDWR, value);
>>> -	value = GMII_RRDPSR | 0x8000;
>>> -	phy_write(phy, GMII_ERCR, value);
>>> -	value = 0x2222;
>>> -	phy_write(phy, GMII_ERDWR, value);
>>> +	if (IS_BUILTIN(CONFIG_PHYLIB)) {
>>> +		/* Set delay values */
>>> +		value = GMII_RCCPSR | 0x8000;
>>> +		phy_write(phy, GMII_ERCR, value);
>>> +		value = 0xF2F4;
>>> +		phy_write(phy, GMII_ERDWR, value);
>>> +		value = GMII_RRDPSR | 0x8000;
>>> +		phy_write(phy, GMII_ERCR, value);
>>> +		value = 0x2222;
>>> +		phy_write(phy, GMII_ERDWR, value);
>>> +	}
>>> 
>>> 	return 0;
>>> }
>>> 
>>> static void __init sama5_dt_device_init(void)
>>> {
>>> -	if (of_machine_is_compatible("atmel,sama5d3xcm"))
>>> +	if (of_machine_is_compatible("atmel,sama5d3xcm") && IS_BUILTIN(CONFIG_PHYLIB))
>>> 		phy_register_fixup_for_uid(PHY_ID_KSZ9021, MICREL_PHY_ID_MASK,
>>> 			ksz9021rn_phy_fixup);
>>> 
>>> -- 
>>> 1.8.1.2
>>> 
> 
> 
> -- 
> Alexandre Belloni, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v3] ARM: at91: Fix link breakage when !CONFIG_PHYLIB
  2013-05-27 12:59       ` Jean-Christophe PLAGNIOL-VILLARD
@ 2013-05-27 16:04         ` Alexandre Belloni
  -1 siblings, 0 replies; 24+ messages in thread
From: Alexandre Belloni @ 2013-05-27 16:04 UTC (permalink / raw)
  To: Jean-Christophe PLAGNIOL-VILLARD
  Cc: Nicolas Ferre, linux-arm-kernel, linux-kernel, Ludovic Desroches

On 27/05/2013 14:59, Jean-Christophe PLAGNIOL-VILLARD wrote:
>
> no will not as you do ifdef CONFIG_PHYLIB and if CONFIG_PHYLIB=m the macro is defended
>
> we do this all the time in the kernel
>
> I add the IS_BUILTIN & co to be used when a simple inline is not sufficient

Ok, it took me a while to understand what you were trying to explain.
I'm sending a patch set now. I don't believe it will be taken in for
3.10 though.

Regards,

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


^ permalink raw reply	[flat|nested] 24+ messages in thread
* [PATCH v3] ARM: at91: Fix link breakage when !CONFIG_PHYLIB
@ 2013-05-27 16:04         ` Alexandre Belloni
  0 siblings, 0 replies; 24+ messages in thread
From: Alexandre Belloni @ 2013-05-27 16:04 UTC (permalink / raw)
  To: linux-arm-kernel

On 27/05/2013 14:59, Jean-Christophe PLAGNIOL-VILLARD wrote:
>
> no will not as you do ifdef CONFIG_PHYLIB and if CONFIG_PHYLIB=m the macro is defended
>
> we do this all the time in the kernel
>
> I add the IS_BUILTIN & co to be used when a simple inline is not sufficient

Ok, it took me a while to understand what you were trying to explain.
I'm sending a patch set now. I don't believe it will be taken in for
3.10 though.

Regards,

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v3] ARM: at91: Fix link breakage when !CONFIG_PHYLIB
  2013-05-27 12:42     ` Arnd Bergmann
@ 2013-06-04 13:49       ` Nicolas Ferre
  -1 siblings, 0 replies; 24+ messages in thread
From: Nicolas Ferre @ 2013-06-04 13:49 UTC (permalink / raw)
  To: Arnd Bergmann, Alexandre Belloni, Jean-Christophe Plagniol-Villard
  Cc: linux-arm-kernel, Ludovic Desroches, linux-kernel

On 27/05/2013 14:42, Arnd Bergmann :
> On Monday 27 May 2013 14:05:13 Nicolas Ferre wrote:
>> On 27/05/2013 12:48, Alexandre Belloni :
>> A v3 is not needed simply for this. Thanks, anyway!
>>
>> One question though: why not just add
>>
>>           select PHYLIB
>>
>> In Kconfig entry:
>>
>>    config MACH_SAMA5_DT
>>
>> ?
>>
>> (note that I like to play with everybody's nerves ;-))
>
> That would make it impossible to build a sama5 kernel without networking
> support, which is probably not what you want.
>
> Jean-Christophe's suggestion to add the inline helper is best.

It seems that it has been clearly rejected by David. So the best 
solution remains to...

> Aside from
> that, please note that
>
>>> diff --git a/arch/arm/mach-at91/board-dt-sama5.c b/arch/arm/mach-at91/board-dt-sama5.c
>>> index 705305e..e9ce541 100644
>>> --- a/arch/arm/mach-at91/board-dt-sama5.c
>>> +++ b/arch/arm/mach-at91/board-dt-sama5.c
>>> @@ -47,22 +47,24 @@ static int ksz9021rn_phy_fixup(struct phy_device *phy)
>>>    #define GMII_ERCR	11
>>>    #define GMII_ERDWR	12
>>>
>>> -	/* Set delay values */
>>> -	value = GMII_RCCPSR | 0x8000;
>>> -	phy_write(phy, GMII_ERCR, value);
>>> -	value = 0xF2F4;
>>> -	phy_write(phy, GMII_ERDWR, value);
>>> -	value = GMII_RRDPSR | 0x8000;
>>> -	phy_write(phy, GMII_ERCR, value);
>>> -	value = 0x2222;
>>> -	phy_write(phy, GMII_ERDWR, value);
>>> +	if (IS_BUILTIN(CONFIG_PHYLIB)) {
>>> +		/* Set delay values */
>>> +		value = GMII_RCCPSR | 0x8000;
>>> +		phy_write(phy, GMII_ERCR, value);
>>> +		value = 0xF2F4;
>>> +		phy_write(phy, GMII_ERDWR, value);
>>> +		value = GMII_RRDPSR | 0x8000;
>>> +		phy_write(phy, GMII_ERCR, value);
>>> +		value = 0x2222;
>>> +		phy_write(phy, GMII_ERDWR, value);
>>> +	}
>>>
>>>    	return 0;
>>>    }
>
> This part of the patch is not actually needed if you
>
>>>    static void __init sama5_dt_device_init(void)
>>>    {
>>> -	if (of_machine_is_compatible("atmel,sama5d3xcm"))
>>> +	if (of_machine_is_compatible("atmel,sama5d3xcm") && IS_BUILTIN(CONFIG_PHYLIB))
>>>    		phy_register_fixup_for_uid(PHY_ID_KSZ9021, MICREL_PHY_ID_MASK,
>>>    			ksz9021rn_phy_fixup);
>
> do this change, or the inline helper.

... simply use this "IS_BUILTIN(CONFIG_PHYLIB)" here -----^^^^^^^^^ (and 
not above as suggested by Arnd).

Jean-Christophe, Alexandre: do you agree with this conclusion?

I plan to stack this patch for next at91-fixes batch.

Bye,
-- 
Nicolas Ferre

^ permalink raw reply	[flat|nested] 24+ messages in thread
* [PATCH v3] ARM: at91: Fix link breakage when !CONFIG_PHYLIB
@ 2013-06-04 13:49       ` Nicolas Ferre
  0 siblings, 0 replies; 24+ messages in thread
From: Nicolas Ferre @ 2013-06-04 13:49 UTC (permalink / raw)
  To: linux-arm-kernel

On 27/05/2013 14:42, Arnd Bergmann :
> On Monday 27 May 2013 14:05:13 Nicolas Ferre wrote:
>> On 27/05/2013 12:48, Alexandre Belloni :
>> A v3 is not needed simply for this. Thanks, anyway!
>>
>> One question though: why not just add
>>
>>           select PHYLIB
>>
>> In Kconfig entry:
>>
>>    config MACH_SAMA5_DT
>>
>> ?
>>
>> (note that I like to play with everybody's nerves ;-))
>
> That would make it impossible to build a sama5 kernel without networking
> support, which is probably not what you want.
>
> Jean-Christophe's suggestion to add the inline helper is best.

It seems that it has been clearly rejected by David. So the best 
solution remains to...

> Aside from
> that, please note that
>
>>> diff --git a/arch/arm/mach-at91/board-dt-sama5.c b/arch/arm/mach-at91/board-dt-sama5.c
>>> index 705305e..e9ce541 100644
>>> --- a/arch/arm/mach-at91/board-dt-sama5.c
>>> +++ b/arch/arm/mach-at91/board-dt-sama5.c
>>> @@ -47,22 +47,24 @@ static int ksz9021rn_phy_fixup(struct phy_device *phy)
>>>    #define GMII_ERCR	11
>>>    #define GMII_ERDWR	12
>>>
>>> -	/* Set delay values */
>>> -	value = GMII_RCCPSR | 0x8000;
>>> -	phy_write(phy, GMII_ERCR, value);
>>> -	value = 0xF2F4;
>>> -	phy_write(phy, GMII_ERDWR, value);
>>> -	value = GMII_RRDPSR | 0x8000;
>>> -	phy_write(phy, GMII_ERCR, value);
>>> -	value = 0x2222;
>>> -	phy_write(phy, GMII_ERDWR, value);
>>> +	if (IS_BUILTIN(CONFIG_PHYLIB)) {
>>> +		/* Set delay values */
>>> +		value = GMII_RCCPSR | 0x8000;
>>> +		phy_write(phy, GMII_ERCR, value);
>>> +		value = 0xF2F4;
>>> +		phy_write(phy, GMII_ERDWR, value);
>>> +		value = GMII_RRDPSR | 0x8000;
>>> +		phy_write(phy, GMII_ERCR, value);
>>> +		value = 0x2222;
>>> +		phy_write(phy, GMII_ERDWR, value);
>>> +	}
>>>
>>>    	return 0;
>>>    }
>
> This part of the patch is not actually needed if you
>
>>>    static void __init sama5_dt_device_init(void)
>>>    {
>>> -	if (of_machine_is_compatible("atmel,sama5d3xcm"))
>>> +	if (of_machine_is_compatible("atmel,sama5d3xcm") && IS_BUILTIN(CONFIG_PHYLIB))
>>>    		phy_register_fixup_for_uid(PHY_ID_KSZ9021, MICREL_PHY_ID_MASK,
>>>    			ksz9021rn_phy_fixup);
>
> do this change, or the inline helper.

... simply use this "IS_BUILTIN(CONFIG_PHYLIB)" here -----^^^^^^^^^ (and 
not above as suggested by Arnd).

Jean-Christophe, Alexandre: do you agree with this conclusion?

I plan to stack this patch for next at91-fixes batch.

Bye,
-- 
Nicolas Ferre

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v3] ARM: at91: Fix link breakage when !CONFIG_PHYLIB
  2013-06-04 13:49       ` Nicolas Ferre
@ 2013-06-04 14:12         ` Alexandre Belloni
  -1 siblings, 0 replies; 24+ messages in thread
From: Alexandre Belloni @ 2013-06-04 14:12 UTC (permalink / raw)
  To: Nicolas Ferre
  Cc: Arnd Bergmann, Jean-Christophe Plagniol-Villard,
	linux-arm-kernel, Ludovic Desroches, linux-kernel

On 04/06/2013 15:49, Nicolas Ferre wrote:
>
> ... simply use this "IS_BUILTIN(CONFIG_PHYLIB)" here -----^^^^^^^^^
> (and not above as suggested by Arnd).
>
> Jean-Christophe, Alexandre: do you agree with this conclusion?
>
> I plan to stack this patch for next at91-fixes batch.
>

Yeah, I prepared and tested that last week but didn't send it yet. I can
do it now if you want but you could probably just edit my patch. I'd
like to fix the other platforms too but that probably can wait.

I believe we would still have to fix the phy_register_fixup*() works as
it is always called from arch/ and is clearly not fool proofed enough.
Maybe DT could help ?

Arnd, do you have any input ?

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


^ permalink raw reply	[flat|nested] 24+ messages in thread
* [PATCH v3] ARM: at91: Fix link breakage when !CONFIG_PHYLIB
@ 2013-06-04 14:12         ` Alexandre Belloni
  0 siblings, 0 replies; 24+ messages in thread
From: Alexandre Belloni @ 2013-06-04 14:12 UTC (permalink / raw)
  To: linux-arm-kernel

On 04/06/2013 15:49, Nicolas Ferre wrote:
>
> ... simply use this "IS_BUILTIN(CONFIG_PHYLIB)" here -----^^^^^^^^^
> (and not above as suggested by Arnd).
>
> Jean-Christophe, Alexandre: do you agree with this conclusion?
>
> I plan to stack this patch for next at91-fixes batch.
>

Yeah, I prepared and tested that last week but didn't send it yet. I can
do it now if you want but you could probably just edit my patch. I'd
like to fix the other platforms too but that probably can wait.

I believe we would still have to fix the phy_register_fixup*() works as
it is always called from arch/ and is clearly not fool proofed enough.
Maybe DT could help ?

Arnd, do you have any input ?

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v3] ARM: at91: Fix link breakage when !CONFIG_PHYLIB
  2013-06-04 14:12         ` Alexandre Belloni
@ 2013-06-04 15:12           ` Arnd Bergmann
  -1 siblings, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2013-06-04 15:12 UTC (permalink / raw)
  To: Alexandre Belloni
  Cc: Nicolas Ferre, Jean-Christophe Plagniol-Villard,
	linux-arm-kernel, Ludovic Desroches, linux-kernel

On Tuesday 04 June 2013 16:12:24 Alexandre Belloni wrote:
> On 04/06/2013 15:49, Nicolas Ferre wrote:
> >
> > ... simply use this "IS_BUILTIN(CONFIG_PHYLIB)" here -----^^^^^^^^^
> > (and not above as suggested by Arnd).
> >
> > Jean-Christophe, Alexandre: do you agree with this conclusion?
> >
> > I plan to stack this patch for next at91-fixes batch.
> >
> 
> Yeah, I prepared and tested that last week but didn't send it yet. I can
> do it now if you want but you could probably just edit my patch. I'd
> like to fix the other platforms too but that probably can wait.
> 
> I believe we would still have to fix the phy_register_fixup*() works as
> it is always called from arch/ and is clearly not fool proofed enough.
> Maybe DT could help ?
> 
> Arnd, do you have any input ?

I've replied to the earlier thread now. Using if(IS_ENABLED(CONFIG_NET))
or if(IS_ENABLED(CONFIG_PHYLIB)) in platform code should be just fine and
is not a lof of extra effort.

As I explained in the other mail, I think IS_ENABLED() is better than
IS_BUILTIN() because the case where the network driver is a loadable
module would cause a silent run-time error with IS_BUILTIN, but a
link time error with IS_ENABLED, and we can prevent that error by
correctly expressing the dependency in Kconfig, e.g.

config THIS_BOARD
	select PHYLIB if THIS_BOARDS_NIC

	Arnd

^ permalink raw reply	[flat|nested] 24+ messages in thread
* [PATCH v3] ARM: at91: Fix link breakage when !CONFIG_PHYLIB
@ 2013-06-04 15:12           ` Arnd Bergmann
  0 siblings, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2013-06-04 15:12 UTC (permalink / raw)
  To: linux-arm-kernel

On Tuesday 04 June 2013 16:12:24 Alexandre Belloni wrote:
> On 04/06/2013 15:49, Nicolas Ferre wrote:
> >
> > ... simply use this "IS_BUILTIN(CONFIG_PHYLIB)" here -----^^^^^^^^^
> > (and not above as suggested by Arnd).
> >
> > Jean-Christophe, Alexandre: do you agree with this conclusion?
> >
> > I plan to stack this patch for next at91-fixes batch.
> >
> 
> Yeah, I prepared and tested that last week but didn't send it yet. I can
> do it now if you want but you could probably just edit my patch. I'd
> like to fix the other platforms too but that probably can wait.
> 
> I believe we would still have to fix the phy_register_fixup*() works as
> it is always called from arch/ and is clearly not fool proofed enough.
> Maybe DT could help ?
> 
> Arnd, do you have any input ?

I've replied to the earlier thread now. Using if(IS_ENABLED(CONFIG_NET))
or if(IS_ENABLED(CONFIG_PHYLIB)) in platform code should be just fine and
is not a lof of extra effort.

As I explained in the other mail, I think IS_ENABLED() is better than
IS_BUILTIN() because the case where the network driver is a loadable
module would cause a silent run-time error with IS_BUILTIN, but a
link time error with IS_ENABLED, and we can prevent that error by
correctly expressing the dependency in Kconfig, e.g.

config THIS_BOARD
	select PHYLIB if THIS_BOARDS_NIC

	Arnd

^ permalink raw reply	[flat|nested] 24+ messages in thread
end of thread, other threads:[~2013-06-04 15:15 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-27 10:48 [PATCH v3] ARM: at91: Fix link breakage when !CONFIG_PHYLIB Alexandre Belloni
2013-05-27 10:48 ` Alexandre Belloni
2013-05-27 11:26 ` Jean-Christophe PLAGNIOL-VILLARD
2013-05-27 11:26   ` Jean-Christophe PLAGNIOL-VILLARD
2013-05-27 12:50   ` Alexandre Belloni
2013-05-27 12:50     ` Alexandre Belloni
2013-05-27 12:59     ` Jean-Christophe PLAGNIOL-VILLARD
2013-05-27 12:59       ` Jean-Christophe PLAGNIOL-VILLARD
2013-05-27 16:04       ` Alexandre Belloni
2013-05-27 16:04         ` Alexandre Belloni
2013-05-27 12:05 ` Nicolas Ferre
2013-05-27 12:05   ` Nicolas Ferre
2013-05-27 12:42   ` Arnd Bergmann
2013-05-27 12:42     ` Arnd Bergmann
2013-06-04 13:49     ` Nicolas Ferre
2013-06-04 13:49       ` Nicolas Ferre
2013-06-04 14:12       ` Alexandre Belloni
2013-06-04 14:12         ` Alexandre Belloni
2013-06-04 15:12         ` Arnd Bergmann
2013-06-04 15:12           ` Arnd Bergmann
2013-05-27 12:43   ` Jean-Christophe PLAGNIOL-VILLARD
2013-05-27 12:43     ` Jean-Christophe PLAGNIOL-VILLARD
2013-05-27 12:46   ` Alexandre Belloni
2013-05-27 12:46     ` Alexandre Belloni

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.