| Message ID | 4E7B0DE6.9020807@hartkopp.net |
|---|---|
| State | Superseded, archived |
| Delegated to: | David Miller |
| Headers | show |
Hi Oliver, On 09/22/2011 12:28 PM, Oliver Hartkopp wrote: > This patch adds support for SJW user settings to not set the synchronization > jump width (SJW) to 1 in any case when using the in-kernel bittiming > calculation. > > The ip-tool from iproute2 already supports to pass the user defined SJW > value. The given SJW value is sanitized with the controller specific sjw_max > and the calculated tseg2 value. As the SJW can have values up to 4 providing > this value will lead to the maximum possible SJW automatically. A higher SJW > allows higher controller oscillator tolerances. > > Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net> > > --- > > v2: Resend due to wrong mail address header encoding. > > diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c > index 9bf1116..25695bd 100644 > --- a/drivers/net/can/dev.c > +++ b/drivers/net/can/dev.c > @@ -150,7 +150,19 @@ static int can_calc_bittiming(struct net_device *dev, > struct can_bittiming *bt) > bt->prop_seg = tseg1 / 2; > bt->phase_seg1 = tseg1 - bt->prop_seg; > bt->phase_seg2 = tseg2; > - bt->sjw = 1; > + > + /* check for sjw user settings */ > + if (!bt->sjw || !btc->sjw_max) > + bt->sjw = 1; > + else { > + /* bt->sjw is at least 1 -> sanitize upper bound to sjw_max */ > + if (bt->sjw > btc->sjw_max) > + bt->sjw = btc->sjw_max; > + /* bt->sjw must not be higher than tseg2 */ > + if (tseg2 < bt->sjw) > + bt->sjw = tseg2; > + } > + As I already said, I expected "bt->sjw" to be always 0 when can_calc_bittiming() is called. But the software somehow allows to preset "bt->sjw", which is not intended as the help of the ip utility shows: $ ip link set can0 type can help Usage: ip link set DEVICE type can [ bitrate BITRATE [ sample-point SAMPLE-POINT] ] | [ tq TQ prop-seg PROP_SEG phase-seg1 PHASE-SEG1 phase-seg2 PHASE-SEG2 [ sjw SJW ] ] I actually hesitate to extend can_calc_bittiming(). I suggest using an external tool to calculate proper bit-timing parameters and set them with "ip link set DEVICE type can tq ...". See also: http://lxr.linux.no/#linux+v3.0.4/Documentation/networking/can.txt#L742 Wolfgang. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/22/11 15:07, Wolfgang Grandegger wrote: > Hi Oliver, > > On 09/22/2011 12:28 PM, Oliver Hartkopp wrote: >> This patch adds support for SJW user settings to not set the synchronization >> jump width (SJW) to 1 in any case when using the in-kernel bittiming >> calculation. >> >> The ip-tool from iproute2 already supports to pass the user defined SJW >> value. The given SJW value is sanitized with the controller specific sjw_max >> and the calculated tseg2 value. As the SJW can have values up to 4 providing >> this value will lead to the maximum possible SJW automatically. A higher SJW >> allows higher controller oscillator tolerances. (..) > As I already said, I expected "bt->sjw" to be always 0 when > can_calc_bittiming() is called. But the software somehow allows to > preset "bt->sjw", which is not intended as the help of the ip utility shows: What was the technical reason for this 'not intended' that time? > I actually hesitate to extend can_calc_bittiming(). I suggest using an > external tool to calculate proper bit-timing parameters and set them > with "ip link set DEVICE type can tq ...". Yes, i know the possibility to specify the bittiming via time-quanta (tq). But i think the in-kernel bittiming calculation is pretty good and solves most usual cases. For me it's just an additional requirement to specify SJW as MIN(tseg2, max_sjw) to extend the clock source tolerance - which can be provided as an additional option to the bittiming calculation without pain (IMO). If i can 'tune' the sample point, why should i not have any influence to the synchronization jump width generation then? And btw. a help text can really be changed easily ;-) - [ bitrate BITRATE [ sample-point SAMPLE-POINT] ] | + [ bitrate BITRATE [ sample-point SAMPLE-POINT] [sjw SJW] ] | Regards, Oliver -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/22/2011 06:26 PM, Oliver Hartkopp wrote: > On 09/22/11 15:07, Wolfgang Grandegger wrote: > >> Hi Oliver, >> >> On 09/22/2011 12:28 PM, Oliver Hartkopp wrote: >>> This patch adds support for SJW user settings to not set the synchronization >>> jump width (SJW) to 1 in any case when using the in-kernel bittiming >>> calculation. >>> >>> The ip-tool from iproute2 already supports to pass the user defined SJW >>> value. The given SJW value is sanitized with the controller specific sjw_max >>> and the calculated tseg2 value. As the SJW can have values up to 4 providing >>> this value will lead to the maximum possible SJW automatically. A higher SJW >>> allows higher controller oscillator tolerances. > > (..) > > >> As I already said, I expected "bt->sjw" to be always 0 when >> can_calc_bittiming() is called. But the software somehow allows to >> preset "bt->sjw", which is not intended as the help of the ip utility shows: > > > What was the technical reason for this 'not intended' that time? Fiddling with SJW is something for *expert* only. You need to know what's going on the bus (electrically). To be clear, I'm not such an expert. >> I actually hesitate to extend can_calc_bittiming(). I suggest using an >> external tool to calculate proper bit-timing parameters and set them >> with "ip link set DEVICE type can tq ...". > > > Yes, i know the possibility to specify the bittiming via time-quanta (tq). This is what we called the "expert mode". > But i think the in-kernel bittiming calculation is pretty good and solves most > usual cases. For me it's just an additional requirement to specify SJW as What's your reason to increase SJW? Likely because you get bus errors with the standard settings. How should it be set? Likely you need to fiddle with other parameters as well. For sure, no business for a normal CAN user. > MIN(tseg2, max_sjw) to extend the clock source tolerance - which can be > provided as an additional option to the bittiming calculation without pain > (IMO). If i can 'tune' the sample point, why should i not have any influence > to the synchronization jump width generation then? Bitrate and sample point is something obvious for normal users. > And btw. a help text can really be changed easily ;-) > > - [ bitrate BITRATE [ sample-point SAMPLE-POINT] ] | > + [ bitrate BITRATE [ sample-point SAMPLE-POINT] [sjw SJW] ] | Well, I have stronger arguments against it ;-). Any other opinions or suggestions? Wolfgang. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Wolfgang, i put Pavel and Andrey into CC, maybe they also have an opinion about this. I just talked to a specialist who told me, that a higher SJW is better - if possible. And if an expert told me so and i had a simple config interface to fullfill it, it would be great for me ;-) Here is a link to the patch and this discussion: http://patchwork.ozlabs.org/patch/115932/ Regards, Oliver On 09/22/11 20:10, Wolfgang Grandegger wrote: > On 09/22/2011 06:26 PM, Oliver Hartkopp wrote: >> On 09/22/11 15:07, Wolfgang Grandegger wrote: >> >>> Hi Oliver, >>> >>> On 09/22/2011 12:28 PM, Oliver Hartkopp wrote: >>>> This patch adds support for SJW user settings to not set the synchronization >>>> jump width (SJW) to 1 in any case when using the in-kernel bittiming >>>> calculation. >>>> >>>> The ip-tool from iproute2 already supports to pass the user defined SJW >>>> value. The given SJW value is sanitized with the controller specific sjw_max >>>> and the calculated tseg2 value. As the SJW can have values up to 4 providing >>>> this value will lead to the maximum possible SJW automatically. A higher SJW >>>> allows higher controller oscillator tolerances. >> >> (..) >> >> >>> As I already said, I expected "bt->sjw" to be always 0 when >>> can_calc_bittiming() is called. But the software somehow allows to >>> preset "bt->sjw", which is not intended as the help of the ip utility shows: >> >> >> What was the technical reason for this 'not intended' that time? > > Fiddling with SJW is something for *expert* only. You need to know > what's going on the bus (electrically). To be clear, I'm not such an expert. > >>> I actually hesitate to extend can_calc_bittiming(). I suggest using an >>> external tool to calculate proper bit-timing parameters and set them >>> with "ip link set DEVICE type can tq ...". >> >> >> Yes, i know the possibility to specify the bittiming via time-quanta (tq). > > This is what we called the "expert mode". > >> But i think the in-kernel bittiming calculation is pretty good and solves most >> usual cases. For me it's just an additional requirement to specify SJW as > > What's your reason to increase SJW? Likely because you get bus errors > with the standard settings. How should it be set? Likely you need to > fiddle with other parameters as well. For sure, no business for a normal > CAN user. > >> MIN(tseg2, max_sjw) to extend the clock source tolerance - which can be >> provided as an additional option to the bittiming calculation without pain >> (IMO). If i can 'tune' the sample point, why should i not have any influence >> to the synchronization jump width generation then? > > Bitrate and sample point is something obvious for normal users. > >> And btw. a help text can really be changed easily ;-) >> >> - [ bitrate BITRATE [ sample-point SAMPLE-POINT] ] | >> + [ bitrate BITRATE [ sample-point SAMPLE-POINT] [sjw SJW] ] | > > Well, I have stronger arguments against it ;-). > > Any other opinions or suggestions? > > Wolfgang. > > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Oliver, On 09/22/2011 09:41 PM, Oliver Hartkopp wrote: > Hi Wolfgang, > > i put Pavel and Andrey into CC, maybe they also have an opinion about this. > I just talked to a specialist who told me, that a higher SJW is better - if Then let us set it to 4 (maximum), by default. But other documents recommend a value of 1. > possible. And if an expert told me so and i had a simple config interface to > fullfill it, it would be great for me ;-) Anyway, if SJW does not depend on other bit-timing parameters and it usually works fine with the kernel calculated parameters I would no longer vote against this patch. Wolfgang, -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello Oliver and Wolfgang, On Friday 23 September 2011 09:24:20 Wolfgang Grandegger wrote: > Hi Oliver, > > On 09/22/2011 09:41 PM, Oliver Hartkopp wrote: > Then let us set it to 4 (maximum), by default. But other documents > recommend a value of 1. I am not expert for CAN timing nor I have time to go through documentation at this moment. But I hope that I have some sense/experience for electronic and dynamic systems and we have done more designs utilizing CAN at university and at our company. So there are some thought about SJW based on my experience. There could be some inaccuracies, if you want better analysis, I need time for that. 1) The CAN needs fast local roundrip time for correct operation of dominant/recessive arbitration. The time is sum of propagation from CAN controller chip, propagation delay of optocoupler or other galvanic isolation, slew rate and delay in Tx circuitry of transceiver, charging the wire, Rx part of receiver, optocoupler, controller Rx filtering and Rx,Tx logic level comparator. Iw we count necessity to synchronize this between multiple CAN nodes then whole roundtrip time has to be smaller than 50-80% of single bit bit time quantum. 2) From the noise and stabilization of voltage on the wire the sampling point should be in the middle of bit pulse delayed by round-trip delay from 1. This is 50% of bit time (i.e. dependent on bitrate) + round trip delay (dependent on delay in controller and concrete board circuitry). 3) But sampling point has to allow to decide about collision before next bit Tx is started => it cannot be moved after end of the given Tx bit time interval. 4) It is necessary to synchronize bit timing in all CAN controllers during arbitration (ID sending) phase. The first sync is done after receiving of the first edge on the wire. It can be other node start of Tx or our own Tx dominant level. Controller shifts its Tx timing such way, that start of the next Tx bit interval should result in Rx transition at the end the sensed Rx bit. I.e. it counts only TSEG2 til next bit start. 5) There are some more things to consider if triple sampling and filtering is enabled to suppress noise on wire. It can be used only, if time quantum clocks are fast enough to fit this sampling interval between stabilization of delayed Rx logic level and end of Tx bit time interval. 6) If the clock frequencies of all nodes CAN controllers are guaranteed to be same, then no more synchronization is necessary during message Tx/Rx (SJW=0). But that assumption does not hold. That is why bitstuffing is used and guarantees that there is at least one logic level transition (edge) after each 6 bit time. If there is zero roundtrip propagation delay and sampling in 50% of bit time interval then maximal skew/frequency difference of the clocks could be (1+1/6*50%) - 1 i.e. 8% This means, that SJW bigger than 8% of whole bit time would not help to synchronize bitrate difference, because for such case setup cannot work anyway. Propagation delay is not zero then there is even less time left for sampling point shift which would not cause incorrect bit data detection so reasonable maximum is probably lower. I expect that for reasonable precise setup of bitrate when exact ratio is found and crystal based oscillators there is the best option small SJW i.e. 1 or for very fast TQ clock equivalent of 1% - 2% of bittime. For nonexact ratio or low quality clocks sources, bigger SJW values make sense. But big value has other disadvantage. If there is bigger jitter in Tx/Rx delays or clocks then it is propagated back to bit timing and stability of system composed from multiple nodes could be questionable. There is even bigger interval where noise pulse could cause lost of the synchronization. On base of above analysis, I think that blindly set SJW on maximum is not good idea. It should be at least limited to 5% of bit time. Because bit timing calculation is not what everybody want to do again and again, the actual code tries to hide differences of CAN controllers and provide at least partially understandable knobs to user with parameters independent on concrete setup low level details to allow set bitrate for usual Joe user. The basic parameters are chosen such way, that user need not to care about actual TQ clock and selecting prescaller, that is why sampling point is in percent of bittime. SJW is more problematic, but may it be use of 2 or 5% of bittime by default with assurance that zero is replaced by one, would serve to most people pleasure. May it be, that 0.1 of percent should be used as unit for external parameter too. I hope that actual calculation works reasonably well. But if it should be enhanced, then it would worth to add additional parameter except crystal/controller clock source freqency to the concrete boards drivers. It should be measured round-trip delay of given/used transceiver/optocouplers combination. This would allow to have sampling point setup yet more independent on given HW and same value could be used for different bitrates. But there is still unknown parameter capacity/length of connected wires so there is still something left to user consideration. Best wishes, Pavel Pisa e-mail: pisa@cmp.felk.cvut.cz www: http://cmp.felk.cvut.cz/~pisa university: http://dce.fel.cvut.cz/ company: http://www.pikron.com/ > > possible. And if an expert told me so and i had a simple config interface > > to fullfill it, it would be great for me ;-) > > Anyway, if SJW does not depend on other bit-timing parameters and it > usually works fine with the kernel calculated parameters I would no > longer vote against this patch. > > Wolfgang, -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello Pavel, hello Wolfgang, thanks for the detailed feedback! Trying to summarize the text for me, i see these relevant topics (in my probably limited view): On 09/23/11 11:32, Pavel Pisa wrote: > On Friday 23 September 2011 09:24:20 Wolfgang Grandegger wrote: >> Then let us set it to 4 (maximum), by default. But other documents >> recommend a value of 1. Putting the SJW to it's max value by default - which is then reduced by tseg2 is obviously bad (see below). SJW = 1 should stay the default value. (..) > 6) If the clock frequencies of all nodes CAN controllers > are guaranteed to be same, then no more synchronization > is necessary during message Tx/Rx (SJW=0). But that assumption > does not hold. That is why bitstuffing is used and guarantees > that there is at least one logic level transition (edge) > after each 6 bit time. Ah - i see. (..) > I expect that for reasonable precise setup of bitrate > when exact ratio is found and crystal based oscillators > there is the best option small SJW i.e. 1 or for very > fast TQ clock equivalent of 1% - 2% of bittime. Which already was the default. > For nonexact ratio or low quality clocks sources, > bigger SJW values make sense. But big value has other > disadvantage. If there is bigger jitter in Tx/Rx delays > or clocks then it is propagated back to bit timing > and stability of system composed from multiple nodes > could be questionable. There is even bigger interval > where noise pulse could cause lost of the synchronization. > > On base of above analysis, I think that blindly set SJW > on maximum is not good idea. It should be at least limited > to 5% of bit time. Ok. > Because bit timing calculation is not what everybody > want to do again and again, the actual code tries > to hide differences of CAN controllers and provide > at least partially understandable knobs to user > with parameters independent on concrete setup low level > details to allow set bitrate for usual Joe user. Yes - like me ;-) > The basic parameters are chosen such way, that user need > not to care about actual TQ clock and selecting prescaller, > that is why sampling point is in percent of bittime. Btw. defining a sampling point other than CAN CIA recommendations could be seen as a user specific requirement that is nothing for user Joe too. > SJW is more problematic, but may it be use of 2 or 5% > of bittime by default with assurance that zero is > replaced by one, would serve to most people pleasure. > May it be, that 0.1 of percent should be used as unit > for external parameter too. And then you would like to calculate the SJW based on this percent value? I'm a bit concerned about this additional complexity (see below). > I hope that actual calculation works reasonably well. Yes, it does. > But if it should be enhanced, then it would worth > to add additional parameter except crystal/controller > clock source freqency to the concrete boards drivers. > It should be measured round-trip delay of given/used > transceiver/optocouplers combination. This would > allow to have sampling point setup yet more independent > on given HW and same value could be used for different > bitrates. But there is still unknown parameter > capacity/length of connected wires so there is still > something left to user consideration. As i remember from the discussions for the existing algorithm the complexity is already high enough, so that adding these new details may lead to new problems (in kernel space), we do not see so far. I think, if someone want's to go THAT deep the already existing 'expert mode' with the time-quanta based setting of the bitrate is the right choice. The in-kernel bittiming calculation is the best approach for user Joe as you already pointed out. Besides the sampling point (which has a impact on the bittiming calculation) the tuning of the SJW value would be a second feature for 'advanced' users that can be modified from the outside. As we still preserve the reasonable default of SJW=1, adding a new SJW option to tune just only the resulting SJW value looks risk-free to me. (..) >> Anyway, if SJW does not depend on other bit-timing parameters and it >> usually works fine with the kernel calculated parameters I would no >> longer vote against this patch. That's nice. Your Acked-by would be great. I would provide a patch for the 'ip' tools help text then. Fortunately there is no code change necessary in iplink_can.c and this functionality can be used with all iproute2 versions if the kernel supports it. And if not, it's really easy to patch ;-) Many thanks, Oliver -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Pavel, On 09/23/2011 11:32 AM, Pavel Pisa wrote: > Hello Oliver and Wolfgang, > > On Friday 23 September 2011 09:24:20 Wolfgang Grandegger wrote: >> Hi Oliver, >> >> On 09/22/2011 09:41 PM, Oliver Hartkopp wrote: >> Then let us set it to 4 (maximum), by default. But other documents >> recommend a value of 1. > > I am not expert for CAN timing nor I have time to go through > documentation at this moment. But I hope that I have some > sense/experience for electronic and dynamic systems > and we have done more designs utilizing CAN at university > and at our company. > > So there are some thought about SJW based on my experience. > There could be some inaccuracies, if you want better > analysis, I need time for that. > > 1) The CAN needs fast local roundrip time for correct operation > of dominant/recessive arbitration. > The time is sum of propagation from CAN controller chip, > propagation delay of optocoupler or other galvanic isolation, > slew rate and delay in Tx circuitry of transceiver, charging > the wire, Rx part of receiver, optocoupler, controller > Rx filtering and Rx,Tx logic level comparator. > Iw we count necessity to synchronize this between multiple > CAN nodes then whole roundtrip time has to be smaller than > 50-80% of single bit bit time quantum. > > 2) From the noise and stabilization of voltage on the wire > the sampling point should be in the middle of bit pulse > delayed by round-trip delay from 1. This is 50% of bit > time (i.e. dependent on bitrate) + round trip delay (dependent > on delay in controller and concrete board circuitry). > > 3) But sampling point has to allow to decide about collision > before next bit Tx is started => it cannot be moved after > end of the given Tx bit time interval. > > 4) It is necessary to synchronize bit timing in all > CAN controllers during arbitration (ID sending) phase. > The first sync is done after receiving of the first > edge on the wire. It can be other node start of Tx > or our own Tx dominant level. Controller shifts its Tx > timing such way, that start of the next Tx bit interval > should result in Rx transition at the end the sensed > Rx bit. I.e. it counts only TSEG2 til next bit start. > > 5) There are some more things to consider if triple > sampling and filtering is enabled to suppress noise > on wire. It can be used only, if time quantum clocks > are fast enough to fit this sampling interval between > stabilization of delayed Rx logic level and end of Tx bit > time interval. > > 6) If the clock frequencies of all nodes CAN controllers > are guaranteed to be same, then no more synchronization > is necessary during message Tx/Rx (SJW=0). But that assumption > does not hold. That is why bitstuffing is used and guarantees > that there is at least one logic level transition (edge) > after each 6 bit time. If there is zero roundtrip propagation > delay and sampling in 50% of bit time interval then > maximal skew/frequency difference of the clocks could be > > (1+1/6*50%) - 1 i.e. 8% > > This means, that SJW bigger than 8% of whole bit time > would not help to synchronize bitrate difference, because > for such case setup cannot work anyway. Propagation delay > is not zero then there is even less time left for sampling > point shift which would not cause incorrect bit data > detection so reasonable maximum is probably lower. > > I expect that for reasonable precise setup of bitrate > when exact ratio is found and crystal based oscillators > there is the best option small SJW i.e. 1 or for very > fast TQ clock equivalent of 1% - 2% of bittime. > For nonexact ratio or low quality clocks sources, > bigger SJW values make sense. But big value has other > disadvantage. If there is bigger jitter in Tx/Rx delays > or clocks then it is propagated back to bit timing > and stability of system composed from multiple nodes > could be questionable. There is even bigger interval > where noise pulse could cause lost of the synchronization. > > On base of above analysis, I think that blindly set SJW > on maximum is not good idea. It should be at least limited > to 5% of bit time. > > Because bit timing calculation is not what everybody > want to do again and again, the actual code tries > to hide differences of CAN controllers and provide > at least partially understandable knobs to user > with parameters independent on concrete setup low level > details to allow set bitrate for usual Joe user. > The basic parameters are chosen such way, that user need > not to care about actual TQ clock and selecting prescaller, > that is why sampling point is in percent of bittime. > SJW is more problematic, but may it be use of 2 or 5% > of bittime by default with assurance that zero is > replaced by one, would serve to most people pleasure. > May it be, that 0.1 of percent should be used as unit > for external parameter too. > > I hope that actual calculation works reasonably well. > But if it should be enhanced, then it would worth > to add additional parameter except crystal/controller > clock source freqency to the concrete boards drivers. > It should be measured round-trip delay of given/used > transceiver/optocouplers combination. This would > allow to have sampling point setup yet more independent > on given HW and same value could be used for different > bitrates. But there is still unknown parameter > capacity/length of connected wires so there is still > something left to user consideration. Thanks for your detailed explanation. It clearly shows that adjusting SJW is non-trivial and nothing a normal CAN user should deal with. When adjusting SJW, do you also need to tweek other bit-timing parameters, e.g. tq? I mean, would "ip link set can0 type can bitrate x sampling-point y sjw z" work for your setup or do you need to use the expert mode setting via "ip link set can0 type can tq ..." anyway? Wolfgang. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Pavel, On 09/23/2011 11:32 AM, Pavel Pisa wrote: > Hello Oliver and Wolfgang, ... > I hope that actual calculation works reasonably well. > But if it should be enhanced, then it would worth > to add additional parameter except crystal/controller > clock source freqency to the concrete boards drivers. > It should be measured round-trip delay of given/used > transceiver/optocouplers combination. This would > allow to have sampling point setup yet more independent > on given HW and same value could be used for different > bitrates. But there is still unknown parameter > capacity/length of connected wires so there is still > something left to user consideration. The actual in-kernel bit-timing calculation is known to work well for most, not all, CAN users, especially with "good" clock source frequencies (a multiple of 8 MHz). That's fine and intended. The results of that algorithm are not always satisfactory. Such cases should then be handled in user-space, either manually or by an advanced algorithm. In the BerliOS "can-utils" there we have already "can-calc-bit-timing". Any enhancements and improvements would be nice to share, indeed. Do you already have/use some code for the enhancement mentioned above? Wolfgang. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/24/11 09:22, Wolfgang Grandegger wrote: > On 09/23/2011 11:32 AM, Pavel Pisa wrote: >> On base of above analysis, I think that blindly set SJW >> on maximum is not good idea. It should be at least limited >> to 5% of bit time. (..) >> SJW is more problematic, but may it be use of 2 or 5% >> of bittime by default with assurance that zero is >> replaced by one, would serve to most people pleasure. (..) >> But there is still unknown parameter >> capacity/length of connected wires so there is still >> something left to user consideration. > > Thanks for your detailed explanation. It clearly shows that adjusting > SJW is non-trivial and nothing a normal CAN user should deal with. When > adjusting SJW, do you also need to tweek other bit-timing parameters, > e.g. tq? I mean, would "ip link set can0 type can bitrate x > sampling-point y sjw z" work for your setup or do you need to use the > expert mode setting via "ip link set can0 type can tq ..." anyway? Hello Wolfgang, i double checked a specification where i got my requirement to influence the SJW value from. It says (non literally): "Put the SJW to the highest possible value only reduced by tseg2." The fact that this requirement might no fit to any CAN setup or may not be in best academical shape is not my problem. My requirement is to provide an easy way to move the SJW away from it's default value, which is currently hard-coded to 1 in the can-dev framework when using it's bittiming calculation function. As almost everything is already done (only the provided SJW is not evaluated in dev.c) this patch is IMO an valid option to support it. If Joe user can tune the sampling point, why should he not be able to influence the SJW if he thinks, he knows what he's doing? Especially as the good working bittiming calculation is not touched in any way and the default SJW remains 1 (even if '0' is provided as user input). Regards, Oliver -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/27/2011 07:32 PM, Oliver Hartkopp wrote: > On 09/24/11 09:22, Wolfgang Grandegger wrote: > >> On 09/23/2011 11:32 AM, Pavel Pisa wrote: > >>> On base of above analysis, I think that blindly set SJW >>> on maximum is not good idea. It should be at least limited >>> to 5% of bit time. > > > (..) > > >>> SJW is more problematic, but may it be use of 2 or 5% >>> of bittime by default with assurance that zero is >>> replaced by one, would serve to most people pleasure. > > > (..) > >>> But there is still unknown parameter >>> capacity/length of connected wires so there is still >>> something left to user consideration. >> >> Thanks for your detailed explanation. It clearly shows that adjusting >> SJW is non-trivial and nothing a normal CAN user should deal with. When >> adjusting SJW, do you also need to tweek other bit-timing parameters, >> e.g. tq? I mean, would "ip link set can0 type can bitrate x >> sampling-point y sjw z" work for your setup or do you need to use the >> expert mode setting via "ip link set can0 type can tq ..." anyway? > > > Hello Wolfgang, > > i double checked a specification where i got my requirement to influence the > SJW value from. It says (non literally): > > "Put the SJW to the highest possible value only reduced by tseg2." > > The fact that this requirement might no fit to any CAN setup or may not be in > best academical shape is not my problem. My requirement is to provide an easy > way to move the SJW away from it's default value, which is currently > hard-coded to 1 in the can-dev framework when using it's bittiming calculation > function. OK. > As almost everything is already done (only the provided SJW is not evaluated > in dev.c) this patch is IMO an valid option to support it. If Joe user can > tune the sampling point, why should he not be able to influence the SJW if he > thinks, he knows what he's doing? > > Especially as the good working bittiming calculation is not touched in any way > and the default SJW remains 1 (even if '0' is provided as user input). Still not sure if it's useful. Anyway, just added my acked-by to your patch. Thanks, Wolfgang. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/22/2011 12:28 PM, Oliver Hartkopp wrote: > This patch adds support for SJW user settings to not set the synchronization > jump width (SJW) to 1 in any case when using the in-kernel bittiming > calculation. > > The ip-tool from iproute2 already supports to pass the user defined SJW > value. The given SJW value is sanitized with the controller specific sjw_max > and the calculated tseg2 value. As the SJW can have values up to 4 providing > this value will lead to the maximum possible SJW automatically. A higher SJW > allows higher controller oscillator tolerances. > > Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net> Acked-by: Wolfgang Grandegger <wg@grandegger.com> Thanks, Wolfgang. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Sep 22, 2011 at 12:28:54PM +0200, Oliver Hartkopp wrote: > This patch adds support for SJW user settings to not set the synchronization > jump width (SJW) to 1 in any case when using the in-kernel bittiming > calculation. > > The ip-tool from iproute2 already supports to pass the user defined SJW > value. The given SJW value is sanitized with the controller specific sjw_max > and the calculated tseg2 value. As the SJW can have values up to 4 providing > this value will lead to the maximum possible SJW automatically. A higher SJW > allows higher controller oscillator tolerances. > Since both iproute2 & the bittiming calculation seem to be ready for it, activating the SJW setting looks like a good idea. Thanks, Kurt Acked-by: Kurt Van Dijck <kurt.van.dijck@eia.be> -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c index 9bf1116..25695bd 100644 --- a/drivers/net/can/dev.c +++ b/drivers/net/can/dev.c @@ -150,7 +150,19 @@ static int can_calc_bittiming(struct net_device *dev, struct can_bittiming *bt) bt->prop_seg = tseg1 / 2; bt->phase_seg1 = tseg1 - bt->prop_seg; bt->phase_seg2 = tseg2; - bt->sjw = 1; + + /* check for sjw user settings */ + if (!bt->sjw || !btc->sjw_max) + bt->sjw = 1; + else { + /* bt->sjw is at least 1 -> sanitize upper bound to sjw_max */ + if (bt->sjw > btc->sjw_max) + bt->sjw = btc->sjw_max; + /* bt->sjw must not be higher than tseg2 */ + if (tseg2 < bt->sjw) + bt->sjw = tseg2; + } + bt->brp = best_brp; /* real bit-rate */
This patch adds support for SJW user settings to not set the synchronization jump width (SJW) to 1 in any case when using the in-kernel bittiming calculation. The ip-tool from iproute2 already supports to pass the user defined SJW value. The given SJW value is sanitized with the controller specific sjw_max and the calculated tseg2 value. As the SJW can have values up to 4 providing this value will lead to the maximum possible SJW automatically. A higher SJW allows higher controller oscillator tolerances. Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net> --- v2: Resend due to wrong mail address header encoding. bt->bitrate = priv->clock.freq / (bt->brp * (tseg1 + tseg2 + 1)); -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html