Re: [PATCH v2 4/5] can: add netlink interface for CAN-FD Transmitter Delay Compensation (TDC)

From: Vincent MAILHOL <mailhol.vincent@wanadoo.fr>
To: Marc Kleine-Budde <mkl@pengutronix.de>
Cc: linux-can <linux-can@vger.kernel.org>,
	Oliver Hartkopp <socketcan@hartkopp.net>,
	Wolfgang Grandegger <wg@grandegger.com>
Subject: Re: [PATCH v2 4/5] can: add netlink interface for CAN-FD Transmitter Delay Compensation (TDC)
Date: Wed, 17 Mar 2021 00:16:01 +0900	[thread overview]
Message-ID: <CAMZ6RqJyMXzog1mu3S62yMAxJorTg0D5VL5OYKALYRoMxN_DdQ@mail.gmail.com> (raw)
In-Reply-To: <20210315155900.a6l5l5aeuvsgn55x@pengutronix.de>

On Tue. 16 Mar 2021 at 00:59, Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> On 24.02.2021 09:20:07, Vincent Mailhol wrote:
> > Add the netlink interface for TDC parameters of struct can_tdc and
> > can_tdc_const.
> >
> > Contrary to the can_bittiming(_const) structures for which there is
> > just a single IFLA_CAN(_DATA)_BITTMING(_CONST) entry per structure,
> > here, an IFLA_CAN_TDC* entry is added for each of the TDC parameters
> > of the newly introduced struct can_tdc and struct can_tdc_const.
> >
> > For struct can_tdc, these are:
> >       IFLA_CAN_TDCV
> >       IFLA_CAN_TDCO
> >       IFLA_CAN_TDCF
> >
> > For struct can_tdc_const, these are:
> >       IFLA_CAN_TDCV_MAX_CONST
> >       IFLA_CAN_TDCO_MAX_CONST
> >       IFLA_CAN_TDCF_MAX_CONST
> >
> > This is done so that changes can be applied in the future to the
> > structures without breaking the netlink interface.
> >
> > All the new parameters are defined as u32. This arbitrary choice is
> > done to mimic the other bittiming values with are also all of type
> > u32. An u16 would have been sufficient to hold the TDC values.
>
> I just had a look at the ethtool-netlink interface:
>
> | Documentation/networking/ethtool-netlink.rst
>
> this is much better designed than the CAN netlink interface. It was done
> by the pros and much later than CAN. :D So I'd like to have a similar
> structure for new CAN netlink stuff.
>
> So I think I'll remove this patch for now from can-next-testing. The
> kernel internal interface to tdc is still OK, we can leave it as is and
> change it if needed. But netlink is user space and I'd like to have it
> properly designed.

Understood. However, I will need more time to read and understand
the ethtool-netlink interface. The new patch will come later, I
do not know when.

Yours sincerely,
Vincent