[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250902151828.GU2163762@google.com>
Date: Tue, 2 Sep 2025 16:18:28 +0100
From: Lee Jones <lee@...nel.org>
To: Marcos Del Sol Vives <marcos@...a.pet>
Cc: linux-kernel@...r.kernel.org, Linus Walleij <linus.walleij@...aro.org>,
Bartosz Golaszewski <brgl@...ev.pl>,
Michael Walle <mwalle@...nel.org>,
Bjorn Helgaas <bhelgaas@...gle.com>, linux-gpio@...r.kernel.org,
linux-pci@...r.kernel.org
Subject: Re: [PATCH v4 3/3] mfd: vortex: implement new driver for Vortex
southbridges
On Fri, 22 Aug 2025, Marcos Del Sol Vives wrote:
> This new driver loads resources related to southbridges available in DM&P
> Vortex devices, currently only the GPIO pins.
>
> Signed-off-by: Marcos Del Sol Vives <marcos@...a.pet>
> ---
> MAINTAINERS | 1 +
> drivers/mfd/Kconfig | 9 +++
> drivers/mfd/Makefile | 1 +
> drivers/mfd/vortex-sb.c | 135 ++++++++++++++++++++++++++++++++++++++++
> include/linux/pci_ids.h | 3 +
> 5 files changed, 149 insertions(+)
> create mode 100644 drivers/mfd/vortex-sb.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index afa88f446630..63e410e23e95 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -26957,6 +26957,7 @@ VORTEX HARDWARE SUPPORT
> R: Marcos Del Sol Vives <marcos@...a.pet>
> S: Maintained
> F: drivers/gpio/gpio-vortex.c
> +F: drivers/mfd/vortex-sb.c
>
> VRF
> M: David Ahern <dsahern@...nel.org>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 425c5fba6cb1..fe54bb22687d 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -2008,6 +2008,15 @@ config MFD_VX855
> VIA VX855/VX875 south bridge. You will need to enable the vx855_spi
> and/or vx855_gpio drivers for this to do anything useful.
>
> +config MFD_VORTEX_SB
> + tristate "Vortex southbridge"
> + select MFD_CORE
> + depends on PCI
> + help
> + Say yes here if you want to have support for the southbridge
> + present on Vortex SoCs. You will need to enable the vortex-gpio
> + driver for this to do anything useful.
> +
> config MFD_ARIZONA
> select REGMAP
> select REGMAP_IRQ
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index f7bdedd5a66d..2504ba311f1a 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -202,6 +202,7 @@ obj-$(CONFIG_MFD_JANZ_CMODIO) += janz-cmodio.o
> obj-$(CONFIG_MFD_TPS6586X) += tps6586x.o
> obj-$(CONFIG_MFD_VX855) += vx855.o
> obj-$(CONFIG_MFD_WL1273_CORE) += wl1273-core.o
> +obj-$(CONFIG_MFD_VORTEX_SB) += vortex-sb.o
>
> si476x-core-y := si476x-cmd.o si476x-prop.o si476x-i2c.o
> obj-$(CONFIG_MFD_SI476X_CORE) += si476x-core.o
> diff --git a/drivers/mfd/vortex-sb.c b/drivers/mfd/vortex-sb.c
> new file mode 100644
> index 000000000000..141fa19773e2
> --- /dev/null
> +++ b/drivers/mfd/vortex-sb.c
> @@ -0,0 +1,135 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * MFD southbridge driver for Vortex SoCs
Drop references to MFD.
Call it "Core southbridge ..."
> + *
> + * Author: Marcos Del Sol Vives <marcos@...a.pet>
> + *
> + * Based on the RDC321x MFD driver by Florian Fainelli and Bernhard Loos
Drop this.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/platform_device.h>
> +#include <linux/pci.h>
> +#include <linux/mfd/core.h>
Alphabetical.
> +
> +struct vortex_southbridge {
> + const struct mfd_cell *cells;
> + int n_cells;
> +};
Why is this needed?
> +/* Layout for Vortex86DX/MX */
> +static const struct resource vortex_dx_gpio_resources[] = {
> + {
> + .name = "dat1",
> + .start = 0x78,
Define _all_ magic numbers.
> + .end = 0x7C,
> + .flags = IORESOURCE_IO,
> + }, {
> + .name = "dir1",
> + .start = 0x98,
> + .end = 0x9C,
> + .flags = IORESOURCE_IO,
> + }
Use DEFINE_RES_*() macros.
> +};
> +
> +static const struct mfd_cell vortex_dx_sb_cells[] = {
> + {
> + .name = "vortex-gpio",
> + .resources = vortex_dx_gpio_resources,
> + .num_resources = ARRAY_SIZE(vortex_dx_gpio_resources),
> + },
> +};
It's not an MFD until you have more than one device.
> +static const struct vortex_southbridge vortex_dx_sb = {
> + .cells = vortex_dx_sb_cells,
> + .n_cells = ARRAY_SIZE(vortex_dx_sb_cells),
> +};
> +
> +/* Layout for Vortex86DX2/DX3 */
> +static const struct resource vortex_dx2_gpio_resources[] = {
> + {
> + .name = "dat1",
> + .start = 0x78,
> + .end = 0x7C,
> + .flags = IORESOURCE_IO,
> + }, {
> + .name = "dat2",
> + .start = 0x100,
> + .end = 0x105,
> + .flags = IORESOURCE_IO,
> + }, {
> + .name = "dir1",
> + .start = 0x98,
> + .end = 0x9D,
> + .flags = IORESOURCE_IO,
> + }, {
> + .name = "dir2",
> + .start = 0x93,
> + .end = 0x97,
> + .flags = IORESOURCE_IO,
> + }
> +};
As above.
> +static const struct mfd_cell vortex_dx2_sb_cells[] = {
> + {
> + .name = "vortex-gpio",
> + .resources = vortex_dx2_gpio_resources,
> + .num_resources = ARRAY_SIZE(vortex_dx2_gpio_resources),
> + },
> +};
> +
> +static const struct vortex_southbridge vortex_dx2_sb = {
> + .cells = vortex_dx2_sb_cells,
> + .n_cells = ARRAY_SIZE(vortex_dx2_sb_cells),
> +};
> +
> +static int vortex_sb_probe(struct pci_dev *pdev,
> + const struct pci_device_id *ent)
Why line-feed here and not 2 lines down?
> +{
> + struct vortex_southbridge *priv = (struct vortex_southbridge *) ent->driver_data;
s/priv/ddata/
> + int err;
> +
> + /*
> + * In the Vortex86DX3, the southbridge appears twice (on both 00:07.0
> + * and 00:07.1). Register only once for .0.
> + *
> + * Other Vortex boards (eg Vortex86MX+) have the southbridge exposed
> + * only once, also at 00:07.0.
> + */
> + if (PCI_FUNC(pdev->devfn) != 0)
> + return -ENODEV;
> +
> + err = pci_enable_device(pdev);
> + if (err) {
> + dev_err(&pdev->dev, "failed to enable device\n");
> + return err;
> + }
> +
> + return devm_mfd_add_devices(&pdev->dev, PLATFORM_DEVID_NONE,
> + priv->cells, priv->n_cells,
> + NULL, 0, NULL);
> +}
> +
> +static const struct pci_device_id vortex_sb_table[] = {
> + /* Vortex86DX */
> + { PCI_DEVICE_DATA(RDC, R6031, &vortex_dx_sb) },
We're not passing one initialisation API's data (MFD) through another (PCI).
Pass a device ID (if you don't already have one) and match on that instead.
> + /* Vortex86DX2/DX3 */
> + { PCI_DEVICE_DATA(RDC, R6035, &vortex_dx2_sb) },
> + /* Vortex86MX */
> + { PCI_DEVICE_DATA(RDC, R6036, &vortex_dx_sb) },
> + {}
> +};
> +MODULE_DEVICE_TABLE(pci, vortex_sb_table);
> +
> +static struct pci_driver vortex_sb_driver = {
> + .name = "vortex-sb",
> + .id_table = vortex_sb_table,
> + .probe = vortex_sb_probe,
> +};
> +
Remove this line.
> +module_pci_driver(vortex_sb_driver);
> +
> +MODULE_AUTHOR("Marcos Del Sol Vives <marcos@...a.pet>");
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Vortex MFD southbridge driver");
> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> index 92ffc4373f6d..2c7afebd94ea 100644
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -2412,6 +2412,9 @@
> #define PCI_VENDOR_ID_RDC 0x17f3
> #define PCI_DEVICE_ID_RDC_R6020 0x6020
> #define PCI_DEVICE_ID_RDC_R6030 0x6030
> +#define PCI_DEVICE_ID_RDC_R6031 0x6031
> +#define PCI_DEVICE_ID_RDC_R6035 0x6035
> +#define PCI_DEVICE_ID_RDC_R6036 0x6036
> #define PCI_DEVICE_ID_RDC_R6040 0x6040
> #define PCI_DEVICE_ID_RDC_R6060 0x6060
> #define PCI_DEVICE_ID_RDC_R6061 0x6061
> --
> 2.34.1
>
--
Lee Jones [李琼斯]
Powered by blists - more mailing lists