Hi Simon,
Thanks for taking the time to check for my comments
On 4/3/20 20:11, Simon Glass wrote:
Hi Walter,
On Wed, 4 Mar 2020 at 12:40, Walter Lozano <[email protected]> wrote:
When OF_PLATDATA is enabled DT information is parsed and platdata
structures are populated. In this context the links between DT nodes are
represented as pointers to platdata structures, and there is no clear way
to access to the device which owns the structure.
This patch implements a set of functions:
- device_find_by_platdata
- uclass_find_device_by_platdata
to access to the device.
Signed-off-by: Walter Lozano <[email protected]>
---
drivers/core/device.c | 19 +++++++++++++++++++
drivers/core/uclass.c | 34 ++++++++++++++++++++++++++++++++++
include/dm/device.h | 2 ++
include/dm/uclass-internal.h | 3 +++
include/dm/uclass.h | 2 ++
5 files changed, 60 insertions(+)
This is interesting. Could you also add the motivation for this? It's
not clear to me who would call this function.
I have been reviewing the OF_PLATDATA support as an R&D project, in this
context, in order to have
a better understanding on the possibilities and limitations I decided to add
its support to iMX6,
more particularly to the MMC drivers. The link issue arises when I tried to
setup the GPIO for
Card Detection, which is trivial when DT is available. However, when
OF_PLATDATA is enabled
this seems, at least for me, not straightforward.
In order to overcome this limitation I think that having a set of functions to
find/get devices
based on platdata could be useful. Of course, there might be a better
approach/idea, so that is
the motivation for this RFC.
An example of the usage could be
#if CONFIG_IS_ENABLED(DM_GPIO)
struct udevice *gpiodev;
ret = uclass_get_device_by_platdata(UCLASS_GPIO, (void
*)dtplat->cd_gpios->node, &gpiodev);
if (ret)
return ret;
ret = gpio_dev_request_index(gpiodev, gpiodev->name, "cd-gpios",
dtplat->cd_gpios->arg[0], GPIOD_IS_IN,
dtplat->cd_gpios->arg[1], &priv->cd_gpio);
if (ret)
return ret;
#endif
This is part of my current work, a series of patches to add OF_PLATDATA support
as explained.
Does this make sense to you?
Also it relates to another thing I've been thinking about for a while,
which is to validate that all the structs pointed to are correct.
E.g. if every struct had a magic number like:
struct tpm_platdata {
DM_STRUCT(UCLASS_TPM, DM_STRUCT_PLATDATA, ...)
fields here
};
then we could check the structure pointers are correct.
DM_STRUCT() would define to nothing if we were not building with
CONFIG_DM_DEBUG or similar.
Interesting, I think it could be useful and save us from headaches while
debugging.
Thanks for sharing this idea.
Anyway, I wonder whether you could expand your definition a bit so you
have an enum for the different types of struct you can request:
enum dm_struct_t {
DM_STRUCT_PLATDATA,
...
DM_STRUCT_COUNT,
};
and modify the function so it can request it via the enum?
Let me check if I understand correctly, your suggestion is to do
something like diff --git a/include/dm/uclass.h b/include/dm/uclass.h
index 92c07f8426..bf09dadf3f 100644 --- a/include/dm/uclass.h +++
b/include/dm/uclass.h
@@ -167,8 +167,8 @@ int uclass_get_device(enum uclass_id id, int index,
struct udevice **devp);
int uclass_get_device_by_name(enum uclass_id id, const char *name,
struct udevice **devp); -int
uclass_get_device_by_platdata(enum uclass_id id, void *platdata,
- struct udevice **devp);
+int uclass_get_device_by_struct(enum uclass_id id, enum dm_struct_t
struct_id, + void *struct_pointer, struct
udevice **devp); /** * uclass_get_device_by_seq() - Get a uclass
device based on an ID and sequence *
If that is the case, I would be happy to help.
Also, if my understanding is correct, could you elaborate which cases
are you trying to cover with this approach? Regards,
Walter
