linux-can.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Marc Kleine-Budde <mkl@pengutronix.de>
To: Vincent Mailhol <vincent.mailhol@gmail.com>
Cc: linux-can@vger.kernel.org,
	Thomas Kopp <thomas.kopp@microchip.com>,
	kernel@pengutronix.de, Mark Bath <mark@baggywrinkle.co.uk>
Subject: Re: [PATCH v2 14/17] can: bittiming: can_sjw_set_default(): use Phase Seg2 / 2 as default for SJW
Date: Mon, 6 Feb 2023 14:01:46 +0100	[thread overview]
Message-ID: <20230206130146.pkrlmcimeph5urpd@pengutronix.de> (raw)
In-Reply-To: <20230202120738.3f3amaj5bdntjmhc@pengutronix.de>

[-- Attachment #1: Type: text/plain, Size: 2143 bytes --]

On 02.02.2023 13:07:38, Marc Kleine-Budde wrote:
> On 02.02.2023 20:57:42, Vincent Mailhol wrote:
> > On Thu. 2 Feb 2023 at 20:09, Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> > > "The (Re-)Synchronization Jump Width (SJW) defines how far a
> > >  resynchronization may move the Sample Point inside the limits defined
> > >  by the Phase Buffer Segments to compensate for edge phase errors." [1]
> > >
> > > In other words, this means that the SJW parameter controls the CAN
> > > controller's tolerance to frequency errors compared to other CAN
> > > controllers.
> > >
> > > If the user space doesn't provide a SJW parameter, the
> > > kernel chooses a default value of 1. This has proven to be a good
> > > default value for CAN controllers, but no longer for modern
> >                     ^^^^^^^^^^^^^^^
> > > controllers.
> > 
> > Are you missing a word here? You oppose CAN controllers to modern
> > ones.
> > 
> > I think the point is Classical CAN only controllers vs. CAN-FD
> > controllers. A CAN-FD controller is able to sample at bitrates up to 5
> > or 8 Mbits and have maximum bitimming values five or eight times the
> > ones of a Classical CAN only controller (which is only capable of
> > sampling 1 Mbits).
> > 
> > I propose this instead:
> > 
> >   This has proven to be a good default value for Classical CAN
> >   controllers, but no longer for modern CAN-FD ones.
> 
> The difference that matters here is not that the controllers support
> CAN-FD, but that they have a much greater max tseg{1,2} compared to the
> sja1000. But that's only the case on CAN-FD controller.
> 
> Will change the description as you proposed!

There are no other changes on this series, I've fixed the patch
description as suggested. I'll take this series without resending it.

Thanks again for the review!

regards,
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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

  reply	other threads:[~2023-02-06 13:19 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-02 11:08 [PATCH v2 00/17] can: bittiming: cleanups and rework SJW handling Marc Kleine-Budde
2023-02-02 11:08 ` [PATCH v2 01/17] can: bittiming(): replace open coded variants of can_bit_time() Marc Kleine-Budde
2023-02-02 11:08 ` [PATCH v2 02/17] can: bittiming: can_fixup_bittiming(): use CAN_SYNC_SEG instead of 1 Marc Kleine-Budde
2023-02-02 11:08 ` [PATCH v2 03/17] can: bittiming: can_fixup_bittiming(): set effective tq Marc Kleine-Budde
2023-02-02 11:08 ` [PATCH v2 04/17] can: bittiming: can_get_bittiming(): use direct return and remove unneeded else Marc Kleine-Budde
2023-02-02 11:08 ` [PATCH v2 05/17] can: dev: register_candev(): ensure that bittiming const are valid Marc Kleine-Budde
2023-02-02 11:08 ` [PATCH v2 06/17] can: dev: register_candev(): bail out if both fixed bit rates and bit timing constants are provided Marc Kleine-Budde
2023-02-02 11:08 ` [PATCH v2 07/17] can: netlink: can_validate(): validate sample point for CAN and CAN-FD Marc Kleine-Budde
2023-02-02 11:08 ` [PATCH v2 08/17] can: netlink: can_changelink(): convert from netdev_err() to NL_SET_ERR_MSG_FMT() Marc Kleine-Budde
2023-02-02 11:08 ` [PATCH v2 09/17] can: bittiming: can_changelink() pass extack down callstack Marc Kleine-Budde
2023-02-02 11:08 ` [PATCH v2 10/17] can: bittiming: factor out can_sjw_set_default() and can_sjw_check() Marc Kleine-Budde
2023-02-02 11:51   ` Vincent Mailhol
2023-02-02 11:53     ` Marc Kleine-Budde
2023-02-02 12:04       ` Vincent Mailhol
2023-02-02 11:08 ` [PATCH v2 11/17] can: bittiming: can_fixup_bittiming(): report error via netlink and harmonize error value Marc Kleine-Budde
2023-02-02 11:08 ` [PATCH v2 12/17] can: bittiming: can_sjw_check(): " Marc Kleine-Budde
2023-02-02 11:08 ` [PATCH v2 13/17] can: bittiming: can_sjw_check(): check that SJW is not longer than either Phase Buffer Segment Marc Kleine-Budde
2023-02-02 11:08 ` [PATCH v2 14/17] can: bittiming: can_sjw_set_default(): use Phase Seg2 / 2 as default for SJW Marc Kleine-Budde
2023-02-02 11:57   ` Vincent Mailhol
2023-02-02 12:07     ` Marc Kleine-Budde
2023-02-06 13:01       ` Marc Kleine-Budde [this message]
2023-02-02 11:08 ` [PATCH v2 15/17] can: bittiming: can_calc_bittiming(): clean up SJW handling Marc Kleine-Budde
2023-02-02 11:08 ` [PATCH v2 16/17] can: bittiming: can_calc_bittiming(): convert from netdev_err() to NL_SET_ERR_MSG_FMT() Marc Kleine-Budde
2023-02-02 11:08 ` [PATCH v2 17/17] can: bittiming: can_validate_bitrate(): report error via netlink Marc Kleine-Budde
2023-02-02 12:05 ` [PATCH v2 00/17] can: bittiming: cleanups and rework SJW handling Vincent Mailhol
2023-02-02 12:17   ` Marc Kleine-Budde

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230206130146.pkrlmcimeph5urpd@pengutronix.de \
    --to=mkl@pengutronix.de \
    --cc=kernel@pengutronix.de \
    --cc=linux-can@vger.kernel.org \
    --cc=mark@baggywrinkle.co.uk \
    --cc=thomas.kopp@microchip.com \
    --cc=vincent.mailhol@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).