[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <51ed5660-10fe-4a6f-ad99-9741187341b1@kernel.org>
Date: Wed, 12 Mar 2025 11:54:23 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Nipun Gupta <nipun.gupta@....com>, linux-kernel@...r.kernel.org,
robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
derek.kiernan@....com, dragan.cvetic@....com, arnd@...db.de,
gregkh@...uxfoundation.org
Cc: praveen.jain@....com, harpreet.anand@....com, nikhil.agarwal@....com,
srivatsa@...il.mit.edu, code@...icks.com, ptsm@...ux.microsoft.com
Subject: Re: [RFC PATCH 1/2] drivers/misc: add silex multipk driver
On 12/03/2025 10:54, Nipun Gupta wrote:
> +
> SILICON LABS WIRELESS DRIVERS (for WFxxx series)
> M: Jérôme Pouiller <jerome.pouiller@...abs.com>
> S: Supported
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index 56bc72c7ce4a..8c5c72c540a6 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -632,6 +632,12 @@ config MCHP_LAN966X_PCI
> - lan966x-miim (MDIO_MSCC_MIIM)
> - lan966x-switch (LAN966X_SWITCH)
>
> +config SILEX_MPK
> + tristate "Silex MultiPK driver"
> + depends on OF
Why can't this be compile tested?
Which arch uses it? Which hardware?
> + help
> + Enable Silex MultiPK support
> +
> source "drivers/misc/c2port/Kconfig"
> source "drivers/misc/eeprom/Kconfig"
> source "drivers/misc/cb710/Kconfig"
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index 545aad06d088..456758b29f71 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -75,3 +75,4 @@ lan966x-pci-objs := lan966x_pci.o
> lan966x-pci-objs += lan966x_pci.dtbo.o
> obj-$(CONFIG_MCHP_LAN966X_PCI) += lan966x-pci.o
> obj-y += keba/
> +obj-$(CONFIG_SILEX_MPK) += silex_mpk.o
> diff --git a/drivers/misc/silex_mpk.c b/drivers/misc/silex_mpk.c
> new file mode 100644
> index 000000000000..e03579780761
> --- /dev/null
> +++ b/drivers/misc/silex_mpk.c
> @@ -0,0 +1,860 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2018-2021 Silex Insight sa
> + * Copyright (c) 2018-2021 Beerten Engineering scs
> + * Copyright (c) 2025 Advanced Micro Devices, Inc.
> + */
> +
> +#include <linux/version.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/device.h>
> +#include <linux/interrupt.h>
> +#include <linux/slab.h>
> +#include <linux/platform_device.h>
> +#include <linux/of.h>
Drop, won't be needed.
> +#include <linux/of_platform.h>
Where do you use it?
> +#include <linux/of_address.h>
Where do you use it?
> +#include <linux/cdev.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/io.h>
> +#include <linux/idr.h>
> +#include <linux/atomic.h>
> +#include <linux/delay.h>
> +#include <linux/eventfd.h>
> +#include <linux/kthread.h>
> +#include <linux/sched/signal.h>
> +#include <linux/bitops.h>
> +#include <uapi/linux/eventpoll.h>
> +#include <uapi/misc/silex_mpk.h>
> +
> +#include "silex_mpk_defs.h"
...
> +
> +static int multipk_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct multipk_dev *mpkdev;
> + struct resource *memres;
> + int irq, ret;
> +
> + ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
> + if (ret < 0)
> + return ret;
> +
> + mpkdev = devm_kzalloc(dev, sizeof(*mpkdev), GFP_KERNEL);
> + if (!mpkdev)
> + return -ENOMEM;
> + mpkdev->dev = dev;
> +
> + memres = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + mpkdev->regs = devm_ioremap_resource(dev, memres);
Use wrapper for both.
> + if (IS_ERR(mpkdev->regs))
> + return PTR_ERR(mpkdev->regs);
> + mpkdev->regsphys = memres->start;
> + memres = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> + mpkdev->dbrg_regs = devm_ioremap_resource(dev, memres);
Use wrapper for both.
> + if (IS_ERR(mpkdev->dbrg_regs))
> + return PTR_ERR(mpkdev->dbrg_regs);
> + mpkdev->dbrg_regsphys = memres->start;
> + platform_set_drvdata(pdev, mpkdev);
> +
> + /* Only a single IRQ is supported */
> + if (platform_irq_count(pdev) != 1)
> + return -ENODEV;
> +
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0)
> + return -ENODEV;
> +
> + return multipk_create_device(mpkdev, dev, irq);
> +}
> +
> +static void multipk_remove(struct platform_device *pdev)
> +{
> + struct multipk_dev *mpkdev = platform_get_drvdata(pdev);
> +
> + multipk_remove_device(mpkdev);
> +}
> +
> +static const struct of_device_id multipk_match[] = {
> + { .compatible = "multipk" },
NAK, you do not have such compatible.
> + { },
> +};
> +
> +static struct platform_driver multipk_pdrv = {
> + .probe = multipk_probe,
> + .remove = multipk_remove,
> + .driver = {
> + .name = DRIVER_NAME,
> + .of_match_table = of_match_ptr(multipk_match),
Drop of_match_ptr, you have warnings here.
> + },
> +};
> +
> +static int __init multipk_init(void)
> +{
> + dev_t devt;
> + int ret;
> +
> + multipk_class = class_create("multipk");
> + if (IS_ERR(multipk_class)) {
> + ret = PTR_ERR(multipk_class);
> + pr_err("can't register class\n");
> + goto err;
> + }
> + ret = alloc_chrdev_region(&devt, 0, MULTIPK_MAX_DEVICES, "multipk");
> + if (ret) {
> + pr_err("can't register character device\n");
> + goto err_class;
> + }
> + multipk_major = MAJOR(devt);
> + multipk_minor = MINOR(devt);
> +
> + ret = platform_driver_register(&multipk_pdrv);
> + if (ret) {
> + pr_err("can't register platform driver\n");
> + goto err_unchr;
> + }
> +
> + return 0;
> +err_unchr:
> + unregister_chrdev_region(devt, MULTIPK_MAX_DEVICES);
> +err_class:
> + class_destroy(multipk_class);
> +err:
> + return ret;
> +}
> +
> +static void __exit multipk_exit(void)
> +{
> + platform_driver_unregister(&multipk_pdrv);
> +
> + unregister_chrdev_region(MKDEV(multipk_major, 0), MULTIPK_MAX_DEVICES);
> +
> + class_destroy(multipk_class);
> +}
> +
> +module_init(multipk_init);
> +module_exit(multipk_exit);
I don't understand why loading a standard driver creates all this on my
device.
> +
> +MODULE_DESCRIPTION("Driver for Silex Multipk Asymmetric crypto accelerator");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:" DRIVER_NAME);
Drop
Best regards,
Krzysztof
Powered by blists - more mailing lists