| Message ID | 20240125140502.22400-4-o451686892@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Random Number Generator fixes | expand |
On 25.01.24 15:05, Weizhao Ouyang wrote: > Add rng list command to list all probed RNG device. Thank you for your contribution. Would the following be more accurate? The 'rng list' command probes all RNG devices and list those devices that are successfully probed. > > Signed-off-by: Weizhao Ouyang <o451686892@gmail.com> > --- > cmd/rng.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/cmd/rng.c b/cmd/rng.c > index 52f722c7af..4818133f94 100644 > --- a/cmd/rng.c > +++ b/cmd/rng.c > @@ -18,6 +18,19 @@ static int do_rng(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) > int devnum; > struct udevice *dev; > int ret = CMD_RET_SUCCESS; > + int idx; Please, put the definition into the code block where it is used. > + > + if (argc == 2 && !strcmp(argv[1], "list")) { int idx = 0; > + uclass_foreach_dev_probe(UCLASS_RNG, dev) { > + printf("RNG #%d - %s\n", dev->seq_, dev->name); ++idx; > + } > + > + if (!idx) { > + printf("*** no RNG devices available ***\n"); If idx is not initialized, depending on the random value of idx on the stack you get a printout or not, e.g. => rng list RNG #0 - riscv_zkr RNG #1 - virtio-rng#8 *** no RNG devices available *** Do we need the message to be so noisy? How about log_err("No RNG device\n"); > + return CMD_RET_FAILURE; > + } We prefer having an empty line before return statements. Best regards Heinrich > + return CMD_RET_SUCCESS; > + } > > switch (argc) { > case 1: > @@ -57,6 +70,8 @@ static int do_rng(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) > } > > U_BOOT_LONGHELP(rng, > + "[list]\n" > + " - list all the probe rng device\n" > "[dev [n]]\n" > " - print n random bytes(max 64) read from dev\n"); >
On Thu, Jan 25, 2024 at 10:49 PM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > On 25.01.24 15:05, Weizhao Ouyang wrote: > > Add rng list command to list all probed RNG device. > > Thank you for your contribution. > > Would the following be more accurate? > > The 'rng list' command probes all RNG devices and list those devices > that are successfully probed. Yeah. > > > > > Signed-off-by: Weizhao Ouyang <o451686892@gmail.com> > > --- > > cmd/rng.c | 15 +++++++++++++++ > > 1 file changed, 15 insertions(+) > > > > diff --git a/cmd/rng.c b/cmd/rng.c > > index 52f722c7af..4818133f94 100644 > > --- a/cmd/rng.c > > +++ b/cmd/rng.c > > @@ -18,6 +18,19 @@ static int do_rng(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) > > int devnum; > > struct udevice *dev; > > int ret = CMD_RET_SUCCESS; > > + int idx; > > Please, put the definition into the code block where it is used. Sorry I imported the draft code, will remove it in the next version. > > > + > > + if (argc == 2 && !strcmp(argv[1], "list")) { > > int idx = 0; > > > + uclass_foreach_dev_probe(UCLASS_RNG, dev) { > > + printf("RNG #%d - %s\n", dev->seq_, dev->name); > > ++idx; > > > + } > > + > > + if (!idx) { > > + printf("*** no RNG devices available ***\n"); > > If idx is not initialized, depending on the random value of idx on the > stack you get a printout or not, e.g. > > => rng list > RNG #0 - riscv_zkr > RNG #1 - virtio-rng#8 > *** no RNG devices available *** > > Do we need the message to be so noisy? How about > > log_err("No RNG device\n"); > > > > + return CMD_RET_FAILURE; > > + } > > We prefer having an empty line before return statements. > > Best regards > > Heinrich > > > + return CMD_RET_SUCCESS; > > + } > > > > switch (argc) { > > case 1: > > @@ -57,6 +70,8 @@ static int do_rng(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) > > } > > > > U_BOOT_LONGHELP(rng, > > + "[list]\n" > > + " - list all the probe rng device\n" > > "[dev [n]]\n" > > " - print n random bytes(max 64) read from dev\n"); > > > BR, Weizhao
diff --git a/cmd/rng.c b/cmd/rng.c index 52f722c7af..4818133f94 100644 --- a/cmd/rng.c +++ b/cmd/rng.c @@ -18,6 +18,19 @@ static int do_rng(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) int devnum; struct udevice *dev; int ret = CMD_RET_SUCCESS; + int idx; + + if (argc == 2 && !strcmp(argv[1], "list")) { + uclass_foreach_dev_probe(UCLASS_RNG, dev) { + printf("RNG #%d - %s\n", dev->seq_, dev->name); + } + + if (!idx) { + printf("*** no RNG devices available ***\n"); + return CMD_RET_FAILURE; + } + return CMD_RET_SUCCESS; + } switch (argc) { case 1: @@ -57,6 +70,8 @@ static int do_rng(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) } U_BOOT_LONGHELP(rng, + "[list]\n" + " - list all the probe rng device\n" "[dev [n]]\n" " - print n random bytes(max 64) read from dev\n");
Add rng list command to list all probed RNG device. Signed-off-by: Weizhao Ouyang <o451686892@gmail.com> --- cmd/rng.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)