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

[Andy Shevchenko]

On Thu, Mar 21, 2024 at 03:22:05AM +0800, Sui Jingfeng wrote:
> On 2024/3/20 18:39, Andy Shevchenko wrote:
> > 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.

Yes, for all firmware property providers there is a callback.

> > 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?

Point to what? This arch/x86/kernel/devicetree.c ?

> 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.

Do they support Device Tree? Do you want to enable them in ACPI environment?

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

Please do.

> > > 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.\

Which chip? Flash memory / ROM or you meant something like FPGA here?
For the latter there is another discussion on how to use DT overlays
in ACPI-enabled environments for the FPGA configurations.

> 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?

Yes, but it misses details about board files approach. See also above.

..

> > 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.

What does "emulation" mean? Can you elaborate a bit?

> 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.

-- 
With Best Regards,
Andy Shevchenko