Re: [RFC PATCH 1/2] drivers/misc: add silex multipk driver

[Krzysztof Kozlowski <krzk@kernel.org>]

On 12/03/2025 10:54, Nipun Gupta wrote:
> +
>  SILICON LABS WIRELESS DRIVERS (for WFxxx series)
>  M:	Jérôme Pouiller <jerome.pouiller@silabs.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