[PATCH 1/2] [ARM] i2c/pxa: remove unused macro

[PATCH 1/2] [ARM] i2c/pxa: remove unused macro

[Uwe Kleine-König]

Commit

	beea494 ([ARM] Remove EEPROM slave emulation from i2c-pxa driver.)

removed all uses of eedbg, so the definition can go, too.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Cc: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
Cc: Ben Dooks <ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org>
Cc: Russell King <rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
Cc: Eric Miao <eric.miao-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
Cc: Roel Kluin <roel.kluin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
---
 drivers/i2c/busses/i2c-pxa.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
index 0495557..40052c0 100644
--- a/drivers/i2c/busses/i2c-pxa.c
+++ b/drivers/i2c/busses/i2c-pxa.c
@@ -216,8 +216,6 @@ static void i2c_pxa_show_state(struct pxa_i2c *i2c, int lno, const char *fname)
 #define decode_ICR(val) do { } while (0)
 #endif
 
-#define eedbg(lvl, x...) do { if ((lvl) < 1) { printk(KERN_DEBUG "" x); } } while(0)
-
 static void i2c_pxa_master_complete(struct pxa_i2c *i2c, int ret);
 static irqreturn_t i2c_pxa_handler(int this_irq, void *dev_id);
 
-- 
1.6.5.2

[PATCH 2/2] [ARM] i2c/pxa: only define 'blue_murder'-function if DEBUG is #defined

[Uwe Kleine-König]

From: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

This talkative function is also called on timeouts. As timeouts can
happen on regular writes to EEPROMs (no error case), this creates false
positives.  Giving lots of details is interesting only for developers
anyhow, so just use the function if DEBUG is #defined.

Signed-off-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Cc: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
Cc: Ben Dooks <ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org>
Cc: Russell King <rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
Cc: Eric Miao <eric.miao-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
Cc: Roel Kluin <roel.kluin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
---
 drivers/i2c/busses/i2c-pxa.c |   24 ++++++++++++++----------
 1 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
index 40052c0..009e52e 100644
--- a/drivers/i2c/busses/i2c-pxa.c
+++ b/drivers/i2c/busses/i2c-pxa.c
@@ -208,16 +208,6 @@ static void i2c_pxa_show_state(struct pxa_i2c *i2c, int lno, const char *fname)
 }
 
 #define show_state(i2c) i2c_pxa_show_state(i2c, __LINE__, __func__)
-#else
-#define i2c_debug	0
-
-#define show_state(i2c) do { } while (0)
-#define decode_ISR(val) do { } while (0)
-#define decode_ICR(val) do { } while (0)
-#endif
-
-static void i2c_pxa_master_complete(struct pxa_i2c *i2c, int ret);
-static irqreturn_t i2c_pxa_handler(int this_irq, void *dev_id);
 
 static void i2c_pxa_scream_blue_murder(struct pxa_i2c *i2c, const char *why)
 {
@@ -233,6 +223,20 @@ static void i2c_pxa_scream_blue_murder(struct pxa_i2c *i2c, const char *why)
 	printk("\n");
 }
 
+#else /* ifdef DEBUG */
+
+#define i2c_debug	0
+
+#define show_state(i2c) do { } while (0)
+#define decode_ISR(val) do { } while (0)
+#define decode_ICR(val) do { } while (0)
+#define i2c_pxa_scream_blue_murder(i2c, why) do { } while (0)
+
+#endif /* ifdef DEBUG / else */
+
+static void i2c_pxa_master_complete(struct pxa_i2c *i2c, int ret);
+static irqreturn_t i2c_pxa_handler(int this_irq, void *dev_id);
+
 static inline int i2c_pxa_is_slavemode(struct pxa_i2c *i2c)
 {
 	return !(readl(_ICR(i2c)) & ICR_SCLE);
-- 
1.6.5.2

Re: [PATCH 1/2] [ARM] i2c/pxa: remove unused macro

[Pavel Machek]

On Tue 2009-11-03 20:03:19, Uwe Kleine-König wrote:
> Commit
> 
> 	beea494 ([ARM] Remove EEPROM slave emulation from i2c-pxa driver.)
> 
> removed all uses of eedbg, so the definition can go, too.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> Cc: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
> Cc: Ben Dooks <ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org>
> Cc: Russell King <rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
> Cc: Eric Miao <eric.miao-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
> Cc: Roel Kluin <roel.kluin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Acked-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 1/2] [ARM] i2c/pxa: remove unused macro

[Uwe Kleine-König]

Hello,

> Cc: Eric Miao <eric.miao-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
postmaster-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org claims the address ymiao3-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org isn't
valid.  So I think Eric got a new one.

If you answer to that thread please take care to use
eric.y.miao-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org which hopefully works.

Best regards
Uwe

-- 
Pengutronix e.K.                              | Uwe Kleine-König            |
Industrial Linux Solutions                    | http://www.pengutronix.de/  |

Re: [PATCH 1/2] [ARM] i2c/pxa: remove unused macro

[Russell King]

On Tue, Nov 03, 2009 at 08:03:19PM +0100, Uwe Kleine-König wrote:
> Commit
> 
> 	beea494 ([ARM] Remove EEPROM slave emulation from i2c-pxa driver.)

Oh great.  Some people may remember the LX code that I submitted to
the mailing list a while back.  Removing the slave emulation buggers
that platform up.  I do intend to submit that once the issue of where
the PCON support should be located is resolved - which is still
pending.

Please revert this change, thanks.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: [PATCH 1/2] [ARM] i2c/pxa: remove unused macro

[Ben Dooks]

On Tue, Nov 03, 2009 at 09:01:15PM +0000, Russell King wrote:
> On Tue, Nov 03, 2009 at 08:03:19PM +0100, Uwe Kleine-K?nig wrote:
> > Commit
> > 
> > 	beea494 ([ARM] Remove EEPROM slave emulation from i2c-pxa driver.)
> 
> Oh great.  Some people may remember the LX code that I submitted to
> the mailing list a while back.  Removing the slave emulation buggers
> that platform up.  I do intend to submit that once the issue of where
> the PCON support should be located is resolved - which is still
> pending.
> 
> Please revert this change, thanks.

Would you like it reverted in the next merge window, or when the
LX code is actually added?

I'll NAK any changes to remove the eedbg() for the moment.

-- 
Ben (ben-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org, http://www.fluff.org/)

  'a smiley only costs 4 bytes'

Re: [PATCH 1/2] [ARM] i2c/pxa: remove unused macro

[Ben Dooks]

On Tue, Nov 03, 2009 at 08:03:19PM +0100, Uwe Kleine-K??nig wrote:
> Commit
> 
> 	beea494 ([ARM] Remove EEPROM slave emulation from i2c-pxa driver.)
> 
> removed all uses of eedbg, so the definition can go, too.

I belive Russell is trying to sort out the code that was using this
so I would like to put this on hold until we sort out what is going
on.

-- 
Ben (ben-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org, http://www.fluff.org/)

  'a smiley only costs 4 bytes'

Re: [PATCH 1/2] [ARM] i2c/pxa: remove unused macro

[Uwe Kleine-König]

Hello,

On Tue, Nov 03, 2009 at 09:01:15PM +0000, Russell King wrote:
> On Tue, Nov 03, 2009 at 08:03:19PM +0100, Uwe Kleine-König wrote:
> > Commit
> > 
> > 	beea494 ([ARM] Remove EEPROM slave emulation from i2c-pxa driver.)
> 
> Oh great.  Some people may remember the LX code that I submitted to
> the mailing list a while back.  Removing the slave emulation buggers
> that platform up.  I do intend to submit that once the issue of where
> the PCON support should be located is resolved - which is still
> pending.
> 
> Please revert this change, thanks.
"this" means beea494 or my patch?  I'm unsure because you (Russell) are
the author of beea494.

The main reason for this series was the 2nd patch.  While creating it I
noticed that eedbg was unused.

For this second patch (only defining i2c_pxa_scream_blue_murder if DEBUG
is #defined) should I rebase it to beea494^ or should I revert beea494
and put it on top of the reverted beea494?

Thanks
Uwe

-- 
Pengutronix e.K.                              | Uwe Kleine-König            |
Industrial Linux Solutions                    | http://www.pengutronix.de/  |

Re: [PATCH 1/2] [ARM] i2c/pxa: remove unused macro

[Russell King - ARM Linux]

On Tue, Nov 03, 2009 at 10:12:49PM +0000, Ben Dooks wrote:
> On Tue, Nov 03, 2009 at 09:01:15PM +0000, Russell King wrote:
> > On Tue, Nov 03, 2009 at 08:03:19PM +0100, Uwe Kleine-K?nig wrote:
> > > Commit
> > > 
> > > 	beea494 ([ARM] Remove EEPROM slave emulation from i2c-pxa driver.)
> > 
> > Oh great.  Some people may remember the LX code that I submitted to
> > the mailing list a while back.  Removing the slave emulation buggers
> > that platform up.  I do intend to submit that once the issue of where
> > the PCON support should be located is resolved - which is still
> > pending.
> > 
> > Please revert this change, thanks.
> 
> Would you like it reverted in the next merge window, or when the
> LX code is actually added?

Actually, the change was correct, and was part of the LX patchset - I just
misremembered where stuff was with it.

> I'll NAK any changes to remove the eedbg() for the moment.

That change is also fine, consider it acked:

Acked-by: Russell King <rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>

Sorry for the noise.

[PATCH 1/2 RESENT] [ARM] i2c/pxa: remove unused macro

[Uwe Kleine-König]

Commit

	beea494 ([ARM] Remove EEPROM slave emulation from i2c-pxa driver.)

removed all uses of eedbg, so the definition can go, too.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Cc: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
Cc: Ben Dooks <ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org>
Acked-by: Russell King <rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
Cc: Eric Miao <eric.y.miao-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Roel Kluin <roel.kluin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Acked-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
---
 drivers/i2c/busses/i2c-pxa.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
index 0495557..40052c0 100644
--- a/drivers/i2c/busses/i2c-pxa.c
+++ b/drivers/i2c/busses/i2c-pxa.c
@@ -216,8 +216,6 @@ static void i2c_pxa_show_state(struct pxa_i2c *i2c, int lno, const char *fname)
 #define decode_ICR(val) do { } while (0)
 #endif
 
-#define eedbg(lvl, x...) do { if ((lvl) < 1) { printk(KERN_DEBUG "" x); } } while(0)
-
 static void i2c_pxa_master_complete(struct pxa_i2c *i2c, int ret);
 static irqreturn_t i2c_pxa_handler(int this_irq, void *dev_id);
 
-- 
1.6.5.2

[PATCH 2/2 RESENT] [ARM] i2c/pxa: only define 'blue_murder'-function if DEBUG is #defined

[Uwe Kleine-König]

From: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

This talkative function is also called on timeouts. As timeouts can
happen on regular writes to EEPROMs (no error case), this creates false
positives.  Giving lots of details is interesting only for developers
anyhow, so just use the function if DEBUG is #defined.

Signed-off-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Cc: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
Cc: Ben Dooks <ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org>
Cc: Russell King <rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
Cc: Eric Miao <eric.y.miao-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Roel Kluin <roel.kluin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
---
 drivers/i2c/busses/i2c-pxa.c |   24 ++++++++++++++----------
 1 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
index 40052c0..009e52e 100644
--- a/drivers/i2c/busses/i2c-pxa.c
+++ b/drivers/i2c/busses/i2c-pxa.c
@@ -208,16 +208,6 @@ static void i2c_pxa_show_state(struct pxa_i2c *i2c, int lno, const char *fname)
 }
 
 #define show_state(i2c) i2c_pxa_show_state(i2c, __LINE__, __func__)
-#else
-#define i2c_debug	0
-
-#define show_state(i2c) do { } while (0)
-#define decode_ISR(val) do { } while (0)
-#define decode_ICR(val) do { } while (0)
-#endif
-
-static void i2c_pxa_master_complete(struct pxa_i2c *i2c, int ret);
-static irqreturn_t i2c_pxa_handler(int this_irq, void *dev_id);
 
 static void i2c_pxa_scream_blue_murder(struct pxa_i2c *i2c, const char *why)
 {
@@ -233,6 +223,20 @@ static void i2c_pxa_scream_blue_murder(struct pxa_i2c *i2c, const char *why)
 	printk("\n");
 }
 
+#else /* ifdef DEBUG */
+
+#define i2c_debug	0
+
+#define show_state(i2c) do { } while (0)
+#define decode_ISR(val) do { } while (0)
+#define decode_ICR(val) do { } while (0)
+#define i2c_pxa_scream_blue_murder(i2c, why) do { } while (0)
+
+#endif /* ifdef DEBUG / else */
+
+static void i2c_pxa_master_complete(struct pxa_i2c *i2c, int ret);
+static irqreturn_t i2c_pxa_handler(int this_irq, void *dev_id);
+
 static inline int i2c_pxa_is_slavemode(struct pxa_i2c *i2c)
 {
 	return !(readl(_ICR(i2c)) & ICR_SCLE);
-- 
1.6.5.2

Re: [PATCH 2/2 RESENT] [ARM] i2c/pxa: only define 'blue_murder'-function if DEBUG is #defined

[Eric Miao]

2009/11/9 Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>:
> From: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>
> This talkative function is also called on timeouts. As timeouts can
> happen on regular writes to EEPROMs (no error case), this creates false
> positives.  Giving lots of details is interesting only for developers
> anyhow, so just use the function if DEBUG is #defined.
>

Well, this does look cleaner. Feel free to take my Ack.

[PATCH 1/2 RESEND#2] i2c/pxa: remove unused macro

[Uwe Kleine-König]

Commit

	beea494 ([ARM] Remove EEPROM slave emulation from i2c-pxa driver.)

removed all uses of eedbg, so the definition can go, too.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Cc: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
Cc: Ben Dooks <ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org>
Acked-by: Russell King <rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
Cc: Eric Miao <eric.y.miao-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Roel Kluin <roel.kluin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Acked-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
---
 drivers/i2c/busses/i2c-pxa.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
index 0495557..40052c0 100644
--- a/drivers/i2c/busses/i2c-pxa.c
+++ b/drivers/i2c/busses/i2c-pxa.c
@@ -216,8 +216,6 @@ static void i2c_pxa_show_state(struct pxa_i2c *i2c, int lno, const char *fname)
 #define decode_ICR(val) do { } while (0)
 #endif
 
-#define eedbg(lvl, x...) do { if ((lvl) < 1) { printk(KERN_DEBUG "" x); } } while(0)
-
 static void i2c_pxa_master_complete(struct pxa_i2c *i2c, int ret);
 static irqreturn_t i2c_pxa_handler(int this_irq, void *dev_id);
 
-- 
1.6.5.2

[PATCH 2/2 RESEND#2] i2c/pxa: only define 'blue_murder'-function if DEBUG is #defined

[Uwe Kleine-König]

From: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

This talkative function is also called on timeouts. As timeouts can
happen on regular writes to EEPROMs (no error case), this creates false
positives.  Giving lots of details is interesting only for developers
anyhow, so just use the function if DEBUG is #defined.

Signed-off-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Cc: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
Cc: Ben Dooks <ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org>
Cc: Russell King <rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
Acked-by: Eric Miao <eric.y.miao-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Roel Kluin <roel.kluin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
---
 drivers/i2c/busses/i2c-pxa.c |   24 ++++++++++++++----------
 1 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
index 40052c0..009e52e 100644
--- a/drivers/i2c/busses/i2c-pxa.c
+++ b/drivers/i2c/busses/i2c-pxa.c
@@ -208,16 +208,6 @@ static void i2c_pxa_show_state(struct pxa_i2c *i2c, int lno, const char *fname)
 }
 
 #define show_state(i2c) i2c_pxa_show_state(i2c, __LINE__, __func__)
-#else
-#define i2c_debug	0
-
-#define show_state(i2c) do { } while (0)
-#define decode_ISR(val) do { } while (0)
-#define decode_ICR(val) do { } while (0)
-#endif
-
-static void i2c_pxa_master_complete(struct pxa_i2c *i2c, int ret);
-static irqreturn_t i2c_pxa_handler(int this_irq, void *dev_id);
 
 static void i2c_pxa_scream_blue_murder(struct pxa_i2c *i2c, const char *why)
 {
@@ -233,6 +223,20 @@ static void i2c_pxa_scream_blue_murder(struct pxa_i2c *i2c, const char *why)
 	printk("\n");
 }
 
+#else /* ifdef DEBUG */
+
+#define i2c_debug	0
+
+#define show_state(i2c) do { } while (0)
+#define decode_ISR(val) do { } while (0)
+#define decode_ICR(val) do { } while (0)
+#define i2c_pxa_scream_blue_murder(i2c, why) do { } while (0)
+
+#endif /* ifdef DEBUG / else */
+
+static void i2c_pxa_master_complete(struct pxa_i2c *i2c, int ret);
+static irqreturn_t i2c_pxa_handler(int this_irq, void *dev_id);
+
 static inline int i2c_pxa_is_slavemode(struct pxa_i2c *i2c)
 {
 	return !(readl(_ICR(i2c)) & ICR_SCLE);
-- 
1.6.5.2

Re: [PATCH 1/2 RESEND#2] i2c/pxa: remove unused macro

[Uwe Kleine-König]

Hello,

On Thu, Nov 19, 2009 at 08:15:40PM +0100, Uwe Kleine-König wrote:
> Commit
> 
> 	beea494 ([ARM] Remove EEPROM slave emulation from i2c-pxa driver.)
> 
> removed all uses of eedbg, so the definition can go, too.
does anyone care to take this and the followup patch?

Best regards
Uwe

-- 
Pengutronix e.K.                              | Uwe Kleine-König            |
Industrial Linux Solutions                    | http://www.pengutronix.de/  |

Re: [PATCH 1/2 RESEND#2] i2c/pxa: remove unused macro

[Uwe Kleine-König]

Hi Jean,

On Thu, Nov 19, 2009 at 08:15:40PM +0100, Uwe Kleine-König wrote:
> Commit
> 
> 	beea494 ([ARM] Remove EEPROM slave emulation from i2c-pxa driver.)
> 
> removed all uses of eedbg, so the definition can go, too.
do you have comments to my patches?  I think they should go in via your
tree.

Best regards
Uwe

-- 
Pengutronix e.K.                              | Uwe Kleine-König            |
Industrial Linux Solutions                    | http://www.pengutronix.de/  |

Re: [PATCH 1/2 RESEND#2] i2c/pxa: remove unused macro

[Jean Delvare]

On Tue, 8 Dec 2009 22:18:10 +0100, Uwe Kleine-König wrote:
> Hi Jean,
> 
> On Thu, Nov 19, 2009 at 08:15:40PM +0100, Uwe Kleine-König wrote:
> > Commit
> > 
> > 	beea494 ([ARM] Remove EEPROM slave emulation from i2c-pxa driver.)
> > 
> > removed all uses of eedbg, so the definition can go, too.
> do you have comments to my patches?  I think they should go in via your
> tree.

I think you should read MAINTAINERS again.

-- 
Jean Delvare

Re: [PATCH 1/2 RESEND#2] i2c/pxa: remove unused macro

[Uwe Kleine-König]

Hi Jean,

On Tue, Dec 08, 2009 at 10:44:56PM +0100, Jean Delvare wrote:
> On Tue, 8 Dec 2009 22:18:10 +0100, Uwe Kleine-König wrote:
> > Hi Jean,
> > 
> > On Thu, Nov 19, 2009 at 08:15:40PM +0100, Uwe Kleine-König wrote:
> > > Commit
> > > 
> > > 	beea494 ([ARM] Remove EEPROM slave emulation from i2c-pxa driver.)
> > > 
> > > removed all uses of eedbg, so the definition can go, too.
> > do you have comments to my patches?  I think they should go in via your
> > tree.
> 
> I think you should read MAINTAINERS again.
Ah, so the maintainer is Ben Dooks, but the tree seems to be your's.

So I assume you take it when Ben give's his S-o-b or Ben has write
access to the tree?

Ben, what do you think about my patches?

Best regards
Uwe

-- 
Pengutronix e.K.                              | Uwe Kleine-König            |
Industrial Linux Solutions                    | http://www.pengutronix.de/  |

Re: [PATCH 1/2 RESEND#2] i2c/pxa: remove unused macro

[Jean Delvare]

On Tue, 8 Dec 2009 23:07:24 +0100, Uwe Kleine-König wrote:
> Hi Jean,
> 
> On Tue, Dec 08, 2009 at 10:44:56PM +0100, Jean Delvare wrote:
> > On Tue, 8 Dec 2009 22:18:10 +0100, Uwe Kleine-König wrote:
> > > Hi Jean,
> > > 
> > > On Thu, Nov 19, 2009 at 08:15:40PM +0100, Uwe Kleine-König wrote:
> > > > Commit
> > > > 
> > > > 	beea494 ([ARM] Remove EEPROM slave emulation from i2c-pxa driver.)
> > > > 
> > > > removed all uses of eedbg, so the definition can go, too.
> > > do you have comments to my patches?  I think they should go in via your
> > > tree.
> > 
> > I think you should read MAINTAINERS again.
> Ah, so the maintainer is Ben Dooks, but the tree seems to be your's.
>
> So I assume you take it when Ben give's his S-o-b or Ben has write
> access to the tree?

No. I have my tree, Ben has its own. Ben's isn't listed in MAINTAINERS,
thus the confusion.

Ben, can you please add your tree to MAINTAINERS to make things clearer
for contributors?

Thanks,
-- 
Jean Delvare

Re: [PATCH 1/2 RESEND#2] i2c/pxa: remove unused macro

[Uwe Kleine-König]

Hi Ben,

On Wed, Dec 09, 2009 at 09:11:46AM +0100, Jean Delvare wrote:
> On Tue, 8 Dec 2009 23:07:24 +0100, Uwe Kleine-König wrote:
> > On Tue, Dec 08, 2009 at 10:44:56PM +0100, Jean Delvare wrote:
> > > On Tue, 8 Dec 2009 22:18:10 +0100, Uwe Kleine-König wrote:
> > > > do you have comments to my patches?  I think they should go in via your
> > > > tree.
> > > 
> > > I think you should read MAINTAINERS again.
> > Ah, so the maintainer is Ben Dooks, but the tree seems to be your's.
> >
> > So I assume you take it when Ben give's his S-o-b or Ben has write
> > access to the tree?
> 
> No. I have my tree, Ben has its own. Ben's isn't listed in MAINTAINERS,
> thus the confusion.
> 
> Ben, can you please add your tree to MAINTAINERS to make things clearer
> for contributors?
any updates here?  comments for my patches?

Best regards
Uwe

-- 
Pengutronix e.K.                              | Uwe Kleine-König            |
Industrial Linux Solutions                    | http://www.pengutronix.de/  |

Re: [PATCH 1/2 RESEND#2] i2c/pxa: remove unused macro

[Uwe Kleine-König]

Hey Ben,

On Wed, Dec 09, 2009 at 09:11:46AM +0100, Jean Delvare wrote:
> On Tue, 8 Dec 2009 23:07:24 +0100, Uwe Kleine-König wrote:
> > Hi Jean,
> > 
> > On Tue, Dec 08, 2009 at 10:44:56PM +0100, Jean Delvare wrote:
> > > On Tue, 8 Dec 2009 22:18:10 +0100, Uwe Kleine-König wrote:
> > > > Hi Jean,
> > > > 
> > > > On Thu, Nov 19, 2009 at 08:15:40PM +0100, Uwe Kleine-König wrote:
> > > > > Commit
> > > > > 
> > > > > 	beea494 ([ARM] Remove EEPROM slave emulation from i2c-pxa driver.)
> > > > > 
> > > > > removed all uses of eedbg, so the definition can go, too.
> > > > do you have comments to my patches?  I think they should go in via your
> > > > tree.
> > > 
> > > I think you should read MAINTAINERS again.
> > Ah, so the maintainer is Ben Dooks, but the tree seems to be your's.
> >
> > So I assume you take it when Ben give's his S-o-b or Ben has write
> > access to the tree?
> 
> No. I have my tree, Ben has its own. Ben's isn't listed in MAINTAINERS,
> thus the confusion.
> 
> Ben, can you please add your tree to MAINTAINERS to make things clearer
> for contributors?
In the meantime I have three i2c patches without any feedback by you.
As Jean is convinced that you are responsible to take them can you
please have a look and comment?

My patches are also available via git at

	git://git.pengutronix.de/git/ukl/linux-2.6.git next/i2c

Shortlog and diffstat can be found below.

Best regards
Uwe

Uwe Kleine-König (2):
      i2c/pxa: remove unused macro
      i2c-imx: call ioremap only after request_mem_region

Wolfram Sang (1):
      i2c/pxa: only define 'blue_murder'-function if DEBUG is #defined

 drivers/i2c/busses/i2c-imx.c |   21 +++++++++++----------
 drivers/i2c/busses/i2c-pxa.c |   26 ++++++++++++++------------
 2 files changed, 25 insertions(+), 22 deletions(-)

-- 
Pengutronix e.K.                              | Uwe Kleine-König            |
Industrial Linux Solutions                    | http://www.pengutronix.de/  |

Re: [PATCH 1/2 RESEND#2] i2c/pxa: remove unused macro

[Uwe Kleine-König]

Hello Ben,

> > Ben, can you please add your tree to MAINTAINERS to make things clearer
> > for contributors?
I just sent out a patch for that to linux-i2c.

> In the meantime I have three i2c patches without any feedback by you.
> As Jean is convinced that you are responsible to take them can you
> please have a look and comment?
> 
> My patches are also available via git at
> 
> 	git://git.pengutronix.de/git/ukl/linux-2.6.git next/i2c
> 
> Shortlog and diffstat can be found below.
> 
> Best regards
> Uwe
> 
> Uwe Kleine-König (2):
>       i2c/pxa: remove unused macro
>       i2c-imx: call ioremap only after request_mem_region
> 
> Wolfram Sang (1):
>       i2c/pxa: only define 'blue_murder'-function if DEBUG is #defined
hmm, you took one of those (the i2c-imx patch) but not the other two
with still no comment?!

Best regards
Uwe

-- 
Pengutronix e.K.                              | Uwe Kleine-König            |
Industrial Linux Solutions                    | http://www.pengutronix.de/  |

Re: my i2c-patches for 2.6.34

[Uwe Kleine-König]

Hello Ben,

this mail is the fourth try to get some comments on the pxa patches.
Can you please consider to take them for 2.6.34?  Or do you think they
need an ack by someone?

I'll resend them as a reply to this mail, or you can get them via git,
see below.

Best regards
Uwe

The following changes since commit 75ef7cdda2daa35be9e070ac8e5258759ac03d06:
  Linus Torvalds (1):
        Merge git://git.kernel.org/.../davem/net-2.6

are available in the git repository at:

  git://git.pengutronix.de/git/ukl/linux-2.6.git next/i2c

Uwe Kleine-König (2):
      i2c/pxa: remove unused macro
      MAINTAINERS: add i2c tree for embedded platforms

Wolfram Sang (1):
      i2c/pxa: only define 'blue_murder'-function if DEBUG is #defined

 MAINTAINERS                  |    1 +
 drivers/i2c/busses/i2c-pxa.c |   26 ++++++++++++++------------
 2 files changed, 15 insertions(+), 12 deletions(-)

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH 1/3] i2c/pxa: remove unused macro

[Uwe Kleine-König]

Commit

	beea494 ([ARM] Remove EEPROM slave emulation from i2c-pxa driver.)

removed all uses of eedbg, so the definition can go, too.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Acked-by: Russell King <rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
Cc: Eric Miao <eric.y.miao-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Roel Kluin <roel.kluin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Acked-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
---
 drivers/i2c/busses/i2c-pxa.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
index 7647a20..b3c6c9d 100644
--- a/drivers/i2c/busses/i2c-pxa.c
+++ b/drivers/i2c/busses/i2c-pxa.c
@@ -216,8 +216,6 @@ static void i2c_pxa_show_state(struct pxa_i2c *i2c, int lno, const char *fname)
 #define decode_ICR(val) do { } while (0)
 #endif
 
-#define eedbg(lvl, x...) do { if ((lvl) < 1) { printk(KERN_DEBUG "" x); } } while(0)
-
 static void i2c_pxa_master_complete(struct pxa_i2c *i2c, int ret);
 static irqreturn_t i2c_pxa_handler(int this_irq, void *dev_id);
 
-- 
1.6.6.2

[PATCH 2/3] i2c/pxa: only define 'blue_murder'-function if DEBUG is #defined

[Uwe Kleine-König]

From: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

This talkative function is also called on timeouts. As timeouts can
happen on regular writes to EEPROMs (no error case), this creates false
positives.  Giving lots of details is interesting only for developers
anyhow, so just use the function if DEBUG is #defined.

Signed-off-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Cc: Russell King <rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
Acked-by: Eric Miao <eric.y.miao-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Roel Kluin <roel.kluin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
---
 drivers/i2c/busses/i2c-pxa.c |   24 ++++++++++++++----------
 1 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
index b3c6c9d..d8a9159 100644
--- a/drivers/i2c/busses/i2c-pxa.c
+++ b/drivers/i2c/busses/i2c-pxa.c
@@ -208,16 +208,6 @@ static void i2c_pxa_show_state(struct pxa_i2c *i2c, int lno, const char *fname)
 }
 
 #define show_state(i2c) i2c_pxa_show_state(i2c, __LINE__, __func__)
-#else
-#define i2c_debug	0
-
-#define show_state(i2c) do { } while (0)
-#define decode_ISR(val) do { } while (0)
-#define decode_ICR(val) do { } while (0)
-#endif
-
-static void i2c_pxa_master_complete(struct pxa_i2c *i2c, int ret);
-static irqreturn_t i2c_pxa_handler(int this_irq, void *dev_id);
 
 static void i2c_pxa_scream_blue_murder(struct pxa_i2c *i2c, const char *why)
 {
@@ -233,6 +223,20 @@ static void i2c_pxa_scream_blue_murder(struct pxa_i2c *i2c, const char *why)
 	printk("\n");
 }
 
+#else /* ifdef DEBUG */
+
+#define i2c_debug	0
+
+#define show_state(i2c) do { } while (0)
+#define decode_ISR(val) do { } while (0)
+#define decode_ICR(val) do { } while (0)
+#define i2c_pxa_scream_blue_murder(i2c, why) do { } while (0)
+
+#endif /* ifdef DEBUG / else */
+
+static void i2c_pxa_master_complete(struct pxa_i2c *i2c, int ret);
+static irqreturn_t i2c_pxa_handler(int this_irq, void *dev_id);
+
 static inline int i2c_pxa_is_slavemode(struct pxa_i2c *i2c)
 {
 	return !(readl(_ICR(i2c)) & ICR_SCLE);
-- 
1.6.6.2

[PATCH 3/3] MAINTAINERS: add i2c tree for embedded platforms

[Uwe Kleine-König]

Signed-off-by: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Acked-by: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
---
 MAINTAINERS |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 2533fc4..a3c936c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2626,6 +2626,7 @@ M:	"Ben Dooks (embedded platforms)" <ben-linux@fluff.org>
 L:	linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
 W:	http://i2c.wiki.kernel.org/
 T:	quilt kernel.org/pub/linux/kernel/people/jdelvare/linux-2.6/jdelvare-i2c/
+T:	git git://git.fluff.org/bjdooks/linux.git
 S:	Maintained
 F:	Documentation/i2c/
 F:	drivers/i2c/
-- 
1.6.6.2

Re: [PATCH 2/3] i2c/pxa: only define 'blue_murder'-function if DEBUG is #defined

[Russell King - ARM Linux]

On Wed, Feb 24, 2010 at 12:01:45PM +0100, Uwe Kleine-König wrote:
> From: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> 
> This talkative function is also called on timeouts. As timeouts can
> happen on regular writes to EEPROMs (no error case), this creates false
> positives.  Giving lots of details is interesting only for developers
> anyhow, so just use the function if DEBUG is #defined.

Are you sure this is safe?  If you time out the write before it completes,
how do you know if the write was successful?

I don't think this is "no error code" nor "false positive".  If the timeout
is too short for your EEPROMs, then the timeout needs to be increased.

Re: [PATCH 3/3] MAINTAINERS: add i2c tree for embedded platforms

[Russell King - ARM Linux]

On Wed, Feb 24, 2010 at 12:01:46PM +0100, Uwe Kleine-König wrote:
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> Acked-by: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
> ---
>  MAINTAINERS |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2533fc4..a3c936c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2626,6 +2626,7 @@ M:	"Ben Dooks (embedded platforms)" <ben-linux@fluff.org>
>  L:	linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>  W:	http://i2c.wiki.kernel.org/
>  T:	quilt kernel.org/pub/linux/kernel/people/jdelvare/linux-2.6/jdelvare-i2c/
> +T:	git git://git.fluff.org/bjdooks/linux.git

This is wrong.  This is the same tree which I pull ARM stuff from, so it
needs qualifying with a branch name.

Re: [PATCH 3/3] MAINTAINERS: add i2c tree for embedded platforms

[Uwe Kleine-König]

On Sun, Feb 28, 2010 at 03:57:54PM +0000, Russell King - ARM Linux wrote:
> On Wed, Feb 24, 2010 at 12:01:46PM +0100, Uwe Kleine-König wrote:
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > Acked-by: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
> > ---
> >  MAINTAINERS |    1 +
> >  1 files changed, 1 insertions(+), 0 deletions(-)
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 2533fc4..a3c936c 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -2626,6 +2626,7 @@ M:	"Ben Dooks (embedded platforms)" <ben-linux@fluff.org>
> >  L:	linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> >  W:	http://i2c.wiki.kernel.org/
> >  T:	quilt kernel.org/pub/linux/kernel/people/jdelvare/linux-2.6/jdelvare-i2c/
> > +T:	git git://git.fluff.org/bjdooks/linux.git
> 
> This is wrong.  This is the same tree which I pull ARM stuff from, so it
> needs qualifying with a branch name.
$(git ls-remote git://git.fluff.org/bjdooks/linux.git) suggests two
candidates:

	for-linus/i2c
	next-i2c

And I think for now this is only parsed by humans and everbody should be
able to notice that e.g. "next-s3c" doesn't contain i2c patches in the
presence of the two above branches.

So unless there is a syntax to specify more than one branch I suggest to
keep it as is.  Or should we only list next-i2c?  Ben, what do you
think?

(Maybe we can just make it:

	T: git git://git.fluff.org/bjdooks/linux.git for-linus/i2c
	T: git git://git.fluff.org/bjdooks/linux.git next-i2c

but this is a bit too much IMHO.  (Who volunteers to list all branches
of tip in MAINTAINERS? ;-))

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: [PATCH 3/3] MAINTAINERS: add i2c tree for embedded platforms

[Jean Delvare]

On Mon, 1 Mar 2010 10:07:14 +0100, Uwe Kleine-König wrote:
> On Sun, Feb 28, 2010 at 03:57:54PM +0000, Russell King - ARM Linux wrote:
> > On Wed, Feb 24, 2010 at 12:01:46PM +0100, Uwe Kleine-König wrote:
> > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > > Acked-by: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
> > > ---
> > >  MAINTAINERS |    1 +
> > >  1 files changed, 1 insertions(+), 0 deletions(-)
> > > 
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index 2533fc4..a3c936c 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -2626,6 +2626,7 @@ M:	"Ben Dooks (embedded platforms)" <ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org>
> > >  L:	linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > >  W:	http://i2c.wiki.kernel.org/
> > >  T:	quilt kernel.org/pub/linux/kernel/people/jdelvare/linux-2.6/jdelvare-i2c/
> > > +T:	git git://git.fluff.org/bjdooks/linux.git
> > 
> > This is wrong.  This is the same tree which I pull ARM stuff from, so it
> > needs qualifying with a branch name.
> $(git ls-remote git://git.fluff.org/bjdooks/linux.git) suggests two
> candidates:
> 
> 	for-linus/i2c
> 	next-i2c
> 
> And I think for now this is only parsed by humans and everbody should be
> able to notice that e.g. "next-s3c" doesn't contain i2c patches in the
> presence of the two above branches.
> 
> So unless there is a syntax to specify more than one branch I suggest to
> keep it as is.  Or should we only list next-i2c?  Ben, what do you
> think?

I presume that for-linus/i2c is a temporary subset of next-i2c for
Linus' consumption. So we can ignore for-linus/i2c and only list
next-i2c.

> (Maybe we can just make it:
> 
> 	T: git git://git.fluff.org/bjdooks/linux.git for-linus/i2c
> 	T: git git://git.fluff.org/bjdooks/linux.git next-i2c
> 
> but this is a bit too much IMHO.  (Who volunteers to list all branches
> of tip in MAINTAINERS? ;-))
> 
> Best regards
> Uwe
> 


-- 
Jean Delvare

[PATCH] MAINTAINERS: add i2c tree for embedded platforms

[Uwe Kleine-König]

Signed-off-by: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Acked-by: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
---
 MAINTAINERS |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index f520dd0..a01745b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2658,6 +2658,7 @@ M:	"Ben Dooks (embedded platforms)" <ben-linux@fluff.org>
 L:	linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
 W:	http://i2c.wiki.kernel.org/
 T:	quilt kernel.org/pub/linux/kernel/people/jdelvare/linux-2.6/jdelvare-i2c/
+T:	git git://git.fluff.org/bjdooks/linux.git next-i2c
 S:	Maintained
 F:	Documentation/i2c/
 F:	drivers/i2c/
-- 
1.7.0

Re: [PATCH 2/3] i2c/pxa: only define 'blue_murder'-function if DEBUG is #defined

[Uwe Kleine-König]

Hi Wolfram,

On Sun, Feb 28, 2010 at 03:55:02PM +0000, Russell King - ARM Linux wrote:
> On Wed, Feb 24, 2010 at 12:01:45PM +0100, Uwe Kleine-König wrote:
> > From: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > 
> > This talkative function is also called on timeouts. As timeouts can
> > happen on regular writes to EEPROMs (no error case), this creates false
> > positives.  Giving lots of details is interesting only for developers
> > anyhow, so just use the function if DEBUG is #defined.
> 
> Are you sure this is safe?  If you time out the write before it completes,
> how do you know if the write was successful?
> 
> I don't think this is "no error code" nor "false positive".  If the timeout
> is too short for your EEPROMs, then the timeout needs to be increased.
any thoughts about this by you?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: [PATCH] MAINTAINERS: add i2c tree for embedded platforms

[Uwe Kleine-König]

Hi Ben,

On Mon, Mar 01, 2010 at 12:12:16PM +0100, Uwe Kleine-König wrote:
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> Acked-by: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
> ---
>  MAINTAINERS |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f520dd0..a01745b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2658,6 +2658,7 @@ M:	"Ben Dooks (embedded platforms)" <ben-linux@fluff.org>
>  L:	linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>  W:	http://i2c.wiki.kernel.org/
>  T:	quilt kernel.org/pub/linux/kernel/people/jdelvare/linux-2.6/jdelvare-i2c/
> +T:	git git://git.fluff.org/bjdooks/linux.git next-i2c
>  S:	Maintained
>  F:	Documentation/i2c/
>  F:	drivers/i2c/
> -- 
> 1.7.0

ping

Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: [PATCH 1/3] i2c/pxa: remove unused macro

[Uwe Kleine-König]

Hi Ben,

On Wed, Feb 24, 2010 at 12:01:44PM +0100, Uwe Kleine-König wrote:
> Commit
> 
> 	beea494 ([ARM] Remove EEPROM slave emulation from i2c-pxa driver.)
> 
> removed all uses of eedbg, so the definition can go, too.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> Acked-by: Russell King <rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
> Cc: Eric Miao <eric.y.miao-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: Roel Kluin <roel.kluin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Acked-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
ping

Uwe
> ---
>  drivers/i2c/busses/i2c-pxa.c |    2 --
>  1 files changed, 0 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
> index 7647a20..b3c6c9d 100644
> --- a/drivers/i2c/busses/i2c-pxa.c
> +++ b/drivers/i2c/busses/i2c-pxa.c
> @@ -216,8 +216,6 @@ static void i2c_pxa_show_state(struct pxa_i2c *i2c, int lno, const char *fname)
>  #define decode_ICR(val) do { } while (0)
>  #endif
>  
> -#define eedbg(lvl, x...) do { if ((lvl) < 1) { printk(KERN_DEBUG "" x); } } while(0)
> -
>  static void i2c_pxa_master_complete(struct pxa_i2c *i2c, int ret);
>  static irqreturn_t i2c_pxa_handler(int this_irq, void *dev_id);
>  
> -- 
> 1.6.6.2

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: [PATCH 1/3] i2c/pxa: remove unused macro

[Roel Kluin]

Op 22-03-10 22:04, Uwe Kleine-König schreef:
> Hi Ben,
> 
> On Wed, Feb 24, 2010 at 12:01:44PM +0100, Uwe Kleine-König wrote:
>> Commit
>>
>> 	beea494 ([ARM] Remove EEPROM slave emulation from i2c-pxa driver.)
>>
>> removed all uses of eedbg, so the definition can go, too.
>>
>> Signed-off-by: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>> Acked-by: Russell King <rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
>> Cc: Eric Miao <eric.y.miao-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> Cc: Roel Kluin <roel.kluin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> Acked-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
> ping

FWIW:

Acked-by: Roel Kluin <roel.kluin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Re: [PATCH 2/3] i2c/pxa: only define 'blue_murder'-function if DEBUG is #defined

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1718 bytes --]


Sorry to bump this old thread, it dropped off my todo-list :(

On Sun, Feb 28, 2010 at 03:55:02PM +0000, Russell King - ARM Linux wrote:
> On Wed, Feb 24, 2010 at 12:01:45PM +0100, Uwe Kleine-König wrote:
> > From: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > 
> > This talkative function is also called on timeouts. As timeouts can
> > happen on regular writes to EEPROMs (no error case), this creates false
> > positives.  Giving lots of details is interesting only for developers
> > anyhow, so just use the function if DEBUG is #defined.
> 
> Are you sure this is safe?  If you time out the write before it completes,
> how do you know if the write was successful?
> 
> I don't think this is "no error code" nor "false positive".  If the timeout
> is too short for your EEPROMs, then the timeout needs to be increased.

I am sure this is safe because we have retries. The eeprom driver first tries
to write data without a delay, because EEPROMs often have buffers. Once the
buffers are full, the chip will not answer to the next write request which will
result in a timeout for this write request. This is expected, so it will be
retried after some delay. Something like -EBUSY. Only if another "outer"
timeout passed after some retries, then we have a problem and this should be
user visible. But the timeout for the write request is nothing exceptional and
the user doesn't need to be informed about it, especially not in this detail.
This is what the patch is addressing.

Kind regards,

   Wolfram

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 2/3] i2c/pxa: only define 'blue_murder'-function if DEBUG is #defined

[Russell King - ARM Linux]

On Sun, Apr 18, 2010 at 03:22:51PM +0200, Wolfram Sang wrote:
> 
> Sorry to bump this old thread, it dropped off my todo-list :(
> 
> On Sun, Feb 28, 2010 at 03:55:02PM +0000, Russell King - ARM Linux wrote:
> > On Wed, Feb 24, 2010 at 12:01:45PM +0100, Uwe Kleine-König wrote:
> > > From: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > > 
> > > This talkative function is also called on timeouts. As timeouts can
> > > happen on regular writes to EEPROMs (no error case), this creates false
> > > positives.  Giving lots of details is interesting only for developers
> > > anyhow, so just use the function if DEBUG is #defined.
> > 
> > Are you sure this is safe?  If you time out the write before it completes,
> > how do you know if the write was successful?
> > 
> > I don't think this is "no error code" nor "false positive".  If the timeout
> > is too short for your EEPROMs, then the timeout needs to be increased.
> 
> I am sure this is safe because we have retries. The eeprom driver first tries
> to write data without a delay, because EEPROMs often have buffers. Once the
> buffers are full, the chip will not answer to the next write request which will
> result in a timeout for this write request. This is expected, so it will be
> retried after some delay. Something like -EBUSY. Only if another "outer"
> timeout passed after some retries, then we have a problem and this should be
> user visible. But the timeout for the write request is nothing exceptional and
> the user doesn't need to be informed about it, especially not in this detail.
> This is what the patch is addressing.

And what if it's not an EEPROM that you're talking to?

Re: [PATCH 2/3] i2c/pxa: only define 'blue_murder'-function if DEBUG is #defined

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1398 bytes --]

> > I am sure this is safe because we have retries. The eeprom driver first tries
> > to write data without a delay, because EEPROMs often have buffers. Once the
> > buffers are full, the chip will not answer to the next write request which will
> > result in a timeout for this write request. This is expected, so it will be
> > retried after some delay. Something like -EBUSY. Only if another "outer"
> > timeout passed after some retries, then we have a problem and this should be
> > user visible. But the timeout for the write request is nothing exceptional and
> > the user doesn't need to be informed about it, especially not in this detail.
> > This is what the patch is addressing.
> 
> And what if it's not an EEPROM that you're talking to?

That's up to the corresponding driver. The driver is still notified via the
return value how many i2c-messages could be transmitted. If this is not equal
to what the driver intended, then it can decide to retry or notify the user.
And that is the apropriate level. Doing all this printout at the bus-driver
level is only interesting for developers. Users are frightened by the "timeout"
in their logs although everything might be as expected. A bus driver can't
know.

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 2/3] i2c/pxa: only define 'blue_murder'-function if DEBUG is #defined

[Pavel Machek]

On Sun 2010-04-18 16:05:48, Wolfram Sang wrote:
> > > I am sure this is safe because we have retries. The eeprom driver first tries
> > > to write data without a delay, because EEPROMs often have buffers. Once the
> > > buffers are full, the chip will not answer to the next write request which will
> > > result in a timeout for this write request. This is expected, so it will be
> > > retried after some delay. Something like -EBUSY. Only if another "outer"
> > > timeout passed after some retries, then we have a problem and this should be
> > > user visible. But the timeout for the write request is nothing exceptional and
> > > the user doesn't need to be informed about it, especially not in this detail.
> > > This is what the patch is addressing.
> > 
> > And what if it's not an EEPROM that you're talking to?
> 
> That's up to the corresponding driver. The driver is still notified via the
> return value how many i2c-messages could be transmitted. If this is not equal
> to what the driver intended, then it can decide to retry or notify the user.
> And that is the apropriate level. Doing all this printout at the bus-driver
> level is only interesting for developers. Users are frightened by the "timeout"
> in their logs although everything might be as expected. A bus driver can't
> know.

I guess expected conditions should not trigger log spam...?
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html