Hey, just some short thoughts,....I'll be offline till Monday.
Wolfgang Grandegger wrote:
> 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
hmmm...that if ... else if... else if looks strange...
are the control modes mutually exclusive?
>> 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.
Yes, I think that's a quite good and simple idea. However there's at
least one driver that has special restrictions, IIRC brp >4 for tripple
sampling. The driver has to check in it's open() function then. Then
"ifconfig up" would fail....same goes for "intelligent" can cards with a
limited set of fixed bitrates.
This means canconfig bitrate does succeed, but ifconfig up does fail. We
can rename the callback into "do_check_bittiming"....
I'm not familiar with the netlink interface, but is it possible to set
the bittiming and control mode atomically? Can you call the
"do_X_bittiming" accordingly?
I just noticed, it's already possible to not assign "do_set_bittiming",
it's checked for NULL before calling it. :)
Have a nice weekend,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Socketcan-core mailing list [email protected] https://lists.berlios.de/mailman/listinfo/socketcan-core
