| Message ID | 20190930144112.175618-40-sjg@chromium.org |
|---|---|
| State | Changes Requested |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | common: Further reduce common.h | expand |
On Mon, Sep 30, 2019 at 4:58 PM Simon Glass <sjg@chromium.org> wrote: > > Move this function into the irq_legacy.h header file. > > Signed-off-by: Simon Glass <sjg@chromium.org> > --- > > arch/m68k/lib/traps.c | 1 + > arch/mips/lib/traps.c | 1 + > include/common.h | 1 - > include/irq_legacy.h | 2 ++ > 4 files changed, 4 insertions(+), 1 deletion(-) > > diff --git a/arch/m68k/lib/traps.c b/arch/m68k/lib/traps.c > index 5d802077453..b2e7e9fe407 100644 > --- a/arch/m68k/lib/traps.c > +++ b/arch/m68k/lib/traps.c > @@ -8,6 +8,7 @@ > */ > > #include <common.h> > +#include <irq_legacy.h> > #include <watchdog.h> > #include <command.h> > #include <asm/processor.h> > diff --git a/arch/mips/lib/traps.c b/arch/mips/lib/traps.c > index ff02d437a87..d173a1b0134 100644 > --- a/arch/mips/lib/traps.c > +++ b/arch/mips/lib/traps.c > @@ -12,6 +12,7 @@ > > #include <common.h> > #include <cpu_legacy.h> > +#include <irq_legacy.h> > #include <asm/mipsregs.h> > #include <asm/addrspace.h> > #include <asm/system.h> > diff --git a/include/common.h b/include/common.h > index a292d919390..e946e4d9b7f 100644 > --- a/include/common.h > +++ b/include/common.h > @@ -145,7 +145,6 @@ void relocate_code(ulong); > void relocate_code(ulong, gd_t *, ulong) __attribute__ ((noreturn)); > #endif > ulong get_endaddr (void); > -void trap_init (ulong); > > void s_init(void); > > diff --git a/include/irq_legacy.h b/include/irq_legacy.h > index 91b523d043a..a029c54b702 100644 > --- a/include/irq_legacy.h > +++ b/include/irq_legacy.h > @@ -21,4 +21,6 @@ void reset_timer(void); > void enable_interrupts(void); > int disable_interrupts(void); > > +void trap_init(unsigned long reloc_addr); > + > #endif > -- hm, at least on MIPS this is regular CPU exception handling and has nothing to do with interrupts nor with DM. I don't know about M68 but there it looks like traps are used for exceptions and interrupts. I think it would better fit into a include/traps.h or so.
Hi Daniel, On Mon, 30 Sep 2019 at 09:13, Daniel Schwierzeck <daniel.schwierzeck@gmail.com> wrote: > > On Mon, Sep 30, 2019 at 4:58 PM Simon Glass <sjg@chromium.org> wrote: > > > > Move this function into the irq_legacy.h header file. > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > --- > > > > arch/m68k/lib/traps.c | 1 + > > arch/mips/lib/traps.c | 1 + > > include/common.h | 1 - > > include/irq_legacy.h | 2 ++ > > 4 files changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/arch/m68k/lib/traps.c b/arch/m68k/lib/traps.c > > index 5d802077453..b2e7e9fe407 100644 > > --- a/arch/m68k/lib/traps.c > > +++ b/arch/m68k/lib/traps.c > > @@ -8,6 +8,7 @@ > > */ > > > > #include <common.h> > > +#include <irq_legacy.h> > > #include <watchdog.h> > > #include <command.h> > > #include <asm/processor.h> > > diff --git a/arch/mips/lib/traps.c b/arch/mips/lib/traps.c > > index ff02d437a87..d173a1b0134 100644 > > --- a/arch/mips/lib/traps.c > > +++ b/arch/mips/lib/traps.c > > @@ -12,6 +12,7 @@ > > > > #include <common.h> > > #include <cpu_legacy.h> > > +#include <irq_legacy.h> > > #include <asm/mipsregs.h> > > #include <asm/addrspace.h> > > #include <asm/system.h> > > diff --git a/include/common.h b/include/common.h > > index a292d919390..e946e4d9b7f 100644 > > --- a/include/common.h > > +++ b/include/common.h > > @@ -145,7 +145,6 @@ void relocate_code(ulong); > > void relocate_code(ulong, gd_t *, ulong) __attribute__ ((noreturn)); > > #endif > > ulong get_endaddr (void); > > -void trap_init (ulong); > > > > void s_init(void); > > > > diff --git a/include/irq_legacy.h b/include/irq_legacy.h > > index 91b523d043a..a029c54b702 100644 > > --- a/include/irq_legacy.h > > +++ b/include/irq_legacy.h > > @@ -21,4 +21,6 @@ void reset_timer(void); > > void enable_interrupts(void); > > int disable_interrupts(void); > > > > +void trap_init(unsigned long reloc_addr); > > + > > #endif > > -- > > hm, at least on MIPS this is regular CPU exception handling and has > nothing to do with interrupts nor with DM. I don't know about M68 but > there it looks like traps are used for exceptions and interrupts. I > think it would better fit into a include/traps.h or so. I think we tend to think of interrupts and exceptions as being similar. Are you suggesting creating a new traps_legacy.h? Regards, Simon
On Mon, Sep 30, 2019 at 5:37 PM Simon Glass <sjg@chromium.org> wrote: > > Hi Daniel, > > On Mon, 30 Sep 2019 at 09:13, Daniel Schwierzeck > <daniel.schwierzeck@gmail.com> wrote: > > > > On Mon, Sep 30, 2019 at 4:58 PM Simon Glass <sjg@chromium.org> wrote: > > > > > > Move this function into the irq_legacy.h header file. > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > > --- > > > > > > arch/m68k/lib/traps.c | 1 + > > > arch/mips/lib/traps.c | 1 + > > > include/common.h | 1 - > > > include/irq_legacy.h | 2 ++ > > > 4 files changed, 4 insertions(+), 1 deletion(-) > > > > > > diff --git a/arch/m68k/lib/traps.c b/arch/m68k/lib/traps.c > > > index 5d802077453..b2e7e9fe407 100644 > > > --- a/arch/m68k/lib/traps.c > > > +++ b/arch/m68k/lib/traps.c > > > @@ -8,6 +8,7 @@ > > > */ > > > > > > #include <common.h> > > > +#include <irq_legacy.h> > > > #include <watchdog.h> > > > #include <command.h> > > > #include <asm/processor.h> > > > diff --git a/arch/mips/lib/traps.c b/arch/mips/lib/traps.c > > > index ff02d437a87..d173a1b0134 100644 > > > --- a/arch/mips/lib/traps.c > > > +++ b/arch/mips/lib/traps.c > > > @@ -12,6 +12,7 @@ > > > > > > #include <common.h> > > > #include <cpu_legacy.h> > > > +#include <irq_legacy.h> > > > #include <asm/mipsregs.h> > > > #include <asm/addrspace.h> > > > #include <asm/system.h> > > > diff --git a/include/common.h b/include/common.h > > > index a292d919390..e946e4d9b7f 100644 > > > --- a/include/common.h > > > +++ b/include/common.h > > > @@ -145,7 +145,6 @@ void relocate_code(ulong); > > > void relocate_code(ulong, gd_t *, ulong) __attribute__ ((noreturn)); > > > #endif > > > ulong get_endaddr (void); > > > -void trap_init (ulong); > > > > > > void s_init(void); > > > > > > diff --git a/include/irq_legacy.h b/include/irq_legacy.h > > > index 91b523d043a..a029c54b702 100644 > > > --- a/include/irq_legacy.h > > > +++ b/include/irq_legacy.h > > > @@ -21,4 +21,6 @@ void reset_timer(void); > > > void enable_interrupts(void); > > > int disable_interrupts(void); > > > > > > +void trap_init(unsigned long reloc_addr); > > > + > > > #endif > > > -- > > > > hm, at least on MIPS this is regular CPU exception handling and has > > nothing to do with interrupts nor with DM. I don't know about M68 but > > there it looks like traps are used for exceptions and interrupts. I > > think it would better fit into a include/traps.h or so. > > I think we tend to think of interrupts and exceptions as being similar. > > Are you suggesting creating a new traps_legacy.h? > either traps.h without _legacy because this code is not legacy at all. As far as I understand from your patch series, you're mainly using _legacy to separate non-DM code? Another option would be init.h because board_init_r.c:initr_trap() is the only caller.
On Mon, Sep 30, 2019 at 05:43:29PM +0200, Daniel Schwierzeck wrote: > On Mon, Sep 30, 2019 at 5:37 PM Simon Glass <sjg@chromium.org> wrote: > > > > Hi Daniel, > > > > On Mon, 30 Sep 2019 at 09:13, Daniel Schwierzeck > > <daniel.schwierzeck@gmail.com> wrote: > > > > > > On Mon, Sep 30, 2019 at 4:58 PM Simon Glass <sjg@chromium.org> wrote: > > > > > > > > Move this function into the irq_legacy.h header file. > > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > > > --- > > > > > > > > arch/m68k/lib/traps.c | 1 + > > > > arch/mips/lib/traps.c | 1 + > > > > include/common.h | 1 - > > > > include/irq_legacy.h | 2 ++ > > > > 4 files changed, 4 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/arch/m68k/lib/traps.c b/arch/m68k/lib/traps.c > > > > index 5d802077453..b2e7e9fe407 100644 > > > > --- a/arch/m68k/lib/traps.c > > > > +++ b/arch/m68k/lib/traps.c > > > > @@ -8,6 +8,7 @@ > > > > */ > > > > > > > > #include <common.h> > > > > +#include <irq_legacy.h> > > > > #include <watchdog.h> > > > > #include <command.h> > > > > #include <asm/processor.h> > > > > diff --git a/arch/mips/lib/traps.c b/arch/mips/lib/traps.c > > > > index ff02d437a87..d173a1b0134 100644 > > > > --- a/arch/mips/lib/traps.c > > > > +++ b/arch/mips/lib/traps.c > > > > @@ -12,6 +12,7 @@ > > > > > > > > #include <common.h> > > > > #include <cpu_legacy.h> > > > > +#include <irq_legacy.h> > > > > #include <asm/mipsregs.h> > > > > #include <asm/addrspace.h> > > > > #include <asm/system.h> > > > > diff --git a/include/common.h b/include/common.h > > > > index a292d919390..e946e4d9b7f 100644 > > > > --- a/include/common.h > > > > +++ b/include/common.h > > > > @@ -145,7 +145,6 @@ void relocate_code(ulong); > > > > void relocate_code(ulong, gd_t *, ulong) __attribute__ ((noreturn)); > > > > #endif > > > > ulong get_endaddr (void); > > > > -void trap_init (ulong); > > > > > > > > void s_init(void); > > > > > > > > diff --git a/include/irq_legacy.h b/include/irq_legacy.h > > > > index 91b523d043a..a029c54b702 100644 > > > > --- a/include/irq_legacy.h > > > > +++ b/include/irq_legacy.h > > > > @@ -21,4 +21,6 @@ void reset_timer(void); > > > > void enable_interrupts(void); > > > > int disable_interrupts(void); > > > > > > > > +void trap_init(unsigned long reloc_addr); > > > > + > > > > #endif > > > > -- > > > > > > hm, at least on MIPS this is regular CPU exception handling and has > > > nothing to do with interrupts nor with DM. I don't know about M68 but > > > there it looks like traps are used for exceptions and interrupts. I > > > think it would better fit into a include/traps.h or so. > > > > I think we tend to think of interrupts and exceptions as being similar. > > > > Are you suggesting creating a new traps_legacy.h? > > > > either traps.h without _legacy because this code is not legacy at all. > As far as I understand from your patch series, you're mainly using > _legacy to separate non-DM code? > > Another option would be init.h because board_init_r.c:initr_trap() is > the only caller. Yes, we need to be careful when calling something legacy or not. init.h also sounds like a good place in this case, thanks!
Hi Daniel, On Mon, 30 Sep 2019 at 09:43, Daniel Schwierzeck <daniel.schwierzeck@gmail.com> wrote: > > On Mon, Sep 30, 2019 at 5:37 PM Simon Glass <sjg@chromium.org> wrote: > > > > Hi Daniel, > > > > On Mon, 30 Sep 2019 at 09:13, Daniel Schwierzeck > > <daniel.schwierzeck@gmail.com> wrote: > > > > > > On Mon, Sep 30, 2019 at 4:58 PM Simon Glass <sjg@chromium.org> wrote: > > > > > > > > Move this function into the irq_legacy.h header file. > > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > > > --- > > > > > > > > arch/m68k/lib/traps.c | 1 + > > > > arch/mips/lib/traps.c | 1 + > > > > include/common.h | 1 - > > > > include/irq_legacy.h | 2 ++ > > > > 4 files changed, 4 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/arch/m68k/lib/traps.c b/arch/m68k/lib/traps.c > > > > index 5d802077453..b2e7e9fe407 100644 > > > > --- a/arch/m68k/lib/traps.c > > > > +++ b/arch/m68k/lib/traps.c > > > > @@ -8,6 +8,7 @@ > > > > */ > > > > > > > > #include <common.h> > > > > +#include <irq_legacy.h> > > > > #include <watchdog.h> > > > > #include <command.h> > > > > #include <asm/processor.h> > > > > diff --git a/arch/mips/lib/traps.c b/arch/mips/lib/traps.c > > > > index ff02d437a87..d173a1b0134 100644 > > > > --- a/arch/mips/lib/traps.c > > > > +++ b/arch/mips/lib/traps.c > > > > @@ -12,6 +12,7 @@ > > > > > > > > #include <common.h> > > > > #include <cpu_legacy.h> > > > > +#include <irq_legacy.h> > > > > #include <asm/mipsregs.h> > > > > #include <asm/addrspace.h> > > > > #include <asm/system.h> > > > > diff --git a/include/common.h b/include/common.h > > > > index a292d919390..e946e4d9b7f 100644 > > > > --- a/include/common.h > > > > +++ b/include/common.h > > > > @@ -145,7 +145,6 @@ void relocate_code(ulong); > > > > void relocate_code(ulong, gd_t *, ulong) __attribute__ ((noreturn)); > > > > #endif > > > > ulong get_endaddr (void); > > > > -void trap_init (ulong); > > > > > > > > void s_init(void); > > > > > > > > diff --git a/include/irq_legacy.h b/include/irq_legacy.h > > > > index 91b523d043a..a029c54b702 100644 > > > > --- a/include/irq_legacy.h > > > > +++ b/include/irq_legacy.h > > > > @@ -21,4 +21,6 @@ void reset_timer(void); > > > > void enable_interrupts(void); > > > > int disable_interrupts(void); > > > > > > > > +void trap_init(unsigned long reloc_addr); > > > > + > > > > #endif > > > > -- > > > > > > hm, at least on MIPS this is regular CPU exception handling and has > > > nothing to do with interrupts nor with DM. I don't know about M68 but > > > there it looks like traps are used for exceptions and interrupts. I > > > think it would better fit into a include/traps.h or so. > > > > I think we tend to think of interrupts and exceptions as being similar. > > > > Are you suggesting creating a new traps_legacy.h? > > > > either traps.h without _legacy because this code is not legacy at all. > As far as I understand from your patch series, you're mainly using > _legacy to separate non-DM code? Yes that's right. It makes it clear what needs changing. It would be worth thinking about how to include traps_init() into a driver somewhere. > > Another option would be init.h because board_init_r.c:initr_trap() is > the only caller. That sounds good. Regards, SImon
diff --git a/arch/m68k/lib/traps.c b/arch/m68k/lib/traps.c index 5d802077453..b2e7e9fe407 100644 --- a/arch/m68k/lib/traps.c +++ b/arch/m68k/lib/traps.c @@ -8,6 +8,7 @@ */ #include <common.h> +#include <irq_legacy.h> #include <watchdog.h> #include <command.h> #include <asm/processor.h> diff --git a/arch/mips/lib/traps.c b/arch/mips/lib/traps.c index ff02d437a87..d173a1b0134 100644 --- a/arch/mips/lib/traps.c +++ b/arch/mips/lib/traps.c @@ -12,6 +12,7 @@ #include <common.h> #include <cpu_legacy.h> +#include <irq_legacy.h> #include <asm/mipsregs.h> #include <asm/addrspace.h> #include <asm/system.h> diff --git a/include/common.h b/include/common.h index a292d919390..e946e4d9b7f 100644 --- a/include/common.h +++ b/include/common.h @@ -145,7 +145,6 @@ void relocate_code(ulong); void relocate_code(ulong, gd_t *, ulong) __attribute__ ((noreturn)); #endif ulong get_endaddr (void); -void trap_init (ulong); void s_init(void); diff --git a/include/irq_legacy.h b/include/irq_legacy.h index 91b523d043a..a029c54b702 100644 --- a/include/irq_legacy.h +++ b/include/irq_legacy.h @@ -21,4 +21,6 @@ void reset_timer(void); void enable_interrupts(void); int disable_interrupts(void); +void trap_init(unsigned long reloc_addr); + #endif
Move this function into the irq_legacy.h header file. Signed-off-by: Simon Glass <sjg@chromium.org> --- arch/m68k/lib/traps.c | 1 + arch/mips/lib/traps.c | 1 + include/common.h | 1 - include/irq_legacy.h | 2 ++ 4 files changed, 4 insertions(+), 1 deletion(-)