* Re: [PATCH v2] i2c: core: helper function to detect slave mode
From: Vladimir Zapolskiy @ 2017-01-06 21:49 UTC (permalink / raw)
To: Luis Oliveira, wsa-z923LK4zBo2bacvFa/9K2g,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
jarkko.nikula-VuQAYsv1563Yd54FQh9/CA,
andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA,
mika.westerberg-VuQAYsv1563Yd54FQh9/CA,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Ramiro.Oliveira-HKixBCOQz3hWk0Htik3J/w,
Joao.Pinto-HKixBCOQz3hWk0Htik3J/w,
CARLOS.PALMINHA-HKixBCOQz3hWk0Htik3J/w
In-Reply-To: <9ada747bd53bb5005a82f913412262b34b6e96b8.1483725370.git.lolivei-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
Hi Luis,
On 01/06/2017 08:14 PM, Luis Oliveira wrote:
> This function has the purpose of mode detection by checking the
> device nodes for a reg matching with the I2C_OWN_SLAVE_ADDREESS flag.
> Currently only checks using OF functions (ACPI slave not supported yet).
>
I've accidentally reviewed v1, please take the same comments for v2.
--
With best wishes,
Vladimir
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] i2c: core: helper function to detect slave mode
From: Vladimir Zapolskiy @ 2017-01-06 21:46 UTC (permalink / raw)
To: Luis Oliveira, wsa-z923LK4zBo2bacvFa/9K2g,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
jarkko.nikula-VuQAYsv1563Yd54FQh9/CA,
andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA,
mika.westerberg-VuQAYsv1563Yd54FQh9/CA,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Ramiro.Oliveira-HKixBCOQz3hWk0Htik3J/w,
Joao.Pinto-HKixBCOQz3hWk0Htik3J/w,
CARLOS.PALMINHA-HKixBCOQz3hWk0Htik3J/w
In-Reply-To: <f4ec62119145bc70c938800c91eb396d30457304.1483636071.git.lolivei-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
Hello Luis,
On 01/05/2017 07:24 PM, Luis Oliveira wrote:
> This function has the purpose of mode detection by checking the
> device nodes for a reg matching with the I2C_OWN_SLAVE_ADDREESS flag.
> Currently only checks using OF functions (ACPI slave not supported yet).
>
> Signed-off-by: Luis Oliveira <lolivei-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
> ---
> Due to the need of checking if the I2C slave address is our own (in
> other words: if we are the I2C slave) I created a helper function
> (proposed to me by @Andy) to enable that check.
> Currently (because I am not able to test it using ACPI) it only
> supports devicetree declarations.
>
> drivers/i2c/i2c-core.c | 19 +++++++++++++++++++
> include/linux/i2c.h | 1 +
> 2 files changed, 20 insertions(+)
>
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index 3de95a29024c..48e705b23c59 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -3691,6 +3691,25 @@ int i2c_slave_unregister(struct i2c_client *client)
> return ret;
> }
> EXPORT_SYMBOL_GPL(i2c_slave_unregister);
> +
> +int i2c_slave_mode_detect(struct device *dev)
> +{
> + struct device_node *child;
> + u32 reg;
> +
> + if (IS_BUILTIN(CONFIG_OF) && dev->of_node) {
IS_BUILTIN(CONFIG_OF) looks excessive, check for non-NULL dev->of_node
should be sufficient.
But then you may do for_each_child_of_node() loop totally unconditionally,
because of_get_next_child() correctly handle NULL as the first argument.
> + for_each_child_of_node(dev->of_node, child) {
> + of_property_read_u32(child, "reg", ®);
> + if (reg & I2C_OWN_SLAVE_ADDRESS)
> + return 1;
Leaked incremented reference to a child device node, please add
of_node_put(child) before return in the loop body.
Also reg variable may be uninitialized here, if child device node
does not have "reg" property.
> + }
> + } else if (IS_BUILTIN(CONFIG_ACPI) && ACPI_HANDLE(dev)) {
> + dev_dbg(dev, "ACPI slave is not supported yet\n");
> + }
If so, then it might be better to drop else-if stub for now.
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(i2c_slave_mode_detect);
> +
> #endif
>
> MODULE_AUTHOR("Simon G. Vogl <simon-nD9nYVNVf00W+b/DJNNodF6hYfS7NtTn@public.gmane.org>");
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index b2109c522dec..53cf99569af5 100644
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -282,6 +282,7 @@ enum i2c_slave_event {
>
> extern int i2c_slave_register(struct i2c_client *client, i2c_slave_cb_t slave_cb);
> extern int i2c_slave_unregister(struct i2c_client *client);
> +extern int i2c_slave_mode_detect(struct device *dev);
>
> static inline int i2c_slave_event(struct i2c_client *client,
> enum i2c_slave_event event, u8 *val)
>
--
With best wishes,
Vladimir
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] i2c: i2c-cadence: Don't register the adapter until it's ready
From: Vladimir Zapolskiy @ 2017-01-06 21:34 UTC (permalink / raw)
To: Mike Looijmans, linux-i2c
Cc: linux-kernel, linux-arm-kernel, wsa, soren.brinkmann,
michal.simek
In-Reply-To: <1483613240-32501-1-git-send-email-mike.looijmans@topic.nl>
Hello Mike,
On 01/05/2017 12:47 PM, Mike Looijmans wrote:
> The driver calls i2c_add_adapter before writing to config registers,
> resulting in dmesg output like this, where devices fail to initialize:
>
> cdns-i2c ff030000.i2c: timeout waiting on completion
> pca953x 1-0041: failed reading register
> pca953x: probe of 1-0041 failed with error -110
> at24 1-0050: 512 byte 24c04 EEPROM, writable, 1 bytes/write
> cdns-i2c ff030000.i2c: 100 kHz mmio ff030000 irq 197
>
> The adapter is being used before it completed the "probe". To fix
> this, make "i2c_add_adapter" the last thing it calls in probe.
> It also makes sense to show the adapter initialization before
> the devices on the bus.
commonly "it also" in a commit message means a change, which should be done
separately, and this is the case here as well.
Because the adapter registration i2c_add_adapter() can fail, information
about the adapter initialization would be expected only in case of
successful registration.
The information sent to the kernel log buffer here is quite trivial,
probably dev_info() can be just removed, but in any case it should be
a separate change.
> Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
--
With best wishes,
Vladimir
^ permalink raw reply
* Re: [PATCH] i2c: core: helper function to detect slave mode
From: Andy Shevchenko @ 2017-01-06 21:32 UTC (permalink / raw)
To: Luis Oliveira
Cc: Wolfram Sang, Rob Herring, Mark Rutland, Jarkko Nikula,
Andy Shevchenko, Mika Westerberg,
linux-i2c-u79uwXL29TY76Z2rM5mHXA, devicetree,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Ramiro.Oliveira-HKixBCOQz3hWk0Htik3J/w, Joao Pinto,
CARLOS.PALMINHA-HKixBCOQz3hWk0Htik3J/w
In-Reply-To: <c73ee9ab-64a1-0419-314d-21d4575ca7bf-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
On Fri, Jan 6, 2017 at 7:46 PM, Luis Oliveira
<Luis.Oliveira-HKixBCOQz3hWk0Htik3J/w@public.gmane.org> wrote:
> On 06-Jan-17 17:17, Andy Shevchenko wrote:
>> On Fri, Jan 6, 2017 at 7:15 PM, Luis Oliveira
>> <Luis.Oliveira-HKixBCOQz3hWk0Htik3J/w@public.gmane.org> wrote:
>>> On 06-Jan-17 16:29, Andy Shevchenko wrote:
>>>> On Thu, Jan 5, 2017 at 7:24 PM, Luis Oliveira
>>
>>>> Please, add kernel doc description here, important thing is to explain
>>>> return codes in Return: section of it.
>>>>
>>>>> +int i2c_slave_mode_detect(struct device *dev)
>>
>> Just to make sure you didn't miss this one.
>>
>>
> Yes, I saw it. You were talking of something like this, right?
>
> /**
> * i2c_slave_mode_detect - detect operation mode
> * @dev: The device owning the bus
> *
> * This checks the device nodes for a I2C slave by checking the address
a -> an
> * used.
> *
> * Returns true if a I2C slave is detected, otherwise returns false.
It has int type and I would reword this accordingly.
Something like
* Return:
* 1 when an I2C slave is detected, 0 for I2C master mode,
* and negative error otherwise.
> */
--
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [Patch V1] i2c: imx-lpi2c: add VLLS mode support
From: Vladimir Zapolskiy @ 2017-01-06 21:20 UTC (permalink / raw)
To: Gao Pan, wsa, wsa-dev; +Cc: linux-i2c, frank.li, fugang.duan
In-Reply-To: <1483521616-17914-1-git-send-email-pandy.gao@nxp.com>
Hi,
On 01/04/2017 11:20 AM, Gao Pan wrote:
> When system enters VLLS mode, module power is turned off. As a result,
> all registers are reset to HW default value. After exiting VLLS mode,
> registers are still in default mode. As a result, the pinctrl settings
> are incorrect, which will affect the module function.
>
> The patch recovers the pinctrl setting when exit VLLS mode.
>
> Signed-off-by: Gao Pan <pandy.gao@nxp.com>
> ---
> drivers/i2c/busses/i2c-imx-lpi2c.c | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
> diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c
> index c62b7cd..e528c1d1 100644
> --- a/drivers/i2c/busses/i2c-imx-lpi2c.c
> +++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
> @@ -636,12 +636,34 @@ static int lpi2c_imx_remove(struct platform_device *pdev)
> return 0;
> }
>
> +#ifdef CONFIG_PM_SLEEP
> +static int lpi2c_imx_suspend(struct device *dev)
> +{
> + pinctrl_pm_select_sleep_state(dev);
> +
> + return 0;
> +}
> +
> +static int lpi2c_imx_resume(struct device *dev)
> +{
> + pinctrl_pm_select_default_state(dev);
> +
> + return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(imx_lpi2c_pm, lpi2c_imx_suspend, lpi2c_imx_resume);
> +#define IMX_LPI2C_PM (&imx_lpi2c_pm)
> +#else
> +#define IMX_LPI2C_PM NULL
Please drop #else branch and IMX_LPI2C_PM completely.
> +#endif
> +
> static struct platform_driver lpi2c_imx_driver = {
> .probe = lpi2c_imx_probe,
> .remove = lpi2c_imx_remove,
> .driver = {
> .name = DRIVER_NAME,
> .of_match_table = lpi2c_imx_of_match,
> + .pm = IMX_LPI2C_PM,
Here it should be:
.pm = &imx_lpi2c_pm,
> },
> };
>
--
With best wishes,
Vladimir
^ permalink raw reply
* [PATCH v2] i2c: core: helper function to detect slave mode
From: Luis Oliveira @ 2017-01-06 18:14 UTC (permalink / raw)
To: wsa, robh+dt, mark.rutland, jarkko.nikula, andriy.shevchenko,
mika.westerberg, linux-i2c, devicetree, linux-kernel
Cc: Luis.Oliveira, Ramiro.Oliveira, Joao.Pinto, CARLOS.PALMINHA
This function has the purpose of mode detection by checking the
device nodes for a reg matching with the I2C_OWN_SLAVE_ADDREESS flag.
Currently only checks using OF functions (ACPI slave not supported yet).
Signed-off-by: Luis Oliveira <lolivei@synopsys.com>
Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
Changes V1->V2:
- Kernel doc added to i2c-core. (@Andy)
- Moved the definitions to the OF branch. (@Andy)
- Changed the return type from int to bool. (@Rob)
drivers/i2c/i2c-core.c | 28 ++++++++++++++++++++++++++++
include/linux/i2c.h | 1 +
2 files changed, 29 insertions(+)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 3de95a29024c..937605e65976 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -3691,6 +3691,34 @@ int i2c_slave_unregister(struct i2c_client *client)
return ret;
}
EXPORT_SYMBOL_GPL(i2c_slave_unregister);
+
+/**
+ * i2c_slave_mode_detect - detect operation mode
+ * @dev: The device owning the bus
+ *
+ * This checks the device nodes for a I2C slave by checking the address
+ * used.
+ *
+ * Returns true if a I2C slave is detected, otherwise returns false.
+ */
+bool i2c_slave_mode_detect(struct device *dev)
+{
+ if (IS_BUILTIN(CONFIG_OF) && dev->of_node) {
+ struct device_node *child;
+ u32 reg;
+
+ for_each_child_of_node(dev->of_node, child) {
+ of_property_read_u32(child, "reg", ®);
+ if (reg & I2C_OWN_SLAVE_ADDRESS)
+ return true;
+ }
+ } else if (IS_BUILTIN(CONFIG_ACPI) && ACPI_HANDLE(dev)) {
+ dev_dbg(dev, "ACPI slave is not supported yet\n");
+ }
+ return false;
+}
+EXPORT_SYMBOL_GPL(i2c_slave_mode_detect);
+
#endif
MODULE_AUTHOR("Simon G. Vogl <simon@tk.uni-linz.ac.at>");
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index b2109c522dec..bfc3e0cee57e 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -282,6 +282,7 @@ enum i2c_slave_event {
extern int i2c_slave_register(struct i2c_client *client, i2c_slave_cb_t slave_cb);
extern int i2c_slave_unregister(struct i2c_client *client);
+extern bool i2c_slave_mode_detect(struct device *dev);
static inline int i2c_slave_event(struct i2c_client *client,
enum i2c_slave_event event, u8 *val)
--
2.11.0
^ permalink raw reply related
* Re: [PATCH] i2c: core: helper function to detect slave mode
From: Luis Oliveira @ 2017-01-06 17:46 UTC (permalink / raw)
To: Andy Shevchenko, Luis Oliveira
Cc: Wolfram Sang, Rob Herring, Mark Rutland, Jarkko Nikula,
Andy Shevchenko, Mika Westerberg, linux-i2c, devicetree,
linux-kernel@vger.kernel.org, Ramiro.Oliveira, Joao Pinto,
CARLOS.PALMINHA
In-Reply-To: <CAHp75Vdv3daVnkFmGc7gXWoqcy+CNgyqdtRXkvTJQpjyJX3eDg@mail.gmail.com>
On 06-Jan-17 17:17, Andy Shevchenko wrote:
> On Fri, Jan 6, 2017 at 7:15 PM, Luis Oliveira
> <Luis.Oliveira@synopsys.com> wrote:
>> On 06-Jan-17 16:29, Andy Shevchenko wrote:
>>> On Thu, Jan 5, 2017 at 7:24 PM, Luis Oliveira
>
>>> Please, add kernel doc description here, important thing is to explain
>>> return codes in Return: section of it.
>>>
>>>> +int i2c_slave_mode_detect(struct device *dev)
>
> Just to make sure you didn't miss this one.
>
>
Yes, I saw it. You were talking of something like this, right?
/**
* i2c_slave_mode_detect - detect operation mode
* @dev: The device owning the bus
*
* This checks the device nodes for a I2C slave by checking the address
* used.
*
* Returns true if a I2C slave is detected, otherwise returns false.
*/
^ permalink raw reply
* Re: [PATCH] i2c: core: helper function to detect slave mode
From: Andy Shevchenko @ 2017-01-06 17:17 UTC (permalink / raw)
To: Luis Oliveira
Cc: Wolfram Sang, Rob Herring, Mark Rutland, Jarkko Nikula,
Andy Shevchenko, Mika Westerberg,
linux-i2c-u79uwXL29TY76Z2rM5mHXA, devicetree,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Ramiro.Oliveira-HKixBCOQz3hWk0Htik3J/w, Joao Pinto,
CARLOS.PALMINHA-HKixBCOQz3hWk0Htik3J/w
In-Reply-To: <475d9c2a-da11-97d0-d0b6-37ccd4990f18-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
On Fri, Jan 6, 2017 at 7:15 PM, Luis Oliveira
<Luis.Oliveira-HKixBCOQz3hWk0Htik3J/w@public.gmane.org> wrote:
> On 06-Jan-17 16:29, Andy Shevchenko wrote:
>> On Thu, Jan 5, 2017 at 7:24 PM, Luis Oliveira
>> Please, add kernel doc description here, important thing is to explain
>> return codes in Return: section of it.
>>
>>> +int i2c_slave_mode_detect(struct device *dev)
Just to make sure you didn't miss this one.
--
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] i2c: core: helper function to detect slave mode
From: Luis Oliveira @ 2017-01-06 17:15 UTC (permalink / raw)
To: Andy Shevchenko, Luis Oliveira
Cc: Wolfram Sang, Rob Herring, Mark Rutland, Jarkko Nikula,
Andy Shevchenko, Mika Westerberg, linux-i2c, devicetree,
linux-kernel@vger.kernel.org, Ramiro.Oliveira, Joao Pinto,
CARLOS.PALMINHA
In-Reply-To: <CAHp75VdB+pApFgjFOwWYYQFUmxv5k6OYKgB4fstuy=3=2QesBQ@mail.gmail.com>
On 06-Jan-17 16:29, Andy Shevchenko wrote:
> On Thu, Jan 5, 2017 at 7:24 PM, Luis Oliveira
> <Luis.Oliveira@synopsys.com> wrote:
>> This function has the purpose of mode detection by checking the
>> device nodes for a reg matching with the I2C_OWN_SLAVE_ADDREESS flag.
>> Currently only checks using OF functions (ACPI slave not supported yet).
>
> The code looks good, one important comment below, after addressing it:
>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>
> P.S. Btw, we have Suggested-by tag for giving credit for suggestion.
> Please use it.
>
I will do it in the V2, thank you.
>> --- a/drivers/i2c/i2c-core.c
>> +++ b/drivers/i2c/i2c-core.c
>> @@ -3691,6 +3691,25 @@ int i2c_slave_unregister(struct i2c_client *client)
>
> Please, add kernel doc description here, important thing is to explain
> return codes in Return: section of it.
>
>> +int i2c_slave_mode_detect(struct device *dev)
>> +{
>
>> + struct device_node *child;
>> + u32 reg;
>
> I would consider them as private to the OF branch. But it's really
> minor and up to you (I don't remember if we have such style examples
> in i2c core code).
I can change that in the V2 too.
>
>> +
>> + if (IS_BUILTIN(CONFIG_OF) && dev->of_node) {
>> + for_each_child_of_node(dev->of_node, child) {
>> + of_property_read_u32(child, "reg", ®);
>> + if (reg & I2C_OWN_SLAVE_ADDRESS)
>> + return 1;
>> + }
>> + } else if (IS_BUILTIN(CONFIG_ACPI) && ACPI_HANDLE(dev)) {
>> + dev_dbg(dev, "ACPI slave is not supported yet\n");
>> + }
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(i2c_slave_mode_detect);
>
^ permalink raw reply
* Re: [PATCH] i2c: core: helper function to detect slave mode
From: Luis Oliveira @ 2017-01-06 17:12 UTC (permalink / raw)
To: Rob Herring, Luis Oliveira
Cc: wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org, Mark Rutland,
jarkko.nikula-VuQAYsv1563Yd54FQh9/CA@public.gmane.org,
Andy Shevchenko,
mika.westerberg-VuQAYsv1563Yd54FQh9/CA@public.gmane.org,
linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, roliveir,
Joao Pinto, CARLOS.PALMINHA-HKixBCOQz3hWk0Htik3J/w
In-Reply-To: <CAL_JsqK1=R6M=vrW5aqmQtMHUGGws_Hb=VHb9RNcvesjfkCjRA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 06-Jan-17 16:35, Rob Herring wrote:
> On Thu, Jan 5, 2017 at 11:24 AM, Luis Oliveira
> <Luis.Oliveira-HKixBCOQz3hWk0Htik3J/w@public.gmane.org> wrote:
>> This function has the purpose of mode detection by checking the
>> device nodes for a reg matching with the I2C_OWN_SLAVE_ADDREESS flag.
>> Currently only checks using OF functions (ACPI slave not supported yet).
>>
>> Signed-off-by: Luis Oliveira <lolivei-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
>> ---
>> Due to the need of checking if the I2C slave address is our own (in
>> other words: if we are the I2C slave) I created a helper function
>> (proposed to me by @Andy) to enable that check.
>> Currently (because I am not able to test it using ACPI) it only
>> supports devicetree declarations.
>>
>> drivers/i2c/i2c-core.c | 19 +++++++++++++++++++
>> include/linux/i2c.h | 1 +
>> 2 files changed, 20 insertions(+)
>>
>> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
>> index 3de95a29024c..48e705b23c59 100644
>> --- a/drivers/i2c/i2c-core.c
>> +++ b/drivers/i2c/i2c-core.c
>> @@ -3691,6 +3691,25 @@ int i2c_slave_unregister(struct i2c_client *client)
>> return ret;
>> }
>> EXPORT_SYMBOL_GPL(i2c_slave_unregister);
>> +
>> +int i2c_slave_mode_detect(struct device *dev)
>
> This can be bool instead. Otherwise, looks good to me.
>
Ok great, thank you.
>> +{
>> + struct device_node *child;
>> + u32 reg;
>> +
>> + if (IS_BUILTIN(CONFIG_OF) && dev->of_node) {
>> + for_each_child_of_node(dev->of_node, child) {
>> + of_property_read_u32(child, "reg", ®);
>> + if (reg & I2C_OWN_SLAVE_ADDRESS)
>> + return 1;
>> + }
>> + } else if (IS_BUILTIN(CONFIG_ACPI) && ACPI_HANDLE(dev)) {
>> + dev_dbg(dev, "ACPI slave is not supported yet\n");
>> + }
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(i2c_slave_mode_detect);
>> +
>> #endif
>>
>> MODULE_AUTHOR("Simon G. Vogl <simon-nD9nYVNVf00W+b/DJNNodF6hYfS7NtTn@public.gmane.org>");
>> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
>> index b2109c522dec..53cf99569af5 100644
>> --- a/include/linux/i2c.h
>> +++ b/include/linux/i2c.h
>> @@ -282,6 +282,7 @@ enum i2c_slave_event {
>>
>> extern int i2c_slave_register(struct i2c_client *client, i2c_slave_cb_t slave_cb);
>> extern int i2c_slave_unregister(struct i2c_client *client);
>> +extern int i2c_slave_mode_detect(struct device *dev);
>>
>> static inline int i2c_slave_event(struct i2c_client *client,
>> enum i2c_slave_event event, u8 *val)
>> --
>> 2.11.0
>>
>>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] i2c: core: helper function to detect slave mode
From: Rob Herring @ 2017-01-06 16:35 UTC (permalink / raw)
To: Luis Oliveira
Cc: wsa@the-dreams.de, Mark Rutland, jarkko.nikula@linux.intel.com,
Andy Shevchenko, mika.westerberg@linux.intel.com,
linux-i2c@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, roliveir, Joao Pinto,
CARLOS.PALMINHA
In-Reply-To: <f4ec62119145bc70c938800c91eb396d30457304.1483636071.git.lolivei@synopsys.com>
On Thu, Jan 5, 2017 at 11:24 AM, Luis Oliveira
<Luis.Oliveira@synopsys.com> wrote:
> This function has the purpose of mode detection by checking the
> device nodes for a reg matching with the I2C_OWN_SLAVE_ADDREESS flag.
> Currently only checks using OF functions (ACPI slave not supported yet).
>
> Signed-off-by: Luis Oliveira <lolivei@synopsys.com>
> ---
> Due to the need of checking if the I2C slave address is our own (in
> other words: if we are the I2C slave) I created a helper function
> (proposed to me by @Andy) to enable that check.
> Currently (because I am not able to test it using ACPI) it only
> supports devicetree declarations.
>
> drivers/i2c/i2c-core.c | 19 +++++++++++++++++++
> include/linux/i2c.h | 1 +
> 2 files changed, 20 insertions(+)
>
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index 3de95a29024c..48e705b23c59 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -3691,6 +3691,25 @@ int i2c_slave_unregister(struct i2c_client *client)
> return ret;
> }
> EXPORT_SYMBOL_GPL(i2c_slave_unregister);
> +
> +int i2c_slave_mode_detect(struct device *dev)
This can be bool instead. Otherwise, looks good to me.
> +{
> + struct device_node *child;
> + u32 reg;
> +
> + if (IS_BUILTIN(CONFIG_OF) && dev->of_node) {
> + for_each_child_of_node(dev->of_node, child) {
> + of_property_read_u32(child, "reg", ®);
> + if (reg & I2C_OWN_SLAVE_ADDRESS)
> + return 1;
> + }
> + } else if (IS_BUILTIN(CONFIG_ACPI) && ACPI_HANDLE(dev)) {
> + dev_dbg(dev, "ACPI slave is not supported yet\n");
> + }
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(i2c_slave_mode_detect);
> +
> #endif
>
> MODULE_AUTHOR("Simon G. Vogl <simon@tk.uni-linz.ac.at>");
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index b2109c522dec..53cf99569af5 100644
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -282,6 +282,7 @@ enum i2c_slave_event {
>
> extern int i2c_slave_register(struct i2c_client *client, i2c_slave_cb_t slave_cb);
> extern int i2c_slave_unregister(struct i2c_client *client);
> +extern int i2c_slave_mode_detect(struct device *dev);
>
> static inline int i2c_slave_event(struct i2c_client *client,
> enum i2c_slave_event event, u8 *val)
> --
> 2.11.0
>
>
^ permalink raw reply
* Re: [PATCH] i2c: core: helper function to detect slave mode
From: Andy Shevchenko @ 2017-01-06 16:29 UTC (permalink / raw)
To: Luis Oliveira
Cc: Wolfram Sang, Rob Herring, Mark Rutland, Jarkko Nikula,
Andy Shevchenko, Mika Westerberg, linux-i2c, devicetree,
linux-kernel@vger.kernel.org, Ramiro.Oliveira, Joao Pinto,
CARLOS.PALMINHA
In-Reply-To: <f4ec62119145bc70c938800c91eb396d30457304.1483636071.git.lolivei@synopsys.com>
On Thu, Jan 5, 2017 at 7:24 PM, Luis Oliveira
<Luis.Oliveira@synopsys.com> wrote:
> This function has the purpose of mode detection by checking the
> device nodes for a reg matching with the I2C_OWN_SLAVE_ADDREESS flag.
> Currently only checks using OF functions (ACPI slave not supported yet).
The code looks good, one important comment below, after addressing it:
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
P.S. Btw, we have Suggested-by tag for giving credit for suggestion.
Please use it.
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -3691,6 +3691,25 @@ int i2c_slave_unregister(struct i2c_client *client)
Please, add kernel doc description here, important thing is to explain
return codes in Return: section of it.
> +int i2c_slave_mode_detect(struct device *dev)
> +{
> + struct device_node *child;
> + u32 reg;
I would consider them as private to the OF branch. But it's really
minor and up to you (I don't remember if we have such style examples
in i2c core code).
> +
> + if (IS_BUILTIN(CONFIG_OF) && dev->of_node) {
> + for_each_child_of_node(dev->of_node, child) {
> + of_property_read_u32(child, "reg", ®);
> + if (reg & I2C_OWN_SLAVE_ADDRESS)
> + return 1;
> + }
> + } else if (IS_BUILTIN(CONFIG_ACPI) && ACPI_HANDLE(dev)) {
> + dev_dbg(dev, "ACPI slave is not supported yet\n");
> + }
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(i2c_slave_mode_detect);
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v3] i2c: designware: add reset interface
From: Arnd Bergmann @ 2017-01-06 11:32 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Wolfram Sang, jarkko.nikula, linux-i2c, p.zabel, Zhangfei Gao,
Andy Shevchenko, mika.westerberg
In-Reply-To: <1483544119.9552.223.camel@linux.intel.com>
On Wednesday, January 4, 2017 5:35:19 PM CET Andy Shevchenko wrote:
> > > @@ -270,10 +280,18 @@ static int dw_i2c_plat_probe(struct
> > > platform_device *pdev)
> > > }
> > >
> > > r = i2c_dw_probe(dev);
> > > - if (r && !dev->pm_runtime_disabled)
> > > - pm_runtime_disable(&pdev->dev);
> > > + if (r)
> > > + goto exit_probe;
> > >
> > > return r;
> > > +
> > > +exit_probe:
> > > + if (!dev->pm_runtime_disabled)
> > > + pm_runtime_disable(&pdev->dev);
> > > +exit_reset:
> > > + if (!IS_ERR_OR_NULL(dev->rst))
> > > + reset_control_assert(dev->rst);
> > > + return r;
> > >
> >
> > try to avoid the IS_ERR_OR_NULL() check, it usually indicates either
> > a bad interface, or that the interface is used wrong.
>
> Please, fix reset framework first than.
>
> For my understanding:
> It should return NULL for optional reset control.
> It should not fail on NULL argument.
I think we discussed that a few times. Your suggestion makes sense
to me, but I don't know why we don't already do that, maybe there
is a good reason.
Arnd
^ permalink raw reply
* [PATCH] i2c: print correct device invalid address
From: John Garry @ 2017-01-06 11:02 UTC (permalink / raw)
To: wsa; +Cc: linux-i2c, linux-kernel, linuxarm, John Garry
In of_i2c_register_device(), when the check for
device address validity fails we print the info.addr,
which has not been assigned properly.
Fix this by printing the actual invalid address.
Signed-off-by: John Garry <john.garry@huawei.com>
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index cf9e396..03cfcdd 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -1708,7 +1708,7 @@ static struct i2c_client *of_i2c_register_device(struct i2c_adapter *adap,
if (i2c_check_addr_validity(addr, info.flags)) {
dev_err(&adap->dev, "of_i2c: invalid addr=%x on %s\n",
- info.addr, node->full_name);
+ addr, node->full_name);
return ERR_PTR(-EINVAL);
}
--
1.9.1
^ permalink raw reply related
* [PATCH] i2c: core: helper function to detect slave mode
From: Luis Oliveira @ 2017-01-05 17:24 UTC (permalink / raw)
To: wsa-z923LK4zBo2bacvFa/9K2g, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, jarkko.nikula-VuQAYsv1563Yd54FQh9/CA,
andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA,
mika.westerberg-VuQAYsv1563Yd54FQh9/CA,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Luis.Oliveira-HKixBCOQz3hWk0Htik3J/w,
Ramiro.Oliveira-HKixBCOQz3hWk0Htik3J/w,
Joao.Pinto-HKixBCOQz3hWk0Htik3J/w,
CARLOS.PALMINHA-HKixBCOQz3hWk0Htik3J/w
This function has the purpose of mode detection by checking the
device nodes for a reg matching with the I2C_OWN_SLAVE_ADDREESS flag.
Currently only checks using OF functions (ACPI slave not supported yet).
Signed-off-by: Luis Oliveira <lolivei-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
---
Due to the need of checking if the I2C slave address is our own (in
other words: if we are the I2C slave) I created a helper function
(proposed to me by @Andy) to enable that check.
Currently (because I am not able to test it using ACPI) it only
supports devicetree declarations.
drivers/i2c/i2c-core.c | 19 +++++++++++++++++++
include/linux/i2c.h | 1 +
2 files changed, 20 insertions(+)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 3de95a29024c..48e705b23c59 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -3691,6 +3691,25 @@ int i2c_slave_unregister(struct i2c_client *client)
return ret;
}
EXPORT_SYMBOL_GPL(i2c_slave_unregister);
+
+int i2c_slave_mode_detect(struct device *dev)
+{
+ struct device_node *child;
+ u32 reg;
+
+ if (IS_BUILTIN(CONFIG_OF) && dev->of_node) {
+ for_each_child_of_node(dev->of_node, child) {
+ of_property_read_u32(child, "reg", ®);
+ if (reg & I2C_OWN_SLAVE_ADDRESS)
+ return 1;
+ }
+ } else if (IS_BUILTIN(CONFIG_ACPI) && ACPI_HANDLE(dev)) {
+ dev_dbg(dev, "ACPI slave is not supported yet\n");
+ }
+ return 0;
+}
+EXPORT_SYMBOL_GPL(i2c_slave_mode_detect);
+
#endif
MODULE_AUTHOR("Simon G. Vogl <simon-nD9nYVNVf00W+b/DJNNodF6hYfS7NtTn@public.gmane.org>");
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index b2109c522dec..53cf99569af5 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -282,6 +282,7 @@ enum i2c_slave_event {
extern int i2c_slave_register(struct i2c_client *client, i2c_slave_cb_t slave_cb);
extern int i2c_slave_unregister(struct i2c_client *client);
+extern int i2c_slave_mode_detect(struct device *dev);
static inline int i2c_slave_event(struct i2c_client *client,
enum i2c_slave_event event, u8 *val)
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH v7 05/12] mux: support simplified bindings for single-user gpio mux
From: Peter Rosin @ 2017-01-05 16:21 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Wolfram Sang, Rob Herring, Mark Rutland, Jonathan Cameron,
Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
Jonathan Corbet, Arnd Bergmann, Greg Kroah-Hartman,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1483532187-28494-6-git-send-email-peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
On 2017-01-04 13:16, Peter Rosin wrote:
> Signed-off-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
> ---
> drivers/mux/mux-core.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++--
> drivers/mux/mux-gpio.c | 56 ++--------------------------------
> include/linux/mux.h | 7 +++++
> 3 files changed, 89 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/mux/mux-core.c b/drivers/mux/mux-core.c
> index 21da15a264ad..d887ae1c0e55 100644
> --- a/drivers/mux/mux-core.c
> +++ b/drivers/mux/mux-core.c
> @@ -15,6 +15,7 @@
> #include <linux/device.h>
> #include <linux/err.h>
> #include <linux/idr.h>
> +#include <linux/gpio/consumer.h>
> #include <linux/module.h>
> #include <linux/mux.h>
> #include <linux/of.h>
> @@ -288,6 +289,63 @@ static struct mux_chip *of_find_mux_chip_by_node(struct device_node *np)
> return dev ? to_mux_chip(dev) : NULL;
> }
>
> +#ifdef CONFIG_MUX_GPIO
> +
> +static int mux_gpio_set(struct mux_control *mux, int state)
> +{
> + struct mux_gpio *mux_gpio = mux_chip_priv(mux->chip);
> + int i;
> +
> + for (i = 0; i < mux_gpio->gpios->ndescs; i++)
> + mux_gpio->val[i] = (state >> i) & 1;
> +
> + gpiod_set_array_value_cansleep(mux_gpio->gpios->ndescs,
> + mux_gpio->gpios->desc,
> + mux_gpio->val);
> +
> + return 0;
> +}
> +
> +static const struct mux_control_ops mux_gpio_ops = {
> + .set = mux_gpio_set,
> +};
> +
> +struct mux_chip *mux_gpio_alloc(struct device *dev)
> +{
> + struct mux_chip *mux_chip;
> + struct mux_gpio *mux_gpio;
> + int pins;
> + int ret;
> +
> + pins = gpiod_count(dev, "mux");
> + if (pins < 0)
> + return ERR_PTR(pins);
> +
> + mux_chip = devm_mux_chip_alloc(dev, 1, sizeof(*mux_gpio) +
> + pins * sizeof(*mux_gpio->val));
> + if (!mux_chip)
> + return ERR_PTR(-ENOMEM);
> +
> + mux_gpio = mux_chip_priv(mux_chip);
> + mux_gpio->val = (int *)(mux_gpio + 1);
> + mux_chip->ops = &mux_gpio_ops;
> +
> + mux_gpio->gpios = devm_gpiod_get_array(dev, "mux", GPIOD_OUT_LOW);
> + if (IS_ERR(mux_gpio->gpios)) {
> + ret = PTR_ERR(mux_gpio->gpios);
> + if (ret != -EPROBE_DEFER)
> + dev_err(dev, "failed to get gpios\n");
> + return ERR_PTR(ret);
> + }
> + WARN_ON(pins != mux_gpio->gpios->ndescs);
> + mux_chip->mux->states = 1 << pins;
> +
> + return mux_chip;
> +}
> +EXPORT_SYMBOL_GPL(mux_gpio_alloc);
> +
> +#endif /* CONFIG_MUX_GPIO */
> +
> struct mux_control *mux_control_get(struct device *dev, const char *mux_name)
> {
> struct device_node *np = dev->of_node;
> @@ -307,9 +365,28 @@ struct mux_control *mux_control_get(struct device *dev, const char *mux_name)
> ret = of_parse_phandle_with_args(np,
> "mux-controls", "#mux-control-cells",
> index, &args);
> +
> +#ifdef CONFIG_MUX_GPIO
> + if (ret == -ENOENT && !mux_name && gpiod_count(dev, "mux") > 0) {
> + mux_chip = mux_gpio_alloc(dev);
> + if (!IS_ERR(mux_chip)) {
> + ret = devm_mux_chip_register(dev, mux_chip);
> + if (ret < 0) {
> + dev_err(dev, "failed to register mux-chip\n");
> + return ERR_PTR(ret);
> + }
> + get_device(&mux_chip->dev);
> + return mux_chip->mux;
> + }
> +
> + ret = PTR_ERR(mux_chip);
> + }
> +#endif
> +
> if (ret) {
> - dev_err(dev, "%s: failed to get mux-control %s(%i)\n",
> - np->full_name, mux_name ?: "", index);
> + if (ret != -EPROBE_DEFER)
> + dev_err(dev, "%s: failed to get mux-control %s(%i)\n",
> + np->full_name, mux_name ?: "", index);
> return ERR_PTR(ret);
> }
>
> diff --git a/drivers/mux/mux-gpio.c b/drivers/mux/mux-gpio.c
> index 76b52bc63470..8a7bfbc0c4bb 100644
> --- a/drivers/mux/mux-gpio.c
> +++ b/drivers/mux/mux-gpio.c
> @@ -11,37 +11,12 @@
> */
>
> #include <linux/err.h>
> -#include <linux/gpio/consumer.h>
> #include <linux/module.h>
> #include <linux/mux.h>
> #include <linux/of_platform.h>
> #include <linux/platform_device.h>
> #include <linux/property.h>
Instead of moving the mux-gpio guts from mux-gpio.c to mux-core.c, I
will instead make CONFIG_MUX_GPIO a bool option (no module possible)
and call it from the mux-core. That will be cleaner and less of a
break of abstractions in my opinion.
Cheers,
Peter
> -struct mux_gpio {
> - struct gpio_descs *gpios;
> - int *val;
> -};
> -
> -static int mux_gpio_set(struct mux_control *mux, int state)
> -{
> - struct mux_gpio *mux_gpio = mux_chip_priv(mux->chip);
> - int i;
> -
> - for (i = 0; i < mux_gpio->gpios->ndescs; i++)
> - mux_gpio->val[i] = (state >> i) & 1;
> -
> - gpiod_set_array_value_cansleep(mux_gpio->gpios->ndescs,
> - mux_gpio->gpios->desc,
> - mux_gpio->val);
> -
> - return 0;
> -}
> -
> -static const struct mux_control_ops mux_gpio_ops = {
> - .set = mux_gpio_set,
> -};
> -
> static const struct of_device_id mux_gpio_dt_ids[] = {
> { .compatible = "mux-gpio", },
> { /* sentinel */ }
> @@ -51,38 +26,13 @@ MODULE_DEVICE_TABLE(of, mux_gpio_dt_ids);
> static int mux_gpio_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> - struct device_node *np = dev->of_node;
> struct mux_chip *mux_chip;
> - struct mux_gpio *mux_gpio;
> - int pins;
> u32 idle_state;
> int ret;
>
> - if (!np)
> - return -ENODEV;
> -
> - pins = gpiod_count(dev, "mux");
> - if (pins < 0)
> - return pins;
> -
> - mux_chip = devm_mux_chip_alloc(dev, 1, sizeof(*mux_gpio) +
> - pins * sizeof(*mux_gpio->val));
> - if (!mux_chip)
> - return -ENOMEM;
> -
> - mux_gpio = mux_chip_priv(mux_chip);
> - mux_gpio->val = (int *)(mux_gpio + 1);
> - mux_chip->ops = &mux_gpio_ops;
> -
> - mux_gpio->gpios = devm_gpiod_get_array(dev, "mux", GPIOD_OUT_LOW);
> - if (IS_ERR(mux_gpio->gpios)) {
> - ret = PTR_ERR(mux_gpio->gpios);
> - if (ret != -EPROBE_DEFER)
> - dev_err(dev, "failed to get gpios\n");
> - return ret;
> - }
> - WARN_ON(pins != mux_gpio->gpios->ndescs);
> - mux_chip->mux->states = 1 << pins;
> + mux_chip = mux_gpio_alloc(dev);
> + if (IS_ERR(mux_chip))
> + return PTR_ERR(mux_chip);
>
> ret = device_property_read_u32(dev, "idle-state", &idle_state);
> if (ret >= 0) {
> diff --git a/include/linux/mux.h b/include/linux/mux.h
> index 3b9439927f11..3bfee23cfb8c 100644
> --- a/include/linux/mux.h
> +++ b/include/linux/mux.h
> @@ -241,4 +241,11 @@ struct mux_control *devm_mux_control_get(struct device *dev,
> */
> void devm_mux_control_put(struct device *dev, struct mux_control *mux);
>
> +struct mux_gpio {
> + struct gpio_descs *gpios;
> + int *val;
> +};
> +
> +struct mux_chip *mux_gpio_alloc(struct device *dev);
> +
> #endif /* _LINUX_MUX_H */
>
^ permalink raw reply
* Re: [PATCH v5 6/7] i2c: designware: enable SLAVE in platform module
From: Andy Shevchenko @ 2017-01-05 12:43 UTC (permalink / raw)
To: Luis Oliveira, wsa, robh+dt, mark.rutland, jarkko.nikula,
mika.westerberg, linux-i2c, devicetree, linux-kernel
Cc: Ramiro.Oliveira, Joao.Pinto, CARLOS.PALMINHA
In-Reply-To: <9747bb4d-9a52-789d-32e9-ddbef9e129e4@synopsys.com>
On Thu, 2017-01-05 at 12:13 +0000, Luis Oliveira wrote:
> On 28-Dec-16 18:10, Luis Oliveira wrote:
> > On 28-Dec-16 17:10, Andy Shevchenko wrote:
> > > On Wed, 2016-12-28 at 16:41 +0000, Luis Oliveira wrote:
> > > > On 28-Dec-16 16:31, Andy Shevchenko wrote:
> > > So, ACPI has a property to support slave mode for I2CSerialBus()
> > > macro.
> > >
> > > I would propose to create a helper function in i2c-core.c which
> > > will be
> > > responsible for mode detection
> > >
> > > ... i2c_slave_mode_detect()
> > > {
> > > ...
> > > if (IS_BUILTIN(CONFIG_OF) && dev->of_node) {
> > > ... (use of_*() here) ...
> > > } else if (IS_BUILTIN(CONFIG_ACPI) && ACPI_HANDLE(dev))
> > > dev_dbg(..., "ACPI slave is not supported yet\n");
> > > ... to master ...
> > > } else {
> > > ... default to master ...
> > > }
> > > }
> > > EXPORT_...();
> > >
> > > Make it as a separate patch.
> > >
> >
> > Oh I see, yes it looks good. I will check it. Thanks
> >
>
> Hi Andy,
>
> I implemented a helper function as you proposed and it looks like
> this:
>
> int i2c_slave_mode_detect(struct device *dev)
> {
> struct device_node *child;
> u32 reg;
>
> if (IS_BUILTIN(CONFIG_OF) && dev->of_node) {
> for_each_child_of_node(dev->of_node, child) {
> of_property_read_u32(child, "reg", ®);
> if (reg & I2C_OWN_SLAVE_ADDRESS)
> return 1;
> }
> } else if (IS_BUILTIN(CONFIG_ACPI) && ACPI_HANDLE(dev))
> dev_dbg(dev, "ACPI slave is not supported yet\n");
Curly braces here as well.
> else
> return 0;
For now this is not needed
> return 0;
> }
>
> Before I submit the patch to the i2c-core.c I wonder if I could have
> some
> comment on the implementation.
Glad you did it. See above. Otherwise looks good to me.
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply
* Re: [PATCH v2] i2c: do not enable fall back to Host Notify by default
From: Pali Rohár @ 2017-01-05 12:39 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Wolfram Sang, Rob Herring, Benjamin Tissoires,
Michał Kępień, Jean Delvare, Takashi Iwai,
linux-i2c, devicetree, linux-kernel
In-Reply-To: <20170105045722.GA17958@dtor-ws>
On Wednesday 04 January 2017 20:57:22 Dmitry Torokhov wrote:
> Falling back unconditionally to HostNotify as primary client's interrupt
> breaks some drivers which alter their functionality depending on whether
> interrupt is present or not, so let's introduce a board flag telling I2C
> core explicitly if we want wired interrupt or HostNotify-based one:
> I2C_CLIENT_HOST_NOTIFY.
>
> For DT-based systems we introduce "host-notify" property that we convert
> to I2C_CLIENT_HOST_NOTIFY board flag.
>
> Tested-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Acked-by: Pali Rohár <pali.rohar@gmail.com>
--
Pali Rohár
pali.rohar@gmail.com
^ permalink raw reply
* Re: [PATCH v5 6/7] i2c: designware: enable SLAVE in platform module
From: Luis Oliveira @ 2017-01-05 12:13 UTC (permalink / raw)
To: Andy Shevchenko, Luis Oliveira, wsa, robh+dt, mark.rutland,
jarkko.nikula, mika.westerberg, linux-i2c, devicetree,
linux-kernel
Cc: Ramiro.Oliveira, Joao.Pinto, CARLOS.PALMINHA
In-Reply-To: <b27405b9-b768-34e7-d61a-433100f9856d@synopsys.com>
On 28-Dec-16 18:10, Luis Oliveira wrote:
> On 28-Dec-16 17:10, Andy Shevchenko wrote:
>> On Wed, 2016-12-28 at 16:41 +0000, Luis Oliveira wrote:
>>> On 28-Dec-16 16:31, Andy Shevchenko wrote:
>>>> On Wed, 2016-12-28 at 15:53 +0000, Luis Oliveira wrote:
>>>>> On 28-Dec-16 15:44, Andy Shevchenko wrote:
>>>>>> On Wed, 2016-12-28 at 14:43 +0000, Luis Oliveira wrote:
>>>>>>> - Slave mode selected in platform module (devicetree support
>>>>>>> only)
>>>>>>> - Check for ACPI - not supported in SLAVE mode:
>>>>>>> - Changed the ifndef style to the use of ACPI_HANDLE that
>>>>>>> returns
>>>>>>> NULL
>>>>>>> if the device was not enumerated from ACPI namespace.
>>>>>>
>>>>>> I'm not sure what is wrong with ACPI?
>>>>>
>>>>> I dont have a way to test it. Just that.
>>>>
>>>> Okay, can you provide an excerpt to see how it will look like in
>>>> DTS?
>>>
>>> Yes, it looks like this now:
>>>
>>> i2c@0x2000 {
>>> compatible = "snps,designware-i2c";
>>> #address-cells = <1>;
>>> #size-cells = <0>;
>>> reg = <0x2000 0x100>;
>>> clock-frequency = <400000>;
>>> clocks = <&i2cclk>;
>>> interrupts = <0>;
>>>
>>> eeprom@64 {
>>> compatible = "linux,slave-24c02";
>>> reg = <0x40000064>;
>>> };
>>> };
>>
>> +1 to Carlos' comment.
>
> Agree, I'm on it.
>
>>
>>>>
>>>>>>> - dev->functionality = I2C_FUNC_10BIT_ADDR |
>>>>>>> DW_IC_DEFAULT_FUNCTIONALITY;
>>>>>>> -
>>>>>>> - i2c_dw_configure_master(pdev);
>>>>>>> + if (ACPI_HANDLE(&pdev->dev) == NULL) {
>>>>>>
>>>>>> I don't think you need this at all.
>>>>>
>>>>> This is to avoid the use of the "ifdef" style I used before.
>>>>
>>>> My point is to drop it completely.
>>>>
>>>>>>
>>>>>>> + device_for_each_child_node(&pdev->dev, child)
>>>>>>> {
>>>>>>
>>>>>> This is resource agnostic.
>>>>>>
>>>>>>> + fwnode_property_read_u32(child,
>>>>>>> "reg",
>>>>>>> ®);
>>>>>>
>>>>>> This is as well.
>>>>>
>>>>> Are you suggesting I use of_ functions?
>>>>
>>>> Nope. See above.
>>
>> So, ACPI has a property to support slave mode for I2CSerialBus() macro.
>>
>> I would propose to create a helper function in i2c-core.c which will be
>> responsible for mode detection
>>
>> ... i2c_slave_mode_detect()
>> {
>> ...
>> if (IS_BUILTIN(CONFIG_OF) && dev->of_node) {
>> ... (use of_*() here) ...
>> } else if (IS_BUILTIN(CONFIG_ACPI) && ACPI_HANDLE(dev))
>> dev_dbg(..., "ACPI slave is not supported yet\n");
>> ... to master ...
>> } else {
>> ... default to master ...
>> }
>> }
>> EXPORT_...();
>>
>> Make it as a separate patch.
>>
>
> Oh I see, yes it looks good. I will check it. Thanks
>
Hi Andy,
I implemented a helper function as you proposed and it looks like this:
int i2c_slave_mode_detect(struct device *dev)
{
struct device_node *child;
u32 reg;
if (IS_BUILTIN(CONFIG_OF) && dev->of_node) {
for_each_child_of_node(dev->of_node, child) {
of_property_read_u32(child, "reg", ®);
if (reg & I2C_OWN_SLAVE_ADDRESS)
return 1;
}
} else if (IS_BUILTIN(CONFIG_ACPI) && ACPI_HANDLE(dev))
dev_dbg(dev, "ACPI slave is not supported yet\n");
else
return 0;
return 0;
}
Before I submit the patch to the i2c-core.c I wonder if I could have some
comment on the implementation.
Thanks,
Luis
^ permalink raw reply
* [PATCH] i2c: exynos5: fix arbitration lost handling
From: Andrzej Hajda @ 2017-01-05 12:06 UTC (permalink / raw)
To: linux-i2c
Cc: Andrzej Hajda, Bartlomiej Zolnierkiewicz, Marek Szyprowski,
Wolfram Sang, Krzysztof Kozlowski, Javier Martinez Canillas,
linux-samsung-soc
In-Reply-To: <CGME20170105120727eucas1p2b9452792b7d13d551974305b478a26e2@eucas1p2.samsung.com>
In case of arbitration lost adequate interrupt sometimes is not signaled. As
a result transfer timeouts and is not retried, as it should. To avoid such
cases code is added to check transaction status in case of every interrupt.
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
Hi,
I am not I2C specialist, this patch is a result of debugging issues
with SiI8620 MHL driver on TM2 device. I guess the main problem is
with SiI8620 chip, but exynos5 i2c is also guilty - with i2c-gpio driver
it works correctly - there is only one arbitration lost at 1st transaction
(which is properly handled - transfer is repeated succesfully). In case
of exynos5 there are much more arbitration losts. Lowering bus speed
decreases frequency of losts but do not eliminate them.
Any help/hint, what could cause such issues?
I guess increasing adap.retries should also decrease fail probability,
currently for exynos5 it is set to 3.
Regards
Andrzej
---
drivers/i2c/busses/i2c-exynos5.c | 28 +++++++++++++++++++++++++++-
1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-exynos5.c b/drivers/i2c/busses/i2c-exynos5.c
index bea6071..a01c1ae 100644
--- a/drivers/i2c/busses/i2c-exynos5.c
+++ b/drivers/i2c/busses/i2c-exynos5.c
@@ -130,12 +130,32 @@
/* I2C_TRANS_STATUS register bits */
#define HSI2C_MASTER_BUSY (1u << 17)
#define HSI2C_SLAVE_BUSY (1u << 16)
+
+/* I2C_TRANS_STATUS register bits for Exynos5 variant */
#define HSI2C_TIMEOUT_AUTO (1u << 4)
#define HSI2C_NO_DEV (1u << 3)
#define HSI2C_NO_DEV_ACK (1u << 2)
#define HSI2C_TRANS_ABORT (1u << 1)
#define HSI2C_TRANS_DONE (1u << 0)
+/* I2C_TRANS_STATUS register bits for Exynos7 variant */
+#define HSI2C_MASTER_ST_MASK 0xf
+#define HSI2C_MASTER_ST_IDLE 0x0
+#define HSI2C_MASTER_ST_START 0x1
+#define HSI2C_MASTER_ST_RESTART 0x2
+#define HSI2C_MASTER_ST_STOP 0x3
+#define HSI2C_MASTER_ST_MASTER_ID 0x4
+#define HSI2C_MASTER_ST_ADDR0 0x5
+#define HSI2C_MASTER_ST_ADDR1 0x6
+#define HSI2C_MASTER_ST_ADDR2 0x7
+#define HSI2C_MASTER_ST_ADDR_SR 0x8
+#define HSI2C_MASTER_ST_READ 0x9
+#define HSI2C_MASTER_ST_WRITE 0xa
+#define HSI2C_MASTER_ST_NO_ACK 0xb
+#define HSI2C_MASTER_ST_LOSE 0xc
+#define HSI2C_MASTER_ST_WAIT 0xd
+#define HSI2C_MASTER_ST_WAIT_CMD 0xe
+
/* I2C_ADDR register bits */
#define HSI2C_SLV_ADDR_SLV(x) ((x & 0x3ff) << 0)
#define HSI2C_SLV_ADDR_MAS(x) ((x & 0x3ff) << 10)
@@ -437,6 +457,7 @@ static irqreturn_t exynos5_i2c_irq(int irqno, void *dev_id)
int_status = readl(i2c->regs + HSI2C_INT_STATUS);
writel(int_status, i2c->regs + HSI2C_INT_STATUS);
+ trans_status = readl(i2c->regs + HSI2C_TRANS_STATUS);
/* handle interrupt related to the transfer status */
if (i2c->variant->hw == HSI2C_EXYNOS7) {
@@ -460,8 +481,13 @@ static irqreturn_t exynos5_i2c_irq(int irqno, void *dev_id)
i2c->state = -ETIMEDOUT;
goto stop;
}
+
+ switch (trans_status & HSI2C_MASTER_ST_MASK) {
+ case HSI2C_MASTER_ST_LOSE:
+ i2c->state = -EAGAIN;
+ goto stop;
+ }
} else if (int_status & HSI2C_INT_I2C) {
- trans_status = readl(i2c->regs + HSI2C_TRANS_STATUS);
if (trans_status & HSI2C_NO_DEV_ACK) {
dev_dbg(i2c->dev, "No ACK from device\n");
i2c->state = -ENXIO;
--
2.7.4
^ permalink raw reply related
* [PATCH] i2c: i2c-cadence: Don't register the adapter until it's ready
From: Mike Looijmans @ 2017-01-05 10:47 UTC (permalink / raw)
To: linux-i2c
Cc: wsa, Mike Looijmans, michal.simek, linux-kernel, linux-arm-kernel,
soren.brinkmann
The driver calls i2c_add_adapter before writing to config registers,
resulting in dmesg output like this, where devices fail to initialize:
cdns-i2c ff030000.i2c: timeout waiting on completion
pca953x 1-0041: failed reading register
pca953x: probe of 1-0041 failed with error -110
at24 1-0050: 512 byte 24c04 EEPROM, writable, 1 bytes/write
cdns-i2c ff030000.i2c: 100 kHz mmio ff030000 irq 197
The adapter is being used before it completed the "probe". To fix
this, make "i2c_add_adapter" the last thing it calls in probe.
It also makes sense to show the adapter initialization before
the devices on the bus.
Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
---
drivers/i2c/busses/i2c-cadence.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c
index 6869712..7793bb8 100644
--- a/drivers/i2c/busses/i2c-cadence.c
+++ b/drivers/i2c/busses/i2c-cadence.c
@@ -962,10 +962,6 @@ static int cdns_i2c_probe(struct platform_device *pdev)
goto err_clk_dis;
}
- ret = i2c_add_adapter(&id->adap);
- if (ret < 0)
- goto err_clk_dis;
-
/*
* Cadence I2C controller has a bug wherein it generates
* invalid read transaction after HW timeout in master receiver mode.
@@ -978,6 +974,10 @@ static int cdns_i2c_probe(struct platform_device *pdev)
dev_info(&pdev->dev, "%u kHz mmio %08lx irq %d\n",
id->i2c_clk / 1000, (unsigned long)r_mem->start, id->irq);
+ ret = i2c_add_adapter(&id->adap);
+ if (ret < 0)
+ goto err_clk_dis;
+
return 0;
err_clk_dis:
--
1.9.1
^ permalink raw reply related
* Re: [PATCH] i2c: i801: Register optional lis3lv02d i2c device on Dell machines
From: Benjamin Tissoires @ 2017-01-05 9:26 UTC (permalink / raw)
To: Pali Rohár
Cc: Dmitry Torokhov, Wolfram Sang, Michał Kępień,
Jean Delvare, Steven Honeyman, Valdis.Kletnieks, Jochen Eisinger,
Gabriele Mazzotta, Andy Lutomirski, Mario_Limonciello, Alex Hung,
Takashi Iwai, linux-i2c, linux-kernel, platform-driver-x86
In-Reply-To: <20170105085419.GB31083@pali>
On Jan 05 2017 or thereabouts, Pali Rohár wrote:
> On Wednesday 04 January 2017 09:54:56 Dmitry Torokhov wrote:
> > On Wed, Jan 04, 2017 at 06:46:19PM +0100, Wolfram Sang wrote:
> > >
> > > > How about:
> > > > ---
> > > > From daa7571bbf337704332c0cfeec9b8fd5aeae596f Mon Sep 17 00:00:00 2001
> > > > From: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> > > > Date: Wed, 4 Jan 2017 18:26:54 +0100
> > > > Subject: [PATCH] I2C: add the source of the IRQ in struct i2c_client
> > > >
> > > > With commit 4d5538f5882a ("i2c: use an IRQ to report Host Notify events,
> > > > not alert"), the IRQ provided in struct i2c_client might be assigned while
> > > > it has not been explicitly declared by either the platform information
> > > > or OF or ACPI.
> > > > Some drivers (lis3lv02d) rely on the fact that the IRQ gets assigned or
> > > > not to trigger a different behavior (exposing /dev/freefall in this case).
> > > >
> > > > Provide a way for others to know who set the IRQ and so they can behave
> > > > accordingly.
> > > >
> > > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> > > > ---
> > > > drivers/i2c/i2c-core.c | 7 +++++++
> > > > include/linux/i2c.h | 11 +++++++++++
> > > > 2 files changed, 18 insertions(+)
> > > >
> > > > diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> > > > index cf9e396..226c75d 100644
> > > > --- a/drivers/i2c/i2c-core.c
> > > > +++ b/drivers/i2c/i2c-core.c
> > > > @@ -935,8 +935,12 @@ static int i2c_device_probe(struct device *dev)
> > > > irq = of_irq_get_byname(dev->of_node, "irq");
> > > > if (irq == -EINVAL || irq == -ENODATA)
> > > > irq = of_irq_get(dev->of_node, 0);
> > > > + if (irq > 0)
> > > > + client->irq_source = I2C_IRQ_SOURCE_OF;
> > > > } else if (ACPI_COMPANION(dev)) {
> > > > irq = acpi_dev_gpio_irq_get(ACPI_COMPANION(dev), 0);
> > > > + if (irq > 0)
> > > > + client->irq_source = I2C_IRQ_SOURCE_ACPI;
> > > > }
> > > > if (irq == -EPROBE_DEFER)
> > > > return irq;
> > > > @@ -947,6 +951,8 @@ static int i2c_device_probe(struct device *dev)
> > > > if (irq < 0) {
> > > > dev_dbg(dev, "Using Host Notify IRQ\n");
> > > > irq = i2c_smbus_host_notify_to_irq(client);
> > > > + if (irq > 0)
> > > > + client->irq_source = I2C_IRQ_SOURCE_HOST_NOTIFY;
> > > > }
> > > > if (irq < 0)
> > > > irq = 0;
> > > > @@ -1317,6 +1323,7 @@ i2c_new_device(struct i2c_adapter *adap, struct i2c_board_info const *info)
> > > > client->flags = info->flags;
> > > > client->addr = info->addr;
> > > > client->irq = info->irq;
> > > > + client->irq_source = I2C_IRQ_SOURCE_PLATFORM;
> > > >
> > > > strlcpy(client->name, info->type, sizeof(client->name));
> > > >
> > > > diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> > > > index b2109c5..7d0368d 100644
> > > > --- a/include/linux/i2c.h
> > > > +++ b/include/linux/i2c.h
> > > > @@ -213,6 +213,13 @@ struct i2c_driver {
> > > > };
> > > > #define to_i2c_driver(d) container_of(d, struct i2c_driver, driver)
> > > >
> > > > +enum i2c_irq_source {
> > > > + I2C_IRQ_SOURCE_PLATFORM,
> > > > + I2C_IRQ_SOURCE_OF,
> > > > + I2C_IRQ_SOURCE_ACPI,
> > > > + I2C_IRQ_SOURCE_HOST_NOTIFY,
> > > > +};
> > > > +
> > > > /**
> > > > * struct i2c_client - represent an I2C slave device
> > > > * @flags: I2C_CLIENT_TEN indicates the device uses a ten bit chip address;
> > > > @@ -227,6 +234,9 @@ struct i2c_driver {
> > > > * userspace_devices list
> > > > * @slave_cb: Callback when I2C slave mode of an adapter is used. The adapter
> > > > * calls it to pass on slave events to the slave driver.
> > > > + * @irq_source: Enum which provides the source of the IRQ. Useful to know
> > > > + * if the IRQ was issued from Host Notify or if it was provided by an other
> > > > + * component.
> > >
> > > I'd think some documentation somewhere makes sense why we need to
> > > distinguish this in some cases?
> >
> > I'd rather drivers be oblivious of the source of interrupt. If they need
> > to distinguish between them that means that our IRQ abstration failed.
> >
> > >
> > > > *
> > > > * An i2c_client identifies a single device (i.e. chip) connected to an
> > > > * i2c bus. The behaviour exposed to Linux is defined by the driver
> > > > @@ -245,6 +255,7 @@ struct i2c_client {
> > > > #if IS_ENABLED(CONFIG_I2C_SLAVE)
> > > > i2c_slave_cb_t slave_cb; /* callback for slave mode */
> > > > #endif
> > > > + enum i2c_irq_source irq_source; /* which component assigned the irq */
> > > > };
> > > > #define to_i2c_client(d) container_of(d, struct i2c_client, dev)
> > > >
> > > > Dmitry, Wolfram, Jean, would this be acceptable for you?
> > >
> > > Adding something to i2c_driver is not exactly cheap, but from what I
> > > glimpsed from this thread, this is one of the cleanest solution to this
> > > problem?
> > >
> >
> > As Benjamin said, it is really property of device [instance], not
> > driver. I.e. driver could handle both wired IRQ and HostNotify-based
> > scheme similarly, it is device (and board) that knows how stuff is
> > connected.
> >
> > Maybe we could do something like this (untested):
> >
> >
> > From e362a0277fd1bd6112f258664d8831d9bc6b78da Mon Sep 17 00:00:00 2001
> > From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > Date: Wed, 4 Jan 2017 09:33:43 -0800
> > Subject: [PATCH] i2c: do not enable fall back to Host Notify by default
> >
> > Falling back unconditionally to HostNotify as primary client's interrupt
> > breaks some drivers which alter their functionality depending on whether
> > interrupt is present or not, so let's introduce a board flag telling I2C
> > core explicitly if we want wired interrupt or HostNotify-based one:
> > I2C_CLIENT_HOST_NOTIFY.
> >
> > For DT-based systems we introduce "host-notofy" property that we convert
> > to I2C_CLIENT_HOST_NOTIFY board flag.
> >
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > ---
> > Documentation/devicetree/bindings/i2c/i2c.txt | 8 ++++++++
> > drivers/i2c/i2c-core.c | 17 ++++++++---------
> > include/linux/i2c.h | 1 +
> > 3 files changed, 17 insertions(+), 9 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt
> > index 5fa691e6f638..cee9d5055fa2 100644
> > --- a/Documentation/devicetree/bindings/i2c/i2c.txt
> > +++ b/Documentation/devicetree/bindings/i2c/i2c.txt
> > @@ -62,6 +62,9 @@ wants to support one of the below features, it should adapt the bindings below.
> > "irq" and "wakeup" names are recognized by I2C core, other names are
> > left to individual drivers.
> >
> > +- host-notify
> > + device uses SMBus host notify protocol instead of interrupt line.
> > +
> > - multi-master
> > states that there is another master active on this bus. The OS can use
> > this information to adapt power management to keep the arbitration awake
> > @@ -81,6 +84,11 @@ Binding may contain optional "interrupts" property, describing interrupts
> > used by the device. I2C core will assign "irq" interrupt (or the very first
> > interrupt if not using interrupt names) as primary interrupt for the slave.
> >
> > +Alternatively, devices supporting SMbus Host Notify, and connected to
> > +adapters that support this feature, may use "host-notify" property. I2C
> > +core will create a virtual interrupt for Host Notify and assign it as
> > +primary interrupt for the slave.
> > +
> > Also, if device is marked as a wakeup source, I2C core will set up "wakeup"
> > interrupt for the device. If "wakeup" interrupt name is not present in the
> > binding, then primary interrupt will be used as wakeup interrupt.
> > diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> > index cf9e396d7702..250969fa7670 100644
> > --- a/drivers/i2c/i2c-core.c
> > +++ b/drivers/i2c/i2c-core.c
> > @@ -931,7 +931,10 @@ static int i2c_device_probe(struct device *dev)
> > if (!client->irq) {
> > int irq = -ENOENT;
> >
> > - if (dev->of_node) {
> > + if (client->flags & I2C_CLIENT_HOST_HOTIFY) {
> > + dev_dbg(dev, "Using Host Notify IRQ\n");
> > + irq = i2c_smbus_host_notify_to_irq(client);
> > + } else if (dev->of_node) {
> > irq = of_irq_get_byname(dev->of_node, "irq");
> > if (irq == -EINVAL || irq == -ENODATA)
> > irq = of_irq_get(dev->of_node, 0);
> > @@ -940,14 +943,7 @@ static int i2c_device_probe(struct device *dev)
> > }
> > if (irq == -EPROBE_DEFER)
> > return irq;
> > - /*
> > - * ACPI and OF did not find any useful IRQ, try to see
> > - * if Host Notify can be used.
> > - */
> > - if (irq < 0) {
> > - dev_dbg(dev, "Using Host Notify IRQ\n");
> > - irq = i2c_smbus_host_notify_to_irq(client);
> > - }
> > +
> > if (irq < 0)
> > irq = 0;
> >
> > @@ -1716,6 +1712,9 @@ static struct i2c_client *of_i2c_register_device(struct i2c_adapter *adap,
> > info.of_node = of_node_get(node);
> > info.archdata = &dev_ad;
> >
> > + if (of_read_property_bool(node, "host-notify"))
> > + info.flags |= I2C_CLIENT_HOST_NOTIFY;
> > +
> > if (of_get_property(node, "wakeup-source", NULL))
> > info.flags |= I2C_CLIENT_WAKE;
> >
> > diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> > index b2109c522dec..4b45ec46161f 100644
> > --- a/include/linux/i2c.h
> > +++ b/include/linux/i2c.h
> > @@ -665,6 +665,7 @@ i2c_unlock_adapter(struct i2c_adapter *adapter)
> > #define I2C_CLIENT_TEN 0x10 /* we have a ten bit chip address */
> > /* Must equal I2C_M_TEN below */
> > #define I2C_CLIENT_SLAVE 0x20 /* we are the slave */
> > +#define I2C_CLIENT_HOST_NOTIFY 0x40 /* We want to use I2C host notify */
> > #define I2C_CLIENT_WAKE 0x80 /* for board_info; true iff can wake */
> > #define I2C_CLIENT_SCCB 0x9000 /* Use Omnivision SCCB protocol */
> > /* Must match I2C_M_STOP|IGNORE_NAK */
> > --
> > 2.11.0.390.gc69c2f50cf-goog
> >
> >
>
> Looks good, this seems to be elegant solution to our problem.
>
> But then it is needed to patch those touchpad drivers to add that
> I2C_CLIENT_HOST_NOTIFY flag, right?
>
Yes, but currently the 2 drivers that are using Host Notify upstream are
rmi_smbus and elan_i2c. Both don't have an automatic binding (yet), so
there is nothing to worry about for now.
Cheers,
Benjamin
^ permalink raw reply
* [PATCH v8 5/5] ARM: configs: stm32: Add I2C support for STM32 defconfig
From: M'boumba Cedric Madianga @ 2017-01-05 9:07 UTC (permalink / raw)
To: wsa, robh+dt, mcoquelin.stm32, alexandre.torgue, linus.walleij,
patrice.chotard, linux, linux-i2c, devicetree, linux-arm-kernel,
linux-kernel, u.kleine-koenig
Cc: M'boumba Cedric Madianga
In-Reply-To: <1483607246-14771-1-git-send-email-cedric.madianga@gmail.com>
This patch adds I2C support for STM32 default configuration
Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
---
arch/arm/configs/stm32_defconfig | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/arm/configs/stm32_defconfig b/arch/arm/configs/stm32_defconfig
index 5a72d69..323d2a3 100644
--- a/arch/arm/configs/stm32_defconfig
+++ b/arch/arm/configs/stm32_defconfig
@@ -47,6 +47,9 @@ CONFIG_SERIAL_NONSTANDARD=y
CONFIG_SERIAL_STM32=y
CONFIG_SERIAL_STM32_CONSOLE=y
# CONFIG_HW_RANDOM is not set
+CONFIG_I2C=y
+CONFIG_I2C_CHARDEV=y
+CONFIG_I2C_STM32F4=y
# CONFIG_HWMON is not set
# CONFIG_USB_SUPPORT is not set
CONFIG_NEW_LEDS=y
--
1.9.1
^ permalink raw reply related
* [PATCH v8 4/5] ARM: dts: stm32: Add I2C1 support for STM32429 eval board
From: M'boumba Cedric Madianga @ 2017-01-05 9:07 UTC (permalink / raw)
To: wsa, robh+dt, mcoquelin.stm32, alexandre.torgue, linus.walleij,
patrice.chotard, linux, linux-i2c, devicetree, linux-arm-kernel,
linux-kernel, u.kleine-koenig
Cc: M'boumba Cedric Madianga
In-Reply-To: <1483607246-14771-1-git-send-email-cedric.madianga@gmail.com>
This patch adds I2C1 instance support for STM32x9I-Eval board.
Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
---
arch/arm/boot/dts/stm32429i-eval.dts | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arch/arm/boot/dts/stm32429i-eval.dts b/arch/arm/boot/dts/stm32429i-eval.dts
index 76f7206..c943539 100644
--- a/arch/arm/boot/dts/stm32429i-eval.dts
+++ b/arch/arm/boot/dts/stm32429i-eval.dts
@@ -146,3 +146,9 @@
pinctrl-names = "default";
status = "okay";
};
+
+&i2c1 {
+ pinctrl-0 = <&i2c1_pins_b>;
+ pinctrl-names = "default";
+ status = "okay";
+};
--
1.9.1
^ permalink raw reply related
* [PATCH v8 2/5] i2c: Add STM32F4 I2C driver
From: M'boumba Cedric Madianga @ 2017-01-05 9:07 UTC (permalink / raw)
To: wsa, robh+dt, mcoquelin.stm32, alexandre.torgue, linus.walleij,
patrice.chotard, linux, linux-i2c, devicetree, linux-arm-kernel,
linux-kernel, u.kleine-koenig
Cc: M'boumba Cedric Madianga
In-Reply-To: <1483607246-14771-1-git-send-email-cedric.madianga@gmail.com>
This patch adds support for the STM32F4 I2C controller.
Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
---
drivers/i2c/busses/Kconfig | 10 +
drivers/i2c/busses/Makefile | 1 +
drivers/i2c/busses/i2c-stm32f4.c | 866 +++++++++++++++++++++++++++++++++++++++
3 files changed, 877 insertions(+)
create mode 100644 drivers/i2c/busses/i2c-stm32f4.c
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 0cdc844..2719208 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -886,6 +886,16 @@ config I2C_ST
This driver can also be built as module. If so, the module
will be called i2c-st.
+config I2C_STM32F4
+ tristate "STMicroelectronics STM32F4 I2C support"
+ depends on ARCH_STM32 || COMPILE_TEST
+ help
+ Enable this option to add support for STM32 I2C controller embedded
+ in STM32F4 SoCs.
+
+ This driver can also be built as module. If so, the module
+ will be called i2c-stm32f4.
+
config I2C_STU300
tristate "ST Microelectronics DDC I2C interface"
depends on MACH_U300
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 1c1bac8..a2c6ff5 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -85,6 +85,7 @@ obj-$(CONFIG_I2C_SH_MOBILE) += i2c-sh_mobile.o
obj-$(CONFIG_I2C_SIMTEC) += i2c-simtec.o
obj-$(CONFIG_I2C_SIRF) += i2c-sirf.o
obj-$(CONFIG_I2C_ST) += i2c-st.o
+obj-$(CONFIG_I2C_STM32F4) += i2c-stm32f4.o
obj-$(CONFIG_I2C_STU300) += i2c-stu300.o
obj-$(CONFIG_I2C_SUN6I_P2WI) += i2c-sun6i-p2wi.o
obj-$(CONFIG_I2C_TEGRA) += i2c-tegra.o
diff --git a/drivers/i2c/busses/i2c-stm32f4.c b/drivers/i2c/busses/i2c-stm32f4.c
new file mode 100644
index 0000000..23e5757
--- /dev/null
+++ b/drivers/i2c/busses/i2c-stm32f4.c
@@ -0,0 +1,866 @@
+/*
+ * Driver for STMicroelectronics STM32 I2C controller
+ *
+ * This I2C controller is described in the STM32F429/439 Soc reference manual.
+ * Please see below a link to the documentation:
+ * http://www.st.com/resource/en/reference_manual/DM00031020.pdf
+ *
+ * Copyright (C) M'boumba Cedric Madianga 2016
+ * Author: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
+ *
+ * This driver is based on i2c-st.c
+ *
+ * License terms: GNU General Public License (GPL), version 2
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/reset.h>
+
+/* STM32F4 I2C offset registers */
+#define STM32F4_I2C_CR1 0x00
+#define STM32F4_I2C_CR2 0x04
+#define STM32F4_I2C_DR 0x10
+#define STM32F4_I2C_SR1 0x14
+#define STM32F4_I2C_SR2 0x18
+#define STM32F4_I2C_CCR 0x1C
+#define STM32F4_I2C_TRISE 0x20
+#define STM32F4_I2C_FLTR 0x24
+
+/* STM32F4 I2C control 1*/
+#define STM32F4_I2C_CR1_POS BIT(11)
+#define STM32F4_I2C_CR1_ACK BIT(10)
+#define STM32F4_I2C_CR1_STOP BIT(9)
+#define STM32F4_I2C_CR1_START BIT(8)
+#define STM32F4_I2C_CR1_PE BIT(0)
+
+/* STM32F4 I2C control 2 */
+#define STM32F4_I2C_CR2_FREQ_MASK GENMASK(5, 0)
+#define STM32F4_I2C_CR2_FREQ(n) ((n) & STM32F4_I2C_CR2_FREQ_MASK)
+#define STM32F4_I2C_CR2_ITBUFEN BIT(10)
+#define STM32F4_I2C_CR2_ITEVTEN BIT(9)
+#define STM32F4_I2C_CR2_ITERREN BIT(8)
+#define STM32F4_I2C_CR2_IRQ_MASK (STM32F4_I2C_CR2_ITBUFEN | \
+ STM32F4_I2C_CR2_ITEVTEN | \
+ STM32F4_I2C_CR2_ITERREN)
+
+/* STM32F4 I2C Status 1 */
+#define STM32F4_I2C_SR1_AF BIT(10)
+#define STM32F4_I2C_SR1_ARLO BIT(9)
+#define STM32F4_I2C_SR1_BERR BIT(8)
+#define STM32F4_I2C_SR1_TXE BIT(7)
+#define STM32F4_I2C_SR1_RXNE BIT(6)
+#define STM32F4_I2C_SR1_BTF BIT(2)
+#define STM32F4_I2C_SR1_ADDR BIT(1)
+#define STM32F4_I2C_SR1_SB BIT(0)
+#define STM32F4_I2C_SR1_ITEVTEN_MASK (STM32F4_I2C_SR1_BTF | \
+ STM32F4_I2C_SR1_ADDR | \
+ STM32F4_I2C_SR1_SB)
+#define STM32F4_I2C_SR1_ITBUFEN_MASK (STM32F4_I2C_SR1_TXE | \
+ STM32F4_I2C_SR1_RXNE)
+#define STM32F4_I2C_SR1_ITERREN_MASK (STM32F4_I2C_SR1_AF | \
+ STM32F4_I2C_SR1_ARLO | \
+ STM32F4_I2C_SR1_BERR)
+
+/* STM32F4 I2C Status 2 */
+#define STM32F4_I2C_SR2_BUSY BIT(1)
+
+/* STM32F4 I2C Control Clock */
+#define STM32F4_I2C_CCR_CCR_MASK GENMASK(11, 0)
+#define STM32F4_I2C_CCR_CCR(n) ((n) & STM32F4_I2C_CCR_CCR_MASK)
+#define STM32F4_I2C_CCR_FS BIT(15)
+#define STM32F4_I2C_CCR_DUTY BIT(14)
+
+/* STM32F4 I2C Trise */
+#define STM32F4_I2C_TRISE_VALUE_MASK GENMASK(5, 0)
+#define STM32F4_I2C_TRISE_VALUE(n) ((n) & STM32F4_I2C_TRISE_VALUE_MASK)
+
+/* STM32F4 I2C Filter */
+#define STM32F4_I2C_FLTR_DNF_MASK GENMASK(3, 0)
+#define STM32F4_I2C_FLTR_DNF(n) ((n) & STM32F4_I2C_FLTR_DNF_MASK)
+#define STM32F4_I2C_FLTR_ANOFF BIT(4)
+
+#define STM32F4_I2C_MIN_FREQ 2U
+#define STM32F4_I2C_MAX_FREQ 46U
+#define HZ_TO_MHZ 1000000
+
+enum stm32f4_i2c_speed {
+ STM32F4_I2C_SPEED_STANDARD, /* 100 kHz */
+ STM32F4_I2C_SPEED_FAST, /* 400 kHz */
+ STM32F4_I2C_SPEED_END,
+};
+
+/**
+ * struct stm32f4_i2c_timings - per-Mode tuning parameters
+ * @duty: Fast mode duty cycle
+ * @scl_period: SCL period in microsecond to reach 100kHz/400kHz SCL frequency
+ */
+struct stm32f4_i2c_timings {
+ u32 duty;
+ u32 scl_period;
+};
+
+/**
+ * struct stm32f4_i2c_msg - client specific data
+ * @addr: 8-bit slave addr, including r/w bit
+ * @count: number of bytes to be transferred
+ * @buf: data buffer
+ * @result: result of the transfer
+ * @stop: last I2C msg to be sent, i.e. STOP to be generated
+ */
+struct stm32f4_i2c_msg {
+ u8 addr;
+ u32 count;
+ u8 *buf;
+ int result;
+ bool stop;
+};
+
+/**
+ * struct stm32f4_i2c_dev - private data of the controller
+ * @adap: I2C adapter for this controller
+ * @dev: device for this controller
+ * @base: virtual memory area
+ * @complete: completion of I2C message
+ * @clk: hw i2c clock
+ * speed: I2C clock frequency of the controller. Standard or Fast only supported
+ * @msg: I2C transfer information
+ */
+struct stm32f4_i2c_dev {
+ struct i2c_adapter adap;
+ struct device *dev;
+ void __iomem *base;
+ struct completion complete;
+ struct clk *clk;
+ int speed;
+ struct stm32f4_i2c_msg msg;
+};
+
+/*
+ * In standard mode:
+ * SCL period = SCL high period = SCL low period = CCR * I2C parent clk period
+ *
+ * In fast mode:
+ * If Duty = 0; SCL high period = 1 * CCR * I2C parent clk period
+ * SCL low period = 2 * CCR * I2C parent clk period
+ * If Duty = 1; SCL high period = 9 * CCR * I2C parent clk period
+ * SCL low period = 16 * CCR * I2C parent clk period
+ * In order to reach 400 kHz with lower I2C parent clk frequencies we always set
+ * Duty = 1
+ *
+ * For both modes, we have CCR = SCL period * I2C parent clk frequency
+ * with scl_period = 5 microseconds in Standard mode and scl_period = 1
+ * microsecond in Fast Mode in order to satisfy scl_high and scl_low periods
+ * constraints defined by i2c bus specification
+ */
+static struct stm32f4_i2c_timings i2c_timings[] = {
+ [STM32F4_I2C_SPEED_STANDARD] = {
+ .duty = 0,
+ .scl_period = 5,
+ },
+ [STM32F4_I2C_SPEED_FAST] = {
+ .duty = 1,
+ .scl_period = 1,
+ },
+};
+
+static inline void stm32f4_i2c_set_bits(void __iomem *reg, u32 mask)
+{
+ writel_relaxed(readl_relaxed(reg) | mask, reg);
+}
+
+static inline void stm32f4_i2c_clr_bits(void __iomem *reg, u32 mask)
+{
+ writel_relaxed(readl_relaxed(reg) & ~mask, reg);
+}
+
+static void stm32f4_i2c_disable_irq(struct stm32f4_i2c_dev *i2c_dev)
+{
+ void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR2;
+
+ stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR2_IRQ_MASK);
+}
+
+static int stm32f4_i2c_set_periph_clk_freq(struct stm32f4_i2c_dev *i2c_dev)
+{
+ u32 clk_rate, cr2, freq;
+
+ /*
+ * The minimum allowed frequency is 2MHz, the maximum APB frequency is
+ * limited by an intrinsic value of 46 MHz
+ */
+ clk_rate = clk_get_rate(i2c_dev->clk);
+ freq = DIV_ROUND_UP(clk_rate, HZ_TO_MHZ);
+ if (freq < STM32F4_I2C_MIN_FREQ || freq > STM32F4_I2C_MAX_FREQ) {
+ dev_err(i2c_dev->dev, "out of scope parent clk freq at %dMHz\n",
+ freq);
+ return -EINVAL;
+ }
+
+ cr2 = readl_relaxed(i2c_dev->base + STM32F4_I2C_CR2);
+ cr2 &= ~STM32F4_I2C_CR2_FREQ_MASK;
+ cr2 |= STM32F4_I2C_CR2_FREQ(freq);
+ writel_relaxed(cr2, i2c_dev->base + STM32F4_I2C_CR2);
+
+ return 0;
+}
+
+static void stm32f4_i2c_set_rise_time(struct stm32f4_i2c_dev *i2c_dev)
+{
+ u32 trise, freq, cr2;
+
+ /*
+ * These bits must be programmed with the maximum SCL rise time given in
+ * the I2C bus specification, incremented by 1.
+ *
+ * In standard mode, the maximum allowed SCL rise time is 1000 ns.
+ * If, in the I2C_CR2 register, the value of FREQ[5:0] bits is equal to
+ * 0x08 so period = 125 ns therefore the TRISE[5:0] bits must be
+ * programmed with 09h.(1000 ns / 125 ns = 8 + 1)
+ * So, for I2C standard mode TRISE = FREQ[5:0] + 1
+ *
+ * In fast mode, the maximum allowed SCL rise time is 300 ns.
+ * If, in the I2C_CR2 register, the value of FREQ[5:0] bits is equal to
+ * 0x08 so period = 125 ns therefore the TRISE[5:0] bits must be
+ * programmed with 03h.(300 ns / 125 ns = 2 + 1)
+ * So, for I2C fast mode TRISE = FREQ[5:0] * 300 / 1000 + 1
+ */
+
+ cr2 = readl_relaxed(i2c_dev->base + STM32F4_I2C_CR2);
+ freq = cr2 & STM32F4_I2C_CR2_FREQ_MASK;
+
+ if (i2c_dev->speed == STM32F4_I2C_SPEED_STANDARD)
+ trise = freq + 1;
+ else
+ trise = freq * 300 / 1000 + 1;
+
+ writel_relaxed(STM32F4_I2C_TRISE_VALUE(trise),
+ i2c_dev->base + STM32F4_I2C_TRISE);
+}
+
+static void stm32f4_i2c_set_speed_mode(struct stm32f4_i2c_dev *i2c_dev)
+{
+ struct stm32f4_i2c_timings *t = &i2c_timings[i2c_dev->speed];
+ u32 cr2, ccr, freq, val;
+
+ ccr = readl_relaxed(i2c_dev->base + STM32F4_I2C_CCR);
+ ccr &= ~(STM32F4_I2C_CCR_FS | STM32F4_I2C_CCR_DUTY |
+ STM32F4_I2C_CCR_CCR_MASK);
+
+ /*
+ * Please see the comments above regarding i2c_timings[] declaration
+ * to understand the below calculation
+ */
+ cr2 = readl_relaxed(i2c_dev->base + STM32F4_I2C_CR2);
+ freq = cr2 & STM32F4_I2C_CR2_FREQ_MASK;
+ val = freq * t->scl_period;
+ ccr |= STM32F4_I2C_CCR_CCR(val);
+
+ if (t->duty)
+ ccr |= STM32F4_I2C_CCR_FS | STM32F4_I2C_CCR_DUTY;
+
+ writel_relaxed(ccr, i2c_dev->base + STM32F4_I2C_CCR);
+}
+
+static void stm32f4_i2c_set_filter(struct stm32f4_i2c_dev *i2c_dev)
+{
+ u32 filter;
+
+ /* Enable analog noise filter and disable digital noise filter */
+ filter = readl_relaxed(i2c_dev->base + STM32F4_I2C_FLTR);
+ filter &= ~(STM32F4_I2C_FLTR_ANOFF | STM32F4_I2C_FLTR_DNF_MASK);
+ writel_relaxed(filter, i2c_dev->base + STM32F4_I2C_FLTR);
+}
+
+/**
+ * stm32f4_i2c_hw_config() - Prepare I2C block
+ * @i2c_dev: Controller's private data
+ */
+static int stm32f4_i2c_hw_config(struct stm32f4_i2c_dev *i2c_dev)
+{
+ void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR1;
+ int ret = 0;
+
+ /* Disable I2C */
+ stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_PE);
+
+ ret = stm32f4_i2c_set_periph_clk_freq(i2c_dev);
+ if (ret)
+ return ret;
+
+ stm32f4_i2c_set_rise_time(i2c_dev);
+
+ stm32f4_i2c_set_speed_mode(i2c_dev);
+
+ stm32f4_i2c_set_filter(i2c_dev);
+
+ /* Enable I2C */
+ stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_PE);
+
+ return ret;
+}
+
+static int stm32f4_i2c_wait_free_bus(struct stm32f4_i2c_dev *i2c_dev)
+{
+ u32 status;
+ int ret;
+
+ ret = readl_relaxed_poll_timeout(i2c_dev->base + STM32F4_I2C_SR2,
+ status,
+ !(status & STM32F4_I2C_SR2_BUSY),
+ 10, 1000);
+ if (ret) {
+ dev_dbg(i2c_dev->dev, "bus not free\n");
+ ret = -EBUSY;
+ }
+
+ return ret;
+}
+
+/**
+ * stm32f4_i2c_write_ byte() - Write a byte in the data register
+ * @i2c_dev: Controller's private data
+ * @byte: Data to write in the register
+ */
+static void stm32f4_i2c_write_byte(struct stm32f4_i2c_dev *i2c_dev, u8 byte)
+{
+ writel_relaxed(byte, i2c_dev->base + STM32F4_I2C_DR);
+}
+
+/**
+ * stm32f4_i2c_write_msg() - Fill the data register in write mode
+ * @i2c_dev: Controller's private data
+ *
+ * This function fills the data register with I2C transfer buffer
+ */
+static void stm32f4_i2c_write_msg(struct stm32f4_i2c_dev *i2c_dev)
+{
+ struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
+
+ stm32f4_i2c_write_byte(i2c_dev, *msg->buf++);
+ msg->count--;
+}
+
+static void stm32f4_i2c_read_msg(struct stm32f4_i2c_dev *i2c_dev)
+{
+ struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
+ u32 rbuf;
+
+ rbuf = readl_relaxed(i2c_dev->base + STM32F4_I2C_DR);
+ *msg->buf++ = rbuf & 0xff;
+ msg->count--;
+}
+
+static void stm32f4_i2c_terminate_xfer(struct stm32f4_i2c_dev *i2c_dev)
+{
+ struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
+ void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR2;
+
+ stm32f4_i2c_disable_irq(i2c_dev);
+
+ reg = i2c_dev->base + STM32F4_I2C_CR1;
+ if (msg->stop)
+ stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_STOP);
+ else
+ stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_START);
+
+ complete(&i2c_dev->complete);
+}
+
+/**
+ * stm32f4_i2c_handle_write() - Handle FIFO empty interrupt in case of write
+ * @i2c_dev: Controller's private data
+ */
+static void stm32f4_i2c_handle_write(struct stm32f4_i2c_dev *i2c_dev)
+{
+ struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
+ void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR2;
+
+ if (msg->count) {
+ stm32f4_i2c_write_msg(i2c_dev);
+ if (!msg->count) {
+ /* Disable buffer interrupts for RXNE/TXE events */
+ stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR2_ITBUFEN);
+ }
+ } else {
+ stm32f4_i2c_terminate_xfer(i2c_dev);
+ }
+}
+
+/**
+ * stm32f4_i2c_handle_read() - Handle FIFO empty interrupt in case of read
+ * @i2c_dev: Controller's private data
+ */
+static void stm32f4_i2c_handle_read(struct stm32f4_i2c_dev *i2c_dev)
+{
+ struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
+ void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR2;
+
+ switch (msg->count) {
+ case 1:
+ stm32f4_i2c_disable_irq(i2c_dev);
+ stm32f4_i2c_read_msg(i2c_dev);
+ complete(&i2c_dev->complete);
+ break;
+ /*
+ * For 2 or 3-byte reception, we do not have to read the data register
+ * when RXNE occurs as we have to wait for byte transferred finished
+ * event before reading data. So, here we just disable buffer
+ * interrupt in order to avoid another system preemption due to RXNE
+ * event
+ */
+ case 2:
+ case 3:
+ stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR2_ITBUFEN);
+ break;
+ /* For N byte reception with N > 3 we directly read data register */
+ default:
+ stm32f4_i2c_read_msg(i2c_dev);
+ }
+}
+
+/**
+ * stm32f4_i2c_handle_rx_btf() - Handle byte transfer finished interrupt
+ * in case of read
+ * @i2c_dev: Controller's private data
+ */
+static void stm32f4_i2c_handle_rx_btf(struct stm32f4_i2c_dev *i2c_dev)
+{
+ struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
+ void __iomem *reg;
+ u32 mask;
+ int i;
+
+ switch (msg->count) {
+ case 2:
+ /*
+ * In order to correctly send the Stop or Repeated Start
+ * condition on the I2C bus, the STOP/START bit has to be set
+ * before reading the last two bytes.
+ * After that, we could read the last two bytes, disable
+ * remaining interrupts and notify the end of xfer to the
+ * client
+ */
+ reg = i2c_dev->base + STM32F4_I2C_CR1;
+ if (msg->stop)
+ stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_STOP);
+ else
+ stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_START);
+
+ for (i = 2; i > 0; i--)
+ stm32f4_i2c_read_msg(i2c_dev);
+
+ reg = i2c_dev->base + STM32F4_I2C_CR2;
+ mask = STM32F4_I2C_CR2_ITEVTEN | STM32F4_I2C_CR2_ITERREN;
+ stm32f4_i2c_clr_bits(reg, mask);
+
+ complete(&i2c_dev->complete);
+ break;
+ case 3:
+ /*
+ * In order to correctly send the ACK on the I2C bus for the
+ * last two bytes, we have to set ACK bit before reading the
+ * third last data byte
+ */
+ reg = i2c_dev->base + STM32F4_I2C_CR1;
+ stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_ACK);
+ stm32f4_i2c_read_msg(i2c_dev);
+ break;
+ default:
+ stm32f4_i2c_read_msg(i2c_dev);
+ }
+}
+
+/**
+ * stm32f4_i2c_handle_rx_addr() - Handle address matched interrupt in case of
+ * master receiver
+ * @i2c_dev: Controller's private data
+ */
+static void stm32f4_i2c_handle_rx_addr(struct stm32f4_i2c_dev *i2c_dev)
+{
+ struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
+ void __iomem *reg;
+
+ switch (msg->count) {
+ case 0:
+ stm32f4_i2c_terminate_xfer(i2c_dev);
+ /* Clear ADDR flag */
+ readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
+ break;
+ case 1:
+ /*
+ * Single byte reception:
+ * Enable NACK, clear ADDR flag and generate STOP or RepSTART
+ */
+ reg = i2c_dev->base + STM32F4_I2C_CR1;
+ stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_ACK);
+ readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
+ if (msg->stop)
+ stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_STOP);
+ else
+ stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_START);
+ break;
+ case 2:
+ /*
+ * 2-byte reception:
+ * Enable NACK and set POS
+ */
+ reg = i2c_dev->base + STM32F4_I2C_CR1;
+ stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_ACK);
+ stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_POS);
+ readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
+ break;
+
+ default:
+ /* N-byte reception: Enable ACK */
+ reg = i2c_dev->base + STM32F4_I2C_CR1;
+ stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_ACK);
+ readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
+ break;
+ }
+}
+
+/**
+ * stm32f4_i2c_isr_event() - Interrupt routine for I2C bus event
+ * @irq: interrupt number
+ * @data: Controller's private data
+ */
+static irqreturn_t stm32f4_i2c_isr_event(int irq, void *data)
+{
+ struct stm32f4_i2c_dev *i2c_dev = data;
+ struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
+ u32 possible_status = STM32F4_I2C_SR1_ITEVTEN_MASK;
+ void __iomem *reg;
+ u32 status, ien, event;
+
+ ien = readl_relaxed(i2c_dev->base + STM32F4_I2C_CR2);
+ ien &= STM32F4_I2C_CR2_IRQ_MASK;
+
+ /* Update possible_status if buffer interrupt is enabled */
+ if (ien & STM32F4_I2C_CR2_ITBUFEN)
+ possible_status |= STM32F4_I2C_SR1_ITBUFEN_MASK;
+
+ status = readl_relaxed(i2c_dev->base + STM32F4_I2C_SR1);
+ event = status & possible_status;
+ if (!event) {
+ dev_dbg(i2c_dev->dev,
+ "spurious evt irq (status=0x%08x, ien=0x%08x)\n",
+ status, ien);
+ return IRQ_NONE;
+ }
+
+ if (event & STM32F4_I2C_SR1_SB)
+ stm32f4_i2c_write_byte(i2c_dev, msg->addr);
+
+ if (event & STM32F4_I2C_SR1_ADDR) {
+ if (msg->addr & I2C_M_RD)
+ stm32f4_i2c_handle_rx_addr(i2c_dev);
+ else
+ readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
+
+ /* Enable buffer interrupts for RXNE/TXE events */
+ reg = i2c_dev->base + STM32F4_I2C_CR2;
+ stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR2_ITBUFEN);
+ }
+
+ if ((event & STM32F4_I2C_SR1_TXE) && !(msg->addr & I2C_M_RD))
+ stm32f4_i2c_handle_write(i2c_dev);
+
+ if ((event & STM32F4_I2C_SR1_RXNE) && (msg->addr & I2C_M_RD))
+ stm32f4_i2c_handle_read(i2c_dev);
+
+ if (event & STM32F4_I2C_SR1_BTF) {
+ if (msg->addr & I2C_M_RD)
+ stm32f4_i2c_handle_rx_btf(i2c_dev);
+ else
+ stm32f4_i2c_handle_write(i2c_dev);
+ }
+
+ return IRQ_HANDLED;
+}
+
+/**
+ * stm32f4_i2c_isr_error() - Interrupt routine for I2C bus error
+ * @irq: interrupt number
+ * @data: Controller's private data
+ */
+static irqreturn_t stm32f4_i2c_isr_error(int irq, void *data)
+{
+ struct stm32f4_i2c_dev *i2c_dev = data;
+ struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
+ void __iomem *reg;
+ u32 status;
+
+ status = readl_relaxed(i2c_dev->base + STM32F4_I2C_SR1);
+
+ /* Arbitration lost */
+ if (status & STM32F4_I2C_SR1_ARLO) {
+ reg = i2c_dev->base + STM32F4_I2C_SR1;
+ stm32f4_i2c_clr_bits(reg, STM32F4_I2C_SR1_ARLO);
+ msg->result = -EAGAIN;
+ }
+
+ /*
+ * Acknowledge failure:
+ * In master transmitter mode a Stop must be generated by software
+ */
+ if (status & STM32F4_I2C_SR1_AF) {
+ if (!(msg->addr & I2C_M_RD)) {
+ reg = i2c_dev->base + STM32F4_I2C_CR1;
+ stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_STOP);
+ }
+ reg = i2c_dev->base + STM32F4_I2C_SR1;
+ stm32f4_i2c_clr_bits(reg, STM32F4_I2C_SR1_AF);
+ msg->result = -EIO;
+ }
+
+ /* Bus error */
+ if (status & STM32F4_I2C_SR1_BERR) {
+ reg = i2c_dev->base + STM32F4_I2C_SR1;
+ stm32f4_i2c_clr_bits(reg, STM32F4_I2C_SR1_BERR);
+ msg->result = -EIO;
+ }
+
+ stm32f4_i2c_disable_irq(i2c_dev);
+ complete(&i2c_dev->complete);
+
+ return IRQ_HANDLED;
+}
+
+/**
+ * stm32f4_i2c_xfer_msg() - Transfer a single I2C message
+ * @i2c_dev: Controller's private data
+ * @msg: I2C message to transfer
+ * @is_first: first message of the sequence
+ * @is_last: last message of the sequence
+ */
+static int stm32f4_i2c_xfer_msg(struct stm32f4_i2c_dev *i2c_dev,
+ struct i2c_msg *msg, bool is_first,
+ bool is_last)
+{
+ struct stm32f4_i2c_msg *f4_msg = &i2c_dev->msg;
+ void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR1;
+ unsigned long timeout;
+ u32 mask;
+ int ret;
+
+ f4_msg->addr = i2c_8bit_addr_from_msg(msg);
+ f4_msg->buf = msg->buf;
+ f4_msg->count = msg->len;
+ f4_msg->result = 0;
+ f4_msg->stop = is_last;
+
+ reinit_completion(&i2c_dev->complete);
+
+ /* Enable events and errors interrupts */
+ mask = STM32F4_I2C_CR2_ITEVTEN | STM32F4_I2C_CR2_ITERREN;
+ stm32f4_i2c_set_bits(i2c_dev->base + STM32F4_I2C_CR2, mask);
+
+ if (is_first) {
+ ret = stm32f4_i2c_wait_free_bus(i2c_dev);
+ if (ret)
+ return ret;
+
+ /* START generation */
+ stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_START);
+ }
+
+ timeout = wait_for_completion_timeout(&i2c_dev->complete,
+ i2c_dev->adap.timeout);
+ ret = f4_msg->result;
+
+ /* Disable POS position Ack */
+ stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_POS);
+
+ if (!timeout)
+ ret = -ETIMEDOUT;
+
+ return ret;
+}
+
+/**
+ * stm32f4_i2c_xfer() - Transfer combined I2C message
+ * @i2c_adap: Adapter pointer to the controller
+ * @msgs: Pointer to data to be written.
+ * @num: Number of messages to be executed
+ */
+static int stm32f4_i2c_xfer(struct i2c_adapter *i2c_adap, struct i2c_msg msgs[],
+ int num)
+{
+ struct stm32f4_i2c_dev *i2c_dev = i2c_get_adapdata(i2c_adap);
+ int ret, i;
+
+ ret = clk_enable(i2c_dev->clk);
+ if (ret) {
+ dev_err(i2c_dev->dev, "Failed to enable clock\n");
+ return ret;
+ }
+
+ for (i = 0; i < num && !ret; i++)
+ ret = stm32f4_i2c_xfer_msg(i2c_dev, &msgs[i], i == 0,
+ i == num - 1);
+
+ clk_disable(i2c_dev->clk);
+
+ return (ret < 0) ? ret : num;
+}
+
+static u32 stm32f4_i2c_func(struct i2c_adapter *adap)
+{
+ return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
+}
+
+static struct i2c_algorithm stm32f4_i2c_algo = {
+ .master_xfer = stm32f4_i2c_xfer,
+ .functionality = stm32f4_i2c_func,
+};
+
+static int stm32f4_i2c_probe(struct platform_device *pdev)
+{
+ struct device_node *np = pdev->dev.of_node;
+ struct stm32f4_i2c_dev *i2c_dev;
+ struct resource *res;
+ u32 irq_event, irq_error, clk_rate;
+ struct i2c_adapter *adap;
+ struct reset_control *rst;
+ int ret;
+
+ i2c_dev = devm_kzalloc(&pdev->dev, sizeof(*i2c_dev), GFP_KERNEL);
+ if (!i2c_dev)
+ return -ENOMEM;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ i2c_dev->base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(i2c_dev->base))
+ return PTR_ERR(i2c_dev->base);
+
+ irq_event = irq_of_parse_and_map(np, 0);
+ if (!irq_event) {
+ dev_err(&pdev->dev, "IRQ event missing or invalid\n");
+ return -EINVAL;
+ }
+
+ irq_error = irq_of_parse_and_map(np, 1);
+ if (!irq_error) {
+ dev_err(&pdev->dev, "IRQ error missing or invalid\n");
+ return -EINVAL;
+ }
+
+ i2c_dev->clk = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(i2c_dev->clk)) {
+ dev_err(&pdev->dev, "Error: Missing controller clock\n");
+ return PTR_ERR(i2c_dev->clk);
+ }
+ ret = clk_prepare_enable(i2c_dev->clk);
+ if (ret) {
+ dev_err(i2c_dev->dev, "Failed to prepare_enable clock\n");
+ return ret;
+ }
+
+ rst = devm_reset_control_get(&pdev->dev, NULL);
+ if (IS_ERR(rst)) {
+ dev_err(&pdev->dev, "Error: Missing controller reset\n");
+ ret = PTR_ERR(rst);
+ goto clk_free;
+ }
+ reset_control_assert(rst);
+ udelay(2);
+ reset_control_deassert(rst);
+
+ i2c_dev->speed = STM32F4_I2C_SPEED_STANDARD;
+ ret = of_property_read_u32(np, "clock-frequency", &clk_rate);
+ if (!ret && clk_rate >= 40000)
+ i2c_dev->speed = STM32F4_I2C_SPEED_FAST;
+
+ i2c_dev->dev = &pdev->dev;
+
+ ret = devm_request_irq(&pdev->dev, irq_event, stm32f4_i2c_isr_event, 0,
+ pdev->name, i2c_dev);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to request irq event %i\n",
+ irq_event);
+ goto clk_free;
+ }
+
+ ret = devm_request_irq(&pdev->dev, irq_error, stm32f4_i2c_isr_error, 0,
+ pdev->name, i2c_dev);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to request irq error %i\n",
+ irq_error);
+ goto clk_free;
+ }
+
+ ret = stm32f4_i2c_hw_config(i2c_dev);
+ if (ret)
+ goto clk_free;
+
+ adap = &i2c_dev->adap;
+ i2c_set_adapdata(adap, i2c_dev);
+ snprintf(adap->name, sizeof(adap->name), "STM32 I2C(%pa)", &res->start);
+ adap->owner = THIS_MODULE;
+ adap->timeout = 2 * HZ;
+ adap->retries = 0;
+ adap->algo = &stm32f4_i2c_algo;
+ adap->dev.parent = &pdev->dev;
+ adap->dev.of_node = pdev->dev.of_node;
+
+ init_completion(&i2c_dev->complete);
+
+ ret = i2c_add_adapter(adap);
+ if (ret)
+ goto clk_free;
+
+ platform_set_drvdata(pdev, i2c_dev);
+
+ clk_disable(i2c_dev->clk);
+
+ dev_info(i2c_dev->dev, "STM32F4 I2C driver registered\n");
+
+ return 0;
+
+clk_free:
+ clk_disable_unprepare(i2c_dev->clk);
+ return ret;
+}
+
+static int stm32f4_i2c_remove(struct platform_device *pdev)
+{
+ struct stm32f4_i2c_dev *i2c_dev = platform_get_drvdata(pdev);
+
+ i2c_del_adapter(&i2c_dev->adap);
+
+ clk_unprepare(i2c_dev->clk);
+
+ return 0;
+}
+
+static const struct of_device_id stm32f4_i2c_match[] = {
+ { .compatible = "st,stm32f4-i2c", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, stm32f4_i2c_match);
+
+static struct platform_driver stm32f4_i2c_driver = {
+ .driver = {
+ .name = "stm32f4-i2c",
+ .of_match_table = stm32f4_i2c_match,
+ },
+ .probe = stm32f4_i2c_probe,
+ .remove = stm32f4_i2c_remove,
+};
+
+module_platform_driver(stm32f4_i2c_driver);
+
+MODULE_AUTHOR("M'boumba Cedric Madianga <cedric.madianga@gmail.com>");
+MODULE_DESCRIPTION("STMicroelectronics STM32F4 I2C driver");
+MODULE_LICENSE("GPL v2");
--
1.9.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox