Wolfgang Grandegger wrote:
> christian pellegrin wrote:
>> Thanks to Paul we discovered the problem: in the socketcan version
>> with SYSFS enabled the set_bittiming is called during open.
> 
> Ah, right, I already forgot :-(.
> 
>> Unfortunately it doesn't happen without SYSFS or in the netdev-next
>> version. Here is the patch that fixes this (the state restore stuff is
>> needed because the mcp251x is usually kept in sleep mode to avoid
>> power consumption (let's be green! ;-) ))
>>
>> Should I post it to netdev-next too?
> 
> Yes, please.
> 
>> Signed-off-by: Christian Pellegrin <[email protected]>
>>
>> ---
>> Index: drivers/net/can/mcp251x.c
>> ===================================================================
>> --- drivers/net/can/mcp251x.c        (revision 1082)
>> +++ drivers/net/can/mcp251x.c        (working copy)
>> @@ -581,7 +581,13 @@
>>      struct mcp251x_priv *priv = netdev_priv(net);
>>      struct can_bittiming *bt = &priv->can.bittiming;
>>      struct spi_device *spi = priv->spi;
>> +    u8 state;
>>
>> +    /* Store original mode and set mode to config */
>> +    state = mcp251x_read_reg(spi, CANSTAT) & CANCTRL_REQOP_MASK;
>> +    mcp251x_write_bits(spi, CANCTRL, CANCTRL_REQOP_MASK,
>> +                       CANCTRL_REQOP_CONF);
>> +
>>      mcp251x_write_reg(spi, CNF1, ((bt->sjw - 1) << CNF1_SJW_SHIFT) |
>>                        (bt->brp - 1));
>>      mcp251x_write_reg(spi, CNF2, CNF2_BTLMODE |
>> @@ -596,6 +602,9 @@
>>               mcp251x_read_reg(spi, CNF2),
>>               mcp251x_read_reg(spi, CNF3));
>>
>> +    /* Restore original state */
>> +    mcp251x_write_bits(spi, CANCTRL, CANCTRL_REQOP_MASK, state);
>> +
>>      return 0;
>>  }
>>
>> @@ -610,6 +619,8 @@
>>              return ret;
>>      }
>>
>> +    mcp251x_do_set_bittiming(net);
>> +
>>      /* Enable RX0->RX1 buffer roll over and disable filters */
>>      mcp251x_write_bits(spi, RXBCTRL(0),
>>                         RXBCTRL_BUKT | RXBCTRL_RXM0 | RXBCTRL_RXM1,
>> @@ -1027,7 +1038,15 @@
>>      SET_NETDEV_DEV(net, &spi->dev);
>>
> 
> I think you should then also remove the line:
> 
>   priv->can.do_set_bittiming = mcp251x_do_set_bittiming;

I forgot to mention that open_candev() shall be called first in
mcp251x_open(). And call close_candev in case of an error. You patch
does not fix that.

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

Reply via email to