Re: [PATCH v4 2/3] misc: tc9564: introduce base PCI driver

[Alex Elder <elder@riscstar.com>]

On 9/18/26 12:30 PM, Bjorn Helgaas wrote:
> [+cc Andrea, Herve, Lizhi for of_pci_make_dev_node() quirks]
> 
> On Fri, Sep 18, 2026 at 10:26:57AM -0500, Alex Elder wrote:
>> The Toshiba TC9564 is small and highly-specialized SoC that implements
>> a PCIe switch as well as an Ethernet AVB/TSN bridge.  In addition to
>> these, the SoC implements other functions, including a reset and clock
>> controller, an address translation unit, and a few other devices.  PCIe
>> BARs provide access to registers that manage these IP blocks, and the
>> SoC is modeled using a PCI endpoint bus in devicetree.  This allows the
>> IP blocks to be bound to platform drivers that do MMIO via the PCI BARs.
>>
>> Create a new PCI driver under drivers/misc that binds with the embedded
>> PCI endpoint functions within the TC9564 SoC.  Because these functions
>> will use devicetree pci-ep-bus to provide access to other IP blocks
>> within the TC9564 chip, the main purpose of this driver is to do basic
>> PCI initialization, then call of_platform_default_populate() to scan for
>> the any endpoint bus children, and probe all devices defined therein.
>>
>> Because we're using pci-ep-bus, we need to use the PCI quirks mechanism
>> to have of_pci_make_dev_node() be called for each endpoint device in
>> pci_bus_add_device() (via pci_fixup_device(pci_fixup_final, dev)).
>>
>> Co-developed-by: Daniel Thompson <daniel@riscstar.com>
>> Signed-off-by: Daniel Thompson <daniel@riscstar.com>
>> Signed-off-by: Alex Elder <elder@riscstar.com>
> 
> Acked-by: Bjorn Helgaas <bhelgaas@google.com>	# quirks.c
> 
> Not an issue for this patch, but I'm not sure the quirk mechanism is
> the best mechanism for doing this.  It's not working around a device
> defect like most quirks do.

I pretty much agree with you.  I think I mentioned this before
(though it might have been in a private conversation) that it
is an intentional act to call of_pci_make_dev_node() for an
endpoint (and not just a bridge).  And that's different from
a hardware quirk.  You only need to do it if there's a pci-ep-bus
sub-node on the endpoint.  (I'd have to verify this on the other
users of this approach to be 100% sure though.)

> I wonder if pci_bus_add_device() should unconditionally call a wrapper
> that calls of_pci_make_dev_node() for bridges and any device that
> appears in an allow-list.  I guess it sort of amounts to the same
> thing in the end, but it might be a little more explicit and not
> subject to CONFIG_PCI_QUIRKS.

Yes, I think it would be better to separate this case from PCI
quirks, which are really intended for anomalous hardware behavior.

I'd like to get these things merged, but would be willing to work
on this sort of thing (and/or on separating this driver type, as
Arnd suggested elsewhere).

Thanks a lot Bjorn.

					-Alex

>> +++ b/drivers/pci/quirks.c
>> @@ -6391,6 +6391,7 @@ 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);
>> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_TOSHIBA, 0x0220, of_pci_make_dev_node);
>>   
>>   /*
>>    * Devices known to require a longer delay before first config space access
>> -- 
>> 2.53.0
>>