2006-10-10 23:36:40

by Jeff Garzik

Subject: [PATCH] WAN/pc300: handle, propagate minor errors

- move definition of 'tmc' and 'br' locals closer to usage

- handle clock_rate_calc() error

- propagate errors back to upper level open routine

Signed-off-by: Jeff Garzik <[email protected]>

---

drivers/net/wan/pc300_drv.c | 24 +++++++++++++++++++-----
1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wan/pc300_drv.c b/drivers/net/wan/pc300_drv.c
index 5823e3b..2ae3776 100644
--- a/drivers/net/wan/pc300_drv.c
+++ b/drivers/net/wan/pc300_drv.c
@@ -2867,7 +2867,6 @@ static int ch_config(pc300dev_t * d)
uclong clktype = chan->conf.phys_settings.clock_type;
ucshort encoding = chan->conf.proto_settings.encoding;
ucshort parity = chan->conf.proto_settings.parity;
- int tmc, br;
ucchar md0, md2;

/* Reset the channel */
@@ -2940,8 +2939,12 @@ static int ch_config(pc300dev_t * d)
case PC300_RSV:
case PC300_X21:
if (clktype == CLOCK_INT || clktype == CLOCK_TXINT) {
+ int tmc, br;
+
/* Calculate the clkrate parameters */
tmc = clock_rate_calc(clkrate, card->hw.clock, &br);
+ if (tmc < 0)
+ return -EIO;
cpc_writeb(scabase + M_REG(TMCT, ch), tmc);
cpc_writeb(scabase + M_REG(TXS, ch),
(TXS_DTRXC | TXS_IBRG | br));
@@ -3097,14 +3100,16 @@ static int cpc_attach(struct net_device
return 0;
}

-static void cpc_opench(pc300dev_t * d)
+static int cpc_opench(pc300dev_t * d)
{
pc300ch_t *chan = (pc300ch_t *) d->chan;
pc300_t *card = (pc300_t *) chan->card;
- int ch = chan->channel;
+ int ch = chan->channel, rc;
void __iomem *scabase = card->hw.scabase;

- ch_config(d);
+ rc = ch_config(d);
+ if (rc)
+ return rc;

rx_config(d);

@@ -3113,6 +3118,8 @@ static void cpc_opench(pc300dev_t * d)
/* Assert RTS and DTR */
cpc_writeb(scabase + M_REG(CTL, ch),
cpc_readb(scabase + M_REG(CTL, ch)) & ~(CTL_RTS | CTL_DTR));
+
+ return 0;
}

static void cpc_closech(pc300dev_t * d)
@@ -3168,9 +3175,16 @@ #endif
}

sprintf(ifr.ifr_name, "%s", dev->name);
- cpc_opench(d);
+ result = cpc_opench(d);
+ if (result)
+ goto err_out;
+
netif_start_queue(dev);
return 0;
+
+err_out:
+ hdlc_close(dev);
+ return result;
}

static int cpc_close(struct net_device *dev)

2006-10-11 12:24:16

by Krzysztof Halasa

Subject: Re: [PATCH] WAN/pc300: handle, propagate minor errors

Hi,

Jeff Garzik <[email protected]> writes:

> - move definition of 'tmc' and 'br' locals closer to usage
>
> - handle clock_rate_calc() error
>
> - propagate errors back to upper level open routine

> drivers/net/wan/pc300_drv.c | 24 +++++++++++++++++++-----

Looks good (not sure if my ACK counts, I'm not the maintainer).


FYI: I think the pc300 driver would benefit from, I'd say, a bit
of maintenance. Cyclades don't sell PC300 anymore (at least their
WWW doesn't list them) and I don't know if they're going to touch
the driver.

I have a much simpler driver for PC300 X.21 and V.24/V.35 models
(could be included in the official kernel), but it lacks support
for T1/E1 cards.

I could look at their driver and try to incorporate T1/E1 support
into my driver, but without access to the hardware it would be
tricky at best.

Not sure what should we do. Perhaps waiting for Cyclades is the
only option.

PC300 are nice cards, it would be sad if the driver support
deteriorated.
--
Krzysztof Halasa