From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pj1-f44.google.com (mail-pj1-f44.google.com [209.85.216.44]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 8AF832D7DC0 for ; Thu, 25 Dec 2025 07:29:04 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.216.44 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1766647750; cv=none; b=eilmtA8O4yg0QurcwiBAqE/ny72RUob1yhasWVtg+lgjjkLJJFSfpvWikXy5r69Y1MV87Rmjdjxyw8kLc76WXdW0aE37NK8vp/FwrP/AI0TjypDSRj41Va4LhOko3GIfr/W19nuhEYzIHII1pWuLpumwZkFg934rr7wr1ZTRlP8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1766647750; c=relaxed/simple; bh=8vRLfhIUeM10MZQMkRFvfBNtxfHbjybdfprvT3apTE0=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=EF2W4QynisYxxOheRO+N8eBBRa+9/y08TQNbYoytuPC8TmLl1QswE2ZraB0fkWQjM8V1svnhevzCtejZUewAxUARtJaKGoJ0rFrZUwgt4aOxBfVZgGAFT/9bzuE6yHmTeVffxqId1JkfXtUKB0NK/FoASFfcadP42lpKIqN94Wc= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=ilbkhPYD; arc=none smtp.client-ip=209.85.216.44 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="ilbkhPYD" Received: by mail-pj1-f44.google.com with SMTP id 98e67ed59e1d1-34ab8e0df53so6204966a91.3 for ; Wed, 24 Dec 2025 23:29:04 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1766647744; x=1767252544; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=H4PxTLcRhV02JhrM3zAG0ZiRLecDRevSACvJtyy7e18=; b=ilbkhPYD5dBYW1UZdsEiWf0GMP5PI/CBrf9BYOC0gaQmlOf4esaNBaOHk751LjXw+i O7opan1b+FKhkxt3RW9+dXbUSQUHi6eQoQA8JEALAHOugdfmsAh9NdHJoZtMSCu6chDf gteEp+gpV9HfT2DWwsDNWR8auiCUthV9c9XDJH219mkhu+8Gg6kHOtjm9reS2WlY9Sa8 +4qG1KQzD6Su1tQVn5zMKxZEZ6Ff94FMiuV+ZGAyAgBfmlTdpz7YoLE4+iCh/GlM5TSa KxBidVvRGKRBrq4e2mjqfGKgHG8+Sd4E3eGwR2scsgs+bDbv/lhhk1SAu908g7mC2vx5 208g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1766647744; x=1767252544; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=H4PxTLcRhV02JhrM3zAG0ZiRLecDRevSACvJtyy7e18=; b=ZO2nPYavuw/rNQ0E3t+p28QuLtC9Q4pPluHd6/a/+cnsw/xtdCcFwJ0SM3IuBq3wEF GftwAdn4UT2VzQJ+pNxXLe9b84mdqBVpHRiERqQbU5tuKgqb3gp2Qd4dzRd/PYttDl7u SMsUq3YUHs8qRsb+ktB2B36E0kFtz76ocS7cOVFXQQvAf6pXbtxHwgXIuZ76Wvw/3W2k 752N3A33rxuvlKhFJnubamaK/+TPyS5XrNWq6b1rhwsPjJTXE/0080tSdYozRqH7mV/P Uw6zsBm9SL13W17i7OlgCjW/IangnP9PcPduoGr4uGRhn0r4tVSGNyOT+pEZkkjTgc+P qL6w== X-Forwarded-Encrypted: i=1; AJvYcCWblmjMQlIrcwLdOqKP9lxSV3P7l9tAsdvai80YCO3H9FsDQOV6qMt+DTslb95Hvrm/HW0bXaCG7ImfGpA=@vger.kernel.org X-Gm-Message-State: AOJu0Yx9MRP97aHBWAuSMe11304tQPDLWAevgfpIfs+kVnq7rQhy7QHy Na2IJEouKVrYprBw+A3OCMUCz41/1vwqmqQTES/EPEkyJK5kH2Oyl+Ca X-Gm-Gg: AY/fxX6Eit5b5ByIvk4qCVOd8k+A75aFSuqvqJmzaHhG2SlYMU6hPPczvPTqreB9qc3 F+rCwPomVznCmqNY3odCXVxDGom4HsEl/RR2X0WqytT1Kb65aLCe3Hk/iig5L1LvRdDHQcsbVk0 TC29G13Nqt6fjfZicGyd1Ey88wLgIG+uPxUVpOry7nN2UOTSO/Hg7hTsfwWjS/K9ljF7xSow36q T/90ivvXnR+bq4OGlka4bAUjMsTLALOLYCvC4qVdjBTpaLsJ7dGYZyJ57o8rx2OD98gNX5Bbjto 1rAoGzmp8sA15yf0240xc08Hxrwrr8wlo/47AtNFsO71fb6ecQTb3loyIxLjv+NwLeBCiuSoCM0 X/mHf7VP3EdOUTtcg3Glp48SkZIF0YhyrsRNAk05uSwnQJnCSZfSshnppqqtHLw+JATNtd1L0FW vupqGqhiyt0qNx3gPUvfojlK7sFbk= X-Google-Smtp-Source: AGHT+IG6opa3oqxFSPA/MNWdnHMCdRbA3sZg+BBm70hMufpLc03qIkSSDXXBZ4h+R9OYRZU0x56APg== X-Received: by 2002:a05:6a21:3282:b0:366:14ac:e1fa with SMTP id adf61e73a8af0-376aa6eadfemr19958794637.76.1766647743863; Wed, 24 Dec 2025 23:29:03 -0800 (PST) Received: from rakuram-MSI ([2409:40f4:2007:f705:4461:8ef4:d041:3e1c]) by smtp.gmail.com with ESMTPSA id 98e67ed59e1d1-34e70d4f7e2sm20218153a91.2.2025.12.24.23.29.01 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 24 Dec 2025 23:29:03 -0800 (PST) From: Rakuram Eswaran To: mailhol@kernel.org Cc: linux-can@vger.kernel.org, linux-kernel@vger.kernel.org, mkl@pengutronix.de, rakuram.e96@gmail.com, socketcan@hartkopp.net Subject: Re: [RFC PATCH 1/2] can: dummy_can: add CAN termination support Date: Thu, 25 Dec 2025 12:58:54 +0530 Message-ID: <20251225072857.53085-1-rakuram.e96@gmail.com> X-Mailer: git-send-email 2.51.0 In-Reply-To: a85b8659-d4c8-443b-abb1-ae557a2a9896@kernel.org References: Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Hi Vincent, Thank you for the review and the detailed feedback. On Wed, 24 Dec 2025 at 03:03, Vincent Mailhol wrote: > > Hi Rakuram, > > Thanks for the patch. My comments are only on the cosmetic aspect. > > Le 27/11/2025 à 20:18, Rakuram Eswaran a écrit : > > Add support for configuring bus termination in the dummy_can driver. > > This allows users to emulate a properly terminated CAN bus when > > setting up virtual test environments. > > > > Signed-off-by: Rakuram Eswaran > > --- > > Tested the termination setting using below iproute commands: > > > > ip link set can0 type can termination 120 > > ip link set can0 type can termination off > > When you test, do not forget to also try incorrect values ;) > > ip link set can0 type can termination 100 Noted. I did test with invalid numeric values and observed that they were ignored. > ip link set can0 type can termination potato Good point — I had not explicitly tested non-numeric inputs. I will verify this case as well. > > (I think that the code is correct, just see this as a generic > comment). > > > drivers/net/can/dummy_can.c | 21 +++++++++++++++++++++ > > 1 file changed, 21 insertions(+) > > > > diff --git a/drivers/net/can/dummy_can.c b/drivers/net/can/dummy_can.c > > index 41953655e3d3..2949173547e6 100644 > > --- a/drivers/net/can/dummy_can.c > > +++ b/drivers/net/can/dummy_can.c > > @@ -23,6 +23,21 @@ struct dummy_can { > > > > static struct dummy_can *dummy_can; > > > > +static const u16 dummy_can_termination_const[] = { > > + CAN_TERMINATION_DISABLED, /* 0 = off */ > > + 120, /* 120 Ohms */ > > +}; > > + > > +static int dummy_can_set_termination(struct net_device *dev, u16 term) > > +{ > > + struct dummy_can *priv = netdev_priv(dev); > > + > > + netdev_dbg(dev, "set termination to %u Ohms\n", term); > > + priv->can.termination = term; > > + > > + return 0; > > +} > > The driver has a kind of structure: > > - first the const bittiming struct declarations > - then the dummy_can_print_*() functions > - finally the actual code > > Try to preserve this structure when adding your changes. Acknowledged. I will reorder the termination-related additions to match the existing structure in the next revision. > > > static const struct can_bittiming_const dummy_can_bittiming_const = { > > .name = "dummy_can CC", > > .tseg1_min = 2, > > @@ -250,6 +265,12 @@ static int __init dummy_can_init(void) > > priv->can.xl.data_bittiming_const = &dummy_can_xl_databittiming_const; > > priv->can.xl.tdc_const = &dummy_can_xl_tdc_const; > > priv->can.xl.pwm_const = &dummy_can_pwm_const; > > + > > + /* Advertise software termination support */ > > This comment doesn't add much value. You may omit it. > Ack. Will remove it. > > + priv->can.termination_const = dummy_can_termination_const; > > + priv->can.termination_const_cnt = ARRAY_SIZE(dummy_can_termination_const); > > + priv->can.do_set_termination = dummy_can_set_termination; > > Here also try to maintain so kind of order: your declaration of > dummy_can_termination_const is before the other const struct > declarations, but the priv->can assignment is done after the other > assignments. Not a big deal but it is nicer to keep the declaration > and the assignments in the same order. > Ack. I will align the declaration and the corresponding assignments to keep the ordering consistent. I will address these points and send an updated version. Is it okay to send the next version without the RFC tag? Best Regards, Rakuram