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

[Alex Elder <elder@riscstar.com>]

On 9/15/26 1:22 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] tc9564: missing Kconfig dependencies on OF and PCI_DYNAMIC_OF_NODES
> --
> 
> commit e821d4f3b3a6f26cf2df684fa3788ceb3794a058
> Author: Alex Elder <elder@riscstar.com>
> Subject: misc: tc9564: introduce base PCI driver
> 
> This commit introduces a base PCI driver for the Toshiba TC9564 SoC. It binds
> with embedded PCIe endpoint functions within the chip to perform basic PCI
> initialization. The driver then uses Open Firmware APIs to populate and probe
> endpoint bus children.
> 
>> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
>> --- a/drivers/misc/Kconfig
>> +++ b/drivers/misc/Kconfig
>> @@ -568,6 +568,15 @@ config MCHP_LAN966X_PCI
>>   	    - lan966x-miim (MDIO_MSCC_MIIM)
>>   	    - lan966x-switch (LAN966X_SWITCH)
>>   
>> +config TC9564_PCI
>> +	tristate "Toshiba TC9564 PCI function support"
>> +	depends on PCI
> 
> [Severity: Medium]
> Should this configuration also depend on OF and PCI_DYNAMIC_OF_NODES?

I thought about this but intentionally omitted it, because
*this* driver doesn't depend on PCI_DYNAMIC_OF_NODES (at
least, not at compile time).
> Because the driver relies entirely on Open Firmware to function, compiling
> it without CONFIG_OF or CONFIG_PCI_DYNAMIC_OF_NODES will result in a
> successful build but an unconditional runtime probe failure.

I don't mind adding the dependency.  Even if not having
it is technically OK, for the driver to actually function
as desired those things must be defined.

If anyone feels strongly that adding "depends on OF" and
"depends on PCI_DYNAMIC_OF_NODES" to the TC9564_PCI Kconfig
option should *not* be there, please say so.

I'm not going to post a new version of this series for
at least a few days, but without objection I'll add
these changes when I do.

					-Alex

> [ ... ]
> 
>> diff --git a/drivers/misc/tc9564-pci.c b/drivers/misc/tc9564-pci.c
>> --- /dev/null
>> +++ b/drivers/misc/tc9564-pci.c
>> @@ -0,0 +1,84 @@
> [ ... ]
>> +static int
>> +tc9564_function_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	struct device_node *np;
>> +	int ret;
>> +
>> +	/* Despite being a PCI device, we require devicetree */
>> +	np = dev_of_node(dev);
>> +	if (!np)
>> +		return dev_err_probe(dev, -EINVAL, "no devicetree node\n");
> 
> [Severity: Medium]
> Will this probe fail unconditionally if the required dependencies are missing?
> 
> If CONFIG_OF is disabled, dev_of_node() will always return NULL, causing
> dev_err_probe() to trigger here.>
> Similarly, this commit relies on of_pci_make_dev_node() via a PCI quirk to
> dynamically create the devicetree node for the PCIe function. If
> CONFIG_PCI_DYNAMIC_OF_NODES is disabled, of_pci_make_dev_node() compiles
> to an empty stub, meaning the node is never created.
>