Re: [PATCH v5 6/9] cmd: rng: Add support for selecting RNG device

2022-03-24 Thread Sughosh Ganu
hi Heinrich,

On Mon, 14 Mar 2022 at 03:53, Simon Glass  wrote:
>
> Hi Sughosh,
>
> On Sun, 13 Mar 2022 at 08:49, Sughosh Ganu  wrote:
> >
> > The 'rng' u-boot command is used for printing a select number of
> > random bytes on the console. Currently, the RNG device from which the
> > random bytes are read is fixed. However, a platform can have multiple
> > RNG devices, one example being qemu, which has a virtio RNG device and
> > the RNG pseudo device through the TPM chip.
> >
> > Extend the 'rng' command so that the user can provide the RNG device
> > number from which the random bytes are to be read. This will be the
> > device index under the RNG uclass.
> >
> > Signed-off-by: Sughosh Ganu 
> > Tested-by: Heinrich Schuchardt 
> > Reviewed-by: Ilias Apalodimas 
> > ---
> >
> > Changes since V4:
> >
> > * Use uclass_get_device_by_seq API to get the RNG device as suggested
> >   by Simon
> >
> >  cmd/rng.c | 31 +++
> >  1 file changed, 23 insertions(+), 8 deletions(-)

Can you please pick this up, as this has been reviewed and is not
related to the TPM RNG changes which are still under discussion.
Thanks.

-sughosh

>
> Reviewed-by: Simon Glass 
>
> with the nit below fixed
>
> >
> > diff --git a/cmd/rng.c b/cmd/rng.c
> > index 1ad5a096c0..2ddf27545f 100644
> > --- a/cmd/rng.c
> > +++ b/cmd/rng.c
> > @@ -13,19 +13,34 @@
> >
> >  static int do_rng(struct cmd_tbl *cmdtp, int flag, int argc, char *const 
> > argv[])
> >  {
> > -   size_t n = 0x40;
> > +   size_t n;
> > struct udevice *dev;
> > void *buf;
> > +   int devnum;
> > int ret = CMD_RET_SUCCESS;
> >
> > -   if (uclass_get_device(UCLASS_RNG, 0, ) || !dev) {
> > +   switch (argc) {
> > +   case 1:
> > +   devnum = 0;
> > +   n = 0x40;
> > +   break;
> > +   case 2:
> > +   devnum = hextoul(argv[1], NULL);
> > +   n = 0x40;
> > +   break;
> > +   case 3:
> > +   devnum = hextoul(argv[1], NULL);
> > +   n = hextoul(argv[2], NULL);
> > +   break;
> > +   default:
> > +   return CMD_RET_USAGE;
> > +   }
> > +
> > +   if (uclass_get_device_by_seq(UCLASS_RNG, devnum, ) || !dev) {
>
> Please check the function comments: you can drop the '|| !dev' bit
> since it returns an error if no device is found.
>
>
> > printf("No RNG device\n");
> > return CMD_RET_FAILURE;
> > }
> >
> > -   if (argc >= 2)
> > -   n = hextoul(argv[1], NULL);
> > -
> > buf = malloc(n);
> > if (!buf) {
> > printf("Out of memory\n");
> > @@ -46,12 +61,12 @@ static int do_rng(struct cmd_tbl *cmdtp, int flag, int 
> > argc, char *const argv[])
> >
> >  #ifdef CONFIG_SYS_LONGHELP
> >  static char rng_help_text[] =
> > -   "[n]\n"
> > -   "  - print n random bytes\n";
> > +   "[dev [n]]\n"
> > +   "  - print n random bytes read from dev\n";
> >  #endif
> >
> >  U_BOOT_CMD(
> > -   rng, 2, 0, do_rng,
> > +   rng, 3, 0, do_rng,
> > "print bytes from the hardware random number generator",
> > rng_help_text
> >  );
> > --
> > 2.25.1
> >
>
> Regards,
> SImon


Re: [PATCH v5 6/9] cmd: rng: Add support for selecting RNG device

2022-03-13 Thread Simon Glass
Hi Sughosh,

On Sun, 13 Mar 2022 at 08:49, Sughosh Ganu  wrote:
>
> The 'rng' u-boot command is used for printing a select number of
> random bytes on the console. Currently, the RNG device from which the
> random bytes are read is fixed. However, a platform can have multiple
> RNG devices, one example being qemu, which has a virtio RNG device and
> the RNG pseudo device through the TPM chip.
>
> Extend the 'rng' command so that the user can provide the RNG device
> number from which the random bytes are to be read. This will be the
> device index under the RNG uclass.
>
> Signed-off-by: Sughosh Ganu 
> Tested-by: Heinrich Schuchardt 
> Reviewed-by: Ilias Apalodimas 
> ---
>
> Changes since V4:
>
> * Use uclass_get_device_by_seq API to get the RNG device as suggested
>   by Simon
>
>  cmd/rng.c | 31 +++
>  1 file changed, 23 insertions(+), 8 deletions(-)

Reviewed-by: Simon Glass 

with the nit below fixed

>
> diff --git a/cmd/rng.c b/cmd/rng.c
> index 1ad5a096c0..2ddf27545f 100644
> --- a/cmd/rng.c
> +++ b/cmd/rng.c
> @@ -13,19 +13,34 @@
>
>  static int do_rng(struct cmd_tbl *cmdtp, int flag, int argc, char *const 
> argv[])
>  {
> -   size_t n = 0x40;
> +   size_t n;
> struct udevice *dev;
> void *buf;
> +   int devnum;
> int ret = CMD_RET_SUCCESS;
>
> -   if (uclass_get_device(UCLASS_RNG, 0, ) || !dev) {
> +   switch (argc) {
> +   case 1:
> +   devnum = 0;
> +   n = 0x40;
> +   break;
> +   case 2:
> +   devnum = hextoul(argv[1], NULL);
> +   n = 0x40;
> +   break;
> +   case 3:
> +   devnum = hextoul(argv[1], NULL);
> +   n = hextoul(argv[2], NULL);
> +   break;
> +   default:
> +   return CMD_RET_USAGE;
> +   }
> +
> +   if (uclass_get_device_by_seq(UCLASS_RNG, devnum, ) || !dev) {

Please check the function comments: you can drop the '|| !dev' bit
since it returns an error if no device is found.


> printf("No RNG device\n");
> return CMD_RET_FAILURE;
> }
>
> -   if (argc >= 2)
> -   n = hextoul(argv[1], NULL);
> -
> buf = malloc(n);
> if (!buf) {
> printf("Out of memory\n");
> @@ -46,12 +61,12 @@ static int do_rng(struct cmd_tbl *cmdtp, int flag, int 
> argc, char *const argv[])
>
>  #ifdef CONFIG_SYS_LONGHELP
>  static char rng_help_text[] =
> -   "[n]\n"
> -   "  - print n random bytes\n";
> +   "[dev [n]]\n"
> +   "  - print n random bytes read from dev\n";
>  #endif
>
>  U_BOOT_CMD(
> -   rng, 2, 0, do_rng,
> +   rng, 3, 0, do_rng,
> "print bytes from the hardware random number generator",
> rng_help_text
>  );
> --
> 2.25.1
>

Regards,
SImon


[PATCH v5 6/9] cmd: rng: Add support for selecting RNG device

2022-03-13 Thread Sughosh Ganu
The 'rng' u-boot command is used for printing a select number of
random bytes on the console. Currently, the RNG device from which the
random bytes are read is fixed. However, a platform can have multiple
RNG devices, one example being qemu, which has a virtio RNG device and
the RNG pseudo device through the TPM chip.

Extend the 'rng' command so that the user can provide the RNG device
number from which the random bytes are to be read. This will be the
device index under the RNG uclass.

Signed-off-by: Sughosh Ganu 
Tested-by: Heinrich Schuchardt 
Reviewed-by: Ilias Apalodimas 
---

Changes since V4:

* Use uclass_get_device_by_seq API to get the RNG device as suggested
  by Simon

 cmd/rng.c | 31 +++
 1 file changed, 23 insertions(+), 8 deletions(-)

diff --git a/cmd/rng.c b/cmd/rng.c
index 1ad5a096c0..2ddf27545f 100644
--- a/cmd/rng.c
+++ b/cmd/rng.c
@@ -13,19 +13,34 @@
 
 static int do_rng(struct cmd_tbl *cmdtp, int flag, int argc, char *const 
argv[])
 {
-   size_t n = 0x40;
+   size_t n;
struct udevice *dev;
void *buf;
+   int devnum;
int ret = CMD_RET_SUCCESS;
 
-   if (uclass_get_device(UCLASS_RNG, 0, ) || !dev) {
+   switch (argc) {
+   case 1:
+   devnum = 0;
+   n = 0x40;
+   break;
+   case 2:
+   devnum = hextoul(argv[1], NULL);
+   n = 0x40;
+   break;
+   case 3:
+   devnum = hextoul(argv[1], NULL);
+   n = hextoul(argv[2], NULL);
+   break;
+   default:
+   return CMD_RET_USAGE;
+   }
+
+   if (uclass_get_device_by_seq(UCLASS_RNG, devnum, ) || !dev) {
printf("No RNG device\n");
return CMD_RET_FAILURE;
}
 
-   if (argc >= 2)
-   n = hextoul(argv[1], NULL);
-
buf = malloc(n);
if (!buf) {
printf("Out of memory\n");
@@ -46,12 +61,12 @@ static int do_rng(struct cmd_tbl *cmdtp, int flag, int 
argc, char *const argv[])
 
 #ifdef CONFIG_SYS_LONGHELP
 static char rng_help_text[] =
-   "[n]\n"
-   "  - print n random bytes\n";
+   "[dev [n]]\n"
+   "  - print n random bytes read from dev\n";
 #endif
 
 U_BOOT_CMD(
-   rng, 2, 0, do_rng,
+   rng, 3, 0, do_rng,
"print bytes from the hardware random number generator",
rng_help_text
 );
-- 
2.25.1