diff mbox series

[v6,5/6] can: ctucanfd: CTU CAN FD open-source IP core - platform/SoC support.

Message ID 2a90e1a7d57f0fec42604cd399acf25af5689148.1603354744.git.pisa@cmp.felk.cvut.cz (mailing list archive)
State Not Applicable
Delegated to: Netdev Maintainers
Headers show
Series CTU CAN FD open-source IP core SocketCAN driver, PCI, platform integration and documentation | expand

Commit Message

Pavel Pisa Oct. 22, 2020, 8:36 a.m. UTC
Platform bus adaptation for CTU CAN FD open-source IP core.

The core has been tested together with OpenCores SJA1000
modified to be CAN FD frames tolerant on MicroZed Zynq based
MZ_APO education kits designed by Petr Porazil from PiKRON.com
company. FPGA design

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

The kit description at the Computer Architectures course pages

  https://cw.fel.cvut.cz/wiki/courses/b35apo/documentation/mz_apo/start .

Kit carrier board and mechanics design source files

  https://gitlab.com/pikron/projects/mz_apo/microzed_apo

The work is documented in Martin Jeřábek's diploma theses
Open-source and Open-hardware CAN FD Protocol Support

  https://dspace.cvut.cz/handle/10467/80366
.

Signed-off-by: Pavel Pisa <pisa@cmp.felk.cvut.cz>
Signed-off-by: Martin Jerabek <martin.jerabek01@gmail.com>
Signed-off-by: Ondrej Ille <ondrej.ille@gmail.com>
---
 drivers/net/can/ctucanfd/Kconfig              |  11 ++
 drivers/net/can/ctucanfd/Makefile             |   3 +
 .../net/can/ctucanfd/ctu_can_fd_platform.c    | 142 ++++++++++++++++++
 3 files changed, 156 insertions(+)
 create mode 100644 drivers/net/can/ctucanfd/ctu_can_fd_platform.c

Comments

Pavel Machek Oct. 22, 2020, 11:43 a.m. UTC | #1
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?

> @@ -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?

Best regards,
							Pavel
Pavel Pisa Oct. 22, 2020, 4:06 p.m. UTC | #2
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
Pavel Machek Oct. 22, 2020, 8:50 p.m. UTC | #3
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.

COMPILE_TEST is not a problem. But you need to make this depend on
main CONFIG_ option to disallow CAN_CTUCANFD=M,
CAN_CTUCANFD_PLATFORM=y combination.

> > > @@ -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.

git can handle renames. Or you can use the new names for module
names...?

Best regards,
							Pavel
diff mbox series

Patch

diff --git a/drivers/net/can/ctucanfd/Kconfig b/drivers/net/can/ctucanfd/Kconfig
index fb4d3cda6885..01fcfe5873cc 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 || COMPILE_TEST
+	help
+	  The core has been tested together with OpenCores SJA1000
+	  modified to be CAN FD frames tolerant on MicroZed Zynq based
+	  MZ_APO education kits designed by Petr Porazil from PiKRON.com
+	  company. FPGA design https://gitlab.fel.cvut.cz/canbus/zynq/zynq-can-sja1000-top.
+	  The kit description at the Computer Architectures course pages
+	  https://cw.fel.cvut.cz/b182/courses/b35apo/documentation/mz_apo/start .
+
 endif
diff --git a/drivers/net/can/ctucanfd/Makefile b/drivers/net/can/ctucanfd/Makefile
index eb945260952d..a77ca72d534e 100644
--- a/drivers/net/can/ctucanfd/Makefile
+++ b/drivers/net/can/ctucanfd/Makefile
@@ -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
diff --git a/drivers/net/can/ctucanfd/ctu_can_fd_platform.c b/drivers/net/can/ctucanfd/ctu_can_fd_platform.c
new file mode 100644
index 000000000000..c35b16b8566b
--- /dev/null
+++ b/drivers/net/can/ctucanfd/ctu_can_fd_platform.c
@@ -0,0 +1,142 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*******************************************************************************
+ *
+ * CTU CAN FD IP Core
+ *
+ * Copyright (C) 2015-2018 Ondrej Ille <ondrej.ille@gmail.com> FEE CTU
+ * Copyright (C) 2018-2020 Ondrej Ille <ondrej.ille@gmail.com> self-funded
+ * Copyright (C) 2018-2019 Martin Jerabek <martin.jerabek01@gmail.com> FEE CTU
+ * Copyright (C) 2018-2020 Pavel Pisa <pisa@cmp.felk.cvut.cz> FEE CTU/self-funded
+ *
+ * Project advisors:
+ *     Jiri Novak <jnovak@fel.cvut.cz>
+ *     Pavel Pisa <pisa@cmp.felk.cvut.cz>
+ *
+ * Department of Measurement         (http://meas.fel.cvut.cz/)
+ * Faculty of Electrical Engineering (http://www.fel.cvut.cz)
+ * Czech Technical University        (http://www.cvut.cz/)
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ ******************************************************************************/
+
+#include <linux/module.h>
+#include <linux/netdevice.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+
+#include "ctu_can_fd.h"
+
+#define DRV_NAME	"ctucanfd"
+
+static void ctucan_platform_set_drvdata(struct device *dev,
+					struct net_device *ndev)
+{
+	struct platform_device *pdev = container_of(dev, struct platform_device,
+						    dev);
+
+	platform_set_drvdata(pdev, ndev);
+}
+
+/**
+ * ctucan_platform_probe - Platform registration call
+ * @pdev:	Handle to the platform device structure
+ *
+ * This function does all the memory allocation and registration for the CAN
+ * device.
+ *
+ * Return: 0 on success and failure value on error
+ */
+static int ctucan_platform_probe(struct platform_device *pdev)
+{
+	struct resource *res; /* IO mem resources */
+	struct device	*dev = &pdev->dev;
+	void __iomem *addr;
+	int ret;
+	unsigned int ntxbufs;
+	int irq;
+
+	/* Get the virtual base address for the device */
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	addr = devm_ioremap_resource(dev, res);
+	if (IS_ERR(addr)) {
+		dev_err(dev, "Cannot remap address.\n");
+		ret = PTR_ERR(addr);
+		goto err;
+	}
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		dev_err(dev, "Cannot find interrupt.\n");
+		ret = irq;
+		goto err;
+	}
+
+	/* Number of tx bufs might be change in HW for future. If so,
+	 * it will be passed as property via device tree
+	 */
+	ntxbufs = 4;
+	ret = ctucan_probe_common(dev, addr, irq, ntxbufs, 0,
+				  1, ctucan_platform_set_drvdata);
+
+	if (ret < 0)
+		platform_set_drvdata(pdev, NULL);
+
+err:
+	return ret;
+}
+
+/**
+ * ctucan_platform_remove - Unregister the device after releasing the resources
+ * @pdev:	Handle to the platform device structure
+ *
+ * This function frees all the resources allocated to the device.
+ * Return: 0 always
+ */
+static int ctucan_platform_remove(struct platform_device *pdev)
+{
+	struct net_device *ndev = platform_get_drvdata(pdev);
+	struct ctucan_priv *priv = netdev_priv(ndev);
+
+	netdev_dbg(ndev, "ctucan_remove");
+
+	unregister_candev(ndev);
+	pm_runtime_disable(&pdev->dev);
+	netif_napi_del(&priv->napi);
+	free_candev(ndev);
+
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(ctucan_platform_pm_ops, ctucan_suspend, ctucan_resume);
+
+/* Match table for OF platform binding */
+static const struct of_device_id ctucan_of_match[] = {
+	{ .compatible = "ctu,ctucanfd-2", },
+	{ .compatible = "ctu,ctucanfd", },
+	{ /* end of list */ },
+};
+MODULE_DEVICE_TABLE(of, ctucan_of_match);
+
+static struct platform_driver ctucanfd_driver = {
+	.probe	= ctucan_platform_probe,
+	.remove	= ctucan_platform_remove,
+	.driver	= {
+		.name = DRV_NAME,
+		.pm = &ctucan_platform_pm_ops,
+		.of_match_table	= ctucan_of_match,
+	},
+};
+
+module_platform_driver(ctucanfd_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Martin Jerabek");
+MODULE_DESCRIPTION("CTU CAN FD for platform");