On Wed, Feb 14, 2024 at 02:18:31PM +0530, Manivannan Sadhasivam wrote:
> +/**
> + * of_pci_bridge_d3 - Check if the bridge is supporting D3 states or not
> + *
> + * @node: device tree node of the bridge
> + *
> + * Return: True if the bridge is supporting D3 states, False otherwise.
A lot of kernel-doc uses %true and %false.
> +bool of_pci_bridge_d3(struct device_node *node)
> +{
> + return of_property_read_bool(node, "supports-d3");
> +}
What's the difference between of_property_read_bool() and
of_property_present()? When should one use which?
The former has 691 occurrences in the tree, the latter 120.
The latter would seem more "literary" / readable here,
but maybe that's just me.
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -1142,6 +1142,9 @@ static inline bool platform_pci_bridge_d3(struct pci_dev *dev)
> if (pci_use_mid_pm())
> return false;
>
> + if (dev->dev.of_node)
> + return of_pci_bridge_d3(dev->dev.of_node);
> +
> return acpi_pci_bridge_d3(dev);
> }
This will result in an unnecessary test on non-DT platforms (e.g. ACPI)
whether dev->dev.of_node is set.
Please use dev_of_node() instead of "dev->dev.of_node" so that the
code added here can be optimized away by the compiler on non-DT
platforms (due to the IS_ENABLED(CONFIG_OF)).
Thanks,
Lukas
