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

[Nicholas Piggin <npiggin@gmail.com>]

Excerpts from Paul Mackerras's message of June 18, 2021 1:46 pm:
> 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;
> +}

Why is realmode required? No cache inhibited mappings yet?

mtmsrd with L=0 is defined to be context synchronizing in isa 3, so I 
don't think the isync would be required. There is a bit of code around 
arch/powerpc that does this, maybe it used to be needed or some other
implementations needed it?

That's just for my curiosity, it doesn't really hurt to have them
there.

Thanks,
Nick