Re: [PATCH V2 12/13] i3c: mipi-i3c-hci-pci: Add LTR support for Intel controllers

[Frank Li <Frank.li@nxp.com>]

On Mon, Nov 17, 2025 at 08:09:41PM +0200, Adrian Hunter wrote:
> On 17/11/2025 19:54, Frank Li wrote:
> > On Mon, Nov 17, 2025 at 07:47:05PM +0200, Adrian Hunter wrote:
> >> On 17/11/2025 17:59, Frank Li wrote:
> >>> On Mon, Nov 17, 2025 at 01:36:36PM +0200, Adrian Hunter wrote:
> >>>> On 14/11/2025 22:09, Frank Li wrote:
> >>>>> On Fri, Nov 14, 2025 at 08:13:55PM +0200, Adrian Hunter wrote:
> >>>>>> Add support for Latency Tolerance Reporting (LTR) for Intel controllers.
> >>>>>>
> >>>>>> Implement PM ->set_latency_tolerance() callback to set LTR register values.
> >>>>>> Also expose LTR register values via debugfs.
> >>>>>>
> >>>>>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> >>>>>> ---
> >>>>>>
> >>>>>>
> >>>>>> Changes in V2:
> >>>>>>
> >>>>>> 	Make use of FIELD_PREP()
> >>>>>> 	Improve handling of values that exceed the maximum
> >>>>>>
> >>>>>>
> >>>>>>  .../master/mipi-i3c-hci/mipi-i3c-hci-pci.c    | 125 ++++++++++++++++++
> >>>>>>  1 file changed, 125 insertions(+)
> >>>>>>
> >>>>> ...
> >>>>>> +static void intel_ltr_expose(struct device *dev)
> >>>>>> +{
> >>>>>> +	dev->power.set_latency_tolerance = intel_ltr_set;
> >>>>>> +	dev_pm_qos_expose_latency_tolerance(dev);
> >>>>>> +}
> >>>>>> +
> >>>>>> +static void intel_ltr_hide(struct device *dev)
> >>>>>> +{
> >>>>>> +	dev_pm_qos_hide_latency_tolerance(dev);
> >>>>>> +	dev->power.set_latency_tolerance = NULL;
> >>>>>> +}
> >>>>>> +
> >>>>>> +static struct dentry *intel_actualize_debugfs_root(bool add)
> >>>>>> +{
> >>>>>> +	static struct dentry *debugfs_root;
> >>>>>> +	static DEFINE_MUTEX(lock);
> >>>>>> +	static int ref_cnt;
> >>>>>> +
> >>>>>> +	guard(mutex)(&lock);
> >>>>>> +
> >>>>>> +	ref_cnt += add ? 1 : -1;
> >>>>>> +
> >>>>>> +	if (ref_cnt) {
> >>>>>> +		if (IS_ERR_OR_NULL(debugfs_root))
> >>>>>> +			debugfs_root = debugfs_create_dir("intel_i3c", NULL);
> >>>>>
> >>>>> I think it'd better to create common "i3c" debugfs root. So your device
> >>>>> use debugfs_create_dir(dev_name(&hci->pci->dev), ...) under this common
> >>>>> root.
> >>>>
> >>>> The debugfs files are being created for the PCI device to contain PCI
> >>>> LTR information.  The i3c module has nothing to do with that.
> >>>>
> >>>> Also, currently, there is no module dependency between this PCI driver
> >>>> and i3c, but a dependency would be necessary to coordinate the creation
> >>>> and deletion of an 'i3c' debugfs directory.
> >>>>
> >>>> The extra complexity can't be justified.
> >>>
> >>> Don't post new versin before we close open discussion. I see your post
> >>> patch before this message.
> >>
> >> Sorry, but it is late in the release cycle, and it saves time when we
> >> are in different time zones.
> >>
> >>>
> >>> So, I think not necessary to create to root, which cause complex, (need
> >>> lock, and refer counter).
> >>
> >> So you are OK with V3?
> >
> > In v3, still create "intel_i3c" root dir, how about direct create dev
> > under root
> >
> > 	debugfs_create_dir(dev_name(&hci->pci->dev), NULL);
> >
> > to reduce unneccesary complex, (like mutex lock and ref counter).
>
> Could do.  I added intel_i3c to match intel_lpss which is also
> PCI devices with LTR e.g.

You can use similar method to create root at module_init.

static int __init intel_lpss_init(void)
{
        intel_lpss_debugfs = debugfs_create_dir("intel_lpss", NULL);
        return 0;
}
module_init(intel_lpss_init);

Frank

>
> # tree /sys/kernel/debug/intel_lpss
> /sys/kernel/debug/intel_lpss
> |-- 0000:00:19.0
> |   |-- active_ltr
> |   |-- capabilities
> |   `-- idle_ltr
> `-- 0000:00:19.1
>     |-- active_ltr
>     |-- capabilities
>     `-- idle_ltr
>
> 3 directories, 6 files
> # tree /sys/kernel/debug/intel_i3c
> /sys/kernel/debug/intel_i3c
> |-- 0000:00:11.0
> |   |-- active_ltr
> |   `-- idle_ltr
> `-- 0000:00:11.1
>     |-- active_ltr
>     `-- idle_ltr
>
> 3 directories, 4 files
> #
>
>
> >
> > Frank
> >
> >>
> >>>
> >>> Frank
> >>>
> >>>>
> >>>>>
> >>>>> ...
> >>>>>
> >>>>>> +
> >>>>>>  static const struct mipi_i3c_hci_pci_info intel_info = {
> >>>>>>  	.init = intel_i3c_init,
> >>>>>> +	.exit = intel_exit,
> >>>>>>  };
> >>>>>>
> >>>>>>  static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
> >>>>>> --
> >>>>>> 2.51.0
> >>>>>>
> >>>>>>
> >>>>>> --
> >>>>>> linux-i3c mailing list
> >>>>>> linux-i3c@lists.infradead.org
> >>>>>> http://lists.infradead.org/mailman/listinfo/linux-i3c
> >>>>
> >>>>
> >>>> --
> >>>> linux-i3c mailing list
> >>>> linux-i3c@lists.infradead.org
> >>>> http://lists.infradead.org/mailman/listinfo/linux-i3c
> >>
> >>
> >> --
> >> linux-i3c mailing list
> >> linux-i3c@lists.infradead.org
> >> http://lists.infradead.org/mailman/listinfo/linux-i3c
>
>
> --
> linux-i3c mailing list
> linux-i3c@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c