| Message ID | 20241218102707.76272-1-quic_prashk@quicinc.com |
|---|---|
| Headers | show |
| Series | Disable USB U1/U2 entry for QC targets | expand |
On Wed, Dec 18, 2024 at 03:56:48PM +0530, Prashanth K wrote: > Enabling U1 and U2 power-saving states can lead to stability and > performance issues, particularly for latency-sensitive or high- > throughput applications. These low-power link states are intended > to reduce power consumption by allowing the device to enter partial > low-power modes during idle periods. However, they can sometimes > result in unexpected behavior. Over the years, some of the issues > seen are as follows: > [..] > > This series was earlier started by Krishna Kurapati where he disabled > U1/U2 on some SM targets. I'm extending this to more devices including > Auto, Compute and IOT platforms. On a side note, this quirk has been > already included on some mobile targets like SM8550/8650. Why are you resending previous patches rather than adding another series on top of it? > > Changes in v2: > - Removed the wrongly added quirks from tcsr_mutex node. > - Link to v2: https://lore.kernel.org/all/20241213095237.1409174-1-quic_prashk@quicinc.com/ What was changed in v3? > > Link to RFC: > https://lore.kernel.org/all/20241107073650.13473-1-quic_kriskura@quicinc.com/#Z31arch:arm64:boot:dts:qcom:sm8250.dtsi >
On 18-12-24 04:57 pm, Dmitry Baryshkov wrote: > On Wed, Dec 18, 2024 at 03:56:48PM +0530, Prashanth K wrote: >> Enabling U1 and U2 power-saving states can lead to stability and >> performance issues, particularly for latency-sensitive or high- >> throughput applications. These low-power link states are intended >> to reduce power consumption by allowing the device to enter partial >> low-power modes during idle periods. However, they can sometimes >> result in unexpected behavior. Over the years, some of the issues >> seen are as follows: >> > > [..] > >> >> This series was earlier started by Krishna Kurapati where he disabled >> U1/U2 on some SM targets. I'm extending this to more devices including >> Auto, Compute and IOT platforms. On a side note, this quirk has been >> already included on some mobile targets like SM8550/8650. > > Why are you resending previous patches rather than adding another series > on top of it? > Hi Dmitry, RFC had only one patch with quirks (to disable u1/u2) only for few targets (SM8150, 8250, 8350, 8450). It was later decided to split it into per-file commits as per the review comments. Hence I clubbed Krishna's changes along with few more targets. Let me know if this needs to be changed. >> >> Changes in v2: >> - Removed the wrongly added quirks from tcsr_mutex node. >> - Link to v2: https://lore.kernel.org/all/20241213095237.1409174-1-quic_prashk@quicinc.com/ > > What was changed in v3? It was supposed to be "Changes in v3" instead of v2. > >> >> Link to RFC: >> https://lore.kernel.org/all/20241107073650.13473-1-quic_kriskura@quicinc.com/#Z31arch:arm64:boot:dts:qcom:sm8250.dtsi >> > Regards, Prashanth K
On Wed, Dec 18, 2024 at 05:18:50PM +0530, Prashanth K wrote: > > > On 18-12-24 04:57 pm, Dmitry Baryshkov wrote: > > On Wed, Dec 18, 2024 at 03:56:48PM +0530, Prashanth K wrote: > >> Enabling U1 and U2 power-saving states can lead to stability and > >> performance issues, particularly for latency-sensitive or high- > >> throughput applications. These low-power link states are intended > >> to reduce power consumption by allowing the device to enter partial > >> low-power modes during idle periods. However, they can sometimes > >> result in unexpected behavior. Over the years, some of the issues > >> seen are as follows: > >> > > > > [..] > > > >> > >> This series was earlier started by Krishna Kurapati where he disabled > >> U1/U2 on some SM targets. I'm extending this to more devices including > >> Auto, Compute and IOT platforms. On a side note, this quirk has been > >> already included on some mobile targets like SM8550/8650. > > > > Why are you resending previous patches rather than adding another series > > on top of it? > > > Hi Dmitry, > > RFC had only one patch with quirks (to disable u1/u2) only for few > targets (SM8150, 8250, 8350, 8450). It was later decided to split it > into per-file commits as per the review comments. Hence I clubbed > Krishna's changes along with few more targets. Let me know if this needs > to be changed. No, it's fine. The text in the commit message lead me to a wrong conclusion. > > >> > >> Changes in v2: > >> - Removed the wrongly added quirks from tcsr_mutex node. > >> - Link to v2: https://lore.kernel.org/all/20241213095237.1409174-1-quic_prashk@quicinc.com/ > > > > What was changed in v3? > It was supposed to be "Changes in v3" instead of v2. Then where is a changelog between RFC and v2? Please consider switching to the b4 tool, it handles such issues for you. > > > >> > >> Link to RFC: > >> https://lore.kernel.org/all/20241107073650.13473-1-quic_kriskura@quicinc.com/#Z31arch:arm64:boot:dts:qcom:sm8250.dtsi > >> > > > Regards, > Prashanth K >
On 19-12-24 08:37 am, Dmitry Baryshkov wrote: > On Wed, Dec 18, 2024 at 05:18:50PM +0530, Prashanth K wrote: >> >> >> On 18-12-24 04:57 pm, Dmitry Baryshkov wrote: >>> On Wed, Dec 18, 2024 at 03:56:48PM +0530, Prashanth K wrote: >>>> Enabling U1 and U2 power-saving states can lead to stability and >>>> performance issues, particularly for latency-sensitive or high- >>>> throughput applications. These low-power link states are intended >>>> to reduce power consumption by allowing the device to enter partial >>>> low-power modes during idle periods. However, they can sometimes >>>> result in unexpected behavior. Over the years, some of the issues >>>> seen are as follows: >>>> >>> >>> [..] >>> >>>> >>>> This series was earlier started by Krishna Kurapati where he disabled >>>> U1/U2 on some SM targets. I'm extending this to more devices including >>>> Auto, Compute and IOT platforms. On a side note, this quirk has been >>>> already included on some mobile targets like SM8550/8650. >>> >>> Why are you resending previous patches rather than adding another series >>> on top of it? >>> >> Hi Dmitry, >> >> RFC had only one patch with quirks (to disable u1/u2) only for few >> targets (SM8150, 8250, 8350, 8450). It was later decided to split it >> into per-file commits as per the review comments. Hence I clubbed >> Krishna's changes along with few more targets. Let me know if this needs >> to be changed. > > No, it's fine. The text in the commit message lead me to a wrong > conclusion. > Ok sure. >> >>>> >>>> Changes in v2: >>>> - Removed the wrongly added quirks from tcsr_mutex node. >>>> - Link to v2: https://lore.kernel.org/all/20241213095237.1409174-1-quic_prashk@quicinc.com/ >>> >>> What was changed in v3? >> It was supposed to be "Changes in v3" instead of v2. > > Then where is a changelog between RFC and v2? > > Please consider switching to the b4 tool, it handles such issues for > you. > Ok, Should I send a new version updating the cover letter? >>> >>>> >>>> Link to RFC: >>>> https://lore.kernel.org/all/20241107073650.13473-1-quic_kriskura@quicinc.com/#Z31arch:arm64:boot:dts:qcom:sm8250.dtsi >>>> >>> >> Regards, >> Prashanth K >> >
On Thu, Dec 19, 2024 at 09:47:12AM +0530, Prashanth K wrote: > > > On 19-12-24 08:37 am, Dmitry Baryshkov wrote: > > On Wed, Dec 18, 2024 at 05:18:50PM +0530, Prashanth K wrote: > >> > >> > >> On 18-12-24 04:57 pm, Dmitry Baryshkov wrote: > >>> On Wed, Dec 18, 2024 at 03:56:48PM +0530, Prashanth K wrote: > >>>> Enabling U1 and U2 power-saving states can lead to stability and > >>>> performance issues, particularly for latency-sensitive or high- > >>>> throughput applications. These low-power link states are intended > >>>> to reduce power consumption by allowing the device to enter partial > >>>> low-power modes during idle periods. However, they can sometimes > >>>> result in unexpected behavior. Over the years, some of the issues > >>>> seen are as follows: > >>>> > >>> > >>> [..] > >>> > >>>> > >>>> This series was earlier started by Krishna Kurapati where he disabled > >>>> U1/U2 on some SM targets. I'm extending this to more devices including > >>>> Auto, Compute and IOT platforms. On a side note, this quirk has been > >>>> already included on some mobile targets like SM8550/8650. > >>> > >>> Why are you resending previous patches rather than adding another series > >>> on top of it? > >>> > >> Hi Dmitry, > >> > >> RFC had only one patch with quirks (to disable u1/u2) only for few > >> targets (SM8150, 8250, 8350, 8450). It was later decided to split it > >> into per-file commits as per the review comments. Hence I clubbed > >> Krishna's changes along with few more targets. Let me know if this needs > >> to be changed. > > > > No, it's fine. The text in the commit message lead me to a wrong > > conclusion. > > > Ok sure. > >> > >>>> > >>>> Changes in v2: > >>>> - Removed the wrongly added quirks from tcsr_mutex node. > >>>> - Link to v2: https://lore.kernel.org/all/20241213095237.1409174-1-quic_prashk@quicinc.com/ > >>> > >>> What was changed in v3? > >> It was supposed to be "Changes in v3" instead of v2. > > > > Then where is a changelog between RFC and v2? > > > > Please consider switching to the b4 tool, it handles such issues for > > you. > > > Ok, Should I send a new version updating the cover letter? For now you can provide data in the reply. Just make sure to include it in the cover letter if the patchset gets reposted. > >>> > >>>> > >>>> Link to RFC: > >>>> https://lore.kernel.org/all/20241107073650.13473-1-quic_kriskura@quicinc.com/#Z31arch:arm64:boot:dts:qcom:sm8250.dtsi > >>>> > >>> > >> Regards, > >> Prashanth K > >> > > >
On 18.12.2024 11:26 AM, Prashanth K wrote: > Enabling U1 and U2 power-saving states can lead to stability and > performance issues, particularly for latency-sensitive or high- > throughput applications. These low-power link states are intended > to reduce power consumption by allowing the device to enter partial > low-power modes during idle periods. However, they can sometimes > result in unexpected behavior. Over the years, some of the issues > seen are as follows: For the series: Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> Konrad