> > > + reset_test = ccan_read_reg_8bit(priv, CCAN_CFG_STAT); > > > + > > > + if (!(reset_test & CCAN_RST_MASK)) { > > > + netdev_alert(ndev, "Not in reset mode, cannot set bit > > timing\n"); > > > + return -EPERM; > > > + } > > > > > > You don't see nedev_alert() used very often. If this is fatal then netdev_err(). > > > > Also, EPERM? man 3 errno say: > > > > EPERM Operation not permitted (POSIX.1-2001). > > > > Why is this a permission issue? > > Will use netdev_err() and return -EWOULDBLOCK instead. I'm not sure that is any better. EAGAIN Resource temporarily unavailable (may be the same value as EWOULDBLOCK) (POSIX.1-2001). This is generally used when the kernel expects user space to try a system call again, and it might then work. Is that what you expect here? > > > +static irqreturn_t ccan_interrupt(int irq, void *dev_id) { > > > + struct net_device *ndev = (struct net_device *)dev_id; > > > > dev_id is a void *, so you don't need the cast. > > OK, drop it. Please look at the whole patch. There might be other instances where a void * is used with a cast, which can be removed. This was just the first i spotted. Andrew