[v1,0/2] Fix the wrong order of phy callbacks

Message

Hongxing Zhu Aug. 22, 2022, 1:10 p.m. UTC

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

Comments

Alexander Stein Aug. 26, 2022, 6:26 a.m. UTC | #1

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 <[e-mail]>

Thanks again!
Alexander

Hongxing Zhu Aug. 26, 2022, 6:56 a.m. UTC | #2

> -----Original Message-----
> From: Alexander Stein <[e-mail]>
> Sent: 2022年8月26日 14:26
> To: Hongxing Zhu <[e-mail]>
> Cc: [e-mail]; [e-mail];
> [e-mail]; [e-mail]; Marcel Ziswiler
> <[e-mail]>; [e-mail];
> [e-mail]; Hongxing Zhu <[e-mail]>;
> [e-mail]; [e-mail];
> [e-mail]; [e-mail];
> [e-mail]; dl-linux-imx <[e-mail]>
> 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 <[e-mail]>
> 
Hi Alexander:
It's great. Thanks for your kindly help.

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

Ahmad Fatoum Aug. 29, 2022, 6:33 a.m. UTC | #3

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?

> 
>

Hongxing Zhu Aug. 30, 2022, 3:47 a.m. UTC | #4

> -----Original Message-----
> From: Ahmad Fatoum <[e-mail]>
> Sent: 2022年8月29日 14:34
> To: Hongxing Zhu <[e-mail]>; [e-mail];
> [e-mail]; [e-mail]; [e-mail]; Marcel
> Ziswiler <[e-mail]>; [e-mail]
> Cc: [e-mail]; [e-mail]; dl-linux-imx
> <[e-mail]>; [e-mail]; [e-mail];
> [e-mail]
> 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:
> [phone]    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:
> [phone]-5555 |

Ahmad Fatoum Aug. 30, 2022, 7:15 a.m. UTC | #5

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&data=05%7C01%7Chongxing.zhu%40nxp.com%7C3277
>> 61bac5b9407d977008da89886b15%7C686ea1d3bc2b4c6fa92cd99c5c30163
>> 5%7C0%7C0%7C637973516254656985%7CUnknown%7CTWFpbGZsb3d8eyJ
>> WIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7
>> C3000%7C%7C%7C&sdata=hasOtX77%2Fea5yLbV8C7wOfdZStF5erHOL58
>> Zf6ZZbo4%3D&reserved=0  |
>> 31137 Hildesheim, Germany                  | Phone:
>> [phone]    |
>> Amtsgericht Hildesheim, HRA 2686           | Fax:
>> [phone]-5555 |

Hongxing Zhu Aug. 30, 2022, 7:50 a.m. UTC | #6

> -----Original Message-----
> From: Ahmad Fatoum <[e-mail]>
> Sent: 2022年8月30日 15:16
> To: Hongxing Zhu <[e-mail]>; [e-mail];
> [e-mail]; [e-mail]; [e-mail]; Marcel
> Ziswiler <[e-mail]>; [e-mail]
> Cc: [e-mail]; [e-mail]; dl-linux-imx
> <[e-mail]>; [e-mail]; [e-mail];
> [e-mail]
> 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:
> >> [phone]    |
> >> Amtsgericht Hildesheim, HRA 2686           | Fax:
> >> [phone]-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:
> [phone]    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:
> [phone]-5555 |

Bjorn Helgaas Aug. 30, 2022, 3:05 p.m. UTC | #7

On Tue, Aug 30, 2022 at 07:50:55AM +0000, Hongxing Zhu wrote:
> > -----Original Message-----
> > From: Ahmad Fatoum <[e-mail]>
> > Sent: 2022年8月30日 15:16
> > To: Hongxing Zhu <[e-mail]>; [e-mail];
> > [e-mail]; [e-mail]; [e-mail]; Marcel
> > Ziswiler <[e-mail]>; [e-mail]
> > Cc: [e-mail]; [e-mail]; dl-linux-imx
> > <[e-mail]>; [e-mail]; [e-mail];
> > [e-mail]
> > 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

Hongxing Zhu Aug. 31, 2022, 1:37 a.m. UTC | #8

> -----Original Message-----
> From: Bjorn Helgaas <[e-mail]>
> Sent: 2022年8月30日 23:06
> To: Hongxing Zhu <[e-mail]>
> Cc: Ahmad Fatoum <[e-mail]>; [e-mail];
> [e-mail]; [e-mail]; [e-mail]; Marcel
> Ziswiler <[e-mail]>; [e-mail];
> [e-mail]; [e-mail]; dl-linux-imx
> <[e-mail]>; [e-mail]; [e-mail];
> [e-mail]
> 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 <[e-mail]>
> > > Sent: 2022年8月30日 15:16
> > > To: Hongxing Zhu <[e-mail]>; [e-mail];
> > > [e-mail]; [e-mail]; [e-mail];
> > > Marcel Ziswiler <[e-mail]>; [e-mail]
> > > Cc: [e-mail]; [e-mail];
> > > dl-linux-imx <[e-mail]>; [e-mail];
> > > [e-mail]; [e-mail]
> > > 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

Lorenzo Pieralisi Sept. 27, 2022, 10:40 a.m. UTC | #9

On Wed, Aug 31, 2022 at 01:37:02AM +0000, Hongxing Zhu wrote:
> > -----Original Message-----
> > From: Bjorn Helgaas <[e-mail]>
> > Sent: 2022年8月30日 23:06
> > To: Hongxing Zhu <[e-mail]>
> > Cc: Ahmad Fatoum <[e-mail]>; [e-mail];
> > [e-mail]; [e-mail]; [e-mail]; Marcel
> > Ziswiler <[e-mail]>; [e-mail];
> > [e-mail]; [e-mail]; dl-linux-imx
> > <[e-mail]>; [e-mail]; [e-mail];
> > [e-mail]
> > 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 <[e-mail]>
> > > > Sent: 2022年8月30日 15:16
> > > > To: Hongxing Zhu <[e-mail]>; [e-mail];
> > > > [e-mail]; [e-mail]; [e-mail];
> > > > Marcel Ziswiler <[e-mail]>; [e-mail]
> > > > Cc: [e-mail]; [e-mail];
> > > > dl-linux-imx <[e-mail]>; [e-mail];
> > > > [e-mail]; [e-mail]
> > > > 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

Hongxing Zhu Sept. 29, 2022, 12:51 a.m. UTC | #10

> -----Original Message-----
> From: Lorenzo Pieralisi <[e-mail]>
> Sent: 2022年9月27日 18:41
> To: Hongxing Zhu <[e-mail]>
> Cc: Bjorn Helgaas <[e-mail]>; Ahmad Fatoum
> <[e-mail]>; [e-mail]; [e-mail];
> [e-mail]; [e-mail]; Marcel Ziswiler
> <[e-mail]>; [e-mail]; [e-mail];
> [e-mail]; dl-linux-imx <[e-mail]>;
> [e-mail]; [e-mail];
> [e-mail]
> 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 <[e-mail]>
> > > Sent: 2022年8月30日 23:06
> > > To: Hongxing Zhu <[e-mail]>
> > > Cc: Ahmad Fatoum <[e-mail]>; [e-mail];
> > > [e-mail]; [e-mail]; [e-mail];
> > > Marcel Ziswiler <[e-mail]>; [e-mail];
> > > [e-mail]; [e-mail];
> > > dl-linux-imx <[e-mail]>; [e-mail];
> > > [e-mail]; [e-mail]
> > > 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 <[e-mail]>
> > > > > Sent: 2022年8月30日 15:16
> > > > > To: Hongxing Zhu <[e-mail]>; [e-mail];
> > > > > [e-mail]; [e-mail];
> > > > > [e-mail]; Marcel Ziswiler <[e-mail]>;
> > > > > [e-mail]
> > > > > Cc: [e-mail]; [e-mail];
> > > > > dl-linux-imx <[e-mail]>; [e-mail];
> > > > > [e-mail];
> > > > > [e-mail]
> > > > > 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/[e-mail]/
Thanks.
Best Regards
Richard Zhu

> Thanks,
> Lorenzo