Re: [GIT PATCH] char/misc patches for 3.10-rc1

Re: [GIT PATCH] char/misc patches for 3.10-rc1

[David Brown]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> There are other things wrong with that whole SSBI driver crap that you
> seem to be ignoring:
>
>  - it's not a bus, it's just a driver. Just because some people call
> it "serial bus" doesn't make it magically about a "bus". I can call an
> ethernet driver an "ethernet bus driver", and it may be technically
> correct, but it is still bullshit. And ethernet is damn more a real
> bus than that SSBI driver is. That's just a pure serial driver for a
> very specific piece of embedded hardware. Stop calling it a bus.

Correct.  Despite having "bus" in the name, it isn't really a bus.  It's
a point-to-point serial interface to a non-addressible device.

>  - The whole Kconfig thing is complete and utter garbage. There is no
> excuse what-so-ever for ever asking the user about it. Not on x86, not
> on ARM. The drivers that actually *use* that magical serial line
> driver should just have selected it.

Agreed

>  - I'm not seeing what commonalities this thing can have with anything
> else. Did anybody look at the code? There's nothing generic there.

This driver has one purpose, and will only ever have this one purpose:
to connect the MSM SOC to a small family of power management chips.  It
is theoretically possible to connect it to other things, and I think
there have been some obscure designs that do this, but not anything that
is going to be supported in the kernel.

> So move it to a saner place, fix the kconfig idiocy, and don't make
> noises as if it's some generic driver, much less some generic bus.
> It's not.

So, what is this saner place?  The hardware is theoretically shared
between ARM and Hexagon, but I don't know the hexagon plans to support
it, I've added them to the CC.

I'm not sure why this shouldn't be in the drivers/mfd directory
alongside the various pm*.c drivers that use it.  It isn't going to be
used for anything else.

Given the bikeshedding that happened when Ken pushed the driver out last
time, though, I'm sure this will create a firestorm of disagreement over
its location.

David

-- 
sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

Re: [GIT PATCH] char/misc patches for 3.10-rc1

[Arnd Bergmann]

On Monday 29 April 2013, David Brown wrote:
> I'm not sure why this shouldn't be in the drivers/mfd directory
> alongside the various pm*.c drivers that use it.  It isn't going to be
> used for anything else.
> 
> Given the bikeshedding that happened when Ken pushed the driver out last
> time, though, I'm sure this will create a firestorm of disagreement over
> its location.

Well, the only reason I see against putting it into drivers/mfd is that
this place is becoming a dumping ground for stuff that doesn't have a
real home anywhere else. On the other hand, this one would fit better
than a lot of drivers that are already there, and it would actually remove
the need for a global include/linux/*.h header file as the interface.

	Arnd

Re: [GIT PATCH] char/misc patches for 3.10-rc1

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 564 bytes --]

On Mon, Apr 29, 2013 at 11:16:28PM +0200, Arnd Bergmann wrote:

> Well, the only reason I see against putting it into drivers/mfd is that
> this place is becoming a dumping ground for stuff that doesn't have a
> real home anywhere else. On the other hand, this one would fit better
> than a lot of drivers that are already there, and it would actually remove
> the need for a global include/linux/*.h header file as the interface.

Well, this device should have a bunch of function drivers hanging off it
shouldn't it?  That's pretty much the definition of a MFD.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [GIT PATCH] char/misc patches for 3.10-rc1

[David Brown]

Mark Brown <broonie@kernel.org> writes:

> On Mon, Apr 29, 2013 at 11:16:28PM +0200, Arnd Bergmann wrote:
>
>> Well, the only reason I see against putting it into drivers/mfd is that
>> this place is becoming a dumping ground for stuff that doesn't have a
>> real home anywhere else. On the other hand, this one would fit better
>> than a lot of drivers that are already there, and it would actually remove
>> the need for a global include/linux/*.h header file as the interface.
>
> Well, this device should have a bunch of function drivers hanging off it
> shouldn't it?  That's pretty much the definition of a MFD.

Perhaps it's better to think of it as the library that other mfd devices
use to talk to their device.  ssbi is not itself an MFD device, it is
part of the other drivers.

David

-- 
sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

Re: [GIT PATCH] char/misc patches for 3.10-rc1

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 434 bytes --]

On Thu, May 02, 2013 at 01:53:03PM -0700, David Brown wrote:
> Mark Brown <broonie@kernel.org> writes:

> > Well, this device should have a bunch of function drivers hanging off it
> > shouldn't it?  That's pretty much the definition of a MFD.

> Perhaps it's better to think of it as the library that other mfd devices
> use to talk to their device.  ssbi is not itself an MFD device, it is
> part of the other drivers.

Yes, quite.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [GIT PATCH] char/misc patches for 3.10-rc1

[Linus Torvalds]

On Mon, Apr 29, 2013 at 2:08 PM, David Brown <davidb@codeaurora.org> wrote:
>
> So, what is this saner place?  The hardware is theoretically shared
> between ARM and Hexagon, but I don't know the hexagon plans to support
> it, I've added them to the CC.

So I actually wouldn't have complained about it in drivers/misc/ssbi/
or something like that. I don't think that's necessarily the best
place for it, and maybe somebody can come up with better places. But
even just in drivers/misc, at least you've moved it away to the point
that it doesn't potentially compete with the pathname auto-completion
of the fifty million other drivers we have under drivers/.

Or maybe "drivers/platform/something-or-other" if there are possible
other things that get shared together with this thing? I don't care
that deeply, as long as it's just off in a little corner of the
universe, rather than smack-dab in the middle.

              Linus

Re: [GIT PATCH] char/misc patches for 3.10-rc1

[Arnd Bergmann]

On Monday 29 April 2013, Linus Torvalds wrote:
> Or maybe "drivers/platform/something-or-other" if there are possible
> other things that get shared together with this thing? I don't care
> that deeply, as long as it's just off in a little corner of the
> universe, rather than smack-dab in the middle.

I usually try to prevent ARM platform specific code to go into
drivers/platform, because of fear that this might become the place
that people use to sneak in code we would not allow them to add
to arch/arm/ for one reason or another. Based on David's explanation,
I think drivers/mfd makes the most sense. I'll follow up with a patch.

	Arnd

MFD: move ssbi driver into drivers/mfd

[Arnd Bergmann]

There is no reason for ssbi to have its own top-level driver directory
when the only users of this interface are all MFD drivers. The only
mainline driver using it at the moment (PM8921) is marked broken and in
fact does not compile. I have verified that fixing the trivial build
breakage in pm8921 links in the new ssbi code just fine, but that
can be a separate patch.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Samuel Ortiz <sameo@linux.intel.com>
---
 drivers/Kconfig              |  2 --
 drivers/Makefile             |  1 -
 drivers/mfd/Kconfig          |  3 ++-
 drivers/mfd/Makefile         |  2 +-
 drivers/{ssbi => mfd}/ssbi.c |  0
 drivers/ssbi/Kconfig         | 16 ----------------
 drivers/ssbi/Makefile        |  1 -
 7 files changed, 3 insertions(+), 22 deletions(-)

diff --git a/drivers/Kconfig b/drivers/Kconfig
index 78a956e..202fa6d 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -52,8 +52,6 @@ source "drivers/i2c/Kconfig"
 
 source "drivers/spi/Kconfig"
 
-source "drivers/ssbi/Kconfig"
-
 source "drivers/hsi/Kconfig"
 
 source "drivers/pps/Kconfig"
diff --git a/drivers/Makefile b/drivers/Makefile
index 33360de..3c200a2 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -114,7 +114,6 @@ obj-y				+= firmware/
 obj-$(CONFIG_CRYPTO)		+= crypto/
 obj-$(CONFIG_SUPERH)		+= sh/
 obj-$(CONFIG_ARCH_SHMOBILE)	+= sh/
-obj-$(CONFIG_SSBI)		+= ssbi/
 ifndef CONFIG_ARCH_USES_GETTIMEOFFSET
 obj-y				+= clocksource/
 endif
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index ca86581..5150833 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -991,7 +991,8 @@ config MFD_PM8XXX
 
 config MFD_PM8921_CORE
 	tristate "Qualcomm PM8921 PMIC chip"
-	depends on SSBI && BROKEN
+	depends on (ARCH_MSM || HEXAGON)
+	depends on BROKEN
 	select MFD_CORE
 	select MFD_PM8XXX
 	help
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index b90409c..3b95b47 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -133,7 +133,7 @@ obj-$(CONFIG_MFD_VX855)		+= vx855.o
 obj-$(CONFIG_MFD_WL1273_CORE)	+= wl1273-core.o
 obj-$(CONFIG_MFD_CS5535)	+= cs5535-mfd.o
 obj-$(CONFIG_MFD_OMAP_USB_HOST)	+= omap-usb-host.o omap-usb-tll.o
-obj-$(CONFIG_MFD_PM8921_CORE) 	+= pm8921-core.o
+obj-$(CONFIG_MFD_PM8921_CORE) 	+= pm8921-core.o ssbi.o
 obj-$(CONFIG_MFD_PM8XXX_IRQ) 	+= pm8xxx-irq.o
 obj-$(CONFIG_TPS65911_COMPARATOR)	+= tps65911-comparator.o
 obj-$(CONFIG_MFD_TPS65090)	+= tps65090.o
diff --git a/drivers/ssbi/ssbi.c b/drivers/mfd/ssbi.c
similarity index 100%
rename from drivers/ssbi/ssbi.c
rename to drivers/mfd/ssbi.c
diff --git a/drivers/ssbi/Kconfig b/drivers/ssbi/Kconfig
deleted file mode 100644
index 1ae4040a..0000000
--- a/drivers/ssbi/Kconfig
+++ /dev/null
@@ -1,16 +0,0 @@
-#
-# SSBI bus support
-#
-
-menu "Qualcomm MSM SSBI bus support"
-
-config SSBI
-	tristate "Qualcomm Single-wire Serial Bus Interface (SSBI)"
-	help
-	  If you say yes to this option, support will be included for the
-	  built-in SSBI interface on Qualcomm MSM family processors.
-
-	  This is required for communicating with Qualcomm PMICs and
-	  other devices that have the SSBI interface.
-
-endmenu
diff --git a/drivers/ssbi/Makefile b/drivers/ssbi/Makefile
deleted file mode 100644
index 38fb70c..0000000
--- a/drivers/ssbi/Makefile
+++ /dev/null
@@ -1 +0,0 @@
-obj-$(CONFIG_SSBI) += ssbi.o

Re: MFD: move ssbi driver into drivers/mfd

[Greg KH]

On Tue, Apr 30, 2013 at 12:00:19AM +0200, Arnd Bergmann wrote:
> There is no reason for ssbi to have its own top-level driver directory
> when the only users of this interface are all MFD drivers. The only
> mainline driver using it at the moment (PM8921) is marked broken and in
> fact does not compile. I have verified that fixing the trivial build
> breakage in pm8921 links in the new ssbi code just fine, but that
> can be a separate patch.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Cc: Samuel Ortiz <sameo@linux.intel.com>

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: MFD: move ssbi driver into drivers/mfd

[Nicolas Pitre]

On Tue, 30 Apr 2013, Arnd Bergmann wrote:

> There is no reason for ssbi to have its own top-level driver directory
> when the only users of this interface are all MFD drivers. The only
> mainline driver using it at the moment (PM8921) is marked broken and in
> fact does not compile. I have verified that fixing the trivial build
> breakage in pm8921 links in the new ssbi code just fine, but that
> can be a separate patch.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Cc: Samuel Ortiz <sameo@linux.intel.com>

Acked-by: Nicolas Pitre <nico@linaro.org>


> ---
>  drivers/Kconfig              |  2 --
>  drivers/Makefile             |  1 -
>  drivers/mfd/Kconfig          |  3 ++-
>  drivers/mfd/Makefile         |  2 +-
>  drivers/{ssbi => mfd}/ssbi.c |  0
>  drivers/ssbi/Kconfig         | 16 ----------------
>  drivers/ssbi/Makefile        |  1 -
>  7 files changed, 3 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/Kconfig b/drivers/Kconfig
> index 78a956e..202fa6d 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -52,8 +52,6 @@ source "drivers/i2c/Kconfig"
>  
>  source "drivers/spi/Kconfig"
>  
> -source "drivers/ssbi/Kconfig"
> -
>  source "drivers/hsi/Kconfig"
>  
>  source "drivers/pps/Kconfig"
> diff --git a/drivers/Makefile b/drivers/Makefile
> index 33360de..3c200a2 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -114,7 +114,6 @@ obj-y				+= firmware/
>  obj-$(CONFIG_CRYPTO)		+= crypto/
>  obj-$(CONFIG_SUPERH)		+= sh/
>  obj-$(CONFIG_ARCH_SHMOBILE)	+= sh/
> -obj-$(CONFIG_SSBI)		+= ssbi/
>  ifndef CONFIG_ARCH_USES_GETTIMEOFFSET
>  obj-y				+= clocksource/
>  endif
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index ca86581..5150833 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -991,7 +991,8 @@ config MFD_PM8XXX
>  
>  config MFD_PM8921_CORE
>  	tristate "Qualcomm PM8921 PMIC chip"
> -	depends on SSBI && BROKEN
> +	depends on (ARCH_MSM || HEXAGON)
> +	depends on BROKEN
>  	select MFD_CORE
>  	select MFD_PM8XXX
>  	help
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index b90409c..3b95b47 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -133,7 +133,7 @@ obj-$(CONFIG_MFD_VX855)		+= vx855.o
>  obj-$(CONFIG_MFD_WL1273_CORE)	+= wl1273-core.o
>  obj-$(CONFIG_MFD_CS5535)	+= cs5535-mfd.o
>  obj-$(CONFIG_MFD_OMAP_USB_HOST)	+= omap-usb-host.o omap-usb-tll.o
> -obj-$(CONFIG_MFD_PM8921_CORE) 	+= pm8921-core.o
> +obj-$(CONFIG_MFD_PM8921_CORE) 	+= pm8921-core.o ssbi.o
>  obj-$(CONFIG_MFD_PM8XXX_IRQ) 	+= pm8xxx-irq.o
>  obj-$(CONFIG_TPS65911_COMPARATOR)	+= tps65911-comparator.o
>  obj-$(CONFIG_MFD_TPS65090)	+= tps65090.o
> diff --git a/drivers/ssbi/ssbi.c b/drivers/mfd/ssbi.c
> similarity index 100%
> rename from drivers/ssbi/ssbi.c
> rename to drivers/mfd/ssbi.c
> diff --git a/drivers/ssbi/Kconfig b/drivers/ssbi/Kconfig
> deleted file mode 100644
> index 1ae4040a..0000000
> --- a/drivers/ssbi/Kconfig
> +++ /dev/null
> @@ -1,16 +0,0 @@
> -#
> -# SSBI bus support
> -#
> -
> -menu "Qualcomm MSM SSBI bus support"
> -
> -config SSBI
> -	tristate "Qualcomm Single-wire Serial Bus Interface (SSBI)"
> -	help
> -	  If you say yes to this option, support will be included for the
> -	  built-in SSBI interface on Qualcomm MSM family processors.
> -
> -	  This is required for communicating with Qualcomm PMICs and
> -	  other devices that have the SSBI interface.
> -
> -endmenu
> diff --git a/drivers/ssbi/Makefile b/drivers/ssbi/Makefile
> deleted file mode 100644
> index 38fb70c..0000000
> --- a/drivers/ssbi/Makefile
> +++ /dev/null
> @@ -1 +0,0 @@
> -obj-$(CONFIG_SSBI) += ssbi.o
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

Re: MFD: move ssbi driver into drivers/mfd

[David Brown]

Arnd Bergmann <arnd@arndb.de> writes:

> There is no reason for ssbi to have its own top-level driver directory
> when the only users of this interface are all MFD drivers. The only
> mainline driver using it at the moment (PM8921) is marked broken and in
> fact does not compile. I have verified that fixing the trivial build
> breakage in pm8921 links in the new ssbi code just fine, but that
> can be a separate patch.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Cc: Samuel Ortiz <sameo@linux.intel.com>

Acked-by: David Brown <davidb@codeaurora.org>

Coming soon, I hope, are going to be the DT conversion of the 8921
driver, and possibly drivers for a few of the other pmic chips.

And, thanks for moving this.

David

-- 
sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

Re: MFD: move ssbi driver into drivers/mfd

[Samuel Ortiz]

Hi Arnd,

On Tue, Apr 30, 2013 at 12:00:19AM +0200, Arnd Bergmann wrote:
> There is no reason for ssbi to have its own top-level driver directory
> when the only users of this interface are all MFD drivers. The only
> mainline driver using it at the moment (PM8921) is marked broken and in
> fact does not compile. I have verified that fixing the trivial build
> breakage in pm8921 links in the new ssbi code just fine, but that
> can be a separate patch.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Cc: Samuel Ortiz <sameo@linux.intel.com>
> ---
>  drivers/Kconfig              |  2 --
>  drivers/Makefile             |  1 -
>  drivers/mfd/Kconfig          |  3 ++-
>  drivers/mfd/Makefile         |  2 +-
>  drivers/{ssbi => mfd}/ssbi.c |  0
>  drivers/ssbi/Kconfig         | 16 ----------------
>  drivers/ssbi/Makefile        |  1 -
>  7 files changed, 3 insertions(+), 22 deletions(-)
I suppose we don't expect any non MFD drivers to use this interface ?  If
that's so, I'll take it through mfd-next.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

Re: MFD: move ssbi driver into drivers/mfd

[Arnd Bergmann]

On Tuesday 30 April 2013, Samuel Ortiz wrote:
> I suppose we don't expect any non MFD drivers to use this interface ?

Yes, that is correct.

> If that's so, I'll take it through mfd-next.

Ok, thanks!

	Arnd

Re: MFD: move ssbi driver into drivers/mfd

[Samuel Ortiz]

Hi Arnd,

On Tue, Apr 30, 2013 at 12:00:19AM +0200, Arnd Bergmann wrote:
> There is no reason for ssbi to have its own top-level driver directory
> when the only users of this interface are all MFD drivers. The only
> mainline driver using it at the moment (PM8921) is marked broken and in
> fact does not compile. I have verified that fixing the trivial build
> breakage in pm8921 links in the new ssbi code just fine, but that
> can be a separate patch.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Cc: Samuel Ortiz <sameo@linux.intel.com>
> ---
>  drivers/Kconfig              |  2 --
>  drivers/Makefile             |  1 -
>  drivers/mfd/Kconfig          |  3 ++-
>  drivers/mfd/Makefile         |  2 +-
>  drivers/{ssbi => mfd}/ssbi.c |  0
>  drivers/ssbi/Kconfig         | 16 ----------------
>  drivers/ssbi/Makefile        |  1 -
>  7 files changed, 3 insertions(+), 22 deletions(-)
Applied to my mfd-next tree, thanks.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/