This patch series fixes bugs/warnings, cleans up the code and adds
support for PXA910 family of devices to PXA I2C bus driver.
There has been one attempt made sometime back in 2012 to upstream
some of the patches from below list, but did not get follow up later.
I have consolidated all the patches, cleaned them up, splited into
logical changes, added new patches and submitting it now.
I tried to maintain authorship & Signoff except where I did some
significant changes to the code/logic.
Link to previous post:
http://permalink.gmane.org/gmane.linux.drivers.i2c/13557
Testing:
- Basic testing on PMIC device on I2C-0 interface
- Boot tested on platform based on PXA1928
- Probe is successfully passing
- Read few registers of PMIC (RTC, ID, etc...) during boot
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
Leilei Shang (1):
i2c: pxa: keep i2c irq ON in suspend
Shouming Wang (1):
i2c: pxa: Return I2C_RETRY when timeout in pio mode
Vaibhav Hiremath (7):
i2c: pxa: No need to set slave addr for i2c master mode reset
i2c: pxa: Update debug function to dump more info on error
i2c:pxa: Use devm_ variants in probe function
Documentation: binding: add new property 'disable_after_xfer' to
i2c-pxa
i2c: pxa: Add support for pxa910/988 & new configuration features
i2c: pxa: Add ILCR (tLow & tHigh) configuration support
Documentation: binding: add sclk adjustment properties to i2c-pxa
Yi Zhang (1):
i2c: pxa: enable/disable i2c module across msg xfer
Yipeng Yao (1):
i2c: pxa: Fix compile warning in 64bit mode
Documentation/devicetree/bindings/i2c/i2c-pxa.txt | 18 ++
drivers/i2c/busses/i2c-pxa.c | 261 ++++++++++++++++------
2 files changed, 211 insertions(+), 68 deletions(-)
--
1.9.1
From: Leilei Shang <redacted>
During suspend there may still be some i2c access happening, as the
interrupt is shared between multiple drivers.
And if we don't keep i2c irq ON, there may be i2c access timeout if
i2c is in irq mode of operation.
Signed-off-by: Raul Xiong <redacted>
Signed-off-by: Xiaofan Tian <redacted>
[vaibhav.hiremath@linaro.org: updated Changelog]
Signed-off-by: Vaibhav Hiremath <redacted>
Cc: Wolfram Sang <redacted>
---
drivers/i2c/busses/i2c-pxa.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
Normally i2c controller works as master, so slave addr is not needed, or
it will impact some slave device (eg. ST NFC chip) i2c accesses, because
it has the same i2c address with controller.
For example,
On the pxa1928 based platform, where PMIC (88pm860) is present @0x30
address on TWSI0 interface, and if we set 0x30 as a slave address in
pxa1928 TWSI0 module, all the transactions towards PMIC would go for toss.
Signed-off-by: Jett.Zhou <redacted>
Signed-off-by: Vaibhav Hiremath <redacted>
Acked-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
drivers/i2c/busses/i2c-pxa.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Update i2c_pxa_scream_blue_murder() fn to print more information
in case of error.
Also, use dev_err variants instead of printk.
Signed-off-by: Jett.Zhou <redacted>
Signed-off-by: Vaibhav Hiremath <redacted>
Cc: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
---
drivers/i2c/busses/i2c-pxa.c | 22 +++++++++++++++-------
1 file changed, 15 insertions(+), 7 deletions(-)
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
This patch cleans up i2c_pxa_probe() function,
- Use devm_ variants wherever
This will clean both probe exit and i2c_pxa_remove() functions
- Check platform resource before parsing any other data from DT/platform
- Use dev_err on failure from i2c_add_numbered_adapter()
- Use pr_info instead of printk for KERN_INFO
Signed-off-by: Vaibhav Hiremath <redacted>
Acked-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
drivers/i2c/busses/i2c-pxa.c | 81 ++++++++++++++++----------------------------
1 file changed, 30 insertions(+), 51 deletions(-)
@@ -1158,10 +1158,23 @@ static int i2c_pxa_probe(struct platform_device *dev)structresource*res=NULL;intret,irq;-i2c=kzalloc(sizeof(structpxa_i2c),GFP_KERNEL);-if(!i2c){-ret=-ENOMEM;-gotoemalloc;+i2c=devm_kzalloc(&dev->dev,sizeof(structpxa_i2c),GFP_KERNEL);+if(!i2c)+return-ENOMEM;++res=platform_get_resource(dev,IORESOURCE_MEM,0);++i2c->reg_base=devm_ioremap_resource(&dev->dev,res);+if(IS_ERR(i2c->reg_base)){+dev_err(&dev->dev,"failed to map resource: %ld\n",+PTR_ERR(i2c->reg_base));+returnPTR_ERR(i2c->reg_base);+}++irq=platform_get_irq(dev,0);+if(irq<0){+dev_err(&dev->dev,"no irq resource: %d\n",irq);+returnirq;}/* Default adapter num to device id; i2c_pxa_probe_dt can override. */
@@ -1171,19 +1184,7 @@ static int i2c_pxa_probe(struct platform_device *dev)if(ret>0)ret=i2c_pxa_probe_pdata(dev,i2c,&i2c_type);if(ret<0)-gotoeclk;--res=platform_get_resource(dev,IORESOURCE_MEM,0);-irq=platform_get_irq(dev,0);-if(res==NULL||irq<0){-ret=-ENODEV;-gotoeclk;-}--if(!request_mem_region(res->start,resource_size(res),res->name)){-ret=-ENOMEM;-gotoeclk;-}+returnret;i2c->adap.owner=THIS_MODULE;i2c->adap.retries=5;
@@ -1193,16 +1194,10 @@ static int i2c_pxa_probe(struct platform_device *dev)strlcpy(i2c->adap.name,"pxa_i2c-i2c",sizeof(i2c->adap.name));-i2c->clk=clk_get(&dev->dev,NULL);+i2c->clk=devm_clk_get(&dev->dev,NULL);if(IS_ERR(i2c->clk)){-ret=PTR_ERR(i2c->clk);-gotoeclk;-}--i2c->reg_base=ioremap(res->start,resource_size(res));-if(!i2c->reg_base){-ret=-EIO;-gotoeremap;+dev_err(&dev->dev,"failed to get the clk: %ld\n",PTR_ERR(i2c->clk));+returnPTR_ERR(i2c->clk);}i2c->reg_ibmr=i2c->reg_base+pxa_reg_layout[i2c_type].ibmr;
@@ -1244,11 +1239,13 @@ static int i2c_pxa_probe(struct platform_device *dev)i2c->adap.algo=&i2c_pxa_pio_algorithm;}else{i2c->adap.algo=&i2c_pxa_algorithm;-ret=request_irq(irq,i2c_pxa_handler,+ret=devm_request_irq(&dev->dev,irq,i2c_pxa_handler,IRQF_SHARED|IRQF_NO_SUSPEND,dev_name(&dev->dev),i2c);-if(ret)+if(ret){+dev_err(&dev->dev,"failed to request irq: %d\n",ret);gotoereqirq;+}}i2c_pxa_reset(i2c);
From: Yi Zhang <redacted>
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 <redacted>
Signed-off-by: Vaibhav Hiremath <redacted>
---
Note that, in order _NOT_ to break existing slave support, we can not
enable this property by default. The only option is to use DT property
to control this feature.
drivers/i2c/busses/i2c-pxa.c | 43 +++++++++++++++++++++++++++++++++++++++++--
1 file changed, 41 insertions(+), 2 deletions(-)
@@ -832,6 +850,9 @@ static int i2c_pxa_pio_xfer(struct i2c_adapter *adap,structpxa_i2c*i2c=adap->algo_data;intret,i;+/* Enable i2c unit */+i2c_pxa_enable(i2c,true);+/* If the I2C controller is disabled we need to reset it(probablyduetoasuspend/resumedestroyingstate).Wedothishereaswecanthenavoidworryingaboutresumingthe
@@ -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);+returnret;}
@@ -1067,6 +1093,9 @@ static int i2c_pxa_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int numstructpxa_i2c*i2c=adap->algo_data;intret,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 numret=-EREMOTEIO;out:i2c_pxa_set_slave(i2c,ret);+/* disable i2c unit */+if(i2c->disable_after_xfer)+i2c_pxa_enable(i2c,false);+returnret;}
@@ -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))
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 <redacted>
Signed-off-by: Yi Zhang <redacted>
Signed-off-by: Vaibhav Hiremath <redacted>
---
drivers/i2c/busses/i2c-pxa.c | 42 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 41 insertions(+), 1 deletion(-)
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 <redacted>
Signed-off-by: Jett.Zhou <redacted>
Signed-off-by: Yi Zhang <redacted>
---
drivers/i2c/busses/i2c-pxa.c | 66 ++++++++++++++++++++++++++++++++++++++++----
1 file changed, 60 insertions(+), 6 deletions(-)
@@ -507,6 +510,33 @@ static void i2c_pxa_set_slave(struct pxa_i2c *i2c, int errcode)#define i2c_pxa_set_slave(i2c, err) do { } while (0)#endif+staticvoidi2c_pxa_do_sclk_adj(structpxa_i2c*i2c)+{+unsignedintreg_ilcr;++reg_ilcr=readl(_ILCR(i2c));++/* For standard/fast mode tlow and thigh counters are same */+if(i2c->sclk_tlow_load_cnt){+unsignedintmask,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));+}+staticvoidi2c_pxa_reset(structpxa_i2c*i2c){pr_debug("Resetting I2C Controller Unit\n");
@@ -1198,6 +1230,26 @@ static int i2c_pxa_probe_dt(struct platform_device *pdev, struct pxa_i2c *i2c,*i2c_types=(enumpxa_i2c_types)(of_id->data);+/* optional properties */+if(of_device_is_compatible(np,"mrvl,mmp-twsi")){+unsignedinttlow=0,thigh=0;+unsignedintclk_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");+}return0;}
@@ -1248,6 +1300,14 @@ static int i2c_pxa_probe(struct platform_device *dev)returnirq;}+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));+returnPTR_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 +1325,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));-returnPTR_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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <redacted>
---
Documentation/devicetree/bindings/i2c/i2c-pxa.txt | 13 +++++++++++++
1 file changed, 13 insertions(+)
@@ -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@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@d4025000 {
Driver 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 <redacted>
---
Documentation/devicetree/bindings/i2c/i2c-pxa.txt | 5 +++++
1 file changed, 5 insertions(+)
@@ -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@d4011000 { compatible = "mrvl,mmp-twsi";
From: Shouming Wang <redacted>
In case of timeout in pio mode of operation return I2C_RETRY.
This behavior will be same as interrupt mode of operation.
Signed-off-by: Shouming Wang <redacted>
[vaibhav.hiremath@linaro.org: Updated changelog]
Signed-off-by: Vaibhav Hiremath <redacted>
Acked-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
drivers/i2c/busses/i2c-pxa.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
From: Wolfram Sang <hidden> Date: 2015-07-14 11:34:42
On Tue, Jul 14, 2015 at 01:06:39PM +0530, Vaibhav Hiremath wrote:
This patch series fixes bugs/warnings, cleans up the code and adds
support for PXA910 family of devices to PXA I2C bus driver.
There has been one attempt made sometime back in 2012 to upstream
some of the patches from below list, but did not get follow up later.
I have consolidated all the patches, cleaned them up, splited into
logical changes, added new patches and submitting it now.
I tried to maintain authorship & Signoff except where I did some
significant changes to the code/logic.
So, I applied patches 1-6 to for-next to make some progress.
The others need more thought because of the bindings which shall be
discussed replying to the patches in question.
Thanks for the updated work with lots of proper references.
On Tuesday 14 July 2015 05:04 PM, Wolfram Sang wrote:
On Tue, Jul 14, 2015 at 01:06:39PM +0530, Vaibhav Hiremath wrote:
quoted
This patch series fixes bugs/warnings, cleans up the code and adds
support for PXA910 family of devices to PXA I2C bus driver.
There has been one attempt made sometime back in 2012 to upstream
some of the patches from below list, but did not get follow up later.
I have consolidated all the patches, cleaned them up, splited into
logical changes, added new patches and submitting it now.
I tried to maintain authorship & Signoff except where I did some
significant changes to the code/logic.
So, I applied patches 1-6 to for-next to make some progress.
The others need more thought because of the bindings which shall be
discussed replying to the patches in question.
Thanks for the updated work with lots of proper references.
OK, Thanks and no issues.
Lets discuss more on the bindings.
Thanks,
Vaibhav
On Tuesday 14 July 2015 05:05 PM, Wolfram Sang wrote:
quoted
+ i2c->reg_base = devm_ioremap_resource(&dev->dev, res);+ if (IS_ERR(i2c->reg_base)) {+ dev_err(&dev->dev, "failed to map resource: %ld\n",+ PTR_ERR(i2c->reg_base));+ return PTR_ERR(i2c->reg_base);+ }
One change I did when applying: removed this error message.
devm_ioremap_resource prints out the errors it finds.
devm_ioremap_resource doesn't print return value.
So this additional error message would print one of, -EINVAL, -EBUSY
or -ENOMEM.
That was the reason I kept it.
If you feel it is not required, I am OK to remove it.
Thanks for the update, it certainly saved one more version :) .
Thanks,
Vaibhav