Re: [PATCH v2 5/9] powerpc/microwatt: Use standard 16550 UART for console

[Christophe Leroy <christophe.leroy@csgroup.eu>]

Le 18/06/2021 à 05:46, Paul Mackerras a écrit :
> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> 
> This adds support to the Microwatt platform to use the standard
> 16550-style UART which available in the standalone Microwatt FPGA.
> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Signed-off-by: Paul Mackerras <paulus@ozlabs.org>
> ---
>   arch/powerpc/boot/dts/microwatt.dts      | 27 ++++++++++++----
>   arch/powerpc/kernel/udbg_16550.c         | 39 ++++++++++++++++++++++++
>   arch/powerpc/platforms/microwatt/Kconfig |  1 +
>   arch/powerpc/platforms/microwatt/setup.c |  2 ++
>   4 files changed, 63 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/powerpc/boot/dts/microwatt.dts b/arch/powerpc/boot/dts/microwatt.dts
> index 04e5dd92270e..974abbdda249 100644
> --- a/arch/powerpc/boot/dts/microwatt.dts
> +++ b/arch/powerpc/boot/dts/microwatt.dts
> @@ -6,6 +6,10 @@ / {
>   	model-name = "microwatt";
>   	compatible = "microwatt-soc";
>   
> +	aliases {
> +		serial0 = &UART0;
> +	};
> +
>   	reserved-memory {
>   		#size-cells = <0x02>;
>   		#address-cells = <0x02>;
> @@ -89,12 +93,6 @@ PowerPC,Microwatt@0 {
>   		};
>   	};
>   
> -	chosen {
> -		bootargs = "";
> -		ibm,architecture-vec-5 = [19 00 10 00 00 00 00 00 00 00 00 00 00 00 00 00
> -					  00 00 00 00 00 00 00 00 40 00 40];
> -	};
> -
>   	soc@c0000000 {
>   		compatible = "simple-bus";
>   		#address-cells = <1>;
> @@ -119,5 +117,22 @@ ICS: interrupt-controller@5000 {
>   			#interrupt-cells = <2>;
>   		};
>   
> +		UART0: serial@2000 {
> +			device_type = "serial";
> +			compatible = "ns16550";
> +			reg = <0x2000 0x8>;
> +			clock-frequency = <100000000>;
> +			current-speed = <115200>;
> +			reg-shift = <2>;
> +			fifo-size = <16>;
> +			interrupts = <0x10 0x1>;
> +		};
> +	};
> +
> +	chosen {
> +		bootargs = "";
> +		ibm,architecture-vec-5 = [19 00 10 00 00 00 00 00 00 00 00 00 00 00 00 00
> +					  00 00 00 00 00 00 00 00 40 00 40];
> +		stdout-path = &UART0;
>   	};
>   };
> diff --git a/arch/powerpc/kernel/udbg_16550.c b/arch/powerpc/kernel/udbg_16550.c
> index 9356b60d6030..8513aa49614e 100644
> --- a/arch/powerpc/kernel/udbg_16550.c
> +++ b/arch/powerpc/kernel/udbg_16550.c
> @@ -296,3 +296,42 @@ void __init udbg_init_40x_realmode(void)
>   }
>   
>   #endif /* CONFIG_PPC_EARLY_DEBUG_40x */
> +
> +#ifdef CONFIG_PPC_EARLY_DEBUG_MICROWATT
> +
> +#define UDBG_UART_MW_ADDR	((void __iomem *)0xc0002000)
> +
> +static u8 udbg_uart_in_isa300_rm(unsigned int reg)
> +{
> +	uint64_t msr = mfmsr();
> +	uint8_t  c;
> +
> +	mtmsr(msr & ~(MSR_EE|MSR_DR));
> +	isync();
> +	eieio();
> +	c = __raw_rm_readb(UDBG_UART_MW_ADDR + (reg << 2));
> +	mtmsr(msr);
> +	isync();
> +	return c;
> +}

How do you make sure that GCC won't emit any access to the stack between the two mtmsr() ?

What about using real_205_readb() and real_205_writeb() instead ?

> +
> +static void udbg_uart_out_isa300_rm(unsigned int reg, u8 val)
> +{
> +	uint64_t msr = mfmsr();
> +
> +	mtmsr(msr & ~(MSR_EE|MSR_DR));
> +	isync();
> +	eieio();
> +	__raw_rm_writeb(val, UDBG_UART_MW_ADDR + (reg << 2));
> +	mtmsr(msr);
> +	isync();
> +}
> +
> +void __init udbg_init_debug_microwatt(void)
> +{
> +	udbg_uart_in = udbg_uart_in_isa300_rm;
> +	udbg_uart_out = udbg_uart_out_isa300_rm;
> +	udbg_use_uart();
> +}
> +
> +#endif /* CONFIG_PPC_EARLY_DEBUG_MICROWATT */
> diff --git a/arch/powerpc/platforms/microwatt/Kconfig b/arch/powerpc/platforms/microwatt/Kconfig
> index b52c869c0eb8..50ed0cedb5f1 100644
> --- a/arch/powerpc/platforms/microwatt/Kconfig
> +++ b/arch/powerpc/platforms/microwatt/Kconfig
> @@ -6,6 +6,7 @@ config PPC_MICROWATT
>   	select PPC_ICS_NATIVE
>   	select PPC_ICP_NATIVE
>   	select PPC_NATIVE
> +	select PPC_UDBG_16550
>   	help
>             This option enables support for FPGA-based Microwatt implementations.
>   
> diff --git a/arch/powerpc/platforms/microwatt/setup.c b/arch/powerpc/platforms/microwatt/setup.c
> index 1c1b7791fa57..0b02603bdb74 100644
> --- a/arch/powerpc/platforms/microwatt/setup.c
> +++ b/arch/powerpc/platforms/microwatt/setup.c
> @@ -14,6 +14,7 @@
>   #include <asm/machdep.h>
>   #include <asm/time.h>
>   #include <asm/xics.h>
> +#include <asm/udbg.h>
>   
>   static void __init microwatt_init_IRQ(void)
>   {
> @@ -35,5 +36,6 @@ define_machine(microwatt) {
>   	.name			= "microwatt",
>   	.probe			= microwatt_probe,
>   	.init_IRQ		= microwatt_init_IRQ,
> +	.progress		= udbg_progress,
>   	.calibrate_decr		= generic_calibrate_decr,
>   };
>