[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <b0b09073-f64f-4a2c-9294-713d7970f120@quicinc.com>
Date: Mon, 21 Apr 2025 11:47:43 +0530
From: Vivek Pernamitta <quic_vpernami@...cinc.com>
To: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
CC: Veerabhadrarao Badiganti <quic_vbadigan@...cinc.com>,
<mhi@...ts.linux.dev>, <linux-arm-msm@...r.kernel.org>,
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] bus: mhi: host: pci: Disable runtime PM for QDU100
On 4/18/2025 6:15 PM, Manivannan Sadhasivam wrote:
> On Fri, Apr 18, 2025 at 03:40:48PM +0530, Vivek Pernamitta wrote:
>>
>>
>> On 4/18/2025 3:04 PM, Vivek Pernamitta wrote:
>>>
>>>
>>> On 4/18/2025 2:06 PM, Manivannan Sadhasivam wrote:
>>>> On Fri, Apr 18, 2025 at 11:55:24AM +0530, Vivek Pernamitta wrote:
>>>>>
>>>>>
>>>>> On 4/17/2025 11:37 AM, Manivannan Sadhasivam wrote:
>>>>>> On Thu, Apr 17, 2025 at 10:00:38AM +0530, Veerabhadrarao
>>>>>> Badiganti wrote:
>>>>>>>
>>>>>>> On 4/14/2025 1:17 PM, Vivek Pernamitta wrote:
>>>>>>>> The QDU100 device does not support the MHI M3 state,
>>>>>>>> necessitating the
>>>>>>>> disabling of runtime PM for this device. Since the
>>>>>>>> PCIe core framework
>>>>>>>> enables runtime PM by default for all clients, it is
>>>>>>>> essential to disable
>>>>>>>> runtime PM if the device does not support Low Power Mode (LPM).
>>>>>>>>
>>>>
>>>> Not true... See below.
>>>>
>>>>>>>> Signed-off-by: Vivek Pernamitta<quic_vpernami@...cinc.com>
>>>>>>>> ---
>>>>>>>> drivers/bus/mhi/host/pci_generic.c | 10 ++++++++++
>>>>>>>> 1 file changed, 10 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/drivers/bus/mhi/host/pci_generic.c
>>>>>>>> b/drivers/bus/mhi/ host/pci_generic.c
>>>>>>>> index 03aa887952098661a488650053a357f883d1559b..a011fd2d48c57cf9d1aec74040153267a206d797
>>>>>>>> 100644
>>>>>>>> --- a/drivers/bus/mhi/host/pci_generic.c
>>>>>>>> +++ b/drivers/bus/mhi/host/pci_generic.c
>>>>>>>> @@ -43,6 +43,7 @@
>>>>>>>> * @mru_default: default MRU size for MBIM network packets
>>>>>>>> * @sideband_wake: Devices using dedicated
>>>>>>>> sideband GPIO for wakeup instead
>>>>>>>> * of inband wake support (such as sdx24)
>>>>>>>> + * @pm_disable: disables runtime PM (optional)
>>>>>>>> */
>>>>>>>> struct mhi_pci_dev_info {
>>>>>>>> const struct mhi_controller_config *config;
>>>>>>>> @@ -54,6 +55,7 @@ struct mhi_pci_dev_info {
>>>>>>>> unsigned int dma_data_width;
>>>>>>>> unsigned int mru_default;
>>>>>>>> bool sideband_wake;
>>>>>>>> + bool pm_disable;
>>>>>>>> };
>>>>>>>> #define MHI_CHANNEL_CONFIG_UL(ch_num, ch_name,
>>>>>>>> el_count, ev_ring) \
>>>>>>>> @@ -295,6 +297,7 @@ static const struct
>>>>>>>> mhi_pci_dev_info mhi_qcom_qdu100_info = {
>>>>>>>> .bar_num = MHI_PCI_DEFAULT_BAR_NUM,
>>>>>>>> .dma_data_width = 32,
>>>>>>>> .sideband_wake = false,
>>>>>>>> + .pm_disable = true,
>>>>>>>
>>>>>>> |no_m3|orno_|m3_support|would be more suitable than|pm_disable|
>>>>>>
>>>>>> Yes!
>>>>>>
>> We named variable pm_disable because the M3 state is implicitly set during
>> runtime PM suspend. Both are not needed and not fully supported for the
>> QDU100 accelerator card, so we want to disable runtime suspend for it.
>
> You are mixing terms here. Only M3 is not supported by the device. The fact that
> you do not want to enable runtime PM for the device to avoid latency is a side
> effect of that. So you need to name the variable as 'no_m3'.
sure, I have posted an new patchset v3 with renaming the variable name
to "no_m3".
>
> - Mani
>
Powered by blists - more mailing lists