netdev - Re: [PATCH v5 5/6] can: ctucanfd: CTU CAN FD open-source IP core - platform and next steps and mainlining chances

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <202008251125.41514.pisa@cmp.felk.cvut.cz>
Date:   Tue, 25 Aug 2020 11:25:41 +0200
From:   Pavel Pisa <pisa@....felk.cvut.cz>
To:     Randy Dunlap <rdunlap@...radead.org>,
        "Marc Kleine-Budde" <mkl@...gutronix.de>,
        Rob Herring <robh+dt@...nel.org>,
        Drew Fustini <pdp7pdp7@...il.com>
Cc:     linux-can@...r.kernel.org, devicetree@...r.kernel.org,
        Oliver Hartkopp <socketcan@...tkopp.net>,
        Wolfgang Grandegger <wg@...ndegger.com>,
        David Miller <davem@...emloft.net>, mark.rutland@....com,
        Carsten Emde <c.emde@...dl.org>, armbru@...hat.com,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        Marin Jerabek <martin.jerabek01@...il.com>,
        Ondrej Ille <ondrej.ille@...il.com>,
        Jiri Novak <jnovak@....cvut.cz>,
        Jaroslav Beran <jara.beran@...il.com>,
        Petr Porazil <porazil@...ron.com>, Pavel Machek <pavel@....cz>
Subject: Re: [PATCH v5 5/6] can: ctucanfd: CTU CAN FD open-source IP core - platform and next steps and mainlining chances

Hello Randy and Rob,

thanks much for review, I have corrected FPGA spelling
and binding YAML license.

On Sunday 16 of August 2020 01:28:13 Randy Dunlap wrote:
> On 8/15/20 12:43 PM, Pavel Pisa wrote:
> > diff --git a/drivers/net/can/ctucanfd/Kconfig
> > b/drivers/net/can/ctucanfd/Kconfig index e1636373628a..a8c9cc38f216
> > 100644
> > --- a/drivers/net/can/ctucanfd/Kconfig
> > +++ b/drivers/net/can/ctucanfd/Kconfig
> > @@ -21,4 +21,15 @@ config CAN_CTUCANFD_PCI
> >  	  PCIe board with PiKRON.com designed transceiver riser shield is
> > available at https://gitlab.fel.cvut.cz/canbus/pcie-ctu_can_fd .
> >
> > +config CAN_CTUCANFD_PLATFORM
> > +	tristate "CTU CAN-FD IP core platform (FPGA, SoC) driver"
> > +	depends on OF
>
> Can this be
> 	depends on OF || COMPILE_TEST
> ?

I am not sure for this change. Is it ensured/documented somewhere that
header files provide dummy definition such way, that OF drivers builds
even if OF support is disabled? If I remember well, CTU CAN FD OF
module build fails if attempted in the frame of native x86_64
build where OF has been disabled. Does COMPILE_TEST ensure that
such build succeeds.

As for the next steps, I expect that without any review of Marc Kleine-Budde
or Wolfgang Grandegger from initial attempt for submission from February 2019,
we are at the end of the road now.

If there is confirmed preference, I would shorten license headers in the
C files, but I am not sure if SPDX-License-Identifier is recognized by 
copyright law and because code and CTU CAN FD IP can be used outside
of Linux kernel by others, we would like to keep legally binding preamble.
It is reduced by not listing address to obtain complete GPL-2.0 from anyway.
And change of preamble requires to update main repository, because
header files are generated from IP core IPXACT definition by Python
based tools. 

I am aware of only one other suggestion not followed yet and it
is separation of part of ctucan_tx_interrupt() function into new
one suggested by Pavel Machek. I agree that function length of 108
lines is big. When blank lines are removed we are on 68 lines and 28
lines are switch statement. The function consist of two nested loops.
External one required to ensure no lost interrupt when edge triggered
delivery or implementation is used. For me personally, it is more
readable in the actual format then to separate and propagate local
variables to another function. And particular function code received
only formatting and ctu_can_fd_ -> ctucan_hw_ rename in past year
so it is tested many/many times by manual PCI test and automated
Zynq tests. Each of the following pipelines which contains two jobs
ands by test of FPGA design and driver build and tests on real HW  

   https://gitlab.fel.cvut.cz/canbus/zynq/zynq-can-sja1000-top/pipelines

You can go through years of the testing and development back.
So I have even tendency to not shuffle code which does not
result in indisputable better readability and breaks more than year
of unmodified code successful (pass) test result line and confidence.

Because I understand that you all are loaded a lot I expect that after
ACK/review-by by Rob, there is no need to send v6 to
  devicetree@...r.kernel.org
I am not sure about cross-post to
  netdev@...r.kernel.org
  linux-kernel@...r.kernel.org
when the progress is stuck on
  linux-can@...r.kernel.org
Problem is that linux-can seems to eat core driver patch, probably because it 
is too long.

Thanks to all for patience and if somebody does want to be loaded by minor
updates, resends and pings to linux-can, send me note to not bother you
again.

Thanks for your time,

Pavel

PS: I would be available on Drew Fustini's LPC 2020
    BoF: upstream drivers for open source FPGA SoC peripherals 
    today. If there is interrest I can provide some information
    and show some overview and results.