Re: [PATCH] i2c: rk3x: init module as subsys call

[Heiko Stuebner]

Hi Tao,

Am Dienstag, 5. Januar 2016, 15:42:32 schrieb Huang, Tao:
> On 2016年01月05日 15:02, Heiko Stuebner wrote:
> > Hi Jianqun,
> > 
> > Am Dienstag, 5. Januar 2016, 11:02:18 schrieb jianqun.xu:
> >> From: Xu Jianqun 
> >> 
> >> There is a requirement from pmic device, which is on the i2c bus,
> >> that the pmic needs to be called earlier then devices powered by
> >> the outputs of the pmic, if not, the devices maybe fail to probe.
> >> 
> >> For example, a pmic on i2c0, and touchscreen device on i2c2,
> >> i2c0: - pmic(rk818)
> >> i2c2: - ts(gt911), powered by rk818 on i2c0
> >> 
> >> The problem will happen if the i2c2 node in dts file is ordered
> >> before i2c0 node, then ts(gt911) will be probed before pmic(rk818),
> >> since the power from the pmic(rk818) for ts(gt911) hasn't enabled,
> >> so ts(gt911) will fail to probe due to the failure of i2c test.
> >> 
> >> But if we set the i2c0 node before i2c2, there is no this issue.
> >> 
> >> The stable way to make sure that pmic can be intalized before other
> >> peripher devices is to make the pmic module be subsys call, the i2c
> >> module need to be subsys call firstly.
> > 
> > I do believe that came up in the past already and the direction from
> > then
> > was (and most likely still is) that drivers should make use of the
> > probe-
> > deferral mechanism instead of wiggling with the initcall ordering.
> 
> I don't think this is a good idea. This will trigger a lots of init call
> failed. Before pmic init, all i2c device driver transmit will failed,
> and because i2c is slow bus, and i2c transmit may failed by other
> reasons, so the i2c driver and i2c device driver will try many times to
> make sure the transmit completion. These unnecessary transmission will
> make Linux boot very slow.

In general, the slowdown won't be _this_ much if touchscreen drivers need 
one deferral-round before i2c is available. I'm also only pointing out 
things I remember from the last time this came up. 

rk3x-i2c even was here already:
http://www.spinics.net/lists/linux-i2c/msg16680.html


> I2C bus should be subsys, and we can easy resolve this problem, why we
> depends on a complicated and slow implementation?

because it's the only safe way to do that. Because now you need i2c-init at 
subsys-init time, some months later some other soc may need some other 
ordering, especially needing i2c-init later/earlier.

Going through the deferral mechanism is the only way currently available to 
actually make this work on all socs.

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.


> > Your touchscreen will have a "xyz-supply" property and I think the
> > regulator-framework should already emit a -EPROBE_DEFER at
> > regulator_get,
> > when the regulator is specified but not available yet.
> 
> Unfortunately, mostly driver do not support regulator api. They are
> suppose power is on.

Having touchscreen drivers support its proper supply-regulators is not 
rocket science ;-) [0] , so I would consider this a bug in the touchscreen 
driver itself.

Just look into the datasheet and add the appropriate supplies to the drivers 
in question.


Heiko

[0] citiing my own work: 
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/input/touchscreen/zforce_ts.c#n806

> 
> Thanks.
> Huang, Tao
> 
> > Heiko
> > 
> >> Signed-off-by: Xu Jianqun 
> >> ---
> >> 
> >>  drivers/i2c/busses/i2c-rk3x.c | 12 +++-
> >>  1 file changed, 11 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/drivers/i2c/busses/i2c-rk3x.c
> >> b/drivers/i2c/busses/i2c-rk3x.c index c1935eb..00e5959 100644
> >> --- a/drivers/i2c/busses/i2c-rk3x.c
> >> +++ b/drivers/i2c/busses/i2c-rk3x.c
> >> @@ -1037,7 +1037,17 @@ static struct platform_driver rk3x_i2c_driver =
> >> {
> >> 
> >>},
> >>  
> >>  };
> >> 
> >> -module_platform_driver(rk3x_i2c_driver);
> >> +static int __init rk3x_i2c_init(void)
> >> +{
> >> +  return platform_driver_register(_i2c_driver);
> >> +}
> >> +subsys_initcall(rk3x_i2c_init);
> >> +
> >> +static void __exit rk3x_i2c_exit(void)
> >> +{
> >> +  platform_driver_unregister(_i2c_driver);
> >> +}
> >> +module_exit(rk3x_i2c_exit);
> >> 
> >>  MODULE_DESCRIPTION("Rockchip RK3xxx I2C Bus driver");
> >>  MODULE_AUTHOR("Max Schwarz ");

--
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: rk3x: init module as subsys call

[Wolfram Sang]

> > 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: [PATCH] i2c/designware: enable i2c controller to suspend/resume asynchronously

[Jarkko Nikula]

Hi

On 12/24/2015 04:30 PM, Fu, Zhonghui wrote:

Now, PM core supports asynchronous suspend/resume mode for devices
during system suspend/resume, and the power state transition of one
device may be completed in separate kernel thread. PM core ensures
all power state transition dependency between devices. This patch
enables designware i2c controllers to suspend/resume asynchronously.
This will take advantage of multicore and improve system suspend/resume
speed. After enabling all i2c devices, i2c adapters and i2c controllers
on ASUS T100TA tablet, the system suspend-to-idle time is reduced to
about 510ms from 750ms, and the system resume time is reduced to about
790ms from 900ms.


Nice reduction :-)


diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c 
b/drivers/i2c/busses/i2c-designware-platdrv.c
index 6b00061..395130b 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -230,6 +230,7 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
}

adap = >adapter;
+   device_enable_async_suspend(>dev);
adap->owner = THIS_MODULE;
adap->class = I2C_CLASS_DEPRECATED;
ACPI_COMPANION_SET(>dev, ACPI_COMPANION(>dev));


Does device_enable_async_suspend() need to be called before enabling 
runtime PM? I suppose not since there appears to have also related sysfs 
node for toggling it runtime.


I'm thinking if you could move the device_enable_async_suspend() call 
into drivers/i2c/busses/i2c-designware-core.c: i2c_dw_probe() and then 
also PCI enumerated adapter could take advantage of it.


--
Jarkko
--
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 01/10] i2c-mux: add common core data for every mux instance

[Peter Rosin]

Hi Guenter,

On 2016-01-04 16:37, Guenter Roeck wrote:
> On 01/04/2016 07:10 AM, Peter Rosin wrote:
>> From: Peter Rosin 
>>
>> The initial core mux structure starts off small with only the parent
>> adapter pointer, which all muxes have, and a priv pointer for mux
>> driver private data.
>>
>> Add i2c_mux_alloc function to unify the creation of a mux.
>>
>> Where appropriate, pass around the mux core structure instead of the
>> parent adapter or the driver private data.
>>
>> Remove the parent adapter pointer from the driver private data for all
>> mux drivers.
>>
>> Signed-off-by: Peter Rosin 
>> ---
>>   drivers/i2c/i2c-mux.c  | 35 
>> -
>>   drivers/i2c/muxes/i2c-arb-gpio-challenge.c | 24 +++-
>>   drivers/i2c/muxes/i2c-mux-gpio.c   | 20 +
>>   drivers/i2c/muxes/i2c-mux-pca9541.c| 36 
>> --
>>   drivers/i2c/muxes/i2c-mux-pca954x.c| 22 +-
>>   drivers/i2c/muxes/i2c-mux-pinctrl.c| 24 +++-
>>   drivers/i2c/muxes/i2c-mux-reg.c| 25 -
>>   include/linux/i2c-mux.h| 14 +++-
>>   8 files changed, 129 insertions(+), 71 deletions(-)
>>

*snip*

>> +struct i2c_mux_core *i2c_mux_alloc(struct device *dev, int sizeof_priv)
>> +{
>> +struct i2c_mux_core *muxc;
>> +
>> +muxc = devm_kzalloc(dev, sizeof(*muxc), GFP_KERNEL);
>> +if (!muxc)
>> +return NULL;
>> +if (sizeof_priv) {
>> +muxc->priv = devm_kzalloc(dev, sizeof_priv, GFP_KERNEL);
>> +if (!muxc->priv)
>> +goto fail;
>> +}
> 
> Why not just allocate sizeof(*muxc) + sizeof_priv in a single operation
> and then assign muxc->priv to muxc + 1 if sizeof_priv > 0 ?

Why indeed, good suggestion.

*snip*

>> @@ -134,13 +134,14 @@ static int i2c_arbitrator_probe(struct platform_device 
>> *pdev)
>>   return -EINVAL;
>>   }
>>
>> -arb = devm_kzalloc(dev, sizeof(*arb), GFP_KERNEL);
>> -if (!arb) {
>> -dev_err(dev, "Cannot allocate i2c_arbitrator_data\n");
>> +muxc = i2c_mux_alloc(dev, sizeof(*arb));
>> +if (!muxc) {
>> +dev_err(dev, "Cannot allocate i2c_mux_core structure\n");
> 
> Unnecessary error message.
> 

Right, I'll remove that (and the others just like it).

I'll see if I can cook up a v2 that also converts the i2c muxes elsewhere in
drivers/ that I wasn't aware of.

Cheers,
Peter
--
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: rk3x: init module as subsys call

[Huang, Tao]

Hi, Heiko:

On 2016年01月05日 16:00, Heiko Stuebner wrote:
> Hi Tao,
> 
> Am Dienstag, 5. Januar 2016, 15:42:32 schrieb Huang, Tao:

>> I don't think this is a good idea. This will trigger a lots of init call
>> failed. Before pmic init, all i2c device driver transmit will failed,
>> and because i2c is slow bus, and i2c transmit may failed by other
>> reasons, so the i2c driver and i2c device driver will try many times to
>> make sure the transmit completion. These unnecessary transmission will
>> make Linux boot very slow.
> 
> In general, the slowdown won't be _this_ much if touchscreen drivers need 
> one deferral-round before i2c is available. I'm also only pointing out 
> things I remember from the last time this came up. 
> 
> rk3x-i2c even was here already:
> http://www.spinics.net/lists/linux-i2c/msg16680.html

OK. I don't agree with the rule, but we will follow it.

> 
> 
>> I2C bus should be subsys, and we can easy resolve this problem, why we
>> depends on a complicated and slow implementation?
> 
> because it's the only safe way to do that. Because now you need i2c-init at 
> subsys-init time, some months later some other soc may need some other 
> ordering, especially needing i2c-init later/earlier.
> 
> Going through the deferral mechanism is the only way currently available to 
> actually make this work on all socs.
> 
> 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.
> 
> 
>>> Your touchscreen will have a "xyz-supply" property and I think the
>>> regulator-framework should already emit a -EPROBE_DEFER at
>>> regulator_get,
>>> when the regulator is specified but not available yet.
>>
>> Unfortunately, mostly driver do not support regulator api. They are
>> suppose power is on.
> 
> Having touchscreen drivers support its proper supply-regulators is not 
> rocket science ;-) [0] , so I would consider this a bug in the touchscreen 
> driver itself.

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 :(

Thanks!
Huang, Tao

--
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/8] i2c-mux: add common core data for every mux instance

[Peter Rosin]

From: Peter Rosin 

The initial core mux structure starts off small with only the parent
adapter pointer, which all muxes have, and a priv pointer for mux
driver private data.

Add i2c_mux_alloc function to unify the creation of a mux.

Where appropriate, pass around the mux core structure instead of the
parent adapter or the driver private data.

Remove the parent adapter pointer from the driver private data for all
mux drivers.

Signed-off-by: Peter Rosin 
---
 drivers/i2c/i2c-mux.c| 28 +-
 drivers/i2c/muxes/i2c-arb-gpio-challenge.c   | 24 +--
 drivers/i2c/muxes/i2c-mux-gpio.c | 20 
 drivers/i2c/muxes/i2c-mux-pca9541.c  | 35 ++--
 drivers/i2c/muxes/i2c-mux-pca954x.c  | 19 ++-
 drivers/i2c/muxes/i2c-mux-pinctrl.c  | 23 +-
 drivers/i2c/muxes/i2c-mux-reg.c  | 23 ++
 drivers/iio/imu/inv_mpu6050/inv_mpu_core.c   | 10 +++-
 drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h|  1 +
 drivers/media/dvb-frontends/m88ds3103.c  | 10 +++-
 drivers/media/dvb-frontends/m88ds3103_priv.h |  1 +
 drivers/media/dvb-frontends/rtl2830.c| 10 +++-
 drivers/media/dvb-frontends/rtl2830_priv.h   |  1 +
 drivers/media/dvb-frontends/rtl2832.c| 10 +++-
 drivers/media/dvb-frontends/rtl2832_priv.h   |  1 +
 drivers/media/dvb-frontends/si2168.c | 10 +++-
 drivers/media/dvb-frontends/si2168_priv.h|  1 +
 drivers/media/usb/cx231xx/cx231xx-core.c |  3 +++
 drivers/media/usb/cx231xx/cx231xx-i2c.c  | 13 +--
 drivers/media/usb/cx231xx/cx231xx.h  |  2 ++
 drivers/of/unittest.c| 16 +++--
 include/linux/i2c-mux.h  | 14 ++-
 22 files changed, 187 insertions(+), 88 deletions(-)

diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
index 00fc5b1c7b66..c2163f6b51d5 100644
--- a/drivers/i2c/i2c-mux.c
+++ b/drivers/i2c/i2c-mux.c
@@ -31,8 +31,8 @@
 struct i2c_mux_priv {
struct i2c_adapter adap;
struct i2c_algorithm algo;
+   struct i2c_mux_core *muxc;
 
-   struct i2c_adapter *parent;
struct device *mux_dev;
void *mux_priv;
u32 chan_id;
@@ -45,7 +45,8 @@ static int i2c_mux_master_xfer(struct i2c_adapter *adap,
   struct i2c_msg msgs[], int num)
 {
struct i2c_mux_priv *priv = adap->algo_data;
-   struct i2c_adapter *parent = priv->parent;
+   struct i2c_mux_core *muxc = priv->muxc;
+   struct i2c_adapter *parent = muxc->parent;
int ret;
 
/* Switch to the right mux port and perform the transfer. */
@@ -65,7 +66,8 @@ static int i2c_mux_smbus_xfer(struct i2c_adapter *adap,
  int size, union i2c_smbus_data *data)
 {
struct i2c_mux_priv *priv = adap->algo_data;
-   struct i2c_adapter *parent = priv->parent;
+   struct i2c_mux_core *muxc = priv->muxc;
+   struct i2c_adapter *parent = muxc->parent;
int ret;
 
/* Select the right mux port and perform the transfer. */
@@ -84,7 +86,7 @@ static int i2c_mux_smbus_xfer(struct i2c_adapter *adap,
 static u32 i2c_mux_functionality(struct i2c_adapter *adap)
 {
struct i2c_mux_priv *priv = adap->algo_data;
-   struct i2c_adapter *parent = priv->parent;
+   struct i2c_adapter *parent = priv->muxc->parent;
 
return parent->algo->functionality(parent);
 }
@@ -102,7 +104,20 @@ static unsigned int i2c_mux_parent_classes(struct 
i2c_adapter *parent)
return class;
 }
 
-struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter *parent,
+struct i2c_mux_core *i2c_mux_alloc(struct device *dev, int sizeof_priv)
+{
+   struct i2c_mux_core *muxc;
+
+   muxc = devm_kzalloc(dev, sizeof(*muxc) + sizeof_priv, GFP_KERNEL);
+   if (!muxc)
+   return NULL;
+   if (sizeof_priv)
+   muxc->priv = muxc + 1;
+   return muxc;
+}
+EXPORT_SYMBOL_GPL(i2c_mux_alloc);
+
+struct i2c_adapter *i2c_add_mux_adapter(struct i2c_mux_core *muxc,
struct device *mux_dev,
void *mux_priv, u32 force_nr, u32 chan_id,
unsigned int class,
@@ -111,6 +126,7 @@ struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter 
*parent,
int (*deselect) (struct i2c_adapter *,
 void *, u32))
 {
+   struct i2c_adapter *parent = muxc->parent;
struct i2c_mux_priv *priv;
char symlink_name[20];
int ret;
@@ -120,7 +136,7 @@ struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter 
*parent,
return NULL;
 
/* Set up private adapter data */
-   priv->parent = parent;
+   priv->muxc = muxc;
priv->mux_dev = mux_dev;
priv->mux_priv = 

[PATCH v2 2/8] i2c-mux: move select and deselect ops to i2c_mux_core

[Peter Rosin]

From: Peter Rosin 

The mux select and deselect ops are common to the mux most of the time,
so store the ops in the mux core.

Change the select and deselect op to work in terms of the mux core instead
of the child adapter. No driver uses the child adapter anyway, and if it
is needed in a future mux driver it can be worked out from the channel id.

i2c-arb-gpio-challenge is special in that it needs the mux device pointer
in the select op, so store that device pointer in the mux core as well.
This pointer is going to get further use in later commits.

i2c-mux-pca954x is special since it does not add its deselect op to all
its child adapters, handle this by adding a mask that makes the deselect
op a no-operation for child adapters not wishing to deselect the mux.

Signed-off-by: Peter Rosin 
---
 drivers/i2c/i2c-mux.c  | 31 ++
 drivers/i2c/muxes/i2c-arb-gpio-challenge.c | 18 -
 drivers/i2c/muxes/i2c-mux-gpio.c   | 19 +-
 drivers/i2c/muxes/i2c-mux-pca9541.c| 18 +
 drivers/i2c/muxes/i2c-mux-pca954x.c| 30 +
 drivers/i2c/muxes/i2c-mux-pinctrl.c| 20 ---
 drivers/i2c/muxes/i2c-mux-reg.c| 21 +---
 drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 17 +++-
 drivers/media/dvb-frontends/m88ds3103.c|  8 
 drivers/media/dvb-frontends/rtl2830.c  |  8 
 drivers/media/dvb-frontends/rtl2832.c  | 15 ---
 drivers/media/dvb-frontends/si2168.c   | 13 +++--
 drivers/media/usb/cx231xx/cx231xx-i2c.c| 12 
 drivers/of/unittest.c  |  7 +++
 include/linux/i2c-mux.h| 15 ---
 15 files changed, 118 insertions(+), 134 deletions(-)

diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
index c2163f6b51d5..6c5cb9f7649b 100644
--- a/drivers/i2c/i2c-mux.c
+++ b/drivers/i2c/i2c-mux.c
@@ -32,13 +32,8 @@ struct i2c_mux_priv {
struct i2c_adapter adap;
struct i2c_algorithm algo;
struct i2c_mux_core *muxc;
-
struct device *mux_dev;
-   void *mux_priv;
u32 chan_id;
-
-   int (*select)(struct i2c_adapter *, void *mux_priv, u32 chan_id);
-   int (*deselect)(struct i2c_adapter *, void *mux_priv, u32 chan_id);
 };
 
 static int i2c_mux_master_xfer(struct i2c_adapter *adap,
@@ -51,11 +46,11 @@ static int i2c_mux_master_xfer(struct i2c_adapter *adap,
 
/* Switch to the right mux port and perform the transfer. */
 
-   ret = priv->select(parent, priv->mux_priv, priv->chan_id);
+   ret = muxc->select(muxc, priv->chan_id);
if (ret >= 0)
ret = __i2c_transfer(parent, msgs, num);
-   if (priv->deselect)
-   priv->deselect(parent, priv->mux_priv, priv->chan_id);
+   if (muxc->deselect)
+   muxc->deselect(muxc, priv->chan_id);
 
return ret;
 }
@@ -72,12 +67,12 @@ static int i2c_mux_smbus_xfer(struct i2c_adapter *adap,
 
/* Select the right mux port and perform the transfer. */
 
-   ret = priv->select(parent, priv->mux_priv, priv->chan_id);
+   ret = muxc->select(muxc, priv->chan_id);
if (ret >= 0)
ret = parent->algo->smbus_xfer(parent, addr, flags,
read_write, command, size, data);
-   if (priv->deselect)
-   priv->deselect(parent, priv->mux_priv, priv->chan_id);
+   if (muxc->deselect)
+   muxc->deselect(muxc, priv->chan_id);
 
return ret;
 }
@@ -113,18 +108,15 @@ struct i2c_mux_core *i2c_mux_alloc(struct device *dev, 
int sizeof_priv)
return NULL;
if (sizeof_priv)
muxc->priv = muxc + 1;
+   muxc->dev = dev;
return muxc;
 }
 EXPORT_SYMBOL_GPL(i2c_mux_alloc);
 
 struct i2c_adapter *i2c_add_mux_adapter(struct i2c_mux_core *muxc,
-   struct device *mux_dev,
-   void *mux_priv, u32 force_nr, u32 chan_id,
-   unsigned int class,
-   int (*select) (struct i2c_adapter *,
-  void *, u32),
-   int (*deselect) (struct i2c_adapter *,
-void *, u32))
+   struct device *mux_dev,
+   u32 force_nr, u32 chan_id,
+   unsigned int class)
 {
struct i2c_adapter *parent = muxc->parent;
struct i2c_mux_priv *priv;
@@ -138,10 +130,7 @@ struct i2c_adapter *i2c_add_mux_adapter(struct 
i2c_mux_core *muxc,
/* Set up private adapter data */
priv->muxc = muxc;
priv->mux_dev = mux_dev;
-   priv->mux_priv = mux_priv;
priv->chan_id = chan_id;
-  

[PATCH v2 4/8] i2c-mux: remove the mux dev pointer from the mux per channel data

[Peter Rosin]

From: Peter Rosin 

The dev pointer is readily available in the mux core struct, no point in
keeping multiple copies around.

The patch also fixes a bug in rtl2832, which attached its mux slave
adapter to the device owning the mux parent adapter instead of
attaching it to its own device.

Signed-off-by: Peter Rosin 
---
 drivers/i2c/i2c-mux.c  | 24 
 drivers/i2c/muxes/i2c-arb-gpio-challenge.c |  2 +-
 drivers/i2c/muxes/i2c-mux-gpio.c   |  3 +--
 drivers/i2c/muxes/i2c-mux-pca9541.c|  2 +-
 drivers/i2c/muxes/i2c-mux-pca954x.c|  3 +--
 drivers/i2c/muxes/i2c-mux-pinctrl.c|  3 +--
 drivers/i2c/muxes/i2c-mux-reg.c|  3 +--
 drivers/iio/imu/inv_mpu6050/inv_mpu_core.c |  2 +-
 drivers/media/dvb-frontends/m88ds3103.c|  2 +-
 drivers/media/dvb-frontends/rtl2830.c  |  2 +-
 drivers/media/dvb-frontends/rtl2832.c  |  2 +-
 drivers/media/dvb-frontends/si2168.c   |  2 +-
 drivers/media/usb/cx231xx/cx231xx-i2c.c|  3 ---
 drivers/of/unittest.c  |  2 +-
 include/linux/i2c-mux.h|  1 -
 15 files changed, 24 insertions(+), 32 deletions(-)

diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
index 7ba0308537a8..5c1088079231 100644
--- a/drivers/i2c/i2c-mux.c
+++ b/drivers/i2c/i2c-mux.c
@@ -32,7 +32,6 @@ struct i2c_mux_priv {
struct i2c_adapter adap;
struct i2c_algorithm algo;
struct i2c_mux_core *muxc;
-   struct device *mux_dev;
u32 chan_id;
 };
 
@@ -137,7 +136,6 @@ struct i2c_mux_core *i2c_mux_alloc(struct device *dev, int 
sizeof_priv)
 EXPORT_SYMBOL_GPL(i2c_mux_alloc);
 
 int i2c_add_mux_adapter(struct i2c_mux_core *muxc,
-   struct device *mux_dev,
u32 force_nr, u32 chan_id,
unsigned int class)
 {
@@ -162,7 +160,6 @@ int i2c_add_mux_adapter(struct i2c_mux_core *muxc,
 
/* Set up private adapter data */
priv->muxc = muxc;
-   priv->mux_dev = mux_dev;
priv->chan_id = chan_id;
 
/* Need to do algo dynamically because we don't know ahead
@@ -197,11 +194,11 @@ int i2c_add_mux_adapter(struct i2c_mux_core *muxc,
 * Try to populate the mux adapter's of_node, expands to
 * nothing if !CONFIG_OF.
 */
-   if (mux_dev->of_node) {
+   if (muxc->dev->of_node) {
struct device_node *child;
u32 reg;
 
-   for_each_child_of_node(mux_dev->of_node, child) {
+   for_each_child_of_node(muxc->dev->of_node, child) {
ret = of_property_read_u32(child, "reg", );
if (ret)
continue;
@@ -215,8 +212,9 @@ int i2c_add_mux_adapter(struct i2c_mux_core *muxc,
/*
 * Associate the mux channel with an ACPI node.
 */
-   if (has_acpi_companion(mux_dev))
-   acpi_preset_companion(>adap.dev, ACPI_COMPANION(mux_dev),
+   if (has_acpi_companion(muxc->dev))
+   acpi_preset_companion(>adap.dev,
+ ACPI_COMPANION(muxc->dev),
  chan_id);
 
if (force_nr) {
@@ -233,12 +231,14 @@ int i2c_add_mux_adapter(struct i2c_mux_core *muxc,
return ret;
}
 
-   WARN(sysfs_create_link(>adap.dev.kobj, _dev->kobj, 
"mux_device"),
-  "can't create symlink to mux device\n");
+   WARN(sysfs_create_link(>adap.dev.kobj, >dev->kobj,
+  "mux_device"),
+"can't create symlink to mux device\n");
 
snprintf(symlink_name, sizeof(symlink_name), "channel-%u", chan_id);
-   WARN(sysfs_create_link(_dev->kobj, >adap.dev.kobj, 
symlink_name),
-  "can't create symlink for channel %u\n", 
chan_id);
+   WARN(sysfs_create_link(>dev->kobj, >adap.dev.kobj,
+  symlink_name),
+"can't create symlink for channel %u\n", chan_id);
dev_info(>dev, "Added multiplexed i2c bus %d\n",
 i2c_adapter_id(>adap));
 
@@ -259,7 +259,7 @@ void i2c_del_mux_adapters(struct i2c_mux_core *muxc)
 
snprintf(symlink_name, sizeof(symlink_name),
 "channel-%u", priv->chan_id);
-   sysfs_remove_link(>mux_dev->kobj, symlink_name);
+   sysfs_remove_link(>dev->kobj, symlink_name);
 
sysfs_remove_link(>adap.dev.kobj, "mux_device");
i2c_del_adapter(adap);
diff --git a/drivers/i2c/muxes/i2c-arb-gpio-challenge.c 
b/drivers/i2c/muxes/i2c-arb-gpio-challenge.c
index e0558e8a0e74..c2bc18c7921f 100644
--- a/drivers/i2c/muxes/i2c-arb-gpio-challenge.c
+++ b/drivers/i2c/muxes/i2c-arb-gpio-challenge.c
@@ -204,7 +204,7 @@ static int i2c_arbitrator_probe(struct platform_device 
*pdev)
}
 
/* Actually add the mux adapter */
- 

[PATCH v2 3/8] i2c-mux: move the slave side adapter management to i2c_mux_core

[Peter Rosin]

From: Peter Rosin 

All muxes have slave side adapters, many have some arbitrary number of
them. Handle this in the mux core, so that drivers are simplified.

Add i2c_mux_reserve_adapter that can be used when it is known in advance
how many child adapters that is to be added. This avoids reallocating
memory.

Drop i2c_del_mux_adapter and replace it with i2c_del_mux_adapters, since
no mux driver is dynamically deleting individual child adapters anyway.

Signed-off-by: Peter Rosin 
---
 drivers/i2c/i2c-mux.c| 71 ++--
 drivers/i2c/muxes/i2c-arb-gpio-challenge.c   | 10 ++--
 drivers/i2c/muxes/i2c-mux-gpio.c | 23 +++--
 drivers/i2c/muxes/i2c-mux-pca9541.c  | 13 ++---
 drivers/i2c/muxes/i2c-mux-pca954x.c  | 26 --
 drivers/i2c/muxes/i2c-mux-pinctrl.c  | 27 +++
 drivers/i2c/muxes/i2c-mux-reg.c  | 28 ---
 drivers/iio/imu/inv_mpu6050/inv_mpu_core.c   | 12 ++---
 drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h|  1 -
 drivers/media/dvb-frontends/m88ds3103.c  | 11 ++---
 drivers/media/dvb-frontends/m88ds3103_priv.h |  1 -
 drivers/media/dvb-frontends/rtl2830.c| 10 ++--
 drivers/media/dvb-frontends/rtl2830_priv.h   |  1 -
 drivers/media/dvb-frontends/rtl2832.c| 11 ++---
 drivers/media/dvb-frontends/rtl2832_priv.h   |  1 -
 drivers/media/dvb-frontends/si2168.c | 10 ++--
 drivers/media/dvb-frontends/si2168_priv.h|  1 -
 drivers/media/usb/cx231xx/cx231xx-core.c |  3 +-
 drivers/media/usb/cx231xx/cx231xx-i2c.c  | 26 +-
 drivers/media/usb/cx231xx/cx231xx.h  |  2 +-
 drivers/of/unittest.c| 28 ---
 include/linux/i2c-mux.h  | 15 --
 22 files changed, 149 insertions(+), 182 deletions(-)

diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
index 6c5cb9f7649b..7ba0308537a8 100644
--- a/drivers/i2c/i2c-mux.c
+++ b/drivers/i2c/i2c-mux.c
@@ -99,6 +99,29 @@ static unsigned int i2c_mux_parent_classes(struct 
i2c_adapter *parent)
return class;
 }
 
+int i2c_mux_reserve_adapters(struct i2c_mux_core *muxc, int adapters)
+{
+   struct i2c_adapter **adapter;
+
+   if (adapters <= muxc->max_adapters)
+   return 0;
+
+   adapter = devm_kmalloc_array(muxc->dev,
+adapters, sizeof(*adapter),
+GFP_KERNEL);
+   if (!adapter)
+   return -ENOMEM;
+
+   memcpy(adapter, muxc->adapter,
+  muxc->max_adapters * sizeof(*adapter));
+
+   devm_kfree(muxc->dev, muxc->adapter);
+   muxc->adapter = adapter;
+   muxc->max_adapters = adapters;
+   return 0;
+}
+EXPORT_SYMBOL_GPL(i2c_mux_reserve_adapters);
+
 struct i2c_mux_core *i2c_mux_alloc(struct device *dev, int sizeof_priv)
 {
struct i2c_mux_core *muxc;
@@ -113,19 +136,29 @@ struct i2c_mux_core *i2c_mux_alloc(struct device *dev, 
int sizeof_priv)
 }
 EXPORT_SYMBOL_GPL(i2c_mux_alloc);
 
-struct i2c_adapter *i2c_add_mux_adapter(struct i2c_mux_core *muxc,
-   struct device *mux_dev,
-   u32 force_nr, u32 chan_id,
-   unsigned int class)
+int i2c_add_mux_adapter(struct i2c_mux_core *muxc,
+   struct device *mux_dev,
+   u32 force_nr, u32 chan_id,
+   unsigned int class)
 {
struct i2c_adapter *parent = muxc->parent;
struct i2c_mux_priv *priv;
char symlink_name[20];
int ret;
 
+   if (muxc->adapters >= muxc->max_adapters) {
+   int new_max = 2 * muxc->max_adapters;
+
+   if (!new_max)
+   new_max = 1;
+   ret = i2c_mux_reserve_adapters(muxc, new_max);
+   if (ret)
+   return ret;
+   }
+
priv = kzalloc(sizeof(struct i2c_mux_priv), GFP_KERNEL);
if (!priv)
-   return NULL;
+   return -ENOMEM;
 
/* Set up private adapter data */
priv->muxc = muxc;
@@ -197,7 +230,7 @@ struct i2c_adapter *i2c_add_mux_adapter(struct i2c_mux_core 
*muxc,
"failed to add mux-adapter (error=%d)\n",
ret);
kfree(priv);
-   return NULL;
+   return ret;
}
 
WARN(sysfs_create_link(>adap.dev.kobj, _dev->kobj, 
"mux_device"),
@@ -209,23 +242,31 @@ struct i2c_adapter *i2c_add_mux_adapter(struct 
i2c_mux_core *muxc,
dev_info(>dev, "Added multiplexed i2c bus %d\n",
 i2c_adapter_id(>adap));
 
-   return >adap;
+   muxc->adapter[muxc->adapters++] = >adap;
+   return 0;
 }
 EXPORT_SYMBOL_GPL(i2c_add_mux_adapter);
 
-void i2c_del_mux_adapter(struct i2c_adapter *adap)
+void i2c_del_mux_adapters(struct i2c_mux_core *muxc)
 {
-   struct 

Re: [PATCH v2 3/8] i2c-mux: move the slave side adapter management to i2c_mux_core

[kbuild test robot]

Hi Peter,

[auto build test ERROR on wsa/i2c/for-next]
[also build test ERROR on v4.4-rc8 next-20160105]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improving the system]

url:
https://github.com/0day-ci/linux/commits/Peter-Rosin/i2c-mux-cleanup-and-locking-update/20160106-000205
base:   https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux i2c/for-next
config: i386-randconfig-s1-201601 (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c: In function 
'inv_mpu_acpi_create_mux_client':
>> drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c:185:37: error: 'struct 
>> inv_mpu6050_state' has no member named 'mux_adapter'
  st->mux_client = i2c_new_device(st->mux_adapter, );
^

vim +185 drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c

a35c5d1a Srinivas Pandruvada 2015-01-30  179
*name = '\0';
a35c5d1a Srinivas Pandruvada 2015-01-30  180
strlcat(info.type, "-client",
a35c5d1a Srinivas Pandruvada 2015-01-30  181
sizeof(info.type));
a35c5d1a Srinivas Pandruvada 2015-01-30  182} else
a35c5d1a Srinivas Pandruvada 2015-01-30  183return 
0; /* no secondary addr, which is OK */
a35c5d1a Srinivas Pandruvada 2015-01-30  184}
a35c5d1a Srinivas Pandruvada 2015-01-30 @185st->mux_client = 
i2c_new_device(st->mux_adapter, );
a35c5d1a Srinivas Pandruvada 2015-01-30  186if (!st->mux_client)
a35c5d1a Srinivas Pandruvada 2015-01-30  187return -ENODEV;
a35c5d1a Srinivas Pandruvada 2015-01-30  188  

:: The code at line 185 was first introduced by commit
:: a35c5d1aa96aa6cc70e91786cbe9be4db23f8f4a iio: imu: inv_mpu6050: Create 
mux clients for ACPI

:: TO: Srinivas Pandruvada <srinivas.pandruv...@linux.intel.com>
:: CC: Jonathan Cameron <ji...@kernel.org>

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: Binary data

[PATCH v2 6/8] i2c: allow adapter drivers to override the adapter locking

[Peter Rosin]

From: Peter Rosin 

Add i2c_lock_bus() and i2c_unlock_bus(), which call the new lock_bus and
unlock_bus ops in the adapter. These funcs/ops take an additional flags
argument that indicates for what purpose the adapter is locked.

There are two flags, I2C_LOCK_ADAPTER and I2C_LOCK_SEGMENT, but they are
both implemented the same. For now. Locking the adapter means that the
whole bus is locked, locking the segment means that only the current bus
segment is locked (i.e. i2c traffic on the parent side of mux is still
allowed even if the child side of the mux is locked.

Also support a trylock_bus op (but no function to call it, as it is not
expected to be needed outside of the i2c core).

Implement i2c_lock_adapter/i2c_unlock_adapter in terms of the new locking
scheme (i.e. lock with the I2C_LOCK_ADAPTER flag).

Annotate some of the locking with explicit I2C_LOCK_SEGMENT flags.

Signed-off-by: Peter Rosin 
---
 drivers/i2c/i2c-core.c | 40 ++--
 include/linux/i2c.h| 28 ++--
 2 files changed, 48 insertions(+), 20 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index ba8eb087f224..34a7748b4652 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -958,10 +958,10 @@ static int i2c_check_addr_busy(struct i2c_adapter 
*adapter, int addr)
 }
 
 /**
- * i2c_lock_adapter - Get exclusive access to an I2C bus segment
+ * i2c_adapter_lock_bus - Get exclusive access to an I2C bus segment
  * @adapter: Target I2C bus segment
  */
-void i2c_lock_adapter(struct i2c_adapter *adapter)
+static void i2c_adapter_lock_bus(struct i2c_adapter *adapter, int flags)
 {
struct i2c_adapter *parent = i2c_parent_is_i2c_adapter(adapter);
 
@@ -970,27 +970,26 @@ void i2c_lock_adapter(struct i2c_adapter *adapter)
else
rt_mutex_lock(>bus_lock);
 }
-EXPORT_SYMBOL_GPL(i2c_lock_adapter);
 
 /**
- * i2c_trylock_adapter - Try to get exclusive access to an I2C bus segment
+ * i2c_adapter_trylock_bus - Try to get exclusive access to an I2C bus segment
  * @adapter: Target I2C bus segment
  */
-static int i2c_trylock_adapter(struct i2c_adapter *adapter)
+static int i2c_adapter_trylock_bus(struct i2c_adapter *adapter, int flags)
 {
struct i2c_adapter *parent = i2c_parent_is_i2c_adapter(adapter);
 
if (parent)
-   return i2c_trylock_adapter(parent);
+   return parent->trylock_bus(parent, flags);
else
return rt_mutex_trylock(>bus_lock);
 }
 
 /**
- * i2c_unlock_adapter - Release exclusive access to an I2C bus segment
+ * i2c_adapter_unlock_bus - Release exclusive access to an I2C bus segment
  * @adapter: Target I2C bus segment
  */
-void i2c_unlock_adapter(struct i2c_adapter *adapter)
+static void i2c_adapter_unlock_bus(struct i2c_adapter *adapter, int flags)
 {
struct i2c_adapter *parent = i2c_parent_is_i2c_adapter(adapter);
 
@@ -999,7 +998,6 @@ void i2c_unlock_adapter(struct i2c_adapter *adapter)
else
rt_mutex_unlock(>bus_lock);
 }
-EXPORT_SYMBOL_GPL(i2c_unlock_adapter);
 
 static void i2c_dev_set_name(struct i2c_adapter *adap,
 struct i2c_client *client)
@@ -1545,6 +1543,12 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
return -EINVAL;
}
 
+   if (!adap->lock_bus) {
+   adap->lock_bus = i2c_adapter_lock_bus;
+   adap->trylock_bus = i2c_adapter_trylock_bus;
+   adap->unlock_bus = i2c_adapter_unlock_bus;
+   }
+
rt_mutex_init(>bus_lock);
mutex_init(>userspace_clients_lock);
INIT_LIST_HEAD(>userspace_clients);
@@ -2254,16 +2258,16 @@ int i2c_transfer(struct i2c_adapter *adap, struct 
i2c_msg *msgs, int num)
 #endif
 
if (in_atomic() || irqs_disabled()) {
-   ret = i2c_trylock_adapter(adap);
+   ret = adap->trylock_bus(adap, I2C_LOCK_SEGMENT);
if (!ret)
/* I2C activity is ongoing. */
return -EAGAIN;
} else {
-   i2c_lock_adapter(adap);
+   i2c_lock_bus(adap, I2C_LOCK_SEGMENT);
}
 
ret = __i2c_transfer(adap, msgs, num);
-   i2c_unlock_adapter(adap);
+   i2c_unlock_bus(adap, I2C_LOCK_SEGMENT);
 
return ret;
} else {
@@ -3038,7 +3042,7 @@ s32 i2c_smbus_xfer(struct i2c_adapter *adapter, u16 addr, 
unsigned short flags,
flags &= I2C_M_TEN | I2C_CLIENT_PEC | I2C_CLIENT_SCCB;
 
if (adapter->algo->smbus_xfer) {
-   i2c_lock_adapter(adapter);
+   i2c_lock_bus(adapter, I2C_LOCK_SEGMENT);
 
/* Retry automatically on arbitration loss */
orig_jiffies = jiffies;
@@ -3052,7 +3056,7 @@ s32 i2c_smbus_xfer(struct i2c_adapter 

[PATCH v2 7/8] i2c: muxes always lock the parent adapter

[Peter Rosin]

From: Peter Rosin 

Instead of checking for i2c parent adapters for every lock/unlock, simply
override the locking for muxes to always lock/unlock the parent adapter
directly.

Signed-off-by: Peter Rosin 
---
 drivers/i2c/i2c-core.c | 21 +++--
 drivers/i2c/i2c-mux.c  | 27 +++
 2 files changed, 30 insertions(+), 18 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 34a7748b4652..4683777f81ca 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -963,12 +963,7 @@ static int i2c_check_addr_busy(struct i2c_adapter 
*adapter, int addr)
  */
 static void i2c_adapter_lock_bus(struct i2c_adapter *adapter, int flags)
 {
-   struct i2c_adapter *parent = i2c_parent_is_i2c_adapter(adapter);
-
-   if (parent)
-   i2c_lock_adapter(parent);
-   else
-   rt_mutex_lock(>bus_lock);
+   rt_mutex_lock(>bus_lock);
 }
 
 /**
@@ -977,12 +972,7 @@ static void i2c_adapter_lock_bus(struct i2c_adapter 
*adapter, int flags)
  */
 static int i2c_adapter_trylock_bus(struct i2c_adapter *adapter, int flags)
 {
-   struct i2c_adapter *parent = i2c_parent_is_i2c_adapter(adapter);
-
-   if (parent)
-   return parent->trylock_bus(parent, flags);
-   else
-   return rt_mutex_trylock(>bus_lock);
+   return rt_mutex_trylock(>bus_lock);
 }
 
 /**
@@ -991,12 +981,7 @@ static int i2c_adapter_trylock_bus(struct i2c_adapter 
*adapter, int flags)
  */
 static void i2c_adapter_unlock_bus(struct i2c_adapter *adapter, int flags)
 {
-   struct i2c_adapter *parent = i2c_parent_is_i2c_adapter(adapter);
-
-   if (parent)
-   i2c_unlock_adapter(parent);
-   else
-   rt_mutex_unlock(>bus_lock);
+   rt_mutex_unlock(>bus_lock);
 }
 
 static void i2c_dev_set_name(struct i2c_adapter *adap,
diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
index 5c1088079231..dd8a790cb4cc 100644
--- a/drivers/i2c/i2c-mux.c
+++ b/drivers/i2c/i2c-mux.c
@@ -98,6 +98,30 @@ static unsigned int i2c_mux_parent_classes(struct 
i2c_adapter *parent)
return class;
 }
 
+static void i2c_parent_lock_bus(struct i2c_adapter *adapter, int flags)
+{
+   struct i2c_mux_priv *priv = adapter->algo_data;
+   struct i2c_adapter *parent = priv->muxc->parent;
+
+   parent->lock_bus(parent, flags);
+}
+
+static int i2c_parent_trylock_bus(struct i2c_adapter *adapter, int flags)
+{
+   struct i2c_mux_priv *priv = adapter->algo_data;
+   struct i2c_adapter *parent = priv->muxc->parent;
+
+   return parent->trylock_bus(parent, flags);
+}
+
+static void i2c_parent_unlock_bus(struct i2c_adapter *adapter, int flags)
+{
+   struct i2c_mux_priv *priv = adapter->algo_data;
+   struct i2c_adapter *parent = priv->muxc->parent;
+
+   parent->unlock_bus(parent, flags);
+}
+
 int i2c_mux_reserve_adapters(struct i2c_mux_core *muxc, int adapters)
 {
struct i2c_adapter **adapter;
@@ -181,6 +205,9 @@ int i2c_add_mux_adapter(struct i2c_mux_core *muxc,
priv->adap.retries = parent->retries;
priv->adap.timeout = parent->timeout;
priv->adap.quirks = parent->quirks;
+   priv->adap.lock_bus = i2c_parent_lock_bus;
+   priv->adap.trylock_bus = i2c_parent_trylock_bus;
+   priv->adap.unlock_bus = i2c_parent_unlock_bus;
 
/* Sanity check on class */
if (i2c_mux_parent_classes(parent) & class)
-- 
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 1/8] i2c-mux: add common core data for every mux instance

[Guenter Roeck]

On 01/05/2016 07:57 AM, Peter Rosin wrote:

From: Peter Rosin 

The initial core mux structure starts off small with only the parent
adapter pointer, which all muxes have, and a priv pointer for mux
driver private data.

Add i2c_mux_alloc function to unify the creation of a mux.

Where appropriate, pass around the mux core structure instead of the
parent adapter or the driver private data.

Remove the parent adapter pointer from the driver private data for all
mux drivers.

Signed-off-by: Peter Rosin 
---
  drivers/i2c/i2c-mux.c| 28 +-
  drivers/i2c/muxes/i2c-arb-gpio-challenge.c   | 24 +--
  drivers/i2c/muxes/i2c-mux-gpio.c | 20 
  drivers/i2c/muxes/i2c-mux-pca9541.c  | 35 ++--
  drivers/i2c/muxes/i2c-mux-pca954x.c  | 19 ++-
  drivers/i2c/muxes/i2c-mux-pinctrl.c  | 23 +-
  drivers/i2c/muxes/i2c-mux-reg.c  | 23 ++
  drivers/iio/imu/inv_mpu6050/inv_mpu_core.c   | 10 +++-
  drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h|  1 +
  drivers/media/dvb-frontends/m88ds3103.c  | 10 +++-
  drivers/media/dvb-frontends/m88ds3103_priv.h |  1 +
  drivers/media/dvb-frontends/rtl2830.c| 10 +++-
  drivers/media/dvb-frontends/rtl2830_priv.h   |  1 +
  drivers/media/dvb-frontends/rtl2832.c| 10 +++-
  drivers/media/dvb-frontends/rtl2832_priv.h   |  1 +
  drivers/media/dvb-frontends/si2168.c | 10 +++-
  drivers/media/dvb-frontends/si2168_priv.h|  1 +
  drivers/media/usb/cx231xx/cx231xx-core.c |  3 +++
  drivers/media/usb/cx231xx/cx231xx-i2c.c  | 13 +--
  drivers/media/usb/cx231xx/cx231xx.h  |  2 ++
  drivers/of/unittest.c| 16 +++--
  include/linux/i2c-mux.h  | 14 ++-
  22 files changed, 187 insertions(+), 88 deletions(-)

diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
index 00fc5b1c7b66..c2163f6b51d5 100644
--- a/drivers/i2c/i2c-mux.c
+++ b/drivers/i2c/i2c-mux.c
@@ -31,8 +31,8 @@
  struct i2c_mux_priv {
struct i2c_adapter adap;
struct i2c_algorithm algo;
+   struct i2c_mux_core *muxc;

-   struct i2c_adapter *parent;
struct device *mux_dev;
void *mux_priv;
u32 chan_id;
@@ -45,7 +45,8 @@ static int i2c_mux_master_xfer(struct i2c_adapter *adap,
   struct i2c_msg msgs[], int num)
  {
struct i2c_mux_priv *priv = adap->algo_data;
-   struct i2c_adapter *parent = priv->parent;
+   struct i2c_mux_core *muxc = priv->muxc;
+   struct i2c_adapter *parent = muxc->parent;
int ret;

/* Switch to the right mux port and perform the transfer. */
@@ -65,7 +66,8 @@ static int i2c_mux_smbus_xfer(struct i2c_adapter *adap,
  int size, union i2c_smbus_data *data)
  {
struct i2c_mux_priv *priv = adap->algo_data;
-   struct i2c_adapter *parent = priv->parent;
+   struct i2c_mux_core *muxc = priv->muxc;
+   struct i2c_adapter *parent = muxc->parent;
int ret;

/* Select the right mux port and perform the transfer. */
@@ -84,7 +86,7 @@ static int i2c_mux_smbus_xfer(struct i2c_adapter *adap,
  static u32 i2c_mux_functionality(struct i2c_adapter *adap)
  {
struct i2c_mux_priv *priv = adap->algo_data;
-   struct i2c_adapter *parent = priv->parent;
+   struct i2c_adapter *parent = priv->muxc->parent;

return parent->algo->functionality(parent);
  }
@@ -102,7 +104,20 @@ static unsigned int i2c_mux_parent_classes(struct 
i2c_adapter *parent)
return class;
  }

-struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter *parent,
+struct i2c_mux_core *i2c_mux_alloc(struct device *dev, int sizeof_priv)
+{
+   struct i2c_mux_core *muxc;
+
+   muxc = devm_kzalloc(dev, sizeof(*muxc) + sizeof_priv, GFP_KERNEL);
+   if (!muxc)
+   return NULL;
+   if (sizeof_priv)
+   muxc->priv = muxc + 1;
+   return muxc;
+}
+EXPORT_SYMBOL_GPL(i2c_mux_alloc);
+
+struct i2c_adapter *i2c_add_mux_adapter(struct i2c_mux_core *muxc,
struct device *mux_dev,
void *mux_priv, u32 force_nr, u32 chan_id,
unsigned int class,
@@ -111,6 +126,7 @@ struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter 
*parent,
int (*deselect) (struct i2c_adapter *,
 void *, u32))
  {
+   struct i2c_adapter *parent = muxc->parent;
struct i2c_mux_priv *priv;
char symlink_name[20];
int ret;
@@ -120,7 +136,7 @@ struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter 
*parent,
return NULL;

/* Set up private adapter data */
-   priv->parent = parent;
+   priv->muxc 

[PATCH v2 8/8] i2c-mux: relax locking of the top i2c adapter during i2c controlled muxing

[Peter Rosin]

From: Peter Rosin 

With a i2c topology like the following

   GPIO ---|  -- BAT1
|  v /
   I2C  -+--+ MUX
 |   \
   EEPROM -- BAT2

there is a locking problem with the GPIO controller since it is a client
on the same i2c bus that it muxes. Transfers to the mux clients (e.g. BAT1)
will lock the whole i2c bus prior to attempting to switch the mux to the
correct i2c segment. In the above case, the GPIO device is an I/O expander
with an i2c interface, and since the GPIO subsystem knows nothing (and
rightfully so) about the lockless needs of the i2c mux code, this results
in a deadlock when the GPIO driver issues i2c transfers to modify the
mux.

So, observing that while it is needed to have the i2c bus locked during the
actual MUX update in order to avoid random garbage on the slave side, it
is not strictly a must to have it locked over the whole sequence of a full
select-transfer-deselect mux client operation. The mux itself needs to be
locked, so transfers to clients behind the mux are serialized, and the mux
needs to be stable during all i2c traffic (otherwise individual mux slave
segments might see garbage, or worse).

Add devive tree properties (bool named i2c-controlled) to i2c-mux-gpio and
i2c-mux-pinctrl that asserts that the the gpio/pinctrl is controlled via
the same i2c bus that it muxes.

Modify the i2c mux locking so that muxes that are "i2c-controlled" locks
the mux instead of the whole i2c bus when there is a transfer to the slave
side of the mux. This lock serializes transfers to the slave side of the
mux.

Modify the select-transfer-deselect code for "i2c-controlled" muxes so
that each of the select-transfer-deselect ops locks the mux parent
adapter individually.

Signed-off-by: Peter Rosin 
---
 .../devicetree/bindings/i2c/i2c-mux-gpio.txt   |   2 +
 .../devicetree/bindings/i2c/i2c-mux-pinctrl.txt|   4 +
 drivers/i2c/i2c-mux.c  | 109 +++--
 drivers/i2c/muxes/i2c-mux-gpio.c   |   3 +
 drivers/i2c/muxes/i2c-mux-pinctrl.c|   3 +
 include/linux/i2c-mux-gpio.h   |   2 +
 include/linux/i2c-mux-pinctrl.h|   2 +
 include/linux/i2c-mux.h|   2 +
 8 files changed, 120 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-gpio.txt 
b/Documentation/devicetree/bindings/i2c/i2c-mux-gpio.txt
index 66709a825541..17670b997d81 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mux-gpio.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-mux-gpio.txt
@@ -28,6 +28,8 @@ Required properties:
 Optional properties:
 - idle-state: value to set the muxer to when idle. When no value is
   given, it defaults to the last value used.
+- i2c-controlled: The muxed I2C bus is also used to control all the gpios
+  used for muxing.
 
 For each i2c child node, an I2C child bus will be created. They will
 be numbered based on their order in the device tree.
diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pinctrl.txt 
b/Documentation/devicetree/bindings/i2c/i2c-mux-pinctrl.txt
index ae8af1694e95..8374a1f7a709 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mux-pinctrl.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pinctrl.txt
@@ -23,6 +23,10 @@ Required properties:
 - i2c-parent: The phandle of the I2C bus that this multiplexer's master-side
   port is connected to.
 
+Optional properties:
+- i2c-controlled: The muxed I2C bus is also used to control all the pinctrl
+  pins used for muxing.
+
 Also required are:
 
 * Standard pinctrl properties that specify the pin mux state for each child
diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
index dd8a790cb4cc..c4d4b14a5399 100644
--- a/drivers/i2c/i2c-mux.c
+++ b/drivers/i2c/i2c-mux.c
@@ -54,6 +54,25 @@ static int i2c_mux_master_xfer(struct i2c_adapter *adap,
return ret;
 }
 
+static int __i2c_mux_master_xfer(struct i2c_adapter *adap,
+struct i2c_msg msgs[], int num)
+{
+   struct i2c_mux_priv *priv = adap->algo_data;
+   struct i2c_mux_core *muxc = priv->muxc;
+   struct i2c_adapter *parent = muxc->parent;
+   int ret;
+
+   /* Switch to the right mux port and perform the transfer. */
+
+   ret = muxc->select(muxc, priv->chan_id);
+   if (ret >= 0)
+   ret = i2c_transfer(parent, msgs, num);
+   if (muxc->deselect)
+   muxc->deselect(muxc, priv->chan_id);
+
+   return ret;
+}
+
 static int i2c_mux_smbus_xfer(struct i2c_adapter *adap,
  u16 addr, unsigned short flags,
  char read_write, u8 command,
@@ -76,6 +95,28 @@ static int i2c_mux_smbus_xfer(struct i2c_adapter *adap,
return ret;
 }
 
+static int __i2c_mux_smbus_xfer(struct i2c_adapter 

[PATCH v2 5/8] i2c-mux: pinctrl: get rid of the driver private struct device pointer

[Peter Rosin]

From: Peter Rosin 

There is a copy of the device pointer in the mux core.

Signed-off-by: Peter Rosin 
---
 drivers/i2c/muxes/i2c-mux-pinctrl.c | 23 ---
 1 file changed, 8 insertions(+), 15 deletions(-)

diff --git a/drivers/i2c/muxes/i2c-mux-pinctrl.c 
b/drivers/i2c/muxes/i2c-mux-pinctrl.c
index 0dc912898813..38129850cbe4 100644
--- a/drivers/i2c/muxes/i2c-mux-pinctrl.c
+++ b/drivers/i2c/muxes/i2c-mux-pinctrl.c
@@ -26,7 +26,6 @@
 #include 
 
 struct i2c_mux_pinctrl {
-   struct device *dev;
struct i2c_mux_pinctrl_platform_data *pdata;
struct pinctrl *pinctrl;
struct pinctrl_state **states;
@@ -51,6 +50,7 @@ static int i2c_mux_pinctrl_deselect(struct i2c_mux_core 
*muxc, u32 chan)
 static int i2c_mux_pinctrl_parse_dt(struct i2c_mux_pinctrl *mux,
struct platform_device *pdev)
 {
+   struct i2c_mux_core *muxc = platform_get_drvdata(pdev);
struct device_node *np = pdev->dev.of_node;
int num_names, i, ret;
struct device_node *adapter_np;
@@ -60,15 +60,12 @@ static int i2c_mux_pinctrl_parse_dt(struct i2c_mux_pinctrl 
*mux,
return 0;
 
mux->pdata = devm_kzalloc(>dev, sizeof(*mux->pdata), GFP_KERNEL);
-   if (!mux->pdata) {
-   dev_err(mux->dev,
-   "Cannot allocate i2c_mux_pinctrl_platform_data\n");
+   if (!mux->pdata)
return -ENOMEM;
-   }
 
num_names = of_property_count_strings(np, "pinctrl-names");
if (num_names < 0) {
-   dev_err(mux->dev, "Cannot parse pinctrl-names: %d\n",
+   dev_err(muxc->dev, "Cannot parse pinctrl-names: %d\n",
num_names);
return num_names;
}
@@ -76,23 +73,21 @@ static int i2c_mux_pinctrl_parse_dt(struct i2c_mux_pinctrl 
*mux,
mux->pdata->pinctrl_states = devm_kzalloc(>dev,
sizeof(*mux->pdata->pinctrl_states) * num_names,
GFP_KERNEL);
-   if (!mux->pdata->pinctrl_states) {
-   dev_err(mux->dev, "Cannot allocate pinctrl_states\n");
+   if (!mux->pdata->pinctrl_states)
return -ENOMEM;
-   }
 
for (i = 0; i < num_names; i++) {
ret = of_property_read_string_index(np, "pinctrl-names", i,
>pdata->pinctrl_states[mux->pdata->bus_count]);
if (ret < 0) {
-   dev_err(mux->dev, "Cannot parse pinctrl-names: %d\n",
+   dev_err(muxc->dev, "Cannot parse pinctrl-names: %d\n",
ret);
return ret;
}
if (!strcmp(mux->pdata->pinctrl_states[mux->pdata->bus_count],
"idle")) {
if (i != num_names - 1) {
-   dev_err(mux->dev, "idle state must be last\n");
+   dev_err(muxc->dev, "idle state must be last\n");
return -EINVAL;
}
mux->pdata->pinctrl_state_idle = "idle";
@@ -103,13 +98,13 @@ static int i2c_mux_pinctrl_parse_dt(struct i2c_mux_pinctrl 
*mux,
 
adapter_np = of_parse_phandle(np, "i2c-parent", 0);
if (!adapter_np) {
-   dev_err(mux->dev, "Cannot parse i2c-parent\n");
+   dev_err(muxc->dev, "Cannot parse i2c-parent\n");
return -ENODEV;
}
adapter = of_find_i2c_adapter_by_node(adapter_np);
of_node_put(adapter_np);
if (!adapter) {
-   dev_err(mux->dev, "Cannot find parent bus\n");
+   dev_err(muxc->dev, "Cannot find parent bus\n");
return -EPROBE_DEFER;
}
mux->pdata->parent_bus_num = i2c_adapter_id(adapter);
@@ -139,8 +134,6 @@ static int i2c_mux_pinctrl_probe(struct platform_device 
*pdev)
mux = i2c_mux_priv(muxc);
platform_set_drvdata(pdev, muxc);
 
-   mux->dev = >dev;
-
mux->pdata = dev_get_platdata(>dev);
if (!mux->pdata) {
ret = i2c_mux_pinctrl_parse_dt(mux, pdev);
-- 
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/8] i2c mux cleanup and locking update

[Peter Rosin]

From: Peter Rosin 

Hi!

I have a pair of boards with this i2c topology:

   GPIO ---|  -- BAT1
|  v /
   I2C  -+--B---+ MUX
 |   \
   EEPROM -- BAT2

(B denotes the boundary between the boards)

The problem with this is that the GPIO controller sits on the same i2c bus
that it MUXes. For pca954x devices this is worked around by using unlocked
transfers when updating the MUX. I have no such luck as the GPIO is a general
purpose IO expander and the MUX is just a random bidirectional MUX, unaware
of the fact that it is muxing an i2c bus, and extending unlocked transfers
into the GPIO subsystem is too ugly to even think about. But the general hw
approach is sane in my opinion, with the number of connections between the
two boards minimized. To put is plainly, I need support for it.

So, I observe that while it is needed to have the i2c bus locked during the
actual MUX update in order to avoid random garbage on the slave side, it
is not strictly a must to have it locked over the whole sequence of a full
select-transfer-deselect operation. The MUX itself needs to be locked, so
transfers to clients behind the mux are serialized, and the MUX needs to be
stable during all i2c traffic (otherwise individual mux slave segments
might see garbage).

This series accomplishes this by adding a dt property to i2c-mux-gpio and
i2c-mux-pinctrl that can be used to state that the mux is updated by means
of the muxed master bus, and that the select-transfer-deselect operations
should be locked individually. When this holds, the i2c bus *is* locked
during muxing, since the muxing happens as part of i2c transfers. This
is true even if the MUX is updated with several transfers to the GPIO (at
least as long as *all* MUX changes are using the i2s master bus). A lock
is added to the mux so that transfers through the mux are serialized.

Concerns:
- The locking is perhaps too complex?
- I worry about the priority inheritance aspect of the adapter lock. When
  the transfers behind the mux are divided into select-transfer-deselect all
  locked individually, low priority transfers get more chances to interfere
  with high priority transfers.
- When doing an i2c_transfer() in_atomic() context of with irqs_disabled(),
  there is a higher possibility that the mux is not returned to its idle
  state after a failed (-EAGAIN) transfer due to trylock.

To summarize the series, there's some i2c-mux infrastructure cleanup work
first (I think that part stands by itself as desireable regardless), the
locking changes are in the last three patches of the series, with the real
meat in 8/8.

PS. needs a bunch of testing, I do not have access to all the involved hw

Changes since v1:
- Allocate mux core and (optional) priv in a combined allocation.
- Killed dev_err messages triggered by memory allocation failure.
- Fix the device specific i2c muxes that I had overlooked.
- Rebased on top of v4.4-rc8 (was based on v4.4-rc6 previously).

Cheers,
Peter

Peter Rosin (8):
  i2c-mux: add common core data for every mux instance
  i2c-mux: move select and deselect ops to i2c_mux_core
  i2c-mux: move the slave side adapter management to i2c_mux_core
  i2c-mux: remove the mux dev pointer from the mux per channel data
  i2c-mux: pinctrl: get rid of the driver private struct device pointer
  i2c: allow adapter drivers to override the adapter locking
  i2c: muxes always lock the parent adapter
  i2c-mux: relax locking of the top i2c adapter during i2c controlled
muxing

 .../devicetree/bindings/i2c/i2c-mux-gpio.txt   |   2 +
 .../devicetree/bindings/i2c/i2c-mux-pinctrl.txt|   4 +
 drivers/i2c/i2c-core.c |  59 ++---
 drivers/i2c/i2c-mux.c  | 272 +
 drivers/i2c/muxes/i2c-arb-gpio-challenge.c |  46 ++--
 drivers/i2c/muxes/i2c-mux-gpio.c   |  58 ++---
 drivers/i2c/muxes/i2c-mux-pca9541.c|  58 +++--
 drivers/i2c/muxes/i2c-mux-pca954x.c|  66 ++---
 drivers/i2c/muxes/i2c-mux-pinctrl.c|  89 +++
 drivers/i2c/muxes/i2c-mux-reg.c|  63 ++---
 drivers/iio/imu/inv_mpu6050/inv_mpu_core.c |  33 +--
 drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h  |   2 +-
 drivers/media/dvb-frontends/m88ds3103.c|  23 +-
 drivers/media/dvb-frontends/m88ds3103_priv.h   |   2 +-
 drivers/media/dvb-frontends/rtl2830.c  |  24 +-
 drivers/media/dvb-frontends/rtl2830_priv.h |   2 +-
 drivers/media/dvb-frontends/rtl2832.c  |  30 ++-
 drivers/media/dvb-frontends/rtl2832_priv.h |   2 +-
 drivers/media/dvb-frontends/si2168.c   |  29 ++-
 drivers/media/dvb-frontends/si2168_priv.h  |   2 +-
 drivers/media/usb/cx231xx/cx231xx-core.c   |   6 +-
 drivers/media/usb/cx231xx/cx231xx-i2c.c

Re: [PATCH v2 1/8] i2c-mux: add common core data for every mux instance

[Peter Rosin]

Hi Guenter,

[BTW, if anyone feels spammed by this series, please drop me a note]

On 2016-01-05 17:42, Guenter Roeck wrote:
> On 01/05/2016 07:57 AM, Peter Rosin wrote:
>> From: Peter Rosin 
>>
>> The initial core mux structure starts off small with only the parent
>> adapter pointer, which all muxes have, and a priv pointer for mux
>> driver private data.
>>
>> Add i2c_mux_alloc function to unify the creation of a mux.
>>
>> Where appropriate, pass around the mux core structure instead of the
>> parent adapter or the driver private data.
>>
>> Remove the parent adapter pointer from the driver private data for all
>> mux drivers.
>>
>> Signed-off-by: Peter Rosin 
>> ---
>>   drivers/i2c/i2c-mux.c| 28 +-
>>   drivers/i2c/muxes/i2c-arb-gpio-challenge.c   | 24 +--
>>   drivers/i2c/muxes/i2c-mux-gpio.c | 20 
>>   drivers/i2c/muxes/i2c-mux-pca9541.c  | 35 
>> ++--
>>   drivers/i2c/muxes/i2c-mux-pca954x.c  | 19 ++-
>>   drivers/i2c/muxes/i2c-mux-pinctrl.c  | 23 +-
>>   drivers/i2c/muxes/i2c-mux-reg.c  | 23 ++
>>   drivers/iio/imu/inv_mpu6050/inv_mpu_core.c   | 10 +++-
>>   drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h|  1 +
>>   drivers/media/dvb-frontends/m88ds3103.c  | 10 +++-
>>   drivers/media/dvb-frontends/m88ds3103_priv.h |  1 +
>>   drivers/media/dvb-frontends/rtl2830.c| 10 +++-
>>   drivers/media/dvb-frontends/rtl2830_priv.h   |  1 +
>>   drivers/media/dvb-frontends/rtl2832.c| 10 +++-
>>   drivers/media/dvb-frontends/rtl2832_priv.h   |  1 +
>>   drivers/media/dvb-frontends/si2168.c | 10 +++-
>>   drivers/media/dvb-frontends/si2168_priv.h|  1 +
>>   drivers/media/usb/cx231xx/cx231xx-core.c |  3 +++
>>   drivers/media/usb/cx231xx/cx231xx-i2c.c  | 13 +--
>>   drivers/media/usb/cx231xx/cx231xx.h  |  2 ++
>>   drivers/of/unittest.c| 16 +++--
>>   include/linux/i2c-mux.h  | 14 ++-
>>   22 files changed, 187 insertions(+), 88 deletions(-)
>>
>> diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
>> index 00fc5b1c7b66..c2163f6b51d5 100644
>> --- a/drivers/i2c/i2c-mux.c
>> +++ b/drivers/i2c/i2c-mux.c
>> @@ -31,8 +31,8 @@
>>   struct i2c_mux_priv {
>>   struct i2c_adapter adap;
>>   struct i2c_algorithm algo;
>> +struct i2c_mux_core *muxc;
>>
>> -struct i2c_adapter *parent;
>>   struct device *mux_dev;
>>   void *mux_priv;
>>   u32 chan_id;
>> @@ -45,7 +45,8 @@ static int i2c_mux_master_xfer(struct i2c_adapter *adap,
>>  struct i2c_msg msgs[], int num)
>>   {
>>   struct i2c_mux_priv *priv = adap->algo_data;
>> -struct i2c_adapter *parent = priv->parent;
>> +struct i2c_mux_core *muxc = priv->muxc;
>> +struct i2c_adapter *parent = muxc->parent;
>>   int ret;
>>
>>   /* Switch to the right mux port and perform the transfer. */
>> @@ -65,7 +66,8 @@ static int i2c_mux_smbus_xfer(struct i2c_adapter *adap,
>> int size, union i2c_smbus_data *data)
>>   {
>>   struct i2c_mux_priv *priv = adap->algo_data;
>> -struct i2c_adapter *parent = priv->parent;
>> +struct i2c_mux_core *muxc = priv->muxc;
>> +struct i2c_adapter *parent = muxc->parent;
>>   int ret;
>>
>>   /* Select the right mux port and perform the transfer. */
>> @@ -84,7 +86,7 @@ static int i2c_mux_smbus_xfer(struct i2c_adapter *adap,
>>   static u32 i2c_mux_functionality(struct i2c_adapter *adap)
>>   {
>>   struct i2c_mux_priv *priv = adap->algo_data;
>> -struct i2c_adapter *parent = priv->parent;
>> +struct i2c_adapter *parent = priv->muxc->parent;
>>
>>   return parent->algo->functionality(parent);
>>   }
>> @@ -102,7 +104,20 @@ static unsigned int i2c_mux_parent_classes(struct 
>> i2c_adapter *parent)
>>   return class;
>>   }
>>
>> -struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter *parent,
>> +struct i2c_mux_core *i2c_mux_alloc(struct device *dev, int sizeof_priv)
>> +{
>> +struct i2c_mux_core *muxc;
>> +
>> +muxc = devm_kzalloc(dev, sizeof(*muxc) + sizeof_priv, GFP_KERNEL);
>> +if (!muxc)
>> +return NULL;
>> +if (sizeof_priv)
>> +muxc->priv = muxc + 1;
>> +return muxc;
>> +}
>> +EXPORT_SYMBOL_GPL(i2c_mux_alloc);
>> +
>> +struct i2c_adapter *i2c_add_mux_adapter(struct i2c_mux_core *muxc,
>>   struct device *mux_dev,
>>   void *mux_priv, u32 force_nr, u32 chan_id,
>>   unsigned int class,
>> @@ -111,6 +126,7 @@ struct i2c_adapter *i2c_add_mux_adapter(struct 
>> i2c_adapter *parent,
>>   int (*deselect) (struct i2c_adapter *,
>>void *, u32))
>>   {
>> +struct i2c_adapter *parent = muxc->parent;
>>   struct i2c_mux_priv 

Re: [PATCH v2 00/16] intel-lpss: support non-ACPI platforms

[Laura Abbott]

On 12/06/2015 05:44 PM, Rafael J. Wysocki wrote:

On Monday, November 30, 2015 05:11:28 PM Andy Shevchenko wrote:

This series includes few logical sets that bring a support of non-ACPI
platforms for Intel Skylake.

First part is a refactoring of built-in device properties support:
- keep single value inside the structure
- provide helper macros to define built-in properties
- fall back to secondary fwnode if primary has no asked property

Second is a propagating built-in device properties in platform core.

Third one is modifications to MFD code and intel-lpss.c driver in particular
to define and pass built-in properties to the individual drivers.

And last part is a fix for I2C bug found on Lenovo Yoga hardware and a first
converted user.

Built-in device properties is an alternative to platform data. It provides a
unified API that drivers can use to cover all cases at once: DT, ACPI, and
built-in properties.

With this series applied a platform data can be considered obsolete. Moreover,
built-in device properties allow to adjust the existing configuration, for
example, in cases when ACPI values are wrong on some platforms.

The series has been tested on available hardware and doesn't break current
behaviour. But we ask people who have the affected hardware to apply the series
on your side and check with Lenovo hardware.

Changelog v2:
- fix isuues found by kbuild bot (kbuild)
- append a patch to propagate device properties in polatform code (Arnd)
- update few existing and add couple of new patches due to above
- check with kmemleak

Andy Shevchenko (9):
   device property: always check for fwnode type
   device property: rename helper functions
   device property: refactor built-in properties support
   device property: keep single value inplace
   device property: improve readability of macros
   device property: return -EINVAL when property isn't found in ACPI
   device property: Fallback to secondary fwnode if primary misses the
 property
   mfd: core: propagate device properties to sub devices drivers
   mfd: intel-lpss: Pass HSUART configuration via properties

Heikki Krogerus (1):
   device property: helper macros for property entry creation

Mika Westerberg (6):
   device property: Take a copy of the property set
   driver core: platform: Add support for built-in device properties
   driver core: Do not overwrite secondary fwnode with NULL if it is set
   mfd: intel-lpss: Add support for passing device properties
   mfd: intel-lpss: Pass SDA hold time to I2C host controller driver
   i2c: designware: Convert to use unified device property API


I'm going to queue up this series for v4.5.

If there are any problems with it or objections from anyone, please let me know.




Raising an old thread, I pulled this series into Fedora rawhide and
while it worked for Lenovo Yoga we received a report that it caused
a regression on the Dell Inspiron 7559 (see
https://bugzilla.redhat.com/show_bug.cgi?id=1275718#c27) . I haven't
asked the reporter about bisecting to see which patch broke it.
Were there any known follow up patches?

Thanks,
Laura


--
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 3/8] i2c-mux: move the slave side adapter management to i2c_mux_core

[Peter Rosin]

Hi,

This should fix it (I'm not sending a v3 right away).

Cheers,
Peter

diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c 
b/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c
index 1c982a56acd5..5de993deca7e 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c
@@ -16,6 +16,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include "inv_mpu_iio.h"
@@ -182,7 +183,7 @@ int inv_mpu_acpi_create_mux_client(struct inv_mpu6050_state 
*st)
} else
return 0; /* no secondary addr, which is OK */
}
-   st->mux_client = i2c_new_device(st->mux_adapter, );
+   st->mux_client = i2c_new_device(st->muxc->adapter[0], );
if (!st->mux_client)
return -ENODEV;
 
-- 


On 2016-01-05 17:49, kbuild test robot wrote:
> Hi Peter,
> 
> [auto build test ERROR on wsa/i2c/for-next]
> [also build test ERROR on v4.4-rc8 next-20160105]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improving the system]
> 
> url:
> https://github.com/0day-ci/linux/commits/Peter-Rosin/i2c-mux-cleanup-and-locking-update/20160106-000205
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux i2c/for-next
> config: i386-randconfig-s1-201601 (attached as .config)
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=i386 
> 
> All errors (new ones prefixed by >>):
> 
>drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c: In function 
> 'inv_mpu_acpi_create_mux_client':
>>> drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c:185:37: error: 'struct 
>>> inv_mpu6050_state' has no member named 'mux_adapter'
>   st->mux_client = i2c_new_device(st->mux_adapter, );
> ^
> 
> vim +185 drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c
> 
> a35c5d1a Srinivas Pandruvada 2015-01-30  179  
> *name = '\0';
> a35c5d1a Srinivas Pandruvada 2015-01-30  180  
> strlcat(info.type, "-client",
> a35c5d1a Srinivas Pandruvada 2015-01-30  181  
> sizeof(info.type));
> a35c5d1a Srinivas Pandruvada 2015-01-30  182  } else
> a35c5d1a Srinivas Pandruvada 2015-01-30  183  
> return 0; /* no secondary addr, which is OK */
> a35c5d1a Srinivas Pandruvada 2015-01-30  184  }
> a35c5d1a Srinivas Pandruvada 2015-01-30 @185  st->mux_client 
> = i2c_new_device(st->mux_adapter, );
> a35c5d1a Srinivas Pandruvada 2015-01-30  186  if 
> (!st->mux_client)
> a35c5d1a Srinivas Pandruvada 2015-01-30  187  return 
> -ENODEV;
> a35c5d1a Srinivas Pandruvada 2015-01-30  188  
> 
> :: The code at line 185 was first introduced by commit
> :: a35c5d1aa96aa6cc70e91786cbe9be4db23f8f4a iio: imu: inv_mpu6050: Create 
> mux clients for ACPI
> 
> :: TO: Srinivas Pandruvada <srinivas.pandruv...@linux.intel.com>
> :: CC: Jonathan Cameron <ji...@kernel.org>
> 
> ---
> 0-DAY kernel test infrastructureOpen Source Technology Center
> https://lists.01.org/pipermail/kbuild-all   Intel Corporation
> 
--
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 0/8] i2c mux cleanup and locking update

[Wolfram Sang]

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: [RESEND PATCH v2 0/9] eeprom: at24: at24cs series serial number read

[Wolfram Sang]

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 Rosin]

Hi Wolfram,

On 2016-01-05 19:48, Wolfram Sang wrote:
> 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.

Yeah, I started with dipping just the toes, but now it rather feels like
I'm fully submerged at the deep end...

> 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.

Right, I'll hold back on sending updates for trivial stuff until the
big picture stuff has been cleared.

Cheers,
Peter
--
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 v4] i2c: designware: Do not require clock when SSCN and FFCN are provided

[Loc Ho]

Hi,

>> 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.
>

Will let you know when I have moment to give this a try on X-Gene.

-Loc
--
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: designware: retry transfer on transient failure

[Baruch Siach]

Hi Andy,

On Tue, Jan 05, 2016 at 02:22:28PM +0200, Andy Shevchenko wrote:
> On Mon, 2016-01-04 at 20:49 +0100, Wolfram Sang wrote:
> > 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.
> 
> Ah, good. So, I'm fine with it if no one has strong argument.

So can you give your ack to the original patch then?

Thanks,
baruch

-- 
 http://baruch.siach.name/blog/  ~. .~   Tk Open Systems
=}ooO--U--Ooo{=
   - bar...@tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
--
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: ismt-bus completion wait timed out issue

[sushil]

Puneet Shenoy  writes:

> 
> Hey Bill,
> 
> I see that you are the author of the i2c-ismt driver so thought will check 
with you on a issue that I have been
> seeing on my setup.
> 
> My setup has an Intel Atom S1220 Processor which has a PMBUS device on its 
SMBUS 2.0 controller (SMBus—Bus
> 0:D19:F0-1). 
> 
> Now while accessing(read/write) the PMBUS device, I see the following 
error messages:
> [10799.333083] ismt_smbus :00:13.1: completion wait timed out
> [10800.337089] ismt_smbus :00:13.1: completion wait timed out
> [10801.341097] ismt_smbus :00:13.1: completion wait timed out
> [10802.345105] ismt_smbus :00:13.1: completion wait timed out
> 
> All read/write access to the device fail when its in this state. The issue 
is hard to reproduce and happens
> once in a while.
> 
> I tried rmmod/modprobe the driver, but that doesn't help. I dumped the 
relevant registers that are
> dev_dbg'ed in the driver and compared the data between the failed and 
working cases and I don't see any
> difference. 
> Once in a while when I am in the failed state, I see the In Progress Bit 
(Bit 0) in the MSTS (Master Status
> Register) being set but its not always the case.
> 
> Have you seen this issue before ? Any pointers on how to debug this ?
> 
> Appreciate any help.
> Thanks
> Puneet--
> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
> the body of a message to majordomo@...
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

Puneet,
Did you get anything on this? 
Even i am also facing same error with smbus controller 2. 


N�r��yb�X��ǧv�^�)޺{.n�+{��g"��^n�r���z���h�&���G���h�(�階�ݢj"���m��z�ޖ���f���h���~�m�

Re: [PATCH v2 3/8] i2c-mux: move the slave side adapter management to i2c_mux_core

[Peter Rosin]

Ouch, this got lost in the shuffle, don't bother testing without it.
It will be included in v3.

[the reason is that my test hw relies on vendor patches, and I have to
rebase before sending. I.e., I can only compile-test the stuff I'm
actually sending out. Inconvenient.]

diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
index c4d4b14a5399..c5a5886d8be1 100644
--- a/drivers/i2c/i2c-mux.c
+++ b/drivers/i2c/i2c-mux.c
@@ -215,10 +215,12 @@ int i2c_mux_reserve_adapters(struct i2c_mux_core *muxc, 
int adapters)
if (!adapter)
return -ENOMEM;
 
-   memcpy(adapter, muxc->adapter,
-  muxc->max_adapters * sizeof(*adapter));
+   if (muxc->adapter) {
+   memcpy(adapter, muxc->adapter,
+  muxc->max_adapters * sizeof(*adapter));
+   devm_kfree(muxc->dev, muxc->adapter);
+   }
 
-   devm_kfree(muxc->dev, muxc->adapter);
muxc->adapter = adapter;
muxc->max_adapters = adapters;
return 0;
-- 


On 2016-01-05 16:57, Peter Rosin wrote:
> From: Peter Rosin 
> 
> All muxes have slave side adapters, many have some arbitrary number of
> them. Handle this in the mux core, so that drivers are simplified.
> 
> Add i2c_mux_reserve_adapter that can be used when it is known in advance
> how many child adapters that is to be added. This avoids reallocating
> memory.
> 
> Drop i2c_del_mux_adapter and replace it with i2c_del_mux_adapters, since
> no mux driver is dynamically deleting individual child adapters anyway.
> 
> Signed-off-by: Peter Rosin 
> ---
>  drivers/i2c/i2c-mux.c| 71 
> ++--
>  drivers/i2c/muxes/i2c-arb-gpio-challenge.c   | 10 ++--
>  drivers/i2c/muxes/i2c-mux-gpio.c | 23 +++--
>  drivers/i2c/muxes/i2c-mux-pca9541.c  | 13 ++---
>  drivers/i2c/muxes/i2c-mux-pca954x.c  | 26 --
>  drivers/i2c/muxes/i2c-mux-pinctrl.c  | 27 +++
>  drivers/i2c/muxes/i2c-mux-reg.c  | 28 ---
>  drivers/iio/imu/inv_mpu6050/inv_mpu_core.c   | 12 ++---
>  drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h|  1 -
>  drivers/media/dvb-frontends/m88ds3103.c  | 11 ++---
>  drivers/media/dvb-frontends/m88ds3103_priv.h |  1 -
>  drivers/media/dvb-frontends/rtl2830.c| 10 ++--
>  drivers/media/dvb-frontends/rtl2830_priv.h   |  1 -
>  drivers/media/dvb-frontends/rtl2832.c| 11 ++---
>  drivers/media/dvb-frontends/rtl2832_priv.h   |  1 -
>  drivers/media/dvb-frontends/si2168.c | 10 ++--
>  drivers/media/dvb-frontends/si2168_priv.h|  1 -
>  drivers/media/usb/cx231xx/cx231xx-core.c |  3 +-
>  drivers/media/usb/cx231xx/cx231xx-i2c.c  | 26 +-
>  drivers/media/usb/cx231xx/cx231xx.h  |  2 +-
>  drivers/of/unittest.c| 28 ---
>  include/linux/i2c-mux.h  | 15 --
>  22 files changed, 149 insertions(+), 182 deletions(-)
> 
> diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
> index 6c5cb9f7649b..7ba0308537a8 100644
> --- a/drivers/i2c/i2c-mux.c
> +++ b/drivers/i2c/i2c-mux.c
> @@ -99,6 +99,29 @@ static unsigned int i2c_mux_parent_classes(struct 
> i2c_adapter *parent)
>   return class;
>  }
>  
> +int i2c_mux_reserve_adapters(struct i2c_mux_core *muxc, int adapters)
> +{
> + struct i2c_adapter **adapter;
> +
> + if (adapters <= muxc->max_adapters)
> + return 0;
> +
> + adapter = devm_kmalloc_array(muxc->dev,
> +  adapters, sizeof(*adapter),
> +  GFP_KERNEL);
> + if (!adapter)
> + return -ENOMEM;
> +
> + memcpy(adapter, muxc->adapter,
> +muxc->max_adapters * sizeof(*adapter));
> +
> + devm_kfree(muxc->dev, muxc->adapter);
> + muxc->adapter = adapter;
> + muxc->max_adapters = adapters;
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(i2c_mux_reserve_adapters);
> +
>  struct i2c_mux_core *i2c_mux_alloc(struct device *dev, int sizeof_priv)
>  {
>   struct i2c_mux_core *muxc;
> @@ -113,19 +136,29 @@ struct i2c_mux_core *i2c_mux_alloc(struct device *dev, 
> int sizeof_priv)
>  }
>  EXPORT_SYMBOL_GPL(i2c_mux_alloc);
>  
> -struct i2c_adapter *i2c_add_mux_adapter(struct i2c_mux_core *muxc,
> - struct device *mux_dev,
> - u32 force_nr, u32 chan_id,
> - unsigned int class)
> +int i2c_add_mux_adapter(struct i2c_mux_core *muxc,
> + struct device *mux_dev,
> + u32 force_nr, u32 chan_id,
> + unsigned int class)
>  {
>   struct i2c_adapter *parent = muxc->parent;
>   struct i2c_mux_priv *priv;
>   char symlink_name[20];
>   int ret;
>  
> + if (muxc->adapters >= muxc->max_adapters) {
> + int new_max = 2 * muxc->max_adapters;
> +
> + if (!new_max)
> +   

Re: [PATCH 2/2] i2c: designware: Allow build Baytrail semaphore support when IOSF_MBI=m

[David E. Box]

Hi

Sorry I missed this discussion. I believe the following code in
i2c_dw_eval_lock_support() should make it so that it doesn't matter how
IOSF_MBI is built:

   if (!iosf_mbi_available())
   return -EPROBE_DEFER;

I added this to address i2c_designware probing before iosf_mbi. It worked but
I do not recall if IOSF_MBI=m was the problem scenario. If so you can just
change it to:

   depends in I2C_DESIGNWARE_PLATFORM && IOSF_MBI

Give me a few days to confirm on my Baytrail device. 

David

On Thu, Dec 10, 2015 at 01:48:44PM +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)
>   help
> This driver enables managed host access to the PMIC I2C bus on select
> Intel BayTrail platforms using the X-Powers AXP288 PMIC. It allows
> -- 
> 2.6.2
> 
--
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 00/16] intel-lpss: support non-ACPI platforms

[Rafael J. Wysocki]

On Tuesday, January 05, 2016 09:57:35 AM Laura Abbott wrote:
> On 12/06/2015 05:44 PM, Rafael J. Wysocki wrote:
> > On Monday, November 30, 2015 05:11:28 PM Andy Shevchenko wrote:
> >> This series includes few logical sets that bring a support of non-ACPI
> >> platforms for Intel Skylake.
> >>
> >> First part is a refactoring of built-in device properties support:
> >> - keep single value inside the structure
> >> - provide helper macros to define built-in properties
> >> - fall back to secondary fwnode if primary has no asked property
> >>
> >> Second is a propagating built-in device properties in platform core.
> >>
> >> Third one is modifications to MFD code and intel-lpss.c driver in 
> >> particular
> >> to define and pass built-in properties to the individual drivers.
> >>
> >> And last part is a fix for I2C bug found on Lenovo Yoga hardware and a 
> >> first
> >> converted user.
> >>
> >> Built-in device properties is an alternative to platform data. It provides 
> >> a
> >> unified API that drivers can use to cover all cases at once: DT, ACPI, and
> >> built-in properties.
> >>
> >> With this series applied a platform data can be considered obsolete. 
> >> Moreover,
> >> built-in device properties allow to adjust the existing configuration, for
> >> example, in cases when ACPI values are wrong on some platforms.
> >>
> >> The series has been tested on available hardware and doesn't break current
> >> behaviour. But we ask people who have the affected hardware to apply the 
> >> series
> >> on your side and check with Lenovo hardware.
> >>
> >> Changelog v2:
> >> - fix isuues found by kbuild bot (kbuild)
> >> - append a patch to propagate device properties in polatform code (Arnd)
> >> - update few existing and add couple of new patches due to above
> >> - check with kmemleak
> >>
> >> Andy Shevchenko (9):
> >>device property: always check for fwnode type
> >>device property: rename helper functions
> >>device property: refactor built-in properties support
> >>device property: keep single value inplace
> >>device property: improve readability of macros
> >>device property: return -EINVAL when property isn't found in ACPI
> >>device property: Fallback to secondary fwnode if primary misses the
> >>  property
> >>mfd: core: propagate device properties to sub devices drivers
> >>mfd: intel-lpss: Pass HSUART configuration via properties
> >>
> >> Heikki Krogerus (1):
> >>device property: helper macros for property entry creation
> >>
> >> Mika Westerberg (6):
> >>device property: Take a copy of the property set
> >>driver core: platform: Add support for built-in device properties
> >>driver core: Do not overwrite secondary fwnode with NULL if it is set
> >>mfd: intel-lpss: Add support for passing device properties
> >>mfd: intel-lpss: Pass SDA hold time to I2C host controller driver
> >>i2c: designware: Convert to use unified device property API
> >
> > I'm going to queue up this series for v4.5.
> >
> > If there are any problems with it or objections from anyone, please let me 
> > know.
> >
> 
> 
> Raising an old thread, I pulled this series into Fedora rawhide and
> while it worked for Lenovo Yoga we received a report that it caused
> a regression on the Dell Inspiron 7559 (see
> https://bugzilla.redhat.com/show_bug.cgi?id=1275718#c27) . I haven't
> asked the reporter about bisecting to see which patch broke it.
> Were there any known follow up patches?

There were a few.

All of them are in my linux-next branch if you can try this one.

Alternatively, I can expose a branch with these to you to test.

Thanks,
Rafael

--
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: designware: retry transfer on transient failure

[Andy Shevchenko]

On Mon, 2016-01-04 at 20:49 +0100, Wolfram Sang wrote:
> > 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 :)

Point taken.

> 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.

Ah, good. So, I'm fine with it if no one has strong argument.

-- 
Andy Shevchenko 
Intel Finland Oy

--
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/designware: enable i2c controller to suspend/resume asynchronously

[Andy Shevchenko]

On Tue, 2016-01-05 at 10:53 +0200, Jarkko Nikula wrote:
> Hi
> 
> On 12/24/2015 04:30 PM, Fu, Zhonghui wrote:
> > Now, PM core supports asynchronous suspend/resume mode for devices
> > during system suspend/resume, and the power state transition of one
> > device may be completed in separate kernel thread. PM core ensures
> > all power state transition dependency between devices. This patch
> > enables designware i2c controllers to suspend/resume
> > asynchronously.
> > This will take advantage of multicore and improve system
> > suspend/resume
> > speed. After enabling all i2c devices, i2c adapters and i2c
> > controllers
> > on ASUS T100TA tablet, the system suspend-to-idle time is reduced
> > to
> > about 510ms from 750ms, and the system resume time is reduced to
> > about
> > 790ms from 900ms.
> > 
> Nice reduction :-)
> 
> > diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c
> > b/drivers/i2c/busses/i2c-designware-platdrv.c
> > index 6b00061..395130b 100644
> > --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> > +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> > @@ -230,6 +230,7 @@ static int dw_i2c_plat_probe(struct
> > platform_device *pdev)
> >     }
> > 
> >     adap = >adapter;
> > +   device_enable_async_suspend(>dev);
> >     adap->owner = THIS_MODULE;
> >     adap->class = I2C_CLASS_DEPRECATED;
> >     ACPI_COMPANION_SET(>dev, ACPI_COMPANION(
> > >dev));
> 
> Does device_enable_async_suspend() need to be called before enabling 
> runtime PM? I suppose not since there appears to have also related
> sysfs 
> node for toggling it runtime.
> 
> I'm thinking if you could move the device_enable_async_suspend() call
> into drivers/i2c/busses/i2c-designware-core.c: i2c_dw_probe() and
> then 
> also PCI enumerated adapter could take advantage of it.

I concern about Intel BayTrail-T / Braswell / CherryTrail cases, since
we have non-trivial PM for LPSS there. Zhonghui, have you a chance to
stress test this on platforms based on mentioned SoCs?


-- 
Andy Shevchenko 
Intel Finland Oy

--
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