[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 <ryan.har...@linaro.org> wrote:
> On 21 October 2015 at 13:50, Tom Rini <tr...@konsulko.com> wrote:
>> On Wed, Oct 21, 2015 at 12:00:03PM +0100, Ryan Harkin wrote:
>>> On 20 October 2015 at 16:38, Tom Rini <tr...@konsulko.com> 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 <liviu.du...@foss.arm.com>
>>> > > Cc: Ryan Harkin <ryan.har...@linaro.org>
>>> > > Signed-off-by: Linus Walleij <linus.wall...@linaro.org>
>>> > > ---
>>> > > 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
