Hi Marc,
On 05/19/2010 08:16 PM, Marc Kleine-Budde wrote:
> Hello,
>
> I just notided that the "ctrlmode" variable is handled in most drivers
> only in "set_bittiming" function, here the function from the sja:
>
>> static int sja1000_set_bittiming(struct net_device *dev)
>> {
>> struct sja1000_priv *priv = netdev_priv(dev);
>> struct can_bittiming *bt = &priv->can.bittiming;
>> u8 btr0, btr1;
>>
>> btr0 = ((bt->brp - 1) & 0x3f) | (((bt->sjw - 1) & 0x3) << 6);
>> btr1 = ((bt->prop_seg + bt->phase_seg1 - 1) & 0xf) |
>> (((bt->phase_seg2 - 1) & 0x7) << 4);
>> if (priv->can.ctrlmode & CAN_CTRLMODE_3_SAMPLES)
>> btr1 |= 0x80;
>>
>> dev_info(dev->dev.parent,
>> "setting BTR0=0x%02x BTR1=0x%02x\n", btr0, btr1);
>>
>> priv->write_reg(priv, REG_BTR0, btr0);
>> priv->write_reg(priv, REG_BTR1, btr1);
>>
>> return 0;
>> }
>
> Consider the following sequence:
> $ modprobe sja1000-platform
> $ canconfig can0 bitrate 250000 # using pengutronix's canutils
> $ ifconfig can0 up
>
> # do some testing, with a bitrate of 250000
>
> $ ifconfig can0 down
>
> # decide to activate tripple samling:
>
> $ canconfig can0 ctrlmode triple-sampling on
> $ ifconfig can0
>
> Contrary to the expectation the tripple sampling mode isn't activated.
Ah, I see. That's needs to be fixed.
> All other control modes that are set during bittiming aren't handled as
> well. For e.g. the flexcan (I'm just hacking on) it's the tripple
> sample, listen only and loopback mode.
The latter two are usually set in the open function when the device is
started. See:
http://lxr.linux.no/#linux+v2.6.34/drivers/net/can/mcp251x.c#L506
> Thinking a bit about the problem...what are the pros and cons of calling
> the do_set_bittiming when the bittiming is changed comapred to call it
> in the "ndo_open" function, e.g. via "open_candev".
Some drivers already do that mainly because it could not be done
earlier, e.g. the MPC512x driver. Just do not set the do_set_bittiming
callback and call the set_bittiming function in the open. Well, there
are not that much cons. Important is that the bittiming parameters are
verified when they are set, but that's already the case.
> This would simplify the flexcan driver, because it must be in a certain
> state (clocks enabled but device in freeze mode) for the bittiming
> configuration. For power saving reasons the clock stay off between
> _probe and _open.
Yep. Maybe it's even more flexible to remove the do_set_bittiming
callback completely.
Wolfgang.
_______________________________________________
Socketcan-core mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/socketcan-core
