Re: [PATCH 11/11] i2c: pxa: no need slave addr for i2c master mode reset

2012-11-16 Thread Wolfram Sang
Hi,

On Thu, Nov 08, 2012 at 06:25:24AM -0800, Leo Song wrote:

> Would you please help to review and merge these 11 patches for
> drivers/i2c/busses/i2c-pxa.c?

There are already a few formal things:

- For large series, please write a cover-letter describing the series in
  total (use --cover-letter). The omap guys do this very well, check the
  list archive.

- why would upstream need Change-Id: in the patch description?

- the cover letter should also state what testing you did. the pxa
  driver is used in lots of different SoCs and I'd like to know how
  much this was tested on other systems regarding regressions.

- if you can't test it on other machines, ask for help. Putting alkml on
  CC is one way to do this

- I'd ask you to add alkml anyway since your patches include hooks to
  the mach and there are people having more experience on this platform
  than me

I didn't really have a look at the code yet, since I ask you to resend
it fixing the above points first. But from a glimpse, you should try to
avoid adding function pointers to platform_data at all costs. Maybe you
can think of alternative solutions.

Regards and thanks for the submission,

   Wolfram

-- 
Pengutronix e.K.   | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/  |


signature.asc
Description: Digital signature


RE: [PATCH 11/11] i2c: pxa: no need slave addr for i2c master mode reset

2012-11-08 Thread Leo Song
Hi Ben Dooks/Wolfram Sang,

Would you please help to review and merge these 11 patches for 
drivers/i2c/busses/i2c-pxa.c?

[PATCH 01/11] i2c: pxa: support hardware lock 
[PATCH 02/11] i2c: pxa: support pxa910 in id table 
[PATCH 03/11] i2c: pxa: add more error handling for i2c controller 
[PATCH 04/11] i2c: pxa: fix irq unbalanced warning 
[PATCH 05/11] i2c: pxa: add bus reset for platform data; 
[PATCH 06/11] i2c: pxa: Keep i2c clock enabled when system suspends/resumes. 
[PATCH 07/11] i2c: pxa: keep i2c irq on in suspend 
[PATCH 08/11] i2c: pxa: bugfix the slave addr in the transaction 
[PATCH 09/11] i2c: pxa: add qos as constraint for cpu-idle 
[PATCH 10/11] i2c: pxa: modify the parameters of i2c_bus_reset() 
[PATCH 11/11] i2c: pxa: no need slave addr for i2c master mode reset 

Thanks a lot.

Best Wishes, 
  
Leo Song
Tel: 6194-9745, Marvell (Shanghai)
B3-F4-4D12, No. 399 of Keyuan Rd, Pudong District, Shanghai 


>-Original Message-
>From: Leo Song [mailto:lia...@marvell.com]
>Sent: 2012年11月8日 22:18
>To: Ben Dooks; Wolfram Sang
>Cc: Leo Song; Chao Xie; linux-i2c@vger.kernel.org; Jett Zhou
>Subject: [PATCH 11/11] i2c: pxa: no need slave addr for i2c master mode
>reset
>
>From: "Jett.Zhou" 
>
>Normally i2c controller works as master, slave addr is not needed, or it
>will impact some slave device(eg. ST NFC chip) i2c access, because it
>has
>the same i2c address with controller.
>
>Change-Id: Iaf6e16cf2e211d242b21086e491b751ea311a000
>Signed-off-by: Jett.Zhou 
>
>diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
>index 3346fef..2dc3af3 100644
>--- a/drivers/i2c/busses/i2c-pxa.c
>+++ b/drivers/i2c/busses/i2c-pxa.c
>@@ -490,8 +490,10 @@ static void i2c_pxa_reset(struct pxa_i2c *i2c)
>   writel(I2C_ISR_INIT, _ISR(i2c));
>   writel(readl(_ICR(i2c)) & ~ICR_UR, _ICR(i2c));
>
>+#ifdef CONFIG_I2C_PXA_SLAVE
>   if (i2c->reg_isar)
>   writel(i2c->slave_addr, _ISAR(i2c));
>+#endif
>
>   /* set control register values */
>   writel(I2C_ICR_INIT | (i2c->fast_mode ? ICR_FM : 0), _ICR(i2c));
>--
>1.7.5.4

N�Р骒r��yb�X�肚�v�^�)藓{.n�+�伐�{��g"��^n�r■�z���h�ㄨ��&Ⅷ�G���h�(�茛j"���m赇z罐��帼f"�h���~�m�

[PATCH 11/11] i2c: pxa: no need slave addr for i2c master mode reset

2012-11-08 Thread Leo Song
From: "Jett.Zhou" 

Normally i2c controller works as master, slave addr is not needed, or it
will impact some slave device(eg. ST NFC chip) i2c access, because it has
the same i2c address with controller.

Change-Id: Iaf6e16cf2e211d242b21086e491b751ea311a000
Signed-off-by: Jett.Zhou 

diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
index 3346fef..2dc3af3 100644
--- a/drivers/i2c/busses/i2c-pxa.c
+++ b/drivers/i2c/busses/i2c-pxa.c
@@ -490,8 +490,10 @@ static void i2c_pxa_reset(struct pxa_i2c *i2c)
writel(I2C_ISR_INIT, _ISR(i2c));
writel(readl(_ICR(i2c)) & ~ICR_UR, _ICR(i2c));
 
+#ifdef CONFIG_I2C_PXA_SLAVE
if (i2c->reg_isar)
writel(i2c->slave_addr, _ISAR(i2c));
+#endif
 
/* set control register values */
writel(I2C_ICR_INIT | (i2c->fast_mode ? ICR_FM : 0), _ICR(i2c));
-- 
1.7.5.4

--
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