* Re: [PATCH v2] i2c: i2c-mux-gpio: update mux with gpiod_set_array_value_cansleep
From: Wolfram Sang @ 2016-11-24 15:14 UTC (permalink / raw)
To: Peter Rosin; +Cc: linux-kernel, Peter Korsgaard, Wolfram Sang, linux-i2c
In-Reply-To: <1479908035-18284-1-git-send-email-peda@axentia.se>
[-- Attachment #1: Type: text/plain, Size: 813 bytes --]
Hi Peter,
> + int values[mux->data.n_gpios];
Hmm, my code checkers complain about this line:
CHECK drivers/i2c/muxes/i2c-mux-gpio.c
SPARSE
drivers/i2c/muxes/i2c-mux-gpio.c:29:29: warning: Variable length array is used.
SMATCH
drivers/i2c/muxes/i2c-mux-gpio.c:29:29: warning: Variable length array is used.
Worth to fix it?
BTW (unrelated to your patch), the compiler complains about:
In file included from drivers/i2c/muxes/i2c-mux-gpio.c:18:0:
drivers/i2c/muxes/../../gpio/gpiolib.h:88:27: warning: ‘gpio_suffixes’ defined but not used [-Wunused-const-variable=]
static const char * const gpio_suffixes[] = { "gpios", "gpio" };
which pointed out this line to me:
18 #include "../../gpio/gpiolib.h"
which is probably worth fixing, too?
Thanks,
Wolfram
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* [PATCH v4 0/7] mux controller abstraction and iio/i2c muxes
From: Peter Rosin @ 2016-11-24 15:18 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Rosin, Wolfram Sang, Rob Herring, Mark Rutland,
Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Jonathan Corbet, Arnd Bergmann,
Greg Kroah-Hartman, linux-i2c, devicetree, linux-iio, linux-doc
Hi!
v3 -> v4 changes
- 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
- 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
- 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.
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
generic i2c mux driver. The new i2c mux I called "simple"
since it is only hooking the i2c muxing and the new mux
controller (much like the alsa simple card driver does for ASoC).
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) to be i2c-mux-simple muxes.
I'm using an rwsem to lock a mux, but that isn't really a
perfect fit. Is there a better locking primitive that I don't
know about that fits better? I had a mutex at one point, but
that didn't allow any concurrent accesses at all. At least
the rwsem allows concurrent access as long as all users
agree on the mux state, but I suspect that the rwsem will
degrade to the mutex situation pretty quickly if there is
any contention.
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...
Cheers,
Peter
Peter Rosin (7):
dt-bindings: document devicetree bindings for mux-controllers and
mux-gpio
misc: minimal mux subsystem and gpio-based mux controller
iio: inkern: api for manipulating ext_info of iio channels
dt-bindings: iio: iio-mux: document iio-mux bindings
iio: multiplexer: new iio category and iio-mux driver
dt-bindings: i2c: i2c-mux-simple: document i2c-mux-simple bindings
i2c: i2c-mux-simple: new driver
.../devicetree/bindings/i2c/i2c-mux-simple.txt | 76 ++++
.../bindings/iio/multiplexer/iio-mux.txt | 47 +++
.../devicetree/bindings/misc/mux-controller.txt | 106 +++++
.../devicetree/bindings/misc/mux-gpio.txt | 78 ++++
Documentation/driver-model/devres.txt | 6 +-
MAINTAINERS | 14 +
drivers/i2c/muxes/Kconfig | 13 +
drivers/i2c/muxes/Makefile | 1 +
drivers/i2c/muxes/i2c-mux-simple.c | 179 ++++++++
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 | 457 +++++++++++++++++++++
drivers/misc/Kconfig | 23 ++
drivers/misc/Makefile | 2 +
drivers/misc/mux-core.c | 311 ++++++++++++++
drivers/misc/mux-gpio.c | 124 ++++++
include/linux/iio/consumer.h | 37 ++
include/linux/mux.h | 160 ++++++++
21 files changed, 1719 insertions(+), 1 deletion(-)
create mode 100644 Documentation/devicetree/bindings/i2c/i2c-mux-simple.txt
create mode 100644 Documentation/devicetree/bindings/iio/multiplexer/iio-mux.txt
create mode 100644 Documentation/devicetree/bindings/misc/mux-controller.txt
create mode 100644 Documentation/devicetree/bindings/misc/mux-gpio.txt
create mode 100644 drivers/i2c/muxes/i2c-mux-simple.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/misc/mux-core.c
create mode 100644 drivers/misc/mux-gpio.c
create mode 100644 include/linux/mux.h
--
2.1.4
^ permalink raw reply
* [PATCH v4 1/7] dt-bindings: document devicetree bindings for mux-controllers and mux-gpio
From: Peter Rosin @ 2016-11-24 15:18 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Rosin, Wolfram Sang, Rob Herring, Mark Rutland,
Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Jonathan Corbet, Arnd Bergmann,
Greg Kroah-Hartman, linux-i2c, devicetree, linux-iio, linux-doc
In-Reply-To: <1480000687-5630-1-git-send-email-peda@axentia.se>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
.../devicetree/bindings/misc/mux-controller.txt | 106 +++++++++++++++++++++
.../devicetree/bindings/misc/mux-gpio.txt | 78 +++++++++++++++
MAINTAINERS | 5 +
3 files changed, 189 insertions(+)
create mode 100644 Documentation/devicetree/bindings/misc/mux-controller.txt
create mode 100644 Documentation/devicetree/bindings/misc/mux-gpio.txt
diff --git a/Documentation/devicetree/bindings/misc/mux-controller.txt b/Documentation/devicetree/bindings/misc/mux-controller.txt
new file mode 100644
index 000000000000..8e0733f60a37
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/mux-controller.txt
@@ -0,0 +1,106 @@
+Common multiplexer controller bindings
+
+A mux controller will have one, or several, consumer devices that uses the
+mux controller. Thus, a mux controller can possibly control several
+parallel multiplexers. If there are several consumer devices, the node for
+a mux controller will have one child node for each consumer of the multiplexer
+controller. If there is only one consumer of the mux controller, the mux
+controller node can be the child (with node name 'mux-controller') of the
+consumer node.
+
+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.
+
+Example:
+
+ /*
+ * One consumer of a 2-way multiplexer controller by one GPIO-line,
+ * with the multiplexer controller as a child node.
+ */
+ adc-mux {
+ compatible = "iio-mux";
+ io-channels = <&adc 0>;
+ io-channel-names = "parent";
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ mux-controller {
+ compatible = "mux-gpio";
+
+ mux-gpios = <&pioA 0 GPIO_ACTIVE_HIGH>;
+ };
+
+ sync-1@0 {
+ reg = <0>;
+ };
+
+ in@1 {
+ reg = <1>;
+ };
+ };
+
+ /*
+ * 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 {
+ compatible = "mux-gpio";
+
+ mux-gpios = <&pioA 0 GPIO_ACTIVE_HIGH>,
+ <&pioA 1 GPIO_ACTIVE_HIGH>;
+
+ adc-mux {
+ compatible = "iio-mux";
+ io-channels = <&adc 0>;
+ io-channel-names = "parent";
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ sync-1@0 {
+ reg = <0>;
+ };
+
+ in@1 {
+ reg = <1>;
+ };
+
+ out@2 {
+ reg = <2>;
+ };
+
+ sync-2@3 {
+ reg = <3>;
+ };
+ };
+
+ i2c-mux {
+ compatible = "i2c-mux-simple,mux-locked";
+ i2c-parent = <&i2c1>;
+
+ #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/misc/mux-gpio.txt b/Documentation/devicetree/bindings/misc/mux-gpio.txt
new file mode 100644
index 000000000000..23b87913f4b3
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/mux-gpio.txt
@@ -0,0 +1,78 @@
+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 : "mux-gpio"
+- mux-gpios : list of gpios used to control the multiplexer, least
+ significant bit first.
+* 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 multiplexer state is defined as the number represented by the
+multiplexer GPIO pins, where the first pin is the least significant
+bit. And active pin is a binary 1, an inactive pin is a binary 0.
+
+Example:
+ mux {
+ compatible = "mux-gpio";
+
+ mux-gpios = <&pioA 0 GPIO_ACTIVE_HIGH>,
+ <&pioA 1 GPIO_ACTIVE_HIGH>;
+
+ adc {
+ compatible = "iio-mux";
+ io-channels = <&adc 0>;
+ io-channel-names = "parent";
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ sync-1@0 {
+ reg = <0>;
+ };
+
+ in@1 {
+ reg = <1>;
+ };
+
+ out@2 {
+ reg = <2>;
+ };
+
+ sync-2@3 {
+ reg = <3>;
+ };
+ };
+
+ i2c-mux {
+ compatible = "i2c-mux-simple,mux-locked";
+ i2c-parent = <&i2c1>;
+
+ #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/MAINTAINERS b/MAINTAINERS
index 4ebcd8df29d2..35869ed0a50e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8402,6 +8402,11 @@ 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/misc/mux-*
+
MULTISOUND SOUND DRIVER
M: Andrew Veliath <andrewtv@usa.net>
S: Maintained
--
2.1.4
^ permalink raw reply related
* [PATCH v4 2/7] misc: minimal mux subsystem and gpio-based mux controller
From: Peter Rosin @ 2016-11-24 15:18 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Rosin, Wolfram Sang, Rob Herring, Mark Rutland,
Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Jonathan Corbet, Arnd Bergmann,
Greg Kroah-Hartman, linux-i2c, devicetree, linux-iio, linux-doc
In-Reply-To: <1480000687-5630-1-git-send-email-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.
This new mux controller subsystem comes with a single backend driver
that controls gpio based multiplexers.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
Documentation/driver-model/devres.txt | 6 +-
MAINTAINERS | 2 +
drivers/misc/Kconfig | 23 +++
drivers/misc/Makefile | 2 +
drivers/misc/mux-core.c | 311 ++++++++++++++++++++++++++++++++++
drivers/misc/mux-gpio.c | 124 ++++++++++++++
include/linux/mux.h | 160 +++++++++++++++++
7 files changed, 627 insertions(+), 1 deletion(-)
create mode 100644 drivers/misc/mux-core.c
create mode 100644 drivers/misc/mux-gpio.c
create mode 100644 include/linux/mux.h
diff --git a/Documentation/driver-model/devres.txt b/Documentation/driver-model/devres.txt
index ca9d1eb46bc0..d64ede85b61b 100644
--- a/Documentation/driver-model/devres.txt
+++ b/Documentation/driver-model/devres.txt
@@ -330,7 +330,11 @@ MEM
devm_kzalloc()
MFD
- devm_mfd_add_devices()
+ devm_mfd_add_devices()
+
+MUX
+ devm_mux_control_get()
+ devm_mux_control_put()
PER-CPU MEM
devm_alloc_percpu()
diff --git a/MAINTAINERS b/MAINTAINERS
index 35869ed0a50e..9635c9708a13 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8406,6 +8406,8 @@ MULTIPLEXER SUBSYSTEM
M: Peter Rosin <peda@axentia.se>
S: Maintained
F: Documentation/devicetree/bindings/misc/mux-*
+F: include/linux/mux.h
+F: drivers/misc/mux-*
MULTISOUND SOUND DRIVER
M: Andrew Veliath <andrewtv@usa.net>
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 64971baf11fa..a3ca79e082c7 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -766,6 +766,29 @@ config PANEL_BOOT_MESSAGE
An empty message will only clear the display at driver init time. Any other
printf()-formatted message is valid with newline and escape codes.
+config 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.
+
+if MULTIPLEXER
+
+config MUX_GPIO
+ tristate "GPIO-controlled MUX controller"
+ depends on OF && GPIOLIB
+ help
+ GPIO-controlled MUX controller.
+
+ To compile this driver as a module, choose M here: the module will
+ be called mux-gpio.
+
+endif
+
source "drivers/misc/c2port/Kconfig"
source "drivers/misc/eeprom/Kconfig"
source "drivers/misc/cb710/Kconfig"
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 31983366090a..0befa2bba762 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -53,6 +53,8 @@ obj-$(CONFIG_ECHO) += echo/
obj-$(CONFIG_VEXPRESS_SYSCFG) += vexpress-syscfg.o
obj-$(CONFIG_CXL_BASE) += cxl/
obj-$(CONFIG_PANEL) += panel.o
+obj-$(CONFIG_MULTIPLEXER) += mux-core.o
+obj-$(CONFIG_MUX_GPIO) += mux-gpio.o
lkdtm-$(CONFIG_LKDTM) += lkdtm_core.o
lkdtm-$(CONFIG_LKDTM) += lkdtm_bugs.o
diff --git a/drivers/misc/mux-core.c b/drivers/misc/mux-core.c
new file mode 100644
index 000000000000..6617fc78c072
--- /dev/null
+++ b/drivers/misc/mux-core.c
@@ -0,0 +1,311 @@
+/*
+ * Multiplexer subsystem
+ *
+ * Copyright (C) 2016 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/idr.h>
+#include <linux/module.h>
+#include <linux/mux.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/slab.h>
+
+static struct class mux_class = {
+ .name = "mux",
+ .owner = THIS_MODULE,
+};
+
+static int __init mux_init(void)
+{
+ return class_register(&mux_class);
+}
+
+static void __exit mux_exit(void)
+{
+ class_unregister(&mux_class);
+}
+
+static DEFINE_IDA(mux_ida);
+
+static void mux_control_release(struct device *dev)
+{
+ struct mux_control *mux = to_mux_control(dev);
+
+ ida_simple_remove(&mux_ida, mux->id);
+ kfree(mux);
+}
+
+static struct device_type mux_type = {
+ .name = "mux-control",
+ .release = mux_control_release,
+};
+
+struct mux_control *mux_control_alloc(struct device *dev, size_t sizeof_priv)
+{
+ struct mux_control *mux;
+
+ mux = kzalloc(sizeof(*mux) + sizeof_priv, GFP_KERNEL);
+ if (!mux)
+ return NULL;
+
+ mux->dev.class = &mux_class;
+ mux->dev.type = &mux_type;
+ mux->dev.parent = dev;
+ mux->dev.of_node = dev->of_node;
+ dev_set_drvdata(&mux->dev, mux);
+
+ mux->id = ida_simple_get(&mux_ida, 0, 0, GFP_KERNEL);
+ if (mux->id < 0) {
+ pr_err("muxX failed to get a device id\n");
+ kfree(mux);
+ return NULL;
+ }
+ dev_set_name(&mux->dev, "mux%d", mux->id);
+
+ init_rwsem(&mux->lock);
+ mux->cached_state = -1;
+ mux->idle_state = -1;
+
+ device_initialize(&mux->dev);
+
+ return mux;
+}
+EXPORT_SYMBOL_GPL(mux_control_alloc);
+
+int mux_control_register(struct mux_control *mux)
+{
+ int ret;
+
+ ret = device_add(&mux->dev);
+ if (ret < 0)
+ return ret;
+
+ if (mux->drv_pdev)
+ return ret;
+
+ ret = of_platform_populate(mux->dev.of_node, NULL, NULL, &mux->dev);
+ if (ret < 0)
+ device_del(&mux->dev);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(mux_control_register);
+
+void mux_control_unregister(struct mux_control *mux)
+{
+ if (!mux->drv_pdev)
+ of_platform_depopulate(&mux->dev);
+
+ device_del(&mux->dev);
+}
+EXPORT_SYMBOL_GPL(mux_control_unregister);
+
+void mux_control_put(struct mux_control *mux)
+{
+ struct platform_device *drv_pdev;
+
+ if (!mux)
+ return;
+ put_device(&mux->dev);
+
+ if (!mux->drv_pdev)
+ return;
+
+ if (atomic_read(&mux->dev.kobj.kref.refcount) != 1)
+ return;
+
+ /*
+ * Only one ref left, and the mux core created the driver
+ * that presumably holds it. Time to release the driver so
+ * that it can let go of the final ref.
+ */
+ drv_pdev = mux->drv_pdev;
+ mux->drv_pdev = NULL;
+ platform_device_unregister(drv_pdev);
+}
+EXPORT_SYMBOL_GPL(mux_control_put);
+
+static int mux_control_set(struct mux_control *mux, int state)
+{
+ int ret = mux->ops->set(mux, state);
+
+ mux->cached_state = ret < 0 ? -1 : state;
+
+ return ret;
+}
+
+int mux_control_select(struct mux_control *mux, int state)
+{
+ int ret;
+
+ if (down_read_trylock(&mux->lock)) {
+ if (mux->cached_state == state)
+ return 0;
+
+ /* Sigh, the mux needs updating... */
+ up_read(&mux->lock);
+ }
+
+ /* ...or it's just contended. */
+ down_write(&mux->lock);
+
+ if (mux->cached_state == state) {
+ /*
+ * Hmmm, someone else changed the mux to my liking.
+ * That makes me wonder how long I waited for nothing?
+ */
+ downgrade_write(&mux->lock);
+ return 0;
+ }
+
+ ret = mux_control_set(mux, state);
+ if (ret < 0) {
+ if (mux->idle_state != -1)
+ mux_control_set(mux, mux->idle_state);
+
+ up_write(&mux->lock);
+ return ret;
+ }
+
+ downgrade_write(&mux->lock);
+
+ return 1;
+}
+EXPORT_SYMBOL_GPL(mux_control_select);
+
+int mux_control_deselect(struct mux_control *mux)
+{
+ int ret = 0;
+
+ if (mux->idle_state != -1 && mux->cached_state != mux->idle_state)
+ ret = mux_control_set(mux, mux->idle_state);
+
+ up_read(&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_control *of_find_mux_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_control(dev) : NULL;
+}
+
+struct mux_control *mux_control_get(struct device *dev)
+{
+ struct device_node *mux_np;
+ struct platform_device *drv_pdev;
+ struct mux_control *mux;
+
+ if (!dev->of_node)
+ return ERR_PTR(-ENODEV);
+
+ mux_np = of_get_child_by_name(dev->of_node, "mux-controller");
+ if (!mux_np) {
+ mux = of_find_mux_by_node(dev->of_node->parent);
+ if (!mux)
+ return ERR_PTR(-EPROBE_DEFER);
+
+ return mux;
+ }
+
+ drv_pdev = of_platform_device_create(mux_np, "mux-controller", dev);
+ of_node_put(mux_np);
+
+ if (!drv_pdev)
+ return ERR_PTR(-EPROBE_DEFER);
+
+ mux = of_find_mux_by_node(mux_np);
+ if (!mux) {
+ platform_device_unregister(drv_pdev);
+ return ERR_PTR(-ENODEV);
+ }
+
+ /*
+ * Aiee, holding a reference to the driver that holds a
+ * reference back. Circular deps, and refcounts never
+ * hit zero -> leak.
+ * So, watch for the mux-controller refcount to hit one
+ * and release the driver-ref then, knowing that the
+ * driver will (probably) not let go of its back-ref as
+ * long as the mux core holds a ref to it.
+ */
+
+ mux->drv_pdev = drv_pdev;
+ return mux;
+}
+EXPORT_SYMBOL_GPL(mux_control_get);
+
+static void devm_mux_control_free(struct device *dev, void *res)
+{
+ struct mux_control *mux = *(struct mux_control **)res;
+
+ mux_control_put(mux);
+}
+
+struct mux_control *devm_mux_control_get(struct device *dev)
+{
+ struct mux_control **ptr, *mux;
+
+ ptr = devres_alloc(devm_mux_control_free, sizeof(*ptr), GFP_KERNEL);
+ if (!ptr)
+ return ERR_PTR(-ENOMEM);
+
+ mux = mux_control_get(dev);
+ if (IS_ERR(mux)) {
+ devres_free(ptr);
+ return mux;
+ }
+
+ *ptr = mux;
+ devres_add(dev, ptr);
+
+ return mux;
+}
+EXPORT_SYMBOL_GPL(devm_mux_control_get);
+
+static int devm_mux_control_match(struct device *dev, void *res, void *data)
+{
+ struct mux_control **r = res;
+
+ if (!r || !*r) {
+ WARN_ON(!r || !*r);
+ return 0;
+ }
+
+ return *r == data;
+}
+
+void devm_mux_control_put(struct device *dev, struct mux_control *mux)
+{
+ WARN_ON(devres_release(dev, devm_mux_control_free,
+ devm_mux_control_match, mux));
+}
+EXPORT_SYMBOL_GPL(devm_mux_control_put);
+
+subsys_initcall(mux_init);
+module_exit(mux_exit);
+
+MODULE_AUTHOR("Peter Rosin <peda@axentia.se");
+MODULE_DESCRIPTION("MUX subsystem");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/misc/mux-gpio.c b/drivers/misc/mux-gpio.c
new file mode 100644
index 000000000000..66798d920e17
--- /dev/null
+++ b/drivers/misc/mux-gpio.c
@@ -0,0 +1,124 @@
+/*
+ * GPIO-controlled multiplexer driver
+ *
+ * Copyright (C) 2016 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.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+
+struct mux_gpio {
+ struct gpio_descs *gpios;
+};
+
+static int mux_gpio_set(struct mux_control *mux, int state)
+{
+ struct mux_gpio *mux_gpio = mux_control_priv(mux);
+ int values[mux_gpio->gpios->ndescs];
+ int i;
+
+ for (i = 0; i < mux_gpio->gpios->ndescs; i++)
+ values[i] = (state >> i) & 1;
+
+ gpiod_set_array_value_cansleep(mux_gpio->gpios->ndescs,
+ mux_gpio->gpios->desc,
+ values);
+
+ 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 = "mux-gpio", },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, mux_gpio_dt_ids);
+
+static int mux_gpio_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct device_node *np = pdev->dev.of_node;
+ struct mux_control *mux;
+ struct mux_gpio *mux_gpio;
+ u32 idle_state;
+ int ret;
+
+ if (!np)
+ return -ENODEV;
+
+ mux = mux_control_alloc(dev, sizeof(*mux_gpio));
+ if (!mux)
+ return -ENOMEM;
+ mux_gpio = mux_control_priv(mux);
+ mux->ops = &mux_gpio_ops;
+
+ platform_set_drvdata(pdev, mux);
+
+ mux_gpio->gpios = devm_gpiod_get_array(dev, "mux", GPIOD_OUT_LOW);
+ if (IS_ERR(mux_gpio->gpios)) {
+ if (PTR_ERR(mux_gpio->gpios) != -EPROBE_DEFER)
+ dev_err(dev, "failed to get gpios\n");
+ mux_control_put(mux);
+ return PTR_ERR(mux_gpio->gpios);
+ }
+ mux->states = 1 << mux_gpio->gpios->ndescs;
+
+ ret = of_property_read_u32(np, "idle-state", &idle_state);
+ if (ret >= 0) {
+ if (idle_state >= mux->states) {
+ dev_err(dev, "invalid idle-state %u\n", idle_state);
+ return -EINVAL;
+ }
+ mux->idle_state = idle_state;
+ }
+
+ ret = mux_control_register(mux);
+ if (ret < 0) {
+ dev_err(dev, "failed to register mux_control\n");
+ mux_control_put(mux);
+ return ret;
+ }
+
+ dev_info(dev, "%u-way mux-controller registered\n", mux->states);
+
+ return 0;
+}
+
+static int mux_gpio_remove(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct mux_control *mux = to_mux_control(dev);
+
+ mux_control_unregister(mux);
+ mux_control_put(mux);
+
+ return 0;
+}
+
+static struct platform_driver mux_gpio_driver = {
+ .driver = {
+ .name = "mux-gpio",
+ .of_match_table = of_match_ptr(mux_gpio_dt_ids),
+ },
+ .probe = mux_gpio_probe,
+ .remove = mux_gpio_remove,
+};
+module_platform_driver(mux_gpio_driver);
+
+MODULE_AUTHOR("Peter Rosin <peda@axentia.se");
+MODULE_DESCRIPTION("GPIO-controlled multiplexer driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mux.h b/include/linux/mux.h
new file mode 100644
index 000000000000..be7c138f9228
--- /dev/null
+++ b/include/linux/mux.h
@@ -0,0 +1,160 @@
+/*
+ * mux.h - definitions for the multiplexer interface
+ *
+ * Copyright (C) 2016 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_H
+#define _LINUX_MUX_H
+
+#include <linux/device.h>
+#include <linux/rwsem.h>
+
+struct mux_control;
+struct platform_device;
+
+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.
+ * @dev: Device structure.
+ * @id: Used to identify the device internally.
+ * @states: The number of mux controller states.
+ * @cached_state: The current mux controller state, or -1 if none.
+ * @idle_state: The mux controller state to use when inactive, or -1
+ * for none.
+ * @ops: Mux controller operations.
+ */
+struct mux_control {
+ struct rw_semaphore lock; /* protects the state of the mux */
+
+ struct device dev;
+ int id;
+ struct platform_device *drv_pdev;
+
+ unsigned int states;
+ int cached_state;
+ int idle_state;
+
+ const struct mux_control_ops *ops;
+};
+
+#define to_mux_control(x) container_of((x), struct mux_control, dev)
+
+/**
+ * mux_control_priv() - Get the extra memory reserved by mux_control_alloc().
+ * @mux: The mux-control to get the extra memory from.
+ *
+ * Return: Pointer to the private memory requested by the allocator.
+ */
+static inline void *mux_control_priv(struct mux_control *mux)
+{
+ return mux + 1;
+}
+
+/**
+ * mux_control_alloc() - Allocate a mux-control.
+ * @dev: The device implementing the mux interface.
+ * @sizeof_priv: Size of extra memory area for private use by the caller.
+ *
+ * Return: A pointer to the new mux-control, NULL on failure.
+ */
+struct mux_control *mux_control_alloc(struct device *dev, size_t sizeof_priv);
+
+/**
+ * mux_control_register() - Register a mux-control, thus readying it for use.
+ * @mux: The mux-control to register.
+ *
+ * Do not retry registration of the same mux-control on failure. You should
+ * instead put it away with mux_control_put() 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_control_register(struct mux_control *mux);
+
+/**
+ * mux_control_unregister() - Take the mux-control off-line.
+ * @mux: The mux-control to unregister.
+ *
+ * mux_control_unregister() reverses the effects of mux_control_register().
+ * But not completely, you should not try to call mux_control_register()
+ * on a mux-control that has been registered before.
+ */
+void mux_control_unregister(struct mux_control *mux);
+
+/**
+ * mux_control_put() - Put away the mux-control for good.
+ * @mux: The mux-control to put away.
+ *
+ * mux_control_put() reverses the effects of either mux_control_alloc() or
+ * mux_control_get().
+ */
+void mux_control_put(struct mux_control *mux);
+
+/**
+ * mux_control_select() - Select the given multiplexer state.
+ * @mux: The mux-control to request a change of state from.
+ * @state: The new requested state.
+ *
+ * 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 if the requested state was already active, or 1 it the
+ * mux-control state was changed to the requested state. Or a negavive
+ * errno on error.
+ *
+ * Note that the difference in return value of zero or one is of
+ * questionable value; especially if the mux-control has several independent
+ * consumers, which is something the consumers should not be making
+ * assumptions about.
+ */
+int mux_control_select(struct mux_control *mux, int state);
+
+/**
+ * mux_control_deselect() - Deselect the previously selected multiplexer state.
+ * @mux: The mux-control to deselect.
+ *
+ * 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 for others to access.
+ */
+int mux_control_deselect(struct mux_control *mux);
+
+/**
+ * mux_control_get() - Get the mux-control for a device.
+ * @dev: The device that needs a 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);
+
+/**
+ * devm_mux_control_get() - Get the mux-control for a device, with resource
+ * management.
+ * @dev: The device that needs a 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);
+
+/**
+ * devm_mux_control_put() - Resource-managed version mux_control_put().
+ * @dev: The device that originally got the mux-control.
+ * @mux: The mux-control to put away.
+ *
+ * Note that you do not normally need to call this function.
+ */
+void devm_mux_control_put(struct device *dev, struct mux_control *mux);
+
+#endif /* _LINUX_MUX_H */
--
2.1.4
^ permalink raw reply related
* [PATCH v4 3/7] iio: inkern: api for manipulating ext_info of iio channels
From: Peter Rosin @ 2016-11-24 15:18 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Rosin, Wolfram Sang, Rob Herring, Mark Rutland,
Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Jonathan Corbet, Arnd Bergmann,
Greg Kroah-Hartman, linux-i2c, devicetree, linux-iio, linux-doc
In-Reply-To: <1480000687-5630-1-git-send-email-peda@axentia.se>
Extend the inkern api with functions for reading and writing ext_info
of iio channels.
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 b0f4630a163f..4848b8129e6c 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -863,3 +863,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
* [PATCH v4 4/7] dt-bindings: iio: iio-mux: document iio-mux bindings
From: Peter Rosin @ 2016-11-24 15:18 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Peter Rosin, Wolfram Sang, Rob Herring, Mark Rutland,
Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Jonathan Corbet, Arnd Bergmann,
Greg Kroah-Hartman, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1480000687-5630-1-git-send-email-peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
Signed-off-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
---
.../bindings/iio/multiplexer/iio-mux.txt | 47 ++++++++++++++++++++++
MAINTAINERS | 6 +++
2 files changed, 53 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/multiplexer/iio-mux.txt
diff --git a/Documentation/devicetree/bindings/iio/multiplexer/iio-mux.txt b/Documentation/devicetree/bindings/iio/multiplexer/iio-mux.txt
new file mode 100644
index 000000000000..f5eccdfbed40
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/multiplexer/iio-mux.txt
@@ -0,0 +1,47 @@
+IIO 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 : "iio-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>;
+
+Required properties for iio-mux child nodes:
+- reg : The multiplexer state as described in ../misc/mux-controller.txt
+
+For each iio-mux child, an iio channel will be created whose number will
+match the mux controller state.
+
+Example:
+ adc-mux {
+ compatible = "iio-mux";
+ io-channels = <&adc 0>;
+ io-channel-names = "parent";
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ mux-controller {
+ compatible = "mux-gpio";
+
+ mux-gpios = <&pioA 0 GPIO_ACTIVE_HIGH>,
+ <&pioA 1 GPIO_ACTIVE_HIGH>;
+ };
+
+ sync@0 {
+ reg = <0>;
+ };
+
+ in@1 {
+ reg = <1>;
+ };
+
+ system-regulator@2 {
+ reg = <2>;
+ };
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index 9635c9708a13..7f02f58dfc37 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6233,6 +6233,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-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
+L: linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+S: Maintained
+F: Documentation/devicetree/bindings/iio/multiplexer/iio-mux.txt
+
IIO SUBSYSTEM AND DRIVERS
M: Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
R: Hartmut Knaack <knaack.h-Mmb7MZpHnFY@public.gmane.org>
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v4 6/7] dt-bindings: i2c: i2c-mux-simple: document i2c-mux-simple bindings
From: Peter Rosin @ 2016-11-24 15:18 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Peter Rosin, Wolfram Sang, Rob Herring, Mark Rutland,
Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Jonathan Corbet, Arnd Bergmann,
Greg Kroah-Hartman, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1480000687-5630-1-git-send-email-peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
Signed-off-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
---
.../devicetree/bindings/i2c/i2c-mux-simple.txt | 76 ++++++++++++++++++++++
1 file changed, 76 insertions(+)
create mode 100644 Documentation/devicetree/bindings/i2c/i2c-mux-simple.txt
diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-simple.txt b/Documentation/devicetree/bindings/i2c/i2c-mux-simple.txt
new file mode 100644
index 000000000000..ae534a0f87f3
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-mux-simple.txt
@@ -0,0 +1,76 @@
+Simple 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-simple,mux-locked or i2c-mux-simple,parent-locked
+- i2c-parent: The phandle of the I2C bus that this multiplexer's master-side
+ port is connected to.
+* 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 ../misc/mux-controller.txt
+
+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:
+ i2c-mux {
+ compatible = "i2c-mux-simple,mux-locked";
+ i2c-parent = <&i2c1>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ mux-controller {
+ compatible = "mux-gpio";
+
+ mux-gpios = <&pioA 0 GPIO_ACTIVE_HIGH>,
+ <&pioA 1 GPIO_ACTIVE_HIGH>;
+ };
+
+ 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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v4 7/7] i2c: i2c-mux-simple: new driver
From: Peter Rosin @ 2016-11-24 15:18 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Peter Rosin, Wolfram Sang, Rob Herring, Mark Rutland,
Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Jonathan Corbet, Arnd Bergmann,
Greg Kroah-Hartman, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1480000687-5630-1-git-send-email-peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
This is a generic simple i2c mux that uses the generic 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.
Signed-off-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
---
drivers/i2c/muxes/Kconfig | 13 +++
drivers/i2c/muxes/Makefile | 1 +
drivers/i2c/muxes/i2c-mux-simple.c | 179 +++++++++++++++++++++++++++++++++++++
3 files changed, 193 insertions(+)
create mode 100644 drivers/i2c/muxes/i2c-mux-simple.c
diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig
index 10b3d17ae3ea..565921e09a96 100644
--- a/drivers/i2c/muxes/Kconfig
+++ b/drivers/i2c/muxes/Kconfig
@@ -73,6 +73,19 @@ config I2C_MUX_REG
This driver can also be built as a module. If so, the module
will be called i2c-mux-reg.
+config I2C_MUX_SIMPLE
+ tristate "Simple I2C multiplexer"
+ select MULTIPLEXER
+ depends on OF
+ help
+ If you say yes to this option, support will be included for a
+ simple generic I2C multiplexer. This driver provides access to
+ I2C busses connected through a MUX, which is controlled
+ by a generic MUX controller.
+
+ This driver can also be built as a module. If so, the module
+ will be called i2c-mux-simple.
+
config I2C_DEMUX_PINCTRL
tristate "pinctrl-based I2C demultiplexer"
depends on PINCTRL && OF
diff --git a/drivers/i2c/muxes/Makefile b/drivers/i2c/muxes/Makefile
index 9948fa45037f..6821d95c92a3 100644
--- a/drivers/i2c/muxes/Makefile
+++ b/drivers/i2c/muxes/Makefile
@@ -11,5 +11,6 @@ obj-$(CONFIG_I2C_MUX_PCA9541) += i2c-mux-pca9541.o
obj-$(CONFIG_I2C_MUX_PCA954x) += i2c-mux-pca954x.o
obj-$(CONFIG_I2C_MUX_PINCTRL) += i2c-mux-pinctrl.o
obj-$(CONFIG_I2C_MUX_REG) += i2c-mux-reg.o
+obj-$(CONFIG_I2C_MUX_SIMPLE) += i2c-mux-simple.o
ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG
diff --git a/drivers/i2c/muxes/i2c-mux-simple.c b/drivers/i2c/muxes/i2c-mux-simple.c
new file mode 100644
index 000000000000..461d9c21b7db
--- /dev/null
+++ b/drivers/i2c/muxes/i2c-mux-simple.c
@@ -0,0 +1,179 @@
+/*
+ * Generic simple I2C multiplexer
+ *
+ * Copyright (C) 2016 Axentia Technologies AB
+ *
+ * Author: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
+ *
+ * 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.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-simple,parent-locked",
+ .data = (void *)0, },
+ { .compatible = "i2c-mux-simple,mux-locked",
+ .data = (void *)1, },
+ {},
+};
+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;
+ const struct of_device_id *match;
+ 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);
+ 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);
+
+ match = of_match_device(of_match_ptr(i2c_mux_of_match), dev);
+ if (match)
+ muxc->mux_locked = !!of_device_get_match_data(dev);
+
+ 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) {
+ 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-simple",
+ .of_match_table = i2c_mux_of_match,
+ },
+};
+module_platform_driver(i2c_mux_driver);
+
+MODULE_DESCRIPTION("Simple I2C multiplexer driver");
+MODULE_AUTHOR("Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>");
+MODULE_LICENSE("GPL v2");
--
2.1.4
^ permalink raw reply related
* Re: [PATCH 1/2] i2c: designware: report short transfers
From: Wolfram Sang @ 2016-11-24 15:18 UTC (permalink / raw)
To: Russell King
Cc: Andrew Jackson, Liviu Dudau, Mika Westerberg, Wolfram Sang,
Jarkko Nikula, Andy Shevchenko, linux-i2c, linux-arm-kernel
In-Reply-To: <E1c7p12-0000R0-S8@rmk-PC.armlinux.org.uk>
[-- Attachment #1: Type: text/plain, Size: 316 bytes --]
On Fri, Nov 18, 2016 at 07:40:04PM +0000, Russell King wrote:
> Rather than reporting success for a short transfer due to interrupt
> latency, report an error both to the caller, as well as to the kernel
> log.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Applied to for-current, thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 2/2] i2c: designware: fix rx fifo depth tracking
From: Wolfram Sang @ 2016-11-24 15:18 UTC (permalink / raw)
To: Russell King
Cc: Andrew Jackson, Liviu Dudau, Mika Westerberg, Wolfram Sang,
Jarkko Nikula, Andy Shevchenko, linux-i2c, linux-arm-kernel
In-Reply-To: <E1c7p18-0000R6-4w@rmk-PC.armlinux.org.uk>
[-- Attachment #1: Type: text/plain, Size: 1484 bytes --]
On Fri, Nov 18, 2016 at 07:40:10PM +0000, Russell King wrote:
> When loading the TX fifo to receive bytes on the I2C bus, we incorrectly
> count the number of bytes:
>
> rx_limit = dev->rx_fifo_depth - dw_readl(dev, DW_IC_RXFLR);
>
> while (buf_len > 0 && tx_limit > 0 && rx_limit > 0) {
> if (rx_limit - dev->rx_outstanding <= 0)
> break;
> rx_limit--;
> dev->rx_outstanding++;
> }
>
> DW_IC_RXFLR indicates how many bytes are available to be read in the
> FIFO, dev->rx_fifo_depth is the FIFO size, and dev->rx_outstanding is
> the number of bytes that we've requested to be read so far, but which
> have not been read.
>
> Firstly, increasing dev->rx_outstanding and decreasing rx_limit and then
> comparing them results in each byte consuming "two" bytes in this
> tracking, so this is obviously wrong.
>
> Secondly, the number of bytes that _could_ be received into the FIFO at
> any time is the number of bytes we have so far requested but not yet
> read from the FIFO - in other words dev->rx_outstanding.
>
> So, in order to request enough bytes to fill the RX FIFO, we need to
> request dev->rx_fifo_depth - dev->rx_outstanding bytes.
>
> Modifying the code thusly results in us reaching the maximum number of
> bytes outstanding each time we queue more "receive" operations, provided
> the transfer allows that to happen.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Applied to for-current, thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v2] i2c: i2c-mux-gpio: update mux with gpiod_set_array_value_cansleep
From: Peter Rosin @ 2016-11-24 15:45 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linux-kernel, Peter Korsgaard, Wolfram Sang, linux-i2c
In-Reply-To: <20161124151420.GF4271@katana>
On 2016-11-24 16:14, Wolfram Sang wrote:
> Hi Peter,
>
>> + int values[mux->data.n_gpios];
>
> Hmm, my code checkers complain about this line:
>
> CHECK drivers/i2c/muxes/i2c-mux-gpio.c
> SPARSE
> drivers/i2c/muxes/i2c-mux-gpio.c:29:29: warning: Variable length array is used.
> SMATCH
> drivers/i2c/muxes/i2c-mux-gpio.c:29:29: warning: Variable length array is used.
>
> Worth to fix it?
Yes, especially since I spotted an unrelated bug in my patch.
Regarding this though, I just thought (smallish) variable length
arrays were ok. But I guess smallish isn't very exact...
> BTW (unrelated to your patch), the compiler complains about:
>
> In file included from drivers/i2c/muxes/i2c-mux-gpio.c:18:0:
> drivers/i2c/muxes/../../gpio/gpiolib.h:88:27: warning: ‘gpio_suffixes’ defined but not used [-Wunused-const-variable=]
> static const char * const gpio_suffixes[] = { "gpios", "gpio" };
>
> which pointed out this line to me:
>
> 18 #include "../../gpio/gpiolib.h"
>
> which is probably worth fixing, too?
Yes, I never liked that include, but I don't know how to get
from struct gpio_desc * to the relevant struct device *
without it...
It's this line that needs it:
gpio_dev = &gpio_dec->gdev->dev;
If you can replace it with something neater, go for it!
Cheers,
Peter
^ permalink raw reply
* [PATCH v4 5/7] iio: multiplexer: new iio category and iio-mux driver
From: Peter Rosin @ 2016-11-24 15:18 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Rosin, Wolfram Sang, Rob Herring, Mark Rutland,
Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Jonathan Corbet, Arnd Bergmann,
Greg Kroah-Hartman, linux-i2c, devicetree, linux-iio, linux-doc
In-Reply-To: <1480000687-5630-1-git-send-email-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
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.
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 | 457 ++++++++++++++++++++++++++++++++++++++
6 files changed, 484 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 7f02f58dfc37..52312a12769f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6238,6 +6238,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..70a044510686
--- /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
+ 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..8ad001c248f9
--- /dev/null
+++ b/drivers/iio/multiplexer/iio-mux.c
@@ -0,0 +1,457 @@
+/*
+ * IIO multiplexer driver
+ *
+ * Copyright (C) 2016 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.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+
+struct mux_ext_info_cache {
+ char *data;
+ size_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;
+
+ 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,
+ struct device_node *child_np, 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;
+ u32 state;
+ char *page = NULL;
+ int num_ext_info;
+ int i;
+ int ret;
+
+ chan->indexed = 1;
+ chan->output = pchan->output;
+ chan->datasheet_name = child_np->name;
+ 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);
+
+ ret = of_property_read_u32(child_np, "reg", &state);
+ if (ret < 0) {
+ dev_err(dev, "no reg property for node '%s'\n", child_np->name);
+ return ret;
+ }
+
+ if (state >= mux->control->states) {
+ dev_err(dev, "invalid reg %u\n", state);
+ return -EINVAL;
+ }
+
+ for (i = 0; i < idx; ++i) {
+ if (mux->chan[i].channel == state) {
+ dev_err(dev, "double use of reg %u\n", state);
+ 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;
+}
+
+static int mux_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct device_node *np = pdev->dev.of_node;
+ struct device_node *child_np;
+ struct iio_dev *indio_dev;
+ struct iio_channel *parent;
+ struct mux *mux;
+ 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;
+ for_each_child_of_node(np, child_np) {
+ if (of_get_property(child_np, "reg", NULL))
+ 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);
+ 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;
+ for_each_child_of_node(np, child_np) {
+ if (!of_get_property(child_np, "reg", NULL))
+ continue;
+
+ ret = mux_configure_channel(dev, mux, child_np, i);
+ if (ret < 0)
+ return ret;
+ i++;
+ }
+
+ 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 = "iio-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
* [PATCH v3] i2c: i2c-mux-gpio: update mux with gpiod_set_array_value_cansleep
From: Peter Rosin @ 2016-11-24 18:19 UTC (permalink / raw)
To: linux-kernel; +Cc: Peter Rosin, Peter Korsgaard, Wolfram Sang, linux-i2c
If the gpio controller supports it and the gpio lines are concentrated
to one gpio chip, the mux controller pins will get updated simultaneously.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/i2c/muxes/i2c-mux-gpio.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
Hi!
v2 -> v3 changes
- preallocate temporary storage for the values array needed by the
gpiod_set_array_value_cansleep call, killing a warning about variable
length arrays reported by Wolfram.
- allocate a correctly sized array for gpios (number of pins, not
number of channels).
v1 -> v2 changes
- rebase onto something without interference from other patches
Cheers,
Peter
diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c
index e5cf26eefa97..655684d621a4 100644
--- a/drivers/i2c/muxes/i2c-mux-gpio.c
+++ b/drivers/i2c/muxes/i2c-mux-gpio.c
@@ -21,6 +21,8 @@
struct gpiomux {
struct i2c_mux_gpio_platform_data data;
unsigned gpio_base;
+ struct gpio_desc **gpios;
+ int *values;
};
static void i2c_mux_gpio_set(const struct gpiomux *mux, unsigned val)
@@ -28,8 +30,10 @@ static void i2c_mux_gpio_set(const struct gpiomux *mux, unsigned val)
int i;
for (i = 0; i < mux->data.n_gpios; i++)
- gpio_set_value_cansleep(mux->gpio_base + mux->data.gpios[i],
- val & (1 << i));
+ mux->values[i] = (val >> i) & 1;
+
+ gpiod_set_array_value_cansleep(mux->data.n_gpios,
+ mux->gpios, mux->values);
}
static int i2c_mux_gpio_select(struct i2c_mux_core *muxc, u32 chan)
@@ -176,12 +180,16 @@ static int i2c_mux_gpio_probe(struct platform_device *pdev)
if (!parent)
return -EPROBE_DEFER;
- muxc = i2c_mux_alloc(parent, &pdev->dev, mux->data.n_values, 0, 0,
+ muxc = i2c_mux_alloc(parent, &pdev->dev, mux->data.n_values,
+ mux->data.n_gpios * sizeof(*mux->gpios) +
+ mux->data.n_gpios * sizeof(*mux->values), 0,
i2c_mux_gpio_select, NULL);
if (!muxc) {
ret = -ENOMEM;
goto alloc_failed;
}
+ mux->gpios = muxc->priv;
+ mux->values = (int *)(mux->gpios + mux->data.n_gpios);
muxc->priv = mux;
platform_set_drvdata(pdev, muxc);
@@ -219,10 +227,12 @@ static int i2c_mux_gpio_probe(struct platform_device *pdev)
goto err_request_gpio;
}
+ gpio_desc = gpio_to_desc(gpio_base + mux->data.gpios[i]);
+ mux->gpios[i] = gpio_desc;
+
if (!muxc->mux_locked)
continue;
- gpio_desc = gpio_to_desc(gpio_base + mux->data.gpios[i]);
gpio_dev = &gpio_desc->gdev->dev;
muxc->mux_locked = i2c_root_adapter(gpio_dev) == root;
}
--
2.1.4
^ permalink raw reply related
* Re: [PATCH] Add IDT 89HPESx EEPROM/CSR driver
From: Serge Semin @ 2016-11-24 18:42 UTC (permalink / raw)
To: Greg KH; +Cc: arnd, wsa, jdelvare, linux-i2c, linux-kernel, Sergey.Semin
In-Reply-To: <20161030135358.GA9026@kroah.com>
On Sun, Oct 30, 2016 at 09:53:58AM -0400, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Mon, Oct 03, 2016 at 02:13:45AM +0300, Serge Semin wrote:
> > Hello linux folks,
> >
> > This driver primarily is developed to give an access to EEPROM of IDT
> > PCIe-switches. Such switches provide a simple SMBus interface to perform
> > IO-operations from/to EEPROM, which is located at private (also called master)
> > SMBus of the switches. Using that interface this driver creates a simple
> > binary sysfs-file in the device directory:
> > /sys/bus/i2c/devices/<bus>-<devaddr>/eeprom
> > In case if read-only flag is specified in the device dts-node, user-space
> > applications won't be able to write to the EEPROM sysfs-node.
> >
> > Additionally IDT 89HPESx SMBus interface provides an ability to write/read
> > data of device CSRs. This driver exposes corresponding sysfs-file to perform
> > simple IO operations using that facility for just basic debug purpose.
>
> If it's for debugging, please put it in debugfs, not in sysfs. sysfs
> files for one-off drivers is usually discouraged, but at the least, you
> have to document it in a Documentation/ABI/ file. For debugfs files, we
> don't care, you can do whatever you want in them :)
>
Undrestood. I'll move it to Debugfs.
> > Particularly next file is created in the device specific sysfs-directory:
> > /sys/bus/i2c/devices/<bus>-<devaddr>/csr
> > Format of the sysfs-node is:
> > $ cat /sys/bus/i2c/devices/<bus>-<devaddr>/csr;
> > <CSR address>:<CSR value>
> > So reading the content of the sysfs-file gives current CSR address and
> > it value. If user-space application wishes to change current CSR address,
> > it can just write a proper value to the sysfs-file:
> > $ echo "<CSR address>" > /sys/bus/i2c/devices/<bus>-<devaddr>/csr
> > If it wants to change the CSR value as well, the format of the write
> > operation is:
> > $ echo "<CSR address>:<CSR value>" > \
> > /sys/bus/i2c/devices/<bus>-<devaddr>/csr;
> > CSR address and value can be any of hexadecimal, decimal or octal format.
>
> Yeah, that all should go into debugfs.
>
Ok.
> > The driver supports the most of the commonly available SMBus operations:
> > SMBus i2c block, SMBus block, smbus word and byte. The code has been tested
> > to be built for x32/x64 MIPS architecture and tested on the x32 MIPS machine.
> > The patch was applied on top of commit c6935931c1894ff857616ff8549b61236a19148f
> > of master branch of repository
> > git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git
> >
> >
> > Thanks,
> >
> > =============================
> > Serge V. Semin
> > Leading Programmer
> > Embedded SW development group
> > T-platforms
> > =============================
> >
> > Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
>
> meta-comment, the "hello", and "thanks" and signature doesn't need to be
> in a changelog text, next time you can drop that. See the many kernel
> patches on the mailing lists for examples of how to do this.
>
Understood.
>
> >
> > ---
> > .../devicetree/bindings/misc/idt_89hpesx.txt | 41 +
>
>
> Can you split this out into a separate file and be sure to cc: the patch
> to the devicetree maintainers?
>
> > drivers/misc/eeprom/Kconfig | 10 +
> > drivers/misc/eeprom/Makefile | 1 +
> > drivers/misc/eeprom/idt_89hpesx.c | 1483 ++++++++++++++++++++
> > 4 files changed, 1535 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/misc/idt_89hpesx.txt
> > create mode 100644 drivers/misc/eeprom/idt_89hpesx.c
> >
> > diff --git a/Documentation/devicetree/bindings/misc/idt_89hpesx.txt b/Documentation/devicetree/bindings/misc/idt_89hpesx.txt
> > new file mode 100644
> > index 0000000..469cc93
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/misc/idt_89hpesx.txt
> > @@ -0,0 +1,41 @@
> > +EEPROM / CSR SMBus-slave interface of IDT 89HPESx devices
> > +
> > +Required properties:
> > + - compatible : should be "<manufacturer>,<type>"
> > + Basically there is only one manufacturer: idt, but some
> > + compatible devices may be produced in future. Following devices
> > + are supported: 89hpes8nt2, 89hpes12nt3, 89hpes24nt6ag2,
> > + 89hpes32nt8ag2, 89hpes32nt8bg2, 89hpes12nt12g2, 89hpes16nt16g2,
> > + 89hpes24nt24g2, 89hpes32nt24ag2, 89hpes32nt24bg2;
> > + 89hpes12n3, 89hpes12n3a, 89hpes24n3, 89hpes24n3a;
> > + 89hpes32h8, 89hpes32h8g2, 89hpes48h12, 89hpes48h12g2,
> > + 89hpes48h12ag2, 89hpes16h16, 89hpes22h16, 89hpes22h16g2,
> > + 89hpes34h16, 89hpes34h16g2, 89hpes64h16, 89hpes64h16g2,
> > + 89hpes64h16ag2;
> > + 89hpes12t3g2, 89hpes24t3g2, 89hpes16t4, 89hpes4t4g2,
> > + 89hpes10t4g2, 89hpes16t4g2, 89hpes16t4ag2, 89hpes5t5,
> > + 89hpes6t5, 89hpes8t5, 89hpes8t5a, 89hpes24t6, 89hpes6t6g2,
> > + 89hpes24t6g2, 89hpes16t7, 89hpes32t8, 89hpes32t8g2,
> > + 89hpes48t12, 89hpes48t12g2.
> > + Current implementation of the driver doesn't have any device-
> > + specific functionalities. But since each of them differs
> > + by registers mapping, CSRs read/write restrictions can be
> > + added in future.
> > + - reg : I2C address of the IDT 89HPES device.
> > +
> > +Optional properties:
> > + - read-only : Parameterless property disables writes to the EEPROM
> > + - idt,eesize : Size of EEPROM device connected to IDT 89HPES i2c-master bus
> > + (default value is 4096 bytes if option isn't specified)
> > + - idt,eeaddr : Custom address of EEPROM device
> > + (If not specified IDT 89HPESx device will try to communicate
> > + with EEPROM sited by default address - 0x50)
> > +
> > +Example:
> > + idt_pcie_sw@60 {
> > + compatible = "idt,89hpes12nt3";
> > + reg = <0x60>;
> > + read-only;
> > + idt,eesize = <65536>;
> > + idt,eeaddr = <0x50>;
> > + };
> > diff --git a/drivers/misc/eeprom/Kconfig b/drivers/misc/eeprom/Kconfig
> > index c4e41c2..de58762 100644
> > --- a/drivers/misc/eeprom/Kconfig
> > +++ b/drivers/misc/eeprom/Kconfig
> > @@ -100,4 +100,14 @@ config EEPROM_DIGSY_MTC_CFG
> >
> > If unsure, say N.
> >
> > +config EEPROM_IDT_89HPESX
> > + tristate "IDT 89HPESx PCIe-swtiches EEPROM / CSR support"
> > + depends on I2C && SYSFS
> > + help
> > + Enable this driver to get read/write access to EEPROM / CSRs
> > + over IDT PCIe-swtich i2c-slave interface.
> > +
> > + This driver can also be built as a module. If so, the module
> > + will be called idt_89hpesx.
> > +
> > endmenu
> > diff --git a/drivers/misc/eeprom/Makefile b/drivers/misc/eeprom/Makefile
> > index fc1e81d..90a5262 100644
> > --- a/drivers/misc/eeprom/Makefile
> > +++ b/drivers/misc/eeprom/Makefile
> > @@ -5,3 +5,4 @@ obj-$(CONFIG_EEPROM_MAX6875) += max6875.o
> > obj-$(CONFIG_EEPROM_93CX6) += eeprom_93cx6.o
> > obj-$(CONFIG_EEPROM_93XX46) += eeprom_93xx46.o
> > obj-$(CONFIG_EEPROM_DIGSY_MTC_CFG) += digsy_mtc_eeprom.o
> > +obj-$(CONFIG_EEPROM_IDT_89HPESX) += idt_89hpesx.o
> > diff --git a/drivers/misc/eeprom/idt_89hpesx.c b/drivers/misc/eeprom/idt_89hpesx.c
> > new file mode 100644
> > index 0000000..f95f4f0
> > --- /dev/null
> > +++ b/drivers/misc/eeprom/idt_89hpesx.c
> > @@ -0,0 +1,1483 @@
> > +/*
> > + * This file is provided under a GPLv2 license. When using or
> > + * redistributing this file, you may do so under that license.
> > + *
> > + * GPL LICENSE SUMMARY
> > + *
> > + * Copyright (C) 2016 T-Platforms All Rights Reserved.
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms and conditions of the GNU General Public License,
> > + * version 2, as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful, but WITHOUT
> > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> > + * more details.
> > + *
> > + * You should have received a copy of the GNU General Public License along
> > + * with this program; if not, it can be found <http://www.gnu.org/licenses/>.
> > + *
> > + * The full GNU General Public License is included in this distribution in
> > + * the file called "COPYING".
> > + *
> > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> > + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> > + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> > + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> > + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> > + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> > + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> > + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> > + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> > + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> > + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> > + *
> > + * IDT PCIe-switch NTB Linux driver
> > + *
> > + * Contact Information:
> > + * Serge Semin <fancer.lancer@gmail.com>, <Sergey.Semin@t-platforms.ru>
> > + */
> > +/*
> > + * NOTE of the IDT 89HPESx SMBus-slave interface driver
> > + * This driver primarily is developed to have an access to EEPROM device of
> > + * IDT PCIe-switches. IDT provides a simple SMBus interface to perform IO-
> > + * operations from/to EEPROM, which is located at private (so called Master)
> > + * SMBus of switches. Using that interface this the driver creates a simple
> > + * binary sysfs-file in the device directory:
> > + * /sys/bus/i2c/devices/<bus>-<devaddr>/eeprom
> > + * In case if read-only flag is specified in the dts-node of device desription,
> > + * User-space applications won't be able to write to the EEPROM sysfs-node.
> > + * Additionally IDT 89HPESx SMBus interface has an ability to write/read
> > + * data of device CSRs. This driver exposes another sysfs-file to perform
> > + * simple IO operations using that ability for just basic debug purpose.
> > + * Particularly next file is created in the device specific sysfs-directory:
> > + * /sys/bus/i2c/devices/<bus>-<devaddr>/csr
> > + * Format of the sysfs-node is:
> > + * $ cat /sys/bus/i2c/devices/<bus>-<devaddr>/csr;
> > + * <CSR address>:<CSR value>
> > + * So reading the content of the sysfs-file gives current CSR address and
> > + * it value. If User-space application wishes to change current CSR address,
> > + * it can just write a proper value to the sysfs-file:
> > + * $ echo "<CSR address>" > /sys/bus/i2c/devices/<bus>-<devaddr>/csr
> > + * If it wants to change the CSR value as well, the format of the write
> > + * operation is:
> > + * $ echo "<CSR address>:<CSR value>" > \
> > + * /sys/bus/i2c/devices/<bus>-<devaddr>/csr;
> > + * CSR address and value can be any of hexadecimal, decimal or octal format.
> > + */
> > +
> > +/*
> > + * Note: You can load this module with either option 'dyndbg=+p' or define the
> > + * next preprocessor constant
>
> What does this option do? I don't understand it.
>
> > + */
> > +/* #define DEBUG */
>
> Ah, just drop all of this, dynamic debugging is known how to be used
> through the whole kernel, no need to document it again :)
>
Ok. I'll drop all of this down.
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/init.h>
> > +#include <linux/module.h>
> > +#include <linux/types.h>
> > +#include <linux/sizes.h>
> > +#include <linux/slab.h>
> > +#include <linux/mutex.h>
> > +#include <linux/sysfs.h>
> > +#include <linux/mod_devicetable.h>
> > +#include <linux/of.h>
> > +#include <linux/i2c.h>
> > +#include <linux/pci_ids.h>
>
> why pci_ids.h?
>
I need to have PCI ids to probe the IDT device. As I already said IDT PCIe
switch has a separate i2c slave interface with an access to all device CSRs,
which are basically PCI configuration spaces of all switch ports. So when
device is probed, the driver reads VID and DID to make sure, that it is
really connected to IDT switch.
> > +
> > +#define IDT_NAME "89hpesx"
> > +#define IDT_89HPESX_DESC "IDT 89HPESx SMBus-slave interface driver"
> > +#define IDT_89HPESX_VER "1.0"
> > +
> > +MODULE_DESCRIPTION(IDT_89HPESX_DESC);
> > +MODULE_VERSION(IDT_89HPESX_VER);
> > +MODULE_LICENSE("GPL v2");
> > +MODULE_AUTHOR("T-platforms");
> > +
> > +/*
> > + * Some common constant used in the driver for better readability:
> > + * @SUCCESS: Success of a function execution
> > + */
> > +#define SUCCESS (0)
>
> No need to define this, we all know that 0 is "success", this is the
> kernel.
>
Ok, I'll delete it from the code.
> > +
> > +/*
> > + * struct idt_89hpesx_dev - IDT 89HPESx device data structure
> > + * @eesize: Size of EEPROM in bytes (calculated from "idt,eecompatible")
> > + * @eero: EEPROM Read-only flag
> > + * @eeaddr: EEPROM custom address
> > + *
> > + * @inieecmd: Initial cmd value for EEPROM read/write operations
> > + * @inicsrcmd: Initial cmd value for CSR read/write operations
> > + * @iniccode: Initialial command code value for IO-operations
> > + *
> > + * @csr: CSR address to perform read operation
> > + *
> > + * @smb_write: SMBus write method
> > + * @smb_read: SMBus read method
> > + * @smb_mtx: SMBus mutex
> > + *
> > + * @client: i2c client used to perform IO operations
> > + *
> > + * @eenode: EEPROM sysfs-node to read/write data to/from EEPROM
> > + * @regnode: Register sysfs-node to read/write CSRs
> > + */
> > +struct idt_smb_seq;
> > +struct idt_89hpesx_dev {
> > + u32 eesize;
> > + bool eero;
> > + u8 eeaddr;
> > +
> > + u8 inieecmd;
> > + u8 inicsrcmd;
> > + u8 iniccode;
> > +
> > + atomic_t csr;
> > +
> > + int (*smb_write)(struct idt_89hpesx_dev *, const struct idt_smb_seq *);
> > + int (*smb_read)(struct idt_89hpesx_dev *, struct idt_smb_seq *);
> > + struct mutex smb_mtx;
> > +
> > + struct i2c_client *client;
> > +
> > + struct bin_attribute eenode;
> > + struct bin_attribute regnode;
> > +};
> > +#define to_pdev_kobj(__kobj) \
> > + dev_get_drvdata(container_of(__kobj, struct device, kobj))
> > +
> > +/*
> > + * struct idt_smb_seq - sequence of data to be read/written from/to IDT 89HPESx
> > + * @ccode: SMBus command code
> > + * @bytecnt: Byte count of operation
> > + * @data: Data to by written
> > + */
> > +struct idt_smb_seq {
> > + u8 ccode;
> > + u8 bytecnt;
> > + u8 *data;
> > +};
> > +
> > +/*
> > + * struct idt_eeprom_seq - sequence of data to be read/written from/to EEPROM
> > + * @cmd: Transaction CMD
> > + * @eeaddr: EEPROM custom address
> > + * @memaddr: Internal memory address of EEPROM
> > + * @data: Data to be written at the memory address
> > + */
> > +struct idt_eeprom_seq {
> > + u8 cmd;
> > + u8 eeaddr;
> > + u16 memaddr;
>
> endian? Is this big or little?
>
Since the IDT-switch is PCIe device, it's little endian. Of course, the code
does the proper conversions. You can see it in the functions:
idt_eeprom_write()/idt_eeprom_read().
> > + u8 data;
> > +} __packed;
> > +
> > +/*
> > + * struct idt_csr_seq - sequence of data to be read/written from/to CSR
> > + * @cmd: Transaction CMD
> > + * @csraddr: Internal IDT device CSR address
> > + * @data: Data to be read/written from/to the CSR address
> > + */
> > +struct idt_csr_seq {
> > + u8 cmd;
> > + u16 csraddr;
> > + u32 data;
>
> Same for these, what endian are they?
>
> > +} __packed;
> > +
> > +/*
> > + * SMBus command code macros
> > + * @CCODE_END: Indicates the end of transaction
> > + * @CCODE_START: Indicates the start of transaction
> > + * @CCODE_CSR: CSR read/write transaction
> > + * @CCODE_EEPROM: EEPROM read/write transaction
> > + * @CCODE_BYTE: Supplied data has BYTE length
> > + * @CCODE_WORD: Supplied data has WORD length
> > + * @CCODE_BLOCK: Supplied data has variable length passed in bytecnt
> > + * byte right following CCODE byte
> > + */
> > +#define CCODE_END ((u8)0x01)
> > +#define CCODE_START ((u8)0x02)
> > +#define CCODE_CSR ((u8)0x00)
> > +#define CCODE_EEPROM ((u8)0x04)
> > +#define CCODE_BYTE ((u8)0x00)
> > +#define CCODE_WORD ((u8)0x20)
> > +#define CCODE_BLOCK ((u8)0x40)
> > +#define CCODE_PEC ((u8)0x80)
> > +
> > +/*
> > + * EEPROM command macros
> > + * @EEPROM_OP_WRITE: EEPROM write operation
> > + * @EEPROM_OP_READ: EEPROM read operation
> > + * @EEPROM_USA: Use specified address of EEPROM
> > + * @EEPROM_NAERR: EEPROM device is not ready to respond
> > + * @EEPROM_LAERR: EEPROM arbitration loss error
> > + * @EEPROM_MSS: EEPROM misplace start & stop bits error
> > + * @EEPROM_WR_CNT: Bytes count to perform write operation
> > + * @EEPROM_WRRD_CNT: Bytes count to write before reading
> > + * @EEPROM_RD_CNT: Bytes count to perform read operation
> > + * @EEPROM_DEF_SIZE: Fall back size of EEPROM
> > + * @EEPROM_DEF_ADDR: Defatul EEPROM address
> > + */
> > +#define EEPROM_OP_WRITE ((u8)0x00)
> > +#define EEPROM_OP_READ ((u8)0x01)
> > +#define EEPROM_USA ((u8)0x02)
> > +#define EEPROM_NAERR ((u8)0x08)
> > +#define EEPROM_LAERR ((u8)0x10)
> > +#define EEPROM_MSS ((u8)0x20)
> > +#define EEPROM_WR_CNT ((u8)5)
> > +#define EEPROM_WRRD_CNT ((u8)4)
> > +#define EEPROM_RD_CNT ((u8)5)
> > +#define EEPROM_DEF_SIZE ((u16)4096)
> > +#define EEPROM_DEF_ADDR ((u8)0x50)
> > +
> > +/*
> > + * CSR command macros
> > + * @CSR_DWE: Enable all four bytes of the operation
> > + * @CSR_OP_WRITE: CSR write operation
> > + * @CSR_OP_READ: CSR read operation
> > + * @CSR_RERR: Read operation error
> > + * @CSR_WERR: Write operation error
> > + * @CSR_WR_CNT: Bytes count to perform write operation
> > + * @CSR_WRRD_CNT: Bytes count to write before reading
> > + * @CSR_RD_CNT: Bytes count to perform read operation
> > + * @CSR_MAX: Maximum CSR address
> > + * @CSR_DEF: Default CSR address
> > + * @CSR_REAL_ADDR: CSR real unshifted address
> > + */
> > +#define CSR_DWE ((u8)0x0F)
> > +#define CSR_OP_WRITE ((u8)0x00)
> > +#define CSR_OP_READ ((u8)0x10)
> > +#define CSR_RERR ((u8)0x40)
> > +#define CSR_WERR ((u8)0x80)
> > +#define CSR_WR_CNT ((u8)7)
> > +#define CSR_WRRD_CNT ((u8)3)
> > +#define CSR_RD_CNT ((u8)7)
> > +#define CSR_MAX ((u32)0x3FFFF)
> > +#define CSR_DEF ((u16)0x0000)
> > +#define CSR_REAL_ADDR(val) ((unsigned int)val << 2)
> > +
> > +/*
> > + * IDT 89HPESx basic register
> > + * @IDT_VIDDID_CSR: PCIe VID and DID of IDT 89HPESx
> > + * @IDT_VID_MASK: Mask of VID
> > + */
> > +#define IDT_VIDDID_CSR ((u32)0x0000)
> > +#define IDT_VID_MASK ((u32)0xFFFF)
> > +
> > +/*
> > + * IDT 89HPESx can send NACK when new command is sent before previous one
> > + * fininshed execution. In this case driver retries operation
> > + * certain times.
> > + * @RETRY_CNT: Number of retries before giving up and fail
> > + * @idt_smb_safe: Generate a retry loop on corresponding SMBus method
> > + */
> > +#define RETRY_CNT (128)
> > +#define idt_smb_safe(ops, args...) ({ \
> > + int __retry = RETRY_CNT; \
> > + s32 __sts; \
> > + do { \
> > + __sts = i2c_smbus_ ## ops ## _data(args); \
> > + } while (__retry-- && __sts < SUCCESS); \
> > + __sts; \
> > +})
> > +
> > +/*
> > + * Wrapper dev_err/dev_warn/dev_info/dev_dbg macros
> > + */
> > +#define dev_err_idt(pdev, args...) \
> > + dev_err(&pdev->client->dev, ## args)
> > +#define dev_warn_idt(pdev, args...) \
> > + dev_warn(&pdev->client->dev, ## args)
> > +#define dev_info_idt(pdev, args...) \
> > + dev_info(&pdev->client->dev, ## args)
> > +#define dev_dbg_idt(pdev, args...) \
> > + dev_dbg(&pdev->client->dev, ## args)
>
> ick, don't wrap them, just use them as-is, it's not that hard to write
> them out please.
>
Ok.
> > +
> > +/*===========================================================================
> > + * i2c bus level IO-operations
> > + *===========================================================================
> > + */
> > +
> > +/*
> > + * idt_smb_write_byte() - SMBus write method when I2C_SMBUS_BYTE_DATA operation
> > + * is only available
> > + * @pdev: Pointer to the driver data
> > + * @seq: Sequence of data to be written
> > + */
> > +static int idt_smb_write_byte(struct idt_89hpesx_dev *pdev,
> > + const struct idt_smb_seq *seq)
> > +{
> > + s32 sts;
> > + u8 ccode;
> > + int idx;
> > +
> > + /* Loop over the supplied data sending byte one-by-one */
> > + for (idx = 0; idx < seq->bytecnt; idx++) {
> > + /* Collect the command code byte */
> > + ccode = seq->ccode | CCODE_BYTE;
> > + if (idx == 0)
> > + ccode |= CCODE_START;
> > + if (idx == seq->bytecnt - 1)
> > + ccode |= CCODE_END;
> > +
> > + /* Send data to the device */
> > + sts = idt_smb_safe(write_byte, pdev->client, ccode,
> > + seq->data[idx]);
> > + if (sts != SUCCESS)
> > + return (int)sts;
> > + }
> > +
> > + return SUCCESS;
> > +}
> > +
> > +/*
> > + * idt_smb_read_byte() - SMBus read method when I2C_SMBUS_BYTE_DATA operation
> > + * is only available
> > + * @pdev: Pointer to the driver data
> > + * @seq: Buffer to read data to
> > + */
> > +static int idt_smb_read_byte(struct idt_89hpesx_dev *pdev,
> > + struct idt_smb_seq *seq)
> > +{
> > + s32 sts;
> > + u8 ccode;
> > + int idx;
> > +
> > + /* Loop over the supplied buffer receiving byte one-by-one */
> > + for (idx = 0; idx < seq->bytecnt; idx++) {
> > + /* Collect the command code byte */
> > + ccode = seq->ccode | CCODE_BYTE;
> > + if (idx == 0)
> > + ccode |= CCODE_START;
> > + if (idx == seq->bytecnt - 1)
> > + ccode |= CCODE_END;
> > +
> > + /* Read data from the device */
> > + sts = idt_smb_safe(read_byte, pdev->client, ccode);
> > + if (sts < SUCCESS)
> > + return (int)sts;
> > +
> > + seq->data[idx] = (u8)sts;
> > + }
> > +
> > + return SUCCESS;
> > +}
> > +
> > +/*
> > + * idt_smb_write_word() - SMBus write method when I2C_SMBUS_BYTE_DATA and
> > + * I2C_FUNC_SMBUS_WORD_DATA operations are available
> > + * @pdev: Pointer to the driver data
> > + * @seq: Sequence of data to be written
> > + */
> > +static int idt_smb_write_word(struct idt_89hpesx_dev *pdev,
> > + const struct idt_smb_seq *seq)
> > +{
> > + s32 sts;
> > + u8 ccode;
> > + int idx, evencnt;
> > +
> > + /* Calculate the even count of data to send */
> > + evencnt = seq->bytecnt - (seq->bytecnt % 2);
> > +
> > + /* Loop over the supplied data sending two bytes at a time */
> > + for (idx = 0; idx < evencnt; idx += 2) {
> > + /* Collect the command code byte */
> > + ccode = seq->ccode | CCODE_WORD;
> > + if (idx == 0)
> > + ccode |= CCODE_START;
> > + if (idx == evencnt - 2)
> > + ccode |= CCODE_END;
> > +
> > + /* Send word data to the device */
> > + sts = idt_smb_safe(write_word, pdev->client, ccode,
> > + *(u16 *)&seq->data[idx]);
> > + if (sts != SUCCESS)
> > + return (int)sts;
> > + }
> > +
> > + /* If there is odd number of bytes then send just one last byte */
> > + if (seq->bytecnt != evencnt) {
> > + /* Collect the command code byte */
> > + ccode = seq->ccode | CCODE_BYTE | CCODE_END;
> > + if (idx == 0)
> > + ccode |= CCODE_START;
> > +
> > + /* Send byte data to the device */
> > + sts = idt_smb_safe(write_byte, pdev->client, ccode,
> > + seq->data[idx]);
> > + if (sts != SUCCESS)
> > + return (int)sts;
> > + }
> > +
> > + return SUCCESS;
> > +}
> > +
> > +/*
> > + * idt_smb_read_word() - SMBus read method when I2C_SMBUS_BYTE_DATA and
> > + * I2C_FUNC_SMBUS_WORD_DATA operations are available
> > + * @pdev: Pointer to the driver data
> > + * @seq: Buffer to read data to
> > + */
> > +static int idt_smb_read_word(struct idt_89hpesx_dev *pdev,
> > + struct idt_smb_seq *seq)
> > +{
> > + s32 sts;
> > + u8 ccode;
> > + int idx, evencnt;
> > +
> > + /* Calculate the even count of data to send */
> > + evencnt = seq->bytecnt - (seq->bytecnt % 2);
> > +
> > + /* Loop over the supplied data reading two bytes at a time */
> > + for (idx = 0; idx < evencnt; idx += 2) {
> > + /* Collect the command code byte */
> > + ccode = seq->ccode | CCODE_WORD;
> > + if (idx == 0)
> > + ccode |= CCODE_START;
> > + if (idx == evencnt - 2)
> > + ccode |= CCODE_END;
> > +
> > + /* Read word data from the device */
> > + sts = idt_smb_safe(read_word, pdev->client, ccode);
> > + if (sts < SUCCESS)
> > + return (int)sts;
> > +
> > + *(u16 *)&seq->data[idx] = (u16)sts;
> > + }
> > +
> > + /* If there is odd number of bytes then receive just one last byte */
> > + if (seq->bytecnt != evencnt) {
> > + /* Collect the command code byte */
> > + ccode = seq->ccode | CCODE_BYTE | CCODE_END;
> > + if (idx == 0)
> > + ccode |= CCODE_START;
> > +
> > + /* Read last data byte from the device */
> > + sts = idt_smb_safe(read_byte, pdev->client, ccode);
> > + if (sts < SUCCESS)
> > + return (int)sts;
> > +
> > + seq->data[idx] = (u8)sts;
> > + }
> > +
> > + return SUCCESS;
> > +}
> > +
> > +/*
> > + * idt_smb_write_block() - SMBus write method when I2C_SMBUS_BLOCK_DATA
> > + * operation is available
> > + * @pdev: Pointer to the driver data
> > + * @seq: Sequence of data to be written
> > + */
> > +static int idt_smb_write_block(struct idt_89hpesx_dev *pdev,
> > + const struct idt_smb_seq *seq)
> > +{
> > + u8 ccode;
> > +
> > + /* Return error if too much data passed to send */
> > + if (seq->bytecnt > I2C_SMBUS_BLOCK_MAX)
> > + return -EINVAL;
> > +
> > + /* Collect the command code byte */
> > + ccode = seq->ccode | CCODE_BLOCK | CCODE_START | CCODE_END;
> > +
> > + /* Send block of data to the device */
> > + return idt_smb_safe(write_block, pdev->client, ccode, seq->bytecnt,
> > + seq->data);
> > +}
> > +
> > +/*
> > + * idt_smb_read_block() - SMBus read method when I2C_SMBUS_BLOCK_DATA
> > + * operation is available
> > + * @pdev: Pointer to the driver data
> > + * @seq: Buffer to read data to
> > + */
> > +static int idt_smb_read_block(struct idt_89hpesx_dev *pdev,
> > + struct idt_smb_seq *seq)
> > +{
> > + s32 sts;
> > + u8 ccode;
> > +
> > + /* Return error if too much data passed to send */
> > + if (seq->bytecnt > I2C_SMBUS_BLOCK_MAX)
> > + return -EINVAL;
> > +
> > + /* Collect the command code byte */
> > + ccode = seq->ccode | CCODE_BLOCK | CCODE_START | CCODE_END;
> > +
> > + /* Read block of data from the device */
> > + sts = idt_smb_safe(read_block, pdev->client, ccode, seq->data);
> > + if (sts != seq->bytecnt)
> > + return (sts < SUCCESS ? sts : -ENODATA);
> > +
> > + return SUCCESS;
> > +}
> > +
> > +/*
> > + * idt_smb_write_i2c_block() - SMBus write method when I2C_SMBUS_I2C_BLOCK_DATA
> > + * operation is available
> > + * @pdev: Pointer to the driver data
> > + * @seq: Sequence of data to be written
> > + *
> > + * NOTE It's usual SMBus write block operation, except the actual data length is
> > + * sent as first byte of data
> > + */
> > +static int idt_smb_write_i2c_block(struct idt_89hpesx_dev *pdev,
> > + const struct idt_smb_seq *seq)
> > +{
> > + u8 ccode, buf[I2C_SMBUS_BLOCK_MAX + 1];
> > +
> > + /* Return error if too much data passed to send */
> > + if (seq->bytecnt > I2C_SMBUS_BLOCK_MAX)
> > + return -EINVAL;
> > +
> > + /* Collect the data to send. Length byte must be added prior the data */
> > + buf[0] = seq->bytecnt;
> > + memcpy(&buf[1], seq->data, seq->bytecnt);
> > +
> > + /* Collect the command code byte */
> > + ccode = seq->ccode | CCODE_BLOCK | CCODE_START | CCODE_END;
> > +
> > + /* Send length and block of data to the device */
> > + return idt_smb_safe(write_i2c_block, pdev->client, ccode,
> > + seq->bytecnt + 1, buf);
> > +}
> > +
> > +/*
> > + * idt_smb_read_i2c_block() - SMBus read method when I2C_SMBUS_I2C_BLOCK_DATA
> > + * operation is available
> > + * @pdev: Pointer to the driver data
> > + * @seq: Buffer to read data to
> > + *
> > + * NOTE It's usual SMBus read block operation, except the actual data length is
> > + * retrieved as first byte of data
> > + */
> > +static int idt_smb_read_i2c_block(struct idt_89hpesx_dev *pdev,
> > + struct idt_smb_seq *seq)
> > +{
> > + u8 ccode, buf[I2C_SMBUS_BLOCK_MAX + 1];
> > + s32 sts;
> > +
> > + /* Return error if too much data passed to send */
> > + if (seq->bytecnt > I2C_SMBUS_BLOCK_MAX)
> > + return -EINVAL;
> > +
> > + /* Collect the command code byte */
> > + ccode = seq->ccode | CCODE_BLOCK | CCODE_START | CCODE_END;
> > +
> > + /* Read length and block of data from the device */
> > + sts = idt_smb_safe(read_i2c_block, pdev->client, ccode,
> > + seq->bytecnt + 1, buf);
> > + if (sts != seq->bytecnt + 1)
> > + return (sts < SUCCESS ? sts : -ENODATA);
> > + if (buf[0] != seq->bytecnt)
> > + return -ENODATA;
> > +
> > + /* Copy retrieved data to the output data buffer */
> > + memcpy(seq->data, &buf[1], seq->bytecnt);
> > +
> > + return SUCCESS;
> > +}
> > +
> > +/*===========================================================================
> > + * EEPROM IO-operations
> > + *===========================================================================
> > + */
> > +
> > +/*
> > + * idt_eeprom_write() - EEPROM write operation
> > + * @pdev: Pointer to the driver data
> > + * @memaddr: Start EEPROM memory address
> > + * @len: Length of data to be written
> > + * @data: Data to be written to EEPROM
> > + */
> > +static int idt_eeprom_write(struct idt_89hpesx_dev *pdev, u16 memaddr, u16 len,
> > + const u8 *data)
> > +{
> > + struct idt_eeprom_seq eeseq;
> > + struct idt_smb_seq smbseq;
> > + int ret, retry;
> > + u16 idx;
> > +
> > + /* Initialize SMBus sequence fields */
> > + smbseq.ccode = pdev->iniccode | CCODE_EEPROM;
> > + smbseq.data = (u8 *)&eeseq;
> > +
> > + /* Send data byte-by-byte, checking if it is successfully written */
> > + for (idx = 0; idx < len; idx++, memaddr++) {
> > + /* Lock IDT SMBus device */
> > + mutex_lock(&pdev->smb_mtx);
> > +
> > + /* Perform write operation */
> > + smbseq.bytecnt = EEPROM_WR_CNT;
> > + eeseq.cmd = pdev->inieecmd | EEPROM_OP_WRITE;
> > + eeseq.eeaddr = pdev->eeaddr;
> > + eeseq.memaddr = cpu_to_le16(memaddr);
> > + eeseq.data = data[idx];
> > + ret = pdev->smb_write(pdev, &smbseq);
> > + if (ret != SUCCESS) {
> > + dev_err_idt(pdev,
> > + "Failed to write 0x%04hx:0x%02hhx to eeprom",
> > + memaddr, data[idx]);
> > + goto err_mutex_unlock;
> > + }
> > +
> > + /*
> > + * Check whether the data is successfully written by reading
> > + * from the same EEPROM memory address. Sometimes EEPROM may
> > + * respond with NACK if it's busy with writing previous data,
> > + * so we need to perform a few attempts of read cycle
> > + */
> > + retry = RETRY_CNT;
> > + do {
> > + /* Send EEPROM memory address to read data from */
> > + smbseq.bytecnt = EEPROM_WRRD_CNT;
> > + eeseq.cmd = pdev->inieecmd | EEPROM_OP_READ;
> > + ret = pdev->smb_write(pdev, &smbseq);
> > + if (ret != SUCCESS) {
> > + dev_err_idt(pdev,
> > + "Failed to init mem address 0x%02hhx",
> > + memaddr);
> > + goto err_mutex_unlock;
> > + }
> > +
> > + /* Perform read operation */
> > + smbseq.bytecnt = EEPROM_RD_CNT;
> > + eeseq.data = ~data[idx];
> > + ret = pdev->smb_read(pdev, &smbseq);
> > + if (ret != SUCCESS) {
> > + dev_err_idt(pdev,
> > + "Failed to read mem address 0x%02hhx",
> > + memaddr);
> > + goto err_mutex_unlock;
> > + }
> > + } while (retry-- && (eeseq.cmd & EEPROM_NAERR));
> > +
> > + /* Check whether IDT successfully sent data to EEPROM */
> > + if (eeseq.cmd & (EEPROM_NAERR | EEPROM_LAERR | EEPROM_MSS)) {
> > + dev_err_idt(pdev, "Communication with EEPROM failed");
> > + ret = -EREMOTEIO;
> > + goto err_mutex_unlock;
> > + }
> > + if (eeseq.data != data[idx]) {
> > + dev_err_idt(pdev,
> > + "Values don't match 0x%02hhx != 0x%02hhx",
> > + eeseq.data, data[idx]);
> > + ret = -EREMOTEIO;
> > + goto err_mutex_unlock;
> > + }
> > +
> > + /* Unlock IDT SMBus device */
> > +err_mutex_unlock:
> > + mutex_unlock(&pdev->smb_mtx);
> > + if (ret != SUCCESS)
> > + return ret;
> > + }
> > +
> > + return SUCCESS;
> > +}
> > +
> > +/*
> > + * idt_eeprom_read() - EEPROM read operation
> > + * @pdev: Pointer to the driver data
> > + * @memaddr: Start EEPROM memory address
> > + * @len: Length of data to read
> > + * @buf: Buffer to read data to
> > + */
> > +static int idt_eeprom_read(struct idt_89hpesx_dev *pdev, u16 memaddr, u16 len,
> > + u8 *buf)
> > +{
> > + struct idt_eeprom_seq eeseq;
> > + struct idt_smb_seq smbseq;
> > + u16 idx;
> > + int ret;
> > +
> > + /* Initialize SMBus sequence fields */
> > + smbseq.ccode = pdev->iniccode | CCODE_EEPROM;
> > + smbseq.data = (u8 *)&eeseq;
> > +
> > + /* Send data one-by-one, checking if it is successfully written */
> > + for (idx = 0; idx < len; idx++, memaddr++) {
> > + /* Lock IDT SMBus device */
> > + mutex_lock(&pdev->smb_mtx);
> > +
> > + /* Send EEPROM memory address to read data from */
> > + smbseq.bytecnt = EEPROM_WRRD_CNT;
> > + eeseq.cmd = pdev->inieecmd | EEPROM_OP_READ;
> > + eeseq.eeaddr = pdev->eeaddr;
> > + eeseq.memaddr = cpu_to_le16(memaddr);
> > + ret = pdev->smb_write(pdev, &smbseq);
> > + if (ret != SUCCESS) {
> > + dev_err_idt(pdev, "Failed to init mem address 0x%02hhx",
> > + memaddr);
> > + goto err_mutex_unlock;
> > + }
> > +
> > + /* Perform read operation (rest of fields stay the same) */
> > + smbseq.bytecnt = EEPROM_RD_CNT;
> > + ret = pdev->smb_read(pdev, &smbseq);
> > + if (ret != SUCCESS) {
> > + dev_err_idt(pdev,
> > + "Failed to read eeprom address 0x%02hhx",
> > + memaddr);
> > + ret = -EREMOTEIO;
> > + goto err_mutex_unlock;
> > + }
> > +
> > + /* Check whether IDT successfully read data from EEPROM */
> > + if (eeseq.cmd & (EEPROM_NAERR | EEPROM_LAERR | EEPROM_MSS)) {
> > + dev_err_idt(pdev, "Communication with eeprom failed");
> > + ret = -EREMOTEIO;
> > + goto err_mutex_unlock;
> > + }
> > +
> > + /* Save retrieved data */
> > + buf[idx] = eeseq.data;
> > +
> > + /* Unlock IDT SMBus device */
> > +err_mutex_unlock:
> > + mutex_unlock(&pdev->smb_mtx);
> > + if (ret != SUCCESS)
> > + return ret;
> > + }
> > +
> > + return SUCCESS;
> > +}
> > +
> > +/*===========================================================================
> > + * CSR IO-operations
> > + *===========================================================================
> > + */
> > +
> > +/*
> > + * idt_csr_write() - CSR write operation
> > + * @pdev: Pointer to the driver data
> > + * @csraddr: CSR address (with no two LS bits)
> > + * @data: Data to be written to CSR
> > + */
> > +static int idt_csr_write(struct idt_89hpesx_dev *pdev, u16 csraddr,
> > + const u32 data)
> > +{
> > + struct idt_csr_seq csrseq;
> > + struct idt_smb_seq smbseq;
> > + int ret;
> > +
> > + /* Initialize SMBus sequence fields */
> > + smbseq.ccode = pdev->iniccode | CCODE_CSR;
> > + smbseq.data = (u8 *)&csrseq;
> > +
> > + /* Lock IDT SMBus device */
> > + mutex_lock(&pdev->smb_mtx);
> > +
> > + /* Perform write operation */
> > + smbseq.bytecnt = CSR_WR_CNT;
> > + csrseq.cmd = pdev->inicsrcmd | CSR_OP_WRITE;
> > + csrseq.csraddr = cpu_to_le16(csraddr);
> > + csrseq.data = cpu_to_le32(data);
> > + ret = pdev->smb_write(pdev, &smbseq);
> > + if (ret != SUCCESS) {
> > + dev_err_idt(pdev, "Failed to write 0x%04x: 0x%04x to csr",
> > + CSR_REAL_ADDR(csraddr), data);
> > + goto err_mutex_unlock;
> > + }
> > +
> > + /* Send CSR address to read data from */
> > + smbseq.bytecnt = CSR_WRRD_CNT;
> > + csrseq.cmd = pdev->inicsrcmd | CSR_OP_READ;
> > + ret = pdev->smb_write(pdev, &smbseq);
> > + if (ret != SUCCESS) {
> > + dev_err_idt(pdev, "Failed to init csr address 0x%04x",
> > + CSR_REAL_ADDR(csraddr));
> > + goto err_mutex_unlock;
> > + }
> > +
> > + /* Perform read operation */
> > + smbseq.bytecnt = CSR_RD_CNT;
> > + ret = pdev->smb_read(pdev, &smbseq);
> > + if (ret != SUCCESS) {
> > + dev_err_idt(pdev, "Failed to read csr 0x%04x",
> > + CSR_REAL_ADDR(csraddr));
> > + goto err_mutex_unlock;
> > + }
> > +
> > + /* Check whether IDT successfully retrieved CSR data */
> > + if (csrseq.cmd & (CSR_RERR | CSR_WERR)) {
> > + dev_err_idt(pdev, "IDT failed to perform CSR r/w");
> > + ret = -EREMOTEIO;
> > + goto err_mutex_unlock;
> > + }
> > +
> > + /* Unlock IDT SMBus device */
> > +err_mutex_unlock:
> > + mutex_unlock(&pdev->smb_mtx);
> > +
> > + return ret;
> > +}
> > +
> > +/*
> > + * idt_csr_read() - CSR read operation
> > + * @pdev: Pointer to the driver data
> > + * @csraddr: CSR address (with no two LS bits)
> > + * @data: Data to be written to CSR
> > + */
> > +static int idt_csr_read(struct idt_89hpesx_dev *pdev, u16 csraddr, u32 *data)
> > +{
> > + struct idt_csr_seq csrseq;
> > + struct idt_smb_seq smbseq;
> > + int ret;
> > +
> > + /* Initialize SMBus sequence fields */
> > + smbseq.ccode = pdev->iniccode | CCODE_CSR;
> > + smbseq.data = (u8 *)&csrseq;
> > +
> > + /* Lock IDT SMBus device */
> > + mutex_lock(&pdev->smb_mtx);
> > +
> > + /* Send CSR register address before reading it */
> > + smbseq.bytecnt = CSR_WRRD_CNT;
> > + csrseq.cmd = pdev->inicsrcmd | CSR_OP_READ;
> > + csrseq.csraddr = cpu_to_le16(csraddr);
> > + ret = pdev->smb_write(pdev, &smbseq);
> > + if (ret != SUCCESS) {
> > + dev_err_idt(pdev, "Failed to init csr address 0x%04x",
> > + CSR_REAL_ADDR(csraddr));
> > + goto err_mutex_unlock;
> > + }
> > +
> > + /* Perform read operation */
> > + smbseq.bytecnt = CSR_RD_CNT;
> > + ret = pdev->smb_read(pdev, &smbseq);
> > + if (ret != SUCCESS) {
> > + dev_err_idt(pdev, "Failed to read csr 0x%04hx",
> > + CSR_REAL_ADDR(csraddr));
> > + goto err_mutex_unlock;
> > + }
> > +
> > + /* Check whether IDT successfully retrieved CSR data */
> > + if (csrseq.cmd & (CSR_RERR | CSR_WERR)) {
> > + dev_err_idt(pdev, "IDT failed to perform CSR r/w");
> > + ret = -EREMOTEIO;
> > + goto err_mutex_unlock;
> > + }
> > +
> > + /* Save data retrieved from IDT */
> > + *data = le32_to_cpu(csrseq.data);
> > +
> > + /* Unlock IDT SMBus device */
> > +err_mutex_unlock:
> > + mutex_unlock(&pdev->smb_mtx);
> > +
> > + return ret;
> > +}
> > +
> > +/*===========================================================================
> > + * Sysfs-nodes IO-operations
> > + *===========================================================================
> > + */
> > +
> > +/*
> > + * idt_sysfs_eeprom_write() - EEPROM sysfs-node write callback
> > + * @filep: Pointer to the file system node
> > + * @kobj: Pointer to the kernel object related to the sysfs-node
> > + * @attr: Attributes of the file
> > + * @buf: Buffer to write data to
> > + * @off: Offset at which data should be written to
> > + * @count: Number of bytes to write
> > + */
> > +static ssize_t idt_sysfs_eeprom_write(struct file *filp, struct kobject *kobj,
> > + struct bin_attribute *attr,
> > + char *buf, loff_t off, size_t count)
> > +{
> > + struct idt_89hpesx_dev *pdev;
> > + int ret;
> > +
> > + /* Retrieve driver data */
> > + pdev = to_pdev_kobj(kobj);
> > +
> > + /* Perform EEPROM write operation */
> > + ret = idt_eeprom_write(pdev, (u16)off, (u16)count, (u8 *)buf);
> > + return (ret != SUCCESS ? ret : count);
> > +}
> > +
> > +/*
> > + * idt_sysfs_eeprom_read() - EEPROM sysfs-node read callback
> > + * @filep: Pointer to the file system node
> > + * @kobj: Pointer to the kernel object related to the sysfs-node
> > + * @attr: Attributes of the file
> > + * @buf: Buffer to write data to
> > + * @off: Offset at which data should be written to
> > + * @count: Number of bytes to write
> > + */
> > +static ssize_t idt_sysfs_eeprom_read(struct file *filp, struct kobject *kobj,
> > + struct bin_attribute *attr,
> > + char *buf, loff_t off, size_t count)
> > +{
> > + struct idt_89hpesx_dev *pdev;
> > + int ret;
> > +
> > + /* Retrieve driver data */
> > + pdev = to_pdev_kobj(kobj);
> > +
> > + /* Perform EEPROM read operation */
> > + ret = idt_eeprom_read(pdev, (u16)off, (u16)count, (u8 *)buf);
> > + return (ret != SUCCESS ? ret : count);
> > +}
> > +
> > +/*
> > + * idt_sysfs_csr_store() - CSR sysfs-node write callback
> > + * @kobj: Pointer to the kernel object related to the sysfs-node
> > + * @attr: Attributes of the file
> > + * @buf: Buffer to write data to
> > + * @count: Size of the buffer
> > + *
> > + * It accepts either "0x<reg addr>:0x<value>" for saving register address
> > + * and writing value to specified DWORD register or "0x<reg addr>" for
> > + * just saving register address in order to perform next read operation.
> > + *
> > + * WARNING No spaces are allowed. Incoming string must be strictly formated as:
> > + * "<reg addr>:<value>". Register address must be aligned within 4 bytes
> > + * (one DWORD).
> > + */
> > +static ssize_t idt_sysfs_csr_store(struct device *dev,
> > + struct device_attribute *attr,
> > + const char *buf, size_t count)
> > +{
> > + struct idt_89hpesx_dev *pdev;
> > + char *colon_ch, *csraddr_str, *csrval_str;
> > + int ret, csraddr_len, csrval_len;
> > + u32 csraddr, csrval;
> > +
> > + /* Retrieve driver data */
> > + pdev = dev_get_drvdata(dev);
> > +
> > + /* Find position of colon in the buffer */
> > + colon_ch = strnchr(buf, count, ':');
> > +
> > + /*
> > + * If there is colon passed then new CSR value should be parsed as
> > + * well, so allocate buffer for CSR address substring.
> > + * If no colon is found, then string must have just one number with
> > + * no new CSR value
> > + */
> > + if (colon_ch != NULL) {
> > + csraddr_len = colon_ch - buf;
> > + csraddr_str =
> > + kmalloc(sizeof(char)*(csraddr_len + 1), GFP_KERNEL);
> > + if (csraddr_str == NULL)
> > + return -ENOMEM;
> > + /* Copy the register address to the substring buffer */
> > + strncpy(csraddr_str, buf, csraddr_len);
> > + csraddr_str[csraddr_len] = '\0';
> > + /* Register value must follow the colon */
> > + csrval_str = colon_ch + 1;
> > + csrval_len = strnlen(csrval_str, count - csraddr_len);
> > + } else /* if (str_colon == NULL) */ {
> > + csraddr_str = (char *)buf; /* Just to shut warning up */
> > + csraddr_len = strnlen(csraddr_str, count);
> > + csrval_str = NULL;
> > + csrval_len = 0;
> > + }
> > +
> > + /* Convert CSR address to u32 value */
> > + ret = kstrtou32(csraddr_str, 0, &csraddr);
> > + if (ret != SUCCESS)
> > + goto free_csraddr_str;
> > +
> > + /* Check whether passed register address is valid */
> > + if (csraddr > CSR_MAX || !IS_ALIGNED(csraddr, SZ_4)) {
> > + ret = -EINVAL;
> > + goto free_csraddr_str;
> > + }
> > +
> > + /* Shift register address to the right so to have u16 address */
> > + csraddr >>= 2;
> > +
> > + /* Parse new CSR value and send it to IDT, if colon has been found */
> > + if (colon_ch != NULL) {
> > + ret = kstrtou32(csrval_str, 0, &csrval);
> > + if (ret != SUCCESS)
> > + goto free_csraddr_str;
> > +
> > + ret = idt_csr_write(pdev, (u16)csraddr, csrval);
> > + if (ret != SUCCESS)
> > + goto free_csraddr_str;
> > + }
> > +
> > + /* Save CSR address in the data structure for future read operations */
> > + atomic_set(&pdev->csr, (int)csraddr);
> > +
> > + /* Free memory only if colon has been found */
> > +free_csraddr_str:
> > + if (colon_ch != NULL)
> > + kfree(csraddr_str);
> > +
> > + return (ret != SUCCESS ? ret : count);
> > +}
> > +
> > +/*
> > + * idt_sysfs_csr_show() - CSR sysfs-node read callback
> > + * @kobj: Pointer to the kernel object related to the sysfs-node
> > + * @attr: Attributes of the file
> > + * @buf: Buffer to write data to
> > + *
> > + * It just prints the pair "0x<reg addr>:0x<value>" to passed buffer.
> > + */
> > +static ssize_t idt_sysfs_csr_show(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + struct idt_89hpesx_dev *pdev;
> > + u32 csraddr, csrval;
> > + int ret;
> > +
> > + /* Retrieve driver data */
> > + pdev = dev_get_drvdata(dev);
> > +
> > + /* Read current CSR address */
> > + csraddr = atomic_read(&pdev->csr);
> > +
> > + /* Perform CSR read operation */
> > + ret = idt_csr_read(pdev, (u16)csraddr, &csrval);
> > + if (ret != SUCCESS)
> > + return ret;
> > +
> > + /* Shift register address to the left so to have real address */
> > + csraddr <<= 2;
> > +
> > + /* Print the "0x<reg addr>:0x<value>" to buffer */
> > + return snprintf(buf, PAGE_SIZE, "0x%05x:0x%08x\n",
> > + (unsigned int)csraddr, (unsigned int)csrval);
> > +}
> > +
> > +/*
> > + * eeprom_attribute - EEPROM sysfs-node attributes
> > + *
> > + * NOTE Size will be changed in compliance with OF node. EEPROM attribute will
> > + * be read-only as well if the corresponding flag is specified in OF node.
> > + */
> > +static struct bin_attribute eeprom_attribute = {
> > + .attr = {
> > + .name = "eeprom",
> > + .mode = S_IRUGO | S_IWUSR
> > + },
> > + .size = EEPROM_DEF_SIZE,
> > + .write = idt_sysfs_eeprom_write,
> > + .read = idt_sysfs_eeprom_read
> > +};
> > +
> > +/*
> > + * csr_attribute - CSR sysfs-node attributes
> > + */
> > +static struct device_attribute csr_attribute = {
> > + .attr = {
> > + .name = "csr",
> > + .mode = S_IRUGO | S_IWUSR
> > + },
> > + .store = idt_sysfs_csr_store,
> > + .show = idt_sysfs_csr_show,
> > +};
>
> Note, for the future, just use DEVICE_ATTR_RW() for stuff like this.
>
> But convert it to debugfs please.
>
> thanks,
>
> greg k-h
Ok. I'll fix all the issues as soon as possible.
^ permalink raw reply
* Re: [PATCH v2] i2c: i2c-mux-gpio: update mux with gpiod_set_array_value_cansleep
From: Wolfram Sang @ 2016-11-24 19:52 UTC (permalink / raw)
To: Peter Rosin; +Cc: linux-kernel, Peter Korsgaard, Wolfram Sang, linux-i2c
In-Reply-To: <46be889f-a558-0174-f638-c685b23c8ed9@axentia.se>
[-- Attachment #1: Type: text/plain, Size: 308 bytes --]
> Yes, I never liked that include, but I don't know how to get
> from struct gpio_desc * to the relevant struct device *
> without it...
Looks to me like we should ask the GPIO maintainers if they are willing
to export this value? I am probably not as good as you in explaining the
details why, though...
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* RE: [patch v9 1/1] i2c: add master driver for mellanox systems
From: Vadim Pasternak @ 2016-11-24 14:55 UTC (permalink / raw)
To: Wolfram Sang
Cc: wsa@the-dreams.de, linux-i2c@vger.kernel.org,
linux-kernel@vger.kernel.org, jiri@resnulli.us, Michael Shych
In-Reply-To: <20161124145201.GD4271@katana>
> -----Original Message-----
> From: Wolfram Sang [mailto:wsa-dev@sang-engineering.com]
> Sent: Thursday, November 24, 2016 4:52 PM
> To: Vadim Pasternak <vadimp@mellanox.com>
> Cc: wsa@the-dreams.de; linux-i2c@vger.kernel.org; linux-
> kernel@vger.kernel.org; jiri@resnulli.us; Michael Shych
> <michaelsh@mellanox.com>
> Subject: Re: [patch v9 1/1] i2c: add master driver for mellanox systems
>
> On Sun, Nov 20, 2016 at 04:56:14PM +0000, vadimp@mellanox.com wrote:
> > From: Vadim Pasternak <vadimp@mellanox.com>
> >
> > Device driver for Mellanox I2C controller logic, implemented in
> > Lattice CPLD device.
> > Device supports:
> > - Master mode
> > - One physical bus
> > - Polling mode
> >
> > The Kconfig currently controlling compilation of this code is:
> > drivers/i2c/busses/Kconfig:config I2C_MLXCPLD
> >
> > Signed-off-by: Michael Shych <michaelsh@mellanox.com>
> > Signed-off-by: Vadim Pasternak <vadimp@mellanox.com>
> > Reviewed-by: Jiri Pirko <jiri@mellanox.com>
> > Reviewed-by: Vladimir Zapolskiy <vz@mleia.com>
>
> Applied to for-next, thanks!
Thank you very,
Vadim.
^ permalink raw reply
* Re: Getting at gpio- and pinctrl-devices as a consumer
From: Linus Walleij @ 2016-11-24 23:29 UTC (permalink / raw)
To: Peter Rosin
Cc: Alexandre Courbot, Wolfram Sang, linux-kernel@vger.kernel.org,
Peter Korsgaard, Wolfram Sang, linux-i2c@vger.kernel.org,
linux-gpio@vger.kernel.org
In-Reply-To: <b481ed92-c9d2-b4da-b484-6d8bec6038f0@axentia.se>
On Thu, Nov 24, 2016 at 10:35 PM, Peter Rosin <peda@axentia.se> wrote:
> The background is that the gpio- and pinctrl-based i2c-mux drivers
> need to know if the device that is used to control the mux of the
> i2c-bus is also sitting on that very same i2c-bus. If it is, the
> locking has to be different and a bit more relaxed. This relaxed
> mode cannot be used always, as that would change the mux behavior
> in an unacceptable way for stuff expecting the (traditional)
> stricter locking. See Documentation/i2c/i2c-topology for more
> details if you need it.
>
> To check this, the i2c mux drivers dig out the device connected to
> each gpio-pin (or pinctrl-state) and walks up the device tree to see
> if the root i2c adapter that is muxed is in the loop.
>
> When I wrote this code, I could not find a clean way to go from a
> struct gpio_desc * to the relevant device, short of doing
>
> #include "../../gpio/gpiolib.h"
>
> gpio_dev = &gpio_desc->gdev->dev;
>
> And similarly for pinctrl:
>
> #include "../../pinctrl/core.h"
>
> struct pinctrl_setting *setting;
> pinctrl_dev = setting->pctldev->dev;
>
> I'm not very proud of that, and wonder if there is a better way
> to get at the needed struct device? If not, then perhaps there
> should be?
Surely if I can be convinced that we need helpers for this
in GPIO and/or pin control we can add them.
They just need to be named something reasonable and
be generally useful for other situations of similar nature.
struct device *gpiod_get_backing_device(struct gpio_desc *d);
Is simple but is it really what you want?
Yours,
Linus Walleij
^ permalink raw reply
* Getting at gpio- and pinctrl-devices as a consumer
From: Peter Rosin @ 2016-11-24 21:35 UTC (permalink / raw)
To: Linus Walleij, Alexandre Courbot
Cc: Wolfram Sang, linux-kernel, Peter Korsgaard, Wolfram Sang,
linux-i2c, linux-gpio
In-Reply-To: <20161124195230.GA1666@katana>
On 2016-11-24 20:52, Wolfram Sang wrote:
>> Yes, I never liked that include, but I don't know how to get
>> from struct gpio_desc * to the relevant struct device *
>> without it...
>
> Looks to me like we should ask the GPIO maintainers if they are willing
> to export this value? I am probably not as good as you in explaining the
> details why, though...
Right. Adding Linus, Alexandre and the gpio list.
The background is that the gpio- and pinctrl-based i2c-mux drivers
need to know if the device that is used to control the mux of the
i2c-bus is also sitting on that very same i2c-bus. If it is, the
locking has to be different and a bit more relaxed. This relaxed
mode cannot be used always, as that would change the mux behavior
in an unacceptable way for stuff expecting the (traditional)
stricter locking. See Documentation/i2c/i2c-topology for more
details if you need it.
To check this, the i2c mux drivers dig out the device connected to
each gpio-pin (or pinctrl-state) and walks up the device tree to see
if the root i2c adapter that is muxed is in the loop.
When I wrote this code, I could not find a clean way to go from a
struct gpio_desc * to the relevant device, short of doing
#include "../../gpio/gpiolib.h"
gpio_dev = &gpio_desc->gdev->dev;
And similarly for pinctrl:
#include "../../pinctrl/core.h"
struct pinctrl_setting *setting;
pinctrl_dev = setting->pctldev->dev;
I'm not very proud of that, and wonder if there is a better way
to get at the needed struct device? If not, then perhaps there
should be?
Cheers,
Peter
^ permalink raw reply
* Re: Getting at gpio- and pinctrl-devices as a consumer
From: Peter Rosin @ 2016-11-25 9:24 UTC (permalink / raw)
To: Linus Walleij
Cc: Alexandre Courbot, Wolfram Sang, linux-kernel@vger.kernel.org,
Peter Korsgaard, Wolfram Sang, linux-i2c@vger.kernel.org,
linux-gpio@vger.kernel.org
In-Reply-To: <CACRpkdb0cdvxgnuOBNfdY+idVcTXMwzKRYfCsFpiRx2qaVrt=w@mail.gmail.com>
On 2016-11-25 00:29, Linus Walleij wrote:
> On Thu, Nov 24, 2016 at 10:35 PM, Peter Rosin <peda@axentia.se> wrote:
>
>> The background is that the gpio- and pinctrl-based i2c-mux drivers
>> need to know if the device that is used to control the mux of the
>> i2c-bus is also sitting on that very same i2c-bus. If it is, the
>> locking has to be different and a bit more relaxed. This relaxed
>> mode cannot be used always, as that would change the mux behavior
>> in an unacceptable way for stuff expecting the (traditional)
>> stricter locking. See Documentation/i2c/i2c-topology for more
>> details if you need it.
>>
>> To check this, the i2c mux drivers dig out the device connected to
>> each gpio-pin (or pinctrl-state) and walks up the device tree to see
>> if the root i2c adapter that is muxed is in the loop.
>>
>> When I wrote this code, I could not find a clean way to go from a
>> struct gpio_desc * to the relevant device, short of doing
>>
>> #include "../../gpio/gpiolib.h"
>>
>> gpio_dev = &gpio_desc->gdev->dev;
>>
>> And similarly for pinctrl:
>>
>> #include "../../pinctrl/core.h"
>>
>> struct pinctrl_setting *setting;
>> pinctrl_dev = setting->pctldev->dev;
>>
>> I'm not very proud of that, and wonder if there is a better way
>> to get at the needed struct device? If not, then perhaps there
>> should be?
>
> Surely if I can be convinced that we need helpers for this
> in GPIO and/or pin control we can add them.
>
> They just need to be named something reasonable and
> be generally useful for other situations of similar nature.
>
> struct device *gpiod_get_backing_device(struct gpio_desc *d);
>
> Is simple but is it really what you want?
Well, my first attempt was to simply have a property in the
devicetree stating that the mux was controlled from the same
i2c bus it was muxing, but that was shot down because it
should be possible to deduce this from the implementation (or
something of that meaning, it was a while ago), which to me
meant examining the "struct device"-tree.
For the gpio_desc it is easy. However, it is worse for the
pinctrl case. The pinctrl consumer currently deals in states,
but each state has a number of pinctrl_settings and it is
these settings that are backed by a device implementing that
state. It is in other words possibly several devices involved
with one pinctrl_state. This can be handled in in three ways
that spring to mind.
1. Return a single device connected to a pinctrl state, if it
is the same device backing all settings, and error out if more
than one device is involved. I mean, how silly would it me to
control a mux from pins not controlled by the same pinctrl?
That just seems extremely odd, like one pin each on two
different i2c-controlled io-expanders sitting on the same
i2c-bus that is also muxed using these two pins. The risk for
regression because of this should be ... low.
2. Return an array of devices, one for each pinctrl setting
connected to the state. This also needs to expose the number
of settings for a state.
3. Introduce some kind of list_for_each_entry wrapper (or
something) so that the consumer can iterate over the settings
connected to a state, coupled with a function to get the
struct device from a setting.
#1 is not as generally useful as #2 or #3, as it assumes
that the corner case is not interesting. But it might very
well be the case that the next user is looking for exactly
this corner case. But at the same time, the next user isn't
here yet, at least not that I know of...
#2 and #3 exposes more of the internals to the consumer, since
the consumer currently does not need to worry about neither
the number of settings connected to a state nor that fact
that there are settings at all. #3 would be closest to the
current implementation in i2c-mux-pinctrl (included below).
So, for the case in question I deem #1 good enough, and it
exposes less of the internals. But #2 or #3 might be better for
the next case, and also makes the current case not risk the
unlikely regression. Any preference?
Cheers,
Peter
static struct i2c_adapter *i2c_mux_pinctrl_root_adapter(
struct pinctrl_state *state)
{
struct i2c_adapter *root = NULL;
struct pinctrl_setting *setting;
struct i2c_adapter *pin_root;
list_for_each_entry(setting, &state->settings, node) {
pin_root = i2c_root_adapter(setting->pctldev->dev);
if (!pin_root)
return NULL;
if (!root)
root = pin_root;
else if (root != pin_root)
return NULL;
}
return root;
}
^ permalink raw reply
* Re: Getting at gpio- and pinctrl-devices as a consumer
From: Linus Walleij @ 2016-11-25 13:39 UTC (permalink / raw)
To: Peter Rosin
Cc: Alexandre Courbot, Wolfram Sang, linux-kernel@vger.kernel.org,
Peter Korsgaard, Wolfram Sang, linux-i2c@vger.kernel.org,
linux-gpio@vger.kernel.org
In-Reply-To: <45787444-a237-7363-5f03-0cd207cb82c2@axentia.se>
On Fri, Nov 25, 2016 at 10:24 AM, Peter Rosin <peda@axentia.se> wrote:
>[Me]
>> struct device *gpiod_get_backing_device(struct gpio_desc *d);
>>
>> Is simple but is it really what you want?
>
> Well, my first attempt was to simply have a property in the
> devicetree stating that the mux was controlled from the same
> i2c bus it was muxing, but that was shot down because it
> should be possible to deduce this from the implementation (or
> something of that meaning, it was a while ago), which to me
> meant examining the "struct device"-tree.
The problem goes into any subsystem providing resources for
a mux in this case, generally for example it is not OK for a
device to runtime suspend or shut down its regulator or turn off
its clock if it is acting as a mux. GPIO and pin control just happens
to be two resource in this specific case.
> For the gpio_desc it is easy. However, it is worse for the
> pinctrl case.
It is annoying to do this in a sense, because it starts to kill
the abstraction we have created exactly in order to avoid
consumers having to worry much about their providers
internals. No we are opening the can and letting the stuff
out all over the place.
Have you looked into the discussion about device dependencies
in general? Isn't this problem mappable as a subset of that?
This was discussed at length at the last kernel summit:
https://lwn.net/Articles/705852/
See especially Rafael's commit
9ed9895370aedd6032af2a9181c62c394d08223b
to driver core in linux-next
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH] Revert "i2c: designware: do not disable adapter after transfer"
From: Jarkko Nikula @ 2016-11-25 15:22 UTC (permalink / raw)
To: linux-i2c
Cc: Wolfram Sang, Andy Shevchenko, Mika Westerberg,
Srinivas Pandruvada, Jarkko Nikula, Lucas De Marchi
This reverts commit 0317e6c0f1dc1ba86b8d9dccc010c5e77b8355fa.
Srinivas reported recently touchscreen and touchpad stopped working in
Haswell based machine in Linux 4.9-rc series with timeout errors from
i2c_designware:
[ 16.508013] i2c_designware INT33C3:00: controller timed out
[ 16.508302] i2c_hid i2c-MSFT0001:02: failed to change power setting.
[ 17.532016] i2c_designware INT33C3:00: controller timed out
[ 18.556022] i2c_designware INT33C3:00: controller timed out
[ 18.556315] i2c_hid i2c-ATML1000:00: failed to retrieve report from device.
I managed to reproduce similar errors on another Haswell based machine
where touchscreen initialization fails maybe in every 1/5 - 1/2 boots.
Since root cause for these errors is not clear yet and debugging is
ongoing it's better to revert this commit as we are near to release.
Reported-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
---
Also https://bugzilla.kernel.org/show_bug.cgi?id=188501 may relate to
that but that's not confirmed yet with the reporter does revert help.
---
drivers/i2c/busses/i2c-designware-core.c | 55 +++++++++++---------------------
1 file changed, 18 insertions(+), 37 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
index c53058d6139c..b403fa5ecf49 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -91,9 +91,7 @@
DW_IC_INTR_TX_ABRT | \
DW_IC_INTR_STOP_DET)
-#define DW_IC_STATUS_ACTIVITY 0x1
-#define DW_IC_STATUS_TFE BIT(2)
-#define DW_IC_STATUS_MST_ACTIVITY BIT(5)
+#define DW_IC_STATUS_ACTIVITY 0x1
#define DW_IC_SDA_HOLD_RX_SHIFT 16
#define DW_IC_SDA_HOLD_RX_MASK GENMASK(23, DW_IC_SDA_HOLD_RX_SHIFT)
@@ -478,25 +476,9 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
{
struct i2c_msg *msgs = dev->msgs;
u32 ic_tar = 0;
- bool enabled;
- enabled = dw_readl(dev, DW_IC_ENABLE_STATUS) & 1;
-
- if (enabled) {
- u32 ic_status;
-
- /*
- * Only disable adapter if ic_tar and ic_con can't be
- * dynamically updated
- */
- ic_status = dw_readl(dev, DW_IC_STATUS);
- if (!dev->dynamic_tar_update_enabled ||
- (ic_status & DW_IC_STATUS_MST_ACTIVITY) ||
- !(ic_status & DW_IC_STATUS_TFE)) {
- __i2c_dw_enable_and_wait(dev, false);
- enabled = false;
- }
- }
+ /* Disable the adapter */
+ __i2c_dw_enable_and_wait(dev, false);
/* if the slave address is ten bit address, enable 10BITADDR */
if (dev->dynamic_tar_update_enabled) {
@@ -526,8 +508,8 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
/* enforce disabled interrupts (due to HW issues) */
i2c_dw_disable_int(dev);
- if (!enabled)
- __i2c_dw_enable(dev, true);
+ /* Enable the adapter */
+ __i2c_dw_enable(dev, true);
/* Clear and enable interrupts */
dw_readl(dev, DW_IC_CLR_INTR);
@@ -708,8 +690,7 @@ static int i2c_dw_handle_tx_abort(struct dw_i2c_dev *dev)
}
/*
- * Prepare controller for a transaction and start transfer by calling
- * i2c_dw_xfer_init()
+ * Prepare controller for a transaction and call i2c_dw_xfer_msg
*/
static int
i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
@@ -752,6 +733,16 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
goto done;
}
+ /*
+ * We must disable the adapter before returning and signaling the end
+ * of the current transfer. Otherwise the hardware might continue
+ * generating interrupts which in turn causes a race condition with
+ * the following transfer. Needs some more investigation if the
+ * additional interrupts are a hardware bug or this driver doesn't
+ * handle them correctly yet.
+ */
+ __i2c_dw_enable(dev, false);
+
if (dev->msg_err) {
ret = dev->msg_err;
goto done;
@@ -893,19 +884,9 @@ static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
*/
tx_aborted:
- if ((stat & (DW_IC_INTR_TX_ABRT | DW_IC_INTR_STOP_DET))
- || dev->msg_err) {
- /*
- * We must disable interruts before returning and signaling
- * the end of the current transfer. Otherwise the hardware
- * might continue generating interrupts for non-existent
- * transfers.
- */
- i2c_dw_disable_int(dev);
- dw_readl(dev, DW_IC_CLR_INTR);
-
+ if ((stat & (DW_IC_INTR_TX_ABRT | DW_IC_INTR_STOP_DET)) || dev->msg_err)
complete(&dev->cmd_complete);
- } else if (unlikely(dev->accessor_flags & ACCESS_INTR_MASK)) {
+ else if (unlikely(dev->accessor_flags & ACCESS_INTR_MASK)) {
/* workaround to trigger pending interrupt */
stat = dw_readl(dev, DW_IC_INTR_MASK);
i2c_dw_disable_int(dev);
--
2.10.2
^ permalink raw reply related
* Re: Getting at gpio- and pinctrl-devices as a consumer
From: Peter Rosin @ 2016-11-25 16:19 UTC (permalink / raw)
To: Linus Walleij
Cc: Alexandre Courbot, Wolfram Sang, linux-kernel@vger.kernel.org,
Peter Korsgaard, Wolfram Sang, linux-i2c@vger.kernel.org,
linux-gpio@vger.kernel.org
In-Reply-To: <CACRpkdbRiT9ZqG23ewTHBDRdHT=1E2ei1qoLUOGr_jd47MNvog@mail.gmail.com>
On 2016-11-25 14:39, Linus Walleij wrote:
> On Fri, Nov 25, 2016 at 10:24 AM, Peter Rosin <peda@axentia.se> wrote:
>> [Me]
>>> struct device *gpiod_get_backing_device(struct gpio_desc *d);
>>>
>>> Is simple but is it really what you want?
>>
>> Well, my first attempt was to simply have a property in the
>> devicetree stating that the mux was controlled from the same
>> i2c bus it was muxing, but that was shot down because it
>> should be possible to deduce this from the implementation (or
>> something of that meaning, it was a while ago), which to me
>> meant examining the "struct device"-tree.
>
> The problem goes into any subsystem providing resources for
> a mux in this case, generally for example it is not OK for a
> device to runtime suspend or shut down its regulator or turn off
> its clock if it is acting as a mux. GPIO and pin control just happens
> to be two resource in this specific case.
Just to be clear, we are only talking of muxers used to mux an
i2c-bus. But if some other popular bus is muxed the problems would
probably be similar.
If there is any action (that is not aware of the mux) on the muxed
i2c-bus as the result of a i2c-mux-foo driver requesting a change to
update the mux state, this will deadlock. Unless of course the mux
is operated with the more relaxed locking (mux-locked instead of
parent-locked). So yes, if some multi-function chip provides a
gpio pin to some i2c-mux, and if this chip for some reason
triggers some strange action when a gpio change is requested, and
this action in turn triggers a transfer on the muxed i2c-bus, it
will not work as intended (it will deadlock). I.e., there are all
kinds of ways to defeat the current check to determine if
i2c-mux-gpio and i2c-mux-pinctrl need relaxed locking. But none
or them worked in this setting before I introduced the more relaxed
mux-locked mode either, so I do not feel all that bad about it.
And the checks can of course be improved if needed.
>> For the gpio_desc it is easy. However, it is worse for the
>> pinctrl case.
>
> It is annoying to do this in a sense, because it starts to kill
> the abstraction we have created exactly in order to avoid
> consumers having to worry much about their providers
> internals. No we are opening the can and letting the stuff
> out all over the place.
Sorry about that. If it matters I'd be just as happy to have the
devicetree author declare the situation explicitly for the i2c-mux
driver, and I didn't really try to argue for that. Maybe it can
be argued that the needed little piece of info isn't really all that
specific to the linux implementation?
Let me try:
-----------8<-----------
It is unreasonable to require that the i2c-mux code can determine
if the devices needed to operate the mux needs to use the i2c-bus
that is muxed. This requires domain knowledge of the whole system,
and fingers everywhere. There should not be a design restriction
on the system such that this question must have answer.
Assuming everybody agrees with the above, the question becomes: Is
it required that the i2c-mux code knows if any devices needed to
operate the mux needs to use the i2c-bus that is muxed?
I say yes.
Case one: Normal iomem gpios directly from some SOC are used to
mux an i2c-bus. If the i2c-bus is not locked as these gpios are
updated, you might see half a transfer on one i2c child channel,
and the other half on some other channel. Or worse. This might
wreak havoc among the client devices. I.e. the i2c-mux must in
this case lock the i2c-bus during the gpio update to make sure
nothing bad hits the clients devices.
Case two: A gpio update might need to use the muxed i2c-bus. The
simplest case is if the io-expander used to operate an i2c-mux sits
on the same i2c-bus that it is muxing, but it might be a lot more
complex with a chain of dependencies ending up causing an i2c
transfer (also, the client devices are known to handle broken crap
on the i2c-bus and are not affected by a little bit of garbage).
I.e. the i2c-mux must in this case make sure to not lock the
i2c-bus during the gpio update.
In other words, without a full view of the system, it is not
possible to know if the i2c-bus should or should not be locked
when updating the gpios used to operate a mux on the i2c-bus.
-----------8<-----------
I can take the above argument to the dt list and perhaps win them
over. But that doesn't mean that we can simply remove the code,
since doing so would regress anything depending on the current
behavior of (at least sometimes) automatically finding the i2c
dependency. But maybe the users are few and able to add a dt
property in case it's needed?
And I like code that works without configuration, so I would
prefer it to remain and continue to discover the easy case.
> Have you looked into the discussion about device dependencies
> in general? Isn't this problem mappable as a subset of that?
Haven't seen it before. My understanding of the driver model is
not the best, and the docs are behind which makes it hard to
know what to believe. The discussions generally fly a bit over
my head, I guess I simply need to dig into the code a bit more...
> This was discussed at length at the last kernel summit:
> https://lwn.net/Articles/705852/
>
> See especially Rafael's commit
> 9ed9895370aedd6032af2a9181c62c394d08223b
> to driver core in linux-next
This looks like it could be useful; given a gpio, look up its
device, then do a deep search of the supplier list and look
for any device sitting on the muxed i2c-bus. But of course
consumers might also get a notifier and react. I think it's
next to impossible to automatically cover all cases...
An that still requires the i2c-mux driver to lookup the backing
device of gpios (or device_s_ for pinctrl-states).
Cheers,
Peter
^ permalink raw reply
* Re: [PATCH] Revert "i2c: designware: do not disable adapter after transfer"
From: Wolfram Sang @ 2016-11-25 22:24 UTC (permalink / raw)
To: Jarkko Nikula
Cc: linux-i2c, Wolfram Sang, Andy Shevchenko, Mika Westerberg,
Srinivas Pandruvada, Lucas De Marchi
In-Reply-To: <20161125152227.24396-1-jarkko.nikula@linux.intel.com>
[-- Attachment #1: Type: text/plain, Size: 1182 bytes --]
On Fri, Nov 25, 2016 at 05:22:27PM +0200, Jarkko Nikula wrote:
> This reverts commit 0317e6c0f1dc1ba86b8d9dccc010c5e77b8355fa.
>
> Srinivas reported recently touchscreen and touchpad stopped working in
> Haswell based machine in Linux 4.9-rc series with timeout errors from
> i2c_designware:
>
> [ 16.508013] i2c_designware INT33C3:00: controller timed out
> [ 16.508302] i2c_hid i2c-MSFT0001:02: failed to change power setting.
> [ 17.532016] i2c_designware INT33C3:00: controller timed out
> [ 18.556022] i2c_designware INT33C3:00: controller timed out
> [ 18.556315] i2c_hid i2c-ATML1000:00: failed to retrieve report from device.
>
> I managed to reproduce similar errors on another Haswell based machine
> where touchscreen initialization fails maybe in every 1/5 - 1/2 boots.
> Since root cause for these errors is not clear yet and debugging is
> ongoing it's better to revert this commit as we are near to release.
>
> Reported-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Applied to for-current, thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Xmas Offer
From: Mrs Julie Leach @ 2016-11-26 10:26 UTC (permalink / raw)
To: Recipients
You are a recipient to Mrs Julie Leach Donation of $3 million USD. Contact ( julieleach93@gmail.com ) for claims.
^ permalink raw reply
* Re: [PATCH v4 0/7] mux controller abstraction and iio/i2c muxes
From: Peter Rosin @ 2016-11-26 18:38 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Wolfram Sang, Rob Herring, Mark Rutland, Jonathan Cameron,
Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
Jonathan Corbet, Arnd Bergmann, Greg Kroah-Hartman,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1480000687-5630-1-git-send-email-peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
On 2016-11-24 16:18, Peter Rosin wrote:
> v3 -> v4 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...
>
> v2 -> v3 changes
> - have the mux-controller in the parent node of any mux-controller consumer,
> to hopefully satisfy complaint from Rob about dt complexity.
I did some further tests and both of these attempts to support fancier
devicetree bindings have severe problems. I will remove them for v5 and
go back to having a phandle reference to the mux-controller from the
consumer (unless I get some revelation of course and just get it). I'm
simply not yet understanding the driver model well enough to pull this
off at the moment...
Cheers,
Peter
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox