* [PATCH u-boot-marvell 1/9] arm: mvebu: a38x: serdes: Add comments and use macros in PCIe code
2021-09-24 20:59 [PATCH u-boot-marvell 0/9] a38x serdes cleanup Marek Behún
@ 2021-09-24 20:59 ` Marek Behún
2021-10-08 6:24 ` Stefan Roese
2021-09-24 20:59 ` [PATCH u-boot-marvell 2/9] arm: mvebu: a38x: serdes: Remove duplicate macro SOC_CTRL_REG Marek Behún
` (9 subsequent siblings)
10 siblings, 1 reply; 21+ messages in thread
From: Marek Behún @ 2021-09-24 20:59 UTC (permalink / raw)
To: Stefan Roese; +Cc: u-boot, pali, Marek Behún
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
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH u-boot-marvell 1/9] arm: mvebu: a38x: serdes: Add comments and use macros in PCIe code
2021-09-24 20:59 ` [PATCH u-boot-marvell 1/9] arm: mvebu: a38x: serdes: Add comments and use macros in PCIe code Marek Behún
@ 2021-10-08 6:24 ` Stefan Roese
0 siblings, 0 replies; 21+ messages in thread
From: Stefan Roese @ 2021-10-08 6:24 UTC (permalink / raw)
To: Marek Behún; +Cc: u-boot, pali
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(-)
>
> 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);
> }
>
>
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH u-boot-marvell 2/9] arm: mvebu: a38x: serdes: Remove duplicate macro SOC_CTRL_REG
2021-09-24 20:59 [PATCH u-boot-marvell 0/9] a38x serdes cleanup Marek Behún
2021-09-24 20:59 ` [PATCH u-boot-marvell 1/9] arm: mvebu: a38x: serdes: Add comments and use macros in PCIe code Marek Behún
@ 2021-09-24 20:59 ` Marek Behún
2021-10-08 6:26 ` Stefan Roese
2021-09-24 20:59 ` [PATCH u-boot-marvell 3/9] arm: mvebu: a38x: serdes: Add comments for hws_pex_config() code Marek Behún
` (8 subsequent siblings)
10 siblings, 1 reply; 21+ messages in thread
From: Marek Behún @ 2021-09-24 20:59 UTC (permalink / raw)
To: Stefan Roese; +Cc: u-boot, pali, Marek Behún
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
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH u-boot-marvell 2/9] arm: mvebu: a38x: serdes: Remove duplicate macro SOC_CTRL_REG
2021-09-24 20:59 ` [PATCH u-boot-marvell 2/9] arm: mvebu: a38x: serdes: Remove duplicate macro SOC_CTRL_REG Marek Behún
@ 2021-10-08 6:26 ` Stefan Roese
0 siblings, 0 replies; 21+ messages in thread
From: Stefan Roese @ 2021-10-08 6:26 UTC (permalink / raw)
To: Marek Behún; +Cc: u-boot, pali
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
--
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH u-boot-marvell 3/9] arm: mvebu: a38x: serdes: Add comments for hws_pex_config() code
2021-09-24 20:59 [PATCH u-boot-marvell 0/9] a38x serdes cleanup Marek Behún
2021-09-24 20:59 ` [PATCH u-boot-marvell 1/9] arm: mvebu: a38x: serdes: Add comments and use macros in PCIe code Marek Behún
2021-09-24 20:59 ` [PATCH u-boot-marvell 2/9] arm: mvebu: a38x: serdes: Remove duplicate macro SOC_CTRL_REG Marek Behún
@ 2021-09-24 20:59 ` Marek Behún
2021-10-08 6:27 ` Stefan Roese
2021-09-24 20:59 ` [PATCH u-boot-marvell 4/9] arm: mvebu: a38x: serdes: Don't overwrite read-only SAR PCIe registers Marek Behún
` (7 subsequent siblings)
10 siblings, 1 reply; 21+ messages in thread
From: Marek Behún @ 2021-09-24 20:59 UTC (permalink / raw)
To: Stefan Roese; +Cc: u-boot, pali, Marek Behún
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
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH u-boot-marvell 3/9] arm: mvebu: a38x: serdes: Add comments for hws_pex_config() code
2021-09-24 20:59 ` [PATCH u-boot-marvell 3/9] arm: mvebu: a38x: serdes: Add comments for hws_pex_config() code Marek Behún
@ 2021-10-08 6:27 ` Stefan Roese
0 siblings, 0 replies; 21+ messages in thread
From: Stefan Roese @ 2021-10-08 6:27 UTC (permalink / raw)
To: Marek Behún; +Cc: u-boot, pali
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
--
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH u-boot-marvell 4/9] arm: mvebu: a38x: serdes: Don't overwrite read-only SAR PCIe registers
2021-09-24 20:59 [PATCH u-boot-marvell 0/9] a38x serdes cleanup Marek Behún
` (2 preceding siblings ...)
2021-09-24 20:59 ` [PATCH u-boot-marvell 3/9] arm: mvebu: a38x: serdes: Add comments for hws_pex_config() code Marek Behún
@ 2021-09-24 20:59 ` Marek Behún
2021-10-08 6:27 ` Stefan Roese
2021-09-24 20:59 ` [PATCH u-boot-marvell 5/9] arm: mvebu: a38x: serdes: Don't set PCIe Common Clock Configuration Marek Behún
` (6 subsequent siblings)
10 siblings, 1 reply; 21+ messages in thread
From: Marek Behún @ 2021-09-24 20:59 UTC (permalink / raw)
To: Stefan Roese; +Cc: u-boot, pali, Marek Behún
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
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH u-boot-marvell 4/9] arm: mvebu: a38x: serdes: Don't overwrite read-only SAR PCIe registers
2021-09-24 20:59 ` [PATCH u-boot-marvell 4/9] arm: mvebu: a38x: serdes: Don't overwrite read-only SAR PCIe registers Marek Behún
@ 2021-10-08 6:27 ` Stefan Roese
0 siblings, 0 replies; 21+ messages in thread
From: Stefan Roese @ 2021-10-08 6:27 UTC (permalink / raw)
To: Marek Behún; +Cc: u-boot, pali
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
--
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH u-boot-marvell 5/9] arm: mvebu: a38x: serdes: Don't set PCIe Common Clock Configuration
2021-09-24 20:59 [PATCH u-boot-marvell 0/9] a38x serdes cleanup Marek Behún
` (3 preceding siblings ...)
2021-09-24 20:59 ` [PATCH u-boot-marvell 4/9] arm: mvebu: a38x: serdes: Don't overwrite read-only SAR PCIe registers Marek Behún
@ 2021-09-24 20:59 ` Marek Behún
2021-10-08 6:28 ` Stefan Roese
2021-09-24 20:59 ` [PATCH u-boot-marvell 6/9] arm: mvebu: a38x: serdes: Don't overwrite PCI device ID Marek Behún
` (5 subsequent siblings)
10 siblings, 1 reply; 21+ messages in thread
From: Marek Behún @ 2021-09-24 20:59 UTC (permalink / raw)
To: Stefan Roese; +Cc: u-boot, pali, Marek Behún
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
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH u-boot-marvell 5/9] arm: mvebu: a38x: serdes: Don't set PCIe Common Clock Configuration
2021-09-24 20:59 ` [PATCH u-boot-marvell 5/9] arm: mvebu: a38x: serdes: Don't set PCIe Common Clock Configuration Marek Behún
@ 2021-10-08 6:28 ` Stefan Roese
0 siblings, 0 replies; 21+ messages in thread
From: Stefan Roese @ 2021-10-08 6:28 UTC (permalink / raw)
To: Marek Behún; +Cc: u-boot, pali
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
--
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH u-boot-marvell 6/9] arm: mvebu: a38x: serdes: Don't overwrite PCI device ID
2021-09-24 20:59 [PATCH u-boot-marvell 0/9] a38x serdes cleanup Marek Behún
` (4 preceding siblings ...)
2021-09-24 20:59 ` [PATCH u-boot-marvell 5/9] arm: mvebu: a38x: serdes: Don't set PCIe Common Clock Configuration Marek Behún
@ 2021-09-24 20:59 ` Marek Behún
2021-10-08 6:28 ` Stefan Roese
2021-09-24 20:59 ` [PATCH u-boot-marvell 7/9] arm: mvebu: a38x: serdes: Don't configure PCIe cards in SerDes init code Marek Behún
` (4 subsequent siblings)
10 siblings, 1 reply; 21+ messages in thread
From: Marek Behún @ 2021-09-24 20:59 UTC (permalink / raw)
To: Stefan Roese; +Cc: u-boot, pali, Marek Behún
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
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH u-boot-marvell 6/9] arm: mvebu: a38x: serdes: Don't overwrite PCI device ID
2021-09-24 20:59 ` [PATCH u-boot-marvell 6/9] arm: mvebu: a38x: serdes: Don't overwrite PCI device ID Marek Behún
@ 2021-10-08 6:28 ` Stefan Roese
0 siblings, 0 replies; 21+ messages in thread
From: Stefan Roese @ 2021-10-08 6:28 UTC (permalink / raw)
To: Marek Behún; +Cc: u-boot, pali
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
--
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH u-boot-marvell 7/9] arm: mvebu: a38x: serdes: Don't configure PCIe cards in SerDes init code
2021-09-24 20:59 [PATCH u-boot-marvell 0/9] a38x serdes cleanup Marek Behún
` (5 preceding siblings ...)
2021-09-24 20:59 ` [PATCH u-boot-marvell 6/9] arm: mvebu: a38x: serdes: Don't overwrite PCI device ID Marek Behún
@ 2021-09-24 20:59 ` Marek Behún
2021-10-08 6:29 ` Stefan Roese
2021-09-24 20:59 ` [PATCH u-boot-marvell 8/9] arm: mvebu: a38x: serdes: Remove unused PCIe macros and functions Marek Behún
` (3 subsequent siblings)
10 siblings, 1 reply; 21+ messages in thread
From: Marek Behún @ 2021-09-24 20:59 UTC (permalink / raw)
To: Stefan Roese; +Cc: u-boot, pali, Marek Behún
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
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH u-boot-marvell 7/9] arm: mvebu: a38x: serdes: Don't configure PCIe cards in SerDes init code
2021-09-24 20:59 ` [PATCH u-boot-marvell 7/9] arm: mvebu: a38x: serdes: Don't configure PCIe cards in SerDes init code Marek Behún
@ 2021-10-08 6:29 ` Stefan Roese
0 siblings, 0 replies; 21+ messages in thread
From: Stefan Roese @ 2021-10-08 6:29 UTC (permalink / raw)
To: Marek Behún; +Cc: u-boot, pali
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(-)
>
> 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;
> }
>
>
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH u-boot-marvell 8/9] arm: mvebu: a38x: serdes: Remove unused PCIe macros and functions
2021-09-24 20:59 [PATCH u-boot-marvell 0/9] a38x serdes cleanup Marek Behún
` (6 preceding siblings ...)
2021-09-24 20:59 ` [PATCH u-boot-marvell 7/9] arm: mvebu: a38x: serdes: Don't configure PCIe cards in SerDes init code Marek Behún
@ 2021-09-24 20:59 ` Marek Behún
2021-10-08 6:29 ` Stefan Roese
2021-09-24 20:59 ` [PATCH u-boot-marvell 9/9] arm: mvebu: a38x: serdes: Update comment about PCIE*_ENABLE_* defines Marek Behún
` (2 subsequent siblings)
10 siblings, 1 reply; 21+ messages in thread
From: Marek Behún @ 2021-09-24 20:59 UTC (permalink / raw)
To: Stefan Roese; +Cc: u-boot, pali, Marek Behún
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
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH u-boot-marvell 8/9] arm: mvebu: a38x: serdes: Remove unused PCIe macros and functions
2021-09-24 20:59 ` [PATCH u-boot-marvell 8/9] arm: mvebu: a38x: serdes: Remove unused PCIe macros and functions Marek Behún
@ 2021-10-08 6:29 ` Stefan Roese
0 siblings, 0 replies; 21+ messages in thread
From: Stefan Roese @ 2021-10-08 6:29 UTC (permalink / raw)
To: Marek Behún; +Cc: u-boot, pali
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
--
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH u-boot-marvell 9/9] arm: mvebu: a38x: serdes: Update comment about PCIE*_ENABLE_* defines
2021-09-24 20:59 [PATCH u-boot-marvell 0/9] a38x serdes cleanup Marek Behún
` (7 preceding siblings ...)
2021-09-24 20:59 ` [PATCH u-boot-marvell 8/9] arm: mvebu: a38x: serdes: Remove unused PCIe macros and functions Marek Behún
@ 2021-09-24 20:59 ` Marek Behún
2021-10-08 6:30 ` Stefan Roese
2021-10-08 6:31 ` [PATCH u-boot-marvell 0/9] a38x serdes cleanup Stefan Roese
2021-10-08 9:18 ` Stefan Roese
10 siblings, 1 reply; 21+ messages in thread
From: Marek Behún @ 2021-09-24 20:59 UTC (permalink / raw)
To: Stefan Roese; +Cc: u-boot, pali, Marek Behún
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
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH u-boot-marvell 9/9] arm: mvebu: a38x: serdes: Update comment about PCIE*_ENABLE_* defines
2021-09-24 20:59 ` [PATCH u-boot-marvell 9/9] arm: mvebu: a38x: serdes: Update comment about PCIE*_ENABLE_* defines Marek Behún
@ 2021-10-08 6:30 ` Stefan Roese
0 siblings, 0 replies; 21+ messages in thread
From: Stefan Roese @ 2021-10-08 6:30 UTC (permalink / raw)
To: Marek Behún; +Cc: u-boot, pali
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
--
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH u-boot-marvell 0/9] a38x serdes cleanup
2021-09-24 20:59 [PATCH u-boot-marvell 0/9] a38x serdes cleanup Marek Behún
` (8 preceding siblings ...)
2021-09-24 20:59 ` [PATCH u-boot-marvell 9/9] arm: mvebu: a38x: serdes: Update comment about PCIE*_ENABLE_* defines Marek Behún
@ 2021-10-08 6:31 ` Stefan Roese
2021-10-08 9:18 ` Stefan Roese
10 siblings, 0 replies; 21+ messages in thread
From: Stefan Roese @ 2021-10-08 6:31 UTC (permalink / raw)
To: Marek Behún; +Cc: u-boot, pali
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
--
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
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH u-boot-marvell 0/9] a38x serdes cleanup
2021-09-24 20:59 [PATCH u-boot-marvell 0/9] a38x serdes cleanup Marek Behún
` (9 preceding siblings ...)
2021-10-08 6:31 ` [PATCH u-boot-marvell 0/9] a38x serdes cleanup Stefan Roese
@ 2021-10-08 9:18 ` Stefan Roese
10 siblings, 0 replies; 21+ messages in thread
From: Stefan Roese @ 2021-10-08 9:18 UTC (permalink / raw)
To: Marek Behún; +Cc: u-boot, pali
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
^ permalink raw reply [flat|nested] 21+ messages in thread