On Fri, Nov 13, 2009 at 01:10:31PM +0100, Wolfgang Grandegger wrote:
> This patch introduces some helper functions to make the code
> more readable, especially the function mscan_rx_poll().
> Unfortunately, there are still various coding style issue
> which should be fixed for kernel inclusion

Which ones?

> 
> Signed-off-by: Wolfgang Grandegger <[email protected]>
> ---
>  kernel/2.6/drivers/net/can/mscan/mscan.c |  176 
> ++++++++++++++++---------------
>  1 file changed, 95 insertions(+), 81 deletions(-)
> 
> Index: trunk/kernel/2.6/drivers/net/can/mscan/mscan.c
> ===================================================================
> --- trunk.orig/kernel/2.6/drivers/net/can/mscan/mscan.c
> +++ trunk/kernel/2.6/drivers/net/can/mscan/mscan.c
> @@ -343,6 +343,94 @@ static inline int check_set_state(struct
>       return ret;
>  }
>  
> +static inline void mscan_get_rx_frame(struct net_device *dev,
> +                                   struct can_frame *frame)

inline is discouraged. There is a paragraph in CodingStyle about it.

> +{
> +     struct mscan_regs *regs = (struct mscan_regs *)dev->base_addr;
> +     u32 can_id;
> +     int i;
> +
> +     can_id = in_be16(&regs->rx.idr1_0);
> +     if (can_id & (1 << 3)) {
> +             frame->can_id = CAN_EFF_FLAG;
> +             can_id = ((can_id << 16) | in_be16(&regs->rx.idr3_2));
> +             can_id = ((can_id & 0xffe00000) |
> +                       ((can_id & 0x7ffff) << 2)) >> 2;
> +     } else {
> +             can_id >>= 4;
> +             frame->can_id = 0;
> +     }
> +
> +     frame->can_id |= can_id >> 1;
> +     if (can_id & 1)
> +             frame->can_id |= CAN_RTR_FLAG;
> +     frame->can_dlc = in_8(&regs->rx.dlr) & 0xf;
> +
> +     if (!(frame->can_id & CAN_RTR_FLAG)) {
> +             void __iomem *data = &regs->rx.dsr1_0;
> +             u16 *payload = (u16 *) frame->data;
> +             for (i = 0; i < (frame->can_dlc + 1) / 2; i++) {
> +                     *payload++ = in_be16(data);
> +                     data += 2 + _MSCAN_RESERVED_DSR_SIZE;
> +             }
> +     }
> +
> +     out_8(&regs->canrflg, MSCAN_RXF);
> +}
> +
> +static inline void mscan_get_err_frame(struct net_device *dev,
> +                                    struct can_frame *frame, u8 canrflg)
> +{
> +     struct mscan_regs *regs = (struct mscan_regs *)dev->base_addr;
> +#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,23)
> +     struct net_device_stats *stats = can_get_stats(dev);
> +#else
> +     struct net_device_stats *stats = &dev->stats;
> +#endif
> +     struct mscan_priv *priv = netdev_priv(dev);
> +
> +     dev_dbg(ND2D(dev), "error interrupt (canrflg=%#x)\n", canrflg);
> +     frame->can_id = CAN_ERR_FLAG;
> +
> +     if (canrflg & MSCAN_OVRIF) {
> +             frame->can_id |= CAN_ERR_CRTL;
> +             frame->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
> +             stats->rx_over_errors++;
> +             stats->rx_errors++;
> +     } else {
> +             frame->data[1] = 0;
> +     }
> +
> +     if (check_set_state(dev, canrflg)) {
> +             switch (priv->can.state) {
> +             case CAN_STATE_ERROR_WARNING:
> +                     frame->can_id |= CAN_ERR_CRTL;
> +                     priv->can.can_stats.error_warning++;
> +                     if ((priv->shadow_statflg & MSCAN_RSTAT_MSK) <
> +                         (canrflg & MSCAN_RSTAT_MSK))
> +                             frame->data[1] |= CAN_ERR_CRTL_RX_WARNING;
> +                     if ((priv->shadow_statflg & MSCAN_TSTAT_MSK) <
> +                         (canrflg & MSCAN_TSTAT_MSK))
> +                             frame->data[1] |= CAN_ERR_CRTL_TX_WARNING;
> +                     break;
> +             case CAN_STATE_ERROR_PASSIVE:
> +                     frame->can_id |= CAN_ERR_CRTL;
> +                     priv->can.can_stats.error_passive++;
> +                     frame->data[1] |= CAN_ERR_CRTL_RX_PASSIVE;
> +                     break;
> +             case CAN_STATE_BUS_OFF:
> +                     frame->can_id |= CAN_ERR_BUSOFF;
> +                     can_bus_off(dev);
> +                     break;
> +             default:
> +                     break;
> +             }
> +     }
> +     priv->shadow_statflg = canrflg & MSCAN_STAT_MSK;
> +     frame->can_dlc = CAN_ERR_DLC;
> +     out_8(&regs->canrflg, MSCAN_ERR_IF);
> +}
> +
>  #if LINUX_VERSION_CODE > KERNEL_VERSION(2,6,23)
>  static int mscan_rx_poll(struct napi_struct *napi, int quota)
>  #else
> @@ -369,9 +457,7 @@ static int mscan_rx_poll(struct net_devi
>       int ret = 1;
>       struct sk_buff *skb;
>       struct can_frame *frame;
> -     u32 can_id;
>       u8 canrflg;
> -     int i;
>  
>       while (npackets < quota && ((canrflg = in_8(&regs->canrflg)) &
>                                   (MSCAN_RXF | MSCAN_ERR_IF))) {
> @@ -385,88 +471,16 @@ static int mscan_rx_poll(struct net_devi
>                       continue;
>               }
>  
> -             if (canrflg & MSCAN_RXF) {
> -                     can_id = in_be16(&regs->rx.idr1_0);
> -                     if (can_id & (1 << 3)) {
> -                             frame->can_id = CAN_EFF_FLAG;
> -                             can_id = ((can_id << 16) |
> -                                       in_be16(&regs->rx.idr3_2));
> -                             can_id = ((can_id & 0xffe00000) |
> -                                       ((can_id & 0x7ffff) << 2)) >> 2;
> -                     } else {
> -                             can_id >>= 4;
> -                             frame->can_id = 0;
> -                     }
> -
> -                     frame->can_id |= can_id >> 1;
> -                     if (can_id & 1)
> -                             frame->can_id |= CAN_RTR_FLAG;
> -                     frame->can_dlc = in_8(&regs->rx.dlr) & 0xf;
> -
> -                     if (!(frame->can_id & CAN_RTR_FLAG)) {
> -                             void __iomem *data = &regs->rx.dsr1_0;
> -                             u16 *payload = (u16 *) frame->data;
> -                             for (i = 0; i < (frame->can_dlc + 1) / 2; i++) {
> -                                     *payload++ = in_be16(data);
> -                                     data += 2 + _MSCAN_RESERVED_DSR_SIZE;
> -                             }
> -                     }
> +             if (canrflg & MSCAN_RXF)
> +                     mscan_get_rx_frame(dev, frame);
> +             else if (canrflg & MSCAN_ERR_IF)
> +                     mscan_get_err_frame(dev, frame, canrflg);
>  
> -                     out_8(&regs->canrflg, MSCAN_RXF);
>  #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,32)
> -                     dev->last_rx = jiffies;
> +             dev->last_rx = jiffies;
>  #endif
> -                     stats->rx_packets++;
> -                     stats->rx_bytes += frame->can_dlc;
> -             } else if (canrflg & MSCAN_ERR_IF) {
> -                     dev_dbg(ND2D(dev), "error interrupt (canrflg=%#x)\n",
> -                             canrflg);
> -                     frame->can_id = CAN_ERR_FLAG;
> -
> -                     if (canrflg & MSCAN_OVRIF) {
> -                             frame->can_id |= CAN_ERR_CRTL;
> -                             frame->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
> -                             stats->rx_over_errors++;
> -                             stats->rx_errors++;
> -                     } else
> -                             frame->data[1] = 0;
> -
> -                     if (check_set_state(dev, canrflg)) {
> -                             switch (priv->can.state) {
> -                             case CAN_STATE_ERROR_WARNING:
> -                                     frame->can_id |= CAN_ERR_CRTL;
> -                                     priv->can.can_stats.error_warning++;
> -                                     if ((priv->shadow_statflg &
> -                                          MSCAN_RSTAT_MSK) <
> -                                         (canrflg & MSCAN_RSTAT_MSK))
> -                                             frame->data[1] |=
> -                                                     CAN_ERR_CRTL_RX_WARNING;
> -
> -                                     if ((priv->shadow_statflg &
> -                                          MSCAN_TSTAT_MSK) <
> -                                         (canrflg & MSCAN_TSTAT_MSK))
> -                                             frame->data[1] |=
> -                                                     CAN_ERR_CRTL_TX_WARNING;
> -                                     break;
> -                             case CAN_STATE_ERROR_PASSIVE:
> -                                     frame->can_id |= CAN_ERR_CRTL;
> -                                     priv->can.can_stats.error_passive++;
> -                                     frame->data[1] |=
> -                                             CAN_ERR_CRTL_RX_PASSIVE;
> -                                     break;
> -                             case CAN_STATE_BUS_OFF:
> -                                     frame->can_id |= CAN_ERR_BUSOFF;
> -                                     can_bus_off(dev);
> -                                     break;
> -                             default:
> -                                     break;
> -                             }
> -                     }
> -                     priv->shadow_statflg = canrflg & MSCAN_STAT_MSK;
> -                     frame->can_dlc = CAN_ERR_DLC;
> -                     out_8(&regs->canrflg, MSCAN_ERR_IF);
> -             }
> -
> +             stats->rx_packets++;
> +             stats->rx_bytes += frame->can_dlc;
>               npackets++;
>               netif_receive_skb(skb);
>       }

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Attachment: signature.asc
Description: Digital signature

_______________________________________________
Socketcan-core mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/socketcan-core

Reply via email to