All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] Fix Altera PCIe configuration type handling
@ 2019-05-24  6:07 Ley Foon Tan
  2019-05-24  6:07 ` [PATCH 1/2] PCI: altera: Fix configuration type based on secondary number Ley Foon Tan
  2019-05-24  6:07 ` [PATCH 2/2] PCI: altera: Remove cfgrdX and cfgwrX Ley Foon Tan
  0 siblings, 2 replies; 5+ messages in thread
From: Ley Foon Tan @ 2019-05-24  6:07 UTC (permalink / raw)
  To: Bjorn Helgaas, Lorenzo Pieralisi
  Cc: linux-kernel, linux-pci, lftan.linux, Ley Foon Tan

This fix issue when access config from PCIe switch.

Stratix 10 PCIe controller does not support Type 1 to Type 0 conversion
as previous version (V1) does.

The PCIe controller need to send Type 0 config TLP if the targeting bus
matches with the secondary bus number, which is when the TLP is targeting
the immediate device on the link.

The PCIe controller send Type 1 config TLP if the targeting bus is
larger than the secondary bus, which is when the TLP is targeting the
device not immediate on the link.

Ley Foon Tan (2):
  PCI: altera: Fix configuration type based on secondary number
  PCI: altera: Remove cfgrdX and cfgwrX

 drivers/pci/controller/pcie-altera.c | 47 ++++++++++++++--------------
 1 file changed, 24 insertions(+), 23 deletions(-)

-- 
2.19.0


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

* [PATCH 1/2] PCI: altera: Fix configuration type based on secondary number
  2019-05-24  6:07 [PATCH 0/2] Fix Altera PCIe configuration type handling Ley Foon Tan
@ 2019-05-24  6:07 ` Ley Foon Tan
  2019-05-30 15:25   ` Lorenzo Pieralisi
  2019-05-24  6:07 ` [PATCH 2/2] PCI: altera: Remove cfgrdX and cfgwrX Ley Foon Tan
  1 sibling, 1 reply; 5+ messages in thread
From: Ley Foon Tan @ 2019-05-24  6:07 UTC (permalink / raw)
  To: Bjorn Helgaas, Lorenzo Pieralisi
  Cc: linux-kernel, linux-pci, lftan.linux, Ley Foon Tan

This fix issue when access config from PCIe switch.

Stratix 10 PCIe controller does not support Type 1 to Type 0 conversion
as previous version (V1) does.

The PCIe controller need to send Type 0 config TLP if the targeting bus
matches with the secondary bus number, which is when the TLP is targeting
the immediate device on the link.

The PCIe controller send Type 1 config TLP if the targeting bus is
larger than the secondary bus, which is when the TLP is targeting the
device not immediate on the link.

Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
---
 drivers/pci/controller/pcie-altera.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/pcie-altera.c b/drivers/pci/controller/pcie-altera.c
index 27222071ace7..047bcc214f9b 100644
--- a/drivers/pci/controller/pcie-altera.c
+++ b/drivers/pci/controller/pcie-altera.c
@@ -44,6 +44,8 @@
 #define S10_RP_RXCPL_STATUS		0x200C
 #define S10_RP_CFG_ADDR(pcie, reg)	\
 	(((pcie)->hip_base) + (reg) + (1 << 20))
+#define S10_RP_SECONDARY(pcie)		\
+	readb(S10_RP_CFG_ADDR(pcie, PCI_SECONDARY_BUS))
 
 /* TLP configuration type 0 and 1 */
 #define TLP_FMTTYPE_CFGRD0		0x04	/* Configuration Read Type 0 */
@@ -63,6 +65,14 @@
 	((((bus == pcie->root_bus_nr) ? pcie->pcie_data->cfgwr0		\
 				: pcie->pcie_data->cfgwr1) << 24) |	\
 				TLP_PAYLOAD_SIZE)
+#define S10_TLP_CFGRD_DW0(pcie, bus)					\
+	(((((bus) > S10_RP_SECONDARY(pcie)) ? pcie->pcie_data->cfgrd0	\
+				: pcie->pcie_data->cfgrd1) << 24) |	\
+				TLP_PAYLOAD_SIZE)
+#define S10_TLP_CFGWR_DW0(pcie, bus)					\
+	(((((bus) > S10_RP_SECONDARY(pcie)) ? pcie->pcie_data->cfgwr0	\
+				: pcie->pcie_data->cfgwr1) << 24) |	\
+				TLP_PAYLOAD_SIZE)
 #define TLP_CFG_DW1(pcie, tag, be)	\
 	(((TLP_REQ_ID(pcie->root_bus_nr,  RP_DEVFN)) << 16) | (tag << 8) | (be))
 #define TLP_CFG_DW2(bus, devfn, offset)	\
@@ -327,7 +337,11 @@ static int tlp_cfg_dword_read(struct altera_pcie *pcie, u8 bus, u32 devfn,
 {
 	u32 headers[TLP_HDR_SIZE];
 
-	headers[0] = TLP_CFGRD_DW0(pcie, bus);
+	if (pcie->pcie_data->version == ALTERA_PCIE_V1)
+		headers[0] = TLP_CFGRD_DW0(pcie, bus);
+	else
+		headers[0] = S10_TLP_CFGRD_DW0(pcie, bus);
+
 	headers[1] = TLP_CFG_DW1(pcie, TLP_READ_TAG, byte_en);
 	headers[2] = TLP_CFG_DW2(bus, devfn, where);
 
@@ -342,7 +356,11 @@ static int tlp_cfg_dword_write(struct altera_pcie *pcie, u8 bus, u32 devfn,
 	u32 headers[TLP_HDR_SIZE];
 	int ret;
 
-	headers[0] = TLP_CFGWR_DW0(pcie, bus);
+	if (pcie->pcie_data->version == ALTERA_PCIE_V1)
+		headers[0] = TLP_CFGWR_DW0(pcie, bus);
+	else
+		headers[0] = S10_TLP_CFGWR_DW0(pcie, bus);
+
 	headers[1] = TLP_CFG_DW1(pcie, TLP_WRITE_TAG, byte_en);
 	headers[2] = TLP_CFG_DW2(bus, devfn, where);
 
-- 
2.19.0


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

* [PATCH 2/2] PCI: altera: Remove cfgrdX and cfgwrX
  2019-05-24  6:07 [PATCH 0/2] Fix Altera PCIe configuration type handling Ley Foon Tan
  2019-05-24  6:07 ` [PATCH 1/2] PCI: altera: Fix configuration type based on secondary number Ley Foon Tan
@ 2019-05-24  6:07 ` Ley Foon Tan
  1 sibling, 0 replies; 5+ messages in thread
From: Ley Foon Tan @ 2019-05-24  6:07 UTC (permalink / raw)
  To: Bjorn Helgaas, Lorenzo Pieralisi
  Cc: linux-kernel, linux-pci, lftan.linux, Ley Foon Tan

No longer need cfgrdX and cfgwrX since we have separate defines for
TLP_CFG*_DW0 and S10_TLP_CFG*_DW0, so remove them.

Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
---
 drivers/pci/controller/pcie-altera.c | 33 +++++++---------------------
 1 file changed, 8 insertions(+), 25 deletions(-)

diff --git a/drivers/pci/controller/pcie-altera.c b/drivers/pci/controller/pcie-altera.c
index 047bcc214f9b..d96980a4e327 100644
--- a/drivers/pci/controller/pcie-altera.c
+++ b/drivers/pci/controller/pcie-altera.c
@@ -58,20 +58,20 @@
 #define RP_DEVFN			0
 #define TLP_REQ_ID(bus, devfn)		(((bus) << 8) | (devfn))
 #define TLP_CFGRD_DW0(pcie, bus)					\
-	((((bus == pcie->root_bus_nr) ? pcie->pcie_data->cfgrd0		\
-				: pcie->pcie_data->cfgrd1) << 24) |	\
+	((((bus == pcie->root_bus_nr) ? TLP_FMTTYPE_CFGRD0		\
+				: TLP_FMTTYPE_CFGRD1) << 24) |	\
 				TLP_PAYLOAD_SIZE)
 #define TLP_CFGWR_DW0(pcie, bus)					\
-	((((bus == pcie->root_bus_nr) ? pcie->pcie_data->cfgwr0		\
-				: pcie->pcie_data->cfgwr1) << 24) |	\
+	((((bus == pcie->root_bus_nr) ? TLP_FMTTYPE_CFGWR0		\
+				: TLP_FMTTYPE_CFGWR1) << 24) |	\
 				TLP_PAYLOAD_SIZE)
 #define S10_TLP_CFGRD_DW0(pcie, bus)					\
-	(((((bus) > S10_RP_SECONDARY(pcie)) ? pcie->pcie_data->cfgrd0	\
-				: pcie->pcie_data->cfgrd1) << 24) |	\
+	(((((bus) > S10_RP_SECONDARY(pcie)) ? TLP_FMTTYPE_CFGRD1	\
+				: TLP_FMTTYPE_CFGRD0) << 24) |	\
 				TLP_PAYLOAD_SIZE)
 #define S10_TLP_CFGWR_DW0(pcie, bus)					\
-	(((((bus) > S10_RP_SECONDARY(pcie)) ? pcie->pcie_data->cfgwr0	\
-				: pcie->pcie_data->cfgwr1) << 24) |	\
+	(((((bus) > S10_RP_SECONDARY(pcie)) ? TLP_FMTTYPE_CFGWR1	\
+				: TLP_FMTTYPE_CFGWR0) << 24) |	\
 				TLP_PAYLOAD_SIZE)
 #define TLP_CFG_DW1(pcie, tag, be)	\
 	(((TLP_REQ_ID(pcie->root_bus_nr,  RP_DEVFN)) << 16) | (tag << 8) | (be))
@@ -87,11 +87,6 @@
 
 #define DWORD_MASK			3
 
-#define S10_TLP_FMTTYPE_CFGRD0		0x05
-#define S10_TLP_FMTTYPE_CFGRD1		0x04
-#define S10_TLP_FMTTYPE_CFGWR0		0x45
-#define S10_TLP_FMTTYPE_CFGWR1		0x44
-
 enum altera_pcie_version {
 	ALTERA_PCIE_V1 = 0,
 	ALTERA_PCIE_V2,
@@ -124,10 +119,6 @@ struct altera_pcie_data {
 	const struct altera_pcie_ops *ops;
 	enum altera_pcie_version version;
 	u32 cap_offset;		/* PCIe capability structure register offset */
-	u32 cfgrd0;
-	u32 cfgrd1;
-	u32 cfgwr0;
-	u32 cfgwr1;
 };
 
 struct tlp_rp_regpair_t {
@@ -784,20 +775,12 @@ static const struct altera_pcie_data altera_pcie_1_0_data = {
 	.ops = &altera_pcie_ops_1_0,
 	.cap_offset = 0x80,
 	.version = ALTERA_PCIE_V1,
-	.cfgrd0 = TLP_FMTTYPE_CFGRD0,
-	.cfgrd1 = TLP_FMTTYPE_CFGRD1,
-	.cfgwr0 = TLP_FMTTYPE_CFGWR0,
-	.cfgwr1 = TLP_FMTTYPE_CFGWR1,
 };
 
 static const struct altera_pcie_data altera_pcie_2_0_data = {
 	.ops = &altera_pcie_ops_2_0,
 	.version = ALTERA_PCIE_V2,
 	.cap_offset = 0x70,
-	.cfgrd0 = S10_TLP_FMTTYPE_CFGRD0,
-	.cfgrd1 = S10_TLP_FMTTYPE_CFGRD1,
-	.cfgwr0 = S10_TLP_FMTTYPE_CFGWR0,
-	.cfgwr1 = S10_TLP_FMTTYPE_CFGWR1,
 };
 
 static const struct of_device_id altera_pcie_of_match[] = {
-- 
2.19.0


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

* Re: [PATCH 1/2] PCI: altera: Fix configuration type based on secondary number
  2019-05-24  6:07 ` [PATCH 1/2] PCI: altera: Fix configuration type based on secondary number Ley Foon Tan
@ 2019-05-30 15:25   ` Lorenzo Pieralisi
  2019-06-12  2:23     ` Ley Foon Tan
  0 siblings, 1 reply; 5+ messages in thread
From: Lorenzo Pieralisi @ 2019-05-30 15:25 UTC (permalink / raw)
  To: Ley Foon Tan; +Cc: Bjorn Helgaas, linux-kernel, linux-pci, lftan.linux

On Fri, May 24, 2019 at 02:07:25PM +0800, Ley Foon Tan wrote:
> This fix issue when access config from PCIe switch.
> 
> Stratix 10 PCIe controller does not support Type 1 to Type 0 conversion
> as previous version (V1) does.
> 
> The PCIe controller need to send Type 0 config TLP if the targeting bus
> matches with the secondary bus number, which is when the TLP is targeting
> the immediate device on the link.
> 
> The PCIe controller send Type 1 config TLP if the targeting bus is
> larger than the secondary bus, which is when the TLP is targeting the
> device not immediate on the link.
> 
> Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
> ---
>  drivers/pci/controller/pcie-altera.c | 22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/pcie-altera.c b/drivers/pci/controller/pcie-altera.c
> index 27222071ace7..047bcc214f9b 100644
> --- a/drivers/pci/controller/pcie-altera.c
> +++ b/drivers/pci/controller/pcie-altera.c
> @@ -44,6 +44,8 @@
>  #define S10_RP_RXCPL_STATUS		0x200C
>  #define S10_RP_CFG_ADDR(pcie, reg)	\
>  	(((pcie)->hip_base) + (reg) + (1 << 20))
> +#define S10_RP_SECONDARY(pcie)		\
> +	readb(S10_RP_CFG_ADDR(pcie, PCI_SECONDARY_BUS))
>  
>  /* TLP configuration type 0 and 1 */
>  #define TLP_FMTTYPE_CFGRD0		0x04	/* Configuration Read Type 0 */
> @@ -63,6 +65,14 @@
>  	((((bus == pcie->root_bus_nr) ? pcie->pcie_data->cfgwr0		\
>  				: pcie->pcie_data->cfgwr1) << 24) |	\
>  				TLP_PAYLOAD_SIZE)
> +#define S10_TLP_CFGRD_DW0(pcie, bus)					\
> +	(((((bus) > S10_RP_SECONDARY(pcie)) ? pcie->pcie_data->cfgrd0	\
> +				: pcie->pcie_data->cfgrd1) << 24) |	\
> +				TLP_PAYLOAD_SIZE)
> +#define S10_TLP_CFGWR_DW0(pcie, bus)					\
> +	(((((bus) > S10_RP_SECONDARY(pcie)) ? pcie->pcie_data->cfgwr0	\
> +				: pcie->pcie_data->cfgwr1) << 24) |	\
> +				TLP_PAYLOAD_SIZE)
>  #define TLP_CFG_DW1(pcie, tag, be)	\
>  	(((TLP_REQ_ID(pcie->root_bus_nr,  RP_DEVFN)) << 16) | (tag << 8) | (be))
>  #define TLP_CFG_DW2(bus, devfn, offset)	\
> @@ -327,7 +337,11 @@ static int tlp_cfg_dword_read(struct altera_pcie *pcie, u8 bus, u32 devfn,
>  {
>  	u32 headers[TLP_HDR_SIZE];
>  
> -	headers[0] = TLP_CFGRD_DW0(pcie, bus);
> +	if (pcie->pcie_data->version == ALTERA_PCIE_V1)
> +		headers[0] = TLP_CFGRD_DW0(pcie, bus);
> +	else
> +		headers[0] = S10_TLP_CFGRD_DW0(pcie, bus);
> +
>  	headers[1] = TLP_CFG_DW1(pcie, TLP_READ_TAG, byte_en);
>  	headers[2] = TLP_CFG_DW2(bus, devfn, where);
>  
> @@ -342,7 +356,11 @@ static int tlp_cfg_dword_write(struct altera_pcie *pcie, u8 bus, u32 devfn,
>  	u32 headers[TLP_HDR_SIZE];
>  	int ret;
>  
> -	headers[0] = TLP_CFGWR_DW0(pcie, bus);
> +	if (pcie->pcie_data->version == ALTERA_PCIE_V1)
> +		headers[0] = TLP_CFGWR_DW0(pcie, bus);
> +	else
> +		headers[0] = S10_TLP_CFGWR_DW0(pcie, bus);
> +

Why don't you rewrite all these macros as an eg:

static inline u32 get_tlp_header()
{}

where you can also handle the version and everything needed to
detect what header should be set-up ?

Lorenzo

>  	headers[1] = TLP_CFG_DW1(pcie, TLP_WRITE_TAG, byte_en);
>  	headers[2] = TLP_CFG_DW2(bus, devfn, where);
>  
> -- 
> 2.19.0
> 

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

* Re: [PATCH 1/2] PCI: altera: Fix configuration type based on secondary number
  2019-05-30 15:25   ` Lorenzo Pieralisi
@ 2019-06-12  2:23     ` Ley Foon Tan
  0 siblings, 0 replies; 5+ messages in thread
From: Ley Foon Tan @ 2019-06-12  2:23 UTC (permalink / raw)
  To: Lorenzo Pieralisi; +Cc: Ley Foon Tan, Bjorn Helgaas, linux-kernel, linux-pci

On Thu, May 30, 2019 at 11:25 PM Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
>
> On Fri, May 24, 2019 at 02:07:25PM +0800, Ley Foon Tan wrote:
> > This fix issue when access config from PCIe switch.
> >
> > Stratix 10 PCIe controller does not support Type 1 to Type 0 conversion
> > as previous version (V1) does.
> >
> > The PCIe controller need to send Type 0 config TLP if the targeting bus
> > matches with the secondary bus number, which is when the TLP is targeting
> > the immediate device on the link.
> >
> > The PCIe controller send Type 1 config TLP if the targeting bus is
> > larger than the secondary bus, which is when the TLP is targeting the
> > device not immediate on the link.
> >
> > Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
> > ---
> >  drivers/pci/controller/pcie-altera.c | 22 ++++++++++++++++++++--
> >  1 file changed, 20 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/pci/controller/pcie-altera.c b/drivers/pci/controller/pcie-altera.c
> > index 27222071ace7..047bcc214f9b 100644
> > --- a/drivers/pci/controller/pcie-altera.c
> > +++ b/drivers/pci/controller/pcie-altera.c
> > @@ -44,6 +44,8 @@
> >  #define S10_RP_RXCPL_STATUS          0x200C
> >  #define S10_RP_CFG_ADDR(pcie, reg)   \
> >       (((pcie)->hip_base) + (reg) + (1 << 20))
> > +#define S10_RP_SECONDARY(pcie)               \
> > +     readb(S10_RP_CFG_ADDR(pcie, PCI_SECONDARY_BUS))
> >
> >  /* TLP configuration type 0 and 1 */
> >  #define TLP_FMTTYPE_CFGRD0           0x04    /* Configuration Read Type 0 */
> > @@ -63,6 +65,14 @@
> >       ((((bus == pcie->root_bus_nr) ? pcie->pcie_data->cfgwr0         \
> >                               : pcie->pcie_data->cfgwr1) << 24) |     \
> >                               TLP_PAYLOAD_SIZE)
> > +#define S10_TLP_CFGRD_DW0(pcie, bus)                                 \
> > +     (((((bus) > S10_RP_SECONDARY(pcie)) ? pcie->pcie_data->cfgrd0   \
> > +                             : pcie->pcie_data->cfgrd1) << 24) |     \
> > +                             TLP_PAYLOAD_SIZE)
> > +#define S10_TLP_CFGWR_DW0(pcie, bus)                                 \
> > +     (((((bus) > S10_RP_SECONDARY(pcie)) ? pcie->pcie_data->cfgwr0   \
> > +                             : pcie->pcie_data->cfgwr1) << 24) |     \
> > +                             TLP_PAYLOAD_SIZE)
> >  #define TLP_CFG_DW1(pcie, tag, be)   \
> >       (((TLP_REQ_ID(pcie->root_bus_nr,  RP_DEVFN)) << 16) | (tag << 8) | (be))
> >  #define TLP_CFG_DW2(bus, devfn, offset)      \
> > @@ -327,7 +337,11 @@ static int tlp_cfg_dword_read(struct altera_pcie *pcie, u8 bus, u32 devfn,
> >  {
> >       u32 headers[TLP_HDR_SIZE];
> >
> > -     headers[0] = TLP_CFGRD_DW0(pcie, bus);
> > +     if (pcie->pcie_data->version == ALTERA_PCIE_V1)
> > +             headers[0] = TLP_CFGRD_DW0(pcie, bus);
> > +     else
> > +             headers[0] = S10_TLP_CFGRD_DW0(pcie, bus);
> > +
> >       headers[1] = TLP_CFG_DW1(pcie, TLP_READ_TAG, byte_en);
> >       headers[2] = TLP_CFG_DW2(bus, devfn, where);
> >
> > @@ -342,7 +356,11 @@ static int tlp_cfg_dword_write(struct altera_pcie *pcie, u8 bus, u32 devfn,
> >       u32 headers[TLP_HDR_SIZE];
> >       int ret;
> >
> > -     headers[0] = TLP_CFGWR_DW0(pcie, bus);
> > +     if (pcie->pcie_data->version == ALTERA_PCIE_V1)
> > +             headers[0] = TLP_CFGWR_DW0(pcie, bus);
> > +     else
> > +             headers[0] = S10_TLP_CFGWR_DW0(pcie, bus);
> > +
>
> Why don't you rewrite all these macros as an eg:
>
> static inline u32 get_tlp_header()
> {}
>
> where you can also handle the version and everything needed to
> detect what header should be set-up ?
>
Okay, will change this.

Thanks.

Regards
Ley Foon

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

end of thread, other threads:[~2019-06-12  2:23 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-24  6:07 [PATCH 0/2] Fix Altera PCIe configuration type handling Ley Foon Tan
2019-05-24  6:07 ` [PATCH 1/2] PCI: altera: Fix configuration type based on secondary number Ley Foon Tan
2019-05-30 15:25   ` Lorenzo Pieralisi
2019-06-12  2:23     ` Ley Foon Tan
2019-05-24  6:07 ` [PATCH 2/2] PCI: altera: Remove cfgrdX and cfgwrX Ley Foon Tan

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.