On Mon, Sep 4, 2023 at 3:29 AM Andy Shevchenko
<[email protected]> wrote:
>
> On Mon, Sep 04, 2023 at 12:12:44PM +0200, Bartosz Golaszewski wrote:
> > On Mon, Sep 4, 2023 at 12:05 PM Andy Shevchenko
> > <[email protected]> wrote:
> > > On Mon, Sep 04, 2023 at 11:47:54AM +0200, Bartosz Golaszewski wrote:
> > > > On Mon, Sep 4, 2023 at 11:40 AM Andy Shevchenko
> > > > <[email protected]> wrote:
> > > > > On Mon, Sep 04, 2023 at 11:22:32AM +0200, Bartosz Golaszewski wrote:
> > > > > > On Mon, Sep 4, 2023 at 10:59 AM Andy Shevchenko
> > > > > > <[email protected]> wrote:
> > > > > > > On Sat, Sep 02, 2023 at 04:40:05PM +0200, Bartosz Golaszewski wrote:
> > > > > > > > On Fri, Sep 1, 2023 at 11:10 PM Andy Shevchenko
> > > > > > > > <[email protected]> wrote:
> > > > > > > > > On Fri, Sep 01, 2023 at 08:32:40PM +0200, Bartosz Golaszewski wrote:
>
> ...
>
> > > > > > > > > > - /* Used by sysfs and configfs callbacks. */
> > > > > > > > > > - dev_set_drvdata(&gc->gpiodev->dev, chip);
> > > > > > > > > > + /* Used by sysfs callbacks. */
> > > > > > > > > > + dev_set_drvdata(swnode->dev, chip);
> > > > > > > > >
> > > > > > > > > dev pointer of firmware node is solely for dev links. Is it the case here?
> > > > > > > > > Seems to me you luckily abuse it.
> > > > > > > >
> > > > > > > > I don't think so. If anything we have a helper in the form of
> > > > > > > > get_dev_from_fwnode() but it takes reference to the device while we
> > > > > > > > don't need it - we know it'll be there because we created it.
> > > > > > > >
> > > > > > > > This information (struct device of the GPIO device) can also be
> > > > > > > > retrieved by iterating over the device children of the top platform
> > > > > > > > device and comparing their fwnodes against the one we got passed down
> > > > > > > > from probe() but it's just so many extra steps.
> > > > > > > >
> > > > > > > > Or we can have a getter in gpio/driver.h for that but I don't want to
> > > > > > > > expose another interface is we can simply use the fwnode.
> > > > > > >
Sorry for being late to the party.
> > > > > > > dev pointer in the fwnode strictly speaking is optional. No-one, except
> > > > > > > its solely user, should rely on it (its presence and lifetime).
> > > > > >
> > > > > > Where is this documented? Because just by a quick glance into
> > > > > > drivers/base/core.c I can tell that if a device has an fwnode then
> > > > > > fwnode->dev gets assigned when the device is created and cleared when
> > > > > > it's removed (note: note even attached to driver, just
> > > > > > created/removed). Seems like pretty reliable behavior to me.
> > > > >
> > > > > Yes, and even that member in fwnode is a hack in my opinion. We should not mix
> > > > > layers and the idea in the future to get rid of the fwnode_handle to be
> > > > > _embedded_ into struct device. It should be separate entity, and device
> > > > > instance may use it as a linked list. Currently we have a few problems because
> > > > > of the this design mistake.
> > > >
> > > > I don't see how this would work if fwnodes can exist before struct
> > > > device is even created.
> > >
> > > That's whole idea behind swnodes. They (ideally) should be created _before_
> > > any other object they are being used with. This is how it works today.
> >
> > Yes, this is what I meant: if fwnodes can be created before struct
> > device (as it is now) and their life-time is separated then how could
> > you possibly make the fwnode part of struct device?
> >
> > > And doing swnode->dev = ... contradicts a lot: layering, lifetime objects, etc.
I understand what you are trying to say about layering, but there are
no lifetime violations here.
> >
> > No it doesn't. We have the software node - the template for the
> > device. It can only be populated with a single device entry.
>
> Which is wrong assumption. Software nodes (and firmware nodes) in general
> can be shared. Which device pointer you want to add there?
I don't think this is any harder to handle than how a device's
secondary fwnode is handled in set_primary_fwnode(). For secondary
fwnodes, you just WARN and overwrite it and move on.
> Which one
> should be next when one of the devices is gone?
Similar to how set_primary_fwnode() handles deletion (NULL), you can
handle the same for when a device is removed. You can check the parent
or the bus for another device with the same fwnode and set it.
> No, simply no. Do not use it!
Using fwnode_handle->dev is no different than searching a bus for a
device which has dev->fwnode match the fwnode you are looking for.
In both cases, you are just going to get the first device that was
added. It's completely pointless to force searching a bus to find the
device with a specific fwnode.
In the special cases where one fwnode has multiple devices, no generic
code is going to always handle the device search correctly. The
framework adding those devices probably knows what's the right thing
to do based on which of the N devices with the same fwnode they are
trying to find.
I understand it's not great, but blindly saying "search the bus" isn't
really improving anything here and just makes things unnecessarily
inefficient.
-Saravana
>
> > Once it's done, I don't see why you wouldn't want to assign this device to
> > its corresponding software node. Provided locking is in place etc.
> >
> > > > They - after all - represent the actual
> > > > physical device hierarchy which may or may not be populated at
> > > > run-time depending on many factors.
> > >
> > > No. This is a mistaken assumption.
> >
> > How so?
>
> See above.
>
> > > > Once populated, being able to retrieve the software representation of
> > > > the device (struct device) from the node from which it was populated
> > > > sounds like a reasonable thing to do. What are those problems and are
> > > > they even linked to this issue?
> > > >
> > > > > The get_dev_from_fwnode() is used only in devlink and I want to keep it that way.
> > > > > Nobody else should use it, really.
> > > >
> > > > I don't care all that much, I can get the device from the children of
> > > > the platform device. Still comparing fwnodes, though this time the
> > > > other way around.
> > >
> > > Fine, but do not use dev pointer from fwnode, esp. software node.
> >
> > I will do it but I'd like to clarify the above at some point.
>
> The relationship between device instance(s) and firmware node instance(s)
> is m:n, where each of them can be from 0 to ... x or y.
>
> There is no unique mapping between two.
On Tue, Feb 20, 2024 at 05:46:27PM -0800, Saravana Kannan wrote:
> On Mon, Sep 4, 2023 at 3:29 AM Andy Shevchenko
> <[email protected]> wrote:
> > On Mon, Sep 04, 2023 at 12:12:44PM +0200, Bartosz Golaszewski wrote:
> > > On Mon, Sep 4, 2023 at 12:05 PM Andy Shevchenko
> > > <[email protected]> wrote:
> > > > On Mon, Sep 04, 2023 at 11:47:54AM +0200, Bartosz Golaszewski wrote:
> > > > > On Mon, Sep 4, 2023 at 11:40 AM Andy Shevchenko
> > > > > <[email protected]> wrote:
> > > > > > On Mon, Sep 04, 2023 at 11:22:32AM +0200, Bartosz Golaszewski wrote:
> > > > > > > On Mon, Sep 4, 2023 at 10:59 AM Andy Shevchenko
> > > > > > > <[email protected]> wrote:
> > > > > > > > On Sat, Sep 02, 2023 at 04:40:05PM +0200, Bartosz Golaszewski wrote:
> > > > > > > > > On Fri, Sep 1, 2023 at 11:10 PM Andy Shevchenko
> > > > > > > > > <[email protected]> wrote:
> > > > > > > > > > On Fri, Sep 01, 2023 at 08:32:40PM +0200, Bartosz Golaszewski wrote:
..
> > > > > > > > > > > - /* Used by sysfs and configfs callbacks. */
> > > > > > > > > > > - dev_set_drvdata(&gc->gpiodev->dev, chip);
> > > > > > > > > > > + /* Used by sysfs callbacks. */
> > > > > > > > > > > + dev_set_drvdata(swnode->dev, chip);
> > > > > > > > > >
> > > > > > > > > > dev pointer of firmware node is solely for dev links. Is it the case here?
> > > > > > > > > > Seems to me you luckily abuse it.
> > > > > > > > >
> > > > > > > > > I don't think so. If anything we have a helper in the form of
> > > > > > > > > get_dev_from_fwnode() but it takes reference to the device while we
> > > > > > > > > don't need it - we know it'll be there because we created it.
> > > > > > > > >
> > > > > > > > > This information (struct device of the GPIO device) can also be
> > > > > > > > > retrieved by iterating over the device children of the top platform
> > > > > > > > > device and comparing their fwnodes against the one we got passed down
> > > > > > > > > from probe() but it's just so many extra steps.
> > > > > > > > >
> > > > > > > > > Or we can have a getter in gpio/driver.h for that but I don't want to
> > > > > > > > > expose another interface is we can simply use the fwnode.
> > > > > > > >
>
> Sorry for being late to the party.
You decided to make a blast from the past due to the last patches from me? :-)
> > > > > > > > dev pointer in the fwnode strictly speaking is optional. No-one, except
> > > > > > > > its solely user, should rely on it (its presence and lifetime).
> > > > > > >
> > > > > > > Where is this documented? Because just by a quick glance into
> > > > > > > drivers/base/core.c I can tell that if a device has an fwnode then
> > > > > > > fwnode->dev gets assigned when the device is created and cleared when
> > > > > > > it's removed (note: note even attached to driver, just
> > > > > > > created/removed). Seems like pretty reliable behavior to me.
> > > > > >
> > > > > > Yes, and even that member in fwnode is a hack in my opinion. We should not mix
> > > > > > layers and the idea in the future to get rid of the fwnode_handle to be
> > > > > > _embedded_ into struct device. It should be separate entity, and device
> > > > > > instance may use it as a linked list. Currently we have a few problems because
> > > > > > of the this design mistake.
> > > > >
> > > > > I don't see how this would work if fwnodes can exist before struct
> > > > > device is even created.
> > > >
> > > > That's whole idea behind swnodes. They (ideally) should be created _before_
> > > > any other object they are being used with. This is how it works today.
> > >
> > > Yes, this is what I meant: if fwnodes can be created before struct
> > > device (as it is now) and their life-time is separated then how could
> > > you possibly make the fwnode part of struct device?
> > >
> > > > And doing swnode->dev = ... contradicts a lot: layering, lifetime objects, etc.
>
> I understand what you are trying to say about layering, but there are
> no lifetime violations here.
There is. Software node is not firmware node, their lifetime is the same or
wider than the respective device (often, they are statically defined without
any device in mind).
> > > No it doesn't. We have the software node - the template for the
> > > device. It can only be populated with a single device entry.
> >
> > Which is wrong assumption. Software nodes (and firmware nodes) in general
> > can be shared. Which device pointer you want to add there?
>
> I don't think this is any harder to handle than how a device's
> secondary fwnode is handled in set_primary_fwnode(). For secondary
> fwnodes, you just WARN and overwrite it and move on.
The whole concept of a single linked list with limitation to up to two
nodes and being the part of the struct fwnode_handle itself appears to
be problematic. We have a lot of tricks here and there instead of properly
having a list head in the struct device without any limitations in number
of nodes with a priority based on the appearance in the list.
For the details you may ask USB DWC3 developers and related to that.
> > Which one should be next when one of the devices is gone?
>
> Similar to how set_primary_fwnode() handles deletion (NULL), you can
> handle the same for when a device is removed. You can check the parent
> or the bus for another device with the same fwnode and set it.
> > No, simply no. Do not use it!
>
> Using fwnode_handle->dev is no different than searching a bus for a
> device which has dev->fwnode match the fwnode you are looking for.
>
> In both cases, you are just going to get the first device that was
> added. It's completely pointless to force searching a bus to find the
> device with a specific fwnode.
>
> In the special cases where one fwnode has multiple devices, no generic
> code is going to always handle the device search correctly. The
> framework adding those devices probably knows what's the right thing
> to do based on which of the N devices with the same fwnode they are
> trying to find.
>
> I understand it's not great, but blindly saying "search the bus" isn't
> really improving anything here and just makes things unnecessarily
> inefficient.
Is there any _good_ documentation for devlinks and all that fields in the
struct fwnode? Why should we use that without any understanding of the
purposes of that field. We, as device property developers, hadn't introduced
that field and never required it. It's an alien to device properties APIs.
--
With Best Regards,
Andy Shevchenko
On Wed, Feb 21, 2024 at 2:47 AM Saravana Kannan <[email protected]> wrote:
>
> On Mon, Sep 4, 2023 at 3:29 AM Andy Shevchenko
> <[email protected]> wrote:
> >
> > On Mon, Sep 04, 2023 at 12:12:44PM +0200, Bartosz Golaszewski wrote:
> > > On Mon, Sep 4, 2023 at 12:05 PM Andy Shevchenko
> > > <[email protected]> wrote:
> > > > On Mon, Sep 04, 2023 at 11:47:54AM +0200, Bartosz Golaszewski wrote:
> > > > > On Mon, Sep 4, 2023 at 11:40 AM Andy Shevchenko
> > > > > <[email protected]> wrote:
> > > > > > On Mon, Sep 04, 2023 at 11:22:32AM +0200, Bartosz Golaszewski wrote:
> > > > > > > On Mon, Sep 4, 2023 at 10:59 AM Andy Shevchenko
> > > > > > > <[email protected]> wrote:
> > > > > > > > On Sat, Sep 02, 2023 at 04:40:05PM +0200, Bartosz Golaszewski wrote:
> > > > > > > > > On Fri, Sep 1, 2023 at 11:10 PM Andy Shevchenko
> > > > > > > > > <[email protected]> wrote:
> > > > > > > > > > On Fri, Sep 01, 2023 at 08:32:40PM +0200, Bartosz Golaszewski wrote:
> >
> > ...
> >
> > > > > > > > > > > - /* Used by sysfs and configfs callbacks. */
> > > > > > > > > > > - dev_set_drvdata(&gc->gpiodev->dev, chip);
> > > > > > > > > > > + /* Used by sysfs callbacks. */
> > > > > > > > > > > + dev_set_drvdata(swnode->dev, chip);
> > > > > > > > > >
> > > > > > > > > > dev pointer of firmware node is solely for dev links. Is it the case here?
> > > > > > > > > > Seems to me you luckily abuse it.
> > > > > > > > >
> > > > > > > > > I don't think so. If anything we have a helper in the form of
> > > > > > > > > get_dev_from_fwnode() but it takes reference to the device while we
> > > > > > > > > don't need it - we know it'll be there because we created it.
> > > > > > > > >
> > > > > > > > > This information (struct device of the GPIO device) can also be
> > > > > > > > > retrieved by iterating over the device children of the top platform
> > > > > > > > > device and comparing their fwnodes against the one we got passed down
> > > > > > > > > from probe() but it's just so many extra steps.
> > > > > > > > >
> > > > > > > > > Or we can have a getter in gpio/driver.h for that but I don't want to
> > > > > > > > > expose another interface is we can simply use the fwnode.
> > > > > > > >
>
> Sorry for being late to the party.
>
> > > > > > > > dev pointer in the fwnode strictly speaking is optional. No-one, except
> > > > > > > > its solely user, should rely on it (its presence and lifetime).
> > > > > > >
> > > > > > > Where is this documented? Because just by a quick glance into
> > > > > > > drivers/base/core.c I can tell that if a device has an fwnode then
> > > > > > > fwnode->dev gets assigned when the device is created and cleared when
> > > > > > > it's removed (note: note even attached to driver, just
> > > > > > > created/removed). Seems like pretty reliable behavior to me.
> > > > > >
> > > > > > Yes, and even that member in fwnode is a hack in my opinion. We should not mix
> > > > > > layers and the idea in the future to get rid of the fwnode_handle to be
> > > > > > _embedded_ into struct device. It should be separate entity, and device
> > > > > > instance may use it as a linked list. Currently we have a few problems because
> > > > > > of the this design mistake.
> > > > >
> > > > > I don't see how this would work if fwnodes can exist before struct
> > > > > device is even created.
> > > >
> > > > That's whole idea behind swnodes. They (ideally) should be created _before_
> > > > any other object they are being used with. This is how it works today.
> > >
> > > Yes, this is what I meant: if fwnodes can be created before struct
> > > device (as it is now) and their life-time is separated then how could
> > > you possibly make the fwnode part of struct device?
> > >
> > > > And doing swnode->dev = ... contradicts a lot: layering, lifetime objects, etc.
>
> I understand what you are trying to say about layering, but there are
> no lifetime violations here.
>
> > >
> > > No it doesn't. We have the software node - the template for the
> > > device. It can only be populated with a single device entry.
> >
> > Which is wrong assumption. Software nodes (and firmware nodes) in general
> > can be shared. Which device pointer you want to add there?
>
> I don't think this is any harder to handle than how a device's
> secondary fwnode is handled in set_primary_fwnode(). For secondary
> fwnodes, you just WARN and overwrite it and move on.
>
> > Which one
> > should be next when one of the devices is gone?
>
> Similar to how set_primary_fwnode() handles deletion (NULL), you can
> handle the same for when a device is removed. You can check the parent
> or the bus for another device with the same fwnode and set it.
>
> > No, simply no. Do not use it!
>
> Using fwnode_handle->dev is no different than searching a bus for a
> device which has dev->fwnode match the fwnode you are looking for.
>
> In both cases, you are just going to get the first device that was
> added. It's completely pointless to force searching a bus to find the
> device with a specific fwnode.
>
> In the special cases where one fwnode has multiple devices, no generic
> code is going to always handle the device search correctly. The
> framework adding those devices probably knows what's the right thing
> to do based on which of the N devices with the same fwnode they are
> trying to find.
>
> I understand it's not great, but blindly saying "search the bus" isn't
> really improving anything here and just makes things unnecessarily
> inefficient.
>
> -Saravana
Thanks for the input. I've since moved to using device_find_child()
but will keep it in mind for the future.
Bart
>
> >
> > > Once it's done, I don't see why you wouldn't want to assign this device to
> > > its corresponding software node. Provided locking is in place etc.
> > >
> > > > > They - after all - represent the actual
> > > > > physical device hierarchy which may or may not be populated at
> > > > > run-time depending on many factors.
> > > >
> > > > No. This is a mistaken assumption.
> > >
> > > How so?
> >
> > See above.
> >
> > > > > Once populated, being able to retrieve the software representation of
> > > > > the device (struct device) from the node from which it was populated
> > > > > sounds like a reasonable thing to do. What are those problems and are
> > > > > they even linked to this issue?
> > > > >
> > > > > > The get_dev_from_fwnode() is used only in devlink and I want to keep it that way.
> > > > > > Nobody else should use it, really.
> > > > >
> > > > > I don't care all that much, I can get the device from the children of
> > > > > the platform device. Still comparing fwnodes, though this time the
> > > > > other way around.
> > > >
> > > > Fine, but do not use dev pointer from fwnode, esp. software node.
> > >
> > > I will do it but I'd like to clarify the above at some point.
> >
> > The relationship between device instance(s) and firmware node instance(s)
> > is m:n, where each of them can be from 0 to ... x or y.
> >
> > There is no unique mapping between two.
On Wed, Feb 21, 2024 at 4:59 AM Andy Shevchenko
<[email protected]> wrote:
>
> On Tue, Feb 20, 2024 at 05:46:27PM -0800, Saravana Kannan wrote:
> > On Mon, Sep 4, 2023 at 3:29 AM Andy Shevchenko
> > <[email protected]> wrote:
> > > On Mon, Sep 04, 2023 at 12:12:44PM +0200, Bartosz Golaszewski wrote:
> > > > On Mon, Sep 4, 2023 at 12:05 PM Andy Shevchenko
> > > > <[email protected]> wrote:
> > > > > On Mon, Sep 04, 2023 at 11:47:54AM +0200, Bartosz Golaszewski wrote:
> > > > > > On Mon, Sep 4, 2023 at 11:40 AM Andy Shevchenko
> > > > > > <[email protected]> wrote:
> > > > > > > On Mon, Sep 04, 2023 at 11:22:32AM +0200, Bartosz Golaszewski wrote:
> > > > > > > > On Mon, Sep 4, 2023 at 10:59 AM Andy Shevchenko
> > > > > > > > <[email protected]> wrote:
> > > > > > > > > On Sat, Sep 02, 2023 at 04:40:05PM +0200, Bartosz Golaszewski wrote:
> > > > > > > > > > On Fri, Sep 1, 2023 at 11:10 PM Andy Shevchenko
> > > > > > > > > > <[email protected]> wrote:
> > > > > > > > > > > On Fri, Sep 01, 2023 at 08:32:40PM +0200, Bartosz Golaszewski wrote:
>
> ...
>
> > > > > > > > > > > > - /* Used by sysfs and configfs callbacks. */
> > > > > > > > > > > > - dev_set_drvdata(&gc->gpiodev->dev, chip);
> > > > > > > > > > > > + /* Used by sysfs callbacks. */
> > > > > > > > > > > > + dev_set_drvdata(swnode->dev, chip);
> > > > > > > > > > >
> > > > > > > > > > > dev pointer of firmware node is solely for dev links. Is it the case here?
> > > > > > > > > > > Seems to me you luckily abuse it.
> > > > > > > > > >
> > > > > > > > > > I don't think so. If anything we have a helper in the form of
> > > > > > > > > > get_dev_from_fwnode() but it takes reference to the device while we
> > > > > > > > > > don't need it - we know it'll be there because we created it.
> > > > > > > > > >
> > > > > > > > > > This information (struct device of the GPIO device) can also be
> > > > > > > > > > retrieved by iterating over the device children of the top platform
> > > > > > > > > > device and comparing their fwnodes against the one we got passed down
> > > > > > > > > > from probe() but it's just so many extra steps.
> > > > > > > > > >
> > > > > > > > > > Or we can have a getter in gpio/driver.h for that but I don't want to
> > > > > > > > > > expose another interface is we can simply use the fwnode.
> > > > > > > > >
> >
> > Sorry for being late to the party.
>
> You decided to make a blast from the past due to the last patches from me? :-)
Yeah :) I meant to reply to this when you sent it, but was swamped and
forgot about it.
>
> > > > > > > > > dev pointer in the fwnode strictly speaking is optional. No-one, except
> > > > > > > > > its solely user, should rely on it (its presence and lifetime).
> > > > > > > >
> > > > > > > > Where is this documented? Because just by a quick glance into
> > > > > > > > drivers/base/core.c I can tell that if a device has an fwnode then
> > > > > > > > fwnode->dev gets assigned when the device is created and cleared when
> > > > > > > > it's removed (note: note even attached to driver, just
> > > > > > > > created/removed). Seems like pretty reliable behavior to me.
> > > > > > >
> > > > > > > Yes, and even that member in fwnode is a hack in my opinion. We should not mix
> > > > > > > layers and the idea in the future to get rid of the fwnode_handle to be
> > > > > > > _embedded_ into struct device. It should be separate entity, and device
> > > > > > > instance may use it as a linked list. Currently we have a few problems because
> > > > > > > of the this design mistake.
> > > > > >
> > > > > > I don't see how this would work if fwnodes can exist before struct
> > > > > > device is even created.
> > > > >
> > > > > That's whole idea behind swnodes. They (ideally) should be created _before_
> > > > > any other object they are being used with. This is how it works today.
> > > >
> > > > Yes, this is what I meant: if fwnodes can be created before struct
> > > > device (as it is now) and their life-time is separated then how could
> > > > you possibly make the fwnode part of struct device?
> > > >
> > > > > And doing swnode->dev = ... contradicts a lot: layering, lifetime objects, etc.
> >
> > I understand what you are trying to say about layering, but there are
> > no lifetime violations here.
>
> There is. Software node is not firmware node, their lifetime is the same or
> wider than the respective device (often, they are statically defined without
> any device in mind).
>
> > > > No it doesn't. We have the software node - the template for the
> > > > device. It can only be populated with a single device entry.
> > >
> > > Which is wrong assumption. Software nodes (and firmware nodes) in general
> > > can be shared. Which device pointer you want to add there?
> >
> > I don't think this is any harder to handle than how a device's
> > secondary fwnode is handled in set_primary_fwnode(). For secondary
> > fwnodes, you just WARN and overwrite it and move on.
>
> The whole concept of a single linked list with limitation to up to two
> nodes and being the part of the struct fwnode_handle itself appears to
> be problematic. We have a lot of tricks here and there instead of properly
> having a list head in the struct device without any limitations in number
> of nodes with a priority based on the appearance in the list.
>
> For the details you may ask USB DWC3 developers and related to that.
>
> > > Which one should be next when one of the devices is gone?
> >
> > Similar to how set_primary_fwnode() handles deletion (NULL), you can
> > handle the same for when a device is removed. You can check the parent
> > or the bus for another device with the same fwnode and set it.
>
> > > No, simply no. Do not use it!
> >
> > Using fwnode_handle->dev is no different than searching a bus for a
> > device which has dev->fwnode match the fwnode you are looking for.
> >
> > In both cases, you are just going to get the first device that was
> > added. It's completely pointless to force searching a bus to find the
> > device with a specific fwnode.
> >
> > In the special cases where one fwnode has multiple devices, no generic
> > code is going to always handle the device search correctly. The
> > framework adding those devices probably knows what's the right thing
> > to do based on which of the N devices with the same fwnode they are
> > trying to find.
> >
> > I understand it's not great, but blindly saying "search the bus" isn't
> > really improving anything here and just makes things unnecessarily
> > inefficient.
>
> Is there any _good_ documentation for devlinks and all that fields in the
> struct fwnode? Why should we use that without any understanding of the
> purposes of that field. We, as device property developers, hadn't introduced
> that field and never required it. It's an alien to device properties APIs.
If I add some inline documentation for these fields, will you be more
open to letting people use this as a way to look up devices? I'm happy
to do that for you.
Thanks,
Saravana
On Thu, Feb 22, 2024 at 05:01:04PM -0800, Saravana Kannan wrote:
> On Wed, Feb 21, 2024 at 4:59 AM Andy Shevchenko
> <[email protected]> wrote:
> > On Tue, Feb 20, 2024 at 05:46:27PM -0800, Saravana Kannan wrote:
..
> > Is there any _good_ documentation for devlinks and all that fields in the
> > struct fwnode? Why should we use that without any understanding of the
> > purposes of that field. We, as device property developers, hadn't introduced
> > that field and never required it. It's an alien to device properties APIs.
>
> If I add some inline documentation for these fields, will you be more
> open to letting people use this as a way to look up devices? I'm happy
> to do that for you.
I consider documentation patches to be always welcome. But it doesn't mean
we allow to use that fields in device property APIs without a very good
justification.
--
With Best Regards,
Andy Shevchenko