Re: [PATCH v4 2/6] dt-bindings: net: can: binding for CTU CAN FD open-source IP core.

Pavel Pisa
2020-08-06T17:55:49Z

Hello Pavel and Rob,

thanks much for review.

On Thursday 06 of August 2020 16:47:13 Rob Herring wrote:
> On Tue, Aug 04, 2020 at 11:20:21AM +0200, Pavel Machek wrote:
> > On Tue 2020-08-04 11:18:17, Pavel Machek wrote:
> > > Hi!
> > >
> > > > The commit text again to make checkpatch happy.
> > >
> > > ?

The checkpatch reports as a problem when there is no description
of the patch. At least for patch

[PATCH v4 1/6] dt-bindings: vendor-prefix: add prefix for the Czech Technical University in Prague.

I consider that little pontificate but I have fullfiled its suggestion
with remark, that in this case, It is not my intention to add these
promotions. I remove the reference to patchcheck from these commit messages.

> > > > + oneOf:
> > > > + - items:
> > > > + - const: ctu,ctucanfd
> > > > + - const: ctu,canfd-2
> > > > + - const: ctu,ctucanfd
> > >
> > > For consistency, can we have ctu,canfd-1, ctu,canfd-2?
> >
> > Make it ctu,ctucanfd-1, ctu,ctucanfd-2... to make it consistent with
> > the file names.
>
> If you are going to do version numbers, please define where they come
> from. Hopefully some tag of the h/w IP version...
>
> Better yet, put version numbers in the h/w registers itself and you
> don't need different compatibles.

The actual major version of the core is 2. The minor intended
for release was 1. But we wait for driver inclusion and release
and IP core release has not been realized. Sources moved to
2.2-pre version and compiled core reports 2.2 now.
There is added control bit for protocol exception
behavior selection and minor enhancements in sync of standard
and data rate bittimes starts.

Yes, version can be obtained from hardware.
There is magic and version in the first core register.
See 3.1.1 DEVICE_ID section of the manual (page 22/28)

http://canbus.pages.fel.cvut.cz/ctucanfd_ip_core/Progdokum.pdf

As for the DT identifier we use "ctu,ctucanfd" in more projects already.
Some devices are in the wild now. So I would prefer to keep compatibility
with that name. Other name reflects that this driver is compatible with major
version 2 of the core. It can be "ctu,ctucanfd-2". I am not sure if the
repeat of "ctu" is good idea, but yes, full sources prefix is "ctucanfd".
The second alias can be omitted alltogether. But I am not sure, there can
be one day fundamental change between IP core versions which would be better
handled by change of PCI ID and DT ID. It is questionable if attempt to keep
single driver for more too different versions would be more manageable
or convoluted than two fully independent ones. May it be we do not need
to solve that because by that time it would be "ctu,ctucanxl".

At this time, our actual first first choic for the IP core identifier
is ctu,ctucanfd.

As for the pointed description, I would remove them from version 5
according to your reference. My personal one is to keep documentation
(even of actual/local functional setup) directly in the sources and mainline
to find it out when I or somebody else need to recreate or update designs,
my biological memory is already worn out by past events.

I am not sure if I should wait for subsystem maintainers review now
or sent new patches version. I may get to its preparation tommorrow
or may it be later because I want to take some time in
countrysite/mountains.

Best wishes

Pavel
--
Pavel Pisa
phone: +420 603531357
e-mail: pisa@cmp.felk.cvut.cz
Department of Control Engineering FEE CVUT
Karlovo namesti 13, 121 35, Prague 2
university: http://dce.fel.cvut.cz/
personal: http://cmp.felk.cvut.cz/~pisa
projects: https://www.openhub.net/accounts/ppisa
CAN related:http://canbus.pages.fel.cvut.cz/