Re: [U-Boot] Antwort: Re: QSPI "sf probe ...", "sf read ..." on Altera SoC FPGA

[Goldschmidt Simon]

Hi Clémént,

> Did you also test the saveenv and sf unlock ?

I did test saveenv and it works. I did not test sf protection.

> Did you get some strange behaviors after a "warm reboot" from linux ?

Indeed, warm reboot fails. When rebooting via "reboot" command from
linux, the last thing I see is SPL writing "Trying to boot from SPI".

I haven't been able to debug this further, yet.

Also, I still can't sf read without disabling the data cache :-(

Regards,
Simon

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

[U-Boot] [PATCH] sf: ensure flash device is in 3-byte address mode

[Goldschmidt Simon]

On some boards where the spi flash is not reset during warm reboot,
the chip has to be manually set into 3-byte address mode.

Signed-off-by: Simon Goldschmidt 
---
 drivers/mtd/spi/sf_internal.h |  2 ++
 drivers/mtd/spi/spi_flash.c   | 53 +++
 2 files changed, 55 insertions(+)

diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
index 839cdbe1b0..06dee0a4ea 100644
--- a/drivers/mtd/spi/sf_internal.h
+++ b/drivers/mtd/spi/sf_internal.h
@@ -62,6 +62,8 @@ enum spi_nor_option_flags {
 #define CMD_READ_STATUS1   0x35
 #define CMD_READ_CONFIG0x35
 #define CMD_FLAG_STATUS0x70
+#define CMD_EN4B   0xB7
+#define CMD_EX4B   0xE9
 
 /* Bank addr access commands */
 #ifdef CONFIG_SPI_FLASH_BAR
diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
index 34f68881ed..8db2882075 100644
--- a/drivers/mtd/spi/spi_flash.c
+++ b/drivers/mtd/spi/spi_flash.c
@@ -165,6 +165,55 @@ bar_end:
 }
 #endif
 
+static int set_4byte(struct spi_flash *flash, const struct spi_flash_info 
*info,
+   u8 enable)
+{
+   int ret;
+   bool need_wren = false;
+   u8 cmd;
+
+   if (flash->size <= SPI_FLASH_16MB_BOUN)
+   return 0;
+
+   switch (JEDEC_MFR(info)) {
+   case SPI_FLASH_CFI_MFR_STMICRO:
+   /* Some Micron need WREN command; all will accept it */
+   need_wren = true;
+   case SPI_FLASH_CFI_MFR_MACRONIX:
+   case SPI_FLASH_CFI_MFR_WINBOND:
+   ret = spi_claim_bus(flash->spi);
+   if (ret) {
+   debug("SF: Unable to claim SPI bus\n");
+   return ret;
+   }
+
+   if (need_wren) {
+   ret = spi_flash_cmd_write_enable(flash);
+   if (ret < 0) {
+   debug("SF: enabling write failed\n");
+   spi_release_bus(flash->spi);
+   return ret;
+   }
+   }
+
+   cmd = enable ? CMD_EN4B : CMD_EX4B;
+   ret = spi_flash_cmd_write(flash->spi, , 1, NULL, 0);
+   if (ret) {
+   debug("SF: fail to %s 4-byte address mode\n",
+   enable ? "enter" : "exit");
+   }
+   if (need_wren)
+   if (spi_flash_cmd_write_disable(flash) < 0)
+   debug("SF: disabling write failed\n");
+   spi_release_bus(flash->spi);
+   return ret;
+   default:
+   /* Spansion style handled by bar_write  */
+   break;
+   }
+   return 0;
+}
+
 #ifdef CONFIG_SF_DUAL_FLASH
 static void spi_flash_dual(struct spi_flash *flash, u32 *addr)
 {
@@ -1086,6 +1135,10 @@ int spi_flash_scan(struct spi_flash *flash)
flash->flags |= SNOR_F_USE_FSR;
 #endif
 
+   /* disable 4-byte addressing if the device exceeds 16MiB */
+   if (flash->size > SPI_FLASH_16MB_BOUN)
+   set_4byte(flash, info, 0);
+
/* Configure the BAR - discover bank cmds and read current bank */
 #ifdef CONFIG_SPI_FLASH_BAR
ret = read_bar(flash, info);
-- 
2.12.2.windows.2

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

[U-Boot] Debugging SPL

[Goldschmidt Simon]

Hi,

When trying to debug SPL on the socfpga platform with CONFIG_OF_EMBED, I get an 
error message "SPL image too big" when linking SPL.
It seems this is because the U-Boot DTB (~16 Kbyte) is not stripped for SPL 
when building this way.

Is this a known issue? Is there a workaround other than manually creating a 
dedicated DTB for debugging SPL in the OF_EMBED mode?

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

Re: [U-Boot] Antwort: Re: QSPI "sf probe ...", "sf read ..." on Altera SoC FPGA

[Goldschmidt Simon]

Hi Clement,

> Did you also test the saveenv and sf unlock ?

Not yet.

> Did you get some strange behaviors after a "warm reboot" from linux ?

Unfortunately, I'm not that far, yet. My Linux image only uses the sd-card for 
now.

Regards,
Simon

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

Re: [U-Boot] Antwort: Re: QSPI "sf probe ...", "sf read ..." on Altera SoC FPGA

[Goldschmidt Simon]

On 09/22/2017 02:20 PM, Clément Péron wrote:
> Sorry these are my local commits you can find them here :
> 
> https://patchwork.ozlabs.org/patch/765992/
> https://patchwork.ozlabs.org/patch/765996/
> https://patchwork.ozlabs.org/patch/765997/
> https://patchwork.ozlabs.org/patch/765998/

Tested on socfpga_cyclone5_socrates by applying these 4 patches to
2017.09. Works as expected.
I had to disable the data cache to actually get the data from qspi to
ram, but that's a totally different issue.

Tested-by: Simon Goldschmidt 

As I'm rather new to this list and project, when can we expect to have
this patch committed?

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

Re: [U-Boot] Antwort: Re: QSPI "sf probe ...", "sf read ..." on Altera SoC FPGA

[Goldschmidt Simon]

On 09/27/2017 06:54 AM, Hannes Schmelzer wrote:
> On 09/22/2017 02:20 PM, Clément Péron wrote:
>> Sorry these are my local commits you can find them here :
>>
>> https://patchwork.ozlabs.org/patch/765992/
>> https://patchwork.ozlabs.org/patch/765996/
>> https://patchwork.ozlabs.org/patch/765997/
>> https://patchwork.ozlabs.org/patch/765998/
> Hi,
> just tested this on my cyclone5 board, but unfortunately without success.
> 
> ---
> U-Boot SPL 2017.09-00354-g824def8 (Sep 27 2017 - 06:47:19)
> ()
> Hit any key to stop autoboot:  0
> => sf probe
> SF: Detected n25q512 with page size 256 Bytes, erase size 64 KiB, total
> 64 MiB
> ### ERROR ### Please RESET the board ###
> 
> 
> same behavior as before.

Have you compared your dts to the other socfpga_cyclone5_*.dts files?

I have tested this on the socrates board and it would also give me the
above error until I change the "compatible" string of the flash chip
from "n25q00" to "spi-flash".

Without that, the sf code seems to try to use the first child node as
flash chip which results in a divide-by-zero error because it passes a
frequency of 0 Hz to the set_speed callback of the qspi driver, which
it is not prepared to handle:

If cadence_qspi_apb_config_baudrate_div is called with sclk_hz == 0,
the DIV_ROUND_UP macro fails to check the denominator for zero.

I wonder where this should be fixed: in the core sf code or in this
driver...

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

Re: [U-Boot] [PATCH v2] spi: kirkwood: avoid divide by zero crash

[Goldschmidt Simon]

On Mon, Nov 13, 2017 at 06:26 AM, Baruch Siach wrote:
>On Sun, Nov 12, 2017 at 08:42:28PM +0100, Heinrich Schuchardt wrote:
>> On 11/12/2017 04:30 PM, Baruch Siach wrote:
>> > Calling .set_speed with zero speed is definitely a bug. Instead of 
>> > crashing, set hz to 1 so that we get the lowest possible frequency rate.
>> 
>> Did this actually occur? Why?
> 
> Yes. CONFIG_ARCH_MVEBU selects CONFIG_DM_SPI_FLASH. In 
> cmd/sf.c:do_spi_flash_probe(),
> speed is set to 0 when CONFIG_DM_SPI_FLASH, passed to 
> spi_flash_probe_bus_cs(),
> and from there to spi_get_bus_and_cs(). 
> Unfortunately, since the ClearFog SPI flash node has no "spi-flash" compatible
> string, the spi_flash_std driver is not bound, so spi_child_post_bind() 
> returns
> early without calling spi_slave_ofdata_to_platdata(). The 
> dm_spi_slave_platdata
> speed field is left uninitialized, and we end up with
> speed=0 when calling spi_set_speed_mode().
> 
> This should be fixed with http://patchwork.ozlabs.org/patch/837360/ for 
> ClearFog,
> by adding the "spi-flash" compatible. But "spi-flash" is non standard. Most 
> kernel
> DT files use "jedec,spi-nor" instead. So you can expect this issue to show up
> again in the future.

This is exactly what I just got with the cadence-spi driver when starting my
mach-socfpga board with the device tree working for linux.

I also 'fixed' it by changing the dts, but I'd rather use the same dts for 
linux and U-Boot.

Should this be fixed in the core code or is it a bug of each driver not 
detecting the '0'?

> A workaround is to provide speed argument in the 'sf probe' command line.

This is not a valid workaround for me: the spi-nor is my boot device and I got
this error even in SPL when loading U-Boot. Not using 'sf probe'.


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

[U-Boot] [PATCH] rockchip: doc: update U-Boot location info

[Goldschmidt Simon]

The U-Boot location has been moved to block 16384.
This is 8MB, not 4MB.

Signed-off-by: Simon Goldschmidt 
---
 doc/README.rockchip | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/README.rockchip b/doc/README.rockchip
index 9d5af3d53d..a7761af3e8 100644
--- a/doc/README.rockchip
+++ b/doc/README.rockchip
@@ -102,7 +102,7 @@ To write an image that boots from an SD card (assumed to be 
/dev/sdc):
sudo dd if=firefly-rk3288/u-boot-dtb.img of=/dev/sdc seek=16384
 
 This puts the Rockchip header and SPL image first and then places the U-Boot
-image at block 16384 (i.e. 4MB from the start of the SD card). This
+image at block 16384 (i.e. 8MB from the start of the SD card). This
 corresponds with this setting in U-Boot:
 
#define CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR 0x4000
-- 
2.12.2.windows.2

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

Re: [U-Boot] [U-Boot, 2/3] rockchip: doc: update U-Boot location info

[Goldschmidt Simon]

>> I just found this commit has calculated the size wrong. 16384 blocks should 
>> be 8MB, not 4MB.
> 
> Yes, 8MB is what expected here and even Kever[1] commented the same, what is 
> 4MB here? can you elaborate.

I know. It's only wrong in 'doc/README.rockchip' at line 105 where it says:

"image at block 16384 (i.e. 4MB from the start of the SD card)"


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

Re: [U-Boot] [PATCH] spl: make CONFIG_OF_EMBED pass dts through fdtgrep

[Goldschmidt Simon]

Hi Lukasz

> > Building spl with CONFIG_OF_EMBED enabled results in an error message
>   ^^^ - is the CONFIG_OF_EMBED a standard
>   feature for SPL?

Well, it's not really a standard feature I want to use in my final
product. But if I want to debug SPL or U-Boot on my socfpga (using
ARM's DS-5 for Altera), I need the device tree to be included in the
binary that the debugger downloads to the target.

That's what CONFIG_OF_EMBED is for, right? I even need this to debug
U-Boot, as the standard procedure for my target is to first start SPL
via the debugger, halt at a specific hardware breakpoint and the load
U-Boot which I want to debug.

> > on my board: "SPL image too big". This is because the fdtgrep build
> > step is only executed for CONFIG_OF_SEPARATE.
> 
> So the problem is with not enough runs of dtb stripping?

No, the problem that the dtb is not stripped at all for CONFIG_OF_EMBED.
The only thing that is stripped is 'spl/u-boot-spl.dtb' which is only
created for CONFIG_OF_SEPARATE.

> 
> >
> > Fix this by moving the fdtgrep build step ('cmd_fdtgreo') from
> > scripts/Makefile.spl to dts/Makefile so that the reduced dtb is
> > available for all kinds of spl builds.
> 
> Ok.
> 
> >
> > The resulting variable name for the embedded device tree blob changes,
> > too, which is why common.h and fdtdec.c have tiny changes.
> >
> > Signed-off-by: Simon Goldschmidt 
> > ---
> >  dts/Makefile | 35 +++
> >  include/common.h |  1 +
> >  lib/fdtdec.c |  4 
> >  scripts/Makefile.spl | 20 ++--
> >  4 files changed, 38 insertions(+), 22 deletions(-)
> >
> > diff --git a/dts/Makefile b/dts/Makefile index 3a93dafb51..c9b2a89441
> > 100644
> > --- a/dts/Makefile
> > +++ b/dts/Makefile
> > @@ -22,10 +22,29 @@ DTB := $(ARCH_PATH)/$(DEVICE_TREE).dtb
> > dtb_depends += $(DTB:.dtb=.dts)  endif
> >
> > +# Pass the original device tree file through fdtgrep twice. The
> > first pass +# removes any unwanted nodes (i.e. those which don't have
> > the +# 'u-boot,dm-pre-reloc' property and thus are not needed by SPL.
> > The second +# pass removes various unused properties from the
> > remaining nodes. +# The output is typically a much smaller device tree
> > file. +ifeq ($(CONFIG_TPL_BUILD),y)
> > +fdtgrep_props := -b u-boot,dm-pre-reloc -b u-boot,dm-tpl else
> > +fdtgrep_props := -b u-boot,dm-pre-reloc -b u-boot,dm-spl endif
> > +quiet_cmd_fdtgrep = FDTGREP $@
> > +  cmd_fdtgrep = $(objtree)/tools/fdtgrep $(fdtgrep_props) -RT $<
> > \
> > +   -n /chosen -n /config -O dtb | \
> > +   $(objtree)/tools/fdtgrep -r -O dtb - -o $@ \
> > +   $(addprefix -P ,$(subst
> > $\",,$(CONFIG_OF_SPL_REMOVE_PROPS))) +
> > +$(obj)/dt-spl.dtb: $(DTB) $(objtree)/tools/fdtgrep FORCE
> > +   $(call if_changed,fdtgrep)
> > +
> >  $(obj)/dt.dtb: $(DTB) FORCE
> > $(call if_changed,shipped)
> >
> > -targets += dt.dtb
> > +targets += dt.dtb dt-spl.dtb
> >
> >  $(DTB): $(dtb_depends)
> >  ifeq ($(EXT_DTB),)
> > @@ -42,14 +61,22 @@ endif
> >  arch-dtbs:
> > $(Q)$(MAKE) $(build)=$(ARCH_PATH) dtbs
> >
> > -.SECONDARY: $(obj)/dt.dtb.S
> > +.SECONDARY: $(obj)/dt.dtb.S $(obj)/dt-spl.dtb.S
> >
> > +
> > +ifeq ($(CONFIG_SPL_BUILD),y)
> > +obj-$(CONFIG_OF_EMBED) := dt-spl.dtb.o # support "out-of-tree" build
> > +for dtb-spl
> > +$(obj)/dt-spl.dtb.o: $(obj)/dt-spl.dtb.S FORCE
> > +   $(call if_changed_dep,as_o_S)
> > +else
> >  obj-$(CONFIG_OF_EMBED) := dt.dtb.o
> > +endif
> >
> > -dtbs: $(obj)/dt.dtb
> > +dtbs: $(obj)/dt.dtb $(obj)/dt-spl.dtb
> > @:
> >
> > -clean-files := dt.dtb.S
> > +clean-files := dt.dtb.S dt-spl.dtb.S
> >
> >  # Let clean descend into dts directories
> >  subdir-
> > += ../arch/arm/dts ../arch/microblaze/dts ../arch/mips/dts
> > +../arch/sandbox/dts ../arch/x86/dts
> > diff --git a/include/common.h b/include/common.h index
> > e14e1daa88..6e24545178 100644 --- a/include/common.h
> > +++ b/include/common.h
> > @@ -201,6 +201,7 @@ int last_stage_init(void);  extern ulong
> > monitor_flash_len;  int mac_read_from_eeprom(void);
> >  extern u8 __dtb_dt_begin[];/* embedded device tree blob */
> > +extern u8 __dtb_dt_spl_begin[];/* embedded device tree blob
> > for SPL/TPL */ int set_cpu_clk_info(void);  int mdm_init(void);  int
> > print_cpuinfo(void); diff --git a/lib/fdtdec.c b/lib/fdtdec.c index
> > 45f3fe7baf..0eb0b92261 100644
> > --- a/lib/fdtdec.c
> > +++ b/lib/fdtdec.c
> > @@ -1266,7 +1266,11 @@ int fdtdec_setup(void)  # endif  # ifdef
> > CONFIG_OF_EMBED
> > /* Get a pointer to the FDT */
> > +#  ifdef CONFIG_SPL_BUILD
> > +   gd->fdt_blob = __dtb_dt_spl_begin;
> > +#  else
> > gd->fdt_blob = __dtb_dt_begin;
> > +#  endif
> >  # elif defined CONFIG_OF_SEPARATE
> >  #  ifdef CONFIG_SPL_BUILD
> > /* FDT is at end of BSS unless it is in a different memory region */
> > diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl index
> > 

Re: [U-Boot] [PATCH] fpga: allow programming fpga from FIT image for all FPGA drivers

[Goldschmidt Simon]

On 28.11.2017 14:46, Michal Simek wrote:
> On 28.11.2017 10:08, Goldschmidt Simon wrote:
>> Simon Goldschmidt wrote:
>>> Hi Simon,
>>>
>>> Simon Glass wrote:
>>>> I see that, although it is adding to the fpga header so presumably
>>>> making it harder for someone to move this over.
>>>
>>> Yes, I'm not happy with changing the header and even xilinx C file to
>>> add functionality for altera. However, this is due to the fact that a
>>> core file still depends on an dogs implementation, which I removed.
>>
>> This should have meant "on an fpga implementation". No dogs or offences here.
>> My mobile phone does not know the word "fpga", obviously.
>>
>>>
>>>> Does anyone on cc know the plan fr conversion of FPGA to driver model?
>>>> It does not look too tricky from a quick look at the header file.
>>>
>>> It does not look tricky, indeed. However, we should try to match this
>>> to the Linux implementation regarding what's needed in the device
>>> tree. And I just don't know that part, yet ;-)
> 
> I have not a problem to enable others fpgas to use this functionality.
> And transition should be done for all these drivers when someone has a time 
> to do
> it.

Great, thanks! So via which tree would this be included?

Regards,
Simon

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

Re: [U-Boot] [PATCH] sf: ensure flash device is in 3-byte address mode

[Goldschmidt Simon]

Hi Jagan,

On Fri, Nov 10, 2017 08:04, Jagan Teki wrote:
>>> I've similar change on my patchwork, since no-one tested Will CC you by re-
> basing it please have test?
>>
>> Yes, of course I'd like to test this. Where do I find your patch?
> 
> Will rebase and send to ML soon.

Any progress here? Any chance that this one and the other fixes
needed for cadence_qspi to work correctly get included in 2018.01?

The following patches would be required for this in addition to the
3-byte mode switch you wanted to submit:

"spi: cadence_spi: Adopt Linux DT bindings":
https://patchwork.ozlabs.org/project/uboot/list/?series=13864

Revert "spi: cadence_qspi_apb: Use 32 bit indirect read transaction
when possible" (this one goes on top of the above)
https://patchwork.ozlabs.org/patch/838871/

BTW: the series "spi: cadence_spi_apb: fix using bouncebuf with
writeback dcache" can be closed when the reverting patch above is
applied.

I already sent reviewed-by and tested-by for the first series
above but I can't see them in patchwork.

Please let me know if there's anything missing or anything I can
do to get this pushed.

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

Re: [U-Boot] [PATCH] fpga: allow programming fpga from FIT image for all FPGA drivers

[Goldschmidt Simon]

Simon Goldschmidt wrote:
> Hi Simon,
> 
> Simon Glass wrote:
> > I see that, although it is adding to the fpga header so presumably
> > making it harder for someone to move this over.
> 
> Yes, I'm not happy with changing the header and even xilinx C file to add
> functionality for altera. However, this is due to the fact that a core file 
> still depends
> on an dogs implementation, which I removed.

This should have meant "on an fpga implementation". No dogs or offences here.
My mobile phone does not know the word "fpga", obviously.

> 
> > Does anyone on cc know the plan fr conversion of FPGA to driver model?
> > It does not look too tricky from a quick look at the header file.
> 
> It does not look tricky, indeed. However, we should try to match this to the 
> Linux
> implementation regarding what's needed in the device tree. And I just don't 
> know
> that part, yet ;-)
> 


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

Re: [U-Boot] [PATCH] fpga: allow programming fpga from FIT image for all FPGA drivers

[Goldschmidt Simon]

Hi Simon,

Simon Glass wrote:
> I see that, although it is adding to the fpga header so presumably
> making it harder for someone to move this over.

Yes, I'm not happy with changing the header and even xilinx C file to add 
functionality for altera. However, this is due to the fact that a core file 
still depends on an dogs implementation, which I removed.

> Does anyone on cc know the plan fr conversion of FPGA to driver model?
> It does not look too tricky from a quick look at the header file.

It does not look tricky, indeed. However, we should try to match this to the 
Linux implementation regarding what's needed in the device tree. And I just 
don't know that part, yet ;-)

Regards,
Simon

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

Re: [U-Boot] [PATCH] fpga: allow programming fpga from FIT image for all FPGA drivers

[Goldschmidt Simon]

Hi,

On 4.12.2017 15:27, Michal Simek wrote:
> [..]
> ok. Then applied to my xilinx tree.

Great to hear that, thanks!

Which release will this be in? Does it fit into 2018.01 or has that
window already closed?

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

Re: [U-Boot] [PATCH] sf: ensure flash device is in 3-byte address mode

[Goldschmidt Simon]

+ Lukasz (as a reviewer of my patch[1])

On Mon, Dec 4, 2017 at 8:20, Jagan Teki wrote:
> This is the patch[1] for 4-byte addressing, but I would wonder how can proceed
> operations with 4-byte if we disable during probe.
> 
> [1] http://git.denx.de/?p=u-boot-
> spi.git;a=commitdiff;h=fd0c22a90772379c4c11ba09347d36cc8ee17dca

OK, so your patch does something different than what I did.

I was trying to keep the change to U-Boot as small as possible, only
fixing this issue I was seeing:

After a soft-reboot where the SPI chip was not reset, it is left in
4-byte addressing mode (linux uses this mode, obviously). Remember
that 4-byte mode is not a permanent setting, so we can enter and
leave it any time we like by issuing a command.

U-Boot uses the Bank Address Register (BAR) for spi flash chips with
more than 16 MByte, so it impclitly assumes that the chip is in
3-byte address mode. As I see it, your patch is worth a discussion
named "should we use 4-byte addressing mode on spi flash chips?".
I do think this is a better alternative than writing BAR! But this
change probably needs discussion and testing.

Until we discussed and tested that, could we push my patch[1] into
v2018.01? This is really a rather tiny bugfix I need for soft reboot,
compared to using 4-byte address mode.

[1] https://patchwork.ozlabs.org/patch/826919/

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

Re: [U-Boot] [PATCH] fpga: allow programming fpga from FIT image for all FPGA drivers

[Goldschmidt Simon]

On 5.12.2017 08:22, Michal Simek wrote:
> > Which release will this be in? Does it fit into 2018.01 or has that
> > window already closed?
> 
> I believe so.

Sorry to bother you again, I'm just not sure I understood your answer.

Was that "I beleive so" meant as "yes, 2018.01 unless someone rejects
it"? Because Tom just wrote patches already on the ML were OK for that
release?

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

[U-Boot] [PATCH] Revert "spi: cadence_qspi_apb: Use 32 bit indirect read transaction when possible"

[Goldschmidt Simon]

This reverts commit b63b46313ed29e9b0c36b3d6b9407f6eade40c8f.

This commit changed cadence_qspi_apb to use bouncebuf.c, which invalidates
the data cache after reading. This is meant for dma transfers only and
breaks the cadence_qspi driver which copies via cpu only: data that is
copied by the cpu is in cache only and the cache invalidation at the end
throws away this data.

Signed-off-by: Simon Goldschmidt 
---
 drivers/spi/cadence_qspi_apb.c | 22 ++
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index e02f2217f4..5d5b6f0d35 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -634,8 +634,6 @@ int cadence_qspi_apb_indirect_read_execute(struct 
cadence_spi_platdata *plat,
 {
unsigned int remaining = n_rx;
unsigned int bytes_to_read = 0;
-   struct bounce_buffer bb;
-   u8 *bb_rxbuf;
int ret;
 
writel(n_rx, plat->regbase + CQSPI_REG_INDIRECTRDBYTES);
@@ -644,11 +642,6 @@ int cadence_qspi_apb_indirect_read_execute(struct 
cadence_spi_platdata *plat,
writel(CQSPI_REG_INDIRECTRD_START,
   plat->regbase + CQSPI_REG_INDIRECTRD);
 
-   ret = bounce_buffer_start(, (void *)rxbuf, n_rx, GEN_BB_WRITE);
-   if (ret)
-   return ret;
-   bb_rxbuf = bb.bounce_buffer;
-
while (remaining > 0) {
ret = cadence_qspi_wait_for_data(plat);
if (ret < 0) {
@@ -662,13 +655,12 @@ int cadence_qspi_apb_indirect_read_execute(struct 
cadence_spi_platdata *plat,
bytes_to_read *= CQSPI_FIFO_WIDTH;
bytes_to_read = bytes_to_read > remaining ?
remaining : bytes_to_read;
-   readsl(plat->ahbbase, bb_rxbuf, bytes_to_read >> 2);
-   if (bytes_to_read % 4)
-   readsb(plat->ahbbase,
-  bb_rxbuf + rounddown(bytes_to_read, 4),
-  bytes_to_read % 4);
-
-   bb_rxbuf += bytes_to_read;
+   /* Handle non-4-byte aligned access to avoid data 
abort. */
+   if (((uintptr_t)rxbuf % 4) || (bytes_to_read % 4))
+   readsb(plat->ahbbase, rxbuf, bytes_to_read);
+   else
+   readsl(plat->ahbbase, rxbuf, bytes_to_read >> 
2);
+   rxbuf += bytes_to_read;
remaining -= bytes_to_read;
bytes_to_read = cadence_qspi_get_rd_sram_level(plat);
}
@@ -685,7 +677,6 @@ int cadence_qspi_apb_indirect_read_execute(struct 
cadence_spi_platdata *plat,
/* Clear indirect completion status */
writel(CQSPI_REG_INDIRECTRD_DONE,
   plat->regbase + CQSPI_REG_INDIRECTRD);
-   bounce_buffer_stop();
 
return 0;
 
@@ -693,7 +684,6 @@ failrd:
/* Cancel the indirect read */
writel(CQSPI_REG_INDIRECTRD_CANCEL,
   plat->regbase + CQSPI_REG_INDIRECTRD);
-   bounce_buffer_stop();
return ret;
 }
 
-- 
2.12.2.windows.2

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

[U-Boot] Working with patchwork

[Goldschmidt Simon]

Hi,

as I'm quite new to U-Boot and its patch workflow: what do I have to do to send 
a reviewed-by or tested-by tag for a patch? I sent it as a reply to this list 
but I can't see them in patchwork.

Did I miss something here? Or could it be a problem in the email format used?

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

[U-Boot] [PATCH] fpga: allow programming fpga from FIT image for all FPGA drivers

[Goldschmidt Simon]

This drops the limit that fpga is only loaded from FIT images for Xilinx.
This is done by moving the 'partial' check from 'common/image.c' to
'drivers/fpga/xilinx.c' (the only driver supporting partial images yet)
and supplies a weak default implementation in 'drivers/fpga/fpga.c'.

Signed-off-by: Simon Goldschmidt 
---
 common/bootm.c|  2 +-
 common/image.c|  6 ++
 drivers/fpga/fpga.c   |  9 +
 drivers/fpga/xilinx.c | 13 +
 include/fpga.h|  1 +
 5 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/common/bootm.c b/common/bootm.c
index 9493a306cd..adb12137c7 100644
--- a/common/bootm.c
+++ b/common/bootm.c
@@ -248,7 +248,7 @@ int bootm_find_images(int flag, int argc, char * const 
argv[])
 #endif
 
 #if IMAGE_ENABLE_FIT
-#if defined(CONFIG_FPGA) && defined(CONFIG_FPGA_XILINX)
+#if defined(CONFIG_FPGA)
/* find bitstreams */
ret = boot_get_fpga(argc, argv, , IH_ARCH_DEFAULT,
NULL, NULL);
diff --git a/common/image.c b/common/image.c
index 06fdca129c..9fd72b9423 100644
--- a/common/image.c
+++ b/common/image.c
@@ -1223,7 +1223,7 @@ int boot_get_setup(bootm_headers_t *images, uint8_t arch,
 }
 
 #if IMAGE_ENABLE_FIT
-#if defined(CONFIG_FPGA) && defined(CONFIG_FPGA_XILINX)
+#if defined(CONFIG_FPGA)
 int boot_get_fpga(int argc, char * const argv[], bootm_headers_t *images,
  uint8_t arch, const ulong *ld_start, ulong * const ld_len)
 {
@@ -1234,8 +1234,6 @@ int boot_get_fpga(int argc, char * const argv[], 
bootm_headers_t *images,
const char *uname, *name;
int err;
int devnum = 0; /* TODO support multi fpga platforms */
-   const fpga_desc * const desc = fpga_get_desc(devnum);
-   xilinx_desc *desc_xilinx = desc->devdesc;
 
/* Check to see if the images struct has a FIT configuration */
if (!genimg_has_config(images)) {
@@ -1280,7 +1278,7 @@ int boot_get_fpga(int argc, char * const argv[], 
bootm_headers_t *images,
return fit_img_result;
}
 
-   if (img_len >= desc_xilinx->size) {
+   if (!fpga_is_partial_data(devnum, img_len)) {
name = "full";
err = fpga_loadbitstream(devnum, (char *)img_data,
 img_len, BIT_FULL);
diff --git a/drivers/fpga/fpga.c b/drivers/fpga/fpga.c
index e0fb1b4e78..6aead27f16 100644
--- a/drivers/fpga/fpga.c
+++ b/drivers/fpga/fpga.c
@@ -171,6 +171,15 @@ int fpga_add(fpga_type devtype, void *desc)
 }
 
 /*
+ * Return 1 if the fpga data is partial.
+ * This is only required for fpga drivers that support bitstream_type.
+ */
+int __weak fpga_is_partial_data(int devnum, size_t img_len)
+{
+   return 0;
+}
+
+/*
  * Convert bitstream data and load into the fpga
  */
 int __weak fpga_loadbitstream(int devnum, char *fpgadata, size_t size,
diff --git a/drivers/fpga/xilinx.c b/drivers/fpga/xilinx.c
index 941f30010a..3c05760969 100644
--- a/drivers/fpga/xilinx.c
+++ b/drivers/fpga/xilinx.c
@@ -24,6 +24,19 @@ static int xilinx_validate(xilinx_desc *desc, char *fn);
 
 /* - */
 
+int fpga_is_partial_data(int devnum, size_t img_len)
+{
+   const fpga_desc * const desc = fpga_get_desc(devnum);
+   xilinx_desc *desc_xilinx = desc->devdesc;
+
+   /* Check datasize against FPGA size */
+   if (img_len >= desc_xilinx->size)
+   return 0;
+
+   /* datasize is smaller, must be partial data */
+   return 1;
+}
+
 int fpga_loadbitstream(int devnum, char *fpgadata, size_t size,
   bitstream_type bstype)
 {
diff --git a/include/fpga.h b/include/fpga.h
index d768fb1417..4d6da790b7 100644
--- a/include/fpga.h
+++ b/include/fpga.h
@@ -54,6 +54,7 @@ void fpga_init(void);
 int fpga_add(fpga_type devtype, void *desc);
 int fpga_count(void);
 const fpga_desc *const fpga_get_desc(int devnum);
+int fpga_is_partial_data(int devnum, size_t img_len);
 int fpga_load(int devnum, const void *buf, size_t bsize,
  bitstream_type bstype);
 int fpga_fsload(int devnum, const void *buf, size_t size,
-- 
2.12.2.windows.2

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

Re: [U-Boot] Working with patchwork

[Goldschmidt Simon]

Hi Lukasz,

thanks for taking the time :-)

> > as I'm quite new to U-Boot and its patch workflow: what do I have to
> > do to send a reviewed-by or tested-by tag for a patch? I sent it as a
> > reply to this list but I can't see them in patchwork.
> >
> > Did I miss something here? Or could it be a problem in the email
> > format used?
> 
> I can confirm that when I send reviewed-by/acked-by, those are added to
> patchwork.
> 
> Which mailing program do you use?

Unfortunately, I'm using Outlook. It already took some time with our
IT department to get it to send raw text messages. But that seemed to
have worked, I thought (at least I could register to a kernel mailing
list with majordomo).

However, I can't really access the plain text of my own mails sent by
Outlook, so I can only check what was sent by searching online list
archives and at least this one has "anonymized" my email address:

https://www.mail-archive.com/u-boot@lists.denx.de/msg269392.html

I'm still glad to hear anything that could help me :-)

Best regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH] fpga: allow programming fpga from FIT image for all FPGA drivers

[Goldschmidt Simon]

Hi,

> Simon Glass wrote:
> On 10 November 2017 at 07:17, Goldschmidt Simon <sgoldschmidt@de.pepperl-
> fuchs.com> wrote:
> > This drops the limit that fpga is only loaded from FIT images for Xilinx.
> > This is done by moving the 'partial' check from 'common/image.c' to
> > 'drivers/fpga/xilinx.c' (the only driver supporting partial images
> > yet) and supplies a weak default implementation in 'drivers/fpga/fpga.c'.
> >
> > Signed-off-by: Simon Goldschmidt <sgoldschm...@de.pepperl-fuchs.com>
> > ---
> >  common/bootm.c|  2 +-
> >  common/image.c|  6 ++
> >  drivers/fpga/fpga.c   |  9 +
> >  drivers/fpga/xilinx.c | 13 +
> >  include/fpga.h|  1 +
> >  5 files changed, 26 insertions(+), 5 deletions(-)
> >
> 
> I think the FPGA subsystem should move to driver model.

I can understand the need for that. However, I don't have the expertise
to do so, I guess. Also, I would have thought this requirement would be
raised to someone actually adding/changing fpga code (like the recent
Intel activities on this list).

In contrast to this, I see my patch more like cleaning the code, moving
Xilinx dependent code from 'common/image.c' to 'drivers/fpga/xilinx.c'.
This cleanup in the common directory is rather independent of migrating
the fpga subsystem to driver model.


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

[U-Boot] [PATCH] spl: make CONFIG_OF_EMBED pass dts through fdtgrep

[Goldschmidt Simon]

Building spl with CONFIG_OF_EMBED enabled results in an error message
on my board: "SPL image too big". This is because the fdtgrep build
step is only executed for CONFIG_OF_SEPARATE.

Fix this by moving the fdtgrep build step ('cmd_fdtgreo') from
scripts/Makefile.spl to dts/Makefile so that the reduced dtb is
available for all kinds of spl builds.

The resulting variable name for the embedded device tree blob changes,
too, which is why common.h and fdtdec.c have tiny changes.

Signed-off-by: Simon Goldschmidt 
---
 dts/Makefile | 35 +++
 include/common.h |  1 +
 lib/fdtdec.c |  4 
 scripts/Makefile.spl | 20 ++--
 4 files changed, 38 insertions(+), 22 deletions(-)

diff --git a/dts/Makefile b/dts/Makefile
index 3a93dafb51..c9b2a89441 100644
--- a/dts/Makefile
+++ b/dts/Makefile
@@ -22,10 +22,29 @@ DTB := $(ARCH_PATH)/$(DEVICE_TREE).dtb
 dtb_depends += $(DTB:.dtb=.dts)
 endif
 
+# Pass the original device tree file through fdtgrep twice. The first pass
+# removes any unwanted nodes (i.e. those which don't have the
+# 'u-boot,dm-pre-reloc' property and thus are not needed by SPL. The second
+# pass removes various unused properties from the remaining nodes.
+# The output is typically a much smaller device tree file.
+ifeq ($(CONFIG_TPL_BUILD),y)
+fdtgrep_props := -b u-boot,dm-pre-reloc -b u-boot,dm-tpl
+else
+fdtgrep_props := -b u-boot,dm-pre-reloc -b u-boot,dm-spl
+endif
+quiet_cmd_fdtgrep = FDTGREP $@
+  cmd_fdtgrep = $(objtree)/tools/fdtgrep $(fdtgrep_props) -RT $< \
+   -n /chosen -n /config -O dtb | \
+   $(objtree)/tools/fdtgrep -r -O dtb - -o $@ \
+   $(addprefix -P ,$(subst $\",,$(CONFIG_OF_SPL_REMOVE_PROPS)))
+
+$(obj)/dt-spl.dtb: $(DTB) $(objtree)/tools/fdtgrep FORCE
+   $(call if_changed,fdtgrep)
+
 $(obj)/dt.dtb: $(DTB) FORCE
$(call if_changed,shipped)
 
-targets += dt.dtb
+targets += dt.dtb dt-spl.dtb
 
 $(DTB): $(dtb_depends)
 ifeq ($(EXT_DTB),)
@@ -42,14 +61,22 @@ endif
 arch-dtbs:
$(Q)$(MAKE) $(build)=$(ARCH_PATH) dtbs
 
-.SECONDARY: $(obj)/dt.dtb.S
+.SECONDARY: $(obj)/dt.dtb.S $(obj)/dt-spl.dtb.S
 
+
+ifeq ($(CONFIG_SPL_BUILD),y)
+obj-$(CONFIG_OF_EMBED) := dt-spl.dtb.o
+# support "out-of-tree" build for dtb-spl
+$(obj)/dt-spl.dtb.o: $(obj)/dt-spl.dtb.S FORCE
+   $(call if_changed_dep,as_o_S)
+else
 obj-$(CONFIG_OF_EMBED) := dt.dtb.o
+endif
 
-dtbs: $(obj)/dt.dtb
+dtbs: $(obj)/dt.dtb $(obj)/dt-spl.dtb
@:
 
-clean-files := dt.dtb.S
+clean-files := dt.dtb.S dt-spl.dtb.S
 
 # Let clean descend into dts directories
 subdir- += ../arch/arm/dts ../arch/microblaze/dts ../arch/mips/dts 
../arch/sandbox/dts ../arch/x86/dts
diff --git a/include/common.h b/include/common.h
index e14e1daa88..6e24545178 100644
--- a/include/common.h
+++ b/include/common.h
@@ -201,6 +201,7 @@ int last_stage_init(void);
 extern ulong monitor_flash_len;
 int mac_read_from_eeprom(void);
 extern u8 __dtb_dt_begin[];/* embedded device tree blob */
+extern u8 __dtb_dt_spl_begin[];/* embedded device tree blob for 
SPL/TPL */
 int set_cpu_clk_info(void);
 int mdm_init(void);
 int print_cpuinfo(void);
diff --git a/lib/fdtdec.c b/lib/fdtdec.c
index 45f3fe7baf..0eb0b92261 100644
--- a/lib/fdtdec.c
+++ b/lib/fdtdec.c
@@ -1266,7 +1266,11 @@ int fdtdec_setup(void)
 # endif
 # ifdef CONFIG_OF_EMBED
/* Get a pointer to the FDT */
+#  ifdef CONFIG_SPL_BUILD
+   gd->fdt_blob = __dtb_dt_spl_begin;
+#  else
gd->fdt_blob = __dtb_dt_begin;
+#  endif
 # elif defined CONFIG_OF_SEPARATE
 #  ifdef CONFIG_SPL_BUILD
/* FDT is at end of BSS unless it is in a different memory region */
diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
index 49b27ac926..d3c176d775 100644
--- a/scripts/Makefile.spl
+++ b/scripts/Makefile.spl
@@ -238,24 +238,8 @@ $(obj)/$(SPL_BIN)-pad.bin: $(obj)/$(SPL_BIN)
@bss_size_str=$(shell $(NM) $< | awk 'BEGIN {size = 0} /__bss_size/ 
{size = $$1} END {print "ibase=16; " toupper(size)}' | bc); \
dd if=/dev/zero of=$@ bs=1 count=$${bss_size_str} 2>/dev/null;
 
-# Pass the original device tree file through fdtgrep twice. The first pass
-# removes any unwanted nodes (i.e. those which don't have the
-# 'u-boot,dm-pre-reloc' property and thus are not needed by SPL. The second
-# pass removes various unused properties from the remaining nodes.
-# The output is typically a much smaller device tree file.
-ifeq ($(CONFIG_TPL_BUILD),y)
-fdtgrep_props := -b u-boot,dm-pre-reloc -b u-boot,dm-tpl
-else
-fdtgrep_props := -b u-boot,dm-pre-reloc -b u-boot,dm-spl
-endif
-quiet_cmd_fdtgrep = FDTGREP $@
-  cmd_fdtgrep = $(objtree)/tools/fdtgrep $(fdtgrep_props) -RT $< \
-   -n /chosen -n /config -O dtb | \
-   $(objtree)/tools/fdtgrep -r -O dtb - -o $@ \
-   $(addprefix -P ,$(subst $\",,$(CONFIG_OF_SPL_REMOVE_PROPS)))
-
-$(obj)/$(SPL_BIN).dtb: dts/dt.dtb $(objtree)/tools/fdtgrep FORCE
-

Re: [U-Boot] [PATCH 1/2] common: make bouncebuf work for non-DMA transfers

[Goldschmidt Simon]

Hi Vignesh,

Vignesh R wrote:
> [..]
> Its not actually unaligned access, cadence QSPI IP on TI platforms do
> not support non-byte accesses except for the last word. As per the TRM:
> "The external master is only permitted to issue 32-bit data interface
> writes until the last word of an indirect
> transfer"

OK, I thought it was unaligned access because of reading your commit
message talking about data aborts. But this was in the write part
indeed.

>> So given my explanation above, what's the preferred way to fix this?
> 
> Sorry, I overlooked the fact that bounce_buffer_stop() is calling
> invalidate_dcache_range(). Somehow, this did not show problems on my
> platform although its a writeback cache.
>
> I would suggest to revert commit b63b46313 ("spi: cadence_qspi_apb: Use
> 32 bit indirect read transaction when possible"). I have seen that non
> 32 bit accesses cause problems only while writing to flash but not
> during read operations although the TRM states that both reads and
> writes are affected.
> But, since reverting b63b46313 as such does not break TI platforms, I
> would prefer sending a revert.
> Meanwhile, I will work on a better patch later.

I have not actually tested writing with dcache enabled, but from
reading the code, it should not be a problem. I'll test that then.

> Lets not touch bouncebuf for now.

OK. I take it that bouncebuf should be only used for dma transfers.
In that case, shouldn't we revert the write patch, too, eventually?
Even if it does not break data consistency like in the read direction,
messing around with the cache while doing cpu transfers at the very
least will drop performance. Plus it seems like a bad example.

We can do that later, of course, as it should work like it is now.

>> I need this driver fixed, so whatever way will be accepted is fine by
>> me, I guess. Just let me know.
>> 
>
> Sorry for the trouble. I am okay with reverting the patch affecting read
> path.

OK. I'll send a patch hat reverts the read path then.

Thank you for your help!

Regards,
Simon

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

Re: [U-Boot] [PATCH 1/2] common: make bouncebuf work for non-DMA transfers

[Goldschmidt Simon]

Simon Goldschmidt wrote:
>Marek Vasut wrote:
>> So what alignment problems do you observe ? If you copy using the CPU
>> only, why do you need the bounce buffer at all ? I don't quite get it.
> 
> Sorry for not explaining it good enough:
> I don't observe any alignment problems. mach-socfpga can do unaligned
> accesses as well. This driver does CPU based copy, but since it transfers
> words, not bytes, I guess on other platforms, the CPU might fail when trying
> to read a word from an unaligned pointer.
>
> Vignesh added this about a year ago and from that commit message, it
> seems like he was observing alignment errors on his TI platform.
> Also, those commits have a reviewed-by tag from you and Jagan.
>
> For me, reverting these patches would be OK as well, but I guess Vinesh's
> TI platform kind of needs them?

So to clarify it again, the cadence_spi driver has to transfer 32-bit
words to/from the hardware and the driver uses 'readsl' and 'writesl'
to do this. Now if the source/destination buffer is not aligned, this
results in an error on platforms that do not support unaligned access.

We have three options here:
a) fix all call stacks calling dm_spi_ops->xfer to use aligned buffers
   (which could be hard when looking at 'sf read')
b) fix the driver by doing malloc + memcpy or loading byte by byte from
   ram
c) add framework functions doing malloc + memcpy (which is nearly the
   same as bouncebuf)

This 32-bit spi transfer mode does not seem to be used too often, all
other drivers I looked at are transferring byte by byte and thus can
not be used as an example.

Additionally, the TI platform Vignesh used obviously does not support
unaligned access (which is why he added using bouncebuf here although
no dma is used) while mach-socfpga supports unaligned accesses by
default.

So given my explanation above, what's the preferred way to fix this?

I thought a framework solution would be better, which is why I modified
bouncebuf to work with this, but as cadence_qspi is the only driver
using bouncebuf in this fashion for now, I'm open for suggestions.

I need this driver fixed, so whatever way will be accepted is fine by
me, I guess. Just let me know.

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

Re: [U-Boot] [PATCH v4 2/5] dts: cadence_spi: Sync DT bindings with Linux

[Goldschmidt Simon]

> Jason Rush wrote:
> Adopt the Linux DT bindings and clean-up duplicate
> and unused values.
> ---
> Changes for v4:
>- Rebased

Reviewed-by: Simon Goldschmidt 

> 
>  arch/arm/dts/keystone-k2g-evm.dts |  8 
>  arch/arm/dts/keystone-k2g.dtsi|  5 +++--
>  arch/arm/dts/socfpga.dtsi |  5 +++--
>  arch/arm/dts/socfpga_arria10.dtsi |  4 ++--
>  arch/arm/dts/socfpga_arria5_socdk.dts |  9 -
>  arch/arm/dts/socfpga_cyclone5_is1.dts |  9 -
>  arch/arm/dts/socfpga_cyclone5_socdk.dts   |  9 -
>  arch/arm/dts/socfpga_cyclone5_sockit.dts  |  9 -
>  arch/arm/dts/socfpga_cyclone5_socrates.dts|  9 -
>  arch/arm/dts/socfpga_cyclone5_sr1500.dts  |  9 -
>  arch/arm/dts/socfpga_cyclone5_vining_fpga.dts | 18 --
>  arch/arm/dts/stv0991.dts  | 12 +++-
>  12 files changed, 51 insertions(+), 55 deletions(-)
> 
> diff --git a/arch/arm/dts/keystone-k2g-evm.dts b/arch/arm/dts/keystone-k2g-
> evm.dts
> index de208b3..ba566a4 100644
> --- a/arch/arm/dts/keystone-k2g-evm.dts
> +++ b/arch/arm/dts/keystone-k2g-evm.dts
> @@ -76,10 +76,10 @@
>  spi-max-frequency = <9600>;
>  #address-cells = <1>;
>  #size-cells = <1>;
> -tshsl-ns = <392>;
> -tsd2d-ns = <392>;
> -tchsh-ns = <100>;
> -tslch-ns = <100>;
> +cdns,tshsl-ns = <392>;
> +cdns,tsd2d-ns = <392>;
> +cdns,tchsh-ns = <100>;
> +cdns,tslch-ns = <100>;
>   block-size = <18>;
> 
> 
> diff --git a/arch/arm/dts/keystone-k2g.dtsi b/arch/arm/dts/keystone-k2g.dtsi
> index 7b2fae6..9bcfea6 100644
> --- a/arch/arm/dts/keystone-k2g.dtsi
> +++ b/arch/arm/dts/keystone-k2g.dtsi
> @@ -92,8 +92,9 @@
> <0x2400 0x400>;
>   interrupts = ;
>   num-cs = <4>;
> - fifo-depth = <256>;
> - sram-size = <256>;
> + cdns,fifo-depth = <256>;
> + cdns,fifo-width = <4>;
> + cdns,trigger-address = <0x2400>;
>   status = "disabled";
>   };
> 
> diff --git a/arch/arm/dts/socfpga.dtsi b/arch/arm/dts/socfpga.dtsi
> index 8588221..7557aa0 100644
> --- a/arch/arm/dts/socfpga.dtsi
> +++ b/arch/arm/dts/socfpga.dtsi
> @@ -644,8 +644,9 @@
>   clocks = <_clk>;
>   ext-decoder = <0>;  /* external decoder */
>   num-cs = <4>;
> - fifo-depth = <128>;
> - sram-size = <128>;
> + cdns,fifo-depth = <128>;
> + cdns,fifo-width = <4>;
> + cdns,trigger-address = <0x>;
>   bus-num = <2>;
>   status = "disabled";
>   };
> diff --git a/arch/arm/dts/socfpga_arria10.dtsi 
> b/arch/arm/dts/socfpga_arria10.dtsi
> index 377700d..abfd0bc 100644
> --- a/arch/arm/dts/socfpga_arria10.dtsi
> +++ b/arch/arm/dts/socfpga_arria10.dtsi
> @@ -734,8 +734,8 @@
>   clocks = <_main_clk>;
>   ext-decoder = <0>;  /* external decoder */
>   num-chipselect = <4>;
> - fifo-depth = <128>;
> - sram-size = <512>;
> + cdns,fifo-depth = <128>;
> + cdns,fifo-width = <4>;
>   bus-num = <2>;
>   status = "disabled";
>   };
> diff --git a/arch/arm/dts/socfpga_arria5_socdk.dts
> b/arch/arm/dts/socfpga_arria5_socdk.dts
> index 7265058..1e91a65 100644
> --- a/arch/arm/dts/socfpga_arria5_socdk.dts
> +++ b/arch/arm/dts/socfpga_arria5_socdk.dts
> @@ -94,10 +94,9 @@
>   m25p,fast-read;
>   page-size = <256>;
>   block-size = <16>; /* 2^16, 64KB */
> - read-delay = <4>;  /* delay value in read data capture register 
> */
> - tshsl-ns = <50>;
> - tsd2d-ns = <50>;
> - tchsh-ns = <4>;
> - tslch-ns = <4>;
> + cdns,tshsl-ns = <50>;
> + cdns,tsd2d-ns = <50>;
> + cdns,tchsh-ns = <4>;
> + cdns,tslch-ns = <4>;
>   };
>  };
> diff --git a/arch/arm/dts/socfpga_cyclone5_is1.dts
> b/arch/arm/dts/socfpga_cyclone5_is1.dts
> index 16a3283..2e2b71f 100644
> --- a/arch/arm/dts/socfpga_cyclone5_is1.dts
> +++ b/arch/arm/dts/socfpga_cyclone5_is1.dts
> @@ -93,11 +93,10 @@
>   m25p,fast-read;
>   page-size = <256>;
>   block-size = <16>; /* 2^16, 64KB */
> - read-delay = <4>;  /* delay value in read data capture register 
> */
> - tshsl-ns = <50>;
> - tsd2d-ns = 

Re: [U-Boot] [PATCH v4 3/5] config: cadence_spi: Remove defines read from DT

[Goldschmidt Simon]

> Jason Rush wrote:
> Cleanup unused #define values that are read from the DT.
> ---
> Changes for v4:
>- Rebased

Reviewed-by: Simon Goldschmidt 

Tested on a socfpga-cyclonev board:
Tested-by: Simon Goldschmidt 

Best regards,
Simon

> 
>  include/configs/k2g_evm.h| 1 -
>  include/configs/socfpga_common.h | 1 -
>  include/configs/stv0991.h| 1 -
>  3 files changed, 3 deletions(-)
> 
> diff --git a/include/configs/k2g_evm.h b/include/configs/k2g_evm.h
> index df81c09..535e712 100644
> --- a/include/configs/k2g_evm.h
> +++ b/include/configs/k2g_evm.h
> @@ -93,7 +93,6 @@
>  #ifndef CONFIG_SPL_BUILD
>  #define CONFIG_CADENCE_QSPI
>  #define CONFIG_CQSPI_REF_CLK 38400
> -#define CONFIG_CQSPI_DECODER 0x0
>  #define CONFIG_BOUNCE_BUFFER
>  #endif
> 
> diff --git a/include/configs/socfpga_common.h
> b/include/configs/socfpga_common.h
> index 8a7debb..e5e2f83 100644
> --- a/include/configs/socfpga_common.h
> +++ b/include/configs/socfpga_common.h
> @@ -185,7 +185,6 @@ unsigned int cm_get_l4_sp_clk_hz(void);
>  unsigned int cm_get_qspi_controller_clk_hz(void);
>  #define CONFIG_CQSPI_REF_CLK cm_get_qspi_controller_clk_hz()
>  #endif
> -#define CONFIG_CQSPI_DECODER 0
>  #define CONFIG_BOUNCE_BUFFER
> 
>  /*
> diff --git a/include/configs/stv0991.h b/include/configs/stv0991.h
> index c99fb67..fd96979 100644
> --- a/include/configs/stv0991.h
> +++ b/include/configs/stv0991.h
> @@ -63,7 +63,6 @@
>  + * QSPI support
>  + */
>  #ifdef CONFIG_OF_CONTROL /* QSPI is controlled via DT */
> -#define CONFIG_CQSPI_DECODER 0
>  #define CONFIG_CQSPI_REF_CLK ((30/4)/2)*1000*1000
>  #define CONFIG_BOUNCE_BUFFER
> 
> --
> 2.1.4

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

[U-Boot] [PATCH v2] Revert "spi: cadence_qspi_apb: Use 32 bit indirect read transaction when possible"

[Goldschmidt Simon]

This reverts commit b63b46313ed29e9b0c36b3d6b9407f6eade40c8f.

This commit changed cadence_qspi_apb to use bouncebuf.c, which invalidates
the data cache after reading. This is meant for dma transfers only and
breaks the cadence_qspi driver which copies via cpu only: data that is
copied by the cpu is in cache only and the cache invalidation at the end
throws away this data.

Signed-off-by: Simon Goldschmidt 
---

Changes for v2:
   - Rebased on top of Jason's patchset v4 "Sync DT bindings with Linux"

 drivers/spi/cadence_qspi_apb.c | 22 ++
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index 8309ab8794..c7cb33aa8f 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -627,8 +627,6 @@ int cadence_qspi_apb_indirect_read_execute(struct 
cadence_spi_platdata *plat,
 {
unsigned int remaining = n_rx;
unsigned int bytes_to_read = 0;
-   struct bounce_buffer bb;
-   u8 *bb_rxbuf;
int ret;
 
writel(n_rx, plat->regbase + CQSPI_REG_INDIRECTRDBYTES);
@@ -637,11 +635,6 @@ int cadence_qspi_apb_indirect_read_execute(struct 
cadence_spi_platdata *plat,
writel(CQSPI_REG_INDIRECTRD_START,
   plat->regbase + CQSPI_REG_INDIRECTRD);
 
-   ret = bounce_buffer_start(, (void *)rxbuf, n_rx, GEN_BB_WRITE);
-   if (ret)
-   return ret;
-   bb_rxbuf = bb.bounce_buffer;
-
while (remaining > 0) {
ret = cadence_qspi_wait_for_data(plat);
if (ret < 0) {
@@ -655,13 +648,12 @@ int cadence_qspi_apb_indirect_read_execute(struct 
cadence_spi_platdata *plat,
bytes_to_read *= plat->fifo_width;
bytes_to_read = bytes_to_read > remaining ?
remaining : bytes_to_read;
-   readsl(plat->ahbbase, bb_rxbuf, bytes_to_read >> 2);
-   if (bytes_to_read % 4)
-   readsb(plat->ahbbase,
-  bb_rxbuf + rounddown(bytes_to_read, 4),
-  bytes_to_read % 4);
-
-   bb_rxbuf += bytes_to_read;
+   /* Handle non-4-byte aligned access to avoid data 
abort. */
+   if (((uintptr_t)rxbuf % 4) || (bytes_to_read % 4))
+   readsb(plat->ahbbase, rxbuf, bytes_to_read);
+   else
+   readsl(plat->ahbbase, rxbuf, bytes_to_read >> 
2);
+   rxbuf += bytes_to_read;
remaining -= bytes_to_read;
bytes_to_read = cadence_qspi_get_rd_sram_level(plat);
}
@@ -678,7 +670,6 @@ int cadence_qspi_apb_indirect_read_execute(struct 
cadence_spi_platdata *plat,
/* Clear indirect completion status */
writel(CQSPI_REG_INDIRECTRD_DONE,
   plat->regbase + CQSPI_REG_INDIRECTRD);
-   bounce_buffer_stop();
 
return 0;
 
@@ -686,7 +677,6 @@ failrd:
/* Cancel the indirect read */
writel(CQSPI_REG_INDIRECTRD_CANCEL,
   plat->regbase + CQSPI_REG_INDIRECTRD);
-   bounce_buffer_stop();
return ret;
 }
 
-- 
2.12.2.windows.2

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

Re: [U-Boot] [PATCH v4 1/5] spi: cadence_spi: Sync DT bindings with Linux

[Goldschmidt Simon]

> Jason Rush wrote:
> Adopt the Linux DT bindings. This also fixes an issue
> with the indaddrtrig register on the Cadence QSPI
> device being programmed with the wrong value for the
> socfpga arch.
> ---
> Changes for v4:
>- Rebased
> 

Reviewed-by: Simon Goldschmidt 

Tested on a socfpga-cyclonev board:
Tested-by: Simon Goldschmidt 

Best regards,
Simon

>  drivers/spi/cadence_qspi.c | 20 
>  drivers/spi/cadence_qspi.h |  6 +-
>  drivers/spi/cadence_qspi_apb.c | 15 ---
>  3 files changed, 21 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c
> index 9a6e41f..991a716 100644
> --- a/drivers/spi/cadence_qspi.c
> +++ b/drivers/spi/cadence_qspi.c
> @@ -212,7 +212,7 @@ static int cadence_spi_xfer(struct udevice *dev, unsigned
> int bitlen,
> 
>   /* Set Chip select */
>   cadence_qspi_apb_chipselect(base, spi_chip_select(dev),
> - CONFIG_CQSPI_DECODER);
> + plat->is_decoded_cs);
> 
>   if ((flags & SPI_XFER_END) || (flags == 0)) {
>   if (priv->cmd_len == 0) {
> @@ -296,7 +296,11 @@ static int cadence_spi_ofdata_to_platdata(struct udevice
> *bus)
> 
>   plat->regbase = (void *)data[0];
>   plat->ahbbase = (void *)data[2];
> - plat->sram_size = fdtdec_get_int(blob, node, "sram-size", 128);
> + plat->is_decoded_cs = fdtdec_get_bool(blob, node, "cdns,is-decoded-
> cs");
> + plat->fifo_depth = fdtdec_get_uint(blob, node, "cdns,fifo-depth", 128);
> + plat->fifo_width = fdtdec_get_uint(blob, node, "cdns,fifo-width", 4);
> + plat->trigger_address = fdtdec_get_uint(blob, node,
> + "cdns,trigger-address", 0);
> 
>   /* All other paramters are embedded in the child node */
>   subnode = fdt_first_subnode(blob, node);
> @@ -310,12 +314,12 @@ static int cadence_spi_ofdata_to_platdata(struct
> udevice *bus)
>  50);
> 
>   /* Read other parameters from DT */
> - plat->page_size = fdtdec_get_int(blob, subnode, "page-size", 256);
> - plat->block_size = fdtdec_get_int(blob, subnode, "block-size", 16);
> - plat->tshsl_ns = fdtdec_get_int(blob, subnode, "tshsl-ns", 200);
> - plat->tsd2d_ns = fdtdec_get_int(blob, subnode, "tsd2d-ns", 255);
> - plat->tchsh_ns = fdtdec_get_int(blob, subnode, "tchsh-ns", 20);
> - plat->tslch_ns = fdtdec_get_int(blob, subnode, "tslch-ns", 20);
> + plat->page_size = fdtdec_get_uint(blob, subnode, "page-size", 256);
> + plat->block_size = fdtdec_get_uint(blob, subnode, "block-size", 16);
> + plat->tshsl_ns = fdtdec_get_uint(blob, subnode, "cdns,tshsl-ns", 200);
> + plat->tsd2d_ns = fdtdec_get_uint(blob, subnode, "cdns,tsd2d-ns", 255);
> + plat->tchsh_ns = fdtdec_get_uint(blob, subnode, "cdns,tchsh-ns", 20);
> + plat->tslch_ns = fdtdec_get_uint(blob, subnode, "cdns,tslch-ns", 20);
> 
>   debug("%s: regbase=%p ahbbase=%p max-frequency=%d page-
> size=%d\n",
> __func__, plat->regbase, plat->ahbbase, plat->max_hz,
> diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h
> index d1927a4..8315421 100644
> --- a/drivers/spi/cadence_qspi.h
> +++ b/drivers/spi/cadence_qspi.h
> @@ -18,14 +18,18 @@ struct cadence_spi_platdata {
>   unsigned intmax_hz;
>   void*regbase;
>   void*ahbbase;
> + boolis_decoded_cs;
> + u32 fifo_depth;
> + u32 fifo_width;
> + u32 trigger_address;
> 
> + // Flash parameters
>   u32 page_size;
>   u32 block_size;
>   u32 tshsl_ns;
>   u32 tsd2d_ns;
>   u32 tchsh_ns;
>   u32 tslch_ns;
> - u32 sram_size;
>  };
> 
>  struct cadence_spi_priv {
> diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
> index e02f221..8309ab8 100644
> --- a/drivers/spi/cadence_qspi_apb.c
> +++ b/drivers/spi/cadence_qspi_apb.c
> @@ -37,10 +37,6 @@
>  #define CQSPI_REG_RETRY  1
>  #define CQSPI_POLL_IDLE_RETRY3
> 
> -#define CQSPI_FIFO_WIDTH 4
> -
> -#define CQSPI_REG_SRAM_THRESHOLD_WORDS   50
> -
>  /* Transfer mode */
>  #define CQSPI_INST_TYPE_SINGLE   0
>  #define CQSPI_INST_TYPE_DUAL 1
> @@ -51,9 +47,6 @@
>  #define CQSPI_DUMMY_CLKS_PER_BYTE8
>  #define CQSPI_DUMMY_BYTES_MAX4
> 
> -#define CQSPI_REG_SRAM_FILL_THRESHOLD\
> - ((CQSPI_REG_SRAM_SIZE_WORD / 2) * CQSPI_FIFO_WIDTH)
> -
>  /
>   * Controller's configuration and status register (offset from QSPI_BASE)
>   
> 

[U-Boot] spi: cadence_qspi_apb: cache issues on mach-socfpga

[Goldschmidt Simon]

Hi,

I ran into the same issue with the cadence qspi driver and dcache as Jason
reported (in febuary, I think - I started to monitor U-Boot in july only).

May I ask what's the status here? I do need fixes for this to keep
mach-socfpga running with qspi. I currently set dcache off globally and
it works, but I still need the trigger-address fix (which is fixed by Jason's
patch to use linux DT bindings).

Regarding the bounce buffer change: isn't it wrong to use the generic bounce
buffer here? Given the dcache calls in it, it seems like this one is for DMA
but we're doing cpu copy in the cadence qspi driver.

Anything I can do to get this pushed?

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

[U-Boot] [PATCH 2/2] spi: cadence_spi_apb: fix using bouncebuf with writeback dcache

[Goldschmidt Simon]

The last two commits on this file have added bounce buffer handling
to indirect read and write transfers. However, these are cpu-only
transfers and bouncebuf seems to be written for dma transfers only
(it invalidates the dcache in bouncebuf_stop, which throws away data
copied by the cpu that are still in cache only).

The last two commits resulted in reading random data on mach-socfpga.

By using the new flag GEN_BB_NODMA, cadence_qspi is fixed on that
platform (although the 'Sync DT bindings with Linux' patches from
Jason Rush are still needed to make it work).

Signed-off-by: Simon Goldschmidt 
---
 drivers/spi/cadence_qspi_apb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index 7d335519b0..a3e1c84758 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -645,7 +645,7 @@ int cadence_qspi_apb_indirect_read_execute(struct 
cadence_spi_platdata *plat,
writel(CQSPI_REG_INDIRECTRD_START,
   plat->regbase + CQSPI_REG_INDIRECTRD);
 
-   ret = bounce_buffer_start(, (void *)rxbuf, n_rx, GEN_BB_WRITE);
+   ret = bounce_buffer_start(, (void *)rxbuf, n_rx, 
GEN_BB_WRITE|GEN_BB_NODMA);
if (ret)
return ret;
bb_rxbuf = bb.bounce_buffer;
@@ -743,7 +743,7 @@ int cadence_qspi_apb_indirect_write_execute(struct 
cadence_spi_platdata *plat,
 * Handle non-4-byte aligned accesses via bounce buffer to
 * avoid data abort.
 */
-   ret = bounce_buffer_start(, (void *)txbuf, n_tx, GEN_BB_READ);
+   ret = bounce_buffer_start(, (void *)txbuf, n_tx, 
GEN_BB_READ|GEN_BB_NODMA);
if (ret)
return ret;
bb_txbuf = bb.bounce_buffer;
-- 
2.12.2.windows.2

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

[U-Boot] [PATCH 0/2] spi: cadence_spi_apb: fix using bouncebuf with writeback dcache

[Goldschmidt Simon]

Currently, cadence_spi_apb is broken at least on mach-socfpga:

Commits 57897c13de03ac0136d64641a3eab526c6810387 and
b63b46313ed29e9b0c36b3d6b9407f6eade40c8f added bounce buffer handling to
cadence_qspi_apb. This is the first usage of bounce buffers for non-DMA
transfer. As it turns out, bounce buffers like they were did not work
with writeback data cache.

Since the TI K2G SoC seems to need aligned buffers, I chose to make it
work with bounce buffers by adding a new flag that tells bouncebuf to
not issue dcach commands (instead of reverting the above commits).

To make cadence_qspi_apb work on mach-socfpga, we still need the 'Sync
DT bindings with Linux' patches from Jason Rush, but this patch is one
of the required things we need to get qspi support working again on
mach-socfpga.
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH 1/2] common: make bouncebuf work for non-DMA transfers

[Goldschmidt Simon]

Bounce buffer may be used for CPU-only transfers (this is currently
the case for cadence_qspi). However, in this case, invalidating the
data cache might throw away copied data that is still in the cache
only.

To make CPU-only transfers work with bouncebuf (but still take
advantage of having aligned buffers), a new flag 'GEN_BB_NODMA' is
introduced. If this flag is set, cache flushing/invalidation is
skipped and only the alignment part of bouncebuf is active.

Signed-off-by: Simon Goldschmidt 
---
 common/bouncebuf.c  | 9 +
 include/bouncebuf.h | 9 +
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/common/bouncebuf.c b/common/bouncebuf.c
index 054d9e0302..0d18477f13 100644
--- a/common/bouncebuf.c
+++ b/common/bouncebuf.c
@@ -55,16 +55,17 @@ int bounce_buffer_start(struct bounce_buffer *state, void 
*data,
 * Flush data to RAM so DMA reads can pick it up,
 * and any CPU writebacks don't race with DMA writes
 */
-   flush_dcache_range((unsigned long)state->bounce_buffer,
-   (unsigned long)(state->bounce_buffer) +
-   state->len_aligned);
+   if (!(state->flags & GEN_BB_NODMA))
+   flush_dcache_range((unsigned long)state->bounce_buffer,
+   (unsigned long)(state->bounce_buffer) +
+   state->len_aligned);
 
return 0;
 }
 
 int bounce_buffer_stop(struct bounce_buffer *state)
 {
-   if (state->flags & GEN_BB_WRITE) {
+   if ((state->flags & (GEN_BB_WRITE|GEN_BB_NODMA)) == GEN_BB_WRITE) {
/* Invalidate cache so that CPU can see any newly DMA'd data */
invalidate_dcache_range((unsigned long)state->bounce_buffer,
(unsigned long)(state->bounce_buffer) +
diff --git a/include/bouncebuf.h b/include/bouncebuf.h
index 5ffa99bc13..c6720b3b2e 100644
--- a/include/bouncebuf.h
+++ b/include/bouncebuf.h
@@ -37,6 +37,15 @@
  */
 #define GEN_BB_RW  (GEN_BB_READ | GEN_BB_WRITE)
 
+/*
+ * GEN_BB_NODMA -- Data is read/written by CPU only (no DMA), so no cache
+ * flushing and invalidating is required. Not passing this for GEN_BB_WRITE
+ * transfers done by the CPU (not DMA) may result in invalid data as the data
+ * written into the cache is lost by invalidating the dcache after the transfer
+ * (unless a writethrough cache is used).
+ */
+#define GEN_BB_NODMA   (1 << 2)
+
 struct bounce_buffer {
/* Copy of data parameter passed to start() */
void *user_buffer;
-- 
2.12.2.windows.2

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

Re: [U-Boot] cadence_qspi_apb: cache issues on mach-socfpga

[Goldschmidt Simon]

Rush, Jason A wrote;:
> I don't believe the patchset I submitted for DT bindings were merged in.

I can confirm that. I'd strongly vote for them to get in as cadence_qspi
is otherwise not usable on mach socfpga.

How can I ensure a tested-by from me gets related to that patch set?
I can't reply to it as I was not subscribed to the list at that time.

>> Regarding the bounce buffer change: isn't it wrong to use the generic bounce
>> buffer here? Given the dcache calls in it, it seems like this one is for DMA
>> but we're doing cpu copy in the cadence qspi driver.
> 
> I tend to agree for the socfpga architecture; however, I'm not very familiar 
> with
> the bounce buffer and dcache.  From what I recall, it looked like the data is 
> copied
> from the QSPI by the CPU on the socfpga so the data itself might be in the 
> dcache?
> Then the call to bounce_buffer_stop invalidates the dcache, so the QSPI data 
> that
> was there is now invalidated, and whatever random data in higher level memory
> is copied over the QSPI data.

Right. That's what my results were, too. I only discovered this after starting
this thread and sent a patch later today

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

Re: [U-Boot] [PATCH 1/2] common: make bouncebuf work for non-DMA transfers

[Goldschmidt Simon]

Marek Vasut wrote:
> Why don't you just fix the cache operations in the driver ?

This driver is copying CPU only. There are no cache operations involved!
Vignesh added the bounce buffer obviously to fix alignment issues on
his platform.

> This bounce-buffer for only CPU operations is just wasting CPU cycles
> and degrades performance and I just cannot find a good reason for it.

I only thought using the bounce buffer here would be better than to
add memcpy/memmove for alignment reasons in the driver (in terms
of possible code duplication).

I can of course implement this in a different way.

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

Re: [U-Boot] [PATCH 1/2] common: make bouncebuf work for non-DMA transfers

[Goldschmidt Simon]

Marek Vasut wrote:
> So what alignment problems do you observe ? If you copy using the CPU
> only, why do you need the bounce buffer at all ? I don't quite get it.

Sorry for not explaining it good enough:
I don't observe any alignment problems. mach-socfpga can do unaligned
accesses as well. This driver does CPU based copy, but since it transfers
words, not bytes, I guess on other platforms, the CPU might fail when trying
to read a word from an unaligned pointer.

Vignesh added this about a year ago and from that commit message, it
seems like he was observing alignment errors on his TI platform.
Also, those commits have a reviewed-by tag from you and Jagan.

For me, reverting these patches would be OK as well, but I guess Vinesh's
TI platform kind of needs them?


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

Re: [U-Boot] cadence_qspi_apb: cache issues on mach-socfpga

[Goldschmidt Simon]

Marek Vasut wrote:
>>> I don't believe the patchset I submitted for DT bindings were merged in.
>>
>> I can confirm that. I'd strongly vote for them to get in as cadence_qspi
>> is otherwise not usable on mach socfpga.
>>
>> How can I ensure a tested-by from me gets related to that patch set?
>> I can't reply to it as I was not subscribed to the list at that time.
> 
> You should rebase that patch anyway and then resend it.

I could do that work but I guess it would be up to Jason to do that,
regarding authorship?

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

Re: [U-Boot] [PATCH] sf: ensure flash device is in 3-byte address mode

[Goldschmidt Simon]

On Mon, Oct 30, 2017 at 7:26 AM, Jagan Teki  wrote:
> I've similar change on my patchwork, since no-one tested Will CC you by 
> re-basing it please have test?

Yes, of course I'd like to test this. Where do I find your patch?

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

Re: [U-Boot] [U-Boot, 2/3] rockchip: doc: update U-Boot location info

[Goldschmidt Simon]

>> Update rockchip U-Boot location to 0x4000/16384.
>> 
>> Signed-off-by: Kever Yang 
>> Acked-by: Philipp Tomsich 
>> Reviewed-by: Philipp Tomsich 
>> ---
>> 
>>  doc/README.rockchip | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>> 
> 
> Applied to u-boot-rockchip, thanks!

I just found this commit has calculated the size wrong. 16384 blocks should be 
8MB, not 4MB.


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

Re: [U-Boot] [PATCH] sf: ensure flash device is in 3-byte address mode

[Goldschmidt Simon]

On 2017-10-07 09:23 AM, Prabhakar Kushwaha wrote:
> Dear Jagan, Simon,
> 
> > -Original Message-
> > From: U-Boot [mailto:u-boot-boun...@lists.denx.de] On Behalf Of Jagan
> > Teki
> > Sent: Thursday, December 07, 2017 11:19 AM
> > To: Goldschmidt Simon <sgoldschm...@de.pepperl-fuchs.com>
> > Cc: u-boot@lists.denx.de
> > Subject: Re: [U-Boot] [PATCH] sf: ensure flash device is in 3-byte
> > address mode
> >
> > On Tue, Dec 5, 2017 at 11:50 AM, Goldschmidt Simon
> > <sgoldschm...@de.pepperl-fuchs.com> wrote:
> > > + Lukasz (as a reviewer of my patch[1])
> > >
> > > On Mon, Dec 4, 2017 at 8:20, Jagan Teki wrote:
> > >> This is the patch[1] for 4-byte addressing, but I would wonder how
> > >> can
> > proceed
> > >> operations with 4-byte if we disable during probe.
> > >>
> > >> [1]
> > https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fgit.
> > denx
> > .de%2F%3Fp%3Du-boot-
> > =02%7C01%7Cprabhakar.kushwaha%40nxp.com%7Ca37e67c0f5fd431396
> > 5f08d53d3649b8%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6364
> >
> 82225771650679=CBQkKDXTE1g1mvEbYuyiBApW2NTxQFCirGeJV9uzX8E
> > %3D=0
> > >> spi.git;a=commitdiff;h=fd0c22a90772379c4c11ba09347d36cc8ee17dca
> > >
> > > OK, so your patch does something different than what I did.
> > >
> > > I was trying to keep the change to U-Boot as small as possible, only
> > > fixing this issue I was seeing:
> > >
> > > After a soft-reboot where the SPI chip was not reset, it is left in
> > > 4-byte addressing mode (linux uses this mode, obviously). Remember
> > > that 4-byte mode is not a permanent setting, so we can enter and
> > > leave it any time we like by issuing a command.
> > >
> > > U-Boot uses the Bank Address Register (BAR) for spi flash chips with
> > > more than 16 MByte, so it impclitly assumes that the chip is in
> > > 3-byte address mode. As I see it, your patch is worth a discussion
> > > named "should we use 4-byte addressing mode on spi flash chips?".
> > > I do think this is a better alternative than writing BAR! But this
> > > change probably needs discussion and testing.
> >
> > OK, will review your patch.
> >
> 
> Other solution to this problem could have been "adding support of 4byte
> addressing".
> 
> There will always be a requirement of supporting >16MB flash.

I would be very happy to have 4-byte addressing support. I just
thought it would be better to first fix the soft-reboot issue I am
having (and at least one other person on this list also had).

Plus, I haven't seen a workign patch for 4-byte addressing on this
list, yet. My patch has no side effects, works and could be merged
for 2018.01.

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

Re: [U-Boot] Linux hang

[Goldschmidt Simon]

Pepperl+Fuchs GmbH, Mannheim
Geschaeftsfuehrer/Managing Directors: Dr.-Ing. Gunther Kegel (Vors./CEO), 
Werner Guthier, Mehmet Hatiboglu
Vorsitzender des Aufsichtsrats/Chairman of the supervisory board: Claus Michael
Registergericht/Register Court: AG Mannheim HRB 4713
On 2017-12-07 12:01, Siegmund, Jan wrote:
> Hi all,
> does anybody have an idea for the following problem?
>
> * FPGA is programmed using an overlay
> * FPGA writes to SDRAM via the FPGA2SDRAM-bridge
> * Linux hangs and the watchdog resets the board (the FPGA stays programmed)
> * After the reset and boot, the FPGA is reprogrammed using the same overlay
> * Now, the FPGA can write to the SDRAM without a problem
> [..]

I haven't tried this method of programming the fpga yet (only
programmed from U-Boot for now). But reading the SPL source code, it
seems as the bridges are taken out of reset if the fpga is programmed
when the SPL runs. That's a difference. And it would mean using your
way of programming the fpga, the bridges might still be in reset.

Regards,
Simon

Wichtiger Hinweis:
Diese E-Mail einschliesslich ihrer Anhaenge enthaelt vertrauliche und rechtlich 
geschuetzte Informationen, die nur fuer den Adressaten bestimmt sind.
Sollten Sie nicht der bezeichnete Adressat sein, so teilen Sie dies bitte dem 
Absender umgehend mit und loeschen Sie diese Nachricht und ihre Anhaenge. Die 
unbefugte Weitergabe, das Anfertigen von Kopien und jede Veraenderung der 
E-Mail ist untersagt. Der Absender haftet nicht fuer Inhalte von veraenderten 
E-Mails.


Important Information:
This e-mail message including its attachments contains confidential and legally 
protected information solely intended for the addressee. If you are not the 
intended addressee of this message, please contact the addresser immediately 
and delete this message including its attachments. The unauthorized 
dissemination, copying and change of this e-mail are strictly forbidden. The 
addresser shall not be liable for the content of such changed e-mails.
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [RFC 0/5] sf: Update spi-nor framework

[Goldschmidt Simon]

On Tuesday 12 December 2017 11:51, Vignesh R wrote:
> [...]
> Many of the newer SPI NOR flashes such as MT35x do not support U-Boot's
> legacy way of accessing >128Mb region.
> Are you planning to submit dm-spi-nor anytime soon? If not, then IMO, at
> least patch 2/5 is worth considering for now.

I haven't tested that patch, but I assume it uses 4-byte mode instead
of the bank register? That would certainly improve the situation in
my case, too, where the flash is in 4-byte mode after soft reset and
we would have to set it back to 3-byte mode.

(https://patchwork.ozlabs.org/patch/826919/)


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

[U-Boot] [PATCH] arm: socfpga: correctly reserve SRAM for boot counter

[Goldschmidt Simon]

Bootcounter for is1 and sr1500 boards somewhat relied on struct global data 
alignment gap
at the end of internal SRAM. Let's fix this by explicitly reserving some bytes.

Signed-off-by: Simon Goldschmidt 
---
 include/configs/socfpga_common.h | 6 +-
 include/configs/socfpga_is1.h| 3 ++-
 include/configs/socfpga_sr1500.h | 3 ++-
 3 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/include/configs/socfpga_common.h b/include/configs/socfpga_common.h
index 4de2aa7929..1934aea86d 100644
--- a/include/configs/socfpga_common.h
+++ b/include/configs/socfpga_common.h
@@ -35,8 +35,12 @@
 #define CONFIG_SYS_INIT_RAM_ADDR   0xFFE0
 #define CONFIG_SYS_INIT_RAM_SIZE   0x4 /* 256KB */
 #endif
+/* Reserve bytes at the end of SRAM? */
+#ifndef SOCFPGA_INIT_RAM_END_RESERVE
+#define SOCFPGA_INIT_RAM_END_RESERVE   0
+#endif
 #define CONFIG_SYS_INIT_SP_OFFSET  \
-   (CONFIG_SYS_INIT_RAM_SIZE - GENERATED_GBL_DATA_SIZE)
+   (CONFIG_SYS_INIT_RAM_SIZE - SOCFPGA_INIT_RAM_END_RESERVE)
 #define CONFIG_SYS_INIT_SP_ADDR\
(CONFIG_SYS_INIT_RAM_ADDR + CONFIG_SYS_INIT_SP_OFFSET)
 
diff --git a/include/configs/socfpga_is1.h b/include/configs/socfpga_is1.h
index c233c208a5..b243fd29cd 100644
--- a/include/configs/socfpga_is1.h
+++ b/include/configs/socfpga_is1.h
@@ -27,8 +27,9 @@
 #include 
 
 /*
- * Bootcounter
+ * Bootcounter (preserve the last 2 lwords for the boot-counter)
  */
+#define CONFIG_SYS_INIT_RAM_END_RESERVE8
 #define CONFIG_SYS_BOOTCOUNT_BE
 
 #endif /* __CONFIG_SOCFPGA_IS1_H__ */
diff --git a/include/configs/socfpga_sr1500.h b/include/configs/socfpga_sr1500.h
index c835d23235..ff71712566 100644
--- a/include/configs/socfpga_sr1500.h
+++ b/include/configs/socfpga_sr1500.h
@@ -26,8 +26,9 @@
 #define CONFIG_SPI_N25Q256A_RESET
 
 /*
- * Bootcounter
+ * Bootcounter (preserve the last 2 lwords for the boot-counter)
  */
+#define SOCFPGA_INIT_RAM_END_RESERVE   8
 #define CONFIG_SYS_BOOTCOUNT_BE
 
 /* Environment setting for SPI flash */
-- 
2.17.0.windows.1

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

Re: [U-Boot] QSPI "sf probe ...", "sf read ..." on Altera SoC FPGA

[Goldschmidt Simon]

On Fri, 05/01/2018 Marek Vasut wrote:
> On 01/05/2018 04:49 PM, Goldschmidt Simon wrote:



>>>> OK, so I need these patches to get qspi work on socfpga:
>>>>
>>>> - Series "spi: cadence_spi: Adopt Linux DT bindings" (v4) from Jason Rush:
>>>>   https://patchwork.ozlabs.org/project/uboot/list/?series=13864
>>>> - Patch "Revert "spi: cadence_qspi_apb: Use 32 bit indirect read 
>>>> transaction when possible" (v2)
>>>>   https://patchwork.ozlabs.org/patch/838871/
>>>
>>> I've waited for ack/tested-by from marek or someone who usually worked
>>> on these cadence.
>>
>> Vignesh acked, who already did some of the last changes. But Ok, I've
>> added Marek to the loop.
>>
>> Marek, do you see any problems here? Are you running QSPI on the
>> socfpga platform anywhere?
> I am not entirely sure what this partial thread is all about, do you
> need some patches Acked ? Repost them including the Acks collected
> already and CC me, I want to review them. PW link is not enough.

Marek, you were one of the people addressed in "to:" when Jason Rush
sent "[PATCH v4 1/5] spi: cadence_spi: Sync DT bindings with Linux"
and successors on Nov. 16th 2017.

You were also in the "to:" field when I sent "[PATCH v4 1/5] spi:
cadence_spi: Sync DT bindings with Linux" on Nov. 17th 2017.

Should I resend them anyway?

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

Re: [U-Boot] QSPI "sf probe ...", "sf read ..." on Altera SoC FPGA

[Goldschmidt Simon]

On Fri, 05/01/2018, Marek Vasut wrote:
> On 01/05/2018 08:31 PM, Goldschmidt Simon wrote:
>> On Fri, 05/01/2018 Marek Vasut wrote:
>>> On 01/05/2018 04:49 PM, Goldschmidt Simon wrote:
>>
>> 
>>
>>>>>> OK, so I need these patches to get qspi work on socfpga:
>>>>>>
>>>>>> - Series "spi: cadence_spi: Adopt Linux DT bindings" (v4) from Jason 
>>>>>> Rush:
>>>>>>   https://patchwork.ozlabs.org/project/uboot/list/?series=13864
>>>>>> - Patch "Revert "spi: cadence_qspi_apb: Use 32 bit indirect read 
>>>>>> transaction when possible" (v2)
>>>>>>   https://patchwork.ozlabs.org/patch/838871/
>>>>>
>>>>> I've waited for ack/tested-by from marek or someone who usually worked
>>>>> on these cadence.
>>>>
>>>> Vignesh acked, who already did some of the last changes. But Ok, I've
>>>> added Marek to the loop.
>>>>
>>>> Marek, do you see any problems here? Are you running QSPI on the
>>>> socfpga platform anywhere?
>>> I am not entirely sure what this partial thread is all about, do you
>>> need some patches Acked ? Repost them including the Acks collected
>>> already and CC me, I want to review them. PW link is not enough.
>>
>> Marek, you were one of the people addressed in "to:" when Jason Rush
>> sent "[PATCH v4 1/5] spi: cadence_spi: Sync DT bindings with Linux"
>> and successors on Nov. 16th 2017.
>>
>> You were also in the "to:" field when I sent "[PATCH v4 1/5] spi:
>> cadence_spi: Sync DT bindings with Linux" on Nov. 17th 2017.
>>
>> Should I resend them anyway?
> 
> I really dunno what to make of this sparse thread, sorry. Also, I don't
> quite remember what happened in this thread on November 17th, sorry.
> 
> So maybe you should elaborate what you expect me to do ... or just
> resend the patches, yes.

OK, I'll resend the patches then. I'll try to combine the 2 series
into one but have to check with Jason first.

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

Re: [U-Boot] [PATCH 0/3] cadence-quadspi: Fix issues with non 32bit aligned accesses

[Goldschmidt Simon]

On 08/01/2018 12:18m Vignesh R wrote:
> This series reverts use of bounce_buf.c for non-DMA related alignment 
> restriction
> and replaces it with local bounce buffer to handle problems with non 32 bit 
> aligned
> writes on TI platforms.
> Based on top of Jason's series:
> https://patchwork.ozlabs.org/cover/856431/
> 
> Tested on K2G EVM.
> 
> Goldschmidt Simon (1):
>   Revert "spi: cadence_qspi_apb: Use 32 bit indirect read transaction
> when possible"
> 
> Vignesh R (2):
>   Revert "spi: cadence_qspi_apb: Use 32 bit indirect write transaction
> when possible"
>   spi: cadence_qspi_apb: Make flash writes 32 bit aligned
> 
>  drivers/spi/cadence_qspi.c   | 13 -
>  drivers/spi/cadence_qspi.h   |  1 +
>  drivers/spi/cadence_qspi_apb.c   | 42 
> +---
>  include/configs/k2g_evm.h|  1 -
>  include/configs/socfpga_common.h |  1 -
>  include/configs/stv0991.h|  1 -
>  6 files changed, 22 insertions(+), 37 deletions(-)

For this whole series:
Tested on a socfpga-cyclonev board (with a Micron N25QL256A):
Tested-by: Simon Goldschmidt <sgoldschm...@de.pepperl-fuchs.com>

After applying this series on top of Jason's v5, qspi on the socfpga
is finally working without local fixes!

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

Re: [U-Boot] [PATCH 3/3] spi: cadence_qspi_apb: Make flash writes 32 bit aligned

[Goldschmidt Simon]

On Mon, 08/01/2018 12:18, Vignesh R wrote:
> Make flash writes 32 bit aligned by using bounce buffers to deal with non 32 
> bit
> aligned buffers. Allocate a 512 byte bounce buffer (max known page size
> currently) for this case.

Looking at drivers/mtd/spi/sf_dataflash.c, I see at least one chip
with 1024 byte page size (at45db642d).
This should probably be a constant somewhere, or at least be checked
at runtime, see below.

> This is required because as per TI K2G TRM[1], the external master is only
> permitted to issue 32-bit data interface writes until the last word of an 
> indirect
> transfer. Otherwise indirect writes is known to fail sometimes.
> 
> [1] http://www.ti.com/lit/ug/spruhy8g/spruhy8g.pdf
> 
> Signed-off-by: Vignesh R 
> ---
>  drivers/spi/cadence_qspi.c | 13 -
>  drivers/spi/cadence_qspi.h |  1 +
>  drivers/spi/cadence_qspi_apb.c | 10 +-
>  3 files changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c index
> 991a7160bdb8..49c9b477e678 100644
> --- a/drivers/spi/cadence_qspi.c
> +++ b/drivers/spi/cadence_qspi.c
> @@ -158,6 +158,10 @@ static int cadence_spi_probe(struct udevice *bus)
> 
>   priv->regbase = plat->regbase;
>   priv->ahbbase = plat->ahbbase;
> + /* Bounce buf to handle unaligned writes. 512B is max pagesize */
> + priv->bounce_buf = malloc(512);

This should probably be a named constant.

> + if (!priv->bounce_buf)
> + return -ENOMEM;
> 
>   if (!priv->qspi_is_init) {
>   cadence_qspi_apb_controller_init(plat);
> @@ -259,8 +263,15 @@ static int cadence_spi_xfer(struct udevice *dev, unsigned
> int bitlen,
>   err = cadence_qspi_apb_indirect_write_setup
>   (plat, priv->cmd_len, cmd_buf);
>   if (!err) {
> + const u8 *txbuf = dout;
> +
> + if ((uintptr_t)txbuf % 4) {
> + memcpy(priv->bounce_buf, dout,
> +data_bytes);

Without checking the size of the buffer allocated for bounce_buf here,
you risk a buffer overflow for chips with larger page size.

> + txbuf = priv->bounce_buf;
> + }
>   err = cadence_qspi_apb_indirect_write_execute
> - (plat, data_bytes, dout);
> + (plat, data_bytes, txbuf);
>   }
>   break;
>   default:
> diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h index
> 83154210bd95..c97419af3de3 100644
> --- a/drivers/spi/cadence_qspi.h
> +++ b/drivers/spi/cadence_qspi.h
> @@ -43,6 +43,7 @@ struct cadence_spi_priv {
>   unsigned intqspi_calibrated_hz;
>   unsigned intqspi_calibrated_cs;
>   unsigned intprevious_hz;
> + void*bounce_buf;
>  };
> 
>  /* Functions call declaration */
> diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
> index fa8af33ae19b..fd3ece4cb129 100644
> --- a/drivers/spi/cadence_qspi_apb.c
> +++ b/drivers/spi/cadence_qspi_apb.c
> @@ -727,11 +727,11 @@ int cadence_qspi_apb_indirect_write_execute(struct
> cadence_spi_platdata *plat,
> 
>   while (remaining > 0) {
>   write_bytes = remaining > page_size ? page_size : remaining;
> - /* Handle non-4-byte aligned access to avoid data abort. */
> - if (((uintptr_t)txbuf % 4) || (write_bytes % 4))
> - writesb(plat->ahbbase, txbuf, write_bytes);
> - else
> - writesl(plat->ahbbase, txbuf, write_bytes >> 2);
> + writesl(plat->ahbbase, txbuf, write_bytes >> 2);
> + if (write_bytes % 4)
> + writesb(plat->ahbbase,
> + txbuf + rounddown(write_bytes, 4),
> + write_bytes % 4);
> 
>   ret = wait_for_bit("QSPI", plat->regbase +
> CQSPI_REG_SDRAMLEVEL,
>  CQSPI_REG_SDRAMLEVEL_WR_MASK <<
> --
> 2.15.1

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

Re: [U-Boot] QSPI "sf probe ...", "sf read ..." on Altera SoC FPGA

[Goldschmidt Simon]

On Mon, 08/012018 06:27, Vignesh R wrote:
> On Monday 08 January 2018 09:10 AM, Jason Rush wrote:
> [...]
> >>> 1. The indaddrtrig register was being programmed with an incorrect
> >>> value for socfpga as the result of assuming it should be programmed
> >>> with the same address as the ahbbase address.  This issue is
> >>> resolved by adopting the Linux DT bindings, which has an independent
> >>> setting for the indaddrtrig register so the register can be set correctly 
> >>> on all
> architectures.  Plus it aligns the DT between u-boot and Linux.
> >> That should be an easy patch, so this is the patchset 0/5..5/5 that
> >> you just submitted ?
> > Yes. I saw you Acked it, thank you.
> >>> 2. The cadence driver was modified at one point to use the bouncebuf
> >>> functions to fix an issue on a TI architecture that expected, where
> >>> if I recall correctly all reads except the last have to be 32-bit
> >>> reads.  However, since the bouncebuf was designed for DMA transfers,
> >>> it invalidates the data cache after reading, but since the cadence
> >>> is using cpu transfers the newly read data is thrown away when the
> >>> cache is invalidated.  This issue is resolved by reverting the commit that
> introduced using the bounce buffer for read operations, which according to
> Vignesh don't cause any issues to the TI architecture.
> >> Hm, I wonder why you need bounce buffer at all here. The CQSPI
> >> literally reads/writes a register space (or some FIFO in register
> >> space), there is no DMA involved at all. I also wonder why we have to
> >> manipulate with cache at all here.
> >
> > I agree, I don't believe this needs a bounce buffer at all.  This
> > isn't a DMA, there is no need for cache manipulation.  Vignesh
> > understands the problem better than I do on the TI platform, but I
> > believe it was used since it was an easy way to ensure the register 
> > read/writes
> were all 32-bits wide up until the last read/write.
> 
> Yes, that was the intention. Unfortunately, I chose to use common bounce 
> buffer
> implementation which was doing cache manipulations.
> 
> > I believe the bounce buffer should be removed from the CQSPI driver
> > and a different solution should be implemented, but Vignesh should
> > weigh in on that since it effects his architecture.
> >
> 
> CQSPI on TI K2G has problems with non 32 bit aligned write operations.
> But read operations are unaffected. Therefore I have Ack'ed Simon's patch
> reverting bouncebuf for read. For writes, I have patches to revert common
> bouncebuf usage and use a local pagesize buffer for overcome alignment issue. 
> I
> am waiting for current patch backlogs to be merged so that its easy for 
> testing w/o
> specifying bunch of dependent patches.
> 
> Or if Simon agrees, I can add his patch to my series post it to mailing list 
> (rebased
> on top of Jason's series)?

Well, it's not really "my" patch, anyway. It reverts a commit of
yours, so sure, as long as this does not stand in the way of getting
qspi running on 2018.03, go ahead and itegrate it in your patchset.

I'd be happy to have this sent now so I can test both patchsets on
top of 2018.01(-rc).

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

Re: [U-Boot] QSPI "sf probe ...", "sf read ..." on Altera SoC FPGA

[Goldschmidt Simon]

+ Marek (as Jagan wants an ack)

On 05/01/2018 Jagan Teki wrote:
> On Fri, Jan 5, 2018 at 5:32 PM, Goldschmidt Simon wrote:
>> + Vignesh
>> + Jason
>>
>> On Wed, 03/01/2018 16:57, Goldschmidt Simon wrote:
>>> On Wed, 03/01/2018 14:51, Jagan Teki wrote:
>>> >> There were already patches posted on this list by me and others, but
>>> >> unfortunately they haven't made it into the repository, yet.
>>> >>
>>> >> Jagan, could you comment on the status of these fixes? I can search
>>> >> for the patchwork items related if you want me to.
>>> >
>>> > 2 out of 1 of this[1] have some discussion still going is it?
>>> >
>>> > [1] https://patchwork.ozlabs.org/patch/838195/
>>>
>>> No, that series should be dropped. I don't know if I can do anything about 
>>> that in
>>> patchwork though?
>>>
>>> Let me check the patches from my upstreaming queue when I'm back at work
>>> tomorrow. I'll send a list of patchwork items I needed to get QSPI running 
>>> on
<>> mach-socfpga.
>>
>> OK, so I need these patches to get qspi work on socfpga:
>>
>> - Series "spi: cadence_spi: Adopt Linux DT bindings" (v4) from Jason Rush:
>>   https://patchwork.ozlabs.org/project/uboot/list/?series=13864
>> - Patch "Revert "spi: cadence_qspi_apb: Use 32 bit indirect read transaction 
>> when possible" (v2)
>>   https://patchwork.ozlabs.org/patch/838871/
> 
> I've waited for ack/tested-by from marek or someone who usually worked
> on these cadence.

Vignesh acked, who already did some of the last changes. But Ok, I've
added Marek to the loop.

Marek, do you see any problems here? Are you running QSPI on the
socfpga platform anywhere?

> 
>>
>> All patches were discussed with Vignesh in November. Could we make
>> sure these make it into 2018.03 now that we missed 2018.01?
>>
>> Aside from that, I have this patch running which ensures my QSPI (that
>> does not have a reset line) is put into 3 byte address mode that
>> U-Boot needs. This would be *very* helpful, too:
>> https://patchwork.ozlabs.org/patch/826919/
> 
> issue discussing with spi-nor changes as well, we will figure it out
> and try for best possible.

Ok, this is a different issue anyway. It is not related to socfpga
or cadence qspi. Maybe I can even trick my Linux to use 4 byte opcodes
instead of the 4 byte mode...

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

[U-Boot] [PATCH] ddr: altera: silence PHY calibration unless in debug mode

[Goldschmidt Simon]

This driver has been using printf() including filename since it was
added. Convert to using debug() instead.

Signed-off-by: Simon Goldschmidt 
---

 drivers/ddr/altera/sequencer.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/ddr/altera/sequencer.c b/drivers/ddr/altera/sequencer.c
index 6c6bd90e94..42e87b50d3 100644
--- a/drivers/ddr/altera/sequencer.c
+++ b/drivers/ddr/altera/sequencer.c
@@ -3534,7 +3534,7 @@ static void debug_mem_calibrate(int pass)
u32 debug_info;
 
if (pass) {
-   printf("%s: CALIBRATION PASSED\n", __FILE__);
+   debug("%s: CALIBRATION PASSED\n", __FILE__);
 
gbl->fom_in /= 2;
gbl->fom_out /= 2;
@@ -3553,7 +3553,7 @@ static void debug_mem_calibrate(int pass)
writel(debug_info, _mgr_cfg->cal_debug_info);
writel(PHY_MGR_CAL_SUCCESS, _mgr_cfg->cal_status);
} else {
-   printf("%s: CALIBRATION FAILED\n", __FILE__);
+   debug("%s: CALIBRATION FAILED\n", __FILE__);
 
debug_info = gbl->error_stage;
debug_info |= gbl->error_substage << 8;
@@ -3570,7 +3570,7 @@ static void debug_mem_calibrate(int pass)
writel(debug_info, _reg_file->failing_stage);
}
 
-   printf("%s: Calibration complete\n", __FILE__);
+   debug("%s: Calibration complete\n", __FILE__);
 }
 
 /**
@@ -3741,7 +3741,7 @@ int sdram_calibration_full(void)
 
initialize_tracking();
 
-   printf("%s: Preparing to start memory calibration\n", __FILE__);
+   debug("%s: Preparing to start memory calibration\n", __FILE__);
 
debug("%s:%d\n", __func__, __LINE__);
debug_cond(DLEVEL >= 1,
-- 
2.11.0

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

Re: [U-Boot] [PATCH v2 0/3] cadence-quadspi: Fix issues with non 32bit aligned accesses

[Goldschmidt Simon]

On Tue 09/01/18 14:19, Vignesh R wrote:
> This series reverts use of bounce_buf.c for non-DMA related alignment 
> restriction
> and replaces it with local bounce buffer to handle problems with non 32 bit 
> aligned
> writes on TI platforms.
> Based on top of Jason's series:
> https://patchwork.ozlabs.org/cover/856431/
> 
> Tested on K2G EVM.

For this whole series:
Tested on a socfpga-cyclonev board (with a Micron N25QL256A):
Tested-by: Simon Goldschmidt 

After applying this series on top of Jason's v5, qspi on the socfpga is finally
working without local fixes!

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

Re: [U-Boot] QSPI "sf probe ...", "sf read ..." on Altera SoC FPGA

[Goldschmidt Simon]

+ Jagan

On Wed, 03/01/2018, Mr. goldenstreet wrote:
> hey, i have looked at this thread:
> http://u-boot.10912.n7.nabble.com/QSPI-quot-sf-probe-quot-quot-sf-read-quot-on-
> Altera-SoC-FPGA-td304882.html
> 
> i'm having the same problem with Arria 5, when i try to use the "sf probe"
> command on the nor flash, the result i'm getting0 is:
> 
> SF: Detected n25q512 with page size 256 Bytes, erase size 64 KiB, total
> 64 MiB
> ### ERROR ### Please RESET the board ###
> 
>  i have already tried to use the patches for the uboot code and also to add 
> more
> defines before making the uboot, but it still doesn't work for me.
> 
> any ideas? thanks ahead.

There were already patches posted on this list by me and others, but
unfortunately they haven't made it into the repository, yet.

Jagan, could you comment on the status of these fixes? I can search
for the patchwork items related if you want me to.

I'd really like these fixes to go in without waiting for the new
spi-nor code.

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

Re: [U-Boot] QSPI "sf probe ...", "sf read ..." on Altera SoC FPGA

[Goldschmidt Simon]

+ Vignesh
+ Jason

On Wed, 03/01/2018 16:57, Goldschmidt Simon wrote:
> On Wed, 03/01/2018 14:51, Jagan Teki wrote:
> >> There were already patches posted on this list by me and others, but
> >> unfortunately they haven't made it into the repository, yet.
> >>
> >> Jagan, could you comment on the status of these fixes? I can search
> >> for the patchwork items related if you want me to.
> >
> > 2 out of 1 of this[1] have some discussion still going is it?
> >
> > [1] https://patchwork.ozlabs.org/patch/838195/
> 
> No, that series should be dropped. I don't know if I can do anything about 
> that in
> patchwork though?
> 
> Let me check the patches from my upstreaming queue when I'm back at work
> tomorrow. I'll send a list of patchwork items I needed to get QSPI running on
> mach-socfpga.

OK, so I need these patches to get qspi work on socfpga:

- Series "spi: cadence_spi: Adopt Linux DT bindings" (v4) from Jason Rush:
  https://patchwork.ozlabs.org/project/uboot/list/?series=13864
- Patch "Revert "spi: cadence_qspi_apb: Use 32 bit indirect read transaction 
when possible" (v2)
  https://patchwork.ozlabs.org/patch/838871/

All patches were discussed with Vignesh in November. Could we make
sure these make it into 2018.03 now that we missed 2018.01?

Aside from that, I have this patch running which ensures my QSPI (that
does not have a reset line) is put into 3 byte address mode that
U-Boot needs. This would be *very* helpful, too:
https://patchwork.ozlabs.org/patch/826919/

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

Re: [U-Boot] [PATCH] spl: unbreak CONFIG_SPL_MULTI_DTB_FIT after fixing CONFIG_OF_EMBED

[Goldschmidt Simon]

On Wed, 10/01/18 09:48, Lokesh Vutla wrote:
> On Wednesday 10 January 2018 12:32 PM, Goldschmidt Simon wrote:
> > Commit 9bd76b80 "spl: make CONFIG_OF_EMBED pass dts through fdtgrep"
> > moved the fdtgrep code from scripts/Makefile.spl to dts/Makefile so
> > that the dtb is stripped in embedded mode, too.
> >
> > This broke CONFIG_SPL_MULTI_DTB_FIT where fdtgrep is still called from
> > scripts/Makefile.spl to strip all dtbs in CONFIG_SPL_OF_LIST.
> > To fix that, cmd_fdtgrep is brought back into scripts/Makefile.spl at
> > the downside of having it duplicated in two makefiles.
> 
> Missing Signed-off-by? Also a Reported-by credits would be nice :)

Right. I'll add that in a next version, sorry.

> 
> > ---
> >  scripts/Makefile.spl | 16 
> >  1 file changed, 16 insertions(+)
> >
> > diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl index
> > 64390e5785..72e74f14c3 100644
> > --- a/scripts/Makefile.spl
> > +++ b/scripts/Makefile.spl
> > @@ -239,6 +239,22 @@ $(obj)/$(SPL_BIN)-pad.bin: $(obj)/$(SPL_BIN)
> > @bss_size_str=$(shell $(NM) $< | awk 'BEGIN {size = 0} /__bss_size/ 
> > {size
> = $$1} END {print "ibase=16; " toupper(size)}' | bc); \
> > dd if=/dev/zero of=$@ bs=1 count=$${bss_size_str} 2>/dev/null;
> >
> > +# Pass the original device tree file through fdtgrep twice. The first
> > +pass # removes any unwanted nodes (i.e. those which don't have the #
> > +'u-boot,dm-pre-reloc' property and thus are not needed by SPL. The
> > +second # pass removes various unused properties from the remaining nodes.
> > +# The output is typically a much smaller device tree file.
> > +ifeq ($(CONFIG_TPL_BUILD),y)
> > +fdtgrep_props := -b u-boot,dm-pre-reloc -b u-boot,dm-tpl else
> > +fdtgrep_props := -b u-boot,dm-pre-reloc -b u-boot,dm-spl endif
> > +quiet_cmd_fdtgrep = FDTGREP $@
> > +  cmd_fdtgrep = $(objtree)/tools/fdtgrep $(fdtgrep_props) -RT $< \
> > +   -n /chosen -n /config -O dtb | \
> > +   $(objtree)/tools/fdtgrep -r -O dtb - -o $@ \
> > +   $(addprefix -P ,$(subst
> $\",,$(CONFIG_OF_SPL_REMOVE_PROPS)))
> > +
> 
> hmm..we are duplicating code here. Why can't dtb build for
> CONFIG_OF_EMBED be moved to scripts/Makefile.spl?

I don't like the code duplication either. Back in November, I tried
to create the fdtgrep'ed ftd from Makefile.spl but failed. Seems I'm
not a pro in makefiles... Of course I'd appreciate it if you'd find
a solution for that!

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

Re: [U-Boot] [PATCH] ARM: socfpga: Convert callers of cm_write_with_phase for wait_for_bit_le32

[Goldschmidt Simon]

On 01/26/2018 17:28, Tom Rini wrote:
> Now that we have and use wait_for_bit_le32() available, the callers of
> cm_write_with_phase() should not be casting values to u32 and instead we
> expect a const void *, so provide that directly.
> 
> Fixes: 48263504c8d5 ("wait_bit: use wait_for_bit_le32 and remove 
> wait_for_bit")
> Cc: Marek Vasut 
> Signed-off-by: Tom Rini 
> ---
> arch/arm/mach-socfpga/clock_manager_gen5.c | 10 +-
> 1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm/mach-socfpga/clock_manager_gen5.c 
> b/arch/arm/mach-socfpga/clock_manager_gen5.c
> index 3d048ba3e432..4e5b6d169371 100644
> --- a/arch/arm/mach-socfpga/clock_manager_gen5.c
> +++ b/arch/arm/mach-socfpga/clock_manager_gen5.c
> @@ -33,7 +33,7 @@ static void cm_write_ctrl(u32 val)
> }
> 
>  /* function to write a clock register that has phase information */
> -static int cm_write_with_phase(u32 value, u32 reg_address, u32 mask)
> +static int cm_write_with_phase(u32 value, const void *reg_address, u32 mask)

Shouldn't that better be a non-const void * since it is used for
writing a value, too? That 'const' is later casted away by
__arch_putl(), but it still looks wrong to me.

Simon


>  {
>int ret;
> 
> @@ -268,26 +268,26 @@ int cm_basic_init(const struct cm_config * const cfg)
>  * are aligned nicely; so we can change any phase.
>  */
> ret = cm_write_with_phase(cfg->ddrdqsclk,
> - (u32)_manager_base->sdr_pll.ddrdqsclk,
> + _manager_base->sdr_pll.ddrdqsclk,
>   CLKMGR_SDRPLLGRP_DDRDQSCLK_PHASE_MASK);
> if (ret)
> return ret;
> 
> /* SDRAM DDR2XDQSCLK */
> ret = cm_write_with_phase(cfg->ddr2xdqsclk,
> - 
> (u32)_manager_base->sdr_pll.ddr2xdqsclk,
> + _manager_base->sdr_pll.ddr2xdqsclk,
>   CLKMGR_SDRPLLGRP_DDR2XDQSCLK_PHASE_MASK);
> if (ret)
> return ret;
> 
> ret = cm_write_with_phase(cfg->ddrdqclk,
> - (u32)_manager_base->sdr_pll.ddrdqclk,
> + _manager_base->sdr_pll.ddrdqclk,
>   CLKMGR_SDRPLLGRP_DDRDQCLK_PHASE_MASK);
> if (ret)
> return ret;
> 
> ret = cm_write_with_phase(cfg->s2fuser2clk,
> - 
> (u32)_manager_base->sdr_pll.s2fuser2clk,
> + _manager_base->sdr_pll.s2fuser2clk,
>   CLKMGR_SDRPLLGRP_S2FUSER2CLK_PHASE_MASK);
> if (ret)
> return ret;
> --
> 2.7.4
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [RESEND PATCH] ddr: altera: silence PHY calibration unless in debug mode

[Goldschmidt Simon]

This driver has been using printf() including filename since it was
added. Convert to using debug() instead.

Signed-off-by: Simon Goldschmidt 
---
 drivers/ddr/altera/sequencer.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/ddr/altera/sequencer.c b/drivers/ddr/altera/sequencer.c
index 6c6bd90e94..42e87b50d3 100644
--- a/drivers/ddr/altera/sequencer.c
+++ b/drivers/ddr/altera/sequencer.c
@@ -3534,7 +3534,7 @@ static void debug_mem_calibrate(int pass)
u32 debug_info;
 
if (pass) {
-   printf("%s: CALIBRATION PASSED\n", __FILE__);
+   debug("%s: CALIBRATION PASSED\n", __FILE__);
 
gbl->fom_in /= 2;
gbl->fom_out /= 2;
@@ -3553,7 +3553,7 @@ static void debug_mem_calibrate(int pass)
writel(debug_info, _mgr_cfg->cal_debug_info);
writel(PHY_MGR_CAL_SUCCESS, _mgr_cfg->cal_status);
} else {
-   printf("%s: CALIBRATION FAILED\n", __FILE__);
+   debug("%s: CALIBRATION FAILED\n", __FILE__);
 
debug_info = gbl->error_stage;
debug_info |= gbl->error_substage << 8;
@@ -3570,7 +3570,7 @@ static void debug_mem_calibrate(int pass)
writel(debug_info, _reg_file->failing_stage);
}
 
-   printf("%s: Calibration complete\n", __FILE__);
+   debug("%s: Calibration complete\n", __FILE__);
 }
 
 /**
@@ -3741,7 +3741,7 @@ int sdram_calibration_full(void)
 
initialize_tracking();
 
-   printf("%s: Preparing to start memory calibration\n", __FILE__);
+   debug("%s: Preparing to start memory calibration\n", __FILE__);
 
debug("%s:%d\n", __func__, __LINE__);
debug_cond(DLEVEL >= 1,
-- 
2.12.2.windows.2

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

[U-Boot] [PATCH] spl: unbreak CONFIG_SPL_MULTI_DTB_FIT after fixing CONFIG_OF_EMBED

[Goldschmidt Simon]

Commit 9bd76b80 "spl: make CONFIG_OF_EMBED pass dts through fdtgrep"
moved the fdtgrep code from scripts/Makefile.spl to dts/Makefile so
that the dtb is stripped in embedded mode, too.

This broke CONFIG_SPL_MULTI_DTB_FIT where fdtgrep is still called
from scripts/Makefile.spl to strip all dtbs in CONFIG_SPL_OF_LIST.
To fix that, cmd_fdtgrep is brought back into scripts/Makefile.spl
at the downside of having it duplicated in two makefiles.
---
 scripts/Makefile.spl | 16 
 1 file changed, 16 insertions(+)

diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
index 64390e5785..72e74f14c3 100644
--- a/scripts/Makefile.spl
+++ b/scripts/Makefile.spl
@@ -239,6 +239,22 @@ $(obj)/$(SPL_BIN)-pad.bin: $(obj)/$(SPL_BIN)
@bss_size_str=$(shell $(NM) $< | awk 'BEGIN {size = 0} /__bss_size/ 
{size = $$1} END {print "ibase=16; " toupper(size)}' | bc); \
dd if=/dev/zero of=$@ bs=1 count=$${bss_size_str} 2>/dev/null;
 
+# Pass the original device tree file through fdtgrep twice. The first pass
+# removes any unwanted nodes (i.e. those which don't have the
+# 'u-boot,dm-pre-reloc' property and thus are not needed by SPL. The second
+# pass removes various unused properties from the remaining nodes.
+# The output is typically a much smaller device tree file.
+ifeq ($(CONFIG_TPL_BUILD),y)
+fdtgrep_props := -b u-boot,dm-pre-reloc -b u-boot,dm-tpl
+else
+fdtgrep_props := -b u-boot,dm-pre-reloc -b u-boot,dm-spl
+endif
+quiet_cmd_fdtgrep = FDTGREP $@
+  cmd_fdtgrep = $(objtree)/tools/fdtgrep $(fdtgrep_props) -RT $< \
+   -n /chosen -n /config -O dtb | \
+   $(objtree)/tools/fdtgrep -r -O dtb - -o $@ \
+   $(addprefix -P ,$(subst $\",,$(CONFIG_OF_SPL_REMOVE_PROPS)))
+
 $(obj)/$(SPL_BIN).dtb: dts/dt-spl.dtb FORCE
$(call if_changed,copy)
 
-- 
2.12.2.windows.2

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

Re: [U-Boot] [PATCH] spl: make CONFIG_OF_EMBED pass dts through fdtgrep

[Goldschmidt Simon]

On Tue, 09/01/18 15:43, Lokesh Vutla wrote:
> On Sunday 26 November 2017 05:08 PM, Simon Glass wrote:
> > On 21 November 2017 at 05:29, Goldschmidt Simon
> > <sgoldschm...@de.pepperl-fuchs.com> wrote:
> >> Building spl with CONFIG_OF_EMBED enabled results in an error message
> >> on my board: "SPL image too big". This is because the fdtgrep build
> >> step is only executed for CONFIG_OF_SEPARATE.
> >>
> >> Fix this by moving the fdtgrep build step ('cmd_fdtgreo') from
> >> scripts/Makefile.spl to dts/Makefile so that the reduced dtb is
> >> available for all kinds of spl builds.
> >>
> >> The resulting variable name for the embedded device tree blob
> >> changes, too, which is why common.h and fdtdec.c have tiny changes.
> >>
> >> Signed-off-by: Simon Goldschmidt <sgoldschm...@de.pepperl-fuchs.com>
> 
> This is breaking SPL build with CONFIG_SPL_MULTI_DTB_FIT enabled as it tries
> to call fdtgrep for shrinking all the dtbs in SPL_OF_LIST.

You're right. Sorry for that. I'll send a patch that fixes this.

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

Re: [U-Boot] [PATCH v3 12/15] env: Allow to build multiple environments in Kconfig

[Goldschmidt Simon]

On 01.02.2018 20:47, Maxime Ripard wrote:
> On Thu, Feb 01, 2018 at 11:06:14AM +0100, Simon Goldschmidt wrote:
>> On 23.01.2018 21:17, Maxime Ripard wrote:
>> > Now that we have everything in place in the code, let's allow to build
>> > multiple environments backend through Kconfig.
>> >
>> > Reviewed-by: Andre Przywara 
>> > Reviewed-by: Lukasz Majewski 
>> > Reviewed-by: Simon Glass 
>> > Signed-off-by: Maxime Ripard 
>>
>> I get a build error when enabling CONFIG_ENV_IS_IN_SPI_FLASH and
>> CONFIG_ENV_IS_IN_MMC at the same time.
> 
> Is that happening in any of the current defconfig right now? Or is it
> only when you add more environments?

No, this is with my own config. I'm trying out the new feature :-)

> 
>> The build error is in host tools, not in U-Boot or SPL itself. In fact, this
>> is not specific to CONFIG_ENV_IS_IN_SPI_FLASH but to the combination of
>> CONFIG_ENV_IS_IN_MMC and any of the environments marked as ENVCRC- in
>> tools/Makefile.
>>
>> The actual error is that the compiler does not know standard types in efi.h
>> and mmc.h, e.g.:
>> In file included from include/blk.h:11:0,
>>  from include/part.h:10,
>>  from include/mmc.h:16,
>>  from include/environment.h:168,
>>  from ./tools/../env/embedded.c:16,
>>  from tools/env/embedded.c:2:
>> include/efi.h:32:2: error: unknown type name ‘u8’
>>   u8 b[16];
>>   ^~
>>
>> I can't think of a correct fix right now...
> 
> I'm not sure what it could be either, that file looks like it would
> need a quite big rework, in order to be able to operate properly.
> 
> Do you actually need those? Maybe we can just disable those in Kconfig
> to forbid such a combination?

I planned to have the environment in both SPI flash and eMMC to
be able to use a common U-Boot binary accross multiple boards.

I don't need 'tools/envcrc' though. And this is where the build
error is.

Maybe we could disable the combination for 'tools/envcrc' only?

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

Re: [U-Boot] [PATCH] cmd: nvedit: env_get_f must check for env_get_char error codes

[Goldschmidt Simon]

On 02/02/2018 21:04, York Sun wrote:
> On 02/02/2018 10:51 AM, Maxime Ripard wrote:
> 
> 
> 
>
 Simon,

 This patch looks correct. But it doesn't fix NOR flash. Do you have plan
 to add .get_char function to other drivers? Without that function, we
 cannot get env variables before relocation.
>>>
>>> Ehrm, sorry  I don't plan to do that, no: my target seems to run fine
>>> without this.
>>>
>>> Given that only the eeprom and nvram env drivers support the get_char
>>> method, I don't know if this is widely used at all. Maybe a better fallback
>>> would be to just remove that get_char code path totally and always load from
>>> the internal (default) environment until the full environment is available
>>> (after relocation).
>>>
>>> After all, the environment variables loaded via get_char are not CRC checked
>>> at all. To me, this is another indication that this code is not really
>>> useful and should probably be removed.
>>
>> To be honest, I'm not really sure what get_char was here for in the
>> first place, so getting rid of it sounds like a good idea :)
>>
> 
> On almost all my boards, a variable hwconfig is read before relocation
> to determine DDR configuration. This has been broken. I don't mind you
> remove some dead code. But this is breaking almost all my boards booting
> from NOR flash.

When talking about removing a feature I meant the code that
exists for 2 env drivers only where single characters are loaded
from an external environment without checking its crc first.

The thing you seem to need is reading from the default environment
before relocation. This should not be removed, of course!

I might prepare a patch to better show what I mean...

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

Re: [U-Boot] [PATCH v3 09/15] env: Support multiple environments

[Goldschmidt Simon]

On 02/07/2018 21:18, York Sun wrote:
> On 02/07/2018 12:43 AM, Maxime Ripard wrote:
>> Hi,
>>
>> On Tue, Jan 30, 2018 at 11:02:49PM +, York Sun wrote:
>>> On 01/30/2018 12:16 PM, York Sun wrote:
 On 01/30/2018 11:40 AM, York Sun wrote:
> On 01/30/2018 12:19 AM, Simon Goldschmidt wrote:
>> On 23.01.2018 21:16, Maxime Ripard wrote:
>>> Now that we have everything in place to support multiple environment, 
>>> let's
>>> make sure the current code can use it.
>>>
>>> The priority used between the various environment is the same one that 
>>> was
>>> used in the code previously.
>>>
>>> At read / init times, the highest priority environment is going to be
>>> detected,
>>
>> Does priority handling really work here? Most env drivers seem to ignore
>> the return value of env_import and may thus return success although
>> importing the environment failed (only reading the data from the device
>> succeeded).
>>
>> This is from reading the code, I haven't had a chance to test this, yet.
>
> It is broken on my LS1043ARDB with simply NOR flash. I am trying to
> determine what went wrong.
>

 I found the problem. The variable "env_load_location" is static. It is
 probably not write-able during booting from flash. It is expected to be
 set during ENVOP_INIT. But if I print this variable, it has
 ENVL_UNKNOWN. I can make it work by moving env_load_location to global
 data structure.
>>
>> That would work for me.
>>
 That being said, this addition of multiple environments really slows
 down the booting process for me. I see every time env_get_char() is
 called, env_driver_lookup() runs. A simple call of env_get_f() gets
 slowed down dramatically. I didn't find out where the most time is spent
 yet.

 Does anyone else experience this unbearable slowness?

>>>
>>> I found the problem. In patch #3 in this series, the default get_char()
>>> was dropped so there is no driver for a plain NOR flash. A quick (and
>>> maybe dirty) fix is this
>>>
>>>
>>> diff --git a/env/env.c b/env/env.c
>>> index edfb575..210bae2 100644
>>> --- a/env/env.c
>>> +++ b/env/env.c
>>> @@ -159,7 +159,7 @@ int env_get_char(int index)
>>> int ret;
>>>
>>> if (!drv->get_char)
>>> -   continue;
>>> +   return *(uchar *)(gd->env_addr + index);
>>>
>>> if (!env_has_inited(drv->location))
>>> continue;
>>
>> And this too.
> 
> If you agree with this fix (actually revert your change earlier), I can
> send out a patch.

I still think we should get rid of the 'get_char' callback for
the env drivers. While that could have made sense for some boards
before the conversion to multiple environments (although I
doubt that, as the environment is *not* checked for
validity in this call), its meaning is totally lost when having
multiple env drivers active.

The whole purpose of multiple env drivers is that we select a
valid driver in the 'load' callback. How could we possibly know
that the 'get_char' callback of the highest prio env driver is
what we want?

I'd rather make 'env_get_char' weak and let boards decide if they
really want this behaviour.

A quick search through the current code base shows me *no* usage
of 'get_char' in nvram.c (CONFIG_SYS_NVRAM_ACCESS_ROUTINE is never
defined) and only 7 defconfigs that use ENV_IS_IN_EEPROM:

imx31_phycore_defconfig
imx31_phycore_eet_defconfig
km_kirkwood_128m16_defconfig
km_kirkwood_defconfig
km_kirkwood_pci_defconfig
mgcoge3un_defconfig
portl2_defconfig

Are these seven boards worth keeping this feature?

Simon

>>
>>> With this temporary fix, my flash chip works again and I can boot all
>>> the way up in a timely manner. I still don't like to call
>>> env_driver_lookup() thousands of times to get a simple env variable.
>>>
>>> Can Maxime post a quick post soon?
>>
>> Given that you already made all the debugging, and the patches, and I
>> have no way to test, I guess it would make more sense if you did it :)
> 
> Yes, I have tested on my boards. I will send out this patch.
> 
> York
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 2/2] env: Add back default action of get_char in env_get_char()

[Goldschmidt Simon]

Maxime, York,

I've just sent a patch that removes 'get_char' callback from the env 
drivers. This should restore the old behaviour we had before supporting 
multiple environment drivers (for all but eeprom, of course).



Simon


On 08.02.2018 23:10, Maxime Ripard wrote:

On Thu, Feb 08, 2018 at 10:52:20AM +0100, Simon Goldschmidt wrote:

On 08.02.2018 09:47, Maxime Ripard wrote:

On Wed, Feb 07, 2018 at 02:17:12PM -0800, York Sun wrote:

Commit 8a3a7e2270b3 ("env: Pass additional parameters to the env
lookup function") dropped the default action if driver doesn't have
get_char() defined. This causes failure to get environmental
variables from NOR flash. Add back this default action for now.

Signed-off-by: York Sun 
CC: Maxime Ripard 
---
Limited test on LS1043ARDB.

   env/env.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/env/env.c b/env/env.c
index edfb575..210bae2 100644
--- a/env/env.c
+++ b/env/env.c
@@ -159,7 +159,7 @@ int env_get_char(int index)
int ret;
if (!drv->get_char)
-   continue;
+   return *(uchar *)(gd->env_addr + index);

Thinking more about this, I think this would break the case where the
first environment in your list has no get_char method, but the second
might.


How can we decide which way is wanted? With your patch below, we might end
up loading chars from a low-prio environment (which is not CRC checked) in
the early boot stage. Later, we load the environment from another env driver
with higher priority.

Ah, right.


That's why I suggested removing the 'get_char' callback from the env drivers
:-) Early boot stage environment lookups would still work the old way
reading from 'gd->env_addr + index'.

If that works on York's board, I'm all in.

Maxime



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

[U-Boot] [PATCH] env: restore old env_get_char() behaviour

[Goldschmidt Simon]

With multiple environments, the 'get_char' callback for env
drivers does not really make sense any more because it is
only supported by two drivers (eeprom and nvram).

To restore single character loading for these drivers,
override 'env_get_char_spec'.

Signed-off-by: Simon Goldschmidt 
---
 env/eeprom.c  |  6 --
 env/env.c | 29 +++--
 env/nvram.c   |  8 
 include/environment.h | 11 ---
 4 files changed, 15 insertions(+), 39 deletions(-)

diff --git a/env/eeprom.c b/env/eeprom.c
index 55d19d9d99..63842d6ff3 100644
--- a/env/eeprom.c
+++ b/env/eeprom.c
@@ -61,7 +61,10 @@ static int eeprom_bus_write(unsigned dev_addr, unsigned 
offset,
return rcode;
 }
 
-static int env_eeprom_get_char(int index)
+/** Call this function from overridden env_get_char_spec() if you need
+ * this functionality.
+ */
+int env_eeprom_get_char(int index)
 {
uchar c;
unsigned int off = CONFIG_ENV_OFFSET;
@@ -228,7 +231,6 @@ static int env_eeprom_save(void)
 U_BOOT_ENV_LOCATION(eeprom) = {
.location   = ENVL_EEPROM,
ENV_NAME("EEPROM")
-   .get_char   = env_eeprom_get_char,
.load   = env_eeprom_load,
.save   = env_save_ptr(env_eeprom_save),
 };
diff --git a/env/env.c b/env/env.c
index 9a89832c1a..ab12606207 100644
--- a/env/env.c
+++ b/env/env.c
@@ -149,32 +149,17 @@ static struct env_driver *env_driver_lookup(enum 
env_operation op, int prio)
return drv;
 }
 
-int env_get_char(int index)
+__weak int env_get_char_spec(int index)
 {
-   struct env_driver *drv;
-   int prio;
+   return *(uchar *)(gd->env_addr + index);
+}
 
+int env_get_char(int index)
+{
if (gd->env_valid == ENV_INVALID)
return default_environment[index];
-
-   for (prio = 0; (drv = env_driver_lookup(ENVOP_GET_CHAR, prio)); prio++) 
{
-   int ret;
-
-   if (!drv->get_char)
-   continue;
-
-   if (!env_has_inited(drv->location))
-   continue;
-
-   ret = drv->get_char(index);
-   if (!ret)
-   return 0;
-
-   debug("%s: Environment %s failed to load (err=%d)\n", __func__,
- drv->name, ret);
-   }
-
-   return -ENODEV;
+   else
+   return env_get_char_spec(index);
 }
 
 int env_load(void)
diff --git a/env/nvram.c b/env/nvram.c
index 6f76fe4b8d..7cc62b631e 100644
--- a/env/nvram.c
+++ b/env/nvram.c
@@ -41,7 +41,10 @@ env_t *env_ptr = (env_t *)CONFIG_ENV_ADDR;
 #endif
 
 #ifdef CONFIG_SYS_NVRAM_ACCESS_ROUTINE
-static int env_nvram_get_char(int index)
+/** Call this function from overridden env_get_char_spec() if you need
+ * this functionality.
+ */
+int env_nvram_get_char(int index)
 {
uchar c;
 
@@ -113,9 +116,6 @@ static int env_nvram_init(void)
 U_BOOT_ENV_LOCATION(nvram) = {
.location   = ENVL_NVRAM,
ENV_NAME("NVRAM")
-#ifdef CONFIG_SYS_NVRAM_ACCESS_ROUTINE
-   .get_char   = env_nvram_get_char,
-#endif
.load   = env_nvram_load,
.save   = env_save_ptr(env_nvram_save),
.init   = env_nvram_init,
diff --git a/include/environment.h b/include/environment.h
index 6044b9e1b4..8696573d0d 100644
--- a/include/environment.h
+++ b/include/environment.h
@@ -217,17 +217,6 @@ struct env_driver {
const char *name;
enum env_location location;
 
-   /**
-* get_char() - Read a character from the environment
-*
-* This method is optional. If not provided, a default implementation
-* will read from gd->env_addr.
-*
-* @index: Index of character to read (0=first)
-* @return character read, or -ve on error
-*/
-   int (*get_char)(int index);
-
/**
 * load() - Load the environment from storage
 *
-- 
2.14.1
 
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v2 3/4] Convert socfpga: select CONFIG_HW_WATCHDOG support for ARCH_SOCFPGA

[Goldschmidt Simon]

On 09.02.2018 23:14, Lukasz Majewski wrote:

All Socfpga boards from ./include/configs/socfpga_* define
CONFIG_HW_WATCHDOG.
To ease CONFIG_HW_WATCHDOG conversion to Kconfig select it in
config ARCH_SOCFPGA (arch/arm/Kconfig) section.


I do have board configs where the internal watchdog is not used and 
should be disabled (because there's an external one). Also, given that 
this is an FPGA, I suppose having non-upstreamed boards is not uncommon.


I'm not too familiar with these settings though: can I leave the 
watchdog disabled when CONFIG_HW_WATCHDOG is off? Before, I just haven't 
enabled this in my own board config...


Simon



Signed-off-by: Lukasz Majewski 
---

Changes in v2:
- None

  arch/arm/Kconfig | 1 +
  include/configs/socfpga_arria10_socdk.h  | 2 --
  include/configs/socfpga_arria5_socdk.h   | 2 --
  include/configs/socfpga_cyclone5_socdk.h | 2 --
  include/configs/socfpga_de0_nano_soc.h   | 2 --
  include/configs/socfpga_de10_nano.h  | 2 --
  include/configs/socfpga_de1_soc.h| 2 --
  include/configs/socfpga_is1.h| 2 --
  include/configs/socfpga_mcvevk.h | 2 --
  include/configs/socfpga_sockit.h | 2 --
  include/configs/socfpga_socrates.h   | 2 --
  include/configs/socfpga_sr1500.h | 2 --
  include/configs/socfpga_vining_fpga.h| 2 --
  13 files changed, 1 insertion(+), 24 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 225f57e847..b4c79d6499 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -702,6 +702,7 @@ config ARCH_SOCFPGA
select DM_SPI_FLASH
select DM_SPI
select ENABLE_ARM_SOC_BOOT0_HOOK
+   select HW_WATCHDOG
select ARCH_EARLY_INIT_R
select ARCH_MISC_INIT
select SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION
diff --git a/include/configs/socfpga_arria10_socdk.h 
b/include/configs/socfpga_arria10_socdk.h
index 83718dd2c9..82bb48b277 100644
--- a/include/configs/socfpga_arria10_socdk.h
+++ b/include/configs/socfpga_arria10_socdk.h
@@ -9,8 +9,6 @@
  
  #include 
  
-#define CONFIG_HW_WATCHDOG

-
  /* Booting Linux */
  #define CONFIG_LOADADDR   0x0100
  #define CONFIG_SYS_LOAD_ADDR  CONFIG_LOADADDR
diff --git a/include/configs/socfpga_arria5_socdk.h 
b/include/configs/socfpga_arria5_socdk.h
index 6b6d54b97b..cd5aac65e9 100644
--- a/include/configs/socfpga_arria5_socdk.h
+++ b/include/configs/socfpga_arria5_socdk.h
@@ -8,8 +8,6 @@
  
  #include 
  
-#define CONFIG_HW_WATCHDOG

-
  /* Memory configurations */
  #define PHYS_SDRAM_1_SIZE 0x4000  /* 1GiB on SoCDK */
  
diff --git a/include/configs/socfpga_cyclone5_socdk.h b/include/configs/socfpga_cyclone5_socdk.h

index 018a0c3bb4..9c5bd648e3 100644
--- a/include/configs/socfpga_cyclone5_socdk.h
+++ b/include/configs/socfpga_cyclone5_socdk.h
@@ -8,8 +8,6 @@
  
  #include 
  
-#define CONFIG_HW_WATCHDOG

-
  /* Memory configurations */
  #define PHYS_SDRAM_1_SIZE 0x4000  /* 1GiB on SoCDK */
  
diff --git a/include/configs/socfpga_de0_nano_soc.h b/include/configs/socfpga_de0_nano_soc.h

index 275ed7ffeb..e5db00e366 100644
--- a/include/configs/socfpga_de0_nano_soc.h
+++ b/include/configs/socfpga_de0_nano_soc.h
@@ -8,8 +8,6 @@
  
  #include 
  
-#define CONFIG_HW_WATCHDOG

-
  /* Memory configurations */
  #define PHYS_SDRAM_1_SIZE 0x4000  /* 1GiB */
  
diff --git a/include/configs/socfpga_de10_nano.h b/include/configs/socfpga_de10_nano.h

index bb50fcf1ff..656af1104d 100644
--- a/include/configs/socfpga_de10_nano.h
+++ b/include/configs/socfpga_de10_nano.h
@@ -8,8 +8,6 @@
  
  #include 
  
-#define CONFIG_HW_WATCHDOG

-
  /* Memory configurations */
  #define PHYS_SDRAM_1_SIZE 0x4000  /* 1GiB */
  
diff --git a/include/configs/socfpga_de1_soc.h b/include/configs/socfpga_de1_soc.h

index 05975c9bde..f57b950425 100644
--- a/include/configs/socfpga_de1_soc.h
+++ b/include/configs/socfpga_de1_soc.h
@@ -8,8 +8,6 @@
  
  #include 
  
-#define CONFIG_HW_WATCHDOG

-
  /* Memory configurations */
  #define PHYS_SDRAM_1_SIZE 0x4000  /* 1GiB */
  
diff --git a/include/configs/socfpga_is1.h b/include/configs/socfpga_is1.h

index 46f5f135dd..dc318e50dc 100644
--- a/include/configs/socfpga_is1.h
+++ b/include/configs/socfpga_is1.h
@@ -9,8 +9,6 @@
  
  #include 
  
-#define CONFIG_HW_WATCHDOG

-
  /* Memory configurations */
  #define PHYS_SDRAM_1_SIZE 0x1000
  
diff --git a/include/configs/socfpga_mcvevk.h b/include/configs/socfpga_mcvevk.h

index 404f064e94..f13463b8b0 100644
--- a/include/configs/socfpga_mcvevk.h
+++ b/include/configs/socfpga_mcvevk.h
@@ -8,8 +8,6 @@
  
  #include 
  
-#define CONFIG_HW_WATCHDOG

-
  /* Memory configurations */
  #define PHYS_SDRAM_1_SIZE 0x4000  /* 1GiB on MCV */
  
diff --git a/include/configs/socfpga_sockit.h b/include/configs/socfpga_sockit.h

index b4f31c42c5..0bbc7e0105 100644
--- 

Re: [U-Boot] QSPI "sf probe ...", "sf read ..." on Altera SoC FPGA

[Goldschmidt Simon]

On Wed, 03/01/2018 14:51, Jagan Teki wrote:
>> There were already patches posted on this list by me and others, but
>> unfortunately they haven't made it into the repository, yet.
>>
>> Jagan, could you comment on the status of these fixes? I can search
>> for the patchwork items related if you want me to.
> 
> 2 out of 1 of this[1] have some discussion still going is it?
> 
> [1] https://patchwork.ozlabs.org/patch/838195/

No, that series should be dropped. I don't know if I can do anything
about that in patchwork though?

Let me check the patches from my upstreaming queue when I'm back at
work tomorrow. I'll send a list of patchwork items I needed to get
QSPI running on mach-socfpga.

Thanks,
Simon

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

Re: [U-Boot] [PATCH] ARM: socfpga: Convert to DM serial

[Goldschmidt Simon]

On 30.07.2018 16:04, Marek Vasut wrote:

On 07/30/2018 04:03 PM, Simon Goldschmidt wrote:


On 12.05.2018 22:28, Marek Vasut wrote:

Pull the serial port configuration from DT and use DM serial instead
of having the serial configuration in two places, DT and board config.

Signed-off-by: Marek Vasut 
Cc: Chin Liang See 
Cc: Dinh Nguyen 
---
   arch/arm/Kconfig | 3 +++
   arch/arm/dts/socfpga.dtsi    | 2 ++
   arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts | 1 +
   include/configs/socfpga_common.h | 8 
   4 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 532aa41a87..2012ac6410 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -737,6 +737,7 @@ config ARCH_SOCFPGA
   select ARCH_MISC_INIT
   select CPU_V7A
   select DM
+    select DM_SERIAL
   select ENABLE_ARM_SOC_BOOT0_HOOK
   select OF_CONTROL
   select SPL_LIBCOMMON_SUPPORT
@@ -746,11 +747,13 @@ config ARCH_SOCFPGA
   select SPL_NAND_SUPPORT if SPL_NAND_DENALI
   select SPL_OF_CONTROL
   select SPL_SERIAL_SUPPORT
+    select SPL_DM_SERIAL
   select SPL_SPI_FLASH_SUPPORT if SPL_SPI_SUPPORT
   select SPL_SPI_SUPPORT if DM_SPI
   select SPL_WATCHDOG_SUPPORT
   select SUPPORT_SPL
   select SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION_TYPE
+    select SYS_NS16550
   select SYS_THUMB_BUILD
   imply CMD_MTDPARTS
   imply CRC32_VERIFY
diff --git a/arch/arm/dts/socfpga.dtsi b/arch/arm/dts/socfpga.dtsi
index e64127fcb2..314449478d 100644
--- a/arch/arm/dts/socfpga.dtsi
+++ b/arch/arm/dts/socfpga.dtsi
@@ -737,6 +737,7 @@
   reg-shift = <2>;
   reg-io-width = <4>;
   clocks = <_sp_clk>;
+    clock-frequency = <1>;
   };
     uart1: serial1@ffc03000 {
@@ -746,6 +747,7 @@
   reg-shift = <2>;
   reg-io-width = <4>;
   clocks = <_sp_clk>;
+    clock-frequency = <1>;
   };
     rst: rstmgr@ffd05000 {
diff --git a/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts
b/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts
index b573d0e658..06b61cb0af 100644
--- a/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts
+++ b/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts
@@ -24,6 +24,7 @@
   };
      {
+    clock-frequency = <5000>;
   u-boot,dm-pre-reloc;
   status = "okay";
   };
diff --git a/include/configs/socfpga_common.h
b/include/configs/socfpga_common.h
index 54b9edc97c..a60da85499 100644
--- a/include/configs/socfpga_common.h
+++ b/include/configs/socfpga_common.h
@@ -173,14 +173,6 @@ unsigned int cm_get_qspi_controller_clk_hz(void);
    * Serial Driver
    */
   #define CONFIG_SYS_NS16550_SERIAL
-#define CONFIG_SYS_NS16550_REG_SIZE    -4
-#if defined(CONFIG_TARGET_SOCFPGA_GEN5)
-#define CONFIG_SYS_NS16550_COM1    SOCFPGA_UART0_ADDRESS
-#define CONFIG_SYS_NS16550_CLK    1
-#elif defined(CONFIG_TARGET_SOCFPGA_ARRIA10)
-#define CONFIG_SYS_NS16550_COM1    SOCFPGA_UART1_ADDRESS
-#define CONFIG_SYS_NS16550_CLK    5000
-#endif
     /*
    * USB


Unfortunately I saw this just now, but it seems this breaks GEN5 SPL? At
least git-bisect told me that 73172753f4f3351ed1c9d2f6586fc599ce4e728c
is the first bad commit.

I tested socfpga_socrates_defconfig on my socrates board.

Any idea what's wrong there?

Nope, this should work fine. Can you investigate ?


OK, I'll try.


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

Re: [U-Boot] [PATCH v5] u-boot: remove driver lookup loop from env_save()

[Goldschmidt Simon]

+ Tom:

I don't know via which tree this would go in. I think you took the last 
env changes directly?


v1..v4 are detached threads, I can send you links if you need them.


Thanks,

Simon


On 26.07.2018 16:02, Maxime Ripard wrote:

On Thu, Jul 26, 2018 at 07:16:36AM +0200, Simon Goldschmidt wrote:

Simon, Maxime,

any input on this? Via which tree would this be pushed? Should we CC Tom?

It would be nice if 2018.09 had this bug fixed (endless loop in env_save()
on error).

This looks good to me but Tom will probably end up this patch, so you
definitely want to cc him.

Maxime



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

Re: [U-Boot] [PATCH] ARM: socfpga: Convert to DM serial

[Goldschmidt Simon]

On 30.07.2018 16:04, Marek Vasut wrote:

On 07/30/2018 04:03 PM, Simon Goldschmidt wrote:


On 12.05.2018 22:28, Marek Vasut wrote:

Pull the serial port configuration from DT and use DM serial instead
of having the serial configuration in two places, DT and board config.

Signed-off-by: Marek Vasut 
Cc: Chin Liang See 
Cc: Dinh Nguyen 
---
   arch/arm/Kconfig | 3 +++
   arch/arm/dts/socfpga.dtsi    | 2 ++
   arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts | 1 +
   include/configs/socfpga_common.h | 8 
   4 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 532aa41a87..2012ac6410 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -737,6 +737,7 @@ config ARCH_SOCFPGA
   select ARCH_MISC_INIT
   select CPU_V7A
   select DM
+    select DM_SERIAL
   select ENABLE_ARM_SOC_BOOT0_HOOK
   select OF_CONTROL
   select SPL_LIBCOMMON_SUPPORT
@@ -746,11 +747,13 @@ config ARCH_SOCFPGA
   select SPL_NAND_SUPPORT if SPL_NAND_DENALI
   select SPL_OF_CONTROL
   select SPL_SERIAL_SUPPORT
+    select SPL_DM_SERIAL
   select SPL_SPI_FLASH_SUPPORT if SPL_SPI_SUPPORT
   select SPL_SPI_SUPPORT if DM_SPI
   select SPL_WATCHDOG_SUPPORT
   select SUPPORT_SPL
   select SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION_TYPE
+    select SYS_NS16550
   select SYS_THUMB_BUILD
   imply CMD_MTDPARTS
   imply CRC32_VERIFY
diff --git a/arch/arm/dts/socfpga.dtsi b/arch/arm/dts/socfpga.dtsi
index e64127fcb2..314449478d 100644
--- a/arch/arm/dts/socfpga.dtsi
+++ b/arch/arm/dts/socfpga.dtsi
@@ -737,6 +737,7 @@
   reg-shift = <2>;
   reg-io-width = <4>;
   clocks = <_sp_clk>;
+    clock-frequency = <1>;
   };
     uart1: serial1@ffc03000 {
@@ -746,6 +747,7 @@
   reg-shift = <2>;
   reg-io-width = <4>;
   clocks = <_sp_clk>;
+    clock-frequency = <1>;
   };
     rst: rstmgr@ffd05000 {
diff --git a/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts
b/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts
index b573d0e658..06b61cb0af 100644
--- a/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts
+++ b/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts
@@ -24,6 +24,7 @@
   };
      {
+    clock-frequency = <5000>;
   u-boot,dm-pre-reloc;
   status = "okay";
   };
diff --git a/include/configs/socfpga_common.h
b/include/configs/socfpga_common.h
index 54b9edc97c..a60da85499 100644
--- a/include/configs/socfpga_common.h
+++ b/include/configs/socfpga_common.h
@@ -173,14 +173,6 @@ unsigned int cm_get_qspi_controller_clk_hz(void);
    * Serial Driver
    */
   #define CONFIG_SYS_NS16550_SERIAL
-#define CONFIG_SYS_NS16550_REG_SIZE    -4
-#if defined(CONFIG_TARGET_SOCFPGA_GEN5)
-#define CONFIG_SYS_NS16550_COM1    SOCFPGA_UART0_ADDRESS
-#define CONFIG_SYS_NS16550_CLK    1
-#elif defined(CONFIG_TARGET_SOCFPGA_ARRIA10)
-#define CONFIG_SYS_NS16550_COM1    SOCFPGA_UART1_ADDRESS
-#define CONFIG_SYS_NS16550_CLK    5000
-#endif
     /*
    * USB


Unfortunately I saw this just now, but it seems this breaks GEN5 SPL? At
least git-bisect told me that 73172753f4f3351ed1c9d2f6586fc599ce4e728c
is the first bad commit.

I tested socfpga_socrates_defconfig on my socrates board.

Any idea what's wrong there?

Nope, this should work fine. Can you investigate ?


Ok, so after adding "u-boot,dm-pre-reloc" to uart0 in 
socfpga_cyclone5_socrates.dts, U-Boot works (combined with an old SPL). 
SPL still does not work. Any idea? How does SPL get the uart?


Thanks for any pointers.

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

Re: [U-Boot] [PATCH] ddr: altera: silence PHY calibration unless in debug mode

[Goldschmidt Simon]

On 01/11/2018 12:28, Marek Vasut wrote:
> On 01/11/2018 08:19 AM, Goldschmidt Simon wrote:
>> This driver has been using printf() including filename since it was
>> added. Convert to using debug() instead.
>>
>> Signed-off-by: Simon Goldschmidt <sgoldschm...@de.pepperl-fuchs.com>
> 
> Applied, thanks.

I can't see this one in u-boot-socfpga. Anything wrong here?

Regards,
Simon

>> ---
>>
>>  drivers/ddr/altera/sequencer.c | 8 
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/ddr/altera/sequencer.c b/drivers/ddr/altera/sequencer.c
>> index 6c6bd90e94..42e87b50d3 100644
>> --- a/drivers/ddr/altera/sequencer.c
>> +++ b/drivers/ddr/altera/sequencer.c
>> @@ -3534,7 +3534,7 @@ static void debug_mem_calibrate(int pass)
>>   u32 debug_info;
>>
>>   if (pass) {
>> - printf("%s: CALIBRATION PASSED\n", __FILE__);
>> + debug("%s: CALIBRATION PASSED\n", __FILE__);
>>
>>   gbl->fom_in /= 2;
>>   gbl->fom_out /= 2;
>> @@ -3553,7 +3553,7 @@ static void debug_mem_calibrate(int pass)
>>   writel(debug_info, _mgr_cfg->cal_debug_info);
>>   writel(PHY_MGR_CAL_SUCCESS, _mgr_cfg->cal_status);
>>   } else {
>> - printf("%s: CALIBRATION FAILED\n", __FILE__);
>> + debug("%s: CALIBRATION FAILED\n", __FILE__);
<>
>>   debug_info = gbl->error_stage;
>>   debug_info |= gbl->error_substage << 8;
>> @@ -3570,7 +3570,7 @@ static void debug_mem_calibrate(int pass)
>>   writel(debug_info, _reg_file->failing_stage);
>>   }
>>
>> - printf("%s: Calibration complete\n", __FILE__);
>> + debug("%s: Calibration complete\n", __FILE__);
>>  }
>>
>>  /**
>> @@ -3741,7 +3741,7 @@ int sdram_calibration_full(void)
>>
>>   initialize_tracking();
>>
>> - printf("%s: Preparing to start memory calibration\n", __FILE__);
>> + debug("%s: Preparing to start memory calibration\n", __FILE__);
>>
>>   debug("%s:%d\n", __func__, __LINE__);
>>   debug_cond(DLEVEL >= 1,
>>
> 
> 
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot