[PATCH-v5 0/5] i2c: pxa: Add support for PXA910 family of device

[PATCH-v5 0/5] i2c: pxa: Add support for PXA910 family of device

[Vaibhav Hiremath]

This patch-series is subset of the original patch-series, submitted
on 14 Jul 2015.
Link to Original Patch-series - https://lkml.org/lkml/2015/7/14/80

The first six patches have been already queued up for upstream. So this
patch-series is respin of remaining 5 patches.

Testing:
  - Basic testing on PMIC device on I2C-0 interface
  - Boot tested on platform based on PXA1928
  - Read few registers of PMIC (RTC, ID, etc...) during boot

V4 => V5
=======
Link to V3: https://lkml.org/lkml/2015/7/14/80
  - Dropped First 6 patches as they are already accepted, queued for upstream
  - Fixed a bug in PATCH [5/5], where for non PXA910 devices it as resulting into
    NULL pointer dereference.


For the record, pasting all the details from original patch-series -

V3 => V4
=======
Link to V3: http://www.spinics.net/lists/devicetree/msg85904.html
 - [PATCH 06/11] Removed unnecessary dev_err on devm_kzalloc() check
 - [PATCH 06/11] Removed return check on platform_get_resource(), as 
   devm_ioremap_resource() does it for us.
   Also, brought up the devm_ioremap_resource() function call in the execution
   sequence, as no point in delaying it if we do not have resource.
   It make sense, after this change.
 - [PATCH 04/11] Typecast changed to 'enum pxa_i2c_types'
   Also updated the subject line "Removed ==> Fix"

V2 => V3
=======
Link to V2: http://www.spinics.net/lists/linux-i2c/msg20059.html
 - Removed PATCH [4/12] related to reset of I2C module.
   Suggested by "Robert Jarzmik"
 - Updated commit description for,
    PATCH [11/12]: Mentioned reasoning about moment of clk_get code.
    PATCH [12/12]: for DT property node.
 - Added Acked by "Robert Jarzmik" to patched which he acked.

V1 => V2:
========
Link to V1 - http://lists.infradead.org/pipermail/linux-arm-kernel/2015-May/347012.html
 - Fixed all comments from "Robert Jarzmik" and "Wolfram Sang"
 - Dropped Patch
    05/12: using core bus reset implementation - under work.
    Will submit shortly.
    08/12: NAKed and dropped
 - Separated DT binding patch from driver changes, for easy merge


Vaibhav Hiremath (4):
  Documentation: binding: add new property 'disable_after_xfer' to
    i2c-pxa
  i2c: pxa: Add support for pxa910/988 & new configuration features
  Documentation: binding: add sclk adjustment properties to i2c-pxa
  i2c: pxa: Add ILCR (tLow & tHigh) configuration support

Yi Zhang (1):
  i2c: pxa: enable/disable i2c module across msg xfer

 Documentation/devicetree/bindings/i2c/i2c-pxa.txt |  18 +++
 drivers/i2c/busses/i2c-pxa.c                      | 154 ++++++++++++++++++++--
 2 files changed, 163 insertions(+), 9 deletions(-)

-- 
1.9.1

[PATCH-v5 1/5] Documentation: binding: add new property 'disable_after_xfer' to i2c-pxa

[Vaibhav Hiremath]

Driver will now supports enable/disable across msg xfer, which user
can control it by new DT property -

i2c-disable-after-xfer : If set, driver will disable I2C module after msg
 xfer and enable it back before xfer.

Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
---
 Documentation/devicetree/bindings/i2c/i2c-pxa.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-pxa.txt b/Documentation/devicetree/bindings/i2c/i2c-pxa.txt
index 12b78ac..9657db5 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-pxa.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-pxa.txt
@@ -18,6 +18,11 @@ Recommended properties :
    status register of i2c controller instead.
  - mrvl,i2c-fast-mode : Enable fast mode of i2c controller.
 
+Optional properties :
+
+ - i2c-disable-after-xfer : If set, driver will disable I2C module
+   after msg xfer and enable it again before xfer.
+
 Examples:
 	twsi1: i2c at d4011000 {
 		compatible = "mrvl,mmp-twsi";
-- 
1.9.1

[PATCH-v5 1/5] Documentation: binding: add new property 'disable_after_xfer' to i2c-pxa

[Matt Porter]

On Tue, Jul 21, 2015 at 06:11:02PM +0530, Vaibhav Hiremath wrote:
> Driver will now supports enable/disable across msg xfer, which user
> can control it by new DT property -
> 
> i2c-disable-after-xfer : If set, driver will disable I2C module after msg
>  xfer and enable it back before xfer.

If this is a new property specific to this Marvell part, it needs
the vendor prefix as in mrvl,i2c-disable-after-xfer

Or, it couldn't hurt to start an i2c.txt for generic i2c bindings
and store it there as this and others later in this series would
reasonably apply to other controllers.

-Matt

> Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
> ---
>  Documentation/devicetree/bindings/i2c/i2c-pxa.txt | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-pxa.txt b/Documentation/devicetree/bindings/i2c/i2c-pxa.txt
> index 12b78ac..9657db5 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-pxa.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c-pxa.txt
> @@ -18,6 +18,11 @@ Recommended properties :
>     status register of i2c controller instead.
>   - mrvl,i2c-fast-mode : Enable fast mode of i2c controller.
>  
> +Optional properties :
> +
> + - i2c-disable-after-xfer : If set, driver will disable I2C module
> +   after msg xfer and enable it again before xfer.
> +
>  Examples:
>  	twsi1: i2c at d4011000 {
>  		compatible = "mrvl,mmp-twsi";
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH-v5 1/5] Documentation: binding: add new property 'disable_after_xfer' to i2c-pxa

[Vaibhav Hiremath]

On Monday 27 July 2015 07:39 PM, Matt Porter wrote:
> On Tue, Jul 21, 2015 at 06:11:02PM +0530, Vaibhav Hiremath wrote:
>> Driver will now supports enable/disable across msg xfer, which user
>> can control it by new DT property -
>>
>> i2c-disable-after-xfer : If set, driver will disable I2C module after msg
>>   xfer and enable it back before xfer.
>
> If this is a new property specific to this Marvell part, it needs
> the vendor prefix as in mrvl,i2c-disable-after-xfer
>

We discussed about this, I think in V1 or V2. Decided to use generic
name, as feature (in turn property) could be used by other drivers as
well.

> Or, it couldn't hurt to start an i2c.txt for generic i2c bindings
> and store it there as this and others later in this series would
> reasonably apply to other controllers.
>

Yeah, we could start i2c.txt, probably better to have separate new
patch all together.


Thanks,
Vaibhav

[PATCH-v5 1/5] Documentation: binding: add new property 'disable_after_xfer' to i2c-pxa

[Wolfram Sang]

> Yeah, we could start i2c.txt, probably better to have separate new
> patch all together.

I will start such a file today as part of the i2c slave framework
update which introduces flags to the reg property. Will post to the i2c
list this week.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150805/6af38758/attachment.sig>

[PATCH-v5 1/5] Documentation: binding: add new property 'disable_after_xfer' to i2c-pxa

[Vaibhav Hiremath]

On Wednesday 05 August 2015 02:19 PM, Wolfram Sang wrote:
> I will start such a file today as part of the i2c slave framework
> update which introduces flags to the reg property. Will post to the i2c
> list this week.

Great and thanks for taking this.

I believe, better to wait for your patch and then rebase my patch-
series against it. right?

Request to review the this patch-series, so that I can incorporate all
comments in V6.

Thanks,
Vaibhav

[PATCH-v5 1/5] Documentation: binding: add new property 'disable_after_xfer' to i2c-pxa

[Matt Porter]

On Wed, Aug 05, 2015 at 10:49:18AM +0200, Wolfram Sang wrote:
> 
> > Yeah, we could start i2c.txt, probably better to have separate new
> > patch all together.
> 
> I will start such a file today as part of the i2c slave framework
> update which introduces flags to the reg property. Will post to the i2c
> list this week.
> 

That sounds great, Wolfram, that will be a nice improvement for the
binding docs.

Thanks,
Matt

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150805/abf4d5c3/attachment.sig>

[PATCH-v5 1/5] Documentation: binding: add new property 'disable_after_xfer' to i2c-pxa

[Matt Porter]

On Wed, Aug 05, 2015 at 12:04:25PM +0530, Vaibhav Hiremath wrote:
> 
> 
> On Monday 27 July 2015 07:39 PM, Matt Porter wrote:
> >On Tue, Jul 21, 2015 at 06:11:02PM +0530, Vaibhav Hiremath wrote:
> >>Driver will now supports enable/disable across msg xfer, which user
> >>can control it by new DT property -
> >>
> >>i2c-disable-after-xfer : If set, driver will disable I2C module after msg
> >>  xfer and enable it back before xfer.
> >
> >If this is a new property specific to this Marvell part, it needs
> >the vendor prefix as in mrvl,i2c-disable-after-xfer
> >
> 
> We discussed about this, I think in V1 or V2. Decided to use generic
> name, as feature (in turn property) could be used by other drivers as
> well.

Ahh, ok, thanks...coming in too late on this one. :)

> >Or, it couldn't hurt to start an i2c.txt for generic i2c bindings
> >and store it there as this and others later in this series would
> >reasonably apply to other controllers.
> >
> 
> Yeah, we could start i2c.txt, probably better to have separate new
> patch all together.

Great, yes, I've always found it hard to follow i2c generic bindings
since they aren't defined in a clear place like other similar hardware
(e.g. SPI).

-Matt

[PATCH-v5 1/5] Documentation: binding: add new property 'disable_after_xfer' to i2c-pxa

[Vaibhav Hiremath]

On Wednesday 05 August 2015 07:50 PM, Matt Porter wrote:
> On Wed, Aug 05, 2015 at 12:04:25PM +0530, Vaibhav Hiremath wrote:
>>
>>
>> On Monday 27 July 2015 07:39 PM, Matt Porter wrote:
>>> On Tue, Jul 21, 2015 at 06:11:02PM +0530, Vaibhav Hiremath wrote:
>>>> Driver will now supports enable/disable across msg xfer, which user
>>>> can control it by new DT property -
>>>>
>>>> i2c-disable-after-xfer : If set, driver will disable I2C module after msg
>>>>   xfer and enable it back before xfer.
>>>
>>> If this is a new property specific to this Marvell part, it needs
>>> the vendor prefix as in mrvl,i2c-disable-after-xfer
>>>
>>
>> We discussed about this, I think in V1 or V2. Decided to use generic
>> name, as feature (in turn property) could be used by other drivers as
>> well.
>
> Ahh, ok, thanks...coming in too late on this one. :)
>
>>> Or, it couldn't hurt to start an i2c.txt for generic i2c bindings
>>> and store it there as this and others later in this series would
>>> reasonably apply to other controllers.
>>>
>>
>> Yeah, we could start i2c.txt, probably better to have separate new
>> patch all together.
>
> Great, yes, I've always found it hard to follow i2c generic bindings
> since they aren't defined in a clear place like other similar hardware
> (e.g. SPI).
>

Wolfram has already owned up on creating i2c.txt for generic properties.
So, we will be soon close to others.

And thanks for your comments.
Please let me know if you have any other comments on patch-series, so
that I can incorporate it in next version.

Thanks,
Vaibhav

[PATCH-v5 2/5] i2c: pxa: enable/disable i2c module across msg xfer

[Vaibhav Hiremath]

From: Yi Zhang <yizhang@marvell.com>

Enable i2c module/unit before transmission and disable when it
finishes.

why?
It's because the i2c bus may be disturbed if the slave device,
typically a touch, powers on.

As we do not want to break slave mode support, this patch introduces
DT property to control disable of the I2C module after xfer in master
mode of operation.

i2c-disable-after-xfer : If set, driver will disable I2C module after
msg xfer

Signed-off-by: Yi Zhang <yizhang@marvell.com>
Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
---
 drivers/i2c/busses/i2c-pxa.c | 43 +++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 41 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
index 66cf437..abf04f2 100644
--- a/drivers/i2c/busses/i2c-pxa.c
+++ b/drivers/i2c/busses/i2c-pxa.c
@@ -161,6 +161,7 @@ struct pxa_i2c {
 	unsigned char		master_code;
 	unsigned long		rate;
 	bool			highmode_enter;
+	bool			disable_after_xfer;
 };
 
 #define _IBMR(i2c)	((i2c)->reg_ibmr)
@@ -284,6 +285,24 @@ static void i2c_pxa_scream_blue_murder(struct pxa_i2c *i2c, const char *why)
 static void i2c_pxa_master_complete(struct pxa_i2c *i2c, int ret);
 static irqreturn_t i2c_pxa_handler(int this_irq, void *dev_id);
 
+/* enable/disable i2c unit */
+static inline int i2c_pxa_is_enabled(struct pxa_i2c *i2c)
+{
+	return (readl(_ICR(i2c)) & ICR_IUE);
+}
+
+static inline void i2c_pxa_enable(struct pxa_i2c *i2c, bool enable)
+{
+	if (enable) {
+		if (!i2c_pxa_is_enabled(i2c)) {
+			writel(readl(_ICR(i2c)) | ICR_IUE, _ICR(i2c));
+			udelay(100);
+		}
+	} else {
+		writel(readl(_ICR(i2c)) & ~ICR_IUE, _ICR(i2c));
+	}
+}
+
 static inline int i2c_pxa_is_slavemode(struct pxa_i2c *i2c)
 {
 	return !(readl(_ICR(i2c)) & ICR_SCLE);
@@ -480,8 +499,7 @@ static void i2c_pxa_reset(struct pxa_i2c *i2c)
 	i2c_pxa_set_slave(i2c, 0);
 
 	/* enable unit */
-	writel(readl(_ICR(i2c)) | ICR_IUE, _ICR(i2c));
-	udelay(100);
+	i2c_pxa_enable(i2c, true);
 }
 
 
@@ -832,6 +850,9 @@ static int i2c_pxa_pio_xfer(struct i2c_adapter *adap,
 	struct pxa_i2c *i2c = adap->algo_data;
 	int ret, i;
 
+	/* Enable i2c unit */
+	i2c_pxa_enable(i2c, true);
+
 	/* If the I2C controller is disabled we need to reset it
 	  (probably due to a suspend/resume destroying state). We do
 	  this here as we can then avoid worrying about resuming the
@@ -852,6 +873,11 @@ static int i2c_pxa_pio_xfer(struct i2c_adapter *adap,
 	ret = -EREMOTEIO;
  out:
 	i2c_pxa_set_slave(i2c, ret);
+
+	/* disable i2c unit */
+	if (i2c->disable_after_xfer)
+		i2c_pxa_enable(i2c, false);
+
 	return ret;
 }
 
@@ -1067,6 +1093,9 @@ static int i2c_pxa_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num
 	struct pxa_i2c *i2c = adap->algo_data;
 	int ret, i;
 
+	/* Enable i2c unit */
+	i2c_pxa_enable(i2c, true);
+
 	for (i = adap->retries; i >= 0; i--) {
 		ret = i2c_pxa_do_xfer(i2c, msgs, num);
 		if (ret != I2C_RETRY)
@@ -1080,6 +1109,10 @@ static int i2c_pxa_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num
 	ret = -EREMOTEIO;
  out:
 	i2c_pxa_set_slave(i2c, ret);
+	/* disable i2c unit */
+	if (i2c->disable_after_xfer)
+		i2c_pxa_enable(i2c, false);
+
 	return ret;
 }
 
@@ -1120,6 +1153,9 @@ static int i2c_pxa_probe_dt(struct platform_device *pdev, struct pxa_i2c *i2c,
 	/* For device tree we always use the dynamic or alias-assigned ID */
 	i2c->adap.nr = -1;
 
+	i2c->disable_after_xfer = of_property_read_bool(np,
+				"i2c-disable-after-xfer");
+
 	if (of_get_property(np, "mrvl,i2c-polling", NULL))
 		i2c->use_pio = 1;
 	if (of_get_property(np, "mrvl,i2c-fast-mode", NULL))
@@ -1264,6 +1300,9 @@ static int i2c_pxa_probe(struct platform_device *dev)
 
 	platform_set_drvdata(dev, i2c);
 
+	if (i2c->disable_after_xfer)
+		i2c_pxa_enable(i2c, false);
+
 #ifdef CONFIG_I2C_PXA_SLAVE
 	dev_info(&i2c->adap.dev, " PXA I2C adapter, slave address %d\n",
 		i2c->slave_addr);
-- 
1.9.1

[PATCH-v5 2/5] i2c: pxa: enable/disable i2c module across msg xfer

[Shubhrajyoti Datta]

hi ,
On Tue, Jul 21, 2015 at 6:11 PM, Vaibhav Hiremath
<vaibhav.hiremath@linaro.org> wrote:
> From: Yi Zhang <yizhang@marvell.com>
>
> Enable i2c module/unit before transmission and disable when it
> finishes.
>
> why?
> It's because the i2c bus may be disturbed if the slave device,
> typically a touch, powers on.

Why should that be an issue?
Is it an errata.

In which mode it is an issue slave / master or both?

>
> As we do not want to break slave mode support, this patch introduces
> DT property to control disable of the I2C module after xfer in master
> mode of operation.
>
> i2c-disable-after-xfer : If set, driver will disable I2C module after
> msg xfer
>
> Signed-off-by: Yi Zhang <yizhang@marvell.com>
> Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
> ---

[PATCH-v5 2/5] i2c: pxa: enable/disable i2c module across msg xfer

[Vaibhav Hiremath]

On Wednesday 05 August 2015 04:16 PM, Shubhrajyoti Datta wrote:
> hi ,
> On Tue, Jul 21, 2015 at 6:11 PM, Vaibhav Hiremath
> <vaibhav.hiremath@linaro.org> wrote:
>> From: Yi Zhang <yizhang@marvell.com>
>>
>> Enable i2c module/unit before transmission and disable when it
>> finishes.
>>
>> why?
>> It's because the i2c bus may be disturbed if the slave device,
>> typically a touch, powers on.
>
> Why should that be an issue?
> Is it an errata.
>

Not I am aware of.

> In which mode it is an issue slave / master or both?
>

since it talks about touch, I expect it to be master mode.

Thanks,
Vaibhav

[PATCH-v5 3/5] i2c: pxa: Add support for pxa910/988 & new configuration features

[Vaibhav Hiremath]

TWSI_ILCR & TWSI_IWCR registers are used to adjust clock rate
of standard & fast mode in pxa910/988; so this patch adds these two new
entries to "struct pxa_reg_layout" and "struct pxa_i2c".

As discussed in the previous patch-series, the idea here is to add standard
DT properties for ilcr and iwcr configuration fields.
In case of Master ilcr is used for low/high time and in case of slave mode
of operation iwcr is used for setup/hold time.

Signed-off-by: Jett.Zhou <jtzhou@marvell.com>
Signed-off-by: Yi Zhang <yizhang@marvell.com>
Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
---
 drivers/i2c/busses/i2c-pxa.c | 42 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 41 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
index abf04f2..8d76197 100644
--- a/drivers/i2c/busses/i2c-pxa.c
+++ b/drivers/i2c/busses/i2c-pxa.c
@@ -46,12 +46,15 @@ struct pxa_reg_layout {
 	u32 icr;
 	u32 isr;
 	u32 isar;
+	u32 ilcr;
+	u32 iwcr;
 };
 
 enum pxa_i2c_types {
 	REGS_PXA2XX,
 	REGS_PXA3XX,
 	REGS_CE4100,
+	REGS_PXA910,
 };
 
 /*
@@ -79,12 +82,22 @@ static struct pxa_reg_layout pxa_reg_layout[] = {
 		.isr =	0x04,
 		/* no isar register */
 	},
+	[REGS_PXA910] = {
+		.ibmr = 0x00,
+		.idbr = 0x08,
+		.icr =	0x10,
+		.isr =	0x18,
+		.isar = 0x20,
+		.ilcr = 0x28,
+		.iwcr = 0x30,
+	},
 };
 
 static const struct platform_device_id i2c_pxa_id_table[] = {
 	{ "pxa2xx-i2c",		REGS_PXA2XX },
 	{ "pxa3xx-pwri2c",	REGS_PXA3XX },
 	{ "ce4100-i2c",		REGS_CE4100 },
+	{ "pxa910-i2c",		REGS_PXA910 },
 	{ },
 };
 MODULE_DEVICE_TABLE(platform, i2c_pxa_id_table);
@@ -124,6 +137,24 @@ MODULE_DEVICE_TABLE(platform, i2c_pxa_id_table);
 #define ISR_SAD		(1 << 9)	   /* slave address detected */
 #define ISR_BED		(1 << 10)	   /* bus error no ACK/NAK */
 
+/* bit field shift & mask */
+#define ILCR_SLV_SHIFT		0
+#define ILCR_SLV_MASK		(0x1FF << ILCR_SLV_SHIFT)
+#define ILCR_FLV_SHIFT		9
+#define ILCR_FLV_MASK		(0x1FF << ILCR_FLV_SHIFT)
+#define ILCR_HLVL_SHIFT		18
+#define ILCR_HLVL_MASK		(0x1FF << ILCR_HLVL_SHIFT)
+#define ILCR_HLVH_SHIFT		27
+#define ILCR_HLVH_MASK		(0x1F << ILCR_HLVH_SHIFT)
+
+#define IWCR_CNT_SHIFT		0
+#define IWCR_CNT_MASK		(0x1F << IWCR_CNT_SHIFT)
+#define IWCR_HS_CNT1_SHIFT	5
+#define IWCR_HS_CNT1_MASK	(0x1F << IWCR_HS_CNT1_SHIFT)
+#define IWCR_HS_CNT2_SHIFT	10
+#define IWCR_HS_CNT2_MASK	(0x1F << IWCR_HS_CNT2_SHIFT)
+
+
 struct pxa_i2c {
 	spinlock_t		lock;
 	wait_queue_head_t	wait;
@@ -150,6 +181,8 @@ struct pxa_i2c {
 	void __iomem		*reg_icr;
 	void __iomem		*reg_isr;
 	void __iomem		*reg_isar;
+	void __iomem		*reg_ilcr;
+	void __iomem		*reg_iwcr;
 
 	unsigned long		iobase;
 	unsigned long		iosize;
@@ -169,6 +202,8 @@ struct pxa_i2c {
 #define _ICR(i2c)	((i2c)->reg_icr)
 #define _ISR(i2c)	((i2c)->reg_isr)
 #define _ISAR(i2c)	((i2c)->reg_isar)
+#define _ILCR(i2c)	((i2c)->reg_ilcr)
+#define _IWCR(i2c)	((i2c)->reg_iwcr)
 
 /*
  * I2C Slave mode address
@@ -1135,7 +1170,7 @@ static const struct i2c_algorithm i2c_pxa_pio_algorithm = {
 static const struct of_device_id i2c_pxa_dt_ids[] = {
 	{ .compatible = "mrvl,pxa-i2c", .data = (void *)REGS_PXA2XX },
 	{ .compatible = "mrvl,pwri2c", .data = (void *)REGS_PXA3XX },
-	{ .compatible = "mrvl,mmp-twsi", .data = (void *)REGS_PXA2XX },
+	{ .compatible = "mrvl,mmp-twsi", .data = (void *)REGS_PXA910 },
 	{}
 };
 MODULE_DEVICE_TABLE(of, i2c_pxa_dt_ids);
@@ -1243,6 +1278,11 @@ static int i2c_pxa_probe(struct platform_device *dev)
 	if (i2c_type != REGS_CE4100)
 		i2c->reg_isar = i2c->reg_base + pxa_reg_layout[i2c_type].isar;
 
+	if (i2c_type == REGS_PXA910) {
+		i2c->reg_ilcr = i2c->reg_base + pxa_reg_layout[i2c_type].ilcr;
+		i2c->reg_iwcr = i2c->reg_base + pxa_reg_layout[i2c_type].iwcr;
+	}
+
 	i2c->iobase = res->start;
 	i2c->iosize = resource_size(res);
 
-- 
1.9.1

[PATCH-v5 4/5] Documentation: binding: add sclk adjustment properties to i2c-pxa

[Vaibhav Hiremath]

With addition of PXA910 family of devices, the TWSI module supports
new feature which allows us to adjust SCLK. i2c-pxa driver takes input
configuration in nsec and converts it to respective bit-fields,

 - i2c-sclk-low-time-ns : SCLK low time (tlow)
   This property is used along with mode selection.
 - i2c-sclk-high-time-ns : SCLK high time (thigh)
 - i2c-start-hold-time-ns : Used in case of high speed mode for start bit
   hold/setup wait counter.
 - i2c-stop-hold-time-ns : Used in case of high speed mode for stop bit
   hold/setup wait counter.
 - i2c-sda-hold-time-ns : Used to calculate hold/setup wait counter for
   standard and fast mode.

Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
---
 Documentation/devicetree/bindings/i2c/i2c-pxa.txt | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-pxa.txt b/Documentation/devicetree/bindings/i2c/i2c-pxa.txt
index 9657db5..bb4df60 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-pxa.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-pxa.txt
@@ -23,12 +23,25 @@ Optional properties :
  - i2c-disable-after-xfer : If set, driver will disable I2C module
    after msg xfer and enable it again before xfer.
 
+   (Applicable to PXA910 family):
+
+ - i2c-sclk-low-time-ns : SCLK low time (tlow), for standard/fast/high
+   speed mode.
+   This property is used along with mode selection. Driver uses this property
+   to set low/high time for standard and fast speed mode, as HW counter
+   bit-field is same for both.
+ - i2c-sclk-high-time-ns : SCLK high time (thigh), Used in case of high speed
+   mode only.
+
 Examples:
 	twsi1: i2c at d4011000 {
 		compatible = "mrvl,mmp-twsi";
 		reg = <0xd4011000 0x1000>;
 		interrupts = <7>;
 		mrvl,i2c-fast-mode;
+
+		i2c-sclk-low-time-ns = <988>;
+		i2c-sclk-high-time-ns = <988>;
 	};
 	
 	twsi2: i2c at d4025000 {
-- 
1.9.1

[PATCH-v5 4/5] Documentation: binding: add sclk adjustment properties to i2c-pxa

[Matt Porter]

On Tue, Jul 21, 2015 at 06:11:05PM +0530, Vaibhav Hiremath wrote:
> With addition of PXA910 family of devices, the TWSI module supports
> new feature which allows us to adjust SCLK. i2c-pxa driver takes input
> configuration in nsec and converts it to respective bit-fields,
> 
>  - i2c-sclk-low-time-ns : SCLK low time (tlow)
>    This property is used along with mode selection.
>  - i2c-sclk-high-time-ns : SCLK high time (thigh)
>  - i2c-start-hold-time-ns : Used in case of high speed mode for start bit
>    hold/setup wait counter.
>  - i2c-stop-hold-time-ns : Used in case of high speed mode for stop bit
>    hold/setup wait counter.
>  - i2c-sda-hold-time-ns : Used to calculate hold/setup wait counter for
>    standard and fast mode.

Again, these should have a mrvl prefix..very specific to this PXA
controller.

-Matt

> Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
> ---
>  Documentation/devicetree/bindings/i2c/i2c-pxa.txt | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-pxa.txt b/Documentation/devicetree/bindings/i2c/i2c-pxa.txt
> index 9657db5..bb4df60 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-pxa.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c-pxa.txt
> @@ -23,12 +23,25 @@ Optional properties :
>   - i2c-disable-after-xfer : If set, driver will disable I2C module
>     after msg xfer and enable it again before xfer.
>  
> +   (Applicable to PXA910 family):
> +
> + - i2c-sclk-low-time-ns : SCLK low time (tlow), for standard/fast/high
> +   speed mode.
> +   This property is used along with mode selection. Driver uses this property
> +   to set low/high time for standard and fast speed mode, as HW counter
> +   bit-field is same for both.
> + - i2c-sclk-high-time-ns : SCLK high time (thigh), Used in case of high speed
> +   mode only.
> +
>  Examples:
>  	twsi1: i2c at d4011000 {
>  		compatible = "mrvl,mmp-twsi";
>  		reg = <0xd4011000 0x1000>;
>  		interrupts = <7>;
>  		mrvl,i2c-fast-mode;
> +
> +		i2c-sclk-low-time-ns = <988>;
> +		i2c-sclk-high-time-ns = <988>;
>  	};
>  	
>  	twsi2: i2c at d4025000 {
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH-v5 4/5] Documentation: binding: add sclk adjustment properties to i2c-pxa

[Vaibhav Hiremath]

On Monday 27 July 2015 07:41 PM, Matt Porter wrote:
> On Tue, Jul 21, 2015 at 06:11:05PM +0530, Vaibhav Hiremath wrote:
>> With addition of PXA910 family of devices, the TWSI module supports
>> new feature which allows us to adjust SCLK. i2c-pxa driver takes input
>> configuration in nsec and converts it to respective bit-fields,
>>
>>   - i2c-sclk-low-time-ns : SCLK low time (tlow)
>>     This property is used along with mode selection.
>>   - i2c-sclk-high-time-ns : SCLK high time (thigh)
>>   - i2c-start-hold-time-ns : Used in case of high speed mode for start bit
>>     hold/setup wait counter.
>>   - i2c-stop-hold-time-ns : Used in case of high speed mode for stop bit
>>     hold/setup wait counter.
>>   - i2c-sda-hold-time-ns : Used to calculate hold/setup wait counter for
>>     standard and fast mode.
>
> Again, these should have a mrvl prefix..very specific to this PXA
> controller.
>

Sorry for delayed response, was down with health issue for week.

Coming back to your comment,

No, they are not Marvell specific. If you refer back to my V1, we
discussed about it and concluded that we should use generic property
here.
There are quite a few drivers using same properties for similar purpose.

Thanks,
Vaibhav

[PATCH-v5 5/5] i2c: pxa: Add ILCR (tLow & tHigh) configuration support

[Vaibhav Hiremath]

With addition of PXA910 family of devices, the TWSI module supports
SCL clock adjustment using ILCR register.

This patch enables the control and configuration of ICLR through DT
properties,

i2c-sclk-high-time-ns:
  SCLK high time (tHigh), for standard/fast/high speed mode
i2c-sclk-low-time-ns:
  SCLK low time (tLow), for standard/fast/high speed mode

Note that in case of standard and fast mod, the tLow and tHigh counters
are same, and software will use tLow value.

Also, brought up devm_clk_get() fn above i2c_pxa_probe_dt(), as it
uses clk rate for timing calculations.

Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
Signed-off-by: Jett.Zhou <jtzhou@marvell.com>
Signed-off-by: Yi Zhang <yizhang@marvell.com>
---
 drivers/i2c/busses/i2c-pxa.c | 69 ++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 63 insertions(+), 6 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
index 8d76197..6012ae5 100644
--- a/drivers/i2c/busses/i2c-pxa.c
+++ b/drivers/i2c/busses/i2c-pxa.c
@@ -195,6 +195,9 @@ struct pxa_i2c {
 	unsigned long		rate;
 	bool			highmode_enter;
 	bool			disable_after_xfer;
+
+	unsigned int		sclk_thigh_load_cnt;
+	unsigned int		sclk_tlow_load_cnt;
 };
 
 #define _IBMR(i2c)	((i2c)->reg_ibmr)
@@ -507,6 +510,36 @@ static void i2c_pxa_set_slave(struct pxa_i2c *i2c, int errcode)
 #define i2c_pxa_set_slave(i2c, err)	do { } while (0)
 #endif
 
+static void i2c_pxa_do_sclk_adj(struct pxa_i2c *i2c)
+{
+	unsigned int reg_ilcr;
+
+	if (!i2c->reg_ilcr)
+		return;
+
+	reg_ilcr = readl(_ILCR(i2c));
+
+	/* For standard/fast mode tlow and thigh counters are same */
+	if (i2c->sclk_tlow_load_cnt) {
+		unsigned int mask, shift;
+
+		mask = i2c->high_mode ? ILCR_HLVL_MASK :
+			i2c->fast_mode ? ILCR_FLV_MASK : ILCR_SLV_MASK;
+		shift = i2c->high_mode ? ILCR_HLVL_SHIFT :
+			i2c->fast_mode ? ILCR_FLV_SHIFT : ILCR_SLV_SHIFT;
+
+		reg_ilcr &= ~mask;
+		reg_ilcr |= i2c->sclk_tlow_load_cnt << shift;
+	}
+
+	if (i2c->high_mode && i2c->sclk_thigh_load_cnt) {
+		reg_ilcr &= ~ILCR_HLVH_MASK;
+		reg_ilcr |= i2c->sclk_thigh_load_cnt << ILCR_HLVH_SHIFT;
+	}
+
+	writel(reg_ilcr, _ILCR(i2c));
+}
+
 static void i2c_pxa_reset(struct pxa_i2c *i2c)
 {
 	pr_debug("Resetting I2C Controller Unit\n");
@@ -526,6 +559,8 @@ static void i2c_pxa_reset(struct pxa_i2c *i2c)
 	writel(I2C_ICR_INIT | (i2c->fast_mode ? ICR_FM : 0), _ICR(i2c));
 	writel(readl(_ICR(i2c)) | (i2c->high_mode ? ICR_HS : 0), _ICR(i2c));
 
+	i2c_pxa_do_sclk_adj(i2c);
+
 #ifdef CONFIG_I2C_PXA_SLAVE
 	dev_info(&i2c->adap.dev, "Enabling slave mode\n");
 	writel(readl(_ICR(i2c)) | ICR_SADIE | ICR_ALDIE | ICR_SSDIE, _ICR(i2c));
@@ -1198,6 +1233,26 @@ static int i2c_pxa_probe_dt(struct platform_device *pdev, struct pxa_i2c *i2c,
 
 	*i2c_types = (enum pxa_i2c_types)(of_id->data);
 
+	/* optional properties */
+	if (of_device_is_compatible(np, "mrvl,mmp-twsi")) {
+		unsigned int tlow = 0, thigh = 0;
+		unsigned int clk_ns;
+
+		/* clock time in nsec */
+		clk_ns = 1000000 / (i2c->rate / 1000);
+
+		of_property_read_u32(np, "i2c-sclk-high-time-ns", &thigh);
+		i2c->sclk_thigh_load_cnt = thigh / clk_ns;
+
+		of_property_read_u32(np, "i2c-sclk-low-time-ns", &tlow);
+		i2c->sclk_tlow_load_cnt = tlow / clk_ns;
+
+		/* For std/fast mode tlow & thigh have same bit-fields */
+		if (!i2c->high_mode &&
+			(i2c->sclk_tlow_load_cnt != i2c->sclk_thigh_load_cnt))
+			dev_warn(&i2c->adap.dev,
+				"mismatch of tLow & tHigh values, using tLow\n");
+	}
 	return 0;
 }
 
@@ -1248,6 +1303,14 @@ static int i2c_pxa_probe(struct platform_device *dev)
 		return irq;
 	}
 
+	i2c->clk = devm_clk_get(&dev->dev, NULL);
+	if (IS_ERR(i2c->clk)) {
+		dev_err(&dev->dev, "failed to get the clk: %ld\n", PTR_ERR(i2c->clk));
+		return PTR_ERR(i2c->clk);
+	}
+
+	i2c->rate = clk_get_rate(i2c->clk);
+
 	/* Default adapter num to device id; i2c_pxa_probe_dt can override. */
 	i2c->adap.nr = dev->id;
 
@@ -1265,12 +1328,6 @@ static int i2c_pxa_probe(struct platform_device *dev)
 
 	strlcpy(i2c->adap.name, "pxa_i2c-i2c", sizeof(i2c->adap.name));
 
-	i2c->clk = devm_clk_get(&dev->dev, NULL);
-	if (IS_ERR(i2c->clk)) {
-		dev_err(&dev->dev, "failed to get the clk: %ld\n", PTR_ERR(i2c->clk));
-		return PTR_ERR(i2c->clk);
-	}
-
 	i2c->reg_ibmr = i2c->reg_base + pxa_reg_layout[i2c_type].ibmr;
 	i2c->reg_idbr = i2c->reg_base + pxa_reg_layout[i2c_type].idbr;
 	i2c->reg_icr = i2c->reg_base + pxa_reg_layout[i2c_type].icr;
-- 
1.9.1

[PATCH-v5 5/5] i2c: pxa: Add ILCR (tLow & tHigh) configuration support

[Vaibhav Hiremath]

On Tuesday 21 July 2015 06:11 PM, Vaibhav Hiremath wrote:
> With addition of PXA910 family of devices, the TWSI module supports
> SCL clock adjustment using ILCR register.
>
> This patch enables the control and configuration of ICLR through DT
> properties,
>
> i2c-sclk-high-time-ns:
>    SCLK high time (tHigh), for standard/fast/high speed mode
> i2c-sclk-low-time-ns:
>    SCLK low time (tLow), for standard/fast/high speed mode
>
> Note that in case of standard and fast mod, the tLow and tHigh counters
> are same, and software will use tLow value.
>
> Also, brought up devm_clk_get() fn above i2c_pxa_probe_dt(), as it
> uses clk rate for timing calculations.
>
> Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
> Signed-off-by: Jett.Zhou <jtzhou@marvell.com>
> Signed-off-by: Yi Zhang <yizhang@marvell.com>
> ---
>   drivers/i2c/busses/i2c-pxa.c | 69 ++++++++++++++++++++++++++++++++++++++++----
>   1 file changed, 63 insertions(+), 6 deletions(-)
>

Robert,

It would be helpful if you can test this patch-series and confirm that
it now fixes the NULL pointer deference issue.

Thanks,
Vaibhav

[PATCH-v5 5/5] i2c: pxa: Add ILCR (tLow & tHigh) configuration support

[Robert Jarzmik]

Vaibhav Hiremath <vaibhav.hiremath@linaro.org> writes:

> Robert,
>
> It would be helpful if you can test this patch-series and confirm that
> it now fixes the NULL pointer deference issue.
>
> Thanks,
> Vaibhav
Hi Vaibhav,

My next slot is probably this comming Sunday. I'll do the test and report.

Cheers.

-- 
Robert

[PATCH-v5 5/5] i2c: pxa: Add ILCR (tLow & tHigh) configuration support

[Vaibhav Hiremath]

On Thursday 06 August 2015 12:41 AM, Robert Jarzmik wrote:
> My next slot is probably this comming Sunday. I'll do the test and report

Thanks a lot.

Thanks,
Vaibhav

[PATCH-v5 5/5] i2c: pxa: Add ILCR (tLow & tHigh) configuration support

[Robert Jarzmik]

Vaibhav Hiremath <vaibhav.hiremath@linaro.org> writes:

> Robert,
>
> It would be helpful if you can test this patch-series and confirm that
> it now fixes the NULL pointer deference issue.
Tested, it works on pxa27x in master mode, in non-DT mode.

For all non-DT patches, you can add my :
Tested-by: Robert Jarzmik <robert.jarzmik@free.fr>

Cheers.

-- 
Robert

[PATCH-v5 5/5] i2c: pxa: Add ILCR (tLow & tHigh) configuration support

[Vaibhav Hiremath]

On Sunday 09 August 2015 05:48 PM, Robert Jarzmik wrote:
> Vaibhav Hiremath <vaibhav.hiremath@linaro.org> writes:
>
>> Robert,
>>
>> It would be helpful if you can test this patch-series and confirm that
>> it now fixes the NULL pointer deference issue.
> Tested, it works on pxa27x in master mode, in non-DT mode.
>
> For all non-DT patches, you can add my :
> Tested-by: Robert Jarzmik <robert.jarzmik@free.fr>
>

Thanks Robert,

Wolfram,
Request you to please take a look at this series.
We can have discussion on DT properties as well.

Thanks,
Vaibhav