Re: [PATCH 1/4] PCI: of: Add of_pci_clkreq_present()

[Bjorn Helgaas <helgaas@kernel.org>]

On Wed, Oct 22, 2025 at 11:05:50PM +0530, Manivannan Sadhasivam wrote:
> On Wed, Oct 22, 2025 at 11:22:29AM -0500, Bjorn Helgaas wrote:
> > On Wed, Oct 22, 2025 at 03:59:13PM +0530, Manivannan Sadhasivam wrote:
> > > On Wed, Oct 22, 2025 at 06:13:59PM +0800, Shawn Lin wrote:
> > > > 在 2025/10/22 星期三 18:02, Manivannan Sadhasivam 写道:
> > > > > On Tue, Oct 21, 2025 at 03:48:24PM +0800, Shawn Lin wrote:
> > > > > > of_pci_clkreq_present() is used by host drivers to decide whether the clkreq#
> > > > > > is properly connected and could enable L1.1/L1.2 support.
> > > > > > 
> > > > > > Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> > > > > > ---
> > > > > >   drivers/pci/of.c  | 18 ++++++++++++++++++
> > > > > >   drivers/pci/pci.h |  6 ++++++
> > > > > >   2 files changed, 24 insertions(+)
> > > > > > 
> > > > > > diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> > > > > > index 3579265f1198..52c6d365083b 100644
> > > > > > --- a/drivers/pci/of.c
> > > > > > +++ b/drivers/pci/of.c
> > > > > > @@ -1010,3 +1010,21 @@ int of_pci_get_equalization_presets(struct device *dev,
> > > > > >   	return 0;
> > > > > >   }
> > > > > >   EXPORT_SYMBOL_GPL(of_pci_get_equalization_presets);
> > > > > > +
> > > > > > +/**
> > > > > > + * of_pci_clkreq_present() - Check if the "supports-clkreq" is present
> > > > > 
> > > > > I don't see a benefit of this API, tbh. The API name creates an
> > > > > impression that the API will check for the presence of CLKREQ#
> > > > > signal in DT, but it checks for the presence of the
> > > > > 'supports-clkreq' property. Even though the presence of the
> > > > > property implies that the CLKREQ# routing is available, I'd
> > > > > prefer to check for the property explicitly instead of hiding it
> > > > > inside this API.
> > > > 
> > > > It makes sense.
> > > > 
> > > > Will the name of_pci_supports_clkreq_present() look good? Or we
> > > > just drop it and let host drivers to explicitly check
> > > > supports-clkreq inside their code?
> > > 
> > > I'd prefer to drop the API.
> > 
> > An API might help with consistency across DT bindings.  We don't want
> > drivers to pick their own names for 'supports-clkreq'.
> 
> I don't know what you mean by 'drivers to pick their own names'.
> With or without this API, the drivers have to use a local variable:

I just meant different names for the 'supports-clkreq' DT property.
But the schema I mentioned should be enough for that.