[PATCH u-boot-marvell 0/9] a38x serdes cleanup

newer
[PATCH u-boot-marvell v2 0/6]...

older
[PATCH 1/3] phy: marvell: a3700:...

Marek Behún

24 Sep 2021 24 Sep '21

10:59 p.m.

Hi Stefan,

this series by Pali cleans unneeded code in arch/arm/mach-mvebu/serdes/a38x.

Pali studied the code, added comments about what the code does, and then removed unneeded parts, wich explanations in commits.

Marek

Pali Rohár (9): arm: mvebu: a38x: serdes: Add comments and use macros in PCIe code arm: mvebu: a38x: serdes: Remove duplicate macro SOC_CTRL_REG arm: mvebu: a38x: serdes: Add comments for hws_pex_config() code arm: mvebu: a38x: serdes: Don't overwrite read-only SAR PCIe registers arm: mvebu: a38x: serdes: Don't set PCIe Common Clock Configuration arm: mvebu: a38x: serdes: Don't overwrite PCI device ID arm: mvebu: a38x: serdes: Don't configure PCIe cards in SerDes init code arm: mvebu: a38x: serdes: Remove unused PCIe macros and functions arm: mvebu: a38x: serdes: Update comment about PCIE*_ENABLE_* defines

arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c | 297 +----------------- arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.h | 68 +--- .../serdes/a38x/high_speed_env_spec.c | 42 +-- 3 files changed, 22 insertions(+), 385 deletions(-)

-- 2.32.0

Show replies by date

Marek Behún

24 Sep 24 Sep

10:59 p.m.

New subject: [PATCH u-boot-marvell 1/9] arm: mvebu: a38x: serdes: Add comments and use macros in PCIe code

From: Pali Rohár pali@kernel.org

Replace magic register offsets by macros to make code more readable. Add comments about what this code is doing.

Signed-off-by: Pali Rohár pali@kernel.org Reviewed-by: Marek Behún marek.behun@nic.cz --- .../serdes/a38x/high_speed_env_spec.c | 37 +++++++++++++------ 1 file changed, 25 insertions(+), 12 deletions(-)

diff --git a/arch/arm/mach-mvebu/serdes/a38x/high_speed_env_spec.c b/arch/arm/mach-mvebu/serdes/a38x/high_speed_env_spec.c index 3b41c7d49b..09192acef2 100644 --- a/arch/arm/mach-mvebu/serdes/a38x/high_speed_env_spec.c +++ b/arch/arm/mach-mvebu/serdes/a38x/high_speed_env_spec.c @@ -1723,31 +1723,44 @@ int serdes_power_up_ctrl(u32 serdes_num, int serdes_power_up, reg_data &= ~0x4000; reg_write(SOC_CONTROL_REG1, reg_data);

- reg_data = - reg_read(((PEX_IF_REGS_BASE(pex_idx)) + - 0x6c)); + /* Set Maximum Link Width to X1 or X4 */ + reg_data = reg_read(PEX_CFG_DIRECT_ACCESS( + pex_idx, + PEX_LINK_CAPABILITY_REG)); reg_data &= ~0x3f0; if (is_pex_by1 == 1) reg_data |= 0x10; else reg_data |= 0x40; - reg_write(((PEX_IF_REGS_BASE(pex_idx)) + 0x6c), + reg_write(PEX_CFG_DIRECT_ACCESS( + pex_idx, + PEX_LINK_CAPABILITY_REG), reg_data);

- reg_data = - reg_read(((PEX_IF_REGS_BASE(pex_idx)) + - 0x6c)); + /* Set Maximum Link Speed to 5 GT/s */ + reg_data = reg_read(PEX_CFG_DIRECT_ACCESS( + pex_idx, + PEX_LINK_CAPABILITY_REG)); reg_data &= ~0xf; reg_data |= 0x2; - reg_write(((PEX_IF_REGS_BASE(pex_idx)) + 0x6c), + reg_write(PEX_CFG_DIRECT_ACCESS( + pex_idx, + PEX_LINK_CAPABILITY_REG), reg_data);

- reg_data = - reg_read(((PEX_IF_REGS_BASE(pex_idx)) + - 0x70)); + /* + * Set Common Clock Configuration to indicates + * that both devices on the link use a + * distributed common reference clock. + */ + reg_data = reg_read(PEX_CFG_DIRECT_ACCESS( + pex_idx, + PEX_LINK_CTRL_STAT_REG)); reg_data &= ~0x40; reg_data |= 0x40; - reg_write(((PEX_IF_REGS_BASE(pex_idx)) + 0x70), + reg_write(PEX_CFG_DIRECT_ACCESS( + pex_idx, + PEX_LINK_CTRL_STAT_REG), reg_data); }

-- 2.32.0

Stefan Roese

8 Oct 8 Oct

8:24 a.m.

New subject: [PATCH u-boot-marvell 1/9] arm: mvebu: a38x: serdes: Add comments and use macros in PCIe code

On 24.09.21 22:59, Marek Behún wrote:

...

From: Pali Rohár pali@kernel.org

Replace magic register offsets by macros to make code more readable. Add comments about what this code is doing.

Signed-off-by: Pali Rohár pali@kernel.org Reviewed-by: Marek Behún marek.behun@nic.cz

Reviewed-by: Stefan Roese sr@denx.de

Thanks, Stefan

...

.../serdes/a38x/high_speed_env_spec.c | 37 +++++++++++++------ 1 file changed, 25 insertions(+), 12 deletions(-)

```
		reg_data =
```

		    reg_read(((PEX_IF_REGS_BASE(pex_idx)) +

```
			      0x6c));
```

		/* Set Maximum Link Width to X1 or X4 */

		reg_data = reg_read(PEX_CFG_DIRECT_ACCESS(

```
				     pex_idx,
```

				     PEX_LINK_CAPABILITY_REG));
	reg_data &= ~0x3f0;
	if (is_pex_by1 == 1)
		reg_data |= 0x10;
	else
		reg_data |= 0x40;

		reg_write(((PEX_IF_REGS_BASE(pex_idx)) + 0x6c),

```
		reg_write(PEX_CFG_DIRECT_ACCESS(
```
```
			   pex_idx,
```

			   PEX_LINK_CAPABILITY_REG),
		  reg_data);

```
		reg_data =
```

		    reg_read(((PEX_IF_REGS_BASE(pex_idx)) +

```
			      0x6c));
```

		/* Set Maximum Link Speed to 5 GT/s */

		reg_data = reg_read(PEX_CFG_DIRECT_ACCESS(

```
				     pex_idx,
```

				     PEX_LINK_CAPABILITY_REG));
	reg_data &= ~0xf;
	reg_data |= 0x2;

		reg_write(((PEX_IF_REGS_BASE(pex_idx)) + 0x6c),

```
		reg_write(PEX_CFG_DIRECT_ACCESS(
```
```
			   pex_idx,
```

			   PEX_LINK_CAPABILITY_REG),
		  reg_data);

```
		reg_data =
```

		    reg_read(((PEX_IF_REGS_BASE(pex_idx)) +

```
			      0x70));
```

```
		/*
```

		 * Set Common Clock Configuration to indicates

		 * that both devices on the link use a

		 * distributed common reference clock.

```
		 */
```

		reg_data = reg_read(PEX_CFG_DIRECT_ACCESS(

```
				     pex_idx,
```

				     PEX_LINK_CTRL_STAT_REG));
	reg_data &= ~0x40;
	reg_data |= 0x40;

		reg_write(((PEX_IF_REGS_BASE(pex_idx)) + 0x70),

```
		reg_write(PEX_CFG_DIRECT_ACCESS(
```
```
			   pex_idx,
```

			   PEX_LINK_CTRL_STAT_REG),
		  reg_data);
}

Viele Grüße, Stefan

-- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

Marek Behún

24 Sep 24 Sep

10:59 p.m.

New subject: [PATCH u-boot-marvell 2/9] arm: mvebu: a38x: serdes: Remove duplicate macro SOC_CTRL_REG

From: Pali Rohár pali@kernel.org

SoC Control 1 Register (offset 0x18204) is already defined by macro SOC_CONTROL_REG1.

Use macro SOC_CONTROL_REG1 instead of macro SOC_CTRL_REG in ctrl_pex.c code and remove the other definition.

Signed-off-by: Pali Rohár pali@kernel.org Reviewed-by: Marek Behún marek.behun@nic.cz --- arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c | 4 ++-- arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.h | 4 ---- 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c b/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c index adef3331a7..717bcfb29c 100644 --- a/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c +++ b/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c @@ -49,7 +49,7 @@ int hws_pex_config(const struct serdes_map *serdes_map, u8 count) reg_write(PEX_CAPABILITIES_REG(pex_idx), tmp); }

- tmp = reg_read(SOC_CTRL_REG); + tmp = reg_read(SOC_CONTROL_REG1); tmp &= ~0x03;

for (idx = 0; idx < count; idx++) { @@ -79,7 +79,7 @@ int hws_pex_config(const struct serdes_map *serdes_map, u8 count) } }

- reg_write(SOC_CTRL_REG, tmp); + reg_write(SOC_CONTROL_REG1, tmp);

/* Support gen1/gen2 */ DEBUG_INIT_FULL_S("Support gen1/gen2\n"); diff --git a/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.h b/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.h index 3f30b6bf97..a882d24208 100644 --- a/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.h +++ b/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.h @@ -14,10 +14,6 @@ /* PCI Express Control and Status Registers */ #define MAX_PEX_BUSSES 256

-#define MISC_REGS_OFFSET 0x18200 -#define MV_MISC_REGS_BASE MISC_REGS_OFFSET -#define SOC_CTRL_REG (MV_MISC_REGS_BASE + 0x4) - #define PEX_IF_REGS_OFFSET(if) ((if) > 0 ? \ (0x40000 + ((if) - 1) * 0x4000) : \ 0x80000)

-- 2.32.0

Stefan Roese

8 Oct 8 Oct

8:26 a.m.

New subject: [PATCH u-boot-marvell 2/9] arm: mvebu: a38x: serdes: Remove duplicate macro SOC_CTRL_REG

On 24.09.21 22:59, Marek Behún wrote:

...

From: Pali Rohár pali@kernel.org

SoC Control 1 Register (offset 0x18204) is already defined by macro SOC_CONTROL_REG1.

Use macro SOC_CONTROL_REG1 instead of macro SOC_CTRL_REG in ctrl_pex.c code and remove the other definition.

Signed-off-by: Pali Rohár pali@kernel.org Reviewed-by: Marek Behún marek.behun@nic.cz

Reviewed-by: Stefan Roese sr@denx.de

Thanks, Stefan

...

arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c | 4 ++-- arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.h | 4 ---- 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c b/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c index adef3331a7..717bcfb29c 100644 --- a/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c +++ b/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c @@ -49,7 +49,7 @@ int hws_pex_config(const struct serdes_map *serdes_map, u8 count) reg_write(PEX_CAPABILITIES_REG(pex_idx), tmp); }

tmp = reg_read(SOC_CTRL_REG);

tmp = reg_read(SOC_CONTROL_REG1); tmp &= ~0x03;

for (idx = 0; idx < count; idx++) {

@@ -79,7 +79,7 @@ int hws_pex_config(const struct serdes_map *serdes_map, u8 count) } }

reg_write(SOC_CTRL_REG, tmp);

reg_write(SOC_CONTROL_REG1, tmp);

/* Support gen1/gen2 */ DEBUG_INIT_FULL_S("Support gen1/gen2\n");

diff --git a/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.h b/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.h index 3f30b6bf97..a882d24208 100644 --- a/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.h +++ b/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.h @@ -14,10 +14,6 @@ /* PCI Express Control and Status Registers */ #define MAX_PEX_BUSSES 256

-#define MISC_REGS_OFFSET 0x18200 -#define MV_MISC_REGS_BASE MISC_REGS_OFFSET -#define SOC_CTRL_REG (MV_MISC_REGS_BASE + 0x4)

#define PEX_IF_REGS_OFFSET(if) ((if) > 0 ? \ (0x40000 + ((if) - 1) * 0x4000) : \ 0x80000)

Viele Grüße, Stefan

Marek Behún

24 Sep 24 Sep

10:59 p.m.

New subject: [PATCH u-boot-marvell 3/9] arm: mvebu: a38x: serdes: Add comments for hws_pex_config() code

From: Pali Rohár pali@kernel.org

Add comments to understand what this magic code is doing.

Signed-off-by: Pali Rohár pali@kernel.org Reviewed-by: Marek Behún marek.behun@nic.cz --- arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c b/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c index 717bcfb29c..0eb31d589c 100644 --- a/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c +++ b/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c @@ -42,6 +42,7 @@ int hws_pex_config(const struct serdes_map *serdes_map, u8 count) continue; }

+ /* Set Device/Port Type to RootComplex */ pex_idx = serdes_type - PEX0; tmp = reg_read(PEX_CAPABILITIES_REG(pex_idx)); tmp &= ~(0xf << 20); @@ -122,12 +123,18 @@ int hws_pex_config(const struct serdes_map *serdes_map, u8 count) }

next_busno++; + + /* + * Read maximum link speed. It must be 0x2 (5.0 GT/s) as this + * value was set in serdes_power_up_ctrl() function. + */ temp_pex_reg = reg_read((PEX_CFG_DIRECT_ACCESS (pex_idx, PEX_LINK_CAPABILITY_REG))); temp_pex_reg &= 0xf; if (temp_pex_reg != 0x2) continue;

+ /* Read negotiated link speed */ temp_reg = (reg_read(PEX_CFG_DIRECT_ACCESS( pex_idx, PEX_LINK_CTRL_STAT_REG)) & @@ -155,6 +162,7 @@ int hws_pex_config(const struct serdes_map *serdes_map, u8 count) continue; }

+ /* Find start of the PCI Express Capability registers */ while ((pex_config_read(pex_idx, first_busno, 0, 0, addr) & 0xff) != 0x10) { addr = (pex_config_read(pex_idx, first_busno, 0, @@ -173,11 +181,15 @@ int hws_pex_config(const struct serdes_map *serdes_map, u8 count) tmp = reg_read(PEX_LINK_CTRL_STATUS2_REG(pex_idx)); DEBUG_RD_REG(PEX_LINK_CTRL_STATUS2_REG(pex_idx), tmp); tmp &= ~(BIT(0) | BIT(1)); - tmp |= BIT(1); + tmp |= BIT(1); /* Force Target Link Speed to 5.0 GT/s */ tmp |= BIT(6); /* Select Deemphasize (-3.5d_b) */ reg_write(PEX_LINK_CTRL_STATUS2_REG(pex_idx), tmp); DEBUG_WR_REG(PEX_LINK_CTRL_STATUS2_REG(pex_idx), tmp);

+ /* + * Enable Auto Speed change. When set, link will issue link + * speed change to max possible link speed. + */ tmp = reg_read(PEX_CTRL_REG(pex_idx)); DEBUG_RD_REG(PEX_CTRL_REG(pex_idx), tmp); tmp |= BIT(10);

-- 2.32.0

Stefan Roese

8 Oct 8 Oct

8:27 a.m.

New subject: [PATCH u-boot-marvell 3/9] arm: mvebu: a38x: serdes: Add comments for hws_pex_config() code

On 24.09.21 22:59, Marek Behún wrote:

...

From: Pali Rohár pali@kernel.org

Add comments to understand what this magic code is doing.

Signed-off-by: Pali Rohár pali@kernel.org Reviewed-by: Marek Behún marek.behun@nic.cz

Reviewed-by: Stefan Roese sr@denx.de

Thanks, Stefan

...

arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c b/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c index 717bcfb29c..0eb31d589c 100644 --- a/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c +++ b/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c @@ -42,6 +42,7 @@ int hws_pex_config(const struct serdes_map *serdes_map, u8 count) continue; }
/* Set Device/Port Type to RootComplex */
pex_idx = serdes_type - PEX0; tmp = reg_read(PEX_CAPABILITIES_REG(pex_idx)); tmp &= ~(0xf << 20);
@@ -122,12 +123,18 @@ int hws_pex_config(const struct serdes_map *serdes_map, u8 count) }
next_busno++;
/*
 * Read maximum link speed. It must be 0x2 (5.0 GT/s) as this
 * value was set in serdes_power_up_ctrl() function.
 */
temp_pex_reg = reg_read((PEX_CFG_DIRECT_ACCESS (pex_idx, PEX_LINK_CAPABILITY_REG))); temp_pex_reg &= 0xf; if (temp_pex_reg != 0x2) continue;
/* Read negotiated link speed */
temp_reg = (reg_read(PEX_CFG_DIRECT_ACCESS( pex_idx, PEX_LINK_CTRL_STAT_REG)) &
@@ -155,6 +162,7 @@ int hws_pex_config(const struct serdes_map *serdes_map, u8 count) continue; }
/* Find start of the PCI Express Capability registers */
while ((pex_config_read(pex_idx, first_busno, 0, 0, addr) & 0xff) != 0x10) { addr = (pex_config_read(pex_idx, first_busno, 0,
@@ -173,11 +181,15 @@ int hws_pex_config(const struct serdes_map *serdes_map, u8 count) tmp = reg_read(PEX_LINK_CTRL_STATUS2_REG(pex_idx)); DEBUG_RD_REG(PEX_LINK_CTRL_STATUS2_REG(pex_idx), tmp); tmp &= ~(BIT(0) | BIT(1));
tmp |= BIT(1);
tmp |= BIT(1);	/* Force Target Link Speed to 5.0 GT/s */
tmp |= BIT(6); /* Select Deemphasize (-3.5d_b) */ reg_write(PEX_LINK_CTRL_STATUS2_REG(pex_idx), tmp); DEBUG_WR_REG(PEX_LINK_CTRL_STATUS2_REG(pex_idx), tmp);
/*
 * Enable Auto Speed change. When set, link will issue link
 * speed change to max possible link speed.
 */
tmp = reg_read(PEX_CTRL_REG(pex_idx)); DEBUG_RD_REG(PEX_CTRL_REG(pex_idx), tmp); tmp |= BIT(10);

Viele Grüße, Stefan

Marek Behún

24 Sep 24 Sep

10:59 p.m.

New subject: [PATCH u-boot-marvell 4/9] arm: mvebu: a38x: serdes: Don't overwrite read-only SAR PCIe registers

From: Pali Rohár pali@kernel.org

Device/Port Type bits of PCIe Root Port PCI Express Capabilities Register are read-only SAR registers and are initialized according to current mode configured by PCIe controller. Changing PCIe controller mode (from Root Complex mode to Endpoint mode or the other way) is possible via PCI Express Control Register (offset 0x41A00), bit 1 (ConfRoot Complex). This has to be done in PCIe controller driver (in our case pci_mvebu.c). Note that default mode is Root Complex.

Maximum Link Speed bits of PCIe Root Port Link Capabilities Register are platform specific and overwriting them does not make sense. They are set by PCIe controller according to current SerDes configuration. For A38x it is 5.0 GT/s if SerDes supports appropriate speed.

Maximum Link Width bits of PCIe Root Port Link Capabilities Register are read-only SAR registers, but unfortunately if this is not set correctly here, then access PCI config space of the endpoint card behind this Root Port does not work.

Signed-off-by: Pali Rohár pali@kernel.org Reviewed-by: Marek Behún marek.behun@nic.cz --- arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c | 22 ---------- arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.h | 4 ++ .../serdes/a38x/high_speed_env_spec.c | 40 +++++++------------ 3 files changed, 19 insertions(+), 47 deletions(-)

diff --git a/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c b/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c index 0eb31d589c..7c18df8113 100644 --- a/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c +++ b/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c @@ -28,28 +28,6 @@ int hws_pex_config(const struct serdes_map *serdes_map, u8 count)

DEBUG_INIT_FULL_S("\n### hws_pex_config ###\n");

- for (idx = 0; idx < count; idx++) { - serdes_type = serdes_map[idx].serdes_type; - /* configuration for PEX only */ - if ((serdes_type != PEX0) && (serdes_type != PEX1) && - (serdes_type != PEX2) && (serdes_type != PEX3)) - continue; - - if ((serdes_type != PEX0) && - ((serdes_map[idx].serdes_mode == PEX_ROOT_COMPLEX_X4) || - (serdes_map[idx].serdes_mode == PEX_END_POINT_X4))) { - /* for PEX by4 - relevant for the first port only */ - continue; - } - - /* Set Device/Port Type to RootComplex */ - pex_idx = serdes_type - PEX0; - tmp = reg_read(PEX_CAPABILITIES_REG(pex_idx)); - tmp &= ~(0xf << 20); - tmp |= (0x4 << 20); - reg_write(PEX_CAPABILITIES_REG(pex_idx), tmp); - } - tmp = reg_read(SOC_CONTROL_REG1); tmp &= ~0x03;

diff --git a/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.h b/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.h index a882d24208..5d70166fc5 100644 --- a/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.h +++ b/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.h @@ -6,8 +6,12 @@ #ifndef _CTRL_PEX_H #define _CTRL_PEX_H

+#include <pci.h> #include "high_speed_env_spec.h"

+/* Direct access to PEX0 Root Port's PCIe Capability structure */ +#define PEX0_RP_PCIE_CFG_OFFSET (0x00080000 + 0x60) + /* Sample at Reset */ #define MPP_SAMPLE_AT_RESET(id) (0xe4200 + (id * 4))

diff --git a/arch/arm/mach-mvebu/serdes/a38x/high_speed_env_spec.c b/arch/arm/mach-mvebu/serdes/a38x/high_speed_env_spec.c index 09192acef2..a712fa8994 100644 --- a/arch/arm/mach-mvebu/serdes/a38x/high_speed_env_spec.c +++ b/arch/arm/mach-mvebu/serdes/a38x/high_speed_env_spec.c @@ -1714,7 +1714,7 @@ int serdes_power_up_ctrl(u32 serdes_num, int serdes_power_up, (serdes_mode == PEX_END_POINT_X1); pex_idx = serdes_type - PEX0;

- if ((is_pex_by1 == 1) || (serdes_type == PEX0)) { + if (serdes_type == PEX0) { /* For PEX by 4, init only the PEX 0 */ reg_data = reg_read(SOC_CONTROL_REG1); if (is_pex_by1 == 1) @@ -1723,30 +1723,20 @@ int serdes_power_up_ctrl(u32 serdes_num, int serdes_power_up, reg_data &= ~0x4000; reg_write(SOC_CONTROL_REG1, reg_data);

- /* Set Maximum Link Width to X1 or X4 */ - reg_data = reg_read(PEX_CFG_DIRECT_ACCESS( - pex_idx, - PEX_LINK_CAPABILITY_REG)); - reg_data &= ~0x3f0; - if (is_pex_by1 == 1) - reg_data |= 0x10; - else - reg_data |= 0x40; - reg_write(PEX_CFG_DIRECT_ACCESS( - pex_idx, - PEX_LINK_CAPABILITY_REG), - reg_data); - - /* Set Maximum Link Speed to 5 GT/s */ - reg_data = reg_read(PEX_CFG_DIRECT_ACCESS( - pex_idx, - PEX_LINK_CAPABILITY_REG)); - reg_data &= ~0xf; - reg_data |= 0x2; - reg_write(PEX_CFG_DIRECT_ACCESS( - pex_idx, - PEX_LINK_CAPABILITY_REG), - reg_data); + /* + * Set Maximum Link Width to X1 or X4 in Root + * Port's PCIe Link Capability register. + * This register is read-only but if is not set + * correctly then access to PCI config space of + * endpoint card behind this Root Port does not + * work. + */ + reg_data = reg_read(PEX0_RP_PCIE_CFG_OFFSET + + PCI_EXP_LNKCAP); + reg_data &= ~PCI_EXP_LNKCAP_MLW; + reg_data |= (is_pex_by1 ? 1 : 4) << 4; + reg_write(PEX0_RP_PCIE_CFG_OFFSET + + PCI_EXP_LNKCAP, reg_data);

/* * Set Common Clock Configuration to indicates

-- 2.32.0

Stefan Roese

8 Oct 8 Oct

8:27 a.m.

New subject: [PATCH u-boot-marvell 4/9] arm: mvebu: a38x: serdes: Don't overwrite read-only SAR PCIe registers

On 24.09.21 22:59, Marek Behún wrote:

...

From: Pali Rohár pali@kernel.org

Device/Port Type bits of PCIe Root Port PCI Express Capabilities Register are read-only SAR registers and are initialized according to current mode configured by PCIe controller. Changing PCIe controller mode (from Root Complex mode to Endpoint mode or the other way) is possible via PCI Express Control Register (offset 0x41A00), bit 1 (ConfRoot Complex). This has to be done in PCIe controller driver (in our case pci_mvebu.c). Note that default mode is Root Complex.

Maximum Link Speed bits of PCIe Root Port Link Capabilities Register are platform specific and overwriting them does not make sense. They are set by PCIe controller according to current SerDes configuration. For A38x it is 5.0 GT/s if SerDes supports appropriate speed.

Maximum Link Width bits of PCIe Root Port Link Capabilities Register are read-only SAR registers, but unfortunately if this is not set correctly here, then access PCI config space of the endpoint card behind this Root Port does not work.

Signed-off-by: Pali Rohár pali@kernel.org Reviewed-by: Marek Behún marek.behun@nic.cz

Reviewed-by: Stefan Roese sr@denx.de

Thanks, Stefan

...

arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c | 22 ---------- arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.h | 4 ++ .../serdes/a38x/high_speed_env_spec.c | 40 +++++++------------ 3 files changed, 19 insertions(+), 47 deletions(-)

diff --git a/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c b/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c index 0eb31d589c..7c18df8113 100644 --- a/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c +++ b/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c @@ -28,28 +28,6 @@ int hws_pex_config(const struct serdes_map *serdes_map, u8 count)

DEBUG_INIT_FULL_S("\n### hws_pex_config ###\n");
for (idx = 0; idx < count; idx++) {
serdes_type = serdes_map[idx].serdes_type;
/* configuration for PEX only */
if ((serdes_type != PEX0) && (serdes_type != PEX1) &&
    (serdes_type != PEX2) && (serdes_type != PEX3))
	continue;
if ((serdes_type != PEX0) &&
    ((serdes_map[idx].serdes_mode == PEX_ROOT_COMPLEX_X4) ||
     (serdes_map[idx].serdes_mode == PEX_END_POINT_X4))) {
	/* for PEX by4 - relevant for the first port only */
	continue;
}
/* Set Device/Port Type to RootComplex */
pex_idx = serdes_type - PEX0;
tmp = reg_read(PEX_CAPABILITIES_REG(pex_idx));
tmp &= ~(0xf << 20);
tmp |= (0x4 << 20);
reg_write(PEX_CAPABILITIES_REG(pex_idx), tmp);
}

tmp = reg_read(SOC_CONTROL_REG1); tmp &= ~0x03;
diff --git a/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.h b/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.h index a882d24208..5d70166fc5 100644 --- a/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.h +++ b/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.h @@ -6,8 +6,12 @@ #ifndef _CTRL_PEX_H #define _CTRL_PEX_H

+#include <pci.h> #include "high_speed_env_spec.h"

+/* Direct access to PEX0 Root Port's PCIe Capability structure */ +#define PEX0_RP_PCIE_CFG_OFFSET (0x00080000 + 0x60)

/* Sample at Reset */ #define MPP_SAMPLE_AT_RESET(id) (0xe4200 + (id * 4))

diff --git a/arch/arm/mach-mvebu/serdes/a38x/high_speed_env_spec.c b/arch/arm/mach-mvebu/serdes/a38x/high_speed_env_spec.c index 09192acef2..a712fa8994 100644 --- a/arch/arm/mach-mvebu/serdes/a38x/high_speed_env_spec.c +++ b/arch/arm/mach-mvebu/serdes/a38x/high_speed_env_spec.c @@ -1714,7 +1714,7 @@ int serdes_power_up_ctrl(u32 serdes_num, int serdes_power_up, (serdes_mode == PEX_END_POINT_X1); pex_idx = serdes_type - PEX0;
	if ((is_pex_by1 == 1) || (serdes_type == PEX0)) {
	if (serdes_type == PEX0) {
	/* For PEX by 4, init only the PEX 0 */
	reg_data = reg_read(SOC_CONTROL_REG1);
	if (is_pex_by1 == 1)
@@ -1723,30 +1723,20 @@ int serdes_power_up_ctrl(u32 serdes_num, int serdes_power_up, reg_data &= ~0x4000; reg_write(SOC_CONTROL_REG1, reg_data);
		/* Set Maximum Link Width to X1 or X4 */
		reg_data = reg_read(PEX_CFG_DIRECT_ACCESS(
				     pex_idx,
				     PEX_LINK_CAPABILITY_REG));
		reg_data &= ~0x3f0;
		if (is_pex_by1 == 1)
			reg_data |= 0x10;
		else
			reg_data |= 0x40;
		reg_write(PEX_CFG_DIRECT_ACCESS(
			   pex_idx,
			   PEX_LINK_CAPABILITY_REG),
			  reg_data);
		/* Set Maximum Link Speed to 5 GT/s */
		reg_data = reg_read(PEX_CFG_DIRECT_ACCESS(
				     pex_idx,
				     PEX_LINK_CAPABILITY_REG));
		reg_data &= ~0xf;
		reg_data |= 0x2;
		reg_write(PEX_CFG_DIRECT_ACCESS(
			   pex_idx,
			   PEX_LINK_CAPABILITY_REG),
			  reg_data);
		/*
		 * Set Maximum Link Width to X1 or X4 in Root
		 * Port's PCIe Link Capability register.
		 * This register is read-only but if is not set
		 * correctly then access to PCI config space of
		 * endpoint card behind this Root Port does not
		 * work.
		 */
		reg_data = reg_read(PEX0_RP_PCIE_CFG_OFFSET +
				    PCI_EXP_LNKCAP);
		reg_data &= ~PCI_EXP_LNKCAP_MLW;
		reg_data |= (is_pex_by1 ? 1 : 4) << 4;
		reg_write(PEX0_RP_PCIE_CFG_OFFSET +
			  PCI_EXP_LNKCAP, reg_data);

	/*
	 * Set Common Clock Configuration to indicates

Viele Grüße, Stefan

Marek Behún

24 Sep 24 Sep

10:59 p.m.

New subject: [PATCH u-boot-marvell 5/9] arm: mvebu: a38x: serdes: Don't set PCIe Common Clock Configuration

From: Pali Rohár pali@kernel.org

Enabling Common Clock Configuration bit in PCIe Root Port Link Control Register should not be done unconditionally. It is enabled by operating system as part of ASPM. Also after enabling Common Clock Configuration it is required to do more work, like retraining link. Some cards may be broken due to this incomplete Common Clock Configuration and some cards are broken and do not support ASPM at all.

Remove this incomplete code for Common Clock Configuration. It really should not be done in SerDes code as it is not related to SerDes, but to PCIe subsystem.

Signed-off-by: Pali Rohár pali@kernel.org Reviewed-by: Marek Behún marek.behun@nic.cz --- .../mach-mvebu/serdes/a38x/high_speed_env_spec.c | 15 --------------- 1 file changed, 15 deletions(-)

diff --git a/arch/arm/mach-mvebu/serdes/a38x/high_speed_env_spec.c b/arch/arm/mach-mvebu/serdes/a38x/high_speed_env_spec.c index a712fa8994..824f4d3e3d 100644 --- a/arch/arm/mach-mvebu/serdes/a38x/high_speed_env_spec.c +++ b/arch/arm/mach-mvebu/serdes/a38x/high_speed_env_spec.c @@ -1737,21 +1737,6 @@ int serdes_power_up_ctrl(u32 serdes_num, int serdes_power_up, reg_data |= (is_pex_by1 ? 1 : 4) << 4; reg_write(PEX0_RP_PCIE_CFG_OFFSET + PCI_EXP_LNKCAP, reg_data); - - /* - * Set Common Clock Configuration to indicates - * that both devices on the link use a - * distributed common reference clock. - */ - reg_data = reg_read(PEX_CFG_DIRECT_ACCESS( - pex_idx, - PEX_LINK_CTRL_STAT_REG)); - reg_data &= ~0x40; - reg_data |= 0x40; - reg_write(PEX_CFG_DIRECT_ACCESS( - pex_idx, - PEX_LINK_CTRL_STAT_REG), - reg_data); }

CHECK_STATUS(mv_seq_exec(serdes_num, PEX_POWER_UP_SEQ));

-- 2.32.0

Stefan Roese

8 Oct 8 Oct

8:28 a.m.

New subject: [PATCH u-boot-marvell 5/9] arm: mvebu: a38x: serdes: Don't set PCIe Common Clock Configuration

On 24.09.21 22:59, Marek Behún wrote:

...

From: Pali Rohár pali@kernel.org

Enabling Common Clock Configuration bit in PCIe Root Port Link Control Register should not be done unconditionally. It is enabled by operating system as part of ASPM. Also after enabling Common Clock Configuration it is required to do more work, like retraining link. Some cards may be broken due to this incomplete Common Clock Configuration and some cards are broken and do not support ASPM at all.

Remove this incomplete code for Common Clock Configuration. It really should not be done in SerDes code as it is not related to SerDes, but to PCIe subsystem.

Signed-off-by: Pali Rohár pali@kernel.org Reviewed-by: Marek Behún marek.behun@nic.cz

Reviewed-by: Stefan Roese sr@denx.de

Thanks, Stefan

...

.../mach-mvebu/serdes/a38x/high_speed_env_spec.c | 15 --------------- 1 file changed, 15 deletions(-)

diff --git a/arch/arm/mach-mvebu/serdes/a38x/high_speed_env_spec.c b/arch/arm/mach-mvebu/serdes/a38x/high_speed_env_spec.c index a712fa8994..824f4d3e3d 100644 --- a/arch/arm/mach-mvebu/serdes/a38x/high_speed_env_spec.c +++ b/arch/arm/mach-mvebu/serdes/a38x/high_speed_env_spec.c @@ -1737,21 +1737,6 @@ int serdes_power_up_ctrl(u32 serdes_num, int serdes_power_up, reg_data |= (is_pex_by1 ? 1 : 4) << 4; reg_write(PEX0_RP_PCIE_CFG_OFFSET + PCI_EXP_LNKCAP, reg_data);
		/*
		 * Set Common Clock Configuration to indicates
		 * that both devices on the link use a
		 * distributed common reference clock.
		 */
		reg_data = reg_read(PEX_CFG_DIRECT_ACCESS(
				     pex_idx,
				     PEX_LINK_CTRL_STAT_REG));
		reg_data &= ~0x40;
		reg_data |= 0x40;
		reg_write(PEX_CFG_DIRECT_ACCESS(
			   pex_idx,
			   PEX_LINK_CTRL_STAT_REG),
			  reg_data);
}

CHECK_STATUS(mv_seq_exec(serdes_num, PEX_POWER_UP_SEQ));

Viele Grüße, Stefan

Marek Behún

24 Sep 24 Sep

10:59 p.m.

New subject: [PATCH u-boot-marvell 6/9] arm: mvebu: a38x: serdes: Don't overwrite PCI device ID

From: Pali Rohár pali@kernel.org

PCI device ID is part of the PCIe controller SoC / revision. For Root Complex mode (which is the default and the only mode supported currently by U-Boot and Linux kernel), it is PCI device ID of PCIe Root Port device.

If there is some issue with this device ID, it should be set / updated by PCIe controller driver (pci_mvebu.c), as this register resides in address space of the controller. It shouldn't be done in SerDes initialization code.

In the worst case (a specific board for example) it could be done via U-Boot's weak function board_pex_config().

But it should not be overwritten globally for all A38x devices.

Signed-off-by: Pali Rohár pali@kernel.org Reviewed-by: Marek Behún marek.behun@nic.cz --- arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c | 27 ---------------------- 1 file changed, 27 deletions(-)

diff --git a/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c b/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c index 7c18df8113..a7e45a5550 100644 --- a/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c +++ b/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c @@ -186,33 +186,6 @@ int hws_pex_config(const struct serdes_map *serdes_map, u8 count) (": Link upgraded to Gen2 based on client capabilities\n"); }

- /* Update pex DEVICE ID */ - ctrl_mode = sys_env_model_get(); - - for (idx = 0; idx < count; idx++) { - serdes_type = serdes_map[idx].serdes_type; - /* configuration for PEX only */ - if ((serdes_type != PEX0) && (serdes_type != PEX1) && - (serdes_type != PEX2) && (serdes_type != PEX3)) - continue; - - if ((serdes_type != PEX0) && - ((serdes_map[idx].serdes_mode == PEX_ROOT_COMPLEX_X4) || - (serdes_map[idx].serdes_mode == PEX_END_POINT_X4))) { - /* for PEX by4 - relevant for the first port only */ - continue; - } - - pex_idx = serdes_type - PEX0; - dev_id = reg_read(PEX_CFG_DIRECT_ACCESS - (pex_idx, PEX_DEVICE_AND_VENDOR_ID)); - dev_id &= 0xffff; - dev_id |= ((ctrl_mode << 16) & 0xffff0000); - reg_write(PEX_CFG_DIRECT_ACCESS - (pex_idx, PEX_DEVICE_AND_VENDOR_ID), dev_id); - } - DEBUG_INIT_FULL_C("Update PEX Device ID ", ctrl_mode, 4); - return MV_OK; }

-- 2.32.0

Stefan Roese

8 Oct 8 Oct

8:28 a.m.

New subject: [PATCH u-boot-marvell 6/9] arm: mvebu: a38x: serdes: Don't overwrite PCI device ID

On 24.09.21 22:59, Marek Behún wrote:

...

From: Pali Rohár pali@kernel.org

PCI device ID is part of the PCIe controller SoC / revision. For Root Complex mode (which is the default and the only mode supported currently by U-Boot and Linux kernel), it is PCI device ID of PCIe Root Port device.

If there is some issue with this device ID, it should be set / updated by PCIe controller driver (pci_mvebu.c), as this register resides in address space of the controller. It shouldn't be done in SerDes initialization code.

In the worst case (a specific board for example) it could be done via U-Boot's weak function board_pex_config().

But it should not be overwritten globally for all A38x devices.

Signed-off-by: Pali Rohár pali@kernel.org Reviewed-by: Marek Behún marek.behun@nic.cz

Reviewed-by: Stefan Roese sr@denx.de

Thanks, Stefan

...

arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c | 27 ---------------------- 1 file changed, 27 deletions(-)

diff --git a/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c b/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c index 7c18df8113..a7e45a5550 100644 --- a/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c +++ b/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c @@ -186,33 +186,6 @@ int hws_pex_config(const struct serdes_map *serdes_map, u8 count) (": Link upgraded to Gen2 based on client capabilities\n"); }
/* Update pex DEVICE ID */

ctrl_mode = sys_env_model_get();

for (idx = 0; idx < count; idx++) {
serdes_type = serdes_map[idx].serdes_type;
/* configuration for PEX only */
if ((serdes_type != PEX0) && (serdes_type != PEX1) &&
    (serdes_type != PEX2) && (serdes_type != PEX3))
	continue;
if ((serdes_type != PEX0) &&
    ((serdes_map[idx].serdes_mode == PEX_ROOT_COMPLEX_X4) ||
     (serdes_map[idx].serdes_mode == PEX_END_POINT_X4))) {
	/* for PEX by4 - relevant for the first port only */
	continue;
}
pex_idx = serdes_type - PEX0;
dev_id = reg_read(PEX_CFG_DIRECT_ACCESS
		  (pex_idx, PEX_DEVICE_AND_VENDOR_ID));
dev_id &= 0xffff;
dev_id |= ((ctrl_mode << 16) & 0xffff0000);
reg_write(PEX_CFG_DIRECT_ACCESS
	  (pex_idx, PEX_DEVICE_AND_VENDOR_ID), dev_id);
}

DEBUG_INIT_FULL_C("Update PEX Device ID ", ctrl_mode, 4);

return MV_OK; }

Viele Grüße, Stefan

Marek Behún

24 Sep 24 Sep

10:59 p.m.

New subject: [PATCH u-boot-marvell 7/9] arm: mvebu: a38x: serdes: Don't configure PCIe cards in SerDes init code

From: Pali Rohár pali@kernel.org

This code is trying to parse PCIe config space of PCIe card connected on the other end of link and then is trying to force 5.0 GT/s speed via Target Link Speed bits in PCIe Root Port Link Control 2 Register on the local part of link if it sees that card supports 5.0 GT/s via Max Link Speed bits in Link Capabilities Register.

The code is incorrect for more reasons: - Accessing config space of an endpoint card cannot be done immediately. If the PCIe link is not up, reading vendor/device ID registers will return all ones. - Parsing is incomplete, so it can cause issues even for working cards.

Moreover there is no need to force speed to 5.0 GT/s via Target Link Speed bits on PCIe Root Port Link Control 2 Register. Hardware changes speed from 2.5 GT/s to 5.0 GT/s autonomously when it is supported.

Most importantly, this code does not change link speed at all, since because after updating Target Link Speed bits on PCIe Root Port Link Control 2 Register, it is required to retrain the link, and the code for that is completely missing.

The code was probably needed for making buggy endpoint cards work. Such a workaround, though, should be implemented via PCIe subsystem (via quirks, for example), as buggy cards could also affect other PCIe controllers.

Note that this code is fully unrelated to a38x SerDes code and really should not have been included in SerDes initialization. Usage of magic constants without names and comments made this SerDes code hard to read and understand.

Remove this PCIe application code from low level SerDes code. As this code is configuring only 5.0 GT/s part, in the worst case, it could leave buggy cards at the initial speed of 2.5 GT/s (if somehow before this change they could have been "upgraded" to 5.0 GT/s speed even with missing link retraining). Compliant cards which just need longer initialization should work better after this change.

Signed-off-by: Pali Rohár pali@kernel.org Reviewed-by: Marek Behún marek.behun@nic.cz --- arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c | 128 +-------------------- 1 file changed, 1 insertion(+), 127 deletions(-)

diff --git a/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c b/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c index a7e45a5550..0445b43def 100644 --- a/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c +++ b/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c @@ -21,10 +21,8 @@ __weak void board_pex_config(void)

int hws_pex_config(const struct serdes_map *serdes_map, u8 count) { - u32 pex_idx, tmp, next_busno, first_busno, temp_pex_reg, - temp_reg, addr, dev_id, ctrl_mode; enum serdes_type serdes_type; - u32 idx; + u32 idx, tmp;

DEBUG_INIT_FULL_S("\n### hws_pex_config ###\n");

@@ -60,132 +58,8 @@ int hws_pex_config(const struct serdes_map *serdes_map, u8 count)

reg_write(SOC_CONTROL_REG1, tmp);

- /* Support gen1/gen2 */ - DEBUG_INIT_FULL_S("Support gen1/gen2\n"); - board_pex_config();

- next_busno = 0; - mdelay(150); - - for (idx = 0; idx < count; idx++) { - serdes_type = serdes_map[idx].serdes_type; - DEBUG_INIT_FULL_S(" serdes_type=0x"); - DEBUG_INIT_FULL_D(serdes_type, 8); - DEBUG_INIT_FULL_S("\n"); - DEBUG_INIT_FULL_S(" idx=0x"); - DEBUG_INIT_FULL_D(idx, 8); - DEBUG_INIT_FULL_S("\n"); - - /* Configuration for PEX only */ - if ((serdes_type != PEX0) && (serdes_type != PEX1) && - (serdes_type != PEX2) && (serdes_type != PEX3)) - continue; - - if ((serdes_type != PEX0) && - ((serdes_map[idx].serdes_mode == PEX_ROOT_COMPLEX_X4) || - (serdes_map[idx].serdes_mode == PEX_END_POINT_X4))) { - /* for PEX by4 - relevant for the first port only */ - continue; - } - - pex_idx = serdes_type - PEX0; - tmp = reg_read(PEX_DBG_STATUS_REG(pex_idx)); - - first_busno = next_busno; - if ((tmp & 0x7f) != 0x7e) { - DEBUG_INIT_S("PCIe, Idx "); - DEBUG_INIT_D(pex_idx, 1); - DEBUG_INIT_S(": detected no link\n"); - continue; - } - - next_busno++; - - /* - * Read maximum link speed. It must be 0x2 (5.0 GT/s) as this - * value was set in serdes_power_up_ctrl() function. - */ - temp_pex_reg = reg_read((PEX_CFG_DIRECT_ACCESS - (pex_idx, PEX_LINK_CAPABILITY_REG))); - temp_pex_reg &= 0xf; - if (temp_pex_reg != 0x2) - continue; - - /* Read negotiated link speed */ - temp_reg = (reg_read(PEX_CFG_DIRECT_ACCESS( - pex_idx, - PEX_LINK_CTRL_STAT_REG)) & - 0xf0000) >> 16; - - /* Check if the link established is GEN1 */ - DEBUG_INIT_FULL_S - ("Checking if the link established is gen1\n"); - if (temp_reg != 0x1) - continue; - - pex_local_bus_num_set(pex_idx, first_busno); - pex_local_dev_num_set(pex_idx, 1); - DEBUG_INIT_FULL_S("PCIe, Idx "); - DEBUG_INIT_FULL_D(pex_idx, 1); - - DEBUG_INIT_S(":** Link is Gen1, check the EP capability\n"); - /* link is Gen1, check the EP capability */ - addr = pex_config_read(pex_idx, first_busno, 0, 0, 0x34) & 0xff; - DEBUG_INIT_FULL_C("pex_config_read: return addr=0x%x", addr, 4); - if (addr == 0xff) { - DEBUG_INIT_FULL_C - ("pex_config_read: return 0xff -->PCIe (%d): Detected No Link.", - pex_idx, 1); - continue; - } - - /* Find start of the PCI Express Capability registers */ - while ((pex_config_read(pex_idx, first_busno, 0, 0, addr) - & 0xff) != 0x10) { - addr = (pex_config_read(pex_idx, first_busno, 0, - 0, addr) & 0xff00) >> 8; - } - - /* Check for Gen2 and above */ - if ((pex_config_read(pex_idx, first_busno, 0, 0, - addr + 0xc) & 0xf) < 0x2) { - DEBUG_INIT_S("PCIe, Idx "); - DEBUG_INIT_D(pex_idx, 1); - DEBUG_INIT_S(": remains Gen1\n"); - continue; - } - - tmp = reg_read(PEX_LINK_CTRL_STATUS2_REG(pex_idx)); - DEBUG_RD_REG(PEX_LINK_CTRL_STATUS2_REG(pex_idx), tmp); - tmp &= ~(BIT(0) | BIT(1)); - tmp |= BIT(1); /* Force Target Link Speed to 5.0 GT/s */ - tmp |= BIT(6); /* Select Deemphasize (-3.5d_b) */ - reg_write(PEX_LINK_CTRL_STATUS2_REG(pex_idx), tmp); - DEBUG_WR_REG(PEX_LINK_CTRL_STATUS2_REG(pex_idx), tmp); - - /* - * Enable Auto Speed change. When set, link will issue link - * speed change to max possible link speed. - */ - tmp = reg_read(PEX_CTRL_REG(pex_idx)); - DEBUG_RD_REG(PEX_CTRL_REG(pex_idx), tmp); - tmp |= BIT(10); - reg_write(PEX_CTRL_REG(pex_idx), tmp); - DEBUG_WR_REG(PEX_CTRL_REG(pex_idx), tmp); - - /* - * We need to wait 10ms before reading the PEX_DBG_STATUS_REG - * in order not to read the status of the former state - */ - mdelay(10); - - DEBUG_INIT_S("PCIe, Idx "); - DEBUG_INIT_D(pex_idx, 1); - DEBUG_INIT_S - (": Link upgraded to Gen2 based on client capabilities\n"); - } - return MV_OK; }

-- 2.32.0

Stefan Roese

8 Oct 8 Oct

8:29 a.m.

New subject: [PATCH u-boot-marvell 7/9] arm: mvebu: a38x: serdes: Don't configure PCIe cards in SerDes init code

On 24.09.21 22:59, Marek Behún wrote:

...

From: Pali Rohár pali@kernel.org

This code is trying to parse PCIe config space of PCIe card connected on the other end of link and then is trying to force 5.0 GT/s speed via Target Link Speed bits in PCIe Root Port Link Control 2 Register on the local part of link if it sees that card supports 5.0 GT/s via Max Link Speed bits in Link Capabilities Register.

The code is incorrect for more reasons:

Accessing config space of an endpoint card cannot be done immediately. If the PCIe link is not up, reading vendor/device ID registers will return all ones.

Parsing is incomplete, so it can cause issues even for working cards.

Moreover there is no need to force speed to 5.0 GT/s via Target Link Speed bits on PCIe Root Port Link Control 2 Register. Hardware changes speed from 2.5 GT/s to 5.0 GT/s autonomously when it is supported.

Most importantly, this code does not change link speed at all, since because after updating Target Link Speed bits on PCIe Root Port Link Control 2 Register, it is required to retrain the link, and the code for that is completely missing.

The code was probably needed for making buggy endpoint cards work. Such a workaround, though, should be implemented via PCIe subsystem (via quirks, for example), as buggy cards could also affect other PCIe controllers.

Note that this code is fully unrelated to a38x SerDes code and really should not have been included in SerDes initialization. Usage of magic constants without names and comments made this SerDes code hard to read and understand.

Remove this PCIe application code from low level SerDes code. As this code is configuring only 5.0 GT/s part, in the worst case, it could leave buggy cards at the initial speed of 2.5 GT/s (if somehow before this change they could have been "upgraded" to 5.0 GT/s speed even with missing link retraining). Compliant cards which just need longer initialization should work better after this change.

Signed-off-by: Pali Rohár pali@kernel.org Reviewed-by: Marek Behún marek.behun@nic.cz

Reviewed-by: Stefan Roese sr@denx.de

Thanks, Stefan

...

arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c | 128 +-------------------- 1 file changed, 1 insertion(+), 127 deletions(-)

int hws_pex_config(const struct serdes_map *serdes_map, u8 count) {

u32 pex_idx, tmp, next_busno, first_busno, temp_pex_reg,
```
   temp_reg, addr, dev_id, ctrl_mode;
```
enum serdes_type serdes_type;
u32 idx;

u32 idx, tmp;

DEBUG_INIT_FULL_S("\n### hws_pex_config ###\n");

@@ -60,132 +58,8 @@ int hws_pex_config(const struct serdes_map *serdes_map, u8 count)

reg_write(SOC_CONTROL_REG1, tmp);

/* Support gen1/gen2 */
DEBUG_INIT_FULL_S("Support gen1/gen2\n");
board_pex_config();
next_busno = 0;
mdelay(150);
for (idx = 0; idx < count; idx++) {

serdes_type = serdes_map[idx].serdes_type;

```
DEBUG_INIT_FULL_S(" serdes_type=0x");
```
```
DEBUG_INIT_FULL_D(serdes_type, 8);
```
```
DEBUG_INIT_FULL_S("\n");
```
```
DEBUG_INIT_FULL_S(" idx=0x");
```
```
DEBUG_INIT_FULL_D(idx, 8);
```
```
DEBUG_INIT_FULL_S("\n");
```
```
/* Configuration for PEX only */
```

if ((serdes_type != PEX0) && (serdes_type != PEX1) &&

    (serdes_type != PEX2) && (serdes_type != PEX3))

```
	continue;
```
```
if ((serdes_type != PEX0) &&
```

    ((serdes_map[idx].serdes_mode == PEX_ROOT_COMPLEX_X4) ||

     (serdes_map[idx].serdes_mode == PEX_END_POINT_X4))) {

	/* for PEX by4 - relevant for the first port only */

```
	continue;
```
```
}
```
```
pex_idx = serdes_type - PEX0;
```

tmp = reg_read(PEX_DBG_STATUS_REG(pex_idx));

```
first_busno = next_busno;
```
```
if ((tmp & 0x7f) != 0x7e) {
```
```
	DEBUG_INIT_S("PCIe, Idx ");
```
```
	DEBUG_INIT_D(pex_idx, 1);
```
```
	DEBUG_INIT_S(": detected no link\n");
```
```
	continue;
```
```
}
```
```
next_busno++;
```
```
/*
```

 * Read maximum link speed. It must be 0x2 (5.0 GT/s) as this

 * value was set in serdes_power_up_ctrl() function.

```
 */
```

temp_pex_reg = reg_read((PEX_CFG_DIRECT_ACCESS

			 (pex_idx, PEX_LINK_CAPABILITY_REG)));

```
temp_pex_reg &= 0xf;
```
```
if (temp_pex_reg != 0x2)
```
```
	continue;
```
```
/* Read negotiated link speed */
```

temp_reg = (reg_read(PEX_CFG_DIRECT_ACCESS(

```
			     pex_idx,
```
```
			     PEX_LINK_CTRL_STAT_REG)) &
```
```
	    0xf0000) >> 16;
```

/* Check if the link established is GEN1 */

```
DEBUG_INIT_FULL_S
```

	("Checking if the link established is gen1\n");

```
if (temp_reg != 0x1)
```
```
	continue;
```

pex_local_bus_num_set(pex_idx, first_busno);

```
pex_local_dev_num_set(pex_idx, 1);
```
```
DEBUG_INIT_FULL_S("PCIe, Idx ");
```
```
DEBUG_INIT_FULL_D(pex_idx, 1);
```

DEBUG_INIT_S(":** Link is Gen1, check the EP capability\n");

/* link is Gen1, check the EP capability */

addr = pex_config_read(pex_idx, first_busno, 0, 0, 0x34) & 0xff;

DEBUG_INIT_FULL_C("pex_config_read: return addr=0x%x", addr, 4);

```
if (addr == 0xff) {
```
```
	DEBUG_INIT_FULL_C
```

		("pex_config_read: return 0xff -->PCIe (%d): Detected No Link.",

```
		 pex_idx, 1);
```
```
	continue;
```
```
}
```

/* Find start of the PCI Express Capability registers */

while ((pex_config_read(pex_idx, first_busno, 0, 0, addr)

```
	& 0xff) != 0x10) {
```

	addr = (pex_config_read(pex_idx, first_busno, 0,

```
				0, addr) & 0xff00) >> 8;
```
```
}
```
```
/* Check for Gen2 and above */
```

if ((pex_config_read(pex_idx, first_busno, 0, 0,

```
		     addr + 0xc) & 0xf) < 0x2) {
```
```
	DEBUG_INIT_S("PCIe, Idx ");
```
```
	DEBUG_INIT_D(pex_idx, 1);
```
```
	DEBUG_INIT_S(": remains Gen1\n");
```
```
	continue;
```
```
}
```

tmp = reg_read(PEX_LINK_CTRL_STATUS2_REG(pex_idx));

DEBUG_RD_REG(PEX_LINK_CTRL_STATUS2_REG(pex_idx), tmp);

```
tmp &= ~(BIT(0) | BIT(1));
```

tmp |= BIT(1);	/* Force Target Link Speed to 5.0 GT/s */

tmp |= BIT(6);	/* Select Deemphasize (-3.5d_b) */

reg_write(PEX_LINK_CTRL_STATUS2_REG(pex_idx), tmp);

DEBUG_WR_REG(PEX_LINK_CTRL_STATUS2_REG(pex_idx), tmp);

```
/*
```

 * Enable Auto Speed change. When set, link will issue link

 * speed change to max possible link speed.

```
 */
```
```
tmp = reg_read(PEX_CTRL_REG(pex_idx));
```

DEBUG_RD_REG(PEX_CTRL_REG(pex_idx), tmp);

```
tmp |= BIT(10);
```
```
reg_write(PEX_CTRL_REG(pex_idx), tmp);
```

DEBUG_WR_REG(PEX_CTRL_REG(pex_idx), tmp);

```
/*
```

 * We need to wait 10ms before reading the PEX_DBG_STATUS_REG

 * in order not to read the status of the former state

```
 */
```
```
mdelay(10);
```
```
DEBUG_INIT_S("PCIe, Idx ");
```
```
DEBUG_INIT_D(pex_idx, 1);
```
```
DEBUG_INIT_S
```

	(": Link upgraded to Gen2 based on client capabilities\n");

}
return MV_OK; }

Viele Grüße, Stefan

Marek Behún

24 Sep 24 Sep

10:59 p.m.

New subject: [PATCH u-boot-marvell 8/9] arm: mvebu: a38x: serdes: Remove unused PCIe macros and functions

From: Pali Rohár pali@kernel.org

Remove unused PCIe functions from SerDes code. They are unused and are duplicated either from generic PCIe code or from pci_mvebu.c.

Remove also unused PCIe macros from SerDes code. They are just obfuscated variants of standards macros in include/pci.h or in pci_mvebu.c.

Signed-off-by: Pali Rohár pali@kernel.org Reviewed-by: Marek Behún marek.behun@nic.cz --- arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c | 128 --------------------- arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.h | 60 ---------- 2 files changed, 188 deletions(-)

diff --git a/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c b/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c index 0445b43def..55c3f9ca39 100644 --- a/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c +++ b/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c @@ -62,131 +62,3 @@ int hws_pex_config(const struct serdes_map *serdes_map, u8 count)

return MV_OK; } - -int pex_local_bus_num_set(u32 pex_if, u32 bus_num) -{ - u32 pex_status; - - DEBUG_INIT_FULL_S("\n### pex_local_bus_num_set ###\n"); - - if (bus_num >= MAX_PEX_BUSSES) { - DEBUG_INIT_C("pex_local_bus_num_set: Illegal bus number %d\n", - bus_num, 4); - return MV_BAD_PARAM; - } - - pex_status = reg_read(PEX_STATUS_REG(pex_if)); - pex_status &= ~PXSR_PEX_BUS_NUM_MASK; - pex_status |= - (bus_num << PXSR_PEX_BUS_NUM_OFFS) & PXSR_PEX_BUS_NUM_MASK; - reg_write(PEX_STATUS_REG(pex_if), pex_status); - - return MV_OK; -} - -int pex_local_dev_num_set(u32 pex_if, u32 dev_num) -{ - u32 pex_status; - - DEBUG_INIT_FULL_S("\n### pex_local_dev_num_set ###\n"); - - pex_status = reg_read(PEX_STATUS_REG(pex_if)); - pex_status &= ~PXSR_PEX_DEV_NUM_MASK; - pex_status |= - (dev_num << PXSR_PEX_DEV_NUM_OFFS) & PXSR_PEX_DEV_NUM_MASK; - reg_write(PEX_STATUS_REG(pex_if), pex_status); - - return MV_OK; -} - -/* - * pex_config_read - Read from configuration space - * - * DESCRIPTION: - * This function performs a 32 bit read from PEX configuration space. - * It supports both type 0 and type 1 of Configuration Transactions - * (local and over bridge). In order to read from local bus segment, use - * bus number retrieved from pex_local_bus_num_get(). Other bus numbers - * will result configuration transaction of type 1 (over bridge). - * - * INPUT: - * pex_if - PEX interface number. - * bus - PEX segment bus number. - * dev - PEX device number. - * func - Function number. - * reg_offs - Register offset. - * - * OUTPUT: - * None. - * - * RETURN: - * 32bit register data, 0xffffffff on error - */ -u32 pex_config_read(u32 pex_if, u32 bus, u32 dev, u32 func, u32 reg_off) -{ - u32 pex_data = 0; - u32 local_dev, local_bus; - u32 pex_status; - - pex_status = reg_read(PEX_STATUS_REG(pex_if)); - local_dev = - ((pex_status & PXSR_PEX_DEV_NUM_MASK) >> PXSR_PEX_DEV_NUM_OFFS); - local_bus = - ((pex_status & PXSR_PEX_BUS_NUM_MASK) >> PXSR_PEX_BUS_NUM_OFFS); - - /* - * In PCI Express we have only one device number - * and this number is the first number we encounter - * else that the local_dev - * spec pex define return on config read/write on any device - */ - if (bus == local_bus) { - if (local_dev == 0) { - /* - * if local dev is 0 then the first number we encounter - * after 0 is 1 - */ - if ((dev != 1) && (dev != local_dev)) - return MV_ERROR; - } else { - /* - * if local dev is not 0 then the first number we - * encounter is 0 - */ - if ((dev != 0) && (dev != local_dev)) - return MV_ERROR; - } - } - - /* Creating PEX address to be passed */ - pex_data = (bus << PXCAR_BUS_NUM_OFFS); - pex_data |= (dev << PXCAR_DEVICE_NUM_OFFS); - pex_data |= (func << PXCAR_FUNC_NUM_OFFS); - /* Legacy register space */ - pex_data |= (reg_off & PXCAR_REG_NUM_MASK); - /* Extended register space */ - pex_data |= (((reg_off & PXCAR_REAL_EXT_REG_NUM_MASK) >> - PXCAR_REAL_EXT_REG_NUM_OFFS) << PXCAR_EXT_REG_NUM_OFFS); - pex_data |= PXCAR_CONFIG_EN; - - /* Write the address to the PEX configuration address register */ - reg_write(PEX_CFG_ADDR_REG(pex_if), pex_data); - - /* - * In order to let the PEX controller absorbed the address - * of the read transaction we perform a validity check that - * the address was written - */ - if (pex_data != reg_read(PEX_CFG_ADDR_REG(pex_if))) - return MV_ERROR; - - /* Cleaning Master Abort */ - reg_bit_set(PEX_CFG_DIRECT_ACCESS(pex_if, PEX_STATUS_AND_COMMAND), - PXSAC_MABORT); - /* Read the Data returned in the PEX Data register */ - pex_data = reg_read(PEX_CFG_DATA_REG(pex_if)); - - DEBUG_INIT_FULL_C(" --> ", pex_data, 4); - - return pex_data; -} diff --git a/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.h b/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.h index 5d70166fc5..55a4c267c4 100644 --- a/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.h +++ b/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.h @@ -12,28 +12,6 @@ /* Direct access to PEX0 Root Port's PCIe Capability structure */ #define PEX0_RP_PCIE_CFG_OFFSET (0x00080000 + 0x60)

-/* Sample at Reset */ -#define MPP_SAMPLE_AT_RESET(id) (0xe4200 + (id * 4)) - -/* PCI Express Control and Status Registers */ -#define MAX_PEX_BUSSES 256 - -#define PEX_IF_REGS_OFFSET(if) ((if) > 0 ? \ - (0x40000 + ((if) - 1) * 0x4000) : \ - 0x80000) -#define PEX_IF_REGS_BASE(if) (PEX_IF_REGS_OFFSET(if)) -#define PEX_CAPABILITIES_REG(if) ((PEX_IF_REGS_BASE(if)) + 0x60) -#define PEX_LINK_CTRL_STATUS2_REG(if) ((PEX_IF_REGS_BASE(if)) + 0x90) -#define PEX_CTRL_REG(if) ((PEX_IF_REGS_BASE(if)) + 0x1a00) -#define PEX_STATUS_REG(if) ((PEX_IF_REGS_BASE(if)) + 0x1a04) -#define PEX_DBG_STATUS_REG(if) ((PEX_IF_REGS_BASE(if)) + 0x1a64) -#define PEX_LINK_CAPABILITY_REG 0x6c -#define PEX_LINK_CTRL_STAT_REG 0x70 -#define PXSR_PEX_DEV_NUM_OFFS 16 /* Device Number Indication */ -#define PXSR_PEX_DEV_NUM_MASK (0x1f << PXSR_PEX_DEV_NUM_OFFS) -#define PXSR_PEX_BUS_NUM_OFFS 8 /* Bus Number Indication */ -#define PXSR_PEX_BUS_NUM_MASK (0xff << PXSR_PEX_BUS_NUM_OFFS) - /* PEX_CAPABILITIES_REG fields */ #define PCIE0_ENABLE_OFFS 0 #define PCIE0_ENABLE_MASK (0x1 << PCIE0_ENABLE_OFFS) @@ -44,45 +22,7 @@ #define PCIE3_ENABLE_OFFS 3 #define PCIE4_ENABLE_MASK (0x1 << PCIE3_ENABLE_OFFS)

-/* Controller revision info */ -#define PEX_DEVICE_AND_VENDOR_ID 0x000 -#define PEX_CFG_DIRECT_ACCESS(if, reg) (PEX_IF_REGS_BASE(if) + (reg)) - -/* PCI Express Configuration Address Register */ -#define PXCAR_REG_NUM_OFFS 2 -#define PXCAR_REG_NUM_MAX 0x3f -#define PXCAR_REG_NUM_MASK (PXCAR_REG_NUM_MAX << \ - PXCAR_REG_NUM_OFFS) -#define PXCAR_FUNC_NUM_OFFS 8 -#define PXCAR_FUNC_NUM_MAX 0x7 -#define PXCAR_FUNC_NUM_MASK (PXCAR_FUNC_NUM_MAX << \ - PXCAR_FUNC_NUM_OFFS) -#define PXCAR_DEVICE_NUM_OFFS 11 -#define PXCAR_DEVICE_NUM_MAX 0x1f -#define PXCAR_DEVICE_NUM_MASK (PXCAR_DEVICE_NUM_MAX << \ - PXCAR_DEVICE_NUM_OFFS) -#define PXCAR_BUS_NUM_OFFS 16 -#define PXCAR_BUS_NUM_MAX 0xff -#define PXCAR_BUS_NUM_MASK (PXCAR_BUS_NUM_MAX << \ - PXCAR_BUS_NUM_OFFS) -#define PXCAR_EXT_REG_NUM_OFFS 24 -#define PXCAR_EXT_REG_NUM_MAX 0xf - -#define PEX_CFG_ADDR_REG(if) ((PEX_IF_REGS_BASE(if)) + 0x18f8) -#define PEX_CFG_DATA_REG(if) ((PEX_IF_REGS_BASE(if)) + 0x18fc) - -#define PXCAR_REAL_EXT_REG_NUM_OFFS 8 -#define PXCAR_REAL_EXT_REG_NUM_MASK (0xf << PXCAR_REAL_EXT_REG_NUM_OFFS) - -#define PXCAR_CONFIG_EN BIT(31) -#define PEX_STATUS_AND_COMMAND 0x004 -#define PXSAC_MABORT BIT(29) /* Recieved Master Abort */ - int hws_pex_config(const struct serdes_map *serdes_map, u8 count); -int pex_local_bus_num_set(u32 pex_if, u32 bus_num); -int pex_local_dev_num_set(u32 pex_if, u32 dev_num); -u32 pex_config_read(u32 pex_if, u32 bus, u32 dev, u32 func, u32 reg_off); - void board_pex_config(void);

#endif

-- 2.32.0

Stefan Roese

8 Oct 8 Oct

8:29 a.m.

New subject: [PATCH u-boot-marvell 8/9] arm: mvebu: a38x: serdes: Remove unused PCIe macros and functions

On 24.09.21 22:59, Marek Behún wrote:

...

From: Pali Rohár pali@kernel.org

Remove unused PCIe functions from SerDes code. They are unused and are duplicated either from generic PCIe code or from pci_mvebu.c.

Remove also unused PCIe macros from SerDes code. They are just obfuscated variants of standards macros in include/pci.h or in pci_mvebu.c.

Signed-off-by: Pali Rohár pali@kernel.org Reviewed-by: Marek Behún marek.behun@nic.cz

Reviewed-by: Stefan Roese sr@denx.de

Thanks, Stefan

...

arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c | 128 --------------------- arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.h | 60 ---------- 2 files changed, 188 deletions(-)

diff --git a/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c b/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c index 0445b43def..55c3f9ca39 100644 --- a/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c +++ b/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c @@ -62,131 +62,3 @@ int hws_pex_config(const struct serdes_map *serdes_map, u8 count)

return MV_OK; }

-int pex_local_bus_num_set(u32 pex_if, u32 bus_num) -{
u32 pex_status;

DEBUG_INIT_FULL_S("\n### pex_local_bus_num_set ###\n");

if (bus_num >= MAX_PEX_BUSSES) {
DEBUG_INIT_C("pex_local_bus_num_set: Illegal bus number %d\n",
	     bus_num, 4);
return MV_BAD_PARAM;
}

pex_status = reg_read(PEX_STATUS_REG(pex_if));

pex_status &= ~PXSR_PEX_BUS_NUM_MASK;

pex_status |=
   (bus_num << PXSR_PEX_BUS_NUM_OFFS) & PXSR_PEX_BUS_NUM_MASK;
reg_write(PEX_STATUS_REG(pex_if), pex_status);

return MV_OK;
-}

-int pex_local_dev_num_set(u32 pex_if, u32 dev_num) -{
u32 pex_status;

DEBUG_INIT_FULL_S("\n### pex_local_dev_num_set ###\n");

pex_status = reg_read(PEX_STATUS_REG(pex_if));

pex_status &= ~PXSR_PEX_DEV_NUM_MASK;

pex_status |=
   (dev_num << PXSR_PEX_DEV_NUM_OFFS) & PXSR_PEX_DEV_NUM_MASK;
reg_write(PEX_STATUS_REG(pex_if), pex_status);

return MV_OK;
-}

-/*
pex_config_read - Read from configuration space

DESCRIPTION:
  This function performs a 32 bit read from PEX configuration space.
  It supports both type 0 and type 1 of Configuration Transactions
  (local and over bridge). In order to read from local bus segment, use
  bus number retrieved from pex_local_bus_num_get(). Other bus numbers
  will result configuration transaction of type 1 (over bridge).
INPUT:
  pex_if   - PEX interface number.
  bus      - PEX segment bus number.
  dev      - PEX device number.
  func     - Function number.
  reg_offs - Register offset.
OUTPUT:
  None.
RETURN:
  32bit register data, 0xffffffff on error
*/
-u32 pex_config_read(u32 pex_if, u32 bus, u32 dev, u32 func, u32 reg_off) -{
u32 pex_data = 0;

u32 local_dev, local_bus;

u32 pex_status;

pex_status = reg_read(PEX_STATUS_REG(pex_if));

local_dev =
   ((pex_status & PXSR_PEX_DEV_NUM_MASK) >> PXSR_PEX_DEV_NUM_OFFS);
local_bus =
   ((pex_status & PXSR_PEX_BUS_NUM_MASK) >> PXSR_PEX_BUS_NUM_OFFS);
/*
* In PCI Express we have only one device number
* and this number is the first number we encounter
* else that the local_dev
* spec pex define return on config read/write on any device
*/
if (bus == local_bus) {
if (local_dev == 0) {
	/*
	 * if local dev is 0 then the first number we encounter
	 * after 0 is 1
	 */
	if ((dev != 1) && (dev != local_dev))
		return MV_ERROR;
} else {
	/*
	 * if local dev is not 0 then the first number we
	 * encounter is 0
	 */
	if ((dev != 0) && (dev != local_dev))
		return MV_ERROR;
}
}

/* Creating PEX address to be passed */

pex_data = (bus << PXCAR_BUS_NUM_OFFS);

pex_data |= (dev << PXCAR_DEVICE_NUM_OFFS);

pex_data |= (func << PXCAR_FUNC_NUM_OFFS);

/* Legacy register space */

pex_data |= (reg_off & PXCAR_REG_NUM_MASK);

/* Extended register space */

pex_data |= (((reg_off & PXCAR_REAL_EXT_REG_NUM_MASK) >>
      PXCAR_REAL_EXT_REG_NUM_OFFS) << PXCAR_EXT_REG_NUM_OFFS);
pex_data |= PXCAR_CONFIG_EN;

/* Write the address to the PEX configuration address register */

reg_write(PEX_CFG_ADDR_REG(pex_if), pex_data);

/*
* In order to let the PEX controller absorbed the address
* of the read transaction we perform a validity check that
* the address was written
*/
if (pex_data != reg_read(PEX_CFG_ADDR_REG(pex_if)))
return MV_ERROR;
/* Cleaning Master Abort */

reg_bit_set(PEX_CFG_DIRECT_ACCESS(pex_if, PEX_STATUS_AND_COMMAND),
    PXSAC_MABORT);
/* Read the Data returned in the PEX Data register */

pex_data = reg_read(PEX_CFG_DATA_REG(pex_if));

DEBUG_INIT_FULL_C(" --> ", pex_data, 4);

return pex_data;
-} diff --git a/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.h b/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.h index 5d70166fc5..55a4c267c4 100644 --- a/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.h +++ b/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.h @@ -12,28 +12,6 @@ /* Direct access to PEX0 Root Port's PCIe Capability structure */ #define PEX0_RP_PCIE_CFG_OFFSET (0x00080000 + 0x60)

-/* Sample at Reset */ -#define MPP_SAMPLE_AT_RESET(id) (0xe4200 + (id * 4))

-/* PCI Express Control and Status Registers */ -#define MAX_PEX_BUSSES 256

-#define PEX_IF_REGS_OFFSET(if) ((if) > 0 ? \
			 (0x40000 + ((if) - 1) * 0x4000) : \
			 0x80000)
-#define PEX_IF_REGS_BASE(if) (PEX_IF_REGS_OFFSET(if)) -#define PEX_CAPABILITIES_REG(if) ((PEX_IF_REGS_BASE(if)) + 0x60) -#define PEX_LINK_CTRL_STATUS2_REG(if) ((PEX_IF_REGS_BASE(if)) + 0x90) -#define PEX_CTRL_REG(if) ((PEX_IF_REGS_BASE(if)) + 0x1a00) -#define PEX_STATUS_REG(if) ((PEX_IF_REGS_BASE(if)) + 0x1a04) -#define PEX_DBG_STATUS_REG(if) ((PEX_IF_REGS_BASE(if)) + 0x1a64) -#define PEX_LINK_CAPABILITY_REG 0x6c -#define PEX_LINK_CTRL_STAT_REG 0x70 -#define PXSR_PEX_DEV_NUM_OFFS 16 /* Device Number Indication */ -#define PXSR_PEX_DEV_NUM_MASK (0x1f << PXSR_PEX_DEV_NUM_OFFS) -#define PXSR_PEX_BUS_NUM_OFFS 8 /* Bus Number Indication */ -#define PXSR_PEX_BUS_NUM_MASK (0xff << PXSR_PEX_BUS_NUM_OFFS)

/* PEX_CAPABILITIES_REG fields */ #define PCIE0_ENABLE_OFFS 0 #define PCIE0_ENABLE_MASK (0x1 << PCIE0_ENABLE_OFFS)

@@ -44,45 +22,7 @@ #define PCIE3_ENABLE_OFFS 3 #define PCIE4_ENABLE_MASK (0x1 << PCIE3_ENABLE_OFFS)

-/* Controller revision info */ -#define PEX_DEVICE_AND_VENDOR_ID 0x000 -#define PEX_CFG_DIRECT_ACCESS(if, reg) (PEX_IF_REGS_BASE(if) + (reg))

-/* PCI Express Configuration Address Register */ -#define PXCAR_REG_NUM_OFFS 2 -#define PXCAR_REG_NUM_MAX 0x3f -#define PXCAR_REG_NUM_MASK (PXCAR_REG_NUM_MAX << \
			 PXCAR_REG_NUM_OFFS)
-#define PXCAR_FUNC_NUM_OFFS 8 -#define PXCAR_FUNC_NUM_MAX 0x7 -#define PXCAR_FUNC_NUM_MASK (PXCAR_FUNC_NUM_MAX << \
			 PXCAR_FUNC_NUM_OFFS)
-#define PXCAR_DEVICE_NUM_OFFS 11 -#define PXCAR_DEVICE_NUM_MAX 0x1f -#define PXCAR_DEVICE_NUM_MASK (PXCAR_DEVICE_NUM_MAX << \
			 PXCAR_DEVICE_NUM_OFFS)
-#define PXCAR_BUS_NUM_OFFS 16 -#define PXCAR_BUS_NUM_MAX 0xff -#define PXCAR_BUS_NUM_MASK (PXCAR_BUS_NUM_MAX << \
			 PXCAR_BUS_NUM_OFFS)
-#define PXCAR_EXT_REG_NUM_OFFS 24 -#define PXCAR_EXT_REG_NUM_MAX 0xf

-#define PEX_CFG_ADDR_REG(if) ((PEX_IF_REGS_BASE(if)) + 0x18f8) -#define PEX_CFG_DATA_REG(if) ((PEX_IF_REGS_BASE(if)) + 0x18fc)

-#define PXCAR_REAL_EXT_REG_NUM_OFFS 8 -#define PXCAR_REAL_EXT_REG_NUM_MASK (0xf << PXCAR_REAL_EXT_REG_NUM_OFFS)

-#define PXCAR_CONFIG_EN BIT(31) -#define PEX_STATUS_AND_COMMAND 0x004 -#define PXSAC_MABORT BIT(29) /* Recieved Master Abort */

int hws_pex_config(const struct serdes_map *serdes_map, u8 count);

-int pex_local_bus_num_set(u32 pex_if, u32 bus_num); -int pex_local_dev_num_set(u32 pex_if, u32 dev_num); -u32 pex_config_read(u32 pex_if, u32 bus, u32 dev, u32 func, u32 reg_off);

void board_pex_config(void);

#endif

Viele Grüße, Stefan

Marek Behún

24 Sep 24 Sep

10:59 p.m.

New subject: [PATCH u-boot-marvell 9/9] arm: mvebu: a38x: serdes: Update comment about PCIE*_ENABLE_* defines

From: Pali Rohár pali@kernel.org

These are part of SOC_CONTROL_REG1 register, not PEX_CAPABILITIES_REG.

Signed-off-by: Pali Rohár pali@kernel.org Reviewed-by: Marek Behún marek.behun@nic.cz --- arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.h b/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.h index 55a4c267c4..64193d5288 100644 --- a/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.h +++ b/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.h @@ -12,7 +12,7 @@ /* Direct access to PEX0 Root Port's PCIe Capability structure */ #define PEX0_RP_PCIE_CFG_OFFSET (0x00080000 + 0x60)

-/* PEX_CAPABILITIES_REG fields */ +/* SOC_CONTROL_REG1 fields */ #define PCIE0_ENABLE_OFFS 0 #define PCIE0_ENABLE_MASK (0x1 << PCIE0_ENABLE_OFFS) #define PCIE1_ENABLE_OFFS 1

-- 2.32.0

Stefan Roese

8 Oct 8 Oct

8:30 a.m.

New subject: [PATCH u-boot-marvell 9/9] arm: mvebu: a38x: serdes: Update comment about PCIE*_ENABLE_* defines

On 24.09.21 22:59, Marek Behún wrote:

...

From: Pali Rohár pali@kernel.org

These are part of SOC_CONTROL_REG1 register, not PEX_CAPABILITIES_REG.

Signed-off-by: Pali Rohár pali@kernel.org Reviewed-by: Marek Behún marek.behun@nic.cz

Reviewed-by: Stefan Roese sr@denx.de

Thanks, Stefan

...

arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.h b/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.h index 55a4c267c4..64193d5288 100644 --- a/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.h +++ b/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.h @@ -12,7 +12,7 @@ /* Direct access to PEX0 Root Port's PCIe Capability structure */ #define PEX0_RP_PCIE_CFG_OFFSET (0x00080000 + 0x60)

-/* PEX_CAPABILITIES_REG fields */ +/* SOC_CONTROL_REG1 fields */ #define PCIE0_ENABLE_OFFS 0 #define PCIE0_ENABLE_MASK (0x1 << PCIE0_ENABLE_OFFS) #define PCIE1_ENABLE_OFFS 1

Viele Grüße, Stefan

Stefan Roese

8:31 a.m.

Hi Pali & Marek,

On 24.09.21 22:59, Marek Behún wrote:

...

Hi Stefan,

this series by Pali cleans unneeded code in arch/arm/mach-mvebu/serdes/a38x.

Pali studied the code, added comments about what the code does, and then removed unneeded parts, wich explanations in commits.

Many thanks for this impressive work.

Thanks, Stefan

...

Marek

Pali Rohár (9): arm: mvebu: a38x: serdes: Add comments and use macros in PCIe code arm: mvebu: a38x: serdes: Remove duplicate macro SOC_CTRL_REG arm: mvebu: a38x: serdes: Add comments for hws_pex_config() code arm: mvebu: a38x: serdes: Don't overwrite read-only SAR PCIe registers arm: mvebu: a38x: serdes: Don't set PCIe Common Clock Configuration arm: mvebu: a38x: serdes: Don't overwrite PCI device ID arm: mvebu: a38x: serdes: Don't configure PCIe cards in SerDes init code arm: mvebu: a38x: serdes: Remove unused PCIe macros and functions arm: mvebu: a38x: serdes: Update comment about PCIE*_ENABLE_* defines

arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c | 297 +----------------- arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.h | 68 +--- .../serdes/a38x/high_speed_env_spec.c | 42 +-- 3 files changed, 22 insertions(+), 385 deletions(-)

Viele Grüße, Stefan

Stefan Roese

11:18 a.m.

On 24.09.21 22:59, Marek Behún wrote:

...

Hi Stefan,

this series by Pali cleans unneeded code in arch/arm/mach-mvebu/serdes/a38x.

Pali studied the code, added comments about what the code does, and then removed unneeded parts, wich explanations in commits.

Marek

Pali Rohár (9): arm: mvebu: a38x: serdes: Add comments and use macros in PCIe code arm: mvebu: a38x: serdes: Remove duplicate macro SOC_CTRL_REG arm: mvebu: a38x: serdes: Add comments for hws_pex_config() code arm: mvebu: a38x: serdes: Don't overwrite read-only SAR PCIe registers arm: mvebu: a38x: serdes: Don't set PCIe Common Clock Configuration arm: mvebu: a38x: serdes: Don't overwrite PCI device ID arm: mvebu: a38x: serdes: Don't configure PCIe cards in SerDes init code arm: mvebu: a38x: serdes: Remove unused PCIe macros and functions arm: mvebu: a38x: serdes: Update comment about PCIE*_ENABLE_* defines

arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c | 297 +----------------- arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.h | 68 +--- .../serdes/a38x/high_speed_env_spec.c | 42 +-- 3 files changed, 22 insertions(+), 385 deletions(-)

Applied to u-boot-marvell/master

Thanks, Stefan

1313

Age (days ago)

1327

Last active (days ago)

List overview

Download

20 comments

2 participants

tags (0)

participants (2)

Marek Behún
Stefan Roese