netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Pavel Pisa <pisa@cmp.felk.cvut.cz>
To: Pavel Machek <pavel@ucw.cz>
Cc: linux-can@vger.kernel.org, devicetree@vger.kernel.org,
	"Marc Kleine-Budde" <mkl@pengutronix.de>,
	Oliver Hartkopp <socketcan@hartkopp.net>,
	Wolfgang Grandegger <wg@grandegger.com>,
	David Miller <davem@davemloft.net>,
	Rob Herring <robh+dt@kernel.org>,
	mark.rutland@arm.com, Carsten Emde <c.emde@osadl.org>,
	armbru@redhat.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	Marin Jerabek <martin.jerabek01@gmail.com>,
	Ondrej Ille <ondrej.ille@gmail.com>,
	Jiri Novak <jnovak@fel.cvut.cz>,
	Jaroslav Beran <jara.beran@gmail.com>,
	Petr Porazil <porazil@pikron.com>,
	Drew Fustini <pdp7pdp7@gmail.com>,
	Randy Dunlap <rdunlap@infradead.org>
Subject: Re: [PATCH v6 5/6] can: ctucanfd: CTU CAN FD open-source IP core - platform/SoC support.
Date: Thu, 22 Oct 2020 18:06:19 +0200	[thread overview]
Message-ID: <202010221806.19253.pisa@cmp.felk.cvut.cz> (raw)
In-Reply-To: <20201022114306.GA31933@duo.ucw.cz>

Hello Pavel,

thanks for review.

On Thursday 22 of October 2020 13:43:06 Pavel Machek wrote:
> Hi!
>
> > +++ 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 || COMPILE_TEST
> > +	help
>
> This is likely wrong, as it can enable config of CAN_CTUCANFD=M,
> CAN_CTUCANFD_PLATFORM=y, right?

My original code has not || COMPILE_TEST alternative.

But I have been asked to add it

On Sunday 16 of August 2020 01:28:13 Randy Dunlap wrote:
> Can this be
>         depends on OF || COMPILE_TEST
> ?

I have send discussion later that I am not sure if it is right
but followed suggestion. If there is no other reply now,
I would drop || COMPILE_TEST. I believe that then it is correct
for regular use. I ma not sure about all consequences of COMPILE_TEST
missing.

> > @@ -8,3 +8,6 @@ ctucanfd-y := ctu_can_fd.o ctu_can_fd_hw.o
> >
> >  obj-$(CONFIG_CAN_CTUCANFD_PCI) += ctucanfd_pci.o
> >  ctucanfd_pci-y := ctu_can_fd_pci.o
> > +
> > +obj-$(CONFIG_CAN_CTUCANFD_PLATFORM) += ctucanfd_platform.o
> > +ctucanfd_platform-y += ctu_can_fd_platform.o
>
> Can you simply add right object files directly?

This is more tough question. We have kept sources
as ctu_can_fd.c, ctu_can_fd_hw.c etc. to produce
final ctucanfd.ko which matches device tree entry etc.
after name simplification now...
So we move from underscores to ctucanfd on more places.
So yes, we can rename ctu_can_fd.c to ctucanfd_drv.c + others
keep final ctucanfd.ko and change to single file based objects
ctucanfd_platform.c and ctucanfd_pci.c

If you think that it worth to be redone, I would do that.
It would disrupt sources history, may it be blames, merging
etc... but I would invest effort into it if asked for. 

Best wishes,

                Pavel

  reply	other threads:[~2020-10-22 16:07 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-22  8:36 [PATCH v6 0/6] CTU CAN FD open-source IP core SocketCAN driver, PCI, platform integration and documentation Pavel Pisa
2020-10-22  8:36 ` [PATCH v6 1/6] dt-bindings: vendor-prefix: add prefix for the Czech Technical University in Prague Pavel Pisa
2020-10-22  8:36 ` [PATCH v6 2/6] dt-bindings: net: can: binding for CTU CAN FD open-source IP core Pavel Pisa
2020-10-22  8:58   ` Pavel Machek
2020-10-22  8:36 ` [PATCH v6 3/6] can: ctucanfd: add support for CTU CAN FD open-source IP core - bus independent part Pavel Pisa
2020-10-22 11:02   ` Pavel Machek
2020-10-22 20:21     ` Pavel Pisa
     [not found]       ` <CAA7Zjpam0uFCXwXS4_X5Sq3wJcNUSxOxPiTm860OXDNs-xHgyg@mail.gmail.com>
2020-10-26 16:11         ` Pavel Pisa
2020-10-26 13:04     ` Pavel Pisa
2020-10-22  8:36 ` [PATCH v6 4/6] can: ctucanfd: CTU CAN FD open-source IP core - PCI bus support Pavel Pisa
2020-10-22 11:39   ` Pavel Machek
2020-10-22 16:19     ` Pavel Pisa
2020-10-22  8:36 ` [PATCH v6 5/6] can: ctucanfd: CTU CAN FD open-source IP core - platform/SoC support Pavel Pisa
2020-10-22 11:43   ` Pavel Machek
2020-10-22 16:06     ` Pavel Pisa [this message]
2020-10-22 20:50       ` Pavel Machek
2020-10-22  8:36 ` [PATCH v6 6/6] docs: ctucanfd: CTU CAN FD open-source IP core documentation Pavel Pisa
2020-10-22 11:25   ` Pavel Machek
2020-10-22 15:24     ` Pavel Pisa

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=202010221806.19253.pisa@cmp.felk.cvut.cz \
    --to=pisa@cmp.felk.cvut.cz \
    --cc=armbru@redhat.com \
    --cc=c.emde@osadl.org \
    --cc=davem@davemloft.net \
    --cc=devicetree@vger.kernel.org \
    --cc=jara.beran@gmail.com \
    --cc=jnovak@fel.cvut.cz \
    --cc=linux-can@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=martin.jerabek01@gmail.com \
    --cc=mkl@pengutronix.de \
    --cc=netdev@vger.kernel.org \
    --cc=ondrej.ille@gmail.com \
    --cc=pavel@ucw.cz \
    --cc=pdp7pdp7@gmail.com \
    --cc=porazil@pikron.com \
    --cc=rdunlap@infradead.org \
    --cc=robh+dt@kernel.org \
    --cc=socketcan@hartkopp.net \
    --cc=wg@grandegger.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).