Re: [PATCH] i2c: davinci: Fix bus rate calculation on Keystone SoC
On 06/18/2015 05:00 AM, Sekhar Nori wrote:
On Thursday 18 June 2015 02:23 PM, Alexander Sverdlin wrote:
According to KeyStone Architecture I2C User Guide,
module clock frequency
master clock frequency = --
(ICCL + 6) + (ICCH + 6)
i.e. d in i2c_davinci_calc_clk_dividers() should be fixed and
not dependent from module clock prescaler PSC on these SoCs.
Signed-off-by: Alexander Sverdlin alexander.sverd...@nokia.com
---
RFC: If someone from TI has an idea how to improve the coverage of future
Keystone
revisions -- hints/patches are welcome. The current ID check is based on
Davinci/Keystone datasheets and is at least working on real Keystone II.
+ Murali who works on Keystone devices in TI.
+ Grygorii
+ Grygorii has been involved in the Keystone related enhancement and
reviewing prior patches. Need to have his ack for this change
Murali
drivers/i2c/busses/i2c-davinci.c | 10 ++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index 4a110af..3d78f6a 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -60,6 +60,8 @@
#define DAVINCI_I2C_IVR_REG 0x28
#define DAVINCI_I2C_EMDR_REG 0x2c
#define DAVINCI_I2C_PSC_REG 0x30
+#define DAVINCI_I2C_ICPID1_REG 0x34
+#define DAVINCI_I2C_ICPID2_REG 0x38
#define DAVINCI_I2C_FUNC_REG 0x48
#define DAVINCI_I2C_DIR_REG 0x4c
#define DAVINCI_I2C_DIN_REG 0x50
@@ -203,6 +205,9 @@ static void i2c_davinci_calc_clk_dividers(struct
davinci_i2c_dev *dev)
* where if PSC == 0, d = 7,
* if PSC == 1, d = 6
* if PSC 1 , d = 5
+*
+* Note:
+* d is always 6 on Keystone I2C controller
*/
/* get minimum of 7 MHz clock, but max of 12 MHz */
@@ -211,6 +216,11 @@ static void i2c_davinci_calc_clk_dividers(struct
davinci_i2c_dev *dev)
psc++; /* better to run under spec than over */
d = (psc = 2) ? 5 : 7 - psc;
+ if (davinci_i2c_read_reg(dev, DAVINCI_I2C_ICPID2_REG) == 0x2206) {
+ dev_dbg(dev-dev, Keystone SoC detected\n);
+ d = 6;
+ }
I think its better to use a different compatible string for i2c on
keystone devices rather than using a fixed hardcoded IP version.
Thanks,
Sekhar
--
Murali Karicheri
Linux Kernel, Keystone
--
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: davinci: Add block read functionality for IPMI
On 5/22/2014 5:00 AM, Wolfram Sang wrote:
Hi,
thanks for the patch.
+/* capabilities */
+#define I2C_CAPABILITIES (I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | \
+I2C_FUNC_SMBUS_READ_BLOCK_DATA)
I don't see the need for a seperate define.
+
struct davinci_i2c_dev {
struct device *dev;
void __iomem*base;
@@ -318,7 +322,13 @@ i2c_davinci_xfer_msg(struct i2c_adapter *adap, struct
i2c_msg
*msg, int stop)
davinci_i2c_write_reg(dev, DAVINCI_I2C_SAR_REG, msg-addr);
dev-buf = msg-buf;
- dev-buf_len = msg-len;
+
+/* if first received byte is length, set buf_len = 0x as flag */
+ if (msg-flags I2C_M_RECV_LEN)
+ dev-buf_len = 0x;
a) this magic value should be a define instead of a comment
b) i2c messages easily have a 16 bit range, so 0x is a troublesome
choice.
+ else
+ dev-buf_len = msg-len;
+
dev-stop = stop;
davinci_i2c_write_reg(dev, DAVINCI_I2C_CNT_REG, dev-buf_len); @@ -456,7
+466,7 @@ i2c_davinci_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int
num)
static u32 i2c_davinci_func(struct i2c_adapter *adap) {
- return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
+ return I2C_CAPABILITIES;
}
static void terminate_read(struct davinci_i2c_dev *dev) @@ -528,10 +538,32 @@
static
irqreturn_t i2c_davinci_isr(int this_irq, void *dev_id)
case DAVINCI_I2C_IVR_RDR:
if (dev-buf_len) {
- *dev-buf++ =
- davinci_i2c_read_reg(dev,
-DAVINCI_I2C_DRR_REG);
+ *dev-buf++ = davinci_i2c_read_reg(dev,
+ DAVINCI_I2C_DRR_REG);
+ /*
+* check if the first received byte is message
+* length, i.e, I2C_M_RECV_LEN
+*/
+ if (dev-buf_len == 0x)
+ dev-buf_len = *(dev-buf - 1) + 1;
Please rework the code to get rid of the '- 1' and '+ 1'. They look
hackish and make the code less readable.
+
dev-buf_len--;
+ /*
+* send NACK/STOP bits BEFORE last byte is
+* received
+*/
+ if (dev-buf_len == 1) {
+ w = davinci_i2c_read_reg(dev,
+ DAVINCI_I2C_MDR_REG);
+ w |= DAVINCI_I2C_MDR_NACK;
+ davinci_i2c_write_reg(dev,
+ DAVINCI_I2C_MDR_REG, w);
+
+ w |= DAVINCI_I2C_MDR_STP;
+ davinci_i2c_write_reg(dev,
+ DAVINCI_I2C_MDR_REG, w);
+ }
+
Looks like an unreleated change to me? Why is this I2C_M_RECV_LEN
specific?
Kind regards,
Wolfram
Wolfram,
Thanks for reviewing the patch. I will review your comments and get back
to you. This patch
is tested on a customer board and thus it might be a while before I can
incorporate these changes
and provide an updated patch to the list. Meanwhile I will be reviewing
the comment in the
next few weeks and get back to you in case I have questions.
Thanks and regards,
Murali
--
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] i2c: davinci: Add block read functionality for IPMI
Intelligent Plaform Management Interface (IPMI) requires I2C driver
to support block read, where the first byte received from slave is
the length of following data:-
Added length check if the read type is block read (I2C_M_RECV_LEN)
Send NACK/STOP bits before last byte is received
Signed-off-by: Garrett Ding g-d...@ti.com
Signed-off-by: Murali Karicheri m-kariche...@ti.com
Tested-by: Garrett Ding g-d...@ti.com
CC: Sekhar Nori nsek...@ti.com
CC: Kevin Hilman khil...@deeprootsystems.com
CC: Wolfram Sang w...@the-dreams.de
CC: Santosh Shilimkar santosh.shilim...@ti.com
---
Tested on a customer board based on K2HK SoC
drivers/i2c/busses/i2c-davinci.c | 42 +-
1 file changed, 37 insertions(+), 5 deletions(-)
diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index 389bc68..cd97920 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -97,6 +97,10 @@
#define DAVINCI_I2C_IMR_NACK BIT(1)
#define DAVINCI_I2C_IMR_AL BIT(0)
+/* capabilities */
+#define I2C_CAPABILITIES (I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | \
+I2C_FUNC_SMBUS_READ_BLOCK_DATA)
+
struct davinci_i2c_dev {
struct device *dev;
void __iomem*base;
@@ -318,7 +322,13 @@ i2c_davinci_xfer_msg(struct i2c_adapter *adap, struct
i2c_msg *msg, int stop)
davinci_i2c_write_reg(dev, DAVINCI_I2C_SAR_REG, msg-addr);
dev-buf = msg-buf;
- dev-buf_len = msg-len;
+
+/* if first received byte is length, set buf_len = 0x as flag */
+ if (msg-flags I2C_M_RECV_LEN)
+ dev-buf_len = 0x;
+ else
+ dev-buf_len = msg-len;
+
dev-stop = stop;
davinci_i2c_write_reg(dev, DAVINCI_I2C_CNT_REG, dev-buf_len);
@@ -456,7 +466,7 @@ i2c_davinci_xfer(struct i2c_adapter *adap, struct i2c_msg
msgs[], int num)
static u32 i2c_davinci_func(struct i2c_adapter *adap)
{
- return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
+ return I2C_CAPABILITIES;
}
static void terminate_read(struct davinci_i2c_dev *dev)
@@ -528,10 +538,32 @@ static irqreturn_t i2c_davinci_isr(int this_irq, void
*dev_id)
case DAVINCI_I2C_IVR_RDR:
if (dev-buf_len) {
- *dev-buf++ =
- davinci_i2c_read_reg(dev,
-DAVINCI_I2C_DRR_REG);
+ *dev-buf++ = davinci_i2c_read_reg(dev,
+ DAVINCI_I2C_DRR_REG);
+ /*
+* check if the first received byte is message
+* length, i.e, I2C_M_RECV_LEN
+*/
+ if (dev-buf_len == 0x)
+ dev-buf_len = *(dev-buf - 1) + 1;
+
dev-buf_len--;
+ /*
+* send NACK/STOP bits BEFORE last byte is
+* received
+*/
+ if (dev-buf_len == 1) {
+ w = davinci_i2c_read_reg(dev,
+ DAVINCI_I2C_MDR_REG);
+ w |= DAVINCI_I2C_MDR_NACK;
+ davinci_i2c_write_reg(dev,
+ DAVINCI_I2C_MDR_REG, w);
+
+ w |= DAVINCI_I2C_MDR_STP;
+ davinci_i2c_write_reg(dev,
+ DAVINCI_I2C_MDR_REG, w);
+ }
+
if (dev-buf_len)
continue;
--
1.7.9.5
--
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
