* [PATCH 2/2] i2c: sh_mobile: Add per-Generation fallback bindings
From: Simon Horman @ 2016-12-07 10:39 UTC (permalink / raw)
To: Wolfram Sang
Cc: Magnus Damm, linux-i2c, linux-renesas-soc, devicetree,
Simon Horman
In-Reply-To: <1481106689-22312-1-git-send-email-horms+renesas@verge.net.au>
Add per-Generation fallback bindings for R-Car SoCs.
This is in keeping with the compatibility string scheme is being adopted
for drivers for Renesas SoCs.
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
Documentation/devicetree/bindings/i2c/i2c-sh_mobile.txt | 17 ++++++++++++++---
drivers/i2c/busses/i2c-sh_mobile.c | 2 ++
2 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/i2c/i2c-sh_mobile.txt b/Documentation/devicetree/bindings/i2c/i2c-sh_mobile.txt
index 214f94c25d37..7716acc55dec 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-sh_mobile.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-sh_mobile.txt
@@ -1,8 +1,7 @@
Device tree configuration for Renesas IIC (sh_mobile) driver
Required properties:
-- compatible : "renesas,iic-<soctype>". "renesas,rmobile-iic" as fallback
- Examples with soctypes are:
+- compatible :
- "renesas,iic-r8a73a4" (R-Mobile APE6)
- "renesas,iic-r8a7740" (R-Mobile A1)
- "renesas,iic-r8a7790" (R-Car H2)
@@ -12,6 +11,17 @@ Required properties:
- "renesas,iic-r8a7794" (R-Car E2)
- "renesas,iic-r8a7795" (R-Car H3)
- "renesas,iic-sh73a0" (SH-Mobile AG5)
+ - "renesas,rcar-gen2-iic" (generic R-Car Gen2 compatible device)
+ - "renesas,rcar-gen3-iic" (generic R-Car Gen3 compatible device)
+ - "renesas,rmobile-iic" (generic device)
+
+ When compatible with a generic R-Car version, nodes
+ must list the SoC-specific version corresponding to
+ the platform first followed by the generic R-Car
+ version.
+
+ renesas,rmobile-iic must always follow.
+
- reg : address start and address range size of device
- interrupts : interrupt of device
- clocks : clock for device
@@ -31,7 +41,8 @@ Pinctrl properties might be needed, too. See there.
Example:
iic0: i2c@e6500000 {
- compatible = "renesas,iic-r8a7790", "renesas,rmobile-iic";
+ compatible = "renesas,iic-r8a7790", "renesas,rcar-gen2-iic",
+ "renesas,rmobile-iic";
reg = <0 0xe6500000 0 0x425>;
interrupts = <0 174 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&mstp3_clks R8A7790_CLK_IIC0>;
diff --git a/drivers/i2c/busses/i2c-sh_mobile.c b/drivers/i2c/busses/i2c-sh_mobile.c
index 6335ad35902d..3d9ebe6e5716 100644
--- a/drivers/i2c/busses/i2c-sh_mobile.c
+++ b/drivers/i2c/busses/i2c-sh_mobile.c
@@ -834,7 +834,9 @@ static const struct of_device_id sh_mobile_i2c_dt_ids[] = {
{ .compatible = "renesas,iic-r8a7792", .data = &fast_clock_dt_config },
{ .compatible = "renesas,iic-r8a7793", .data = &fast_clock_dt_config },
{ .compatible = "renesas,iic-r8a7794", .data = &fast_clock_dt_config },
+ { .compatible = "renesas,rcar-gen2-iic", .data = &fast_clock_dt_config },
{ .compatible = "renesas,iic-r8a7795", .data = &fast_clock_dt_config },
+ { .compatible = "renesas,rcar-gen3-iic", .data = &fast_clock_dt_config },
{ .compatible = "renesas,iic-sh73a0", .data = &fast_clock_dt_config },
{ .compatible = "renesas,rmobile-iic", .data = &default_dt_config },
{},
--
2.7.0.rc3.207.g0ac5344
^ permalink raw reply related
* [PATCH 1/2] i2c: sh_mobile: List rmobile fallback compatibility last
From: Simon Horman @ 2016-12-07 10:39 UTC (permalink / raw)
To: Wolfram Sang
Cc: Magnus Damm, linux-i2c, linux-renesas-soc, devicetree,
Simon Horman
In-Reply-To: <1481106689-22312-1-git-send-email-horms+renesas@verge.net.au>
Improve readability by listing the rmobile fallback compatibility string
after the more-specific compatibility strings they provide a fallback for.
This does not effect run-time behaviour as it is the order in the DTB that
determines which compatibility string is used.
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
drivers/i2c/busses/i2c-sh_mobile.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-sh_mobile.c b/drivers/i2c/busses/i2c-sh_mobile.c
index 192f36f00e4d..6335ad35902d 100644
--- a/drivers/i2c/busses/i2c-sh_mobile.c
+++ b/drivers/i2c/busses/i2c-sh_mobile.c
@@ -827,7 +827,6 @@ static const struct sh_mobile_dt_config r8a7740_dt_config = {
};
static const struct of_device_id sh_mobile_i2c_dt_ids[] = {
- { .compatible = "renesas,rmobile-iic", .data = &default_dt_config },
{ .compatible = "renesas,iic-r8a73a4", .data = &fast_clock_dt_config },
{ .compatible = "renesas,iic-r8a7740", .data = &r8a7740_dt_config },
{ .compatible = "renesas,iic-r8a7790", .data = &fast_clock_dt_config },
@@ -837,6 +836,7 @@ static const struct of_device_id sh_mobile_i2c_dt_ids[] = {
{ .compatible = "renesas,iic-r8a7794", .data = &fast_clock_dt_config },
{ .compatible = "renesas,iic-r8a7795", .data = &fast_clock_dt_config },
{ .compatible = "renesas,iic-sh73a0", .data = &fast_clock_dt_config },
+ { .compatible = "renesas,rmobile-iic", .data = &default_dt_config },
{},
};
MODULE_DEVICE_TABLE(of, sh_mobile_i2c_dt_ids);
--
2.7.0.rc3.207.g0ac5344
^ permalink raw reply related
* [PATCH 0/2] i2c: sh_mobile: fallback enhancements
From: Simon Horman @ 2016-12-07 10:31 UTC (permalink / raw)
To: Wolfram Sang
Cc: Magnus Damm, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Simon Horman
Hi,
this short series aims to improve the fallback compatibility strings
for the i2c-sh_mobile driver.
Simon Horman (2):
i2c: sh_mobile: List rmobile fallback compatibility last
i2c: sh_mobile: Add per-Generation fallback bindings
Documentation/devicetree/bindings/i2c/i2c-sh_mobile.txt | 17 ++++++++++++++---
drivers/i2c/busses/i2c-sh_mobile.c | 4 +++-
2 files changed, 17 insertions(+), 4 deletions(-)
--
2.7.0.rc3.207.g0ac5344
--
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 V2] i2c: designware: fix wrong tx/rx fifo for ACPI
From: Andy Shevchenko @ 2016-12-07 10:15 UTC (permalink / raw)
To: Tin Huynh, Jarkko Nikula, Mika Westerberg, Wolfram Sang
Cc: linux-i2c, linux-kernel, linux-acpi, Loc Ho, Thang Nguyen,
Phong Vo, patches
In-Reply-To: <1481011073-29143-1-git-send-email-tnhuynh@apm.com>
On Tue, 2016-12-06 at 14:57 +0700, Tin Huynh wrote:
> ACPI always sets txfifo and rxfifo to 32. This configuration will
> cause problem if the IP core supports a fifo size of less than 32.
> The driver should read the fifo size from the IP and select the
> smaller one of the two.
>
> Signed-off-by: Tin Huynh <tnhuynh@apm.com>
>
The idea looks good, but see my comment below.
> >dev);
> + u32 param1, tx_fifo_depth, rx_fifo_depth;
> struct dw_i2c_dev *dev;
> struct i2c_adapter *adap;
> struct resource *mem;
> @@ -246,12 +247,18 @@ static int dw_i2c_plat_probe(struct
> platform_device *pdev)
> 1000000);
> }
>
> + param1 = i2c_dw_read_comp_param(dev);
> + tx_fifo_depth = ((param1 >> 16) & 0xff) + 1;
> + rx_fifo_depth = ((param1 >> 8) & 0xff) + 1;
> if (!dev->tx_fifo_depth) {
> - u32 param1 = i2c_dw_read_comp_param(dev);
> -
> - dev->tx_fifo_depth = ((param1 >> 16) & 0xff) + 1;
> - dev->rx_fifo_depth = ((param1 >> 8) & 0xff) + 1;
> + dev->tx_fifo_depth = tx_fifo_depth;
> + dev->rx_fifo_depth = rx_fifo_depth;
> dev->adapter.nr = pdev->id;
> + } else if (tx_fifo_depth) {
> + dev->tx_fifo_depth = min_t(u32, dev->tx_fifo_depth,
> + tx_fifo_depth
> );
> + dev->rx_fifo_depth = min_t(u32, dev->rx_fifo_depth,
> + rx_fifo_dept
Can we move this to a separate function like
static void dw_i2c_set_fifo_size(... *dev, ... id)
{
u32 param, tx_fifo_depth, rx_fifo_depth;
param = i2c_dw_read_comp_param(dev);
tx_fifo_depth = ((param >> 16) & 0xff) + 1;
rx_fifo_depth = ((param >> 8) & 0xff) + 1;
if (!dev->tx_fifo_depth) {
dev->tx_fifo_depth = tx_fifo_depth;
dev->rx_fifo_depth = rx_fifo_depth;
dev->adapter.nr = id;
}
dev->tx_fifo_depth = min_t(u32, dev->tx_fifo_depth,
tx_fifo_depth);
dev->rx_fifo_depth = min_t(u32, dev->rx_fifo_depth,
rx_fifo_depth);
}
?
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply
* Re: [PATCH] i2c: rk3x: keep i2c irq ON in suspend
From: David.Wu @ 2016-12-07 3:37 UTC (permalink / raw)
To: Doug Anderson
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Heiko Stuebner, Wolfram Sang,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
open list:ARM/Rockchip SoC...,
linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <CAD=FV=VgGy5hA4+7nvLs4NPNRuopak+bTW34S6X6S_vWgtnkjg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hi Doug,
在 2016/12/7 0:31, Doug Anderson 写道:
> Hi,
>
> On Tue, Dec 6, 2016 at 12:12 AM, David.Wu <david.wu@rock-chips.com> wrote:
>> Hi Heiko,
>>
>> 在 2016/12/5 18:54, Heiko Stuebner 写道:
>>>
>>> Hi David,
>>>
>>> Am Montag, 5. Dezember 2016, 16:02:59 CET schrieb David Wu:
>>>>
>>>> During suspend there may still be some i2c access happening.
>>>> And if we don't keep i2c irq ON, there may be i2c access timeout if
>>>> i2c is in irq mode of operation.
>>>
>>>
>>> can you describe the issue you're trying to fix a bit more please?
>>
>>
>> Sometimes we could see the i2c timeout errors during suspend/resume, which
>> makes the duration of suspend/resume too longer.
>>
>> [ 484.171541] CPU4: Booted secondary processor [410fd082]
>> [ 485.172777] rk3x-i2c ff3c0000.i2c: timeout, ipd: 0x10, state: 1
>> [ 486.172760] rk3x-i2c ff3c0000.i2c: timeout, ipd: 0x10, state: 1
>> [ 487.172759] rk3x-i2c ff3c0000.i2c: timeout, ipd: 0x10, state: 1
>> [ 487.172840] cpu cpu4: _set_opp_voltage: failed to set voltage (800000
>> 800000 800000 mV): -110
>> [ 487.172874] cpu cpu4: failed to set volt 800000
>>
>>>
>>> I.e. I'd think the i2c-core does suspend i2c-client devices first, so that
>>> these should be able to finish up their ongoing transfers and not start
>>> any
>>> new ones instead?
>>>
>>> Your irq can still happen slightly after the system started going to
>>> actually
>>> sleep, so to me it looks like you just widened the window where irqs can
>>> be
>>> handled. Especially as your irq could also just simply stem from the start
>>> state, so you cannot even be sure if your transaction actually is
>>> finished.
>>
>>
>> Okay, you are right. I want to give it a double insurance at first, but it
>> may hide the unhappend issue.
>>
>>>
>>> So to me it looks like the i2c-connected device driver should be fixed
>>> instead?
>>
>>
>> I tell them to fix it in rk808 driver.
>
> To me it seems like perhaps cpufreq should not be changing frequencies
> until it is resumed properly. Presumably if all the ordering is done
> right then cpufreq should be resumed _after_ the i2c regulator so you
> should be OK. ...or am I somehow confused about that?
yes，the cpufreq and regulator should start i2c job after they resume
properly.
>
> Also note that previous i2c busses I worked with simply returned -EIO
> in the case where they were called when suspended. See
> "i2c-exynos5.c" and "i2c-s3c2410.c".
In "i2c-exynos5.c", it seems that using the "i2c->suspended" to protect
i2c transfer works most of the time. Of course it could prevent the next
new i2c transfer to start. But in one case, if the current i2c job was
not finished until the i2c irq was disabled by system suspend, the i2c
timeout error would also happen, as the current i2c job may have a large
data to transfer and it lasts from a long time.
So is it necessary to add a mutex lock to wait the current job to be
finished before the "i2c->suspended" is changed in i2c_suspend_noirq()?
However, the i2c_suspend_noirq() is called after suspend_device_irqs()
in system suspend, it means that the i2c timeout errors could happen
during the time between suspend_device_irqs() and i2c_suspend_noirq(),
if there were i2c transfers started.
>
> -Doug
>
>
>
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply
* Re: [Patch V1] i2c: fsl-lpi2c: read lpi2c fifo size in probe()
From: Vladimir Zapolskiy @ 2016-12-07 1:01 UTC (permalink / raw)
To: Gao Pan, wsa, wsa-dev, cmo, robh; +Cc: linux-i2c, frank.li, fugang.duan
In-Reply-To: <1480649896-5222-1-git-send-email-pandy.gao@nxp.com>
On 12/02/2016 05:38 AM, Gao Pan wrote:
> The lpi2c fifo size is a read only parameter resides Parameter
> Register. It's better to read lpi2c tx/rx fifo size in probe()
> other than just define a macro for it.
>
> Signed-off-by: Gao Pan <pandy.gao@nxp.com>
> ---
Reviewed-by: Vladimir Zapolskiy <vz@mleia.com>
--
With best wishes,
Vladimir
^ permalink raw reply
* Re: [PATCH v5 1/2] i2c: aspeed: added driver for Aspeed I2C
From: Vladimir Zapolskiy @ 2016-12-07 0:57 UTC (permalink / raw)
To: Brendan Higgins, wsa-z923LK4zBo2bacvFa/9K2g, clg-Bxea+6Xhats,
mouse-Pma6HLj0uuo, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8
Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, joel-U3u1mxZcP9KHXe+LvDLADg,
openbmc-uLR06cmDAlY/bJ5BZ2RsiQ
In-Reply-To: <1480467618-7497-2-git-send-email-brendanhiggins-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
Hello Brendan,
please find review comments below.
After reviewing device tree binding now I'm confident that you should split
the file into two and then send interrupt controller driver for a separate
review by IRQCHIP maintainers. The interrupt controller driver will be
quite simple (as almost all of them), but it deserves its own review
and it should be placed under drivers/irqchip.
On 11/30/2016 03:00 AM, Brendan Higgins wrote:
> Added initial master and slave support for Aspeed I2C controller.
> Supports fourteen busses present in ast24xx and ast25xx BMC SoCs by
> Aspeed.
>
> Signed-off-by: Brendan Higgins <brendanhiggins-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> ---
> Changes for v2:
> - Added single module_init (multiple was breaking some builds).
> Changes for v3:
> - Removed "bus" device tree param; now extracted from bus address offset
> Changes for v4:
> - I2C adapter number is now generated dynamically unless specified in alias.
> Changes for v5:
> - Removed irq_chip used to multiplex IRQ and replaced it with dummy_irq_chip
> along with some other IRQ cleanup.
> - Addressed comments from Cedric, and Vladimir, mostly stylistic things and
> using devm managed resources.
> - Increased max clock frequency before the bus is put in HighSpeed mode, as
> per Kachalov's comment.
> ---
> drivers/i2c/busses/Kconfig | 10 +
> drivers/i2c/busses/Makefile | 1 +
> drivers/i2c/busses/i2c-aspeed.c | 839 ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 850 insertions(+)
> create mode 100644 drivers/i2c/busses/i2c-aspeed.c
>
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index d252276..e8cf750 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -325,6 +325,16 @@ config I2C_POWERMAC
>
> comment "I2C system bus drivers (mostly embedded / system-on-chip)"
>
> +config I2C_ASPEED
> + tristate "Aspeed AST2xxx SoC I2C Controller"
> + depends on ARCH_ASPEED
> + help
> + If you say yes to this option, support will be included for the
> + Aspeed AST2xxx SoC I2C controller.
> +
> + This driver can also be built as a module. If so, the module
> + will be called i2c-aspeed.
> +
> config I2C_AT91
> tristate "Atmel AT91 I2C Two-Wire interface (TWI)"
> depends on ARCH_AT91
> diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
> index 29764cc..73fec22 100644
> --- a/drivers/i2c/busses/Makefile
> +++ b/drivers/i2c/busses/Makefile
> @@ -29,6 +29,7 @@ obj-$(CONFIG_I2C_HYDRA) += i2c-hydra.o
> obj-$(CONFIG_I2C_POWERMAC) += i2c-powermac.o
>
> # Embedded system I2C/SMBus host controller drivers
> +obj-$(CONFIG_I2C_ASPEED) += i2c-aspeed.o
> obj-$(CONFIG_I2C_AT91) += i2c-at91.o
> obj-$(CONFIG_I2C_AU1550) += i2c-au1550.o
> obj-$(CONFIG_I2C_AXXIA) += i2c-axxia.o
> diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
> new file mode 100644
> index 0000000..0e68808
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-aspeed.c
> @@ -0,0 +1,839 @@
> +/*
> + * I2C adapter for the ASPEED I2C bus.
> + *
> + * Copyright (C) 2012-2016 ASPEED Technology Inc.
> + * Copyright 2016 IBM Corporation
> + * Copyright 2016 Google, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/completion.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/i2c.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/irq.h>
> +#include <linux/irqchip/chained_irq.h>
> +#include <linux/irqdomain.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +/* I2C Register */
> +#define ASPEED_I2C_FUN_CTRL_REG 0x00
> +#define ASPEED_I2C_AC_TIMING_REG1 0x04
> +#define ASPEED_I2C_AC_TIMING_REG2 0x08
> +#define ASPEED_I2C_INTR_CTRL_REG 0x0c
> +#define ASPEED_I2C_INTR_STS_REG 0x10
> +#define ASPEED_I2C_CMD_REG 0x14
> +#define ASPEED_I2C_DEV_ADDR_REG 0x18
> +#define ASPEED_I2C_BYTE_BUF_REG 0x20
> +
> +#define ASPEED_I2C_NUM_BUS 14
> +
> +/* Global Register Definition */
> +/* 0x00 : I2C Interrupt Status Register */
> +/* 0x08 : I2C Interrupt Target Assignment */
> +
> +/* Device Register Definition */
> +/* 0x00 : I2CD Function Control Register */
> +#define ASPEED_I2CD_MULTI_MASTER_DIS BIT(15)
> +#define ASPEED_I2CD_SDA_DRIVE_1T_EN BIT(8)
> +#define ASPEED_I2CD_M_SDA_DRIVE_1T_EN BIT(7)
> +#define ASPEED_I2CD_M_HIGH_SPEED_EN BIT(6)
> +#define ASPEED_I2CD_SLAVE_EN BIT(1)
> +#define ASPEED_I2CD_MASTER_EN BIT(0)
> +
> +/* 0x08 : I2CD Clock and AC Timing Control Register #2 */
> +#define ASPEED_NO_TIMEOUT_CTRL 0
> +
> +
checkpatch complains here:
CHECK: Please don't use multiple blank lines
#60: FILE: drivers/i2c/busses/i2c-aspeed.c:60:
> +/* 0x0c : I2CD Interrupt Control Register &
> + * 0x10 : I2CD Interrupt Status Register
> + *
> + * These share bit definitions, so use the same values for the enable &
> + * status bits.
> + */
> +#define ASPEED_I2CD_INTR_SDA_DL_TIMEOUT BIT(14)
> +#define ASPEED_I2CD_INTR_BUS_RECOVER_DONE BIT(13)
> +#define ASPEED_I2CD_INTR_SLAVE_MATCH BIT(7)
> +#define ASPEED_I2CD_INTR_SCL_TIMEOUT BIT(6)
> +#define ASPEED_I2CD_INTR_ABNORMAL BIT(5)
> +#define ASPEED_I2CD_INTR_NORMAL_STOP BIT(4)
> +#define ASPEED_I2CD_INTR_ARBIT_LOSS BIT(3)
> +#define ASPEED_I2CD_INTR_RX_DONE BIT(2)
> +#define ASPEED_I2CD_INTR_TX_NAK BIT(1)
> +#define ASPEED_I2CD_INTR_TX_ACK BIT(0)
> +#define ASPEED_I2CD_INTR_ERROR \
> + (ASPEED_I2CD_INTR_ARBIT_LOSS | \
> + ASPEED_I2CD_INTR_ABNORMAL | \
> + ASPEED_I2CD_INTR_SCL_TIMEOUT | \
> + ASPEED_I2CD_INTR_SDA_DL_TIMEOUT | \
> + ASPEED_I2CD_INTR_TX_NAK)
> +#define ASPEED_I2CD_INTR_ALL \
> + (ASPEED_I2CD_INTR_SDA_DL_TIMEOUT | \
> + ASPEED_I2CD_INTR_BUS_RECOVER_DONE | \
> + ASPEED_I2CD_INTR_SCL_TIMEOUT | \
> + ASPEED_I2CD_INTR_ABNORMAL | \
> + ASPEED_I2CD_INTR_NORMAL_STOP | \
> + ASPEED_I2CD_INTR_ARBIT_LOSS | \
> + ASPEED_I2CD_INTR_RX_DONE | \
> + ASPEED_I2CD_INTR_TX_NAK | \
> + ASPEED_I2CD_INTR_TX_ACK)
> +
> +/* 0x14 : I2CD Command/Status Register */
> +#define ASPEED_I2CD_SCL_LINE_STS BIT(18)
> +#define ASPEED_I2CD_SDA_LINE_STS BIT(17)
> +#define ASPEED_I2CD_BUS_BUSY_STS BIT(16)
> +#define ASPEED_I2CD_BUS_RECOVER_CMD BIT(11)
> +
> +/* Command Bit */
> +#define ASPEED_I2CD_M_STOP_CMD BIT(5)
> +#define ASPEED_I2CD_M_S_RX_CMD_LAST BIT(4)
> +#define ASPEED_I2CD_M_RX_CMD BIT(3)
> +#define ASPEED_I2CD_S_TX_CMD BIT(2)
> +#define ASPEED_I2CD_M_TX_CMD BIT(1)
> +#define ASPEED_I2CD_M_START_CMD BIT(0)
> +
> +/* 0x18 : I2CD Slave Device Address Register */
> +#define ASPEED_I2CD_DEV_ADDR_MASK GENMASK(6, 0)
> +
> +enum aspeed_i2c_slave_state {
> + ASPEED_I2C_SLAVE_START,
> + ASPEED_I2C_SLAVE_READ_REQUESTED,
> + ASPEED_I2C_SLAVE_READ_PROCESSED,
> + ASPEED_I2C_SLAVE_WRITE_REQUESTED,
> + ASPEED_I2C_SLAVE_WRITE_RECEIVED,
> + ASPEED_I2C_SLAVE_STOP,
> +};
> +
> +struct aspeed_i2c_bus {
> + struct i2c_adapter adap;
> + struct device *dev;
> + void __iomem *base;
> + spinlock_t lock;
checkpatch complains here:
CHECK: spinlock_t definition without comment
#124: FILE: drivers/i2c/busses/i2c-aspeed.c:124:
> + struct completion cmd_complete;
> + int irq;
> + /* Transaction state. */
> + struct i2c_msg *msg;
> + int msg_pos;
> + u32 cmd_err;
> +#if IS_ENABLED(CONFIG_I2C_SLAVE)
> + struct i2c_client *slave;
> + enum aspeed_i2c_slave_state slave_state;
> +#endif
> +};
> +
> +struct aspeed_i2c_controller {
> + struct device *dev;
> + void __iomem *base;
> + int irq;
> + struct irq_domain *irq_domain;
> +};
> +
> +static inline void aspeed_i2c_write(struct aspeed_i2c_bus *bus, u32 val,
> + u32 reg)
> +{
> + writel(val, bus->base + reg);
> +}
> +
> +static inline u32 aspeed_i2c_read(struct aspeed_i2c_bus *bus, u32 reg)
> +{
> + return readl(bus->base + reg);
> +}
> +
> +static int aspeed_i2c_recover_bus(struct aspeed_i2c_bus *bus)
> +{
> + unsigned long time_left, flags;
> + int ret = 0;
> + u32 command;
> +
> + spin_lock_irqsave(&bus->lock, flags);
> + command = aspeed_i2c_read(bus, ASPEED_I2C_CMD_REG);
> +
> + if (command & ASPEED_I2CD_SDA_LINE_STS) {
> + /* Bus is idle: no recovery needed. */
> + if (command & ASPEED_I2CD_SCL_LINE_STS)
> + goto out;
> + dev_dbg(bus->dev, "bus hung (state %x), attempting recovery\n",
> + command);
> +
> + aspeed_i2c_write(bus, ASPEED_I2CD_M_STOP_CMD,
> + ASPEED_I2C_CMD_REG);
> + reinit_completion(&bus->cmd_complete);
> + spin_unlock_irqrestore(&bus->lock, flags);
> +
> + time_left = wait_for_completion_timeout(
> + &bus->cmd_complete, bus->adap.timeout);
> +
> + spin_lock_irqsave(&bus->lock, flags);
> + if (time_left == 0)
> + ret = -ETIMEDOUT;
> + else if (bus->cmd_err)
> + ret = -EIO;
> + /* Bus error. */
> + } else {
> + dev_dbg(bus->dev, "bus hung (state %x), attempting recovery\n",
> + command);
> +
> + aspeed_i2c_write(bus, ASPEED_I2CD_BUS_RECOVER_CMD,
> + ASPEED_I2C_CMD_REG);
> + reinit_completion(&bus->cmd_complete);
> + spin_unlock_irqrestore(&bus->lock, flags);
> +
> + time_left = wait_for_completion_timeout(
> + &bus->cmd_complete, bus->adap.timeout);
> +
> + spin_lock_irqsave(&bus->lock, flags);
> + if (time_left == 0)
> + ret = -ETIMEDOUT;
> + else if (bus->cmd_err)
> + ret = -EIO;
> + /* Recovery failed. */
> + else if (!(aspeed_i2c_read(bus, ASPEED_I2C_CMD_REG) &
> + ASPEED_I2CD_SDA_LINE_STS))
> + ret = -EIO;
> + }
> +
> +out:
> + spin_unlock_irqrestore(&bus->lock, flags);
> +
> + return ret;
> +}
> +
> +#if IS_ENABLED(CONFIG_I2C_SLAVE)
> +static bool aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus)
> +{
> + u32 command, irq_status, status_ack = 0;
> + struct i2c_client *slave = bus->slave;
> + bool irq_handled = true;
> + u8 value;
> +
> + spin_lock(&bus->lock);
> + if (!slave) {
> + irq_handled = false;
> + goto out;
> + }
> +
> + command = aspeed_i2c_read(bus, ASPEED_I2C_CMD_REG);
> + irq_status = aspeed_i2c_read(bus, ASPEED_I2C_INTR_STS_REG);
> +
> + /* Slave was requested, restart state machine. */
> + if (irq_status & ASPEED_I2CD_INTR_SLAVE_MATCH) {
> + status_ack |= ASPEED_I2CD_INTR_SLAVE_MATCH;
> + bus->slave_state = ASPEED_I2C_SLAVE_START;
> + }
> +
> + /* Slave is not currently active, irq was for someone else. */
> + if (bus->slave_state == ASPEED_I2C_SLAVE_STOP) {
> + irq_handled = false;
> + goto out;
> + }
> +
> + dev_dbg(bus->dev, "slave irq status 0x%08x, cmd 0x%08x\n",
> + irq_status, command);
> +
> + /* Slave was sent something. */
> + if (irq_status & ASPEED_I2CD_INTR_RX_DONE) {
> + value = aspeed_i2c_read(bus, ASPEED_I2C_BYTE_BUF_REG) >> 8;
> + /* Handle address frame. */
> + if (bus->slave_state == ASPEED_I2C_SLAVE_START) {
> + if (value & 0x1)
> + bus->slave_state =
> + ASPEED_I2C_SLAVE_READ_REQUESTED;
> + else
> + bus->slave_state =
> + ASPEED_I2C_SLAVE_WRITE_REQUESTED;
> + }
> + status_ack |= ASPEED_I2CD_INTR_RX_DONE;
> + }
> +
> + /* Slave was asked to stop. */
> + if (irq_status & ASPEED_I2CD_INTR_NORMAL_STOP) {
> + status_ack |= ASPEED_I2CD_INTR_NORMAL_STOP;
> + bus->slave_state = ASPEED_I2C_SLAVE_STOP;
> + }
> + if (irq_status & ASPEED_I2CD_INTR_TX_NAK) {
> + status_ack |= ASPEED_I2CD_INTR_TX_NAK;
> + bus->slave_state = ASPEED_I2C_SLAVE_STOP;
> + }
> +
> + switch (bus->slave_state) {
> + case ASPEED_I2C_SLAVE_READ_REQUESTED:
> + if (irq_status & ASPEED_I2CD_INTR_TX_ACK)
> + dev_err(bus->dev, "Unexpected ACK on read request.\n");
> + bus->slave_state = ASPEED_I2C_SLAVE_READ_PROCESSED;
> +
> + i2c_slave_event(slave, I2C_SLAVE_READ_REQUESTED, &value);
> + aspeed_i2c_write(bus, value, ASPEED_I2C_BYTE_BUF_REG);
> + aspeed_i2c_write(bus, ASPEED_I2CD_S_TX_CMD, ASPEED_I2C_CMD_REG);
> + break;
> + case ASPEED_I2C_SLAVE_READ_PROCESSED:
> + status_ack |= ASPEED_I2CD_INTR_TX_ACK;
> + if (!(irq_status & ASPEED_I2CD_INTR_TX_ACK))
> + dev_err(bus->dev,
> + "Expected ACK after processed read.\n");
> + i2c_slave_event(slave, I2C_SLAVE_READ_PROCESSED, &value);
> + aspeed_i2c_write(bus, value, ASPEED_I2C_BYTE_BUF_REG);
> + aspeed_i2c_write(bus, ASPEED_I2CD_S_TX_CMD, ASPEED_I2C_CMD_REG);
> + break;
> + case ASPEED_I2C_SLAVE_WRITE_REQUESTED:
> + bus->slave_state = ASPEED_I2C_SLAVE_WRITE_RECEIVED;
> + i2c_slave_event(slave, I2C_SLAVE_WRITE_REQUESTED, &value);
> + break;
> + case ASPEED_I2C_SLAVE_WRITE_RECEIVED:
> + i2c_slave_event(slave, I2C_SLAVE_WRITE_RECEIVED, &value);
> + break;
> + case ASPEED_I2C_SLAVE_STOP:
> + i2c_slave_event(slave, I2C_SLAVE_STOP, &value);
> + break;
> + default:
> + dev_err(bus->dev, "unhandled slave_state: %d\n",
> + bus->slave_state);
> + break;
> + }
> +
> + if (status_ack != irq_status)
> + dev_err(bus->dev,
> + "irq handled != irq. expected %x, but was %x\n",
> + irq_status, status_ack);
> + aspeed_i2c_write(bus, status_ack, ASPEED_I2C_INTR_STS_REG);
> +
> +out:
> + spin_unlock(&bus->lock);
> + return irq_handled;
> +}
> +#endif
> +
> +static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
> +{
> + u32 irq_status;
> +
> + spin_lock(&bus->lock);
> + irq_status = aspeed_i2c_read(bus, ASPEED_I2C_INTR_STS_REG);
> + bus->cmd_err = irq_status & ASPEED_I2CD_INTR_ERROR;
> +
> + dev_dbg(bus->dev, "master irq status 0x%08x\n", irq_status);
> +
> + /* No message to transfer. */
> + if (bus->cmd_err ||
> + (irq_status & ASPEED_I2CD_INTR_NORMAL_STOP) ||
> + (irq_status & ASPEED_I2CD_INTR_BUS_RECOVER_DONE)) {
> + complete(&bus->cmd_complete);
> + goto out;
> + } else if (!bus->msg || bus->msg_pos >= bus->msg->len)
> + goto out;
checkpatch complains here:
CHECK: braces {} should be used on all arms of this statement
#329: FILE: drivers/i2c/busses/i2c-aspeed.c:329:
> +
> + if ((bus->msg->flags & I2C_M_RD) &&
> + (irq_status & ASPEED_I2CD_INTR_RX_DONE)) {
> + bus->msg->buf[bus->msg_pos++] = aspeed_i2c_read(
> + bus, ASPEED_I2C_BYTE_BUF_REG) >> 8;
> + if (bus->msg_pos + 1 < bus->msg->len)
> + aspeed_i2c_write(bus, ASPEED_I2CD_M_RX_CMD,
> + ASPEED_I2C_CMD_REG);
> + else if (bus->msg_pos < bus->msg->len)
> + aspeed_i2c_write(bus, ASPEED_I2CD_M_RX_CMD |
> + ASPEED_I2CD_M_S_RX_CMD_LAST,
> + ASPEED_I2C_CMD_REG);
> + } else if (!(bus->msg->flags & I2C_M_RD) &&
> + (irq_status & ASPEED_I2CD_INTR_TX_ACK)) {
> + aspeed_i2c_write(bus, bus->msg->buf[bus->msg_pos++],
> + ASPEED_I2C_BYTE_BUF_REG);
checkpatch complains here:
CHECK: Alignment should match open parenthesis
#351: FILE: drivers/i2c/busses/i2c-aspeed.c:351:
> + aspeed_i2c_write(bus, ASPEED_I2CD_M_TX_CMD, ASPEED_I2C_CMD_REG);
> + }
> +
> + /* Transmission complete: notify caller. */
> + if (bus->msg_pos >= bus->msg->len)
> + complete(&bus->cmd_complete);
> +out:
> + aspeed_i2c_write(bus, irq_status, ASPEED_I2C_INTR_STS_REG);
> + spin_unlock(&bus->lock);
> +
> + return true;
> +}
> +
> +static irqreturn_t aspeed_i2c_bus_irq(int irq, void *dev_id)
> +{
> + struct aspeed_i2c_bus *bus = dev_id;
> +
> +#if IS_ENABLED(CONFIG_I2C_SLAVE)
> + if (aspeed_i2c_slave_irq(bus)) {
> + dev_dbg(bus->dev, "irq handled by slave.\n");
> + return IRQ_HANDLED;
> + }
> +#endif
> +
> + if (aspeed_i2c_master_irq(bus)) {
> + dev_dbg(bus->dev, "irq handled by master.\n");
> + return IRQ_HANDLED;
> + }
aspeed_i2c_master_irq() always return 'true', this means that
without functional change you can move dev_dbg() into
aspeed_i2c_master_irq() function, and then remove the if-check
for return value.
> +
> + dev_err(bus->dev, "irq not handled properly!\n");
> + return IRQ_NONE;
This is dead code, if functional changes in aspeed_i2c_master_irq()
are not done, please remove it.
> +}
> +
> +static int aspeed_i2c_master_single_xfer(struct i2c_adapter *adap,
> + struct i2c_msg *msg)
checkpatch complains here:
CHECK: Alignment should match open parenthesis
#386: FILE: drivers/i2c/busses/i2c-aspeed.c:386:
> +{
> + u32 command = ASPEED_I2CD_M_START_CMD | ASPEED_I2CD_M_TX_CMD;
> + struct aspeed_i2c_bus *bus = adap->algo_data;
> + unsigned long time_left, flags;
> + int ret = msg->len;
> + u8 slave_addr;
> +
> + spin_lock_irqsave(&bus->lock, flags);
> + bus->msg = msg;
> + bus->msg_pos = 0;
> + slave_addr = msg->addr << 1;
> +
> + if (msg->flags & I2C_M_RD) {
> + slave_addr |= 1;
> + command |= ASPEED_I2CD_M_RX_CMD;
> + if (msg->len == 1)
> + command |= ASPEED_I2CD_M_S_RX_CMD_LAST;
> + }
> +
> + aspeed_i2c_write(bus, slave_addr, ASPEED_I2C_BYTE_BUF_REG);
> + aspeed_i2c_write(bus, command, ASPEED_I2C_CMD_REG);
> + reinit_completion(&bus->cmd_complete);
> + spin_unlock_irqrestore(&bus->lock, flags);
> +
> + time_left = wait_for_completion_timeout(
> + &bus->cmd_complete, bus->adap.timeout);
> + if (time_left == 0)
> + return -ETIMEDOUT;
> +
> + spin_lock_irqsave(&bus->lock, flags);
> + if (bus->cmd_err)
> + ret = -EIO;
> +
> + bus->msg = NULL;
> + spin_unlock_irqrestore(&bus->lock, flags);
> +
> + return ret;
> +}
> +
> +static int aspeed_i2c_master_xfer(struct i2c_adapter *adap,
> + struct i2c_msg *msgs, int num)
> +{
> + struct aspeed_i2c_bus *bus = adap->algo_data;
> + unsigned long time_left, flags;
> + int ret, i;
> +
> + /* If bus is busy, attempt recovery. We assume a single master
> + * environment.
> + */
> + if (aspeed_i2c_read(bus, ASPEED_I2C_CMD_REG) &
> + ASPEED_I2CD_BUS_BUSY_STS) {
> + ret = aspeed_i2c_recover_bus(bus);
> + if (ret)
> + return ret;
> + }
> +
> + for (i = 0; i < num; i++) {
> + ret = aspeed_i2c_master_single_xfer(adap, &msgs[i]);
> + if (ret < 0)
> + break;
Please insert an empty line here to improve readability.
> + /* TODO: Support other forms of I2C protocol mangling. */
> + if (msgs[i].flags & I2C_M_STOP) {
> + spin_lock_irqsave(&bus->lock, flags);
> + aspeed_i2c_write(bus, ASPEED_I2CD_M_STOP_CMD,
> + ASPEED_I2C_CMD_REG);
checkpatch complains here:
CHECK: Alignment should match open parenthesis
#451: FILE: drivers/i2c/busses/i2c-aspeed.c:451:
> + reinit_completion(&bus->cmd_complete);
> + spin_unlock_irqrestore(&bus->lock, flags);
> +
> + time_left = wait_for_completion_timeout(
> + &bus->cmd_complete, bus->adap.timeout);
> + if (time_left == 0)
> + return -ETIMEDOUT;
> + }
> + }
> +
> + spin_lock_irqsave(&bus->lock, flags);
> + aspeed_i2c_write(bus, ASPEED_I2CD_M_STOP_CMD, ASPEED_I2C_CMD_REG);
> + reinit_completion(&bus->cmd_complete);
> + spin_unlock_irqrestore(&bus->lock, flags);
> +
> + time_left = wait_for_completion_timeout(
> + &bus->cmd_complete, bus->adap.timeout);
> + if (time_left == 0)
> + return -ETIMEDOUT;
> +
> + /* If nothing went wrong, return number of messages transferred. */
> + if (ret < 0)
> + return ret;
> + else
> + return i;
> +}
> +
> +static u32 aspeed_i2c_functionality(struct i2c_adapter *adap)
> +{
> + return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_SMBUS_BLOCK_DATA;
> +}
> +
> +#if IS_ENABLED(CONFIG_I2C_SLAVE)
> +static int aspeed_i2c_reg_slave(struct i2c_client *client)
> +{
> + u32 addr_reg_val, func_ctrl_reg_val;
> + struct aspeed_i2c_bus *bus;
> + unsigned long flags;
> +
> + bus = client->adapter->algo_data;
> + spin_lock_irqsave(&bus->lock, flags);
> + if (bus->slave) {
> + spin_unlock_irqrestore(&bus->lock, flags);
> + return -EINVAL;
> + }
> +
> + /* Set slave addr. */
> + addr_reg_val = aspeed_i2c_read(bus, ASPEED_I2C_DEV_ADDR_REG);
> + addr_reg_val &= ~ASPEED_I2CD_DEV_ADDR_MASK;
> + addr_reg_val |= client->addr & ASPEED_I2CD_DEV_ADDR_MASK;
> + aspeed_i2c_write(bus, addr_reg_val, ASPEED_I2C_DEV_ADDR_REG);
> +
> + /* Switch from master mode to slave mode. */
> + func_ctrl_reg_val = aspeed_i2c_read(bus, ASPEED_I2C_FUN_CTRL_REG);
> + func_ctrl_reg_val &= ~ASPEED_I2CD_MASTER_EN;
> + func_ctrl_reg_val |= ASPEED_I2CD_SLAVE_EN;
> + aspeed_i2c_write(bus, func_ctrl_reg_val, ASPEED_I2C_FUN_CTRL_REG);
> +
> + bus->slave = client;
> + bus->slave_state = ASPEED_I2C_SLAVE_STOP;
> + spin_unlock_irqrestore(&bus->lock, flags);
> +
> + return 0;
> +}
> +
> +static int aspeed_i2c_unreg_slave(struct i2c_client *client)
> +{
> + struct aspeed_i2c_bus *bus = client->adapter->algo_data;
> + u32 func_ctrl_reg_val;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&bus->lock, flags);
> + if (!bus->slave) {
> + spin_unlock_irqrestore(&bus->lock, flags);
> + return -EINVAL;
> + }
> +
> + /* Switch from slave mode to master mode. */
> + func_ctrl_reg_val = aspeed_i2c_read(bus, ASPEED_I2C_FUN_CTRL_REG);
> + func_ctrl_reg_val &= ~ASPEED_I2CD_SLAVE_EN;
> + func_ctrl_reg_val |= ASPEED_I2CD_MASTER_EN;
> + aspeed_i2c_write(bus, func_ctrl_reg_val, ASPEED_I2C_FUN_CTRL_REG);
> +
> + bus->slave = NULL;
> + spin_unlock_irqrestore(&bus->lock, flags);
> +
> + return 0;
> +}
> +#endif
> +
> +static const struct i2c_algorithm aspeed_i2c_algo = {
> + .master_xfer = aspeed_i2c_master_xfer,
> + .functionality = aspeed_i2c_functionality,
> +#if IS_ENABLED(CONFIG_I2C_SLAVE)
> + .reg_slave = aspeed_i2c_reg_slave,
> + .unreg_slave = aspeed_i2c_unreg_slave,
> +#endif
> +};
> +
> +static u32 aspeed_i2c_get_clk_reg_val(u32 divider_ratio)
> +{
> + u32 scl_low, scl_high, data;
> + unsigned int inc = 0, div;
> +
> + for (div = 0; divider_ratio >= 16; div++) {
> + inc |= (divider_ratio & 1);
> + divider_ratio >>= 1;
> + }
> +
> + divider_ratio += inc;
> + scl_low = (divider_ratio >> 1) - 1;
> + scl_high = divider_ratio - scl_low - 2;
> + data = 0x77700300 | (scl_high << 16) | (scl_low << 12) | div;
> + return data;
> +}
> +
> +static int aspeed_i2c_init_clk(struct aspeed_i2c_bus *bus,
> + struct platform_device *pdev)
checkpatch complains here:
CHECK: Alignment should match open parenthesis
#569: FILE: drivers/i2c/busses/i2c-aspeed.c:569:
> +{
> + u32 clk_freq, divider_ratio;
> + struct clk *pclk;
> + int ret;
> +
> + pclk = devm_clk_get(&pdev->dev, NULL);
> + if (IS_ERR(pclk)) {
> + dev_err(&pdev->dev, "clk_get failed\n");
> + return PTR_ERR(pclk);
> + }
> + ret = of_property_read_u32(pdev->dev.of_node,
> + "clock-frequency", &clk_freq);
checkpatch complains here:
CHECK: Alignment should match open parenthesis
#581: FILE: drivers/i2c/busses/i2c-aspeed.c:581:
> + if (ret < 0) {
> + dev_err(&pdev->dev,
> + "Could not read clock-frequency property\n");
checkpatch complains here:
CHECK: Alignment should match open parenthesis
#584: FILE: drivers/i2c/busses/i2c-aspeed.c:584:
> + clk_freq = 100000;
> + }
> + divider_ratio = clk_get_rate(pclk) / clk_freq;
> + /* We just need the clock rate, we don't actually use the clk object. */
> + devm_clk_put(&pdev->dev, pclk);
> +
> + /* Set AC Timing */
> + if (clk_freq / 1000 > 1000) {
> + aspeed_i2c_write(bus, aspeed_i2c_read(bus,
> + ASPEED_I2C_FUN_CTRL_REG) |
> + ASPEED_I2CD_M_HIGH_SPEED_EN |
> + ASPEED_I2CD_M_SDA_DRIVE_1T_EN |
> + ASPEED_I2CD_SDA_DRIVE_1T_EN,
> + ASPEED_I2C_FUN_CTRL_REG);
> +
> + aspeed_i2c_write(bus, 0x3, ASPEED_I2C_AC_TIMING_REG2);
> + aspeed_i2c_write(bus, aspeed_i2c_get_clk_reg_val(divider_ratio),
> + ASPEED_I2C_AC_TIMING_REG1);
checkpatch complains here:
CHECK: Alignment should match open parenthesis
#602: FILE: drivers/i2c/busses/i2c-aspeed.c:602:
> + } else {
> + aspeed_i2c_write(bus, aspeed_i2c_get_clk_reg_val(divider_ratio),
> + ASPEED_I2C_AC_TIMING_REG1);
checkpatch complains here:
CHECK: Alignment should match open parenthesis
#605: FILE: drivers/i2c/busses/i2c-aspeed.c:605:
> + aspeed_i2c_write(bus, ASPEED_NO_TIMEOUT_CTRL,
> + ASPEED_I2C_AC_TIMING_REG2);
> + }
> +
> + return 0;
> +}
> +
> +static int aspeed_i2c_probe_bus(struct platform_device *pdev)
> +{
> + struct aspeed_i2c_bus *bus;
> + struct resource *res;
> + int ret;
> +
> + bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
> + if (!bus)
> + return -ENOMEM;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + bus->base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(bus->base))
> + return PTR_ERR(bus->base);
> +
> + bus->irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
> + ret = devm_request_irq(&pdev->dev, bus->irq, aspeed_i2c_bus_irq,
> + IRQF_SHARED, dev_name(&pdev->dev), bus);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "failed to request interrupt\n");
> + return ret;
> + }
> +
> + /* Initialize the I2C adapter */
> + spin_lock_init(&bus->lock);
> + init_completion(&bus->cmd_complete);
> + bus->adap.owner = THIS_MODULE;
> + bus->adap.retries = 0;
> + bus->adap.timeout = 5 * HZ;
> + bus->adap.algo = &aspeed_i2c_algo;
> + bus->adap.algo_data = bus;
> + bus->adap.dev.parent = &pdev->dev;
> + bus->adap.dev.of_node = pdev->dev.of_node;
> + snprintf(bus->adap.name, sizeof(bus->adap.name), "Aspeed i2c");
> +
> + bus->dev = &pdev->dev;
> +
> + /* reset device: disable master & slave functions */
> + aspeed_i2c_write(bus, 0, ASPEED_I2C_FUN_CTRL_REG);
> +
> + ret = aspeed_i2c_init_clk(bus, pdev);
> + if (ret < 0)
> + return ret;
> +
> + /* Enable Master Mode */
> + aspeed_i2c_write(bus, aspeed_i2c_read(bus, ASPEED_I2C_FUN_CTRL_REG) |
> + ASPEED_I2CD_MASTER_EN |
> + ASPEED_I2CD_MULTI_MASTER_DIS, ASPEED_I2C_FUN_CTRL_REG);
> +
> + /* Set interrupt generation of I2C controller */
> + aspeed_i2c_write(bus, ASPEED_I2CD_INTR_ALL, ASPEED_I2C_INTR_CTRL_REG);
> +
> + ret = i2c_add_adapter(&bus->adap);
> + if (ret < 0)
> + return ret;
> +
> + platform_set_drvdata(pdev, bus);
> +
> + dev_info(bus->dev, "i2c bus %d registered, irq %d\n",
> + bus->adap.nr, bus->irq);
checkpatch complains here:
CHECK: Alignment should match open parenthesis
#672: FILE: drivers/i2c/busses/i2c-aspeed.c:672:
> +
> + return 0;
> +}
> +
> +static int aspeed_i2c_remove_bus(struct platform_device *pdev)
> +{
> + struct aspeed_i2c_bus *bus = platform_get_drvdata(pdev);
> +
> + i2c_del_adapter(&bus->adap);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id aspeed_i2c_bus_of_table[] = {
> + { .compatible = "aspeed,ast2400-i2c-bus", },
> + { .compatible = "aspeed,ast2500-i2c-bus", },
checkpatch complains here:
WARNING: DT compatible string "aspeed,ast2400-i2c-bus" appears un-documented -- check ./Documentation/devicetree/bindings/
#687: FILE: drivers/i2c/busses/i2c-aspeed.c:687:
WARNING: DT compatible string "aspeed,ast2500-i2c-bus" appears un-documented -- check ./Documentation/devicetree/bindings/
#688: FILE: drivers/i2c/busses/i2c-aspeed.c:688:
Device tree binding documentation must precede the driver.
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, aspeed_i2c_bus_of_table);
> +
> +static struct platform_driver aspeed_i2c_bus_driver = {
> + .probe = aspeed_i2c_probe_bus,
> + .remove = aspeed_i2c_remove_bus,
> + .driver = {
> + .name = "ast-i2c-bus",
> + .of_match_table = aspeed_i2c_bus_of_table,
> + },
> +};
> +
> +/*
> + * The aspeed chip provides a single hardware interrupt for all of the I2C
> + * busses, so we use a dummy interrupt chip to translate this single interrupt
> + * into multiple interrupts, each associated with a single I2C bus.
> + */
> +static void aspeed_i2c_controller_irq(struct irq_desc *desc)
> +{
> + struct aspeed_i2c_controller *c = irq_desc_get_handler_data(desc);
> + struct irq_chip *chip = irq_desc_get_chip(desc);
> + unsigned long p, status;
> + unsigned int bus_irq;
> +
> + chained_irq_enter(chip, desc);
> + status = readl(c->base);
> + for_each_set_bit(p, &status, ASPEED_I2C_NUM_BUS) {
> + bus_irq = irq_find_mapping(c->irq_domain, p);
> + generic_handle_irq(bus_irq);
> + }
> + chained_irq_exit(chip, desc);
> +}
> +
> +/*
> + * Set simple handler and mark IRQ as valid. Nothing interesting to do here
> + * since we are using a dummy interrupt chip.
> + */
> +static int aspeed_i2c_map_irq_domain(struct irq_domain *domain,
> + unsigned int irq, irq_hw_number_t hwirq)
> +{
> + irq_set_chip_and_handler(irq, &dummy_irq_chip, handle_simple_irq);
> + irq_set_chip_data(irq, domain->host_data);
> +
> + return 0;
> +}
> +
> +static const struct irq_domain_ops aspeed_i2c_irq_domain_ops = {
> + .map = aspeed_i2c_map_irq_domain,
> +};
> +
> +static int aspeed_i2c_probe_controller(struct platform_device *pdev)
> +{
> + struct aspeed_i2c_controller *controller;
> + struct device_node *np;
> + struct resource *res;
> +
> + controller = devm_kzalloc(&pdev->dev, sizeof(*controller), GFP_KERNEL);
> + if (!controller)
> + return -ENOMEM;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + controller->base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(controller->base))
> + return PTR_ERR(controller->base);
> +
> + controller->irq = platform_get_irq(pdev, 0);
> + if (controller->irq < 0)
> + return -ENXIO;
here it should be "return controller->irq;"
> +
> + controller->irq_domain = irq_domain_add_linear(pdev->dev.of_node,
> + ASPEED_I2C_NUM_BUS, &aspeed_i2c_irq_domain_ops, NULL);
> + if (!controller->irq_domain)
> + return -ENOMEM;
> +
> + controller->irq_domain->name = "ast-i2c-domain";
> +
> + irq_set_chained_handler_and_data(controller->irq,
> + aspeed_i2c_controller_irq, controller);
checkpatch complains here:
CHECK: Alignment should match open parenthesis
#767: FILE: drivers/i2c/busses/i2c-aspeed.c:767:
But due to the long name of the function this warning can be ignored.
> +
> + controller->dev = &pdev->dev;
> +
> + platform_set_drvdata(pdev, controller);
> +
> + dev_info(controller->dev, "i2c controller registered, irq %d\n",
> + controller->irq);
checkpatch complains here:
CHECK: Alignment should match open parenthesis
#774: FILE: drivers/i2c/busses/i2c-aspeed.c:774:
> +
> + for_each_child_of_node(pdev->dev.of_node, np) {
> + of_platform_device_create(np, NULL, &pdev->dev);
> + }
This should be removed, when you correct the layout of device tree nodes
as I described in the comments to v6 2/2.
> +
> + return 0;
> +}
> +
> +static int aspeed_i2c_remove_controller(struct platform_device *pdev)
> +{
> + struct aspeed_i2c_controller *controller = platform_get_drvdata(pdev);
> +
> + irq_domain_remove(controller->irq_domain);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id aspeed_i2c_controller_of_table[] = {
> + { .compatible = "aspeed,ast2400-i2c-controller", },
> + { .compatible = "aspeed,ast2500-i2c-controller", },
checkpatch complains here:
WARNING: DT compatible string "aspeed,ast2400-i2c-controller" appears un-documented -- check ./Documentation/devicetree/bindings/
#793: FILE: drivers/i2c/busses/i2c-aspeed.c:793:
WARNING: DT compatible string "aspeed,ast2500-i2c-controller" appears un-documented -- check ./Documentation/devicetree/bindings/
#794: FILE: drivers/i2c/busses/i2c-aspeed.c:794:
Device tree binding documentation must precede the driver.
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, aspeed_i2c_controller_of_table);
> +
> +static struct platform_driver aspeed_i2c_controller_driver = {
> + .probe = aspeed_i2c_probe_controller,
> + .remove = aspeed_i2c_remove_controller,
> + .driver = {
> + .name = "ast-i2c-controller",
It might be good to get from the name that this is an interrupt controller
driver.
> + .of_match_table = aspeed_i2c_controller_of_table,
> + },
> +};
> +
> +static int __init aspeed_i2c_driver_init(void)
> +{
> + int ret;
> +
> + ret = platform_driver_register(&aspeed_i2c_controller_driver);
> + if (ret < 0) {
> + platform_driver_unregister(&aspeed_i2c_controller_driver);
> + return ret;
> + }
> +
> + ret = platform_driver_register(&aspeed_i2c_bus_driver);
> +
> + if (ret < 0) {
> + platform_driver_unregister(&aspeed_i2c_bus_driver);
> + platform_driver_unregister(&aspeed_i2c_controller_driver);
> + return ret;
Now it is done incorrectly, please unregister only successfully registered
device drivers.
This will be simplified after decoupling the driver into two independent
drivers.
> + }
> +
> + return 0;
> +}
> +module_init(aspeed_i2c_driver_init);
> +
> +static void __exit aspeed_i2c_driver_exit(void)
> +{
> + platform_driver_unregister(&aspeed_i2c_bus_driver);
> + platform_driver_unregister(&aspeed_i2c_controller_driver);
This will be simplified after decoupling the driver into two independent
drivers.
> +}
> +module_exit(aspeed_i2c_driver_exit);
> +
> +MODULE_AUTHOR("Brendan Higgins <brendanhiggins-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>");
> +MODULE_DESCRIPTION("Aspeed I2C Bus Driver");
> +MODULE_LICENSE("GPL");
>From the header this is GPL v2 only driver, then please use
MODULE_LICENSE("GPL v2");
--
With best wishes,
Vladimir
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v5 2/2] i2c: aspeed: added documentation for Aspeed I2C driver
From: Vladimir Zapolskiy @ 2016-12-07 0:19 UTC (permalink / raw)
To: Brendan Higgins, wsa-z923LK4zBo2bacvFa/9K2g, clg-Bxea+6Xhats,
mouse-Pma6HLj0uuo, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8
Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, joel-U3u1mxZcP9KHXe+LvDLADg,
openbmc-uLR06cmDAlY/bJ5BZ2RsiQ
In-Reply-To: <1480467618-7497-3-git-send-email-brendanhiggins-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
Hello Brendan,
please find review comments below.
On 11/30/2016 03:00 AM, Brendan Higgins wrote:
> Added device tree binding documentation for Aspeed I2C controller and
> busses.
This is not the exact description of the added bindings, here you add two
device tree bindings of interrupt controller device and I2C bus controller
device.
Please separate the description into two separate files, place one into
../interrupt-controller/aspeed,ast2400-i2c-ic.txt and another one into
../i2c/aspeed,ast2400-i2c.txt file
The description of device tree bindings must precede the actual drivers,
assuming that the file with two drivers is also split into two files
there should be 4 patches for v6:
1) device tree description of the interrupt controller,
2) interrupt controller driver,
3) device tree description of the I2C bus controller,
4) I2C bus controller driver.
> Signed-off-by: Brendan Higgins <brendanhiggins-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> ---
> Changes for v2:
> - None
> Changes for v3:
> - Removed reference to "bus" device tree param
> Changes for v4:
> - None
> Changes for v5:
> - None
> ---
> .../devicetree/bindings/i2c/i2c-aspeed.txt | 61 ++++++++++++++++++++++
> 1 file changed, 61 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
>
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt b/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
> new file mode 100644
> index 0000000..dd11a97
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
> @@ -0,0 +1,61 @@
> +Device tree configuration for the I2C controller and busses on the AST24XX
> +and AST25XX SoCs.
> +
> +Controller:
Interrupt controller.
> +
> + Required Properties:
> + - #address-cells : should be 1
> + - #size-cells : should be 1
> + - #interrupt-cells : should be 1
> + - compatible : should be "aspeed,ast2400-i2c-controller"
> + or "aspeed,ast2500-i2c-controller"
> + - reg : address start and range of controller
> + - ranges : defines address offset and range for busses
> + - interrupts : interrupt number
> + - clocks : root clock of bus, should reference the APB
> + clock
> + - clock-ranges : specifies that child busses can inherit clocks
> + - interrupt-controller : denotes that the controller receives and fires
> + new interrupts for child busses
> +
> +Bus:
I2C bus controller.
> +
> + Required Properties:
> + - #address-cells : should be 1
> + - #size-cells : should be 0
> + - reg : address offset and range of bus
> + - compatible : should be "aspeed,ast2400-i2c-bus"
> + or "aspeed,ast2500-i2c-bus"
> + - interrupts : interrupt number
> +
> + Optional Properties:
> + - clock-frequency : frequency of the bus clock in Hz
> + defaults to 100 kHz when not specified
> +
> +Example:
> +
> +i2c: i2c@1e78a000 {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + #interrupt-cells = <1>;
> +
> + compatible = "aspeed,ast2400-i2c-controller";
> + reg = <0x1e78a000 0x40>;
> + ranges = <0 0x1e78a000 0x1000>;
> + interrupts = <12>;
> + clocks = <&clk_apb>;
> + clock-ranges;
> + interrupt-controller;
> +
> + i2c0: i2c-bus@40 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0x40 0x40>;
> + compatible = "aspeed,ast2400-i2c-bus";
> + clock-frequency = <100000>;
> + status = "disabled";
> + interrupts = <0>;
> + interrupt-parent = <&i2c>;
> + };
> +};
The selected layout of device tree nodes does not reflect the actual
hardware, I2C bus controller (sub-)devices can not be children of the
interrupt controller device, they are only consumers of interrupts from
the interrupt controller device.
In this case the proper and expected device tree description should
look like the one below:
i2c {
compatible = "simple-bus";
#address-cells = <1>;
#size-cells = <1>;
ranges = <0 0x1e78a000 0x1000>;
i2c-ic: interrupt-controller@0 {
compatible = "aspeed,ast2400-i2c-ic";
reg = <0x0 0x40>;
clocks = <&clk_apb>;
interrupt-controller;
interrupts = <12>;
#interrupt-cells = <1>;
};
i2c0: i2c@40 {
compatible = "aspeed,ast2400-i2c";
reg = <0x40 0x40>;
#address-cells = <1>;
#size-cells = <0>;
clock-frequency = <100000>;
interrupt-parent = <&i2c-ic>;
interrupts = <0>;
status = "disabled";
};
i2c1: i2c-bus@80 {
.....
};
};
--
With best wishes,
Vladimir
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFC v2 2/2] i2c: Pass i2c_device_id to probe func when using DT ids through ACPI
From: Dmitry Torokhov @ 2016-12-06 17:00 UTC (permalink / raw)
To: Phong Vo
Cc: mika.westerberg, leonard.crestez, wsa+renesas, rafael.j.wysocki,
len.brown, irina.tirdea, octavian.purdila, daniel.baluta,
linux-kernel, linux-acpi, linux-i2c, Tin Huynh, Loc Ho, patches
In-Reply-To: <8a69cdff155793e8e687d527d9ace97f@mail.gmail.com>
On Tue, Nov 01, 2016 at 01:14:16PM +0700, Phong Vo wrote:
> >From: Mika Westerberg <mika.westerberg@linux.intel.com>
> >Subject: Re: [RFC v2 2/2] i2c: Pass i2c_device_id to probe func when
> using DT ids through ACPI
> >Date: Monday 13th June 2016 09:26:55 UTC (5 months ago)
> >
> >On Fri, Jun 10, 2016 at 06:57:36PM +0300, Crestez Dan Leonard wrote:
> >> On 06/10/2016 09:32 AM, Mika Westerberg wrote:
> >> > On Thu, Jun 09, 2016 at 04:06:03PM +0300, Crestez Dan Leonard wrote:
> >> >> When devices are instatiated through devicetree the i2c_client->name
> is
> >> >> set to the compatible string with company name stripped out. This is
> >> >> then matched to the i2c_device_id table to pass the device_id to the
> >> >> probe function. This id parameter is used by some device drivers to
> >> >> differentiate between model numbers.
> >> >>
> >> >> When using ACPI this id parameter is NULL and the driver usually
> needs
> >> >> to do ACPI-specific differentiation.
> >> >>
> >> >> This patch attempts to find a valid i2c_device_id when using ACPI
> with
> >> >> DT-like compatible strings.
> >> >
> >> > So I don't really understand why it would be good idea to pass
> >> > i2c_device_id for devices which are matched against their ACPI/DT
> >> > tables. Apparently DT is already doing that so maybe there is some
> >> > reason.
> >> >
> >> > Anyway, why not fill in the device name when it is first enumerated
> >> > if it uses DT compatible property? Just like DT does.
> >> >
> >> This automatic matching of i2c_device_id works for devicetree because
> >> of_i2c_register_device sets i2c_board_info.type to the compatible
> string
> >> with the vendor prefix removed. For I2C devices described via ACPI the
> >> i2c_board_info.type string is set to the ACPI device name. This ends up
> >> something like "PRP0001:00".
> >>
> >> This could be changed in acpi_i2c_get_info to use the of_compatible
> >> string from DSD if present. Is that what you mean? That would work and
> >> it would be cleaner than my patch. Something like this:
> >>
> >> diff --git drivers/i2c/i2c-core.c drivers/i2c/i2c-core.c
> >> index 1e0ef9b..ba2fe7f 100644
> >> --- drivers/i2c/i2c-core.c
> >> +++ drivers/i2c/i2c-core.c
> >> @@ -181,7 +181,24 @@ static int acpi_i2c_get_info(struct acpi_device
> >*adev,
> >>
> >> acpi_dev_free_resource_list(&resource_list);
> >>
> >> - strlcpy(info->type, dev_name(&adev->dev), sizeof(info->type));
> >> + /*
> >> + * If we have a DT id set info.type to the first compatible
> >> string with
> >> + * the vendor prefix stripped. This is similar to
> >of_modalias_node
> >> + */
> >> + if (adev->data.of_compatible) {
> >> + const union acpi_object *obj;
> >> + const char *str, *chr;
> >> +
> >> + obj = adev->data.of_compatible;
> >> + if (obj->type == ACPI_TYPE_PACKAGE)
> >> + obj = obj->package.elements;
> >> + str = obj->string.pointer;
> >> + chr = strchr(str, ',');
> >> + if (chr)
> >> + str = chr + 1;
> >> + strlcpy(info->type, str, sizeof(info->type));
> >> + } else
> >> + strlcpy(info->type, dev_name(&adev->dev),
> >> sizeof(info->type));
> >>
> >> return 0;
> >> }>
> >
> >Yes, that's what I mean.
> >
> >> The biggest concern is that this would change the i2c device name
> >> between kernel versions. Is that acceptable?
> >
> >I don't think that is a problem since I still have not seen a single
> >system using ACPI _DSD so I would not expect anything to break.
> >
> >However, I'm still not convinced it is good idea to return i2c_device_id
> >from a completely different table if we match using ACPI/DT table.
>
> All,
>
> Is there a conclusion on this? We have been tackling the same issue and
> incidentally arrived at a
> similar solution as like Lenard proposed in the patch above.
FWIW I agree with Mika (I think?) and I do not like that we mangle OF
data. This causes issues with module loading (where every OF ID has to
be stripped and added to the legacy table), and so forth. Drivers can
either use of_device_get_match_data() to get "real" OF id, or maybe we
could temporarily transform of_device_id into i2c_device_id in i2c bus
code and pass it to probe(), and do the same with ACPI match data.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] i2c: rk3x: keep i2c irq ON in suspend
From: Doug Anderson @ 2016-12-06 16:31 UTC (permalink / raw)
To: David.Wu
Cc: Heiko Stuebner, Wolfram Sang,
linux-arm-kernel@lists.infradead.org,
open list:ARM/Rockchip SoC..., linux-i2c@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <1d570232-af87-467e-e6f4-5b514d583a02@rock-chips.com>
Hi,
On Tue, Dec 6, 2016 at 12:12 AM, David.Wu <david.wu@rock-chips.com> wrote:
> Hi Heiko,
>
> 在 2016/12/5 18:54, Heiko Stuebner 写道:
>>
>> Hi David,
>>
>> Am Montag, 5. Dezember 2016, 16:02:59 CET schrieb David Wu:
>>>
>>> During suspend there may still be some i2c access happening.
>>> And if we don't keep i2c irq ON, there may be i2c access timeout if
>>> i2c is in irq mode of operation.
>>
>>
>> can you describe the issue you're trying to fix a bit more please?
>
>
> Sometimes we could see the i2c timeout errors during suspend/resume, which
> makes the duration of suspend/resume too longer.
>
> [ 484.171541] CPU4: Booted secondary processor [410fd082]
> [ 485.172777] rk3x-i2c ff3c0000.i2c: timeout, ipd: 0x10, state: 1
> [ 486.172760] rk3x-i2c ff3c0000.i2c: timeout, ipd: 0x10, state: 1
> [ 487.172759] rk3x-i2c ff3c0000.i2c: timeout, ipd: 0x10, state: 1
> [ 487.172840] cpu cpu4: _set_opp_voltage: failed to set voltage (800000
> 800000 800000 mV): -110
> [ 487.172874] cpu cpu4: failed to set volt 800000
>
>>
>> I.e. I'd think the i2c-core does suspend i2c-client devices first, so that
>> these should be able to finish up their ongoing transfers and not start
>> any
>> new ones instead?
>>
>> Your irq can still happen slightly after the system started going to
>> actually
>> sleep, so to me it looks like you just widened the window where irqs can
>> be
>> handled. Especially as your irq could also just simply stem from the start
>> state, so you cannot even be sure if your transaction actually is
>> finished.
>
>
> Okay, you are right. I want to give it a double insurance at first, but it
> may hide the unhappend issue.
>
>>
>> So to me it looks like the i2c-connected device driver should be fixed
>> instead?
>
>
> I tell them to fix it in rk808 driver.
To me it seems like perhaps cpufreq should not be changing frequencies
until it is resumed properly. Presumably if all the ordering is done
right then cpufreq should be resumed _after_ the i2c regulator so you
should be OK. ...or am I somehow confused about that?
Also note that previous i2c busses I worked with simply returned -EIO
in the case where they were called when suspended. See
"i2c-exynos5.c" and "i2c-s3c2410.c".
-Doug
^ permalink raw reply
* [PATCH v4 i2c/for-next] i2c: rcar: Add per-Generation fallback bindings
From: Simon Horman @ 2016-12-06 16:01 UTC (permalink / raw)
To: Wolfram Sang
Cc: Magnus Damm, linux-i2c, linux-renesas-soc, Rob Herring,
devicetree, Simon Horman
In the case of Renesas R-Car hardware we know that there are generations of
SoCs, e.g. Gen 2 and Gen 3. But beyond that it's not clear what the
relationship between IP blocks might be. For example, I believe that
r8a7790 is older than r8a7791 but that doesn't imply that the latter is a
descendant of the former or vice versa.
We can, however, by examining the documentation and behaviour of the
hardware at run-time observe that the current driver implementation appears
to be compatible with the IP blocks on SoCs within a given generation.
For the above reasons and convenience when enabling new SoCs a
per-generation fallback compatibility string scheme is being adopted for
drivers for Renesas SoCs.
Also:
* Deprecate renesas,i2c-rcar. It seems poorly named as it is only
compatible with R-Car Gen 1. It also appears unused in mainline.
* Add some text to describe per-SoC bindings
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v5
* s/R8A7797/R8A777/
v4
* Correct grammar in changelog
v3
* Consistently use renesas,<family>-<module> for new compat strings
* Drop RFC designation
v2
* Include accidently omitted i2c-rcar.c portion of patch
---
Documentation/devicetree/bindings/i2c/i2c-rcar.txt | 32 ++++++++++++++--------
drivers/i2c/busses/i2c-rcar.c | 5 +++-
2 files changed, 24 insertions(+), 13 deletions(-)
diff --git a/Documentation/devicetree/bindings/i2c/i2c-rcar.txt b/Documentation/devicetree/bindings/i2c/i2c-rcar.txt
index 239632a0d709..2b8bd33dbf8d 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-rcar.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-rcar.txt
@@ -1,17 +1,25 @@
I2C for R-Car platforms
Required properties:
-- compatible: Must be one of
- "renesas,i2c-rcar"
- "renesas,i2c-r8a7778"
- "renesas,i2c-r8a7779"
- "renesas,i2c-r8a7790"
- "renesas,i2c-r8a7791"
- "renesas,i2c-r8a7792"
- "renesas,i2c-r8a7793"
- "renesas,i2c-r8a7794"
- "renesas,i2c-r8a7795"
- "renesas,i2c-r8a7796"
+- compatible:
+ "renesas,i2c-r8a7778" if the device is a part of a R8A7778 SoC.
+ "renesas,i2c-r8a7779" if the device is a part of a R8A7779 SoC.
+ "renesas,i2c-r8a7790" if the device is a part of a R8A7790 SoC.
+ "renesas,i2c-r8a7791" if the device is a part of a R8A7791 SoC.
+ "renesas,i2c-r8a7792" if the device is a part of a R8A7792 SoC.
+ "renesas,i2c-r8a7793" if the device is a part of a R8A7793 SoC.
+ "renesas,i2c-r8a7794" if the device is a part of a R8A7794 SoC.
+ "renesas,i2c-r8a7795" if the device is a part of a R8A7795 SoC.
+ "renesas,i2c-r8a7796" if the device is a part of a R8A7796 SoC.
+ "renesas,rcar-gen1-i2c" for a generic R-Car Gen1 compatible device.
+ "renesas,rcar-gen2-i2c" for a generic R-Car Gen2 compatible device.
+ "renesas,rcar-gen3-i2c" for a generic R-Car Gen3 compatible device.
+ "renesas,i2c-rcar" (deprecated)
+
+ When compatible with the generic version, nodes must list the
+ SoC-specific version corresponding to the platform first followed
+ by the generic version.
+
- reg: physical base address of the controller and length of memory mapped
region.
- interrupts: interrupt specifier.
@@ -33,7 +41,7 @@ Examples :
i2c0: i2c@e6508000 {
#address-cells = <1>;
#size-cells = <0>;
- compatible = "renesas,i2c-r8a7791";
+ compatible = "renesas,i2c-r8a7791", "renesas,rcar-gen2-i2c";
reg = <0 0xe6508000 0 0x40>;
interrupts = <0 287 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&mstp9_clks R8A7791_CLK_I2C0>;
diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index 726615e54f2a..622def6b43e2 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -793,7 +793,6 @@ static const struct i2c_algorithm rcar_i2c_algo = {
};
static const struct of_device_id rcar_i2c_dt_ids[] = {
- { .compatible = "renesas,i2c-rcar", .data = (void *)I2C_RCAR_GEN1 },
{ .compatible = "renesas,i2c-r8a7778", .data = (void *)I2C_RCAR_GEN1 },
{ .compatible = "renesas,i2c-r8a7779", .data = (void *)I2C_RCAR_GEN1 },
{ .compatible = "renesas,i2c-r8a7790", .data = (void *)I2C_RCAR_GEN2 },
@@ -803,6 +802,10 @@ static const struct of_device_id rcar_i2c_dt_ids[] = {
{ .compatible = "renesas,i2c-r8a7794", .data = (void *)I2C_RCAR_GEN2 },
{ .compatible = "renesas,i2c-r8a7795", .data = (void *)I2C_RCAR_GEN3 },
{ .compatible = "renesas,i2c-r8a7796", .data = (void *)I2C_RCAR_GEN3 },
+ { .compatible = "renesas,i2c-rcar", .data = (void *)I2C_RCAR_GEN1 }, // Deprecated
+ { .compatible = "renesas,rcar-gen1-i2c", .data = (void *)I2C_RCAR_GEN1 },
+ { .compatible = "renesas,rcar-gen2-i2c", .data = (void *)I2C_RCAR_GEN2 },
+ { .compatible = "renesas,rcar-gen3-i2c", .data = (void *)I2C_RCAR_GEN3 },
{},
};
MODULE_DEVICE_TABLE(of, rcar_i2c_dt_ids);
--
2.7.0.rc3.207.g0ac5344
^ permalink raw reply related
* Re: [PATCH v6 4/9] dt-bindings: iio: iio-mux: document iio-mux bindings
From: Peter Rosin @ 2016-12-06 9:18 UTC (permalink / raw)
To: Rob Herring
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Wolfram Sang, Mark Rutland,
Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Jonathan Corbet, Arnd Bergmann,
Greg Kroah-Hartman, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161205232624.5wjvmchd6jxd24ir@rob-hp-laptop>
On 2016-12-06 00:26, Rob Herring wrote:
> On Wed, Nov 30, 2016 at 09:16:58AM +0100, Peter Rosin wrote:
>> Signed-off-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
>> ---
>> .../bindings/iio/multiplexer/iio-mux.txt | 40 ++++++++++++++++++++++
>> MAINTAINERS | 6 ++++
>> 2 files changed, 46 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/iio/multiplexer/iio-mux.txt
>
> I'm still not convinced about this binding, but don't really have more
> comments ATM. Sending 6 versions in 2 weeks or so doesn't really help
> either.
Sorry about the noise, I'll try to be more careful going forward. On
the flip side, I haven't touched the code since v6.
I don't see how bindings that are as flexible as the current (and
original) phandle link between the mux consumer and the mux controller
would look, and at the same time be simpler to understand. You need
to be able to refer to a mux controller from several mux consumers, and
you need to support several mux controllers in one node (the ADG792A
case). And, AFAICT, the complex case wasn't really the problem, it was
that it is overly complex to describe the simple case of one mux
consumer and one mux controller. But in your comment for v2 [1] you
said that I was working around limitations with shared GPIO pins. But
solving that in the GPIO subsystem would not solve all that the
phandle approach is solving, since you would not have support for
ADG792A (or other non-GPIO controlled muxes). So, I think listing
the gpio pins inside the mux consumer node is a non-starter, the mux
controller has to live in its own node with its own compatible.
Would you be happier if I managed to marry the phandle approach with
the option of having the mux controller as a child node of the mux
consumer for the simple case?
I added an example at the end of this message (the same as the first
example in v4 [2], at least in principle) for easy comparison between
the phandle and the controller-in-child-node approaches. I can't say
that I personally find the difference all that significant, and do not
think it is worth it. As I see it, the "simple option" would just muddy
the waters...
[1] http://marc.info/?l=linux-kernel&m=147948334204795&w=2
[2] http://marc.info/?l=linux-kernel&m=148001364904240&w=2
>> diff --git a/Documentation/devicetree/bindings/iio/multiplexer/iio-mux.txt b/Documentation/devicetree/bindings/iio/multiplexer/iio-mux.txt
>> new file mode 100644
>> index 000000000000..8080cf790d82
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/multiplexer/iio-mux.txt
>> @@ -0,0 +1,40 @@
>> +IIO multiplexer bindings
>> +
>> +If a multiplexer is used to select which hardware signal is fed to
>> +e.g. an ADC channel, these bindings describe that situation.
>> +
>> +Required properties:
>> +- compatible : "iio-mux"
>
> This is a Linuxism. perhaps "adc-mux".
No, that's not general enough, it could just as well be used to mux a
temperature sensor. Or whatever. Hmmm, given that "iio-mux" is bad, perhaps
"io-channel-mux" is better? That matches the io-channels property used to
refer to the parent channel.
>> +- io-channels : Channel node of the parent channel that has multiplexed
>> + input.
>> +- io-channel-names : Should be "parent".
>> +- #address-cells = <1>;
>> +- #size-cells = <0>;
>> +- mux-controls : Mux controller node to use for operating the mux
>> +- channels : List of strings, labeling the mux controller states.
>> +
>> +The multiplexer state as described in ../misc/mux-controller.txt
>> +
>> +For each non-empty string in the channels property, an iio channel will
>> +be created. The number of this iio channel is the same as the index into
>> +the list of strings in the channels property, and also matches the mux
>> +controller state.
>> +
>> +Example:
>> + mux: mux-controller {
>> + compatible = "mux-gpio";
>> + #mux-control-cells = <0>;
>> +
>> + mux-gpios = <&pioA 0 GPIO_ACTIVE_HIGH>,
>> + <&pioA 1 GPIO_ACTIVE_HIGH>;
>> + };
>> +
>> + adc-mux {
>> + compatible = "iio-mux";
>> + io-channels = <&adc 0>;
>> + io-channel-names = "parent";
>> +
>> + mux-controls = <&mux>;
>> +
>> + channels = "sync", "in", system-regulator";
>> + };
Describing the same as above, but with the mux controller as a child
node.
adc-mux {
compatible = "iio-mux";
io-channels = <&adc 0>;
io-channel-names = "parent";
channels = "sync", "in", system-regulator";
mux-controller {
compatible = "mux-gpio";
mux-gpios = <&pioA 0 GPIO_ACTIVE_HIGH>,
<&pioA 1 GPIO_ACTIVE_HIGH>;
};
};
Cheers,
Peter
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] i2c: rk3x: keep i2c irq ON in suspend
From: David.Wu @ 2016-12-06 8:12 UTC (permalink / raw)
To: Heiko Stuebner
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, wsa-z923LK4zBo2bacvFa/9K2g,
dianders-F7+t8E8rja9g9hUCZPvPmw,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1737869.ycGphUXI8X@phil>
Hi Heiko,
在 2016/12/5 18:54, Heiko Stuebner 写道:
> Hi David,
>
> Am Montag, 5. Dezember 2016, 16:02:59 CET schrieb David Wu:
>> During suspend there may still be some i2c access happening.
>> And if we don't keep i2c irq ON, there may be i2c access timeout if
>> i2c is in irq mode of operation.
>
> can you describe the issue you're trying to fix a bit more please?
Sometimes we could see the i2c timeout errors during suspend/resume,
which makes the duration of suspend/resume too longer.
[ 484.171541] CPU4: Booted secondary processor [410fd082]
[ 485.172777] rk3x-i2c ff3c0000.i2c: timeout, ipd: 0x10, state: 1
[ 486.172760] rk3x-i2c ff3c0000.i2c: timeout, ipd: 0x10, state: 1
[ 487.172759] rk3x-i2c ff3c0000.i2c: timeout, ipd: 0x10, state: 1
[ 487.172840] cpu cpu4: _set_opp_voltage: failed to set voltage (800000
800000 800000 mV): -110
[ 487.172874] cpu cpu4: failed to set volt 800000
>
> I.e. I'd think the i2c-core does suspend i2c-client devices first, so that
> these should be able to finish up their ongoing transfers and not start any
> new ones instead?
>
> Your irq can still happen slightly after the system started going to actually
> sleep, so to me it looks like you just widened the window where irqs can be
> handled. Especially as your irq could also just simply stem from the start
> state, so you cannot even be sure if your transaction actually is finished.
Okay, you are right. I want to give it a double insurance at first, but
it may hide the unhappend issue.
>
> So to me it looks like the i2c-connected device driver should be fixed instead?
I tell them to fix it in rk808 driver.
>
> In the past I solved this for example in the zforce_ts driver [0] to
> prevent i2c transfers from happening during suspend.
>
>
> Heiko
>
> [0] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/input/touchscreen/zforce_ts.c
>
>
>>
>> Signed-off-by: David Wu <david.wu@rock-chips.com>
>> ---
>> drivers/i2c/busses/i2c-rk3x.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
>> index df22066..67af32a 100644
>> --- a/drivers/i2c/busses/i2c-rk3x.c
>> +++ b/drivers/i2c/busses/i2c-rk3x.c
>> @@ -1261,7 +1261,7 @@ static int rk3x_i2c_probe(struct platform_device
>> *pdev) }
>>
>> ret = devm_request_irq(&pdev->dev, irq, rk3x_i2c_irq,
>> - 0, dev_name(&pdev->dev), i2c);
>> + IRQF_NO_SUSPEND, dev_name(&pdev->dev), i2c);
>> if (ret < 0) {
>> dev_err(&pdev->dev, "cannot request IRQ\n");
>> return ret;
>
>
>
>
>
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply
* [PATCH V2] i2c: designware: fix wrong tx/rx fifo for ACPI
From: Tin Huynh @ 2016-12-06 7:57 UTC (permalink / raw)
To: Jarkko Nikula, Andy Shevchenko, Mika Westerberg, Wolfram Sang
Cc: linux-i2c, linux-kernel, linux-acpi, Loc Ho, Thang Nguyen,
Phong Vo, patches, Tin Huynh
ACPI always sets txfifo and rxfifo to 32. This configuration will
cause problem if the IP core supports a fifo size of less than 32.
The driver should read the fifo size from the IP and select the
smaller one of the two.
Signed-off-by: Tin Huynh <tnhuynh@apm.com>
---
drivers/i2c/busses/i2c-designware-platdrv.c | 15 +++++++++++----
1 files changed, 11 insertions(+), 4 deletions(-)
Change from V1:
-Revert the default 32 for fifo, read parameter from IP core
and pick the smaller one of the two.
-Correct the title to describe new approach.
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 0b42a12..76b061f 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -153,6 +153,7 @@ static int i2c_dw_plat_prepare_clk(struct dw_i2c_dev *i_dev, bool prepare)
static int dw_i2c_plat_probe(struct platform_device *pdev)
{
struct dw_i2c_platform_data *pdata = dev_get_platdata(&pdev->dev);
+ u32 param1, tx_fifo_depth, rx_fifo_depth;
struct dw_i2c_dev *dev;
struct i2c_adapter *adap;
struct resource *mem;
@@ -246,12 +247,18 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
1000000);
}
+ param1 = i2c_dw_read_comp_param(dev);
+ tx_fifo_depth = ((param1 >> 16) & 0xff) + 1;
+ rx_fifo_depth = ((param1 >> 8) & 0xff) + 1;
if (!dev->tx_fifo_depth) {
- u32 param1 = i2c_dw_read_comp_param(dev);
-
- dev->tx_fifo_depth = ((param1 >> 16) & 0xff) + 1;
- dev->rx_fifo_depth = ((param1 >> 8) & 0xff) + 1;
+ dev->tx_fifo_depth = tx_fifo_depth;
+ dev->rx_fifo_depth = rx_fifo_depth;
dev->adapter.nr = pdev->id;
+ } else if (tx_fifo_depth) {
+ dev->tx_fifo_depth = min_t(u32, dev->tx_fifo_depth,
+ tx_fifo_depth);
+ dev->rx_fifo_depth = min_t(u32, dev->rx_fifo_depth,
+ rx_fifo_depth);
}
adap = &dev->adapter;
--
1.7.1
^ permalink raw reply related
* Re: [PATCH v6 4/9] dt-bindings: iio: iio-mux: document iio-mux bindings
From: Rob Herring @ 2016-12-05 23:26 UTC (permalink / raw)
To: Peter Rosin
Cc: linux-kernel, Wolfram Sang, Mark Rutland, Jonathan Cameron,
Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
Jonathan Corbet, Arnd Bergmann, Greg Kroah-Hartman, linux-i2c,
devicetree, linux-iio, linux-doc
In-Reply-To: <1480493823-21462-5-git-send-email-peda@axentia.se>
On Wed, Nov 30, 2016 at 09:16:58AM +0100, Peter Rosin wrote:
> Signed-off-by: Peter Rosin <peda@axentia.se>
> ---
> .../bindings/iio/multiplexer/iio-mux.txt | 40 ++++++++++++++++++++++
> MAINTAINERS | 6 ++++
> 2 files changed, 46 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/multiplexer/iio-mux.txt
I'm still not convinced about this binding, but don't really have more
comments ATM. Sending 6 versions in 2 weeks or so doesn't really help
either.
> diff --git a/Documentation/devicetree/bindings/iio/multiplexer/iio-mux.txt b/Documentation/devicetree/bindings/iio/multiplexer/iio-mux.txt
> new file mode 100644
> index 000000000000..8080cf790d82
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/multiplexer/iio-mux.txt
> @@ -0,0 +1,40 @@
> +IIO multiplexer bindings
> +
> +If a multiplexer is used to select which hardware signal is fed to
> +e.g. an ADC channel, these bindings describe that situation.
> +
> +Required properties:
> +- compatible : "iio-mux"
This is a Linuxism. perhaps "adc-mux".
> +- io-channels : Channel node of the parent channel that has multiplexed
> + input.
> +- io-channel-names : Should be "parent".
> +- #address-cells = <1>;
> +- #size-cells = <0>;
> +- mux-controls : Mux controller node to use for operating the mux
> +- channels : List of strings, labeling the mux controller states.
> +
> +The multiplexer state as described in ../misc/mux-controller.txt
> +
> +For each non-empty string in the channels property, an iio channel will
> +be created. The number of this iio channel is the same as the index into
> +the list of strings in the channels property, and also matches the mux
> +controller state.
> +
> +Example:
> + mux: mux-controller {
> + compatible = "mux-gpio";
> + #mux-control-cells = <0>;
> +
> + mux-gpios = <&pioA 0 GPIO_ACTIVE_HIGH>,
> + <&pioA 1 GPIO_ACTIVE_HIGH>;
> + };
> +
> + adc-mux {
> + compatible = "iio-mux";
> + io-channels = <&adc 0>;
> + io-channel-names = "parent";
> +
> + mux-controls = <&mux>;
> +
> + channels = "sync", "in", system-regulator";
> + };
^ permalink raw reply
* Re: [PATCH v5 2/2] i2c: aspeed: added documentation for Aspeed I2C driver
From: Rob Herring @ 2016-12-05 22:28 UTC (permalink / raw)
To: Brendan Higgins
Cc: wsa, vz, clg, mouse, mark.rutland, linux-i2c, devicetree, joel,
openbmc
In-Reply-To: <1480467618-7497-3-git-send-email-brendanhiggins@google.com>
On Tue, Nov 29, 2016 at 05:00:18PM -0800, Brendan Higgins wrote:
> Added device tree binding documentation for Aspeed I2C controller and
> busses.
>
> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
> ---
> Changes for v2:
> - None
> Changes for v3:
> - Removed reference to "bus" device tree param
> Changes for v4:
> - None
> Changes for v5:
> - None
> ---
> .../devicetree/bindings/i2c/i2c-aspeed.txt | 61 ++++++++++++++++++++++
> 1 file changed, 61 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH 09/39] Annotate hardware config module parameters in drivers/i2c/
From: One Thousand Gnomes @ 2016-12-05 21:09 UTC (permalink / raw)
To: David Howells
Cc: Jean Delvare, minyard, linux-kernel, Wolfram Sang,
linux-security-module, keyrings, linux-i2c
In-Reply-To: <23573.1480601572@warthog.procyon.org.uk>
O> > > Suggested-by: One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk>
> >
> > I know this is only a Suggested-by and not a Signed-off-by, but still I
> > believe the Developer's Certificate of Origin applies, and it says:
> > "using your real name (sorry, no pseudonyms or anonymous
> > contributions.)"
>
> I asked him what he prefers - but no response.
I didn't see that question but "Alan Cox" is probably saner - the
thousand gnomes is an old Linus joke 8)
> > i2c-piix4.c:module_param (force, int, 0);
> > i2c-sis630.c:module_param(force, bool, 0);
> > i2c-viapro.c:module_param(force, bool, 0);
>
> I don't know either. One could argue it *should* be locked down because its
> need appears to reflect a BIOS bug.
And none of those should show up in that usage form on a box new enough
to have EFI secure boot. Those that do have i2c busses will report them
via ACPI by that period.
Alan
^ permalink raw reply
* Re: [PATCH] i2c: rk3x: keep i2c irq ON in suspend
From: Heiko Stuebner @ 2016-12-05 10:54 UTC (permalink / raw)
To: David Wu
Cc: wsa-z923LK4zBo2bacvFa/9K2g, dianders-F7+t8E8rja9g9hUCZPvPmw,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1480924979-13450-1-git-send-email-david.wu-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
Hi David,
Am Montag, 5. Dezember 2016, 16:02:59 CET schrieb David Wu:
> During suspend there may still be some i2c access happening.
> And if we don't keep i2c irq ON, there may be i2c access timeout if
> i2c is in irq mode of operation.
can you describe the issue you're trying to fix a bit more please?
I.e. I'd think the i2c-core does suspend i2c-client devices first, so that
these should be able to finish up their ongoing transfers and not start any
new ones instead?
Your irq can still happen slightly after the system started going to actually
sleep, so to me it looks like you just widened the window where irqs can be
handled. Especially as your irq could also just simply stem from the start
state, so you cannot even be sure if your transaction actually is finished.
So to me it looks like the i2c-connected device driver should be fixed instead?
In the past I solved this for example in the zforce_ts driver [0] to
prevent i2c transfers from happening during suspend.
Heiko
[0] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/input/touchscreen/zforce_ts.c
>
> Signed-off-by: David Wu <david.wu-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> ---
> drivers/i2c/busses/i2c-rk3x.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
> index df22066..67af32a 100644
> --- a/drivers/i2c/busses/i2c-rk3x.c
> +++ b/drivers/i2c/busses/i2c-rk3x.c
> @@ -1261,7 +1261,7 @@ static int rk3x_i2c_probe(struct platform_device
> *pdev) }
>
> ret = devm_request_irq(&pdev->dev, irq, rk3x_i2c_irq,
> - 0, dev_name(&pdev->dev), i2c);
> + IRQF_NO_SUSPEND, dev_name(&pdev->dev), i2c);
> if (ret < 0) {
> dev_err(&pdev->dev, "cannot request IRQ\n");
> return ret;
--
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
* [PATCH] i2c: rk3x: keep i2c irq ON in suspend
From: David Wu @ 2016-12-05 8:02 UTC (permalink / raw)
To: heiko-4mtYJXux2i+zQB+pC5nmwQ, wsa-z923LK4zBo2bacvFa/9K2g
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
dianders-F7+t8E8rja9g9hUCZPvPmw,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-i2c-u79uwXL29TY76Z2rM5mHXA, David Wu,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
During suspend there may still be some i2c access happening.
And if we don't keep i2c irq ON, there may be i2c access timeout if
i2c is in irq mode of operation.
Signed-off-by: David Wu <david.wu-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
---
drivers/i2c/busses/i2c-rk3x.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
index df22066..67af32a 100644
--- a/drivers/i2c/busses/i2c-rk3x.c
+++ b/drivers/i2c/busses/i2c-rk3x.c
@@ -1261,7 +1261,7 @@ static int rk3x_i2c_probe(struct platform_device *pdev)
}
ret = devm_request_irq(&pdev->dev, irq, rk3x_i2c_irq,
- 0, dev_name(&pdev->dev), i2c);
+ IRQF_NO_SUSPEND, dev_name(&pdev->dev), i2c);
if (ret < 0) {
dev_err(&pdev->dev, "cannot request IRQ\n");
return ret;
--
1.9.1
^ permalink raw reply related
* Re: [PATCH V1] rtc: ds1307: Add ACPI support
From: 'Tin Huynh' via rtc-linux @ 2016-12-05 7:48 UTC (permalink / raw)
To: Alessandro Zummo, Alexandre Belloni
Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-acpi-u79uwXL29TY76Z2rM5mHXA,
rtc-linux-/JYPxA39Uh5TLH3MbocFFw, Loc Ho, Thang Nguyen, Phong Vo,
patches, Tin Huynh
In-Reply-To: <1480474651-13984-1-git-send-email-tnhuynh-qTEPVZfXA3Y@public.gmane.org>
On Wed, Nov 30, 2016 at 9:57 AM, Tin Huynh <tnhuynh-qTEPVZfXA3Y@public.gmane.org> wrote:
>
> This patch enables ACPI support for rtc-ds1307 driver.
>
> Signed-off-by: Tin Huynh <tnhuynh-qTEPVZfXA3Y@public.gmane.org>
> ---
> drivers/rtc/rtc-ds1307.c | 51 ++++++++++++++++++++++++++++++++++++++-------
> 1 files changed, 43 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
> index 4e31036..2a1a4d3 100644
> --- a/drivers/rtc/rtc-ds1307.c
> +++ b/drivers/rtc/rtc-ds1307.c
> @@ -11,6 +11,7 @@
> * published by the Free Software Foundation.
> */
>
> +#include <linux/acpi.h>
> #include <linux/bcd.h>
> #include <linux/i2c.h>
> #include <linux/init.h>
> @@ -191,6 +192,26 @@ static u8 do_trickle_setup_ds1339(struct i2c_client *,
> };
> MODULE_DEVICE_TABLE(i2c, ds1307_id);
>
> +#ifdef CONFIG_ACPI
> +static const struct acpi_device_id ds1307_acpi_ids[] = {
> + { .id = "DS1307", .driver_data = ds_1307 },
> + { .id = "DS1337", .driver_data = ds_1337 },
> + { .id = "DS1338", .driver_data = ds_1338 },
> + { .id = "DS1339", .driver_data = ds_1339 },
> + { .id = "DS1388", .driver_data = ds_1388 },
> + { .id = "DS1340", .driver_data = ds_1340 },
> + { .id = "DS3231", .driver_data = ds_3231 },
> + { .id = "M41T00", .driver_data = m41t00 },
> + { .id = "MCP7940X", .driver_data = mcp794xx },
> + { .id = "MCP7941X", .driver_data = mcp794xx },
> + { .id = "PT7C4338", .driver_data = ds_1307 },
> + { .id = "RX8025", .driver_data = rx_8025 },
> + { .id = "ISL12057", .driver_data = ds_1337 },
> + { }
> +};
> +MODULE_DEVICE_TABLE(acpi, ds1307_acpi_ids);
> +#endif
> +
> /*----------------------------------------------------------------------*/
>
> #define BLOCK_DATA_MAX_TRIES 10
> @@ -874,7 +895,7 @@ static u8 do_trickle_setup_ds1339(struct i2c_client *client,
> return setup;
> }
>
> -static void ds1307_trickle_of_init(struct i2c_client *client,
> +static void ds1307_trickle_init(struct i2c_client *client,
> struct chip_desc *chip)
> {
> uint32_t ohms = 0;
> @@ -882,9 +903,10 @@ static void ds1307_trickle_of_init(struct i2c_client *client,
>
> if (!chip->do_trickle_setup)
> goto out;
> - if (of_property_read_u32(client->dev.of_node, "trickle-resistor-ohms" , &ohms))
> + if (device_property_read_u32(&client->dev, "trickle-resistor-ohms",
> + &ohms))
> goto out;
> - if (of_property_read_bool(client->dev.of_node, "trickle-diode-disable"))
> + if (device_property_read_bool(&client->dev, "trickle-diode-disable"))
> diode = false;
> chip->trickle_charger_setup = chip->do_trickle_setup(client,
> ohms, diode);
> @@ -1268,7 +1290,7 @@ static int ds1307_probe(struct i2c_client *client,
> struct ds1307 *ds1307;
> int err = -ENODEV;
> int tmp, wday;
> - struct chip_desc *chip = &chips[id->driver_data];
> + struct chip_desc *chip;
> struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
> bool want_irq = false;
> bool ds1307_can_wakeup_device = false;
> @@ -1297,11 +1319,23 @@ static int ds1307_probe(struct i2c_client *client,
> i2c_set_clientdata(client, ds1307);
>
> ds1307->client = client;
> - ds1307->type = id->driver_data;
> + if (id) {
> + chip = &chips[id->driver_data];
> + ds1307->type = id->driver_data;
> + } else {
> + const struct acpi_device_id *acpi_id;
> +
> + acpi_id = acpi_match_device(ACPI_PTR(ds1307_acpi_ids),
> + &client->dev);
> + if (!acpi_id)
> + return -ENODEV;
> + chip = &chips[acpi_id->driver_data];
> + ds1307->type = acpi_id->driver_data;
> + }
>
> - if (!pdata && client->dev.of_node)
> - ds1307_trickle_of_init(client, chip);
> - else if (pdata && pdata->trickle_charger_setup)
> + if (!pdata)
> + ds1307_trickle_init(client, chip);
> + else if (pdata->trickle_charger_setup)
> chip->trickle_charger_setup = pdata->trickle_charger_setup;
>
> if (chip->trickle_charger_setup && chip->trickle_charger_reg) {
> @@ -1678,6 +1712,7 @@ static int ds1307_remove(struct i2c_client *client)
> static struct i2c_driver ds1307_driver = {
> .driver = {
> .name = "rtc-ds1307",
> + .acpi_match_table = ACPI_PTR(ds1307_acpi_ids),
> },
> .probe = ds1307_probe,
> .remove = ds1307_remove,
> --
> 1.7.1
>
Hi ,
Any update for this patch ?
Thank you ,
Tin
--
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* Re: [PATCH v3 0/7] mux controller abstraction and iio/i2c muxes
From: Jonathan Cameron @ 2016-12-03 10:11 UTC (permalink / raw)
To: Peter Rosin, Lars-Peter Clausen, linux-kernel
Cc: Wolfram Sang, Rob Herring, Mark Rutland, Hartmut Knaack,
Peter Meerwald-Stadler, Jonathan Corbet, Arnd Bergmann,
Greg Kroah-Hartman, linux-i2c, devicetree, linux-iio, linux-doc
In-Reply-To: <f649f482-5c1c-dd07-161a-556a9c2b31ff@axentia.se>
On 29/11/16 16:02, Peter Rosin wrote:
> On 2016-11-27 13:00, Jonathan Cameron wrote:
>> On 23/11/16 11:47, Peter Rosin wrote:
>>> On 2016-11-22 21:58, Lars-Peter Clausen wrote:
>>>> On 11/21/2016 02:17 PM, Peter Rosin wrote:
>>>> [...]
>>>>> I have a piece of hardware that is using the same 3 GPIO pins
>>>>> to control four 8-way muxes. Three of them control ADC lines
>>>>> to an ADS1015 chip with an iio driver, and the last one
>>>>> controls the SDA line of an i2c bus. We have some deployed
>>>>> code to handle this, but you do not want to see it or ever
>>>>> hear about it. I'm not sure why I even mention it. Anyway,
>>>>> the situation has nagged me to no end for quite some time.
>>>>>
>>>>> So, after first getting more intimate with the i2c muxing code
>>>>> and later discovering the drivers/iio/inkern.c file and
>>>>> writing a couple of drivers making use of it, I came up with
>>>>> what I think is an acceptable solution; add a generic mux
>>>>> controller driver (and subsystem) that is shared between all
>>>>> instances, and combine that with an iio mux driver and a new
>>>>> generic i2c mux driver. The new i2c mux I called "simple"
>>>>> since it is only hooking the i2c muxing and the new mux
>>>>> controller (much like the alsa simple card driver does for ASoC).
>>>>
>>>> While abstracting this properly is all nice and good and the way it should
>>>> be done, but it also adds a lot of complexity and the devicetree adds a lot
>>>> of restrictions on what can actually be represented.
>>>
>>> This is a characterization without any specifics. But is the
>>> characterization true? You have two complaints, complexity
>>> and restrictions with bindings.
>>>
>>>> There is a certain point where the fabric on a PCB becomes so complex that
>>>> it deserves to be a device on its own (like the audio fabric drivers).
>>>> Especially when the hardware is built with a certain application in mind and
>>>> the driver is supposed to impose policy which reflects this application. The
>>>> latter can often not properly be described with the primitives the
>>>> devicetree can offer.
>>>>
>>>> And I think your setup is very borderline what can be done in a declarative
>>>> way only and it adds a lot of complexity over a more imperative solution in
>>>> form of a driver. I think it is worth investigating about having a driver
>>>> that is specific to your fabric and handles the interdependencies of the
>>>> discrete components.
>>>
>>> So, there are three "new" concepts:
>>>
>>> 1. Sticking a mux in front of an AD-converter. That's not all that
>>> novel, nor complex. Quite the opposite, I'd say. In fact, I find it
>>> a bit amazing that there is no in-kernel support for it.
>> As ever first person who needs it and has the skills to write it gets to do it ;)
>> Congratulations Peter ;)
>>>
>>> 2. Reusing the same GPIO-pins to drive different muxes. There are
>>> obviously chips that work this way (as Jonathan pointed out) and
>>> these will at some point get used in Linux devices. I guess they
>>> already are used, but that people handle them in userspace. Or
>>> something? If this is complex, which I question, it will still need
>>> support at some point. At least that's what I believe.
>>>
>>> 3. Using the same GPIO pins to mux things handled by different
>>> subsystems. Right, this is a bit crazy, and I'd rather not have this
>>> requirement, but this HW is what it is so I'll need to handle it in
>>> some way. It is also what stops me from falling back to a userspace
>>> solution, which is probably connected to why #1 and #2 is not supported
>>> by the kernel; everybody probably does muxing in userspace. Which is
>>> not necessarily a good idea, nor how it's supposed to be done...
>>>
>>> So, the only thing that's out of the ordinary (as I see it), is #3.
>>> The question that then needs an answer is how the in-kernel solution
>>> for #1 and #2 would look if we do not consider #3.
>>>
>>> And I claim that the desired solution to #1 and #2 is pretty close
>>> to my proposal.
>>>
>>> A. You do not want mux-controller drivers in every subsystem that
>>> needs them.
>> Agreed.
>>>
>>> B. You do not want every mux-consumer to know the specifics of how to
>>> operate every kind of mux; there are muxes that are not controlled
>>> with GPIO pins...
>>>
>>> C. When you implement muxing in a subsystem, there will in some cases
>>> be a need to handle parallel muxing, where there is a need to share
>>> mux-controllers.
>>>
>>> It just feels right to separate out the mux-controller and refer to
>>> it from where a mux is needed. It solves #1 and #2. And, of course,
>>> as a bonus #3 is also solved. But my bias is obvious.
>>>
>>> And that leads us to the restrictions with the bindings. And the same
>>> thing happens; the solution for #2 also solves #3.
>>>
>>> So how do you refer to a mux-controller from where it's needed? My
>>> first proposal used a dt phandle, for the second round I put them in
>>> the parent node. It would be super if it was possible for the mux-
>>> consumer to create the mux-controller device from the same dt
>>> node that is already bound to the mux-consumer. The problem is that
>>> the mux-consumer should not hard-code which mux-controller device it
>>> should create. The best I can think of is some kind of 'mux-compatible'
>>> attribute, that works like the standard 'compatible' attribute. That
>>> would simplify the bindings for the normal case (#1) where the mux-
>>> controller isn't shared (#2 and #3). Maybe it's possible to fix this
>>> issue somehow? I simply don't know?
>> As Lars stated, it's marginal. The question is not at what point do we
>> 'have to' bother with a fabric driver, but rather at what point does it
>> make a our lives easier.
>>
>> Take you nastiest mux case described earlier.
>> The ideal would be to represent the ADC and 3 muxes as (approximately) a
>> single ADC to userspace that just happens to have somewhere near 23 inputs.
>>
>> To do that in device tree we need to describe
>>
>> 1 The adc
>> 2 The three muxes
>> 3 The software representation to pull all of these back into a single device.
>>
>> That last part to my mind trips the balance to the point where a fabric driver
>> would make sense. It's not complex. Just a few lines of code tying all the
>> elements together without ending up with a fairly fiendish setup to describe in
>> device tree.
>>
>> Also just wait until we have muxes stacked on muxes, with cross overs occuring.
>> Some of the ASoC parts can actually have effective loops if you try all the mux
>> combinations.
>>
>> So question is do we have a 'simple case description' in device tree or force
>> fabric drivers everywhere? I think I'm in favour of the simple case - which handles
>> one of your two uses nicely. The second one to do the the recombining of channels after
>> the muxes, ends up looking to me like it needs a fabric driver.
>>
>> Note we are only talking about bindings vs code based description here. I agree
>> entirely with the concept of a generic mux subsystem.
>
> Ok, take the simple case - an adc with a mux in front of it.
>
> We do not want to build a whole new iio-mux subsystem like the one under
> i2c, so from iio we want to refer to the actual mux controller driver
> which lives under the mux subsystem. Getting this far is what solves my
> "second need" [2] from the v2 thread.
>
> Agreed, doing this w/o a fabric driver spills the guts and it might be
> cleaner to build a fabric driver that takes a reference to the dpot and
> the mux controller and just knows the rest. In the end this fabric driver
> requires two things to actually make a difference; some way to instantiate
> drivers without the help of device-tree and some way make those drivers
> only provide in-kernel access (and preferably it should hide the dpot from
> userspace too, while at it).
>
> But doing all that in a fabric driver is not going to change the fact that
> the iio-mux driver is useful on its own. I bet someone else is going to
> reuse it somewhere down the line. Which means that a fabric driver would
> perhaps be nice for my "second need", but not critical, it works pretty
> well to just piggyback on the general solution .
Absolutely. No denying the usefulness of having a nice little mux subsystem
with the resulting iio-mux driver.
>
> Over to my "first need" [1] from the v2 thread.
>
> The above iio-mux driver handles the three muxed adc lines beautifully,
> just refer all three iio-muxes to the same mux controller. Agreed, with
> a fabric driver I could get one device with 25 channels instead of three
> devices with 8 channels each plus one unmuxed line directly from the adc
> device. However, that doesn't bother me at all, I may even think it is
> preferable because otherwise I'd have to come up with some way to
> identify which channel is which in that big 25-channel jungle. Another
> drawback with having a fabric driver here is that it would need to be an
> i2c-mux driver, because one of the key points of doing a fabric driver
> for my first need was to hide the shared mux, right? Instead, I have the
> new i2c-mux-simple driver that builds an i2c-mux using any mux controller
> from the mux subsystem (something that may prove useful to others in the
> future), which in my case is the same mux controller that also muxes the
> three adc lines.
>
> In short, doing fabric drivers buys me almost nothing besides having a
> slightly more distinct device tree. All the components used to describe
> this are either already accepted drivers or usable by others (at least
> the way I see it).
Fair enough. Perhaps it's not worthwhile in this case, but lets keep
the concept in mind as we move forward and see if anyone else has
more complex hardware than you do ;)
(there is always something out there!)
>
> Cheers,
> Peter
>
> [1] three adc lines and an i2c bus muxed with the same gpio pins
> [2] mcp4561 dpot -> dpot-dac -> envelope-detector-adc -> iio-mux
>
^ permalink raw reply
* Re: [PATCH v4 i2c/for-next] i2c: rcar: Add per-Generation fallback bindings
From: Geert Uytterhoeven @ 2016-12-02 14:59 UTC (permalink / raw)
To: Simon Horman
Cc: Wolfram Sang, Magnus Damm, Linux I2C, Linux-Renesas, Rob Herring,
devicetree@vger.kernel.org
In-Reply-To: <1480688736-7175-1-git-send-email-horms+renesas@verge.net.au>
On Fri, Dec 2, 2016 at 3:25 PM, Simon Horman <horms+renesas@verge.net.au> wrote:
> In the case of Renesas R-Car hardware we know that there are generations of
> SoCs, e.g. Gen 2 and Gen 3. But beyond that it's not clear what the
> relationship between IP blocks might be. For example, I believe that
> r8a7790 is older than r8a7791 but that doesn't imply that the latter is a
> descendant of the former or vice versa.
>
> We can, however, by examining the documentation and behaviour of the
> hardware at run-time observe that the current driver implementation appears
> to be compatible with the IP blocks on SoCs within a given generation.
>
> For the above reasons and convenience when enabling new SoCs a
> per-generation fallback compatibility string scheme is being adopted for
> drivers for Renesas SoCs.
>
> Also:
> * Deprecate renesas,i2c-rcar. It seems poorly named as it is only
> compatible with R-Car Gen 1. It also appears unused in mainline.
> * Add some text to describe per-SoC bindings
>
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> --- a/Documentation/devicetree/bindings/i2c/i2c-rcar.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c-rcar.txt
> + "renesas,i2c-r8a7779" if the device is a part of a R8A7797 SoC.
R8A7779
Sorry for not noticing before.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* [PATCH v4 i2c/for-next] i2c: rcar: Add per-Generation fallback bindings
From: Simon Horman @ 2016-12-02 14:25 UTC (permalink / raw)
To: Wolfram Sang
Cc: Magnus Damm, linux-i2c, linux-renesas-soc, Rob Herring,
devicetree, Simon Horman
In the case of Renesas R-Car hardware we know that there are generations of
SoCs, e.g. Gen 2 and Gen 3. But beyond that it's not clear what the
relationship between IP blocks might be. For example, I believe that
r8a7790 is older than r8a7791 but that doesn't imply that the latter is a
descendant of the former or vice versa.
We can, however, by examining the documentation and behaviour of the
hardware at run-time observe that the current driver implementation appears
to be compatible with the IP blocks on SoCs within a given generation.
For the above reasons and convenience when enabling new SoCs a
per-generation fallback compatibility string scheme is being adopted for
drivers for Renesas SoCs.
Also:
* Deprecate renesas,i2c-rcar. It seems poorly named as it is only
compatible with R-Car Gen 1. It also appears unused in mainline.
* Add some text to describe per-SoC bindings
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
v4
* Correct grammar in changelog
v3
* Consistently use renesas,<family>-<module> for new compat strings
* Drop RFC designation
v2
* Include accidently omitted i2c-rcar.c portion of patch
---
Documentation/devicetree/bindings/i2c/i2c-rcar.txt | 32 ++++++++++++++--------
drivers/i2c/busses/i2c-rcar.c | 5 +++-
2 files changed, 24 insertions(+), 13 deletions(-)
diff --git a/Documentation/devicetree/bindings/i2c/i2c-rcar.txt b/Documentation/devicetree/bindings/i2c/i2c-rcar.txt
index 239632a0d709..50c378ccb8e7 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-rcar.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-rcar.txt
@@ -1,17 +1,25 @@
I2C for R-Car platforms
Required properties:
-- compatible: Must be one of
- "renesas,i2c-rcar"
- "renesas,i2c-r8a7778"
- "renesas,i2c-r8a7779"
- "renesas,i2c-r8a7790"
- "renesas,i2c-r8a7791"
- "renesas,i2c-r8a7792"
- "renesas,i2c-r8a7793"
- "renesas,i2c-r8a7794"
- "renesas,i2c-r8a7795"
- "renesas,i2c-r8a7796"
+- compatible:
+ "renesas,i2c-r8a7778" if the device is a part of a R8A7778 SoC.
+ "renesas,i2c-r8a7779" if the device is a part of a R8A7797 SoC.
+ "renesas,i2c-r8a7790" if the device is a part of a R8A7790 SoC.
+ "renesas,i2c-r8a7791" if the device is a part of a R8A7791 SoC.
+ "renesas,i2c-r8a7792" if the device is a part of a R8A7792 SoC.
+ "renesas,i2c-r8a7793" if the device is a part of a R8A7793 SoC.
+ "renesas,i2c-r8a7794" if the device is a part of a R8A7794 SoC.
+ "renesas,i2c-r8a7795" if the device is a part of a R8A7795 SoC.
+ "renesas,i2c-r8a7796" if the device is a part of a R8A7796 SoC.
+ "renesas,rcar-gen1-i2c" for a generic R-Car Gen1 compatible device.
+ "renesas,rcar-gen2-i2c" for a generic R-Car Gen2 compatible device.
+ "renesas,rcar-gen3-i2c" for a generic R-Car Gen3 compatible device.
+ "renesas,i2c-rcar" (deprecated)
+
+ When compatible with the generic version, nodes must list the
+ SoC-specific version corresponding to the platform first followed
+ by the generic version.
+
- reg: physical base address of the controller and length of memory mapped
region.
- interrupts: interrupt specifier.
@@ -33,7 +41,7 @@ Examples :
i2c0: i2c@e6508000 {
#address-cells = <1>;
#size-cells = <0>;
- compatible = "renesas,i2c-r8a7791";
+ compatible = "renesas,i2c-r8a7791", "renesas,rcar-gen2-i2c";
reg = <0 0xe6508000 0 0x40>;
interrupts = <0 287 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&mstp9_clks R8A7791_CLK_I2C0>;
diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index 726615e54f2a..622def6b43e2 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -793,7 +793,6 @@ static const struct i2c_algorithm rcar_i2c_algo = {
};
static const struct of_device_id rcar_i2c_dt_ids[] = {
- { .compatible = "renesas,i2c-rcar", .data = (void *)I2C_RCAR_GEN1 },
{ .compatible = "renesas,i2c-r8a7778", .data = (void *)I2C_RCAR_GEN1 },
{ .compatible = "renesas,i2c-r8a7779", .data = (void *)I2C_RCAR_GEN1 },
{ .compatible = "renesas,i2c-r8a7790", .data = (void *)I2C_RCAR_GEN2 },
@@ -803,6 +802,10 @@ static const struct of_device_id rcar_i2c_dt_ids[] = {
{ .compatible = "renesas,i2c-r8a7794", .data = (void *)I2C_RCAR_GEN2 },
{ .compatible = "renesas,i2c-r8a7795", .data = (void *)I2C_RCAR_GEN3 },
{ .compatible = "renesas,i2c-r8a7796", .data = (void *)I2C_RCAR_GEN3 },
+ { .compatible = "renesas,i2c-rcar", .data = (void *)I2C_RCAR_GEN1 }, // Deprecated
+ { .compatible = "renesas,rcar-gen1-i2c", .data = (void *)I2C_RCAR_GEN1 },
+ { .compatible = "renesas,rcar-gen2-i2c", .data = (void *)I2C_RCAR_GEN2 },
+ { .compatible = "renesas,rcar-gen3-i2c", .data = (void *)I2C_RCAR_GEN3 },
{},
};
MODULE_DEVICE_TABLE(of, rcar_i2c_dt_ids);
--
2.7.0.rc3.207.g0ac5344
^ permalink raw reply related
* [PULL REQUEST] i2c for 4.9
From: Wolfram Sang @ 2016-12-02 13:21 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-i2c, linux-kernel, Jan Glauber
[-- Attachment #1: Type: text/plain, Size: 970 bytes --]
Linus,
here is the revert for the regression of the i2c-octeon driver I
mentioned last time. I wished for a bit more feedback, but all people
working actively on it are in need of this patch, so here it goes.
Please pull,
Wolfram
The following changes since commit e5517c2a5a49ed5e99047008629f1cd60246ea0e:
Linux 4.9-rc7 (2016-11-27 13:08:04 -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 dfa2ccc30e6556bd526f54f0e16fc9e5af4293cb:
Revert "i2c: octeon: thunderx: Limit register access retries" (2016-11-29 20:04:21 +0100)
----------------------------------------------------------------
Jan Glauber (1):
Revert "i2c: octeon: thunderx: Limit register access retries"
drivers/i2c/busses/i2c-octeon-core.c | 4 +---
drivers/i2c/busses/i2c-octeon-core.h | 27 +++++++++++----------------
2 files changed, 12 insertions(+), 19 deletions(-)
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* [wsa:renesas/thermal-on-4.10-next 2/4] rcar_gen3_thermal.c:undefined reference to `__divdi3'
From: kbuild test robot @ 2016-12-02 8:42 UTC (permalink / raw)
To: Wolfram Sang; +Cc: kbuild-all, linux-i2c
[-- Attachment #1: Type: text/plain, Size: 1326 bytes --]
tree: https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/thermal-on-4.10-next
head: 5c2f1bfbcb5345890559511c6a59cfca40fdc05e
commit: 00a445b11c2266cb05771ab01f691bb585e34ebb [2/4] thermal: rcar_gen3_thermal: Add R-Car Gen3 thermal driver
config: i386-allyesconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
git checkout 00a445b11c2266cb05771ab01f691bb585e34ebb
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
drivers/built-in.o: In function `rcar_gen3_thermal_get_temp':
>> rcar_gen3_thermal.c:(.text+0x2255458): undefined reference to `__divdi3'
rcar_gen3_thermal.c:(.text+0x225548a): undefined reference to `__divdi3'
drivers/built-in.o: In function `rcar_gen3_thermal_probe':
rcar_gen3_thermal.c:(.text+0x225564c): undefined reference to `__divdi3'
rcar_gen3_thermal.c:(.text+0x2255677): undefined reference to `__divdi3'
rcar_gen3_thermal.c:(.text+0x22556b7): undefined reference to `__divdi3'
drivers/built-in.o:rcar_gen3_thermal.c:(.text+0x22556e6): more undefined references to `__divdi3' follow
---
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: 56223 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