Re: [PATCH v4 3/3] mfd: vortex: implement new driver for Vortex southbridges

[Lee Jones <lee@kernel.org>]

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@orca.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@orca.pet>
>  S:	Maintained
>  F:	drivers/gpio/gpio-vortex.c
> +F:	drivers/mfd/vortex-sb.c
>  
>  VRF
>  M:	David Ahern <dsahern@kernel.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@orca.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@orca.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 [李琼斯]