Re: [PATCH-v4 00/11] i2c: pxa: Fixes, cleanup and support for pxa910 family
On Monday 20 July 2015 12:36 PM, Vaibhav Hiremath wrote: On Saturday 18 July 2015 01:19 AM, Robert Jarzmik wrote: Vaibhav Hiremath vaibhav.hirem...@linaro.org writes: 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. I made a simple try on my reference platform with the whole patchset. It oopses on a NULL dereference. The stack is in [1]. I think it boils down to : - i2c_pxa_do_sclk_adj() - reg_ilcr = readl(_ILCR(i2c)); I also think the faulty patch is : - i2c: pxa: Add ILCR (tLow tHigh) configuration support My case, an I2C master case, I'd like you to find the issue and fix it. Which is this reference platform? Can you share few details - - reference Platform? - DT file if you could - Boot log (if you could) I am using pxa1928 based platform, and I do not see any issues. Having said that, I see issues in the patch for non PXA910 platform, where i2c_pxa_do_sclk_adj() will be called unconditionally and obviously reg_ilcr and reg_wcr are not set. I will fix this and send the patch. Thanks, Vaibhav -- 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-v4 00/11] i2c: pxa: Fixes, cleanup and support for pxa910 family
On Saturday 18 July 2015 01:19 AM, Robert Jarzmik wrote: Vaibhav Hiremath vaibhav.hirem...@linaro.org writes: 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. I made a simple try on my reference platform with the whole patchset. It oopses on a NULL dereference. The stack is in [1]. I think it boils down to : - i2c_pxa_do_sclk_adj() - reg_ilcr = readl(_ILCR(i2c)); I also think the faulty patch is : - i2c: pxa: Add ILCR (tLow tHigh) configuration support My case, an I2C master case, I'd like you to find the issue and fix it. Which is this reference platform? Can you share few details - - reference Platform? - DT file if you could - Boot log (if you could) I am using pxa1928 based platform, and I do not see any issues. Thanks, Vaibhav -- 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-v4 00/11] i2c: pxa: Fixes, cleanup and support for pxa910 family
On Monday 20 July 2015 12:39 PM, Vaibhav Hiremath wrote:
On Monday 20 July 2015 12:36 PM, Vaibhav Hiremath wrote:
On Saturday 18 July 2015 01:19 AM, Robert Jarzmik wrote:
Vaibhav Hiremath vaibhav.hirem...@linaro.org writes:
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.
I made a simple try on my reference platform with the whole patchset.
It oopses on a NULL dereference.
The stack is in [1].
I think it boils down to :
- i2c_pxa_do_sclk_adj()
- reg_ilcr = readl(_ILCR(i2c));
I also think the faulty patch is :
- i2c: pxa: Add ILCR (tLow tHigh) configuration support
My case, an I2C master case, I'd like you to find the issue and fix it.
Which is this reference platform?
Can you share few details -
- reference Platform?
- DT file if you could
- Boot log (if you could)
I am using pxa1928 based platform, and I do not see any issues.
Having said that,
I see issues in the patch for non PXA910 platform, where
i2c_pxa_do_sclk_adj() will be called unconditionally and obviously
reg_ilcr and reg_wcr are not set.
I will fix this and send the patch.
This should fix the issue -
hvaibhav@hvaibhav-ThinkPad-T440p:~/projects/mainline/linux$ git diff
drivers/i2c/busses/i2c-pxa.c
diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
index e0aa087..9e372fc 100644
--- a/drivers/i2c/busses/i2c-pxa.c
+++ b/drivers/i2c/busses/i2c-pxa.c
@@ -590,6 +590,9 @@ 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 you are ok, I will re-spin the patch and submit.
Thanks,
Vaibhav
--
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-v4 00/11] i2c: pxa: Fixes, cleanup and support for pxa910 family
Vaibhav Hiremath vaibhav.hirem...@linaro.org writes:
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.
I made a simple try on my reference platform with the whole patchset.
It oopses on a NULL dereference.
The stack is in [1].
I think it boils down to :
- i2c_pxa_do_sclk_adj()
- reg_ilcr = readl(_ILCR(i2c));
I also think the faulty patch is :
- i2c: pxa: Add ILCR (tLow tHigh) configuration support
My case, an I2C master case, I'd like you to find the issue and fix it.
Cheers.
--
Robert
[1] Stack
Unable to handle kernel NULL pointer dereference at virtual address
pgd = c0004000
[] *pgd=
Internal error: Oops: 5 [#1] PREEMPT ARM
Modules linked in:
CPU: 0 PID: 1 Comm: swapper Not tainted 4.2.0-rc2-next-20150713+ #705
Hardware name: MIO A701
task: c3856bc0 ti: c3858000 task.ti: c3858000
PC is at i2c_pxa_reset+0x114/0x1cc
LR is at i2c_pxa_probe+0x35c/0x428
pc : [c02ee4b0]lr : [c02ef080]psr: 6853
sp : c3859d50 ip : c3859d70 fp : c3859d6c
r10: c38f75ac r9 : c06c7668 r8 :
r7 : c394a300 r6 : c06c42f0 r5 : c38f7410 r4 : 0002
r3 : 87a0 r2 : f2301690 r1 : r0 :
Flags: nZCv IRQs on FIQs off Mode SVC_32 ISA ARM Segment kernel
Control: 397f Table: a0004000 DAC: 0017
Process swapper (pid: 1, stack limit = 0xc3858198)
Stack: (0xc3859d50 to 0xc385a000)
9d40: 4080 c38f7410 c06c42e0 c06c42f0
9d60: c3859dbc c3859d70 c02ef080 c02ee3a8 4080 c38d7b40 c38f7410 c3859d88
9d80: c014eb08 c04d1878 0001 c06c42f0 c06c42f0 c06f10c8
9da0: fdfb c0678550 c3859ddc c3859dc0 c0280b18 c02eed30
9dc0: c06c42f0 c0731330 c0705098 c06f10c8 c3859e0c c3859de0 c027e850 c0280ad0
9de0: c3859e0c c3859df0 c06c42f0 c06f10c8 c06c4324 c0704fc0 c0678550
9e00: c3859e2c c3859e10 c027eb64 c027e6c8 c06f10c8 c027eaec
9e20: c3859e54 c3859e30 c027c5c4 c027eaf8 c389b96c c38dc6d0 c3940c78 c06f10c8
9e40: c39408e0 c06ec778 c3859e64 c3859e58 c027e228 c027c56c c3859e94 c3859e68
9e60: c027dbc8 c027e20c c05803d0 c394a3e0 c3859e94 c06f10c8 c06c2380 c394a3e0
9e80: c069a888 c3859eac c3859e98 c027f6d4 c027dac0 c06c2380 c06c2380
9ea0: c3859ebc c3859eb0 c0280a38 c027f630 c3859ecc c3859ec0 c069a8a0 c02809ec
9ec0: c3859f4c c3859ed0 c000975c c069a894 c3859efc c3859ee0 c3859efc c3859ee8
9ee0: c00389e0 c0201e98 c3ffcbe1 c3859f4c c3859f00 c0038c00 c00389cc
9f00: c004710c c003eea8 0004 0004 c3ffcbfd c0615b48 c057cd28
9f20: 0004 c070f7a0 0004 c070f7a0 c06bd020 c070f7a0 c06a64b0
9f40: c3859f94 c3859f50 c0678e3c c000963c 0004 0004 c0678550
9f60: c0497ffc 0085 c0497ffc
9f80: c3859fac c3859f98 c0498014 c0678d44 c3858000
9fa0: c3859fb0 c000a5f0 c0498008
9fc0:
9fe0: 0013 b3ff8c5d 4cc3d76f
[c02ee4b0] (i2c_pxa_reset) from [c02ef080] (i2c_pxa_probe+0x35c/0x428)
[c02ef080] (i2c_pxa_probe) from [c0280b18] (platform_drv_probe+0x54/0xb0)
[c0280b18] (platform_drv_probe) from [c027e850]
(driver_probe_device+0x194/0x430)
[c027e850] (driver_probe_device) from [c027eb64] (__driver_attach+0x78/0x9c)
[c027eb64] (__driver_attach) from [c027c5c4] (bus_for_each_dev+0x64/0xb4)
[c027c5c4] (bus_for_each_dev) from [c027e228] (driver_attach+0x28/0x30)
[c027e228] (driver_attach) from [c027dbc8] (bus_add_driver+0x114/0x274)
[c027dbc8] (bus_add_driver) from [c027f6d4] (driver_register+0xb0/0xf8)
[c027f6d4] (driver_register) from [c0280a38]
(__platform_driver_register+0x58/0x6c)
[c0280a38] (__platform_driver_register) from [c069a8a0]
(i2c_adap_pxa_init+0x18/0x20)
[c069a8a0] (i2c_adap_pxa_init) from [c000975c] (do_one_initcall+0x12c/0x220)
[c000975c] (do_one_initcall) from [c0678e3c]
(kernel_init_freeable+0x104/0x1d8)
[c0678e3c] (kernel_init_freeable) from [c0498014] (kernel_init+0x18/0xfc)
[c0498014] (kernel_init) from [c000a5f0] (ret_from_fork+0x14/0x24)
Code: 03a0 e1803003 e5823000 e5950318 (e5903000)
---[ end trace 76138ae455db32c0 ]---
[2] Disassembly of the i2c_pxa_reset() and its inlined i2c_pxa_do_sclk_adj()
512
513 static void i2c_pxa_do_sclk_adj(struct pxa_i2c *i2c)
514 {
515 unsigned int reg_ilcr;
516
517 reg_ilcr = readl(_ILCR(i2c));
0xc02ee4ac +272: ldr r0, [r5, #792] ; 0x318
518
519 /* For standard/fast mode tlow and thigh counters are same */
520 if (i2c-sclk_tlow_load_cnt) {
0xc02ee4b4 +280: ldr r12, [r5, #828] ; 0x33c
[PATCH-v4 00/11] i2c: pxa: Fixes, cleanup and support for pxa910 family
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 -- 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-v4 00/11] i2c: pxa: Fixes, cleanup and support for pxa910 family
On Tuesday 14 July 2015 05:04 PM, Wolfram Sang wrote: 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. OK, Thanks and no issues. Lets discuss more on the bindings. Thanks, Vaibhav -- 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-v4 00/11] i2c: pxa: Fixes, cleanup and support for pxa910 family
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. signature.asc Description: Digital signature
