Hi Kurt, On Fri, Oct 02, 2009 at 05:28:56PM +0200, Kurt Van Dijck wrote: > This part does the generic sja1000 hook. > I used 'static inline' functions to keep the open_candev/close_candev > functions readable. > > Signed-off-by: Kurt Van Dijck <[email protected]> > --- > Index: drivers/net/can/sja1000/sja1000.h > =================================================================== > --- drivers/net/can/sja1000/sja1000.h (revision 1066) > +++ drivers/net/can/sja1000/sja1000.h (working copy) > @@ -171,6 +171,14 @@ > u16 flags; /* custom mode flags */ > u8 ocr; /* output control register */ > u8 cdr; /* clock divider register */ > + > + /*
Should have two asterisks if it is meant as nanodoc.
> + * enable function pointer:
> + * @dev : the device, parent of the net_device
> + * @active : 1 = enable, 0 = disable
> + * @returns : 0 or -error
> + */
> + int (*enable)(struct device *dev, int active);
> };
>
> struct net_device *alloc_sja1000dev(int sizeof_priv);
> Index: drivers/net/can/sja1000/sja1000.c
> ===================================================================
> --- drivers/net/can/sja1000/sja1000.c (revision 1066)
> +++ drivers/net/can/sja1000/sja1000.c (working copy)
> @@ -541,6 +541,24 @@
> }
> EXPORT_SYMBOL_GPL(sja1000_interrupt);
>
> +static inline int sja1000_ext_enable(struct net_device *dev)
> +{
> + struct sja1000_priv *priv = netdev_priv(dev);
> +
> + if (!priv->enable)
> + return 0;
Isn't an errorcode more apropriate?
> + return priv->enable(dev->dev.parent, 1);
> +}
> +
> +static inline void sja1000_ext_disable(struct net_device *dev)
> +{
> + struct sja1000_priv *priv = netdev_priv(dev);
> +
> + if (!priv->enable)
> + return;
> + priv->enable(dev->dev.parent, 0);
Invert the if-logic and save a line here.
> +}
> +
> static int sja1000_open(struct net_device *dev)
> {
> struct sja1000_priv *priv = netdev_priv(dev);
> @@ -549,10 +567,17 @@
> /* set chip into reset mode */
> set_reset_mode(dev);
>
> + /* do external initialization */
> + err = sja1000_ext_enable(dev);
> + if (err)
> + return err;
> +
> /* common open */
> err = open_candev(dev);
> - if (err)
> + if (err) {
> + sja1000_ext_disable(dev);
> return err;
> + }
disable is void.
>
> /* register interrupt handler, if not done by the device driver */
> if (!(priv->flags & SJA1000_CUSTOM_IRQ_HANDLER)) {
> @@ -560,6 +585,7 @@
> dev->name, (void *)dev);
> if (err) {
> close_candev(dev);
> + sja1000_ext_disable(dev);
> return -EAGAIN;
> }
> }
> @@ -589,6 +615,7 @@
> free_irq(dev->irq, (void *)dev);
>
> close_candev(dev);
> + sja1000_ext_disable(dev);
>
> priv->open_time = 0;
>
> _______________________________________________
> Socketcan-core mailing list
> [email protected]
> https://lists.berlios.de/mailman/listinfo/socketcan-core
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
signature.asc
Description: Digital signature
_______________________________________________ Socketcan-core mailing list [email protected] https://lists.berlios.de/mailman/listinfo/socketcan-core
