* Re: [PATCH 4/4] i2c: octeon: thunderx: Add I2C_CLASS_HWMON
From: Wolfram Sang @ 2017-01-25 20:49 UTC (permalink / raw)
To: Jan Glauber
Cc: Wolfram Sang, Paul Burton, Steven J . Hill, linux-i2c, linux-mips,
David Daney
In-Reply-To: <20161211220434.GH2552@katana>
[-- Attachment #1: Type: text/plain, Size: 696 bytes --]
On Sun, Dec 11, 2016 at 11:04:35PM +0100, Wolfram Sang wrote:
> On Fri, Dec 09, 2016 at 10:31:58AM +0100, Jan Glauber wrote:
> > It was reported that ipmi_ssif fails to create the
> > ipmi device on some systems if the adapter class is not containing
> > I2C_CLASS_HWMON. Fix it by setting the class.
> >
> > Reported-by: Vadim Lomovtsev <Vadim.Lomovtsev@caviumnetworks.com>
> > Signed-off-by: Jan Glauber <jglauber@cavium.com>
>
> The intention of adapter classes is to *limit* probing to a certain
> class of devices. If a class is needed to *enable* probing, then
> something there looks wrong. From the details given, this must be solved
> elsewhere I'd say.
Makes sense?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v3] i2c: core: helper function to detect slave mode
From: Wolfram Sang @ 2017-01-25 20:45 UTC (permalink / raw)
To: Luis Oliveira
Cc: robh+dt, mark.rutland, jarkko.nikula, andriy.shevchenko,
mika.westerberg, linux-i2c, devicetree, linux-kernel, vz,
Ramiro.Oliveira, Joao.Pinto, CARLOS.PALMINHA
In-Reply-To: <58edd25a0fbe50b6d5abef14b7e46c63a1e06830.1484663257.git.lolivei@synopsys.com>
[-- Attachment #1: Type: text/plain, Size: 558 bytes --]
> + * i2c_slave_mode_detect - detect operation mode
I'd rather name it 'i2c_detect_slave_mode'
> + * @dev: The device owning the bus
> + *
> + * This checks the device nodes for an I2C slave by checking the address
> + * used.
> + *
> + * Returns true if an I2C slave is detected, otherwise returns false.
Both paragraphs could be a little more explicit. It is not about
"slaves" or "clients" in general, but about those entries which make the
current master act as a slave, too.
The code looks good to me, so we are close to go!
Thanks,
Wolfram
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v10 3/5] ARM: dts: stm32: Add I2C1 support for STM32F429 SoC
From: Wolfram Sang @ 2017-01-25 20:25 UTC (permalink / raw)
To: M'boumba Cedric Madianga
Cc: robh+dt, mcoquelin.stm32, alexandre.torgue, linus.walleij,
patrice.chotard, linux, linux-i2c, devicetree, linux-arm-kernel,
linux-kernel, u.kleine-koenig
In-Reply-To: <1484832316-5594-4-git-send-email-cedric.madianga@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 337 bytes --]
On Thu, Jan 19, 2017 at 02:25:14PM +0100, M'boumba Cedric Madianga wrote:
> This patch adds I2C1 support for STM32F429 SoC
>
> Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
> Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
Note that patches 3-5 should go via stm-tree or arm-soc. Rather not i2c.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v10 2/5] i2c: Add STM32F4 I2C driver
From: Wolfram Sang @ 2017-01-25 20:25 UTC (permalink / raw)
To: M'boumba Cedric Madianga
Cc: robh+dt, mcoquelin.stm32, alexandre.torgue, linus.walleij,
patrice.chotard, linux, linux-i2c, devicetree, linux-arm-kernel,
linux-kernel, u.kleine-koenig
In-Reply-To: <1484832316-5594-3-git-send-email-cedric.madianga@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 346 bytes --]
On Thu, Jan 19, 2017 at 02:25:13PM +0100, M'boumba Cedric Madianga wrote:
> This patch adds support for the STM32F4 I2C controller.
>
> Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
Applied to for-next with Uwe's ack (which looked more like a review to
me, but well...), thanks to all involved for keeping at it!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v10 1/5] dt-bindings: Document the STM32 I2C bindings
From: Wolfram Sang @ 2017-01-25 20:24 UTC (permalink / raw)
To: M'boumba Cedric Madianga
Cc: robh+dt, mcoquelin.stm32, alexandre.torgue, linus.walleij,
patrice.chotard, linux, linux-i2c, devicetree, linux-arm-kernel,
linux-kernel, u.kleine-koenig
In-Reply-To: <1484832316-5594-2-git-send-email-cedric.madianga@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 317 bytes --]
On Thu, Jan 19, 2017 at 02:25:12PM +0100, M'boumba Cedric Madianga wrote:
> This patch adds documentation of device tree bindings for the STM32 I2C
> controller.
>
> Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
> Acked-by: Rob Herring <robh@kernel.org>
Applied to for-next, thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v2 00/13] coordinate cht i2c-pmic and i915-punit accesses
From: Wolfram Sang @ 2017-01-25 20:18 UTC (permalink / raw)
To: Hans de Goede
Cc: Daniel Vetter, Jani Nikula, Ville Syrjälä,
Jarkko Nikula, Len Brown, Andy Shevchenko, Thomas Gleixner,
H . Peter Anvin, intel-gfx, dri-devel, Mika Westerberg,
Takashi Iwai, russianneuromancer @ ya . ru, linux-i2c
In-Reply-To: <20170123210958.18410-1-hdegoede@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 306 bytes --]
> So the plan (again with Wolfram's ack) is for all these patches
> including the i2c-designware ones to go upstream through the drm-intel
> tree, to avoid an intermediate state were things don't work.
Yes. I'd just like to pull in an immutable branch into my i2c/for-next
once this series is accepted.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH v4 3/3] iio: distance: srf08: add driver ABI documentation
From: Andreas Klinger @ 2017-01-25 19:07 UTC (permalink / raw)
To: jic23-DgEjT+Ai2ygdnm+yROfE0A, knaack.h-Mmb7MZpHnFY,
lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, ktsai-GubuWUlQtMwciDkP5Hr2oA,
wsa-z923LK4zBo2bacvFa/9K2g, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
pawel.moll-5wv7dgnIgG8, mark.rutland-5wv7dgnIgG8,
ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
galak-sgV2jX0FEOL9JmXXK+q4OQ, trivial-DgEjT+Ai2ygdnm+yROfE0A,
mranostay-Re5JQEeQqe8AvxtiuMwx3w,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: ak-n176/SwNRljddJNmlsFzeA
Add sysfs-bus-iio-distance-srf08 for individual attributes of the driver,
especially:
- sensitivity which the device documentation calls gain for amplifying the
signal
- max_range for limiting the maximum distance for expected echos and
therefore limiting the time waiting for telegrams
Signed-off-by: Andreas Klinger <ak-n176/SwNRljddJNmlsFzeA@public.gmane.org>
---
.../ABI/testing/sysfs-bus-iio-distance-srf08 | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-distance-srf08
diff --git a/Documentation/ABI/testing/sysfs-bus-iio-distance-srf08 b/Documentation/ABI/testing/sysfs-bus-iio-distance-srf08
new file mode 100644
index 000000000000..0a1ca1487fa9
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-distance-srf08
@@ -0,0 +1,22 @@
+What /sys/bus/iio/devices/iio:deviceX/sensor_sensitivity
+Date: January 2017
+KernelVersion: 4.11
+Contact: linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+Description:
+ Show or set the gain boost of the amp, from 0-31 range.
+ default 31
+
+What /sys/bus/iio/devices/iio:deviceX/sensor_max_range
+Date: January 2017
+KernelVersion: 4.11
+Contact: linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+Description:
+ Show or set the maximum range between the sensor and the
+ first object echoed in meters. Default value is 6.020.
+ This setting limits the time the driver is waiting for a
+ echo.
+ Showing the range of available values is represented as the
+ minimum value, the step and the maximum value, all enclosed
+ in square brackets.
+ Example:
+ [0.043 0.043 11.008]
--
2.1.4
^ permalink raw reply related
* [PATCH v4 1/3] iio: distance: srf08: add trivial DT binding
From: Andreas Klinger @ 2017-01-25 19:06 UTC (permalink / raw)
To: jic23, knaack.h, lars, pmeerw, linux-iio, linux-kernel, ktsai,
wsa, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
trivial, mranostay, linux-i2c, devicetree
Cc: ak
- Add DT binding for devantech,srf08
- Add vendor devantech to vendor list
Signed-off-by: Andreas Klinger <ak@it-klinger.de>
Acked-by: Rob Herring <robh@kernel.org>
---
Documentation/devicetree/bindings/i2c/trivial-devices.txt | 1 +
Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
2 files changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
index 539874490492..86c6930c3c91 100644
--- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
+++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
@@ -36,6 +36,7 @@ dallas,ds1775 Tiny Digital Thermometer and Thermostat
dallas,ds3232 Extremely Accurate I²C RTC with Integrated Crystal and SRAM
dallas,ds4510 CPU Supervisor with Nonvolatile Memory and Programmable I/O
dallas,ds75 Digital Thermometer and Thermostat
+devantech,srf08 Devantech SRF08 ultrasonic ranger
dlg,da9053 DA9053: flexible system level PMIC with multicore support
dlg,da9063 DA9063: system PMIC for quad-core application processors
epson,rx8010 I2C-BUS INTERFACE REAL TIME CLOCK MODULE
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index 4696bb5c2198..80325e602403 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -65,6 +65,7 @@ dallas Maxim Integrated Products (formerly Dallas Semiconductor)
davicom DAVICOM Semiconductor, Inc.
delta Delta Electronics, Inc.
denx Denx Software Engineering
+devantech Devantech, Ltd.
digi Digi International Inc.
digilent Diglent, Inc.
dlg Dialog Semiconductor
--
2.1.4
^ permalink raw reply related
* [PATCH v4 2/3] iio: distance: srf08: add IIO driver for us ranger
From: Andreas Klinger @ 2017-01-25 19:07 UTC (permalink / raw)
To: jic23-DgEjT+Ai2ygdnm+yROfE0A, knaack.h-Mmb7MZpHnFY,
lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, ktsai-GubuWUlQtMwciDkP5Hr2oA,
wsa-z923LK4zBo2bacvFa/9K2g, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
pawel.moll-5wv7dgnIgG8, mark.rutland-5wv7dgnIgG8,
ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
galak-sgV2jX0FEOL9JmXXK+q4OQ, trivial-DgEjT+Ai2ygdnm+yROfE0A,
mranostay-Re5JQEeQqe8AvxtiuMwx3w,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: ak-n176/SwNRljddJNmlsFzeA
This is the IIO driver for devantech srf08 ultrasonic ranger which can be
used to measure the distances to an object.
The sensor supports I2C with some registers.
Supported Features include:
- read the distance in ranging mode in centimeters
- output of the driver is directly the read value
- together with the scale the driver delivers the distance in meters
- only the first echo of the nearest object is delivered
- set sensitivity as analog value in the range of 0-31 means setting
gain register on device
- set range registers; userspace enters max. range in millimeters in
43 mm steps
Features not supported by this driver:
- ranging mode in inches or in microseconds
- ANN mode
- change I2C address through this driver
- light sensor
The driver was added in the directory "proximity" of the iio subsystem and
the menu in den config is now called "Proximity and distance sensors"
Signed-off-by: Andreas Klinger <ak-n176/SwNRljddJNmlsFzeA@public.gmane.org>
---
drivers/iio/proximity/Kconfig | 13 +-
drivers/iio/proximity/Makefile | 1 +
drivers/iio/proximity/srf08.c | 398 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 411 insertions(+), 1 deletion(-)
create mode 100644 drivers/iio/proximity/srf08.c
diff --git a/drivers/iio/proximity/Kconfig b/drivers/iio/proximity/Kconfig
index ef4c73db5b53..ab96cb7a0054 100644
--- a/drivers/iio/proximity/Kconfig
+++ b/drivers/iio/proximity/Kconfig
@@ -18,7 +18,7 @@ config AS3935
endmenu
-menu "Proximity sensors"
+menu "Proximity and distance sensors"
config LIDAR_LITE_V2
tristate "PulsedLight LIDAR sensor"
@@ -45,4 +45,15 @@ config SX9500
To compile this driver as a module, choose M here: the
module will be called sx9500.
+config SRF08
+ tristate "Devantech SRF08 ultrasonic ranger sensor"
+ depends on I2C
+ help
+ Say Y here to build a driver for Devantech SRF08 ultrasonic
+ ranger sensor. This driver can be used to measure the distance
+ of objects.
+
+ To compile this driver as a module, choose M here: the
+ module will be called srf08.
+
endmenu
diff --git a/drivers/iio/proximity/Makefile b/drivers/iio/proximity/Makefile
index 9aadd9a8ee99..e914c2a5dd49 100644
--- a/drivers/iio/proximity/Makefile
+++ b/drivers/iio/proximity/Makefile
@@ -5,4 +5,5 @@
# When adding new entries keep the list in alphabetical order
obj-$(CONFIG_AS3935) += as3935.o
obj-$(CONFIG_LIDAR_LITE_V2) += pulsedlight-lidar-lite-v2.o
+obj-$(CONFIG_SRF08) += srf08.o
obj-$(CONFIG_SX9500) += sx9500.o
diff --git a/drivers/iio/proximity/srf08.c b/drivers/iio/proximity/srf08.c
new file mode 100644
index 000000000000..49316cbf7c60
--- /dev/null
+++ b/drivers/iio/proximity/srf08.c
@@ -0,0 +1,398 @@
+/*
+ * srf08.c - Support for Devantech SRF08 ultrasonic ranger
+ *
+ * Copyright (c) 2016 Andreas Klinger <ak-n176/SwNRljddJNmlsFzeA@public.gmane.org>
+ *
+ * This file is subject to the terms and conditions of version 2 of
+ * the GNU General Public License. See the file COPYING in the main
+ * directory of this archive for more details.
+ *
+ * For details about the device see:
+ * http://www.robot-electronics.co.uk/htm/srf08tech.html
+ */
+
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/delay.h>
+#include <linux/module.h>
+#include <linux/bitops.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+
+/* registers of SRF08 device */
+#define SRF08_WRITE_COMMAND 0x00 /* Command Register */
+#define SRF08_WRITE_MAX_GAIN 0x01 /* Max Gain Register: 0 .. 31 */
+#define SRF08_WRITE_RANGE 0x02 /* Range Register: 0 .. 255 */
+#define SRF08_READ_SW_REVISION 0x00 /* Software Revision */
+#define SRF08_READ_LIGHT 0x01 /* Light Sensor during last echo */
+#define SRF08_READ_ECHO_1_HIGH 0x02 /* Range of first echo received */
+#define SRF08_READ_ECHO_1_LOW 0x03 /* Range of first echo received */
+
+#define SRF08_CMD_RANGING_CM 0x51 /* Ranging Mode - Result in cm */
+
+#define SRF08_DEFAULT_GAIN 1025 /* default analogue value of Gain */
+#define SRF08_DEFAULT_RANGE 6020 /* default value of Range in mm */
+
+struct srf08_data {
+ struct i2c_client *client;
+ int sensitivity; /* Gain */
+ int range_mm; /* max. Range in mm */
+ struct mutex lock;
+};
+
+/*
+ * in the documentation one can read about the "Gain" of the device
+ * which is used here for amplifying the signal and filtering out unwanted
+ * ones.
+ * But with ADC's this term is already used differently and that's why it
+ * is called "Sensitivity" here.
+ */
+static const int srf08_sensitivity[] = {
+ 94, 97, 100, 103, 107, 110, 114, 118,
+ 123, 128, 133, 139, 145, 152, 159, 168,
+ 177, 187, 199, 212, 227, 245, 265, 288,
+ 317, 352, 395, 450, 524, 626, 777, 1025 };
+
+static int srf08_read_ranging(struct srf08_data *data)
+{
+ struct i2c_client *client = data->client;
+ int ret, i;
+ int waittime;
+
+ mutex_lock(&data->lock);
+
+ ret = i2c_smbus_write_byte_data(data->client,
+ SRF08_WRITE_COMMAND, SRF08_CMD_RANGING_CM);
+ if (ret < 0) {
+ dev_err(&client->dev, "write command - err: %d\n", ret);
+ mutex_unlock(&data->lock);
+ return ret;
+ }
+
+ /*
+ * we read here until a correct version number shows up as
+ * suggested by the documentation
+ *
+ * with an ultrasonic speed of 343 m/s and a roundtrip of it
+ * sleep the expected duration and try to read from the device
+ * if nothing useful is read try it in a shorter grid
+ *
+ * polling for not more than 20 ms should be enough
+ */
+ waittime = 1 + data->range_mm / 172;
+ msleep(waittime);
+ for (i = 0; i < 4; i++) {
+ ret = i2c_smbus_read_byte_data(data->client,
+ SRF08_READ_SW_REVISION);
+
+ /* check if a valid version number is read */
+ if (ret < 255 && ret > 0)
+ break;
+ msleep(5);
+ }
+
+ if (ret >= 255 || ret <= 0) {
+ dev_err(&client->dev, "device not ready\n");
+ mutex_unlock(&data->lock);
+ return -EIO;
+ }
+
+ ret = i2c_smbus_read_word_swapped(data->client,
+ SRF08_READ_ECHO_1_HIGH);
+ if (ret < 0) {
+ dev_err(&client->dev, "cannot read distance: ret=%d\n", ret);
+ mutex_unlock(&data->lock);
+ return ret;
+ }
+
+ mutex_unlock(&data->lock);
+
+ return ret;
+}
+
+static int srf08_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *channel, int *val,
+ int *val2, long mask)
+{
+ struct srf08_data *data = iio_priv(indio_dev);
+ int ret;
+
+ if (channel->type != IIO_DISTANCE)
+ return -EINVAL;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ ret = srf08_read_ranging(data);
+ if (ret < 0)
+ return ret;
+ *val = ret;
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_SCALE:
+ /* 1 LSB is 1 cm */
+ *val = 0;
+ *val2 = 10000;
+ return IIO_VAL_INT_PLUS_MICRO;
+ default:
+ return -EINVAL;
+ }
+}
+
+static ssize_t srf08_show_range_mm_available(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "[0.043 0.043 11.008]\n");
+}
+
+static IIO_DEVICE_ATTR(sensor_max_range_available, S_IRUGO,
+ srf08_show_range_mm_available, NULL, 0);
+
+static ssize_t srf08_show_range_mm(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+ struct srf08_data *data = iio_priv(indio_dev);
+
+ return sprintf(buf, "%d.%03d\n", data->range_mm / 1000,
+ data->range_mm % 1000);
+}
+
+/*
+ * set the range of the sensor to an even multiple of 43 mm
+ * which corresponds to 1 LSB in the register
+ *
+ * register value corresponding range
+ * 0x00 43 mm
+ * 0x01 86 mm
+ * 0x02 129 mm
+ * ...
+ * 0xFF 11008 mm
+ */
+static ssize_t srf08_write_range_mm(struct srf08_data *data, unsigned int val)
+{
+ int ret;
+ struct i2c_client *client = data->client;
+ unsigned int mod;
+ u8 regval;
+
+ ret = val / 43 - 1;
+ mod = val % 43;
+
+ if (mod || (ret < 0) || (ret > 255))
+ return -EINVAL;
+
+ regval = ret;
+
+ mutex_lock(&data->lock);
+
+ ret = i2c_smbus_write_byte_data(client, SRF08_WRITE_RANGE, regval);
+ if (ret < 0) {
+ dev_err(&client->dev, "write_range - err: %d\n", ret);
+ mutex_unlock(&data->lock);
+ return ret;
+ }
+
+ data->range_mm = val;
+
+ mutex_unlock(&data->lock);
+
+ return 0;
+}
+
+static ssize_t srf08_store_range_mm(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t len)
+{
+ struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+ struct srf08_data *data = iio_priv(indio_dev);
+ int ret;
+ int integer, fract;
+
+ ret = iio_str_to_fixpoint(buf, 100, &integer, &fract);
+ if (ret)
+ return ret;
+
+ ret = srf08_write_range_mm(data, integer * 1000 + fract);
+ if (ret < 0)
+ return ret;
+
+ return len;
+}
+
+static IIO_DEVICE_ATTR(sensor_max_range, S_IRUGO | S_IWUSR,
+ srf08_show_range_mm, srf08_store_range_mm, 0);
+
+static ssize_t srf08_show_sensitivity_available(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ int i, len = 0;
+
+ for (i = 0; i < ARRAY_SIZE(srf08_sensitivity); i++)
+ len += sprintf(buf + len, "%d ", srf08_sensitivity[i]);
+
+ len += sprintf(buf + len, "\n");
+
+ return len;
+}
+
+static IIO_DEVICE_ATTR(sensor_sensitivity_available, S_IRUGO,
+ srf08_show_sensitivity_available, NULL, 0);
+
+static ssize_t srf08_show_sensitivity(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+ struct srf08_data *data = iio_priv(indio_dev);
+ int len;
+
+ len = sprintf(buf, "%d\n", data->sensitivity);
+
+ return len;
+}
+
+static ssize_t srf08_write_sensitivity(struct srf08_data *data,
+ unsigned int val)
+{
+ struct i2c_client *client = data->client;
+ int ret, i;
+ u8 regval;
+
+ for (i = 0; i < ARRAY_SIZE(srf08_sensitivity); i++)
+ if (val == srf08_sensitivity[i]) {
+ regval = i;
+ break;
+ }
+
+ if (i >= ARRAY_SIZE(srf08_sensitivity))
+ return -EINVAL;
+
+ mutex_lock(&data->lock);
+
+ ret = i2c_smbus_write_byte_data(client,
+ SRF08_WRITE_MAX_GAIN, regval);
+ if (ret < 0) {
+ dev_err(&client->dev, "write_sensitivity - err: %d\n", ret);
+ mutex_unlock(&data->lock);
+ return ret;
+ }
+
+ data->sensitivity = val;
+
+ mutex_unlock(&data->lock);
+
+ return 0;
+}
+
+static ssize_t srf08_store_sensitivity(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t len)
+{
+ struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+ struct srf08_data *data = iio_priv(indio_dev);
+ int ret;
+ unsigned int val;
+
+ ret = kstrtouint(buf, 10, &val);
+ if (ret)
+ return ret;
+
+ ret = srf08_write_sensitivity(data, val);
+ if (ret < 0)
+ return ret;
+
+ return len;
+}
+
+static IIO_DEVICE_ATTR(sensor_sensitivity, S_IRUGO | S_IWUSR,
+ srf08_show_sensitivity, srf08_store_sensitivity, 0);
+
+static struct attribute *srf08_attributes[] = {
+ &iio_dev_attr_sensor_max_range.dev_attr.attr,
+ &iio_dev_attr_sensor_max_range_available.dev_attr.attr,
+ &iio_dev_attr_sensor_sensitivity.dev_attr.attr,
+ &iio_dev_attr_sensor_sensitivity_available.dev_attr.attr,
+ NULL,
+};
+
+static const struct attribute_group srf08_attribute_group = {
+ .attrs = srf08_attributes,
+};
+
+static const struct iio_chan_spec srf08_channels[] = {
+ {
+ .type = IIO_DISTANCE,
+ .info_mask_separate =
+ BIT(IIO_CHAN_INFO_RAW) |
+ BIT(IIO_CHAN_INFO_SCALE),
+ },
+};
+
+static const struct iio_info srf08_info = {
+ .read_raw = srf08_read_raw,
+ .attrs = &srf08_attribute_group,
+ .driver_module = THIS_MODULE,
+};
+
+static int srf08_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct iio_dev *indio_dev;
+ struct srf08_data *data;
+ int ret;
+
+ if (!i2c_check_functionality(client->adapter,
+ I2C_FUNC_SMBUS_READ_BYTE_DATA |
+ I2C_FUNC_SMBUS_WRITE_BYTE_DATA |
+ I2C_FUNC_SMBUS_READ_WORD_DATA))
+ return -ENODEV;
+
+ indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ data = iio_priv(indio_dev);
+ i2c_set_clientdata(client, indio_dev);
+ data->client = client;
+
+ indio_dev->name = "srf08";
+ indio_dev->dev.parent = &client->dev;
+ indio_dev->modes = INDIO_DIRECT_MODE;
+ indio_dev->info = &srf08_info;
+ indio_dev->channels = srf08_channels;
+ indio_dev->num_channels = ARRAY_SIZE(srf08_channels);
+
+ mutex_init(&data->lock);
+
+ /*
+ * set default values of device here
+ * these register values cannot be read from the hardware
+ * therefore set driver specific default values
+ */
+ ret = srf08_write_range_mm(data, SRF08_DEFAULT_RANGE);
+ if (ret < 0)
+ return ret;
+
+ ret = srf08_write_sensitivity(data, SRF08_DEFAULT_GAIN);
+ if (ret < 0)
+ return ret;
+
+ return devm_iio_device_register(&client->dev, indio_dev);
+}
+
+static const struct i2c_device_id srf08_id[] = {
+ { "srf08", 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, srf08_id);
+
+static struct i2c_driver srf08_driver = {
+ .driver = {
+ .name = "srf08",
+ },
+ .probe = srf08_probe,
+ .id_table = srf08_id,
+};
+module_i2c_driver(srf08_driver);
+
+MODULE_AUTHOR("Andreas Klinger <ak-n176/SwNRljddJNmlsFzeA@public.gmane.org>");
+MODULE_DESCRIPTION("Devantech SRF08 ultrasonic ranger driver");
+MODULE_LICENSE("GPL");
--
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 0/3] iio: distance: srf08: add IIO driver for us ranger
From: Andreas Klinger @ 2017-01-25 19:06 UTC (permalink / raw)
To: jic23-DgEjT+Ai2ygdnm+yROfE0A, knaack.h-Mmb7MZpHnFY,
lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, ktsai-GubuWUlQtMwciDkP5Hr2oA,
wsa-z923LK4zBo2bacvFa/9K2g, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
pawel.moll-5wv7dgnIgG8, mark.rutland-5wv7dgnIgG8,
ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
galak-sgV2jX0FEOL9JmXXK+q4OQ, trivial-DgEjT+Ai2ygdnm+yROfE0A,
mranostay-Re5JQEeQqe8AvxtiuMwx3w,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: ak-n176/SwNRljddJNmlsFzeA
This patch series adds IIO driver support for srf08 ultrasonic ranger
devices.
The first patch add a trivial device tree binding for the device together
with a new vendor devantech.
The second patch is the IIO driver which in turn is using I2C to talk to
the device.
The third patch documents the added userspace ABI
Documentation about the sensor can be found here:
http://www.robot-electronics.co.uk/htm/srf08tech.html
Changes in v4:
Jonathan suggested to use the same units for the max_range attribute as
used for the distance raw value which is now changed in the driver. Thanks
for the review and detailed suggestions.
* Patch 2: "iio: distance: srf08: add IIO driver for us ranger"
- changed units of max_range attribute to meter
- changed max_range_available to print out min-, step- and max-value
instead of each single value
* Patch 3: "iio: distance: srf08: add driver ABI documentation"
- removed standard elements from documentation
- changed documentation of max_range_available attribute
Changes in v3:
Thanks to the reviews of Jonathan, Rob and Lars.
Some changes to the driver were made:
* Patch 2: "iio: distance: srf08: add IIO driver for us ranger"
- defaults values for sensitivity (gain) and max. range are set in
probe()
- while waiting for ultrasonic echo first wait as long as the physically
measurement need as a function of the max. range and then wait in
smaller steps until the response can be read.
- attributes are now called sensor_sensitivity instead of gain and
sensor_max_range instead of range_mm
- device-name is now "srf08"
- config menu is now calles "Proximity and distance sensors" instead of a
separate menu
* Patch 3: "iio: distance: srf08: add driver ABI documentation"
- newly created to document the driver specific interfaces
Changes in v2:
Lots of updates thanks to Peters really fast review within 30 minutes
after first submission of the driver.
* Patch 2: "iio: distance: srf08: add IIO driver for us ranger"
- alphabetic order in Makefile
- use of u8 while accessing registers
- avoid endianness problems with 16 bit values
- missing return value checks
- some explaining documentation added
Andreas Klinger (3):
iio: distance: srf08: add trivial DT binding
iio: distance: srf08: add IIO driver for us ranger
iio: distance: srf08: add driver ABI documentation
.../ABI/testing/sysfs-bus-iio-distance-srf08 | 22 ++
.../devicetree/bindings/i2c/trivial-devices.txt | 1 +
.../devicetree/bindings/vendor-prefixes.txt | 1 +
drivers/iio/proximity/Kconfig | 13 +-
drivers/iio/proximity/Makefile | 1 +
drivers/iio/proximity/srf08.c | 398 +++++++++++++++++++++
6 files changed, 435 insertions(+), 1 deletion(-)
create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-distance-srf08
create mode 100644 drivers/iio/proximity/srf08.c
--
2.1.4
^ permalink raw reply
* Re: [PATCH v5 5/5] i2c: mux: pca954x: Add irq-mask-enable to delay enabling irqs
From: Peter Rosin @ 2017-01-25 11:30 UTC (permalink / raw)
To: Danielle Costantino, Phil Reid
Cc: Wolfram Sang, robh+dt, mark.rutland, linux-i2c,
Linux device trees
In-Reply-To: <d9e92666-0c92-891d-7481-1800f5b4efac@gmail.com>
On 2017-01-25 10:17, Danielle Costantino wrote:
>
>
> On 01/25/2017 12:15 AM, Peter Rosin wrote:
>> On 2017-01-25 04:50, Danielle Costantino wrote:
>>> On Mon, Jan 23, 2017 at 1:02 AM, Phil Reid <preid@electromag.com.au> wrote:
>>>> On 20/01/2017 06:56, Peter Rosin wrote:
>>>>> On 2017-01-19 08:48, Phil Reid wrote:
>>>>>> On 18/01/2017 20:19, Peter Rosin wrote:
>>>>>>> On 2017-01-17 09:00, Phil Reid wrote:
>>>>
>>>> [snip]
>>>>
>>>>
>>>>>>> Hmm, this whole thing is fiddly and while it solves your problem it doesn't
>>>>>>> allow for solving the more general problem when there are "problematic"
>>>>>>> devices mixed with other devices. At least, I don't see it. And the
>>>>>>> limitations we are walking into with tracking number of enables etc suggests
>>>>>>> that we are attacking this at the wrong level. Maybe you should try to work
>>>>>>> around the hw limitations not in the pca954x driver, but in the irq core?
>>>>>>
>>>>>> I'm looking at the option of getting the hardware changed to not route
>>>>>> the irq for my chips thru the i2c mux. Fortunately the hardware is going thru a
>>>>>> revision for some other changes. Messing with the irq core sounds dangerous
>>>>>> with my level of knowledge.
>>>>>
>>>>> Yeah, but I bet you'd get some attention from people with more irq
>>>>> experience. That can't be bad :-)
>>>>>
>>>>>> The other way I think I can tackle it after reading the datasheet for the ltc1760 is that
>>>>>> it'll deassert it's irq (smbalert) line when the host sends a ARA request on the bus segment.
>>>>>> There's a driver in the kernel for this already, but it's not DT enable and doesn't
>>>>>> handle multiple bus segments. I'll have a look at that as well.
>>>>>> Pretty sure it would need the mux to become an irq parent as per patch 1-3 of this series.
>>>>>> This would be so the system can figure out which segment to do the poll on.
>>>>>
>>>>> Yeah sounds neater. It has the slight drawback that it may not work
>>>>> for pure i2c buses since it an SMB thing??
>>> If you need to send SMBus commands like ARA, you should be using an
>>> SMBus 2.0+ compatible bus multiplexer. muxes like the pca954x do not
>>> automatically select the bus segment that the ARA is destined for. It
>>> usually is more efficient to only request the data you need from each
>>> device, rather than checking every segment on each interrupt for the
>> We are not talking about checking every segment on every interrupt, we
>> are talking about checking the segments indicated by the INTx bits in
>> the control register.
>>
>>> cause, one could implement a delayed worker to schedule
>>> checking+clearing the interrupt at a time when the bus is selected for
>>> use by another slave device on that segment when possible. This will
>>> reduce the number of bus setup operations per transfer to slaves on
>>> deeper busses, reducing your i2c latency.
>> i2c traffic scheduling does not exist in linux to the best of my knowledge,
>> it's all handled with a simple mutex AFAIK. So, while traffic scheduling
>> is an interesting problem, I think it is out of scope at this time.
>>
>> If you happen to have a pca954x mux (with irq support) and happen to
>> have devices behind it that needs ARA support, I just don't see how any
>> of the above is relevant. Yes, it could be more efficient to have the
>> hardware done differently, but that is in many cases not a possibility.
>> You have to make do with what you have, and if that costs latency, then
>> there is little to do about that. You only have to make the new stuff
>> optional so that old working setups don't suffer.
> Yes traffic scheduling of I2C transactions is out of the scope of the
> current problem but I thought that it may be useful to bring it up.
>>>>> BTW, why do you need special treatment for multiple segments? Will it not
>>>>> simply have an ARA appear on whatever i2c bus the device sits on? And if
>>>>> something requests to send an ARA message on a bus that happens to be a
>>>>> muxed segment, my mental picture is that the mux will be operated as usual
>>>>> so that the ARA appears on the muxed segment. Maybe I'm missing something?
>>>>
>>>> My think was the following.
>>>> When the SMBALERT is asserted a ARA needs to be sent by the master.
>>>> If the device sending the SMBALERT is behind a mux when need to know which segment of the bus to enable.
>>>> Using shared interrupts should work I think, but you have to iterate thru each bus segment.
>>>> If the alert device is nested behind a couple of muxes this could get expensive.
>>>> But yeah otherwise I think the correct mux segment will get enabled automatically.
>>>> The current SMBALERT driver only seems to attached to the root i2c adapter.
>>> Using ARA in a multi mux environment is very expensive, setting up
>>> each mux segment and then sending ARA will consume more time than it
>>> would take if the mux structure was optimized to service devices on
>>> similar busses, reducing the setup operation count. ARA was only
>>> implemented on the root bus due to the design of the arbiter and mux
>>> cannot guarantee that you are the only owner of the bus when you are
>>> disconnected.
>> ARA handling should take the irq register of the pca95x into account and
>> only send ARAs to indicated mux segments. If more than one segment needs
>> servicing, then of course one ARA for each segment needs to be sent. If
>> someone builds hardware like this and then expect no latency, that someone
>> will hopefully at least learn something.
>>
>> I do not know how this fits with the existing ARA handling (it probably
>> doesn't), but what needs to happen is fairly easy to picture.
>>
>>> You would also need to handle the cases where segments
>>> lock up and must be released using the bus reset mechanism, this
>>> resets the IRQs as well. The added cost of a I2C read to coincide the
>>> write operation to the mux when the irq pin is asserted is
>>> unacceptable for low latency applications.
>> I bet there are a lot of corner cases, and yes, this added cost has to be
>> optional. Perhaps with each mux child segment opting in for ARA support?
>> If a mux then has a way to determine that an ARA isn't needed for some of
>> its child segments (by reading some status register), then linux is free
>> to not send ARAs there. And more importantly, if no mux child segment opts
>> in for ARA support, it should be possible to preserve current behavior...
> I agree with making ARA handling for child bus segments optional based
> on the need of ARA or other SMBus features. We have needed IRQ support
> for muxes for a while now, I was always concerned about dead locks
> between setting up the bus and servicing the IRQ. The current patch v6
> 0/3 appears to handle child IRQs without selecting the bus associated
> with it. Is the plan to have the irq handling of the child require an
> i2c transaction on its registered bus, which will in turn select that
> bus?
You are correct in that the current proposed patches do not support
having a device being master on a mux child segment and expect that
any traffic hits the root adapter. A device has no way to lock the
mux in the correct position short of having its driver issue an
ordinary transaction (i.e. ->i2c_transfer or ->smbus_xfer) on the
adapter it sits on. And after the transaction, there is no guarantee
that the mux will stay in the desired position.
> I also thought i2c transactions were not allowed inside an IRQ
> handler?
They are allowed in a threaded handler, which is very similar to a
delayed work. If it is not in fact exactly that?
> Could a delayed work struct could be used to fire off the read
> transaction that would then trigger the nested IRQs (based on the set
> bits)? Also the PCA954x has an internal OR of the INT signals coming
> into it, not an AND like the patch says.
All INT signals on the pca954x are active-low. The only sensible way for
the mux to combine its active-low inputs into an active-low output is
with an AND gate. If any of the inputs fire, you want the output to
also fire. So, I'd say that commit message is fine.
> Before allowing IRQs to be
> handled by the assertion of the active low INT out signal from the
> PCA954x all child interrupts must be de-asserted else the IRQ will
> always be set. You could read the value of the interrupt register to see
> if all bits are clear before allowing unmasking of the devices interrupts.
This is not possible with the pca954x. There is no way to prevent them
from asserting their output if one of their inputs is asserted. Which
means that if the irq is shared, and if something else have activated
it, there will be an irq flood. This is exactly the kind of trouble
Phil is trying to work around since his chargers continuously fire
interrupts when there is no driver to quiet them down.
I don't know if going with ARAs can help him, but I think it should be
possible. I also don't know how difficult it is going to be to make
this work and also be compatible with whatever ARA support there is
already. I'm not familiar with how the current ARA support works...
Cheers,
peda
^ permalink raw reply
* Re: [PATCH v2 3/3] i2c: zx2967: add i2c controller driver for ZTE's zx2967 family
From: Shawn Guo @ 2017-01-25 9:38 UTC (permalink / raw)
To: Baoyou Xie
Cc: andy.shevchenko-Re5JQEeQqe8AvxtiuMwx3w,
jun.nie-QSEj5FYQhm4dnm+yROfE0A, wsa-z923LK4zBo2bacvFa/9K2g,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
xie.baoyou-Th6q7B73Y6EnDS1+zs4M5A,
chen.chaokai-Th6q7B73Y6EnDS1+zs4M5A,
wang.qiang01-Th6q7B73Y6EnDS1+zs4M5A
In-Reply-To: <1485147806-17792-3-git-send-email-baoyou.xie-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
On Mon, Jan 23, 2017 at 01:03:26PM +0800, Baoyou Xie wrote:
> This patch adds i2c controller driver for ZTE's zx2967 family.
>
> Signed-off-by: Baoyou Xie <baoyou.xie-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
> drivers/i2c/busses/Kconfig | 9 +
> drivers/i2c/busses/Makefile | 1 +
> drivers/i2c/busses/i2c-zx2967.c | 690 ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 700 insertions(+)
> create mode 100644 drivers/i2c/busses/i2c-zx2967.c
>
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index e4a603e..9609f45 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -1246,4 +1246,13 @@ config I2C_OPAL
> This driver can also be built as a module. If so, the module will be
> called as i2c-opal.
>
> +config I2C_ZX2967
> + tristate "ZTE zx2967 I2C support"
> + depends on ARCH_ZX
> + default y
> + help
> + Selecting this option will add ZX2967 I2C driver.
> + This driver can also be built as a module. If so, the module will be
> + called zx2967_i2c.
Called zx2967_i2c? Is it the .ko filename or the name we see in lsmod
command output? From what I have seen, neither of the names is
zx2967_i2c.
Also, I see the following failure in module installation.
$ insmod i2c-zx2967.ko
i2c_zx2967: Unknown symbol __clk_get_enable_count (err 0)
Error: could not insert module i2c-zx2967.ko: Unknown symbol in module
> +
> endmenu
> diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
> index beb4809..16b2901 100644
> --- a/drivers/i2c/busses/Makefile
> +++ b/drivers/i2c/busses/Makefile
> @@ -101,6 +101,7 @@ obj-$(CONFIG_I2C_XILINX) += i2c-xiic.o
> obj-$(CONFIG_I2C_XLR) += i2c-xlr.o
> obj-$(CONFIG_I2C_XLP9XX) += i2c-xlp9xx.o
> obj-$(CONFIG_I2C_RCAR) += i2c-rcar.o
> +obj-$(CONFIG_I2C_ZX2967) += i2c-zx2967.o
>
> # External I2C/SMBus adapter drivers
> obj-$(CONFIG_I2C_DIOLAN_U2C) += i2c-diolan-u2c.o
> diff --git a/drivers/i2c/busses/i2c-zx2967.c b/drivers/i2c/busses/i2c-zx2967.c
> new file mode 100644
> index 0000000..d38b7b7
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-zx2967.c
> @@ -0,0 +1,690 @@
> +/*
> + * ZTE's zx2967 family i2c bus controller driver
> + *
> + * Copyright (C) 2017 ZTE Ltd.
> + *
> + * Author: Baoyou Xie <baoyou.xie-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> + *
> + * License terms: GNU General Public License (GPL) version 2
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
clk-provider.h is only meant to be included by clock driver, not client
device drivers.
> +#include <linux/delay.h>
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/pinctrl/consumer.h>
> +#include <linux/platform_device.h>
> +
> +#define REG_CMD 0x04
> +#define REG_DEVADDR_H 0x0C
> +#define REG_DEVADDR_L 0x10
> +#define REG_CLK_DIV_FS 0x14
> +#define REG_CLK_DIV_HS 0x18
> +#define REG_WRCONF 0x1C
> +#define REG_RDCONF 0x20
> +#define REG_DATA 0x24
> +#define REG_STAT 0x28
> +
> +#define I2C_STOP 0
> +#define I2C_MASTER BIT(0)
> +#define I2C_ADDR_MODE_TEN BIT(1)
> +#define I2C_IRQ_MSK_ENABLE BIT(3)
> +#define I2C_RW_READ BIT(4)
> +#define I2C_CMB_RW_EN BIT(5)
> +#define I2C_START BIT(6)
> +#define I2C_ADDR_MODE_TEN BIT(1)
> +
> +#define I2C_WFIFO_RESET BIT(7)
> +#define I2C_RFIFO_RESET BIT(7)
> +
> +#define I2C_IRQ_ACK_CLEAR BIT(7)
> +#define I2C_INT_MASK GENMASK(6, 0)
> +
> +#define I2C_TRANS_DONE BIT(0)
> +#define I2C_ERROR_DEVICE BIT(1)
> +#define I2C_ERROR_DATA BIT(2)
> +#define I2C_ERROR_MASK GENMASK(2, 1)
> +
> +#define I2C_SR_BUSY BIT(6)
> +
> +#define I2C_SR_EDEVICE BIT(1)
> +#define I2C_SR_EDATA BIT(2)
> +
> +#define I2C_FIFO_MAX 16
> +
> +#define I2C_TIMEOUT msecs_to_jiffies(1000)
> +
> +struct zx2967_i2c_info {
> + spinlock_t lock;
> + struct device *dev;
> + struct i2c_adapter adap;
> + struct clk *clk;
> + struct completion complete;
> + u32 clk_freq;
> + struct pinctrl *pin_ctrl;
> + void __iomem *reg_base;
> + size_t residue;
> + int id;
I don't see how this field is being used.
> + int irq;
> + int msg_rd;
> + u8 *buf;
> + u8 access_cnt;
> + bool is_suspended;
> +};
> +
> +static void zx2967_i2c_writel(struct zx2967_i2c_info *zx_i2c,
> + u32 val, unsigned long reg)
> +{
> + writel_relaxed(val, zx_i2c->reg_base + reg);
> +}
> +
> +static u32 zx2967_i2c_readl(struct zx2967_i2c_info *zx_i2c, unsigned long reg)
> +{
> + return readl_relaxed(zx_i2c->reg_base + reg);
> +}
> +
> +static void zx2967_i2c_writesb(struct zx2967_i2c_info *zx_i2c,
> + void *data, unsigned long reg, int len)
> +{
> + writesb(zx_i2c->reg_base + reg, data, len);
> +}
> +
> +static void zx2967_i2c_readsb(struct zx2967_i2c_info *zx_i2c,
> + void *data, unsigned long reg, int len)
> +{
> + readsb(zx_i2c->reg_base + reg, data, len);
> +}
> +
> +static void zx2967_i2c_start_ctrl(struct zx2967_i2c_info *zx_i2c)
> +{
> + u32 status;
> + u32 ctl;
> +
> + status = zx2967_i2c_readl(zx_i2c, REG_STAT);
> + status |= I2C_IRQ_ACK_CLEAR;
> + zx2967_i2c_writel(zx_i2c, status, REG_STAT);
> +
> + ctl = zx2967_i2c_readl(zx_i2c, REG_CMD);
> + if (zx_i2c->msg_rd)
> + ctl |= I2C_RW_READ;
> + else
> + ctl &= ~I2C_RW_READ;
> + ctl &= ~I2C_CMB_RW_EN;
> + ctl |= I2C_START;
> + zx2967_i2c_writel(zx_i2c, ctl, REG_CMD);
> +}
> +
> +static int zx2967_i2c_flush_fifos(struct zx2967_i2c_info *zx_i2c)
> +{
> + u32 val;
> + u32 offset;
> +
> + if (zx_i2c->msg_rd) {
> + offset = REG_RDCONF;
> + val = I2C_RFIFO_RESET;
> + } else {
> + offset = REG_WRCONF;
> + val = I2C_WFIFO_RESET;
> + }
> +
> + val |= zx2967_i2c_readl(zx_i2c, offset);
> + zx2967_i2c_writel(zx_i2c, val, offset);
> +
> + return 0;
> +}
> +
> +static int zx2967_i2c_empty_rx_fifo(struct zx2967_i2c_info *zx_i2c, u32 size)
> +{
> + u8 val[I2C_FIFO_MAX] = {0};
We need only one space between 'u8' and 'val'.
> + int i;
> +
> + if (size > I2C_FIFO_MAX) {
> + dev_err(zx_i2c->dev, "fifo size %d over the max value %d\n",
> + size, I2C_FIFO_MAX);
> + return -EINVAL;
> + }
> +
> + zx2967_i2c_readsb(zx_i2c, val, REG_DATA, size);
> + for (i = 0; i < size; i++) {
> + *(zx_i2c->buf++) = val[i];
> + zx_i2c->residue--;
> + if (zx_i2c->residue <= 0)
> + break;
> + }
> +
> + barrier();
> +
> + return 0;
> +}
> +
> +static int zx2967_i2c_fill_tx_fifo(struct zx2967_i2c_info *zx_i2c)
> +{
> + u8 *buf = zx_i2c->buf;
> + size_t residue = zx_i2c->residue;
> +
> + if (residue == 0) {
> + dev_err(zx_i2c->dev, "residue is %d\n", (int)residue);
> + return -EINVAL;
> + }
> +
> + if (residue <= I2C_FIFO_MAX) {
> + zx2967_i2c_writesb(zx_i2c, buf, REG_DATA, residue);
> +
> + /* Again update before writing to FIFO to make sure isr sees. */
> + zx_i2c->residue = 0;
> + zx_i2c->buf = NULL;
> + } else {
> + zx2967_i2c_writesb(zx_i2c, buf, REG_DATA, I2C_FIFO_MAX);
> + zx_i2c->residue -= I2C_FIFO_MAX;
> + zx_i2c->buf += I2C_FIFO_MAX;
> + }
> +
> + barrier();
> +
> + return 0;
> +}
> +
> +static int zx2967_i2c_reset_hardware(struct zx2967_i2c_info *zx_i2c)
> +{
> + u32 val;
> + u32 clk_div;
> + u32 status;
> +
> + val = I2C_MASTER | I2C_IRQ_MSK_ENABLE;
> + zx2967_i2c_writel(zx_i2c, val, REG_CMD);
> +
> + clk_div = clk_get_rate(zx_i2c->clk) / zx_i2c->clk_freq - 1;
> + zx2967_i2c_writel(zx_i2c, clk_div, REG_CLK_DIV_FS);
> + zx2967_i2c_writel(zx_i2c, clk_div, REG_CLK_DIV_HS);
> +
> + zx2967_i2c_writel(zx_i2c, I2C_FIFO_MAX - 1, REG_WRCONF);
> + zx2967_i2c_writel(zx_i2c, I2C_FIFO_MAX - 1, REG_RDCONF);
> + zx2967_i2c_writel(zx_i2c, 1, REG_RDCONF);
> +
> + if (zx2967_i2c_flush_fifos(zx_i2c))
zx2967_i2c_flush_fifos() seems to always return 0.
> + return -ETIMEDOUT;
> +
> + status = zx2967_i2c_readl(zx_i2c, REG_STAT);
> + if (status & (I2C_SR_BUSY))
The parentheses is not necessary.
> + return -EBUSY;
> + if (status & (I2C_SR_EDEVICE | I2C_SR_EDATA))
> + return -EIO;
> +
> + enable_irq(zx_i2c->irq);
> +
> + return 0;
> +}
> +
> +static void zx2967_i2c_isr_clr(struct zx2967_i2c_info *zx_i2c)
> +{
> + u32 status;
> +
> + status = zx2967_i2c_readl(zx_i2c, REG_STAT);
> + status |= I2C_IRQ_ACK_CLEAR;
> + zx2967_i2c_writel(zx_i2c, status, REG_STAT);
> +}
> +
> +static irqreturn_t zx2967_i2c_isr(int irq, void *dev_id)
> +{
> + u32 status;
> + struct zx2967_i2c_info *zx_i2c = (struct zx2967_i2c_info *)dev_id;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&zx_i2c->lock, flags);
> +
> + status = zx2967_i2c_readl(zx_i2c, REG_STAT) & I2C_INT_MASK;
> + zx2967_i2c_isr_clr(zx_i2c);
> +
> + if (status & I2C_ERROR_MASK) {
> + spin_unlock_irqrestore(&zx_i2c->lock, flags);
> + return IRQ_HANDLED;
> + }
> +
> + if (status & I2C_TRANS_DONE)
> + complete(&zx_i2c->complete);
> +
> + spin_unlock_irqrestore(&zx_i2c->lock, flags);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static void zx2967_enable_tenbit(struct zx2967_i2c_info *zx_i2c, __u16 addr)
> +{
> + u16 val = (addr >> 7) & 0x7;
> +
> + if (val > 0) {
> + zx2967_i2c_writel(zx_i2c, val, REG_DEVADDR_H);
> + val = (zx2967_i2c_readl(zx_i2c, REG_CMD)) | I2C_ADDR_MODE_TEN;
> + zx2967_i2c_writel(zx_i2c, val, REG_CMD);
> + }
> +}
> +
> +static int
> +zx2967_i2c_xfer_read_bytes(struct zx2967_i2c_info *zx_i2c, u32 bytes)
> +{
> + unsigned long time_left;
> +
> + reinit_completion(&zx_i2c->complete);
> + zx2967_i2c_writel(zx_i2c, bytes - 1, REG_RDCONF);
> + zx2967_i2c_start_ctrl(zx_i2c);
> +
> + time_left = wait_for_completion_timeout(&zx_i2c->complete,
> + I2C_TIMEOUT);
> + if (time_left == 0) {
> + dev_err(zx_i2c->dev, "read i2c transfer timed out\n");
> + disable_irq(zx_i2c->irq);
> + zx2967_i2c_reset_hardware(zx_i2c);
> + return -EIO;
> + }
> +
> + zx2967_i2c_empty_rx_fifo(zx_i2c, bytes);
The function could return error.
> + return 0;
> +}
> +
> +static int zx2967_i2c_xfer_read(struct zx2967_i2c_info *zx_i2c)
> +{
> + int ret;
> + int i;
> +
> + for (i = 0; i < zx_i2c->access_cnt; i++) {
> + ret = zx2967_i2c_xfer_read_bytes(zx_i2c, I2C_FIFO_MAX);
> + if (ret)
> + return ret;
> + }
> +
> + if (zx_i2c->residue > 0) {
> + ret = zx2967_i2c_xfer_read_bytes(zx_i2c, I2C_FIFO_MAX);
> + if (ret)
> + return ret;
> + }
> +
> + zx_i2c->residue = 0;
> + zx_i2c->access_cnt = 0;
> + return 0;
> +}
> +
> +static int
> +zx2967_i2c_xfer_write_bytes(struct zx2967_i2c_info *zx_i2c, u32 bytes)
> +{
> + unsigned long time_left;
> +
> + reinit_completion(&zx_i2c->complete);
> + zx2967_i2c_fill_tx_fifo(zx_i2c);
It could return error.
> + zx2967_i2c_start_ctrl(zx_i2c);
> +
> + time_left = wait_for_completion_timeout(&zx_i2c->complete,
> + I2C_TIMEOUT);
> + if (time_left == 0) {
> + dev_err(zx_i2c->dev, "write i2c transfer timed out\n");
> + disable_irq(zx_i2c->irq);
> + zx2967_i2c_reset_hardware(zx_i2c);
> + return -EIO;
> + }
> +
> + return 0;
> +}
> +
> +static int zx2967_i2c_xfer_write(struct zx2967_i2c_info *zx_i2c)
> +{
> + int ret;
> + int i;
> +
> + for (i = 0; i < zx_i2c->access_cnt; i++) {
> + ret = zx2967_i2c_xfer_write_bytes(zx_i2c, I2C_FIFO_MAX);
> + if (ret)
> + return ret;
> + }
> +
> + if (zx_i2c->residue > 0) {
> + ret = zx2967_i2c_xfer_write_bytes(zx_i2c, I2C_FIFO_MAX);
> + if (ret)
> + return ret;
> + }
> +
> + zx_i2c->residue = 0;
> + zx_i2c->access_cnt = 0;
> + return 0;
> +}
> +
> +static int zx2967_i2c_xfer_msg(struct zx2967_i2c_info *zx_i2c,
> + struct i2c_msg *msg)
> +{
> + if (msg->len == 0)
> + return -EINVAL;
> +
> + zx2967_i2c_flush_fifos(zx_i2c);
> +
> + zx_i2c->buf = msg->buf;
> + zx_i2c->residue = msg->len;
> + zx_i2c->access_cnt = msg->len / I2C_FIFO_MAX;
> + zx_i2c->msg_rd = (msg->flags & I2C_M_RD);
> +
> + if (zx_i2c->msg_rd)
> + return zx2967_i2c_xfer_read(zx_i2c);
> +
> + return zx2967_i2c_xfer_write(zx_i2c);
> +}
> +
> +static int zx2967_i2c_xfer(struct i2c_adapter *adap,
> + struct i2c_msg *msgs, int num)
> +{
> + struct zx2967_i2c_info *zx_i2c = i2c_get_adapdata(adap);
> + int ret;
> + int i;
> +
> + if (zx_i2c->is_suspended)
> + return -EBUSY;
> +
> + zx2967_i2c_writel(zx_i2c, (msgs->addr & 0x7f), REG_DEVADDR_L);
> + zx2967_i2c_writel(zx_i2c, (msgs->addr >> 7) & 0x7, REG_DEVADDR_H);
> + if (zx2967_i2c_readl(zx_i2c, REG_DEVADDR_H) > 0)
> + zx2967_enable_tenbit(zx_i2c, msgs->addr);
> +
> + for (i = 0; i < num; i++) {
> + ret = zx2967_i2c_xfer_msg(zx_i2c, &msgs[i]);
> + if (num > 1)
> + usleep_range(1000, 2000);
> + if (ret)
> + return ret;
The 'ret' check should probably be put before usleep_range check?
> + }
> +
> + return num;
> +}
> +
> +static void
> +zx2967_smbus_xfer_prepare(struct zx2967_i2c_info *zx_i2c, u16 addr,
> + char read_write, u8 command, int size,
> + union i2c_smbus_data *data)
> +{
> + u32 val;
> +
> + val = zx2967_i2c_readl(zx_i2c, REG_RDCONF);
> + val |= I2C_RFIFO_RESET;
> + zx2967_i2c_writel(zx_i2c, val, REG_RDCONF);
> + zx2967_i2c_writel(zx_i2c, (addr & 0x7f), REG_DEVADDR_L);
> +
> + zx2967_enable_tenbit(zx_i2c, addr);
> + val = zx2967_i2c_readl(zx_i2c, REG_CMD);
> + val &= ~I2C_RW_READ;
> + zx2967_i2c_writel(zx_i2c, val, REG_CMD);
> +
> + switch (size) {
> + case I2C_SMBUS_BYTE:
> + zx2967_i2c_writel(zx_i2c, command, REG_DATA);
> + break;
> + case I2C_SMBUS_BYTE_DATA:
> + zx2967_i2c_writel(zx_i2c, command, REG_DATA);
> + if (read_write == I2C_SMBUS_WRITE)
> + zx2967_i2c_writel(zx_i2c, data->byte, REG_DATA);
> + break;
> + case I2C_SMBUS_WORD_DATA:
> + zx2967_i2c_writel(zx_i2c, command, REG_DATA);
> + if (read_write == I2C_SMBUS_WRITE) {
> + zx2967_i2c_writel(zx_i2c, (data->word >> 8), REG_DATA);
> + zx2967_i2c_writel(zx_i2c, (data->word & 0xff),
> + REG_DATA);
> + }
> + break;
> + }
> +}
> +
> +static int zx2967_smbus_xfer_read(struct zx2967_i2c_info *zx_i2c, int size,
> + union i2c_smbus_data *data)
> +{
> + unsigned long time_left;
> + u8 buf[2];
> + u32 val;
> +
> + reinit_completion(&zx_i2c->complete);
> +
> + val = zx2967_i2c_readl(zx_i2c, REG_CMD);
> + val |= I2C_CMB_RW_EN;
> + zx2967_i2c_writel(zx_i2c, val, REG_CMD);
> +
> + val = zx2967_i2c_readl(zx_i2c, REG_CMD);
> + val |= I2C_START;
> + zx2967_i2c_writel(zx_i2c, val, REG_CMD);
> +
> + time_left = wait_for_completion_timeout(&zx_i2c->complete,
> + I2C_TIMEOUT);
> + if (time_left == 0) {
> + dev_err(zx_i2c->dev, "i2c read transfer timed out\n");
> + disable_irq(zx_i2c->irq);
> + zx2967_i2c_reset_hardware(zx_i2c);
> + return -EIO;
> + }
> +
> + usleep_range(1000, 2000);
> + switch (size) {
> + case I2C_SMBUS_BYTE:
> + case I2C_SMBUS_BYTE_DATA:
> + val = zx2967_i2c_readl(zx_i2c, REG_DATA);
> + data->byte = val;
> + break;
> + case I2C_SMBUS_WORD_DATA:
> + case I2C_SMBUS_PROC_CALL:
> + buf[0] = zx2967_i2c_readl(zx_i2c, REG_DATA);
> + buf[1] = zx2967_i2c_readl(zx_i2c, REG_DATA);
> + data->word = (buf[0] << 8) | buf[1];
> + break;
> + default:
> + dev_warn(zx_i2c->dev, "Unsupported transaction %d\n", size);
> + return -EOPNOTSUPP;
> + }
> +
> + return 0;
> +}
> +
> +static int zx2967_smbus_xfer_write(struct zx2967_i2c_info *zx_i2c)
> +{
> + unsigned long time_left;
> + u32 val;
> +
> + reinit_completion(&zx_i2c->complete);
> + val = zx2967_i2c_readl(zx_i2c, REG_CMD);
> + val |= I2C_START;
> + zx2967_i2c_writel(zx_i2c, val, REG_CMD);
> +
> + time_left = wait_for_completion_timeout(&zx_i2c->complete,
> + I2C_TIMEOUT);
> + if (time_left == 0) {
> + dev_err(zx_i2c->dev, "i2c write transfer timed out\n");
> + disable_irq(zx_i2c->irq);
> + zx2967_i2c_reset_hardware(zx_i2c);
> + return -EIO;
> + }
> +
> + return 0;
> +}
> +
> +static int zx2967_smbus_xfer(struct i2c_adapter *adap, u16 addr,
> + unsigned short flags, char read_write,
> + u8 command, int size, union i2c_smbus_data *data)
> +{
> + struct zx2967_i2c_info *zx_i2c = i2c_get_adapdata(adap);
> +
> + if (size == I2C_SMBUS_QUICK)
> + read_write = I2C_SMBUS_WRITE;
> +
> + switch (size) {
> + case I2C_SMBUS_QUICK:
> + case I2C_SMBUS_BYTE:
> + case I2C_SMBUS_BYTE_DATA:
> + case I2C_SMBUS_WORD_DATA:
> + zx2967_smbus_xfer_prepare(zx_i2c, addr, read_write,
> + command, size, data);
> + break;
> + default:
> + dev_warn(&adap->dev, "Unsupported transaction %d\n", size);
> + return -EOPNOTSUPP;
> + }
> +
> + if (read_write == I2C_SMBUS_READ)
> + return zx2967_smbus_xfer_read(zx_i2c, size, data);
> +
> + return zx2967_smbus_xfer_write(zx_i2c);
> +}
> +
> +#define ZX2967_I2C_FUNCS (I2C_FUNC_SMBUS_QUICK | I2C_FUNC_SMBUS_BYTE | \
> + I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA | \
> + I2C_FUNC_SMBUS_BLOCK_DATA | I2C_FUNC_SMBUS_PROC_CALL | \
> + I2C_FUNC_I2C | I2C_FUNC_SMBUS_I2C_BLOCK)
Have a newline here.
> +static u32 zx2967_i2c_func(struct i2c_adapter *adap)
> +{
> + return ZX2967_I2C_FUNCS;
> +}
> +
> +static int __maybe_unused zx2967_i2c_suspend(struct device *dev)
> +{
> + struct zx2967_i2c_info *zx_i2c = dev_get_drvdata(dev);
> +
> + zx_i2c->is_suspended = true;
> + if (__clk_get_enable_count(zx_i2c->clk) == 1)
> + clk_disable_unprepare(zx_i2c->clk);
clk_prepare_enable() and clk_disable_unprepare() should always be called
in a balanced way. In that case, you shouldn't need to check enable
count.
> +
> + return 0;
> +}
> +
> +static int __maybe_unused zx2967_i2c_resume(struct device *dev)
> +{
> + struct zx2967_i2c_info *zx_i2c = dev_get_drvdata(dev);
> +
> + zx_i2c->is_suspended = false;
> + clk_prepare_enable(zx_i2c->clk);
> +
> + return 0;
> +}
> +
> +#ifdef CONFIG_PM
> +static const struct dev_pm_ops zx2967_i2c_dev_pm_ops = {
> + .suspend = zx2967_i2c_suspend,
> + .resume = zx2967_i2c_resume,
> +};
> +#define ZX2967_I2C_DEV_PM_OPS (&zx2967_i2c_dev_pm_ops)
> +#else
> +#define ZX2967_I2C_DEV_PM_OPS NULL
> +#endif
> +
> +static const struct i2c_algorithm zx2967_i2c_algo = {
> + .master_xfer = zx2967_i2c_xfer,
> + .smbus_xfer = zx2967_smbus_xfer,
> + .functionality = zx2967_i2c_func,
> +};
> +
> +static const struct of_device_id zx2967_i2c_of_match[] = {
> + { .compatible = "zte,zx296718-i2c", },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, zx2967_i2c_of_match);
> +
> +static int zx2967_i2c_probe(struct platform_device *pdev)
> +{
> + struct zx2967_i2c_info *zx_i2c;
> + void __iomem *reg_base;
> + struct resource *res;
> + struct clk *clk;
> + int ret;
> +
> + zx_i2c = devm_kzalloc(&pdev->dev, sizeof(*zx_i2c), GFP_KERNEL);
> + if (!zx_i2c)
> + return -ENOMEM;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + reg_base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(reg_base))
> + return PTR_ERR(reg_base);
> +
> + clk = devm_clk_get(&pdev->dev, NULL);
> + if (IS_ERR(clk)) {
> + dev_err(&pdev->dev, "missing controller clock");
> + return PTR_ERR(clk);
> + }
> +
> + ret = clk_prepare_enable(clk);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to enable i2c_clk\n");
> + return ret;
> + }
> +
> + ret = platform_get_irq(pdev, 0);
> + if (ret < 0)
> + return ret;
> + zx_i2c->irq = ret;
> +
> + device_property_read_u32(&pdev->dev, "clock-frequency",
> + &zx_i2c->clk_freq);
Since you list it in bindings doc as a required property, shouldn't you
check the return of the call to make sure the property is found
successfully?
> + zx_i2c->reg_base = reg_base;
> + zx_i2c->clk = clk;
> + zx_i2c->id = pdev->id;
> + zx_i2c->dev = &pdev->dev;
> +
> + zx_i2c->pin_ctrl = devm_pinctrl_get_select(&pdev->dev, "pinctrl-0");
> + if (IS_ERR(zx_i2c->pin_ctrl))
> + dev_info(&pdev->dev, "pinctrl-0 pin is not specified in dts\n");
It seems that I did not make my comment clear on the previous version.
I was trying to say you do not need to explicitly call pinctrl function,
if what you need is just to request 'default' pin state. Take a look at
commit ab78029ecc34 ("drivers/pinctrl: grab default handles from device
core"), and you will know what I'm saying.
Shawn
> +
> + spin_lock_init(&zx_i2c->lock);
> + init_completion(&zx_i2c->complete);
> + platform_set_drvdata(pdev, zx_i2c);
> +
> + ret = zx2967_i2c_reset_hardware(zx_i2c);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to initialize i2c controller\n");
> + goto err_clk_unprepare;
> + }
> +
> + ret = devm_request_irq(&pdev->dev, zx_i2c->irq,
> + zx2967_i2c_isr, 0, dev_name(&pdev->dev), zx_i2c);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to request irq %i\n", zx_i2c->irq);
> + goto err_clk_unprepare;
> + }
> +
> + i2c_set_adapdata(&zx_i2c->adap, zx_i2c);
> + zx_i2c->adap.owner = THIS_MODULE;
> + zx_i2c->adap.class = I2C_CLASS_DEPRECATED;
> + strlcpy(zx_i2c->adap.name, "zx2967 i2c adapter",
> + sizeof(zx_i2c->adap.name));
> + zx_i2c->adap.algo = &zx2967_i2c_algo;
> + zx_i2c->adap.dev.parent = &pdev->dev;
> + zx_i2c->adap.nr = pdev->id;
> + zx_i2c->adap.dev.of_node = pdev->dev.of_node;
> +
> + ret = i2c_add_numbered_adapter(&zx_i2c->adap);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to add zx2967 i2c adapter\n");
> + goto err_clk_unprepare;
> + }
> +
> + return 0;
> +
> +err_clk_unprepare:
> + clk_disable_unprepare(zx_i2c->clk);
> + return ret;
> +}
> +
> +static int zx2967_i2c_remove(struct platform_device *pdev)
> +{
> + struct zx2967_i2c_info *zx_i2c = platform_get_drvdata(pdev);
> +
> + i2c_del_adapter(&zx_i2c->adap);
> + clk_disable_unprepare(zx_i2c->clk);
> +
> + return 0;
> +}
> +
> +static struct platform_driver zx2967_i2c_driver = {
> + .probe = zx2967_i2c_probe,
> + .remove = zx2967_i2c_remove,
> + .driver = {
> + .name = "zx2967_i2c",
> + .of_match_table = zx2967_i2c_of_match,
> + .pm = ZX2967_I2C_DEV_PM_OPS,
> + },
> +};
> +module_platform_driver(zx2967_i2c_driver);
> +
> +MODULE_AUTHOR("Baoyou Xie <baoyou.xie-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>");
> +MODULE_DESCRIPTION("ZTE zx2967 I2C Bus Controller driver");
> +MODULE_LICENSE("GPL v2");
> --
> 2.7.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
* Re: [PATCH v5 5/5] i2c: mux: pca954x: Add irq-mask-enable to delay enabling irqs
From: Danielle Costantino @ 2017-01-25 9:17 UTC (permalink / raw)
To: Peter Rosin, Phil Reid
Cc: Wolfram Sang, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, linux-i2c, Linux device trees
In-Reply-To: <f203f28f-51c8-a7a0-6e82-16d587310be9-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 7584 bytes --]
On 01/25/2017 12:15 AM, Peter Rosin wrote:
> On 2017-01-25 04:50, Danielle Costantino wrote:
>> On Mon, Jan 23, 2017 at 1:02 AM, Phil Reid <preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org> wrote:
>>> On 20/01/2017 06:56, Peter Rosin wrote:
>>>> On 2017-01-19 08:48, Phil Reid wrote:
>>>>> On 18/01/2017 20:19, Peter Rosin wrote:
>>>>>> On 2017-01-17 09:00, Phil Reid wrote:
>>>
>>> [snip]
>>>
>>>
>>>>>> Hmm, this whole thing is fiddly and while it solves your problem it doesn't
>>>>>> allow for solving the more general problem when there are "problematic"
>>>>>> devices mixed with other devices. At least, I don't see it. And the
>>>>>> limitations we are walking into with tracking number of enables etc suggests
>>>>>> that we are attacking this at the wrong level. Maybe you should try to work
>>>>>> around the hw limitations not in the pca954x driver, but in the irq core?
>>>>>
>>>>> I'm looking at the option of getting the hardware changed to not route
>>>>> the irq for my chips thru the i2c mux. Fortunately the hardware is going thru a
>>>>> revision for some other changes. Messing with the irq core sounds dangerous
>>>>> with my level of knowledge.
>>>>
>>>> Yeah, but I bet you'd get some attention from people with more irq
>>>> experience. That can't be bad :-)
>>>>
>>>>> The other way I think I can tackle it after reading the datasheet for the ltc1760 is that
>>>>> it'll deassert it's irq (smbalert) line when the host sends a ARA request on the bus segment.
>>>>> There's a driver in the kernel for this already, but it's not DT enable and doesn't
>>>>> handle multiple bus segments. I'll have a look at that as well.
>>>>> Pretty sure it would need the mux to become an irq parent as per patch 1-3 of this series.
>>>>> This would be so the system can figure out which segment to do the poll on.
>>>>
>>>> Yeah sounds neater. It has the slight drawback that it may not work
>>>> for pure i2c buses since it an SMB thing??
>> If you need to send SMBus commands like ARA, you should be using an
>> SMBus 2.0+ compatible bus multiplexer. muxes like the pca954x do not
>> automatically select the bus segment that the ARA is destined for. It
>> usually is more efficient to only request the data you need from each
>> device, rather than checking every segment on each interrupt for the
> We are not talking about checking every segment on every interrupt, we
> are talking about checking the segments indicated by the INTx bits in
> the control register.
>
>> cause, one could implement a delayed worker to schedule
>> checking+clearing the interrupt at a time when the bus is selected for
>> use by another slave device on that segment when possible. This will
>> reduce the number of bus setup operations per transfer to slaves on
>> deeper busses, reducing your i2c latency.
> i2c traffic scheduling does not exist in linux to the best of my knowledge,
> it's all handled with a simple mutex AFAIK. So, while traffic scheduling
> is an interesting problem, I think it is out of scope at this time.
>
> If you happen to have a pca954x mux (with irq support) and happen to
> have devices behind it that needs ARA support, I just don't see how any
> of the above is relevant. Yes, it could be more efficient to have the
> hardware done differently, but that is in many cases not a possibility.
> You have to make do with what you have, and if that costs latency, then
> there is little to do about that. You only have to make the new stuff
> optional so that old working setups don't suffer.
Yes traffic scheduling of I2C transactions is out of the scope of the
current problem but I thought that it may be useful to bring it up.
>>>> BTW, why do you need special treatment for multiple segments? Will it not
>>>> simply have an ARA appear on whatever i2c bus the device sits on? And if
>>>> something requests to send an ARA message on a bus that happens to be a
>>>> muxed segment, my mental picture is that the mux will be operated as usual
>>>> so that the ARA appears on the muxed segment. Maybe I'm missing something?
>>>
>>> My think was the following.
>>> When the SMBALERT is asserted a ARA needs to be sent by the master.
>>> If the device sending the SMBALERT is behind a mux when need to know which segment of the bus to enable.
>>> Using shared interrupts should work I think, but you have to iterate thru each bus segment.
>>> If the alert device is nested behind a couple of muxes this could get expensive.
>>> But yeah otherwise I think the correct mux segment will get enabled automatically.
>>> The current SMBALERT driver only seems to attached to the root i2c adapter.
>> Using ARA in a multi mux environment is very expensive, setting up
>> each mux segment and then sending ARA will consume more time than it
>> would take if the mux structure was optimized to service devices on
>> similar busses, reducing the setup operation count. ARA was only
>> implemented on the root bus due to the design of the arbiter and mux
>> cannot guarantee that you are the only owner of the bus when you are
>> disconnected.
> ARA handling should take the irq register of the pca95x into account and
> only send ARAs to indicated mux segments. If more than one segment needs
> servicing, then of course one ARA for each segment needs to be sent. If
> someone builds hardware like this and then expect no latency, that someone
> will hopefully at least learn something.
>
> I do not know how this fits with the existing ARA handling (it probably
> doesn't), but what needs to happen is fairly easy to picture.
>
>> You would also need to handle the cases where segments
>> lock up and must be released using the bus reset mechanism, this
>> resets the IRQs as well. The added cost of a I2C read to coincide the
>> write operation to the mux when the irq pin is asserted is
>> unacceptable for low latency applications.
> I bet there are a lot of corner cases, and yes, this added cost has to be
> optional. Perhaps with each mux child segment opting in for ARA support?
> If a mux then has a way to determine that an ARA isn't needed for some of
> its child segments (by reading some status register), then linux is free
> to not send ARAs there. And more importantly, if no mux child segment opts
> in for ARA support, it should be possible to preserve current behavior...
I agree with making ARA handling for child bus segments optional based
on the need of ARA or other SMBus features. We have needed IRQ support
for muxes for a while now, I was always concerned about dead locks
between setting up the bus and servicing the IRQ. The current patch v6
0/3 appears to handle child IRQs without selecting the bus associated
with it. Is the plan to have the irq handling of the child require an
i2c transaction on its registered bus, which will in turn select that
bus? I also thought i2c transactions were not allowed inside an IRQ
handler? Could a delayed work struct could be used to fire off the read
transaction that would then trigger the nested IRQs (based on the set
bits)? Also the PCA954x has an internal OR of the INT signals coming
into it, not an AND like the patch says. Before allowing IRQs to be
handled by the assertion of the active low INT out signal from the
PCA954x all child interrupts must be de-asserted else the IRQ will
always be set. You could read the value of the interrupt register to see
if all bits are clear before allowing unmasking of the devices interrupts.
>
> Cheers,
> peda
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 213 bytes --]
^ permalink raw reply
* Re: [PATCH v5 5/5] i2c: mux: pca954x: Add irq-mask-enable to delay enabling irqs
From: Peter Rosin @ 2017-01-25 8:15 UTC (permalink / raw)
To: Danielle Costantino, Phil Reid
Cc: Wolfram Sang, robh+dt, mark.rutland, linux-i2c,
Linux device trees
In-Reply-To: <CAAVjN7eGNsQOPiy0HuDq26MPNrD0xk7uxiN2+UO=hyuCSBa5nw@mail.gmail.com>
On 2017-01-25 04:50, Danielle Costantino wrote:
> On Mon, Jan 23, 2017 at 1:02 AM, Phil Reid <preid@electromag.com.au> wrote:
>>
>> On 20/01/2017 06:56, Peter Rosin wrote:
>>>
>>> On 2017-01-19 08:48, Phil Reid wrote:
>>>>
>>>> On 18/01/2017 20:19, Peter Rosin wrote:
>>>>>
>>>>> On 2017-01-17 09:00, Phil Reid wrote:
>>
>>
>> [snip]
>>
>>
>>>>> Hmm, this whole thing is fiddly and while it solves your problem it doesn't
>>>>> allow for solving the more general problem when there are "problematic"
>>>>> devices mixed with other devices. At least, I don't see it. And the
>>>>> limitations we are walking into with tracking number of enables etc suggests
>>>>> that we are attacking this at the wrong level. Maybe you should try to work
>>>>> around the hw limitations not in the pca954x driver, but in the irq core?
>>>>
>>>>
>>>> I'm looking at the option of getting the hardware changed to not route
>>>> the irq for my chips thru the i2c mux. Fortunately the hardware is going thru a
>>>> revision for some other changes. Messing with the irq core sounds dangerous
>>>> with my level of knowledge.
>>>
>>>
>>> Yeah, but I bet you'd get some attention from people with more irq
>>> experience. That can't be bad :-)
>>>
>>>> The other way I think I can tackle it after reading the datasheet for the ltc1760 is that
>>>> it'll deassert it's irq (smbalert) line when the host sends a ARA request on the bus segment.
>>>> There's a driver in the kernel for this already, but it's not DT enable and doesn't
>>>> handle multiple bus segments. I'll have a look at that as well.
>>>> Pretty sure it would need the mux to become an irq parent as per patch 1-3 of this series.
>>>> This would be so the system can figure out which segment to do the poll on.
>>>
>>>
>>> Yeah sounds neater. It has the slight drawback that it may not work
>>> for pure i2c buses since it an SMB thing??
>
> If you need to send SMBus commands like ARA, you should be using an
> SMBus 2.0+ compatible bus multiplexer. muxes like the pca954x do not
> automatically select the bus segment that the ARA is destined for. It
> usually is more efficient to only request the data you need from each
> device, rather than checking every segment on each interrupt for the
We are not talking about checking every segment on every interrupt, we
are talking about checking the segments indicated by the INTx bits in
the control register.
> cause, one could implement a delayed worker to schedule
> checking+clearing the interrupt at a time when the bus is selected for
> use by another slave device on that segment when possible. This will
> reduce the number of bus setup operations per transfer to slaves on
> deeper busses, reducing your i2c latency.
i2c traffic scheduling does not exist in linux to the best of my knowledge,
it's all handled with a simple mutex AFAIK. So, while traffic scheduling
is an interesting problem, I think it is out of scope at this time.
If you happen to have a pca954x mux (with irq support) and happen to
have devices behind it that needs ARA support, I just don't see how any
of the above is relevant. Yes, it could be more efficient to have the
hardware done differently, but that is in many cases not a possibility.
You have to make do with what you have, and if that costs latency, then
there is little to do about that. You only have to make the new stuff
optional so that old working setups don't suffer.
>>>
>>> BTW, why do you need special treatment for multiple segments? Will it not
>>> simply have an ARA appear on whatever i2c bus the device sits on? And if
>>> something requests to send an ARA message on a bus that happens to be a
>>> muxed segment, my mental picture is that the mux will be operated as usual
>>> so that the ARA appears on the muxed segment. Maybe I'm missing something?
>>
>>
>> My think was the following.
>> When the SMBALERT is asserted a ARA needs to be sent by the master.
>> If the device sending the SMBALERT is behind a mux when need to know which segment of the bus to enable.
>> Using shared interrupts should work I think, but you have to iterate thru each bus segment.
>> If the alert device is nested behind a couple of muxes this could get expensive.
>> But yeah otherwise I think the correct mux segment will get enabled automatically.
>> The current SMBALERT driver only seems to attached to the root i2c adapter.
>
> Using ARA in a multi mux environment is very expensive, setting up
> each mux segment and then sending ARA will consume more time than it
> would take if the mux structure was optimized to service devices on
> similar busses, reducing the setup operation count. ARA was only
> implemented on the root bus due to the design of the arbiter and mux
> cannot guarantee that you are the only owner of the bus when you are
> disconnected.
ARA handling should take the irq register of the pca95x into account and
only send ARAs to indicated mux segments. If more than one segment needs
servicing, then of course one ARA for each segment needs to be sent. If
someone builds hardware like this and then expect no latency, that someone
will hopefully at least learn something.
I do not know how this fits with the existing ARA handling (it probably
doesn't), but what needs to happen is fairly easy to picture.
> You would also need to handle the cases where segments
> lock up and must be released using the bus reset mechanism, this
> resets the IRQs as well. The added cost of a I2C read to coincide the
> write operation to the mux when the irq pin is asserted is
> unacceptable for low latency applications.
I bet there are a lot of corner cases, and yes, this added cost has to be
optional. Perhaps with each mux child segment opting in for ARA support?
If a mux then has a way to determine that an ARA isn't needed for some of
its child segments (by reading some status register), then linux is free
to not send ARAs there. And more importantly, if no mux child segment opts
in for ARA support, it should be possible to preserve current behavior...
Cheers,
peda
^ permalink raw reply
* Re: [PATCH v5 5/5] i2c: mux: pca954x: Add irq-mask-enable to delay enabling irqs
From: Danielle Costantino @ 2017-01-25 3:50 UTC (permalink / raw)
To: Phil Reid
Cc: Peter Rosin, Wolfram Sang, robh+dt, mark.rutland, linux-i2c,
Linux device trees
In-Reply-To: <35c35708-3bbc-88d1-1d60-4faf68ca5af9@electromag.com.au>
On Mon, Jan 23, 2017 at 1:02 AM, Phil Reid <preid@electromag.com.au> wrote:
>
> On 20/01/2017 06:56, Peter Rosin wrote:
>>
>> On 2017-01-19 08:48, Phil Reid wrote:
>>>
>>> On 18/01/2017 20:19, Peter Rosin wrote:
>>>>
>>>> On 2017-01-17 09:00, Phil Reid wrote:
>
>
> [snip]
>
>
>>>> Hmm, this whole thing is fiddly and while it solves your problem it doesn't
>>>> allow for solving the more general problem when there are "problematic"
>>>> devices mixed with other devices. At least, I don't see it. And the
>>>> limitations we are walking into with tracking number of enables etc suggests
>>>> that we are attacking this at the wrong level. Maybe you should try to work
>>>> around the hw limitations not in the pca954x driver, but in the irq core?
>>>
>>>
>>> I'm looking at the option of getting the hardware changed to not route
>>> the irq for my chips thru the i2c mux. Fortunately the hardware is going thru a
>>> revision for some other changes. Messing with the irq core sounds dangerous
>>> with my level of knowledge.
>>
>>
>> Yeah, but I bet you'd get some attention from people with more irq
>> experience. That can't be bad :-)
>>
>>> The other way I think I can tackle it after reading the datasheet for the ltc1760 is that
>>> it'll deassert it's irq (smbalert) line when the host sends a ARA request on the bus segment.
>>> There's a driver in the kernel for this already, but it's not DT enable and doesn't
>>> handle multiple bus segments. I'll have a look at that as well.
>>> Pretty sure it would need the mux to become an irq parent as per patch 1-3 of this series.
>>> This would be so the system can figure out which segment to do the poll on.
>>
>>
>> Yeah sounds neater. It has the slight drawback that it may not work
>> for pure i2c buses since it an SMB thing??
If you need to send SMBus commands like ARA, you should be using an
SMBus 2.0+ compatible bus multiplexer. muxes like the pca954x do not
automatically select the bus segment that the ARA is destined for. It
usually is more efficient to only request the data you need from each
device, rather than checking every segment on each interrupt for the
cause, one could implement a delayed worker to schedule
checking+clearing the interrupt at a time when the bus is selected for
use by another slave device on that segment when possible. This will
reduce the number of bus setup operations per transfer to slaves on
deeper busses, reducing your i2c latency.
>>
>> BTW, why do you need special treatment for multiple segments? Will it not
>> simply have an ARA appear on whatever i2c bus the device sits on? And if
>> something requests to send an ARA message on a bus that happens to be a
>> muxed segment, my mental picture is that the mux will be operated as usual
>> so that the ARA appears on the muxed segment. Maybe I'm missing something?
>
>
> My think was the following.
> When the SMBALERT is asserted a ARA needs to be sent by the master.
> If the device sending the SMBALERT is behind a mux when need to know which segment of the bus to enable.
> Using shared interrupts should work I think, but you have to iterate thru each bus segment.
> If the alert device is nested behind a couple of muxes this could get expensive.
> But yeah otherwise I think the correct mux segment will get enabled automatically.
> The current SMBALERT driver only seems to attached to the root i2c adapter.
Using ARA in a multi mux environment is very expensive, setting up
each mux segment and then sending ARA will consume more time than it
would take if the mux structure was optimized to service devices on
similar busses, reducing the setup operation count. ARA was only
implemented on the root bus due to the design of the arbiter and mux
cannot guarantee that you are the only owner of the bus when you are
disconnected. You would also need to handle the cases where segments
lock up and must be released using the bus reset mechanism, this
resets the IRQs as well. The added cost of a I2C read to coincide the
write operation to the mux when the irq pin is asserted is
unacceptable for low latency applications.
>>
>>
>>> But p4-5 could be dropped which is where we're stuck I think.
>>
>>
>> Yes, I dislike to add a workaround for a specific case that might get
>> in the way for anybody wishing to fix a bigger, more generic, problem...
>>
>>> Looking at this approach it shouldn't matter if the ltc1760 driver has registered yet or not.
>>> This approach possibly has a lot more generic appeal I think.
>>>
>>> Thoughts on just submitting p1-3 for now while I figure out the SMB alert approach?
>>
>>
>> Yes, looks like a plan. Thanks in advance!
>
>
> Thanks, I'll do a new version with just p1-3.
>
>
>>>>>
>>>>> - if (!data->irq_mask)
>>>>> - enable_irq(data->client->irq);
>>>>> data->irq_mask |= BIT(pos);
>>>>> + if (!data->irq_enabled
>>>>> + && (data->irq_mask & mask_enable) == mask_enable) {
>>>>
>>>>
>>>> I think the coding standard says that the && should be at the end of the
>>>> first line. Didn't checkpatch complain?
>>>
>>>
>>> No it didn't complain. and I wasn't sure which way to do this.
>>
>>
>> Ah, you need the --strict option for that to show up...
>>
> Haven't come across that option, I'll give it a try in future.
>
> --
> Regards
> Phil Reid
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
- Danielle Costantino
^ permalink raw reply
* [PATCH v6 0/3] i2c: mux: pca954x: Add interrupt controller support
From: Phil Reid @ 2017-01-25 1:31 UTC (permalink / raw)
To: peda, wsa, robh+dt, mark.rutland, preid, linux-i2c, devicetree
Various muxes can aggregate multiple interrupts from each i2c bus.
All of the muxes with interrupt support combine the active low irq lines
using an internal 'and' function and generate a combined active low
output. The muxes do provide the ability to read a control register to
determine which irq is active. By making the mux an irq controller isr
latenct can potentially be reduced by reading the status register and
then only calling the registered isr on that bus segment.
Changes from v5:
- p3 Added Peter's Ack.
- Drop p4/5
Changes from v4:
- p4: Change definition of irq_mask_enable to an array.
- p4: Removed acks due to change requested by Peter
- p5: Parse array of enables. Currently only supports 1 chip
But dt specification will allow expansion to handle
multple irq consume chips to be registered on a bus segment
- p5: Fix up logic related to enabling and disable irq's.
Use a flag to indicate when irq has been enabled.
Changes from v3:
- p3: Add spin lock to irq mask / unmask.
- p4: Add Rob's ack.
Changes from v2:
- p1: Added Acked-by
- p5: fixup 2 typos
Changes from v1:
- Update for new ACPI table
- Fix typo in documentation
- Fix typo in function names
- Fix typo in irq name
- Added spaces around '+' / '='
- Change goto label names
- Change property name from i2c-mux-irq-mask-en to nxp,irq-mask-enable
- Change variable name irq_mask_en to irq_mask_enable
- Add commentt about irq_mask_enable
- Added Acked-By's
Phil Reid (3):
i2c: mux: pca954x: Add missing pca9542 definition to chip_desc
dt: bindings: i2c-mux-pca954x: Add documentation for interrupt
controller
i2c: mux: pca954x: Add interrupt controller support
.../devicetree/bindings/i2c/i2c-mux-pca954x.txt | 14 +-
drivers/i2c/muxes/i2c-mux-pca954x.c | 150 ++++++++++++++++++++-
2 files changed, 159 insertions(+), 5 deletions(-)
--
1.8.3.1
^ permalink raw reply
* [PATCH v6 2/3] dt: bindings: i2c-mux-pca954x: Add documentation for interrupt controller
From: Phil Reid @ 2017-01-25 1:31 UTC (permalink / raw)
To: peda, wsa, robh+dt, mark.rutland, preid, linux-i2c, devicetree
In-Reply-To: <1485307868-5408-1-git-send-email-preid@electromag.com.au>
Various muxes can aggregate multiple irq lines and provide a control
register to determine the active line. Add bindings for interrupt
controller support.
Acked-by: Peter Rosin <peda@axentia.se>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Phil Reid <preid@electromag.com.au>
---
Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
index cf53d5f..aa09704 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
@@ -19,7 +19,14 @@ Optional Properties:
- i2c-mux-idle-disconnect: Boolean; if defined, forces mux to disconnect all
children in idle state. This is necessary for example, if there are several
multiplexers on the bus and the devices behind them use same I2C addresses.
-
+ - interrupt-parent: Phandle for the interrupt controller that services
+ interrupts for this device.
+ - interrupts: Interrupt mapping for IRQ.
+ - interrupt-controller: Marks the device node as an interrupt controller.
+ - #interrupt-cells : Should be two.
+ - first cell is the pin number
+ - second cell is used to specify flags.
+ See also Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
Example:
@@ -29,6 +36,11 @@ Example:
#size-cells = <0>;
reg = <0x74>;
+ interrupt-parent = <&ipic>;
+ interrupts = <17 IRQ_TYPE_LEVEL_LOW>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+
i2c@2 {
#address-cells = <1>;
#size-cells = <0>;
--
1.8.3.1
^ permalink raw reply related
* [PATCH v6 3/3] i2c: mux: pca954x: Add interrupt controller support
From: Phil Reid @ 2017-01-25 1:31 UTC (permalink / raw)
To: peda, wsa, robh+dt, mark.rutland, preid, linux-i2c, devicetree
In-Reply-To: <1485307868-5408-1-git-send-email-preid@electromag.com.au>
Various muxes can aggregate multiple interrupts from each i2c bus.
All of the muxes with interrupt support combine the active low irq lines
using an internal 'and' function and generate a combined active low
output. The muxes do provide the ability to read a control register to
determine which irq is active. By making the mux an irq controller isr
latency can potentially be reduced by reading the status register and
then only calling the registered isr on that bus segment.
As there is no irq masking on the mux irq are disabled until irq_unmask is
called at least once.
Acked-by: Peter Rosin <peda@axentia.se>
Signed-off-by: Phil Reid <preid@electromag.com.au>
---
drivers/i2c/muxes/i2c-mux-pca954x.c | 141 +++++++++++++++++++++++++++++++++++-
1 file changed, 139 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
index bbf088e..f55da88 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -41,14 +41,20 @@
#include <linux/i2c.h>
#include <linux/i2c-mux.h>
#include <linux/i2c/pca954x.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
#include <linux/module.h>
#include <linux/of.h>
#include <linux/of_device.h>
+#include <linux/of_irq.h>
#include <linux/pm.h>
#include <linux/slab.h>
+#include <linux/spinlock.h>
#define PCA954X_MAX_NCHANS 8
+#define PCA954X_IRQ_OFFSET 4
+
enum pca_type {
pca_9540,
pca_9542,
@@ -63,6 +69,7 @@ enum pca_type {
struct chip_desc {
u8 nchans;
u8 enable; /* used for muxes only */
+ u8 has_irq;
enum muxtype {
pca954x_ismux = 0,
pca954x_isswi
@@ -75,6 +82,10 @@ struct pca954x {
u8 last_chan; /* last register value */
u8 deselect;
struct i2c_client *client;
+
+ struct irq_domain *irq;
+ unsigned int irq_mask;
+ spinlock_t lock;
};
/* Provide specs for the PCA954x types we know about */
@@ -87,19 +98,23 @@ struct pca954x {
[pca_9542] = {
.nchans = 2,
.enable = 0x4,
+ .has_irq = 1,
.muxtype = pca954x_ismux,
},
[pca_9543] = {
.nchans = 2,
+ .has_irq = 1,
.muxtype = pca954x_isswi,
},
[pca_9544] = {
.nchans = 4,
.enable = 0x4,
+ .has_irq = 1,
.muxtype = pca954x_ismux,
},
[pca_9545] = {
.nchans = 4,
+ .has_irq = 1,
.muxtype = pca954x_isswi,
},
[pca_9547] = {
@@ -222,6 +237,114 @@ static int pca954x_deselect_mux(struct i2c_mux_core *muxc, u32 chan)
return pca954x_reg_write(muxc->parent, client, data->last_chan);
}
+static irqreturn_t pca954x_irq_handler(int irq, void *dev_id)
+{
+ struct pca954x *data = dev_id;
+ unsigned int child_irq;
+ int ret, i, handled;
+
+ ret = i2c_smbus_read_byte(data->client);
+ if (ret < 0)
+ return IRQ_NONE;
+
+ for (i = 0; i < data->chip->nchans; i++) {
+ if (ret & BIT(PCA954X_IRQ_OFFSET + i)) {
+ child_irq = irq_linear_revmap(data->irq, i);
+ handle_nested_irq(child_irq);
+ handled++;
+ }
+ }
+ return handled ? IRQ_HANDLED : IRQ_NONE;
+}
+
+static void pca954x_irq_mask(struct irq_data *idata)
+{
+ struct pca954x *data = irq_data_get_irq_chip_data(idata);
+ unsigned int pos = idata->hwirq;
+ unsigned long flags;
+
+ spin_lock_irqsave(&data->lock, flags);
+
+ data->irq_mask &= ~BIT(pos);
+ if (!data->irq_mask)
+ disable_irq(data->client->irq);
+
+ spin_unlock_irqrestore(&data->lock, flags);
+}
+
+static void pca954x_irq_unmask(struct irq_data *idata)
+{
+ struct pca954x *data = irq_data_get_irq_chip_data(idata);
+ unsigned int pos = idata->hwirq;
+ unsigned long flags;
+
+ spin_lock_irqsave(&data->lock, flags);
+
+ if (!data->irq_mask)
+ enable_irq(data->client->irq);
+ data->irq_mask |= BIT(pos);
+
+ spin_unlock_irqrestore(&data->lock, flags);
+}
+
+static int pca954x_irq_set_type(struct irq_data *idata, unsigned int type)
+{
+ if ((type & IRQ_TYPE_SENSE_MASK) != IRQ_TYPE_LEVEL_LOW)
+ return -EINVAL;
+ return 0;
+}
+
+static struct irq_chip pca954x_irq_chip = {
+ .name = "i2c-mux-pca954x",
+ .irq_mask = pca954x_irq_mask,
+ .irq_unmask = pca954x_irq_unmask,
+ .irq_set_type = pca954x_irq_set_type,
+};
+
+static int pca954x_irq_setup(struct i2c_mux_core *muxc)
+{
+ struct pca954x *data = i2c_mux_priv(muxc);
+ struct i2c_client *client = data->client;
+ int c, err, irq;
+
+ if (!data->chip->has_irq || client->irq <= 0)
+ return 0;
+
+ spin_lock_init(&data->lock);
+
+ data->irq = irq_domain_add_linear(client->dev.of_node,
+ data->chip->nchans,
+ &irq_domain_simple_ops, data);
+ if (!data->irq)
+ return -ENODEV;
+
+ for (c = 0; c < data->chip->nchans; c++) {
+ irq = irq_create_mapping(data->irq, c);
+ irq_set_chip_data(irq, data);
+ irq_set_chip_and_handler(irq, &pca954x_irq_chip,
+ handle_simple_irq);
+ }
+
+ err = devm_request_threaded_irq(&client->dev, data->client->irq, NULL,
+ pca954x_irq_handler,
+ IRQF_ONESHOT | IRQF_SHARED,
+ "pca954x", data);
+ if (err)
+ goto err_req_irq;
+
+ disable_irq(data->client->irq);
+
+ return 0;
+err_req_irq:
+ for (c = 0; c < data->chip->nchans; c++) {
+ irq = irq_find_mapping(data->irq, c);
+ irq_dispose_mapping(irq);
+ }
+ irq_domain_remove(data->irq);
+
+ return err;
+}
+
/*
* I2C init/probing/exit functions
*/
@@ -286,6 +409,10 @@ static int pca954x_probe(struct i2c_client *client,
idle_disconnect_dt = of_node &&
of_property_read_bool(of_node, "i2c-mux-idle-disconnect");
+ ret = pca954x_irq_setup(muxc);
+ if (ret)
+ goto fail_del_adapters;
+
/* Now create an adapter for each channel */
for (num = 0; num < data->chip->nchans; num++) {
bool idle_disconnect_pd = false;
@@ -311,7 +438,7 @@ static int pca954x_probe(struct i2c_client *client,
dev_err(&client->dev,
"failed to register multiplexed adapter"
" %d as bus %d\n", num, force);
- goto virt_reg_failed;
+ goto fail_del_adapters;
}
}
@@ -322,7 +449,7 @@ static int pca954x_probe(struct i2c_client *client,
return 0;
-virt_reg_failed:
+fail_del_adapters:
i2c_mux_del_adapters(muxc);
return ret;
}
@@ -330,6 +457,16 @@ static int pca954x_probe(struct i2c_client *client,
static int pca954x_remove(struct i2c_client *client)
{
struct i2c_mux_core *muxc = i2c_get_clientdata(client);
+ struct pca954x *data = i2c_mux_priv(muxc);
+ int c, irq;
+
+ if (data->irq) {
+ for (c = 0; c < data->chip->nchans; c++) {
+ irq = irq_find_mapping(data->irq, c);
+ irq_dispose_mapping(irq);
+ }
+ irq_domain_remove(data->irq);
+ }
i2c_mux_del_adapters(muxc);
return 0;
--
1.8.3.1
^ permalink raw reply related
* [PATCH v6 1/3] i2c: mux: pca954x: Add missing pca9542 definition to chip_desc
From: Phil Reid @ 2017-01-25 1:31 UTC (permalink / raw)
To: peda-koto5C5qi+TLoDKTGw+V6w, wsa-z923LK4zBo2bacvFa/9K2g,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1485307868-5408-1-git-send-email-preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>
The spec for the pca954x was missing. This chip is the same as the pca9540
except that it has interrupt lines. While the i2c_device_id table mapped
the pca9542 to the pca9540 definition the compatible table did not. In
preparation for irq support add the pca9542 definition.
Acked-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
Signed-off-by: Phil Reid <preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>
---
drivers/i2c/muxes/i2c-mux-pca954x.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
index dd18b9c..bbf088e 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -84,6 +84,11 @@ struct pca954x {
.enable = 0x4,
.muxtype = pca954x_ismux,
},
+ [pca_9542] = {
+ .nchans = 2,
+ .enable = 0x4,
+ .muxtype = pca954x_ismux,
+ },
[pca_9543] = {
.nchans = 2,
.muxtype = pca954x_isswi,
@@ -110,7 +115,7 @@ struct pca954x {
static const struct i2c_device_id pca954x_id[] = {
{ "pca9540", pca_9540 },
- { "pca9542", pca_9540 },
+ { "pca9542", pca_9542 },
{ "pca9543", pca_9543 },
{ "pca9544", pca_9544 },
{ "pca9545", pca_9545 },
@@ -124,7 +129,7 @@ struct pca954x {
#ifdef CONFIG_ACPI
static const struct acpi_device_id pca954x_acpi_ids[] = {
{ .id = "PCA9540", .driver_data = pca_9540 },
- { .id = "PCA9542", .driver_data = pca_9540 },
+ { .id = "PCA9542", .driver_data = pca_9542 },
{ .id = "PCA9543", .driver_data = pca_9543 },
{ .id = "PCA9544", .driver_data = pca_9544 },
{ .id = "PCA9545", .driver_data = pca_9545 },
--
1.8.3.1
--
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
* Issue with i2c-designware-platdrv's suspend/runtime-suspend handling
From: John Stultz @ 2017-01-24 22:03 UTC (permalink / raw)
To: Jarkko Nikula
Cc: Andy Shevchenko, Mika Westerberg, Wolfram Sang, linux-i2c, lkml,
zhangfei, Guodong Xu, Amit Pundir
I noticed that with my hikey board, on resume from suspend I'm getting
the following WARNING:
[ 54.334054] ------------[ cut here ]------------
[ 54.334077] WARNING: CPU: 0 PID: 2217 at drivers/clk/clk.c:594
clk_core_disable+0x20/0x78
[ 54.334080]
[ 54.334090] CPU: 0 PID: 2217 Comm: system_server Not tainted
4.9.0-00046-gee9ec2c #2067
[ 54.334094] Hardware name: HiKey Development Board (DT)
[ 54.334099] task: ffffffc074863200 task.stack: ffffffc061d1c000
[ 54.334105] PC is at clk_core_disable+0x20/0x78
[ 54.334111] LR is at clk_core_disable_lock+0x20/0x38
[ 54.334116] pc : [<ffffff80084ab728>] lr : [<ffffff80084ab848>]
pstate: 800001c5
[ 54.334119] sp : ffffffc061d1faf0
[ 54.334128] x29: ffffffc061d1faf0 x28: 0000000000000000
[ 54.334136] x27: 0000000000000002 x26: ffffff8008d47000
[ 54.334143] x25: ffffff8008596000 x24: ffffff8008d15498
[ 54.334151] x23: ffffff8008db9000 x22: ffffffc075074870
[ 54.334158] x21: 0000000000000002 x20: ffffffc005f09500
[ 54.334165] x19: 0000000000000140 x18: 0000000000000001
[ 54.334172] x17: 0000000000000000 x16: 0000000000000000
[ 54.334180] x15: ffffffc005f43dc0 x14: 0000000000010000
[ 54.334187] x13: ffffff8008d906f8 x12: ffffff8008d15790
[ 54.334196] x11: ffffff8008d15000 x10: ffffff8008d8d000
[ 54.334204] x9 : 0000000000000000 x8 : ffffffc077f26218
[ 54.334212] x7 : 0000000000000000 x6 : 0000000000000000
[ 54.334220] x5 : ffffffc077f2b090 x4 : ffffffc061d1fa80
[ 54.334228] x3 : ffffff80084ab62c x2 : ffffff80084ab62c
[ 54.334236] x1 : 0000000000000000 x0 : ffffffc005f09500
[ 54.334239]
[ 54.334243] ---[ end trace 6474a624fb2fd658 ]---
[ 54.334248] Call trace:
[ 54.334254] Exception stack(0xffffffc061d1f920 to 0xffffffc061d1fa50)
[ 54.334262] f920: 0000000000000140 0000008000000000
ffffffc061d1faf0 ffffff80084ab728
[ 54.334269] f940: ffffff8008ccc090 ffffffc074863200
0000000000000000 6f6674616c703d4d
[ 54.334276] f960: ffffffc061d1f970 ffffff8008087990
ffffffc061d1f9b0 ffffff8008450c6c
[ 54.334283] f980: ffffff8008cc8000 ffffffc061d1fa90
ffffff8008ccc090 ffffffc074863200
[ 54.334290] f9a0: ffffff8008db6170 ffffffc061d1fa08
ffffffc061d1f9c0 ffffff8008087990
[ 54.334297] f9c0: ffffffc005f09500 0000000000000000
ffffff80084ab62c ffffff80084ab62c
[ 54.334303] f9e0: ffffffc061d1fa80 ffffffc077f2b090
0000000000000000 0000000000000000
[ 54.334310] fa00: ffffffc077f26218 0000000000000000
ffffff8008d8d000 ffffff8008d15000
[ 54.334317] fa20: ffffff8008d15790 ffffff8008d906f8
0000000000010000 ffffffc005f43dc0
[ 54.334322] fa40: 0000000000000000 0000000000000000
[ 54.334328] [<ffffff80084ab728>] clk_core_disable+0x20/0x78
[ 54.334336] [<ffffff80084ad26c>] clk_disable+0x1c/0x30
[ 54.334349] [<ffffff80086d926c>] i2c_dw_plat_prepare_clk.isra.0+0x44/0x80
[ 54.334356] [<ffffff80086d930c>] dw_i2c_plat_suspend+0x24/0x38
[ 54.334367] [<ffffff800858ad54>] platform_pm_suspend+0x24/0x58
[ 54.334377] [<ffffff8008595558>] dpm_run_callback.isra.7+0x20/0x68
[ 54.334385] [<ffffff8008595fb4>] __device_suspend+0x10c/0x298
[ 54.334393] [<ffffff8008597154>] dpm_suspend+0x10c/0x228
[ 54.334401] [<ffffff8008597540>] dpm_suspend_start+0x68/0x78
[ 54.334412] [<ffffff80080fa058>] suspend_devices_and_enter+0xb8/0x458
[ 54.334419] [<ffffff80080fa5e4>] pm_suspend+0x1ec/0x248
[ 54.334426] [<ffffff80080f924c>] state_store+0x94/0xa8
[ 54.334434] [<ffffff80084366dc>] kobj_attr_store+0x14/0x28
[ 54.334444] [<ffffff8008249a28>] sysfs_kf_write+0x48/0x58
[ 54.334451] [<ffffff8008248d20>] kernfs_fop_write+0xb0/0x1d8
[ 54.334461] [<ffffff80081cd714>] __vfs_write+0x1c/0x100
[ 54.334469] [<ffffff80081cd9a0>] vfs_write+0xa0/0x1b8
[ 54.334477] [<ffffff80081cdb9c>] SyS_write+0x44/0xa0
[ 54.334485] [<ffffff8008082ef0>] el0_svc_naked+0x24/0x28
Doing some further debugging, it seems the problem is that the device
is being runtime suspended, and then at suspend time, we're calling
the same logic, calling i2c_dw_plat_prepare_clk, which causes the clk
count warning.
Removing the runtime pm ops:
- SET_RUNTIME_PM_OPS(dw_i2c_plat_suspend, dw_i2c_plat_resume, NULL)
+// SET_RUNTIME_PM_OPS(dw_i2c_plat_suspend, dw_i2c_plat_resume, NULL)
seems to avoid the warning, but clearly isn't ideal. :)
Should there be some logic keep track of the suspend state for the
dw_i2c_dev device so we don't try to suspend (or resume) it twice? Or
is there something else I'm missing to keep this from happening?
thanks
-john
^ permalink raw reply
* Re: [PATCH v2 06/13] i2c: designware-baytrail: Disallow the CPU to enter C6 or C7 while holding the punit semaphore
From: Hans de Goede @ 2017-01-24 16:48 UTC (permalink / raw)
To: Andy Shevchenko, Daniel Vetter, Jani Nikula,
Ville Syrjälä, Jarkko Nikula, Wolfram Sang, Len Brown,
Thomas Gleixner, H . Peter Anvin
Cc: intel-gfx, dri-devel, Mika Westerberg, Takashi Iwai,
russianneuromancer @ ya . ru, linux-i2c
In-Reply-To: <1485251504.2133.296.camel@linux.intel.com>
Hi,
On 01/24/2017 10:51 AM, Andy Shevchenko wrote:
> On Mon, 2017-01-23 at 22:09 +0100, Hans de Goede wrote:
>> On my cherrytrail tablet with axp288 pmic, just doing a bunch of
>> repeated
>> reads from the pmic, e.g. "i2cdump -y 14 0x34" would lookup the tablet
>> in
>> 1 - 3 runs guaranteed.
>>
>> This seems to be causes by the cpu trying to enter C6 or C7 while we
>> hold
>> the punit bus semaphore, at which point everything just hangs.
>>
>> Avoid this by disallowing the CPU to enter C6 or C7 before acquiring
>> the
>> punit bus semaphore.
>
>
>> Changes in v5:
>> -Update the pm_qos for a latency of 0 *before* requesting the
>> semaphore,
>> instead of doing it while waiting for the request to be acked
>
> So, that's why you put to reset_semaphore() instead of
> baytrail_i2c_release(), right?
That is why the goto out gets introduced, baytrail_i2c_release()
and reset_semaphore() are functionally equivalent,
baytrail_i2c_release() does a bunch of argument error checking
and then calls reset_semaphore().
Regards,
Hans
> I perhaps missed your answer to my comment about that.
>
>>> @@ -56,6 +57,8 @@ static void reset_semaphore(struct dw_i2c_dev
>>> *dev)
>> data &= ~PUNIT_SEMAPHORE_BIT;
>> if (iosf_mbi_write(BT_MBI_UNIT_PMC, MBI_REG_WRITE,
>> PUNIT_SEMAPHORE, data))
>> dev_err(dev->dev, "iosf failed to reset punit
>> semaphore during write\n");
>> +
>> + pm_qos_update_request(&dev->pm_qos, PM_QOS_DEFAULT_VALUE);
>> }
>
^ permalink raw reply
* Re: [PATCH v3 0/3] Init device ids from ACPI of_compatible
From: Andy Shevchenko @ 2017-01-24 15:19 UTC (permalink / raw)
To: Jarkko Nikula
Cc: Dan O'Donovan,
linux-acpi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Rafael J . Wysocki, Mika Westerberg, Mark Brown, Len Brown,
linux-i2c-u79uwXL29TY76Z2rM5mHXA, Wolfram Sang, linux-spi,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <6ff3138c-ac16-d310-28b7-cee0b0e5f11c-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
On Tue, Jan 24, 2017 at 5:11 PM, Jarkko Nikula <jarkko.nikula-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
> After addressing comments from Andy
>
> Reviewed-by: Jarkko Nikula <jarkko.nikula-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> Tested-by: Jarkko Nikula <jarkko.nikula-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> ---------------------------------------------------------------------
> Intel Finland Oy
> Registered Address: PL 281, 00181 Helsinki Business Identity Code: 0357606 -
> 4 Domiciled in Helsinki
> This e-mail and any attachments may contain confidential material for
> the sole use of the intended recipient(s). Any review or distribution
> by others is strictly prohibited. If you are not the intended
> recipient, please contact the sender and delete all copies.
Ai-ai-ai ^^^
--
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" 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
* Re: [PATCH v3 0/3] Init device ids from ACPI of_compatible
From: Jarkko Nikula @ 2017-01-24 15:11 UTC (permalink / raw)
To: Dan O'Donovan, linux-acpi, Rafael J . Wysocki,
Mika Westerberg, Mark Brown
Cc: Len Brown, linux-i2c, Wolfram Sang, linux-spi, linux-kernel
In-Reply-To: <1485187737-22414-1-git-send-email-dan@emutex.com>
On 01/23/2017 06:08 PM, Dan O'Donovan wrote:
> (Re-submitting this patch set originally created by Leonard Crestez,
> possibly abandoned by Leonard due to a change in employment)
>
> When using devicetree, stuff like i2c_client.name or spi_device.modalias
> is initialized to the first DT compatible id with the vendor prefix
> stripped. Since some drivers rely on this in order to differentiate between
> hardware variants try to replicate it when using ACPI with DT ids.
>
> This also makes it so that the i2c_device_id parameter passed to probe is
> non-NULL when matching with ACPI and DT ids.
>
> Tested using ACPI overlays but there is no actual dependency. This series
> just extends the PRP0001 feature to be more useful for I2C/SPI.
>
> The patches only touches the ACPI-specific parts of the i2c and spi core.
>
> Here is an example .dsl for an SPI accelerometer connected to minnowboard max:
>
> Device (ACCL)
> {
> Name (_ADR, Zero)
> Name (_HID, "PRP0001")
> Name (_UID, One)
>
> Method (_CRS, 0, Serialized)
> {
> Name (RBUF, ResourceTemplate ()
> {
> SPISerialBus(1, PolarityLow, FourWireMode, 16,
> ControllerInitiated, 1000000, ClockPolarityLow,
> ClockPhaseFirst, "\\_SB.SPI1",)
> GpioInt (Edge, ActiveHigh, Exclusive, PullDown, 0x0000,
> "\\_SB.GPO2", 0x00, ResourceConsumer, , )
> { // Pin list
> 1
> }
> })
> Return (RBUF)
> }
> Name (_DSD, Package ()
> {
> ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> Package ()
> {
> Package () {"compatible", "st,lis3dh"},
> }
> })
> }
>
> Link to v2: https://lkml.org/lkml/2016/7/13/392
> Changes:
> * Use appropriate subject prefix for each subsystem (Mark Brown)
> * Use ACPI info as before if getting OF info fails (Mark Brown)
> * Minor cosmetic/readability improvements (Rafael J. Wysocki)
>
> Link to v1: https://www.spinics.net/lists/linux-acpi/msg66469.html
> Changes:
> * Rebase on after acpi overlays got it.
> * Change acpi_of_modalias outlen param to size_t
> * Use {} after else
>
> Crestez Dan Leonard (3):
> ACPI / bus: Export acpi_of_modalias equiv of of_modalias_node
> i2c: acpi: Initialize info.type from of_compatible
> spi: acpi: Initialize modalias from of_compatible
>
> drivers/acpi/bus.c | 35 +++++++++++++++++++++++++++++++++++
> drivers/i2c/i2c-core.c | 8 +++++++-
> drivers/spi/spi.c | 10 +++++++++-
> include/acpi/acpi_bus.h | 1 +
> 4 files changed, 52 insertions(+), 2 deletions(-)
>
After addressing comments from Andy
Reviewed-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Tested-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki
Business Identity Code: 0357606 - 4
Domiciled in Helsinki
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
^ permalink raw reply
* Re: [PATCH 1/2] Documentation: devicetree: Add i2c binding for mediatek MT2701 Soc Platform
From: Matthias Brugger @ 2017-01-24 15:03 UTC (permalink / raw)
To: Jun Gao, Wolfram Sang
Cc: srv_heupstream, devicetree, linux-i2c, linux-arm-kernel,
linux-kernel, linux-mediatek, erin.lo
In-Reply-To: <1484719658-11901-2-git-send-email-jun.gao@mediatek.com>
On 01/18/2017 07:07 AM, Jun Gao wrote:
> From: Jun Gao <jun.gao@mediatek.com>
>
> This add i2c DT binding to i2c-mt6577.txt for MT2701.
>
> Signed-off-by: Jun Gao <jun.gao@mediatek.com>
> ---
> .../devicetree/bindings/i2c/i2c-mt6577.txt | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mt6577.txt
> b/Documentation/devicetree/bindings/i2c/i2c-mt6577.txt
> index 0ce6fa3..ef22ecf 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-mt6577.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c-mt6577.txt
> @@ -4,11 +4,12 @@ The Mediatek's I2C controller is used to interface with
> I2C devices.
>
> Required properties:
> - compatible: value should be either of the following.
> - (a) "mediatek,mt6577-i2c", for i2c compatible with mt6577 i2c.
> - (b) "mediatek,mt6589-i2c", for i2c compatible with mt6589 i2c.
> - (c) "mediatek,mt8127-i2c", for i2c compatible with mt8127 i2c.
> - (d) "mediatek,mt8135-i2c", for i2c compatible with mt8135 i2c.
> - (e) "mediatek,mt8173-i2c", for i2c compatible with mt8173 i2c.
> + "mediatek,mt2701-i2c", for i2c compatible with mt2701 i2c.
> + "mediatek,mt6577-i2c", for i2c compatible with mt6577 i2c.
> + "mediatek,mt6589-i2c", for i2c compatible with mt6589 i2c.
> + "mediatek,mt8127-i2c", for i2c compatible with mt8127 i2c.
> + "mediatek,mt8135-i2c", for i2c compatible with mt8135 i2c.
> + "mediatek,mt8173-i2c", for i2c compatible with mt8173 i2c.
> - reg: physical base address of the controller and dma base, length of
> memory
> mapped region.
> - interrupts: interrupt number to the cpu.
>
Actually what you need for mt2701 is what you put into the dts file:
compatible = "mediatek,mt2701-i2c", "mediatek,mt6577-i2c";
"mediatek,mt2701-i2c" alone is not supported in the driver.
While at it would you mind to fix the bindings description for the other
SoCs as well?
Thanks,
Matthias
^ permalink raw reply
* [PATCH] i2c/pasemi: Remove hardcoded bus numbers on smbus
From: Darren Stevens @ 2017-01-24 12:51 UTC (permalink / raw)
To: wsa, linux-i2c, linux-kernel
Cc: Pat Wall, R.T.Dickinson, linuxppc-dev, Christian Zigotzky
[-- Attachment #1: Type: text/plain, Size: 178 bytes --]
AmigaOS...........: http://yam.ch/
Unix/MacOS/Windows: http://www.mozilla.com/thunderbird/
General information about MIME can be found at:
http://en.wikipedia.org/wiki/MIME
[-- Attachment #2: Type: text/plain, Size: 397 bytes --]
The pasemi smbus controller uses PCI_FUNC(dev->devfn) to define which
number bus to attach to, however this fails when something else is
probed first, for example an ATI Radeon graphics card will claim 9 or
10 busses, including the ones the pasemi wants.
Patch the driver to call i2c_add_adapter rather than
i2c_add_numbered_adapter.
Signed-off-by: Darren Stevens <darren@stevens-zone.net>
---
[-- Attachment #3: i2c.patch --]
[-- Type: text/plain, Size: 826 bytes --]
diff --git a/drivers/i2c/busses/i2c-pasemi.c b/drivers/i2c/busses/i2c-pasemi.c
index df1dbc9..05847fd 100644
--- a/drivers/i2c/busses/i2c-pasemi.c
+++ b/drivers/i2c/busses/i2c-pasemi.c
@@ -365,7 +365,6 @@ static int pasemi_smb_probe(struct pci_dev *dev,
smbus->adapter.class = I2C_CLASS_HWMON | I2C_CLASS_SPD;
smbus->adapter.algo = &smbus_algorithm;
smbus->adapter.algo_data = smbus;
- smbus->adapter.nr = PCI_FUNC(dev->devfn);
/* set up the sysfs linkage to our parent device */
smbus->adapter.dev.parent = &dev->dev;
@@ -373,7 +372,7 @@ static int pasemi_smb_probe(struct pci_dev *dev,
reg_write(smbus, REG_CTL, (CTL_MTR | CTL_MRR |
(CLK_100K_DIV & CTL_CLK_M)));
- error = i2c_add_numbered_adapter(&smbus->adapter);
+ error = i2c_add_adapter(&smbus->adapter);
if (error)
goto out_release_region;
^ permalink raw reply related
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