* [PATCH] netxen: only half timeout waited
@ 2009-02-16 13:55 Roel Kluin
2009-02-17 7:33 ` Jarek Poplawski
0 siblings, 1 reply; 3+ messages in thread
From: Roel Kluin @ 2009-02-16 13:55 UTC (permalink / raw)
To: dhananjay; +Cc: netdev, Andrew Morton
Only half the timeout was waited, due to the double increment, and an error
occurred one too early.
Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
diff --git a/drivers/net/netxen/netxen_nic_niu.c b/drivers/net/netxen/netxen_nic_niu.c
index c3b9c83..df3aa08 100644
--- a/drivers/net/netxen/netxen_nic_niu.c
+++ b/drivers/net/netxen/netxen_nic_niu.c
@@ -147,12 +147,11 @@ int netxen_niu_gbe_phy_read(struct netxen_adapter *adapter, long reg,
NETXEN_NIU_GB_MII_MGMT_INDICATE(0),
&status, 4))
return -EIO;
- timeout++;
} while ((netxen_get_gb_mii_mgmt_busy(status)
|| netxen_get_gb_mii_mgmt_notvalid(status))
&& (timeout++ < NETXEN_NIU_PHY_WAITMAX));
- if (timeout < NETXEN_NIU_PHY_WAITMAX) {
+ if (timeout <= NETXEN_NIU_PHY_WAITMAX) {
if (adapter->hw_read_wx(adapter,
NETXEN_NIU_GB_MII_MGMT_STATUS(0),
readval, 4))
@@ -240,11 +239,10 @@ int netxen_niu_gbe_phy_write(struct netxen_adapter *adapter, long reg,
NETXEN_NIU_GB_MII_MGMT_INDICATE(0),
&status, 4))
return -EIO;
- timeout++;
} while ((netxen_get_gb_mii_mgmt_busy(status))
&& (timeout++ < NETXEN_NIU_PHY_WAITMAX));
- if (timeout < NETXEN_NIU_PHY_WAITMAX)
+ if (timeout <= NETXEN_NIU_PHY_WAITMAX)
result = 0;
else
result = -EIO;
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] netxen: only half timeout waited
2009-02-16 13:55 [PATCH] netxen: only half timeout waited Roel Kluin
@ 2009-02-17 7:33 ` Jarek Poplawski
2009-02-17 7:41 ` Jarek Poplawski
0 siblings, 1 reply; 3+ messages in thread
From: Jarek Poplawski @ 2009-02-17 7:33 UTC (permalink / raw)
To: Roel Kluin; +Cc: dhananjay, netdev, Andrew Morton
On 16-02-2009 14:55, Roel Kluin wrote:
> Only half the timeout was waited, due to the double increment, and an error
> occurred one too early.
>
> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
> ---
> diff --git a/drivers/net/netxen/netxen_nic_niu.c b/drivers/net/netxen/netxen_nic_niu.c
> index c3b9c83..df3aa08 100644
> --- a/drivers/net/netxen/netxen_nic_niu.c
> +++ b/drivers/net/netxen/netxen_nic_niu.c
> @@ -147,12 +147,11 @@ int netxen_niu_gbe_phy_read(struct netxen_adapter *adapter, long reg,
> NETXEN_NIU_GB_MII_MGMT_INDICATE(0),
> &status, 4))
> return -EIO;
> - timeout++;
> } while ((netxen_get_gb_mii_mgmt_busy(status)
> || netxen_get_gb_mii_mgmt_notvalid(status))
> && (timeout++ < NETXEN_NIU_PHY_WAITMAX));
>
> - if (timeout < NETXEN_NIU_PHY_WAITMAX) {
> + if (timeout <= NETXEN_NIU_PHY_WAITMAX) {
Roel, very good caches, but IMHO in all such cases changing to
++timeout with 'if (timeout == NETXEN_NIU_PHY_WAITMAX)' where
possible, should be really more readable. At least wrt. next similar
patches.
Thanks,
Jarek P.
> if (adapter->hw_read_wx(adapter,
> NETXEN_NIU_GB_MII_MGMT_STATUS(0),
> readval, 4))
> @@ -240,11 +239,10 @@ int netxen_niu_gbe_phy_write(struct netxen_adapter *adapter, long reg,
> NETXEN_NIU_GB_MII_MGMT_INDICATE(0),
> &status, 4))
> return -EIO;
> - timeout++;
> } while ((netxen_get_gb_mii_mgmt_busy(status))
> && (timeout++ < NETXEN_NIU_PHY_WAITMAX));
>
> - if (timeout < NETXEN_NIU_PHY_WAITMAX)
> + if (timeout <= NETXEN_NIU_PHY_WAITMAX)
> result = 0;
> else
> result = -EIO;
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] netxen: only half timeout waited
2009-02-17 7:33 ` Jarek Poplawski
@ 2009-02-17 7:41 ` Jarek Poplawski
0 siblings, 0 replies; 3+ messages in thread
From: Jarek Poplawski @ 2009-02-17 7:41 UTC (permalink / raw)
To: Roel Kluin; +Cc: dhananjay, netdev, Andrew Morton
On Tue, Feb 17, 2009 at 07:33:12AM +0000, Jarek Poplawski wrote:
> On 16-02-2009 14:55, Roel Kluin wrote:
> > Only half the timeout was waited, due to the double increment, and an error
> > occurred one too early.
> >
> > Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
> > ---
> > diff --git a/drivers/net/netxen/netxen_nic_niu.c b/drivers/net/netxen/netxen_nic_niu.c
> > index c3b9c83..df3aa08 100644
> > --- a/drivers/net/netxen/netxen_nic_niu.c
> > +++ b/drivers/net/netxen/netxen_nic_niu.c
> > @@ -147,12 +147,11 @@ int netxen_niu_gbe_phy_read(struct netxen_adapter *adapter, long reg,
> > NETXEN_NIU_GB_MII_MGMT_INDICATE(0),
> > &status, 4))
> > return -EIO;
> > - timeout++;
> > } while ((netxen_get_gb_mii_mgmt_busy(status)
> > || netxen_get_gb_mii_mgmt_notvalid(status))
> > && (timeout++ < NETXEN_NIU_PHY_WAITMAX));
> >
> > - if (timeout < NETXEN_NIU_PHY_WAITMAX) {
> > + if (timeout <= NETXEN_NIU_PHY_WAITMAX) {
>
> Roel, very good caches, but IMHO in all such cases changing to
> ++timeout with 'if (timeout == NETXEN_NIU_PHY_WAITMAX)' where
Hmm... of course 'if (timeout == NETXEN_NIU_PHY_WAITMAX)' for errcode;
in this particular case: 'if (timeout < NETXEN_NIU_PHY_WAITMAX)' would
be OK.
Jarek P.
> possible, should be really more readable. At least wrt. next similar
> patches.
>
> Thanks,
> Jarek P.
>
> > if (adapter->hw_read_wx(adapter,
> > NETXEN_NIU_GB_MII_MGMT_STATUS(0),
> > readval, 4))
> > @@ -240,11 +239,10 @@ int netxen_niu_gbe_phy_write(struct netxen_adapter *adapter, long reg,
> > NETXEN_NIU_GB_MII_MGMT_INDICATE(0),
> > &status, 4))
> > return -EIO;
> > - timeout++;
> > } while ((netxen_get_gb_mii_mgmt_busy(status))
> > && (timeout++ < NETXEN_NIU_PHY_WAITMAX));
> >
> > - if (timeout < NETXEN_NIU_PHY_WAITMAX)
> > + if (timeout <= NETXEN_NIU_PHY_WAITMAX)
> > result = 0;
> > else
> > result = -EIO;
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2009-02-17 7:41 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-02-16 13:55 [PATCH] netxen: only half timeout waited Roel Kluin
2009-02-17 7:33 ` Jarek Poplawski
2009-02-17 7:41 ` Jarek Poplawski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox