On Tue, Dec 17, 2024 at 02:14:34PM +0100, Lukas Wunner wrote:
> On Mon, Dec 16, 2024 at 10:45:21AM +0530, Manivannan Sadhasivam wrote:
> > On Sun, Dec 15, 2024 at 06:19:22PM +0100, Lukas Wunner wrote:
> > > On Tue, Dec 10, 2024 at 03:25:24PM +0530, Manivannan Sadhasivam wrote:
> > > > diff --git a/drivers/pci/pwrctrl/core.c b/drivers/pci/pwrctrl/core.c
> > > > index 2fb174db91e5..9cc7e2b7f2b5 100644
> > > > --- a/drivers/pci/pwrctrl/core.c
> > > > +++ b/drivers/pci/pwrctrl/core.c
> > > > @@ -44,7 +44,7 @@ static void rescan_work_func(struct work_struct *work)
> > > > struct pci_pwrctrl, work);
> > > >
> > > > pci_lock_rescan_remove();
> > > > - pci_rescan_bus(to_pci_dev(pwrctrl->dev->parent)->bus);
> > > > + pci_rescan_bus(to_pci_host_bridge(pwrctrl->dev->parent)->bus);
> > > > pci_unlock_rescan_remove();
> > > > }
> > >
> > > Remind me, what's the purpose of this? I'm guessing that it
> > > recursively creates the platform devices below the newly
> > > powered up device, is that correct? If so, is it still
> > > necessary? Doesn't the new approach automatically create
> > > those devices upon their enumeration?
> >
> > If the pwrctrl driver is available at the time of platform device creation,
> > this is not needed. But if the driver is not available at that time and
> > probed later, then we need to rescan the bus to enumerate the devices.
>
> I see. Sounds like this can be made conditional on the caller
> being a module. I think you could achieve this with the following
> in pci_pwrctl_device_set_ready():
>
> - schedule_work(&pwrctl->work);
> + if (is_module_address(_RET_IP_))
> + schedule_work(&pwrctl->work);
>
> Though you'd also have to declare pci_pwrctl_device_set_ready()
> "__attribute__((always_inline))" so that it gets inlined into
> devm_pci_pwrctl_device_set_ready() and the condition works there
> as well.
>
I'd prefer to skip the rescan if the pwrctrl device is created and let the
pci_pwrctrl_device_set_ready() initiate rescan once the device is powered on.
This way, we could avoid scanning for the device twice.
> I'm wondering whether the bus notifier is still necessary with
> the new scheme. Since the power control device is instantiated
> and destroyed in unison with the pci_dev, can't the device link
> always be created on instantiation of the power control device?
>
I did move the devlink handling out of bus notifier callback with commit,
b458ff7e8176 ("PCI/pwrctl: Ensure that pwrctl drivers are probed before PCI
client drivers").
The bus notifier is only used to set 'of_node_reused' flag to indicate that the
corresponding DT node is already used.
- Mani
--
மணிவண்ணன் சதாசிவம்
