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

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