From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2EB30C19F28 for ; Wed, 27 Jul 2022 18:30:54 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S239460AbiG0Saw (ORCPT ); Wed, 27 Jul 2022 14:30:52 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43636 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S239763AbiG0Sai (ORCPT ); Wed, 27 Jul 2022 14:30:38 -0400 X-Greylist: delayed 1154 seconds by postgrey-1.37 at lindbergh.monkeyblade.net; Wed, 27 Jul 2022 10:28:47 PDT Received: from mail.enpas.org (zhong.enpas.org [IPv6:2a03:4000:2:537::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id BA0CC107291; Wed, 27 Jul 2022 10:28:47 -0700 (PDT) Received: from [127.0.0.1] (localhost [127.0.0.1]) by mail.enpas.org (Postfix) with ESMTPSA id 6E70CFFCE8; Wed, 27 Jul 2022 17:28:46 +0000 (UTC) Date: Wed, 27 Jul 2022 19:28:39 +0200 From: Max Staudt To: Marc Kleine-Budde , Dario Binacchi Cc: linux-kernel@vger.kernel.org, linux-can@vger.kernel.org, Oliver Hartkopp , michael@amarulasolutions.com, Amarula patchwork , Jeroen Hofstee , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Wolfgang Grandegger , netdev@vger.kernel.org Subject: Re: [RFC PATCH v3 8/9] can: slcan: add support to set bit time register (btr) Message-ID: <20220727192839.707a3453.max@enpas.org> In-Reply-To: <20220727113054.ffcckzlcipcxer2c@pengutronix.de> References: <20220726210217.3368497-1-dario.binacchi@amarulasolutions.com> <20220726210217.3368497-9-dario.binacchi@amarulasolutions.com> <20220727113054.ffcckzlcipcxer2c@pengutronix.de> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-can@vger.kernel.org On Wed, 27 Jul 2022 13:30:54 +0200 Marc Kleine-Budde 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