Re: [PATCH] i2c: omap: fix usage of IS_ERR_VALUE with pm_runtime_get_sync
On Thu, Mar 27, 2014 at 11:18:33AM -0500, Nishanth Menon wrote: > we use IS_ERR_VALUE to check for error values of pm_runtime_get_sync, > when the value can only be < 0 in the case of err. Replace the > check with a simpler < 0 check. > > This fixes the coccicheck warnings: > linux-2.6/drivers/i2c/busses/i2c-omap.c:1157:5-24: > pm_runtime_get_sync returns < 0 as error. Unecessary IS_ERR_VALUE at > line 1158 > linux-2.6/drivers/i2c/busses/i2c-omap.c:1278:7-26: > pm_runtime_get_sync returns < 0 as error. Unecessary IS_ERR_VALUE at > line 1279 > drivers/i2c/busses/i2c-omap.c:638:5-24: > pm_runtime_get_sync returns < 0 as error. Unecessary IS_ERR_VALUE at > line 639 > > Signed-off-by: Nishanth Menon Applied to for-next, thanks! signature.asc Description: Digital signature
Re: [PATCH v4] i2c: new bus driver for efm32
On Tue, Mar 25, 2014 at 11:48:46AM +0100, Uwe Kleine-König wrote: > This was tested on a EFM32GG-DK3750 devboard that has a temperature > sensor and an eeprom on its i2c bus. > > Signed-off-by: Uwe Kleine-König Applied to for-next, thanks! signature.asc Description: Digital signature
[PULL REQUEST] i2c for 3.14
Linus, the build fix from my last request unveiled another build problem which is fixed with this patch. Please pull. Thanks, Wolfram The following changes since commit dcb99fd9b08cfe1afe426af4d8d3cbc429190f15: Linux 3.14-rc7 (2014-03-16 18:51:24 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-current for you to fetch changes up to 5f12c5eca6e6b7aeb4b2028d579f614b4fe7a81f: i2c: cpm: Fix build by adding of_address.h and of_irq.h (2014-03-24 14:54:21 +0100) Scott Wood (1): i2c: cpm: Fix build by adding of_address.h and of_irq.h drivers/i2c/busses/i2c-cpm.c | 2 ++ 1 file changed, 2 insertions(+) signature.asc Description: Digital signature
Re: [RFC Patch v0 1/3] i2c-smbus: Add poll interface for smbus alert
On March 27, 2014 7:44:56 AM GMT+00:00, Jean Delvare wrote:
>On Wed, 26 Mar 2014 17:42:10 -0700, Srinivas Pandruvada wrote:
>> The current i2c smbus alert module depends on smbus alert mechanism
>> supported by underlying bus drivers. By specifications, these alerts
>> can be polled if there is no hardware support.
>> Currently multiple drivers who needs smbus alerts are creating
>> a new i2c dummy device with address 0x0C (ARA register), by luck
>> they don't co-exist. Otherwise i2c device creation will fail.
>> Added a poll interface, so that all these driver can call a common
>> interface to poll. Even if they polli, all drivers bound to an
>> adapater will be notified by their alert callback if ARA register
>> read is successful.
>>
>> Signed-off-by: Srinivas Pandruvada
>
>> ---
>> drivers/i2c/i2c-smbus.c | 23 +++
>> include/linux/i2c-smbus.h | 3 +++
>> 2 files changed, 26 insertions(+)
>>
>> diff --git a/drivers/i2c/i2c-smbus.c b/drivers/i2c/i2c-smbus.c
>> index fc99f0d..e274f20 100644
>> --- a/drivers/i2c/i2c-smbus.c
>> +++ b/drivers/i2c/i2c-smbus.c
>> @@ -72,6 +72,29 @@ static int smbus_do_alert(struct device *dev, void
>*addrp)
>> return -EBUSY;
>> }
>>
>> +int i2c_smbus_ara_poll(const struct i2c_client *client)
>> +{
>> +union i2c_smbus_data data;
>> +int status;
>> +struct alert_data alert_data;
>> +
>> +status = i2c_smbus_xfer(client->adapter, 0x0C, 0,
>> +I2C_SMBUS_READ, 0,
>> +I2C_SMBUS_BYTE, &data);
>> +if (status < 0)
>> +return status;
>> +
>> +alert_data.flag = data.byte & 1;
>> +alert_data.addr = data.byte >> 1;
>> +
>> +/* Notify driver for the device which issued the alert */
>> +device_for_each_child(&client->adapter->dev, &alert_data,
>> +smbus_do_alert);
>> +
>> +return data.byte;
>> +}
>
>This is essentially duplicating code from smbus_alert(), but in a
>hackish way, as the ARA is never properly reserved. Your bus driver
>should really register the ARA with i2c_setup_smbus_alert().
>
>I see that the code may not properly deal with the polled case
>everywhere but it should be pretty trivial to deal with. For example,
>check for alert->irq > 0 before re-enabling the irq in smbus_alert(). I
>don't immediately see any other change needed.
>
>If SMBus alert polling is done from the i2c device driver, we'll have
>to find a standard way for i2c device drivers to retrieve the ara
>client associated with an i2c_adapter. However I still need to be
>convinced that this makes any sense at all. Ultimately the alert will
>call the i2c device drivers's alert() callback. If the i2c device
>driver needs to do that, there's no need to go through ARA, it might as
>well just call the callback by itself.
>
>So can you please explain what problem exactly you are trying to solve?
As I understand it the issue is that some parts will not clear their internal
interrupt status unless an at a read occurs and presumably the read has to get
the correct address?
To my mind we should have polling inside the core and it should ensure all
devices that want to have replied. Have I miss understood how Ara is supposed
to work but should we not know which device to call the callback on?
>
>> +EXPORT_SYMBOL_GPL(i2c_smbus_ara_poll);
>> +
>> /*
>> * The alert IRQ handler needs to hand work off to a task which can
>issue
>> * SMBus calls, because those sleeping calls can't be made in IRQ
>context.
>> diff --git a/include/linux/i2c-smbus.h b/include/linux/i2c-smbus.h
>> index 8f1b086..f70755d 100644
>> --- a/include/linux/i2c-smbus.h
>> +++ b/include/linux/i2c-smbus.h
>> @@ -48,4 +48,7 @@ struct i2c_client *i2c_setup_smbus_alert(struct
>i2c_adapter *adapter,
>> struct i2c_smbus_alert_setup *setup);
>> int i2c_handle_smbus_alert(struct i2c_client *ara);
>>
>> +/* Interface to poll smbus alert */
>> +int i2c_smbus_ara_poll(const struct i2c_client *client);
>> +
>> #endif /* _LINUX_I2C_SMBUS_H */
--
Sent from my Android phone with K-9 Mail. Please excuse my brevity.
--
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: omap: fix usage of IS_ERR_VALUE with pm_runtime_get_sync
On Thu, Mar 27, 2014 at 11:18:33AM -0500, Nishanth Menon wrote: > we use IS_ERR_VALUE to check for error values of pm_runtime_get_sync, > when the value can only be < 0 in the case of err. Replace the > check with a simpler < 0 check. > > This fixes the coccicheck warnings: > linux-2.6/drivers/i2c/busses/i2c-omap.c:1157:5-24: > pm_runtime_get_sync returns < 0 as error. Unecessary IS_ERR_VALUE at > line 1158 > linux-2.6/drivers/i2c/busses/i2c-omap.c:1278:7-26: > pm_runtime_get_sync returns < 0 as error. Unecessary IS_ERR_VALUE at > line 1279 > drivers/i2c/busses/i2c-omap.c:638:5-24: > pm_runtime_get_sync returns < 0 as error. Unecessary IS_ERR_VALUE at > line 639 > > Signed-off-by: Nishanth Menon Reviewed-by: Felipe Balbi > --- > Patch based on: v3.14-rc8 > coccicheck report based on next-20140326 tag > > drivers/i2c/busses/i2c-omap.c |6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c > index 90dcc2e..078a3de 100644 > --- a/drivers/i2c/busses/i2c-omap.c > +++ b/drivers/i2c/busses/i2c-omap.c > @@ -636,7 +636,7 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg > msgs[], int num) > int r; > > r = pm_runtime_get_sync(dev->dev); > - if (IS_ERR_VALUE(r)) > + if (r < 0) > goto out; > > r = omap_i2c_wait_for_bb(dev); > @@ -1155,7 +1155,7 @@ omap_i2c_probe(struct platform_device *pdev) > pm_runtime_use_autosuspend(dev->dev); > > r = pm_runtime_get_sync(dev->dev); > - if (IS_ERR_VALUE(r)) > + if (r < 0) > goto err_free_mem; > > /* > @@ -1276,7 +1276,7 @@ static int omap_i2c_remove(struct platform_device *pdev) > > i2c_del_adapter(&dev->adapter); > ret = pm_runtime_get_sync(&pdev->dev); > - if (IS_ERR_VALUE(ret)) > + if (ret < 0) > return ret; > > omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, 0); > -- > 1.7.9.5 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- balbi signature.asc Description: Digital signature
[PATCH] i2c: omap: fix usage of IS_ERR_VALUE with pm_runtime_get_sync
we use IS_ERR_VALUE to check for error values of pm_runtime_get_sync, when the value can only be < 0 in the case of err. Replace the check with a simpler < 0 check. This fixes the coccicheck warnings: linux-2.6/drivers/i2c/busses/i2c-omap.c:1157:5-24: pm_runtime_get_sync returns < 0 as error. Unecessary IS_ERR_VALUE at line 1158 linux-2.6/drivers/i2c/busses/i2c-omap.c:1278:7-26: pm_runtime_get_sync returns < 0 as error. Unecessary IS_ERR_VALUE at line 1279 drivers/i2c/busses/i2c-omap.c:638:5-24: pm_runtime_get_sync returns < 0 as error. Unecessary IS_ERR_VALUE at line 639 Signed-off-by: Nishanth Menon --- Patch based on: v3.14-rc8 coccicheck report based on next-20140326 tag drivers/i2c/busses/i2c-omap.c |6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index 90dcc2e..078a3de 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -636,7 +636,7 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num) int r; r = pm_runtime_get_sync(dev->dev); - if (IS_ERR_VALUE(r)) + if (r < 0) goto out; r = omap_i2c_wait_for_bb(dev); @@ -1155,7 +1155,7 @@ omap_i2c_probe(struct platform_device *pdev) pm_runtime_use_autosuspend(dev->dev); r = pm_runtime_get_sync(dev->dev); - if (IS_ERR_VALUE(r)) + if (r < 0) goto err_free_mem; /* @@ -1276,7 +1276,7 @@ static int omap_i2c_remove(struct platform_device *pdev) i2c_del_adapter(&dev->adapter); ret = pm_runtime_get_sync(&pdev->dev); - if (IS_ERR_VALUE(ret)) + if (ret < 0) return ret; omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, 0); -- 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: [RFC Patch v0 1/3] i2c-smbus: Add poll interface for smbus alert
On 03/27/2014 12:44 AM, Jean Delvare wrote:
On Wed, 26 Mar 2014 17:42:10 -0700, Srinivas Pandruvada wrote:
The current i2c smbus alert module depends on smbus alert mechanism
supported by underlying bus drivers. By specifications, these alerts
can be polled if there is no hardware support.
Currently multiple drivers who needs smbus alerts are creating
a new i2c dummy device with address 0x0C (ARA register), by luck
they don't co-exist. Otherwise i2c device creation will fail.
Added a poll interface, so that all these driver can call a common
interface to poll. Even if they polli, all drivers bound to an
adapater will be notified by their alert callback if ARA register
read is successful.
Signed-off-by: Srinivas Pandruvada
---
drivers/i2c/i2c-smbus.c | 23 +++
include/linux/i2c-smbus.h | 3 +++
2 files changed, 26 insertions(+)
diff --git a/drivers/i2c/i2c-smbus.c b/drivers/i2c/i2c-smbus.c
index fc99f0d..e274f20 100644
--- a/drivers/i2c/i2c-smbus.c
+++ b/drivers/i2c/i2c-smbus.c
@@ -72,6 +72,29 @@ static int smbus_do_alert(struct device *dev, void *addrp)
return -EBUSY;
}
+int i2c_smbus_ara_poll(const struct i2c_client *client)
+{
+ union i2c_smbus_data data;
+ int status;
+ struct alert_data alert_data;
+
+ status = i2c_smbus_xfer(client->adapter, 0x0C, 0,
+ I2C_SMBUS_READ, 0,
+ I2C_SMBUS_BYTE, &data);
+ if (status < 0)
+ return status;
+
+ alert_data.flag = data.byte & 1;
+ alert_data.addr = data.byte >> 1;
+
+ /* Notify driver for the device which issued the alert */
+ device_for_each_child(&client->adapter->dev, &alert_data,
+ smbus_do_alert);
+
+ return data.byte;
+}
This is essentially duplicating code from smbus_alert(), but in a
hackish way, as the ARA is never properly reserved. Your bus driver
should really register the ARA with i2c_setup_smbus_alert().
I see that the code may not properly deal with the polled case
everywhere but it should be pretty trivial to deal with. For example,
check for alert->irq > 0 before re-enabling the irq in smbus_alert(). I
don't immediately see any other change needed.
If SMBus alert polling is done from the i2c device driver, we'll have
to find a standard way for i2c device drivers to retrieve the ara
client associated with an i2c_adapter. However I still need to be
convinced that this makes any sense at all. Ultimately the alert will
call the i2c device drivers's alert() callback. If the i2c device
driver needs to do that, there's no need to go through ARA, it might as
well just call the callback by itself.
So can you please explain what problem exactly you are trying to solve?
The current i2c clients and proposed patches are creating dummy devices.
You can't create two devices with same address. So they can't coexist.
Also 0x0c is a valid i2c address, so we can't create a new i2c device.
We have products there are devices on i2C on 0x0c. So don't you think
after calling
i2c_setup_smbus_alert()
the other device can really be setup by calling i2c_new_device(). This
function
does a busy_check for same addresses.
The idea is if some i2c client triggers ARA read, it doesn't destroy the
real receiver of the the smb alert and call respective alert() callbacks,
+EXPORT_SYMBOL_GPL(i2c_smbus_ara_poll);
+
/*
* The alert IRQ handler needs to hand work off to a task which can issue
* SMBus calls, because those sleeping calls can't be made in IRQ context.
diff --git a/include/linux/i2c-smbus.h b/include/linux/i2c-smbus.h
index 8f1b086..f70755d 100644
--- a/include/linux/i2c-smbus.h
+++ b/include/linux/i2c-smbus.h
@@ -48,4 +48,7 @@ struct i2c_client *i2c_setup_smbus_alert(struct i2c_adapter
*adapter,
struct i2c_smbus_alert_setup *setup);
int i2c_handle_smbus_alert(struct i2c_client *ara);
+/* Interface to poll smbus alert */
+int i2c_smbus_ara_poll(const struct i2c_client *client);
+
#endif /* _LINUX_I2C_SMBUS_H */
--
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: [RFC Patch v0 2/3] i2c-smbus: Allow building with I2C_HELPER_AUTO support
On 03/26/2014 11:57 PM, Jean Delvare wrote: On Wed, 26 Mar 2014 17:42:11 -0700, Srinivas Pandruvada wrote: Currentlty the config is conditional on !I2C_HELPER_AUTO. I don't know their dependency, but without this we can't build. Nack. Just because you don't understand something and did not bother looking into it, is no good reason to break a perfectly working and valid mechanism. That is why it RFC. Just read the Kconfig help text for I2C_HELPER_AUTO and you should be able to figure it out. Also grep the kernel tree for "select I2C_SMBUS". As a summary: any driver which depends on i2c-smbus, should select it in Kconfig. Signed-off-by: Srinivas Pandruvada --- drivers/i2c/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig index 7b7ea32..212ed73 100644 --- a/drivers/i2c/Kconfig +++ b/drivers/i2c/Kconfig @@ -74,7 +74,7 @@ config I2C_HELPER_AUTO In doubt, say Y. config I2C_SMBUS - tristate "SMBus-specific protocols" if !I2C_HELPER_AUTO + tristate "SMBus-specific protocols" help Say Y here if you want support for SMBus extensions to the I2C specification. At the moment, the only supported extension is -- 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: [RFC Patch v0 1/3] i2c-smbus: Add poll interface for smbus alert
On Wed, 26 Mar 2014 17:42:10 -0700, Srinivas Pandruvada wrote:
> The current i2c smbus alert module depends on smbus alert mechanism
> supported by underlying bus drivers. By specifications, these alerts
> can be polled if there is no hardware support.
> Currently multiple drivers who needs smbus alerts are creating
> a new i2c dummy device with address 0x0C (ARA register), by luck
> they don't co-exist. Otherwise i2c device creation will fail.
> Added a poll interface, so that all these driver can call a common
> interface to poll. Even if they polli, all drivers bound to an
> adapater will be notified by their alert callback if ARA register
> read is successful.
>
> Signed-off-by: Srinivas Pandruvada
> ---
> drivers/i2c/i2c-smbus.c | 23 +++
> include/linux/i2c-smbus.h | 3 +++
> 2 files changed, 26 insertions(+)
>
> diff --git a/drivers/i2c/i2c-smbus.c b/drivers/i2c/i2c-smbus.c
> index fc99f0d..e274f20 100644
> --- a/drivers/i2c/i2c-smbus.c
> +++ b/drivers/i2c/i2c-smbus.c
> @@ -72,6 +72,29 @@ static int smbus_do_alert(struct device *dev, void *addrp)
> return -EBUSY;
> }
>
> +int i2c_smbus_ara_poll(const struct i2c_client *client)
> +{
> + union i2c_smbus_data data;
> + int status;
> + struct alert_data alert_data;
> +
> + status = i2c_smbus_xfer(client->adapter, 0x0C, 0,
> + I2C_SMBUS_READ, 0,
> + I2C_SMBUS_BYTE, &data);
> + if (status < 0)
> + return status;
> +
> + alert_data.flag = data.byte & 1;
> + alert_data.addr = data.byte >> 1;
> +
> + /* Notify driver for the device which issued the alert */
> + device_for_each_child(&client->adapter->dev, &alert_data,
> + smbus_do_alert);
> +
> + return data.byte;
> +}
This is essentially duplicating code from smbus_alert(), but in a
hackish way, as the ARA is never properly reserved. Your bus driver
should really register the ARA with i2c_setup_smbus_alert().
I see that the code may not properly deal with the polled case
everywhere but it should be pretty trivial to deal with. For example,
check for alert->irq > 0 before re-enabling the irq in smbus_alert(). I
don't immediately see any other change needed.
If SMBus alert polling is done from the i2c device driver, we'll have
to find a standard way for i2c device drivers to retrieve the ara
client associated with an i2c_adapter. However I still need to be
convinced that this makes any sense at all. Ultimately the alert will
call the i2c device drivers's alert() callback. If the i2c device
driver needs to do that, there's no need to go through ARA, it might as
well just call the callback by itself.
So can you please explain what problem exactly you are trying to solve?
> +EXPORT_SYMBOL_GPL(i2c_smbus_ara_poll);
> +
> /*
> * The alert IRQ handler needs to hand work off to a task which can issue
> * SMBus calls, because those sleeping calls can't be made in IRQ context.
> diff --git a/include/linux/i2c-smbus.h b/include/linux/i2c-smbus.h
> index 8f1b086..f70755d 100644
> --- a/include/linux/i2c-smbus.h
> +++ b/include/linux/i2c-smbus.h
> @@ -48,4 +48,7 @@ struct i2c_client *i2c_setup_smbus_alert(struct i2c_adapter
> *adapter,
>struct i2c_smbus_alert_setup *setup);
> int i2c_handle_smbus_alert(struct i2c_client *ara);
>
> +/* Interface to poll smbus alert */
> +int i2c_smbus_ara_poll(const struct i2c_client *client);
> +
> #endif /* _LINUX_I2C_SMBUS_H */
--
Jean Delvare
SUSE L3 Support
--
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
