All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v15 00/13] mux controller abstraction and iio/i2c muxes
@ 2017-05-14 19:51 Peter Rosin
  2017-05-14 19:51 ` [PATCH v15 01/13] devres: trivial whitespace fix Peter Rosin
                   ` (13 more replies)
  0 siblings, 14 replies; 26+ messages in thread
From: Peter Rosin @ 2017-05-14 19:51 UTC (permalink / raw)
  To: linux-kernel, Greg Kroah-Hartman
  Cc: Peter Rosin, Wolfram Sang, Rob Herring, Mark Rutland,
	Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Jonathan Corbet, linux-i2c, devicetree,
	linux-iio, linux-doc, Andrew Morton, Colin Ian King,
	Paul Gortmaker, Philipp Zabel, kernel

From: Peter Rosin <peda@axentia.se>

Hi Greg,

Philipp found problems in v14 with using a mutex for locking that was
the outcome of the review for v13, so I'm now using a semaphore instead
of the rwsem that was in v13. That at least got rid of the scary call
to downgrade_write. However, I'm still unsure about what you actually
meant with your comment about lack of sparse markings [1]. I did add
__must_check to the funcs that selects the mux, but I've got this
feeling that this is not what you meant?

Anyway, I have acted on all your comments from the v13 review, at
least I think I did. So, please apply.

[1] https://lkml.org/lkml/2017/4/21/794


v14 -> v15 changes
- Rebased onto v4.12-rc1
- Add the mmio-mux driver from Philipp Zabel to the end of the series.
- Using a mutex to lock the mux state did not work out for Philipp Zabel
  and his video-mux, which is locking the mux over long periods of time.
  So, switch to a semaphore (where different tasks can call down/up).
- Remove unused devm_mux_chip_free, devm_mux_chip_unregister and
  devm_mux_control_put. Those are currently unused and can be added when
  someone needs them.
- Add more words in the kernel-doc comments for mux_control_select,
  mux_control_try_select and mux_control_deselect on the calling rules.
- Add "|| COMPILE_TEST" to the Kconfig depends for all new drivers.

[older changes follow below]


This adds a new mux controller subsystem with an interface for accessing
mux controllers, along with three drivers providing the interface (gpio,
mmio and adg792) and two consumers (iio and i2c). This is done in a way
that several consumers can independently access the same mux controller
if one controller controls several multiplexers, thus allowing sharing.
But sharing is by no means required, of course. It is perfectly fine to
have a single consumer of a dedicated mux controller controlling only
one mux for said consumer.

The prediction is that the typical use case will be for gpio-based muxing
(which is also what drove the development), where the below schematics
show the flexibility with one gpio-based mux controller being shared by
the iio-mux and i2c-mux-gpmux drivers.

    .----.
    |GPO0|----------.
    |GPO1|--------. |
    |    |        | |
    |    |     .-------.
    |    |     |dg4052a|
    |    |     |       |
    |ADC0|-----|X    X0|---- signal X0
    |    |     |     X1|---- signal X1
    |    |     |     X2|---- signal X2
    |    |     |     X3|---- signal X3
    |    |     |       |
    |SDA0|-----|Y    Y0|---- i2c segment Y0
    |SCL0|--.  |     Y1|---- i2c segment Y1
    '----'  |  |     Y2|---- i2c segment Y2
            |  |     Y3|---- i2c segment Y3
            |  '-------'
            |                0 1 2 3   (feed SCL0 to each of
            |                | | | |    the 4 muxed segments)
            '----------------+-+-+-'

GPO0 and GPO1 may also be fed to further parallel muxers, which is perhaps
desired in a real application to minimize digital noise from the I2C Y
channel leaking into the analog X channel. I.e. it might be a good idea
to separate the analog and digital signals...

And the below hypothetical schematics indicate something similar but using
the I2C-based adg792a multiplexer instead.

    .----.
    |SDA0|----------.
    |SCL0|--------. |
    |    |        | |
    |    |     .-------.
    |    |     |adg792a|
    |    |     |       |
    |ADC0|-----|D1  S1A|---- signal S1A
    |    |     |    S1B|---- signal S1B
    |    |     |    S1C|---- signal S1C
    |    |     |    S1D|---- signal S1D
    |    |     |       |
    |SDA1|---+-|D2  S2A|---- i2c segment S2A
    |SCL1|-. | |    S2B|---- i2c segment S2B
    '----' | | |    S2C|---- i2c segment S2C
           | | |    S2D|---- i2c segment S2D
           | | |       |
           | '-|D3  S3A|---- i2c segment S3A
           |   |    S3B|---- i2c segment S3B
           |   |    S3C|---- i2c segment S3C
           |   |    S3D|---- i2c segment S3D
           |   '-------'
           |                 A B C D   A B C D  (feed SCL1 to each of
           |                 | | | |   | | | |   the 8 muxed segments)
           '-----------------+-+-+-+---+-+-+-'


Background:

I have a piece of hardware that is using the same 3 GPIO pins
to control four 8-way muxes. Three of them control ADC lines
to an ADS1015 chip with an iio driver, and the last one
controls the SDA line of an i2c bus. We have some deployed
code to handle this, but you do not want to see it or ever
hear about it. I'm not sure why I even mention it. Anyway,
the situation has nagged me to no end for quite some time.

So, after first getting more intimate with the i2c muxing code
and later discovering the drivers/iio/inkern.c file and
writing a couple of drivers making use of it, I came up with
what I think is an acceptable solution; add a generic mux
controller driver (and subsystem) that is shared between all
instances, and combine that with an iio mux driver and a new
general purpose i2c mux driver that is only hooking the i2c
muxing and the new mux controller.

One thing that I would like to do, but don't see a solution
for, is to move the mux control code that is present in
various drivers in drivers/i2c/muxes to this new minimalistic
muxing subsystem, thus converting all present i2c muxes (but
perhaps not gates and arbitrators).

I'm using an ordinary semaphore to lock the mux, but that isn't
really a perfect fit since multiple consumers of the same mux
controller need not lock each other out when they want the same
mux state. I had a rwsem for a long while, but that degenerated
into the above semaphore case when there was contention.
Originally I had a mutex (before v1) and that was even back for
a brief time (in v14), but a mutex is not a good fit for some
consumers needing to lock the mux for a long time.

Also, the "mux" name feels a bit ambitious, there are many muxes
in the world, and this tiny bit of code is probably not good
enough to be a nice fit for all...


Older changes:

v13 -> v14 changes:  https://lkml.org/lkml/2017/4/24/153
- Go back to a mutex instead of a rwsem to protect the mux state. [Greg K-H]
- Add __must_check to the mux_control_select function. [Greg, was this what
  you meant with "sparse markings"? I assume not, but I don't know...]
- Don't return hint on mux state changes when calling mux_control_select. [Greg]
- Add mux_control_try_select function. [Philipp Zabel]
- Split the mux.h #include into mux/driver.h and mux/consumer.h [Philipp]
- Add mux_control_states function to be able to completely hide struct
  mux_control details from mux consumers.
- Include the constants from the DT bindings include, instead of
  redefining them. [Greg]
- Make mux_chip_alloc (and devm_mux_chip_alloc) return ERR_PTR instead
  of NULL on error. [Greg]
- Document why mux_chip_alloc and mux_chip_register are split up, and
  what is supposed to happen between the two calls, [Greg] and...
- ...specifically mention that mux drivers should never clobber
  cached_state [Philipp].
- Support module unloading of the core (and initialize mux_ida on init). [Greg]
- Do not mention link order when commenting on why there is a subsys_initcall
  instead of an ordinare module_init in the mux-core. [Greg]
- Use __func__ instead of spelling out the function name. [Greg]
- Drop cargo cult usage of WARN_ON. [Greg]
- Drop unused forward declaration of struct platform_device.
- Move kernel doc from the function declarations to the definitions. [Greg]
- Use unsigned values for the state in the consumer interface. [Philipp]
- WARN if consumers request an invalid state. [Philipp]
- Split out mux-gpio from the core patch (03/10 -> 03/11 and 04/11). [Greg]
- Drop OF Kconfig dep from mux-gpio. [Greg]
- Fail the adg792a probe if the I2C adapter does not support
  I2C_FUNC_SMBUS_BYTE_DATA.
- For clarity, handle the adg792a active low reset bit in a helper function.
- Make use of the preexisting mux variable in the adg792a driver instead of
  chasing the same pointers a second time.
- Use device_property_read_* instead of of_property_read_* in adg792a.
- Added a "thank you" section to the patch adding the mux core (patch 03/11).

v12 -> v13 changes:  https://lkml.org/lkml/2017/4/13/493
- Philipp Zabel noticed a bad compatible string in the gpio muxer.
  I amended patch 3/10 with his patch.
- Fixed reversed sense of the reset bit in the adg792 muxer (patch 10/10).

v11 -> v12 changes:  https://lkml.org/lkml/2017/3/27/464
- patches 11 and 12 folded into patches 6 and 3 respectively.

v10 -> v11 changes:  https://lkml.org/lkml/2017/3/27/336
- added a fixes-tag and an ack from Jonathan on patch 11
- added a new patch (12) with a fix for messed up error path reported
  by Paul Gortmaker.
- fixed some editorial nitpicks in the documentation comments in patch 3.

v9 -> v10 changes:  https://lkml.org/lkml/2017/3/10/411
- rebased onto v4.11-rc1
- added reviewed-by tags from Rob on patches 7 and 9
- added a new patch (11) with a fix for an unsigned compare with less than
  zero detected by CoverityScan and reported by Colin Ian King
- allowed the mux core to be built as a module, after discussion with Paul
  Gortmaker
- added explicit includes of linux/export.h and linux/init.h from the mux
  core, also noted by Paul
- fixed trivial whitespace issue in drivers/mux/Makefile
- added trailing '>' after my mail address in MODULE_AUTHOR, which was missing
  in all new modules in drivers/mux

v8 -> v9 changes:  https://lkml.org/lkml/2017/2/8/394
- dropped the suffix from the compatible string of the i2c-mux-simple
  binding (was ,mux-locked or ,parent-locked) and add an optional
  mux-locked property instead to change the desired locking behavior
  from the default parent-locked
- add description of the difference between mux-locked and parent-locked
- renamed i2c-mux-simple to i2c-mux (bindings for this general purpose
  i2c mux are in i2c-mux-gpmux.txt since i2c-mux.txt is already occupied
  by the common i2c-mux bindings)
- changed compatible from mux-gpio to gpio-mux
- changed bindings for idle-state back to a single array, but add defines
  for as-is and hi-Z thus avoiding magic numbers
- make use of the above defines in the code as well
- make idle-state a common mux property described in mux-controller.txt
  instead of repeating the info in individual mux controller bindings
- drop the adi,parallel property from the adg792 bindings and piggy-back
  on the #mux-control-cells property
- refrain from using the compatible string as node name
- dropped the simplified bindings for single-user gpio mux
- added acks on patches 2/10 and 5/10 from Rob

v7 -> v8 changes:  https://lkml.org/lkml/2017/1/18/745
- cleanup the implementation of the simplified gpio bindings, but still...
- ...reorder patches so that they appear last in the series (patches 11
  and 12 were patches 4 and 5 in v7) since Jonathan convinced me that
  they were perhaps not such a good idea after all. But I still wanted
  to show the last version I had and I'm still a bit undecided...
- added some words to the remaining otherwise empty commit messages
- added various acks/reviews from Jonathan and Wolfram.
- move mux last in drivers/Kconfig and drivers/Makefile
- bump copyright years
- centralize error reporting of common operatinons to the mux core
- add WARN_ON for faulty usage of mux_chip_register
- simplify code for other WARN_ON call sites

v6 -> v7 changes:  https://lkml.org/lkml/2017/1/4/315
- move from drivers/misc/mux-* to drivers/mux/
  [I will remove Cc:s to Arnd and Greg for v8, when/if that happens]
- add managed versions of mux_chip_alloc and mux_chip_register
- use the above in mux-gpio.c and mux-adg792a.c
- also use them to support a simplified binding of a gpio mux for the common
  case where there is a single consumer (and no specific idle requirements)
- new binding for describing idle behaviour of mux-adg792a
- add bindings for the gpo-pins on the mux-adg792g
- use device_property_read_u32 instead of of_property_read_u32 in mux-gpio.c
- rename iio mux compatible to io-channel-mux (was iio-mux)
- remove linuxism in the bindings (it was mentioning a function name)
- add missing quote in the example in the io-channel-mux binding
- factor out preparatory cleanup of devres docs to its own patch
- add blank line in mux_chip_free
- use SIMPLE_{PARENT,MUX}_LOCKED instead of magic numbers {0,1} in
  i2c-mux-simple.c
- add some acks and a reviewed-by from Jonathan

v5 -> v6 changes:  https://lkml.org/lkml/2016/11/30/466
- fix stupidity in mux_chip_priv, mux_gpio_remove and adg792a_remove.
- change the devicetree bindings for the iio-mux to use a list of strings
  (channels property) instead of a list children.

v4 -> v5 changes:  https://lkml.org/lkml/2016/11/29/224
- remove support for fancier dt layouts and go back to the phandle
  approach from v2 and before, killing the horrible non-working
  refcounting crap from v4 and avoiding a bunch of life-time issues
  in v3.
- introduce the concept of a mux-chip, that can hold one or more
  mux-controllers (inspired by the pwm subsystem).
- add dt #mux-control-cells property needed to get to the desired
  mux controller if a mux chip provides more than one.
- take away the option to build the mux-core as a module.
- if the mux controller has an idle state, make sure the mux controller
  is set up in the idle state initially (when it should be idle).
- do not use a variable length array on the stack in mux_gpio_set to
  temporarily store the gpio state, preallocate space instead.
- fix resource leak on one failure path in mux_gpio_probe.
- driver for Analog Devices ADG792A/G, literally the first mux chip
  I found on the Internet with an i2c interface (that was not a
  dedicated i2c multiplexer like PCA9547) which I used to verify
  that the abstractions in the mux core are up to the task. Untested,
  just proof of concept that at least looks pretty and compiles...
- various touch-ups.

v3 -> v4 changes:  https://lkml.org/lkml/2016/11/24/664
- rebased onto next-20161122 (depends on recent _available iio changes).
- added support for having the mux-controller in a child node of a
  mux-consumer if it is a sole consumer, to hopefully even further satisfy
  the complaint from Rob (and later Lars-Peter) about dt complexity.
- the above came at the cost of some rather horrible refcounting code,
  please review and suggest how it should be done...
- changed to register a device class instead of a bus.
- pass in the parent device into mux_control_alloc and require less
  work from mux-control drivers.
- changed device names from mux:control%d to mux%d
- move kernel-doc from mux-core.c to mux.h (and add some bits).
- give the gpio driver a chance to update all mux pins at once.
- factor out iio ext_info lookup into new helper function. /Lars-Peter
- use an unsigned type for the iio ext_info count. /Lars-Peter
- unified "brag strings" in the file headers.

v2 -> v3 changes:  https://lkml.org/lkml/2016/11/21/332
- have the mux-controller in the parent node of any mux-controller consumer,
  to hopefully satisfy complaint from Rob about dt complexity.
- improve commit message of the mux subsystem commit, making it more
  general, as requested by Jonathan.
- remove priv member from struct mux_control and calculate it on the
  fly. /Jonathan
- make the function comments in mux-core.c kernel doc. /Jonathan
- add devm_mux_control_* to Documentation/driver.model/devres.txt. /Jonathan
- add common dt bindings for mux-controllers, refer to them from the
  mux-gpio bindings. /Rob
- clarify how the gpio pins map to the mux state. /Rob
- separate CONFIG_ variables for the mux core and the mux gpio driver.
- improve Kconfig help texts.
- make CONFIG_MUX_GPIO depend on CONFIG_GPIOLIB.
- keep track of the number of mux states in the mux core.
- since the iio channel number is used as mux state, it was possible
  to drop the state member from the mux_child struct.
- cleanup dt bindings for i2c-mux-simple, it had some of copy-paste
  problems from ots origin (i2c-mux-gpio).
- select the mux control subsystem in config for the i2c-mux-simple driver.
- add entries to MAINTAINERS and my sign-off, I'm now satisfied and know
  nothing in this to be ashamed of.

v1 -> v2 changes:  https://lkml.org/lkml/2016/11/17/918
- fixup export of mux_control_put reported by kbuild
- drop devicetree iio-ext-info property as noted by Lars-Peter,
  and replace the functionality by exposing all ext_info
  attributes of the parent channel for each of the muxed
  channels. A cache on top of that and each muxed channel
  gets its own view of the ext_info of the parent channel.
- implement idle-state for muxes
- clear out the cache on failure in order to force a mux
  update on the following use
- cleanup the probe of i2c-mux-simple driver
- fix a bug in the i2c-mux-simple driver, where failure in
  the selection of the mux caused a deadlock when the mux
  was later unconditionally deselected.

v1:  https://lkml.org/lkml/2016/11/16/812

Cheers,
peda

Peter Rosin (11):
  devres: trivial whitespace fix
  dt-bindings: document devicetree bindings for mux-controllers and
    gpio-mux
  mux: minimal mux subsystem
  mux: gpio: add mux controller driver for gpio based multiplexers
  iio: inkern: api for manipulating ext_info of iio channels
  dt-bindings: iio: io-channel-mux: document io-channel-mux bindings
  iio: multiplexer: new iio category and iio-mux driver
  dt-bindings: i2c: i2c-mux: document general purpose i2c-mux bindings
  i2c: i2c-mux-gpmux: new driver
  dt-bindings: mux-adg792a: document devicetree bindings for ADG792A/G
    mux
  mux: adg792a: add mux controller driver for ADG792A/G

Philipp Zabel (2):
  dt-bindings: add mmio-based syscon mux controller DT bindings
  mux: mmio-based syscon mux controller

 Documentation/ABI/testing/sysfs-class-mux          |  16 +
 .../devicetree/bindings/i2c/i2c-mux-gpmux.txt      |  99 ++++
 .../bindings/iio/multiplexer/io-channel-mux.txt    |  39 ++
 .../devicetree/bindings/mux/adi,adg792a.txt        |  75 +++
 Documentation/devicetree/bindings/mux/gpio-mux.txt |  69 +++
 Documentation/devicetree/bindings/mux/mmio-mux.txt |  60 +++
 .../devicetree/bindings/mux/mux-controller.txt     | 157 ++++++
 Documentation/driver-model/devres.txt              |   7 +-
 MAINTAINERS                                        |  16 +
 drivers/Kconfig                                    |   2 +
 drivers/Makefile                                   |   1 +
 drivers/i2c/muxes/Kconfig                          |  13 +
 drivers/i2c/muxes/Makefile                         |   1 +
 drivers/i2c/muxes/i2c-mux-gpmux.c                  | 173 +++++++
 drivers/iio/Kconfig                                |   1 +
 drivers/iio/Makefile                               |   1 +
 drivers/iio/inkern.c                               |  60 +++
 drivers/iio/multiplexer/Kconfig                    |  18 +
 drivers/iio/multiplexer/Makefile                   |   6 +
 drivers/iio/multiplexer/iio-mux.c                  | 459 +++++++++++++++++
 drivers/mux/Kconfig                                |  59 +++
 drivers/mux/Makefile                               |   8 +
 drivers/mux/mux-adg792a.c                          | 157 ++++++
 drivers/mux/mux-core.c                             | 547 +++++++++++++++++++++
 drivers/mux/mux-gpio.c                             | 114 +++++
 drivers/mux/mux-mmio.c                             | 141 ++++++
 include/dt-bindings/mux/mux.h                      |  16 +
 include/linux/iio/consumer.h                       |  37 ++
 include/linux/mux/consumer.h                       |  32 ++
 include/linux/mux/driver.h                         | 108 ++++
 30 files changed, 2491 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/testing/sysfs-class-mux
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-mux-gpmux.txt
 create mode 100644 Documentation/devicetree/bindings/iio/multiplexer/io-channel-mux.txt
 create mode 100644 Documentation/devicetree/bindings/mux/adi,adg792a.txt
 create mode 100644 Documentation/devicetree/bindings/mux/gpio-mux.txt
 create mode 100644 Documentation/devicetree/bindings/mux/mmio-mux.txt
 create mode 100644 Documentation/devicetree/bindings/mux/mux-controller.txt
 create mode 100644 drivers/i2c/muxes/i2c-mux-gpmux.c
 create mode 100644 drivers/iio/multiplexer/Kconfig
 create mode 100644 drivers/iio/multiplexer/Makefile
 create mode 100644 drivers/iio/multiplexer/iio-mux.c
 create mode 100644 drivers/mux/Kconfig
 create mode 100644 drivers/mux/Makefile
 create mode 100644 drivers/mux/mux-adg792a.c
 create mode 100644 drivers/mux/mux-core.c
 create mode 100644 drivers/mux/mux-gpio.c
 create mode 100644 drivers/mux/mux-mmio.c
 create mode 100644 include/dt-bindings/mux/mux.h
 create mode 100644 include/linux/mux/consumer.h
 create mode 100644 include/linux/mux/driver.h

-- 
2.1.4

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

* [PATCH v15 01/13] devres: trivial whitespace fix
  2017-05-14 19:51 [PATCH v15 00/13] mux controller abstraction and iio/i2c muxes Peter Rosin
@ 2017-05-14 19:51 ` Peter Rosin
  2017-05-14 19:51 ` [PATCH v15 02/13] dt-bindings: document devicetree bindings for mux-controllers and gpio-mux Peter Rosin
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 26+ messages in thread
From: Peter Rosin @ 2017-05-14 19:51 UTC (permalink / raw)
  To: linux-kernel, Greg Kroah-Hartman
  Cc: Peter Rosin, Wolfram Sang, Rob Herring, Mark Rutland,
	Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Jonathan Corbet, linux-i2c, devicetree,
	linux-iio, linux-doc, Andrew Morton, Colin Ian King,
	Paul Gortmaker, Philipp Zabel, kernel

From: Peter Rosin <peda@axentia.se>

Everything else is indented with two spaces, so fix the odd one out.

Acked-by: Jonathan Cameron <jic23@kernel.org>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
 Documentation/driver-model/devres.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/driver-model/devres.txt b/Documentation/driver-model/devres.txt
index e72587fe477d..af08b4cd7968 100644
--- a/Documentation/driver-model/devres.txt
+++ b/Documentation/driver-model/devres.txt
@@ -335,7 +335,7 @@ MEM
   devm_kzalloc()
 
 MFD
- devm_mfd_add_devices()
+  devm_mfd_add_devices()
 
 PER-CPU MEM
   devm_alloc_percpu()
-- 
2.1.4

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

* [PATCH v15 02/13] dt-bindings: document devicetree bindings for mux-controllers and gpio-mux
  2017-05-14 19:51 [PATCH v15 00/13] mux controller abstraction and iio/i2c muxes Peter Rosin
  2017-05-14 19:51 ` [PATCH v15 01/13] devres: trivial whitespace fix Peter Rosin
@ 2017-05-14 19:51 ` Peter Rosin
  2017-05-14 19:51 ` [PATCH v15 03/13] mux: minimal mux subsystem Peter Rosin
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 26+ messages in thread
From: Peter Rosin @ 2017-05-14 19:51 UTC (permalink / raw)
  To: linux-kernel, Greg Kroah-Hartman
  Cc: Peter Rosin, Wolfram Sang, Rob Herring, Mark Rutland,
	Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Jonathan Corbet, linux-i2c, devicetree,
	linux-iio, linux-doc, Andrew Morton, Colin Ian King,
	Paul Gortmaker, Philipp Zabel, kernel

From: Peter Rosin <peda@axentia.se>

Allow specifying that a single multiplexer controller can be used to
control several parallel multiplexers, thus enabling sharing of the
multiplexer controller by different consumers.

Add a binding for a first mux controller in the form of a GPIO based mux
controller.

Acked-by: Jonathan Cameron <jic23@kernel.org>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
 Documentation/devicetree/bindings/mux/gpio-mux.txt |  69 +++++++++
 .../devicetree/bindings/mux/mux-controller.txt     | 157 +++++++++++++++++++++
 MAINTAINERS                                        |   6 +
 include/dt-bindings/mux/mux.h                      |  16 +++
 4 files changed, 248 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mux/gpio-mux.txt
 create mode 100644 Documentation/devicetree/bindings/mux/mux-controller.txt
 create mode 100644 include/dt-bindings/mux/mux.h

diff --git a/Documentation/devicetree/bindings/mux/gpio-mux.txt b/Documentation/devicetree/bindings/mux/gpio-mux.txt
new file mode 100644
index 000000000000..b8f746344d80
--- /dev/null
+++ b/Documentation/devicetree/bindings/mux/gpio-mux.txt
@@ -0,0 +1,69 @@
+GPIO-based multiplexer controller bindings
+
+Define what GPIO pins are used to control a multiplexer. Or several
+multiplexers, if the same pins control more than one multiplexer.
+
+Required properties:
+- compatible : "gpio-mux"
+- mux-gpios : list of gpios used to control the multiplexer, least
+	      significant bit first.
+- #mux-control-cells : <0>
+* Standard mux-controller bindings as decribed in mux-controller.txt
+
+Optional properties:
+- idle-state : if present, the state the mux will have when idle. The
+	       special state MUX_IDLE_AS_IS is the default.
+
+The multiplexer state is defined as the number represented by the
+multiplexer GPIO pins, where the first pin is the least significant
+bit. An active pin is a binary 1, an inactive pin is a binary 0.
+
+Example:
+
+	mux: mux-controller {
+		compatible = "gpio-mux";
+		#mux-control-cells = <0>;
+
+		mux-gpios = <&pioA 0 GPIO_ACTIVE_HIGH>,
+			    <&pioA 1 GPIO_ACTIVE_HIGH>;
+	};
+
+	adc-mux {
+		compatible = "io-channel-mux";
+		io-channels = <&adc 0>;
+		io-channel-names = "parent";
+
+		mux-controls = <&mux>;
+
+		channels = "sync-1", "in", "out", "sync-2";
+	};
+
+	i2c-mux {
+		compatible = "i2c-mux";
+		i2c-parent = <&i2c1>;
+
+		mux-controls = <&mux>;
+
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		i2c@0 {
+			reg = <0>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			ssd1307: oled@3c {
+				/* ... */
+			};
+		};
+
+		i2c@3 {
+			reg = <3>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			pca9555: pca9555@20 {
+				/* ... */
+			};
+		};
+	};
diff --git a/Documentation/devicetree/bindings/mux/mux-controller.txt b/Documentation/devicetree/bindings/mux/mux-controller.txt
new file mode 100644
index 000000000000..4f47e4bd2fa0
--- /dev/null
+++ b/Documentation/devicetree/bindings/mux/mux-controller.txt
@@ -0,0 +1,157 @@
+Common multiplexer controller bindings
+======================================
+
+A multiplexer (or mux) controller will have one, or several, consumer devices
+that uses the mux controller. Thus, a mux controller can possibly control
+several parallel multiplexers. Presumably there will be at least one
+multiplexer needed by each consumer, but a single mux controller can of course
+control several multiplexers for a single consumer.
+
+A mux controller provides a number of states to its consumers, and the state
+space is a simple zero-based enumeration. I.e. 0-1 for a 2-way multiplexer,
+0-7 for an 8-way multiplexer, etc.
+
+
+Consumers
+---------
+
+Mux controller consumers should specify a list of mux controllers that they
+want to use with a property containing a 'mux-ctrl-list':
+
+	mux-ctrl-list ::= <single-mux-ctrl> [mux-ctrl-list]
+	single-mux-ctrl ::= <mux-ctrl-phandle> [mux-ctrl-specifier]
+	mux-ctrl-phandle : phandle to mux controller node
+	mux-ctrl-specifier : array of #mux-control-cells specifying the
+			     given mux controller (controller specific)
+
+Mux controller properties should be named "mux-controls". The exact meaning of
+each mux controller property must be documented in the device tree binding for
+each consumer. An optional property "mux-control-names" may contain a list of
+strings to label each of the mux controllers listed in the "mux-controls"
+property.
+
+Drivers for devices that use more than a single mux controller can use the
+"mux-control-names" property to map the name of the requested mux controller
+to an index into the list given by the "mux-controls" property.
+
+mux-ctrl-specifier typically encodes the chip-relative mux controller number.
+If the mux controller chip only provides a single mux controller, the
+mux-ctrl-specifier can typically be left out.
+
+Example:
+
+	/* One consumer of a 2-way mux controller (one GPIO-line) */
+	mux: mux-controller {
+		compatible = "gpio-mux";
+		#mux-control-cells = <0>;
+
+		mux-gpios = <&pioA 0 GPIO_ACTIVE_HIGH>;
+	};
+
+	adc-mux {
+		compatible = "io-channel-mux";
+		io-channels = <&adc 0>;
+		io-channel-names = "parent";
+
+		mux-controls = <&mux>;
+		mux-control-names = "adc";
+
+		channels = "sync", "in";
+	};
+
+Note that in the example above, specifying the "mux-control-names" is redundant
+because there is only one mux controller in the list. However, if the driver
+for the consumer node in fact asks for a named mux controller, that name is of
+course still required.
+
+	/*
+	 * Two consumers (one for an ADC line and one for an i2c bus) of
+	 * parallel 4-way multiplexers controlled by the same two GPIO-lines.
+	 */
+	mux: mux-controller {
+		compatible = "gpio-mux";
+		#mux-control-cells = <0>;
+
+		mux-gpios = <&pioA 0 GPIO_ACTIVE_HIGH>,
+			    <&pioA 1 GPIO_ACTIVE_HIGH>;
+	};
+
+	adc-mux {
+		compatible = "io-channel-mux";
+		io-channels = <&adc 0>;
+		io-channel-names = "parent";
+
+		mux-controls = <&mux>;
+
+		channels = "sync-1", "in", "out", "sync-2";
+	};
+
+	i2c-mux {
+		compatible = "i2c-mux";
+		i2c-parent = <&i2c1>;
+
+		mux-controls = <&mux>;
+
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		i2c@0 {
+			reg = <0>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			ssd1307: oled@3c {
+				/* ... */
+			};
+		};
+
+		i2c@3 {
+			reg = <3>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			pca9555: pca9555@20 {
+				/* ... */
+			};
+		};
+	};
+
+
+Mux controller nodes
+--------------------
+
+Mux controller nodes must specify the number of cells used for the
+specifier using the '#mux-control-cells' property.
+
+Optionally, mux controller nodes can also specify the state the mux should
+have when it is idle. The idle-state property is used for this. If the
+idle-state is not present, the mux controller is typically left as is when
+it is idle. For multiplexer chips that expose several mux controllers, the
+idle-state property is an array with one idle state for each mux controller.
+
+The special value (-1) may be used to indicate that the mux should be left
+as is when it is idle. This is the default, but can still be useful for
+mux controller chips with more than one mux controller, particularly when
+there is a need to "step past" a mux controller and set some other idle
+state for a mux controller with a higher index.
+
+Some mux controllers have the ability to disconnect the input/output of the
+multiplexer. Using this disconnected high-impedance state as the idle state
+is indicated with idle state (-2).
+
+These constants are available in
+
+      #include <dt-bindings/mux/mux.h>
+
+as MUX_IDLE_AS_IS (-1) and MUX_IDLE_DISCONNECT (-2).
+
+An example mux controller node look like this (the adg972a chip is a triple
+4-way multiplexer):
+
+	mux: mux-controller@50 {
+		compatible = "adi,adg792a";
+		reg = <0x50>;
+		#mux-control-cells = <1>;
+
+		idle-state = <MUX_IDLE_DISCONNECT MUX_IDLE_AS_IS 2>;
+	};
diff --git a/MAINTAINERS b/MAINTAINERS
index f7d568b8f133..e17dd0bf2c9f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8709,6 +8709,12 @@ S:	Orphan
 F:	drivers/mmc/host/mmc_spi.c
 F:	include/linux/spi/mmc_spi.h
 
+MULTIPLEXER SUBSYSTEM
+M:	Peter Rosin <peda@axentia.se>
+S:	Maintained
+F:	Documentation/devicetree/bindings/mux/
+F:	include/linux/dt-bindings/mux/
+
 MULTISOUND SOUND DRIVER
 M:	Andrew Veliath <andrewtv@usa.net>
 S:	Maintained
diff --git a/include/dt-bindings/mux/mux.h b/include/dt-bindings/mux/mux.h
new file mode 100644
index 000000000000..c8e855c4a609
--- /dev/null
+++ b/include/dt-bindings/mux/mux.h
@@ -0,0 +1,16 @@
+/*
+ * This header provides constants for most Multiplexer bindings.
+ *
+ * Most Multiplexer bindings specify an idle state. In most cases, the
+ * the multiplexer can be left as is when idle, and in some cases it can
+ * disconnect the input/output and leave the multiplexer in a high
+ * impedance state.
+ */
+
+#ifndef _DT_BINDINGS_MUX_MUX_H
+#define _DT_BINDINGS_MUX_MUX_H
+
+#define MUX_IDLE_AS_IS      (-1)
+#define MUX_IDLE_DISCONNECT (-2)
+
+#endif
-- 
2.1.4

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

* [PATCH v15 03/13] mux: minimal mux subsystem
  2017-05-14 19:51 [PATCH v15 00/13] mux controller abstraction and iio/i2c muxes Peter Rosin
  2017-05-14 19:51 ` [PATCH v15 01/13] devres: trivial whitespace fix Peter Rosin
  2017-05-14 19:51 ` [PATCH v15 02/13] dt-bindings: document devicetree bindings for mux-controllers and gpio-mux Peter Rosin
@ 2017-05-14 19:51 ` Peter Rosin
  2017-05-17  9:38   ` Philipp Zabel
  2017-05-14 19:51 ` [PATCH v15 04/13] mux: gpio: add mux controller driver for gpio based multiplexers Peter Rosin
                   ` (10 subsequent siblings)
  13 siblings, 1 reply; 26+ messages in thread
From: Peter Rosin @ 2017-05-14 19:51 UTC (permalink / raw)
  To: linux-kernel, Greg Kroah-Hartman
  Cc: Peter Rosin, Wolfram Sang, Rob Herring, Mark Rutland,
	Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Jonathan Corbet, linux-i2c, devicetree,
	linux-iio, linux-doc, Andrew Morton, Colin Ian King,
	Paul Gortmaker, Philipp Zabel, kernel

From: Peter Rosin <peda@axentia.se>

Add a new minimalistic subsystem that handles multiplexer controllers.
When multiplexers are used in various places in the kernel, and the
same multiplexer controller can be used for several independent things,
there should be one place to implement support for said multiplexer
controller.

A single multiplexer controller can also be used to control several
parallel multiplexers, that are in turn used by different subsystems
in the kernel, leading to a need to coordinate multiplexer accesses.
The multiplexer subsystem handles this coordination.

Thanks go out to Lars-Peter Clausen, Jonathan Cameron, Rob Herring,
Wolfram Sang, Paul Gortmaker, Dan Carpenter, Colin Ian King, Greg
Kroah-Hartman and last but certainly not least to Philipp Zabel for
helpful comments, reviews, patches and general encouragement!

Reviewed-by: Jonathan Cameron <jic23@kernel.org>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
 Documentation/ABI/testing/sysfs-class-mux |  16 +
 Documentation/driver-model/devres.txt     |   5 +
 MAINTAINERS                               |   3 +
 drivers/Kconfig                           |   2 +
 drivers/Makefile                          |   1 +
 drivers/mux/Kconfig                       |  16 +
 drivers/mux/Makefile                      |   5 +
 drivers/mux/mux-core.c                    | 547 ++++++++++++++++++++++++++++++
 include/linux/mux/consumer.h              |  32 ++
 include/linux/mux/driver.h                | 108 ++++++
 10 files changed, 735 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-class-mux
 create mode 100644 drivers/mux/Kconfig
 create mode 100644 drivers/mux/Makefile
 create mode 100644 drivers/mux/mux-core.c
 create mode 100644 include/linux/mux/consumer.h
 create mode 100644 include/linux/mux/driver.h

diff --git a/Documentation/ABI/testing/sysfs-class-mux b/Documentation/ABI/testing/sysfs-class-mux
new file mode 100644
index 000000000000..8715f9c7bd4f
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-mux
@@ -0,0 +1,16 @@
+What:		/sys/class/mux/
+Date:		April 2017
+KernelVersion:	4.13
+Contact:	Peter Rosin <peda@axentia.se>
+Description:
+		The mux/ class sub-directory belongs to the Generic MUX
+		Framework and provides a sysfs interface for using MUX
+		controllers.
+
+What:		/sys/class/mux/muxchipN/
+Date:		April 2017
+KernelVersion:	4.13
+Contact:	Peter Rosin <peda@axentia.se>
+Description:
+		A /sys/class/mux/muxchipN directory is created for each
+		probed MUX chip where N is a simple enumeration.
diff --git a/Documentation/driver-model/devres.txt b/Documentation/driver-model/devres.txt
index af08b4cd7968..40e4787c399c 100644
--- a/Documentation/driver-model/devres.txt
+++ b/Documentation/driver-model/devres.txt
@@ -337,6 +337,11 @@ MEM
 MFD
   devm_mfd_add_devices()
 
+MUX
+  devm_mux_chip_alloc()
+  devm_mux_chip_register()
+  devm_mux_control_get()
+
 PER-CPU MEM
   devm_alloc_percpu()
   devm_free_percpu()
diff --git a/MAINTAINERS b/MAINTAINERS
index e17dd0bf2c9f..a0cce325b08e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8712,8 +8712,11 @@ F:	include/linux/spi/mmc_spi.h
 MULTIPLEXER SUBSYSTEM
 M:	Peter Rosin <peda@axentia.se>
 S:	Maintained
+F:	Documentation/ABI/testing/mux/sysfs-class-mux*
 F:	Documentation/devicetree/bindings/mux/
 F:	include/linux/dt-bindings/mux/
+F:	include/linux/mux/
+F:	drivers/mux/
 
 MULTISOUND SOUND DRIVER
 M:	Andrew Veliath <andrewtv@usa.net>
diff --git a/drivers/Kconfig b/drivers/Kconfig
index ba2901e76769..505c676fa9c7 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -206,4 +206,6 @@ source "drivers/fsi/Kconfig"
 
 source "drivers/tee/Kconfig"
 
+source "drivers/mux/Kconfig"
+
 endmenu
diff --git a/drivers/Makefile b/drivers/Makefile
index cfabd141dba2..dfdcda00bfe3 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -181,3 +181,4 @@ obj-$(CONFIG_NVMEM)		+= nvmem/
 obj-$(CONFIG_FPGA)		+= fpga/
 obj-$(CONFIG_FSI)		+= fsi/
 obj-$(CONFIG_TEE)		+= tee/
+obj-$(CONFIG_MULTIPLEXER)	+= mux/
diff --git a/drivers/mux/Kconfig b/drivers/mux/Kconfig
new file mode 100644
index 000000000000..23ab2cde83b1
--- /dev/null
+++ b/drivers/mux/Kconfig
@@ -0,0 +1,16 @@
+#
+# Multiplexer devices
+#
+
+menuconfig MULTIPLEXER
+	tristate "Multiplexer subsystem"
+	help
+	  Multiplexer controller subsystem. Multiplexers are used in a
+	  variety of settings, and this subsystem abstracts their use
+	  so that the rest of the kernel sees a common interface. When
+	  multiple parallel multiplexers are controlled by one single
+	  multiplexer controller, this subsystem also coordinates the
+	  multiplexer accesses.
+
+	  To compile the subsystem as a module, choose M here: the module will
+	  be called mux-core.
diff --git a/drivers/mux/Makefile b/drivers/mux/Makefile
new file mode 100644
index 000000000000..09f0299e109d
--- /dev/null
+++ b/drivers/mux/Makefile
@@ -0,0 +1,5 @@
+#
+# Makefile for multiplexer devices.
+#
+
+obj-$(CONFIG_MULTIPLEXER)	+= mux-core.o
diff --git a/drivers/mux/mux-core.c b/drivers/mux/mux-core.c
new file mode 100644
index 000000000000..90b8995f07cb
--- /dev/null
+++ b/drivers/mux/mux-core.c
@@ -0,0 +1,547 @@
+/*
+ * Multiplexer subsystem
+ *
+ * Copyright (C) 2017 Axentia Technologies AB
+ *
+ * Author: Peter Rosin <peda@axentia.se>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#define pr_fmt(fmt) "mux-core: " fmt
+
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/export.h>
+#include <linux/idr.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/mux/consumer.h>
+#include <linux/mux/driver.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/slab.h>
+
+/*
+ * The idle-as-is "state" is not an actual state that may be selected, it
+ * only implies that the state should not be changed. So, use that state
+ * as indication that the cached state of the multiplexer is unknown.
+ */
+#define MUX_CACHE_UNKNOWN MUX_IDLE_AS_IS
+
+static struct class mux_class = {
+	.name = "mux",
+	.owner = THIS_MODULE,
+};
+
+static DEFINE_IDA(mux_ida);
+
+static int __init mux_init(void)
+{
+	ida_init(&mux_ida);
+	return class_register(&mux_class);
+}
+
+static void __exit mux_exit(void)
+{
+	class_register(&mux_class);
+	ida_destroy(&mux_ida);
+}
+
+static void mux_chip_release(struct device *dev)
+{
+	struct mux_chip *mux_chip = to_mux_chip(dev);
+
+	ida_simple_remove(&mux_ida, mux_chip->id);
+	kfree(mux_chip);
+}
+
+static struct device_type mux_type = {
+	.name = "mux-chip",
+	.release = mux_chip_release,
+};
+
+/**
+ * mux_chip_alloc() - Allocate a mux-chip.
+ * @dev: The parent device implementing the mux interface.
+ * @controllers: The number of mux controllers to allocate for this chip.
+ * @sizeof_priv: Size of extra memory area for private use by the caller.
+ *
+ * After allocating the mux-chip with the desired number of mux controllers
+ * but before registering the chip, the mux driver is required to configure
+ * the number of valid mux states in the mux_chip->mux[N].states members and
+ * the desired idle state in the returned mux_chip->mux[N].idle_state members.
+ * The default idle state is MUX_IDLE_AS_IS. The mux driver also needs to
+ * provide a pointer to the operations struct in the mux_chip->ops member
+ * before registering the mux-chip with mux_chip_register.
+ *
+ * Return: A pointer to the new mux-chip, or an ERR_PTR with a negative errno.
+ */
+struct mux_chip *mux_chip_alloc(struct device *dev,
+				unsigned int controllers, size_t sizeof_priv)
+{
+	struct mux_chip *mux_chip;
+	int i;
+
+	if (WARN_ON(!dev || !controllers))
+		return ERR_PTR(-EINVAL);
+
+	mux_chip = kzalloc(sizeof(*mux_chip) +
+			   controllers * sizeof(*mux_chip->mux) +
+			   sizeof_priv, GFP_KERNEL);
+	if (!mux_chip)
+		return ERR_PTR(-ENOMEM);
+
+	mux_chip->mux = (struct mux_control *)(mux_chip + 1);
+	mux_chip->dev.class = &mux_class;
+	mux_chip->dev.type = &mux_type;
+	mux_chip->dev.parent = dev;
+	mux_chip->dev.of_node = dev->of_node;
+	dev_set_drvdata(&mux_chip->dev, mux_chip);
+
+	mux_chip->id = ida_simple_get(&mux_ida, 0, 0, GFP_KERNEL);
+	if (mux_chip->id < 0) {
+		int err = mux_chip->id;
+
+		pr_err("muxchipX failed to get a device id\n");
+		kfree(mux_chip);
+		return ERR_PTR(err);
+	}
+	dev_set_name(&mux_chip->dev, "muxchip%d", mux_chip->id);
+
+	mux_chip->controllers = controllers;
+	for (i = 0; i < controllers; ++i) {
+		struct mux_control *mux = &mux_chip->mux[i];
+
+		mux->chip = mux_chip;
+		sema_init(&mux->lock, 1);
+		mux->cached_state = MUX_CACHE_UNKNOWN;
+		mux->idle_state = MUX_IDLE_AS_IS;
+	}
+
+	device_initialize(&mux_chip->dev);
+
+	return mux_chip;
+}
+EXPORT_SYMBOL_GPL(mux_chip_alloc);
+
+static int mux_control_set(struct mux_control *mux, int state)
+{
+	int ret = mux->chip->ops->set(mux, state);
+
+	mux->cached_state = ret < 0 ? MUX_CACHE_UNKNOWN : state;
+
+	return ret;
+}
+
+/**
+ * mux_chip_register() - Register a mux-chip, thus readying the controllers
+ *			 for use.
+ * @mux_chip: The mux-chip to register.
+ *
+ * Do not retry registration of the same mux-chip on failure. You should
+ * instead put it away with mux_chip_free() and allocate a new one, if you
+ * for some reason would like to retry registration.
+ *
+ * Return: Zero on success or a negative errno on error.
+ */
+int mux_chip_register(struct mux_chip *mux_chip)
+{
+	int i;
+	int ret;
+
+	for (i = 0; i < mux_chip->controllers; ++i) {
+		struct mux_control *mux = &mux_chip->mux[i];
+
+		if (mux->idle_state == mux->cached_state)
+			continue;
+
+		ret = mux_control_set(mux, mux->idle_state);
+		if (ret < 0) {
+			dev_err(&mux_chip->dev, "unable to set idle state\n");
+			return ret;
+		}
+	}
+
+	ret = device_add(&mux_chip->dev);
+	if (ret < 0)
+		dev_err(&mux_chip->dev,
+			"device_add failed in %s: %d\n", __func__, ret);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(mux_chip_register);
+
+/**
+ * mux_chip_unregister() - Take the mux-chip off-line.
+ * @mux_chip: The mux-chip to unregister.
+ *
+ * mux_chip_unregister() reverses the effects of mux_chip_register().
+ * But not completely, you should not try to call mux_chip_register()
+ * on a mux-chip that has been registered before.
+ */
+void mux_chip_unregister(struct mux_chip *mux_chip)
+{
+	device_del(&mux_chip->dev);
+}
+EXPORT_SYMBOL_GPL(mux_chip_unregister);
+
+/**
+ * mux_chip_free() - Free the mux-chip for good.
+ * @mux_chip: The mux-chip to free.
+ *
+ * mux_chip_free() reverses the effects of mux_chip_alloc().
+ */
+void mux_chip_free(struct mux_chip *mux_chip)
+{
+	if (!mux_chip)
+		return;
+
+	put_device(&mux_chip->dev);
+}
+EXPORT_SYMBOL_GPL(mux_chip_free);
+
+static void devm_mux_chip_release(struct device *dev, void *res)
+{
+	struct mux_chip *mux_chip = *(struct mux_chip **)res;
+
+	mux_chip_free(mux_chip);
+}
+
+/**
+ * devm_mux_chip_alloc() - Resource-managed version of mux_chip_alloc().
+ * @dev: The parent device implementing the mux interface.
+ * @controllers: The number of mux controllers to allocate for this chip.
+ * @sizeof_priv: Size of extra memory area for private use by the caller.
+ *
+ * See mux_chip_alloc() for more details.
+ *
+ * Return: A pointer to the new mux-chip, or an ERR_PTR with a negative errno.
+ */
+struct mux_chip *devm_mux_chip_alloc(struct device *dev,
+				     unsigned int controllers,
+				     size_t sizeof_priv)
+{
+	struct mux_chip **ptr, *mux_chip;
+
+	ptr = devres_alloc(devm_mux_chip_release, sizeof(*ptr), GFP_KERNEL);
+	if (!ptr)
+		return ERR_PTR(-ENOMEM);
+
+	mux_chip = mux_chip_alloc(dev, controllers, sizeof_priv);
+	if (IS_ERR(mux_chip)) {
+		devres_free(ptr);
+		return mux_chip;
+	}
+
+	*ptr = mux_chip;
+	devres_add(dev, ptr);
+
+	return mux_chip;
+}
+EXPORT_SYMBOL_GPL(devm_mux_chip_alloc);
+
+static void devm_mux_chip_reg_release(struct device *dev, void *res)
+{
+	struct mux_chip *mux_chip = *(struct mux_chip **)res;
+
+	mux_chip_unregister(mux_chip);
+}
+
+/**
+ * devm_mux_chip_register() - Resource-managed version mux_chip_register().
+ * @dev: The parent device implementing the mux interface.
+ * @mux_chip: The mux-chip to register.
+ *
+ * See mux_chip_register() for more details.
+ *
+ * Return: Zero on success or a negative errno on error.
+ */
+int devm_mux_chip_register(struct device *dev,
+			   struct mux_chip *mux_chip)
+{
+	struct mux_chip **ptr;
+	int res;
+
+	ptr = devres_alloc(devm_mux_chip_reg_release, sizeof(*ptr), GFP_KERNEL);
+	if (!ptr)
+		return -ENOMEM;
+
+	res = mux_chip_register(mux_chip);
+	if (res) {
+		devres_free(ptr);
+		return res;
+	}
+
+	*ptr = mux_chip;
+	devres_add(dev, ptr);
+
+	return res;
+}
+EXPORT_SYMBOL_GPL(devm_mux_chip_register);
+
+/**
+ * mux_control_states() - Query the number of multiplexer states.
+ * @mux: The mux-control to query.
+ *
+ * Return: The number of multiplexer states.
+ */
+unsigned int mux_control_states(struct mux_control *mux)
+{
+	return mux->states;
+}
+EXPORT_SYMBOL_GPL(mux_control_states);
+
+/*
+ * The mux->lock must be down when calling this function.
+ */
+static int __mux_control_select(struct mux_control *mux, int state)
+{
+	int ret;
+
+	if (WARN_ON(state < 0 || state >= mux->states))
+		return -EINVAL;
+
+	if (mux->cached_state == state)
+		return 0;
+
+	ret = mux_control_set(mux, state);
+	if (ret >= 0)
+		return 0;
+
+	/* The mux update failed, try to revert if appropriate... */
+	if (mux->idle_state != MUX_IDLE_AS_IS)
+		mux_control_set(mux, mux->idle_state);
+
+	return ret;
+}
+
+/**
+ * mux_control_select() - Select the given multiplexer state.
+ * @mux: The mux-control to request a change of state from.
+ * @state: The new requested state.
+ *
+ * On successfully selecting the mux-control state, it will be locked until
+ * there is a call to mux_control_deselect(). If the mux-control is already
+ * selected when mux_control_select() is called, the caller will be blocked
+ * until mux_control_deselect() is called (by someone else).
+ *
+ * Therefore, make sure to call mux_control_deselect() when the operation is
+ * complete and the mux-control is free for others to use, but do not call
+ * mux_control_deselect() if mux_control_select() fails.
+ *
+ * Return: 0 when the mux-control state has the requested state or a negative
+ * errno on error.
+ */
+int mux_control_select(struct mux_control *mux, unsigned int state)
+{
+	int ret;
+
+	ret = down_killable(&mux->lock);
+	if (ret < 0)
+		return ret;
+
+	ret = __mux_control_select(mux, state);
+
+	if (ret < 0)
+		up(&mux->lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(mux_control_select);
+
+/**
+ * mux_control_try_select() - Try to select the given multiplexer state.
+ * @mux: The mux-control to request a change of state from.
+ * @state: The new requested state.
+ *
+ * On successfully selecting the mux-control state, it will be locked until
+ * mux_control_deselect() called.
+ *
+ * Therefore, make sure to call mux_control_deselect() when the operation is
+ * complete and the mux-control is free for others to use, but do not call
+ * mux_control_deselect() if mux_control_try_select() fails.
+ *
+ * Return: 0 when the mux-control state has the requested state or a negative
+ * errno on error. Specifically -EBUSY if the mux-control is contended.
+ */
+int mux_control_try_select(struct mux_control *mux, unsigned int state)
+{
+	int ret;
+
+	if (down_trylock(&mux->lock))
+		return -EBUSY;
+
+	ret = __mux_control_select(mux, state);
+
+	if (ret < 0)
+		up(&mux->lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(mux_control_try_select);
+
+/**
+ * mux_control_deselect() - Deselect the previously selected multiplexer state.
+ * @mux: The mux-control to deselect.
+ *
+ * It is required that a single call is made to mux_control_deselect() for
+ * each and every successful call made to either of mux_control_select() or
+ * mux_control_try_select().
+ *
+ * Return: 0 on success and a negative errno on error. An error can only
+ * occur if the mux has an idle state. Note that even if an error occurs, the
+ * mux-control is unlocked and is thus free for the next access.
+ */
+int mux_control_deselect(struct mux_control *mux)
+{
+	int ret = 0;
+
+	if (mux->idle_state != MUX_IDLE_AS_IS &&
+	    mux->idle_state != mux->cached_state)
+		ret = mux_control_set(mux, mux->idle_state);
+
+	up(&mux->lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(mux_control_deselect);
+
+static int of_dev_node_match(struct device *dev, const void *data)
+{
+	return dev->of_node == data;
+}
+
+static struct mux_chip *of_find_mux_chip_by_node(struct device_node *np)
+{
+	struct device *dev;
+
+	dev = class_find_device(&mux_class, NULL, np, of_dev_node_match);
+
+	return dev ? to_mux_chip(dev) : NULL;
+}
+
+/**
+ * mux_control_get() - Get the mux-control for a device.
+ * @dev: The device that needs a mux-control.
+ * @mux_name: The name identifying the mux-control.
+ *
+ * Return: A pointer to the mux-control, or an ERR_PTR with a negative errno.
+ */
+struct mux_control *mux_control_get(struct device *dev, const char *mux_name)
+{
+	struct device_node *np = dev->of_node;
+	struct of_phandle_args args;
+	struct mux_chip *mux_chip;
+	unsigned int controller;
+	int index = 0;
+	int ret;
+
+	if (mux_name) {
+		index = of_property_match_string(np, "mux-control-names",
+						 mux_name);
+		if (index < 0) {
+			dev_err(dev, "mux controller '%s' not found\n",
+				mux_name);
+			return ERR_PTR(index);
+		}
+	}
+
+	ret = of_parse_phandle_with_args(np,
+					 "mux-controls", "#mux-control-cells",
+					 index, &args);
+	if (ret) {
+		dev_err(dev, "%s: failed to get mux-control %s(%i)\n",
+			np->full_name, mux_name ?: "", index);
+		return ERR_PTR(ret);
+	}
+
+	mux_chip = of_find_mux_chip_by_node(args.np);
+	of_node_put(args.np);
+	if (!mux_chip)
+		return ERR_PTR(-EPROBE_DEFER);
+
+	if (args.args_count > 1 ||
+	    (!args.args_count && (mux_chip->controllers > 1))) {
+		dev_err(dev, "%s: wrong #mux-control-cells for %s\n",
+			np->full_name, args.np->full_name);
+		return ERR_PTR(-EINVAL);
+	}
+
+	controller = 0;
+	if (args.args_count)
+		controller = args.args[0];
+
+	if (controller >= mux_chip->controllers) {
+		dev_err(dev, "%s: bad mux controller %u specified in %s\n",
+			np->full_name, controller, args.np->full_name);
+		return ERR_PTR(-EINVAL);
+	}
+
+	get_device(&mux_chip->dev);
+	return &mux_chip->mux[controller];
+}
+EXPORT_SYMBOL_GPL(mux_control_get);
+
+/**
+ * mux_control_put() - Put away the mux-control for good.
+ * @mux: The mux-control to put away.
+ *
+ * mux_control_put() reverses the effects of mux_control_get().
+ */
+void mux_control_put(struct mux_control *mux)
+{
+	put_device(&mux->chip->dev);
+}
+EXPORT_SYMBOL_GPL(mux_control_put);
+
+static void devm_mux_control_release(struct device *dev, void *res)
+{
+	struct mux_control *mux = *(struct mux_control **)res;
+
+	mux_control_put(mux);
+}
+
+/**
+ * devm_mux_control_get() - Get the mux-control for a device, with resource
+ *			    management.
+ * @dev: The device that needs a mux-control.
+ * @mux_name: The name identifying the mux-control.
+ *
+ * Return: Pointer to the mux-control, or an ERR_PTR with a negative errno.
+ */
+struct mux_control *devm_mux_control_get(struct device *dev,
+					 const char *mux_name)
+{
+	struct mux_control **ptr, *mux;
+
+	ptr = devres_alloc(devm_mux_control_release, sizeof(*ptr), GFP_KERNEL);
+	if (!ptr)
+		return ERR_PTR(-ENOMEM);
+
+	mux = mux_control_get(dev, mux_name);
+	if (IS_ERR(mux)) {
+		devres_free(ptr);
+		return mux;
+	}
+
+	*ptr = mux;
+	devres_add(dev, ptr);
+
+	return mux;
+}
+EXPORT_SYMBOL_GPL(devm_mux_control_get);
+
+/*
+ * Using subsys_initcall instead of module_init here to try to ensure - for
+ * the non-modular case - that the subsystem is initialized when mux consumers
+ * and mux controllers start to use it.
+ * For the modular case, the ordering is ensured with module dependencies.
+ */
+subsys_initcall(mux_init);
+module_exit(mux_exit);
+
+MODULE_DESCRIPTION("Multiplexer subsystem");
+MODULE_AUTHOR("Peter Rosin <peda@axentia.se>");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mux/consumer.h b/include/linux/mux/consumer.h
new file mode 100644
index 000000000000..5577e1b773c4
--- /dev/null
+++ b/include/linux/mux/consumer.h
@@ -0,0 +1,32 @@
+/*
+ * mux/consumer.h - definitions for the multiplexer consumer interface
+ *
+ * Copyright (C) 2017 Axentia Technologies AB
+ *
+ * Author: Peter Rosin <peda@axentia.se>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef _LINUX_MUX_CONSUMER_H
+#define _LINUX_MUX_CONSUMER_H
+
+struct device;
+struct mux_control;
+
+unsigned int mux_control_states(struct mux_control *mux);
+int __must_check mux_control_select(struct mux_control *mux,
+				    unsigned int state);
+int __must_check mux_control_try_select(struct mux_control *mux,
+					unsigned int state);
+int mux_control_deselect(struct mux_control *mux);
+
+struct mux_control *mux_control_get(struct device *dev, const char *mux_name);
+void mux_control_put(struct mux_control *mux);
+
+struct mux_control *devm_mux_control_get(struct device *dev,
+					 const char *mux_name);
+
+#endif /* _LINUX_MUX_CONSUMER_H */
diff --git a/include/linux/mux/driver.h b/include/linux/mux/driver.h
new file mode 100644
index 000000000000..35c3579c3304
--- /dev/null
+++ b/include/linux/mux/driver.h
@@ -0,0 +1,108 @@
+/*
+ * mux/driver.h - definitions for the multiplexer driver interface
+ *
+ * Copyright (C) 2017 Axentia Technologies AB
+ *
+ * Author: Peter Rosin <peda@axentia.se>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef _LINUX_MUX_DRIVER_H
+#define _LINUX_MUX_DRIVER_H
+
+#include <dt-bindings/mux/mux.h>
+#include <linux/device.h>
+#include <linux/semaphore.h>
+
+struct mux_chip;
+struct mux_control;
+
+/**
+ * struct mux_control_ops -	Mux controller operations for a mux chip.
+ * @set:			Set the state of the given mux controller.
+ */
+struct mux_control_ops {
+	int (*set)(struct mux_control *mux, int state);
+};
+
+/**
+ * struct mux_control -	Represents a mux controller.
+ * @lock:		Protects the mux controller state.
+ * @chip:		The mux chip that is handling this mux controller.
+ * @cached_state:	The current mux controller state, or -1 if none.
+ * @states:		The number of mux controller states.
+ * @idle_state:		The mux controller state to use when inactive, or one
+ *			of MUX_IDLE_AS_IS and MUX_IDLE_DISCONNECT.
+ *
+ * Mux drivers may only change @states and @idle_state, and may only do so
+ * between allocation and registration of the mux controller. Specifically,
+ * @cached_state is internal to the mux core and should never be written by
+ * mux drivers.
+ */
+struct mux_control {
+	struct semaphore lock; /* protects the state of the mux */
+
+	struct mux_chip *chip;
+	int cached_state;
+
+	unsigned int states;
+	int idle_state;
+};
+
+/**
+ * struct mux_chip -	Represents a chip holding mux controllers.
+ * @controllers:	Number of mux controllers handled by the chip.
+ * @mux:		Array of mux controllers that are handled.
+ * @dev:		Device structure.
+ * @id:			Used to identify the device internally.
+ * @ops:		Mux controller operations.
+ */
+struct mux_chip {
+	unsigned int controllers;
+	struct mux_control *mux;
+	struct device dev;
+	int id;
+
+	const struct mux_control_ops *ops;
+};
+
+#define to_mux_chip(x) container_of((x), struct mux_chip, dev)
+
+/**
+ * mux_chip_priv() - Get the extra memory reserved by mux_chip_alloc().
+ * @mux_chip: The mux-chip to get the private memory from.
+ *
+ * Return: Pointer to the private memory reserved by the allocator.
+ */
+static inline void *mux_chip_priv(struct mux_chip *mux_chip)
+{
+	return &mux_chip->mux[mux_chip->controllers];
+}
+
+struct mux_chip *mux_chip_alloc(struct device *dev,
+				unsigned int controllers, size_t sizeof_priv);
+int mux_chip_register(struct mux_chip *mux_chip);
+void mux_chip_unregister(struct mux_chip *mux_chip);
+void mux_chip_free(struct mux_chip *mux_chip);
+
+struct mux_chip *devm_mux_chip_alloc(struct device *dev,
+				     unsigned int controllers,
+				     size_t sizeof_priv);
+int devm_mux_chip_register(struct device *dev, struct mux_chip *mux_chip);
+
+/**
+ * mux_control_get_index() - Get the index of the given mux controller
+ * @mux: The mux-control to get the index for.
+ *
+ * Return: The index of the mux controller within the mux chip the mux
+ * controller is a part of.
+ */
+static inline unsigned int mux_control_get_index(struct mux_control *mux)
+{
+	return mux - mux->chip->mux;
+}
+
+#endif /* _LINUX_MUX_DRIVER_H */
-- 
2.1.4

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

* [PATCH v15 04/13] mux: gpio: add mux controller driver for gpio based multiplexers
  2017-05-14 19:51 [PATCH v15 00/13] mux controller abstraction and iio/i2c muxes Peter Rosin
                   ` (2 preceding siblings ...)
  2017-05-14 19:51 ` [PATCH v15 03/13] mux: minimal mux subsystem Peter Rosin
@ 2017-05-14 19:51 ` Peter Rosin
  2017-05-17  9:38   ` Philipp Zabel
  2017-05-14 19:51 ` [PATCH v15 05/13] iio: inkern: api for manipulating ext_info of iio channels Peter Rosin
                   ` (9 subsequent siblings)
  13 siblings, 1 reply; 26+ messages in thread
From: Peter Rosin @ 2017-05-14 19:51 UTC (permalink / raw)
  To: linux-kernel, Greg Kroah-Hartman
  Cc: Peter Rosin, Wolfram Sang, Rob Herring, Mark Rutland,
	Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Jonathan Corbet, linux-i2c, devicetree,
	linux-iio, linux-doc, Andrew Morton, Colin Ian King,
	Paul Gortmaker, Philipp Zabel, kernel

From: Peter Rosin <peda@axentia.se>

The driver builds a single multiplexer controller using a number
of gpio pins. For N pins, there will be 2^N possible multiplexer
states. The GPIO pins can be connected (by the hardware) to several
multiplexers, which in that case will be operated in parallel.

Reviewed-by: Jonathan Cameron <jic23@kernel.org>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/mux/Kconfig    |  18 ++++++++
 drivers/mux/Makefile   |   1 +
 drivers/mux/mux-gpio.c | 114 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 133 insertions(+)
 create mode 100644 drivers/mux/mux-gpio.c

diff --git a/drivers/mux/Kconfig b/drivers/mux/Kconfig
index 23ab2cde83b1..738670aaecb7 100644
--- a/drivers/mux/Kconfig
+++ b/drivers/mux/Kconfig
@@ -14,3 +14,21 @@ menuconfig MULTIPLEXER
 
 	  To compile the subsystem as a module, choose M here: the module will
 	  be called mux-core.
+
+if MULTIPLEXER
+
+config MUX_GPIO
+	tristate "GPIO-controlled Multiplexer"
+	depends on GPIOLIB || COMPILE_TEST
+	help
+	  GPIO-controlled Multiplexer controller.
+
+	  The driver builds a single multiplexer controller using a number
+	  of gpio pins. For N pins, there will be 2^N possible multiplexer
+	  states. The GPIO pins can be connected (by the hardware) to several
+	  multiplexers, which in that case will be operated in parallel.
+
+	  To compile the driver as a module, choose M here: the module will
+	  be called mux-gpio.
+
+endif
diff --git a/drivers/mux/Makefile b/drivers/mux/Makefile
index 09f0299e109d..bb16953f6290 100644
--- a/drivers/mux/Makefile
+++ b/drivers/mux/Makefile
@@ -3,3 +3,4 @@
 #
 
 obj-$(CONFIG_MULTIPLEXER)	+= mux-core.o
+obj-$(CONFIG_MUX_GPIO)		+= mux-gpio.o
diff --git a/drivers/mux/mux-gpio.c b/drivers/mux/mux-gpio.c
new file mode 100644
index 000000000000..468bf1709606
--- /dev/null
+++ b/drivers/mux/mux-gpio.c
@@ -0,0 +1,114 @@
+/*
+ * GPIO-controlled multiplexer driver
+ *
+ * Copyright (C) 2017 Axentia Technologies AB
+ *
+ * Author: Peter Rosin <peda@axentia.se>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/err.h>
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+#include <linux/mux/driver.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+
+struct mux_gpio {
+	struct gpio_descs *gpios;
+	int *val;
+};
+
+static int mux_gpio_set(struct mux_control *mux, int state)
+{
+	struct mux_gpio *mux_gpio = mux_chip_priv(mux->chip);
+	int i;
+
+	for (i = 0; i < mux_gpio->gpios->ndescs; i++)
+		mux_gpio->val[i] = (state >> i) & 1;
+
+	gpiod_set_array_value_cansleep(mux_gpio->gpios->ndescs,
+				       mux_gpio->gpios->desc,
+				       mux_gpio->val);
+
+	return 0;
+}
+
+static const struct mux_control_ops mux_gpio_ops = {
+	.set = mux_gpio_set,
+};
+
+static const struct of_device_id mux_gpio_dt_ids[] = {
+	{ .compatible = "gpio-mux", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, mux_gpio_dt_ids);
+
+static int mux_gpio_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct mux_chip *mux_chip;
+	struct mux_gpio *mux_gpio;
+	int pins;
+	s32 idle_state;
+	int ret;
+
+	pins = gpiod_count(dev, "mux");
+	if (pins < 0)
+		return pins;
+
+	mux_chip = devm_mux_chip_alloc(dev, 1, sizeof(*mux_gpio) +
+				       pins * sizeof(*mux_gpio->val));
+	if (IS_ERR(mux_chip))
+		return PTR_ERR(mux_chip);
+
+	mux_gpio = mux_chip_priv(mux_chip);
+	mux_gpio->val = (int *)(mux_gpio + 1);
+	mux_chip->ops = &mux_gpio_ops;
+
+	mux_gpio->gpios = devm_gpiod_get_array(dev, "mux", GPIOD_OUT_LOW);
+	if (IS_ERR(mux_gpio->gpios)) {
+		ret = PTR_ERR(mux_gpio->gpios);
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "failed to get gpios\n");
+		return ret;
+	}
+	WARN_ON(pins != mux_gpio->gpios->ndescs);
+	mux_chip->mux->states = 1 << pins;
+
+	ret = device_property_read_u32(dev, "idle-state", (u32 *)&idle_state);
+	if (ret >= 0 && idle_state != MUX_IDLE_AS_IS) {
+		if (idle_state < 0 || idle_state >= mux_chip->mux->states) {
+			dev_err(dev, "invalid idle-state %u\n", idle_state);
+			return -EINVAL;
+		}
+
+		mux_chip->mux->idle_state = idle_state;
+	}
+
+	ret = devm_mux_chip_register(dev, mux_chip);
+	if (ret < 0)
+		return ret;
+
+	dev_info(dev, "%u-way mux-controller registered\n",
+		 mux_chip->mux->states);
+
+	return 0;
+}
+
+static struct platform_driver mux_gpio_driver = {
+	.driver = {
+		.name = "gpio-mux",
+		.of_match_table	= of_match_ptr(mux_gpio_dt_ids),
+	},
+	.probe = mux_gpio_probe,
+};
+module_platform_driver(mux_gpio_driver);
+
+MODULE_DESCRIPTION("GPIO-controlled multiplexer driver");
+MODULE_AUTHOR("Peter Rosin <peda@axentia.se>");
+MODULE_LICENSE("GPL v2");
-- 
2.1.4

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

* [PATCH v15 05/13] iio: inkern: api for manipulating ext_info of iio channels
  2017-05-14 19:51 [PATCH v15 00/13] mux controller abstraction and iio/i2c muxes Peter Rosin
                   ` (3 preceding siblings ...)
  2017-05-14 19:51 ` [PATCH v15 04/13] mux: gpio: add mux controller driver for gpio based multiplexers Peter Rosin
@ 2017-05-14 19:51 ` Peter Rosin
  2017-05-14 19:51 ` [PATCH v15 06/13] dt-bindings: iio: io-channel-mux: document io-channel-mux bindings Peter Rosin
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 26+ messages in thread
From: Peter Rosin @ 2017-05-14 19:51 UTC (permalink / raw)
  To: linux-kernel, Greg Kroah-Hartman
  Cc: Peter Rosin, Wolfram Sang, Rob Herring, Mark Rutland,
	Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Jonathan Corbet, linux-i2c, devicetree,
	linux-iio, linux-doc, Andrew Morton, Colin Ian King,
	Paul Gortmaker, Philipp Zabel, kernel

From: Peter Rosin <peda@axentia.se>

Extend the inkern api with functions for reading and writing ext_info
of iio channels.

Acked-by: Jonathan Cameron <jic23@kernel.org>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/iio/inkern.c         | 60 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/iio/consumer.h | 37 +++++++++++++++++++++++++++
 2 files changed, 97 insertions(+)

diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index 7a13535dc3e9..8292ad4435ea 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -869,3 +869,63 @@ int iio_write_channel_raw(struct iio_channel *chan, int val)
 	return ret;
 }
 EXPORT_SYMBOL_GPL(iio_write_channel_raw);
+
+unsigned int iio_get_channel_ext_info_count(struct iio_channel *chan)
+{
+	const struct iio_chan_spec_ext_info *ext_info;
+	unsigned int i = 0;
+
+	if (!chan->channel->ext_info)
+		return i;
+
+	for (ext_info = chan->channel->ext_info; ext_info->name; ext_info++)
+		++i;
+
+	return i;
+}
+EXPORT_SYMBOL_GPL(iio_get_channel_ext_info_count);
+
+static const struct iio_chan_spec_ext_info *iio_lookup_ext_info(
+						const struct iio_channel *chan,
+						const char *attr)
+{
+	const struct iio_chan_spec_ext_info *ext_info;
+
+	if (!chan->channel->ext_info)
+		return NULL;
+
+	for (ext_info = chan->channel->ext_info; ext_info->name; ++ext_info) {
+		if (!strcmp(attr, ext_info->name))
+			return ext_info;
+	}
+
+	return NULL;
+}
+
+ssize_t iio_read_channel_ext_info(struct iio_channel *chan,
+				  const char *attr, char *buf)
+{
+	const struct iio_chan_spec_ext_info *ext_info;
+
+	ext_info = iio_lookup_ext_info(chan, attr);
+	if (!ext_info)
+		return -EINVAL;
+
+	return ext_info->read(chan->indio_dev, ext_info->private,
+			      chan->channel, buf);
+}
+EXPORT_SYMBOL_GPL(iio_read_channel_ext_info);
+
+ssize_t iio_write_channel_ext_info(struct iio_channel *chan, const char *attr,
+				   const char *buf, size_t len)
+{
+	const struct iio_chan_spec_ext_info *ext_info;
+
+	ext_info = iio_lookup_ext_info(chan, attr);
+	if (!ext_info)
+		return -EINVAL;
+
+	return ext_info->write(chan->indio_dev, ext_info->private,
+			       chan->channel, buf, len);
+}
+EXPORT_SYMBOL_GPL(iio_write_channel_ext_info);
diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h
index 47eeec3218b5..5e347a9805fd 100644
--- a/include/linux/iio/consumer.h
+++ b/include/linux/iio/consumer.h
@@ -312,4 +312,41 @@ int iio_read_channel_scale(struct iio_channel *chan, int *val,
 int iio_convert_raw_to_processed(struct iio_channel *chan, int raw,
 	int *processed, unsigned int scale);
 
+/**
+ * iio_get_channel_ext_info_count() - get number of ext_info attributes
+ *				      connected to the channel.
+ * @chan:		The channel being queried
+ *
+ * Returns the number of ext_info attributes
+ */
+unsigned int iio_get_channel_ext_info_count(struct iio_channel *chan);
+
+/**
+ * iio_read_channel_ext_info() - read ext_info attribute from a given channel
+ * @chan:		The channel being queried.
+ * @attr:		The ext_info attribute to read.
+ * @buf:		Where to store the attribute value. Assumed to hold
+ *			at least PAGE_SIZE bytes.
+ *
+ * Returns the number of bytes written to buf (perhaps w/o zero termination;
+ * it need not even be a string), or an error code.
+ */
+ssize_t iio_read_channel_ext_info(struct iio_channel *chan,
+				  const char *attr, char *buf);
+
+/**
+ * iio_write_channel_ext_info() - write ext_info attribute from a given channel
+ * @chan:		The channel being queried.
+ * @attr:		The ext_info attribute to read.
+ * @buf:		The new attribute value. Strings needs to be zero-
+ *			terminated, but the terminator should not be included
+ *			in the below len.
+ * @len:		The size of the new attribute value.
+ *
+ * Returns the number of accepted bytes, which should be the same as len.
+ * An error code can also be returned.
+ */
+ssize_t iio_write_channel_ext_info(struct iio_channel *chan, const char *attr,
+				   const char *buf, size_t len);
+
 #endif
-- 
2.1.4

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

* [PATCH v15 06/13] dt-bindings: iio: io-channel-mux: document io-channel-mux bindings
  2017-05-14 19:51 [PATCH v15 00/13] mux controller abstraction and iio/i2c muxes Peter Rosin
                   ` (4 preceding siblings ...)
  2017-05-14 19:51 ` [PATCH v15 05/13] iio: inkern: api for manipulating ext_info of iio channels Peter Rosin
@ 2017-05-14 19:51 ` Peter Rosin
  2017-05-14 19:51 ` [PATCH v15 07/13] iio: multiplexer: new iio category and iio-mux driver Peter Rosin
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 26+ messages in thread
From: Peter Rosin @ 2017-05-14 19:51 UTC (permalink / raw)
  To: linux-kernel, Greg Kroah-Hartman
  Cc: Peter Rosin, Wolfram Sang, Rob Herring, Mark Rutland,
	Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Jonathan Corbet, linux-i2c, devicetree,
	linux-iio, linux-doc, Andrew Morton, Colin Ian King,
	Paul Gortmaker, Philipp Zabel, kernel

From: Peter Rosin <peda@axentia.se>

Describe how a multiplexer can be used to select which signal is fed to
an io-channel.

Acked-by: Jonathan Cameron <jic23@kernel.org>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
 .../bindings/iio/multiplexer/io-channel-mux.txt    | 39 ++++++++++++++++++++++
 MAINTAINERS                                        |  6 ++++
 2 files changed, 45 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/multiplexer/io-channel-mux.txt

diff --git a/Documentation/devicetree/bindings/iio/multiplexer/io-channel-mux.txt b/Documentation/devicetree/bindings/iio/multiplexer/io-channel-mux.txt
new file mode 100644
index 000000000000..c82794002595
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/multiplexer/io-channel-mux.txt
@@ -0,0 +1,39 @@
+I/O channel multiplexer bindings
+
+If a multiplexer is used to select which hardware signal is fed to
+e.g. an ADC channel, these bindings describe that situation.
+
+Required properties:
+- compatible : "io-channel-mux"
+- io-channels : Channel node of the parent channel that has multiplexed
+		input.
+- io-channel-names : Should be "parent".
+- #address-cells = <1>;
+- #size-cells = <0>;
+- mux-controls : Mux controller node to use for operating the mux
+- channels : List of strings, labeling the mux controller states.
+
+For each non-empty string in the channels property, an io-channel will
+be created. The number of this io-channel is the same as the index into
+the list of strings in the channels property, and also matches the mux
+controller state. The mux controller state is described in
+../mux/mux-controller.txt
+
+Example:
+	mux: mux-controller {
+		compatible = "mux-gpio";
+		#mux-control-cells = <0>;
+
+		mux-gpios = <&pioA 0 GPIO_ACTIVE_HIGH>,
+			    <&pioA 1 GPIO_ACTIVE_HIGH>;
+	};
+
+	adc-mux {
+		compatible = "io-channel-mux";
+		io-channels = <&adc 0>;
+		io-channel-names = "parent";
+
+		mux-controls = <&mux>;
+
+		channels = "sync", "in", "system-regulator";
+	};
diff --git a/MAINTAINERS b/MAINTAINERS
index a0cce325b08e..eea8432b2df1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6474,6 +6474,12 @@ F:	Documentation/ABI/testing/sysfs-bus-iio-adc-envelope-detector
 F:	Documentation/devicetree/bindings/iio/adc/envelope-detector.txt
 F:	drivers/iio/adc/envelope-detector.c
 
+IIO MULTIPLEXER
+M:	Peter Rosin <peda@axentia.se>
+L:	linux-iio@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/iio/multiplexer/iio-mux.txt
+
 IIO SUBSYSTEM AND DRIVERS
 M:	Jonathan Cameron <jic23@kernel.org>
 R:	Hartmut Knaack <knaack.h@gmx.de>
-- 
2.1.4

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

* [PATCH v15 07/13] iio: multiplexer: new iio category and iio-mux driver
  2017-05-14 19:51 [PATCH v15 00/13] mux controller abstraction and iio/i2c muxes Peter Rosin
                   ` (5 preceding siblings ...)
  2017-05-14 19:51 ` [PATCH v15 06/13] dt-bindings: iio: io-channel-mux: document io-channel-mux bindings Peter Rosin
@ 2017-05-14 19:51 ` Peter Rosin
  2017-05-14 19:51 ` [PATCH v15 08/13] dt-bindings: i2c: i2c-mux: document general purpose i2c-mux bindings Peter Rosin
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 26+ messages in thread
From: Peter Rosin @ 2017-05-14 19:51 UTC (permalink / raw)
  To: linux-kernel, Greg Kroah-Hartman
  Cc: Peter Rosin, Wolfram Sang, Rob Herring, Mark Rutland,
	Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Jonathan Corbet, linux-i2c, devicetree,
	linux-iio, linux-doc, Andrew Morton, Colin Ian King,
	Paul Gortmaker, Philipp Zabel, kernel

From: Peter Rosin <peda@axentia.se>

When a multiplexer changes how an iio device behaves (for example
by feeding different signals to an ADC), this driver can be used
to create one virtual iio channel for each multiplexer state.

Depends on the generic multiplexer subsystem.

Cache any ext_info values from the parent iio channel, creating a private
copy of the ext_info attributes for each multiplexer state/channel.

Reviewed-by: Jonathan Cameron <jic23@kernel.org>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
 MAINTAINERS                       |   1 +
 drivers/iio/Kconfig               |   1 +
 drivers/iio/Makefile              |   1 +
 drivers/iio/multiplexer/Kconfig   |  18 ++
 drivers/iio/multiplexer/Makefile  |   6 +
 drivers/iio/multiplexer/iio-mux.c | 459 ++++++++++++++++++++++++++++++++++++++
 6 files changed, 486 insertions(+)
 create mode 100644 drivers/iio/multiplexer/Kconfig
 create mode 100644 drivers/iio/multiplexer/Makefile
 create mode 100644 drivers/iio/multiplexer/iio-mux.c

diff --git a/MAINTAINERS b/MAINTAINERS
index eea8432b2df1..c89d70c29089 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6479,6 +6479,7 @@ M:	Peter Rosin <peda@axentia.se>
 L:	linux-iio@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/iio/multiplexer/iio-mux.txt
+F:	drivers/iio/multiplexer/iio-mux.c
 
 IIO SUBSYSTEM AND DRIVERS
 M:	Jonathan Cameron <jic23@kernel.org>
diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
index a918270d6f54..b3c8c6ef0dff 100644
--- a/drivers/iio/Kconfig
+++ b/drivers/iio/Kconfig
@@ -83,6 +83,7 @@ source "drivers/iio/humidity/Kconfig"
 source "drivers/iio/imu/Kconfig"
 source "drivers/iio/light/Kconfig"
 source "drivers/iio/magnetometer/Kconfig"
+source "drivers/iio/multiplexer/Kconfig"
 source "drivers/iio/orientation/Kconfig"
 if IIO_TRIGGER
    source "drivers/iio/trigger/Kconfig"
diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
index 33fa4026f92c..93c769cd99bf 100644
--- a/drivers/iio/Makefile
+++ b/drivers/iio/Makefile
@@ -28,6 +28,7 @@ obj-y += humidity/
 obj-y += imu/
 obj-y += light/
 obj-y += magnetometer/
+obj-y += multiplexer/
 obj-y += orientation/
 obj-y += potentiometer/
 obj-y += potentiostat/
diff --git a/drivers/iio/multiplexer/Kconfig b/drivers/iio/multiplexer/Kconfig
new file mode 100644
index 000000000000..735a7b0e6fd8
--- /dev/null
+++ b/drivers/iio/multiplexer/Kconfig
@@ -0,0 +1,18 @@
+#
+# Multiplexer drivers
+#
+# When adding new entries keep the list in alphabetical order
+
+menu "Multiplexers"
+
+config IIO_MUX
+	tristate "IIO multiplexer driver"
+	select MULTIPLEXER
+	depends on OF || COMPILE_TEST
+	help
+	  Say yes here to build support for the IIO multiplexer.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called iio-mux.
+
+endmenu
diff --git a/drivers/iio/multiplexer/Makefile b/drivers/iio/multiplexer/Makefile
new file mode 100644
index 000000000000..68be3c4abd07
--- /dev/null
+++ b/drivers/iio/multiplexer/Makefile
@@ -0,0 +1,6 @@
+#
+# Makefile for industrial I/O multiplexer drivers
+#
+
+# When adding new entries keep the list in alphabetical order
+obj-$(CONFIG_IIO_MUX) += iio-mux.o
diff --git a/drivers/iio/multiplexer/iio-mux.c b/drivers/iio/multiplexer/iio-mux.c
new file mode 100644
index 000000000000..37ba007f8dca
--- /dev/null
+++ b/drivers/iio/multiplexer/iio-mux.c
@@ -0,0 +1,459 @@
+/*
+ * IIO multiplexer driver
+ *
+ * Copyright (C) 2017 Axentia Technologies AB
+ *
+ * Author: Peter Rosin <peda@axentia.se>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/err.h>
+#include <linux/iio/consumer.h>
+#include <linux/iio/iio.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/mux/consumer.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+
+struct mux_ext_info_cache {
+	char *data;
+	ssize_t size;
+};
+
+struct mux_child {
+	struct mux_ext_info_cache *ext_info_cache;
+};
+
+struct mux {
+	int cached_state;
+	struct mux_control *control;
+	struct iio_channel *parent;
+	struct iio_dev *indio_dev;
+	struct iio_chan_spec *chan;
+	struct iio_chan_spec_ext_info *ext_info;
+	struct mux_child *child;
+};
+
+static int iio_mux_select(struct mux *mux, int idx)
+{
+	struct mux_child *child = &mux->child[idx];
+	struct iio_chan_spec const *chan = &mux->chan[idx];
+	int ret;
+	int i;
+
+	ret = mux_control_select(mux->control, chan->channel);
+	if (ret < 0) {
+		mux->cached_state = -1;
+		return ret;
+	}
+
+	if (mux->cached_state == chan->channel)
+		return 0;
+
+	if (chan->ext_info) {
+		for (i = 0; chan->ext_info[i].name; ++i) {
+			const char *attr = chan->ext_info[i].name;
+			struct mux_ext_info_cache *cache;
+
+			cache = &child->ext_info_cache[i];
+
+			if (cache->size < 0)
+				continue;
+
+			ret = iio_write_channel_ext_info(mux->parent, attr,
+							 cache->data,
+							 cache->size);
+
+			if (ret < 0) {
+				mux_control_deselect(mux->control);
+				mux->cached_state = -1;
+				return ret;
+			}
+		}
+	}
+	mux->cached_state = chan->channel;
+
+	return 0;
+}
+
+static void iio_mux_deselect(struct mux *mux)
+{
+	mux_control_deselect(mux->control);
+}
+
+static int mux_read_raw(struct iio_dev *indio_dev,
+			struct iio_chan_spec const *chan,
+			int *val, int *val2, long mask)
+{
+	struct mux *mux = iio_priv(indio_dev);
+	int idx = chan - mux->chan;
+	int ret;
+
+	ret = iio_mux_select(mux, idx);
+	if (ret < 0)
+		return ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		ret = iio_read_channel_raw(mux->parent, val);
+		break;
+
+	case IIO_CHAN_INFO_SCALE:
+		ret = iio_read_channel_scale(mux->parent, val, val2);
+		break;
+
+	default:
+		ret = -EINVAL;
+	}
+
+	iio_mux_deselect(mux);
+
+	return ret;
+}
+
+static int mux_read_avail(struct iio_dev *indio_dev,
+			  struct iio_chan_spec const *chan,
+			  const int **vals, int *type, int *length,
+			  long mask)
+{
+	struct mux *mux = iio_priv(indio_dev);
+	int idx = chan - mux->chan;
+	int ret;
+
+	ret = iio_mux_select(mux, idx);
+	if (ret < 0)
+		return ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		*type = IIO_VAL_INT;
+		ret = iio_read_avail_channel_raw(mux->parent, vals, length);
+		break;
+
+	default:
+		ret = -EINVAL;
+	}
+
+	iio_mux_deselect(mux);
+
+	return ret;
+}
+
+static int mux_write_raw(struct iio_dev *indio_dev,
+			 struct iio_chan_spec const *chan,
+			 int val, int val2, long mask)
+{
+	struct mux *mux = iio_priv(indio_dev);
+	int idx = chan - mux->chan;
+	int ret;
+
+	ret = iio_mux_select(mux, idx);
+	if (ret < 0)
+		return ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		ret = iio_write_channel_raw(mux->parent, val);
+		break;
+
+	default:
+		ret = -EINVAL;
+	}
+
+	iio_mux_deselect(mux);
+
+	return ret;
+}
+
+static const struct iio_info mux_info = {
+	.read_raw = mux_read_raw,
+	.read_avail = mux_read_avail,
+	.write_raw = mux_write_raw,
+	.driver_module = THIS_MODULE,
+};
+
+static ssize_t mux_read_ext_info(struct iio_dev *indio_dev, uintptr_t private,
+				 struct iio_chan_spec const *chan, char *buf)
+{
+	struct mux *mux = iio_priv(indio_dev);
+	int idx = chan - mux->chan;
+	ssize_t ret;
+
+	ret = iio_mux_select(mux, idx);
+	if (ret < 0)
+		return ret;
+
+	ret = iio_read_channel_ext_info(mux->parent,
+					mux->ext_info[private].name,
+					buf);
+
+	iio_mux_deselect(mux);
+
+	return ret;
+}
+
+static ssize_t mux_write_ext_info(struct iio_dev *indio_dev, uintptr_t private,
+				  struct iio_chan_spec const *chan,
+				  const char *buf, size_t len)
+{
+	struct device *dev = indio_dev->dev.parent;
+	struct mux *mux = iio_priv(indio_dev);
+	int idx = chan - mux->chan;
+	char *new;
+	ssize_t ret;
+
+	if (len >= PAGE_SIZE)
+		return -EINVAL;
+
+	ret = iio_mux_select(mux, idx);
+	if (ret < 0)
+		return ret;
+
+	new = devm_kmemdup(dev, buf, len + 1, GFP_KERNEL);
+	if (!new) {
+		iio_mux_deselect(mux);
+		return -ENOMEM;
+	}
+
+	new[len] = 0;
+
+	ret = iio_write_channel_ext_info(mux->parent,
+					 mux->ext_info[private].name,
+					 buf, len);
+	if (ret < 0) {
+		iio_mux_deselect(mux);
+		devm_kfree(dev, new);
+		return ret;
+	}
+
+	devm_kfree(dev, mux->child[idx].ext_info_cache[private].data);
+	mux->child[idx].ext_info_cache[private].data = new;
+	mux->child[idx].ext_info_cache[private].size = len;
+
+	iio_mux_deselect(mux);
+
+	return ret;
+}
+
+static int mux_configure_channel(struct device *dev, struct mux *mux,
+				 u32 state, const char *label, int idx)
+{
+	struct mux_child *child = &mux->child[idx];
+	struct iio_chan_spec *chan = &mux->chan[idx];
+	struct iio_chan_spec const *pchan = mux->parent->channel;
+	char *page = NULL;
+	int num_ext_info;
+	int i;
+	int ret;
+
+	chan->indexed = 1;
+	chan->output = pchan->output;
+	chan->datasheet_name = label;
+	chan->ext_info = mux->ext_info;
+
+	ret = iio_get_channel_type(mux->parent, &chan->type);
+	if (ret < 0) {
+		dev_err(dev, "failed to get parent channel type\n");
+		return ret;
+	}
+
+	if (iio_channel_has_info(pchan, IIO_CHAN_INFO_RAW))
+		chan->info_mask_separate |= BIT(IIO_CHAN_INFO_RAW);
+	if (iio_channel_has_info(pchan, IIO_CHAN_INFO_SCALE))
+		chan->info_mask_separate |= BIT(IIO_CHAN_INFO_SCALE);
+
+	if (iio_channel_has_available(pchan, IIO_CHAN_INFO_RAW))
+		chan->info_mask_separate_available |= BIT(IIO_CHAN_INFO_RAW);
+
+	if (state >= mux_control_states(mux->control)) {
+		dev_err(dev, "too many channels\n");
+		return -EINVAL;
+	}
+
+	chan->channel = state;
+
+	num_ext_info = iio_get_channel_ext_info_count(mux->parent);
+	if (num_ext_info) {
+		page = devm_kzalloc(dev, PAGE_SIZE, GFP_KERNEL);
+		if (!page)
+			return -ENOMEM;
+	}
+	child->ext_info_cache = devm_kzalloc(dev,
+					     sizeof(*child->ext_info_cache) *
+					     num_ext_info, GFP_KERNEL);
+	for (i = 0; i < num_ext_info; ++i) {
+		child->ext_info_cache[i].size = -1;
+
+		if (!pchan->ext_info[i].write)
+			continue;
+		if (!pchan->ext_info[i].read)
+			continue;
+
+		ret = iio_read_channel_ext_info(mux->parent,
+						mux->ext_info[i].name,
+						page);
+		if (ret < 0) {
+			dev_err(dev, "failed to get ext_info '%s'\n",
+				pchan->ext_info[i].name);
+			return ret;
+		}
+		if (ret >= PAGE_SIZE) {
+			dev_err(dev, "too large ext_info '%s'\n",
+				pchan->ext_info[i].name);
+			return -EINVAL;
+		}
+
+		child->ext_info_cache[i].data = devm_kmemdup(dev, page, ret + 1,
+							     GFP_KERNEL);
+		child->ext_info_cache[i].data[ret] = 0;
+		child->ext_info_cache[i].size = ret;
+	}
+
+	if (page)
+		devm_kfree(dev, page);
+
+	return 0;
+}
+
+/*
+ * Same as of_property_for_each_string(), but also keeps track of the
+ * index of each string.
+ */
+#define of_property_for_each_string_index(np, propname, prop, s, i)	\
+	for (prop = of_find_property(np, propname, NULL),		\
+	     s = of_prop_next_string(prop, NULL),			\
+	     i = 0;							\
+	     s;								\
+	     s = of_prop_next_string(prop, s),				\
+	     i++)
+
+static int mux_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = pdev->dev.of_node;
+	struct iio_dev *indio_dev;
+	struct iio_channel *parent;
+	struct mux *mux;
+	struct property *prop;
+	const char *label;
+	u32 state;
+	int sizeof_ext_info;
+	int children;
+	int sizeof_priv;
+	int i;
+	int ret;
+
+	if (!np)
+		return -ENODEV;
+
+	parent = devm_iio_channel_get(dev, "parent");
+	if (IS_ERR(parent)) {
+		if (PTR_ERR(parent) != -EPROBE_DEFER)
+			dev_err(dev, "failed to get parent channel\n");
+		return PTR_ERR(parent);
+	}
+
+	sizeof_ext_info = iio_get_channel_ext_info_count(parent);
+	if (sizeof_ext_info) {
+		sizeof_ext_info += 1; /* one extra entry for the sentinel */
+		sizeof_ext_info *= sizeof(*mux->ext_info);
+	}
+
+	children = 0;
+	of_property_for_each_string(np, "channels", prop, label) {
+		if (*label)
+			children++;
+	}
+	if (children <= 0) {
+		dev_err(dev, "not even a single child\n");
+		return -EINVAL;
+	}
+
+	sizeof_priv = sizeof(*mux);
+	sizeof_priv += sizeof(*mux->child) * children;
+	sizeof_priv += sizeof(*mux->chan) * children;
+	sizeof_priv += sizeof_ext_info;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof_priv);
+	if (!indio_dev)
+		return -ENOMEM;
+
+	mux = iio_priv(indio_dev);
+	mux->child = (struct mux_child *)(mux + 1);
+	mux->chan = (struct iio_chan_spec *)(mux->child + children);
+
+	platform_set_drvdata(pdev, indio_dev);
+
+	mux->parent = parent;
+	mux->cached_state = -1;
+
+	indio_dev->name = dev_name(dev);
+	indio_dev->dev.parent = dev;
+	indio_dev->info = &mux_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = mux->chan;
+	indio_dev->num_channels = children;
+	if (sizeof_ext_info) {
+		mux->ext_info = devm_kmemdup(dev,
+					     parent->channel->ext_info,
+					     sizeof_ext_info, GFP_KERNEL);
+		if (!mux->ext_info)
+			return -ENOMEM;
+
+		for (i = 0; mux->ext_info[i].name; ++i) {
+			if (parent->channel->ext_info[i].read)
+				mux->ext_info[i].read = mux_read_ext_info;
+			if (parent->channel->ext_info[i].write)
+				mux->ext_info[i].write = mux_write_ext_info;
+			mux->ext_info[i].private = i;
+		}
+	}
+
+	mux->control = devm_mux_control_get(dev, NULL);
+	if (IS_ERR(mux->control)) {
+		if (PTR_ERR(mux->control) != -EPROBE_DEFER)
+			dev_err(dev, "failed to get control-mux\n");
+		return PTR_ERR(mux->control);
+	}
+
+	i = 0;
+	of_property_for_each_string_index(np, "channels", prop, label, state) {
+		if (!*label)
+			continue;
+
+		ret = mux_configure_channel(dev, mux, state, label, i++);
+		if (ret < 0)
+			return ret;
+	}
+
+	ret = devm_iio_device_register(dev, indio_dev);
+	if (ret) {
+		dev_err(dev, "failed to register iio device\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static const struct of_device_id mux_match[] = {
+	{ .compatible = "io-channel-mux" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, mux_match);
+
+static struct platform_driver mux_driver = {
+	.probe = mux_probe,
+	.driver = {
+		.name = "iio-mux",
+		.of_match_table = mux_match,
+	},
+};
+module_platform_driver(mux_driver);
+
+MODULE_DESCRIPTION("IIO multiplexer driver");
+MODULE_AUTHOR("Peter Rosin <peda@axentia.se>");
+MODULE_LICENSE("GPL v2");
-- 
2.1.4

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

* [PATCH v15 08/13] dt-bindings: i2c: i2c-mux: document general purpose i2c-mux bindings
  2017-05-14 19:51 [PATCH v15 00/13] mux controller abstraction and iio/i2c muxes Peter Rosin
                   ` (6 preceding siblings ...)
  2017-05-14 19:51 ` [PATCH v15 07/13] iio: multiplexer: new iio category and iio-mux driver Peter Rosin
@ 2017-05-14 19:51 ` Peter Rosin
  2017-05-14 19:51 ` [PATCH v15 09/13] i2c: i2c-mux-gpmux: new driver Peter Rosin
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 26+ messages in thread
From: Peter Rosin @ 2017-05-14 19:51 UTC (permalink / raw)
  To: linux-kernel, Greg Kroah-Hartman
  Cc: Peter Rosin, Wolfram Sang, Rob Herring, Mark Rutland,
	Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Jonathan Corbet, linux-i2c, devicetree,
	linux-iio, linux-doc, Andrew Morton, Colin Ian King,
	Paul Gortmaker, Philipp Zabel, kernel

From: Peter Rosin <peda@axentia.se>

Describe how a general purpose multiplexer controller is used to mux an
i2c bus.

Acked-by: Jonathan Cameron <jic23@kernel.org>
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
 .../devicetree/bindings/i2c/i2c-mux-gpmux.txt      | 99 ++++++++++++++++++++++
 1 file changed, 99 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-mux-gpmux.txt

diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-gpmux.txt b/Documentation/devicetree/bindings/i2c/i2c-mux-gpmux.txt
new file mode 100644
index 000000000000..2907dab56298
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-mux-gpmux.txt
@@ -0,0 +1,99 @@
+General Purpose I2C Bus Mux
+
+This binding describes an I2C bus multiplexer that uses a mux controller
+from the mux subsystem to route the I2C signals.
+
+                                  .-----.  .-----.
+                                  | dev |  | dev |
+    .------------.                '-----'  '-----'
+    | SoC        |                   |        |
+    |            |          .--------+--------'
+    |   .------. |  .------+    child bus A, on MUX value set to 0
+    |   | I2C  |-|--| Mux  |
+    |   '------' |  '--+---+    child bus B, on MUX value set to 1
+    |   .------. |     |    '----------+--------+--------.
+    |   | MUX- | |     |               |        |        |
+    |   | Ctrl |-|-----+            .-----.  .-----.  .-----.
+    |   '------' |                  | dev |  | dev |  | dev |
+    '------------'                  '-----'  '-----'  '-----'
+
+Required properties:
+- compatible: i2c-mux
+- i2c-parent: The phandle of the I2C bus that this multiplexer's master-side
+  port is connected to.
+- mux-controls: The phandle of the mux controller to use for operating the
+  mux.
+* Standard I2C mux properties. See i2c-mux.txt in this directory.
+* I2C child bus nodes. See i2c-mux.txt in this directory. The sub-bus number
+  is also the mux-controller state described in ../mux/mux-controller.txt
+
+Optional properties:
+- mux-locked: If present, explicitly allow unrelated I2C transactions on the
+  parent I2C adapter at these times:
+   + during setup of the multiplexer
+   + between setup of the multiplexer and the child bus I2C transaction
+   + between the child bus I2C transaction and releasing of the multiplexer
+   + during releasing of the multiplexer
+  However, I2C transactions to devices behind all I2C multiplexers connected
+  to the same parent adapter that this multiplexer is connected to are blocked
+  for the full duration of the complete multiplexed I2C transaction (i.e.
+  including the times covered by the above list).
+  If mux-locked is not present, the multiplexer is assumed to be parent-locked.
+  This means that no unrelated I2C transactions are allowed on the parent I2C
+  adapter for the complete multiplexed I2C transaction.
+  The properties of mux-locked and parent-locked multiplexers are discussed
+  in more detail in Documentation/i2c/i2c-topology.
+
+For each i2c child node, an I2C child bus will be created. They will
+be numbered based on their order in the device tree.
+
+Whenever an access is made to a device on a child bus, the value set
+in the relevant node's reg property will be set as the state in the
+mux controller.
+
+Example:
+	mux: mux-controller {
+		compatible = "gpio-mux";
+		#mux-control-cells = <0>;
+
+		mux-gpios = <&pioA 0 GPIO_ACTIVE_HIGH>,
+			    <&pioA 1 GPIO_ACTIVE_HIGH>;
+	};
+
+	i2c-mux {
+		compatible = "i2c-mux";
+		mux-locked;
+		i2c-parent = <&i2c1>;
+
+		mux-controls = <&mux>;
+
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		i2c@1 {
+			reg = <1>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			ssd1307: oled@3c {
+				compatible = "solomon,ssd1307fb-i2c";
+				reg = <0x3c>;
+				pwms = <&pwm 4 3000>;
+				reset-gpios = <&gpio2 7 1>;
+				reset-active-low;
+			};
+		};
+
+		i2c@3 {
+			reg = <3>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			pca9555: pca9555@20 {
+				compatible = "nxp,pca9555";
+				gpio-controller;
+				#gpio-cells = <2>;
+				reg = <0x20>;
+			};
+		};
+	};
-- 
2.1.4

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

* [PATCH v15 09/13] i2c: i2c-mux-gpmux: new driver
  2017-05-14 19:51 [PATCH v15 00/13] mux controller abstraction and iio/i2c muxes Peter Rosin
                   ` (7 preceding siblings ...)
  2017-05-14 19:51 ` [PATCH v15 08/13] dt-bindings: i2c: i2c-mux: document general purpose i2c-mux bindings Peter Rosin
@ 2017-05-14 19:51 ` Peter Rosin
  2017-05-14 19:51 ` [PATCH v15 10/13] dt-bindings: mux-adg792a: document devicetree bindings for ADG792A/G mux Peter Rosin
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 26+ messages in thread
From: Peter Rosin @ 2017-05-14 19:51 UTC (permalink / raw)
  To: linux-kernel, Greg Kroah-Hartman
  Cc: Peter Rosin, Wolfram Sang, Rob Herring, Mark Rutland,
	Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Jonathan Corbet, linux-i2c, devicetree,
	linux-iio, linux-doc, Andrew Morton, Colin Ian King,
	Paul Gortmaker, Philipp Zabel, kernel

From: Peter Rosin <peda@axentia.se>

This is a general purpose i2c mux that uses a multiplexer controlled by
the multiplexer subsystem to do the muxing.

The user can select if the mux is to be mux-locked and parent-locked
as described in Documentation/i2c/i2c-topology.

Acked-by: Jonathan Cameron <jic23@kernel.org>
Acked-by: Wolfram Sang <wsa@the-dreams.de>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/i2c/muxes/Kconfig         |  13 +++
 drivers/i2c/muxes/Makefile        |   1 +
 drivers/i2c/muxes/i2c-mux-gpmux.c | 173 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 187 insertions(+)
 create mode 100644 drivers/i2c/muxes/i2c-mux-gpmux.c

diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig
index 1e160fc37ecc..2c64d0e0740f 100644
--- a/drivers/i2c/muxes/Kconfig
+++ b/drivers/i2c/muxes/Kconfig
@@ -30,6 +30,19 @@ config I2C_MUX_GPIO
 	  This driver can also be built as a module.  If so, the module
 	  will be called i2c-mux-gpio.
 
+config I2C_MUX_GPMUX
+	tristate "General Purpose I2C multiplexer"
+	select MULTIPLEXER
+	depends on OF || COMPILE_TEST
+	help
+	  If you say yes to this option, support will be included for a
+	  general purpose I2C multiplexer. This driver provides access to
+	  I2C busses connected through a MUX, which in turn is controlled
+	  by a MUX-controller from the MUX subsystem.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called i2c-mux-gpmux.
+
 config I2C_MUX_LTC4306
 	tristate "LTC LTC4306/5 I2C multiplexer"
 	select GPIOLIB
diff --git a/drivers/i2c/muxes/Makefile b/drivers/i2c/muxes/Makefile
index ff7618cd5312..4a67d3199877 100644
--- a/drivers/i2c/muxes/Makefile
+++ b/drivers/i2c/muxes/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_I2C_ARB_GPIO_CHALLENGE)	+= i2c-arb-gpio-challenge.o
 obj-$(CONFIG_I2C_DEMUX_PINCTRL)		+= i2c-demux-pinctrl.o
 
 obj-$(CONFIG_I2C_MUX_GPIO)	+= i2c-mux-gpio.o
+obj-$(CONFIG_I2C_MUX_GPMUX)	+= i2c-mux-gpmux.o
 obj-$(CONFIG_I2C_MUX_LTC4306)	+= i2c-mux-ltc4306.o
 obj-$(CONFIG_I2C_MUX_MLXCPLD)	+= i2c-mux-mlxcpld.o
 obj-$(CONFIG_I2C_MUX_PCA9541)	+= i2c-mux-pca9541.o
diff --git a/drivers/i2c/muxes/i2c-mux-gpmux.c b/drivers/i2c/muxes/i2c-mux-gpmux.c
new file mode 100644
index 000000000000..92cf5f48afe6
--- /dev/null
+++ b/drivers/i2c/muxes/i2c-mux-gpmux.c
@@ -0,0 +1,173 @@
+/*
+ * General Purpose I2C multiplexer
+ *
+ * Copyright (C) 2017 Axentia Technologies AB
+ *
+ * Author: Peter Rosin <peda@axentia.se>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/i2c.h>
+#include <linux/i2c-mux.h>
+#include <linux/module.h>
+#include <linux/mux/consumer.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+
+struct mux {
+	struct mux_control *control;
+
+	bool do_not_deselect;
+};
+
+static int i2c_mux_select(struct i2c_mux_core *muxc, u32 chan)
+{
+	struct mux *mux = i2c_mux_priv(muxc);
+	int ret;
+
+	ret = mux_control_select(mux->control, chan);
+	mux->do_not_deselect = ret < 0;
+
+	return ret;
+}
+
+static int i2c_mux_deselect(struct i2c_mux_core *muxc, u32 chan)
+{
+	struct mux *mux = i2c_mux_priv(muxc);
+
+	if (mux->do_not_deselect)
+		return 0;
+
+	return mux_control_deselect(mux->control);
+}
+
+static struct i2c_adapter *mux_parent_adapter(struct device *dev)
+{
+	struct device_node *np = dev->of_node;
+	struct device_node *parent_np;
+	struct i2c_adapter *parent;
+
+	parent_np = of_parse_phandle(np, "i2c-parent", 0);
+	if (!parent_np) {
+		dev_err(dev, "Cannot parse i2c-parent\n");
+		return ERR_PTR(-ENODEV);
+	}
+	parent = of_find_i2c_adapter_by_node(parent_np);
+	of_node_put(parent_np);
+	if (!parent)
+		return ERR_PTR(-EPROBE_DEFER);
+
+	return parent;
+}
+
+static const struct of_device_id i2c_mux_of_match[] = {
+	{ .compatible = "i2c-mux", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, i2c_mux_of_match);
+
+static int i2c_mux_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct device_node *child;
+	struct i2c_mux_core *muxc;
+	struct mux *mux;
+	struct i2c_adapter *parent;
+	int children;
+	int ret;
+
+	if (!np)
+		return -ENODEV;
+
+	mux = devm_kzalloc(dev, sizeof(*mux), GFP_KERNEL);
+	if (!mux)
+		return -ENOMEM;
+
+	mux->control = devm_mux_control_get(dev, NULL);
+	if (IS_ERR(mux->control)) {
+		if (PTR_ERR(mux->control) != -EPROBE_DEFER)
+			dev_err(dev, "failed to get control-mux\n");
+		return PTR_ERR(mux->control);
+	}
+
+	parent = mux_parent_adapter(dev);
+	if (IS_ERR(parent)) {
+		if (PTR_ERR(parent) != -EPROBE_DEFER)
+			dev_err(dev, "failed to get i2c-parent adapter\n");
+		return PTR_ERR(parent);
+	}
+
+	children = of_get_child_count(np);
+
+	muxc = i2c_mux_alloc(parent, dev, children, 0, 0,
+			     i2c_mux_select, i2c_mux_deselect);
+	if (!muxc) {
+		ret = -ENOMEM;
+		goto err_parent;
+	}
+	muxc->priv = mux;
+
+	platform_set_drvdata(pdev, muxc);
+
+	muxc->mux_locked = of_property_read_bool(np, "mux-locked");
+
+	for_each_child_of_node(np, child) {
+		u32 chan;
+
+		ret = of_property_read_u32(child, "reg", &chan);
+		if (ret < 0) {
+			dev_err(dev, "no reg property for node '%s'\n",
+				child->name);
+			goto err_children;
+		}
+
+		if (chan >= mux_control_states(mux->control)) {
+			dev_err(dev, "invalid reg %u\n", chan);
+			ret = -EINVAL;
+			goto err_children;
+		}
+
+		ret = i2c_mux_add_adapter(muxc, 0, chan, 0);
+		if (ret)
+			goto err_children;
+	}
+
+	dev_info(dev, "%d-port mux on %s adapter\n", children, parent->name);
+
+	return 0;
+
+err_children:
+	i2c_mux_del_adapters(muxc);
+err_parent:
+	i2c_put_adapter(parent);
+
+	return ret;
+}
+
+static int i2c_mux_remove(struct platform_device *pdev)
+{
+	struct i2c_mux_core *muxc = platform_get_drvdata(pdev);
+
+	i2c_mux_del_adapters(muxc);
+	i2c_put_adapter(muxc->parent);
+
+	return 0;
+}
+
+static struct platform_driver i2c_mux_driver = {
+	.probe	= i2c_mux_probe,
+	.remove	= i2c_mux_remove,
+	.driver	= {
+		.name	= "i2c-mux-gpmux",
+		.of_match_table = i2c_mux_of_match,
+	},
+};
+module_platform_driver(i2c_mux_driver);
+
+MODULE_DESCRIPTION("General Purpose I2C multiplexer driver");
+MODULE_AUTHOR("Peter Rosin <peda@axentia.se>");
+MODULE_LICENSE("GPL v2");
-- 
2.1.4

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

* [PATCH v15 10/13] dt-bindings: mux-adg792a: document devicetree bindings for ADG792A/G mux
  2017-05-14 19:51 [PATCH v15 00/13] mux controller abstraction and iio/i2c muxes Peter Rosin
                   ` (8 preceding siblings ...)
  2017-05-14 19:51 ` [PATCH v15 09/13] i2c: i2c-mux-gpmux: new driver Peter Rosin
@ 2017-05-14 19:51 ` Peter Rosin
  2017-05-14 19:51 ` [PATCH v15 11/13] mux: adg792a: add mux controller driver for ADG792A/G Peter Rosin
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 26+ messages in thread
From: Peter Rosin @ 2017-05-14 19:51 UTC (permalink / raw)
  To: linux-kernel, Greg Kroah-Hartman
  Cc: Peter Rosin, Wolfram Sang, Rob Herring, Mark Rutland,
	Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Jonathan Corbet, linux-i2c, devicetree,
	linux-iio, linux-doc, Andrew Morton, Colin Ian King,
	Paul Gortmaker, Philipp Zabel, kernel

From: Peter Rosin <peda@axentia.se>

Analog Devices ADG792A/G is a triple 4:1 mux.

Acked-by: Jonathan Cameron <jic23@kernel.org>
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
 .../devicetree/bindings/mux/adi,adg792a.txt        | 75 ++++++++++++++++++++++
 1 file changed, 75 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mux/adi,adg792a.txt

diff --git a/Documentation/devicetree/bindings/mux/adi,adg792a.txt b/Documentation/devicetree/bindings/mux/adi,adg792a.txt
new file mode 100644
index 000000000000..96b787a69f50
--- /dev/null
+++ b/Documentation/devicetree/bindings/mux/adi,adg792a.txt
@@ -0,0 +1,75 @@
+Bindings for Analog Devices ADG792A/G Triple 4:1 Multiplexers
+
+Required properties:
+- compatible : "adi,adg792a" or "adi,adg792g"
+- #mux-control-cells : <0> if parallel (the three muxes are bound together
+  with a single mux controller controlling all three muxes), or <1> if
+  not (one mux controller for each mux).
+* Standard mux-controller bindings as described in mux-controller.txt
+
+Optional properties for ADG792G:
+- gpio-controller : if present, #gpio-cells below is required.
+- #gpio-cells : should be <2>
+			  - First cell is the GPO line number, i.e. 0 or 1
+			  - Second cell is used to specify active high (0)
+			    or active low (1)
+
+Optional properties:
+- idle-state : if present, array of states that the mux controllers will have
+  when idle. The special state MUX_IDLE_AS_IS is the default and
+  MUX_IDLE_DISCONNECT is also supported.
+
+States 0 through 3 correspond to signals A through D in the datasheet.
+
+Example:
+
+	/*
+	 * Three independent mux controllers (of which one is used).
+	 * Mux 0 is disconnected when idle, mux 1 idles in the previously
+	 * selected state and mux 2 idles with signal B.
+	 */
+	&i2c0 {
+		mux: mux-controller@50 {
+			compatible = "adi,adg792a";
+			reg = <0x50>;
+			#mux-control-cells = <1>;
+
+			idle-state = <MUX_IDLE_DISCONNECT MUX_IDLE_AS_IS 1>;
+		};
+	};
+
+	adc-mux {
+		compatible = "io-channel-mux";
+		io-channels = <&adc 0>;
+		io-channel-names = "parent";
+
+		mux-controls = <&mux 2>;
+
+		channels = "sync-1", "", "out";
+	};
+
+
+	/*
+	 * Three parallel muxes with one mux controller, useful e.g. if
+	 * the adc is differential, thus needing two signals to be muxed
+	 * simultaneously for correct operation.
+	 */
+	&i2c0 {
+		pmux: mux-controller@50 {
+			compatible = "adi,adg792a";
+			reg = <0x50>;
+			#mux-control-cells = <0>;
+
+			idle-state = <1>;
+		};
+	};
+
+	diff-adc-mux {
+		compatible = "io-channel-mux";
+		io-channels = <&adc 0>;
+		io-channel-names = "parent";
+
+		mux-controls = <&pmux>;
+
+		channels = "sync-1", "", "out";
+	};
-- 
2.1.4

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

* [PATCH v15 11/13] mux: adg792a: add mux controller driver for ADG792A/G
  2017-05-14 19:51 [PATCH v15 00/13] mux controller abstraction and iio/i2c muxes Peter Rosin
                   ` (9 preceding siblings ...)
  2017-05-14 19:51 ` [PATCH v15 10/13] dt-bindings: mux-adg792a: document devicetree bindings for ADG792A/G mux Peter Rosin
@ 2017-05-14 19:51 ` Peter Rosin
  2017-05-14 19:51 ` [PATCH v15 12/13] dt-bindings: add mmio-based syscon mux controller DT bindings Peter Rosin
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 26+ messages in thread
From: Peter Rosin @ 2017-05-14 19:51 UTC (permalink / raw)
  To: linux-kernel, Greg Kroah-Hartman
  Cc: Peter Rosin, Wolfram Sang, Rob Herring, Mark Rutland,
	Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Jonathan Corbet, linux-i2c, devicetree,
	linux-iio, linux-doc, Andrew Morton, Colin Ian King,
	Paul Gortmaker, Philipp Zabel, kernel

From: Peter Rosin <peda@axentia.se>

Analog Devices ADG792A/G is a triple 4:1 mux.

Reviewed-by: Jonathan Cameron <jic23@kernel.org>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/mux/Kconfig       |  12 ++++
 drivers/mux/Makefile      |   1 +
 drivers/mux/mux-adg792a.c | 157 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 170 insertions(+)
 create mode 100644 drivers/mux/mux-adg792a.c

diff --git a/drivers/mux/Kconfig b/drivers/mux/Kconfig
index 738670aaecb7..c4d050645605 100644
--- a/drivers/mux/Kconfig
+++ b/drivers/mux/Kconfig
@@ -17,6 +17,18 @@ menuconfig MULTIPLEXER
 
 if MULTIPLEXER
 
+config MUX_ADG792A
+	tristate "Analog Devices ADG792A/ADG792G Multiplexers"
+	depends on I2C || COMPILE_TEST
+	help
+	  ADG792A and ADG792G Wide Bandwidth Triple 4:1 Multiplexers
+
+	  The driver supports both operating the three multiplexers in
+	  parallel and operating them independently.
+
+	  To compile the driver as a module, choose M here: the module will
+	  be called mux-adg792a.
+
 config MUX_GPIO
 	tristate "GPIO-controlled Multiplexer"
 	depends on GPIOLIB || COMPILE_TEST
diff --git a/drivers/mux/Makefile b/drivers/mux/Makefile
index bb16953f6290..b00a7d37d2fb 100644
--- a/drivers/mux/Makefile
+++ b/drivers/mux/Makefile
@@ -3,4 +3,5 @@
 #
 
 obj-$(CONFIG_MULTIPLEXER)	+= mux-core.o
+obj-$(CONFIG_MUX_ADG792A)	+= mux-adg792a.o
 obj-$(CONFIG_MUX_GPIO)		+= mux-gpio.o
diff --git a/drivers/mux/mux-adg792a.c b/drivers/mux/mux-adg792a.c
new file mode 100644
index 000000000000..12aa221ab90d
--- /dev/null
+++ b/drivers/mux/mux-adg792a.c
@@ -0,0 +1,157 @@
+/*
+ * Multiplexer driver for Analog Devices ADG792A/G Triple 4:1 mux
+ *
+ * Copyright (C) 2017 Axentia Technologies AB
+ *
+ * Author: Peter Rosin <peda@axentia.se>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/mux/driver.h>
+#include <linux/property.h>
+
+#define ADG792A_LDSW		BIT(0)
+#define ADG792A_RESETB		BIT(1)
+#define ADG792A_DISABLE(mux)	(0x50 | (mux))
+#define ADG792A_DISABLE_ALL	(0x5f)
+#define ADG792A_MUX(mux, state)	(0xc0 | (((mux) + 1) << 2) | (state))
+#define ADG792A_MUX_ALL(state)	(0xc0 | (state))
+
+static int adg792a_write_cmd(struct i2c_client *i2c, u8 cmd, int reset)
+{
+	u8 data = ADG792A_RESETB | ADG792A_LDSW;
+
+	/* ADG792A_RESETB is active low, the chip resets when it is zero. */
+	if (reset)
+		data &= ~ADG792A_RESETB;
+
+	return i2c_smbus_write_byte_data(i2c, cmd, data);
+}
+
+static int adg792a_set(struct mux_control *mux, int state)
+{
+	struct i2c_client *i2c = to_i2c_client(mux->chip->dev.parent);
+	u8 cmd;
+
+	if (mux->chip->controllers == 1) {
+		/* parallel mux controller operation */
+		if (state == MUX_IDLE_DISCONNECT)
+			cmd = ADG792A_DISABLE_ALL;
+		else
+			cmd = ADG792A_MUX_ALL(state);
+	} else {
+		unsigned int controller = mux_control_get_index(mux);
+
+		if (state == MUX_IDLE_DISCONNECT)
+			cmd = ADG792A_DISABLE(controller);
+		else
+			cmd = ADG792A_MUX(controller, state);
+	}
+
+	return adg792a_write_cmd(i2c, cmd, 0);
+}
+
+static const struct mux_control_ops adg792a_ops = {
+	.set = adg792a_set,
+};
+
+static int adg792a_probe(struct i2c_client *i2c,
+			 const struct i2c_device_id *id)
+{
+	struct device *dev = &i2c->dev;
+	struct mux_chip *mux_chip;
+	s32 idle_state[3];
+	u32 cells;
+	int ret;
+	int i;
+
+	if (!i2c_check_functionality(i2c->adapter, I2C_FUNC_SMBUS_BYTE_DATA))
+		return -ENODEV;
+
+	ret = device_property_read_u32(dev, "#mux-control-cells", &cells);
+	if (ret < 0)
+		return ret;
+	if (cells >= 2)
+		return -EINVAL;
+
+	mux_chip = devm_mux_chip_alloc(dev, cells ? 3 : 1, 0);
+	if (IS_ERR(mux_chip))
+		return PTR_ERR(mux_chip);
+
+	mux_chip->ops = &adg792a_ops;
+
+	ret = adg792a_write_cmd(i2c, ADG792A_DISABLE_ALL, 1);
+	if (ret < 0)
+		return ret;
+
+	ret = device_property_read_u32_array(dev, "idle-state",
+					     (u32 *)idle_state,
+					     mux_chip->controllers);
+	if (ret < 0) {
+		idle_state[0] = MUX_IDLE_AS_IS;
+		idle_state[1] = MUX_IDLE_AS_IS;
+		idle_state[2] = MUX_IDLE_AS_IS;
+	}
+
+	for (i = 0; i < mux_chip->controllers; ++i) {
+		struct mux_control *mux = &mux_chip->mux[i];
+
+		mux->states = 4;
+
+		switch (idle_state[i]) {
+		case MUX_IDLE_DISCONNECT:
+		case MUX_IDLE_AS_IS:
+		case 0 ... 4:
+			mux->idle_state = idle_state[i];
+			break;
+		default:
+			dev_err(dev, "invalid idle-state %d\n", idle_state[i]);
+			return -EINVAL;
+		}
+	}
+
+	ret = devm_mux_chip_register(dev, mux_chip);
+	if (ret < 0)
+		return ret;
+
+	if (cells)
+		dev_info(dev, "3x single pole quadruple throw muxes registered\n");
+	else
+		dev_info(dev, "triple pole quadruple throw mux registered\n");
+
+	return 0;
+}
+
+static const struct i2c_device_id adg792a_id[] = {
+	{ .name = "adg792a", },
+	{ .name = "adg792g", },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, adg792a_id);
+
+static const struct of_device_id adg792a_of_match[] = {
+	{ .compatible = "adi,adg792a", },
+	{ .compatible = "adi,adg792g", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, adg792a_of_match);
+
+static struct i2c_driver adg792a_driver = {
+	.driver		= {
+		.name		= "adg792a",
+		.of_match_table = of_match_ptr(adg792a_of_match),
+	},
+	.probe		= adg792a_probe,
+	.id_table	= adg792a_id,
+};
+module_i2c_driver(adg792a_driver);
+
+MODULE_DESCRIPTION("Analog Devices ADG792A/G Triple 4:1 mux driver");
+MODULE_AUTHOR("Peter Rosin <peda@axentia.se>");
+MODULE_LICENSE("GPL v2");
-- 
2.1.4

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

* [PATCH v15 12/13] dt-bindings: add mmio-based syscon mux controller DT bindings
  2017-05-14 19:51 [PATCH v15 00/13] mux controller abstraction and iio/i2c muxes Peter Rosin
                   ` (10 preceding siblings ...)
  2017-05-14 19:51 ` [PATCH v15 11/13] mux: adg792a: add mux controller driver for ADG792A/G Peter Rosin
@ 2017-05-14 19:51 ` Peter Rosin
  2017-05-14 19:51 ` [PATCH v15 13/13] mux: mmio-based syscon mux controller Peter Rosin
  2017-06-03 10:26 ` [PATCH v15 00/13] mux controller abstraction and iio/i2c muxes Greg Kroah-Hartman
  13 siblings, 0 replies; 26+ messages in thread
From: Peter Rosin @ 2017-05-14 19:51 UTC (permalink / raw)
  To: linux-kernel, Greg Kroah-Hartman
  Cc: Philipp Zabel, Peter Rosin, Wolfram Sang, Rob Herring,
	Mark Rutland, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Jonathan Corbet,
	linux-i2c, devicetree, linux-iio, linux-doc, Andrew Morton,
	Colin Ian King, Paul Gortmaker, kernel

From: Philipp Zabel <p.zabel@pengutronix.de>

This adds device tree binding documentation for mmio-based syscon
multiplexers controlled by a bitfields in a syscon register range.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
 Documentation/devicetree/bindings/mux/mmio-mux.txt | 60 ++++++++++++++++++++++
 1 file changed, 60 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mux/mmio-mux.txt

diff --git a/Documentation/devicetree/bindings/mux/mmio-mux.txt b/Documentation/devicetree/bindings/mux/mmio-mux.txt
new file mode 100644
index 000000000000..a9bfb4d8b6ac
--- /dev/null
+++ b/Documentation/devicetree/bindings/mux/mmio-mux.txt
@@ -0,0 +1,60 @@
+MMIO register bitfield-based multiplexer controller bindings
+
+Define register bitfields to be used to control multiplexers. The parent
+device tree node must be a syscon node to provide register access.
+
+Required properties:
+- compatible : "mmio-mux"
+- #mux-control-cells : <1>
+- mux-reg-masks : an array of register offset and pre-shifted bitfield mask
+                  pairs, each describing a single mux control.
+* Standard mux-controller bindings as decribed in mux-controller.txt
+
+Optional properties:
+- idle-states : if present, the state the muxes will have when idle. The
+		special state MUX_IDLE_AS_IS is the default.
+
+The multiplexer state of each multiplexer is defined as the value of the
+bitfield described by the corresponding register offset and bitfield mask pair
+in the mux-reg-masks array, accessed through the parent syscon.
+
+Example:
+
+	syscon {
+		compatible = "syscon";
+
+		mux: mux-controller {
+			compatible = "mmio-mux";
+			#mux-control-cells = <1>;
+
+			mux-reg-masks = <0x3 0x30>, /* 0: reg 0x3, bits 5:4 */
+					<0x3 0x40>, /* 1: reg 0x3, bit 6 */
+			idle-states = <MUX_IDLE_AS_IS>, <0>;
+		};
+	};
+
+	video-mux {
+		compatible = "video-mux";
+		mux-controls = <&mux 0>;
+
+		ports {
+			/* inputs 0..3 */
+			port@0 {
+				reg = <0>;
+			};
+			port@1 {
+				reg = <1>;
+			};
+			port@2 {
+				reg = <2>;
+			};
+			port@3 {
+				reg = <3>;
+			};
+
+			/* output */
+			port@4 {
+				reg = <4>;
+			};
+		};
+	};
-- 
2.1.4

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

* [PATCH v15 13/13] mux: mmio-based syscon mux controller
  2017-05-14 19:51 [PATCH v15 00/13] mux controller abstraction and iio/i2c muxes Peter Rosin
                   ` (11 preceding siblings ...)
  2017-05-14 19:51 ` [PATCH v15 12/13] dt-bindings: add mmio-based syscon mux controller DT bindings Peter Rosin
@ 2017-05-14 19:51 ` Peter Rosin
  2017-06-03 10:26 ` [PATCH v15 00/13] mux controller abstraction and iio/i2c muxes Greg Kroah-Hartman
  13 siblings, 0 replies; 26+ messages in thread
From: Peter Rosin @ 2017-05-14 19:51 UTC (permalink / raw)
  To: linux-kernel, Greg Kroah-Hartman
  Cc: Philipp Zabel, Peter Rosin, Wolfram Sang, Rob Herring,
	Mark Rutland, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Jonathan Corbet,
	linux-i2c, devicetree, linux-iio, linux-doc, Andrew Morton,
	Colin Ian King, Paul Gortmaker, kernel

From: Philipp Zabel <p.zabel@pengutronix.de>

This adds a driver for mmio-based syscon multiplexers controlled by
bitfields in a syscon register range.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/mux/Kconfig    |  13 +++++
 drivers/mux/Makefile   |   1 +
 drivers/mux/mux-mmio.c | 141 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 155 insertions(+)
 create mode 100644 drivers/mux/mux-mmio.c

diff --git a/drivers/mux/Kconfig b/drivers/mux/Kconfig
index c4d050645605..e8f1df74644c 100644
--- a/drivers/mux/Kconfig
+++ b/drivers/mux/Kconfig
@@ -43,4 +43,17 @@ config MUX_GPIO
 	  To compile the driver as a module, choose M here: the module will
 	  be called mux-gpio.
 
+config MUX_MMIO
+	tristate "MMIO register bitfield-controlled Multiplexer"
+	depends on (OF && MFD_SYSCON) || COMPILE_TEST
+	help
+	  MMIO register bitfield-controlled Multiplexer controller.
+
+	  The driver builds multiplexer controllers for bitfields in a syscon
+	  register. For N bit wide bitfields, there will be 2^N possible
+	  multiplexer states.
+
+	  To compile the driver as a module, choose M here: the module will
+	  be called mux-mmio.
+
 endif
diff --git a/drivers/mux/Makefile b/drivers/mux/Makefile
index b00a7d37d2fb..6bac5b0fea13 100644
--- a/drivers/mux/Makefile
+++ b/drivers/mux/Makefile
@@ -5,3 +5,4 @@
 obj-$(CONFIG_MULTIPLEXER)	+= mux-core.o
 obj-$(CONFIG_MUX_ADG792A)	+= mux-adg792a.o
 obj-$(CONFIG_MUX_GPIO)		+= mux-gpio.o
+obj-$(CONFIG_MUX_MMIO)		+= mux-mmio.o
diff --git a/drivers/mux/mux-mmio.c b/drivers/mux/mux-mmio.c
new file mode 100644
index 000000000000..37c1de359a70
--- /dev/null
+++ b/drivers/mux/mux-mmio.c
@@ -0,0 +1,141 @@
+/*
+ * MMIO register bitfield-controlled multiplexer driver
+ *
+ * Copyright (C) 2017 Pengutronix, Philipp Zabel <kernel@pengutronix.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/bitops.h>
+#include <linux/err.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/mux/driver.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+
+static int mux_mmio_set(struct mux_control *mux, int state)
+{
+	struct regmap_field **fields = mux_chip_priv(mux->chip);
+
+	return regmap_field_write(fields[mux_control_get_index(mux)], state);
+}
+
+static const struct mux_control_ops mux_mmio_ops = {
+	.set = mux_mmio_set,
+};
+
+static const struct of_device_id mux_mmio_dt_ids[] = {
+	{ .compatible = "mmio-mux", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, mux_mmio_dt_ids);
+
+static int mux_mmio_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct regmap_field **fields;
+	struct mux_chip *mux_chip;
+	struct regmap *regmap;
+	int num_fields;
+	int ret;
+	int i;
+
+	regmap = syscon_node_to_regmap(np->parent);
+	if (IS_ERR(regmap)) {
+		ret = PTR_ERR(regmap);
+		dev_err(dev, "failed to get regmap: %d\n", ret);
+		return ret;
+	}
+
+	ret = of_property_count_u32_elems(np, "mux-reg-masks");
+	if (ret == 0 || ret % 2)
+		ret = -EINVAL;
+	if (ret < 0) {
+		dev_err(dev, "mux-reg-masks property missing or invalid: %d\n",
+			ret);
+		return ret;
+	}
+	num_fields = ret / 2;
+
+	mux_chip = devm_mux_chip_alloc(dev, num_fields, num_fields *
+				       sizeof(*fields));
+	if (IS_ERR(mux_chip))
+		return PTR_ERR(mux_chip);
+
+	fields = mux_chip_priv(mux_chip);
+
+	for (i = 0; i < num_fields; i++) {
+		struct mux_control *mux = &mux_chip->mux[i];
+		struct reg_field field;
+		s32 idle_state = MUX_IDLE_AS_IS;
+		u32 reg, mask;
+		int bits;
+
+		ret = of_property_read_u32_index(np, "mux-reg-masks",
+						 2 * i, &reg);
+		if (!ret)
+			ret = of_property_read_u32_index(np, "mux-reg-masks",
+							 2 * i + 1, &mask);
+		if (ret < 0) {
+			dev_err(dev, "bitfield %d: failed to read mux-reg-masks property: %d\n",
+				i, ret);
+			return ret;
+		}
+
+		field.reg = reg;
+		field.msb = fls(mask) - 1;
+		field.lsb = ffs(mask) - 1;
+
+		if (mask != GENMASK(field.msb, field.lsb)) {
+			dev_err(dev, "bitfield %d: invalid mask 0x%x\n",
+				i, mask);
+			return -EINVAL;
+		}
+
+		fields[i] = devm_regmap_field_alloc(dev, regmap, field);
+		if (IS_ERR(fields[i])) {
+			ret = PTR_ERR(fields[i]);
+			dev_err(dev, "bitfield %d: failed allocate: %d\n",
+				i, ret);
+			return ret;
+		}
+
+		bits = 1 + field.msb - field.lsb;
+		mux->states = 1 << bits;
+
+		of_property_read_u32_index(np, "idle-states", i,
+					   (u32 *)&idle_state);
+		if (idle_state != MUX_IDLE_AS_IS) {
+			if (idle_state < 0 || idle_state >= mux->states) {
+				dev_err(dev, "bitfield: %d: out of range idle state %d\n",
+					i, idle_state);
+				return -EINVAL;
+			}
+
+			mux->idle_state = idle_state;
+		}
+	}
+
+	mux_chip->ops = &mux_mmio_ops;
+
+	return devm_mux_chip_register(dev, mux_chip);
+}
+
+static struct platform_driver mux_mmio_driver = {
+	.driver = {
+		.name = "mmio-mux",
+		.of_match_table	= of_match_ptr(mux_mmio_dt_ids),
+	},
+	.probe = mux_mmio_probe,
+};
+module_platform_driver(mux_mmio_driver);
+
+MODULE_DESCRIPTION("MMIO register bitfield-controlled multiplexer driver");
+MODULE_AUTHOR("Philipp Zabel <p.zabel@pengutronix.de>");
+MODULE_LICENSE("GPL v2");
-- 
2.1.4

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

* Re: [PATCH v15 03/13] mux: minimal mux subsystem
  2017-05-14 19:51 ` [PATCH v15 03/13] mux: minimal mux subsystem Peter Rosin
@ 2017-05-17  9:38   ` Philipp Zabel
  0 siblings, 0 replies; 26+ messages in thread
From: Philipp Zabel @ 2017-05-17  9:38 UTC (permalink / raw)
  To: Peter Rosin
  Cc: linux-kernel, Greg Kroah-Hartman, Peter Rosin, Wolfram Sang,
	Rob Herring, Mark Rutland, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Jonathan Corbet,
	linux-i2c, devicetree, linux-iio, linux-doc, Andrew Morton,
	Colin Ian King, Paul Gortmaker, kernel

On Sun, 2017-05-14 at 21:51 +0200, Peter Rosin wrote:
> From: Peter Rosin <peda@axentia.se>
> 
> Add a new minimalistic subsystem that handles multiplexer controllers.
> When multiplexers are used in various places in the kernel, and the
> same multiplexer controller can be used for several independent things,
> there should be one place to implement support for said multiplexer
> controller.
> 
> A single multiplexer controller can also be used to control several
> parallel multiplexers, that are in turn used by different subsystems
> in the kernel, leading to a need to coordinate multiplexer accesses.
> The multiplexer subsystem handles this coordination.
> 
> Thanks go out to Lars-Peter Clausen, Jonathan Cameron, Rob Herring,
> Wolfram Sang, Paul Gortmaker, Dan Carpenter, Colin Ian King, Greg
> Kroah-Hartman and last but certainly not least to Philipp Zabel for
> helpful comments, reviews, patches and general encouragement!
> 
> Reviewed-by: Jonathan Cameron <jic23@kernel.org>
> Signed-off-by: Peter Rosin <peda@axentia.se>

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
(using video-mux [1] on i.MX6)

[1] https://patchwork.kernel.org/patch/9712131/

regards
Philipp

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

* Re: [PATCH v15 04/13] mux: gpio: add mux controller driver for gpio based multiplexers
  2017-05-14 19:51 ` [PATCH v15 04/13] mux: gpio: add mux controller driver for gpio based multiplexers Peter Rosin
@ 2017-05-17  9:38   ` Philipp Zabel
  0 siblings, 0 replies; 26+ messages in thread
From: Philipp Zabel @ 2017-05-17  9:38 UTC (permalink / raw)
  To: Peter Rosin
  Cc: linux-kernel, Greg Kroah-Hartman, Peter Rosin, Wolfram Sang,
	Rob Herring, Mark Rutland, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Jonathan Corbet,
	linux-i2c, devicetree, linux-iio, linux-doc, Andrew Morton,
	Colin Ian King, Paul Gortmaker, kernel

On Sun, 2017-05-14 at 21:51 +0200, Peter Rosin wrote:
> From: Peter Rosin <peda@axentia.se>
> 
> The driver builds a single multiplexer controller using a number
> of gpio pins. For N pins, there will be 2^N possible multiplexer
> states. The GPIO pins can be connected (by the hardware) to several
> multiplexers, which in that case will be operated in parallel.
> 
> Reviewed-by: Jonathan Cameron <jic23@kernel.org>
> Signed-off-by: Peter Rosin <peda@axentia.se>

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>

regards
Philipp

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

* Re: [PATCH v15 00/13] mux controller abstraction and iio/i2c muxes
  2017-05-14 19:51 [PATCH v15 00/13] mux controller abstraction and iio/i2c muxes Peter Rosin
                   ` (12 preceding siblings ...)
  2017-05-14 19:51 ` [PATCH v15 13/13] mux: mmio-based syscon mux controller Peter Rosin
@ 2017-06-03 10:26 ` Greg Kroah-Hartman
  2017-06-03 10:31   ` Greg Kroah-Hartman
  2017-06-03 18:37   ` Luc Van Oostenryck
  13 siblings, 2 replies; 26+ messages in thread
From: Greg Kroah-Hartman @ 2017-06-03 10:26 UTC (permalink / raw)
  To: Peter Rosin
  Cc: linux-kernel, Peter Rosin, Wolfram Sang, Rob Herring,
	Mark Rutland, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Jonathan Corbet,
	linux-i2c, devicetree, linux-iio, linux-doc, Andrew Morton,
	Colin Ian King, Paul Gortmaker, Philipp Zabel, kernel

On Sun, May 14, 2017 at 09:51:03PM +0200, Peter Rosin wrote:
> From: Peter Rosin <peda@axentia.se>
> 
> Hi Greg,
> 
> Philipp found problems in v14 with using a mutex for locking that was
> the outcome of the review for v13, so I'm now using a semaphore instead
> of the rwsem that was in v13. That at least got rid of the scary call
> to downgrade_write. However, I'm still unsure about what you actually
> meant with your comment about lack of sparse markings [1]. I did add
> __must_check to the funcs that selects the mux, but I've got this
> feeling that this is not what you meant?

I thought there was a way to mark a function as requiring a lock be held
when it is being called.  Does sparse not support that anymore?

thanks,

greg k-h

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

* Re: [PATCH v15 00/13] mux controller abstraction and iio/i2c muxes
  2017-06-03 10:26 ` [PATCH v15 00/13] mux controller abstraction and iio/i2c muxes Greg Kroah-Hartman
@ 2017-06-03 10:31   ` Greg Kroah-Hartman
  2017-06-03 21:37     ` Peter Rosin
  2017-06-03 18:37   ` Luc Van Oostenryck
  1 sibling, 1 reply; 26+ messages in thread
From: Greg Kroah-Hartman @ 2017-06-03 10:31 UTC (permalink / raw)
  To: Peter Rosin
  Cc: linux-kernel, Peter Rosin, Wolfram Sang, Rob Herring,
	Mark Rutland, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Jonathan Corbet,
	linux-i2c, devicetree, linux-iio, linux-doc, Andrew Morton,
	Colin Ian King, Paul Gortmaker, Philipp Zabel, kernel

On Sat, Jun 03, 2017 at 07:26:27PM +0900, Greg Kroah-Hartman wrote:
> On Sun, May 14, 2017 at 09:51:03PM +0200, Peter Rosin wrote:
> > From: Peter Rosin <peda@axentia.se>
> > 
> > Hi Greg,
> > 
> > Philipp found problems in v14 with using a mutex for locking that was
> > the outcome of the review for v13, so I'm now using a semaphore instead
> > of the rwsem that was in v13. That at least got rid of the scary call
> > to downgrade_write. However, I'm still unsure about what you actually
> > meant with your comment about lack of sparse markings [1]. I did add
> > __must_check to the funcs that selects the mux, but I've got this
> > feeling that this is not what you meant?
> 
> I thought there was a way to mark a function as requiring a lock be held
> when it is being called.  Does sparse not support that anymore?

Anyway, not a big deal.  I still worry about the calls blocking when
people are not expecting them to, but it is just the nature of th api I
guess.

All now queued up, nice work, thanks for sticking with this.

greg k-h

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

* Re: [PATCH v15 00/13] mux controller abstraction and iio/i2c muxes
  2017-06-03 10:26 ` [PATCH v15 00/13] mux controller abstraction and iio/i2c muxes Greg Kroah-Hartman
  2017-06-03 10:31   ` Greg Kroah-Hartman
@ 2017-06-03 18:37   ` Luc Van Oostenryck
  2017-06-03 19:34     ` Greg Kroah-Hartman
  1 sibling, 1 reply; 26+ messages in thread
From: Luc Van Oostenryck @ 2017-06-03 18:37 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Peter Rosin, open list, Peter Rosin, Wolfram Sang, Rob Herring,
	Mark Rutland, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Jonathan Corbet,
	linux-i2c, devicetree, linux-iio, linux-doc, Andrew Morton,
	Colin Ian King, Paul Gortmaker, Philipp Zabel, kernel

On Sat, Jun 3, 2017 at 12:26 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Sun, May 14, 2017 at 09:51:03PM +0200, Peter Rosin wrote:
>> From: Peter Rosin <peda@axentia.se>
>>
>> Hi Greg,
>>
>> Philipp found problems in v14 with using a mutex for locking that was
>> the outcome of the review for v13, so I'm now using a semaphore instead
>> of the rwsem that was in v13. That at least got rid of the scary call
>> to downgrade_write. However, I'm still unsure about what you actually
>> meant with your comment about lack of sparse markings [1]. I did add
>> __must_check to the funcs that selects the mux, but I've got this
>> feeling that this is not what you meant?
>
> I thought there was a way to mark a function as requiring a lock be held
> when it is being called.  Does sparse not support that anymore?

sparse still support these annotations, of course.
In this case, I suppose you're talking about '__must_hold()' which
*must* be used instead of a pair of '__releases()' + '__acquires()'
when the lock is help on function entry and exit.

Cheers,
-- Luc Van Oostenryck

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

* Re: [PATCH v15 00/13] mux controller abstraction and iio/i2c muxes
  2017-06-03 18:37   ` Luc Van Oostenryck
@ 2017-06-03 19:34     ` Greg Kroah-Hartman
  2017-06-03 20:26       ` Luc Van Oostenryck
  0 siblings, 1 reply; 26+ messages in thread
From: Greg Kroah-Hartman @ 2017-06-03 19:34 UTC (permalink / raw)
  To: Luc Van Oostenryck
  Cc: Peter Rosin, open list, Peter Rosin, Wolfram Sang, Rob Herring,
	Mark Rutland, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Jonathan Corbet,
	linux-i2c, devicetree, linux-iio, linux-doc, Andrew Morton,
	Colin Ian King, Paul Gortmaker, Philipp Zabel, kernel

On Sat, Jun 03, 2017 at 08:37:21PM +0200, Luc Van Oostenryck wrote:
> On Sat, Jun 3, 2017 at 12:26 PM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> > On Sun, May 14, 2017 at 09:51:03PM +0200, Peter Rosin wrote:
> >> From: Peter Rosin <peda@axentia.se>
> >>
> >> Hi Greg,
> >>
> >> Philipp found problems in v14 with using a mutex for locking that was
> >> the outcome of the review for v13, so I'm now using a semaphore instead
> >> of the rwsem that was in v13. That at least got rid of the scary call
> >> to downgrade_write. However, I'm still unsure about what you actually
> >> meant with your comment about lack of sparse markings [1]. I did add
> >> __must_check to the funcs that selects the mux, but I've got this
> >> feeling that this is not what you meant?
> >
> > I thought there was a way to mark a function as requiring a lock be held
> > when it is being called.  Does sparse not support that anymore?
> 
> sparse still support these annotations, of course.
> In this case, I suppose you're talking about '__must_hold()' which
> *must* be used instead of a pair of '__releases()' + '__acquires()'
> when the lock is help on function entry and exit.

Ah, yes, that's what I was thinking of.  I don't know if sparse can
track things like this across an exported symbol, so I doubt it really
will help here.  Sorry for the noise.

thanks,

greg k-h

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

* Re: [PATCH v15 00/13] mux controller abstraction and iio/i2c muxes
  2017-06-03 19:34     ` Greg Kroah-Hartman
@ 2017-06-03 20:26       ` Luc Van Oostenryck
  2017-06-03 21:29         ` Peter Rosin
  0 siblings, 1 reply; 26+ messages in thread
From: Luc Van Oostenryck @ 2017-06-03 20:26 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Peter Rosin, open list, Peter Rosin, Wolfram Sang, Rob Herring,
	Mark Rutland, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Jonathan Corbet,
	linux-i2c, devicetree, linux-iio, linux-doc, Andrew Morton,
	Colin Ian King, Paul Gortmaker, Philipp Zabel, kernel

On Sat, Jun 3, 2017 at 9:34 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Sat, Jun 03, 2017 at 08:37:21PM +0200, Luc Van Oostenryck wrote:
>> On Sat, Jun 3, 2017 at 12:26 PM, Greg Kroah-Hartman
>> <gregkh@linuxfoundation.org> wrote:
>> > On Sun, May 14, 2017 at 09:51:03PM +0200, Peter Rosin wrote:
>> >> From: Peter Rosin <peda@axentia.se>
>> >>
>> >> Hi Greg,
>> >>
>> >> Philipp found problems in v14 with using a mutex for locking that was
>> >> the outcome of the review for v13, so I'm now using a semaphore instead
>> >> of the rwsem that was in v13. That at least got rid of the scary call
>> >> to downgrade_write. However, I'm still unsure about what you actually
>> >> meant with your comment about lack of sparse markings [1]. I did add
>> >> __must_check to the funcs that selects the mux, but I've got this
>> >> feeling that this is not what you meant?
>> >
>> > I thought there was a way to mark a function as requiring a lock be held
>> > when it is being called.  Does sparse not support that anymore?
>>
>> sparse still support these annotations, of course.
>> In this case, I suppose you're talking about '__must_hold()' which
>> *must* be used instead of a pair of '__releases()' + '__acquires()'
>> when the lock is help on function entry and exit.
>
> Ah, yes, that's what I was thinking of.  I don't know if sparse can
> track things like this across an exported symbol, so I doubt it really
> will help here.  Sorry for the noise.

No problem, I'm glad to help for sparse related things.

I didn't saw the code in question because the lkml.org link Peter
gave didn't work for me and I don't know much about exported symbols
(but I think the sole effect is to add some data in some symbol table).
But these annotations just work based on the declarations, very much
like type checking. So if you have something in scope like the following:

void do_stuff_locked(struct s *ptr) __must_hold(*ptr);

...

void do_stuff_unlocked(struct s *ptr)
{
        ...
        do_stuff_locked(ptr);        // will warn
        ...
}

You will have a warning from sparse unless the code preceding and following
the call to do_stuff_locked() lock & then unlock 'ptr', generaly
indirectly by a pair
of functions, the one before with an '__acquires()' in its declaration
the one after
with a '__releases()' in its declaration:

void lock_stuff(struct s *ptr) __acquires(*ptr);
void unlock_stuff(struct s *ptr) __releases(*ptr);

void do_stuff_unlocked(struct s *ptr)
{
        lock_stuff(ptr);
        do_stuff_locked(ptr);        // won't warn
        unlock_stuff(ptr);
}


Regards,
-- Luc

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

* Re: [PATCH v15 00/13] mux controller abstraction and iio/i2c muxes
  2017-06-03 20:26       ` Luc Van Oostenryck
@ 2017-06-03 21:29         ` Peter Rosin
  2017-06-03 22:21             ` Luc Van Oostenryck
  0 siblings, 1 reply; 26+ messages in thread
From: Peter Rosin @ 2017-06-03 21:29 UTC (permalink / raw)
  To: Luc Van Oostenryck, Greg Kroah-Hartman
  Cc: Peter Rosin, open list, Wolfram Sang, Rob Herring, Mark Rutland,
	Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Jonathan Corbet, linux-i2c, devicetree,
	linux-iio, linux-doc, Andrew Morton, Colin Ian King,
	Paul Gortmaker, Philipp Zabel, kernel

On 2017-06-03 22:26, Luc Van Oostenryck wrote:
> On Sat, Jun 3, 2017 at 9:34 PM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
>> On Sat, Jun 03, 2017 at 08:37:21PM +0200, Luc Van Oostenryck wrote:
>>> On Sat, Jun 3, 2017 at 12:26 PM, Greg Kroah-Hartman
>>> <gregkh@linuxfoundation.org> wrote:
>>>> On Sun, May 14, 2017 at 09:51:03PM +0200, Peter Rosin wrote:
>>>>> From: Peter Rosin <peda@axentia.se>
>>>>>
>>>>> Hi Greg,
>>>>>
>>>>> Philipp found problems in v14 with using a mutex for locking that was
>>>>> the outcome of the review for v13, so I'm now using a semaphore instead
>>>>> of the rwsem that was in v13. That at least got rid of the scary call
>>>>> to downgrade_write. However, I'm still unsure about what you actually
>>>>> meant with your comment about lack of sparse markings [1]. I did add
>>>>> __must_check to the funcs that selects the mux, but I've got this
>>>>> feeling that this is not what you meant?
>>>>
>>>> I thought there was a way to mark a function as requiring a lock be held
>>>> when it is being called.  Does sparse not support that anymore?
>>>
>>> sparse still support these annotations, of course.
>>> In this case, I suppose you're talking about '__must_hold()' which
>>> *must* be used instead of a pair of '__releases()' + '__acquires()'
>>> when the lock is help on function entry and exit.
>>
>> Ah, yes, that's what I was thinking of.  I don't know if sparse can
>> track things like this across an exported symbol, so I doubt it really
>> will help here.  Sorry for the noise.
> 
> No problem, I'm glad to help for sparse related things.
> 
> I didn't saw the code in question because the lkml.org link Peter
> gave didn't work for me and I don't know much about exported symbols
> (but I think the sole effect is to add some data in some symbol table).
> But these annotations just work based on the declarations, very much
> like type checking. So if you have something in scope like the following:
> 
> void do_stuff_locked(struct s *ptr) __must_hold(*ptr);
> 
> ...
> 
> void do_stuff_unlocked(struct s *ptr)
> {
>         ...
>         do_stuff_locked(ptr);        // will warn
>         ...
> }
> 
> You will have a warning from sparse unless the code preceding and following
> the call to do_stuff_locked() lock & then unlock 'ptr', generaly
> indirectly by a pair
> of functions, the one before with an '__acquires()' in its declaration
> the one after
> with a '__releases()' in its declaration:
> 
> void lock_stuff(struct s *ptr) __acquires(*ptr);
> void unlock_stuff(struct s *ptr) __releases(*ptr);
> 
> void do_stuff_unlocked(struct s *ptr)
> {
>         lock_stuff(ptr);
>         do_stuff_locked(ptr);        // won't warn
>         unlock_stuff(ptr);
> }

Ok, thanks for the explanation! The above was what I gathered when I
looked around, and since it didn't really fit the usage pattern of the
mux api I was stomped. When comparing the mux code with the above,
mux_control_select would be an __acquires (albeit a conditional one,
but let's not muddy the waters unnecessarily) and mux_control_deselect
would be a __releases.

But for long time mux consumers, like the video mux, it must be OK to
only acquire the mux, and not release it right away in the same context,
which I assume will be very hard for sparse to handle sanely? E.g. I
think sparse also complains if there are unbalanced __acquires and
__releases in some context, no?

Cheers,
peda

BTW, the core mux code is at the below link if the lkml link continues
to fail:
https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git/commit/?h=char-misc-testing&id=a3b02a9c6591ce154cd44e2383406390a45b530c

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

* Re: [PATCH v15 00/13] mux controller abstraction and iio/i2c muxes
  2017-06-03 10:31   ` Greg Kroah-Hartman
@ 2017-06-03 21:37     ` Peter Rosin
  2017-06-04  7:46       ` Wolfram Sang
  0 siblings, 1 reply; 26+ messages in thread
From: Peter Rosin @ 2017-06-03 21:37 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Peter Rosin
  Cc: linux-kernel, Wolfram Sang, Rob Herring, Mark Rutland,
	Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Jonathan Corbet, linux-i2c, devicetree,
	linux-iio, linux-doc, Andrew Morton, Colin Ian King,
	Paul Gortmaker, Philipp Zabel, kernel

On 2017-06-03 12:31, Greg Kroah-Hartman wrote:
> On Sat, Jun 03, 2017 at 07:26:27PM +0900, Greg Kroah-Hartman wrote:
>> On Sun, May 14, 2017 at 09:51:03PM +0200, Peter Rosin wrote:
>>> From: Peter Rosin <peda@axentia.se>
>>>
>>> Hi Greg,
>>>
>>> Philipp found problems in v14 with using a mutex for locking that was
>>> the outcome of the review for v13, so I'm now using a semaphore instead
>>> of the rwsem that was in v13. That at least got rid of the scary call
>>> to downgrade_write. However, I'm still unsure about what you actually
>>> meant with your comment about lack of sparse markings [1]. I did add
>>> __must_check to the funcs that selects the mux, but I've got this
>>> feeling that this is not what you meant?
>>
>> I thought there was a way to mark a function as requiring a lock be held
>> when it is being called.  Does sparse not support that anymore?
> 
> Anyway, not a big deal.  I still worry about the calls blocking when
> people are not expecting them to, but it is just the nature of th api I
> guess.

Yeah, first come first serve. I don't know what else I can do, except maybe
follow up with a timed version of mux_control_select()...

> All now queued up, nice work, thanks for sticking with this.

*big sigh of relief*

I was getting pretty fed up with the series to be honest :-), so thanks
a bunch for taking it!

Cheers,
peda

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

* Re: [PATCH v15 00/13] mux controller abstraction and iio/i2c muxes
@ 2017-06-03 22:21             ` Luc Van Oostenryck
  0 siblings, 0 replies; 26+ messages in thread
From: Luc Van Oostenryck @ 2017-06-03 22:21 UTC (permalink / raw)
  To: Peter Rosin
  Cc: Greg Kroah-Hartman, Peter Rosin, open list, Wolfram Sang,
	Rob Herring, Mark Rutland, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Jonathan Corbet,
	linux-i2c, devicetree, linux-iio, linux-doc, Andrew Morton,
	Colin Ian King, Paul Gortmaker, Philipp Zabel, kernel

On Sat, Jun 3, 2017 at 11:29 PM, Peter Rosin <peda@axentia.se> wrote:
> On 2017-06-03 22:26, Luc Van Oostenryck wrote:
>> On Sat, Jun 3, 2017 at 9:34 PM, Greg Kroah-Hartman
>> <gregkh@linuxfoundation.org> wrote:
>>> On Sat, Jun 03, 2017 at 08:37:21PM +0200, Luc Van Oostenryck wrote:
>>>> On Sat, Jun 3, 2017 at 12:26 PM, Greg Kroah-Hartman
>>>> <gregkh@linuxfoundation.org> wrote:
>>>>> On Sun, May 14, 2017 at 09:51:03PM +0200, Peter Rosin wrote:
>>>>>> From: Peter Rosin <peda@axentia.se>
>>>>>>
>>>>>> Hi Greg,
>>>>>>
>>>>>> Philipp found problems in v14 with using a mutex for locking that was
>>>>>> the outcome of the review for v13, so I'm now using a semaphore instead
>>>>>> of the rwsem that was in v13. That at least got rid of the scary call
>>>>>> to downgrade_write. However, I'm still unsure about what you actually
>>>>>> meant with your comment about lack of sparse markings [1]. I did add
>>>>>> __must_check to the funcs that selects the mux, but I've got this
>>>>>> feeling that this is not what you meant?
>>>>>
>>>>> I thought there was a way to mark a function as requiring a lock be held
>>>>> when it is being called.  Does sparse not support that anymore?
>>>>
>>>> sparse still support these annotations, of course.
>>>> In this case, I suppose you're talking about '__must_hold()' which
>>>> *must* be used instead of a pair of '__releases()' + '__acquires()'
>>>> when the lock is help on function entry and exit.
>>>
>>> Ah, yes, that's what I was thinking of.  I don't know if sparse can
>>> track things like this across an exported symbol, so I doubt it really
>>> will help here.  Sorry for the noise.
>>
>> No problem, I'm glad to help for sparse related things.
>>
>> I didn't saw the code in question because the lkml.org link Peter
>> gave didn't work for me and I don't know much about exported symbols
>> (but I think the sole effect is to add some data in some symbol table).
>> But these annotations just work based on the declarations, very much
>> like type checking. So if you have something in scope like the following:
>>
>> void do_stuff_locked(struct s *ptr) __must_hold(*ptr);
>>
>> ...
>>
>> void do_stuff_unlocked(struct s *ptr)
>> {
>>         ...
>>         do_stuff_locked(ptr);        // will warn
>>         ...
>> }
>>
>> You will have a warning from sparse unless the code preceding and following
>> the call to do_stuff_locked() lock & then unlock 'ptr', generaly
>> indirectly by a pair
>> of functions, the one before with an '__acquires()' in its declaration
>> the one after
>> with a '__releases()' in its declaration:
>>
>> void lock_stuff(struct s *ptr) __acquires(*ptr);
>> void unlock_stuff(struct s *ptr) __releases(*ptr);
>>
>> void do_stuff_unlocked(struct s *ptr)
>> {
>>         lock_stuff(ptr);
>>         do_stuff_locked(ptr);        // won't warn
>>         unlock_stuff(ptr);
>> }
>
> Ok, thanks for the explanation! The above was what I gathered when I
> looked around, and since it didn't really fit the usage pattern of the
> mux api I was stomped. When comparing the mux code with the above,
> mux_control_select would be an __acquires (albeit a conditional one,
> but let's not muddy the waters unnecessarily) and mux_control_deselect
> would be a __releases.
>
> But for long time mux consumers, like the video mux, it must be OK to
> only acquire the mux, and not release it right away in the same context,
> which I assume will be very hard for sparse to handle sanely? E.g. I
> think sparse also complains if there are unbalanced __acquires and
> __releases in some context, no?

Yes, there are some limitations.

You can do conditional locks, it's what spin_trylock() and
{read,write}_trylock()
do. something like the following will be OK:

void do_stuff_unlocked(struct s *ptr)
{
         if (trylock_stuff(ptr)) {
                  do_stuff_locked(ptr);        // won't warn
                  unlock_stuff(ptr);
         }
}

sparse won't complain because everything is correct within each block.
But something that sparse will complain about would be:

void foobar(struct s *ptr)
{
         if (some condition)
                  lock_stuff(ptr);
                                                           // will warn
         ... some code ...

         if (same condition) {
                  do_stuff_locked(ptr);
                  unlock_stuff(ptr);
         }
}

While correct from the point of view of locking, sparse won't be able
to take into account
that indeed, the two ifs are fro the same condition. sparse will only
see that after the first
if there will be one path where the lock will be held and another one
where it won't and
will this issue a 'different lock contexts for basic block' warning.

For this mux code, for example, the function mux_control_select() will
have a problem
because depending on the return value of __mux_control_select() you will want to
take back the lock or not, so the exit state of this function is not
well defined regarding
locking. It's the same for mux_control_try_select().

I hope this render the possibilities a bit clearer.
Cheers,
-- Luc

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

* Re: [PATCH v15 00/13] mux controller abstraction and iio/i2c muxes
@ 2017-06-03 22:21             ` Luc Van Oostenryck
  0 siblings, 0 replies; 26+ messages in thread
From: Luc Van Oostenryck @ 2017-06-03 22:21 UTC (permalink / raw)
  To: Peter Rosin
  Cc: Greg Kroah-Hartman, Peter Rosin, open list, Wolfram Sang,
	Rob Herring, Mark Rutland, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Jonathan Corbet,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	linux-doc-u79uwXL29TY76Z2rM5mHXA, Andrew Morton, Colin Ian King,
	Paul Gortmaker, Philipp Zabel, kernel-bIcnvbaLZ9MEGnE8C9+IrQ

On Sat, Jun 3, 2017 at 11:29 PM, Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org> wrote:
> On 2017-06-03 22:26, Luc Van Oostenryck wrote:
>> On Sat, Jun 3, 2017 at 9:34 PM, Greg Kroah-Hartman
>> <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org> wrote:
>>> On Sat, Jun 03, 2017 at 08:37:21PM +0200, Luc Van Oostenryck wrote:
>>>> On Sat, Jun 3, 2017 at 12:26 PM, Greg Kroah-Hartman
>>>> <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org> wrote:
>>>>> On Sun, May 14, 2017 at 09:51:03PM +0200, Peter Rosin wrote:
>>>>>> From: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
>>>>>>
>>>>>> Hi Greg,
>>>>>>
>>>>>> Philipp found problems in v14 with using a mutex for locking that was
>>>>>> the outcome of the review for v13, so I'm now using a semaphore instead
>>>>>> of the rwsem that was in v13. That at least got rid of the scary call
>>>>>> to downgrade_write. However, I'm still unsure about what you actually
>>>>>> meant with your comment about lack of sparse markings [1]. I did add
>>>>>> __must_check to the funcs that selects the mux, but I've got this
>>>>>> feeling that this is not what you meant?
>>>>>
>>>>> I thought there was a way to mark a function as requiring a lock be held
>>>>> when it is being called.  Does sparse not support that anymore?
>>>>
>>>> sparse still support these annotations, of course.
>>>> In this case, I suppose you're talking about '__must_hold()' which
>>>> *must* be used instead of a pair of '__releases()' + '__acquires()'
>>>> when the lock is help on function entry and exit.
>>>
>>> Ah, yes, that's what I was thinking of.  I don't know if sparse can
>>> track things like this across an exported symbol, so I doubt it really
>>> will help here.  Sorry for the noise.
>>
>> No problem, I'm glad to help for sparse related things.
>>
>> I didn't saw the code in question because the lkml.org link Peter
>> gave didn't work for me and I don't know much about exported symbols
>> (but I think the sole effect is to add some data in some symbol table).
>> But these annotations just work based on the declarations, very much
>> like type checking. So if you have something in scope like the following:
>>
>> void do_stuff_locked(struct s *ptr) __must_hold(*ptr);
>>
>> ...
>>
>> void do_stuff_unlocked(struct s *ptr)
>> {
>>         ...
>>         do_stuff_locked(ptr);        // will warn
>>         ...
>> }
>>
>> You will have a warning from sparse unless the code preceding and following
>> the call to do_stuff_locked() lock & then unlock 'ptr', generaly
>> indirectly by a pair
>> of functions, the one before with an '__acquires()' in its declaration
>> the one after
>> with a '__releases()' in its declaration:
>>
>> void lock_stuff(struct s *ptr) __acquires(*ptr);
>> void unlock_stuff(struct s *ptr) __releases(*ptr);
>>
>> void do_stuff_unlocked(struct s *ptr)
>> {
>>         lock_stuff(ptr);
>>         do_stuff_locked(ptr);        // won't warn
>>         unlock_stuff(ptr);
>> }
>
> Ok, thanks for the explanation! The above was what I gathered when I
> looked around, and since it didn't really fit the usage pattern of the
> mux api I was stomped. When comparing the mux code with the above,
> mux_control_select would be an __acquires (albeit a conditional one,
> but let's not muddy the waters unnecessarily) and mux_control_deselect
> would be a __releases.
>
> But for long time mux consumers, like the video mux, it must be OK to
> only acquire the mux, and not release it right away in the same context,
> which I assume will be very hard for sparse to handle sanely? E.g. I
> think sparse also complains if there are unbalanced __acquires and
> __releases in some context, no?

Yes, there are some limitations.

You can do conditional locks, it's what spin_trylock() and
{read,write}_trylock()
do. something like the following will be OK:

void do_stuff_unlocked(struct s *ptr)
{
         if (trylock_stuff(ptr)) {
                  do_stuff_locked(ptr);        // won't warn
                  unlock_stuff(ptr);
         }
}

sparse won't complain because everything is correct within each block.
But something that sparse will complain about would be:

void foobar(struct s *ptr)
{
         if (some condition)
                  lock_stuff(ptr);
                                                           // will warn
         ... some code ...

         if (same condition) {
                  do_stuff_locked(ptr);
                  unlock_stuff(ptr);
         }
}

While correct from the point of view of locking, sparse won't be able
to take into account
that indeed, the two ifs are fro the same condition. sparse will only
see that after the first
if there will be one path where the lock will be held and another one
where it won't and
will this issue a 'different lock contexts for basic block' warning.

For this mux code, for example, the function mux_control_select() will
have a problem
because depending on the return value of __mux_control_select() you will want to
take back the lock or not, so the exit state of this function is not
well defined regarding
locking. It's the same for mux_control_try_select().

I hope this render the possibilities a bit clearer.
Cheers,
-- Luc

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

* Re: [PATCH v15 00/13] mux controller abstraction and iio/i2c muxes
  2017-06-03 21:37     ` Peter Rosin
@ 2017-06-04  7:46       ` Wolfram Sang
  0 siblings, 0 replies; 26+ messages in thread
From: Wolfram Sang @ 2017-06-04  7:46 UTC (permalink / raw)
  To: Peter Rosin
  Cc: Greg Kroah-Hartman, Peter Rosin, linux-kernel, Rob Herring,
	Mark Rutland, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Jonathan Corbet,
	linux-i2c, devicetree, linux-iio, linux-doc, Andrew Morton,
	Colin Ian King, Paul Gortmaker, Philipp Zabel, kernel

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


> > All now queued up, nice work, thanks for sticking with this.
> 
> *big sigh of relief*

I can imagine. Great work, Peda! Congrats.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

end of thread, other threads:[~2017-06-04  7:46 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-14 19:51 [PATCH v15 00/13] mux controller abstraction and iio/i2c muxes Peter Rosin
2017-05-14 19:51 ` [PATCH v15 01/13] devres: trivial whitespace fix Peter Rosin
2017-05-14 19:51 ` [PATCH v15 02/13] dt-bindings: document devicetree bindings for mux-controllers and gpio-mux Peter Rosin
2017-05-14 19:51 ` [PATCH v15 03/13] mux: minimal mux subsystem Peter Rosin
2017-05-17  9:38   ` Philipp Zabel
2017-05-14 19:51 ` [PATCH v15 04/13] mux: gpio: add mux controller driver for gpio based multiplexers Peter Rosin
2017-05-17  9:38   ` Philipp Zabel
2017-05-14 19:51 ` [PATCH v15 05/13] iio: inkern: api for manipulating ext_info of iio channels Peter Rosin
2017-05-14 19:51 ` [PATCH v15 06/13] dt-bindings: iio: io-channel-mux: document io-channel-mux bindings Peter Rosin
2017-05-14 19:51 ` [PATCH v15 07/13] iio: multiplexer: new iio category and iio-mux driver Peter Rosin
2017-05-14 19:51 ` [PATCH v15 08/13] dt-bindings: i2c: i2c-mux: document general purpose i2c-mux bindings Peter Rosin
2017-05-14 19:51 ` [PATCH v15 09/13] i2c: i2c-mux-gpmux: new driver Peter Rosin
2017-05-14 19:51 ` [PATCH v15 10/13] dt-bindings: mux-adg792a: document devicetree bindings for ADG792A/G mux Peter Rosin
2017-05-14 19:51 ` [PATCH v15 11/13] mux: adg792a: add mux controller driver for ADG792A/G Peter Rosin
2017-05-14 19:51 ` [PATCH v15 12/13] dt-bindings: add mmio-based syscon mux controller DT bindings Peter Rosin
2017-05-14 19:51 ` [PATCH v15 13/13] mux: mmio-based syscon mux controller Peter Rosin
2017-06-03 10:26 ` [PATCH v15 00/13] mux controller abstraction and iio/i2c muxes Greg Kroah-Hartman
2017-06-03 10:31   ` Greg Kroah-Hartman
2017-06-03 21:37     ` Peter Rosin
2017-06-04  7:46       ` Wolfram Sang
2017-06-03 18:37   ` Luc Van Oostenryck
2017-06-03 19:34     ` Greg Kroah-Hartman
2017-06-03 20:26       ` Luc Van Oostenryck
2017-06-03 21:29         ` Peter Rosin
2017-06-03 22:21           ` Luc Van Oostenryck
2017-06-03 22:21             ` Luc Van Oostenryck

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.