Re: [PATCH v2] xen: remove <asm/delay.h>

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

On Fri, 2023-11-10 at 10:33 +0100, Jan Beulich wrote:
> On 10.11.2023 10:30, Oleksii wrote:
> > On Tue, 2023-10-31 at 16:28 +0200, Oleksii Kurochko wrote:
> > > <asm/delay.h> only declares udelay() function so udelay()
> > > declaration was moved to xen/delay.h.
> > > 
> > > For x86, __udelay() was renamed to udelay() and removed
> > > inclusion of <asm/delay.h> in x86 code.
> > > 
> > > For ppc, udelay() stub definition was moved to ppc/stubs.c.
> > > 
> > > Suggested-by: Jan Beulich <jbeulich@xxxxxxxx>
> > > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> > > Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> > > ---
> > > Changes in v2:
> > >  - rebase on top of the latest staging.
> > >  - add Suggested-by:/Reviewed-by: Jan Beulich
> > > <jbeulich@xxxxxxxx>.
> > >  - remove <asm/delay.h> for PPC.
> > >  - remove changes related to RISC-V's <asm/delay.h> as they've
> > > not
> > >    introduced in staging branch yet.
> > > ---
> > >  xen/arch/arm/include/asm/delay.h  | 14 --------------
> > >  xen/arch/ppc/include/asm/delay.h  | 12 ------------
> > >  xen/arch/ppc/stubs.c              |  7 +++++++
> > >  xen/arch/x86/cpu/microcode/core.c |  2 +-
> > >  xen/arch/x86/delay.c              |  2 +-
> > >  xen/arch/x86/include/asm/delay.h  | 13 -------------
> > >  xen/include/xen/delay.h           |  2 +-
> > >  7 files changed, 10 insertions(+), 42 deletions(-)
> > >  delete mode 100644 xen/arch/arm/include/asm/delay.h
> > >  delete mode 100644 xen/arch/ppc/include/asm/delay.h
> > >  delete mode 100644 xen/arch/x86/include/asm/delay.h
> > > 
> > > diff --git a/xen/arch/arm/include/asm/delay.h
> > > b/xen/arch/arm/include/asm/delay.h
> > > deleted file mode 100644
> > > index 042907d9d5..0000000000
> > > --- a/xen/arch/arm/include/asm/delay.h
> > > +++ /dev/null
> > > @@ -1,14 +0,0 @@
> > > -#ifndef _ARM_DELAY_H
> > > -#define _ARM_DELAY_H
> > > -
> > > -extern void udelay(unsigned long usecs);
> > > -
> > > -#endif /* defined(_ARM_DELAY_H) */
> > > -/*
> > > - * Local variables:
> > > - * mode: C
> > > - * c-file-style: "BSD"
> > > - * c-basic-offset: 4
> > > - * indent-tabs-mode: nil
> > > - * End:
> > > - */
> > > diff --git a/xen/arch/ppc/include/asm/delay.h
> > > b/xen/arch/ppc/include/asm/delay.h
> > > deleted file mode 100644
> > > index da6635888b..0000000000
> > > --- a/xen/arch/ppc/include/asm/delay.h
> > > +++ /dev/null
> > > @@ -1,12 +0,0 @@
> > > -/* SPDX-License-Identifier: GPL-2.0-only */
> > > -#ifndef __ASM_PPC_DELAY_H__
> > > -#define __ASM_PPC_DELAY_H__
> > > -
> > > -#include <xen/lib.h>
> > > -
> > > -static inline void udelay(unsigned long usecs)
> > > -{
> > > -    BUG_ON("unimplemented");
> > > -}
> > > -
> > > -#endif /* __ASM_PPC_DELAY_H__ */
> > > diff --git a/xen/arch/ppc/stubs.c b/xen/arch/ppc/stubs.c
> > > index 4c276b0e39..a96e45626d 100644
> > > --- a/xen/arch/ppc/stubs.c
> > > +++ b/xen/arch/ppc/stubs.c
> > > @@ -337,3 +337,10 @@ int __init parse_arch_dom0_param(const char
> > > *s,
> > > const char *e)
> > >  {
> > >      BUG_ON("unimplemented");
> > >  }
> > > +
> > > +/* delay.c */
> > > +
> > > +void udelay(unsigned long usecs)
> > > +{
> > > +    BUG_ON("unimplemented");
> > > +}
> > > diff --git a/xen/arch/x86/cpu/microcode/core.c
> > > b/xen/arch/x86/cpu/microcode/core.c
> > > index 65ebeb50de..22d5e04552 100644
> > > --- a/xen/arch/x86/cpu/microcode/core.c
> > > +++ b/xen/arch/x86/cpu/microcode/core.c
> > > @@ -23,6 +23,7 @@
> > >  
> > >  #include <xen/alternative-call.h>
> > >  #include <xen/cpu.h>
> > > +#include <xen/delay.h>
> > >  #include <xen/earlycpio.h>
> > >  #include <xen/err.h>
> > >  #include <xen/guest_access.h>
> > > @@ -35,7 +36,6 @@
> > >  
> > >  #include <asm/apic.h>
> > >  #include <asm/cpu-policy.h>
> > > -#include <asm/delay.h>
> > >  #include <asm/nmi.h>
> > >  #include <asm/processor.h>
> > >  #include <asm/setup.h>
> > > diff --git a/xen/arch/x86/delay.c b/xen/arch/x86/delay.c
> > > index 2662c26272..b3a41881a1 100644
> > > --- a/xen/arch/x86/delay.c
> > > +++ b/xen/arch/x86/delay.c
> > > @@ -15,7 +15,7 @@
> > >  #include <asm/msr.h>
> > >  #include <asm/processor.h>
> > >  
> > > -void __udelay(unsigned long usecs)
> > > +void udelay(unsigned long usecs)
> > >  {
> > >      unsigned long ticks = usecs * (cpu_khz / 1000);
> > >      unsigned long s, e;
> > > diff --git a/xen/arch/x86/include/asm/delay.h
> > > b/xen/arch/x86/include/asm/delay.h
> > > deleted file mode 100644
> > > index 9be2f46590..0000000000
> > > --- a/xen/arch/x86/include/asm/delay.h
> > > +++ /dev/null
> > > @@ -1,13 +0,0 @@
> > > -#ifndef _X86_DELAY_H
> > > -#define _X86_DELAY_H
> > > -
> > > -/*
> > > - * Copyright (C) 1993 Linus Torvalds
> > > - *
> > > - * Delay routines calling functions in arch/i386/lib/delay.c
> > > - */
> > > -
> > > -extern void __udelay(unsigned long usecs);
> > > -#define udelay(n) __udelay(n)
> > > -
> > > -#endif /* defined(_X86_DELAY_H) */
> > > diff --git a/xen/include/xen/delay.h b/xen/include/xen/delay.h
> > > index 9150226271..8fd3b8f99f 100644
> > > --- a/xen/include/xen/delay.h
> > > +++ b/xen/include/xen/delay.h
> > > @@ -3,7 +3,7 @@
> > >  
> > >  /* Copyright (C) 1993 Linus Torvalds */
> > >  
> > > -#include <asm/delay.h>
> > > +void udelay(unsigned long usecs);
> > >  
> > >  static inline void mdelay(unsigned long msec)
> > >  {
> > 
> > I forgot to update xen/arch/{x86,arm,ppc}/include/asm/Makefile:
> >     generic-y += delay.h
> > 
> > Should I send a new patch version or it can be updated durig merge?
> 
> My take is that such a change wouldn't be appropriate to do by the
> committer. But: Why would this be needed? You're not introducing
> asm-generic/delay.h. Everything is moved to just xen/delay.h.
You are right. I started to rework other patches of generic headers
patch series and there are places where asm/Makefile should be updated
so automatically decided that it is needed here too.
My fault.

Thanks.

~ Oleksii