Re: [PATCH] support i2c 10-bit addressing
Hi Peter, Guenter,
First of all: the right list to discuss this is linux-i2c (Cc'd) with
me in Cc.
On Sat, 28 Mar 2015 11:01:30 -0700, Guenter Roeck wrote:
On 03/28/2015 06:17 AM, Peter Chang wrote:
sigh gmail appears to have futzed w/ the attachment type.
2015-03-28 6:12 GMT-07:00 Peter Chang d...@google.com:
the address parsing doesn't have the adapter's support bits yet, so it
looks a little out of place.
There is a reason for discouraging attachments: If one replies (like me here),
the source code is not part of the reply, making a review really difficult.
Not sure what the above comment is supposed to mean. I can not parse it,
sorry.
Neither can I.
Unless I misunderstand the code, it now accepts addresses up to 0x3ff
unconditionally. If the adapter doesn't support 10 bit addresses, the
code then doesn't even try to set 10-bit address mode. 10-bit addresses
should not be accepted if the adapter does not support it.
It's even worse than that. The code also considers all addresses below
0x77 to always be 7-bit addresses, which is not correct. I2C specifies
two address ranges: a 7-bit one (valid addresses from 0x03 to 0x77) and
a 10-bit one (valid addresses 0x00 to 0x3ff.) These two ranges do NOT
overlap, even though the numbers are the same. In other words, 10-bit
address 0x10 exists and is NOT the same as 7-bit address 0x10.
This means that you can't guess whether the user means a 7-bit address
or a 10-bit address. The intent must be expressed explicitly with a
command-line parameter, and then the address validity must be checked
according to this parameter.
I would suggest to rearrange the code a bit to include the 10bit check
in check_funcs. Something like
if (...
|| check_funcs(file, size, pec, address 0x77)
|| ...
might do. This would make the code easier to read and address
the problem where a 10-bit address is provided but not supported by
the adapter.
The condition must be more explicit than address 0x77, I'd rather
expect something like:
check_funcs(file, size, pec, tenbit)
where tenbit is set by passing -t on the command line, for example.
Then
if (address 0x77 ioctl(file, I2C_TENBIT, 1) 0) {
fprintf(stderr, ...);
return -errno;
}
should work in set_slave_addr without the need to pass funcs to it.
Unfortunately not, as addresses = 0x77 are equally valid 10-bit
addresses. You must explicitly tell set_slave_addr whether the address
is a 7-bit or 10-bit one. But please do that by passing a flag (e.g.
tenbit as above) to it, not funcs.
You'll also need to update the Usage: strings for the various tools.
Correct, and the manual pages too.
--
Jean Delvare
SUSE L3 Support
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c: dln2: set the device tree node of the adapter
On Fri, 27 Mar 2015, Octavian Purdila wrote: This patch makes sure the platform device tree node is inherited by the adapter device. This allows the DLN2 bus to work with i2c devices defined in the device tree. Signed-off-by: Octavian Purdila octavian.purd...@intel.com --- drivers/i2c/busses/i2c-dln2.c | 1 + 1 file changed, 1 insertion(+) Acked-by: Lee Jones lee.jo...@linaro.org diff --git a/drivers/i2c/busses/i2c-dln2.c b/drivers/i2c/busses/i2c-dln2.c index b3fb86a..db1e20a 100644 --- a/drivers/i2c/busses/i2c-dln2.c +++ b/drivers/i2c/busses/i2c-dln2.c @@ -210,6 +210,7 @@ static int dln2_i2c_probe(struct platform_device *pdev) dln2-adapter.class = I2C_CLASS_HWMON; dln2-adapter.algo = dln2_i2c_usb_algorithm; dln2-adapter.dev.parent = dev; + dln2-adapter.dev.of_node = dev-of_node; i2c_set_adapdata(dln2-adapter, dln2); snprintf(dln2-adapter.name, sizeof(dln2-adapter.name), %s-%s-%d, dln2-i2c, dev_name(pdev-dev.parent), dln2-port); -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- 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: dln2: set the device tree node of the adapter
On Fri, 27 Mar 2015, Wolfram Sang wrote: On Fri, Mar 27, 2015 at 05:37:10PM +0200, Octavian Purdila wrote: This patch makes sure the platform device tree node is inherited by the adapter device. This allows the DLN2 bus to work with i2c devices defined in the device tree. Signed-off-by: Octavian Purdila octavian.purd...@intel.com Applied to for-next, thanks! Ah sorry. I read this _after_ sending my Ack. Feel free to apply it, or not. -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- 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 02/86] i2c/i801: linux/pci_ids.h - uapi/linux/pci_ids.h
On Sun, 29 Mar 2015 15:37:30 +0200, Michael S. Tsirkin wrote: Header moved from linux/pci_ids.h to uapi/linux/pci_ids.h, update i2c documentation accordingly. Signed-off-by: Michael S. Tsirkin m...@redhat.com --- Documentation/i2c/busses/i2c-i801 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/i2c/busses/i2c-i801 b/Documentation/i2c/busses/i2c-i801 index 82f48f7..8036644 100644 --- a/Documentation/i2c/busses/i2c-i801 +++ b/Documentation/i2c/busses/i2c-i801 @@ -141,7 +141,7 @@ host bridge PCI device. Get yours with lspci -n -v -s 00:00.0: Here the host bridge ID is 2570 (82865G/PE/P), the subvendor ID is 1043 (Asus) and the subdevice ID is 80f2 (P4P800-X). You can find the symbolic -names for the bridge ID and the subvendor ID in include/linux/pci_ids.h, +names for the bridge ID and the subvendor ID in include/uapi/linux/pci_ids.h, and then add a case for your subdevice ID at the right place in drivers/pci/quirks.c. Then please give it very good testing, to make sure that the unhidden SMBus doesn't conflict with e.g. ACPI. No objection from me. Reviewed-by: Jean Delvare jdelv...@suse.de -- Jean Delvare SUSE L3 Support -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/3] I2C: mediatek: Add driver for MediaTek MT8173 I2C controller
Hi Sascha,
On Mon, 2015-03-23 at 08:39 +0100, Sascha Hauer wrote:
On Sat, Mar 21, 2015 at 02:05:22PM +0800, Eddie Huang wrote:
Add mediatek MT8173 I2C controller driver. Compare to I2C controller
of earlier mediatek SoC, MT8173 fix write-then-read limitation, and
also increase message size to 64kb.
[...]
+static const struct i2c_adapter_quirks mt8173_i2c_quirks = {
+ .max_num_msgs = MAX_MSG_NUM_MT8173,
+ .max_write_len = MAX_DMA_TRANS_SIZE_MT8173,
+ .max_read_len = MAX_DMA_TRANS_SIZE_MT8173,
+ .max_comb_1st_msg_len = MAX_DMA_TRANS_SIZE_MT8173,
+ .max_comb_2nd_msg_len = MAX_WRRD_TRANS_SIZE_MT8173,
+};
+
static int mtk_i2c_probe(struct platform_device *pdev)
{
int ret = 0;
@@ -587,7 +626,8 @@ static int mtk_i2c_probe(struct platform_device *pdev)
return -EINVAL;
i2c-platform_compat = mtk_get_device_prop(pdev);
- if (i2c-have_pmic (i2c-platform_compat COMPAT_MT6577))
+ if (i2c-have_pmic (i2c-platform_compat
+ (COMPAT_MT6577 | COMPAT_MT8173)))
return -EINVAL;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -613,7 +653,10 @@ static int mtk_i2c_probe(struct platform_device *pdev)
i2c-adap.dev.parent = pdev-dev;
i2c-adap.owner = THIS_MODULE;
i2c-adap.algo = mtk_i2c_algorithm;
- i2c-adap.quirks = mt6577_i2c_quirks;
+ if (i2c-platform_compat COMPAT_MT8173)
+ i2c-adap.quirks = mt8173_i2c_quirks;
+ else
+ i2c-adap.quirks = mt6577_i2c_quirks;
Instead of putting an integer into struct of_device_id you should
introduce a
struct mtk_i2c_data {
struct i2c_adapter_quirks quirks;
int compat;
/* Additional SoC specific data */
};
and put a pointer to this directly into the of_device_id. This way you
need less casting and can put newly discovered differences diretly into
some data struct and don't have to introduce if(socxy) ... else ...
everytime.
Thanks your suggestion. I will use capability struct like
struct mtk_i2c_compatible {
unsigned char pmic_i2c;
unsigned char dcm;
unsigned char auto_restart;
const struct i2c_adapter_quirks *quirks;
};
And pass in of_device_id, then remove all if(socxy) else ... in
driver.
Eddie
--
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 v5 2/3] I2C: mediatek: Add driver for MediaTek I2C controller
Hi Sascha,
On Mon, 2015-03-23 at 09:42 +0100, Sascha Hauer wrote:
On Sat, Mar 21, 2015 at 02:05:21PM +0800, Eddie Huang wrote:
From: Xudong Chen xudong.c...@mediatek.com
The mediatek SoCs have I2C controller that handle I2C transfer.
This patch include common I2C bus driver.
This driver is compatible with I2C controller on mt65xx/mt81xx.
Signed-off-by: Xudong Chen xudong.c...@mediatek.com
Signed-off-by: Liguo Zhang liguo.zh...@mediatek.com
Signed-off-by: Eddie Huang eddie.hu...@mediatek.com
---
drivers/i2c/busses/Kconfig | 9 +
drivers/i2c/busses/Makefile | 1 +
drivers/i2c/busses/i2c-mt65xx.c | 705
3 files changed, 715 insertions(+)
create mode 100644 drivers/i2c/busses/i2c-mt65xx.c
+
+static void mtk_i2c_clock_disable(struct mtk_i2c *i2c)
+{
+ if (i2c-have_pmic)
+ clk_disable_unprepare(i2c-clk_pmic);
+
+ clk_disable_unprepare(i2c-clk_main);
+ clk_disable_unprepare(i2c-clk_dma);
+}
+
+static inline void mtk_i2c_init_hw(struct mtk_i2c *i2c)
Please let the compiler decide whether to inline this or not.
Will fix it.
+{
+ mtk_i2c_writew(I2C_SOFT_RST, i2c, OFFSET_SOFTRESET);
+ /* Set ioconfig */
+ if (i2c-use_push_pull)
+ mtk_i2c_writew(I2C_IO_CONFIG_PUSH_PULL, i2c, OFFSET_IO_CONFIG);
+ else
+ mtk_i2c_writew(I2C_IO_CONFIG_OPEN_DRAIN, i2c, OFFSET_IO_CONFIG);
+
+ if (i2c-platform_compat COMPAT_MT6577)
+ mtk_i2c_writew(I2C_DCM_DISABLE, i2c, OFFSET_DCM_EN);
+
+ mtk_i2c_writew(i2c-timing_reg, i2c, OFFSET_TIMING);
+ mtk_i2c_writew(i2c-high_speed_reg, i2c, OFFSET_HS);
+}
+
[...]
+ step_cnt = step_div;
+ sclk = hclk / (2 * sample_cnt * step_cnt);
+ if (sclk khz) {
+ dev_dbg(i2c-dev, %s mode: unsupported speed (%ldkhz)\n,
+ (mode == HS_MODE) ? HS : ST/FT, (long int)khz);
+ return -ENOTSUPP;
+ }
+
+ step_cnt--;
+ sample_cnt--;
+
+ if (mode == HS_MODE) {
This is the only place where the HS_MODE is actually tested for.
Dropping this enum and using i2c-speed_hz MAX_FS_MODE_SPEED directly
here would improve readability.
Will fix it.
+ /* Set the hign speed mode register */
+ i2c-timing_reg = I2C_FS_TIME_INIT_VALUE;
+ i2c-high_speed_reg = I2C_TIME_DEFAULT_VALUE |
+ (sample_cnt I2C_TIMING_SAMPLE_COUNT_MASK) 12 |
+ (step_cnt I2C_TIMING_SAMPLE_COUNT_MASK) 8;
+ } else {
+ i2c-timing_reg =
+ (sample_cnt I2C_TIMING_SAMPLE_COUNT_MASK) 8 |
+ (step_cnt I2C_TIMING_STEP_DIV_MASK) 0;
+ /* Disable the high speed transaction */
+ i2c-high_speed_reg = I2C_TIME_CLR_VALUE;
+ }
+
+ return 0;
+}
+
[...]
+ if (i2c-speed_hz 40)
+ control_reg |= I2C_CONTROL_RS;
+ if (i2c-op == I2C_MASTER_WRRD)
+ control_reg |= I2C_CONTROL_DIR_CHANGE | I2C_CONTROL_RS;
+ mtk_i2c_writew(control_reg, i2c, OFFSET_CONTROL);
+
+ /* set start condition */
+ if (i2c-speed_hz = 10)
+ mtk_i2c_writew(I2C_ST_START_CON, i2c, OFFSET_EXT_CONF);
+ else
+ mtk_i2c_writew(I2C_FS_START_CON, i2c, OFFSET_EXT_CONF);
+
+ if (~control_reg I2C_CONTROL_RS)
+ mtk_i2c_writew(I2C_DELAY_LEN, i2c, OFFSET_DELAY_LEN);
speed = 40 here to make this more obvious?
There are two cases, not only speed=40, but I2C_MASTER_WRRD. I tend
to keep it.
+
+ addr_reg = msgs-addr 1;
+ if (i2c-op == I2C_MASTER_RD)
+ addr_reg |= 0x1;
+ mtk_i2c_writew(addr_reg, i2c, OFFSET_SLAVE_ADDR);
+
+ /* Clear interrupt status */
+ mtk_i2c_writew(I2C_HS_NACKERR | I2C_ACKERR | I2C_TRANSAC_COMP,
+ i2c, OFFSET_INTR_STAT);
+ mtk_i2c_writew(I2C_FIFO_ADDR_CLR, i2c, OFFSET_FIFO_ADDR_CLR);
+
+ /* Enable interrupt */
+ mtk_i2c_writew(I2C_HS_NACKERR | I2C_ACKERR | I2C_TRANSAC_COMP,
+ i2c, OFFSET_INTR_MASK);
Why do you enable/disable interrupts for each transfer? Enabling them
once and just acknowledge them in the interrupt handler should be
enough.
This can avoid unwanted I2C interrupt. For example, I2C transfer error,
and cause timeout, I2C driver report error to caller. Then I2C error
interrupt happen.
+ if (!i2c-trans_stop tmo == 0) {
+ dev_dbg(i2c-dev, addr: %x, transfer timeout\n, msgs-addr);
+ mtk_i2c_init_hw(i2c);
+ return -ETIMEDOUT;
+ }
+
+ spin_lock(i2c-irqlock);
+ irqstat = i2c-irq_stat;
+ spin_unlock(i2c-irqlock);
A plain spin_lock can't protect you against the interrupt handler, see
https://www.kernel.org/pub/linux/kernel/people/rusty/kernel-locking/c214.html.
You need at least spin_lock_irq().
Anyway, I think this spin_lock can be removed since it only protects the
irq_stat
[PATCH v2] i2c: i2c-mux-gpio: remove error messages for probe deferrals
Probe deferral is not an error case. It happens only when
the necessary dependencies are not there yet.
The driver core is already printing a message when a driver
requests probe deferral, so this can be traced in the logs
without these error prints.
This patch removes the error messages for these deferral cases.
Signed-off-by: Ionut Nicu ioan.nicu@nokia.com
---
drivers/i2c/muxes/i2c-mux-gpio.c | 10 +++---
1 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c
index f5798eb..70db992 100644
--- a/drivers/i2c/muxes/i2c-mux-gpio.c
+++ b/drivers/i2c/muxes/i2c-mux-gpio.c
@@ -76,10 +76,9 @@ static int i2c_mux_gpio_probe_dt(struct gpiomux *mux,
return -ENODEV;
}
adapter = of_find_i2c_adapter_by_node(adapter_np);
- if (!adapter) {
- dev_err(pdev-dev, Cannot find parent bus\n);
+ if (!adapter)
return -EPROBE_DEFER;
- }
+
mux-data.parent = i2c_adapter_id(adapter);
put_device(adapter-dev);
@@ -177,11 +176,8 @@ static int i2c_mux_gpio_probe(struct platform_device *pdev)
}
parent = i2c_get_adapter(mux-data.parent);
- if (!parent) {
- dev_err(pdev-dev, Parent adapter (%d) not found\n,
- mux-data.parent);
+ if (!parent)
return -EPROBE_DEFER;
- }
mux-parent = parent;
mux-gpio_base = gpio_base;
--
1.7.1
--
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 v5 2/3] I2C: mediatek: Add driver for MediaTek I2C controller
On Mon, Mar 30, 2015 at 04:14:12PM +0800, Eddie Huang wrote: Hi Sascha, [...] + if (i2c-speed_hz 40) + control_reg |= I2C_CONTROL_RS; + if (i2c-op == I2C_MASTER_WRRD) + control_reg |= I2C_CONTROL_DIR_CHANGE | I2C_CONTROL_RS; + mtk_i2c_writew(control_reg, i2c, OFFSET_CONTROL); + + /* set start condition */ + if (i2c-speed_hz = 10) + mtk_i2c_writew(I2C_ST_START_CON, i2c, OFFSET_EXT_CONF); + else + mtk_i2c_writew(I2C_FS_START_CON, i2c, OFFSET_EXT_CONF); + + if (~control_reg I2C_CONTROL_RS) + mtk_i2c_writew(I2C_DELAY_LEN, i2c, OFFSET_DELAY_LEN); speed = 40 here to make this more obvious? There are two cases, not only speed=40, but I2C_MASTER_WRRD. I tend to keep it. Still it looks strange. You only ever write this default value to the register. Putting this register write under an if() seems bogus since the same value will be in the register the next time this code is executed. It looks like you should move this register write to some initialization function. + + /* Enable interrupt */ + mtk_i2c_writew(I2C_HS_NACKERR | I2C_ACKERR | I2C_TRANSAC_COMP, + i2c, OFFSET_INTR_MASK); Why do you enable/disable interrupts for each transfer? Enabling them once and just acknowledge them in the interrupt handler should be enough. This can avoid unwanted I2C interrupt. For example, I2C transfer error, and cause timeout, I2C driver report error to caller. Then I2C error interrupt happen. So isn't the same unwanted interrupt then just delayed until you enable the interrupts again? Is this something that really happens or just something you think that might happen? Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | -- 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 v2 0/4] arm: tegra: implement NVEC driver using tegra i2c.
NVEC driver contains code to manage tegra i2c controller in slave mode. I2C slave support was implemented in linux kernel. The goal of this patch serie is to implement I2C slave mode in tegra drived and rework NVEC driver to use it. Patches are based on i2c for-next. Changes for v2: - rebased on top of new i2c slave framework. - old code is removed in separate patch - documentation patch is integrated to main nvec patch Thanks in advance Andrey Danin (4): i2c: tegra: implement slave mode staging/nvec: reimplement on top of tegra i2c driver staging/nvec: remove old code dt: paz00: define nvec as child of i2c bus .../devicetree/bindings/nvec/nvidia,nvec.txt | 21 +- arch/arm/boot/dts/tegra20-paz00.dts| 22 +- drivers/i2c/busses/Kconfig | 1 + drivers/i2c/busses/i2c-tegra.c | 119 ++ drivers/staging/nvec/nvec.c| 411 +++-- drivers/staging/nvec/nvec.h| 10 - 6 files changed, 269 insertions(+), 315 deletions(-) -- 1.9.1 -- 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 v2 2/4] staging/nvec: reimplement on top of tegra i2c driver
Remove i2c controller related code and use tegra i2c driver in slave mode.
Update nvec documentation.
Signed-off-by: Andrey Danin danind...@mail.ru
---
Changes for v2:
- remove extra new line
- keep old functions to simplify review
- move nvec_state enum to nvec.c
- use nvec-slave instead of nvec in dts to keep ABI compatibility
- rebased on top of new i2c slave framework
- delay workaround moved to tegra-i2c
- documentation patch is integrated in this commit
- reverted a log message to minimize changes
---
.../devicetree/bindings/nvec/nvidia,nvec.txt | 21 +-
drivers/staging/nvec/nvec.c| 258 +
drivers/staging/nvec/nvec.h| 10 -
3 files changed, 169 insertions(+), 120 deletions(-)
diff --git a/Documentation/devicetree/bindings/nvec/nvidia,nvec.txt
b/Documentation/devicetree/bindings/nvec/nvidia,nvec.txt
index 5ae601e..aba34095 100644
--- a/Documentation/devicetree/bindings/nvec/nvidia,nvec.txt
+++ b/Documentation/devicetree/bindings/nvec/nvidia,nvec.txt
@@ -1,21 +1,6 @@
NVIDIA compliant embedded controller
Required properties:
-- compatible : should be nvidia,nvec.
-- reg : the iomem of the i2c slave controller
-- interrupts : the interrupt line of the i2c slave controller
-- clock-frequency : the frequency of the i2c bus
-- gpios : the gpio used for ec request
-- slave-addr: the i2c address of the slave controller
-- clocks : Must contain an entry for each entry in clock-names.
- See ../clocks/clock-bindings.txt for details.
-- clock-names : Must include the following entries:
- Tegra20/Tegra30:
- - div-clk
- - fast-clk
- Tegra114:
- - div-clk
-- resets : Must contain an entry for each entry in reset-names.
- See ../reset/reset.txt for details.
-- reset-names : Must include the following entries:
- - i2c
+- compatible : should be nvidia,nvec-slave.
+- reg: the i2c address of the slave controller
+- request-gpios : the gpio used for ec request
diff --git a/drivers/staging/nvec/nvec.c b/drivers/staging/nvec/nvec.c
index 5868ebb..f4c5527 100644
--- a/drivers/staging/nvec/nvec.c
+++ b/drivers/staging/nvec/nvec.c
@@ -25,8 +25,8 @@
#include linux/err.h
#include linux/gpio.h
#include linux/interrupt.h
+#include linux/i2c.h
#include linux/io.h
-#include linux/irq.h
#include linux/of.h
#include linux/of_gpio.h
#include linux/list.h
@@ -39,25 +39,19 @@
#include nvec.h
-#define I2C_CNFG 0x00
-#define I2C_CNFG_PACKET_MODE_EN(110)
-#define I2C_CNFG_NEW_MASTER_SFM(111)
-#define I2C_CNFG_DEBOUNCE_CNT_SHIFT12
-
-#define I2C_SL_CNFG0x20
-#define I2C_SL_NEWSL (12)
-#define I2C_SL_NACK(11)
-#define I2C_SL_RESP(10)
-#define I2C_SL_IRQ (13)
-#define END_TRANS (14)
-#define RCVD (12)
-#define RNW(11)
-
-#define I2C_SL_RCVD0x24
-#define I2C_SL_STATUS 0x28
-#define I2C_SL_ADDR1 0x2c
-#define I2C_SL_ADDR2 0x30
-#define I2C_SL_DELAY_COUNT 0x3c
+
+#define I2C_SL_ST_END_TRANS(14)
+#define I2C_SL_ST_IRQ (13)
+#define I2C_SL_ST_RCVD (12)
+#define I2C_SL_ST_RNW (11)
+
+
+enum nvec_state {
+ ST_NONE,
+ ST_RX,
+ ST_TX,
+ ST_TRANS_START,
+};
/**
* enum nvec_msg_category - Message categories for nvec_msg_alloc()
@@ -479,7 +473,7 @@ static void nvec_tx_completed(struct nvec_chip *nvec)
nvec-tx-pos = 0;
nvec_gpio_set_value(nvec, 0);
} else {
- nvec-state = 0;
+ nvec-state = ST_NONE;
}
}
@@ -497,7 +491,7 @@ static void nvec_rx_completed(struct nvec_chip *nvec)
(uint) nvec-rx-pos);
nvec_msg_free(nvec, nvec-rx);
- nvec-state = 0;
+ nvec-state = ST_NONE;
/* Battery quirk - Often incomplete, and likes to crash */
if (nvec-rx-data[0] == NVEC_BAT)
@@ -514,7 +508,7 @@ static void nvec_rx_completed(struct nvec_chip *nvec)
spin_unlock(nvec-rx_lock);
- nvec-state = 0;
+ nvec-state = ST_NONE;
if (!nvec_msg_is_event(nvec-rx))
complete(nvec-ec_transfer);
@@ -522,6 +516,7 @@ static void nvec_rx_completed(struct nvec_chip *nvec)
schedule_work(nvec-rx_work);
}
+#if 0
/**
* nvec_invalid_flags - Send an error message about invalid flags and jump
* @nvec: The nvec device
@@ -536,6 +531,7 @@ static void nvec_invalid_flags(struct nvec_chip *nvec,
unsigned int status,
if (reset)
nvec-state = 0;
}
+#endif /* FIXME: remove old code */
/**
* nvec_tx_set - Set the message to transfer (nvec-tx)
@@ -566,6 +562,8 @@ static void nvec_tx_set(struct nvec_chip *nvec)
(uint)nvec-tx-size, nvec-tx-data[1]);
}
+
+#if 0
/**
* nvec_interrupt -
[PATCH v2 1/4] i2c: tegra: implement slave mode
Initialization code is based on NVEC driver.
There is a HW bug in AP20 that was also mentioned in kernel sources
for Toshiba AC100.
Signed-off-by: Andrey Danin danind...@mail.ru
---
Changes for v2:
- remove hack from tegra_i2c_clock_disable
- replace slave status helper functions with local variables
- add constant for default delay count value
- add 10-bit address support
- remove read_slave_start_delay init as zero
- don't reset controller during slave registration
- slave isr returns int instead of bool
- make status related variables in slave u32 instead of unsigned int
- enable i2c slave in Kconfig
- rebase on top of new i2c slave framework
- delay workaround was added from nvec
---
drivers/i2c/busses/Kconfig | 1 +
drivers/i2c/busses/i2c-tegra.c | 119 +
2 files changed, 120 insertions(+)
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index db09881..a2b259b 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -852,6 +852,7 @@ config I2C_SUN6I_P2WI
config I2C_TEGRA
tristate NVIDIA Tegra internal I2C controller
depends on ARCH_TEGRA
+ select I2C_SLAVE
help
If you say yes to this option, support will be included for the
I2C controller embedded in NVIDIA Tegra SOCs
diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index 1bcd75e..e378827 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -42,8 +42,17 @@
#define I2C_SL_CNFG0x020
#define I2C_SL_CNFG_NACK (11)
#define I2C_SL_CNFG_NEWSL (12)
+#define I2C_SL_RCVD0x024
+#define I2C_SL_STATUS 0x028
+#define I2C_SL_ST_IRQ (13)
+#define I2C_SL_ST_END_TRANS(14)
+#define I2C_SL_ST_RCVD (12)
+#define I2C_SL_ST_RNW (11)
#define I2C_SL_ADDR1 0x02c
#define I2C_SL_ADDR2 0x030
+#define I2C_SL_ADDR2_TEN_BIT_MODE 1
+#define I2C_SL_DELAY_COUNT 0x03c
+#define I2C_SL_DELAY_COUNT_DEFAULT 0x1E
#define I2C_TX_FIFO0x050
#define I2C_RX_FIFO0x054
#define I2C_PACKET_TRANSFER_STATUS 0x058
@@ -125,6 +134,8 @@ enum msg_end_type {
* @clk_divisor_std_fast_mode: Clock divisor in standard/fast mode. It is
* applicable if there is no fast clock source i.e. single clock
* source.
+ * @slave_read_start_delay: Workaround for AP20 I2C Slave Controller bug. Delay
+ * before writing data byte into register I2C_SL_RCVD.
*/
struct tegra_i2c_hw_feature {
@@ -133,6 +144,7 @@ struct tegra_i2c_hw_feature {
bool has_single_clk_source;
int clk_divisor_hs_mode;
int clk_divisor_std_fast_mode;
+ int slave_read_start_delay;
};
/**
@@ -173,6 +185,7 @@ struct tegra_i2c_dev {
int msg_read;
u32 bus_clk_rate;
bool is_suspended;
+ struct i2c_client *slave;
};
static void dvc_writel(struct tegra_i2c_dev *i2c_dev, u32 val, unsigned long
reg)
@@ -461,12 +474,78 @@ static int tegra_i2c_init(struct tegra_i2c_dev *i2c_dev)
return err;
}
+static void tegra_i2c_slave_write(struct tegra_i2c_dev *i2c_dev, u32 val)
+{
+ i2c_writel(i2c_dev, val, I2C_SL_RCVD);
+
+ /*
+* TODO: A correct fix needs to be found for this.
+*
+* We experience less incomplete messages with this delay than without
+* it, but we don't know why. Help is appreciated.
+*/
+ udelay(100);
+}
+
+static int tegra_i2c_slave_isr(int irq, struct tegra_i2c_dev *i2c_dev)
+{
+ u32 status;
+ u8 value;
+ u8 dummy;
+ u32 is_slave_irq, is_read, is_trans_start, is_trans_end;
+
+ if (!i2c_dev-slave || !i2c_dev-slave-slave_cb)
+ return -EINVAL;
+
+ status = i2c_readl(i2c_dev, I2C_SL_STATUS);
+
+ is_slave_irq = (status I2C_SL_ST_IRQ);
+ is_read = (status I2C_SL_ST_RNW);
+ is_trans_start = (status I2C_SL_ST_RCVD);
+ is_trans_end = (status I2C_SL_ST_END_TRANS);
+
+ if (!is_slave_irq)
+ return -EINVAL;
+
+ /* master sent stop */
+ if (is_trans_end) {
+ i2c_slave_event(i2c_dev-slave, I2C_SLAVE_STOP, dummy);
+ if (!is_trans_start)
+ return 0;
+ }
+
+ if (is_read) {
+ /* i2c master reads data from us */
+ i2c_slave_event(i2c_dev-slave,
+ is_trans_start ? I2C_SLAVE_READ_REQUESTED
+ : I2C_SLAVE_READ_PROCESSED,
+ value);
+ if (is_trans_start i2c_dev-hw-slave_read_start_delay)
+
[PATCH v2 3/4] staging/nvec: remove old code
Signed-off-by: Andrey Danin danind...@mail.ru
---
Changes for v2:
- it is initial verion
---
drivers/staging/nvec/nvec.c | 211
1 file changed, 211 deletions(-)
diff --git a/drivers/staging/nvec/nvec.c b/drivers/staging/nvec/nvec.c
index f4c5527..f39f516 100644
--- a/drivers/staging/nvec/nvec.c
+++ b/drivers/staging/nvec/nvec.c
@@ -516,23 +516,6 @@ static void nvec_rx_completed(struct nvec_chip *nvec)
schedule_work(nvec-rx_work);
}
-#if 0
-/**
- * nvec_invalid_flags - Send an error message about invalid flags and jump
- * @nvec: The nvec device
- * @status: The status flags
- * @reset: Whether we shall jump to state 0.
- */
-static void nvec_invalid_flags(struct nvec_chip *nvec, unsigned int status,
- bool reset)
-{
- dev_err(nvec-dev, unexpected status flags 0x%02x during state %i\n,
- status, nvec-state);
- if (reset)
- nvec-state = 0;
-}
-#endif /* FIXME: remove old code */
-
/**
* nvec_tx_set - Set the message to transfer (nvec-tx)
* @nvec: A struct nvec_chip
@@ -562,200 +545,6 @@ static void nvec_tx_set(struct nvec_chip *nvec)
(uint)nvec-tx-size, nvec-tx-data[1]);
}
-
-#if 0
-/**
- * nvec_interrupt - Interrupt handler
- * @irq: The IRQ
- * @dev: The nvec device
- *
- * Interrupt handler that fills our RX buffers and empties our TX
- * buffers. This uses a finite state machine with ridiculous amounts
- * of error checking, in order to be fairly reliable.
- */
-static irqreturn_t nvec_interrupt(int irq, void *dev)
-{
- unsigned long status;
- unsigned int received = 0;
- unsigned char to_send = 0xff;
- const unsigned long irq_mask = I2C_SL_IRQ | END_TRANS | RCVD | RNW;
- struct nvec_chip *nvec = dev;
- unsigned int state = nvec-state;
-
- status = readl(nvec-base + I2C_SL_STATUS);
-
- /* Filter out some errors */
- if ((status irq_mask) == 0 (status ~irq_mask) != 0) {
- dev_err(nvec-dev, unexpected irq mask %lx\n, status);
- return IRQ_HANDLED;
- }
- if ((status I2C_SL_IRQ) == 0) {
- dev_err(nvec-dev, Spurious IRQ\n);
- return IRQ_HANDLED;
- }
-
- /* The EC did not request a read, so it send us something, read it */
- if ((status RNW) == 0) {
- received = readl(nvec-base + I2C_SL_RCVD);
- if (status RCVD)
- writel(0, nvec-base + I2C_SL_RCVD);
- }
-
- if (status == (I2C_SL_IRQ | RCVD))
- nvec-state = 0;
-
- switch (nvec-state) {
- case 0: /* Verify that its a transfer start, the rest later */
- if (status != (I2C_SL_IRQ | RCVD))
- nvec_invalid_flags(nvec, status, false);
- break;
- case 1: /* command byte */
- if (status != I2C_SL_IRQ) {
- nvec_invalid_flags(nvec, status, true);
- } else {
- nvec-rx = nvec_msg_alloc(nvec, NVEC_MSG_RX);
- /* Should not happen in a normal world */
- if (unlikely(nvec-rx == NULL)) {
- nvec-state = 0;
- break;
- }
- nvec-rx-data[0] = received;
- nvec-rx-pos = 1;
- nvec-state = 2;
- }
- break;
- case 2: /* first byte after command */
- if (status == (I2C_SL_IRQ | RNW | RCVD)) {
- udelay(33);
- if (nvec-rx-data[0] != 0x01) {
- dev_err(nvec-dev,
- Read without prior read command\n);
- nvec-state = 0;
- break;
- }
- nvec_msg_free(nvec, nvec-rx);
- nvec-state = 3;
- nvec_tx_set(nvec);
- BUG_ON(nvec-tx-size 1);
- to_send = nvec-tx-data[0];
- nvec-tx-pos = 1;
- } else if (status == (I2C_SL_IRQ)) {
- BUG_ON(nvec-rx == NULL);
- nvec-rx-data[1] = received;
- nvec-rx-pos = 2;
- nvec-state = 4;
- } else {
- nvec_invalid_flags(nvec, status, true);
- }
- break;
- case 3: /* EC does a block read, we transmit data */
- if (status END_TRANS) {
- nvec_tx_completed(nvec);
- } else if ((status RNW) == 0 || (status RCVD)) {
- nvec_invalid_flags(nvec, status, true);
- } else if (nvec-tx nvec-tx-pos nvec-tx-size) {
- to_send
[PATCH v2 4/4] dt: paz00: define nvec as child of i2c bus
NVEC driver was reimplemented to use tegra i2c. Use common i2c bindings
for NVEC node.
Signed-off-by: Andrey Danin danind...@mail.ru
---
Changes for v2:
- swap reg and request-gpios properties
- use nvec-slave instead of nvec to keep ABI compatibility
- place doc in separate patch
---
arch/arm/boot/dts/tegra20-paz00.dts | 22 +-
1 file changed, 9 insertions(+), 13 deletions(-)
diff --git a/arch/arm/boot/dts/tegra20-paz00.dts
b/arch/arm/boot/dts/tegra20-paz00.dts
index ed7e100..cd5e6ef 100644
--- a/arch/arm/boot/dts/tegra20-paz00.dts
+++ b/arch/arm/boot/dts/tegra20-paz00.dts
@@ -288,20 +288,16 @@
clock-frequency = 10;
};
- nvec@7000c500 {
- compatible = nvidia,nvec;
- reg = 0x7000c500 0x100;
- interrupts = GIC_SPI 92 IRQ_TYPE_LEVEL_HIGH;
- #address-cells = 1;
- #size-cells = 0;
+ i2c@7000c500 {
+ status = okay;
clock-frequency = 8;
- request-gpios = gpio TEGRA_GPIO(V, 2) GPIO_ACTIVE_HIGH;
- slave-addr = 138;
- clocks = tegra_car TEGRA20_CLK_I2C3,
-tegra_car TEGRA20_CLK_PLL_P_OUT3;
- clock-names = div-clk, fast-clk;
- resets = tegra_car 67;
- reset-names = i2c;
+
+ nvec: nvec@45 {
+ compatible = nvidia,nvec-slave;
+ reg = 0x45;
+ request-gpios = gpio TEGRA_GPIO(V, 2)
+ GPIO_ACTIVE_HIGH;
+ };
};
i2c@7000d000 {
--
1.9.1
--
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
