2022-08-22 13:44:42

by Richard Zhu

[permalink] [raw]
Subject: [PATCH v1 0/2] Fix the wrong order of phy callbacks

Refer [1], phy_init() must be called before phy_power_on().
This series used to fix the wrong order of the phy_init() and phy_power_on(),
introduced by commit 1aa97b002258 ("phy: freescale: pcie: Initialize the imx8 pcie standalone phy driver")
Tested on i.MX8MM EVK board when one NVME device is used.

[1]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/phy/phy-core.c?id=v5.19-rc1#n233

[PATCH v1 1/2] PCI: imx6: Fix the wrong order of phy_init() and
[PATCH v1 2/2] phy: freescale: imx8m-pcie: Fix the wrong order of


2022-08-22 13:56:12

by Richard Zhu

[permalink] [raw]
Subject: [PATCH v1 2/2] phy: freescale: imx8m-pcie: Fix the wrong order of phy_init() and phy_power_on()

Refer to phy_core driver, phy_init() must be called before phy_power_on().
Fix the wrong order of phy_init() and phy_power_on() here.

Fixes: 1aa97b002258 ("phy: freescale: pcie: Initialize the imx8 pcie standalone phy driver")
Signed-off-by: Richard Zhu <[email protected]>
---
drivers/phy/freescale/phy-fsl-imx8m-pcie.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
index ad7d2edfc414..c93286483b42 100644
--- a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
+++ b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
@@ -59,7 +59,7 @@ struct imx8_pcie_phy {
bool clkreq_unused;
};

-static int imx8_pcie_phy_init(struct phy *phy)
+static int imx8_pcie_phy_power_on(struct phy *phy)
{
int ret;
u32 val, pad_mode;
@@ -137,14 +137,14 @@ static int imx8_pcie_phy_init(struct phy *phy)
return ret;
}

-static int imx8_pcie_phy_power_on(struct phy *phy)
+static int imx8_pcie_phy_init(struct phy *phy)
{
struct imx8_pcie_phy *imx8_phy = phy_get_drvdata(phy);

return clk_prepare_enable(imx8_phy->clk);
}

-static int imx8_pcie_phy_power_off(struct phy *phy)
+static int imx8_pcie_phy_exit(struct phy *phy)
{
struct imx8_pcie_phy *imx8_phy = phy_get_drvdata(phy);

@@ -155,8 +155,8 @@ static int imx8_pcie_phy_power_off(struct phy *phy)

static const struct phy_ops imx8_pcie_phy_ops = {
.init = imx8_pcie_phy_init,
+ .exit = imx8_pcie_phy_exit,
.power_on = imx8_pcie_phy_power_on,
- .power_off = imx8_pcie_phy_power_off,
.owner = THIS_MODULE,
};

--
2.25.1

2022-08-26 06:35:14

by Alexander Stein

[permalink] [raw]
Subject: Re: [PATCH v1 0/2] Fix the wrong order of phy callbacks

Hello Richard,

Am Montag, 22. August 2022, 15:10:54 CEST schrieb Richard Zhu:
> Refer [1], phy_init() must be called before phy_power_on().
> This series used to fix the wrong order of the phy_init() and
> phy_power_on(), introduced by commit 1aa97b002258 ("phy: freescale: pcie:
> Initialize the imx8 pcie standalone phy driver") Tested on i.MX8MM EVK
> board when one NVME device is used.
>
> [1]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/d
> rivers/phy/phy-core.c?id=v5.19-rc1#n233
>
> [PATCH v1 1/2] PCI: imx6: Fix the wrong order of phy_init() and
> [PATCH v1 2/2] phy: freescale: imx8m-pcie: Fix the wrong order of

Together with your imx8mp patch series on TQMa8MPxl + MBa8MPxL:
Tested-by: Alexander Stein <[email protected]>

Thanks again!
Alexander



2022-08-26 07:13:39

by Richard Zhu

[permalink] [raw]
Subject: RE: [PATCH v1 0/2] Fix the wrong order of phy callbacks

> -----Original Message-----
> From: Alexander Stein <[email protected]>
> Sent: 2022??8??26?? 14:26
> To: Hongxing Zhu <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]; Marcel Ziswiler
> <[email protected]>; [email protected];
> [email protected]; Hongxing Zhu <[email protected]>;
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; dl-linux-imx <[email protected]>
> Subject: Re: [PATCH v1 0/2] Fix the wrong order of phy callbacks
>
> Hello Richard,
>
> Am Montag, 22. August 2022, 15:10:54 CEST schrieb Richard Zhu:
> > Refer [1], phy_init() must be called before phy_power_on().
> > This series used to fix the wrong order of the phy_init() and
> > phy_power_on(), introduced by commit 1aa97b002258 ("phy: freescale:
> pcie:
> > Initialize the imx8 pcie standalone phy driver") Tested on i.MX8MM EVK
> > board when one NVME device is used.
> >
> > [1]https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fg
> >
> it.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.gi
> >
> t%2Ftree%2Fd&amp;data=05%7C01%7Chongxing.zhu%40nxp.com%7C56c370
> 2db1e74
> >
> 67682ea08da872bde2f%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0
> %7C63797
> >
> 0919720713123%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLC
> JQIjoiV2lu
> >
> MzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=3jj
> vnOExs
> > uES8A%2F2kTxlYTuTglNSK3TLY2GAR8TmMiQ%3D&amp;reserved=0
> > rivers/phy/phy-core.c?id=v5.19-rc1#n233
> >
> > [PATCH v1 1/2] PCI: imx6: Fix the wrong order of phy_init() and [PATCH
> > v1 2/2] phy: freescale: imx8m-pcie: Fix the wrong order of
>
> Together with your imx8mp patch series on TQMa8MPxl + MBa8MPxL:
> Tested-by: Alexander Stein <[email protected]>
>
Hi Alexander:
It's great. Thanks for your kindly help.

Best Regards
Richard Zhu
> Thanks again!
> Alexander
>
>

2022-08-29 07:05:05

by Ahmad Fatoum

[permalink] [raw]
Subject: Re: [PATCH v1 0/2] Fix the wrong order of phy callbacks

Hi,

On 22.08.22 15:10, Richard Zhu wrote:
> Refer [1], phy_init() must be called before phy_power_on().
> This series used to fix the wrong order of the phy_init() and phy_power_on(),
> introduced by commit 1aa97b002258 ("phy: freescale: pcie: Initialize the imx8 pcie standalone phy driver")
> Tested on i.MX8MM EVK board when one NVME device is used.
>
> [1]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/phy/phy-core.c?id=v5.19-rc1#n233
>
> [PATCH v1 1/2] PCI: imx6: Fix the wrong order of phy_init() and
> [PATCH v1 2/2] phy: freescale: imx8m-pcie: Fix the wrong order of

This introduces an intermittent breakage. Can you squash?

>
>


--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |

2022-08-30 04:01:54

by Richard Zhu

[permalink] [raw]
Subject: RE: [PATCH v1 0/2] Fix the wrong order of phy callbacks


> -----Original Message-----
> From: Ahmad Fatoum <[email protected]>
> Sent: 2022??8??29?? 14:34
> To: Hongxing Zhu <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected]; Marcel
> Ziswiler <[email protected]>; [email protected]
> Cc: [email protected]; [email protected]; dl-linux-imx
> <[email protected]>; [email protected]; [email protected];
> [email protected]
> Subject: Re: [PATCH v1 0/2] Fix the wrong order of phy callbacks
>
> Hi,
>
> On 22.08.22 15:10, Richard Zhu wrote:
> > Refer [1], phy_init() must be called before phy_power_on().
> > This series used to fix the wrong order of the phy_init() and
> > phy_power_on(), introduced by commit 1aa97b002258 ("phy: freescale:
> > pcie: Initialize the imx8 pcie standalone phy driver") Tested on i.MX8MM EVK
> board when one NVME device is used.
> >
> > [1]https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fg
> >
> it.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.gi
> >
> t%2Ftree%2Fdrivers%2Fphy%2Fphy-core.c%3Fid%3Dv5.19-rc1%23n233&amp;
> data
> >
> =05%7C01%7Chongxing.zhu%40nxp.com%7C327761bac5b9407d977008da89
> 886b15%7
> >
> C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63797351625465698
> 5%7CUnkno
> >
> wn%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1ha
> WwiL
> >
> CJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=6SnkRdLAX2tLp1iGgE78d
> JAm%2BLOf
> > LLpANfR1e4xIYFY%3D&amp;reserved=0
> >
> > [PATCH v1 1/2] PCI: imx6: Fix the wrong order of phy_init() and [PATCH
> > v1 2/2] phy: freescale: imx8m-pcie: Fix the wrong order of
>
> This introduces an intermittent breakage. Can you squash?

Hi Ahmad:
Thanks for your comments.
Do you mean to squash this fix to the preview series?
I'm afraid that it's not easy to do that.
Because there are a lot of pci-imx6 code changes after
commit: 1aa97b002258 ("phy: freescale: pcie: Initialize the imx8 pcie standalone phy driver").

Best Regards
Richard Zhu
>
> >
> >
>
>
> --
> Pengutronix e.K. |
> |
> Steuerwalder Str. 21 |
> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.pe
> ngutronix.de%2F&amp;data=05%7C01%7Chongxing.zhu%40nxp.com%7C3277
> 61bac5b9407d977008da89886b15%7C686ea1d3bc2b4c6fa92cd99c5c30163
> 5%7C0%7C0%7C637973516254656985%7CUnknown%7CTWFpbGZsb3d8eyJ
> WIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7
> C3000%7C%7C%7C&amp;sdata=hasOtX77%2Fea5yLbV8C7wOfdZStF5erHOL58
> Zf6ZZbo4%3D&amp;reserved=0 |
> 31137 Hildesheim, Germany | Phone:
> +49-5121-206917-0 |
> Amtsgericht Hildesheim, HRA 2686 | Fax:
> +49-5121-206917-5555 |

2022-08-30 07:45:57

by Ahmad Fatoum

[permalink] [raw]
Subject: Re: [PATCH v1 0/2] Fix the wrong order of phy callbacks

Hello Richard,

On 30.08.22 05:47, Hongxing Zhu wrote:
>>> [PATCH v1 1/2] PCI: imx6: Fix the wrong order of phy_init() and [PATCH
>>> v1 2/2] phy: freescale: imx8m-pcie: Fix the wrong order of
>>
>> This introduces an intermittent breakage. Can you squash?
>
> Hi Ahmad:
> Thanks for your comments.
> Do you mean to squash this fix to the preview series?
> I'm afraid that it's not easy to do that.
> Because there are a lot of pci-imx6 code changes after
> commit: 1aa97b002258 ("phy: freescale: pcie: Initialize the imx8 pcie standalone phy driver").

The way I understand it, if a bisect ends up between your two patches, i.MX8M
PCIe will be broken, whereas it worked before. I thus wonder if we shouldn't
instead squash this series here into a single patch.

Cheers,
Ahmad


>
> Best Regards
> Richard Zhu
>>
>>>
>>>
>>
>>
>> --
>> Pengutronix e.K. |
>> |
>> Steuerwalder Str. 21 |
>> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.pe
>> ngutronix.de%2F&amp;data=05%7C01%7Chongxing.zhu%40nxp.com%7C3277
>> 61bac5b9407d977008da89886b15%7C686ea1d3bc2b4c6fa92cd99c5c30163
>> 5%7C0%7C0%7C637973516254656985%7CUnknown%7CTWFpbGZsb3d8eyJ
>> WIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7
>> C3000%7C%7C%7C&amp;sdata=hasOtX77%2Fea5yLbV8C7wOfdZStF5erHOL58
>> Zf6ZZbo4%3D&amp;reserved=0 |
>> 31137 Hildesheim, Germany | Phone:
>> +49-5121-206917-0 |
>> Amtsgericht Hildesheim, HRA 2686 | Fax:
>> +49-5121-206917-5555 |


--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |

2022-08-30 08:59:20

by Richard Zhu

[permalink] [raw]
Subject: RE: [PATCH v1 0/2] Fix the wrong order of phy callbacks

> -----Original Message-----
> From: Ahmad Fatoum <[email protected]>
> Sent: 2022??8??30?? 15:16
> To: Hongxing Zhu <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected]; Marcel
> Ziswiler <[email protected]>; [email protected]
> Cc: [email protected]; [email protected]; dl-linux-imx
> <[email protected]>; [email protected]; [email protected];
> [email protected]
> Subject: Re: [PATCH v1 0/2] Fix the wrong order of phy callbacks
>
> Hello Richard,
>
> On 30.08.22 05:47, Hongxing Zhu wrote:
> >>> [PATCH v1 1/2] PCI: imx6: Fix the wrong order of phy_init() and
> >>> [PATCH
> >>> v1 2/2] phy: freescale: imx8m-pcie: Fix the wrong order of
> >>
> >> This introduces an intermittent breakage. Can you squash?
> >
> > Hi Ahmad:
> > Thanks for your comments.
> > Do you mean to squash this fix to the preview series?
> > I'm afraid that it's not easy to do that.
> > Because there are a lot of pci-imx6 code changes after
> > commit: 1aa97b002258 ("phy: freescale: pcie: Initialize the imx8 pcie
> standalone phy driver").
>
> The way I understand it, if a bisect ends up between your two patches, i.MX8M
> PCIe will be broken, whereas it worked before. I thus wonder if we shouldn't
> instead squash this series here into a single patch.
Yes, it's a possible case when do the bisect.
Since these changes are belong to different git repo.
It will bring maintain difficulties if these two patches are squashed into a
single one.
It's difficult to make a choice.

Best Regards
Richard Zhu
>
> Cheers,
> Ahmad
>
>
> >
> > Best Regards
> > Richard Zhu
> >>
> >>>
> >>>
> >>
> >>
> >> --
> >> Pengutronix e.K. |
> >> |
> >> Steuerwalder Str. 21 |
> >> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.
> >>
> pe%2F&amp;data=05%7C01%7Chongxing.zhu%40nxp.com%7C9950124bc71c
> 4e8c3c6
> >>
> 708da8a577051%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63
> 79744054
> >>
> 06906036%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoi
> V2luMzIi
> >>
> LCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=vKFXx
> 4XDjRsK
> >> Kqh9aBGOmDTNGnDJuUcJhOnTf7boCEw%3D&amp;reserved=0
> >>
> ngutronix.de%2F&amp;data=05%7C01%7Chongxing.zhu%40nxp.com%7C3277
> >>
> 61bac5b9407d977008da89886b15%7C686ea1d3bc2b4c6fa92cd99c5c30163
> >>
> 5%7C0%7C0%7C637973516254656985%7CUnknown%7CTWFpbGZsb3d8eyJ
> >>
> WIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7
> >>
> C3000%7C%7C%7C&amp;sdata=hasOtX77%2Fea5yLbV8C7wOfdZStF5erHOL58
> >> Zf6ZZbo4%3D&amp;reserved=0 |
> >> 31137 Hildesheim, Germany | Phone:
> >> +49-5121-206917-0 |
> >> Amtsgericht Hildesheim, HRA 2686 | Fax:
> >> +49-5121-206917-5555 |
>
>
> --
> Pengutronix e.K. |
> |
> Steuerwalder Str. 21 |
> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.pe
> ngutronix.de%2F&amp;data=05%7C01%7Chongxing.zhu%40nxp.com%7C9950
> 124bc71c4e8c3c6708da8a577051%7C686ea1d3bc2b4c6fa92cd99c5c301635
> %7C0%7C0%7C637974405406906036%7CUnknown%7CTWFpbGZsb3d8eyJW
> IjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3
> 000%7C%7C%7C&amp;sdata=xWe0unt2cJs44zJf10h%2B3fsvxk6SuON6tDcACH
> owImA%3D&amp;reserved=0 |
> 31137 Hildesheim, Germany | Phone:
> +49-5121-206917-0 |
> Amtsgericht Hildesheim, HRA 2686 | Fax:
> +49-5121-206917-5555 |

2022-08-30 15:11:45

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v1 0/2] Fix the wrong order of phy callbacks

On Tue, Aug 30, 2022 at 07:50:55AM +0000, Hongxing Zhu wrote:
> > -----Original Message-----
> > From: Ahmad Fatoum <[email protected]>
> > Sent: 2022年8月30日 15:16
> > To: Hongxing Zhu <[email protected]>; [email protected];
> > [email protected]; [email protected]; [email protected]; Marcel
> > Ziswiler <[email protected]>; [email protected]
> > Cc: [email protected]; [email protected]; dl-linux-imx
> > <[email protected]>; [email protected]; [email protected];
> > [email protected]
> > Subject: Re: [PATCH v1 0/2] Fix the wrong order of phy callbacks

The above quoting style makes it harder than necessary to follow
conversation. See hints at:

https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
https://people.kernel.org/tglx/notes-about-netiquette

The ideal thing would be something like this, where there's a single
line for each sender:

On Tue, Aug 30, 2022 at 07:50:55AM +0000, Hongxing Zhu wrote:
> On Tue, Aug 30, 2022, at 03:16PM, Ahmad Fatoum wrote:
> > On 30.08.22 05:47, Hongxing Zhu wrote:
> > > Do you mean to squash this fix to the preview series?
> > > I'm afraid that it's not easy to do that.
> > > Because there are a lot of pci-imx6 code changes after
> > > commit: 1aa97b002258 ("phy: freescale: pcie: Initialize the imx8 pcie
> > > standalone phy driver").
> >
> > The way I understand it, if a bisect ends up between your two
> > patches, i.MX8M PCIe will be broken, whereas it worked before. I
> > thus wonder if we shouldn't instead squash this series here into a
> > single patch.
>
> Yes, it's a possible case when do the bisect.
> Since these changes are belong to different git repo.

I don't understand the point about different git repos. Patch 1/2
touches drivers/pci/controller/dwc/pci-imx6.c, patch 2/2 touches
drivers/phy/freescale/phy-fsl-imx8m-pcie.c. They're in different
directories, of course, but are in the same Linux kernel source repo.

They're maintained by different people, but we can easily deal with
that by getting an ack from one and merging via the other.

> It will bring maintain difficulties if these two patches are squashed into a
> single one.
> It's difficult to make a choice.

What maintenance difficulty do you see here? I think it looks
*easier* if these are squashed -- that would avoid the possibility of
backporting one without the other, which would certainly be a problem.

If a bisect lands after patch 1/2 but before 2/2, it looks like i.MX8M
will break unnecessarily.

I think Ahmad is right that patches 1/2 and 2/2 should be squashed
into a single patch to avoid this bisection hole.

Bjorn

2022-08-31 02:42:24

by Richard Zhu

[permalink] [raw]
Subject: RE: [PATCH v1 0/2] Fix the wrong order of phy callbacks

> -----Original Message-----
> From: Bjorn Helgaas <[email protected]>
> Sent: 2022??8??30?? 23:06
> To: Hongxing Zhu <[email protected]>
> Cc: Ahmad Fatoum <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected]; Marcel
> Ziswiler <[email protected]>; [email protected];
> [email protected]; [email protected]; dl-linux-imx
> <[email protected]>; [email protected]; [email protected];
> [email protected]
> Subject: Re: [PATCH v1 0/2] Fix the wrong order of phy callbacks
>
> On Tue, Aug 30, 2022 at 07:50:55AM +0000, Hongxing Zhu wrote:
> > > -----Original Message-----
> > > From: Ahmad Fatoum <[email protected]>
> > > Sent: 2022??8??30?? 15:16
> > > To: Hongxing Zhu <[email protected]>; [email protected];
> > > [email protected]; [email protected]; [email protected];
> > > Marcel Ziswiler <[email protected]>; [email protected]
> > > Cc: [email protected]; [email protected];
> > > dl-linux-imx <[email protected]>; [email protected];
> > > [email protected]; [email protected]
> > > Subject: Re: [PATCH v1 0/2] Fix the wrong order of phy callbacks
>
> The above quoting style makes it harder than necessary to follow conversation.
> See hints at:
>
>
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fen.wiki
> pedia.org%2Fwiki%2FPosting_style%23Interleaved_style&amp;data=05%7C01
> %7Chongxing.zhu%40nxp.com%7C70f38214c8f94c5932b408da8a991996%7C
> 686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637974687433589209
> %7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLC
> JBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=ceZmDd
> sNeC9nU6qmt2qXR03fQn33vY%2FqvBIYzeWW7mk%3D&amp;reserved=0
>
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpeople.
> kernel.org%2Ftglx%2Fnotes-about-netiquette&amp;data=05%7C01%7Chongxi
> ng.zhu%40nxp.com%7C70f38214c8f94c5932b408da8a991996%7C686ea1d3b
> c2b4c6fa92cd99c5c301635%7C0%7C0%7C637974687433589209%7CUnkno
> wn%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1ha
> WwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=PkhpGGf7FcvsQTjc0
> hUMpr9Q4JVKeFoLR4foA6zt6jg%3D&amp;reserved=0
>
> The ideal thing would be something like this, where there's a single line for
> each sender:
>
> On Tue, Aug 30, 2022 at 07:50:55AM +0000, Hongxing Zhu wrote:
> > On Tue, Aug 30, 2022, at 03:16PM, Ahmad Fatoum wrote:
> > > On 30.08.22 05:47, Hongxing Zhu wrote:
> > > > Do you mean to squash this fix to the preview series?
> > > > I'm afraid that it's not easy to do that.
> > > > Because there are a lot of pci-imx6 code changes after
> > > > commit: 1aa97b002258 ("phy: freescale: pcie: Initialize the imx8
> > > > pcie standalone phy driver").
> > >
> > > The way I understand it, if a bisect ends up between your two
> > > patches, i.MX8M PCIe will be broken, whereas it worked before. I
> > > thus wonder if we shouldn't instead squash this series here into a
> > > single patch.
> >
> > Yes, it's a possible case when do the bisect.
> > Since these changes are belong to different git repo.
>
> I don't understand the point about different git repos. Patch 1/2 touches
> drivers/pci/controller/dwc/pci-imx6.c, patch 2/2 touches
> drivers/phy/freescale/phy-fsl-imx8m-pcie.c. They're in different directories,
> of course, but are in the same Linux kernel source repo.
>
> They're maintained by different people, but we can easily deal with that by
> getting an ack from one and merging via the other.
>
> > It will bring maintain difficulties if these two patches are squashed
> > into a single one.
> > It's difficult to make a choice.
>
> What maintenance difficulty do you see here? I think it looks
> *easier* if these are squashed -- that would avoid the possibility of backporting
> one without the other, which would certainly be a problem.
>
> If a bisect lands after patch 1/2 but before 2/2, it looks like i.MX8M will break
> unnecessarily.
>
> I think Ahmad is right that patches 1/2 and 2/2 should be squashed into a
> single patch to avoid this bisection hole.
Hi Bjorn:
Thanks for your comments.
My descriptions are not accurate enough. These two files are maintained by
different people, I'm afraid to bring troubles to the maintainers if
I squash these two patch into a single one before.
Now, I understood the situation.
I'm glad to squash them to avoid the bisetion hole.
Thanks again.

Best Regards
Richard Zhu
>
> Bjorn

2022-09-27 11:03:33

by Lorenzo Pieralisi

[permalink] [raw]
Subject: Re: [PATCH v1 0/2] Fix the wrong order of phy callbacks

On Wed, Aug 31, 2022 at 01:37:02AM +0000, Hongxing Zhu wrote:
> > -----Original Message-----
> > From: Bjorn Helgaas <[email protected]>
> > Sent: 2022年8月30日 23:06
> > To: Hongxing Zhu <[email protected]>
> > Cc: Ahmad Fatoum <[email protected]>; [email protected];
> > [email protected]; [email protected]; [email protected]; Marcel
> > Ziswiler <[email protected]>; [email protected];
> > [email protected]; [email protected]; dl-linux-imx
> > <[email protected]>; [email protected]; [email protected];
> > [email protected]
> > Subject: Re: [PATCH v1 0/2] Fix the wrong order of phy callbacks
> >
> > On Tue, Aug 30, 2022 at 07:50:55AM +0000, Hongxing Zhu wrote:
> > > > -----Original Message-----
> > > > From: Ahmad Fatoum <[email protected]>
> > > > Sent: 2022年8月30日 15:16
> > > > To: Hongxing Zhu <[email protected]>; [email protected];
> > > > [email protected]; [email protected]; [email protected];
> > > > Marcel Ziswiler <[email protected]>; [email protected]
> > > > Cc: [email protected]; [email protected];
> > > > dl-linux-imx <[email protected]>; [email protected];
> > > > [email protected]; [email protected]
> > > > Subject: Re: [PATCH v1 0/2] Fix the wrong order of phy callbacks
> >
> > The above quoting style makes it harder than necessary to follow conversation.
> > See hints at:
> >
> >
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fen.wiki
> > pedia.org%2Fwiki%2FPosting_style%23Interleaved_style&amp;data=05%7C01
> > %7Chongxing.zhu%40nxp.com%7C70f38214c8f94c5932b408da8a991996%7C
> > 686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637974687433589209
> > %7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLC
> > JBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=ceZmDd
> > sNeC9nU6qmt2qXR03fQn33vY%2FqvBIYzeWW7mk%3D&amp;reserved=0
> >
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpeople.
> > kernel.org%2Ftglx%2Fnotes-about-netiquette&amp;data=05%7C01%7Chongxi
> > ng.zhu%40nxp.com%7C70f38214c8f94c5932b408da8a991996%7C686ea1d3b
> > c2b4c6fa92cd99c5c301635%7C0%7C0%7C637974687433589209%7CUnkno
> > wn%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1ha
> > WwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=PkhpGGf7FcvsQTjc0
> > hUMpr9Q4JVKeFoLR4foA6zt6jg%3D&amp;reserved=0
> >
> > The ideal thing would be something like this, where there's a single line for
> > each sender:
> >
> > On Tue, Aug 30, 2022 at 07:50:55AM +0000, Hongxing Zhu wrote:
> > > On Tue, Aug 30, 2022, at 03:16PM, Ahmad Fatoum wrote:
> > > > On 30.08.22 05:47, Hongxing Zhu wrote:
> > > > > Do you mean to squash this fix to the preview series?
> > > > > I'm afraid that it's not easy to do that.
> > > > > Because there are a lot of pci-imx6 code changes after
> > > > > commit: 1aa97b002258 ("phy: freescale: pcie: Initialize the imx8
> > > > > pcie standalone phy driver").
> > > >
> > > > The way I understand it, if a bisect ends up between your two
> > > > patches, i.MX8M PCIe will be broken, whereas it worked before. I
> > > > thus wonder if we shouldn't instead squash this series here into a
> > > > single patch.
> > >
> > > Yes, it's a possible case when do the bisect.
> > > Since these changes are belong to different git repo.
> >
> > I don't understand the point about different git repos. Patch 1/2 touches
> > drivers/pci/controller/dwc/pci-imx6.c, patch 2/2 touches
> > drivers/phy/freescale/phy-fsl-imx8m-pcie.c. They're in different directories,
> > of course, but are in the same Linux kernel source repo.
> >
> > They're maintained by different people, but we can easily deal with that by
> > getting an ack from one and merging via the other.
> >
> > > It will bring maintain difficulties if these two patches are squashed
> > > into a single one.
> > > It's difficult to make a choice.
> >
> > What maintenance difficulty do you see here? I think it looks
> > *easier* if these are squashed -- that would avoid the possibility of backporting
> > one without the other, which would certainly be a problem.
> >
> > If a bisect lands after patch 1/2 but before 2/2, it looks like i.MX8M will break
> > unnecessarily.
> >
> > I think Ahmad is right that patches 1/2 and 2/2 should be squashed into a
> > single patch to avoid this bisection hole.
> Hi Bjorn:
> Thanks for your comments.
> My descriptions are not accurate enough. These two files are maintained by
> different people, I'm afraid to bring troubles to the maintainers if
> I squash these two patch into a single one before.
> Now, I understood the situation.
> I'm glad to squash them to avoid the bisetion hole.

Are you sending an updated patch ?

Thanks,
Lorenzo

2022-09-29 01:21:05

by Richard Zhu

[permalink] [raw]
Subject: RE: [PATCH v1 0/2] Fix the wrong order of phy callbacks

> -----Original Message-----
> From: Lorenzo Pieralisi <[email protected]>
> Sent: 2022??9??27?? 18:41
> To: Hongxing Zhu <[email protected]>
> Cc: Bjorn Helgaas <[email protected]>; Ahmad Fatoum
> <[email protected]>; [email protected]; [email protected];
> [email protected]; [email protected]; Marcel Ziswiler
> <[email protected]>; [email protected]; [email protected];
> [email protected]; dl-linux-imx <[email protected]>;
> [email protected]; [email protected];
> [email protected]
> Subject: Re: [PATCH v1 0/2] Fix the wrong order of phy callbacks
>
> On Wed, Aug 31, 2022 at 01:37:02AM +0000, Hongxing Zhu wrote:
> > > -----Original Message-----
> > > From: Bjorn Helgaas <[email protected]>
> > > Sent: 2022??8??30?? 23:06
> > > To: Hongxing Zhu <[email protected]>
> > > Cc: Ahmad Fatoum <[email protected]>; [email protected];
> > > [email protected]; [email protected]; [email protected];
> > > Marcel Ziswiler <[email protected]>; [email protected];
> > > [email protected]; [email protected];
> > > dl-linux-imx <[email protected]>; [email protected];
> > > [email protected]; [email protected]
> > > Subject: Re: [PATCH v1 0/2] Fix the wrong order of phy callbacks
> > >
> > > On Tue, Aug 30, 2022 at 07:50:55AM +0000, Hongxing Zhu wrote:
> > > > > -----Original Message-----
> > > > > From: Ahmad Fatoum <[email protected]>
> > > > > Sent: 2022??8??30?? 15:16
> > > > > To: Hongxing Zhu <[email protected]>; [email protected];
> > > > > [email protected]; [email protected];
> > > > > [email protected]; Marcel Ziswiler <[email protected]>;
> > > > > [email protected]
> > > > > Cc: [email protected]; [email protected];
> > > > > dl-linux-imx <[email protected]>; [email protected];
> > > > > [email protected];
> > > > > [email protected]
> > > > > Subject: Re: [PATCH v1 0/2] Fix the wrong order of phy callbacks
> > >
> > > The above quoting style makes it harder than necessary to follow
> conversation.
> > > See hints at:
> > >
> > >
> > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fen
> > > .wiki%2F&amp;data=05%7C01%7Chongxing.zhu%40nxp.com%7C983d5b6
> 7c735444
> > >
> 0c2fc08daa074bd79%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7
> C63799
> > >
> 8720505182178%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLC
> JQIjoiV2
> > >
> luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=
> HCEzr
> > >
> rShsMQJp3xDjKrCO0YNtl%2BhciThAC%2F58imB%2FOU%3D&amp;reserved=0
> > >
> pedia.org%2Fwiki%2FPosting_style%23Interleaved_style&amp;data=05%7C0
> > >
> 1 %7Chongxing.zhu%40nxp.com%7C70f38214c8f94c5932b408da8a991996%
> 7C
> > >
> 686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637974687433589209
> > > %7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMz
> IiLC
> > >
> JBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=ceZmDd
> > > sNeC9nU6qmt2qXR03fQn33vY%2FqvBIYzeWW7mk%3D&amp;reserved=0
> > >
> > >
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpeople.
> > >
> kernel.org%2Ftglx%2Fnotes-about-netiquette&amp;data=05%7C01%7Chongxi
> > >
> ng.zhu%40nxp.com%7C70f38214c8f94c5932b408da8a991996%7C686ea1d3b
> > >
> c2b4c6fa92cd99c5c301635%7C0%7C0%7C637974687433589209%7CUnkno
> > >
> wn%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1ha
> > >
> WwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=PkhpGGf7FcvsQTjc0
> > > hUMpr9Q4JVKeFoLR4foA6zt6jg%3D&amp;reserved=0
> > >
> > > The ideal thing would be something like this, where there's a single
> > > line for each sender:
> > >
> > > On Tue, Aug 30, 2022 at 07:50:55AM +0000, Hongxing Zhu wrote:
> > > > On Tue, Aug 30, 2022, at 03:16PM, Ahmad Fatoum wrote:
> > > > > On 30.08.22 05:47, Hongxing Zhu wrote:
> > > > > > Do you mean to squash this fix to the preview series?
> > > > > > I'm afraid that it's not easy to do that.
> > > > > > Because there are a lot of pci-imx6 code changes after
> > > > > > commit: 1aa97b002258 ("phy: freescale: pcie: Initialize the
> > > > > > imx8 pcie standalone phy driver").
> > > > >
> > > > > The way I understand it, if a bisect ends up between your two
> > > > > patches, i.MX8M PCIe will be broken, whereas it worked before. I
> > > > > thus wonder if we shouldn't instead squash this series here into
> > > > > a single patch.
> > > >
> > > > Yes, it's a possible case when do the bisect.
> > > > Since these changes are belong to different git repo.
> > >
> > > I don't understand the point about different git repos. Patch 1/2
> > > touches drivers/pci/controller/dwc/pci-imx6.c, patch 2/2 touches
> > > drivers/phy/freescale/phy-fsl-imx8m-pcie.c. They're in different
> > > directories, of course, but are in the same Linux kernel source repo.
> > >
> > > They're maintained by different people, but we can easily deal with
> > > that by getting an ack from one and merging via the other.
> > >
> > > > It will bring maintain difficulties if these two patches are
> > > > squashed into a single one.
> > > > It's difficult to make a choice.
> > >
> > > What maintenance difficulty do you see here? I think it looks
> > > *easier* if these are squashed -- that would avoid the possibility
> > > of backporting one without the other, which would certainly be a problem.
> > >
> > > If a bisect lands after patch 1/2 but before 2/2, it looks like
> > > i.MX8M will break unnecessarily.
> > >
> > > I think Ahmad is right that patches 1/2 and 2/2 should be squashed
> > > into a single patch to avoid this bisection hole.
> > Hi Bjorn:
> > Thanks for your comments.
> > My descriptions are not accurate enough. These two files are
> > maintained by different people, I'm afraid to bring troubles to the
> > maintainers if I squash these two patch into a single one before.
> > Now, I understood the situation.
> > I'm glad to squash them to avoid the bisetion hole.
>
> Are you sending an updated patch ?
>
Hi Lorenzo:
The v3 patch had been sent.
Here it is.
https://patchwork.kernel.org/project/linux-pci/patch/[email protected]/
Thanks.
Best Regards
Richard Zhu

> Thanks,
> Lorenzo