* Re: [v1] i2c: busses: i2c-designware-pcidrv:- Unmap region obtained by pcim_iomap_regions
From: kbuild test robot @ 2016-12-23 11:15 UTC (permalink / raw)
To: Arvind Yadav
Cc: kbuild-all, jarkko.nikula, wsa, mika.westerberg,
andriy.shevchenko, linux-i2c, linux-kernel
In-Reply-To: <1482487893-17589-1-git-send-email-arvind.yadav.cs@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3016 bytes --]
Hi Arvind,
[auto build test WARNING on wsa/i2c/for-next]
[also build test WARNING on v4.9 next-20161223]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Arvind-Yadav/i2c-busses-i2c-designware-pcidrv-Unmap-region-obtained-by-pcim_iomap_regions/20161223-185314
base: https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-next
config: x86_64-randconfig-x007-201651 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All warnings (new ones prefixed by >>):
drivers/i2c/busses/i2c-designware-pcidrv.c: In function 'i2c_dw_pci_probe':
>> drivers/i2c/busses/i2c-designware-pcidrv.c:285:1: warning: control reaches end of non-void function [-Wreturn-type]
}
^
vim +285 drivers/i2c/busses/i2c-designware-pcidrv.c
fe20ff5c Dirk Brandewie 2011-10-06 269 adap->class = 0;
8eb5c87a Dustin Byford 2015-10-23 270 ACPI_COMPANION_SET(&adap->dev, ACPI_COMPANION(&pdev->dev));
fe20ff5c Dirk Brandewie 2011-10-06 271 adap->nr = controller->bus_num;
089c729a Mika Westerberg 2014-02-19 272
d80d1341 Jarkko Nikula 2015-10-12 273 r = i2c_dw_probe(dev);
d80d1341 Jarkko Nikula 2015-10-12 274 if (r)
5c99d5d6 Arvind Yadav 2016-12-23 275 goto error;
fe20ff5c Dirk Brandewie 2011-10-06 276
43452335 Mika Westerberg 2013-04-10 277 pm_runtime_set_autosuspend_delay(&pdev->dev, 1000);
43452335 Mika Westerberg 2013-04-10 278 pm_runtime_use_autosuspend(&pdev->dev);
be58eda7 Mika Westerberg 2014-02-04 279 pm_runtime_put_autosuspend(&pdev->dev);
18dbdda8 Dirk Brandewie 2011-10-06 280 pm_runtime_allow(&pdev->dev);
18dbdda8 Dirk Brandewie 2011-10-06 281
fe20ff5c Dirk Brandewie 2011-10-06 282 return 0;
5c99d5d6 Arvind Yadav 2016-12-23 283 error:
5c99d5d6 Arvind Yadav 2016-12-23 284 pcim_iounmap_regions(pdev, 1 << 0);
fe20ff5c Dirk Brandewie 2011-10-06 @285 }
fe20ff5c Dirk Brandewie 2011-10-06 286
0b255e92 Bill Pemberton 2012-11-27 287 static void i2c_dw_pci_remove(struct pci_dev *pdev)
fe20ff5c Dirk Brandewie 2011-10-06 288 {
fe20ff5c Dirk Brandewie 2011-10-06 289 struct dw_i2c_dev *dev = pci_get_drvdata(pdev);
fe20ff5c Dirk Brandewie 2011-10-06 290
18dbdda8 Dirk Brandewie 2011-10-06 291 i2c_dw_disable(dev);
18dbdda8 Dirk Brandewie 2011-10-06 292 pm_runtime_forbid(&pdev->dev);
18dbdda8 Dirk Brandewie 2011-10-06 293 pm_runtime_get_noresume(&pdev->dev);
:::::: The code at line 285 was first introduced by commit
:::::: fe20ff5c7e9ca7f5369aacc7d7ca3efeda3b90fe i2c-designware: Add support for Designware core behind PCI devices.
:::::: TO: Dirk Brandewie <dirk.brandewie@gmail.com>
:::::: CC: Ben Dooks <ben-linux@fluff.org>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 27138 bytes --]
^ permalink raw reply
* Re: [v1] i2c: busses: i2c-designware-pcidrv:- Unmap region obtained by pcim_iomap_regions
From: Mika Westerberg @ 2016-12-23 10:44 UTC (permalink / raw)
To: Arvind Yadav
Cc: jarkko.nikula, wsa, andriy.shevchenko, linux-i2c, linux-kernel
In-Reply-To: <1482487893-17589-1-git-send-email-arvind.yadav.cs@gmail.com>
On Fri, Dec 23, 2016 at 03:41:33PM +0530, Arvind Yadav wrote:
> Release IO memory mapping, if i2c_dw_pci_probe is not successful.
Point of pcim_iomap_regions() is that the regions are released
automatically. So there is no need for explicit release.
^ permalink raw reply
* Re: [PATCH v2] i2c: designware: add reset interface
From: Philipp Zabel @ 2016-12-23 10:26 UTC (permalink / raw)
To: zhangfei
Cc: Ramiro Oliveira, Andy Shevchenko, Wolfram Sang,
mika.westerberg@linux.intel.com, jarkko.nikula@linux.intel.com,
linux-arm-kernel@lists.infradead.org, linux-i2c@vger.kernel.org
In-Reply-To: <49a5d8a4-864e-e80e-eee0-57c876d25aaf@linaro.org>
Hi Zhangfei,
Am Freitag, den 16.12.2016, 11:01 +0800 schrieb zhangfei:
> Hi, Philipp
>
> On 2016年12月16日 10:45, zhangfei wrote:
[...]
> Sorry, a bit confused.
> Is that mean we should not use devm_reset_control_get_optional()
> While driver should decide whether use
> devm_reset_control_get_optional_exclusive() or
> devm_reset_control_get_optional_shared()
> What if different platform has different requirement?
>
> Looks the difference between _exclusive and _shared is refcount,
> How about handle this inside, and not decided by interface?
Because there is a significant difference in how the actual reset line
behaves. The shared resets are clock-like, and should only be used if
the device needs them to be deasserted to be enabled, but is fine if
they have been deasserted earlier or if they are not immediately
asserted after the device is disabled, but some time later.
For the shared / non-exclusive resets calling reset_control_assert and
then reset_control_deassert is not guaranteed to do anything at all,
because another user of the reset line could keep it deasserted.
If the device needs a reset to be issued at a certain point in time on
the other hand, for example to reset its state machine or registers to a
known good state, calling assert must guarantee to actually assert the
reset. This can only be done if the reset is exclusive.
Whether guaranteed asserts (exclusive reset lines) are necessary depends
on the device, so this decision has to be made in the driver.
regards
Philipp
^ permalink raw reply
* [v1] i2c: busses: i2c-designware-pcidrv:- Unmap region obtained by pcim_iomap_regions
From: Arvind Yadav @ 2016-12-23 10:11 UTC (permalink / raw)
To: jarkko.nikula, wsa, mika.westerberg
Cc: andriy.shevchenko, linux-i2c, linux-kernel
Release IO memory mapping, if i2c_dw_pci_probe is not successful.
Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
---
drivers/i2c/busses/i2c-designware-pcidrv.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
index d6423cf..75e6e27 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -228,8 +228,10 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
}
dev = devm_kzalloc(&pdev->dev, sizeof(struct dw_i2c_dev), GFP_KERNEL);
- if (!dev)
- return -ENOMEM;
+ if (!dev) {
+ r = -ENOMEM;
+ goto error;
+ }
dev->clk = NULL;
dev->controller = controller;
@@ -241,7 +243,7 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
if (controller->setup) {
r = controller->setup(pdev, controller);
if (r)
- return r;
+ goto error;
}
dev->functionality = controller->functionality |
@@ -270,7 +272,7 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
r = i2c_dw_probe(dev);
if (r)
- return r;
+ goto error;
pm_runtime_set_autosuspend_delay(&pdev->dev, 1000);
pm_runtime_use_autosuspend(&pdev->dev);
@@ -278,6 +280,8 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
pm_runtime_allow(&pdev->dev);
return 0;
+error:
+ pcim_iounmap_regions(pdev, 1 << 0);
}
static void i2c_dw_pci_remove(struct pci_dev *pdev)
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH v7 2/5] i2c: Add STM32F4 I2C driver
From: Uwe Kleine-König @ 2016-12-23 9:00 UTC (permalink / raw)
To: M'boumba Cedric Madianga
Cc: wsa-z923LK4zBo2bacvFa/9K2g, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mcoquelin.stm32-Re5JQEeQqe8AvxtiuMwx3w,
alexandre.torgue-qxv4g6HH51o,
linus.walleij-QSEj5FYQhm4dnm+yROfE0A, patrice.chotard-qxv4g6HH51o,
linux-I+IVW8TIWO2tmTQ+vhA3Yw, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1482413704-17531-3-git-send-email-cedric.madianga-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Hello,
On Thu, Dec 22, 2016 at 02:35:01PM +0100, M'boumba Cedric Madianga wrote:
> This patch adds support for the STM32F4 I2C controller.
>
> Signed-off-by: M'boumba Cedric Madianga <cedric.madianga-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> drivers/i2c/busses/Kconfig | 10 +
> drivers/i2c/busses/Makefile | 1 +
> drivers/i2c/busses/i2c-stm32f4.c | 896 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 907 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..ca11dee
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-stm32f4.c
> @@ -0,0 +1,896 @@
> +/*
> + * 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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> + *
> + * 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_SWRST BIT(15)
> +#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))
((n) & STM32F4_I2C_CR2_FREQ_MASK)
should be enough.
> +#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))
ditto
> +#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 42U
> +#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 low/high period in microsecond
> + * @mul_ccr: Value to be multiplied to CCR to reach 100Khz/400Khz SCL frequency
> + * @min_ccr: Minimum clock ctrl reg value to reach 100Khz/400Khz SCL frequency
s/Khz/ kHz/
> + */
> +struct stm32f4_i2c_timings {
> + u32 duty;
> + u32 scl_period;
> + u32 mul_ccr;
> + u32 min_ccr;
> +};
> +
> +/**
> + * 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;
You bought the argument about alignment of = in stm32f4_i2c_driver. The
same logic applies here.
> +};
> +
> +/**
> + * 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;
> +};
ditto
> +
> +/*
> + * In standard mode:
> + * SCL high period = SCL low period = CCR * I2C CLK period
> + * So, CCR = SCL period * I2C CLK frequency
is "SCL period" the same as "SCL low period"? I2C CLK frequency is the
input clk? If so, it's confusing that it has I2C in its name. The
reference manual calls it PCLK1 (parent clock?).
> + * In fast mode:
> + * DUTY = 0: Fast mode tlow/thigh = 2
> + * DUTY = 1: Fast mode tlow/thigh = 16/9
> + * If Duty = 0; SCL high period = 1 * CCR * I2C CLK period
> + * SCL low period = 2 * CCR * I2C CLK period
> + * If Duty = 1; SCL high period = 9 * CCR * I2C CLK period
> + * SCL low period = 16 * CCR * I2C CLK period
I'd drop the first two lines about the proportions.
> + *
> + * Note that Duty has to bet set to reach 400khz in Fast mode
s/khz/ kHz/
I don't understand why DUTY is required to reach 400 kHz. Given a parent
freq of 30 MHz, with CCR = 25 and DUTY = 0 we have:
t_high = 25 * 33.333 ns = 833.333 ns
t_low = 2 * 25 * 33.333 ns = 1666.667 ns
then t_high and t_low satisfy the i2c bus specification
(t_low > 1300 ns, t_high > 600 ns) and we have t_low + t_high = 2500 ns
= 1 / 400 kHz.
Where is the error?
> + * So, in order to cover both SCL high/low with Duty = 1,
> + * CCR = 16 * SCL period * I2C CLK frequency
I don't get that. Actually you need to use low + high, so
CCR = parentrate / (25 * 400 kHz), right?
> + *
> + * Please note that the minimum allowed value is 0x04, except in FAST DUTY mode
> + * where the minimum allowed value is 0x01
> + */
> +static struct stm32f4_i2c_timings i2c_timings[] = {
> + [STM32F4_I2C_SPEED_STANDARD] = {
> + .mul_ccr = 1,
> + .min_ccr = 4,
> + .duty = 0,
> + .scl_period = 5,
> + },
> + [STM32F4_I2C_SPEED_FAST] = {
> + .mul_ccr = 16,
> + .min_ccr = 1,
> + .duty = 1,
> + .scl_period = 2,
> + },
> +};
> +
> +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_soft_reset(struct stm32f4_i2c_dev *i2c_dev)
> +{
> + void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR1;
> + u32 val;
> +
> + val = readl_relaxed(reg);
> + writel_relaxed(val | STM32F4_I2C_CR1_SWRST, reg);
> + writel_relaxed(val, 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 void stm32f4_i2c_set_periph_clk_freq(struct stm32f4_i2c_dev *i2c_dev)
> +{
> + u32 clk_rate, cr2, freq;
> +
> + /*
> + * The minimum allowed frequency is 2 MHz, the maximum frequency is
> + * limited by the maximum APB frequency 42 MHz
> + */
> + cr2 = readl_relaxed(i2c_dev->base + STM32F4_I2C_CR2);
> + cr2 &= ~STM32F4_I2C_CR2_FREQ_MASK;
> + clk_rate = clk_get_rate(i2c_dev->clk);
> + freq = DIV_ROUND_UP(clk_rate, HZ_TO_MHZ);
> + freq = clamp(freq, STM32F4_I2C_MIN_FREQ, STM32F4_I2C_MAX_FREQ);
> + cr2 |= STM32F4_I2C_CR2_FREQ(freq);
> + writel_relaxed(cr2, i2c_dev->base + STM32F4_I2C_CR2);
Last round I suggested error checking here instead of silent clamping.
> +}
> +
> +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;
if freq is big such that freq * 300 overflows does this result in a
wrong result, or does the compiler optimize correctly?
> + 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 * t->mul_ccr;
> + if (val < t->min_ccr)
> + val = t->min_ccr;
> + 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 void stm32f4_i2c_hw_config(struct stm32f4_i2c_dev *i2c_dev)
> +{
> + void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR1;
> +
> + /* Disable I2C */
> + stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_PE);
> +
> + stm32f4_i2c_set_periph_clk_freq(i2c_dev);
> +
> + 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);
This is the first write to STM32F4_I2C_CR1, right? So the state from the
bootloader leaks here. This probably works most of the time, but if it
makes problems later, that's hard to debug. Also, what if the bootloader
already did some i2c transfers and kept the PE bit 1? I read in the
manual that PE must be 0 for some things. So this only works most of the
time.
> +}
> +
> +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_err(i2c_dev->dev, "bus not free\n");
drop error message please or degrade to dev_debug
> + 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;
> + case 2:
> + case 3:
> + stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR2_ITBUFEN);
> + break;
> + 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:
> + reg = i2c_dev->base + STM32F4_I2C_CR1;
> + /* Generate STOP or repeated Start */
> + if (msg->stop)
> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_STOP);
> + else
> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_START);
> +
> + /* Read two last data bytes */
> + for (i = 2; i > 0; i--)
> + stm32f4_i2c_read_msg(i2c_dev);
> +
> + /* Disable events and error interrupts */
> + 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:
> + /* Enable ACK and read data */
> + 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;
> + }
> +}
This is still not really understandable.
> +
> +/**
> + * 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;
> + void __iomem *reg;
> + u32 status, possible_status, ien;
> + int flag;
> +
> + ien = readl_relaxed(i2c_dev->base + STM32F4_I2C_CR2);
> + ien &= STM32F4_I2C_CR2_IRQ_MASK;
> + possible_status = 0;
This can already be done when declaring possible_status.
> +
> + /* Check possible status combinations */
> + if (ien & STM32F4_I2C_CR2_ITEVTEN) {
> + possible_status = STM32F4_I2C_SR1_ITEVTEN_MASK;
> + if (ien & STM32F4_I2C_CR2_ITBUFEN)
> + possible_status |= STM32F4_I2C_SR1_ITBUFEN_MASK;
> + }
> +
> + status = readl_relaxed(i2c_dev->base + STM32F4_I2C_SR1);
> +
> + if (!(status & possible_status)) {
> + dev_dbg(i2c_dev->dev,
> + "spurious evt irq (status=0x%08x, ien=0x%08x)\n",
> + status, ien);
> + return IRQ_NONE;
> + }
> +
> + while (status & possible_status) {
> + /* Use __fls() to check error bits first */
> + flag = __fls(status & possible_status);
> +
> + switch (1 << flag) {
> + case STM32F4_I2C_SR1_SB:
> + stm32f4_i2c_write_byte(i2c_dev, msg->addr);
> + break;
> +
> + case 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);
> + possible_status |= STM32F4_I2C_SR1_ITBUFEN_MASK;
> + break;
> +
> + case STM32F4_I2C_SR1_BTF:
> + if (msg->addr & I2C_M_RD)
> + stm32f4_i2c_handle_rx_btf(i2c_dev);
> + else
> + stm32f4_i2c_handle_write(i2c_dev);
> + break;
> +
> + case STM32F4_I2C_SR1_TXE:
> + stm32f4_i2c_handle_write(i2c_dev);
> + break;
> +
> + case STM32F4_I2C_SR1_RXNE:
> + stm32f4_i2c_handle_read(i2c_dev);
> + break;
> +
> + default:
> + dev_err(i2c_dev->dev,
> + "evt irq unhandled: status=0x%08x)\n",
> + status);
> + return IRQ_NONE;
> + }
> + status &= ~(1 << flag);
> + }
I wouldn't do this in a loop. Just do:
if (status & STM32F4_I2C_SR1_SB) {
...
}
if (status & ...) {
}
Then it's obvious by reading the code in which order they are handled
without the need to check the definitions. Do you really need to jugle
with possible_status?
> +
> + 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, possible_status, ien;
> + int flag;
> +
> + ien = readl_relaxed(i2c_dev->base + STM32F4_I2C_CR2);
> + ien &= STM32F4_I2C_CR2_IRQ_MASK;
> + possible_status = 0;
> +
> + /* Check possible status combinations */
> + if (ien & STM32F4_I2C_CR2_ITERREN)
> + possible_status = STM32F4_I2C_SR1_ITERREN_MASK;
> +
> + status = readl_relaxed(i2c_dev->base + STM32F4_I2C_SR1);
> +
> + if (!(status & possible_status)) {
> + dev_dbg(i2c_dev->dev,
> + "spurious err it (status=0x%08x, ien=0x%08x)\n",
> + status, ien);
> + return IRQ_NONE;
> + }
> +
> + /* Use __fls() to check error bits first */
> + flag = __fls(status & possible_status);
> +
> + switch (1 << flag) {
> + case STM32F4_I2C_SR1_BERR:
> + reg = i2c_dev->base + STM32F4_I2C_SR1;
> + stm32f4_i2c_clr_bits(reg, STM32F4_I2C_SR1_BERR);
> + msg->result = -EIO;
> + break;
> +
> + case STM32F4_I2C_SR1_ARLO:
> + reg = i2c_dev->base + STM32F4_I2C_SR1;
> + stm32f4_i2c_clr_bits(reg, STM32F4_I2C_SR1_ARLO);
> + msg->result = -EAGAIN;
> + break;
> +
> + case STM32F4_I2C_SR1_AF:
> + reg = i2c_dev->base + STM32F4_I2C_CR1;
> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_STOP);
> + msg->result = -EIO;
> + break;
> +
> + default:
> + dev_err(i2c_dev->dev,
> + "err it unhandled: status=0x%08x)\n", status);
> + return IRQ_NONE;
> + }
You only check a single irq flag here.
> +
> + stm32f4_i2c_soft_reset(i2c_dev);
> + 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 PEC position Ack */
> + stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_POS);
This is the only place mentioning PEC. Should this be about POS instead?
> +
> + 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;
> + }
> +
> + stm32f4_i2c_hw_config(i2c_dev);
> +
> + 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(i2c_dev->clk);
> + if (ret) {
> + dev_err(i2c_dev->dev, "Failed to prepare 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);
Starting here this irq might trigger. Can this happen? If so,
stm32f4_i2c_isr_event is called without the adapter being registered.
Probably not an issue as the controller was just reset.
> + 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;
> + }
> +
> + 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);
> +
> + dev_info(i2c_dev->dev, "STM32F4 I2C driver registered\n");
> +
> + return 0;
> +
> +clk_free:
> + clk_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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>");
> +MODULE_DESCRIPTION("STMicroelectronics STM32F4 I2C driver");
> +MODULE_LICENSE("GPL v2");
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
--
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 v7 3/5] ARM: dts: stm32: Add I2C1 support for STM32F429 SoC
From: Uwe Kleine-König @ 2016-12-22 19:11 UTC (permalink / raw)
To: M'boumba Cedric Madianga
Cc: wsa, robh+dt, mcoquelin.stm32, alexandre.torgue, linus.walleij,
patrice.chotard, linux, linux-i2c, devicetree, linux-arm-kernel,
linux-kernel
In-Reply-To: <1482413704-17531-4-git-send-email-cedric.madianga@gmail.com>
Hello,
On Thu, Dec 22, 2016 at 02:35:02PM +0100, M'boumba Cedric Madianga wrote:
> @@ -337,6 +350,16 @@
> slew-rate = <2>;
> };
> };
> +
> + i2c1_pins_b: i2c1@0 {
> + pins1 {
> + pinmux = <STM32F429_PB9_FUNC_I2C1_SDA>;
> + drive-open-drain;
> + };
> + pins2 {
> + pinmux = <STM32F429_PB6_FUNC_I2C1_SCL>;
> + };
the second doesn't need the open-drain property? Why?
> + };
> };
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* Re: [v1] i2c: busses: i2c-designware-pcidrv:- Handle return NULL error from pcim_iomap_table
From: arvind Yadav @ 2016-12-22 17:11 UTC (permalink / raw)
To: Andy Shevchenko, jarkko.nikula, wsa
Cc: mika.westerberg, linux-i2c, linux-kernel
In-Reply-To: <1482417309.9552.128.camel@linux.intel.com>
Yes, It will not fail. Sorry for the noise.
Thanks
-Arvind
On Thursday 22 December 2016 08:05 PM, Andy Shevchenko wrote:
> On Thu, 2016-12-22 at 17:09 +0530, Arvind Yadav wrote:
>> Here, If pcim_iomap_table will fail. It will return NULL.
>> Kernel can run into a NULL-pointer dereference.
>> This error check will avoid NULL pointer dereference.
>>
>> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
>> ---
>> drivers/i2c/busses/i2c-designware-pcidrv.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c
>> b/drivers/i2c/busses/i2c-designware-pcidrv.c
>> index d6423cf..6a1907d 100644
>> --- a/drivers/i2c/busses/i2c-designware-pcidrv.c
>> +++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
>> @@ -235,6 +235,10 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
>> dev->controller = controller;
>> dev->get_clk_rate_khz = i2c_dw_get_clk_rate_khz;
>> dev->base = pcim_iomap_table(pdev)[0];
>> + if (!dev->base) {
>> + dev_err(&pdev->dev, "I/O map table allocation
>> failed\n");
>> + return -ENOMEM;
>> + }
> NAK.
>
>
^ permalink raw reply
* [PATCH] i2c: mv64xxx: add suspend/resume support
From: Thomas Petazzoni @ 2016-12-22 15:54 UTC (permalink / raw)
To: Wolfram Sang, linux-i2c
Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Gregory Clement,
linux-arm-kernel, Grzegorz Jaszczyk, Thomas Petazzoni
From: Grzegorz Jaszczyk <jaz@semihalf.com>
This commit implements suspend/resume support in the mv64xxx I2C
controller driver. There is no need to implement a ->suspend() hook, as
calling mv64xxx_i2c_hw_init() at ->resume() time is enough.
Signed-off-by: Grzegorz Jaszczyk <jaz@semihalf.com>
Reviewed-by: Nadav Haklai <nadavh@marvell.com>
Reviewed-by: Lior Amsalem <alior@marvell.com>
Tested-by: Lior Amsalem <alior@marvell.com>
[Thomas: switch to dev_pm_ops, fix build warning when !PM.]
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
drivers/i2c/busses/i2c-mv64xxx.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
index b4dec08..a50bd68 100644
--- a/drivers/i2c/busses/i2c-mv64xxx.c
+++ b/drivers/i2c/busses/i2c-mv64xxx.c
@@ -977,11 +977,32 @@ mv64xxx_i2c_remove(struct platform_device *dev)
return 0;
}
+#ifdef CONFIG_PM
+static int mv64xxx_i2c_resume(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct mv64xxx_i2c_data *drv_data = platform_get_drvdata(pdev);
+
+ mv64xxx_i2c_hw_init(drv_data);
+
+ return 0;
+}
+
+static const struct dev_pm_ops mv64xxx_i2c_pm = {
+ .resume = mv64xxx_i2c_resume,
+};
+
+#define mv64xxx_i2c_pm_ops (&mv64xxx_i2c_pm)
+#else
+#define mv64xxx_i2c_pm_ops NULL
+#endif
+
static struct platform_driver mv64xxx_i2c_driver = {
.probe = mv64xxx_i2c_probe,
.remove = mv64xxx_i2c_remove,
.driver = {
.name = MV64XXX_I2C_CTLR_NAME,
+ .pm = mv64xxx_i2c_pm_ops,
.of_match_table = mv64xxx_i2c_of_match_table,
},
};
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v4 3/5] i2c: designware: Add slave definitions
From: Andy Shevchenko @ 2016-12-22 15:29 UTC (permalink / raw)
To: Luis Oliveira, Rob Herring
Cc: wsa@the-dreams.de, mark.rutland@arm.com,
jarkko.nikula@linux.intel.com, mika.westerberg@linux.intel.com,
linux-i2c@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, Ramiro.Oliveira@synopsys.com,
Joao.Pinto@synopsys.com, CARLOS.PALMINHA@synopsys.com
In-Reply-To: <3ef11852-8dc5-dbb3-7dc3-1ec8f4fc58da@synopsys.com>
On Thu, 2016-12-22 at 14:59 +0000, Luis Oliveira wrote:
> On 13-Dec-16 14:11, Rob Herring wrote:
> > Something like this:
> >
> > of_for_each_child_node(child) {
> > of_property_read_u32(child, "reg", ®);
> > if (reg & I2C_OWN_SLAVE_ADDRESS))
> > im_a_slave = true;
> > }
> >
> > ...rather than testing "mode" is equal to "slave".
> >
> > Rob
> >
>
> Hi Rob, Andy,
>
> I'm struggling to implement your suggestion @Rob. I checked the
> tegra124-jetson-tk1.dts that uses that approach but I have some
> doubts.
>
> My DT is as follows
>
> i2c@0x2000 {
> compatible = "snps,designware-i2c";
> reg = <0x2000 0x100>;
> clock-frequency = <400000>;
> clocks = <&i2cclk>;
> interrupts = <0>;
>
> I could add something like this:
>
> eeprom@64 {
> compatible = "linux,slave-24c02";
> reg = <(I2C_OWN_SLAVE_ADDRESS | 0x64)>;
> }
>
> But I think this is different form what I was doing before. I have two
> questions:
>
> - This way the I2C controller is identified as a slave controller or
> just the
> subnode eeprom?
> - This way looks like my slave address will be fixed
>
> In the previous Patch v3 submission @Andy suggested a property that
> selects mode
> that I did and it's working. And you @Rob suggested to do it a common
> property.
> It is implemented in the DT like:
>
> mode = "slave";
>
> So before I do this changes can you please agree both if you still
> think this is
> the best approach?
I'm a bit lost in the discussion (and TBH busy by something else), so I
would agree on whatever you and Rob make an agreement on.
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply
* Re: [PATCH v4 3/5] i2c: designware: Add slave definitions
From: Luis Oliveira @ 2016-12-22 14:59 UTC (permalink / raw)
To: Rob Herring, Luis de Oliveira
Cc: wsa@the-dreams.de, mark.rutland@arm.com,
jarkko.nikula@linux.intel.com, andriy.shevchenko@linux.intel.com,
mika.westerberg@linux.intel.com, linux-i2c@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
Ramiro.Oliveira@synopsys.com, Joao.Pinto@synopsys.com,
CARLOS.PALMINHA@synopsys.com
In-Reply-To: <CAL_Jsq+DDx460jcLj34_X9MdtCqyc4izwc=3ThnFri+NjBaGUA@mail.gmail.com>
On 13-Dec-16 14:11, Rob Herring wrote:
> Again, please don't top post. And your line wrapping is messed up.
> IOW, you can't use Outlook.
>
> On Tue, Dec 13, 2016 at 4:50 AM, Luis de Oliveira
> <Luis.Oliveira@synopsys.com> wrote:
>> The controller for i2c-designware cannot be slave/master at the same time and it has to be enabled knowing beforehand if we want it to be slave or master by something outside of the controller itself.
>>
>> I as looking and I see the use of this I2C_OWN_SLAVE_ADDRESS with the "linux,slave-24c02" slave interface but I am not seeing how it will help me identify a selected i2c-designware block as a "slave" device before instantiation. I'm sorry if I'm not understanding properly.
>> I use the "linux,slave-24c02" to instantiate the i2c-designware as a slave with an address so I can do write/read operations, it is how I tested it.
>
> Something like this:
>
> of_for_each_child_node(child) {
> of_property_read_u32(child, "reg", ®);
> if (reg & I2C_OWN_SLAVE_ADDRESS))
> im_a_slave = true;
> }
>
> ...rather than testing "mode" is equal to "slave".
>
> Rob
>
Hi Rob, Andy,
I'm struggling to implement your suggestion @Rob. I checked the
tegra124-jetson-tk1.dts that uses that approach but I have some doubts.
My DT is as follows
i2c@0x2000 {
compatible = "snps,designware-i2c";
reg = <0x2000 0x100>;
clock-frequency = <400000>;
clocks = <&i2cclk>;
interrupts = <0>;
I could add something like this:
eeprom@64 {
compatible = "linux,slave-24c02";
reg = <(I2C_OWN_SLAVE_ADDRESS | 0x64)>;
}
But I think this is different form what I was doing before. I have two questions:
- This way the I2C controller is identified as a slave controller or just the
subnode eeprom?
- This way looks like my slave address will be fixed
In the previous Patch v3 submission @Andy suggested a property that selects mode
that I did and it's working. And you @Rob suggested to do it a common property.
It is implemented in the DT like:
mode = "slave";
So before I do this changes can you please agree both if you still think this is
the best approach?
Thank you both for your time,
Luis
>>
>> Luis
>>
>> -----Original Message-----
>> From: Rob Herring [mailto:robh@kernel.org]
>> Sent: Monday, December 12, 2016 23:16
>> To: Luis de Oliveira <Luis.Oliveira@synopsys.com>
>> Cc: wsa@the-dreams.de; mark.rutland@arm.com; jarkko.nikula@linux.intel.com; andriy.shevchenko@linux.intel.com; mika.westerberg@linux.intel.com; linux-i2c@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; Ramiro.Oliveira@synopsys.com; Joao.Pinto@synopsys.com; CARLOS.PALMINHA@synopsys.com
>> Subject: Re: [PATCH v4 3/5] i2c: designware: Add slave definitions
>>
>> On Mon, Dec 12, 2016 at 12:35 PM, Luis de Oliveira <Luis.Oliveira@synopsys.com> wrote:
>>> Hi all,
>>
>> Please don't top post.
>>
>>>
>>> The slave address could be set by the I2C slave backend so I can't use it to setup the controller.
>>> A boolean property was my initial approach then Andy and Wolfram Sang suggested the use of compatible strings and later It was suggested to use a property to select mode but I can do it again if it's the best way.
>>> Can you please tell me where should it be documented?
>>
>> bindings/i2c/i2c.txt.
>>
>> Actually, looking at this some more, we already have a way to describe the controller being a slave device with the I2C_OWN_SLAVE_ADDRESS flag in the reg property. We should just need a helper to read reg property of each child and check for the bit set.
>>
>> Rob
^ permalink raw reply
* Re: [v1] i2c: busses: i2c-designware-pcidrv:- Handle return NULL error from pcim_iomap_table
From: Andy Shevchenko @ 2016-12-22 14:35 UTC (permalink / raw)
To: Arvind Yadav, jarkko.nikula, wsa; +Cc: mika.westerberg, linux-i2c, linux-kernel
In-Reply-To: <1482406759-13094-1-git-send-email-arvind.yadav.cs@gmail.com>
On Thu, 2016-12-22 at 17:09 +0530, Arvind Yadav wrote:
> Here, If pcim_iomap_table will fail. It will return NULL.
> Kernel can run into a NULL-pointer dereference.
> This error check will avoid NULL pointer dereference.
>
> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
> ---
> drivers/i2c/busses/i2c-designware-pcidrv.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c
> b/drivers/i2c/busses/i2c-designware-pcidrv.c
> index d6423cf..6a1907d 100644
> --- a/drivers/i2c/busses/i2c-designware-pcidrv.c
> +++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
> @@ -235,6 +235,10 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
> dev->controller = controller;
> dev->get_clk_rate_khz = i2c_dw_get_clk_rate_khz;
> dev->base = pcim_iomap_table(pdev)[0];
> + if (!dev->base) {
> + dev_err(&pdev->dev, "I/O map table allocation
> failed\n");
> + return -ENOMEM;
> + }
NAK.
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply
* [PATCH v7 5/5] ARM: configs: stm32: Add I2C support for STM32 defconfig
From: M'boumba Cedric Madianga @ 2016-12-22 13:35 UTC (permalink / raw)
To: wsa, robh+dt, mcoquelin.stm32, alexandre.torgue, linus.walleij,
patrice.chotard, linux, linux-i2c, devicetree, linux-arm-kernel,
linux-kernel
Cc: M'boumba Cedric Madianga
In-Reply-To: <1482413704-17531-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 e7b56d4..9494eaf 100644
--- a/arch/arm/configs/stm32_defconfig
+++ b/arch/arm/configs/stm32_defconfig
@@ -52,6 +52,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 v7 4/5] ARM: dts: stm32: Add I2C1 support for STM32429 eval board
From: M'boumba Cedric Madianga @ 2016-12-22 13:35 UTC (permalink / raw)
To: wsa, robh+dt, mcoquelin.stm32, alexandre.torgue, linus.walleij,
patrice.chotard, linux, linux-i2c, devicetree, linux-arm-kernel,
linux-kernel
Cc: M'boumba Cedric Madianga
In-Reply-To: <1482413704-17531-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 afb90bc..74e0045 100644
--- a/arch/arm/boot/dts/stm32429i-eval.dts
+++ b/arch/arm/boot/dts/stm32429i-eval.dts
@@ -141,3 +141,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 v7 3/5] ARM: dts: stm32: Add I2C1 support for STM32F429 SoC
From: M'boumba Cedric Madianga @ 2016-12-22 13:35 UTC (permalink / raw)
To: wsa, robh+dt, mcoquelin.stm32, alexandre.torgue, linus.walleij,
patrice.chotard, linux, linux-i2c, devicetree, linux-arm-kernel,
linux-kernel
Cc: M'boumba Cedric Madianga
In-Reply-To: <1482413704-17531-1-git-send-email-cedric.madianga@gmail.com>
This patch adds I2C1 support for STM32F429 SoC
Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
---
arch/arm/boot/dts/stm32f429.dtsi | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi
index 7de52ee..2277a2d 100644
--- a/arch/arm/boot/dts/stm32f429.dtsi
+++ b/arch/arm/boot/dts/stm32f429.dtsi
@@ -48,6 +48,7 @@
#include "skeleton.dtsi"
#include "armv7-m.dtsi"
#include <dt-bindings/pinctrl/stm32f429-pinfunc.h>
+#include <dt-bindings/mfd/stm32f4-rcc.h>
/ {
clocks {
@@ -140,6 +141,18 @@
status = "disabled";
};
+ i2c1: i2c@40005400 {
+ compatible = "st,stm32f4-i2c";
+ reg = <0x40005400 0x400>;
+ interrupts = <31>,
+ <32>;
+ resets = <&rcc STM32F4_APB1_RESET(I2C1)>;
+ clocks = <&rcc 0 STM32F4_APB1_CLOCK(I2C1)>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+ };
+
usart7: serial@40007800 {
compatible = "st,stm32-usart", "st,stm32-uart";
reg = <0x40007800 0x400>;
@@ -337,6 +350,16 @@
slew-rate = <2>;
};
};
+
+ i2c1_pins_b: i2c1@0 {
+ pins1 {
+ pinmux = <STM32F429_PB9_FUNC_I2C1_SDA>;
+ drive-open-drain;
+ };
+ pins2 {
+ pinmux = <STM32F429_PB6_FUNC_I2C1_SCL>;
+ };
+ };
};
rcc: rcc@40023810 {
--
1.9.1
^ permalink raw reply related
* [PATCH v7 2/5] i2c: Add STM32F4 I2C driver
From: M'boumba Cedric Madianga @ 2016-12-22 13:35 UTC (permalink / raw)
To: wsa, robh+dt, mcoquelin.stm32, alexandre.torgue, linus.walleij,
patrice.chotard, linux, linux-i2c, devicetree, linux-arm-kernel,
linux-kernel
Cc: M'boumba Cedric Madianga
In-Reply-To: <1482413704-17531-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 | 896 +++++++++++++++++++++++++++++++++++++++
3 files changed, 907 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..ca11dee
--- /dev/null
+++ b/drivers/i2c/busses/i2c-stm32f4.c
@@ -0,0 +1,896 @@
+/*
+ * 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_SWRST BIT(15)
+#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 42U
+#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 low/high period in microsecond
+ * @mul_ccr: Value to be multiplied to CCR to reach 100Khz/400Khz SCL frequency
+ * @min_ccr: Minimum clock ctrl reg value to reach 100Khz/400Khz SCL frequency
+ */
+struct stm32f4_i2c_timings {
+ u32 duty;
+ u32 scl_period;
+ u32 mul_ccr;
+ u32 min_ccr;
+};
+
+/**
+ * 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 high period = SCL low period = CCR * I2C CLK period
+ * So, CCR = SCL period * I2C CLK frequency
+ *
+ * In fast mode:
+ * DUTY = 0: Fast mode tlow/thigh = 2
+ * DUTY = 1: Fast mode tlow/thigh = 16/9
+ * If Duty = 0; SCL high period = 1 * CCR * I2C CLK period
+ * SCL low period = 2 * CCR * I2C CLK period
+ * If Duty = 1; SCL high period = 9 * CCR * I2C CLK period
+ * SCL low period = 16 * CCR * I2C CLK period
+ *
+ * Note that Duty has to bet set to reach 400khz in Fast mode
+ * So, in order to cover both SCL high/low with Duty = 1,
+ * CCR = 16 * SCL period * I2C CLK frequency
+ *
+ * Please note that the minimum allowed value is 0x04, except in FAST DUTY mode
+ * where the minimum allowed value is 0x01
+ */
+static struct stm32f4_i2c_timings i2c_timings[] = {
+ [STM32F4_I2C_SPEED_STANDARD] = {
+ .mul_ccr = 1,
+ .min_ccr = 4,
+ .duty = 0,
+ .scl_period = 5,
+ },
+ [STM32F4_I2C_SPEED_FAST] = {
+ .mul_ccr = 16,
+ .min_ccr = 1,
+ .duty = 1,
+ .scl_period = 2,
+ },
+};
+
+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_soft_reset(struct stm32f4_i2c_dev *i2c_dev)
+{
+ void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR1;
+ u32 val;
+
+ val = readl_relaxed(reg);
+ writel_relaxed(val | STM32F4_I2C_CR1_SWRST, reg);
+ writel_relaxed(val, 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 void stm32f4_i2c_set_periph_clk_freq(struct stm32f4_i2c_dev *i2c_dev)
+{
+ u32 clk_rate, cr2, freq;
+
+ /*
+ * The minimum allowed frequency is 2 MHz, the maximum frequency is
+ * limited by the maximum APB frequency 42 MHz
+ */
+ cr2 = readl_relaxed(i2c_dev->base + STM32F4_I2C_CR2);
+ cr2 &= ~STM32F4_I2C_CR2_FREQ_MASK;
+ clk_rate = clk_get_rate(i2c_dev->clk);
+ freq = DIV_ROUND_UP(clk_rate, HZ_TO_MHZ);
+ freq = clamp(freq, STM32F4_I2C_MIN_FREQ, STM32F4_I2C_MAX_FREQ);
+ cr2 |= STM32F4_I2C_CR2_FREQ(freq);
+ writel_relaxed(cr2, i2c_dev->base + STM32F4_I2C_CR2);
+}
+
+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 * t->mul_ccr;
+ if (val < t->min_ccr)
+ val = t->min_ccr;
+ 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 void stm32f4_i2c_hw_config(struct stm32f4_i2c_dev *i2c_dev)
+{
+ void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR1;
+
+ /* Disable I2C */
+ stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_PE);
+
+ stm32f4_i2c_set_periph_clk_freq(i2c_dev);
+
+ 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);
+}
+
+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_err(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;
+ case 2:
+ case 3:
+ stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR2_ITBUFEN);
+ break;
+ 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:
+ reg = i2c_dev->base + STM32F4_I2C_CR1;
+ /* Generate STOP or repeated Start */
+ if (msg->stop)
+ stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_STOP);
+ else
+ stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_START);
+
+ /* Read two last data bytes */
+ for (i = 2; i > 0; i--)
+ stm32f4_i2c_read_msg(i2c_dev);
+
+ /* Disable events and error interrupts */
+ 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:
+ /* Enable ACK and read data */
+ 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;
+ void __iomem *reg;
+ u32 status, possible_status, ien;
+ int flag;
+
+ ien = readl_relaxed(i2c_dev->base + STM32F4_I2C_CR2);
+ ien &= STM32F4_I2C_CR2_IRQ_MASK;
+ possible_status = 0;
+
+ /* Check possible status combinations */
+ if (ien & STM32F4_I2C_CR2_ITEVTEN) {
+ possible_status = STM32F4_I2C_SR1_ITEVTEN_MASK;
+ if (ien & STM32F4_I2C_CR2_ITBUFEN)
+ possible_status |= STM32F4_I2C_SR1_ITBUFEN_MASK;
+ }
+
+ status = readl_relaxed(i2c_dev->base + STM32F4_I2C_SR1);
+
+ if (!(status & possible_status)) {
+ dev_dbg(i2c_dev->dev,
+ "spurious evt irq (status=0x%08x, ien=0x%08x)\n",
+ status, ien);
+ return IRQ_NONE;
+ }
+
+ while (status & possible_status) {
+ /* Use __fls() to check error bits first */
+ flag = __fls(status & possible_status);
+
+ switch (1 << flag) {
+ case STM32F4_I2C_SR1_SB:
+ stm32f4_i2c_write_byte(i2c_dev, msg->addr);
+ break;
+
+ case 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);
+ possible_status |= STM32F4_I2C_SR1_ITBUFEN_MASK;
+ break;
+
+ case STM32F4_I2C_SR1_BTF:
+ if (msg->addr & I2C_M_RD)
+ stm32f4_i2c_handle_rx_btf(i2c_dev);
+ else
+ stm32f4_i2c_handle_write(i2c_dev);
+ break;
+
+ case STM32F4_I2C_SR1_TXE:
+ stm32f4_i2c_handle_write(i2c_dev);
+ break;
+
+ case STM32F4_I2C_SR1_RXNE:
+ stm32f4_i2c_handle_read(i2c_dev);
+ break;
+
+ default:
+ dev_err(i2c_dev->dev,
+ "evt irq unhandled: status=0x%08x)\n",
+ status);
+ return IRQ_NONE;
+ }
+ status &= ~(1 << flag);
+ }
+
+ 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, possible_status, ien;
+ int flag;
+
+ ien = readl_relaxed(i2c_dev->base + STM32F4_I2C_CR2);
+ ien &= STM32F4_I2C_CR2_IRQ_MASK;
+ possible_status = 0;
+
+ /* Check possible status combinations */
+ if (ien & STM32F4_I2C_CR2_ITERREN)
+ possible_status = STM32F4_I2C_SR1_ITERREN_MASK;
+
+ status = readl_relaxed(i2c_dev->base + STM32F4_I2C_SR1);
+
+ if (!(status & possible_status)) {
+ dev_dbg(i2c_dev->dev,
+ "spurious err it (status=0x%08x, ien=0x%08x)\n",
+ status, ien);
+ return IRQ_NONE;
+ }
+
+ /* Use __fls() to check error bits first */
+ flag = __fls(status & possible_status);
+
+ switch (1 << flag) {
+ case STM32F4_I2C_SR1_BERR:
+ reg = i2c_dev->base + STM32F4_I2C_SR1;
+ stm32f4_i2c_clr_bits(reg, STM32F4_I2C_SR1_BERR);
+ msg->result = -EIO;
+ break;
+
+ case STM32F4_I2C_SR1_ARLO:
+ reg = i2c_dev->base + STM32F4_I2C_SR1;
+ stm32f4_i2c_clr_bits(reg, STM32F4_I2C_SR1_ARLO);
+ msg->result = -EAGAIN;
+ break;
+
+ case STM32F4_I2C_SR1_AF:
+ reg = i2c_dev->base + STM32F4_I2C_CR1;
+ stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_STOP);
+ msg->result = -EIO;
+ break;
+
+ default:
+ dev_err(i2c_dev->dev,
+ "err it unhandled: status=0x%08x)\n", status);
+ return IRQ_NONE;
+ }
+
+ stm32f4_i2c_soft_reset(i2c_dev);
+ 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 PEC 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;
+ }
+
+ stm32f4_i2c_hw_config(i2c_dev);
+
+ 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(i2c_dev->clk);
+ if (ret) {
+ dev_err(i2c_dev->dev, "Failed to prepare 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;
+ }
+
+ 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);
+
+ dev_info(i2c_dev->dev, "STM32F4 I2C driver registered\n");
+
+ return 0;
+
+clk_free:
+ clk_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
* [PATCH v7 1/5] dt-bindings: Document the STM32 I2C bindings
From: M'boumba Cedric Madianga @ 2016-12-22 13:35 UTC (permalink / raw)
To: wsa, robh+dt, mcoquelin.stm32, alexandre.torgue, linus.walleij,
patrice.chotard, linux, linux-i2c, devicetree, linux-arm-kernel,
linux-kernel
Cc: M'boumba Cedric Madianga
In-Reply-To: <1482413704-17531-1-git-send-email-cedric.madianga@gmail.com>
This patch adds documentation of device tree bindings for the STM32 I2C
controller.
Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
Acked-by: Rob Herring <robh@kernel.org>
---
.../devicetree/bindings/i2c/i2c-stm32.txt | 33 ++++++++++++++++++++++
1 file changed, 33 insertions(+)
create mode 100644 Documentation/devicetree/bindings/i2c/i2c-stm32.txt
diff --git a/Documentation/devicetree/bindings/i2c/i2c-stm32.txt b/Documentation/devicetree/bindings/i2c/i2c-stm32.txt
new file mode 100644
index 0000000..78eaf7b
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-stm32.txt
@@ -0,0 +1,33 @@
+* I2C controller embedded in STMicroelectronics STM32 I2C platform
+
+Required properties :
+- compatible : Must be "st,stm32f4-i2c"
+- reg : Offset and length of the register set for the device
+- interrupts : Must contain the interrupt id for I2C event and then the
+ interrupt id for I2C error.
+- resets: Must contain the phandle to the reset controller.
+- clocks: Must contain the input clock of the I2C instance.
+- A pinctrl state named "default" must be defined to set pins in mode of
+ operation for I2C transfer
+- #address-cells = <1>;
+- #size-cells = <0>;
+
+Optional properties :
+- clock-frequency : Desired I2C bus clock frequency in Hz. If not specified,
+ the default 100 kHz frequency will be used. As only Normal and Fast modes
+ are supported, possible values are 100000 and 400000.
+
+Example :
+
+ i2c@40005400 {
+ compatible = "st,stm32f4-i2c";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x40005400 0x400>;
+ interrupts = <31>,
+ <32>;
+ resets = <&rcc 277>;
+ clocks = <&rcc 0 149>;
+ pinctrl-0 = <&i2c1_sda_pin>, <&i2c1_scl_pin>;
+ pinctrl-names = "default";
+ };
--
1.9.1
^ permalink raw reply related
* [PATCH v7 0/5] Add support for the STM32F4 I2C
From: M'boumba Cedric Madianga @ 2016-12-22 13:34 UTC (permalink / raw)
To: wsa-z923LK4zBo2bacvFa/9K2g, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mcoquelin.stm32-Re5JQEeQqe8AvxtiuMwx3w,
alexandre.torgue-qxv4g6HH51o,
linus.walleij-QSEj5FYQhm4dnm+yROfE0A, patrice.chotard-qxv4g6HH51o,
linux-I+IVW8TIWO2tmTQ+vhA3Yw, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: M'boumba Cedric Madianga
This patchset adds support for the I2C controller embedded in STM32F4xx SoC.
It enables I2C transfer in interrupt mode with Standard-mode and Fast-mode bus
speed.
Changes since v6:
- Add commit message for the patches in defconfig, .dtsi and .dts files (Alex)
- Order I2C instance base address in .dtsi file (Alex)
- Add commit message for the patch in stm32429i-eval.dts (Alex)
- Add link to the STM32F4 Soc ref manual where I2C device is described (Uwe)
- Use more usal way to define constants with several lines (Uwe)
- Remove rate variable from stm32f4_i2c_timings as it is not used (Uwe)
- Remove irq variable from stm32f4_i2c_dev struct are they are only needed
during probe (Uwe)
- Add comment from datasheet to explain stm32f4_i2c_timings values (Uwe)
- Rework i2c soft_reset implementation (Uwe)
- Replace "it" by "irq" as it is a more usual abbreviation for interrupt (Uwe)
- Add comment from datasheet to explain periph clk freq calculation (Uwe)
- Use DIV_ROUND_UP instead of plain division when required (Uwe)
- Add comment from datasheet to explain timing rise calculation (Uwe)
- Rework timing rise calculation by using shorter computation (Uwe)
- Remove (u8) cast when reading I2C data register (Uwe)
- Rework isr_event routine to handle several events during one call of the
routine (Uwe)
- Precise which type of irq is failed when a irq request error occurs (Uwe)
- Use devm_request_irq() instead of devm_request_threaded_irq() to avoid
spurious evt irq when clearing status registers in threaded context
M'boumba Cedric Madianga (5):
dt-bindings: Document the STM32 I2C bindings
i2c: Add STM32F4 I2C driver
ARM: dts: stm32: Add I2C1 support for STM32F429 SoC
ARM: dts: stm32: Add I2C1 support for STM32429 eval board
ARM: configs: stm32: Add I2C support for STM32 defconfig
.../devicetree/bindings/i2c/i2c-stm32.txt | 33 +
arch/arm/boot/dts/stm32429i-eval.dts | 6 +
arch/arm/boot/dts/stm32f429.dtsi | 23 +
arch/arm/configs/stm32_defconfig | 3 +
drivers/i2c/busses/Kconfig | 10 +
drivers/i2c/busses/Makefile | 1 +
drivers/i2c/busses/i2c-stm32f4.c | 896 +++++++++++++++++++++
7 files changed, 972 insertions(+)
create mode 100644 Documentation/devicetree/bindings/i2c/i2c-stm32.txt
create mode 100644 drivers/i2c/busses/i2c-stm32f4.c
--
1.9.1
--
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: [v1] i2c: busses: i2c-designware-pcidrv:- Handle return NULL error from pcim_iomap_table
From: Mika Westerberg @ 2016-12-22 11:50 UTC (permalink / raw)
To: Arvind Yadav
Cc: jarkko.nikula, wsa, andriy.shevchenko, linux-i2c, linux-kernel
In-Reply-To: <1482406759-13094-1-git-send-email-arvind.yadav.cs@gmail.com>
On Thu, Dec 22, 2016 at 05:09:19PM +0530, Arvind Yadav wrote:
> Here, If pcim_iomap_table will fail. It will return NULL.
> Kernel can run into a NULL-pointer dereference.
> This error check will avoid NULL pointer dereference.
>
> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
> ---
> drivers/i2c/busses/i2c-designware-pcidrv.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
> index d6423cf..6a1907d 100644
> --- a/drivers/i2c/busses/i2c-designware-pcidrv.c
> +++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
> @@ -235,6 +235,10 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
> dev->controller = controller;
> dev->get_clk_rate_khz = i2c_dw_get_clk_rate_khz;
> dev->base = pcim_iomap_table(pdev)[0];
> + if (!dev->base) {
> + dev_err(&pdev->dev, "I/O map table allocation failed\n");
> + return -ENOMEM;
Are you sure this can actually happen?
IIRC pcim_iomap_regions() (which is called before this) makes sure the
BARs you access here are valid.
^ permalink raw reply
* [v1] i2c: busses: i2c-designware-pcidrv:- Handle return NULL error from pcim_iomap_table
From: Arvind Yadav @ 2016-12-22 11:39 UTC (permalink / raw)
To: jarkko.nikula, wsa
Cc: andriy.shevchenko, mika.westerberg, linux-i2c, linux-kernel
Here, If pcim_iomap_table will fail. It will return NULL.
Kernel can run into a NULL-pointer dereference.
This error check will avoid NULL pointer dereference.
Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
---
drivers/i2c/busses/i2c-designware-pcidrv.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
index d6423cf..6a1907d 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -235,6 +235,10 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
dev->controller = controller;
dev->get_clk_rate_khz = i2c_dw_get_clk_rate_khz;
dev->base = pcim_iomap_table(pdev)[0];
+ if (!dev->base) {
+ dev_err(&pdev->dev, "I/O map table allocation failed\n");
+ return -ENOMEM;
+ }
dev->dev = &pdev->dev;
dev->irq = pdev->irq;
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH] i2c: uniphier[-f]: fix bool logic calculation
From: Masahiro Yamada @ 2016-12-21 15:39 UTC (permalink / raw)
To: Joe Perches
Cc: linux-i2c, Linux Kernel Mailing List, linux-arm-kernel,
Wolfram Sang
In-Reply-To: <1482256538.1984.23.camel@perches.com>
Hi Joe,
2016-12-21 2:55 GMT+09:00 Joe Perches <joe@perches.com>:
> On Wed, 2016-12-21 at 01:20 +0900, Masahiro Yamada wrote:
>> Hi.
>>
>> I have not got any comment, but does this seem
>> a right thing to do?
>
>> This code is working, but it should not depend on how "bool" is
>> typedef'ed, or the bit position of I2C_M_RD.
>
> <shrug>
>
> I think bool can be guaranteed to be _Bool.
>
> So a change not necessary as the original code
> has a c90 guarantee of the same result.
>
> 6.3.1.2 Boolean type
> 1
> When any scalar value is converted to _Bool, the result is 0 if the value compares equal
> to 0; otherwise, the result is 1.
>
Thanks for your comments!
_Bool works very nicely.
I have seen some (not very nice) projects
that define like "typedef char bool;"
So, I was wondering if I should write code
that works regardless how bool is defined.
Just my two cents.
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* Re: [PATCH] i2c: uniphier[-f]: fix bool logic calculation
From: Joe Perches @ 2016-12-20 17:55 UTC (permalink / raw)
To: Masahiro Yamada, linux-i2c
Cc: Linux Kernel Mailing List, linux-arm-kernel, Wolfram Sang
In-Reply-To: <CAK7LNATrR9dSuuUZR98f=uOZqSGqUBnUNZmrUqUZgubHHE+X4A@mail.gmail.com>
On Wed, 2016-12-21 at 01:20 +0900, Masahiro Yamada wrote:
> Hi.
>
> I have not got any comment, but does this seem
> a right thing to do?
> This code is working, but it should not depend on how "bool" is
> typedef'ed, or the bit position of I2C_M_RD.
<shrug>
I think bool can be guaranteed to be _Bool.
So a change not necessary as the original code
has a c90 guarantee of the same result.
6.3.1.2 Boolean type
1
When any scalar value is converted to _Bool, the result is 0 if the value compares equal
to 0; otherwise, the result is 1.
> 2016-10-19 13:38 GMT+09:00 Masahiro Yamada <yamada.masahiro@socionext.com>:
[]
> > diff --git a/drivers/i2c/busses/i2c-uniphier-f.c b/drivers/i2c/busses/i2c-uniphier-f.c
[]
> > @@ -309,7 +309,7 @@ static int uniphier_fi2c_master_xfer_one(struct i2c_adapter *adap,
> > struct i2c_msg *msg, bool stop)
> > {
> > struct uniphier_fi2c_priv *priv = i2c_get_adapdata(adap);
> > - bool is_read = msg->flags & I2C_M_RD;
> > + bool is_read = !!(msg->flags & I2C_M_RD);
> > unsigned long time_left;
> >
> > dev_dbg(&adap->dev, "%s: addr=0x%02x, len=%d, stop=%d\n",
^ permalink raw reply
* Re: [PATCH] i2c: uniphier[-f]: fix bool logic calculation
From: Masahiro Yamada @ 2016-12-20 16:20 UTC (permalink / raw)
To: linux-i2c
Cc: Masahiro Yamada, Linux Kernel Mailing List, linux-arm-kernel,
Wolfram Sang
In-Reply-To: <1476851911-21729-1-git-send-email-yamada.masahiro@socionext.com>
Hi.
I have not got any comment, but does this seem
a right thing to do?
2016-10-19 13:38 GMT+09:00 Masahiro Yamada <yamada.masahiro@socionext.com>:
> This code is working, but it should not depend on how "bool" is
> typedef'ed, or the bit position of I2C_M_RD.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
> drivers/i2c/busses/i2c-uniphier-f.c | 2 +-
> drivers/i2c/busses/i2c-uniphier.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-uniphier-f.c b/drivers/i2c/busses/i2c-uniphier-f.c
> index db9105e..b54448e 100644
> --- a/drivers/i2c/busses/i2c-uniphier-f.c
> +++ b/drivers/i2c/busses/i2c-uniphier-f.c
> @@ -309,7 +309,7 @@ static int uniphier_fi2c_master_xfer_one(struct i2c_adapter *adap,
> struct i2c_msg *msg, bool stop)
> {
> struct uniphier_fi2c_priv *priv = i2c_get_adapdata(adap);
> - bool is_read = msg->flags & I2C_M_RD;
> + bool is_read = !!(msg->flags & I2C_M_RD);
> unsigned long time_left;
>
> dev_dbg(&adap->dev, "%s: addr=0x%02x, len=%d, stop=%d\n",
> diff --git a/drivers/i2c/busses/i2c-uniphier.c b/drivers/i2c/busses/i2c-uniphier.c
> index 56e92af..cc80bb2 100644
> --- a/drivers/i2c/busses/i2c-uniphier.c
> +++ b/drivers/i2c/busses/i2c-uniphier.c
> @@ -177,7 +177,7 @@ static int uniphier_i2c_stop(struct i2c_adapter *adap)
> static int uniphier_i2c_master_xfer_one(struct i2c_adapter *adap,
> struct i2c_msg *msg, bool stop)
> {
> - bool is_read = msg->flags & I2C_M_RD;
> + bool is_read = !!(msg->flags & I2C_M_RD);
> bool recovery = false;
> int ret;
>
> --
> 1.9.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* Re: [PATCH] misc: eeprom: implement compatible DT probing
From: Linus Walleij @ 2016-12-20 15:00 UTC (permalink / raw)
To: Peter Rosin
Cc: Wolfram Sang, linux-i2c@vger.kernel.org,
devicetree@vger.kernel.org
In-Reply-To: <806f55f3-3fed-572d-4859-7c7dc76c5e08@axentia.se>
On Fri, Dec 9, 2016 at 6:48 AM, Peter Rosin <peda@axentia.se> wrote:
> On 2016-12-09 00:32, Linus Walleij wrote:
>> Bah I probably just screwed up syntactically and now worked around
>> a non-existing problem. I will try as you suggest, just "vendor,type"
>> and see if it works. It probably does.
(It does)
> But it is a bit strange. Grepping for compatible.*24c finds quite
> a few instances of "bad" compatible strings.
>
> Many on the patterns at,24c256 and at24,24c256 (should be probably
> be atmel,24c256) but also a few atmel,at24c16 and atmel,at24c128b.
> I don't understand how those last ones ever worked, if it is not
> working for you? Especially those with the trailing "b". WTF?
Those are the erroneous device trees that I copied as inspiration
instead of looking directly at the bindings.
Yours,
Linus Walleij
^ permalink raw reply
* [PULL REQUEST] i2c for 4.10
From: Wolfram Sang @ 2016-12-20 8:11 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-i2c, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1393 bytes --]
Linus,
seond pull request from I2C with a set of bugfixes.
Please pull.
Thanks,
Wolfram
The following changes since commit 59331c215daf600a650e281b6e8ef3e1ed1174c2:
Merge tag 'ceph-for-4.10-rc1' of git://github.com/ceph/ceph-client (2016-12-16 11:23:34 -0800)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-current
for you to fetch changes up to 649ac63a9ae5e08b7123f2fa98c2bf42f033bdb9:
i2c: mux: mlxcpld: fix i2c mux selection caching (2016-12-18 08:54:38 +0100)
----------------------------------------------------------------
Jan Glauber (1):
i2c: octeon: thunderx: Limit register access retries
Peter Rosin (1):
i2c: mux: mlxcpld: fix i2c mux selection caching
Russell King (1):
i2c: mux: pca954x: fix i2c mux selection caching
Tin Huynh (2):
i2c: xgene: Fix missing code of DTB support
i2c: designware: fix wrong Tx/Rx FIFO for ACPI
drivers/i2c/busses/i2c-designware-platdrv.c | 31 ++++++++++++++++++++++-------
drivers/i2c/busses/i2c-octeon-core.c | 4 +++-
drivers/i2c/busses/i2c-octeon-core.h | 21 ++++++++++++++-----
drivers/i2c/busses/i2c-xgene-slimpro.c | 1 +
drivers/i2c/muxes/i2c-mux-mlxcpld.c | 24 ++++++++++++----------
drivers/i2c/muxes/i2c-mux-pca954x.c | 5 ++++-
6 files changed, 61 insertions(+), 25 deletions(-)
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: interface to trigger a bus recovery sequence
From: Wolfram Sang @ 2016-12-18 23:14 UTC (permalink / raw)
To: Jorge Ramirez; +Cc: linux-i2c
In-Reply-To: <9d29f4a6-50ae-2400-4a4d-79ba7ed48a53@linaro.org>
[-- Attachment #1: Type: text/plain, Size: 399 bytes --]
> I was looking into the i2c bus recovery sequence and I was wondering why
> there doesn't seem to be an interface (ioctl/sysfs) that allows a user to
> trigger a recovery sequence.
I can't think of a reason why this should be possible. The I2C bus
driver should figure out when things go wrong and do the apropriate
actions. Also, the I2C specs have a clear definition when to use bus
recovery.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
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