| Message ID | 20240627091137.370572-7-herve.codina@bootlin.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add support for the LAN966x PCI device using a DT overlay | expand |
On Thu, 27 Jun 2024, Herve Codina wrote: > Add a PCI driver that handles the LAN966x PCI device using a device-tree > overlay. This overlay is applied to the PCI device DT node and allows to > describe components that are present in the device. > > The memory from the device-tree is remapped to the BAR memory thanks to > "ranges" properties computed at runtime by the PCI core during the PCI > enumeration. > > The PCI device itself acts as an interrupt controller and is used as the > parent of the internal LAN966x interrupt controller to route the > interrupts to the assigned PCI INTx interrupt. Not entirely sure why this is in MFD. Also I'm unsure of his current views, but Greg has voiced pretty big feelings about representing PCI drivers as Platform ones in the past. / Lee > Signed-off-by: Herve Codina <herve.codina@bootlin.com> > --- > drivers/mfd/Kconfig | 24 ++++ > drivers/mfd/Makefile | 4 + > drivers/mfd/lan966x_pci.c | 229 +++++++++++++++++++++++++++++++++++ > drivers/mfd/lan966x_pci.dtso | 167 +++++++++++++++++++++++++ > drivers/pci/quirks.c | 1 + > 5 files changed, 425 insertions(+) > create mode 100644 drivers/mfd/lan966x_pci.c > create mode 100644 drivers/mfd/lan966x_pci.dtso > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig > index 266b4f54af60..15db144bc09b 100644 > --- a/drivers/mfd/Kconfig > +++ b/drivers/mfd/Kconfig > @@ -144,6 +144,30 @@ config MFD_ATMEL_FLEXCOM > by the probe function of this MFD driver according to a device tree > property. > > +config MFD_LAN966X_PCI > + tristate "Microchip LAN966x PCIe Support" > + depends on PCI > + select OF > + select OF_OVERLAY > + select IRQ_DOMAIN > + help > + This enables the support for the LAN966x PCIe device. > + This is used to drive the LAN966x PCIe device from the host system > + to which it is connected. > + > + This driver uses an overlay to load other drivers to support for > + LAN966x internal components. > + Even if this driver does not depend on these other drivers, in order > + to have a fully functional board, the following drivers are needed: > + - fixed-clock (COMMON_CLK) > + - lan966x-oic (LAN966X_OIC) > + - lan966x-cpu-syscon (MFD_SYSCON) > + - lan966x-switch-reset (RESET_MCHP_SPARX5) > + - lan966x-pinctrl (PINCTRL_OCELOT) > + - lan966x-serdes (PHY_LAN966X_SERDES) > + - lan966x-miim (MDIO_MSCC_MIIM) > + - lan966x-switch (LAN966X_SWITCH) > + > config MFD_ATMEL_HLCDC > tristate "Atmel HLCDC (High-end LCD Controller)" > select MFD_CORE > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile > index c66f07edcd0e..165a9674ff48 100644 > --- a/drivers/mfd/Makefile > +++ b/drivers/mfd/Makefile > @@ -284,3 +284,7 @@ rsmu-i2c-objs := rsmu_core.o rsmu_i2c.o > rsmu-spi-objs := rsmu_core.o rsmu_spi.o > obj-$(CONFIG_MFD_RSMU_I2C) += rsmu-i2c.o > obj-$(CONFIG_MFD_RSMU_SPI) += rsmu-spi.o > + > +lan966x-pci-objs := lan966x_pci.o > +lan966x-pci-objs += lan966x_pci.dtbo.o > +obj-$(CONFIG_MFD_LAN966X_PCI) += lan966x-pci.o > diff --git a/drivers/mfd/lan966x_pci.c b/drivers/mfd/lan966x_pci.c > new file mode 100644 > index 000000000000..c69350449b15 > --- /dev/null > +++ b/drivers/mfd/lan966x_pci.c > @@ -0,0 +1,229 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Microchip LAN966x PCI driver > + * > + * Copyright (c) 2024 Microchip Technology Inc. and its subsidiaries. > + * > + * Authors: > + * Clément Léger <clement.leger@bootlin.com> > + * Hervé Codina <herve.codina@bootlin.com> > + */ > + > +#include <linux/irq.h> > +#include <linux/irqdomain.h> > +#include <linux/module.h> > +#include <linux/of_platform.h> > +#include <linux/pci.h> > +#include <linux/pci_ids.h> > +#include <linux/slab.h> > + > +/* Embedded dtbo symbols created by cmd_wrap_S_dtb in scripts/Makefile.lib */ > +extern char __dtbo_lan966x_pci_begin[]; > +extern char __dtbo_lan966x_pci_end[]; > + > +struct pci_dev_intr_ctrl { > + struct pci_dev *pci_dev; > + struct irq_domain *irq_domain; > + int irq; > +}; > + > +static int pci_dev_irq_domain_map(struct irq_domain *d, unsigned int virq, irq_hw_number_t hw) > +{ > + irq_set_chip_and_handler(virq, &dummy_irq_chip, handle_simple_irq); > + return 0; > +} > + > +static const struct irq_domain_ops pci_dev_irq_domain_ops = { > + .map = pci_dev_irq_domain_map, > + .xlate = irq_domain_xlate_onecell, > +}; > + > +static irqreturn_t pci_dev_irq_handler(int irq, void *data) > +{ > + struct pci_dev_intr_ctrl *intr_ctrl = data; > + int ret; > + > + ret = generic_handle_domain_irq(intr_ctrl->irq_domain, 0); > + return IRQ_RETVAL(!ret); > +} > + > +static struct pci_dev_intr_ctrl *pci_dev_create_intr_ctrl(struct pci_dev *pdev) > +{ > + struct pci_dev_intr_ctrl *intr_ctrl; > + struct fwnode_handle *fwnode; > + int ret; > + > + if (!pdev->irq) > + return ERR_PTR(-EOPNOTSUPP); > + > + fwnode = dev_fwnode(&pdev->dev); > + if (!fwnode) > + return ERR_PTR(-ENODEV); > + > + intr_ctrl = kmalloc(sizeof(*intr_ctrl), GFP_KERNEL); > + if (!intr_ctrl) > + return ERR_PTR(-ENOMEM); > + > + intr_ctrl->pci_dev = pdev; > + > + intr_ctrl->irq_domain = irq_domain_create_linear(fwnode, 1, &pci_dev_irq_domain_ops, > + intr_ctrl); > + if (!intr_ctrl->irq_domain) { > + pci_err(pdev, "Failed to create irqdomain\n"); > + ret = -ENOMEM; > + goto err_free_intr_ctrl; > + } > + > + ret = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_INTX); > + if (ret < 0) { > + pci_err(pdev, "Unable alloc irq vector (%d)\n", ret); > + goto err_remove_domain; > + } > + intr_ctrl->irq = pci_irq_vector(pdev, 0); > + ret = request_irq(intr_ctrl->irq, pci_dev_irq_handler, IRQF_SHARED, > + dev_name(&pdev->dev), intr_ctrl); > + if (ret) { > + pci_err(pdev, "Unable to request irq %d (%d)\n", intr_ctrl->irq, ret); > + goto err_free_irq_vector; > + } > + > + return intr_ctrl; > + > +err_free_irq_vector: > + pci_free_irq_vectors(pdev); > +err_remove_domain: > + irq_domain_remove(intr_ctrl->irq_domain); > +err_free_intr_ctrl: > + kfree(intr_ctrl); > + return ERR_PTR(ret); > +} > + > +static void pci_dev_remove_intr_ctrl(struct pci_dev_intr_ctrl *intr_ctrl) > +{ > + free_irq(intr_ctrl->irq, intr_ctrl); > + pci_free_irq_vectors(intr_ctrl->pci_dev); > + irq_dispose_mapping(irq_find_mapping(intr_ctrl->irq_domain, 0)); > + irq_domain_remove(intr_ctrl->irq_domain); > + kfree(intr_ctrl); > +} > + > +static void devm_pci_dev_remove_intr_ctrl(void *data) > +{ > + struct pci_dev_intr_ctrl *intr_ctrl = data; > + > + pci_dev_remove_intr_ctrl(intr_ctrl); > +} > + > +static int devm_pci_dev_create_intr_ctrl(struct pci_dev *pdev) > +{ > + struct pci_dev_intr_ctrl *intr_ctrl; > + > + intr_ctrl = pci_dev_create_intr_ctrl(pdev); > + if (IS_ERR(intr_ctrl)) > + return PTR_ERR(intr_ctrl); > + > + return devm_add_action_or_reset(&pdev->dev, devm_pci_dev_remove_intr_ctrl, intr_ctrl); > +} > + > +struct lan966x_pci { > + struct device *dev; > + int ovcs_id; > +}; > + > +static int lan966x_pci_load_overlay(struct lan966x_pci *data) > +{ > + u32 dtbo_size = __dtbo_lan966x_pci_end - __dtbo_lan966x_pci_begin; > + void *dtbo_start = __dtbo_lan966x_pci_begin; > + int ret; > + > + ret = of_overlay_fdt_apply(dtbo_start, dtbo_size, &data->ovcs_id, dev_of_node(data->dev)); > + if (ret) > + return ret; > + > + return 0; > +} > + > +static void lan966x_pci_unload_overlay(struct lan966x_pci *data) > +{ > + of_overlay_remove(&data->ovcs_id); > +} > + > +static int lan966x_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) > +{ > + struct device *dev = &pdev->dev; > + struct lan966x_pci *data; > + int ret; > + > + /* > + * On ACPI system, fwnode can point to the ACPI node. > + * This driver needs an of_node to be used as the device-tree overlay > + * target. This of_node should be set by the PCI core if it succeeds in > + * creating it (CONFIG_PCI_DYNAMIC_OF_NODES feature). > + * Check here for the validity of this of_node. > + */ > + if (!dev_of_node(dev)) { > + dev_err(dev, "Missing of_node for device\n"); > + return -EINVAL; > + } > + > + /* Need to be done before devm_pci_dev_create_intr_ctrl. > + * It allocates an IRQ and so pdev->irq is updated. > + */ > + ret = pcim_enable_device(pdev); > + if (ret) > + return ret; > + > + ret = devm_pci_dev_create_intr_ctrl(pdev); > + if (ret) > + return ret; > + > + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); > + if (!data) > + return -ENOMEM; > + > + pci_set_drvdata(pdev, data); > + data->dev = dev; > + > + ret = lan966x_pci_load_overlay(data); > + if (ret) > + return ret; > + > + pci_set_master(pdev); > + > + ret = of_platform_default_populate(dev_of_node(dev), NULL, dev); > + if (ret) > + goto err_unload_overlay; > + > + return 0; > + > +err_unload_overlay: > + lan966x_pci_unload_overlay(data); > + return ret; > +} > + > +static void lan966x_pci_remove(struct pci_dev *pdev) > +{ > + struct lan966x_pci *data = pci_get_drvdata(pdev); > + > + of_platform_depopulate(data->dev); > + > + lan966x_pci_unload_overlay(data); > +} > + > +static struct pci_device_id lan966x_pci_ids[] = { > + { PCI_DEVICE(PCI_VENDOR_ID_EFAR, 0x9660) }, > + { } > +}; > +MODULE_DEVICE_TABLE(pci, lan966x_pci_ids); > + > +static struct pci_driver lan966x_pci_driver = { > + .name = "mchp_lan966x_pci", > + .id_table = lan966x_pci_ids, > + .probe = lan966x_pci_probe, > + .remove = lan966x_pci_remove, > +}; > +module_pci_driver(lan966x_pci_driver); > + > +MODULE_AUTHOR("Herve Codina <herve.codina@bootlin.com>"); > +MODULE_DESCRIPTION("Microchip LAN966x PCI driver"); > +MODULE_LICENSE("GPL"); > diff --git a/drivers/mfd/lan966x_pci.dtso b/drivers/mfd/lan966x_pci.dtso > new file mode 100644 > index 000000000000..041f4319e4cd > --- /dev/null > +++ b/drivers/mfd/lan966x_pci.dtso > @@ -0,0 +1,167 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2022 Microchip UNG > + */ > + > +#include <dt-bindings/clock/microchip,lan966x.h> > +#include <dt-bindings/interrupt-controller/irq.h> > +#include <dt-bindings/mfd/atmel-flexcom.h> > +#include <dt-bindings/phy/phy-lan966x-serdes.h> > +#include <dt-bindings/gpio/gpio.h> > + > +/dts-v1/; > +/plugin/; > + > +/ { > + fragment@0 { > + target-path=""; > + __overlay__ { > + #address-cells = <3>; > + #size-cells = <2>; > + > + pci-ep-bus@0 { > + compatible = "simple-bus"; > + #address-cells = <1>; > + #size-cells = <1>; > + > + /* > + * map @0xe2000000 (32MB) to BAR0 (CPU) > + * map @0xe0000000 (16MB) to BAR1 (AMBA) > + */ > + ranges = <0xe2000000 0x00 0x00 0x00 0x2000000 > + 0xe0000000 0x01 0x00 0x00 0x1000000>; > + > + oic: oic@e00c0120 { > + compatible = "microchip,lan966x-oic"; > + #interrupt-cells = <2>; > + interrupt-controller; > + interrupts = <0>; /* PCI INTx assigned interrupt */ > + reg = <0xe00c0120 0x190>; > + }; > + > + cpu_clk: cpu_clk { > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <600000000>; // CPU clock = 600MHz > + }; > + > + ddr_clk: ddr_clk { > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <30000000>; // Fabric clock = 30MHz > + }; > + > + sys_clk: sys_clk { > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <15625000>; // System clock = 15.625MHz > + }; > + > + cpu_ctrl: syscon@e00c0000 { > + compatible = "microchip,lan966x-cpu-syscon", "syscon"; > + reg = <0xe00c0000 0xa8>; > + }; > + > + reset: reset@e200400c { > + compatible = "microchip,lan966x-switch-reset"; > + reg = <0xe200400c 0x4>; > + reg-names = "gcb"; > + #reset-cells = <1>; > + cpu-syscon = <&cpu_ctrl>; > + }; > + > + gpio: pinctrl@e2004064 { > + compatible = "microchip,lan966x-pinctrl"; > + reg = <0xe2004064 0xb4>, > + <0xe2010024 0x138>; > + resets = <&reset 0>; > + reset-names = "switch"; > + gpio-controller; > + #gpio-cells = <2>; > + gpio-ranges = <&gpio 0 0 78>; > + interrupt-parent = <&oic>; > + interrupt-controller; > + interrupts = <17 IRQ_TYPE_LEVEL_HIGH>; > + #interrupt-cells = <2>; > + > + tod_pins: tod_pins { > + pins = "GPIO_36"; > + function = "ptpsync_1"; > + }; > + > + fc0_a_pins: fcb4-i2c-pins { > + /* RXD, TXD */ > + pins = "GPIO_9", "GPIO_10"; > + function = "fc0_a"; > + }; > + > + }; > + > + serdes: serdes@e202c000 { > + compatible = "microchip,lan966x-serdes"; > + reg = <0xe202c000 0x9c>, > + <0xe2004010 0x4>; > + #phy-cells = <2>; > + }; > + > + mdio1: mdio@e200413c { > + #address-cells = <1>; > + #size-cells = <0>; > + compatible = "microchip,lan966x-miim"; > + reg = <0xe200413c 0x24>, > + <0xe2010020 0x4>; > + > + resets = <&reset 0>; > + reset-names = "switch"; > + > + lan966x_phy0: ethernet-lan966x_phy@1 { > + reg = <1>; > + }; > + > + lan966x_phy1: ethernet-lan966x_phy@2 { > + reg = <2>; > + }; > + }; > + > + switch: switch@e0000000 { > + compatible = "microchip,lan966x-switch"; > + reg = <0xe0000000 0x0100000>, > + <0xe2000000 0x0800000>; > + reg-names = "cpu", "gcb"; > + > + interrupt-parent = <&oic>; > + interrupts = <12 IRQ_TYPE_LEVEL_HIGH>, > + <9 IRQ_TYPE_LEVEL_HIGH>; > + interrupt-names = "xtr", "ana"; > + > + resets = <&reset 0>; > + reset-names = "switch"; > + > + pinctrl-names = "default"; > + pinctrl-0 = <&tod_pins>; > + > + ethernet-ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port0: port@0 { > + phy-handle = <&lan966x_phy0>; > + > + reg = <0>; > + phy-mode = "gmii"; > + phys = <&serdes 0 CU(0)>; > + }; > + > + port1: port@1 { > + phy-handle = <&lan966x_phy1>; > + > + reg = <1>; > + phy-mode = "gmii"; > + phys = <&serdes 1 CU(1)>; > + }; > + }; > + }; > + }; > + }; > + }; > +}; > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > index 568410e64ce6..30b64994784c 100644 > --- a/drivers/pci/quirks.c > +++ b/drivers/pci/quirks.c > @@ -6241,6 +6241,7 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0xa76e, dpc_log_size); > DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_XILINX, 0x5020, of_pci_make_dev_node); > DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_XILINX, 0x5021, of_pci_make_dev_node); > DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_REDHAT, 0x0005, of_pci_make_dev_node); > +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_EFAR, 0x9660, of_pci_make_dev_node); > > /* > * Devices known to require a longer delay before first config space access > -- > 2.45.0 >
Hi Lee, On Thu, 11 Jul 2024 16:29:52 +0100 Lee Jones <lee@kernel.org> wrote: > On Thu, 27 Jun 2024, Herve Codina wrote: > > > Add a PCI driver that handles the LAN966x PCI device using a device-tree > > overlay. This overlay is applied to the PCI device DT node and allows to > > describe components that are present in the device. > > > > The memory from the device-tree is remapped to the BAR memory thanks to > > "ranges" properties computed at runtime by the PCI core during the PCI > > enumeration. > > > > The PCI device itself acts as an interrupt controller and is used as the > > parent of the internal LAN966x interrupt controller to route the > > interrupts to the assigned PCI INTx interrupt. > > Not entirely sure why this is in MFD. This PCI driver purpose is to instanciate many other drivers using a DT overlay. I think MFD is the right subsystem. It acts as an interrupt controller because we need to have a bridge between the device-tree interrupt world and the the PCI world. This bridge is needed and specific to the driver in order to have resources available from the device-tree world present in the applied overlay. > > Also I'm unsure of his current views, but Greg has voiced pretty big > feelings about representing PCI drivers as Platform ones in the past. PCI drivers as Plaform ones ? I probably missed something. Can you give me more details ? Best regards, Hervé
On Thu, Jul 11, 2024 at 06:44:38PM +0200, Herve Codina wrote: > Hi Lee, > > On Thu, 11 Jul 2024 16:29:52 +0100 > Lee Jones <lee@kernel.org> wrote: > > > On Thu, 27 Jun 2024, Herve Codina wrote: > > > > > Add a PCI driver that handles the LAN966x PCI device using a device-tree > > > overlay. This overlay is applied to the PCI device DT node and allows to > > > describe components that are present in the device. > > > > > > The memory from the device-tree is remapped to the BAR memory thanks to > > > "ranges" properties computed at runtime by the PCI core during the PCI > > > enumeration. > > > > > > The PCI device itself acts as an interrupt controller and is used as the > > > parent of the internal LAN966x interrupt controller to route the > > > interrupts to the assigned PCI INTx interrupt. > > > > Not entirely sure why this is in MFD. > > This PCI driver purpose is to instanciate many other drivers using a DT > overlay. I think MFD is the right subsystem. Please use the aux bus for that, that is what is was specifically designed for, and what it is being used for today. thanks, greg k-h
On Thu, Jul 11, 2024 at 1:08 PM Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > > On Thu, Jul 11, 2024 at 06:44:38PM +0200, Herve Codina wrote: > > Hi Lee, > > > > On Thu, 11 Jul 2024 16:29:52 +0100 > > Lee Jones <lee@kernel.org> wrote: > > > > > On Thu, 27 Jun 2024, Herve Codina wrote: > > > > > > > Add a PCI driver that handles the LAN966x PCI device using a device-tree > > > > overlay. This overlay is applied to the PCI device DT node and allows to > > > > describe components that are present in the device. > > > > > > > > The memory from the device-tree is remapped to the BAR memory thanks to > > > > "ranges" properties computed at runtime by the PCI core during the PCI > > > > enumeration. > > > > > > > > The PCI device itself acts as an interrupt controller and is used as the > > > > parent of the internal LAN966x interrupt controller to route the > > > > interrupts to the assigned PCI INTx interrupt. > > > > > > Not entirely sure why this is in MFD. > > > > This PCI driver purpose is to instanciate many other drivers using a DT > > overlay. I think MFD is the right subsystem. It is a Multi-function Device, but it doesn't appear to use any of the MFD subsystem. So maybe drivers/soc/? Another dumping ground, but it is a driver for an SoC exposed as a PCI device. > Please use the aux bus for that, that is what is was specifically > designed for, and what it is being used for today. We discussed this already[1]. Different usecase, but needs the same thing. Like I said before, the bus and device used for DT MMIO devices is like it or not platform bus/device. In this case, all the child devices are already supported as platform devices. There would be zero benefit to add all the boilerplate to make their drivers both platform and aux bus drivers. Plus there is zero DT support in aux bus. Rob [1] https://lore.kernel.org/all/Y9kuxrL3XaCG+blk@kroah.com/
On Thu, Jul 11, 2024 at 02:33:26PM -0600, Rob Herring wrote: > In this case, all the child devices are already supported as platform > devices. There would be zero benefit to add all the boilerplate to > make their drivers both platform and aux bus drivers. Plus there is > zero DT support in aux bus. It is by design that there is 0 DT support in aux bus :) But ok, I'll trust you on this usage... greg k-h
Hi Rob, Conor, On Thu, 11 Jul 2024 14:33:26 -0600 Rob Herring <robh@kernel.org> wrote: > On Thu, Jul 11, 2024 at 1:08 PM Greg Kroah-Hartman > <gregkh@linuxfoundation.org> wrote: > > > > On Thu, Jul 11, 2024 at 06:44:38PM +0200, Herve Codina wrote: > > > Hi Lee, > > > > > > On Thu, 11 Jul 2024 16:29:52 +0100 > > > Lee Jones <lee@kernel.org> wrote: > > > > > > > On Thu, 27 Jun 2024, Herve Codina wrote: > > > > > > > > > Add a PCI driver that handles the LAN966x PCI device using a device-tree > > > > > overlay. This overlay is applied to the PCI device DT node and allows to > > > > > describe components that are present in the device. > > > > > > > > > > The memory from the device-tree is remapped to the BAR memory thanks to > > > > > "ranges" properties computed at runtime by the PCI core during the PCI > > > > > enumeration. > > > > > > > > > > The PCI device itself acts as an interrupt controller and is used as the > > > > > parent of the internal LAN966x interrupt controller to route the > > > > > interrupts to the assigned PCI INTx interrupt. > > > > > > > > Not entirely sure why this is in MFD. > > > > > > This PCI driver purpose is to instanciate many other drivers using a DT > > > overlay. I think MFD is the right subsystem. > > It is a Multi-function Device, but it doesn't appear to use any of the > MFD subsystem. So maybe drivers/soc/? Another dumping ground, but it > is a driver for an SoC exposed as a PCI device. > In drivers/soc, drivers/soc/microchip/ could be the right place. Conor, are you open to have the PCI LAN966x device driver in drivers/soc/microchip/ ? Best regards, Hervé
On Fri, Jul 12, 2024, at 15:11, Herve Codina wrote: > On Thu, 11 Jul 2024 14:33:26 -0600 Rob Herring <robh@kernel.org> wrote: >> On Thu, Jul 11, 2024 at 1:08 PM Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: >> > > >> > > This PCI driver purpose is to instanciate many other drivers using a DT >> > > overlay. I think MFD is the right subsystem. >> >> It is a Multi-function Device, but it doesn't appear to use any of the >> MFD subsystem. So maybe drivers/soc/? Another dumping ground, but it >> is a driver for an SoC exposed as a PCI device. >> > > In drivers/soc, drivers/soc/microchip/ could be the right place. > > Conor, are you open to have the PCI LAN966x device driver in > drivers/soc/microchip/ ? That sounds like a much worse fit than drivers/mfd: the code here does not actually run on the lan966x soc, it instead runs on whatever other machine you happen to plug it into as a PCI device. Arnd
Hi Arnd, On Fri, 12 Jul 2024 16:14:31 +0200 "Arnd Bergmann" <arnd@arndb.de> wrote: > On Fri, Jul 12, 2024, at 15:11, Herve Codina wrote: > > On Thu, 11 Jul 2024 14:33:26 -0600 Rob Herring <robh@kernel.org> wrote: > >> On Thu, Jul 11, 2024 at 1:08 PM Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > > >> > > > >> > > This PCI driver purpose is to instanciate many other drivers using a DT > >> > > overlay. I think MFD is the right subsystem. > >> > >> It is a Multi-function Device, but it doesn't appear to use any of the > >> MFD subsystem. So maybe drivers/soc/? Another dumping ground, but it > >> is a driver for an SoC exposed as a PCI device. > >> > > > > In drivers/soc, drivers/soc/microchip/ could be the right place. > > > > Conor, are you open to have the PCI LAN966x device driver in > > drivers/soc/microchip/ ? > > That sounds like a much worse fit than drivers/mfd: the code > here does not actually run on the lan966x soc, it instead runs > on whatever other machine you happen to plug it into as a > PCI device. Maybe drivers/misc ? Best regards, Hervé
On Mon, Jul 15, 2024, at 14:12, Herve Codina wrote: > Hi Arnd, > > On Fri, 12 Jul 2024 16:14:31 +0200 > "Arnd Bergmann" <arnd@arndb.de> wrote: > >> On Fri, Jul 12, 2024, at 15:11, Herve Codina wrote: >> > On Thu, 11 Jul 2024 14:33:26 -0600 Rob Herring <robh@kernel.org> wrote: >> >> On Thu, Jul 11, 2024 at 1:08 PM Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: >> >> >> > > >> >> > > This PCI driver purpose is to instanciate many other drivers using a DT >> >> > > overlay. I think MFD is the right subsystem. >> >> >> >> It is a Multi-function Device, but it doesn't appear to use any of the >> >> MFD subsystem. So maybe drivers/soc/? Another dumping ground, but it >> >> is a driver for an SoC exposed as a PCI device. >> >> >> > >> > In drivers/soc, drivers/soc/microchip/ could be the right place. >> > >> > Conor, are you open to have the PCI LAN966x device driver in >> > drivers/soc/microchip/ ? >> >> That sounds like a much worse fit than drivers/mfd: the code >> here does not actually run on the lan966x soc, it instead runs >> on whatever other machine you happen to plug it into as a >> PCI device. > > Maybe drivers/misc ? That's probably a little better, and there is already drivers/misc/mchp_pci1xxxx in there, which also has some aux devices. Maybe we need a new place and then move both of these and some of the similar devices from drivers/mfd to that, but we don't really have to pick one now. Arnd
On Tue, 16 Jul 2024 16:44:12 +0200 "Arnd Bergmann" <arnd@arndb.de> wrote: > On Mon, Jul 15, 2024, at 14:12, Herve Codina wrote: > > Hi Arnd, > > > > On Fri, 12 Jul 2024 16:14:31 +0200 > > "Arnd Bergmann" <arnd@arndb.de> wrote: > > > >> On Fri, Jul 12, 2024, at 15:11, Herve Codina wrote: > >> > On Thu, 11 Jul 2024 14:33:26 -0600 Rob Herring <robh@kernel.org> wrote: > >> >> On Thu, Jul 11, 2024 at 1:08 PM Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > >> > >> >> > > > >> >> > > This PCI driver purpose is to instanciate many other drivers using a DT > >> >> > > overlay. I think MFD is the right subsystem. > >> >> > >> >> It is a Multi-function Device, but it doesn't appear to use any of the > >> >> MFD subsystem. So maybe drivers/soc/? Another dumping ground, but it > >> >> is a driver for an SoC exposed as a PCI device. > >> >> > >> > > >> > In drivers/soc, drivers/soc/microchip/ could be the right place. > >> > > >> > Conor, are you open to have the PCI LAN966x device driver in > >> > drivers/soc/microchip/ ? > >> > >> That sounds like a much worse fit than drivers/mfd: the code > >> here does not actually run on the lan966x soc, it instead runs > >> on whatever other machine you happen to plug it into as a > >> PCI device. > > > > Maybe drivers/misc ? > > That's probably a little better, and there is already > drivers/misc/mchp_pci1xxxx in there, which also has some > aux devices. > > Maybe we need a new place and then move both of these > and some of the similar devices from drivers/mfd to that, but > we don't really have to pick one now. > In the next iteration, I plan to move the lan966x pci driver in drivers/misc/ Not sure that it needs to be in a subdir. Best regards Hervé
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig index 266b4f54af60..15db144bc09b 100644 --- a/drivers/mfd/Kconfig +++ b/drivers/mfd/Kconfig @@ -144,6 +144,30 @@ config MFD_ATMEL_FLEXCOM by the probe function of this MFD driver according to a device tree property. +config MFD_LAN966X_PCI + tristate "Microchip LAN966x PCIe Support" + depends on PCI + select OF + select OF_OVERLAY + select IRQ_DOMAIN + help + This enables the support for the LAN966x PCIe device. + This is used to drive the LAN966x PCIe device from the host system + to which it is connected. + + This driver uses an overlay to load other drivers to support for + LAN966x internal components. + Even if this driver does not depend on these other drivers, in order + to have a fully functional board, the following drivers are needed: + - fixed-clock (COMMON_CLK) + - lan966x-oic (LAN966X_OIC) + - lan966x-cpu-syscon (MFD_SYSCON) + - lan966x-switch-reset (RESET_MCHP_SPARX5) + - lan966x-pinctrl (PINCTRL_OCELOT) + - lan966x-serdes (PHY_LAN966X_SERDES) + - lan966x-miim (MDIO_MSCC_MIIM) + - lan966x-switch (LAN966X_SWITCH) + config MFD_ATMEL_HLCDC tristate "Atmel HLCDC (High-end LCD Controller)" select MFD_CORE diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile index c66f07edcd0e..165a9674ff48 100644 --- a/drivers/mfd/Makefile +++ b/drivers/mfd/Makefile @@ -284,3 +284,7 @@ rsmu-i2c-objs := rsmu_core.o rsmu_i2c.o rsmu-spi-objs := rsmu_core.o rsmu_spi.o obj-$(CONFIG_MFD_RSMU_I2C) += rsmu-i2c.o obj-$(CONFIG_MFD_RSMU_SPI) += rsmu-spi.o + +lan966x-pci-objs := lan966x_pci.o +lan966x-pci-objs += lan966x_pci.dtbo.o +obj-$(CONFIG_MFD_LAN966X_PCI) += lan966x-pci.o diff --git a/drivers/mfd/lan966x_pci.c b/drivers/mfd/lan966x_pci.c new file mode 100644 index 000000000000..c69350449b15 --- /dev/null +++ b/drivers/mfd/lan966x_pci.c @@ -0,0 +1,229 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Microchip LAN966x PCI driver + * + * Copyright (c) 2024 Microchip Technology Inc. and its subsidiaries. + * + * Authors: + * Clément Léger <clement.leger@bootlin.com> + * Hervé Codina <herve.codina@bootlin.com> + */ + +#include <linux/irq.h> +#include <linux/irqdomain.h> +#include <linux/module.h> +#include <linux/of_platform.h> +#include <linux/pci.h> +#include <linux/pci_ids.h> +#include <linux/slab.h> + +/* Embedded dtbo symbols created by cmd_wrap_S_dtb in scripts/Makefile.lib */ +extern char __dtbo_lan966x_pci_begin[]; +extern char __dtbo_lan966x_pci_end[]; + +struct pci_dev_intr_ctrl { + struct pci_dev *pci_dev; + struct irq_domain *irq_domain; + int irq; +}; + +static int pci_dev_irq_domain_map(struct irq_domain *d, unsigned int virq, irq_hw_number_t hw) +{ + irq_set_chip_and_handler(virq, &dummy_irq_chip, handle_simple_irq); + return 0; +} + +static const struct irq_domain_ops pci_dev_irq_domain_ops = { + .map = pci_dev_irq_domain_map, + .xlate = irq_domain_xlate_onecell, +}; + +static irqreturn_t pci_dev_irq_handler(int irq, void *data) +{ + struct pci_dev_intr_ctrl *intr_ctrl = data; + int ret; + + ret = generic_handle_domain_irq(intr_ctrl->irq_domain, 0); + return IRQ_RETVAL(!ret); +} + +static struct pci_dev_intr_ctrl *pci_dev_create_intr_ctrl(struct pci_dev *pdev) +{ + struct pci_dev_intr_ctrl *intr_ctrl; + struct fwnode_handle *fwnode; + int ret; + + if (!pdev->irq) + return ERR_PTR(-EOPNOTSUPP); + + fwnode = dev_fwnode(&pdev->dev); + if (!fwnode) + return ERR_PTR(-ENODEV); + + intr_ctrl = kmalloc(sizeof(*intr_ctrl), GFP_KERNEL); + if (!intr_ctrl) + return ERR_PTR(-ENOMEM); + + intr_ctrl->pci_dev = pdev; + + intr_ctrl->irq_domain = irq_domain_create_linear(fwnode, 1, &pci_dev_irq_domain_ops, + intr_ctrl); + if (!intr_ctrl->irq_domain) { + pci_err(pdev, "Failed to create irqdomain\n"); + ret = -ENOMEM; + goto err_free_intr_ctrl; + } + + ret = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_INTX); + if (ret < 0) { + pci_err(pdev, "Unable alloc irq vector (%d)\n", ret); + goto err_remove_domain; + } + intr_ctrl->irq = pci_irq_vector(pdev, 0); + ret = request_irq(intr_ctrl->irq, pci_dev_irq_handler, IRQF_SHARED, + dev_name(&pdev->dev), intr_ctrl); + if (ret) { + pci_err(pdev, "Unable to request irq %d (%d)\n", intr_ctrl->irq, ret); + goto err_free_irq_vector; + } + + return intr_ctrl; + +err_free_irq_vector: + pci_free_irq_vectors(pdev); +err_remove_domain: + irq_domain_remove(intr_ctrl->irq_domain); +err_free_intr_ctrl: + kfree(intr_ctrl); + return ERR_PTR(ret); +} + +static void pci_dev_remove_intr_ctrl(struct pci_dev_intr_ctrl *intr_ctrl) +{ + free_irq(intr_ctrl->irq, intr_ctrl); + pci_free_irq_vectors(intr_ctrl->pci_dev); + irq_dispose_mapping(irq_find_mapping(intr_ctrl->irq_domain, 0)); + irq_domain_remove(intr_ctrl->irq_domain); + kfree(intr_ctrl); +} + +static void devm_pci_dev_remove_intr_ctrl(void *data) +{ + struct pci_dev_intr_ctrl *intr_ctrl = data; + + pci_dev_remove_intr_ctrl(intr_ctrl); +} + +static int devm_pci_dev_create_intr_ctrl(struct pci_dev *pdev) +{ + struct pci_dev_intr_ctrl *intr_ctrl; + + intr_ctrl = pci_dev_create_intr_ctrl(pdev); + if (IS_ERR(intr_ctrl)) + return PTR_ERR(intr_ctrl); + + return devm_add_action_or_reset(&pdev->dev, devm_pci_dev_remove_intr_ctrl, intr_ctrl); +} + +struct lan966x_pci { + struct device *dev; + int ovcs_id; +}; + +static int lan966x_pci_load_overlay(struct lan966x_pci *data) +{ + u32 dtbo_size = __dtbo_lan966x_pci_end - __dtbo_lan966x_pci_begin; + void *dtbo_start = __dtbo_lan966x_pci_begin; + int ret; + + ret = of_overlay_fdt_apply(dtbo_start, dtbo_size, &data->ovcs_id, dev_of_node(data->dev)); + if (ret) + return ret; + + return 0; +} + +static void lan966x_pci_unload_overlay(struct lan966x_pci *data) +{ + of_overlay_remove(&data->ovcs_id); +} + +static int lan966x_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) +{ + struct device *dev = &pdev->dev; + struct lan966x_pci *data; + int ret; + + /* + * On ACPI system, fwnode can point to the ACPI node. + * This driver needs an of_node to be used as the device-tree overlay + * target. This of_node should be set by the PCI core if it succeeds in + * creating it (CONFIG_PCI_DYNAMIC_OF_NODES feature). + * Check here for the validity of this of_node. + */ + if (!dev_of_node(dev)) { + dev_err(dev, "Missing of_node for device\n"); + return -EINVAL; + } + + /* Need to be done before devm_pci_dev_create_intr_ctrl. + * It allocates an IRQ and so pdev->irq is updated. + */ + ret = pcim_enable_device(pdev); + if (ret) + return ret; + + ret = devm_pci_dev_create_intr_ctrl(pdev); + if (ret) + return ret; + + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); + if (!data) + return -ENOMEM; + + pci_set_drvdata(pdev, data); + data->dev = dev; + + ret = lan966x_pci_load_overlay(data); + if (ret) + return ret; + + pci_set_master(pdev); + + ret = of_platform_default_populate(dev_of_node(dev), NULL, dev); + if (ret) + goto err_unload_overlay; + + return 0; + +err_unload_overlay: + lan966x_pci_unload_overlay(data); + return ret; +} + +static void lan966x_pci_remove(struct pci_dev *pdev) +{ + struct lan966x_pci *data = pci_get_drvdata(pdev); + + of_platform_depopulate(data->dev); + + lan966x_pci_unload_overlay(data); +} + +static struct pci_device_id lan966x_pci_ids[] = { + { PCI_DEVICE(PCI_VENDOR_ID_EFAR, 0x9660) }, + { } +}; +MODULE_DEVICE_TABLE(pci, lan966x_pci_ids); + +static struct pci_driver lan966x_pci_driver = { + .name = "mchp_lan966x_pci", + .id_table = lan966x_pci_ids, + .probe = lan966x_pci_probe, + .remove = lan966x_pci_remove, +}; +module_pci_driver(lan966x_pci_driver); + +MODULE_AUTHOR("Herve Codina <herve.codina@bootlin.com>"); +MODULE_DESCRIPTION("Microchip LAN966x PCI driver"); +MODULE_LICENSE("GPL"); diff --git a/drivers/mfd/lan966x_pci.dtso b/drivers/mfd/lan966x_pci.dtso new file mode 100644 index 000000000000..041f4319e4cd --- /dev/null +++ b/drivers/mfd/lan966x_pci.dtso @@ -0,0 +1,167 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2022 Microchip UNG + */ + +#include <dt-bindings/clock/microchip,lan966x.h> +#include <dt-bindings/interrupt-controller/irq.h> +#include <dt-bindings/mfd/atmel-flexcom.h> +#include <dt-bindings/phy/phy-lan966x-serdes.h> +#include <dt-bindings/gpio/gpio.h> + +/dts-v1/; +/plugin/; + +/ { + fragment@0 { + target-path=""; + __overlay__ { + #address-cells = <3>; + #size-cells = <2>; + + pci-ep-bus@0 { + compatible = "simple-bus"; + #address-cells = <1>; + #size-cells = <1>; + + /* + * map @0xe2000000 (32MB) to BAR0 (CPU) + * map @0xe0000000 (16MB) to BAR1 (AMBA) + */ + ranges = <0xe2000000 0x00 0x00 0x00 0x2000000 + 0xe0000000 0x01 0x00 0x00 0x1000000>; + + oic: oic@e00c0120 { + compatible = "microchip,lan966x-oic"; + #interrupt-cells = <2>; + interrupt-controller; + interrupts = <0>; /* PCI INTx assigned interrupt */ + reg = <0xe00c0120 0x190>; + }; + + cpu_clk: cpu_clk { + compatible = "fixed-clock"; + #clock-cells = <0>; + clock-frequency = <600000000>; // CPU clock = 600MHz + }; + + ddr_clk: ddr_clk { + compatible = "fixed-clock"; + #clock-cells = <0>; + clock-frequency = <30000000>; // Fabric clock = 30MHz + }; + + sys_clk: sys_clk { + compatible = "fixed-clock"; + #clock-cells = <0>; + clock-frequency = <15625000>; // System clock = 15.625MHz + }; + + cpu_ctrl: syscon@e00c0000 { + compatible = "microchip,lan966x-cpu-syscon", "syscon"; + reg = <0xe00c0000 0xa8>; + }; + + reset: reset@e200400c { + compatible = "microchip,lan966x-switch-reset"; + reg = <0xe200400c 0x4>; + reg-names = "gcb"; + #reset-cells = <1>; + cpu-syscon = <&cpu_ctrl>; + }; + + gpio: pinctrl@e2004064 { + compatible = "microchip,lan966x-pinctrl"; + reg = <0xe2004064 0xb4>, + <0xe2010024 0x138>; + resets = <&reset 0>; + reset-names = "switch"; + gpio-controller; + #gpio-cells = <2>; + gpio-ranges = <&gpio 0 0 78>; + interrupt-parent = <&oic>; + interrupt-controller; + interrupts = <17 IRQ_TYPE_LEVEL_HIGH>; + #interrupt-cells = <2>; + + tod_pins: tod_pins { + pins = "GPIO_36"; + function = "ptpsync_1"; + }; + + fc0_a_pins: fcb4-i2c-pins { + /* RXD, TXD */ + pins = "GPIO_9", "GPIO_10"; + function = "fc0_a"; + }; + + }; + + serdes: serdes@e202c000 { + compatible = "microchip,lan966x-serdes"; + reg = <0xe202c000 0x9c>, + <0xe2004010 0x4>; + #phy-cells = <2>; + }; + + mdio1: mdio@e200413c { + #address-cells = <1>; + #size-cells = <0>; + compatible = "microchip,lan966x-miim"; + reg = <0xe200413c 0x24>, + <0xe2010020 0x4>; + + resets = <&reset 0>; + reset-names = "switch"; + + lan966x_phy0: ethernet-lan966x_phy@1 { + reg = <1>; + }; + + lan966x_phy1: ethernet-lan966x_phy@2 { + reg = <2>; + }; + }; + + switch: switch@e0000000 { + compatible = "microchip,lan966x-switch"; + reg = <0xe0000000 0x0100000>, + <0xe2000000 0x0800000>; + reg-names = "cpu", "gcb"; + + interrupt-parent = <&oic>; + interrupts = <12 IRQ_TYPE_LEVEL_HIGH>, + <9 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "xtr", "ana"; + + resets = <&reset 0>; + reset-names = "switch"; + + pinctrl-names = "default"; + pinctrl-0 = <&tod_pins>; + + ethernet-ports { + #address-cells = <1>; + #size-cells = <0>; + + port0: port@0 { + phy-handle = <&lan966x_phy0>; + + reg = <0>; + phy-mode = "gmii"; + phys = <&serdes 0 CU(0)>; + }; + + port1: port@1 { + phy-handle = <&lan966x_phy1>; + + reg = <1>; + phy-mode = "gmii"; + phys = <&serdes 1 CU(1)>; + }; + }; + }; + }; + }; + }; +}; diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index 568410e64ce6..30b64994784c 100644 --- a/drivers/pci/quirks.c +++ b/drivers/pci/quirks.c @@ -6241,6 +6241,7 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0xa76e, dpc_log_size); DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_XILINX, 0x5020, of_pci_make_dev_node); DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_XILINX, 0x5021, of_pci_make_dev_node); DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_REDHAT, 0x0005, of_pci_make_dev_node); +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_EFAR, 0x9660, of_pci_make_dev_node); /* * Devices known to require a longer delay before first config space access
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