Re: [U-Boot] [PATCH 1/4] [v2] nand_base: trivial: fix comment read/write comment
Hi Ben, Replace an incorrect 'read' with 'write' in a comment. Signed-off-by: Ben Gardiner bengardi...@nanometrics.ca Acked-by: Detlev Zundel d...@denx.de -- Whenever you find yourself on the side of the majority it is time to pause and reflect. -- Mark Twain -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/4] [v2] nand_util: convert nand_write_skip_bad() to flags
Hi Ben, In a future commit the behaviour of nand_write_skip_bad() will be further extended. Convert the only flag currently passed to the nand_write_ skip_bad() function to a bitfield of only one allocated member. This should avoid an explosion of int's at the end of the parameter list or the ambiguous calls like nand_write_skip_bad(info, offset, len, buf, 0, 1, 1); nand_write_skip_bad(info, offset, len, buf, 0, 1, 0); Instead there will be: nand_write_skip_bad(info, offset, len, buf, WITH_YAFFS_OOB | WITH_OTHER); Signed-off-by: Ben Gardiner bengardi...@nanometrics.ca CC: Detlev Zundel d...@denx.de --- Changes since v1: * rebased to HEAD of git://git.denx.de/u-boot-nand-flash.git : ff7b4a0 (env_nand: zero-initialize variable nand_erase_options) * renamed the flag from WITH_OOB to WITH_YAFFS_OOB (Detlev Zundel) * introduce 'WITH_DEFAULTS' flag defined as 0 so as to convert also the remaining nand_write_skip_bad() call (Detlev Zundel) I don't remember requesting this change - and rereading my mails I cannot find anything either. Actually I consider this define to be overkill. What I was asking for was to change all occurrences of the function call - a 0 for flags is ok with me. If you remove that again, you can add my Acked-by: Detlev Zundel d...@denx.de Cheers Detlev -- I've been examining the existing [linux] kernel configuration system, and I have about concluded that the best favor we could do everybody involved with it is to take it out behind the barn and shoot it through the head. -- Eric S. Raymond on linux-kbuild Mar 2000 -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 4/4] [v2] cmd_nand: add nand write.trimffs command
Hi Ben, Add another nand write. variant, trimffs. This command will request of nand_write_skip_bad() that all trailing all-0xff pages will be dropped from eraseblocks when they are written to flash as-per the reccommended behaviour of the UBI FAQ [1]. The function that implements this timming is the drop_ffs() function by Artem Bityutskiy, ported from the mtd-utils tree. [1] http://www.linux-mtd.infradead.org/doc/ubi.html#L_flasher_algo Signed-off-by: Ben Gardiner bengardi...@nanometrics.ca CC: Artem Bityutskiy dedeki...@gmail.com CC: Detlev Zundel d...@denx.de --- Detlev, I did not do any erased-state checks since the implicit assumption in other nand commands is that the user is required to do their own erasing beforehand. Yes, I have come to the same conclusion. I have another request though - please add the new command to doc/README.nand. Thanks! Cheers Detlev -- Modern technique has made it possible for leisure, within limits, to be not the prerogative of small privileged classes, but a right evenly distributed throughout the community. The morality of work is the morality of slaves, and the modern world has no need of slavery.-- Bertrand Russell -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/4] [v2] nand_util: drop trailing all-0xff pages if requested
Hi Ben, Add a flag to nand_read_skip_bad() such that if true, any trailing pages in an eraseblock whose contents are entirely 0xff will be dropped. The implementation is via a new drop_ffs() function which is based on the function of the same name from the ubiformat utility by Artem Bityutskiy. This is as-per the reccomendations of the UBI FAQ [1] Signed-off-by: Ben Gardiner bengardi...@nanometrics.ca CC: Artem Bityutskiy dedeki...@gmail.com Acked-by: Detlev Zundel d...@denx.de Cheers Detlev -- Oh, didn't you know, the Lord did the original programming of the universe in COBOL. - That's why the world is the evil work of Satan. A true divine being would have used Scheme. - And, if so, Jesus would have been crucified on a big lambda symbol. -- K. Chafin, K. Schilling D. Hanley, on comp.lang.lisp -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 0/4] Accurate boot time measurement
Hi Simon, [...] I believe I have covered this ground very thoroughly and would like advice please on what to do next. The options I can see are: As Graeme points out, you got enough positive feedback that I encourage you to continue and address the comments. OK, it would be nice to have a note from Wolfgang since without his OK it won't make it in, right? Ultimately we are a democratic community, so when you get enough support the changes will go in. Usually we find a consensus acceptable to all parties before resorting to such formal measures however. Cheers Detlev -- Q: Why did the chicken cross the Moebius strip? A: To get to the other ... er, um ... -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 1/5] TFTP: replace server with remote in local variable names
Hi Luca, With the upcoming TFTP server implementation, the remote node can be either a client or a server, so avoid ambiguities. Signed-off-by: Luca Ceresoli luca.ceres...@comelit.it Cc: Wolfgang Denk w...@denx.de Acked-by: Detlev Zundel d...@denx.de Cheers Detlev -- Algebraists do it in a ring, in fields, in groups. -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 4/5] TFTP: add tftpsrv command
Luca Ceresoli luca.ceres...@comelit.it writes: Signed-off-by: Luca Ceresoli luca.ceres...@comelit.it Cc: Wolfgang Denk w...@denx.de Acked-by: Detlev Zundel d...@denx.de Cheers Detlev -- -- Question authority! -- Yeah, says who? -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 2/5] TFTP: rename STATE_RRQ to STATE_SEND_RRQ
Luca Ceresoli luca.ceres...@comelit.it writes: With the upcoming TFTP server implementation, requests can be either outgoing or incoming, so avoid ambiguities. Signed-off-by: Luca Ceresoli luca.ceres...@comelit.it Cc: Wolfgang Denk w...@denx.de Acked-by: Detlev Zundel d...@denx.de Cheers Detlev -- More than any other time in history, mankind faces a crossroads. One path leads to despair and utter hopelessness. The other to total extinction. Let us pray, we have the wisdom to choose correctly. -- Woody Allen -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 3/5] TFTP: net/tftp.c: add server mode receive
Hi Luca, Signed-off-by: Luca Ceresoli luca.ceres...@comelit.it Cc: Wolfgang Denk w...@denx.de Acked-by: Detlev Zundel d...@denx.de Just for the future: [...] diff --git a/net/tftp.h b/net/tftp.h index e3dfb26..3abdf7b 100644 --- a/net/tftp.h +++ b/net/tftp.h @@ -2,6 +2,8 @@ * LiMon - BOOTP/TFTP. * * Copyright 1994, 1995, 2000 Neil Russell. + * Copyright 2011 Comelit Group SpA + * Luca Ceresoli luca.ceres...@comelit.it * (See License) */ @@ -16,6 +18,10 @@ /* tftp.c */ extern void TftpStart (void); /* Begin TFTP get */ +#ifdef CONFIG_CMD_TFTPSRV +extern void TftpStartServer(void); /* Wait for incoming TFTP put */ +#endif + /**/ #endif /* __TFTP_H__ */ I think we don't need the ifdefs in header files, or am I missing something? I _really_ like to avoid ifdefs wherever we can ;) Cheers Detlev -- I have a computer on which I can install any code I choose. I don't think that is a security flaw. On the contrary, if only one company can install a new version, that is a grave security flaw for me as a user. -- Richard Stallman e1mjdjd-00010l...@fencepost.gnu.org -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 5/5] net/tftp.c: fix typo
Luca Ceresoli luca.ceres...@comelit.it writes: Signed-off-by: Luca Ceresoli luca.ceres...@comelit.it Cc: Wolfgang Denk w...@denx.de Acked-by: Detlev Zundel d...@denx.de Cheers Detlev -- The mathematician's patterns, like the painter's or the poet's, must be beautiful; the ideas, like the colours or the words, must fit together in a harmonious way. Beauty is the first test: there is no permanent place in the world for ugly mathematics. -- G. H. Hardy -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 0/4] Accurate boot time measurement
Hi Simon, [...] I believe I have covered this ground very thoroughly and would like advice please on what to do next. The options I can see are: As Graeme points out, you got enough positive feedback that I encourage you to continue and address the comments. - change the code to use a fallback when a microsecond timer is not available Excuse me for not diving into that too deep, but from my top-level overview, it would seem like a good idea that if an architecture _has_ a microsecond timer, it could use generic code to massage that into the HZ=1000 API that we currently use with the addition of offering the extended precision. Your code then would need such a fallback and could work with the best precision offered by the architecture. New architectures with microsecond timers would need to only implement the new microsecond timer infrastructure and be done with it. I know that this probably ignores much of the reality, but it is as much as I can say with the time that I have available. As a side-note, I would always suggest looking at how Linux solves similar problems and follow examples set there. - integrate with boot progress The current boot progress to me seems pretty ugly and could do with a make-over. So if it is possible to merge those two logical functions, we would have a win-win situation. - something else? drop it? No, please don't - I think you created enough interest to carry on. Cheers Detlev -- Perfecting oneself is as much unlearning as it is learning. -- Edsger Dijkstra -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC] Duplicated string prefix
Hi Gray, The string prefixes *** ERROR: and WARNING: (or variations thereof) is duplicated numerous times throughout u-boot. Replacing with e.g. a 'putserr' function which prepends the string prefix to the unique part of the error message and then displays it may be a worthwhile space saving. You should be able to tell beforehand how much this could save by analyzing the strings in a current U-Boot compile, right? This way one would know whether it makes sense to embark on that journey. _If_ we decide to go down that route, I'd suggest using functions comparable to what Linux offers, e.g. something like pr_err, pr_warning, etc. Cheers Detlev -- Perfecting oneself is as much unlearning as it is learning. -- Edsger Dijkstra -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] phylib: Detect link on 10G devices correctly
Hi Andy,
gen10g_startup() had 2 bugs:
1) It had a boolean logic error in checking the MMD mask, and
always checked all of them.
Good catch.
2) It checked devices which don't actually report link state, which
meant that it would never believe the link was fully up.
Fix the boolean logic, and then mask the MMD mask so only link-reporting
devices are checked.
Signed-off-by: Andy Fleming aflem...@freescale.com
Reported-by: Ed Swarthout ed.swarth...@freescale.com
---
drivers/net/phy/generic_10g.c |8 ++--
include/linux/mdio.h |8
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/net/phy/generic_10g.c b/drivers/net/phy/generic_10g.c
index 315c508..1c3e69e 100644
--- a/drivers/net/phy/generic_10g.c
+++ b/drivers/net/phy/generic_10g.c
@@ -36,7 +36,7 @@ int gen10g_shutdown(struct phy_device *phydev)
int gen10g_startup(struct phy_device *phydev)
{
int devad, reg;
- u32 mmd_mask = phydev-mmds;
+ u32 mmd_mask = phydev-mmds MDIO_DEVS_LINK;
So we are introducing this new macro, right? I don't see this in Linux
- how is this handled there? If we do have to create a new name, can't
we use something more self-documenting, e.g. MDIO_DEVS_REPORT_LINK or
such?
Cheers
Detlev
--
Quantum physicists can either know how fast they do it, or where they
do it, but not both.
--
DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 0/4] Accurate boot time measurement
Hi Simon and Wolfgang, [...] In terms of all this discussion I can see your point :-) I did have expressions of interest from two people including one I thought was at your company, which I why I went to the effort to clean up and submit this. At that time I didn't realise it would be such a touchy subject. I don't believe this topic to be touchy, it's just that Wolfgang trying to keep the whole code base in shape has a healthy inertia before introducing changes that may be difficult to keep consistent over the multitude of SoCs that we support. To throw in my personal view again, I still would like to see such an infrastructure to get into U-Boot code. I fully agree with Wolfgang that practically the printfs and timing are a method already in place to do measurements. On the other hand my experience has shown that for some reason or other this has never been widely used. So effectively, it wasn't too helpful for the project itself. So I still believe that if we _had_ an infrastructure like you propose, we would get more people interested in _actually_ measuring and improving the code base, which would certainly be a good thing. Of course we have systems that are very limited in their ressources, but this is the reason why many features of U-Boot are opt-in features not forcing any resource usage on such boards. So if some of them are so short on resources that they cannot use such a timing framework, then so be it. But as most of the probably are in the legacy code base, they should not stop us from getting getting positive effects for the currently important boards and architectures. Cheers Detlev -- A change in language can transform our appreciation of the cosmos -- Benjamin Lee Whorf -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCHv2] sntp: avoid use of uninitialized variable
Hi Chris,
From: Luuk Paulussen luuk.paulus...@alliedtelesis.co.nz
When we use the ntpserverip environment variable argv[1] may not be set.
Printing the error message using the NetNtpServerIP variable ensures the
correct output in both cases.
Signed-off-by: Luuk Paulussen luuk.paulus...@alliedtelesis.co.nz
Acked-by: Chris Packham chris.pack...@alliedtelesis.co.nz
Cc: Ben Warren biggerbadder...@gmail.com
---
Changes since v1:
- run through checkpatch.pl, fix line 80 chars
common/cmd_net.c |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/common/cmd_net.c b/common/cmd_net.c
index 8c6f5c8..fae3c7f 100644
--- a/common/cmd_net.c
+++ b/common/cmd_net.c
@@ -324,7 +324,8 @@ int do_sntp (cmd_tbl_t *cmdtp, int flag, int argc, char *
const argv[])
else NetTimeOffset = simple_strtol (toff, NULL, 10);
if (NetLoop(SNTP) 0) {
- printf(SNTP failed: host %s not responding\n, argv[1]);
+ printf(SNTP failed: host %pI4 not responding\n,
+ NetNtpServerIP);
return 1;
}
Acked-by: Detlev Zundel d...@denx.de
Cheers
Detlev
--
Is this a private fight or can anyone join in?
-- Old Irish saying
--
DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 3/6] TFTP: rename server to remote
Hi Luca, [...] I removed the checkpatch-related changes: they are now on the tftp cleanup patch series that I submitted on saturday, and on top of which v3 of the TFTP server will be based. Thanks for your persistence - it is very much appreciated! Detlev -- The proprietary-Unix players proved so ponderous, so blind, and so inept at marketing that Microsoft was able to grab away a large part of their market with the shockingly inferior technology of its Windows operating system. -- A Brief History of Hackerdom by Eric Steven Raymond -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 5/6] TFTP: net/tftp.c: add server mode receive
Hi Luca, [...] Hm, I have to admit that I do not really like adding so many ifdefs into the tftp code and even into the state machine code. Can you give me a number on how much larger u-boot gets on your platform with and without the server support? If this is not too much, maybe we should include support always. If it is too much, maybe we can at least keep the state machine without ifdefs - if I see it correctly, this will only add at maximum a few bytes and STATE_RECW_WRQ will simply never be entered, correct? Here are my measurements for the dig297 board (ARMV7 OMAP3): textdata bss dec hex 3570858684 214264 580033 8d9c1 TFTPSRV on 3563278660 214264 579251 8d6b3 TFTPSRV off, without #ifs 3563558660 214268 579283 8d6d3 TFTPSRV off, with #ifs (as in PATCH v2) The TFTP server adds 730 bytes to the text, so I guess it's worth to keep it optional. Ok, thanks for the number. To my big surprise, removing most bad-looking #ifs (all of those that save just one line) I got a *smaller* U-Boot! I repeated the test three times to be extra sure, the figures are correct: without the #ifs we save a few bytes. Obvious enough, I'm going to remove the #ifs. :) Another instance of the difficulty to predict modern compilers. Cheers Detlev -- But in terms of creative information, information that people can use or enjoy, and that will be used and enjoyed more the more people who have it, always we should encourage the copying. -- Richard M. Stallman -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Fix incorrect use of getenv() before relocation
9f13a3d..a0c4b10 100644
--- a/board/digsy_mtc/digsy_mtc.c
+++ b/board/digsy_mtc/digsy_mtc.c
@@ -191,15 +191,16 @@ phys_size_t initdram(int board_type)
int checkboard(void)
{
- char *s = getenv(serial#);
+ char buf[64];
+ int i = getenv_f(serial#, buf, sizeof(buf));
puts (Board: InterControl digsyMTC);
#if defined(CONFIG_DIGSY_REV5)
puts ( rev5);
#endif
- if (s != NULL) {
- puts(, );
- puts(s);
+ if (i 0) {
+ puts(, serial# );
+ puts(buf);
}
putc('\n');
This changes the output. On the other hand, this feature was never used
on this board, so I'm ok with removing this read altogether.
[...]
diff --git a/board/socrates/socrates.c b/board/socrates/socrates.c
index 72e7401..65fb70a 100644
--- a/board/socrates/socrates.c
+++ b/board/socrates/socrates.c
@@ -52,15 +52,17 @@ ulong flash_get_size (ulong base, int banknum);
int checkboard (void)
{
volatile ccsr_gur_t *gur = (void *)(CONFIG_SYS_MPC85xx_GUTS_ADDR);
-
- char *src;
+ char buf[64];
int f;
- char *s = getenv(serial#);
+ int i = getenv_f(serial#, buf, sizeof(buf));
+#ifdef CONFIG_PCI
+ char *src;
+#endif
puts(Board: Socrates);
- if (s != NULL) {
+ if (i 0) {
puts(, serial# );
- puts(s);
+ puts(buf);
}
putc('\n');
Looks good.
Cheers
Detlev
--
Another helpful hint for successful MIME processing:
application/msword; rm -f %s; description=MS Word Text;
--
DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] boot-up time optimization. Where to start?
Hi Simon, [...] I have a patch which displays boot time in microseconds if it is of interest to anyone: Timer summary in microseconds: MarkElapsed Stage 0 0 awake 193,298193,298 usb_start 1,342,411 1,149,113 eth_start 3,767,039 2,424,628 bootp_start 3,790,121 23,082 bootp_stop 3,790,293172 tftp start 4,761,459971,166 tftp done 4,761,489 30 bootm_start 4,892,145130,656 start_kernel Of course we are interested, please show us the code. Such code, even if it does not get into mainline can be a good starter for other people benchmarking their port. Thanks Detlev -- One of my most productive days was throwing away 1000 lines of code. - Ken Thompson. -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] U-boot 2010.12 Ping failed on DP83849C
Hi Jerry, I have a custom MPC8313ERD board loaded with u-boot 1.1.6 and the network PHY is NetSemi DP83849C. The PHY info is copied from DP83865 and the network works fine. Recently I am trying to upgrade u-boot to 2010.12 but I got Ping failed and host is not alive message when I ping the same host. After enabling debug, it shows: Trying TSEC0 Speed: 10, half duplex Using TSEC0 device sending ARP for 0a350275 ARP broadcast 1 TSEC0: tsec: tx error ARP broadcast 2 TSEC0: tsec: tx buffers full ping failed; host 10.53.2.117 is not alive After googling the problem, I learned this may be related to the some kind of bus conflict when DMA move ip packet to transmit bugger. My question is what caused this error in 2010.12 version and not in 1.1.6? Those versions are so far away codewise, that I there surely is no easy answer. _If_ your code was in mainline all the way from 1.1.6 to 2010.12 you could have used git bisect, but as this is (very likely) not the case, this will not work for you. So the easiest way is to start debugging the problem yourself. Best wishes Detlev Zundel -- Oh, didn't you know, the Lord did the original programming of the universe in COBOL. - That's why the world is the evil work of Satan. A true divine being would have used Scheme. - And, if so, Jesus would have been crucified on a big lambda symbol. -- K. Chafin, K. Schilling D. Hanley, on comp.lang.lisp -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [U-Boot-Users] U-Boot WinCE booting support?
Hi, On Tuesday, May 03, 2011 08:43:39 AM Balaji P wrote: Hi I am working in Arm based board which can support both linux and wince. We need unified bootloader to boot both. Please provide me some help. CCing uboot mailing list ... I can't help you more We do not have support for booting WinCE ready. On the other hand, if you know exactly what the WinCE kernel needs in terms of arguments to pass to it, it should not be too hard to implement support for it. Step 1 would be to declare a new operating system type for the U-Boot images in include/image.h ;) Cheers Detlev -- Paul Simon sang Fifty Ways to Lose Your Lover, because quite frankly, A Million Ways to Tell a Developer He Is a D*ckhead doesn't scan nearly as well. But I'm sure he thought about it. -- linux/Documentation/ManagementStyle -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 3/6] TFTP: rename server to remote
Hi Luca, Detlev Zundel wrote: I'll start a new thread to discuss this. Hopefully we then come up with a policy to stick into that wiki page. Now that the checkpatch policy is much more clear, I started to clean up the networking stuff, on top of which the TFTP server sits. Thanks a lot for starting this cleanup. Your work is very much appreciated. net/net.c alone counts 76 errors and 197 warnings. In terms of modified lines, it's going to be a big job, so I am doing it in separate patches, one per each category of errors/warnings (whitespace issues, lines 80 chars, parentheses issues etc). Is this choice ok? This makes sense to me yes. Also, it's going to be a bigger job than the TFTP server itself, so I'll send a separate patchset for the cleanup work, and hold the TFTP server until the cleanup is worked out. If you really start the clenaup, then this order makes absolute sense. Thanks in advance! Detlev -- Every time history repeats itself the price goes up. -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/3] arm: at91: ether: Prepare for mach-types.h changes
Hi Igor,
On 05/01/11 22:38, Reinhard Meyer wrote:
Dear Igor Grinberg,
at91 ethernet module used machine_is_cbs337() macro for board specific
Linux compatibility issue.
Use compile time defines instead.
Signed-off-by: Igor Grinberggrinb...@compulab.co.il
---
arch/arm/cpu/arm920t/at91rm9200/ether.c | 18 +-
1 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/arch/arm/cpu/arm920t/at91rm9200/ether.c
b/arch/arm/cpu/arm920t/at91rm9200/ether.c
index e1cdeba..4aeb883 100644
--- a/arch/arm/cpu/arm920t/at91rm9200/ether.c
+++ b/arch/arm/cpu/arm920t/at91rm9200/ether.c
@@ -201,15 +201,15 @@ int eth_init (bd_t * bd)
* that MicroMonitor behavior so we avoid needing to make such OS code
* care about which bootloader was used.
*/
-if (machine_is_csb337()) {
-p_mac-EMAC_SA2H = (enetaddr[0] 8) | (enetaddr[1]);
-p_mac-EMAC_SA2L = (enetaddr[2] 24) | (enetaddr[3] 16)
- | (enetaddr[4] 8) | (enetaddr[5]);
-} else {
-p_mac-EMAC_SA2L = (enetaddr[3] 24) | (enetaddr[2] 16)
- | (enetaddr[1] 8) | (enetaddr[0]);
-p_mac-EMAC_SA2H = (enetaddr[5] 8) | (enetaddr[4]);
-}
+#ifdef CONFIG_MACH_CSB337
+p_mac-EMAC_SA2H = (enetaddr[0] 8) | (enetaddr[1]);
+p_mac-EMAC_SA2L = (enetaddr[2] 24) | (enetaddr[3] 16)
+| (enetaddr[4] 8) | (enetaddr[5]);
+#else
+p_mac-EMAC_SA2L = (enetaddr[3] 24) | (enetaddr[2] 16)
+| (enetaddr[1] 8) | (enetaddr[0]);
+p_mac-EMAC_SA2H = (enetaddr[5] 8) | (enetaddr[4]);
+#endif
p_mac-EMAC_RBQP = (long) (rbfdt[0]);
p_mac-EMAC_RSR= ~(AT91C_EMAC_RSR_OVR | AT91C_EMAC_REC |
AT91C_EMAC_BNA);
There is nothing wrong with your patch itself, but it let me to take a
closer look at the
reasoning of why there is a machine dependency. The full code at this
section is:
eth_getenv_enetaddr(ethaddr, enetaddr);
/* The CSB337 originally used a version of the MicroMonitor bootloader
* which saved Ethernet addresses in the wrong order. Operating
* systems (like Linux) know this, and apply a workaround. Replicate
* that MicroMonitor behavior so we avoid needing to make such OS code
* care about which bootloader was used.
*/
if (machine_is_csb337()) {
p_mac-EMAC_SA2H = (enetaddr[0] 8) | (enetaddr[1]);
p_mac-EMAC_SA2L = (enetaddr[2] 24) | (enetaddr[3] 16)
| (enetaddr[4] 8) | (enetaddr[5]);
} else {
p_mac-EMAC_SA2L = (enetaddr[3] 24) | (enetaddr[2] 16)
| (enetaddr[1] 8) | (enetaddr[0]);
p_mac-EMAC_SA2H = (enetaddr[5] 8) | (enetaddr[4]);
}
So, for the sake of a(nother) broken bootloader and a workaround in Linux we
store the MAC address in the wrong order? What if U-Boot itself is used to
make
LAN accesses?
Well, I've read the comment before preparing the patch.
Actually, I felt like: this should be thrown away!.
Also, I haven't found csb337 board in the tree...
I didn't want to decide for you (If I'm not mistaken,
you are the maintainer of Atmel) what to do with it, so I left it.
Do you think we should remove this?
I would love to send another patch to remove this completely.
I'd say remove it. Why do I say that?
[dzu@pollux u-boot-testing (master)]$ make csb337_config
make: *** No rule to make target `csb337_config'. Stop.
make: *** [csb337_config] Error 1
[dzu@pollux u-boot-testing (master)]$ grep -i csb337 Makefile
[dzu@pollux u-boot-testing (master)]$ grep -i csb337 boards.cfg
[dzu@pollux u-boot-testing (master)]$
Cheers
Detlev
--
Indeed, the author firmly believes that the best serious work is also
good fun. We needn't apologize if we enjoy doing research.
-- Donald Knuth
--
DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/7] JFFS2: Calculate buf_len before we read data from flash
Hi Baidu,
Hi, Detlev :
2011/4/30 Detlev Zundel d...@denx.de:
Hi Baidu,
[...]
diff --git a/fs/jffs2/jffs2_1pass.c b/fs/jffs2/jffs2_1pass.c
index 8eb77b1..be6ac78 100644
--- a/fs/jffs2/jffs2_1pass.c
+++ b/fs/jffs2/jffs2_1pass.c
@@ -1643,6 +1643,8 @@ jffs2_1pass_build_lists(struct part_info * part)
case JFFS2_NODETYPE_INODE:
if (buf_ofs + buf_len ofs + sizeof(struct
jffs2_raw_inode)) {
+ buf_len = min_t(uint32_t, buf_size,
sector_ofs
+ + part-sector_size - ofs);
I am somewhat uncomfortable that buf_len is used in the if condition
before it is recalculated inside. Are you sure that this cannot lead to
problems, i.e. can buf_len become larger than it was when entering the
condition?
It works well in my boards.
Aha. I am all happy for you that this works, but we really need to be
as sure as we can be that the code does the right thing. It works for
me is not enough.
Please read every line code in the uboot jffs2 carefully. Know more
about JFFS2.
I was asking you a question about a potential problem that I see with
your changes. Can you answer the question or not? If you cannot answer
it, then you are changing things that you do not understand. Personally
I don't trust bug-fixes that are not understood completely.
Cheers
Detlev
--
Mit einem Leben wie dem meinen, Doktor - wer braucht da noch Traeume?
- Alex Portnoy
--
DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 6/7] JFFS2: scanning performance improvement
the open parenthesis '('
#615: FILE: fs/jffs2/jffs2_nand_1pass.c:810:
+ if(! jffs_init_1pass_list(part))
ERROR: trailing whitespace
#617: FILE: fs/jffs2/jffs2_nand_1pass.c:812:
+^I$
ERROR: patch seems to be corrupt (line wrapped?)
#716: FILE: fs/jffs2/jffs2_1pass.c:800:
inode, char *dest)
WARNING: line over 80 characters
#721: FILE: fs/jffs2/jffs2_1pass.c:804:
+ putLabeledWord(UNKNOWN COMPRESSION
METHOD =3D , jNode-compr);
WARNING: line over 80 characters
#739: FILE: fs/jffs2/jffs2_1pass.c:1560:
+ /* Scan only DEFAULT_EMPTY_SCAN_SIZE of 0xFF before declaring
it's =
WARNING: line over 80 characters
#747: FILE: fs/jffs2/jffs2_1pass.c:1565:
+ printf(Block at 0x%08x is empty (erased)\n,
sector_ofs);
WARNING: line over 80 characters
#751: FILE: fs/jffs2/jffs2_1pass.c:1568:
+ /* Now ofs is a complete physical flash offset as it always
was... */
WARNING: line over 80 characters
#765: FILE: fs/jffs2/jffs2_nand_1pass.c:354:
+ putLabeledWord(UNKNOWN COMPRESSION METHOD =3D
, inode-compr);
WARNING: line over 80 characters
#783: FILE: fs/jffs2/jffs2_nand_1pass.c:829:
+ if (jffs2_fill_scan_buf(nand, buf, offset,
DEFAULT_EMPTY_SCAN_SIZE))
WARNING: line over 80 characters
#790: FILE: fs/jffs2/jffs2_nand_1pass.c:832:
+ /* Scan only DEFAULT_EMPTY_SCAN_SIZE of 0xFF before declaring
it's =
WARNING: line over 80 characters
#792: FILE: fs/jffs2/jffs2_nand_1pass.c:833:
+ while (ofs DEFAULT_EMPTY_SCAN_SIZE *(uint32_t
*)(buf[ofs]) =
WARNING: line over 80 characters
#801: FILE: fs/jffs2/jffs2_nand_1pass.c:837:
+ if (jffs2_fill_scan_buf(nand, buf + DEFAULT_EMPTY_SCAN_SIZE,
offset + =
WARNING: line over 80 characters
#936: FILE: fs/jffs2/jffs2_1pass.c:1616:
+ /* If we're only checking the beginning of a
block with a cleanmarker,
ERROR: trailing whitespace
#938: FILE: fs/jffs2/jffs2_1pass.c:1618:
+^I^I^I^Iif((buf_ofs == sector_ofs) $
WARNING: suspect code indent for conditional statements (32, 36)
#938: FILE: fs/jffs2/jffs2_1pass.c:1618:
+ if((buf_ofs == sector_ofs)
[...]
+ printf(%d bytes at start of block seems
clean... assuming all clean\n,
ERROR: space required before the open parenthesis '('
#938: FILE: fs/jffs2/jffs2_1pass.c:1618:
+ if((buf_ofs == sector_ofs)
ERROR: trailing whitespace
#939: FILE: fs/jffs2/jffs2_1pass.c:1619:
+^I^I^I^I(empty_start == sector_ofs +sizeof(struct jffs2_unknown_node)))
{ $
WARNING: line over 80 characters
#939: FILE: fs/jffs2/jffs2_1pass.c:1619:
+ (empty_start == sector_ofs +sizeof(struct
jffs2_unknown_node))) {
ERROR: need consistent spacing around '+' (ctx:WxV)
#939: FILE: fs/jffs2/jffs2_1pass.c:1619:
+ (empty_start == sector_ofs +sizeof(struct
jffs2_unknown_node))) {
^
WARNING: line over 80 characters
#940: FILE: fs/jffs2/jffs2_1pass.c:1620:
+ printf(%d bytes at start of block seems
clean... assuming all clean\n,
WARNING: line over 80 characters
#941: FILE: fs/jffs2/jffs2_1pass.c:1621:
+
EMPTY_SCAN_SIZE(part-sector_size));
ERROR: trailing whitespace
#958: FILE: fs/jffs2/jffs2_1pass.c:1644:
+^I^I^I$
total: 32 errors, 22 warnings, 369 lines checked
NOTE: whitespace errors detected, you may wish to use scripts/cleanpatch or
scripts/cleanfile
/home/dzu/transfer/am has style problems, please review. If any of these
errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Cheers
Detlev
[1]
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob_plain;f=scripts/checkpatch.pl
[2] http://www.denx.de/wiki/U-Boot/CodingStyle
--
It's amazing I won. I was running against peace, prosperity, and
incumbency.
-- George H.W. Bush, 06/14/2001, talking to Swedish prime
minister Goran Perrson, unaware that a live tv camera was still on
--
DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 0/5] introduce nand write.ubi, and drop ffs for jffs2 too
Hi Artem, On Fri, 2011-04-29 at 13:58 +0200, Detlev Zundel wrote: Hi Ben, It was found that on da850evm, where the NAND ECC used does not map all 0xff data to 0xff ECC, that flashing UBI and JFFS2 image from U-boot with nand write[.e] command resulted in alot of ECC errors... for UBI the result was an unmountable filesystem on second attach from linux. For JFFS2 the result was a multitude of ECC errors printed on the cosole on the second mount in Linux -- the filesystem remains mountable for awhile but eventually collapses. I am not sure that I can follow you here so I have to ask you to clarify the problem for me. I understand that a page of 0xffs does _not_ have an ECC of all 0xffs. Actually I would be surprised if there was any ECC having this property, so I doubt that this is da850 specific, correct? So I am wondering about two things: - If I erase a page in NAND, will the ECC be updated by someone or will it be 0xffs? If the latter, then is it normal to have ECC errors on freshly erased pages? - If we (correctly) write 0xffs even to an erased page, a generated ECC should match this content, so I do not see where ECC errors should come from in this setting. Summarily, I do not understand where the ECC errors came from in your setup and what the faulting party in that scenario actually is/was. Can you please enlighten me? I do not know why I'm in CC, but I see that the code to skip all 0xFFs looks like it was copied from UBI. The reason why UBI and UBI user-space tools skip NAND pages with all 0xFFs when writing is described here: http://www.linux-mtd.infradead.org/doc/ubi.html#L_flasher_algo If it is too long to read, in short: 1. if we write to flash, we always write to an erased eraseblock, obviously :-) Yes - I simply wasn't sure if the software layers below (in U-Boot) would do an erase on demand before writing. Reading the code, this isn't the case (I should have known this), so the skipping should really be safe. 2. erased erase block contains all 0xFFs, so skipping 0xFF NAND pages is harmless. 3. writing 0xFF pages has side-effects - the ECC bytes in OOB will be used 4. If we are flashing an UBIFS image, UBIFS will use the half-filled eraseblocks, and if the free pages were written with 0xFFs, they'll become unusable. So we skip 0xFF pages at the end. Not all, only the last ones. E.g., if your eraseblock consists of 4 pages, and you write data, 0xFFs, data, 0xFFs, then we'll only write data, 0xFFs, data, so that the last NAND page can be used later. Sorry for my poor English, and I'm writing in a hurry - have to have to a pub to farewell several colleagues who are leaving the company :) Thanks for your input. So I still fail to see where the ECC errors come from. The closes thing that makes sense for me, is that Bens problem very likely wasn't ECC connected at all but the result of the not capable to write twice. I.e. his NAND flashes cannot be written to twice. When he flashed the images in U-Boot, there were trailing empty blocks that got programmed and UBIFS assumed that it _could_ write to them so it tried and failed and somehow got tripped up over this. If this is the case then we should change the commit message to point to the real problem that this patch fixes. Cheers Detlev -- ``The number of UNIX installations has grown to 10, with more expected.'' Unix Programmers Manual -- 1972 The number of UNIX variants has grown to dozens, with more expected. -- 2001 -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] boot-up time optimization. Where to start?
Hi Alexander,
Dear Wolfgang,
Am Mittwoch, 27. April 2011, 21:08:50 schrieb Wolfgang Denk:
In message 201104271759.11818.alexander.st...@systec-electronic.com you
wrote:
Setting stdin, stdout and stderr takes a lot of time (IIRC ~500ms). Which
IMO is useless on a bootloader without LCD support.
Statements like this are completely worhtless if you don;t tell
exactly on which architecture and board, and with which exact version
of U-Boot such numbers have been measured.
Ok, let me be more precise on this.
We used U-Boot v2010.09 on a custom board running on an I.MX35 (ARM1136).
We noticed the following code snippet took relatively long.
From common/console.c in console_init_r(void):
/* Setting environment variables */
for (i = 0; i 3; i++) {
setenv(stdio_names[i], stdio_devices[i]-name);
}
We added PIN toggling around this part of code and measured something 100ms.
A collegue said it was ~100ms, I remembered ~500ms. Dunno who is right.
It doesn't really matter who is right - 100ms is way off for setting
these variables. Looking into common/cmd_nvedit.c, these variables have
a special handling and there are ifdef's involved, so its not
straightforward to read. You should really find out, where in there the
time is spent for your board and fix the problem ;)
Cheers
Detlev
--
I can understand that things like user-level libraries have to take crazy people
into account, but the kernel internal libraries definitely do not.
-- Linus Torvalds
--
DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] boot-up time optimization. Where to start?
Hi Alexander,
[...]
Ok, let me be more precise on this.
We used U-Boot v2010.09 on a custom board running on an I.MX35 (ARM1136).
We noticed the following code snippet took relatively long.
From common/console.c in console_init_r(void):
/* Setting environment variables */
for (i = 0; i 3; i++) {
setenv(stdio_names[i], stdio_devices[i]-name);
}
We added PIN toggling around this part of code and measured something
100ms. A collegue said it was ~100ms, I remembered ~500ms. Dunno who is
right.
It doesn't really matter who is right - 100ms is way off for setting
these variables. Looking into common/cmd_nvedit.c, these variables have
a special handling and there are ifdef's involved, so its not
straightforward to read. You should really find out, where in there the
time is spent for your board and fix the problem ;)
Our 'fix' was removing the stated lines at all.
That is not a fix but simply ignoring a problem. Maybe if you find out
why these lines have such an unexpected run-time, you will solve more
problems also.
We have a mantra here on the mailing list, so let me introduce you to
it:
Solve the problems one by one in the order that you encounter them.
Every ignored problem will come back later and catch you. Really, it
will.
Now repeat after me ;)
Cheers
Detlev
--
There is no need to be rigid in carrying out policies about what changes
to install. To do a good job of maintaining Emacs doesn't require acting
like government bureaucrats.
-- Richard Stallman e1mix3y-0005iz...@fencepost.gnu.org
--
DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3] cosmetic: cmd_bdinfo.c: clean up by using checkpatch.pl
Hi Macpaul, cmd_bdinfo.c: clean up with 2.6.38 checkpatch.pl Signed-off-by: Macpaul Lin macp...@andestech.com Looks sane to me, thanks! Acked-by: Detlev Zundel d...@denx.de -- Peace of mind isn't at all superficial to technical work. It's the whole thing. That which produces it is good work and that which destroys it is bad work. -- Robert M. Pirsig -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] cmd_bdinfo.c: clean up with 2.6.38 checkpatch.pl
Hi Macpaul, Hi Detlev, 2011/4/27 Detlev Zundel d...@denx.de: Hi Macpaul, clean up with 2.6.38 checkpatch.pl There is indeed only one non-whitespace hunk in there: It's ok that you do the line wrapping, but actually with such lists we require alphabetical sorting of the clauses, so can you please juggle the list so that the XILINX_405 gets to the end? Since I've correct the subject of this patch v3 according to the cosmetic rule, it seems patch v3 is out of this mailing thread. This change of the subject was the reason why I did not require you changing it as I know that Wolfgang currently matches patch versions by subject strings. Correct threading however uses References: mail headers and can cope with changing subjects. Somehow you seem to know this, as v3 indeed _has_ such a correct header and thus the threading in my mail reader works just fine ;) Cheers Detlev -- 0x2B | ~0x2B -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Update and Cut down mach types
Hi Igor,
[...]
+1 to all said above, though some minor patching should be done:
u-boot $ grep -rn machine_is_ --exclude=mach-types.h *
arch/arm/cpu/arm920t/at91rm9200/ether.c:204:if (machine_is_csb337()) {
board/ti/omap1610inn/omap1610innovator.c:66:if (machine_is_omap_h2())
board/ti/omap1610inn/omap1610innovator.c:68:else if
(machine_is_omap_innovator())
Thanks for finding those spots. Care to send a patch for these?
Cheers
Detlev
--
We have a live-manual. It's called emacs-de...@gnu.org.
You can stick to just reading it, but you can skip to a specific chapter
by simply sending an email asking for it ;-)
-- Stefan Monnier
--
DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] fw_setenv usage for multiple copies of U-Boot Environment Variables
Hi Amarendra, Hi Wolfgang Denk, Thank you for the reply. Yes I am talking about redundant environment. If we have four redundant environment copies in the same partition(mtd1)... Does the below fw_env.config configuration hold good ? U-Boot cannot cope with more than two redundant copies. # File: fw_env.config # Configuration file for fw_(printenv/saveenv) utility. # Up to two entries are valid, in this case the redundand # environment sector is assumed present. Read the text carefully: Up to two entries are valid # MTD device name Device offset Env. size Flash sector size /dev/mtd1 0x0 0x02000 0x2 /dev/mtd1 0x2 0x02000 0x2 /dev/mtd1 0x4 0x02000 0x2 /dev/mtd1 0x6 0x02000 0x2 On the other hand why exactly do you need four redundant environments? The idea behind redundant environments is that a powercut _while_ updating one copy can be handled by having a backup copy. It is _not_ meant to handle faulting devices in any way. Cheers Detlev -- I have always observed that the pretensions of all people are in exact inverse ratio to their merits; this is one of the axioms of morals.-- Joseph Lagrange -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] strawman Fastboot in U-Boot Design Doc
Hi John, Here a first draft design doc. It is based in part on the Fastboot implementation in the rowboat git repo (pointer below). Thanks for this piece of documentation. Itcertainly helped me understand that the boot part in fastboot is kind of a misnomer, right? Again it shows that a bad description can lead thoughts astray [...] Purpose === Mainline support for Android Fastboot would be useful. Arguing the merits of Fastboot vs DFU is not in the scope of this document. This document is to discuss design goals/requirements of an implementation of Fastboot in U-Boot. So in essence you say that fastboot is an alternative implementation of the idea behind DFU? Or is DFU (functionally) a subset of fastboot? It may be worth extending on that aspect so that people knowing DFU can hook into the discussion. [...] Questions Should a Fastboot host program be included in the U-Boot source? I would certainly welcome a reference implementation for a GNU/Linux host. Hopefully this will prevent multiple works later on. Maybe even writing a small library and having a simple command line wrapper would be good (like flat device tree and libfdt). Best wishes Detlev -- Is this a private fight or can anyone join in? -- Old Irish saying -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] README: Clarify difference of CONFIG_WATCHDOG and CONFIG_HW_WATCHDOG
Hi Mike, On Wed, Apr 27, 2011 at 11:25, Detlev Zundel wrote: Now that we have the documentation, the code should be changed to reflect it ;) Asd far as I can see, these are the places where HW_WATCHDOG is used instead of WATCHDOG: the trouble is that watchdog.h doesnt seem to match, nor does the name. hardware means to me hardware as in the cpu hardware. Yes, I understand that the situation is not 100% ideal, but I do not share your pessimism. Currently we should straighten out the inconsistencies present in the code and then subsequently we can think about aligning it with what we agree on to be a good name for. #ifdef CONFIG_HW_WATCHDOG ... #else /* * Maybe a software watchdog? */ ... #endif i dont see how a watchdog in the SoC could be a software watchdog or a non-hardware watchdog. True, we should align the comments in here also - I'll se if I can come up with something sensible. the watchdog system is a legacy mess, so probably be easier to just gut the whole thing. A agree that the naming is not ideal, but this isn't a good enough argument for removing it. Apart from that, this feature is used in many commercial systems... Cheers Detlev -- config LGUEST If unsure, say N. If curious, say M. If masochistic, say Y. -- linux/drivers/lguest/Kconfig -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] cmd_bdinfo.c: clean up with 2.6.38 checkpatch.pl
Hi Macpaul, Correct threading however uses References: mail headers and can cope with changing subjects. Somehow you seem to know this, as v3 indeed _has_ such a correct header and thus the threading in my mail reader works just fine ;) Yes. I did fix the References and In-reply-to headers. However v3 it doesn't goes to the correct threadning in gmail. :p Gmail cannot handle references: headers? That is news to me. I find it always interesting that basic century old things seem to be forgotten by modern all bells and whistles software Cheers Detlev -- Windows Vista? And what a vista! All you see as you look around your garden is a 60foot high brick wall -- Crosbie Fitch. -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] cmd_bdinfo.c: clean up with 2.6.38 checkpatch.pl
Hi Wolfgang, [...] This change of the subject was the reason why I did not require you changing it as I know that Wolfgang currently matches patch versions by subject strings. This is not quite correct. I refer to the Subject: only if no other thread information is present, i. e. if we have working In-reply-to: and/or References: hewaders this works fine for me, too. Ah ok. Sorry for misremembering and thanks for the correction. Cheers Detlev -- Cyberwar is certainly not a myth. But you haven't seen it yet, despite the attacks on Estonia. Cyberwar is warfare in cyberspace. And warfare involves massive death and destruction. When you see it, you'll know it. -- Bruce Schneier, Nov. 2007 -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/5] nand_util: convert nand_write_skip_bad() to flags
Hi Ben,
In a future commit the behaviour of nand_write_skip_bad()
will be further extended.
Convert the only flag currently passed to the nand_write_
skip_bad() function to a bitfield of only one allocated
member. This should avoid an explosion of int's at the
end of the parameter list or the ambiguous calls like
nand_write_skip_bad(info, offset, len, buf, 0, 1, 1);
nand_write_skip_bad(info, offset, len, buf, 0, 1, 0);
Instead there will be:
nand_write_skip_bad(info, offset, len, buf, WITH_OOB |
WITH_OTHER);
Signed-off-by: Ben Gardiner bengardi...@nanometrics.ca
---
common/cmd_nand.c|3 ++-
drivers/mtd/nand/nand_util.c |8
include/nand.h |5 -
3 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/common/cmd_nand.c b/common/cmd_nand.c
index 7bd37de..69b2acc 100644
--- a/common/cmd_nand.c
+++ b/common/cmd_nand.c
@@ -581,7 +581,8 @@ int do_nand(cmd_tbl_t * cmdtp, int flag, int argc, char *
const argv[])
printf(Unknown nand command suffix '%s'.\n,
s);
return 1;
}
- ret = nand_write_skip_bad(nand, off, rwsize, (u_char
*)addr, 1);
+ ret = nand_write_skip_bad(nand, off, rwsize,
+ (u_char *)addr, WITH_OOB);
#endif
} else if (!strcmp(s, .oob)) {
/* out-of-band data */
I see an occurrence of nand_write_skip_bad just above this if block.
Please replace this also.
diff --git a/drivers/mtd/nand/nand_util.c b/drivers/mtd/nand/nand_util.c
index 5a6f7ae..2bd8758 100644
--- a/drivers/mtd/nand/nand_util.c
+++ b/drivers/mtd/nand/nand_util.c
@@ -448,11 +448,11 @@ static int check_skip_len(nand_info_t *nand, loff_t
offset, size_t length)
* @param offset offset in flash
* @param length buffer length
* @param bufferbuffer to read from
- * @param withoobwhether write with yaffs format
+ * @param flags flags mmofying the behaviour of the write to
NAND
* @return 0 in case of success
*/
int nand_write_skip_bad(nand_info_t *nand, loff_t offset, size_t *length,
- u_char *buffer, int withoob)
+ u_char *buffer, int flags)
{
int rval = 0, blocksize;
size_t left_to_write = *length;
@@ -460,7 +460,7 @@ int nand_write_skip_bad(nand_info_t *nand, loff_t offset,
size_t *length,
int need_skip;
#ifdef CONFIG_CMD_NAND_YAFFS
- if (withoob) {
+ if (flags WITH_OOB) {
int pages;
pages = nand-erasesize / nand-writesize;
blocksize = (pages * nand-oobsize) + nand-erasesize;
@@ -529,7 +529,7 @@ int nand_write_skip_bad(nand_info_t *nand, loff_t offset,
size_t *length,
write_size = blocksize - block_offset;
#ifdef CONFIG_CMD_NAND_YAFFS
- if (withoob) {
+ if (flags WITH_OOB) {
int page, pages;
size_t pagesize = nand-writesize;
size_t pagesize_oob = pagesize + nand-oobsize;
diff --git a/include/nand.h b/include/nand.h
index 7459bd0..628317a 100644
--- a/include/nand.h
+++ b/include/nand.h
@@ -114,8 +114,11 @@ typedef struct nand_erase_options nand_erase_options_t;
int nand_read_skip_bad(nand_info_t *nand, loff_t offset, size_t *length,
u_char *buffer);
+
+#define WITH_OOB (1 0) /* whether write with yaffs format */
+
If this flag is really only relevant for YAFFS, then please include this
in its name, i.e. rename it to WITH_YAFFS_OOB.
int nand_write_skip_bad(nand_info_t *nand, loff_t offset, size_t *length,
- u_char *buffer, int withoob);
+ u_char *buffer, int flags);
int nand_erase_opts(nand_info_t *meminfo, const nand_erase_options_t *opts);
#define NAND_LOCK_STATUS_TIGHT 0x01
Cheers
Detlev
--
Those who do not understand Unix are condemned to reinvent it,
poorly.
- Henry Spencer, University of Toronto Unix hack
--
DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 4/5] cmd_nand: add nand write.ubi command
Hi Ben, Add another nand write. variant, ubi. This command will request of nand_write_skip_bad() that all trailing all-0xff pages will be dropped from eraseblocks as they are written as-per the reccommended behaviour of the UBI FAQ. If I understand the code correctly, then the assumption is that writing FFs to an erased flash is essentially a no-op, right? This is not really UBI specific, so why don't we use a name like e.g. trimffs for the new functionality? Moreover now that I think about it, I can imagine a corner case where the flash is not erased at positions where the image contains ffs. As I read your code, the ffs will silently be dropped and no error will be generated, although the contents of the image will _not_ correpsond to the contents in flash. If this is right, then this has potential for great confusion. Maybe we should check that the flash is really erased at the positions corresponding to ffs? Cheers Detlev -- It's bad civic hygiene to build technologies that could someday be used to facilitate a police state. No matter what the eavesdroppers and censors say, these systems put us all at greater risk. Communications systems that have no inherent eavesdropping capabilities are more secure than systems with those capabilities built in. -- Bruce Schneier -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 0/5] introduce nand write.ubi, and drop ffs for jffs2 too
Hi Ben, It was found that on da850evm, where the NAND ECC used does not map all 0xff data to 0xff ECC, that flashing UBI and JFFS2 image from U-boot with nand write[.e] command resulted in alot of ECC errors... for UBI the result was an unmountable filesystem on second attach from linux. For JFFS2 the result was a multitude of ECC errors printed on the cosole on the second mount in Linux -- the filesystem remains mountable for awhile but eventually collapses. I am not sure that I can follow you here so I have to ask you to clarify the problem for me. I understand that a page of 0xffs does _not_ have an ECC of all 0xffs. Actually I would be surprised if there was any ECC having this property, so I doubt that this is da850 specific, correct? So I am wondering about two things: - If I erase a page in NAND, will the ECC be updated by someone or will it be 0xffs? If the latter, then is it normal to have ECC errors on freshly erased pages? - If we (correctly) write 0xffs even to an erased page, a generated ECC should match this content, so I do not see where ECC errors should come from in this setting. Summarily, I do not understand where the ECC errors came from in your setup and what the faulting party in that scenario actually is/was. Can you please enlighten me? Thanks Detlev -- SWYM XYZ (Sympathize with your machinery). [..] In fact it is often called a no-op, because it performs no operation. It does, however, keep the machine running smoothly, just as real-world swimming helps to keep programmers healthy. -- Donald Knuth, Fascicle 1 -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] MX31: change return value of get_cpu_rev
Hi Stefano, [...] @@ -129,7 +129,7 @@ char *get_cpu_rev(void) for (i = 0; i ARRAY_SIZE(mx31_cpu_type); i++) if (srev == mx31_cpu_type[i].srev) return mx31_cpu_type[i].v; - return unknown; + return srev; Hm, so we drop the unknown case and return the srev unchanged. Yes, I have changed this behavior. I thought, if the revision is not in the table, it is better to know which is the value of srev register. I do not know if it is better to print only an unknown or get directly the value of the register, to check in some documentation which new version was put on the board. Well, I do not insist on unknown - all I want is some way that someone reading the message can tell whether it is a translated or an untranslated value. I.e. adding a second (sensible) output string for no translation is also ok for me. Cheers Detlev -- Man is a fool, and woman, for tolerating him, is a damned fool -- Mark Twain -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/7] JFFS2: Bug fix for summary support
Hi, 1/ Get the latest DIRENT For example, if you create a file in linux jffs2 which config summary support, then you delete the file , you will not see the file in linux jffs2. But you can also see this file in uboot after you reset the system. That is because all the nodes in jffs2 which config summary will not be marked as obsolete. The deleted file's DIRENT node will be seen in uboot. So what we need to do is to get the latest DIRENT whose ino in DIRENT is 0. Sorry, but I do not understand that last sentence. Can you clarify this please? Than we will not see this file in uboot which is what we want. 2/ Add CONFIG_SYS_JFFS2_SORT_FRAGMENTS definition,if we config jffs2 summary in uboot. All the inodes of a file will not marked as obsolete, if they do not sort in the list struct b_node *, the latest data in inode may be overwritten by the older one. Also I have trouble making sense of this text. Signed-off-by: Baidu Liu liucai@gmail.com --- fs/jffs2/jffs2_1pass.c | 17 - fs/jffs2/jffs2_nand_1pass.c | 17 + include/jffs2/jffs2.h | 10 ++ 3 files changed, 35 insertions(+), 9 deletions(-) [...] diff --git a/include/jffs2/jffs2.h b/include/jffs2/jffs2.h index 651f94c..5b006c0 100644 --- a/include/jffs2/jffs2.h +++ b/include/jffs2/jffs2.h @@ -41,6 +41,16 @@ #include asm/types.h #include jffs2/load_kernel.h +#ifdef CONFIG_JFFS2_SUMMARY +#ifndef CONFIG_SYS_JFFS2_SORT_FRAGMENTS +/* +we should define CONFIG_SYS_JFFS2_SORT_FRAGMENTS,if +CONFIG_JFFS2_SUMMARY is enabled. +*/ +#define CONFIG_SYS_JFFS2_SORT_FRAGMENTS +#endif +#endif + #define JFFS2_SUPER_MAGIC 0x72b6 /* Values we may expect to find in the 'magic' field */ If JFFS2_SUMMARY _needs_ SORT_FRAGMENTS, then we should say so, i.e. /* CONFIG_JFFS2_SUMMARY will not work correctly without CONFIG_SYS_JFFS2_SORT_FRAGMENTS */ Alas, technically I do not understand why that is the case. So I invite people more knowledgeable with JFFS2 to comment on this bit. [time passes] Wait a minute - I tried to understand the code here - is it possible that SORT_FRAGMENTS really is needed _whenever_ we have a read-write JFFS2 filesystem? I.e. even without summary support we will have problems without SORT_FRAGMENTS? Wow, if this is true, then the option is certainly named completely misleading and most boards using JFFS2 actually use incorrect code We should define this option by default and only let people undefine it if they know exactly what they do. Cheers Detlev -- An elephant is a mouse with an operating system. -- Donald Knuth -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/7] JFFS2: Calculate buf_len before we read data from flash
Hi Baidu,
1/ We should calculate the buf_len before we call
get_fl_mem().
If I read your change correctly, then do you mean the following?
When we know what we want to read, we can calculate buf_len to be the
maximum size of the data to be read. Without this, we usually read
EMPTY_SCAN_SIZE which is too much.
This is only an optimization change.
Maybe this text will help people understand better what the change tries
to do.
diff --git a/fs/jffs2/jffs2_1pass.c b/fs/jffs2/jffs2_1pass.c
index 8eb77b1..be6ac78 100644
--- a/fs/jffs2/jffs2_1pass.c
+++ b/fs/jffs2/jffs2_1pass.c
@@ -1643,6 +1643,8 @@ jffs2_1pass_build_lists(struct part_info * part)
case JFFS2_NODETYPE_INODE:
if (buf_ofs + buf_len ofs + sizeof(struct
jffs2_raw_inode)) {
+ buf_len = min_t(uint32_t, buf_size,
sector_ofs
+ + part-sector_size - ofs);
I am somewhat uncomfortable that buf_len is used in the if condition
before it is recalculated inside. Are you sure that this cannot lead to
problems, i.e. can buf_len become larger than it was when entering the
condition?
get_fl_mem((u32)part-offset + ofs,
buf_len, buf);
buf_ofs = ofs;
@@ -1659,11 +1661,11 @@ jffs2_1pass_build_lists(struct part_info * part)
}
break;
case JFFS2_NODETYPE_DIRENT:
- if (buf_ofs + buf_len ofs + sizeof(struct
- jffs2_raw_dirent) +
- ((struct
- jffs2_raw_dirent *)
- node)-nsize) {
+ if (buf_ofs + buf_len ofs +
+ sizeof(struct jffs2_raw_dirent) +
+ ((struct jffs2_raw_dirent
*)node)-nsize) {
+ buf_len = min_t(uint32_t, buf_size,
sector_ofs
+ + part-sector_size - ofs);
The same question for this case.
Cheers
Detlev
--
Those who do not understand Unix are condemned to reinvent it,
poorly.
- Henry Spencer, University of Toronto Unix hack
--
DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 4/7] JFFS2: Improve error checking
Hi Baidu, Check the return value when we do malloc. Signed-off-by: Baidu Liu liucai@gmail.com --- fs/jffs2/jffs2_1pass.c | 12 ++-- fs/jffs2/jffs2_nand_1pass.c |5 - 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/fs/jffs2/jffs2_1pass.c b/fs/jffs2/jffs2_1pass.c index be6ac78..b3d94af 100644 --- a/fs/jffs2/jffs2_1pass.c +++ b/fs/jffs2/jffs2_1pass.c @@ -662,7 +662,8 @@ jffs2_free_cache(struct part_info *part) pL = (struct b_lists *)part-jffs2_priv; free_nodes(pL-frag); free_nodes(pL-dir); - free(pL-readbuf); + if(pL-readbuf) + free(pL-readbuf); free(pL); } } This looks ok. @@ -1470,9 +1471,16 @@ jffs2_1pass_build_lists(struct part_info * part) /* lcd_off(); */ /* if we are building a list we need to refresh the cache. */ - jffs_init_1pass_list(part); + if(! jffs_init_1pass_list(part)) + return 0; + This is strange. We now check for an error of jffs2_init_1pass_list, which currently always returns 0, so let's see where you change that. Ah, you don't (it's in line 671 in this file). It's only in jffs2_nand_1pass that you do a change, but even there we have a problem: diff --git a/fs/jffs2/jffs2_nand_1pass.c b/fs/jffs2/jffs2_nand_1pass.c index 9bad690..885fa3c 100644 --- a/fs/jffs2/jffs2_nand_1pass.c +++ b/fs/jffs2/jffs2_nand_1pass.c @@ -251,6 +251,7 @@ jffs_init_1pass_list(struct part_info *part) pL-dir.listCompare = compare_dirents; pL-frag.listCompare = compare_inodes; #endif + return 1; When malloc fails, we get no error output. } return 0; } @@ -806,7 +807,9 @@ jffs2_1pass_build_lists(struct part_info * part) nand = nand_info + id-num; /* if we are building a list we need to refresh the cache. */ - jffs_init_1pass_list(part); + if(! jffs_init_1pass_list(part)) + return 0; + And the caller fails also, so the user in this case will see no error message and no output. Not good. Cheers Detlev -- Für jemanden, der in eine Religion geboren wurde, in der das Ringen um eine einzige Seele ein Stafettenlauf über viele Jahrhunderte sein kann [..], hat das Tempo des Christentums etwas Schwindelerregendes. Wenn der Hinduismus friedlich dahinfließt wie der Ganges, dann ist das Christentum Toronto in der Rushhour.-- Yann Martel -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 5/7] JFFS2: Change DEFAULT_EMPTY_SCAN_SIZE to 256 Bytes
Hi Baidu, 1/ Syncs up with jffs2 in the linux kernel: If the first 256 Bytes is 0xff,we get the conclusion that the sector is empty. Signed-off-by: Baidu Liu liucai@gmail.com --- fs/jffs2/jffs2_1pass.c | 11 ++- fs/jffs2/jffs2_nand_1pass.c | 13 ++--- include/jffs2/jffs2.h |2 ++ 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/fs/jffs2/jffs2_1pass.c b/fs/jffs2/jffs2_1pass.c index b3d94af..62ba250 100644 --- a/fs/jffs2/jffs2_1pass.c +++ b/fs/jffs2/jffs2_1pass.c @@ -801,7 +801,7 @@ jffs2_1pass_read_inode(struct b_lists *pL, u32 inode, char *dest) #endif default: /* unknown */ - putLabeledWord(UNKOWN COMPRESSION METHOD = , jNode-compr); + putLabeledWord(UNKNOWN COMPRESSION METHOD = , jNode-compr); put_fl_mem(jNode, pL-readbuf); return -1; break; This typo change is not mentioned in the change log and really does not belong here. Please put it into a separate changeset. [...] diff --git a/fs/jffs2/jffs2_nand_1pass.c b/fs/jffs2/jffs2_nand_1pass.c index 885fa3c..5afe779 100644 --- a/fs/jffs2/jffs2_nand_1pass.c +++ b/fs/jffs2/jffs2_nand_1pass.c @@ -351,7 +351,7 @@ jffs2_1pass_read_inode(struct b_lists *pL, u32 ino, char *dest, #endif default: /* unknown */ - putLabeledWord(UNKOWN COMPRESSION METHOD = , inode-compr); + putLabeledWord(UNKNOWN COMPRESSION METHOD = , inode-compr); return -1; } } Dito. Cheers Detlev -- Ftpd never switches uid and euid, it uses setfsuid(2) instead. The main reason is that uid switching has been exploited in several breakins, but the sheer ugliness of uid switching counts too. -- pure-ftpd(8) -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 6/7] JFFS2: scanning performance improvement
Hi Baidu,
1/ Sync with kernel.
If the 256-(struct jffs2_unknown_node *) bytes are
0xff after the cleanmarker. We get the conclusion that
the sector is empty.
Signed-off-by: Baidu Liu liucai@gmail.com
---
fs/jffs2/jffs2_1pass.c | 19 +--
1 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/fs/jffs2/jffs2_1pass.c b/fs/jffs2/jffs2_1pass.c
index 62ba250..bbfab2c 100644
--- a/fs/jffs2/jffs2_1pass.c
+++ b/fs/jffs2/jffs2_1pass.c
@@ -1596,16 +1596,14 @@ jffs2_1pass_build_lists(struct part_info * part)
if (*(uint32_t *)(buf[ofs-buf_ofs]) == 0x) {
uint32_t inbuf_ofs;
- uint32_t empty_start, scan_end;
+ uint32_t empty_start;
empty_start = ofs;
ofs += 4;
- scan_end = min_t(uint32_t, EMPTY_SCAN_SIZE(
- part-sector_size)/8,
- buf_len);
+
more_empty:
inbuf_ofs = ofs - buf_ofs;
- while (inbuf_ofs scan_end) {
+ while (inbuf_ofs buf_len) {
if (*(uint32_t *)(buf[inbuf_ofs]) !=
0x)
goto scan_more;
@@ -1615,6 +1613,15 @@ jffs2_1pass_build_lists(struct part_info * part)
}
/* Ran off end. */
+ /* If we're only checking the beginning of a
block with a cleanmarker,
+bail now */
+ if((buf_ofs == sector_ofs)
+ (empty_start == sector_ofs +sizeof(struct
jffs2_unknown_node))) {
+ printf(%d bytes at start of block seems
clean... assuming all clean\n,
+
EMPTY_SCAN_SIZE(part-sector_size));
+ break;
+ }
+
This has style-problems. Actually all of your patches have style
problems. Checking them as one single patch gives:
total: 32 errors, 22 warnings, 369 lines checked
Please rework the whole series.
Thanks
Detlev
--
I have always observed that the pretensions of all people are in
exact inverse ratio to their merits; this is one of the axioms of
morals.-- Joseph Lagrange
--
DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V2] MX31: change return value of get_cpu_rev
Hi Stefano,
Drop warnings in get_cpu_rev and changes the return value
(a u32 instead of char * is returned) of the function
to be coherent with other processors.
Signed-off-by: Stefano Babic sba...@denx.de
CC: Detlev Zundel d...@denx.de
CC: Fabio Estevam fabio.este...@freescale.com
---
Changes:
- commit message
- unknown will be printed together with the
revision register if the value of the register
cannot be translated.
arch/arm/cpu/arm1136/mx31/generic.c | 31
arch/arm/include/asm/arch-mx31/imx-regs.h |2 +-
2 files changed, 19 insertions(+), 14 deletions(-)
diff --git a/arch/arm/cpu/arm1136/mx31/generic.c
b/arch/arm/cpu/arm1136/mx31/generic.c
index 18572b9..af6da91 100644
--- a/arch/arm/cpu/arm1136/mx31/generic.c
+++ b/arch/arm/cpu/arm1136/mx31/generic.c
@@ -107,18 +107,18 @@ void mx31_set_pad(enum iomux_pins pin, u32 config)
}
struct mx3_cpu_type mx31_cpu_type[] = {
- { .srev = 0x00, .v = 1.0 },
- { .srev = 0x10, .v = 1.1 },
- { .srev = 0x11, .v = 1.1 },
- { .srev = 0x12, .v = 1.15 },
- { .srev = 0x13, .v = 1.15 },
- { .srev = 0x14, .v = 1.2 },
- { .srev = 0x15, .v = 1.2 },
- { .srev = 0x28, .v = 2.0 },
- { .srev = 0x29, .v = 2.0 },
+ { .srev = 0x00, .v = 0x10 },
+ { .srev = 0x10, .v = 0x11 },
+ { .srev = 0x11, .v = 0x11 },
+ { .srev = 0x12, .v = 0x1F },
+ { .srev = 0x13, .v = 0x1F },
+ { .srev = 0x14, .v = 0x12 },
+ { .srev = 0x15, .v = 0x12 },
+ { .srev = 0x28, .v = 0x20 },
+ { .srev = 0x29, .v = 0x20 },
};
-char *get_cpu_rev(void)
+u32 get_cpu_rev(void)
{
u32 i, srev;
@@ -129,7 +129,8 @@ char *get_cpu_rev(void)
for (i = 0; i ARRAY_SIZE(mx31_cpu_type); i++)
if (srev == mx31_cpu_type[i].srev)
return mx31_cpu_type[i].v;
- return unknown;
+
+ return srev | 0x8000;
}
char *get_reset_cause(void)
@@ -161,8 +162,12 @@ char *get_reset_cause(void)
#if defined(CONFIG_DISPLAY_CPUINFO)
int print_cpuinfo (void)
{
- printf(CPU: Freescale i.MX31 rev %s at %d MHz.,
- get_cpu_rev(), mx31_get_mcu_main_clk() / 100);
+ u32 srev = get_cpu_rev();
+
+ printf(CPU: Freescale i.MX31 rev %d.%d %s at %d MHz.,
Use a space less here, i.e. rev %d.%d%s at %d...
+ (srev 0xF0) 4, (srev 0x0F),
+ ((srev 0x8000) ? unknown : ),
.. and unknown here, so we always get only one space.
Ok, ok, I'm getting carried away ;)
Cheers
Detlev
--
SWYM XYZ (Sympathize with your machinery). [..] In fact it is often
called a no-op, because it performs no operation. It does, however,
keep the machine running smoothly, just as real-world swimming helps
to keep programmers healthy. -- Donald Knuth, Fascicle 1
--
DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/7] JFFS2: Bug fix for summary support
Hi Baidu, Hi, Detlev : 2011/4/29 Detlev Zundel d...@denx.de: Hi, 1/ Get the latest DIRENT For example, if you create a file in linux jffs2 which config summary support, then you delete the file , you will not see the file in linux jffs2. But you can also see this file in uboot after you reset the system. That is because all the nodes in jffs2 which config summary will not be marked as obsolete. The deleted file's DIRENT node will be seen in uboot. So what we need to do is to get the latest DIRENT whose ino in DIRENT is 0. Sorry, but I do not understand that last sentence. Can you clarify this please? This is just give an example. If you create a file, then you DELETE it in jffs2 . In previous code where you do NOT config SUMMARY. There will be two DIRENT nodes in flash. One is obsoleted where the ino=1, another one is valid where the ino=0. When we type ls command, the file will NOT shown because uboot will not show the DIRENT where the ino =0 which means the file is deleted. But when we config SUMMARY in kernel jffs2. The two DIRENTs are BOTH valid. When we type ls command, the file will be SHOWN because we find the first DIRENT where the ino is not 0. The result is obviously wrong. The more I read the jffs2 code, the more I do not understand. In my eyes, the summary feature should just be a shortcut for not scanning the whole eraseblock, but I fail to see substantial differences. Specifically, I do not understand why your example should be a problem. So maybe you should correct me where I'm wrong: - we create a file in Linux. So we get a dirent with versino = 1, ino 0. This will be in the summary of its jeb. - we delete the file in Linux. So we get a dirent with version = 2 and ino = 0. If this is not in the same jeb than the previous dirent, then it will be in the summary of its own jeb, otherwise it will obsolete the old dirent in the summary directly. Is this correct? Now when U-Boot reads the summaries, it will enter both dirents into its list and 'jffs2_1pass_find_inode' will find the dirent with version 2 having ino = 0. So the file should not be listed. Where is this going wrong? I'm really trying hard to understand the changes, but to comment on them sensibly, I have to understand what is going on... So what we need to do is to find the DIRENT with the latest version. In this example ,the second DIRENT has highest version which means latest. And the ino is 0. Then we will not show it. Yes, I think I understand what you try to do, I explicitely said that I did not understand this sentence: So what we need to do is to get the latest DIRENT whose ino in DIRENT is 0 What does this mean? Do you mean Before we list any DIRENT, we first check if we find a later DIRENT which has ino = 0. If this is the case, then the file has been erased and should not be displayed? Alas as I wrote above, I do not understand why this is not handled already by current code. Than we will not see this file in uboot which is what we want. 2/ Add CONFIG_SYS_JFFS2_SORT_FRAGMENTS definition,if we config jffs2 summary in uboot. All the inodes of a file will not marked as obsolete, if they do not sort in the list struct b_node *, the latest data in inode may be overwritten by the older one. Also I have trouble making sense of this text. There may have many data inode in jfffs2 which represent the same range in the same file, when we use SUMMARY. They are overlapped. But only the data inode which has the latest version represent the actual data. So we need to sort the data inode, where we make the latest data inode after the obsoleted data inode. Then the latest data inode will not be overwritten when use use fsload command in uboot. Sorry, I am still not understanding this. Can you please state explicitely where you think there is an error and how your changes fix the error? Using summary or not, we can have multiple fragments for a file in a JFFS2 filesystem and the code seems to handle this fine. The summary should only be a optimization for the scanning phase, it should not behave differently. This is why I want to understand this. SUMMARY and SORT_FRAGMENTS should be orthogonal. If they are not, then we most certainly have another problem. [...] diff --git a/include/jffs2/jffs2.h b/include/jffs2/jffs2.h index 651f94c..5b006c0 100644 --- a/include/jffs2/jffs2.h +++ b/include/jffs2/jffs2.h @@ -41,6 +41,16 @@ #include asm/types.h #include jffs2/load_kernel.h +#ifdef CONFIG_JFFS2_SUMMARY +#ifndef CONFIG_SYS_JFFS2_SORT_FRAGMENTS +/* +we should define CONFIG_SYS_JFFS2_SORT_FRAGMENTS,if +CONFIG_JFFS2_SUMMARY is enabled. +*/ +#define CONFIG_SYS_JFFS2_SORT_FRAGMENTS +#endif +#endif + #define JFFS2_SUPER_MAGIC 0x72b6 /* Values we may expect to find in the 'magic' field */ If JFFS2_SUMMARY _needs_ SORT_FRAGMENTS, then we should say so, i.e. /* CONFIG_JFFS2_SUMMARY will not work correctly without
Re: [U-Boot] [PATCH 3/7] JFFS2: Calculate buf_len before we read data from flash
Hi Baidu,
[...]
diff --git a/fs/jffs2/jffs2_1pass.c b/fs/jffs2/jffs2_1pass.c
index 8eb77b1..be6ac78 100644
--- a/fs/jffs2/jffs2_1pass.c
+++ b/fs/jffs2/jffs2_1pass.c
@@ -1643,6 +1643,8 @@ jffs2_1pass_build_lists(struct part_info * part)
case JFFS2_NODETYPE_INODE:
if (buf_ofs + buf_len ofs + sizeof(struct
jffs2_raw_inode)) {
+ buf_len = min_t(uint32_t, buf_size,
sector_ofs
+ + part-sector_size - ofs);
I am somewhat uncomfortable that buf_len is used in the if condition
before it is recalculated inside. Are you sure that this cannot lead to
problems, i.e. can buf_len become larger than it was when entering the
condition?
It works well in my boards.
Aha. I am all happy for you that this works, but we really need to be
as sure as we can be that the code does the right thing. It works for
me is not enough.
Cheers
Detlev
--
Als ich entführt wurde, begannen meine Eltern aktiv zu werden.
Sie vermieteten mein Zimmer.
-- Woody Allen
--
DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 4/7] JFFS2: Improve error checking
Hi Baidu, Hi, Detlev @@ -1470,9 +1471,16 @@ jffs2_1pass_build_lists(struct part_info * part) /* lcd_off(); */ /* if we are building a list we need to refresh the cache. */ - jffs_init_1pass_list(part); + if(! jffs_init_1pass_list(part)) + return 0; + This is strange. We now check for an error of jffs2_init_1pass_list, which currently always returns 0, so let's see where you change that. Ah, you don't (it's in line 671 in this file). It's only in jffs2_nand_1pass that you do a change, but even there we have a problem: Yes, we check the return value of function jffs_init_1pass_list(). Also we add the check in nand flash. I do not konw what you are talking about. The function 'jffs_init_1pass_list' is implemented in two files, i.e. in fs/jffs2/jffs2_1pass.c and in fs/jffs2/jffs2_nand_1pass.c. Your patch inserts the actual malloc error checking only in the latter file, whereas the check for return code is done in both files. Just look at your changes - how could your new test in jffs2_1pass ever fail as you did not change the called function? This is _plain inconsistent_ - you missed to do the same error checking for the NOR flash case. diff --git a/fs/jffs2/jffs2_nand_1pass.c b/fs/jffs2/jffs2_nand_1pass.c index 9bad690..885fa3c 100644 --- a/fs/jffs2/jffs2_nand_1pass.c +++ b/fs/jffs2/jffs2_nand_1pass.c @@ -251,6 +251,7 @@ jffs_init_1pass_list(struct part_info *part) pL-dir.listCompare = compare_dirents; pL-frag.listCompare = compare_inodes; #endif + return 1; When malloc fails, we get no error output. You are too strict. Search the malloc in uboot. There are many places which do not even check the return value. This is getting more and more ridiculous. When writing new code, we always have to conform to our own standards. Now you insert an error check but if it fails, you do not inform the user that he ran out of memory but simply provide empty output? So there is _no way_ of telling a failed malloc from an empty directory? And to argument for this broken behavious you point to other places that do no error checking? Sorry, you are loosing my interest of reviewing your code. Best wishes Detlev -- Peace of mind isn't at all superficial to technical work. It's the whole thing. That which produces it is good work and that which destroys it is bad work. -- Robert M. Pirsig -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Policy for checkpatch usage?
Hi Andreas, Am Donnerstag, den 21.04.2011, 17:46 +0200 schrieb Detlev Zundel: Thanks for the input. Current base for discussion: Checkpatch is a tool that can help you find some style problems, but is imperfect, and the things it complains about are of varying importance. So use common sense in interpreting the results. Warnings that clearly only make sense in the Linux kernel can be ignored. This includes Use #include linux/$file instead of asm/$file for example. If you encounter warnings for existing code, not modified by your patch, consider submitting a separate, cosmetic-only patch -- clearly described as such -- that *precedes* your substantive patch. For minor modifications (e.g. changed arguments of a function call), adhere to the present codingstyle of the module. Relating checkpatch warnings can be ignored in this case. A respective note in the commit or cover letter why they are ignored is desired. Anyone unhappy with that? Will this help newcomers? Sounds sensible to me. The extra paragraph above is to circumvent intermixing codingstyle inside a file. For example in common/main.c, there are function (arg, arg) all around. Not checkpatch compliant (and ugly, IMHO). Now when I modify one of this calls, e.g add an argument, I'd have to change this. Which leads to a mixture of styles in the file, which is even worse than than adhering to the broken style. Thanks, I've updated the wiki page accordingly[1]. In general, I'd suggest a - maybe informal - policy that while codingstyle is important, there are more relevant aspects like elegant code, sensible interfaces, proper use of static and const and so on. I'm sure any developer is willing to debate about these issues, but most volunteers (esp. company or freelance coders) will resign after the 3rd round of whitespace ping-pong... I fully agree, but on the other hand if we do have a Codingstyle document _and_ a tool to check for it, there should be no more than one round and even this one round should only be neccessary if the poster didn't previously read the Codingstyle document ;) Not sure how to phrase that without contradicting the basic idea, maybe with an additional paragraph like: New modules have to be as checkpatch compliant as possible. Violations have to be justified in the commit or cover letter. Let's skip this. My experience is that people want clear and sharp-cut policies so that they can decide for themselves _before_ posting whether they adhere to them. If we cannot achieve such a strictness in our forumlation, I expect such clauses to generate more discussion than they solve. Thanks everyone for the input! Detlev [1] http://www.denx.de/wiki/U-Boot/Patches -- System going down at 1:45 this afternoon for disk crashing. -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v5] ARM: mx31: Print the silicon version
Hi Stefano,
On 04/15/2011 08:07 PM, Stefano Babic wrote:
On 04/12/2011 04:18 AM, Fabio Estevam wrote:
Use the same method of the Linux kernel to print the MX31 silicon version
on
boot.
Tested on a MX31PDK with a 2.0 silicon, where it shows:
CPU: Freescale i.MX31 rev 2.0 at 531 MHz
Signed-off-by: Fabio Estevam fabio.este...@freescale.com
Applied to u-boot-imx, thanks.
Fabio,
I have not noted before that your patch introduce a warning:
generic.c: In function 'get_cpu_rev':
generic.c:131: warning: return discards qualifiers from pointer target type
This is due to the usage of the const in the mx3_cpu_type:
struct mx3_cpu_type {
u8 srev;
const char *v;
Do you agree if I drop myself the const attribute on u-boot-imx before
pulling your patch to the arm tree ?
Sorry to jump in late, but why not change get_cpu_rev to 'const char *'
which it actually is?
Cheers
Detlev
--
Emacs is the way to purify your soul using garbage collection.
--
DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] MAKEALL: add -h/--help options
Hi Mike, Convert all the comments at the top of the file into help text for people to easily get at with standard -h/--help options. Signed-off-by: Mike Frysinger vap...@gentoo.org Nice! Acked-by: Detlev Zundel d...@denx.de -- .. most of the finest products of an applied mathematician's fancy must be rejected, as soon as they have been created, for the brutal but sufficient reason that they do not fit the facts.-- G. H. Hardy -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V6 2/3] PMIC: Add dialog pmic support
Hi Stefano, On 04/25/2011 04:59 AM, Jason Hui wrote: Hi, Stefano, Hi Jason, If you still want me to include the DA9053 support into fsl_pmic, could you please extend the fsl_pmic support to easily add another vender's pmic support first? Really I have not understood your question. What do you mean ? Then, Can you tell me how to add the dialog pmic support into fsl_pmi.c file which is dedicated for freescale mc13xxx? Thanks, IMHO the details for each Power Controllers are inside the corresponding header, that contains the register description, in your xace DA09053.h. The fsl_pmic.c expones only a general way to speak with the controller via SPI or I2C interface. If I see your patch, the only real difference with the original file is that a single byte is read/written with the DA09053, while the mc13xxx have 32-bit registers. You can add this option as CONFIG_ switch, or in another way. Only the board is aware about which PMIC is used. In fact, the fsl_pmic.c does not include neither mc13892.h nor mc13783.h. Then why not rename the driver to something else than fsl_pmic? Maybe this is what Jason really is asking? I have not looked any deeper, but maybe something like i2c_pmic or such... Cheers Detlev -- Don't trust everything you read, and don't assume every poster in a thread is actually relevant to the problem. -- Stefan Monnier jwvlj1gk44h.fsf-monnier+em...@gnu.org -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH,V2] JFFS2: accelerate scanning.
Hi Baidu, Hi,Detlev : 2011/4/19 Detlev Zundel d...@denx.de: Hi Baidu, Syncs up with jffs2 in the linux kernel: 1/ Change DEFAULT_EMPTY_SCAN_SIZE from 4KB to 256 Bytes. 2/ If the 1KB data is 0xFF after the cleanmarker, skip and scan the next sector. 3/ Change the buffer size from 4KB to 128KB which is the common size of erase block. There is no common size of erase block. Looking into the Linux code, it uses max(erase block size, 128k) for its buffer to speed up reading from NAND and the 128k seem to be a kmalloc limit. So maybe a increase buffer size from 4KiB to 128KiB to reduce number of read operations would be more fitting. By the way, does this change contribute to the performance increase at all, or is the increase simply due to DEFAULT_EMPTY_SCAN_SIZE? Yes, I think it is useful to speed up the scanning. Don't get me wrong, but I was not asking whether you think it speeds up the scanning. When it comes to performance, I learnt to trust numbers olnly. This may in part be because I myself occassionally was completely wrong in predicting performance issues. So I am still eager to see actual numbers if this _really_ speeds up scanning. Cheers Detlev -- Die meisten schaetzen nicht, was sie verstehen; aber was sie nicht fassen koennen, verehren sie. Um geschaetzt zu werden, muessen die Sachen Muehe kosten; daher wird geruehmt, wer nicht verstanden wird. --- Baltasar Gracian -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V2] JFFS2: bug fix for summary support.
Hi Baidu, [...] Looking over the changes, I do _see_ changes in code, so you should tell us about them. [...] diff --git a/include/jffs2/jffs2.h b/include/jffs2/jffs2.h index 651f94c..5b006c0 100644 --- a/include/jffs2/jffs2.h +++ b/include/jffs2/jffs2.h @@ -41,6 +41,16 @@ #include asm/types.h #include jffs2/load_kernel.h +#ifdef CONFIG_JFFS2_SUMMARY +#ifndef CONFIG_SYS_JFFS2_SORT_FRAGMENTS +/* +we should define CONFIG_SYS_JFFS2_SORT_FRAGMENTS,if +CONFIG_JFFS2_SUMMARY is enabled. +*/ +#define CONFIG_SYS_JFFS2_SORT_FRAGMENTS +#endif +#endif + #define JFFS2_SUPER_MAGIC 0x72b6 /* Values we may expect to find in the 'magic' field */ I liked the previous version better: diff --git a/include/jffs2/jffs2.h b/include/jffs2/jffs2.h index 651f94c..c01a76e 100644 --- a/include/jffs2/jffs2.h +++ b/include/jffs2/jffs2.h @@ -41,6 +41,17 @@ #include asm/types.h #include jffs2/load_kernel.h +#ifdef CONFIG_JFFS2_SUMMARY +#ifndef CONFIG_SYS_JFFS2_SORT_FRAGMENTS +/* + * if we define summary in jffs2, we also need to define + * CONFIG_SYS_JFFS2_SORT_FRAGMENTS. If not, the data in latest inode may be + * overwritten by the old one. +*/ +#error need to define CONFIG_SYS_JFFS2_SORT_FRAGMENTS,if summary is enabled +#endif +#endif + #define JFFS2_SUPER_MAGIC 0x72b6 Why did you change this to the worse? If we use summary in uboot, we MUST also define CONFIG_SYS_JFFS2_SORT_FRAGMENTS. The reason is : All the inodes of a file will not marked as obsolete, if they do not sort in the list struct b_node *, the latest data in inode may be overwritten by the older one. Yes, this is what you wrote in your first version and what I would like to see. My question was why you deleted the (helpful) comment in your second version... Cheers Detlev -- Wenn ein Kopf und ein Buch zusammenstossen und es klingt hohl; ist denn das allemal im Buche? - Lichtenberg -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] cmd_nvedit.c: clean up with checkpatch
Hi, Code clean up of cmd_nvedit.c by using checkpatch.pl. Thanks for taking the initiative to do some cleanup! Signed-off-by: Macpaul Lin macp...@andestech.com Still, the patch fails for me: Applying: cmd_nvedit.c: clean up with checkpatch Using index info to reconstruct a base tree... error: patch failed: common/cmd_nvedit.c:807 error: common/cmd_nvedit.c: patch does not apply Did you hand edit your patch? It does not apply to blobs recorded in its index. Cannot fall back to three-way merge. Patch failed at 0001 cmd_nvedit.c: clean up with checkpatch Cheers Detlev -- Computer scientists do it depth-first. -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 1/2] MX31: mx31pdk: Add watchdog support
Hi Fabio,
Ping?
Regards,
Fabio Estevam
On 4/10/2011 3:17 PM, Fabio Estevam wrote:
Signed-off-by: Fabio Estevam fabio.este...@freescale.com
---
Changes since v1:
- define BOARD_LATE_INIT in /mx31pdk.h
board/freescale/mx31pdk/mx31pdk.c | 16
include/configs/mx31pdk.h |3 +++
2 files changed, 19 insertions(+), 0 deletions(-)
diff --git a/board/freescale/mx31pdk/mx31pdk.c
b/board/freescale/mx31pdk/mx31pdk.c
index 3f291fc..4ef548f 100644
--- a/board/freescale/mx31pdk/mx31pdk.c
+++ b/board/freescale/mx31pdk/mx31pdk.c
@@ -28,9 +28,17 @@
#include netdev.h
#include asm/arch/clock.h
#include asm/arch/imx-regs.h
+#include watchdog.h
DECLARE_GLOBAL_DATA_PTR;
+#ifdef CONFIG_HW_WATCHDOG
+void hw_watchdog_reset(void)
+{
+mxc_hw_watchdog_reset();
+}
+#endif
+
I was just discussing this with Wolfgang again and actually you should
use CONFIG_WATCHDOG for this. The latter was meant for CPU internal
watchdog functionality, whereas CONFIG_HW_WATCHDOG was for external
(i.e. i2c or other) separate watchdog chips. I'm working on a patch to
README to make this more widely known.
int dram_init(void)
{
/* dram_init must store complete ramsize in gd-ram_size */
@@ -68,6 +76,14 @@ int board_init(void)
return 0;
}
+int board_late_init(void)
+{
+#ifdef CONFIG_HW_WATCHDOG
+mxc_hw_watchdog_enable();
+#endif
+return 0;
+}
+
int checkboard(void)
{
printf(Board: i.MX31 MAX PDK (3DS)\n);
diff --git a/include/configs/mx31pdk.h b/include/configs/mx31pdk.h
index d4c6d16..f5d3ee7 100644
--- a/include/configs/mx31pdk.h
+++ b/include/configs/mx31pdk.h
@@ -61,6 +61,7 @@
#define CONFIG_MXC_UART 1
#define CONFIG_SYS_MX31_UART1 1
+#define CONFIG_HW_WATCHDOG
#define CONFIG_HARD_SPI 1
#define CONFIG_MXC_SPI 1
@@ -98,6 +99,8 @@
*/
#undef CONFIG_CMD_IMLS
+#define BOARD_LATE_INIT
+
#define CONFIG_BOOTDELAY3
#define CONFIG_EXTRA_ENV_SETTINGS
\
Why cannot we put the code into i.mx31 specific code and only keep the
config board specific? In this patch there is (again) zero board
dependency, so all i.MX31 boards should be able to profit from the work.
Thanks
Detlev
--
So maybe I lost track of my point. But if I left a mark, C-x C-x should be
all I need.
-- David Kastrup, emacs-devel.gnu.org 87hbj37v4x@lola.goethe.zz
--
DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/2] MX31: mx31pdk: Add watchdog support
Hi Stefano, On 03/09/2011 05:35 PM, Fabio Estevam wrote: Signed-off-by: Fabio Estevam fabio.este...@freescale.com --- board/freescale/mx31pdk/mx31pdk.c | 16 include/configs/mx31pdk.h |1 + 2 files changed, 17 insertions(+), 0 deletions(-) diff --git a/board/freescale/mx31pdk/mx31pdk.c b/board/freescale/mx31pdk/mx31pdk.c index a9f0fb4..4a5d3ef 100644 Applied to u-boot-imx, thanks. Ah, obviously I'm a bit late on this. Can you please make sure that a follow-up patch according to my suggestion sent just now gets into the tree? Thanks! Detlev -- The Speedo3 is very similar to other Intel network chips, that is to say apparently designed on a different planet. -- drivers/net/eepro100.c in Linux source -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Update and Cut down mach types
Hi Reinhard, [...] why don't we just create the #define MACH_xxx lines directly from the http://www.arm.linux.org.uk/developer/machines/download.php;. We don't need all the *_is_* macros in u-boot anyway. Then we would have just a few 1000 lines of #define MACH_* ... and we could update whenever we feel like it and decouple from the linux interval. This looks like a nice clean backwards compatible solution. Cheers Detlev -- Win32 sucks so hard it could pull matter out of a Black Hole. -- Pohl Longsine -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3] cmd_nvedit.c: clean up with checkpatch
Hi, Code clean up of cmd_nvedit.c by using checkpatch.pl. Signed-off-by: Macpaul Lin macp...@andestech.com Visual inspection shows only cosmetic changes, so the patch looks good. For the next time, we should follow the (newly added) clause in our wiki[1] and have a ommit text like cosmetic: code cleanup of cmd_nvedit.c using checkpatch.pl. As this is very new, I won't insists, so Acked-by: Detlev Zundel d...@denx.de Thanks! Detlev -- Q: Does that mean we also shouldn't be using IRQF_SAMPLE_RANDOM on interrupt [sources]? A: Yes. The flag needs to be taken out and shot. -- Matt Mackall 1239116251.14392.133.camel@calx -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] 回覆: [PATCH v2] cmd_nvedit.c: clean up with checkpatch
Hi Macpaul, HI Detlev, 2011/4/27 macp...@andestech.com Hi Detlev, Signed-off-by: Macpaul Lin macp...@andestech.com Still, the patch fails for me: Applying: cmd_nvedit.c: clean up with checkpatch Using index info to reconstruct a base tree... Please check the same mail thread, I've send patch v3 already. Thanks For your quick reference, the patch v3 is here in patchworks http://patchwork.ozlabs.org/patch/92979/ I'm not sure if it is in the thread in your mail application. The mail indeed showed up in my mailer also as it has a references header. Although personally I would have preferred if this header pointed to the version 2, I see that other people prefer v1. However, it appears right the correct thread in gmail and in outlook. Maybe there is something wrong in my mailer or server's configuration. :~| Version 3 of the patch looked good and I had no problem applying it out of my inbox. Thanks! Detlev -- This reminds me about 'garbage collection is evil' arguments with people who haven't used lisp, and are in those first few years where C still feels cool. -- Brian Spilsbury -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] cmd_bdinfo.c: clean up with 2.6.38 checkpatch.pl
Hi Macpaul, clean up with 2.6.38 checkpatch.pl There is indeed only one non-whitespace hunk in there: @@ -69,8 +69,9 @@ int do_bdinfo ( cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) defined(CONFIG_440SP) || defined(CONFIG_440SPE) print_str (procfreq, strmhz(buf, bd-bi_procfreq)); print_str (plb_busfreq, strmhz(buf, bd-bi_plb_busfreq)); -#if defined(CONFIG_405GP) || defined(CONFIG_405EP) || defined(CONFIG_XILINX_405) || \ -defined(CONFIG_440EP) || defined(CONFIG_440GR) || defined(CONFIG_440SPE) || \ +#ifdefined(CONFIG_405GP) || defined(CONFIG_405EP) || \ + defined(CONFIG_XILINX_405) || defined(CONFIG_440EP) || \ + defined(CONFIG_440GR) || defined(CONFIG_440SPE) || \ defined(CONFIG_440EPX) || defined(CONFIG_440GRX) print_str (pci_busfreq, strmhz(buf, bd-bi_pci_busfreq)); #endif It's ok that you do the line wrapping, but actually with such lists we require alphabetical sorting of the clauses, so can you please juggle the list so that the XILINX_405 gets to the end? Thanks! Detlev -- The continental people think life is a game. The English think cricket is a game. -- George Mikes -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v5] ARM: mx31: Print the silicon version
Hi Stefano,
On 04/27/2011 11:11 AM, Detlev Zundel wrote:
This is due to the usage of the const in the mx3_cpu_type:
struct mx3_cpu_type {
u8 srev;
const char *v;
Do you agree if I drop myself the const attribute on u-boot-imx before
pulling your patch to the arm tree ?
Sorry to jump in late,
.. not so late, we can change it...
but why not change get_cpu_rev to 'const char *'
which it actually is?
This is correct. However, I have not noted before that the last
introduced get_cpu_rev() in MX31 is an exception in u-boot. For all
other processors, it returns a u32 and it is defined as u32
get_cpu_rev(void).
Fabio's patch introduces a variant, because it is defined as const char
*get_cpu_rev(void). I propose to change its name (get_cpu_rev_string ?)
and add the static attribute, to make clear it is different as the
get_cpu_rev() already implemented in u-boot.
I see. Can't we consolidate all those functions into more general
functions higher up in the specialization graph? I.e. why have static
non-conforming functions when we could have one generic function to
handle all users?
Cheers
Detlev
--
System going down at 1:45 this afternoon for disk crashing.
--
DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH] README: Clarify difference of CONFIG_WATCHDOG and CONFIG_HW_WATCHDOG
Now that we have the documentation, the code should be changed to reflect it ;) Asd far as I can see, these are the places where HW_WATCHDOG is used instead of WATCHDOG: arch/blackfin/cpu/blackfin/watchdog.c arch/m68k/cpu/mcf547x_8x/cpu.c The relevant maintainers are on CC. Signed-off-by: Detlev Zundel d...@denx.de Cc: Mike Frysinger vap...@gentoo.org Cc: TsiChungLiew tsi-chung.l...@freescale.com --- README | 17 - 1 files changed, 12 insertions(+), 5 deletions(-) diff --git a/README b/README index cdbb9de..63f5647 100644 --- a/README +++ b/README @@ -1,5 +1,5 @@ # -# (C) Copyright 2000 - 2009 +# (C) Copyright 2000 - 2011 # Wolfgang Denk, DENX Software Engineering, w...@denx.de. # # See file CREDITS for list of people who contributed to this @@ -719,10 +719,17 @@ The following options need to be configured: - Watchdog: CONFIG_WATCHDOG If this variable is defined, it enables watchdog - support. There must be support in the platform specific - code for a watchdog. For the 8xx and 8260 CPUs, the - SIU Watchdog feature is enabled in the SYPCR - register. + support for the SoC. There must be support in the SoC + specific code for a watchdog. For the 8xx and 8260 + CPUs, the SIU Watchdog feature is enabled in the SYPCR + register. When supported for a specific SoC is + available, then no further board specific code should + be needed to use it. + + CONFIG_HW_WATCHDOG + When using a watchdog circuitry external to the used + SoC, then define this variable and provide board + specific code for the hw_watchdog_reset function. - U-Boot Version: CONFIG_VERSION_VARIABLE -- 1.7.4.4 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Policy for checkpatch usage?
Hi Wolfgang, Dear Graeme Russ, In message BANLkTimdJkETWEOrJFuxq4hN8gG+DvY=-a...@mail.gmail.com you wrote: What about my other suggestion - A checkpatch summary with an expalation for any warnings or errors? See for example my heads-up for checkpatch warnings - http://lists.denx.de/pipermail/u-boot/2011-April/090144.html For issues like this one should do what the last lines of the checkpatch output say: If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. But as I showed, we already know that we do have false positives in U-Boot as we miss some Linux constructs. We should refrain from taking this as false positives to the checkpatch maintainers. Either we can suppress them somehow or maybe we should list them? Cheers Detlev -- Warning: this comic occasionally contains strong language (which may be unsuit- able for children), unusual humor (which may be unsuitable for adults), and ad- vanced mathematics (which may be unsuitable for liberal-arts majors). /xkcd.org -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Policy for checkpatch usage?
Hi Scott, I vote for checkpatch is a tool that can help you find some style problems, but is imperfect, and the things it complains about are of varying importance. If you insist on zero warnings, what's the difference between a warning and an error? And will there then be a U-Boot-specific coding style document to match? Will anyone that wants to submit a patch that checkpatch erroneously complains about have to first submit a patch for checkpatch (first learning Perl if need be)? So you would agree to this text: Checkpatch is a tool that can help you find some style problems, but is imperfect, and the things it complains about are of varying importance. So use common sense in interpreting the results. Warnings that clearly only make sense in the Linux kernel can be ignored. What about the problem with checkpatch errors in current code, i.e. the origin of this sentence: Also warnings produced for context lines (i.e. existing code) rather than actual changes can also be ignored. Do we want this? There's a lot more common sense that needs to be applied when writing software than where to stick what kind and amount of whitespace. Guidelines are good -- zero-tolerance obedience to a script, not so much. I agree 100%, but I also understand that people want to see some guideline that they can refer to. So let's see how good a compromise we can find. Thanks Detlev -- Some people unfortunately like jumping up and down about spaces but not code. [...] I'd rather read good poetry written in very bad hand writing than bad poetry written in beautiful handwriting, and I think the same is true of code. -- Alan Cox 20090701130018.115ce...@lxorguk.ukuu.org.uk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Policy for checkpatch usage?
Hi Fabi, At 10:49 21.04.2011 -0400, Eric Cooper wrote: On Thu, Apr 21, 2011 at 04:29:17PM +0200, Detlev Zundel wrote: What about the problem with checkpatch errors in current code, i.e. the origin of this sentence: Also warnings produced for context lines (i.e. existing code) rather than actual changes can also be ignored. How about replacing it with this: If you encounter warnings for existing code, not modified by your patch, consider submitting a separate, cosmetic-only patch -- clearly described as such -- that *precedes* your substantive patch. Is that even possible? The cosmetic patch itself will be surrounded by context lines which may fire up a warning. So these lines need to be changed as well to satisy checkpatch. But this new patch will again include several context lines... until you have to fix up the whole file. Or did I misunderstand? It may become an iterative process. Fortunately our source files are finite, so this process has a fixpoint ;) Cheers Detlev -- debian is a prototype for a future version of emacs. -- Thien-Thi Nguyen in 7eekubiffq@ada2.unipv.it -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Policy for checkpatch usage?
Hi Eric,
On Thu, Apr 21, 2011 at 04:56:36PM +0200, Fabian Cenedese wrote:
Is that even possible? The cosmetic patch itself will be surrounded
by context lines which may fire up a warning. So these lines need
to be changed as well to satisy checkpatch. But this new patch
will again include several context lines... until you have to fix up the
whole file. Or did I misunderstand?
What's wrong with (cosmetically) fixing all the files that a patch
touches? That way the project gets incremental cleanup of the code
base as it evolves.
Of course such a cleanup would be nice indeed, but I assume that many
people do not want to go this extra mile - alas I do not want to require
it.
(It would be easy to automate a check for whitespace-only patches to
ease the job of the custodians. Line-breaking and other style changes
might still require eyeballing.)
Like this?
#!/bin/sh
die() {
echo $@ 12
exit 1
}
if [ $# -lt 1 ]; then
die usage: `basename $0` patchname
fi
[ -z $(git status -s) ] || die Repository not clean - refusing to continue
git apply $1
if [ -n $(git diff -b) ]; then
echo Patch contains non-whitespace changes:
git diff -b
ret=1
else
echo Patch is only a cosmetic change
ret=0
fi
git reset --hard
exit ${ret}
Cheers
Detlev
--
Question: If you were redesigning UNIX, what would you do differently?
Ken Thompson: I'd spell creat with an e.
--
DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Policy for checkpatch usage?
Hi Eric, On Thu, Apr 21, 2011 at 04:29:17PM +0200, Detlev Zundel wrote: What about the problem with checkpatch errors in current code, i.e. the origin of this sentence: Also warnings produced for context lines (i.e. existing code) rather than actual changes can also be ignored. How about replacing it with this: Thanks for the input. Current base for discussion: Checkpatch is a tool that can help you find some style problems, but is imperfect, and the things it complains about are of varying importance. So use common sense in interpreting the results. Warnings that clearly only make sense in the Linux kernel can be ignored. This includes Use #include linux/$file instead of asm/$file for example. If you encounter warnings for existing code, not modified by your patch, consider submitting a separate, cosmetic-only patch -- clearly described as such -- that *precedes* your substantive patch. Anyone unhappy with that? Will this help newcomers? Happy easter everyone, and remember around easter some people like to hide eggs, _not_ bugs ;) Detlev -- Whenever you find yourself on the side of the majority it is time to pause and reflect. -- Mark Twain -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Update and Cut down mach types
Hi Michael, [...] I did that and got the following reply (without quotes due to cut-and-paste) [...] The normal URL which you fetch the file from (as contained within the file) will give you the full listing rather than the cut-down version. There really is no need for uboot to go picking the copy up from the mainline kernel. So we can just use that version and everything remains sane? Then let's do that and move on to other topics ;) Cheers Detlev -- If we knew what it was we were doing, we wouldn't call it research. -- Einstein -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 3/6] TFTP: rename server to remote
Hi Luca, Hi, just a few e-mails ago along this thread Albert Aribaud wrote: My opinion is that you should make sure that at least the code you touch is checkpatch-clean, so yes, you should fix that; but there is no need to submit 'checkpatch-compliance' patches. Just fix the line here so that checkpatch does not complain. So I proceeded along that way. Now Detlev Zundel wrote: ... Hm, I see. Still, can we have one commit (with cosmetic in the changelog) that silences checkpatch but does not have any functional changes? We really try hard to separate cosmetic from functional changes. This makes reviewing (and debugging) so much easier. While I appreciate the careful review of my patches, I cannot hide that it is discouraging for new contributors to be requested for contradictory modifications. Sorry for that, but it is only that we start using checkpatch more aggressively, that such problems turn up which we did not yet agree on how to solve. There should be one precise policy, and that should be clearly documented. I fully agree. http://www.denx.de/wiki/U-Boot/CodingStyle is the place where I would expect to find it. I'll start a new thread to discuss this. Hopefully we then come up with a policy to stick into that wiki page. Thanks for bearing with me Detlev -- In God we trust. All others we monitor -- NSA motto -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] UEC phy not working on first try
Hi Matthias, I am using u-boot.2010.09 on a MPC8568-based board. When trying to boot over one of the UEC network interfaces, it is not working instantly. The cable is connected from the beginning, but I have to issue a manual 'boot' command, sometimes even multiple times before the link is correct. This is the output: Do all the UEC interfaces show the same symptoms? Yes, this happens for all UECs. And I think it worked until we switched from u-boot 2009.11.1 to 2010.09 - but there's so much change that I didn't find the reason for it, yet. Ah! I didn't realize that you are in the excellent position to have one good and one bad commit. Simply use git bisect to find the problematic commit and show that to us. Best wishes Detlev -- ... and I will continue to use a low-level language to indicate how machines actually compute. Readers who only want to see algorithms that are already packaged in a plug-in way, using a trendy language, should buy other people's books. -- Donald Knuth, Fascicle 1 -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 3/6] TFTP: rename server to remote
Hi Luca,
Il 19/04/2011 16:18, Detlev Zundel ha scritto:
Hi Luca,
With the upcoming TFTP server implementation, the remote node can be
either a client or a server, so avoid ambiguities.
Signed-off-by: Luca Ceresoliluca.ceres...@comelit.it
Cc: Wolfgang Denkw...@denx.de
---
Changes in v2:
- fixed checkpatch issues.
net/tftp.c | 42 +-
1 files changed, 21 insertions(+), 21 deletions(-)
diff --git a/net/tftp.c b/net/tftp.c
index 00abec3..da545c6 100644
--- a/net/tftp.c
+++ b/net/tftp.c
@@ -55,18 +55,18 @@ enum {
TFTP_ERR_FILE_ALREADY_EXISTS = 6,
};
-static IPaddr_t TftpServerIP;
-static int TftpServerPort; /* The UDP port at their end
*/
-static int TftpOurPort;/* The UDP port at our end
*/
+static IPaddr_t TftpRemoteIP;
+static int TftpRemotePort; /* The UDP port at their end*/
+static int TftpOurPort;/* The UDP port at our end */
static intTftpTimeoutCount;
-static ulong TftpBlock; /* packet sequence number
*/
-static ulong TftpLastBlock; /* last packet sequence number
received */
-static ulong TftpBlockWrap; /* count of sequence number
wraparounds */
-static ulong TftpBlockWrapOffset;/* memory offset due to
wrapping*/
+static ulong TftpBlock; /* packet sequence number
*/
+static ulong TftpLastBlock; /* last packet sequence number received
*/
+static ulong TftpBlockWrap; /* count of sequence number wraparounds
*/
+static ulong TftpBlockWrapOffset; /* memory offset due to wrapping
*/
These changes are indentation only changes, so they should be in a
separate patch.
It's needed for checkpatch compliance.
I'm trying to understand the problems involved, but looking at this
again, it is not clear to me what you say here. When I run your version
1 of the patches (where you only do the rename) through checkpatch, I
get:
WARNING: line over 80 characters
#116: FILE: net/tftp.c:59:
+static int TftpRemotePort; /* The UDP port at their end
*/
WARNING: consider using kstrto* in preference to simple_strtol
#215: FILE: net/tftp.c:619:
+ TftpRemotePort = simple_strtol(ep, NULL, 10);
total: 0 errors, 2 warnings, 99 lines checked
/home/dzu/transfer/p2 has style problems, please review. If any of these
errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
So I'm not sure why you say that the other changes are needed for
checkpatch. What exactly do you mean by this?
Thanks
Detlev
--
C hasn't changed much since the 1970s. And let's face it it's ugly.
Can't we do better? C++? (Sorry, never mind.)
-- Rob Pike
--
DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] Policy for checkpatch usage?
Hi, currently our CodingStyle[1] has this to say about checkpatch.pl: Make sure to run the checkpatch.pl script from the Linux source tree to check your patches. Note that this should be done before posting on the mailing list! Now this is not very clear as to what should be done with regard to the results of such a checkpatch run. I think we all agree that warnings tailored to the linux kernel like this can be ignored: WARNING: consider using kstrto* in preference to simple_strtol #215: FILE: net/tftp.c:619: + TftpRemotePort = simple_strtol(ep, NULL, 10); From this it follows, that we _cannot_ require 0 warnings from checkpatch. Maybe we can require 0 _errors_? Moreover, it's only recently that I saw checkpatch warning about lines that are not even changed in a patch but are only in the context, as for exmaple here: WARNING: unnecessary whitespace before a quoted newline #293: FILE: net/net.c:1604: debug(Got ICMP ECHO REQUEST, return %d bytes \n, Now when this is fixed, the new patch will then contain a cosmetic change only intermixed with the real changes. Personally I think this is problematic and contrary to the intention of checking a _patch_, but it's the way it is. I think we all agree that we should make reviews as easy as possible. In order for that, we need to separate cosmetic from functional changes. Otherwise in large patches it is very difficult to find the meat of a patch. Only recently this exact problem made Andy Fleming resplit a large commit[2] into functional and cosmetic[3] changes (ironically the cosmetic changes were added as a followup to my request of fixing checkpatch problems). As much as this is appreciated, it is clear that it takes some effort to do. Ideally we should prevent such work right from the start. So what exact wording should we use in our CodingStyle to prevent such problems, or should we even start our own version of checkpatch tailored to our project? As a base for discussion, what about this: Use common sense in interpreting the results of checkpatch. Warnings that clearly only make sense in the Linux kernel can be ignored. Also warnings produced for _context lines_ rather than actual changes can also be ignored. Thanks Detlev [1] http://www.denx.de/wiki/view/U-Boot/CodingStyle [2] http://article.gmane.org/gmane.comp.boot-loaders.u-boot/97068 [3] http://article.gmane.org/gmane.comp.boot-loaders.u-boot/97129 -- config LGUEST If unsure, say N. If curious, say M. If masochistic, say Y. -- linux/drivers/lguest/Kconfig -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] UEC phy not working on first try
Hi Matthias, I am using u-boot.2010.09 on a MPC8568-based board. Ah! I didn't realize that you are in the excellent position to have one good and one bad commit. Simply use git bisect to find the problematic commit and show that to us. so far I traced it down to the adjust_link call in uec_init from file uec.c (line 1234 in release 2011.03). This call wasn't there in release 2009.11.1 and removing that line solves the issue here. git blame shows this commit to be responsible: commit 582c55a0274f38e6e7e35b95e7ab81d3e912f700 Author: Heiko Schocher h...@denx.de Date: Wed Jan 20 09:04:28 2010 +0100 83xx, uec: split enet_interface in two variables There's no sensible reason to unite speed and interface type into one variable. So split this variable enet_interface into two vars: enet_interface_type, which hold the interface type and speed. Also: add the possibility for switching between 10 and 100 MBit interfaces on the fly, when running in FAST_ETH mode. Signed-off-by: Heiko Schocher h...@denx.de Signed-off-by: Ben Warren biggerbadder...@gmail.com I don't know, why this went in and what the drawbacks are without it. Maybe someone on this list knows... (Sorry, I didn't search for a specific commit in git, yet. I just diffed pertinent files around the qe|uec|net drivers first.) Hm, looking at adjust_link code, it seems that for you someone set mii_info-link to 0 which gets you into the else clause of this function. On a quick grep, I cannot find any place where this happens, but I guess this is your problem (or at least one of them) ;) Maybe Heiko can contribute some more insights? Happy sunny Easter all! 8-) Same to you, thanks! Detlev -- There is no need to be rigid in carrying out policies about what changes to install. To do a good job of maintaining Emacs doesn't require acting like government bureaucrats. -- Richard Stallman e1mix3y-0005iz...@fencepost.gnu.org -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] checkpatch - theory and practice [was: [PATCH v2 3/6] TFTP: rename server to remote]
Hi Luca, It's needed for checkpatch compliance. I'm trying to understand the problems involved, but looking at this again, it is not clear to me what you say here. When I run your version 1 of the patches (where you only do the rename) through checkpatch, I get: WARNING: line over 80 characters #116: FILE: net/tftp.c:59: +static int TftpRemotePort; /* The UDP port at their end */ WARNING: consider using kstrto* in preference to simple_strtol #215: FILE: net/tftp.c:619: + TftpRemotePort = simple_strtol(ep, NULL, 10); total: 0 errors, 2 warnings, 99 lines checked /home/dzu/transfer/p2 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. So I'm not sure why you say that the other changes are needed for checkpatch. What exactly do you mean by this? All the comments were nicely columned before my patchset. Reducing the length of a line would have broken this. I chose to change all of them in order to preserve the pre-existing coding style. Ok, this makes sense, alas the wording it's needed for checkpatch compliance was somewhat misleading. Ideally only the relevant changes should be in one commit and re-indentation to align everything again should be in a separate commit. As we saw that checkpatch also looks at context lines, this commit usually needs to be logically _before_ your own changes. Probably the easiest way to achieve this is to commit the changes separately and reorder them with git rebase -i. I amended the wiki page[1] in the hope of getting more light into these things. Cheers Detlev [1] http://www.denx.de/wiki/U-Boot/Patches -- Irrationality is the square root of all evil. -- Douglas Hofstadter -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Policy for checkpatch usage?
Hi Graeme, On Wednesday, April 20, 2011, Detlev Zundel d...@denx.de wrote: Hi, As a base for discussion, what about this: Use common sense in interpreting the results of checkpatch. Warnings that clearly only make sense in the Linux kernel can be ignored. Also warnings produced for _context lines_ rather than actual changes can also be ignored. One man's common sense is another's idiocy I vote for a zero warnings, zero errors U-Boot specific checkpatch Forking checkpatch means that someone has to invest work to maintain it. Judging from the linux repo there are quite some changes in every iteration of the kernel, so this will be work for us also: [dzu@pollux linux (master)]$ git rev-list v2.6.35..v2.6.36 scripts/checkpatch.pl | wc -l 7 [dzu@pollux linux (master)]$ git rev-list v2.6.36..v2.6.37 scripts/checkpatch.pl | wc -l 19 [dzu@pollux linux (master)]$ git rev-list v2.6.37..v2.6.38 scripts/checkpatch.pl | wc -l 4 Do we want to invest this work? Do you volunteer? ;) Can't we disable individual messages not relevant for us? Like -Wno-kstrto or such things? Cheers Detlev -- The number you have dialed is imaginary. Please rotate your phone 90 degrees and try again. -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Policy for checkpatch usage?
Hi Graeme, On Wednesday, April 20, 2011, Graeme Russ graeme.r...@gmail.com wrote: On Wednesday, April 20, 2011, Detlev Zundel d...@denx.de wrote: Hi, As a base for discussion, what about this: Use common sense in interpreting the results of checkpatch. Warnings that clearly only make sense in the Linux kernel can be ignored. Also warnings produced for _context lines_ rather than actual changes can also be ignored. One man's common sense is another's idiocy I vote for a zero warnings, zero errors U-Boot specific checkpatch I also think that all patches should be submitted with a checkpatch summary with an explaination for any errors or warnings - this will at least save a little effort for the maintainers and reduce the number of patches bounced only to have the checkpatch problems argued away by the author anyway When we accept 0 errors and 0 warnings only, then we will always see the same text :) As long as we are not there, I do agree but then we should come up with a recipe on how to automate this. I looked into git format-patch but it does not seem to have such an option. Does anyone have a clever one-liner for this? Cheers Detlev -- Math and Alcohol don't mix, so... PLEASE DON'T DRINK AND DERIVE [Motto of the society: Mathematicians Against Drunk Deriving] -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] TFTP - check for len == 0 before storing
Hi David, I had a problem with an old TFTP Server which is sending a second last data block with len == 0 at end. It's clearly not RFC conform, but I still made a additional check in u-boot/tftp to avoid a wrong filesize value. This wrong filesize value caused some trouble by NAND operations. Please look at our patch guidelines[1] on how to submit patches. Especially a signed-off-by is missing on this patch. Moreover, please put the explanation into the commit log so that one can learn the rationale for the change when studying the source with git. Thanks Detlev [1] http://www.denx.de/wiki/U-Boot/Patches -- LISP has jokingly been described as the most intelligent way to misuse a computer. I think that description a great compliment because it transmits the full flavour of liberation: it has assisted a number of our most gifted fellow humans in thinking previously impossible thoughts. - Edsger W. Dijkstra -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 0/6] TFTP server
Hi Luca,
Hi,
here's the tftpsrv patch series, with checkpatch warnings fixed.
Only patches 2 and 3 are actually changed. Patch 1 is unchanged and patches
4-6
have been simply rebased.
There are still a few checkpatch issues (!), though, that I think can be
ignored
and I'm not going to fix unless there's a specific request.
Here they are:
WARNING: consider using kstrto* in preference to simple_strtol
#125: FILE: net/tftp.c:619:
+TftpRemotePort = simple_strtol(ep, NULL, 10);
Which is Linux-only
WARNING: do not add new typedefs
#61: FILE: include/net.h:367:
+typedef enum { BOOTP, RARP, ARP, TFTP, DHCP, PING, DNS, NFS, CDP, NETCONS,
SNTP,
Is a typedef *enum* considered bad in U-Boot?
For new code yes. We (well at least I) much rather prefer to see what
type a variable is without looking up the typedef for it.
Cheers
Detlev
--
Für jemanden, der in eine Religion geboren wurde, in der das Ringen um eine
einzige Seele ein Stafettenlauf über viele Jahrhunderte sein kann [..], hat
das Tempo des Christentums etwas Schwindelerregendes. Wenn der Hinduismus
friedlich dahinfließt wie der Ganges, dann ist das Christentum Toronto in
der Rushhour.-- Yann Martel
--
DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 1/6] README: remove spurious line
Hi Luca, Signed-off-by: Luca Ceresoli luca.ceres...@comelit.it Cc: Wolfgang Denk w...@denx.de --- Changes in v2: none. README |1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/README b/README index 4917e26..b9b0fcb 100644 --- a/README +++ b/README @@ -1299,7 +1299,6 @@ The following options need to be configured: driver in use must provide a function: mcast() to join/leave a multicast group. - CONFIG_BOOTP_RANDOM_DELAY - BOOTP Recovery Mode: CONFIG_BOOTP_RANDOM_DELAY Acked-by: Detlev Zundel d...@denx.de -- debian is a prototype for a future version of emacs. -- Thien-Thi Nguyen in 7eekubiffq@ada2.unipv.it -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 2/6] NET: pass source IP address to packet handlers
Hi Luca, This is needed for the upcoming TFTP server implementation. This also simplifies PingHandler() and fixes rxhand_f documentation. Signed-off-by: Luca Ceresoli luca.ceres...@comelit.it Cc: Wolfgang Denk w...@denx.de --- Changes in v2: - fixed checkpatch issues. drivers/net/netconsole.c |5 +++-- include/net.h| 15 ++- net/bootp.c |9 ++--- net/dns.c|2 +- net/net.c| 30 +- net/nfs.c|2 +- net/rarp.c |3 ++- net/sntp.c |3 ++- net/tftp.c |3 ++- 9 files changed, 44 insertions(+), 28 deletions(-) [...] diff --git a/net/net.c b/net/net.c index a609632..132f99b 100644 --- a/net/net.c +++ b/net/net.c [...] @@ -1596,11 +1598,12 @@ NetReceive(volatile uchar * inpkt, int len) * IP header OK. Pass the packet to the current handler. */ /* XXX point to ip packet */ - (*packetHandler)((uchar *)ip, 0, 0, 0); + (*packetHandler)((uchar *)ip, 0, src_ip, 0, 0); return; case ICMP_ECHO_REQUEST: - debug(Got ICMP ECHO REQUEST, return %d bytes \n, - ETHER_HDR_SIZE + len); + debug(Got ICMP ECHO REQUEST, + return %d bytes\n, + ETHER_HDR_SIZE + len); memcpy (et-et_dest[0], et-et_src[0], 6); memcpy (et-et_src[ 0], NetOurEther, 6); This second hunk is not related to the patch at hand, so strictly speaking it should not be in here. It's not enough to NAK the patch however, just something to look out for in the future. Apart from that the changes look good, so Acked-by: Detlev Zundel d...@denx.de -- LISP has jokingly been described as the most intelligent way to misuse a computer. I think that description a great compliment because it transmits the full flavour of liberation: it has assisted a number of our most gifted fellow humans in thinking previously impossible thoughts. - Edsger W. Dijkstra -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 3/6] TFTP: rename server to remote
Hi Luca,
With the upcoming TFTP server implementation, the remote node can be
either a client or a server, so avoid ambiguities.
Signed-off-by: Luca Ceresoli luca.ceres...@comelit.it
Cc: Wolfgang Denk w...@denx.de
---
Changes in v2:
- fixed checkpatch issues.
net/tftp.c | 42 +-
1 files changed, 21 insertions(+), 21 deletions(-)
diff --git a/net/tftp.c b/net/tftp.c
index 00abec3..da545c6 100644
--- a/net/tftp.c
+++ b/net/tftp.c
@@ -55,18 +55,18 @@ enum {
TFTP_ERR_FILE_ALREADY_EXISTS = 6,
};
-static IPaddr_t TftpServerIP;
-static int TftpServerPort; /* The UDP port at their end
*/
-static int TftpOurPort;/* The UDP port at our end
*/
+static IPaddr_t TftpRemoteIP;
+static int TftpRemotePort; /* The UDP port at their end*/
+static int TftpOurPort;/* The UDP port at our end */
static int TftpTimeoutCount;
-static ulong TftpBlock; /* packet sequence number
*/
-static ulong TftpLastBlock; /* last packet sequence number received
*/
-static ulong TftpBlockWrap; /* count of sequence number wraparounds
*/
-static ulong TftpBlockWrapOffset;/* memory offset due to wrapping
*/
+static ulong TftpBlock; /* packet sequence number */
+static ulong TftpLastBlock; /* last packet sequence number received */
+static ulong TftpBlockWrap; /* count of sequence number wraparounds */
+static ulong TftpBlockWrapOffset; /* memory offset due to wrapping */
These changes are indentation only changes, so they should be in a
separate patch.
static int TftpState;
#ifdef CONFIG_TFTP_TSIZE
-static int TftpTsize; /* The file size reported by the server
*/
-static short TftpNumchars; /* The number of hashes we printed
*/
+static int TftpTsize; /* The file size reported by the server */
+static short TftpNumchars; /* The number of hashes we printed */
dito.
[...]
@@ -421,7 +421,7 @@ TftpHandler(uchar *pkt, unsigned dest, IPaddr_t sip,
unsigned src,
/*
* Acknoledge the block just received, which will prompt
- * the server for the next one.
+ * the remote for the next one.
Hey, while you're at it, please fix the Acknoledge typo ;)
[...]
@@ -568,7 +568,7 @@ TftpStart (void)
strncpy(tftp_filename, BootFile, MAX_LEN);
tftp_filename[MAX_LEN-1] = 0;
} else {
- TftpServerIP = string_to_ip (BootFile);
+ TftpRemoteIP = string_to_ip(BootFile);
Whitespace fix.
Apart from that, patch looks simple enough, so
Acked-by: Detlev Zundel d...@denx.de
--
14474011154664524427946373126085988481573677491474835889066354349131199152128
If you know why this number is perfect - you're probably a mathematician...
--
DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 4/6] TFTP: rename STATE_RRQ to STATE_SEND_RRQ
Hi Luca, With the upcoming TFTP server implementation, requests can be either outgoing or incoming, so avoid ambiguities. Signed-off-by: Luca Ceresoli luca.ceres...@comelit.it Cc: Wolfgang Denk w...@denx.de --- Changes in v2: none. Acked-by: Detlev Zundel d...@denx.de -- We can forgive a man for making a useful thing as long as he does not admire it. The only excuse for making a useless thing is that one admires it intensely. --- Oscar Wilde -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 5/6] TFTP: net/tftp.c: add server mode receive
Hi Luca, Signed-off-by: Luca Ceresoli luca.ceres...@comelit.it Cc: Wolfgang Denk w...@denx.de --- Changes in v2: none. Only one comment below net/tftp.c | 72 --- net/tftp.h |6 + 2 files changed, 74 insertions(+), 4 deletions(-) diff --git a/net/tftp.c b/net/tftp.c index e166a63..87eb0b8 100644 --- a/net/tftp.c +++ b/net/tftp.c @@ -2,6 +2,8 @@ * Copyright 1994, 1995, 2000 Neil Russell. * (See License) * Copyright 2000, 2001 DENX Software Engineering, Wolfgang Denk, w...@denx.de + * Copyright 2011 Comelit Group SpA, + * Luca Ceresoli luca.ceres...@comelit.it */ #include common.h @@ -74,6 +76,7 @@ static shortTftpNumchars; /* The number of hashes we printed */ #define STATE_TOO_LARGE 3 #define STATE_BAD_MAGIC 4 #define STATE_OACK 5 +#define STATE_RECV_WRQ 6 #define TFTP_BLOCK_SIZE 512 /* default TFTP block size */ #define TFTP_SEQUENCE_SIZE ((ulong)(116))/* sequence number is 16 bit */ @@ -241,6 +244,10 @@ TftpSend (void) TftpBlock=ext2_find_next_zero_bit(Bitmap,(Mapsize*8),0); /*..falling..*/ #endif + +#ifdef CONFIG_CMD_TFTPSRV + case STATE_RECV_WRQ: +#endif case STATE_DATA: xp = pkt; s = (ushort *)pkt; @@ -293,7 +300,11 @@ TftpHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src, #endif return; } - if (TftpState != STATE_SEND_RRQ src != TftpRemotePort) + if (TftpState != STATE_SEND_RRQ +#ifdef CONFIG_CMD_TFTPSRV + TftpState != STATE_RECV_WRQ +#endif + src != TftpRemotePort) return; Hm, I have to admit that I do not really like adding so many ifdefs into the tftp code and even into the state machine code. Can you give me a number on how much larger u-boot gets on your platform with and without the server support? If this is not too much, maybe we should include support always. If it is too much, maybe we can at least keep the state machine without ifdefs - if I see it correctly, this will only add at maximum a few bytes and STATE_RECW_WRQ will simply never be entered, correct? Cheers Detlev -- Zivilisation ist der Zaubertrick, der uns unsere wahre Natur verbirgt. -- Salman Rushdie -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 6/6] TFTP: add tftpsrv command
Hi Luca, Signed-off-by: Luca Ceresoli luca.ceres...@comelit.it Cc: Wolfgang Denk w...@denx.de --- Changes in v2: none. Acked-by: Detlev Zundel d...@denx.de -- I have always observed that the pretensions of all people are in exact inverse ratio to their merits; this is one of the axioms of morals.-- Joseph Lagrange -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V2] JFFS2: bug fix for summary support.
Hi Baidu, This patch fixes some issues with JFFS2 summary support in U-Boot. 1/ Bug fix for summary support: we need to get the latest DIRENT. For example, if you create a file in linux jffs2 which config summary support, then you delete the file , you will not see the file in linux jffs2. But you can also see this file in uboot after you reset the system. That is because all the nodes in jffs2 which config summary will not be marked as obsolete. The deleted file's DIRENT node will be seen in uboot. So what we need to do is to get the latest DIRENT whose ino in DIRENT is 0. Than we will not see this file in uboot which is what we want. 2/ Add CONFIG_SYS_JFFS2_SORT_FRAGMENTS definition,if we config jffs2 summary in uboot. 3/ Avoid allocate too big memory if the biggest file in JFFS2 is too long. We only allocate one node size for pL-readbuf. For example, if the biggest file in the jffs2 is 15MB in the 16MB jffs2 system. Our previous code will allocate a buffer(pL-readbuf) as 15MB length. 4/ Improve error checking in the scanning. I missed saying this explicitely in my first review that we should strive to isolate changes into single commits. As you have added even more changes into a single commit this has now become somewhat untolerable. Please split into individual functional changes (git add -i can work wonders here), i.e. the list items in the changelog should become individual commits. This also includes the added WATCHDOG_RESET not mentioned in the changelog at all ;) Changes for v2: - Add detail description for the patch. Are you sure you only changed the description? The diffstat from this and the last time disagree - this time: --- fs/jffs2/jffs2_1pass.c | 60 +- fs/jffs2/jffs2_nand_1pass.c | 28 +++- include/jffs2/jffs2.h | 10 +++ 3 files changed, 67 insertions(+), 31 deletions(-) Last time: fs/jffs2/jffs2_1pass.c | 53 +- fs/jffs2/jffs2_nand_1pass.c | 24 ++- include/jffs2/jffs2.h | 11 + 3 files changed, 60 insertions(+), 28 deletions(-) Looking over the changes, I do _see_ changes in code, so you should tell us about them. [...] diff --git a/include/jffs2/jffs2.h b/include/jffs2/jffs2.h index 651f94c..5b006c0 100644 --- a/include/jffs2/jffs2.h +++ b/include/jffs2/jffs2.h @@ -41,6 +41,16 @@ #include asm/types.h #include jffs2/load_kernel.h +#ifdef CONFIG_JFFS2_SUMMARY +#ifndef CONFIG_SYS_JFFS2_SORT_FRAGMENTS +/* +we should define CONFIG_SYS_JFFS2_SORT_FRAGMENTS,if +CONFIG_JFFS2_SUMMARY is enabled. +*/ +#define CONFIG_SYS_JFFS2_SORT_FRAGMENTS +#endif +#endif + #define JFFS2_SUPER_MAGIC 0x72b6 /* Values we may expect to find in the 'magic' field */ I liked the previous version better: diff --git a/include/jffs2/jffs2.h b/include/jffs2/jffs2.h index 651f94c..c01a76e 100644 --- a/include/jffs2/jffs2.h +++ b/include/jffs2/jffs2.h @@ -41,6 +41,17 @@ #include asm/types.h #include jffs2/load_kernel.h +#ifdef CONFIG_JFFS2_SUMMARY +#ifndef CONFIG_SYS_JFFS2_SORT_FRAGMENTS +/* + * if we define summary in jffs2, we also need to define + * CONFIG_SYS_JFFS2_SORT_FRAGMENTS. If not, the data in latest inode may be + * overwritten by the old one. +*/ +#error need to define CONFIG_SYS_JFFS2_SORT_FRAGMENTS,if summary is enabled +#endif +#endif + #define JFFS2_SUPER_MAGIC 0x72b6 Why did you change this to the worse? Cheers Detlev -- Q: Why did the chicken cross the Moebius strip? A: To get to the other ... er, um ... -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH,V2] JFFS2: accelerate scanning.
Hi Baidu, Syncs up with jffs2 in the linux kernel: 1/ Change DEFAULT_EMPTY_SCAN_SIZE from 4KB to 256 Bytes. 2/ If the 1KB data is 0xFF after the cleanmarker, skip and scan the next sector. 3/ Change the buffer size from 4KB to 128KB which is the common size of erase block. There is no common size of erase block. Looking into the Linux code, it uses max(erase block size, 128k) for its buffer to speed up reading from NAND and the 128k seem to be a kmalloc limit. So maybe a increase buffer size from 4KiB to 128KiB to reduce number of read operations would be more fitting. By the way, does this change contribute to the performance increase at all, or is the increase simply due to DEFAULT_EMPTY_SCAN_SIZE? Also as for the other patch, can you split the commit into the individual changes corresponding to the list items? In this way, one could also easily measure which change really speeds up the operation... Thanks! Detlev -- It is practically impossible to teach good programming to students that have had a prior exposure to BASIC: as potential programmers they are mentally mutilated beyond hope of regeneration.-- Edsger Dijkstra -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v5] MX31: Introduce get_reset_cause()
Hi Fabio, Signed-off-by: Fabio Estevam fabio.este...@freescale.com --- Changes since v4: - Make get_reset_cause CPU specific code and no need to add any board specific call. arch/arm/cpu/arm1136/mx31/generic.c | 29 - 1 files changed, 28 insertions(+), 1 deletions(-) Thanks, this looks good. Now all i.MX31 boards profit! Acked-by: Detlev Zundel d...@denx.de -- config LGUEST If unsure, say N. If curious, say M. If masochistic, say Y. -- linux/drivers/lguest/Kconfig -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] UEC phy not working on first try
Hi, Hi all- I am using u-boot.2010.09 on a MPC8568-based board. When trying to boot over one of the UEC network interfaces, it is not working instantly. The cable is connected from the beginning, but I have to issue a manual 'boot' command, sometimes even multiple times before the link is correct. This is the output: Do all the UEC interfaces show the same symptoms? [- snip -] Net: UEC0, UEC1, UEC3 [PRIME], eTSEC0, eTSEC1 Hit any key to stop autoboot: 0 UEC: PHY is Generic MII (70431) UEC3: Full Duplex switching to rgmii 100 UEC3: Speed 100BT UEC3: Link is up UEC3: Link is down The driver thinks the link went away again. When the cable is connected all the time, then this is an indication of something going wrong. You should try to diagnose, why the software gets this link change and if it really is an information that the phy provides, if the physical path to the phy is stable. Best wishes Detlev Zundel -- Those who do not understand Unix are condemned to reinvent it, poorly. - Henry Spencer, University of Toronto Unix hack -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 3/6] TFTP: rename server to remote
Hi Luca, [...] Whitespace fix. checkpatch again. Hm, I see. Still, can we have one commit (with cosmetic in the changelog) that silences checkpatch but does not have any functional changes? We really try hard to separate cosmetic from functional changes. This makes reviewing (and debugging) so much easier. This is the second time that I personally encounter that problem and I still don't see an automated way to take care of this. This simply is the result that checkpatch will flag only new errors when there are no _old_ ones. The reality of course looks different: [dzu@pollux u-boot-testing (master)]$ ../linux/scripts/checkpatch.pl -f net/net.c [...] total: 76 errors, 193 warnings, 1934 lines checked net/net.c has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. If we strictly do require checkpatch cleanliness, maybe we should start cleaning up the code base as is first with only cosmetic changes. Do I see volunteers? ;) Cheers Detlev -- Life is complex. It has real and imaginary components. -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 2/3] MX31: Introduce get_reset_cause()
Hi Fabio and Stefan, On 04/15/2011 04:21 AM, Fabio Estevam wrote: Introduce get_reset_cause() function to indicate the source of the reset. Signed-off-by: Fabio Estevam fabio.este...@freescale.com --- arch/arm/cpu/arm1136/mx31/generic.c | 26 ++ arch/arm/include/asm/arch-mx31/imx-regs.h |2 ++ 2 files changed, 28 insertions(+), 0 deletions(-) Hi Fabio, diff --git a/arch/arm/include/asm/arch-mx31/imx-regs.h b/arch/arm/include/asm/arch-mx31/imx-regs.h index 7f2..2b0881d 100644 --- a/arch/arm/include/asm/arch-mx31/imx-regs.h +++ b/arch/arm/include/asm/arch-mx31/imx-regs.h @@ -27,6 +27,8 @@ #if !(defined(__KERNEL_STRICT_NAMES) || defined(__ASSEMBLY__)) #include asm/types.h +extern char *get_reset_cause(void); + The imx-regs.h contains only defines and structures for the processor, and no prototypes. Function prototypes are then listed in sys_proto.h, not only for i.MX processor. I prefer to maintain this rule and to not add prototypes to this file. Why can't we have this code in the cpu specific code _without_ any call from a board so _ecery_ board gets the additional information for free? See for example arch/powerpc/cpu/mpc512x/cpu.c where this is done for RSR on 5121. I do not understand what the problem is here, can someone enlighten me please? Cheers Detlev -- The X approach to device independence is to treat everything like a MicroVAX framebuffer on acid. -- The UNIX Haters Handbook -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V3 1/2] MX5: factor out boot cause funciton to common code
Hi Stefano,
On 04/15/2011 02:47 PM, Fabio Estevam wrote:
+char *get_reset_cause(void)
+{
+u32 cause;
+struct src *src_regs = (struct src
*)SRC_BASE_ADDR;
+
+cause = readl(src_regs-srsr);
You need to mask the 7 LSB of SRSR register.
If you don´t bit 16 can still affect its result.
Why ? As this becomes a general function for i.MX5, should we not
provide a way to check all significant bits ? Why should we exclude the
warm boot bit to be checked and printed out ?
And _please_ (as indictated in my i.MX31 mail) use the code for _all_
iMX51 boards withoput the need for them to call a function and print the
result.
Thanks!
Detlev
--
Choosing which tool to use is a problem for most users. Therefore
when one tool came along that did everything Perl (Ugly) it took over.
-- Rob Pike
--
DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 2/2] MX31: mx31pdk: Print the cause of reset
Hi Fabio,
Signed-off-by: Fabio Estevam fabio.este...@freescale.com
---
Changes since v1:
- Use 3 bits for rcsr mask
board/freescale/mx31pdk/mx31pdk.c | 25 -
1 files changed, 24 insertions(+), 1 deletions(-)
diff --git a/board/freescale/mx31pdk/mx31pdk.c
b/board/freescale/mx31pdk/mx31pdk.c
index 4ef548f..5fc6319 100644
--- a/board/freescale/mx31pdk/mx31pdk.c
+++ b/board/freescale/mx31pdk/mx31pdk.c
@@ -86,7 +86,30 @@ int board_late_init(void)
int checkboard(void)
{
- printf(Board: i.MX31 MAX PDK (3DS)\n);
+ u32 cause;
+ struct clock_control_regs *ccm =
+ (struct clock_control_regs *)CCM_BASE;
+ puts(Board: MX31PDK [);
+
+ cause = ccm-rcsr 0x07;
+ switch (cause) {
+ case 0x:
+ puts(POR);
+ break;
+ case 0x0001:
+ puts(RST);
+ break;
+ case 0x0002:
+ puts(WDOG);
+ break;
+ case 0x0006:
+ puts(JTAG);
+ break;
+ default:
+ puts(unknown);
+ }
+
+ puts(]\n);
return 0;
}
Didn't we agree to move this into CPU specific code so other i.MX31
boards will profit from it as well? Can you remind me why this does not
happen?
Cheers
Detlev
--
Don't trust everything you read, and don't assume every poster in
a thread is actually relevant to the problem.
-- Stefan Monnier jwvlj1gk44h.fsf-monnier+em...@gnu.org
--
DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/3] Initialize second PHY on OpenRD-Client and OpenRD-Ultimate.
Hi Clint, (rework of Julian Pidancet's patch) --- board/Marvell/openrd_base/openrd_base.c | 24 include/configs/openrd_base.h | 14 -- 2 files changed, 32 insertions(+), 6 deletions(-) Sorry to jump in a little bit late here, but can you please look at the excellent work of Andy Flemin on an universal phy infrastructure[1]? Can't we use that also as a base for your board and eliminate duplicated phy code? Thanks for looking into this! Detlev [1] http://article.gmane.org/gmane.comp.boot-loaders.u-boot/97560 -- ;; Self-replicator in ELisp ((lambda (l) (prin1-to-string (list l (list (quote quote) l (quote (lambda (l) (prin1-to-string (list l (list (quote quote) l)) -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] JFFS2: bug fix for summary support.
;
+ if(! jffs_init_1pass_list(part))
+ return 0;
+
+ pL = (struct b_lists *)part-jffs2_priv;
buf = malloc(buf_size);
+ if (!buf) {
+ printf(jffs2_1pass_build_lists: malloc failed\n);
+ return 0;
+ }
+
puts (Scanning JFFS2 FS: );
/* start at the beginning of the partition */
Ah I see, jffs_init_1pass_list can now make the process fail. This is
worth mentioning in the commit log, i.e. improve error checking or
something. After all, this changes how the code behaves, so it should
be documented.
Cheers
Detlev
--
NAN - No Acronym Neccessary
--
DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] JFFS2: accelerate scanning.
-offset + ofs, buf_len,
buf);
buf_ofs = ofs;
goto more_empty;
}
+
if (node-magic != JFFS2_MAGIC_BITMASK ||
!hdr_crc(node)) {
ofs += 4;
@@ -1650,6 +1657,8 @@ jffs2_1pass_build_lists(struct part_info * part)
case JFFS2_NODETYPE_INODE:
if (buf_ofs + buf_len ofs + sizeof(struct
jffs2_raw_inode)) {
+ buf_len = min_t(uint32_t, buf_size,
sector_ofs
+ + part-sector_size -
ofs);
get_fl_mem((u32)part-offset + ofs,
buf_len, buf);
buf_ofs = ofs;
@@ -1671,6 +1680,8 @@ jffs2_1pass_build_lists(struct part_info * part)
((struct
jffs2_raw_dirent *)
node)-nsize) {
+ buf_len = min_t(uint32_t, buf_size,
sector_ofs
+ + part-sector_size -
ofs);
get_fl_mem((u32)part-offset + ofs,
buf_len, buf);
buf_ofs = ofs;
diff --git a/fs/jffs2/jffs2_nand_1pass.c b/fs/jffs2/jffs2_nand_1pass.c
index e3bc536..16bb2e9 100644
--- a/fs/jffs2/jffs2_nand_1pass.c
+++ b/fs/jffs2/jffs2_nand_1pass.c
@@ -833,7 +833,7 @@ jffs2_1pass_build_lists(struct part_info * part)
return 0;
ofs = 0;
- /* Scan only 4KiB of 0xFF before declaring it's empty */
+ /* Scan only 1KiB of 0xFF before declaring it's empty */
while (ofs EMPTY_SCAN_SIZE *(uint32_t *)(buf[ofs]) ==
0x)
ofs += 4;
if (ofs == EMPTY_SCAN_SIZE)
On a whole - is this optimization safe with regard to uncompressed files
containing only 0xFFs?
Cheers
Detlev
--
One of my most productive days was throwing away 1000 lines of code.
- Ken Thompson.
--
DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] MX51: mx51evk: Use struct to access SRSR register instead of offset
Hi Fabio, Use struct to access SRSR register instead of offset. While at it mask the 7 bits of SRSR for correctness. Signed-off-by: Fabio Estevam fabio.este...@freescale.com The SRSR is a MX51 register and thus not board specific. Can we please move this reset cause detection into common mx51 code so other boards will profit from it also? Thanks Detlev -- (7) It is always something (7a) (corollary). Good, Fast, Cheap: Pick any two (you can't have all three). -- The Twelve Networking Truths (RFC 1925) -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v5 0/9] Universal PHY Infrastructure
Hi Andy, Or PHY Lib for U-Boot. I've given acked-bys to parts of the changes that you sent already. As you already ignored them the last time, I'm not reposting them. If you want, you can add them yourself of course. Cheers Detlev -- Modern methods of production have given us the possibility of ease and security for all; we have chosen, instead, to have overwork for some and starvation for others. -- Bertrand Russell -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v5 8/9] fsl: Change fsl_phy_enet_if to phy_interface_t
Hi Andy, The fsl_phy_enet_if enum was, essentially, the phy_interface_t enum. This meant that drivers which used fsl_phy_enet_if to deal with PHY interfaces would have to convert between the two (or we would have to have them mirror each other, and deal with the ensuing maintenance headache). Instead, we switch all clients of fsl_phy_enet_if over to phy_interface_t, which should become the standard, anyway. Signed-off-by: Andy Fleming aflem...@freescale.com Looks good, so Acked-by: Detlev Zundel d...@denx.de Cheers Detlev -- Ich glaube, daß die Weltanschauung, die aus der modernen Physik hervorgeht, mit unserer gegenwärtigen Gesellschaft unvereinbar ist, weil sie den har- monischen Zusammenhängen, die wir in der Natur beaobachten, nicht Rechnung trägt. -- Fritjof Capra -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
