Excerpts from Paul Mackerras's message of June 18, 2021 10:12 pm: > On Fri, Jun 18, 2021 at 05:40:40PM +1000, Nicholas Piggin wrote: >> Excerpts from Paul Mackerras's message of June 18, 2021 1:46 pm: >> > From: Benjamin Herrenschmidt <b...@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 <b...@kernel.crashing.org> >> > Signed-off-by: Paul Mackerras <pau...@ozlabs.org> > ... >> > +#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? > > Because it's EARLY debug, for use in the very early stages of boot > when the kernel's radix tree may or may not have been initialized. > The easiest way to make a function that works correctly whether or not > the radix tree has been initialized and the MMU turned on is to > temporarily turn off the MMU for data accesses and use lbzcix/stbcix
Ah makes sense. > (which Microwatt has, even though it doesn't implement hypervisor > mode). > > (I don't know which "yet" you meant - "yet" in the process of booting a > kernel, or "yet" in the process of Microwatt's development? Microwatt > certainly does have cache-inhibited mappings and has done since the > MMU was first introduced.) I did mean mappings to the UART, but good to get both answers :D > > In fact the defconfig I add later in the series doesn't enable > CONFIG_PPC_EARLY_DEBUG_MICROWATT, but it's there if it's needed for > debugging. > >> 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. > > Right, and in fact mtmsrd is marked as a single-issue instruction in > Microwatt, so it should work with no isyncs or eieios. Presumably Ben > copied the isync/eieio pattern from somewhere else. Makes sense. Well I don't have any objection to the series. Thanks, Nick