[PATCH v2 0/4] Misc. PowerPC MPC83xx fixes/cleanups
This patchset contains a few small fixes/cleanups for the MPC83xx platform.
Signed-off-by: J. Neuschäfer j.ne@posteo.net --- Changes in v2: - Address feedback from Sinan Akman by clarifying some commit messages - Link to v1: https://lore.kernel.org/r/20241215-mpc83xx-misc-v1-0-8cd6daae4f93@posteo.net
--- J. Neuschäfer (4): powerpc: mpc83xx: Fix timer value calculation powerpc: mpc83xx: Allow including initreg.h into multiple files powerpc: mpc83xx: Use defined constant for SPCR[TBEN] gpio: mpc8xxx: Preserve pre-init state of outputs
arch/powerpc/cpu/mpc83xx/initreg/initreg.h | 8 ++++---- arch/powerpc/cpu/mpc83xx/interrupts.c | 3 ++- drivers/gpio/mpc8xxx_gpio.c | 12 +++++++++++- drivers/timer/mpc83xx_timer.c | 2 +- 4 files changed, 18 insertions(+), 7 deletions(-) --- base-commit: 337bfdce5aa52637b482b182db959b9bb058f4b7 change-id: 20241206-mpc83xx-misc-f438f7e054d8
Best regards,
From: "J. Neuschäfer" j.ne@posteo.net
TBU and TBL are specified as two 32-bit registers that form a 64-bit value, but the calculation only shifted TBU by 16 bits.
Fix this by actually shifting 32 bits.
Reviewed-by: Sinan Akman sinan@writeme.com Signed-off-by: J. Neuschäfer j.ne@posteo.net --- drivers/timer/mpc83xx_timer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/timer/mpc83xx_timer.c b/drivers/timer/mpc83xx_timer.c index 9da74479aaa678bfab861950a770b216b604c433..f92009e4ccc8256d4c5f4d11e414dfc141f5a043 100644 --- a/drivers/timer/mpc83xx_timer.c +++ b/drivers/timer/mpc83xx_timer.c @@ -206,7 +206,7 @@ static u64 mpc83xx_timer_get_count(struct udevice *dev) tbl = mftb(); } while (tbu != mftbu());
- return (tbu * 0x10000ULL) + tbl; + return (uint64_t)tbu << 32 | tbl; }
static int mpc83xx_timer_probe(struct udevice *dev)
From: "J. Neuschäfer" j.ne@posteo.net
Globals defined in headers can result in multiple-definition errors while linking, if they are visible beyond the current translation unit.
This hasn't been a problem for initreg.h so far, but would become a problem in the next patch, where I use a constant from initreg.h in a second C file.
Reviewed-by: Sinan Akman sinan@writeme.com Signed-off-by: J. Neuschäfer j.ne@posteo.net ---
v2: - Clarify when the visibility issue becomes a problem --- arch/powerpc/cpu/mpc83xx/initreg/initreg.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/cpu/mpc83xx/initreg/initreg.h b/arch/powerpc/cpu/mpc83xx/initreg/initreg.h index 63aa5c946696ee0368bb3453b40ff0110f0fbcfd..ea1176e7fe10dbb549125ab6b2706fee05a92734 100644 --- a/arch/powerpc/cpu/mpc83xx/initreg/initreg.h +++ b/arch/powerpc/cpu/mpc83xx/initreg/initreg.h @@ -13,7 +13,7 @@ #define SPCR_TSECBDP_MASK 0x00000C00 #define SPCR_TSECEP_MASK 0x00000300
- const __be32 spcr_mask = + static const __be32 spcr_mask = #if defined(CONFIG_SPCR_OPT) && !defined(CONFIG_SPCR_OPT_UNSET) SPCR_OPT_MASK | #endif @@ -27,7 +27,7 @@ SPCR_TSEC2EP_MASK | #endif 0; - const __be32 spcr_val = + static const __be32 spcr_val = #if defined(CONFIG_SPCR_OPT) && !defined(CONFIG_SPCR_OPT_UNSET) CONFIG_SPCR_OPT | #endif @@ -42,7 +42,7 @@ #endif 0;
- const __be32 lcrr_mask = + static const __be32 lcrr_mask = #if defined(CONFIG_LCRR_DBYP) && !defined(CONFIG_LCRR_DBYP_UNSET) LCRR_DBYP | #endif @@ -60,7 +60,7 @@ #endif 0;
- const __be32 lcrr_val = + static const __be32 lcrr_val = #if defined(CONFIG_LCRR_DBYP) && !defined(CONFIG_LCRR_DBYP_UNSET) CONFIG_LCRR_DBYP | #endif
From: "J. Neuschäfer" j.ne@posteo.net
To increase readability, use the defined constant instead of specifying SPCR[TBEN] as a number.
Reviewed-by: Sinan Akman sinan@writeme.com Signed-off-by: J. Neuschäfer j.ne@posteo.net --- arch/powerpc/cpu/mpc83xx/interrupts.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/cpu/mpc83xx/interrupts.c b/arch/powerpc/cpu/mpc83xx/interrupts.c index af517213f176cf59cc82b54dbe5247800a6c40e6..bb37a1332c877951e766a8891687dae3f05b6ef8 100644 --- a/arch/powerpc/cpu/mpc83xx/interrupts.c +++ b/arch/powerpc/cpu/mpc83xx/interrupts.c @@ -12,6 +12,7 @@ #include <asm/global_data.h> #include <asm/processor.h> #include <asm/ptrace.h> +#include "initreg/initreg.h"
DECLARE_GLOBAL_DATA_PTR;
@@ -29,7 +30,7 @@ void interrupt_init_cpu (unsigned *decrementer_count)
/* Enable e300 time base */
- immr->sysconf.spcr |= 0x00400000; + immr->sysconf.spcr |= SPCR_TBEN_MASK; }
/*
From: "J. Neuschäfer" j.ne@posteo.net
The mpc8xxx_gpio driver contains a workaround for certain chips where the previously written state of outputs cannot be read back from the GPIO data (GPDAT) register (MPC8572/MPC8536). This workaround consists of tracking the state of GPDAT in a "shadow register" (i.e. a software variable). The shadow register is initialized to zero.
This results in a problem w.r.t. outputs that are configured to a high (1) state before U-Boot runs, but not touched by U-Boot itself: Due to the zero-initialization, these GPIOs end up being set to zero, the first time that any other output is set.
To avoid such issues initialize the GPDAT shadow register to the value previously held by any outputs, if possible. On MPC8572/MPC8536 this should make no difference, i.e. the shadow register should be initialized to zero on these chips.
This patch has been tested on a MPC8314E-based board.
Reviewed-by: Sinan Akman sinan@writeme.com Signed-off-by: J. Neuschäfer j.ne@posteo.net ---
V2: - Clarify how the patch has been tested --- drivers/gpio/mpc8xxx_gpio.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/gpio/mpc8xxx_gpio.c b/drivers/gpio/mpc8xxx_gpio.c index e9bd38f162c10c76460bc63f93c03a711cb4d38e..709d04017d154ecae1e8d16d7c35ced521157c4d 100644 --- a/drivers/gpio/mpc8xxx_gpio.c +++ b/drivers/gpio/mpc8xxx_gpio.c @@ -204,7 +204,17 @@ static int mpc8xxx_gpio_plat_to_priv(struct udevice *dev) return -ENOMEM;
priv->gpio_count = plat->ngpios; - priv->dat_shadow = 0; + + /* + * On platforms that do support reading back output values, we want to + * try preserving them, so that we don't accidentally set unrelated + * GPIOs to zero in mpc8xxx_gpio_set_value. + */ + if (priv->little_endian) + priv->dat_shadow = in_le32(&priv->base->gpdat) & in_le32(&priv->base->gpdir); + else + priv->dat_shadow = in_be32(&priv->base->gpdat) & in_be32(&priv->base->gpdir); +
priv->type = driver_data;
participants (2)
-
J. Neuschäfer via B4 Relay -
Tom Rini