* [PATCH] net/fsl_pq_mdio: use spin_event_timeout() to poll the indicator register
@ 2012-07-03 23:16 Timur Tabi
2012-07-09 6:59 ` David Miller
0 siblings, 1 reply; 6+ messages in thread
From: Timur Tabi @ 2012-07-03 23:16 UTC (permalink / raw)
To: Andy Fleming, davem, netdev
Macro spin_event_timeout() was designed for simple polling of hardware
registers with a timeout, so use it when we poll the MIIMIND register.
This allows us to return an error code instead of polling indefinitely.
Note that PHY_INIT_TIMEOUT is a count of loop iterations, so we can't use
it for spin_event_timeout(), which asks for microseconds.
Signed-off-by: Timur Tabi <timur@freescale.com>
---
drivers/net/ethernet/freescale/fsl_pq_mdio.c | 25 +++++++++++++++----------
1 files changed, 15 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fsl_pq_mdio.c b/drivers/net/ethernet/freescale/fsl_pq_mdio.c
index 9eb8159..ab0fabd 100644
--- a/drivers/net/ethernet/freescale/fsl_pq_mdio.c
+++ b/drivers/net/ethernet/freescale/fsl_pq_mdio.c
@@ -64,6 +64,8 @@ struct fsl_pq_mdio_priv {
int fsl_pq_local_mdio_write(struct fsl_pq_mdio __iomem *regs, int mii_id,
int regnum, u16 value)
{
+ u32 status;
+
/* Set the PHY address and the register address we want to write */
out_be32(®s->miimadd, (mii_id << 8) | regnum);
@@ -71,10 +73,10 @@ int fsl_pq_local_mdio_write(struct fsl_pq_mdio __iomem *regs, int mii_id,
out_be32(®s->miimcon, value);
/* Wait for the transaction to finish */
- while (in_be32(®s->miimind) & MIIMIND_BUSY)
- cpu_relax();
+ status = spin_event_timeout(!(in_be32(®s->miimind) & MIIMIND_BUSY),
+ 1000, 0);
- return 0;
+ return status ? 0 : -ETIMEDOUT;
}
/*
@@ -91,6 +93,7 @@ int fsl_pq_local_mdio_read(struct fsl_pq_mdio __iomem *regs,
int mii_id, int regnum)
{
u16 value;
+ u32 status;
/* Set the PHY address and the register address we want to read */
out_be32(®s->miimadd, (mii_id << 8) | regnum);
@@ -99,9 +102,11 @@ int fsl_pq_local_mdio_read(struct fsl_pq_mdio __iomem *regs,
out_be32(®s->miimcom, 0);
out_be32(®s->miimcom, MII_READ_COMMAND);
- /* Wait for the transaction to finish */
- while (in_be32(®s->miimind) & (MIIMIND_NOTVALID | MIIMIND_BUSY))
- cpu_relax();
+ /* Wait for the transaction to finish, normally less than 100us */
+ status = spin_event_timeout(!(in_be32(®s->miimind) &
+ (MIIMIND_NOTVALID | MIIMIND_BUSY)), 1000, 0);
+ if (!status)
+ return -ETIMEDOUT;
/* Grab the value of the register from miimstat */
value = in_be32(®s->miimstat);
@@ -144,7 +149,7 @@ int fsl_pq_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
static int fsl_pq_mdio_reset(struct mii_bus *bus)
{
struct fsl_pq_mdio __iomem *regs = fsl_pq_mdio_get_regs(bus);
- int timeout = PHY_INIT_TIMEOUT;
+ u32 status;
mutex_lock(&bus->mdio_lock);
@@ -155,12 +160,12 @@ static int fsl_pq_mdio_reset(struct mii_bus *bus)
out_be32(®s->miimcfg, MIIMCFG_INIT_VALUE);
/* Wait until the bus is free */
- while ((in_be32(®s->miimind) & MIIMIND_BUSY) && timeout--)
- cpu_relax();
+ status = spin_event_timeout(!(in_be32(®s->miimind) & MIIMIND_BUSY),
+ 1000, 0);
mutex_unlock(&bus->mdio_lock);
- if (timeout < 0) {
+ if (!status) {
printk(KERN_ERR "%s: The MII Bus is stuck!\n",
bus->name);
return -EBUSY;
--
1.7.3.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] net/fsl_pq_mdio: use spin_event_timeout() to poll the indicator register
2012-07-03 23:16 [PATCH] net/fsl_pq_mdio: use spin_event_timeout() to poll the indicator register Timur Tabi
@ 2012-07-09 6:59 ` David Miller
2012-07-09 14:31 ` Tabi Timur-B04825
0 siblings, 1 reply; 6+ messages in thread
From: David Miller @ 2012-07-09 6:59 UTC (permalink / raw)
To: timur; +Cc: afleming, netdev
From: Timur Tabi <timur@freescale.com>
Date: Tue, 3 Jul 2012 18:16:21 -0500
> Macro spin_event_timeout() was designed for simple polling of hardware
> registers with a timeout, so use it when we poll the MIIMIND register.
> This allows us to return an error code instead of polling indefinitely.
>
> Note that PHY_INIT_TIMEOUT is a count of loop iterations, so we can't use
> it for spin_event_timeout(), which asks for microseconds.
>
> Signed-off-by: Timur Tabi <timur@freescale.com>
Define a macro for the timeout value rather than use an arbitrary
constant.
> + status = spin_event_timeout(!(in_be32(®s->miimind) & MIIMIND_BUSY),
> + 1000, 0);
This indentation is absolutely terrible.
> + status = spin_event_timeout(!(in_be32(®s->miimind) &
> + (MIIMIND_NOTVALID | MIIMIND_BUSY)), 1000, 0);
Same here.
> + status = spin_event_timeout(!(in_be32(®s->miimind) & MIIMIND_BUSY),
> + 1000, 0);
And here too.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net/fsl_pq_mdio: use spin_event_timeout() to poll the indicator register
2012-07-09 6:59 ` David Miller
@ 2012-07-09 14:31 ` Tabi Timur-B04825
2012-07-09 19:50 ` Jan Ceuleers
2012-07-09 21:17 ` David Miller
0 siblings, 2 replies; 6+ messages in thread
From: Tabi Timur-B04825 @ 2012-07-09 14:31 UTC (permalink / raw)
To: David Miller; +Cc: Fleming Andy-AFLEMING, netdev@vger.kernel.org
David Miller wrote:
> Define a macro for the timeout value rather than use an arbitrary
> constant.
Ok.
>> + status = spin_event_timeout(!(in_be32(®s->miimind) & MIIMIND_BUSY),
>> + 1000, 0);
>
> This indentation is absolutely terrible.
Can you give me a clue as to how you think it should look? I could not
come up with a good way to break up these lines and keep them under 80
characters.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net/fsl_pq_mdio: use spin_event_timeout() to poll the indicator register
2012-07-09 14:31 ` Tabi Timur-B04825
@ 2012-07-09 19:50 ` Jan Ceuleers
2012-07-09 21:17 ` David Miller
1 sibling, 0 replies; 6+ messages in thread
From: Jan Ceuleers @ 2012-07-09 19:50 UTC (permalink / raw)
To: Tabi Timur-B04825
Cc: David Miller, Fleming Andy-AFLEMING, netdev@vger.kernel.org
On 07/09/2012 04:31 PM, Tabi Timur-B04825 wrote:
>> This indentation is absolutely terrible.
>
> Can you give me a clue as to how you think it should look? I could not
> come up with a good way to break up these lines and keep them under 80
> characters.
>
How about this:
+ status = spin_event_timeout(!(in_be32(®s->miimind) & MIIMIND_BUSY),
+ 1000, 0);
On the second line use as many tabs as possible, then continue with spaces until the '1' in 1000 lines up with the '!'.
HTH, Jan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net/fsl_pq_mdio: use spin_event_timeout() to poll the indicator register
2012-07-09 14:31 ` Tabi Timur-B04825
2012-07-09 19:50 ` Jan Ceuleers
@ 2012-07-09 21:17 ` David Miller
2012-07-09 21:19 ` Timur Tabi
1 sibling, 1 reply; 6+ messages in thread
From: David Miller @ 2012-07-09 21:17 UTC (permalink / raw)
To: B04825; +Cc: afleming, netdev
From: Tabi Timur-B04825 <B04825@freescale.com>
Date: Mon, 9 Jul 2012 14:31:33 +0000
> David Miller wrote:
>
>> Define a macro for the timeout value rather than use an arbitrary
>> constant.
>
> Ok.
>
>>> + status = spin_event_timeout(!(in_be32(®s->miimind) & MIIMIND_BUSY),
>>> + 1000, 0);
>>
>> This indentation is absolutely terrible.
>
> Can you give me a clue as to how you think it should look? I could not
> come up with a good way to break up these lines and keep them under 80
> characters.
It's not the length of the first line, it's how the second line was
indented.
Always line up the arguments on the second and subsequent lines right
after the column containing the openning "(" on the first line.
foo = function(arg1,
arg2);
Do you really mean that:
foo = function(arg1,
arg2);
doesn't look like complete and utter shit to you?
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-07-09 21:19 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-07-03 23:16 [PATCH] net/fsl_pq_mdio: use spin_event_timeout() to poll the indicator register Timur Tabi
2012-07-09 6:59 ` David Miller
2012-07-09 14:31 ` Tabi Timur-B04825
2012-07-09 19:50 ` Jan Ceuleers
2012-07-09 21:17 ` David Miller
2012-07-09 21:19 ` Timur Tabi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).