AMD isp4 capture is a v4l2 media device which implements media controller
interface. It has one sub-device (AMD ISP4 sub-device) endpoint which can
be connected to a remote CSI2 TX endpoint. It supports only one physical
interface for now. Also add ISP4 driver related entry info into the
MAINTAINERS file
Co-developed-by: Svetoslav Stoilov <Svetoslav.Stoilov@amd.com>
Signed-off-by: Svetoslav Stoilov <Svetoslav.Stoilov@amd.com>
Signed-off-by: Bin Du <Bin.Du@amd.com>
Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
Tested-by: Alexey Zagorodnikov <xglooom@gmail.com>
---
MAINTAINERS | 13 +++
drivers/media/platform/Kconfig | 1 +
drivers/media/platform/Makefile | 1 +
drivers/media/platform/amd/Kconfig | 3 +
drivers/media/platform/amd/Makefile | 3 +
drivers/media/platform/amd/isp4/Kconfig | 14 +++
drivers/media/platform/amd/isp4/Makefile | 6 ++
drivers/media/platform/amd/isp4/isp4.c | 121 +++++++++++++++++++++++
drivers/media/platform/amd/isp4/isp4.h | 20 ++++
9 files changed, 182 insertions(+)
create mode 100644 drivers/media/platform/amd/Kconfig
create mode 100644 drivers/media/platform/amd/Makefile
create mode 100644 drivers/media/platform/amd/isp4/Kconfig
create mode 100644 drivers/media/platform/amd/isp4/Makefile
create mode 100644 drivers/media/platform/amd/isp4/isp4.c
create mode 100644 drivers/media/platform/amd/isp4/isp4.h
diff --git a/MAINTAINERS b/MAINTAINERS
index cd7ff55b5d32..3640a1e3262c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1133,6 +1133,19 @@ T: git git://git.kernel.org/pub/scm/linux/kernel/git/iommu/linux.git
F: drivers/iommu/amd/
F: include/linux/amd-iommu.h
+AMD ISP4 DRIVER
+M: Bin Du <bin.du@amd.com>
+M: Nirujogi Pratap <pratap.nirujogi@amd.com>
+L: linux-media@vger.kernel.org
+S: Maintained
+T: git git://linuxtv.org/media.git
+F: drivers/media/platform/amd/Kconfig
+F: drivers/media/platform/amd/Makefile
+F: drivers/media/platform/amd/isp4/Kconfig
+F: drivers/media/platform/amd/isp4/Makefile
+F: drivers/media/platform/amd/isp4/isp4.c
+F: drivers/media/platform/amd/isp4/isp4.h
+
AMD KFD
M: Felix Kuehling <Felix.Kuehling@amd.com>
L: amd-gfx@lists.freedesktop.org
diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index 9287faafdce5..772c70665510 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -63,6 +63,7 @@ config VIDEO_MUX
# Platform drivers - Please keep it alphabetically sorted
source "drivers/media/platform/allegro-dvt/Kconfig"
+source "drivers/media/platform/amd/Kconfig"
source "drivers/media/platform/amlogic/Kconfig"
source "drivers/media/platform/amphion/Kconfig"
source "drivers/media/platform/aspeed/Kconfig"
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index 6fd7db0541c7..b207bd8d8022 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -6,6 +6,7 @@
# Place here, alphabetically sorted by directory
# (e. g. LC_ALL=C sort Makefile)
obj-y += allegro-dvt/
+obj-y += amd/
obj-y += amlogic/
obj-y += amphion/
obj-y += aspeed/
diff --git a/drivers/media/platform/amd/Kconfig b/drivers/media/platform/amd/Kconfig
new file mode 100644
index 000000000000..25af49f246b2
--- /dev/null
+++ b/drivers/media/platform/amd/Kconfig
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0+
+
+source "drivers/media/platform/amd/isp4/Kconfig"
diff --git a/drivers/media/platform/amd/Makefile b/drivers/media/platform/amd/Makefile
new file mode 100644
index 000000000000..8bfc1955f22e
--- /dev/null
+++ b/drivers/media/platform/amd/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0+
+
+obj-y += isp4/
diff --git a/drivers/media/platform/amd/isp4/Kconfig b/drivers/media/platform/amd/isp4/Kconfig
new file mode 100644
index 000000000000..d4e4ad436600
--- /dev/null
+++ b/drivers/media/platform/amd/isp4/Kconfig
@@ -0,0 +1,14 @@
+# SPDX-License-Identifier: GPL-2.0+
+
+config AMD_ISP4
+ tristate "AMD ISP4 and camera driver"
+ depends on DRM_AMD_ISP && VIDEO_DEV
+ select VIDEOBUF2_CORE
+ select VIDEOBUF2_MEMOPS
+ select VIDEOBUF2_V4L2
+ select VIDEO_V4L2_SUBDEV_API
+ help
+ This is support for AMD ISP4 and camera subsystem driver.
+ Say Y here to enable the ISP4 and camera device for video capture.
+ To compile this driver as a module, choose M here. The module will
+ be called amd_capture.
diff --git a/drivers/media/platform/amd/isp4/Makefile b/drivers/media/platform/amd/isp4/Makefile
new file mode 100644
index 000000000000..de0092dad26f
--- /dev/null
+++ b/drivers/media/platform/amd/isp4/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Copyright (C) 2025 Advanced Micro Devices, Inc.
+
+obj-$(CONFIG_AMD_ISP4) += amd_capture.o
+amd_capture-objs := isp4.o
diff --git a/drivers/media/platform/amd/isp4/isp4.c b/drivers/media/platform/amd/isp4/isp4.c
new file mode 100644
index 000000000000..a3fc2462d70f
--- /dev/null
+++ b/drivers/media/platform/amd/isp4/isp4.c
@@ -0,0 +1,121 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2025 Advanced Micro Devices, Inc.
+ */
+
+#include <linux/pm_runtime.h>
+#include <linux/vmalloc.h>
+#include <media/v4l2-ioctl.h>
+
+#include "isp4.h"
+
+#define VIDEO_BUF_NUM 5
+
+#define ISP4_DRV_NAME "amd_isp_capture"
+
+const char *isp4_irq_name[] = {
+ "isp_irq_stream1",
+ "isp_irq_global"
+};
+
+/* interrupt num */
+static const u32 isp4_ringbuf_interrupt_num[] = {
+ 0, /* ISP_4_1__SRCID__ISP_RINGBUFFER_WPT9 */
+ 4, /* ISP_4_1__SRCID__ISP_RINGBUFFER_WPT12 */
+};
+
+static irqreturn_t isp4_irq_handler(int irq, void *arg)
+{
+ return IRQ_HANDLED;
+}
+
+static int isp4_capture_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct isp4_device *isp_dev;
+ int i, irq, ret;
+
+ isp_dev = devm_kzalloc(dev, sizeof(*isp_dev), GFP_KERNEL);
+ if (!isp_dev)
+ return -ENOMEM;
+
+ isp_dev->pdev = pdev;
+ dev->init_name = ISP4_DRV_NAME;
+
+ for (i = 0; i < ARRAY_SIZE(isp4_ringbuf_interrupt_num); i++) {
+ irq = platform_get_irq(pdev, isp4_ringbuf_interrupt_num[i]);
+ if (irq < 0)
+ return dev_err_probe(dev, irq,
+ "fail to get irq %d\n",
+ isp4_ringbuf_interrupt_num[i]);
+ ret = devm_request_irq(dev, irq, isp4_irq_handler, 0,
+ isp4_irq_name[i], dev);
+ if (ret)
+ return dev_err_probe(dev, ret, "fail to req irq %d\n",
+ irq);
+ }
+
+ /* Link the media device within the v4l2_device */
+ isp_dev->v4l2_dev.mdev = &isp_dev->mdev;
+
+ /* Initialize media device */
+ strscpy(isp_dev->mdev.model, "amd_isp41_mdev",
+ sizeof(isp_dev->mdev.model));
+ snprintf(isp_dev->mdev.bus_info, sizeof(isp_dev->mdev.bus_info),
+ "platform:%s", ISP4_DRV_NAME);
+ isp_dev->mdev.dev = dev;
+ media_device_init(&isp_dev->mdev);
+
+ /* register v4l2 device */
+ snprintf(isp_dev->v4l2_dev.name, sizeof(isp_dev->v4l2_dev.name),
+ "AMD-V4L2-ROOT");
+ ret = v4l2_device_register(dev, &isp_dev->v4l2_dev);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "fail register v4l2 device\n");
+
+ ret = media_device_register(&isp_dev->mdev);
+ if (ret) {
+ dev_err(dev, "fail to register media device %d\n", ret);
+ goto err_unreg_v4l2;
+ }
+
+ platform_set_drvdata(pdev, isp_dev);
+
+ pm_runtime_set_suspended(dev);
+ pm_runtime_enable(dev);
+
+ return 0;
+
+err_unreg_v4l2:
+ v4l2_device_unregister(&isp_dev->v4l2_dev);
+
+ return dev_err_probe(dev, ret, "isp probe fail\n");
+}
+
+static void isp4_capture_remove(struct platform_device *pdev)
+{
+ struct isp4_device *isp_dev = platform_get_drvdata(pdev);
+
+ media_device_unregister(&isp_dev->mdev);
+ v4l2_device_unregister(&isp_dev->v4l2_dev);
+}
+
+static struct platform_driver isp4_capture_drv = {
+ .probe = isp4_capture_probe,
+ .remove = isp4_capture_remove,
+ .driver = {
+ .name = ISP4_DRV_NAME,
+ .owner = THIS_MODULE,
+ }
+};
+
+module_platform_driver(isp4_capture_drv);
+
+MODULE_ALIAS("platform:" ISP4_DRV_NAME);
+MODULE_IMPORT_NS("DMA_BUF");
+
+MODULE_DESCRIPTION("AMD ISP4 Driver");
+MODULE_AUTHOR("Bin Du <bin.du@amd.com>");
+MODULE_AUTHOR("Pratap Nirujogi <pratap.nirujogi@amd.com>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/media/platform/amd/isp4/isp4.h b/drivers/media/platform/amd/isp4/isp4.h
new file mode 100644
index 000000000000..326b8094e99e
--- /dev/null
+++ b/drivers/media/platform/amd/isp4/isp4.h
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (C) 2025 Advanced Micro Devices, Inc.
+ */
+
+#ifndef _ISP4_H_
+#define _ISP4_H_
+
+#include <media/v4l2-device.h>
+#include <media/videobuf2-memops.h>
+
+#define ISP4_GET_ISP_REG_BASE(isp4sd) (((isp4sd))->mmio)
+
+struct isp4_device {
+ struct v4l2_device v4l2_dev;
+ struct media_device mdev;
+ struct platform_device *pdev;
+};
+
+#endif /* _ISP4_H_ */
--
2.34.1On 24/10/2025 11:06, Bin Du wrote:
> diff --git a/drivers/media/platform/amd/isp4/isp4.c b/drivers/media/platform/amd/isp4/isp4.c
> new file mode 100644
> index 000000000000..a3fc2462d70f
> --- /dev/null
> +++ b/drivers/media/platform/amd/isp4/isp4.c
> @@ -0,0 +1,121 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2025 Advanced Micro Devices, Inc.
> + */
> +
> +#include <linux/pm_runtime.h>
> +#include <linux/vmalloc.h>
> +#include <media/v4l2-ioctl.h>
> +
> +#include "isp4.h"
> +
> +#define VIDEO_BUF_NUM 5
> +
> +#define ISP4_DRV_NAME "amd_isp_capture"
> +
> +const char *isp4_irq_name[] = {
Why isn't this static?
> + "isp_irq_stream1",
> + "isp_irq_global"
> +};
> +
> +/* interrupt num */
> +static const u32 isp4_ringbuf_interrupt_num[] = {
> + 0, /* ISP_4_1__SRCID__ISP_RINGBUFFER_WPT9 */
> + 4, /* ISP_4_1__SRCID__ISP_RINGBUFFER_WPT12 */
> +};
> +
> +static irqreturn_t isp4_irq_handler(int irq, void *arg)
> +{
> + return IRQ_HANDLED;
> +}
> +
> +static int isp4_capture_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct isp4_device *isp_dev;
> + int i, irq, ret;
> +
> + isp_dev = devm_kzalloc(dev, sizeof(*isp_dev), GFP_KERNEL);
> + if (!isp_dev)
> + return -ENOMEM;
> +
> + isp_dev->pdev = pdev;
> + dev->init_name = ISP4_DRV_NAME;
> +
> + for (i = 0; i < ARRAY_SIZE(isp4_ringbuf_interrupt_num); i++) {
> + irq = platform_get_irq(pdev, isp4_ringbuf_interrupt_num[i]);
> + if (irq < 0)
> + return dev_err_probe(dev, irq,
> + "fail to get irq %d\n",
> + isp4_ringbuf_interrupt_num[i]);
> + ret = devm_request_irq(dev, irq, isp4_irq_handler, 0,
> + isp4_irq_name[i], dev);
> + if (ret)
> + return dev_err_probe(dev, ret, "fail to req irq %d\n",
> + irq);
> + }
> +
> + /* Link the media device within the v4l2_device */
> + isp_dev->v4l2_dev.mdev = &isp_dev->mdev;
> +
> + /* Initialize media device */
> + strscpy(isp_dev->mdev.model, "amd_isp41_mdev",
> + sizeof(isp_dev->mdev.model));
> + snprintf(isp_dev->mdev.bus_info, sizeof(isp_dev->mdev.bus_info),
> + "platform:%s", ISP4_DRV_NAME);
> + isp_dev->mdev.dev = dev;
> + media_device_init(&isp_dev->mdev);
> +
> + /* register v4l2 device */
> + snprintf(isp_dev->v4l2_dev.name, sizeof(isp_dev->v4l2_dev.name),
> + "AMD-V4L2-ROOT");
> + ret = v4l2_device_register(dev, &isp_dev->v4l2_dev);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "fail register v4l2 device\n");
> +
> + ret = media_device_register(&isp_dev->mdev);
> + if (ret) {
> + dev_err(dev, "fail to register media device %d\n", ret);
> + goto err_unreg_v4l2;
> + }
> +
> + platform_set_drvdata(pdev, isp_dev);
> +
> + pm_runtime_set_suspended(dev);
> + pm_runtime_enable(dev);
> +
> + return 0;
> +
> +err_unreg_v4l2:
> + v4l2_device_unregister(&isp_dev->v4l2_dev);
> +
> + return dev_err_probe(dev, ret, "isp probe fail\n");
No, don't print generic error thus multiple errors. Drop this and keep
informative dev_err_probe() in applicable places.
> +}
> +
> +static void isp4_capture_remove(struct platform_device *pdev)
> +{
> + struct isp4_device *isp_dev = platform_get_drvdata(pdev);
> +
> + media_device_unregister(&isp_dev->mdev);
> + v4l2_device_unregister(&isp_dev->v4l2_dev);
> +}
> +
> +static struct platform_driver isp4_capture_drv = {
> + .probe = isp4_capture_probe,
> + .remove = isp4_capture_remove,
> + .driver = {
> + .name = ISP4_DRV_NAME,
> + .owner = THIS_MODULE,
It is v5 but you still did not run standard tools. You try to upstream
12 year old code without cleaning it up.
Please run standard kernel tools for static analysis, like coccinelle,
smatch and sparse, and fix reported warnings. Also please check for
warnings when building with W=1 for gcc and clang. Most of these
commands (checks or W=1 build) can build specific targets, like some
directory, to narrow the scope to only your code. The code here looks
like it needs a fix. Feel free to get in touch if the warning is not clear.
> + }
> +};
> +
> +module_platform_driver(isp4_capture_drv);
> +
> +MODULE_ALIAS("platform:" ISP4_DRV_NAME);
You should not need MODULE_ALIAS() in normal cases. If you need it,
usually it means your device ID table is wrong (e.g. misses either
entries or MODULE_DEVICE_TABLE()). MODULE_ALIAS() is not a substitute
for incomplete ID table.
Best regards,
Krzysztof
Many thanks for the review, Krzysztof
On 10/24/2025 9:57 PM, Krzysztof Kozlowski wrote:
> On 24/10/2025 11:06, Bin Du wrote:
>> diff --git a/drivers/media/platform/amd/isp4/isp4.c b/drivers/media/platform/amd/isp4/isp4.c
>> new file mode 100644
>> index 000000000000..a3fc2462d70f
>> --- /dev/null
>> +++ b/drivers/media/platform/amd/isp4/isp4.c
>> @@ -0,0 +1,121 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Copyright (C) 2025 Advanced Micro Devices, Inc.
>> + */
>> +
>> +#include <linux/pm_runtime.h>
>> +#include <linux/vmalloc.h>
>> +#include <media/v4l2-ioctl.h>
>> +
>> +#include "isp4.h"
>> +
>> +#define VIDEO_BUF_NUM 5
>> +
>> +#define ISP4_DRV_NAME "amd_isp_capture"
>> +
>> +const char *isp4_irq_name[] = {
>
> Why isn't this static?
>
Sure, will set it to static and check other similar variables.
>> + "isp_irq_stream1",
>> + "isp_irq_global"
>> +};
>> +
>> +/* interrupt num */
>> +static const u32 isp4_ringbuf_interrupt_num[] = {
>> + 0, /* ISP_4_1__SRCID__ISP_RINGBUFFER_WPT9 */
>> + 4, /* ISP_4_1__SRCID__ISP_RINGBUFFER_WPT12 */
>> +};
>> +
>> +static irqreturn_t isp4_irq_handler(int irq, void *arg)
>> +{
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static int isp4_capture_probe(struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + struct isp4_device *isp_dev;
>> + int i, irq, ret;
>> +
>> + isp_dev = devm_kzalloc(dev, sizeof(*isp_dev), GFP_KERNEL);
>> + if (!isp_dev)
>> + return -ENOMEM;
>> +
>> + isp_dev->pdev = pdev;
>> + dev->init_name = ISP4_DRV_NAME;
>> +
>> + for (i = 0; i < ARRAY_SIZE(isp4_ringbuf_interrupt_num); i++) {
>> + irq = platform_get_irq(pdev, isp4_ringbuf_interrupt_num[i]);
>> + if (irq < 0)
>> + return dev_err_probe(dev, irq,
>> + "fail to get irq %d\n",
>> + isp4_ringbuf_interrupt_num[i]);
>> + ret = devm_request_irq(dev, irq, isp4_irq_handler, 0,
>> + isp4_irq_name[i], dev);
>> + if (ret)
>> + return dev_err_probe(dev, ret, "fail to req irq %d\n",
>> + irq);
>> + }
>> +
>> + /* Link the media device within the v4l2_device */
>> + isp_dev->v4l2_dev.mdev = &isp_dev->mdev;
>> +
>> + /* Initialize media device */
>> + strscpy(isp_dev->mdev.model, "amd_isp41_mdev",
>> + sizeof(isp_dev->mdev.model));
>> + snprintf(isp_dev->mdev.bus_info, sizeof(isp_dev->mdev.bus_info),
>> + "platform:%s", ISP4_DRV_NAME);
>> + isp_dev->mdev.dev = dev;
>> + media_device_init(&isp_dev->mdev);
>> +
>> + /* register v4l2 device */
>> + snprintf(isp_dev->v4l2_dev.name, sizeof(isp_dev->v4l2_dev.name),
>> + "AMD-V4L2-ROOT");
>> + ret = v4l2_device_register(dev, &isp_dev->v4l2_dev);
>> + if (ret)
>> + return dev_err_probe(dev, ret,
>> + "fail register v4l2 device\n");
>> +
>> + ret = media_device_register(&isp_dev->mdev);
>> + if (ret) {
>> + dev_err(dev, "fail to register media device %d\n", ret);
>> + goto err_unreg_v4l2;
>> + }
>> +
>> + platform_set_drvdata(pdev, isp_dev);
>> +
>> + pm_runtime_set_suspended(dev);
>> + pm_runtime_enable(dev);
>> +
>> + return 0;
>> +
>> +err_unreg_v4l2:
>> + v4l2_device_unregister(&isp_dev->v4l2_dev);
>> +
>> + return dev_err_probe(dev, ret, "isp probe fail\n");
>
> No, don't print generic error thus multiple errors. Drop this and keep
> informative dev_err_probe() in applicable places.
>
>
Very good suggestion, will implement it accordingly.
>> +}
>> +
>> +static void isp4_capture_remove(struct platform_device *pdev)
>> +{
>> + struct isp4_device *isp_dev = platform_get_drvdata(pdev);
>> +
>> + media_device_unregister(&isp_dev->mdev);
>> + v4l2_device_unregister(&isp_dev->v4l2_dev);
>> +}
>> +
>> +static struct platform_driver isp4_capture_drv = {
>> + .probe = isp4_capture_probe,
>> + .remove = isp4_capture_remove,
>> + .driver = {
>> + .name = ISP4_DRV_NAME,
>> + .owner = THIS_MODULE,
>
> It is v5 but you still did not run standard tools. You try to upstream
> 12 year old code without cleaning it up.
>
yes, no need to set .owner because the core will do it. Will clean it up.
> Please run standard kernel tools for static analysis, like coccinelle,
> smatch and sparse, and fix reported warnings. Also please check for
> warnings when building with W=1 for gcc and clang. Most of these
> commands (checks or W=1 build) can build specific targets, like some
> directory, to narrow the scope to only your code. The code here looks
> like it needs a fix. Feel free to get in touch if the warning is not clear.
>
Really appreciate the instructions, will set up what you suggested and
address the warnings. Suppose it can catch the issues you mentioned above.
>> + }
>> +};
>> +
>> +module_platform_driver(isp4_capture_drv);
>> +
>> +MODULE_ALIAS("platform:" ISP4_DRV_NAME);
>
> You should not need MODULE_ALIAS() in normal cases. If you need it,
> usually it means your device ID table is wrong (e.g. misses either
> entries or MODULE_DEVICE_TABLE()). MODULE_ALIAS() is not a substitute
> for incomplete ID table.
>
Thanks for the clarification, yes, MODULE_ALIAS() does not apply in our
case and should be removed.
>
> Best regards,
> Krzysztof
--
Regards,
Bin
On 28/10/2025 09:30, Du, Bin wrote:
>>> + }
>>> +};
>>> +
>>> +module_platform_driver(isp4_capture_drv);
>>> +
>>> +MODULE_ALIAS("platform:" ISP4_DRV_NAME);
>>
>> You should not need MODULE_ALIAS() in normal cases. If you need it,
>> usually it means your device ID table is wrong (e.g. misses either
>> entries or MODULE_DEVICE_TABLE()). MODULE_ALIAS() is not a substitute
>> for incomplete ID table.
>>
>
> Thanks for the clarification, yes, MODULE_ALIAS() does not apply in our
> case and should be removed.
You don't have ID table, so probably this should be fixed, unless it is
instantiated by some other driver, e.g. as MFD. That's the only typical
use case for alias in the drivers.
Best regards,
Krzysztof
Thanks Krzysztof.
On 10/28/2025 4:41 PM, Krzysztof Kozlowski wrote:
> On 28/10/2025 09:30, Du, Bin wrote:
>>>> + }
>>>> +};
>>>> +
>>>> +module_platform_driver(isp4_capture_drv);
>>>> +
>>>> +MODULE_ALIAS("platform:" ISP4_DRV_NAME);
>>>
>>> You should not need MODULE_ALIAS() in normal cases. If you need it,
>>> usually it means your device ID table is wrong (e.g. misses either
>>> entries or MODULE_DEVICE_TABLE()). MODULE_ALIAS() is not a substitute
>>> for incomplete ID table.
>>>
>>
>> Thanks for the clarification, yes, MODULE_ALIAS() does not apply in our
>> case and should be removed.
>
>
> You don't have ID table, so probably this should be fixed, unless it is
> instantiated by some other driver, e.g. as MFD. That's the only typical
> use case for alias in the drivers.
>
Yes, our device is a MFD, so alias works for it, as alternative, we can
add module device table, it should work either, could you please
indicate your preferred option?
> Best regards,
> Krzysztof
--
Regards,
Bin
On 28/10/2025 10:00, Du, Bin wrote:
> Thanks Krzysztof.
>
> On 10/28/2025 4:41 PM, Krzysztof Kozlowski wrote:
>> On 28/10/2025 09:30, Du, Bin wrote:
>>>>> + }
>>>>> +};
>>>>> +
>>>>> +module_platform_driver(isp4_capture_drv);
>>>>> +
>>>>> +MODULE_ALIAS("platform:" ISP4_DRV_NAME);
>>>>
>>>> You should not need MODULE_ALIAS() in normal cases. If you need it,
>>>> usually it means your device ID table is wrong (e.g. misses either
>>>> entries or MODULE_DEVICE_TABLE()). MODULE_ALIAS() is not a substitute
>>>> for incomplete ID table.
>>>>
>>>
>>> Thanks for the clarification, yes, MODULE_ALIAS() does not apply in our
>>> case and should be removed.
>>
>>
>> You don't have ID table, so probably this should be fixed, unless it is
>> instantiated by some other driver, e.g. as MFD. That's the only typical
>> use case for alias in the drivers.
>>
>
> Yes, our device is a MFD, so alias works for it, as alternative, we can
> add module device table, it should work either, could you please
> indicate your preferred option?
>
So it is spawned by drivers/gpu/drm/amd/amdgpu/isp_v4_1_0.c or
drivers/gpu/drm/amd/amdgpu/isp_v4_1_1.c ?
If so, then your code is fine, that's correct use of MODULE_ALIAS.
Best regards,
Krzysztof
On 10/28/2025 5:06 PM, Krzysztof Kozlowski wrote:
> On 28/10/2025 10:00, Du, Bin wrote:
>> Thanks Krzysztof.
>>
>> On 10/28/2025 4:41 PM, Krzysztof Kozlowski wrote:
>>> On 28/10/2025 09:30, Du, Bin wrote:
>>>>>> + }
>>>>>> +};
>>>>>> +
>>>>>> +module_platform_driver(isp4_capture_drv);
>>>>>> +
>>>>>> +MODULE_ALIAS("platform:" ISP4_DRV_NAME);
>>>>>
>>>>> You should not need MODULE_ALIAS() in normal cases. If you need it,
>>>>> usually it means your device ID table is wrong (e.g. misses either
>>>>> entries or MODULE_DEVICE_TABLE()). MODULE_ALIAS() is not a substitute
>>>>> for incomplete ID table.
>>>>>
>>>>
>>>> Thanks for the clarification, yes, MODULE_ALIAS() does not apply in our
>>>> case and should be removed.
>>>
>>>
>>> You don't have ID table, so probably this should be fixed, unless it is
>>> instantiated by some other driver, e.g. as MFD. That's the only typical
>>> use case for alias in the drivers.
>>>
>>
>> Yes, our device is a MFD, so alias works for it, as alternative, we can
>> add module device table, it should work either, could you please
>> indicate your preferred option?
>>
>
>
> So it is spawned by drivers/gpu/drm/amd/amdgpu/isp_v4_1_0.c or
> drivers/gpu/drm/amd/amdgpu/isp_v4_1_1.c ?
>
yes, it's spawned by drivers/gpu/drm/amd/amdgpu/isp_v4_1_1.c
> If so, then your code is fine, that's correct use of MODULE_ALIAS.
>
Thanks for the quick confirmation, so, we'll keep it.
> Best regards,
> Krzysztof
--
Regards,
Bin
Hi Krzysztof,
We’ve set up the clang build and kernel static analysis tools and found
additional warnings aside from those you noted. We will resolve these
issues and submit version 6, including fixes for other version 5 review
comments, by end of this week.
Many thanks.
On 10/28/2025 5:10 PM, Du, Bin wrote:
>
>
> On 10/28/2025 5:06 PM, Krzysztof Kozlowski wrote:
>> On 28/10/2025 10:00, Du, Bin wrote:
>>> Thanks Krzysztof.
>>>
>>> On 10/28/2025 4:41 PM, Krzysztof Kozlowski wrote:
>>>> On 28/10/2025 09:30, Du, Bin wrote:
>>>>>>> + }
>>>>>>> +};
>>>>>>> +
>>>>>>> +module_platform_driver(isp4_capture_drv);
>>>>>>> +
>>>>>>> +MODULE_ALIAS("platform:" ISP4_DRV_NAME);
>>>>>>
>>>>>> You should not need MODULE_ALIAS() in normal cases. If you need it,
>>>>>> usually it means your device ID table is wrong (e.g. misses either
>>>>>> entries or MODULE_DEVICE_TABLE()). MODULE_ALIAS() is not a substitute
>>>>>> for incomplete ID table.
>>>>>>
>>>>>
>>>>> Thanks for the clarification, yes, MODULE_ALIAS() does not apply in
>>>>> our
>>>>> case and should be removed.
>>>>
>>>>
>>>> You don't have ID table, so probably this should be fixed, unless it is
>>>> instantiated by some other driver, e.g. as MFD. That's the only typical
>>>> use case for alias in the drivers.
>>>>
>>>
>>> Yes, our device is a MFD, so alias works for it, as alternative, we can
>>> add module device table, it should work either, could you please
>>> indicate your preferred option?
>>>
>>
>>
>> So it is spawned by drivers/gpu/drm/amd/amdgpu/isp_v4_1_0.c or
>> drivers/gpu/drm/amd/amdgpu/isp_v4_1_1.c ?
>>
>
> yes, it's spawned by drivers/gpu/drm/amd/amdgpu/isp_v4_1_1.c
>
>> If so, then your code is fine, that's correct use of MODULE_ALIAS.
>>
>
> Thanks for the quick confirmation, so, we'll keep it.
>
>> Best regards,
>> Krzysztof
>
--
Regards,
Bin
© 2016 - 2026 Red Hat, Inc.