From: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
To: Andrew Murray <andrew.murray@arm.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
Masami Hiramatsu <masami.hiramatsu@linaro.org>,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
Jassi Brar <jaswinder.singh@linaro.org>,
Bjorn Helgaas <bhelgaas@google.com>,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 2/2] PCI: uniphier: Add checking whether PERST# is deasserted
Date: Thu, 07 Nov 2019 20:52:39 +0900 [thread overview]
Message-ID: <20191107205239.65C1.4A936039@socionext.com> (raw)
In-Reply-To: <20191107100207.GV9723@e119886-lin.cambridge.arm.com>
Hi Andrew,
Thank you for your comments.
On Thu, 7 Nov 2019 10:02:08 +0000 <andrew.murray@arm.com> wrote:
> On Thu, Nov 07, 2019 at 01:58:15PM +0900, Kunihiko Hayashi wrote:
> > When PERST# is asserted once, EP configuration will be initialized.
>
> I don't quite understand this - does the EP/RC mode depend on how often
> PERST# is toggled?
I think of connecting this RC controller and EP based on `Linux PCI
endpoint framework' in another machine.
While this RC driver is probing, the EP driver might be also probing and
configurating itself using configfs. If PERST# is toggled after the EP
has done its configuration, this configuration will be lost.
I expect that the EP configurates after RC has toggled PERST#, however,
there is no way to synchronize both of them.
>
> > If PERST# has been already deasserted, it isn't necessary to assert
> > here.
>
> What is the motativation for this patch? Is it to avoid a delay during
> boot, or to fix some bug? Isn't it nice to always reset the IP before
> use anyway?
Since EP device usually works without its configuration after PERST#,
deassering PERST# here is no problem. Since bus reset breaks configuration
of EP as shown above, bus reset should be done before EP has probed.
Maybe boot sequence in host machine will do.
>
> >
> > This checks whether PERST# is deasserted using PCL_PINMON register,
> > and adds omit controlling PERST#.
>
> Should this read 'and omits controlling PERST#'?
Yes, I'll fix it.
>
> >
> > Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
> > ---
> > drivers/pci/controller/dwc/pcie-uniphier.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-uniphier.c b/drivers/pci/controller/dwc/pcie-uniphier.c
> > index 8fd7bad..1ea4220 100644
> > --- a/drivers/pci/controller/dwc/pcie-uniphier.c
> > +++ b/drivers/pci/controller/dwc/pcie-uniphier.c
> > @@ -22,6 +22,9 @@
> >
> > #include "pcie-designware.h"
> >
> > +#define PCL_PINMON 0x0028
> > +#define PCL_PINMON_PERST_OUT BIT(16)
> > +
> > #define PCL_PINCTRL0 0x002c
> > #define PCL_PERST_PLDN_REGEN BIT(12)
> > #define PCL_PERST_NOE_REGEN BIT(11)
> > @@ -100,6 +103,11 @@ static void uniphier_pcie_init_rc(struct uniphier_pcie_priv *priv)
> > val |= PCL_SYS_AUX_PWR_DET;
> > writel(val, priv->base + PCL_APP_PM0);
> >
> > + /* return if PERST# is already deasserted */
>
> This comment just describes what the following line does which may be
> self-explanatory, perhaps a better comment would describe why we avoid
> a reset.
Indeed. I'll write the reason here.
Thank you,
---
Best Regards,
Kunihiko Hayashi
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2019-11-07 11:52 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-07 4:58 [PATCH 1/2] PCI: uniphier: Set mode register to host mode Kunihiko Hayashi
2019-11-07 4:58 ` [PATCH 2/2] PCI: uniphier: Add checking whether PERST# is deasserted Kunihiko Hayashi
2019-11-07 10:02 ` Andrew Murray
2019-11-07 11:52 ` Kunihiko Hayashi [this message]
2019-11-07 12:46 ` Andrew Murray
2019-11-08 7:30 ` Kunihiko Hayashi
2019-11-08 9:59 ` Andrew Murray
2019-11-21 16:47 ` Lorenzo Pieralisi
2019-11-22 11:53 ` Kunihiko Hayashi
2019-12-04 10:05 ` Kunihiko Hayashi
2019-12-06 6:58 ` Kishon Vijay Abraham I
2019-12-06 8:58 ` Kunihiko Hayashi
2019-12-06 9:01 ` Kishon Vijay Abraham I
2019-12-09 2:35 ` Kunihiko Hayashi
2019-11-07 9:52 ` [PATCH 1/2] PCI: uniphier: Set mode register to host mode Andrew Murray
2019-11-21 16:49 ` Lorenzo Pieralisi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20191107205239.65C1.4A936039@socionext.com \
--to=hayashi.kunihiko@socionext.com \
--cc=andrew.murray@arm.com \
--cc=bhelgaas@google.com \
--cc=jaswinder.singh@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=lorenzo.pieralisi@arm.com \
--cc=masami.hiramatsu@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).