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

[Adrian Hunter]

On 17/11/2025 20:56, Frank Li wrote:
> 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 at 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);

mipi-i3c-hci-pci is not an Intel driver.  It can be used for any
PCI device that supports MIPI I3C.

At module init, the driver does not know whether there are any
Intel PCI devices, so it does not know whether to create "intel_i3c".
Nor does it know whether other (non-Intel) devices need debugfs.

It would be preferable to use either the current approach, or as
you already suggested debugfs_create_dir(dev_name(&hci->pci->dev), NULL)