* [PATCH v2] PCI: mt7621: Sleep a bit after power on the PCIs phy ports @ 2022-12-31 7:40 Sergio Paracuellos 2023-01-23 8:55 ` Sergio Paracuellos 2023-02-03 9:29 ` Lorenzo Pieralisi 0 siblings, 2 replies; 6+ messages in thread From: Sergio Paracuellos @ 2022-12-31 7:40 UTC (permalink / raw) To: linux-pci; +Cc: bhelgaas, lpieralisi, robh, kw, matthias.bgg, linux-kernel Some devices like ZBT WE1326 and ZBT WF3526-P and some Netgear models need to sleep a bit after call to mt7621_pcie_init_port() driver function to get into reliable boots for both warm and hard resets. The needed time for these devices to always detect the ports seems to be from 75 to 100 milliseconds. There is no datasheet or something similar to really understand why this extra time is needed in these devices but not in most of the boards which use mt7621 SoC. This issue has been reported by openWRT community and the complete discussion is in [0]. The selected time of 100 milliseconds has been also tested in these devices ending up in an always working platform. Hence, properly add the extra 100 milliseconds msleep() function call to make also these devices work. [0]: https://github.com/openwrt/openwrt/pull/11220 Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com> --- Hi Bjorn / Lorenzo, As per Lorenzo comments in v1[0] here it is the patch with changes in commit message and introducing a new definition for this needed extra delay time. I wish you the best new year for you both. Changes in v2: - Add a new define 'INIT_PORTS_DELAY_MS' avoiding to reuse 'PERST_DELAY_MS'. - Rewrite commit message and add a link to openWRT discussion. Previous patch lore link: [0]: https://lore.kernel.org/lkml/20221209071703.2891714-1-sergio.paracuellos@gmail.com/T/ Thanks, Sergio Paracuellos drivers/pci/controller/pcie-mt7621.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/pci/controller/pcie-mt7621.c b/drivers/pci/controller/pcie-mt7621.c index ee7aad09d627..63a5f4463a9f 100644 --- a/drivers/pci/controller/pcie-mt7621.c +++ b/drivers/pci/controller/pcie-mt7621.c @@ -60,6 +60,7 @@ #define PCIE_PORT_LINKUP BIT(0) #define PCIE_PORT_CNT 3 +#define INIT_PORTS_DELAY_MS 100 #define PERST_DELAY_MS 100 /** @@ -369,6 +370,7 @@ static int mt7621_pcie_init_ports(struct mt7621_pcie *pcie) } } + msleep(INIT_PORTS_DELAY_MS); mt7621_pcie_reset_ep_deassert(pcie); tmp = NULL; -- 2.25.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] PCI: mt7621: Sleep a bit after power on the PCIs phy ports 2022-12-31 7:40 [PATCH v2] PCI: mt7621: Sleep a bit after power on the PCIs phy ports Sergio Paracuellos @ 2023-01-23 8:55 ` Sergio Paracuellos 2023-02-02 16:27 ` Lorenzo Pieralisi 2023-02-03 9:29 ` Lorenzo Pieralisi 1 sibling, 1 reply; 6+ messages in thread From: Sergio Paracuellos @ 2023-01-23 8:55 UTC (permalink / raw) To: linux-pci; +Cc: bhelgaas, lpieralisi, robh, kw, matthias.bgg, linux-kernel Hi! On Sat, Dec 31, 2022 at 8:40 AM Sergio Paracuellos <sergio.paracuellos@gmail.com> wrote: > > Some devices like ZBT WE1326 and ZBT WF3526-P and some Netgear models need > to sleep a bit after call to mt7621_pcie_init_port() driver function to get > into reliable boots for both warm and hard resets. The needed time for these > devices to always detect the ports seems to be from 75 to 100 milliseconds. > There is no datasheet or something similar to really understand why this > extra time is needed in these devices but not in most of the boards which > use mt7621 SoC. This issue has been reported by openWRT community and the > complete discussion is in [0]. The selected time of 100 milliseconds has > been also tested in these devices ending up in an always working platform. > Hence, properly add the extra 100 milliseconds msleep() function call to make > also these devices work. > > [0]: https://github.com/openwrt/openwrt/pull/11220 > > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com> > --- > Hi Bjorn / Lorenzo, > > As per Lorenzo comments in v1[0] here it is the patch with changes in commit > message and introducing a new definition for this needed extra delay time. > I wish you the best new year for you both. > > Changes in v2: > - Add a new define 'INIT_PORTS_DELAY_MS' avoiding to reuse 'PERST_DELAY_MS'. > - Rewrite commit message and add a link to openWRT discussion. > > Previous patch lore link: > [0]: https://lore.kernel.org/lkml/20221209071703.2891714-1-sergio.paracuellos@gmail.com/T/ > > Thanks, > Sergio Paracuellos > > drivers/pci/controller/pcie-mt7621.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/pci/controller/pcie-mt7621.c b/drivers/pci/controller/pcie-mt7621.c > index ee7aad09d627..63a5f4463a9f 100644 > --- a/drivers/pci/controller/pcie-mt7621.c > +++ b/drivers/pci/controller/pcie-mt7621.c > @@ -60,6 +60,7 @@ > #define PCIE_PORT_LINKUP BIT(0) > #define PCIE_PORT_CNT 3 > > +#define INIT_PORTS_DELAY_MS 100 > #define PERST_DELAY_MS 100 > > /** > @@ -369,6 +370,7 @@ static int mt7621_pcie_init_ports(struct mt7621_pcie *pcie) > } > } > > + msleep(INIT_PORTS_DELAY_MS); > mt7621_pcie_reset_ep_deassert(pcie); > > tmp = NULL; > -- > 2.25.1 > Gentle ping on this patch :-). Thanks, Sergio Paracuellos ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] PCI: mt7621: Sleep a bit after power on the PCIs phy ports 2023-01-23 8:55 ` Sergio Paracuellos @ 2023-02-02 16:27 ` Lorenzo Pieralisi 2023-02-02 18:45 ` Sergio Paracuellos 0 siblings, 1 reply; 6+ messages in thread From: Lorenzo Pieralisi @ 2023-02-02 16:27 UTC (permalink / raw) To: Sergio Paracuellos Cc: linux-pci, bhelgaas, robh, kw, matthias.bgg, linux-kernel On Mon, Jan 23, 2023 at 09:55:20AM +0100, Sergio Paracuellos wrote: > Hi! > > On Sat, Dec 31, 2022 at 8:40 AM Sergio Paracuellos > <sergio.paracuellos@gmail.com> wrote: > > > > Some devices like ZBT WE1326 and ZBT WF3526-P and some Netgear models need > > to sleep a bit after call to mt7621_pcie_init_port() driver function to get > > into reliable boots for both warm and hard resets. The needed time for these > > devices to always detect the ports seems to be from 75 to 100 milliseconds. > > There is no datasheet or something similar to really understand why this > > extra time is needed in these devices but not in most of the boards which > > use mt7621 SoC. This issue has been reported by openWRT community and the > > complete discussion is in [0]. The selected time of 100 milliseconds has > > been also tested in these devices ending up in an always working platform. > > Hence, properly add the extra 100 milliseconds msleep() function call to make > > also these devices work. > > > > [0]: https://github.com/openwrt/openwrt/pull/11220 > > > > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com> > > --- > > Hi Bjorn / Lorenzo, > > > > As per Lorenzo comments in v1[0] here it is the patch with changes in commit > > message and introducing a new definition for this needed extra delay time. > > I wish you the best new year for you both. > > > > Changes in v2: > > - Add a new define 'INIT_PORTS_DELAY_MS' avoiding to reuse 'PERST_DELAY_MS'. > > - Rewrite commit message and add a link to openWRT discussion. > > > > Previous patch lore link: > > [0]: https://lore.kernel.org/lkml/20221209071703.2891714-1-sergio.paracuellos@gmail.com/T/ > > > > Thanks, > > Sergio Paracuellos > > > > drivers/pci/controller/pcie-mt7621.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/pci/controller/pcie-mt7621.c b/drivers/pci/controller/pcie-mt7621.c > > index ee7aad09d627..63a5f4463a9f 100644 > > --- a/drivers/pci/controller/pcie-mt7621.c > > +++ b/drivers/pci/controller/pcie-mt7621.c > > @@ -60,6 +60,7 @@ > > #define PCIE_PORT_LINKUP BIT(0) > > #define PCIE_PORT_CNT 3 > > > > +#define INIT_PORTS_DELAY_MS 100 > > #define PERST_DELAY_MS 100 > > > > /** > > @@ -369,6 +370,7 @@ static int mt7621_pcie_init_ports(struct mt7621_pcie *pcie) > > } > > } > > > > + msleep(INIT_PORTS_DELAY_MS); > > mt7621_pcie_reset_ep_deassert(pcie); > > > > tmp = NULL; > > -- > > 2.25.1 > > > > Gentle ping on this patch :-). I was about to merge it - there are a couple of things that I don't like. First one is the comment, "Sleep a bit" does not mean anything. I'd rather say "delay port initialization" or something like that, be precise please. More importantly, this is a fix but it is not clear from the commit log. Please report what's wrong in the commit log. I will rework the commit log and merge it then (if you want to avoid another version just post an updated log here and I will merge it). Thanks, Lorenzo ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] PCI: mt7621: Sleep a bit after power on the PCIs phy ports 2023-02-02 16:27 ` Lorenzo Pieralisi @ 2023-02-02 18:45 ` Sergio Paracuellos 0 siblings, 0 replies; 6+ messages in thread From: Sergio Paracuellos @ 2023-02-02 18:45 UTC (permalink / raw) To: Lorenzo Pieralisi Cc: linux-pci, bhelgaas, robh, kw, matthias.bgg, linux-kernel Hi Lorenzo, Thanks for your comments. On Thu, Feb 2, 2023 at 5:27 PM Lorenzo Pieralisi <lpieralisi@kernel.org> wrote: > > On Mon, Jan 23, 2023 at 09:55:20AM +0100, Sergio Paracuellos wrote: > > Hi! > > > > On Sat, Dec 31, 2022 at 8:40 AM Sergio Paracuellos > > <sergio.paracuellos@gmail.com> wrote: > > > > > > Some devices like ZBT WE1326 and ZBT WF3526-P and some Netgear models need > > > to sleep a bit after call to mt7621_pcie_init_port() driver function to get > > > into reliable boots for both warm and hard resets. The needed time for these > > > devices to always detect the ports seems to be from 75 to 100 milliseconds. > > > There is no datasheet or something similar to really understand why this > > > extra time is needed in these devices but not in most of the boards which > > > use mt7621 SoC. This issue has been reported by openWRT community and the > > > complete discussion is in [0]. The selected time of 100 milliseconds has > > > been also tested in these devices ending up in an always working platform. > > > Hence, properly add the extra 100 milliseconds msleep() function call to make > > > also these devices work. > > > > > > [0]: https://github.com/openwrt/openwrt/pull/11220 > > > > > > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com> > > > --- > > > Hi Bjorn / Lorenzo, > > > > > > As per Lorenzo comments in v1[0] here it is the patch with changes in commit > > > message and introducing a new definition for this needed extra delay time. > > > I wish you the best new year for you both. > > > > > > Changes in v2: > > > - Add a new define 'INIT_PORTS_DELAY_MS' avoiding to reuse 'PERST_DELAY_MS'. > > > - Rewrite commit message and add a link to openWRT discussion. > > > > > > Previous patch lore link: > > > [0]: https://lore.kernel.org/lkml/20221209071703.2891714-1-sergio.paracuellos@gmail.com/T/ > > > > > > Thanks, > > > Sergio Paracuellos > > > > > > drivers/pci/controller/pcie-mt7621.c | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/drivers/pci/controller/pcie-mt7621.c b/drivers/pci/controller/pcie-mt7621.c > > > index ee7aad09d627..63a5f4463a9f 100644 > > > --- a/drivers/pci/controller/pcie-mt7621.c > > > +++ b/drivers/pci/controller/pcie-mt7621.c > > > @@ -60,6 +60,7 @@ > > > #define PCIE_PORT_LINKUP BIT(0) > > > #define PCIE_PORT_CNT 3 > > > > > > +#define INIT_PORTS_DELAY_MS 100 > > > #define PERST_DELAY_MS 100 > > > > > > /** > > > @@ -369,6 +370,7 @@ static int mt7621_pcie_init_ports(struct mt7621_pcie *pcie) > > > } > > > } > > > > > > + msleep(INIT_PORTS_DELAY_MS); > > > mt7621_pcie_reset_ep_deassert(pcie); > > > > > > tmp = NULL; > > > -- > > > 2.25.1 > > > > > > > Gentle ping on this patch :-). > > I was about to merge it - there are a couple of things that I don't > like. > > First one is the comment, "Sleep a bit" does not mean anything. I'd > rather say "delay port initialization" or something like that, be > precise please. How about: PCI: mt7621: delay phy ports initialization > > More importantly, this is a fix but it is not clear from the commit > log. Please report what's wrong in the commit log. The problem is that PCI ports are not detected properly without this delay for those devices so plugged cards on them do not work at all. I don't really know how to write the commit log clearer but I'll try adding this cards plugged not working stuff :-) We can add maybe also if you want: Fixes: 2bdd5238e756 ("PCI: mt7621: Add MediaTek MT7621 PCIe host controller driver") > > I will rework the commit log and merge it then (if you want to avoid > another version just post an updated log here and I will merge it). Some devices like ZBT WE1326 and ZBT WF3526-P and some Netgear models need to sleep a bit after calling the mt7621_pcie_init_port() driver function to get into reliable boots for both warm and hard resets. The needed time for these devices to always detect the ports seems to be from 75 to 100 milliseconds. Without ports properly detected no card plugged into the PCI's work at all. There is no datasheet or something similar to really understand why this extra time is needed in these devices but not in most of the boards which use mt7621 SoC. This issue has been reported by openWRT community and the complete discussion is in [0]. The selected time of 100 milliseconds has been also tested in these devices ending up in an always working platform. Hence, properly add the extra 100 milliseconds msleep() function call to make also these devices work. [0]: https://github.com/openwrt/openwrt/pull/11220 > > Thanks, > Lorenzo Thanks, Sergio Paracuellos ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] PCI: mt7621: Sleep a bit after power on the PCIs phy ports 2022-12-31 7:40 [PATCH v2] PCI: mt7621: Sleep a bit after power on the PCIs phy ports Sergio Paracuellos 2023-01-23 8:55 ` Sergio Paracuellos @ 2023-02-03 9:29 ` Lorenzo Pieralisi 2023-02-03 9:31 ` Sergio Paracuellos 1 sibling, 1 reply; 6+ messages in thread From: Lorenzo Pieralisi @ 2023-02-03 9:29 UTC (permalink / raw) To: linux-pci, Sergio Paracuellos Cc: Lorenzo Pieralisi, bhelgaas, kw, linux-kernel, robh, matthias.bgg On Sat, 31 Dec 2022 08:40:41 +0100, Sergio Paracuellos wrote: > Some devices like ZBT WE1326 and ZBT WF3526-P and some Netgear models need > to sleep a bit after call to mt7621_pcie_init_port() driver function to get > into reliable boots for both warm and hard resets. The needed time for these > devices to always detect the ports seems to be from 75 to 100 milliseconds. > There is no datasheet or something similar to really understand why this > extra time is needed in these devices but not in most of the boards which > use mt7621 SoC. This issue has been reported by openWRT community and the > complete discussion is in [0]. The selected time of 100 milliseconds has > been also tested in these devices ending up in an always working platform. > Hence, properly add the extra 100 milliseconds msleep() function call to make > also these devices work. > > [...] Applied to pci/controller/mt7621, thanks! [1/1] PCI: mt7621: Sleep a bit after power on the PCIs phy ports https://git.kernel.org/pci/pci/c/0cb2a8f3456f Thanks, Lorenzo ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] PCI: mt7621: Sleep a bit after power on the PCIs phy ports 2023-02-03 9:29 ` Lorenzo Pieralisi @ 2023-02-03 9:31 ` Sergio Paracuellos 0 siblings, 0 replies; 6+ messages in thread From: Sergio Paracuellos @ 2023-02-03 9:31 UTC (permalink / raw) To: Lorenzo Pieralisi Cc: linux-pci, bhelgaas, kw, linux-kernel, robh, matthias.bgg On Fri, Feb 3, 2023 at 10:29 AM Lorenzo Pieralisi <lpieralisi@kernel.org> wrote: > > On Sat, 31 Dec 2022 08:40:41 +0100, Sergio Paracuellos wrote: > > Some devices like ZBT WE1326 and ZBT WF3526-P and some Netgear models need > > to sleep a bit after call to mt7621_pcie_init_port() driver function to get > > into reliable boots for both warm and hard resets. The needed time for these > > devices to always detect the ports seems to be from 75 to 100 milliseconds. > > There is no datasheet or something similar to really understand why this > > extra time is needed in these devices but not in most of the boards which > > use mt7621 SoC. This issue has been reported by openWRT community and the > > complete discussion is in [0]. The selected time of 100 milliseconds has > > been also tested in these devices ending up in an always working platform. > > Hence, properly add the extra 100 milliseconds msleep() function call to make > > also these devices work. > > > > [...] > > Applied to pci/controller/mt7621, thanks! > > [1/1] PCI: mt7621: Sleep a bit after power on the PCIs phy ports > https://git.kernel.org/pci/pci/c/0cb2a8f3456f > > Thanks, > Lorenzo Thanks! Sergio Paracuellos ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-02-03 9:31 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-12-31 7:40 [PATCH v2] PCI: mt7621: Sleep a bit after power on the PCIs phy ports Sergio Paracuellos 2023-01-23 8:55 ` Sergio Paracuellos 2023-02-02 16:27 ` Lorenzo Pieralisi 2023-02-02 18:45 ` Sergio Paracuellos 2023-02-03 9:29 ` Lorenzo Pieralisi 2023-02-03 9:31 ` Sergio Paracuellos
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.