On 12.09.2022 17:28:15, Vincent Mailhol wrote: > > > Not directly a criticism of this patch (as things were already like > > > that), but if the user provides an incorrect value for SJW (or any > > > other bittiming argument), wouldn't it be better to inform? Returning > > > -EINVAL might be too violent. Maybe a dmesg would be good? > > > > I'd say it would be consistent to keep returning -EINVAL (at least for the SJW>(min p1,p2)) condition. > > > > The same is done for FD Bitrate < Arbitration bitrate and both conditions have the same level of justification in the ISO 11898-1:2015 > > "The data bit time shall have the same length as the nominal bit time or it shall be shorter than the nominal bit time." > > > > "In nominal bit time and in data bit time, SJW shall be less than or equal to the minimum of these two items: Phase_Seg1 and Phase_Seg2." > > > > Shall is a binding requirement to be ISO conformant in this case. > > What you say makes sense. > > Also, I was assuming that can_fixup_bittiming() was already doing the > out of range check: > https://elixir.bootlin.com/linux/v6.0-rc1/source/drivers/net/can/dev/bittiming.c#L27 > > But in reality, only one of either can_calc_bittiming(), > can_fixup_bittiming() or can_validate_bitrate() is being called. And > thus, can_validate_bitrate() might be called with out of range values > and in that case the neltink interface should return -ERANGE (for > example if sjw > sjw_max). > > Sees that there is more work to do here than initially anticipated. If you configure the bitrate + sjw there will be a check for sjw within the limits (< sjw_max, < min(phase_seg1, phase_seg2)). If you configure the low level timing parameters there will be limit checks on the user supplied phase_seg1, phase_seg2 and sjw and a limit check on the calculated brp. What should be the return value if these checks fail? I think it's best to always return -EINVAL and set an error message with NL_SET_ERR_MSG_FMT(). Marc -- Pengutronix e.K. | Marc Kleine-Budde | Embedded Linux | https://www.pengutronix.de | Vertretung West/Dortmund | Phone: +49-231-2826-924 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Attachment:
signature.asc
Description: PGP signature
