| 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 |
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
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
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 --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");