All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marc Kleine-Budde <mkl@pengutronix.de>
To: Dario Binacchi <dario.binacchi@amarulasolutions.com>
Cc: linux-kernel@vger.kernel.org, michael@amarulasolutions.com,
	Amarula patchwork <linux-amarula@amarulasolutions.com>,
	Oliver Hartkopp <socketcan@hartkopp.net>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Vincent Mailhol <mailhol.vincent@wanadoo.fr>,
	Wolfgang Grandegger <wg@grandegger.com>,
	linux-can@vger.kernel.org, netdev@vger.kernel.org
Subject: Re: [PATCH v3 05/13] can: netlink: dump bitrate 0 if can_priv::bittiming.bitrate is -1U
Date: Mon, 13 Jun 2022 09:10:58 +0200	[thread overview]
Message-ID: <20220613071058.h6bmy6emswh76q5s@pengutronix.de> (raw)
In-Reply-To: <20220612213927.3004444-6-dario.binacchi@amarulasolutions.com>

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

On 12.06.2022 23:39:19, Dario Binacchi wrote:
> Adding Netlink support to the slcan driver made it necessary to set the
> bitrate to a fake value (-1U) to prevent open_candev() from failing. In
> this case the command `ip --details -s -s link show' would print
> 4294967295 as the bitrate value. The patch change this value in 0.
> 
> Suggested-by: Marc Kleine-Budde <mkl@pengutronix.de>
> Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> ---
> 
> (no changes since v1)
> 
>  drivers/net/can/dev/netlink.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/can/dev/netlink.c b/drivers/net/can/dev/netlink.c
> index 7633d98e3912..788a6752fcc7 100644
> --- a/drivers/net/can/dev/netlink.c
> +++ b/drivers/net/can/dev/netlink.c
> @@ -505,11 +505,16 @@ static int can_fill_info(struct sk_buff *skb, const struct net_device *dev)
>  	struct can_ctrlmode cm = {.flags = priv->ctrlmode};
>  	struct can_berr_counter bec = { };
>  	enum can_state state = priv->state;
> +	__u32 bitrate = priv->bittiming.bitrate;
> +	int ret = 0;
>  
>  	if (priv->do_get_state)
>  		priv->do_get_state(dev, &state);
>  
> -	if ((priv->bittiming.bitrate &&

What about changing this line to:

        if ((priv->bittiming.bitrate && priv->bittiming.bitrate != -1 &&

This would make the code a lot cleaner. Can you think of a nice macro
name for the -1?

0 could be CAN_BITRATE_UNCONFIGURED or _UNSET. For -1 I cannot find a
catchy name, something like CAN_BITRATE_CONFIGURED_UNKOWN or
SET_UNKNOWN.

The macros can be added to bittiming.h and be part of this patch. Ofq
course the above code (and slcan.c) would make use of the macros instead
of using 0 and -1.

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:[~2022-06-13  7:11 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-12 21:39 [PATCH v3 00/13] can: slcan: extend supported features Dario Binacchi
2022-06-12 21:39 ` [PATCH v3 01/13] can: slcan: use the BIT() helper Dario Binacchi
2022-06-12 21:39 ` [PATCH v3 02/13] can: slcan: use netdev helpers to print out messages Dario Binacchi
2022-06-12 21:39 ` [PATCH v3 03/13] can: slcan: use the alloc_can_skb() helper Dario Binacchi
2022-06-12 21:39 ` [PATCH v3 04/13] can: slcan: use CAN network device driver API Dario Binacchi
2022-06-13  7:01   ` Marc Kleine-Budde
2022-06-12 21:39 ` [PATCH v3 05/13] can: netlink: dump bitrate 0 if can_priv::bittiming.bitrate is -1U Dario Binacchi
2022-06-13  7:10   ` Marc Kleine-Budde [this message]
2022-06-13 20:44     ` Dario Binacchi
2022-06-14  7:24       ` Marc Kleine-Budde
2022-06-12 21:39 ` [PATCH v3 06/13] can: slcan: allow to send commands to the adapter Dario Binacchi
2022-06-12 21:39 ` [PATCH v3 07/13] can: slcan: set bitrate by CAN device driver API Dario Binacchi
2022-06-13  7:17   ` Marc Kleine-Budde
2022-06-12 21:39 ` [PATCH v3 08/13] can: slcan: send the open command to the adapter Dario Binacchi
2022-06-12 21:39 ` [PATCH v3 09/13] can: slcan: send the close " Dario Binacchi
2022-06-13  7:22   ` Marc Kleine-Budde
2022-06-12 21:39 ` [PATCH v3 10/13] can: slcan: move driver into separate sub directory Dario Binacchi
2022-06-12 21:39 ` [PATCH v3 11/13] can: slcan: add ethtool support to reset adapter errors Dario Binacchi
2022-06-12 21:39 ` [PATCH v3 12/13] can: slcan: extend the protocol with error info Dario Binacchi
2022-06-13  7:32   ` Marc Kleine-Budde
2022-06-13 21:04     ` Dario Binacchi
2022-06-12 21:39 ` [PATCH v3 13/13] can: slcan: extend the protocol with CAN state info Dario Binacchi
2022-06-13  7:37   ` Marc Kleine-Budde
2022-06-14  6:29     ` Dario Binacchi
2022-06-14  7:22       ` 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=20220613071058.h6bmy6emswh76q5s@pengutronix.de \
    --to=mkl@pengutronix.de \
    --cc=dario.binacchi@amarulasolutions.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=kuba@kernel.org \
    --cc=linux-amarula@amarulasolutions.com \
    --cc=linux-can@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mailhol.vincent@wanadoo.fr \
    --cc=michael@amarulasolutions.com \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.