| Message ID | 4d5c53499bafe7717815f948801bd5aedaa05c12.1647904780.git.pisa@cmp.felk.cvut.cz (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | CTU CAN FD open-source IP core SocketCAN driver, PCI, platform integration and documentation | expand |
Hi Pavel, On Tue, Mar 22, 2022 at 1:06 AM Pavel Pisa <pisa@cmp.felk.cvut.cz> wrote: > 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> Thanks for your patch, which is now commit e8f0c23a2415fa8f ("can: ctucanfd: CTU CAN FD open-source IP core - platform/SoC support.") in linux-can-next/master. > --- /dev/null > +++ b/drivers/net/can/ctucanfd/ctucanfd_platform.c > +/* Match table for OF platform binding */ > +static const struct of_device_id ctucan_of_match[] = { > + { .compatible = "ctu,ctucanfd-2", }, Do you need to match on the above compatible value? The driver seems to treat the hardware the same, and the DT bindings state the compatible value below should always be present. > + { .compatible = "ctu,ctucanfd", }, > + { /* end of list */ }, > +}; > +MODULE_DEVICE_TABLE(of, ctucan_of_match); Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hello Geert, On Tuesday 03 of May 2022 13:37:46 Geert Uytterhoeven wrote: > Hi Pavel, > > --- /dev/null > > +++ b/drivers/net/can/ctucanfd/ctucanfd_platform.c > > > > +/* Match table for OF platform binding */ > > +static const struct of_device_id ctucan_of_match[] = { > > + { .compatible = "ctu,ctucanfd-2", }, > > Do you need to match on the above compatible value? > The driver seems to treat the hardware the same, and the DT > bindings state the compatible value below should always be present. I would keep it because there will be newer revisions and releases of the core and I consider "ctu,ctucanfd" as the match to generic one with maximal attempt to adjust to the version from provided info registers but identification with the fixed version "ctu,ctucanfd-2" ensures that some old hardware which is in the wild is directly recognized even at /sys level and if we need to do some workarounds for autodetection etc. it can be recognized. I understand that in ideal world that should never be required if we keep compatability right and if there is really new major completely incompatible revision we would need to use new identifier anyway. But we and the world is not perfect, so I would keep that safety option. May it be there are designs with 5 years old IP core version in the wild and may it be even with this ID.... May be they need some version specific adjustment... Yes version should be readable from IP, hopefully, but that was experimental version and I am not sure how much register map changed from that days (can be analyzed from GIT if support request appears). You know how it works with feebacks when chain of subcontracted companies does integration for carmaker etc... For example there appears questions and interest to update it to actual Debian for LinCAN after ten or 15 years of silence because that or origina CAN drivers solution works somewhere... So I am personally more inclined to keep versionned ID. Best wishes, Pavel
On 03.05.2022 17:07:21, Pavel Pisa wrote: > Hello Geert, > > On Tuesday 03 of May 2022 13:37:46 Geert Uytterhoeven wrote: > > Hi Pavel, > > > --- /dev/null > > > +++ b/drivers/net/can/ctucanfd/ctucanfd_platform.c > > > > > > +/* Match table for OF platform binding */ > > > +static const struct of_device_id ctucan_of_match[] = { > > > + { .compatible = "ctu,ctucanfd-2", }, > > > > Do you need to match on the above compatible value? > > The driver seems to treat the hardware the same, and the DT > > bindings state the compatible value below should always be present. > > I would keep it because there will be newer revisions and releases > of the core and I consider "ctu,ctucanfd" as the match to generic > one with maximal attempt to adjust to the version from provided > info registers but identification with the fixed version > "ctu,ctucanfd-2" ensures that some old hardware which is > in the wild is directly recognized even at /sys level > and if we need to do some workarounds for autodetection > etc. it can be recognized. As Geert said: - There are 2 bindings in the driver which are (currently) treated the same. - The binding documentation says devices must always have the ctu,ctucanfd compatible. This means (currently) the ctu,ctucanfd-2 is not needed in the driver. We can add it back once we need it. Or are there devices that have a compatible of ctu,ctucanfd-2 without stating to be compatible with ctu,ctucanfd? regards, Marc
diff --git a/drivers/net/can/ctucanfd/Kconfig b/drivers/net/can/ctucanfd/Kconfig index d6e59522f4cf0..48963efc7f199 100644 --- a/drivers/net/can/ctucanfd/Kconfig +++ b/drivers/net/can/ctucanfd/Kconfig @@ -20,3 +20,15 @@ config CAN_CTUCANFD_PCI The project providing FPGA design for Intel EP4CGX15 based DB4CGX15 PCIe board with PiKRON.com designed transceiver riser shield is available at https://gitlab.fel.cvut.cz/canbus/pcie-ctucanfd . + +config CAN_CTUCANFD_PLATFORM + tristate "CTU CAN-FD IP core platform (FPGA, SoC) driver" + depends on CAN_CTUCANFD + 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/wiki/courses/b35apo/documentation/mz_apo/start . diff --git a/drivers/net/can/ctucanfd/Makefile b/drivers/net/can/ctucanfd/Makefile index 48555c3511e7c..8078f1f2c30fc 100644 --- a/drivers/net/can/ctucanfd/Makefile +++ b/drivers/net/can/ctucanfd/Makefile @@ -7,3 +7,4 @@ obj-$(CONFIG_CAN_CTUCANFD) := ctucanfd.o ctucanfd-y := ctucanfd_base.o obj-$(CONFIG_CAN_CTUCANFD_PCI) += ctucanfd_pci.o +obj-$(CONFIG_CAN_CTUCANFD_PLATFORM) += ctucanfd_platform.o diff --git a/drivers/net/can/ctucanfd/ctucanfd_platform.c b/drivers/net/can/ctucanfd/ctucanfd_platform.c new file mode 100644 index 0000000000000..5e48060686622 --- /dev/null +++ b/drivers/net/can/ctucanfd/ctucanfd_platform.c @@ -0,0 +1,132 @@ +// 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-2021 Ondrej Ille <ondrej.ille@gmail.com> self-funded + * Copyright (C) 2018-2019 Martin Jerabek <martin.jerabek01@gmail.com> FEE CTU + * Copyright (C) 2018-2022 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/) + ******************************************************************************/ + +#include <linux/module.h> +#include <linux/netdevice.h> +#include <linux/of.h> +#include <linux/platform_device.h> +#include <linux/pm_runtime.h> + +#include "ctucanfd.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");