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.

regards,
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 |