Document powerpc nvram parameters
diff mbox

Message ID 201606012116.u51LBneF026116@mx0a-001b2d01.pphosted.com
State Changes Requested
Headers show

Commit Message

Murilo Opsfelder Araujo June 1, 2016, 9:15 p.m. UTC
Signed-off-by: Murilo Opsfelder Araujo <muriloo@linux.vnet.ibm.com>
---
 discover/platform-powerpc.c | 39 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

--
2.9.0.rc1

Comments

Samuel Mendoza-Jonas June 2, 2016, 6:40 a.m. UTC | #1
On Wed, Jun 01, 2016 at 06:15:36PM -0300, Murilo Opsfelder Araujo wrote:
> Signed-off-by: Murilo Opsfelder Araujo <muriloo@linux.vnet.ibm.com>
> ---
>  discover/platform-powerpc.c | 39 ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 38 insertions(+), 1 deletion(-)
> 
> diff --git a/discover/platform-powerpc.c b/discover/platform-powerpc.c
> index 1961304..563e0ad 100644
> --- a/discover/platform-powerpc.c
> +++ b/discover/platform-powerpc.c
> @@ -51,14 +51,51 @@ struct platform_powerpc {
> 
>  static const char *known_params[] = {
>  	"auto-boot?",
> +	/* Whether Petitboot should automatically boot.  (true or
> +	 * false)
> +	 */
> +
>  	"petitboot,network",
> +	/* Network settings.  Examples:
> +	 *
> +	 * petitboot,network=aa:bb:cc:dd:ee:ff,dhcp
> +	 * petitboot,network=aa:bb:cc:dd:ee:ff,static,192.168.1.2/24,192.168.1.1 dns,192.168.1.1
> +	 */
> +
>  	"petitboot,timeout",
> -	"petitboot,bootdev",
> +	/* How many seconds Petitboot should wait until auto-booting.
> +	 * (Integer)
> +	 */
> +
> +	"petitboot,bootdev",  /* deprecated */
>  	"petitboot,bootdevs",
> +	/* Space-separated list defining the relative boot priority of
> +	 * certain devices, e.g. boot a specific disk first, otherwise
> +	 * network.  Example:
> +	 *
> +	 * petitboot,bootdevs=uuid:a46c84b3-366a-4b38-880a-f674c30d490d network
> +	 */
> +
>  	"petitboot,language",
> +	/* Set UI default language.  Example:
> +	 *
> +	 * petitboot,language=es_ES.utf8
> +	 */
> +
>  	"petitboot,debug?",
> +	/* Debug flag to increase logging verbosity.  (true or
> +	 * false) */
> +
>  	"petitboot,write?",
> +	/* Whether Petitboot should be allowed to make changes to
> +	 * disks.  (true or false)
> +	 */
> +
>  	"petitboot,snapshots?",
> +	/* Whether Petitboot should use device-mapper snapshots when
> +	 * mounting disks.  (true or false)
> +	 */
> +
>  	NULL,
>  };
> 

Thanks Murilo! I like the idea of documenting these options *somewhere*,
but I'm not sure inline in the array is the place, it adds a lot of
height to the declaration. Perhaps it could go above the array slightly
compressed instead?
I suppose the question is, who is the intended audience for this
information. If it's for developers, perhaps each parameter can be
described in the function that interprets it. If it's for users (which
for 90% of cases it probably shouldn't be) then maybe it could go in
some other documentation, whether a file in the source tree, or on a
website.
Thoughts?

sam

> --
> 2.9.0.rc1
> 
> _______________________________________________
> Petitboot mailing list
> Petitboot@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/petitboot
Murilo Opsfelder Araujo June 2, 2016, 2:26 p.m. UTC | #2
On 06/02/2016 03:40 AM, Samuel Mendoza-Jonas wrote:
[...]
> Thanks Murilo! I like the idea of documenting these options *somewhere*,
> but I'm not sure inline in the array is the place, it adds a lot of
> height to the declaration. Perhaps it could go above the array slightly
> compressed instead?

I like that too.

> I suppose the question is, who is the intended audience for this
> information.

I believe the audience is whoever reads the source code, be them
developers or not.

> If it's for developers, perhaps each parameter can be
> described in the function that interprets it.

I confess I'm more inclined to document the purpose of a variable close
to its declaration.

> If it's for users (which for 90% of cases it probably shouldn't be)
> then maybe it could go in some other documentation, whether a file
> in the source tree, or on a website.

What about in man/pb-discover.8 under DESCRIPTION section?  Or even in
man/petitboot.8?
Murilo Opsfelder Araujo July 5, 2016, 1:05 p.m. UTC | #3
On 06/02/2016 11:26 AM, Murilo Opsfelder Araújo wrote:
[...]
> What about in man/pb-discover.8 under DESCRIPTION section?  Or even in
> man/petitboot.8?

Hi, maintainers.

Do we have an agreement on what would be the best place to document
these parameters?
Samuel Mendoza-Jonas July 5, 2016, 10:24 p.m. UTC | #4
On Tue, 2016-07-05 at 10:05 -0300, Murilo Opsfelder Araújo wrote:
> On 06/02/2016 11:26 AM, Murilo Opsfelder Araújo wrote:
> [...]
> > What about in man/pb-discover.8 under DESCRIPTION section?  Or even in
> > man/petitboot.8?
> 
> Hi, maintainers.
> 
> Do we have an agreement on what would be the best place to document
> these parameters?
> 

Hi Murilo,

Ah I must have missed that last question in your email, excuse the late
response.

I'm going to vote against the man pages since the NVRAM parameters are
not necessarily something a user would and/or should be concerned with.
I think something like your original patch but more succinct would be
fine, perhaps as mentioned listed above the parameters.

Cheers,
Sam

Patch
diff mbox

diff --git a/discover/platform-powerpc.c b/discover/platform-powerpc.c
index 1961304..563e0ad 100644
--- a/discover/platform-powerpc.c
+++ b/discover/platform-powerpc.c
@@ -51,14 +51,51 @@  struct platform_powerpc {

 static const char *known_params[] = {
 	"auto-boot?",
+	/* Whether Petitboot should automatically boot.  (true or
+	 * false)
+	 */
+
 	"petitboot,network",
+	/* Network settings.  Examples:
+	 *
+	 * petitboot,network=aa:bb:cc:dd:ee:ff,dhcp
+	 * petitboot,network=aa:bb:cc:dd:ee:ff,static,192.168.1.2/24,192.168.1.1 dns,192.168.1.1
+	 */
+
 	"petitboot,timeout",
-	"petitboot,bootdev",
+	/* How many seconds Petitboot should wait until auto-booting.
+	 * (Integer)
+	 */
+
+	"petitboot,bootdev",  /* deprecated */
 	"petitboot,bootdevs",
+	/* Space-separated list defining the relative boot priority of
+	 * certain devices, e.g. boot a specific disk first, otherwise
+	 * network.  Example:
+	 *
+	 * petitboot,bootdevs=uuid:a46c84b3-366a-4b38-880a-f674c30d490d network
+	 */
+
 	"petitboot,language",
+	/* Set UI default language.  Example:
+	 *
+	 * petitboot,language=es_ES.utf8
+	 */
+
 	"petitboot,debug?",
+	/* Debug flag to increase logging verbosity.  (true or
+	 * false) */
+
 	"petitboot,write?",
+	/* Whether Petitboot should be allowed to make changes to
+	 * disks.  (true or false)
+	 */
+
 	"petitboot,snapshots?",
+	/* Whether Petitboot should use device-mapper snapshots when
+	 * mounting disks.  (true or false)
+	 */
+
 	NULL,
 };