Re: [PATCH 10/10] i2c: pca954x: get rid of the i2c deadlock workaround

[Lars-Peter Clausen]

On 01/04/2016 04:10 PM, Peter Rosin wrote:
> From: Peter Rosin 
> 
> Signed-off-by: Peter Rosin 

It would be quite good if the commit messaged said why it is now safe to
remove the workaround.

--
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 1/3] i2c: Revert "i2c: xiic: Do not reset controller before every transfer"

[Lars-Peter Clausen]

On 11/17/2015 03:28 PM, Shubhrajyoti Datta wrote:
> On Tue, Nov 17, 2015 at 1:04 PM, Lars-Peter Clausen <l...@metafoo.de> wrote:
>> On 11/17/2015 06:17 AM, Shubhrajyoti Datta wrote:
>>> On Mon, Nov 16, 2015 at 7:12 PM, Lars-Peter Clausen <l...@metafoo.de> wrote:
>>>> Commit d701667bb331 ("i2c: xiic: Do not reset controller before every
>>>> transfer") removed the reinitialization of the controller before the start
>>>> of each transfer. Apparently this change is not safe to make and the commit
>>>> results in random I2C bus failures.
>>>
>>> Which is the platform and the ip version that you  saw the issue.
>>> Did you see the issue with read and write as  well?
>>
>> The IP version is the axi-iic v2.0 Revision 8. I've tested this on a few
>> platforms, custom ones and standard ones and I could reproduce it on most.
>> One of them was on the ZED board. The one where I couldn't reproduce it was
>> the ZC706. But that doesn't necessarily mean it doesn't happen there, just
>> that it is not triggered by the testcase.
> All the boards having the same version of the ip is what I have understood.
> 
> Thanks for the info I will try to  reproduce the issue.
> 
>>
>> The problem is that it is random corruption,
> Of registers?

To be honest I don't know if there is corruption during I2C write transfers,
but there is definitely corruption during read transactions.

- Lars

--
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: xiic: Prevent concurrent running of the IRQ handler and __xiic_start_xfer()

[Lars-Peter Clausen]

Prior to commit e6c9a037bc8a ("i2c: xiic: Remove the disabling of
interrupts") IRQs where disabled when the initial __xiic_start_xfer() was
called. After the commit the interrupt is enabled while the function is
running, this means it is possible for the interrupt to be triggered while
the function is still running. When this happens the internal data
structures get corrupted and undefined behavior can occur like the
following crash:

Internal error: Oops: 17 [#1] PREEMPT SMP ARM
Modules linked in:
CPU: 0 PID: 2040 Comm: i2cdetect Not tainted 4.0.0-02856-g047a308 #10956
Hardware name: Xilinx Zynq Platform
task: ee0c9500 ti: e99a2000 task.ti: e99a2000
PC is at __xiic_start_xfer+0x6c4/0x7c8
LR is at __xiic_start_xfer+0x690/0x7c8
pc : []lr : []psr: 800f0013
sp : e99a3da8  ip :   fp : 
r10: 0001  r9 : 600f0013  r8 : f018
r7 : f018  r6 : c064e444  r5 : 0017  r4 : ee031010
r3 :   r2 :   r1 : 600f0013  r0 : 000f
Flags: Nzcv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment user
Control: 18c5387d  Table: 29a5404a  DAC: 0015
Process i2cdetect (pid: 2040, stack limit = 0xe99a2210)
Stack: (0xe99a3da8 to 0xe99a4000)
3da0:   ee031010  0001 ee031020 ee031224 
c02bc5ec
3dc0: ee34c604  ee0c9500 e99a3dcc e99a3dd0 e99a3dd0 e99a3dd8 
c069f0e8
3de0:  ee031020 c064e100 90bb e99a3e48 c02b6590 ee031020 
0001
3e00: e99a3e48 ee031020  e99a3e63 0001 c02b6ec4  

3e20:  c02b7320 e99a3ef0   e99e3df0  

3e40: 0103 2814575f 003e c00a e99a3e85 0001003e ee0c 
e99a3e63
3e60: eefd3578 c064e61c ee0c9500 c0041e04 056c e9a56db8 6e5a 
b6f5c000
3e80: ee0c9548 eefd0040 0001 eefd3540 ee0c9500 eefd39a0 c064b540 
ee0c9500
3ea0:  ee92b000  bef4862c ee34c600 e99ecdc0 0720 
0003
3ec0: e99a2000   c02b8b30    
e99a3f24
3ee0: b6e8   c04257e8  e99a3f24 c02b8f08 
0703
3f00: 0003 c02116bc ee935300  bef4862c ee34c600 e99ecdc0 
c02b91f0
3f20: e99ecdc0 0720 bef4862c eeb725f8 e99ecdc0 c00c9e2c 0003 
0003
3f40: ee248dc0  ee248dc8 0002 eeb7c1a8   
c00bb360
3f60:   0003 ee248dc0 bef4862c e99ecdc0 e99ecdc0 
0720
3f80: 0003 e99a2000  c00c9f68   b6f22000 
0036
3fa0: c000dfa4 c000de20   0003 0720 bef4862c 
bef4862c
3fc0:   b6f22000 0036   b6f6 

3fe0: 00013040 bef48614 8cab b6ecdbe6 400f0030 0003 2f7fd821 
2f7fdc21
[] (__xiic_start_xfer) from [] 
(xiic_xfer+0x94/0x168)
[] (xiic_xfer) from [] (__i2c_transfer+0x4c/0x7c)
[] (__i2c_transfer) from [] (i2c_transfer+0x9c/0xc4)
[] (i2c_transfer) from [] 
(i2c_smbus_xfer+0x3a0/0x4ec)
[] (i2c_smbus_xfer) from [] 
(i2cdev_ioctl_smbus+0xb0/0x214)
[] (i2cdev_ioctl_smbus) from [] 
(i2cdev_ioctl+0xa0/0x1d4)
[] (i2cdev_ioctl) from [] (do_vfs_ioctl+0x4b0/0x5b8)
[] (do_vfs_ioctl) from [] (SyS_ioctl+0x34/0x5c)
[] (SyS_ioctl) from [] (ret_fast_syscall+0x0/0x34)
Code: e283300c e5843210 eafffe64 e5943210 (e1d320b4)

The issue can easily be reproduced by performing I2C access under high
system load or IO load.

To fix the issue protect the invocation to __xiic_start_xfer() form
xiic_start_xfer() with the same lock that is used to protect the interrupt
handler.

Fixes: e6c9a037bc8a ("i2c: xiic: Remove the disabling of interrupts")
Signed-off-by: Lars-Peter Clausen <l...@metafoo.de>
---
 drivers/i2c/busses/i2c-xiic.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
index 705cf69..0b20449 100644
--- a/drivers/i2c/busses/i2c-xiic.c
+++ b/drivers/i2c/busses/i2c-xiic.c
@@ -664,9 +664,8 @@ static void xiic_start_xfer(struct xiic_i2c *i2c)
 {
spin_lock(>lock);
xiic_reinit(i2c);
-   spin_unlock(>lock);
-
__xiic_start_xfer(i2c);
+   spin_unlock(>lock);
 }
 
 static int xiic_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
-- 
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: Revert "i2c: xiic: Do not reset controller before every transfer"

[Lars-Peter Clausen]

Commit d701667bb331 ("i2c: xiic: Do not reset controller before every
transfer") removed the reinitialization of the controller before the start
of each transfer. Apparently this change is not safe to make and the commit
results in random I2C bus failures.

An easy way to trigger the issue is to run i2cdetect.

Without the patch applied:
 0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f
 00:  -- -- -- -- -- -- -- -- -- -- -- -- --
 10: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
 20: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
 30: -- -- -- -- -- -- -- -- UU UU -- UU 3c -- -- UU
 40: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
 50: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
 60: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
 70: -- -- -- -- -- -- -- --

With the patch applied every other or so invocation:
 0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f
 00:  03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f
 10: 10 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f
 20: 20 21 22 23 24 25 26 27 28 29 2a 2b 2c 2d 2e 2f
 30: -- -- -- -- -- -- -- -- UU UU -- UU 3c -- -- UU
 40: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
 50: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
 60: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
 70: -- -- -- -- -- -- -- --

So revert the commit for now.

Fixes: d701667bb331 ("i2c: xiic: Do not reset controller before every transfer")
Signed-off-by: Lars-Peter Clausen <l...@metafoo.de>
---
 drivers/i2c/busses/i2c-xiic.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
index e23a7b0..705cf69 100644
--- a/drivers/i2c/busses/i2c-xiic.c
+++ b/drivers/i2c/busses/i2c-xiic.c
@@ -662,6 +662,9 @@ static void __xiic_start_xfer(struct xiic_i2c *i2c)
 
 static void xiic_start_xfer(struct xiic_i2c *i2c)
 {
+   spin_lock(>lock);
+   xiic_reinit(i2c);
+   spin_unlock(>lock);
 
__xiic_start_xfer(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 3/3] i2c: xiic: Replace spinlock with mutex

[Lars-Peter Clausen]

All protected sections are only called from sleep-able context, so there is
no need to use a spinlock.

Signed-off-by: Lars-Peter Clausen <l...@metafoo.de>
---
 drivers/i2c/busses/i2c-xiic.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
index 0b20449..6efd200 100644
--- a/drivers/i2c/busses/i2c-xiic.c
+++ b/drivers/i2c/busses/i2c-xiic.c
@@ -70,7 +70,7 @@ struct xiic_i2c {
wait_queue_head_t   wait;
struct i2c_adapter  adap;
struct i2c_msg  *tx_msg;
-   spinlock_t  lock;
+   struct mutexlock;
unsigned inttx_pos;
unsigned intnmsgs;
enum xilinx_i2c_state   state;
@@ -369,7 +369,7 @@ static irqreturn_t xiic_process(int irq, void *dev_id)
 * To find which interrupts are pending; AND interrupts pending with
 * interrupts masked.
 */
-   spin_lock(>lock);
+   mutex_lock(>lock);
isr = xiic_getreg32(i2c, XIIC_IISR_OFFSET);
ier = xiic_getreg32(i2c, XIIC_IIER_OFFSET);
pend = isr & ier;
@@ -497,7 +497,7 @@ out:
dev_dbg(i2c->adap.dev.parent, "%s clr: 0x%x\n", __func__, clr);
 
xiic_setreg32(i2c, XIIC_IISR_OFFSET, clr);
-   spin_unlock(>lock);
+   mutex_unlock(>lock);
return IRQ_HANDLED;
 }
 
@@ -662,10 +662,10 @@ static void __xiic_start_xfer(struct xiic_i2c *i2c)
 
 static void xiic_start_xfer(struct xiic_i2c *i2c)
 {
-   spin_lock(>lock);
+   mutex_lock(>lock);
xiic_reinit(i2c);
__xiic_start_xfer(i2c);
-   spin_unlock(>lock);
+   mutex_unlock(>lock);
 }
 
 static int xiic_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
@@ -745,7 +745,7 @@ static int xiic_i2c_probe(struct platform_device *pdev)
i2c->adap.dev.parent = >dev;
i2c->adap.dev.of_node = pdev->dev.of_node;
 
-   spin_lock_init(>lock);
+   mutex_init(>lock);
init_waitqueue_head(>wait);
 
ret = devm_request_threaded_irq(>dev, irq, xiic_isr,
-- 
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 1/3] i2c: Revert "i2c: xiic: Do not reset controller before every transfer"

[Lars-Peter Clausen]

On 11/17/2015 06:17 AM, Shubhrajyoti Datta wrote:
> On Mon, Nov 16, 2015 at 7:12 PM, Lars-Peter Clausen <l...@metafoo.de> wrote:
>> Commit d701667bb331 ("i2c: xiic: Do not reset controller before every
>> transfer") removed the reinitialization of the controller before the start
>> of each transfer. Apparently this change is not safe to make and the commit
>> results in random I2C bus failures.
> 
> Which is the platform and the ip version that you  saw the issue.
> Did you see the issue with read and write as  well?

The IP version is the axi-iic v2.0 Revision 8. I've tested this on a few
platforms, custom ones and standard ones and I could reproduce it on most.
One of them was on the ZED board. The one where I couldn't reproduce it was
the ZC706. But that doesn't necessarily mean it doesn't happen there, just
that it is not triggered by the testcase.

The problem is that it is random corruption, so some I2C devices might start
to behave strangely at some point. The only good more or less reliable way
to reproduce it that I found was to run i2cdetect a couple of times and at
least one of them will produce strange behavior.

>>
>> An easy way to trigger the issue is to run i2cdetect.
>>
>> Without the patch applied:
>>  0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f
>>  00:  -- -- -- -- -- -- -- -- -- -- -- -- --
>>  10: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
>>  20: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
>>  30: -- -- -- -- -- -- -- -- UU UU -- UU 3c -- -- UU
>>  40: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
>>  50: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
>>  60: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
>>  70: -- -- -- -- -- -- -- --
>>
>> With the patch applied every other or so invocation:
>>  0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f
>>  00:  03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f
>>  10: 10 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f
>>  20: 20 21 22 23 24 25 26 27 28 29 2a 2b 2c 2d 2e 2f
>>  30: -- -- -- -- -- -- -- -- UU UU -- UU 3c -- -- UU
>>  40: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
>>  50: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
>>  60: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
>>  70: -- -- -- -- -- -- -- --
>>
> I assume that you have these many peripherals.
> on the bus am I right?

Sorry, I should have been more clear. The first one is before the patch that
introduces the issue, the second one is with the patch that introduces the
issue.

- Lars
--
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 4/4] iio: humidity: si7020: added No Hold read mode

[Lars-Peter Clausen]

>> So maybe more like this:
>>
>> static inline bool i2c_check_quirks(struct i2c_adapter *adap, u64 quirks)
>> {
>> if (!adap->quirks)
>> return false;
>> return (adap->quirks->flags & quirks) == quirks;
>> }
> 
> Should I use bool (like in your snippet) or int (like 
> i2c_check_functionality) as return type?

I'd use bool, given that the result is a boolean value. It's semantically
more clear this way.
--
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 4/4] iio: humidity: si7020: added No Hold read mode

[Lars-Peter Clausen]

On 10/28/2015 07:58 AM, Nicola Corna wrote:
[...]
> + holdmode = !((*client)->adapter->quirks &&
> + (*client)->adapter->quirks->flags &
[...]
> +client->adapter->quirks &&
> +client->adapter->quirks->flags & I2C_AQ_NO_CLK_STRETCH)

This is rather ugly, can we get a helper in the I2C core something along the
lines of

i2c_check_quirks(client->adapter, I2C_AQ_NO_CLK_STRETCH)

- Lars

--
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 4/4] iio: humidity: si7020: added No Hold read mode

[Lars-Peter Clausen]

On 10/28/2015 07:35 PM, Nicola Corna wrote:
> October 28 2015 10:38 AM, "Lars-Peter Clausen" <l...@metafoo.de> wrote:
> 
>> On 10/28/2015 07:58 AM, Nicola Corna wrote:
>> [...]
>>
>>> + holdmode = !((*client)->adapter->quirks &&
>>> + (*client)->adapter->quirks->flags &
>>
>> [...]
>>
>>> + client->adapter->quirks &&
>>> + client->adapter->quirks->flags & I2C_AQ_NO_CLK_STRETCH)
>>
>> This is rather ugly, can we get a helper in the I2C core something along the
>> lines of
>>
>> i2c_check_quirks(client->adapter, I2C_AQ_NO_CLK_STRETCH)
>>
>> - Lars
> 
> Something like this?
> 
> ---
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index a69a9a0..a06ffc0 100644
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -613,6 +613,12 @@ static inline int i2c_check_functionality(struct 
> i2c_adapter *adap, u32 func)
> return (func & i2c_get_functionality(adap)) == func;
> }
> 
> +/* Return 1 if adapter has the specified quirks, 0 if not. */
> +static inline int i2c_check_quirks(struct i2c_adapter *adap, u64 quirks)
> +{
> + return (quirks & (adap->quirks ? adap->quirks->flags : 0)) == quirks;
> +}

This is not a code obfuscation contest ;)

So maybe more like this:

static inline bool i2c_check_quirks(struct i2c_adapter *adap, u64 quirks)
{
if (!adap->quirks)
return false;
return (adap->quirks->flags & quirks) == quirks;
}

And please use kernel-doc for the documentation.
--
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: Don't enable PM runtime on the adapter device

[Lars-Peter Clausen]

On 04/16/2015 12:33 PM, Sylwester Nawrocki wrote:

On 16/04/15 12:10, Charles Keepax wrote:

Commit 523c5b89640e (i2c: Remove support for legacy PM) removed the PM
ops from the bus type, which causes the pm operations on the s3c2410
adapter device to fail (-ENOSUPP in rpm_callback). The adapter device
doesn't get bound to a driver and as such can't have its own pm_runtime
callbacks. Previously this was fine as the bus callbacks would have been
used, but now this can cause devices which use PM runtime and are
attached over I2C to fail to resume.

This commit fixes this issue by just doing the PM operations directly on
the I2C device, rather than the adapter device in the driver and adding
some stub callbacks for runtime suspend and resume.

Signed-off-by: Charles Keepax ckee...@opensource.wolfsonmicro.com
---
  drivers/i2c/busses/i2c-s3c2410.c |   21 -
  1 files changed, 16 insertions(+), 5 deletions(-)



@@ -1253,7 +1253,6 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, i2c);



Wouldn't adding

pm_runtime_no_callbacks(pdev-dev);

here let us avoid the runtime resume/suspend stubs?


Or just do pm_runtime_no_callbacks on the adapter device. Preferably in the 
I2C core. That should solve the issue without requiring any other changes.


- Lars
--
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 21/66] rtl2830: implement own I2C locking

[Lars-Peter Clausen]

On 02/02/2015 09:33 PM, Wolfram Sang wrote:



Ok, this may eventually work ok for now, but a further change at the I2C
core could easily break it. So, we need to double check about such
patch with the I2C maintainer.

Jean,

Are you ok with such patch? If so, please ack.


Jean handed over I2C to me in late 2012 :)


Basic problem here is that I2C-mux itself is controlled by same I2C device
which implements I2C adapter for tuner.

Here is what connections looks like:
  ___  
|  USB IF   |   |   demod|   |tuner   |
|---|   ||   ||
|   |--I2C--|-/ -|--I2C--||
|I2C master |   |  I2C mux   |   | I2C slave  |
|___|   ||   ||


So when tuner is called via I2C, it needs recursively call same I2C adapter
which is already locked. More elegant solution would be indeed nice.


So, AFAIU this is the same problem that I2C based mux devices have (like
drivers/i2c/muxes/i2c-mux-pca954x.c)? They also use the unlocked
transfers...


But those are all called with the lock for the adapter being held. I'm not 
convinced this is the same for this patch. This patch seems to add a device 
mutex which protects against concurrent access to the bus with the device 
itself, but not against concurrent access with other devices on the same bus.

--
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] iio: ak8975: Added autodetect feature for ACPI

[Lars-Peter Clausen]

Added I2C to Cc.

On 12/15/2014 10:19 PM, Srinivas Pandruvada wrote:

Using i2c auto detect feature and auto device creation feature,
enumerate ak8975 device, by checking their presence.
This is needed because when this device sits behind an i2c mux, there
is no way to define i2c mux in ACPI. This will enable ak8975 on
windows based tablets/laptops running Linux when connected via a mux.
Since DT model already can define an i2c mux and devices connected to
it, this feature is only enabled for ACPI.



This is quite a bit of a hack. Did they decide to not include the device in 
the ACPI description at all or is there a special id for INV6050+AK8975?


How does Windows decide whether there is a device or not?


Signed-off-by: Srinivas Pandruvada srinivas.pandruv...@linux.intel.com
---
  drivers/iio/magnetometer/ak8975.c | 35 +++
  1 file changed, 35 insertions(+)

diff --git a/drivers/iio/magnetometer/ak8975.c 
b/drivers/iio/magnetometer/ak8975.c
index 0d10a4b..c3455bd 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -820,6 +820,36 @@ static const struct i2c_device_id ak8975_id[] = {

  MODULE_DEVICE_TABLE(i2c, ak8975_id);

+#if defined(CONFIG_ACPI)
+static int ak8975_detect(struct i2c_client *temp_client,
+struct i2c_board_info *info)
+{
+   struct i2c_adapter *adapter;
+   int i, j;
+   int ret;
+
+   /* autodetect only when we are behind a mux */
+   adapter = i2c_parent_is_i2c_adapter(temp_client-adapter);
+   if (!adapter)
+   return -ENODEV;
+
+   for (i = 0; i  AK_MAX_TYPE; ++i) {
+   ret = ak8975_who_i_am(temp_client, i);
+   if (ret = 0) {
+   for (j = 0; j  ARRAY_SIZE(ak8975_id) - 1; ++j) {
+   if (i == (int)ak8975_id[j].driver_data) {
+   strlcpy(info-type, ak8975_id[j].name,
+   I2C_NAME_SIZE);
+   return 0;
+   }
+   }
+   }
+   }
+
+   return -ENODEV;
+}
+#endif
+
  static const struct of_device_id ak8975_of_match[] = {
{ .compatible = asahi-kasei,ak8975, },
{ .compatible = ak8975, },
@@ -841,6 +871,11 @@ static struct i2c_driver ak8975_driver = {
},
.probe  = ak8975_probe,
.id_table   = ak8975_id,
+#if defined(CONFIG_ACPI)
+   .class  = I2C_CLASS_HWMON,
+   .address_list   = I2C_ADDRS(0x0C, 0x0D, 0x0E, 0x0F),
+   .detect = ak8975_detect,


In contrast to the commit message this is always enabled if the kernel 
contains ACPI support, not if the device is a ACPI device.



+#endif
  };
  module_i2c_driver(ak8975_driver);




--
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] iio: ak8975: Added autodetect feature for ACPI

[Lars-Peter Clausen]

On 12/18/2014 05:52 PM, Srinivas Pandruvada wrote:

On Thu, 2014-12-18 at 17:28 +0100, Lars-Peter Clausen wrote:

Added I2C to Cc.

On 12/15/2014 10:19 PM, Srinivas Pandruvada wrote:

Using i2c auto detect feature and auto device creation feature,
enumerate ak8975 device, by checking their presence.
This is needed because when this device sits behind an i2c mux, there
is no way to define i2c mux in ACPI. This will enable ak8975 on
windows based tablets/laptops running Linux when connected via a mux.
Since DT model already can define an i2c mux and devices connected to
it, this feature is only enabled for ACPI.



This is quite a bit of a hack.

Why?
Auto detect is standard feature of i2c devices. This is using standard
auto detect feature provided by the framework.


Auto detect is ugly, slow and unreliable, it's kind of like the last straw 
if nothing else works.


Autodetect will be executed for every adapter with every device that 
supports auto detection, so you want to keep the amount of devices that do 
auto detection to a minimum in order to avoid both false positives and 
reduce boot time.




  Did they decide to not include the device in
the ACPI description at all or is there a special id for INV6050+AK8975?

This device needs has one combined id. Windows has a singe driver
processing both as a combo device.


Ok, then use the combined id to instantiate INV6050+AK8975.

- Lars

--
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] iio: ak8975: Added autodetect feature for ACPI

[Lars-Peter Clausen]

On 12/18/2014 06:30 PM, Srinivas Pandruvada wrote:

On Thu, 2014-12-18 at 18:05 +0100, Lars-Peter Clausen wrote:

On 12/18/2014 05:52 PM, Srinivas Pandruvada wrote:

On Thu, 2014-12-18 at 17:28 +0100, Lars-Peter Clausen wrote:

Added I2C to Cc.

On 12/15/2014 10:19 PM, Srinivas Pandruvada wrote:

Using i2c auto detect feature and auto device creation feature,
enumerate ak8975 device, by checking their presence.
This is needed because when this device sits behind an i2c mux, there
is no way to define i2c mux in ACPI. This will enable ak8975 on
windows based tablets/laptops running Linux when connected via a mux.
Since DT model already can define an i2c mux and devices connected to
it, this feature is only enabled for ACPI.



This is quite a bit of a hack.

Why?
Auto detect is standard feature of i2c devices. This is using standard
auto detect feature provided by the framework.


Auto detect is ugly, slow and unreliable, it's kind of like the last straw
if nothing else works.

That is true here. You can't enumerate this device by ACPI. As discussed
before we created i2c mux in inv6050 so that we can use AK8975 in bypass
mode. I added some API to create i2c device on this mux, which Wolfram
didn't like. He wanted to enumerate using existing mechanisms.


If there is only a single ACPI ID that says this is a INV6050 with a AK8975 
attached then the way to handle this is to have a driver that binds to that id 
and creates both devices.


- Lars

--
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 2/2] i2c: Remove support for legacy PM

[Lars-Peter Clausen]

On 12/04/2014 07:11 PM, Wolfram Sang wrote:

On Sun, Nov 30, 2014 at 05:52:32PM +0100, Lars-Peter Clausen wrote:

There haven't been any I2C driver that use the legacy suspend/resume
callbacks for a while now and new drivers are supposed to use PM ops. So
remove support for legacy suspend/resume for I2C drivers.

Since there aren't any special bus specific things to do during
suspend/resume and since the PM core will automatically fallback directly to
using the device's PM ops if no bus PM ops are specified there is no need to
have any I2C bus PM ops.

Signed-off-by: Lars-Peter Clausen l...@metafoo.de


I'll keep this for 3.20, 3.19 is too close now. But thanks, the diffstat
looks great :) Just curious, has buildbot tested this already?



No, but I did allyesconfig on both ARM and x86. As well as a `grep  
i2c_driver   -A20 * -r | grep '\(suspend\|resume\)'` and running the 
following cocci script.


@@
identifier fn;
identifier drv;
@@
struct i2c_driver drv  = {
...,
*   .suspend = fn,
...,
};

@@
identifier fn;
identifier drv;
@@
struct i2c_driver drv = {
...,
*   .resume = fn,
...,
};

None of those methods found anything, so I'm quite confident that there are 
no users left.


- Lars
--
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/4] devicetree: bindings: Add defeature-repeated-start property for Cadence I2C

[Lars-Peter Clausen]

On 12/02/2014 03:15 PM, Wolfram Sang wrote:

What do you do when disable repeated start? Sending STOP and START? If
so, this is really something different than repeated start. By using
I2C_FUNC_I2C a user expects repeated start, so if the HW does not
support it, we should say so and don't try to emulate it with something
different.


Yes, we send stop.


As said before, this is wrong. Another master could interfere between
the messages when using stop+start. This is no replacement for repeated
start.


More importantly a lot of I2C slaves also reset their internal state machine 
on a stop. So e.g. if reading a register is implemented by doing 
start,write,repeated start,read,stop and you replace that with 
start,write,stop,start,read,stop you'll always read register zero instead of 
the register you wanted to read.


- Lars

--
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] Documentation: i2c: Use PM ops instead of legacy suspend/resume

[Lars-Peter Clausen]

New drivers should use PM ops instead of the legacy suspend/resume
callbacks. Update the I2C device driver guides to reflect this.

Signed-off-by: Lars-Peter Clausen l...@metafoo.de
---
 Documentation/i2c/upgrading-clients | 6 ++
 Documentation/i2c/writing-clients   | 8 
 2 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/Documentation/i2c/upgrading-clients 
b/Documentation/i2c/upgrading-clients
index 8e5fbd8..ccba3ff 100644
--- a/Documentation/i2c/upgrading-clients
+++ b/Documentation/i2c/upgrading-clients
@@ -79,11 +79,10 @@ static struct i2c_driver example_driver = {
.driver = {
.owner  = THIS_MODULE,
.name   = example,
+   .pm = example_pm_ops,
},
.attach_adapter = example_attach_adapter,
.detach_client  = example_detach,
-   .suspend= example_suspend,
-   .resume = example_resume,
 };
 
 
@@ -272,10 +271,9 @@ static struct i2c_driver example_driver = {
.driver = {
.owner  = THIS_MODULE,
.name   = example,
+   .pm = example_pm_ops,
},
.id_table   = example_idtable,
.probe  = example_probe,
.remove = example_remove,
-   .suspend= example_suspend,
-   .resume = example_resume,
 };
diff --git a/Documentation/i2c/writing-clients 
b/Documentation/i2c/writing-clients
index 6b344b5..a755b14 100644
--- a/Documentation/i2c/writing-clients
+++ b/Documentation/i2c/writing-clients
@@ -36,6 +36,7 @@ MODULE_DEVICE_TABLE(i2c, foo_idtable);
 static struct i2c_driver foo_driver = {
.driver = {
.name   = foo,
+   .pm = foo_pm_ops,  /* optional */
},
 
.id_table   = foo_idtable,
@@ -47,8 +48,6 @@ static struct i2c_driver foo_driver = {
.address_list   = normal_i2c,
 
.shutdown   = foo_shutdown, /* optional */
-   .suspend= foo_suspend,  /* optional */
-   .resume = foo_resume,   /* optional */
.command= foo_command,  /* optional, deprecated */
 }
 
@@ -279,8 +278,9 @@ Power Management
 
 If your I2C device needs special handling when entering a system low
 power state -- like putting a transceiver into a low power mode, or
-activating a system wakeup mechanism -- do that in the suspend() method.
-The resume() method should reverse what the suspend() method does.
+activating a system wakeup mechanism -- do that by implementing the
+appropriate callbacks for the dev_pm_ops of the driver (like suspend
+and resume).
 
 These are standard driver model calls, and they work just like they
 would for any other driver stack.  The calls can sleep, and can use
-- 
1.8.0

--
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: Remove support for legacy PM

[Lars-Peter Clausen]

There haven't been any I2C driver that use the legacy suspend/resume
callbacks for a while now and new drivers are supposed to use PM ops. So
remove support for legacy suspend/resume for I2C drivers.

Since there aren't any special bus specific things to do during
suspend/resume and since the PM core will automatically fallback directly to
using the device's PM ops if no bus PM ops are specified there is no need to
have any I2C bus PM ops.

Signed-off-by: Lars-Peter Clausen l...@metafoo.de
---
 drivers/i2c/i2c-core.c | 118 -
 include/linux/i2c.h|   4 --
 2 files changed, 122 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 3105bd2..f23c03b 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -695,101 +695,6 @@ static void i2c_device_shutdown(struct device *dev)
driver-shutdown(client);
 }
 
-#ifdef CONFIG_PM_SLEEP
-static int i2c_legacy_suspend(struct device *dev, pm_message_t mesg)
-{
-   struct i2c_client *client = i2c_verify_client(dev);
-   struct i2c_driver *driver;
-
-   if (!client || !dev-driver)
-   return 0;
-   driver = to_i2c_driver(dev-driver);
-   if (!driver-suspend)
-   return 0;
-   return driver-suspend(client, mesg);
-}
-
-static int i2c_legacy_resume(struct device *dev)
-{
-   struct i2c_client *client = i2c_verify_client(dev);
-   struct i2c_driver *driver;
-
-   if (!client || !dev-driver)
-   return 0;
-   driver = to_i2c_driver(dev-driver);
-   if (!driver-resume)
-   return 0;
-   return driver-resume(client);
-}
-
-static int i2c_device_pm_suspend(struct device *dev)
-{
-   const struct dev_pm_ops *pm = dev-driver ? dev-driver-pm : NULL;
-
-   if (pm)
-   return pm_generic_suspend(dev);
-   else
-   return i2c_legacy_suspend(dev, PMSG_SUSPEND);
-}
-
-static int i2c_device_pm_resume(struct device *dev)
-{
-   const struct dev_pm_ops *pm = dev-driver ? dev-driver-pm : NULL;
-
-   if (pm)
-   return pm_generic_resume(dev);
-   else
-   return i2c_legacy_resume(dev);
-}
-
-static int i2c_device_pm_freeze(struct device *dev)
-{
-   const struct dev_pm_ops *pm = dev-driver ? dev-driver-pm : NULL;
-
-   if (pm)
-   return pm_generic_freeze(dev);
-   else
-   return i2c_legacy_suspend(dev, PMSG_FREEZE);
-}
-
-static int i2c_device_pm_thaw(struct device *dev)
-{
-   const struct dev_pm_ops *pm = dev-driver ? dev-driver-pm : NULL;
-
-   if (pm)
-   return pm_generic_thaw(dev);
-   else
-   return i2c_legacy_resume(dev);
-}
-
-static int i2c_device_pm_poweroff(struct device *dev)
-{
-   const struct dev_pm_ops *pm = dev-driver ? dev-driver-pm : NULL;
-
-   if (pm)
-   return pm_generic_poweroff(dev);
-   else
-   return i2c_legacy_suspend(dev, PMSG_HIBERNATE);
-}
-
-static int i2c_device_pm_restore(struct device *dev)
-{
-   const struct dev_pm_ops *pm = dev-driver ? dev-driver-pm : NULL;
-
-   if (pm)
-   return pm_generic_restore(dev);
-   else
-   return i2c_legacy_resume(dev);
-}
-#else /* !CONFIG_PM_SLEEP */
-#define i2c_device_pm_suspend  NULL
-#define i2c_device_pm_resume   NULL
-#define i2c_device_pm_freeze   NULL
-#define i2c_device_pm_thaw NULL
-#define i2c_device_pm_poweroff NULL
-#define i2c_device_pm_restore  NULL
-#endif /* !CONFIG_PM_SLEEP */
-
 static void i2c_client_dev_release(struct device *dev)
 {
kfree(to_i2c_client(dev));
@@ -834,27 +739,12 @@ static const struct attribute_group 
*i2c_dev_attr_groups[] = {
NULL
 };
 
-static const struct dev_pm_ops i2c_device_pm_ops = {
-   .suspend = i2c_device_pm_suspend,
-   .resume = i2c_device_pm_resume,
-   .freeze = i2c_device_pm_freeze,
-   .thaw = i2c_device_pm_thaw,
-   .poweroff = i2c_device_pm_poweroff,
-   .restore = i2c_device_pm_restore,
-   SET_RUNTIME_PM_OPS(
-   pm_generic_runtime_suspend,
-   pm_generic_runtime_resume,
-   NULL
-   )
-};
-
 struct bus_type i2c_bus_type = {
.name   = i2c,
.match  = i2c_device_match,
.probe  = i2c_device_probe,
.remove = i2c_device_remove,
.shutdown   = i2c_device_shutdown,
-   .pm = i2c_device_pm_ops,
 };
 EXPORT_SYMBOL_GPL(i2c_bus_type);
 
@@ -1850,14 +1740,6 @@ int i2c_register_driver(struct module *owner, struct 
i2c_driver *driver)
if (res)
return res;
 
-   /* Drivers should switch to dev_pm_ops instead. */
-   if (driver-suspend)
-   pr_warn(i2c-core: driver [%s] using legacy suspend method\n,
-   driver-driver.name);
-   if (driver-resume)
-   pr_warn(i2c-core: driver [%s] using legacy resume method\n

Re: [PATCH] i2c: Add generic support passing secondary devices addresses

[Lars-Peter Clausen]

On 09/22/2014 12:45 PM, Mika Westerberg wrote:

On Fri, Sep 05, 2014 at 04:02:19PM +0200, Jean-Michel Hautbois wrote:

Some I2C devices have multiple addresses assigned, for example each address
corresponding to a different internal register map page of the device.
So far drivers which need support for this have handled this with a driver
specific and non-generic implementation, e.g. passing the additional address
via platform data.

This patch provides a new helper function called i2c_new_secondary_device()
which is intended to provide a generic way to get the secondary address
as well as instantiate a struct i2c_client for the secondary address.

The function expects a pointer to the primary i2c_client, a name
for the secondary address and an optional default address. The name is used
as a handle to specify which secondary address to get.

The default address is used as a fallback in case no secondary address
was explicitly specified. In case no secondary address and no default
address were specified the function returns NULL.

For now the function only supports look-up of the secondary address
from devicetree, but it can be extended in the future
to for example support board files and/or ACPI.

Signed-off-by: Jean-Michel Hautbois jean-michel.hautb...@vodalys.com


Sorry, just noticed this one.

Srinivas (CC'd) and I did similar patch series here:

http://patchwork.ozlabs.org/patch/338342/

We should probably collaborate on this one to get both DT and ACPI
supported.


Yes. The idea was to keep the interface of the API generic so it can be used 
by ACPI or other device topology description mechanisms as well.


But it looks as if the ACPI case is a bit more complex and we may need a 
revision of the API. How for example in the ACPI case do you know which 
address is which, when different parts of a chip are addressed using 
different addresses?


- Lars

--
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: Add generic support passing secondary devices addresses

[Lars-Peter Clausen]

On 09/22/2014 03:45 PM, Mika Westerberg wrote:

On Mon, Sep 22, 2014 at 03:27:36PM +0200, Lars-Peter Clausen wrote:

On 09/22/2014 12:45 PM, Mika Westerberg wrote:

On Fri, Sep 05, 2014 at 04:02:19PM +0200, Jean-Michel Hautbois wrote:

Some I2C devices have multiple addresses assigned, for example each address
corresponding to a different internal register map page of the device.
So far drivers which need support for this have handled this with a driver
specific and non-generic implementation, e.g. passing the additional address
via platform data.

This patch provides a new helper function called i2c_new_secondary_device()
which is intended to provide a generic way to get the secondary address
as well as instantiate a struct i2c_client for the secondary address.

The function expects a pointer to the primary i2c_client, a name
for the secondary address and an optional default address. The name is used
as a handle to specify which secondary address to get.

The default address is used as a fallback in case no secondary address
was explicitly specified. In case no secondary address and no default
address were specified the function returns NULL.

For now the function only supports look-up of the secondary address

from devicetree, but it can be extended in the future

to for example support board files and/or ACPI.

Signed-off-by: Jean-Michel Hautbois jean-michel.hautb...@vodalys.com


Sorry, just noticed this one.

Srinivas (CC'd) and I did similar patch series here:

http://patchwork.ozlabs.org/patch/338342/


Sorry I gave wrong link. That one is older version.

Here is the current:

http://patchwork.ozlabs.org/patch/386409/
http://patchwork.ozlabs.org/patch/386410/



We should probably collaborate on this one to get both DT and ACPI
supported.


Yes. The idea was to keep the interface of the API generic so it can be used
by ACPI or other device topology description mechanisms as well.

But it looks as if the ACPI case is a bit more complex and we may need a
revision of the API. How for example in the ACPI case do you know which
address is which, when different parts of a chip are addressed using
different addresses?


Unfortunately there is no way in ACPI 5.0 to find out which is which. So
we trust the ordering of I2cSerialBus() resources. Even that has been
problematic because some vendors then list things like SMBus ARA
addresses there in random order :-(

Our API has following signature:

int i2c_address_by_index(struct i2c_client *client, int index,
 struct i2c_board_info *info,
 struct i2c_adapter **adapter)

and we use index to find out which address to use.

Note also that in ACPI it is possible that the I2cSerialBus() resource
points to another I2C host controller, so we need to have 'adapter'
parameter as well.



Ok, looks like there are two main differences in the two implementations.

1) The ACPI one uses a integer index and the DT one uses a string index to 
lookup the device.


The problem with the index lookup is that the order is binding specific. So 
it might be different between e.g. the devicetree binding and the ACPI 
binding. This makes it quite hard to use the API in a generic way and you'd 
end up with hacks like:


if (client-dev.of_node)
index = 3;
else if (ACPI_COMPANION(client-dev))
index = 1;
else
index = 5;


So we might need a extra translation table which maps a name to a ACPI index 
and then we could use the name as the generic index in the driver.


2) The ACPI implementation returns the i2c_board_info and the adapter, while 
the DT implementation returns the instantiated I2C client device.


It might make sense to have both. I image that most drivers are just 
interested in creating a new client device and will simply pass the board 
info and adapter they got to i2c_new_device(). In this case it makes sense 
to have a helper function which already does this internally to avoid 
boilerplate code duplication.


There will probably some special cases though in which case the driver wants 
to get the adapter and the board info and then manually call 
i2c_new_device() after having done some additional steps.


- Lars
--
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: Add generic support passing secondary devices addresses

[Lars-Peter Clausen]

On 09/21/2014 07:49 PM, Wolfram Sang wrote:



This raises the first question for me: Are the additional addresses
configurable? Sadly, I can't find good documentation for the adv7604.
Otherwise, if I know I have a adv7604 and know its addresses, this
information should go into the driver and not the DT.



They are. The current driver hard codes the other addresses, but that's not
working when you have multiple adv7604s on the same I2C bus.


How is this configured? I can't imagine every address has its own
setting, but rather some 1-3 pins which select a certain block of
addresses?



Every secondary address has its own register and can be set to any valid I2C 
address.


- Lars
--
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: Add generic support passing secondary devices addresses

[Lars-Peter Clausen]

On 09/20/2014 06:49 PM, Wolfram Sang wrote:

On Fri, Sep 05, 2014 at 04:02:19PM +0200, Jean-Michel Hautbois wrote:

Some I2C devices have multiple addresses assigned, for example each address
corresponding to a different internal register map page of the device.
So far drivers which need support for this have handled this with a driver
specific and non-generic implementation, e.g. passing the additional address
via platform data.


This raises the first question for me: Are the additional addresses
configurable? Sadly, I can't find good documentation for the adv7604.
Otherwise, if I know I have a adv7604 and know its addresses, this
information should go into the driver and not the DT.



They are. The current driver hard codes the other addresses, but that's not 
working when you have multiple adv7604s on the same I2C bus.


- Lars
--
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/2] Allow DT parsing of secondary devices

[Lars-Peter Clausen]

On 08/29/2014 05:15 PM, Jean-Michel Hautbois wrote:

This is based on reg and reg-names in DT.
Example:

reg = 0x10 0x20 0x30;
reg-names = main, io, test;

This function will create dummy devices io and test
with addresses 0x20 and 0x30 respectively.

Signed-off-by: Jean-Michel Hautbois jean-michel.hautb...@vodalys.com


This needs a better description explaining the problem and how it is solved. 
How about


i2c: Add generic support passing secondary devices addresses

Some I2C devices have multiple addresses assigned, for example each address 
corresponding to a different internal register map page of the device. So 
far drivers which need support for this have handled this with a driver 
specific and non-generic implementation, e.g. passing the additional address 
via platform data.


This patch provides a new helper function called i2c_new_secondary_device() 
which is intended to provide a generic way to get the secondary address as 
well as instantiate a struct i2c_client for the secondary address. The 
function expects a pointer to the primary i2c_client, a name for the 
secondary address and an optional default address. The name is used as a 
handle to specify which secondary address to get. The default address is 
used as a fallback in case no secondary address was explicitly specified. In 
case no secondary address and no default address were specified the function 
returns NULL.


For now the function only supports look-up of the secondary address from 
devicetree, but it can be extended in the future to for example support 
board files and/or ACPI.




---
  drivers/i2c/i2c-core.c | 20 
  include/linux/i2c.h|  6 ++
  2 files changed, 26 insertions(+)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 632057a..5eb414d 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -798,6 +798,26 @@ struct i2c_client *i2c_new_dummy(struct i2c_adapter 
*adapter, u16 address)
  }
  EXPORT_SYMBOL_GPL(i2c_new_dummy);



The function needs a kernel doc description.


+struct i2c_client *i2c_new_secondary_device(struct i2c_client *client,
+   const char *name,
+   u32 default_addr)


The I2C framework commonly uses u16 for the type of a I2C address.



+{
+   int i, addr;


addr needs to be u32 here since it is passed to of_property_read_u32_index().


+   struct device_node *np;
+
+   np = client-dev.of_node;


of_node can be NULL, this needs to be handled. Ideally by using the default 
address.



+   i = of_property_match_string(np, reg-names, name);
+   if (i = 0)
+   of_property_read_u32_index(np, reg, i, addr);


of_property_read_u32_index() can fail, this needs to be handled.


+   else
+   addr = default_addr;


If no address was specified and default_addr is 0 the function should return 
NULL.



+
+   dev_dbg(client-adapter-dev, Address for %s : 0x%x\n, name, addr);
+   return i2c_new_dummy(client-adapter, addr);
+}
+EXPORT_SYMBOL_GPL(i2c_new_secondary_device);
+
+
  /* - 
*/

  /* I2C bus adapters -- one roots each I2C or SMBUS segment */
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index a95efeb..2d143d7 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -322,6 +322,12 @@ extern int i2c_probe_func_quick_read(struct i2c_adapter *, 
unsigned short addr);
  extern struct i2c_client *
  i2c_new_dummy(struct i2c_adapter *adap, u16 address);

+/* Use reg/reg-names in DT in order to get extra addresses */


The comment should go into i2c-core.c and be in proper kernel doc fully 
explaining the function, its parameters and the behavior.



+extern struct i2c_client *
+i2c_new_secondary_device(struct i2c_client *client,
+   const char *name,
+   u32 default_addr);
+
  extern void i2c_unregister_device(struct i2c_client *);
  #endif /* I2C */




--
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] adv7604: Use DT parsing in dummy creation

[Lars-Peter Clausen]

On 08/29/2014 05:15 PM, Jean-Michel Hautbois wrote:

This patch uses DT in order to parse addresses for dummy devices of adv7604.
The ADV7604 has thirteen 256-byte maps that can be accessed via the main
I²C ports. Each map has it own I²C address and acts
as a standard slave device on the I²C bus.

If nothing is defined, it uses default addresses.
The main prupose is using two adv76xx on the same i2c bus.


Ideally this patch is split up in two patches. One patch adding support for 
i2c_new_secondary_device() and one patch adding support for DT for the adv7604.


[...]

+static const char const *adv7604_secondary_names[] = {
+   main, /* ADV7604_PAGE_IO */


How about [ADV7604_PAGE_IO] = main, instead of having the comment, this 
makes things more explicit.



+   avlink, /* ADV7604_PAGE_AVLINK */
+   cec, /* ADV7604_PAGE_CEC */
+   infoframe, /* ADV7604_PAGE_INFOFRAME */
+   esdp, /* ADV7604_PAGE_ESDP */
+   dpp, /* ADV7604_PAGE_DPP */
+   afe, /* ADV7604_PAGE_AFE */
+   rep, /* ADV7604_PAGE_REP */
+   edid, /* ADV7604_PAGE_EDID */
+   hdmi, /* ADV7604_PAGE_HDMI */
+   test, /* ADV7604_PAGE_TEST */
+   cp, /* ADV7604_PAGE_CP */
+   vdp /* ADV7604_PAGE_VDP */
+};
+
  /* --- */

  static inline struct adv7604_state *to_state(struct v4l2_subdev *sd)
@@ -2528,13 +2544,31 @@ static void adv7604_unregister_clients(struct 
adv7604_state *state)
  }

  static struct i2c_client *adv7604_dummy_client(struct v4l2_subdev *sd,
-   u8 addr, u8 io_reg)
+   unsigned int i)
  {
struct i2c_client *client = v4l2_get_subdevdata(sd);
+   struct adv7604_platform_data *pdata = client-dev.platform_data;
+   unsigned int io_reg = 0xf2 + i;
+   unsigned int default_addr = io_read(sd, io_reg)  1;
+   struct i2c_client *new_client;
+
+   if (IS_ENABLED(CONFIG_OF)) {


No CONFIG_OF. i2c_new_secondary_device() is supposed to be the generic 
method of instantiating the secondary i2c_client, regardless of how the 
address is specified. For this driver we still need to keep the old method 
of instantiation via platform data for legacy reasons for now. So what this 
should look like is:



if (pdata  pdata-i2c_addresses[i])
new_client = i2c_new_dummy(client-adapter,
pdata-i2c_addresses[i]);
else
new_client = i2c_new_secondary_device(client,
adv7604_secondary_names[i], default_addr);



+   /* Try to find it in DT */
+   new_client = i2c_new_secondary_device(client,
+   adv7604_secondary_names[i], default_addr);
+   } else if (pdata) {
+   if (pdata-i2c_addresses[i])
+   new_client = i2c_new_dummy(client-adapter,
+   pdata-i2c_addresses[i]);
+   else
+   new_client = i2c_new_dummy(client-adapter,
+   default_addr);
+   }


[...]
--
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/2] Allow DT parsing of secondary devices

[Lars-Peter Clausen]

On 08/29/2014 05:15 PM, Jean-Michel Hautbois wrote:

This is based on reg and reg-names in DT.
Example:

reg = 0x10 0x20 0x30;
reg-names = main, io, test;

This function will create dummy devices io and test
with addresses 0x20 and 0x30 respectively.

Signed-off-by: Jean-Michel Hautbois jean-michel.hautb...@vodalys.com


This needs a better description explaining the problem and how it is solved. 
How about


i2c: Add generic support passing secondary devices addresses

Some I2C devices have multiple addresses assigned, for example each address 
corresponding to a different internal register map page of the device. So 
far drivers which need support for this have handled this with a driver 
specific and non-generic implementation, e.g. passing the additional address 
via platform data.


This patch provides a new helper function called i2c_new_secondary_device() 
which is intended to provide a generic way to get the secondary address as 
well as instantiate a struct i2c_client for the secondary address. The 
function expects a pointer to the primary i2c_client, a name for the 
secondary address and an optional default address. The name is used as a 
handle to specify which secondary address to get. The default address is 
used as a fallback in case no secondary address was explicitly specified. In 
case no secondary address and no default address were specified the function 
returns NULL.


For now the function only supports look-up of the secondary address from 
devicetree, but it can be extended in the future to for example support 
board files and/or ACPI.



The patch should also update the I2C devicetree bindings documentation to 
explain how bindings for devices with multiple addresses work.



---
  drivers/i2c/i2c-core.c | 20 
  include/linux/i2c.h|  6 ++
  2 files changed, 26 insertions(+)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 632057a..5eb414d 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -798,6 +798,26 @@ struct i2c_client *i2c_new_dummy(struct i2c_adapter 
*adapter, u16 address)
  }
  EXPORT_SYMBOL_GPL(i2c_new_dummy);



The function needs a kernel doc description.


+struct i2c_client *i2c_new_secondary_device(struct i2c_client *client,
+   const char *name,
+   u32 default_addr)


The I2C framework commonly uses u16 for the type of a I2C address.



+{
+   int i, addr;


addr needs to be u32 here since it is passed to of_property_read_u32_index().


+   struct device_node *np;
+
+   np = client-dev.of_node;


of_node can be NULL, this needs to be handled. Ideally by using the default 
address.



+   i = of_property_match_string(np, reg-names, name);
+   if (i = 0)
+   of_property_read_u32_index(np, reg, i, addr);


of_property_read_u32_index() can fail, this needs to be handled.


+   else
+   addr = default_addr;


If no address was specified and default_addr is 0 the function should return 
NULL.



+
+   dev_dbg(client-adapter-dev, Address for %s : 0x%x\n, name, addr);
+   return i2c_new_dummy(client-adapter, addr);
+}
+EXPORT_SYMBOL_GPL(i2c_new_secondary_device);
+
+
  /* - 
*/

  /* I2C bus adapters -- one roots each I2C or SMBUS segment */
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index a95efeb..2d143d7 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -322,6 +322,12 @@ extern int i2c_probe_func_quick_read(struct i2c_adapter *, 
unsigned short addr);
  extern struct i2c_client *
  i2c_new_dummy(struct i2c_adapter *adap, u16 address);

+/* Use reg/reg-names in DT in order to get extra addresses */


The comment should go into i2c-core.c and be in proper kernel doc fully 
explaining the function, its parameters and the behavior.



+extern struct i2c_client *
+i2c_new_secondary_device(struct i2c_client *client,
+   const char *name,
+   u32 default_addr);
+
  extern void i2c_unregister_device(struct i2c_client *);
  #endif /* I2C */




--
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 1/2] i2c-mux-gpio: test if the gpio can sleep

[Lars-Peter Clausen]

On 10/10/2013 10:39 AM, Ionut Nicu wrote:
 Some gpio chips may have get/set operations that
 can sleep. For this type of chips we must use the
 _cansleep() version of gpio_set_value.
 
 Signed-off-by: Ionut Nicu ioan.nicu@nsn.com
 ---
  drivers/i2c/muxes/i2c-mux-gpio.c |   11 ---
  1 files changed, 8 insertions(+), 3 deletions(-)
 
 diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c 
 b/drivers/i2c/muxes/i2c-mux-gpio.c
 index a764da7..b5f17ef 100644
 --- a/drivers/i2c/muxes/i2c-mux-gpio.c
 +++ b/drivers/i2c/muxes/i2c-mux-gpio.c
 @@ -27,11 +27,16 @@ struct gpiomux {
  
  static void i2c_mux_gpio_set(const struct gpiomux *mux, unsigned val)
  {
 + unsigned gpio;
   int i;
  
 - for (i = 0; i  mux-data.n_gpios; i++)
 - gpio_set_value(mux-gpio_base + mux-data.gpios[i],
 -val  (1  i));
 + for (i = 0; i  mux-data.n_gpios; i++) {
 + gpio = mux-gpio_base + mux-data.gpios[i];
 + if (gpio_cansleep(gpio))
 + gpio_set_value_cansleep(gpio, val  (1  i));
 + else
 + gpio_set_value(gpio, val  (1  i));

The proper way to do this is just always use the _cansleep() version.
gpio_set_value() only works for chips which do not sleep,
gpio_set_value_cansleep() works for both those who do sleep and those who do
not.

 + }
  }
  
  static int i2c_mux_gpio_select(struct i2c_adapter *adap, void *data, u32 
 chan)
 

--
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 1/2] i2c-mux-gpio: test if the gpio can sleep

[Lars-Peter Clausen]

On 10/10/2013 09:43 PM, Wolfram Sang wrote:

On Thu, Oct 10, 2013 at 10:46:41AM +0200, Lars-Peter Clausen wrote:


+   if (gpio_cansleep(gpio))
+   gpio_set_value_cansleep(gpio, val  (1  i));
+   else
+   gpio_set_value(gpio, val  (1  i));


The proper way to do this is just always use the _cansleep() version.
gpio_set_value() only works for chips which do not sleep,
gpio_set_value_cansleep() works for both those who do sleep and those who do
not.


To the gpio-list: Has it been considered to have sth. like
gpio_set_value and gpio_set_value_nosleep? I'd think it makes more sense
to have the specific function have the specific name.


It has been a few times, but I think the conclusion has always been that it is 
now too late to invert the semantics of gpio_set_value(). If you want to look 
up the discussions the keyword is gpio_set_value_atomic().


- Lars

--
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/3] i2c: xilinx: Set tx direction in write operation

[Lars-Peter Clausen]

On 10/04/2013 01:55 PM, Wolfram Sang wrote:
 On Fri, Oct 04, 2013 at 11:53:49AM +0200, Michal Simek wrote:
 On 10/04/2013 07:46 AM, Wolfram Sang wrote:

 +  cr = xiic_getreg32(i2c, XIIC_CR_REG_OFFSET);
 +  cr |= XIIC_CR_DIR_IS_TX_MASK;
 +  xiic_setreg32(i2c, XIIC_CR_REG_OFFSET, cr);
 +

 Is there no need to clear the bit again when receiving?

 This bit is cleared in xiic_xfer() - xiic_start_xfer() -xiic_reinit()

 xiic_setreg8(i2c, XIIC_CR_REG_OFFSET, XIIC_CR_TX_FIFO_RESET_MASK);
 
 A bit implicit, but OK.
 
 And did
 transferring ever work if this bit was never set before?

 I really don't know. We have switched from old driver to this new mainline 
 one
 and based on our eeprom testing we have found that this bit hasn't been 
 setup properly.

 It is described here.
 http://www.xilinx.com/support/documentation/ip_documentation/axi_iic/v1_02_a/axi_iic_ds756.pdf
 page 28 - step 3.

 IIC Master Transmitter with a Repeated Start
 1. Write the IIC device address to the TX_FIFO.
 2. Write data to TX_FIFO.
 3. Write to Control Register (CR) to set MSMS = 1 and TX = 1.
 4. Continue writing data to TX_FIFO.
 5. Wait for transmit FIFO empty interrupt. This implies the IIC has 
 throttled the bus.
 6. Write to CR to set RSTA = 1.
 
 Repeated start is not happening in the driver as well, or am I
 overlooking something?
 
 7. Write IIC device address to TX_FIFO.
 8. Write all data except last byte to TX_FIFO.
 9. Wait for transmit FIFO empty interrupt. This implies the IIC has 
 throttled the bus.
 10. Write to CR to set MSMS = 0. The IIC generates a stop condition at the 
 end of the last byte.
 11. Write last byte of data to TX_FIFO.
 
 CCing more people who worked on the driver in the past and might have
 experiences

The current version works fine here. The driver uses whats described in the
datasheet as dynamic controller logic flow and not the standard
controller logic flow. The sequence Michal mentioned above is from the
standard controller logic flow section.

- Lars

--
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/8] [media] s5c73m3: Don't use i2c_client-driver

[Lars-Peter Clausen]

The 'driver' field of the i2c_client struct is redundant and is going to be
removed. The results of the expressions 'client-driver.driver-field' and
'client-dev.driver-field' are identical, so replace all occurrences of the
former with the later.

Signed-off-by: Lars-Peter Clausen l...@metafoo.de
Cc: Kyungmin Park kyungmin.p...@samsung.com
Cc: Andrzej Hajda a.ha...@samsung.com
---
 drivers/media/i2c/s5c73m3/s5c73m3-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/s5c73m3/s5c73m3-core.c 
b/drivers/media/i2c/s5c73m3/s5c73m3-core.c
index b76ec0e..1083890 100644
--- a/drivers/media/i2c/s5c73m3/s5c73m3-core.c
+++ b/drivers/media/i2c/s5c73m3/s5c73m3-core.c
@@ -1581,7 +1581,7 @@ static int s5c73m3_probe(struct i2c_client *client,
oif_sd = state-oif_sd;
 
v4l2_subdev_init(sd, s5c73m3_subdev_ops);
-   sd-owner = client-driver-driver.owner;
+   sd-owner = client-dev.driver-owner;
v4l2_set_subdevdata(sd, state);
strlcpy(sd-name, S5C73M3, sizeof(sd-name));
 
-- 
1.8.0

--
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 8/8] i2c: Remove redundant 'driver' field from the i2c_client struct

[Lars-Peter Clausen]

The 'driver' field of the i2c_client struct is redundant. The same data can be
accessed through to_i2c_driver(client-dev.driver). The generated code for both
approaches in more or less the same.

E.g. on ARM the expression client-driver-command(...) generates

...
ldr r3, [r0, #28]
ldr r3, [r3, #32]
blx r3
...

and the expression to_i2c_driver(client-dev.driver)-command(...) generates

...
ldr r3, [r0, #160]
ldr r3, [r3, #-4]
blx r3
...

Other architectures will generate similar code.

All users of the 'driver' field outside of the I2C core have already been
converted. So this only leaves the core itself. This patch converts the
remaining few users in the I2C core and then removes the 'driver' field from the
i2c_client struct.

Signed-off-by: Lars-Peter Clausen l...@metafoo.de
---
 drivers/i2c/i2c-core.c  | 21 -
 drivers/i2c/i2c-smbus.c | 10 ++
 include/linux/i2c.h |  2 --
 3 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 29d3f04..430c001 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -248,17 +248,16 @@ static int i2c_device_probe(struct device *dev)
driver = to_i2c_driver(dev-driver);
if (!driver-probe || !driver-id_table)
return -ENODEV;
-   client-driver = driver;
+
if (!device_can_wakeup(client-dev))
device_init_wakeup(client-dev,
client-flags  I2C_CLIENT_WAKE);
dev_dbg(dev, probe\n);
 
status = driver-probe(client, i2c_match_id(driver-id_table, client));
-   if (status) {
-   client-driver = NULL;
+   if (status)
i2c_set_clientdata(client, NULL);
-   }
+
return status;
 }
 
@@ -279,10 +278,9 @@ static int i2c_device_remove(struct device *dev)
dev-driver = NULL;
status = 0;
}
-   if (status == 0) {
-   client-driver = NULL;
+   if (status == 0)
i2c_set_clientdata(client, NULL);
-   }
+
return status;
 }
 
@@ -1606,9 +1604,14 @@ static int i2c_cmd(struct device *dev, void *_arg)
 {
struct i2c_client   *client = i2c_verify_client(dev);
struct i2c_cmd_arg  *arg = _arg;
+   struct i2c_driver   *driver;
+
+   if (!client || !client-dev.driver)
+   return 0;
 
-   if (client  client-driver  client-driver-command)
-   client-driver-command(client, arg-cmd, arg-arg);
+   driver = to_i2c_driver(client-dev.driver);
+   if (driver-command)
+   driver-command(client, arg-cmd, arg-arg);
return 0;
 }
 
diff --git a/drivers/i2c/i2c-smbus.c b/drivers/i2c/i2c-smbus.c
index 44d4c60..c99b229 100644
--- a/drivers/i2c/i2c-smbus.c
+++ b/drivers/i2c/i2c-smbus.c
@@ -46,6 +46,7 @@ static int smbus_do_alert(struct device *dev, void *addrp)
 {
struct i2c_client *client = i2c_verify_client(dev);
struct alert_data *data = addrp;
+   struct i2c_driver *driver;
 
if (!client || client-addr != data-addr)
return 0;
@@ -54,12 +55,13 @@ static int smbus_do_alert(struct device *dev, void *addrp)
 
/*
 * Drivers should either disable alerts, or provide at least
-* a minimal handler.  Lock so client-driver won't change.
+* a minimal handler.  Lock so the driver won't change.
 */
device_lock(dev);
-   if (client-driver) {
-   if (client-driver-alert)
-   client-driver-alert(client, data-flag);
+   if (client-dev.driver) {
+   driver = to_i2c_driver(client-dev.driver);
+   if (driver-alert)
+   driver-alert(client, data-flag);
else
dev_warn(client-dev, no driver alert()!\n);
} else
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 2ab11dc..eff50e0 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -205,7 +205,6 @@ struct i2c_driver {
  * @name: Indicates the type of the device, usually a chip name that's
  * generic enough to hide second-sourcing and compatible revisions.
  * @adapter: manages the bus segment hosting this I2C device
- * @driver: device's driver, hence pointer to access routines
  * @dev: Driver model device node for the slave.
  * @irq: indicates the IRQ generated by this device (if any)
  * @detected: member of an i2c_driver.clients list or i2c-core's
@@ -222,7 +221,6 @@ struct i2c_client {
/* _LOWER_ 7 bits   */
char name[I2C_NAME_SIZE];
struct i2c_adapter *adapter;/* the adapter we sit on*/
-   struct i2c_driver *driver;  /* and our access routines

[PATCH 6/8] ALSA: ppc: keywest: Don't use i2c_client-driver

[Lars-Peter Clausen]

The 'driver' field of the i2c_client struct is redundant and is going to be
removed. Use 'to_i2c_driver(client-dev.driver)' instead to get direct
access to the i2c_driver struct.

Signed-off-by: Lars-Peter Clausen l...@metafoo.de
---
 sound/ppc/keywest.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/ppc/keywest.c b/sound/ppc/keywest.c
index 01aecc2..0d1c27e 100644
--- a/sound/ppc/keywest.c
+++ b/sound/ppc/keywest.c
@@ -65,7 +65,7 @@ static int keywest_attach_adapter(struct i2c_adapter *adapter)
 * already bound. If not it means binding failed, and then there
 * is no point in keeping the device instantiated.
 */
-   if (!keywest_ctx-client-driver) {
+   if (!keywest_ctx-client-dev.driver) {
i2c_unregister_device(keywest_ctx-client);
keywest_ctx-client = NULL;
return -ENODEV;
@@ -76,7 +76,7 @@ static int keywest_attach_adapter(struct i2c_adapter *adapter)
 * This is safe because i2c-core holds the core_lock mutex for us.
 */
list_add_tail(keywest_ctx-client-detected,
- keywest_ctx-client-driver-clients);
+ to_i2c_driver(keywest_ctx-client-dev.driver)-clients);
return 0;
 }
 
-- 
1.8.0

--
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 4/8] drm: encoder_slave: Don't use i2c_client-driver

[Lars-Peter Clausen]

The 'driver' field of the i2c_client struct is redundant and is going to be
removed. The results of the expressions 'client-driver.driver-field' and
'client-dev.driver-field' are identical, so replace all occurrences of the
former with the later. To get direct access to the i2c_driver struct use
'to_i2c_driver(client-dev.driver)'.

Signed-off-by: Lars-Peter Clausen l...@metafoo.de
---
 drivers/gpu/drm/drm_encoder_slave.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_encoder_slave.c 
b/drivers/gpu/drm/drm_encoder_slave.c
index 0cfb60f..d18b88b 100644
--- a/drivers/gpu/drm/drm_encoder_slave.c
+++ b/drivers/gpu/drm/drm_encoder_slave.c
@@ -67,12 +67,12 @@ int drm_i2c_encoder_init(struct drm_device *dev,
goto fail;
}
 
-   if (!client-driver) {
+   if (!client-dev.driver) {
err = -ENODEV;
goto fail_unregister;
}
 
-   module = client-driver-driver.owner;
+   module = client-dev.driver-owner;
if (!try_module_get(module)) {
err = -ENODEV;
goto fail_unregister;
@@ -80,7 +80,7 @@ int drm_i2c_encoder_init(struct drm_device *dev,
 
encoder-bus_priv = client;
 
-   encoder_drv = to_drm_i2c_encoder_driver(client-driver);
+   encoder_drv = 
to_drm_i2c_encoder_driver(to_i2c_driver(client-dev.driver));
 
err = encoder_drv-encoder_init(client, dev, encoder);
if (err)
@@ -111,7 +111,7 @@ void drm_i2c_encoder_destroy(struct drm_encoder 
*drm_encoder)
 {
struct drm_encoder_slave *encoder = to_encoder_slave(drm_encoder);
struct i2c_client *client = drm_i2c_encoder_get_client(drm_encoder);
-   struct module *module = client-driver-driver.owner;
+   struct module *module = client-dev.driver-owner;
 
i2c_unregister_device(client);
encoder-bus_priv = NULL;
-- 
1.8.0

--
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/8] i2c: Remove redundant driver field from the i2c_client struct

[Lars-Peter Clausen]

Hi,

This series removes the redundant driver field from the i2c_client struct. The
field is redundant since the same pointer can be accessed through
to_i2c_driver(client-dev.driver). The commit log suggests that the field has
been around since forever (since before v2.6.12-rc2) and it looks as if it was
simply forgotten to remove it during the conversion of the i2c framework to the
generic device driver model.

Nevertheless there are a still a few users of the field around. This series
first updates all users to use an alternative method of accessing the same data
and then the last patch removes the driver field from the i2c_client struct.

Note that due to this changes on most architectures neither the code size nor
the type of generated instructions will change. This is due to the fact that we
aren't really interested in the pointer value itself, but rather want to
dereference it to access one of the fields of the struct. offset_of() (and hence
to_i2c_driver) works by subtracting a offset from the pointer, so the compiler
can internally create the sum of these two offsets and use that to access the
field.

E.g. on ARM the expression client-driver-command(...) generates

...
ldr r3, [r0, #28]
ldr r3, [r3, #32]
blx r3
...

and the expression to_i2c_driver(client-dev.driver)-command(...) generates

...
ldr r3, [r0, #160]
ldr r3, [r3, #-4]
blx r3
...

Other architectures will generate similar code.

The most common pattern is to use the i2c_driver to get to the device_driver
struct embedded in it. The same struct can easily be accessed through the device
struct embedded in the i2c_client struct.  E.g. client-driver-driver.field can
be replaced by client-dev.driver-field. Here again the generated code is
almost identical and only the offsets differ.

E.g. on ARM the expression 'client-driver-driver.owner' generates

ldr r3, [r0, #28]
ldr r0, [r3, #44]

and 'client-dev.driver-owner' generates

ldr r3, [r0, #160]
ldr r0, [r3, #8]

The kernel overall code size is slightly reduced since the code that manages the
driver field is removed and of course also the runtime memory footprint of the
i2c_client struct is reduced.

- Lars

Lars-Peter Clausen (8):
  [media] s5c73m3: Don't use i2c_client-driver
  [media] exynos4-is: Don't use i2c_client-driver
  [media] core: Don't use i2c_client-driver
  drm: encoder_slave: Don't use i2c_client-driver
  drm: nouveau: Don't use i2c_client-driver
  ALSA: ppc: keywest: Don't use i2c_client-driver
  ASoC: imx-wm8962: Don't use i2c_client-driver
  i2c: Remove redundant 'driver' field from the i2c_client struct

 drivers/gpu/drm/drm_encoder_slave.c|  8 
 drivers/gpu/drm/nouveau/core/subdev/therm/ic.c |  3 ++-
 drivers/i2c/i2c-core.c | 21 -
 drivers/i2c/i2c-smbus.c| 10 ++
 drivers/media/i2c/s5c73m3/s5c73m3-core.c   |  2 +-
 drivers/media/platform/exynos4-is/media-dev.c  |  6 +++---
 drivers/media/v4l2-core/tuner-core.c   |  6 +++---
 drivers/media/v4l2-core/v4l2-common.c  | 10 +-
 include/linux/i2c.h|  2 --
 include/media/v4l2-common.h|  2 +-
 sound/ppc/keywest.c|  4 ++--
 sound/soc/fsl/imx-wm8962.c |  2 +-
 12 files changed, 40 insertions(+), 36 deletions(-)

-- 
1.8.0

--
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 5/8] drm: nouveau: Don't use i2c_client-driver

[Lars-Peter Clausen]

The 'driver' field of the i2c_client struct is redundant and is going to be
removed. Use 'to_i2c_driver(client-dev.driver)' instead to get direct access to
the i2c_driver struct.

Signed-off-by: Lars-Peter Clausen l...@metafoo.de
Cc: Martin Peres martin.pe...@labri.fr
---
 drivers/gpu/drm/nouveau/core/subdev/therm/ic.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/core/subdev/therm/ic.c 
b/drivers/gpu/drm/nouveau/core/subdev/therm/ic.c
index 8b3adec..eae939d 100644
--- a/drivers/gpu/drm/nouveau/core/subdev/therm/ic.c
+++ b/drivers/gpu/drm/nouveau/core/subdev/therm/ic.c
@@ -41,7 +41,8 @@ probe_monitoring_device(struct nouveau_i2c_port *i2c,
if (!client)
return false;
 
-   if (!client-driver || client-driver-detect(client, info)) {
+   if (!client-dev.driver ||
+   to_i2c_driver(client-dev.driver)-detect(client, info)) {
i2c_unregister_device(client);
return false;
}
-- 
1.8.0

--
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 7/8] ASoC: imx-wm8962: Don't use i2c_client-driver

[Lars-Peter Clausen]

The 'driver' field of the i2c_client struct is redundant and is going to be
removed. Check i2c_client-dev.driver instead to see if a driver is bound to the
device.

Signed-off-by: Lars-Peter Clausen l...@metafoo.de
---
 sound/soc/fsl/imx-wm8962.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/fsl/imx-wm8962.c b/sound/soc/fsl/imx-wm8962.c
index 6c60666..f84ecbf 100644
--- a/sound/soc/fsl/imx-wm8962.c
+++ b/sound/soc/fsl/imx-wm8962.c
@@ -215,7 +215,7 @@ static int imx_wm8962_probe(struct platform_device *pdev)
goto fail;
}
codec_dev = of_find_i2c_device_by_node(codec_np);
-   if (!codec_dev || !codec_dev-driver) {
+   if (!codec_dev || !codec_dev-dev.driver) {
dev_err(pdev-dev, failed to find codec platform device\n);
ret = -EINVAL;
goto fail;
-- 
1.8.0

--
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/8] [media] core: Don't use i2c_client-driver

[Lars-Peter Clausen]

The 'driver' field of the i2c_client struct is redundant and is going to be
removed. The results of the expressions 'client-driver.driver-field' and
'client-dev.driver-field' are identical, so replace all occurrences of the
former with the later.

Signed-off-by: Lars-Peter Clausen l...@metafoo.de
---
 drivers/media/v4l2-core/tuner-core.c  |  6 +++---
 drivers/media/v4l2-core/v4l2-common.c | 10 +-
 include/media/v4l2-common.h   |  2 +-
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/media/v4l2-core/tuner-core.c 
b/drivers/media/v4l2-core/tuner-core.c
index ddc9379..4133af0 100644
--- a/drivers/media/v4l2-core/tuner-core.c
+++ b/drivers/media/v4l2-core/tuner-core.c
@@ -43,7 +43,7 @@
 
 #define UNSET (-1U)
 
-#define PREFIX (t-i2c-driver-driver.name)
+#define PREFIX (t-i2c-dev.driver-name)
 
 /*
  * Driver modprobe parameters
@@ -452,7 +452,7 @@ static void set_type(struct i2c_client *c, unsigned int 
type,
}
 
tuner_dbg(%s %s I2C addr 0x%02x with type %d used for 0x%02x\n,
- c-adapter-name, c-driver-driver.name, c-addr  1, type,
+ c-adapter-name, c-dev.driver-name, c-addr  1, type,
  t-mode_mask);
return;
 
@@ -556,7 +556,7 @@ static void tuner_lookup(struct i2c_adapter *adap,
int mode_mask;
 
if (pos-i2c-adapter != adap ||
-   strcmp(pos-i2c-driver-driver.name, tuner))
+   strcmp(pos-i2c-dev.driver-name, tuner))
continue;
 
mode_mask = pos-mode_mask;
diff --git a/drivers/media/v4l2-core/v4l2-common.c 
b/drivers/media/v4l2-core/v4l2-common.c
index 037d7a5..433d6d7 100644
--- a/drivers/media/v4l2-core/v4l2-common.c
+++ b/drivers/media/v4l2-core/v4l2-common.c
@@ -236,14 +236,14 @@ void v4l2_i2c_subdev_init(struct v4l2_subdev *sd, struct 
i2c_client *client,
v4l2_subdev_init(sd, ops);
sd-flags |= V4L2_SUBDEV_FL_IS_I2C;
/* the owner is the same as the i2c_client's driver owner */
-   sd-owner = client-driver-driver.owner;
+   sd-owner = client-dev.driver-owner;
sd-dev = client-dev;
/* i2c_client and v4l2_subdev point to one another */
v4l2_set_subdevdata(sd, client);
i2c_set_clientdata(client, sd);
/* initialize name */
snprintf(sd-name, sizeof(sd-name), %s %d-%04x,
-   client-driver-driver.name, i2c_adapter_id(client-adapter),
+   client-dev.driver-name, i2c_adapter_id(client-adapter),
client-addr);
 }
 EXPORT_SYMBOL_GPL(v4l2_i2c_subdev_init);
@@ -274,11 +274,11 @@ struct v4l2_subdev *v4l2_i2c_new_subdev_board(struct 
v4l2_device *v4l2_dev,
   loaded. This delay-load mechanism doesn't work if other drivers
   want to use the i2c device, so explicitly loading the module
   is the best alternative. */
-   if (client == NULL || client-driver == NULL)
+   if (client == NULL || client-dev.driver == NULL)
goto error;
 
/* Lock the module so we can safely get the v4l2_subdev pointer */
-   if (!try_module_get(client-driver-driver.owner))
+   if (!try_module_get(client-dev.driver-owner))
goto error;
sd = i2c_get_clientdata(client);
 
@@ -287,7 +287,7 @@ struct v4l2_subdev *v4l2_i2c_new_subdev_board(struct 
v4l2_device *v4l2_dev,
if (v4l2_device_register_subdev(v4l2_dev, sd))
sd = NULL;
/* Decrease the module use count to match the first try_module_get. */
-   module_put(client-driver-driver.owner);
+   module_put(client-dev.driver-owner);
 
 error:
/* If we have a client but no subdev, then something went wrong and
diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
index 16550c4..a707529 100644
--- a/include/media/v4l2-common.h
+++ b/include/media/v4l2-common.h
@@ -35,7 +35,7 @@
printk(level %s %d-%04x:  fmt, name, i2c_adapter_id(adapter), addr , 
## arg)
 
 #define v4l_client_printk(level, client, fmt, arg...)  \
-   v4l_printk(level, (client)-driver-driver.name, (client)-adapter, \
+   v4l_printk(level, (client)-dev.driver-name, (client)-adapter, \
   (client)-addr, fmt , ## arg)
 
 #define v4l_err(client, fmt, arg...) \
-- 
1.8.0

--
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/8] [media] exynos4-is: Don't use i2c_client-driver

[Lars-Peter Clausen]

The 'driver' field of the i2c_client struct is redundant and is going to be
removed. The results of the expressions 'client-driver.driver-field' and
'client-dev.driver-field' are identical, so replace all occurrences of the
former with the later.

Signed-off-by: Lars-Peter Clausen l...@metafoo.de
Cc: Kyungmin Park kyungmin.p...@samsung.com
Cc: Sylwester Nawrocki s.nawro...@samsung.com
---
 drivers/media/platform/exynos4-is/media-dev.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/exynos4-is/media-dev.c 
b/drivers/media/platform/exynos4-is/media-dev.c
index a835112..7a4ee4c 100644
--- a/drivers/media/platform/exynos4-is/media-dev.c
+++ b/drivers/media/platform/exynos4-is/media-dev.c
@@ -411,8 +411,8 @@ static int fimc_md_of_add_sensor(struct fimc_md *fmd,
 
device_lock(client-dev);
 
-   if (!client-driver ||
-   !try_module_get(client-driver-driver.owner)) {
+   if (!client-dev.driver ||
+   !try_module_get(client-dev.driver-owner)) {
ret = -EPROBE_DEFER;
v4l2_info(fmd-v4l2_dev, No driver found for %s\n,
node-full_name);
@@ -442,7 +442,7 @@ static int fimc_md_of_add_sensor(struct fimc_md *fmd,
fmd-num_sensors++;
 
 mod_put:
-   module_put(client-driver-driver.owner);
+   module_put(client-dev.driver-owner);
 dev_put:
device_unlock(client-dev);
put_device(client-dev);
-- 
1.8.0

--
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/6] i2c: Ignore return value of i2c_del_adapter()

[Lars-Peter Clausen]

Hi Jean,

On 03/30/2013 08:55 AM, Jean Delvare wrote:
 Hi Lars-Peter,
 
 On Sat,  9 Mar 2013 19:16:46 +0100, Lars-Peter Clausen wrote:
 i2c_del_adapter() always returns 0.
 
 I beg you pardon? i2c_del_adapter() starts with:
 
 int i2c_del_adapter(struct i2c_adapter *adap)
 {
   int res = 0;
   struct i2c_adapter *found;
   struct i2c_client *client, *next;
 
   /* First make sure that this adapter was ever added */
   mutex_lock(core_lock);
   found = idr_find(i2c_adapter_idr, adap-nr);
   mutex_unlock(core_lock);
   if (found != adap) {
   pr_debug(i2c-core: attempting to delete unregistered 
adapter [%s]\n, adap-name);
   return -EINVAL;
   }
 
   /* Tell drivers about this removal */
   mutex_lock(core_lock);
   res = bus_for_each_drv(i2c_bus_type, NULL, adap,
  __process_removed_adapter);
   mutex_unlock(core_lock);
   if (res)
   return res;
 (...)
 
 So, no, it doesn't always return 0.
 

Patch 1 and 2 in this series remove those two instances:
https://lkml.org/lkml/2013/3/9/72
https://lkml.org/lkml/2013/3/9/86

For an explanation why this should be done please take a look at the cover
letter to this patch series https://lkml.org/lkml/2013/3/9/73

- Lars
--
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 0/6] Make return type of i2c_del_adapter() (and i2c_del_mux_adapter()) void

[Lars-Peter Clausen]

On 03/30/2013 09:13 PM, Jean Delvare wrote:
 Hi Lars,
 
 On Sat,  9 Mar 2013 19:16:43 +0100, Lars-Peter Clausen wrote:
 Currently i2c_del_adapter() returns 0 on success and potentially an error 
 code
 on failure. Unfortunately this doesn't mix too well with the Linux device 
 driver
 model. (...)
 
 I see:
 
 struct device_driver {
   (...)
   int (*probe) (struct device *dev);
   int (*remove) (struct device *dev);

 So the driver core does allow remove functions to return an error.

Well, the return type is int, but the return value is never checked. So you
can return an error value, but the device driver core is going to care and
the device is still removed. So any code which does return an error in it's
probe function in the assumption that this means the device will still be
present is broken and leaves the system in an undefined state. So if that
happens the kernel will probably crash sooner or later, or if you are lucky
you only created a few resources leaks.

And no we can't change the core to handle errors from a drivers remove
callback. It's a basic inherent property of the Linux device driver model
that any device can be removed at any time.

 Are you going to fix all subsystems as you are doing for i2c now, and then
 change device_driver.remove to return void? If not, I don't see the
 point of changing it in i2c.
 

As I said it's a bug if a driver returns an error in its remove function.
And the fact that the return type of the remove callback is int is pretty
much misleading in this regard, so the long term goal is to make the return
type void. But that's a long way to go until we get there, fixing the return
type of i2c_del_adapter() is kind of the low hanging fruit.

- Lars
--
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/6] i2c: Remove detach_adapter

[Lars-Peter Clausen]

The detach_adapter callback has been deprecated for quite some time and has no
user left. Keeping it alive blocks other cleanups, so remove it.

Signed-off-by: Lars-Peter Clausen l...@metafoo.de
---
 drivers/i2c/i2c-core.c | 33 ++---
 include/linux/i2c.h|  7 ++-
 2 files changed, 12 insertions(+), 28 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index f7dfe87..a853cb3 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -47,7 +47,7 @@
 
 /* core_lock protects i2c_adapter_idr, and guarantees
that device detection, deletion of detected devices, and attach_adapter
-   and detach_adapter calls are serialized */
+   calls are serialized */
 static DEFINE_MUTEX(core_lock);
 static DEFINE_IDR(i2c_adapter_idr);
 
@@ -1013,11 +1013,10 @@ int i2c_add_numbered_adapter(struct i2c_adapter *adap)
 }
 EXPORT_SYMBOL_GPL(i2c_add_numbered_adapter);
 
-static int i2c_do_del_adapter(struct i2c_driver *driver,
+static void i2c_do_del_adapter(struct i2c_driver *driver,
  struct i2c_adapter *adapter)
 {
struct i2c_client *client, *_n;
-   int res;
 
/* Remove the devices we created ourselves as the result of hardware
 * probing (using a driver's detect method) */
@@ -1029,16 +1028,6 @@ static int i2c_do_del_adapter(struct i2c_driver *driver,
i2c_unregister_device(client);
}
}
-
-   if (!driver-detach_adapter)
-   return 0;
-   dev_warn(adapter-dev, %s: detach_adapter method is deprecated\n,
-driver-driver.name);
-   res = driver-detach_adapter(adapter);
-   if (res)
-   dev_err(adapter-dev, detach_adapter failed (%d) 
-   for driver [%s]\n, res, driver-driver.name);
-   return res;
 }
 
 static int __unregister_client(struct device *dev, void *dummy)
@@ -1059,7 +1048,8 @@ static int __unregister_dummy(struct device *dev, void 
*dummy)
 
 static int __process_removed_adapter(struct device_driver *d, void *data)
 {
-   return i2c_do_del_adapter(to_i2c_driver(d), data);
+   i2c_do_del_adapter(to_i2c_driver(d), data);
+   return 0;
 }
 
 /**
@@ -1072,7 +1062,6 @@ static int __process_removed_adapter(struct device_driver 
*d, void *data)
  */
 int i2c_del_adapter(struct i2c_adapter *adap)
 {
-   int res = 0;
struct i2c_adapter *found;
struct i2c_client *client, *next;
 
@@ -1088,11 +1077,9 @@ int i2c_del_adapter(struct i2c_adapter *adap)
 
/* Tell drivers about this removal */
mutex_lock(core_lock);
-   res = bus_for_each_drv(i2c_bus_type, NULL, adap,
+   bus_for_each_drv(i2c_bus_type, NULL, adap,
   __process_removed_adapter);
mutex_unlock(core_lock);
-   if (res)
-   return res;
 
/* Remove devices instantiated from sysfs */
mutex_lock_nested(adap-userspace_clients_lock,
@@ -,8 +1098,8 @@ int i2c_del_adapter(struct i2c_adapter *adap)
 * we can't remove the dummy devices during the first pass: they
 * could have been instantiated by real devices wishing to clean
 * them up properly, so we give them a chance to do that first. */
-   res = device_for_each_child(adap-dev, NULL, __unregister_client);
-   res = device_for_each_child(adap-dev, NULL, __unregister_dummy);
+   device_for_each_child(adap-dev, NULL, __unregister_client);
+   device_for_each_child(adap-dev, NULL, __unregister_dummy);
 
 #ifdef CONFIG_I2C_COMPAT
class_compat_remove_link(i2c_adapter_compat_class, adap-dev,
@@ -1208,9 +1195,9 @@ EXPORT_SYMBOL(i2c_register_driver);
 
 static int __process_removed_driver(struct device *dev, void *data)
 {
-   if (dev-type != i2c_adapter_type)
-   return 0;
-   return i2c_do_del_adapter(data, to_i2c_adapter(dev));
+   if (dev-type == i2c_adapter_type)
+   i2c_do_del_adapter(data, to_i2c_adapter(dev));
+   return 0;
 }
 
 /**
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index d0c4db7..8ffab3f 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -125,7 +125,6 @@ extern s32 i2c_smbus_write_i2c_block_data(const struct 
i2c_client *client,
  * struct i2c_driver - represent an I2C device driver
  * @class: What kind of i2c device we instantiate (for detect)
  * @attach_adapter: Callback for bus addition (deprecated)
- * @detach_adapter: Callback for bus removal (deprecated)
  * @probe: Callback for device binding
  * @remove: Callback for device unbinding
  * @shutdown: Callback for device shutdown
@@ -162,12 +161,10 @@ extern s32 i2c_smbus_write_i2c_block_data(const struct 
i2c_client *client,
 struct i2c_driver {
unsigned int class;
 
-   /* Notifies the driver that a new bus has appeared or is about to be
-* removed. You should avoid using this, it will be removed in a
-* near future.
+   /* Notifies the driver

[PATCH 0/6] Make return type of i2c_del_adapter() (and i2c_del_mux_adapter()) void

[Lars-Peter Clausen]

Currently i2c_del_adapter() returns 0 on success and potentially an error code
on failure. Unfortunately this doesn't mix too well with the Linux device driver
model. An i2c_adapter is usually registered in a drivers probe callback and
removed in the drivers remove callback. The Linux device driver model assumes
that a device may disappear at any time, e.g. because it is on a hot-plug-able
bus and the device is physically removed or because it is unbound from it's
driver. This means that a driver's remove callback is not allowed to fail. This
has lead to code fragments like the following:

rc = i2c_del_adapter(board-i2c_adap);
BUG_ON(rc);

Currently there are two code paths which can cause to i2c_del_adapter() to
return an error code:
1) If the adapter that is passed to i2c_del_adapter() is not registered
i2c_del_adapter() returns -EINVAL
2) If an I2C client, which is registered as a child to the adapter, implements
the detach_adapter callback and detach_adapter returns an error the removal of
the adapter is aborted and i2c_del_adapter() returns the error returned by the
clients detach_adapter callback.

The first probably never happens unless there is a serious bug in the driver
calling i2c_del_adapter(), but I think even then, for the sake of sanitizing the
API we can argue that the purpose of i2c_del_adapter() is to remove the adapter
from the system. If the adapter currently isn't registered this becomes a no-op
and we can return success, without having to do anything. The second also never
happens, since the detach_adapter callback has been deprecated for quite some
time now and there are currently no drivers left which implement it.

So realisticly i2c_del_adapter() already always returns 0 and all code checking
for errors is more or less dead code. And in fact the majority of the users of
i2c_del_adapter() already ignore the return value, but there are still some
drivers (both newer and older) which assume that i2c_del_adapter() might fail.
This series aims at making it explicit that i2c_del_adapter() can not fail by
making its return type void.

The series will start by making i2c_del_adapter() always return success. For
this it will update the case, where a non-registered adapter is passed in, to
return 0 instead of -EINVAL and remove all code related to the unused
detach_adapter callback. Afterward the series will update all users of
i2c_del_adapter() to ignore the return value (since it is always 0 now). And
finally the series will change the return type of i2c_del_adapter() to void.

The same is true for i2c_del_mux_adapter() which is mostly just a wrapper
around i2c_del_adapter(), so it is also updated in this series.

- Lars

Lars-Peter Clausen (6):
  i2c: Remove detach_adapter
  i2c: i2c_del_adapter: Don't treat removing a non-registered adapter
as error
  i2c: Ignore return value of i2c_del_adapter()
  i2c: Make return type of i2c_del_adapter() void
  i2c: Ignore the return value of i2c_del_mux_adapter()
  i2c: Make the return type of i2c_del_mux_adapter() void

 drivers/gpu/drm/gma500/oaktrail_hdmi_i2c.c   |  3 +--
 drivers/i2c/busses/i2c-amd756-s4882.c|  6 +
 drivers/i2c/busses/i2c-at91.c|  5 ++--
 drivers/i2c/busses/i2c-cbus-gpio.c   |  4 ++-
 drivers/i2c/busses/i2c-intel-mid.c   |  3 +--
 drivers/i2c/busses/i2c-mv64xxx.c |  5 ++--
 drivers/i2c/busses/i2c-mxs.c |  5 +---
 drivers/i2c/busses/i2c-nforce2-s4985.c   |  6 +
 drivers/i2c/busses/i2c-powermac.c| 10 ++-
 drivers/i2c/busses/i2c-puv3.c| 10 ++-
 drivers/i2c/busses/i2c-viperboard.c  |  5 ++--
 drivers/i2c/i2c-core.c   | 39 +---
 drivers/i2c/i2c-mux.c|  9 ++-
 drivers/i2c/muxes/i2c-mux-pca954x.c  |  6 ++---
 drivers/media/pci/bt8xx/bttv-input.c |  6 ++---
 drivers/media/pci/mantis/mantis_i2c.c|  4 ++-
 drivers/net/ethernet/sfc/falcon.c|  6 ++---
 drivers/staging/media/go7007/go7007-driver.c |  7 ++---
 include/linux/i2c-mux.h  |  2 +-
 include/linux/i2c.h  |  9 +++
 20 files changed, 48 insertions(+), 102 deletions(-)

-- 
1.8.0

--
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/6] i2c: i2c_del_adapter: Don't treat removing a non-registered adapter as error

[Lars-Peter Clausen]

Currently i2c_del_adapter() returns -EINVAL when it gets an adapter which is not
registered. But none of the users of i2c_del_adapter() depend on this behavior,
so for the sake of being able to sanitize the return type of i2c_del_adapter
argue, that the purpose of i2c_del_adapter() is to remove an I2C adapter from
the system. If the adapter is not registered in the first place this becomes a
no-op. So we can return success without having to do anything.

Signed-off-by: Lars-Peter Clausen l...@metafoo.de
---
 drivers/i2c/i2c-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index a853cb3..7727d33 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -1072,7 +1072,7 @@ int i2c_del_adapter(struct i2c_adapter *adap)
if (found != adap) {
pr_debug(i2c-core: attempting to delete unregistered 
 adapter [%s]\n, adap-name);
-   return -EINVAL;
+   return 0;
}
 
/* Tell drivers about this removal */
-- 
1.8.0

--
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/6] i2c: Ignore return value of i2c_del_adapter()

[Lars-Peter Clausen]

i2c_del_adapter() always returns 0. So all checks testing whether it will be
non zero will always evaluate to false and the conditional code is dead code.
This patch updates all callers of i2c_del_mux_adapter() to ignore the return
value and assume that it will always succeed (which it will). In a subsequent
patch the return type of i2c_del_adapter() will be made void.

Cc: Jean Delvare kh...@linux-fr.org
Cc: Guan Xuetao g...@mprc.pku.edu.cn
Cc: Mauro Carvalho Chehab mche...@redhat.com
Cc: Ben Hutchings bhutchi...@solarflare.com
Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
Cc: Alan Cox a...@linux.intel.com
Cc: Nicolas Ferre nicolas.fe...@atmel.com
Cc: Aaro Koskinen aaro.koski...@iki.fi
Cc: Marek Vasut ma...@denx.de (commit_signer:11/20=55%)
Cc: Shawn Guo shawn@linaro.org
Cc: Benjamin Herrenschmidt b...@kernel.crashing.org
Cc: Lars Poeschel poesc...@lemonage.de
Signed-off-by: Lars-Peter Clausen l...@metafoo.de
---
 drivers/gpu/drm/gma500/oaktrail_hdmi_i2c.c   |  3 +--
 drivers/i2c/busses/i2c-amd756-s4882.c|  6 +-
 drivers/i2c/busses/i2c-at91.c|  5 ++---
 drivers/i2c/busses/i2c-cbus-gpio.c   |  4 +++-
 drivers/i2c/busses/i2c-intel-mid.c   |  3 +--
 drivers/i2c/busses/i2c-mv64xxx.c |  5 ++---
 drivers/i2c/busses/i2c-mxs.c |  5 +
 drivers/i2c/busses/i2c-nforce2-s4985.c   |  6 +-
 drivers/i2c/busses/i2c-powermac.c| 10 ++
 drivers/i2c/busses/i2c-puv3.c| 10 ++
 drivers/i2c/busses/i2c-viperboard.c  |  5 ++---
 drivers/i2c/i2c-mux.c|  5 +
 drivers/media/pci/bt8xx/bttv-input.c |  6 +++---
 drivers/media/pci/mantis/mantis_i2c.c|  4 +++-
 drivers/net/ethernet/sfc/falcon.c|  6 ++
 drivers/staging/media/go7007/go7007-driver.c |  7 ++-
 16 files changed, 29 insertions(+), 61 deletions(-)

diff --git a/drivers/gpu/drm/gma500/oaktrail_hdmi_i2c.c 
b/drivers/gpu/drm/gma500/oaktrail_hdmi_i2c.c
index 88627e3..1eb86c7 100644
--- a/drivers/gpu/drm/gma500/oaktrail_hdmi_i2c.c
+++ b/drivers/gpu/drm/gma500/oaktrail_hdmi_i2c.c
@@ -319,8 +319,7 @@ void oaktrail_hdmi_i2c_exit(struct pci_dev *dev)
struct hdmi_i2c_dev *i2c_dev;
 
hdmi_dev = pci_get_drvdata(dev);
-   if (i2c_del_adapter(oaktrail_hdmi_i2c_adapter))
-   DRM_DEBUG_DRIVER(Failed to delete hdmi-i2c adapter\n);
+   i2c_del_adapter(oaktrail_hdmi_i2c_adapter);
 
i2c_dev = hdmi_dev-i2c_dev;
kfree(i2c_dev);
diff --git a/drivers/i2c/busses/i2c-amd756-s4882.c 
b/drivers/i2c/busses/i2c-amd756-s4882.c
index 378fcb5..07f01ac 100644
--- a/drivers/i2c/busses/i2c-amd756-s4882.c
+++ b/drivers/i2c/busses/i2c-amd756-s4882.c
@@ -169,11 +169,7 @@ static int __init amd756_s4882_init(void)
}
 
/* Unregister physical bus */
-   error = i2c_del_adapter(amd756_smbus);
-   if (error) {
-   dev_err(amd756_smbus.dev, Physical bus removal failed\n);
-   goto ERROR0;
-   }
+   i2c_del_adapter(amd756_smbus);
 
printk(KERN_INFO Enabling SMBus multiplexing for Tyan S4882\n);
/* Define the 5 virtual adapters and algorithms structures */
diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
index 75195e3..6604587 100644
--- a/drivers/i2c/busses/i2c-at91.c
+++ b/drivers/i2c/busses/i2c-at91.c
@@ -785,12 +785,11 @@ static int at91_twi_probe(struct platform_device *pdev)
 static int at91_twi_remove(struct platform_device *pdev)
 {
struct at91_twi_dev *dev = platform_get_drvdata(pdev);
-   int rc;
 
-   rc = i2c_del_adapter(dev-adapter);
+   i2c_del_adapter(dev-adapter);
clk_disable_unprepare(dev-clk);
 
-   return rc;
+   return 0;
 }
 
 #ifdef CONFIG_PM
diff --git a/drivers/i2c/busses/i2c-cbus-gpio.c 
b/drivers/i2c/busses/i2c-cbus-gpio.c
index 98386d6..1be13ac 100644
--- a/drivers/i2c/busses/i2c-cbus-gpio.c
+++ b/drivers/i2c/busses/i2c-cbus-gpio.c
@@ -206,7 +206,9 @@ static int cbus_i2c_remove(struct platform_device *pdev)
 {
struct i2c_adapter *adapter = platform_get_drvdata(pdev);
 
-   return i2c_del_adapter(adapter);
+   i2c_del_adapter(adapter);
+
+   return 0;
 }
 
 static int cbus_i2c_probe(struct platform_device *pdev)
diff --git a/drivers/i2c/busses/i2c-intel-mid.c 
b/drivers/i2c/busses/i2c-intel-mid.c
index 323fa01..0fb6597 100644
--- a/drivers/i2c/busses/i2c-intel-mid.c
+++ b/drivers/i2c/busses/i2c-intel-mid.c
@@ -1082,8 +1082,7 @@ static void intel_mid_i2c_remove(struct pci_dev *dev)
 {
struct intel_mid_i2c_private *mrst = pci_get_drvdata(dev);
intel_mid_i2c_disable(mrst-adap);
-   if (i2c_del_adapter(mrst-adap))
-   dev_err(dev-dev, Failed to delete i2c adapter);
+   i2c_del_adapter(mrst-adap);
 
free_irq(dev-irq, mrst);
iounmap(mrst-base);
diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
index 8b20ef8..3bbd65d

[PATCH 4/6] i2c: Make return type of i2c_del_adapter() void

[Lars-Peter Clausen]

i2c_del_adapter() is usually called from a drivers remove callback. The Linux
device driver model does not allow the remove callback to fail and all resources
allocated in the probe callback need to be freed, as well as all resources which
have been provided to the rest of the kernel(for example a I2C adapter) need to
be revoked. So any function revoking such resources isn't allowed to fail
either. i2c_del_adapter() adheres to this requirement and will never fail. But
i2c_del_adapter()'s return type is int, which may cause driver authors to think
that it can fail. This led to code constructs like:

ret = i2c_del_adapter(...);
BUG_ON(ret);

Since i2c_del_adapter() always returns 0 the BUG_ON is never hit and essentially
becomes dead code, which means it can be removed. Making the return type of
i2c_del_adapter() void makes it explicit that the function will never fail and
should prevent constructs like the above from re-appearing in the kernel code.

All callers of i2c_del_adapter() have already been updated in a previous patch
to ignore the return value, so the conversion of the return type from int to
void can be done without causing any build failures.

Signed-off-by: Lars-Peter Clausen l...@metafoo.de
---
 drivers/i2c/i2c-core.c | 6 ++
 include/linux/i2c.h| 2 +-
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 7727d33..838d030 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -1060,7 +1060,7 @@ static int __process_removed_adapter(struct device_driver 
*d, void *data)
  * This unregisters an I2C adapter which was previously registered
  * by @i2c_add_adapter or @i2c_add_numbered_adapter.
  */
-int i2c_del_adapter(struct i2c_adapter *adap)
+void i2c_del_adapter(struct i2c_adapter *adap)
 {
struct i2c_adapter *found;
struct i2c_client *client, *next;
@@ -1072,7 +1072,7 @@ int i2c_del_adapter(struct i2c_adapter *adap)
if (found != adap) {
pr_debug(i2c-core: attempting to delete unregistered 
 adapter [%s]\n, adap-name);
-   return 0;
+   return;
}
 
/* Tell drivers about this removal */
@@ -1124,8 +1124,6 @@ int i2c_del_adapter(struct i2c_adapter *adap)
/* Clear the device structure in case this adapter is ever going to be
   added again */
memset(adap-dev, 0, sizeof(adap-dev));
-
-   return 0;
 }
 EXPORT_SYMBOL(i2c_del_adapter);
 
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 8ffab3f..dec1702 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -447,7 +447,7 @@ void i2c_unlock_adapter(struct i2c_adapter *);
  */
 #if defined(CONFIG_I2C) || defined(CONFIG_I2C_MODULE)
 extern int i2c_add_adapter(struct i2c_adapter *);
-extern int i2c_del_adapter(struct i2c_adapter *);
+extern void i2c_del_adapter(struct i2c_adapter *);
 extern int i2c_add_numbered_adapter(struct i2c_adapter *);
 
 extern int i2c_register_driver(struct module *, struct i2c_driver *);
-- 
1.8.0

--
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 5/6] i2c: Ignore the return value of i2c_del_mux_adapter()

[Lars-Peter Clausen]

i2c_del_mux_adapter() always returns 0. So all checks testing whether it will be
non zero will always evaluate to false and the conditional code is dead code.
This patch updates all callers of i2c_del_mux_adapter() to ignore its return
value and assume that it will always succeed (which it will). A subsequent
patch will make the return type of i2c_del_mux_adapter() void.

Cc: Guenter Roeck guenter.ro...@ericsson.com
Signed-off-by: Lars-Peter Clausen l...@metafoo.de
---
 drivers/i2c/muxes/i2c-mux-pca954x.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c 
b/drivers/i2c/muxes/i2c-mux-pca954x.c
index 8e43872..a531d80 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -262,13 +262,11 @@ static int pca954x_remove(struct i2c_client *client)
 {
struct pca954x *data = i2c_get_clientdata(client);
const struct chip_desc *chip = chips[data-type];
-   int i, err;
+   int i;
 
for (i = 0; i  chip-nchans; ++i)
if (data-virt_adaps[i]) {
-   err = i2c_del_mux_adapter(data-virt_adaps[i]);
-   if (err)
-   return err;
+   i2c_del_mux_adapter(data-virt_adaps[i]);
data-virt_adaps[i] = NULL;
}
 
-- 
1.8.0

--
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] I2C: xiic: Add OF binding support

[Lars-Peter Clausen]

Signed-off-by: Lars-Peter Clausen l...@metafoo.de
---
 Documentation/devicetree/bindings/i2c/xiic.txt |   22 ++
 drivers/i2c/busses/i2c-xiic.c  |   23 ++-
 2 files changed, 40 insertions(+), 5 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/i2c/xiic.txt

diff --git a/Documentation/devicetree/bindings/i2c/xiic.txt 
b/Documentation/devicetree/bindings/i2c/xiic.txt
new file mode 100644
index 000..ceabbe9
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/xiic.txt
@@ -0,0 +1,22 @@
+Xilinx IIC controller:
+
+Required properties:
+- compatible : Must be xlnx,xps-iic-2.00.a
+- reg : IIC register location and length
+- interrupts : IIC controller unterrupt
+- #address-cells = 1
+- #size-cells = 0
+
+Optional properties:
+- Child nodes conforming to i2c bus binding
+
+Example:
+
+   axi_iic_0: i2c@4080 {
+   compatible = xlnx,xps-iic-2.00.a;
+   interrupts =  1 2 ;
+   reg =  0x4080 0x1 ;
+
+   #size-cells = 0;
+   #address-cells = 1;
+   };
diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
index 2bded76..641d0e5 100644
--- a/drivers/i2c/busses/i2c-xiic.c
+++ b/drivers/i2c/busses/i2c-xiic.c
@@ -40,6 +40,7 @@
 #include linux/i2c-xiic.h
 #include linux/io.h
 #include linux/slab.h
+#include linux/of_i2c.h
 
 #define DRIVER_NAME xiic-i2c
 
@@ -705,8 +706,6 @@ static int __devinit xiic_i2c_probe(struct platform_device 
*pdev)
goto resource_missing;
 
pdata = (struct xiic_i2c_platform_data *) pdev-dev.platform_data;
-   if (!pdata)
-   return -EINVAL;
 
i2c = kzalloc(sizeof(*i2c), GFP_KERNEL);
if (!i2c)
@@ -730,6 +729,7 @@ static int __devinit xiic_i2c_probe(struct platform_device 
*pdev)
i2c-adap = xiic_adapter;
i2c_set_adapdata(i2c-adap, i2c);
i2c-adap.dev.parent = pdev-dev;
+   i2c-adap.dev.of_node = pdev-dev.of_node;
 
xiic_reinit(i2c);
 
@@ -748,9 +748,13 @@ static int __devinit xiic_i2c_probe(struct platform_device 
*pdev)
goto add_adapter_failed;
}
 
-   /* add in known devices to the bus */
-   for (i = 0; i  pdata-num_devices; i++)
-   i2c_new_device(i2c-adap, pdata-devices + i);
+   if (pdata) {
+   /* add in known devices to the bus */
+   for (i = 0; i  pdata-num_devices; i++)
+   i2c_new_device(i2c-adap, pdata-devices + i);
+   }
+
+   of_i2c_register_devices(i2c-adap);
 
return 0;
 
@@ -795,12 +799,21 @@ static int __devexit xiic_i2c_remove(struct 
platform_device* pdev)
return 0;
 }
 
+#if defined(CONFIG_OF)
+static const struct of_device_id xiic_of_match[] __devinitconst = {
+   { .compatible = xlnx,xps-iic-2.00.a, },
+   {},
+};
+MODULE_DEVICE_TABLE(of, xiic_of_match);
+#endif
+
 static struct platform_driver xiic_i2c_driver = {
.probe   = xiic_i2c_probe,
.remove  = __devexit_p(xiic_i2c_remove),
.driver  = {
.owner = THIS_MODULE,
.name = DRIVER_NAME,
+   .of_match_table = of_match_ptr(xiic_of_match),
},
 };
 
-- 
1.7.9.5

--
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 v3 0/2] i2c/of: Populate multiplexed i2c busses from the device tree.

[Lars-Peter Clausen]

On 04/12/2012 11:14 PM, David Daney wrote:
 From: David Daney david.da...@cavium.com
 
 v3: Integrate changes from Lars-Peter Clausen to make better use of
 the of_*() infrastructure.  Get rid of ugly #ifdefs.
 
 v2: Update bindings to use reg insutead of cell-index
 
 v1: Unchanged from the original RFC where I said:
 
   We need to populate our I2C devices from the device tree, but some
   of our systems have multiplexers which currently break this process.
 
   The basic idea is to have the generic i2c-mux framework propagate
   the device_node for the child bus into the corresponding
   i2c_adapter, and then call of_i2c_register_devices().  This means
   that the device tree bindings for *all* i2c muxes must have some
   common properties.  I try to document these in mux.txt.
 
 This is now tested against 3.4-rc2 and is still working well.
 

I've been using these patches with a pca9548 and a pca9546 for a while now.
Both:

Tested-by: Lars-Peter Clausen l...@metafoo.de

--
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/5] SPI: Add helper macro for spi_driver boilerplate

[Lars-Peter Clausen]

On 11/24/2011 01:13 AM, Ben Dooks wrote:
 On Wed, Nov 16, 2011 at 10:12:54AM -0700, Grant Likely wrote:
 On Wed, Nov 16, 2011 at 2:13 AM, Lars-Peter Clausen l...@metafoo.de wrote:
 This patch introduces the module_spi_driver macro which is a convenience 
 macro
 for SPI driver modules similar to module_platform_driver. It is intended to 
 be
 used by drivers which init/exit section does nothing but register/unregister
 the SPI driver. By using this macro it is possible to eliminate a few lines 
 of
 boilerplate code per SPI driver.

 Signed-off-by: Lars-Peter Clausen l...@metafoo.de

 Acked-by: Grant Likely grant.lik...@secretlab.ca
 
 I'm begining to think we need to make some of these driver and device
 bits more generic... there seems to be so much similar but not quite
 the same code.
 

I've been thinking the same. A good start would probably be consolidating
the platform/spi/i2c device id handling code. Since those are all name based
ids the code for handling them looks rather similar, though there are some
minor differences.

- Lars
--
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/5] Generalize module_platform_driver

[Lars-Peter Clausen]

Grant Likely recently introduced the module_platform_driver macro which can be
used to eliminate a few lines on boilerplate code in platform driver modules.
The same approach can be used to do the same for other bus type drivers.

The first patch of this series generalizes the module_platform_driver macro and
introduces the module_driver macro. It is similar to module_platform_driver
macro but has two additional parameters to pass in the driver register and
unregister function. The intend is that this macro is used to construct bus
specific macros for generating the driver register/unregister boilerplate code.

The next two patches in this series add the module_i2c_driver and
module_spi_driver macro which use the module_driver macro to generate the I2C
and SPI driver boilerplate code.

The last two patches convert the driver found in the IIO framework to use the
new module_i2c_driver and module_spi_driver macros to demonstrate their
potential and remove over 700 lines of code.

While this series only introduces these kind of helper macros for I2C and SPI
bus drivers the same scheme should be applicable to most other bus driver types.
For example PCI and USB seem to be good candidates.

It probably makes sense to merge the first three patches together. The last two
can probably, since this is not urgent, wait until the first three have reached
mainline.

- Lars

Lars-Peter Clausen (5):
  drivercore: Generalize module_platform_driver
  I2C: Add helper macro for i2c_driver boilerplate
  SPI: Add helper macro for spi_driver boilerplate
  staging:iio: Use module_i2c_driver to register I2C drivers
  staging:iio: Use module_spi_driver to register SPI driver

 drivers/staging/iio/accel/adis16201_core.c  |   13 +
 drivers/staging/iio/accel/adis16203_core.c  |   13 +
 drivers/staging/iio/accel/adis16204_core.c  |   13 +
 drivers/staging/iio/accel/adis16209_core.c  |   13 +
 drivers/staging/iio/accel/adis16220_core.c  |   13 +
 drivers/staging/iio/accel/adis16240_core.c  |   13 +
 drivers/staging/iio/accel/kxsd9.c   |   13 +
 drivers/staging/iio/accel/lis3l02dq_core.c  |   13 +
 drivers/staging/iio/accel/sca3000_core.c|   13 +
 drivers/staging/iio/adc/ad7192.c|   13 +
 drivers/staging/iio/adc/ad7280a.c   |   13 +
 drivers/staging/iio/adc/ad7291.c|   14 +-
 drivers/staging/iio/adc/ad7298_core.c   |   13 +
 drivers/staging/iio/adc/ad7476_core.c   |   13 +
 drivers/staging/iio/adc/ad7606_spi.c|   13 +
 drivers/staging/iio/adc/ad7780.c|   13 +
 drivers/staging/iio/adc/ad7793.c|   13 +
 drivers/staging/iio/adc/ad7816.c|   14 +-
 drivers/staging/iio/adc/ad7887_core.c   |   13 +
 drivers/staging/iio/adc/ad799x_core.c   |   14 +-
 drivers/staging/iio/adc/adt7310.c   |   14 +-
 drivers/staging/iio/adc/adt7410.c   |   14 +-
 drivers/staging/iio/adc/max1363_core.c  |   14 +-
 drivers/staging/iio/addac/adt7316-i2c.c |   14 +-
 drivers/staging/iio/addac/adt7316-spi.c |   14 +-
 drivers/staging/iio/cdc/ad7150.c|   14 +-
 drivers/staging/iio/cdc/ad7152.c|   14 +-
 drivers/staging/iio/cdc/ad7746.c|   14 +-
 drivers/staging/iio/dac/ad5064.c|   13 +
 drivers/staging/iio/dac/ad5360.c|   13 +
 drivers/staging/iio/dac/ad5446.c|   13 +
 drivers/staging/iio/dac/ad5504.c|   13 +
 drivers/staging/iio/dac/ad5624r_spi.c   |   13 +
 drivers/staging/iio/dac/ad5686.c|   13 +
 drivers/staging/iio/dac/ad5791.c|   13 +
 drivers/staging/iio/dac/max517.c|   14 +-
 drivers/staging/iio/dds/ad5930.c|   13 +
 drivers/staging/iio/dds/ad9832.c|   13 +
 drivers/staging/iio/dds/ad9834.c|   13 +
 drivers/staging/iio/dds/ad9850.c|   13 +
 drivers/staging/iio/dds/ad9852.c|   13 +
 drivers/staging/iio/dds/ad9910.c|   13 +
 drivers/staging/iio/dds/ad9951.c|   13 +
 drivers/staging/iio/gyro/adis16080_core.c   |   13 +
 drivers/staging/iio/gyro/adis16130_core.c   |   13 +
 drivers/staging/iio/gyro/adis16260_core.c   |   13 +
 drivers/staging/iio/gyro/adxrs450_core.c|   13 +
 drivers/staging

[PATCH 3/5] SPI: Add helper macro for spi_driver boilerplate

[Lars-Peter Clausen]

This patch introduces the module_spi_driver macro which is a convenience macro
for SPI driver modules similar to module_platform_driver. It is intended to be
used by drivers which init/exit section does nothing but register/unregister
the SPI driver. By using this macro it is possible to eliminate a few lines of
boilerplate code per SPI driver.

Signed-off-by: Lars-Peter Clausen l...@metafoo.de
---
 include/linux/spi/spi.h |   11 +++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index bb4f5fb..176fce9 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -200,6 +200,17 @@ static inline void spi_unregister_driver(struct spi_driver 
*sdrv)
driver_unregister(sdrv-driver);
 }
 
+/**
+ * module_spi_driver() - Helper macro for registering a SPI driver
+ * @__spi_driver: spi_driver struct
+ *
+ * Helper macro for SPI drivers which do not do anything special in module
+ * init/exit. This eliminates a lot of boilerplate. Each module may only
+ * use this macro once, and calling it replaces module_init() and module_exit()
+ */
+#define module_spi_driver(__spi_driver) \
+   module_driver(__spi_driver, spi_register_driver, \
+   spi_unregister_driver)
 
 /**
  * struct spi_master - interface to SPI master controller
-- 
1.7.7.1


--
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/5] I2C: Add helper macro for i2c_driver boilerplate

[Lars-Peter Clausen]

This patch introduces the module_i2c_driver macro which is a convenience macro
for I2C driver modules similar to module_platform_driver. It is intended to be
used by drivers which init/exit section does nothing but register/unregister
the I2C driver. By using this macro it is possible to eliminate a few lines of
boilerplate code per I2C driver.

Signed-off-by: Lars-Peter Clausen l...@metafoo.de
---
 include/linux/i2c.h |   13 +
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index a81bf6d..7e92854 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -485,6 +485,19 @@ static inline int i2c_adapter_id(struct i2c_adapter *adap)
 {
return adap-nr;
 }
+
+/**
+ * module_i2c_driver() - Helper macro for registering a I2C driver
+ * @__i2c_driver: i2c_driver struct
+ *
+ * Helper macro for I2C drivers which do not do anything special in module
+ * init/exit. This eliminates a lot of boilerplate. Each module may only
+ * use this macro once, and calling it replaces module_init() and module_exit()
+ */
+#define module_i2c_driver(__i2c_driver) \
+   module_driver(__i2c_driver, i2c_add_driver, \
+   i2c_del_driver)
+
 #endif /* I2C */
 #endif /* __KERNEL__ */
 
-- 
1.7.7.1


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