[PATCH v2 0/2] bootstd: Fix a handful of doc typos in bootmeth

[PATCH v2 0/2] bootstd: Fix a handful of doc typos in bootmeth

[Mattijs Korpershoek]

While working on bootflow, we noticed some wording inconsistencies with
bootmeth versus bootmethod.

According to [1], we should use bootmeth(s), not bootmethod(s):

"""
For version 2, a new naming scheme is used as above:

    - bootdev is used instead of bootdevice, because 'device' is overused,
        is everywhere in U-Boot, can be confused with udevice
    - bootmeth - because 'method' is too vanilla, appears 1300 times in
        U-Boot
"""

[1] c.f. https://lore.kernel.org/u-boot/20211023232635.9195-1-sjg@chromium.org/

First patch fixes some trivial typos, second patch replaces all
occurences of bootmethod(s) -> bootmeth(s).

Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
---
Changes in v2:
- Made into a series
- New patch replaces all bootmethod(s) -> bootmeth(s)
- Link to v1: https://lore.kernel.org/r/20240603-bootmeth-typos-v1-1-6edbdb469f7c@baylibre.com

---
Mattijs Korpershoek (2):
      bootstd: Fix a handful of doc typos in bootmeth
      bootstd: Replace bootmethod(s) -> bootmeth(s)

 board/sandbox/sandbox.env |  2 +-
 boot/bootmeth-uclass.c    |  2 +-
 include/bootmeth.h        | 42 +++++++++++++++++++++---------------------
 include/extlinux.h        |  2 +-
 test/boot/bootflow.c      |  2 +-
 test/boot/bootmeth.c      |  6 +++---
 6 files changed, 28 insertions(+), 28 deletions(-)
---
base-commit: ea722aa5eb33740ae77e8816aeb72b385e621cd0
change-id: 20240603-bootmeth-typos-47c865e70ccf

Best regards,
-- 
Mattijs Korpershoek <mkorpershoek@baylibre.com>

[PATCH v2 1/2] bootstd: Fix a handful of doc typos in bootmeth

[Mattijs Korpershoek]

Fix some trivial typos found by browsing the code.
Done with flyspell.

Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de>
Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
---
 include/bootmeth.h | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/bootmeth.h b/include/bootmeth.h
index 0fc36104ece0..529c4d813d82 100644
--- a/include/bootmeth.h
+++ b/include/bootmeth.h
@@ -40,7 +40,7 @@ struct bootmeth_ops {
 	/**
 	 * get_state_desc() - get detailed state information
 	 *
-	 * Prodecues a textual description of the state of the bootmeth. This
+	 * Produces a textual description of the state of the bootmeth. This
 	 * can include newline characters if it extends to multiple lines. It
 	 * must be a nul-terminated string.
 	 *
@@ -138,7 +138,7 @@ struct bootmeth_ops {
 	 * @dev:	Bootmethod device to boot
 	 * @bflow:	Bootflow to boot
 	 * Return: does not return on success, since it should boot the
-	 *	Operating Systemn. Returns -EFAULT if that fails, -ENOTSUPP if
+	 *	Operating System. Returns -EFAULT if that fails, -ENOTSUPP if
 	 *	trying method resulted in finding out that is not actually
 	 *	supported for this boot and should not be tried again unless
 	 *	something changes, other -ve on other error
@@ -151,7 +151,7 @@ struct bootmeth_ops {
 /**
  * bootmeth_get_state_desc() - get detailed state information
  *
- * Prodecues a textual description of the state of the bootmeth. This
+ * Produces a textual description of the state of the bootmeth. This
  * can include newline characters if it extends to multiple lines. It
  * must be a nul-terminated string.
  *
@@ -244,7 +244,7 @@ int bootmeth_read_file(struct udevice *dev, struct bootflow *bflow,
  * @dev:	Bootmethod device to use
  * @bflow:	Bootflow to read
  * Return: does not return on success, since it should boot the
- *	Operating Systemn. Returns -EFAULT if that fails, other -ve on
+ *	Operating System. Returns -EFAULT if that fails, other -ve on
  *	other error
  */
 int bootmeth_read_all(struct udevice *dev, struct bootflow *bflow);
@@ -255,7 +255,7 @@ int bootmeth_read_all(struct udevice *dev, struct bootflow *bflow);
  * @dev:	Bootmethod device to boot
  * @bflow:	Bootflow to boot
  * Return: does not return on success, since it should boot the
- *	Operating Systemn. Returns -EFAULT if that fails, other -ve on
+ *	Operating System. Returns -EFAULT if that fails, other -ve on
  *	other error
  */
 int bootmeth_boot(struct udevice *dev, struct bootflow *bflow);
@@ -264,7 +264,7 @@ int bootmeth_boot(struct udevice *dev, struct bootflow *bflow);
  * bootmeth_setup_iter_order() - Set up the ordering of bootmeths to scan
  *
  * This sets up the ordering information in @iter, based on the selected
- * ordering of the bootmethds in bootstd_priv->bootmeth_order. If there is no
+ * ordering of the bootmeths in bootstd_priv->bootmeth_order. If there is no
  * ordering there, then all bootmethods are added
  *
  * @iter: Iterator to update with the order

-- 
2.45.0

Re: [PATCH v2 1/2] bootstd: Fix a handful of doc typos in bootmeth

[Guillaume LA ROQUE]

Hi,


Le 04/06/2024 à 17:15, Mattijs Korpershoek a écrit :
> Fix some trivial typos found by browsing the code.
> Done with flyspell.
>
> Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de>
> Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
> ---
>   include/bootmeth.h | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/include/bootmeth.h b/include/bootmeth.h
> index 0fc36104ece0..529c4d813d82 100644
> --- a/include/bootmeth.h
> +++ b/include/bootmeth.h
> @@ -40,7 +40,7 @@ struct bootmeth_ops {
>   	/**
>   	 * get_state_desc() - get detailed state information
>   	 *
> -	 * Prodecues a textual description of the state of the bootmeth. This
> +	 * Produces a textual description of the state of the bootmeth. This
>   	 * can include newline characters if it extends to multiple lines. It
>   	 * must be a nul-terminated string.
>   	 *
> @@ -138,7 +138,7 @@ struct bootmeth_ops {
>   	 * @dev:	Bootmethod device to boot
>   	 * @bflow:	Bootflow to boot
>   	 * Return: does not return on success, since it should boot the
> -	 *	Operating Systemn. Returns -EFAULT if that fails, -ENOTSUPP if
> +	 *	Operating System. Returns -EFAULT if that fails, -ENOTSUPP if
>   	 *	trying method resulted in finding out that is not actually
>   	 *	supported for this boot and should not be tried again unless
>   	 *	something changes, other -ve on other error
> @@ -151,7 +151,7 @@ struct bootmeth_ops {
>   /**
>    * bootmeth_get_state_desc() - get detailed state information
>    *
> - * Prodecues a textual description of the state of the bootmeth. This
> + * Produces a textual description of the state of the bootmeth. This
>    * can include newline characters if it extends to multiple lines. It
>    * must be a nul-terminated string.
>    *
> @@ -244,7 +244,7 @@ int bootmeth_read_file(struct udevice *dev, struct bootflow *bflow,
>    * @dev:	Bootmethod device to use
>    * @bflow:	Bootflow to read
>    * Return: does not return on success, since it should boot the
> - *	Operating Systemn. Returns -EFAULT if that fails, other -ve on
> + *	Operating System. Returns -EFAULT if that fails, other -ve on
>    *	other error
>    */
>   int bootmeth_read_all(struct udevice *dev, struct bootflow *bflow);
> @@ -255,7 +255,7 @@ int bootmeth_read_all(struct udevice *dev, struct bootflow *bflow);
>    * @dev:	Bootmethod device to boot
>    * @bflow:	Bootflow to boot
>    * Return: does not return on success, since it should boot the
> - *	Operating Systemn. Returns -EFAULT if that fails, other -ve on
> + *	Operating System. Returns -EFAULT if that fails, other -ve on
>    *	other error
>    */
>   int bootmeth_boot(struct udevice *dev, struct bootflow *bflow);
> @@ -264,7 +264,7 @@ int bootmeth_boot(struct udevice *dev, struct bootflow *bflow);
>    * bootmeth_setup_iter_order() - Set up the ordering of bootmeths to scan
>    *
>    * This sets up the ordering information in @iter, based on the selected
> - * ordering of the bootmethds in bootstd_priv->bootmeth_order. If there is no
> + * ordering of the bootmeths in bootstd_priv->bootmeth_order. If there is no
>    * ordering there, then all bootmethods are added
>    *
>    * @iter: Iterator to update with the order
>
Reviewed-by: Guillaume La Roque<glaroque@baylibre.com>

Guillaume

Re: [PATCH v2 1/2] bootstd: Fix a handful of doc typos in bootmeth

[Julien Masson]

On Mon 10 Jun 2024 at 11:46, Mattijs Korpershoek <mkorpershoek@baylibre.com> wrote:

> Fix some trivial typos found by browsing the code.
> Done with flyspell.
> 
> Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de>
> Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
> ---
>  include/bootmeth.h | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/include/bootmeth.h b/include/bootmeth.h
> index 0fc36104ece0..529c4d813d82 100644
> --- a/include/bootmeth.h
> +++ b/include/bootmeth.h
> @@ -40,7 +40,7 @@ struct bootmeth_ops {
>  	/**
>  	 * get_state_desc() - get detailed state information
>  	 *
> -	 * Prodecues a textual description of the state of the bootmeth. This
> +	 * Produces a textual description of the state of the bootmeth. This
>  	 * can include newline characters if it extends to multiple lines. It
>  	 * must be a nul-terminated string.
>  	 *
> @@ -138,7 +138,7 @@ struct bootmeth_ops {
>  	 * @dev:	Bootmethod device to boot
>  	 * @bflow:	Bootflow to boot
>  	 * Return: does not return on success, since it should boot the
> -	 *	Operating Systemn. Returns -EFAULT if that fails, -ENOTSUPP if
> +	 *	Operating System. Returns -EFAULT if that fails, -ENOTSUPP if
>  	 *	trying method resulted in finding out that is not actually
>  	 *	supported for this boot and should not be tried again unless
>  	 *	something changes, other -ve on other error
> @@ -151,7 +151,7 @@ struct bootmeth_ops {
>  /**
>   * bootmeth_get_state_desc() - get detailed state information
>   *
> - * Prodecues a textual description of the state of the bootmeth. This
> + * Produces a textual description of the state of the bootmeth. This
>   * can include newline characters if it extends to multiple lines. It
>   * must be a nul-terminated string.
>   *
> @@ -244,7 +244,7 @@ int bootmeth_read_file(struct udevice *dev, struct bootflow *bflow,
>   * @dev:	Bootmethod device to use
>   * @bflow:	Bootflow to read
>   * Return: does not return on success, since it should boot the
> - *	Operating Systemn. Returns -EFAULT if that fails, other -ve on
> + *	Operating System. Returns -EFAULT if that fails, other -ve on
>   *	other error
>   */
>  int bootmeth_read_all(struct udevice *dev, struct bootflow *bflow);
> @@ -255,7 +255,7 @@ int bootmeth_read_all(struct udevice *dev, struct bootflow *bflow);
>   * @dev:	Bootmethod device to boot
>   * @bflow:	Bootflow to boot
>   * Return: does not return on success, since it should boot the
> - *	Operating Systemn. Returns -EFAULT if that fails, other -ve on
> + *	Operating System. Returns -EFAULT if that fails, other -ve on
>   *	other error
>   */
>  int bootmeth_boot(struct udevice *dev, struct bootflow *bflow);
> @@ -264,7 +264,7 @@ int bootmeth_boot(struct udevice *dev, struct bootflow *bflow);
>   * bootmeth_setup_iter_order() - Set up the ordering of bootmeths to scan
>   *
>   * This sets up the ordering information in @iter, based on the selected
> - * ordering of the bootmethds in bootstd_priv->bootmeth_order. If there is no
> + * ordering of the bootmeths in bootstd_priv->bootmeth_order. If there is no
>   * ordering there, then all bootmethods are added
>   *
>   * @iter: Iterator to update with the order
> 
> -- 
> 2.45.0
> 
> 

Reviewed-by: Julien Masson <jmasson@baylibre.com>
-- 
Julien Masson

[PATCH v2 2/2] bootstd: Replace bootmethod(s) -> bootmeth(s)

[Mattijs Korpershoek]

According to [1], we should use bootmeth when describing the
struct bootmeth:

"""
For version 2, a new naming scheme is used as above:

    - bootdev is used instead of bootdevice, because 'device' is overused,
        is everywhere in U-Boot, can be confused with udevice
    - bootmeth - because 'method' is too vanilla, appears 1300 times in
        U-Boot
"""

Replace all occurences in various comments for consistency.

[1] https://lore.kernel.org/u-boot/20211023232635.9195-1-sjg@chromium.org/
Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
---
 board/sandbox/sandbox.env |  2 +-
 boot/bootmeth-uclass.c    |  2 +-
 include/bootmeth.h        | 30 +++++++++++++++---------------
 include/extlinux.h        |  2 +-
 test/boot/bootflow.c      |  2 +-
 test/boot/bootmeth.c      |  6 +++---
 6 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/board/sandbox/sandbox.env b/board/sandbox/sandbox.env
index a2c19702d64d..564dce78a898 100644
--- a/board/sandbox/sandbox.env
+++ b/board/sandbox/sandbox.env
@@ -10,7 +10,7 @@ eth6addr=02:00:11:22:33:47
 ipaddr=192.0.2.1
 
 /*
- * These are used for distro boot which is not supported. But once bootmethod
+ * These are used for distro boot which is not supported. But once bootmeth
  * is provided these will be used again.
  */
 bootm_size=0x10000000
diff --git a/boot/bootmeth-uclass.c b/boot/bootmeth-uclass.c
index 1d157d54dbdd..e3475f46b34c 100644
--- a/boot/bootmeth-uclass.c
+++ b/boot/bootmeth-uclass.c
@@ -167,7 +167,7 @@ int bootmeth_setup_iter_order(struct bootflow_iter *iter, bool include_global)
 			if (pass)
 				iter->first_glob_method = upto;
 			/*
-			 * Get a list of bootmethods, in seq order (i.e. using
+			 * Get a list of bootmeths, in seq order (i.e. using
 			 * aliases). There may be gaps so try to count up high
 			 * enough to find them all.
 			 */
diff --git a/include/bootmeth.h b/include/bootmeth.h
index 529c4d813d82..2570d9593d49 100644
--- a/include/bootmeth.h
+++ b/include/bootmeth.h
@@ -47,7 +47,7 @@ struct bootmeth_ops {
 	 * This may involve reading state from the system, e.g. some data in
 	 * the firmware area.
 	 *
-	 * @dev:	Bootmethod device to check
+	 * @dev:	Bootmeth device to check
 	 * @buf:	Buffer to place the info in (terminator must fit)
 	 * @maxsize:	Size of buffer
 	 * Returns: 0 if OK, -ENOSPC is buffer is too small, other -ve error if
@@ -74,7 +74,7 @@ struct bootmeth_ops {
 	 *
 	 * It may update only the flags in @iter
 	 *
-	 * @dev:	Bootmethod device to check against
+	 * @dev:	Bootmeth device to check against
 	 * @iter:	On entry, provides bootdev, hwpart, part
 	 * Return: 0 if OK, -ENOTSUPP if this bootdev is not supported
 	 */
@@ -83,7 +83,7 @@ struct bootmeth_ops {
 	/**
 	 * read_bootflow() - read a bootflow for a device
 	 *
-	 * @dev:	Bootmethod device to use
+	 * @dev:	Bootmeth device to use
 	 * @bflow:	On entry, provides dev, hwpart, part and method.
 	 *	Returns updated bootflow if found
 	 * Return: 0 if OK, -ve on error
@@ -96,7 +96,7 @@ struct bootmeth_ops {
 	 * This provides a bootflow file to the bootmeth, to see if it is valid.
 	 * If it is, the bootflow is set up accordingly.
 	 *
-	 * @dev:	Bootmethod device to use
+	 * @dev:	Bootmeth device to use
 	 * @bflow:	On entry, provides bootdev.
 	 *	Returns updated bootflow if found
 	 * @buf:	Buffer containing the possible bootflow file
@@ -111,7 +111,7 @@ struct bootmeth_ops {
 	 *
 	 * Read a file from the same place as the bootflow came from
 	 *
-	 * @dev:	Bootmethod device to use
+	 * @dev:	Bootmeth device to use
 	 * @bflow:	Bootflow providing info on where to read from
 	 * @file_path:	Path to file (may be absolute or relative)
 	 * @addr:	Address to load file
@@ -126,7 +126,7 @@ struct bootmeth_ops {
 	/**
 	 * readall() - read all files for a bootflow
 	 *
-	 * @dev:	Bootmethod device to boot
+	 * @dev:	Bootmeth device to boot
 	 * @bflow:	Bootflow to read
 	 * Return: 0 if OK, -EIO on I/O error, other -ve on other error
 	 */
@@ -135,7 +135,7 @@ struct bootmeth_ops {
 	/**
 	 * boot() - boot a bootflow
 	 *
-	 * @dev:	Bootmethod device to boot
+	 * @dev:	Bootmeth device to boot
 	 * @bflow:	Bootflow to boot
 	 * Return: does not return on success, since it should boot the
 	 *	Operating System. Returns -EFAULT if that fails, -ENOTSUPP if
@@ -158,7 +158,7 @@ struct bootmeth_ops {
  * This may involve reading state from the system, e.g. some data in
  * the firmware area.
  *
- * @dev:	Bootmethod device to check
+ * @dev:	Bootmeth device to check
  * @buf:	Buffer to place the info in (terminator must fit)
  * @maxsize:	Size of buffer
  * Returns: 0 if OK, -ENOSPC is buffer is too small, other -ve error if
@@ -185,7 +185,7 @@ int bootmeth_get_state_desc(struct udevice *dev, char *buf, int maxsize);
  *
  * It may update only the flags in @iter
  *
- * @dev:	Bootmethod device to check against
+ * @dev:	Bootmeth device to check against
  * @iter:	On entry, provides bootdev, hwpart, part
  * Return: 0 if OK, -ENOTSUPP if this bootdev is not supported
  */
@@ -194,7 +194,7 @@ int bootmeth_check(struct udevice *dev, struct bootflow_iter *iter);
 /**
  * bootmeth_read_bootflow() - set up a bootflow for a device
  *
- * @dev:	Bootmethod device to check
+ * @dev:	Bootmeth device to check
  * @bflow:	On entry, provides dev, hwpart, part and method.
  *	Returns updated bootflow if found
  * Return: 0 if OK, -ve on error
@@ -207,7 +207,7 @@ int bootmeth_read_bootflow(struct udevice *dev, struct bootflow *bflow);
  * This provides a bootflow file to the bootmeth, to see if it is valid.
  * If it is, the bootflow is set up accordingly.
  *
- * @dev:	Bootmethod device to use
+ * @dev:	Bootmeth device to use
  * @bflow:	On entry, provides bootdev.
  *	Returns updated bootflow if found
  * @buf:	Buffer containing the possible bootflow file (must be allocated
@@ -223,7 +223,7 @@ int bootmeth_set_bootflow(struct udevice *dev, struct bootflow *bflow,
  *
  * Read a file from the same place as the bootflow came from
  *
- * @dev:	Bootmethod device to use
+ * @dev:	Bootmeth device to use
  * @bflow:	Bootflow providing info on where to read from
  * @file_path:	Path to file (may be absolute or relative)
  * @addr:	Address to load file
@@ -241,7 +241,7 @@ int bootmeth_read_file(struct udevice *dev, struct bootflow *bflow,
  * Some bootmeths delay reading of large files until booting is requested. This
  * causes those files to be read.
  *
- * @dev:	Bootmethod device to use
+ * @dev:	Bootmeth device to use
  * @bflow:	Bootflow to read
  * Return: does not return on success, since it should boot the
  *	Operating System. Returns -EFAULT if that fails, other -ve on
@@ -252,7 +252,7 @@ int bootmeth_read_all(struct udevice *dev, struct bootflow *bflow);
 /**
  * bootmeth_boot() - boot a bootflow
  *
- * @dev:	Bootmethod device to boot
+ * @dev:	Bootmeth device to boot
  * @bflow:	Bootflow to boot
  * Return: does not return on success, since it should boot the
  *	Operating System. Returns -EFAULT if that fails, other -ve on
@@ -265,7 +265,7 @@ int bootmeth_boot(struct udevice *dev, struct bootflow *bflow);
  *
  * This sets up the ordering information in @iter, based on the selected
  * ordering of the bootmeths in bootstd_priv->bootmeth_order. If there is no
- * ordering there, then all bootmethods are added
+ * ordering there, then all bootmeths are added
  *
  * @iter: Iterator to update with the order
  * @include_global: true to add the global bootmeths, in which case they appear
diff --git a/include/extlinux.h b/include/extlinux.h
index 721ba46371cc..4d26a0a8ab74 100644
--- a/include/extlinux.h
+++ b/include/extlinux.h
@@ -12,7 +12,7 @@
 /**
  * struct extlinux_info - useful information for extlinux_getfile()
  *
- * @dev: bootmethod device being used to boot
+ * @dev: bootmeth device being used to boot
  * @bflow: bootflow being booted
  */
 struct extlinux_info {
diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
index 674d4c05f83f..24f3b5fbe109 100644
--- a/test/boot/bootflow.c
+++ b/test/boot/bootflow.c
@@ -400,7 +400,7 @@ BOOTSTD_TEST(bootflow_system, UT_TESTF_DM | UT_TESTF_SCAN_PDATA |
 	     UT_TESTF_SCAN_FDT);
 #endif
 
-/* Check disabling a bootmethod if it requests it */
+/* Check disabling a bootmeth if it requests it */
 static int bootflow_iter_disable(struct unit_test_state *uts)
 {
 	struct udevice *bootstd, *dev;
diff --git a/test/boot/bootmeth.c b/test/boot/bootmeth.c
index e498eee036eb..b9a3f48a95ef 100644
--- a/test/boot/bootmeth.c
+++ b/test/boot/bootmeth.c
@@ -37,7 +37,7 @@ BOOTSTD_TEST(bootmeth_cmd_list, UT_TESTF_DM | UT_TESTF_SCAN_FDT);
 /* Check 'bootmeth order' command */
 static int bootmeth_cmd_order(struct unit_test_state *uts)
 {
-	/* Select just one bootmethod */
+	/* Select just one bootmeth */
 	console_record_reset_enable();
 	ut_assertok(run_command("bootmeth order extlinux", 0));
 	ut_assert_console_end();
@@ -138,12 +138,12 @@ static int bootmeth_env(struct unit_test_state *uts)
 
 	ut_assertok(bootstd_get_priv(&std));
 
-	/* Select just one bootmethod */
+	/* Select just one bootmeth */
 	console_record_reset_enable();
 	ut_assertok(env_set("bootmeths", "extlinux"));
 	ut_asserteq(1, std->bootmeth_count);
 
-	/* Select an invalid bootmethod */
+	/* Select an invalid bootmeth */
 	ut_asserteq(1, run_command("setenv bootmeths fred", 0));
 	ut_assert_nextline("Unknown bootmeth 'fred'");
 	ut_assert_nextlinen("## Error inserting");

-- 
2.45.0

Re: [PATCH v2 2/2] bootstd: Replace bootmethod(s) -> bootmeth(s)

[Quentin Schulz]

Hi Mattijs,

On 6/4/24 5:15 PM, Mattijs Korpershoek wrote:
> According to [1], we should use bootmeth when describing the
> struct bootmeth:
> 
> """
> For version 2, a new naming scheme is used as above:
> 
>      - bootdev is used instead of bootdevice, because 'device' is overused,
>          is everywhere in U-Boot, can be confused with udevice
>      - bootmeth - because 'method' is too vanilla, appears 1300 times in
>          U-Boot
> """
> 
> Replace all occurences in various comments for consistency.
> 

s/occurences/occurrences/

Sorry, was too tempting to highlight a typo in a commit that fixes typos :)

(Don't send a v3 for this please :) )

> [1] https://lore.kernel.org/u-boot/20211023232635.9195-1-sjg@chromium.org/

Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de>

Thanks!
Quentin

Re: [PATCH v2 2/2] bootstd: Replace bootmethod(s) -> bootmeth(s)

[Mattijs Korpershoek]

Hi Quentin,

Thank you for the review.

On mar., juin 04, 2024 at 17:25, Quentin Schulz <quentin.schulz@cherry.de> wrote:

> Hi Mattijs,
>
> On 6/4/24 5:15 PM, Mattijs Korpershoek wrote:
>> According to [1], we should use bootmeth when describing the
>> struct bootmeth:
>> 
>> """
>> For version 2, a new naming scheme is used as above:
>> 
>>      - bootdev is used instead of bootdevice, because 'device' is overused,
>>          is everywhere in U-Boot, can be confused with udevice
>>      - bootmeth - because 'method' is too vanilla, appears 1300 times in
>>          U-Boot
>> """
>> 
>> Replace all occurences in various comments for consistency.
>> 
>
> s/occurences/occurrences/

Urgh, I guess I spot typos from others but not my own ones :/

>
> Sorry, was too tempting to highlight a typo in a commit that fixes typos :)
>
> (Don't send a v3 for this please :) )

I won't. I hope the person applying this patch can do a fixup.

>
>> [1] https://lore.kernel.org/u-boot/20211023232635.9195-1-sjg@chromium.org/
>
> Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de>
>
> Thanks!
> Quentin

Re: [PATCH v2 2/2] bootstd: Replace bootmethod(s) -> bootmeth(s)

[Heinrich Schuchardt]

On 6/4/24 17:15, Mattijs Korpershoek wrote:
> According to [1], we should use bootmeth when describing the
> struct bootmeth:
>
> """
> For version 2, a new naming scheme is used as above:
>
>      - bootdev is used instead of bootdevice, because 'device' is overused,
>          is everywhere in U-Boot, can be confused with udevice

Boot devices are udevices though they don't relate to hardware but to an
abstract concept.

bootdev is just an abbreviation. This does not make the meaning any clearer.

>      - bootmeth - because 'method' is too vanilla, appears 1300 times in
>          U-Boot
> """

Avoiding abbreviations like bootdev and bootmeth improved readability.

Best regards

Heinrich

>
> Replace all occurences in various comments for consistency.
>
> [1] https://lore.kernel.org/u-boot/20211023232635.9195-1-sjg@chromium.org/
> Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
> ---
>   board/sandbox/sandbox.env |  2 +-
>   boot/bootmeth-uclass.c    |  2 +-
>   include/bootmeth.h        | 30 +++++++++++++++---------------
>   include/extlinux.h        |  2 +-
>   test/boot/bootflow.c      |  2 +-
>   test/boot/bootmeth.c      |  6 +++---
>   6 files changed, 22 insertions(+), 22 deletions(-)
>
> diff --git a/board/sandbox/sandbox.env b/board/sandbox/sandbox.env
> index a2c19702d64d..564dce78a898 100644
> --- a/board/sandbox/sandbox.env
> +++ b/board/sandbox/sandbox.env
> @@ -10,7 +10,7 @@ eth6addr=02:00:11:22:33:47
>   ipaddr=192.0.2.1
>
>   /*
> - * These are used for distro boot which is not supported. But once bootmethod
> + * These are used for distro boot which is not supported. But once bootmeth
>    * is provided these will be used again.
>    */
>   bootm_size=0x10000000
> diff --git a/boot/bootmeth-uclass.c b/boot/bootmeth-uclass.c
> index 1d157d54dbdd..e3475f46b34c 100644
> --- a/boot/bootmeth-uclass.c
> +++ b/boot/bootmeth-uclass.c
> @@ -167,7 +167,7 @@ int bootmeth_setup_iter_order(struct bootflow_iter *iter, bool include_global)
>   			if (pass)
>   				iter->first_glob_method = upto;
>   			/*
> -			 * Get a list of bootmethods, in seq order (i.e. using
> +			 * Get a list of bootmeths, in seq order (i.e. using
>   			 * aliases). There may be gaps so try to count up high
>   			 * enough to find them all.
>   			 */
> diff --git a/include/bootmeth.h b/include/bootmeth.h
> index 529c4d813d82..2570d9593d49 100644
> --- a/include/bootmeth.h
> +++ b/include/bootmeth.h
> @@ -47,7 +47,7 @@ struct bootmeth_ops {
>   	 * This may involve reading state from the system, e.g. some data in
>   	 * the firmware area.
>   	 *
> -	 * @dev:	Bootmethod device to check
> +	 * @dev:	Bootmeth device to check
>   	 * @buf:	Buffer to place the info in (terminator must fit)
>   	 * @maxsize:	Size of buffer
>   	 * Returns: 0 if OK, -ENOSPC is buffer is too small, other -ve error if
> @@ -74,7 +74,7 @@ struct bootmeth_ops {
>   	 *
>   	 * It may update only the flags in @iter
>   	 *
> -	 * @dev:	Bootmethod device to check against
> +	 * @dev:	Bootmeth device to check against
>   	 * @iter:	On entry, provides bootdev, hwpart, part
>   	 * Return: 0 if OK, -ENOTSUPP if this bootdev is not supported
>   	 */
> @@ -83,7 +83,7 @@ struct bootmeth_ops {
>   	/**
>   	 * read_bootflow() - read a bootflow for a device
>   	 *
> -	 * @dev:	Bootmethod device to use
> +	 * @dev:	Bootmeth device to use
>   	 * @bflow:	On entry, provides dev, hwpart, part and method.
>   	 *	Returns updated bootflow if found
>   	 * Return: 0 if OK, -ve on error
> @@ -96,7 +96,7 @@ struct bootmeth_ops {
>   	 * This provides a bootflow file to the bootmeth, to see if it is valid.
>   	 * If it is, the bootflow is set up accordingly.
>   	 *
> -	 * @dev:	Bootmethod device to use
> +	 * @dev:	Bootmeth device to use
>   	 * @bflow:	On entry, provides bootdev.
>   	 *	Returns updated bootflow if found
>   	 * @buf:	Buffer containing the possible bootflow file
> @@ -111,7 +111,7 @@ struct bootmeth_ops {
>   	 *
>   	 * Read a file from the same place as the bootflow came from
>   	 *
> -	 * @dev:	Bootmethod device to use
> +	 * @dev:	Bootmeth device to use
>   	 * @bflow:	Bootflow providing info on where to read from
>   	 * @file_path:	Path to file (may be absolute or relative)
>   	 * @addr:	Address to load file
> @@ -126,7 +126,7 @@ struct bootmeth_ops {
>   	/**
>   	 * readall() - read all files for a bootflow
>   	 *
> -	 * @dev:	Bootmethod device to boot
> +	 * @dev:	Bootmeth device to boot
>   	 * @bflow:	Bootflow to read
>   	 * Return: 0 if OK, -EIO on I/O error, other -ve on other error
>   	 */
> @@ -135,7 +135,7 @@ struct bootmeth_ops {
>   	/**
>   	 * boot() - boot a bootflow
>   	 *
> -	 * @dev:	Bootmethod device to boot
> +	 * @dev:	Bootmeth device to boot
>   	 * @bflow:	Bootflow to boot
>   	 * Return: does not return on success, since it should boot the
>   	 *	Operating System. Returns -EFAULT if that fails, -ENOTSUPP if
> @@ -158,7 +158,7 @@ struct bootmeth_ops {
>    * This may involve reading state from the system, e.g. some data in
>    * the firmware area.
>    *
> - * @dev:	Bootmethod device to check
> + * @dev:	Bootmeth device to check
>    * @buf:	Buffer to place the info in (terminator must fit)
>    * @maxsize:	Size of buffer
>    * Returns: 0 if OK, -ENOSPC is buffer is too small, other -ve error if
> @@ -185,7 +185,7 @@ int bootmeth_get_state_desc(struct udevice *dev, char *buf, int maxsize);
>    *
>    * It may update only the flags in @iter
>    *
> - * @dev:	Bootmethod device to check against
> + * @dev:	Bootmeth device to check against
>    * @iter:	On entry, provides bootdev, hwpart, part
>    * Return: 0 if OK, -ENOTSUPP if this bootdev is not supported
>    */
> @@ -194,7 +194,7 @@ int bootmeth_check(struct udevice *dev, struct bootflow_iter *iter);
>   /**
>    * bootmeth_read_bootflow() - set up a bootflow for a device
>    *
> - * @dev:	Bootmethod device to check
> + * @dev:	Bootmeth device to check
>    * @bflow:	On entry, provides dev, hwpart, part and method.
>    *	Returns updated bootflow if found
>    * Return: 0 if OK, -ve on error
> @@ -207,7 +207,7 @@ int bootmeth_read_bootflow(struct udevice *dev, struct bootflow *bflow);
>    * This provides a bootflow file to the bootmeth, to see if it is valid.
>    * If it is, the bootflow is set up accordingly.
>    *
> - * @dev:	Bootmethod device to use
> + * @dev:	Bootmeth device to use
>    * @bflow:	On entry, provides bootdev.
>    *	Returns updated bootflow if found
>    * @buf:	Buffer containing the possible bootflow file (must be allocated
> @@ -223,7 +223,7 @@ int bootmeth_set_bootflow(struct udevice *dev, struct bootflow *bflow,
>    *
>    * Read a file from the same place as the bootflow came from
>    *
> - * @dev:	Bootmethod device to use
> + * @dev:	Bootmeth device to use
>    * @bflow:	Bootflow providing info on where to read from
>    * @file_path:	Path to file (may be absolute or relative)
>    * @addr:	Address to load file
> @@ -241,7 +241,7 @@ int bootmeth_read_file(struct udevice *dev, struct bootflow *bflow,
>    * Some bootmeths delay reading of large files until booting is requested. This
>    * causes those files to be read.
>    *
> - * @dev:	Bootmethod device to use
> + * @dev:	Bootmeth device to use
>    * @bflow:	Bootflow to read
>    * Return: does not return on success, since it should boot the
>    *	Operating System. Returns -EFAULT if that fails, other -ve on
> @@ -252,7 +252,7 @@ int bootmeth_read_all(struct udevice *dev, struct bootflow *bflow);
>   /**
>    * bootmeth_boot() - boot a bootflow
>    *
> - * @dev:	Bootmethod device to boot
> + * @dev:	Bootmeth device to boot
>    * @bflow:	Bootflow to boot
>    * Return: does not return on success, since it should boot the
>    *	Operating System. Returns -EFAULT if that fails, other -ve on
> @@ -265,7 +265,7 @@ int bootmeth_boot(struct udevice *dev, struct bootflow *bflow);
>    *
>    * This sets up the ordering information in @iter, based on the selected
>    * ordering of the bootmeths in bootstd_priv->bootmeth_order. If there is no
> - * ordering there, then all bootmethods are added
> + * ordering there, then all bootmeths are added
>    *
>    * @iter: Iterator to update with the order
>    * @include_global: true to add the global bootmeths, in which case they appear
> diff --git a/include/extlinux.h b/include/extlinux.h
> index 721ba46371cc..4d26a0a8ab74 100644
> --- a/include/extlinux.h
> +++ b/include/extlinux.h
> @@ -12,7 +12,7 @@
>   /**
>    * struct extlinux_info - useful information for extlinux_getfile()
>    *
> - * @dev: bootmethod device being used to boot
> + * @dev: bootmeth device being used to boot
>    * @bflow: bootflow being booted
>    */
>   struct extlinux_info {
> diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
> index 674d4c05f83f..24f3b5fbe109 100644
> --- a/test/boot/bootflow.c
> +++ b/test/boot/bootflow.c
> @@ -400,7 +400,7 @@ BOOTSTD_TEST(bootflow_system, UT_TESTF_DM | UT_TESTF_SCAN_PDATA |
>   	     UT_TESTF_SCAN_FDT);
>   #endif
>
> -/* Check disabling a bootmethod if it requests it */
> +/* Check disabling a bootmeth if it requests it */
>   static int bootflow_iter_disable(struct unit_test_state *uts)
>   {
>   	struct udevice *bootstd, *dev;
> diff --git a/test/boot/bootmeth.c b/test/boot/bootmeth.c
> index e498eee036eb..b9a3f48a95ef 100644
> --- a/test/boot/bootmeth.c
> +++ b/test/boot/bootmeth.c
> @@ -37,7 +37,7 @@ BOOTSTD_TEST(bootmeth_cmd_list, UT_TESTF_DM | UT_TESTF_SCAN_FDT);
>   /* Check 'bootmeth order' command */
>   static int bootmeth_cmd_order(struct unit_test_state *uts)
>   {
> -	/* Select just one bootmethod */
> +	/* Select just one bootmeth */
>   	console_record_reset_enable();
>   	ut_assertok(run_command("bootmeth order extlinux", 0));
>   	ut_assert_console_end();
> @@ -138,12 +138,12 @@ static int bootmeth_env(struct unit_test_state *uts)
>
>   	ut_assertok(bootstd_get_priv(&std));
>
> -	/* Select just one bootmethod */
> +	/* Select just one bootmeth */
>   	console_record_reset_enable();
>   	ut_assertok(env_set("bootmeths", "extlinux"));
>   	ut_asserteq(1, std->bootmeth_count);
>
> -	/* Select an invalid bootmethod */
> +	/* Select an invalid bootmeth */
>   	ut_asserteq(1, run_command("setenv bootmeths fred", 0));
>   	ut_assert_nextline("Unknown bootmeth 'fred'");
>   	ut_assert_nextlinen("## Error inserting");
>

Re: [PATCH v2 2/2] bootstd: Replace bootmethod(s) -> bootmeth(s)

[Mattijs Korpershoek]

Hi Heinrich,

Thank you for your review.

On dim., juin 16, 2024 at 09:38, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:

> On 6/4/24 17:15, Mattijs Korpershoek wrote:
>> According to [1], we should use bootmeth when describing the
>> struct bootmeth:
>>
>> """
>> For version 2, a new naming scheme is used as above:
>>
>>      - bootdev is used instead of bootdevice, because 'device' is overused,
>>          is everywhere in U-Boot, can be confused with udevice
>
> Boot devices are udevices though they don't relate to hardware but to an
> abstract concept.
>
> bootdev is just an abbreviation. This does not make the meaning any clearer.

Per my understanding, the name for this concept is "bootdev", not
"boot device", see:

https://docs.u-boot.org/en/latest/develop/bootstd.html#introduction

>
>>      - bootmeth - because 'method' is too vanilla, appears 1300 times in
>>          U-Boot
>> """
>
> Avoiding abbreviations like bootdev and bootmeth improved readability.

The above paragraph is quoted from email [1].
In this email, Simon made the choice to use bootmeth and bootdev
when pushing the initial implementation.

This patch just corrects the places where the older terminology
(bootmethod, bootdevice) was still used.

So i'm a bit confused on why this patch got rejected.
Is it preferable to keep two terminologies for the same concept?

Thanks
Mattijs


[1] https://lore.kernel.org/u-boot/20211023232635.9195-1-sjg@chromium.org/

>
> Best regards
>
> Heinrich
>
>>
>> Replace all occurences in various comments for consistency.
>>
>> [1] https://lore.kernel.org/u-boot/20211023232635.9195-1-sjg@chromium.org/
>> Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
>> ---
>>   board/sandbox/sandbox.env |  2 +-
>>   boot/bootmeth-uclass.c    |  2 +-
>>   include/bootmeth.h        | 30 +++++++++++++++---------------
>>   include/extlinux.h        |  2 +-
>>   test/boot/bootflow.c      |  2 +-
>>   test/boot/bootmeth.c      |  6 +++---
>>   6 files changed, 22 insertions(+), 22 deletions(-)
>>
>> diff --git a/board/sandbox/sandbox.env b/board/sandbox/sandbox.env
>> index a2c19702d64d..564dce78a898 100644
>> --- a/board/sandbox/sandbox.env
>> +++ b/board/sandbox/sandbox.env
>> @@ -10,7 +10,7 @@ eth6addr=02:00:11:22:33:47
>>   ipaddr=192.0.2.1
>>
>>   /*
>> - * These are used for distro boot which is not supported. But once bootmethod
>> + * These are used for distro boot which is not supported. But once bootmeth
>>    * is provided these will be used again.
>>    */
>>   bootm_size=0x10000000
>> diff --git a/boot/bootmeth-uclass.c b/boot/bootmeth-uclass.c
>> index 1d157d54dbdd..e3475f46b34c 100644
>> --- a/boot/bootmeth-uclass.c
>> +++ b/boot/bootmeth-uclass.c
>> @@ -167,7 +167,7 @@ int bootmeth_setup_iter_order(struct bootflow_iter *iter, bool include_global)
>>   			if (pass)
>>   				iter->first_glob_method = upto;
>>   			/*
>> -			 * Get a list of bootmethods, in seq order (i.e. using
>> +			 * Get a list of bootmeths, in seq order (i.e. using
>>   			 * aliases). There may be gaps so try to count up high
>>   			 * enough to find them all.
>>   			 */
>> diff --git a/include/bootmeth.h b/include/bootmeth.h
>> index 529c4d813d82..2570d9593d49 100644
>> --- a/include/bootmeth.h
>> +++ b/include/bootmeth.h
>> @@ -47,7 +47,7 @@ struct bootmeth_ops {
>>   	 * This may involve reading state from the system, e.g. some data in
>>   	 * the firmware area.
>>   	 *
>> -	 * @dev:	Bootmethod device to check
>> +	 * @dev:	Bootmeth device to check
>>   	 * @buf:	Buffer to place the info in (terminator must fit)
>>   	 * @maxsize:	Size of buffer
>>   	 * Returns: 0 if OK, -ENOSPC is buffer is too small, other -ve error if
>> @@ -74,7 +74,7 @@ struct bootmeth_ops {
>>   	 *
>>   	 * It may update only the flags in @iter
>>   	 *
>> -	 * @dev:	Bootmethod device to check against
>> +	 * @dev:	Bootmeth device to check against
>>   	 * @iter:	On entry, provides bootdev, hwpart, part
>>   	 * Return: 0 if OK, -ENOTSUPP if this bootdev is not supported
>>   	 */
>> @@ -83,7 +83,7 @@ struct bootmeth_ops {
>>   	/**
>>   	 * read_bootflow() - read a bootflow for a device
>>   	 *
>> -	 * @dev:	Bootmethod device to use
>> +	 * @dev:	Bootmeth device to use
>>   	 * @bflow:	On entry, provides dev, hwpart, part and method.
>>   	 *	Returns updated bootflow if found
>>   	 * Return: 0 if OK, -ve on error
>> @@ -96,7 +96,7 @@ struct bootmeth_ops {
>>   	 * This provides a bootflow file to the bootmeth, to see if it is valid.
>>   	 * If it is, the bootflow is set up accordingly.
>>   	 *
>> -	 * @dev:	Bootmethod device to use
>> +	 * @dev:	Bootmeth device to use
>>   	 * @bflow:	On entry, provides bootdev.
>>   	 *	Returns updated bootflow if found
>>   	 * @buf:	Buffer containing the possible bootflow file
>> @@ -111,7 +111,7 @@ struct bootmeth_ops {
>>   	 *
>>   	 * Read a file from the same place as the bootflow came from
>>   	 *
>> -	 * @dev:	Bootmethod device to use
>> +	 * @dev:	Bootmeth device to use
>>   	 * @bflow:	Bootflow providing info on where to read from
>>   	 * @file_path:	Path to file (may be absolute or relative)
>>   	 * @addr:	Address to load file
>> @@ -126,7 +126,7 @@ struct bootmeth_ops {
>>   	/**
>>   	 * readall() - read all files for a bootflow
>>   	 *
>> -	 * @dev:	Bootmethod device to boot
>> +	 * @dev:	Bootmeth device to boot
>>   	 * @bflow:	Bootflow to read
>>   	 * Return: 0 if OK, -EIO on I/O error, other -ve on other error
>>   	 */
>> @@ -135,7 +135,7 @@ struct bootmeth_ops {
>>   	/**
>>   	 * boot() - boot a bootflow
>>   	 *
>> -	 * @dev:	Bootmethod device to boot
>> +	 * @dev:	Bootmeth device to boot
>>   	 * @bflow:	Bootflow to boot
>>   	 * Return: does not return on success, since it should boot the
>>   	 *	Operating System. Returns -EFAULT if that fails, -ENOTSUPP if
>> @@ -158,7 +158,7 @@ struct bootmeth_ops {
>>    * This may involve reading state from the system, e.g. some data in
>>    * the firmware area.
>>    *
>> - * @dev:	Bootmethod device to check
>> + * @dev:	Bootmeth device to check
>>    * @buf:	Buffer to place the info in (terminator must fit)
>>    * @maxsize:	Size of buffer
>>    * Returns: 0 if OK, -ENOSPC is buffer is too small, other -ve error if
>> @@ -185,7 +185,7 @@ int bootmeth_get_state_desc(struct udevice *dev, char *buf, int maxsize);
>>    *
>>    * It may update only the flags in @iter
>>    *
>> - * @dev:	Bootmethod device to check against
>> + * @dev:	Bootmeth device to check against
>>    * @iter:	On entry, provides bootdev, hwpart, part
>>    * Return: 0 if OK, -ENOTSUPP if this bootdev is not supported
>>    */
>> @@ -194,7 +194,7 @@ int bootmeth_check(struct udevice *dev, struct bootflow_iter *iter);
>>   /**
>>    * bootmeth_read_bootflow() - set up a bootflow for a device
>>    *
>> - * @dev:	Bootmethod device to check
>> + * @dev:	Bootmeth device to check
>>    * @bflow:	On entry, provides dev, hwpart, part and method.
>>    *	Returns updated bootflow if found
>>    * Return: 0 if OK, -ve on error
>> @@ -207,7 +207,7 @@ int bootmeth_read_bootflow(struct udevice *dev, struct bootflow *bflow);
>>    * This provides a bootflow file to the bootmeth, to see if it is valid.
>>    * If it is, the bootflow is set up accordingly.
>>    *
>> - * @dev:	Bootmethod device to use
>> + * @dev:	Bootmeth device to use
>>    * @bflow:	On entry, provides bootdev.
>>    *	Returns updated bootflow if found
>>    * @buf:	Buffer containing the possible bootflow file (must be allocated
>> @@ -223,7 +223,7 @@ int bootmeth_set_bootflow(struct udevice *dev, struct bootflow *bflow,
>>    *
>>    * Read a file from the same place as the bootflow came from
>>    *
>> - * @dev:	Bootmethod device to use
>> + * @dev:	Bootmeth device to use
>>    * @bflow:	Bootflow providing info on where to read from
>>    * @file_path:	Path to file (may be absolute or relative)
>>    * @addr:	Address to load file
>> @@ -241,7 +241,7 @@ int bootmeth_read_file(struct udevice *dev, struct bootflow *bflow,
>>    * Some bootmeths delay reading of large files until booting is requested. This
>>    * causes those files to be read.
>>    *
>> - * @dev:	Bootmethod device to use
>> + * @dev:	Bootmeth device to use
>>    * @bflow:	Bootflow to read
>>    * Return: does not return on success, since it should boot the
>>    *	Operating System. Returns -EFAULT if that fails, other -ve on
>> @@ -252,7 +252,7 @@ int bootmeth_read_all(struct udevice *dev, struct bootflow *bflow);
>>   /**
>>    * bootmeth_boot() - boot a bootflow
>>    *
>> - * @dev:	Bootmethod device to boot
>> + * @dev:	Bootmeth device to boot
>>    * @bflow:	Bootflow to boot
>>    * Return: does not return on success, since it should boot the
>>    *	Operating System. Returns -EFAULT if that fails, other -ve on
>> @@ -265,7 +265,7 @@ int bootmeth_boot(struct udevice *dev, struct bootflow *bflow);
>>    *
>>    * This sets up the ordering information in @iter, based on the selected
>>    * ordering of the bootmeths in bootstd_priv->bootmeth_order. If there is no
>> - * ordering there, then all bootmethods are added
>> + * ordering there, then all bootmeths are added
>>    *
>>    * @iter: Iterator to update with the order
>>    * @include_global: true to add the global bootmeths, in which case they appear
>> diff --git a/include/extlinux.h b/include/extlinux.h
>> index 721ba46371cc..4d26a0a8ab74 100644
>> --- a/include/extlinux.h
>> +++ b/include/extlinux.h
>> @@ -12,7 +12,7 @@
>>   /**
>>    * struct extlinux_info - useful information for extlinux_getfile()
>>    *
>> - * @dev: bootmethod device being used to boot
>> + * @dev: bootmeth device being used to boot
>>    * @bflow: bootflow being booted
>>    */
>>   struct extlinux_info {
>> diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
>> index 674d4c05f83f..24f3b5fbe109 100644
>> --- a/test/boot/bootflow.c
>> +++ b/test/boot/bootflow.c
>> @@ -400,7 +400,7 @@ BOOTSTD_TEST(bootflow_system, UT_TESTF_DM | UT_TESTF_SCAN_PDATA |
>>   	     UT_TESTF_SCAN_FDT);
>>   #endif
>>
>> -/* Check disabling a bootmethod if it requests it */
>> +/* Check disabling a bootmeth if it requests it */
>>   static int bootflow_iter_disable(struct unit_test_state *uts)
>>   {
>>   	struct udevice *bootstd, *dev;
>> diff --git a/test/boot/bootmeth.c b/test/boot/bootmeth.c
>> index e498eee036eb..b9a3f48a95ef 100644
>> --- a/test/boot/bootmeth.c
>> +++ b/test/boot/bootmeth.c
>> @@ -37,7 +37,7 @@ BOOTSTD_TEST(bootmeth_cmd_list, UT_TESTF_DM | UT_TESTF_SCAN_FDT);
>>   /* Check 'bootmeth order' command */
>>   static int bootmeth_cmd_order(struct unit_test_state *uts)
>>   {
>> -	/* Select just one bootmethod */
>> +	/* Select just one bootmeth */
>>   	console_record_reset_enable();
>>   	ut_assertok(run_command("bootmeth order extlinux", 0));
>>   	ut_assert_console_end();
>> @@ -138,12 +138,12 @@ static int bootmeth_env(struct unit_test_state *uts)
>>
>>   	ut_assertok(bootstd_get_priv(&std));
>>
>> -	/* Select just one bootmethod */
>> +	/* Select just one bootmeth */
>>   	console_record_reset_enable();
>>   	ut_assertok(env_set("bootmeths", "extlinux"));
>>   	ut_asserteq(1, std->bootmeth_count);
>>
>> -	/* Select an invalid bootmethod */
>> +	/* Select an invalid bootmeth */
>>   	ut_asserteq(1, run_command("setenv bootmeths fred", 0));
>>   	ut_assert_nextline("Unknown bootmeth 'fred'");
>>   	ut_assert_nextlinen("## Error inserting");
>>

Re: [PATCH v2 2/2] bootstd: Replace bootmethod(s) -> bootmeth(s)

[Quentin Schulz]

Hi all,

On 6/17/24 8:31 AM, Mattijs Korpershoek wrote:
> Hi Heinrich,
> 
> Thank you for your review.
> 
> On dim., juin 16, 2024 at 09:38, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> 
>> On 6/4/24 17:15, Mattijs Korpershoek wrote:
>>> According to [1], we should use bootmeth when describing the
>>> struct bootmeth:
>>>
>>> """
>>> For version 2, a new naming scheme is used as above:
>>>
>>>       - bootdev is used instead of bootdevice, because 'device' is overused,
>>>           is everywhere in U-Boot, can be confused with udevice
>>
>> Boot devices are udevices though they don't relate to hardware but to an
>> abstract concept.
>>
>> bootdev is just an abbreviation. This does not make the meaning any clearer.
> 
> Per my understanding, the name for this concept is "bootdev", not
> "boot device", see:
> 
> https://docs.u-boot.org/en/latest/develop/bootstd.html#introduction
> 
>>
>>>       - bootmeth - because 'method' is too vanilla, appears 1300 times in
>>>           U-Boot
>>> """
>>
>> Avoiding abbreviations like bootdev and bootmeth improved readability.
> 
> The above paragraph is quoted from email [1].
> In this email, Simon made the choice to use bootmeth and bootdev
> when pushing the initial implementation.
> 
> This patch just corrects the places where the older terminology
> (bootmethod, bootdevice) was still used.
> 

The current wording is just incorrect, so it needs to be fixed. We have 
two choices: use the struct/abbreviated name (bootdevice -> bootdev; 
bootmethod -> bootmeth) or the full name (bootdevice -> boot device; 
bootmethod -> boot method).

Heinrich are you suggesting we go for full name instead?

board/sandbox/sandbox.env should be using bootmeth instead as that's the 
name of the feature?

Cheers,
Quentin

Re: [PATCH v2 2/2] bootstd: Replace bootmethod(s) -> bootmeth(s)

[Heinrich Schuchardt]

On 17.06.24 11:00, Quentin Schulz wrote:
> Hi all,
>
> On 6/17/24 8:31 AM, Mattijs Korpershoek wrote:
>> Hi Heinrich,
>>
>> Thank you for your review.
>>
>> On dim., juin 16, 2024 at 09:38, Heinrich Schuchardt
>> <xypron.glpk@gmx.de> wrote:
>>
>>> On 6/4/24 17:15, Mattijs Korpershoek wrote:
>>>> According to [1], we should use bootmeth when describing the
>>>> struct bootmeth:
>>>>
>>>> """
>>>> For version 2, a new naming scheme is used as above:
>>>>
>>>>       - bootdev is used instead of bootdevice, because 'device' is
>>>> overused,
>>>>           is everywhere in U-Boot, can be confused with udevice
>>>
>>> Boot devices are udevices though they don't relate to hardware but to an
>>> abstract concept.
>>>
>>> bootdev is just an abbreviation. This does not make the meaning any
>>> clearer.
>>
>> Per my understanding, the name for this concept is "bootdev", not
>> "boot device", see:
>>
>> https://docs.u-boot.org/en/latest/develop/bootstd.html#introduction
>>
>>>
>>>>       - bootmeth - because 'method' is too vanilla, appears 1300
>>>> times in
>>>>           U-Boot
>>>> """
>>>
>>> Avoiding abbreviations like bootdev and bootmeth improved readability.
>>
>> The above paragraph is quoted from email [1].
>> In this email, Simon made the choice to use bootmeth and bootdev
>> when pushing the initial implementation.
>>
>> This patch just corrects the places where the older terminology
>> (bootmethod, bootdevice) was still used.
>>
>
> The current wording is just incorrect, so it needs to be fixed. We have
> two choices: use the struct/abbreviated name (bootdevice -> bootdev;
> bootmethod -> bootmeth) or the full name (bootdevice -> boot device;
> bootmethod -> boot method).

The English languages has three types of compound words: solid,
hyphenated, open. bootmethod, boot-method, boot method all mean the same.

According to https://www.merriam-webster.com/help/faq-compound-words:
"Compound nouns are usually written as one word."

See also "U.S. Government Publishing Office Style Manual", chapter 6,
"COMPOUNDING RULES",
https://www.govinfo.gov/content/pkg/GPO-STYLEMANUAL-2000/pdf/GPO-STYLEMANUAL-2000.pdf

We should avoid unnecessary abbreviations.

Best regards

Heinrich

>
> Heinrich are you suggesting we go for full name instead?
>
> board/sandbox/sandbox.env should be using bootmeth instead as that's the
> name of the feature?
>
> Cheers,
> Quentin

Re: [PATCH v2 2/2] bootstd: Replace bootmethod(s) -> bootmeth(s)

[Quentin Schulz]

Hi Heinrich,

On 6/17/24 11:24 AM, Heinrich Schuchardt wrote:
> On 17.06.24 11:00, Quentin Schulz wrote:
>> Hi all,
>>
>> On 6/17/24 8:31 AM, Mattijs Korpershoek wrote:
>>> Hi Heinrich,
>>>
>>> Thank you for your review.
>>>
>>> On dim., juin 16, 2024 at 09:38, Heinrich Schuchardt
>>> <xypron.glpk@gmx.de> wrote:
>>>
>>>> On 6/4/24 17:15, Mattijs Korpershoek wrote:
>>>>> According to [1], we should use bootmeth when describing the
>>>>> struct bootmeth:
>>>>>
>>>>> """
>>>>> For version 2, a new naming scheme is used as above:
>>>>>
>>>>>       - bootdev is used instead of bootdevice, because 'device' is
>>>>> overused,
>>>>>           is everywhere in U-Boot, can be confused with udevice
>>>>
>>>> Boot devices are udevices though they don't relate to hardware but 
>>>> to an
>>>> abstract concept.
>>>>
>>>> bootdev is just an abbreviation. This does not make the meaning any
>>>> clearer.
>>>
>>> Per my understanding, the name for this concept is "bootdev", not
>>> "boot device", see:
>>>
>>> https://docs.u-boot.org/en/latest/develop/bootstd.html#introduction
>>>
>>>>
>>>>>       - bootmeth - because 'method' is too vanilla, appears 1300
>>>>> times in
>>>>>           U-Boot
>>>>> """
>>>>
>>>> Avoiding abbreviations like bootdev and bootmeth improved readability.
>>>
>>> The above paragraph is quoted from email [1].
>>> In this email, Simon made the choice to use bootmeth and bootdev
>>> when pushing the initial implementation.
>>>
>>> This patch just corrects the places where the older terminology
>>> (bootmethod, bootdevice) was still used.
>>>
>>
>> The current wording is just incorrect, so it needs to be fixed. We have
>> two choices: use the struct/abbreviated name (bootdevice -> bootdev;
>> bootmethod -> bootmeth) or the full name (bootdevice -> boot device;
>> bootmethod -> boot method).
> 
> The English languages has three types of compound words: solid,
> hyphenated, open. bootmethod, boot-method, boot method all mean the same.
> 
> According to 
> https://www.merriam-webster.com/help/faq-compound-words:
> "Compound nouns are usually written as one word."
> 
> See also "U.S. Government Publishing Office Style Manual", chapter 6,
> "COMPOUNDING RULES",
> https://www.govinfo.gov/content/pkg/GPO-STYLEMANUAL-2000/pdf/GPO-STYLEMANUAL-2000.pdf
> 
> We should avoid unnecessary abbreviations.
> 

It's too late, the abbreviations are used in code already.

For documentation:
https://docs.u-boot.org/en/latest/search.html?q=bootmethod&check_keywords=yes&area=default#

bootmethod returns no match.

https://docs.u-boot.org/en/latest/develop/bootstd.html#bootmeth
https://docs.u-boot.org/en/latest/develop/expo.html#motivation
https://docs.u-boot.org/en/latest/usage/cmd/bootflow.html#bootflow-list
use "boot method"

all other instances are of bootmeth/bootmeths instead. A little 
consistency here wouldn't hurt. I don't think the current wording is 
consistent. I am non-native, if I read bootmethod, I assume it is used 
with this wording in the code, but it actually isn't, it's called 
bootmeth. If I read "boot method" I understand a way of booting, and I 
will likely not grep in the source code to find how that works.

If it's a NACK, please say so. Otherwise please provide clear 
instructions so we know the wording we should be using for this to be 
accepted.

Cheers,
Quentin

Re: [PATCH v2 2/2] bootstd: Replace bootmethod(s) -> bootmeth(s)

[Simon Glass]

Hi,

On Sun, 16 Jun 2024 at 01:43, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 6/4/24 17:15, Mattijs Korpershoek wrote:
> > According to [1], we should use bootmeth when describing the
> > struct bootmeth:
> >
> > """
> > For version 2, a new naming scheme is used as above:
> >
> >      - bootdev is used instead of bootdevice, because 'device' is overused,
> >          is everywhere in U-Boot, can be confused with udevice
>
> Boot devices are udevices though they don't relate to hardware but to an
> abstract concept.
>
> bootdev is just an abbreviation. This does not make the meaning any clearer.
>
> >      - bootmeth - because 'method' is too vanilla, appears 1300 times in
> >          U-Boot
> > """
>
> Avoiding abbreviations like bootdev and bootmeth improved readability.

I think people need to learn the terms used in the code. Having a term
used consistently seems like a win to me. The documentation helps here
too, e.g. [1]

Reviewed-by: Simon Glass <sjg@chromium.org>

Regards,
Simon

[1] https://docs.u-boot.org/en/latest/develop/bootstd.html#bootmeth