Re: email address of Boris Brezillon
On Thu, 12 Dec 2019 10:00:06 +0100 Heinrich Schuchardt wrote: > On 12/12/19 9:32 AM, Simon Goldschmidt wrote: > > Kind of off-topic, but Boris's address at Bootlin doesn't exist anymore and > > I > > keep getting mail delivery error responses. > > > > Do we have any kind of marking such addresses as "don't use" to patman so > > this > > won't happen in the future? > > File .mailmap could be used to map to Boris's new mail address. > > Linux's .mailmap has these entries: > > Boris Brezillon > Boris Brezillon > Boris Brezillon > Boris Brezillon > > since patch 7677ea0e8843e1a45e35253c0c5e22db11a99a62. > > @Boris > Will it be ok for you if we also route U-Boot mails to your kernel.org > address? Sure, no problem.
Re: [U-Boot] [PATCH v4 00/20] SF: Migrate to Linux SPI NOR framework
[PM] Hi Vignesh, On Fri, 8 Feb 2019 18:36:05 +0530 Vignesh R wrote: > On 07/02/19 6:13 PM, Jagan Teki wrote: > > On Tue, Feb 5, 2019 at 11:29 AM Vignesh R wrote: > [...] > >> > >> Vignesh R (20): > >> configs: Move CONFIG_SPI_FLASH into defconfigs > >> bitops: Fix GENMASK definition for Sandbox > >> spi: spi-mem: Allow use of spi_mem_exec_op for all SPI modes > >> spi: spi-mem: Extend spi_mem_adjust_op_size() to honor max xfer size > >> spi: spi-mem: Claim SPI bus before spi mem access > >> spi: Add non DM version of SPI_MEM > >> sh: bitops: add hweight*() macros > >> mtd: spi: Port SPI NOR framework from Linux > >> mtd: spi: spi-nor-core: Add SPI MEM support > >> mtd: spi: spi-nor-core: Add 4 Byte addressing support > >> mtd: spi: spi-nor-core: Add SFDP support > >> mtd: spi: spi-nor-core: Add back U-Boot specific features > >> mtd: spi: sf_probe: Add "jedec,spi-nor" compatible string > >> mtd: spi: Switch to new SPI NOR framework > >> mtd: spi: Remove unused files > >> mtd: spi: Add lightweight SPI flash stack for SPL > >> spl: Kconfig: Enable SPI_FLASH_TINY by default for SPL > >> configs: Remove SF_DUAL_FLASH > >> configs: Don't use SPI_FLASH_BAR as default > >> MAINTAINERS: Add an entry for SPI NOR > > > > Update trivial change on this patch. > > > > Applied to u-boot-spi/master, thanks for the big changes and welcome aboard! > > > > Thanks Jagan! Thanks all for testing out this series > Kudos for getting this spi-nor port merged in u-boot! I hope you'll continue contributing to the Linux stack even though you're now the maintainer of the uboot version ;-). Regards, Boris ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v4 00/20] SF: Migrate to Linux SPI NOR framework
On Wed, 13 Feb 2019 14:42:57 +0100 Boris Brezillon wrote: > [PM] Not so private actually :D. > Hi Vignesh, > > On Fri, 8 Feb 2019 18:36:05 +0530 > Vignesh R wrote: > > > On 07/02/19 6:13 PM, Jagan Teki wrote: > > > On Tue, Feb 5, 2019 at 11:29 AM Vignesh R wrote: > > [...] > > >> > > >> Vignesh R (20): > > >> configs: Move CONFIG_SPI_FLASH into defconfigs > > >> bitops: Fix GENMASK definition for Sandbox > > >> spi: spi-mem: Allow use of spi_mem_exec_op for all SPI modes > > >> spi: spi-mem: Extend spi_mem_adjust_op_size() to honor max xfer size > > >> spi: spi-mem: Claim SPI bus before spi mem access > > >> spi: Add non DM version of SPI_MEM > > >> sh: bitops: add hweight*() macros > > >> mtd: spi: Port SPI NOR framework from Linux > > >> mtd: spi: spi-nor-core: Add SPI MEM support > > >> mtd: spi: spi-nor-core: Add 4 Byte addressing support > > >> mtd: spi: spi-nor-core: Add SFDP support > > >> mtd: spi: spi-nor-core: Add back U-Boot specific features > > >> mtd: spi: sf_probe: Add "jedec,spi-nor" compatible string > > >> mtd: spi: Switch to new SPI NOR framework > > >> mtd: spi: Remove unused files > > >> mtd: spi: Add lightweight SPI flash stack for SPL > > >> spl: Kconfig: Enable SPI_FLASH_TINY by default for SPL > > >> configs: Remove SF_DUAL_FLASH > > >> configs: Don't use SPI_FLASH_BAR as default > > >> MAINTAINERS: Add an entry for SPI NOR > > > > > > Update trivial change on this patch. > > > > > > Applied to u-boot-spi/master, thanks for the big changes and welcome > > > aboard! > > > > > > > Thanks Jagan! Thanks all for testing out this series > > > > Kudos for getting this spi-nor port merged in u-boot! > I hope you'll continue contributing to the Linux stack even though > you're now the maintainer of the uboot version ;-). > > Regards, > > Boris ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH 2/3] spi: Add support for the Aspeed ast2500 SPI controllers
+Vignesh Hi Cédric, On Fri, 25 Jan 2019 18:28:02 +0100 Cédric Le Goater wrote: > Hello > > On 10/10/18 2:02 PM, Cédric Le Goater wrote: > > Hello Boris, > > > > On 10/10/18 9:32 AM, Boris Brezillon wrote: > >> Hi Cédric, > >> > >> On Wed, 10 Oct 2018 11:46:56 +0530 > >> Jagan Teki wrote: > >> > >>> On Mon, Oct 8, 2018 at 11:32 AM Cédric Le Goater wrote: > >>>> > >>>> On 10/4/18 5:57 PM, Jagan Teki wrote: > >>>>> On Fri, Sep 28, 2018 at 5:20 PM Cédric Le Goater wrote: > >>>>> > >>>>>> > >>>>>> Hello Simon, > >>>>>> > >>>>>> > >>>>>> The Aspeed AST2500 FMC controller can handle SPI flash and NOR flash > >>>>>> memory, > >>>>>> and the Aspeed AST2500 SPI Flash Controllers only SPI. If there is some > >>>>>> misunderstanding on this driver, it might come from the fact it is > >>>>>> closer > >>>>>> to a SPI-NOR driver like we have in Linux, than a generic SPI driver. > >>>>>> The stm32 SPI driver is somewhat similar. > >>>>>> > >>>>>> Should we move it under drivers/mtd/spi/ maybe ? > >>>>> > >>>>> Seems with new spi-mem in Linux flash memory driver rely on spi-mem > >>>>> instead of mtd/spi-nor. So I think you can handle this via new > >>>>> spi-mem. have you send any patches to Linux? > >>>> > >>>> No, not yet. The patchset is sent : > >>>> > >>>> https://patchwork.ozlabs.org/cover/933293/ > >>>> > >>>> is not using spimem. I was not aware of that change in the spi-nor layer > >>>> at the time. I will take a look. > >> > >> Indeed, if you have some time to convert the Linux aspeed driver to > >> the spi-mem interface that would be appreciated. > > > > Yes. That's the plan. I have a series on the way but I will see if I can > > rework a v2 to use spi-mem. > > I took a look at spi-mem and worked on an initial spi-aspeed-smc driver > using the new framework in Linux 5.0.x. It looks good but I have a couple > of questions. Great! > > > The first is about performing direct accesses on the AHB window on which > the flash contents is mapped. We have introduced the dirmap API/interface exactly for this purpose, and the SPI NOR layer will use it in 5.1 (see [1]). > > How do you distinguish a flash read (fast, dual, etc) from a RDSFPD command > for instance ? Why do you need to know the access type? > Are the drivers expected to check the SPI OP command and > depending on the target/command redirect to the appropriate address space ? Definitely not, the SPI MEM layer is supposed to be memory-type agnostic, so you should not guess the operation type based on the opcode. For direct mapping accesses, just implement the ->dirmap_xxx hooks at the controller level and you'll be able to use the feature. > > Also, Aspeed SPI controllers have a Read Timing Compensation Register which > defines different data input delay cycles depending on SPI clock rates. This > register is supposed to be tuned when the flash chip characteristics are > known, after the first bus scan. Is there a way to know that our SPI slave > is alive and well detected before starting hammering successive reads on it > to see how it behaves. Vignesh mentioned that a while back (couldn't find the thread where this discussion happened) and I suggested adding a new hook to do this "link training" process where you'd pass a spi_mem_op template + the expected result so that the controller can test different setups until it finds a working one. > > > I think the U-Boot and Linux driver will be very similar wrt the issues > above ? I hope so. Thanks for working on this conversion. Boris [1]https://patchwork.kernel.org/patch/10670755/ ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH 03/16] spi: Add non DM version of SPI_MEM
On Thu, 13 Dec 2018 04:40:30 +0530 Jagan Teki wrote: > > I do really understand your intention about the real question. > - Any code or generic code will add in U-Boot should be driver-model > driven, are you agree this point? > Yes- thanks. > No - we need to have separate discussion. Depends on what you mean by driver-model driven. Yes, applying the DM sometimes makes sense, but blindly trying to push it everywhere just for the sake of being "DM compliant" is a huge mistake IMO. One example of the thing you suggested which didn't make sense at all: force MTD users to manipulate udevice objects instead of mtd_info ones. > > Any code that related to spi, or spi-flash should be driver-model > driven, ie what my AIM as a Maintainer (ie only reason for my spi-nor > changes resist for long time to fit). You seem to use the term "driver-model" a lot without clearly explaining what you have in mind. The driver-model should be used where it makes sense, but some of your suggestions don't make any sense to me. Like the proposal to add a SPI NOR uclass while we already have an MTD uclass which works just fine for all kind of flash devices. Oh, and this strict rule that says "don't provide wrappers to handle non-DM compliant cases" is just wrong. As I said, none of these dummy wrappers prevent you from enforcing DM_SPI conversion, and it allows us to support existing HW while getting rid of the old code base. Plus, I suggested to declare the spi-nor driver as an MTD uclass so it's not like we're completely ignoring your comments :P. ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH 03/16] spi: Add non DM version of SPI_MEM
On Thu, 13 Dec 2018 02:15:16 +0530 Jagan Teki wrote: > On Thu, Dec 13, 2018 at 2:10 AM Boris Brezillon > wrote: > > > > Hi Jagan, > > > > On Thu, 13 Dec 2018 01:55:08 +0530 > > Jagan Teki wrote: > > > > > On Wed, Dec 12, 2018 at 11:08 PM Vignesh R wrote: > > > > > > > > Add non DM version of SPI_MEM to support easy migration to new SPI NOR > > > > framework. This can be removed once DM_SPI conversion is complete. > > > > > > Our intention to use new driver to follow dm, why we need to support > > > non-dm? any usecases? > > > > Looks like we're having the same discussion over and over. Vignesh is > > dropping spi_flash.c which AFAICT was not depending on DM_SPI, so, if > > we want to keep everyone happy while getting rid of some legacy code, > > that's the only solution. DM conversion is a nice goal, but it's kind > > of orthogonal to what Vignesh is working on. If DM_SPI conversion > > happens before the spi-nor stuff is merged (which I doubt) then this > > patch can simply be dropped. > > spi_flash.c is a core code not a specific driver it belongs. spi-mem > is new feature driver how come new driver will support legacy non-dm > do we have legacy use for that(ie what I'm asking about usecase) I recommend that you read the spi-mem code carefully. spi-mem is not driver specific, it's a thin layer on top of spi and driver *can* (but are not forced to) provide optimized methods to execute spi-mem operations. When that's not the case, the implementation falls back to regular spi transfers. AFAIK, both DM and non-DM drivers support regular spi transfers, right? So why should we depend on DM_SPI? And more importantly, if we do that, that means we can't get rid of spi_flash.c since some users might still have non-DM SPI drivers, which in turn means we keep more legacy code for no good reasons. You want non-DM SPI controller drivers to go away, then remove them, instead of blocking other changes using this excuse. ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH 03/16] spi: Add non DM version of SPI_MEM
On Wed, 12 Dec 2018 22:07:44 +0100 Jagan Teki wrote: > On Wed 12 Dec, 2018, 10:02 PM Boris Brezillon wrote: > > > On Thu, 13 Dec 2018 02:15:16 +0530 > > Jagan Teki wrote: > > > > > On Thu, Dec 13, 2018 at 2:10 AM Boris Brezillon > > > wrote: > > > > > > > > Hi Jagan, > > > > > > > > On Thu, 13 Dec 2018 01:55:08 +0530 > > > > Jagan Teki wrote: > > > > > > > > > On Wed, Dec 12, 2018 at 11:08 PM Vignesh R > > wrote: > > > > > > > > > > > > Add non DM version of SPI_MEM to support easy migration to new SPI > > NOR > > > > > > framework. This can be removed once DM_SPI conversion is > > complete. > > > > > > > > > > Our intention to use new driver to follow dm, why we need to support > > > > > non-dm? any usecases? > > > > > > > > Looks like we're having the same discussion over and over. Vignesh is > > > > dropping spi_flash.c which AFAICT was not depending on DM_SPI, so, if > > > > we want to keep everyone happy while getting rid of some legacy code, > > > > that's the only solution. DM conversion is a nice goal, but it's kind > > > > of orthogonal to what Vignesh is working on. If DM_SPI conversion > > > > happens before the spi-nor stuff is merged (which I doubt) then this > > > > patch can simply be dropped. > > > > > > spi_flash.c is a core code not a specific driver it belongs. spi-mem > > > is new feature driver how come new driver will support legacy non-dm > > > do we have legacy use for that(ie what I'm asking about usecase) > > > > I recommend that you read the spi-mem code carefully. spi-mem is not > > driver specific, it's a thin layer on top of spi and driver *can* (but > > are not forced to) provide optimized methods to execute spi-mem > > operations. When that's not the case, the implementation falls back to > > regular spi transfers. AFAIK, both DM and non-DM drivers support > > regular spi transfers, right? So why should we depend on DM_SPI? And > > more importantly, if we do that, that means we can't get rid of > > spi_flash.c since some users might still have non-DM SPI drivers, which > > in turn means we keep more legacy code for no good reasons. > > > > I understand spi-mem is core file, but new code too. Sorry, I don't get it. > > > > You want non-DM SPI controller drivers to go away, then remove them, > > instead of blocking other changes using this excuse. > > > > Please understand uboot development flow, legacy driver can be removed if > possible once migration expire and NEW drivers or code must be dm driven. Sorry, but I think you're the one misunderstanding what we are trying to do here. Vignesh changes have simply no impact on the DM SPI conversion you're aiming at. All Vignesh does is provide a dummy wrapper for non-DM drivers, which would probably have been implemented by Miquel if you had not been so insistent on your precious DM_SPI conversion. That was not really a problem for spi-nand, as we were adding support for a new feature. This is not the case here. SPI NORs are already partially supported by the u-boot spi flash layer, and we need to keep things in a working state for those that were using it and didn't have their SPI controller drivers converted to the DM. This leaves us 2 options: 1/ keep the sf_flash code as is and add a new spi-nor code base 2/ replace spi_flash code by the spi-nor layer imported from Linux Vignesh chose option #2 which has the benefit of avoiding code duplication. Given the discussion we're having right now, I'm wondering if it wouldn't be easier to go for option #1 in order to avoid those endless discussions... ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH 03/16] spi: Add non DM version of SPI_MEM
Hi Jagan, On Thu, 13 Dec 2018 01:55:08 +0530 Jagan Teki wrote: > On Wed, Dec 12, 2018 at 11:08 PM Vignesh R wrote: > > > > Add non DM version of SPI_MEM to support easy migration to new SPI NOR > > framework. This can be removed once DM_SPI conversion is complete. > > Our intention to use new driver to follow dm, why we need to support > non-dm? any usecases? Looks like we're having the same discussion over and over. Vignesh is dropping spi_flash.c which AFAICT was not depending on DM_SPI, so, if we want to keep everyone happy while getting rid of some legacy code, that's the only solution. DM conversion is a nice goal, but it's kind of orthogonal to what Vignesh is working on. If DM_SPI conversion happens before the spi-nor stuff is merged (which I doubt) then this patch can simply be dropped. ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH 10/16] configs: Get rid of SPI_FLASH_BAR
On Thu, 13 Dec 2018 02:11:48 +0530 Jagan Teki wrote: > On Wed, Dec 12, 2018 at 11:15 PM Vignesh R wrote: > > > > Now that we have new SPI NOR framework in place that supports 4 byte > > addressing mode by default, get rid of CONFIG_SPI_FLASH_BAR > > I already mentioned in previous mail, BAR is not exact replacement for > 4-byte. Some controllers do handle > 16MB flashes even-though > controller support 3-byte addressing. we have these board since from > 2014. That's something you should be able to detect with spi_mem_supports_op(). If the controller does not support sending 4 byte addresses, it should return -ENOTSUPP and the framework should fallback to BAR setting (if the NOR supports it). Do we really need a config option for that? ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH 09/16] sf_mtd: Simply mtd operations
On Wed, 12 Dec 2018 23:02:21 +0530
Vignesh R wrote:
> @@ -39,13 +37,12 @@ static int spi_flash_mtd_erase(struct mtd_info *mtd,
> struct erase_info *instr)
> static int spi_flash_mtd_read(struct mtd_info *mtd, loff_t from, size_t len,
> size_t *retlen, u_char *buf)
> {
> - struct spi_flash *flash = mtd->priv;
> int err;
>
> - if (!flash)
> + if (!mtd || !mtd->priv)
> return -ENODEV;
>
> - err = spi_flash_read(flash, from, len, buf);
> + err = mtd->_read(mtd, from, len, retlen, buf);
Please use the wrappers instead of calling those hooks directly.
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH 2/2] mtd: Get rid of board_mtdparts_default()
On Wed, 12 Dec 2018 12:37:04 +0100
Ladislav Michl wrote:
> Now problem is that IGEPv2 comes with quite many configurations, some of
> them are even customized, so static configuration is a show stopper
> mainly as I do not know what devices are in field.
> Another issue is how ubispl code works: It expects struct ubispl_info
> filled with (among others) peb_offset of ubi partition. ubispl code counts
> in terms of eraseblocks regardless of their size. So we would need to touch
> this number when using static mtdparts.
Okay.
>
> > > Hence runtime detection. That code could be used
> > > on all OMAP3 boards as BootROM reads up to first four sectors searching
> > > for SPL (MLO).
> >
> > Note that, for the nand side of things, you can also automate that using
> > a u-boot script:
> >
> > nand info; setexpr splsize ${nand_erasesize} * 4; setenv mtdparts
> > mtdparts=omap2-nand:0x${splsize}(SPL),-(UBI)
>
> That seems as a way to go!
Glad you like this idea.
>
> > Shouldn't be hard to patch the onenand cmd to also expose writesize,
> > erasesize and oobsize.
>
> Side note: I never fully understand why is OneNAND using separate set of
> commands.
Hehe. That's exactly what Miquel tries to address with the mtd command
(one command to rule them all).
>
> Could you hold merging your paches until I implement above idea and test
> it on a few boards? I know u-boot is now using two months merge window,
> which is unfortunate, so I'll try to do it as soon as possible, but I do
> not think I'll finish it till end of week.
No worries. This is not urgent and can definitely wait 2019.04.
Thanks,
Boris
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH 2/2] mtd: Get rid of board_mtdparts_default()
Hi Ladislav,
On Tue, 11 Dec 2018 23:55:26 +0100
Ladislav Michl wrote:
> Hi Boris,
>
> On Mon, Dec 10, 2018 at 04:38:50PM +0100, Boris Brezillon wrote:
> > The only implementer of this function has been patched to use
> > CONFIG_MTD{IDS,PARTS}_DEFAULT instead. Let's get rid of this function
> > and the associated CONFIG_SYS_MTDPARTS_RUNTIME option.
>
> the only implementer of this fuction did so for a good reason. What is
> a motivation to remove it?
Simplifying the code (see this discussion [1] which led me to send
this patchset).
>
> The requirement is to be able to use single u-boot binary on all igep2
> boards ever produced. These comes with various NAND and OneNAND chips
> and I was not able to come with single static partition configuration
> to support them all.
That's actually the question I asked Enric in [1]. Can you list all
the memory organization you have (for NAND and OneNAND chips)? I mean,
the SPL part size depends on the NAND/OneNAND erase block size, and
board vendors try to use similar flashes when they source different
parts (same page size, same block size, ...). Assuming this is the
case, you should always have the same layout for OneNAND/NAND devices,
hence my proposal to define those parts statically.
> Hence runtime detection. That code could be used
> on all OMAP3 boards as BootROM reads up to first four sectors searching
> for SPL (MLO).
Note that, for the nand side of things, you can also automate that using
a u-boot script:
nand info; setexpr splsize ${nand_erasesize} * 4; setenv mtdparts
mtdparts=omap2-nand:0x${splsize}(SPL),-(UBI)
Shouldn't be hard to patch the onenand cmd to also expose writesize,
erasesize and oobsize.
Regards,
Boris
[1]https://www.mail-archive.com/u-boot@lists.denx.de/msg304933.html
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v4 1/6] common: command: Fix command auto-completion
Hi Simon,
On Mon, 10 Dec 2018 18:04:20 -0700
Simon Glass wrote:
> Hi Boris,
>
> On Mon, 3 Dec 2018 at 14:54, Boris Brezillon
> wrote:
> >
> > When auto-completing command arguments, the last argument is not
> > necessarily the one we need to auto-complete. When the last character is
> > a space, a tab or '\0' what we want instead is list all possible values,
> > or if there's only one possible value, place this value on the command
> > line instead of trying to suffix the last valid argument with missing
> > chars.
> >
> > Signed-off-by: Boris Brezillon
> > Reviewed-by: Tom Rini
> > ---
> > Changes in v4:
> > -None
> >
> > Changes in v3:
> > - Add Tom's R-b
> >
> > Changes in v2:
> > - None
> > ---
> > common/command.c | 12 ++--
> > 1 file changed, 10 insertions(+), 2 deletions(-)
>
> I wonder if you might be able to add a test for this into test/py/tests ?
This is what I made, but I'm really bad at writing python scripts, so
don't hesitate to propose propose something else (especially for the
autocomp_command() method).
Regards,
Boris
--->8---
diff --git a/test/py/tests/test_autocomp.py b/test/py/tests/test_autocomp.py
new file mode 100644
index ..cb93cf7e8a87
--- /dev/null
+++ b/test/py/tests/test_autocomp.py
@@ -0,0 +1,18 @@
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2018 Bootlin
+
+import pytest
+import u_boot_utils
+
+@pytest.mark.buildconfigspec('auto_complete', 'cmd_memory', 'cmd_importenv')
+def test_env_print_autocomp(u_boot_console):
+"""Test that env print auto-completion works as expected."""
+
+ram_base = u_boot_utils.find_ram_base(u_boot_console)
+addr = '%08x' % ram_base
+u_boot_console.run_command('mw.l ' + addr + ' 0x0')
+u_boot_console.run_command('env import -d -b ' + addr)
+u_boot_console.run_command('env set foo bar')
+expected_response = 'foo'
+response = u_boot_console.autocomp_command('printenv ', 'foo')
+assert(expected_response in response)
diff --git a/test/py/u_boot_console_base.py b/test/py/u_boot_console_base.py
index 326b2ac51fbf..b8a2d0c84cda 100644
--- a/test/py/u_boot_console_base.py
+++ b/test/py/u_boot_console_base.py
@@ -137,6 +137,53 @@ class ConsoleBase(object):
self.p.close()
self.logstream.close()
+def autocomp_command(self, cmd, expect):
+"""Try to auto-complete a command
+
+The command is not executed, we just try to auto-complete it by
+appending a \t at the end.
+
+Args:
+cmd: The command to auto-complete.
+expect: The expected auto-completion.
+"""
+
+if self.at_prompt and \
+self.at_prompt_logevt != self.logstream.logfile.cur_evt:
+self.logstream.write(self.prompt, implicit=True)
+
+try:
+self.at_prompt = False
+cmd += '\t'
+while cmd:
+# Limit max outstanding data, so UART FIFOs don't overflow
+chunk = cmd[:self.max_fifo_fill]
+cmd = cmd[self.max_fifo_fill:]
+self.p.send(chunk)
+escchunk = chunk.replace('\t', '')
+m = self.p.expect([escchunk] + self.bad_patterns)
+if m != 0:
+self.at_prompt = False
+raise Exception('Bad pattern found on console: ' +
+self.bad_pattern_ids[m - 1])
+
+m = self.p.expect([expect])
+if m != 0:
+self.at_prompt = False
+raise Exception('Bad pattern found on console: ' +
+self.bad_pattern_ids[m - 1])
+self.at_prompt = True
+self.at_prompt_logevt = self.logstream.logfile.cur_evt
+# Only strip \r\n; space/TAB might be significant if testing
+# indentation.
+return self.p.after.strip('\r\n')
+except Exception as ex:
+self.log.error(str(ex))
+self.cleanup_spawn()
+raise
+finally:
+self.log.timestamp()
+
def run_command(self, cmd, wait_for_echo=True, send_nl=True,
wait_for_prompt=True):
"""Execute a command via the U-Boot console.
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH 2/2] mtd: Get rid of board_mtdparts_default()
The only implementer of this function has been patched to use
CONFIG_MTD{IDS,PARTS}_DEFAULT instead. Let's get rid of this function
and the associated CONFIG_SYS_MTDPARTS_RUNTIME option.
Signed-off-by: Boris Brezillon
---
board/isee/igep00x0/igep00x0.c | 17 -
cmd/mtdparts.c | 6 --
drivers/mtd/mtd_uboot.c | 10 ++
include/configs/omap3_igep00x0.h | 2 --
scripts/config_whitelist.txt | 1 -
5 files changed, 2 insertions(+), 34 deletions(-)
diff --git a/board/isee/igep00x0/igep00x0.c b/board/isee/igep00x0/igep00x0.c
index 367af82d4a16..3552be6f3902 100644
--- a/board/isee/igep00x0/igep00x0.c
+++ b/board/isee/igep00x0/igep00x0.c
@@ -239,20 +239,3 @@ int misc_init_r(void)
return 0;
}
-
-void board_mtdparts_default(const char **mtdids, const char **mtdparts)
-{
- struct mtd_info *mtd = get_mtd_device(NULL, 0);
- if (mtd) {
- static char ids[24];
- static char parts[48];
- const char *linux_name = "omap2-nand";
- if (strncmp(mtd->name, "onenand0", 8) == 0)
- linux_name = "omap2-onenand";
- snprintf(ids, sizeof(ids), "%s=%s", mtd->name, linux_name);
- snprintf(parts, sizeof(parts), "mtdparts=%s:%dk(SPL),-(UBI)",
-linux_name, 4 * mtd->erasesize >> 10);
- *mtdids = ids;
- *mtdparts = parts;
- }
-}
diff --git a/cmd/mtdparts.c b/cmd/mtdparts.c
index f7ed1a077974..6b5644523898 100644
--- a/cmd/mtdparts.c
+++ b/cmd/mtdparts.c
@@ -122,9 +122,6 @@ DECLARE_GLOBAL_DATA_PTR;
#define MTDPARTS_DEFAULT NULL
#endif
#endif
-#if defined(CONFIG_SYS_MTDPARTS_RUNTIME)
-extern void board_mtdparts_default(const char **mtdids, const char **mtdparts);
-#endif
static const char *mtdids_default = MTDIDS_DEFAULT;
static const char *mtdparts_default = MTDPARTS_DEFAULT;
@@ -1733,9 +1730,6 @@ int mtdparts_init(void)
memset(last_ids, 0, sizeof(last_ids));
memset(last_parts, 0, sizeof(last_parts));
memset(last_partition, 0, sizeof(last_partition));
-#if defined(CONFIG_SYS_MTDPARTS_RUNTIME)
- board_mtdparts_default(_default, _default);
-#endif
use_defaults = 1;
initialized = 1;
}
diff --git a/drivers/mtd/mtd_uboot.c b/drivers/mtd/mtd_uboot.c
index d638f700d041..ed619abac390 100644
--- a/drivers/mtd/mtd_uboot.c
+++ b/drivers/mtd/mtd_uboot.c
@@ -13,8 +13,6 @@
#define MTD_NAME_MAX_LEN 20
-void board_mtdparts_default(const char **mtdids, const char **mtdparts);
-
static const char *get_mtdids(void)
{
__maybe_unused const char *mtdparts = NULL;
@@ -23,9 +21,7 @@ static const char *get_mtdids(void)
if (mtdids)
return mtdids;
-#if defined(CONFIG_SYS_MTDPARTS_RUNTIME)
- board_mtdparts_default(, );
-#elif defined(MTDIDS_DEFAULT)
+#if defined(MTDIDS_DEFAULT)
mtdids = MTDIDS_DEFAULT;
#elif defined(CONFIG_MTDIDS_DEFAULT)
mtdids = CONFIG_MTDIDS_DEFAULT;
@@ -133,9 +129,7 @@ static const char *get_mtdparts(void)
if (mtdparts || !use_defaults)
return mtdparts;
-#if defined(CONFIG_SYS_MTDPARTS_RUNTIME)
- board_mtdparts_default(, );
-#elif defined(MTDPARTS_DEFAULT)
+#if defined(MTDPARTS_DEFAULT)
mtdparts = MTDPARTS_DEFAULT;
#elif defined(CONFIG_MTDPARTS_DEFAULT)
mtdparts = CONFIG_MTDPARTS_DEFAULT;
diff --git a/include/configs/omap3_igep00x0.h b/include/configs/omap3_igep00x0.h
index b9d65697521b..280a094cdbae 100644
--- a/include/configs/omap3_igep00x0.h
+++ b/include/configs/omap3_igep00x0.h
@@ -87,8 +87,6 @@
#endif
-#define CONFIG_SYS_MTDPARTS_RUNTIME
-
/* OneNAND config */
#define CONFIG_USE_ONENAND_BOARD_INIT
#define CONFIG_SYS_ONENAND_BASEONENAND_MAP
diff --git a/scripts/config_whitelist.txt b/scripts/config_whitelist.txt
index b8addeaf693a..72608071c486 100644
--- a/scripts/config_whitelist.txt
+++ b/scripts/config_whitelist.txt
@@ -3511,7 +3511,6 @@ CONFIG_SYS_MRAM_SIZE
CONFIG_SYS_MSC0_VAL
CONFIG_SYS_MSC1_VAL
CONFIG_SYS_MSC2_VAL
-CONFIG_SYS_MTDPARTS_RUNTIME
CONFIG_SYS_MX5_CLK32
CONFIG_SYS_MX5_HCLK
CONFIG_SYS_MX6_CLK32
--
2.17.1
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH 1/2] configs: igep: Define default mtdids/mtdparts
We are trying to get rid of the weak board_mtdparts_default() function
and we need to make sure igep defconfigs have proper proper
CONFIG_MTD{IDS,PARTS}_DEFAULT before doing that.
Signed-off-by: Boris Brezillon
---
configs/igep0032_defconfig | 2 ++
configs/igep00x0_defconfig | 2 ++
2 files changed, 4 insertions(+)
diff --git a/configs/igep0032_defconfig b/configs/igep0032_defconfig
index 383648789c53..d2a614c98f6d 100644
--- a/configs/igep0032_defconfig
+++ b/configs/igep0032_defconfig
@@ -28,6 +28,8 @@ CONFIG_CMD_SPI=y
CONFIG_CMD_CACHE=y
CONFIG_CMD_EXT4_WRITE=y
CONFIG_CMD_MTDPARTS=y
+CONFIG_MTDIDS_DEFAULT="nand0=omap2-nand,onenand0=omap2-onenand"
+CONFIG_MTDPARTS_DEFAULT="omap2-nand:512k(SPL),-(UBI);omap2-onenand:512k(SPL),-(UBI)"
CONFIG_CMD_UBI=y
# CONFIG_CMD_UBIFS is not set
CONFIG_NET_RANDOM_ETHADDR=y
diff --git a/configs/igep00x0_defconfig b/configs/igep00x0_defconfig
index f2989e34e12e..5d3e109ee3c2 100644
--- a/configs/igep00x0_defconfig
+++ b/configs/igep00x0_defconfig
@@ -28,6 +28,8 @@ CONFIG_CMD_SPI=y
CONFIG_CMD_CACHE=y
CONFIG_CMD_EXT4_WRITE=y
CONFIG_CMD_MTDPARTS=y
+CONFIG_MTDIDS_DEFAULT="nand0=omap2-nand,onenand0=omap2-onenand"
+CONFIG_MTDPARTS_DEFAULT="omap2-nand:512k(SPL),-(UBI);omap2-onenand:512k(SPL),-(UBI)"
CONFIG_CMD_UBI=y
# CONFIG_CMD_UBIFS is not set
CONFIG_NET_RANDOM_ETHADDR=y
--
2.17.1
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [RFC PATCH v2 00/11] SF: Migrate to Linux SPI NOR framework
On Mon, 10 Dec 2018 18:32:09 +0530 Jagan Teki wrote: > On Thu, Dec 6, 2018 at 11:55 PM Vignesh R wrote: > > > > Hi Jagan, > > > > On 06-Dec-18 10:44 PM, Jagan Teki wrote: > > > On Tue, Dec 4, 2018 at 5:56 PM Vignesh R wrote: > > >> > > >> U-Boot SPI NOR support (sf layer) is quite outdated as it does not > > >> support 4 byte addressing opcodes, SFDP table parsing and different > > >> types of > > >> quad mode enable sequences. Many newer flashes no longer support BANK > > >> registers used by sf layer to a access >16MB space. > > >> Also, many SPI controllers have special MMIO interfaces which provide > > >> accelerated read/write access but require knowledge of flash parameters > > >> to make use of it. Recent spi-mem layer provides a way to support such > > >> flashes but sf layer isn't using that. > > >> This patch series syncs SPI NOR framework from Linux v4.19. It also adds > > >> spi-mem support on top. > > >> So, we gain 4byte addressing support and SFDP support. This makes > > >> migrating to U-Boot MTD framework easier. > > > > > > We(someone) has proposed this sync before, but we(at-least I) rely on > > > implementing via DM not direct sync to Linux. > > > > As I said in my cover letter, U-Boot sf layer is unable to support newer > > flashes mainly due to lack of 4 byte addressing and proper support for > > MMIO capable SPI controllers. > > My idea of fixing this is to borrow _features_ from Linux SPI NOR "as > > is". All that's needed is stateless 4 byte addressing, SFDP > > parsing(optionally), Quad/Octal support and spi-mem like abstraction for > > MMIO capable Controllers. I see no point in re-coding them from ground up. > > > > Could you be more specific on what you would like to see here in DM way? > > I have no issues in adapting this code to any framework here in U-Boot. > > Linux has driver model and SPI NOR subsystem is a framework and > > therefore any code ported from Linux will inherently have those > > abstractions. The only difference I see wrt your code in branch below vs > > this series is SPI-NOR uclass. This can be easily achieved by moving > > nor->ops out of struct spi_nor into uclass abstraction. > > Upstream Linux is anyways merging m25p80 and spi-nor so I did not see a > > need for SPI NOR uclass. I am okay to change that if you insist on > > having it. > > Merging or syncing spi-nor features stuff from Linux is good, I'm not > stopping that. but this can be do by satisfying u-boot driver-model > with proper architectural model. I know you take care but I'm not sure > ie what can be manageable for long term. > > Let's discuss the proper architectural model, so-that we can move > further to incorporate the changes accordingly. (thanks at last we > have a thread to discuss) Having discussions about the long term plan is good, as long as it's not blocking support for new features for too long. Look, you've been discussing the spi-nor stuff for more than 1 year now, and people are still waiting it. Yogesh's attempt seems to go in the right direction, so let's not block that just because we don't know how to integrate things in the DM (that's not entirely BTW, I suggested an approach in one of my previous reply). > > Linux m25p80 is moved to spi-nor right? so does controllers on spi-nor > should be reside in same area like drivers/mtd/spi-nor or it should be > part of spi-mem. The last mail with Boris, noted all spi-nor can't be > fit into spi-mem(sorry I lost the thread) > > Example: we have zynq qspi it support BAR(with >16MB flashes), dual > qspi ect so does it comes under spi-mem or spi-nor? Everything should go in drivers/spi/ and be exposed as spi/spi-mem controllers. So yes, that's one aspect where Linux and u-boot should differ IMO, at least until we have all SPI NOR controller drivers converted to spi-mem in Linux. > > So, if no driver should be part of spi-nor and all can be handle > spi-mem even-though they have controller specific features, yes we can > skip SPI_NOR_UCLASS otherwise we need spi-nor uclass that can be child > uclass of MTD. That's the idea, yes. Note that I'm open to any discussion regarding the needed features and how we want expose that at the spi-mem level. ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v4 00/11] mtd/sf: Various fixes
+Tom for the question about missing SoBs.
Hi Jagan,
On Thu, 6 Dec 2018 00:48:47 +0530
Jagan Teki wrote:
> On Sun, Dec 2, 2018 at 3:25 PM Boris Brezillon
> wrote:
> >
> > Hello,
> >
> > This is the 4th version of the mtd / sf fixes patchset. This v4 just
> > adds a new check in del_mtd_device() (and a debug() when
> > del_mtd_partitions() fails).
> >
> > Regards,
> >
> > Boris
> >
> > P.S.: travis-ci results =>
> > https://travis-ci.org/bbrezillon/u-boot/builds/461943011
> >
> > Boris Brezillon (11):
> > mtd: Add a function to report when the MTD dev list has been updated
> > mtd: Parse mtdparts/mtdids again when the MTD list has been updated
> > mtd: Delete partitions attached to the device when a device is deleted
> > mtd: sf: Make sure we don't register the same device twice
> > mtd: Use get_mtdids() instead of env_get("mtdids") in
> > mtd_search_alternate_name()
> > mtd: Be more strict on the "mtdparts=" prefix check
> > mtd: Make sure the name passed in mtdparts fits in mtd_name[]
> > mtd: Make sure we don't parse MTD partitions belonging to another dev
> > mtd: Don't stop MTD partition creation when it fails on one device
> > mtd: sf: Unregister the MTD device prior to removing the spi_flash obj
> > mtd: sf: Make sf_mtd.c more robust
>
> Applied to u-boot-spi/master
I see that those patches have been merged in mainline, which is great.
Just have one question: looks like your SoB is missing while you're
clearly reported as the committer. Since I found the same problem on
2614a208471e ("common: command: tempory buffer should have size of
command line buf"), I wanted to know if this is a common practice in
u-boot to not add SoBs when maintainers apply patches to their tree?
Regards,
Boris
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [RFC PATCH v2 00/11] SF: Migrate to Linux SPI NOR framework
On Thu, 6 Dec 2018 23:55:30 +0530 Vignesh R wrote: > On 06-Dec-18 10:44 PM, Jagan Teki wrote: > > On Tue, Dec 4, 2018 at 5:56 PM Vignesh R wrote: > >> > >> U-Boot SPI NOR support (sf layer) is quite outdated as it does not > >> support 4 byte addressing opcodes, SFDP table parsing and different types > >> of > >> quad mode enable sequences. Many newer flashes no longer support BANK > >> registers used by sf layer to a access >16MB space. > >> Also, many SPI controllers have special MMIO interfaces which provide > >> accelerated read/write access but require knowledge of flash parameters > >> to make use of it. Recent spi-mem layer provides a way to support such > >> flashes but sf layer isn't using that. > >> This patch series syncs SPI NOR framework from Linux v4.19. It also adds > >> spi-mem support on top. > >> So, we gain 4byte addressing support and SFDP support. This makes > >> migrating to U-Boot MTD framework easier. > > > > We(someone) has proposed this sync before, but we(at-least I) rely on > > implementing via DM not direct sync to Linux. > > As I said in my cover letter, U-Boot sf layer is unable to support newer > flashes mainly due to lack of 4 byte addressing and proper support for > MMIO capable SPI controllers. > My idea of fixing this is to borrow _features_ from Linux SPI NOR "as > is". All that's needed is stateless 4 byte addressing, SFDP > parsing(optionally), Quad/Octal support and spi-mem like abstraction for > MMIO capable Controllers. I see no point in re-coding them from ground up. > > Could you be more specific on what you would like to see here in DM way? > I have no issues in adapting this code to any framework here in U-Boot. > Linux has driver model and SPI NOR subsystem is a framework and > therefore any code ported from Linux will inherently have those > abstractions. The only difference I see wrt your code in branch below vs > this series is SPI-NOR uclass. This can be easily achieved by moving > nor->ops out of struct spi_nor into uclass abstraction. > Upstream Linux is anyways merging m25p80 and spi-nor so I did not see a > need for SPI NOR uclass. I am okay to change that if you insist on > having it. > No, please don't add a SPI_NOR_UCLASS. We already have the MTD_UCLASS which is barely used by drivers, so if anything, we should work on converting drivers to this UCLASS and maybe automate a few more things for MTD drivers (like MTD dev registration/deregistration [1]) instead of adding yet another UCLASS. If you need an example of how integration with the DM (and the MTD UCLASS) should be been done, you can look at the SPI NAND driver [2]. [1]http://code.bulix.org/9lart9-519406 [2]http://git.denx.de/?p=u-boot.git;a=blob;f=drivers/mtd/nand/spi/core.c;h=cb8ffa3fa96a16dd40dda3553bbb171107e71df7;hb=HEAD#l1249 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v3 13/28] mtd: ensure MTD is compiled when ENV_IS_IN_FLASH is selected
Hello Wolfgang, On Thu, 06 Dec 2018 09:06:05 +0100 Wolfgang Denk wrote: > Dear Miquel, > > In message <20181205153218.36f6ed4e@xps13> you wrote: > > > > I took a rather small configuration: stm32f429-discovery_defconfig > > which uses CONFIG_MTD_NOR_FLASH. Without CONFIG_MTD, u-boot.bin is > > 209416 bytes. With CONFIG_MTD, u-boot.bin is 214540 bytes. This is an > > additional 5124 bytes which represent about 2% of the entire size. > > So it's a 5 k code increase for zero benefit. There's no short term benefit, but the long term benefit is ease better maintainability. > > > I am talking about U-Boot only, there is a CONFIG_SPL_MTD_SUPPORT > > option that must be enabled to compile MTD in the SPL so the change > > I propose do not enlarge the SPL. > > This is even worse - the SPL is frequently at the size limits. I think you missed the *no*. There's no overhead at all for the SPL. Either the platform was already enabling CONFIG_SPL_MTD_SUPPORT and the MTD code was already compiled in before Miquel's patch, or this option is disabled, and the SPL still does *not* embed the MTD layer. > > Please understand that there is a fundamental difference between > parallel NOR flash and things like NAND, SPI NOR etc. - the latter > are storage devices, which are usually handled as block device or > similar, i. e. you always need a driver to read data. Parallel NOR > is not storage, it is _memory_, which is directly mapped int o > memory. You can execute code from it. That's only partially true. Yes, you can read from a parallel NORs like if it was memory because memory controllers embedded in SoCs provide your a direct mmio mapping. That's not true for write accesses, as NORs need to be erased before you can write on it. The MTD layer is here to abstract that, such that flash users only have to know how to manipulate an MTD device instead of having one backend per flash-type (actually it's even one per-flash-type+mem-bus). > > Reading data from parallel NOR comes at zero overhead (except maybe > for setting up the memory controller to map the NOR into the address > space). The overhead can be minimal if we want (we can have a tiny MTD layer) current: flash_read() proposed: mtd_read() mtd_to_flash_read_wrapper() flash_read() and when I say minimal, I mean both in term of size and perfs (we're talking about an indirect jump and a few extra instructions in mtd_to_flash_read_wrapper()). The benefit, well, a single entry point for all kind of mtd accesses. That means we can have an MTD backend for env retrieval instead of one per flash type. Same goes for DFU backends, and maybe even for SPL boots. To sum-up: less code to maintain. And I wouldn't be surprized if we were actually reducing the image footprint in some cases (when support for several flash types is enabled). > > Adding a dependency on the MTD layer is broken. > > > Today, there are multiple boards having more than one type of MTD > > device (eg. raw NAND and SPI-NOR). In this case you need to compile two > > commands which interface only with the subsystem they have been written > > for. We propose to kill this approach and instead use an 'mtd' command > > which shares the same code for all MTD devices: less duplication, more > > users, and in the end, a reduced size. And I am not event talking about > > all the code that has been added over the years to "avoid using MTD" > > which enlarges the binary as well. > > This is perfectly fine. But you must not break fundamental features > that have been there ever since the beginning of the project. > > Simple things - like reading the environment from parallel NOR > (which boils down to calling ofa memcpy()) must not be bloated with > unneeded layers like MTD - even if these are needed and useful > elsewhere. > > Please do not understand me wrong: I fully agree with the MTD rework > in general, and I'm happy to see it happen. But please do it in a > way not to break existing basic use cases. Okay, maybe we can put aside the parallel NOR case for now, but I do think even parallel NOR accesses would benefit from the MTD layer. Regards, Boris ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v4 00/11] mtd/sf: Various fixes
Hi Jagan, On Sun, 2 Dec 2018 10:54:21 +0100 Boris Brezillon wrote: > Hello, > > This is the 4th version of the mtd / sf fixes patchset. This v4 just > adds a new check in del_mtd_device() (and a debug() when > del_mtd_partitions() fails). > > Regards, > > Boris > > P.S.: travis-ci results => > https://travis-ci.org/bbrezillon/u-boot/builds/461943011 Please don't wait too long before queuing this series and sending a fixes PR to Tom. I don't want to end up in the same situation we were for 2018.11. Thanks, Boris ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v3 26/28] cmd: make MTD commands depend on MTD
On Wed, 5 Dec 2018 11:48:32 +0100
Miquel Raynal wrote:
> Hi Boris,
>
> Boris Brezillon wrote on Wed, 5 Dec 2018
> 11:42:08 +0100:
>
> > On Wed, 5 Dec 2018 00:57:12 +0100
> > Miquel Raynal wrote:
> >
> > > Defconfigs have been fixed, now we can add proper dependencies in
> > > Kconfig. SPI FLASH is still not dependent on MTD (deeper rework needed).
> > >
> > > Signed-off-by: Miquel Raynal
> > > ---
> > > cmd/Kconfig | 10 +++---
> > > 1 file changed, 7 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/cmd/Kconfig b/cmd/Kconfig
> > > index cf58174013..7c166a07e6 100644
> > > --- a/cmd/Kconfig
> > > +++ b/cmd/Kconfig
> > > @@ -679,6 +679,7 @@ config CMD_FDC
> > > config CMD_FLASH
> > > bool "flinfo, erase, protect"
> > > default y
> > > + depends on MTD
> > > help
> > > NOR flash support.
> > > flinfo - print FLASH memory information
> > > @@ -868,6 +869,7 @@ config CMD_MMC_SWRITE
> > >
> > > config CMD_MTD
> > > bool "mtd"
> > > + depends on MTD
> > > select MTD_PARTITIONS
> > > help
> > > MTD commands support.
> > > @@ -875,6 +877,7 @@ config CMD_MTD
> > > config CMD_NAND
> > > bool "nand"
> > > default y if NAND_SUNXI
> > > + depends on MTD_RAW_NAND
> > > help
> > > NAND support.
> > >
> > > @@ -915,6 +918,7 @@ config CMD_MMC_SPI
> > >
> > > config CMD_ONENAND
> > > bool "onenand - access to onenand device"
> > > + depends on MTD
> > > help
> > > OneNAND is a brand of NAND ('Not AND' gate) flash which provides
> > > various useful features. This command allows reading, writing,
> > > @@ -1733,7 +1737,7 @@ config CMD_JFFS2
> > >
> > > config CMD_MTDPARTS
> > > bool "MTD partition support"
> > > - select MTD if (CMD_NAND || NAND)
> > > + depends on MTD
> > > help
> > > MTD partitioning tool support.
> > > It is strongly encouraged to avoid using this command
> > > @@ -1754,14 +1758,14 @@ endif
> > >
> > > config MTDIDS_DEFAULT
> > > string "Default MTD IDs"
> > > - depends on MTD_PARTITIONS || CMD_MTDPARTS || CMD_NAND || CMD_FLASH
> > > + depends on MTD || SPI_FLASH
> >
> > Can't we have MTD enabled without MTD_PARTITIONS? I guess we don't need
> > to expose these options if MTD_PARTITIONS is disabled.
>
> That's the theory. (Travis) Experience shows that adding a
> dependency on MTD_PARTITIONS when removing the dependency on the above
> commands is too restrictive and some header files using
> MTDIDS/MTDPARTS_DEFAULT will produce build issues. This is insane but I
> did not want to debug this issue and, anyway, it is harmless to have
> these strings defined.
Fair enough.
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v3 28/28] mtd: properly handle SPL kbuild lines
Hi Miquel, On Wed, 5 Dec 2018 00:57:14 +0100 Miquel Raynal wrote: > Instead of compiling just a few files from the root Makefile, rework > the MTD Makefile tree to take into account SPL's needs. > As mentioned in my review of patch 1, I think patch 1, 27 and 28 could be merged in a single patch that would be placed at the end of this series as the final (real) Makefile cleanup. BTW, good job! I can only imagine how painful it was to do this without breaking builds. Regards, Boris ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v3 26/28] cmd: make MTD commands depend on MTD
On Wed, 5 Dec 2018 00:57:12 +0100
Miquel Raynal wrote:
> Defconfigs have been fixed, now we can add proper dependencies in
> Kconfig. SPI FLASH is still not dependent on MTD (deeper rework needed).
>
> Signed-off-by: Miquel Raynal
> ---
> cmd/Kconfig | 10 +++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index cf58174013..7c166a07e6 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -679,6 +679,7 @@ config CMD_FDC
> config CMD_FLASH
> bool "flinfo, erase, protect"
> default y
> + depends on MTD
> help
> NOR flash support.
> flinfo - print FLASH memory information
> @@ -868,6 +869,7 @@ config CMD_MMC_SWRITE
>
> config CMD_MTD
> bool "mtd"
> + depends on MTD
> select MTD_PARTITIONS
> help
> MTD commands support.
> @@ -875,6 +877,7 @@ config CMD_MTD
> config CMD_NAND
> bool "nand"
> default y if NAND_SUNXI
> + depends on MTD_RAW_NAND
> help
> NAND support.
>
> @@ -915,6 +918,7 @@ config CMD_MMC_SPI
>
> config CMD_ONENAND
> bool "onenand - access to onenand device"
> + depends on MTD
> help
> OneNAND is a brand of NAND ('Not AND' gate) flash which provides
> various useful features. This command allows reading, writing,
> @@ -1733,7 +1737,7 @@ config CMD_JFFS2
>
> config CMD_MTDPARTS
> bool "MTD partition support"
> - select MTD if (CMD_NAND || NAND)
> + depends on MTD
> help
> MTD partitioning tool support.
> It is strongly encouraged to avoid using this command
> @@ -1754,14 +1758,14 @@ endif
>
> config MTDIDS_DEFAULT
> string "Default MTD IDs"
> - depends on MTD_PARTITIONS || CMD_MTDPARTS || CMD_NAND || CMD_FLASH
> + depends on MTD || SPI_FLASH
Can't we have MTD enabled without MTD_PARTITIONS? I guess we don't need
to expose these options if MTD_PARTITIONS is disabled.
> help
> Defines a default MTD IDs list for use with MTD partitions in the
> Linux MTD command line partitions format.
>
> config MTDPARTS_DEFAULT
> string "Default MTD partition scheme"
> - depends on MTD_PARTITIONS || CMD_MTDPARTS || CMD_NAND || CMD_FLASH
> + depends on MTD || SPI_FLASH
> help
> Defines a default MTD partitioning scheme in the Linux MTD command
> line partitions format
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v3 23/28] mtd: ubi: remove dependency on command in Kconfig
On Wed, 5 Dec 2018 00:57:09 +0100 Miquel Raynal wrote: > Now that the defconfigs have been fixed, remove the dependencies on > commands in Kconfig. No changes in Kconfig in this patch. > The necessary symbols are enabled so this will > not break anything. > > Signed-off-by: Miquel Raynal > --- > drivers/mtd/Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile > index 486634b562..68b1029734 100644 > --- a/drivers/mtd/Makefile > +++ b/drivers/mtd/Makefile > @@ -24,5 +24,5 @@ ifeq ($(CONFIG_SPL_BUILD)$(CONFIG_TPL_BUILD),) > obj-y += nand/ > obj-y += onenand/ > obj-y += spi/ > -obj-$(CONFIG_CMD_UBI) += ubi/ > +obj-$(CONFIG_MTD_UBI) += ubi/ > endif ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v3 25/28] cmd: nand/sf: isolate legacy code
On Wed, 5 Dec 2018 00:57:11 +0100 Miquel Raynal wrote: > The 'sf' command is not supposed to rely on the MTD stack, but both > 'sf' and 'nand' commands use helpers located in mtd_uboot.c. Despite > their location, these functions do not depend at all on the MTD > stack. > > This file (drivers/mtd/mtd_uboot.c) is only compiled if CONFIG_MTD is > selected, which is incoherent with the current situation. Solve this ^inconsistent > by mobing these three functions (which are only used by the above two ^ moving > commands) out of mtd_uboot.c and put them in a C file only compiled > with cmd/sf.c and cmd/nand.c. > > Signed-off-by: Miquel Raynal > --- > cmd/Makefile| 4 +- > cmd/legacy-mtd-utils.c | 99 + > cmd/legacy-mtd-utils.h | 14 ++ > cmd/nand.c | 2 + > cmd/sf.c| 2 + > drivers/mtd/mtd_uboot.c | 94 -- > include/linux/mtd/mtd.h | 6 --- > 7 files changed, 119 insertions(+), 102 deletions(-) > create mode 100644 cmd/legacy-mtd-utils.c > create mode 100644 cmd/legacy-mtd-utils.h > > diff --git a/cmd/Makefile b/cmd/Makefile > index 0534ddc679..54c54c062d 100644 > --- a/cmd/Makefile > +++ b/cmd/Makefile > @@ -94,7 +94,7 @@ obj-$(CONFIG_CMD_MMC_SPI) += mmc_spi.o > obj-$(CONFIG_MP) += mp.o > obj-$(CONFIG_CMD_MTD) += mtd.o > obj-$(CONFIG_CMD_MTDPARTS) += mtdparts.o > -obj-$(CONFIG_CMD_NAND) += nand.o > +obj-$(CONFIG_CMD_NAND) += nand.o legacy-mtd-utils.o > obj-$(CONFIG_CMD_NET) += net.o > obj-$(CONFIG_CMD_ONENAND) += onenand.o > obj-$(CONFIG_CMD_OSD) += osd.o > @@ -115,7 +115,7 @@ obj-$(CONFIG_CMD_ROCKUSB) += rockusb.o > obj-$(CONFIG_SANDBOX) += host.o > obj-$(CONFIG_CMD_SATA) += sata.o > obj-$(CONFIG_CMD_NVME) += nvme.o > -obj-$(CONFIG_CMD_SF) += sf.o > +obj-$(CONFIG_CMD_SF) += sf.o legacy-mtd-utils.o That won't work if you enable both CMD_SF and CMD_NAND. The linker will complain that some symbols are duplicated. Either you add a new hidden Kconfig option that is selected by CMD_NAND and CMD_SF, or you use the ifeq trick: ifeq (,$(findstring y,$(CONFIG_CMD_NAND)$(CONFIG_CMD_SF))) obj-y += legacy-mtd-utils.o endif ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v3 21/28] mtd: nor: NOR flashes depend on MTD
On Wed, 5 Dec 2018 00:57:07 +0100 Miquel Raynal wrote: > A NOR flash needs the MTD core, ensure this dependency is met by > adding a "depends on" in Kconfig. This is fine since defconfigs have > been fixed. > > Signed-off-by: Miquel Raynal > --- > drivers/mtd/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig > index 345046c2a6..0832f5b411 100644 > --- a/drivers/mtd/Kconfig > +++ b/drivers/mtd/Kconfig > @@ -19,6 +19,7 @@ config DM_MTD > > config MTD_NOR_FLASH > bool "Enable parallel NOR flash support" > + depends on MTD > help > Enable support for parallel NOR flash. > Don't know if this applies here, but if you have almost all options in Kconfig that depend on MTD, you can add an if MTD endif section. ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v3 22/28] mtd: nand: remove dependency on commands in Kconfig
On Wed, 5 Dec 2018 00:57:08 +0100 Miquel Raynal wrote: > Now that the defconfigs have been fixed, remove the dependencies on > commands in Kconfig. That's not what I'm seeing in this diff ;-). > The necessary symbols are enabled so this will > not break anything. > > Signed-off-by: Miquel Raynal > --- > drivers/mtd/nand/Makefile | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile > index 99ca69ef95..e5849dc02a 100644 > --- a/drivers/mtd/nand/Makefile > +++ b/drivers/mtd/nand/Makefile > @@ -2,7 +2,5 @@ > > nandcore-objs := core.o bbt.o > obj-$(CONFIG_MTD_NAND_CORE) += nandcore.o > -ifneq (,$(findstring y,$(CONFIG_CMD_NAND)$(CONFIG_CMD_MTD))) > -obj-y += raw/ > -endif > +obj-$(CONFIG_MTD_RAW_NAND) += raw/ > obj-$(CONFIG_MTD_SPI_NAND) += spi/ ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v3 20/28] dfu: add dependency on the NAND core
On Wed, 5 Dec 2018 00:57:06 +0100 Miquel Raynal wrote: > CONFIG_DFU_NAND needs the NAND core being compiled. > > Also fix the colibri_vf defconfig to reflect this dependency. > > Signed-off-by: Miquel Raynal Reviewed-by: Boris Brezillon BTW, it would be great to have an MTD backend instead of having one per type of flash ;-). > --- > configs/colibri_vf_defconfig | 1 + > drivers/dfu/Kconfig | 1 + > 2 files changed, 2 insertions(+) > > diff --git a/configs/colibri_vf_defconfig b/configs/colibri_vf_defconfig > index 305dfcccb5..73b7055de1 100644 > --- a/configs/colibri_vf_defconfig > +++ b/configs/colibri_vf_defconfig > @@ -50,6 +50,7 @@ CONFIG_FSL_ESDHC=y > CONFIG_NAND_VF610_NFC=y > CONFIG_SYS_NAND_VF610_NFC_60_ECC_BYTES=y > CONFIG_MTD=y > +CONFIG_MTD_RAW_NAND=y > CONFIG_MTD_UBI=y > CONFIG_MTD_UBI_FASTMAP=y > CONFIG_PHYLIB=y > diff --git a/drivers/dfu/Kconfig b/drivers/dfu/Kconfig > index 4692736c9d..5ee260913c 100644 > --- a/drivers/dfu/Kconfig > +++ b/drivers/dfu/Kconfig > @@ -31,6 +31,7 @@ config DFU_MMC > config DFU_NAND > bool "NAND back end for DFU" > depends on CMD_MTDPARTS > + depends on MTD_RAW_NAND > help > This option enables using DFU to read and write to NAND based > storage. ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v3 19/28] mtd: nand: add includes in NAND core to avoid warnings
On Wed, 5 Dec 2018 00:57:05 +0100 Miquel Raynal wrote: > Because of the include's game, when some files are compiled for a SPI > NAND device, no warning appears. But when it is for a raw NAND device, > GCC complains. Fix these warning by including . > > Signed-off-by: Miquel Raynal Reviewed-by: Boris Brezillon > --- > drivers/mtd/nand/bbt.c | 1 + > drivers/mtd/nand/core.c | 1 + > 2 files changed, 2 insertions(+) > > diff --git a/drivers/mtd/nand/bbt.c b/drivers/mtd/nand/bbt.c > index 7e0ad3190c..f3d05e6757 100644 > --- a/drivers/mtd/nand/bbt.c > +++ b/drivers/mtd/nand/bbt.c > @@ -9,6 +9,7 @@ > > #define pr_fmt(fmt) "nand-bbt: " fmt > > +#include > #include > #ifndef __UBOOT__ > #include > diff --git a/drivers/mtd/nand/core.c b/drivers/mtd/nand/core.c > index 0b793695cc..3abaef23c5 100644 > --- a/drivers/mtd/nand/core.c > +++ b/drivers/mtd/nand/core.c > @@ -9,6 +9,7 @@ > > #define pr_fmt(fmt) "nand: " fmt > > +#include > #ifndef __UBOOT__ > #include > #endif ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v3 18/28] configs: remove MTD support from bcm11130 and M54418TWR defconfigs
On Wed, 5 Dec 2018 00:57:04 +0100 Miquel Raynal wrote: > While the right Kconfig entries were selected, because of the missing > CMD_NAND symbol the raw NAND core was never compiled. Remove it from > the defconfigs otherwise the build will fail. See my comment on patch 17. ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v3 17/28] configs: remove raw NAND core from k2g defconfigs
On Wed, 5 Dec 2018 00:57:03 +0100 Miquel Raynal wrote: > Due to previous Makefile organization, the raw NAND subdirectory was > not compiled in if CMD_NAND was not enabled. Because the Denali driver > does not compile with these boards (undefined environment offset), > remove the dependency within the defconfig over the controller driver > (was ignored anyway in the past). Maybe this should be moved before the Makefile re-organization if this the build of those defconfigs. > > Signed-off-by: Miquel Raynal > --- > configs/k2g_evm_defconfig| 1 - > configs/k2g_hs_evm_defconfig | 1 - > 2 files changed, 2 deletions(-) > > diff --git a/configs/k2g_evm_defconfig b/configs/k2g_evm_defconfig > index dee00ec8d4..b474781ccb 100644 > --- a/configs/k2g_evm_defconfig > +++ b/configs/k2g_evm_defconfig > @@ -41,7 +41,6 @@ CONFIG_MMC_OMAP_HS=y > CONFIG_MTD=y > CONFIG_MTD_UBI=y > CONFIG_MTD_RAW_NAND=y > -CONFIG_NAND_DAVINCI=y > CONFIG_DM_SPI_FLASH=y > CONFIG_SPI_FLASH=y > CONFIG_SPI_FLASH_BAR=y > diff --git a/configs/k2g_hs_evm_defconfig b/configs/k2g_hs_evm_defconfig > index b8c395d78d..a50a7abf30 100644 > --- a/configs/k2g_hs_evm_defconfig > +++ b/configs/k2g_hs_evm_defconfig > @@ -34,7 +34,6 @@ CONFIG_MMC_OMAP_HS=y > CONFIG_MTD=y > CONFIG_MTD_UBI=y > CONFIG_MTD_RAW_NAND=y > -CONFIG_NAND_DAVINCI=y > CONFIG_DM_SPI_FLASH=y > CONFIG_SPI_FLASH=y > CONFIG_SPI_FLASH_BAR=y ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v3 16/28] configs: move CONFIG_MTD in defconfigs when set in arch includes
On Wed, 5 Dec 2018 00:57:02 +0100 Miquel Raynal wrote: > Let's be consistent and always declare CONFIG_MTD from the defconfig > file when needed. > > Signed-off-by: Miquel Raynal Reviewed-by: Boris Brezillon One comment below. > --- > configs/socfpga_stratix10_defconfig | 1 + > configs/turris_mox_defconfig | 1 + > include/configs/mvebu_armada-37xx.h | 1 - > include/configs/socfpga_stratix10_socdk.h | 1 - > 4 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/configs/socfpga_stratix10_defconfig > b/configs/socfpga_stratix10_defconfig > index 5f3d733a8b..292dbd6973 100644 > --- a/configs/socfpga_stratix10_defconfig > +++ b/configs/socfpga_stratix10_defconfig > @@ -38,6 +38,7 @@ CONFIG_DM_I2C=y > CONFIG_SYS_I2C_DW=y > CONFIG_DM_MMC=y > CONFIG_MMC_DW=y > +CONFIG_MTD=y > CONFIG_SPI_FLASH=y > CONFIG_SPI_FLASH_BAR=y > CONFIG_SPI_FLASH_SPANSION=y > diff --git a/configs/turris_mox_defconfig b/configs/turris_mox_defconfig > index 749ed31acd..13e2af7e1b 100644 > --- a/configs/turris_mox_defconfig > +++ b/configs/turris_mox_defconfig > @@ -42,6 +42,7 @@ CONFIG_DM_MMC=y > CONFIG_MMC_SDHCI=y > CONFIG_MMC_SDHCI_SDMA=y > CONFIG_MMC_SDHCI_XENON=y > +CONFIG_MTD=y > CONFIG_SPI_FLASH=y > CONFIG_SPI_FLASH_MACRONIX=y > CONFIG_SPI_FLASH_SPANSION=y > diff --git a/include/configs/mvebu_armada-37xx.h > b/include/configs/mvebu_armada-37xx.h > index f93ab0f830..640267c9c2 100644 > --- a/include/configs/mvebu_armada-37xx.h > +++ b/include/configs/mvebu_armada-37xx.h > @@ -64,7 +64,6 @@ > #define CONFIG_SF_DEFAULT_SPEED 100 > #define CONFIG_SF_DEFAULT_MODE SPI_MODE_0 > #define CONFIG_ENV_SPI_MODE CONFIG_SF_DEFAULT_MODE > -#define CONFIG_MTD /* needed for mtdparts commands */ > #define CONFIG_MTD_PARTITIONS/* required for UBI partition > support */ > > /* Environment in SPI NOR flash */ > diff --git a/include/configs/socfpga_stratix10_socdk.h > b/include/configs/socfpga_stratix10_socdk.h > index 22e1dc84a1..967784e379 100644 > --- a/include/configs/socfpga_stratix10_socdk.h > +++ b/include/configs/socfpga_stratix10_socdk.h > @@ -76,7 +76,6 @@ > #endif /* CONFIG_ENV_IS_IN_SPI_FLASH */ > > #ifndef CONFIG_SPL_BUILD > -#define CONFIG_MTD > #define CONFIG_MTD_PARTITIONS Do you get rid of CONFIG_MTD_PARTITIONS at some point? > #define MTDIDS_DEFAULT "nor0=ff705000.spi.0" > #endif /* CONFIG_SPL_BUILD */ ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v3 24/28] cmd: mtdparts: show Kconfig options only if the command is selected
On Wed, 5 Dec 2018 00:57:10 +0100 Miquel Raynal wrote: > Guard mtdparts options with an if/endif statement in Kconfig. Also > move the Kconfig entry of the option right after the entry of the > command. > > Signed-off-by: Miquel Raynal > --- > cmd/Kconfig | 21 +++-- > 1 file changed, 11 insertions(+), 10 deletions(-) > > diff --git a/cmd/Kconfig b/cmd/Kconfig > index bf9cc0c52d..cf58174013 100644 > --- a/cmd/Kconfig > +++ b/cmd/Kconfig > @@ -1741,6 +1741,17 @@ config CMD_MTDPARTS > declare the partitions in the mtdparts environment variable > but better use the MTD stack and the 'mtd' command instead. > > +if CMD_MTDPARTS > +config CMD_MTDPARTS_SPREAD > + bool "Padd partition size to take account of bad blocks" We usually use "depends on" in such cases depends on CMD_MTDPARTS > + help > + This enables the 'spread' sub-command of the mtdparts command. > + This command will modify the existing mtdparts variable by increasing > + the size of the partitions such that 1) each partition's net size is > + at least as large as the size specified in the mtdparts variable and > + 2) each partition starts on a good block. > +endif > + > config MTDIDS_DEFAULT > string "Default MTD IDs" > depends on MTD_PARTITIONS || CMD_MTDPARTS || CMD_NAND || CMD_FLASH > @@ -1755,16 +1766,6 @@ config MTDPARTS_DEFAULT > Defines a default MTD partitioning scheme in the Linux MTD command > line partitions format > > -config CMD_MTDPARTS_SPREAD > - bool "Padd partition size to take account of bad blocks" > - depends on CMD_MTDPARTS > - help > - This enables the 'spread' sub-command of the mtdparts command. > - This command will modify the existing mtdparts variable by increasing > - the size of the partitions such that 1) each partition's net size is > - at least as large as the size specified in the mtdparts variable and > - 2) each partition starts on a good block. > - > config CMD_REISER > bool "reiser - Access to reiserfs filesystems" > help ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v3 15/28] mtd: ensure MTD is compiled when CMD_MTDPARTS is selected
On Wed, 5 Dec 2018 00:57:01 +0100 Miquel Raynal wrote: > MTD support must be enabled when using mtdparts. Indeed, functions > like get_mtd_info(), get_mtd_device() and put_mtd_device() are in > drivers/mtd/mtd_uboot.c and are built only with CONFIG_MTD. > > Signed-off-by: Miquel Raynal Reviewed-by: Boris Brezillon ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v3 14/28] mtd: ensure CMD_NAND is compiled when its options are selected
On Wed, 5 Dec 2018 00:57:00 +0100 Miquel Raynal wrote: > In some files, options of CMD_NAND are selected but not the command > itself. Fix this inconsistency. > > Signed-off-by: Miquel Raynal Reviewed-by: Boris Brezillon ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v3 13/28] mtd: ensure MTD is compiled when ENV_IS_IN_FLASH is selected
On Wed, 5 Dec 2018 00:56:59 +0100 Miquel Raynal wrote: > MTD support must be enabled when the environment is in NOR. > > Signed-off-by: Miquel Raynal > --- > configs/armadillo-800eva_defconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/configs/armadillo-800eva_defconfig > b/configs/armadillo-800eva_defconfig > index b1d923c069..45736a9b01 100644 > --- a/configs/armadillo-800eva_defconfig > +++ b/configs/armadillo-800eva_defconfig > @@ -32,6 +32,7 @@ CONFIG_CMD_PING=y > # CONFIG_CMD_MISC is not set > CONFIG_ENV_IS_IN_FLASH=y > # CONFIG_MMC is not set > +CONFIG_MTD=y Don't we need CONFIG_MTD_NOR_FLASH too? > CONFIG_SH_ETHER=y > CONFIG_SCIF_CONSOLE=y > CONFIG_OF_LIBFDT=y ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v3 12/28] mtd: ensure MTD_RAW_NAND is compiled when ENV_IS_IN_NAND is selected
On Wed, 5 Dec 2018 00:56:58 +0100 Miquel Raynal wrote: > Raw NAND support must be enabled when the environment is in NAND. > > Signed-off-by: Miquel Raynal Reviewed-by: Boris Brezillon ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v3 11/28] mtd: ensure UBI is compiled when ENV_IS_IN_UBI is selected
On Wed, 5 Dec 2018 00:56:57 +0100 Miquel Raynal wrote: > UBI must be enabled when the environment is in UBI. > > Signed-off-by: Miquel Raynal Reviewed-by: Boris Brezillon ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v3 10/28] mtd: ensure UBI is compiled when CMD_UBI is selected
On Wed, 5 Dec 2018 00:56:56 +0100 Miquel Raynal wrote: > UBI must be enabled when CMD_UBI is used, this is mandatory and will > later be reflected thanks to a "depends on" in Kconfig. But first, > defconfigs needs to be fixed. > > Signed-off-by: Miquel Raynal Reviewed-by: Boris Brezillon ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v3 09/28] mtd: ensure MTD is compiled when UBI is used
On Wed, 5 Dec 2018 00:56:55 +0100 Miquel Raynal wrote: > MTD must be enabled when UBI is used, this is mandatory and will later > be reflected thanks to a "depends on" in Kconfig. But first, > defconfigs needs to be fixed. ^ need > > Signed-off-by: Miquel Raynal Reviewed-by: Boris Brezillon ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v3 08/28] mtd: ensure UBI is compiled when using fastmap
On Wed, 5 Dec 2018 00:56:54 +0100 Miquel Raynal wrote: > UBI must be enabled when using fastmap, reflect this is defconfigs. > > Signed-off-by: Miquel Raynal Reviewed-by: Boris Brezillon ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v3 07/28] mtd: ensure MTD is compiled when there is a SPI NOR flash using MTD
On Wed, 5 Dec 2018 00:56:53 +0100 Miquel Raynal wrote: > MTD must be enabled when there is a SPI NOR flash using the > SPI_FLASH_MTD config entry. > > Signed-off-by: Miquel Raynal With Vignesh's comment addressed Reviewed-by: Boris Brezillon ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v3 06/28] mtd: ensure MTD/the raw NAND core are compiled when there is a NAND flash
On Wed, 5 Dec 2018 00:56:52 +0100 Miquel Raynal wrote: > Both symbols must be enabled when there is a raw NAND driver > selected. Also enable them when CONFIG_CMD_NAND is selected to do not > break any build during later cleanup. "... to avoid breaking things when we'll further rework the MTD dependency description" or something like along those lines. > > Signed-off-by: Miquel Raynal Other than that Reviewed-by: Boris Brezillon ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v3 01/28] Makefile: move MTD-related lines in coherent Makefiles
On Wed, 5 Dec 2018 00:56:47 +0100 Miquel Raynal wrote: > Move MTD-related lines out of the root Makefile. Put them in their > respective directories. > > Enclose some of these new lines to skip them when building the SPL > (the SPL directly points to what is needed). > > CONFIG_CMD_NAND is the symbol indicating support for raw NAND > devices. This logic is broken and will be fixed afterwards. To let the > 'mtd' command also use the raw NAND core, we add it in the > condition. This will be cleaned later. > > Signed-off-by: Miquel Raynal > --- > Makefile | 5 - > drivers/Makefile | 3 ++- > drivers/mtd/Makefile | 6 ++ > drivers/mtd/nand/Makefile | 3 +++ > 4 files changed, 11 insertions(+), 6 deletions(-) > > diff --git a/Makefile b/Makefile > index 552687db53..435d0257dd 100644 > --- a/Makefile > +++ b/Makefile > @@ -688,11 +688,6 @@ libs-y += drivers/ > libs-y += drivers/dma/ > libs-y += drivers/gpio/ > libs-y += drivers/i2c/ > -libs-y += drivers/mtd/ > -libs-$(CONFIG_CMD_NAND) += drivers/mtd/nand/raw/ > -libs-y += drivers/mtd/onenand/ > -libs-$(CONFIG_CMD_UBI) += drivers/mtd/ubi/ > -libs-y += drivers/mtd/spi/ > libs-y += drivers/net/ > libs-y += drivers/net/phy/ > libs-y += drivers/pci/ > diff --git a/drivers/Makefile b/drivers/Makefile > index 4453c62ad3..e7c93875bb 100644 > --- a/drivers/Makefile > +++ b/drivers/Makefile > @@ -6,7 +6,7 @@ obj-$(CONFIG_$(SPL_TPL_)DRIVERS_MISC_SUPPORT) += misc/ > sysreset/ firmware/ > obj-$(CONFIG_$(SPL_TPL_)I2C_SUPPORT) += i2c/ > obj-$(CONFIG_$(SPL_TPL_)LED) += led/ > obj-$(CONFIG_$(SPL_TPL_)MMC_SUPPORT) += mmc/ > -obj-$(CONFIG_$(SPL_TPL_)NAND_SUPPORT) += mtd/nand/raw/ > +obj-$(CONFIG_$(SPL_TPL_)NAND_SUPPORT) += mtd/nand/ I know re-ordering those patches is painful, but I'd prefer to have this patch moved at the end so that you directly include mtd instead of having the Makefile layering partially fixed (fixed for the non-SPL case, but not for the SPL/TPL case). > obj-$(CONFIG_$(SPL_TPL_)PHY) += phy/ > obj-$(CONFIG_$(SPL_TPL_)PINCTRL) += pinctrl/ > obj-$(CONFIG_$(SPL_TPL_)RAM) += ram/ > @@ -99,6 +99,7 @@ obj-$(CONFIG_QE) += qe/ > obj-$(CONFIG_U_QE) += qe/ > obj-y += mailbox/ > obj-y += memory/ > +obj-y += mtd/ > obj-y += pwm/ > obj-y += reset/ > obj-y += input/ > diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile > index 22ceda93c0..aca3a0b811 100644 > --- a/drivers/mtd/Makefile > +++ b/drivers/mtd/Makefile > @@ -19,4 +19,10 @@ obj-$(CONFIG_ST_SMI) += st_smi.o > obj-$(CONFIG_STM32_FLASH) += stm32_flash.o > obj-$(CONFIG_RENESAS_RPC_HF) += renesas_rpc_hf.o > > +# SPL will manually build the files it needs > +ifeq ($(CONFIG_SPL_BUILD)$(CONFIG_TPL_BUILD),) > obj-y += nand/ > +obj-y += onenand/ > +obj-y += spi/ > +obj-$(CONFIG_CMD_UBI) += ubi/ > +endif > diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile > index a358bc680e..99ca69ef95 100644 > --- a/drivers/mtd/nand/Makefile > +++ b/drivers/mtd/nand/Makefile > @@ -2,4 +2,7 @@ > > nandcore-objs := core.o bbt.o > obj-$(CONFIG_MTD_NAND_CORE) += nandcore.o > +ifneq (,$(findstring y,$(CONFIG_CMD_NAND)$(CONFIG_CMD_MTD))) > +obj-y += raw/ > +endif > obj-$(CONFIG_MTD_SPI_NAND) += spi/ ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v3 04/28] mtd: rename CONFIG_MTD_DEVICE -> CONFIG_MTD
On Wed, 5 Dec 2018 00:56:50 +0100 Miquel Raynal wrote: > Like in Linux, just use CONFIG_MTD to compile the MTD stack. > > Signed-off-by: Miquel Raynal Reviewed-by: Boris Brezillon ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v3 05/28] mtd: ensure MTD is compiled when there is a NOR flash
On Wed, 5 Dec 2018 00:56:51 +0100 Miquel Raynal wrote: > CONFIG_MTD must be enabled when there is a NOR flash selected. > > Signed-off-by: Miquel Raynal Reviewed-by: Boris Brezillon ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v3 03/28] mtd: rename CONFIG_MTD -> CONFIG_DM_MTD
On Wed, 5 Dec 2018 00:56:49 +0100 Miquel Raynal wrote: > CONFIG_MTD must be reserved for the MTD core. Like any other > subsystem, prefix the symbol by DM when it comes to DM support. > > Signed-off-by: Miquel Raynal Reviewed-by: Boris Brezillon ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v3 02/28] mtd: rename CONFIG_NAND -> CONFIG_MTD_RAW_NAND
On Wed, 5 Dec 2018 00:56:48 +0100 Miquel Raynal wrote: > Add more clarity by changing the Kconfig entry name. > > Signed-off-by: Miquel Raynal Reviewed-by: Boris Brezillon ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v2] common: command: Add support for $ auto-completion
Add the dollar_complete() function to auto-complete arguments starting
with a '$' and use it in the cmd_auto_complete() path such that all
args starting with a $ can be auto-completed based on the available env
vars.
Signed-off-by: Boris Brezillon
---
Changes in v2:
- Call dollar_complete() from cmd_auto_complete() to provide $
auto-completion to everyone (suggested by Wolfgang)
---
common/command.c | 33 ++--
env/common.c | 50 +---
include/common.h | 3 ++-
3 files changed, 76 insertions(+), 10 deletions(-)
diff --git a/common/command.c b/common/command.c
index 19f0534a76ea..0a93322814a6 100644
--- a/common/command.c
+++ b/common/command.c
@@ -143,22 +143,38 @@ int cmd_usage(const cmd_tbl_t *cmdtp)
#ifdef CONFIG_AUTO_COMPLETE
+static char env_complete_buf[512];
+
int var_complete(int argc, char * const argv[], char last_char, int maxv, char
*cmdv[])
{
- static char tmp_buf[512];
int space;
space = last_char == '\0' || isblank(last_char);
if (space && argc == 1)
- return env_complete("", maxv, cmdv, sizeof(tmp_buf), tmp_buf);
+ return env_complete("", maxv, cmdv, sizeof(env_complete_buf),
+ env_complete_buf, false);
if (!space && argc == 2)
- return env_complete(argv[1], maxv, cmdv, sizeof(tmp_buf),
tmp_buf);
+ return env_complete(argv[1], maxv, cmdv,
+ sizeof(env_complete_buf),
+ env_complete_buf, false);
return 0;
}
+static int dollar_complete(int argc, char * const argv[], char last_char,
+ int maxv, char *cmdv[])
+{
+ /* Make sure the last argument starts with a $. */
+ if (argc < 1 || argv[argc - 1][0] != '$' ||
+ last_char == '\0' || isblank(last_char))
+ return 0;
+
+ return env_complete(argv[argc - 1], maxv, cmdv,
sizeof(env_complete_buf),
+ env_complete_buf, true);
+}
+
/*/
int complete_subcmdv(cmd_tbl_t *cmdtp, int count, int argc,
@@ -357,9 +373,14 @@ int cmd_auto_complete(const char *const prompt, char *buf,
int *np, int *colp)
/* separate into argv */
argc = make_argv(tmp_buf, sizeof(argv)/sizeof(argv[0]), argv);
- /* do the completion and return the possible completions */
- i = complete_cmdv(argc, argv, last_char,
- sizeof(cmdv) / sizeof(cmdv[0]), cmdv);
+ /* first try a $ completion */
+ i = dollar_complete(argc, argv, last_char,
+ sizeof(cmdv) / sizeof(cmdv[0]), cmdv);
+ if (!i) {
+ /* do the completion and return the possible completions */
+ i = complete_cmdv(argc, argv, last_char,
+ sizeof(cmdv) / sizeof(cmdv[0]), cmdv);
+ }
/* no match; bell and out */
if (i == 0) {
diff --git a/env/common.c b/env/common.c
index 3317cef35522..aa9a097bced0 100644
--- a/env/common.c
+++ b/env/common.c
@@ -241,31 +241,75 @@ void env_relocate(void)
}
#if defined(CONFIG_AUTO_COMPLETE) && !defined(CONFIG_SPL_BUILD)
-int env_complete(char *var, int maxv, char *cmdv[], int bufsz, char *buf)
+int env_complete(char *var, int maxv, char *cmdv[], int bufsz, char *buf,
+bool dollar_comp)
{
ENTRY *match;
int found, idx;
+ if (dollar_comp) {
+ /*
+* When doing $ completion, the first character should
+* obviously be a '$'.
+*/
+ if (var[0] != '$')
+ return 0;
+
+ var++;
+
+ /*
+* The second one, if present, should be a '{', as some
+* configuration of the u-boot shell expand ${var} but not
+* $var.
+*/
+ if (var[0] == '{')
+ var++;
+ else if (var[0] != '\0')
+ return 0;
+ }
+
idx = 0;
found = 0;
cmdv[0] = NULL;
+
while ((idx = hmatch_r(var, idx, , _htab))) {
int vallen = strlen(match->key) + 1;
- if (found >= maxv - 2 || bufsz < vallen)
+ if (found >= maxv - 2 ||
+ bufsz < vallen + (dollar_comp ? 3 : 0))
break;
cmdv[found++] = buf;
+
+ /* Add the '${' prefix to each var when doing $ completion. */
+ if (dollar_comp) {
+ strcpy(buf, "${");
+ buf += 2;
+ bufsz -= 3;
+ }
+
memcpy(buf, match->k
Re: [U-Boot] [PATCH 0/2] cmd: auto-complete args starting with a $
On Tue, 4 Dec 2018 11:33:13 +0100
Boris Brezillon wrote:
> >
> > i. e. this is a feature of the shell and not of any command.
> > Implementing this a zillion times for each of the commands in
> > inacceptable. Also, implementing it for one command and not for
> > another makese no sense - that would violate the Principle of Least
> > Surprise, as you have something which looks convenient but does not
> > work everywhere.
>
> Okay, I understand. I can try to hook that up at the common/command.c
> level.
I thought it would be much more complicated: here is the diff adding $
auto-completion for everyone. The only drawback I see with this approach
is that cmd/sub-cmd arg auto-completion cannot be bound to cmd->maxargs
anymore, but I guess that's acceptable.
--->8---
diff --git a/common/command.c b/common/command.c
index 754ab9bbc396..b9a44d4ea7a8 100644
--- a/common/command.c
+++ b/common/command.c
@@ -373,9 +373,14 @@ int cmd_auto_complete(const char *const prompt, char *buf,
int *np, int *colp)
/* separate into argv */
argc = make_argv(tmp_buf, sizeof(argv)/sizeof(argv[0]), argv);
- /* do the completion and return the possible completions */
- i = complete_cmdv(argc, argv, last_char,
- sizeof(cmdv) / sizeof(cmdv[0]), cmdv);
+ /* first try a $ completion */
+ i = dollar_complete(argc, argv, last_char,
+ sizeof(cmdv) / sizeof(cmdv[0]), cmdv);
+ if (!i) {
+ /* do the completion and return the possible completions */
+ i = complete_cmdv(argc, argv, last_char,
+ sizeof(cmdv) / sizeof(cmdv[0]), cmdv);
+ }
/* no match; bell and out */
if (i == 0) {
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH 0/2] cmd: auto-complete args starting with a $
On Tue, 04 Dec 2018 14:00:47 +0100 Wolfgang Denk wrote: > Dear Boris, > > In message <20181204113313.577178ac@bbrezillon> you wrote: > > > > > But is this not based on the code of mtd_name_complete() which is > > > only availabole when MTD is present? > > > > Nope. See patch 1, the code is completely independent from the mtd cmd. > > OK, then I misread the patches. > > > Apart from the "make that available to everyone" comment, is there > > anything else you think should be changed? IOW, what is not compliant > > with a standard shell auto-completion in my proposal? > > I can't say easily from the code. I'd have to see this running (in > the sandbox, for example). Actually, that's how I tested it. You can easily test it with the echo command after applying patch 1 of this series + the following diff. --->8--- diff --git a/cmd/echo.c b/cmd/echo.c index 5b018d9349ab..857f268c6a61 100644 --- a/cmd/echo.c +++ b/cmd/echo.c @@ -47,9 +47,10 @@ static int do_echo(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) return 0; } -U_BOOT_CMD( +U_BOOT_CMD_COMPLETE( echo, CONFIG_SYS_MAXARGS, 1, do_echo, "echo args to console", "[args..]\n" - "- echo args to console; \\c suppresses newline" + "- echo args to console; \\c suppresses newline", + dollar_complete ); ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [RFC PATCH v2 00/11] SF: Migrate to Linux SPI NOR framework
On Tue, 4 Dec 2018 17:56:48 +0530 Vignesh R wrote: > U-Boot SPI NOR support (sf layer) is quite outdated as it does not > support 4 byte addressing opcodes, SFDP table parsing and different types of > quad mode enable sequences. Many newer flashes no longer support BANK > registers used by sf layer to a access >16MB space. > Also, many SPI controllers have special MMIO interfaces which provide > accelerated read/write access but require knowledge of flash parameters > to make use of it. Recent spi-mem layer provides a way to support such > flashes but sf layer isn't using that. > This patch series syncs SPI NOR framework from Linux v4.19. It also adds > spi-mem support on top. > So, we gain 4byte addressing support and SFDP support. This makes > migrating to U-Boot MTD framework easier. Glad to see that happen sooner than I had expected. Looks like the patch series is in a good shape already, and I'm happy to see the u-boot spi-nor layer being based on the Linux one. Good job, and thanks again for working on that! Boris ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [RFC PATCH v2 09/11] sf_mtd: Simply mtd operations
On Tue, 4 Dec 2018 17:56:57 +0530
Vignesh R wrote:
> Now that there is new SPI NOR framework, simplify mtd device
> registration and read/write/erase operations.
>
> Signed-off-by: Vignesh R
> ---
> drivers/mtd/spi/sf_internal.h | 2 +-
> drivers/mtd/spi/sf_mtd.c | 39 ---
> drivers/mtd/spi/sf_probe.c| 2 +-
> 3 files changed, 15 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
> index 7e7d400cdbdf..8b445bb0b506 100644
> --- a/drivers/mtd/spi/sf_internal.h
> +++ b/drivers/mtd/spi/sf_internal.h
> @@ -99,6 +99,6 @@ int spi_flash_cmd_get_sw_write_prot(struct spi_flash
> *flash);
>
> #ifdef CONFIG_SPI_FLASH_MTD
> int spi_flash_mtd_register(struct spi_flash *flash);
> -void spi_flash_mtd_unregister(void);
> +void spi_flash_mtd_unregister(struct spi_flash *flash);
> #endif
> #endif /* _SF_INTERNAL_H_ */
> diff --git a/drivers/mtd/spi/sf_mtd.c b/drivers/mtd/spi/sf_mtd.c
> index 58d7e4439903..9c07ba3cadf7 100644
> --- a/drivers/mtd/spi/sf_mtd.c
> +++ b/drivers/mtd/spi/sf_mtd.c
> @@ -9,17 +9,15 @@
> #include
> #include
>
> -static struct mtd_info sf_mtd_info;
> static char sf_mtd_name[8];
>
> static int spi_flash_mtd_erase(struct mtd_info *mtd, struct erase_info
> *instr)
> {
> - struct spi_flash *flash = mtd->priv;
> int err;
>
> instr->state = MTD_ERASING;
>
> - err = spi_flash_erase(flash, instr->addr, instr->len);
> + err = mtd->_erase(mtd, instr);
Please don't dereference mtd hooks directly. I'm pretty sure the
mtd_erase() overhead is negligible, and if it's not, you can
implement dummy wrappers for the SPL case to reduce it to zero.
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH 0/2] cmd: auto-complete args starting with a $
Hi Wolfgang,
On Tue, 04 Dec 2018 11:14:31 +0100
Wolfgang Denk wrote:
> Dear Boris,
>
> In message <20181204105448.63b9af8c@bbrezillon> you wrote:
> >
> > > > It's pretty common to pass arguments that start with a $ and are then
> > > > expanded by the shell, and I'm this kind of lazy guy that hits tab all
> > > > the time and expects the shell to suggest something appropriate. So
> > > > here is a patchset adding support for ${} auto-completion and using the
> > > > new helper from the mtd command.
> > >
> > > You mean, this feature depends on MTD support? this is not goot.
> >
> > Not at all, MTD is just the first user of this dollar_complete()
> > helper. That's what I meant.
>
> But is this not based on the code of mtd_name_complete() which is
> only availabole when MTD is present?
Nope. See patch 1, the code is completely independent from the mtd cmd.
>
> > > Also, you should make this feature configurable. Not everybody may
> > > want to use it or may have the memory available.
> >
> > It already depends on CONFIG_AUTO_COMPLETE, and each command has to
> > explicitly call the helper to support ${} auto-completion. Not that $
> > auto-completion only makes sense for some arguments, not all of them.
> > It's the responsibility of the command itself to decide when it should
> > be used.
>
> Um... this is not hat I would expect - if we implement
> auto-completion for variables, it should IMO work similar to what a
> standard shell is doing.
>
> In Bash, I can do:
>
> -> foo=1234
> -> echo $f
> $f $flag $foo
And that's pretty much how it works except that right now, echo has to
be patched to support $ auto-completion.
> -> echo $f
For the record, not all shell configs expand $foo, some require that foo
be enclosed in ${} (${foo}), that's why the current proposal always
auto-completes $ as ${.
>
> i. e. this is a feature of the shell and not of any command.
> Implementing this a zillion times for each of the commands in
> inacceptable. Also, implementing it for one command and not for
> another makese no sense - that would violate the Principle of Least
> Surprise, as you have something which looks convenient but does not
> work everywhere.
Okay, I understand. I can try to hook that up at the common/command.c
level.
>
> > > Technically - should autocompletion not be prevented for escaped
> > > '$' characters, i. e. hitting TAB after a '\$' sequence should NOT
> > > expand?
> >
> > It's already the case. The argument has to start with $ or ${ to be
> > auto-completed.
>
> We should implement this such that behaviour is similar to what we
> see on a standard shell.
Apart from the "make that available to everyone" comment, is there
anything else you think should be changed? IOW, what is not compliant
with a standard shell auto-completion in my proposal?
Regards,
Boris
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH 0/2] cmd: auto-complete args starting with a $
On Tue, 04 Dec 2018 10:44:19 +0100
Wolfgang Denk wrote:
> Dear Boris,
>
> In message <20181203220726.19370-1-boris.brezil...@bootlin.com> you wrote:
> >
> > It's pretty common to pass arguments that start with a $ and are then
> > expanded by the shell, and I'm this kind of lazy guy that hits tab all
> > the time and expects the shell to suggest something appropriate. So
> > here is a patchset adding support for ${} auto-completion and using the
> > new helper from the mtd command.
>
> You mean, this feature depends on MTD support? this is not goot.
Not at all, MTD is just the first user of this dollar_complete()
helper. That's what I meant.
>
> Also, you should make this feature configurable. Not everybody may
> want to use it or may have the memory available.
It already depends on CONFIG_AUTO_COMPLETE, and each command has to
explicitly call the helper to support ${} auto-completion. Not that $
auto-completion only makes sense for some arguments, not all of them.
It's the responsibility of the command itself to decide when it should
be used.
>
> Technically - should autocompletion not be prevented for escaped
> '$' characters, i. e. hitting TAB after a '\$' sequence should NOT
> expand?
It's already the case. The argument has to start with $ or ${ to be
auto-completed.
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH 0/2] cmd: auto-complete args starting with a $
Hello,
It's pretty common to pass arguments that start with a $ and are then
expanded by the shell, and I'm this kind of lazy guy that hits tab all
the time and expects the shell to suggest something appropriate. So
here is a patchset adding support for ${} auto-completion and using the
new helper from the mtd command.
Using it elsewhere should be pretty easy (I already patched env set to
use it too [1]).
Note that this patch series depends on [2].
Regards,
Boris
[1]https://github.com/bbrezillon/u-boot/commit/9f37c5b6471c7479aa043524d31531f06128771c
[2]https://patchwork.ozlabs.org/project/uboot/list/?series=79552
Boris Brezillon (2):
common: command: Provide a dollar_complete() helper
cmd: mtd: auto-complete args starting with a $ when appropriate
cmd/mtd.c | 40 -
common/command.c | 22 ++---
env/common.c | 50 ---
include/command.h | 2 ++
include/common.h | 3 ++-
5 files changed, 105 insertions(+), 12 deletions(-)
--
2.17.1
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH 2/2] cmd: mtd: auto-complete args starting with a $ when appropriate
It's quite usual to have RAM or flash address stored in env vars.
Use $ auto-completion for such arguments.
Signed-off-by: Boris Brezillon
---
cmd/mtd.c | 40 +++-
1 file changed, 35 insertions(+), 5 deletions(-)
diff --git a/cmd/mtd.c b/cmd/mtd.c
index cda702d18b16..b13dbef3e888 100644
--- a/cmd/mtd.c
+++ b/cmd/mtd.c
@@ -517,6 +517,36 @@ static int mtd_name_complete(int argc, char * const
argv[], char last_char,
return n_found;
}
+
+static int subcmd_complete(int maxargs, int argc, char * const argv[],
+ char last_char, int maxv, char *cmdv[])
+{
+ if (argc > maxargs)
+ return 0;
+
+ if (argc <= 2)
+ return mtd_name_complete(argc, argv, last_char, maxv, cmdv);
+
+ return dollar_complete(argc, argv, last_char, maxv, cmdv);
+}
+
+static int four_args_complete(int argc, char * const argv[], char last_char,
+ int maxv, char *cmdv[])
+{
+ return subcmd_complete(5, argc, argv, last_char, maxv, cmdv);
+}
+
+static int three_args_complete(int argc, char * const argv[], char last_char,
+ int maxv, char *cmdv[])
+{
+ return subcmd_complete(4, argc, argv, last_char, maxv, cmdv);
+}
+
+static int one_arg_complete(int argc, char * const argv[], char last_char,
+ int maxv, char *cmdv[])
+{
+ return subcmd_complete(2, argc, argv, last_char, maxv, cmdv);
+}
#endif /* CONFIG_AUTO_COMPLETE */
static char mtd_help_text[] =
@@ -548,12 +578,12 @@ static char mtd_help_text[] =
U_BOOT_CMD_WITH_SUBCMDS(mtd, "MTD utils", mtd_help_text,
U_BOOT_SUBCMD_MKENT(list, 1, 1, do_mtd_list),
U_BOOT_SUBCMD_MKENT_COMPLETE(read, 5, 0, do_mtd_io,
-mtd_name_complete),
+four_args_complete),
U_BOOT_SUBCMD_MKENT_COMPLETE(write, 5, 0, do_mtd_io,
-mtd_name_complete),
+four_args_complete),
U_BOOT_SUBCMD_MKENT_COMPLETE(dump, 4, 0, do_mtd_io,
-mtd_name_complete),
+three_args_complete),
U_BOOT_SUBCMD_MKENT_COMPLETE(erase, 4, 0, do_mtd_erase,
-mtd_name_complete),
+three_args_complete),
U_BOOT_SUBCMD_MKENT_COMPLETE(bad, 2, 1, do_mtd_bad,
-mtd_name_complete));
+one_arg_complete));
--
2.17.1
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH 1/2] common: command: Provide a dollar_complete() helper
Add an helper to auto-complete arguments starting with a '$' with what's
available in the environment.
Signed-off-by: Boris Brezillon
---
common/command.c | 22 ++---
env/common.c | 50 ---
include/command.h | 2 ++
include/common.h | 3 ++-
4 files changed, 70 insertions(+), 7 deletions(-)
diff --git a/common/command.c b/common/command.c
index 19f0534a76ea..754ab9bbc396 100644
--- a/common/command.c
+++ b/common/command.c
@@ -143,22 +143,38 @@ int cmd_usage(const cmd_tbl_t *cmdtp)
#ifdef CONFIG_AUTO_COMPLETE
+static char env_complete_buf[512];
+
int var_complete(int argc, char * const argv[], char last_char, int maxv, char
*cmdv[])
{
- static char tmp_buf[512];
int space;
space = last_char == '\0' || isblank(last_char);
if (space && argc == 1)
- return env_complete("", maxv, cmdv, sizeof(tmp_buf), tmp_buf);
+ return env_complete("", maxv, cmdv, sizeof(env_complete_buf),
+ env_complete_buf, false);
if (!space && argc == 2)
- return env_complete(argv[1], maxv, cmdv, sizeof(tmp_buf),
tmp_buf);
+ return env_complete(argv[1], maxv, cmdv,
+ sizeof(env_complete_buf),
+ env_complete_buf, false);
return 0;
}
+int dollar_complete(int argc, char * const argv[], char last_char, int maxv,
+ char *cmdv[])
+{
+ /* Make sure the last argument starts with a $. */
+ if (argc < 1 || argv[argc - 1][0] != '$' ||
+ last_char == '\0' || isblank(last_char))
+ return 0;
+
+ return env_complete(argv[argc - 1], maxv, cmdv,
sizeof(env_complete_buf),
+ env_complete_buf, true);
+}
+
/*/
int complete_subcmdv(cmd_tbl_t *cmdtp, int count, int argc,
diff --git a/env/common.c b/env/common.c
index 3317cef35522..aa9a097bced0 100644
--- a/env/common.c
+++ b/env/common.c
@@ -241,31 +241,75 @@ void env_relocate(void)
}
#if defined(CONFIG_AUTO_COMPLETE) && !defined(CONFIG_SPL_BUILD)
-int env_complete(char *var, int maxv, char *cmdv[], int bufsz, char *buf)
+int env_complete(char *var, int maxv, char *cmdv[], int bufsz, char *buf,
+bool dollar_comp)
{
ENTRY *match;
int found, idx;
+ if (dollar_comp) {
+ /*
+* When doing $ completion, the first character should
+* obviously be a '$'.
+*/
+ if (var[0] != '$')
+ return 0;
+
+ var++;
+
+ /*
+* The second one, if present, should be a '{', as some
+* configuration of the u-boot shell expand ${var} but not
+* $var.
+*/
+ if (var[0] == '{')
+ var++;
+ else if (var[0] != '\0')
+ return 0;
+ }
+
idx = 0;
found = 0;
cmdv[0] = NULL;
+
while ((idx = hmatch_r(var, idx, , _htab))) {
int vallen = strlen(match->key) + 1;
- if (found >= maxv - 2 || bufsz < vallen)
+ if (found >= maxv - 2 ||
+ bufsz < vallen + (dollar_comp ? 3 : 0))
break;
cmdv[found++] = buf;
+
+ /* Add the '${' prefix to each var when doing $ completion. */
+ if (dollar_comp) {
+ strcpy(buf, "${");
+ buf += 2;
+ bufsz -= 3;
+ }
+
memcpy(buf, match->key, vallen);
buf += vallen;
bufsz -= vallen;
+
+ if (dollar_comp) {
+ /*
+* This one is a bit odd: vallen already contains the
+* '\0' character but we need to add the '}' suffix,
+* hence the buf - 1 here. strcpy() will add the '\0'
+* character just after '}'. buf is then incremented
+* to account for the extra '}' we just added.
+*/
+ strcpy(buf - 1, "}");
+ buf++;
+ }
}
qsort(cmdv, found, sizeof(cmdv[0]), strcmp_compar);
if (idx)
- cmdv[found++] = "...";
+ cmdv[found++] = dollar_comp ? "${...}" : "...";
cmdv[found] = NULL;
return found;
diff --git a/include/command.h b/include/command.h
index 461b17447c0d..c512ec6854d0 100644
--- a/include/command.h
+++ b/include/command.h
@@ -84,6 +84,8 @@ static inline bool cmd_
[U-Boot] [PATCH v4 6/6] cmd: adc: Use the sub-command infrastructure
And you get sub-command auto-completion for free.
Signed-off-by: Boris Brezillon
Reviewed-by: Tom Rini
---
Changes in v4:
-None
Changes in v3:
- Add Tom's R-b
Changes in v2:
- New patch
---
cmd/adc.c | 33 +
1 file changed, 5 insertions(+), 28 deletions(-)
diff --git a/cmd/adc.c b/cmd/adc.c
index 2d635acbd950..381961cf51a4 100644
--- a/cmd/adc.c
+++ b/cmd/adc.c
@@ -146,37 +146,14 @@ static int do_adc_scan(cmd_tbl_t *cmdtp, int flag, int
argc,
return CMD_RET_SUCCESS;
}
-static cmd_tbl_t cmd_adc_sub[] = {
- U_BOOT_CMD_MKENT(list, 1, 1, do_adc_list, "", ""),
- U_BOOT_CMD_MKENT(info, 2, 1, do_adc_info, "", ""),
- U_BOOT_CMD_MKENT(single, 3, 1, do_adc_single, "", ""),
- U_BOOT_CMD_MKENT(scan, 3, 1, do_adc_scan, "", ""),
-};
-
-static int do_adc(cmd_tbl_t *cmdtp, int flag, int argc,
- char *const argv[])
-{
- cmd_tbl_t *c;
-
- if (argc < 2)
- return CMD_RET_USAGE;
-
- /* Strip off leading 'adc' command argument */
- argc--;
- argv++;
-
- c = find_cmd_tbl(argv[0], _adc_sub[0], ARRAY_SIZE(cmd_adc_sub));
-
- if (c)
- return c->cmd(cmdtp, flag, argc, argv);
- else
- return CMD_RET_USAGE;
-}
-
static char adc_help_text[] =
"list - list ADC devices\n"
"adc info - Get ADC device info\n"
"adc single - Get Single data of ADC device channel\n"
"adc scan [channel mask] - Scan all [or masked] ADC channels";
-U_BOOT_CMD(adc, 4, 1, do_adc, "ADC sub-system", adc_help_text);
+U_BOOT_CMD_WITH_SUBCMDS(adc, "ADC sub-system", adc_help_text,
+ U_BOOT_SUBCMD_MKENT(list, 1, 1, do_adc_list),
+ U_BOOT_SUBCMD_MKENT(info, 2, 1, do_adc_info),
+ U_BOOT_SUBCMD_MKENT(single, 3, 1, do_adc_single),
+ U_BOOT_SUBCMD_MKENT(scan, 3, 1, do_adc_scan));
--
2.17.1
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v4 5/6] cmd: mtd: Use the subcmd infrastructure to declare mtd sub-commands
It's way simpler this way, and we also gain auto-completion support for
free (MTD name auto-completion has been added with mtd_name_complete())
Signed-off-by: Boris Brezillon
Reviewed-by: Tom Rini
---
Changes in v4:
- Fix maxargs in mtd bad sub-cmd
- s/do_mtd_name_complete()/mtd_name_complete()/
Changes in v3:
- Add Tom's R-b
Changes in v2:
- Adjust based on changes done in the sub-cmd infra
---
cmd/mtd.c | 476 --
1 file changed, 281 insertions(+), 195 deletions(-)
diff --git a/cmd/mtd.c b/cmd/mtd.c
index 614222398467..cda702d18b16 100644
--- a/cmd/mtd.c
+++ b/cmd/mtd.c
@@ -15,6 +15,22 @@
#include
#include
+#include
+
+static struct mtd_info *get_mtd_by_name(const char *name)
+{
+ struct mtd_info *mtd;
+
+ mtd_probe_devices();
+
+ mtd = get_mtd_device_nm(name);
+ if (IS_ERR_OR_NULL(mtd))
+ printf("MTD device %s not found, ret %ld\n", name,
+ PTR_ERR(mtd));
+
+ return mtd;
+}
+
static uint mtd_len_to_pages(struct mtd_info *mtd, u64 len)
{
do_div(len, mtd->writesize);
@@ -177,7 +193,8 @@ static bool mtd_oob_write_is_empty(struct mtd_oob_ops *op)
return true;
}
-static int do_mtd_list(void)
+static int do_mtd_list(cmd_tbl_t *cmdtp, int flag, int argc,
+ char * const argv[])
{
struct mtd_info *mtd;
int dev_nb = 0;
@@ -221,229 +238,287 @@ static int mtd_special_write_oob(struct mtd_info *mtd,
u64 off,
return ret;
}
-static int do_mtd(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+static int do_mtd_io(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
{
+ bool dump, read, raw, woob, write_empty_pages, has_pages = false;
+ u64 start_off, off, len, remaining, default_len;
+ struct mtd_oob_ops io_op = {};
+ uint user_addr = 0, npages;
+ const char *cmd = argv[0];
struct mtd_info *mtd;
- const char *cmd;
- char *mtd_name;
+ u32 oob_len;
+ u8 *buf;
+ int ret;
- /* All MTD commands need at least two arguments */
if (argc < 2)
return CMD_RET_USAGE;
- /* Parse the command name and its optional suffixes */
- cmd = argv[1];
-
- /* List the MTD devices if that is what the user wants */
- if (strcmp(cmd, "list") == 0)
- return do_mtd_list();
-
- /*
-* The remaining commands require also at least a device ID.
-* Check the selected device is valid. Ensure it is probed.
-*/
- if (argc < 3)
- return CMD_RET_USAGE;
-
- mtd_name = argv[2];
- mtd_probe_devices();
- mtd = get_mtd_device_nm(mtd_name);
- if (IS_ERR_OR_NULL(mtd)) {
- printf("MTD device %s not found, ret %ld\n",
- mtd_name, PTR_ERR(mtd));
+ mtd = get_mtd_by_name(argv[1]);
+ if (IS_ERR_OR_NULL(mtd))
return CMD_RET_FAILURE;
+
+ if (mtd->type == MTD_NANDFLASH || mtd->type == MTD_MLCNANDFLASH)
+ has_pages = true;
+
+ dump = !strncmp(cmd, "dump", 4);
+ read = dump || !strncmp(cmd, "read", 4);
+ raw = strstr(cmd, ".raw");
+ woob = strstr(cmd, ".oob");
+ write_empty_pages = !has_pages || strstr(cmd, ".dontskipff");
+
+ argc -= 2;
+ argv += 2;
+
+ if (!dump) {
+ if (!argc) {
+ ret = CMD_RET_USAGE;
+ goto out_put_mtd;
+ }
+
+ user_addr = simple_strtoul(argv[0], NULL, 16);
+ argc--;
+ argv++;
}
- put_mtd_device(mtd);
- argc -= 3;
- argv += 3;
+ start_off = argc > 0 ? simple_strtoul(argv[0], NULL, 16) : 0;
+ if (!mtd_is_aligned_with_min_io_size(mtd, start_off)) {
+ printf("Offset not aligned with a page (0x%x)\n",
+ mtd->writesize);
+ ret = CMD_RET_FAILURE;
+ goto out_put_mtd;
+ }
- /* Do the parsing */
- if (!strncmp(cmd, "read", 4) || !strncmp(cmd, "dump", 4) ||
- !strncmp(cmd, "write", 5)) {
- bool has_pages = mtd->type == MTD_NANDFLASH ||
-mtd->type == MTD_MLCNANDFLASH;
- bool dump, read, raw, woob, write_empty_pages;
- struct mtd_oob_ops io_op = {};
- uint user_addr = 0, npages;
- u64 start_off, off, len, remaining, default_len;
- u32 oob_len;
- u8 *buf;
- int ret;
+ default_len = dump ? mtd->writesize : mtd->size;
+ len = argc > 1 ? simple_strtoul(argv[1], NULL, 16) : default_len;
+ if (!mtd_is_aligned_with_min_io_size(mtd, len)) {
+ len = r
[U-Boot] [PATCH v4 2/6] common: command: Expose a generic helper to auto-complete sub commands
Some commands have a table of sub-commands. With minor adjustments,
complete_cmdv() is able to provide auto-completion for sub-commands
(it's just about passing the table of commands instead of taking the
global one).
We rename this function into complete_subcmd() and implement
complete_cmdv() as a wrapper around complete_subcmdv().
Signed-off-by: Boris Brezillon
Reviewed-by: Tom Rini
---
Changes in v4:
-None
Changes in v3:
- Add Tom's R-b
Changes in v2:
- None
---
common/command.c | 20
include/command.h | 3 +++
2 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/common/command.c b/common/command.c
index 435824356b50..e13cb47ac18b 100644
--- a/common/command.c
+++ b/common/command.c
@@ -161,11 +161,11 @@ int var_complete(int argc, char * const argv[], char
last_char, int maxv, char *
/*/
-static int complete_cmdv(int argc, char * const argv[], char last_char, int
maxv, char *cmdv[])
+int complete_subcmdv(cmd_tbl_t *cmdtp, int count, int argc,
+char * const argv[], char last_char,
+int maxv, char *cmdv[])
{
#ifdef CONFIG_CMDLINE
- cmd_tbl_t *cmdtp = ll_entry_start(cmd_tbl_t, cmd);
- const int count = ll_entry_count(cmd_tbl_t, cmd);
const cmd_tbl_t *cmdend = cmdtp + count;
const char *p;
int len, clen;
@@ -193,7 +193,7 @@ static int complete_cmdv(int argc, char * const argv[],
char last_char, int maxv
/* more than one arg or one but the start of the next */
if (argc > 1 || last_char == '\0' || isblank(last_char)) {
- cmdtp = find_cmd(argv[0]);
+ cmdtp = find_cmd_tbl(argv[0], cmdtp, count);
if (cmdtp == NULL || cmdtp->complete == NULL) {
cmdv[0] = NULL;
return 0;
@@ -238,6 +238,18 @@ static int complete_cmdv(int argc, char * const argv[],
char last_char, int maxv
#endif
}
+static int complete_cmdv(int argc, char * const argv[], char last_char,
+int maxv, char *cmdv[])
+{
+#ifdef CONFIG_CMDLINE
+ return complete_subcmdv(ll_entry_start(cmd_tbl_t, cmd),
+ ll_entry_count(cmd_tbl_t, cmd), argc, argv,
+ last_char, maxv, cmdv);
+#else
+ return 0;
+#endif
+}
+
static int make_argv(char *s, int argvsz, char *argv[])
{
int argc = 0;
diff --git a/include/command.h b/include/command.h
index 200c7a5e9f4e..89efcecfa926 100644
--- a/include/command.h
+++ b/include/command.h
@@ -54,6 +54,9 @@ int _do_help (cmd_tbl_t *cmd_start, int cmd_items, cmd_tbl_t
* cmdtp, int
flag, int argc, char * const argv[]);
cmd_tbl_t *find_cmd(const char *cmd);
cmd_tbl_t *find_cmd_tbl (const char *cmd, cmd_tbl_t *table, int table_len);
+int complete_subcmdv(cmd_tbl_t *cmdtp, int count, int argc,
+char * const argv[], char last_char, int maxv,
+char *cmdv[]);
extern int cmd_usage(const cmd_tbl_t *cmdtp);
--
2.17.1
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v4 4/6] command: commands: Add macros to declare commands with subcmds
Most cmd/xxx.c source files expose several commands through a single
entry point. Some of them are doing the sub-command parsing manually in
their do_() function, others are declaring a table of sub-commands
and then use find_cmd_tbl() to delegate the request to the sub command
handler.
In either case, the amount of code to do that is not negligible and
repetitive, not to mention that almost no commands are implementing
the auto-completion hook, which means most u-boot commands lack
auto-completion.
Provide several macros to easily define commands exposing sub-commands.
Signed-off-by: Boris Brezillon
Reviewed-by: Tom Rini
---
Changes in v4:
-None
Changes in v3:
- Add Tom's R-b
- Fix the commit message
Changes in v2:
- Adjust based on the changes done in the sub-command infra
---
include/command.h | 78 +++
1 file changed, 78 insertions(+)
diff --git a/include/command.h b/include/command.h
index bb93f022c514..461b17447c0d 100644
--- a/include/command.h
+++ b/include/command.h
@@ -209,6 +209,70 @@ int board_run_command(const char *cmdline);
# define _CMD_HELP(x)
#endif
+#ifdef CONFIG_NEEDS_MANUAL_RELOC
+#define U_BOOT_SUBCMDS_RELOC(_cmdname) \
+ static void _cmdname##_subcmds_reloc(void) \
+ { \
+ static int relocated; \
+ \
+ if (relocated) \
+ return; \
+ \
+ fixup_cmdtable(_cmdname##_subcmds, \
+ ARRAY_SIZE(_cmdname##_subcmds)); \
+ relocated = 1; \
+ }
+#else
+#define U_BOOT_SUBCMDS_RELOC(_cmdname) \
+ static void _cmdname##_subcmds_reloc(void) { }
+#endif
+
+#define U_BOOT_SUBCMDS_DO_CMD(_cmdname)
\
+ static int do_##_cmdname(cmd_tbl_t *cmdtp, int flag, int argc, \
+char * const argv[], int *repeatable) \
+ { \
+ cmd_tbl_t *subcmd; \
+ \
+ _cmdname##_subcmds_reloc(); \
+ \
+ /* We need at least the cmd and subcmd names. */\
+ if (argc < 2 || argc > CONFIG_SYS_MAXARGS) \
+ return CMD_RET_USAGE; \
+ \
+ subcmd = find_cmd_tbl(argv[1], _cmdname##_subcmds, \
+ ARRAY_SIZE(_cmdname##_subcmds)); \
+ if (!subcmd || argc - 1 > subcmd->maxargs) \
+ return CMD_RET_USAGE; \
+ \
+ if (flag == CMD_FLAG_REPEAT && \
+ !cmd_is_repeatable(subcmd)) \
+ return CMD_RET_SUCCESS; \
+ \
+ return subcmd->cmd_rep(subcmd, flag, argc - 1, \
+ argv + 1, repeatable); \
+ }
+
+#ifdef CONFIG_AUTO_COMPLETE
+#define U_BOOT_SUBCMDS_COMPLETE(_cmdname) \
+ static int complete_##_cmdname(int argc, char * const argv[], \
+ char last_char, int maxv,\
+ char *cmdv[])\
+ { \
+ return complete_subcmdv(_cmdname##_subcmds, \
+ ARRAY_SIZE(_cmdname##_subcmds), \
+ argc - 1, argv + 1, last_char, \
+ maxv, cmdv);\
+ }
+#else
+#define U_BOOT_SUBCMDS_COMPLETE(_cmdname)
+#endif
+
+#define U_BOOT_SUBCMDS(_cmdname, ...) \
+ static cmd_tbl_t _cmdname##_subcmds[] = { __VA_ARGS__ };\
+ U_BOOT_SUBCMDS_RELOC(_cmdname) \
+ U_BOOT_SUBCMDS_DO_CMD(_cmdname) \
+ U_BOOT_SUBCMDS_COMPLETE(_cmdname)
+
[U-Boot] [PATCH v4 3/6] common: command: Rework the 'cmd is repeatable' logic
The repeatable property is currently attached to the main command and
sub-commands have no way to change the repeatable value (the
->repeatable field in sub-command entries is ignored).
Replace the ->repeatable field by an extended ->cmd() hook (called
->cmd_rep()) which takes a new int pointer to store the repeatable cap
of the command being executed.
With this trick, we can let sub-commands decide whether they are
repeatable or not.
We also patch mmc and dtimg who are testing the ->repeatable field
directly (they now use cmd_is_repeatable() instead), and fix the help
entry manually since it doesn't use the U_BOOT_CMD() macro.
Signed-off-by: Boris Brezillon
Reviewed-by: Tom Rini
---
Changes in v4:
-None
Changes in v3:
- Add Tom's R-b
Changes in v2:
- New patch
---
cmd/dtimg.c | 2 +-
cmd/help.c| 2 +-
cmd/mmc.c | 4 ++--
common/command.c | 36
include/command.h | 52 +++
5 files changed, 84 insertions(+), 12 deletions(-)
diff --git a/cmd/dtimg.c b/cmd/dtimg.c
index 65c8d101b98e..ae7d82f26dd2 100644
--- a/cmd/dtimg.c
+++ b/cmd/dtimg.c
@@ -116,7 +116,7 @@ static int do_dtimg(cmd_tbl_t *cmdtp, int flag, int argc,
char * const argv[])
if (!cp || argc > cp->maxargs)
return CMD_RET_USAGE;
- if (flag == CMD_FLAG_REPEAT && !cp->repeatable)
+ if (flag == CMD_FLAG_REPEAT && !cmd_is_repeatable(cp))
return CMD_RET_SUCCESS;
return cp->cmd(cmdtp, flag, argc, argv);
diff --git a/cmd/help.c b/cmd/help.c
index 503fa632e74e..fa2010c67eb1 100644
--- a/cmd/help.c
+++ b/cmd/help.c
@@ -29,7 +29,7 @@ U_BOOT_CMD(
/* This does not use the U_BOOT_CMD macro as ? can't be used in symbol names */
ll_entry_declare(cmd_tbl_t, question_mark, cmd) = {
- "?",CONFIG_SYS_MAXARGS, 1, do_help,
+ "?",CONFIG_SYS_MAXARGS, cmd_always_repeatable, do_help,
"alias for 'help'",
#ifdef CONFIG_SYS_LONGHELP
""
diff --git a/cmd/mmc.c b/cmd/mmc.c
index 3920a1836a59..42e38900df12 100644
--- a/cmd/mmc.c
+++ b/cmd/mmc.c
@@ -247,7 +247,7 @@ static int do_mmcrpmb(cmd_tbl_t *cmdtp, int flag,
if (cp == NULL || argc > cp->maxargs)
return CMD_RET_USAGE;
- if (flag == CMD_FLAG_REPEAT && !cp->repeatable)
+ if (flag == CMD_FLAG_REPEAT && !cmd_is_repeatable(cp))
return CMD_RET_SUCCESS;
mmc = init_mmc_device(curr_device, false);
@@ -907,7 +907,7 @@ static int do_mmcops(cmd_tbl_t *cmdtp, int flag, int argc,
char * const argv[])
if (cp == NULL || argc > cp->maxargs)
return CMD_RET_USAGE;
- if (flag == CMD_FLAG_REPEAT && !cp->repeatable)
+ if (flag == CMD_FLAG_REPEAT && !cmd_is_repeatable(cp))
return CMD_RET_SUCCESS;
if (curr_device < 0) {
diff --git a/common/command.c b/common/command.c
index e13cb47ac18b..19f0534a76ea 100644
--- a/common/command.c
+++ b/common/command.c
@@ -501,6 +501,30 @@ void fixup_cmdtable(cmd_tbl_t *cmdtp, int size)
}
#endif
+int cmd_always_repeatable(cmd_tbl_t *cmdtp, int flag, int argc,
+ char * const argv[], int *repeatable)
+{
+ *repeatable = 1;
+
+ return cmdtp->cmd(cmdtp, flag, argc, argv);
+}
+
+int cmd_never_repeatable(cmd_tbl_t *cmdtp, int flag, int argc,
+char * const argv[], int *repeatable)
+{
+ *repeatable = 0;
+
+ return cmdtp->cmd(cmdtp, flag, argc, argv);
+}
+
+int cmd_discard_repeatable(cmd_tbl_t *cmdtp, int flag, int argc,
+ char * const argv[])
+{
+ int repeatable;
+
+ return cmdtp->cmd_rep(cmdtp, flag, argc, argv, );
+}
+
/**
* Call a command function. This should be the only route in U-Boot to call
* a command, so that we can track whether we are waiting for input or
@@ -510,13 +534,15 @@ void fixup_cmdtable(cmd_tbl_t *cmdtp, int size)
* @param flag Some flags normally 0 (see CMD_FLAG_.. above)
* @param argc Number of arguments (arg 0 must be the command text)
* @param argv Arguments
+ * @param repeatable Can the command be repeated
* @return 0 if command succeeded, else non-zero (CMD_RET_...)
*/
-static int cmd_call(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+static int cmd_call(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
+ int *repeatable)
{
int result;
- result = (cmdtp->cmd)(cmdtp, flag, argc, argv);
+ result = cmdtp->cmd_rep(cmdtp, flag, argc, argv, repeatable);
if (result)
debug("Command failed, result=%d\n", result);
return result;
@@ -553,12 +579,14 @@ enum command_ret_t cmd_process(int flag, int argc, char *
const argv[],
[U-Boot] [PATCH v4 0/6] cmd: Simplify support for sub-commands
Hello, Here is the 4th version of the sub-cmd patchset fixing the maxargs def of the mtd bad sub-cmd (I noticed the bug just after sending v3 :-/). Regards, Boris Boris Brezillon (6): common: command: Fix command auto-completion common: command: Expose a generic helper to auto-complete sub commands common: command: Rework the 'cmd is repeatable' logic command: commands: Add macros to declare commands with subcmds cmd: mtd: Use the subcmd infrastructure to declare mtd sub-commands cmd: adc: Use the sub-command infrastructure cmd/adc.c | 33 +--- cmd/dtimg.c | 2 +- cmd/help.c| 2 +- cmd/mmc.c | 4 +- cmd/mtd.c | 476 +++--- common/command.c | 68 ++- include/command.h | 133 - 7 files changed, 477 insertions(+), 241 deletions(-) -- 2.17.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v4 1/6] common: command: Fix command auto-completion
When auto-completing command arguments, the last argument is not
necessarily the one we need to auto-complete. When the last character is
a space, a tab or '\0' what we want instead is list all possible values,
or if there's only one possible value, place this value on the command
line instead of trying to suffix the last valid argument with missing
chars.
Signed-off-by: Boris Brezillon
Reviewed-by: Tom Rini
---
Changes in v4:
-None
Changes in v3:
- Add Tom's R-b
Changes in v2:
- None
---
common/command.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/common/command.c b/common/command.c
index 2433a89e0a8e..435824356b50 100644
--- a/common/command.c
+++ b/common/command.c
@@ -362,13 +362,21 @@ int cmd_auto_complete(const char *const prompt, char
*buf, int *np, int *colp)
sep = NULL;
seplen = 0;
if (i == 1) { /* one match; perfect */
- k = strlen(argv[argc - 1]);
+ if (last_char != '\0' && !isblank(last_char))
+ k = strlen(argv[argc - 1]);
+ else
+ k = 0;
+
s = cmdv[0] + k;
len = strlen(s);
sep = " ";
seplen = 1;
} else if (i > 1 && (j = find_common_prefix(cmdv)) != 0) { /* more */
- k = strlen(argv[argc - 1]);
+ if (last_char != '\0' && !isblank(last_char))
+ k = strlen(argv[argc - 1]);
+ else
+ k = 0;
+
j -= k;
if (j > 0) {
s = cmdv[0] + k;
--
2.17.1
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v3 0/6] cmd: Simplify support for sub-commands
On Mon, 3 Dec 2018 22:14:52 +0100 Boris Brezillon wrote: > Hello, > > Here is the 3nd version of the sub-cmd patchset. This version fixes a > few typos in commit messages. > > Regards, > > Boris > > Boris Brezillon (6): > common: command: Fix command auto-completion > common: command: Expose a generic helper to auto-complete sub commands > common: command: Rework the 'cmd is repeatable' logic > command: commands: Add macros to declare commands with subcmds > cmd: mtd: Use the subcmd infrastructure to declare mtd sub-commands I found a bug in this patch. Please ignore this version, I'll send a new one. > cmd: adc: Use the sub-command infrastructure > > cmd/adc.c | 33 +--- > cmd/dtimg.c | 2 +- > cmd/help.c| 2 +- > cmd/mmc.c | 4 +- > cmd/mtd.c | 476 +++--- > common/command.c | 68 ++- > include/command.h | 133 - > 7 files changed, 477 insertions(+), 241 deletions(-) > ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v3 5/6] cmd: mtd: Use the subcmd infrastructure to declare mtd sub-commands
On Mon, 3 Dec 2018 22:14:57 +0100
Boris Brezillon wrote:
> It's way simpler this way, and we also gain auto-completion support for
> free (MTD name auto-completion has been added with do_mtd_name_complete())
>
> Signed-off-by: Boris Brezillon
> Reviewed-by: Tom Rini
> ---
> Changes in v3:
> - Add Tom's R-b
>
> Changes in v2:
> - Adjust based on changes done in the sub-cmd infra
> ---
> cmd/mtd.c | 476 --
> 1 file changed, 281 insertions(+), 195 deletions(-)
>
> diff --git a/cmd/mtd.c b/cmd/mtd.c
> index 614222398467..5b415aaa1d86 100644
> --- a/cmd/mtd.c
> +++ b/cmd/mtd.c
> @@ -15,6 +15,22 @@
> #include
> #include
>
> +#include
> +
> +static struct mtd_info *get_mtd_by_name(const char *name)
> +{
> + struct mtd_info *mtd;
> +
> + mtd_probe_devices();
> +
> + mtd = get_mtd_device_nm(name);
> + if (IS_ERR_OR_NULL(mtd))
> + printf("MTD device %s not found, ret %ld\n", name,
> +PTR_ERR(mtd));
> +
> + return mtd;
> +}
> +
> static uint mtd_len_to_pages(struct mtd_info *mtd, u64 len)
> {
> do_div(len, mtd->writesize);
> @@ -177,7 +193,8 @@ static bool mtd_oob_write_is_empty(struct mtd_oob_ops *op)
> return true;
> }
>
> -static int do_mtd_list(void)
> +static int do_mtd_list(cmd_tbl_t *cmdtp, int flag, int argc,
> +char * const argv[])
> {
> struct mtd_info *mtd;
> int dev_nb = 0;
> @@ -221,229 +238,287 @@ static int mtd_special_write_oob(struct mtd_info
> *mtd, u64 off,
> return ret;
> }
>
> -static int do_mtd(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> +static int do_mtd_io(cmd_tbl_t *cmdtp, int flag, int argc, char * const
> argv[])
> {
> + bool dump, read, raw, woob, write_empty_pages, has_pages = false;
> + u64 start_off, off, len, remaining, default_len;
> + struct mtd_oob_ops io_op = {};
> + uint user_addr = 0, npages;
> + const char *cmd = argv[0];
> struct mtd_info *mtd;
> - const char *cmd;
> - char *mtd_name;
> + u32 oob_len;
> + u8 *buf;
> + int ret;
>
> - /* All MTD commands need at least two arguments */
> if (argc < 2)
> return CMD_RET_USAGE;
>
> - /* Parse the command name and its optional suffixes */
> - cmd = argv[1];
> -
> - /* List the MTD devices if that is what the user wants */
> - if (strcmp(cmd, "list") == 0)
> - return do_mtd_list();
> -
> - /*
> - * The remaining commands require also at least a device ID.
> - * Check the selected device is valid. Ensure it is probed.
> - */
> - if (argc < 3)
> - return CMD_RET_USAGE;
> -
> - mtd_name = argv[2];
> - mtd_probe_devices();
> - mtd = get_mtd_device_nm(mtd_name);
> - if (IS_ERR_OR_NULL(mtd)) {
> - printf("MTD device %s not found, ret %ld\n",
> -mtd_name, PTR_ERR(mtd));
> + mtd = get_mtd_by_name(argv[1]);
> + if (IS_ERR_OR_NULL(mtd))
> return CMD_RET_FAILURE;
> +
> + if (mtd->type == MTD_NANDFLASH || mtd->type == MTD_MLCNANDFLASH)
> + has_pages = true;
> +
> + dump = !strncmp(cmd, "dump", 4);
> + read = dump || !strncmp(cmd, "read", 4);
> + raw = strstr(cmd, ".raw");
> + woob = strstr(cmd, ".oob");
> + write_empty_pages = !has_pages || strstr(cmd, ".dontskipff");
> +
> + argc -= 2;
> + argv += 2;
> +
> + if (!dump) {
> + if (!argc) {
> + ret = CMD_RET_USAGE;
> + goto out_put_mtd;
> + }
> +
> + user_addr = simple_strtoul(argv[0], NULL, 16);
> + argc--;
> + argv++;
> }
> - put_mtd_device(mtd);
>
> - argc -= 3;
> - argv += 3;
> + start_off = argc > 0 ? simple_strtoul(argv[0], NULL, 16) : 0;
> + if (!mtd_is_aligned_with_min_io_size(mtd, start_off)) {
> + printf("Offset not aligned with a page (0x%x)\n",
> +mtd->writesize);
> + ret = CMD_RET_FAILURE;
> + goto out_put_mtd;
> + }
>
> - /* Do the parsing */
> - if (!strncmp(cmd, "read", 4) || !strncmp(cmd, "dump", 4) ||
> - !strncmp(cmd, "write", 5)) {
> - bool has_pages = mtd->type == MTD_NANDFLASH ||
> - mtd->type == MTD_MLCNANDFLASH;
> -
[U-Boot] [PATCH v3 6/6] cmd: adc: Use the sub-command infrastructure
And you get sub-command auto-completion for free.
Signed-off-by: Boris Brezillon
Reviewed-by: Tom Rini
---
Changes in v3:
- Add Tom's R-b
Changes in v2:
- New patch
---
cmd/adc.c | 33 +
1 file changed, 5 insertions(+), 28 deletions(-)
diff --git a/cmd/adc.c b/cmd/adc.c
index 2d635acbd950..381961cf51a4 100644
--- a/cmd/adc.c
+++ b/cmd/adc.c
@@ -146,37 +146,14 @@ static int do_adc_scan(cmd_tbl_t *cmdtp, int flag, int
argc,
return CMD_RET_SUCCESS;
}
-static cmd_tbl_t cmd_adc_sub[] = {
- U_BOOT_CMD_MKENT(list, 1, 1, do_adc_list, "", ""),
- U_BOOT_CMD_MKENT(info, 2, 1, do_adc_info, "", ""),
- U_BOOT_CMD_MKENT(single, 3, 1, do_adc_single, "", ""),
- U_BOOT_CMD_MKENT(scan, 3, 1, do_adc_scan, "", ""),
-};
-
-static int do_adc(cmd_tbl_t *cmdtp, int flag, int argc,
- char *const argv[])
-{
- cmd_tbl_t *c;
-
- if (argc < 2)
- return CMD_RET_USAGE;
-
- /* Strip off leading 'adc' command argument */
- argc--;
- argv++;
-
- c = find_cmd_tbl(argv[0], _adc_sub[0], ARRAY_SIZE(cmd_adc_sub));
-
- if (c)
- return c->cmd(cmdtp, flag, argc, argv);
- else
- return CMD_RET_USAGE;
-}
-
static char adc_help_text[] =
"list - list ADC devices\n"
"adc info - Get ADC device info\n"
"adc single - Get Single data of ADC device channel\n"
"adc scan [channel mask] - Scan all [or masked] ADC channels";
-U_BOOT_CMD(adc, 4, 1, do_adc, "ADC sub-system", adc_help_text);
+U_BOOT_CMD_WITH_SUBCMDS(adc, "ADC sub-system", adc_help_text,
+ U_BOOT_SUBCMD_MKENT(list, 1, 1, do_adc_list),
+ U_BOOT_SUBCMD_MKENT(info, 2, 1, do_adc_info),
+ U_BOOT_SUBCMD_MKENT(single, 3, 1, do_adc_single),
+ U_BOOT_SUBCMD_MKENT(scan, 3, 1, do_adc_scan));
--
2.17.1
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v3 4/6] command: commands: Add macros to declare commands with subcmds
Most cmd/xxx.c source files expose several commands through a single
entry point. Some of them are doing the sub-command parsing manually in
their do_() function, others are declaring a table of sub-commands
and then use find_cmd_tbl() to delegate the request to the sub command
handler.
In either case, the amount of code to do that is not negligible and
repetitive, not to mention that almost no commands are implementing
the auto-completion hook, which means most u-boot commands lack
auto-completion.
Provide several macros to easily define commands exposing sub-commands.
Signed-off-by: Boris Brezillon
Reviewed-by: Tom Rini
---
Changes in v3:
- Add Tom's R-b
- Fix the commit message
Changes in v2:
- Adjust based on the changes done in the sub-command infra
---
include/command.h | 78 +++
1 file changed, 78 insertions(+)
diff --git a/include/command.h b/include/command.h
index bb93f022c514..461b17447c0d 100644
--- a/include/command.h
+++ b/include/command.h
@@ -209,6 +209,70 @@ int board_run_command(const char *cmdline);
# define _CMD_HELP(x)
#endif
+#ifdef CONFIG_NEEDS_MANUAL_RELOC
+#define U_BOOT_SUBCMDS_RELOC(_cmdname) \
+ static void _cmdname##_subcmds_reloc(void) \
+ { \
+ static int relocated; \
+ \
+ if (relocated) \
+ return; \
+ \
+ fixup_cmdtable(_cmdname##_subcmds, \
+ ARRAY_SIZE(_cmdname##_subcmds)); \
+ relocated = 1; \
+ }
+#else
+#define U_BOOT_SUBCMDS_RELOC(_cmdname) \
+ static void _cmdname##_subcmds_reloc(void) { }
+#endif
+
+#define U_BOOT_SUBCMDS_DO_CMD(_cmdname)
\
+ static int do_##_cmdname(cmd_tbl_t *cmdtp, int flag, int argc, \
+char * const argv[], int *repeatable) \
+ { \
+ cmd_tbl_t *subcmd; \
+ \
+ _cmdname##_subcmds_reloc(); \
+ \
+ /* We need at least the cmd and subcmd names. */\
+ if (argc < 2 || argc > CONFIG_SYS_MAXARGS) \
+ return CMD_RET_USAGE; \
+ \
+ subcmd = find_cmd_tbl(argv[1], _cmdname##_subcmds, \
+ ARRAY_SIZE(_cmdname##_subcmds)); \
+ if (!subcmd || argc - 1 > subcmd->maxargs) \
+ return CMD_RET_USAGE; \
+ \
+ if (flag == CMD_FLAG_REPEAT && \
+ !cmd_is_repeatable(subcmd)) \
+ return CMD_RET_SUCCESS; \
+ \
+ return subcmd->cmd_rep(subcmd, flag, argc - 1, \
+ argv + 1, repeatable); \
+ }
+
+#ifdef CONFIG_AUTO_COMPLETE
+#define U_BOOT_SUBCMDS_COMPLETE(_cmdname) \
+ static int complete_##_cmdname(int argc, char * const argv[], \
+ char last_char, int maxv,\
+ char *cmdv[])\
+ { \
+ return complete_subcmdv(_cmdname##_subcmds, \
+ ARRAY_SIZE(_cmdname##_subcmds), \
+ argc - 1, argv + 1, last_char, \
+ maxv, cmdv);\
+ }
+#else
+#define U_BOOT_SUBCMDS_COMPLETE(_cmdname)
+#endif
+
+#define U_BOOT_SUBCMDS(_cmdname, ...) \
+ static cmd_tbl_t _cmdname##_subcmds[] = { __VA_ARGS__ };\
+ U_BOOT_SUBCMDS_RELOC(_cmdname) \
+ U_BOOT_SUBCMDS_DO_CMD(_cmdname) \
+ U_BOOT_SUBCMDS_COMPLETE(_cmdname)
+
#ifdef
[U-Boot] [PATCH v3 5/6] cmd: mtd: Use the subcmd infrastructure to declare mtd sub-commands
It's way simpler this way, and we also gain auto-completion support for
free (MTD name auto-completion has been added with do_mtd_name_complete())
Signed-off-by: Boris Brezillon
Reviewed-by: Tom Rini
---
Changes in v3:
- Add Tom's R-b
Changes in v2:
- Adjust based on changes done in the sub-cmd infra
---
cmd/mtd.c | 476 --
1 file changed, 281 insertions(+), 195 deletions(-)
diff --git a/cmd/mtd.c b/cmd/mtd.c
index 614222398467..5b415aaa1d86 100644
--- a/cmd/mtd.c
+++ b/cmd/mtd.c
@@ -15,6 +15,22 @@
#include
#include
+#include
+
+static struct mtd_info *get_mtd_by_name(const char *name)
+{
+ struct mtd_info *mtd;
+
+ mtd_probe_devices();
+
+ mtd = get_mtd_device_nm(name);
+ if (IS_ERR_OR_NULL(mtd))
+ printf("MTD device %s not found, ret %ld\n", name,
+ PTR_ERR(mtd));
+
+ return mtd;
+}
+
static uint mtd_len_to_pages(struct mtd_info *mtd, u64 len)
{
do_div(len, mtd->writesize);
@@ -177,7 +193,8 @@ static bool mtd_oob_write_is_empty(struct mtd_oob_ops *op)
return true;
}
-static int do_mtd_list(void)
+static int do_mtd_list(cmd_tbl_t *cmdtp, int flag, int argc,
+ char * const argv[])
{
struct mtd_info *mtd;
int dev_nb = 0;
@@ -221,229 +238,287 @@ static int mtd_special_write_oob(struct mtd_info *mtd,
u64 off,
return ret;
}
-static int do_mtd(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+static int do_mtd_io(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
{
+ bool dump, read, raw, woob, write_empty_pages, has_pages = false;
+ u64 start_off, off, len, remaining, default_len;
+ struct mtd_oob_ops io_op = {};
+ uint user_addr = 0, npages;
+ const char *cmd = argv[0];
struct mtd_info *mtd;
- const char *cmd;
- char *mtd_name;
+ u32 oob_len;
+ u8 *buf;
+ int ret;
- /* All MTD commands need at least two arguments */
if (argc < 2)
return CMD_RET_USAGE;
- /* Parse the command name and its optional suffixes */
- cmd = argv[1];
-
- /* List the MTD devices if that is what the user wants */
- if (strcmp(cmd, "list") == 0)
- return do_mtd_list();
-
- /*
-* The remaining commands require also at least a device ID.
-* Check the selected device is valid. Ensure it is probed.
-*/
- if (argc < 3)
- return CMD_RET_USAGE;
-
- mtd_name = argv[2];
- mtd_probe_devices();
- mtd = get_mtd_device_nm(mtd_name);
- if (IS_ERR_OR_NULL(mtd)) {
- printf("MTD device %s not found, ret %ld\n",
- mtd_name, PTR_ERR(mtd));
+ mtd = get_mtd_by_name(argv[1]);
+ if (IS_ERR_OR_NULL(mtd))
return CMD_RET_FAILURE;
+
+ if (mtd->type == MTD_NANDFLASH || mtd->type == MTD_MLCNANDFLASH)
+ has_pages = true;
+
+ dump = !strncmp(cmd, "dump", 4);
+ read = dump || !strncmp(cmd, "read", 4);
+ raw = strstr(cmd, ".raw");
+ woob = strstr(cmd, ".oob");
+ write_empty_pages = !has_pages || strstr(cmd, ".dontskipff");
+
+ argc -= 2;
+ argv += 2;
+
+ if (!dump) {
+ if (!argc) {
+ ret = CMD_RET_USAGE;
+ goto out_put_mtd;
+ }
+
+ user_addr = simple_strtoul(argv[0], NULL, 16);
+ argc--;
+ argv++;
}
- put_mtd_device(mtd);
- argc -= 3;
- argv += 3;
+ start_off = argc > 0 ? simple_strtoul(argv[0], NULL, 16) : 0;
+ if (!mtd_is_aligned_with_min_io_size(mtd, start_off)) {
+ printf("Offset not aligned with a page (0x%x)\n",
+ mtd->writesize);
+ ret = CMD_RET_FAILURE;
+ goto out_put_mtd;
+ }
- /* Do the parsing */
- if (!strncmp(cmd, "read", 4) || !strncmp(cmd, "dump", 4) ||
- !strncmp(cmd, "write", 5)) {
- bool has_pages = mtd->type == MTD_NANDFLASH ||
-mtd->type == MTD_MLCNANDFLASH;
- bool dump, read, raw, woob, write_empty_pages;
- struct mtd_oob_ops io_op = {};
- uint user_addr = 0, npages;
- u64 start_off, off, len, remaining, default_len;
- u32 oob_len;
- u8 *buf;
- int ret;
+ default_len = dump ? mtd->writesize : mtd->size;
+ len = argc > 1 ? simple_strtoul(argv[1], NULL, 16) : default_len;
+ if (!mtd_is_aligned_with_min_io_size(mtd, len)) {
+ len = round_up(len, mtd->writesize);
+ printf("Size not on a page boundary (
[U-Boot] [PATCH v3 3/6] common: command: Rework the 'cmd is repeatable' logic
The repeatable property is currently attached to the main command and
sub-commands have no way to change the repeatable value (the
->repeatable field in sub-command entries is ignored).
Replace the ->repeatable field by an extended ->cmd() hook (called
->cmd_rep()) which takes a new int pointer to store the repeatable cap
of the command being executed.
With this trick, we can let sub-commands decide whether they are
repeatable or not.
We also patch mmc and dtimg who are testing the ->repeatable field
directly (they now use cmd_is_repeatable() instead), and fix the help
entry manually since it doesn't use the U_BOOT_CMD() macro.
Signed-off-by: Boris Brezillon
Reviewed-by: Tom Rini
---
Changes in v3:
- Add Tom's R-b
Changes in v2:
- New patch
---
cmd/dtimg.c | 2 +-
cmd/help.c| 2 +-
cmd/mmc.c | 4 ++--
common/command.c | 36
include/command.h | 52 +++
5 files changed, 84 insertions(+), 12 deletions(-)
diff --git a/cmd/dtimg.c b/cmd/dtimg.c
index 65c8d101b98e..ae7d82f26dd2 100644
--- a/cmd/dtimg.c
+++ b/cmd/dtimg.c
@@ -116,7 +116,7 @@ static int do_dtimg(cmd_tbl_t *cmdtp, int flag, int argc,
char * const argv[])
if (!cp || argc > cp->maxargs)
return CMD_RET_USAGE;
- if (flag == CMD_FLAG_REPEAT && !cp->repeatable)
+ if (flag == CMD_FLAG_REPEAT && !cmd_is_repeatable(cp))
return CMD_RET_SUCCESS;
return cp->cmd(cmdtp, flag, argc, argv);
diff --git a/cmd/help.c b/cmd/help.c
index 503fa632e74e..fa2010c67eb1 100644
--- a/cmd/help.c
+++ b/cmd/help.c
@@ -29,7 +29,7 @@ U_BOOT_CMD(
/* This does not use the U_BOOT_CMD macro as ? can't be used in symbol names */
ll_entry_declare(cmd_tbl_t, question_mark, cmd) = {
- "?",CONFIG_SYS_MAXARGS, 1, do_help,
+ "?",CONFIG_SYS_MAXARGS, cmd_always_repeatable, do_help,
"alias for 'help'",
#ifdef CONFIG_SYS_LONGHELP
""
diff --git a/cmd/mmc.c b/cmd/mmc.c
index 3920a1836a59..42e38900df12 100644
--- a/cmd/mmc.c
+++ b/cmd/mmc.c
@@ -247,7 +247,7 @@ static int do_mmcrpmb(cmd_tbl_t *cmdtp, int flag,
if (cp == NULL || argc > cp->maxargs)
return CMD_RET_USAGE;
- if (flag == CMD_FLAG_REPEAT && !cp->repeatable)
+ if (flag == CMD_FLAG_REPEAT && !cmd_is_repeatable(cp))
return CMD_RET_SUCCESS;
mmc = init_mmc_device(curr_device, false);
@@ -907,7 +907,7 @@ static int do_mmcops(cmd_tbl_t *cmdtp, int flag, int argc,
char * const argv[])
if (cp == NULL || argc > cp->maxargs)
return CMD_RET_USAGE;
- if (flag == CMD_FLAG_REPEAT && !cp->repeatable)
+ if (flag == CMD_FLAG_REPEAT && !cmd_is_repeatable(cp))
return CMD_RET_SUCCESS;
if (curr_device < 0) {
diff --git a/common/command.c b/common/command.c
index e13cb47ac18b..19f0534a76ea 100644
--- a/common/command.c
+++ b/common/command.c
@@ -501,6 +501,30 @@ void fixup_cmdtable(cmd_tbl_t *cmdtp, int size)
}
#endif
+int cmd_always_repeatable(cmd_tbl_t *cmdtp, int flag, int argc,
+ char * const argv[], int *repeatable)
+{
+ *repeatable = 1;
+
+ return cmdtp->cmd(cmdtp, flag, argc, argv);
+}
+
+int cmd_never_repeatable(cmd_tbl_t *cmdtp, int flag, int argc,
+char * const argv[], int *repeatable)
+{
+ *repeatable = 0;
+
+ return cmdtp->cmd(cmdtp, flag, argc, argv);
+}
+
+int cmd_discard_repeatable(cmd_tbl_t *cmdtp, int flag, int argc,
+ char * const argv[])
+{
+ int repeatable;
+
+ return cmdtp->cmd_rep(cmdtp, flag, argc, argv, );
+}
+
/**
* Call a command function. This should be the only route in U-Boot to call
* a command, so that we can track whether we are waiting for input or
@@ -510,13 +534,15 @@ void fixup_cmdtable(cmd_tbl_t *cmdtp, int size)
* @param flag Some flags normally 0 (see CMD_FLAG_.. above)
* @param argc Number of arguments (arg 0 must be the command text)
* @param argv Arguments
+ * @param repeatable Can the command be repeated
* @return 0 if command succeeded, else non-zero (CMD_RET_...)
*/
-static int cmd_call(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+static int cmd_call(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
+ int *repeatable)
{
int result;
- result = (cmdtp->cmd)(cmdtp, flag, argc, argv);
+ result = cmdtp->cmd_rep(cmdtp, flag, argc, argv, repeatable);
if (result)
debug("Command failed, result=%d\n", result);
return result;
@@ -553,12 +579,14 @@ enum command_ret_t cmd_process(int flag, int argc, char *
const argv[],
/* If OK so far, then do the command
[U-Boot] [PATCH v3 1/6] common: command: Fix command auto-completion
When auto-completing command arguments, the last argument is not
necessarily the one we need to auto-complete. When the last character is
a space, a tab or '\0' what we want instead is list all possible values,
or if there's only one possible value, place this value on the command
line instead of trying to suffix the last valid argument with missing
chars.
Signed-off-by: Boris Brezillon
Reviewed-by: Tom Rini
---
Changes in v3:
- Add Tom's R-b
Changes in v2:
- None
---
common/command.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/common/command.c b/common/command.c
index 2433a89e0a8e..435824356b50 100644
--- a/common/command.c
+++ b/common/command.c
@@ -362,13 +362,21 @@ int cmd_auto_complete(const char *const prompt, char
*buf, int *np, int *colp)
sep = NULL;
seplen = 0;
if (i == 1) { /* one match; perfect */
- k = strlen(argv[argc - 1]);
+ if (last_char != '\0' && !isblank(last_char))
+ k = strlen(argv[argc - 1]);
+ else
+ k = 0;
+
s = cmdv[0] + k;
len = strlen(s);
sep = " ";
seplen = 1;
} else if (i > 1 && (j = find_common_prefix(cmdv)) != 0) { /* more */
- k = strlen(argv[argc - 1]);
+ if (last_char != '\0' && !isblank(last_char))
+ k = strlen(argv[argc - 1]);
+ else
+ k = 0;
+
j -= k;
if (j > 0) {
s = cmdv[0] + k;
--
2.17.1
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v3 2/6] common: command: Expose a generic helper to auto-complete sub commands
Some commands have a table of sub-commands. With minor adjustments,
complete_cmdv() is able to provide auto-completion for sub-commands
(it's just about passing the table of commands instead of taking the
global one).
We rename this function into complete_subcmd() and implement
complete_cmdv() as a wrapper around complete_subcmdv().
Signed-off-by: Boris Brezillon
Reviewed-by: Tom Rini
---
Changes in v3:
- Add Tom's R-b
Changes in v2:
- None
---
common/command.c | 20
include/command.h | 3 +++
2 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/common/command.c b/common/command.c
index 435824356b50..e13cb47ac18b 100644
--- a/common/command.c
+++ b/common/command.c
@@ -161,11 +161,11 @@ int var_complete(int argc, char * const argv[], char
last_char, int maxv, char *
/*/
-static int complete_cmdv(int argc, char * const argv[], char last_char, int
maxv, char *cmdv[])
+int complete_subcmdv(cmd_tbl_t *cmdtp, int count, int argc,
+char * const argv[], char last_char,
+int maxv, char *cmdv[])
{
#ifdef CONFIG_CMDLINE
- cmd_tbl_t *cmdtp = ll_entry_start(cmd_tbl_t, cmd);
- const int count = ll_entry_count(cmd_tbl_t, cmd);
const cmd_tbl_t *cmdend = cmdtp + count;
const char *p;
int len, clen;
@@ -193,7 +193,7 @@ static int complete_cmdv(int argc, char * const argv[],
char last_char, int maxv
/* more than one arg or one but the start of the next */
if (argc > 1 || last_char == '\0' || isblank(last_char)) {
- cmdtp = find_cmd(argv[0]);
+ cmdtp = find_cmd_tbl(argv[0], cmdtp, count);
if (cmdtp == NULL || cmdtp->complete == NULL) {
cmdv[0] = NULL;
return 0;
@@ -238,6 +238,18 @@ static int complete_cmdv(int argc, char * const argv[],
char last_char, int maxv
#endif
}
+static int complete_cmdv(int argc, char * const argv[], char last_char,
+int maxv, char *cmdv[])
+{
+#ifdef CONFIG_CMDLINE
+ return complete_subcmdv(ll_entry_start(cmd_tbl_t, cmd),
+ ll_entry_count(cmd_tbl_t, cmd), argc, argv,
+ last_char, maxv, cmdv);
+#else
+ return 0;
+#endif
+}
+
static int make_argv(char *s, int argvsz, char *argv[])
{
int argc = 0;
diff --git a/include/command.h b/include/command.h
index 200c7a5e9f4e..89efcecfa926 100644
--- a/include/command.h
+++ b/include/command.h
@@ -54,6 +54,9 @@ int _do_help (cmd_tbl_t *cmd_start, int cmd_items, cmd_tbl_t
* cmdtp, int
flag, int argc, char * const argv[]);
cmd_tbl_t *find_cmd(const char *cmd);
cmd_tbl_t *find_cmd_tbl (const char *cmd, cmd_tbl_t *table, int table_len);
+int complete_subcmdv(cmd_tbl_t *cmdtp, int count, int argc,
+char * const argv[], char last_char, int maxv,
+char *cmdv[]);
extern int cmd_usage(const cmd_tbl_t *cmdtp);
--
2.17.1
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v3 0/6] cmd: Simplify support for sub-commands
Hello, Here is the 3nd version of the sub-cmd patchset. This version fixes a few typos in commit messages. Regards, Boris Boris Brezillon (6): common: command: Fix command auto-completion common: command: Expose a generic helper to auto-complete sub commands common: command: Rework the 'cmd is repeatable' logic command: commands: Add macros to declare commands with subcmds cmd: mtd: Use the subcmd infrastructure to declare mtd sub-commands cmd: adc: Use the sub-command infrastructure cmd/adc.c | 33 +--- cmd/dtimg.c | 2 +- cmd/help.c| 2 +- cmd/mmc.c | 4 +- cmd/mtd.c | 476 +++--- common/command.c | 68 ++- include/command.h | 133 - 7 files changed, 477 insertions(+), 241 deletions(-) -- 2.17.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v4 08/11] mtd: Make sure we don't parse MTD partitions belonging to another dev
The mtdparts variable might contain partition definitions for several
MTD devices. Each partition layout is separated by a ';', so let's
make sure we don't pick a wrong name when mtdparts is malformed.
Fixes: 5db66b3aee6f ("cmd: mtd: add 'mtd' command")
Signed-off-by: Boris Brezillon
Tested-by: Heiko Schocher
---
Changes in v4:
- Add T-b tag
Changes in v3:
- None
Changes in v2:
- New patch
---
drivers/mtd/mtd_uboot.c | 17 -
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/mtd/mtd_uboot.c b/drivers/mtd/mtd_uboot.c
index 0eda36278309..6a36948b917b 100644
--- a/drivers/mtd/mtd_uboot.c
+++ b/drivers/mtd/mtd_uboot.c
@@ -155,6 +155,7 @@ int mtd_probe_devices(void)
static char *old_mtdids;
const char *mtdparts = get_mtdparts();
const char *mtdids = get_mtdids();
+ const char *mtdparts_next = mtdparts;
bool remaining_partitions = true;
struct mtd_info *mtd;
@@ -219,13 +220,22 @@ int mtd_probe_devices(void)
mtdparts += 9;
/* For each MTD device in mtdparts */
- while (mtdparts[0] != '\0') {
+ for (; mtdparts[0] != '\0'; mtdparts = mtdparts_next) {
char mtd_name[MTD_NAME_MAX_LEN], *colon;
struct mtd_partition *parts;
unsigned int mtd_name_len;
int nparts, ret;
+ mtdparts_next = strchr(mtdparts, ';');
+ if (!mtdparts_next)
+ mtdparts_next = mtdparts + strlen(mtdparts);
+ else
+ mtdparts_next++;
+
colon = strchr(mtdparts, ':');
+ if (colon > mtdparts_next)
+ colon = NULL;
+
if (!colon) {
printf("Wrong mtdparts: %s\n", mtdparts);
return -EINVAL;
@@ -263,10 +273,7 @@ int mtd_probe_devices(void)
if (ret || IS_ERR_OR_NULL(mtd)) {
printf("Could not find a valid device for %s\n",
mtd_name);
- mtdparts = strchr(mtdparts, ';');
- if (mtdparts)
- mtdparts++;
-
+ mtdparts = mtdparts_next;
continue;
}
}
--
2.17.1
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v4 09/11] mtd: Don't stop MTD partition creation when it fails on one device
MTD partition creation code is a bit tricky. It tries to figure out
when things have changed (either MTD dev list or mtdparts/mtdids vars)
and when that happens it first deletes all the partitions that had been
previously created and then creates the new ones based on the new
mtdparts/mtdids values.
But before deleting the old partitions, it ensures that none of the
currently registered parts are being used and bails out when that's
not the case. So, we end up in a situation where, if at least one MTD
dev has one of its partitions used by someone (UBI for instance), the
partitions update logic no longer works for other devs.
Rework the code to relax the logic and allow updates of MTD parts on
devices that are not being used (we still refuse to updates parts on
devices who have at least one of their partitions used by someone).
Fixes: 5db66b3aee6f ("cmd: mtd: add 'mtd' command")
Signed-off-by: Boris Brezillon
Tested-by: Heiko Schocher
---
Changes in v4:
- Add T-b tag
Changes in v3:
- None
Changes in v2:
- New patch
Changes in v3:
- Re-create partitions when our last attempt do delete all existing
parts failed. This way we can update parts after ubi detach has
been called without having to change mtdparts/mtdids.
---
drivers/mtd/mtd_uboot.c | 96 +
drivers/mtd/mtdpart.c | 12 ++
include/linux/mtd/mtd.h | 2 +
3 files changed, 82 insertions(+), 28 deletions(-)
diff --git a/drivers/mtd/mtd_uboot.c b/drivers/mtd/mtd_uboot.c
index 6a36948b917b..d638f700d041 100644
--- a/drivers/mtd/mtd_uboot.c
+++ b/drivers/mtd/mtd_uboot.c
@@ -149,6 +149,54 @@ static const char *get_mtdparts(void)
return mtdparts;
}
+static int mtd_del_parts(struct mtd_info *mtd, bool quiet)
+{
+ int ret;
+
+ if (!mtd_has_partitions(mtd))
+ return 0;
+
+ /* do not delete partitions if they are in use. */
+ if (mtd_partitions_used(mtd)) {
+ if (!quiet)
+ printf("\"%s\" partitions still in use, can't delete
them\n",
+ mtd->name);
+ return -EACCES;
+ }
+
+ ret = del_mtd_partitions(mtd);
+ if (ret)
+ return ret;
+
+ return 1;
+}
+
+static bool mtd_del_all_parts_failed;
+
+static void mtd_del_all_parts(void)
+{
+ struct mtd_info *mtd;
+ int ret = 0;
+
+ mtd_del_all_parts_failed = false;
+
+ /*
+* It is not safe to remove entries from the mtd_for_each_device loop
+* as it uses idr indexes and the partitions removal is done in bulk
+* (all partitions of one device at the same time), so break and
+* iterate from start each time a new partition is found and deleted.
+*/
+ do {
+ mtd_for_each_device(mtd) {
+ ret = mtd_del_parts(mtd, false);
+ if (ret > 0)
+ break;
+ else if (ret < 0)
+ mtd_del_all_parts_failed = true;
+ }
+ } while (ret > 0);
+}
+
int mtd_probe_devices(void)
{
static char *old_mtdparts;
@@ -156,18 +204,19 @@ int mtd_probe_devices(void)
const char *mtdparts = get_mtdparts();
const char *mtdids = get_mtdids();
const char *mtdparts_next = mtdparts;
- bool remaining_partitions = true;
struct mtd_info *mtd;
mtd_probe_uclass_mtd_devs();
/*
-* Check if mtdparts/mtdids changed or if the MTD dev list was updated
-* since last call, otherwise: exit
+* Check if mtdparts/mtdids changed, if the MTD dev list was updated
+* or if our previous attempt to delete existing partititions failed.
+* In any of these cases we want to update the partitions, otherwise,
+* everything is up-to-date and we can return 0 directly.
*/
if ((!mtdparts && !old_mtdparts && !mtdids && !old_mtdids) ||
(mtdparts && old_mtdparts && mtdids && old_mtdids &&
-!mtd_dev_list_updated() &&
+!mtd_dev_list_updated() && !mtd_del_all_parts_failed &&
!strcmp(mtdparts, old_mtdparts) &&
!strcmp(mtdids, old_mtdids)))
return 0;
@@ -178,32 +227,12 @@ int mtd_probe_devices(void)
old_mtdparts = strdup(mtdparts);
old_mtdids = strdup(mtdids);
- /* If at least one partition is still in use, do not delete anything */
- mtd_for_each_device(mtd) {
- if (mtd->usecount) {
- printf("Partition \"%s\" already in use, aborting\n",
- mtd->name);
- return -EACCES;
- }
- }
-
/*
-* Everything looks clear, remove all partitions. It is not safe
[U-Boot] [PATCH v4 11/11] mtd: sf: Make sf_mtd.c more robust
SPI flash based MTD devs can be registered/unregistered at any time
through the sf probe command or the spi_flash_free() function.
This commit does not try to fix the root cause as it would probably
require rewriting most of the code and have an mtd_info object
instance per spi_flash object (not to mention that the the spi-flash
layer is likely to be replaced by a spi-nor layer ported from Linux).
Instead, we try to be as safe as can be by checking the code returned
by del_mtd_device() and complain loudly when there's nothing we can
do about the deregistration failure. When that happens we also reset
sf_mtd_info.priv to NULL, and check for NULL pointer in the mtd hooks
so that -ENODEV is returned instead of hitting a NULL pointer
dereference exception when the MTD instance is later accessed by a user.
Signed-off-by: Boris Brezillon
Tested-by: Heiko Schocher
---
Changes in v4:
- Add T-b tags
Changes in v3:
- New patch
---
drivers/mtd/spi/sf_mtd.c | 39 ---
1 file changed, 36 insertions(+), 3 deletions(-)
diff --git a/drivers/mtd/spi/sf_mtd.c b/drivers/mtd/spi/sf_mtd.c
index aabbc3589435..68c36002bee2 100644
--- a/drivers/mtd/spi/sf_mtd.c
+++ b/drivers/mtd/spi/sf_mtd.c
@@ -18,6 +18,9 @@ static int spi_flash_mtd_erase(struct mtd_info *mtd, struct
erase_info *instr)
struct spi_flash *flash = mtd->priv;
int err;
+ if (!flash)
+ return -ENODEV;
+
instr->state = MTD_ERASING;
err = spi_flash_erase(flash, instr->addr, instr->len);
@@ -39,6 +42,9 @@ static int spi_flash_mtd_read(struct mtd_info *mtd, loff_t
from, size_t len,
struct spi_flash *flash = mtd->priv;
int err;
+ if (!flash)
+ return -ENODEV;
+
err = spi_flash_read(flash, from, len, buf);
if (!err)
*retlen = len;
@@ -52,6 +58,9 @@ static int spi_flash_mtd_write(struct mtd_info *mtd, loff_t
to, size_t len,
struct spi_flash *flash = mtd->priv;
int err;
+ if (!flash)
+ return -ENODEV;
+
err = spi_flash_write(flash, to, len, buf);
if (!err)
*retlen = len;
@@ -76,8 +85,13 @@ int spi_flash_mtd_register(struct spi_flash *flash)
{
int ret;
- if (sf_mtd_registered)
- del_mtd_device(_mtd_info);
+ if (sf_mtd_registered) {
+ ret = del_mtd_device(_mtd_info);
+ if (ret)
+ return ret;
+
+ sf_mtd_registered = false;
+ }
sf_mtd_registered = false;
memset(_mtd_info, 0, sizeof(sf_mtd_info));
@@ -110,5 +124,24 @@ int spi_flash_mtd_register(struct spi_flash *flash)
void spi_flash_mtd_unregister(void)
{
- del_mtd_device(_mtd_info);
+ int ret;
+
+ if (!sf_mtd_registered)
+ return;
+
+ ret = del_mtd_device(_mtd_info);
+ if (!ret) {
+ sf_mtd_registered = false;
+ return;
+ }
+
+ /*
+* Setting mtd->priv to NULL is the best we can do. Thanks to that,
+* the MTD layer can still call mtd hooks without risking a
+* use-after-free bug. Still, things should be fixed to prevent the
+* spi_flash object from being destroyed when del_mtd_device() fails.
+*/
+ sf_mtd_info.priv = NULL;
+ printf("Failed to unregister MTD %s and the spi_flash object is going
away: you're in deep trouble!",
+ sf_mtd_info.name);
}
--
2.17.1
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v4 10/11] mtd: sf: Unregister the MTD device prior to removing the spi_flash obj
The DM implementation of spi_flash_free() does not unregister the MTD
device before removing the spi dev object. This leads to a use-after-free
bug when the MTD device is later accessed by a MTD user (observed when
attaching the device to UBI after env_sf_load() has called
spi_flash_free()).
Implement ->remove() and call spi_flash_mtd_unregister() from there.
Fixes: 9fe6d8716e09 ("mtd, spi: Add MTD layer driver")
Signed-off-by: Boris Brezillon
Tested-by: Heiko Schocher
Reviewed-by: Jagan Teki
---
Changes in v4:
- Add T-b/R-b tags
Changes in v3:
- New patch
---
drivers/mtd/spi/sf_probe.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
index 5a2e932de8f8..00f8558e7019 100644
--- a/drivers/mtd/spi/sf_probe.c
+++ b/drivers/mtd/spi/sf_probe.c
@@ -144,6 +144,14 @@ static int spi_flash_std_probe(struct udevice *dev)
return spi_flash_probe_slave(flash);
}
+static int spi_flash_std_remove(struct udevice *dev)
+{
+#ifdef CONFIG_SPI_FLASH_MTD
+ spi_flash_mtd_unregister();
+#endif
+ return 0;
+}
+
static const struct dm_spi_flash_ops spi_flash_std_ops = {
.read = spi_flash_std_read,
.write = spi_flash_std_write,
@@ -161,6 +169,7 @@ U_BOOT_DRIVER(spi_flash_std) = {
.id = UCLASS_SPI_FLASH,
.of_match = spi_flash_std_ids,
.probe = spi_flash_std_probe,
+ .remove = spi_flash_std_remove,
.priv_auto_alloc_size = sizeof(struct spi_flash),
.ops= _flash_std_ops,
};
--
2.17.1
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v4 04/11] mtd: sf: Make sure we don't register the same device twice
spi_flash_mtd_register() can be called several times and each time it
will register the same mtd_info instance like if it was a new one.
The MTD ID allocation gets crazy when that happens, so let's track the
status of the sf_mtd_info object to avoid that.
Fixes: 9fe6d8716e09 ("mtd, spi: Add MTD layer driver")
Signed-off-by: Boris Brezillon
Tested-by: Heiko Schocher
Reviewed-by: Jagan Teki
---
Changes in v4:
- Add T-b/R-b tags
Changes in v3:
- None
Changes in v2
- None
---
drivers/mtd/spi/sf_mtd.c | 13 -
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/spi/sf_mtd.c b/drivers/mtd/spi/sf_mtd.c
index 58d7e4439903..aabbc3589435 100644
--- a/drivers/mtd/spi/sf_mtd.c
+++ b/drivers/mtd/spi/sf_mtd.c
@@ -10,6 +10,7 @@
#include
static struct mtd_info sf_mtd_info;
+static bool sf_mtd_registered;
static char sf_mtd_name[8];
static int spi_flash_mtd_erase(struct mtd_info *mtd, struct erase_info *instr)
@@ -73,6 +74,12 @@ static int spi_flash_mtd_number(void)
int spi_flash_mtd_register(struct spi_flash *flash)
{
+ int ret;
+
+ if (sf_mtd_registered)
+ del_mtd_device(_mtd_info);
+
+ sf_mtd_registered = false;
memset(_mtd_info, 0, sizeof(sf_mtd_info));
sprintf(sf_mtd_name, "nor%d", spi_flash_mtd_number());
@@ -94,7 +101,11 @@ int spi_flash_mtd_register(struct spi_flash *flash)
sf_mtd_info.numeraseregions = 0;
sf_mtd_info.erasesize = flash->sector_size;
- return add_mtd_device(_mtd_info);
+ ret = add_mtd_device(_mtd_info);
+ if (!ret)
+ sf_mtd_registered = true;
+
+ return ret;
}
void spi_flash_mtd_unregister(void)
--
2.17.1
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v4 06/11] mtd: Be more strict on the "mtdparts=" prefix check
strstr() does not guarantee that the string we're searching for is
placed at the beginning. Use strncmp() instead.
Fixes: 5db66b3aee6f ("cmd: mtd: add 'mtd' command")
Signed-off-by: Boris Brezillon
Tested-by: Heiko Schocher
---
Changes in v4:
- Add T-b tag
Changes in v3:
- None
Changes in v2:
- New patch
---
drivers/mtd/mtd_uboot.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mtd/mtd_uboot.c b/drivers/mtd/mtd_uboot.c
index c4434d70520d..d551aee20203 100644
--- a/drivers/mtd/mtd_uboot.c
+++ b/drivers/mtd/mtd_uboot.c
@@ -215,7 +215,7 @@ int mtd_probe_devices(void)
return 0;
/* Start the parsing by ignoring the extra 'mtdparts=' prefix, if any */
- if (strstr(mtdparts, "mtdparts="))
+ if (!strncmp(mtdparts, "mtdparts=", sizeof("mtdparts=") - 1))
mtdparts += 9;
/* For each MTD device in mtdparts */
--
2.17.1
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v4 05/11] mtd: Use get_mtdids() instead of env_get("mtdids") in mtd_search_alternate_name()
The environment is not guaranteed to contain a valid mtdids variable
when called from mtd_search_alternate_name(). Call get_mtdids() instead
of env_get("mtdids").
Fixes: ff4afa8a981e ("mtd: uboot: search for an equivalent MTD name with the
mtdids")
Signed-off-by: Boris Brezillon
Reviewed-by: Miquel Raynal
Tested-by: Heiko Schocher
---
Changes in v4:
- Add T-b tag
Changes in v3:
- None
Changes in v2:
- Add Miquel's R-b
- Move board_mtdparts_default() prototype def to fix a build failure
---
drivers/mtd/mtd_uboot.c | 49 -
1 file changed, 24 insertions(+), 25 deletions(-)
diff --git a/drivers/mtd/mtd_uboot.c b/drivers/mtd/mtd_uboot.c
index 6a3e64395de2..c4434d70520d 100644
--- a/drivers/mtd/mtd_uboot.c
+++ b/drivers/mtd/mtd_uboot.c
@@ -13,6 +13,29 @@
#define MTD_NAME_MAX_LEN 20
+void board_mtdparts_default(const char **mtdids, const char **mtdparts);
+
+static const char *get_mtdids(void)
+{
+ __maybe_unused const char *mtdparts = NULL;
+ const char *mtdids = env_get("mtdids");
+
+ if (mtdids)
+ return mtdids;
+
+#if defined(CONFIG_SYS_MTDPARTS_RUNTIME)
+ board_mtdparts_default(, );
+#elif defined(MTDIDS_DEFAULT)
+ mtdids = MTDIDS_DEFAULT;
+#elif defined(CONFIG_MTDIDS_DEFAULT)
+ mtdids = CONFIG_MTDIDS_DEFAULT;
+#endif
+
+ if (mtdids)
+ env_set("mtdids", mtdids);
+
+ return mtdids;
+}
/**
* mtd_search_alternate_name - Search an alternate name for @mtdname thanks to
@@ -34,7 +57,7 @@ int mtd_search_alternate_name(const char *mtdname, char
*altname,
const char *mtdids, *equal, *comma, *dev_id, *mtd_id;
int dev_id_len, mtd_id_len;
- mtdids = env_get("mtdids");
+ mtdids = get_mtdids();
if (!mtdids)
return -EINVAL;
@@ -92,30 +115,6 @@ static void mtd_probe_uclass_mtd_devs(void) { }
#endif
#if defined(CONFIG_MTD_PARTITIONS)
-extern void board_mtdparts_default(const char **mtdids,
- const char **mtdparts);
-
-static const char *get_mtdids(void)
-{
- __maybe_unused const char *mtdparts = NULL;
- const char *mtdids = env_get("mtdids");
-
- if (mtdids)
- return mtdids;
-
-#if defined(CONFIG_SYS_MTDPARTS_RUNTIME)
- board_mtdparts_default(, );
-#elif defined(MTDIDS_DEFAULT)
- mtdids = MTDIDS_DEFAULT;
-#elif defined(CONFIG_MTDIDS_DEFAULT)
- mtdids = CONFIG_MTDIDS_DEFAULT;
-#endif
-
- if (mtdids)
- env_set("mtdids", mtdids);
-
- return mtdids;
-}
#define MTDPARTS_MAXLEN 512
--
2.17.1
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v4 07/11] mtd: Make sure the name passed in mtdparts fits in mtd_name[]
The local mtd_name[] variable is limited in size. Return an error if
the name passed in mtdparts does not fit in this local var.
Fixes: 5db66b3aee6f ("cmd: mtd: add 'mtd' command")
Signed-off-by: Boris Brezillon
Tested-by: Heiko Schocher
---
Changes in v4:
- Add T-b tag
Changes in v3:
- None
Changes in v2:
- New patch
---
drivers/mtd/mtd_uboot.c | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/mtd/mtd_uboot.c b/drivers/mtd/mtd_uboot.c
index d551aee20203..0eda36278309 100644
--- a/drivers/mtd/mtd_uboot.c
+++ b/drivers/mtd/mtd_uboot.c
@@ -222,8 +222,8 @@ int mtd_probe_devices(void)
while (mtdparts[0] != '\0') {
char mtd_name[MTD_NAME_MAX_LEN], *colon;
struct mtd_partition *parts;
- int mtd_name_len, nparts;
- int ret;
+ unsigned int mtd_name_len;
+ int nparts, ret;
colon = strchr(mtdparts, ':');
if (!colon) {
@@ -231,7 +231,12 @@ int mtd_probe_devices(void)
return -EINVAL;
}
- mtd_name_len = colon - mtdparts;
+ mtd_name_len = (unsigned int)(colon - mtdparts);
+ if (mtd_name_len + 1 > sizeof(mtd_name)) {
+ printf("MTD name too long: %s\n", mtdparts);
+ return -EINVAL;
+ }
+
strncpy(mtd_name, mtdparts, mtd_name_len);
mtd_name[mtd_name_len] = '\0';
/* Move the pointer forward (including the ':') */
--
2.17.1
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v4 01/11] mtd: Add a function to report when the MTD dev list has been updated
We need to parse mtdparts/mtids again everytime a device has been
added/removed from the MTD list, but there's currently no way to know
when such an update has been done.
Add an ->updated field to the idr struct that we set to true every time
a device is added/removed and expose a function returning the value
of this field and resetting it to false.
Signed-off-by: Boris Brezillon
Tested-by: Heiko Schocher
---
Changes in v4:
- Add T-b tag
Changes in v2:
- None
Changes in v2:
- None
---
drivers/mtd/mtdcore.c | 16 +++-
include/linux/mtd/mtd.h | 1 +
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index fb6c779abbfe..7a15ded8c883 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -87,14 +87,17 @@ struct idr_layer {
struct idr {
struct idr_layer id[MAX_IDR_ID];
+ bool updated;
};
#define DEFINE_IDR(name) struct idr name;
void idr_remove(struct idr *idp, int id)
{
- if (idp->id[id].used)
+ if (idp->id[id].used) {
idp->id[id].used = 0;
+ idp->updated = true;
+ }
return;
}
@@ -134,6 +137,7 @@ int idr_alloc(struct idr *idp, void *ptr, int start, int
end, gfp_t gfp_mask)
if (idl->used == 0) {
idl->used = 1;
idl->ptr = ptr;
+ idp->updated = true;
return i;
}
i++;
@@ -155,6 +159,16 @@ struct mtd_info *__mtd_next_device(int i)
}
EXPORT_SYMBOL_GPL(__mtd_next_device);
+bool mtd_dev_list_updated(void)
+{
+ if (mtd_idr.updated) {
+ mtd_idr.updated = false;
+ return true;
+ }
+
+ return false;
+}
+
#ifndef __UBOOT__
static LIST_HEAD(mtd_notifiers);
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index 68e591532492..d20ebd820289 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -581,6 +581,7 @@ int mtd_arg_off_size(int argc, char *const argv[], int
*idx, loff_t *off,
void mtd_get_len_incl_bad(struct mtd_info *mtd, uint64_t offset,
const uint64_t length, uint64_t *len_incl_bad,
int *truncated);
+bool mtd_dev_list_updated(void);
/* drivers/mtd/mtd_uboot.c */
int mtd_search_alternate_name(const char *mtdname, char *altname,
--
2.17.1
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v4 03/11] mtd: Delete partitions attached to the device when a device is deleted
If we don't do that, partitions might still be exposed while the
underlying device is gone.
Fixes: 2a74930da57f ("mtd: mtdpart: implement proper partition handling")
Signed-off-by: Boris Brezillon
Tested-by: Heiko Schocher
---
Changes in v4:
- Test the del_mtd_partitions() ret code and propagate errors
- Add a debug() message when del_mtd_partitions() returns an error
- Add Heiko's T-b
Changes in v3:
- None
Changes in v2:
- Fix build failures when CONFIG_MTD_PARTITIONS is not enabled
---
drivers/mtd/mtdcore.c | 7 +++
include/linux/mtd/mtd.h | 15 +++
2 files changed, 22 insertions(+)
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 7a15ded8c883..cb7ca38d0744 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -528,6 +528,13 @@ int del_mtd_device(struct mtd_info *mtd)
struct mtd_notifier *not;
#endif
+ ret = del_mtd_partitions(mtd);
+ if (ret) {
+ debug("Failed to delete MTD partitions attached to %s (err
%d)\n",
+ mtd->name, ret);
+ return ret;
+ }
+
mutex_lock(_table_mutex);
if (idr_find(_idr, mtd->index) != mtd) {
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index d20ebd820289..4d0096d9f1d8 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -562,8 +562,23 @@ unsigned mtd_mmap_capabilities(struct mtd_info *mtd);
/* drivers/mtd/mtdcore.h */
int add_mtd_device(struct mtd_info *mtd);
int del_mtd_device(struct mtd_info *mtd);
+
+#ifdef CONFIG_MTD_PARTITIONS
int add_mtd_partitions(struct mtd_info *, const struct mtd_partition *, int);
int del_mtd_partitions(struct mtd_info *);
+#else
+static inline int add_mtd_partitions(struct mtd_info *mtd,
+const struct mtd_partition *parts,
+int nparts)
+{
+ return 0;
+}
+
+static inline int del_mtd_partitions(struct mtd_info *mtd)
+{
+ return 0;
+}
+#endif
struct mtd_info *__mtd_next_device(int i);
#define mtd_for_each_device(mtd) \
--
2.17.1
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v4 02/11] mtd: Parse mtdparts/mtdids again when the MTD list has been updated
Updates to the MTD device list should trigger a new parsing of the
mtdids/mtdparts vars even if those vars haven't changed.
Fixes: 5db66b3aee6f ("cmd: mtd: add 'mtd' command")
Signed-off-by: Boris Brezillon
Tested-by: Heiko Schocher
---
Changes in v4:
- Add T-b tag
Changes in v3:
- None
Changes in v2:
- None
---
drivers/mtd/mtd_uboot.c | 18 +-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/mtd_uboot.c b/drivers/mtd/mtd_uboot.c
index 5ca560c96879..6a3e64395de2 100644
--- a/drivers/mtd/mtd_uboot.c
+++ b/drivers/mtd/mtd_uboot.c
@@ -161,9 +161,13 @@ int mtd_probe_devices(void)
mtd_probe_uclass_mtd_devs();
- /* Check if mtdparts/mtdids changed since last call, otherwise: exit */
+ /*
+* Check if mtdparts/mtdids changed or if the MTD dev list was updated
+* since last call, otherwise: exit
+*/
if ((!mtdparts && !old_mtdparts && !mtdids && !old_mtdids) ||
(mtdparts && old_mtdparts && mtdids && old_mtdids &&
+!mtd_dev_list_updated() &&
!strcmp(mtdparts, old_mtdparts) &&
!strcmp(mtdids, old_mtdids)))
return 0;
@@ -201,6 +205,12 @@ int mtd_probe_devices(void)
}
}
+ /*
+* Call mtd_dev_list_updated() to clear updates generated by our own
+* parts removal loop.
+*/
+ mtd_dev_list_updated();
+
/* If either mtdparts or mtdids is empty, then exit */
if (!mtdparts || !mtdids)
return 0;
@@ -281,6 +291,12 @@ int mtd_probe_devices(void)
put_mtd_device(mtd);
}
+ /*
+* Call mtd_dev_list_updated() to clear updates generated by our own
+* parts registration loop.
+*/
+ mtd_dev_list_updated();
+
return 0;
}
#else
--
2.17.1
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v4 00/11] mtd/sf: Various fixes
Hello,
This is the 4th version of the mtd / sf fixes patchset. This v4 just
adds a new check in del_mtd_device() (and a debug() when
del_mtd_partitions() fails).
Regards,
Boris
P.S.: travis-ci results =>
https://travis-ci.org/bbrezillon/u-boot/builds/461943011
Boris Brezillon (11):
mtd: Add a function to report when the MTD dev list has been updated
mtd: Parse mtdparts/mtdids again when the MTD list has been updated
mtd: Delete partitions attached to the device when a device is deleted
mtd: sf: Make sure we don't register the same device twice
mtd: Use get_mtdids() instead of env_get("mtdids") in
mtd_search_alternate_name()
mtd: Be more strict on the "mtdparts=" prefix check
mtd: Make sure the name passed in mtdparts fits in mtd_name[]
mtd: Make sure we don't parse MTD partitions belonging to another dev
mtd: Don't stop MTD partition creation when it fails on one device
mtd: sf: Unregister the MTD device prior to removing the spi_flash obj
mtd: sf: Make sf_mtd.c more robust
drivers/mtd/mtd_uboot.c| 185 +
drivers/mtd/mtdcore.c | 23 -
drivers/mtd/mtdpart.c | 12 +++
drivers/mtd/spi/sf_mtd.c | 48 +-
drivers/mtd/spi/sf_probe.c | 9 ++
include/linux/mtd/mtd.h| 18
6 files changed, 233 insertions(+), 62 deletions(-)
--
2.17.1
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [RFC PATCH 1/6] mtd: spi: Port SPI NOR framework from Linux
On Thu, 29 Nov 2018 22:56:04 +0530
Vignesh R wrote:
>
> >> +const struct flash_info spi_nor_ids[] = {
> >> +#ifdef CONFIG_SPI_FLASH_ATMEL /* ATMEL */
> >> + /* Atmel -- some are (confusingly) marketed as "DataFlash" */
> >> + { "at26df321", INFO(0x1f4700, 0, 64 * 1024, 64, SECT_4K) },
> >> + { "at25df321a", INFO(0x1f4701, 0, 64 * 1024, 64, SECT_4K) },
> >> +
> >> + { "at45db011d", INFO(0x1f2200, 0, 64 * 1024, 4, SECT_4K) },
> >> + { "at45db021d", INFO(0x1f2300, 0, 64 * 1024, 8, SECT_4K) },
> >> + { "at45db041d", INFO(0x1f2400, 0, 64 * 1024, 8, SECT_4K) },
> >> + { "at45db081d", INFO(0x1f2500, 0, 64 * 1024, 16, SECT_4K) },
> >> + { "at45db161d", INFO(0x1f2600, 0, 64 * 1024, 32, SECT_4K) },
> >> + { "at45db321d", INFO(0x1f2700, 0, 64 * 1024, 64, SECT_4K) },
> >> + { "at45db641d", INFO(0x1f2800, 0, 64 * 1024, 128, SECT_4K) },
> >> + { "at26df081a", INFO(0x1f4501, 0, 64 * 1024, 16, SECT_4K) },
> >> +#endif
> >
> > If you're really short in space (for the SPL build), you might want to
> > consider fined-grained selection of the chips you want to support. One
> > more reason to do that is that board manufacturers usually source SPI
> > NOR parts from different vendors for the same design. With a
> > per-manufacturer selection logic, you'll have to enable several of them
> > to have an SPL that works on all variants.
> >
> > I didn't try, but you might be able to place each NOR chip in its own
> > section and decide which one to keep at link time (will require some
> > macros to define flash_info entries + a linker script to decide which
> > sections you want to discard/keep at link time).
> >
>
> IIUC, you are proposing per board linker script that will select/choose
> which flash parts would to be supported by that board's SPL?
Not exactly a per-board linker script. More something that would be
automatically generated based on Kconfig option where you'd list all the
NORs you want to support. Of course, the default would be to have
everything enabled, but that allow people who have hard size
constraint to shrink the spi-nor obj a bit more.
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [RFC PATCH 1/6] mtd: spi: Port SPI NOR framework from Linux
On Wed, 28 Nov 2018 22:56:02 +0530
Vignesh R wrote:
> +#if defined(CONFIG_DM_SPI) && defined(CONFIG_SPI_MEM)
> +#define spi_nor_mem_exec_op spi_mem_exec_op
> +#else
> +/*
> + * This function is to support transition to DM_SPI. Will be removed
> + * once all boards are converted to DM_SPI
> + */
> +
> +static int spi_nor_mem_exec_op(struct spi_slave *slave,
> +const struct spi_mem_op *op)
> +{
> + unsigned int pos = 0;
> + const u8 *tx_buf = NULL;
> + u8 *rx_buf = NULL;
> + u8 *op_buf;
> + int op_len;
> + u32 flag;
> + int ret;
> + int i;
> +
> + if (op->data.nbytes) {
> + if (op->data.dir == SPI_MEM_DATA_IN)
> + rx_buf = op->data.buf.in;
> + else
> + tx_buf = op->data.buf.out;
> + }
> +
> + op_len = sizeof(op->cmd.opcode) + op->addr.nbytes + op->dummy.nbytes;
> + op_buf = calloc(1, op_len);
> +
> + ret = spi_claim_bus(slave);
> + if (ret < 0)
> + return ret;
> +
> + op_buf[pos++] = op->cmd.opcode;
> +
> + if (op->addr.nbytes) {
> + for (i = 0; i < op->addr.nbytes; i++)
> + op_buf[pos + i] = op->addr.val >>
> + (8 * (op->addr.nbytes - i - 1));
> +
> + pos += op->addr.nbytes;
> + }
> +
> + if (op->dummy.nbytes)
> + memset(op_buf + pos, 0xff, op->dummy.nbytes);
> +
> + /* 1st transfer: opcode + address + dummy cycles */
> + flag = SPI_XFER_BEGIN;
> + /* Make sure to set END bit if no tx or rx data messages follow */
> + if (!tx_buf && !rx_buf)
> + flag |= SPI_XFER_END;
> +
> + ret = spi_xfer(slave, op_len * 8, op_buf, NULL, flag);
> + if (ret)
> + return ret;
> +
> + /* 2nd transfer: rx or tx data path */
> + if (tx_buf || rx_buf) {
> + ret = spi_xfer(slave, op->data.nbytes * 8, tx_buf,
> +rx_buf, SPI_XFER_END);
> + if (ret)
> + return ret;
> + }
> +
> + spi_release_bus(slave);
> +
> + for (i = 0; i < pos; i++)
> + debug("%02x ", op_buf[i]);
> + debug("| [%dB %s] ",
> + tx_buf || rx_buf ? op->data.nbytes : 0,
> + tx_buf || rx_buf ? (tx_buf ? "out" : "in") : "-");
> + for (i = 0; i < op->data.nbytes; i++)
> + debug("%02x ", tx_buf ? tx_buf[i] : rx_buf[i]);
> + debug("[ret %d]\n", ret);
> +
> + free(op_buf);
> +
> + if (ret < 0)
> + return ret;
> +
> + return 0;
> +}
> +#endif
I'd recommend putting this alternate spi_mem_exec_op() implementation
in drivers/spi/. Either you create a drivers/spi/spi-mem-no-dm.c file
or you add #ifdef CONFIG_DM_SPI #else #endif sections directly in
drivers/spi/spi-mem.c.
> +const struct flash_info spi_nor_ids[] = {
> +#ifdef CONFIG_SPI_FLASH_ATMEL/* ATMEL */
> + /* Atmel -- some are (confusingly) marketed as "DataFlash" */
> + { "at26df321", INFO(0x1f4700, 0, 64 * 1024, 64, SECT_4K) },
> + { "at25df321a", INFO(0x1f4701, 0, 64 * 1024, 64, SECT_4K) },
> +
> + { "at45db011d", INFO(0x1f2200, 0, 64 * 1024, 4, SECT_4K) },
> + { "at45db021d", INFO(0x1f2300, 0, 64 * 1024, 8, SECT_4K) },
> + { "at45db041d", INFO(0x1f2400, 0, 64 * 1024, 8, SECT_4K) },
> + { "at45db081d", INFO(0x1f2500, 0, 64 * 1024, 16, SECT_4K) },
> + { "at45db161d", INFO(0x1f2600, 0, 64 * 1024, 32, SECT_4K) },
> + { "at45db321d", INFO(0x1f2700, 0, 64 * 1024, 64, SECT_4K) },
> + { "at45db641d", INFO(0x1f2800, 0, 64 * 1024, 128, SECT_4K) },
> + { "at26df081a", INFO(0x1f4501, 0, 64 * 1024, 16, SECT_4K) },
> +#endif
If you're really short in space (for the SPL build), you might want to
consider fined-grained selection of the chips you want to support. One
more reason to do that is that board manufacturers usually source SPI
NOR parts from different vendors for the same design. With a
per-manufacturer selection logic, you'll have to enable several of them
to have an SPL that works on all variants.
I didn't try, but you might be able to place each NOR chip in its own
section and decide which one to keep at link time (will require some
macros to define flash_info entries + a linker script to decide which
sections you want to discard/keep at link time).
Note that I'd be okay to convert Linux flash_info table to the new
macro usage if you choose to do that.
Just a quick example:
#define SPI_FLASH_INFO(_name, ...)
ll_entry_declare(struct flash_info, _name, spi_nor_ids) = {
.name = #_name,
__VAR_ARGS__
};
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 0/6] cmd: Simplify support for sub-commands
On Thu, 29 Nov 2018 00:39:15 +0100 Boris Brezillon wrote: > Hello, > > Here is the 2nd version of the sub-cmd patchset. This version > simplifies the sub-cmd declaration syntax and allows per > sub-cmd maxargs and repeatable check. > > I also added a patch showing how simple it is to convert an existing > command to this infrastructure. I converting a bunch of other cmds [1] > but I keep that for later, as I don't want to scare reviewers with a > 20+ patch series. Noticed a few typos in my commit messages. I'll fix them up in a v3 unless the person in charge of the common/cmd dirs want to fix them when applying the patches. Let me know if you want me to send the patches converting other cmds (see [1)] to this infrastructure. > > Regards, > > Boris > > [1]https://github.com/bbrezillon/u-boot/commits/sub-cmds > > Changes since v1: > - Drop a few params in the subcmd macro def > - Get repeatable and maxargs info from the subcmd instead of the main > one > - Convert the adc command > > Boris Brezillon (6): > common: command: Fix command auto-completion > common: command: Expose a generic helper to auto-complete sub commands > common: command: Rework the 'cmd is repeatable' logic > command: commands: Add macros to declare commands with subcmds > cmd: mtd: Use the subcmd infrastructure to declare mtd sub-commands > cmd: adc: Use the sub-command infrastructure > > cmd/adc.c | 31 +-- > cmd/dtimg.c | 2 +- > cmd/help.c| 2 +- > cmd/mmc.c | 4 +- > cmd/mtd.c | 476 +++--- > common/command.c | 68 ++- > include/command.h | 133 - > 7 files changed, 476 insertions(+), 240 deletions(-) > ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 4/6] command: commands: Add macros to declare commands with subcmds
On Thu, 29 Nov 2018 00:39:19 +0100
Boris Brezillon wrote:
> Most cmd/xxx.c source files expose several commands through a single
> entry point. Some of them are doing the sub-command parsing manually in
> their do_() function, others are declaring a table of sub-commands
> and then use find_cmd_tbl() to delegate the request to the sub command
> handler.
>
> In both case, the amount of code to do that is not negligible and
^cases
> repetitive, not to mention that almost no commands are implementing
> a auto-completion hook, which means most u-boot lack auto-completion.
^the^commands
>
> Provide several macros to easily define sub-commands and commands
> exposing such sub-commands.
"
Provide several macros to easily define commands exposing sub-commands.
"
>
> Signed-off-by: Boris Brezillon
> ---
> Changes in v2:
> - Remove _maxargs argument
> ---
> include/command.h | 78 +++
> 1 file changed, 78 insertions(+)
>
> diff --git a/include/command.h b/include/command.h
> index bb93f022c514..461b17447c0d 100644
> --- a/include/command.h
> +++ b/include/command.h
> @@ -209,6 +209,70 @@ int board_run_command(const char *cmdline);
> # define _CMD_HELP(x)
> #endif
>
> +#ifdef CONFIG_NEEDS_MANUAL_RELOC
> +#define U_BOOT_SUBCMDS_RELOC(_cmdname)
> \
> + static void _cmdname##_subcmds_reloc(void) \
> + { \
> + static int relocated; \
> + \
> + if (relocated) \
> + return; \
> + \
> + fixup_cmdtable(_cmdname##_subcmds, \
> +ARRAY_SIZE(_cmdname##_subcmds)); \
> + relocated = 1; \
> + }
> +#else
> +#define U_BOOT_SUBCMDS_RELOC(_cmdname)
> \
> + static void _cmdname##_subcmds_reloc(void) { }
> +#endif
> +
> +#define U_BOOT_SUBCMDS_DO_CMD(_cmdname)
> \
> + static int do_##_cmdname(cmd_tbl_t *cmdtp, int flag, int argc, \
> + char * const argv[], int *repeatable) \
> + { \
> + cmd_tbl_t *subcmd; \
> + \
> + _cmdname##_subcmds_reloc(); \
> + \
> + /* We need at least the cmd and subcmd names. */\
> + if (argc < 2 || argc > CONFIG_SYS_MAXARGS) \
> + return CMD_RET_USAGE; \
> + \
> + subcmd = find_cmd_tbl(argv[1], _cmdname##_subcmds, \
> + ARRAY_SIZE(_cmdname##_subcmds)); \
> + if (!subcmd || argc - 1 > subcmd->maxargs) \
> + return CMD_RET_USAGE; \
> + \
> + if (flag == CMD_FLAG_REPEAT && \
> + !cmd_is_repeatable(subcmd)) \
> + return CMD_RET_SUCCESS; \
> + \
> + return subcmd->cmd_rep(subcmd, flag, argc - 1, \
> +argv + 1, repeatable); \
> + }
> +
> +#ifdef CONFIG_AUTO_COMPLETE
> +#define U_BOOT_SUBCMDS_COMPLETE(_cmdname)\
> + static int complete_##_cmdname(int argc, char * const argv[], \
> +char last_char, int maxv,\
> +char *cmdv[])\
> + { \
> + return complete_subcmdv(_cmdname##_subcmds, \
> + ARRAY_SIZE(_cmdname##_subcmds), \
> + argc - 1, argv + 1, last_char, \
> +
Re: [U-Boot] [PATCH v2 2/6] common: command: Expose a generic helper to auto-complete sub commands
On Thu, 29 Nov 2018 00:39:17 +0100
Boris Brezillon wrote:
> Some commands have a table of sub-commands. With a minor adjustments,
^ s/a//
> complete_cmdv() is able to provide auto-completion for sub-commands
> (it's just about passing the table of commands instead of taking the
> global one).
> We rename this function into complete_subcmd() and implement
> complete_cmdv() as a wrapper around complete_subcmdv().
>
> Signed-off-by: Boris Brezillon
> ---
> common/command.c | 20
> include/command.h | 3 +++
> 2 files changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/common/command.c b/common/command.c
> index 435824356b50..e13cb47ac18b 100644
> --- a/common/command.c
> +++ b/common/command.c
> @@ -161,11 +161,11 @@ int var_complete(int argc, char * const argv[], char
> last_char, int maxv, char *
>
>
> /*/
>
> -static int complete_cmdv(int argc, char * const argv[], char last_char, int
> maxv, char *cmdv[])
> +int complete_subcmdv(cmd_tbl_t *cmdtp, int count, int argc,
> + char * const argv[], char last_char,
> + int maxv, char *cmdv[])
> {
> #ifdef CONFIG_CMDLINE
> - cmd_tbl_t *cmdtp = ll_entry_start(cmd_tbl_t, cmd);
> - const int count = ll_entry_count(cmd_tbl_t, cmd);
> const cmd_tbl_t *cmdend = cmdtp + count;
> const char *p;
> int len, clen;
> @@ -193,7 +193,7 @@ static int complete_cmdv(int argc, char * const argv[],
> char last_char, int maxv
>
> /* more than one arg or one but the start of the next */
> if (argc > 1 || last_char == '\0' || isblank(last_char)) {
> - cmdtp = find_cmd(argv[0]);
> + cmdtp = find_cmd_tbl(argv[0], cmdtp, count);
> if (cmdtp == NULL || cmdtp->complete == NULL) {
> cmdv[0] = NULL;
> return 0;
> @@ -238,6 +238,18 @@ static int complete_cmdv(int argc, char * const argv[],
> char last_char, int maxv
> #endif
> }
>
> +static int complete_cmdv(int argc, char * const argv[], char last_char,
> + int maxv, char *cmdv[])
> +{
> +#ifdef CONFIG_CMDLINE
> + return complete_subcmdv(ll_entry_start(cmd_tbl_t, cmd),
> + ll_entry_count(cmd_tbl_t, cmd), argc, argv,
> + last_char, maxv, cmdv);
> +#else
> + return 0;
> +#endif
> +}
> +
> static int make_argv(char *s, int argvsz, char *argv[])
> {
> int argc = 0;
> diff --git a/include/command.h b/include/command.h
> index 200c7a5e9f4e..89efcecfa926 100644
> --- a/include/command.h
> +++ b/include/command.h
> @@ -54,6 +54,9 @@ int _do_help (cmd_tbl_t *cmd_start, int cmd_items,
> cmd_tbl_t * cmdtp, int
> flag, int argc, char * const argv[]);
> cmd_tbl_t *find_cmd(const char *cmd);
> cmd_tbl_t *find_cmd_tbl (const char *cmd, cmd_tbl_t *table, int table_len);
> +int complete_subcmdv(cmd_tbl_t *cmdtp, int count, int argc,
> + char * const argv[], char last_char, int maxv,
> + char *cmdv[]);
>
> extern int cmd_usage(const cmd_tbl_t *cmdtp);
>
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 01/24] Makefile: move MTD-related lines in coherent Makefiles
On Thu, 29 Nov 2018 10:16:50 +0100 Miquel Raynal wrote: > Hi Boris, > > Boris Brezillon wrote on Thu, 29 Nov 2018 > 00:46:23 +0100: > > > On Thu, 29 Nov 2018 00:07:37 +0100 > > Miquel Raynal wrote: > > > > > > > --- a/drivers/Makefile > > > +++ b/drivers/Makefile > > > @@ -6,7 +6,7 @@ obj-$(CONFIG_$(SPL_TPL_)DRIVERS_MISC_SUPPORT) += misc/ > > > sysreset/ firmware/ > > > obj-$(CONFIG_$(SPL_TPL_)I2C_SUPPORT) += i2c/ > > > obj-$(CONFIG_$(SPL_TPL_)LED) += led/ > > > obj-$(CONFIG_$(SPL_TPL_)MMC_SUPPORT) += mmc/ > > > -obj-$(CONFIG_$(SPL_TPL_)NAND_SUPPORT) += mtd/nand/raw/ > > > +obj-$(CONFIG_$(SPL_TPL_)NAND_SUPPORT) += mtd/nand/ > > > > Can't we have > > += mtd/ > > > > instead? > > You are right the SPL Makefile lines needs to be improved as well. I > propose the following diff for that. This is much better. Thanks for addressing that. Boris ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 24/24] mtd: drop CONFIG_MTD_PARTITIONS
On Thu, 29 Nov 2018 00:08:00 +0100 Miquel Raynal wrote: > There is no point in compiling mtdparts.c only in certain > circumstances. Whether MTD is needed and it will be built, or MTD is > not needed and the file will be ignored. You're overlooking the image size constraint. Some might want to have MTD support without partitioning, and we shouldn't force them to pull this code in if they don't need it. ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 22/24] cmd: make all MTD commands depend on MTD
On Thu, 29 Nov 2018 00:07:58 +0100
Miquel Raynal wrote:
> Defconfigs have been fixed, now we can add proper dependencies in
> Kconfig.
>
> Signed-off-by: Miquel Raynal
> ---
> cmd/Kconfig | 8 +++-
> drivers/mtd/Kconfig | 1 +
> 2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index e2973b3c51..717e814c56 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -868,6 +868,7 @@ config CMD_MMC_SWRITE
>
> config CMD_MTD
> bool "mtd"
> + depends on MTD
> select MTD_PARTITIONS
> help
> MTD commands support.
> @@ -875,6 +876,7 @@ config CMD_MTD
> config CMD_NAND
> bool "nand"
> default y if NAND_SUNXI
> + depends on MTD_RAW_NAND
> help
> NAND support.
>
> @@ -915,6 +917,7 @@ config CMD_MMC_SPI
>
> config CMD_ONENAND
> bool "onenand - access to onenand device"
> + depends on MTD
> help
> OneNAND is a brand of NAND ('Not AND' gate) flash which provides
> various useful features. This command allows reading, writing,
> @@ -1014,9 +1017,11 @@ config CMD_SDRAM
>
> config CMD_SF
> bool "sf"
> + depends on MTD
The SF cmd completely bypasses the MTD framework, so this one is not
needed (yet).
> help
> SPI Flash support
>
> +if CMD_SF
> config CMD_SF_TEST
> bool "sf test - Allow testing of SPI flash"
> help
> @@ -1027,6 +1032,7 @@ config CMD_SF_TEST
> Mbps (Million Bits Per Second). This value should approximately
> equal the SPI bus speed for a single-bit-wide SPI bus, assuming
> everything is working properly.
> +endif
>
> config CMD_SPI
> bool "sspi"
> @@ -1733,7 +1739,7 @@ config CMD_JFFS2
>
> config CMD_MTDPARTS
> bool "MTD partition support"
> - select MTD_DEVICE if (CMD_NAND || NAND)
> + depends on MTD
> help
> MTD partitioning tool support.
> It is strongly encouraged to avoid using this command
> diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig
> index 345046c2a6..0832f5b411 100644
> --- a/drivers/mtd/Kconfig
> +++ b/drivers/mtd/Kconfig
> @@ -19,6 +19,7 @@ config DM_MTD
>
> config MTD_NOR_FLASH
> bool "Enable parallel NOR flash support"
> + depends on MTD
> help
> Enable support for parallel NOR flash.
>
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 18/24] mtd: rawnand: compile-in the NAND core
On Thu, 29 Nov 2018 00:07:54 +0100 Miquel Raynal wrote: > The NAND core should be selected by both SPI NAND and raw NAND > drivers. > > Signed-off-by: Miquel Raynal > --- > drivers/mtd/nand/raw/Kconfig | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig > index a1fd476b0d..5624e47d65 100644 > --- a/drivers/mtd/nand/raw/Kconfig > +++ b/drivers/mtd/nand/raw/Kconfig > @@ -1,6 +1,8 @@ > > menuconfig MTD_RAW_NAND > bool "Raw NAND Device Support" > + select MTD_NAND_CORE > + I'm pretty sure this is not needed (yet). Let's not compile things we don't need as some devices have storage constraints. ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 07/24] mtd: ensure MTD is compiled when there is a SPI NOR flash
On Thu, 29 Nov 2018 00:07:43 +0100 Miquel Raynal wrote: > MTD must be enabled when there is a SPI NOR flash. Not sure this is required, as the SF layer is mostly independent from the MTD layer, except for the sf_mtd.c portion. It's probably safer to only enable CONFIG_MTD when CONFIG_SPI_FLASH_MTD is enabled. > Also enable it when CONFIG_CMD_SF is selected to do not > break any build during later cleanup. > > Signed-off-by: Miquel Raynal ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 04/24] mtd: rename CONFIG_MTD_DEVICE -> CONFIG_MTD
On Thu, 29 Nov 2018 00:07:40 +0100 Miquel Raynal wrote: > Like in Linux, just use CONFIG_MTD to copile the MTD stack. ^ compile > > Signed-off-by: Miquel Raynal ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 01/24] Makefile: move MTD-related lines in coherent Makefiles
On Thu, 29 Nov 2018 00:07:37 +0100 Miquel Raynal wrote: > --- a/drivers/Makefile > +++ b/drivers/Makefile > @@ -6,7 +6,7 @@ obj-$(CONFIG_$(SPL_TPL_)DRIVERS_MISC_SUPPORT) += misc/ > sysreset/ firmware/ > obj-$(CONFIG_$(SPL_TPL_)I2C_SUPPORT) += i2c/ > obj-$(CONFIG_$(SPL_TPL_)LED) += led/ > obj-$(CONFIG_$(SPL_TPL_)MMC_SUPPORT) += mmc/ > -obj-$(CONFIG_$(SPL_TPL_)NAND_SUPPORT) += mtd/nand/raw/ > +obj-$(CONFIG_$(SPL_TPL_)NAND_SUPPORT) += mtd/nand/ Can't we have += mtd/ instead? ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v2 6/6] cmd: adc: Use the sub-command infrastructure
And you get sub-command auto-completion for free.
Signed-off-by: Boris Brezillon
---
cmd/adc.c | 31 ---
1 file changed, 4 insertions(+), 27 deletions(-)
diff --git a/cmd/adc.c b/cmd/adc.c
index c8857ed147e7..5f06f361b642 100644
--- a/cmd/adc.c
+++ b/cmd/adc.c
@@ -86,35 +86,12 @@ static int do_adc_single(cmd_tbl_t *cmdtp, int flag, int
argc,
return CMD_RET_SUCCESS;
}
-static cmd_tbl_t cmd_adc_sub[] = {
- U_BOOT_CMD_MKENT(list, 1, 1, do_adc_list, "", ""),
- U_BOOT_CMD_MKENT(info, 2, 1, do_adc_info, "", ""),
- U_BOOT_CMD_MKENT(single, 3, 1, do_adc_single, "", ""),
-};
-
-static int do_adc(cmd_tbl_t *cmdtp, int flag, int argc,
- char *const argv[])
-{
- cmd_tbl_t *c;
-
- if (argc < 2)
- return CMD_RET_USAGE;
-
- /* Strip off leading 'adc' command argument */
- argc--;
- argv++;
-
- c = find_cmd_tbl(argv[0], _adc_sub[0], ARRAY_SIZE(cmd_adc_sub));
-
- if (c)
- return c->cmd(cmdtp, flag, argc, argv);
- else
- return CMD_RET_USAGE;
-}
-
static char adc_help_text[] =
"list - list ADC devices\n"
"adc info - Get ADC device info\n"
"adc single - Get Single data of ADC device channel";
-U_BOOT_CMD(adc, 4, 1, do_adc, "ADC sub-system", adc_help_text);
+U_BOOT_CMD_WITH_SUBCMDS(adc, "ADC sub-system", adc_help_text,
+ U_BOOT_SUBCMD_MKENT(list, 1, 1, do_adc_list),
+ U_BOOT_SUBCMD_MKENT(info, 2, 1, do_adc_info),
+ U_BOOT_SUBCMD_MKENT(single, 3, 1, do_adc_single));
--
2.17.1
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v2 5/6] cmd: mtd: Use the subcmd infrastructure to declare mtd sub-commands
It's way simpler this way, and we also gain auto-completion support for
free (MTD name auto-completion has been added with do_mtd_name_complete())
Signed-off-by: Boris Brezillon
---
Changes in v2:
- Drop maxargs arg
---
cmd/mtd.c | 476 --
1 file changed, 281 insertions(+), 195 deletions(-)
diff --git a/cmd/mtd.c b/cmd/mtd.c
index 614222398467..5b415aaa1d86 100644
--- a/cmd/mtd.c
+++ b/cmd/mtd.c
@@ -15,6 +15,22 @@
#include
#include
+#include
+
+static struct mtd_info *get_mtd_by_name(const char *name)
+{
+ struct mtd_info *mtd;
+
+ mtd_probe_devices();
+
+ mtd = get_mtd_device_nm(name);
+ if (IS_ERR_OR_NULL(mtd))
+ printf("MTD device %s not found, ret %ld\n", name,
+ PTR_ERR(mtd));
+
+ return mtd;
+}
+
static uint mtd_len_to_pages(struct mtd_info *mtd, u64 len)
{
do_div(len, mtd->writesize);
@@ -177,7 +193,8 @@ static bool mtd_oob_write_is_empty(struct mtd_oob_ops *op)
return true;
}
-static int do_mtd_list(void)
+static int do_mtd_list(cmd_tbl_t *cmdtp, int flag, int argc,
+ char * const argv[])
{
struct mtd_info *mtd;
int dev_nb = 0;
@@ -221,229 +238,287 @@ static int mtd_special_write_oob(struct mtd_info *mtd,
u64 off,
return ret;
}
-static int do_mtd(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+static int do_mtd_io(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
{
+ bool dump, read, raw, woob, write_empty_pages, has_pages = false;
+ u64 start_off, off, len, remaining, default_len;
+ struct mtd_oob_ops io_op = {};
+ uint user_addr = 0, npages;
+ const char *cmd = argv[0];
struct mtd_info *mtd;
- const char *cmd;
- char *mtd_name;
+ u32 oob_len;
+ u8 *buf;
+ int ret;
- /* All MTD commands need at least two arguments */
if (argc < 2)
return CMD_RET_USAGE;
- /* Parse the command name and its optional suffixes */
- cmd = argv[1];
-
- /* List the MTD devices if that is what the user wants */
- if (strcmp(cmd, "list") == 0)
- return do_mtd_list();
-
- /*
-* The remaining commands require also at least a device ID.
-* Check the selected device is valid. Ensure it is probed.
-*/
- if (argc < 3)
- return CMD_RET_USAGE;
-
- mtd_name = argv[2];
- mtd_probe_devices();
- mtd = get_mtd_device_nm(mtd_name);
- if (IS_ERR_OR_NULL(mtd)) {
- printf("MTD device %s not found, ret %ld\n",
- mtd_name, PTR_ERR(mtd));
+ mtd = get_mtd_by_name(argv[1]);
+ if (IS_ERR_OR_NULL(mtd))
return CMD_RET_FAILURE;
+
+ if (mtd->type == MTD_NANDFLASH || mtd->type == MTD_MLCNANDFLASH)
+ has_pages = true;
+
+ dump = !strncmp(cmd, "dump", 4);
+ read = dump || !strncmp(cmd, "read", 4);
+ raw = strstr(cmd, ".raw");
+ woob = strstr(cmd, ".oob");
+ write_empty_pages = !has_pages || strstr(cmd, ".dontskipff");
+
+ argc -= 2;
+ argv += 2;
+
+ if (!dump) {
+ if (!argc) {
+ ret = CMD_RET_USAGE;
+ goto out_put_mtd;
+ }
+
+ user_addr = simple_strtoul(argv[0], NULL, 16);
+ argc--;
+ argv++;
}
- put_mtd_device(mtd);
- argc -= 3;
- argv += 3;
+ start_off = argc > 0 ? simple_strtoul(argv[0], NULL, 16) : 0;
+ if (!mtd_is_aligned_with_min_io_size(mtd, start_off)) {
+ printf("Offset not aligned with a page (0x%x)\n",
+ mtd->writesize);
+ ret = CMD_RET_FAILURE;
+ goto out_put_mtd;
+ }
- /* Do the parsing */
- if (!strncmp(cmd, "read", 4) || !strncmp(cmd, "dump", 4) ||
- !strncmp(cmd, "write", 5)) {
- bool has_pages = mtd->type == MTD_NANDFLASH ||
-mtd->type == MTD_MLCNANDFLASH;
- bool dump, read, raw, woob, write_empty_pages;
- struct mtd_oob_ops io_op = {};
- uint user_addr = 0, npages;
- u64 start_off, off, len, remaining, default_len;
- u32 oob_len;
- u8 *buf;
- int ret;
+ default_len = dump ? mtd->writesize : mtd->size;
+ len = argc > 1 ? simple_strtoul(argv[1], NULL, 16) : default_len;
+ if (!mtd_is_aligned_with_min_io_size(mtd, len)) {
+ len = round_up(len, mtd->writesize);
+ printf("Size not on a page boundary (0x%x), rounding to
0x%llx\n",
+ mtd->writesize, len);
+ }
