Re: [PATCH v3 06/15] ARM: at91: import lowlevel clock initialization from at91bootstrap
Hi Ahmad. > >> @@ -1,4 +1,5 @@ > >> obj-y += setup.o > >> +pbl-y += at91_pmc_ll.o > > > > Will this always make my pbl image larger or do we have some linker > > magic that throws away the unused code? > > My understanding is that barebox instructs GCC to place each out-of-line > function > into a separate section (with -ffunction-sections) and then the linker prunes > all > unreferenced sections. > > > If the pbl image become larger then we need to add this in a conditional > > manner > > as we cannot allow this for all at91 variants. > > I used to have conditional function definitions in a prior version (albeit > for the SDRAM variants initialization routines) and Sascha commented that > the #ifdefery can be removed as the linker takes care of it. I should have known already that we used --function-sections as I worked with this in the kernel in my former life. Much better than ifdeffery all over the code or in Makefiles. Sam ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH v3 04/15] ARM: at91: replace at91sam9_ddrsdr.h with at91bootstrap's at91_ddrsdrc.h
Hi Ahmad.
> >
> > If we introduced the following small helpers we could simplify the call
> > sites
> > to just use at91_get_ddram_size(AT91SAM9G45_BASE_DDRSDRC0) and similar.
> >
> > static inline u32 at91_get_ddram_size(void __force __iomem * addr)
> > {
> > return __at91_get_ddram_size(IOMEM(addr), true);
> > }
> >
> > static inline u32 at91sam9g45_get_ddram_size(void __force __iomem * addr)
> > {
> > return __at91_get_ddram_size(IOMEM(addr), false);
> > }
>
> That would still require the pointer cast to pacify the -Wint-conversion.
> And (void*)ADDR looks only marginally better IOMEM(ADDR) IMO
> at the cost of having sparse miss passing memory pointers
> (__force __iomem would be equivalent to just __attribute__((noderef)),
> wouldn't it?)
>
> My preference would've been that AT91SAM9G45_BASE_DDRSDRC0 already expands
> to a void __iomem *.
The main idea was to get rid of the bool argument and use more explicit function
names. Moving IOMEM() was the added extra.
> We could do that in a follow-up patch.
Agreed, maybe later if we do it.
The original patch in this mail was fine.
You can add my:
Reviewed-by: Sam Ravnborg
If you respin the series.
Sam
___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH v3 05/15] ARM: at91: watchdog: implement at91_wdt_disable
On Tue, Apr 02, 2019 at 08:39:56PM +0200, Ahmad Fatoum wrote:
> Hello Sam,
>
> On 2/4/19 19:38, Sam Ravnborg wrote:
> > Hi Ahmad.
> >
> > On Mon, Apr 01, 2019 at 12:18:13PM +0200, Ahmad Fatoum wrote:
> >> Low level init code might want to disable the watchdog in PBL.
> >> Provide a helper to do so.
> >>
> >> Signed-off-by: Ahmad Fatoum
> >> ---
> >> arch/arm/mach-at91/include/mach/at91_wdt.h | 16
> >> 1 file changed, 16 insertions(+)
> >>
> >> diff --git a/arch/arm/mach-at91/include/mach/at91_wdt.h
> >> b/arch/arm/mach-at91/include/mach/at91_wdt.h
> >> index 36d37b9d2d64..d295d35d1b5c 100644
> >> --- a/arch/arm/mach-at91/include/mach/at91_wdt.h
> >> +++ b/arch/arm/mach-at91/include/mach/at91_wdt.h
> >> @@ -35,4 +35,20 @@
> >> #define AT91_WDT_WDUNF (1 << 0)/*
> >> Watchdog Underflow */
> >> #define AT91_WDT_WDERR (1 << 1)/*
> >> Watchdog Error */
> >>
> >> +#ifndef __ASSEMBLY__
> >> +// SPDX-License-Identifier: BSD-1-Clause
> >> +/*
> >> + * Copyright (c) 2006, Atmel Corporation
> >> + */
> >> +
> >> +#include
> >> +
> >> +static inline void at91_wdt_disable(void __iomem *wdt_base)
> >
> > As we are copying a lot of code from at91bootstrap could
> > we then use the same name for this function:
> > driver/at91_wdt.c:void at91_disable_wdt(void)
>
> ${arch}_${subsystem}_${function} seems to be the usual naming scheme
> for "globals". Why differ? I don't see the utility of copying
> the at91bootstrap name.
Yeah, keep it in line with barebox is better.
It was more a something I stumbled upon comment.
Here you can also add my:
Reviewed-by: Sam Ravnborg
Sam
___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH v3 06/15] ARM: at91: import lowlevel clock initialization from at91bootstrap
Hello Sam, On 2/4/19 19:42, Sam Ravnborg wrote: > Hi Ahmad. > > On Mon, Apr 01, 2019 at 12:18:14PM +0200, Ahmad Fatoum wrote: >> For use by future at91 first stage bootloaders, this commit imports >> https://github.com/linux4sam/at91bootstrap/blob/v3.8.12/driver/pmc.c >> >> Signed-off-by: Ahmad Fatoum >> --- >> arch/arm/mach-at91/Makefile | 1 + >> arch/arm/mach-at91/at91_pmc_ll.c | 184 ++ >> arch/arm/mach-at91/include/mach/at91_pmc.h| 24 ++- >> arch/arm/mach-at91/include/mach/at91_pmc_ll.h | 78 >> 4 files changed, 284 insertions(+), 3 deletions(-) >> create mode 100644 arch/arm/mach-at91/at91_pmc_ll.c >> create mode 100644 arch/arm/mach-at91/include/mach/at91_pmc_ll.h >> >> diff --git a/arch/arm/mach-at91/Makefile b/arch/arm/mach-at91/Makefile >> index d81683ac121a..91b06c085107 100644 >> --- a/arch/arm/mach-at91/Makefile >> +++ b/arch/arm/mach-at91/Makefile >> @@ -1,4 +1,5 @@ >> obj-y += setup.o >> +pbl-y += at91_pmc_ll.o > > Will this always make my pbl image larger or do we have some linker > magic that throws away the unused code? My understanding is that barebox instructs GCC to place each out-of-line function into a separate section (with -ffunction-sections) and then the linker prunes all unreferenced sections. > If the pbl image become larger then we need to add this in a conditional > manner > as we cannot allow this for all at91 variants. I used to have conditional function definitions in a prior version (albeit for the SDRAM variants initialization routines) and Sascha commented that the #ifdefery can be removed as the linker takes care of it. > > Sam > -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH v3 05/15] ARM: at91: watchdog: implement at91_wdt_disable
Hello Sam,
On 2/4/19 19:38, Sam Ravnborg wrote:
> Hi Ahmad.
>
> On Mon, Apr 01, 2019 at 12:18:13PM +0200, Ahmad Fatoum wrote:
>> Low level init code might want to disable the watchdog in PBL.
>> Provide a helper to do so.
>>
>> Signed-off-by: Ahmad Fatoum
>> ---
>> arch/arm/mach-at91/include/mach/at91_wdt.h | 16
>> 1 file changed, 16 insertions(+)
>>
>> diff --git a/arch/arm/mach-at91/include/mach/at91_wdt.h
>> b/arch/arm/mach-at91/include/mach/at91_wdt.h
>> index 36d37b9d2d64..d295d35d1b5c 100644
>> --- a/arch/arm/mach-at91/include/mach/at91_wdt.h
>> +++ b/arch/arm/mach-at91/include/mach/at91_wdt.h
>> @@ -35,4 +35,20 @@
>> #define AT91_WDT_WDUNF (1 << 0)/*
>> Watchdog Underflow */
>> #define AT91_WDT_WDERR (1 << 1)/*
>> Watchdog Error */
>>
>> +#ifndef __ASSEMBLY__
>> +// SPDX-License-Identifier: BSD-1-Clause
>> +/*
>> + * Copyright (c) 2006, Atmel Corporation
>> + */
>> +
>> +#include
>> +
>> +static inline void at91_wdt_disable(void __iomem *wdt_base)
>
> As we are copying a lot of code from at91bootstrap could
> we then use the same name for this function:
> driver/at91_wdt.c:void at91_disable_wdt(void)
${arch}_${subsystem}_${function} seems to be the usual naming scheme
for "globals". Why differ? I don't see the utility of copying
the at91bootstrap name.
>
> In at91bootstrap the address is a build time constant,
> but passing it as an value is better.
> So that part I do not ask you to consider to copy.
>
> Sam
>
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH v3 04/15] ARM: at91: replace at91sam9_ddrsdr.h with at91bootstrap's at91_ddrsdrc.h
Hi Sam,
On 2/4/19 19:25, Sam Ravnborg wrote:
> Hi Ahmad.
>
> On Mon, Apr 01, 2019 at 12:18:12PM +0200, Ahmad Fatoum wrote:
>> Instead of adding missing definitions to the existing at91sam9_ddrsdr.h
>> and adapting the incoming DDRAM initialization code from at91bootstrap,
>> just replace the lightly used existing header with:
>> https://github.com/linux4sam/at91bootstrap/blob/v3.8.12/include/arch/at91_ddrsdrc.h
>>
>> For easier comprehension, the replacement is done in three steps:
>> This last step copies the memory size querying functions from
>> at91sam9_ddrsdr.h
>> to at91_ddrsdrc.h, then deletes it and fixes all references.
>>
>> Signed-off-by: Ahmad Fatoum
>> ---
>
>
>> diff --git a/arch/arm/mach-at91/include/mach/at91_ddrsdrc.h
>> b/arch/arm/mach-at91/include/mach/at91_ddrsdrc.h
>> index 57d0d8f489c4..7e68e7dd63eb 100644
>> --- a/arch/arm/mach-at91/include/mach/at91_ddrsdrc.h
>> +++ b/arch/arm/mach-at91/include/mach/at91_ddrsdrc.h
>> @@ -285,4 +285,48 @@
>> #define AT91C_DDRC2_WPVS(0x1UL << 0)
>> #define AT91C_DDRC2_WPSRC (0xUL << 8)
>>
>> +#ifndef __ASSEMBLY__
>> +#include
>> +#include
>> +
>> +static inline u32 at91_get_ddram_size(void * __iomem base, bool is_nb)
>> +{
>
> I should have brought this up in the previous patch-set, but it continue to
> itch me.
> So you get the feedback here
>
> We have at91_get_ddram_size(IOMEM(xxx), {true or false})
>
> If we introduced the following small helpers we could simplify the call sites
> to just use at91_get_ddram_size(AT91SAM9G45_BASE_DDRSDRC0) and similar.
>
> static inline u32 at91_get_ddram_size(void __force __iomem * addr)
> {
> return __at91_get_ddram_size(IOMEM(addr), true);
> }
>
> static inline u32 at91sam9g45_get_ddram_size(void __force __iomem * addr)
> {
> return __at91_get_ddram_size(IOMEM(addr), false);
> }
That would still require the pointer cast to pacify the -Wint-conversion.
And (void*)ADDR looks only marginally better IOMEM(ADDR) IMO
at the cost of having sparse miss passing memory pointers
(__force __iomem would be equivalent to just __attribute__((noderef)), wouldn't
it?)
My preference would've been that AT91SAM9G45_BASE_DDRSDRC0 already expands
to a void __iomem *. We could do that in a follow-up patch.
>
> But then for the few call sites it is not really worth it anyway.
> Anyway, just a small itch. Just ignore it.
>
> Sam
>
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH v3 00/15] ARM: at91: microchip-kz9477-evb: support first stage boot
Hi Ahmad. It is super that you are working on this and I hope we can have bootstrap support for more at91 variants in the near future. > This is (the hopefully final and bestest) v3 of a series that pulls in > enough of at91bootstrap to make barebox usable as first stage bootloader > for the SAMA5 family of AT91 SoCs. Sorry for gettting back to this patch set only now. I have been through all patches and there was a few minor comments here and there. Let me know if this triggers any questions. Sam ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH v3 15/15] doc: microchip-ksz9477-evb: add documentation
Hi Ahmad. Thanks for adding the docs. On Mon, Apr 01, 2019 at 12:18:23PM +0200, Ahmad Fatoum wrote: > Signed-off-by: Ahmad Fatoum > --- > .../boards/at91/microchip-ksz9477-evb.rst | 38 ++- > 1 file changed, 36 insertions(+), 2 deletions(-) > > diff --git a/Documentation/boards/at91/microchip-ksz9477-evb.rst > b/Documentation/boards/at91/microchip-ksz9477-evb.rst > index 4c4c4aecbfb3..2a68c2a552cd 100644 > --- a/Documentation/boards/at91/microchip-ksz9477-evb.rst > +++ b/Documentation/boards/at91/microchip-ksz9477-evb.rst > @@ -1,11 +1,45 @@ > Microchip KSZ 9477 Evaluation board > === > > -This is an evaluation board for a switch that uses the at91sam9x5 CPU. > +This is an evaluation board for the KSZ9477 switch that uses the sama5d36 > CPU. > The board uses Device Tree and supports multi image. > > -Building barebox: > +Building barebox as second stage bootloader: > > .. code-block:: sh > >make ARCH=arm microchip_ksz9477_evb_defconfig > + > +There are also a separate defconfig for operating barebox as first stage > +bootloader originating from SD Card. > +This configuration doesn't yet support device-tree use as the NVM bootloader > +(SoC ROM code) requires the first stage bootloader to fit into 64K. > + > +Generally, the first stage may comes from any of the following boot > +sources (in that order): > + > +* SPI0 CS0 Flash > +* SD Card > +* NAND Flash > +* SPI0 CS1 Flash > +* I2C EEPROM > + > +After being loaded into SRAM by the NVM bootloader, the first stage does low > +level clock initialization, configuration of the DDRAM controller and > +bootstraps the second stage boot loader. > + > +SD Card Bootstrap > +- > + > +For boot from SD card, barebox additionally needs to be configured as > +first stage bootloader: > + > +.. code-block:: sh > + > + make ARCH=arm microchip_ksz9477_evb_bootstrap_mmc_defconfig > + > +The resulting barebox image must be renamed to ``BOOT.BIN`` When I am away from barebox development for some weeks I always have troubles what it the right file to use. I suggest to use specific filename rather than the vauge "resulting barebox image". Sam ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH v3 14/15] ARM: at91: microchip-ksz9477: provide board code fallback
Hi Ahmad.
On Mon, Apr 01, 2019 at 12:18:22PM +0200, Ahmad Fatoum wrote:
> The newly added device tree based first stage fails to load the second
> stage from MMC, which might be in relation to a preceding atmel_mci
> "command/data timeout" message.
I had similar troubles with MMC on at91sam9263.
After some digging the fix was simple:
See 5feabc1e6737742f1cf6a1c41921f68b4dbb5c10 ("arm: at91: fix clock to mci1 for
at91sam9263")
Maybe you are hit by something as simple as this?
> +
> +static const struct devfs_partition sama5d3_xplained_nand0_partitions[] = {
> + {
> + .offset = 0x0,
> + .size = SZ_256K,
> + .flags = DEVFS_PARTITION_FIXED,
> + .name = "at91bootstrap_raw",
> + .bbname = "at91bootstrap",
> + },
Strange partition name now we use barebox as first stage.
> +++ b/arch/arm/configs/microchip_ksz9477_evb_bootstrap_mmc_defconfig
Naming confuses me.
Do we stick to the name "first stage" or do we use "bootstrap"?
For me they are the same, but I get confused when we use both.
Sam
___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH v3 10/15] images: at91: differentiate between first and second stage images
Hi Ahmad. On Mon, Apr 01, 2019 at 12:18:18PM +0200, Ahmad Fatoum wrote: > Incoming microchip-ksz9477-evb first stage will add one more entry point > for the first stage. As there is a little reason to use the same piggy > data for both images (BOOT.BIN, the first stage, is limited to 64K), have > CONFIG_AT91_LOAD_BAREBOX_SRAM decide which stage should be built. > > Signed-off-by: Ahmad Fatoum > --- > arch/arm/mach-at91/Kconfig | 2 +- > images/Makefile.at91 | 12 +--- > 2 files changed, 10 insertions(+), 4 deletions(-) > > diff --git a/arch/arm/mach-at91/Kconfig b/arch/arm/mach-at91/Kconfig > index 8e1bf0629ab7..60f427d7d483 100644 > --- a/arch/arm/mach-at91/Kconfig > +++ b/arch/arm/mach-at91/Kconfig > @@ -615,7 +615,7 @@ config AT91_BOOTSTRAP > select BOOTSTRAP > > config AT91_LOAD_BAREBOX_SRAM > - bool "at91 load barebox in sram" > + bool "at91 barebox image will be loaded into SRAM" > depends on SHELL_NONE || HAVE_AT91_LOAD_BAREBOX_SRAM > default y if SHELL_NONE > > diff --git a/images/Makefile.at91 b/images/Makefile.at91 > index acdb591d2452..3f1dd57f6c58 100644 > --- a/images/Makefile.at91 > +++ b/images/Makefile.at91 > @@ -4,12 +4,18 @@ > > pblb-$(CONFIG_MACH_AT91SAM9X5EK) += start_at91sam9x5ek > FILE_barebox-at91sam9x5ek.img = start_at91sam9x5ek.pblb > -image-$(CONFIG_MACH_AT91SAM9X5EK) += barebox-at91sam9x5ek.img > +at91-barebox-$(CONFIG_MACH_AT91SAM9X5EK) += barebox-at91sam9x5ek.img > > pblb-$(CONFIG_MACH_AT91SAM9263EK) += start_at91sam9263ek > FILE_barebox-at91sam9263ek.img = start_at91sam9263ek.pblb > -image-$(CONFIG_MACH_AT91SAM9263EK) += barebox-at91sam9263ek.img > +at91-barebox-$(CONFIG_MACH_AT91SAM9263EK) += barebox-at91sam9263ek.img > > pblb-$(CONFIG_MACH_MICROCHIP_KSZ9477_EVB) += start_sama5d3_xplained_ung8071 > FILE_barebox-microchip-ksz9477-evb.img = start_sama5d3_xplained_ung8071.pblb > -image-$(CONFIG_MACH_MICROCHIP_KSZ9477_EVB) += > barebox-microchip-ksz9477-evb.img > +at91-barebox-$(CONFIG_MACH_MICROCHIP_KSZ9477_EVB) += > barebox-microchip-ksz9477-evb.img > + > +ifdef CONFIG_AT91_LOAD_BAREBOX_SRAM > +image-y += $(at91-boot-bin-y) > +else > +image-y += $(at91-barebox-y) > +endif I cannot follow what you do in the line "image-y += $(at91-boot-bin-y)". Could you try to elaborate. Sam ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH v3 06/15] ARM: at91: import lowlevel clock initialization from at91bootstrap
Hi Ahmad. On Mon, Apr 01, 2019 at 12:18:14PM +0200, Ahmad Fatoum wrote: > For use by future at91 first stage bootloaders, this commit imports > https://github.com/linux4sam/at91bootstrap/blob/v3.8.12/driver/pmc.c > > Signed-off-by: Ahmad Fatoum > --- > arch/arm/mach-at91/Makefile | 1 + > arch/arm/mach-at91/at91_pmc_ll.c | 184 ++ > arch/arm/mach-at91/include/mach/at91_pmc.h| 24 ++- > arch/arm/mach-at91/include/mach/at91_pmc_ll.h | 78 > 4 files changed, 284 insertions(+), 3 deletions(-) > create mode 100644 arch/arm/mach-at91/at91_pmc_ll.c > create mode 100644 arch/arm/mach-at91/include/mach/at91_pmc_ll.h > > diff --git a/arch/arm/mach-at91/Makefile b/arch/arm/mach-at91/Makefile > index d81683ac121a..91b06c085107 100644 > --- a/arch/arm/mach-at91/Makefile > +++ b/arch/arm/mach-at91/Makefile > @@ -1,4 +1,5 @@ > obj-y += setup.o > +pbl-y += at91_pmc_ll.o Will this always make my pbl image larger or do we have some linker magic that throws away the unused code? If the pbl image become larger then we need to add this in a conditional manner as we cannot allow this for all at91 variants. Sam ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH v3 05/15] ARM: at91: watchdog: implement at91_wdt_disable
Hi Ahmad. On Mon, Apr 01, 2019 at 12:18:13PM +0200, Ahmad Fatoum wrote: > Low level init code might want to disable the watchdog in PBL. > Provide a helper to do so. > > Signed-off-by: Ahmad Fatoum > --- > arch/arm/mach-at91/include/mach/at91_wdt.h | 16 > 1 file changed, 16 insertions(+) > > diff --git a/arch/arm/mach-at91/include/mach/at91_wdt.h > b/arch/arm/mach-at91/include/mach/at91_wdt.h > index 36d37b9d2d64..d295d35d1b5c 100644 > --- a/arch/arm/mach-at91/include/mach/at91_wdt.h > +++ b/arch/arm/mach-at91/include/mach/at91_wdt.h > @@ -35,4 +35,20 @@ > #define AT91_WDT_WDUNF (1 << 0)/* > Watchdog Underflow */ > #define AT91_WDT_WDERR (1 << 1)/* > Watchdog Error */ > > +#ifndef __ASSEMBLY__ > +// SPDX-License-Identifier: BSD-1-Clause > +/* > + * Copyright (c) 2006, Atmel Corporation > + */ > + > +#include > + > +static inline void at91_wdt_disable(void __iomem *wdt_base) As we are copying a lot of code from at91bootstrap could we then use the same name for this function: driver/at91_wdt.c:void at91_disable_wdt(void) In at91bootstrap the address is a build time constant, but passing it as an value is better. So that part I do not ask you to consider to copy. Sam ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH v3 04/15] ARM: at91: replace at91sam9_ddrsdr.h with at91bootstrap's at91_ddrsdrc.h
Hi Ahmad.
On Mon, Apr 01, 2019 at 12:18:12PM +0200, Ahmad Fatoum wrote:
> Instead of adding missing definitions to the existing at91sam9_ddrsdr.h
> and adapting the incoming DDRAM initialization code from at91bootstrap,
> just replace the lightly used existing header with:
> https://github.com/linux4sam/at91bootstrap/blob/v3.8.12/include/arch/at91_ddrsdrc.h
>
> For easier comprehension, the replacement is done in three steps:
> This last step copies the memory size querying functions from
> at91sam9_ddrsdr.h
> to at91_ddrsdrc.h, then deletes it and fixes all references.
>
> Signed-off-by: Ahmad Fatoum
> ---
> diff --git a/arch/arm/mach-at91/include/mach/at91_ddrsdrc.h
> b/arch/arm/mach-at91/include/mach/at91_ddrsdrc.h
> index 57d0d8f489c4..7e68e7dd63eb 100644
> --- a/arch/arm/mach-at91/include/mach/at91_ddrsdrc.h
> +++ b/arch/arm/mach-at91/include/mach/at91_ddrsdrc.h
> @@ -285,4 +285,48 @@
> #define AT91C_DDRC2_WPVS (0x1UL << 0)
> #define AT91C_DDRC2_WPSRC(0xUL << 8)
>
> +#ifndef __ASSEMBLY__
> +#include
> +#include
> +
> +static inline u32 at91_get_ddram_size(void * __iomem base, bool is_nb)
> +{
I should have brought this up in the previous patch-set, but it continue to
itch me.
So you get the feedback here
We have at91_get_ddram_size(IOMEM(xxx), {true or false})
If we introduced the following small helpers we could simplify the call sites
to just use at91_get_ddram_size(AT91SAM9G45_BASE_DDRSDRC0) and similar.
static inline u32 at91_get_ddram_size(void __force __iomem * addr)
{
return __at91_get_ddram_size(IOMEM(addr), true);
}
static inline u32 at91sam9g45_get_ddram_size(void __force __iomem * addr)
{
return __at91_get_ddram_size(IOMEM(addr), false);
}
But then for the few call sites it is not really worth it anyway.
Anyway, just a small itch. Just ignore it.
Sam
___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH] clk: at91: fix compilation errors in sama5d2.c
Hi Ahmad. Thanks for looking into this. On Mon, Apr 01, 2019 at 12:32:57PM +0200, Ahmad Fatoum wrote: > sama5d2 was added along with the update to the upstream > device tree bindings, but wasn't wired in anywhere. > > To prepare for usage in future sama5d2 support, fix > compilation errors related to absence of locks and > unavailability of audio/i2s clock/pll handling. > > Signed-off-by: Ahmad Fatoum Another approach could have been to stub out the non-existing functions to make diff between the kernel and the barebox variant smaller. Did you consider that approach? The whole section in CONFIG_HAVE_AT91_AUDIO_PLL in dt-compat.c is also something we could drop btw. if we go the "drop all audio" stuff from clk. Also prototypes in pmc.h The patch itself looked fine. But I think that from a maintenance point of view should stub us out of it. Let me know what you think. Sam ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH v2 11/10] clk: at91: fix warning about missing const-safety
On Mon, Apr 01, 2019 at 11:34:27PM +0200, Ahmad Fatoum wrote: > compiling clk-generated.c results in: > > warning: passing argument 1 of 'memcpy' discards 'const' qualifier from > pointer target type [-Wdiscarded-qualifiers] > memcpy(gck->hw.parent_names, parent_names, parents_array_size); > ~~~^ > > Avoid this by replacing the xzalloc+memcpy pair with xmemdup. > Zero-initialization of the buffer isn't necessary, because > memcpy spans the whole buffer. > > Suggested-by: Sam Ravnborg > Signed-off-by: Ahmad Fatoum Reviewed-by: Sam Ravnborg ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: Issues with SPI on iMX7 Sabre Board
Hello Kai, On 4/2/19 3:22 PM, Kai Che wrote: > Hi all, > > I'm trying to communicate with SPI in barebox on the MC iMX7 Sabre Board. > > I added the options in my barebox config: > > CONFIG_SPI=y > CONFIG_CMD_SPI=y How about CONFIG_DRIVER_SPI_IMX? > > But there is no SPI bus when I try to communicate with: > > barebox@Freescale i.MX7 SabreSD Board:/ spi 0x1 > spi bus 0 not found > spi: No such device > > Could someone please tell me if a SPI driver exists for the iMX7 Sabre Board? > > Thanks a lot, > > Kai > ___ > barebox mailing list > barebox@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/barebox > -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Issues with SPI on iMX7 Sabre Board
Hi all, I'm trying to communicate with SPI in barebox on the MC iMX7 Sabre Board. I added the options in my barebox config: CONFIG_SPI=y CONFIG_CMD_SPI=y But there is no SPI bus when I try to communicate with: barebox@Freescale i.MX7 SabreSD Board:/ spi 0x1 spi bus 0 not found spi: No such device Could someone please tell me if a SPI driver exists for the iMX7 Sabre Board? Thanks a lot, Kai ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
