[RESEND PATCH v2 0/3] Standardize link status check to return bool

Hans Zhang posted 3 patches 7 months, 1 week ago
drivers/pci/controller/cadence/pci-j721e.c             | 6 +-----
drivers/pci/controller/dwc/pci-dra7xx.c                | 4 ++--
drivers/pci/controller/dwc/pci-exynos.c                | 4 ++--
drivers/pci/controller/dwc/pci-keystone.c              | 5 ++---
drivers/pci/controller/dwc/pci-meson.c                 | 6 +++---
drivers/pci/controller/dwc/pcie-armada8k.c             | 6 +++---
drivers/pci/controller/dwc/pcie-designware.c           | 2 +-
drivers/pci/controller/dwc/pcie-designware.h           | 4 ++--
drivers/pci/controller/dwc/pcie-dw-rockchip.c          | 2 +-
drivers/pci/controller/dwc/pcie-histb.c                | 9 +++------
drivers/pci/controller/dwc/pcie-keembay.c              | 2 +-
drivers/pci/controller/dwc/pcie-kirin.c                | 7 ++-----
drivers/pci/controller/dwc/pcie-qcom-ep.c              | 2 +-
drivers/pci/controller/dwc/pcie-qcom.c                 | 4 ++--
drivers/pci/controller/dwc/pcie-rcar-gen4.c            | 2 +-
drivers/pci/controller/dwc/pcie-spear13xx.c            | 7 ++-----
drivers/pci/controller/dwc/pcie-tegra194.c             | 4 ++--
drivers/pci/controller/dwc/pcie-uniphier.c             | 2 +-
drivers/pci/controller/dwc/pcie-visconti.c             | 4 ++--
drivers/pci/controller/mobiveil/pcie-layerscape-gen4.c | 9 ++-------
drivers/pci/controller/mobiveil/pcie-mobiveil.h        | 2 +-
21 files changed, 37 insertions(+), 56 deletions(-)
[RESEND PATCH v2 0/3] Standardize link status check to return bool
Posted by Hans Zhang 7 months, 1 week ago
1. PCI: dwc: Standardize link status check to return bool.
2. PCI: mobiveil: Refactor link status check.
3. PCI: cadence: Simplify j721e link status check.

---
Changes for RESEND:
- add Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Changes for v2:
- Remove the return of some functions (!!) .
- Patches 2/3 and 3/3 have not been modified.

Based on the following branch:
https://web.git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/log/?h=controller/dw-rockchip
---

Hans Zhang (3):
  PCI: dwc: Standardize link status check to return bool
  PCI: mobiveil: Refactor link status check
  PCI: cadence: Simplify j721e link status check

 drivers/pci/controller/cadence/pci-j721e.c             | 6 +-----
 drivers/pci/controller/dwc/pci-dra7xx.c                | 4 ++--
 drivers/pci/controller/dwc/pci-exynos.c                | 4 ++--
 drivers/pci/controller/dwc/pci-keystone.c              | 5 ++---
 drivers/pci/controller/dwc/pci-meson.c                 | 6 +++---
 drivers/pci/controller/dwc/pcie-armada8k.c             | 6 +++---
 drivers/pci/controller/dwc/pcie-designware.c           | 2 +-
 drivers/pci/controller/dwc/pcie-designware.h           | 4 ++--
 drivers/pci/controller/dwc/pcie-dw-rockchip.c          | 2 +-
 drivers/pci/controller/dwc/pcie-histb.c                | 9 +++------
 drivers/pci/controller/dwc/pcie-keembay.c              | 2 +-
 drivers/pci/controller/dwc/pcie-kirin.c                | 7 ++-----
 drivers/pci/controller/dwc/pcie-qcom-ep.c              | 2 +-
 drivers/pci/controller/dwc/pcie-qcom.c                 | 4 ++--
 drivers/pci/controller/dwc/pcie-rcar-gen4.c            | 2 +-
 drivers/pci/controller/dwc/pcie-spear13xx.c            | 7 ++-----
 drivers/pci/controller/dwc/pcie-tegra194.c             | 4 ++--
 drivers/pci/controller/dwc/pcie-uniphier.c             | 2 +-
 drivers/pci/controller/dwc/pcie-visconti.c             | 4 ++--
 drivers/pci/controller/mobiveil/pcie-layerscape-gen4.c | 9 ++-------
 drivers/pci/controller/mobiveil/pcie-mobiveil.h        | 2 +-
 21 files changed, 37 insertions(+), 56 deletions(-)


base-commit: 286ed198b899739862456f451eda884558526a9d
-- 
2.25.1
Re: [RESEND PATCH v2 0/3] Standardize link status check to return bool
Posted by Krzysztof Wilczyński 7 months, 1 week ago
> 1. PCI: dwc: Standardize link status check to return bool.
> 2. PCI: mobiveil: Refactor link status check.
> 3. PCI: cadence: Simplify j721e link status check.

Do not bother sending such cover letters.  This adds no value.

Please read the following:

  - https://www.kernel.org/doc/html/latest/process/submitting-patches.html
  - https://kernelnewbies.org/PatchSeries

> ---
> Changes for RESEND:
> - add Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Resending a patch is not a place to add new tags.

Thank you!

	Krzysztof
Re: [RESEND PATCH v2 0/3] Standardize link status check to return bool
Posted by Hans Zhang 7 months, 1 week ago

On 2025/5/13 18:21, Krzysztof Wilczyński wrote:
>> 1. PCI: dwc: Standardize link status check to return bool.
>> 2. PCI: mobiveil: Refactor link status check.
>> 3. PCI: cadence: Simplify j721e link status check.
> 
> Do not bother sending such cover letters.  This adds no value.
> 

Dear Krzysztof,

Thank you very much for your reply. In my future work, I will make 
improvements.



> Please read the following:
> 
>    - https://www.kernel.org/doc/html/latest/process/submitting-patches.html
>    - https://kernelnewbies.org/PatchSeries
> 
>> ---
>> Changes for RESEND:
>> - add Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> 
> Resending a patch is not a place to add new tags.
> 

Sorry, this is also the first time I have done this. For other patches 
in the future, I will do this in the new version.

> Thank you!
> 
> 	Krzysztof


Best regards,
Hans

Re: [RESEND PATCH v2 0/3] Standardize link status check to return bool
Posted by Krzysztof Wilczyński 7 months, 1 week ago
Hello,

[...]
> Sorry, this is also the first time I have done this. For other patches in
> the future, I will do this in the new version.

See the following:

  - https://lore.kernel.org/linux-pci/20250513145728.GA3513600@rocinante

While I cannot speak for other maintainers, I am going to change the
approach to the "RESEND" patches that I used to personally have.

But, if in doubt, it's always fine to send another version.

Thank you!

	Krzysztof
Re: [RESEND PATCH v2 0/3] Standardize link status check to return bool
Posted by Hans Zhang 7 months, 1 week ago

On 2025/5/13 23:04, Krzysztof Wilczyński wrote:
> Hello,
> 
> [...]
>> Sorry, this is also the first time I have done this. For other patches in
>> the future, I will do this in the new version.
> 
> See the following:
> 
>    - https://lore.kernel.org/linux-pci/20250513145728.GA3513600@rocinante
> 
> While I cannot speak for other maintainers, I am going to change the
> approach to the "RESEND" patches that I used to personally have.
> 
> But, if in doubt, it's always fine to send another version.
> 


Dear Krzysztof,

Ok. From now on, I will handle similar problems in the new version.

Best regards,
Hans

Re: [RESEND PATCH v2 0/3] Standardize link status check to return bool
Posted by Niklas Cassel 7 months, 1 week ago

On 13 May 2025 17:09:58 CEST, Hans Zhang <18255117159@163.com> wrote:
>
>
>On 2025/5/13 23:04, Krzysztof Wilczyński wrote:
>> Hello,
>> 
>> [...]
>>> Sorry, this is also the first time I have done this. For other patches in
>>> the future, I will do this in the new version.
>> 
>> See the following:
>> 
>>    - https://lore.kernel.org/linux-pci/20250513145728.GA3513600@rocinante
>> 
>> While I cannot speak for other maintainers, I am going to change the
>> approach to the "RESEND" patches that I used to personally have.
>> 
>> But, if in doubt, it's always fine to send another version.
>> 
>
>
>Dear Krzysztof,
>
>Ok. From now on, I will handle similar problems in the new version.

While I agree that it is always fine to send a new revision (which has picked up tags),
having the RESEND tag is gently informing that the series might have fell thorough the cracks.

This information might be lost/less obvious if simply sending a new revision (assuming that commit log and code is unchanged).


Kind regards,
Niklas
Re: [RESEND PATCH v2 0/3] Standardize link status check to return bool
Posted by Niklas Cassel 7 months, 1 week ago
Hello Krzysztof,

On Tue, May 13, 2025 at 07:21:15PM +0900, Krzysztof Wilczyński wrote:
> > ---
> > Changes for RESEND:
> > - add Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> 
> Resending a patch is not a place to add new tags.

While I realize that:
https://docs.kernel.org/process/submitting-patches.html#don-t-get-discouraged-or-impatient

states:
"""
"RESEND" only applies to resubmission of a patch or patch series which
have not been modified in any way from the previous submission.
"""

I would assume that this only refers to the commit log and code,
and that picking up tags has to be an acceptable exception.

If I take myself as an example, I would not be happy if I spent time
reviewing a large patch series, but because the maintainers somehow
missed that series, so the patch author has to RESEND it (without
picking up tags), my Reviewed-by tags get lost.


Kind regards,
Niklas
Re: [RESEND PATCH v2 0/3] Standardize link status check to return bool
Posted by Krzysztof Wilczyński 7 months, 1 week ago
Hello,

> > > Changes for RESEND:
> > > - add Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > 
> > Resending a patch is not a place to add new tags.
> 
> While I realize that:
> https://docs.kernel.org/process/submitting-patches.html#don-t-get-discouraged-or-impatient
> 
> states:
> """
> "RESEND" only applies to resubmission of a patch or patch series which
> have not been modified in any way from the previous submission.
> """

Yes, I would often take verbiage of this document verbatim, but..

> I would assume that this only refers to the commit log and code,
> and that picking up tags has to be an acceptable exception.

The above comment prompted me to inquire with a more senior maintainers,
purely as I was curious what the opinions/preferences would be. And, as
such, the replies I've got were:

  - No, follow the documentation
  - I don't care, really
  - It's OK, make sure to pick the tag, if it makes sense

So, wide spectrum of answers.  As such, I take it as, "it's up to the
maintainer", for lack of less ambiguous answers.

> If I take myself as an example, I would not be happy if I spent time
> reviewing a large patch series, but because the maintainers somehow
> missed that series, so the patch author has to RESEND it (without
> picking up tags), my Reviewed-by tags get lost.

While it's not about making you happy, I agree, that trying to preserve the
tag might be the correct approach here.  As such, I will adopt this approach,
whereas with other it might vary.

Thank you!

	Krzysztof
Re: [RESEND PATCH v2 0/3] Standardize link status check to return bool
Posted by Manivannan Sadhasivam 7 months, 1 week ago
On Sun, May 11, 2025 at 12:07:07AM +0800, Hans Zhang wrote:
> 1. PCI: dwc: Standardize link status check to return bool.
> 2. PCI: mobiveil: Refactor link status check.
> 3. PCI: cadence: Simplify j721e link status check.

Please do not paste the patch subjects in cover letter. Cover letter should
elaborate the issue this series is fixing, its purpose, any dependency etc...

- Mani

> 
> ---
> Changes for RESEND:
> - add Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> 
> Changes for v2:
> - Remove the return of some functions (!!) .
> - Patches 2/3 and 3/3 have not been modified.
> 
> Based on the following branch:
> https://web.git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/log/?h=controller/dw-rockchip
> ---
> 
> Hans Zhang (3):
>   PCI: dwc: Standardize link status check to return bool
>   PCI: mobiveil: Refactor link status check
>   PCI: cadence: Simplify j721e link status check
> 
>  drivers/pci/controller/cadence/pci-j721e.c             | 6 +-----
>  drivers/pci/controller/dwc/pci-dra7xx.c                | 4 ++--
>  drivers/pci/controller/dwc/pci-exynos.c                | 4 ++--
>  drivers/pci/controller/dwc/pci-keystone.c              | 5 ++---
>  drivers/pci/controller/dwc/pci-meson.c                 | 6 +++---
>  drivers/pci/controller/dwc/pcie-armada8k.c             | 6 +++---
>  drivers/pci/controller/dwc/pcie-designware.c           | 2 +-
>  drivers/pci/controller/dwc/pcie-designware.h           | 4 ++--
>  drivers/pci/controller/dwc/pcie-dw-rockchip.c          | 2 +-
>  drivers/pci/controller/dwc/pcie-histb.c                | 9 +++------
>  drivers/pci/controller/dwc/pcie-keembay.c              | 2 +-
>  drivers/pci/controller/dwc/pcie-kirin.c                | 7 ++-----
>  drivers/pci/controller/dwc/pcie-qcom-ep.c              | 2 +-
>  drivers/pci/controller/dwc/pcie-qcom.c                 | 4 ++--
>  drivers/pci/controller/dwc/pcie-rcar-gen4.c            | 2 +-
>  drivers/pci/controller/dwc/pcie-spear13xx.c            | 7 ++-----
>  drivers/pci/controller/dwc/pcie-tegra194.c             | 4 ++--
>  drivers/pci/controller/dwc/pcie-uniphier.c             | 2 +-
>  drivers/pci/controller/dwc/pcie-visconti.c             | 4 ++--
>  drivers/pci/controller/mobiveil/pcie-layerscape-gen4.c | 9 ++-------
>  drivers/pci/controller/mobiveil/pcie-mobiveil.h        | 2 +-
>  21 files changed, 37 insertions(+), 56 deletions(-)
> 
> 
> base-commit: 286ed198b899739862456f451eda884558526a9d
> -- 
> 2.25.1
> 

-- 
மணிவண்ணன் சதாசிவம்
Re: [RESEND PATCH v2 0/3] Standardize link status check to return bool
Posted by Hans Zhang 7 months, 1 week ago

On 2025/5/13 17:40, Manivannan Sadhasivam wrote:
> On Sun, May 11, 2025 at 12:07:07AM +0800, Hans Zhang wrote:
>> 1. PCI: dwc: Standardize link status check to return bool.
>> 2. PCI: mobiveil: Refactor link status check.
>> 3. PCI: cadence: Simplify j721e link status check.
> 
> Please do not paste the patch subjects in cover letter. Cover letter should
> elaborate the issue this series is fixing, its purpose, any dependency etc...
> 

Dear Mani,

Thank you very much for your reply and reminder. In future submissions, 
I will pay attention to this point.

Best regards,
Hans

> - Mani
> 
>>
>> ---
>> Changes for RESEND:
>> - add Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>>
>> Changes for v2:
>> - Remove the return of some functions (!!) .
>> - Patches 2/3 and 3/3 have not been modified.
>>
>> Based on the following branch:
>> https://web.git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/log/?h=controller/dw-rockchip
>> ---
>>
>> Hans Zhang (3):
>>    PCI: dwc: Standardize link status check to return bool
>>    PCI: mobiveil: Refactor link status check
>>    PCI: cadence: Simplify j721e link status check
>>
>>   drivers/pci/controller/cadence/pci-j721e.c             | 6 +-----
>>   drivers/pci/controller/dwc/pci-dra7xx.c                | 4 ++--
>>   drivers/pci/controller/dwc/pci-exynos.c                | 4 ++--
>>   drivers/pci/controller/dwc/pci-keystone.c              | 5 ++---
>>   drivers/pci/controller/dwc/pci-meson.c                 | 6 +++---
>>   drivers/pci/controller/dwc/pcie-armada8k.c             | 6 +++---
>>   drivers/pci/controller/dwc/pcie-designware.c           | 2 +-
>>   drivers/pci/controller/dwc/pcie-designware.h           | 4 ++--
>>   drivers/pci/controller/dwc/pcie-dw-rockchip.c          | 2 +-
>>   drivers/pci/controller/dwc/pcie-histb.c                | 9 +++------
>>   drivers/pci/controller/dwc/pcie-keembay.c              | 2 +-
>>   drivers/pci/controller/dwc/pcie-kirin.c                | 7 ++-----
>>   drivers/pci/controller/dwc/pcie-qcom-ep.c              | 2 +-
>>   drivers/pci/controller/dwc/pcie-qcom.c                 | 4 ++--
>>   drivers/pci/controller/dwc/pcie-rcar-gen4.c            | 2 +-
>>   drivers/pci/controller/dwc/pcie-spear13xx.c            | 7 ++-----
>>   drivers/pci/controller/dwc/pcie-tegra194.c             | 4 ++--
>>   drivers/pci/controller/dwc/pcie-uniphier.c             | 2 +-
>>   drivers/pci/controller/dwc/pcie-visconti.c             | 4 ++--
>>   drivers/pci/controller/mobiveil/pcie-layerscape-gen4.c | 9 ++-------
>>   drivers/pci/controller/mobiveil/pcie-mobiveil.h        | 2 +-
>>   21 files changed, 37 insertions(+), 56 deletions(-)
>>
>>
>> base-commit: 286ed198b899739862456f451eda884558526a9d
>> -- 
>> 2.25.1
>>
>
Re: [RESEND PATCH v2 0/3] Standardize link status check to return bool
Posted by Manivannan Sadhasivam 7 months, 1 week ago
On Sun, 11 May 2025 00:07:07 +0800, Hans Zhang wrote:
> 1. PCI: dwc: Standardize link status check to return bool.
> 2. PCI: mobiveil: Refactor link status check.
> 3. PCI: cadence: Simplify j721e link status check.
> 

Applied, thanks!

[1/3] PCI: dwc: Standardize link status check to return bool
      commit: f46bfb1d3c6a601caad90eb3c11a1e1e17cccb1a
[2/3] PCI: mobiveil: Refactor link status check
      commit: 0a9d6a3d0fd1650b9ee00bc8150828e19cadaf23
[3/3] PCI: cadence: Simplify j721e link status check
      commit: 1a176b25f5d6f00c6c44729c006379b9a6dbc703

Best regards,
-- 
Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Re: [RESEND PATCH v2 0/3] Standardize link status check to return bool
Posted by Niklas Cassel 7 months, 1 week ago
Hello Mani,

On Tue, May 13, 2025 at 10:33:59AM +0100, Manivannan Sadhasivam wrote:
> 
> On Sun, 11 May 2025 00:07:07 +0800, Hans Zhang wrote:
> > 1. PCI: dwc: Standardize link status check to return bool.
> > 2. PCI: mobiveil: Refactor link status check.
> > 3. PCI: cadence: Simplify j721e link status check.
> > 
> 
> Applied, thanks!
> 
> [1/3] PCI: dwc: Standardize link status check to return bool
>       commit: f46bfb1d3c6a601caad90eb3c11a1e1e17cccb1a
> [2/3] PCI: mobiveil: Refactor link status check
>       commit: 0a9d6a3d0fd1650b9ee00bc8150828e19cadaf23
> [3/3] PCI: cadence: Simplify j721e link status check
>       commit: 1a176b25f5d6f00c6c44729c006379b9a6dbc703
> 

This was all applied to the dw-rockchip branch.

Was that intentional?

My guess is that perhaps you thought that
"PCI: dwc: Standardize link status check to return bool"
was going to conflict with Hans's other commit:
5e5a3bf48eed ("PCI: dw-rockchip: Use rockchip_pcie_link_up() to check link
up instead of open coding")

but at least from looking at the diff, they don't seem to touch the same
lines, but perhaps you got a conflict anyway?



mobiveil and cadence patches seem unrelated to dw-rockchip
(unrelated to DWC even).

If it was intentional, all is good, but perhaps the branch
should have a more generic name, rather than dw-rockchip,
especially now when the reset-slot and qcom-reset slot patches
are also on the same branch.


Kind regards,
Niklas
Re: [RESEND PATCH v2 0/3] Standardize link status check to return bool
Posted by Manivannan Sadhasivam 7 months, 1 week ago
On Fri, May 16, 2025 at 10:52:17AM +0200, Niklas Cassel wrote:
> Hello Mani,
> 
> On Tue, May 13, 2025 at 10:33:59AM +0100, Manivannan Sadhasivam wrote:
> > 
> > On Sun, 11 May 2025 00:07:07 +0800, Hans Zhang wrote:
> > > 1. PCI: dwc: Standardize link status check to return bool.
> > > 2. PCI: mobiveil: Refactor link status check.
> > > 3. PCI: cadence: Simplify j721e link status check.
> > > 
> > 
> > Applied, thanks!
> > 
> > [1/3] PCI: dwc: Standardize link status check to return bool
> >       commit: f46bfb1d3c6a601caad90eb3c11a1e1e17cccb1a
> > [2/3] PCI: mobiveil: Refactor link status check
> >       commit: 0a9d6a3d0fd1650b9ee00bc8150828e19cadaf23
> > [3/3] PCI: cadence: Simplify j721e link status check
> >       commit: 1a176b25f5d6f00c6c44729c006379b9a6dbc703
> > 
> 
> This was all applied to the dw-rockchip branch.
> 
> Was that intentional?

Yes it was.

> 
> My guess is that perhaps you thought that
> "PCI: dwc: Standardize link status check to return bool"
> was going to conflict with Hans's other commit:
> 5e5a3bf48eed ("PCI: dw-rockchip: Use rockchip_pcie_link_up() to check link
> up instead of open coding")
> 
> but at least from looking at the diff, they don't seem to touch the same
> lines, but perhaps you got a conflict anyway?
> 

I think I got a conflict and I saw that the cover letter mentioned dw-rockchip
as a dependency, so I applied to that branch.

> 
> 
> mobiveil and cadence patches seem unrelated to dw-rockchip
> (unrelated to DWC even).
> 
> If it was intentional, all is good, but perhaps the branch
> should have a more generic name, rather than dw-rockchip,
> especially now when the reset-slot and qcom-reset slot patches
> are also on the same branch.
> 

Yeah, I agree. Since there are 3 series on this branch, we need to pick a smart
name ;) I will do so. Thanks!

- Mani

-- 
மணிவண்ணன் சதாசிவம்