Messages in this thread | | | Date | Wed, 27 Mar 2024 16:16:07 +0000 | From | Simon Horman <> | Subject | Re: [PATCH v6 12/16] PCI/pwrctl: add PCI power control core code |
| |
On Mon, Mar 25, 2024 at 02:16:20PM +0100, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > Some PCI devices must be powered-on before they can be detected on the > bus. Introduce a simple framework reusing the existing PCI OF > infrastructure. > > The way this works is: a DT node representing a PCI device connected to > the port can be matched against its power control platform driver. If > the match succeeds, the driver is responsible for powering-up the device > and calling pcie_pwrctl_device_set_ready() which will trigger a PCI bus > rescan as well as subscribe to PCI bus notifications. > > When the device is detected and created, we'll make it consume the same > DT node that the platform device did. When the device is bound, we'll > create a device link between it and the parent power control device. > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Hi Bartosz,
some minor Kernel doc nits from my side.
..
> diff --git a/drivers/pci/pwrctl/core.c b/drivers/pci/pwrctl/core.c
..
> +/** > + * devm_pci_pwrctl_device_set_ready - Managed variant of > + * pci_pwrctl_device_set_ready(). > + *
nit: @dev should be documented here
> + * @pwrctl: PCI power control data > + * > + * Returns: > + * 0 on success, negative error number on error. > + */ > +int devm_pci_pwrctl_device_set_ready(struct device *dev, > + struct pci_pwrctl *pwrctl) > +{ > + int ret; > + > + ret = pci_pwrctl_device_set_ready(pwrctl); > + if (ret) > + return ret; > + > + return devm_add_action_or_reset(dev, > + devm_pci_pwrctl_device_unset_ready, > + pwrctl); > +} > +EXPORT_SYMBOL_GPL(devm_pci_pwrctl_device_set_ready); > + > +MODULE_AUTHOR("Bartosz Golaszewski <bartosz.golaszewski@linaro.org>"); > +MODULE_DESCRIPTION("PCI Device Power Control core driver"); > +MODULE_LICENSE("GPL"); > diff --git a/include/linux/pci-pwrctl.h b/include/linux/pci-pwrctl.h > new file mode 100644 > index 000000000000..ae8324ea7eeb > --- /dev/null > +++ b/include/linux/pci-pwrctl.h > @@ -0,0 +1,51 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * Copyright (C) 2024 Linaro Ltd. > + */ > + > +#ifndef __PCI_PWRCTL_H__ > +#define __PCI_PWRCTL_H__ > + > +#include <linux/notifier.h> > + > +struct device; > +struct device_link; > + > +/* > + * This is a simple framework for solving the issue of PCI devices that require > + * certain resources (regulators, GPIOs, clocks) to be enabled before the > + * device can actually be detected on the PCI bus. > + * > + * The idea is to reuse the platform bus to populate OF nodes describing the > + * PCI device and its resources, let these platform devices probe and enable > + * relevant resources and then trigger a rescan of the PCI bus allowing for the > + * same device (with a second associated struct device) to be registered with > + * the PCI subsystem. > + * > + * To preserve a correct hierarchy for PCI power management and device reset, > + * we create a device link between the power control platform device (parent) > + * and the supplied PCI device (child). > + */ > + > +/** > + * struct pci_pwrctl - PCI device power control context. > + * @dev - Address of the power controlling device.
nit: I think this should be "@dev: " rather than "@dev - " As is, "./scripts/kernel-doc -none" complains. > + * > + * An object of this type must be allocated by the PCI power control device and > + * passed to the pwrctl subsystem to trigger a bus rescan and setup a device > + * link with the device once it's up. > + */ > +struct pci_pwrctl { > + struct device *dev; > + > + /* Private, don't use. */
I think Private needs to be followed by a ':' rather than a ',' to keep kernel-doc happy.
> + struct notifier_block nb; > + struct device_link *link; > +}; > + > +int pci_pwrctl_device_set_ready(struct pci_pwrctl *pwrctl); > +void pci_pwrctl_device_unset_ready(struct pci_pwrctl *pwrctl); > +int devm_pci_pwrctl_device_set_ready(struct device *dev, > + struct pci_pwrctl *pwrctl); > + > +#endif /* __PCI_PWRCTL_H__ */ > -- > 2.40.1 > >
| |