Re: [U-Boot] IMX6 NAND boot regression
Hi, On Sat, 2019-02-02 at 05:30 -0800, Adam Ford wrote: > On Sat, Feb 2, 2019 at 12:29 AM Jagan Teki wrote: > > +Adam, Shyam > > > > On Sat, 2 Feb, 2019, 8:49 AM Stefan Agner > > > > Hi Tim, > > > > > > On 02.02.19 03:32, Tim Harvey wrote: > > > > Stefan, > > > > > > > > I'm trying to track down an IMX6 SPL NAND boot regression that started > > > > in v2018.07 with your patch series to mxs_nand. > > > > > > I am sorry about that. Unfortunately I did not had a design at hand where > > > I was able to test the NAND driver in SPL... > > > > > > > I bisected it back to '5346c31e305a37d39f535cc0d5ae87d8b7e81230 mtd: > > > > nand: mxs_nand: use self init'. With this particular patch nand bbt > > > > scanning would crash the board because of nand_chip.scan_btt not being > > > > assigned. This was later fixed in > > > > '96d0be07e7498e7174daa6f3b56fc807b9feb71d MTD: nand: mxs_nand_spl: Fix > > > > empty function pointer for BBT' but cherry-picking that on top of > > > > 5346c31 fixes the immediate crash while scanning but then I find that > > > > mxs_read_page_ecc() hangs on the 4th page of reading u-boot.img from > > > > the NAND. This gets worse 2 patches later where in > > > > '28897e8d21f8e197e259a91c693de09cd81f2d5a: mtd: nand: mxs_nand: use > > > > structure for BCH geometry' I find that the first byte of every page > > > > read is wrong because mxs_nand_swap_block_mark() is getting called on > > > > the page which swaps the first bytes with oob. > > > > > > > > There are several IMX6 boards out there using both NAND and SPL I > > > > believe that I would assume were broken by this series. Any ideas on > > > > the proper resolution? > > > > Look like 2017.03 can be stable boot from nand as for as my test is concern. > > > > We are also trying hard using git bisect, but seems like multiple breakings. > > > > Will keep posted if something move further. > > From a different thread, someone was able to test these patches and > found they fixed their booting issues: > > There was a broken function pointer here that was fixed and applied > the imx-master, but pending merge with master > http://patchwork.ozlabs.org/patch/1019440/ > > Configure ECC from SPL here: > http://patchwork.ozlabs.org/patch/1020160/ > > Remove hard-coded ECC parameters since the patch above can autoset them. > http://patchwork.ozlabs.org/patch/1026638/ > > Maybe those can help. I can confirm that that the commit 5346c31e305a37d39f535cc0d5ae87d8b7e81230 broke booting from NAND for my i.MX6ULL board, so I sticked with version 2018.05. Now, I've tested U-Boot 2019.01 with the three patches Adam suggested and the SPL loader is able to boot from NAND again. Jörg ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] MTD: nand: mxs_nand: Allow driver to auto setup ECC in SPL
Hi, On Thu, 2019-01-17 at 07:16 -0600, Adam Ford wrote: > The initialization of the NAND in SPL hard-coded ecc.bytes, > ecc.size, and ecc.strength which may work for some NAND parts, > but it not appropriate for others. With the pending patch > "mxs_nand: Fix BCH read timeout error on boards requiring ECC" > the driver can auto configure the ECC when these entries are > blank. This patch has been tested in NAND flash with oob 64 > and oob 128. > > Signed-off-by: Adam Ford Maybe you can give me a hint where the driver actually does the auto configuration? I've tested the patch on a custom i.MX6ULL board with a Micron NAND flash. The SPL loader is able to boot from NAND with and without this patch. Tested-by: Jörg Krause > diff --git a/drivers/mtd/nand/raw/mxs_nand.c b/drivers/mtd/nand/raw/mxs_nand.c > index 2d84bfffe2..95fa452cef 100644 > --- a/drivers/mtd/nand/raw/mxs_nand.c > +++ b/drivers/mtd/nand/raw/mxs_nand.c > @@ -1191,9 +1191,6 @@ int mxs_nand_init_spl(struct nand_chip *nand) > nand->ecc.read_page = mxs_nand_ecc_read_page; > > nand->ecc.mode = NAND_ECC_HW; > - nand->ecc.bytes = 9; > - nand->ecc.size = 512; > - nand->ecc.strength = 8; > > return 0; > } ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] MTD: nand: mxs_nand_spl: Fix empty function pointer for BBT
On Thu, 2019-01-03 at 15:52 +, Stefan Agner wrote: > On 30.12.18 17:11, Adam Ford wrote: > > The initialization function calls a nand_chip.scan_bbt(mtd) but > > scan_bbt is never initialized resulting in an undefined function > > pointer. This will direct the function pointer to nand_default_bbt > > defined in the same file. > > > > Signed-off-by: Adam Ford > > Seems sensible > > Acked-by: Stefan Agner Fixes crashing NAND SPL loader on my i.MX6ULL board, which was introduced in commit 5346c31e305a37d39f535cc0d5ae87d8b7e81230 and is present in U-Boot since version 2018.07. Tested on a custom i.MX6ULL board with a Micron NAND flash. Tested-by: Jörg Krause > > > diff --git a/drivers/mtd/nand/raw/mxs_nand_spl.c > > b/drivers/mtd/nand/raw/mxs_nand_spl.c > > index 2d7bbe83cc..c628f3adec 100644 > > --- a/drivers/mtd/nand/raw/mxs_nand_spl.c > > +++ b/drivers/mtd/nand/raw/mxs_nand_spl.c > > @@ -185,6 +185,7 @@ static int mxs_nand_init(void) > > mtd = nand_to_mtd(&nand_chip); > > /* set mtd functions */ > > nand_chip.cmdfunc = mxs_nand_command; > > + nand_chip.scan_bbt = nand_default_bbt; > > nand_chip.numchips = 1; > > > > /* identify flash device */ ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] MTD: mxs_nand: Fix BCH read timeout error on boards requiring ECC
Hi, On Thu, 2019-01-03 at 15:55 +, Stefan Agner wrote: > [this time using reply to all] > > On 03.01.19 03:36, Adam Ford wrote: > > The LogicPD board uses a Micron Flash with ECC. To boot this from > > SPL, the ECC needs to be correctly configured or the BCH engine > > times out. > > > > Signed-off-by: Adam Ford > > Looks good to me, > > Acked-by: Stefan Agner Tested on a custom i.MX6ULL board with Micron NAND flash. Fixes: """ Trying to boot from NAND MXS NAND: BCH read timeout ... MXS NAND: BCH read timeout """ Tested-by: Jörg Krause > > diff --git a/drivers/mtd/nand/raw/mxs_nand.c > > b/drivers/mtd/nand/raw/mxs_nand.c > > index e3341812a2..2d84bfffe2 100644 > > --- a/drivers/mtd/nand/raw/mxs_nand.c > > +++ b/drivers/mtd/nand/raw/mxs_nand.c > > @@ -1163,6 +1163,12 @@ int mxs_nand_init_spl(struct nand_chip *nand) > > > > nand_info->gpmi_regs = (struct mxs_gpmi_regs *)MXS_GPMI_BASE; > > nand_info->bch_regs = (struct mxs_bch_regs *)MXS_BCH_BASE; > > + > > + if (is_mx6sx() || is_mx7()) > > + nand_info->max_ecc_strength_supported = 62; > > + else > > + nand_info->max_ecc_strength_supported = 40; > > + > > err = mxs_nand_alloc_buffers(nand_info); > > if (err) > > return err; > > diff --git a/drivers/mtd/nand/raw/mxs_nand_spl.c > > b/drivers/mtd/nand/raw/mxs_nand_spl.c > > index c628f3adec..ba85baac60 100644 > > --- a/drivers/mtd/nand/raw/mxs_nand_spl.c > > +++ b/drivers/mtd/nand/raw/mxs_nand_spl.c > > @@ -201,6 +201,7 @@ static int mxs_nand_init(void) > > /* setup flash layout (does not scan as we override that) */ > > mtd->size = nand_chip.chipsize; > > nand_chip.scan_bbt(mtd); > > + mxs_nand_setup_ecc(mtd); > > > > return 0; > > } > > ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] Nand boot on imx6q board is broken
Hi, On Thu, 2019-01-31 at 07:22 -0800, Adam Ford wrote: > On Wed, Jan 30, 2019 at 11:40 PM Shyam Saini > wrote: > > Hi Everyone, > > > > I'm trying to boot imx6q board from nand but it seems like mainline > > u-boot nand boot support for imx6q board is broken. > > I spent some time trying to make the imx6q_logic board boot from SPL > from NAND, but I needed to patch a few things. Some of them have yet > to be approved, but if they work for you, maybe it will help get them > approved. > > There was a broken function pointer here that was fixed and applied > the imx-master, but pending merge with master > http://patchwork.ozlabs.org/patch/1019440/ > > Configure ECC from SPL here: > http://patchwork.ozlabs.org/patch/1020160/ > > Remove hard-coded ECC parameters since the patch above can autoset them. > http://patchwork.ozlabs.org/patch/1026638/ > > With those 3 patches and some minor changes to my individual board > file and config file, I was able to boot 2019.01 via SPL from NAND. > Since it was working for you before, I am guessing the board file > stuff and config file stuff is probably already for you. > > adam I can confirm that applying these three patches fixes booting from NAND on a custom i.MX6ULL board with Micron NAND flash. Jörg > > It is working till v2017.05 with this fix [1]. > > > > I'm using this as my stub: > > https://github.com/openedev/u-boot-amarula/tree/icore-nand > > > > > > > > When I git bisect between v2017.05 and v2017.07, found this commit > > which is further breaking the nand boot support: > > -- > > ommit bc1fe9006dfaacc5103b5c7057a62215844957b7 > > Author: Jagan Teki > > Date: Sun May 7 02:43:05 2017 +0530 > > > > icorem6: Make SPL to pick suitable fdt > > > > SPL FIT is able to pick the suitable fdt file for u-boot, > > so add that function through board_fit_config_name_match. > > > > Cc: Stefano Babic > > Cc: Matteo Lisi > > Cc: Michael Trimarchi > > Signed-off-by: Jagan Teki > > - > > And It is fixed with this [2]. > > > > In mainline u-boot we already have fix [1] and [2] available but nand > > boot is still broken. It seems like problem is some where else, fix > > [1] and [2] are just making the bug appear less frequently. > > > > logs: > > [3] nand boot working > > [4] Nand boot not working > > > > Has anyone else faced or fixed the same issue on imx6 board. > > Please let me know. > > > > > > [1] https://paste.ubuntu.com/p/nKq7SNWDrn/ > > [2] https://paste.ubuntu.com/p/tXqbx5dVPJ/ > > [3] https://paste.ubuntu.com/p/DcBQ4gcSCM/ > > [4] https://paste.ubuntu.com/p/WVtrqfdVQT/ > > > > > > Thanks a lot, > > Shyam > > ___ > > U-Boot mailing list > > U-Boot@lists.denx.de > > https://lists.denx.de/listinfo/u-boot > ___ > U-Boot mailing list > U-Boot@lists.denx.de > https://lists.denx.de/listinfo/u-boot ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] test vs itest command confusion when using setexpr
Hi,
I noticed a difference between the `test` and the `itest` command when
using `setexpr`:
Running the test vs the itest command in the shell:
```
=> setexpr i 0xf; while test ${i} -gt 0; do echo $i; setexpr i ${i} - 1; done
=> setexpr i 0xf; while itest ${i} -gt 0; do echo $i; setexpr i ${i} - 1; done
f
e
d
c
b
a
9
8
7
6
5
4
3
2
1
=>
```
The difference between the test and the itest command is that the
integer operations for the test command is done using the base of 10
while for the itest command the operations are done using a base of 16.
lib/test.c
```
simple_strtol(ap[0], NULL, 10) > simple_strtol(ap[2], NULL, 10);
```
lib/itest.c
```
simple_strtol(s, NULL, 16) > simple_strtol(t, NULL, 16);
```
As setexpr stores the variables as hex values, it becomes obvious why
`itest` works, but `test` does not. Is this different behaviour
intended?
Best regards
Jörg Krause
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v4 2/6] mtd: nand: mxs_nand: use self init
urn; > } > memset(nand_info, 0, sizeof(struct mxs_nand_info)); > > + nand = &nand_info->chip; > + mtd = nand_to_mtd(nand); > err = mxs_nand_alloc_buffers(nand_info); > if (err) > goto err1; > @@ -1231,13 +1221,19 @@ int board_nand_init(struct nand_chip *nand) > nand->dev_ready = mxs_nand_device_ready; > nand->select_chip = mxs_nand_select_chip; > nand->block_bad = mxs_nand_block_bad; > - nand->scan_bbt = mxs_nand_scan_bbt; > > nand->read_byte = mxs_nand_read_byte; > > nand->read_buf = mxs_nand_read_buf; > nand->write_buf = mxs_nand_write_buf; > > + /* first scan to find the device and get the page size */ > + if (nand_scan_ident(mtd, CONFIG_SYS_MAX_NAND_DEVICE, NULL)) > + goto err2; > + > + if (mxs_nand_setup_ecc(mtd)) > + goto err2; > + > nand->ecc.read_page = mxs_nand_ecc_read_page; > nand->ecc.write_page= mxs_nand_ecc_write_page; > nand->ecc.read_oob = mxs_nand_ecc_read_oob; > @@ -1249,12 +1245,21 @@ int board_nand_init(struct nand_chip *nand) > nand->ecc.size = 512; > nand->ecc.strength = 8; > > - return 0; > + /* second phase scan */ > + err = nand_scan_tail(mtd); > + if (err) > + goto err2; > + > + err = nand_register(0, mtd); > + if (err) > + goto err2; > + > + return; > > err2: > free(nand_info->data_buf); > free(nand_info->cmd_buf); > err1: > free(nand_info); > - return err; > + return; > } > diff --git a/drivers/mtd/nand/mxs_nand.h b/drivers/mtd/nand/mxs_nand.h > index 9bb7148d98..379ed24f05 100644 > --- a/drivers/mtd/nand/mxs_nand.h > +++ b/drivers/mtd/nand/mxs_nand.h > @@ -8,3 +8,4 @@ > */ > > int mxs_nand_init_spl(struct nand_chip *nand); > +int mxs_nand_setup_ecc(struct mtd_info *mtd); [1] http://git.denx.de/?p=u-boot.git;a=blob;f=drivers/mtd/nand/mxs_nand_spl.c;h=2d7bbe83cce5419ad7030736a4ebd19708d5523f;hb=8c5d4fd0ec222701598a27b26ab7265d4cee45a3#l202 Best regards, Jörg Krause ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH RESEND 0/4] Add CONFIG_SPL_NAND_IDENT
Hi Stefano, On Wed, 2018-06-27 at 09:43 +0200, Stefano Babic wrote: > Hi Jörg, > > On 26/06/2018 11:33, Jörg Krause wrote: > > Hi, > > > > this patch series propably felt off the radar? > > > > I guess yes, but I doubt we can apply the series again. I guess you > should at least rebase it. No problem! I've applied the series without any conflicts on the master branch. Looks like nothing has changed in the mxs nand driver... Best regards, Jörg Krause > Regards, > Stefano > > > Best regards, > > Jörg Krause > > > > On Sun, 2018-01-14 at 19:26 +0100, Jörg Krause wrote: > > > RESEND because adding U-Boot NAND maintainer (Scott Wood) on Cc. > > > > > > When adding SPL support to a custom i.MX6ULL board with Toshiba > > > TC58NVG0S3 NAND chip the MXS NAND SPL loader failed with "Failed > > > to > > > identify". This reason is that the SPL MXS NAND driver only > > > supports > > > ONFi-compliant NAND chips and the mentioned Toshiba NAND chip is > > > non-ONFi. > > > > > > This patch set makes `nand_get_flash_type()` from `nand_base.c` > > > public, > > > so it can be used by any SPL NAND driver, introduced a new config > > > option > > > `CONFIG_SPL_NAND_IDENT` to enable the lookup for supported NAND > > > chips > > > in > > > the chip ID list. Finally, the MXS NAND SPL driver is refactored > > > so > > > that > > > the original ONFi-only identification routine is enabled by > > > default. > > > For > > > non-ONFi NAND chips the newly introduced config option can be > > > used. > > > > > > In my setup the binary size of `u-boot-spl.bin` is increased by > > > about > > > 13 kB when `CONFIG_SPL_NAND_IDENT` is enabled. As the i.MX6, as > > > well > > > as > > > the i.MX28, both have an OCRAM of 128 kB the increase in the > > > binary > > > size > > > is reasonable. > > > > > > Jörg Krause (4): > > > mtd: nand: export nand_get_flash_type function > > > spl, nand: add option CONFIG_SPL_NAND_IDENT to lookup for > > > supported > > > NAND chips > > > mtd: nand: mxs_nand_spl: refactor mxs_flash_ident > > > mtd: nand: mxs_nand_spl: add mxs_flash_full_ident > > > > > > README | 4 > > > drivers/mtd/nand/Makefile | 1 + > > > drivers/mtd/nand/mxs_nand_spl.c | 37 > > > - > > > drivers/mtd/nand/nand_base.c| 3 ++- > > > include/linux/mtd/rawnand.h | 10 +++--- > > > scripts/config_whitelist.txt| 1 + > > > 6 files changed, 51 insertions(+), 5 deletions(-) > > > > > ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH RESEND 0/4] Add CONFIG_SPL_NAND_IDENT
Hi, this patch series propably felt off the radar? Best regards, Jörg Krause On Sun, 2018-01-14 at 19:26 +0100, Jörg Krause wrote: > RESEND because adding U-Boot NAND maintainer (Scott Wood) on Cc. > > When adding SPL support to a custom i.MX6ULL board with Toshiba > TC58NVG0S3 NAND chip the MXS NAND SPL loader failed with "Failed to > identify". This reason is that the SPL MXS NAND driver only supports > ONFi-compliant NAND chips and the mentioned Toshiba NAND chip is > non-ONFi. > > This patch set makes `nand_get_flash_type()` from `nand_base.c` > public, > so it can be used by any SPL NAND driver, introduced a new config > option > `CONFIG_SPL_NAND_IDENT` to enable the lookup for supported NAND chips > in > the chip ID list. Finally, the MXS NAND SPL driver is refactored so > that > the original ONFi-only identification routine is enabled by default. > For > non-ONFi NAND chips the newly introduced config option can be used. > > In my setup the binary size of `u-boot-spl.bin` is increased by about > 13 kB when `CONFIG_SPL_NAND_IDENT` is enabled. As the i.MX6, as well > as > the i.MX28, both have an OCRAM of 128 kB the increase in the binary > size > is reasonable. > > Jörg Krause (4): > mtd: nand: export nand_get_flash_type function > spl, nand: add option CONFIG_SPL_NAND_IDENT to lookup for supported > NAND chips > mtd: nand: mxs_nand_spl: refactor mxs_flash_ident > mtd: nand: mxs_nand_spl: add mxs_flash_full_ident > > README | 4 > drivers/mtd/nand/Makefile | 1 + > drivers/mtd/nand/mxs_nand_spl.c | 37 > - > drivers/mtd/nand/nand_base.c| 3 ++- > include/linux/mtd/rawnand.h | 10 +++--- > scripts/config_whitelist.txt| 1 + > 6 files changed, 51 insertions(+), 5 deletions(-) > ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH 2/2] ARM: dts: imx6ull: add wdog3
The i.MX6ULL has a WDOG3 located at start address 0x021E in the
AIPS-2 memory region [1].
[1] i.MX 6ULL Applications Processor Reference Manual, Rev. 1, 11/2017,
Table 2-3. AIPS-2 memory map, p. 178
Signed-off-by: Jörg Krause
---
arch/arm/dts/imx6ull.dtsi | 8
1 file changed, 8 insertions(+)
diff --git a/arch/arm/dts/imx6ull.dtsi b/arch/arm/dts/imx6ull.dtsi
index 28b8422f31..f8ec649460 100644
--- a/arch/arm/dts/imx6ull.dtsi
+++ b/arch/arm/dts/imx6ull.dtsi
@@ -1026,6 +1026,14 @@
status = "disabled";
};
+ wdog3: wdog@021e4000 {
+ compatible = "fsl,imx6ul-wdt", "fsl,imx21-wdt";
+ reg = <0x021e4000 0x4000>;
+ interrupts = ;
+ clocks = <&clks IMX6UL_CLK_WDOG3>;
+ status = "disabled";
+ };
+
uart2: serial@021e8000 {
compatible = "fsl,imx6ul-uart",
"fsl,imx6q-uart", "fsl,imx21-uart";
--
2.16.2
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH 1/2] ARM: dts: imx6ul: add wdog3
The i.MX6UL has a WDOG3 located at start address 0x021E in the
AIPS-2 memory region [1].
[1] i.MX 6UltraLite Applications Processor Reference Manual, Rev. 1,
04/2016, Table-2-3 AIPS-2 memory map, p. 166
Signed-off-by: Jörg Krause
---
arch/arm/dts/imx6ul.dtsi | 8
1 file changed, 8 insertions(+)
diff --git a/arch/arm/dts/imx6ul.dtsi b/arch/arm/dts/imx6ul.dtsi
index 7affab866f..b63f5a53ac 100644
--- a/arch/arm/dts/imx6ul.dtsi
+++ b/arch/arm/dts/imx6ul.dtsi
@@ -881,6 +881,14 @@
status = "disabled";
};
+ wdog3: wdog@021e4000 {
+ compatible = "fsl,imx6ul-wdt", "fsl,imx21-wdt";
+ reg = <0x021e4000 0x4000>;
+ interrupts = ;
+ clocks = <&clks IMX6UL_CLK_WDOG3>;
+ status = "disabled";
+ };
+
uart2: serial@021e8000 {
compatible = "fsl,imx6ul-uart",
"fsl,imx6q-uart";
--
2.16.2
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH RESEND 2/4] spl, nand: add option CONFIG_SPL_NAND_IDENT to lookup for supported NAND chips
Add the config option `CONFIG_SPL_NAND_IDENT` for using the NAND chip ID list to identify the NAND flash in SPL. Signed-off-by: Jörg Krause --- README | 4 drivers/mtd/nand/Makefile| 1 + scripts/config_whitelist.txt | 1 + 3 files changed, 6 insertions(+) diff --git a/README b/README index 06f3ed057d..9ca5086097 100644 --- a/README +++ b/README @@ -2786,6 +2786,10 @@ FIT uImage format: CONFIG_SPL_NAND_DRIVERS SPL uses normal NAND drivers, not minimal drivers. + CONFIG_SPL_NAND_IDENT + SPL uses the chip ID list to identify the NAND flash. + Requires CONFIG_SPL_NAND_BASE. + CONFIG_SPL_NAND_ECC Include standard software ECC in the SPL diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile index 9f7d9d6ff7..4ed4afde10 100644 --- a/drivers/mtd/nand/Makefile +++ b/drivers/mtd/nand/Makefile @@ -17,6 +17,7 @@ obj-$(CONFIG_SPL_NAND_SIMPLE) += nand_spl_simple.o obj-$(CONFIG_SPL_NAND_LOAD) += nand_spl_load.o obj-$(CONFIG_SPL_NAND_ECC) += nand_ecc.o obj-$(CONFIG_SPL_NAND_BASE) += nand_base.o +obj-$(CONFIG_SPL_NAND_IDENT) += nand_ids.o nand_timings.o obj-$(CONFIG_SPL_NAND_INIT) += nand.o ifeq ($(CONFIG_SPL_ENV_SUPPORT),y) obj-$(CONFIG_ENV_IS_IN_NAND) += nand_util.o diff --git a/scripts/config_whitelist.txt b/scripts/config_whitelist.txt index 4e87d66bea..b01d196f5a 100644 --- a/scripts/config_whitelist.txt +++ b/scripts/config_whitelist.txt @@ -2092,6 +2092,7 @@ CONFIG_SPL_NAND_BASE CONFIG_SPL_NAND_BOOT CONFIG_SPL_NAND_DRIVERS CONFIG_SPL_NAND_ECC +CONFIG_SPL_NAND_IDENT CONFIG_SPL_NAND_INIT CONFIG_SPL_NAND_LOAD CONFIG_SPL_NAND_MINIMAL -- 2.15.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH RESEND 4/4] mtd: nand: mxs_nand_spl: add mxs_flash_full_ident
For now, the existing SPL MXS NAND driver only supports to identify
ONFi-compliant NAND chips. In order to allow identifying
non-ONFi-compliant chips add `mxs_flash_full_ident()` which uses the
`nand_get_flash_type()` functionality from `nand_base.c` to lookup
for supported NAND chips in the chip ID list.
For compatibility reason the full identification support is only
available if the config option `CONFIG_SPL_NAND_IDENT` is enabled.
The lookup was tested on a custom i.MX6ULL board with a Toshiba
TC58NVG1S3HTAI0 NAND chip.
Signed-off-by: Jörg Krause
---
drivers/mtd/nand/mxs_nand_spl.c | 28
1 file changed, 28 insertions(+)
diff --git a/drivers/mtd/nand/mxs_nand_spl.c b/drivers/mtd/nand/mxs_nand_spl.c
index 11d8503127..0c56d9ffc3 100644
--- a/drivers/mtd/nand/mxs_nand_spl.c
+++ b/drivers/mtd/nand/mxs_nand_spl.c
@@ -49,6 +49,28 @@ static void mxs_nand_command(struct mtd_info *mtd, unsigned
int command,
}
}
+#if defined (CONFIG_SPL_NAND_IDENT)
+
+/* Trying to detect the NAND flash using ONFi, JEDEC, and (extended) IDs */
+static int mxs_flash_full_ident(struct mtd_info *mtd)
+{
+ int nand_maf_id, nand_dev_id;
+ struct nand_chip *chip = mtd_to_nand(mtd);
+ struct nand_flash_dev *type;
+
+ type = nand_get_flash_type(mtd, chip, &nand_maf_id, &nand_dev_id, NULL);
+
+ if (IS_ERR(type)) {
+ chip->select_chip(mtd, -1);
+ return PTR_ERR(type);
+ }
+
+ return 0;
+}
+
+#else
+
+/* Trying to detect the NAND flash using ONFi only */
static int mxs_flash_onfi_ident(struct mtd_info *mtd)
{
register struct nand_chip *chip = mtd_to_nand(mtd);
@@ -109,10 +131,16 @@ static int mxs_flash_onfi_ident(struct mtd_info *mtd)
return 0;
}
+#endif /* CONFIG_SPL_NAND_IDENT */
+
static int mxs_flash_ident(struct mtd_info *mtd)
{
int ret;
+#if defined (CONFIG_SPL_NAND_IDENT)
+ ret = mxs_flash_full_ident(mtd);
+#else
ret = mxs_flash_onfi_ident(mtd);
+#endif
return ret;
}
--
2.15.1
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH RESEND 3/4] mtd: nand: mxs_nand_spl: refactor mxs_flash_ident
The existing `mxs_flash_ident()` is limited to identify ONFi compliant
NAND chips only. In order to support non-ONFi NAND chips refactor the
function and rename it to `mxs_flash_onfi_ident()`.
A follow-up patch will add `mxs_flash_full_ident()` which allows to use
the chip ID list to lookup for supported NAND flashs.
Signed-off-by: Jörg Krause
---
drivers/mtd/nand/mxs_nand_spl.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/mxs_nand_spl.c b/drivers/mtd/nand/mxs_nand_spl.c
index 910f76dd9d..11d8503127 100644
--- a/drivers/mtd/nand/mxs_nand_spl.c
+++ b/drivers/mtd/nand/mxs_nand_spl.c
@@ -49,7 +49,7 @@ static void mxs_nand_command(struct mtd_info *mtd, unsigned
int command,
}
}
-static int mxs_flash_ident(struct mtd_info *mtd)
+static int mxs_flash_onfi_ident(struct mtd_info *mtd)
{
register struct nand_chip *chip = mtd_to_nand(mtd);
int i;
@@ -109,6 +109,13 @@ static int mxs_flash_ident(struct mtd_info *mtd)
return 0;
}
+static int mxs_flash_ident(struct mtd_info *mtd)
+{
+ int ret;
+ ret = mxs_flash_onfi_ident(mtd);
+ return ret;
+}
+
static int mxs_read_page_ecc(struct mtd_info *mtd, void *buf, unsigned int
page)
{
register struct nand_chip *chip = mtd_to_nand(mtd);
--
2.15.1
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH RESEND 1/4] mtd: nand: export nand_get_flash_type function
`nand_get_flash_type()` allows identification of supported NAND flashs.
The function is useful in SPL (like mxs_nand_spl.c) to lookup for a NAND
flash (which does not support ONFi) instead of using nand_simple.c and
hard-coding all required NAND parameters.
Signed-off-by: Jörg Krause
---
drivers/mtd/nand/nand_base.c | 3 ++-
include/linux/mtd/rawnand.h | 10 +++---
2 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index eb9f121f81..64e4621aaa 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3755,7 +3755,7 @@ static bool find_full_id_nand(struct mtd_info *mtd,
struct nand_chip *chip,
/*
* Get the flash and manufacturer id and lookup if the type is supported.
*/
-static struct nand_flash_dev *nand_get_flash_type(struct mtd_info *mtd,
+struct nand_flash_dev *nand_get_flash_type(struct mtd_info *mtd,
struct nand_chip *chip,
int *maf_id, int *dev_id,
struct nand_flash_dev *type)
@@ -3927,6 +3927,7 @@ ident_done:
mtd->erasesize >> 10, mtd->writesize, mtd->oobsize);
return type;
}
+EXPORT_SYMBOL(nand_get_flash_type);
#if CONFIG_IS_ENABLED(OF_CONTROL)
DECLARE_GLOBAL_DATA_PTR;
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index 6c3e838d80..3871379fb1 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -23,9 +23,16 @@
#include
struct mtd_info;
+struct nand_chip;
struct nand_flash_dev;
struct device_node;
+/* Get the flash and manufacturer id and lookup if the type is supported. */
+struct nand_flash_dev *nand_get_flash_type(struct mtd_info *mtd,
+ struct nand_chip *chip,
+ int *maf_id, int *dev_id,
+ struct nand_flash_dev *type);
+
/* Scan and identify a NAND device */
int nand_scan(struct mtd_info *mtd, int max_chips);
/*
@@ -248,9 +255,6 @@ typedef enum {
#define NAND_CI_CELLTYPE_MSK 0x0C
#define NAND_CI_CELLTYPE_SHIFT 2
-/* Keep gcc happy */
-struct nand_chip;
-
/* ONFI features */
#define ONFI_FEATURE_16_BIT_BUS(1 << 0)
#define ONFI_FEATURE_EXT_PARAM_PAGE(1 << 7)
--
2.15.1
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH RESEND 0/4] Add CONFIG_SPL_NAND_IDENT
RESEND because adding U-Boot NAND maintainer (Scott Wood) on Cc. When adding SPL support to a custom i.MX6ULL board with Toshiba TC58NVG0S3 NAND chip the MXS NAND SPL loader failed with "Failed to identify". This reason is that the SPL MXS NAND driver only supports ONFi-compliant NAND chips and the mentioned Toshiba NAND chip is non-ONFi. This patch set makes `nand_get_flash_type()` from `nand_base.c` public, so it can be used by any SPL NAND driver, introduced a new config option `CONFIG_SPL_NAND_IDENT` to enable the lookup for supported NAND chips in the chip ID list. Finally, the MXS NAND SPL driver is refactored so that the original ONFi-only identification routine is enabled by default. For non-ONFi NAND chips the newly introduced config option can be used. In my setup the binary size of `u-boot-spl.bin` is increased by about 13 kB when `CONFIG_SPL_NAND_IDENT` is enabled. As the i.MX6, as well as the i.MX28, both have an OCRAM of 128 kB the increase in the binary size is reasonable. Jörg Krause (4): mtd: nand: export nand_get_flash_type function spl, nand: add option CONFIG_SPL_NAND_IDENT to lookup for supported NAND chips mtd: nand: mxs_nand_spl: refactor mxs_flash_ident mtd: nand: mxs_nand_spl: add mxs_flash_full_ident README | 4 drivers/mtd/nand/Makefile | 1 + drivers/mtd/nand/mxs_nand_spl.c | 37 - drivers/mtd/nand/nand_base.c| 3 ++- include/linux/mtd/rawnand.h | 10 +++--- scripts/config_whitelist.txt| 1 + 6 files changed, 51 insertions(+), 5 deletions(-) -- 2.15.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH 1/4] mtd: nand: export nand_get_flash_type function
On Sun, 2018-01-14 at 15:55 -0200, Fabio Estevam wrote: > Hi Jörg, > > On Sun, Jan 14, 2018 at 2:14 PM, Jörg Krause > wrote: > > `nand_get_flash_type()` allows identification of supported NAND flashs. > > The function is useful in SPL (like mxs_nand_spl.c) to lookup for a NAND > > flash (which does not support ONFi) instead of using nand_simple.c and > > hard-coding all required NAND parameters. > > > > Signed-off-by: Jörg Krause > > Thanks for your series. > > You missed to add the U-Boot NAND maintainer (Scott Wood). > > Please resend the series with him on Cc. Thanks, will do! I missed him because the custodian git repos last change is from 2016. Jörg ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH 3/4] mtd: nand: mxs_nand_spl: refactor mxs_flash_ident
The existing `mxs_flash_ident()` is limited to identify ONFi compliant
NAND chips only. In order to support non-ONFi NAND chips refactor the
function and rename it to `mxs_flash_onfi_ident()`.
A follow-up patch will add `mxs_flash_full_ident()` which allows to use
the chip ID list to lookup for supported NAND flashs.
Signed-off-by: Jörg Krause
---
drivers/mtd/nand/mxs_nand_spl.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/mxs_nand_spl.c b/drivers/mtd/nand/mxs_nand_spl.c
index 910f76dd9d..11d8503127 100644
--- a/drivers/mtd/nand/mxs_nand_spl.c
+++ b/drivers/mtd/nand/mxs_nand_spl.c
@@ -49,7 +49,7 @@ static void mxs_nand_command(struct mtd_info *mtd, unsigned
int command,
}
}
-static int mxs_flash_ident(struct mtd_info *mtd)
+static int mxs_flash_onfi_ident(struct mtd_info *mtd)
{
register struct nand_chip *chip = mtd_to_nand(mtd);
int i;
@@ -109,6 +109,13 @@ static int mxs_flash_ident(struct mtd_info *mtd)
return 0;
}
+static int mxs_flash_ident(struct mtd_info *mtd)
+{
+ int ret;
+ ret = mxs_flash_onfi_ident(mtd);
+ return ret;
+}
+
static int mxs_read_page_ecc(struct mtd_info *mtd, void *buf, unsigned int
page)
{
register struct nand_chip *chip = mtd_to_nand(mtd);
--
2.15.1
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH 4/4] mtd: nand: mxs_nand_spl: add mxs_flash_full_ident
For now, the existing SPL MXS NAND driver only supports to identify
ONFi-compliant NAND chips. In order to allow identifying
non-ONFi-compliant chips add `mxs_flash_full_ident()` which uses the
`nand_get_flash_type()` functionality from `nand_base.c` to lookup
for supported NAND chips in the chip ID list.
For compatibility reason the full identification support is only
available if the config option `CONFIG_SPL_NAND_IDENT` is enabled.
The lookup was tested on a custom i.MX6ULL board with a Toshiba
TC58NVG1S3HTAI0 NAND chip.
Signed-off-by: Jörg Krause
---
drivers/mtd/nand/mxs_nand_spl.c | 28
1 file changed, 28 insertions(+)
diff --git a/drivers/mtd/nand/mxs_nand_spl.c b/drivers/mtd/nand/mxs_nand_spl.c
index 11d8503127..0c56d9ffc3 100644
--- a/drivers/mtd/nand/mxs_nand_spl.c
+++ b/drivers/mtd/nand/mxs_nand_spl.c
@@ -49,6 +49,28 @@ static void mxs_nand_command(struct mtd_info *mtd, unsigned
int command,
}
}
+#if defined (CONFIG_SPL_NAND_IDENT)
+
+/* Trying to detect the NAND flash using ONFi, JEDEC, and (extended) IDs */
+static int mxs_flash_full_ident(struct mtd_info *mtd)
+{
+ int nand_maf_id, nand_dev_id;
+ struct nand_chip *chip = mtd_to_nand(mtd);
+ struct nand_flash_dev *type;
+
+ type = nand_get_flash_type(mtd, chip, &nand_maf_id, &nand_dev_id, NULL);
+
+ if (IS_ERR(type)) {
+ chip->select_chip(mtd, -1);
+ return PTR_ERR(type);
+ }
+
+ return 0;
+}
+
+#else
+
+/* Trying to detect the NAND flash using ONFi only */
static int mxs_flash_onfi_ident(struct mtd_info *mtd)
{
register struct nand_chip *chip = mtd_to_nand(mtd);
@@ -109,10 +131,16 @@ static int mxs_flash_onfi_ident(struct mtd_info *mtd)
return 0;
}
+#endif /* CONFIG_SPL_NAND_IDENT */
+
static int mxs_flash_ident(struct mtd_info *mtd)
{
int ret;
+#if defined (CONFIG_SPL_NAND_IDENT)
+ ret = mxs_flash_full_ident(mtd);
+#else
ret = mxs_flash_onfi_ident(mtd);
+#endif
return ret;
}
--
2.15.1
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH 0/4] Add CONFIG_SPL_NAND_IDENT
When adding SPL support to a custom i.MX6ULL board with Toshiba TC58NVG0S3 NAND chip the MXS NAND SPL loader failed with "Failed to identify". This reason is that the SPL MXS NAND driver only supports ONFi-compliant NAND chips and the mentioned Toshiba NAND chip is non-ONFi. This patch set makes `nand_get_flash_type()` from `nand_base.c` public, so it can be used by any SPL NAND driver, introduced a new config option `CONFIG_SPL_NAND_IDENT` to enable the lookup for supported NAND chips in the chip ID list. Finally, the MXS NAND SPL driver is refactored so that the original ONFi-only identification routine is enabled by default. For non-ONFi NAND chips the newly introduced config option can be used. In my setup the binary size of `u-boot-spl.bin` is increased by about 13 kB when `CONFIG_SPL_NAND_IDENT` is enabled. As the i.MX6, as well as the i.MX28, both have an OCRAM of 128 kB the increase in the binary size is reasonable. Jörg Krause (4): mtd: nand: export nand_get_flash_type function spl, nand: add option CONFIG_SPL_NAND_IDENT to lookup for supported NAND chips mtd: nand: mxs_nand_spl: refactor mxs_flash_ident mtd: nand: mxs_nand_spl: add mxs_flash_full_ident README | 4 drivers/mtd/nand/Makefile | 1 + drivers/mtd/nand/mxs_nand_spl.c | 37 - drivers/mtd/nand/nand_base.c| 3 ++- include/linux/mtd/rawnand.h | 10 +++--- scripts/config_whitelist.txt| 1 + 6 files changed, 51 insertions(+), 5 deletions(-) -- 2.15.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH 1/4] mtd: nand: export nand_get_flash_type function
`nand_get_flash_type()` allows identification of supported NAND flashs.
The function is useful in SPL (like mxs_nand_spl.c) to lookup for a NAND
flash (which does not support ONFi) instead of using nand_simple.c and
hard-coding all required NAND parameters.
Signed-off-by: Jörg Krause
---
drivers/mtd/nand/nand_base.c | 3 ++-
include/linux/mtd/rawnand.h | 10 +++---
2 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index eb9f121f81..64e4621aaa 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3755,7 +3755,7 @@ static bool find_full_id_nand(struct mtd_info *mtd,
struct nand_chip *chip,
/*
* Get the flash and manufacturer id and lookup if the type is supported.
*/
-static struct nand_flash_dev *nand_get_flash_type(struct mtd_info *mtd,
+struct nand_flash_dev *nand_get_flash_type(struct mtd_info *mtd,
struct nand_chip *chip,
int *maf_id, int *dev_id,
struct nand_flash_dev *type)
@@ -3927,6 +3927,7 @@ ident_done:
mtd->erasesize >> 10, mtd->writesize, mtd->oobsize);
return type;
}
+EXPORT_SYMBOL(nand_get_flash_type);
#if CONFIG_IS_ENABLED(OF_CONTROL)
DECLARE_GLOBAL_DATA_PTR;
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index 6c3e838d80..3871379fb1 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -23,9 +23,16 @@
#include
struct mtd_info;
+struct nand_chip;
struct nand_flash_dev;
struct device_node;
+/* Get the flash and manufacturer id and lookup if the type is supported. */
+struct nand_flash_dev *nand_get_flash_type(struct mtd_info *mtd,
+ struct nand_chip *chip,
+ int *maf_id, int *dev_id,
+ struct nand_flash_dev *type);
+
/* Scan and identify a NAND device */
int nand_scan(struct mtd_info *mtd, int max_chips);
/*
@@ -248,9 +255,6 @@ typedef enum {
#define NAND_CI_CELLTYPE_MSK 0x0C
#define NAND_CI_CELLTYPE_SHIFT 2
-/* Keep gcc happy */
-struct nand_chip;
-
/* ONFI features */
#define ONFI_FEATURE_16_BIT_BUS(1 << 0)
#define ONFI_FEATURE_EXT_PARAM_PAGE(1 << 7)
--
2.15.1
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH 2/4] spl, nand: add option CONFIG_SPL_NAND_IDENT to lookup for supported NAND chips
Add the config option `CONFIG_SPL_NAND_IDENT` for using the NAND chip ID list to identify the NAND flash in SPL. Signed-off-by: Jörg Krause --- README | 4 drivers/mtd/nand/Makefile| 1 + scripts/config_whitelist.txt | 1 + 3 files changed, 6 insertions(+) diff --git a/README b/README index 06f3ed057d..9ca5086097 100644 --- a/README +++ b/README @@ -2786,6 +2786,10 @@ FIT uImage format: CONFIG_SPL_NAND_DRIVERS SPL uses normal NAND drivers, not minimal drivers. + CONFIG_SPL_NAND_IDENT + SPL uses the chip ID list to identify the NAND flash. + Requires CONFIG_SPL_NAND_BASE. + CONFIG_SPL_NAND_ECC Include standard software ECC in the SPL diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile index 9f7d9d6ff7..4ed4afde10 100644 --- a/drivers/mtd/nand/Makefile +++ b/drivers/mtd/nand/Makefile @@ -17,6 +17,7 @@ obj-$(CONFIG_SPL_NAND_SIMPLE) += nand_spl_simple.o obj-$(CONFIG_SPL_NAND_LOAD) += nand_spl_load.o obj-$(CONFIG_SPL_NAND_ECC) += nand_ecc.o obj-$(CONFIG_SPL_NAND_BASE) += nand_base.o +obj-$(CONFIG_SPL_NAND_IDENT) += nand_ids.o nand_timings.o obj-$(CONFIG_SPL_NAND_INIT) += nand.o ifeq ($(CONFIG_SPL_ENV_SUPPORT),y) obj-$(CONFIG_ENV_IS_IN_NAND) += nand_util.o diff --git a/scripts/config_whitelist.txt b/scripts/config_whitelist.txt index 4e87d66bea..b01d196f5a 100644 --- a/scripts/config_whitelist.txt +++ b/scripts/config_whitelist.txt @@ -2092,6 +2092,7 @@ CONFIG_SPL_NAND_BASE CONFIG_SPL_NAND_BOOT CONFIG_SPL_NAND_DRIVERS CONFIG_SPL_NAND_ECC +CONFIG_SPL_NAND_IDENT CONFIG_SPL_NAND_INIT CONFIG_SPL_NAND_LOAD CONFIG_SPL_NAND_MINIMAL -- 2.15.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v3] mx6ull: Handle the CONFIG_MX6ULL cases correctly
/* 0x1d0/0x1b0 */
> @@ -64,8 +69,10 @@ struct mxs_lcdif_regs {
> mxs_reg_32(hw_lcdif_debug0) /* 0x1f0/0x1d0 */
> mxs_reg_32(hw_lcdif_debug1) /* 0x200/0x1e0 */
> mxs_reg_32(hw_lcdif_debug2) /* 0x1f0 */
> -#if defined(CONFIG_MX6SX) || defined(CONFIG_MX6UL) || defined(CONFIG_MX7) ||
> \
> - defined(CONFIG_MX6SL) || defined(CONFIG_MX6SLL)
> +#if defined(CONFIG_MX6SX) || \
> + defined(CONFIG_MX6SL) || defined(CONFIG_MX6SLL) || \
> + defined(CONFIG_MX6UL) || defined(CONFIG_MX6ULL) || \
> + defined(CONFIG_MX7)
> mxs_reg_32(hw_lcdif_thres)
> mxs_reg_32(hw_lcdif_as_ctrl)
> mxs_reg_32(hw_lcdif_as_buf)
> diff --git a/arch/arm/mach-imx/mx6/Kconfig b/arch/arm/mach-imx/mx6/Kconfig
> index 18e1e67..2552a50 100644
> --- a/arch/arm/mach-imx/mx6/Kconfig
> +++ b/arch/arm/mach-imx/mx6/Kconfig
> @@ -8,7 +8,7 @@ config MX6_SMP
> bool
>
> config MX6
> - select ARM_ERRATA_743622 if !MX6UL
> + select ARM_ERRATA_743622 if !MX6UL && !MX6ULL
> bool
> default y
> imply CMD_FUSE
> diff --git a/arch/arm/mach-imx/mx6/ddr.c b/arch/arm/mach-imx/mx6/ddr.c
> index 0cf391e..52a9a25 100644
> --- a/arch/arm/mach-imx/mx6/ddr.c
> +++ b/arch/arm/mach-imx/mx6/ddr.c
> @@ -631,7 +631,7 @@ void mx6sx_dram_iocfg(unsigned width,
> }
> #endif
>
> -#ifdef CONFIG_MX6UL
> +#if defined(CONFIG_MX6UL) || defined(CONFIG_MX6ULL)
> void mx6ul_dram_iocfg(unsigned width,
> const struct mx6ul_iomux_ddr_regs *ddr,
> const struct mx6ul_iomux_grp_regs *grp)
> diff --git a/drivers/gpio/mxc_gpio.c b/drivers/gpio/mxc_gpio.c
> index c480eba..cfa620b 100644
> --- a/drivers/gpio/mxc_gpio.c
> +++ b/drivers/gpio/mxc_gpio.c
> @@ -47,12 +47,12 @@ static unsigned long gpio_ports[] = {
> #if defined(CONFIG_MX27) || defined(CONFIG_MX53) || defined(CONFIG_MX6) || \
> defined(CONFIG_MX7)
> [4] = GPIO5_BASE_ADDR,
> -#ifndef CONFIG_MX6UL
> +#if !(defined(CONFIG_MX6UL) || defined(CONFIG_MX6ULL))
> [5] = GPIO6_BASE_ADDR,
> #endif
> #endif
> #if defined(CONFIG_MX53) || defined(CONFIG_MX6) || defined(CONFIG_MX7)
> -#ifndef CONFIG_MX6UL
> +#if !(defined(CONFIG_MX6UL) || defined(CONFIG_MX6ULL))
> [6] = GPIO7_BASE_ADDR,
> #endif
> #endif
> diff --git a/include/configs/imx6_spl.h b/include/configs/imx6_spl.h
> index cdb3a37..dd48120 100644
> --- a/include/configs/imx6_spl.h
> +++ b/include/configs/imx6_spl.h
> @@ -55,7 +55,8 @@
> # endif
> #endif
>
> -#if defined(CONFIG_MX6SX) || defined(CONFIG_MX6UL) || defined(CONFIG_MX6SL)
> +#if defined(CONFIG_MX6SX) || defined(CONFIG_MX6SL) || \
> + defined(CONFIG_MX6UL) || defined(CONFIG_MX6ULL)
> #define CONFIG_SPL_BSS_START_ADDR 0x8820
> #define CONFIG_SPL_BSS_MAX_SIZE0x10 /* 1 MB */
> #define CONFIG_SYS_SPL_MALLOC_START0x8830
> diff --git a/include/configs/mx6_common.h b/include/configs/mx6_common.h
> index 5fb85a1..59e6dae 100644
> --- a/include/configs/mx6_common.h
> +++ b/include/configs/mx6_common.h
> @@ -7,7 +7,7 @@
> #ifndef __MX6_COMMON_H
> #define __MX6_COMMON_H
>
> -#ifndef CONFIG_MX6UL
> +#if !(defined(CONFIG_MX6UL) || defined(CONFIG_MX6ULL))
> #ifndef CONFIG_SYS_L2CACHE_OFF
> #define CONFIG_SYS_L2_PL310
> #define CONFIG_SYS_PL310_BASEL2_PL310_BASE
> @@ -37,8 +37,9 @@
> #define CONFIG_REVISION_TAG
>
> /* Boot options */
> -#if (defined(CONFIG_MX6SX) || defined(CONFIG_MX6SL) || \
> - defined(CONFIG_MX6UL) || defined(CONFIG_MX6SLL))
> +#if defined(CONFIG_MX6SL) || defined(CONFIG_MX6SLL) || \
> + defined(CONFIG_MX6SX) || \
> + defined(CONFIG_MX6UL) || defined(CONFIG_MX6ULL)
> #define CONFIG_LOADADDR 0x8200
> #ifndef CONFIG_SYS_TEXT_BASE
> #define CONFIG_SYS_TEXT_BASE 0x8780
Tested on a custom i.MX6ULL board.
Tested-by: Jörg Krause
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH] gpio: Kconfig: DM_GPIO depends on OF_CONTROL
Building U-Boot with DM_GPIO set, but with OF_CONTROL unset fails. Fixes: drivers/gpio/built-in.o: In function `gpio_request_tail': drivers/gpio/gpio-uclass.c:666: undefined reference to `ofnode_get_name' drivers/gpio/built-in.o: In function `_gpio_request_by_name_nodev': drivers/gpio/gpio-uclass.c:693: undefined reference to `ofnode_parse_phandle_with_args' Signed-off-by: Jörg Krause --- drivers/gpio/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index ffeda9425a..d79beeafb5 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -7,6 +7,7 @@ menu "GPIO Support" config DM_GPIO bool "Enable Driver Model for GPIO drivers" depends on DM + depends on OF_CONTROL help Enable driver model for GPIO access. The standard GPIO interface (gpio_get_value(), etc.) is then implemented by -- 2.14.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v2] net/tftp: fix build if CMD_BOOTEFI is not set
Fixes:
net/tftp.c:811: undefined reference to `efi_set_bootdev'
Signed-off-by: Jörg Krause
---
v2:
* remove ifdef for efi header file (suggested by Bin Weng)
---
net/tftp.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/tftp.c b/net/tftp.c
index a5ed8c5d0a..6671b1f7ca 100644
--- a/net/tftp.c
+++ b/net/tftp.c
@@ -805,7 +805,9 @@ void tftp_start(enum proto_t protocol)
printf("Load address: 0x%lx\n", load_addr);
puts("Loading: *\b");
tftp_state = STATE_SEND_RRQ;
+#ifdef CONFIG_CMD_BOOTEFI
efi_set_bootdev("Net", "", tftp_filename);
+#endif
}
time_start = get_timer(0);
--
2.14.1
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH] net/tftp: fix build if CMD_BOOTEFI is not set
Fixes:
net/tftp.c:811: undefined reference to `efi_set_bootdev'
Signed-off-by: Jörg Krause
---
net/tftp.c | 4
1 file changed, 4 insertions(+)
diff --git a/net/tftp.c b/net/tftp.c
index a5ed8c5d0a..4a9a5a5d05 100644
--- a/net/tftp.c
+++ b/net/tftp.c
@@ -8,7 +8,9 @@
#include
#include
+#ifdef CONFIG_CMD_BOOTEFI
#include
+#endif
#include
#include
#include
@@ -805,7 +807,9 @@ void tftp_start(enum proto_t protocol)
printf("Load address: 0x%lx\n", load_addr);
puts("Loading: *\b");
tftp_state = STATE_SEND_RRQ;
+#ifdef CONFIG_CMD_BOOTEFI
efi_set_bootdev("Net", "", tftp_filename);
+#endif
}
time_start = get_timer(0);
--
2.14.1
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] Data Abort with gcc 7.1
Hi, On Thu, 2017-07-13 at 01:43 +0100, Måns Rullgård wrote: > Maxime Ripard writes: > > > Hi, > > > > I recently got a gcc 7.1 based toolchain, and it seems like it > > generates unaligned code, specifically in the net_set_ip_header > > function in my case. > > > > Whenever some packet is sent, this data abort is triggered: > > > > => setenv ipaddr 10.42.0.1; ping 10.42.0.254 > > using musb-hdrc, OUT ep1out IN ep1in STATUS ep2in > > MAC de:ad:be:ef:00:01 > > HOST MAC de:ad:be:af:00:00 > > RNDIS ready > > musb-hdrc: peripheral reset irq lost! > > high speed config #2: 2 mA, Ethernet Gadget, using RNDIS > > USB RNDIS network up! > > Using usb_ether device > > data abort > > pc : [<7ff9db10>] lr : [<7ff9f00c>] > > reloc pc : [<4a043b10>]lr : [<4a04500c>] > > sp : 7bf37cc8 ip : fp : 7ff6236c > > r10: 7ffed2b8 r9 : 7bf39ee8 r8 : 7ffed2b8 > > r7 : 0001 r6 : r5 : 002a r4 : 7ffed30e > > r3 : 1445 r2 : 01002a0a r1 : fe002a0a r0 : 7ffed30e > > Flags: nZCv IRQs off FIQs off Mode SVC_32 > > Resetting CPU ... I'm running into the same issue using a Buildroot GCC 7 toolchain. [snip] > What hardware did this happen on? If it was on ARMv5, adding the packed > attribute is probably the correct fix. If it was ARMv6 or later, > something else is broken as well. > FWIW, the board I'm using is ARMv5 (an i.MX28). Jörg Krause ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v5 5/16] cmd: Add Kconfig option for CMD_MTDPARTS and related options
Hi Maxime, On Wed, 2017-05-31 at 21:47 +0200, Maxime Ripard wrote: > On Wed, May 31, 2017 at 08:27:33AM +0200, Jörg Krause wrote: > > Hi Maxime, > > > > On Tue, 2017-05-30 at 23:09 +0200, Maxime Ripard wrote: > > > Hi Jörg, > > > > > > On Tue, May 30, 2017 at 09:39:57AM +0200, Jörg Krause wrote: > > > > On Mon, 2017-02-27 at 18:22 +0100, Maxime Ripard wrote: > > > > > CMD_MTDPARTS is something the user might or might not want to > > > > > select, > > > > > and > > > > > might depends on (or be selected by) other options too. > > > > > > > > > > This is even truer for the MTDIDS_DEFAULT and MTDPARTS_DEFAULT > > > > > options that > > > > > might change from one board to another, or from one user to the > > > > > other, > > > > > depending on what it expects and what storage devices are > > > > > available. > > > > > > > > > > In order to ease that configuration, add those options to > > > > > Kconfig. > > > > > > > > > > Signed-off-by: Maxime Ripard > > > > > Reviewed-by: Tom Rini > > > > > --- > > > > > cmd/Kconfig| 20 > > > > > cmd/mtdparts.c | 8 > > > > > 2 files changed, 28 insertions(+), 0 deletions(-) > > > > > > > > > > diff --git a/cmd/Kconfig b/cmd/Kconfig > > > > > index ef5315631476..0734d669dbd7 100644 > > > > > --- a/cmd/Kconfig > > > > > +++ b/cmd/Kconfig > > > > > @@ -801,6 +801,26 @@ config CMD_FS_GENERIC > > > > > help > > > > > Enables filesystem commands (e.g. load, ls) that work > > > > > for > > > > > multiple > > > > > fs types. > > > > > + > > > > > +config CMD_MTDPARTS > > > > > + depends on ARCH_SUNXI > > > > > > > > Is there any reason to limit the command for the sunxi arch only? > > > > > > Yes, if we don't, this will generate warnings for each architecture > > > that has not moved that option from their header to Kconfig. > > > > I see! However, wouldn't it be best to migrate all architectures to > > Kconfig instead of doing it seperately? > > Probably, but there was a quite significant number of Kconfig symbols > introduced, used by a very significant number of boards and > architectures. I see! > This was really to unreasonable to do at the time, but we can > definitely do that now. That would be great! Many thanks! Jörg ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v5 5/16] cmd: Add Kconfig option for CMD_MTDPARTS and related options
Hi Maxime, On Wed, 2017-05-31 at 21:47 +0200, Maxime Ripard wrote: > On Wed, May 31, 2017 at 08:27:33AM +0200, Jörg Krause wrote: > > Hi Maxime, > > > > On Tue, 2017-05-30 at 23:09 +0200, Maxime Ripard wrote: > > > Hi Jörg, > > > > > > On Tue, May 30, 2017 at 09:39:57AM +0200, Jörg Krause wrote: > > > > On Mon, 2017-02-27 at 18:22 +0100, Maxime Ripard wrote: > > > > > CMD_MTDPARTS is something the user might or might not want to > > > > > select, > > > > > and > > > > > might depends on (or be selected by) other options too. > > > > > > > > > > This is even truer for the MTDIDS_DEFAULT and MTDPARTS_DEFAULT > > > > > options that > > > > > might change from one board to another, or from one user to the > > > > > other, > > > > > depending on what it expects and what storage devices are > > > > > available. > > > > > > > > > > In order to ease that configuration, add those options to > > > > > Kconfig. > > > > > > > > > > Signed-off-by: Maxime Ripard > > > > > Reviewed-by: Tom Rini > > > > > --- > > > > > cmd/Kconfig| 20 > > > > > cmd/mtdparts.c | 8 > > > > > 2 files changed, 28 insertions(+), 0 deletions(-) > > > > > > > > > > diff --git a/cmd/Kconfig b/cmd/Kconfig > > > > > index ef5315631476..0734d669dbd7 100644 > > > > > --- a/cmd/Kconfig > > > > > +++ b/cmd/Kconfig > > > > > @@ -801,6 +801,26 @@ config CMD_FS_GENERIC > > > > > help > > > > > Enables filesystem commands (e.g. load, ls) that work > > > > > for > > > > > multiple > > > > > fs types. > > > > > + > > > > > +config CMD_MTDPARTS > > > > > + depends on ARCH_SUNXI > > > > > > > > Is there any reason to limit the command for the sunxi arch only? > > > > > > Yes, if we don't, this will generate warnings for each architecture > > > that has not moved that option from their header to Kconfig. > > > > I see! However, wouldn't it be best to migrate all architectures to > > Kconfig instead of doing it seperately? > > Probably, but there was a quite significant number of Kconfig symbols > introduced, used by a very significant number of boards and > architectures. I see! > This was really to unreasonable to do at the time, but we can > definitely do that now. That would be great! Many thanks! Jörg ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v5 5/16] cmd: Add Kconfig option for CMD_MTDPARTS and related options
Hi Maxime, On Tue, 2017-05-30 at 23:09 +0200, Maxime Ripard wrote: > Hi Jörg, > > On Tue, May 30, 2017 at 09:39:57AM +0200, Jörg Krause wrote: > > On Mon, 2017-02-27 at 18:22 +0100, Maxime Ripard wrote: > > > CMD_MTDPARTS is something the user might or might not want to > > > select, > > > and > > > might depends on (or be selected by) other options too. > > > > > > This is even truer for the MTDIDS_DEFAULT and MTDPARTS_DEFAULT > > > options that > > > might change from one board to another, or from one user to the > > > other, > > > depending on what it expects and what storage devices are > > > available. > > > > > > In order to ease that configuration, add those options to > > > Kconfig. > > > > > > Signed-off-by: Maxime Ripard > > > Reviewed-by: Tom Rini > > > --- > > > cmd/Kconfig| 20 > > > cmd/mtdparts.c | 8 > > > 2 files changed, 28 insertions(+), 0 deletions(-) > > > > > > diff --git a/cmd/Kconfig b/cmd/Kconfig > > > index ef5315631476..0734d669dbd7 100644 > > > --- a/cmd/Kconfig > > > +++ b/cmd/Kconfig > > > @@ -801,6 +801,26 @@ config CMD_FS_GENERIC > > > help > > > Enables filesystem commands (e.g. load, ls) that work > > > for > > > multiple > > > fs types. > > > + > > > +config CMD_MTDPARTS > > > + depends on ARCH_SUNXI > > > > Is there any reason to limit the command for the sunxi arch only? > > Yes, if we don't, this will generate warnings for each architecture > that has not moved that option from their header to Kconfig. I see! However, wouldn't it be best to migrate all architectures to Kconfig instead of doing it seperately? Jörg ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v5 7/16] cmd: Expose a Kconfig option to enable UBIFS commands
Hi Maxime, On Mon, 2017-02-27 at 18:22 +0100, Maxime Ripard wrote: > From: Boris Brezillon > > Create a new Kconfig entry to allow CMD_UBIFS selection from Kconfig > and > add an hidden LZO option that can be selected by CMD_UBIFS. > > Signed-off-by: Boris Brezillon > Signed-off-by: Maxime Ripard > --- > cmd/Kconfig | 8 > lib/Kconfig | 2 ++ > 2 files changed, 10 insertions(+), 0 deletions(-) > > diff --git a/cmd/Kconfig b/cmd/Kconfig > index 0734d669dbd7..66d3273fac6f 100644 > --- a/cmd/Kconfig > +++ b/cmd/Kconfig > @@ -835,4 +835,12 @@ config CMD_UBI > (www.linux-mtd.infradead.org). Activate this option if you > want > to use U-Boot UBI commands. > > +config CMD_UBIFS > + tristate "Enable UBIFS - Unsorted block images filesystem > commands" > + select CRC32 > + select RBTREE if ARCH_SUNXI > + select LZO if ARCH_SUNXI RBTREE and LZO shouldn't depend on the sunxi arch. Best regards Jörg Krause ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v5 5/16] cmd: Add Kconfig option for CMD_MTDPARTS and related options
Hi Maxime, On Mon, 2017-02-27 at 18:22 +0100, Maxime Ripard wrote: > CMD_MTDPARTS is something the user might or might not want to select, > and > might depends on (or be selected by) other options too. > > This is even truer for the MTDIDS_DEFAULT and MTDPARTS_DEFAULT > options that > might change from one board to another, or from one user to the > other, > depending on what it expects and what storage devices are available. > > In order to ease that configuration, add those options to Kconfig. > > Signed-off-by: Maxime Ripard > Reviewed-by: Tom Rini > --- > cmd/Kconfig| 20 > cmd/mtdparts.c | 8 > 2 files changed, 28 insertions(+), 0 deletions(-) > > diff --git a/cmd/Kconfig b/cmd/Kconfig > index ef5315631476..0734d669dbd7 100644 > --- a/cmd/Kconfig > +++ b/cmd/Kconfig > @@ -801,6 +801,26 @@ config CMD_FS_GENERIC > help > Enables filesystem commands (e.g. load, ls) that work for > multiple > fs types. > + > +config CMD_MTDPARTS > + depends on ARCH_SUNXI Is there any reason to limit the command for the sunxi arch only? Best regards Jörg Krause ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH] tools: binman: change shebang from python into python2
This tool does not work with Python 3. Change the shebang to make sure the script is run by a Python 2 interpreter. Signed-off-by: Jörg Krause --- tools/binman/binman.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/binman/binman.py b/tools/binman/binman.py index e1cb2fbb6f..857d698b4c 100755 --- a/tools/binman/binman.py +++ b/tools/binman/binman.py @@ -1,4 +1,4 @@ -#!/usr/bin/env python +#!/usr/bin/env python2 # Copyright (c) 2016 Google, Inc # Written by Simon Glass -- 2.12.0 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v6 01/13] binman: Introduce binman, a tool for building binary images
Hi, On Fri, 2016-11-25 at 20:15 -0700, Simon Glass wrote: > This adds the basic code for binman, including command parsing, > processing > of entries and generation of images. > > So far no entry types are supported. These will be added in future > commits > as examples of how to add new types. > > See the README for documentation. > > Signed-off-by: Simon Glass > --- > [snip] > + > +To do > +- > + > +Some ideas: > +- Fill out the device tree to include the final position and size of > each > + entry (since the input file may not always specify these) > +- Use of-platdata to make the information available to code that is > unable > + to use device tree (such as a very small SPL image) > +- Write an image map to a text file > +- Allow easy building of images by specifying just the board name > +- Produce a full Python binding for libfdt (for upstream) > +- Add an option to decode an image into the constituent binaries > +- Suppoort hierarchical images (packing of binaries into another > binary > + which is then placed in the image) > +- Support building an image for a board (-b) more completely, with a > + configurable build directory > +- Consider making binman work with buildman, although if it is used > in the > + Makefile, this will be automatic > +- Implement align-end Any plans to add support for Python 3 as it is done for patman? Best regards, Jörg Krause ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v7 18/21] mtd: nand: Kconfig: Add NAND_MXS entry
On Mi, 2016-10-12 at 11:20 +0530, Jagan Teki wrote: > On Wed, Oct 12, 2016 at 3:40 AM, Jörg Krause > wrote: > > > > On Sa, 2016-10-08 at 18:00 +0530, Jagan Teki wrote: > > > > > > From: Jagan Teki > > > > > > Added kconfig for NAND_MXS driver. > > > > > > Cc: Scott Wood > > > Cc: Simon Glass > > > Cc: Fabio Estevam > > > Cc: Stefano Babic > > > Cc: Peng Fan > > > Cc: Matteo Lisi > > > Cc: Michael Trimarchi > > > Signed-off-by: Jagan Teki > > > --- > > > drivers/mtd/nand/Kconfig | 7 +++ > > > 1 file changed, 7 insertions(+) > > > > > > diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig > > > index 5ce7d6d..df154bf 100644 > > > --- a/drivers/mtd/nand/Kconfig > > > +++ b/drivers/mtd/nand/Kconfig > > > @@ -80,6 +80,13 @@ config NAND_ARASAN > > > controller. This uses the hardware ECC for read and > > > write operations. > > > > > > +config NAND_MXS > > > + bool "MXS NAND support" > > > + depends on MX6 > > > > Isn't mxs supposed to be i.MX23/i.MX28 and not i.MX6? > > Yes will ||ed once the nand config used by MX23/28 are planing to > move > defconfig. I see! Thanks! ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v7 18/21] mtd: nand: Kconfig: Add NAND_MXS entry
On Sa, 2016-10-08 at 18:00 +0530, Jagan Teki wrote: > From: Jagan Teki > > Added kconfig for NAND_MXS driver. > > Cc: Scott Wood > Cc: Simon Glass > Cc: Fabio Estevam > Cc: Stefano Babic > Cc: Peng Fan > Cc: Matteo Lisi > Cc: Michael Trimarchi > Signed-off-by: Jagan Teki > --- > drivers/mtd/nand/Kconfig | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig > index 5ce7d6d..df154bf 100644 > --- a/drivers/mtd/nand/Kconfig > +++ b/drivers/mtd/nand/Kconfig > @@ -80,6 +80,13 @@ config NAND_ARASAN > controller. This uses the hardware ECC for read and > write operations. > > +config NAND_MXS > + bool "MXS NAND support" > + depends on MX6 Isn't mxs supposed to be i.MX23/i.MX28 and not i.MX6? > + help > + This enables NAND driver for the NAND flash controller on > the > + MXS processors. > + > comment "Generic NAND options" > > # Enhance depends when converting drivers to Kconfig which use this > config Best regards Jörg Krause ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Revert "net: phy: delay only if reset handler is registered"
Hi Fabio, On Mo, 2016-01-04 at 10:57 -0200, Fabio Estevam wrote: > Hi Jörg, > > On Mon, Jan 4, 2016 at 10:49 AM, Jörg Krause > wrote: > > > Do you know which PHY is used on mx6cuboxi? > > It is an AR8035. > > > I am wondering if the ANATOP clock setting is correct (25MHz)? > > > > http://git.denx.de/?p=u-boot.git;a=blob;f=board/solidrun/mx6cuboxi/ > > mx6c > > uboxi.c;h=fc37f1eef06da5147e5403d4272d220836c9cfbc;hb=HEAD#l167 > > Yes, this seems correct for this board. > > > Does it help to increase the delay and set it to 15ms? > > It does not help. mx6cuboxi.h defines CONFIG_FEC_MXC and CONFIG_PHYLIB, but does not use cpu_eth_init() [which calls fecmxc_initialize()]. Is there any reason for this? Jörg ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Revert "net: phy: delay only if reset handler is registered"
Hi Fabio,
On Mi, 2015-12-23 at 13:42 -0200, Fabio Estevam wrote:
> Hi Jörg,
>
> On Mon, Nov 23, 2015 at 4:16 PM, Fabio Estevam
> wrote:
> > Hi Jörg ,
> >
> > On Fri, Nov 20, 2015 at 6:37 PM, Fabio Estevam
> > wrote:
> >
> > > Ok, I will test your proposal below on Monday when I get access
> > > to my
> > > mx6sxsabresd, thanks.
> >
> > Your proposal worked fine, thanks. Will send it as a formal patch.
>
> Ethernet on mx6cuboxi is also broken because of
> 59370f3fcd13508
> (""net: phy: delay only if reset handler is registered").
>
> I tried doing:
>
> --- a/board/solidrun/mx6cuboxi/mx6cuboxi.c
> +++ b/board/solidrun/mx6cuboxi/mx6cuboxi.c
> @@ -143,7 +143,7 @@ static void setup_iomux_enet(void)
> SETUP_IOMUX_PADS(enet_pads);
>
> gpio_direction_output(ETH_PHY_RESET, 0);
> - mdelay(2);
> + mdelay(10);
> gpio_set_value(ETH_PHY_RESET, 1);
> }
>
> ,but it did not help.
>
> Any suggestions?
Sorry for the long delay due to christmas vacation.
Do you know which PHY is used on mx6cuboxi?
I am wondering if the ANATOP clock setting is correct (25MHz)?
http://git.denx.de/?p=u-boot.git;a=blob;f=board/solidrun/mx6cuboxi/mx6c
uboxi.c;h=fc37f1eef06da5147e5403d4272d220836c9cfbc;hb=HEAD#l167
Does it help to increase the delay and set it to 15ms?
Best regards
Jörg Krause
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Revert "net: phy: delay only if reset handler is registered"
On Mo, 2015-11-23 at 16:16 -0200, Fabio Estevam wrote: > Hi Jörg , > > On Fri, Nov 20, 2015 at 6:37 PM, Fabio Estevam > wrote: > > > Ok, I will test your proposal below on Monday when I get access to > > my > > mx6sxsabresd, thanks. > > Your proposal worked fine, thanks. Will send it as a formal patch. This is good news! Jörg ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Revert "net: phy: delay only if reset handler is registered"
On Mi, 2015-11-18 at 09:34 -0200, Fabio Estevam wrote:
> Hi Jörg,
>
> On Wed, Nov 18, 2015 at 6:44 AM, Jörg Krause
> wrote:
>
> > I think this is not the right thing to do here. It is true that the
> > AR8035 ethernet chip of the RioTboard needs the clock to be stable
> > for
> > at least 1ms before RESET can be deasserted. This why it fails, if
> > there is no MII reset function defined.
>
> In my case I am testing on a mx6sxsabresd which has two AR8031 chips.
>
> >
> > I think the right place to handle reset delays is the
> > board_eth_init().
> > The value of 15ms currently used looks to me like an arbitrary
> > value. I
> > am not sure where this value is comming from. Some chips need more,
> > some less time to delay after a reset.
> >
> > This is snippet of how I do it for a custom i.MX28-EVK-based board:
> >
> > int board_eth_init(bd_t *bis) {
> > cpu_eth_init(bis);
> > /* Power-on FEC */
> > gpio_direction_output(MX28_PAD_LCD_D21__GPIO_1_21, 0);
> > /* Reset FEC PHY */
> > gpio_direction_output(MX28_PAD_ENET0_RX_CLK__GPIO_4_13, 0);
> > /* Deassert nRST after 25 ms from power-up on (= t_purstd) */
> > udelay(25000);
> > gpio_set_value(MX28_PAD_ENET0_RX_CLK__GPIO_4_13, 1);
> > fecmxc_initialize_multi(bis, 0, 0, MXS_ENET0_BASE);
> > }
>
> We currently reset the Ethernet PHY inside setup_fec() inside
> board_eth_init().
>
> Please check board/freescale/mx6sxsabresd/mx6sxsabresd.c.
Ok, I checked. This is what happens in setup_fec():
1) Enable AR8031 power
2) Assert AR8031 RESET
3) Delay 0.5ms
4) Deassert AR8031 RESET
5) Enable anatop clock
Shouldn't the clock be enabled and become stable before deasserting the
RESET line?
> I have also tried increasing the reset time and still do not have
> Ethernel functional.
The AR8031 datasheets recommands a delay of 10ms.
Best regards
Jörg Krause
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Revert "net: phy: delay only if reset handler is registered"
Hi Fabio,
On Di, 2015-11-17 at 14:25 -0200, Fabio Estevam wrote:
> This reverts commit 59370f3fcd135089c402c93720a87c688abe600c.
>
> This commit breaks ethernet on at least mx6 Riotboard and
> mx6sxsabresd boards.
>
> Reported-by: Catalin Crenguta
> Signed-off-by: Fabio Estevam
> ---
> drivers/net/phy/phy.c | 8 +++-
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index d7364ff..9e68f1d 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -771,13 +771,11 @@ struct phy_device *phy_find_by_mask(struct
> mii_dev *bus, unsigned phy_mask,
> phy_interface_t interface)
> {
> /* Reset the bus */
> - if (bus->reset) {
> + if (bus->reset)
> bus->reset(bus);
>
> - /* Wait 15ms to make sure the PHY has come out of
> hard reset */
> - udelay(15000);
> - }
> -
> + /* Wait 15ms to make sure the PHY has come out of hard reset
> */
> + udelay(15000);
> return get_phy_device_by_mask(bus, phy_mask, interface);
> }
>
I think this is not the right thing to do here. It is true that the
AR8035 ethernet chip of the RioTboard needs the clock to be stable for
at least 1ms before RESET can be deasserted. This why it fails, if
there is no MII reset function defined.
I think the right place to handle reset delays is the board_eth_init().
The value of 15ms currently used looks to me like an arbitrary value. I
am not sure where this value is comming from. Some chips need more,
some less time to delay after a reset.
This is snippet of how I do it for a custom i.MX28-EVK-based board:
int board_eth_init(bd_t *bis) {
cpu_eth_init(bis);
/* Power-on FEC */
gpio_direction_output(MX28_PAD_LCD_D21__GPIO_1_21, 0);
/* Reset FEC PHY */
gpio_direction_output(MX28_PAD_ENET0_RX_CLK__GPIO_4_13, 0);
/* Deassert nRST after 25 ms from power-up on (= t_purstd) */
udelay(25000);
gpio_set_value(MX28_PAD_ENET0_RX_CLK__GPIO_4_13, 1);
fecmxc_initialize_multi(bis, 0, 0, MXS_ENET0_BASE);
}
Best regards
Jörg Krause
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 1/1] tools: mxsboot: calculate ECC block level dynamically
For pages of 2048 bytes the current setting of the ECC Error Correction Level
is only true for an oob size of 64 bytes and wrong for all others.
Instead of hard-coding every possible combination of page size and oob size use
the dynamic calculation of the ECC strength introduced in commit
6121560d7714d6d8e41ce1687a1388a1a8fea4cb.
Cc: Marek Vasut
Signed-off-by: Jörg Krause
---
tools/mxsboot.c | 21 -
1 file changed, 4 insertions(+), 17 deletions(-)
diff --git a/tools/mxsboot.c b/tools/mxsboot.c
index 185b327..15eec91 100644
--- a/tools/mxsboot.c
+++ b/tools/mxsboot.c
@@ -271,23 +271,10 @@ static struct mx28_nand_fcb *mx28_nand_get_fcb(uint32_t
size)
fcb->ecc_block_0_size = 512;
fcb->ecc_block_n_size = 512;
fcb->metadata_bytes = 10;
-
- if (nand_writesize == 2048) {
- fcb->ecc_block_n_ecc_type = 4;
- fcb->ecc_block_0_ecc_type = 4;
- } else if (nand_writesize == 4096) {
- if (nand_oobsize == 128) {
- fcb->ecc_block_n_ecc_type = 4;
- fcb->ecc_block_0_ecc_type = 4;
- } else if (nand_oobsize == 218) {
- fcb->ecc_block_n_ecc_type = 8;
- fcb->ecc_block_0_ecc_type = 8;
- } else if (nand_oobsize == 224) {
- fcb->ecc_block_n_ecc_type = 8;
- fcb->ecc_block_0_ecc_type = 8;
- }
- }
-
+ fcb->ecc_block_n_ecc_type = mx28_nand_get_ecc_strength(
+ nand_writesize, nand_oobsize) >> 1;
+ fcb->ecc_block_0_ecc_type = mx28_nand_get_ecc_strength(
+ nand_writesize, nand_oobsize) >> 1;
if (fcb->ecc_block_n_ecc_type == 0) {
printf("MX28 NAND: Unsupported NAND geometry\n");
goto err;
--
2.5.0
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] GCC 5.2 issue on imx28
On Mi, 2015-08-05 at 20:23 +0100, Måns Rullgård wrote:
> Jörg Krause writes:
>
> > Dear Måns Rullgård, Otavio Salvador,
> >
> > On Di, 2015-07-28 at 14:39 +0100, Måns Rullgård wrote:
> > > Otavio Salvador writes:
> >
> > [snip]
> >
> > > There are two errors reports:
> > >
> > > 1. An undefined reference to the symbol "lowlevel_init"
> > > 2. A complaint about the ".rel.plt" section not being handled by
> > > the
> > >linker script.
> > >
> > > The second error is probably caused by the first. A quick grep
> > > turns
> > > up
> > > this snippet in arch/arm/cpu/arm926ejs/mxs/mxs.c:
> > >
> > > /* Lowlevel init isn't used on i.MX28, so just have a dummy here
> > > */
> > > inline void lowlevel_init(void) {}
> > >
> > > The semantics for non-static functions declared inline have
> > > changed
> > > in
> > > gcc5, causing the above (empty) function not to be emitted as an
> > > external symbol.
> > >
> > > Since that function is only referenced from start.S, it should
> > > not be
> > > declared inline at all. This patch should thus fix your problem:
> > >
> > > diff --git a/arch/arm/cpu/arm926ejs/mxs/mxs.c
> > > b/arch/arm/cpu/arm926ejs/mxs/mxs.c
> > > index ef130ae..b1d8721 100644
> > > --- a/arch/arm/cpu/arm926ejs/mxs/mxs.c
> > > +++ b/arch/arm/cpu/arm926ejs/mxs/mxs.c
> > > @@ -24,7 +24,7 @@
> > > DECLARE_GLOBAL_DATA_PTR;
> > >
> > > /* Lowlevel init isn't used on i.MX28, so just have a dummy here
> > > */
> > > -inline void lowlevel_init(void) {}
> > > +void lowlevel_init(void) {}
> > >
> > > void reset_cpu(ulong ignored) __attribute__((noreturn));
> > >
> >
> > I stumbled over the same problem. Unfortunatly, I did not find this
> > patch before (only the error report from Otavia) and submitted a
> > similar patch [1] which keeps the inline keyword.
> >
> > Best regards
> > Jörg Krause
> >
> > [1] "arm: mxs: make inline function compatible for GCC 5"
> > https://patchwork.ozlabs.org/patch/504043/
>
> Since the function is only referenced from outside the C file, any
> use
> of inline makes little sense to me. While your patch achieves the
> result of creating a linkable instance of the function, it is more
> complicated than it needs to be.
>
In my opinion it quite make sense to use inline for functions
referenced only from other files. The keyword is just an hint to the
compiler and why should it not make sense for other C files? However,
in this case it makes really no difference if lowlevel_init is marked
as inline, gcc will generate an "BL lowlevel_init" instruction in both
cases.
Jörg
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] GCC 5.2 issue on imx28
Dear Måns Rullgård, Otavio Salvador,
On Di, 2015-07-28 at 14:39 +0100, Måns Rullgård wrote:
> Otavio Salvador writes:
[snip]
> There are two errors reports:
>
> 1. An undefined reference to the symbol "lowlevel_init"
> 2. A complaint about the ".rel.plt" section not being handled by the
>linker script.
>
> The second error is probably caused by the first. A quick grep turns
> up
> this snippet in arch/arm/cpu/arm926ejs/mxs/mxs.c:
>
> /* Lowlevel init isn't used on i.MX28, so just have a dummy here */
> inline void lowlevel_init(void) {}
>
> The semantics for non-static functions declared inline have changed
> in
> gcc5, causing the above (empty) function not to be emitted as an
> external symbol.
>
> Since that function is only referenced from start.S, it should not be
> declared inline at all. This patch should thus fix your problem:
>
> diff --git a/arch/arm/cpu/arm926ejs/mxs/mxs.c
> b/arch/arm/cpu/arm926ejs/mxs/mxs.c
> index ef130ae..b1d8721 100644
> --- a/arch/arm/cpu/arm926ejs/mxs/mxs.c
> +++ b/arch/arm/cpu/arm926ejs/mxs/mxs.c
> @@ -24,7 +24,7 @@
> DECLARE_GLOBAL_DATA_PTR;
>
> /* Lowlevel init isn't used on i.MX28, so just have a dummy here */
> -inline void lowlevel_init(void) {}
> +void lowlevel_init(void) {}
>
> void reset_cpu(ulong ignored) __attribute__((noreturn));
>
I stumbled over the same problem. Unfortunatly, I did not find this
patch before (only the error report from Otavia) and submitted a
similar patch [1] which keeps the inline keyword.
Best regards
Jörg Krause
[1] "arm: mxs: make inline function compatible for GCC 5"
https://patchwork.ozlabs.org/patch/504043/
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 1/1] arm: mxs: make inline function compatible for GCC 5
GCC 5 uses C99 inline semantics. To generate external visibility in C99 for
the lowlevel_init() function must be declared with extern inline [1].
The GNU89 inline semantic is the same as C99 extern inline.
Use the __GNUC_STDC_INLINE__ macro to detect if C99 inline semantic will be
used by GCC.
Fixes the build error "undefined reference to `lowlevel_init'" for mx28 based
targets.
[1] "Different semantics for inline functions"
https://gcc.gnu.org/gcc-5/porting_to.html
Signed-off-by: Jörg Krause
---
arch/arm/cpu/arm926ejs/mxs/mxs.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/arm/cpu/arm926ejs/mxs/mxs.c b/arch/arm/cpu/arm926ejs/mxs/mxs.c
index ef130ae..439396b 100644
--- a/arch/arm/cpu/arm926ejs/mxs/mxs.c
+++ b/arch/arm/cpu/arm926ejs/mxs/mxs.c
@@ -24,6 +24,9 @@
DECLARE_GLOBAL_DATA_PTR;
/* Lowlevel init isn't used on i.MX28, so just have a dummy here */
+#if defined(__GNUC_STDC_INLINE__) /* e.g. GCC 5.x default */
+extern
+#endif
inline void lowlevel_init(void) {}
void reset_cpu(ulong ignored) __attribute__((noreturn));
--
2.5.0
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 1/1] net: phy: delay only if reset handler is registered
With commit e3a77218a256edbe201112a39beeed8adcabae3f the MII bus is only
reset if a reset handler is registered. If there is no reset handler there
is no need to wait for a device to come out of the reset.
Signed-off-by: Jörg Krause
---
drivers/net/phy/phy.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 865abab..65c731a 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -763,11 +763,13 @@ struct phy_device *phy_find_by_mask(struct mii_dev *bus,
unsigned phy_mask,
phy_interface_t interface)
{
/* Reset the bus */
- if (bus->reset)
+ if (bus->reset) {
bus->reset(bus);
- /* Wait 15ms to make sure the PHY has come out of hard reset */
- udelay(15000);
+ /* Wait 15ms to make sure the PHY has come out of hard reset */
+ udelay(15000);
+ }
+
return get_phy_device_by_mask(bus, phy_mask, interface);
}
--
2.4.5
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 1/1] net: phy: fix data type of phy_id
phy_id is declared as u32 in create_phy_by_mask and in struct phy_device.
Signed-off-by: Jörg Krause
---
drivers/net/phy/phy.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index c8d08e8..865abab 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -554,7 +554,7 @@ static struct phy_driver *get_phy_driver(struct phy_device
*phydev,
}
static struct phy_device *phy_device_create(struct mii_dev *bus, int addr,
- int phy_id,
+ u32 phy_id,
phy_interface_t interface)
{
struct phy_device *dev;
--
2.4.5
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v2 1/1] Fix musl build
This patch fixes cross-compiling U-Boot tools with the musl C library: * including is needed for ulong * defining _GNU_SOURCE is needed for loff_t Tested for target at91sam9261ek_dataflash_cs3. Signed-off-by: Jörg Krause Cc: Tom Rini --- Changes v1 -> v2: - Fix header include in image.h (reported by Tom Rini) --- include/image.h| 1 + tools/env/fw_env.c | 2 ++ tools/imagetool.h | 1 + tools/proftool.c | 1 + 4 files changed, 5 insertions(+) diff --git a/include/image.h b/include/image.h index 3844be6..60b924a 100644 --- a/include/image.h +++ b/include/image.h @@ -23,6 +23,7 @@ struct lmb; #ifdef USE_HOSTCC +#include /* new uImage format support enabled on host */ #define CONFIG_FIT 1 diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c index 1173eea..daa02a7 100644 --- a/tools/env/fw_env.c +++ b/tools/env/fw_env.c @@ -8,6 +8,8 @@ * SPDX-License-Identifier:GPL-2.0+ */ +#define _GNU_SOURCE + #include #include #include diff --git a/tools/imagetool.h b/tools/imagetool.h index 3e15b4e..b7874f4 100644 --- a/tools/imagetool.h +++ b/tools/imagetool.h @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include diff --git a/tools/proftool.c b/tools/proftool.c index 3482951..9ce7a77 100644 --- a/tools/proftool.c +++ b/tools/proftool.c @@ -16,6 +16,7 @@ #include #include #include +#include #include #include -- 2.3.5 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 1/1] Fix musl build
This patch fixes cross-compiling U-Boot tools with the musl C library: * including is needed for ulong * defining _GNU_SOURCE is needed for loff_t Signed-off-by: Jörg Krause --- include/image.h| 1 + tools/env/fw_env.c | 2 ++ tools/imagetool.h | 1 + tools/proftool.c | 1 + 4 files changed, 5 insertions(+) diff --git a/include/image.h b/include/image.h index 3844be6..ac2fd6e 100644 --- a/include/image.h +++ b/include/image.h @@ -18,6 +18,7 @@ #include "compiler.h" #include +#include /* Define this to avoid #ifdefs later on */ struct lmb; diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c index 1173eea..daa02a7 100644 --- a/tools/env/fw_env.c +++ b/tools/env/fw_env.c @@ -8,6 +8,8 @@ * SPDX-License-Identifier:GPL-2.0+ */ +#define _GNU_SOURCE + #include #include #include diff --git a/tools/imagetool.h b/tools/imagetool.h index 3e15b4e..b7874f4 100644 --- a/tools/imagetool.h +++ b/tools/imagetool.h @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include diff --git a/tools/proftool.c b/tools/proftool.c index 3482951..9ce7a77 100644 --- a/tools/proftool.c +++ b/tools/proftool.c @@ -16,6 +16,7 @@ #include #include #include +#include #include #include -- 2.3.5 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v3 0/3] mtd: nand: mxs: Calculate ECC strength dynamically
This patchset is based on the patch from Peng Fan: "[U-Boot,1/2] mtd:mxs:nand calculate ecc strength dynamically" https://patchwork.ozlabs.org/patch/422756/ Instead of hard-coding every possible oob size / ECC strength combination calculate the ECC strength dynamically to be aligned with the Linux Kernel MTD NAND driver. Also adds the calculation to tools/mxsboot to be aligned with the U-Boot MTD NAND driver. Jörg Krause (2): mtd: nand: mxs: Replace magic number for bits per ECC level with macro tools: mxsboot: Calculate ECC strength dynamically Peng Fan (1): mtd:mxs:nand calculate ecc strength dynamically drivers/mtd/nand/mxs_nand.c | 35 +++ tools/mxsboot.c | 39 --- 2 files changed, 39 insertions(+), 35 deletions(-) -- 2.3.5 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v3 2/3] mtd: nand: mxs: Replace magic number for bits per ECC level with macro
Signed-off-by: Jörg Krause
---
Changes for v3:
- Replace space with tab for macro definition
Changes for v2:
- New patch
---
drivers/mtd/nand/mxs_nand.c | 7 ---
tools/mxsboot.c | 3 ++-
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/mtd/nand/mxs_nand.c b/drivers/mtd/nand/mxs_nand.c
index a463de7..aa06446 100644
--- a/drivers/mtd/nand/mxs_nand.c
+++ b/drivers/mtd/nand/mxs_nand.c
@@ -36,7 +36,7 @@
#defineMXS_NAND_CHUNK_DATA_CHUNK_SIZE_SHIFT0
#endif
#defineMXS_NAND_METADATA_SIZE 10
-
+#defineMXS_NAND_BITS_PER_ECC_LEVEL 13
#defineMXS_NAND_COMMAND_BUFFER_SIZE32
#defineMXS_NAND_BCH_TIMEOUT1
@@ -135,7 +135,7 @@ static uint32_t mxs_nand_ecc_chunk_cnt(uint32_t
page_data_size)
static uint32_t mxs_nand_ecc_size_in_bits(uint32_t ecc_strength)
{
- return ecc_strength * 13;
+ return ecc_strength * MXS_NAND_BITS_PER_ECC_LEVEL;
}
static uint32_t mxs_nand_aux_status_offset(void)
@@ -157,7 +157,8 @@ static inline uint32_t mxs_nand_get_ecc_strength(uint32_t
page_data_size,
* (page oob size - meta data size) * (bits per byte)
*/
ecc_strength = ((page_oob_size - MXS_NAND_METADATA_SIZE) * 8)
- / (13 * mxs_nand_ecc_chunk_cnt(page_data_size));
+ / (MXS_NAND_BITS_PER_ECC_LEVEL *
+ mxs_nand_ecc_chunk_cnt(page_data_size));
return round_down(ecc_strength, 2);
}
diff --git a/tools/mxsboot.c b/tools/mxsboot.c
index 6d48cfb..aaa872b 100644
--- a/tools/mxsboot.c
+++ b/tools/mxsboot.c
@@ -48,6 +48,7 @@ static uint32_t sd_sector = 2048;
#defineMXS_NAND_DMA_DESCRIPTOR_COUNT 4
#defineMXS_NAND_CHUNK_DATA_CHUNK_SIZE 512
#defineMXS_NAND_METADATA_SIZE 10
+#defineMXS_NAND_BITS_PER_ECC_LEVEL 13
#defineMXS_NAND_COMMAND_BUFFER_SIZE32
struct mx28_nand_fcb {
@@ -127,7 +128,7 @@ struct mx28_sd_config_block {
static inline uint32_t mx28_nand_ecc_size_in_bits(uint32_t ecc_strength)
{
- return ecc_strength * 13;
+ return ecc_strength * MXS_NAND_BITS_PER_ECC_LEVEL;
}
static inline uint32_t mx28_nand_get_ecc_strength(uint32_t page_data_size,
--
2.3.5
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v3 3/3] tools: mxsboot: Calculate ECC strength dynamically
Calculating the ECC strength dynamically to be aligned with the mxs NAND
driver and the Linux Kernel.
Signed-off-by: Jörg Krause
---
Changes for v3:
- Coding Style cleanup
Changes for v2:
- New patch
---
tools/mxsboot.c | 35 ++-
1 file changed, 22 insertions(+), 13 deletions(-)
diff --git a/tools/mxsboot.c b/tools/mxsboot.c
index aaa872b..53de452 100644
--- a/tools/mxsboot.c
+++ b/tools/mxsboot.c
@@ -14,6 +14,10 @@
#include "compiler.h"
+/* Taken from */
+#define __round_mask(x, y) ((__typeof__(x))((y)-1))
+#define round_down(x, y) ((x) & ~__round_mask(x, y))
+
/*
* Default BCB layout.
*
@@ -126,6 +130,11 @@ struct mx28_sd_config_block {
struct mx28_sd_drive_info drv_info[1];
};
+static inline uint32_t mx28_nand_ecc_chunk_cnt(uint32_t page_data_size)
+{
+ return page_data_size / MXS_NAND_CHUNK_DATA_CHUNK_SIZE;
+}
+
static inline uint32_t mx28_nand_ecc_size_in_bits(uint32_t ecc_strength)
{
return ecc_strength * MXS_NAND_BITS_PER_ECC_LEVEL;
@@ -134,21 +143,21 @@ static inline uint32_t
mx28_nand_ecc_size_in_bits(uint32_t ecc_strength)
static inline uint32_t mx28_nand_get_ecc_strength(uint32_t page_data_size,
uint32_t page_oob_size)
{
- if (page_data_size == 2048)
- return 8;
-
- if (page_data_size == 4096) {
- if (page_oob_size == 128)
- return 8;
+ int ecc_strength;
- if (page_oob_size == 218)
- return 16;
-
- if (page_oob_size == 224)
- return 16;
- }
+ /*
+* Determine the ECC layout with the formula:
+* ECC bits per chunk = (total page spare data bits) /
+* (bits per ECC level) / (chunks per page)
+* where:
+* total page spare data bits =
+* (page oob size - meta data size) * (bits per byte)
+*/
+ ecc_strength = ((page_oob_size - MXS_NAND_METADATA_SIZE) * 8)
+ / (MXS_NAND_BITS_PER_ECC_LEVEL *
+ mx28_nand_ecc_chunk_cnt(page_data_size));
- return 0;
+ return round_down(ecc_strength, 2);
}
static inline uint32_t mx28_nand_get_mark_offset(uint32_t page_data_size,
--
2.3.5
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v3 1/3] mtd:mxs:nand calculate ecc strength dynamically
From: Peng Fan
Calculate ecc strength according oobsize, but not hardcoded
which is not aligned with kernel driver
Signed-off-by: Peng Fan
Signed-off-by: Ye.Li
Reviewed-by: Marek Vasut
Signed-off-by: Jörg Krause
---
Changes for v3:
- squashed "[U-Boot,v2,3/4] mtd: nand: mxs: Add comment for calculating
ECC strength"
https://patchwork.ozlabs.org/patch/460926/
- Coding Style cleanup
Changes for v2:
- None (original patch from Peng Fan)
---
drivers/mtd/nand/mxs_nand.c | 30 --
1 file changed, 12 insertions(+), 18 deletions(-)
diff --git a/drivers/mtd/nand/mxs_nand.c b/drivers/mtd/nand/mxs_nand.c
index 7a064ab..a463de7 100644
--- a/drivers/mtd/nand/mxs_nand.c
+++ b/drivers/mtd/nand/mxs_nand.c
@@ -146,26 +146,20 @@ static uint32_t mxs_nand_aux_status_offset(void)
static inline uint32_t mxs_nand_get_ecc_strength(uint32_t page_data_size,
uint32_t page_oob_size)
{
- if (page_data_size == 2048) {
- if (page_oob_size == 64)
- return 8;
+ int ecc_strength;
- if (page_oob_size == 112)
- return 14;
- }
-
- if (page_data_size == 4096) {
- if (page_oob_size == 128)
- return 8;
-
- if (page_oob_size == 218)
- return 16;
-
- if (page_oob_size == 224)
- return 16;
- }
+ /*
+* Determine the ECC layout with the formula:
+* ECC bits per chunk = (total page spare data bits) /
+* (bits per ECC level) / (chunks per page)
+* where:
+* total page spare data bits =
+* (page oob size - meta data size) * (bits per byte)
+*/
+ ecc_strength = ((page_oob_size - MXS_NAND_METADATA_SIZE) * 8)
+ / (13 * mxs_nand_ecc_chunk_cnt(page_data_size));
- return 0;
+ return round_down(ecc_strength, 2);
}
static inline uint32_t mxs_nand_get_mark_offset(uint32_t page_data_size,
--
2.3.5
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 3/4] mtd: nand: mxs: Add comment for calculating ECC strength
Hello Heiko,
On Di, 2015-04-14 at 10:02 +0200, Heiko Schocher wrote:
> Hello Jörg,
>
> Am 14.04.2015 08:29, schrieb Jörg Krause:
> > Hello Heiko,
> >
> > On Di, 2015-04-14 at 08:12 +0200, Heiko Schocher wrote:
> > > Hello Jörg,
> > >
> > > Am 13.04.2015 22:17, schrieb Jörg Krause:
> > > > Signed-off-by: Jörg Krause
> > > > ---
> > > >drivers/mtd/nand/mxs_nand.c | 7 +++
> > > >1 file changed, 7 insertions(+)
> > >
> > > nitpick only ...
> >
> > I'm unsure what this comment means translated to German. Something
> > like small changes only?
>
> Yes, something like "pingelig"
Okay, now I understand the translation but not the meaning. Do you
mean that this patch should be squashed into patch 1 for example?
>
> > > > diff --git a/drivers/mtd/nand/mxs_nand.c
> > > > b/drivers/mtd/nand/mxs_nand.c
> > > > index 912fed8..76e47ab 100644
> > > > --- a/drivers/mtd/nand/mxs_nand.c
> > > > +++ b/drivers/mtd/nand/mxs_nand.c
> > > > @@ -148,6 +148,13 @@ static inline uint32_t
> > > > mxs_nand_get_ecc_strength(uint32_t page_data_size,
> > > >{
> > > > int ecc_strength;
> > > >
> > > > + /* Determine the ECC layout with the formula:
> > >
> > > wrong comment style ... please fix also in patch 4/4... thanks.
> >
> > Checkpatch did not complain and I did not know there is a coding
> > style
> > for comments. Should it be:
> > /*
> > * Determine the ECC layout...
>
> Yes.
>
> See: linux:/Documentation/CodingStyle search for
> "The preferred style for long (multi-line) comments is"
Thanks! So shall I send a v3 for this patch or will this be fixed by a
maintainer?
Best regards
Jörg Krause
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 3/4] mtd: nand: mxs: Add comment for calculating ECC strength
Hello Heiko,
On Di, 2015-04-14 at 08:12 +0200, Heiko Schocher wrote:
> Hello Jörg,
>
> Am 13.04.2015 22:17, schrieb Jörg Krause:
> > Signed-off-by: Jörg Krause
> > ---
> > drivers/mtd/nand/mxs_nand.c | 7 +++
> > 1 file changed, 7 insertions(+)
>
> nitpick only ...
I'm unsure what this comment means translated to German. Something
like small changes only?
>
> > diff --git a/drivers/mtd/nand/mxs_nand.c
> > b/drivers/mtd/nand/mxs_nand.c
> > index 912fed8..76e47ab 100644
> > --- a/drivers/mtd/nand/mxs_nand.c
> > +++ b/drivers/mtd/nand/mxs_nand.c
> > @@ -148,6 +148,13 @@ static inline uint32_t
> > mxs_nand_get_ecc_strength(uint32_t page_data_size,
> > {
> > int ecc_strength;
> >
> > + /* Determine the ECC layout with the formula:
>
> wrong comment style ... please fix also in patch 4/4... thanks.
Checkpatch did not complain and I did not know there is a coding style
for comments. Should it be:
/*
* Determine the ECC layout...
Bye
Jörg
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v2 3/4] mtd: nand: mxs: Add comment for calculating ECC strength
Signed-off-by: Jörg Krause
---
drivers/mtd/nand/mxs_nand.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/mtd/nand/mxs_nand.c b/drivers/mtd/nand/mxs_nand.c
index 912fed8..76e47ab 100644
--- a/drivers/mtd/nand/mxs_nand.c
+++ b/drivers/mtd/nand/mxs_nand.c
@@ -148,6 +148,13 @@ static inline uint32_t mxs_nand_get_ecc_strength(uint32_t
page_data_size,
{
int ecc_strength;
+ /* Determine the ECC layout with the formula:
+* ECC bits per chunk = (total page spare data bits) /
+* (bits per ECC level) / (chunks per page)
+* where:
+* total page spare data bits =
+* (page oob size - meta data size) * (bits per byte)
+*/
ecc_strength = ((page_oob_size - MXS_NAND_METADATA_SIZE) * 8)
/ (MXS_NAND_BITS_PER_ECC_LEVEL *
mxs_nand_ecc_chunk_cnt(page_data_size));
--
2.3.5
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v2 2/4] mtd: nand: mxs: Replace magic number for bits per ECC level with macro
Signed-off-by: Jörg Krause
---
drivers/mtd/nand/mxs_nand.c | 7 ---
tools/mxsboot.c | 3 ++-
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/mtd/nand/mxs_nand.c b/drivers/mtd/nand/mxs_nand.c
index a45fcf9..912fed8 100644
--- a/drivers/mtd/nand/mxs_nand.c
+++ b/drivers/mtd/nand/mxs_nand.c
@@ -36,7 +36,7 @@
#defineMXS_NAND_CHUNK_DATA_CHUNK_SIZE_SHIFT0
#endif
#defineMXS_NAND_METADATA_SIZE 10
-
+#define MXS_NAND_BITS_PER_ECC_LEVEL13
#defineMXS_NAND_COMMAND_BUFFER_SIZE32
#defineMXS_NAND_BCH_TIMEOUT1
@@ -135,7 +135,7 @@ static uint32_t mxs_nand_ecc_chunk_cnt(uint32_t
page_data_size)
static uint32_t mxs_nand_ecc_size_in_bits(uint32_t ecc_strength)
{
- return ecc_strength * 13;
+ return ecc_strength * MXS_NAND_BITS_PER_ECC_LEVEL;
}
static uint32_t mxs_nand_aux_status_offset(void)
@@ -149,7 +149,8 @@ static inline uint32_t mxs_nand_get_ecc_strength(uint32_t
page_data_size,
int ecc_strength;
ecc_strength = ((page_oob_size - MXS_NAND_METADATA_SIZE) * 8)
- / (13 * mxs_nand_ecc_chunk_cnt(page_data_size));
+ / (MXS_NAND_BITS_PER_ECC_LEVEL *
+ mxs_nand_ecc_chunk_cnt(page_data_size));
return round_down(ecc_strength, 2);
}
diff --git a/tools/mxsboot.c b/tools/mxsboot.c
index 6d48cfb..aaa872b 100644
--- a/tools/mxsboot.c
+++ b/tools/mxsboot.c
@@ -48,6 +48,7 @@ static uint32_t sd_sector = 2048;
#defineMXS_NAND_DMA_DESCRIPTOR_COUNT 4
#defineMXS_NAND_CHUNK_DATA_CHUNK_SIZE 512
#defineMXS_NAND_METADATA_SIZE 10
+#define MXS_NAND_BITS_PER_ECC_LEVEL13
#defineMXS_NAND_COMMAND_BUFFER_SIZE32
struct mx28_nand_fcb {
@@ -127,7 +128,7 @@ struct mx28_sd_config_block {
static inline uint32_t mx28_nand_ecc_size_in_bits(uint32_t ecc_strength)
{
- return ecc_strength * 13;
+ return ecc_strength * MXS_NAND_BITS_PER_ECC_LEVEL;
}
static inline uint32_t mx28_nand_get_ecc_strength(uint32_t page_data_size,
--
2.3.5
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v2 4/4] tools: mxsboot: Calculate ECC strength dynamically
Calculating the ECC strength dynamically to be aligned with the mxs NAND
driver and the Linux Kernel.
The macro definition for round_down is taken from to
avoid changing the tools/Makefile where the Linux Kernel header files
are not included for building the tools target.
Signed-off-by: Jörg Krause
---
tools/mxsboot.c | 36 ++--
1 file changed, 22 insertions(+), 14 deletions(-)
diff --git a/tools/mxsboot.c b/tools/mxsboot.c
index aaa872b..8fca8cf 100644
--- a/tools/mxsboot.c
+++ b/tools/mxsboot.c
@@ -14,6 +14,10 @@
#include "compiler.h"
+/* Taken from */
+#define __round_mask(x, y) ((__typeof__(x))((y)-1))
+#define round_down(x, y) ((x) & ~__round_mask(x, y))
+
/*
* Default BCB layout.
*
@@ -126,6 +130,11 @@ struct mx28_sd_config_block {
struct mx28_sd_drive_info drv_info[1];
};
+static inline uint32_t mx28_nand_ecc_chunk_cnt(uint32_t page_data_size)
+{
+ return page_data_size / MXS_NAND_CHUNK_DATA_CHUNK_SIZE;
+}
+
static inline uint32_t mx28_nand_ecc_size_in_bits(uint32_t ecc_strength)
{
return ecc_strength * MXS_NAND_BITS_PER_ECC_LEVEL;
@@ -134,21 +143,20 @@ static inline uint32_t
mx28_nand_ecc_size_in_bits(uint32_t ecc_strength)
static inline uint32_t mx28_nand_get_ecc_strength(uint32_t page_data_size,
uint32_t page_oob_size)
{
- if (page_data_size == 2048)
- return 8;
-
- if (page_data_size == 4096) {
- if (page_oob_size == 128)
- return 8;
-
- if (page_oob_size == 218)
- return 16;
-
- if (page_oob_size == 224)
- return 16;
- }
+ int ecc_strength;
+
+ /* Determine the ECC layout with the formula:
+* ECC bits per chunk = (total page spare data bits) /
+* (bits per ECC level) / (chunks per page)
+* where:
+* total page spare data bits =
+* (page oob size - meta data size) * (bits per byte)
+*/
+ ecc_strength = ((page_oob_size - MXS_NAND_METADATA_SIZE) * 8)
+ / (MXS_NAND_BITS_PER_ECC_LEVEL *
+ mx28_nand_ecc_chunk_cnt(page_data_size));
- return 0;
+ return round_down(ecc_strength, 2);
}
static inline uint32_t mx28_nand_get_mark_offset(uint32_t page_data_size,
--
2.3.5
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v2 1/4] mtd:mxs:nand calculate ecc strength dynamically
From: Peng Fan
Calculate ecc strength according oobsize, but not hardcoded
which is not aligned with kernel driver
Signed-off-by: Peng Fan
Signed-off-by: Ye.Li
Reviewed-by: Marek Vasut
Signed-off-by: Jörg Krause
---
drivers/mtd/nand/mxs_nand.c | 22 --
1 file changed, 4 insertions(+), 18 deletions(-)
diff --git a/drivers/mtd/nand/mxs_nand.c b/drivers/mtd/nand/mxs_nand.c
index 7a064ab..a45fcf9 100644
--- a/drivers/mtd/nand/mxs_nand.c
+++ b/drivers/mtd/nand/mxs_nand.c
@@ -146,26 +146,12 @@ static uint32_t mxs_nand_aux_status_offset(void)
static inline uint32_t mxs_nand_get_ecc_strength(uint32_t page_data_size,
uint32_t page_oob_size)
{
- if (page_data_size == 2048) {
- if (page_oob_size == 64)
- return 8;
+ int ecc_strength;
- if (page_oob_size == 112)
- return 14;
- }
-
- if (page_data_size == 4096) {
- if (page_oob_size == 128)
- return 8;
-
- if (page_oob_size == 218)
- return 16;
+ ecc_strength = ((page_oob_size - MXS_NAND_METADATA_SIZE) * 8)
+ / (13 * mxs_nand_ecc_chunk_cnt(page_data_size));
- if (page_oob_size == 224)
- return 16;
- }
-
- return 0;
+ return round_down(ecc_strength, 2);
}
static inline uint32_t mxs_nand_get_mark_offset(uint32_t page_data_size,
--
2.3.5
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v2 0/4] mtd: nand: mxs: Calculate ECC strength dynamically
This series of patches are based on the patch of Peng Fan: https://patchwork.ozlabs.org/patch/422756/ Patch 1 is the originally patch from Peng Fan, Patch 2 and 3 add minor changes to 1 and patch 4 adds the ECC strength calculation to tools/mxsboot to be aligned with the changes made in patch 1 to 3. Instead of hard-coding every possible oob size / ECC strength combination calculate the ECC strength dynamically to be aligned with the Linux Kernel MTD NAND driver. Also adds the calculation to tools/mxsboot to be aligned with the U-Boot MTD NAND driver. Obviously, we have some code redundancy here in mxs_nand.c and mxsboot.c. Jörg Krause (3): mtd: nand: mxs: Replace magic number for bits per ECC level with macro mtd: nand: mxs: Add comment for calculating ECC strength tools: mxsboot: Calculate ECC strength dynamically Peng Fan (1): mtd:mxs:nand calculate ecc strength dynamically drivers/mtd/nand/mxs_nand.c | 36 +++- tools/mxsboot.c | 39 --- 2 files changed, 39 insertions(+), 36 deletions(-) -- 2.3.5 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] mxs_nand: Fix ECC strength for NAND flash with OOB size of 256
Hi Marek, Heiko, On Mo, 2015-04-13 at 11:01 +0200, Heiko Schocher wrote: > Hello Marek, Joerg, > > Am 13.04.2015 10:42, schrieb Marek Vasut: > > On Monday, April 13, 2015 at 10:39:46 AM, Jörg Krause wrote: > > > Hi Heiko, > > > > > > On So, 2015-04-12 at 10:17 +0200, Heiko Schocher wrote: > > > > On the i.mx6 based aristainetos2 board a Toshiba > > > > TH58NYG3S0HBAI4 > > > > is used, which has 4096 pagesize and 256b oob. The ECC strength > > > > was not correct detected by U-Boot > > > > > > > > Signed-off-by: Heiko Schocher > > > > --- > > > > > > > > drivers/mtd/nand/mxs_nand.c | 3 +++ > > > > 1 file changed, 3 insertions(+) > > > > > > > > diff --git a/drivers/mtd/nand/mxs_nand.c > > > > b/drivers/mtd/nand/mxs_nand.c > > > > index 2d2b938..00bf036 100644 > > > > --- a/drivers/mtd/nand/mxs_nand.c > > > > +++ b/drivers/mtd/nand/mxs_nand.c > > > > @@ -163,6 +163,9 @@ static inline uint32_t > > > > mxs_nand_get_ecc_strength(uint32_t page_data_size, > > > > > > > >if (page_oob_size == 224) > > > > > > > >return 16; > > > > > > > > + > > > > + if (page_oob_size == 256) > > > > + return 18; > > > > > > > >} > > > > > > > >return 0; > > > > > > How about calculation the ECC strength dynamically? Peng Fan from > > > Freescale send a patch doing this in December 2014 which was > > > already > > > reviewed by Marek: > > > https://patchwork.ozlabs.org/patch/422756/ > > > > > > It is also necessary to change the calculation in mxsboot... > > > > Would be nice if the patch got applied, but I think there were some > > comments and the patch was never fixed/reposted. If Heiko wants to > > do it, that'd be nice. > > Hmm.. I feel, I have not much time left for fixing such things... I can re-submit the patch from Peng Fan together with my fixes for mxsboot. > Joerg: You wrote on Jan. 27, 2015, 11:14 p.m.: > "I was able to fix mxsboot, but I had difficulties with round_down, > which > is a macro definition in linux/kernel.h. I've copied the macro > definition to mxsboot. I will submit the patch in a seperate mail." > > Did you post such a patch? Was this the onyl problem of the patch > from Peng Fan? No, I didn't. I waited for some comment and then I just forgot about it. The main problem with the patch from Peng Fan was that it was not consistent with mxsboot, which still has the hardcoded oobsizes. I copied the calculation to mxsboot.c, but failed to include linux/kernel.h, because mxsboot is compiled with the host compiler and u-boot with the cross-compiler. So I just copied the macro definition for round_down from kernel.h to mxsboot.c. > "I would like to see a comment or a macro for the magic number 13, > which > is the value for the Galois Field, just for clarification" > > I have no idea what 13 means ... This is cited from the i.MX28 Reference Manual: BCH-codes are a type of block-code, which implies that all error- correction is performed over a block of N-symbols. The BCH operation will be performed over GF (2^13 = 8192), which is the Galois Field consisting of 8191 one-bit symbols. > > > Nice domain name btw ;-) > > Indeed. Thanks :-) > > bye, > Heiko ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] mxs_nand: Fix ECC strength for NAND flash with OOB size of 256
Hi Heiko, On So, 2015-04-12 at 10:17 +0200, Heiko Schocher wrote: > On the i.mx6 based aristainetos2 board a Toshiba TH58NYG3S0HBAI4 > is used, which has 4096 pagesize and 256b oob. The ECC strength > was not correct detected by U-Boot > > Signed-off-by: Heiko Schocher > --- > > drivers/mtd/nand/mxs_nand.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/mtd/nand/mxs_nand.c > b/drivers/mtd/nand/mxs_nand.c > index 2d2b938..00bf036 100644 > --- a/drivers/mtd/nand/mxs_nand.c > +++ b/drivers/mtd/nand/mxs_nand.c > @@ -163,6 +163,9 @@ static inline uint32_t > mxs_nand_get_ecc_strength(uint32_t page_data_size, > > if (page_oob_size == 224) > return 16; > + > + if (page_oob_size == 256) > + return 18; > } > > return 0; How about calculation the ECC strength dynamically? Peng Fan from Freescale send a patch doing this in December 2014 which was already reviewed by Marek: https://patchwork.ozlabs.org/patch/422756/ It is also necessary to change the calculation in mxsboot... Best regards Jörg Krause ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/1] ARM: mxs: get boot mode from OTP
On Do, 2015-03-26 at 10:39 +0100, Jörg Krause wrote: > Reading the GPIOs for getting the boot mode does not show the correct result > for USB boot mode in case the recovery switch, eg. BM2 for switching from NAND > to USB boot mode, is hold down while plugging in USB and released before > U-Boot > is loaded by mxsldr. > > This state is stored in the HW_OCOTP_ROM0 register. HW_OCOTP_ROM0[27:24] maps > to > BM3-BM0, HW_OCOTP_ROM0[28] maps to voltage selector. > > For using mxs_wait_mask_clr() add imx-common/misc.o to the SPL build. > > Signed-off-by: Jörg Krause > Cc: Stefano Babic > --- This patch is superseded by: ARM: mxs: Get boot mode from OCRAM http://patchwork.ozlabs.org/patch/455231/ ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 1/1] ARM: mxs: Get boot mode from OCRAM
Reading the boot mode pins after power-up does not necessarily represent the
boot mode used by the ROM loader. For example the state of a pin may have
changed because a recovery switch which was pressed to enter USB mode is
already released after plugging in USB.
The ROM loader stores the value a fixed address in OCRAM. Use this value
instead of reading the boot map pins.
The GLOBAL_BOOT_MODE_ADDR for i.MX28 is taken from an U-Boot patch for the
MX28EVK:
http://repository.timesys.com/buildsources/u/u-boot/u-boot-2009.08/u-boot-2009.08-mx28-201012211513.patch
Leave the boot mode detection for the i.MX23 untouched. Someone has to test
whether the i.MX ROM loader does also store the boot mode in OCRAM and if the
address match.
This patch superseeds my incorrect patch:
ARM: mxs: get boot mode from OTP
http://patchwork.ozlabs.org/patch/454930/
Signed-off-by: Jörg Krause
Cc: Stefano Babic
---
arch/arm/cpu/arm926ejs/mxs/spl_boot.c | 29 ++---
1 file changed, 6 insertions(+), 23 deletions(-)
diff --git a/arch/arm/cpu/arm926ejs/mxs/spl_boot.c
b/arch/arm/cpu/arm926ejs/mxs/spl_boot.c
index d7956e5..eb8669b 100644
--- a/arch/arm/cpu/arm926ejs/mxs/spl_boot.c
+++ b/arch/arm/cpu/arm926ejs/mxs/spl_boot.c
@@ -49,13 +49,6 @@ static const iomux_cfg_t iomux_boot[] = {
MX23_PAD_LCD_D03__GPIO_1_3 | MUX_CONFIG_BOOTMODE_PAD,
MX23_PAD_LCD_D04__GPIO_1_4 | MUX_CONFIG_BOOTMODE_PAD,
MX23_PAD_LCD_D05__GPIO_1_5 | MUX_CONFIG_BOOTMODE_PAD,
-#elif defined(CONFIG_MX28)
- MX28_PAD_LCD_D00__GPIO_1_0 | MUX_CONFIG_BOOTMODE_PAD,
- MX28_PAD_LCD_D01__GPIO_1_1 | MUX_CONFIG_BOOTMODE_PAD,
- MX28_PAD_LCD_D02__GPIO_1_2 | MUX_CONFIG_BOOTMODE_PAD,
- MX28_PAD_LCD_D03__GPIO_1_3 | MUX_CONFIG_BOOTMODE_PAD,
- MX28_PAD_LCD_D04__GPIO_1_4 | MUX_CONFIG_BOOTMODE_PAD,
- MX28_PAD_LCD_D05__GPIO_1_5 | MUX_CONFIG_BOOTMODE_PAD,
#endif
};
@@ -65,10 +58,10 @@ static uint8_t mxs_get_bootmode_index(void)
int i;
uint8_t masked;
+#if defined(CONFIG_MX23)
/* Setup IOMUX of bootmode pads to GPIO */
mxs_iomux_setup_multiple_pads(iomux_boot, ARRAY_SIZE(iomux_boot));
-#if defined(CONFIG_MX23)
/* Setup bootmode pins as GPIO input */
gpio_direction_input(MX23_PAD_LCD_D00__GPIO_1_0);
gpio_direction_input(MX23_PAD_LCD_D01__GPIO_1_1);
@@ -83,21 +76,11 @@ static uint8_t mxs_get_bootmode_index(void)
bootmode |= (gpio_get_value(MX23_PAD_LCD_D03__GPIO_1_3) ? 1 : 0) << 3;
bootmode |= (gpio_get_value(MX23_PAD_LCD_D05__GPIO_1_5) ? 1 : 0) << 5;
#elif defined(CONFIG_MX28)
- /* Setup bootmode pins as GPIO input */
- gpio_direction_input(MX28_PAD_LCD_D00__GPIO_1_0);
- gpio_direction_input(MX28_PAD_LCD_D01__GPIO_1_1);
- gpio_direction_input(MX28_PAD_LCD_D02__GPIO_1_2);
- gpio_direction_input(MX28_PAD_LCD_D03__GPIO_1_3);
- gpio_direction_input(MX28_PAD_LCD_D04__GPIO_1_4);
- gpio_direction_input(MX28_PAD_LCD_D05__GPIO_1_5);
-
- /* Read bootmode pads */
- bootmode |= (gpio_get_value(MX28_PAD_LCD_D00__GPIO_1_0) ? 1 : 0) << 0;
- bootmode |= (gpio_get_value(MX28_PAD_LCD_D01__GPIO_1_1) ? 1 : 0) << 1;
- bootmode |= (gpio_get_value(MX28_PAD_LCD_D02__GPIO_1_2) ? 1 : 0) << 2;
- bootmode |= (gpio_get_value(MX28_PAD_LCD_D03__GPIO_1_3) ? 1 : 0) << 3;
- bootmode |= (gpio_get_value(MX28_PAD_LCD_D04__GPIO_1_4) ? 1 : 0) << 4;
- bootmode |= (gpio_get_value(MX28_PAD_LCD_D05__GPIO_1_5) ? 1 : 0) << 5;
+ /* The global boot mode will be detected by ROM code and its value
+* is stored at the fixed address 0x00019BF0 in OCRAM.
+*/
+#define GLOBAL_BOOT_MODE_ADDR 0x00019BF0
+ bootmode = __raw_readl(GLOBAL_BOOT_MODE_ADDR);
#endif
for (i = 0; i < ARRAY_SIZE(mxs_boot_modes); i++) {
--
2.3.4
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/1] ARM: mxs: get boot mode from OTP
Hi Stefano, On Do, 2015-03-26 at 13:31 +0100, Stefano Babic wrote: > Hi Jörg, > > On 26/03/2015 10:39, Jörg Krause wrote: > > Reading the GPIOs for getting the boot mode does not show the correct result > > for USB boot mode in case the recovery switch, eg. BM2 for switching from > > NAND > > to USB boot mode, is hold down while plugging in USB and released before > > U-Boot > > is loaded by mxsldr. > > > > This state is stored in the HW_OCOTP_ROM0 register. HW_OCOTP_ROM0[27:24] > > maps to > > BM3-BM0, HW_OCOTP_ROM0[28] maps to voltage selector. > > > > For using mxs_wait_mask_clr() add imx-common/misc.o to the SPL build. > > > > As far as I understand from the manual, bootmode is *always* copied > fromt the OTP after a reset. Then, I agree that is better to use OCOTP > as the GPIOs. GPIOs were already sampled by ROM at reset and their value > could be different when evaluated by SPL. I fear the patch does not work as I expected. With OCOTP_ROM0 the boot mode can be blown, but it does not present the state of the BM pads as I understood from the RM. I thought the patch works because the boot mode is read as USB (0x0) if I do USB recovery, but it's also 0x0 for NAND boot. Sadly, no reliable way to detect boot mode. Sorry for submitting to early! Best regards Jörg Krause ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 1/1] ARM: mxs: get boot mode from OTP
Reading the GPIOs for getting the boot mode does not show the correct result
for USB boot mode in case the recovery switch, eg. BM2 for switching from NAND
to USB boot mode, is hold down while plugging in USB and released before U-Boot
is loaded by mxsldr.
This state is stored in the HW_OCOTP_ROM0 register. HW_OCOTP_ROM0[27:24] maps to
BM3-BM0, HW_OCOTP_ROM0[28] maps to voltage selector.
For using mxs_wait_mask_clr() add imx-common/misc.o to the SPL build.
Signed-off-by: Jörg Krause
Cc: Stefano Babic
---
arch/arm/Makefile | 2 +-
arch/arm/cpu/arm926ejs/mxs/spl_boot.c | 73 +-
arch/arm/include/asm/arch-mxs/regs-ocotp.h | 2 +
drivers/misc/mxs_ocotp.c | 2 -
4 files changed, 14 insertions(+), 65 deletions(-)
diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index 08946de..55fe509 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -37,7 +37,7 @@ libs-y += arch/arm/cpu/
libs-y += arch/arm/lib/
ifeq ($(CONFIG_SPL_BUILD),y)
-ifneq (,$(CONFIG_MX23)$(CONFIG_MX35)$(filter $(SOC), mx25 mx27 mx5 mx6 mx31
mx35))
+ifneq (,$(CONFIG_MX23)$(CONFIG_MX35)$(filter $(SOC), mx25 mx27 mx5 mx6 mx31
mx35 mxs))
libs-y += arch/arm/imx-common/
endif
else
diff --git a/arch/arm/cpu/arm926ejs/mxs/spl_boot.c
b/arch/arm/cpu/arm926ejs/mxs/spl_boot.c
index d7956e5..c5bf3c4 100644
--- a/arch/arm/cpu/arm926ejs/mxs/spl_boot.c
+++ b/arch/arm/cpu/arm926ejs/mxs/spl_boot.c
@@ -40,73 +40,22 @@ void early_delay(int delay)
;
}
-#defineMUX_CONFIG_BOOTMODE_PAD (MXS_PAD_3V3 | MXS_PAD_4MA |
MXS_PAD_NOPULL)
-static const iomux_cfg_t iomux_boot[] = {
-#if defined(CONFIG_MX23)
- MX23_PAD_LCD_D00__GPIO_1_0 | MUX_CONFIG_BOOTMODE_PAD,
- MX23_PAD_LCD_D01__GPIO_1_1 | MUX_CONFIG_BOOTMODE_PAD,
- MX23_PAD_LCD_D02__GPIO_1_2 | MUX_CONFIG_BOOTMODE_PAD,
- MX23_PAD_LCD_D03__GPIO_1_3 | MUX_CONFIG_BOOTMODE_PAD,
- MX23_PAD_LCD_D04__GPIO_1_4 | MUX_CONFIG_BOOTMODE_PAD,
- MX23_PAD_LCD_D05__GPIO_1_5 | MUX_CONFIG_BOOTMODE_PAD,
-#elif defined(CONFIG_MX28)
- MX28_PAD_LCD_D00__GPIO_1_0 | MUX_CONFIG_BOOTMODE_PAD,
- MX28_PAD_LCD_D01__GPIO_1_1 | MUX_CONFIG_BOOTMODE_PAD,
- MX28_PAD_LCD_D02__GPIO_1_2 | MUX_CONFIG_BOOTMODE_PAD,
- MX28_PAD_LCD_D03__GPIO_1_3 | MUX_CONFIG_BOOTMODE_PAD,
- MX28_PAD_LCD_D04__GPIO_1_4 | MUX_CONFIG_BOOTMODE_PAD,
- MX28_PAD_LCD_D05__GPIO_1_5 | MUX_CONFIG_BOOTMODE_PAD,
-#endif
-};
-
static uint8_t mxs_get_bootmode_index(void)
{
- uint8_t bootmode = 0;
- int i;
- uint8_t masked;
-
- /* Setup IOMUX of bootmode pads to GPIO */
- mxs_iomux_setup_multiple_pads(iomux_boot, ARRAY_SIZE(iomux_boot));
-
-#if defined(CONFIG_MX23)
- /* Setup bootmode pins as GPIO input */
- gpio_direction_input(MX23_PAD_LCD_D00__GPIO_1_0);
- gpio_direction_input(MX23_PAD_LCD_D01__GPIO_1_1);
- gpio_direction_input(MX23_PAD_LCD_D02__GPIO_1_2);
- gpio_direction_input(MX23_PAD_LCD_D03__GPIO_1_3);
- gpio_direction_input(MX23_PAD_LCD_D05__GPIO_1_5);
-
- /* Read bootmode pads */
- bootmode |= (gpio_get_value(MX23_PAD_LCD_D00__GPIO_1_0) ? 1 : 0) << 0;
- bootmode |= (gpio_get_value(MX23_PAD_LCD_D01__GPIO_1_1) ? 1 : 0) << 1;
- bootmode |= (gpio_get_value(MX23_PAD_LCD_D02__GPIO_1_2) ? 1 : 0) << 2;
- bootmode |= (gpio_get_value(MX23_PAD_LCD_D03__GPIO_1_3) ? 1 : 0) << 3;
- bootmode |= (gpio_get_value(MX23_PAD_LCD_D05__GPIO_1_5) ? 1 : 0) << 5;
-#elif defined(CONFIG_MX28)
- /* Setup bootmode pins as GPIO input */
- gpio_direction_input(MX28_PAD_LCD_D00__GPIO_1_0);
- gpio_direction_input(MX28_PAD_LCD_D01__GPIO_1_1);
- gpio_direction_input(MX28_PAD_LCD_D02__GPIO_1_2);
- gpio_direction_input(MX28_PAD_LCD_D03__GPIO_1_3);
- gpio_direction_input(MX28_PAD_LCD_D04__GPIO_1_4);
- gpio_direction_input(MX28_PAD_LCD_D05__GPIO_1_5);
-
- /* Read bootmode pads */
- bootmode |= (gpio_get_value(MX28_PAD_LCD_D00__GPIO_1_0) ? 1 : 0) << 0;
- bootmode |= (gpio_get_value(MX28_PAD_LCD_D01__GPIO_1_1) ? 1 : 0) << 1;
- bootmode |= (gpio_get_value(MX28_PAD_LCD_D02__GPIO_1_2) ? 1 : 0) << 2;
- bootmode |= (gpio_get_value(MX28_PAD_LCD_D03__GPIO_1_3) ? 1 : 0) << 3;
- bootmode |= (gpio_get_value(MX28_PAD_LCD_D04__GPIO_1_4) ? 1 : 0) << 4;
- bootmode |= (gpio_get_value(MX28_PAD_LCD_D05__GPIO_1_5) ? 1 : 0) << 5;
-#endif
+ struct mxs_ocotp_regs *ocotp_regs =
+ (struct mxs_ocotp_regs *)MXS_OCOTP_BASE;
+ uint32_t data;
+
+ writel(OCOTP_CTRL_RD_BANK_OPEN, &ocotp_regs->hw_ocotp_ctrl_set);
- for (i = 0; i < ARRAY_SIZE(mxs_boot_modes); i++) {
- masked = bootmode & mxs_boot_modes[i].boot_mask;
- if (masked == mxs_boot_modes[i].boot_pads)
- break;
+ if (mxs_wait_mask_clr(&ocotp_regs-
Re: [U-Boot] Rework the network stack
Joe, Simon,
On Mo, 2015-03-23 at 10:46 -0600, Simon Glass wrote:
> Hi Jörg,
>
> On 22 March 2015 at 14:37, Jörg Krause wrote:
> > Hi Joe,
> >
> > On Sa, 2015-03-21 at 22:59 -0500, Joe Hershberger wrote:
> >> Hi Jörg,
> >>
> >> On Sat, Mar 21, 2015 at 3:33 AM, Jörg Krause
> >> wrote:
> >> >
> >> > Hi all,
> >> >
> >> > there is an issue with the current network stack using netconsole. It's
> >> > impossible to use network commands as TFTP inside netconsole, because
> >> > they try to run as atomic network commands.
> >> >
> >> > The issue was already reported by Stefano Babic in 2010:
> >> > [U-Boot] NetConsole and network API
> >> > http://lists.denx.de/pipermail/u-boot/2010-August/075535.html
> >>
> >> I worked around this problem here:
> >>
> >> http://lists.denx.de/pipermail/u-boot/2012-August/129913.html
> >
> > I know about this patch, and I understand the problem it tries to
> > workaround. But it does not work for using netconsole with another
> > protocol like ping. When for example ping enters the NetLoop() it will
> > halt the network device.
> >
> > The problem for this is here:
> > if (eth_is_on_demand_init() || protocol != NETCONS) {
> > eth_halt();
> > ...
> > }
> >
> > eth_is_on_demand_init() returns correctly with false, because PING !=
> > NETCONS. But the second expression results in true, because PING !=
> > NETCONS.
> >
> >> > I run into the same problem:
> >> > [U-Boot] netconsole: USB Ethernet connection dropping with ping or
> >> > tftpboot
> >> > http://lists.denx.de/pipermail/u-boot/2015-February/203838.html
> >>
> >> I didn't understand what about your case was not able to work given the
> >> workaround I implemented previously. What was different about it?
> >>
> >> > I have looked at the current network stack. The stack is based on the
> >> > concept of atomic network commands. The implementation for netconsole
> >> > looks very confusing.
> >>
> >> There is no doubt that netconsole is quite confusing as it exists today.
> >
> > I tried to fix the issue, but it did not work properly.
> >
> >> > Sascha Hauer has reimplemented the network stack for Barebox:
> >> > http://www.spinics.net/lists/u-boot-v2/msg00914.html
> >> >
> >> > Looking at the current implementation of net.c looks very clean and
> >> > well-designed.
> >>
> >> Thanks for pointing this out. I hadn't gone to look at the network stack in
> >> barebox.
> >>
> >> > What do you think about porting this to U-Boot?
> >>
> >> I can look into this. Naturally there are many other changes to u-boot
> >> network stack since the time barebox forked, so I expect such a port to be
> >> very intensive... most likely a near complete rewrite of Sascha's series,
> >> but I will investigate further.
> >
> > I had a look at the patches and the code base is not entirely the same.
> > But maybe we should just stick to the crucial points of the new network
> > stack:
> >
> > 1) A network device gets initialized when it becomes the current one and
> > gets brought down when it's not used anymore or prior booting to Linux
> > so the connection remains active all the time.
> >
> > My thoughts about this one:
> > Bringing the device down can be done in eth_unregister(). Initializing
> > for the current device can be done in eth_current_changed(). What I like
> > even better is to make eth_current_changed() the sole place for calling
> > init() and halt(). It would have to be extend to take at least two
> > parameters, eth_current and new_current. If eth_current is null just
> > bring up the new device, if new_current is null just bring down the
> > current device.
> >
> > 2) Replace the NetLoop by a connection list where each protocol uses a
> > connection to send and receive packets.
> >
> > This induce to port all protocols to use the new network, of cause.
> >
> > In my opinion it would be also good to do some clean ups and
> > restructuring of code. Put more functions into net-utils, is there a
> > need for eth_init and so on.
>
> Joe has done a lot of work to move U-Boot's network stack over to
> driver model. This is now at u-boot-dm/next waiting for the merge
> window to open.
>
> The connection list idea does not sound like a hugely complex change.
> Are you thinking of trying it out?
I have just noticed today the big series of u-boot-dm patches of Joe. I
will try it out. But before I will start I want to discuss the concept.
What do you think about my thoughts about bringing up and down a network
device?
Best regards,
Jörg
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Rework the network stack
Hi Joe,
On Sa, 2015-03-21 at 22:59 -0500, Joe Hershberger wrote:
> Hi Jörg,
>
> On Sat, Mar 21, 2015 at 3:33 AM, Jörg Krause
> wrote:
> >
> > Hi all,
> >
> > there is an issue with the current network stack using netconsole. It's
> > impossible to use network commands as TFTP inside netconsole, because
> > they try to run as atomic network commands.
> >
> > The issue was already reported by Stefano Babic in 2010:
> > [U-Boot] NetConsole and network API
> > http://lists.denx.de/pipermail/u-boot/2010-August/075535.html
>
> I worked around this problem here:
>
> http://lists.denx.de/pipermail/u-boot/2012-August/129913.html
I know about this patch, and I understand the problem it tries to
workaround. But it does not work for using netconsole with another
protocol like ping. When for example ping enters the NetLoop() it will
halt the network device.
The problem for this is here:
if (eth_is_on_demand_init() || protocol != NETCONS) {
eth_halt();
...
}
eth_is_on_demand_init() returns correctly with false, because PING !=
NETCONS. But the second expression results in true, because PING !=
NETCONS.
> > I run into the same problem:
> > [U-Boot] netconsole: USB Ethernet connection dropping with ping or
> > tftpboot
> > http://lists.denx.de/pipermail/u-boot/2015-February/203838.html
>
> I didn't understand what about your case was not able to work given the
> workaround I implemented previously. What was different about it?
>
> > I have looked at the current network stack. The stack is based on the
> > concept of atomic network commands. The implementation for netconsole
> > looks very confusing.
>
> There is no doubt that netconsole is quite confusing as it exists today.
I tried to fix the issue, but it did not work properly.
> > Sascha Hauer has reimplemented the network stack for Barebox:
> > http://www.spinics.net/lists/u-boot-v2/msg00914.html
> >
> > Looking at the current implementation of net.c looks very clean and
> > well-designed.
>
> Thanks for pointing this out. I hadn't gone to look at the network stack in
> barebox.
>
> > What do you think about porting this to U-Boot?
>
> I can look into this. Naturally there are many other changes to u-boot
> network stack since the time barebox forked, so I expect such a port to be
> very intensive... most likely a near complete rewrite of Sascha's series,
> but I will investigate further.
I had a look at the patches and the code base is not entirely the same.
But maybe we should just stick to the crucial points of the new network
stack:
1) A network device gets initialized when it becomes the current one and
gets brought down when it's not used anymore or prior booting to Linux
so the connection remains active all the time.
My thoughts about this one:
Bringing the device down can be done in eth_unregister(). Initializing
for the current device can be done in eth_current_changed(). What I like
even better is to make eth_current_changed() the sole place for calling
init() and halt(). It would have to be extend to take at least two
parameters, eth_current and new_current. If eth_current is null just
bring up the new device, if new_current is null just bring down the
current device.
2) Replace the NetLoop by a connection list where each protocol uses a
connection to send and receive packets.
This induce to port all protocols to use the new network, of cause.
In my opinion it would be also good to do some clean ups and
restructuring of code. Put more functions into net-utils, is there a
need for eth_init and so on.
Best regards
Jörg Krause
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] Rework the network stack
Hi all, there is an issue with the current network stack using netconsole. It's impossible to use network commands as TFTP inside netconsole, because they try to run as atomic network commands. The issue was already reported by Stefano Babic in 2010: [U-Boot] NetConsole and network API http://lists.denx.de/pipermail/u-boot/2010-August/075535.html I run into the same problem: [U-Boot] netconsole: USB Ethernet connection dropping with ping or tftpboot http://lists.denx.de/pipermail/u-boot/2015-February/203838.html I have looked at the current network stack. The stack is based on the concept of atomic network commands. The implementation for netconsole looks very confusing. Sascha Hauer has reimplemented the network stack for Barebox: http://www.spinics.net/lists/u-boot-v2/msg00914.html Looking at the current implementation of net.c looks very clean and well-designed. What do you think about porting this to U-Boot? Best regards Jörg Krause ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] usbtty on Freescale i.MX28
Hi all, I know there is support of NetConsole for i.MX28, but how about usbtty? Does anyone use it? If not, what are the limitations? Best regards Jörg ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] netconsole: USB Ethernet connection dropping with ping or tftpboot
On Mo, 2015-02-09 at 10:38 -0700, Stephen Warren wrote: > On 02/08/2015 02:25 PM, Jörg Krause wrote: > > On Fr, 2015-02-06 at 11:06 -0700, Stephen Warren wrote: > >> On 02/05/2015 06:06 PM, Jörg Krause wrote: > >>> On Do, 2015-02-05 at 15:23 -0700, Stephen Warren wrote: > >>>> > >>>> b) In ci_bounce(), the bounce buffer is only allocated if the > >>>> user-buffer is already aligned, and if a large-enough bounce buffer > >>>> wasn't previously allocated. If ci_req->b_buf was uninitialized it could > >>>> be non-zero (thus preventing the expected aligned allocation) yet not > >>>> actually aligned enough. > >>> > >>> I can reproduce this issue now. After some "timeout sending packets to > >>> usb ethernet" messages, the bounce buffer somehow gets corrupted. > >>> ci_bounce() is called with an unaligned input buffer length > >>> 'req->length=66', but the bounce buffer length > >>> 'ci_req->b_len=1140305940' or in hex 'ci_req->b_len=0x43f7b014'. This > >>> bounce buffer length is obviously an address, as the following > >>> misaligned error message shows: "CACHE: Misaligned operation at range > >>> [43f7b010, 43f7b070]". > >> > >> Ah, I hadn't realized that was [start, length] rather than [start, end]. > >> > >> The question is: How is ci_req->b_len getting corrupted? Is it simply > >> never initialized, or does something trash that value later? > >> > >> ci_ep_alloc_request() appears to calloc() the whole struct ci_req, so I > >> imagine an initialization/allocating error isn't happening. > >> > >> The only issue there might be some code somehow creating its own struct > >> usb_request instead of calling into the controller's ->alloc_request() > >> function. I vaguely recall fixing some of those, but might have missed > >> some in protocols that I didn't test (i.e. anything other than USB Mass > >> Storage or DFU, although I might have very briefly tested netconsole > >> once?). > >> > >> I would suggest adding a whole ton of printfs() to catch where ci_reqs > >> are being allocated, and where ci_req->b_len is getting written in which > >> ci_req objects, and then mapping that back to the ci_req that the cache > >> alignment error message complains about. Sorry, this will be a bit painful. > >> > >> If the ci_req is always at the same address on different boots of the > >> code, that will make it easier, especially if you have a debugger with a > >> data watchpoint, or can write some code to use any data watchpoint > >> self-hosted debug capability in your CPU. > > > > I think I found the answer. > > > > I used a lot of debug messages and tried to understand the involved > > drivers. I will try to sum up my investigations. > > > > NetLoop is entered first for the ping protocol and then for the > > netconsole protocol: > > --- NetLoop Entry (PING) > > --- NetLoop Entry (NETCONSOLE) > > > > ci_udc is probed and ci_ep_alloc_request() is called for EP0: > > ci_ep_alloc_request usb_ep:0x43fd0028 ci_req:0x43b83220 > > > > In eth_bind() ci_ep_alloc_request() is called for status EP: > > ci_ep_alloc_request usb_ep:0x43fd00a0 ci_req:0x43b7b4a8 > > > > Everything is fine: > > using ci_udc, OUT ep- IN ep- STATUS ep- > > MAC 00:19:b8:00:00:02 > > HOST MAC 00:19:b8:00:00:01 > > > > In eth_set_config() ci_ep_alloc_request() is invoked again for tx_req > > and rx_req: > > ci_ep_alloc_request usb_ep:0x43fd0050 ci_req:0x43b7b568 > > ci_ep_alloc_request usb_ep:0x43fd0078 ci_req:0x43b7b5b0 > > > > Everything is fine: > > high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet > > USB network up! > > > > Now the NetLoop is available for netconsole: > > --- NetState set to 0 (CONTINUE) > > --- NetLoop Init (NETCONSOLE) > > --- NetLoop ARP handler set (43fa3b14) > > [..] > > Got ARP REPLY, set eth addr (00:19:b8:00:00:01) > > --- NetState set to 2 (SUCCESS) > > --- NetLoop UDP handler set () > > --- NetLoop ARP handler set () > > --- NetLoop timeout handler cancelled > > --- NetLoop Success! (NETCONSOLE) > > > > Now it&
Re: [U-Boot] netconsole: USB Ethernet connection dropping with ping or tftpboot
On Fr, 2015-02-06 at 11:06 -0700, Stephen Warren wrote: > On 02/05/2015 06:06 PM, Jörg Krause wrote: > > On Do, 2015-02-05 at 15:23 -0700, Stephen Warren wrote: > >> > >> b) In ci_bounce(), the bounce buffer is only allocated if the > >> user-buffer is already aligned, and if a large-enough bounce buffer > >> wasn't previously allocated. If ci_req->b_buf was uninitialized it could > >> be non-zero (thus preventing the expected aligned allocation) yet not > >> actually aligned enough. > > > > I can reproduce this issue now. After some "timeout sending packets to > > usb ethernet" messages, the bounce buffer somehow gets corrupted. > > ci_bounce() is called with an unaligned input buffer length > > 'req->length=66', but the bounce buffer length > > 'ci_req->b_len=1140305940' or in hex 'ci_req->b_len=0x43f7b014'. This > > bounce buffer length is obviously an address, as the following > > misaligned error message shows: "CACHE: Misaligned operation at range > > [43f7b010, 43f7b070]". > > Ah, I hadn't realized that was [start, length] rather than [start, end]. > > The question is: How is ci_req->b_len getting corrupted? Is it simply > never initialized, or does something trash that value later? > > ci_ep_alloc_request() appears to calloc() the whole struct ci_req, so I > imagine an initialization/allocating error isn't happening. > > The only issue there might be some code somehow creating its own struct > usb_request instead of calling into the controller's ->alloc_request() > function. I vaguely recall fixing some of those, but might have missed > some in protocols that I didn't test (i.e. anything other than USB Mass > Storage or DFU, although I might have very briefly tested netconsole once?). > > I would suggest adding a whole ton of printfs() to catch where ci_reqs > are being allocated, and where ci_req->b_len is getting written in which > ci_req objects, and then mapping that back to the ci_req that the cache > alignment error message complains about. Sorry, this will be a bit painful. > > If the ci_req is always at the same address on different boots of the > code, that will make it easier, especially if you have a debugger with a > data watchpoint, or can write some code to use any data watchpoint > self-hosted debug capability in your CPU. I think I found the answer. I used a lot of debug messages and tried to understand the involved drivers. I will try to sum up my investigations. NetLoop is entered first for the ping protocol and then for the netconsole protocol: --- NetLoop Entry (PING) --- NetLoop Entry (NETCONSOLE) ci_udc is probed and ci_ep_alloc_request() is called for EP0: ci_ep_alloc_request usb_ep:0x43fd0028 ci_req:0x43b83220 In eth_bind() ci_ep_alloc_request() is called for status EP: ci_ep_alloc_request usb_ep:0x43fd00a0 ci_req:0x43b7b4a8 Everything is fine: using ci_udc, OUT ep- IN ep- STATUS ep- MAC 00:19:b8:00:00:02 HOST MAC 00:19:b8:00:00:01 In eth_set_config() ci_ep_alloc_request() is invoked again for tx_req and rx_req: ci_ep_alloc_request usb_ep:0x43fd0050 ci_req:0x43b7b568 ci_ep_alloc_request usb_ep:0x43fd0078 ci_req:0x43b7b5b0 Everything is fine: high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet USB network up! Now the NetLoop is available for netconsole: --- NetState set to 0 (CONTINUE) --- NetLoop Init (NETCONSOLE) --- NetLoop ARP handler set (43fa3b14) [..] Got ARP REPLY, set eth addr (00:19:b8:00:00:01) --- NetState set to 2 (SUCCESS) --- NetLoop UDP handler set () --- NetLoop ARP handler set () --- NetLoop timeout handler cancelled --- NetLoop Success! (NETCONSOLE) Now it's pings turn in the NetLoop. The if (eth_is_on_demand_init()) branch is executed. eth_halt() and later eth_disconnect() and eth_reset_config() are invoked. There the in and out EPs are freed: ci_ep_free_request ci_req:0x43b7b568 ci_ep_free_request ci_req:0x43b7b5b0 However, netconsole tries to send every printf() as an UDP packet to the host. sending UDP to 10.0.0.1/00:19:b8:00:00:01 But the usb_request [1] pointer in usb_eth_send is NULL after the free request: usb_eth_send usb_request: length: 82 usb_eth_send() calls ci_ep_queue() which calls ci_bounce() and because of the corrupted ci_req pointer I get a misaligned cache. So, as far as I understand, the main problem is that netconsole does not knows that the connection is disconnected by the NetLoop? What are your suggestions to this issue? Should we add an addition check for an initialized
Re: [U-Boot] netconsole: USB Ethernet connection dropping with ping or tftpboot
On Do, 2015-02-05 at 15:23 -0700, Stephen Warren wrote:
>
> b) In ci_bounce(), the bounce buffer is only allocated if the
> user-buffer is already aligned, and if a large-enough bounce buffer
> wasn't previously allocated. If ci_req->b_buf was uninitialized it could
> be non-zero (thus preventing the expected aligned allocation) yet not
> actually aligned enough.
I can reproduce this issue now. After some "timeout sending packets to
usb ethernet" messages, the bounce buffer somehow gets corrupted.
ci_bounce() is called with an unaligned input buffer length
'req->length=66', but the bounce buffer length
'ci_req->b_len=1140305940' or in hex 'ci_req->b_len=0x43f7b014'. This
bounce buffer length is obviously an address, as the following
misaligned error message shows: "CACHE: Misaligned operation at range
[43f7b010, 43f7b070]".
Both if conditions in 'align:' are not entered.
This is a snippet from my debug output:
timeout sending packets to usb ethernet
1: 43b7e180 - 43b7e200.
req->length: 66
b_len_1: 96
b_len_2: 96
5: 43b7e660 - 43b7e6c0
timeout sending packets to usb ethernet
1: 43b7e000 - 43b7e080.
req->length: 66
b_len_1: 1140305940
b_len_2: 1140305940
5: 43f7b010 - 43f7b070
CACHE: Misaligned operation at range [43f7b010, 43f7b070]
timeout sending packets to usb ethernet
ping failed; host 10.0.0.1 is not alive
This is the corresponding code (debug number 1 and 5):
static void ci_flush_qh(int ep_num)
{
struct ept_queue_head *head = ci_get_qh(ep_num, 0);
const uint32_t start = (uint32_t)head;
const uint32_t end = start + 2 * sizeof(*head);
printf("1: %x - %x.\n", start, end);
flush_dcache_range(start, end);
}
[..]
static int ci_bounce(struct ci_req *ci_req, int in)
{
struct usb_request *req = &ci_req->req;
uint32_t addr = (uint32_t)req->buf;
uint32_t hwaddr;
uint32_t aligned_used_len;
/* Input buffer address is not aligned. */
if (addr & (ARCH_DMA_MINALIGN - 1)) {
goto align;
}
/* Input buffer length is not aligned. */
if (req->length & (ARCH_DMA_MINALIGN - 1)) {
printf("req->length: %d\n", req->length);
goto align;
}
/* The buffer is well aligned, only flush cache. */
ci_req->hw_len = req->length;
ci_req->hw_buf = req->buf;
goto flush;
align:
printf("b_len_1: %d\n", ci_req->b_len);
if (ci_req->b_buf && req->length > ci_req->b_len) {
printf("A: %d - %d\n", req->length, ci_req->b_len);
free(ci_req->b_buf);
ci_req->b_buf = 0;
}
if (!ci_req->b_buf) {
ci_req->b_len = roundup(req->length, ARCH_DMA_MINALIGN);
ci_req->b_buf = memalign(ARCH_DMA_MINALIGN,
ci_req->b_len);
printf("B: %d - %d\n", req->length, ci_req->b_len);
if (!ci_req->b_buf)
return -ENOMEM;
}
ci_req->hw_len = ci_req->b_len;
ci_req->hw_buf = ci_req->b_buf;
printf("b_len_2: %d\n", ci_req->b_len);
if (in)
memcpy(ci_req->hw_buf, req->buf, req->length);
flush:
hwaddr = (uint32_t)ci_req->hw_buf;
aligned_used_len = roundup(req->length, ARCH_DMA_MINALIGN);
printf("5: %x - %x\n", hwaddr, hwaddr + aligned_used_len);
flush_dcache_range(hwaddr, hwaddr + aligned_used_len);
return 0;
}
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] netconsole: USB Ethernet connection dropping with ping or tftpboot
On Do, 2015-02-05 at 15:23 -0700, Stephen Warren wrote:
> On 02/05/2015 03:10 PM, Jörg Krause wrote:
> > On Do, 2015-02-05 at 08:33 -0700, Stephen Warren wrote:
> >> On 02/05/2015 04:21 AM, Jörg Krause wrote:
> ...
> >>> This reminded me about an issue I had some months ago:
> >>> http://lists.denx.de/pipermail/u-boot/2014-July/182885.html
> >>>
> >>> I enabled debug output in arch/arm/cpu/arm926ejs/cache.c and I get
> >>> error:
> >>> => tftp u-boot.sb
> >>> using ci_udc, OUT ep- IN ep- STATUS ep-
> >>> CACHE: Misaligned operation at range [43f7b010, 43f7b070]
> >>>
> >>> I removed the flush_cache() call in cmd_net.c:netboot_common() as
> >>> suggested by Marek in the thread. But the error message is still there.
> >>
> >> Perhaps make flush_cache() a macro that also passes in the file/line
> >> number where it's called from, and print those out along with he
> >> "misaligned operation" error message?
> >>
> >> (or find some other way to perform a stack dump from within that function).
> >
> > I've added in each function in ci_udc a printf statement before a cache
> > function is called, eg:
> >
> > static void ci_flush_qh(int ep_num)
> > {
> > [..]
> > printf("CACHE: flush_dcache_range %s %d
> > \n",__FUNCTION__,__LINE__);
> > flush_dcache_range(start, end);
> > }
> >
> > I've also looked at all calling functions of flush_cache or
> > flush_dcache_range, but I think there is nothing of relevance.
> >
> > This is a snippet of the output:
> >
> ...
> > CACHE: flush_dcache_range ci_bounce 356
> > CACHE: Misaligned operation at range [43f7b010, 43f7b070]
> > timeout sending packets to usb ethernet
> > ping failed; host 10.0.0.1 is not alive
>
> Which git commit did you build, and which board?
Building tag v2015.01 for a custom i.MX28 board which is not upstream.
It's configuration mainly based on mx28evk and m28evk.
>
> I'm curious what values you have for ARCH_DMA_MINALIGN and
> CONFIG_SYS_CACHELINE_SIZE.
I defined CONFIG_SYS_CACHELINE_SIZE 32 in the config. ARCH_DMA_MINALIGN
is also 32.
>
> Are you sure flush_dcache_range() is the code printing the alignment
> error, not e.g. invalidate_dcache_range()?
I've also added a printf statement to all functions in ci_udc which
calls invalidate_dcache_range(), but it is not logged when the alignment
error occurs. e.g.
using ci_udc, OUT ep- IN ep- STATUS ep-
MAC 00:19:b8:00:00:02
HOST MAC 00:19:b8:00:00:01
CACHE: flush_dcache_range ci_flush_qh 166
CACHE: invalidate_dcache_range ci_invalidate_qh 182
CACHE: flush_dcache_range ci_bounce 356
CACHE: flush_dcache_range ci_flush_qtd 199
CACHE: flush_dcache_range ci_flush_qh 166
>
> The reason I ask most of these questions is that line 356 mentioned in
> your debug spew is in function ci_debounce() in the code I have; no
> doubt I have a different git commit than you have checked out, and the
> debug printfs you added changed the line numbers too.
You're right, the debug printfs changed the line number. It's the
flush_dcache_range() call at line 348 in ci_bounce(). I checked this
after removing all printfs.
> About the only thing I can think of is that:
>
> a) You're not using an up-to-date ci_udc.c; IIRC I fixed quite a few
> cache issues when I reworked it a while back.
Yes, we had a troubleshooting about this last summer. I had problems
with timeouts using tftp. Marek and you worked on this issue.
>
> b) In ci_bounce(), the bounce buffer is only allocated if the
> user-buffer is already aligned, and if a large-enough bounce buffer
> wasn't previously allocated. If ci_req->b_buf was uninitialized it could
> be non-zero (thus preventing the expected aligned allocation) yet not
> actually aligned enough.
Maybe, we should work on this?
>
> c) Perhaps ARCH_DMA_MINALIGN or CONFIG_SYS_CACHELINE_SIZE aren't correct
> or are inconsistent.
>
> Ah. I note that check_cache_range() in either arch/arm/cpu/arm1136/cpu.c
> or arch/arm/cpu/arm926ejs/cache.c uses CONFIG_SYS_CACHELINE_SIZE to
> check for alignment, whereas ci_udc.c uses ARCH_DMA_MINALIGN.
> Inconsistency between those two could be at fault.
Both are set to 32, so this should not be the problem.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] netconsole: USB Ethernet connection dropping with ping or tftpboot
On Do, 2015-02-05 at 14:48 -0600, Joe Hershberger wrote: > > > On Thu, Feb 5, 2015 at 2:39 PM, Jörg Krause wrote: > > > > Hi Joe, > > > > On Do, 2015-02-05 at 13:20 -0600, Joe Hershberger wrote: > > > On Tue, Feb 3, 2015 at 3:44 PM, Jörg Krause > wrote: > > > > But if I use 'ping 10.0.0.1' or 'tftpboot u-boot.sb' the network > > > > connection drops. Both commands work fine if I switch back from > > > > netconsole to serial in-/output. > > > > > > > > This is the output from dmesg: > > > > [31620.215354] usb 3-13: USB disconnect, device number > 24 > > > > [31620.215422] cdc_ether 3-13:1.0 enp0s20u13: unregister > > > > 'cdc_ether' usb-:00:14.0-13, CDC Ethernet Device > > > > > > One thing to note is that the network stack generally is designed > to > > > sit in a state of inactive (i.e. devices halted / inactive). When > a > > > network command is issued, the driver is initialized, then the > command > > > runs, then the driver is halted again. > > > > > > > > > NetConsole breaks this assumption, since the network stack needs > to be > > > up the whole time it is selected. Net console used to bring the > > > network driver up and down for every character it sent. Naturally > > > this was a huge problem on USB network adapters that don't come up > > > fast or any other that doesn't. As a workaround (and its current > > > state) when net console is used, the network stack is lied to > about > > > the state of the driver (telling it the driver is halted or > inited) > > > when the current and previous packets are net console packets. > When a > > > different type (ping or tftp, etc) of network packet is sent, the > > > driver is actually brought down and back up to ensure the > assumptions > > > about the network stack hold true. With more work we can > potentially > > > make these better when net console is enabled. > > > > Thank you for the explanation! What do you mean with more work? Do > you > > have already some ideas in mind? > > I think it might be possible to work through the reasons that these > network functions assume the network interface should be down when > they begin and for certain special cases not bring them down at all > when net console is enabled. I think this is likely to be > non-trivial, though. If you look in include/net.h you'll see > eth_is_on_demand_init(). Changing the logic here would be the first > step to testing. I think I read sth about that the network interface should be down or in a well-defined state for handling over the control to the Linux kernel. There where some problems with the Linux drivers, if I remember rigthly. I will take a closer look at eth_is_on_demand_init. Thank you! ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] netconsole: USB Ethernet connection dropping with ping or tftpboot
On Do, 2015-02-05 at 08:33 -0700, Stephen Warren wrote:
> On 02/05/2015 04:21 AM, Jörg Krause wrote:
> > On Di, 2015-02-03 at 22:44 +0100, Jörg Krause wrote:
> >> I followed the instructions from Marek in 'M28 U-Boot Single-Wire Debug
> >> preview' to enable NetConsole:
> >> http://www.denx-cs.de/?q=blogm28singlewiredebug
> >>
> >> I'm using mxsldr to flash the u-boot.sb image to RAM. This is the dmesg
> >> output from my host:
> >>
> >> [31048.492181] usb 3-13: new high-speed USB device number 24
> >> using xhci_hcd
> >> [31048.667617] cdc_ether 3-13:1.0 eth0: register 'cdc_ether' at
> >> usb-:00:14.0-13, CDC Ethernet Device, 00:19:b8:00:00:01
> >> [31048.669101] cdc_ether 3-13:1.0 enp0s20u13: renamed from eth0
> >> [31048.696758] cdc_ether 3-13:1.0 enp0s20u13: kevent 12 may have
> >> been dropped
> >>
> >> I noticed the 'kevent 12 may have been dropped' message here.
> >>
> >> Nevertheless, NetworkManager shows me a USB Ethernet connection and
> >> using './netconsole 10.0.0.2' I am able to communicate with the device,
> >> e.g. 'nand info' outputs correctly.
> >>
> >> But if I use 'ping 10.0.0.1' or 'tftpboot u-boot.sb' the network
> >> connection drops. Both commands work fine if I switch back from
> >> netconsole to serial in-/output.
> >>
> >> This is the output from dmesg:
> >> [31620.215354] usb 3-13: USB disconnect, device number 24
> >> [31620.215422] cdc_ether 3-13:1.0 enp0s20u13: unregister
> >> 'cdc_ether' usb-:00:14.0-13, CDC Ethernet Device
> >>
> >> Do I missed something?
> >
> > I managed to get serial console and netconsole muxing to work, so I can
> > see all the debug messages:
> >
> > Hit any key to stop autoboot: 5 using ci_udc, OUT ep- IN ep-
> > STATUS ep-
> > MAC 00:19:b8:00:00:02
> > HOST MAC 00:19:b8:00:00:01
> > high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet
> > USB network up!
> > 0
> > => tftp u-boot.sb
> > using ci_udc, OUT ep- IN ep- STATUS ep-
> > timeout sending packets to usb ethernet
> >
> > This reminded me about an issue I had some months ago:
> > http://lists.denx.de/pipermail/u-boot/2014-July/182885.html
> >
> > I enabled debug output in arch/arm/cpu/arm926ejs/cache.c and I get
> > error:
> > => tftp u-boot.sb
> > using ci_udc, OUT ep- IN ep- STATUS ep-
> > CACHE: Misaligned operation at range [43f7b010, 43f7b070]
> >
> > I removed the flush_cache() call in cmd_net.c:netboot_common() as
> > suggested by Marek in the thread. But the error message is still there.
>
> Perhaps make flush_cache() a macro that also passes in the file/line
> number where it's called from, and print those out along with he
> "misaligned operation" error message?
>
> (or find some other way to perform a stack dump from within that function).
I've added in each function in ci_udc a printf statement before a cache
function is called, eg:
static void ci_flush_qh(int ep_num)
{
[..]
printf("CACHE: flush_dcache_range %s %d
\n",__FUNCTION__,__LINE__);
flush_dcache_range(start, end);
}
I've also looked at all calling functions of flush_cache or
flush_dcache_range, but I think there is nothing of relevance.
This is a snippet of the output:
Using usb_ether device
CACHE: flush_dcache_range ci_bounce 356
timeout sending packets to usb ethernet
CACHE: flush_dcache_range ci_bounce 356
CACHE: flush_dcache_range ci_bounce 356
timeout sending packets to usb ethernet
timeout sending packets to usb ethernet
CACHE: flush_dcache_range ci_bounce 356
timeout sending packets to usb ethernet
CACHE: flush_dcache_range ci_bounce 356
CACHE: flush_dcache_range ci_bounce 356
timeout sending packets to usb ethernet
timeout sending packets to usb ethernet
CACHE: flush_dcache_range ci_bounce 356
timeout sending packets to usb ethernet
CACHE: flush_dcache_range ci_flush_qh 166
CACHE: flush_dcache_range ci_bounce 356
timeout sending packets to usb ethernet
CACHE: flush_dcache_range ci_flush_qh 166
CACHE: flush_dcache_range ci_bounce 356
timeout sending packets to usb ether
Re: [U-Boot] netconsole: USB Ethernet connection dropping with ping or tftpboot
Hi Joe, On Do, 2015-02-05 at 13:20 -0600, Joe Hershberger wrote: > On Tue, Feb 3, 2015 at 3:44 PM, Jörg Krause wrote: > > But if I use 'ping 10.0.0.1' or 'tftpboot u-boot.sb' the network > > connection drops. Both commands work fine if I switch back from > > netconsole to serial in-/output. > > > > This is the output from dmesg: > > [31620.215354] usb 3-13: USB disconnect, device number 24 > > [31620.215422] cdc_ether 3-13:1.0 enp0s20u13: unregister > > 'cdc_ether' usb-:00:14.0-13, CDC Ethernet Device > > One thing to note is that the network stack generally is designed to > sit in a state of inactive (i.e. devices halted / inactive). When a > network command is issued, the driver is initialized, then the command > runs, then the driver is halted again. > > > NetConsole breaks this assumption, since the network stack needs to be > up the whole time it is selected. Net console used to bring the > network driver up and down for every character it sent. Naturally > this was a huge problem on USB network adapters that don't come up > fast or any other that doesn't. As a workaround (and its current > state) when net console is used, the network stack is lied to about > the state of the driver (telling it the driver is halted or inited) > when the current and previous packets are net console packets. When a > different type (ping or tftp, etc) of network packet is sent, the > driver is actually brought down and back up to ensure the assumptions > about the network stack hold true. With more work we can potentially > make these better when net console is enabled. Thank you for the explanation! What do you mean with more work? Do you have already some ideas in mind? Best regards Jörg ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] netconsole: USB Ethernet connection dropping with ping or tftpboot
On Di, 2015-02-03 at 22:44 +0100, Jörg Krause wrote: > I followed the instructions from Marek in 'M28 U-Boot Single-Wire Debug > preview' to enable NetConsole: > http://www.denx-cs.de/?q=blogm28singlewiredebug > > I'm using mxsldr to flash the u-boot.sb image to RAM. This is the dmesg > output from my host: > > [31048.492181] usb 3-13: new high-speed USB device number 24 > using xhci_hcd > [31048.667617] cdc_ether 3-13:1.0 eth0: register 'cdc_ether' at > usb-:00:14.0-13, CDC Ethernet Device, 00:19:b8:00:00:01 > [31048.669101] cdc_ether 3-13:1.0 enp0s20u13: renamed from eth0 > [31048.696758] cdc_ether 3-13:1.0 enp0s20u13: kevent 12 may have > been dropped > > I noticed the 'kevent 12 may have been dropped' message here. > > Nevertheless, NetworkManager shows me a USB Ethernet connection and > using './netconsole 10.0.0.2' I am able to communicate with the device, > e.g. 'nand info' outputs correctly. > > But if I use 'ping 10.0.0.1' or 'tftpboot u-boot.sb' the network > connection drops. Both commands work fine if I switch back from > netconsole to serial in-/output. > > This is the output from dmesg: > [31620.215354] usb 3-13: USB disconnect, device number 24 > [31620.215422] cdc_ether 3-13:1.0 enp0s20u13: unregister > 'cdc_ether' usb-:00:14.0-13, CDC Ethernet Device > > Do I missed something? I managed to get serial console and netconsole muxing to work, so I can see all the debug messages: Hit any key to stop autoboot: 5 using ci_udc, OUT ep- IN ep- STATUS ep- MAC 00:19:b8:00:00:02 HOST MAC 00:19:b8:00:00:01 high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet USB network up! 0 => tftp u-boot.sb using ci_udc, OUT ep- IN ep- STATUS ep- timeout sending packets to usb ethernet This reminded me about an issue I had some months ago: http://lists.denx.de/pipermail/u-boot/2014-July/182885.html I enabled debug output in arch/arm/cpu/arm926ejs/cache.c and I get error: => tftp u-boot.sb using ci_udc, OUT ep- IN ep- STATUS ep- CACHE: Misaligned operation at range [43f7b010, 43f7b070] I removed the flush_cache() call in cmd_net.c:netboot_common() as suggested by Marek in the thread. But the error message is still there. Any idea? ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] netconsole: USB Ethernet connection dropping with ping or tftpboot
I followed the instructions from Marek in 'M28 U-Boot Single-Wire Debug preview' to enable NetConsole: http://www.denx-cs.de/?q=blogm28singlewiredebug I'm using mxsldr to flash the u-boot.sb image to RAM. This is the dmesg output from my host: [31048.492181] usb 3-13: new high-speed USB device number 24 using xhci_hcd [31048.667617] cdc_ether 3-13:1.0 eth0: register 'cdc_ether' at usb-:00:14.0-13, CDC Ethernet Device, 00:19:b8:00:00:01 [31048.669101] cdc_ether 3-13:1.0 enp0s20u13: renamed from eth0 [31048.696758] cdc_ether 3-13:1.0 enp0s20u13: kevent 12 may have been dropped I noticed the 'kevent 12 may have been dropped' message here. Nevertheless, NetworkManager shows me a USB Ethernet connection and using './netconsole 10.0.0.2' I am able to communicate with the device, e.g. 'nand info' outputs correctly. But if I use 'ping 10.0.0.1' or 'tftpboot u-boot.sb' the network connection drops. Both commands work fine if I switch back from netconsole to serial in-/output. This is the output from dmesg: [31620.215354] usb 3-13: USB disconnect, device number 24 [31620.215422] cdc_ether 3-13:1.0 enp0s20u13: unregister 'cdc_ether' usb-0000:00:14.0-13, CDC Ethernet Device Do I missed something? Best regards Jörg Krause ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/2] mtd:mxs:nand calculate ecc strength dynamically
On Fr, 2014-12-19 at 12:39 +0800, Peng Fan wrote:
> Calculate ecc strength according oobsize, but not hardcoded
> which is not aligned with kernel driver
>
> Signed-off-by: Peng Fan
> Signed-off-by: Ye.Li
> ---
> drivers/mtd/nand/mxs_nand.c | 22 --
> 1 file changed, 4 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/mtd/nand/mxs_nand.c b/drivers/mtd/nand/mxs_nand.c
> index 7a064ab..a45fcf9 100644
> --- a/drivers/mtd/nand/mxs_nand.c
> +++ b/drivers/mtd/nand/mxs_nand.c
> @@ -146,26 +146,12 @@ static uint32_t mxs_nand_aux_status_offset(void)
> static inline uint32_t mxs_nand_get_ecc_strength(uint32_t page_data_size,
> uint32_t page_oob_size)
> {
> - if (page_data_size == 2048) {
> - if (page_oob_size == 64)
> - return 8;
> + int ecc_strength;
>
> - if (page_oob_size == 112)
> - return 14;
> - }
> -
> - if (page_data_size == 4096) {
> - if (page_oob_size == 128)
> - return 8;
> -
> - if (page_oob_size == 218)
> - return 16;
> + ecc_strength = ((page_oob_size - MXS_NAND_METADATA_SIZE) * 8)
> + / (13 * mxs_nand_ecc_chunk_cnt(page_data_size));
>
> - if (page_oob_size == 224)
> - return 16;
> - }
> -
> - return 0;
> + return round_down(ecc_strength, 2);
> }
>
> static inline uint32_t mxs_nand_get_mark_offset(uint32_t page_data_size,
Many thanks for the patch! But this patch affects mxsboot which is no
not aligned with the U-Boot mxs nand driver.
I was able to fix mxsboot, but I had difficulties with round_down, which
is a macro definition in linux/kernel.h. I've copied the macro
definition to mxsboot. I will submit the patch in a seperate mail.
I would like to see a comment or a macro for the magic number 13, which
is the value for the Galois Field, just for clarification
With fixing mxsboot, I was able to test the patch on a custom
i.MX28-based board assembled with a 1Gbit NAND flash (page size = 2048
bytes, oob size = 128 bytes).
U-Boot correctly reads the NAND info
=> nand info
Device 0: nand0, sector size 128 KiB
Page size 2048 b
OOB size128 b
Erase size 131072 b
Before the patch linux failed to read from the UBI device with an ECC
error:
UBI error: ubi_io_read: error -74 (ECC error)
This patch resolves the error. Linux can read the UBI device now. This
is kernel message:
nand: device found, Manufacturer ID: 0x98, Chip ID: 0xf1
[1.327810] nand: Toshiba NAND 128MiB 3,3V 8-bit
[1.332482] nand: 128MiB, SLC, page size: 2048, OOB size: 128
BCH Geometry :
[1.594658] GF length : 13
[1.594658] ECC Strength : 18
[1.594658] Page Size in Bytes : 2176
[1.594658] Metadata Size in Bytes : 10
[1.594658] ECC Chunk Size in Bytes: 512
[1.594658] ECC Chunk Count: 4
[1.594658] Payload Size in Bytes : 2048
[1.594658] Auxiliary Size in Bytes: 16
[1.594658] Auxiliary Status Offset: 12
[1.594658] Block Mark Byte Offset : 1950
[1.594658] Block Mark Bit Offset : 2
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V2] usb: ci_udc: Allocate the qTD list directly
On 07/02/2014 12:51 AM, Stephen Warren wrote: [...] Loading: ## 4.3 MiB/s done Bytes transferred = 18003 (4653 hex) CACHE: Misaligned operation at range [40008000, 4000c653] OK, that particular error happens well after the network transfer phase of the tftp command, so is likely nothing to do with ci_udc. It'd be great if you could track it down and fix it though. Oh, any idea where to look at? Ah, I bet that 40008000 is your load address; the address that the downloaded data is being copied to? You're right, 40008000 is my load address. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V2] usb: ci_udc: Allocate the qTD list directly
On 07/02/2014 12:36 AM, Stephen Warren wrote:
On 07/01/2014 04:34 PM, Jörg Krause wrote:
On 07/01/2014 01:22 PM, Jörg Krause wrote:
On 07/01/2014 01:19 PM, Marek Vasut wrote:
[snip]
Can you edit arch/arm/cpu/arm926ejs/cache.c and change the debug() to
printf() , then re-test please ? I suspect this might trap something
still. Ah, and please test on u-boot-usb/master with this patch.
No additional output on the console.
What does this mean? Do you see warning messages prefixed with
"CACHE: " ?
No messages prefixed with "CACHE: ". Just the usual error message.
I am sorry, but maybe I edited arch/arm/cpu/arm926ejs/ in a another
branch and compiled in u-boot-usb. If I run now tftp with printf enabled
in arch/arm/cpu/arm926ejs/cache.c I get the following "CACHE: " messages:
CACHE: Misaligned operation at range [40008000, 4000c653]
CACHE: Misaligned operation at range [43fd0b0c, 43fd0b60]
That happens right when you first use the UDC driver right? If so, I
hope that "[U-Boot] [PATCH 1/6] usb: ci_udc: fix ci_flush_{qh, qtd}
calls in ci_udc_probe()" will fix that.
Checkout clean u-boot-usb/master, applied board specific patches and
applied the mentioned patch. Running tftp three times in a row:
=> reset
resetting ...
HTLLCLLC
U-Boot 2014.07-rc3-g0b32423-dirty (Jul 02 2014 - 00:44:53)
CPU: Freescale i.MX28 rev1.2 at 454 MHz
BOOT: NAND, 3V3
DRAM: 64 MiB
NAND: 128 MiB
In:serial
Out: serial
Err: serial
Net: usb_ether [PRIME]
Hit any key to stop autoboot: 0
=> tftp imx28-airlino.dtb
using ci_udc, OUT ep- IN ep- STATUS ep-
MAC 00:19:b8:00:00:02
HOST MAC 00:19:b8:00:00:01
high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet
USB network up!
Using usb_ether device
TFTP from server 10.0.0.1; our IP address is 10.0.0.2
Filename 'imx28-airlino.dtb'.
Load address: 0x40008000
Loading: ##
4.3 MiB/s
done
Bytes transferred = 18003 (4653 hex)
CACHE: Misaligned operation at range [40008000, 4000c653]
=>
using ci_udc, OUT ep- IN ep- STATUS ep-
MAC 00:19:b8:00:00:02
HOST MAC 00:19:b8:00:00:01
high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet
USB network up!
Using usb_ether device
TFTP from server 10.0.0.1; our IP address is 10.0.0.2
Filename 'imx28-airlino.dtb'.
Load address: 0x40008000
Loading: ##
1.7 MiB/s
done
Bytes transferred = 18003 (4653 hex)
CACHE: Misaligned operation at range [40008000, 4000c653]
=>
using ci_udc, OUT ep- IN ep- STATUS ep-
MAC 00:19:b8:00:00:02
HOST MAC 00:19:b8:00:00:01
high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet
USB network up!
Using usb_ether device
TFTP from server 10.0.0.1; our IP address is 10.0.0.2
Filename 'imx28-airlino.dtb'.
Load address: 0x40008000
Loading: ##
3.4 MiB/s
done
Bytes transferred = 18003 (4653 hex)
CACHE: Misaligned operation at range [40008000, 4000c653]
=>
using ci_udc, OUT ep- IN ep- STATUS ep-
MAC 00:19:b8:00:00:02
HOST MAC 00:19:b8:00:00:01
high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet
ERROR: The remote end did not respond in time.
at drivers/usb/gadget/ether.c:2388/usb_eth_init()
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 0/6] usb: ci_udc: fixes and cleanups
On 07/01/2014 11:36 PM, Stephen Warren wrote: [snip] Can you please try one more thing: Go back to u-boot-usb/master plus just your board support patches. Apply the following and test that: diff --git a/drivers/usb/gadget/ci_udc.c b/drivers/usb/gadget/ci_udc.c index a6433e8d2d8d..13be1b73d3d1 100644 --- a/drivers/usb/gadget/ci_udc.c +++ b/drivers/usb/gadget/ci_udc.c @@ -209,9 +209,9 @@ ci_ep_alloc_request(struct usb_ep *ep, unsigned int gfp_flags) ci_req = memalign(ARCH_DMA_MINALIGN, sizeof(*ci_req)); if (!ci_req) return NULL; + memset(ci_req, 0, sizeof(*ci_req)); INIT_LIST_HEAD(&ci_req->queue); - ci_req->b_buf = 0; if (num == 0) controller.ep0_req = ci_req; Thanks. Applied and tested with printf in cache.c enabled. ttp runs more than three times in row without a timeout error. => reset resetting ... HTLLCLLC U-Boot 2014.07-rc3-g88eca85-dirty (Jul 02 2014 - 00:39:20) CPU: Freescale i.MX28 rev1.2 at 454 MHz BOOT: NAND, 3V3 DRAM: 64 MiB NAND: 128 MiB In:serial Out: serial Err: serial Net: usb_ether [PRIME] Hit any key to stop autoboot: 0 => tftp imx28-airlino.dtb using ci_udc, OUT ep- IN ep- STATUS ep- MAC 00:19:b8:00:00:02 HOST MAC 00:19:b8:00:00:01 high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet USB network up! Using usb_ether device TFTP from server 10.0.0.1; our IP address is 10.0.0.2 Filename 'imx28-airlino.dtb'. Load address: 0x40008000 Loading: ## 2.1 MiB/s done Bytes transferred = 18003 (4653 hex) CACHE: Misaligned operation at range [40008000, 4000c653] => CACHE: Misaligned operation at range [43fd0b08, 43fd0b60] CACHE: Misaligned operation at range [43fd0b14, 43fd0b60] using ci_udc, OUT ep- IN ep- STATUS ep- MAC 00:19:b8:00:00:02 HOST MAC 00:19:b8:00:00:01 high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet USB network up! Using usb_ether device TFTP from server 10.0.0.1; our IP address is 10.0.0.2 Filename 'imx28-airlino.dtb'. Load address: 0x40008000 Loading: ## 3.4 MiB/s done Bytes transferred = 18003 (4653 hex) CACHE: Misaligned operation at range [40008000, 4000c653] => CACHE: Misaligned operation at range [43fd0b08, 43fd0b60] CACHE: Misaligned operation at range [43fd0b14, 43fd0b60] using ci_udc, OUT ep- IN ep- STATUS ep- MAC 00:19:b8:00:00:02 HOST MAC 00:19:b8:00:00:01 high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet USB network up! Using usb_ether device TFTP from server 10.0.0.1; our IP address is 10.0.0.2 Filename 'imx28-airlino.dtb'. Load address: 0x40008000 Loading: ## 4.3 MiB/s done Bytes transferred = 18003 (4653 hex) CACHE: Misaligned operation at range [40008000, 4000c653] => CACHE: Misaligned operation at range [43fd0b08, 43fd0b60] CACHE: Misaligned operation at range [43fd0b14, 43fd0b60] using ci_udc, OUT ep- IN ep- STATUS ep- MAC 00:19:b8:00:00:02 HOST MAC 00:19:b8:00:00:01 high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet USB network up! Using usb_ether device TFTP from server 10.0.0.1; our IP address is 10.0.0.2 Filename 'imx28-airlino.dtb'. Load address: 0x40008000 Loading: ## 5.7 MiB/s done Bytes transferred = 18003 (4653 hex) CACHE: Misaligned operation at range [40008000, 4000c653] => CACHE: Misaligned operation at range [43fd0b08, 43fd0b60] CACHE: Misaligned operation at range [43fd0b14, 43fd0b60] using ci_udc, OUT ep- IN ep- STATUS ep- MAC 00:19:b8:00:00:02 HOST MAC 00:19:b8:00:00:01 high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet USB network up! Using usb_ether device TFTP from server 10.0.0.1; our IP address is 10.0.0.2 Filename 'imx28-airlino.dtb'. Load address: 0x40008000 Loading: ## 4.3 MiB/s done Bytes transferred = 18003 (4653 hex) CACHE: Misaligned operation at range [40008000, 4000c653] ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V2] usb: ci_udc: Allocate the qTD list directly
On 07/01/2014 01:22 PM, Jörg Krause wrote: On 07/01/2014 01:19 PM, Marek Vasut wrote: [snip] Can you edit arch/arm/cpu/arm926ejs/cache.c and change the debug() to printf() , then re-test please ? I suspect this might trap something still. Ah, and please test on u-boot-usb/master with this patch. No additional output on the console. What does this mean? Do you see warning messages prefixed with "CACHE: " ? No messages prefixed with "CACHE: ". Just the usual error message. I am sorry, but maybe I edited arch/arm/cpu/arm926ejs/ in a another branch and compiled in u-boot-usb. If I run now tftp with printf enabled in arch/arm/cpu/arm926ejs/cache.c I get the following "CACHE: " messages: CACHE: Misaligned operation at range [40008000, 4000c653] CACHE: Misaligned operation at range [43fd0b0c, 43fd0b60] => using ci_udc, OUT ep- IN ep- STATUS ep- MAC 00:19:b8:00:00:02 HOST MAC 00:19:b8:00:00:01 high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet ERROR: The remote end did not respond in time. at drivers/usb/gadget/ether.c:2392/usb_eth_init() Best regards, Marek Vasut ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 0/6] usb: ci_udc: fixes and cleanups
On 07/01/2014 11:31 PM, Stephen Warren wrote:
On 07/01/2014 03:25 PM, Jörg Krause wrote:
On 07/01/2014 07:41 PM, Stephen Warren wrote:
From: Stephen Warren
This is a series of small fixes and cleanups either required by those
fixes, or enabled now that the fixes are made.
I hope that either patch 1 or 4 might fix the issues Jörg is seeing, but
I'm not sure that will happen. The other patches shouldn't change any
behaviour.
Stephen Warren (6):
usb: ci_udc: fix ci_flush_{qh,qtd} calls in ci_udc_probe()
usb: ci_udc: don't assume QTDs are adjacent when transmitting ZLPs
usb: ci_udc: lift ilist size calculations to global scope
usb: ci_udc: fix items array size/stride calculation
usb: ci_udc: remove controller.items array
usb: ci_udc: don't memalign() struct ci_req allocations
drivers/usb/gadget/ci_udc.c | 62
++---
drivers/usb/gadget/ci_udc.h | 1 -
2 files changed, 30 insertions(+), 33 deletions(-)
Good news! The last patch usb: ci_udc: don't memalign() struct ci_req
allocations removes the timeout error after starting the fourth run of
tftp in a row.
This is how I tested. Checked out u-boot-usb/master branch. Applied the
necessary patches to support our board. Applied the patches step after
step. After applying a patch reset the board and run tftp from console
until an error occured, which is always the fourth run.
How many times did you boot the board for each patch? I'm more
interested in how often the first TFTP after a reset/power-on passes or
fails, so it would be nice to boot the board many times to see what
happens to the first TFTP invocation too. This is especially true since
you'd indicated before that the problem was (at least sometimes) visible
on the first TFTP invocation, and this "it fails the fourth time"
symptom is something completely new.
The problem with the problems on the first run was before applying the
patch usb: ci_udc: fix interaction with CONFIG_USB_ETH_CDC, which was
not upstream on u-boot-imx branch at the moment of writing the error
report. After applying the patch and setting the cachline size to 32,
the first three runs of tftp works fine, but always the fourth time it
fails.
This is the case
until applying patch usb: ci_udc: don't memalign() struct ci_req
allocations, which throws no timeout error within running tftp about 60
times in a row.
That's quite odd. That patch definitely should not affect behaviour (and
indeed I only sent it as an after-thought). If it does, then the only
explanation I can think of is that the malloc heap's alignment changed,
which just happens to hide the bug you're seeing. No doubt, there is
still some lingering cache-flushing bug or similar...
BTW, did you fix the U-Boot header files in your board support patches,
so that everything correctly knows that the cache line size is 32? I
think it's mandatory to fix that issue before testing the USB code.
Yes, I did this: #define CONFIG_SYS_CACHELINE_SIZE 32
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 0/6] usb: ci_udc: fixes and cleanups
On 07/01/2014 07:41 PM, Stephen Warren wrote:
From: Stephen Warren
This is a series of small fixes and cleanups either required by those
fixes, or enabled now that the fixes are made.
I hope that either patch 1 or 4 might fix the issues Jörg is seeing, but
I'm not sure that will happen. The other patches shouldn't change any
behaviour.
Stephen Warren (6):
usb: ci_udc: fix ci_flush_{qh,qtd} calls in ci_udc_probe()
usb: ci_udc: don't assume QTDs are adjacent when transmitting ZLPs
usb: ci_udc: lift ilist size calculations to global scope
usb: ci_udc: fix items array size/stride calculation
usb: ci_udc: remove controller.items array
usb: ci_udc: don't memalign() struct ci_req allocations
drivers/usb/gadget/ci_udc.c | 62 ++---
drivers/usb/gadget/ci_udc.h | 1 -
2 files changed, 30 insertions(+), 33 deletions(-)
Good news! The last patch usb: ci_udc: don't memalign() struct ci_req
allocations removes the timeout error after starting the fourth run of
tftp in a row.
This is how I tested. Checked out u-boot-usb/master branch. Applied the
necessary patches to support our board. Applied the patches step after
step. After applying a patch reset the board and run tftp from console
until an error occured, which is always the fourth run. This is the case
until applying patch usb: ci_udc: don't memalign() struct ci_req
allocations, which throws no timeout error within running tftp about 60
times in a row.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V2] usb: ci_udc: Allocate the qTD list directly
On 07/01/2014 01:35 PM, Marek Vasut wrote: On Tuesday, July 01, 2014 at 01:22:41 PM, Jörg Krause wrote: On 07/01/2014 01:19 PM, Marek Vasut wrote: [snip] Can you edit arch/arm/cpu/arm926ejs/cache.c and change the debug() to printf() , then re-test please ? I suspect this might trap something still. Ah, and please test on u-boot-usb/master with this patch. No additional output on the console. What does this mean? Do you see warning messages prefixed with "CACHE: " ? No messages prefixed with "CACHE: ". Just the usual error message. => using ci_udc, OUT ep- IN ep- STATUS ep- MAC 00:19:b8:00:00:02 HOST MAC 00:19:b8:00:00:01 high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet ERROR: The remote end did not respond in time. at drivers/usb/gadget/ether.c:2392/usb_eth_init() Just to make sure, did you remove any CDC ethernet tunables (like cdc_connect_timeout) from your env ? => printenv cdc_connect_timeout ## Error: "cdc_connect_timeout" not defined Also, no other CDC ethernet variables. These are the related env variables from my config header file: #define CONFIG_IPADDR 10.0.0.2 #define CONFIG_SERVERIP 10.0.0.1 #define CONFIG_NETMASK 255.255.255.0 #define CONFIG_EXTRA_ENV_SETTINGS \ "ethact=usb_ether\0" \ "ethprime=usb_ether\0" \ "usbnet_hostaddr=00:19:B8:00:00:01\0" \ "usbnet_devaddr=00:19:B8:00:00:02\0" \ [...] And these are my settings for USB: /* USB */ #ifdefCONFIG_CMD_USB #define CONFIG_EHCI_MXS_PORT0 #define CONFIG_USB_MAX_CONTROLLER_COUNT1 #define CONFIG_CI_UDC/* ChipIdea CI13xxx UDC */ #define CONFIG_USB_REG_BASE0x8008 #define CONFIG_USBD_HS/* High speed support for usb device and usbtty. */ #define CONFIG_USB_GADGET_DUALSPEED #define CONFIG_USB_ETHER #define CONFIG_USB_ETH_CDC #endif Best regards, Marek Vasut ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V2] usb: ci_udc: Allocate the qTD list directly
On 07/01/2014 01:19 PM, Marek Vasut wrote: [snip] Can you edit arch/arm/cpu/arm926ejs/cache.c and change the debug() to printf() , then re-test please ? I suspect this might trap something still. Ah, and please test on u-boot-usb/master with this patch. No additional output on the console. What does this mean? Do you see warning messages prefixed with "CACHE: " ? No messages prefixed with "CACHE: ". Just the usual error message. => using ci_udc, OUT ep- IN ep- STATUS ep- MAC 00:19:b8:00:00:02 HOST MAC 00:19:b8:00:00:01 high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet ERROR: The remote end did not respond in time. at drivers/usb/gadget/ether.c:2392/usb_eth_init() Best regards, Marek Vasut ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V2] usb: ci_udc: Allocate the qTD list directly
On 07/01/2014 12:17 PM, Marek Vasut wrote:
On Tuesday, July 01, 2014 at 08:51:45 AM, Jörg Krause wrote:
On 07/01/2014 02:04 AM, Marek Vasut wrote:
Instead of weird allocation of ci_drv->items_mem and then even weirder
distribution of offsets in this memory area into ci_drv->items array,
just allocate ci_drv->items as a big slab of aligned memory (replaces
ci_drv->items_mem) and let ci_get_qtd() do the distribution of offsets
in this memory area.
Signed-off-by: Marek Vasut
Cc: Jörg Krause
Cc: Stephen Warren
---
drivers/usb/gadget/ci_udc.c | 19 ++-
drivers/usb/gadget/ci_udc.h | 3 +--
2 files changed, 7 insertions(+), 15 deletions(-)
V2: Rebase on top of u-boot-usb/master instead of the research branch
[...]
I made a test on u-boot-arm/master before (Test#1) and after applying
this patch (Test#2).
Can we please sync on the same codebase (u-boot-usb/master) ?
Sorry, this is a type. I tested on u-boot-usb/master.
After a reset I run this script:
test_usb=setenv i 64; while test ${i} -gt 0; do echo ${i}; tftp
${rootfs_file}; setexpr i ${i} - 1; done; setenv i;
Both tests (Test#1 and Test#2) runs fine with the script. But if I do
run tftp ${rootfs_file} manually from the console, I get the known error
starting the fourth run for both Test#1 and Test#2.
Can you edit arch/arm/cpu/arm926ejs/cache.c and change the debug() to printf() ,
then re-test please ? I suspect this might trap something still. Ah, and please
test on u-boot-usb/master with this patch.
No additional output on the console.
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V2] usb: ci_udc: Allocate the qTD list directly
On 07/01/2014 02:04 AM, Marek Vasut wrote:
Instead of weird allocation of ci_drv->items_mem and then even weirder
distribution of offsets in this memory area into ci_drv->items array,
just allocate ci_drv->items as a big slab of aligned memory (replaces
ci_drv->items_mem) and let ci_get_qtd() do the distribution of offsets
in this memory area.
Signed-off-by: Marek Vasut
Cc: Jörg Krause
Cc: Stephen Warren
---
drivers/usb/gadget/ci_udc.c | 19 ++-
drivers/usb/gadget/ci_udc.h | 3 +--
2 files changed, 7 insertions(+), 15 deletions(-)
V2: Rebase on top of u-boot-usb/master instead of the research branch
diff --git a/drivers/usb/gadget/ci_udc.c b/drivers/usb/gadget/ci_udc.c
index 1af6d12..8333db2 100644
--- a/drivers/usb/gadget/ci_udc.c
+++ b/drivers/usb/gadget/ci_udc.c
@@ -130,7 +130,7 @@ static struct ept_queue_head *ci_get_qh(int ep_num, int
dir_in)
*/
static struct ept_queue_item *ci_get_qtd(int ep_num, int dir_in)
{
- return controller.items[(ep_num * 2) + dir_in];
+ return controller.items + ((ep_num * 2) + dir_in);
}
/**
@@ -769,7 +769,6 @@ static int ci_pullup(struct usb_gadget *gadget, int is_on)
static int ci_udc_probe(void)
{
struct ept_queue_head *head;
- uint8_t *imem;
int i;
const int num = 2 * NUM_ENDPOINTS;
@@ -796,12 +795,12 @@ static int ci_udc_probe(void)
* only one of them is used for the endpoint at time, so we can group
* them together.
*/
- controller.items_mem = memalign(ilist_align, ilist_sz);
- if (!controller.items_mem) {
+ controller.items = memalign(ilist_align, ilist_sz);
+ if (!controller.items) {
free(controller.epts);
return -ENOMEM;
}
- memset(controller.items_mem, 0, ilist_sz);
+ memset(controller.items, 0, ilist_sz);
for (i = 0; i < 2 * NUM_ENDPOINTS; i++) {
/*
@@ -821,12 +820,6 @@ static int ci_udc_probe(void)
head->next = TERMINATE;
head->info = 0;
- imem = controller.items_mem + ((i >> 1) * ilist_ent_sz);
- if (i & 1)
- imem += sizeof(struct ept_queue_item);
-
- controller.items[i] = (struct ept_queue_item *)imem;
-
if (i & 1) {
ci_flush_qh(i - 1);
ci_flush_qtd(i - 1);
@@ -855,7 +848,7 @@ static int ci_udc_probe(void)
ci_ep_alloc_request(&controller.ep[0].ep, 0);
if (!controller.ep0_req) {
- free(controller.items_mem);
+ free(controller.items);
free(controller.epts);
return -ENOMEM;
}
@@ -910,7 +903,7 @@ int usb_gadget_unregister_driver(struct usb_gadget_driver
*driver)
controller.driver = NULL;
ci_ep_free_request(&controller.ep[0].ep, &controller.ep0_req->req);
- free(controller.items_mem);
+ free(controller.items);
free(controller.epts);
return 0;
diff --git a/drivers/usb/gadget/ci_udc.h b/drivers/usb/gadget/ci_udc.h
index c214402..3115b15 100644
--- a/drivers/usb/gadget/ci_udc.h
+++ b/drivers/usb/gadget/ci_udc.h
@@ -102,8 +102,7 @@ struct ci_drv {
struct usb_gadget_driver*driver;
struct ehci_ctrl*ctrl;
struct ept_queue_head *epts;
- struct ept_queue_item *items[2 * NUM_ENDPOINTS];
- uint8_t *items_mem;
+ struct ept_queue_item *items;
struct ci_epep[NUM_ENDPOINTS];
};
I made a test on u-boot-arm/master before (Test#1) and after applying
this patch (Test#2). After a reset I run this script:
test_usb=setenv i 64; while test ${i} -gt 0; do echo ${i}; tftp
${rootfs_file}; setexpr i ${i} - 1; done; setenv i;
Both tests (Test#1 and Test#2) runs fine with the script. But if I do
run tftp ${rootfs_file} manually from the console, I get the known error
starting the fourth run for both Test#1 and Test#2.
I attached a log file for the error.
=> reset
resetting ...
HTLLCLLC
U-Boot 2014.07-rc3-g18e0313 (Jul 01 2014 - 08:32:45)
CPU: Freescale i.MX28 rev1.2 at 454 MHz
BOOT: NAND, 3V3
DRAM: 64 MiB
NAND: 128 MiB
In:serial
Out: serial
Err: serial
Net: usb_ether [PRIME]
Hit any key to stop autoboot: 0
=> tftp ${rootfs_file}
using ci_udc, OUT ep- IN ep- STATUS ep-
MAC 00:19:b8:00:00:02
HOST MAC 00:19:b8:00:00:01
high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet
USB network up!
Using usb_ether device
TFTP from server 10.0.0.1; our IP address is 10.0.0.2
F
Re: [U-Boot] [PATCH] usb: ci_udc: fix interaction with CONFIG_USB_ETH_CDC
On 07/01/2014 12:51 AM, Stephen Warren wrote: On 06/30/2014 04:44 PM, Jörg Krause wrote: On 06/30/2014 09:55 PM, Stephen Warren wrote: On 06/30/2014 10:02 AM, Stephen Warren wrote: On 06/27/2014 07:34 PM, Jörg Krause wrote: On 06/28/2014 01:37 AM, Stephen Warren wrote: On 06/27/2014 05:16 PM, Jörg Krause wrote: On 06/27/2014 11:55 PM, Stephen Warren wrote: On 06/27/2014 03:37 PM, Jörg Krause wrote: I added the last series of patches beginning from 2014-06-10 for testing purposes. The patches from 2014-05-29 were already applied. First series of patches: Applying: usb: ci_udc: call udc_disconnect() from ci_pullup() Applying: usb: ci_udc: fix freeing of ep0 req Applying: usb: ci_udc: fix probe error cleanup Applying: usb: ci_udc: clean up all allocations in unregister Calling tftp the first time after a reset runs fine, I thought the issue you reported was that the *first* time you run the "tftp" command, it has issues such as timeouts? Did I misunderstand, or did that issue somehow go away? That's right! This was the state before applying a series of patches after allow multiple buffer allocs per ep. Now, the first run of tftp runs without any errors. Just to make sure I understand, here's what you saw: 1) tftp works fine to start with. No timeouts even on repeated invocation. I went back in time and now I can be more precise. Everything worked fine until commit usb: ci_udc: allow multiple buffer allocs per ep which introduces timeouts in almost all tftp file downloads. Even the first run of tfpt can end in a timeout. I've pushed out some branches for you to test at: git://github.com/swarren/u-boot.git I looked back to see where the problematic commit 2813006fecda "usb: ci_udc: allow multiple buffer allocs per ep" was first applied. I then started with that same commit, and split up the problematic commit into tiny pieces, and applied each piece in turn. The result is in branch ci-udc-debug-base. If you could please test every commit in that branch: fabf0df9523a usb: ci_udc: remainder of "allow multiple buffer allocs" 373098f12a1d usb: ci_udc: dynamically alloc USB request objects 43fff2b11860 usb: ci_udc: dynamically alloc bounce buffer f21b9989bf83 usb: ci_udc: bounce only actual (rounded) packet len not buffer len for small pkts 09ba064f9a99 usb: ci_udc: debounce only actual packet len not buffer len 7e90ae0e4973 usb: ci_udc: use ci_req-> not ep-> a bit more 9663ff89c764 usb: ci_udc: pass ci_req not ep to bounce/debounce b9fdf9dd3f96 usb: ci_udc: move buffer fields from ep to req 82f35a839e31 usb: ci_udc: introduce struct ci_req c9396bc55069 usb: ci_udc: create "req" variable in bounce/unbounce 173d294b94cf Merge branch 'serial' of git://www.denx.de/git/u-boot-microblaze ... and tell me which commit causes the problem, that would be useful. Since the problem is intermittent, please make sure you test each commit many times (at least 10-20, preferably nearer 100). You can stop early if you see the problem, but if you don't, please test many times to make sure there is no problem. Thanks very much for testing. (when you remove the blank lines between the message you're replying to and your response, it's much harder to read. Perhaps this is because you're writing HTML email. Most people won't read that; please use plain text). Done, I hope it is better now. Commit 373098f12a1d usb: ci_udc: dynamically alloc USB request objects cannot be build. Oh dear. Differentiating between that commit and the next one is perhaps the most important thing. Can you please apply the following patch to that commit and retest it: diff --git a/drivers/usb/gadget/ci_udc.c b/drivers/usb/gadget/ci_udc.c index b00fc2613779..c8c64d755195 100644 --- a/drivers/usb/gadget/ci_udc.c +++ b/drivers/usb/gadget/ci_udc.c @@ -423,7 +423,7 @@ static void handle_ep_complete(struct ci_ep *ep) num, in ? "in" : "out", item->info, item->page0); len = (item->info >> 16) & 0x7fff; - ci_ep->current_req = 0; + ep->current_req = 0; ci_req->req.actual = ci_req->req.length - len; ci_debounce(ci_req, in); Applied and tested. This commit produces also an error, but it differs to the previous one. I attached a log. The error is introduced with the latest commit fabf0df9523a usb: ci_udc: remainder of "allow multiple buffer allocs". I attached the output message for this commit. OK, at least we can rule out all the bounce buffer and cache handling changes. Just FYI: I added CONFIG_SYS_CACHE_SIZE 32 to my config header for all tests. => reset resetting ... HTLLCLLC U-Boot 2014.04-g72d5154-dirty (Jul 01 2014 - 00:58:41) CPU: Freescale i.MX28 rev1.2 at 454 MHz BOOT: NAND, 3V3 DRAM: 64 MiB NAND: 128 MiB In:serial Out: serial Err: serial Net: usb_ether [P
Re: [U-Boot] [PATCH] usb: ci_udc: fix interaction with CONFIG_USB_ETH_CDC
On 06/30/2014 09:55 PM, Stephen Warren wrote:
On 06/30/2014 10:02 AM, Stephen Warren wrote:
On 06/27/2014 07:34 PM, Jörg Krause wrote:
On 06/28/2014 01:37 AM, Stephen Warren wrote:
On 06/27/2014 05:16 PM, Jörg Krause wrote:
On 06/27/2014 11:55 PM, Stephen Warren wrote:
On 06/27/2014 03:37 PM, Jörg Krause wrote:
I added the last series of patches beginning from 2014-06-10 for
testing
purposes. The patches from 2014-05-29 were already applied.
First series of patches:
Applying: usb: ci_udc: call udc_disconnect() from ci_pullup()
Applying: usb: ci_udc: fix freeing of ep0 req
Applying: usb: ci_udc: fix probe error cleanup
Applying: usb: ci_udc: clean up all allocations in unregister
Calling tftp the first time after a reset runs fine,
I thought the issue you reported was that the *first* time you run the
"tftp" command, it has issues such as timeouts? Did I misunderstand, or
did that issue somehow go away?
That's right! This was the state before applying a series of patches
after allow multiple buffer allocs per ep. Now, the first run of tftp
runs without any errors.
Just to make sure I understand, here's what you saw:
1) tftp works fine to start with. No timeouts even on repeated
invocation.
I went back in time and now I can be more precise. Everything worked
fine until commit usb: ci_udc: allow multiple buffer allocs per ep which
introduces timeouts in almost all tftp file downloads. Even the first
run of tfpt can end in a timeout.
I've pushed out some branches for you to test at:
git://github.com/swarren/u-boot.git
I looked back to see where the problematic commit 2813006fecda "usb:
ci_udc: allow multiple buffer allocs per ep" was first applied. I then
started with that same commit, and split up the problematic commit into
tiny pieces, and applied each piece in turn. The result is in branch
ci-udc-debug-base. If you could please test every commit in that branch:
fabf0df9523a usb: ci_udc: remainder of "allow multiple buffer allocs"
373098f12a1d usb: ci_udc: dynamically alloc USB request objects
43fff2b11860 usb: ci_udc: dynamically alloc bounce buffer
f21b9989bf83 usb: ci_udc: bounce only actual (rounded) packet len not buffer
len for small pkts
09ba064f9a99 usb: ci_udc: debounce only actual packet len not buffer len
7e90ae0e4973 usb: ci_udc: use ci_req-> not ep-> a bit more
9663ff89c764 usb: ci_udc: pass ci_req not ep to bounce/debounce
b9fdf9dd3f96 usb: ci_udc: move buffer fields from ep to req
82f35a839e31 usb: ci_udc: introduce struct ci_req
c9396bc55069 usb: ci_udc: create "req" variable in bounce/unbounce
173d294b94cf Merge branch 'serial' of git://www.denx.de/git/u-boot-microblaze
... and tell me which commit causes the problem, that would be useful.
Since the problem is intermittent, please make sure you test each commit
many times (at least 10-20, preferably nearer 100). You can stop early
if you see the problem, but if you don't, please test many times to make
sure there is no problem.
Commit 373098f12a1d usb: ci_udc: dynamically alloc USB request objects
cannot be build.
The error is introduced with the latest commit fabf0df9523a usb: ci_udc:
remainder of "allow multiple buffer allocs". I attached the output
message for this commit.
You mentioned that your board support is not upstream. In order to test,
you should probably do: For each commit I mention above, check it out,
apply the minimal set of patches needed to support your board, and test.
Done. Just added board support.
Please don't apply any other patches while testing this series. Please
only test TFTP download, and not ubifs writes, etc.
This is my test script:
"test_usb=" \
"setenv i 64; " \
"while test ${i} -gt 0; do " \
"echo ${i}; " \
"tftp ${rootfs_file}; " \
"setexpr i ${i} - 1; " \
"done; " \
"setenv i; " \
"\0"
I've also pushed out branch ci-udc-debug-tegra-dfu-working which has a
whole load of other commits I needed to test the split up patches. These
were needed to add board support for the board I'm currently using, and
to fix/enhance the core DFU code. Most of these commits are already
applied in u-boot.git/master, it's just that I cherry-picked them on top
of the old base commit so I would have something to test with.
BTW, I'm going on vacation from Jul 3rd-July 20th. I might answer some
limited email during this time, but I certainly won't have access to any
test HW.
=> reset
resetting ...
HTLLCLLC
U-Boot 2014.04-gfabf0df-dirty (Jul 01 2014 - 00:36:11)
CPU: Freescale i.MX28 rev1.2 at 454 MHz
BOOT: NAND, 3V3
DRAM: 64 MiB
NAND: 128 MiB
In:serial
Out: serial
Err: serial
Net: usb_ether [PRIME]
Warning: Your board does not use generic board. Please read
doc/README.gene
Re: [U-Boot] [PATCH] usb: ci_udc: fix interaction with CONFIG_USB_ETH_CDC
On 06/30/2014 11:15 PM, Marek Vasut wrote: On Monday, June 30, 2014 at 10:55:37 PM, Jörg Krause wrote: [...] 2) You applied "allow multiple buffer allocs per ep" Setting #define CONFIG_SYS_CACHELINE_SIZE 32 to my config file helped here. But still timeouts. First run almost always runs fine, only sometimes timeouts while receiving a packet, but always running to the end. Running tftp after this a second time and more fails with a ERROR: The remote end did not respond in time. at drivers/usb/gadget/ether.c:2388/usb_eth_init(), but sometimes it works. Setting CONFIG_SYS_CACHELINE_SIZE 32 does not make it better (as I previously wrote it). Sorry, this is a typo. It should be CONFIG_SYS_CACHELINE_SIZE 16 (not 32). MX28 has 32b-long cachelines. Setting this to 16 is nonsense. [...] Yes it is. But if I do not set cacheline size to 32 in my config header file it will be 64, as I stated in a previous mail. I checked this and I found that ARCH_DMA_MINALIGN is set to 64, which is not true for Freescale i.MX28. This processor has an ARM926EJ-S, which has an cache line size of 32. In arch/arm/include/asm/cache.h the macro ARCH_DMA_MINALIGN is defined as followed: #ifdef CONFIG_SYS_CACHELINE_SIZE #define ARCH_DMA_MINALIGNCONFIG_SYS_CACHELINE_SIZE #else #define ARCH_DMA_MINALIGN64 #endif And in /arch/arm/cpu/arm926ejs/cache.c as followed: #ifndef CONFIG_SYS_CACHELINE_SIZE #define CONFIG_SYS_CACHELINE_SIZE32 #endif arch/arm/include/asm/cache.h does not see the definition of CONFIG_SYS_CACHELINE_SIZE in /arch/arm/cpu/arm926ejs/cache.c, so it's a bad place to put it in there. I was just curious of the impact on the behaviour of the USB ethernet gadget driver. Best regards, Marek Vasut ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] usb: ci_udc: fix interaction with CONFIG_USB_ETH_CDC
On 06/30/2014 06:02 PM, Stephen Warren wrote: On 06/27/2014 07:34 PM, Jörg Krause wrote: On 06/28/2014 01:37 AM, Stephen Warren wrote: On 06/27/2014 05:16 PM, Jörg Krause wrote: On 06/27/2014 11:55 PM, Stephen Warren wrote: On 06/27/2014 03:37 PM, Jörg Krause wrote: I added the last series of patches beginning from 2014-06-10 for testing purposes. The patches from 2014-05-29 were already applied. First series of patches: Applying: usb: ci_udc: call udc_disconnect() from ci_pullup() Applying: usb: ci_udc: fix freeing of ep0 req Applying: usb: ci_udc: fix probe error cleanup Applying: usb: ci_udc: clean up all allocations in unregister Calling tftp the first time after a reset runs fine, I thought the issue you reported was that the *first* time you run the "tftp" command, it has issues such as timeouts? Did I misunderstand, or did that issue somehow go away? That's right! This was the state before applying a series of patches after allow multiple buffer allocs per ep. Now, the first run of tftp runs without any errors. Just to make sure I understand, here's what you saw: 1) tftp works fine to start with. No timeouts even on repeated invocation. I went back in time and now I can be more precise. Everything worked fine until commit usb: ci_udc: allow multiple buffer allocs per ep which introduces timeouts in almost all tftp file downloads. Even the first run of tfpt can end in a timeout. 2) You applied "allow multiple buffer allocs per ep" Setting #define CONFIG_SYS_CACHELINE_SIZE 32 to my config file helped here. But still timeouts. First run almost always runs fine, only sometimes timeouts while receiving a packet, but always running to the end. Running tftp after this a second time and more fails with a ERROR: The remote end did not respond in time. at drivers/usb/gadget/ether.c:2388/usb_eth_init(), but sometimes it works. Setting CONFIG_SYS_CACHELINE_SIZE 32 does not make it better (as I previously wrote it). Sorry, this is a typo. It should be CONFIG_SYS_CACHELINE_SIZE 16 (not 32). Removing CONFIG_USB_GADGET_DUALSPEED helps a little bit, but I am getting also errors after the second or third run. Sigh. There's been a lot of flip-flopping re: whether the cacheline size affects the issue or not, and whether the first TFTP download always works fine, or whether only the 3rd fails. This makes it very hard for me to understand the issue that you're seeing. For instance, if even the first TFTP download can fail (even if intermittently), then there's clearly a problem with basic driver operation. However, if only the 2nd or 3rd TFTP ever fails, then the problem is likely isolated to some part of the cleanup/shutdown code. Given that your reports of the problem keep flip-flopping about, it makes it hard to decide which part of the code to look at. I am very sorry if I confused you with my attempt to explain you what I am seeing. The "flip-flopping" comes from the different results and behaviour after applying a patch or a series of patches. And I also tried this and that and looked if it changes the behaviour. I must admit that this is not good testing practise. For now, I'm going to simply assume that any TFTP download (1st, 2nd, or 100th) can fail intermittently, and that cache line size is irrelevant. I'll look at the problematic commit you mentioned (2813006fecda "usb: ci_udc: allow multiple buffer allocs per ep") and see if I can create a series of patches that do the same thing, and have you bisect that. If I can do that, I will ask you to test 100 times each: the commit before the bad commit, then each of the commits in my series, always resetting the board between each test and doing a single TFTP download. Then I'd like to see the raw results from those tests. I will do this and I hope it brings some clarifications. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] usb: ci_udc: fix interaction with CONFIG_USB_ETH_CDC
On 06/30/2014 11:37 AM, Marek Vasut wrote:
On Sunday, June 29, 2014 at 10:33:26 PM, Jörg Krause wrote:
On 06/28/2014 10:53 PM, Jörg Krause wrote:
[snip]
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
I did some tests this weekend on u-boot-usb/master branch.
If I run "env default -a" and then "saveenv" after a reset, I get the
same error as running three time "tftp file" in a row.
Log:
U-Boot 2014.07-rc3-g18e0313-dirty (Jun 29 2014 - 21:56:02)
CPU: Freescale i.MX28 rev1.2 at 454 MHz
BOOT: NAND, 3V3
DRAM: 64 MiB
NAND: 128 MiB
In:serial
Out: serial
Err: serial
Net: usb_ether [PRIME]
Hit any key to stop autoboot: 0
=> env default -a
## Resetting to default environment
=> saveenv
Saving Environment to NAND...
Erasing NAND...
Erasing at 0x36 -- 100% complete.
Writing to NAND... OK
=> tftp rootfs.ubifs
using ci_udc, OUT ep- IN ep- STATUS ep-
MAC 00:19:b8:00:00:02
HOST MAC 00:19:b8:00:00:01
high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet
ERROR: The remote end did not respond in time.
at drivers/usb/gadget/ether.c:2388/usb_eth_init()
"env default -a" removes stdin, stdout, stderr, and ver from the output
of "printenv".
Looking at drivers/usb/gadget/ether.c:usb_eth_init I found the
environment variable "cdc_connect_timeout". I played a little bit with
the settings.
1) Using "setenv cdc_connect_timeout 1" from the command line: tftp runs
more then three time in a row. Actually I can run tftp more than ten
times in row and it produces no error. I tested the values 1, 3, and 15
for cdc_connect_timeout.
2) Setting #define CONFIG_EXTRA_ENV_SETTINGS "cdc_connect_timeout=1\0" \
in my config header file. This does not help and produces the error on
the fourth run of tfpd. Tested with values 1 and 3 for timeout.
I just tested the CDC ethernet on M28EVK with u-boot-usb/master and loading
64MiB file from a TFTP server running on a local machine. It seems that for some
reason, in the udc_gadget_handle_interrupts() or somewhere there, it starts not
getting interrupts. Can you try with this change:
diff --git a/drivers/usb/gadget/ci_udc.c b/drivers/usb/gadget/ci_udc.c
index a6433e8..1af6d12 100644
--- a/drivers/usb/gadget/ci_udc.c
+++ b/drivers/usb/gadget/ci_udc.c
@@ -727,14 +727,8 @@ void udc_irq(void)
int usb_gadget_handle_interrupts(void)
{
- u32 value;
- struct ci_udc *udc = (struct ci_udc *)controller.ctrl->hcor;
-
- value = readl(&udc->usbsts);
- if (value)
- udc_irq();
-
- return value;
+ udc_irq();
+ return 0;
}
void udc_disconnect(void)
Does not help, sorry.
Best regards,
Marek Vasut
I run the test with a smaller file of around 18 KB and DEBUG messages
enabled in ci_udc.c. I attached the output for the first run of tftp
imx28-airlino.dtb and the fourth rund of tftp imx28-airlino.dtb, which
fails with an error. Maybe this helps.
using ci_udc, OUT ep- IN ep- STATUS ep-
MAC 00:19:b8:00:00:02
HOST MAC 00:19:b8:00:00:01
-- suspend --
-- reset --
-- portchange 2 High
handle setup GET_DESCRIPTOR, 80, 6 index 0 value 100 length 40
handle_setup: Set ep0 to IN for Data Stage
ept0 in pre-queue req 43b844c0, buffer 43b84580
ept0 in queue len 12, req 43b844c0, buffer 43b84580
ept0 in req 43b844c0, complete 0
handle_ep_complete: flip ep0 dir for Status Stage
flip_ep0_direction: Flipping ep0 to OUT
ept0 out pre-queue req 43b844c0, buffer 43febd20
ept0 out queue len 0, req 43b844c0, buffer 43febd20
ept0 out req 43b844c0, complete 0
-- reset --
-- portchange 2 High
handle setup SET_ADDRESS, 0, 5 index 0 value 1c length 0
handle_setup: Set ep0 to OUT for Data Stage
handle_setup: 0 length: flip ep0 dir for Status Stage
flip_ep0_direction: Flipping ep0 to IN
ept0 in pre-queue req 43b844c0, buffer 43febd20
ept0 in queue len 0, req 43b844c0, buffer 43febd20
ept0 in req 43b844c0, complete 0
handle setup GET_DESCRIPTOR, 80, 6 index 0 value 100 length 12
handle_setup: Set ep0 to IN for Data Stage
ept0 in pre-queue req 43b844c0, buffer 43b84580
ept0 in queue len 12, req 43b844c0, buffer 43b84580
ept0 in req 43b844c0, complete 0
handle_ep_complete: flip ep0 dir for Status Stage
flip_ep0_direction: Flipping ep0 to OUT
ept0 out pre-queue req 43b844c0, buffer 43febd20
ept0 out queue len 0, req 43b844c0, buffer 43febd20
ept0 out req 43b844c0, complete 0
handle setup GET_DESCRIPTOR, 80, 6 index 0 value 200 length 9
handle_setup: Set ep0 to IN for Data Stage
ept0 in pre-queue req 43b844c0, buffer 43b84580
ept0 in queue len 9, req 43b844c0, buffer 43b84580
ept0 in req 43b844c0, complete 0
handle_ep_complete: flip ep0 dir for Status Stage
flip_ep0_direction: Flipping ep0 to OUT
ept0 out pre-queue req 43b844c0, buffer 43febd20
Re: [U-Boot] [PATCH] usb: ci_udc: fix interaction with CONFIG_USB_ETH_CDC
On 06/28/2014 10:53 PM, Jörg Krause wrote: [snip] ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot I did some tests this weekend on u-boot-usb/master branch. If I run "env default -a" and then "saveenv" after a reset, I get the same error as running three time "tftp file" in a row. Log: U-Boot 2014.07-rc3-g18e0313-dirty (Jun 29 2014 - 21:56:02) CPU: Freescale i.MX28 rev1.2 at 454 MHz BOOT: NAND, 3V3 DRAM: 64 MiB NAND: 128 MiB In:serial Out: serial Err: serial Net: usb_ether [PRIME] Hit any key to stop autoboot: 0 => env default -a ## Resetting to default environment => saveenv Saving Environment to NAND... Erasing NAND... Erasing at 0x36 -- 100% complete. Writing to NAND... OK => tftp rootfs.ubifs using ci_udc, OUT ep- IN ep- STATUS ep- MAC 00:19:b8:00:00:02 HOST MAC 00:19:b8:00:00:01 high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet ERROR: The remote end did not respond in time. at drivers/usb/gadget/ether.c:2388/usb_eth_init() "env default -a" removes stdin, stdout, stderr, and ver from the output of "printenv". Looking at drivers/usb/gadget/ether.c:usb_eth_init I found the environment variable "cdc_connect_timeout". I played a little bit with the settings. 1) Using "setenv cdc_connect_timeout 1" from the command line: tftp runs more then three time in a row. Actually I can run tftp more than ten times in row and it produces no error. I tested the values 1, 3, and 15 for cdc_connect_timeout. 2) Setting #define CONFIG_EXTRA_ENV_SETTINGS "cdc_connect_timeout=1\0" \ in my config header file. This does not help and produces the error on the fourth run of tfpd. Tested with values 1 and 3 for timeout. Very, very strange. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] usb: ci_udc: fix interaction with CONFIG_USB_ETH_CDC
On 06/28/2014 10:45 PM, Marek Vasut wrote: On Saturday, June 28, 2014 at 10:37:35 PM, Jörg Krause wrote: On 06/28/2014 09:59 PM, Marek Vasut wrote: I would actually be really glad if you could test plain u-boot-usb/master and see if your USB problem is present there. Also, it would be really nice to isolate the sequence of commands which triggers the bug, so we have a proper reproducible testcase. Anyway, this thread is not a place where I would like to discuss this. This should be discussed in the "original" thread, that is: Subj: [PATCH] usb: ci_udc: fix interaction with CONFIG_USB_ETH_CDC I cloned u-boot-usb/master and tested it. I am running "tftpd rootfs.ubifs" several times. Here is the log (I shortened the loading process bar): [...] => tftp rootfs.ubifs using ci_udc, OUT ep- IN ep- STATUS ep- MAC 00:19:b8:00:00:02 HOST MAC 00:19:b8:00:00:01 high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet USB network up! Using usb_ether device TFTP from server 10.0.0.1; our IP address is 10.0.0.2 Filename 'rootfs.ubifs'. Load address: 0x40008000 Loading: [snip] ##T ### [snip] 1.7 MiB/s Here it's 1.7MB/s ... Sometimes I get a timeout "#T " while loading, which slows down the transfer rate. It is not always the case. done Bytes transferred = 13205504 (c98000 hex) => tftp rootfs.ubifs using ci_udc, OUT ep- IN ep- STATUS ep- MAC 00:19:b8:00:00:02 HOST MAC 00:19:b8:00:00:01 high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet USB network up! Using usb_ether device TFTP from server 10.0.0.1; our IP address is 10.0.0.2 Filename 'rootfs.ubifs'. Load address: 0x40008000 Loading: # [snip] 5.6 MiB/s ... and here it's 5.6 MB/s ... that's weird. [...] The error always appears after calling tftp the third time! The first two runs work fine, but the third one always fails. I run this test about 20 times. And finally, what is the contents of 'rootfs.ubifs' ? It's an rootf ubifs filesystem. I also tested other files. One small u-boot.sb file with a filesize of 400 KB and a 62 MB compressed ubuntu core file (ubuntu-core-14.04-core.ext4.gz). For all I get the same result. Best regards, ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] usb: ci_udc: fix interaction with CONFIG_USB_ETH_CDC
On 06/28/2014 09:59 PM, Marek Vasut wrote: I would actually be really glad if you could test plain u-boot-usb/master and see if your USB problem is present there. Also, it would be really nice to isolate the sequence of commands which triggers the bug, so we have a proper reproducible testcase. Anyway, this thread is not a place where I would like to discuss this. This should be discussed in the "original" thread, that is: Subj: [PATCH] usb: ci_udc: fix interaction with CONFIG_USB_ETH_CDC I cloned u-boot-usb/master and tested it. I am running "tftpd rootfs.ubifs" several times. Here is the log (I shortened the loading process bar): HTLLCLLC U-Boot 2014.07-rc3-g18e0313 (Jun 28 2014 - 22:17:52) CPU: Freescale i.MX28 rev1.2 at 454 MHz BOOT: NAND, 3V3 DRAM: 64 MiB NAND: 128 MiB In:serial Out: serial Err: serial Net: usb_ether [PRIME] Hit any key to stop autoboot: 0 => tftp rootfs.ubifs using ci_udc, OUT ep- IN ep- STATUS ep- MAC 00:19:b8:00:00:02 HOST MAC 00:19:b8:00:00:01 high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet USB network up! Using usb_ether device TFTP from server 10.0.0.1; our IP address is 10.0.0.2 Filename 'rootfs.ubifs'. Load address: 0x40008000 Loading: [snip] ##T ### [snip] 1.7 MiB/s done Bytes transferred = 13205504 (c98000 hex) => tftp rootfs.ubifs using ci_udc, OUT ep- IN ep- STATUS ep- MAC 00:19:b8:00:00:02 HOST MAC 00:19:b8:00:00:01 high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet USB network up! Using usb_ether device TFTP from server 10.0.0.1; our IP address is 10.0.0.2 Filename 'rootfs.ubifs'. Load address: 0x40008000 Loading: # [snip] 5.6 MiB/s done Bytes transferred = 13205504 (c98000 hex) => tftp rootfs.ubifs using ci_udc, OUT ep- IN ep- STATUS ep- MAC 00:19:b8:00:00:02 HOST MAC 00:19:b8:00:00:01 high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet USB network up! Using usb_ether device TFTP from server 10.0.0.1; our IP address is 10.0.0.2 Filename 'rootfs.ubifs'. Load address: 0x40008000 Loading: # [snip] 5.6 MiB/s done Bytes transferred = 13205504 (c98000 hex) => tftp rootfs.ubifs using ci_udc, OUT ep- IN ep- STATUS ep- MAC 00:19:b8:00:00:02 HOST MAC 00:19:b8:00:00:01 high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet ERROR: The remote end did not respond in time. at drivers/usb/gadget/ether.c:2388/usb_eth_init() => The error always appears after calling tftp the third time! The first two runs work fine, but the third one always fails. I run this test about 20 times. Best regards Jörg Krause ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v5 0/5] mtd, ubi, ubifs: resync with Linux-3.14
Dear Wolfgang, On 06/28/2014 01:44 PM, Wolfgang Denk wrote: Dear J�rg, In message <53adfff5.7050...@posteo.de> you wrote: I applied this series of patches on top of a patch from Stephen Warren from 2014-06-23: [PATCH] usb: ci_udc: fix interaction with CONFIG_USB_ETH_CDC. Now my USB Ethernet driver does not work anymore. I am using tfpd for downloading files over USB Ethernet Gadget from my notebook to an Freescale i.MX28 board. It is definitely not wise to testseveral independent changes at once, as you will never know which of them (or any interacton between these) caused the problem. I understand. Since commit 2813006fecdab740c3745b2dcbb06777cdd51dff on u-boot-imx branch the USB Ethernet driver on my custom i.MX28 board is broken. I cannot use tftp for downloading a file. The patch from Stephen Warren fixed most (but not all) problems with the driver. I strongly recommend to apply only one of these patches iitially, then test it, and only if it works, then apply the second patch (series) Stephen Warren, Marek Vasut and I am trying to figure out, why USB Ethernet driver is broken since the stated commit. One assumption of Stephen Warren was that is has something to do with mtd or ubifs. That's why I am reporting this. This is the error message: => run update_rootfs Updating rootfs ... using ci_udc, OUT ep- IN ep- STATUS ep- high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet ERROR: The remote end did not respond in time. at drivers/usb/gadget/ether.c:2383/usb_eth_init() It would have been helpful to show the actual commands here, instead of referring to "update_rootfs" the definition of which is unknown to us. In any case, this looks like a USB gadget driver related issue, which is independent from Heiko's changes. So I think you wpuld have seen the same problem when testing Stephen's patch alone. You should do that, and eventually report the problem against the "usb: ci_udc: fix interaction..." patch. That's right. I already reported the problem to Stephen Warren. The mtd, ubi, ubifs update appears to be innocent here. Seems so. Maybe I will revert the patches from Stephen Warren to the commit where my USB Ethernet driver worked well and apply the mtd, ubi, ubifs patches for testing. Best regards, Wolfgang Denk Best regards Jörg Krause ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] usb: ci_udc: fix interaction with CONFIG_USB_ETH_CDC
On 06/28/2014 01:37 AM, Stephen Warren wrote:
On 06/27/2014 05:16 PM, Jörg Krause wrote:
On 06/27/2014 11:55 PM, Stephen Warren wrote:
On 06/27/2014 03:37 PM, Jörg Krause wrote:
I added the last series of patches beginning from 2014-06-10 for testing
purposes. The patches from 2014-05-29 were already applied.
First series of patches:
Applying: usb: ci_udc: call udc_disconnect() from ci_pullup()
Applying: usb: ci_udc: fix freeing of ep0 req
Applying: usb: ci_udc: fix probe error cleanup
Applying: usb: ci_udc: clean up all allocations in unregister
Calling tftp the first time after a reset runs fine,
I thought the issue you reported was that the *first* time you run the
"tftp" command, it has issues such as timeouts? Did I misunderstand, or
did that issue somehow go away?
That's right! This was the state before applying a series of patches
after allow multiple buffer allocs per ep. Now, the first run of tftp
runs without any errors.
Just to make sure I understand, here's what you saw:
1) tftp works fine to start with. No timeouts even on repeated invocation.
I went back in time and now I can be more precise. Everything worked
fine until commit usb: ci_udc: allow multiple buffer allocs per ep which
introduces timeouts in almost all tftp file downloads. Even the first
run of tfpt can end in a timeout.
2) You applied "allow multiple buffer allocs per ep"
Setting #define CONFIG_SYS_CACHELINE_SIZE 32 to my config file helped
here. But still timeouts. First run almost always runs fine, only
sometimes timeouts while receiving a packet, but always running to the
end. Running tftp after this a second time and more fails with a ERROR:
The remote end did not respond in time. at
drivers/usb/gadget/ether.c:2388/usb_eth_init(), but sometimes it works.
Setting CONFIG_SYS_CACHELINE_SIZE 32 does not make it better (as I
previously wrote it).
Removing CONFIG_USB_GADGET_DUALSPEED helps a little bit, but I am
getting also errors after the second or third run.
3) Now, you see tftp timeouts.
4) You applied "a series of patches *after* allow multiple buffer allocs
per ep"
Applying: usb: ci_udc: parse QTD before over-writing it
Applying: usb: ci_udc: detect queued requests on ep0
Applying: usb: ci_udc: use a single descriptor for ep0
Applying: usb: ci_udc: pre-allocate ep0 req
Applying: usb: ci_udc: complete ep0 direction handling
Applying: usb: ci_udc: call udc_disconnect() from ci_pullup()
Applying: usb: ci_udc: fix freeing of ep0 req
Applying: usb: ci_udc: fix probe error cleanup
Applying: usb: ci_udc: clean up all allocations in unregister
Applying: usb: ci_udc: terminate ep0 INs with a zlp when required
Applying: usb: ci_udc: fix interaction with CONFIG_USB_ETH_CDC
Applying: usb: ci_udc: fix typo in debug message
5) Now, the first tftp command works fine, but repeated invocations fail
(intermittently).
Now tftp runs fine the first time (and sometimes more often) after a
reboot, but after some tftp runs I am getting the ERROR: The remote end
did not respond in time. at
drivers/usb/gadget/ether.c:2388/usb_eth_init(), as mentioned above.
And in (4) above the patch you applied that solved the problem was
"Applying: usb: ci_udc: fix interaction with CONFIG_USB_ETH_CDC"?
But there is still a problem:
I have to wait some seconds before I can run a second time tftp. This is
the output from U-Boot:
=> run update_rootfs
Updating rootfs ...
using ci_udc, OUT ep- IN ep- STATUS ep-
high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet
USB network up!
Using usb_ether device
[snip]
=> run update_rootfs
Updating rootfs ...
using ci_udc, OUT ep- IN ep- STATUS ep-
high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet
ERROR: The remote end did not respond in time.
at drivers/usb/gadget/ether.c:2388/usb_eth_init()
Wait some seconds ...
=> run update_rootfs
Updating rootfs ...
using ci_udc, OUT ep- IN ep- STATUS ep-
high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet
USB network up!
Using usb_ether device
[snip]
Hmm. That's odd. I didn't notice that, but I'll try retesting sometime.
What exactly does $update_rootfs contain? It might be useful to know
some details of your network topology (e.g. is the TFTP server on the
machine that the USB cable is plugged into or further away, and are the
machine and network lightly loaded) and rough sizes of the files you're
downloading.
This is what update_rootfs is doing:
"update_rootfs=echo Updating rootfs ...; " \
"if tftp ${rootfs_file}; then " \
"mtdparts default; " \
"nand erase.part rootfs; " \
"ubi part rootfs; " \
"ubi create rootfs; " \
"ubi write ${loadaddr} rootfs ${filesize}; " \
