[PATCH 00/17] Add support for the LAN966x PCI device using a DT overlay

[PATCH 00/17] 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 improve the MDIO controller driver to properly
   handle its reset signal.

 - Patch 8 fixes an issue in the LAN996x switch driver.

 - Patches 9 to 13 introduce the internal interrupt controller used in
   the LAN996x. It is one of the few peripherals in the LAN996x that
   are only relevant when the LAN996x is used as a PCI device, which is
   why this interrupt controller did not have a driver so far.

 - Patches 14 and 15 make some small additions to the OF core and
   PCI/OF core to consider the PCI device as an interrupt controller.
   This topic was previously mentioned in [1] to avoid the need of
   phandle interrupt parents which are not available at some points.

 - Patches 16 and 17 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

 - Patches 6, 7 and 8 to be taken by network driver maintainers

 - Patches 9 to 13 to be taken by irqchip maintainers

 - Patch 14/15 to be taken by DT/PCI maintainers

 - Patch 16/17 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/

Best regards,
Hervé

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 (12):
  dt-bindings: net: mscc-miim: Add resets property
  net: mdio: mscc-miim: Handle the switch reset
  net: lan966x: remove debugfs directory in probe() error path
  dt-bindings: interrupt-controller: Add support for Microchip LAN966x
    OIC
  irqdomain: Add missing parameter descriptions in docs
  irqdomain: Introduce irq_domain_alloc() and irq_domain_publish()
  irqchip: Add support for LAN966x OIC
  MAINTAINERS: Add the Microchip LAN966x OIC driver entry
  of: dynamic: Introduce of_changeset_add_prop_bool()
  pci: of_property: Add the interrupt-controller property in PCI device
    nodes
  mfd: Add support for LAN966x PCI device
  MAINTAINERS: Add the Microchip LAN966x PCI driver entry

 .../microchip,lan966x-oic.yaml                |  55 ++++
 .../devicetree/bindings/net/mscc,miim.yaml    |   8 +
 MAINTAINERS                                   |  12 +
 drivers/irqchip/Kconfig                       |  12 +
 drivers/irqchip/Makefile                      |   1 +
 drivers/irqchip/irq-lan966x-oic.c             | 301 ++++++++++++++++++
 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 ++++++++-
 .../ethernet/microchip/lan966x/lan966x_main.c |   6 +-
 drivers/net/mdio/mdio-mscc-miim.c             |   8 +
 drivers/of/dynamic.c                          |  25 ++
 drivers/pci/of_property.c                     |  24 ++
 drivers/pci/quirks.c                          |   1 +
 drivers/reset/Kconfig                         |   3 +-
 drivers/reset/core.c                          |   2 +
 drivers/reset/reset-microchip-sparx5.c        |  11 +-
 include/linux/irqdomain.h                     |  16 +
 include/linux/mfd/syscon.h                    |  18 ++
 include/linux/of.h                            |   3 +
 kernel/irq/irqdomain.c                        | 118 +++++--
 23 files changed, 1149 insertions(+), 44 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/microchip,lan966x-oic.yaml
 create mode 100644 drivers/irqchip/irq-lan966x-oic.c
 create mode 100644 drivers/mfd/lan966x_pci.c
 create mode 100644 drivers/mfd/lan966x_pci.dtso

-- 
2.44.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 01/17] 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..164b9bcb49c3 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 intline 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.44.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 02/17] 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 85b27c42cf65..04dbfe317fc7 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.44.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 03/17] 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.44.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 04/17] 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.44.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 05/17] 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.44.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 06/17] dt-bindings: net: mscc-miim: Add resets property

[Herve Codina]

Add the (optional) resets property.
The mscc-miim device is impacted by the switch reset especially when the
mscc-miim device is used as part of the LAN966x PCI device.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 Documentation/devicetree/bindings/net/mscc,miim.yaml | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/mscc,miim.yaml b/Documentation/devicetree/bindings/net/mscc,miim.yaml
index 5b292e7c9e46..a8c92cec85a6 100644
--- a/Documentation/devicetree/bindings/net/mscc,miim.yaml
+++ b/Documentation/devicetree/bindings/net/mscc,miim.yaml
@@ -38,6 +38,14 @@ properties:
 
   clock-frequency: true
 
+  resets:
+    items:
+      - description: Reset controller used for switch core reset (soft reset)
+
+  reset-names:
+    items:
+      - const: switch
+
 required:
   - compatible
   - reg
-- 
2.44.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 07/17] net: mdio: mscc-miim: Handle the switch reset

[Herve Codina]

The mscc-miim device can be impacted by the switch reset, at least when
this device is part of the LAN966x PCI device.

Handle this newly added (optional) resets property.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 drivers/net/mdio/mdio-mscc-miim.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/mdio/mdio-mscc-miim.c b/drivers/net/mdio/mdio-mscc-miim.c
index c29377c85307..6a6c1768f533 100644
--- a/drivers/net/mdio/mdio-mscc-miim.c
+++ b/drivers/net/mdio/mdio-mscc-miim.c
@@ -19,6 +19,7 @@
 #include <linux/platform_device.h>
 #include <linux/property.h>
 #include <linux/regmap.h>
+#include <linux/reset.h>
 
 #define MSCC_MIIM_REG_STATUS		0x0
 #define		MSCC_MIIM_STATUS_STAT_PENDING	BIT(2)
@@ -270,11 +271,18 @@ static int mscc_miim_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
 	struct regmap *mii_regmap, *phy_regmap;
+	struct reset_control *reset;
 	struct device *dev = &pdev->dev;
 	struct mscc_miim_dev *miim;
 	struct mii_bus *bus;
 	int ret;
 
+	reset = devm_reset_control_get_optional_shared(dev, "switch");
+	if (IS_ERR(reset))
+		return dev_err_probe(dev, PTR_ERR(reset), "Failed to get reset\n");
+
+	reset_control_reset(reset);
+
 	mii_regmap = ocelot_regmap_from_resource(pdev, 0,
 						 &mscc_miim_regmap_config);
 	if (IS_ERR(mii_regmap))
-- 
2.44.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 08/17] net: lan966x: remove debugfs directory in probe() error path

[Herve Codina]

A debugfs directory entry is create early during probe(). This entry is
not removed on error path leading to some "already present" issues in
case of EPROBE_DEFER.

Create this entry later in the probe() code to avoid the need to change
many 'return' in 'goto' and add the removal in the already present error
path.

Fixes: 942814840127 ("net: lan966x: Add VCAP debugFS support")
Cc: <stable@vger.kernel.org>
Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 drivers/net/ethernet/microchip/lan966x/lan966x_main.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
index 2635ef8958c8..61d88207eed4 100644
--- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
+++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
@@ -1087,8 +1087,6 @@ static int lan966x_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, lan966x);
 	lan966x->dev = &pdev->dev;
 
-	lan966x->debugfs_root = debugfs_create_dir("lan966x", NULL);
-
 	if (!device_get_mac_address(&pdev->dev, mac_addr)) {
 		ether_addr_copy(lan966x->base_mac, mac_addr);
 	} else {
@@ -1179,6 +1177,8 @@ static int lan966x_probe(struct platform_device *pdev)
 		return dev_err_probe(&pdev->dev, -ENODEV,
 				     "no ethernet-ports child found\n");
 
+	lan966x->debugfs_root = debugfs_create_dir("lan966x", NULL);
+
 	/* init switch */
 	lan966x_init(lan966x);
 	lan966x_stats_init(lan966x);
@@ -1257,6 +1257,8 @@ static int lan966x_probe(struct platform_device *pdev)
 	destroy_workqueue(lan966x->stats_queue);
 	mutex_destroy(&lan966x->stats_lock);
 
+	debugfs_remove_recursive(lan966x->debugfs_root);
+
 	return err;
 }
 
-- 
2.44.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 09/17] dt-bindings: interrupt-controller: Add support for Microchip LAN966x OIC

[Herve Codina]

The Microchip LAN966x outband interrupt controller (OIC) maps the
internal interrupt sources of the LAN966x device to an external
interrupt.
When the LAN966x device is used as a PCI device, the external interrupt
is routed to the PCI interrupt.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 .../microchip,lan966x-oic.yaml                | 55 +++++++++++++++++++
 1 file changed, 55 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/microchip,lan966x-oic.yaml

diff --git a/Documentation/devicetree/bindings/interrupt-controller/microchip,lan966x-oic.yaml b/Documentation/devicetree/bindings/interrupt-controller/microchip,lan966x-oic.yaml
new file mode 100644
index 000000000000..b2adc7174177
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/microchip,lan966x-oic.yaml
@@ -0,0 +1,55 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interrupt-controller/microchip,lan966x-oic.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip LAN966x outband interrupt controller
+
+maintainers:
+  - Herve Codina <herve.codina@bootlin.com>
+
+allOf:
+  - $ref: /schemas/interrupt-controller.yaml#
+
+description: |
+  The Microchip LAN966x outband interrupt controller (OIC) maps the internal
+  interrupt sources of the LAN966x device to an external interrupt.
+  When the LAN966x device is used as a PCI device, the external interrupt is
+  routed to the PCI interrupt.
+
+properties:
+  compatible:
+    const: microchip,lan966x-oic
+
+  '#interrupt-cells':
+    const: 2
+
+  interrupt-controller: true
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+required:
+  - compatible
+  - '#interrupt-cells'
+  - interrupt-controller
+  - interrupts
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    interrupt-controller@e00c0120 {
+        compatible = "microchip,lan966x-oic";
+        reg = <0xe00c0120 0x190>;
+        #interrupt-cells = <2>;
+        interrupt-controller;
+        interrupts = <0>;
+        interrupt-parent = <&intc>;
+    };
+...
-- 
2.44.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 10/17] irqdomain: Add missing parameter descriptions in docs

[Herve Codina]

During compilation, several warning of the following form were raised:
  Function parameter or struct member 'x' not described in 'yyy'

Add the missing function parameter descriptions.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 kernel/irq/irqdomain.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 3dd1c871e091..1ed8c4d3cf2e 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -111,6 +111,7 @@ EXPORT_SYMBOL_GPL(__irq_domain_alloc_fwnode);
 
 /**
  * irq_domain_free_fwnode - Free a non-OF-backed fwnode_handle
+ * @fwnode: fwnode_handle to free
  *
  * Free a fwnode_handle allocated with irq_domain_alloc_fwnode.
  */
@@ -981,6 +982,12 @@ EXPORT_SYMBOL_GPL(__irq_resolve_mapping);
 
 /**
  * irq_domain_xlate_onecell() - Generic xlate for direct one cell bindings
+ * @d: IRQ domain involved in the translation
+ * @ctrlr: the DT node for the device whose interrupt we're translating
+ * @intspec: the interrupt specifier data from the DT
+ * @intsize: the number of entries in @intspec
+ * @out_hwirq: pointer at which to write the hwirq number
+ * @out_type: pointer at which to write the interrupt type
  *
  * Device Tree IRQ specifier translation function which works with one cell
  * bindings where the cell value maps directly to the hwirq number.
@@ -999,6 +1006,12 @@ EXPORT_SYMBOL_GPL(irq_domain_xlate_onecell);
 
 /**
  * irq_domain_xlate_twocell() - Generic xlate for direct two cell bindings
+ * @d: IRQ domain involved in the translation
+ * @ctrlr: the DT node for the device whose interrupt we're translating
+ * @intspec: the interrupt specifier data from the DT
+ * @intsize: the number of entries in @intspec
+ * @out_hwirq: pointer at which to write the hwirq number
+ * @out_type: pointer at which to write the interrupt type
  *
  * Device Tree IRQ specifier translation function which works with two cell
  * bindings where the cell values map directly to the hwirq number
@@ -1017,6 +1030,12 @@ EXPORT_SYMBOL_GPL(irq_domain_xlate_twocell);
 
 /**
  * irq_domain_xlate_onetwocell() - Generic xlate for one or two cell bindings
+ * @d: IRQ domain involved in the translation
+ * @ctrlr: the DT node for the device whose interrupt we're translating
+ * @intspec: the interrupt specifier data from the DT
+ * @intsize: the number of entries in @intspec
+ * @out_hwirq: pointer at which to write the hwirq number
+ * @out_type: pointer at which to write the interrupt type
  *
  * Device Tree IRQ specifier translation function which works with either one
  * or two cell bindings where the cell values map directly to the hwirq number
@@ -1050,6 +1069,10 @@ EXPORT_SYMBOL_GPL(irq_domain_simple_ops);
 /**
  * irq_domain_translate_onecell() - Generic translate for direct one cell
  * bindings
+ * @d: IRQ domain involved in the translation
+ * @fwspec: FW interrupt specifier to translate
+ * @out_hwirq: pointer at which to write the hwirq number
+ * @out_type: pointer at which to write the interrupt type
  */
 int irq_domain_translate_onecell(struct irq_domain *d,
 				 struct irq_fwspec *fwspec,
@@ -1067,6 +1090,10 @@ EXPORT_SYMBOL_GPL(irq_domain_translate_onecell);
 /**
  * irq_domain_translate_twocell() - Generic translate for direct two cell
  * bindings
+ * @d: IRQ domain involved in the translation
+ * @fwspec: FW interrupt specifier to translate
+ * @out_hwirq: pointer at which to write the hwirq number
+ * @out_type: pointer at which to write the interrupt type
  *
  * Device Tree IRQ specifier translation function which works with two cell
  * bindings where the cell values map directly to the hwirq number
-- 
2.44.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 11/17] irqdomain: Introduce irq_domain_alloc() and irq_domain_publish()

[Herve Codina]

The irq_domain_add_*() family functions create an irq_domain and also
publish this newly created to domain. Once an irq_domain is published,
consumers can request IRQ in order to use them.

Some interrupt controller drivers have to perform some more operations
with the created irq_domain in order to have it ready to be used.
For instance:
  - Allocate generic irq chips with irq_alloc_domain_generic_chips()
  - Retrieve the generic irq chips with irq_get_domain_generic_chip()
  - Initialize retrieved chips: set register base address and offsets,
    set several hooks such as irq_mask, irq_unmask, ...

To avoid a window where the domain is published but not yet ready to be
used, introduce irq_domain_alloc_*() family functions to create the
irq_domain and irq_domain_publish() to publish the irq_domain.
With this new functions, any additional initialisation can then be done
between the call creating the irq_domain and the call publishing it.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 include/linux/irqdomain.h | 16 +++++++
 kernel/irq/irqdomain.c    | 91 ++++++++++++++++++++++++++++-----------
 2 files changed, 82 insertions(+), 25 deletions(-)

diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index 21ecf582a0fe..86203e7e6659 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -257,6 +257,22 @@ static inline struct fwnode_handle *irq_domain_alloc_fwnode(phys_addr_t *pa)
 }
 
 void irq_domain_free_fwnode(struct fwnode_handle *fwnode);
+struct irq_domain *irq_domain_alloc(struct fwnode_handle *fwnode, unsigned int size,
+				    irq_hw_number_t hwirq_max, int direct_max,
+				    const struct irq_domain_ops *ops,
+				    void *host_data);
+
+static inline struct irq_domain *irq_domain_alloc_linear(struct fwnode_handle *fwnode,
+							 unsigned int size,
+							 const struct irq_domain_ops *ops,
+							 void *host_data)
+{
+	return irq_domain_alloc(fwnode, size, size, 0, ops, host_data);
+}
+
+void irq_domain_free(struct irq_domain *domain);
+void irq_domain_publish(struct irq_domain *domain);
+void irq_domain_unpublish(struct irq_domain *domain);
 struct irq_domain *__irq_domain_add(struct fwnode_handle *fwnode, unsigned int size,
 				    irq_hw_number_t hwirq_max, int direct_max,
 				    const struct irq_domain_ops *ops,
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 1ed8c4d3cf2e..ed353789fb27 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -231,7 +231,38 @@ static struct irq_domain *__irq_domain_create(struct fwnode_handle *fwnode,
 	return domain;
 }
 
-static void __irq_domain_publish(struct irq_domain *domain)
+struct irq_domain *irq_domain_alloc(struct fwnode_handle *fwnode, unsigned int size,
+				    irq_hw_number_t hwirq_max, int direct_max,
+				    const struct irq_domain_ops *ops,
+				    void *host_data)
+{
+	return __irq_domain_create(fwnode, size, hwirq_max, direct_max, ops,
+				   host_data);
+}
+EXPORT_SYMBOL_GPL(irq_domain_alloc);
+
+/**
+ * irq_domain_free() - Free an irq domain.
+ * @domain: domain to free
+ *
+ * This routine is used to free an irq domain. The caller must ensure
+ * that the domain is not published.
+ */
+void irq_domain_free(struct irq_domain *domain)
+{
+	fwnode_dev_initialized(domain->fwnode, false);
+	fwnode_handle_put(domain->fwnode);
+	if (domain->flags & IRQ_DOMAIN_NAME_ALLOCATED)
+		kfree(domain->name);
+	kfree(domain);
+}
+EXPORT_SYMBOL_GPL(irq_domain_free);
+
+/**
+ * irq_domain_publish() - Publish an irq domain.
+ * @domain: domain to publish
+ */
+void irq_domain_publish(struct irq_domain *domain)
 {
 	mutex_lock(&irq_domain_mutex);
 	debugfs_add_domain_dir(domain);
@@ -240,6 +271,36 @@ static void __irq_domain_publish(struct irq_domain *domain)
 
 	pr_debug("Added domain %s\n", domain->name);
 }
+EXPORT_SYMBOL_GPL(irq_domain_publish);
+
+/**
+ * irq_domain_unpublish() - Unpublish an irq domain.
+ * @domain: domain to unpublish
+ *
+ * This routine is used to unpublish an irq domain. The caller must ensure
+ * that all mappings within the domain have been disposed of prior to
+ * use, depending on the revmap type.
+ */
+void irq_domain_unpublish(struct irq_domain *domain)
+{
+	mutex_lock(&irq_domain_mutex);
+	debugfs_remove_domain_dir(domain);
+
+	WARN_ON(!radix_tree_empty(&domain->revmap_tree));
+
+	list_del(&domain->link);
+
+	/*
+	 * If the going away domain is the default one, reset it.
+	 */
+	if (unlikely(irq_default_domain == domain))
+		irq_set_default_host(NULL);
+
+	mutex_unlock(&irq_domain_mutex);
+
+	pr_debug("Removed domain %s\n", domain->name);
+}
+EXPORT_SYMBOL_GPL(irq_domain_unpublish);
 
 /**
  * __irq_domain_add() - Allocate a new irq_domain data structure
@@ -264,7 +325,7 @@ struct irq_domain *__irq_domain_add(struct fwnode_handle *fwnode, unsigned int s
 	domain = __irq_domain_create(fwnode, size, hwirq_max, direct_max,
 				     ops, host_data);
 	if (domain)
-		__irq_domain_publish(domain);
+		irq_domain_publish(domain);
 
 	return domain;
 }
@@ -280,28 +341,8 @@ EXPORT_SYMBOL_GPL(__irq_domain_add);
  */
 void irq_domain_remove(struct irq_domain *domain)
 {
-	mutex_lock(&irq_domain_mutex);
-	debugfs_remove_domain_dir(domain);
-
-	WARN_ON(!radix_tree_empty(&domain->revmap_tree));
-
-	list_del(&domain->link);
-
-	/*
-	 * If the going away domain is the default one, reset it.
-	 */
-	if (unlikely(irq_default_domain == domain))
-		irq_set_default_host(NULL);
-
-	mutex_unlock(&irq_domain_mutex);
-
-	pr_debug("Removed domain %s\n", domain->name);
-
-	fwnode_dev_initialized(domain->fwnode, false);
-	fwnode_handle_put(domain->fwnode);
-	if (domain->flags & IRQ_DOMAIN_NAME_ALLOCATED)
-		kfree(domain->name);
-	kfree(domain);
+	irq_domain_unpublish(domain);
+	irq_domain_free(domain);
 }
 EXPORT_SYMBOL_GPL(irq_domain_remove);
 
@@ -1183,7 +1224,7 @@ struct irq_domain *irq_domain_create_hierarchy(struct irq_domain *parent,
 		domain->parent = parent;
 		domain->flags |= flags;
 
-		__irq_domain_publish(domain);
+		irq_domain_publish(domain);
 	}
 
 	return domain;
-- 
2.44.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 12/17] irqchip: Add support for LAN966x OIC

[Herve Codina]

The Microchip LAN966x outband interrupt controller (OIC) maps the
internal interrupt sources of the LAN966x device to an external
interrupt.
When the LAN966x device is used as a PCI device, the external interrupt
is routed to the PCI interrupt.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 drivers/irqchip/Kconfig           |  12 ++
 drivers/irqchip/Makefile          |   1 +
 drivers/irqchip/irq-lan966x-oic.c | 301 ++++++++++++++++++++++++++++++
 3 files changed, 314 insertions(+)
 create mode 100644 drivers/irqchip/irq-lan966x-oic.c

diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 72c07a12f5e1..973eebc8d1d1 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -169,6 +169,18 @@ config IXP4XX_IRQ
 	select IRQ_DOMAIN
 	select SPARSE_IRQ
 
+config LAN966X_OIC
+	tristate "Microchip LAN966x OIC Support"
+	select GENERIC_IRQ_CHIP
+	select IRQ_DOMAIN
+	help
+	  Enable support for the LAN966x Outbound Interrupt Controller.
+	  This controller is present on the Microchip LAN966x PCI device and
+	  maps the internal interrupts sources to PCIe interrupt.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called irq-lan966x-oic.
+
 config MADERA_IRQ
 	tristate
 
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index ec4a18380998..762d3078aa3b 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -101,6 +101,7 @@ obj-$(CONFIG_IMX_IRQSTEER)		+= irq-imx-irqsteer.o
 obj-$(CONFIG_IMX_INTMUX)		+= irq-imx-intmux.o
 obj-$(CONFIG_IMX_MU_MSI)		+= irq-imx-mu-msi.o
 obj-$(CONFIG_MADERA_IRQ)		+= irq-madera.o
+obj-$(CONFIG_LAN966X_OIC)		+= irq-lan966x-oic.o
 obj-$(CONFIG_LS1X_IRQ)			+= irq-ls1x.o
 obj-$(CONFIG_TI_SCI_INTR_IRQCHIP)	+= irq-ti-sci-intr.o
 obj-$(CONFIG_TI_SCI_INTA_IRQCHIP)	+= irq-ti-sci-inta.o
diff --git a/drivers/irqchip/irq-lan966x-oic.c b/drivers/irqchip/irq-lan966x-oic.c
new file mode 100644
index 000000000000..342f0dbf16e3
--- /dev/null
+++ b/drivers/irqchip/irq-lan966x-oic.c
@@ -0,0 +1,301 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for the Microchip LAN966x outbound interrupt controller
+ *
+ * Copyright (c) 2024 Technology Inc. and its subsidiaries.
+ *
+ * Authors:
+ *	Horatiu Vultur <horatiu.vultur@microchip.com>
+ *	Clément Léger <clement.leger@bootlin.com>
+ *	Herve Codina <herve.codina@bootlin.com>
+ */
+
+#include <linux/bitops.h>
+#include <linux/build_bug.h>
+#include <linux/interrupt.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/irqchip.h>
+#include <linux/irq.h>
+#include <linux/iopoll.h>
+#include <linux/mfd/core.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/delay.h>
+
+struct lan966x_oic_chip_regs {
+	int reg_off_ena_set;
+	int reg_off_ena_clr;
+	int reg_off_sticky;
+	int reg_off_ident;
+	int reg_off_map;
+};
+
+struct lan966x_oic_data {
+	struct irq_domain *domain;
+	void __iomem *regs;
+	int irq;
+};
+
+#define LAN966X_OIC_NR_IRQ 86
+
+/* Interrupt sticky status */
+#define LAN966X_OIC_INTR_STICKY		0x30
+#define LAN966X_OIC_INTR_STICKY1	0x34
+#define LAN966X_OIC_INTR_STICKY2	0x38
+
+/* Interrupt enable */
+#define LAN966X_OIC_INTR_ENA		0x48
+#define LAN966X_OIC_INTR_ENA1		0x4c
+#define LAN966X_OIC_INTR_ENA2		0x50
+
+/* Atomic clear of interrupt enable */
+#define LAN966X_OIC_INTR_ENA_CLR	0x54
+#define LAN966X_OIC_INTR_ENA_CLR1	0x58
+#define LAN966X_OIC_INTR_ENA_CLR2	0x5c
+
+/* Atomic set of interrupt */
+#define LAN966X_OIC_INTR_ENA_SET	0x60
+#define LAN966X_OIC_INTR_ENA_SET1	0x64
+#define LAN966X_OIC_INTR_ENA_SET2	0x68
+
+/* Mapping of source to destination interrupts (_n = 0..8) */
+#define LAN966X_OIC_DST_INTR_MAP(_n)	0x78
+#define LAN966X_OIC_DST_INTR_MAP1(_n)	0x9c
+#define LAN966X_OIC_DST_INTR_MAP2(_n)	0xc0
+
+/* Currently active interrupt sources per destination (_n = 0..8) */
+#define LAN966X_OIC_DST_INTR_IDENT(_n)	0xe4
+#define LAN966X_OIC_DST_INTR_IDENT1(_n)	0x108
+#define LAN966X_OIC_DST_INTR_IDENT2(_n)	0x12c
+
+static unsigned int lan966x_oic_irq_startup(struct irq_data *data)
+{
+	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(data);
+	struct irq_chip_type *ct = irq_data_get_chip_type(data);
+	struct lan966x_oic_chip_regs *chip_regs = gc->private;
+	u32 map;
+
+	irq_gc_lock(gc);
+
+	/* Map the source interrupt to the destination */
+	map = irq_reg_readl(gc, chip_regs->reg_off_map);
+	map |= data->mask;
+	irq_reg_writel(gc, map, chip_regs->reg_off_map);
+
+	irq_gc_unlock(gc);
+
+	ct->chip.irq_ack(data);
+	ct->chip.irq_unmask(data);
+
+	return 0;
+}
+
+static void lan966x_oic_irq_shutdown(struct irq_data *data)
+{
+	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(data);
+	struct irq_chip_type *ct = irq_data_get_chip_type(data);
+	struct lan966x_oic_chip_regs *chip_regs = gc->private;
+	u32 map;
+
+	ct->chip.irq_mask(data);
+
+	irq_gc_lock(gc);
+
+	/* Unmap the interrupt */
+	map = irq_reg_readl(gc, chip_regs->reg_off_map);
+	map &= ~data->mask;
+	irq_reg_writel(gc, map, chip_regs->reg_off_map);
+
+	irq_gc_unlock(gc);
+}
+
+static int lan966x_oic_irq_set_type(struct irq_data *data, unsigned int flow_type)
+{
+	if (flow_type != IRQ_TYPE_LEVEL_HIGH) {
+		pr_err("lan966x oic doesn't support flow type %d\n", flow_type);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static void lan966x_oic_irq_handler_domain(struct irq_domain *d, u32 first_irq)
+{
+	struct irq_chip_generic *gc = irq_get_domain_generic_chip(d, first_irq);
+	struct lan966x_oic_chip_regs *chip_regs = gc->private;
+	unsigned long ident;
+	unsigned int hwirq;
+
+	ident = irq_reg_readl(gc, chip_regs->reg_off_ident);
+	if (!ident)
+		return;
+
+	for_each_set_bit(hwirq, &ident, 32)
+		generic_handle_domain_irq(d, hwirq + first_irq);
+}
+
+static void lan966x_oic_irq_handler(struct irq_desc *desc)
+{
+	struct irq_domain *d = irq_desc_get_handler_data(desc);
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+
+	chained_irq_enter(chip, desc);
+	lan966x_oic_irq_handler_domain(d, 0);
+	lan966x_oic_irq_handler_domain(d, 32);
+	lan966x_oic_irq_handler_domain(d, 64);
+	chained_irq_exit(chip, desc);
+}
+
+static struct lan966x_oic_chip_regs lan966x_oic_chip_regs[3] = {
+	{
+		.reg_off_ena_set = LAN966X_OIC_INTR_ENA_SET,
+		.reg_off_ena_clr = LAN966X_OIC_INTR_ENA_CLR,
+		.reg_off_sticky = LAN966X_OIC_INTR_STICKY,
+		.reg_off_ident = LAN966X_OIC_DST_INTR_IDENT(0),
+		.reg_off_map = LAN966X_OIC_DST_INTR_MAP(0),
+	}, {
+		.reg_off_ena_set = LAN966X_OIC_INTR_ENA_SET1,
+		.reg_off_ena_clr = LAN966X_OIC_INTR_ENA_CLR1,
+		.reg_off_sticky = LAN966X_OIC_INTR_STICKY1,
+		.reg_off_ident = LAN966X_OIC_DST_INTR_IDENT1(0),
+		.reg_off_map = LAN966X_OIC_DST_INTR_MAP1(0),
+	}, {
+		.reg_off_ena_set = LAN966X_OIC_INTR_ENA_SET2,
+		.reg_off_ena_clr = LAN966X_OIC_INTR_ENA_CLR2,
+		.reg_off_sticky = LAN966X_OIC_INTR_STICKY2,
+		.reg_off_ident = LAN966X_OIC_DST_INTR_IDENT2(0),
+		.reg_off_map = LAN966X_OIC_DST_INTR_MAP2(0),
+	}
+};
+
+static void lan966x_oic_chip_init(struct lan966x_oic_data *lan966x_oic,
+				  struct irq_chip_generic *gc,
+				  struct lan966x_oic_chip_regs *chip_regs)
+{
+	gc->reg_base = lan966x_oic->regs;
+	gc->chip_types[0].regs.enable = chip_regs->reg_off_ena_set;
+	gc->chip_types[0].regs.disable = chip_regs->reg_off_ena_clr;
+	gc->chip_types[0].regs.ack = chip_regs->reg_off_sticky;
+	gc->chip_types[0].chip.irq_startup = lan966x_oic_irq_startup;
+	gc->chip_types[0].chip.irq_shutdown = lan966x_oic_irq_shutdown;
+	gc->chip_types[0].chip.irq_set_type = lan966x_oic_irq_set_type;
+	gc->chip_types[0].chip.irq_mask = irq_gc_mask_disable_reg;
+	gc->chip_types[0].chip.irq_unmask = irq_gc_unmask_enable_reg;
+	gc->chip_types[0].chip.irq_ack = irq_gc_ack_set_bit;
+	gc->private = chip_regs;
+
+	/* Disable all interrupts handled by this chip */
+	irq_reg_writel(gc, ~0, chip_regs->reg_off_ena_clr);
+}
+
+static void lan966x_oic_chip_exit(struct irq_chip_generic *gc)
+{
+	/* Disable and ack all interrupts handled by this chip */
+	irq_reg_writel(gc, ~0, gc->chip_types[0].regs.disable);
+	irq_reg_writel(gc, ~0, gc->chip_types[0].regs.ack);
+}
+
+static int lan966x_oic_probe(struct platform_device *pdev)
+{
+	struct device_node *node = pdev->dev.of_node;
+	struct lan966x_oic_data *lan966x_oic;
+	struct device *dev = &pdev->dev;
+	struct irq_chip_generic *gc;
+	int ret;
+	int i;
+
+	lan966x_oic = devm_kmalloc(dev, sizeof(*lan966x_oic), GFP_KERNEL);
+	if (!lan966x_oic)
+		return -ENOMEM;
+
+	lan966x_oic->regs = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(lan966x_oic->regs))
+		return dev_err_probe(dev, PTR_ERR(lan966x_oic->regs),
+				     "failed to map resource\n");
+
+	lan966x_oic->domain = irq_domain_alloc_linear(of_node_to_fwnode(node),
+						      LAN966X_OIC_NR_IRQ,
+						      &irq_generic_chip_ops, NULL);
+	if (!lan966x_oic->domain) {
+		dev_err(dev, "failed to create an IRQ domain\n");
+		return -EINVAL;
+	}
+
+	lan966x_oic->irq = platform_get_irq(pdev, 0);
+	if (lan966x_oic->irq < 0) {
+		dev_err_probe(dev, lan966x_oic->irq, "failed to get the IRQ\n");
+		goto err_domain_free;
+	}
+
+	ret = irq_alloc_domain_generic_chips(lan966x_oic->domain, 32, 1, "lan966x-oic",
+					     handle_level_irq, 0, 0, 0);
+	if (ret) {
+		dev_err_probe(dev, ret, "failed to alloc irq domain gc\n");
+		goto err_domain_free;
+	}
+
+	/* Init chips */
+	BUILD_BUG_ON(DIV_ROUND_UP(LAN966X_OIC_NR_IRQ, 32) != ARRAY_SIZE(lan966x_oic_chip_regs));
+	for (i = 0; i < ARRAY_SIZE(lan966x_oic_chip_regs); i++) {
+		gc = irq_get_domain_generic_chip(lan966x_oic->domain, i * 32);
+		lan966x_oic_chip_init(lan966x_oic, gc, &lan966x_oic_chip_regs[i]);
+	}
+
+	irq_set_chained_handler_and_data(lan966x_oic->irq, lan966x_oic_irq_handler,
+					 lan966x_oic->domain);
+
+	irq_domain_publish(lan966x_oic->domain);
+	platform_set_drvdata(pdev, lan966x_oic);
+	return 0;
+
+err_domain_free:
+	irq_domain_free(lan966x_oic->domain);
+	return ret;
+}
+
+static void lan966x_oic_remove(struct platform_device *pdev)
+{
+	struct lan966x_oic_data *lan966x_oic = platform_get_drvdata(pdev);
+	struct irq_chip_generic *gc;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(lan966x_oic_chip_regs); i++) {
+		gc = irq_get_domain_generic_chip(lan966x_oic->domain, i * 32);
+		lan966x_oic_chip_exit(gc);
+	}
+
+	irq_set_chained_handler_and_data(lan966x_oic->irq, NULL, NULL);
+
+	for (i = 0; i < LAN966X_OIC_NR_IRQ; i++)
+		irq_dispose_mapping(irq_find_mapping(lan966x_oic->domain, i));
+
+	irq_domain_unpublish(lan966x_oic->domain);
+
+	for (i = 0; i < ARRAY_SIZE(lan966x_oic_chip_regs); i++) {
+		gc = irq_get_domain_generic_chip(lan966x_oic->domain, i * 32);
+		irq_remove_generic_chip(gc, ~0, 0, 0);
+	}
+
+	kfree(lan966x_oic->domain->gc);
+	irq_domain_free(lan966x_oic->domain);
+}
+
+static const struct of_device_id lan966x_oic_of_match[] = {
+	{ .compatible = "microchip,lan966x-oic" },
+	{} /* sentinel */
+};
+MODULE_DEVICE_TABLE(of, lan966x_oic_of_match);
+
+static struct platform_driver lan966x_oic_driver = {
+	.probe = lan966x_oic_probe,
+	.remove_new = lan966x_oic_remove,
+	.driver = {
+		.name = "lan966x-oic",
+		.of_match_table = lan966x_oic_of_match,
+	},
+};
+module_platform_driver(lan966x_oic_driver);
+
+MODULE_AUTHOR("Herve Codina <herve.codina@bootlin.com>");
+MODULE_DESCRIPTION("Microchip LAN966x OIC driver");
+MODULE_LICENSE("GPL");
-- 
2.44.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 13/17] MAINTAINERS: Add the Microchip LAN966x OIC driver entry

[Herve Codina]

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

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

diff --git a/MAINTAINERS b/MAINTAINERS
index ebf03f5f0619..a15f19008d6e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14465,6 +14465,12 @@ L:	netdev@vger.kernel.org
 S:	Maintained
 F:	drivers/net/ethernet/microchip/lan966x/*
 
+MICROCHIP LAN966X OIC DRIVER
+M:	Herve Codina <herve.codina@bootlin.com>
+S:	Maintained
+F:	Documentation/devicetree/bindings/interrupt-controller/microchip,lan966x-oic.yaml
+F:	drivers/irqchip/irq-lan966x-oic.c
+
 MICROCHIP LCDFB DRIVER
 M:	Nicolas Ferre <nicolas.ferre@microchip.com>
 L:	linux-fbdev@vger.kernel.org
-- 
2.44.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 14/17] of: dynamic: Introduce of_changeset_add_prop_bool()

[Herve Codina]

APIs to add some properties in a changeset exist but nothing to add a DT
boolean property (i.e. a property without any values).

Fill this lack with of_changeset_add_prop_bool().

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 drivers/of/dynamic.c | 25 +++++++++++++++++++++++++
 include/linux/of.h   |  3 +++
 2 files changed, 28 insertions(+)

diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index 4d57a4e34105..37d3b0a272dc 100644
--- a/drivers/of/dynamic.c
+++ b/drivers/of/dynamic.c
@@ -1052,3 +1052,28 @@ int of_changeset_add_prop_u32_array(struct of_changeset *ocs,
 	return ret;
 }
 EXPORT_SYMBOL_GPL(of_changeset_add_prop_u32_array);
+
+/**
+ * of_changeset_add_prop_bool - Add a boolean property (i.e. a property without
+ * any values) to a changeset.
+ *
+ * @ocs:	changeset pointer
+ * @np:		device node pointer
+ * @prop_name:	name of the property to be added
+ *
+ * Create a boolean property and add it to a changeset.
+ *
+ * Return: 0 on success, a negative error value in case of an error.
+ */
+int of_changeset_add_prop_bool(struct of_changeset *ocs, struct device_node *np,
+			       const char *prop_name)
+{
+	struct property prop;
+
+	prop.name = (char *)prop_name;
+	prop.length = 0;
+	prop.value = NULL;
+
+	return of_changeset_add_prop_helper(ocs, np, &prop);
+}
+EXPORT_SYMBOL_GPL(of_changeset_add_prop_bool);
diff --git a/include/linux/of.h b/include/linux/of.h
index a0bedd038a05..9633199a954a 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -1652,6 +1652,9 @@ static inline int of_changeset_add_prop_u32(struct of_changeset *ocs,
 	return of_changeset_add_prop_u32_array(ocs, np, prop_name, &val, 1);
 }
 
+int of_changeset_add_prop_bool(struct of_changeset *ocs, struct device_node *np,
+			       const char *prop_name);
+
 #else /* CONFIG_OF_DYNAMIC */
 static inline int of_reconfig_notifier_register(struct notifier_block *nb)
 {
-- 
2.44.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 15/17] pci: of_property: Add the interrupt-controller property in PCI device nodes

[Herve Codina]

PCI devices and bridges DT nodes created during the PCI scan are created
with the interrupt-map property set to handle interrupts.

In order to set this interrupt-map property at a specific level, a
phandle to the parent interrupt controller is needed.
On systems that are not fully described by a device-tree, the parent
interrupt controller may be unavailable (i.e. not described by the
device-tree).

As mentioned in the [1], avoiding the use of the interrupt-map property
and considering a PCI device as an interrupt controller itself avoid the
use of a parent interrupt phandle.

In that case, the PCI device itself as an interrupt controller is
responsible for routing the interrupts described in the device-tree
world (DT overlay) to the PCI interrupts.

Add the 'interrupt-controller' property in the PCI device DT node.

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

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 drivers/pci/of_property.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/pci/of_property.c b/drivers/pci/of_property.c
index c2c7334152bc..9f8b940029ed 100644
--- a/drivers/pci/of_property.c
+++ b/drivers/pci/of_property.c
@@ -183,6 +183,26 @@ static int of_pci_prop_interrupts(struct pci_dev *pdev,
 	return of_changeset_add_prop_u32(ocs, np, "interrupts", (u32)pin);
 }
 
+static int of_pci_prop_intr_ctrl(struct pci_dev *pdev, struct of_changeset *ocs,
+				 struct device_node *np)
+{
+	int ret;
+	u8 pin;
+
+	ret = pci_read_config_byte(pdev, PCI_INTERRUPT_PIN, &pin);
+	if (ret != 0)
+		return ret;
+
+	if (!pin)
+		return 0;
+
+	ret = of_changeset_add_prop_u32(ocs, np, "#interrupt-cells", 1);
+	if (ret)
+		return ret;
+
+	return of_changeset_add_prop_bool(ocs, np, "interrupt-controller");
+}
+
 static int of_pci_prop_intr_map(struct pci_dev *pdev, struct of_changeset *ocs,
 				struct device_node *np)
 {
@@ -334,6 +354,10 @@ int of_pci_add_properties(struct pci_dev *pdev, struct of_changeset *ocs,
 		ret = of_pci_prop_intr_map(pdev, ocs, np);
 		if (ret)
 			return ret;
+	} else {
+		ret = of_pci_prop_intr_ctrl(pdev, ocs, np);
+		if (ret)
+			return ret;
 	}
 
 	ret = of_pci_prop_ranges(pdev, ocs, np);
-- 
2.44.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 16/17] 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 4b023ee229cf..e5f5d2986dd3 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..d9d886a1948f
--- /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/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/pci.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 ret ? IRQ_NONE : IRQ_HANDLED;
+}
+
+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_LEGACY);
+	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;
+	struct pci_dev *pci_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, data->dev->of_node);
+	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;
+
+	if (!dev->of_node) {
+		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;
+
+	dev_set_drvdata(dev, data);
+	data->dev = dev;
+	data->pci_dev = pdev;
+
+	ret = lan966x_pci_load_overlay(data);
+	if (ret)
+		return ret;
+
+	pci_set_master(pdev);
+
+	ret = of_platform_default_populate(dev->of_node, 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 device *dev = &pdev->dev;
+	struct lan966x_pci *data = dev_get_drvdata(dev);
+
+	of_platform_depopulate(dev);
+
+	lan966x_pci_unload_overlay(data);
+
+	pci_clear_master(pdev);
+}
+
+static struct pci_device_id lan966x_pci_ids[] = {
+	{ PCI_DEVICE(0x1055, 0x9660) },
+	{ 0, }
+};
+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 eff7f5df08e2..9933f245b781 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(0x1055, 0x9660, of_pci_make_dev_node);
 
 /*
  * Devices known to require a longer delay before first config space access
-- 
2.44.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 17/17] 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 a15f19008d6e..2dfb010dc211 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14471,6 +14471,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.44.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH 07/17] net: mdio: mscc-miim: Handle the switch reset

[Sai Krishna Gajula]

> -----Original Message-----
> From: Herve Codina <herve.codina@bootlin.com>
> Sent: Tuesday, April 30, 2024 2:07 PM
> To: Herve Codina <herve.codina@bootlin.com>; Thomas Gleixner
> <tglx@linutronix.de>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski
> <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; David S. Miller
> <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub
> Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; Lee Jones
> <lee@kernel.org>; Arnd Bergmann <arnd@arndb.de>; Horatiu Vultur
> <horatiu.vultur@microchip.com>; UNGLinuxDriver@microchip.com; Andrew
> Lunn <andrew@lunn.ch>; Heiner Kallweit <hkallweit1@gmail.com>; Russell
> King <linux@armlinux.org.uk>; Saravana Kannan <saravanak@google.com>;
> Bjorn Helgaas <bhelgaas@google.com>; Philipp Zabel
> <p.zabel@pengutronix.de>; Lars Povlsen <lars.povlsen@microchip.com>;
> Steen Hegelund <Steen.Hegelund@microchip.com>; Daniel Machon
> <daniel.machon@microchip.com>; Alexandre Belloni
> <alexandre.belloni@bootlin.com>
> Cc: linux-kernel@vger.kernel.org; devicetree@vger.kernel.org;
> netdev@vger.kernel.org; linux-pci@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; Allan Nielsen <allan.nielsen@microchip.com>;
> Steen Hegelund <steen.hegelund@microchip.com>; Luca Ceresoli
> <luca.ceresoli@bootlin.com>; Thomas Petazzoni
> <thomas.petazzoni@bootlin.com>
> Subject: [PATCH 07/17] net: mdio: mscc-miim: Handle the switch
> reset
> 
> The mscc-miim device can be impacted by the switch reset, at least when this
> device is part of the LAN966x PCI device.
> 
> Handle this newly added (optional) resets property.
> 
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> ---
>  drivers/net/mdio/mdio-mscc-miim.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/net/mdio/mdio-mscc-miim.c b/drivers/net/mdio/mdio-
> mscc-miim.c
> index c29377c85307..6a6c1768f533 100644
> --- a/drivers/net/mdio/mdio-mscc-miim.c
> +++ b/drivers/net/mdio/mdio-mscc-miim.c
> @@ -19,6 +19,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/property.h>
>  #include <linux/regmap.h>
> +#include <linux/reset.h>
> 
>  #define MSCC_MIIM_REG_STATUS		0x0
>  #define		MSCC_MIIM_STATUS_STAT_PENDING	BIT(2)
> @@ -270,11 +271,18 @@ static int mscc_miim_probe(struct platform_device
> *pdev)  {
>  	struct device_node *np = pdev->dev.of_node;
>  	struct regmap *mii_regmap, *phy_regmap;
> +	struct reset_control *reset;

Please follow reverse x-mass tree order

>  	struct device *dev = &pdev->dev;
>  	struct mscc_miim_dev *miim;
>  	struct mii_bus *bus;
>  	int ret;
> 
> +	reset = devm_reset_control_get_optional_shared(dev, "switch");
> +	if (IS_ERR(reset))
> +		return dev_err_probe(dev, PTR_ERR(reset), "Failed to get
> reset\n");
> +
> +	reset_control_reset(reset);
> +
>  	mii_regmap = ocelot_regmap_from_resource(pdev, 0,
> 
> &mscc_miim_regmap_config);
>  	if (IS_ERR(mii_regmap))
> --
> 2.44.0
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 00/17] Add support for the LAN966x PCI device using a DT overlay

[Andrew Lunn]

On Tue, Apr 30, 2024 at 10:37:09AM +0200, Herve Codina wrote:
> Hi,
> 
> This series adds support for the LAN966x chip when used as a PCI
> device.

> 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.

What host systems have you tested with? Are they all native DT, or
have you tested on an ACPI system? I'm just wondering how well DT
overlay works if i were to plug a LAN966x device into something like
an x86 ComExpress board?

	Andrew

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 07/17] net: mdio: mscc-miim: Handle the switch reset

[Andrew Lunn]

On Tue, Apr 30, 2024 at 10:37:16AM +0200, Herve Codina wrote:
> The mscc-miim device can be impacted by the switch reset, at least when
> this device is part of the LAN966x PCI device.

Just to be sure i understand this correctly. The MDIO bus master can
be reset using the "switch" reset. So you are adding code to ensure
the "switch" reset is out of reset state, so the MDIO bus master
works.

Given that this is a new property, maybe give it a better name to
indicate it resets both the switch and the MDIO bus master?

	 Andrew

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 06/17] dt-bindings: net: mscc-miim: Add resets property

[Andrew Lunn]

On Tue, Apr 30, 2024 at 10:37:15AM +0200, Herve Codina wrote:
> Add the (optional) resets property.
> The mscc-miim device is impacted by the switch reset especially when the
> mscc-miim device is used as part of the LAN966x PCI device.
> 
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> ---
>  Documentation/devicetree/bindings/net/mscc,miim.yaml | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/mscc,miim.yaml b/Documentation/devicetree/bindings/net/mscc,miim.yaml
> index 5b292e7c9e46..a8c92cec85a6 100644
> --- a/Documentation/devicetree/bindings/net/mscc,miim.yaml
> +++ b/Documentation/devicetree/bindings/net/mscc,miim.yaml
> @@ -38,6 +38,14 @@ properties:
>  
>    clock-frequency: true
>  
> +  resets:
> +    items:
> +      - description: Reset controller used for switch core reset (soft reset)

A follow up to the comment on the next patch. I think it should be
made clear in the patch and the binding, the aim is to reset the MDIO
bus master, not the switch. It just happens that the MDIO bus master
is within the domain of the switch core, and so the switch core reset
also resets the MDIO bus master.

Architecturally, this is important. I would not expect the MDIO driver
to be resetting the switch, the switch driver should be doing
that. But we have seen some odd Qualcomm patches where the MDIO driver
has been doing things outside the scope of MDIO, playing with resets
and clocks which are not directly related to the MDIO bus master. I
want to avoid any confusion here, especially when Qualcomm tries
again, and maybe points at this code.

	Andrew

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 08/17] net: lan966x: remove debugfs directory in probe() error path

[Andrew Lunn]

On Tue, Apr 30, 2024 at 10:37:17AM +0200, Herve Codina wrote:
> A debugfs directory entry is create early during probe(). This entry is
> not removed on error path leading to some "already present" issues in
> case of EPROBE_DEFER.
> 
> Create this entry later in the probe() code to avoid the need to change
> many 'return' in 'goto' and add the removal in the already present error
> path.
> 
> Fixes: 942814840127 ("net: lan966x: Add VCAP debugFS support")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>

I know you plan to split this patchset up and submit via different
subsystems. When you do, please post this for net, not net-next, since
it is a fix.

   Andrew

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 08/17] net: lan966x: remove debugfs directory in probe() error path

[Andrew Lunn]

On Tue, Apr 30, 2024 at 10:37:17AM +0200, Herve Codina wrote:
> A debugfs directory entry is create early during probe(). This entry is
> not removed on error path leading to some "already present" issues in
> case of EPROBE_DEFER.
> 
> Create this entry later in the probe() code to avoid the need to change
> many 'return' in 'goto' and add the removal in the already present error
> path.
> 
> Fixes: 942814840127 ("net: lan966x: Add VCAP debugFS support")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 06/17] dt-bindings: net: mscc-miim: Add resets property

[Herve Codina]

Hi Andrew,

On Tue, 30 Apr 2024 15:55:58 +0200
Andrew Lunn <andrew@lunn.ch> wrote:

> On Tue, Apr 30, 2024 at 10:37:15AM +0200, Herve Codina wrote:
> > Add the (optional) resets property.
> > The mscc-miim device is impacted by the switch reset especially when the
> > mscc-miim device is used as part of the LAN966x PCI device.
> > 
> > Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> > ---
> >  Documentation/devicetree/bindings/net/mscc,miim.yaml | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/net/mscc,miim.yaml b/Documentation/devicetree/bindings/net/mscc,miim.yaml
> > index 5b292e7c9e46..a8c92cec85a6 100644
> > --- a/Documentation/devicetree/bindings/net/mscc,miim.yaml
> > +++ b/Documentation/devicetree/bindings/net/mscc,miim.yaml
> > @@ -38,6 +38,14 @@ properties:
> >  
> >    clock-frequency: true
> >  
> > +  resets:
> > +    items:
> > +      - description: Reset controller used for switch core reset (soft reset)  
> 
> A follow up to the comment on the next patch. I think it should be
> made clear in the patch and the binding, the aim is to reset the MDIO
> bus master, not the switch. It just happens that the MDIO bus master
> is within the domain of the switch core, and so the switch core reset
> also resets the MDIO bus master.

Exactly.

> 
> Architecturally, this is important. I would not expect the MDIO driver
> to be resetting the switch, the switch driver should be doing
> that. But we have seen some odd Qualcomm patches where the MDIO driver
> has been doing things outside the scope of MDIO, playing with resets
> and clocks which are not directly related to the MDIO bus master. I
> want to avoid any confusion here, especially when Qualcomm tries
> again, and maybe points at this code.
> 

Sure.

We have the same construction with the pinctrl driver used in the LAN966x
  Documentation/devicetree/bindings/pinctrl/mscc,ocelot-pinctrl.yaml

The reset name is 'switch' in the pinctrl binding.
I can use the same description here as the one present in the pinctrl binding:
  description: Optional shared switch reset.
and keep 'switch' as reset name here (consistent with pinctrl reset name).

What do you think about that ?

Best regards,
Hervé

-- 
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 07/17] net: mdio: mscc-miim: Handle the switch reset

[Herve Codina]

Hi Andrew,

On Tue, 30 Apr 2024 15:46:18 +0200
Andrew Lunn <andrew@lunn.ch> wrote:

> On Tue, Apr 30, 2024 at 10:37:16AM +0200, Herve Codina wrote:
> > The mscc-miim device can be impacted by the switch reset, at least when
> > this device is part of the LAN966x PCI device.  
> 
> Just to be sure i understand this correctly. The MDIO bus master can
> be reset using the "switch" reset. So you are adding code to ensure
> the "switch" reset is out of reset state, so the MDIO bus master
> works.

Exactly.

> 
> Given that this is a new property, maybe give it a better name to
> indicate it resets both the switch and the MDIO bus master?
> 

Replied in the patch in the same series introducing the property
  [PATCH 06/17] dt-bindings: net: mscc-miim: Add resets property

Thanks for the feedback.
Best regards,
Hervé

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 06/17] dt-bindings: net: mscc-miim: Add resets property

[Andrew Lunn]

> We have the same construction with the pinctrl driver used in the LAN966x
>   Documentation/devicetree/bindings/pinctrl/mscc,ocelot-pinctrl.yaml
> 
> The reset name is 'switch' in the pinctrl binding.
> I can use the same description here as the one present in the pinctrl binding:
>   description: Optional shared switch reset.
> and keep 'switch' as reset name here (consistent with pinctrl reset name).
> 
> What do you think about that ?

It would be good to document what it is shared with. So it seems to be
the switch itself, pinctl and MDIO? Anything else?

    Andrew

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 00/17] Add support for the LAN966x PCI device using a DT overlay

[Herve Codina]

Hi Andrew,

On Tue, 30 Apr 2024 15:40:16 +0200
Andrew Lunn <andrew@lunn.ch> wrote:

> On Tue, Apr 30, 2024 at 10:37:09AM +0200, Herve Codina wrote:
> > Hi,
> > 
> > This series adds support for the LAN966x chip when used as a PCI
> > device.  
> 
> > 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.  
> 
> What host systems have you tested with? Are they all native DT, or
> have you tested on an ACPI system? I'm just wondering how well DT
> overlay works if i were to plug a LAN966x device into something like
> an x86 ComExpress board?

I tested on a native DT system (marvell board).

Also I tested on a x86 system (basically a simple PC).
Not all components are available upstream to have it working on a x86 (ACPI)
system. The missing component is not related to the LAN966x PCI driver itself
but in the way DT node are created up to the PCI device.
A DT node at PCI device is needed to have a DT node parent (target) for the
overlay.
The DT node chain is also important because BARs address translations are
done using the 'ranges' property available in each node in the chain.

On a full DT system, the DT looks like (simplified in naming and without the
node properties):
/
  soc {
    ...
    pci_root_bridge {  <-- Present in base dts
       pci_to_pci_bridge {  <-- Created at runtime based on PCI enumeration
            pci_device {  <-- Created at runtime based on PCI enumeration
            }
       }
       pci_device { <-- Created at runtime based on PCI enumeration
       };
    };
  };

On x86:
- A DT root empty node is created.
  This is already upstream from a first proposal [1] and then second one [2].
- PCI-to-PCI bridge and device DT nodes are created at runtime.
  This is the same on DT base systems and this feature is available upstream
  too (last proposal at [3]).

The DT node missing in the chain is the node for the PCI root bridge.
On ACPI, no "base" dts describes this node. The PCI root bridge is described
by ACPI.

I have some draft code that create this DT node when a PCI host bridge is
register if a DT node is not already present and so fill the hole in the DT
node chain.
With that the DT in an ACPI system looks like this:
/
  pci_root_bridge {  <-- Created at runtime when a PCI host bridge is registered
     pci_to_pci_bridge {  <-- Created at runtime based on PCI enumeration
          pci_device {  <-- Created at runtime based on PCI enumeration
          }
     }
     pci_device { <-- Created at runtime based on PCI enumeration
     };
  };

With this node added, the driver works the same way in both DT and ACPI
systems without any modification.

I plan to send the code creating the PCI host bridge node when this current
series is landed in order to add support for DT overlay over PCI on x86
systems.

Also an other series (under review [4]) is needed to avoid struct device
duplication related to the DT nodes creation.

[1] https://lore.kernel.org/lkml/20230317053415.2254616-1-frowand.list@gmail.com/#r
[2] https://lore.kernel.org/lkml/20240217010557.2381548-1-sboyd@kernel.org/
[3] https://lore.kernel.org/lkml/1692120000-46900-1-git-send-email-lizhi.hou@amd.com/
[4] https://lore.kernel.org/lkml/20240423145703.604489-1-herve.codina@bootlin.com/

Best regards,
Hervé

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 00/17] Add support for the LAN966x PCI device using a DT overlay

[Andrew Lunn]

> Also I tested on a x86 system (basically a simple PC).
> Not all components are available upstream to have it working on a x86 (ACPI)
> system. The missing component is not related to the LAN966x PCI driver itself
> but in the way DT node are created up to the PCI device.

Good to hear it nearly "just works". There does not seem to be any
interest in describing complex network devices like this using ACPI,
which is many years behind what we have in DT in terms of building
blocks for networking devices. Like many PCIe devices, the LAN966x is
pretty much self contained, so fits DT overlays nicely.

There is also a slowly growing trend to have PCIe network devices
which Linux controls, rather than offloading to firmware. The wangxun
drivers are another example. So it is great to see the remaining
pieces being put in place to support this.

Thanks
	Andrew

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 12/17] irqchip: Add support for LAN966x OIC

[Simon Horman]

On Tue, Apr 30, 2024 at 10:37:21AM +0200, Herve Codina wrote:
> The Microchip LAN966x outband interrupt controller (OIC) maps the
> internal interrupt sources of the LAN966x device to an external
> interrupt.
> When the LAN966x device is used as a PCI device, the external interrupt
> is routed to the PCI interrupt.
> 
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>

Hi Herve,

> +static int lan966x_oic_probe(struct platform_device *pdev)
> +{
> +	struct device_node *node = pdev->dev.of_node;
> +	struct lan966x_oic_data *lan966x_oic;
> +	struct device *dev = &pdev->dev;
> +	struct irq_chip_generic *gc;
> +	int ret;
> +	int i;
> +
> +	lan966x_oic = devm_kmalloc(dev, sizeof(*lan966x_oic), GFP_KERNEL);
> +	if (!lan966x_oic)
> +		return -ENOMEM;
> +
> +	lan966x_oic->regs = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(lan966x_oic->regs))
> +		return dev_err_probe(dev, PTR_ERR(lan966x_oic->regs),
> +				     "failed to map resource\n");
> +
> +	lan966x_oic->domain = irq_domain_alloc_linear(of_node_to_fwnode(node),
> +						      LAN966X_OIC_NR_IRQ,
> +						      &irq_generic_chip_ops, NULL);

nit: Please consider limiting lines to 80 columns wide in Networking code.

> +	if (!lan966x_oic->domain) {
> +		dev_err(dev, "failed to create an IRQ domain\n");
> +		return -EINVAL;
> +	}
> +
> +	lan966x_oic->irq = platform_get_irq(pdev, 0);
> +	if (lan966x_oic->irq < 0) {
> +		dev_err_probe(dev, lan966x_oic->irq, "failed to get the IRQ\n");
> +		goto err_domain_free;

Hi,

This will result in the function returning ret.
However, ret is uninitialised here.

Flagged by W=1 builds with clang-18, and Smatch.

> +	}
> +
> +	ret = irq_alloc_domain_generic_chips(lan966x_oic->domain, 32, 1, "lan966x-oic",
> +					     handle_level_irq, 0, 0, 0);
> +	if (ret) {
> +		dev_err_probe(dev, ret, "failed to alloc irq domain gc\n");
> +		goto err_domain_free;
> +	}
> +
> +	/* Init chips */
> +	BUILD_BUG_ON(DIV_ROUND_UP(LAN966X_OIC_NR_IRQ, 32) != ARRAY_SIZE(lan966x_oic_chip_regs));
> +	for (i = 0; i < ARRAY_SIZE(lan966x_oic_chip_regs); i++) {
> +		gc = irq_get_domain_generic_chip(lan966x_oic->domain, i * 32);
> +		lan966x_oic_chip_init(lan966x_oic, gc, &lan966x_oic_chip_regs[i]);
> +	}
> +
> +	irq_set_chained_handler_and_data(lan966x_oic->irq, lan966x_oic_irq_handler,
> +					 lan966x_oic->domain);
> +
> +	irq_domain_publish(lan966x_oic->domain);
> +	platform_set_drvdata(pdev, lan966x_oic);
> +	return 0;
> +
> +err_domain_free:
> +	irq_domain_free(lan966x_oic->domain);
> +	return ret;
> +}
> +
> +static void lan966x_oic_remove(struct platform_device *pdev)
> +{
> +	struct lan966x_oic_data *lan966x_oic = platform_get_drvdata(pdev);
> +	struct irq_chip_generic *gc;
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(lan966x_oic_chip_regs); i++) {
> +		gc = irq_get_domain_generic_chip(lan966x_oic->domain, i * 32);
> +		lan966x_oic_chip_exit(gc);
> +	}
> +
> +	irq_set_chained_handler_and_data(lan966x_oic->irq, NULL, NULL);
> +
> +	for (i = 0; i < LAN966X_OIC_NR_IRQ; i++)
> +		irq_dispose_mapping(irq_find_mapping(lan966x_oic->domain, i));
> +
> +	irq_domain_unpublish(lan966x_oic->domain);
> +
> +	for (i = 0; i < ARRAY_SIZE(lan966x_oic_chip_regs); i++) {
> +		gc = irq_get_domain_generic_chip(lan966x_oic->domain, i * 32);
> +		irq_remove_generic_chip(gc, ~0, 0, 0);
> +	}
> +
> +	kfree(lan966x_oic->domain->gc);
> +	irq_domain_free(lan966x_oic->domain);
> +}

...

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 01/17] mfd: syscon: Add reference counting and device managed support

[Simon Horman]

On Tue, Apr 30, 2024 at 10:37:10AM +0200, Herve Codina wrote:
> 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>

...

> diff --git a/include/linux/mfd/syscon.h b/include/linux/mfd/syscon.h
> index c315903f6dab..164b9bcb49c3 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 intline void syscon_put_regmap(struct regmap *regmap)

intline -> inline

> +{
> +}

...

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 01/17] mfd: syscon: Add reference counting and device managed support

[kernel test robot]

Hi Herve,

kernel test robot noticed the following build errors:

[auto build test ERROR on tip/irq/core]
[also build test ERROR on lee-mfd/for-mfd-next pza/reset/next linus/master v6.9-rc6 next-20240430]
[cannot apply to lee-mfd/for-mfd-fixes pza/imx-drm/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Herve-Codina/mfd-syscon-Add-reference-counting-and-device-managed-support/20240430-164912
base:   tip/irq/core
patch link:    https://lore.kernel.org/r/20240430083730.134918-2-herve.codina%40bootlin.com
patch subject: [PATCH 01/17] mfd: syscon: Add reference counting and device managed support
config: i386-buildonly-randconfig-005-20240501 (https://download.01.org/0day-ci/archive/20240501/202405010521.dW27hHxi-lkp@intel.com/config)
compiler: gcc-13 (Ubuntu 13.2.0-4ubuntu3) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240501/202405010521.dW27hHxi-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202405010521.dW27hHxi-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

   In file included from drivers/misc/sram.c:18:
>> include/linux/mfd/syscon.h:76:15: error: expected ';' before 'void'
      76 | static intline void syscon_put_regmap(struct regmap *regmap)
         |               ^~~~~
         |               ;
>> include/linux/mfd/syscon.h:76:21: warning: no previous prototype for 'syscon_put_regmap' [-Wmissing-prototypes]
      76 | static intline void syscon_put_regmap(struct regmap *regmap)
         |                     ^~~~~~~~~~~~~~~~~


vim +76 include/linux/mfd/syscon.h

    75	
  > 76	static intline void syscon_put_regmap(struct regmap *regmap)
    77	{
    78	}
    79	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 01/17] mfd: syscon: Add reference counting and device managed support

[kernel test robot]

Hi Herve,

kernel test robot noticed the following build errors:

[auto build test ERROR on tip/irq/core]
[also build test ERROR on lee-mfd/for-mfd-next pza/reset/next linus/master v6.9-rc6 next-20240430]
[cannot apply to lee-mfd/for-mfd-fixes pza/imx-drm/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Herve-Codina/mfd-syscon-Add-reference-counting-and-device-managed-support/20240430-164912
base:   tip/irq/core
patch link:    https://lore.kernel.org/r/20240430083730.134918-2-herve.codina%40bootlin.com
patch subject: [PATCH 01/17] mfd: syscon: Add reference counting and device managed support
config: x86_64-rhel-8.3 (https://download.01.org/0day-ci/archive/20240501/202405010513.GHoo4Vwg-lkp@intel.com/config)
compiler: gcc-13 (Ubuntu 13.2.0-4ubuntu3) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240501/202405010513.GHoo4Vwg-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202405010513.GHoo4Vwg-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from drivers/net/can/c_can/c_can_platform.c:36:
>> include/linux/mfd/syscon.h:76:15: error: expected ';' before 'void'
      76 | static intline void syscon_put_regmap(struct regmap *regmap)
         |               ^~~~~
         |               ;
   include/linux/mfd/syscon.h:76:21: warning: no previous prototype for 'syscon_put_regmap' [-Wmissing-prototypes]
      76 | static intline void syscon_put_regmap(struct regmap *regmap)
         |                     ^~~~~~~~~~~~~~~~~


vim +76 include/linux/mfd/syscon.h

    75	
  > 76	static intline void syscon_put_regmap(struct regmap *regmap)
    77	{
    78	}
    79	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 12/17] irqchip: Add support for LAN966x OIC

[kernel test robot]

Hi Herve,

kernel test robot noticed the following build warnings:

[auto build test WARNING on tip/irq/core]
[also build test WARNING on linus/master v6.9-rc6 next-20240430]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Herve-Codina/mfd-syscon-Add-reference-counting-and-device-managed-support/20240430-164912
base:   tip/irq/core
patch link:    https://lore.kernel.org/r/20240430083730.134918-13-herve.codina%40bootlin.com
patch subject: [PATCH 12/17] irqchip: Add support for LAN966x OIC
config: hexagon-randconfig-r071-20240501 (https://download.01.org/0day-ci/archive/20240501/202405010842.zim8X3E5-lkp@intel.com/config)
compiler: clang version 15.0.7 (https://github.com/llvm/llvm-project.git 8dfdcc7b7bf66834a761bd8de445840ef68e4d1a)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240501/202405010842.zim8X3E5-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202405010842.zim8X3E5-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from drivers/irqchip/irq-lan966x-oic.c:15:
   In file included from include/linux/interrupt.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __raw_readb(PCI_IOBASE + addr);
                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
   #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
                                                     ^
   In file included from drivers/irqchip/irq-lan966x-oic.c:15:
   In file included from include/linux/interrupt.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
                                                     ^
   In file included from drivers/irqchip/irq-lan966x-oic.c:15:
   In file included from include/linux/interrupt.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writeb(value, PCI_IOBASE + addr);
                               ~~~~~~~~~~ ^
   include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
>> drivers/irqchip/irq-lan966x-oic.c:225:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
           if (lan966x_oic->irq < 0) {
               ^~~~~~~~~~~~~~~~~~~~
   drivers/irqchip/irq-lan966x-oic.c:253:9: note: uninitialized use occurs here
           return ret;
                  ^~~
   drivers/irqchip/irq-lan966x-oic.c:225:2: note: remove the 'if' if its condition is always false
           if (lan966x_oic->irq < 0) {
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/irqchip/irq-lan966x-oic.c:204:9: note: initialize the variable 'ret' to silence this warning
           int ret;
                  ^
                   = 0
   7 warnings generated.


vim +225 drivers/irqchip/irq-lan966x-oic.c

   197	
   198	static int lan966x_oic_probe(struct platform_device *pdev)
   199	{
   200		struct device_node *node = pdev->dev.of_node;
   201		struct lan966x_oic_data *lan966x_oic;
   202		struct device *dev = &pdev->dev;
   203		struct irq_chip_generic *gc;
   204		int ret;
   205		int i;
   206	
   207		lan966x_oic = devm_kmalloc(dev, sizeof(*lan966x_oic), GFP_KERNEL);
   208		if (!lan966x_oic)
   209			return -ENOMEM;
   210	
   211		lan966x_oic->regs = devm_platform_ioremap_resource(pdev, 0);
   212		if (IS_ERR(lan966x_oic->regs))
   213			return dev_err_probe(dev, PTR_ERR(lan966x_oic->regs),
   214					     "failed to map resource\n");
   215	
   216		lan966x_oic->domain = irq_domain_alloc_linear(of_node_to_fwnode(node),
   217							      LAN966X_OIC_NR_IRQ,
   218							      &irq_generic_chip_ops, NULL);
   219		if (!lan966x_oic->domain) {
   220			dev_err(dev, "failed to create an IRQ domain\n");
   221			return -EINVAL;
   222		}
   223	
   224		lan966x_oic->irq = platform_get_irq(pdev, 0);
 > 225		if (lan966x_oic->irq < 0) {
   226			dev_err_probe(dev, lan966x_oic->irq, "failed to get the IRQ\n");
   227			goto err_domain_free;
   228		}
   229	
   230		ret = irq_alloc_domain_generic_chips(lan966x_oic->domain, 32, 1, "lan966x-oic",
   231						     handle_level_irq, 0, 0, 0);
   232		if (ret) {
   233			dev_err_probe(dev, ret, "failed to alloc irq domain gc\n");
   234			goto err_domain_free;
   235		}
   236	
   237		/* Init chips */
   238		BUILD_BUG_ON(DIV_ROUND_UP(LAN966X_OIC_NR_IRQ, 32) != ARRAY_SIZE(lan966x_oic_chip_regs));
   239		for (i = 0; i < ARRAY_SIZE(lan966x_oic_chip_regs); i++) {
   240			gc = irq_get_domain_generic_chip(lan966x_oic->domain, i * 32);
   241			lan966x_oic_chip_init(lan966x_oic, gc, &lan966x_oic_chip_regs[i]);
   242		}
   243	
   244		irq_set_chained_handler_and_data(lan966x_oic->irq, lan966x_oic_irq_handler,
   245						 lan966x_oic->domain);
   246	
   247		irq_domain_publish(lan966x_oic->domain);
   248		platform_set_drvdata(pdev, lan966x_oic);
   249		return 0;
   250	
   251	err_domain_free:
   252		irq_domain_free(lan966x_oic->domain);
   253		return ret;
   254	}
   255	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 15/17] pci: of_property: Add the interrupt-controller property in PCI device nodes

[Bjorn Helgaas]

In subject: s/pci:/PCI:/ to match history. s/Add the/Add/ for brevity.

On Tue, Apr 30, 2024 at 10:37:24AM +0200, Herve Codina wrote:
> PCI devices and bridges DT nodes created during the PCI scan are created
> with the interrupt-map property set to handle interrupts.
> 
> In order to set this interrupt-map property at a specific level, a
> phandle to the parent interrupt controller is needed.
> On systems that are not fully described by a device-tree, the parent
> interrupt controller may be unavailable (i.e. not described by the
> device-tree).

Rewrap into one paragraph or add blank line to separate paragraphs.

> As mentioned in the [1], avoiding the use of the interrupt-map property
> and considering a PCI device as an interrupt controller itself avoid the
> use of a parent interrupt phandle.
> 
> In that case, the PCI device itself as an interrupt controller is
> responsible for routing the interrupts described in the device-tree
> world (DT overlay) to the PCI interrupts.
> 
> Add the 'interrupt-controller' property in the PCI device DT node.
> 
> [1]: https://lore.kernel.org/lkml/CAL_Jsq+je7+9ATR=B6jXHjEJHjn24vQFs4Tvi9=vhDeK9n42Aw@mail.gmail.com/
> 
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> ---
>  drivers/pci/of_property.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/drivers/pci/of_property.c b/drivers/pci/of_property.c
> index c2c7334152bc..9f8b940029ed 100644
> --- a/drivers/pci/of_property.c
> +++ b/drivers/pci/of_property.c
> @@ -183,6 +183,26 @@ static int of_pci_prop_interrupts(struct pci_dev *pdev,
>  	return of_changeset_add_prop_u32(ocs, np, "interrupts", (u32)pin);
>  }
>  
> +static int of_pci_prop_intr_ctrl(struct pci_dev *pdev, struct of_changeset *ocs,
> +				 struct device_node *np)
> +{
> +	int ret;
> +	u8 pin;
> +
> +	ret = pci_read_config_byte(pdev, PCI_INTERRUPT_PIN, &pin);
> +	if (ret != 0)
> +		return ret;
> +
> +	if (!pin)
> +		return 0;
> +
> +	ret = of_changeset_add_prop_u32(ocs, np, "#interrupt-cells", 1);
> +	if (ret)
> +		return ret;
> +
> +	return of_changeset_add_prop_bool(ocs, np, "interrupt-controller");
> +}
> +
>  static int of_pci_prop_intr_map(struct pci_dev *pdev, struct of_changeset *ocs,
>  				struct device_node *np)
>  {
> @@ -334,6 +354,10 @@ int of_pci_add_properties(struct pci_dev *pdev, struct of_changeset *ocs,
>  		ret = of_pci_prop_intr_map(pdev, ocs, np);
>  		if (ret)
>  			return ret;
> +	} else {
> +		ret = of_pci_prop_intr_ctrl(pdev, ocs, np);
> +		if (ret)
> +			return ret;
>  	}
>  
>  	ret = of_pci_prop_ranges(pdev, ocs, np);
> -- 
> 2.44.0
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 10/17] irqdomain: Add missing parameter descriptions in docs

[Thomas Gleixner]

On Tue, Apr 30 2024 at 10:37, Herve Codina wrote:
>  /**
>   * irq_domain_xlate_onecell() - Generic xlate for direct one cell bindings
> + * @d: IRQ domain involved in the translation

Please write out 'Interrupt domain'

> + * @ctrlr: the DT node for the device whose interrupt we're translating

Device tree node. And we are not translating anything.

> + * @intspec: the interrupt specifier data from the DT
> + * @intsize: the number of entries in @intspec
> + * @out_hwirq: pointer at which to write the hwirq number

Pointer to starage for the hardware interrupt number

> + * @out_type: pointer at which to write the interrupt type

...

Please align these in tabular fashion:

+ * @d:         Interrupt domain involved in the translation
+ * @ctrlr:     The device tree node for the device whose interrupt is translated
+ * @intspec:   The interrupt specifier data from the device tree
+ * @intsize:   The number of entries in @intspec
+ * @out_hwirq: Pointer to storage for the hardware interrupt number
+ * @out_type:  Pointer to storage for the interrupt type


>  /**
>   * irq_domain_translate_onecell() - Generic translate for direct one cell
>   * bindings
> + * @d: IRQ domain involved in the translation
> + * @fwspec: FW interrupt specifier to translate

Firmware interrupt specifier

Thanks,

        tglx

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 06/17] dt-bindings: net: mscc-miim: Add resets property

[Herve Codina]

Hi Andrew,

On Tue, 30 Apr 2024 18:31:46 +0200
Andrew Lunn <andrew@lunn.ch> wrote:

> > We have the same construction with the pinctrl driver used in the LAN966x
> >   Documentation/devicetree/bindings/pinctrl/mscc,ocelot-pinctrl.yaml
> > 
> > The reset name is 'switch' in the pinctrl binding.
> > I can use the same description here as the one present in the pinctrl binding:
> >   description: Optional shared switch reset.
> > and keep 'switch' as reset name here (consistent with pinctrl reset name).
> > 
> > What do you think about that ?  
> 
> It would be good to document what it is shared with. So it seems to be
> the switch itself, pinctl and MDIO? Anything else?
> 

To be honest, I know that the GPIO controller (microchip,sparx5-sgpio) is
impacted but I don't know if anything else is impacted by this reset.
I can update the description with:
  description:
    Optional shared switch reset.
    This reset is shared with at least pinctrl, GPIO, MDIO and the switch
    itself.

Does it sound better ?

Best regards
Hervé

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 06/17] dt-bindings: net: mscc-miim: Add resets property

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 1326 bytes --]

On Thu, May 02, 2024 at 11:50:43AM +0200, Herve Codina wrote:
> Hi Andrew,
> 
> On Tue, 30 Apr 2024 18:31:46 +0200
> Andrew Lunn <andrew@lunn.ch> wrote:
> 
> > > We have the same construction with the pinctrl driver used in the LAN966x
> > >   Documentation/devicetree/bindings/pinctrl/mscc,ocelot-pinctrl.yaml
> > > 
> > > The reset name is 'switch' in the pinctrl binding.
> > > I can use the same description here as the one present in the pinctrl binding:
> > >   description: Optional shared switch reset.
> > > and keep 'switch' as reset name here (consistent with pinctrl reset name).
> > > 
> > > What do you think about that ?  
> > 
> > It would be good to document what it is shared with. So it seems to be
> > the switch itself, pinctl and MDIO? Anything else?
> > 
> 
> To be honest, I know that the GPIO controller (microchip,sparx5-sgpio) is
> impacted but I don't know if anything else is impacted by this reset.
> I can update the description with:
>   description:
>     Optional shared switch reset.
>     This reset is shared with at least pinctrl, GPIO, MDIO and the switch
>     itself.
> 
> Does it sound better ?

$dayjob hat off, bindings hat on: If you don't know, can we get someone
from Microchip (there's some and a list in CC) to figure it out?

Cheers,
Conor.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 06/17] dt-bindings: net: mscc-miim: Add resets property

[Andrew Lunn]

On Thu, May 02, 2024 at 11:31:00AM +0100, Conor Dooley wrote:
> On Thu, May 02, 2024 at 11:50:43AM +0200, Herve Codina wrote:
> > Hi Andrew,
> > 
> > On Tue, 30 Apr 2024 18:31:46 +0200
> > Andrew Lunn <andrew@lunn.ch> wrote:
> > 
> > > > We have the same construction with the pinctrl driver used in the LAN966x
> > > >   Documentation/devicetree/bindings/pinctrl/mscc,ocelot-pinctrl.yaml
> > > > 
> > > > The reset name is 'switch' in the pinctrl binding.
> > > > I can use the same description here as the one present in the pinctrl binding:
> > > >   description: Optional shared switch reset.
> > > > and keep 'switch' as reset name here (consistent with pinctrl reset name).
> > > > 
> > > > What do you think about that ?  
> > > 
> > > It would be good to document what it is shared with. So it seems to be
> > > the switch itself, pinctl and MDIO? Anything else?
> > > 
> > 
> > To be honest, I know that the GPIO controller (microchip,sparx5-sgpio) is
> > impacted but I don't know if anything else is impacted by this reset.
> > I can update the description with:
> >   description:
> >     Optional shared switch reset.
> >     This reset is shared with at least pinctrl, GPIO, MDIO and the switch
> >     itself.
> > 
> > Does it sound better ?
> 
> $dayjob hat off, bindings hat on: If you don't know, can we get someone
> from Microchip (there's some and a list in CC) to figure it out?

That is probably a good idea, there is potential for hard to find bugs
here, when a device gets an unexpected reset. Change the order things
probe, or an unexpected EPRODE_DEFER could be interesting.

       Andrew

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 06/17] dt-bindings: net: mscc-miim: Add resets property

[Alexandre Belloni]

On 02/05/2024 14:26:36+0200, Andrew Lunn wrote:
> On Thu, May 02, 2024 at 11:31:00AM +0100, Conor Dooley wrote:
> > On Thu, May 02, 2024 at 11:50:43AM +0200, Herve Codina wrote:
> > > Hi Andrew,
> > > 
> > > On Tue, 30 Apr 2024 18:31:46 +0200
> > > Andrew Lunn <andrew@lunn.ch> wrote:
> > > 
> > > > > We have the same construction with the pinctrl driver used in the LAN966x
> > > > >   Documentation/devicetree/bindings/pinctrl/mscc,ocelot-pinctrl.yaml
> > > > > 
> > > > > The reset name is 'switch' in the pinctrl binding.
> > > > > I can use the same description here as the one present in the pinctrl binding:
> > > > >   description: Optional shared switch reset.
> > > > > and keep 'switch' as reset name here (consistent with pinctrl reset name).
> > > > > 
> > > > > What do you think about that ?  
> > > > 
> > > > It would be good to document what it is shared with. So it seems to be
> > > > the switch itself, pinctl and MDIO? Anything else?
> > > > 
> > > 
> > > To be honest, I know that the GPIO controller (microchip,sparx5-sgpio) is
> > > impacted but I don't know if anything else is impacted by this reset.
> > > I can update the description with:
> > >   description:
> > >     Optional shared switch reset.
> > >     This reset is shared with at least pinctrl, GPIO, MDIO and the switch
> > >     itself.
> > > 
> > > Does it sound better ?
> > 
> > $dayjob hat off, bindings hat on: If you don't know, can we get someone
> > from Microchip (there's some and a list in CC) to figure it out?
> 
> That is probably a good idea, there is potential for hard to find bugs
> here, when a device gets an unexpected reset. Change the order things
> probe, or an unexpected EPRODE_DEFER could be interesting.
> 


The datasheet states:
"The VCore system comprises all the blocks attached to the VCore Shared
Bus (SBA), including the PCIe, DDR, frame DMA, SI slave, and MIIM slave
blocks. The device includes all the blocks attached to the Switch Core
Register Bus (CSR) including the VRAP slave. For more information about
the VCore System blocks, see Figure 5-1."

However, the reset driver protects the VCORE itself by setting bit 5.
Everything else is going to be reset.

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 12/17] irqchip: Add support for LAN966x OIC

[Herve Codina]

Hi Simon,

On Tue, 30 Apr 2024 21:24:51 +0100
Simon Horman <horms@kernel.org> wrote:

> On Tue, Apr 30, 2024 at 10:37:21AM +0200, Herve Codina wrote:
> > The Microchip LAN966x outband interrupt controller (OIC) maps the
> > internal interrupt sources of the LAN966x device to an external
> > interrupt.
> > When the LAN966x device is used as a PCI device, the external interrupt
> > is routed to the PCI interrupt.
> > 
> > Signed-off-by: Herve Codina <herve.codina@bootlin.com>  
> 
> Hi Herve,
> 
> > +static int lan966x_oic_probe(struct platform_device *pdev)
> > +{
> > +	struct device_node *node = pdev->dev.of_node;
> > +	struct lan966x_oic_data *lan966x_oic;
> > +	struct device *dev = &pdev->dev;
> > +	struct irq_chip_generic *gc;
> > +	int ret;
> > +	int i;
> > +
> > +	lan966x_oic = devm_kmalloc(dev, sizeof(*lan966x_oic), GFP_KERNEL);
> > +	if (!lan966x_oic)
> > +		return -ENOMEM;
> > +
> > +	lan966x_oic->regs = devm_platform_ioremap_resource(pdev, 0);
> > +	if (IS_ERR(lan966x_oic->regs))
> > +		return dev_err_probe(dev, PTR_ERR(lan966x_oic->regs),
> > +				     "failed to map resource\n");
> > +
> > +	lan966x_oic->domain = irq_domain_alloc_linear(of_node_to_fwnode(node),
> > +						      LAN966X_OIC_NR_IRQ,
> > +						      &irq_generic_chip_ops, NULL);  
> 
> nit: Please consider limiting lines to 80 columns wide in Networking code.

This will be done in the next iteration.

> 
> > +	if (!lan966x_oic->domain) {
> > +		dev_err(dev, "failed to create an IRQ domain\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	lan966x_oic->irq = platform_get_irq(pdev, 0);
> > +	if (lan966x_oic->irq < 0) {
> > +		dev_err_probe(dev, lan966x_oic->irq, "failed to get the IRQ\n");
> > +		goto err_domain_free;  
> 
> Hi,
> 
> This will result in the function returning ret.
> However, ret is uninitialised here.
> 
> Flagged by W=1 builds with clang-18, and Smatch.

Indeed, this fill be fixed in the next iteration.

Best regards,
Hervé

-- 
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 07/17] net: mdio: mscc-miim: Handle the switch reset

[Herve Codina]

Hi Sai,

On Tue, 30 Apr 2024 09:21:57 +0000
Sai Krishna Gajula <saikrishnag@marvell.com> wrote:

...
> > @@ -270,11 +271,18 @@ static int mscc_miim_probe(struct platform_device
> > *pdev)  {
> >  	struct device_node *np = pdev->dev.of_node;
> >  	struct regmap *mii_regmap, *phy_regmap;
> > +	struct reset_control *reset;  
> 
> Please follow reverse x-mass tree order
> 

Sure, this will be fixed in the next iteration.

Best regards,
Hervé

-- 
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 01/17] mfd: syscon: Add reference counting and device managed support

[Herve Codina]

Hi Simon,

On Tue, 30 Apr 2024 21:34:43 +0100
Simon Horman <horms@kernel.org> wrote:

...

> > +static intline void syscon_put_regmap(struct regmap *regmap)  
> 
> intline -> inline

Sure, this will be fixed in the next iteration.

Best regards,
Hervé

-- 
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 06/17] dt-bindings: net: mscc-miim: Add resets property

[Herve Codina]

Hi,

On Thu, 2 May 2024 15:22:09 +0200
Alexandre Belloni <alexandre.belloni@bootlin.com> wrote:

> On 02/05/2024 14:26:36+0200, Andrew Lunn wrote:
> > On Thu, May 02, 2024 at 11:31:00AM +0100, Conor Dooley wrote:  
> > > On Thu, May 02, 2024 at 11:50:43AM +0200, Herve Codina wrote:  
> > > > Hi Andrew,
> > > > 
> > > > On Tue, 30 Apr 2024 18:31:46 +0200
> > > > Andrew Lunn <andrew@lunn.ch> wrote:
> > > >   
> > > > > > We have the same construction with the pinctrl driver used in the LAN966x
> > > > > >   Documentation/devicetree/bindings/pinctrl/mscc,ocelot-pinctrl.yaml
> > > > > > 
> > > > > > The reset name is 'switch' in the pinctrl binding.
> > > > > > I can use the same description here as the one present in the pinctrl binding:
> > > > > >   description: Optional shared switch reset.
> > > > > > and keep 'switch' as reset name here (consistent with pinctrl reset name).
> > > > > > 
> > > > > > What do you think about that ?    
> > > > > 
> > > > > It would be good to document what it is shared with. So it seems to be
> > > > > the switch itself, pinctl and MDIO? Anything else?
> > > > >   
> > > > 
> > > > To be honest, I know that the GPIO controller (microchip,sparx5-sgpio) is
> > > > impacted but I don't know if anything else is impacted by this reset.
> > > > I can update the description with:
> > > >   description:
> > > >     Optional shared switch reset.
> > > >     This reset is shared with at least pinctrl, GPIO, MDIO and the switch
> > > >     itself.
> > > > 
> > > > Does it sound better ?  
> > > 
> > > $dayjob hat off, bindings hat on: If you don't know, can we get someone
> > > from Microchip (there's some and a list in CC) to figure it out?  
> > 
> > That is probably a good idea, there is potential for hard to find bugs
> > here, when a device gets an unexpected reset. Change the order things
> > probe, or an unexpected EPRODE_DEFER could be interesting.
> >   
> 
> 
> The datasheet states:
> "The VCore system comprises all the blocks attached to the VCore Shared
> Bus (SBA), including the PCIe, DDR, frame DMA, SI slave, and MIIM slave
> blocks. The device includes all the blocks attached to the Switch Core
> Register Bus (CSR) including the VRAP slave. For more information about
> the VCore System blocks, see Figure 5-1."
> 
> However, the reset driver protects the VCORE itself by setting bit 5.
> Everything else is going to be reset.
> 

Right,
I will update the reset description with:
  description:
    Optional shared switch reset.
    This reset is shared with all blocks attached to the Switch Core Register
    Bus (CSR) including VRAP slave.

Is that better ?

Best regards,
Hervé
-- 
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 15/17] pci: of_property: Add the interrupt-controller property in PCI device nodes

[Herve Codina]

Hi Bjorn,

On Wed, 1 May 2024 12:38:26 -0500
Bjorn Helgaas <helgaas@kernel.org> wrote:

> In subject: s/pci:/PCI:/ to match history. s/Add the/Add/ for brevity.

Will be done in the next iteration.

> 
> On Tue, Apr 30, 2024 at 10:37:24AM +0200, Herve Codina wrote:
> > PCI devices and bridges DT nodes created during the PCI scan are created
> > with the interrupt-map property set to handle interrupts.
> > 
> > In order to set this interrupt-map property at a specific level, a
> > phandle to the parent interrupt controller is needed.
> > On systems that are not fully described by a device-tree, the parent
> > interrupt controller may be unavailable (i.e. not described by the
> > device-tree).  
> 
> Rewrap into one paragraph or add blank line to separate paragraphs.

Will be rewrapped in one paragraph in the next iteration.

Thanks for your review.
Best regards,
Hervé

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 09/17] dt-bindings: interrupt-controller: Add support for Microchip LAN966x OIC

[Rob Herring]

On Tue, Apr 30, 2024 at 10:37:18AM +0200, Herve Codina wrote:
> The Microchip LAN966x outband interrupt controller (OIC) maps the
> internal interrupt sources of the LAN966x device to an external
> interrupt.
> When the LAN966x device is used as a PCI device, the external interrupt
> is routed to the PCI interrupt.
> 
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> ---
>  .../microchip,lan966x-oic.yaml                | 55 +++++++++++++++++++
>  1 file changed, 55 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/microchip,lan966x-oic.yaml
> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/microchip,lan966x-oic.yaml b/Documentation/devicetree/bindings/interrupt-controller/microchip,lan966x-oic.yaml
> new file mode 100644
> index 000000000000..b2adc7174177
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/microchip,lan966x-oic.yaml
> @@ -0,0 +1,55 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/interrupt-controller/microchip,lan966x-oic.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Microchip LAN966x outband interrupt controller
> +
> +maintainers:
> +  - Herve Codina <herve.codina@bootlin.com>
> +
> +allOf:
> +  - $ref: /schemas/interrupt-controller.yaml#
> +
> +description: |
> +  The Microchip LAN966x outband interrupt controller (OIC) maps the internal
> +  interrupt sources of the LAN966x device to an external interrupt.
> +  When the LAN966x device is used as a PCI device, the external interrupt is
> +  routed to the PCI interrupt.
> +
> +properties:
> +  compatible:
> +    const: microchip,lan966x-oic
> +
> +  '#interrupt-cells':
> +    const: 2
> +
> +  interrupt-controller: true
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - '#interrupt-cells'
> +  - interrupt-controller
> +  - interrupts
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    interrupt-controller@e00c0120 {
> +        compatible = "microchip,lan966x-oic";
> +        reg = <0xe00c0120 0x190>;

Looks like this is part of some larger block?

> +        #interrupt-cells = <2>;
> +        interrupt-controller;
> +        interrupts = <0>;
> +        interrupt-parent = <&intc>;
> +    };
> +...
> -- 
> 2.44.0
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 12/17] irqchip: Add support for LAN966x OIC

[Steen.Hegelund]

Hi Herve,

On Tue Apr 30, 2024 at 10:37 AM CEST, Herve Codina wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> The Microchip LAN966x outband interrupt controller (OIC) maps the
> internal interrupt sources of the LAN966x device to an external
> interrupt.
> When the LAN966x device is used as a PCI device, the external interrupt
> is routed to the PCI interrupt.
>
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> ---
>  drivers/irqchip/Kconfig           |  12 ++
>  drivers/irqchip/Makefile          |   1 +
>  drivers/irqchip/irq-lan966x-oic.c | 301 ++++++++++++++++++++++++++++++
>  3 files changed, 314 insertions(+)
>  create mode 100644 drivers/irqchip/irq-lan966x-oic.c
>
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index 72c07a12f5e1..973eebc8d1d1 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -169,6 +169,18 @@ config IXP4XX_IRQ
>         select IRQ_DOMAIN
>         select SPARSE_IRQ
>
> +config LAN966X_OIC
> +       tristate "Microchip LAN966x OIC Support"
> +       select GENERIC_IRQ_CHIP
> +       select IRQ_DOMAIN
> +       help
> +         Enable support for the LAN966x Outbound Interrupt Controller.
> +         This controller is present on the Microchip LAN966x PCI device and
> +         maps the internal interrupts sources to PCIe interrupt.
> +
> +         To compile this driver as a module, choose M here: the module
> +         will be called irq-lan966x-oic.
> +
>  config MADERA_IRQ
>         tristate
>
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index ec4a18380998..762d3078aa3b 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -101,6 +101,7 @@ obj-$(CONFIG_IMX_IRQSTEER)          += irq-imx-irqsteer.o
>  obj-$(CONFIG_IMX_INTMUX)               += irq-imx-intmux.o
>  obj-$(CONFIG_IMX_MU_MSI)               += irq-imx-mu-msi.o
>  obj-$(CONFIG_MADERA_IRQ)               += irq-madera.o
> +obj-$(CONFIG_LAN966X_OIC)              += irq-lan966x-oic.o
>  obj-$(CONFIG_LS1X_IRQ)                 += irq-ls1x.o
>  obj-$(CONFIG_TI_SCI_INTR_IRQCHIP)      += irq-ti-sci-intr.o
>  obj-$(CONFIG_TI_SCI_INTA_IRQCHIP)      += irq-ti-sci-inta.o
> diff --git a/drivers/irqchip/irq-lan966x-oic.c b/drivers/irqchip/irq-lan966x-oic.c
> new file mode 100644
> index 000000000000..342f0dbf16e3
> --- /dev/null
> +++ b/drivers/irqchip/irq-lan966x-oic.c
> @@ -0,0 +1,301 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Driver for the Microchip LAN966x outbound interrupt controller
> + *
> + * Copyright (c) 2024 Technology Inc. and its subsidiaries.
> + *
> + * Authors:
> + *     Horatiu Vultur <horatiu.vultur@microchip.com>
> + *     Clément Léger <clement.leger@bootlin.com>
> + *     Herve Codina <herve.codina@bootlin.com>
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/build_bug.h>
> +#include <linux/interrupt.h>
> +#include <linux/irqchip/chained_irq.h>
> +#include <linux/irqchip.h>
> +#include <linux/irq.h>
> +#include <linux/iopoll.h>
> +#include <linux/mfd/core.h>
> +#include <linux/module.h>
> +#include <linux/pci.h>
> +#include <linux/delay.h>
> +
> +struct lan966x_oic_chip_regs {
> +       int reg_off_ena_set;
> +       int reg_off_ena_clr;
> +       int reg_off_sticky;
> +       int reg_off_ident;
> +       int reg_off_map;
> +};
> +
> +struct lan966x_oic_data {
> +       struct irq_domain *domain;
> +       void __iomem *regs;
> +       int irq;
> +};
> +
> +#define LAN966X_OIC_NR_IRQ 86
> +
> +/* Interrupt sticky status */
> +#define LAN966X_OIC_INTR_STICKY                0x30
> +#define LAN966X_OIC_INTR_STICKY1       0x34
> +#define LAN966X_OIC_INTR_STICKY2       0x38
> +
> +/* Interrupt enable */
> +#define LAN966X_OIC_INTR_ENA           0x48
> +#define LAN966X_OIC_INTR_ENA1          0x4c
> +#define LAN966X_OIC_INTR_ENA2          0x50
> +
> +/* Atomic clear of interrupt enable */
> +#define LAN966X_OIC_INTR_ENA_CLR       0x54
> +#define LAN966X_OIC_INTR_ENA_CLR1      0x58
> +#define LAN966X_OIC_INTR_ENA_CLR2      0x5c
> +
> +/* Atomic set of interrupt */
> +#define LAN966X_OIC_INTR_ENA_SET       0x60
> +#define LAN966X_OIC_INTR_ENA_SET1      0x64
> +#define LAN966X_OIC_INTR_ENA_SET2      0x68
> +
> +/* Mapping of source to destination interrupts (_n = 0..8) */

Are the indices really needed on LAN966X_OIC_DST_INTR_MAP* and _IDENT*
You do not appear to be using them?


> +#define LAN966X_OIC_DST_INTR_MAP(_n)   0x78
> +#define LAN966X_OIC_DST_INTR_MAP1(_n)  0x9c
> +#define LAN966X_OIC_DST_INTR_MAP2(_n)  0xc0
> +
> +/* Currently active interrupt sources per destination (_n = 0..8) */
> +#define LAN966X_OIC_DST_INTR_IDENT(_n) 0xe4
> +#define LAN966X_OIC_DST_INTR_IDENT1(_n)        0x108
> +#define LAN966X_OIC_DST_INTR_IDENT2(_n)        0x12c
> +
> +static unsigned int lan966x_oic_irq_startup(struct irq_data *data)
> +{
> +       struct irq_chip_generic *gc = irq_data_get_irq_chip_data(data);
> +       struct irq_chip_type *ct = irq_data_get_chip_type(data);
> +       struct lan966x_oic_chip_regs *chip_regs = gc->private;
> +       u32 map;
> +
> +       irq_gc_lock(gc);
> +
> +       /* Map the source interrupt to the destination */
> +       map = irq_reg_readl(gc, chip_regs->reg_off_map);
> +       map |= data->mask;
> +       irq_reg_writel(gc, map, chip_regs->reg_off_map);
> +
> +       irq_gc_unlock(gc);
> +
> +       ct->chip.irq_ack(data);
> +       ct->chip.irq_unmask(data);
> +
> +       return 0;
> +}
> +
> +static void lan966x_oic_irq_shutdown(struct irq_data *data)
> +{
> +       struct irq_chip_generic *gc = irq_data_get_irq_chip_data(data);
> +       struct irq_chip_type *ct = irq_data_get_chip_type(data);
> +       struct lan966x_oic_chip_regs *chip_regs = gc->private;
> +       u32 map;
> +
> +       ct->chip.irq_mask(data);
> +
> +       irq_gc_lock(gc);
> +
> +       /* Unmap the interrupt */
> +       map = irq_reg_readl(gc, chip_regs->reg_off_map);
> +       map &= ~data->mask;
> +       irq_reg_writel(gc, map, chip_regs->reg_off_map);
> +
> +       irq_gc_unlock(gc);
> +}
> +
> +static int lan966x_oic_irq_set_type(struct irq_data *data, unsigned int flow_type)
> +{
> +       if (flow_type != IRQ_TYPE_LEVEL_HIGH) {
> +               pr_err("lan966x oic doesn't support flow type %d\n", flow_type);
> +               return -EINVAL;
> +       }
> +
> +       return 0;
> +}
> +
> +static void lan966x_oic_irq_handler_domain(struct irq_domain *d, u32 first_irq)
> +{
> +       struct irq_chip_generic *gc = irq_get_domain_generic_chip(d, first_irq);
> +       struct lan966x_oic_chip_regs *chip_regs = gc->private;
> +       unsigned long ident;
> +       unsigned int hwirq;
> +
> +       ident = irq_reg_readl(gc, chip_regs->reg_off_ident);
> +       if (!ident)
> +               return;
> +
> +       for_each_set_bit(hwirq, &ident, 32)
> +               generic_handle_domain_irq(d, hwirq + first_irq);
> +}
> +
> +static void lan966x_oic_irq_handler(struct irq_desc *desc)
> +{
> +       struct irq_domain *d = irq_desc_get_handler_data(desc);
> +       struct irq_chip *chip = irq_desc_get_chip(desc);
> +
> +       chained_irq_enter(chip, desc);
> +       lan966x_oic_irq_handler_domain(d, 0);
> +       lan966x_oic_irq_handler_domain(d, 32);
> +       lan966x_oic_irq_handler_domain(d, 64);
> +       chained_irq_exit(chip, desc);
> +}
> +
> +static struct lan966x_oic_chip_regs lan966x_oic_chip_regs[3] = {
> +       {
> +               .reg_off_ena_set = LAN966X_OIC_INTR_ENA_SET,
> +               .reg_off_ena_clr = LAN966X_OIC_INTR_ENA_CLR,
> +               .reg_off_sticky = LAN966X_OIC_INTR_STICKY,
> +               .reg_off_ident = LAN966X_OIC_DST_INTR_IDENT(0),
> +               .reg_off_map = LAN966X_OIC_DST_INTR_MAP(0),
> +       }, {
> +               .reg_off_ena_set = LAN966X_OIC_INTR_ENA_SET1,
> +               .reg_off_ena_clr = LAN966X_OIC_INTR_ENA_CLR1,
> +               .reg_off_sticky = LAN966X_OIC_INTR_STICKY1,
> +               .reg_off_ident = LAN966X_OIC_DST_INTR_IDENT1(0),
> +               .reg_off_map = LAN966X_OIC_DST_INTR_MAP1(0),
> +       }, {
> +               .reg_off_ena_set = LAN966X_OIC_INTR_ENA_SET2,
> +               .reg_off_ena_clr = LAN966X_OIC_INTR_ENA_CLR2,
> +               .reg_off_sticky = LAN966X_OIC_INTR_STICKY2,
> +               .reg_off_ident = LAN966X_OIC_DST_INTR_IDENT2(0),
> +               .reg_off_map = LAN966X_OIC_DST_INTR_MAP2(0),
> +       }
> +};
> +
> +static void lan966x_oic_chip_init(struct lan966x_oic_data *lan966x_oic,
> +                                 struct irq_chip_generic *gc,
> +                                 struct lan966x_oic_chip_regs *chip_regs)
> +{
> +       gc->reg_base = lan966x_oic->regs;
> +       gc->chip_types[0].regs.enable = chip_regs->reg_off_ena_set;
> +       gc->chip_types[0].regs.disable = chip_regs->reg_off_ena_clr;
> +       gc->chip_types[0].regs.ack = chip_regs->reg_off_sticky;
> +       gc->chip_types[0].chip.irq_startup = lan966x_oic_irq_startup;
> +       gc->chip_types[0].chip.irq_shutdown = lan966x_oic_irq_shutdown;
> +       gc->chip_types[0].chip.irq_set_type = lan966x_oic_irq_set_type;
> +       gc->chip_types[0].chip.irq_mask = irq_gc_mask_disable_reg;
> +       gc->chip_types[0].chip.irq_unmask = irq_gc_unmask_enable_reg;
> +       gc->chip_types[0].chip.irq_ack = irq_gc_ack_set_bit;
> +       gc->private = chip_regs;
> +
> +       /* Disable all interrupts handled by this chip */
> +       irq_reg_writel(gc, ~0, chip_regs->reg_off_ena_clr);
> +}
> +
> +static void lan966x_oic_chip_exit(struct irq_chip_generic *gc)
> +{
> +       /* Disable and ack all interrupts handled by this chip */
> +       irq_reg_writel(gc, ~0, gc->chip_types[0].regs.disable);
> +       irq_reg_writel(gc, ~0, gc->chip_types[0].regs.ack);
> +}
> +
> +static int lan966x_oic_probe(struct platform_device *pdev)
> +{
> +       struct device_node *node = pdev->dev.of_node;
> +       struct lan966x_oic_data *lan966x_oic;
> +       struct device *dev = &pdev->dev;
> +       struct irq_chip_generic *gc;
> +       int ret;
> +       int i;
> +
> +       lan966x_oic = devm_kmalloc(dev, sizeof(*lan966x_oic), GFP_KERNEL);
> +       if (!lan966x_oic)
> +               return -ENOMEM;
> +
> +       lan966x_oic->regs = devm_platform_ioremap_resource(pdev, 0);
> +       if (IS_ERR(lan966x_oic->regs))
> +               return dev_err_probe(dev, PTR_ERR(lan966x_oic->regs),
> +                                    "failed to map resource\n");
> +
> +       lan966x_oic->domain = irq_domain_alloc_linear(of_node_to_fwnode(node),
> +                                                     LAN966X_OIC_NR_IRQ,
> +                                                     &irq_generic_chip_ops, NULL);
> +       if (!lan966x_oic->domain) {
> +               dev_err(dev, "failed to create an IRQ domain\n");
> +               return -EINVAL;
> +       }
> +
> +       lan966x_oic->irq = platform_get_irq(pdev, 0);
> +       if (lan966x_oic->irq < 0) {
> +               dev_err_probe(dev, lan966x_oic->irq, "failed to get the IRQ\n");
> +               goto err_domain_free;
> +       }
> +
> +       ret = irq_alloc_domain_generic_chips(lan966x_oic->domain, 32, 1, "lan966x-oic",
> +                                            handle_level_irq, 0, 0, 0);
> +       if (ret) {
> +               dev_err_probe(dev, ret, "failed to alloc irq domain gc\n");
> +               goto err_domain_free;
> +       }
> +
> +       /* Init chips */
> +       BUILD_BUG_ON(DIV_ROUND_UP(LAN966X_OIC_NR_IRQ, 32) != ARRAY_SIZE(lan966x_oic_chip_regs));
> +       for (i = 0; i < ARRAY_SIZE(lan966x_oic_chip_regs); i++) {
> +               gc = irq_get_domain_generic_chip(lan966x_oic->domain, i * 32);
> +               lan966x_oic_chip_init(lan966x_oic, gc, &lan966x_oic_chip_regs[i]);
> +       }
> +
> +       irq_set_chained_handler_and_data(lan966x_oic->irq, lan966x_oic_irq_handler,
> +                                        lan966x_oic->domain);
> +
> +       irq_domain_publish(lan966x_oic->domain);
> +       platform_set_drvdata(pdev, lan966x_oic);
> +       return 0;
> +
> +err_domain_free:
> +       irq_domain_free(lan966x_oic->domain);
> +       return ret;
> +}
> +
> +static void lan966x_oic_remove(struct platform_device *pdev)
> +{
> +       struct lan966x_oic_data *lan966x_oic = platform_get_drvdata(pdev);
> +       struct irq_chip_generic *gc;
> +       int i;
> +
> +       for (i = 0; i < ARRAY_SIZE(lan966x_oic_chip_regs); i++) {
> +               gc = irq_get_domain_generic_chip(lan966x_oic->domain, i * 32);
> +               lan966x_oic_chip_exit(gc);
> +       }
> +
> +       irq_set_chained_handler_and_data(lan966x_oic->irq, NULL, NULL);
> +
> +       for (i = 0; i < LAN966X_OIC_NR_IRQ; i++)
> +               irq_dispose_mapping(irq_find_mapping(lan966x_oic->domain, i));
> +
> +       irq_domain_unpublish(lan966x_oic->domain);
> +
> +       for (i = 0; i < ARRAY_SIZE(lan966x_oic_chip_regs); i++) {
> +               gc = irq_get_domain_generic_chip(lan966x_oic->domain, i * 32);
> +               irq_remove_generic_chip(gc, ~0, 0, 0);
> +       }
> +
> +       kfree(lan966x_oic->domain->gc);
> +       irq_domain_free(lan966x_oic->domain);
> +}
> +
> +static const struct of_device_id lan966x_oic_of_match[] = {
> +       { .compatible = "microchip,lan966x-oic" },
> +       {} /* sentinel */
> +};
> +MODULE_DEVICE_TABLE(of, lan966x_oic_of_match);
> +
> +static struct platform_driver lan966x_oic_driver = {
> +       .probe = lan966x_oic_probe,
> +       .remove_new = lan966x_oic_remove,
> +       .driver = {
> +               .name = "lan966x-oic",
> +               .of_match_table = lan966x_oic_of_match,
> +       },
> +};
> +module_platform_driver(lan966x_oic_driver);
> +
> +MODULE_AUTHOR("Herve Codina <herve.codina@bootlin.com>");
> +MODULE_DESCRIPTION("Microchip LAN966x OIC driver");
> +MODULE_LICENSE("GPL");
> --
> 2.44.0

Best regards
Steen
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 16/17] mfd: Add support for LAN966x PCI device

[Steen.Hegelund]

Hi Herve,

On Tue Apr 30, 2024 at 10:37 AM CEST, Herve Codina wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> 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 4b023ee229cf..e5f5d2986dd3 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..d9d886a1948f
> --- /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/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/pci.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 ret ? IRQ_NONE : IRQ_HANDLED;
> +}
> +
> +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_LEGACY);
> +       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);
> +}
> +

It looks like the two functions below (and their helper functions) are so
generic that they could be part of the pci driver core support.
Any plans for that?

> +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);
> +}
> +

...
[snip]
...

> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index eff7f5df08e2..9933f245b781 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(0x1055, 0x9660, of_pci_make_dev_node);
>
>  /*
>   * Devices known to require a longer delay before first config space access
> --
> 2.44.0

Best Regards
Steen
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 14/17] of: dynamic: Introduce of_changeset_add_prop_bool()

[Rob Herring]

On Tue, Apr 30, 2024 at 3:39 AM Herve Codina <herve.codina@bootlin.com> wrote:
>
> APIs to add some properties in a changeset exist but nothing to add a DT
> boolean property (i.e. a property without any values).
>
> Fill this lack with of_changeset_add_prop_bool().

Please add a test case.

>
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> ---
>  drivers/of/dynamic.c | 25 +++++++++++++++++++++++++
>  include/linux/of.h   |  3 +++
>  2 files changed, 28 insertions(+)

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 09/17] dt-bindings: interrupt-controller: Add support for Microchip LAN966x OIC

[Herve Codina]

Hi Rob,

On Tue, 7 May 2024 10:28:06 -0500
Rob Herring <robh@kernel.org> wrote:

...
> > +examples:
> > +  - |
> > +    interrupt-controller@e00c0120 {
> > +        compatible = "microchip,lan966x-oic";
> > +        reg = <0xe00c0120 0x190>;  
> 
> Looks like this is part of some larger block?
> 

According to the registers information document:
  https://microchip-ung.github.io/lan9662_reginfo/reginfo_LAN9662.html?select=cpu,intr

The interrupt controller is mapped at offset 0x48 (offset in number of
32bit words).
-> Address offset: 0x48 * 4 = 0x120
-> size: (0x63 + 1) *  4 = 0x190

IMHO, the reg property value looks correct.

Best regards,
Hervé

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 12/17] irqchip: Add support for LAN966x OIC

[Herve Codina]

Hi Steen,

On Wed, 8 May 2024 08:08:30 +0000
<Steen.Hegelund@microchip.com> wrote:

...
> > +/* Mapping of source to destination interrupts (_n = 0..8) */  
> 
> Are the indices really needed on LAN966X_OIC_DST_INTR_MAP* and _IDENT*
> You do not appear to be using them?
> 
> 
> > +#define LAN966X_OIC_DST_INTR_MAP(_n)   0x78

Indeed, I missed them.
These registers are defined from 0 to 8 in the document:
  https://microchip-ung.github.io/lan9662_reginfo/reginfo_LAN9662.html?select=cpu,intr

The code use only the indice 0.
In the next iteration, I will keep indices and update the definition of
registers like that:
  #define LAN966X_OIC_DST_INTR_MAP(_n)   (0x78 + (_n) * 4)

Best regards
Hervé

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 09/17] dt-bindings: interrupt-controller: Add support for Microchip LAN966x OIC

[Rob Herring]

On Mon, May 13, 2024 at 02:37:20PM +0200, Herve Codina wrote:
> Hi Rob,
> 
> On Tue, 7 May 2024 10:28:06 -0500
> Rob Herring <robh@kernel.org> wrote:
> 
> ...
> > > +examples:
> > > +  - |
> > > +    interrupt-controller@e00c0120 {
> > > +        compatible = "microchip,lan966x-oic";
> > > +        reg = <0xe00c0120 0x190>;  
> > 
> > Looks like this is part of some larger block?
> > 
> 
> According to the registers information document:
>   https://microchip-ung.github.io/lan9662_reginfo/reginfo_LAN9662.html?select=cpu,intr
> 
> The interrupt controller is mapped at offset 0x48 (offset in number of
> 32bit words).
> -> Address offset: 0x48 * 4 = 0x120
> -> size: (0x63 + 1) *  4 = 0x190
> 
> IMHO, the reg property value looks correct.

What I mean is h/w blocks don't just start at some address with small 
alignment. That wouldn't work from a physical design standpoint. The 
larger block here is "CPU System Regs". The block as a whole should be 
documented, but maybe that ship already sailed.

Also, here you call it the OIC, but the link above calls it the VCore 
interrupt controller.

Rob

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 09/17] dt-bindings: interrupt-controller: Add support for Microchip LAN966x OIC

[Herve Codina]

Hi Rob,

On Mon, 13 May 2024 09:53:58 -0500
Rob Herring <robh@kernel.org> wrote:

> On Mon, May 13, 2024 at 02:37:20PM +0200, Herve Codina wrote:
> > Hi Rob,
> > 
> > On Tue, 7 May 2024 10:28:06 -0500
> > Rob Herring <robh@kernel.org> wrote:
> > 
> > ...  
> > > > +examples:
> > > > +  - |
> > > > +    interrupt-controller@e00c0120 {
> > > > +        compatible = "microchip,lan966x-oic";
> > > > +        reg = <0xe00c0120 0x190>;    
> > > 
> > > Looks like this is part of some larger block?
> > >   
> > 
> > According to the registers information document:
> >   https://microchip-ung.github.io/lan9662_reginfo/reginfo_LAN9662.html?select=cpu,intr
> > 
> > The interrupt controller is mapped at offset 0x48 (offset in number of
> > 32bit words).  
> > -> Address offset: 0x48 * 4 = 0x120
> > -> size: (0x63 + 1) *  4 = 0x190  
> > 
> > IMHO, the reg property value looks correct.  
> 
> What I mean is h/w blocks don't just start at some address with small 
> alignment. That wouldn't work from a physical design standpoint. The 
> larger block here is "CPU System Regs". The block as a whole should be 
> documented, but maybe that ship already sailed.

The clock controller, also part of the "CPU System Regs" is already defined
and used without the larger block
  Documentation/devicetree/bindings/clock/microchip,lan966x-gck.yaml

IMHO, the binding related to the interrupt controller should be consistent
with the one related to the clock controller.


> 
> Also, here you call it the OIC, but the link above calls it the VCore 
> interrupt controller.

Yes, I call it OIC (Outband Interrupt Controller) as it is its name in the
datasheet explaining how it works.
The datasheet I have is not publicly available and so, I can point only to
the register map (url provided).

I think it would be better to keep "Outband Interrupt Controller" as
mentioned in the datasheet.

Best regards,
Hervé

-- 
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 16/17] mfd: Add support for LAN966x PCI device

[Herve Codina]

Hi Steen,

On Wed, 8 May 2024 08:20:04 +0000
<Steen.Hegelund@microchip.com> wrote:

...
> > +
> > +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 ret ? IRQ_NONE : IRQ_HANDLED;
> > +}
> > +
> > +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_LEGACY);
> > +       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);
> > +}
> > +  
> 
> It looks like the two functions below (and their helper functions) are so
> generic that they could be part of the pci driver core support.
> Any plans for that?

Indeed, I tried to write them in a generic way.
Right now, at least for the next iteration of this series, I don't plan to
move them as part of the PCI code.
This piece of code did not get any feedback and I would prefer to keep them
here for the moment.

Of course, they could be move out of the LAN966x PCI driver later.

> 
> > +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);
> > +}
> > +  
> 

Best regards,
Hervé

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel