linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [GIT PULL] qcom SoC changes for 4.4 *RESEND*
@ 2015-10-14 22:10 Andy Gross
  2015-10-15 21:05 ` Arnd Bergmann
  0 siblings, 1 reply; 9+ messages in thread
From: Andy Gross @ 2015-10-14 22:10 UTC (permalink / raw)
  To: linux-arm-kernel

The following changes since commit 6ff33f3902c3b1c5d0db6b1e2c70b6d76fba357f:

  Linux 4.3-rc1 (2015-09-12 16:35:56 -0700)

are available in the git repository at:

  git://codeaurora.org/quic/kernel/agross-msm.git tags/qcom-soc-for-4.4

for you to fetch changes up to d0bfd7c9b162612de55ca2d204403b90dc278db6:

  soc: qcom: smem: Move RPM message ram out of smem DT node (2015-10-14 14:51:40 -0500)

----------------------------------------------------------------
Qualcomm ARM Based SoC Updates for 4.4

* Implement id_table driver matching in SMD
* Avoid NULL pointer exception on remove of SMEM
* Reorder SMEM/SMD configs
* Make qcom_smem_get() return a pointer
* Handle big endian CPUs correctly in SMEM
* Represent SMD channel layout in structures
* Use __iowrite32_copy() in SMD
* Remove use of VLAIs in SMD
* Handle big endian CPUs correctly in SMD/RPM
* Handle big endian CPUs corretly in SMD
* Reject sending SMD packets that are too large
* Fix endianness issue in SCM __qcom_scm_is_call_available
* Add missing prototype for qcom_scm_is_available()
* Correct SMEM items for upper channels
* Use architecture level to build SCM correctly
* Delete unneeded of_node_put in SMD
* Correct active/slep state flagging in SMD/RPM
* Move RPM message ram out of SMEM DT node

----------------------------------------------------------------
Andy Gross (2):
      firmware: qcom: scm: Add function stubs for ARM64
      Merge tag 'qcom-fixes-for-4.3-rc1' into base-for-4.4

Arnd Bergmann (1):
      firmware: qcom-scm: build for correct architecture level

Bjorn Andersson (4):
      soc: qcom: smd: Implement id_table driver matching
      soc: qcom: smd: Reject send of too big packets
      soc: qcom: smd: Correct SMEM items for upper channels
      soc: qcom: smd-rpm: Correct the active vs sleep state flagging

Julia Lawall (1):
      soc: qcom: smd: delete unneeded of_node_put

Rob Clark (2):
      qcom-scm: fix endianess issue in __qcom_scm_is_call_available
      qcom-scm: add missing prototype for qcom_scm_is_available()

Stephen Boyd (10):
      soc: qcom: smem: Avoid NULL pointer exception on remove
      soc: qcom: Reorder SMEM/SMD configs
      soc: qcom: Make qcom_smem_get() return a pointer
      soc: qcom: smem: Handle big endian CPUs
      soc: qcom: smd: Represent channel layout in structures
      soc: qcom: smd: Use __iowrite32_copy() instead of open-coding it
      soc: qcom: smd: Remove use of VLAIS
      soc: qcom: smd_rpm: Handle big endian CPUs
      soc: qcom: smd: Handle big endian CPUs
      soc: qcom: smem: Move RPM message ram out of smem DT node

 arch/arm/boot/dts/qcom-msm8974.dtsi |   17 +-
 drivers/firmware/Kconfig            |    8 +
 drivers/firmware/Makefile           |    5 +-
 drivers/firmware/qcom_scm-32.c      |    6 +-
 drivers/firmware/qcom_scm-64.c      |   63 ++++++
 drivers/soc/qcom/Kconfig            |   16 +-
 drivers/soc/qcom/smd-rpm.c          |   68 ++++---
 drivers/soc/qcom/smd.c              |  296 +++++++++++++++++-----------
 drivers/soc/qcom/smem.c             |  368 +++++++++++++++++++----------------
 include/linux/qcom_scm.h            |    2 +
 include/linux/soc/qcom/smd.h        |   11 ++
 include/linux/soc/qcom/smem.h       |    2 +-
 12 files changed, 525 insertions(+), 337 deletions(-)
 create mode 100644 drivers/firmware/qcom_scm-64.c

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

* [GIT PULL] qcom SoC changes for 4.4 *RESEND*
  2015-10-14 22:10 [GIT PULL] qcom SoC changes for 4.4 *RESEND* Andy Gross
@ 2015-10-15 21:05 ` Arnd Bergmann
  2015-10-16  2:28   ` Andy Gross
  2015-10-16 16:56   ` Stephen Boyd
  0 siblings, 2 replies; 9+ messages in thread
From: Arnd Bergmann @ 2015-10-15 21:05 UTC (permalink / raw)
  To: linux-arm-kernel

On Wednesday 14 October 2015 17:10:27 Andy Gross wrote:
> Qualcomm ARM Based SoC Updates for 4.4
> 
> * Implement id_table driver matching in SMD
> * Avoid NULL pointer exception on remove of SMEM
> * Reorder SMEM/SMD configs
> * Make qcom_smem_get() return a pointer
> * Handle big endian CPUs correctly in SMEM
> * Represent SMD channel layout in structures
> * Use __iowrite32_copy() in SMD
> * Remove use of VLAIs in SMD
> * Handle big endian CPUs correctly in SMD/RPM
> * Handle big endian CPUs corretly in SMD
> * Reject sending SMD packets that are too large
> * Fix endianness issue in SCM __qcom_scm_is_call_available
> * Add missing prototype for qcom_scm_is_available()
> * Correct SMEM items for upper channels
> * Use architecture level to build SCM correctly
> * Delete unneeded of_node_put in SMD
> * Correct active/slep state flagging in SMD/RPM
> * Move RPM message ram out of SMEM DT node
> 

Pulled into next/drivers, but please try to make the changelogs more readable
in the future. No point listing each small patch here, instead say what you've
done overall and what the major things are. Right now, the merge commit
just duplicates the information like this:

commit 90bb7e0e4f1ad8714f39db232ef14c588297346d
Merge: 5462b10af11d d0bfd7c9b162
Author: Arnd Bergmann <arnd@arndb.de>
Date:   Thu Oct 15 22:56:52 2015 +0200

    Merge tag 'qcom-soc-for-4.4' of git://codeaurora.org/quic/kernel/agross-msm into next/drivers
    
    Pull "Qualcomm ARM Based SoC Updates for 4.4" from Andy Gross:
    
    * Implement id_table driver matching in SMD
    * Avoid NULL pointer exception on remove of SMEM
    * Reorder SMEM/SMD configs
    * Make qcom_smem_get() return a pointer
    * Handle big endian CPUs correctly in SMEM
    * Represent SMD channel layout in structures
    * Use __iowrite32_copy() in SMD
    * Remove use of VLAIs in SMD
    * Handle big endian CPUs correctly in SMD/RPM
    * Handle big endian CPUs corretly in SMD
    * Reject sending SMD packets that are too large
    * Fix endianness issue in SCM __qcom_scm_is_call_available
    * Add missing prototype for qcom_scm_is_available()
    * Correct SMEM items for upper channels
    * Use architecture level to build SCM correctly
    * Delete unneeded of_node_put in SMD
    * Correct active/slep state flagging in SMD/RPM
    * Move RPM message ram out of SMEM DT node
    
    * tag 'qcom-soc-for-4.4' of git://codeaurora.org/quic/kernel/agross-msm:
      soc: qcom: smem: Move RPM message ram out of smem DT node
      soc: qcom: smd-rpm: Correct the active vs sleep state flagging
      soc: qcom: smd: delete unneeded of_node_put
      firmware: qcom-scm: build for correct architecture level
      soc: qcom: smd: Correct SMEM items for upper channels
      qcom-scm: add missing prototype for qcom_scm_is_available()
      qcom-scm: fix endianess issue in __qcom_scm_is_call_available
      soc: qcom: smd: Reject send of too big packets
      soc: qcom: smd: Handle big endian CPUs
      soc: qcom: smd_rpm: Handle big endian CPUs
      soc: qcom: smd: Remove use of VLAIS
      soc: qcom: smd: Use __iowrite32_copy() instead of open-coding it
      soc: qcom: smd: Represent channel layout in structures
      soc: qcom: smem: Handle big endian CPUs
      soc: qcom: Make qcom_smem_get() return a pointer
      soc: qcom: Reorder SMEM/SMD configs
      soc: qcom: smem: Avoid NULL pointer exception on remove
      soc: qcom: smd: Implement id_table driver matching

which looks a bit silly. This also seemed like the right place
to add a bugfix that I had in my queue:

commit 73ebb85444b0472d90bb70a1a9e6b5df3f92c14c
Author: Arnd Bergmann <arnd@arndb.de>
Date:   Tue Oct 13 17:05:39 2015 +0200

    soc: qcom/smem: add HWSPINLOCK dependency
    
    This fixes a build error when smem is enabled without hwspinlock:
    
    drivers/built-in.o: In function `qcom_smem_alloc':
    rockchip-efuse.c:(.text+0x7a3e4): undefined reference to `__hwspin_lock_timeout'
    rockchip-efuse.c:(.text+0x7a568): undefined reference to `__hwspin_unlock'
    drivers/built-in.o: In function `qcom_smem_remove':
    rockchip-efuse.c:(.text+0x7a5cc): undefined reference to `hwspin_lock_free'
    drivers/built-in.o: In function `qcom_smem_probe':
    rockchip-efuse.c:(.text+0x7a960): undefined reference to `hwspin_lock_request_specific'
    rockchip-efuse.c:(.text+0x7a988): undefined reference to `of_hwspin_lock_get_id'
    drivers/built-in.o: In function `qcom_smem_get':
    rockchip-efuse.c:(.text+0x7aa24): undefined reference to `__hwspin_lock_timeout'
    rockchip-efuse.c:(.text+0x7aafc): undefined reference to `__hwspin_unlock'
    
    Signed-off-by: Arnd Bergmann <arnd@arndb.de>

diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index 3e4d2133c3d2..eec76141d9b9 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -22,6 +22,7 @@ config QCOM_PM
 config QCOM_SMEM
 	tristate "Qualcomm Shared Memory Manager (SMEM)"
 	depends on ARCH_QCOM
+	depends on HWSPINLOCK
 	help
 	  Say y here to enable support for the Qualcomm Shared Memory Manager.
 	  The driver provides an interface to items in a heap shared among all

and I applied that on top.

	Arnd

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

* [GIT PULL] qcom SoC changes for 4.4 *RESEND*
  2015-10-15 21:05 ` Arnd Bergmann
@ 2015-10-16  2:28   ` Andy Gross
  2015-10-16 16:56   ` Stephen Boyd
  1 sibling, 0 replies; 9+ messages in thread
From: Andy Gross @ 2015-10-16  2:28 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Oct 15, 2015 at 11:05:27PM +0200, Arnd Bergmann wrote:
> On Wednesday 14 October 2015 17:10:27 Andy Gross wrote:
> > Qualcomm ARM Based SoC Updates for 4.4
> > 
> > * Implement id_table driver matching in SMD
> > * Avoid NULL pointer exception on remove of SMEM
> > * Reorder SMEM/SMD configs
> > * Make qcom_smem_get() return a pointer
> > * Handle big endian CPUs correctly in SMEM
> > * Represent SMD channel layout in structures
> > * Use __iowrite32_copy() in SMD
> > * Remove use of VLAIs in SMD
> > * Handle big endian CPUs correctly in SMD/RPM
> > * Handle big endian CPUs corretly in SMD
> > * Reject sending SMD packets that are too large
> > * Fix endianness issue in SCM __qcom_scm_is_call_available
> > * Add missing prototype for qcom_scm_is_available()
> > * Correct SMEM items for upper channels
> > * Use architecture level to build SCM correctly
> > * Delete unneeded of_node_put in SMD
> > * Correct active/slep state flagging in SMD/RPM
> > * Move RPM message ram out of SMEM DT node
> > 
> 
> Pulled into next/drivers, but please try to make the changelogs more readable
> in the future. No point listing each small patch here, instead say what you've
> done overall and what the major things are. Right now, the merge commit
> just duplicates the information like this:

Fair enough.  I'll condense it next time.  Thanks for the hwspinlock fix on top.

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

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

* [GIT PULL] qcom SoC changes for 4.4 *RESEND*
  2015-10-15 21:05 ` Arnd Bergmann
  2015-10-16  2:28   ` Andy Gross
@ 2015-10-16 16:56   ` Stephen Boyd
  2015-10-16 19:19     ` Arnd Bergmann
  1 sibling, 1 reply; 9+ messages in thread
From: Stephen Boyd @ 2015-10-16 16:56 UTC (permalink / raw)
  To: linux-arm-kernel

On 10/15/2015 02:05 PM, Arnd Bergmann wrote:
>
>  This also seemed like the right place
> to add a bugfix that I had in my queue:
>
> commit 73ebb85444b0472d90bb70a1a9e6b5df3f92c14c
> Author: Arnd Bergmann <arnd@arndb.de>
> Date:   Tue Oct 13 17:05:39 2015 +0200
>
>     soc: qcom/smem: add HWSPINLOCK dependency
>     
>     This fixes a build error when smem is enabled without hwspinlock:
>     
>     drivers/built-in.o: In function `qcom_smem_alloc':
>     rockchip-efuse.c:(.text+0x7a3e4): undefined reference to `__hwspin_lock_timeout'
>     rockchip-efuse.c:(.text+0x7a568): undefined reference to `__hwspin_unlock'
>     drivers/built-in.o: In function `qcom_smem_remove':
>     rockchip-efuse.c:(.text+0x7a5cc): undefined reference to `hwspin_lock_free'
>     drivers/built-in.o: In function `qcom_smem_probe':
>     rockchip-efuse.c:(.text+0x7a960): undefined reference to `hwspin_lock_request_specific'
>     rockchip-efuse.c:(.text+0x7a988): undefined reference to `of_hwspin_lock_get_id'
>     drivers/built-in.o: In function `qcom_smem_get':
>     rockchip-efuse.c:(.text+0x7aa24): undefined reference to `__hwspin_lock_timeout'
>     rockchip-efuse.c:(.text+0x7aafc): undefined reference to `__hwspin_unlock'
>     

Can you share your .config? It looks like there are stubs for these, so
I'm lost how we got undefined references.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

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

* [GIT PULL] qcom SoC changes for 4.4 *RESEND*
  2015-10-16 16:56   ` Stephen Boyd
@ 2015-10-16 19:19     ` Arnd Bergmann
  2015-10-16 20:04       ` Stephen Boyd
  0 siblings, 1 reply; 9+ messages in thread
From: Arnd Bergmann @ 2015-10-16 19:19 UTC (permalink / raw)
  To: linux-arm-kernel

On Friday 16 October 2015 09:56:30 Stephen Boyd wrote:
> On 10/15/2015 02:05 PM, Arnd Bergmann wrote:
> >
> >  This also seemed like the right place
> > to add a bugfix that I had in my queue:
> >
> > commit 73ebb85444b0472d90bb70a1a9e6b5df3f92c14c
> > Author: Arnd Bergmann <arnd@arndb.de>
> > Date:   Tue Oct 13 17:05:39 2015 +0200
> >
> >     soc: qcom/smem: add HWSPINLOCK dependency
> >     
> >     This fixes a build error when smem is enabled without hwspinlock:
> >     
> >     drivers/built-in.o: In function `qcom_smem_alloc':
> >     rockchip-efuse.c:(.text+0x7a3e4): undefined reference to `__hwspin_lock_timeout'
> >     rockchip-efuse.c:(.text+0x7a568): undefined reference to `__hwspin_unlock'
> >     drivers/built-in.o: In function `qcom_smem_remove':
> >     rockchip-efuse.c:(.text+0x7a5cc): undefined reference to `hwspin_lock_free'
> >     drivers/built-in.o: In function `qcom_smem_probe':
> >     rockchip-efuse.c:(.text+0x7a960): undefined reference to `hwspin_lock_request_specific'
> >     rockchip-efuse.c:(.text+0x7a988): undefined reference to `of_hwspin_lock_get_id'
> >     drivers/built-in.o: In function `qcom_smem_get':
> >     rockchip-efuse.c:(.text+0x7aa24): undefined reference to `__hwspin_lock_timeout'
> >     rockchip-efuse.c:(.text+0x7aafc): undefined reference to `__hwspin_unlock'
> >     
> 
> Can you share your .config? It looks like there are stubs for these, so
> I'm lost how we got undefined references.
> 
http://pastebin.com/HtrC510p

The problem is CONFIG_HWSPINLOCK=m && CONFIG_QCOM_SMEM=y. Sorry for sending
an incomplete patch description, I had not noticed those wrapper functions.

My patch fixes the issue, but does not allow you to build QCOM_SMEM without
HWSPINLOCK. If we want that configuration to be valid, we need one of
the two changes below:

diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index eec76141d9b9..cae0ffa19bca 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -22,7 +22,7 @@ config QCOM_PM
 config QCOM_SMEM
 	tristate "Qualcomm Shared Memory Manager (SMEM)"
 	depends on ARCH_QCOM
-	depends on HWSPINLOCK
+	depends on HWSPINLOCK && !HWSPINLOCK
 	help
 	  Say y here to enable support for the Qualcomm Shared Memory Manager.
 	  The driver provides an interface to items in a heap shared among all
diff --git a/include/linux/hwspinlock.h b/include/linux/hwspinlock.h
index 859d673d98c8..cdfd9fd2ba11 100644
--- a/include/linux/hwspinlock.h
+++ b/include/linux/hwspinlock.h
@@ -59,7 +59,7 @@ struct hwspinlock_pdata {
 	int base_id;
 };
 
-#if defined(CONFIG_HWSPINLOCK) || defined(CONFIG_HWSPINLOCK_MODULE)
+#if IS_REACHABLE(CONFIG_HWSPINLOCK)
 
 int hwspin_lock_register(struct hwspinlock_device *bank, struct device *dev,
 		const struct hwspinlock_ops *ops, int base_id, int num_locks);



	Arnd

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

* [GIT PULL] qcom SoC changes for 4.4 *RESEND*
  2015-10-16 19:19     ` Arnd Bergmann
@ 2015-10-16 20:04       ` Stephen Boyd
  2015-10-16 20:17         ` Arnd Bergmann
  0 siblings, 1 reply; 9+ messages in thread
From: Stephen Boyd @ 2015-10-16 20:04 UTC (permalink / raw)
  To: linux-arm-kernel

On 10/16, Arnd Bergmann wrote:
> On Friday 16 October 2015 09:56:30 Stephen Boyd wrote:
> > 
> > Can you share your .config? It looks like there are stubs for these, so
> > I'm lost how we got undefined references.
> > 
> http://pastebin.com/HtrC510p
> 
> The problem is CONFIG_HWSPINLOCK=m && CONFIG_QCOM_SMEM=y. Sorry for sending
> an incomplete patch description, I had not noticed those wrapper functions.
> 
> My patch fixes the issue, but does not allow you to build QCOM_SMEM without
> HWSPINLOCK. If we want that configuration to be valid, we need one of
> the two changes below:
> 
> diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
> index eec76141d9b9..cae0ffa19bca 100644
> --- a/drivers/soc/qcom/Kconfig
> +++ b/drivers/soc/qcom/Kconfig
> @@ -22,7 +22,7 @@ config QCOM_PM
>  config QCOM_SMEM
>  	tristate "Qualcomm Shared Memory Manager (SMEM)"
>  	depends on ARCH_QCOM
> -	depends on HWSPINLOCK
> +	depends on HWSPINLOCK && !HWSPINLOCK

What does this mean? Is it missing an =m?

>  	help
>  	  Say y here to enable support for the Qualcomm Shared Memory Manager.
>  	  The driver provides an interface to items in a heap shared among all
> diff --git a/include/linux/hwspinlock.h b/include/linux/hwspinlock.h
> index 859d673d98c8..cdfd9fd2ba11 100644
> --- a/include/linux/hwspinlock.h
> +++ b/include/linux/hwspinlock.h
> @@ -59,7 +59,7 @@ struct hwspinlock_pdata {
>  	int base_id;
>  };
>  
> -#if defined(CONFIG_HWSPINLOCK) || defined(CONFIG_HWSPINLOCK_MODULE)
> +#if IS_REACHABLE(CONFIG_HWSPINLOCK)
>  
>  int hwspin_lock_register(struct hwspinlock_device *bank, struct device *dev,
>  		const struct hwspinlock_ops *ops, int base_id, int num_locks);
> 

I'd prefer this patch. Is there some way to capture that
QCOM_SMEM must be a module if HWSPINLOCK is a module unless we're
doing a compile test, in which case we don't care?

I guess that would just be:

	depends on HWSPINLOCK || COMPILE_TEST

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

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

* [GIT PULL] qcom SoC changes for 4.4 *RESEND*
  2015-10-16 20:04       ` Stephen Boyd
@ 2015-10-16 20:17         ` Arnd Bergmann
  2015-10-16 21:26           ` Stephen Boyd
  0 siblings, 1 reply; 9+ messages in thread
From: Arnd Bergmann @ 2015-10-16 20:17 UTC (permalink / raw)
  To: linux-arm-kernel

On Friday 16 October 2015 13:04:17 Stephen Boyd wrote:
> On 10/16, Arnd Bergmann wrote:
> > On Friday 16 October 2015 09:56:30 Stephen Boyd wrote:
> > > 
> > > Can you share your .config? It looks like there are stubs for these, so
> > > I'm lost how we got undefined references.
> > > 
> > http://pastebin.com/HtrC510p
> > 
> > The problem is CONFIG_HWSPINLOCK=m && CONFIG_QCOM_SMEM=y. Sorry for sending
> > an incomplete patch description, I had not noticed those wrapper functions.
> > 
> > My patch fixes the issue, but does not allow you to build QCOM_SMEM without
> > HWSPINLOCK. If we want that configuration to be valid, we need one of
> > the two changes below:
> > 
> > diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
> > index eec76141d9b9..cae0ffa19bca 100644
> > --- a/drivers/soc/qcom/Kconfig
> > +++ b/drivers/soc/qcom/Kconfig
> > @@ -22,7 +22,7 @@ config QCOM_PM
> >  config QCOM_SMEM
> >  	tristate "Qualcomm Shared Memory Manager (SMEM)"
> >  	depends on ARCH_QCOM
> > -	depends on HWSPINLOCK
> > +	depends on HWSPINLOCK && !HWSPINLOCK
> 
> What does this mean?

This is the magic way in Kconfig to express what we want ;-)

It says it depends on either HWSPINLOCK being disabled (!HWSPINLOCK),
or it depends on HWSPINLOCK. The latter implies that QCOM_SMEM cannot
be built-in when HWSPINLOCK=m, but they are allowed to both be built-in.

> Is it missing an =m?

No

> >  	help
> >  	  Say y here to enable support for the Qualcomm Shared Memory Manager.
> >  	  The driver provides an interface to items in a heap shared among all
> > diff --git a/include/linux/hwspinlock.h b/include/linux/hwspinlock.h
> > index 859d673d98c8..cdfd9fd2ba11 100644
> > --- a/include/linux/hwspinlock.h
> > +++ b/include/linux/hwspinlock.h
> > @@ -59,7 +59,7 @@ struct hwspinlock_pdata {
> >  	int base_id;
> >  };
> >  
> > -#if defined(CONFIG_HWSPINLOCK) || defined(CONFIG_HWSPINLOCK_MODULE)
> > +#if IS_REACHABLE(CONFIG_HWSPINLOCK)
> >  
> >  int hwspin_lock_register(struct hwspinlock_device *bank, struct device *dev,
> >  		const struct hwspinlock_ops *ops, int base_id, int num_locks);
> > 
> 
> I'd prefer this patch. Is there some way to capture that
> QCOM_SMEM must be a module if HWSPINLOCK is a module unless we're
> doing a compile test, in which case we don't care?

The problem here is that while we avoid the build error, we get the
counterintuitive behavior that you can have the HWSPINLOCK module
loaded but the built-in QCOM_SMEM module does not use it because
it wasn't reachable at compile time.

> I guess that would just be:
> 
> 	depends on HWSPINLOCK || COMPILE_TEST

This would not solve the randconfig build problem.

	Arnd

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

* [GIT PULL] qcom SoC changes for 4.4 *RESEND*
  2015-10-16 20:17         ` Arnd Bergmann
@ 2015-10-16 21:26           ` Stephen Boyd
  2015-10-16 21:37             ` Arnd Bergmann
  0 siblings, 1 reply; 9+ messages in thread
From: Stephen Boyd @ 2015-10-16 21:26 UTC (permalink / raw)
  To: linux-arm-kernel

On 10/16, Arnd Bergmann wrote:
> On Friday 16 October 2015 13:04:17 Stephen Boyd wrote:
> > On 10/16, Arnd Bergmann wrote:
> > > On Friday 16 October 2015 09:56:30 Stephen Boyd wrote:
> > > > 
> > > > Can you share your .config? It looks like there are stubs for these, so
> > > > I'm lost how we got undefined references.
> > > > 
> > > http://pastebin.com/HtrC510p
> > > 
> > > The problem is CONFIG_HWSPINLOCK=m && CONFIG_QCOM_SMEM=y. Sorry for sending
> > > an incomplete patch description, I had not noticed those wrapper functions.
> > > 
> > > My patch fixes the issue, but does not allow you to build QCOM_SMEM without
> > > HWSPINLOCK. If we want that configuration to be valid, we need one of
> > > the two changes below:
> > > 
> > > diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
> > > index eec76141d9b9..cae0ffa19bca 100644
> > > --- a/drivers/soc/qcom/Kconfig
> > > +++ b/drivers/soc/qcom/Kconfig
> > > @@ -22,7 +22,7 @@ config QCOM_PM
> > >  config QCOM_SMEM
> > >  	tristate "Qualcomm Shared Memory Manager (SMEM)"
> > >  	depends on ARCH_QCOM
> > > -	depends on HWSPINLOCK
> > > +	depends on HWSPINLOCK && !HWSPINLOCK
> > 
> > What does this mean?
> 
> This is the magic way in Kconfig to express what we want ;-)
> 
> It says it depends on either HWSPINLOCK being disabled (!HWSPINLOCK),
> or it depends on HWSPINLOCK. The latter implies that QCOM_SMEM cannot
> be built-in when HWSPINLOCK=m, but they are allowed to both be built-in.
> 

I think you mean OR then?

	depends on HWSPINLOCK || !HWSPINLOCK

If I have HWSPINLOCK=y and try menuconfig with your patch, the
smem config isn't selectable. But if HWSPINLOCK=m then the smem
config is visible and smem can only be a module.

> 
> > >  	help
> > >  	  Say y here to enable support for the Qualcomm Shared Memory Manager.
> > >  	  The driver provides an interface to items in a heap shared among all
> > > diff --git a/include/linux/hwspinlock.h b/include/linux/hwspinlock.h
> > > index 859d673d98c8..cdfd9fd2ba11 100644
> > > --- a/include/linux/hwspinlock.h
> > > +++ b/include/linux/hwspinlock.h
> > > @@ -59,7 +59,7 @@ struct hwspinlock_pdata {
> > >  	int base_id;
> > >  };
> > >  
> > > -#if defined(CONFIG_HWSPINLOCK) || defined(CONFIG_HWSPINLOCK_MODULE)
> > > +#if IS_REACHABLE(CONFIG_HWSPINLOCK)
> > >  
> > >  int hwspin_lock_register(struct hwspinlock_device *bank, struct device *dev,
> > >  		const struct hwspinlock_ops *ops, int base_id, int num_locks);
> > > 
> > 
> > I'd prefer this patch. Is there some way to capture that
> > QCOM_SMEM must be a module if HWSPINLOCK is a module unless we're
> > doing a compile test, in which case we don't care?
> 
> The problem here is that while we avoid the build error, we get the
> counterintuitive behavior that you can have the HWSPINLOCK module
> loaded but the built-in QCOM_SMEM module does not use it because
> it wasn't reachable at compile time.
> 

Agreed, which is why I suggested depends on HWSPINLOCK ||
COMPILE_TEST + the IS_REACHABLE patch. That way if hwspinlock=y,
smem can be y or m, if hwspinlock=m, smem can only be m, and if
hwspinlock=n then smem can only be y or m if it has
COMPILE_TEST=y. In the last case the IS_REACHABLE ifdef will "do
the right thing" and make it so that HWSPINLOCK symbols are
stubbed out if hwspinlock=m and smem=y or hwspinlock=n and
smem=y/m.

I don't think we ever care about a kernel that actually runs when
COMPILE_TEST=y, so this seems to be ok.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

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

* [GIT PULL] qcom SoC changes for 4.4 *RESEND*
  2015-10-16 21:26           ` Stephen Boyd
@ 2015-10-16 21:37             ` Arnd Bergmann
  0 siblings, 0 replies; 9+ messages in thread
From: Arnd Bergmann @ 2015-10-16 21:37 UTC (permalink / raw)
  To: linux-arm-kernel

On Friday 16 October 2015 14:26:27 Stephen Boyd wrote:
> On 10/16, Arnd Bergmann wrote:
> > On Friday 16 October 2015 13:04:17 Stephen Boyd wrote:
> > > On 10/16, Arnd Bergmann wrote:
> > > > On Friday 16 October 2015 09:56:30 Stephen Boyd wrote:
> > > > > 
> > > > > Can you share your .config? It looks like there are stubs for these, so
> > > > > I'm lost how we got undefined references.
> > > > > 
> > > > http://pastebin.com/HtrC510p
> > > > 
> > > > The problem is CONFIG_HWSPINLOCK=m && CONFIG_QCOM_SMEM=y. Sorry for sending
> > > > an incomplete patch description, I had not noticed those wrapper functions.
> > > > 
> > > > My patch fixes the issue, but does not allow you to build QCOM_SMEM without
> > > > HWSPINLOCK. If we want that configuration to be valid, we need one of
> > > > the two changes below:
> > > > 
> > > > diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
> > > > index eec76141d9b9..cae0ffa19bca 100644
> > > > --- a/drivers/soc/qcom/Kconfig
> > > > +++ b/drivers/soc/qcom/Kconfig
> > > > @@ -22,7 +22,7 @@ config QCOM_PM
> > > >  config QCOM_SMEM
> > > >  	tristate "Qualcomm Shared Memory Manager (SMEM)"
> > > >  	depends on ARCH_QCOM
> > > > -	depends on HWSPINLOCK
> > > > +	depends on HWSPINLOCK && !HWSPINLOCK
> > > 
> > > What does this mean?
> > 
> > This is the magic way in Kconfig to express what we want ;-)
> > 
> > It says it depends on either HWSPINLOCK being disabled (!HWSPINLOCK),
> > or it depends on HWSPINLOCK. The latter implies that QCOM_SMEM cannot
> > be built-in when HWSPINLOCK=m, but they are allowed to both be built-in.
> > 
> 
> I think you mean OR then?
> 
> 	depends on HWSPINLOCK || !HWSPINLOCK

Yes, sorry about that.

> If I have HWSPINLOCK=y and try menuconfig with your patch, the
> smem config isn't selectable. But if HWSPINLOCK=m then the smem
> config is visible and smem can only be a module.

Ok, I have to admit that I don't even know how Kconfig interprets
(FOO && !FOO), so that's probably the result of my typo.

> > 
> > > >  	help
> > > >  	  Say y here to enable support for the Qualcomm Shared Memory Manager.
> > > >  	  The driver provides an interface to items in a heap shared among all
> > > > diff --git a/include/linux/hwspinlock.h b/include/linux/hwspinlock.h
> > > > index 859d673d98c8..cdfd9fd2ba11 100644
> > > > --- a/include/linux/hwspinlock.h
> > > > +++ b/include/linux/hwspinlock.h
> > > > @@ -59,7 +59,7 @@ struct hwspinlock_pdata {
> > > >  	int base_id;
> > > >  };
> > > >  
> > > > -#if defined(CONFIG_HWSPINLOCK) || defined(CONFIG_HWSPINLOCK_MODULE)
> > > > +#if IS_REACHABLE(CONFIG_HWSPINLOCK)
> > > >  
> > > >  int hwspin_lock_register(struct hwspinlock_device *bank, struct device *dev,
> > > >  		const struct hwspinlock_ops *ops, int base_id, int num_locks);
> > > > 
> > > 
> > > I'd prefer this patch. Is there some way to capture that
> > > QCOM_SMEM must be a module if HWSPINLOCK is a module unless we're
> > > doing a compile test, in which case we don't care?
> > 
> > The problem here is that while we avoid the build error, we get the
> > counterintuitive behavior that you can have the HWSPINLOCK module
> > loaded but the built-in QCOM_SMEM module does not use it because
> > it wasn't reachable at compile time.
> > 
> 
> Agreed, which is why I suggested depends on HWSPINLOCK ||
> COMPILE_TEST + the IS_REACHABLE patch. That way if hwspinlock=y,
> smem can be y or m, if hwspinlock=m, smem can only be m, and if
> hwspinlock=n then smem can only be y or m if it has
> COMPILE_TEST=y. In the last case the IS_REACHABLE ifdef will "do
> the right thing" and make it so that HWSPINLOCK symbols are
> stubbed out if hwspinlock=m and smem=y or hwspinlock=n and
> smem=y/m.

Ah, I see. The downside is still that other users of HWSPINLOCK
might now accidentally build but not work if HWSPINLOCK=m.
 
> I don't think we ever care about a kernel that actually runs when
> COMPILE_TEST=y, so this seems to be ok.

Right, for the QCOM_SMEM support that is clearly ok as long as you
have the Kconfig logic there.

	Arnd

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

end of thread, other threads:[~2015-10-16 21:37 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-14 22:10 [GIT PULL] qcom SoC changes for 4.4 *RESEND* Andy Gross
2015-10-15 21:05 ` Arnd Bergmann
2015-10-16  2:28   ` Andy Gross
2015-10-16 16:56   ` Stephen Boyd
2015-10-16 19:19     ` Arnd Bergmann
2015-10-16 20:04       ` Stephen Boyd
2015-10-16 20:17         ` Arnd Bergmann
2015-10-16 21:26           ` Stephen Boyd
2015-10-16 21:37             ` Arnd Bergmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).