Re: [PATCH] software node: Implement device_get_match_data fwnode callback

[Sui Jingfeng]

Hi,


On 2024/3/20 18:39, Andy Shevchenko wrote:
> +Cc: Vladimir
>
> On Tue, Mar 19, 2024 at 07:42:22AM +0800, Sui Jingfeng wrote:
> > This makes it possible to support (and/or test) a few drivers that
> > originates from DT World on the x86-64 platform. Originally, those
> > drivers using the of_device_get_match_data() function to get match
> > data. For example, drivers/gpu/drm/bridge/simple-bridge.c and
> > drivers/gpu/drm/bridge/display-connector.c. Those drivers works very
> > well in the DT world, however, there is no counterpart to
> > of_device_get_match_data() when porting them to the x86 platform,
> > because x86 CPUs lack DT support.
> This is not true.
>
> First of all, there is counter part that called device_get_match_data().

Are you means that the acpi_fwnode_device_get_match_data() implementation?
As the fwnode API framework has three backend: OF, ACPI, and software node.
If you are hinting me that the acpi backend has the .device_get_match_data
implemented. Then you are right.


> Second, there *is* DT support for the _selected_ x86 based platforms.

Yeah, you maybe right again here. I guess you means that some special
hardware or platform may have a *limited* support?

Can you pointed it out for study of learning purpose?

To speak precisely, there are some drm display bridges drivers are
lack of the DT support on X86. Those display bridges belong to the
device drivers catalogs.

OK, I will update my commit message at the next version if possible,
and try my best to describe the problem precisely.

> > By replacing it with device_get_match_data() and creating a software
> > graph that mimics the OF graph, everything else works fine, except that
> > there isn't an out-of-box replacement for the of_device_get_match_data()
> > function. Because the software node backend of the fwnode framework lacks
> > an implementation for the device_get_match_data callback.
> .device_get_match_data
>
> > Implement device_get_match_data fwnode callback fwnode callback to fill
> .device_get_match_data


OK, thanks a lot.

> > this gap. Device drivers or platform setup codes are expected to provide
> > a "compatible" string property. The value of this string property is used
> > to match against the compatible entries in the of_device_id table. Which
> > is consistent with the original usage style.
> Why do you need to implement the graph in the board file?

It can be inside the chip, there is no clear cut.  I means that
the graph(including fwnode graph, OF graph or swnode graph) can
be used at anywhere. The examples given here may lead you to
think it is board specific, but it is not limited to board specific.

fwnode graph, OF graph and swnode graph, all of them are implements
of the graph. Its common that different hardware vendors bought the
some IP and has been integrated it into their SoC. So it can be inside
of the chip if you want *code sharing*.


Back to the patch itself, we want to keep the three backends aligned as 
much as possible. Is this reasonable enough?

> ...
>
> Have you seen this discussion?
> https://lore.kernel.org/lkml/20230223203713.hcse3mkbq3m6sogb@skbuf/

I really didn't have seen that thread before this patch is sent,
I'm a graphic developer, I'm mainly focus on graphics domain.

Previously, I have implemented similar functionality at the drivers
layer [1][2]. But as the instances grows,  I realized there is a
risk to introducing *boilerplate*.  So I send this patch. [1][2] can
be drop if this patch could be merged.

[1] https://patchwork.freedesktop.org/patch/575414/?series=129040&rev=1

[2] https://patchwork.freedesktop.org/patch/575411/?series=129040&rev=1


After a brief skim,  I guess we encounter similar problems. Oops!
In a nutshell, there is a need to *emulation* on X86 platform,
to suit the need of device-driver coding style of DT world.

Besides, at the swnode backend layer, we should not call
fwnode_property_read_string(), instead, we should usethe property_entry_read_string_array() function. Because the 
fwnode_property_read_string() is belong to upper layer.
While backend implementations should call functions from
bottom layer only.


--
Best regards,
Sui