Re: [PATCH RESEND] i2c/nomadik: runtime PM support
On Tue, Apr 17, 2012 at 11:39 PM, Wolfram Sang w.s...@pengutronix.de wrote: From: Jonas Aaberg jonas.ab...@stericsson.com Turn off the clock and regulator to the I2C block using runtime PM. Cc: Magnus Damm magnus.d...@gmail.com Cc: Rafael J. Wysocki r...@sisk.pl Cc: Mark Brown broo...@opensource.wolfsonmicro.com Signed-off-by: Jonas Aaberg jonas.ab...@stericsson.com Signed-off-by: Linus Walleij linus.wall...@linaro.org Dooks, are you OK with merging this patch now? As I understood, Mark acked it? I didn't dive into the details, so it would be nice for me to have an ACK (or at least a not NACK) from Magnus. I'm a bit hesitant to ack because the runtime suspend/resume callbacks are used differently than we would do on arch/sh and arch/arm/mach-shmobile. This difference may or may not be a good thing. I'm afraid I know too little about the nomadik platform to say anything wise. =) Our drivers assume that the -runtime_suspend() and -runtime_resume() callbacks are executed before/after the power is turned off/on for a certain power domain. The way they are used in this patch more seems like they are expected to be called regardless of the state of the rest of the devices sharing the underlying power domain. You probably want to control the clocks and the regulators directly - independently of the rest of the devices.You may want to look into struct gpd_dev_ops for various ways to interface to the pm domain code. Exactly what is a good fit depends on how the underlying SoC code maps the runtime pm clock and domain code to the actual hardware. Cheers, / magnus -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv8 04/18] I2C: OMAP: I2C register restore only if context is lost
Hi Kevin,
Hi Kevin,
Thanks for your review,
On Tue, Apr 17, 2012 at 7:15 PM, Kevin Hilman khil...@ti.com wrote:
Shubhrajyoti D shubhrajy...@ti.com writes:
Currently i2c register restore is done always.
Adding conditional restore.
The i2c register restore is done only if the context is lost.
OK, minor comment below.
Thanks for the suggestion of the error case restore.
Updated the patch. Also added Tony's ack for the plat part.
From 1ca222f6f50868e07d9a46575e73dd66fd2d542e Mon Sep 17 00:00:00 2001
From: Shubhrajyoti D shubhrajy...@ti.com
Date: Tue, 17 Jan 2012 16:13:03 +0530
Subject: [PATCH] I2C: OMAP: I2C register restore only if context is lost
Currently i2c register restore is done always.
Adding conditional restore.
The i2c register restore is done only if the context is lost
or in case of error to be on the safe side.
Also remove the definition of SYSS_RESETDONE_MASK and use the
one in omap_hwmod.h.
Cc: Kevin Hilman khil...@ti.com
[For the arch/arm/plat-omap/i2c.c part]
Acked-by: Tony Lindgren t...@atomide.com
Signed-off-by: Shubhrajyoti D shubhrajy...@ti.com
---
arch/arm/plat-omap/i2c.c |3 ++
drivers/i2c/busses/i2c-omap.c | 53 +++--
include/linux/i2c-omap.h |1 +
3 files changed, 39 insertions(+), 18 deletions(-)
diff --git a/arch/arm/plat-omap/i2c.c b/arch/arm/plat-omap/i2c.c
index db071bc..4ccab07 100644
--- a/arch/arm/plat-omap/i2c.c
+++ b/arch/arm/plat-omap/i2c.c
@@ -179,6 +179,9 @@ static inline int omap2_i2c_add_bus(int bus_id)
*/
if (cpu_is_omap34xx())
pdata-set_mpu_wkup_lat = omap_pm_set_max_mpu_wakeup_lat_compat;
+
+ pdata-get_context_loss_count = omap_pm_get_dev_context_loss_count;
+
pdev = omap_device_build(name, bus_id, oh, pdata,
sizeof(struct omap_i2c_bus_platform_data),
NULL, 0, 0);
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index a882558..76cf066 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -43,6 +43,7 @@
#include linux/slab.h
#include linux/i2c-omap.h
#include linux/pm_runtime.h
+#include plat/omap_device.h
/* I2C controller revisions */
#define OMAP_I2C_OMAP1_REV_2 0x20
@@ -157,9 +158,6 @@ enum {
#define OMAP_I2C_SYSTEST_SDA_I (1 1)/* SDA line sense in */
#define OMAP_I2C_SYSTEST_SDA_O (1 0)/* SDA line drive out */
-/* OCP_SYSSTATUS bit definitions */
-#define SYSS_RESETDONE_MASK(1 0)
-
/* OCP_SYSCONFIG bit definitions */
#define SYSC_CLOCKACTIVITY_MASK(0x3 8)
#define SYSC_SIDLEMODE_MASK(0x3 3)
@@ -184,6 +182,7 @@ struct omap_i2c_dev {
u32 latency;/* maximum mpu wkup latency */
void(*set_mpu_wkup_lat)(struct device *dev,
long latency);
+ int (*get_context_loss_count)(struct device *dev);
u32 speed; /* Speed of bus in kHz */
u32 dtrev; /* extra revision from DT */
u32 flags;
@@ -206,6 +205,7 @@ struct omap_i2c_dev {
u16 syscstate;
u16 westate;
u16 errata;
+ int dev_lost_count;
};
static const u8 reg_map_ip_v1[] = {
@@ -1025,6 +1025,7 @@ omap_i2c_probe(struct platform_device *pdev)
dev-speed = pdata-clkrate;
dev-flags = pdata-flags;
dev-set_mpu_wkup_lat = pdata-set_mpu_wkup_lat;
+ dev-get_context_loss_count = pdata-get_context_loss_count;
dev-dtrev = pdata-rev;
}
@@ -1151,12 +1152,32 @@ omap_i2c_remove(struct platform_device *pdev)
}
#ifdef CONFIG_PM_RUNTIME
+static void omap_i2c_restore(struct omap_i2c_dev *dev)
+{
+ omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, 0);
+ omap_i2c_write_reg(dev, OMAP_I2C_PSC_REG, dev-pscstate);
+ omap_i2c_write_reg(dev, OMAP_I2C_SCLL_REG, dev-scllstate);
+ omap_i2c_write_reg(dev, OMAP_I2C_SCLH_REG, dev-sclhstate);
+ omap_i2c_write_reg(dev, OMAP_I2C_BUF_REG, dev-bufstate);
+ omap_i2c_write_reg(dev, OMAP_I2C_WE_REG, dev-westate);
+ omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, OMAP_I2C_CON_EN);
+ /*
+* Don't write to this register if the IE state is 0 as it can
+* cause deadlock.
+*/
+ if (dev-iestate)
+ omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, dev-iestate);
+
+}
static int omap_i2c_runtime_suspend(struct device *dev)
{
struct platform_device *pdev = to_platform_device(dev);
struct omap_i2c_dev *_dev = platform_get_drvdata(pdev);
u16 iv;
+ if (_dev-get_context_loss_count)
+ _dev-dev_lost_count = _dev-get_context_loss_count(dev);
+
Re: decode-dimms: Module Configuration Type not reported for DDR2/3
Hi Andriy,
On Tue, 17 Apr 2012 23:03:16 +0300, Andriy Gapon wrote:
on 17/04/2012 14:18 Jean Delvare said the following:
I am attaching two patches, one for DDR2, one for DDR3, please give
them a try.
Thanks a lot!
I am able to test only the DDR2 part at the moment.
With the patch I am getting the following
Module Configuration Type Data ECC
for these modules: http://www.valueram.com/datasheets/kvr800d2e5_2g.pdf
For some definitely non-ECC modules I get the expected
Module Configuration Type No Parity
Thanks for the report, I've applied both patches.
P.S.
I've noticed that the Decoding EEPROM message is printed with
$dimm[$current]-{eeprom} when $opt_side_by_side is true but it is printed
with
$dimm[$current]-{file} in the other case. Not sure if this is on purpose.
I think it is on purpose. -{file} contains the full path so it is
rather long, -{eeprom} only contains the file name so it is much
shorter. The full path would make side-by-side output very wide, that
wouldn't be convenient, that's why the short name is used in this
output mode. In legacy output mode, the long name fits.
We _could_ use the short name in legacy output mode, as the long name
doesn't necessarily add a lot of value. I think I tried to leave the
legacy output unchanged when implementing the side-by-side output, to
avoid any regression in case someone was parsing the output of
decode-dimms. If you believe it would be better to always use the short
name, we could do that, this may even allow for a small code clean-up.
--
Jean Delvare
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] i2c-mpc: avoid I2C abnormal after resuming from deep sleep
On Wed, Feb 29, 2012 at 07:45:23PM +0100, Wolfram Sang wrote: Hi, On Wed, Feb 29, 2012 at 06:39:21PM +0800, Zhao Chenhui wrote: When entering deep sleep, the value in the registers I2CFDR and I2CDFSRR are lost. This causes I2C access to fail after resuming. Add suspend/resume routines to save/restore the registers I2CFDR and I2CDFSRR. Signed-off-by: Zhao Chenhui chenhui.z...@freescale.com Signed-off-by: Li Yang le...@freescale.com --- drivers/i2c/busses/i2c-mpc.c | 34 +- 1 files changed, 33 insertions(+), 1 deletions(-) diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c index a8ebb84..f95a647 100644 --- a/drivers/i2c/busses/i2c-mpc.c +++ b/drivers/i2c/busses/i2c-mpc.c @@ -1,7 +1,9 @@ /* * (C) Copyright 2003-2004 * Humboldt Solutions Ltd, adr...@humboldt.co.uk. - + * + * Copyright 2012 Freescale Semiconductor, Inc. + * I'd think the change is too trivial to claim copyright on the file. Do you want to resend with the comments addressed? We have a company policy for adding copyright claims. And this patch has met our criteria that requires adding copyright. We certainly don't want the policy to be against the common practice of the community. If it is we can consider changing our policy. However I can't find a clear message stating the consensus of what the Linux kernel community think of adding copyright claims. I even found some message that encourage adding copyright to maintain the GPL license. - Leo -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c-mpc: avoid I2C abnormal after resuming from deep sleep
Hi, We certainly don't want the policy to be against the common practice of the community. If it is we can consider changing our policy. Nice, thanks for being open to discussion! However I can't find a clear message stating the consensus of what the Linux kernel community think of adding copyright claims. I even found Well, what I know, the consensus is common sense. What is obviously not clear in a way you could write it down. At least, Ben agrees. some message that encourage adding copyright to maintain the GPL license. I am interested. Do you have a link? Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH v2 0/2] i2c: Don't assume bus nr 0 if none was specified
On Tue, Mar 27, 2012 at 11:10:26AM +0200, Karol Lewandowski wrote: Changes since v1: - Dropped reduntant i2c-octeon change i2c controller drivers used to assume bus number 0 when none (-1) was specified. This worked on non-device tree systems, where one could explicitly specify bus number via platform data. On DT-enabled systems bus number is always -1. This patchset reworks few remaining drivers to use dynamic bus allocation when no id has been provided. Thanks, both applied. -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH 1/3] i2c-s3c2410: Drop unused define
On Wed, Mar 21, 2012 at 08:11:51PM +0100, Karol Lewandowski wrote: Use standard of_match_ptr() to avoid defining variable unused in non device tree builds. Signed-off-by: Karol Lewandowski k.lewando...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com Acked-by: Grant Likely grant.lik...@secretlab.ca I applied this one already. -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH v2] i2c: designware: Add support for 16bit register access
On Wed, Apr 18, 2012 at 09:33:19AM +0200, Stefan Roese wrote:
The STM SPEAr platform can only access the i2c controller register
via 16bit read/write functions. This patch adds support to
automatically detect this 16bit access mode.
Signed-off-by: Stefan Roese s...@denx.de
Thanks for the update, looks mostly good.
Cc: Wolfram Sang w.s...@pengutronix.de
Cc: Viresh Kumar viresh.ku...@st.com
---
v2:
- Removed parenthesis for single-statement block
- Moved swab and access_16bit into accessor_flags
drivers/i2c/busses/i2c-designware-core.c | 27 ++-
drivers/i2c/busses/i2c-designware-core.h |5 -
2 files changed, 26 insertions(+), 6 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-core.c
b/drivers/i2c/busses/i2c-designware-core.c
index df87992..b3c5cfa 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -164,9 +164,15 @@ static char *abort_sources[] = {
u32 dw_readl(struct dw_i2c_dev *dev, int offset)
{
- u32 value = readl(dev-base + offset);
+ u32 value;
- if (dev-swab)
+ if (dev-accessor_flags ACCESS_16BIT)
+ value = readw(dev-base + offset) |
+ (readw(dev-base + offset + 2) 16);
+ else
+ value = readl(dev-base + offset);
+
+ if (dev-accessor_flags ACCESS_SWAP)
return swab32(value);
else
return value;
@@ -174,10 +180,15 @@ u32 dw_readl(struct dw_i2c_dev *dev, int offset)
void dw_writel(struct dw_i2c_dev *dev, u32 b, int offset)
{
- if (dev-swab)
+ if (dev-accessor_flags ACCESS_SWAP)
b = swab32(b);
- writel(b, dev-base + offset);
+ if (dev-accessor_flags ACCESS_16BIT) {
+ writew((u16)b, dev-base + offset);
+ writew((u16)(b 16), dev-base + offset + 2);
+ } else {
+ writel(b, dev-base + offset);
+ }
}
static u32
@@ -254,7 +265,13 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
/* Configure register endianess access */
reg = dw_readl(dev, DW_IC_COMP_TYPE);
if (reg == ___constant_swab32(DW_IC_COMP_TYPE_VALUE)) {
- dev-swab = 1;
+ dev-accessor_flags |= ACCESS_SWAP;
+ reg = DW_IC_COMP_TYPE_VALUE;
May I ask you to use proper if/elseif/else blocks instead of overwriting
reg? I know you didn't come up with the mechanism, yet it looks too
fragile to be extended IMO.
+ }
+
+ /* Configure register access mode 16bit */
+ if (reg == (DW_IC_COMP_TYPE_VALUE 0x)) {
Does it make sense to check reg + 2 for the upper part of the signature?
+ dev-accessor_flags |= ACCESS_16BIT;
reg = DW_IC_COMP_TYPE_VALUE;
}
diff --git a/drivers/i2c/busses/i2c-designware-core.h
b/drivers/i2c/busses/i2c-designware-core.h
index 02d1a2d..9c1840e 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -82,7 +82,7 @@ struct dw_i2c_dev {
unsigned intstatus;
u32 abort_source;
int irq;
- int swab;
+ u32 accessor_flags;
struct i2c_adapter adapter;
u32 functionality;
u32 master_cfg;
@@ -90,6 +90,9 @@ struct dw_i2c_dev {
unsigned intrx_fifo_depth;
};
+#define ACCESS_SWAP 0x0001
+#define ACCESS_16BIT 0x0002
+
extern u32 dw_readl(struct dw_i2c_dev *dev, int offset);
extern void dw_writel(struct dw_i2c_dev *dev, u32 b, int offset);
extern int i2c_dw_init(struct dw_i2c_dev *dev);
--
1.7.10
Thanks,
Wolfram
--
Pengutronix e.K. | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/ |
signature.asc
Description: Digital signature
Re: [PATCH 2/3] i2c-s3c2410: Rework device type handling
On 17.04.2012 19:31, Wolfram Sang wrote:
Hi,
Hi Wolfram!
On Wed, Mar 21, 2012 at 08:11:52PM +0100, Karol Lewandowski wrote:
Reorganize driver a bit to better handle device tree-based systems:
- move machine type to driver's private structure instead of
quering platform device variants in runtime
- replace s3c24xx_i2c_type enum with unsigned int that holds
bitmask with revision-specific quirks
Signed-off-by: Karol Lewandowski k.lewando...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
Okay, so this driver needs to the 'data' field from either
platform_device_id or of_device_id and implements a function for that,
namely s3c24xx_get_device_quirks(). Grant, Rob: I'd think there might be
more drivers in need of that, so maybe it makes sense to have a generic
of-helper function?
---
drivers/i2c/busses/i2c-s3c2410.c | 47
++---
1 files changed, 23 insertions(+), 24 deletions(-)
diff --git a/drivers/i2c/busses/i2c-s3c2410.c
b/drivers/i2c/busses/i2c-s3c2410.c
index 85e3664..f7b6a14 100644
--- a/drivers/i2c/busses/i2c-s3c2410.c
+++ b/drivers/i2c/busses/i2c-s3c2410.c
@@ -44,8 +44,14 @@
#include plat/regs-iic.h
#include plat/iic.h
-/* i2c controller state */
+#ifdef CONFIG_OF
+static const struct of_device_id s3c24xx_i2c_match[];
+#endif
Uh, forward declaration with #ifdef. I'd think we should get this simply
to the front.
Ok, as I think it's better to have dt and non-dt definitions together
I'll move both of_device_id and platform_device_id to the top.
+/* Treat S3C2410 as baseline hardware, anything else is supported via
quirks */
+#define QUIRK_S3C2440 (1 0)
Minor: Is it really a quirk being s3c2440? Maybe FLAG_*, dunno.
In first version[1] of this patch I've used TYPEs for device types
and FLAGS for quirks. However, I've squashed these into quirks after
discussion with Mark[2].
[1] http://permalink.gmane.org/gmane.linux.kernel/1266759
[2] http://permalink.gmane.org/gmane.linux.kernel.samsung-soc/10255
-static inline int s3c24xx_i2c_is2440(struct s3c24xx_i2c *i2c)
+static inline unsigned int s3c24xx_get_device_quirks(struct platform_device
*pdev)
{
-struct platform_device *pdev = to_platform_device(i2c-dev);
-enum s3c24xx_i2c_type type;
-
#ifdef CONFIG_OF
-if (i2c-dev-of_node)
-return of_device_is_compatible(i2c-dev-of_node,
-samsung,s3c2440-i2c);
+if (pdev-dev.of_node) {
+const struct of_device_id *match;
+match = of_match_node(s3c24xx_i2c_match[0], pdev-dev.of_node);
Minor: I think it is more readable to drop the [0]
I prefer explicit version, but I'll drop [] as both you and Thomas
found implicit version more readable.
[ I'll also drop above CONFIG_OF ifdef, as of_match_node() seem
to be always defined since v3.2-rc1. ]
Thanks for review! I'll resubmit updated version shortly.
Regards,
--
Karol Lewandowski | Samsung Poland RD Center | Linux/Platform
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] i2c: designware: Add support for 16bit register access
On Wednesday 18 April 2012 13:24:26 Wolfram Sang wrote:
snip
@@ -254,7 +265,13 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
/* Configure register endianess access */
reg = dw_readl(dev, DW_IC_COMP_TYPE);
if (reg == ___constant_swab32(DW_IC_COMP_TYPE_VALUE)) {
- dev-swab = 1;
+ dev-accessor_flags |= ACCESS_SWAP;
+ reg = DW_IC_COMP_TYPE_VALUE;
May I ask you to use proper if/elseif/else blocks instead of overwriting
reg? I know you didn't come up with the mechanism, yet it looks too
fragile to be extended IMO.
Okay, will do.
+ }
+
+ /* Configure register access mode 16bit */
+ if (reg == (DW_IC_COMP_TYPE_VALUE 0x)) {
Does it make sense to check reg + 2 for the upper part of the signature?
Not from my point of view. Its very unlikely that this driver will be used on
another controller. Its configured (via its address in platform-data or DT)
for exactly this controller. And matching 16bit of this 32bit register seems
enough for me. I would even have no problem to remove this check completely.
Thanks,
Stefan
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] i2c: designware: Add support for 16bit register access
The STM SPEAr platform can only access the i2c controller register
via 16bit read/write functions. This patch adds support to
automatically detect this 16bit access mode.
Signed-off-by: Stefan Roese s...@denx.de
---
v3:
- Changed multiple if statements into one if/elseif/etc statement
to remove the reg overwriting abuse.
v2:
- Removed parenthesis for single-statement block
- Moved swab and access_16bit into accessor_flags
drivers/i2c/busses/i2c-designware-core.c | 31 --
drivers/i2c/busses/i2c-designware-core.h |5 -
2 files changed, 25 insertions(+), 11 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-core.c
b/drivers/i2c/busses/i2c-designware-core.c
index df87992..1e48bec 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -164,9 +164,15 @@ static char *abort_sources[] = {
u32 dw_readl(struct dw_i2c_dev *dev, int offset)
{
- u32 value = readl(dev-base + offset);
+ u32 value;
- if (dev-swab)
+ if (dev-accessor_flags ACCESS_16BIT)
+ value = readw(dev-base + offset) |
+ (readw(dev-base + offset + 2) 16);
+ else
+ value = readl(dev-base + offset);
+
+ if (dev-accessor_flags ACCESS_SWAP)
return swab32(value);
else
return value;
@@ -174,10 +180,15 @@ u32 dw_readl(struct dw_i2c_dev *dev, int offset)
void dw_writel(struct dw_i2c_dev *dev, u32 b, int offset)
{
- if (dev-swab)
+ if (dev-accessor_flags ACCESS_SWAP)
b = swab32(b);
- writel(b, dev-base + offset);
+ if (dev-accessor_flags ACCESS_16BIT) {
+ writew((u16)b, dev-base + offset);
+ writew((u16)(b 16), dev-base + offset + 2);
+ } else {
+ writel(b, dev-base + offset);
+ }
}
static u32
@@ -251,14 +262,14 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
input_clock_khz = dev-get_clk_rate_khz(dev);
- /* Configure register endianess access */
reg = dw_readl(dev, DW_IC_COMP_TYPE);
if (reg == ___constant_swab32(DW_IC_COMP_TYPE_VALUE)) {
- dev-swab = 1;
- reg = DW_IC_COMP_TYPE_VALUE;
- }
-
- if (reg != DW_IC_COMP_TYPE_VALUE) {
+ /* Configure register endianess access */
+ dev-accessor_flags |= ACCESS_SWAP;
+ } else if (reg == (DW_IC_COMP_TYPE_VALUE 0x)) {
+ /* Configure register access mode 16bit */
+ dev-accessor_flags |= ACCESS_16BIT;
+ } else if (reg != DW_IC_COMP_TYPE_VALUE) {
dev_err(dev-dev, Unknown Synopsys component type:
0x%08x\n, reg);
return -ENODEV;
diff --git a/drivers/i2c/busses/i2c-designware-core.h
b/drivers/i2c/busses/i2c-designware-core.h
index 02d1a2d..9c1840e 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -82,7 +82,7 @@ struct dw_i2c_dev {
unsigned intstatus;
u32 abort_source;
int irq;
- int swab;
+ u32 accessor_flags;
struct i2c_adapter adapter;
u32 functionality;
u32 master_cfg;
@@ -90,6 +90,9 @@ struct dw_i2c_dev {
unsigned intrx_fifo_depth;
};
+#define ACCESS_SWAP0x0001
+#define ACCESS_16BIT 0x0002
+
extern u32 dw_readl(struct dw_i2c_dev *dev, int offset);
extern void dw_writel(struct dw_i2c_dev *dev, u32 b, int offset);
extern int i2c_dw_init(struct dw_i2c_dev *dev);
--
1.7.10
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] i2c-s3c2410: Rework device type handling
Hi, Reorganize driver a bit to better handle device tree-based systems: - move machine type to driver's private structure instead of quering platform device variants in runtime - replace s3c24xx_i2c_type enum with unsigned int that holds bitmask with revision-specific quirks Signed-off-by: Karol Lewandowski k.lewando...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com Okay, so this driver needs to the 'data' field from either platform_device_id or of_device_id and implements a function for that, namely s3c24xx_get_device_quirks(). Grant, Rob: I'd think there might be more drivers in need of that, so maybe it makes sense to have a generic of-helper function? Please wait one or two days before resending. Maybe Grant or Rob find some time to answer this question. (Yeah, we can fix it later if such a generic function is introduced somewhen). -/* i2c controller state */ +#ifdef CONFIG_OF +static const struct of_device_id s3c24xx_i2c_match[]; +#endif Uh, forward declaration with #ifdef. I'd think we should get this simply to the front. Ok, as I think it's better to have dt and non-dt definitions together I'll move both of_device_id and platform_device_id to the top. Agreed. +/* Treat S3C2410 as baseline hardware, anything else is supported via quirks */ +#define QUIRK_S3C2440 (1 0) Minor: Is it really a quirk being s3c2440? Maybe FLAG_*, dunno. In first version[1] of this patch I've used TYPEs for device types and FLAGS for quirks. However, I've squashed these into quirks after discussion with Mark[2]. [1] http://permalink.gmane.org/gmane.linux.kernel/1266759 [2] http://permalink.gmane.org/gmane.linux.kernel.samsung-soc/10255 It's minor, keep the QUIRKs. [ I'll also drop above CONFIG_OF ifdef, as of_match_node() seem to be always defined since v3.2-rc1. ] Great. Thanks, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH 3/3] i2c-s3c2410: Add HDMIPHY quirk for S3C2440
Optional properties: + - gpios: The order of the gpios should be the following: SDA, SCL. +The gpio specifier depends on the gpio controller. Required in all cases +except when samsung,i2c-no-gpio is also specified. + - samsung,i2c-no-gpio: input/output lines of the controller are +permanently wired to the respective client, there are no gpio +lines that need to be configured to enable this controller Can't we just skip this property... All standard s3c-24x0 i2c controllers require gpio lines for proper operation, so lack of the gpios property should be considered as an error. However there is a special case of internal, embedded i2c controller which has no such gpio lines at all. - samsung,i2c-slave-addr: Slave address in multi-master enviroment. If not specified, default value is 0. - samsung,i2c-max-bus-freq: Desired frequency in Hz of the bus. If not specified, the default value in Hz is 10. + - samsung,i2c-quirk-hdmiphy: Quirk for HDMI PHY block found on +Exynos4 platform - reduce timeout and reset controller after each +transfer ... and this one, if we declare a new compatible-entry for exynos4? It is not strictly related to Exynos4 SoCs. Exynos4 SoC has 8 standard s3c2440 style i2c controllers and one additional, internal controller for HDMIPHY, which needs some workarounds in the driver. Maybe the quirk should be named 'broken timeout detection' I was wondering since you do what I suggested for platform devices already: + .name = s3c2440-hdmiphy-i2c, + .driver_data= QUIRK_S3C2440 | QUIRK_HDMIPHY | QUIRK_NO_GPIO, Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH v3] i2c: designware: Add support for 16bit register access
On Wed, Apr 18, 2012 at 03:01:41PM +0200, Stefan Roese wrote: The STM SPEAr platform can only access the i2c controller register via 16bit read/write functions. This patch adds support to automatically detect this 16bit access mode. Signed-off-by: Stefan Roese s...@denx.de Even on second thought, I still think checking the upper 16 bits would not have hurt much, but I am taking it as is. Applied. Thanks, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCHv8 04/18] I2C: OMAP: I2C register restore only if context is lost
Datta, Shubhrajyoti shubhrajy...@ti.com writes:
Hi Kevin,
Hi Kevin,
Thanks for your review,
On Tue, Apr 17, 2012 at 7:15 PM, Kevin Hilman khil...@ti.com wrote:
Shubhrajyoti D shubhrajy...@ti.com writes:
Currently i2c register restore is done always.
Adding conditional restore.
The i2c register restore is done only if the context is lost.
OK, minor comment below.
Thanks for the suggestion of the error case restore.
Updated the patch. Also added Tony's ack for the plat part.
From 1ca222f6f50868e07d9a46575e73dd66fd2d542e Mon Sep 17 00:00:00 2001
From: Shubhrajyoti D shubhrajy...@ti.com
Date: Tue, 17 Jan 2012 16:13:03 +0530
Subject: [PATCH] I2C: OMAP: I2C register restore only if context is lost
Currently i2c register restore is done always.
Adding conditional restore.
The i2c register restore is done only if the context is lost
or in case of error to be on the safe side.
Also remove the definition of SYSS_RESETDONE_MASK and use the
one in omap_hwmod.h.
Cc: Kevin Hilman khil...@ti.com
[For the arch/arm/plat-omap/i2c.c part]
Acked-by: Tony Lindgren t...@atomide.com
Signed-off-by: Shubhrajyoti D shubhrajy...@ti.com
---
arch/arm/plat-omap/i2c.c |3 ++
drivers/i2c/busses/i2c-omap.c | 53
+++--
include/linux/i2c-omap.h |1 +
3 files changed, 39 insertions(+), 18 deletions(-)
diff --git a/arch/arm/plat-omap/i2c.c b/arch/arm/plat-omap/i2c.c
index db071bc..4ccab07 100644
--- a/arch/arm/plat-omap/i2c.c
+++ b/arch/arm/plat-omap/i2c.c
@@ -179,6 +179,9 @@ static inline int omap2_i2c_add_bus(int bus_id)
*/
if (cpu_is_omap34xx())
pdata-set_mpu_wkup_lat = omap_pm_set_max_mpu_wakeup_lat_compat;
+
+ pdata-get_context_loss_count = omap_pm_get_dev_context_loss_count;
+
pdev = omap_device_build(name, bus_id, oh, pdata,
sizeof(struct omap_i2c_bus_platform_data),
NULL, 0, 0);
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index a882558..76cf066 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -43,6 +43,7 @@
#include linux/slab.h
#include linux/i2c-omap.h
#include linux/pm_runtime.h
+#include plat/omap_device.h
/* I2C controller revisions */
#define OMAP_I2C_OMAP1_REV_2 0x20
@@ -157,9 +158,6 @@ enum {
#define OMAP_I2C_SYSTEST_SDA_I (1 1)/* SDA line
sense in */
#define OMAP_I2C_SYSTEST_SDA_O (1 0)/* SDA line
drive out */
-/* OCP_SYSSTATUS bit definitions */
-#define SYSS_RESETDONE_MASK (1 0)
-
Unrelated to this patch.
/* OCP_SYSCONFIG bit definitions */
#define SYSC_CLOCKACTIVITY_MASK (0x3 8)
#define SYSC_SIDLEMODE_MASK (0x3 3)
@@ -184,6 +182,7 @@ struct omap_i2c_dev {
u32 latency;/* maximum mpu wkup latency */
void(*set_mpu_wkup_lat)(struct device *dev,
long latency);
+ int (*get_context_loss_count)(struct device *dev);
u32 speed; /* Speed of bus in kHz */
u32 dtrev; /* extra revision from DT */
u32 flags;
@@ -206,6 +205,7 @@ struct omap_i2c_dev {
u16 syscstate;
u16 westate;
u16 errata;
+ int dev_lost_count;
};
static const u8 reg_map_ip_v1[] = {
@@ -1025,6 +1025,7 @@ omap_i2c_probe(struct platform_device *pdev)
dev-speed = pdata-clkrate;
dev-flags = pdata-flags;
dev-set_mpu_wkup_lat = pdata-set_mpu_wkup_lat;
+ dev-get_context_loss_count = pdata-get_context_loss_count;
dev-dtrev = pdata-rev;
}
@@ -1151,12 +1152,32 @@ omap_i2c_remove(struct platform_device *pdev)
}
#ifdef CONFIG_PM_RUNTIME
+static void omap_i2c_restore(struct omap_i2c_dev *dev)
+{
+ omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, 0);
+ omap_i2c_write_reg(dev, OMAP_I2C_PSC_REG, dev-pscstate);
+ omap_i2c_write_reg(dev, OMAP_I2C_SCLL_REG, dev-scllstate);
+ omap_i2c_write_reg(dev, OMAP_I2C_SCLH_REG, dev-sclhstate);
+ omap_i2c_write_reg(dev, OMAP_I2C_BUF_REG, dev-bufstate);
+ omap_i2c_write_reg(dev, OMAP_I2C_WE_REG, dev-westate);
+ omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, OMAP_I2C_CON_EN);
+ /*
+ * Don't write to this register if the IE state is 0 as it can
+ * cause deadlock.
+ */
+ if (dev-iestate)
+ omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, dev-iestate);
+
+}
static int omap_i2c_runtime_suspend(struct device *dev)
{
struct platform_device *pdev = to_platform_device(dev);
struct omap_i2c_dev *_dev = platform_get_drvdata(pdev);
u16 iv;
+ if
Re: [PATCH] i2c: i2c-sh_mobile device tree support
Hi Paul, Of course, if you think it is cramping your SH device tree style then we can easily add a renesas-shmobile-iic entry as well. I obviously don't mind if you wish to use the rmobile naming convention going forward, as the new parts have obviously dropped with the shmobile naming convention, and it's likely you'll even be able to infer different capabilities between rmobile vs shmobile. That's not sufficient cause to prefer one over the other though, so you're still going to have to keep things balanced. Simply having two aliases seems to me to be the easiest solution. alias is a second compatible entry here? Is it okay if this is added with a seperate patch when needed? Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH v3] i2c: designware: Add support for 16bit register access
On Wednesday 18 April 2012 16:00:11 Wolfram Sang wrote: On Wed, Apr 18, 2012 at 03:01:41PM +0200, Stefan Roese wrote: The STM SPEAr platform can only access the i2c controller register via 16bit read/write functions. This patch adds support to automatically detect this 16bit access mode. Signed-off-by: Stefan Roese s...@denx.de Even on second thought, I still think checking the upper 16 bits would not have hurt much, but I am taking it as is. Applied. Thanks, Stefan -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] i2c: tegra: Add delay before reset the controller
On Mon, Apr 02, 2012 at 11:23:02AM +0530, Alok Chauhan wrote: NACK interrupt generated before I2C controller generates the STOP condition on bus. In Software, because of this reset of controller is happening before I2C controller could complete STOP condition. So wait for some time before resetting the controller so that STOP condition has delivered properly on bus. Added delay of 2 clock period before reset the controller in case of NACK error. Signed-off-by: Alok Chauhan al...@nvidia.com Applied with Stephen's ACK and slightly reworded comment and commit msg. Wondering a bit that you want it for 3.5, not for 3.4. Looks like a bugfix to me. -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH] i2c-at91: fix data-loss issue
I'll repost the driver with your fix on positive feedback from you. Thanks for tracking this down. Ben, is there any chance to get this driver into next? I think sending a new series would be good. It depends a bit on the amount of donated tags (Tested-by, especially) if it will make it into 3.5. Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH] i2c-mpc: avoid I2C abnormal after resuming from deep sleep
On Wed, Apr 18, 2012 at 6:33 PM, Wolfram Sang w.s...@pengutronix.de wrote: Hi, We certainly don't want the policy to be against the common practice of the community. If it is we can consider changing our policy. Nice, thanks for being open to discussion! However I can't find a clear message stating the consensus of what the Linux kernel community think of adding copyright claims. I even found Well, what I know, the consensus is common sense. What is obviously not clear in a way you could write it down. At least, Ben agrees. We do honor your point and can be flexible on this. I'm just not sure if our policy need to be changed to align with the common sense of the community. some message that encourage adding copyright to maintain the GPL license. I am interested. Do you have a link? Well. I can't find the exact message. I remember it is from an email discussion in which someone said by encouraging contributors to add copyright claims to a file it would be clear that no one can change the license of the file away from GPL. - Leo -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] i2c-eg20t: Modify MODULE_AUTHOR's email address
On Mon, Mar 26, 2012 at 02:55:23PM +0900, Tomoya MORINAGA wrote: Signed-off-by: Tomoya MORINAGA tomoya.r...@gmail.com Applied. -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH 3/3] i2c-eg20t: change timeout value 50msec to 1000msec
On Mon, Mar 26, 2012 at 02:55:25PM +0900, Tomoya MORINAGA wrote: Currently, during i2c works alone, wait-event timeout is not occurred. However, as CPU load increases, timeout occurs frequently. So, I modified like this patch. Modifying like this patch, I've never seen the timeout event with high load test. Signed-off-by: Tomoya MORINAGA tomoya.r...@gmail.com Applied. (I think we need to clarify the timeouts in I2C, but for now this has to do.) -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH 2/3] i2c-eg20t: Call init() when wait-event timeout occurs
On Mon, Mar 26, 2012 at 02:55:24PM +0900, Tomoya MORINAGA wrote:
Signed-off-by: Tomoya MORINAGA tomoya.r...@gmail.com
Woha, that's copypaste code all over. May I ask to refactor this?
Something like:
rtn = pch_..._wait_for_checked_xfer();
if (rtn)
return rtn;
// All the things you have to do when rtn == 0
in that new function (the name was only a suugestion), you can do all
the checks which are now copy-pasted, e.g.:
458 rtn = pch_i2c_wait_for_xfer_complete(adap);
459 if (rtn == 0) {
460 if (pch_i2c_getack(adap)) {
461 pch_dbg(adap, Receive NACK for slave address
462 setting\n);
463 return -EIO;
464 }
465 } else if (rtn == -EIO) { /* Arbitration Lost */
466 pch_err(adap, Lost Arbitration\n);
467 pch_clrbit(adap-pch_base_address, PCH_I2CSR, I2CMAL_BIT);
468 pch_clrbit(adap-pch_base_address, PCH_I2CSR, I2CMIF_BIT);
469 pch_i2c_init(adap);
470 return -EAGAIN;
471 } else { /* wait-event timeout */
472 pch_i2c_stop(adap);
// Your current fixes added here
473 return -ETIME;
474 }
return 0;
It is only pseudo-code, but I think it can be done and will help the driver.
Thanks,
Wolfram
---
drivers/i2c/busses/i2c-eg20t.c | 16
1 files changed, 16 insertions(+), 0 deletions(-)
diff --git a/drivers/i2c/busses/i2c-eg20t.c b/drivers/i2c/busses/i2c-eg20t.c
index d73975b..714b070 100644
--- a/drivers/i2c/busses/i2c-eg20t.c
+++ b/drivers/i2c/busses/i2c-eg20t.c
@@ -445,7 +445,9 @@ static s32 pch_i2c_writebytes(struct i2c_adapter
*i2c_adap,
pch_i2c_init(adap);
return -EAGAIN;
} else { /* wait-event timeout */
+ pch_err(adap, %s(L.%d):wait-event timeout\n,
__func__, __LINE__);
pch_i2c_stop(adap);
+ pch_i2c_init(adap);
return -ETIME;
}
} else {
@@ -469,7 +471,9 @@ static s32 pch_i2c_writebytes(struct i2c_adapter
*i2c_adap,
pch_i2c_init(adap);
return -EAGAIN;
} else { /* wait-event timeout */
+ pch_err(adap, %s(L.%d):wait-event timeout\n, __func__,
__LINE__);
pch_i2c_stop(adap);
+ pch_i2c_init(adap);
return -ETIME;
}
@@ -490,7 +494,9 @@ static s32 pch_i2c_writebytes(struct i2c_adapter
*i2c_adap,
pch_clrbit(adap-pch_base_address, PCH_I2CSR,
I2CMIF_BIT);
} else { /* wait-event timeout */
+ pch_err(adap, %s(L.%d):wait-event timeout\n,
__func__, __LINE__);
pch_i2c_stop(adap);
+ pch_i2c_init(adap);
return -ETIME;
}
}
@@ -598,7 +604,9 @@ static s32 pch_i2c_readbytes(struct i2c_adapter
*i2c_adap, struct i2c_msg *msgs,
pch_i2c_init(adap);
return -EAGAIN;
} else { /* wait-event timeout */
+ pch_err(adap, %s(L.%d):wait-event timeout\n,
__func__, __LINE__);
pch_i2c_stop(adap);
+ pch_i2c_init(adap);
return -ETIME;
}
pch_i2c_restart(adap);
@@ -621,7 +629,9 @@ static s32 pch_i2c_readbytes(struct i2c_adapter
*i2c_adap, struct i2c_msg *msgs,
pch_i2c_init(adap);
return -EAGAIN;
} else { /* wait-event timeout */
+ pch_err(adap, %s(L.%d):wait-event timeout\n,
__func__, __LINE__);
pch_i2c_stop(adap);
+ pch_i2c_init(adap);
return -ETIME;
}
} else {
@@ -648,7 +658,9 @@ static s32 pch_i2c_readbytes(struct i2c_adapter
*i2c_adap, struct i2c_msg *msgs,
pch_i2c_init(adap);
return -EAGAIN;
} else { /* wait-event timeout */
+ pch_err(adap, %s(L.%d):wait-event timeout\n, __func__,
__LINE__);
pch_i2c_stop(adap);
+ pch_i2c_init(adap);
return -ETIME;
}
@@ -677,7 +689,9 @@ static s32 pch_i2c_readbytes(struct i2c_adapter
*i2c_adap, struct i2c_msg *msgs,
return -EIO;
}
} else { /* wait-event timeout */
+ pch_err(adap, %s(L.%d):wait-event timeout\n,
__func__, __LINE__);
pch_i2c_stop(adap);
+ pch_i2c_init(adap);
return -ETIME;
Re: [PATCH 1/5] i2c: Convert i2c-octeon.c to use device tree.
- if (i2c_data == NULL) {
- dev_err(i2c-dev, no I2C frequency data\n);
+ /*
+* clock-rate is a legacy binding, the official binding is
+* clock-frequency. Try the official one first and then
+* fall back if it doesn't exist.
+*/
+ data = of_get_property(pdev-dev.of_node, clock-frequency,len);
+ if (!data || len != sizeof(*data))
+ data = of_get_property(pdev-dev.of_node, clock-rate,len);
+ if (data len == sizeof(*data)) {
+ i2c-twsi_freq = be32_to_cpup(data);
Can't you use of_property_read_u32?
I will investigate, and use it if possible.
Any outcome?
And shouldn't the bindings be documented? Or are they only standard and
we hide the legacy one?
Regards,
Wolfram
--
Pengutronix e.K. | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/ |
signature.asc
Description: Digital signature
Re: [PATCH 1/5] i2c: Convert i2c-octeon.c to use device tree.
On 04/18/2012 08:16 AM, Wolfram Sang wrote:
- if (i2c_data == NULL) {
- dev_err(i2c-dev, no I2C frequency data\n);
+ /*
+* clock-rate is a legacy binding, the official binding is
+* clock-frequency. Try the official one first and then
+* fall back if it doesn't exist.
+*/
+ data = of_get_property(pdev-dev.of_node, clock-frequency,len);
+ if (!data || len != sizeof(*data))
+ data = of_get_property(pdev-dev.of_node, clock-rate,len);
+ if (data len == sizeof(*data)) {
+ i2c-twsi_freq = be32_to_cpup(data);
Can't you use of_property_read_u32?
I will investigate, and use it if possible.
Any outcome?
Yes, I have implemented Rob's suggestions. A new patch set reflecting
this is coming soon.
And shouldn't the bindings be documented? Or are they only standard and
we hide the legacy one?
Yes, they are documented here:
http://patchwork.linux-mips.org/patch/3536/
look in the cavium-i2c.txt file.
Thanks,
David Daney
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] i2c-s3c2410: Add HDMIPHY quirk for S3C2440
On 18.04.2012 15:46, Wolfram Sang wrote:
Optional properties:
+ - gpios: The order of the gpios should be the following: SDA, SCL.
+The gpio specifier depends on the gpio controller. Required in all
cases
+except when samsung,i2c-no-gpio is also specified.
+ - samsung,i2c-no-gpio: input/output lines of the controller are
+permanently wired to the respective client, there are no gpio
+lines that need to be configured to enable this controller
Can't we just skip this property...
All standard s3c-24x0 i2c controllers require gpio lines for proper
operation,
so lack of the gpios property should be considered as an error. However there
is a special case of internal, embedded i2c controller which has no such
gpio
lines at all.
- samsung,i2c-slave-addr: Slave address in multi-master enviroment. If
not
specified, default value is 0.
- samsung,i2c-max-bus-freq: Desired frequency in Hz of the bus. If not
specified, the default value in Hz is 10.
+ - samsung,i2c-quirk-hdmiphy: Quirk for HDMI PHY block found on
+Exynos4 platform - reduce timeout and reset controller after each
+transfer
... and this one, if we declare a new compatible-entry for exynos4?
It is not strictly related to Exynos4 SoCs. Exynos4 SoC has 8 standard
s3c2440 style
i2c controllers and one additional, internal controller for HDMIPHY, which
needs
some workarounds in the driver. Maybe the quirk should be named 'broken
timeout
detection'
I was wondering since you do what I suggested for platform devices already:
+ .name = s3c2440-hdmiphy-i2c,
+ .driver_data= QUIRK_S3C2440 | QUIRK_HDMIPHY |
QUIRK_NO_GPIO,
Here I've done it this way for compatibility with legacy driver and
board(s).
Device tree is new interface, and I thought that it would be better
to describe things explicitly and separately from device type.
Right now these properties are used only on S3C2440, but I don't
consider it highly unlikely to see these on S3C in future.
Tomasz had similar doubts when I've posted patch that checked these
quirks only for S3C2440:
http://permalink.gmane.org/gmane.linux.drivers.i2c/10305
Thus, I've chosen properties and not separate type.
It's easy to introduce compat string (see below), but given above
I'm afraid that we might end up adding -hdmiphy- variant for every
new version of i2c controller.
E.g.
diff --git a/Documentation/devicetree/bindings/i2c/samsung-i2c.txt
b/Documentation/devicetree/bindings/i2c/samsung-i2c.txt
index c6670f9..b1d106e 100644
--- a/Documentation/devicetree/bindings/i2c/samsung-i2c.txt
+++ b/Documentation/devicetree/bindings/i2c/samsung-i2c.txt
@@ -6,6 +6,8 @@ Required properties:
- compatible: value should be either of the following.
(a) samsung, s3c2410-i2c, for i2c compatible with s3c2410 i2c.
(b) samsung, s3c2440-i2c, for i2c compatible with s3c2440 i2c.
+ (c) samsung, s3c2440-hdmiphy-i2c, for s3c2440-like i2c used
+ inside HDMIPHY block found on several samsung SoCs
- reg: physical base address of the controller and length of memory mapped
region.
- interrupts: interrupt number to the cpu.
@@ -13,18 +15,13 @@ Required properties:
Optional properties:
- gpios: The order of the gpios should be the following: SDA, SCL.
-The gpio specifier depends on the gpio controller. Required in all cases
-except when samsung,i2c-no-gpio is also specified.
- - samsung,i2c-no-gpio: input/output lines of the controller are
-permanently wired to the respective client, there are no gpio
-lines that need to be configured to enable this controller
+The gpio specifier depends on the gpio controller. Required in all
+cases except for samsung,i2c-hdmiphy-i2c whose input/output
+lines are permanently wired to the respective client
- samsung,i2c-slave-addr: Slave address in multi-master enviroment. If not
specified, default value is 0.
- samsung,i2c-max-bus-freq: Desired frequency in Hz of the bus. If not
specified, the default value in Hz is 10.
- - samsung,i2c-quirk-hdmiphy: Quirk for HDMI PHY block found on
-Exynos4 platform - reduce timeout and reset controller after each
-transfer
Example:
diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
index 71438eb..bc82045 100644
--- a/drivers/i2c/busses/i2c-s3c2410.c
+++ b/drivers/i2c/busses/i2c-s3c2410.c
@@ -106,6 +106,8 @@ MODULE_DEVICE_TABLE(platform, s3c24xx_driver_ids);
static const struct of_device_id s3c24xx_i2c_match[] = {
{ .compatible = samsung,s3c2410-i2c, .data = (void *)0 },
{ .compatible = samsung,s3c2440-i2c, .data = (void *)QUIRK_S3C2440 },
+ { .compatible = samsung,s3c2440-hdmiphy-i2c,
+ .data = (void *)(QUIRK_S3C2440 | QUIRK_HDMIPHY | QUIRK_NO_GPIO) },
{},
};
MODULE_DEVICE_TABLE(of, s3c24xx_i2c_match);
@@ -902,12 +904,6 @@
Re: [PATCH v3 5/8] i2c-pnx.c: Fix suspend
On Wed, Apr 04, 2012 at 10:34:37AM +0200, Roland Stigge wrote: In the driver's suspend function, clk_enable() was used instead of clk_disable(). This is corrected with this patch. Signed-off-by: Roland Stigge sti...@antcom.de Reviewed-by: Arnd Bergmann a...@arndb.de CC: sta...@vger.kernel.org Picked up for 3.4, reworded the message header to i2c: pnx: Disable clk in suspend since fix suspend was too generic. Thanks, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH v3 6/8] i2c-pnx.c: Use resources in platforms
On Wed, Apr 04, 2012 at 10:34:38AM +0200, Roland Stigge wrote: As a precondition for device tree conversion, the platforms using i2c-pnx.c are converted to using mem and irq resources instead of platform data. Signed-off-by: Roland Stigge sti...@antcom.de Reviewed-by: Arnd Bergmann a...@arndb.de I trust you and Arnd on the mach stuff. i2c looked okay, devm_* could have been used, but well... Applied to next. -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH v3 7/8] i2c-pnx.c: Remove duplicated i2c.h
On Wed, Apr 04, 2012 at 10:34:39AM +0200, Roland Stigge wrote: The platforms using i2c-pnx.c both defined a duplicated i2c.h (used nowhere else). This patch removes those and integrates the contents into the driver itself. Signed-off-by: Roland Stigge sti...@antcom.de Reviewed-by: Arnd Bergmann a...@arndb.de Applied to next, thanks. -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH v3 8/8] i2c: Add device tree support to i2c-pnx.c
On Wed, Apr 04, 2012 at 10:34:40AM +0200, Roland Stigge wrote: This patch adds device tree support to the pnx-i2c driver by using platform resources for memory region and irq and removing dependency on mach includes. The following platforms are affected: * PNX * LPC31xx (WIP) * LPC32xx The patch is based on a patch by Jon Smirl, working on lpc31xx integration Signed-off-by: Roland Stigge sti...@antcom.de Reviewed-by: Arnd Bergmann a...@arndb.de I added devicetree-discuss to the (quite long) CC list. I am not sure about all of the bindings. --- Applies to v3.4-rc1 Documentation/devicetree/bindings/i2c/pnx.txt | 40 drivers/i2c/busses/i2c-pnx.c | 65 +++--- include/linux/i2c-pnx.h |1 3 files changed, 89 insertions(+), 17 deletions(-) --- /dev/null +++ linux-2.6/Documentation/devicetree/bindings/i2c/pnx.txt @@ -0,0 +1,40 @@ +* NXP PNX I2C Controller + +Required properties: + + - reg: Offset and length of the register set for the device + - compatible: should be nxp,pnx-i2c + - interrupts: configure one interrupt line + - #address-cells: always 1 (for i2c addresses) + - #size-cells: always 0 + +Optional properties: + + - interrupt-parent: the phandle for the interrupt controller that + services interrupts for this device. That one is not optional, or? You always need it but in most cases it gets inherited as I understand. + - clock-frequency: desired I2C bus clock frequency in Hz, Default: 10 Hz + - pnx,timeout: I2C bus timeout in milliseconds, Default: 10 ms To devicetree-folks: Can we argue this is a hardware property of the bus? Then we could introduce a generic timeout property? I see, there is already fsl,timeout, and it won't make sense to have that per vendor? + - slave-addr: Address used by the controller, Hardware default: 110 I'd prefer the hex value here, I2C addresses are mostly specified in hex. Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCH v2] i2c: tegra: Add delay before reset the controller
On 04/18/2012 08:27 AM, Wolfram Sang wrote: On Mon, Apr 02, 2012 at 11:23:02AM +0530, Alok Chauhan wrote: NACK interrupt generated before I2C controller generates the STOP condition on bus. In Software, because of this reset of controller is happening before I2C controller could complete STOP condition. So wait for some time before resetting the controller so that STOP condition has delivered properly on bus. Added delay of 2 clock period before reset the controller in case of NACK error. Signed-off-by: Alok Chauhan al...@nvidia.com Applied with Stephen's ACK and slightly reworded comment and commit msg. Wondering a bit that you want it for 3.5, not for 3.4. Looks like a bugfix to me. Yes, applying this to 3.4 would make sense. When I said for 3.5 earlier, I meant I'd like it to at least get into 3.5 not later. Thanks. -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] i2c: tegra: Add delay before reset the controller
Applied with Stephen's ACK and slightly reworded comment and commit msg. Wondering a bit that you want it for 3.5, not for 3.4. Looks like a bugfix to me. Yes, applying this to 3.4 would make sense. When I said for 3.5 earlier, I meant I'd like it to at least get into 3.5 not later. Done, thanks for clarifying. -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
