Hello Oliver,
Am 14.06.2018 um 07:36 schrieb Oliver Hartkopp:
> On 06/12/2018 05:06 PM, Wolfgang Grandegger wrote:
>
>>>> Is tseg2 always greater than zero at this point? This depends on
>>>> btc->tseg2_min now which is given by the CAN driver, right?
>>>
>>> If we ever get tseg2 equal to zero we get inconsistant configuration
>>> anyway
>>> because:
>>> 1. "bt->phase_seg2 = tseg2" => Phase_Seg2 = 0
>>> 2. if user set SJW value, we would get SJW = 0 by following code:
>>> if (tseg2 < bt->sjw)
>>> bt->sjw = tseg2;
>>>
>>> I can add some code to check for configuration consistancy according
>>> to ISO 11898-1.
>
> see below (at the end)
>
>>> This should be a separate patch anyway, because while SJW value is
>
> right
>
>>> debatable there
>>> are clear requirements on Phase_Seg1, Phase_Seg2 and SJW values at the
>>> ISO 11898-1.
>>>
>>>> This comment has to be adapted or shortened and we need to make sure
>>>> that
>>>> sjw remains at least 1 - even when tseg2_min is zero.
>>>
>>> Could you please help we truncating the comment?
>
> I was thinking about something like this:
>
> (..)
> /* set sjw at least to 1 (min tseg2) if there are no user settings */
> if (!bt->sjw)
> bt->sjw = tseg2;
> (..)
We could use:
/* check for sjw user settings */
if (bt->sjw && btc->sjw_max) {
/* 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;
}
/* according to ISO 11898-1, sjw should be at least 1 */
if (!bt->sjw || !btc->sjw_max)
bt->sjw = 1;
Kind of paranoia, because tseg2_min and sjw_max is never 0. But we
need to check sjw anyway.
>>>> @Wolfgang: Should we generally check for btc->tseg2_min somewhere in
>>>> the
>>>> bittiming code to be greater than zero?
>>>
>>> I will take a look at the can/dev.c tommow and suggest a patch with a
>>> few
>>> consistancy checks.
>>
>> Thanks! "btc->tseg2_min" is a constant defined in the driver. It's
>> always >= 1, otherwise it's a bug. Or have I missed something.
>
> My question was whether we should check the correctness (>0) of
> btc->tseg2_min when starting the bitrate calculation to make sure the
> given constants in the driver were properly defined?
I don't see a need for a runtime check of the constant values tseg2_min
and also sjw_max... but see my comment above.
Wolfgang-
--
To unsubscribe from this list: send the line "unsubscribe linux-can" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
