Re: [PATCH 2/3] xen/ppc: Implement early serial printk on PaPR/pseries

["Shawn Anastasio" <shawn@xxxxxxxxxx>]

On Fri Jun 9, 2023 at 4:22 AM CDT, Jan Beulich wrote:
> >  xen/arch/ppc/Kconfig.debug               |   5 +
> >  xen/arch/ppc/Makefile                    |   3 +-
> >  xen/arch/ppc/boot_of.c                   | 122 +++++++++++++++++++++++
> >  xen/arch/ppc/configs/openpower_defconfig |   1 +
> >  xen/arch/ppc/early_printk.c              |  36 +++++++
> >  xen/arch/ppc/include/asm/boot.h          |  31 ++++++
> >  xen/arch/ppc/include/asm/bug.h           |   6 ++
> >  xen/arch/ppc/include/asm/byteorder.h     |  74 ++++++++++++++
> >  xen/arch/ppc/include/asm/cache.h         |   6 ++
> >  xen/arch/ppc/include/asm/config.h        |   3 +
> >  xen/arch/ppc/include/asm/early_printk.h  |  14 +++
> >  xen/arch/ppc/include/asm/processor.h     |  54 +++++++++-
> >  xen/arch/ppc/include/asm/string.h        |   6 ++
> >  xen/arch/ppc/include/asm/types.h         |  64 ++++++++++++
> >  xen/arch/ppc/ppc64/asm-offsets.c         |  55 ++++++++++
> >  xen/arch/ppc/ppc64/head.S                |  59 +++++++++++
> >  xen/arch/ppc/setup.c                     |  20 +++-
> >  17 files changed, 555 insertions(+), 4 deletions(-)
> >  create mode 100644 xen/arch/ppc/boot_of.c
>
> Unless required, in new additions we tend to prefer dashes over
> underscores. In filenames it is pretty rare that dashes really need
> avoiding.

Thanks for pointing that out -- I'll fix this in v2.

> > --- a/xen/arch/ppc/Kconfig.debug
> > +++ b/xen/arch/ppc/Kconfig.debug
> > @@ -0,0 +1,5 @@
> > +config EARLY_PRINTK
> > +    bool "Enable early printk"
> > +    default DEBUG
> > +    help
> > +      Enables early printk debug messages
> > \ No newline at end of file
>
> There are many examples of this throughout the patch, which you want to
> take care of.

Ditto.

> > --- /dev/null
> > +++ b/xen/arch/ppc/boot_of.c
> > @@ -0,0 +1,122 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
>
> By default we mean to use ...
>
> > --- /dev/null
> > +++ b/xen/arch/ppc/early_printk.c
> > @@ -0,0 +1,36 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
>
> ... the more modern form of this (GPL-2.0-only). Anything deviating from
> that may want justifying in the description.

Got it -- I'll clean this up as well.

> Jan

Thanks,
Shawn