Re: [RFC PATCH 12/14] can: netlink: add CAN XL support

[Oliver Hartkopp <socketcan@hartkopp.net>]

On 04.12.24 12:44, Marc Kleine-Budde wrote:
> On 04.12.2024 12:35:43, Oliver Hartkopp wrote:
>>>>> Also, the main reason for not creating the nest was that I thought
>>>>> that the current bittiming API was stable. I was not aware of the
>>>>> current flaw on how to divide tseg1_min. Maybe we should first discuss
>>>>> how to solve this issue for CAN FD?
>>>>
>>>> I like the current way how you added the CAN XL support.
>>>> It maintains the known usage pattern - and the way how CAN XL bit timings
>>>> are defined is identical to CAN FD (including TDC).
>>>>
>>>> Is the separation of propseg and tseg1 that relevant?
>>>> Does it really need to be exposed to the user?
>>>
>>> There are IIRC at least 2 CAN-FD cores where the prop segment and phase
>>> segment 1 for the data bit timing have not the same width. This means we
>>> have to change the bittiming_const in the kernel.

Sure?

In the end (almost) every CAN controller has the tseg1 register which 
contains prop_seg + phase_seg1 as a sum of these.

The relevant point is behind prop_seg + phase_seg1 and I'm pretty sure 
these "2 CAN-FD cores" will add the values internally too.

I'm a bit concerned that after 40 years someone shows up with the idea 
to spend two registers for the tseg1 value instead of one.

As a both values rely on the same tq can't we just split the tseg1 into 
prop_seg + phase_seg1 values with some common e.g. 70:30 pattern?

IMO changing the bittiming API has no value for the user just to satisfy 
the "2 CAN-FD cores" that came late to the party.

>>> A struct in netlink means we cannot change it.
>>
>> But are we not already in this situation with CAN FD that we can not change
>> the bittiming (const) in the userspace API?
> 
> Yes, we have to support it. But we can add a new nested type that
> serializes the individual members of an improved struct bittiming_const.
> The old user space tools will just keep working, iproute2 can be updated
> to use the new bittiming_const if it's available and fall back to the
> existing one.

Ok. Nice - but maybe obsolete due to my question above ;-)

Best regards,
Oliver