2023-02-23 20:37:26

by Vladimir Oltean

[permalink] [raw]
Subject: Implementation of fwnode_operations :: device_get_match_data() for software nodes?

Hi,

I have a need to instantiate a driver written for OF which calls
device_get_match_data(dev) to get various information based on the
compatible string.

I am creating a software node based on the following properties:

struct property_entry props[2] = {
PROPERTY_ENTRY_STRING("compatible", compatible),
{},
};

(I see I'm not the only one doing this, some drivers/platform/x86/x86-android-tablets.c
and drivers/platform/chrome/chromeos_laptop.c also do it)

and the driver in question does begin to probe, but its match_data is
NULL, because the operation from the title isn't implemented for
software nodes. So probing ultimately fails.

Is there some sort or reason why this doesn't exist, other than a lack
of need?

Can someone please help me with an implementation of this feature?

Thanks,
Vladimir


2023-02-27 12:19:16

by Heikki Krogerus

[permalink] [raw]
Subject: Re: Implementation of fwnode_operations :: device_get_match_data() for software nodes?

Hi Vladimir,

On Thu, Feb 23, 2023 at 10:37:13PM +0200, Vladimir Oltean wrote:
> Hi,
>
> I have a need to instantiate a driver written for OF which calls
> device_get_match_data(dev) to get various information based on the
> compatible string.
>
> I am creating a software node based on the following properties:
>
> struct property_entry props[2] = {
> PROPERTY_ENTRY_STRING("compatible", compatible),
> {},
> };
>
> (I see I'm not the only one doing this, some drivers/platform/x86/x86-android-tablets.c
> and drivers/platform/chrome/chromeos_laptop.c also do it)
>
> and the driver in question does begin to probe, but its match_data is
> NULL, because the operation from the title isn't implemented for
> software nodes. So probing ultimately fails.
>
> Is there some sort or reason why this doesn't exist, other than a lack
> of need?

There has not been any need for it before.

> Can someone please help me with an implementation of this feature?

Try this - I'm sorry, I don't know does it actually work:

diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index 1886995a0b3a3..5262b49c7c790 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -9,6 +9,7 @@
#include <linux/device.h>
#include <linux/kernel.h>
#include <linux/property.h>
+#include <linux/mod_devicetable.h>
#include <linux/slab.h>

#include "base.h"
@@ -379,6 +380,25 @@ static void software_node_put(struct fwnode_handle *fwnode)
kobject_put(&swnode->kobj);
}

+static const void *
+software_node_get_match_data(const struct fwnode_handle *fwnode, const struct device *dev)
+{
+ const struct of_device_id *id;
+ const char *compat;
+
+ if (!dev->driver || !dev->driver->of_match_table)
+ return NULL;
+
+ if (fwnode_property_read_string(fwnode, "compatible", &compat))
+ return NULL;
+
+ for (id = dev->driver->of_match_table; id->compatible[0]; id++)
+ if (!strcmp(compat, id->compatible))
+ return id->data;
+
+ return NULL;
+}
+
static bool software_node_property_present(const struct fwnode_handle *fwnode,
const char *propname)
{
@@ -662,6 +682,7 @@ software_node_graph_parse_endpoint(const struct fwnode_handle *fwnode,
static const struct fwnode_operations software_node_ops = {
.get = software_node_get,
.put = software_node_put,
+ .device_get_match_data = software_node_get_match_data,
.property_present = software_node_property_present,
.property_read_int_array = software_node_read_int_array,
.property_read_string_array = software_node_read_string_array,

--
heikki

2023-02-27 22:27:04

by Andy Shevchenko

[permalink] [raw]
Subject: Re: Implementation of fwnode_operations :: device_get_match_data() for software nodes?

On Thu, Feb 23, 2023 at 10:37:13PM +0200, Vladimir Oltean wrote:
> Hi,
>
> I have a need to instantiate a driver written for OF which calls
> device_get_match_data(dev) to get various information based on the
> compatible string.
>
> I am creating a software node based on the following properties:
>
> struct property_entry props[2] = {
> PROPERTY_ENTRY_STRING("compatible", compatible),
> {},
> };
>
> (I see I'm not the only one doing this, some drivers/platform/x86/x86-android-tablets.c
> and drivers/platform/chrome/chromeos_laptop.c also do it)
>
> and the driver in question does begin to probe, but its match_data is
> NULL, because the operation from the title isn't implemented for
> software nodes. So probing ultimately fails.
>
> Is there some sort or reason why this doesn't exist, other than a lack
> of need?
>
> Can someone please help me with an implementation of this feature?

I believe that there are few reasons for that:
1) (besides that what Heikki mentioned);
2) the software nodes only for quirks, seems you are trying to implement
something that should have to be implemented as proper DT / ACPI device node.

Can you elaborate why do you need that (as you see no other board file requires
this)?

--
With Best Regards,
Andy Shevchenko



2023-02-27 23:08:00

by Vladimir Oltean

[permalink] [raw]
Subject: Re: Implementation of fwnode_operations :: device_get_match_data() for software nodes?

Hi Heikki,

On Mon, Feb 27, 2023 at 02:18:59PM +0200, Heikki Krogerus wrote:
> Try this - I'm sorry, I don't know does it actually work:

Thanks for the patch. I'm not able to tell you right now if it works or
not, because my PowerPC board on which I needed this started developing
this issue in U-Boot over the weekend:

DRAM: Initializing....using SPD
Detected UDIMM i-DIMM
Waiting for D_INIT timeout. Memory may not work.
2 GiB left unmapped

and now I'm in the process of resoldering the DDR slot, process of which
I've had enough for today and I'll continue tomorrow.

2023-02-27 23:44:23

by Vladimir Oltean

[permalink] [raw]
Subject: Re: Implementation of fwnode_operations :: device_get_match_data() for software nodes?

Hi Andy,

On Tue, Feb 28, 2023 at 12:26:19AM +0200, Andy Shevchenko wrote:
> I believe that there are few reasons for that:
> 1) (besides that what Heikki mentioned);
> 2) the software nodes only for quirks, seems you are trying to implement
> something that should have to be implemented as proper DT / ACPI device node.
>
> Can you elaborate why do you need that (as you see no other board file requires
> this)?

Trying to keep the answer short while still answering the question.

I'm working with some hardware which is rather complex (a full SoC with
many peripherals inside) which is controlled by a larger SoC running
Linux, over SPI.

As you point out, to describe the peripherals inside the SPI-controlled
SoC would logically require writing a device tree with their register
addresses within the small SoC address space, interrupt routing, clocks,
yadda yadda.

However, this means several hundreds of lines of DT description, but
this is a SPI device. So it's not like I could toss this description in
some sort of SoC .dtsi which a board file would just include, because
this dtsi might need to be instantiated twice or more in a single board
DTS (depends on how many SPI devices there are, physically), and there
isn't a really good way to parameterize what would be a huge macro
(C preprocessor) essentially.

This, plus that 90% of that device tree description wouldn't tell the
driver something it couldn't know already (nothing board-specific about
this information). I'm not a fan of huge device tree descriptions where
driver-level knowledge would do just fine. That SoC is currently
supported by Linux using some bindings like this (simplifying, of course.
There are some board-specific properties inside this node, which I've omitted):

&spi {
ethernet-switch@0 {
reg = <0>; // chip select
compatible = "compatible";
};

ethernet-switch@1 {
reg = <1>; // chip select
compatible = "compatible";
};
};

To get descriptions for all its peripherals, I'd have to describe it
like this:

&spi {
soc@0 {
reg = <0>; // chip select
compatible = "compatible";
#address-cells = <1>; // address space of the SPI device's memory map
#size-cells = <1>;

ethernet-switch@base-addr-1 {
reg = <base-address-1>;
compatible = "compatible";
};

peripheral@base-addr-2 {
reg = <base-address-2>;
compatible = "compatible";
};

some-other-peripheral@base-addr-3 {
reg = <base-address-3>;
compatible = "compatible";
};

...
};

soc@1 {
// more of the same
};
};

So random idea #1 is: device trees where "ethernet-switch" is a child of
"&spi" (first form) exist in the wild, and that's a fact. To change
those device trees to the new format would break forward compatibility,
since old kernels will not understand what to do with them (no driver
for "soc@0").

Random idea #2: even if I had the option to start fresh, there is just
too much boilerplate to put in the device tree, and I'd still go for the
minimalist bindings. Otherwise it's a pain for the end user (board
device tree author), first of all. Lots of ways to write it wrong and
only a single way to get it right. And no reason to let him do it.

With the minimalist bindings, it becomes the responsibility of the
"ethernet-switch" driver to have knowledge of the peripherals which are
present in that SoC, and instantiate dedicated (not monolithic) drivers
for them somehow, at their right base addresses. My current work in
progress is to create software nodes + mfd (in the spi device driver),
and platform device drivers for peripheral@base-addr-2,
some-other-peripheral@base-addr-3 etc, which have no backing OF node.

There are some other variations on this theme which also made me focus
on software nodes + mfd as a way to make sub-drivers of a larger
OF-based driver more modular, without changing device tree bindings.

2023-03-01 14:33:27

by Andy Shevchenko

[permalink] [raw]
Subject: Re: Implementation of fwnode_operations :: device_get_match_data() for software nodes?

On Tue, Feb 28, 2023 at 01:44:11AM +0200, Vladimir Oltean wrote:
> On Tue, Feb 28, 2023 at 12:26:19AM +0200, Andy Shevchenko wrote:
> > I believe that there are few reasons for that:
> > 1) (besides that what Heikki mentioned);
> > 2) the software nodes only for quirks, seems you are trying to implement
> > something that should have to be implemented as proper DT / ACPI device node.
> >
> > Can you elaborate why do you need that (as you see no other board file requires
> > this)?
>
> Trying to keep the answer short while still answering the question.

Thank you, this is helpful to understand what you want.

Random idea #N+1 based on what you told is: how about DT / ACPI overlays?
Random idea #N+2 is: have you considered FPGA approach?

So, as far as I got it right the device _can_ be considered as hotpluggable
blackbox with a lot of hardware onboard. This is very much reminds me FPGA
sitting behind PCIe hotplug capable interface.

What do we have now there? Can we spread the same approach for your case?

Because to me board files here looks like a hack.

P.S.
Yeah, I know that SPI is not hotpluggable bus per se. It may be that
we actually need to reboot machine after plugging in/out the device.

> I'm working with some hardware which is rather complex (a full SoC with
> many peripherals inside) which is controlled by a larger SoC running
> Linux, over SPI.
>
> As you point out, to describe the peripherals inside the SPI-controlled
> SoC would logically require writing a device tree with their register
> addresses within the small SoC address space, interrupt routing, clocks,
> yadda yadda.
>
> However, this means several hundreds of lines of DT description, but
> this is a SPI device. So it's not like I could toss this description in
> some sort of SoC .dtsi which a board file would just include, because
> this dtsi might need to be instantiated twice or more in a single board
> DTS (depends on how many SPI devices there are, physically), and there
> isn't a really good way to parameterize what would be a huge macro
> (C preprocessor) essentially.
>
> This, plus that 90% of that device tree description wouldn't tell the
> driver something it couldn't know already (nothing board-specific about
> this information). I'm not a fan of huge device tree descriptions where
> driver-level knowledge would do just fine. That SoC is currently
> supported by Linux using some bindings like this (simplifying, of course.
> There are some board-specific properties inside this node, which I've omitted):
>
> &spi {
> ethernet-switch@0 {
> reg = <0>; // chip select
> compatible = "compatible";
> };
>
> ethernet-switch@1 {
> reg = <1>; // chip select
> compatible = "compatible";
> };
> };
>
> To get descriptions for all its peripherals, I'd have to describe it
> like this:
>
> &spi {
> soc@0 {
> reg = <0>; // chip select
> compatible = "compatible";
> #address-cells = <1>; // address space of the SPI device's memory map
> #size-cells = <1>;
>
> ethernet-switch@base-addr-1 {
> reg = <base-address-1>;
> compatible = "compatible";
> };
>
> peripheral@base-addr-2 {
> reg = <base-address-2>;
> compatible = "compatible";
> };
>
> some-other-peripheral@base-addr-3 {
> reg = <base-address-3>;
> compatible = "compatible";
> };
>
> ...
> };
>
> soc@1 {
> // more of the same
> };
> };
>
> So random idea #1 is: device trees where "ethernet-switch" is a child of
> "&spi" (first form) exist in the wild, and that's a fact. To change
> those device trees to the new format would break forward compatibility,
> since old kernels will not understand what to do with them (no driver
> for "soc@0").
>
> Random idea #2: even if I had the option to start fresh, there is just
> too much boilerplate to put in the device tree, and I'd still go for the
> minimalist bindings. Otherwise it's a pain for the end user (board
> device tree author), first of all. Lots of ways to write it wrong and
> only a single way to get it right. And no reason to let him do it.
>
> With the minimalist bindings, it becomes the responsibility of the
> "ethernet-switch" driver to have knowledge of the peripherals which are
> present in that SoC, and instantiate dedicated (not monolithic) drivers
> for them somehow, at their right base addresses. My current work in
> progress is to create software nodes + mfd (in the spi device driver),
> and platform device drivers for peripheral@base-addr-2,
> some-other-peripheral@base-addr-3 etc, which have no backing OF node.
>
> There are some other variations on this theme which also made me focus
> on software nodes + mfd as a way to make sub-drivers of a larger
> OF-based driver more modular, without changing device tree bindings.

--
With Best Regards,
Andy Shevchenko



2023-03-01 14:36:37

by Vladimir Oltean

[permalink] [raw]
Subject: Re: Implementation of fwnode_operations :: device_get_match_data() for software nodes?

On Wed, Mar 01, 2023 at 04:33:16PM +0200, Andy Shevchenko wrote:
> On Tue, Feb 28, 2023 at 01:44:11AM +0200, Vladimir Oltean wrote:
> > On Tue, Feb 28, 2023 at 12:26:19AM +0200, Andy Shevchenko wrote:
> > > I believe that there are few reasons for that:
> > > 1) (besides that what Heikki mentioned);
> > > 2) the software nodes only for quirks, seems you are trying to implement
> > > something that should have to be implemented as proper DT / ACPI device node.
> > >
> > > Can you elaborate why do you need that (as you see no other board file requires
> > > this)?
> >
> > Trying to keep the answer short while still answering the question.
>
> Thank you, this is helpful to understand what you want.
>
> Random idea #N+1 based on what you told is: how about DT / ACPI overlays?
> Random idea #N+2 is: have you considered FPGA approach?
>
> So, as far as I got it right the device _can_ be considered as hotpluggable
> blackbox with a lot of hardware onboard. This is very much reminds me FPGA
> sitting behind PCIe hotplug capable interface.
>
> What do we have now there? Can we spread the same approach for your case?
>
> Because to me board files here looks like a hack.
>
> P.S.
> Yeah, I know that SPI is not hotpluggable bus per se. It may be that
> we actually need to reboot machine after plugging in/out the device.

Can you please give me some clearer references for #N+1 and #N+2?
I haven't considered either of those options and I'm not sure what that
would entail.

2023-03-01 15:10:10

by Andy Shevchenko

[permalink] [raw]
Subject: Re: Implementation of fwnode_operations :: device_get_match_data() for software nodes?

On Wed, Mar 01, 2023 at 04:36:25PM +0200, Vladimir Oltean wrote:
> On Wed, Mar 01, 2023 at 04:33:16PM +0200, Andy Shevchenko wrote:
> > On Tue, Feb 28, 2023 at 01:44:11AM +0200, Vladimir Oltean wrote:
> > > On Tue, Feb 28, 2023 at 12:26:19AM +0200, Andy Shevchenko wrote:
> > > > I believe that there are few reasons for that:
> > > > 1) (besides that what Heikki mentioned);
> > > > 2) the software nodes only for quirks, seems you are trying to implement
> > > > something that should have to be implemented as proper DT / ACPI device node.
> > > >
> > > > Can you elaborate why do you need that (as you see no other board file requires
> > > > this)?
> > >
> > > Trying to keep the answer short while still answering the question.
> >
> > Thank you, this is helpful to understand what you want.
> >
> > Random idea #N+1 based on what you told is: how about DT / ACPI overlays?
> > Random idea #N+2 is: have you considered FPGA approach?
> >
> > So, as far as I got it right the device _can_ be considered as hotpluggable
> > blackbox with a lot of hardware onboard. This is very much reminds me FPGA
> > sitting behind PCIe hotplug capable interface.
> >
> > What do we have now there? Can we spread the same approach for your case?
> >
> > Because to me board files here looks like a hack.
> >
> > P.S.
> > Yeah, I know that SPI is not hotpluggable bus per se. It may be that
> > we actually need to reboot machine after plugging in/out the device.
>
> Can you please give me some clearer references for #N+1 and #N+2?

> I haven't considered either of those options and I'm not sure what that
> would entail.

With overlays you can create the proper DT description stanza and end user's
job is to just put it somewhere and upload via precoded script or so [1].


For the second one I'm not really the expert. But either FPGA framework (if
they have anything working for this), or you also may look at Thunderbolt /
USB4 which uses similar approach while being PCIe devices. Okay, the latter
(USB4) is the PCIe topology, while FPGA is whatever behind the PCI switch.
Meaning that FPGA case from HW p.o.v. is closer to your case.

[1]:https://docs.kernel.org/devicetree/overlay-notes.html

--
With Best Regards,
Andy Shevchenko



2023-03-01 15:25:43

by Vladimir Oltean

[permalink] [raw]
Subject: Re: Implementation of fwnode_operations :: device_get_match_data() for software nodes?

On Wed, Mar 01, 2023 at 05:09:41PM +0200, Andy Shevchenko wrote:
> With overlays you can create the proper DT description stanza and end user's
> job is to just put it somewhere and upload via precoded script or so [1].
>
> [1]:https://docs.kernel.org/devicetree/overlay-notes.html

Ah, okay, no, that's already a no-go, since existing device tree blobs
aren't compiled with the dtc "-@" flag which would generate the __symbols__
node necessary for DT overlays to be applied over them.

That, and it's clunky and uncalled for in general, both from my
perspective as a driver developer and that of a random user, if a driver
would just start requiring device tree overlays for more functionality.
Overlays address none of the complaints I had with large DT bindings
being large in general. They are still equally large, but now, they are
also spread into multiple files.

> For the second one I'm not really the expert. But either FPGA framework (if
> they have anything working for this), or you also may look at Thunderbolt /
> USB4 which uses similar approach while being PCIe devices. Okay, the latter
> (USB4) is the PCIe topology, while FPGA is whatever behind the PCI switch.
> Meaning that FPGA case from HW p.o.v. is closer to your case.

A quick glance at Documentation/driver-api/fpga/ shows that it is a
framework for dealing with reprogrammable hardware, and has infra to
reprogram it. My hardware is fixed-function and doesn't need any of that.

Are you suggesting that I should look at reusing some common infra with
the fpga subsystem instead? A quick grep for device_add in drivers/fpga/
shows a bunch of open-coded device_add() and platform_device_add() calls.
Is this what you wanted me to see or is there something else?

2023-03-01 15:34:56

by Andy Shevchenko

[permalink] [raw]
Subject: Re: Implementation of fwnode_operations :: device_get_match_data() for software nodes?

On Wed, Mar 01, 2023 at 05:25:27PM +0200, Vladimir Oltean wrote:
> On Wed, Mar 01, 2023 at 05:09:41PM +0200, Andy Shevchenko wrote:
> > With overlays you can create the proper DT description stanza and end user's
> > job is to just put it somewhere and upload via precoded script or so [1].
> >
> > [1]:https://docs.kernel.org/devicetree/overlay-notes.html
>
> Ah, okay, no, that's already a no-go, since existing device tree blobs
> aren't compiled with the dtc "-@" flag which would generate the __symbols__
> node necessary for DT overlays to be applied over them.
>
> That, and it's clunky and uncalled for in general, both from my
> perspective as a driver developer and that of a random user, if a driver
> would just start requiring device tree overlays for more functionality.
> Overlays address none of the complaints I had with large DT bindings
> being large in general. They are still equally large, but now, they are
> also spread into multiple files.

But isn't it what you would like to have working for your case?

Even taking into account the fixed HW layout, it would make sense to have more
flexible approach to describe it, no?

> > For the second one I'm not really the expert. But either FPGA framework (if
> > they have anything working for this), or you also may look at Thunderbolt /
> > USB4 which uses similar approach while being PCIe devices. Okay, the latter
> > (USB4) is the PCIe topology, while FPGA is whatever behind the PCI switch.
> > Meaning that FPGA case from HW p.o.v. is closer to your case.
>
> A quick glance at Documentation/driver-api/fpga/ shows that it is a
> framework for dealing with reprogrammable hardware, and has infra to
> reprogram it. My hardware is fixed-function and doesn't need any of that.
>
> Are you suggesting that I should look at reusing some common infra with
> the fpga subsystem instead? A quick grep for device_add in drivers/fpga/
> shows a bunch of open-coded device_add() and platform_device_add() calls.
> Is this what you wanted me to see or is there something else?

Ah, so they don't have a mechanism on how to describe the hardware inside
FPGA _after_ reconfiguration and apply it to the system? That's what I meant
when referred to it.

--
With Best Regards,
Andy Shevchenko



2023-03-01 17:19:18

by Vladimir Oltean

[permalink] [raw]
Subject: Re: Implementation of fwnode_operations :: device_get_match_data() for software nodes?

On Wed, Mar 01, 2023 at 05:34:44PM +0200, Andy Shevchenko wrote:
> On Wed, Mar 01, 2023 at 05:25:27PM +0200, Vladimir Oltean wrote:
> > On Wed, Mar 01, 2023 at 05:09:41PM +0200, Andy Shevchenko wrote:
> > > With overlays you can create the proper DT description stanza and end user's
> > > job is to just put it somewhere and upload via precoded script or so [1].
> > >
> > > [1]:https://docs.kernel.org/devicetree/overlay-notes.html
> >
> > Ah, okay, no, that's already a no-go, since existing device tree blobs
> > aren't compiled with the dtc "-@" flag which would generate the __symbols__
> > node necessary for DT overlays to be applied over them.
> >
> > That, and it's clunky and uncalled for in general, both from my
> > perspective as a driver developer and that of a random user, if a driver
> > would just start requiring device tree overlays for more functionality.
> > Overlays address none of the complaints I had with large DT bindings
> > being large in general. They are still equally large, but now, they are
> > also spread into multiple files.
>
> But isn't it what you would like to have working for your case?
>
> Even taking into account the fixed HW layout, it would make sense to have more
> flexible approach to describe it, no?

Not really, no...
What I would like to have is a (partially) auto- (and dynamically-) generated
firmware description.

I'd need that in order to have a cleaner separation between the device
drivers for the various peripherals on that Ethernet switch SoC.
Currently, there's a lot of monolithic code to drive those peripherals
that lives in drivers/net/dsa/ but would belong to drivers/net/mdio,
drivers/irqchip/, drivers/gpio/, things like that.

What I want is the logic that gets me there, with none of the complications
for things I don't need.

> > > For the second one I'm not really the expert. But either FPGA framework (if
> > > they have anything working for this), or you also may look at Thunderbolt /
> > > USB4 which uses similar approach while being PCIe devices. Okay, the latter
> > > (USB4) is the PCIe topology, while FPGA is whatever behind the PCI switch.
> > > Meaning that FPGA case from HW p.o.v. is closer to your case.
> >
> > A quick glance at Documentation/driver-api/fpga/ shows that it is a
> > framework for dealing with reprogrammable hardware, and has infra to
> > reprogram it. My hardware is fixed-function and doesn't need any of that.
> >
> > Are you suggesting that I should look at reusing some common infra with
> > the fpga subsystem instead? A quick grep for device_add in drivers/fpga/
> > shows a bunch of open-coded device_add() and platform_device_add() calls.
> > Is this what you wanted me to see or is there something else?
>
> Ah, so they don't have a mechanism on how to describe the hardware inside
> FPGA _after_ reconfiguration and apply it to the system? That's what I meant
> when referred to it.

Reading Documentation/devicetree/bindings/fpga/fpga-region.txt (with my
limited and ultra-superficial understanding), I guess that they still
use overlays to describe what should be probed on the reprogrammed regions.

2023-03-01 17:33:39

by Andy Shevchenko

[permalink] [raw]
Subject: Re: Implementation of fwnode_operations :: device_get_match_data() for software nodes?

On Wed, Mar 01, 2023 at 07:18:45PM +0200, Vladimir Oltean wrote:
> On Wed, Mar 01, 2023 at 05:34:44PM +0200, Andy Shevchenko wrote:
> > On Wed, Mar 01, 2023 at 05:25:27PM +0200, Vladimir Oltean wrote:
> > > On Wed, Mar 01, 2023 at 05:09:41PM +0200, Andy Shevchenko wrote:
> > > > With overlays you can create the proper DT description stanza and end user's
> > > > job is to just put it somewhere and upload via precoded script or so [1].
> > > >
> > > > [1]:https://docs.kernel.org/devicetree/overlay-notes.html
> > >
> > > Ah, okay, no, that's already a no-go, since existing device tree blobs
> > > aren't compiled with the dtc "-@" flag which would generate the __symbols__
> > > node necessary for DT overlays to be applied over them.
> > >
> > > That, and it's clunky and uncalled for in general, both from my
> > > perspective as a driver developer and that of a random user, if a driver
> > > would just start requiring device tree overlays for more functionality.
> > > Overlays address none of the complaints I had with large DT bindings
> > > being large in general. They are still equally large, but now, they are
> > > also spread into multiple files.
> >
> > But isn't it what you would like to have working for your case?
> >
> > Even taking into account the fixed HW layout, it would make sense to have more
> > flexible approach to describe it, no?
>
> Not really, no...
> What I would like to have is a (partially) auto- (and dynamically-) generated
> firmware description.
>
> I'd need that in order to have a cleaner separation between the device
> drivers for the various peripherals on that Ethernet switch SoC.
> Currently, there's a lot of monolithic code to drive those peripherals
> that lives in drivers/net/dsa/ but would belong to drivers/net/mdio,
> drivers/irqchip/, drivers/gpio/, things like that.
>
> What I want is the logic that gets me there, with none of the complications
> for things I don't need.
>
> > > > For the second one I'm not really the expert. But either FPGA framework (if
> > > > they have anything working for this), or you also may look at Thunderbolt /
> > > > USB4 which uses similar approach while being PCIe devices. Okay, the latter
> > > > (USB4) is the PCIe topology, while FPGA is whatever behind the PCI switch.
> > > > Meaning that FPGA case from HW p.o.v. is closer to your case.
> > >
> > > A quick glance at Documentation/driver-api/fpga/ shows that it is a
> > > framework for dealing with reprogrammable hardware, and has infra to
> > > reprogram it. My hardware is fixed-function and doesn't need any of that.
> > >
> > > Are you suggesting that I should look at reusing some common infra with
> > > the fpga subsystem instead? A quick grep for device_add in drivers/fpga/
> > > shows a bunch of open-coded device_add() and platform_device_add() calls.
> > > Is this what you wanted me to see or is there something else?
> >
> > Ah, so they don't have a mechanism on how to describe the hardware inside
> > FPGA _after_ reconfiguration and apply it to the system? That's what I meant
> > when referred to it.
>
> Reading Documentation/devicetree/bindings/fpga/fpga-region.txt (with my
> limited and ultra-superficial understanding), I guess that they still
> use overlays to describe what should be probed on the reprogrammed regions.

Yes, that's why I remember overlays approach and FPGA case.

I guess you have very similar requirements to get this done: your case is a
particular one for FPGA, i.e. (re-)loading the same HW layout over and over.

I believe it should be discussed with them being involved. We don't want to
have two approaches of similar things in the kernel.

--
With Best Regards,
Andy Shevchenko



2023-03-01 17:43:22

by Vladimir Oltean

[permalink] [raw]
Subject: Re: Implementation of fwnode_operations :: device_get_match_data() for software nodes?

On Wed, Mar 01, 2023 at 07:33:29PM +0200, Andy Shevchenko wrote:
> Yes, that's why I remember overlays approach and FPGA case.
>
> I guess you have very similar requirements to get this done: your case is a
> particular one for FPGA, i.e. (re-)loading the same HW layout over and over.
>
> I believe it should be discussed with them being involved. We don't want to
> have two approaches of similar things in the kernel.

I don't think comparisons with the denatured case are helpful.
Is "ax + b = 0" a quadratic equation? Well, yes, if you consider it to
be a particular case where the coefficient of x^2 is 0. Do you use
quadratic equation techniques to solve it? No.

I agree we don't want to have multiple approaches of doing the same thing,
but I debate whether I am really doing the same thing?

If software nodes are not designed to be a good fit for my kind of use
case, then what are they designed for?

2024-03-25 17:08:13

by Andy Shevchenko

[permalink] [raw]
Subject: Re: Implementation of fwnode_operations :: device_get_match_data() for software nodes?

On Mon, Mar 25, 2024 at 04:16:27PM +0100, Herve Codina wrote:
> On Mon, 25 Mar 2024 15:41:19 +0200
> Andy Shevchenko <[email protected]> wrote:

..

> > > I agree we don't want to have multiple approaches of doing the same thing,
> > > but I debate whether I am really doing the same thing?
> > >
> > > If software nodes are not designed to be a good fit for my kind of use
> > > case, then what are they designed for?
> >
> > I think the hardware should be described in the respective format. Yet, you
> > have a point that it's too verbose to the cases when we know the layout of
> > the attached (not-hotpluggable) devices.
> >
> > There are discussions [1,2] on how to enable DT for the cases when
> > non-discoverable HW needs to be detected and enumerated.
> >
> > I don't know which solution will eventually be accepted, but my personal
> > opinion here that we would like to distantiate from board files as much
> > as possible.
> >
> > Btw, for the internal (board files) code we may also use property to
> > go with (see how spi-pxa2xx uses that) to distinguish configurations.
> > But it might be not that straight as with driver data.
> >
> > So far, I haven't seen the code (am I mistaken?) which makes use of driver data
> > for software nodes.
> >
> > [1]: https://lore.kernel.org/lkml/[email protected]/
> > [2]: https://lore.kernel.org/lkml/[email protected]/
> >
> > Aux topics which might not directly be related (in order of declining relevance
> > from my p.o.v.):
> > https://lore.kernel.org/lkml/[email protected]/
> > https://lore.kernel.org/lkml/DM6PR12MB3993D5ECA50B27682AEBE19FCD67A@DM6PR12MB3993.namprd12.prod.outlook.com/
> > https://lore.kernel.org/lkml/[email protected]/
> >
>
> I work on PCI DT overlay support.
> The idea is to have a PCI driver that loads a DT overlay to describe the
> hardware embedded in the related PCI device. Some features related to this
> topic are already upstream.
>
> Rob did a presentation of this topic at the Linux Plumber conference last
> year (https://www.youtube.com/watch?v=MVGElnZW7BQ).
>
> IMHO, your use-case is pretty much the same. Of course it is not a PCI
> device but a SPI device. Even if the device beyond the SPI bus cannot be
> memory mapped, the idea seems pretty much the same: describe the hardware
> embedded in a specific device.
>
> You mentioned that you need the '-@' option when you compile your base dtb.
> In fact, it depends on the resources you need to reference from your overlay.
> On my case (DT overlay to describe a PCI device), I don't need any references
> to a base dtb from the overlay. I don't need to use the '-@' option.
> Even more, I started (not yet upstream) to use all of this feature on an ACPI
> base system and it works.
>
> My PCI device is a Microchip LAN9662 PCI device.
> The Microchip LAN9962 can be a "traditional" SoC with CPU cores and several
> IPs but also a PCI device.
> When provided as a PCI device, the internal CPU cores are no more available
> and replaced by a PCI endpoint IP.
> All the accesses done by the SoC CPU cores are replaced by accesses done by
> the host PCI system through the PCI endpoint.
> Drivers were already present upstream (traditional SoC platform driver such
> as i2c mdio, clock, switch, ...) and are used without any modifications for
> the PCI device.
> All the wiring (mapping) between the PCI world and the internal device
> hardware is done using a DT overlay.

Thank you, Herve, for looking into this. As far as I understood, slowly but
successfully the required changes for your use case are being landed. If so,
it makes driver data for software nodes approach even lower priority.

--
With Best Regards,
Andy Shevchenko



2024-03-25 18:51:17

by Andy Shevchenko

[permalink] [raw]
Subject: Re: Implementation of fwnode_operations :: device_get_match_data() for software nodes?

+Cc: people who might be also interested in this topic.

On Wed, Mar 01, 2023 at 07:43:09PM +0200, Vladimir Oltean wrote:
> On Wed, Mar 01, 2023 at 07:33:29PM +0200, Andy Shevchenko wrote:

Sorry for really late reply. I somehow forgot to answer.

> > Yes, that's why I remember overlays approach and FPGA case.
> >
> > I guess you have very similar requirements to get this done: your case is a
> > particular one for FPGA, i.e. (re-)loading the same HW layout over and over.
> >
> > I believe it should be discussed with them being involved. We don't want to
> > have two approaches of similar things in the kernel.
>
> I don't think comparisons with the denatured case are helpful.
> Is "ax + b = 0" a quadratic equation? Well, yes, if you consider it to
> be a particular case where the coefficient of x^2 is 0. Do you use
> quadratic equation techniques to solve it? No.
>
> I agree we don't want to have multiple approaches of doing the same thing,
> but I debate whether I am really doing the same thing?
>
> If software nodes are not designed to be a good fit for my kind of use
> case, then what are they designed for?

I think the hardware should be described in the respective format. Yet, you
have a point that it's too verbose to the cases when we know the layout of
the attached (not-hotpluggable) devices.

There are discussions [1,2] on how to enable DT for the cases when
non-discoverable HW needs to be detected and enumerated.

I don't know which solution will eventually be accepted, but my personal
opinion here that we would like to distantiate from board files as much
as possible.

Btw, for the internal (board files) code we may also use property to
go with (see how spi-pxa2xx uses that) to distinguish configurations.
But it might be not that straight as with driver data.

So far, I haven't seen the code (am I mistaken?) which makes use of driver data
for software nodes.

[1]: https://lore.kernel.org/lkml/[email protected]/
[2]: https://lore.kernel.org/lkml/[email protected]/

Aux topics which might not directly be related (in order of declining relevance
from my p.o.v.):
https://lore.kernel.org/lkml/[email protected]/
https://lore.kernel.org/lkml/DM6PR12MB3993D5ECA50B27682AEBE19FCD67A@DM6PR12MB3993.namprd12.prod.outlook.com/
https://lore.kernel.org/lkml/[email protected]/

--
With Best Regards,
Andy Shevchenko



2024-03-25 23:28:53

by Herve Codina

[permalink] [raw]
Subject: Re: Implementation of fwnode_operations :: device_get_match_data() for software nodes?

Hi Andy,

On Mon, 25 Mar 2024 17:39:03 +0200
Andy Shevchenko <[email protected]> wrote:

> On Mon, Mar 25, 2024 at 04:16:27PM +0100, Herve Codina wrote:
> > On Mon, 25 Mar 2024 15:41:19 +0200
> > Andy Shevchenko <[email protected]> wrote:
>
> ...
>
> > > > I agree we don't want to have multiple approaches of doing the same thing,
> > > > but I debate whether I am really doing the same thing?
> > > >
> > > > If software nodes are not designed to be a good fit for my kind of use
> > > > case, then what are they designed for?
> > >
> > > I think the hardware should be described in the respective format. Yet, you
> > > have a point that it's too verbose to the cases when we know the layout of
> > > the attached (not-hotpluggable) devices.
> > >
> > > There are discussions [1,2] on how to enable DT for the cases when
> > > non-discoverable HW needs to be detected and enumerated.
> > >
> > > I don't know which solution will eventually be accepted, but my personal
> > > opinion here that we would like to distantiate from board files as much
> > > as possible.
> > >
> > > Btw, for the internal (board files) code we may also use property to
> > > go with (see how spi-pxa2xx uses that) to distinguish configurations.
> > > But it might be not that straight as with driver data.
> > >
> > > So far, I haven't seen the code (am I mistaken?) which makes use of driver data
> > > for software nodes.
> > >
> > > [1]: https://lore.kernel.org/lkml/[email protected]/
> > > [2]: https://lore.kernel.org/lkml/[email protected]/
> > >
> > > Aux topics which might not directly be related (in order of declining relevance
> > > from my p.o.v.):
> > > https://lore.kernel.org/lkml/[email protected]/
> > > https://lore.kernel.org/lkml/DM6PR12MB3993D5ECA50B27682AEBE19FCD67A@DM6PR12MB3993.namprd12.prod.outlook.com/
> > > https://lore.kernel.org/lkml/[email protected]/
> > >
> >
> > I work on PCI DT overlay support.
> > The idea is to have a PCI driver that loads a DT overlay to describe the
> > hardware embedded in the related PCI device. Some features related to this
> > topic are already upstream.
> >
> > Rob did a presentation of this topic at the Linux Plumber conference last
> > year (https://www.youtube.com/watch?v=MVGElnZW7BQ).
> >
> > IMHO, your use-case is pretty much the same. Of course it is not a PCI
> > device but a SPI device. Even if the device beyond the SPI bus cannot be
> > memory mapped, the idea seems pretty much the same: describe the hardware
> > embedded in a specific device.
> >
> > You mentioned that you need the '-@' option when you compile your base dtb.
> > In fact, it depends on the resources you need to reference from your overlay.
> > On my case (DT overlay to describe a PCI device), I don't need any references
> > to a base dtb from the overlay. I don't need to use the '-@' option.
> > Even more, I started (not yet upstream) to use all of this feature on an ACPI
> > base system and it works.
> >
> > My PCI device is a Microchip LAN9662 PCI device.
> > The Microchip LAN9962 can be a "traditional" SoC with CPU cores and several
> > IPs but also a PCI device.
> > When provided as a PCI device, the internal CPU cores are no more available
> > and replaced by a PCI endpoint IP.
> > All the accesses done by the SoC CPU cores are replaced by accesses done by
> > the host PCI system through the PCI endpoint.
> > Drivers were already present upstream (traditional SoC platform driver such
> > as i2c mdio, clock, switch, ...) and are used without any modifications for
> > the PCI device.
> > All the wiring (mapping) between the PCI world and the internal device
> > hardware is done using a DT overlay.
>
> Thank you, Herve, for looking into this. As far as I understood, slowly but
> successfully the required changes for your use case are being landed. If so,
> it makes driver data for software nodes approach even lower priority.
>

Yeah, some more changes are still needed to fully support my use case but
everything is on the right track.

Best regards,
Hervé