[PATCH 59/62] ARM: shmobile: ak4642 needs i2c support

[PATCH 59/62] ARM: shmobile: ak4642 needs i2c support

[Arnd Bergmann]

We cannot select SND_SOC_AK4642 if the I2C core layer
is disabled. Making the select statement conditional
ensures that we can build all configurations.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Simon Horman <horms@verge.net.au>
Cc: Magnus Damm <magnus.damm@gmail.com>
Cc: linux-sh@vger.kernel.org
---
 arch/arm/mach-shmobile/Kconfig | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-shmobile/Kconfig b/arch/arm/mach-shmobile/Kconfig
index 5249ff0..6e3904e 100644
--- a/arch/arm/mach-shmobile/Kconfig
+++ b/arch/arm/mach-shmobile/Kconfig
@@ -170,7 +170,7 @@ config MACH_MACKEREL
 	select ARCH_REQUIRE_GPIOLIB
 	select REGULATOR_FIXED_VOLTAGE if REGULATOR
 	select SMSC_PHY if SMSC911X
-	select SND_SOC_AK4642 if SND_SIMPLE_CARD
+	select SND_SOC_AK4642 if SND_SIMPLE_CARD && I2C=y
 	select USE_OF
 
 config MACH_ARMADILLO800EVA
@@ -179,7 +179,7 @@ config MACH_ARMADILLO800EVA
 	select ARCH_REQUIRE_GPIOLIB
 	select REGULATOR_FIXED_VOLTAGE if REGULATOR
 	select SMSC_PHY if SH_ETH
-	select SND_SOC_WM8978 if SND_SIMPLE_CARD
+	select SND_SOC_WM8978 if SND_SIMPLE_CARD && I2C=y
 	select USE_OF
 
 config MACH_ARMADILLO800EVA_REFERENCE
@@ -188,7 +188,7 @@ config MACH_ARMADILLO800EVA_REFERENCE
 	select ARCH_REQUIRE_GPIOLIB
 	select REGULATOR_FIXED_VOLTAGE if REGULATOR
 	select SMSC_PHY if SH_ETH
-	select SND_SOC_WM8978 if SND_SIMPLE_CARD
+	select SND_SOC_WM8978 if SND_SIMPLE_CARD && I2C=y
 	select USE_OF
 	---help---
 	   Use reference implementation of Aramdillo800 EVA board support
@@ -204,7 +204,7 @@ config MACH_BOCKW
 	select REGULATOR_FIXED_VOLTAGE if REGULATOR
 	select RENESAS_INTC_IRQPIN
 	select SND_SOC_AK4554 if SND_SIMPLE_CARD
-	select SND_SOC_AK4642 if SND_SIMPLE_CARD
+	select SND_SOC_AK4642 if SND_SIMPLE_CARD && I2C=y
 	select USE_OF
 
 config MACH_BOCKW_REFERENCE
@@ -277,7 +277,7 @@ config MACH_KZM9G
 	select ARCH_HAS_OPP
 	select ARCH_REQUIRE_GPIOLIB
 	select REGULATOR_FIXED_VOLTAGE if REGULATOR
-	select SND_SOC_AK4642 if SND_SIMPLE_CARD
+	select SND_SOC_AK4642 if SND_SIMPLE_CARD && I2C=y
 	select USE_OF
 
 config MACH_KZM9G_REFERENCE
@@ -285,7 +285,7 @@ config MACH_KZM9G_REFERENCE
 	depends on ARCH_SH73A0
 	select ARCH_REQUIRE_GPIOLIB
 	select REGULATOR_FIXED_VOLTAGE if REGULATOR
-	select SND_SOC_AK4642 if SND_SIMPLE_CARD
+	select SND_SOC_AK4642 if SND_SIMPLE_CARD && I2C=y
 	select USE_OF
 	---help---
 	   Use reference implementation of KZM-A9-GT board support
-- 
1.8.3.2

Re: [PATCH 59/62] ARM: shmobile: ak4642 needs i2c support

[Sergei Shtylyov]

Hello.

On 03/19/2014 10:29 PM, Arnd Bergmann wrote:

> We cannot select SND_SOC_AK4642 if the I2C core layer
> is disabled. Making the select statement conditional
> ensures that we can build all configurations.

    Yes, but you also touch WM8978 selects. Is it the same as AK4642 or you 
just forgot to mention that?

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Cc: Simon Horman <horms@verge.net.au>
> Cc: Magnus Damm <magnus.damm@gmail.com>
> Cc: linux-sh@vger.kernel.org
> ---
>   arch/arm/mach-shmobile/Kconfig | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)

> diff --git a/arch/arm/mach-shmobile/Kconfig b/arch/arm/mach-shmobile/Kconfig
> index 5249ff0..6e3904e 100644
> --- a/arch/arm/mach-shmobile/Kconfig
> +++ b/arch/arm/mach-shmobile/Kconfig
> @@ -170,7 +170,7 @@ config MACH_MACKEREL
>   	select ARCH_REQUIRE_GPIOLIB
>   	select REGULATOR_FIXED_VOLTAGE if REGULATOR
>   	select SMSC_PHY if SMSC911X
> -	select SND_SOC_AK4642 if SND_SIMPLE_CARD
> +	select SND_SOC_AK4642 if SND_SIMPLE_CARD && I2C=y
>   	select USE_OF
>
>   config MACH_ARMADILLO800EVA
> @@ -179,7 +179,7 @@ config MACH_ARMADILLO800EVA
>   	select ARCH_REQUIRE_GPIOLIB
>   	select REGULATOR_FIXED_VOLTAGE if REGULATOR
>   	select SMSC_PHY if SH_ETH
> -	select SND_SOC_WM8978 if SND_SIMPLE_CARD
> +	select SND_SOC_WM8978 if SND_SIMPLE_CARD && I2C=y
>   	select USE_OF
>
>   config MACH_ARMADILLO800EVA_REFERENCE
> @@ -188,7 +188,7 @@ config MACH_ARMADILLO800EVA_REFERENCE
>   	select ARCH_REQUIRE_GPIOLIB
>   	select REGULATOR_FIXED_VOLTAGE if REGULATOR
>   	select SMSC_PHY if SH_ETH
> -	select SND_SOC_WM8978 if SND_SIMPLE_CARD
> +	select SND_SOC_WM8978 if SND_SIMPLE_CARD && I2C=y
>   	select USE_OF
>   	---help---
>   	   Use reference implementation of Aramdillo800 EVA board support
[...]

WBR, Sergei

Re: [PATCH 59/62] ARM: shmobile: ak4642 needs i2c support

[Arnd Bergmann]

On Wednesday 19 March 2014, Sergei Shtylyov wrote:
> On 03/19/2014 10:29 PM, Arnd Bergmann wrote:
> 
> > We cannot select SND_SOC_AK4642 if the I2C core layer
> > is disabled. Making the select statement conditional
> > ensures that we can build all configurations.
> 
>     Yes, but you also touch WM8978 selects. Is it the same as AK4642 or you 
> just forgot to mention that?

My bad, I probably screwed up when I folded some older patches
into a common one.

	Arnd

[PATCH 60/62] ARM: shmobile: work around CONFIG_PHYLIB=m

[Arnd Bergmann]

When phylib is set to be built as a module, the lager and koelsch
boards fail to build:

arch/arm/mach-shmobile/built-in.o: In function `lager_ksz8041_fixup':
:(.text+0x738): undefined reference to `mdiobus_read'
:(.text+0x73c): undefined reference to `mdiobus_write'
arch/arm/mach-shmobile/built-in.o: In function `koelsch_ksz8041_fixup':
:(.text+0x7e8): undefined reference to `mdiobus_read'
:(.text+0x7ec): undefined reference to `mdiobus_write'

To work around that problem, this changes the code to check for
IS_BUILTIN rather than IS_ENABLED, turning the error into a runtime
problem. It's now possible to build random configurations, but the
phy may be set up incorrectly in this case.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Simon Horman <horms@verge.net.au>
Cc: Magnus Damm <magnus.damm@gmail.com>
Cc: linux-sh@vger.kernel.org
---
 arch/arm/mach-shmobile/board-koelsch.c | 2 +-
 arch/arm/mach-shmobile/board-lager.c   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-shmobile/board-koelsch.c b/arch/arm/mach-shmobile/board-koelsch.c
index 5a034ff..b724f33 100644
--- a/arch/arm/mach-shmobile/board-koelsch.c
+++ b/arch/arm/mach-shmobile/board-koelsch.c
@@ -510,7 +510,7 @@ static void __init koelsch_init(void)
 
 	irq_set_irq_type(irq_pin(0), IRQ_TYPE_LEVEL_LOW);
 
-	if (IS_ENABLED(CONFIG_PHYLIB))
+	if (IS_BUILTIN(CONFIG_PHYLIB))
 		phy_register_fixup_for_id("r8a7791-ether-ff:01",
 					  koelsch_ksz8041_fixup);
 }
diff --git a/arch/arm/mach-shmobile/board-lager.c b/arch/arm/mach-shmobile/board-lager.c
index f0104bf..67b1069 100644
--- a/arch/arm/mach-shmobile/board-lager.c
+++ b/arch/arm/mach-shmobile/board-lager.c
@@ -869,7 +869,7 @@ static void __init lager_init(void)
 
 	irq_set_irq_type(irq_pin(0), IRQ_TYPE_LEVEL_LOW);
 
-	if (IS_ENABLED(CONFIG_PHYLIB))
+	if (IS_BUILTIN(CONFIG_PHYLIB))
 		phy_register_fixup_for_id("r8a7790-ether-ff:01",
 					  lager_ksz8041_fixup);
 }
-- 
1.8.3.2

Re: [PATCH 60/62] ARM: shmobile: work around CONFIG_PHYLIB=m

[Simon Horman]

On Wed, Mar 19, 2014 at 08:29:57PM +0100, Arnd Bergmann wrote:
> When phylib is set to be built as a module, the lager and koelsch
> boards fail to build:
> 
> arch/arm/mach-shmobile/built-in.o: In function `lager_ksz8041_fixup':
> :(.text+0x738): undefined reference to `mdiobus_read'
> :(.text+0x73c): undefined reference to `mdiobus_write'
> arch/arm/mach-shmobile/built-in.o: In function `koelsch_ksz8041_fixup':
> :(.text+0x7e8): undefined reference to `mdiobus_read'
> :(.text+0x7ec): undefined reference to `mdiobus_write'
> 
> To work around that problem, this changes the code to check for
> IS_BUILTIN rather than IS_ENABLED, turning the error into a runtime
> problem. It's now possible to build random configurations, but the
> phy may be set up incorrectly in this case.

I wonder if Kconfig for koelsch should be tightened up somehow to
ensure that PHYLIB is either unselected or builtin.

Also, a minor nit, I would prefer changes for different boards
in different patches. But I can split the patch myself if its
not going to be changed otherwise.

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Cc: Simon Horman <horms@verge.net.au>
> Cc: Magnus Damm <magnus.damm@gmail.com>
> Cc: linux-sh@vger.kernel.org
> ---
>  arch/arm/mach-shmobile/board-koelsch.c | 2 +-
>  arch/arm/mach-shmobile/board-lager.c   | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-shmobile/board-koelsch.c b/arch/arm/mach-shmobile/board-koelsch.c
> index 5a034ff..b724f33 100644
> --- a/arch/arm/mach-shmobile/board-koelsch.c
> +++ b/arch/arm/mach-shmobile/board-koelsch.c
> @@ -510,7 +510,7 @@ static void __init koelsch_init(void)
>  
>  	irq_set_irq_type(irq_pin(0), IRQ_TYPE_LEVEL_LOW);
>  
> -	if (IS_ENABLED(CONFIG_PHYLIB))
> +	if (IS_BUILTIN(CONFIG_PHYLIB))
>  		phy_register_fixup_for_id("r8a7791-ether-ff:01",
>  					  koelsch_ksz8041_fixup);
>  }
> diff --git a/arch/arm/mach-shmobile/board-lager.c b/arch/arm/mach-shmobile/board-lager.c
> index f0104bf..67b1069 100644
> --- a/arch/arm/mach-shmobile/board-lager.c
> +++ b/arch/arm/mach-shmobile/board-lager.c
> @@ -869,7 +869,7 @@ static void __init lager_init(void)
>  
>  	irq_set_irq_type(irq_pin(0), IRQ_TYPE_LEVEL_LOW);
>  
> -	if (IS_ENABLED(CONFIG_PHYLIB))
> +	if (IS_BUILTIN(CONFIG_PHYLIB))
>  		phy_register_fixup_for_id("r8a7790-ether-ff:01",
>  					  lager_ksz8041_fixup);
>  }
> -- 
> 1.8.3.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 60/62] ARM: shmobile: work around CONFIG_PHYLIB=m

[Arnd Bergmann]

On Thursday 20 March 2014, Simon Horman wrote:
> On Wed, Mar 19, 2014 at 08:29:57PM +0100, Arnd Bergmann wrote:
> > When phylib is set to be built as a module, the lager and koelsch
> > boards fail to build:
> > 
> > arch/arm/mach-shmobile/built-in.o: In function `lager_ksz8041_fixup':
> > :(.text+0x738): undefined reference to `mdiobus_read'
> > :(.text+0x73c): undefined reference to `mdiobus_write'
> > arch/arm/mach-shmobile/built-in.o: In function `koelsch_ksz8041_fixup':
> > :(.text+0x7e8): undefined reference to `mdiobus_read'
> > :(.text+0x7ec): undefined reference to `mdiobus_write'
> > 
> > To work around that problem, this changes the code to check for
> > IS_BUILTIN rather than IS_ENABLED, turning the error into a runtime
> > problem. It's now possible to build random configurations, but the
> > phy may be set up incorrectly in this case.
> 
> I wonder if Kconfig for koelsch should be tightened up somehow to
> ensure that PHYLIB is either unselected or builtin.
> 
> Also, a minor nit, I would prefer changes for different boards
> in different patches. But I can split the patch myself if its
> not going to be changed otherwise.

I would prefer to take the entire series directly into arm-soc
this time, if you don't mind.

	Arnd

Re: [PATCH 60/62] ARM: shmobile: work around CONFIG_PHYLIB=m

[Simon Horman]

On Fri, Mar 21, 2014 at 04:43:24PM +0100, Arnd Bergmann wrote:
> On Thursday 20 March 2014, Simon Horman wrote:
> > On Wed, Mar 19, 2014 at 08:29:57PM +0100, Arnd Bergmann wrote:
> > > When phylib is set to be built as a module, the lager and koelsch
> > > boards fail to build:
> > > 
> > > arch/arm/mach-shmobile/built-in.o: In function `lager_ksz8041_fixup':
> > > :(.text+0x738): undefined reference to `mdiobus_read'
> > > :(.text+0x73c): undefined reference to `mdiobus_write'
> > > arch/arm/mach-shmobile/built-in.o: In function `koelsch_ksz8041_fixup':
> > > :(.text+0x7e8): undefined reference to `mdiobus_read'
> > > :(.text+0x7ec): undefined reference to `mdiobus_write'
> > > 
> > > To work around that problem, this changes the code to check for
> > > IS_BUILTIN rather than IS_ENABLED, turning the error into a runtime
> > > problem. It's now possible to build random configurations, but the
> > > phy may be set up incorrectly in this case.
> > 
> > I wonder if Kconfig for koelsch should be tightened up somehow to
> > ensure that PHYLIB is either unselected or builtin.
> > 
> > Also, a minor nit, I would prefer changes for different boards
> > in different patches. But I can split the patch myself if its
> > not going to be changed otherwise.
> 
> I would prefer to take the entire series directly into arm-soc
> this time, if you don't mind.

That I'm happy to go along with though I don't understand the motivation
for it. And regardless of how the patches go in I'd prefer if they were
split as I suggested above.


What I'm not so happy about is us potentially moving a problem from being a
compile time error, which is easy to observe, to a run-time behavioural
problem, which may be much less obvious to the beholder.

Re: [PATCH 60/62] ARM: shmobile: work around CONFIG_PHYLIB=m

[Arnd Bergmann]

On Monday 24 March 2014 10:35:56 Simon Horman wrote:
> On Fri, Mar 21, 2014 at 04:43:24PM +0100, Arnd Bergmann wrote:
> > On Thursday 20 March 2014, Simon Horman wrote:
> > > I wonder if Kconfig for koelsch should be tightened up somehow to
> > > ensure that PHYLIB is either unselected or builtin.
> > > 
> > > Also, a minor nit, I would prefer changes for different boards
> > > in different patches. But I can split the patch myself if its
> > > not going to be changed otherwise.
> > 
> > I would prefer to take the entire series directly into arm-soc
> > this time, if you don't mind.
> 
> That I'm happy to go along with though I don't understand the motivation
> for it. And regardless of how the patches go in I'd prefer if they were
> split as I suggested above.

Sorry, I misunderstood you at first. I thought you meant you only split
them up when you apply them yourself. It's no problem for me to split
up this patch as well and then apply it here.

> What I'm not so happy about is us potentially moving a problem from being a
> compile time error, which is easy to observe, to a run-time behavioural
> problem, which may be much less obvious to the beholder.

I agree that it's not nice, but I couldn't come with a better approach,
and we are doing the same thing on other platforms as well.

The problems are:

* leaving the code as it is today prevents me from running all
  'make randconfig' kernels successfully. I use this as an important
  test case for verifying stuff we pull into the arm-soc tree.
  At the moment, I have a 160-patch series that I want to get merged
  upstream over time.

* Using 'select PHYLIB' from the platform is nasty because we want
  to be able to turn off whole subsystems (BLOCK, NET, I2C, ...)
  in Kconfig independent of the board selection. PHYLIB requires
  the network code.

* Using 'depends on' to disable the board option if PHYLIB is
  a module is counterintuitive.

* Makeing PHYLIB always built-in when NETDEVICE is would be a
  bit wasteful.

I don't have any other ideas for solving this.

	Arnd

Re: [PATCH 60/62] ARM: shmobile: work around CONFIG_PHYLIB=m

[Uwe Kleine-König]

On Mon, Mar 24, 2014 at 01:04:57PM +0100, Arnd Bergmann wrote:
> On Monday 24 March 2014 10:35:56 Simon Horman wrote:
> > On Fri, Mar 21, 2014 at 04:43:24PM +0100, Arnd Bergmann wrote:
> > > On Thursday 20 March 2014, Simon Horman wrote:
> > > > I wonder if Kconfig for koelsch should be tightened up somehow to
> > > > ensure that PHYLIB is either unselected or builtin.
> > > > 
> > > > Also, a minor nit, I would prefer changes for different boards
> > > > in different patches. But I can split the patch myself if its
> > > > not going to be changed otherwise.
> > > 
> > > I would prefer to take the entire series directly into arm-soc
> > > this time, if you don't mind.
> > 
> > That I'm happy to go along with though I don't understand the motivation
> > for it. And regardless of how the patches go in I'd prefer if they were
> > split as I suggested above.
> 
> Sorry, I misunderstood you at first. I thought you meant you only split
> them up when you apply them yourself. It's no problem for me to split
> up this patch as well and then apply it here.
> 
> > What I'm not so happy about is us potentially moving a problem from being a
> > compile time error, which is easy to observe, to a run-time behavioural
> > problem, which may be much less obvious to the beholder.
> 
> I agree that it's not nice, but I couldn't come with a better approach,
> and we are doing the same thing on other platforms as well.
> 
> The problems are:
> 
> * leaving the code as it is today prevents me from running all
>   'make randconfig' kernels successfully. I use this as an important
>   test case for verifying stuff we pull into the arm-soc tree.
>   At the moment, I have a 160-patch series that I want to get merged
>   upstream over time.
> 
> * Using 'select PHYLIB' from the platform is nasty because we want
>   to be able to turn off whole subsystems (BLOCK, NET, I2C, ...)
>   in Kconfig independent of the board selection. PHYLIB requires
>   the network code.
> 
> * Using 'depends on' to disable the board option if PHYLIB is
>   a module is counterintuitive.
> 
> * Makeing PHYLIB always built-in when NETDEVICE is would be a
>   bit wasteful.
maybe let the board

	select PHYLIB if NETDEVICE

is a nice compromise?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |