[RFC v2 2/4] dt-bindings: i2c: mux: demux-pinctrl: add bindings
From: Wolfram Sang
These bindings allow an I2C bus to switch between multiple masters. This
is not hot-swichting because connected I2C slaves will be
re-instantiated. It is meant to select the best I2C core at runtime once
the task is known. Example: Prefer i2c-gpio over another I2C core
because of HW errata affetcing your use case.
Signed-off-by: Wolfram Sang
---
.../devicetree/bindings/i2c/i2c-demux-pinctrl.txt | 155 +
1 file changed, 155 insertions(+)
create mode 100644 Documentation/devicetree/bindings/i2c/i2c-demux-pinctrl.txt
diff --git a/Documentation/devicetree/bindings/i2c/i2c-demux-pinctrl.txt
b/Documentation/devicetree/bindings/i2c/i2c-demux-pinctrl.txt
new file mode 100644
index 00..de571be68f4754
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-demux-pinctrl.txt
@@ -0,0 +1,155 @@
+Pinctrl-based I2C Bus DeMux
+
+This binding describes an I2C bus demultiplexer that uses pin multiplexing to
+route the I2C signals, and represents the pin multiplexing configuration using
+the pinctrl device tree bindings. This may be used to select one I2C IP core at
+runtime which may have a better feature set for a given task than another I2C
+IP core on the SoC. The most simple example is to fall back to GPIO bitbanging
+if your current runtime configuration hits an errata of the internal IP core.
+
++---+
+| SoC |
+| | +-+ +-+
+| ++ | | dev | | dev |
+| |I2C IP Core1|--\ | +-+ +-+
+| ++ \---+ | ||
+||Pinctrl|--|--++
+| ++ +---+ |
+| |I2C IP Core2|--/ |
+| ++ |
+| |
++---+
+
+Required properties:
+- compatible: "i2c-demux-pinctrl"
+- i2c-parent: List of phandles of I2C masters available for selection. The
first
+ one will be used as default.
+- i2c-bus-name: The name of this bus. Also needed as pinctrl-name for the I2C
+ parents.
+
+Furthermore, I2C mux properties and child nodes. See mux.txt in this directory.
+
+Example:
+
+Here is a snipplet for a bus to be demuxed. It contains various i2c clients for
+HDMI, so the bus is named "i2c-hdmi":
+
+ i2chdmi: i2c@8 {
+
+ compatible = "i2c-demux-pinctrl";
+ i2c-parent = <&gpioi2c>, <&iic2>, <&i2c2>;
+ i2c-bus-name = "i2c-hdmi";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ak4643: sound-codec@12 {
+ compatible = "asahi-kasei,ak4643";
+
+ #sound-dai-cells = <0>;
+ reg = <0x12>;
+ };
+
+ composite-in@20 {
+ compatible = "adi,adv7180";
+ reg = <0x20>;
+ remote = <&vin1>;
+
+ port {
+ adv7180: endpoint {
+ bus-width = <8>;
+ remote-endpoint = <&vin1ep0>;
+ };
+ };
+ };
+
+ hdmi@39 {
+ compatible = "adi,adv7511w";
+ reg = <0x39>;
+ interrupt-parent = <&gpio1>;
+ interrupts = <15 IRQ_TYPE_LEVEL_LOW>;
+
+ adi,input-depth = <8>;
+ adi,input-colorspace = "rgb";
+ adi,input-clock = "1x";
+ adi,input-style = <1>;
+ adi,input-justification = "evenly";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ adv7511_in: endpoint {
+ remote-endpoint =
<&du_out_lvds0>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ adv7511_out: endpoint {
+ remote-endpoint = <&hdmi_con>;
+ };
+ };
+ };
+ };
+ };
+
+And for clarification, here are the sn
[RFC v2 4/4] ARM: shmobile: r8a7790: rework dts to use i2c demuxer
From: Wolfram Sang
Create a seperate bus for HDMI related I2C slaves and assign it
to a i2c-gpio master. It can be switched to the i2c-rcar or
i2c-sh_mobile core at runtime.
Signed-off-by: Wolfram Sang
---
arch/arm/boot/dts/r8a7790-lager.dts | 141 ++--
1 file changed, 88 insertions(+), 53 deletions(-)
diff --git a/arch/arm/boot/dts/r8a7790-lager.dts
b/arch/arm/boot/dts/r8a7790-lager.dts
index c553abd711eeb3..d8f0ca8e094dad 100644
--- a/arch/arm/boot/dts/r8a7790-lager.dts
+++ b/arch/arm/boot/dts/r8a7790-lager.dts
@@ -49,6 +49,8 @@
aliases {
serial0 = &scifa0;
serial1 = &scifa1;
+ i2c8 = &i2chdmi;
+ i2c9 = &gpioi2c;
};
chosen {
@@ -252,6 +254,79 @@
#clock-cells = <0>;
clock-frequency = <14850>;
};
+
+
+ gpioi2c: i2c@9 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "i2c-gpio";
+ status = "disabled";
+ gpios = <&gpio5 6 GPIO_ACTIVE_HIGH /* sda */
+&gpio5 5 GPIO_ACTIVE_HIGH /* scl */
+ >;
+ i2c-gpio,delay-us = <5>;
+ };
+
+ i2chdmi: i2c@8 {
+
+ compatible = "i2c-demux-pinctrl";
+ i2c-parent = <&gpioi2c>, <&iic2>, <&i2c2>;
+ i2c-bus-name = "i2c-hdmi";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ak4643: sound-codec@12 {
+ compatible = "asahi-kasei,ak4643";
+
+ #sound-dai-cells = <0>;
+ reg = <0x12>;
+ };
+
+ composite-in@20 {
+ compatible = "adi,adv7180";
+ reg = <0x20>;
+ remote = <&vin1>;
+
+ port {
+ adv7180: endpoint {
+ bus-width = <8>;
+ remote-endpoint = <&vin1ep0>;
+ };
+ };
+ };
+
+ hdmi@39 {
+ compatible = "adi,adv7511w";
+ reg = <0x39>;
+ interrupt-parent = <&gpio1>;
+ interrupts = <15 IRQ_TYPE_LEVEL_LOW>;
+
+ adi,input-depth = <8>;
+ adi,input-colorspace = "rgb";
+ adi,input-clock = "1x";
+ adi,input-style = <1>;
+ adi,input-justification = "evenly";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ adv7511_in: endpoint {
+ remote-endpoint =
<&du_out_lvds0>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ adv7511_out: endpoint {
+ remote-endpoint = <&hdmi_con>;
+ };
+ };
+ };
+ };
+ };
};
&du {
@@ -352,6 +427,11 @@
renesas,function = "iic1";
};
+ i2c2_pins: i2c2 {
+ renesas,groups = "i2c2";
+ renesas,function = "i2c2";
+ };
+
iic2_pins: iic2 {
renesas,groups = "iic2";
renesas,function = "iic2";
@@ -532,63 +612,18 @@
pinctrl-names = "default";
};
-&iic2 {
- status = "okay";
- pinctrl-0 = <&iic2_pins>;
- pinctrl-names = "default";
+&i2c2 {
+ pinctrl-0 = <&i2c2_pins>;
+ pinctrl-names = "i2c-hdmi";
clock-frequency = <10>;
+};
- ak4643: codec@12 {
- compatible = "asahi-kasei,ak4643";
- #sound-dai-cells = <0>;
- reg = <0x12>;
- };
-
- composite-in@20 {
- compatible = "adi,adv7180";
- reg = <0x20>;
- remote = <&vin1>;
-
- port {
- adv7180: endpoint {
-
[RFC v2 3/4] i2c: mux: demux-pinctrl: add driver
From: Wolfram Sang
This driver allows an I2C bus to switch between multiple masters. This
is not hot-swichting because connected I2C slaves will be
re-instantiated. It is meant to select the best I2C core at runtime once
the task is known. Example: Prefer i2c-gpio over another I2C core
because of HW errata affetcing your use case.
Signed-off-by: Wolfram Sang
---
drivers/i2c/muxes/Kconfig | 9 ++
drivers/i2c/muxes/Makefile| 2 +
drivers/i2c/muxes/i2c-demux-pinctrl.c | 276 ++
3 files changed, 287 insertions(+)
create mode 100644 drivers/i2c/muxes/i2c-demux-pinctrl.c
diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig
index f06b0e24673b87..7b5da5ff7b16f9 100644
--- a/drivers/i2c/muxes/Kconfig
+++ b/drivers/i2c/muxes/Kconfig
@@ -72,4 +72,13 @@ config I2C_MUX_REG
This driver can also be built as a module. If so, the module
will be called i2c-mux-reg.
+config I2C_DEMUX_PINCTRL
+ tristate "pinctrl-based I2C demultiplexer"
+ depends on PINCTRL
+ select OF_DYNAMIC
+ help
+ If you say yes to this option, support will be included for an I2C
+ demultiplexer that uses the pinctrl subsystem. This is useful if you
+ want to change the I2C master at run-time depending on features.
+
endmenu
diff --git a/drivers/i2c/muxes/Makefile b/drivers/i2c/muxes/Makefile
index e89799b76a9280..7c267c29b19196 100644
--- a/drivers/i2c/muxes/Makefile
+++ b/drivers/i2c/muxes/Makefile
@@ -3,6 +3,8 @@
obj-$(CONFIG_I2C_ARB_GPIO_CHALLENGE) += i2c-arb-gpio-challenge.o
+obj-$(CONFIG_I2C_DEMUX_PINCTRL)+= i2c-demux-pinctrl.o
+
obj-$(CONFIG_I2C_MUX_GPIO) += i2c-mux-gpio.o
obj-$(CONFIG_I2C_MUX_PCA9541) += i2c-mux-pca9541.o
obj-$(CONFIG_I2C_MUX_PCA954x) += i2c-mux-pca954x.o
diff --git a/drivers/i2c/muxes/i2c-demux-pinctrl.c
b/drivers/i2c/muxes/i2c-demux-pinctrl.c
new file mode 100644
index 00..5ec2779b176432
--- /dev/null
+++ b/drivers/i2c/muxes/i2c-demux-pinctrl.c
@@ -0,0 +1,276 @@
+/*
+ * Pinctrl based I2C DeMultiplexer
+ *
+ * Copyright (C) 2015-16 by Wolfram Sang, Sang Engineering
+ * Copyright (C) 2015-16 by Renesas Electronics Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; version 2 of the License.
+ *
+ * See the bindings doc for DTS setup.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+struct i2c_demux_pinctrl_chan {
+ struct device_node *parent_np;
+ struct i2c_adapter *parent_adap;
+ struct of_changeset chgset;
+};
+
+struct i2c_demux_pinctrl_priv {
+ int cur_chan;
+ int num_chan;
+ struct device *dev;
+ const char *bus_name;
+ struct i2c_adapter cur_adap;
+ struct i2c_algorithm algo;
+ struct i2c_demux_pinctrl_chan chan[];
+};
+
+static struct property status_okay = { .name = "status", .length = 3, .value =
"ok" };
+
+static int i2c_demux_master_xfer(struct i2c_adapter *adap, struct i2c_msg
msgs[], int num)
+{
+ struct i2c_demux_pinctrl_priv *priv = adap->algo_data;
+ struct i2c_adapter *parent = priv->chan[priv->cur_chan].parent_adap;
+
+ return __i2c_transfer(parent, msgs, num);
+}
+
+static u32 i2c_demux_functionality(struct i2c_adapter *adap)
+{
+ struct i2c_demux_pinctrl_priv *priv = adap->algo_data;
+ struct i2c_adapter *parent = priv->chan[priv->cur_chan].parent_adap;
+
+ return parent->algo->functionality(parent);
+}
+
+static int i2c_demux_activate_master(struct i2c_demux_pinctrl_priv *priv, u32
new_chan)
+{
+ struct i2c_adapter *adap;
+ struct pinctrl *p;
+ int ret;
+
+ mutex_lock(&of_mutex);
+ ret = of_changeset_apply(&priv->chan[new_chan].chgset);
+ mutex_unlock(&of_mutex);
+ if (ret)
+ goto err;
+
+ adap = of_find_i2c_adapter_by_node(priv->chan[new_chan].parent_np);
+ if (!adap) {
+ ret = -ENODEV;
+ goto err;
+ }
+
+ p = devm_pinctrl_get_select(adap->dev.parent, priv->bus_name);
+ if (IS_ERR(p)) {
+ ret = PTR_ERR(p);
+ goto err_with_put;
+ }
+
+ priv->chan[new_chan].parent_adap = adap;
+ priv->cur_chan = new_chan;
+
+ /* Now fill out current adapter structure. cur_chan must be up to date
*/
+ priv->algo.master_xfer = i2c_demux_master_xfer;
+ priv->algo.functionality = i2c_demux_functionality;
+
+ snprintf(priv->cur_adap.name, sizeof(priv->cur_adap.name),
+"i2c-demux (master i2c-%d)", i2c_adapter_id(adap));
+ priv->cur_adap.owner = THIS_MODULE;
+ priv->cur_adap.algo = &priv->algo;
+ pri
[RFC v2 0/4] i2c/of: switch I2C IP cores at runtime via OF_DYNAMIC
I know this is gonna be a controversial series, but we have a usecase for this :) This series allows an I2C bus to switch between multiple masters, i.e. a n-to-1-demuxer. This is not hot-switching because connected I2C slaves will be re-instantiated. It is meant to select the best I2C core at runtime once the task is known. Example: Prefer i2c-gpio over another I2C core because of HW errata affecting your current runtime configuration. It works by using OF_DYNAMIC and en-/disabling the i2c parent as needed. See the binding docs for more details. Because this is largely using OF_DYNAMIC, I decided to post the whole series to the dt list. Changes since RFC v1: * gracefully handle if i2c adapters are not present at runtime (driver not loaded yet) * added more documentation and examples * properly put the i2c adapters after use * respect PAGE_SIZE in sysfs_show This has been tested on a Renesas Lager board switching between i2c-gpio and two different IP cores (i2c-rcar and i2c-sh_mobile). The rebinding seems to be working as expected. However, in practice, I couldn't use the HDMI i2c slaves with another controller yet, because the rcar-du driver OOPSes when unbinding. This seems unrelated to this series because it can also be triggered via sysfs-unbind. soc-camera also had problems properly cleaning up on unbind, a patch for this already has been sent. So, this series is for sure a good test for the unbind path of drivers. Let me know what you think. Thanks, Wolfram Wolfram Sang (4): of: make of_mutex public dt-bindings: i2c: mux: demux-pinctrl: add bindings i2c: mux: demux-pinctrl: add driver ARM: shmobile: r8a7790: rework dts to use i2c demuxer .../devicetree/bindings/i2c/i2c-demux-pinctrl.txt | 155 arch/arm/boot/dts/r8a7790-lager.dts| 141 +++ drivers/i2c/muxes/Kconfig | 9 + drivers/i2c/muxes/Makefile | 2 + drivers/i2c/muxes/i2c-demux-pinctrl.c | 276 + drivers/of/of_private.h| 1 - include/linux/of.h | 2 + 7 files changed, 532 insertions(+), 54 deletions(-) create mode 100644 Documentation/devicetree/bindings/i2c/i2c-demux-pinctrl.txt create mode 100644 drivers/i2c/muxes/i2c-demux-pinctrl.c -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC v2 1/4] of: make of_mutex public
From: Wolfram Sang
If we want to use OF_DYNAMIC features outside the of framework, we need
to access this lock.
Signed-off-by: Wolfram Sang
---
drivers/of/of_private.h | 1 -
include/linux/of.h | 2 ++
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index 8e882e706cd8c6..f92ec41efb5dfd 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -31,7 +31,6 @@ struct alias_prop {
char stem[0];
};
-extern struct mutex of_mutex;
extern struct list_head aliases_lookup;
extern struct kset *of_kset;
diff --git a/include/linux/of.h b/include/linux/of.h
index dd10626a615fb1..acac979063f067 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -32,6 +32,8 @@
typedef u32 phandle;
typedef u32 ihandle;
+extern struct mutex of_mutex;
+
struct property {
char*name;
int length;
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] DT: i2c: Update vendor prefix for 24c00
On Mon, Jan 04, 2016 at 08:22:33PM +0200, Andy Shevchenko wrote: > On Sat, Jan 2, 2016 at 11:21 PM, Wolfram Sang wrote: > > On Sun, Dec 27, 2015 at 04:57:48PM +0200, Andy Shevchenko wrote: > >> On Wed, Dec 23, 2015 at 9:18 PM, Akshay Bhat > >> wrote: > >> > "at" is not a valid vendor prefix, correcting the same to "atmel" > >> > > >> > >> I'm afraid you can't just do this change alone as it's used in some > >> DTS. Though you may deprecated it along with update of current users. > > > > Well, in Linux, I2C core currently strips the vendor anyhow. This will > > probably be changed somewhen (tm), but for now, the impact for Linux > > should be extremly close to 0. > > Okay, no objections to the original patch then. Heh, I just go reminded that eeproms already have a seperate binding description (Documentation/devicetree/bindings/eeprom/eeprom.txt). I will update the eeprom bindings and remove the entries from trivial devices. signature.asc Description: Digital signature
Re: [RESEND PATCH v2 0/9] eeprom: at24: at24cs series serial number read
On Mon, Jan 04, 2016 at 03:01:54PM +0100, Bartosz Golaszewski wrote: > 2016-01-02 21:50 GMT+01:00 Wolfram Sang : > > On Fri, Dec 11, 2015 at 02:55:10PM +0100, Bartosz Golaszewski wrote: > >> 2015-12-11 13:08 GMT+01:00 Wolfram Sang : > >> > On Wed, Dec 02, 2015 at 11:25:17AM +0100, Bartosz Golaszewski wrote: > >> >> Chips from the at24cs EEPROM series have an additional read-only memory > >> >> area > >> >> containing a factory pre-programmed serial number. In order to access > >> >> it, a > >> >> dummy write must be executed before reading the serial number bytes. > >> > > >> > Can't you instantiate a read-only EEPROM on this second address? Or a > >> > seperate driver attaching to this address? What is the advantage of > >> > having this in at24? > >> > > >> > >> The regular memory area and serial number read-only block share the > >> internal address pointer. We must ensure that there's no race > >> conditions between normal EEPROM reads/writes and serial number reads. > > > > I don't get it. Both, regular at24 reads and the serial read, setup the > > pointer every time by using two messages, first write to set the > > pointer, then read. The per-adapter lock makes sure those two messages > > will not get interrupted. > > If that's correct, then is there any need to have an additional mutex > for at24_data? I can't see a need, yes. > In that case would the preferred method be to access the regular > memory area like before - by allocating, for example, a 24c02 device - > while allocating a second device - in that case 24cs02 - on the > corresponding serial number address would give the user access to the > serial number via the eeprom sysfs attribute (which for the latter > would be read-only and 16 bytes in size)? Yes, a seperate driver for the second address is what I meant to suggest in the above paragraph. Only that the data should probably be exported via the NVMEM framework, not directly via sysfs. We have patches pending doing that for at24. What happens if you assign another at24 instance (read-only) to the second address? I mean, there is not only the serial number, but also a MAC address IIRC. Regards, Wolfram signature.asc Description: Digital signature
Re: [PATCH v2 0/8] i2c mux cleanup and locking update
Peter, > PS. needs a bunch of testing, I do not have access to all the involved hw First of all, thanks for diving into this topic and the huge effort you apparently have put into it. It is obviously a quite intrusive series, so it needs careful review. TBH, I can't really tell when I have the bandwidth to do that, so I hope other people will step up. And yes, it needs serious testing. To all: Although I appreciate any review support, I'd think the first thing to be done should be a very high level review - is this series worth the huge update? Is the path chosen proper? Stuff like this. I'd appreciate Acks or Revs for that. Stuff like fixing checkpatch warnings and other minor stuff should come later. Thanks, Wolfram signature.asc Description: Digital signature
Re: [PATCH] i2c: rk3x: init module as subsys call
> > Tomeu from Collabora is working on some better scheme to optimize device > > probing order but it looks like this may be a bit off still. ... > I don't just talk about touch screen driver, most i2c device driver such > as input sensor/camera/rtc/battery will suffer. So people will see their > drivers do not work or slow down on rk3368 platform :( I totally agree that the current situation is not ideal. This is why it has to be *properly fixed* and not workarounded (which caused other side effects in the past). If you care about it, then please help Tomeu with his patchset. signature.asc Description: Digital signature
Re: linux-next: build failure after merge of the i2c tree
Stephen,
> > After merging the i2c tree, today's linux-next build (powerpc
> > ppc44x_defconfig) failed like this:
Oops, yes.
> > /* Bus timings (in ns) for bit-banging */
> > -static struct i2c_timings {
> > +static struct ibm_i2c_timings {
I changed it to ibm_iic_timings for consistency reasons...
> > volatile struct iic_regs __iomem *iic = dev->vaddr;
> > - const struct i2c_timings* t = &timings[dev->fast_mode ? 1 : 0];
> > + const struct ibm_i2c_timings* t = &timings[dev->fast_mode ? 1 : 0];
... and fixed the placement of the * operator ...
> > u8 mask, v, sda;
> > int i, res;
> >
> > --
> > 2.6.2
>
> Ping?
... and applied to for-next, thanks! The patch got lost because the
i2c-list was not on CC and thus patchwork couldn't catch it.
signature.asc
Description: Digital signature
Re: [PATCH 2/2] i2c: designware: Allow build Baytrail semaphore support when IOSF_MBI=m
On Thu, Dec 10, 2015 at 03:56:27PM +0200, Jarkko Nikula wrote: > On 12/10/2015 02:59 PM, Andy Shevchenko wrote: > >On Thu, 2015-12-10 at 13:48 +0200, Jarkko Nikula wrote: > >>I believe i2c-designware-baytrail.c doesn't have strict dependency > >>that > >>Intel SoC IOSF Sideband support must be always built-in in order to > >>be > >>able to compile support for Intel Baytrail I2C bus sharing HW > >>semaphore. > >> > >>Redefine build dependencies so that CONFIG_IOSF_MBI=y is required > >>only > >>when CONFIG_I2C_DESIGNWARE_PLATFORM is built-in. > >> > >>Signed-off-by: Jarkko Nikula > >>--- > >>Hi David. Can you ack/nak this patch as I'm not fully familiar with > >>this > >>HW semaphore can there be problems when IOSF_MBI is built as a > >>module. > > > > > >>At least I'm getting similar sensible looking "punit semaphore > >>acquired/held for x ms" debug messages when I modprobe/rmmod > >>i2c_designware_platform independently is the CONFIG_IOSF_MBI=y or =m. > >>--- > >> drivers/i2c/busses/Kconfig | 4 +++- > >> 1 file changed, 3 insertions(+), 1 deletion(-) > >> > >>diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig > >>index 69c46fe13777..76f4d024def0 100644 > >>--- a/drivers/i2c/busses/Kconfig > >>+++ b/drivers/i2c/busses/Kconfig > >>@@ -490,7 +490,9 @@ config I2C_DESIGNWARE_PCI > >> > >> config I2C_DESIGNWARE_BAYTRAIL > >>bool "Intel Baytrail I2C semaphore support" > >>- depends on I2C_DESIGNWARE_PLATFORM && IOSF_MBI=y && ACPI > >>+ depends on ACPI > >>+ depends on (I2C_DESIGNWARE_PLATFORM=m && IOSF_MBI) || \ > >>+ (I2C_DESIGNWARE_PLATFORM=y && IOSF_MBI=y) > > > >Would it be more readable in the following way > > > >depends on ACPI > >depends on I2C_DESIGNWARE_PLATFORM > >depends on IOSF_MBI if I2C_DESIGNWARE_PLATFORM=m > >depends on IOSF_MBI=y if I2C_DESIGNWARE_PLATFORM=y > > > For my eyes it looks a bit more complex but I think it's matter of taste. > However, the syntax you are proposing is not supported for "depends on" like > it is for "select" statements. Any news? David? signature.asc Description: Digital signature
Re: [PATCH] i2c: designware: retry transfer on transient failure
> After that, introduce a new property 'linux,i2c-retry-count' to be used > as retries field in struct i2c_adapter. Hmm, to be honest, I always have difficulties with this "retries" parameter; to me "try x milliseconds" makes more sense than "try 5 times". It is there for ages, so we have to stick with it, but frankly, I wouldn't like to expose it too much :) I'm okay with the original patch putting some "sane" initial value. It can be modified at runtime via a i2c-dev ioctl if needed. signature.asc Description: Digital signature
Re: [PATCH v1 1/3] i2c: rk3x: add calc_divs ops for new version
> >> +static int rk3x_i2c_v0_calc_divs(unsigned long clk_rate, unsigned long > >> scl_rate, > >> +unsigned long scl_rise_ns, > >> +unsigned long scl_fall_ns, > >> +unsigned long sda_fall_ns, > > Wolfram did some sturct to assign the parameters from device properties. > It might be re-used here. Yes, I think it makes sense to convert the driver first to use the new i2c_parse_fw_timings() function and see if it fits (or if we need to extend it perhaps). David, does this make sense to you? signature.asc Description: Digital signature
Re: [Patch V1] imx: i2c: fix i2c resource leak with dma transfer
On Wed, Dec 02, 2015 at 05:52:38PM +0800, Gao Pan wrote: > In i2c_imx_dma_xfer(), when dmaengine_prep_slave_single() returns > NULL, the context goto label err_desc and then return. However, > the memory allocated by dmaengine_prep_slave_single() has not been > freed yet, which leads to resource leak. dmaengine_prep_slave_single() still owns allocated memory although it returns NULL? Isn't this a bug? > > (reported by coverity check) > > Signed-off-by: Gao Pan > Signed-off-by: Fugang Duan > --- > drivers/i2c/busses/i2c-imx.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c > index 0ab8424..6dcfff5 100644 > --- a/drivers/i2c/busses/i2c-imx.c > +++ b/drivers/i2c/busses/i2c-imx.c > @@ -397,6 +397,7 @@ static int i2c_imx_dma_xfer(struct imx_i2c_struct > *i2c_imx, > > err_submit: > err_desc: > + dmaengine_terminate_all(dma->chan_using); > dma_unmap_single(chan_dev, dma->dma_buf, > dma->dma_len, dma->dma_data_dir); > err_map: > -- > 1.9.1 > signature.asc Description: Digital signature
Re: [PATCH] i2c: designware: Add support for AMD Seattle I2C
On Tue, Dec 15, 2015 at 03:55:53PM -0600, Suravee Suthikulpanit wrote: > Add device HID AMDI0510 to match the I2C controlers on AMD Seattle platform > > Signed-off-by: Suravee Suthikulpanit Applied to for-next, thanks! signature.asc Description: Digital signature
Re: [PATCH v2] i2c: imx: Remove unneeded comments
On Mon, Jan 04, 2016 at 09:15:37AM -0200, Fabio Estevam wrote: > These multi-lines comments do not follow the standard kernel coding > style. In fact, they are not useful comments, so get rid of them. > > Signed-off-by: Fabio Estevam Applied to for-next, thanks! signature.asc Description: Digital signature
Re: [PATCH v4] i2c: designware: Do not require clock when SSCN and FFCN are provided
On Mon, Jan 04, 2016 at 09:17:35AM -0600, Suravee Suthikulpanit wrote: > The current driver uses input clock source frequency to calculate > values for [SS|FS]_[HC|LC] registers. However, when booting ACPI, we do not > currently have a good way to provide the frequency information. > Instead, we can leverage the SSCN and FFCN ACPI methods, which can be used > to directly provide these values. So, the clock information should > no longer be required during probing. > > However, since clk can be invalid, additional checks must be done where > we are making use of it. > > Signed-off-by: Mika Westerberg > Signed-off-by: Suravee Suthikulpanit > Tested-by: Loc Ho > --- > > Note: This has been tested on AMD Seattle RevB for both DT and ACPI. > > Changes from V3 (https://lkml.org/lkml/2015/12/22/596): > * Add i2c_dw_plat_prepare_clk() per Andy's suggestion > * Add tested-by Loc Ho. The changes from V3 are big enough that I'd appreciate a new Tested-by tag. signature.asc Description: Digital signature
Re: [PATCH 1/2] Document: i2c: Add a dt binding for mediatek MT2701 soc
On Mon, Jan 04, 2016 at 02:15:37PM +0800, Liguo Zhang wrote: > Add a dt binding for the MT2701 soc. > > Signed-off-by: Liguo Zhang This should probably go with the other patch via arm-soc, or? In that case: Acked-by: Wolfram Sang signature.asc Description: Digital signature
Re: [PATCH 04/10] i2c: st: use to_platform_device()
On Sun, Dec 27, 2015 at 09:15:42PM +0800, Geliang Tang wrote: > Use to_platform_device() instead of open-coding it. > > Signed-off-by: Geliang Tang Applied to for-next, thanks! signature.asc Description: Digital signature
Re: [PATCH 3/9] i2c: designware: use to_pci_dev()
On Sun, Dec 27, 2015 at 06:45:59PM +0800, Geliang Tang wrote: > Use to_pci_dev() instead of open-coding it. > > Signed-off-by: Geliang Tang Applied to for-next, thanks! signature.asc Description: Digital signature
Re: [V1, 1/2] i2c: brcmstb: Adding support for CM and DSL SoCs
On Wed, Dec 16, 2015 at 03:49:09PM -0500, Kamal Dasu wrote: > Broadcoms DSL, CM (cable modem)and STB I2C core implementation have > 8 data in/out registers that can transfer 8 bytes or 32 bytes max. > Cable and DSL "Peripheral" i2c cores use single byte per data > register and the STB can use 4 byte per data register transfer. > Adding support to take care of this difference. Accordingly added > the compatible string for SoCs using the "Peripheral" I2C block. > > Signed-off-by: Kamal Dasu Squashed both patches and applied to for-next, thanks! signature.asc Description: Digital signature
Re: [PATCH] i2c: designware: Add support for AMD Seattle I2C
On Wed, Dec 16, 2015 at 06:49:59PM -0600, Suravee Suthikulanit wrote:
> Mika,
>
> On 12/16/2015 8:54 AM, Mika Westerberg wrote:
> >On Wed, Dec 16, 2015 at 08:29:38AM -0600, Suravee Suthikulpanit wrote:
> >>>
> >>>
> >>>On 12/16/2015 03:16 AM, Mika Westerberg wrote:
> >On Tue, Dec 15, 2015 at 08:14:34PM -0600, Suravee Suthikulpanit wrote:
> > >>Hi Mika,
> > >>
> > >>On 12/15/15 15:55, Suravee Suthikulpanit wrote:
> >> >>>Add device HID AMDI0510 to match the I2C controlers on AMD Seattle
> >> >>>platform
> >> >>>
> >> >>>Signed-off-by: Suravee Suthikulpanit
> >> >>>---
> >> >>> drivers/i2c/busses/i2c-designware-platdrv.c | 1 +
> >> >>> 1 file changed, 1 insertion(+)
> >> >>>
> >> >>>diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c
> >> >>>b/drivers/i2c/busses/i2c-designware-platdrv.c
> >> >>>index 57f623b..a027154 100644
> >> >>>--- a/drivers/i2c/busses/i2c-designware-platdrv.c
> >> >>>+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> >> >>>@@ -117,6 +117,7 @@ static const struct acpi_device_id
> >> >>>dw_i2c_acpi_match[] = {
> >> >>>{ "80860F41", 0 },
> >> >>>{ "808622C1", 0 },
> >> >>>{ "AMD0010", 0 },
> >> >>>+ { "AMDI0510", 0 },
> >> >>>{ }
> > >>
> > >>Since this driver seems to be used by several SOCs, and we have been
> > >>adding
> > >>the HID from various SOC vendors. Do you think it would be better to
> > >>assign
> > >>a CID so that each SOC vendor can specify in their ACPI DSDT and we
> > >>can
> > >>match them here?
> >
> >Sure _CID would work here.
> >>>
> >>>Do you know if Synopsys has already provided a CID that we can use for
> >>>this?
> >No.
> >
> >>>If not, who do you think should provide this?
> >Why can't you make _CID for AMD part only? For Intel we are going to get
> >new IDs for every major SoC release no matter what.
> >
> Actually, after discussed with the team. We have decided to go with the
> AMDI0510 at this point, and we will reuse this as CID in future SOC if it
> contains compatible I2C controller.
So, can I take the patch as is?
signature.asc
Description: Digital signature
Re: [PATCH v3] i2c: mediatek: fix i2c multi transfer issue in high speed mode
On Tue, Dec 15, 2015 at 03:22:26PM +0800, Liguo Zhang wrote: > For mt8173 platform with auto restart support, when doing i2c multi > transfer in high speed, we should ignore the first restart irq after > the master code, otherwise the first transfer will be discarded. > > Signed-off-by: Liguo Zhang > Reviewed-by: Eddie Huang Applied to for-next, thanks! signature.asc Description: Digital signature
Re: [PATCH v2] i2c: imx: make bus recovery through pinctrl optional
On Mon, Nov 30, 2015 at 01:55:19PM +0100, Linus Walleij wrote:
> On Fri, Nov 20, 2015 at 10:45 PM, Li Yang wrote:
>
> > - if (IS_ERR(i2c_imx->pinctrl)) {
> > + /* bailout on -ENOMEM or -EPROBE_DEFER, continue for other errors */
> > + if (PTR_ERR(i2c_imx->pinctrl) == -ENOMEM ||
> > + PTR_ERR(i2c_imx->pinctrl) == -EPROBE_DEFER) {
>
> This is one of the evils of deferred probe: you never know if
> something will eventually turn up. It feels wrong to bail out
> on deferred probe...
>
> I have no better idea though.
> Acked-by
Linus, proper ack next time please, so patchwork will pick it up
automatically.
Li Yang: There have been some other changes to the imx driver. Could you
rebase against my for-next or for-4.5 and add Linus' ack? Thanks!
signature.asc
Description: Digital signature
Re: [PATCH 2/2] i2c: imx: Remove unneeded comments
On Sun, Nov 01, 2015 at 02:22:52PM -0200, Fabio Estevam wrote: > From: Fabio Estevam > > These multi-lines comments do not follow the standard kernel coding > style. In fact, they are not useful comments, so get rid of them. > > Signed-off-by: Fabio Estevam There have been some other changes to the imx driver. Could you rebase against my for-next or for-4.5? Thanks! signature.asc Description: Digital signature
Re: [PATCH 1/2] i2c: imx: Improve message log when DMA is not used
On Sun, Nov 01, 2015 at 02:22:51PM -0200, Fabio Estevam wrote: > From: Fabio Estevam > > When DMA cannot be used, it is better to state that the I2C controller > will operate in PIO mode. > > Signed-off-by: Fabio Estevam Applied to for-next, thanks! signature.asc Description: Digital signature
Re: [Patch V1] i2c: imx: improve code readability
On Mon, Nov 02, 2015 at 05:05:30PM +0800, Gao Pan wrote: > Replace of_get_named_gpio_flags with of_get_named_gpio because > the latter has less parameters, which improves code readability. > > Signed-off-by: Fugang Duan > Signed-off-by: Gao Pan Applied to for-next, thanks! signature.asc Description: Digital signature
Re: [Patch V10] i2c: imx: add runtime pm support to improve the performance
On Fri, Dec 11, 2015 at 10:24:09AM +0800, Gao Pan wrote: > In our former i2c driver, i2c clk is enabled and disabled in > xfer function, which contributes to power saving. However, > the clk enable process brings a busy wait delay until the core > is stable. As a result, the performance is sacrificed. > > To weigh the power consumption and i2c bus performance, runtime > pm is the good solution for it. The clk is enabled when a i2c > transfer starts, and disabled after a specifically defined delay. > > If CONFIG_PM is disabled the net result of this patch is that the > clock is never disabled. > > Without the patch the test case (many eeprom reads) executes with approx: > real 1m7.735s > user 0m0.488s > sys 0m20.040s > > With the patch the same test case (many eeprom reads) executes with approx: > real 0m54.241s > user 0m0.440s > sys 0m5.920s > > Signed-off-by: Fugang Duan > Signed-off-by: Gao Pan > Acked-by: Uwe Kleine-König Applied to for-next, thanks! Also much thanks to Uwe and Heiner for the reviews of all these versions! signature.asc Description: Digital signature
Re: [PATCH] DT: i2c: Update vendor prefix for 24c00
On Sun, Dec 27, 2015 at 04:57:48PM +0200, Andy Shevchenko wrote: > On Wed, Dec 23, 2015 at 9:18 PM, Akshay Bhat wrote: > > "at" is not a valid vendor prefix, correcting the same to "atmel" > > > > I'm afraid you can't just do this change alone as it's used in some > DTS. Though you may deprecated it along with update of current users. Well, in Linux, I2C core currently strips the vendor anyhow. This will probably be changed somewhen (tm), but for now, the impact for Linux should be extremly close to 0. > > > Signed-off-by: Akshay Bhat > > --- > > Documentation/devicetree/bindings/i2c/trivial-devices.txt | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt > > b/Documentation/devicetree/bindings/i2c/trivial-devices.txt > > index c50cf13..c4a01c0 100644 > > --- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt > > +++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt > > @@ -20,11 +20,11 @@ adi,adt7476 +/-1C TDM Extended Temp Range I.C > > adi,adt7490+/-1C TDM Extended Temp Range I.C > > adi,adxl345Three-Axis Digital Accelerometer > > adi,adxl346Three-Axis Digital Accelerometer > > (backward-compatibility value "adi,adxl345" must be listed too) > > -at,24c08 i2c serial eeprom (24cxx) > > atmel,24c00i2c serial eeprom (24cxx) > > atmel,24c01i2c serial eeprom (24cxx) > > atmel,24c02i2c serial eeprom (24cxx) > > atmel,24c04i2c serial eeprom (24cxx) > > +atmel,24c08i2c serial eeprom (24cxx) > > atmel,24c16i2c serial eeprom (24cxx) > > atmel,24c32i2c serial eeprom (24cxx) > > atmel,24c64i2c serial eeprom (24cxx) > > -- > > 2.6.3 > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > > the body of a message to majord...@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > Please read the FAQ at http://www.tux.org/lkml/ > > > > -- > With Best Regards, > Andy Shevchenko signature.asc Description: Digital signature
Re: [PATCH] DT: i2c: Add Epson RX8010 to list of trivial devices
On Sun, Dec 27, 2015 at 05:02:48PM +0200, Andy Shevchenko wrote: > On Wed, Dec 23, 2015 at 8:38 PM, Akshay Bhat wrote: > > This adds devicetree documentation for the bindings of rtc-rx8010 > > driver. > > > > Signed-off-by: Akshay Bhat > > --- > > Documentation/devicetree/bindings/i2c/trivial-devices.txt | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt > > b/Documentation/devicetree/bindings/i2c/trivial-devices.txt > > index c50cf13..0f9c1de 100644 > > --- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt > > +++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt > > @@ -49,6 +49,7 @@ dallas,ds4510 CPU Supervisor with Nonvolatile > > Memory and Programmable I/O > > dallas,ds75Digital Thermometer and Thermostat > > 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 > > Is it indeed required to have all those capital letters together? I agree. Fixing that in all Epson RTC entries would be nice. signature.asc Description: Digital signature
Re: [PATCH 0/3] i2c: rcar: adapt PM usage to multi master case
On Wed, Dec 23, 2015 at 05:56:31PM +0100, Wolfram Sang wrote: > If we are in a multi-master scenario, we need to block runtime PM so the > arbitration circuit stays awake. > > So, we define a new binding and adapt the i2c-rcar driver to have an example > implementation. Applied to for-next, thanks! signature.asc Description: Digital signature
Re: [PATCH v3 0/5] i2c: img-scb: enchancements to support i2c on pistachio
On Thu, Nov 19, 2015 at 09:35:12AM +, Sifan Naeem wrote: > The following patches are required to enchance the existing driver to > support i2c on pistachio. > > This patch series depends on the series of fixes posted earlier[1]. > The features added in this series were tested with the earlier fixes > in place. > > Tested on Pistachio bub and on tz1090 using an Adafruit I2C > Non-Volatile FRAM Breakout (256Kbit / 32KByte) eeprom. > > Used i2c buildroot tools to test the eeprom and the other i2c blocks. > Also used dd commands to copy data to and then to dump data from the > eeprom. i2ctransfer was used to test repeated starts and verified > using a scope. Applied to for-next, thanks! And thanks to James and James for the reviews! signature.asc Description: Digital signature
Re: [RESEND PATCH v2 0/9] eeprom: at24: at24cs series serial number read
On Fri, Dec 11, 2015 at 02:55:10PM +0100, Bartosz Golaszewski wrote: > 2015-12-11 13:08 GMT+01:00 Wolfram Sang : > > On Wed, Dec 02, 2015 at 11:25:17AM +0100, Bartosz Golaszewski wrote: > >> Chips from the at24cs EEPROM series have an additional read-only memory > >> area > >> containing a factory pre-programmed serial number. In order to access it, a > >> dummy write must be executed before reading the serial number bytes. > > > > Can't you instantiate a read-only EEPROM on this second address? Or a > > seperate driver attaching to this address? What is the advantage of > > having this in at24? > > > > The regular memory area and serial number read-only block share the > internal address pointer. We must ensure that there's no race > conditions between normal EEPROM reads/writes and serial number reads. I don't get it. Both, regular at24 reads and the serial read, setup the pointer every time by using two messages, first write to set the pointer, then read. The per-adapter lock makes sure those two messages will not get interrupted. So, it looks to me that it would be OK if a serial read access gets inbetween a eeprom read access. Am I wrong? signature.asc Description: Digital signature
Re: [PATCH] DT: i2c: Add Freescale MPL3115 to trivial devices list
On Tue, Dec 29, 2015 at 12:35:16PM -0600, Rob Herring wrote: > On Wed, Dec 23, 2015 at 01:59:07PM -0500, Akshay Bhat wrote: > > This adds devicetree documentation for the bindings of mpl3115 driver. > > > > Signed-off-by: Akshay Bhat > > --- > > Documentation/devicetree/bindings/i2c/trivial-devices.txt | 1 + > > 1 file changed, 1 insertion(+) > > Acked-by: Rob Herring > > Who did you want to apply this? Send patches TO that person which I > would expect to be Wolfram or me. I usually take these. Will do again :) signature.asc Description: PGP signature
Re: i2c-core: One function call less in acpi_i2c_space_handler() after error detection
On Sat, Dec 26, 2015 at 09:52:11AM +0100, SF Markus Elfring wrote: > >> The kfree() function was called in one case by the > >> acpi_i2c_space_handler() function during error handling > >> even if the passed variable "client" contained a null pointer. > > > > This is OK. kfree() is known to be NULL-tolerant and we rely on it in > > various places to keep the code simpler. > > I would appreciate if an unnecessary function call can be avoided here > so that the affected exception handling can become also a bit more efficient. Simpler code is easier to maintain. See your patch, you didn't get it correctly at your first try. Also, this is not a hot path, so I see it as a micro-optimization also adding complexity. I don't favor that. signature.asc Description: PGP signature
Re: [PATCH v2] i2c-core: One function call less in acpi_i2c_space_handler() after error detection
> The kfree() function was called in one case by the > acpi_i2c_space_handler() function during error handling > even if the passed variable "client" contained a null pointer. This is OK. kfree() is known to be NULL-tolerant and we rely on it in various places to keep the code simpler. signature.asc Description: PGP signature
Re: [PATCH 0/2] i2c: proper RuntimePM for the adapter device
> I think the below drivers > > drivers/i2c/busses/i2c-at91.c > drivers/i2c/busses/i2c-designware-core.c > drivers/i2c/busses/i2c-designware-platdrv.c > drivers/i2c/busses/i2c-rcar.c > drivers/i2c/busses/i2c-sh_mobile.c > drivers/i2c/busses/i2c-hix5hd2.c > drivers/i2c/busses/i2c-nomadik.c > drivers/i2c/busses/i2c-omap.c > drivers/i2c/busses/i2c-qup.c > drivers/i2c/busses/i2c-s3c2410.c They use RuntimePM on their platform device, not on the adapter device AFAICS. signature.asc Description: PGP signature
Re: [PATCH 0/2] i2c: proper RuntimePM for the adapter device
> > RuntimePM for the logical adapter device should be handled in a central > > place > > by the core, and not by drivers. This series does exactly that. > > This is good idea. Thanks! > Also maybe we can add a flag that the driver enables. > that way for a short period till the driver removes the call > the double call is not there. The only driver I found was the s3c2410 and I fixed it. Is there another one? Then, I'd update my series because I want a consistent state in one go. signature.asc Description: Digital signature
[PATCH 0/2] i2c: proper RuntimePM for the adapter device
RuntimePM for the logical adapter device should be handled in a central place by the core, and not by drivers. This series does exactly that. This is the first step of harmonizing RuntimePM handling in I2C. Wolfram Sang (2): i2c: always enable RuntimePM for the adapter device i2c: s3c2410: remove superfluous runtime PM calls drivers/i2c/busses/i2c-s3c2410.c | 6 -- drivers/i2c/i2c-core.c | 3 +++ 2 files changed, 3 insertions(+), 6 deletions(-) -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] i2c: s3c2410: remove superfluous runtime PM calls
From: Wolfram Sang
RuntimePM of the adapter device is now taken care of by the core. So, we
can remove these calls.
Signed-off-by: Wolfram Sang
---
drivers/i2c/busses/i2c-s3c2410.c | 6 --
1 file changed, 6 deletions(-)
diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
index 5df819610d5280..362a6de548336b 100644
--- a/drivers/i2c/busses/i2c-s3c2410.c
+++ b/drivers/i2c/busses/i2c-s3c2410.c
@@ -784,7 +784,6 @@ static int s3c24xx_i2c_xfer(struct i2c_adapter *adap,
int retry;
int ret;
- pm_runtime_get_sync(&adap->dev);
ret = clk_enable(i2c->clk);
if (ret)
return ret;
@@ -795,7 +794,6 @@ static int s3c24xx_i2c_xfer(struct i2c_adapter *adap,
if (ret != -EAGAIN) {
clk_disable(i2c->clk);
- pm_runtime_put(&adap->dev);
return ret;
}
@@ -805,7 +803,6 @@ static int s3c24xx_i2c_xfer(struct i2c_adapter *adap,
}
clk_disable(i2c->clk);
- pm_runtime_put(&adap->dev);
return -EREMOTEIO;
}
@@ -1256,8 +1253,6 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev)
return ret;
}
- pm_runtime_enable(&i2c->adap.dev);
-
dev_info(&pdev->dev, "%s: S3C I2C adapter\n", dev_name(&i2c->adap.dev));
return 0;
}
@@ -1273,7 +1268,6 @@ static int s3c24xx_i2c_remove(struct platform_device
*pdev)
clk_unprepare(i2c->clk);
- pm_runtime_disable(&i2c->adap.dev);
pm_runtime_disable(&pdev->dev);
s3c24xx_i2c_deregister_cpufreq(i2c);
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] i2c: always enable RuntimePM for the adapter device
From: Wolfram Sang The adapter device is a logical device. Because of that, it already uses pm_runtime_no_callbacks() in the core. To ensure proper propagation from the children (i2c devices) to the parent of the adapter (the HW device), make sure RuntimePM is enabled in any case. Signed-off-by: Wolfram Sang --- drivers/i2c/i2c-core.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c index e94d2ca2aab4aa..55159e8e53283a 100644 --- a/drivers/i2c/i2c-core.c +++ b/drivers/i2c/i2c-core.c @@ -1610,6 +1610,7 @@ static int i2c_register_adapter(struct i2c_adapter *adap) dev_dbg(&adap->dev, "adapter [%s] registered\n", adap->name); pm_runtime_no_callbacks(&adap->dev); + pm_runtime_enable(&adap->dev); #ifdef CONFIG_I2C_COMPAT res = class_compat_create_link(i2c_adapter_compat_class, &adap->dev, @@ -1864,6 +1865,8 @@ void i2c_del_adapter(struct i2c_adapter *adap) /* device name is gone after device_unregister */ dev_dbg(&adap->dev, "adapter [%s] unregistered\n", adap->name); + pm_runtime_disable(&adap->dev); + /* wait until all references to the device are gone * * FIXME: This is old code and should ideally be replaced by an -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] i2c: document binding for multi-master case
From: Wolfram Sang We need this binding because some I2C master drivers will need to adapt their PM settings for the arbitration circuitry. Acked-by: Geert Uytterhoeven Acked-by: Rob Herring Signed-off-by: Wolfram Sang --- Documentation/devicetree/bindings/i2c/i2c.txt | 5 + 1 file changed, 5 insertions(+) diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt index a00219f5ee0733..c8d977ed847f3c 100644 --- a/Documentation/devicetree/bindings/i2c/i2c.txt +++ b/Documentation/devicetree/bindings/i2c/i2c.txt @@ -54,6 +54,11 @@ wants to support one of the below features, it should adapt the bindings below. "irq" and "wakeup" names are recognized by I2C core, other names are left to individual drivers. +- multi-master + states that there is another master active on this bus. The OS can use + this information to adapt power management to keep the arbitration awake + all the time, for example. + - wakeup-source device can be used as a wakeup source. -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] i2c: rcar: disable PM in multi-master mode
From: Wolfram Sang
In multi master mode, the IP core needs to be always active for
arbitration reasons. Get the config from DT and set up PM depending on
the config.
Acked-by: Geert Uytterhoeven
Signed-off-by: Wolfram Sang
---
drivers/i2c/busses/i2c-rcar.c | 17 ++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index fab841899e65f2..1abeadc8ab7959 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -96,6 +96,9 @@
#define ID_DONE(1 << 2)
#define ID_ARBLOST (1 << 3)
#define ID_NACK(1 << 4)
+/* persistent flags */
+#define ID_P_PM_BLOCKED(1 << 31)
+#define ID_P_MASK ID_P_PM_BLOCKED
enum rcar_i2c_type {
I2C_RCAR_GEN1,
@@ -277,7 +280,7 @@ static void rcar_i2c_next_msg(struct rcar_i2c_priv *priv)
{
priv->msg++;
priv->msgs_left--;
- priv->flags = 0;
+ priv->flags &= ID_P_MASK;
rcar_i2c_prepare_msg(priv);
}
@@ -495,7 +498,7 @@ static int rcar_i2c_master_xfer(struct i2c_adapter *adap,
/* init first message */
priv->msg = msgs;
priv->msgs_left = num;
- priv->flags = ID_FIRST_MSG;
+ priv->flags = (priv->flags & ID_P_MASK) | ID_FIRST_MSG;
rcar_i2c_prepare_msg(priv);
time_left = wait_event_timeout(priv->wait, priv->flags & ID_DONE,
@@ -630,7 +633,13 @@ static int rcar_i2c_probe(struct platform_device *pdev)
goto out_pm_put;
rcar_i2c_init(priv);
- pm_runtime_put(dev);
+
+ /* Don't suspend when multi-master to keep arbitration working */
+ if (of_property_read_bool(dev->of_node, "multi-master"))
+ priv->flags |= ID_P_PM_BLOCKED;
+ else
+ pm_runtime_put(dev);
+
irq = platform_get_irq(pdev, 0);
ret = devm_request_irq(dev, irq, rcar_i2c_irq, 0, dev_name(dev), priv);
@@ -664,6 +673,8 @@ static int rcar_i2c_remove(struct platform_device *pdev)
struct device *dev = &pdev->dev;
i2c_del_adapter(&priv->adap);
+ if (priv->flags & ID_P_PM_BLOCKED)
+ pm_runtime_put(dev);
pm_runtime_disable(dev);
return 0;
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/3] i2c: rcar: adapt PM usage to multi master case
If we are in a multi-master scenario, we need to block runtime PM so the arbitration circuit stays awake. So, we define a new binding and adapt the i2c-rcar driver to have an example implementation. Changes since RFC: * added acks from Rob and Geert (thanks!) * use of_property_read_bool() now instead of of_get_property (thanks Sergei) * way more testing Wolfram Sang (3): i2c: document binding for multi-master case i2c: rcar: remove macros dealing with flags i2c: rcar: disable PM in multi-master mode Documentation/devicetree/bindings/i2c/i2c.txt | 5 drivers/i2c/busses/i2c-rcar.c | 39 --- 2 files changed, 28 insertions(+), 16 deletions(-) -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] i2c: rcar: remove macros dealing with flags
From: Wolfram Sang
These macros don't really hide complexity, but C idioms. Removing them
makes the code easier to read IMO and make a planned extension easier.
Acked-by: Geert Uytterhoeven
Signed-off-by: Wolfram Sang
---
drivers/i2c/busses/i2c-rcar.c | 22 +-
1 file changed, 9 insertions(+), 13 deletions(-)
diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index 74077e9ab8cad6..fab841899e65f2 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -122,9 +122,6 @@ struct rcar_i2c_priv {
#define rcar_i2c_priv_to_dev(p)((p)->adap.dev.parent)
#define rcar_i2c_is_recv(p)((p)->msg->flags & I2C_M_RD)
-#define rcar_i2c_flags_set(p, f) ((p)->flags |= (f))
-#define rcar_i2c_flags_has(p, f) ((p)->flags & (f))
-
#define LOOP_TIMEOUT 1024
@@ -258,7 +255,7 @@ static void rcar_i2c_prepare_msg(struct rcar_i2c_priv *priv)
priv->pos = 0;
if (priv->msgs_left == 1)
- rcar_i2c_flags_set(priv, ID_LAST_MSG);
+ priv->flags |= ID_LAST_MSG;
rcar_i2c_write(priv, ICMAR, (priv->msg->addr << 1) | read);
/*
@@ -266,7 +263,7 @@ static void rcar_i2c_prepare_msg(struct rcar_i2c_priv *priv)
* of ICMSR and ICMCR depends on whether we issue START or REP_START.
Since
* it didn't cause a drawback for me, let's rather be safe than sorry.
*/
- if (rcar_i2c_flags_has(priv, ID_FIRST_MSG)) {
+ if (priv->flags & ID_FIRST_MSG) {
rcar_i2c_write(priv, ICMSR, 0);
rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_START);
} else {
@@ -438,7 +435,7 @@ static irqreturn_t rcar_i2c_irq(int irq, void *ptr)
/* Arbitration lost */
if (msr & MAL) {
- rcar_i2c_flags_set(priv, (ID_DONE | ID_ARBLOST));
+ priv->flags |= ID_DONE | ID_ARBLOST;
goto out;
}
@@ -446,14 +443,14 @@ static irqreturn_t rcar_i2c_irq(int irq, void *ptr)
if (msr & MNR) {
/* HW automatically sends STOP after received NACK */
rcar_i2c_write(priv, ICMIER, RCAR_IRQ_STOP);
- rcar_i2c_flags_set(priv, ID_NACK);
+ priv->flags |= ID_NACK;
goto out;
}
/* Stop */
if (msr & MST) {
priv->msgs_left--; /* The last message also made it */
- rcar_i2c_flags_set(priv, ID_DONE);
+ priv->flags |= ID_DONE;
goto out;
}
@@ -463,7 +460,7 @@ static irqreturn_t rcar_i2c_irq(int irq, void *ptr)
rcar_i2c_irq_send(priv, msr);
out:
- if (rcar_i2c_flags_has(priv, ID_DONE)) {
+ if (priv->flags & ID_DONE) {
rcar_i2c_write(priv, ICMIER, 0);
rcar_i2c_write(priv, ICMSR, 0);
wake_up(&priv->wait);
@@ -501,15 +498,14 @@ static int rcar_i2c_master_xfer(struct i2c_adapter *adap,
priv->flags = ID_FIRST_MSG;
rcar_i2c_prepare_msg(priv);
- time_left = wait_event_timeout(priv->wait,
-rcar_i2c_flags_has(priv, ID_DONE),
+ time_left = wait_event_timeout(priv->wait, priv->flags & ID_DONE,
num * adap->timeout);
if (!time_left) {
rcar_i2c_init(priv);
ret = -ETIMEDOUT;
- } else if (rcar_i2c_flags_has(priv, ID_NACK)) {
+ } else if (priv->flags & ID_NACK) {
ret = -ENXIO;
- } else if (rcar_i2c_flags_has(priv, ID_ARBLOST)) {
+ } else if (priv->flags & ID_ARBLOST) {
ret = -EAGAIN;
} else {
ret = num - priv->msgs_left; /* The number of transfer */
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c: s3c2410: remove superfluous runtime PM calls
> > Asking linux-pm for help here: If we want to support RuntimePM for I2C > > clients, do we need to enable RuntimePM on the logical I2C adapter > > device (the bus master) which is already marked using > > pm_runtime_no_callbacks? > > In theory you don't need to. But there are some advantages if you do: > You get automatic runtime PM time accounting for the adapter device > (how much time active and how much suspended), and suspend events will > propagate from the I2C clients all the way up to the adapter's parent. That's exactly what I want. Thank you very much! signature.asc Description: PGP signature
[PULL REQUEST] i2c for 4.4
Linus, here is a set of "usual" driver bugfixes for the I2C subsystem. Please pull. Thanks, Wolfram The following changes since commit 1ec218373b8ebda821aec00bb156a9c94fad9cd4: Linux 4.4-rc2 (2015-11-22 16:45:59 -0800) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-current for you to fetch changes up to b4cd08aa1f53c831e67dc5c6bc9f9acff27abcba: i2c: rcar: disable runtime PM correctly in slave mode (2015-12-19 12:00:37 +0100) Alexander Sverdlin (1): i2c: davinci: Increase module clock frequency Dmitry V. Krivenok (1): i2c: do not use 0x in front of %pa Gao Pan (1): i2c: imx: init bus recovery info before adding i2c adapter Hans de Goede (1): i2c: mv64xxx: The n clockdiv factor is 0 based on sunxi SoCs Jarkko Nikula (1): i2c: designware: Keep pm_runtime_enable/_disable calls in sync Wolfram Sang (2): i2c: rk3x: populate correct variable for sda_falling_time i2c: rcar: disable runtime PM correctly in slave mode Xiangliang Yu (1): i2c: designware: fix IO timeout issue for AMD controller drivers/i2c/busses/i2c-davinci.c| 11 +-- drivers/i2c/busses/i2c-designware-core.c| 6 ++ drivers/i2c/busses/i2c-designware-core.h| 1 + drivers/i2c/busses/i2c-designware-platdrv.c | 16 ++-- drivers/i2c/busses/i2c-imx.c| 4 ++-- drivers/i2c/busses/i2c-mv64xxx.c| 27 ++- drivers/i2c/busses/i2c-rcar.c | 4 ++-- drivers/i2c/busses/i2c-rk3x.c | 2 +- drivers/i2c/busses/i2c-st.c | 2 +- 9 files changed, 50 insertions(+), 23 deletions(-) signature.asc Description: Digital signature
Re: [PATCH] i2c: s3c2410: remove superfluous runtime PM calls
On Thu, Dec 17, 2015 at 09:46:55AM +, Charles Keepax wrote:
> On Wed, Dec 16, 2015 at 02:53:07PM +0100, Sylwester Nawrocki wrote:
> > On 15/12/15 19:14, Wolfram Sang wrote:
> > > Since commit 6ada5c1e1b077a ("i2c: Mark adapter devices with
> > > pm_runtime_no_callbacks"), runtime PM on adapters turned into a no-op.
> > > So, we can remove these calls.
> >
> > Won't this break i2c client devices that use runtime PM? Not sure
> > if any cases of such client exist now, I'll try to find some time
> > to test this change.
> >
>
> Our CODECs can be controlled over I2C and use runtime PM.
> However, this change doesn't seem to have any adverse effect on
> them that I can find.
>
> Tested-by: Charles Keepax
Thanks for testing!
signature.asc
Description: Digital signature
Re: [PATCH] i2c: s3c2410: remove superfluous runtime PM calls
Hi,
> > Since commit 6ada5c1e1b077a ("i2c: Mark adapter devices with
> > pm_runtime_no_callbacks"), runtime PM on adapters turned into a no-op.
> > So, we can remove these calls.
>
> Won't this break i2c client devices that use runtime PM? Not sure
> if any cases of such client exist now, I'll try to find some time
> to test this change.
Thanks, much appreciated.
> IIRC client's pm_runtime* calls return error if their parent device's
> runtime PM is not enabled. Also enabling runtime PM on the i2c adapter
> allows propagating runtime PM calls up to its parent - the i2c
> controller platform device.
I was thinking the PM core would take care of that. Grepping for
other users of pm_runtime_no_callbacks(), I see that most but not all
use pm_runtime_enable() on the device, too.
Asking linux-pm for help here: If we want to support RuntimePM for I2C
clients, do we need to enable RuntimePM on the logical I2C adapter
device (the bus master) which is already marked using
pm_runtime_no_callbacks?
> Perhaps we could just enable/disable adapter's runtime PM in i2c core.
That would be the proper place to do it, as I see it.
Thanks,
Wolfram
signature.asc
Description: Digital signature
Re: [PATCH] i2c: allow building emev2 without slave mode again
> > My conclusion for now is: > > > > There needs something to be done surely, but currently I don't have the > > bandwidth to do it or even play around with it. I am not fully happy > > with your patches as well because __maybe_unused has some kind of "last > > resort" feeling to me. > > I generally like __maybe_unused, but it's a matter of personal preference. > We could avoid the __maybe_unused if the reg_slave/unreg_slave callback > pointers are always available in struct i2c_algorithm. Yes, I was thinking in this direction, looking at how PM does it. Needs some playing around, though. > Just for reference, see below for my combined patch, in case you decide > to use that at a later point. Thanks a lot! signature.asc Description: Digital signature
Re: [PATCH v2] i2c: rcar: disable runtime PM correctly in slave mode
On Wed, Dec 16, 2015 at 08:05:18PM +0100, Wolfram Sang wrote: > From: Wolfram Sang > > When we also are I2C slave, we need to disable runtime PM because the > address detection mechanism needs to be active all the time. However, we > can reenable runtime PM once the slave instance was unregistered. So, > use pm_runtime_get_sync/put to achieve this, since it has proper > refcounting. pm_runtime_allow/forbid is like a global knob controllable > from userspace which is unsuitable here. > > Signed-off-by: Wolfram Sang Applied to for-current, thanks! signature.asc Description: Digital signature
Re: [PATCH] i2c: emev: select I2C slave support
On Thu, Dec 17, 2015 at 01:09:32PM +0100, Wolfram Sang wrote: > From: Wolfram Sang > > Until we have proper support to make I2C slave support fully optional, > select it to prevent build errors on randconfigs. > > Signed-off-by: Wolfram Sang Applied to for-next, thanks! signature.asc Description: Digital signature
Re: [PATCH] i2c: make i2c_parse_fw_timings() always visible
On Thu, Dec 17, 2015 at 01:32:36PM +0100, Wolfram Sang wrote: > From: Wolfram Sang > > This function used to be DT only, so it lived inside a CONFIG_OF block. > Now it uses device attributes and must be moved outside of it. No > further code changes. > > Reported-by: Jim Davis > Signed-off-by: Wolfram Sang Applied to for-next, thanks! signature.asc Description: Digital signature
Re: randconfig build error with next-20151216, in drivers/i2c/busses
On Wed, Dec 16, 2015 at 09:44:00AM -0700, Jim Davis wrote: > Building with the attached random configuration file, > > ERROR: "i2c_parse_fw_timings" [drivers/i2c/busses/i2c-rcar.ko] undefined! Thanks! I just sent a patch to fix it. signature.asc Description: Digital signature
[PATCH] i2c: make i2c_parse_fw_timings() always visible
From: Wolfram Sang
This function used to be DT only, so it lived inside a CONFIG_OF block.
Now it uses device attributes and must be moved outside of it. No
further code changes.
Reported-by: Jim Davis
Signed-off-by: Wolfram Sang
---
drivers/i2c/i2c-core.c | 104 -
1 file changed, 52 insertions(+), 52 deletions(-)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index b34c412bd2c240..90fe315b6360e2 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -1439,58 +1439,6 @@ static void of_i2c_register_devices(struct i2c_adapter
*adap)
}
}
-/**
- * i2c_parse_fw_timings - get I2C related timing parameters from firmware
- * @dev: The device to scan for I2C timing properties
- * @t: the i2c_timings struct to be filled with values
- * @use_defaults: bool to use sane defaults derived from the I2C specification
- * when properties are not found, otherwise use 0
- *
- * Scan the device for the generic I2C properties describing timing parameters
- * for the signal and fill the given struct with the results. If a property was
- * not found and use_defaults was true, then maximum timings are assumed which
- * are derived from the I2C specification. If use_defaults is not used, the
- * results will be 0, so drivers can apply their own defaults later. The latter
- * is mainly intended for avoiding regressions of existing drivers which want
- * to switch to this function. New drivers almost always should use the
defaults.
- */
-
-void i2c_parse_fw_timings(struct device *dev, struct i2c_timings *t, bool
use_defaults)
-{
- int ret;
-
- memset(t, 0, sizeof(*t));
-
- ret = device_property_read_u32(dev, "clock-frequency", &t->bus_freq_hz);
- if (ret && use_defaults)
- t->bus_freq_hz = 10;
-
- ret = device_property_read_u32(dev, "i2c-scl-rising-time-ns",
&t->scl_rise_ns);
- if (ret && use_defaults) {
- if (t->bus_freq_hz <= 10)
- t->scl_rise_ns = 1000;
- else if (t->bus_freq_hz <= 40)
- t->scl_rise_ns = 300;
- else
- t->scl_rise_ns = 120;
- }
-
- ret = device_property_read_u32(dev, "i2c-scl-falling-time-ns",
&t->scl_fall_ns);
- if (ret && use_defaults) {
- if (t->bus_freq_hz <= 40)
- t->scl_fall_ns = 300;
- else
- t->scl_fall_ns = 120;
- }
-
- device_property_read_u32(dev, "i2c-scl-internal-delay-ns",
&t->scl_int_delay_ns);
-
- ret = device_property_read_u32(dev, "i2c-sda-falling-time-ns",
&t->sda_fall_ns);
- if (ret && use_defaults)
- t->sda_fall_ns = t->scl_fall_ns;
-}
-EXPORT_SYMBOL_GPL(i2c_parse_fw_timings);
-
static int of_dev_node_match(struct device *dev, void *data)
{
return dev->of_node == data;
@@ -1892,6 +1840,58 @@ void i2c_del_adapter(struct i2c_adapter *adap)
}
EXPORT_SYMBOL(i2c_del_adapter);
+/**
+ * i2c_parse_fw_timings - get I2C related timing parameters from firmware
+ * @dev: The device to scan for I2C timing properties
+ * @t: the i2c_timings struct to be filled with values
+ * @use_defaults: bool to use sane defaults derived from the I2C specification
+ * when properties are not found, otherwise use 0
+ *
+ * Scan the device for the generic I2C properties describing timing parameters
+ * for the signal and fill the given struct with the results. If a property was
+ * not found and use_defaults was true, then maximum timings are assumed which
+ * are derived from the I2C specification. If use_defaults is not used, the
+ * results will be 0, so drivers can apply their own defaults later. The latter
+ * is mainly intended for avoiding regressions of existing drivers which want
+ * to switch to this function. New drivers almost always should use the
defaults.
+ */
+
+void i2c_parse_fw_timings(struct device *dev, struct i2c_timings *t, bool
use_defaults)
+{
+ int ret;
+
+ memset(t, 0, sizeof(*t));
+
+ ret = device_property_read_u32(dev, "clock-frequency", &t->bus_freq_hz);
+ if (ret && use_defaults)
+ t->bus_freq_hz = 10;
+
+ ret = device_property_read_u32(dev, "i2c-scl-rising-time-ns",
&t->scl_rise_ns);
+ if (ret && use_defaults) {
+ if (t->bus_freq_hz <= 10)
+ t->scl_rise_ns = 1000;
+ else if (t->bus_freq_hz <= 40)
+ t->scl_rise_ns = 300;
+ else
+ t->scl_rise_ns = 120;
+ }
+
+ ret = device_property_read_u32(dev, "i2c-scl-falling-time-ns",
&t->
[PATCH] i2c: emev: select I2C slave support
From: Wolfram Sang Until we have proper support to make I2C slave support fully optional, select it to prevent build errors on randconfigs. Signed-off-by: Wolfram Sang --- drivers/i2c/busses/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig index eaa7b4a0e48438..0299dfa746a387 100644 --- a/drivers/i2c/busses/Kconfig +++ b/drivers/i2c/busses/Kconfig @@ -532,6 +532,7 @@ config I2C_EG20T config I2C_EMEV2 tristate "EMMA Mobile series I2C adapter" depends on HAVE_CLK + select I2C_SLAVE help If you say yes to this option, support will be included for the I2C interface on the Renesas Electronics EM/EV family of processors. -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c: allow building emev2 without slave mode again
On Mon, Dec 14, 2015 at 11:27:22PM +0100, Arnd Bergmann wrote: > On Monday 14 December 2015 14:52:06 Wolfram Sang wrote: > > > > What about not ifdeffing the inline function and keep the build error > > > > whenever someone uses it without I2C_SLAVE being selected? > > > > > > The inline function is only added there for the case that I2C_SLAVE is > > > disabled, so that would be pointless. > > > > > > However, what we could do is move the extern declaration outside of > > > the #ifdef to make it always visible. The if(IS_ENABLED(CONFIG_I2C_SLAVE)) > > > check should then ensure that it never actually gets called, and we > > > get a link error if some driver gets it wrong. > > > > Yes, that's what I meant: move the whole function (as it was before your > > patch) out of the CONFIG_I2C_SLAVE block. We should get a compiler error > > even, because for !I2C_SLAVE, the client struct will not have the > > slave_cb member. > > > > But we don't want a compile-error for randconfig builds, and we don't > want unnecessary #ifdef in the driver. My conclusion for now is: There needs something to be done surely, but currently I don't have the bandwidth to do it or even play around with it. I am not fully happy with your patches as well because __maybe_unused has some kind of "last resort" feeling to me. So, to get the build failures away immediately I'd simply submit a patch for the emev driver to select I2C_SLAVE and postpone the proper fix to later. That being said, thanks a lot for your input. I will surely come back to it. All the best, Wolfram signature.asc Description: Digital signature
Re: linux-next: Tree for Dec 16 (i2c/busses/i2c-emev2)
On Wed, Dec 16, 2015 at 01:05:20PM -0800, Randy Dunlap wrote: > On 12/15/15 21:43, Stephen Rothwell wrote: > > Hi all, > > > > Changes since 20151215: > > > > The drm-misc tree gained a conflict against Linus' tree. > > > > The i2c tree gained a build failure for which I applied a patch. > > > > The gpio tree gained a build failure so I used the version from > > next-20151215. > > > > on x86_64, when CONFIG_I2C_SLAVE is not enabled: We are working on it... signature.asc Description: Digital signature
[PATCH v2] i2c: rcar: disable runtime PM correctly in slave mode
From: Wolfram Sang When we also are I2C slave, we need to disable runtime PM because the address detection mechanism needs to be active all the time. However, we can reenable runtime PM once the slave instance was unregistered. So, use pm_runtime_get_sync/put to achieve this, since it has proper refcounting. pm_runtime_allow/forbid is like a global knob controllable from userspace which is unsuitable here. Signed-off-by: Wolfram Sang --- Change since v1 is the use of get/put instead of enable/disable. Thanks to Alan and Geert for the pointers! drivers/i2c/busses/i2c-rcar.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c index 7c523dcaee3e48..02f9f5d57b1eae 100644 --- a/drivers/i2c/busses/i2c-rcar.c +++ b/drivers/i2c/busses/i2c-rcar.c @@ -532,7 +532,7 @@ static int rcar_reg_slave(struct i2c_client *slave) if (slave->flags & I2C_CLIENT_TEN) return -EAFNOSUPPORT; - pm_runtime_forbid(rcar_i2c_priv_to_dev(priv)); + pm_runtime_get_sync(rcar_i2c_priv_to_dev(priv)); priv->slave = slave; rcar_i2c_write(priv, ICSAR, slave->addr); @@ -554,7 +554,7 @@ static int rcar_unreg_slave(struct i2c_client *slave) priv->slave = NULL; - pm_runtime_allow(rcar_i2c_priv_to_dev(priv)); + pm_runtime_put(rcar_i2c_priv_to_dev(priv)); return 0; } -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC 3/3] i2c: rcar: disable PM in multi-master mode
From: Wolfram Sang
In multi master mode, the IP core needs to be always active for
arbitration reasons. Get the config from DT and set up PM depending on
the config.
Signed-off-by: Wolfram Sang
---
drivers/i2c/busses/i2c-rcar.c | 17 ++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index 79fd2aab8fa087..7c523dcaee3e48 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -96,6 +96,9 @@
#define ID_DONE(1 << 2)
#define ID_ARBLOST (1 << 3)
#define ID_NACK(1 << 4)
+/* persistent flags */
+#define ID_P_PM_BLOCKED(1 << 31)
+#define ID_P_MASK ID_P_PM_BLOCKED
enum rcar_i2c_type {
I2C_RCAR_GEN1,
@@ -277,7 +280,7 @@ static void rcar_i2c_next_msg(struct rcar_i2c_priv *priv)
{
priv->msg++;
priv->msgs_left--;
- priv->flags = 0;
+ priv->flags &= ID_P_MASK;
rcar_i2c_prepare_msg(priv);
}
@@ -495,7 +498,7 @@ static int rcar_i2c_master_xfer(struct i2c_adapter *adap,
/* init first message */
priv->msg = msgs;
priv->msgs_left = num;
- priv->flags = ID_FIRST_MSG;
+ priv->flags = (priv->flags & ID_P_MASK) | ID_FIRST_MSG;
rcar_i2c_prepare_msg(priv);
time_left = wait_event_timeout(priv->wait, priv->flags & ID_DONE,
@@ -630,7 +633,13 @@ static int rcar_i2c_probe(struct platform_device *pdev)
goto out_pm_put;
rcar_i2c_init(priv);
- pm_runtime_put(dev);
+
+ /* Don't suspend when multi-master to keep arbitration working */
+ if (of_get_property(dev->of_node, "multi-master", NULL))
+ priv->flags |= ID_P_PM_BLOCKED;
+ else
+ pm_runtime_put(dev);
+
irq = platform_get_irq(pdev, 0);
ret = devm_request_irq(dev, irq, rcar_i2c_irq, 0, dev_name(dev), priv);
@@ -664,6 +673,8 @@ static int rcar_i2c_remove(struct platform_device *pdev)
struct device *dev = &pdev->dev;
i2c_del_adapter(&priv->adap);
+ if (priv->flags & ID_P_PM_BLOCKED)
+ pm_runtime_put(dev);
pm_runtime_disable(dev);
return 0;
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC 1/3] i2c: document binding for multi-master case
From: Wolfram Sang We need this binding because some I2C master drivers will need to adapt their PM settings for the arbitration circuitry. I skipped the "i2c-" prefix because I can imagine to be useful outside of i2c. Signed-off-by: Wolfram Sang Cc: devicet...@vger.kernel.org --- Documentation/devicetree/bindings/i2c/i2c.txt | 5 + 1 file changed, 5 insertions(+) diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt index a00219f5ee0733..c8d977ed847f3c 100644 --- a/Documentation/devicetree/bindings/i2c/i2c.txt +++ b/Documentation/devicetree/bindings/i2c/i2c.txt @@ -54,6 +54,11 @@ wants to support one of the below features, it should adapt the bindings below. "irq" and "wakeup" names are recognized by I2C core, other names are left to individual drivers. +- multi-master + states that there is another master active on this bus. The OS can use + this information to adapt power management to keep the arbitration awake + all the time, for example. + - wakeup-source device can be used as a wakeup source. -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC 0/3] i2c: rcar: adapt PM usage to multi master case
If we are in a multi-master scenario, we need to block runtime PM so the arbitration circuit stays awake. So, we define a new binding and adapt the i2c-rcar driver to have an example implementation. This series is RFC because I want to do some more regression testing. The actual functionality works fine here on my Lager board. Wolfram Sang (3): i2c: document binding for multi-master case i2c: rcar: remove macros dealing with flags i2c: rcar: disable PM in multi-master mode Documentation/devicetree/bindings/i2c/i2c.txt | 5 drivers/i2c/busses/i2c-rcar.c | 39 --- 2 files changed, 28 insertions(+), 16 deletions(-) -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC 2/3] i2c: rcar: remove macros dealing with flags
From: Wolfram Sang
These macros don't really hide complexity, but C idioms. Removing them
makes the code easier to read IMO and make a planned extension easier.
Signed-off-by: Wolfram Sang
---
drivers/i2c/busses/i2c-rcar.c | 22 +-
1 file changed, 9 insertions(+), 13 deletions(-)
diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index b2389c492579cf..79fd2aab8fa087 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -122,9 +122,6 @@ struct rcar_i2c_priv {
#define rcar_i2c_priv_to_dev(p)((p)->adap.dev.parent)
#define rcar_i2c_is_recv(p)((p)->msg->flags & I2C_M_RD)
-#define rcar_i2c_flags_set(p, f) ((p)->flags |= (f))
-#define rcar_i2c_flags_has(p, f) ((p)->flags & (f))
-
#define LOOP_TIMEOUT 1024
@@ -258,7 +255,7 @@ static void rcar_i2c_prepare_msg(struct rcar_i2c_priv *priv)
priv->pos = 0;
if (priv->msgs_left == 1)
- rcar_i2c_flags_set(priv, ID_LAST_MSG);
+ priv->flags |= ID_LAST_MSG;
rcar_i2c_write(priv, ICMAR, (priv->msg->addr << 1) | read);
/*
@@ -266,7 +263,7 @@ static void rcar_i2c_prepare_msg(struct rcar_i2c_priv *priv)
* of ICMSR and ICMCR depends on whether we issue START or REP_START.
Since
* it didn't cause a drawback for me, let's rather be safe than sorry.
*/
- if (rcar_i2c_flags_has(priv, ID_FIRST_MSG)) {
+ if (priv->flags & ID_FIRST_MSG) {
rcar_i2c_write(priv, ICMSR, 0);
rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_START);
} else {
@@ -438,7 +435,7 @@ static irqreturn_t rcar_i2c_irq(int irq, void *ptr)
/* Arbitration lost */
if (msr & MAL) {
- rcar_i2c_flags_set(priv, (ID_DONE | ID_ARBLOST));
+ priv->flags |= ID_DONE | ID_ARBLOST;
goto out;
}
@@ -446,14 +443,14 @@ static irqreturn_t rcar_i2c_irq(int irq, void *ptr)
if (msr & MNR) {
/* HW automatically sends STOP after received NACK */
rcar_i2c_write(priv, ICMIER, RCAR_IRQ_STOP);
- rcar_i2c_flags_set(priv, ID_NACK);
+ priv->flags |= ID_NACK;
goto out;
}
/* Stop */
if (msr & MST) {
priv->msgs_left--; /* The last message also made it */
- rcar_i2c_flags_set(priv, ID_DONE);
+ priv->flags |= ID_DONE;
goto out;
}
@@ -463,7 +460,7 @@ static irqreturn_t rcar_i2c_irq(int irq, void *ptr)
rcar_i2c_irq_send(priv, msr);
out:
- if (rcar_i2c_flags_has(priv, ID_DONE)) {
+ if (priv->flags & ID_DONE) {
rcar_i2c_write(priv, ICMIER, 0);
rcar_i2c_write(priv, ICMSR, 0);
wake_up(&priv->wait);
@@ -501,15 +498,14 @@ static int rcar_i2c_master_xfer(struct i2c_adapter *adap,
priv->flags = ID_FIRST_MSG;
rcar_i2c_prepare_msg(priv);
- time_left = wait_event_timeout(priv->wait,
-rcar_i2c_flags_has(priv, ID_DONE),
+ time_left = wait_event_timeout(priv->wait, priv->flags & ID_DONE,
num * adap->timeout);
if (!time_left) {
rcar_i2c_init(priv);
ret = -ETIMEDOUT;
- } else if (rcar_i2c_flags_has(priv, ID_NACK)) {
+ } else if (priv->flags & ID_NACK) {
ret = -ENXIO;
- } else if (rcar_i2c_flags_has(priv, ID_ARBLOST)) {
+ } else if (priv->flags & ID_ARBLOST) {
ret = -EAGAIN;
} else {
ret = num - priv->msgs_left; /* The number of transfer */
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c: rcar: disable runtime PM correctly in slave mode
On Wed, Dec 16, 2015 at 04:55:28PM +0100, Geert Uytterhoeven wrote:
> Hi Wolfram,
>
> On Wed, Dec 16, 2015 at 4:43 PM, Wolfram Sang wrote:
> > I have another case, may I ask your advice about this, too? When an I2C
> > bus is marked in DT as multi-master, then RuntimePM also needs to be
> > disabled, because arbitration detection needs to stay awake. I am
> > currently implementing this for the i2c-rcar driver:
> >
> > - pm_runtime_enable(dev);
> > + /* No RuntimePM in multi-master to keep arbitration working */
> > + if (!of_get_property(dev->of_node, "multi-master", NULL)) {
> > + pm_runtime_enable(dev);
> > + priv->flags |= ID_P_PM;
> > + }
> > +
> > pm_runtime_get_sync(dev);
> > ...
> >
> > @@ -664,7 +673,8 @@ static int rcar_i2c_remove(struct platform_device *pdev)
> > struct device *dev = &pdev->dev;
> >
> > i2c_del_adapter(&priv->adap);
> > - pm_runtime_disable(dev);
> > + if (priv->flags & ID_P_PM)
> > + pm_runtime_disable(dev);
> >
> > return 0;
> > }
> >
> > Here, I'd tend to keep using enable/disable, although get/put would
> > probably also work. What is the rule of thumb using this pattern or the
> > other?
>
> Have you actually tried the above?
Nope, I just finished the sketching phase when Alan's mail came along.
> All our drivers rely on Runtime PM for the power/clock domain handling.
> I believe the pm_runtime_get_sync() won't do anything if you haven't enabled
> Runtime PM for the device, so the device's module clock won't be enabled at
> all.
This was another question I had: Is this a bug or a feature :) But if
you say it is policy, I will count this as "feature" and switch to
get/put here as well.
Thanks,
Wolfram
signature.asc
Description: Digital signature
Re: [PATCH] i2c: rcar: disable runtime PM correctly in slave mode
Alan,
> > When we also are I2C slave, we need to disable runtime PM because the
> > address detection mechanism needs to be active all the time. However, we
> > can reenable runtime PM once the slave instance was unregistered. So,
> > use pm_runtime_disable/enable to achieve this, since it has proper
> > refcounting. pm_runtime_allow/forbid is like a global switch which is
> > unsuitable here.
> >
> > Signed-off-by: Wolfram Sang
> > ---
> >
> > I'd be grateful to get an ACK from a runtime PM expert to verify that my
> > assumptions match reality :)
>
> Yes, disabling runtime PM will do what you want. However you might
> consider using pm_runtime_get_sync() and pm_runtime_put() instead,
> because pm_runtime_enable() does not check to see if the device is idle
> and can be suspended right away. Alternatively, you can call
> pm_runtime_idle() after pm_runtime_enable().
Thank you for your answer! I think I'll go for the get/put solution
here.
I have another case, may I ask your advice about this, too? When an I2C
bus is marked in DT as multi-master, then RuntimePM also needs to be
disabled, because arbitration detection needs to stay awake. I am
currently implementing this for the i2c-rcar driver:
- pm_runtime_enable(dev);
+ /* No RuntimePM in multi-master to keep arbitration working */
+ if (!of_get_property(dev->of_node, "multi-master", NULL)) {
+ pm_runtime_enable(dev);
+ priv->flags |= ID_P_PM;
+ }
+
pm_runtime_get_sync(dev);
...
@@ -664,7 +673,8 @@ static int rcar_i2c_remove(struct platform_device *pdev)
struct device *dev = &pdev->dev;
i2c_del_adapter(&priv->adap);
- pm_runtime_disable(dev);
+ if (priv->flags & ID_P_PM)
+ pm_runtime_disable(dev);
return 0;
}
Here, I'd tend to keep using enable/disable, although get/put would
probably also work. What is the rule of thumb using this pattern or the
other?
> pm_runtime_allow/forbid is even more unsuitable than you said, because
> it can be overridden by the user.
Yeah, I realized his today, too.
Thanks!
Wolfram
signature.asc
Description: Digital signature
[PATCH] i2c: rcar: disable runtime PM correctly in slave mode
From: Wolfram Sang When we also are I2C slave, we need to disable runtime PM because the address detection mechanism needs to be active all the time. However, we can reenable runtime PM once the slave instance was unregistered. So, use pm_runtime_disable/enable to achieve this, since it has proper refcounting. pm_runtime_allow/forbid is like a global switch which is unsuitable here. Signed-off-by: Wolfram Sang --- I'd be grateful to get an ACK from a runtime PM expert to verify that my assumptions match reality :) drivers/i2c/busses/i2c-rcar.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c index b2389c492579cf..dc3343291e7497 100644 --- a/drivers/i2c/busses/i2c-rcar.c +++ b/drivers/i2c/busses/i2c-rcar.c @@ -533,7 +533,7 @@ static int rcar_reg_slave(struct i2c_client *slave) if (slave->flags & I2C_CLIENT_TEN) return -EAFNOSUPPORT; - pm_runtime_forbid(rcar_i2c_priv_to_dev(priv)); + pm_runtime_disable(rcar_i2c_priv_to_dev(priv)); priv->slave = slave; rcar_i2c_write(priv, ICSAR, slave->addr); @@ -555,7 +555,7 @@ static int rcar_unreg_slave(struct i2c_client *slave) priv->slave = NULL; - pm_runtime_allow(rcar_i2c_priv_to_dev(priv)); + pm_runtime_enable(rcar_i2c_priv_to_dev(priv)); return 0; } -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] i2c: xlr: add interrupt support for Sigma Designs chips
On Tue, Dec 15, 2015 at 11:15:06PM +, Mans Rullgard wrote: > The Sigma Designs variant of this controller has the ability to generate > interrupts. This is controlled using two additional registers, oddly > enough overlapping with the defined but unused HDSTATIM. > > This patch adds support for using this feature instead of busy-looping > if an IRQ is specified. > > Signed-off-by: Mans Rullgard Applied to for-next, thanks! signature.asc Description: Digital signature
Re: [PATCH v2 1/2] i2c: xlr: fix extra read/write at end of rx transfer
On Tue, Dec 15, 2015 at 11:15:05PM +, Mans Rullgard wrote: > The BYTECNT register holds the transfer size minus one. Setting it to > the correct value removes the need for a dummy read/write at the end of > each transfer. As zero-length transfers are not supported, do not > advertise I2C_FUNC_SMBUS_QUICK. > > In other words, this patch makes the driver transfer the number of bytes > requested unless this is zero, which is not supported by the hardware > and is thus refused. > > Signed-off-by: Mans Rullgard Applied to for-next, thanks! signature.asc Description: Digital signature
[PATCH] i2c: s3c2410: remove superfluous runtime PM calls
Since commit 6ada5c1e1b077a ("i2c: Mark adapter devices with
pm_runtime_no_callbacks"), runtime PM on adapters turned into a no-op.
So, we can remove these calls.
Signed-off-by: Wolfram Sang
---
drivers/i2c/busses/i2c-s3c2410.c | 6 --
1 file changed, 6 deletions(-)
diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
index 5df819610d5280..362a6de548336b 100644
--- a/drivers/i2c/busses/i2c-s3c2410.c
+++ b/drivers/i2c/busses/i2c-s3c2410.c
@@ -784,7 +784,6 @@ static int s3c24xx_i2c_xfer(struct i2c_adapter *adap,
int retry;
int ret;
- pm_runtime_get_sync(&adap->dev);
ret = clk_enable(i2c->clk);
if (ret)
return ret;
@@ -795,7 +794,6 @@ static int s3c24xx_i2c_xfer(struct i2c_adapter *adap,
if (ret != -EAGAIN) {
clk_disable(i2c->clk);
- pm_runtime_put(&adap->dev);
return ret;
}
@@ -805,7 +803,6 @@ static int s3c24xx_i2c_xfer(struct i2c_adapter *adap,
}
clk_disable(i2c->clk);
- pm_runtime_put(&adap->dev);
return -EREMOTEIO;
}
@@ -1256,8 +1253,6 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev)
return ret;
}
- pm_runtime_enable(&i2c->adap.dev);
-
dev_info(&pdev->dev, "%s: S3C I2C adapter\n", dev_name(&i2c->adap.dev));
return 0;
}
@@ -1273,7 +1268,6 @@ static int s3c24xx_i2c_remove(struct platform_device
*pdev)
clk_unprepare(i2c->clk);
- pm_runtime_disable(&i2c->adap.dev);
pm_runtime_disable(&pdev->dev);
s3c24xx_i2c_deregister_cpufreq(i2c);
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] i2c: xlr: add interrupt support for Sigma Designs chips
On Mon, Nov 02, 2015 at 02:03:38AM +, Mans Rullgard wrote: > The Sigma Designs variant of this controller has the ability to generate > interrupts. This is controlled using two additional registers, oddly > enough overlapping with the defined but unused HDSTATIM. > > This patch adds support for using this feature instead of busy-looping > if an IRQ is specified. > > Signed-off-by: Mans Rullgard Looks good but needs to be resend with the updated patch 2/3. Thanks, Wolfram signature.asc Description: Digital signature
Re: [PATCH 2/3] i2c: xlr: fix extra read/write at end of rx transfer
On Mon, Nov 02, 2015 at 02:03:37AM +, Mans Rullgard wrote: > The BYTECNT register holds the transfer size minus one. Setting it > to the correct value requires a dummy read/write only for zero-length > transfers as it is impossible to request one from the hardware. If a > zero-length transfer is requested, changing the length to 1 and setting > "buf" to a dummy location allows making the main loops less convoluted. > > In other words, this patch makes the driver transfer the number of bytes > requested unless this is zero, which is not supported by the hardware, > in which case one byte is transferred instead. Uh, this is wrong, zero byte should really not transfer anything. We need to fix that and bail out, so probably something like if (!len) return -EOPNOTSUPP; Also, the xlr_func() should mask out I2C_FUNC_SMBUS_QUICK. Other than that, the patch looks good to me. Out of curiosity, your first driver had the registers 32bit apart. Now you can deal with 8bit. Is this configurable on this SoC? Thanks, Wolfram signature.asc Description: Digital signature
Re: [PATCH 1/3] i2c: xlr: add support for Sigma Designs controller variant
On Mon, Nov 02, 2015 at 02:03:36AM +, Mans Rullgard wrote: > Sigma Designs chips use a variant of this controller with the following > differences: > > - The BUSY bit in the STATUS register is inverted > - Bit 8 of the CONFIG register must be set > - The controller can generate interrupts > > This patch adds support for the first two of these. It also calculates > and sets the correct clock divisor if a clk is provided. The bus > frequency is optionally speficied in the device tree node. Fixed the "speficied" typo and... > > Signed-off-by: Mans Rullgard ... applied to for-next, thanks! signature.asc Description: Digital signature
Re: [PATCH] i2c: allow building emev2 without slave mode again
> > What about not ifdeffing the inline function and keep the build error > > whenever someone uses it without I2C_SLAVE being selected? > > The inline function is only added there for the case that I2C_SLAVE is > disabled, so that would be pointless. > > However, what we could do is move the extern declaration outside of > the #ifdef to make it always visible. The if(IS_ENABLED(CONFIG_I2C_SLAVE)) > check should then ensure that it never actually gets called, and we > get a link error if some driver gets it wrong. Yes, that's what I meant: move the whole function (as it was before your patch) out of the CONFIG_I2C_SLAVE block. We should get a compiler error even, because for !I2C_SLAVE, the client struct will not have the slave_cb member. signature.asc Description: Digital signature
Re: [PATCH 0/4] i2c: uniphier: add minor sanity checks to i2c-uniphier/i2c-uniphier-f
On Mon, Nov 30, 2015 at 06:53:32PM +0900, Masahiro Yamada wrote: > > > > Masahiro Yamada (4): > i2c: uniphier: error out if clock rate is zero > i2c: uniphier: error out if bus speed is zero > i2c: uniphier_f: error out if clock rate is zero > i2c: uniphier_f: error out if bus speed is zero Applied to for-next, thanks! signature.asc Description: Digital signature
Re: [PATCH v2 0/9] i2c: add generic support for timing parameters in DT
On Tue, Dec 08, 2015 at 10:37:44AM +0100, Wolfram Sang wrote: > Here is a patch series adding better DT support for timing parameters like the > raise time or the fall time which are generic for the I2C subsystem. There is > a > core function for parsing and an implementation for the RCar driver how to use > it. > > I added people of the designware and rk3x driver to CC because they might be > interested in this new function. They provided the basis for this series, so > it > should be easy to convert them. > > Please have a look, test, comment... > > Thanks, > > Wolfram Applied patches 1-5 to for-next with the change Andy suggested, thanks! Simon, you can pick up patches 6-9 now, I'd think. signature.asc Description: Digital signature
Re: [PATCH v2 2/9] i2c: add generic routine to parse DT for timing information
> Or maybe just be a bit verbose like the original variant
> int ret;
>
> ret = device_property_read…(…);
> if (ret && use_defaults) {
> …
> }
>
> Among all variants I like the latter one here. What do you think?
I agree and fixed it like this.
signature.asc
Description: Digital signature
Re: [PATCH] i2c: allow building emev2 without slave mode again
> > > The slave_cb callback function is supposed to set the 'value' > > > here, > > > > Only if a master wants to READ from us. > > Right, and can this never fail? Exactly. The slave can stretch the clock if the value to be sent to the master needs some processing first, but it must deliver something. > With my current code to turn i2c_slave_event() into an empty inline function > in case of !CONFIG_I2C_SLAVE, the compiler knows that "value" is uninitialized > at the point where we write it to the register, and warns about it. I overlooked that your first patch changed this code, sorry. Now I see the problem. What about not ifdeffing the inline function and keep the build error whenever someone uses it without I2C_SLAVE being selected? Thanks, Wolfram signature.asc Description: Digital signature
Re: touchscreen: edt-ft5x06: Prevent DMA driver from mapping an area on stack
> Frankly speaking I do not know where the fix should actually be. I2C IMX > driver somehow taking care of this or the users of I2C, touchscreen drivers > in this case. In my opinion, the fix should be with the touchscreen driver > however I did like to have feedback or hear opinions on what is the accepted > solution to this. There is no accepted solution to this yet :( DMA is/was still too rare for a serious discussion about this. There is also [1] and probably more... [1] http://patchwork.ozlabs.org/patch/220137/ signature.asc Description: Digital signature
Re: [PATCH 1/2] i2c: designware: Keep pm_runtime_enable/_disable calls in sync
On Thu, Dec 10, 2015 at 01:48:43PM +0200, Jarkko Nikula wrote: > On an hardware shared I2C bus (certain Intel Baytrail SoC platforms) the > runtime PM disable depth keeps increasing over repeated modprobe/rmmod > cycle because pm_runtime_disable() is called without checking should it > be disabled already because of bus sharing. > > This hasn't made any other harm than dev->power.disable_depth keeps > increasing but keep it sync by calling pm_runtime_disable() only when > runtime PM is not disabled. > > Signed-off-by: Jarkko Nikula Applied to for-current, thanks! signature.asc Description: Digital signature
Re: [PATCH v2] I2C: designware: fix IO timeout issue for AMD controller
On Fri, Dec 11, 2015 at 08:02:53PM +0800, Xiangliang Yu wrote: > Because of some hardware limitation, AMD I2C controller can't > trigger pending interrupt if interrupt status has been changed > after clearing interrupt status bits. Then, I2C will lost > interrupt and IO timeout. > > According to hardware design, this patch implements a workaround > to disable i2c controller interrupt and re-enable i2c interrupt > before exiting ISR. > > To reduce the performance impacts on other vendors, use unlikely > function to check flag in ISR. > --- Don't manually add "---". This breaks a lot of workflow scripts. "Patchwork" missed your Signed-off, for example! > Changes in v2: > - pass flags with ->driver_data > - unmask interrupt right after masking This paragraph... > > Signed-off-by: Xiangliang Yu > --- ... needs to go here. However, I fixed it this time and applied to for-current, thanks! signature.asc Description: Digital signature
Re: [PATCH] i2c: allow building emev2 without slave mode again
Hi Arnd, thanks for looking into this, but I don't get your point yet. > The slave_cb callback function is supposed to set the 'value' > here, Only if a master wants to READ from us. > but it might return an error not assign the pointer, An error is only returned if a WRITE from a master was not accepted by the slave backend. > It might be best to change the callback to return 'void' and not > allow it to fail. We need that because in case of an errno, the slave should send NACK to the master instead of ACK. > At least the eeprom slave cannot fail anyway, and it is the only > implementation we have at the moment. True. But giving a slave the possibility to NACK a write should be present IMO. > Alternatively, the inline could return an error, and both bus > drivers check for the error before using 'value'. Hum, it does return an error? return client->slave_cb(client, event, val); You probably mean something else? Regards, Wolfram signature.asc Description: Digital signature
Re: [PATCH v6] at24: Support SMBus read/write of 16-bit devices
On Mon, Nov 16, 2015 at 06:02:19PM -0600, Aaron Sierra wrote: > Previously, the at24 driver would bail out in the case of a 16-bit > addressable EEPROM attached to an SMBus controller. This is because > SMBus block reads and writes don't map to I2C multi-byte reads and > writes when the offset portion is 2 bytes. > > Instead of bailing out, this patch settles for functioning with single > byte read SMBus cycles. Writes can be block or single-byte, depending > on SMBus controller features. > > Read access is not without some risk. Multiple SMBus cycles are > required to read even one byte. If the SMBus has multiple masters and > one accesses this EEPROM between the dummy address write and the > subsequent current-address-read cycle(s), this driver will receive > data from the wrong address. > > Functionality has been tested with the following devices: > > AT24CM01 attached to Intel ISCH SMBus > AT24C512 attached to Intel I801 SMBus > > Read performance: > 3.6 KB/s with 32-byte* access > > *limited to 32-bytes by I2C_SMBUS_BLOCK_MAX. > > Write performance: > 248 B/s with 1-byte page (default) > 3.9 KB/s with 128-byte* page (via platform data) > > *limited to 31-bytes by I2C_SMBUS_BLOCK_MAX - 1. > > Signed-off-by: Nate Case > Signed-off-by: Aaron Sierra > Reviewed-by: Jean Delvare > --- > v2 - Account for changes related to introduction of > i2c_smbus_read_i2c_block_data_or_emulated() > v3 - Consolidate three patches into one > - Expand comments regarding SMBus multi-master read risks. > - Rely on current-address-read for improved read performance (i.e. one > dummy address write followed by multiple individual byte reads). > This improves performance from 1.4 KiB/s to 3.6 KiB/s. > - Use struct at24_data's writebuf instead of kzalloc-ing > - Only limit write_max by 1-byte when accessing a 16-bit device with > block writes instead of attempting to preserve a power-of-two. > - Style fixes (indentation, parentheses, unnecessary masking, etc.) > v4 - Address 16-bit safety in Kconfig > - Set "count" to zero later in at24_smbus_read_block_data() > - Fix over-80-columns issues in at24_eeprom_read() > - Fix write_max off-by-one in at24_probe() > - Check SMBus functionality needed for 16-bit device reads > - Homogenize indentation of SMBus functionality checks for SMBus write > v5 - 16-bit device read needs READ_BYTE not READ_BYTE_DATA > - Clarify write_max limiting with smbus_max > - Add X-ES copyright > v6 - Update comment associated with SMBus functionality testing for 16-bit > device read (READ_BYTE not READ_BYTE_DATA) > > drivers/misc/eeprom/Kconfig | 5 +- > drivers/misc/eeprom/at24.c | 132 > +++- > 2 files changed, 122 insertions(+), 15 deletions(-) I remember I had a lengthy discussion with Guenter Roeck about 16 bit support via smbus. I don't have the bandwidth right now to recap the discussion and check if the concerns apply to your implementation as well. Could you check this thread https://lkml.org/lkml/2015/2/4/436 and report back if it applies to your patch as well? This would speed up the review significantly. Thanks, Wolfram signature.asc Description: Digital signature
Re: [RESEND PATCH v2 0/9] eeprom: at24: at24cs series serial number read
On Wed, Dec 02, 2015 at 11:25:17AM +0100, Bartosz Golaszewski wrote: > Chips from the at24cs EEPROM series have an additional read-only memory area > containing a factory pre-programmed serial number. In order to access it, a > dummy write must be executed before reading the serial number bytes. Can't you instantiate a read-only EEPROM on this second address? Or a seperate driver attaching to this address? What is the advantage of having this in at24? signature.asc Description: Digital signature
Re: [PATCH] i2c: allow building emev2 without slave mode again
> Alternatively, the inline could return an error, and both bus > drivers check for the error before using 'value'. I'll try to check these options tomorrow. signature.asc Description: Digital signature
Re: [PATCH] i2c: allow building emev2 without slave mode again
On Thu, Dec 10, 2015 at 02:14:49PM +0100, Arnd Bergmann wrote: > The emev2 driver stopped compiling in today's linux-next kernel: > > drivers/i2c/busses/i2c-emev2.c: In function 'em_i2c_slave_irq': > drivers/i2c/busses/i2c-emev2.c:233:23: error: storage size of 'event' isn't > known > drivers/i2c/busses/i2c-emev2.c:250:3: error: implicit declaration of function > 'i2c_slave_event' [-Werror=implicit-function-declaration] > drivers/i2c/busses/i2c-emev2.c:250:32: error: 'I2C_SLAVE_STOP' undeclared > (first use in this function) > > It works again if we enable CONFIG_I2C_SLAVE, but it seems wrong > to add a dependency on that symbol: > > * The symbol is user-selectable, but only one or two (including this > one) bus drivers actually implement it, and it makes no sense > if you don't have one of them. > > * The other driver (R-Car) uses 'select I2C_SLAVE', which seems > reasonable in principle, but we should not do that on user > visible symbols. > > * I2C slave mode could be implemented in a lot of other drivers > as an optional feature, but we shouldn't require enabling it > if we don't use it. > > This changes the two drivers that provide I2C slave mode so they > can again build if the slave mode being disabled. To do this, I > move the definition of i2c_slave_event() and enum i2c_slave_event > out of the #ifdef and instead make the assignment of the reg_slave > and unreg_slave pointers optional in the bus drivers. The functions > implementing the feature are unused in that case, so they get > marked as __maybe_unused in order to still give compile-time > coverage. Thanks a lot! Making this clear and consistent was on my todo-list, unfortunately below some other items. Both drivers have quite orthogonal slave_irq routines. What do you think about grouping this and the reg/unreg-calls together and compile them conditionally on I2C_SLAVE? I think the code savings are worth it. signature.asc Description: Digital signature
Re: [Patch V1] i2c: imx: init bus recovery info before adding i2c adapter
> I have sent out an i2c runtime patch which was acked by Uwe Kleine. I > thought it would be accepted, so I sent out this one based on that one. Ah, I see. That is okay to assume. However, this patch is 4.4 material, while the other patch is 4.5 material, so I needed that rebase. Thanks, Wolfram signature.asc Description: Digital signature
Re: [Patch V2] i2c: imx: init bus recovery info before adding i2c adapter
On Wed, Dec 09, 2015 at 11:08:22AM +0800, Gao Pan wrote: > During driver probe, i2c_imx_init_recovery_info() must come before > i2c_add_numbered_adapter(), because the get/set_scl() functions > are assigned in i2c_register_adapter() under the conditon that bus > recover_info are initialized. Otherwise, get/set_scl() function > pointers never get assigned. > > In such case, when i2c_generic_gpio_recovery() is used for bus recovery, > there will be kernel crash because bri->set_scl is NULL. > > The solution to this bug is moving i2c_imx_init_recovery_info() before > i2c_register_adapter(). > > Signed-off-by: Gao Pan > Signed-off-by: Fugang Duan Applied to for-current, thanks! signature.asc Description: Digital signature
Re: [PATCH v2 2/9] i2c: add generic routine to parse DT for timing information
On Tue, Dec 08, 2015 at 02:03:23PM +0100, Wolfram Sang wrote: > > > Too many && use_defaults. What about > > > > memset(t, 0, sizeof(*t)); > > > > device_property_read_u32(dev, "i2c-scl-internal-delay-ns", &t- > > >scl_int_delay_ns); > > > > if (!use_defaults) > > return; > > I like this! Thanks for the input. Oops, too enthusiastic. This skips parsing all the other properties... The defaults should only be applied if reading the properties fails. signature.asc Description: Digital signature
Re: [PATCH v2 2/9] i2c: add generic routine to parse DT for timing information
> Too many && use_defaults. What about > > memset(t, 0, sizeof(*t)); > > device_property_read_u32(dev, "i2c-scl-internal-delay-ns", &t- > >scl_int_delay_ns); > > if (!use_defaults) > return; I like this! Thanks for the input. signature.asc Description: Digital signature
Re: [PATCH v2 2/9] i2c: add generic routine to parse DT for timing information
> I wonder if it makes sense to add "i2c-sda-hold-time-ns" (taken from the > designware driver DT binding) to the timings structure? It is tHD;DAT > parameter in the I2C bus specification. It totally makes sense. I just didn't need it for the RCar driver and didn't want to implement something which isn't actually used. So feel free to add it, or ask me to do it, if you promise to use it in the designware driver :) Thanks for the review! signature.asc Description: Digital signature
[PATCH v2 8/9] ARM: shmobile: r8a7794: dtsi: add internal delay for i2c IPs
From: Wolfram Sang Signed-off-by: Wolfram Sang --- arch/arm/boot/dts/r8a7794.dtsi | 6 ++ 1 file changed, 6 insertions(+) diff --git a/arch/arm/boot/dts/r8a7794.dtsi b/arch/arm/boot/dts/r8a7794.dtsi index 6f2f01914db295..37809b8aae65f9 100644 --- a/arch/arm/boot/dts/r8a7794.dtsi +++ b/arch/arm/boot/dts/r8a7794.dtsi @@ -519,6 +519,7 @@ power-domains = <&cpg_clocks>; #address-cells = <1>; #size-cells = <0>; + i2c-scl-internal-delay-ns = <6>; status = "disabled"; }; @@ -530,6 +531,7 @@ power-domains = <&cpg_clocks>; #address-cells = <1>; #size-cells = <0>; + i2c-scl-internal-delay-ns = <6>; status = "disabled"; }; @@ -541,6 +543,7 @@ power-domains = <&cpg_clocks>; #address-cells = <1>; #size-cells = <0>; + i2c-scl-internal-delay-ns = <6>; status = "disabled"; }; @@ -552,6 +555,7 @@ power-domains = <&cpg_clocks>; #address-cells = <1>; #size-cells = <0>; + i2c-scl-internal-delay-ns = <6>; status = "disabled"; }; @@ -563,6 +567,7 @@ power-domains = <&cpg_clocks>; #address-cells = <1>; #size-cells = <0>; + i2c-scl-internal-delay-ns = <6>; status = "disabled"; }; @@ -574,6 +579,7 @@ power-domains = <&cpg_clocks>; #address-cells = <1>; #size-cells = <0>; + i2c-scl-internal-delay-ns = <6>; status = "disabled"; }; -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 5/9] i2c: rcar: honor additional i2c timings from DT
From: Wolfram Sang
Signed-off-by: Wolfram Sang
---
Documentation/devicetree/bindings/i2c/i2c-rcar.txt | 4
drivers/i2c/busses/i2c-rcar.c | 21 -
2 files changed, 16 insertions(+), 9 deletions(-)
diff --git a/Documentation/devicetree/bindings/i2c/i2c-rcar.txt
b/Documentation/devicetree/bindings/i2c/i2c-rcar.txt
index ea406eb20fa5ad..95e97223a71c83 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-rcar.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-rcar.txt
@@ -20,6 +20,10 @@ Optional properties:
propoerty indicates the default frequency 100 kHz.
- clocks: clock specifier.
+- i2c-scl-falling-time-ns: see i2c.txt
+- i2c-scl-internal-delay-ns: see i2c.txt
+- i2c-scl-rising-time-ns: see i2c.txt
+
Examples :
i2c0: i2c@e6508000 {
diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index c663f4389bf898..b2389c492579cf 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -164,12 +164,15 @@ static int rcar_i2c_bus_barrier(struct rcar_i2c_priv
*priv)
static int rcar_i2c_clock_calculate(struct rcar_i2c_priv *priv, struct
i2c_timings *t)
{
- u32 scgd, cdf, round, ick, scl, cdf_width;
+ u32 scgd, cdf, round, ick, sum, scl, cdf_width;
unsigned long rate;
struct device *dev = rcar_i2c_priv_to_dev(priv);
/* Fall back to previously used values if not supplied */
t->bus_freq_hz = t->bus_freq_hz ?: 10;
+ t->scl_fall_ns = t->scl_fall_ns ?: 35;
+ t->scl_rise_ns = t->scl_rise_ns ?: 200;
+ t->scl_int_delay_ns = t->scl_int_delay_ns ?: 50;
switch (priv->devtype) {
case I2C_RCAR_GEN1:
@@ -193,9 +196,9 @@ static int rcar_i2c_clock_calculate(struct rcar_i2c_priv
*priv, struct i2c_timin
* SCL = ick / (20 + SCGD * 8 + F[(ticf + tr + intd) * ick])
*
* ick : I2C internal clock < 20 MHz
-* ticf : I2C SCL falling time = 35 ns here
-* tr : I2C SCL rising time = 200 ns here
-* intd : LSI internal delay= 50 ns here
+* ticf : I2C SCL falling time
+* tr : I2C SCL rising time
+* intd : LSI internal delay
* clkp : peripheral_clk
* F[] : integer up-valuation
*/
@@ -211,12 +214,12 @@ static int rcar_i2c_clock_calculate(struct rcar_i2c_priv
*priv, struct i2c_timin
* it is impossible to calculate large scale
* number on u32. separate it
*
-* F[(ticf + tr + intd) * ick]
-* = F[(35 + 200 + 50)ns * ick]
-* = F[285 * ick / 10]
-* = F[(ick / 100) * 285 / 1000]
+* F[(ticf + tr + intd) * ick] with sum = (ticf + tr + intd)
+* = F[sum * ick / 10]
+* = F[(ick / 100) * sum / 1000]
*/
- round = (ick + 50) / 100 * 285;
+ sum = t->scl_fall_ns + t->scl_rise_ns + t->scl_int_delay_ns;
+ round = (ick + 50) / 100 * sum;
round = (round + 500) / 1000;
/*
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 6/9] ARM: shmobile: r8a7790: dtsi: add internal delay for i2c IPs
From: Wolfram Sang Signed-off-by: Wolfram Sang --- arch/arm/boot/dts/r8a7790.dtsi | 4 1 file changed, 4 insertions(+) diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi index 6cfd0dc79bbec0..64a3f4a05f36d0 100644 --- a/arch/arm/boot/dts/r8a7790.dtsi +++ b/arch/arm/boot/dts/r8a7790.dtsi @@ -417,6 +417,7 @@ interrupts = <0 287 IRQ_TYPE_LEVEL_HIGH>; clocks = <&mstp9_clks R8A7790_CLK_I2C0>; power-domains = <&cpg_clocks>; + i2c-scl-internal-delay-ns = <110>; status = "disabled"; }; @@ -428,6 +429,7 @@ interrupts = <0 288 IRQ_TYPE_LEVEL_HIGH>; clocks = <&mstp9_clks R8A7790_CLK_I2C1>; power-domains = <&cpg_clocks>; + i2c-scl-internal-delay-ns = <6>; status = "disabled"; }; @@ -439,6 +441,7 @@ interrupts = <0 286 IRQ_TYPE_LEVEL_HIGH>; clocks = <&mstp9_clks R8A7790_CLK_I2C2>; power-domains = <&cpg_clocks>; + i2c-scl-internal-delay-ns = <6>; status = "disabled"; }; @@ -450,6 +453,7 @@ interrupts = <0 290 IRQ_TYPE_LEVEL_HIGH>; clocks = <&mstp9_clks R8A7790_CLK_I2C3>; power-domains = <&cpg_clocks>; + i2c-scl-internal-delay-ns = <110>; status = "disabled"; }; -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 7/9] ARM: shmobile: r8a7791: dtsi: add internal delay for i2c IPs
From: Wolfram Sang Signed-off-by: Wolfram Sang --- arch/arm/boot/dts/r8a7791.dtsi | 6 ++ 1 file changed, 6 insertions(+) diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi index 76b33e895513c1..3161dfcdd002dd 100644 --- a/arch/arm/boot/dts/r8a7791.dtsi +++ b/arch/arm/boot/dts/r8a7791.dtsi @@ -407,6 +407,7 @@ interrupts = <0 287 IRQ_TYPE_LEVEL_HIGH>; clocks = <&mstp9_clks R8A7791_CLK_I2C0>; power-domains = <&cpg_clocks>; + i2c-scl-internal-delay-ns = <6>; status = "disabled"; }; @@ -418,6 +419,7 @@ interrupts = <0 288 IRQ_TYPE_LEVEL_HIGH>; clocks = <&mstp9_clks R8A7791_CLK_I2C1>; power-domains = <&cpg_clocks>; + i2c-scl-internal-delay-ns = <6>; status = "disabled"; }; @@ -429,6 +431,7 @@ interrupts = <0 286 IRQ_TYPE_LEVEL_HIGH>; clocks = <&mstp9_clks R8A7791_CLK_I2C2>; power-domains = <&cpg_clocks>; + i2c-scl-internal-delay-ns = <6>; status = "disabled"; }; @@ -440,6 +443,7 @@ interrupts = <0 290 IRQ_TYPE_LEVEL_HIGH>; clocks = <&mstp9_clks R8A7791_CLK_I2C3>; power-domains = <&cpg_clocks>; + i2c-scl-internal-delay-ns = <6>; status = "disabled"; }; @@ -451,6 +455,7 @@ interrupts = <0 19 IRQ_TYPE_LEVEL_HIGH>; clocks = <&mstp9_clks R8A7791_CLK_I2C4>; power-domains = <&cpg_clocks>; + i2c-scl-internal-delay-ns = <6>; status = "disabled"; }; @@ -463,6 +468,7 @@ interrupts = <0 20 IRQ_TYPE_LEVEL_HIGH>; clocks = <&mstp9_clks R8A7791_CLK_I2C5>; power-domains = <&cpg_clocks>; + i2c-scl-internal-delay-ns = <110>; status = "disabled"; }; -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/9] i2c: add generic routine to parse DT for timing information
From: Wolfram Sang
Inspired from the i2c-rk3x driver (thanks guys!) but refactored and
extended. See built-in docs for further information.
Signed-off-by: Wolfram Sang
---
drivers/i2c/i2c-core.c | 47 +++
include/linux/i2c.h| 18 ++
2 files changed, 65 insertions(+)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index ba8eb087f22465..e94d2ca2aab4aa 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -53,6 +53,7 @@
#include
#include
#include
+#include
#include "i2c-core.h"
@@ -1438,6 +1439,52 @@ static void of_i2c_register_devices(struct i2c_adapter
*adap)
}
}
+/**
+ * i2c_parse_fw_timings - get I2C related timing parameters from firmware
+ * @dev: The device to scan for I2C timing properties
+ * @t: the i2c_timings struct to be filled with values
+ * @use_defaults: bool to use sane defaults derived from the I2C specification
+ * when properties are not found, otherwise use 0
+ *
+ * Scan the device for the generic I2C properties describing timing parameters
+ * for the signal and fill the given struct with the results. If a property was
+ * not found and use_defaults was true, then maximum timings are assumed which
+ * are derived from the I2C specification. If use_defaults is not used, the
+ * results will be 0, so drivers can apply their own defaults later. The latter
+ * is mainly intended for avoiding regressions of existing drivers which want
+ * to switch to this function. New drivers almost always should use the
defaults.
+ */
+
+void i2c_parse_fw_timings(struct device *dev, struct i2c_timings *t, bool
use_defaults)
+{
+ memset(t, 0, sizeof(*t));
+
+ if (device_property_read_u32(dev, "clock-frequency", &t->bus_freq_hz)
&& use_defaults)
+ t->bus_freq_hz = 10;
+
+ if (device_property_read_u32(dev, "i2c-scl-rising-time-ns",
&t->scl_rise_ns) && use_defaults) {
+ if (t->bus_freq_hz <= 10)
+ t->scl_rise_ns = 1000;
+ else if (t->bus_freq_hz <= 40)
+ t->scl_rise_ns = 300;
+ else
+ t->scl_rise_ns = 120;
+ }
+
+ if (device_property_read_u32(dev, "i2c-scl-falling-time-ns",
&t->scl_fall_ns) && use_defaults) {
+ if (t->bus_freq_hz <= 40)
+ t->scl_fall_ns = 300;
+ else
+ t->scl_fall_ns = 120;
+ }
+
+ device_property_read_u32(dev, "i2c-scl-internal-delay-ns",
&t->scl_int_delay_ns);
+
+ if (device_property_read_u32(dev, "i2c-sda-falling-time-ns",
&t->sda_fall_ns) && use_defaults)
+ t->sda_fall_ns = t->scl_fall_ns;
+}
+EXPORT_SYMBOL_GPL(i2c_parse_fw_timings);
+
static int of_dev_node_match(struct device *dev, void *data)
{
return dev->of_node == data;
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 768063baafbf5e..7c45181f41ab7d 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -414,6 +414,22 @@ struct i2c_algorithm {
};
/**
+ * struct i2c_timings - I2C timing information
+ * @bus_freq_hz: the bus frequency in Hz
+ * @scl_rise_ns: time SCL signal takes to rise in ns; t(r) in the I2C
specification
+ * @scl_fall_ns: time SCL signal takes to fall in ns; t(f) in the I2C
specification
+ * @scl_int_delay_ns: time IP core additionally needs to setup SCL in ns
+ * @sda_fall_ns: time SDA signal takes to fall in ns; t(f) in the I2C
specification
+ */
+struct i2c_timings {
+ u32 bus_freq_hz;
+ u32 scl_rise_ns;
+ u32 scl_fall_ns;
+ u32 scl_int_delay_ns;
+ u32 sda_fall_ns;
+};
+
+/**
* struct i2c_bus_recovery_info - I2C bus recovery information
* @recover_bus: Recover routine. Either pass driver's recover_bus() routine,
or
* i2c_generic_scl_recovery() or i2c_generic_gpio_recovery().
@@ -602,6 +618,7 @@ extern void i2c_clients_command(struct i2c_adapter *adap,
extern struct i2c_adapter *i2c_get_adapter(int nr);
extern void i2c_put_adapter(struct i2c_adapter *adap);
+void i2c_parse_fw_timings(struct device *dev, struct i2c_timings *t, bool
use_defaults);
/* Return the functionality mask */
static inline u32 i2c_get_functionality(struct i2c_adapter *adap)
@@ -644,6 +661,7 @@ extern struct i2c_adapter
*of_find_i2c_adapter_by_node(struct device_node *node)
/* must call i2c_put_adapter() when done with returned i2c_adapter device */
struct i2c_adapter *of_get_i2c_adapter_by_node(struct device_node *node);
+
#else
static inline struct i2c_client *of_find_i2c_device_by_node(struct device_node
*node)
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/9] i2c: rcar: refactor probe function a little
From: Wolfram Sang
The probe function is a little bit messy, something here, something
there. Rework it so that there is some order: first init the private
structure, then the adapter, then do HW init. This also allows us to
remove the device argument of the clock calculation function, because it
now can be deduced from the private structure. Also, shorten some lines
where possible. This is a preparation for further refactoring.
Signed-off-by: Wolfram Sang
---
drivers/i2c/busses/i2c-rcar.c | 40 +---
1 file changed, 17 insertions(+), 23 deletions(-)
diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index 3ed1f0aa5eeb16..d4322a9096786f 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -162,15 +162,11 @@ static int rcar_i2c_bus_barrier(struct rcar_i2c_priv
*priv)
return -EBUSY;
}
-static int rcar_i2c_clock_calculate(struct rcar_i2c_priv *priv,
- u32 bus_speed,
- struct device *dev)
+static int rcar_i2c_clock_calculate(struct rcar_i2c_priv *priv, u32 bus_speed)
{
- u32 scgd, cdf;
- u32 round, ick;
- u32 scl;
- u32 cdf_width;
+ u32 scgd, cdf, round, ick, scl, cdf_width;
unsigned long rate;
+ struct device *dev = rcar_i2c_priv_to_dev(priv);
switch (priv->devtype) {
case I2C_RCAR_GEN1:
@@ -610,21 +606,7 @@ static int rcar_i2c_probe(struct platform_device *pdev)
if (IS_ERR(priv->io))
return PTR_ERR(priv->io);
- bus_speed = 10; /* default 100 kHz */
- of_property_read_u32(dev->of_node, "clock-frequency", &bus_speed);
-
priv->devtype = (enum rcar_i2c_type)of_match_device(rcar_i2c_dt_ids,
dev)->data;
-
- pm_runtime_enable(dev);
- pm_runtime_get_sync(dev);
- ret = rcar_i2c_clock_calculate(priv, bus_speed, dev);
- if (ret < 0)
- goto out_pm_put;
-
- rcar_i2c_init(priv);
- pm_runtime_put(dev);
-
- irq = platform_get_irq(pdev, 0);
init_waitqueue_head(&priv->wait);
adap = &priv->adap;
@@ -637,8 +619,20 @@ static int rcar_i2c_probe(struct platform_device *pdev)
i2c_set_adapdata(adap, priv);
strlcpy(adap->name, pdev->name, sizeof(adap->name));
- ret = devm_request_irq(dev, irq, rcar_i2c_irq, 0,
- dev_name(dev), priv);
+ bus_speed = 10; /* default 100 kHz */
+ of_property_read_u32(dev->of_node, "clock-frequency", &bus_speed);
+
+ pm_runtime_enable(dev);
+ pm_runtime_get_sync(dev);
+ ret = rcar_i2c_clock_calculate(priv, bus_speed);
+ if (ret < 0)
+ goto out_pm_put;
+
+ rcar_i2c_init(priv);
+ pm_runtime_put(dev);
+
+ irq = platform_get_irq(pdev, 0);
+ ret = devm_request_irq(dev, irq, rcar_i2c_irq, 0, dev_name(dev), priv);
if (ret < 0) {
dev_err(dev, "cannot get irq %d\n", irq);
goto out_pm_disable;
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/9] i2c: add generic support for timing parameters in DT
Here is a patch series adding better DT support for timing parameters like the raise time or the fall time which are generic for the I2C subsystem. There is a core function for parsing and an implementation for the RCar driver how to use it. I added people of the designware and rk3x driver to CC because they might be interested in this new function. They provided the basis for this series, so it should be easy to convert them. Please have a look, test, comment... Thanks, Wolfram Changes since V1: * switched from DT properties to device properties * that means no dependency on OF and rename of the parsing function * removed a superfluous comment * adapted the RCar driver to the changes * added Rob's ack Changes since RFC: * better tested * added documentation for the new function and the new struct * reworded some commit messages slightly * moved the new property in the dtsi above the "status" property Wolfram Sang (9): i2c: document generic DT bindings for timing parameters i2c: add generic routine to parse DT for timing information i2c: rcar: refactor probe function a little i2c: rcar: switch to i2c generic dt parsing i2c: rcar: honor additional i2c timings from DT ARM: shmobile: r8a7790: dtsi: add internal delay for i2c IPs ARM: shmobile: r8a7791: dtsi: add internal delay for i2c IPs ARM: shmobile: r8a7794: dtsi: add internal delay for i2c IPs arm64: renesas: r8a7795: add internal delay for i2c IPs Documentation/devicetree/bindings/i2c/i2c-rcar.txt | 4 ++ Documentation/devicetree/bindings/i2c/i2c.txt | 31 -- arch/arm/boot/dts/r8a7790.dtsi | 4 ++ arch/arm/boot/dts/r8a7791.dtsi | 6 ++ arch/arm/boot/dts/r8a7794.dtsi | 6 ++ arch/arm64/boot/dts/renesas/r8a7795.dtsi | 7 +++ drivers/i2c/busses/i2c-rcar.c | 67 +++--- drivers/i2c/i2c-core.c | 47 +++ include/linux/i2c.h| 18 ++ 9 files changed, 151 insertions(+), 39 deletions(-) -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 4/9] i2c: rcar: switch to i2c generic dt parsing
From: Wolfram Sang
Switch to the new generic functions. Plain convert, no functionality
added yet.
Signed-off-by: Wolfram Sang
---
drivers/i2c/busses/i2c-rcar.c | 16 +---
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index d4322a9096786f..c663f4389bf898 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -162,12 +162,15 @@ static int rcar_i2c_bus_barrier(struct rcar_i2c_priv
*priv)
return -EBUSY;
}
-static int rcar_i2c_clock_calculate(struct rcar_i2c_priv *priv, u32 bus_speed)
+static int rcar_i2c_clock_calculate(struct rcar_i2c_priv *priv, struct
i2c_timings *t)
{
u32 scgd, cdf, round, ick, scl, cdf_width;
unsigned long rate;
struct device *dev = rcar_i2c_priv_to_dev(priv);
+ /* Fall back to previously used values if not supplied */
+ t->bus_freq_hz = t->bus_freq_hz ?: 10;
+
switch (priv->devtype) {
case I2C_RCAR_GEN1:
cdf_width = 2;
@@ -230,7 +233,7 @@ static int rcar_i2c_clock_calculate(struct rcar_i2c_priv
*priv, u32 bus_speed)
*/
for (scgd = 0; scgd < 0x40; scgd++) {
scl = ick / (20 + (scgd * 8) + round);
- if (scl <= bus_speed)
+ if (scl <= t->bus_freq_hz)
goto scgd_find;
}
dev_err(dev, "it is impossible to calculate best SCL\n");
@@ -238,7 +241,7 @@ static int rcar_i2c_clock_calculate(struct rcar_i2c_priv
*priv, u32 bus_speed)
scgd_find:
dev_dbg(dev, "clk %d/%d(%lu), round %u, CDF:0x%x, SCGD: 0x%x\n",
- scl, bus_speed, clk_get_rate(priv->clk), round, cdf, scgd);
+ scl, t->bus_freq_hz, clk_get_rate(priv->clk), round, cdf, scgd);
/* keep icccr value */
priv->icccr = scgd << cdf_width | cdf;
@@ -588,7 +591,7 @@ static int rcar_i2c_probe(struct platform_device *pdev)
struct i2c_adapter *adap;
struct resource *res;
struct device *dev = &pdev->dev;
- u32 bus_speed;
+ struct i2c_timings i2c_t;
int irq, ret;
priv = devm_kzalloc(dev, sizeof(struct rcar_i2c_priv), GFP_KERNEL);
@@ -619,12 +622,11 @@ static int rcar_i2c_probe(struct platform_device *pdev)
i2c_set_adapdata(adap, priv);
strlcpy(adap->name, pdev->name, sizeof(adap->name));
- bus_speed = 10; /* default 100 kHz */
- of_property_read_u32(dev->of_node, "clock-frequency", &bus_speed);
+ i2c_parse_fw_timings(dev, &i2c_t, false);
pm_runtime_enable(dev);
pm_runtime_get_sync(dev);
- ret = rcar_i2c_clock_calculate(priv, bus_speed);
+ ret = rcar_i2c_clock_calculate(priv, &i2c_t);
if (ret < 0)
goto out_pm_put;
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 9/9] arm64: renesas: r8a7795: add internal delay for i2c IPs
From: Wolfram Sang Signed-off-by: Wolfram Sang --- arch/arm64/boot/dts/renesas/r8a7795.dtsi | 7 +++ 1 file changed, 7 insertions(+) diff --git a/arch/arm64/boot/dts/renesas/r8a7795.dtsi b/arch/arm64/boot/dts/renesas/r8a7795.dtsi index 56d6a5b23a4e38..b02a7ad1583933 100644 --- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi @@ -519,6 +519,7 @@ interrupts = ; clocks = <&cpg CPG_MOD 931>; power-domains = <&cpg>; + i2c-scl-internal-delay-ns = <110>; status = "disabled"; }; @@ -530,6 +531,7 @@ interrupts = ; clocks = <&cpg CPG_MOD 930>; power-domains = <&cpg>; + i2c-scl-internal-delay-ns = <6>; status = "disabled"; }; @@ -541,6 +543,7 @@ interrupts = ; clocks = <&cpg CPG_MOD 929>; power-domains = <&cpg>; + i2c-scl-internal-delay-ns = <6>; status = "disabled"; }; @@ -552,6 +555,7 @@ interrupts = ; clocks = <&cpg CPG_MOD 928>; power-domains = <&cpg>; + i2c-scl-internal-delay-ns = <110>; status = "disabled"; }; @@ -563,6 +567,7 @@ interrupts = ; clocks = <&cpg CPG_MOD 927>; power-domains = <&cpg>; + i2c-scl-internal-delay-ns = <110>; status = "disabled"; }; @@ -574,6 +579,7 @@ interrupts = ; clocks = <&cpg CPG_MOD 919>; power-domains = <&cpg>; + i2c-scl-internal-delay-ns = <110>; status = "disabled"; }; @@ -585,6 +591,7 @@ interrupts = ; clocks = <&cpg CPG_MOD 918>; power-domains = <&cpg>; + i2c-scl-internal-delay-ns = <6>; status = "disabled"; }; -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/9] i2c: document generic DT bindings for timing parameters
From: Wolfram Sang Also, sort the properties alphabetically and make indentation consistent. Wording largely taken from i2c-rk3x.txt, thanks guys! Only "i2c-scl-internal-delay-ns" is new, the rest is used by two drivers already and was documented in their driver binding documentation. Signed-off-by: Wolfram Sang Acked-by: Rob Herring --- Documentation/devicetree/bindings/i2c/i2c.txt | 31 ++- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt index 8a99150ac3a7fd..a00219f5ee0733 100644 --- a/Documentation/devicetree/bindings/i2c/i2c.txt +++ b/Documentation/devicetree/bindings/i2c/i2c.txt @@ -29,12 +29,33 @@ Optional properties These properties may not be supported by all drivers. However, if a driver wants to support one of the below features, it should adapt the bindings below. -- clock-frequency - frequency of bus clock in Hz. -- wakeup-source- device can be used as a wakeup source. +- clock-frequency + frequency of bus clock in Hz. -- interrupts - interrupts used by the device. -- interrupt-names - "irq" and "wakeup" names are recognized by I2C core, - other names are left to individual drivers. +- i2c-scl-falling-time-ns + Number of nanoseconds the SCL signal takes to fall; t(f) in the I2C + specification. + +- i2c-scl-internal-delay-ns + Number of nanoseconds the IP core additionally needs to setup SCL. + +- i2c-scl-rising-time-ns + Number of nanoseconds the SCL signal takes to rise; t(r) in the I2C + specification. + +- i2c-sda-falling-time-ns + Number of nanoseconds the SDA signal takes to fall; t(f) in the I2C + specification. + +- interrupts + interrupts used by the device. + +- interrupt-names + "irq" and "wakeup" names are recognized by I2C core, other names are + left to individual drivers. + +- wakeup-source + device can be used as a wakeup source. Binding may contain optional "interrupts" property, describing interrupts used by the device. I2C core will assign "irq" interrupt (or the very first -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
