[PATCH v3 0/7] Add support for the LAN966x PCI device using a DT overlay

[PATCH v3 0/7] Add support for the LAN966x PCI device using a DT overlay

[Herve Codina]

Hi,

This series adds support for the LAN966x chip when used as a PCI
device.

For reference, the LAN996x chip is a System-on-chip that integrates an
Ethernet switch and a number of other traditional hardware blocks such
as a GPIO controller, I2C controllers, SPI controllers, etc. The
LAN996x can be used in two different modes:

- With Linux running on its Linux built-in ARM cores.
  This mode is already supported by the upstream Linux kernel, with the
  LAN996x described as a standard ARM Device Tree in
  arch/arm/boot/dts/microchip/lan966x.dtsi. Thanks to this support,
  all hardware blocks in the LAN996x already have drivers in the
  upstream Linux kernel.

- As a PCI device, thanks to its built-in PCI endpoint controller.
  In this case, the LAN996x ARM cores are not used, but all peripherals
  of the LAN996x can be accessed by the PCI host using memory-mapped
  I/O through the PCI BARs.

This series aims at supporting this second use-case. As all peripherals
of the LAN996x already have drivers in the Linux kernel, our goal is to
re-use them as-is to support this second use-case.

Therefore, this patch series introduces a PCI driver that binds on the
LAN996x PCI VID/PID, and when probed, instantiates all devices that are
accessible through the PCI BAR. As the list and characteristics of such
devices are non-discoverable, this PCI driver loads a Device Tree
overlay that allows to teach the kernel about which devices are
available, and allows to probe the relevant drivers in kernel, re-using
all existing drivers with no change.

This patch series for now adds a Device Tree overlay that describes an
initial subset of the devices available over PCI in the LAN996x, and
follow-up patch series will add support for more once this initial
support has landed.

In order to add this PCI driver, a number of preparation changes are
needed:

 - Patches 1 to 5 allow the reset driver used for the LAN996x to be
   built as a module. Indeed, in the case where Linux runs on the ARM
   cores, it is common to have the reset driver built-in. However, when
   the LAN996x is used as a PCI device, it makes sense that all its
   drivers can be loaded as modules.

 - Patches 6 and 7 introduce the LAN996x PCI driver itself, together
   with its DT bindings.

We believe all items from the above list can be merged separately, with
no build dependencies. We expect:

 - Patches 1 to 5 to be taken by reset maintainers

 - Patch 6 and 7 by the MFD maintainers

Additionally, we also believe all preparation items in this patch series
can be taken even before there's a final agreement on the last part of
the series (the MFD driver itself).

[1] https://lore.kernel.org/all/CAL_Jsq+je7+9ATR=B6jXHjEJHjn24vQFs4Tvi9=vhDeK9n42Aw@mail.gmail.com/

Compare to the previous iteration:
  https://lore.kernel.org/lkml/20240614173232.1184015-1-herve.codina@bootlin.com/
this v3 series mainly:
  - Suppress patches as they were applied or extracted and handled in
    dedicated series.
  - Update the LAN966x PCI device driver.

Best regards,
Hervé

Changes v2 -> v3
  - Patches 1 and 5
    No changes

  - Patch 6 (v2 patch 18)
    Add a blank line in the commit log to split paragraphs
    Remove unneeded header file inclusion
    Use IRQ_RETVAL()
    Remove blank line
    Use dev_of_node()
    Use pci_{set,get}_drvdata()
    Remove unneeded pci_clear_master() call
    Move { 0, } to { }
    Remove the unneeded pci_dev member from the lan966x_pci structure
    Use PCI_VENDOR_ID_EFAR instead of the hardcoded 0x1055 PCI Vendor ID
    Add a comment related to the of_node check.

  - Patch 7 (v2 patch 19)
    No changes

  Patches removed in v3
    - Patches 6 and 7
      Extracted and sent separately
      https://lore.kernel.org/lkml/20240620120126.412323-1-herve.codina@bootlin.com/

    - Patches 9
      Already applied

    - Patches 8, 10 to 12
      Extracted, reworked and sent separately
      https://lore.kernel.org/lkml/20240614173232.1184015-1-herve.codina@bootlin.com/

    - Patches 13 to 14
      Already applied

Changes v1 -> v2
  - Patch 1
    Fix a typo in syscon.h (s/intline/inline/)

  - Patches 2..5
    No changes

  - Patch 6
    Improve the reset property description

  - Patch 7
    Fix a wrong reverse x-mass tree declaration

  - Patch 8 removed (sent alone to net)
    https://lore.kernel.org/lkml/20240513111853.58668-1-herve.codina@bootlin.com/

  - Patch 8 (v1 patch 9)
    Add 'Reviewed-by: Rob Herring (Arm) <robh@kernel.org>'

  - Patch 9 (v1 patch 10)
    Rephrase and ident parameters descriptions

  - Patch 10 (v1 patch 11)
    No changes

  - Patch 11 (v1 patch 12)
    Fix a missing ret value assignment before a goto in .probe()
    Limit lines to 80 columns
    Use indices in register offset definitions

  - Patch 13 and 14 (new patches in v2)
    Add new test cases for existing of_changeset_add_prop_*()

  - Patch 15 (v1 patch 14)
    No changes

  - Patch 16 (new patches in v2)
    Add tests for of_changeset_add_prop_bool()

  - Patch 17 (v1 patch 15)
    Update commit subject
    Rewrap a paragraph in commit log

  - Patch 18 (v1 patch 16)
    Use PCI_IRQ_INTX instead of PCI_IRQ_LEGACY

  - Patch 19 (v1 patch 17)
    No changes

Clément Léger (5):
  mfd: syscon: Add reference counting and device managed support
  reset: mchp: sparx5: Remove dependencies and allow building as a
    module
  reset: mchp: sparx5: Release syscon when not use anymore
  reset: core: add get_device()/put_device on rcdev
  reset: mchp: sparx5: set the dev member of the reset controller

Herve Codina (2):
  mfd: Add support for LAN966x PCI device
  MAINTAINERS: Add the Microchip LAN966x PCI driver entry

 MAINTAINERS                            |   6 +
 drivers/mfd/Kconfig                    |  24 +++
 drivers/mfd/Makefile                   |   4 +
 drivers/mfd/lan966x_pci.c              | 229 +++++++++++++++++++++++++
 drivers/mfd/lan966x_pci.dtso           | 167 ++++++++++++++++++
 drivers/mfd/syscon.c                   | 145 +++++++++++++++-
 drivers/pci/quirks.c                   |   1 +
 drivers/reset/Kconfig                  |   3 +-
 drivers/reset/core.c                   |   2 +
 drivers/reset/reset-microchip-sparx5.c |  11 +-
 include/linux/mfd/syscon.h             |  18 ++
 11 files changed, 593 insertions(+), 17 deletions(-)
 create mode 100644 drivers/mfd/lan966x_pci.c
 create mode 100644 drivers/mfd/lan966x_pci.dtso

-- 
2.45.0

[PATCH v3 1/7] mfd: syscon: Add reference counting and device managed support

[Herve Codina]

From: Clément Léger <clement.leger@bootlin.com>

Syscon releasing is not supported.
Without release function, unbinding a driver that uses syscon whether
explicitly or due to a module removal left the used syscon in a in-use
state.

For instance a syscon_node_to_regmap() call from a consumer retrieve a
syscon regmap instance. Internally, syscon_node_to_regmap() can create
syscon instance and add it to the existing syscon list. No API is
available to release this syscon instance, remove it from the list and
free it when it is not used anymore.

Introduce reference counting in syscon in order to keep track of syscon
usage using syscon_{get,put}() and add a device managed version of
syscon_regmap_lookup_by_phandle(), to automatically release the syscon
instance on the consumer removal.

Signed-off-by: Clément Léger <clement.leger@bootlin.com>
Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 drivers/mfd/syscon.c       | 145 ++++++++++++++++++++++++++++++++++---
 include/linux/mfd/syscon.h |  18 +++++
 2 files changed, 154 insertions(+), 9 deletions(-)

diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c
index 7d0e91164cba..86898831b842 100644
--- a/drivers/mfd/syscon.c
+++ b/drivers/mfd/syscon.c
@@ -34,6 +34,7 @@ struct syscon {
 	struct regmap *regmap;
 	struct reset_control *reset;
 	struct list_head list;
+	struct kref refcount;
 };
 
 static const struct regmap_config syscon_regmap_config = {
@@ -147,6 +148,8 @@ static struct syscon *of_syscon_register(struct device_node *np, bool check_res)
 
 	syscon->regmap = regmap;
 	syscon->np = np;
+	of_node_get(syscon->np);
+	kref_init(&syscon->refcount);
 
 	spin_lock(&syscon_list_slock);
 	list_add_tail(&syscon->list, &syscon_list);
@@ -168,7 +171,30 @@ static struct syscon *of_syscon_register(struct device_node *np, bool check_res)
 	return ERR_PTR(ret);
 }
 
-static struct regmap *device_node_get_regmap(struct device_node *np,
+static void syscon_free(struct kref *kref)
+{
+	struct syscon *syscon = container_of(kref, struct syscon, refcount);
+
+	spin_lock(&syscon_list_slock);
+	list_del(&syscon->list);
+	spin_unlock(&syscon_list_slock);
+
+	regmap_exit(syscon->regmap);
+	of_node_put(syscon->np);
+	kfree(syscon);
+}
+
+static void syscon_get(struct syscon *syscon)
+{
+	kref_get(&syscon->refcount);
+}
+
+static void syscon_put(struct syscon *syscon)
+{
+	kref_put(&syscon->refcount, syscon_free);
+}
+
+static struct syscon *device_node_get_syscon(struct device_node *np,
 					     bool check_res)
 {
 	struct syscon *entry, *syscon = NULL;
@@ -183,9 +209,23 @@ static struct regmap *device_node_get_regmap(struct device_node *np,
 
 	spin_unlock(&syscon_list_slock);
 
-	if (!syscon)
+	if (!syscon) {
 		syscon = of_syscon_register(np, check_res);
+		if (IS_ERR(syscon))
+			return ERR_CAST(syscon);
+	} else {
+		syscon_get(syscon);
+	}
+
+	return syscon;
+}
 
+static struct regmap *device_node_get_regmap(struct device_node *np,
+					     bool check_res)
+{
+	struct syscon *syscon;
+
+	syscon = device_node_get_syscon(np, check_res);
 	if (IS_ERR(syscon))
 		return ERR_CAST(syscon);
 
@@ -198,12 +238,23 @@ struct regmap *device_node_to_regmap(struct device_node *np)
 }
 EXPORT_SYMBOL_GPL(device_node_to_regmap);
 
-struct regmap *syscon_node_to_regmap(struct device_node *np)
+static struct syscon *syscon_node_to_syscon(struct device_node *np)
 {
 	if (!of_device_is_compatible(np, "syscon"))
 		return ERR_PTR(-EINVAL);
 
-	return device_node_get_regmap(np, true);
+	return device_node_get_syscon(np, true);
+}
+
+struct regmap *syscon_node_to_regmap(struct device_node *np)
+{
+	struct syscon *syscon;
+
+	syscon = syscon_node_to_syscon(np);
+	if (IS_ERR(syscon))
+		return ERR_CAST(syscon);
+
+	return syscon->regmap;
 }
 EXPORT_SYMBOL_GPL(syscon_node_to_regmap);
 
@@ -223,11 +274,11 @@ struct regmap *syscon_regmap_lookup_by_compatible(const char *s)
 }
 EXPORT_SYMBOL_GPL(syscon_regmap_lookup_by_compatible);
 
-struct regmap *syscon_regmap_lookup_by_phandle(struct device_node *np,
-					const char *property)
+static struct syscon *syscon_lookup_by_phandle(struct device_node *np,
+					       const char *property)
 {
 	struct device_node *syscon_np;
-	struct regmap *regmap;
+	struct syscon *syscon;
 
 	if (property)
 		syscon_np = of_parse_phandle(np, property, 0);
@@ -237,12 +288,24 @@ struct regmap *syscon_regmap_lookup_by_phandle(struct device_node *np,
 	if (!syscon_np)
 		return ERR_PTR(-ENODEV);
 
-	regmap = syscon_node_to_regmap(syscon_np);
+	syscon = syscon_node_to_syscon(syscon_np);
 
 	if (property)
 		of_node_put(syscon_np);
 
-	return regmap;
+	return syscon;
+}
+
+struct regmap *syscon_regmap_lookup_by_phandle(struct device_node *np,
+					       const char *property)
+{
+	struct syscon *syscon;
+
+	syscon = syscon_lookup_by_phandle(np, property);
+	if (IS_ERR(syscon))
+		return ERR_CAST(syscon);
+
+	return syscon->regmap;
 }
 EXPORT_SYMBOL_GPL(syscon_regmap_lookup_by_phandle);
 
@@ -293,6 +356,70 @@ struct regmap *syscon_regmap_lookup_by_phandle_optional(struct device_node *np,
 }
 EXPORT_SYMBOL_GPL(syscon_regmap_lookup_by_phandle_optional);
 
+static struct syscon *syscon_from_regmap(struct regmap *regmap)
+{
+	struct syscon *entry, *syscon = NULL;
+
+	spin_lock(&syscon_list_slock);
+
+	list_for_each_entry(entry, &syscon_list, list)
+		if (entry->regmap == regmap) {
+			syscon = entry;
+			break;
+		}
+
+	spin_unlock(&syscon_list_slock);
+
+	return syscon;
+}
+
+void syscon_put_regmap(struct regmap *regmap)
+{
+	struct syscon *syscon;
+
+	syscon = syscon_from_regmap(regmap);
+	if (!syscon)
+		return;
+
+	syscon_put(syscon);
+}
+EXPORT_SYMBOL_GPL(syscon_put_regmap);
+
+static void devm_syscon_release(struct device *dev, void *res)
+{
+	syscon_put(*(struct syscon **)res);
+}
+
+static struct regmap *__devm_syscon_get(struct device *dev,
+					struct syscon *syscon)
+{
+	struct syscon **ptr;
+
+	if (IS_ERR(syscon))
+		return ERR_CAST(syscon);
+
+	ptr = devres_alloc(devm_syscon_release, sizeof(struct syscon *), GFP_KERNEL);
+	if (!ptr) {
+		syscon_put(syscon);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	*ptr = syscon;
+	devres_add(dev, ptr);
+
+	return syscon->regmap;
+}
+
+struct regmap *devm_syscon_regmap_lookup_by_phandle(struct device *dev,
+						    struct device_node *np,
+						    const char *property)
+{
+	struct syscon *syscon = syscon_lookup_by_phandle(np, property);
+
+	return __devm_syscon_get(dev, syscon);
+}
+EXPORT_SYMBOL_GPL(devm_syscon_regmap_lookup_by_phandle);
+
 static int syscon_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
diff --git a/include/linux/mfd/syscon.h b/include/linux/mfd/syscon.h
index c315903f6dab..f742d865a37a 100644
--- a/include/linux/mfd/syscon.h
+++ b/include/linux/mfd/syscon.h
@@ -15,6 +15,7 @@
 #include <linux/errno.h>
 
 struct device_node;
+struct device;
 
 #ifdef CONFIG_MFD_SYSCON
 struct regmap *device_node_to_regmap(struct device_node *np);
@@ -28,6 +29,11 @@ struct regmap *syscon_regmap_lookup_by_phandle_args(struct device_node *np,
 						    unsigned int *out_args);
 struct regmap *syscon_regmap_lookup_by_phandle_optional(struct device_node *np,
 							const char *property);
+void syscon_put_regmap(struct regmap *regmap);
+
+struct regmap *devm_syscon_regmap_lookup_by_phandle(struct device *dev,
+						    struct device_node *np,
+						    const char *property);
 #else
 static inline struct regmap *device_node_to_regmap(struct device_node *np)
 {
@@ -67,6 +73,18 @@ static inline struct regmap *syscon_regmap_lookup_by_phandle_optional(
 	return NULL;
 }
 
+static inline void syscon_put_regmap(struct regmap *regmap)
+{
+}
+
+static inline
+struct regmap *devm_syscon_regmap_lookup_by_phandle(struct device *dev,
+						    struct device_node *np,
+						    const char *property)
+{
+	return NULL;
+}
+
 #endif
 
 #endif /* __LINUX_MFD_SYSCON_H__ */
-- 
2.45.0

[PATCH v3 2/7] reset: mchp: sparx5: Remove dependencies and allow building as a module

[Herve Codina]

From: Clément Léger <clement.leger@bootlin.com>

The sparx5 reset controller depends on the SPARX5 architecture or the
LAN966x SoC.

This reset controller can be used by the LAN966x PCI device and so it
needs to be available on all architectures.
Also the LAN966x PCI device driver can be built as a module and this
reset controller driver has no reason to be a builtin driver in that
case.

Signed-off-by: Clément Léger <clement.leger@bootlin.com>
Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 drivers/reset/Kconfig                  | 3 +--
 drivers/reset/reset-microchip-sparx5.c | 2 ++
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index 7112f5932609..fb9005e2f5b5 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -124,8 +124,7 @@ config RESET_LPC18XX
 	  This enables the reset controller driver for NXP LPC18xx/43xx SoCs.
 
 config RESET_MCHP_SPARX5
-	bool "Microchip Sparx5 reset driver"
-	depends on ARCH_SPARX5 || SOC_LAN966 || COMPILE_TEST
+	tristate "Microchip Sparx5 reset driver"
 	default y if SPARX5_SWITCH
 	select MFD_SYSCON
 	help
diff --git a/drivers/reset/reset-microchip-sparx5.c b/drivers/reset/reset-microchip-sparx5.c
index 636e85c388b0..69915c7b4941 100644
--- a/drivers/reset/reset-microchip-sparx5.c
+++ b/drivers/reset/reset-microchip-sparx5.c
@@ -158,6 +158,7 @@ static const struct of_device_id mchp_sparx5_reset_of_match[] = {
 	},
 	{ }
 };
+MODULE_DEVICE_TABLE(of, mchp_sparx5_reset_of_match);
 
 static struct platform_driver mchp_sparx5_reset_driver = {
 	.probe = mchp_sparx5_reset_probe,
@@ -180,3 +181,4 @@ postcore_initcall(mchp_sparx5_reset_init);
 
 MODULE_DESCRIPTION("Microchip Sparx5 switch reset driver");
 MODULE_AUTHOR("Steen Hegelund <steen.hegelund@microchip.com>");
+MODULE_LICENSE("GPL");
-- 
2.45.0

[PATCH v3 3/7] reset: mchp: sparx5: Release syscon when not use anymore

[Herve Codina]

From: Clément Léger <clement.leger@bootlin.com>

The sparx5 reset controller does not release syscon when it is not used
anymore.

This reset controller is used by the LAN966x PCI device driver.
It can be removed from the system at runtime and needs to release its
consumed syscon on removal.

Use the newly introduced devm_syscon_regmap_lookup_by_phandle() in order
to get the syscon and automatically release it on removal.

Signed-off-by: Clément Léger <clement.leger@bootlin.com>
Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 drivers/reset/reset-microchip-sparx5.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/reset/reset-microchip-sparx5.c b/drivers/reset/reset-microchip-sparx5.c
index 69915c7b4941..c4fe65291a43 100644
--- a/drivers/reset/reset-microchip-sparx5.c
+++ b/drivers/reset/reset-microchip-sparx5.c
@@ -65,15 +65,11 @@ static const struct reset_control_ops sparx5_reset_ops = {
 static int mchp_sparx5_map_syscon(struct platform_device *pdev, char *name,
 				  struct regmap **target)
 {
-	struct device_node *syscon_np;
+	struct device *dev = &pdev->dev;
 	struct regmap *regmap;
 	int err;
 
-	syscon_np = of_parse_phandle(pdev->dev.of_node, name, 0);
-	if (!syscon_np)
-		return -ENODEV;
-	regmap = syscon_node_to_regmap(syscon_np);
-	of_node_put(syscon_np);
+	regmap = devm_syscon_regmap_lookup_by_phandle(dev, dev->of_node, name);
 	if (IS_ERR(regmap)) {
 		err = PTR_ERR(regmap);
 		dev_err(&pdev->dev, "No '%s' map: %d\n", name, err);
-- 
2.45.0

[PATCH v3 4/7] reset: core: add get_device()/put_device on rcdev

[Herve Codina]

From: Clément Léger <clement.leger@bootlin.com>

Since the rcdev structure is allocated by the reset controller drivers
themselves, they need to exists as long as there is a consumer. A call to
module_get() is already existing but that does not work when using
device-tree overlays. In order to guarantee that the underlying reset
controller device does not vanish while using it, add a get_device() call
when retrieving a reset control from a reset controller device and a
put_device() when releasing that control.

Signed-off-by: Clément Léger <clement.leger@bootlin.com>
Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 drivers/reset/core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index dba74e857be6..999c3c41cf21 100644
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -812,6 +812,7 @@ __reset_control_get_internal(struct reset_controller_dev *rcdev,
 	kref_init(&rstc->refcnt);
 	rstc->acquired = acquired;
 	rstc->shared = shared;
+	get_device(rcdev->dev);
 
 	return rstc;
 }
@@ -826,6 +827,7 @@ static void __reset_control_release(struct kref *kref)
 	module_put(rstc->rcdev->owner);
 
 	list_del(&rstc->list);
+	put_device(rstc->rcdev->dev);
 	kfree(rstc);
 }
 
-- 
2.45.0

[PATCH v3 5/7] reset: mchp: sparx5: set the dev member of the reset controller

[Herve Codina]

From: Clément Léger <clement.leger@bootlin.com>

In order to guarantee the device will not be deleted by the reset
controller consumer, set the dev member of the reset controller.

Signed-off-by: Clément Léger <clement.leger@bootlin.com>
Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 drivers/reset/reset-microchip-sparx5.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/reset/reset-microchip-sparx5.c b/drivers/reset/reset-microchip-sparx5.c
index c4fe65291a43..1ef2aa1602e3 100644
--- a/drivers/reset/reset-microchip-sparx5.c
+++ b/drivers/reset/reset-microchip-sparx5.c
@@ -117,6 +117,7 @@ static int mchp_sparx5_reset_probe(struct platform_device *pdev)
 		return err;
 
 	ctx->rcdev.owner = THIS_MODULE;
+	ctx->rcdev.dev = &pdev->dev;
 	ctx->rcdev.nr_resets = 1;
 	ctx->rcdev.ops = &sparx5_reset_ops;
 	ctx->rcdev.of_node = dn;
-- 
2.45.0

[PATCH v3 6/7] mfd: Add support for LAN966x PCI device

[Herve Codina]

Add a PCI driver that handles the LAN966x PCI device using a device-tree
overlay. This overlay is applied to the PCI device DT node and allows to
describe components that are present in the device.

The memory from the device-tree is remapped to the BAR memory thanks to
"ranges" properties computed at runtime by the PCI core during the PCI
enumeration.

The PCI device itself acts as an interrupt controller and is used as the
parent of the internal LAN966x interrupt controller to route the
interrupts to the assigned PCI INTx interrupt.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 drivers/mfd/Kconfig          |  24 ++++
 drivers/mfd/Makefile         |   4 +
 drivers/mfd/lan966x_pci.c    | 229 +++++++++++++++++++++++++++++++++++
 drivers/mfd/lan966x_pci.dtso | 167 +++++++++++++++++++++++++
 drivers/pci/quirks.c         |   1 +
 5 files changed, 425 insertions(+)
 create mode 100644 drivers/mfd/lan966x_pci.c
 create mode 100644 drivers/mfd/lan966x_pci.dtso

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 266b4f54af60..15db144bc09b 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -144,6 +144,30 @@ config MFD_ATMEL_FLEXCOM
 	  by the probe function of this MFD driver according to a device tree
 	  property.
 
+config MFD_LAN966X_PCI
+	tristate "Microchip LAN966x PCIe Support"
+	depends on PCI
+	select OF
+	select OF_OVERLAY
+	select IRQ_DOMAIN
+	help
+	  This enables the support for the LAN966x PCIe device.
+	  This is used to drive the LAN966x PCIe device from the host system
+	  to which it is connected.
+
+	  This driver uses an overlay to load other drivers to support for
+	  LAN966x internal components.
+	  Even if this driver does not depend on these other drivers, in order
+	  to have a fully functional board, the following drivers are needed:
+	    - fixed-clock (COMMON_CLK)
+	    - lan966x-oic (LAN966X_OIC)
+	    - lan966x-cpu-syscon (MFD_SYSCON)
+	    - lan966x-switch-reset (RESET_MCHP_SPARX5)
+	    - lan966x-pinctrl (PINCTRL_OCELOT)
+	    - lan966x-serdes (PHY_LAN966X_SERDES)
+	    - lan966x-miim (MDIO_MSCC_MIIM)
+	    - lan966x-switch (LAN966X_SWITCH)
+
 config MFD_ATMEL_HLCDC
 	tristate "Atmel HLCDC (High-end LCD Controller)"
 	select MFD_CORE
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index c66f07edcd0e..165a9674ff48 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -284,3 +284,7 @@ rsmu-i2c-objs			:= rsmu_core.o rsmu_i2c.o
 rsmu-spi-objs			:= rsmu_core.o rsmu_spi.o
 obj-$(CONFIG_MFD_RSMU_I2C)	+= rsmu-i2c.o
 obj-$(CONFIG_MFD_RSMU_SPI)	+= rsmu-spi.o
+
+lan966x-pci-objs		:= lan966x_pci.o
+lan966x-pci-objs		+= lan966x_pci.dtbo.o
+obj-$(CONFIG_MFD_LAN966X_PCI)	+= lan966x-pci.o
diff --git a/drivers/mfd/lan966x_pci.c b/drivers/mfd/lan966x_pci.c
new file mode 100644
index 000000000000..c69350449b15
--- /dev/null
+++ b/drivers/mfd/lan966x_pci.c
@@ -0,0 +1,229 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Microchip LAN966x PCI driver
+ *
+ * Copyright (c) 2024 Microchip Technology Inc. and its subsidiaries.
+ *
+ * Authors:
+ *	Clément Léger <clement.leger@bootlin.com>
+ *	Hervé Codina <herve.codina@bootlin.com>
+ */
+
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/pci.h>
+#include <linux/pci_ids.h>
+#include <linux/slab.h>
+
+/* Embedded dtbo symbols created by cmd_wrap_S_dtb in scripts/Makefile.lib */
+extern char __dtbo_lan966x_pci_begin[];
+extern char __dtbo_lan966x_pci_end[];
+
+struct pci_dev_intr_ctrl {
+	struct pci_dev *pci_dev;
+	struct irq_domain *irq_domain;
+	int irq;
+};
+
+static int pci_dev_irq_domain_map(struct irq_domain *d, unsigned int virq, irq_hw_number_t hw)
+{
+	irq_set_chip_and_handler(virq, &dummy_irq_chip, handle_simple_irq);
+	return 0;
+}
+
+static const struct irq_domain_ops pci_dev_irq_domain_ops = {
+	.map = pci_dev_irq_domain_map,
+	.xlate = irq_domain_xlate_onecell,
+};
+
+static irqreturn_t pci_dev_irq_handler(int irq, void *data)
+{
+	struct pci_dev_intr_ctrl *intr_ctrl = data;
+	int ret;
+
+	ret = generic_handle_domain_irq(intr_ctrl->irq_domain, 0);
+	return IRQ_RETVAL(!ret);
+}
+
+static struct pci_dev_intr_ctrl *pci_dev_create_intr_ctrl(struct pci_dev *pdev)
+{
+	struct pci_dev_intr_ctrl *intr_ctrl;
+	struct fwnode_handle *fwnode;
+	int ret;
+
+	if (!pdev->irq)
+		return ERR_PTR(-EOPNOTSUPP);
+
+	fwnode = dev_fwnode(&pdev->dev);
+	if (!fwnode)
+		return ERR_PTR(-ENODEV);
+
+	intr_ctrl = kmalloc(sizeof(*intr_ctrl), GFP_KERNEL);
+	if (!intr_ctrl)
+		return ERR_PTR(-ENOMEM);
+
+	intr_ctrl->pci_dev = pdev;
+
+	intr_ctrl->irq_domain = irq_domain_create_linear(fwnode, 1, &pci_dev_irq_domain_ops,
+							 intr_ctrl);
+	if (!intr_ctrl->irq_domain) {
+		pci_err(pdev, "Failed to create irqdomain\n");
+		ret = -ENOMEM;
+		goto err_free_intr_ctrl;
+	}
+
+	ret = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_INTX);
+	if (ret < 0) {
+		pci_err(pdev, "Unable alloc irq vector (%d)\n", ret);
+		goto err_remove_domain;
+	}
+	intr_ctrl->irq = pci_irq_vector(pdev, 0);
+	ret = request_irq(intr_ctrl->irq, pci_dev_irq_handler, IRQF_SHARED,
+			  dev_name(&pdev->dev), intr_ctrl);
+	if (ret) {
+		pci_err(pdev, "Unable to request irq %d (%d)\n", intr_ctrl->irq, ret);
+		goto err_free_irq_vector;
+	}
+
+	return intr_ctrl;
+
+err_free_irq_vector:
+	pci_free_irq_vectors(pdev);
+err_remove_domain:
+	irq_domain_remove(intr_ctrl->irq_domain);
+err_free_intr_ctrl:
+	kfree(intr_ctrl);
+	return ERR_PTR(ret);
+}
+
+static void pci_dev_remove_intr_ctrl(struct pci_dev_intr_ctrl *intr_ctrl)
+{
+	free_irq(intr_ctrl->irq, intr_ctrl);
+	pci_free_irq_vectors(intr_ctrl->pci_dev);
+	irq_dispose_mapping(irq_find_mapping(intr_ctrl->irq_domain, 0));
+	irq_domain_remove(intr_ctrl->irq_domain);
+	kfree(intr_ctrl);
+}
+
+static void devm_pci_dev_remove_intr_ctrl(void *data)
+{
+	struct pci_dev_intr_ctrl *intr_ctrl = data;
+
+	pci_dev_remove_intr_ctrl(intr_ctrl);
+}
+
+static int devm_pci_dev_create_intr_ctrl(struct pci_dev *pdev)
+{
+	struct pci_dev_intr_ctrl *intr_ctrl;
+
+	intr_ctrl = pci_dev_create_intr_ctrl(pdev);
+	if (IS_ERR(intr_ctrl))
+		return PTR_ERR(intr_ctrl);
+
+	return devm_add_action_or_reset(&pdev->dev, devm_pci_dev_remove_intr_ctrl, intr_ctrl);
+}
+
+struct lan966x_pci {
+	struct device *dev;
+	int ovcs_id;
+};
+
+static int lan966x_pci_load_overlay(struct lan966x_pci *data)
+{
+	u32 dtbo_size = __dtbo_lan966x_pci_end - __dtbo_lan966x_pci_begin;
+	void *dtbo_start = __dtbo_lan966x_pci_begin;
+	int ret;
+
+	ret = of_overlay_fdt_apply(dtbo_start, dtbo_size, &data->ovcs_id, dev_of_node(data->dev));
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static void lan966x_pci_unload_overlay(struct lan966x_pci *data)
+{
+	of_overlay_remove(&data->ovcs_id);
+}
+
+static int lan966x_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
+{
+	struct device *dev = &pdev->dev;
+	struct lan966x_pci *data;
+	int ret;
+
+	/*
+	 * On ACPI system, fwnode can point to the ACPI node.
+	 * This driver needs an of_node to be used as the device-tree overlay
+	 * target. This of_node should be set by the PCI core if it succeeds in
+	 * creating it (CONFIG_PCI_DYNAMIC_OF_NODES feature).
+	 * Check here for the validity of this of_node.
+	 */
+	if (!dev_of_node(dev)) {
+		dev_err(dev, "Missing of_node for device\n");
+		return -EINVAL;
+	}
+
+	/* Need to be done before devm_pci_dev_create_intr_ctrl.
+	 * It allocates an IRQ and so pdev->irq is updated.
+	 */
+	ret = pcim_enable_device(pdev);
+	if (ret)
+		return ret;
+
+	ret = devm_pci_dev_create_intr_ctrl(pdev);
+	if (ret)
+		return ret;
+
+	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	pci_set_drvdata(pdev, data);
+	data->dev = dev;
+
+	ret = lan966x_pci_load_overlay(data);
+	if (ret)
+		return ret;
+
+	pci_set_master(pdev);
+
+	ret = of_platform_default_populate(dev_of_node(dev), NULL, dev);
+	if (ret)
+		goto err_unload_overlay;
+
+	return 0;
+
+err_unload_overlay:
+	lan966x_pci_unload_overlay(data);
+	return ret;
+}
+
+static void lan966x_pci_remove(struct pci_dev *pdev)
+{
+	struct lan966x_pci *data = pci_get_drvdata(pdev);
+
+	of_platform_depopulate(data->dev);
+
+	lan966x_pci_unload_overlay(data);
+}
+
+static struct pci_device_id lan966x_pci_ids[] = {
+	{ PCI_DEVICE(PCI_VENDOR_ID_EFAR, 0x9660) },
+	{ }
+};
+MODULE_DEVICE_TABLE(pci, lan966x_pci_ids);
+
+static struct pci_driver lan966x_pci_driver = {
+	.name = "mchp_lan966x_pci",
+	.id_table = lan966x_pci_ids,
+	.probe = lan966x_pci_probe,
+	.remove = lan966x_pci_remove,
+};
+module_pci_driver(lan966x_pci_driver);
+
+MODULE_AUTHOR("Herve Codina <herve.codina@bootlin.com>");
+MODULE_DESCRIPTION("Microchip LAN966x PCI driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/mfd/lan966x_pci.dtso b/drivers/mfd/lan966x_pci.dtso
new file mode 100644
index 000000000000..041f4319e4cd
--- /dev/null
+++ b/drivers/mfd/lan966x_pci.dtso
@@ -0,0 +1,167 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2022 Microchip UNG
+ */
+
+#include <dt-bindings/clock/microchip,lan966x.h>
+#include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/mfd/atmel-flexcom.h>
+#include <dt-bindings/phy/phy-lan966x-serdes.h>
+#include <dt-bindings/gpio/gpio.h>
+
+/dts-v1/;
+/plugin/;
+
+/ {
+	fragment@0 {
+		target-path="";
+		__overlay__ {
+			#address-cells = <3>;
+			#size-cells = <2>;
+
+			pci-ep-bus@0 {
+				compatible = "simple-bus";
+				#address-cells = <1>;
+				#size-cells = <1>;
+
+				/*
+				 * map @0xe2000000 (32MB) to BAR0 (CPU)
+				 * map @0xe0000000 (16MB) to BAR1 (AMBA)
+				 */
+				ranges = <0xe2000000 0x00 0x00 0x00 0x2000000
+				          0xe0000000 0x01 0x00 0x00 0x1000000>;
+
+				oic: oic@e00c0120 {
+					compatible = "microchip,lan966x-oic";
+					#interrupt-cells = <2>;
+					interrupt-controller;
+					interrupts = <0>; /* PCI INTx assigned interrupt */
+					reg = <0xe00c0120 0x190>;
+				};
+
+				cpu_clk: cpu_clk {
+					compatible = "fixed-clock";
+					#clock-cells = <0>;
+					clock-frequency = <600000000>;  // CPU clock = 600MHz
+				};
+
+				ddr_clk: ddr_clk {
+					compatible = "fixed-clock";
+					#clock-cells = <0>;
+					clock-frequency = <30000000>;  // Fabric clock = 30MHz
+				};
+
+				sys_clk: sys_clk {
+					compatible = "fixed-clock";
+					#clock-cells = <0>;
+					clock-frequency = <15625000>;  // System clock = 15.625MHz
+				};
+
+				cpu_ctrl: syscon@e00c0000 {
+					compatible = "microchip,lan966x-cpu-syscon", "syscon";
+					reg = <0xe00c0000 0xa8>;
+				};
+
+				reset: reset@e200400c {
+					compatible = "microchip,lan966x-switch-reset";
+					reg = <0xe200400c 0x4>;
+					reg-names = "gcb";
+					#reset-cells = <1>;
+					cpu-syscon = <&cpu_ctrl>;
+				};
+
+				gpio: pinctrl@e2004064 {
+					compatible = "microchip,lan966x-pinctrl";
+					reg = <0xe2004064 0xb4>,
+					      <0xe2010024 0x138>;
+					resets = <&reset 0>;
+					reset-names = "switch";
+					gpio-controller;
+					#gpio-cells = <2>;
+					gpio-ranges = <&gpio 0 0 78>;
+					interrupt-parent = <&oic>;
+					interrupt-controller;
+					interrupts = <17 IRQ_TYPE_LEVEL_HIGH>;
+					#interrupt-cells = <2>;
+
+					tod_pins: tod_pins {
+						pins = "GPIO_36";
+						function = "ptpsync_1";
+					};
+
+					fc0_a_pins: fcb4-i2c-pins {
+						/* RXD, TXD */
+						pins = "GPIO_9", "GPIO_10";
+						function = "fc0_a";
+					};
+
+				};
+
+				serdes: serdes@e202c000 {
+					compatible = "microchip,lan966x-serdes";
+					reg = <0xe202c000 0x9c>,
+					      <0xe2004010 0x4>;
+					#phy-cells = <2>;
+				};
+
+				mdio1: mdio@e200413c {
+					#address-cells = <1>;
+					#size-cells = <0>;
+					compatible = "microchip,lan966x-miim";
+					reg = <0xe200413c 0x24>,
+					      <0xe2010020 0x4>;
+
+					resets = <&reset 0>;
+					reset-names = "switch";
+
+					lan966x_phy0: ethernet-lan966x_phy@1 {
+						reg = <1>;
+					};
+
+					lan966x_phy1: ethernet-lan966x_phy@2 {
+						reg = <2>;
+					};
+				};
+
+				switch: switch@e0000000 {
+					compatible = "microchip,lan966x-switch";
+					reg = <0xe0000000 0x0100000>,
+					      <0xe2000000 0x0800000>;
+					reg-names = "cpu", "gcb";
+
+					interrupt-parent = <&oic>;
+					interrupts = <12 IRQ_TYPE_LEVEL_HIGH>,
+						     <9 IRQ_TYPE_LEVEL_HIGH>;
+					interrupt-names = "xtr", "ana";
+
+					resets = <&reset 0>;
+					reset-names = "switch";
+
+					pinctrl-names = "default";
+					pinctrl-0 = <&tod_pins>;
+
+					ethernet-ports {
+						#address-cells = <1>;
+						#size-cells = <0>;
+
+						port0: port@0 {
+							phy-handle = <&lan966x_phy0>;
+
+							reg = <0>;
+							phy-mode = "gmii";
+							phys = <&serdes 0 CU(0)>;
+						};
+
+						port1: port@1 {
+							phy-handle = <&lan966x_phy1>;
+
+							reg = <1>;
+							phy-mode = "gmii";
+							phys = <&serdes 1 CU(1)>;
+						};
+					};
+				};
+			};
+		};
+	};
+};
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 568410e64ce6..30b64994784c 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -6241,6 +6241,7 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0xa76e, dpc_log_size);
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_XILINX, 0x5020, of_pci_make_dev_node);
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_XILINX, 0x5021, of_pci_make_dev_node);
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_REDHAT, 0x0005, of_pci_make_dev_node);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_EFAR, 0x9660, of_pci_make_dev_node);
 
 /*
  * Devices known to require a longer delay before first config space access
-- 
2.45.0

[PATCH v3 7/7] MAINTAINERS: Add the Microchip LAN966x PCI driver entry

[Herve Codina]

After contributing the driver, add myself as the maintainer for the
Microchip LAN966x PCI driver.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 MAINTAINERS | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index baeb307344cd..c84ec27ccbe4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14733,6 +14733,12 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/interrupt-controller/microchip,lan966x-oic.yaml
 F:	drivers/irqchip/irq-lan966x-oic.c
 
+MICROCHIP LAN966X PCI DRIVER
+M:	Herve Codina <herve.codina@bootlin.com>
+S:	Maintained
+F:	drivers/mfd/lan966x_pci.c
+F:	drivers/mfd/lan966x_pci.dtso
+
 MICROCHIP LCDFB DRIVER
 M:	Nicolas Ferre <nicolas.ferre@microchip.com>
 L:	linux-fbdev@vger.kernel.org
-- 
2.45.0

Re: [PATCH v3 6/7] mfd: Add support for LAN966x PCI device

[Lee Jones]

On Thu, 27 Jun 2024, Herve Codina wrote:

> Add a PCI driver that handles the LAN966x PCI device using a device-tree
> overlay. This overlay is applied to the PCI device DT node and allows to
> describe components that are present in the device.
> 
> The memory from the device-tree is remapped to the BAR memory thanks to
> "ranges" properties computed at runtime by the PCI core during the PCI
> enumeration.
> 
> The PCI device itself acts as an interrupt controller and is used as the
> parent of the internal LAN966x interrupt controller to route the
> interrupts to the assigned PCI INTx interrupt.

Not entirely sure why this is in MFD.

Also I'm unsure of his current views, but Greg has voiced pretty big
feelings about representing PCI drivers as Platform ones in the past.

/ Lee

> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> ---
>  drivers/mfd/Kconfig          |  24 ++++
>  drivers/mfd/Makefile         |   4 +
>  drivers/mfd/lan966x_pci.c    | 229 +++++++++++++++++++++++++++++++++++
>  drivers/mfd/lan966x_pci.dtso | 167 +++++++++++++++++++++++++
>  drivers/pci/quirks.c         |   1 +
>  5 files changed, 425 insertions(+)
>  create mode 100644 drivers/mfd/lan966x_pci.c
>  create mode 100644 drivers/mfd/lan966x_pci.dtso
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 266b4f54af60..15db144bc09b 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -144,6 +144,30 @@ config MFD_ATMEL_FLEXCOM
>  	  by the probe function of this MFD driver according to a device tree
>  	  property.
>  
> +config MFD_LAN966X_PCI
> +	tristate "Microchip LAN966x PCIe Support"
> +	depends on PCI
> +	select OF
> +	select OF_OVERLAY
> +	select IRQ_DOMAIN
> +	help
> +	  This enables the support for the LAN966x PCIe device.
> +	  This is used to drive the LAN966x PCIe device from the host system
> +	  to which it is connected.
> +
> +	  This driver uses an overlay to load other drivers to support for
> +	  LAN966x internal components.
> +	  Even if this driver does not depend on these other drivers, in order
> +	  to have a fully functional board, the following drivers are needed:
> +	    - fixed-clock (COMMON_CLK)
> +	    - lan966x-oic (LAN966X_OIC)
> +	    - lan966x-cpu-syscon (MFD_SYSCON)
> +	    - lan966x-switch-reset (RESET_MCHP_SPARX5)
> +	    - lan966x-pinctrl (PINCTRL_OCELOT)
> +	    - lan966x-serdes (PHY_LAN966X_SERDES)
> +	    - lan966x-miim (MDIO_MSCC_MIIM)
> +	    - lan966x-switch (LAN966X_SWITCH)
> +
>  config MFD_ATMEL_HLCDC
>  	tristate "Atmel HLCDC (High-end LCD Controller)"
>  	select MFD_CORE
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index c66f07edcd0e..165a9674ff48 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -284,3 +284,7 @@ rsmu-i2c-objs			:= rsmu_core.o rsmu_i2c.o
>  rsmu-spi-objs			:= rsmu_core.o rsmu_spi.o
>  obj-$(CONFIG_MFD_RSMU_I2C)	+= rsmu-i2c.o
>  obj-$(CONFIG_MFD_RSMU_SPI)	+= rsmu-spi.o
> +
> +lan966x-pci-objs		:= lan966x_pci.o
> +lan966x-pci-objs		+= lan966x_pci.dtbo.o
> +obj-$(CONFIG_MFD_LAN966X_PCI)	+= lan966x-pci.o
> diff --git a/drivers/mfd/lan966x_pci.c b/drivers/mfd/lan966x_pci.c
> new file mode 100644
> index 000000000000..c69350449b15
> --- /dev/null
> +++ b/drivers/mfd/lan966x_pci.c
> @@ -0,0 +1,229 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Microchip LAN966x PCI driver
> + *
> + * Copyright (c) 2024 Microchip Technology Inc. and its subsidiaries.
> + *
> + * Authors:
> + *	Clément Léger <clement.leger@bootlin.com>
> + *	Hervé Codina <herve.codina@bootlin.com>
> + */
> +
> +#include <linux/irq.h>
> +#include <linux/irqdomain.h>
> +#include <linux/module.h>
> +#include <linux/of_platform.h>
> +#include <linux/pci.h>
> +#include <linux/pci_ids.h>
> +#include <linux/slab.h>
> +
> +/* Embedded dtbo symbols created by cmd_wrap_S_dtb in scripts/Makefile.lib */
> +extern char __dtbo_lan966x_pci_begin[];
> +extern char __dtbo_lan966x_pci_end[];
> +
> +struct pci_dev_intr_ctrl {
> +	struct pci_dev *pci_dev;
> +	struct irq_domain *irq_domain;
> +	int irq;
> +};
> +
> +static int pci_dev_irq_domain_map(struct irq_domain *d, unsigned int virq, irq_hw_number_t hw)
> +{
> +	irq_set_chip_and_handler(virq, &dummy_irq_chip, handle_simple_irq);
> +	return 0;
> +}
> +
> +static const struct irq_domain_ops pci_dev_irq_domain_ops = {
> +	.map = pci_dev_irq_domain_map,
> +	.xlate = irq_domain_xlate_onecell,
> +};
> +
> +static irqreturn_t pci_dev_irq_handler(int irq, void *data)
> +{
> +	struct pci_dev_intr_ctrl *intr_ctrl = data;
> +	int ret;
> +
> +	ret = generic_handle_domain_irq(intr_ctrl->irq_domain, 0);
> +	return IRQ_RETVAL(!ret);
> +}
> +
> +static struct pci_dev_intr_ctrl *pci_dev_create_intr_ctrl(struct pci_dev *pdev)
> +{
> +	struct pci_dev_intr_ctrl *intr_ctrl;
> +	struct fwnode_handle *fwnode;
> +	int ret;
> +
> +	if (!pdev->irq)
> +		return ERR_PTR(-EOPNOTSUPP);
> +
> +	fwnode = dev_fwnode(&pdev->dev);
> +	if (!fwnode)
> +		return ERR_PTR(-ENODEV);
> +
> +	intr_ctrl = kmalloc(sizeof(*intr_ctrl), GFP_KERNEL);
> +	if (!intr_ctrl)
> +		return ERR_PTR(-ENOMEM);
> +
> +	intr_ctrl->pci_dev = pdev;
> +
> +	intr_ctrl->irq_domain = irq_domain_create_linear(fwnode, 1, &pci_dev_irq_domain_ops,
> +							 intr_ctrl);
> +	if (!intr_ctrl->irq_domain) {
> +		pci_err(pdev, "Failed to create irqdomain\n");
> +		ret = -ENOMEM;
> +		goto err_free_intr_ctrl;
> +	}
> +
> +	ret = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_INTX);
> +	if (ret < 0) {
> +		pci_err(pdev, "Unable alloc irq vector (%d)\n", ret);
> +		goto err_remove_domain;
> +	}
> +	intr_ctrl->irq = pci_irq_vector(pdev, 0);
> +	ret = request_irq(intr_ctrl->irq, pci_dev_irq_handler, IRQF_SHARED,
> +			  dev_name(&pdev->dev), intr_ctrl);
> +	if (ret) {
> +		pci_err(pdev, "Unable to request irq %d (%d)\n", intr_ctrl->irq, ret);
> +		goto err_free_irq_vector;
> +	}
> +
> +	return intr_ctrl;
> +
> +err_free_irq_vector:
> +	pci_free_irq_vectors(pdev);
> +err_remove_domain:
> +	irq_domain_remove(intr_ctrl->irq_domain);
> +err_free_intr_ctrl:
> +	kfree(intr_ctrl);
> +	return ERR_PTR(ret);
> +}
> +
> +static void pci_dev_remove_intr_ctrl(struct pci_dev_intr_ctrl *intr_ctrl)
> +{
> +	free_irq(intr_ctrl->irq, intr_ctrl);
> +	pci_free_irq_vectors(intr_ctrl->pci_dev);
> +	irq_dispose_mapping(irq_find_mapping(intr_ctrl->irq_domain, 0));
> +	irq_domain_remove(intr_ctrl->irq_domain);
> +	kfree(intr_ctrl);
> +}
> +
> +static void devm_pci_dev_remove_intr_ctrl(void *data)
> +{
> +	struct pci_dev_intr_ctrl *intr_ctrl = data;
> +
> +	pci_dev_remove_intr_ctrl(intr_ctrl);
> +}
> +
> +static int devm_pci_dev_create_intr_ctrl(struct pci_dev *pdev)
> +{
> +	struct pci_dev_intr_ctrl *intr_ctrl;
> +
> +	intr_ctrl = pci_dev_create_intr_ctrl(pdev);
> +	if (IS_ERR(intr_ctrl))
> +		return PTR_ERR(intr_ctrl);
> +
> +	return devm_add_action_or_reset(&pdev->dev, devm_pci_dev_remove_intr_ctrl, intr_ctrl);
> +}
> +
> +struct lan966x_pci {
> +	struct device *dev;
> +	int ovcs_id;
> +};
> +
> +static int lan966x_pci_load_overlay(struct lan966x_pci *data)
> +{
> +	u32 dtbo_size = __dtbo_lan966x_pci_end - __dtbo_lan966x_pci_begin;
> +	void *dtbo_start = __dtbo_lan966x_pci_begin;
> +	int ret;
> +
> +	ret = of_overlay_fdt_apply(dtbo_start, dtbo_size, &data->ovcs_id, dev_of_node(data->dev));
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static void lan966x_pci_unload_overlay(struct lan966x_pci *data)
> +{
> +	of_overlay_remove(&data->ovcs_id);
> +}
> +
> +static int lan966x_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct lan966x_pci *data;
> +	int ret;
> +
> +	/*
> +	 * On ACPI system, fwnode can point to the ACPI node.
> +	 * This driver needs an of_node to be used as the device-tree overlay
> +	 * target. This of_node should be set by the PCI core if it succeeds in
> +	 * creating it (CONFIG_PCI_DYNAMIC_OF_NODES feature).
> +	 * Check here for the validity of this of_node.
> +	 */
> +	if (!dev_of_node(dev)) {
> +		dev_err(dev, "Missing of_node for device\n");
> +		return -EINVAL;
> +	}
> +
> +	/* Need to be done before devm_pci_dev_create_intr_ctrl.
> +	 * It allocates an IRQ and so pdev->irq is updated.
> +	 */
> +	ret = pcim_enable_device(pdev);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_pci_dev_create_intr_ctrl(pdev);
> +	if (ret)
> +		return ret;
> +
> +	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	pci_set_drvdata(pdev, data);
> +	data->dev = dev;
> +
> +	ret = lan966x_pci_load_overlay(data);
> +	if (ret)
> +		return ret;
> +
> +	pci_set_master(pdev);
> +
> +	ret = of_platform_default_populate(dev_of_node(dev), NULL, dev);
> +	if (ret)
> +		goto err_unload_overlay;
> +
> +	return 0;
> +
> +err_unload_overlay:
> +	lan966x_pci_unload_overlay(data);
> +	return ret;
> +}
> +
> +static void lan966x_pci_remove(struct pci_dev *pdev)
> +{
> +	struct lan966x_pci *data = pci_get_drvdata(pdev);
> +
> +	of_platform_depopulate(data->dev);
> +
> +	lan966x_pci_unload_overlay(data);
> +}
> +
> +static struct pci_device_id lan966x_pci_ids[] = {
> +	{ PCI_DEVICE(PCI_VENDOR_ID_EFAR, 0x9660) },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(pci, lan966x_pci_ids);
> +
> +static struct pci_driver lan966x_pci_driver = {
> +	.name = "mchp_lan966x_pci",
> +	.id_table = lan966x_pci_ids,
> +	.probe = lan966x_pci_probe,
> +	.remove = lan966x_pci_remove,
> +};
> +module_pci_driver(lan966x_pci_driver);
> +
> +MODULE_AUTHOR("Herve Codina <herve.codina@bootlin.com>");
> +MODULE_DESCRIPTION("Microchip LAN966x PCI driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/mfd/lan966x_pci.dtso b/drivers/mfd/lan966x_pci.dtso
> new file mode 100644
> index 000000000000..041f4319e4cd
> --- /dev/null
> +++ b/drivers/mfd/lan966x_pci.dtso
> @@ -0,0 +1,167 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2022 Microchip UNG
> + */
> +
> +#include <dt-bindings/clock/microchip,lan966x.h>
> +#include <dt-bindings/interrupt-controller/irq.h>
> +#include <dt-bindings/mfd/atmel-flexcom.h>
> +#include <dt-bindings/phy/phy-lan966x-serdes.h>
> +#include <dt-bindings/gpio/gpio.h>
> +
> +/dts-v1/;
> +/plugin/;
> +
> +/ {
> +	fragment@0 {
> +		target-path="";
> +		__overlay__ {
> +			#address-cells = <3>;
> +			#size-cells = <2>;
> +
> +			pci-ep-bus@0 {
> +				compatible = "simple-bus";
> +				#address-cells = <1>;
> +				#size-cells = <1>;
> +
> +				/*
> +				 * map @0xe2000000 (32MB) to BAR0 (CPU)
> +				 * map @0xe0000000 (16MB) to BAR1 (AMBA)
> +				 */
> +				ranges = <0xe2000000 0x00 0x00 0x00 0x2000000
> +				          0xe0000000 0x01 0x00 0x00 0x1000000>;
> +
> +				oic: oic@e00c0120 {
> +					compatible = "microchip,lan966x-oic";
> +					#interrupt-cells = <2>;
> +					interrupt-controller;
> +					interrupts = <0>; /* PCI INTx assigned interrupt */
> +					reg = <0xe00c0120 0x190>;
> +				};
> +
> +				cpu_clk: cpu_clk {
> +					compatible = "fixed-clock";
> +					#clock-cells = <0>;
> +					clock-frequency = <600000000>;  // CPU clock = 600MHz
> +				};
> +
> +				ddr_clk: ddr_clk {
> +					compatible = "fixed-clock";
> +					#clock-cells = <0>;
> +					clock-frequency = <30000000>;  // Fabric clock = 30MHz
> +				};
> +
> +				sys_clk: sys_clk {
> +					compatible = "fixed-clock";
> +					#clock-cells = <0>;
> +					clock-frequency = <15625000>;  // System clock = 15.625MHz
> +				};
> +
> +				cpu_ctrl: syscon@e00c0000 {
> +					compatible = "microchip,lan966x-cpu-syscon", "syscon";
> +					reg = <0xe00c0000 0xa8>;
> +				};
> +
> +				reset: reset@e200400c {
> +					compatible = "microchip,lan966x-switch-reset";
> +					reg = <0xe200400c 0x4>;
> +					reg-names = "gcb";
> +					#reset-cells = <1>;
> +					cpu-syscon = <&cpu_ctrl>;
> +				};
> +
> +				gpio: pinctrl@e2004064 {
> +					compatible = "microchip,lan966x-pinctrl";
> +					reg = <0xe2004064 0xb4>,
> +					      <0xe2010024 0x138>;
> +					resets = <&reset 0>;
> +					reset-names = "switch";
> +					gpio-controller;
> +					#gpio-cells = <2>;
> +					gpio-ranges = <&gpio 0 0 78>;
> +					interrupt-parent = <&oic>;
> +					interrupt-controller;
> +					interrupts = <17 IRQ_TYPE_LEVEL_HIGH>;
> +					#interrupt-cells = <2>;
> +
> +					tod_pins: tod_pins {
> +						pins = "GPIO_36";
> +						function = "ptpsync_1";
> +					};
> +
> +					fc0_a_pins: fcb4-i2c-pins {
> +						/* RXD, TXD */
> +						pins = "GPIO_9", "GPIO_10";
> +						function = "fc0_a";
> +					};
> +
> +				};
> +
> +				serdes: serdes@e202c000 {
> +					compatible = "microchip,lan966x-serdes";
> +					reg = <0xe202c000 0x9c>,
> +					      <0xe2004010 0x4>;
> +					#phy-cells = <2>;
> +				};
> +
> +				mdio1: mdio@e200413c {
> +					#address-cells = <1>;
> +					#size-cells = <0>;
> +					compatible = "microchip,lan966x-miim";
> +					reg = <0xe200413c 0x24>,
> +					      <0xe2010020 0x4>;
> +
> +					resets = <&reset 0>;
> +					reset-names = "switch";
> +
> +					lan966x_phy0: ethernet-lan966x_phy@1 {
> +						reg = <1>;
> +					};
> +
> +					lan966x_phy1: ethernet-lan966x_phy@2 {
> +						reg = <2>;
> +					};
> +				};
> +
> +				switch: switch@e0000000 {
> +					compatible = "microchip,lan966x-switch";
> +					reg = <0xe0000000 0x0100000>,
> +					      <0xe2000000 0x0800000>;
> +					reg-names = "cpu", "gcb";
> +
> +					interrupt-parent = <&oic>;
> +					interrupts = <12 IRQ_TYPE_LEVEL_HIGH>,
> +						     <9 IRQ_TYPE_LEVEL_HIGH>;
> +					interrupt-names = "xtr", "ana";
> +
> +					resets = <&reset 0>;
> +					reset-names = "switch";
> +
> +					pinctrl-names = "default";
> +					pinctrl-0 = <&tod_pins>;
> +
> +					ethernet-ports {
> +						#address-cells = <1>;
> +						#size-cells = <0>;
> +
> +						port0: port@0 {
> +							phy-handle = <&lan966x_phy0>;
> +
> +							reg = <0>;
> +							phy-mode = "gmii";
> +							phys = <&serdes 0 CU(0)>;
> +						};
> +
> +						port1: port@1 {
> +							phy-handle = <&lan966x_phy1>;
> +
> +							reg = <1>;
> +							phy-mode = "gmii";
> +							phys = <&serdes 1 CU(1)>;
> +						};
> +					};
> +				};
> +			};
> +		};
> +	};
> +};
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 568410e64ce6..30b64994784c 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -6241,6 +6241,7 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0xa76e, dpc_log_size);
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_XILINX, 0x5020, of_pci_make_dev_node);
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_XILINX, 0x5021, of_pci_make_dev_node);
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_REDHAT, 0x0005, of_pci_make_dev_node);
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_EFAR, 0x9660, of_pci_make_dev_node);
>  
>  /*
>   * Devices known to require a longer delay before first config space access
> -- 
> 2.45.0
> 

-- 
Lee Jones [李琼斯]

Re: [PATCH v3 1/7] mfd: syscon: Add reference counting and device managed support

[Markus Elfring]

…
> +++ b/drivers/mfd/syscon.c
…
> +static struct syscon *syscon_from_regmap(struct regmap *regmap)
+{
> +	struct syscon *entry, *syscon = NULL;
> +
> +	spin_lock(&syscon_list_slock);
> +
> +	list_for_each_entry(entry, &syscon_list, list)
…
> +	spin_unlock(&syscon_list_slock);
> +
> +	return syscon;
> +}
…

Under which circumstances would you become interested to apply a statement
like “guard(spinlock)(&syscon_list_slock);”?
https://elixir.bootlin.com/linux/v6.10-rc7/source/include/linux/spinlock.h#L561

Regards,
Markus

Re: [PATCH v3 1/7] mfd: syscon: Add reference counting and device managed support

[Herve Codina]

Hi Markus,

On Thu, 11 Jul 2024 18:09:26 +0200
Markus Elfring <Markus.Elfring@web.de> wrote:

> …
> > +++ b/drivers/mfd/syscon.c  
> …
> > +static struct syscon *syscon_from_regmap(struct regmap *regmap)  
> +{
> > +	struct syscon *entry, *syscon = NULL;
> > +
> > +	spin_lock(&syscon_list_slock);
> > +
> > +	list_for_each_entry(entry, &syscon_list, list)  
> …
> > +	spin_unlock(&syscon_list_slock);
> > +
> > +	return syscon;
> > +}  
> …
> 
> Under which circumstances would you become interested to apply a statement
> like “guard(spinlock)(&syscon_list_slock);”?
> https://elixir.bootlin.com/linux/v6.10-rc7/source/include/linux/spinlock.h#L561
> 

I used the spin_{lock,unlock}() pattern call already present in syscon.c.
Of course, I can add a new patch in this series converting syscon.c to
the guard() family and use guard() in my introduced lock/unlock.

Lee, any opinion ?

Best regards,
Hervé

Re: [PATCH v3 6/7] mfd: Add support for LAN966x PCI device

[Herve Codina]

Hi Lee,

On Thu, 11 Jul 2024 16:29:52 +0100
Lee Jones <lee@kernel.org> wrote:

> On Thu, 27 Jun 2024, Herve Codina wrote:
> 
> > Add a PCI driver that handles the LAN966x PCI device using a device-tree
> > overlay. This overlay is applied to the PCI device DT node and allows to
> > describe components that are present in the device.
> > 
> > The memory from the device-tree is remapped to the BAR memory thanks to
> > "ranges" properties computed at runtime by the PCI core during the PCI
> > enumeration.
> > 
> > The PCI device itself acts as an interrupt controller and is used as the
> > parent of the internal LAN966x interrupt controller to route the
> > interrupts to the assigned PCI INTx interrupt.  
> 
> Not entirely sure why this is in MFD.

This PCI driver purpose is to instanciate many other drivers using a DT
overlay. I think MFD is the right subsystem.

It acts as an interrupt controller because we need to have a bridge between the
device-tree interrupt world and the the PCI world.
This bridge is needed and specific to the driver in order to have resources
available from the device-tree world present in the applied overlay.

> 
> Also I'm unsure of his current views, but Greg has voiced pretty big
> feelings about representing PCI drivers as Platform ones in the past.

PCI drivers as Plaform ones ?
I probably missed something. Can you give me more details ?

Best regards,
Hervé

Re: [PATCH v3 1/7] mfd: syscon: Add reference counting and device managed support

[Lee Jones]

On Thu, 11 Jul 2024, Herve Codina wrote:

> Hi Markus,
> 
> On Thu, 11 Jul 2024 18:09:26 +0200
> Markus Elfring <Markus.Elfring@web.de> wrote:
> 
> > …
> > > +++ b/drivers/mfd/syscon.c  
> > …
> > > +static struct syscon *syscon_from_regmap(struct regmap *regmap)  
> > +{
> > > +	struct syscon *entry, *syscon = NULL;
> > > +
> > > +	spin_lock(&syscon_list_slock);
> > > +
> > > +	list_for_each_entry(entry, &syscon_list, list)  
> > …
> > > +	spin_unlock(&syscon_list_slock);
> > > +
> > > +	return syscon;
> > > +}  
> > …
> > 
> > Under which circumstances would you become interested to apply a statement
> > like “guard(spinlock)(&syscon_list_slock);”?
> > https://elixir.bootlin.com/linux/v6.10-rc7/source/include/linux/spinlock.h#L561
> > 
> 
> I used the spin_{lock,unlock}() pattern call already present in syscon.c.
> Of course, I can add a new patch in this series converting syscon.c to
> the guard() family and use guard() in my introduced lock/unlock.
> 
> Lee, any opinion ?

I'm intentionally leaving this one for Arnd.

-- 
Lee Jones [李琼斯]

Re: [PATCH v3 6/7] mfd: Add support for LAN966x PCI device

[Greg Kroah-Hartman]

On Thu, Jul 11, 2024 at 06:44:38PM +0200, Herve Codina wrote:
> Hi Lee,
> 
> On Thu, 11 Jul 2024 16:29:52 +0100
> Lee Jones <lee@kernel.org> wrote:
> 
> > On Thu, 27 Jun 2024, Herve Codina wrote:
> > 
> > > Add a PCI driver that handles the LAN966x PCI device using a device-tree
> > > overlay. This overlay is applied to the PCI device DT node and allows to
> > > describe components that are present in the device.
> > > 
> > > The memory from the device-tree is remapped to the BAR memory thanks to
> > > "ranges" properties computed at runtime by the PCI core during the PCI
> > > enumeration.
> > > 
> > > The PCI device itself acts as an interrupt controller and is used as the
> > > parent of the internal LAN966x interrupt controller to route the
> > > interrupts to the assigned PCI INTx interrupt.  
> > 
> > Not entirely sure why this is in MFD.
> 
> This PCI driver purpose is to instanciate many other drivers using a DT
> overlay. I think MFD is the right subsystem.

Please use the aux bus for that, that is what is was specifically
designed for, and what it is being used for today.

thanks,

greg k-h

Re: [PATCH v3 6/7] mfd: Add support for LAN966x PCI device

[Rob Herring]

On Thu, Jul 11, 2024 at 1:08 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Thu, Jul 11, 2024 at 06:44:38PM +0200, Herve Codina wrote:
> > Hi Lee,
> >
> > On Thu, 11 Jul 2024 16:29:52 +0100
> > Lee Jones <lee@kernel.org> wrote:
> >
> > > On Thu, 27 Jun 2024, Herve Codina wrote:
> > >
> > > > Add a PCI driver that handles the LAN966x PCI device using a device-tree
> > > > overlay. This overlay is applied to the PCI device DT node and allows to
> > > > describe components that are present in the device.
> > > >
> > > > The memory from the device-tree is remapped to the BAR memory thanks to
> > > > "ranges" properties computed at runtime by the PCI core during the PCI
> > > > enumeration.
> > > >
> > > > The PCI device itself acts as an interrupt controller and is used as the
> > > > parent of the internal LAN966x interrupt controller to route the
> > > > interrupts to the assigned PCI INTx interrupt.
> > >
> > > Not entirely sure why this is in MFD.
> >
> > This PCI driver purpose is to instanciate many other drivers using a DT
> > overlay. I think MFD is the right subsystem.

It is a Multi-function Device, but it doesn't appear to use any of the
MFD subsystem. So maybe drivers/soc/? Another dumping ground, but it
is a driver for an SoC exposed as a PCI device.

> Please use the aux bus for that, that is what is was specifically
> designed for, and what it is being used for today.

We discussed this already[1]. Different usecase, but needs the same thing.

Like I said before, the bus and device used for DT MMIO devices is
like it or not platform bus/device.

In this case, all the child devices are already supported as platform
devices. There would be zero benefit to add all the boilerplate to
make their drivers both platform and aux bus drivers. Plus there is
zero DT support in aux bus.

Rob

[1] https://lore.kernel.org/all/Y9kuxrL3XaCG+blk@kroah.com/

Re: [PATCH v3 6/7] mfd: Add support for LAN966x PCI device

[Greg Kroah-Hartman]

On Thu, Jul 11, 2024 at 02:33:26PM -0600, Rob Herring wrote:
> In this case, all the child devices are already supported as platform
> devices. There would be zero benefit to add all the boilerplate to
> make their drivers both platform and aux bus drivers. Plus there is
> zero DT support in aux bus.

It is by design that there is 0 DT support in aux bus :)

But ok, I'll trust you on this usage...

greg k-h

Re: [PATCH v3 6/7] mfd: Add support for LAN966x PCI device

[Herve Codina]

Hi Rob, Conor,

On Thu, 11 Jul 2024 14:33:26 -0600
Rob Herring <robh@kernel.org> wrote:

> On Thu, Jul 11, 2024 at 1:08 PM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Thu, Jul 11, 2024 at 06:44:38PM +0200, Herve Codina wrote:  
> > > Hi Lee,
> > >
> > > On Thu, 11 Jul 2024 16:29:52 +0100
> > > Lee Jones <lee@kernel.org> wrote:
> > >  
> > > > On Thu, 27 Jun 2024, Herve Codina wrote:
> > > >  
> > > > > Add a PCI driver that handles the LAN966x PCI device using a device-tree
> > > > > overlay. This overlay is applied to the PCI device DT node and allows to
> > > > > describe components that are present in the device.
> > > > >
> > > > > The memory from the device-tree is remapped to the BAR memory thanks to
> > > > > "ranges" properties computed at runtime by the PCI core during the PCI
> > > > > enumeration.
> > > > >
> > > > > The PCI device itself acts as an interrupt controller and is used as the
> > > > > parent of the internal LAN966x interrupt controller to route the
> > > > > interrupts to the assigned PCI INTx interrupt.  
> > > >
> > > > Not entirely sure why this is in MFD.  
> > >
> > > This PCI driver purpose is to instanciate many other drivers using a DT
> > > overlay. I think MFD is the right subsystem.  
> 
> It is a Multi-function Device, but it doesn't appear to use any of the
> MFD subsystem. So maybe drivers/soc/? Another dumping ground, but it
> is a driver for an SoC exposed as a PCI device.
> 

In drivers/soc, drivers/soc/microchip/ could be the right place.

Conor, are you open to have the PCI LAN966x device driver in
drivers/soc/microchip/ ?

Best regards,
Hervé

Re: [PATCH v3 6/7] mfd: Add support for LAN966x PCI device

[Arnd Bergmann]

On Fri, Jul 12, 2024, at 15:11, Herve Codina wrote:
> On Thu, 11 Jul 2024 14:33:26 -0600 Rob Herring <robh@kernel.org> wrote:
>> On Thu, Jul 11, 2024 at 1:08 PM Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:

>> > >
>> > > This PCI driver purpose is to instanciate many other drivers using a DT
>> > > overlay. I think MFD is the right subsystem.  
>> 
>> It is a Multi-function Device, but it doesn't appear to use any of the
>> MFD subsystem. So maybe drivers/soc/? Another dumping ground, but it
>> is a driver for an SoC exposed as a PCI device.
>> 
>
> In drivers/soc, drivers/soc/microchip/ could be the right place.
>
> Conor, are you open to have the PCI LAN966x device driver in
> drivers/soc/microchip/ ?

That sounds like a much worse fit than drivers/mfd: the code
here does not actually run on the lan966x soc, it instead runs
on whatever other machine you happen to plug it into as a
PCI device.

      Arnd

Re: [PATCH v3 6/7] mfd: Add support for LAN966x PCI device

[Herve Codina]

Hi Arnd,

On Fri, 12 Jul 2024 16:14:31 +0200
"Arnd Bergmann" <arnd@arndb.de> wrote:

> On Fri, Jul 12, 2024, at 15:11, Herve Codina wrote:
> > On Thu, 11 Jul 2024 14:33:26 -0600 Rob Herring <robh@kernel.org> wrote:  
> >> On Thu, Jul 11, 2024 at 1:08 PM Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:  
> 
> >> > >
> >> > > This PCI driver purpose is to instanciate many other drivers using a DT
> >> > > overlay. I think MFD is the right subsystem.    
> >> 
> >> It is a Multi-function Device, but it doesn't appear to use any of the
> >> MFD subsystem. So maybe drivers/soc/? Another dumping ground, but it
> >> is a driver for an SoC exposed as a PCI device.
> >>   
> >
> > In drivers/soc, drivers/soc/microchip/ could be the right place.
> >
> > Conor, are you open to have the PCI LAN966x device driver in
> > drivers/soc/microchip/ ?  
> 
> That sounds like a much worse fit than drivers/mfd: the code
> here does not actually run on the lan966x soc, it instead runs
> on whatever other machine you happen to plug it into as a
> PCI device.

Maybe drivers/misc ?

Best regards,
Hervé

Re: [PATCH v3 6/7] mfd: Add support for LAN966x PCI device

[Arnd Bergmann]

On Mon, Jul 15, 2024, at 14:12, Herve Codina wrote:
> Hi Arnd,
>
> On Fri, 12 Jul 2024 16:14:31 +0200
> "Arnd Bergmann" <arnd@arndb.de> wrote:
>
>> On Fri, Jul 12, 2024, at 15:11, Herve Codina wrote:
>> > On Thu, 11 Jul 2024 14:33:26 -0600 Rob Herring <robh@kernel.org> wrote:  
>> >> On Thu, Jul 11, 2024 at 1:08 PM Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:  
>> 
>> >> > >
>> >> > > This PCI driver purpose is to instanciate many other drivers using a DT
>> >> > > overlay. I think MFD is the right subsystem.    
>> >> 
>> >> It is a Multi-function Device, but it doesn't appear to use any of the
>> >> MFD subsystem. So maybe drivers/soc/? Another dumping ground, but it
>> >> is a driver for an SoC exposed as a PCI device.
>> >>   
>> >
>> > In drivers/soc, drivers/soc/microchip/ could be the right place.
>> >
>> > Conor, are you open to have the PCI LAN966x device driver in
>> > drivers/soc/microchip/ ?  
>> 
>> That sounds like a much worse fit than drivers/mfd: the code
>> here does not actually run on the lan966x soc, it instead runs
>> on whatever other machine you happen to plug it into as a
>> PCI device.
>
> Maybe drivers/misc ?

That's probably a little better, and there is already
drivers/misc/mchp_pci1xxxx in there, which also has some
aux devices.

Maybe we need a new place and then move both of these
and some of the similar devices from drivers/mfd to that, but
we don't really have to pick one now.

   Arnd

Re: [PATCH v3 6/7] mfd: Add support for LAN966x PCI device

[Herve Codina]

On Tue, 16 Jul 2024 16:44:12 +0200
"Arnd Bergmann" <arnd@arndb.de> wrote:

> On Mon, Jul 15, 2024, at 14:12, Herve Codina wrote:
> > Hi Arnd,
> >
> > On Fri, 12 Jul 2024 16:14:31 +0200
> > "Arnd Bergmann" <arnd@arndb.de> wrote:
> >  
> >> On Fri, Jul 12, 2024, at 15:11, Herve Codina wrote:  
> >> > On Thu, 11 Jul 2024 14:33:26 -0600 Rob Herring <robh@kernel.org> wrote:    
> >> >> On Thu, Jul 11, 2024 at 1:08 PM Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:    
> >>   
> >> >> > >
> >> >> > > This PCI driver purpose is to instanciate many other drivers using a DT
> >> >> > > overlay. I think MFD is the right subsystem.      
> >> >> 
> >> >> It is a Multi-function Device, but it doesn't appear to use any of the
> >> >> MFD subsystem. So maybe drivers/soc/? Another dumping ground, but it
> >> >> is a driver for an SoC exposed as a PCI device.
> >> >>     
> >> >
> >> > In drivers/soc, drivers/soc/microchip/ could be the right place.
> >> >
> >> > Conor, are you open to have the PCI LAN966x device driver in
> >> > drivers/soc/microchip/ ?    
> >> 
> >> That sounds like a much worse fit than drivers/mfd: the code
> >> here does not actually run on the lan966x soc, it instead runs
> >> on whatever other machine you happen to plug it into as a
> >> PCI device.  
> >
> > Maybe drivers/misc ?  
> 
> That's probably a little better, and there is already
> drivers/misc/mchp_pci1xxxx in there, which also has some
> aux devices.
> 
> Maybe we need a new place and then move both of these
> and some of the similar devices from drivers/mfd to that, but
> we don't really have to pick one now.
> 

In the next iteration, I plan to move the lan966x pci driver in
drivers/misc/

Not sure that it needs to be in a subdir.

Best regards
Hervé

Re: [PATCH v3 2/7] reset: mchp: sparx5: Remove dependencies and allow building as a module

[Geert Uytterhoeven]

Hi Hervé and Clément,

On Thu, Jun 27, 2024 at 11:13 AM Herve Codina <herve.codina@bootlin.com> wrote:
> From: Clément Léger <clement.leger@bootlin.com>
>
> The sparx5 reset controller depends on the SPARX5 architecture or the
> LAN966x SoC.
>
> This reset controller can be used by the LAN966x PCI device and so it
> needs to be available on all architectures.
> Also the LAN966x PCI device driver can be built as a module and this
> reset controller driver has no reason to be a builtin driver in that
> case.
>
> Signed-off-by: Clément Léger <clement.leger@bootlin.com>
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>

Thanks for your patch!

> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -124,8 +124,7 @@ config RESET_LPC18XX
>           This enables the reset controller driver for NXP LPC18xx/43xx SoCs.
>
>  config RESET_MCHP_SPARX5
> -       bool "Microchip Sparx5 reset driver"
> -       depends on ARCH_SPARX5 || SOC_LAN966 || COMPILE_TEST
> +       tristate "Microchip Sparx5 reset driver"

This opens up the question to everyone, so I'd rather add a dependency
on MFD_LAN966X_PCI.

>         default y if SPARX5_SWITCH
>         select MFD_SYSCON
>         help

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds