Linux CAN drivers development
 help / color / mirror / Atom feed
From: Max Staudt <max@enpas.org>
To: Marc Kleine-Budde <mkl@pengutronix.de>,
	Dario Binacchi <dario.binacchi@amarulasolutions.com>
Cc: linux-kernel@vger.kernel.org, linux-can@vger.kernel.org,
	Oliver Hartkopp <socketcan@hartkopp.net>,
	michael@amarulasolutions.com,
	Amarula patchwork <linux-amarula@amarulasolutions.com>,
	Jeroen Hofstee <jhofstee@victronenergy.com>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Wolfgang Grandegger <wg@grandegger.com>,
	netdev@vger.kernel.org
Subject: Re: [RFC PATCH v3 8/9] can: slcan: add support to set bit time register (btr)
Date: Wed, 27 Jul 2022 19:28:39 +0200	[thread overview]
Message-ID: <20220727192839.707a3453.max@enpas.org> (raw)
In-Reply-To: <20220727113054.ffcckzlcipcxer2c@pengutronix.de>

On Wed, 27 Jul 2022 13:30:54 +0200
Marc Kleine-Budde <mkl@pengutronix.de> wrote:

> As far as I understand, setting the btr is an alternative way to set the
> bitrate, right? I don't like the idea of poking arbitrary values into a
> hardware from user space.

I agree with Marc here.

This is a modification across the whole stack, specific to a single
device, when there are ways around.


If I understand correctly, the CAN232 "S" command sets one of the fixed
bitrates, whereas "s" sets the two BTR registers. Now the question is,
what do BTR0/BTR1 mean, and what are they? If they are merely a divider
in a CAN controller's master clock, like in ELM327, then you could

  a) Calculate the BTR values from the bitrate userspace requests, or

  b) pre-calculate a huge table of possible bitrates and present them
     all to userspace. Sounds horrible, but that's what I did in can327,
     haha. Maybe I should have reigned them in a little, to the most
     useful values.

  c) just limit the bitrates to whatever seems most useful (like the
     "S" command's table), and let users complain if they really need
     something else. In the meantime, they are still free to slcand or
     minicom to their heart's content before attaching slcan, thanks to
     your backwards compatibility efforts.


In short, to me, this isn't a deal breaker for your patch series.


Max

  parent reply	other threads:[~2022-07-27 18:30 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-26 21:02 [RFC PATCH v3 0/9] can: slcan: extend supported features (step 2) Dario Binacchi
2022-07-26 21:02 ` [RFC PATCH v3 1/9] can: slcan: use KBUILD_MODNAME and define pr_fmt to replace hardcoded names Dario Binacchi
2022-07-26 21:02 ` [RFC PATCH v3 2/9] can: slcan: remove useless header inclusions Dario Binacchi
2022-07-26 21:02 ` [RFC PATCH v3 3/9] can: slcan: remove legacy infrastructure Dario Binacchi
2022-07-27 17:09   ` Max Staudt
2022-07-26 21:02 ` [RFC PATCH v3 4/9] can: slcan: change every `slc' occurrence in `slcan' Dario Binacchi
2022-07-26 21:02 ` [RFC PATCH v3 5/9] can: slcan: use the generic can_change_mtu() Dario Binacchi
2022-07-26 21:02 ` [RFC PATCH v3 6/9] can: slcan: add support for listen-only mode Dario Binacchi
2022-07-26 21:02 ` [RFC PATCH v3 7/9] ethtool: add support to get/set CAN bit time register Dario Binacchi
2022-07-26 21:02 ` [RFC PATCH v3 8/9] can: slcan: add support to set bit time register (btr) Dario Binacchi
2022-07-27 11:30   ` Marc Kleine-Budde
2022-07-27 15:55     ` Dario Binacchi
2022-07-27 17:21       ` Marc Kleine-Budde
2022-07-27 17:28         ` Dario Binacchi
2022-07-27 17:33           ` Max Staudt
2022-07-27 18:23             ` Dario Binacchi
2022-07-27 17:28     ` Max Staudt [this message]
2022-07-27 18:24       ` Marc Kleine-Budde
2022-07-27 20:12         ` Oliver Hartkopp
2022-07-28  6:56           ` Marc Kleine-Budde
2022-07-28  7:36         ` Dario Binacchi
2022-07-28  9:02           ` Marc Kleine-Budde
2022-07-28 10:23             ` Dario Binacchi
2022-07-28 10:50               ` Marc Kleine-Budde
2022-07-28 10:57                 ` Max Staudt
2022-07-29  6:52                   ` Dario Binacchi
2022-07-29  7:33                     ` Marc Kleine-Budde
2022-07-31 15:54                       ` Dario Binacchi
2022-07-31 18:58                         ` Marc Kleine-Budde
2022-07-26 21:02 ` [RFC PATCH v3 9/9] MAINTAINERS: Add maintainer for the slcan driver Dario Binacchi
2022-07-27 18:31 ` [RFC PATCH v3 0/9] can: slcan: extend supported features (step 2) 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=20220727192839.707a3453.max@enpas.org \
    --to=max@enpas.org \
    --cc=dario.binacchi@amarulasolutions.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=jhofstee@victronenergy.com \
    --cc=kuba@kernel.org \
    --cc=linux-amarula@amarulasolutions.com \
    --cc=linux-can@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=michael@amarulasolutions.com \
    --cc=mkl@pengutronix.de \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=socketcan@hartkopp.net \
    --cc=wg@grandegger.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