| Message ID | 20250507071315.394857-24-herve.codina@bootlin.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,01/26] Revert "treewide: Fix probing of devices in DT overlays" | expand |
On Wed, May 07, 2025 at 09:13:05AM +0200, Herve Codina wrote: > Only one device-tree overlay (lan966x_evb_lan9662_nic.dtbo) is handled > and this overlay is directly referenced in lan966x_pci_load_overlay(). > > This avoid to use the code for an other board. > > In order to be more generic and to allow support for other boards (PCI > Vendor/Device IDs), introduce the lan966x_pci_info structure and attach > it to PCI Vendor/Device IDs handled by the driver. > > This structure contains information related to the PCI board such as > information related to the dtbo describing the board we have to load. > > Signed-off-by: Herve Codina <herve.codina@bootlin.com> How big is the dtbo ? This is going in the right direction. I'm just wondering if each dtbo should be wrapped in its own very slim PCI driver, which simply registers its lan966x_pci_info structure to a core driver. Only the needed dtbo will then be loaded into memory as a module, not them all. Pretty much all the pieces are here, so it can be done later. Reviewed-by: Andrew Lunn <andrew@lunn.ch> Andrew
On Thu, 8 May 2025 at 00:24, Andrew Lunn <andrew@lunn.ch> wrote: > On Wed, May 07, 2025 at 09:13:05AM +0200, Herve Codina wrote: > > Only one device-tree overlay (lan966x_evb_lan9662_nic.dtbo) is handled > > and this overlay is directly referenced in lan966x_pci_load_overlay(). > > > > This avoid to use the code for an other board. > > > > In order to be more generic and to allow support for other boards (PCI > > Vendor/Device IDs), introduce the lan966x_pci_info structure and attach > > it to PCI Vendor/Device IDs handled by the driver. > > > > This structure contains information related to the PCI board such as > > information related to the dtbo describing the board we have to load. > > > > Signed-off-by: Herve Codina <herve.codina@bootlin.com> > > How big is the dtbo ? > > This is going in the right direction. I'm just wondering if each dtbo > should be wrapped in its own very slim PCI driver, which simply > registers its lan966x_pci_info structure to a core driver. Only the > needed dtbo will then be loaded into memory as a module, not them all. Alternatively, the dtbo could be loaded through request_firmware(). That could lead to a generic support option in the PCI core, which would fallback to loading pci-<vid>-<pid>.dtbo when no driver is available. > Pretty much all the pieces are here, so it can be done later. Exactly. Gr{oetje,eeting}s, Geert
diff --git a/drivers/misc/lan966x_pci.c b/drivers/misc/lan966x_pci.c index b28066c96534..e136bb17c678 100644 --- a/drivers/misc/lan966x_pci.c +++ b/drivers/misc/lan966x_pci.c @@ -18,10 +18,6 @@ #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_evb_lan9662_nic_begin[]; -extern char __dtbo_lan966x_evb_lan9662_nic_end[]; - struct pci_dev_intr_ctrl { struct pci_dev *pci_dev; struct irq_domain *irq_domain; @@ -118,17 +114,23 @@ static int devm_pci_dev_create_intr_ctrl(struct pci_dev *pdev) return devm_add_action_or_reset(&pdev->dev, devm_pci_dev_remove_intr_ctrl, intr_ctrl); } +struct lan966x_pci_info { + void *dtbo_begin; + void *dtbo_end; +}; + struct lan966x_pci { struct device *dev; int ovcs_id; + const struct lan966x_pci_info *info; }; static int lan966x_pci_load_overlay(struct lan966x_pci *data) { - u32 dtbo_size = __dtbo_lan966x_evb_lan9662_nic_end - __dtbo_lan966x_evb_lan9662_nic_begin; - void *dtbo_start = __dtbo_lan966x_evb_lan9662_nic_begin; + const struct lan966x_pci_info *info = data->info; - return of_overlay_fdt_apply(dtbo_start, dtbo_size, &data->ovcs_id, dev_of_node(data->dev)); + return of_overlay_fdt_apply(info->dtbo_begin, info->dtbo_end - info->dtbo_begin, + &data->ovcs_id, dev_of_node(data->dev)); } static void lan966x_pci_unload_overlay(struct lan966x_pci *data) @@ -169,6 +171,9 @@ static int lan966x_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i pci_set_drvdata(pdev, data); data->dev = dev; + data->info = (const struct lan966x_pci_info *)id->driver_data; + if (!data->info) + return -EINVAL; ret = lan966x_pci_load_overlay(data); if (ret) @@ -196,8 +201,17 @@ static void lan966x_pci_remove(struct pci_dev *pdev) lan966x_pci_unload_overlay(data); } +/* Embedded dtbo symbols created by cmd_wrap_S_dtb in scripts/Makefile.lib */ +extern char __dtbo_lan966x_evb_lan9662_nic_begin[]; +extern char __dtbo_lan966x_evb_lan9662_nic_end[]; + +static struct lan966x_pci_info evb_lan9662_nic_info = { + .dtbo_begin = __dtbo_lan966x_evb_lan9662_nic_begin, + .dtbo_end = __dtbo_lan966x_evb_lan9662_nic_end, +}; + static struct pci_device_id lan966x_pci_ids[] = { - { PCI_DEVICE(PCI_VENDOR_ID_EFAR, 0x9660) }, + { PCI_VDEVICE(EFAR, 0x9660), (kernel_ulong_t)&evb_lan9662_nic_info }, { } }; MODULE_DEVICE_TABLE(pci, lan966x_pci_ids);
Only one device-tree overlay (lan966x_evb_lan9662_nic.dtbo) is handled and this overlay is directly referenced in lan966x_pci_load_overlay(). This avoid to use the code for an other board. In order to be more generic and to allow support for other boards (PCI Vendor/Device IDs), introduce the lan966x_pci_info structure and attach it to PCI Vendor/Device IDs handled by the driver. This structure contains information related to the PCI board such as information related to the dtbo describing the board we have to load. Signed-off-by: Herve Codina <herve.codina@bootlin.com> --- drivers/misc/lan966x_pci.c | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-)