Re: [U-Boot] [PATCH 39/41] common: Move trap_init() out of common.h

2019-09-30 Thread Simon Glass
Hi Daniel,

On Mon, 30 Sep 2019 at 09:43, Daniel Schwierzeck
 wrote:
>
> On Mon, Sep 30, 2019 at 5:37 PM Simon Glass  wrote:
> >
> > Hi Daniel,
> >
> > On Mon, 30 Sep 2019 at 09:13, Daniel Schwierzeck
> >  wrote:
> > >
> > > On Mon, Sep 30, 2019 at 4:58 PM Simon Glass  wrote:
> > > >
> > > > Move this function into the irq_legacy.h header file.
> > > >
> > > > Signed-off-by: Simon Glass 
> > > > ---
> > > >
> > > >  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 
> > > > +#include 
> > > >  #include 
> > > >  #include 
> > > >  #include 
> > > > 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 
> > > >  #include 
> > > > +#include 
> > > >  #include 
> > > >  #include 
> > > >  #include 
> > > > 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 @@ voidrelocate_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
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH 39/41] common: Move trap_init() out of common.h

2019-09-30 Thread Tom Rini
On Mon, Sep 30, 2019 at 05:43:29PM +0200, Daniel Schwierzeck wrote:
> On Mon, Sep 30, 2019 at 5:37 PM Simon Glass  wrote:
> >
> > Hi Daniel,
> >
> > On Mon, 30 Sep 2019 at 09:13, Daniel Schwierzeck
> >  wrote:
> > >
> > > On Mon, Sep 30, 2019 at 4:58 PM Simon Glass  wrote:
> > > >
> > > > Move this function into the irq_legacy.h header file.
> > > >
> > > > Signed-off-by: Simon Glass 
> > > > ---
> > > >
> > > >  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 
> > > > +#include 
> > > >  #include 
> > > >  #include 
> > > >  #include 
> > > > 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 
> > > >  #include 
> > > > +#include 
> > > >  #include 
> > > >  #include 
> > > >  #include 
> > > > 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 @@ voidrelocate_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!

-- 
Tom
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH 39/41] common: Move trap_init() out of common.h

2019-09-30 Thread Daniel Schwierzeck
On Mon, Sep 30, 2019 at 5:37 PM Simon Glass  wrote:
>
> Hi Daniel,
>
> On Mon, 30 Sep 2019 at 09:13, Daniel Schwierzeck
>  wrote:
> >
> > On Mon, Sep 30, 2019 at 4:58 PM Simon Glass  wrote:
> > >
> > > Move this function into the irq_legacy.h header file.
> > >
> > > Signed-off-by: Simon Glass 
> > > ---
> > >
> > >  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 
> > > +#include 
> > >  #include 
> > >  #include 
> > >  #include 
> > > 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 
> > >  #include 
> > > +#include 
> > >  #include 
> > >  #include 
> > >  #include 
> > > 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 @@ voidrelocate_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.

-- 
- Daniel
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH 39/41] common: Move trap_init() out of common.h

2019-09-30 Thread Simon Glass
Hi Daniel,

On Mon, 30 Sep 2019 at 09:13, Daniel Schwierzeck
 wrote:
>
> On Mon, Sep 30, 2019 at 4:58 PM Simon Glass  wrote:
> >
> > Move this function into the irq_legacy.h header file.
> >
> > Signed-off-by: Simon Glass 
> > ---
> >
> >  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 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > 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 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > 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 @@ voidrelocate_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
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH 39/41] common: Move trap_init() out of common.h

2019-09-30 Thread Daniel Schwierzeck
On Mon, Sep 30, 2019 at 4:58 PM Simon Glass  wrote:
>
> Move this function into the irq_legacy.h header file.
>
> Signed-off-by: Simon Glass 
> ---
>
>  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 
> +#include 
>  #include 
>  #include 
>  #include 
> 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 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> 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 @@ voidrelocate_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.


-- 
- Daniel
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot