Re: [U-Boot] [ANN] git server, FTP server

2015-07-15 Thread thomas.langer 
Hello Wolfgang,

Wolfgang Denk wrote on 2015-07-15:

 2. For some time now, we provide not only the classic FTP server for
download of the U-Boot release tarballs, but also a public
directory in the Amazon Cloud Drive [1].  The ACD is supposed to
provide much better connectivity (especially for non-european
users) and much higher download rates.

So far, I have received zero feedback about this.  I would like to
hear about your experiences (good and bad), and what you think
about dropping the FTP server completely.

In addition to the mail from Andy:

I tried to find a way to download a file with wget or a similar tool, which 
would be used by a distribution builder (like Yocto, Buildroot, OpenWrt, ...).
Up to now I failed, as the download button depends on javascript and the 
generated download link for the file [1] has no indication of the file name
(and because of templink inside I am also not sure, how constant this would 
be).

If somebody can provide a way to download the tarballs by name from a 
command line I would accept the ACD as a replacement.
Otherwise I would prefer a server (like ftp) with some kind of stable URLs.

Best Regards,
Thomas

[1] 
https://content-na.drive.amazonaws.com/cdproxy/templink/No5u9y7HLlaVIEwnz3VGucNfORmh8RrskPQGcuDGFkkE0Xnc3?download=true

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

Re: [U-Boot] [PATCH] zynq: timer: Fix wrong timer calculation

2015-04-16 Thread thomas.langer 
Hello Michal,

Michal Simek wrote on 2015-04-16:
 From: Siva Durga Prasad Paladugu siva.durga.palad...@xilinx.com
 
 Fix wrong timer calculation in get_timer_masked incase of overflow. This
 fixes the issue of getting wrong time from get_timer() calls.
 
 Signed-off-by: Siva Durga Prasad Paladugu siva...@xilinx.com
 Signed-off-by: Michal Simek michal.si...@xilinx.com
 ---
 
  arch/arm/cpu/armv7/zynq/timer.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)
 diff --git a/arch/arm/cpu/armv7/zynq/timer.c
 b/arch/arm/cpu/armv7/zynq/timer.c index 303dbcfceafb..5ed9642df9b3
 100644 --- a/arch/arm/cpu/armv7/zynq/timer.c +++
 b/arch/arm/cpu/armv7/zynq/timer.c @@ -93,7 +93,9 @@ ulong
 get_timer_masked(void)
   gd-arch.tbl += gd-arch.lastinc - now;
   } else {
   /* We have an overflow ... */
 - gd-arch.tbl += gd-arch.lastinc + TIMER_LOAD_VAL - now + 1;
 + gd-arch.tbl += gd-arch.lastinc + (TIMER_LOAD_VAL /
 + (gd-arch.timer_rate_hz / CONFIG_SYS_HZ))
 -
 + now + 1;
   }
   gd-arch.lastinc = now;

I had similar problems with the MIPS timer code some time ago.
I solved it by switching to the common timer implementation:
http://git.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=commitdiff;h=a18a477147ce2493ef9ee93b8981b34929fc48a5

I  don't know the arm/zynq hardware in detail, but if you use some free running 
counter for this,
the switch should be simple.

As you can see from the diff, the previous code was also doing some complex 
checks to detect an overflow.
With the common code, it could be reduced to two simple  functions (where one 
could be removed, 
if we update all configs to define a different macro for the timer rate).

Best Regards,
Thomas

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

Re: [U-Boot] [PATCH 18/20] Add Chrome OS config header

2015-02-26 Thread thomas.langer 
Hello Stephen,

 diff --git a/include/configs/chromeos.h b/include/configs/chromeos.h
 
 +/* Stringify a token */
 +#ifndef STRINGIFY
 +#define _STRINGIFY(x)   #x
 +#define STRINGIFY(x)_STRINGIFY(x)
 +#endif
 
 Shouldn't that be in some common header so it isn't ever duplicated?
 
U-Boot has already __stringify(), so this should not be necessary at all.

Best Regards,
Thomas

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

[U-Boot] [PATCH] MIPS: use common code from lib/time.c

2015-01-14 Thread thomas.langer 
From: Thomas Langer thomas.lan...@lantiq.com

The common code just needs the C0_COUNT as free running counter,
without the need of writing and checking C0_COMPARE.

The function get_tbclk() is still implemented here instead of changing
all places of CONFIG_SYS_MIPS_TIMER_FREQ to CONFIG_SYS_TIMER_RATE.

The change was tested on a MIPS32 system, but as the MIPS64 code
was/is the same, this should be no problem.

Signed-off-by: Thomas Langer thomas.lan...@lantiq.com
---
 arch/mips/cpu/mips32/time.c |   59 +++
 arch/mips/cpu/mips64/time.c |   59 +++
 2 files changed, 8 insertions(+), 110 deletions(-)

diff --git a/arch/mips/cpu/mips32/time.c b/arch/mips/cpu/mips32/time.c
index 386f45a..553da5f 100644
--- a/arch/mips/cpu/mips32/time.c
+++ b/arch/mips/cpu/mips32/time.c
@@ -8,63 +8,12 @@
 #include common.h
 #include asm/mipsregs.h
 
-static unsigned long timestamp;
-
-/* how many counter cycles in a jiffy */
-#define CYCLES_PER_JIFFY   \
-   (CONFIG_SYS_MIPS_TIMER_FREQ + CONFIG_SYS_HZ / 2) / CONFIG_SYS_HZ
-
-/*
- * timer without interrupts
- */
-
-int timer_init(void)
-{
-   /* Set up the timer for the first expiration. */
-   write_c0_compare(read_c0_count() + CYCLES_PER_JIFFY);
-
-   return 0;
-}
-
-ulong get_timer(ulong base)
-{
-   unsigned int count;
-   unsigned int expirelo = read_c0_compare();
-
-   /* Check to see if we have missed any timestamps. */
-   count = read_c0_count();
-   while ((count - expirelo)  0x7fff) {
-   expirelo += CYCLES_PER_JIFFY;
-   timestamp++;
-   }
-   write_c0_compare(expirelo);
-
-   return timestamp - base;
-}
-
-void __udelay(unsigned long usec)
+unsigned long notrace timer_read_counter(void)
 {
-   unsigned int tmo;
-
-   tmo = read_c0_count() + (usec * (CONFIG_SYS_MIPS_TIMER_FREQ / 100));
-   while ((tmo - read_c0_count())  0x7fff)
-   /*NOP*/;
+   return read_c0_count();
 }
 
-/*
- * This function is derived from PowerPC code (read timebase as long long).
- * On MIPS it just returns the timer value.
- */
-unsigned long long get_ticks(void)
-{
-   return get_timer(0);
-}
-
-/*
- * This function is derived from PowerPC code (timebase clock frequency).
- * On MIPS it returns the number of timer ticks per second.
- */
-ulong get_tbclk(void)
+ulong notrace get_tbclk(void)
 {
-   return CONFIG_SYS_HZ;
+   return CONFIG_SYS_MIPS_TIMER_FREQ;
 }
diff --git a/arch/mips/cpu/mips64/time.c b/arch/mips/cpu/mips64/time.c
index 0497acf..553da5f 100644
--- a/arch/mips/cpu/mips64/time.c
+++ b/arch/mips/cpu/mips64/time.c
@@ -8,63 +8,12 @@
 #include common.h
 #include asm/mipsregs.h
 
-static unsigned long timestamp;
-
-/* how many counter cycles in a jiffy */
-#define CYCLES_PER_JIFFY\
-   (CONFIG_SYS_MIPS_TIMER_FREQ + CONFIG_SYS_HZ / 2) / CONFIG_SYS_HZ
-
-/*
- * timer without interrupts
- */
-
-int timer_init(void)
-{
-   /* Set up the timer for the first expiration. */
-   write_c0_compare(read_c0_count() + CYCLES_PER_JIFFY);
-
-   return 0;
-}
-
-ulong get_timer(ulong base)
-{
-   unsigned int count;
-   unsigned int expirelo = read_c0_compare();
-
-   /* Check to see if we have missed any timestamps. */
-   count = read_c0_count();
-   while ((count - expirelo)  0x7fff) {
-   expirelo += CYCLES_PER_JIFFY;
-   timestamp++;
-   }
-   write_c0_compare(expirelo);
-
-   return timestamp - base;
-}
-
-void __udelay(unsigned long usec)
+unsigned long notrace timer_read_counter(void)
 {
-   unsigned int tmo;
-
-   tmo = read_c0_count() + (usec * (CONFIG_SYS_MIPS_TIMER_FREQ / 100));
-   while ((tmo - read_c0_count())  0x7fff)
-   /*NOP*/;
+   return read_c0_count();
 }
 
-/*
- * This function is derived from PowerPC code (read timebase as long long).
- * On MIPS it just returns the timer value.
- */
-unsigned long long get_ticks(void)
-{
-   return get_timer(0);
-}
-
-/*
- * This function is derived from PowerPC code (timebase clock frequency).
- * On MIPS it returns the number of timer ticks per second.
- */
-ulong get_tbclk(void)
+ulong notrace get_tbclk(void)
 {
-   return CONFIG_SYS_HZ;
+   return CONFIG_SYS_MIPS_TIMER_FREQ;
 }
-- 
1.7.10.3

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

Re: [U-Boot] [PATCH] cmd_sf: add handler for +len arg for all commands

2014-10-05 Thread thomas.langer 
Hello Maxime,

can you explain the usecase?
I think, only the erase is executed per sector, all other commands are working 
fine with a byte oriented length.

Best regards,
Thomas

---
Sent from my phone.

Maxime Hadjinlian maxime.hadjinl...@gmail.com hat geschrieben:


This patch adds [+]len handler for the all the commands that will
automatically round up the requested erase length to the flash's
sector_size.
It was previously only available for the erase command.

Signed-off-by: Maxime Hadjinlian maxime.hadjinl...@gmail.com
---

 common/cmd_sf.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/common/cmd_sf.c b/common/cmd_sf.c
index c60e8d1..d5af3fe 100644
--- a/common/cmd_sf.c
+++ b/common/cmd_sf.c
@@ -245,8 +245,8 @@ static int do_spi_flash_read_write(int argc, char * const 
argv[])
offset = simple_strtoul(argv[2], endp, 16);
if (*argv[2] == 0 || *endp != 0)
return -1;
-   len = simple_strtoul(argv[3], endp, 16);
-   if (*argv[3] == 0 || *endp != 0)
+   len = sf_parse_len_arg(argv[3], len);
+   if (ret != 1)
return -1;

/* Consistency checking */
@@ -529,15 +529,15 @@ usage:
 U_BOOT_CMD(
sf, 5,  1,  do_spi_flash,
SPI flash sub-system,
+   Note: `+len' round up `len' to block size\n
probe [[bus:]cs] [hz] [mode]   - init flash device on given SPI bus\n
 and chip select\n
-   sf read addr offset len- read `len' bytes starting at\n
+   sf read addr offset [+]len - read `len' bytes starting at\n
 `offset' to memory at `addr'\n
-   sf write addr offset len   - write `len' bytes from memory\n
+   sf write addr offset [+]len- write `len' bytes from memory\n
 at `addr' to flash at `offset'\n
sf erase offset [+]len - erase `len' bytes from `offset'\n
-`+len' round up `len' to block size\n
-   sf update addr offset len  - erase and write `len' bytes from 
memory\n
+   sf update addr offset [+]len   - erase and write `len' bytes from 
memory\n
 at `addr' to flash at `offset'
SF_TEST_HELP
 );
--
2.1.1

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

Re: [U-Boot] [PATCH] cmd_sf: add handler for +len arg for all commands

2014-10-05 Thread thomas.langer 
 On Sunday, October 05, 2014 at 08:40:26 PM, Maxime Hadjinlian wrote:
  Hi Thomas, all,
 
  On Sun, Oct 5, 2014 at 7:43 PM,  thomas.lan...@lantiq.com wrote:
   Hello Maxime,
  
   can you explain the usecase?
   I think, only the erase is executed per sector, all other commands are
   working fine with a byte oriented length.
 
  I need to write a file that is downloaded through TFTP. So I can get
  the filesize through the variable of the same name, but if it's not
  rounded, the write command may fail.
  I can save the filesize in another variable, but at next boot, when I
  need to read this file, I can't read the file, since I only know it's
  size in byte, I need to be able to round it again.
 
 I wonder, do all SPI flashes need to do sector-aligned writes ?

All the serial flashes I have seen so far do support reading and writing with 
any
length, independent from the erase size.
Otherwise the current implementation of env_sf.c would also not work.

 
 Best regards,
 Marek Vasut

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

Re: [U-Boot] [PATCH 4/7] sunxi: Add CONFIG_MACPWR option

2014-07-28 Thread thomas.langer 
Hello Hans,

Hans de Goede wrote on 2014-07-27:
 On some boards the phy needs to be powered up through a gpio, add
 support for this.
 

 @@ -129,6 +129,11 @@ int cpu_eth_init(bd_t *bis)
  {
   __maybe_unused int rc;
 +#ifdef CONFIG_MACPWR

If this is powering a phy, maybe CONFIG_PHYPWR or similar is a better name?
Because PHY and MAC are different things!
And maybe adding GPIO to the name to indicate that the value is a GPIO number?

All of these should be part of the description in the README,
which each CONFIG_ option requires.

 + gpio_direction_output(CONFIG_MACPWR, 1);
 + mdelay(200);
 +#endif
 +
  #ifdef CONFIG_SUNXI_EMAC
   rc = sunxi_emac_initialize(bis);
   if (rc  0) {

Best Regards,
Thomas
---
There are two hard things in computer science: cache invalidation, naming 
things, and off-by-one errors.
---


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

Re: [U-Boot] [PATCH] lib: Add CONFIG_FDT_IGNORE_FIXUP_MEMORY_NODE

2014-04-08 Thread thomas.langer 
Hello Tom,

 Note that ARM provides arch_fixup_memory_node to make sure we have all
 of the bank information populated and then calls fdt_fixup_memory_banks,
 while PowerPC just calls fdt_fixup_memory which calls banks with a '1'
 for number of banks.  MIPS (and everyone else) isn't doing anything
 about this atm, but probably should.

I assume the main reason this is not done for MIPS is the missing support for
providing devicetrees from U-Boot.

Daniel: Do you know if there is any activity to add this?

Best Regards,
Thomas

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

Re: [U-Boot] [PATCH] board_r - fixup functions table after relocation

2014-01-15 Thread thomas.langer 
Hello Alexey,

Alexey Brodkin wrote on 2014-01-15:
 init_sequence_r is just an array that consists of compile-time
 adresses of init functions. Since this is basically an array of integers
 (pointers to void to be more precise) it won't be modified during
 relocation - it will be just copied to new location as it is.
 
 As a consequence on execution after relocation initcall_run_list will
 be jumping to pre-relocation addresses. As long as we don't overwrite
 pre-relocation memory area init calls are executed correctly. But still
 it is dangerous because after relocation we don't expect initially used
 memory to stay untouched.
 
 Signed-off-by: Alexey Brodkin abrod...@synopsys.com
 
 Cc: Tom Rini tr...@ti.com
 Cc: Simon Glass s...@chromium.org
 Cc: Masahiro Yamada yamad...@jp.panasonic.com
 Cc: Doug Anderson diand...@chromium.org
 ---
  common/board_r.c | 5 +
  1 file changed, 5 insertions(+)
 
 diff --git a/common/board_r.c b/common/board_r.c
 index 86ca1cb..8f45943 100644
 --- a/common/board_r.c
 +++ b/common/board_r.c
 @@ -903,9 +903,14 @@ init_fnc_t init_sequence_r[] = {
 
  void board_init_r(gd_t *new_gd, ulong dest_addr)
  {
 + int i;
  #ifndef CONFIG_X86
   gd = new_gd;
  #endif
 + /* Fixup table after relocation */
 + for (i = 0; i  sizeof(init_sequence_r)/sizeof(void *); i++)
 + init_sequence_r[i] += gd-reloc_off;
 +

I think this is only required/possible for architectures which define
CONFIG_NEEDS_MANUAL_RELOC, others don't have gd-reloc_off

   if (initcall_run_list(init_sequence_r))
   hang();
 

Best Regards,
Thomas
---
There are two hard things in computer science: cache invalidation, naming 
things, and off-by-one errors.
---

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

Re: [U-Boot] [PATCH] board_r - fixup functions table after relocation

2014-01-15 Thread thomas.langer 
Alexey Brodkin wrote on 2014-01-15:
 On Wed, 2014-01-15 at 11:27 +, thomas.lan...@lantiq.com wrote:
 I think this is only required/possible for architectures which define
 CONFIG_NEEDS_MANUAL_RELOC, others don't have gd-reloc_off
 
 if (initcall_run_list(init_sequence_r))
 hang();
 
 Hi Thomas,
 
 I think it's a generic item for all boards that use common/board_r.c
 and common/board_f.c. I.e. have CONFIG_BOARD_EARLY_INIT_F and
 CONFIG_BOARD_EARLY_INIT_R defined.
 
 I see that in common/board_f.c it is used for example in
 setup_reloc() and this function is executed regardless platform.
 
Hello Alexey,

it seems that CONFIG_SYS_GENERIC_BOARD is not used by any architecture
without CONFIG_NEEDS_MANUAL_RELOC, so your patch is okay.

Sorry for the noise.

 Regards,
 Alexey

Best Regards,
Thomas
---
There are two hard things in computer science: cache invalidation, naming 
things, and off-by-one errors.
---


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

Re: [U-Boot] [PATCH 10/34] spi: Add zynq qspi controller driver

2013-11-05 Thread thomas.langer 
Hello Jagan,

I have some comments and questions:

Am 05.11.2013 18:51, schrieb Jagannadha Sutradharudu Teki:
 +
 +/* Definitions of the flash commands - Flash insts in ascending order */
 +#define ZYNQ_QSPI_FLASH_INST_WRSR0x01/* Write status register */
 +#define ZYNQ_QSPI_FLASH_INST_PP  0x02/* Page program */
 +#define ZYNQ_QSPI_FLASH_INST_WRDS0x04/* Write disable */
 +#define ZYNQ_QSPI_FLASH_INST_RDSR1   0x05/* Read status register 1 */
 +#define ZYNQ_QSPI_FLASH_INST_WREN0x06/* Write enable */
 +#define ZYNQ_QSPI_FLASH_INST_AFR 0x0B/* Fast read data bytes */
 +#define ZYNQ_QSPI_FLASH_INST_BE_4K   0x20/* Erase 4KiB block */
 +#define ZYNQ_QSPI_FLASH_INST_RDSR2   0x35/* Read status register 2 */
 +#define ZYNQ_QSPI_FLASH_INST_BE_32K  0x52/* Erase 32KiB block */
 +#define ZYNQ_QSPI_FLASH_INST_RDID0x9F/* Read JEDEC ID */
 +#define ZYNQ_QSPI_FLASH_INST_SE  0xD8/* Sector erase 
 (usually 64KB)*/
 +
Why needs the spi controller this list of flash commands?
It is the job of the flash driver to handle this, the spi controller
only forwards this to the devices!

 +
 +/* List of all the QSPI instructions and its format */
 +static struct zynq_qspi_inst_format flash_inst[] = {
 + {ZYNQ_QSPI_FLASH_INST_WRSR, 1, ZYNQ_QSPI_TXD_00_01_OFFSET},
 + {ZYNQ_QSPI_FLASH_INST_PP, 4, ZYNQ_QSPI_TXD_00_00_OFFSET},
 + {ZYNQ_QSPI_FLASH_INST_WRDS, 1, ZYNQ_QSPI_TXD_00_01_OFFSET},
 + {ZYNQ_QSPI_FLASH_INST_RDSR1, 1, ZYNQ_QSPI_TXD_00_01_OFFSET},
 + {ZYNQ_QSPI_FLASH_INST_WREN, 1, ZYNQ_QSPI_TXD_00_01_OFFSET},
 + {ZYNQ_QSPI_FLASH_INST_AFR, 1, ZYNQ_QSPI_TXD_00_01_OFFSET},
 + {ZYNQ_QSPI_FLASH_INST_BE_4K, 4, ZYNQ_QSPI_TXD_00_00_OFFSET},
 + {ZYNQ_QSPI_FLASH_INST_RDSR2, 1, ZYNQ_QSPI_TXD_00_01_OFFSET},
 + {ZYNQ_QSPI_FLASH_INST_BE_32K, 4, ZYNQ_QSPI_TXD_00_00_OFFSET},
 + {ZYNQ_QSPI_FLASH_INST_RDID, 1, ZYNQ_QSPI_TXD_00_01_OFFSET},
 + {ZYNQ_QSPI_FLASH_INST_SE, 4, ZYNQ_QSPI_TXD_00_00_OFFSET},
 + /* Add all the instructions supported by the flash device */
 +};
Is this table used to encode which parts needs DUAL or QUAD transfers?
And are you sure the table is complete? The flash drivers don't use more
instructions?
What happens if some device is connected, which is no flash?
 +
 +int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout,
 + void *din, unsigned long flags)
 +{
 + struct zynq_qspi_slave *zslave = to_zynq_qspi_slave(slave);
 + u32 len = bitlen / 8, tx_tvl;
 + u32 buf, status;
 +
 + debug(spi_xfer: bus:%i cs:%i bitlen:%i len:%i flags:%lx\n,
 +   slave-bus, slave-cs, bitlen, len, flags);
 +
 + if (bitlen == 0)
 + return -1;
 +
 + if (bitlen % 8) {
 + debug(spi_xfer: Non byte aligned SPI transfer\n);
 + return -1;
 + }
 +
 + if (flags  SPI_XFER_BEGIN)
 + spi_cs_activate(slave);
 +
 + zslave-tx_len = len;
 + zslave-rx_len = len;
 + zslave-tx_buf = dout;
 + zslave-rx_buf = din;
 + while (zslave-rx_len  0) {
 + /* Write the data into TX FIFO - tx threshold is fifo_depth */
 + tx_tvl = 0;
 + while ((tx_tvl  zslave-fifo_depth)  zslave-tx_len) {
 + if (zynq_qspi_process_tx(zslave)  0) {
 + flags |= SPI_XFER_END;
 + goto out;
 + }
 + tx_tvl++;
 + }
 +
 + /* Check TX FIFO completion */
 + if (zynq_qspi_check_txfifo(zslave)  0) {
 + flags |= SPI_XFER_END;
 + goto out;
 + }
 +
 + /* Read the data from RX FIFO */
 + status = readl(zslave-base-isr);
 + while (status  ZYNQ_QSPI_IXR_RXNEMPTY_MASK) {
 + buf = readl(zslave-base-rxdr);
 + if (zslave-rx_len  4)
 + zynq_qspi_read(zslave, buf, zslave-rx_len);
 + else
 + zynq_qspi_read(zslave, buf, 4);
 + status = readl(zslave-base-isr);
 + }
 + }
 +
 +out:
 + if (flags  SPI_XFER_END)
 + spi_cs_deactivate(slave);
 +
 + return 0;
 +}
In this function I miss the parts, where the caller (e.g. the flash
driver) tells the controller to transfer some parts in DUAL or QUAD mode!

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

Re: [U-Boot] [PATCH v4 31/36] sf: Add extended read commands support

2013-09-26 Thread thomas.langer 
Hello Jagan,

it seems an almost ready patch for m25p80 driver in the kernel was posted today:
[PATCHv2] drivers: mtd: devices: Add quad read support.
http://thread.gmane.org/gmane.linux.drivers.mtd/48552/focus=48557

Please see how there in the function m25p80_quad_read
the rx_nbits in the transfer structure is set.

Best Regards,
Thomas

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

Re: [U-Boot] [PATCH v4 31/36] sf: Add extended read commands support

2013-09-25 Thread thomas.langer 
Hello Jagan,

 From: Jagan Teki [mailto:jagannadh.t...@gmail.com]
 Sent: Wednesday, September 25, 2013 11:36 AM
 To: Langer Thomas (LQDE RD ST PON SW)
 Cc: Jagannadha Sutradharudu Teki; Tom Rini; jaganna; u-boot@lists.denx.de;
 Todd Legler (tlegler); Willis Max; Syed Hussain; Sascha Silbe
 Subject: Re: [U-Boot] [PATCH v4 31/36] sf: Add extended read commands
 support
 
 On Wed, Sep 25, 2013 at 1:40 AM,  thomas.lan...@lantiq.com wrote:
  Hello Jagan,
 
  Am 24.09.2013 20:36, schrieb Jagannadha Sutradharudu Teki:
  diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
  index ea39d1a..0ac9fab 100644
  --- a/drivers/spi/spi.c
  +++ b/drivers/spi/spi.c
  @@ -7,6 +7,7 @@
   #include common.h
   #include malloc.h
   #include spi.h
  +#include spi_flash.h
 
   void *spi_do_alloc_slave(int offset, int size, unsigned int bus,
 unsigned int cs)
  @@ -20,6 +21,7 @@ void *spi_do_alloc_slave(int offset, int size,
 unsigned int bus,
slave = (struct spi_slave *)(ptr + offset);
slave-bus = bus;
slave-cs = cs;
  + slave-rd_cmd = CMD_READ_ARRAY_FAST;
 
  This is SPI code, not SF! The spi layer should not know such details of
  the slave!
  What about EEPROMs or other SPI slaves, which are NOT spi_flash???
  Examples (just searched for includes of spi.h):
  board/bf527-ezkit/video.c
  drivers/net/enc28j60.c
 
  Please ensure that the layers are not mixed up!
  SPI is an interface type and SF is ONLY ONE user of this interface!
 
 I understand your concern, but here the point is for discovering the
 command set.
 slave-rd_cmd = CMD_READ_ARRAY_FAST;
 is a default controller supported fast read.
 
 spi_flash layer will discover the respective rd_cmd based slave and flash, if
 slave doesn't have any commands to list, means not support
 extended/quad then these fields are filled in spi.c
 
 there is nothing harm for respective drivers or code.

But I think this is the wrong approach!
Why should the spi controller care about, what devices type is connected?
And the CMD is a SF specific thing!
I agree, that the controller should provide information about his features,
but this should only be something like tx_width=4 and rx_width=4,
which would tell the SF layer that quad-io is possible!

No details of serial-flashes are necessary for this!

Please look up similar discussions on the linux-mtd and linux-spi mailing lists!

 --
 Thanks,
 Jagan.

Best Regards,
Thomas

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

Re: [U-Boot] [PATCH v4 31/36] sf: Add extended read commands support

2013-09-25 Thread thomas.langer 
Hello Jagan,

Jagan Teki wrote on 2013-09-25:
 On Wed, Sep 25, 2013 at 3:14 PM,  thomas.lan...@lantiq.com wrote:
 Hello Jagan,
 
 From: Jagan Teki [mailto:jagannadh.t...@gmail.com] Sent: Wednesday,
 September 25, 2013 11:36 AM To: Langer Thomas (LQDE RD ST PON SW) Cc:
 Jagannadha Sutradharudu Teki; Tom Rini; jaganna; u-
 b...@lists.denx.de; Todd Legler (tlegler); Willis Max; Syed Hussain;
 Sascha Silbe Subject: Re: [U-Boot] [PATCH v4 31/36] sf: Add extended
 read commands support
 
 On Wed, Sep 25, 2013 at 1:40 AM,  thomas.lan...@lantiq.com wrote:
 Hello Jagan,
 
 Am 24.09.2013 20:36, schrieb Jagannadha Sutradharudu Teki:
 diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
 index ea39d1a..0ac9fab 100644
 --- a/drivers/spi/spi.c
 +++ b/drivers/spi/spi.c
 @@ -7,6 +7,7 @@
  #include common.h
  #include malloc.h
  #include spi.h
 +#include spi_flash.h
 
  void *spi_do_alloc_slave(int offset, int size, unsigned int bus,
unsigned int cs)
 @@ -20,6 +21,7 @@ void *spi_do_alloc_slave(int offset, int size,
 unsigned int bus,
   slave = (struct spi_slave *)(ptr + offset);
   slave-bus = bus;
   slave-cs = cs;
 + slave-rd_cmd = CMD_READ_ARRAY_FAST;
 
 This is SPI code, not SF! The spi layer should not know such details of
 the slave!
 What about EEPROMs or other SPI slaves, which are NOT spi_flash???
 Examples (just searched for includes of spi.h):
 board/bf527-ezkit/video.c
 drivers/net/enc28j60.c
 
 Please ensure that the layers are not mixed up!
 SPI is an interface type and SF is ONLY ONE user of this interface!
 
 I understand your concern, but here the point is for discovering the
 command set.
 slave-rd_cmd = CMD_READ_ARRAY_FAST;
 is a default controller supported fast read.
 
 spi_flash layer will discover the respective rd_cmd based slave and
 flash, if slave doesn't have any commands to list, means not support
 extended/quad then these fields are filled in spi.c
 
 there is nothing harm for respective drivers or code.
 
 But I think this is the wrong approach!
 Why should the spi controller care about, what devices type is connected?
 And the CMD is a SF specific thing!
 I agree, that the controller should provide information about his features,
 but this should only be something like tx_width=4 and rx_width=4,
 which would tell the SF layer that quad-io is possible!
 
 No details of serial-flashes are necessary for this!
 Yes, no issues.
 I can directly assign to flash side while discovering commends.

I don't understand this sentence. Do you mean commands?
Who will discover the commands?

The SPI controller does not know about the meaning of commands!
The controller only knows I must send this block of data and split it on 1/2/4 
lines
(or similar for other transfers).

Maybe your specific controller behaves in another way, but this is not the 
standard
and you should not force this into the interface.

And another doubt: The might be different commands for dual/quad read/write,
depending on the flash manufacturer. With your solution, the controller drivers
would need these details in advance! Or at least need to be updated on each new
command, which needs to be supported.

 --
 Thanks,
 Jagan.

Best Regards,
Thomas


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

Re: [U-Boot] [PATCH v4 00/36] sf: Add common probe and extended/quad read/write cmds support

2013-09-25 Thread thomas.langer 
Hello Jagan,

Jagan Teki wrote on 2013-09-25:

 Thanks for your comments, see below
 
 On Wed, Sep 25, 2013 at 1:02 AM,  thomas.lan...@lantiq.com wrote:
 Hello Jagan,
 
 Am 24.09.2013 20:38, schrieb Jagannadha Sutradharudu Teki:
 This patch series is a combination of sf: Add common probe support
 sf: Add support for extended/quad read and write commands
 http://www.mail-archive.com/u-boot@lists.denx.de/msg121668.html
 http://u-boot.10912.n7.nabble.com/PATCH-v2-00-10-sf-Add-support-for-
 extended-quad-read-and-write-commands-td160964.html
 
 This patch series adds common probe support for all flash vendors
 except ramtron.
 
 spi_flash_probe is a new addition where all flash driver
 probing is combined into a common file, this means spi_flash_probe.c
 adds a new probing style common to all flashes.
 
 
 Apart from common probing, this series also adds support for
 extended read commands and quad read/write commands.
 
 http://comments.gmane.org/gmane.comp.boot-loaders.u-boot/150148
 http://permalink.gmane.org/gmane.comp.boot-loaders.u-boot/167026
 
 There is a bit discussion going on for supporting this:
 http://u-boot.10912.n7.nabble.com/PATCH-00-12-cmd-sf-Add-support-
 for-read-and-write-instructions-td143749.html
 
 Concept: Initially I tried to add individual sf write/read commands to
 respective flash read/write commands, but later some discussion to
 mainline about this and changed the implementation.
 
 As Michal and me were trying to change this as like the
 implementation will discover the fastest command by taking the
 supported commands from flash and a controller. Controller supported
 commands will always been a priority.
 
 Means I have added rd_cmd and wr_cmd params on spi_flash_id_params
 table. So the flash user may fill these with flash supported commands,
 and also spi controller use is also have rd_cmd and wr_cmd from spi.h,
 so the spi user will fill these with controller supported commands.
 and the resultent command is calculated based fastest commands by
 taking inputs from spi and flash, but spi(controller) has always a
 priority.
 
 Supported commands:
 - CMD_READ_ARRAY_SLOW
 - CMD_READ_ARRAY_FAST
 - CMD_READ_DUAL_OUTPUT_FAST
 - CMD_READ_DUAL_IO_FAST
 - CMD_READ_QUAD_OUTPUT_FAST
 - CMD_PAGE_PROGRAM
 - CMD_QUAD_PAGE_PROGRAM
 I miss an important detail in this series, regarding dual/quad-io support:
 How is the spi controller is informed about the transfer details?
 I see only setting the cmds according the features of flash and controller,
 but no additional indication to the spi controller, that this is then a
 dual-
 or quad-io transfer.How the spi controller should know about this???
 
 This implementation will discover the fastest command by taking the
 supported commands from flash and a controller. Controller supported
 commands will always been a priority.
 
 Means controller driver should have a code to support whether I am
 supporting this
 new extended read/write or quad command.
 
 And this support will discover the fastest command by comparing the
 commands from flash and spi.
 flash anyway we added in params list and controller the respective
 controller will initialize spi slave in the driver itself .
 
 qspi-slave.rd_cmd = READ_CMD_FULL;
 qspi-slave.wr_cmd = PAGE_PROGRAM | QUAD_PAGE_PROGRAM;
 
 Please refer https://github.com/Xilinx/u-boot-xlnx/blob/master-
 next/drivers/spi/zynq_qspi.c

I had a look at this code.  And found exactly, what I expected:
A huge table with flash opcodes!
Why does a spi controller need to know these details?
Because of information about, which part of a message needs single/dual or quad
transfers?
If we would do it this way, each spi controller needs to implement similar 
things!

From my point of view, this information should come from the serial flash 
driver!
There you know details of manufacturer, supported opcodes and so on.
And there you can decide, if you switch a flash to a full quad mode,
that the same opcode now needs to be send on 4 lines!
In the controller driver you don't know these details!

 
 spi_flash layer should send any commands based on the respective
 supported, but it is supported by respective spi controller that should
 controller driver must aware.
 
 This is more generic solution as we discussed so-many months ago.
 http://u-boot.10912.n7.nabble.com/PATCH-00-12-cmd-sf-Add-support-for-
 read-and-write-instructions-td143749.html

Yes, I agree that this a huge step forward!
But I think, we still have not reached a state which everyone can accept.

 
 We tried to discover the supported commands runtime from respective
 flashes, but ie. going to be a huge
 code to decode all these. Instead filling params, like sectors and
 idcodes should be an code driven task as per as
 u-boot code is concern.

As I already said, this is very specific to your controller!

 
 By decoding the cmd itself again? But the data should be a transparent byte
 stream to the controller!
 Otherwise you need a table of commands for decoding also

Re: [U-Boot] [PATCH v4 31/36] sf: Add extended read commands support

2013-09-25 Thread thomas.langer 
Hello Jagan,

Jagan Teki wrote on 2013-09-25:
 On Wed, Sep 25, 2013 at 3:34 PM,  thomas.lan...@lantiq.com wrote:
 Hello Jagan,

 Jagan Teki wrote on 2013-09-25:
 On Wed, Sep 25, 2013 at 3:14 PM,  thomas.lan...@lantiq.com wrote:
 Hello Jagan,

 From: Jagan Teki [mailto:jagannadh.t...@gmail.com] Sent: Wednesday,
 September 25, 2013 11:36 AM To: Langer Thomas (LQDE RD ST PON SW)
 Cc: Jagannadha Sutradharudu Teki; Tom Rini; jaganna; u-
 b...@lists.denx.de; Todd Legler (tlegler); Willis Max; Syed Hussain;
 Sascha Silbe Subject: Re: [U-Boot] [PATCH v4 31/36] sf: Add extended
 read commands support

 On Wed, Sep 25, 2013 at 1:40 AM,  thomas.lan...@lantiq.com wrote:
 Hello Jagan,

 Am 24.09.2013 20:36, schrieb Jagannadha Sutradharudu Teki:
 diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
 index ea39d1a..0ac9fab 100644
 --- a/drivers/spi/spi.c
 +++ b/drivers/spi/spi.c
 @@ -7,6 +7,7 @@
  #include common.h
  #include malloc.h
  #include spi.h
 +#include spi_flash.h

  void *spi_do_alloc_slave(int offset, int size, unsigned int bus,
unsigned int cs)
 @@ -20,6 +21,7 @@ void *spi_do_alloc_slave(int offset, int size,
 unsigned int bus,
   slave = (struct spi_slave *)(ptr + offset);
   slave-bus = bus;
   slave-cs = cs;
 + slave-rd_cmd = CMD_READ_ARRAY_FAST;

 This is SPI code, not SF! The spi layer should not know such details of
 the slave!
 What about EEPROMs or other SPI slaves, which are NOT spi_flash???
 Examples (just searched for includes of spi.h):
 board/bf527-ezkit/video.c
 drivers/net/enc28j60.c

 Please ensure that the layers are not mixed up!
 SPI is an interface type and SF is ONLY ONE user of this interface!

 I understand your concern, but here the point is for discovering the
 command set.
 slave-rd_cmd = CMD_READ_ARRAY_FAST;
 is a default controller supported fast read.

 spi_flash layer will discover the respective rd_cmd based slave and
 flash, if slave doesn't have any commands to list, means not support
 extended/quad then these fields are filled in spi.c

 there is nothing harm for respective drivers or code.

 But I think this is the wrong approach! Why should the spi controller
 care about, what devices type is connected? And the CMD is a SF
 specific thing! I agree, that the controller should provide
 information about his features, but this should only be something
 like tx_width=4 and rx_width=4, which would tell the SF layer
 that quad-io is possible!

 No details of serial-flashes are necessary for this!
 Yes, no issues.
 I can directly assign to flash side while discovering commends.

 I don't understand this sentence. Do you mean commands?
 Who will discover the commands?

 The SPI controller does not know about the meaning of commands! The
 controller only knows I must send this block of data and split it on
 1/2/4 lines (or similar for other transfers).

 implementation will discover the fastest command by taking the supported
 commands from flash and a controller. Controller supported commands
 will always been a priority.


 From SPI controller:
 
 spi_setup_slave() {
 spi = spi_alloc_slave(struct zynq_spi_slave, bus, cs);

 spi-slave.rd_cmd = RD_CMD_FULL;
 spi-slave.wr_cmd = WR_CMD_FULL;
 }
These are already FLASH definitions!

Please define it the other way round:
Add properties for number of rx and tx lines to the spi-slave.
(Do you notice the different naming?)



 From SPI FLASH: (drivers/mtd/spi/spi_flash_probe.c)
 -
 /* Look for the fastest read cmd */
 cmd = fls(params-rd_cmd  flash-spi-rd_cmd);
 if (cmd) {
 cmd = spi_read_cmds_array[cmd - 1]; flash-read_cmd =
 cmd; } else { /* Go for controller supported command */
 flash-read_cmd = CMD_READ_ARRAY_FAST;
 }

 /* Look for the fastest write cmd */
 cmd = fls(params-wr_cmd  flash-spi-wr_cmd);
 if (cmd) {
 cmd = spi_write_cmds_array[cmd - 1]; flash-write_cmd =
 cmd; } else { /* Go for controller supported command */
 flash-write_cmd = CMD_PAGE_PROGRAM;
 }
 include/spi_flash.h:
 ---
 /* Supported write cmds enum list */
 enum spi_write_cmds {
 PAGE_PROGRAM = 1  0,
 QUAD_PAGE_PROGRAM = 1  1,
 };
 #define WR_CMD_FULL PAGE_PROGRAM | QUAD_PAGE_PROGRAM

 /* Supported read cmds enum list */
 enum spi_read_cmds {
 ARRAY_SLOW = 1  0,
 ARRAY_FAST = 1  1,
 DUAL_OUTPUT_FAST = 1  2,
 DUAL_IO_FAST = 1  3,
 QUAD_OUTPUT_FAST = 1  4,
 };
 #define RD_CMD_FULL ARRAY_SLOW | ARRAY_FAST |
 DUAL_OUTPUT_FAST | \
 DUAL_IO_FAST | QUAD_OUTPUT_FAST

 If the controller is not filling slave.rd_cmd and slave.wr_cmd if it's
 not supporting even if connected flash
 supports, fill the

Re: [U-Boot] [PATCH v4 00/36] sf: Add common probe and extended/quad read/write cmds support

2013-09-24 Thread thomas.langer 
Hello Jagan,

Am 24.09.2013 20:38, schrieb Jagannadha Sutradharudu Teki:
 This patch series is a combination of
 sf: Add common probe support
 sf: Add support for extended/quad read and write commands
 http://www.mail-archive.com/u-boot@lists.denx.de/msg121668.html
 http://u-boot.10912.n7.nabble.com/PATCH-v2-00-10-sf-Add-support-for-extended-quad-read-and-write-commands-td160964.html

 This patch series adds common probe support for all flash vendors except 
 ramtron.

 spi_flash_probe is a new addition where all flash driver
 probing is combined into a common file, this means spi_flash_probe.c
 adds a new probing style common to all flashes.


 Apart from common probing, this series also adds support for
 extended read commands and quad read/write commands.

 http://comments.gmane.org/gmane.comp.boot-loaders.u-boot/150148
 http://permalink.gmane.org/gmane.comp.boot-loaders.u-boot/167026

 There is a bit discussion going on for supporting this: 
 http://u-boot.10912.n7.nabble.com/PATCH-00-12-cmd-sf-Add-support-for-read-and-write-instructions-td143749.html

 Concept: 
 Initially I tried to add individual sf write/read commands to respective 
 flash read/write commands, but later some discussion to mainline about this 
 and 
 changed the implementation. 

 As Michal and me were trying to change this as like the 
 implementation will discover the fastest command by taking the supported 
 commands from flash and a controller. Controller supported commands will 
 always been a priority. 

 Means I have added rd_cmd and wr_cmd params on spi_flash_id_params table. 
 So the flash user may fill these with flash supported commands, and also spi 
 controller 
 use is also have rd_cmd and wr_cmd from spi.h, so the spi user will fill 
 these with 
 controller supported commands. and the resultent command is calculated based 
 fastest 
 commands by taking inputs from spi and flash, but spi(controller) has always 
 a priority. 

 Supported commands: 
 - CMD_READ_ARRAY_SLOW 
 - CMD_READ_ARRAY_FAST 
 - CMD_READ_DUAL_OUTPUT_FAST 
 - CMD_READ_DUAL_IO_FAST 
 - CMD_READ_QUAD_OUTPUT_FAST 
 - CMD_PAGE_PROGRAM 
 - CMD_QUAD_PAGE_PROGRAM 
I miss an important detail in this series, regarding dual/quad-io support:
How is the spi controller is informed about the transfer details?
I see only setting the cmds according the features of flash and controller,
but no additional indication to the spi controller, that this is then a
dual-
or quad-io transfer.How the spi controller should know about this???
By decoding the cmd itself again? But the data should be a transparent byte
stream to the controller!
Otherwise you need a table of commands for decoding also inside the
controller
driver, which I consider a bad idea, as you need to update them (in each
driver)
for new cmds added to the serial-flash driver!

In the linux kernel, the solution in the spi layer was to add new
elements to
the transfer structure (struct spi_transfer - bitwidth), which can be set
for each part of a message.
In u-boot we have the spi_xfer function, which has a flags parameter
we could
use for this.

As long as the details for this (including a spi controller driver,
which can
handle dual/quad-io transfers) are not fixed, I would suggest to leave the
patches regarding the extended cmds out of the series of sf-cleanup (which
really looks good!) and fix the issues until the next merge window!

 Testing repo branch:
 ---
 $ git clone git://git.denx.de/u-boot-spi.git
 $ cd u-boot-spi
 $ git checkout -b master-next-test origin/master-next-test

 REQUEST FOR ALL SPI CODE CONTRIBUTORS/USERS, PLEASE TEST THESE CHANGES 
 W.R.T YOUR HW IF POSSIBLE. 

 Please let me know for any issues/concerns/questions. 

 --
 Thanks,
 Jagan.
Best Regards,
Thomas

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

Re: [U-Boot] [PATCH v4 31/36] sf: Add extended read commands support

2013-09-24 Thread thomas.langer 
Hello Jagan,

Am 24.09.2013 20:36, schrieb Jagannadha Sutradharudu Teki:
 diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
 index ea39d1a..0ac9fab 100644
 --- a/drivers/spi/spi.c
 +++ b/drivers/spi/spi.c
 @@ -7,6 +7,7 @@
  #include common.h
  #include malloc.h
  #include spi.h
 +#include spi_flash.h
  
  void *spi_do_alloc_slave(int offset, int size, unsigned int bus,
unsigned int cs)
 @@ -20,6 +21,7 @@ void *spi_do_alloc_slave(int offset, int size, unsigned int 
 bus,
   slave = (struct spi_slave *)(ptr + offset);
   slave-bus = bus;
   slave-cs = cs;
 + slave-rd_cmd = CMD_READ_ARRAY_FAST;

This is SPI code, not SF! The spi layer should not know such details of
the slave!
What about EEPROMs or other SPI slaves, which are NOT spi_flash???
Examples (just searched for includes of spi.h):
board/bf527-ezkit/video.c
drivers/net/enc28j60.c

Please ensure that the layers are not mixed up!
SPI is an interface type and SF is ONLY ONE user of this interface!

   }
  
   return ptr;
 diff --git a/include/spi.h b/include/spi.h
 index c0dab57..093847e 100644
 --- a/include/spi.h
 +++ b/include/spi.h
 @@ -40,11 +40,13 @@
   *   cs: ID of the chip select connected to the slave.
   *   max_write_size: If non-zero, the maximum number of bytes which can
   *   be written at once, excluding command bytes.
 + *   rd_cmd: Read command.
   */
  struct spi_slave {
   unsigned intbus;
   unsigned intcs;
   unsigned int max_write_size;
 + u8 rd_cmd;
  };
  
  /*---

Best Regards,
Thomas

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

Re: [U-Boot] [PATCH v1 0/5]

2013-09-02 Thread thomas.langer 
Hello Stefano,

Stefano Babic wrote on 2013-09-02:

 Some phys have additional registers that are not covered
 by standard. Access to this registers can be done via
 specific sequence according to the phy datasheet.
 The driver for Micrel phy contains some additional function,
 that the board maintainer can call to tune the phy. However,
 these registers cannot be accessed with mmdio command.
 
 Add calback to the phy API to allow to call the function
 for reading / writing extended registers with the mmdio command.
 
 

The basic mechanism used for the access is not specific to Micrel, it is 
defined 
in the IEEE standard [1], Annex 22D: Clause 22 access to Clause 45 MMD 
registers

A presentation for this can be found when you ask google for it [2]

[1]: IEEE Std 802.3-2008 (or 2012): 
http://standards.ieee.org/about/get/802/802.3.html
[2]: 
https://www.google.de/search?q=%22Clause+22+Access+to+Clause+45+Registers%22

So maybe it will be useful to have the access functions in the common phy lib?

I assume that most current PHYs have these method implemented,
as also some extended standard registers needs this.
For example, the EEE (Energy Efficient Ethernet) registers are located in this 
extended
register range.

Probably this can be extended in some follow-up patches, 
I just wanted to share my thoughts ;-)
I currently have not platform on my desk which uses phylib (and current code),
so I cannot provide a patch on my own, sorry.

Best Regards,
Thomas


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

Re: [U-Boot] [PATCH] powerpc: Fix CamelCase checkpatch warnings

2013-08-16 Thread thomas.langer 
Hello Prabhakar,

Prabhakar Kushwaha wrote on 2013-08-16:
 85xx, 86xx PowerPC folders have code variables with CamelCase naming
 conventions. because of this code checkpatch script generates WARNING:
 Avoid CamelCase.
 
 This patch set Convert variables name to normal naming convention and
 modify board, driver files with updated new
   - [PATCH 1/2] powerpc: Fix CamelCase checkpatch warnings
   - [PATCH 2/2] board: Update variable names as per new the structures
 
I think you have to fold both patches together to have the change bisectable!
Otherwise your board code will not compile after applying only the first patch.

Best Regards,
Thomas


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

Re: [U-Boot] [PATCH 1/3] spi: Add zynq qspi controller driver

2013-08-11 Thread thomas.langer 
Hi Jagan,

 +
 +/* Definitions of the flash commands - Flash insts in ascending order */
 +#define ZYNQ_QSPI_FLASH_INST_WRSR0x01/* Write status register */
 +#define ZYNQ_QSPI_FLASH_INST_PP  0x02/* Page program */
 +#define ZYNQ_QSPI_FLASH_INST_WRDS0x04/* Write disable */
 +#define ZYNQ_QSPI_FLASH_INST_RDSR1   0x05/* Read status register 1 */
 +#define ZYNQ_QSPI_FLASH_INST_WREN0x06/* Write enable */
 +#define ZYNQ_QSPI_FLASH_INST_AFR 0x0B/* Fast read data bytes */
 +#define ZYNQ_QSPI_FLASH_INST_BE_4K   0x20/* Erase 4KiB block */
 +#define ZYNQ_QSPI_FLASH_INST_RDSR2   0x35/* Read status register 2 */
 +#define ZYNQ_QSPI_FLASH_INST_BE_32K  0x52/* Erase 32KiB block */
 +#define ZYNQ_QSPI_FLASH_INST_RDID0x9F/* Read JEDEC ID */
 +#define ZYNQ_QSPI_FLASH_INST_SE  0xD8/* Sector erase 
 (usually 64KB)*/

As I have written some time ago, I think it is a bad idea to parse the
flash commands in the spi driver!
The flash driver, who wants to write these commands and should know the details,
which parts of the message needs to be send as dual/quad transfer, should 
provide this information to the spi driver (probably as flags for spi_xfer())

A first idea for the flags:
- the X refers to different numbers, as required by the flash (depending on the
addressing mode (3/4 byte) or if the address is already sent as DUAL/QUAD)
SPI_XFER_X_DUAL
SPI_XFER_X_QUAD

So having 0-5 for X should be enough to support all flashes
- 0 if the command is already in DUAL/QUAD mode
- 5 for command + 4byte address
(I am not sure about dummy bytes on read)

So for example: 
switch to dual transfer after first byte: SPI_XFER_1_DUAL
switch to QUAD transfer after 4 bytes (e.g. 3byte address): SPI_XFER_4_QUAD

Then only the flash driver needs to know these requirements, depending on 
manufacturer and chip type, and the spi driver just reacts on the flags.
Otherwise this driver has to be extended each time a new command is added to 
the flash driver (as you did in some later patches!)

Best regards,
Thomas

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

Re: [U-Boot] [PATCH 11/11] MIPS: MAKEALL: fix lists for MIPSel and MIPS boards

2011-11-25 Thread thomas.langer 
Hello Daniel,

Daniel Schwierzeck wrote on 2011-11-25:
 On Fri, Nov 25, 2011 at 9:49 AM, Marek Vasut marek.va...@gmail.com
 wrote:
 On Thursday 24 November 2011 08:57:56 Daniel Schwierzeck wrote:
 Build dbau1550_el only in LIST_au1xx0_el and LIST_mips_el.
 Also remove obsolete lists for mips5kc.
 
 if possible, i'd really like to kill off all the specialized mips
 lists and do selection purely based on fields in boards.cfg. -mike
 
 I have to agree here.
 M
 
 
 that is currently not possible because -EB and -EL are not properly
 handled in CFLAGS and LDFLAGS.
 Until this is fixed we have to run MAKEALL twice with mips and mipsel
 and different toolchains.

I have seen that barebox[1] has got support for MIPS some releases ago.
They define the endianess in the config, so it is easier to handle in the
build process.
Also the handling of compiler and linker options is more optimized[2].

Maybe we should take advantage of it and port some of this over to u-boot?
I think the current handling of -EB and -EL is still from Wolfgangs first
port of a MIPS board.

Best Regards,
Thomas

PS: Sorry for sending multiple times, I have problem to suppress base64 
encoding.
   I hope it is okay now.

[1] http://www.barebox.org/
[2] 
http://git.pengutronix.de/?p=barebox.git;a=blob;f=arch/mips/Makefile;hb=HEAD 
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 2/4] net: tweak eth_device layout to simplify enetaddr use

2011-11-11 Thread thomas.langer 
Hello Mike,

 + union {
 + u32 enetaddr32;
 + u16 enetaddr16[3];
 + unsigned char enetaddr[6];
 + };

This will work only as long the endianess is matching.

Picking single chars from enetaddr[] and combine them to a u32 register
will be more independent from endianess.

If this goes in, I would like to see at least a comment about the problem.

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

Re: [U-Boot] [PATCH 0/4] MIPS: make config options more generic

2011-07-28 Thread thomas.langer 
Hi Daniel,

Daniel Schwierzeck wrote on 2011-07-27:

 This patch series contains cleanups and enhancements to consolidate
 the INCA-IP related config options and to make them more generic.
 Additionally, the cache operation mode is now configurable.
 All changes are needed to prepare the support of newer MIPS
 based Lantiq XWAY SoCs.


my ack to the complete series:

Acked-by: Thomas Langer thomas.lan...@lantiq.com

Best Regards,
Thomas


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

[U-Boot] [PATCH] fix redundant environment for serial flash

2011-03-29 Thread thomas.langer 
This patch fixes problems in the handling of redundant environment in env_sf.c

The major problem are double calls of free() on the allocated buffers,
which damages the internal data of malloc and crashes on next call.

In addition, the selection of the active environment had errors and compiler 
warnings, which are corrected by this patch.

Signed-off-by: Thomas Langer thomas.lan...@lantiq.com
---
This patch is done and tested against the version 2010.12 and 
applies also cleanly against the u-boot/master.

--- a/common/env_sf.c
+++ b/common/env_sf.c
@@ -59,7 +59,6 @@ DECLARE_GLOBAL_DATA_PTR;
 extern uchar default_environment[];
 
 char * env_name_spec = SPI Flash;
-env_t *env_ptr;
 
 static struct spi_flash *env_flash;
 
@@ -79,7 +78,7 @@ int saveenv(void)
char*saved_buffer = NULL;
u32 sector = 1;
int ret;
-   charflag = OBSOLETE_FLAG, new_flag = ACTIVE_FLAG;
+   charflag = OBSOLETE_FLAG;
 
if (!env_flash) {
env_flash = spi_flash_probe(CONFIG_ENV_SPI_BUS,
@@ -159,7 +158,7 @@ int saveenv(void)
 
gd-env_valid = (gd-env_valid == 2 ? 1 : 2);
 
-   printf(Valid environment: %d\n, gd-env_valid);
+   printf(Valid environment: %d\n, (int)gd-env_valid);
 
  done:
if (saved_buffer)
@@ -174,25 +173,20 @@ void env_relocate_spec(void)
env_t *tmp_env1 = NULL;
env_t *tmp_env2 = NULL;
env_t *ep = NULL;
-   uchar flag1, flag2;
-   /* current_env is set only in case both areas are valid! */
-   int current_env = 0;
 
tmp_env1 = (env_t *)malloc(CONFIG_ENV_SIZE);
tmp_env2 = (env_t *)malloc(CONFIG_ENV_SIZE);
 
if (!tmp_env1 || !tmp_env2) {
-   free(tmp_env1);
-   free(tmp_env2);
set_default_env(!malloc() failed);
-   return;
+   goto out;
}
 
env_flash = spi_flash_probe(CONFIG_ENV_SPI_BUS, CONFIG_ENV_SPI_CS,
CONFIG_ENV_SPI_MAX_HZ, CONFIG_ENV_SPI_MODE);
if (!env_flash) {
set_default_env(!spi_flash_probe() failed);
-   return;
+   goto out;
}
 
ret = spi_flash_read(env_flash, CONFIG_ENV_OFFSET,
@@ -204,33 +198,30 @@ void env_relocate_spec(void)
 
if (crc32(0, tmp_env1-data, ENV_SIZE) == tmp_env1-crc)
crc1_ok = 1;
-   flag1 = tmp_env1-flags;
 
ret = spi_flash_read(env_flash, CONFIG_ENV_OFFSET_REDUND,
CONFIG_ENV_SIZE, tmp_env2);
if (!ret) {
if (crc32(0, tmp_env2-data, ENV_SIZE) == tmp_env2-crc)
crc2_ok = 1;
-   flag2 = tmp_env2-flags;
}
 
if (!crc1_ok  !crc2_ok) {
-   free(tmp_env1);
-   free(tmp_env2);
set_default_env(!bad CRC);
-   return;
+   goto err_read;
} else if (crc1_ok  !crc2_ok) {
gd-env_valid = 1;
-   ep = tmp_env1;
} else if (!crc1_ok  crc2_ok) {
+   gd-env_valid = 2;
+   } else if (tmp_env1-flags == ACTIVE_FLAG 
+  tmp_env2-flags == OBSOLETE_FLAG) {
gd-env_valid = 1;
-   } else if (flag1 == ACTIVE_FLAG  flag2 == OBSOLETE_FLAG) {
-   gd-env_valid = 1;
-   } else if (flag1 == OBSOLETE_FLAG  flag2 == ACTIVE_FLAG) {
+   } else if (tmp_env1-flags == OBSOLETE_FLAG 
+  tmp_env2-flags == ACTIVE_FLAG) {
gd-env_valid = 2;
-   } else if (flag1 == flag2) {
+   } else if (tmp_env1-flags == tmp_env2-flags) {
gd-env_valid = 2;
-   } else if (flag1 == 0xFF) {
+   } else if (tmp_env1-flags == 0xFF) {
gd-env_valid = 2;
} else {
/*
@@ -240,8 +231,6 @@ void env_relocate_spec(void)
gd-env_valid = 2;
}
 
-   free(env_ptr);
-
if (gd-env_valid == 1)
ep = tmp_env1;
else
@@ -257,10 +246,6 @@ err_read:
spi_flash_free(env_flash);
env_flash = NULL;
 out:
-   if (tmp_env1)
-   free(tmp_env1);
-   if (tmp_env2)
-   free(tmp_env2);
free(tmp_env1);
free(tmp_env2);

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

Re: [U-Boot] [RFC] cfi_flash and complex address mapping

2009-03-26 Thread thomas.langer 

 In message 200903261103.28452...@denx.de you wrote:
 
   #define FLASH_FIXUP_ADDR_8(addr)  ((void*)((ulong)(addr)^2))
   #define FLASH_FIXUP_ADDR_16(addr) ((void*)((ulong)(addr)^2))
 ...
  Yes, I think this could be accepted. The overall impact on 
 the driver is not 
  too big. Let's see if others have some complaints about it. 
 If not, I'll 
  accept you patch after you changed some minor details.
 
 I think that should be an inline function instead of a macro.
 
 Best regards,
 
 Wolfgang Denk

Thanks for the comments. I will take it into account before 
sending the complete patch series for the board(s). 
But I have to do some more cleanups before I can do that.

I just wanted to check if this is the right way to go.

Best regards,
Thomas Langer
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [RFC] cfi_flash and complex address mapping

2009-03-25 Thread thomas.langer 
Hi,

I'm in the process of preparing some code for a new board and want to use the
generic cfi flash driver. The problem is EBU (External Bus Unit) of the chip,
which is internally 32bit.

The access to 8/16bit values are always mapped by the EBU, but in a little
endian way, which is bad on a big endian system: On an interface configured
for 16bit, the following conversion from internal address to bus-address is 
done:
CPU external bus
0x  0x0002
0x0002  0x
0x0004  0x0006
0x0006  0x0004

If I implement my own flash_read%/flash_write% to do the address mapping, the
data is also swapped. In this functions I cannot decide if the access is for
the flash control or the data to be programmed.

My current solution is the following patch and these definitions in the board
config file (for a 16bit flash):

#define FLASH_FIXUP_ADDR_8(addr)((void*)((ulong)(addr)^2))
#define FLASH_FIXUP_ADDR_16(addr)   ((void*)((ulong)(addr)^2))

My question now is: Would such a change be accepted for the mainline u-boot?
Or does someone has a better idea?

Thanks for your comments,
Thomas

PS: The patch is against u-boot-2009.01

--- a/drivers/mtd/cfi_flash.c
+++ b/drivers/mtd/cfi_flash.c
@@ -176,6 +176,23 @@ flash_info_t flash_info[CFI_MAX_FLASH_BA
 #define CONFIG_SYS_FLASH_CFI_WIDTH FLASH_CFI_8BIT
 #endif
 
+/*
+ * Check if address fixup macros are defined, define defaults otherwise
+ */
+#ifndef FLASH_FIXUP_ADDR_8
+#define FLASH_FIXUP_ADDR_8(addr)   (addr)
+#endif
+#ifndef FLASH_FIXUP_ADDR_16
+#define FLASH_FIXUP_ADDR_16(addr)  (addr)
+#endif
+#ifndef FLASH_FIXUP_ADDR_32
+#define FLASH_FIXUP_ADDR_32(addr)  (addr)
+#endif
+#ifndef FLASH_FIXUP_ADDR_64
+#define FLASH_FIXUP_ADDR_64(addr)  (addr)
+#endif
+
+
 /* CFI standard query structure */
 struct cfi_qry {
u8  qry[3];
@@ -392,9 +409,9 @@ static inline uchar flash_read_uchar (fl
 
cp = flash_map (info, 0, offset);
 #if defined(__LITTLE_ENDIAN) || defined(CONFIG_SYS_WRITE_SWAPPED_DATA)
-   retval = flash_read8(cp);
+   retval = flash_read8(FLASH_FIXUP_ADDR_8(cp));
 #else
-   retval = flash_read8(cp + info-portwidth - 1);
+   retval = flash_read8(FLASH_FIXUP_ADDR_8(cp) + info-portwidth - 1);
 #endif
flash_unmap (info, 0, offset, cp);
return retval;
@@ -408,7 +425,7 @@ static inline ushort flash_read_word (fl
ushort *addr, retval;
 
addr = flash_map (info, 0, offset);
-   retval = flash_read16 (addr);
+   retval = flash_read16 (FLASH_FIXUP_ADDR_16(addr));
flash_unmap (info, 0, offset, addr);
return retval;
 }
@@ -433,19 +450,28 @@ static ulong flash_read_long (flash_info
debug (long addr is at %p info-portwidth = %d\n, addr,
   info-portwidth);
for (x = 0; x  4 * info-portwidth; x++) {
-   debug (addr[%x] = 0x%x\n, x, flash_read8(addr + x));
+   debug (addr[%x] = 0x%x\n, x,
+   flash_read8(FLASH_FIXUP_ADDR_32(addr) + x));
}
 #endif
 #if defined(__LITTLE_ENDIAN) || defined(CONFIG_SYS_WRITE_SWAPPED_DATA)
-   retval = ((flash_read8(addr)  16) |
- (flash_read8(addr + info-portwidth)  24) |
- (flash_read8(addr + 2 * info-portwidth)) |
- (flash_read8(addr + 3 * info-portwidth)  8));
+   retval = ((flash_read8(FLASH_FIXUP_ADDR_8
+ (addr)  16) |
+ (flash_read8(FLASH_FIXUP_ADDR_8
+ (addr + info-portwidth))  24) |
+ (flash_read8(FLASH_FIXUP_ADDR_8
+ (addr + 2 * info-portwidth))) |
+ (flash_read8(FLASH_FIXUP_ADDR_8
+ (addr + 3 * info-portwidth))  8));
 #else
-   retval = ((flash_read8(addr + 2 * info-portwidth - 1)  24) |
- (flash_read8(addr + info-portwidth - 1)  16) |
- (flash_read8(addr + 4 * info-portwidth - 1)  8) |
- (flash_read8(addr + 3 * info-portwidth - 1)));
+   retval = ((flash_read8(FLASH_FIXUP_ADDR_8
+ (addr + 2 * info-portwidth - 1))  24) |
+ (flash_read8(FLASH_FIXUP_ADDR_8
+ (addr + info-portwidth - 1))  16) |
+ (flash_read8(FLASH_FIXUP_ADDR_8
+ (addr + 4 * info-portwidth - 1))  8) |
+ (flash_read8(FLASH_FIXUP_ADDR_8
+ (addr + 3 * info-portwidth - 1;
 #endif
flash_unmap(info, sect, offset, addr);
 
@@ -466,21 +492,22 @@ static void flash_write_cmd (flash_info_
flash_make_cmd (info, cmd, cword);
switch (info-portwidth) {
case FLASH_CFI_8BIT:
-   debug (fwc addr %p cmd %x %x 8bit x %d bit\n, addr, cmd,
-  cword.c, info-chipwidth  CFI_FLASH_SHIFT_WIDTH);
-   flash_write8(cword.c, addr);
+   debug (fwc addr %p cmd %x %x