Re: [U-Boot] [PATCH] vexpress64: compile Juno PCIe conditionally

2015-11-17 Thread Linus Walleij 
On Mon, Nov 16, 2015 at 7:16 PM, Ryan Harkin  wrote:

> Thanks Tom.  I updated it to this:
>
> /* This function gets replaced by platforms supporting PCIe.
>  * The replacement function, eg. on Juno, initialises the PCIe bus.
>  */
> __weak void vexpress64_pcie_init(void)
> {
> }
>
> ... and checkpatch is happy.
>
> I'll await Linus' preference on how I submit the patch before sending
> out a new version.  I'll put it in a v2 series with the other two
> patches I have pending for NOR support so they are all kept together
> in order.

Bah I can live with it but will never like it :)
Acked-by: Linus Walleij 

Yours,
Linus Walleij
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] vexpress64: compile Juno PCIe conditionally

2015-11-16 Thread Tom Rini 
On Mon, Nov 16, 2015 at 09:46:55AM +, Ryan Harkin wrote:
> On 13 November 2015 at 13:39, Ryan Harkin  wrote:
> > [trying again with Linus on the to: list]
> >
> > Hi Linus,
> >
> > Tom asked for a small change to your patch.  Would mind having a look
> > and re-working?
> >
> > Thanks,
> > Ryan.
> >
> > On 21 October 2015 at 14:16, Ryan Harkin  wrote:
> >> On 21 October 2015 at 13:50, Tom Rini  wrote:
> >>> On Wed, Oct 21, 2015 at 12:00:03PM +0100, Ryan Harkin wrote:
>  On 20 October 2015 at 16:38, Tom Rini  wrote:
> 
>  > On Tue, Oct 20, 2015 at 08:05:40AM +0200, Linus Walleij wrote:
>  >
>  > > Only compile in PCIe support if the board really uses it. Provide
>  > > a stub for the init function if e.g. FVP is being built.
>  > >
>  > > Cc: Liviu Dudau 
>  > > Cc: Ryan Harkin 
>  > > Signed-off-by: Linus Walleij 
>  > > ---
>  > >  board/armltd/vexpress64/Makefile | 3 ++-
>  > >  board/armltd/vexpress64/pcie.c   | 2 --
>  > >  board/armltd/vexpress64/pcie.h   | 4 
>  > >  3 files changed, 6 insertions(+), 3 deletions(-)
>  > >
>  > > diff --git a/board/armltd/vexpress64/Makefile
>  > b/board/armltd/vexpress64/Makefile
>  > > index a35db401b684..b4391a71249a 100644
>  > > --- a/board/armltd/vexpress64/Makefile
>  > > +++ b/board/armltd/vexpress64/Makefile
>  > > @@ -5,4 +5,5 @@
>  > >  # SPDX-License-Identifier:   GPL-2.0+
>  > >  #
>  > >
>  > > -obj-y:= vexpress64.o pcie.o
>  > > +obj-y:= vexpress64.o
>  > > +obj-$(CONFIG_TARGET_VEXPRESS64_JUNO) += pcie.o
>  > > diff --git a/board/armltd/vexpress64/pcie.c
>  > b/board/armltd/vexpress64/pcie.c
>  > > index 7b999e8ef40b..311c4500e3ff 100644
>  > > --- a/board/armltd/vexpress64/pcie.c
>  > > +++ b/board/armltd/vexpress64/pcie.c
>  > > @@ -191,7 +191,5 @@ void xr3pci_init(void)
>  > >
>  > >  void vexpress64_pcie_init(void)
>  > >  {
>  > > -#ifdef CONFIG_TARGET_VEXPRESS64_JUNO
>  > >   xr3pci_init();
>  > > -#endif
>  > >  }
>  > > diff --git a/board/armltd/vexpress64/pcie.h
>  > b/board/armltd/vexpress64/pcie.h
>  > > index 14642f4f5c43..55b276d6af11 100644
>  > > --- a/board/armltd/vexpress64/pcie.h
>  > > +++ b/board/armltd/vexpress64/pcie.h
>  > > @@ -1,6 +1,10 @@
>  > >  #ifndef __VEXPRESS64_PCIE_H__
>  > >  #define __VEXPRESS64_PCIE_H__
>  > >
>  > > +#ifdef CONFIG_TARGET_VEXPRESS64_JUNO
>  > >  void vexpress64_pcie_init(void);
>  > > +#else
>  > > +static inline void vexpress64_pcie_init(void) {}
>  > > +#endif
>  > >
>  > >  #endif /* __VEXPRESS64_PCIE_H__ */
>  >
>  > Alright, so the versatile platform makes life fun at times.
> 
> 
>  This comes back to the refactoring thread we had recently...
> 
> 
> 
>  >   First, my
>  > preference is for weak functions (and I _think_ the linker ends up 
>  > being
>  > smart enough about them to optimize things away, if not, arrg).  
>  > Second,
>  > the question I have is, is this xr3pci_init() bit really a Juno thing 
>  > or
>  > a baseboard thing (I assume no)
> 
> 
>  It's sort-of the same thing on Juno.  Juno has a baseboard and SoC in one
>  build, unlike the previous 32-bit Versatile Express motherboard that 
>  takes
>  core tiles with different cores on it.
> 
>  Juno R0 has the PCIe controller, but it doesn't work.  So the code is 
>  safe
>  to run at this level.  Juno R1 has the controller and it works.
> 
>  or a going to be the same on the next
>  > Cortex-Asomething module thing?
> 
> 
>  Juno R2 will have the PCIe controller too and it should hopefully work.
> >>>
> >>> But it will also be the A52.  Or no, the A72?  Or can't say..
> >>>
> >>
> >> If I knew the answer, "can't say" would probably be the official line.
> >> But I haven't been told of any plans to change the cores, so I am
> >> assuming the next Juno respin will be A57/A53 big.LITTLE.  Just like
> >> we have now but with fixes.
> >>
> >>
>  But this controller is not Cortex-Asomething related.  Another vendor 
>  could
>  put the same IP block on their board, of course.
> >>>
> >>> Right, but it wouldn't be under board/armltd/vexpress64/ either..
> >>>
>  FVP models don't have it and other ARM platforms won't necessarily have 
>  it.
> 
>  Does that help?!
> >>>
> >>> Yes, thanks.  I think it's just a style thing then.  We don't do a lot
> >>> of static inline nop functions, we do __weak functions in the main file
> >>> (and comment about what it should be doing in a real function) and then
> >>> provide the strong version in another file.  So just the pcie.h

Re: [U-Boot] [PATCH] vexpress64: compile Juno PCIe conditionally

2015-11-16 Thread Ryan Harkin 
On 16 November 2015 at 18:03, Tom Rini  wrote:
> On Mon, Nov 16, 2015 at 09:46:55AM +, Ryan Harkin wrote:
>> On 13 November 2015 at 13:39, Ryan Harkin  wrote:
>> > [trying again with Linus on the to: list]
>> >
>> > Hi Linus,
>> >
>> > Tom asked for a small change to your patch.  Would mind having a look
>> > and re-working?
>> >
>> > Thanks,
>> > Ryan.
>> >
>> > On 21 October 2015 at 14:16, Ryan Harkin  wrote:
>> >> On 21 October 2015 at 13:50, Tom Rini  wrote:
>> >>> On Wed, Oct 21, 2015 at 12:00:03PM +0100, Ryan Harkin wrote:
>>  On 20 October 2015 at 16:38, Tom Rini  wrote:
>> 
>>  > On Tue, Oct 20, 2015 at 08:05:40AM +0200, Linus Walleij wrote:
>>  >
>>  > > Only compile in PCIe support if the board really uses it. Provide
>>  > > a stub for the init function if e.g. FVP is being built.
>>  > >
>>  > > Cc: Liviu Dudau 
>>  > > Cc: Ryan Harkin 
>>  > > Signed-off-by: Linus Walleij 
>>  > > ---
>>  > >  board/armltd/vexpress64/Makefile | 3 ++-
>>  > >  board/armltd/vexpress64/pcie.c   | 2 --
>>  > >  board/armltd/vexpress64/pcie.h   | 4 
>>  > >  3 files changed, 6 insertions(+), 3 deletions(-)
>>  > >
>>  > > diff --git a/board/armltd/vexpress64/Makefile
>>  > b/board/armltd/vexpress64/Makefile
>>  > > index a35db401b684..b4391a71249a 100644
>>  > > --- a/board/armltd/vexpress64/Makefile
>>  > > +++ b/board/armltd/vexpress64/Makefile
>>  > > @@ -5,4 +5,5 @@
>>  > >  # SPDX-License-Identifier:   GPL-2.0+
>>  > >  #
>>  > >
>>  > > -obj-y:= vexpress64.o pcie.o
>>  > > +obj-y:= vexpress64.o
>>  > > +obj-$(CONFIG_TARGET_VEXPRESS64_JUNO) += pcie.o
>>  > > diff --git a/board/armltd/vexpress64/pcie.c
>>  > b/board/armltd/vexpress64/pcie.c
>>  > > index 7b999e8ef40b..311c4500e3ff 100644
>>  > > --- a/board/armltd/vexpress64/pcie.c
>>  > > +++ b/board/armltd/vexpress64/pcie.c
>>  > > @@ -191,7 +191,5 @@ void xr3pci_init(void)
>>  > >
>>  > >  void vexpress64_pcie_init(void)
>>  > >  {
>>  > > -#ifdef CONFIG_TARGET_VEXPRESS64_JUNO
>>  > >   xr3pci_init();
>>  > > -#endif
>>  > >  }
>>  > > diff --git a/board/armltd/vexpress64/pcie.h
>>  > b/board/armltd/vexpress64/pcie.h
>>  > > index 14642f4f5c43..55b276d6af11 100644
>>  > > --- a/board/armltd/vexpress64/pcie.h
>>  > > +++ b/board/armltd/vexpress64/pcie.h
>>  > > @@ -1,6 +1,10 @@
>>  > >  #ifndef __VEXPRESS64_PCIE_H__
>>  > >  #define __VEXPRESS64_PCIE_H__
>>  > >
>>  > > +#ifdef CONFIG_TARGET_VEXPRESS64_JUNO
>>  > >  void vexpress64_pcie_init(void);
>>  > > +#else
>>  > > +static inline void vexpress64_pcie_init(void) {}
>>  > > +#endif
>>  > >
>>  > >  #endif /* __VEXPRESS64_PCIE_H__ */
>>  >
>>  > Alright, so the versatile platform makes life fun at times.
>> 
>> 
>>  This comes back to the refactoring thread we had recently...
>> 
>> 
>> 
>>  >   First, my
>>  > preference is for weak functions (and I _think_ the linker ends up 
>>  > being
>>  > smart enough about them to optimize things away, if not, arrg).  
>>  > Second,
>>  > the question I have is, is this xr3pci_init() bit really a Juno thing 
>>  > or
>>  > a baseboard thing (I assume no)
>> 
>> 
>>  It's sort-of the same thing on Juno.  Juno has a baseboard and SoC in 
>>  one
>>  build, unlike the previous 32-bit Versatile Express motherboard that 
>>  takes
>>  core tiles with different cores on it.
>> 
>>  Juno R0 has the PCIe controller, but it doesn't work.  So the code is 
>>  safe
>>  to run at this level.  Juno R1 has the controller and it works.
>> 
>>  or a going to be the same on the next
>>  > Cortex-Asomething module thing?
>> 
>> 
>>  Juno R2 will have the PCIe controller too and it should hopefully work.
>> >>>
>> >>> But it will also be the A52.  Or no, the A72?  Or can't say..
>> >>>
>> >>
>> >> If I knew the answer, "can't say" would probably be the official line.
>> >> But I haven't been told of any plans to change the cores, so I am
>> >> assuming the next Juno respin will be A57/A53 big.LITTLE.  Just like
>> >> we have now but with fixes.
>> >>
>> >>
>>  But this controller is not Cortex-Asomething related.  Another vendor 
>>  could
>>  put the same IP block on their board, of course.
>> >>>
>> >>> Right, but it wouldn't be under board/armltd/vexpress64/ either..
>> >>>
>>  FVP models don't have it and other ARM platforms won't necessarily have 
>>  it.
>> 
>>  Does that help?!
>> >>>
>> >>> Yes, thanks.  I think it's just a style thing then.  We don't do a lot
>> >>> of static inline

Re: [U-Boot] [PATCH] vexpress64: compile Juno PCIe conditionally

2015-11-16 Thread Ryan Harkin 
On 13 November 2015 at 13:39, Ryan Harkin  wrote:
> [trying again with Linus on the to: list]
>
> Hi Linus,
>
> Tom asked for a small change to your patch.  Would mind having a look
> and re-working?
>
> Thanks,
> Ryan.
>
> On 21 October 2015 at 14:16, Ryan Harkin  wrote:
>> On 21 October 2015 at 13:50, Tom Rini  wrote:
>>> On Wed, Oct 21, 2015 at 12:00:03PM +0100, Ryan Harkin wrote:
 On 20 October 2015 at 16:38, Tom Rini  wrote:

 > On Tue, Oct 20, 2015 at 08:05:40AM +0200, Linus Walleij wrote:
 >
 > > Only compile in PCIe support if the board really uses it. Provide
 > > a stub for the init function if e.g. FVP is being built.
 > >
 > > Cc: Liviu Dudau 
 > > Cc: Ryan Harkin 
 > > Signed-off-by: Linus Walleij 
 > > ---
 > >  board/armltd/vexpress64/Makefile | 3 ++-
 > >  board/armltd/vexpress64/pcie.c   | 2 --
 > >  board/armltd/vexpress64/pcie.h   | 4 
 > >  3 files changed, 6 insertions(+), 3 deletions(-)
 > >
 > > diff --git a/board/armltd/vexpress64/Makefile
 > b/board/armltd/vexpress64/Makefile
 > > index a35db401b684..b4391a71249a 100644
 > > --- a/board/armltd/vexpress64/Makefile
 > > +++ b/board/armltd/vexpress64/Makefile
 > > @@ -5,4 +5,5 @@
 > >  # SPDX-License-Identifier:   GPL-2.0+
 > >  #
 > >
 > > -obj-y:= vexpress64.o pcie.o
 > > +obj-y:= vexpress64.o
 > > +obj-$(CONFIG_TARGET_VEXPRESS64_JUNO) += pcie.o
 > > diff --git a/board/armltd/vexpress64/pcie.c
 > b/board/armltd/vexpress64/pcie.c
 > > index 7b999e8ef40b..311c4500e3ff 100644
 > > --- a/board/armltd/vexpress64/pcie.c
 > > +++ b/board/armltd/vexpress64/pcie.c
 > > @@ -191,7 +191,5 @@ void xr3pci_init(void)
 > >
 > >  void vexpress64_pcie_init(void)
 > >  {
 > > -#ifdef CONFIG_TARGET_VEXPRESS64_JUNO
 > >   xr3pci_init();
 > > -#endif
 > >  }
 > > diff --git a/board/armltd/vexpress64/pcie.h
 > b/board/armltd/vexpress64/pcie.h
 > > index 14642f4f5c43..55b276d6af11 100644
 > > --- a/board/armltd/vexpress64/pcie.h
 > > +++ b/board/armltd/vexpress64/pcie.h
 > > @@ -1,6 +1,10 @@
 > >  #ifndef __VEXPRESS64_PCIE_H__
 > >  #define __VEXPRESS64_PCIE_H__
 > >
 > > +#ifdef CONFIG_TARGET_VEXPRESS64_JUNO
 > >  void vexpress64_pcie_init(void);
 > > +#else
 > > +static inline void vexpress64_pcie_init(void) {}
 > > +#endif
 > >
 > >  #endif /* __VEXPRESS64_PCIE_H__ */
 >
 > Alright, so the versatile platform makes life fun at times.


 This comes back to the refactoring thread we had recently...



 >   First, my
 > preference is for weak functions (and I _think_ the linker ends up being
 > smart enough about them to optimize things away, if not, arrg).  Second,
 > the question I have is, is this xr3pci_init() bit really a Juno thing or
 > a baseboard thing (I assume no)


 It's sort-of the same thing on Juno.  Juno has a baseboard and SoC in one
 build, unlike the previous 32-bit Versatile Express motherboard that takes
 core tiles with different cores on it.

 Juno R0 has the PCIe controller, but it doesn't work.  So the code is safe
 to run at this level.  Juno R1 has the controller and it works.

 or a going to be the same on the next
 > Cortex-Asomething module thing?


 Juno R2 will have the PCIe controller too and it should hopefully work.
>>>
>>> But it will also be the A52.  Or no, the A72?  Or can't say..
>>>
>>
>> If I knew the answer, "can't say" would probably be the official line.
>> But I haven't been told of any plans to change the cores, so I am
>> assuming the next Juno respin will be A57/A53 big.LITTLE.  Just like
>> we have now but with fixes.
>>
>>
 But this controller is not Cortex-Asomething related.  Another vendor could
 put the same IP block on their board, of course.
>>>
>>> Right, but it wouldn't be under board/armltd/vexpress64/ either..
>>>
 FVP models don't have it and other ARM platforms won't necessarily have it.

 Does that help?!
>>>
>>> Yes, thanks.  I think it's just a style thing then.  We don't do a lot
>>> of static inline nop functions, we do __weak functions in the main file
>>> (and comment about what it should be doing in a real function) and then
>>> provide the strong version in another file.  So just the pcie.h part
>>> needs changing then.
>>

I'm not familiar with how __weak functions work, but reading Tom's
advice and grepping the code leads me to the diff below.  Tom, is this
what you were looking for?

diff --git a/board/armltd/vexpress64/Makefile b/board/armltd/vexpress64/Makefile
index a35db40..b4391a7 100644
--- a/board/armltd/vexpress64/Makefile
+++

Re: [U-Boot] [PATCH] vexpress64: compile Juno PCIe conditionally

2015-11-13 Thread Ryan Harkin 
Hi Linus,

Tom asked for a small change to your patch.  Would mind having a look
and re-working?

Thanks,
Ryan.

On 21 October 2015 at 14:16, Ryan Harkin  wrote:
> On 21 October 2015 at 13:50, Tom Rini  wrote:
>> On Wed, Oct 21, 2015 at 12:00:03PM +0100, Ryan Harkin wrote:
>>> On 20 October 2015 at 16:38, Tom Rini  wrote:
>>>
>>> > On Tue, Oct 20, 2015 at 08:05:40AM +0200, Linus Walleij wrote:
>>> >
>>> > > Only compile in PCIe support if the board really uses it. Provide
>>> > > a stub for the init function if e.g. FVP is being built.
>>> > >
>>> > > Cc: Liviu Dudau 
>>> > > Cc: Ryan Harkin 
>>> > > Signed-off-by: Linus Walleij 
>>> > > ---
>>> > >  board/armltd/vexpress64/Makefile | 3 ++-
>>> > >  board/armltd/vexpress64/pcie.c   | 2 --
>>> > >  board/armltd/vexpress64/pcie.h   | 4 
>>> > >  3 files changed, 6 insertions(+), 3 deletions(-)
>>> > >
>>> > > diff --git a/board/armltd/vexpress64/Makefile
>>> > b/board/armltd/vexpress64/Makefile
>>> > > index a35db401b684..b4391a71249a 100644
>>> > > --- a/board/armltd/vexpress64/Makefile
>>> > > +++ b/board/armltd/vexpress64/Makefile
>>> > > @@ -5,4 +5,5 @@
>>> > >  # SPDX-License-Identifier:   GPL-2.0+
>>> > >  #
>>> > >
>>> > > -obj-y:= vexpress64.o pcie.o
>>> > > +obj-y:= vexpress64.o
>>> > > +obj-$(CONFIG_TARGET_VEXPRESS64_JUNO) += pcie.o
>>> > > diff --git a/board/armltd/vexpress64/pcie.c
>>> > b/board/armltd/vexpress64/pcie.c
>>> > > index 7b999e8ef40b..311c4500e3ff 100644
>>> > > --- a/board/armltd/vexpress64/pcie.c
>>> > > +++ b/board/armltd/vexpress64/pcie.c
>>> > > @@ -191,7 +191,5 @@ void xr3pci_init(void)
>>> > >
>>> > >  void vexpress64_pcie_init(void)
>>> > >  {
>>> > > -#ifdef CONFIG_TARGET_VEXPRESS64_JUNO
>>> > >   xr3pci_init();
>>> > > -#endif
>>> > >  }
>>> > > diff --git a/board/armltd/vexpress64/pcie.h
>>> > b/board/armltd/vexpress64/pcie.h
>>> > > index 14642f4f5c43..55b276d6af11 100644
>>> > > --- a/board/armltd/vexpress64/pcie.h
>>> > > +++ b/board/armltd/vexpress64/pcie.h
>>> > > @@ -1,6 +1,10 @@
>>> > >  #ifndef __VEXPRESS64_PCIE_H__
>>> > >  #define __VEXPRESS64_PCIE_H__
>>> > >
>>> > > +#ifdef CONFIG_TARGET_VEXPRESS64_JUNO
>>> > >  void vexpress64_pcie_init(void);
>>> > > +#else
>>> > > +static inline void vexpress64_pcie_init(void) {}
>>> > > +#endif
>>> > >
>>> > >  #endif /* __VEXPRESS64_PCIE_H__ */
>>> >
>>> > Alright, so the versatile platform makes life fun at times.
>>>
>>>
>>> This comes back to the refactoring thread we had recently...
>>>
>>>
>>>
>>> >   First, my
>>> > preference is for weak functions (and I _think_ the linker ends up being
>>> > smart enough about them to optimize things away, if not, arrg).  Second,
>>> > the question I have is, is this xr3pci_init() bit really a Juno thing or
>>> > a baseboard thing (I assume no)
>>>
>>>
>>> It's sort-of the same thing on Juno.  Juno has a baseboard and SoC in one
>>> build, unlike the previous 32-bit Versatile Express motherboard that takes
>>> core tiles with different cores on it.
>>>
>>> Juno R0 has the PCIe controller, but it doesn't work.  So the code is safe
>>> to run at this level.  Juno R1 has the controller and it works.
>>>
>>> or a going to be the same on the next
>>> > Cortex-Asomething module thing?
>>>
>>>
>>> Juno R2 will have the PCIe controller too and it should hopefully work.
>>
>> But it will also be the A52.  Or no, the A72?  Or can't say..
>>
>
> If I knew the answer, "can't say" would probably be the official line.
> But I haven't been told of any plans to change the cores, so I am
> assuming the next Juno respin will be A57/A53 big.LITTLE.  Just like
> we have now but with fixes.
>
>
>>> But this controller is not Cortex-Asomething related.  Another vendor could
>>> put the same IP block on their board, of course.
>>
>> Right, but it wouldn't be under board/armltd/vexpress64/ either..
>>
>>> FVP models don't have it and other ARM platforms won't necessarily have it.
>>>
>>> Does that help?!
>>
>> Yes, thanks.  I think it's just a style thing then.  We don't do a lot
>> of static inline nop functions, we do __weak functions in the main file
>> (and comment about what it should be doing in a real function) and then
>> provide the strong version in another file.  So just the pcie.h part
>> needs changing then.
>
> Then it's over to Linus...
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] vexpress64: compile Juno PCIe conditionally

2015-11-13 Thread Ryan Harkin 
[trying again with Linus on the to: list]

Hi Linus,

Tom asked for a small change to your patch.  Would mind having a look
and re-working?

Thanks,
Ryan.

On 21 October 2015 at 14:16, Ryan Harkin  wrote:
> On 21 October 2015 at 13:50, Tom Rini  wrote:
>> On Wed, Oct 21, 2015 at 12:00:03PM +0100, Ryan Harkin wrote:
>>> On 20 October 2015 at 16:38, Tom Rini  wrote:
>>>
>>> > On Tue, Oct 20, 2015 at 08:05:40AM +0200, Linus Walleij wrote:
>>> >
>>> > > Only compile in PCIe support if the board really uses it. Provide
>>> > > a stub for the init function if e.g. FVP is being built.
>>> > >
>>> > > Cc: Liviu Dudau 
>>> > > Cc: Ryan Harkin 
>>> > > Signed-off-by: Linus Walleij 
>>> > > ---
>>> > >  board/armltd/vexpress64/Makefile | 3 ++-
>>> > >  board/armltd/vexpress64/pcie.c   | 2 --
>>> > >  board/armltd/vexpress64/pcie.h   | 4 
>>> > >  3 files changed, 6 insertions(+), 3 deletions(-)
>>> > >
>>> > > diff --git a/board/armltd/vexpress64/Makefile
>>> > b/board/armltd/vexpress64/Makefile
>>> > > index a35db401b684..b4391a71249a 100644
>>> > > --- a/board/armltd/vexpress64/Makefile
>>> > > +++ b/board/armltd/vexpress64/Makefile
>>> > > @@ -5,4 +5,5 @@
>>> > >  # SPDX-License-Identifier:   GPL-2.0+
>>> > >  #
>>> > >
>>> > > -obj-y:= vexpress64.o pcie.o
>>> > > +obj-y:= vexpress64.o
>>> > > +obj-$(CONFIG_TARGET_VEXPRESS64_JUNO) += pcie.o
>>> > > diff --git a/board/armltd/vexpress64/pcie.c
>>> > b/board/armltd/vexpress64/pcie.c
>>> > > index 7b999e8ef40b..311c4500e3ff 100644
>>> > > --- a/board/armltd/vexpress64/pcie.c
>>> > > +++ b/board/armltd/vexpress64/pcie.c
>>> > > @@ -191,7 +191,5 @@ void xr3pci_init(void)
>>> > >
>>> > >  void vexpress64_pcie_init(void)
>>> > >  {
>>> > > -#ifdef CONFIG_TARGET_VEXPRESS64_JUNO
>>> > >   xr3pci_init();
>>> > > -#endif
>>> > >  }
>>> > > diff --git a/board/armltd/vexpress64/pcie.h
>>> > b/board/armltd/vexpress64/pcie.h
>>> > > index 14642f4f5c43..55b276d6af11 100644
>>> > > --- a/board/armltd/vexpress64/pcie.h
>>> > > +++ b/board/armltd/vexpress64/pcie.h
>>> > > @@ -1,6 +1,10 @@
>>> > >  #ifndef __VEXPRESS64_PCIE_H__
>>> > >  #define __VEXPRESS64_PCIE_H__
>>> > >
>>> > > +#ifdef CONFIG_TARGET_VEXPRESS64_JUNO
>>> > >  void vexpress64_pcie_init(void);
>>> > > +#else
>>> > > +static inline void vexpress64_pcie_init(void) {}
>>> > > +#endif
>>> > >
>>> > >  #endif /* __VEXPRESS64_PCIE_H__ */
>>> >
>>> > Alright, so the versatile platform makes life fun at times.
>>>
>>>
>>> This comes back to the refactoring thread we had recently...
>>>
>>>
>>>
>>> >   First, my
>>> > preference is for weak functions (and I _think_ the linker ends up being
>>> > smart enough about them to optimize things away, if not, arrg).  Second,
>>> > the question I have is, is this xr3pci_init() bit really a Juno thing or
>>> > a baseboard thing (I assume no)
>>>
>>>
>>> It's sort-of the same thing on Juno.  Juno has a baseboard and SoC in one
>>> build, unlike the previous 32-bit Versatile Express motherboard that takes
>>> core tiles with different cores on it.
>>>
>>> Juno R0 has the PCIe controller, but it doesn't work.  So the code is safe
>>> to run at this level.  Juno R1 has the controller and it works.
>>>
>>> or a going to be the same on the next
>>> > Cortex-Asomething module thing?
>>>
>>>
>>> Juno R2 will have the PCIe controller too and it should hopefully work.
>>
>> But it will also be the A52.  Or no, the A72?  Or can't say..
>>
>
> If I knew the answer, "can't say" would probably be the official line.
> But I haven't been told of any plans to change the cores, so I am
> assuming the next Juno respin will be A57/A53 big.LITTLE.  Just like
> we have now but with fixes.
>
>
>>> But this controller is not Cortex-Asomething related.  Another vendor could
>>> put the same IP block on their board, of course.
>>
>> Right, but it wouldn't be under board/armltd/vexpress64/ either..
>>
>>> FVP models don't have it and other ARM platforms won't necessarily have it.
>>>
>>> Does that help?!
>>
>> Yes, thanks.  I think it's just a style thing then.  We don't do a lot
>> of static inline nop functions, we do __weak functions in the main file
>> (and comment about what it should be doing in a real function) and then
>> provide the strong version in another file.  So just the pcie.h part
>> needs changing then.
>
> Then it's over to Linus...
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] vexpress64: compile Juno PCIe conditionally

2015-10-21 Thread Ryan Harkin 
On 21 October 2015 at 13:50, Tom Rini  wrote:
> On Wed, Oct 21, 2015 at 12:00:03PM +0100, Ryan Harkin wrote:
>> On 20 October 2015 at 16:38, Tom Rini  wrote:
>>
>> > On Tue, Oct 20, 2015 at 08:05:40AM +0200, Linus Walleij wrote:
>> >
>> > > Only compile in PCIe support if the board really uses it. Provide
>> > > a stub for the init function if e.g. FVP is being built.
>> > >
>> > > Cc: Liviu Dudau 
>> > > Cc: Ryan Harkin 
>> > > Signed-off-by: Linus Walleij 
>> > > ---
>> > >  board/armltd/vexpress64/Makefile | 3 ++-
>> > >  board/armltd/vexpress64/pcie.c   | 2 --
>> > >  board/armltd/vexpress64/pcie.h   | 4 
>> > >  3 files changed, 6 insertions(+), 3 deletions(-)
>> > >
>> > > diff --git a/board/armltd/vexpress64/Makefile
>> > b/board/armltd/vexpress64/Makefile
>> > > index a35db401b684..b4391a71249a 100644
>> > > --- a/board/armltd/vexpress64/Makefile
>> > > +++ b/board/armltd/vexpress64/Makefile
>> > > @@ -5,4 +5,5 @@
>> > >  # SPDX-License-Identifier:   GPL-2.0+
>> > >  #
>> > >
>> > > -obj-y:= vexpress64.o pcie.o
>> > > +obj-y:= vexpress64.o
>> > > +obj-$(CONFIG_TARGET_VEXPRESS64_JUNO) += pcie.o
>> > > diff --git a/board/armltd/vexpress64/pcie.c
>> > b/board/armltd/vexpress64/pcie.c
>> > > index 7b999e8ef40b..311c4500e3ff 100644
>> > > --- a/board/armltd/vexpress64/pcie.c
>> > > +++ b/board/armltd/vexpress64/pcie.c
>> > > @@ -191,7 +191,5 @@ void xr3pci_init(void)
>> > >
>> > >  void vexpress64_pcie_init(void)
>> > >  {
>> > > -#ifdef CONFIG_TARGET_VEXPRESS64_JUNO
>> > >   xr3pci_init();
>> > > -#endif
>> > >  }
>> > > diff --git a/board/armltd/vexpress64/pcie.h
>> > b/board/armltd/vexpress64/pcie.h
>> > > index 14642f4f5c43..55b276d6af11 100644
>> > > --- a/board/armltd/vexpress64/pcie.h
>> > > +++ b/board/armltd/vexpress64/pcie.h
>> > > @@ -1,6 +1,10 @@
>> > >  #ifndef __VEXPRESS64_PCIE_H__
>> > >  #define __VEXPRESS64_PCIE_H__
>> > >
>> > > +#ifdef CONFIG_TARGET_VEXPRESS64_JUNO
>> > >  void vexpress64_pcie_init(void);
>> > > +#else
>> > > +static inline void vexpress64_pcie_init(void) {}
>> > > +#endif
>> > >
>> > >  #endif /* __VEXPRESS64_PCIE_H__ */
>> >
>> > Alright, so the versatile platform makes life fun at times.
>>
>>
>> This comes back to the refactoring thread we had recently...
>>
>>
>>
>> >   First, my
>> > preference is for weak functions (and I _think_ the linker ends up being
>> > smart enough about them to optimize things away, if not, arrg).  Second,
>> > the question I have is, is this xr3pci_init() bit really a Juno thing or
>> > a baseboard thing (I assume no)
>>
>>
>> It's sort-of the same thing on Juno.  Juno has a baseboard and SoC in one
>> build, unlike the previous 32-bit Versatile Express motherboard that takes
>> core tiles with different cores on it.
>>
>> Juno R0 has the PCIe controller, but it doesn't work.  So the code is safe
>> to run at this level.  Juno R1 has the controller and it works.
>>
>> or a going to be the same on the next
>> > Cortex-Asomething module thing?
>>
>>
>> Juno R2 will have the PCIe controller too and it should hopefully work.
>
> But it will also be the A52.  Or no, the A72?  Or can't say..
>

If I knew the answer, "can't say" would probably be the official line.
But I haven't been told of any plans to change the cores, so I am
assuming the next Juno respin will be A57/A53 big.LITTLE.  Just like
we have now but with fixes.


>> But this controller is not Cortex-Asomething related.  Another vendor could
>> put the same IP block on their board, of course.
>
> Right, but it wouldn't be under board/armltd/vexpress64/ either..
>
>> FVP models don't have it and other ARM platforms won't necessarily have it.
>>
>> Does that help?!
>
> Yes, thanks.  I think it's just a style thing then.  We don't do a lot
> of static inline nop functions, we do __weak functions in the main file
> (and comment about what it should be doing in a real function) and then
> provide the strong version in another file.  So just the pcie.h part
> needs changing then.

Then it's over to Linus...
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] vexpress64: compile Juno PCIe conditionally

2015-10-21 Thread Tom Rini 
On Wed, Oct 21, 2015 at 12:00:03PM +0100, Ryan Harkin wrote:
> On 20 October 2015 at 16:38, Tom Rini  wrote:
> 
> > On Tue, Oct 20, 2015 at 08:05:40AM +0200, Linus Walleij wrote:
> >
> > > Only compile in PCIe support if the board really uses it. Provide
> > > a stub for the init function if e.g. FVP is being built.
> > >
> > > Cc: Liviu Dudau 
> > > Cc: Ryan Harkin 
> > > Signed-off-by: Linus Walleij 
> > > ---
> > >  board/armltd/vexpress64/Makefile | 3 ++-
> > >  board/armltd/vexpress64/pcie.c   | 2 --
> > >  board/armltd/vexpress64/pcie.h   | 4 
> > >  3 files changed, 6 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/board/armltd/vexpress64/Makefile
> > b/board/armltd/vexpress64/Makefile
> > > index a35db401b684..b4391a71249a 100644
> > > --- a/board/armltd/vexpress64/Makefile
> > > +++ b/board/armltd/vexpress64/Makefile
> > > @@ -5,4 +5,5 @@
> > >  # SPDX-License-Identifier:   GPL-2.0+
> > >  #
> > >
> > > -obj-y:= vexpress64.o pcie.o
> > > +obj-y:= vexpress64.o
> > > +obj-$(CONFIG_TARGET_VEXPRESS64_JUNO) += pcie.o
> > > diff --git a/board/armltd/vexpress64/pcie.c
> > b/board/armltd/vexpress64/pcie.c
> > > index 7b999e8ef40b..311c4500e3ff 100644
> > > --- a/board/armltd/vexpress64/pcie.c
> > > +++ b/board/armltd/vexpress64/pcie.c
> > > @@ -191,7 +191,5 @@ void xr3pci_init(void)
> > >
> > >  void vexpress64_pcie_init(void)
> > >  {
> > > -#ifdef CONFIG_TARGET_VEXPRESS64_JUNO
> > >   xr3pci_init();
> > > -#endif
> > >  }
> > > diff --git a/board/armltd/vexpress64/pcie.h
> > b/board/armltd/vexpress64/pcie.h
> > > index 14642f4f5c43..55b276d6af11 100644
> > > --- a/board/armltd/vexpress64/pcie.h
> > > +++ b/board/armltd/vexpress64/pcie.h
> > > @@ -1,6 +1,10 @@
> > >  #ifndef __VEXPRESS64_PCIE_H__
> > >  #define __VEXPRESS64_PCIE_H__
> > >
> > > +#ifdef CONFIG_TARGET_VEXPRESS64_JUNO
> > >  void vexpress64_pcie_init(void);
> > > +#else
> > > +static inline void vexpress64_pcie_init(void) {}
> > > +#endif
> > >
> > >  #endif /* __VEXPRESS64_PCIE_H__ */
> >
> > Alright, so the versatile platform makes life fun at times.
> 
> 
> This comes back to the refactoring thread we had recently...
> 
> 
> 
> >   First, my
> > preference is for weak functions (and I _think_ the linker ends up being
> > smart enough about them to optimize things away, if not, arrg).  Second,
> > the question I have is, is this xr3pci_init() bit really a Juno thing or
> > a baseboard thing (I assume no)
> 
> 
> It's sort-of the same thing on Juno.  Juno has a baseboard and SoC in one
> build, unlike the previous 32-bit Versatile Express motherboard that takes
> core tiles with different cores on it.
> 
> Juno R0 has the PCIe controller, but it doesn't work.  So the code is safe
> to run at this level.  Juno R1 has the controller and it works.
> 
> or a going to be the same on the next
> > Cortex-Asomething module thing?
> 
> 
> Juno R2 will have the PCIe controller too and it should hopefully work.

But it will also be the A52.  Or no, the A72?  Or can't say..

> But this controller is not Cortex-Asomething related.  Another vendor could
> put the same IP block on their board, of course.

Right, but it wouldn't be under board/armltd/vexpress64/ either..

> FVP models don't have it and other ARM platforms won't necessarily have it.
> 
> Does that help?!

Yes, thanks.  I think it's just a style thing then.  We don't do a lot
of static inline nop functions, we do __weak functions in the main file
(and comment about what it should be doing in a real function) and then
provide the strong version in another file.  So just the pcie.h part
needs changing then.

-- 
Tom


signature.asc
Description: Digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] vexpress64: compile Juno PCIe conditionally

2015-10-21 Thread Ryan Harkin 
On 20 October 2015 at 16:38, Tom Rini  wrote:

> On Tue, Oct 20, 2015 at 08:05:40AM +0200, Linus Walleij wrote:
>
> > Only compile in PCIe support if the board really uses it. Provide
> > a stub for the init function if e.g. FVP is being built.
> >
> > Cc: Liviu Dudau 
> > Cc: Ryan Harkin 
> > Signed-off-by: Linus Walleij 
> > ---
> >  board/armltd/vexpress64/Makefile | 3 ++-
> >  board/armltd/vexpress64/pcie.c   | 2 --
> >  board/armltd/vexpress64/pcie.h   | 4 
> >  3 files changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/board/armltd/vexpress64/Makefile
> b/board/armltd/vexpress64/Makefile
> > index a35db401b684..b4391a71249a 100644
> > --- a/board/armltd/vexpress64/Makefile
> > +++ b/board/armltd/vexpress64/Makefile
> > @@ -5,4 +5,5 @@
> >  # SPDX-License-Identifier:   GPL-2.0+
> >  #
> >
> > -obj-y:= vexpress64.o pcie.o
> > +obj-y:= vexpress64.o
> > +obj-$(CONFIG_TARGET_VEXPRESS64_JUNO) += pcie.o
> > diff --git a/board/armltd/vexpress64/pcie.c
> b/board/armltd/vexpress64/pcie.c
> > index 7b999e8ef40b..311c4500e3ff 100644
> > --- a/board/armltd/vexpress64/pcie.c
> > +++ b/board/armltd/vexpress64/pcie.c
> > @@ -191,7 +191,5 @@ void xr3pci_init(void)
> >
> >  void vexpress64_pcie_init(void)
> >  {
> > -#ifdef CONFIG_TARGET_VEXPRESS64_JUNO
> >   xr3pci_init();
> > -#endif
> >  }
> > diff --git a/board/armltd/vexpress64/pcie.h
> b/board/armltd/vexpress64/pcie.h
> > index 14642f4f5c43..55b276d6af11 100644
> > --- a/board/armltd/vexpress64/pcie.h
> > +++ b/board/armltd/vexpress64/pcie.h
> > @@ -1,6 +1,10 @@
> >  #ifndef __VEXPRESS64_PCIE_H__
> >  #define __VEXPRESS64_PCIE_H__
> >
> > +#ifdef CONFIG_TARGET_VEXPRESS64_JUNO
> >  void vexpress64_pcie_init(void);
> > +#else
> > +static inline void vexpress64_pcie_init(void) {}
> > +#endif
> >
> >  #endif /* __VEXPRESS64_PCIE_H__ */
>
> Alright, so the versatile platform makes life fun at times.


This comes back to the refactoring thread we had recently...



>   First, my
> preference is for weak functions (and I _think_ the linker ends up being
> smart enough about them to optimize things away, if not, arrg).  Second,
> the question I have is, is this xr3pci_init() bit really a Juno thing or
> a baseboard thing (I assume no)


It's sort-of the same thing on Juno.  Juno has a baseboard and SoC in one
build, unlike the previous 32-bit Versatile Express motherboard that takes
core tiles with different cores on it.

Juno R0 has the PCIe controller, but it doesn't work.  So the code is safe
to run at this level.  Juno R1 has the controller and it works.

or a going to be the same on the next
> Cortex-Asomething module thing?


Juno R2 will have the PCIe controller too and it should hopefully work.

But this controller is not Cortex-Asomething related.  Another vendor could
put the same IP block on their board, of course.

FVP models don't have it and other ARM platforms won't necessarily have it.

Does that help?!



>   The way the code stands today a
> hypothethical A72-based vexpress64 module would need to call
> vexpress64_pcie_init()


Hmmm, that doesn't seem right, does it?


but not strictly xr3pci_init().  It would be a
> little ugly as pcie.c would #if/#elif/#endif the enabler call however so
> we could __weak a no-op in board.c and rename pcie.c to juno.c and have
> a72codenameboard.c later on with its own function.
>
> --
> Tom
>
> ___
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
>
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] vexpress64: compile Juno PCIe conditionally

2015-10-20 Thread Ryan Harkin 
On 20 October 2015 at 09:07, Ryan Harkin  wrote:

>
>
> On 20 October 2015 at 07:05, Linus Walleij 
> wrote:
>
>> Only compile in PCIe support if the board really uses it. Provide
>> a stub for the init function if e.g. FVP is being built.
>>
>> Cc: Liviu Dudau 
>> Cc: Ryan Harkin 
>> Signed-off-by: Linus Walleij 
>> ---
>>  board/armltd/vexpress64/Makefile | 3 ++-
>>  board/armltd/vexpress64/pcie.c   | 2 --
>>  board/armltd/vexpress64/pcie.h   | 4 
>>  3 files changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/board/armltd/vexpress64/Makefile
>> b/board/armltd/vexpress64/Makefile
>> index a35db401b684..b4391a71249a 100644
>> --- a/board/armltd/vexpress64/Makefile
>> +++ b/board/armltd/vexpress64/Makefile
>> @@ -5,4 +5,5 @@
>>  # SPDX-License-Identifier: GPL-2.0+
>>  #
>>
>> -obj-y  := vexpress64.o pcie.o
>> +obj-y  := vexpress64.o
>> +obj-$(CONFIG_TARGET_VEXPRESS64_JUNO)   += pcie.o
>> diff --git a/board/armltd/vexpress64/pcie.c
>> b/board/armltd/vexpress64/pcie.c
>> index 7b999e8ef40b..311c4500e3ff 100644
>> --- a/board/armltd/vexpress64/pcie.c
>> +++ b/board/armltd/vexpress64/pcie.c
>> @@ -191,7 +191,5 @@ void xr3pci_init(void)
>>
>>  void vexpress64_pcie_init(void)
>>  {
>> -#ifdef CONFIG_TARGET_VEXPRESS64_JUNO
>> xr3pci_init();
>> -#endif
>>  }
>> diff --git a/board/armltd/vexpress64/pcie.h
>> b/board/armltd/vexpress64/pcie.h
>> index 14642f4f5c43..55b276d6af11 100644
>> --- a/board/armltd/vexpress64/pcie.h
>> +++ b/board/armltd/vexpress64/pcie.h
>> @@ -1,6 +1,10 @@
>>  #ifndef __VEXPRESS64_PCIE_H__
>>  #define __VEXPRESS64_PCIE_H__
>>
>> +#ifdef CONFIG_TARGET_VEXPRESS64_JUNO
>>  void vexpress64_pcie_init(void);
>> +#else
>> +static inline void vexpress64_pcie_init(void) {}
>> +#endif
>>
>>  #endif /* __VEXPRESS64_PCIE_H__ */
>>
>
> Tom specifically dropped this hunk when he merged Liviu's patch.  But is
> it necessary with the rest of your patch?  If pcie.h is only included in
> pcie.c, which is only built for JUNO, then the code shouldn't need
> conditional compilation. So I think we can drop the hunk, no?
>

Sorry, I'm wrong here.  vexpress64_pcie_init is called unconditionally from
board_init(), so needs a stub definition for non-Juno targets.

I guess another way to do it would be to make the call in board_init()
conditional, but that's not really any different than your patch, so I'm
happy enough with yours.

Reviewed-by: Ryan Harkin 
Tested-by: Ryan Harkin 
(I tested on FVP dram configuration, Juno R0 and Juno R1)
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH] vexpress64: compile Juno PCIe conditionally

2015-10-20 Thread Linus Walleij 
Only compile in PCIe support if the board really uses it. Provide
a stub for the init function if e.g. FVP is being built.

Cc: Liviu Dudau 
Cc: Ryan Harkin 
Signed-off-by: Linus Walleij 
---
 board/armltd/vexpress64/Makefile | 3 ++-
 board/armltd/vexpress64/pcie.c   | 2 --
 board/armltd/vexpress64/pcie.h   | 4 
 3 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/board/armltd/vexpress64/Makefile b/board/armltd/vexpress64/Makefile
index a35db401b684..b4391a71249a 100644
--- a/board/armltd/vexpress64/Makefile
+++ b/board/armltd/vexpress64/Makefile
@@ -5,4 +5,5 @@
 # SPDX-License-Identifier: GPL-2.0+
 #
 
-obj-y  := vexpress64.o pcie.o
+obj-y  := vexpress64.o
+obj-$(CONFIG_TARGET_VEXPRESS64_JUNO)   += pcie.o
diff --git a/board/armltd/vexpress64/pcie.c b/board/armltd/vexpress64/pcie.c
index 7b999e8ef40b..311c4500e3ff 100644
--- a/board/armltd/vexpress64/pcie.c
+++ b/board/armltd/vexpress64/pcie.c
@@ -191,7 +191,5 @@ void xr3pci_init(void)
 
 void vexpress64_pcie_init(void)
 {
-#ifdef CONFIG_TARGET_VEXPRESS64_JUNO
xr3pci_init();
-#endif
 }
diff --git a/board/armltd/vexpress64/pcie.h b/board/armltd/vexpress64/pcie.h
index 14642f4f5c43..55b276d6af11 100644
--- a/board/armltd/vexpress64/pcie.h
+++ b/board/armltd/vexpress64/pcie.h
@@ -1,6 +1,10 @@
 #ifndef __VEXPRESS64_PCIE_H__
 #define __VEXPRESS64_PCIE_H__
 
+#ifdef CONFIG_TARGET_VEXPRESS64_JUNO
 void vexpress64_pcie_init(void);
+#else
+static inline void vexpress64_pcie_init(void) {}
+#endif
 
 #endif /* __VEXPRESS64_PCIE_H__ */
-- 
2.4.3

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

Re: [U-Boot] [PATCH] vexpress64: compile Juno PCIe conditionally

2015-10-20 Thread Ryan Harkin 
On 20 October 2015 at 07:05, Linus Walleij  wrote:

> Only compile in PCIe support if the board really uses it. Provide
> a stub for the init function if e.g. FVP is being built.
>
> Cc: Liviu Dudau 
> Cc: Ryan Harkin 
> Signed-off-by: Linus Walleij 
> ---
>  board/armltd/vexpress64/Makefile | 3 ++-
>  board/armltd/vexpress64/pcie.c   | 2 --
>  board/armltd/vexpress64/pcie.h   | 4 
>  3 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/board/armltd/vexpress64/Makefile
> b/board/armltd/vexpress64/Makefile
> index a35db401b684..b4391a71249a 100644
> --- a/board/armltd/vexpress64/Makefile
> +++ b/board/armltd/vexpress64/Makefile
> @@ -5,4 +5,5 @@
>  # SPDX-License-Identifier: GPL-2.0+
>  #
>
> -obj-y  := vexpress64.o pcie.o
> +obj-y  := vexpress64.o
> +obj-$(CONFIG_TARGET_VEXPRESS64_JUNO)   += pcie.o
> diff --git a/board/armltd/vexpress64/pcie.c
> b/board/armltd/vexpress64/pcie.c
> index 7b999e8ef40b..311c4500e3ff 100644
> --- a/board/armltd/vexpress64/pcie.c
> +++ b/board/armltd/vexpress64/pcie.c
> @@ -191,7 +191,5 @@ void xr3pci_init(void)
>
>  void vexpress64_pcie_init(void)
>  {
> -#ifdef CONFIG_TARGET_VEXPRESS64_JUNO
> xr3pci_init();
> -#endif
>  }
> diff --git a/board/armltd/vexpress64/pcie.h
> b/board/armltd/vexpress64/pcie.h
> index 14642f4f5c43..55b276d6af11 100644
> --- a/board/armltd/vexpress64/pcie.h
> +++ b/board/armltd/vexpress64/pcie.h
> @@ -1,6 +1,10 @@
>  #ifndef __VEXPRESS64_PCIE_H__
>  #define __VEXPRESS64_PCIE_H__
>
> +#ifdef CONFIG_TARGET_VEXPRESS64_JUNO
>  void vexpress64_pcie_init(void);
> +#else
> +static inline void vexpress64_pcie_init(void) {}
> +#endif
>
>  #endif /* __VEXPRESS64_PCIE_H__ */
>

Tom specifically dropped this hunk when he merged Liviu's patch.  But is it
necessary with the rest of your patch?  If pcie.h is only included in
pcie.c, which is only built for JUNO, then the code shouldn't need
conditional compilation. So I think we can drop the hunk, no?
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] vexpress64: compile Juno PCIe conditionally

2015-10-20 Thread Tom Rini 
On Tue, Oct 20, 2015 at 08:05:40AM +0200, Linus Walleij wrote:

> Only compile in PCIe support if the board really uses it. Provide
> a stub for the init function if e.g. FVP is being built.
> 
> Cc: Liviu Dudau 
> Cc: Ryan Harkin 
> Signed-off-by: Linus Walleij 
> ---
>  board/armltd/vexpress64/Makefile | 3 ++-
>  board/armltd/vexpress64/pcie.c   | 2 --
>  board/armltd/vexpress64/pcie.h   | 4 
>  3 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/board/armltd/vexpress64/Makefile 
> b/board/armltd/vexpress64/Makefile
> index a35db401b684..b4391a71249a 100644
> --- a/board/armltd/vexpress64/Makefile
> +++ b/board/armltd/vexpress64/Makefile
> @@ -5,4 +5,5 @@
>  # SPDX-License-Identifier:   GPL-2.0+
>  #
>  
> -obj-y:= vexpress64.o pcie.o
> +obj-y:= vexpress64.o
> +obj-$(CONFIG_TARGET_VEXPRESS64_JUNO) += pcie.o
> diff --git a/board/armltd/vexpress64/pcie.c b/board/armltd/vexpress64/pcie.c
> index 7b999e8ef40b..311c4500e3ff 100644
> --- a/board/armltd/vexpress64/pcie.c
> +++ b/board/armltd/vexpress64/pcie.c
> @@ -191,7 +191,5 @@ void xr3pci_init(void)
>  
>  void vexpress64_pcie_init(void)
>  {
> -#ifdef CONFIG_TARGET_VEXPRESS64_JUNO
>   xr3pci_init();
> -#endif
>  }
> diff --git a/board/armltd/vexpress64/pcie.h b/board/armltd/vexpress64/pcie.h
> index 14642f4f5c43..55b276d6af11 100644
> --- a/board/armltd/vexpress64/pcie.h
> +++ b/board/armltd/vexpress64/pcie.h
> @@ -1,6 +1,10 @@
>  #ifndef __VEXPRESS64_PCIE_H__
>  #define __VEXPRESS64_PCIE_H__
>  
> +#ifdef CONFIG_TARGET_VEXPRESS64_JUNO
>  void vexpress64_pcie_init(void);
> +#else
> +static inline void vexpress64_pcie_init(void) {}
> +#endif
>  
>  #endif /* __VEXPRESS64_PCIE_H__ */

Alright, so the versatile platform makes life fun at times.  First, my
preference is for weak functions (and I _think_ the linker ends up being
smart enough about them to optimize things away, if not, arrg).  Second,
the question I have is, is this xr3pci_init() bit really a Juno thing or
a baseboard thing (I assume no) or a going to be the same on the next
Cortex-Asomething module thing?  The way the code stands today a
hypothethical A72-based vexpress64 module would need to call
vexpress64_pcie_init() but not strictly xr3pci_init().  It would be a
little ugly as pcie.c would #if/#elif/#endif the enabler call however so
we could __weak a no-op in board.c and rename pcie.c to juno.c and have
a72codenameboard.c later on with its own function.

-- 
Tom


signature.asc
Description: Digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot