Re: [PATCH v4 4/5] sf: doc: Add documentation for the 'sf' command
Hi Heinrich, On Sat, 13 Nov 2021 at 08:32, Heinrich Schuchardt wrote: > > Am 13. November 2021 15:21:13 MEZ schrieb Simon Glass : > >Hi Heinrich, > > > >On Sat, 13 Nov 2021 at 04:34, Heinrich Schuchardt wrote: > >> > >> On 9/19/21 23:49, Simon Glass wrote: > >> > This command is fairly complicated so documentation is useful. > >> > Unfortunately I an not sure how the MTD side of things works and cannot > >> > find information about that. > >> > > >> > Signed-off-by: Simon Glass > >> > > >> > Acked-by: Pratyush Yadav > >> > --- > >> > > >> > Changes in v4: > >> > - Split out the 'const' change into a separate patch > >> > - Show the 'sf probe' output in the examples > >> > > >> > Changes in v2: > >> > - Many fixes as suggested by Heinrich > >> > > >> > doc/usage/index.rst | 1 + > >> > doc/usage/sf.rst| 245 > >> > 2 files changed, 246 insertions(+) > >> > create mode 100644 doc/usage/sf.rst > >> > Applied to u-boot-dm, thanks! Applied to u-boot-dm/next, thanks!
Re: [PATCH v4 4/5] sf: doc: Add documentation for the 'sf' command
Hi Heinrich, On Sat, 13 Nov 2021 at 08:32, Heinrich Schuchardt wrote: > > Am 13. November 2021 15:21:13 MEZ schrieb Simon Glass : > >Hi Heinrich, > > > >On Sat, 13 Nov 2021 at 04:34, Heinrich Schuchardt wrote: > >> > >> On 9/19/21 23:49, Simon Glass wrote: > >> > This command is fairly complicated so documentation is useful. > >> > Unfortunately I an not sure how the MTD side of things works and cannot > >> > find information about that. > >> > > >> > Signed-off-by: Simon Glass > >> > > >> > Acked-by: Pratyush Yadav > >> > --- > >> > > >> > Changes in v4: > >> > - Split out the 'const' change into a separate patch > >> > - Show the 'sf probe' output in the examples > >> > > >> > Changes in v2: > >> > - Many fixes as suggested by Heinrich > >> > > >> > doc/usage/index.rst | 1 + > >> > doc/usage/sf.rst| 245 > >> > 2 files changed, 246 insertions(+) > >> > create mode 100644 doc/usage/sf.rst > >> > Applied to u-boot-dm, thanks!
Re: [PATCH v4 4/5] sf: doc: Add documentation for the 'sf' command
Hi Heinrich, On Sat, 13 Nov 2021 at 08:32, Heinrich Schuchardt wrote: > > Am 13. November 2021 15:21:13 MEZ schrieb Simon Glass : > >Hi Heinrich, > > > >On Sat, 13 Nov 2021 at 04:34, Heinrich Schuchardt wrote: > >> > >> On 9/19/21 23:49, Simon Glass wrote: > >> > This command is fairly complicated so documentation is useful. > >> > Unfortunately I an not sure how the MTD side of things works and cannot > >> > find information about that. > >> > > >> > Signed-off-by: Simon Glass > >> > > >> > Acked-by: Pratyush Yadav > >> > --- > >> > > >> > Changes in v4: > >> > - Split out the 'const' change into a separate patch > >> > - Show the 'sf probe' output in the examples > >> > > >> > Changes in v2: > >> > - Many fixes as suggested by Heinrich > >> > > >> > doc/usage/index.rst | 1 + > >> > doc/usage/sf.rst| 245 > >> > 2 files changed, 246 insertions(+) > >> > create mode 100644 doc/usage/sf.rst > >> > > >> > diff --git a/doc/usage/index.rst b/doc/usage/index.rst > >> > index 356f2a56181..9a7b12b7c54 100644 > >> > --- a/doc/usage/index.rst > >> > +++ b/doc/usage/index.rst > >> > @@ -43,6 +43,7 @@ Shell commands > >> > qfw > >> > reset > >> > sbi > >> > + sf > >> > >> Please, keep this list in alphabetical order. > >> > >> > scp03 > >> > setexpr > >> > size > >> > diff --git a/doc/usage/sf.rst b/doc/usage/sf.rst > >> > new file mode 100644 > >> > index 000..71bd1be5175 > >> > --- /dev/null > >> > +++ b/doc/usage/sf.rst > >> > @@ -0,0 +1,245 @@ > >> > +.. SPDX-License-Identifier: GPL-2.0+: > >> > + > >> > +sf command > >> > +== > >> > + > >> > +Synopis > >> > +--- > >> > + > >> > +:: > >> > + > >> > +sf probe [[[:]] [ []]] > >> > +sf read | > >> > +sf write | > >> > +sf erase | > >> > +sf update | > >> > +sf protect lock|unlock > >> > +sf test | > >> > + > >> > +Description > >> > +--- > >> > + > >> > +The *sf* command is used to access SPI flash, supporting > >> > read/write/erase and > >> > +a few other functions. > >> > + > >> > +Probe > >> > +- > >> > + > >> > +The flash must first be probed with *sf probe* before any of the other > >> > +subcommands can be used. All of the parameters are optional: > >> > + > >> > +bus > >> > + SPI bus number containing the SPI-flash chip, e.g. 0. If you don't > >> > know > >> > + the number, you can use 'dm uclass' to see all the spi devices, > >> > + and check the value for 'seq' for each one (here 0 and 2):: > >> > >> I would have expected the 'spi' command to have an info sub-command like > >> the other subsystems. But that is missing. > >> > >> > + > >> > +uclass 89: spi > >> > +0 spi@0 @ 05484960, seq 0 > >> > +1 spi@1 @ 05484b40, seq 2 > >> > + > >> > +cs > >> > + SPI chip-select to use for the chip. This is often 0 and can be > >> > omitted, > >> > + but in some cases multiple slaves are attached to a SPI controller, > >> > + selected by a chip-select line for each one. > >> > + > >> > +hz > >> > + Speed of the SPI bus in hertz. This normally defaults to 10, > >> > i.e. > >> > + 100KHz, which is very slow. Note that if the device exists in the > >> > + device tree, there might be a speed provided there, in which case > >> > this > >> > + setting is ignored. > >> > + > >> > +mode > >> > + SPI mode to use: > >> > + > >> > + = > >> > + Mode Meaning > >> > + = > >> > + 0 CPOL=0, CPHA=0 > >> > + 1 CPOL=0, CPHA=1 > >> > + 2 CPOL=1, CPHA=0 > >> > + 3 CPOL=1, CPHA=1 > >> > + = > >> > + > >> > + Clock phase (CPHA) 0 means that data is transferred (sampled) on > >> > the > >> > + first clock edge; 1 means the second. > >> > + > >> > + Clock polarity (CPOL) controls the idle state of the clock, 0 for > >> > low, > >> > + 1 for high. > >> > + The active state is the opposite of idle. > >> > + > >> > + You may find this `SPI documentation`_ useful. > >> > + > >> > +Parameters for other subcommands (described below) are as follows: > >> > >> I would not expect parameters for other sub-commands in chapter "Probe". > >> > >> Please put all parameters into a separate section "Parameters". This > >> makes navigation easier. > > > >This series was sent back in April and is now at version 5, after > >multiple rounds of changes. This version alone sat here for nearly two > >months. Who will want to write documentation in U-Boot if this is the > >process? > > > >I don't disagree with most of your comments, just the timing, although > >the 'spi info' thing is highly debatable, as you cannot memory-map SPI > >itself, only SPI flash. > > > >The common.h header removal suffered a similar fate and we have never > >resolved that, now 18 months later. > > > >So please, let's get something in and move
Re: [PATCH v4 4/5] sf: doc: Add documentation for the 'sf' command
Am 13. November 2021 15:21:13 MEZ schrieb Simon Glass : >Hi Heinrich, > >On Sat, 13 Nov 2021 at 04:34, Heinrich Schuchardt wrote: >> >> On 9/19/21 23:49, Simon Glass wrote: >> > This command is fairly complicated so documentation is useful. >> > Unfortunately I an not sure how the MTD side of things works and cannot >> > find information about that. >> > >> > Signed-off-by: Simon Glass >> > >> > Acked-by: Pratyush Yadav >> > --- >> > >> > Changes in v4: >> > - Split out the 'const' change into a separate patch >> > - Show the 'sf probe' output in the examples >> > >> > Changes in v2: >> > - Many fixes as suggested by Heinrich >> > >> > doc/usage/index.rst | 1 + >> > doc/usage/sf.rst| 245 >> > 2 files changed, 246 insertions(+) >> > create mode 100644 doc/usage/sf.rst >> > >> > diff --git a/doc/usage/index.rst b/doc/usage/index.rst >> > index 356f2a56181..9a7b12b7c54 100644 >> > --- a/doc/usage/index.rst >> > +++ b/doc/usage/index.rst >> > @@ -43,6 +43,7 @@ Shell commands >> > qfw >> > reset >> > sbi >> > + sf >> >> Please, keep this list in alphabetical order. >> >> > scp03 >> > setexpr >> > size >> > diff --git a/doc/usage/sf.rst b/doc/usage/sf.rst >> > new file mode 100644 >> > index 000..71bd1be5175 >> > --- /dev/null >> > +++ b/doc/usage/sf.rst >> > @@ -0,0 +1,245 @@ >> > +.. SPDX-License-Identifier: GPL-2.0+: >> > + >> > +sf command >> > +== >> > + >> > +Synopis >> > +--- >> > + >> > +:: >> > + >> > +sf probe [[[:]] [ []]] >> > +sf read | >> > +sf write | >> > +sf erase | >> > +sf update | >> > +sf protect lock|unlock >> > +sf test | >> > + >> > +Description >> > +--- >> > + >> > +The *sf* command is used to access SPI flash, supporting read/write/erase >> > and >> > +a few other functions. >> > + >> > +Probe >> > +- >> > + >> > +The flash must first be probed with *sf probe* before any of the other >> > +subcommands can be used. All of the parameters are optional: >> > + >> > +bus >> > + SPI bus number containing the SPI-flash chip, e.g. 0. If you don't >> > know >> > + the number, you can use 'dm uclass' to see all the spi devices, >> > + and check the value for 'seq' for each one (here 0 and 2):: >> >> I would have expected the 'spi' command to have an info sub-command like >> the other subsystems. But that is missing. >> >> > + >> > +uclass 89: spi >> > +0 spi@0 @ 05484960, seq 0 >> > +1 spi@1 @ 05484b40, seq 2 >> > + >> > +cs >> > + SPI chip-select to use for the chip. This is often 0 and can be >> > omitted, >> > + but in some cases multiple slaves are attached to a SPI controller, >> > + selected by a chip-select line for each one. >> > + >> > +hz >> > + Speed of the SPI bus in hertz. This normally defaults to 10, i.e. >> > + 100KHz, which is very slow. Note that if the device exists in the >> > + device tree, there might be a speed provided there, in which case >> > this >> > + setting is ignored. >> > + >> > +mode >> > + SPI mode to use: >> > + >> > + = >> > + Mode Meaning >> > + = >> > + 0 CPOL=0, CPHA=0 >> > + 1 CPOL=0, CPHA=1 >> > + 2 CPOL=1, CPHA=0 >> > + 3 CPOL=1, CPHA=1 >> > + = >> > + >> > + Clock phase (CPHA) 0 means that data is transferred (sampled) on the >> > + first clock edge; 1 means the second. >> > + >> > + Clock polarity (CPOL) controls the idle state of the clock, 0 for >> > low, >> > + 1 for high. >> > + The active state is the opposite of idle. >> > + >> > + You may find this `SPI documentation`_ useful. >> > + >> > +Parameters for other subcommands (described below) are as follows: >> >> I would not expect parameters for other sub-commands in chapter "Probe". >> >> Please put all parameters into a separate section "Parameters". This >> makes navigation easier. > >This series was sent back in April and is now at version 5, after >multiple rounds of changes. This version alone sat here for nearly two >months. Who will want to write documentation in U-Boot if this is the >process? > >I don't disagree with most of your comments, just the timing, although >the 'spi info' thing is highly debatable, as you cannot memory-map SPI >itself, only SPI flash. > >The common.h header removal suffered a similar fate and we have never >resolved that, now 18 months later. > >So please, let's get something in and move forward. We can still in improve the documentation in a follow up patch. Acked-by: Heinrich Schuchardt > >Regards, >Simon
Re: [PATCH v4 4/5] sf: doc: Add documentation for the 'sf' command
Hi Heinrich, On Sat, 13 Nov 2021 at 04:34, Heinrich Schuchardt wrote: > > On 9/19/21 23:49, Simon Glass wrote: > > This command is fairly complicated so documentation is useful. > > Unfortunately I an not sure how the MTD side of things works and cannot > > find information about that. > > > > Signed-off-by: Simon Glass > > > > Acked-by: Pratyush Yadav > > --- > > > > Changes in v4: > > - Split out the 'const' change into a separate patch > > - Show the 'sf probe' output in the examples > > > > Changes in v2: > > - Many fixes as suggested by Heinrich > > > > doc/usage/index.rst | 1 + > > doc/usage/sf.rst| 245 > > 2 files changed, 246 insertions(+) > > create mode 100644 doc/usage/sf.rst > > > > diff --git a/doc/usage/index.rst b/doc/usage/index.rst > > index 356f2a56181..9a7b12b7c54 100644 > > --- a/doc/usage/index.rst > > +++ b/doc/usage/index.rst > > @@ -43,6 +43,7 @@ Shell commands > > qfw > > reset > > sbi > > + sf > > Please, keep this list in alphabetical order. > > > scp03 > > setexpr > > size > > diff --git a/doc/usage/sf.rst b/doc/usage/sf.rst > > new file mode 100644 > > index 000..71bd1be5175 > > --- /dev/null > > +++ b/doc/usage/sf.rst > > @@ -0,0 +1,245 @@ > > +.. SPDX-License-Identifier: GPL-2.0+: > > + > > +sf command > > +== > > + > > +Synopis > > +--- > > + > > +:: > > + > > +sf probe [[[:]] [ []]] > > +sf read | > > +sf write | > > +sf erase | > > +sf update | > > +sf protect lock|unlock > > +sf test | > > + > > +Description > > +--- > > + > > +The *sf* command is used to access SPI flash, supporting read/write/erase > > and > > +a few other functions. > > + > > +Probe > > +- > > + > > +The flash must first be probed with *sf probe* before any of the other > > +subcommands can be used. All of the parameters are optional: > > + > > +bus > > + SPI bus number containing the SPI-flash chip, e.g. 0. If you don't > > know > > + the number, you can use 'dm uclass' to see all the spi devices, > > + and check the value for 'seq' for each one (here 0 and 2):: > > I would have expected the 'spi' command to have an info sub-command like > the other subsystems. But that is missing. > > > + > > +uclass 89: spi > > +0 spi@0 @ 05484960, seq 0 > > +1 spi@1 @ 05484b40, seq 2 > > + > > +cs > > + SPI chip-select to use for the chip. This is often 0 and can be > > omitted, > > + but in some cases multiple slaves are attached to a SPI controller, > > + selected by a chip-select line for each one. > > + > > +hz > > + Speed of the SPI bus in hertz. This normally defaults to 10, i.e. > > + 100KHz, which is very slow. Note that if the device exists in the > > + device tree, there might be a speed provided there, in which case this > > + setting is ignored. > > + > > +mode > > + SPI mode to use: > > + > > + = > > + Mode Meaning > > + = > > + 0 CPOL=0, CPHA=0 > > + 1 CPOL=0, CPHA=1 > > + 2 CPOL=1, CPHA=0 > > + 3 CPOL=1, CPHA=1 > > + = > > + > > + Clock phase (CPHA) 0 means that data is transferred (sampled) on the > > + first clock edge; 1 means the second. > > + > > + Clock polarity (CPOL) controls the idle state of the clock, 0 for low, > > + 1 for high. > > + The active state is the opposite of idle. > > + > > + You may find this `SPI documentation`_ useful. > > + > > +Parameters for other subcommands (described below) are as follows: > > I would not expect parameters for other sub-commands in chapter "Probe". > > Please put all parameters into a separate section "Parameters". This > makes navigation easier. This series was sent back in April and is now at version 5, after multiple rounds of changes. This version alone sat here for nearly two months. Who will want to write documentation in U-Boot if this is the process? I don't disagree with most of your comments, just the timing, although the 'spi info' thing is highly debatable, as you cannot memory-map SPI itself, only SPI flash. The common.h header removal suffered a similar fate and we have never resolved that, now 18 months later. So please, let's get something in and move forward. Regards, Simon
Re: [PATCH v4 4/5] sf: doc: Add documentation for the 'sf' command
On 9/19/21 23:49, Simon Glass wrote: This command is fairly complicated so documentation is useful. Unfortunately I an not sure how the MTD side of things works and cannot find information about that. Signed-off-by: Simon Glass Acked-by: Pratyush Yadav --- Changes in v4: - Split out the 'const' change into a separate patch - Show the 'sf probe' output in the examples Changes in v2: - Many fixes as suggested by Heinrich doc/usage/index.rst | 1 + doc/usage/sf.rst| 245 2 files changed, 246 insertions(+) create mode 100644 doc/usage/sf.rst diff --git a/doc/usage/index.rst b/doc/usage/index.rst index 356f2a56181..9a7b12b7c54 100644 --- a/doc/usage/index.rst +++ b/doc/usage/index.rst @@ -43,6 +43,7 @@ Shell commands qfw reset sbi + sf Please, keep this list in alphabetical order. scp03 setexpr size diff --git a/doc/usage/sf.rst b/doc/usage/sf.rst new file mode 100644 index 000..71bd1be5175 --- /dev/null +++ b/doc/usage/sf.rst @@ -0,0 +1,245 @@ +.. SPDX-License-Identifier: GPL-2.0+: + +sf command +== + +Synopis +--- + +:: + +sf probe [[[:]] [ []]] +sf read | +sf write | +sf erase | +sf update | +sf protect lock|unlock +sf test | + +Description +--- + +The *sf* command is used to access SPI flash, supporting read/write/erase and +a few other functions. + +Probe +- + +The flash must first be probed with *sf probe* before any of the other +subcommands can be used. All of the parameters are optional: + +bus + SPI bus number containing the SPI-flash chip, e.g. 0. If you don't know + the number, you can use 'dm uclass' to see all the spi devices, + and check the value for 'seq' for each one (here 0 and 2):: I would have expected the 'spi' command to have an info sub-command like the other subsystems. But that is missing. + + uclass 89: spi + 0 spi@0 @ 05484960, seq 0 + 1 spi@1 @ 05484b40, seq 2 + +cs + SPI chip-select to use for the chip. This is often 0 and can be omitted, + but in some cases multiple slaves are attached to a SPI controller, + selected by a chip-select line for each one. + +hz + Speed of the SPI bus in hertz. This normally defaults to 10, i.e. + 100KHz, which is very slow. Note that if the device exists in the + device tree, there might be a speed provided there, in which case this + setting is ignored. + +mode + SPI mode to use: + + = + Mode Meaning + = + 0 CPOL=0, CPHA=0 + 1 CPOL=0, CPHA=1 + 2 CPOL=1, CPHA=0 + 3 CPOL=1, CPHA=1 + = + + Clock phase (CPHA) 0 means that data is transferred (sampled) on the + first clock edge; 1 means the second. + + Clock polarity (CPOL) controls the idle state of the clock, 0 for low, + 1 for high. + The active state is the opposite of idle. + + You may find this `SPI documentation`_ useful. + +Parameters for other subcommands (described below) are as follows: I would not expect parameters for other sub-commands in chapter "Probe". Please put all parameters into a separate section "Parameters". This makes navigation easier. + +addr + Memory address to start transfer + +offset + Flash offset to start transfer + +partition + If the parameter is not numeric, it is assumed to be a partition + description in the format , which is not + covered here. This requires CONFIG_CMD_MTDPARTS. + +len + Number of bytes to transfer + +Read + + +Use *sf read* to read from SPI flash to memory. The read will fail if an +attempt is made to read past the end of the flash. + + +Write +~ + +Use *sf write* to write from memory to SPI flash. The SPI flash should be +erased first, since otherwise the result is undefined. + +The write will fail if an attempt is made to read past the end of the flash. + + +Erase +~ + +Use *sf erase* to erase a region of SPI flash. The erase will fail if any part +of the region to be erased is protected or lies past the end of the flash. It +may also fail if the start offset or length are not aligned to an erase region +(e.g. 256 bytes). + + +Update +~~ + +Use *sf update* to automatically erase and update a region of SPI flash from +memory. This works a sector at a time (typical 4KB or 64KB). For each +sector it first checks if the sector already has the right data. If so it is +skipped. If not, the sector is erased and the new data written. Note that if +the length is not a multiple of the erase size, the space after the data in +the last sector will be erased. If the offset does not start at the beginning +of an erase block, the operation will fail. + +Speed statistics are shown including the number of bytes that were already +correct. + + +Protect +~~~ +
Re: [PATCH v4 4/5] sf: doc: Add documentation for the 'sf' command
On Mon, Sep 20, 2021 at 3:19 AM Simon Glass wrote: > > This command is fairly complicated so documentation is useful. > Unfortunately I an not sure how the MTD side of things works and cannot > find information about that. > > Signed-off-by: Simon Glass > > Acked-by: Pratyush Yadav > --- Reviewed-by: Jagan Teki
