diff mbox series

[v2,1/2] rng: move platform_get_rng_device() to rng-uclass.c

Message ID 20231102091648.40520-2-avromanov@salutedevices.com
State Needs Review / ACK
Delegated to: Tom Rini
Headers show
Series Add dm_rng_read_default() helper | expand

Commit Message

Alexey Romanov Nov. 2, 2023, 9:16 a.m. UTC
The correct declaration place for platform_get_rng_device()
function is here. Also, this function is re-implemented to provide
the first successfully probed RNG device.

Signed-off-by: Alexey Romanov <avromanov@salutedevices.com>
---
 drivers/rng/rng-uclass.c | 16 ++++++++++++++++
 include/efi_rng.h        |  2 --
 include/rng.h            | 11 +++++++++++
 lib/efi_loader/efi_rng.c | 27 ---------------------------
 4 files changed, 27 insertions(+), 29 deletions(-)

Comments

Heinrich Schuchardt Nov. 2, 2023, 9:42 p.m. UTC | #1
On 11/2/23 11:16, Alexey Romanov wrote:
> The correct declaration place for platform_get_rng_device()
> function is here. Also, this function is re-implemented to provide
> the first successfully probed RNG device.
>
> Signed-off-by: Alexey Romanov <avromanov@salutedevices.com>
> ---
>   drivers/rng/rng-uclass.c | 16 ++++++++++++++++
>   include/efi_rng.h        |  2 --
>   include/rng.h            | 11 +++++++++++
>   lib/efi_loader/efi_rng.c | 27 ---------------------------
>   4 files changed, 27 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/rng/rng-uclass.c b/drivers/rng/rng-uclass.c
> index 53108e1b31..d7236b9335 100644
> --- a/drivers/rng/rng-uclass.c
> +++ b/drivers/rng/rng-uclass.c
> @@ -9,6 +9,22 @@
>   #include <dm.h>
>   #include <rng.h>
>
> +int platform_get_rng_device(struct udevice **dev)

You are dropping __weak here. The rationale behind this change should be
provided in the commit message.

Best regards

Heinrich

> +{
> +	int ret;
> +	struct udevice *devp;
> +
> +	ret = uclass_first_device_check(UCLASS_RNG, &devp);
> +	if (ret) {
> +		pr_err("Unable to get RNG device (%d)\n", ret);
> +		return -ENODEV;
> +	}
> +
> +	*dev = devp;
> +
> +	return 0;
> +}
> +
>   int dm_rng_read(struct udevice *dev, void *buffer, size_t size)
>   {
>   	const struct dm_rng_ops *ops = device_get_ops(dev);
> diff --git a/include/efi_rng.h b/include/efi_rng.h
> index 3c622381cb..f22e54adb0 100644
> --- a/include/efi_rng.h
> +++ b/include/efi_rng.h
> @@ -23,6 +23,4 @@ struct efi_rng_protocol {
>   				       efi_uintn_t rng_value_length, uint8_t *rng_value);
>   };
>
> -efi_status_t platform_get_rng_device(struct udevice **dev);
> -
>   #endif /* _EFI_RNG_H_ */
> diff --git a/include/rng.h b/include/rng.h
> index 37af554363..255c85d3e8 100644
> --- a/include/rng.h
> +++ b/include/rng.h
> @@ -20,6 +20,17 @@ struct udevice;
>    */
>   int dm_rng_read(struct udevice *dev, void *buffer, size_t size);
>
> +/**
> + * platform_get_rng_device() - retrieve random number generator
> + *
> + * This function retrieves the first udevice implementing a hardware
> + * random number generator. Device is already probed.
> + *
> + * @dev:	udevice
> + * Return:	status code
> + */
> +int platform_get_rng_device(struct udevice **dev);
> +
>   /**
>    * struct dm_rng_ops - operations for the hwrng uclass
>    *
> diff --git a/lib/efi_loader/efi_rng.c b/lib/efi_loader/efi_rng.c
> index bb11d8d0e0..0d8bf770f5 100644
> --- a/lib/efi_loader/efi_rng.c
> +++ b/lib/efi_loader/efi_rng.c
> @@ -17,33 +17,6 @@ DECLARE_GLOBAL_DATA_PTR;
>
>   const efi_guid_t efi_guid_rng_protocol = EFI_RNG_PROTOCOL_GUID;
>
> -/**
> - * platform_get_rng_device() - retrieve random number generator
> - *
> - * This function retrieves the udevice implementing a hardware random
> - * number generator.
> - *
> - * This function may be overridden if special initialization is needed.
> - *
> - * @dev:	udevice
> - * Return:	status code
> - */
> -__weak efi_status_t platform_get_rng_device(struct udevice **dev)
> -{
> -	int ret;
> -	struct udevice *devp;
> -
> -	ret = uclass_get_device(UCLASS_RNG, 0, &devp);
> -	if (ret) {
> -		debug("Unable to get rng device\n");
> -		return EFI_DEVICE_ERROR;
> -	}
> -
> -	*dev = devp;
> -
> -	return EFI_SUCCESS;
> -}
> -
>   /**
>    * rng_getinfo() - get information about random number generation
>    *
Simon Glass Nov. 2, 2023, 10:35 p.m. UTC | #2
Hi Alexey,

On Thu, 2 Nov 2023 at 12:58, Alexey Romanov <avromanov@salutedevices.com> wrote:
>
> The correct declaration place for platform_get_rng_device()
> function is here. Also, this function is re-implemented to provide
> the first successfully probed RNG device.
>
> Signed-off-by: Alexey Romanov <avromanov@salutedevices.com>
> ---
>  drivers/rng/rng-uclass.c | 16 ++++++++++++++++
>  include/efi_rng.h        |  2 --
>  include/rng.h            | 11 +++++++++++
>  lib/efi_loader/efi_rng.c | 27 ---------------------------
>  4 files changed, 27 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/rng/rng-uclass.c b/drivers/rng/rng-uclass.c
> index 53108e1b31..d7236b9335 100644
> --- a/drivers/rng/rng-uclass.c
> +++ b/drivers/rng/rng-uclass.c
> @@ -9,6 +9,22 @@
>  #include <dm.h>
>  #include <rng.h>
>
> +int platform_get_rng_device(struct udevice **dev)
> +{
> +       int ret;
> +       struct udevice *devp;
> +
> +       ret = uclass_first_device_check(UCLASS_RNG, &devp);
> +       if (ret) {
> +               pr_err("Unable to get RNG device (%d)\n", ret);
> +               return -ENODEV;
> +       }
> +
> +       *dev = devp;
> +
> +       return 0;
> +}

Why not delete this function? It doesn't seem to do anything useful
except return the wrong error code if something goes wrong :-)

I worry that people might add other logic to it.

Also you might consider using rng0, rng1 aliases so people can
actually choose the device to use? If you want that you could
uclass_get_device_by_seq()

> +
>  int dm_rng_read(struct udevice *dev, void *buffer, size_t size)
>  {
>         const struct dm_rng_ops *ops = device_get_ops(dev);
> diff --git a/include/efi_rng.h b/include/efi_rng.h
> index 3c622381cb..f22e54adb0 100644
> --- a/include/efi_rng.h
> +++ b/include/efi_rng.h
> @@ -23,6 +23,4 @@ struct efi_rng_protocol {
>                                        efi_uintn_t rng_value_length, uint8_t *rng_value);
>  };
>
> -efi_status_t platform_get_rng_device(struct udevice **dev);
> -
>  #endif /* _EFI_RNG_H_ */
> diff --git a/include/rng.h b/include/rng.h
> index 37af554363..255c85d3e8 100644
> --- a/include/rng.h
> +++ b/include/rng.h
> @@ -20,6 +20,17 @@ struct udevice;
>   */
>  int dm_rng_read(struct udevice *dev, void *buffer, size_t size);
>
> +/**
> + * platform_get_rng_device() - retrieve random number generator
> + *
> + * This function retrieves the first udevice implementing a hardware
> + * random number generator. Device is already probed.
> + *
> + * @dev:       udevice
> + * Return:     status code
> + */
> +int platform_get_rng_device(struct udevice **dev);

Better naming (if you do keep this) is rng_get_first()

There is no such thing as a 'platform' in U-Boot.

> +
>  /**
>   * struct dm_rng_ops - operations for the hwrng uclass
>   *
> diff --git a/lib/efi_loader/efi_rng.c b/lib/efi_loader/efi_rng.c
> index bb11d8d0e0..0d8bf770f5 100644
> --- a/lib/efi_loader/efi_rng.c
> +++ b/lib/efi_loader/efi_rng.c
> @@ -17,33 +17,6 @@ DECLARE_GLOBAL_DATA_PTR;
>
>  const efi_guid_t efi_guid_rng_protocol = EFI_RNG_PROTOCOL_GUID;
>
> -/**
> - * platform_get_rng_device() - retrieve random number generator
> - *
> - * This function retrieves the udevice implementing a hardware random
> - * number generator.
> - *
> - * This function may be overridden if special initialization is needed.
> - *
> - * @dev:       udevice
> - * Return:     status code
> - */
> -__weak efi_status_t platform_get_rng_device(struct udevice **dev)
> -{
> -       int ret;
> -       struct udevice *devp;
> -
> -       ret = uclass_get_device(UCLASS_RNG, 0, &devp);
> -       if (ret) {
> -               debug("Unable to get rng device\n");
> -               return EFI_DEVICE_ERROR;
> -       }
> -
> -       *dev = devp;
> -
> -       return EFI_SUCCESS;
> -}
> -
>  /**
>   * rng_getinfo() - get information about random number generation
>   *
> --
> 2.30.1
>

Regards,
Simon
diff mbox series

Patch

diff --git a/drivers/rng/rng-uclass.c b/drivers/rng/rng-uclass.c
index 53108e1b31..d7236b9335 100644
--- a/drivers/rng/rng-uclass.c
+++ b/drivers/rng/rng-uclass.c
@@ -9,6 +9,22 @@ 
 #include <dm.h>
 #include <rng.h>
 
+int platform_get_rng_device(struct udevice **dev)
+{
+	int ret;
+	struct udevice *devp;
+
+	ret = uclass_first_device_check(UCLASS_RNG, &devp);
+	if (ret) {
+		pr_err("Unable to get RNG device (%d)\n", ret);
+		return -ENODEV;
+	}
+
+	*dev = devp;
+
+	return 0;
+}
+
 int dm_rng_read(struct udevice *dev, void *buffer, size_t size)
 {
 	const struct dm_rng_ops *ops = device_get_ops(dev);
diff --git a/include/efi_rng.h b/include/efi_rng.h
index 3c622381cb..f22e54adb0 100644
--- a/include/efi_rng.h
+++ b/include/efi_rng.h
@@ -23,6 +23,4 @@  struct efi_rng_protocol {
 				       efi_uintn_t rng_value_length, uint8_t *rng_value);
 };
 
-efi_status_t platform_get_rng_device(struct udevice **dev);
-
 #endif /* _EFI_RNG_H_ */
diff --git a/include/rng.h b/include/rng.h
index 37af554363..255c85d3e8 100644
--- a/include/rng.h
+++ b/include/rng.h
@@ -20,6 +20,17 @@  struct udevice;
  */
 int dm_rng_read(struct udevice *dev, void *buffer, size_t size);
 
+/**
+ * platform_get_rng_device() - retrieve random number generator
+ *
+ * This function retrieves the first udevice implementing a hardware
+ * random number generator. Device is already probed.
+ *
+ * @dev:	udevice
+ * Return:	status code
+ */
+int platform_get_rng_device(struct udevice **dev);
+
 /**
  * struct dm_rng_ops - operations for the hwrng uclass
  *
diff --git a/lib/efi_loader/efi_rng.c b/lib/efi_loader/efi_rng.c
index bb11d8d0e0..0d8bf770f5 100644
--- a/lib/efi_loader/efi_rng.c
+++ b/lib/efi_loader/efi_rng.c
@@ -17,33 +17,6 @@  DECLARE_GLOBAL_DATA_PTR;
 
 const efi_guid_t efi_guid_rng_protocol = EFI_RNG_PROTOCOL_GUID;
 
-/**
- * platform_get_rng_device() - retrieve random number generator
- *
- * This function retrieves the udevice implementing a hardware random
- * number generator.
- *
- * This function may be overridden if special initialization is needed.
- *
- * @dev:	udevice
- * Return:	status code
- */
-__weak efi_status_t platform_get_rng_device(struct udevice **dev)
-{
-	int ret;
-	struct udevice *devp;
-
-	ret = uclass_get_device(UCLASS_RNG, 0, &devp);
-	if (ret) {
-		debug("Unable to get rng device\n");
-		return EFI_DEVICE_ERROR;
-	}
-
-	*dev = devp;
-
-	return EFI_SUCCESS;
-}
-
 /**
  * rng_getinfo() - get information about random number generation
  *