i2c/pxa: remove unused macro
PATCH 1/2 i2c/pxa: remove unused macro
Commit
beea494 ([ARM] Remove EEPROM slave emulation from i2c-pxa driver.)
removed all uses of eedbg, so the definition can go, too.
--- drivers/i2c/busses/i2c-pxa.c | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-)
drivers/i2c/busses/i2c-pxa.c +0 -2
@@ -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);
Signature · 1 line
1.6.5.2
PATCH 1/2 i2c/pxa: remove unused macro
Hello,
Quoted 1 line
Cc Eric Miao <[email protected]>
postmaster at marvell.com claims the address ymiao3 at marvell.com 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 at gmail.com which hopefully works.
Best regards
Uwe
Signature · 2 lines
Pengutronix e.K. | Uwe Kleine-K?nig | Industrial Linux Solutions | http://www.pengutronix.de/ |
PATCH 1/2 i2c/pxa: remove unused macro
In reply to Uwe Kleine-KönigQuoted 4 lines · Uwe Kleine-K?nig
Commit
beea494 ([ARM] Remove EEPROM slave emulation from i2c-pxa driver.)
Quoted 8 lines
removed all uses of eedbg, so the definition can go, too.
Signed-off-by Uwe Kleine-K?nig <[email protected]>Cc Jean Delvare <[email protected]>Cc Ben Dooks <[email protected]>Cc Russell King <[email protected]>Cc Eric Miao <[email protected]>Cc Roel Kluin <[email protected]>
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
PATCH 1/2 i2c/pxa: remove unused macro
In reply to Uwe Kleine-KönigCommit
beea494 ([ARM] Remove EEPROM slave emulation from i2c-pxa driver.)
removed all uses of eedbg, so the definition can go, too.
--- drivers/i2c/busses/i2c-pxa.c | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-)
drivers/i2c/busses/i2c-pxa.c +0 -2
@@ -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);
Signature · 1 line
1.6.5.2
PATCH 1/2 i2c/pxa: remove unused macro
Commit
beea494 ([ARM] Remove EEPROM slave emulation from i2c-pxa driver.)
removed all uses of eedbg, so the definition can go, too.
--- drivers/i2c/busses/i2c-pxa.c | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-)
drivers/i2c/busses/i2c-pxa.c +0 -2
@@ -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);
Signature · 1 line
1.6.5.2
PATCH 1/2 i2c/pxa: remove unused macro
Hello,
Quoted 5 lines · 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.
does anyone care to take this and the followup patch?
Best regards
Uwe
Signature · 2 lines
Pengutronix e.K. | Uwe Kleine-K?nig | Industrial Linux Solutions | http://www.pengutronix.de/ |
PATCH 1/2 i2c/pxa: remove unused macro
In reply to Uwe Kleine-KönigHi Jean,
Quoted 5 lines · 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.
do you have comments to my patches? I think they should go in via your tree.
Best regards
Uwe
Signature · 2 lines
Pengutronix e.K. | Uwe Kleine-K?nig | Industrial Linux Solutions | http://www.pengutronix.de/ |
PATCH 1/2 i2c/pxa: remove unused macro
Quoted 10 lines · Uwe Kleine-K?nig
Hi Jean,
Quoted 5 lines · 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.
do you have comments to my patches? I think they should go in via your tree.
I think you should read MAINTAINERS again.
Signature · 1 line
Jean Delvare
PATCH 1/2 i2c/pxa: remove unused macro
Hi Jean,
Quoted 6 lines · Jean Delvare
Quoted 5 lines · Uwe Kleine-K?nig
Hi Jean,
Quoted 2 lines · Uwe Kleine-K?nig
Commit
Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
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
MAINTAINERSagain.
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
Signature · 2 lines
Pengutronix e.K. | Uwe Kleine-K?nig | Industrial Linux Solutions | http://www.pengutronix.de/ |
PATCH 1/2 i2c/pxa: remove unused macro
Quoted 12 lines · Uwe Kleine-K?nig
Hi Jean,
Quoted 9 lines · Jean Delvare
Quoted 8 lines · Uwe Kleine-K?nig
Hi Jean,
Quoted 5 lines · 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.
Quoted 8 lines
Quoted 4 lines
Quoted 2 lines
do you have comments to my patches? I think they should go in via your tree.
I think you should read
MAINTAINERSagain.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,
Signature · 1 line
Jean Delvare
PATCH 1/2 i2c/pxa: remove unused macro
Hi Ben,
Quoted 9 lines · Jean Delvare
Quoted 8 lines · Uwe Kleine-K?nig
Quoted 5 lines · Jean Delvare
Quoted 2 lines · Uwe Kleine-K?nig
do you have comments to my patches? I think they should go in via your tree.
I think you should read
MAINTAINERSagain.Ah, so the maintainer is Ben Dooks, but the tree seems to be your's.
Quoted 8 lines
Quoted 2 lines
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
MAINTAINERSto make things clearer for contributors?
any updates here? comments for my patches?
Best regards
Uwe
Signature · 2 lines
Pengutronix e.K. | Uwe Kleine-K?nig | Industrial Linux Solutions | http://www.pengutronix.de/ |
PATCH 1/2 i2c/pxa: remove unused macro
In reply to Jean DelvareHey Ben,
Quoted 19 lines · Jean Delvare
Quoted 18 lines · Uwe Kleine-K?nig
Hi Jean,
Quoted 6 lines · Jean Delvare
Quoted 5 lines · Uwe Kleine-K?nig
Hi Jean,
Quoted 2 lines · Uwe Kleine-K?nig
Commit
Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
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
MAINTAINERSagain.Ah, so the maintainer is Ben Dooks, but the tree seems to be your's.
Quoted 8 lines
Quoted 2 lines
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
MAINTAINERSto 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(-)
Signature · 2 lines
Pengutronix e.K. | Uwe Kleine-K?nig | Industrial Linux Solutions | http://www.pengutronix.de/ |
PATCH 1/2 i2c/pxa: remove unused macro
Hello Ben,
Quoted 2 lines
Quoted 2 lines
Ben, can you please add your tree to
MAINTAINERSto make things clearer for contributors?
I just sent out a patch for that to linux-i2c.
Quoted 13 lines
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/i2cShortlog and diffstat can be found below.
Best regards
Uwe
Quoted 6 lines
Uwe Kleine-K?nig (2):
i2c/pxa: remove unused macro
i2c-imx: call ioremap only afterrequest_mem_regionWolfram 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
Signature · 2 lines
Pengutronix e.K. | Uwe Kleine-K?nig | Industrial Linux Solutions | http://www.pengutronix.de/ |
PATCH 2/2 i2c/pxa: only define 'blue_murder'-function if DEBUG is #defined
In reply to Uwe Kleine-KönigFrom: Wolfram Sang <[email protected]>
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.
--- drivers/i2c/busses/i2c-pxa.c | 24 ++++++++++++++---------- 1 files changed, 14 insertions(+), 10 deletions(-)
drivers/i2c/busses/i2c-pxa.c +14 -10
@@ -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 );
Signature · 1 line
1.6.5.2
my i2c-patches for 2.6.34
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(-)
Signature · 2 lines
Pengutronix e.K. | Uwe Kleine-K?nig | Industrial Linux Solutions | http://www.pengutronix.de/ |
PATCH 1/3 i2c/pxa: remove unused macro
Commit
beea494 ([ARM] Remove EEPROM slave emulation from i2c-pxa driver.)
removed all uses of eedbg, so the definition can go, too.
--- drivers/i2c/busses/i2c-pxa.c | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-)
drivers/i2c/busses/i2c-pxa.c +0 -2
@@ -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);
Signature · 1 line
1.6.6.2
PATCH 1/3 i2c/pxa: remove unused macro
Hi Ben,
Quoted 11 lines · 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 <[email protected]>Acked-by Russell King <[email protected]>Cc Eric Miao <[email protected]>Cc Roel Kluin <[email protected]>Acked-by Pavel Machek <[email protected]>
ping
Uwe
Quoted 19 lines
--- drivers/i2c/busses/i2c-pxa.c | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-)drivers/i2c/busses/i2c-pxa.c +0
-2@@ -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);Signature · 1 line
1.6.6.2
Signature · 2 lines
Pengutronix e.K. | Uwe Kleine-K?nig | Industrial Linux Solutions | http://www.pengutronix.de/ |
PATCH 1/3 i2c/pxa: remove unused macro
Op 22-03-10 22:04, Uwe Kleine-K?nig schreef:
Quoted 7 lines
Hi Ben,
Quoted 4 lines · Uwe Kleine-K?nig
Commit
beea494 ([ARM] Remove EEPROM slave emulation from i2c-pxa driver.)
Quoted 8 lines
Quoted 7 lines
removed all uses of eedbg, so the definition can go, too.
Signed-off-by Uwe Kleine-K?nig <[email protected]>Acked-by Russell King <[email protected]>Cc Eric Miao <[email protected]>Cc Roel Kluin <[email protected]>Acked-by Pavel Machek <[email protected]>ping
FWIW:
PATCH 2/3 i2c/pxa: only define 'blue_murder'-function if DEBUG is #defined
In reply to Uwe Kleine-KönigFrom: Wolfram Sang <[email protected]>
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.
--- drivers/i2c/busses/i2c-pxa.c | 24 ++++++++++++++---------- 1 files changed, 14 insertions(+), 10 deletions(-)
drivers/i2c/busses/i2c-pxa.c +14 -10
@@ -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 );
Signature · 1 line
1.6.6.2
PATCH 2/3 i2c/pxa: only define 'blue_murder'-function if DEBUG is #defined
Quoted 6 lines · Uwe Kleine-K?nig
From: Wolfram Sang <[email protected]>
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.
PATCH 2/3 i2c/pxa: only define 'blue_murder'-function if DEBUG is #defined
Hi Wolfram,
Quoted 8 lines · Russell King - ARM Linux
Quoted 6 lines · Uwe Kleine-K?nig
From: Wolfram Sang <[email protected]>
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.
Quoted 5 lines
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
Signature · 2 lines
Pengutronix e.K. | Uwe Kleine-K?nig | Industrial Linux Solutions | http://www.pengutronix.de/ |
PATCH 2/3 i2c/pxa: only define 'blue_murder'-function if DEBUG is #defined
In reply to Russell King - ARM LinuxSorry to bump this old thread, it dropped off my todo-list :(
Quoted 8 lines · Russell King - ARM Linux
Quoted 6 lines · Uwe Kleine-K?nig
From: Wolfram Sang <[email protected]>
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.
Quoted 5 lines
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
Signature · 9 lines
Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ | -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 197 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20100418/56a7b943/attachment.sig>
PATCH 2/3 i2c/pxa: only define 'blue_murder'-function if DEBUG is #defined
Quoted 18 lines · Wolfram Sang
Sorry to bump this old thread, it dropped off my todo-list :(
Quoted 8 lines · Russell King - ARM Linux
Quoted 6 lines · Uwe Kleine-K?nig
From: Wolfram Sang <[email protected]>
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.
Quoted 5 lines
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.
Quoted 9 lines
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?
PATCH 2/3 i2c/pxa: only define 'blue_murder'-function if DEBUG is #defined
Quoted 11 lines
Quoted 9 lines
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.
Signature · 9 lines
Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ | -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 197 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20100418/b4e5ff85/attachment.sig>
PATCH 2/3 i2c/pxa: only define 'blue_murder'-function if DEBUG is #defined
Quoted 12 lines · Wolfram Sang
Quoted 11 lines
Quoted 9 lines
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?
Quoted 7 lines
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
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
PATCH 3/3 MAINTAINERS: add i2c tree for embedded platforms
In reply to Uwe Kleine-König--- MAINTAINERS | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
MAINTAINERS +1 -0
@@ -2626,6 +2626,7 @@ M: "Ben Dooks (embedded platforms)" <[email protected]> L: linux-i2c at vger.kernel.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/
Signature · 1 line
1.6.6.2
PATCH 3/3 MAINTAINERS: add i2c tree for embedded platforms
Quoted 11 lines · Uwe Kleine-K?nig
Signed-off-by Uwe Kleine-K?nig <[email protected]>Acked-by Jean Delvare <[email protected]>--- MAINTAINERS | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)MAINTAINERS +0
-0
MAINTAINERS M: "Ben Dooks (embedded platforms)" <[email protected]> · line 2629
Quoted 4 lines
MAINTAINERS +1
-0@@ -2626,6 +2626,7 @@ M: "Ben Dooks (embedded platforms)" <[email protected]> L: linux-i2c at vger.kernel.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.
PATCH 3/3 MAINTAINERS: add i2c tree for embedded platforms
Quoted 7 lines · Russell King - ARM Linux
Quoted 6 lines · Uwe Kleine-K?nig
Signed-off-by Uwe Kleine-K?nig <[email protected]>Acked-by Jean Delvare <[email protected]>--- MAINTAINERS | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
Quoted 12 lines
MAINTAINERS M: "Ben Dooks (embedded platforms)" <[email protected]> · line 2629
Quoted 9 lines
MAINTAINERS +1
-0@@ -2626,6 +2626,7 @@ M: "Ben Dooks (embedded platforms)" <[email protected]> L: linux-i2c at vger.kernel.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.gitThis 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
Signature · 2 lines
Pengutronix e.K. | Uwe Kleine-K?nig | Industrial Linux Solutions | http://www.pengutronix.de/ |
PATCH 3/3 MAINTAINERS: add i2c tree for embedded platforms
Quoted 26 lines · Uwe Kleine-K?nig
Quoted 7 lines · Russell King - ARM Linux
Quoted 6 lines · Uwe Kleine-K?nig
Signed-off-by Uwe Kleine-K?nig <[email protected]>Acked-by Jean Delvare <[email protected]>--- MAINTAINERS | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)Quoted 12 lines
MAINTAINERS M: "Ben Dooks (embedded platforms)" <[email protected]> · line 2629
Quoted 9 lines
MAINTAINERS +1
-0@@ -2626,6 +2626,7 @@ M: "Ben Dooks (embedded platforms)" <[email protected]> L: linux-i2c at vger.kernel.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.gitThis 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
Quoted 7 lines
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.
Quoted 11 lines
(Maybe we can just make it:
T: git
git://git.fluff.org/bjdooks/linux.gitfor-linus/i2c
T: gitgit://git.fluff.org/bjdooks/linux.gitnext-i2cbut this is a bit too much IMHO. (Who volunteers to list all branches of tip in
MAINTAINERS? ;-))Best regards
Uwe
Signature · 1 line
Jean Delvare
PATCH 1/1 MAINTAINERS: add i2c tree for embedded platforms
--- MAINTAINERS | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
MAINTAINERS +1 -0
@@ -2658,6 +2658,7 @@ M: "Ben Dooks (embedded platforms)" <[email protected]> L: linux-i2c at vger.kernel.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/
Signature · 1 line
1.7.0
PATCH 1/1 MAINTAINERS: add i2c tree for embedded platforms
Hi Ben,
Quoted 20 lines · Uwe Kleine-K?nig
Signed-off-by Uwe Kleine-K?nig <[email protected]>Acked-by Jean Delvare <[email protected]>--- MAINTAINERS | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)MAINTAINERS +1
-0@@ -2658,6 +2658,7 @@ M: "Ben Dooks (embedded platforms)" <[email protected]> L: linux-i2c at vger.kernel.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/Signature · 1 line
1.7.0
ping
Uwe
Signature · 2 lines
Pengutronix e.K. | Uwe Kleine-K?nig | Industrial Linux Solutions | http://www.pengutronix.de/ |
PATCH 1/1 MAINTAINERS: add i2c branch for embedded platforms
In reply to Uwe Kleine-KönigWhen I suggested the patch that resulted in
0e53300 (MAINTAINERS: add i2c tree for embedded platforms)
Russell correctly objected that this needs a branch name as there are different branches intended for upstream. Somehow my initial patch was picked up instead of the updated one.
--- MAINTAINERS | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
MAINTAINERS +1 -1
@@ -2843,7 +2843,7 @@ M: "Ben Dooks (embedded platforms)" <[email protected]> L: [email protected] 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 +T: git git://git.fluff.org/bjdooks/linux.git next-i2c S: Maintained F: Documentation/i2c/ F: drivers/i2c/
Signature · 1 line
1.7.2.3
PATCH 2/2 i2c/pxa: only define 'blue_murder'-function if DEBUG is #defined
In reply to Uwe Kleine-KönigFrom: Wolfram Sang <[email protected]>
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.
--- drivers/i2c/busses/i2c-pxa.c | 24 ++++++++++++++---------- 1 files changed, 14 insertions(+), 10 deletions(-)
drivers/i2c/busses/i2c-pxa.c +14 -10
@@ -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 );
Signature · 1 line
1.6.5.2
PATCH 2/2 i2c/pxa: only define 'blue_murder'-function if DEBUG is #defined
2009/11/9 Uwe Kleine-K?nig <[email protected]>:
Quoted 7 lines
From: Wolfram Sang <[email protected]>
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 i2c/pxa: remove unused macro
In reply to Uwe Kleine-KönigQuoted 3 lines · Uwe Kleine-K?nig
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.
Signature · 3 lines
Russell King Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/ maintainer of:
PATCH 1/2 i2c/pxa: remove unused macro
Quoted 5 lines · Russell King
Quoted 3 lines · Uwe Kleine-K?nig
Commit
beea494 ([ARM] Remove EEPROM slave emulation from i2c-pxa driver.)
Quoted 7 lines
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.
Signature · 3 lines
Ben (ben at fluff.org, http://www.fluff.org/) 'a smiley only costs 4 bytes'
PATCH 1/2 i2c/pxa: remove unused macro
Quoted 12 lines · Ben Dooks
Quoted 11 lines · Russell King
Quoted 3 lines · Uwe Kleine-K?nig
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.
Quoted 4 lines
Quoted 1 line
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.
Quoted 1 line
I'll NAK any changes to remove the
eedbg()for the moment.
That change is also fine, consider it acked:
Sorry for the noise.
PATCH 1/2 i2c/pxa: remove unused macro
In reply to Russell KingHello,
Quoted 5 lines · Russell King
Quoted 3 lines · Uwe Kleine-K?nig
Commit
beea494 ([ARM] Remove EEPROM slave emulation from i2c-pxa driver.)
Quoted 7 lines
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
Signature · 2 lines
Pengutronix e.K. | Uwe Kleine-K?nig | Industrial Linux Solutions | http://www.pengutronix.de/ |
PATCH 1/2 i2c/pxa: remove unused macro
In reply to Uwe Kleine-KönigQuoted 5 lines · 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.
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.
Signature · 3 lines
Ben (ben at fluff.org, http://www.fluff.org/) 'a smiley only costs 4 bytes'
PATCH 2/2 i2c/pxa: only define 'blue_murder'-function if DEBUG is #defined
In reply to Uwe Kleine-KönigFrom: Wolfram Sang <[email protected]>
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.
--- drivers/i2c/busses/i2c-pxa.c | 24 ++++++++++++++---------- 1 files changed, 14 insertions(+), 10 deletions(-)
drivers/i2c/busses/i2c-pxa.c +14 -10
@@ -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 );
Signature · 1 line
1.6.5.2