Linux-PCI Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 1/2] PCI: uniphier: Set mode register to host mode
@ 2019-11-07  4:58 Kunihiko Hayashi
  2019-11-07  4:58 ` [PATCH 2/2] PCI: uniphier: Add checking whether PERST# is deasserted Kunihiko Hayashi
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Kunihiko Hayashi @ 2019-11-07  4:58 UTC (permalink / raw)
  To: Lorenzo Pieralisi, Andrew Murray, Bjorn Helgaas
  Cc: linux-pci, linux-arm-kernel, linux-kernel, Masami Hiramatsu,
	Jassi Brar, Kunihiko Hayashi

In order to avoid effect of the initial mode depending on SoCs,
this patch sets the mode register to host(RC) mode.

Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
---
 drivers/pci/controller/dwc/pcie-uniphier.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-uniphier.c b/drivers/pci/controller/dwc/pcie-uniphier.c
index 3f30ee4..8fd7bad 100644
--- a/drivers/pci/controller/dwc/pcie-uniphier.c
+++ b/drivers/pci/controller/dwc/pcie-uniphier.c
@@ -33,6 +33,10 @@
 #define PCL_PIPEMON			0x0044
 #define PCL_PCLK_ALIVE			BIT(15)
 
+#define PCL_MODE			0x8000
+#define PCL_MODE_REGEN			BIT(8)
+#define PCL_MODE_REGVAL			BIT(0)
+
 #define PCL_APP_READY_CTRL		0x8008
 #define PCL_APP_LTSSM_ENABLE		BIT(0)
 
@@ -85,6 +89,12 @@ static void uniphier_pcie_init_rc(struct uniphier_pcie_priv *priv)
 {
 	u32 val;
 
+	/* set RC MODE */
+	val = readl(priv->base + PCL_MODE);
+	val |= PCL_MODE_REGEN;
+	val &= ~PCL_MODE_REGVAL;
+	writel(val, priv->base + PCL_MODE);
+
 	/* use auxiliary power detection */
 	val = readl(priv->base + PCL_APP_PM0);
 	val |= PCL_SYS_AUX_PWR_DET;
-- 
2.7.4


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 2/2] PCI: uniphier: Add checking whether PERST# is deasserted
  2019-11-07  4:58 [PATCH 1/2] PCI: uniphier: Set mode register to host mode Kunihiko Hayashi
@ 2019-11-07  4:58 ` Kunihiko Hayashi
  2019-11-07 10:02   ` Andrew Murray
  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
  2 siblings, 1 reply; 10+ messages in thread
From: Kunihiko Hayashi @ 2019-11-07  4:58 UTC (permalink / raw)
  To: Lorenzo Pieralisi, Andrew Murray, Bjorn Helgaas
  Cc: linux-pci, linux-arm-kernel, linux-kernel, Masami Hiramatsu,
	Jassi Brar, Kunihiko Hayashi

When PERST# is asserted once, EP configuration will be initialized.
If PERST# has been already deasserted, it isn't necessary to assert
here.

This checks whether PERST# is deasserted using PCL_PINMON register,
and adds omit controlling PERST#.

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 */
+	val = readl(priv->base + PCL_PINMON);
+	if (val & PCL_PINMON_PERST_OUT)
+		return;
+
 	/* assert PERST# */
 	val = readl(priv->base + PCL_PINCTRL0);
 	val &= ~(PCL_PERST_NOE_REGVAL | PCL_PERST_OUT_REGVAL
-- 
2.7.4


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] PCI: uniphier: Set mode register to host mode
  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  9:52 ` Andrew Murray
  2019-11-21 16:49 ` Lorenzo Pieralisi
  2 siblings, 0 replies; 10+ messages in thread
From: Andrew Murray @ 2019-11-07  9:52 UTC (permalink / raw)
  To: Kunihiko Hayashi
  Cc: Lorenzo Pieralisi, Bjorn Helgaas, linux-pci, linux-arm-kernel,
	linux-kernel, Masami Hiramatsu, Jassi Brar

On Thu, Nov 07, 2019 at 01:58:14PM +0900, Kunihiko Hayashi wrote:
> In order to avoid effect of the initial mode depending on SoCs,
> this patch sets the mode register to host(RC) mode.
> 
> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
> ---
>  drivers/pci/controller/dwc/pcie-uniphier.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-uniphier.c b/drivers/pci/controller/dwc/pcie-uniphier.c
> index 3f30ee4..8fd7bad 100644
> --- a/drivers/pci/controller/dwc/pcie-uniphier.c
> +++ b/drivers/pci/controller/dwc/pcie-uniphier.c
> @@ -33,6 +33,10 @@
>  #define PCL_PIPEMON			0x0044
>  #define PCL_PCLK_ALIVE			BIT(15)
>  
> +#define PCL_MODE			0x8000
> +#define PCL_MODE_REGEN			BIT(8)
> +#define PCL_MODE_REGVAL			BIT(0)
> +
>  #define PCL_APP_READY_CTRL		0x8008
>  #define PCL_APP_LTSSM_ENABLE		BIT(0)
>  
> @@ -85,6 +89,12 @@ static void uniphier_pcie_init_rc(struct uniphier_pcie_priv *priv)
>  {
>  	u32 val;
>  
> +	/* set RC MODE */
> +	val = readl(priv->base + PCL_MODE);
> +	val |= PCL_MODE_REGEN;
> +	val &= ~PCL_MODE_REGVAL;
> +	writel(val, priv->base + PCL_MODE);
> +

Reviewed-by: Andrew Murray <andrew.murray@arm.com>

>  	/* use auxiliary power detection */
>  	val = readl(priv->base + PCL_APP_PM0);
>  	val |= PCL_SYS_AUX_PWR_DET;
> -- 
> 2.7.4
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] PCI: uniphier: Add checking whether PERST# is deasserted
  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
  0 siblings, 1 reply; 10+ messages in thread
From: Andrew Murray @ 2019-11-07 10:02 UTC (permalink / raw)
  To: Kunihiko Hayashi
  Cc: Lorenzo Pieralisi, Bjorn Helgaas, linux-pci, linux-arm-kernel,
	linux-kernel, Masami Hiramatsu, Jassi Brar

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?

> 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?

> 
> This checks whether PERST# is deasserted using PCL_PINMON register,
> and adds omit controlling PERST#.

Should this read 'and omits controlling PERST#'?

> 
> 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.

Thanks,

Andrew Murray

> +	val = readl(priv->base + PCL_PINMON);
> +	if (val & PCL_PINMON_PERST_OUT)
> +		return;
> +
>  	/* assert PERST# */
>  	val = readl(priv->base + PCL_PINCTRL0);
>  	val &= ~(PCL_PERST_NOE_REGVAL | PCL_PERST_OUT_REGVAL
> -- 
> 2.7.4
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] PCI: uniphier: Add checking whether PERST# is deasserted
  2019-11-07 10:02   ` Andrew Murray
@ 2019-11-07 11:52     ` Kunihiko Hayashi
  2019-11-07 12:46       ` Andrew Murray
  0 siblings, 1 reply; 10+ messages in thread
From: Kunihiko Hayashi @ 2019-11-07 11:52 UTC (permalink / raw)
  To: Andrew Murray
  Cc: Lorenzo Pieralisi, Bjorn Helgaas, linux-pci, linux-arm-kernel,
	linux-kernel, Masami Hiramatsu, Jassi Brar

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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] PCI: uniphier: Add checking whether PERST# is deasserted
  2019-11-07 11:52     ` Kunihiko Hayashi
@ 2019-11-07 12:46       ` Andrew Murray
  2019-11-08  7:30         ` Kunihiko Hayashi
  0 siblings, 1 reply; 10+ messages in thread
From: Andrew Murray @ 2019-11-07 12:46 UTC (permalink / raw)
  To: Kunihiko Hayashi
  Cc: Lorenzo Pieralisi, Bjorn Helgaas, linux-pci, linux-arm-kernel,
	linux-kernel, Masami Hiramatsu, Jassi Brar

On Thu, Nov 07, 2019 at 08:52:39PM +0900, Kunihiko Hayashi wrote:
> 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.
> 

OK I understand where you are coming from now. Please ensure the commit
message gives this rationale.

However, If I understand correctly, doesn't your solution only work some
of the time? What happens if you boot both machines at the same time,
and PERST# isn't asserted prior to the kernel booting?

The only way you can ensure the EP is started after the RC is initialised
is to start the EP after the RC is initialised.

I'm not sure what the solution is here, but it feels like this approach
only partially solves it.

> 
> > 
> > > 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.
> 
Thanks,

Andrew Murray

> Thank you,
> 
> ---
> Best Regards,
> Kunihiko Hayashi
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] PCI: uniphier: Add checking whether PERST# is deasserted
  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
  0 siblings, 2 replies; 10+ messages in thread
From: Kunihiko Hayashi @ 2019-11-08  7:30 UTC (permalink / raw)
  To: Andrew Murray
  Cc: Lorenzo Pieralisi, Bjorn Helgaas, linux-pci, linux-arm-kernel,
	linux-kernel, Masami Hiramatsu, Jassi Brar,
	Kishon Vijay Abraham I

Hi Andrew,
+CC Kishon

On Thu, 7 Nov 2019 12:46:17 +0000 <andrew.murray@arm.com> wrote:

> On Thu, Nov 07, 2019 at 08:52:39PM +0900, Kunihiko Hayashi wrote:
> > 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.
> > 
> 
> OK I understand where you are coming from now. Please ensure the commit
> message gives this rationale.

I'll explain about that in the commit message next.

> However, If I understand correctly, doesn't your solution only work some
> of the time? What happens if you boot both machines at the same time,
> and PERST# isn't asserted prior to the kernel booting?

I think it contains an annoying problem.

If PERST# isn't toggled prior to the kernel booting, PERST# remains asserted
and the RC driver can't access PCI bus.

As a result, this patch works and deasserts PERST# (and EP configuration will
be lost). So boot sequence needs to include deasserting PERST#.

> The only way you can ensure the EP is started after the RC is initialised
> is to start the EP after the RC is initialised.

Yes, it's the only soution for now.

> I'm not sure what the solution is here, but it feels like this approach
> only partially solves it.

Surely relying on outside of the driver doesn't seem to be a complete solution.

If there is the way that `Linux PCI endpoint framework' assumes,
I'd like to follow it, however, I can't find the other way.

Thank you,

---
Best Regards,
Kunihiko Hayashi



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] PCI: uniphier: Add checking whether PERST# is deasserted
  2019-11-08  7:30         ` Kunihiko Hayashi
@ 2019-11-08  9:59           ` Andrew Murray
  2019-11-21 16:47           ` Lorenzo Pieralisi
  1 sibling, 0 replies; 10+ messages in thread
From: Andrew Murray @ 2019-11-08  9:59 UTC (permalink / raw)
  To: Kunihiko Hayashi
  Cc: Lorenzo Pieralisi, Bjorn Helgaas, linux-pci, linux-arm-kernel,
	linux-kernel, Masami Hiramatsu, Jassi Brar,
	Kishon Vijay Abraham I

On Fri, Nov 08, 2019 at 04:30:27PM +0900, Kunihiko Hayashi wrote:
> Hi Andrew,
> +CC Kishon
> 
> On Thu, 7 Nov 2019 12:46:17 +0000 <andrew.murray@arm.com> wrote:
> 
> > On Thu, Nov 07, 2019 at 08:52:39PM +0900, Kunihiko Hayashi wrote:
> > > 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.
> > > 
> > 
> > OK I understand where you are coming from now. Please ensure the commit
> > message gives this rationale.
> 
> I'll explain about that in the commit message next.
> 
> > However, If I understand correctly, doesn't your solution only work some
> > of the time? What happens if you boot both machines at the same time,
> > and PERST# isn't asserted prior to the kernel booting?
> 
> I think it contains an annoying problem.
> 
> If PERST# isn't toggled prior to the kernel booting, PERST# remains asserted
> and the RC driver can't access PCI bus.
> 
> As a result, this patch works and deasserts PERST# (and EP configuration will
> be lost). So boot sequence needs to include deasserting PERST#.
> 
> > The only way you can ensure the EP is started after the RC is initialised
> > is to start the EP after the RC is initialised.
> 
> Yes, it's the only soution for now.
> 
> > I'm not sure what the solution is here, but it feels like this approach
> > only partially solves it.
> 
> Surely relying on outside of the driver doesn't seem to be a complete solution.
> 
> If there is the way that `Linux PCI endpoint framework' assumes,
> I'd like to follow it, however, I can't find the other way.

Indeed, this must be a common problem - many host controller drivers in the
tree toggle perst on start up.

Keen for any feedback from Kishon on this.

Thanks,

Andrew Murray

> 
> Thank you,
> 
> ---
> Best Regards,
> Kunihiko Hayashi
> 
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] PCI: uniphier: Add checking whether PERST# is deasserted
  2019-11-08  7:30         ` Kunihiko Hayashi
  2019-11-08  9:59           ` Andrew Murray
@ 2019-11-21 16:47           ` Lorenzo Pieralisi
  1 sibling, 0 replies; 10+ messages in thread
From: Lorenzo Pieralisi @ 2019-11-21 16:47 UTC (permalink / raw)
  To: Kunihiko Hayashi
  Cc: Andrew Murray, Bjorn Helgaas, linux-pci, linux-arm-kernel,
	linux-kernel, Masami Hiramatsu, Jassi Brar,
	Kishon Vijay Abraham I

On Fri, Nov 08, 2019 at 04:30:27PM +0900, Kunihiko Hayashi wrote:
> > However, If I understand correctly, doesn't your solution only work some
> > of the time? What happens if you boot both machines at the same time,
> > and PERST# isn't asserted prior to the kernel booting?
> 
> I think it contains an annoying problem.
> 
> If PERST# isn't toggled prior to the kernel booting, PERST# remains asserted
> and the RC driver can't access PCI bus.
> 
> As a result, this patch works and deasserts PERST# (and EP configuration will
> be lost). So boot sequence needs to include deasserting PERST#.

I am sorry but I have lost you. Can you explain to us why checking
that PERST# is deasserted guarantees you that:

- The EP has bootstrapped
- It is safe not to toggle it again (and also skip
  uniphier_pcie_ltssm_enable())

Please provide details of the HW configuration so that we understand
what's actually supposed to happen and why this patch fixes the
issue you are facing.

Thanks,
Lorenzo

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] PCI: uniphier: Set mode register to host mode
  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  9:52 ` [PATCH 1/2] PCI: uniphier: Set mode register to host mode Andrew Murray
@ 2019-11-21 16:49 ` Lorenzo Pieralisi
  2 siblings, 0 replies; 10+ messages in thread
From: Lorenzo Pieralisi @ 2019-11-21 16:49 UTC (permalink / raw)
  To: Kunihiko Hayashi
  Cc: Andrew Murray, Bjorn Helgaas, linux-pci, linux-arm-kernel,
	linux-kernel, Masami Hiramatsu, Jassi Brar

On Thu, Nov 07, 2019 at 01:58:14PM +0900, Kunihiko Hayashi wrote:
> In order to avoid effect of the initial mode depending on SoCs,
> this patch sets the mode register to host(RC) mode.
> 
> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
> ---
>  drivers/pci/controller/dwc/pcie-uniphier.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)

Applied to pci/uniphier, thanks.

Lorenzo

> diff --git a/drivers/pci/controller/dwc/pcie-uniphier.c b/drivers/pci/controller/dwc/pcie-uniphier.c
> index 3f30ee4..8fd7bad 100644
> --- a/drivers/pci/controller/dwc/pcie-uniphier.c
> +++ b/drivers/pci/controller/dwc/pcie-uniphier.c
> @@ -33,6 +33,10 @@
>  #define PCL_PIPEMON			0x0044
>  #define PCL_PCLK_ALIVE			BIT(15)
>  
> +#define PCL_MODE			0x8000
> +#define PCL_MODE_REGEN			BIT(8)
> +#define PCL_MODE_REGVAL			BIT(0)
> +
>  #define PCL_APP_READY_CTRL		0x8008
>  #define PCL_APP_LTSSM_ENABLE		BIT(0)
>  
> @@ -85,6 +89,12 @@ static void uniphier_pcie_init_rc(struct uniphier_pcie_priv *priv)
>  {
>  	u32 val;
>  
> +	/* set RC MODE */
> +	val = readl(priv->base + PCL_MODE);
> +	val |= PCL_MODE_REGEN;
> +	val &= ~PCL_MODE_REGVAL;
> +	writel(val, priv->base + PCL_MODE);
> +
>  	/* use auxiliary power detection */
>  	val = readl(priv->base + PCL_APP_PM0);
>  	val |= PCL_SYS_AUX_PWR_DET;
> -- 
> 2.7.4
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, back to index

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
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-07  9:52 ` [PATCH 1/2] PCI: uniphier: Set mode register to host mode Andrew Murray
2019-11-21 16:49 ` Lorenzo Pieralisi

Linux-PCI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-pci/0 linux-pci/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-pci linux-pci/ https://lore.kernel.org/linux-pci \
		linux-pci@vger.kernel.org
	public-inbox-index linux-pci

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-pci


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git