lkml.org 
[lkml]   [2010]   [Nov]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH net-next-2.6 v2] can: Topcliff: PCH_CAN driver: Add Flow control,
    Hello,

    On 11/15/2010 09:30 AM, Tomoya MORINAGA wrote:
    > * Add Flow control
    > * Fix Data copy issue (endianness)
    > * Add Macro prefix "PCH_"
    > * Separate interface register structure
    > * Some functions are unified.
    > * Change MessageObject indication(PCH_RX_OBJ_START, etc..)
    > * Enumerate LEC macro
    > * Move MSI processing from open/close to probe/remove processing
    > * Use BIT(x)
    > * and more...
    >
    > Signed-off-by: Tomoya MORINAGA <tomoya-linux@dsn.okisemi.com>
    > ---
    > drivers/net/can/pch_can.c | 1348 ++++++++++++++++++++-------------------------
    > 1 files changed, 595 insertions(+), 753 deletions(-)
    >
    > diff --git a/drivers/net/can/pch_can.c b/drivers/net/can/pch_can.c
    > index 6727182..6a38593 100644
    > --- a/drivers/net/can/pch_can.c
    > +++ b/drivers/net/can/pch_can.c
    ...
    > - if (status & PCH_LEC_ALL) {
    > + lec = status & PCH_LEC_ALL;
    > + switch (lec) {
    > + case PCH_STUF_ERR:
    > + cf->data[2] |= CAN_ERR_PROT_STUFF;
    > priv->can.can_stats.bus_error++;
    > stats->rx_errors++;
    > - switch (status & PCH_LEC_ALL) {
    > - case PCH_STUF_ERR:
    > - cf->data[2] |= CAN_ERR_PROT_STUFF;
    > - break;
    > - case PCH_FORM_ERR:
    > - cf->data[2] |= CAN_ERR_PROT_FORM;
    > - break;
    > - case PCH_ACK_ERR:
    > - cf->data[2] |= CAN_ERR_PROT_LOC_ACK |
    > - CAN_ERR_PROT_LOC_ACK_DEL;
    > - break;
    > - case PCH_BIT1_ERR:
    > - case PCH_BIT0_ERR:
    > - cf->data[2] |= CAN_ERR_PROT_BIT;
    > - break;
    > - case PCH_CRC_ERR:
    > - cf->data[2] |= CAN_ERR_PROT_LOC_CRC_SEQ |
    > - CAN_ERR_PROT_LOC_CRC_DEL;
    > - break;
    > - default:
    > - iowrite32(status | PCH_LEC_ALL, &priv->regs->stat);
    > - break;
    > - }
    > -
    > + break;
    > + case PCH_FORM_ERR:
    > + cf->data[2] |= CAN_ERR_PROT_FORM;
    > + priv->can.can_stats.bus_error++;
    > + stats->rx_errors++;
    > + break;
    > + case PCH_ACK_ERR:
    > + cf->can_id |= CAN_ERR_ACK;
    > + priv->can.can_stats.bus_error++;
    > + stats->rx_errors++;
    > + break;
    > + case PCH_BIT1_ERR:
    > + case PCH_BIT0_ERR:
    > + cf->data[2] |= CAN_ERR_PROT_BIT;
    > + priv->can.can_stats.bus_error++;
    > + stats->rx_errors++;
    > + break;
    > + case PCH_CRC_ERR:
    > + cf->data[2] |= CAN_ERR_PROT_LOC_CRC_SEQ |
    > + CAN_ERR_PROT_LOC_CRC_DEL;
    > + priv->can.can_stats.bus_error++;
    > + stats->rx_errors++;
    > + break;
    > + case PCH_LEC_ALL: /* Written by CPU. No error status */
    > + break;
    > }

    More comments to the lec handling below.

    > + cf->data[6] = ioread32(&priv->regs->errc) & PCH_TEC;
    > + cf->data[7] = (ioread32(&priv->regs->errc) & PCH_REC) >> 8;

    Could be handle with just *one* register access.

    ...
    > static int pch_can_rx_poll(struct napi_struct *napi, int quota)
    > {
    > struct net_device *ndev = napi->dev;
    > struct pch_can_priv *priv = netdev_priv(ndev);
    > - struct net_device_stats *stats = &(priv->ndev->stats);
    > - u32 dlc;
    > u32 int_stat;
    > int rcv_pkts = 0;
    > u32 reg_stat;
    > - unsigned long flags;
    >
    > int_stat = pch_can_int_pending(priv);
    > if (!int_stat)
    > - return 0;
    > + goto end;
    >
    > -INT_STAT:
    > - if (int_stat == CAN_STATUS_INT) {
    > + if ((int_stat == PCH_STATUS_INT) && (quota > 0)) {
    > reg_stat = ioread32(&priv->regs->stat);
    > if (reg_stat & (PCH_BUS_OFF | PCH_LEC_ALL)) {
    > - if ((reg_stat & PCH_LEC_ALL) != PCH_LEC_ALL)
    > + if ((reg_stat & PCH_LEC_ALL) != PCH_LEC_ALL) {
    > pch_can_error(ndev, reg_stat);
    > + quota--;
    > + }

    Should be:

    if (reg_stat & PCH_BUS_OFF ||
    (reg_stat & PCH_LEC_ALL) != PCH_LEC_ALL) {

    Your lec handling is still not correc, I believe. The driver needs to
    write PCH_LEC_ALL to the "stat" register once in the initialization code
    and then after each error observed (lec != PCH_LEC_ALL). I still do not
    find such code. Could you show us the output of

    "# candump any,0:0,#FFFFFFFF"

    when yo send CAN messages *without* a cable connected?.

    Thanks,

    Wolfgang.


    \
     
     \ /
      Last update: 2010-11-15 15:23    [W:0.027 / U:29.636 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site