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

Dmitry Torokhov
2024-06-27T16:55:49Z

On Sun, Jun 23, 2024 at 12:58:25AM -0700, Dmitry Torokhov wrote:
> On Sun, Jun 23, 2024 at 03:38:23PM +0800, Sui Jingfeng wrote:
> > Hi,
> >
> > On 6/23/24 03:29, Dmitry Torokhov wrote:
> > > > In case of non-OF match (which
> > > > > includes the case where you use software nodes) the match data is coming
> > > > > from matching spi_device_id entry in the driver.
> > > >
> > > > We don't care about much how it is probed now, rather, after the driver
> > > > probed by a non-OF way, how does the additional devices properties
> > > > can be get?
> > > >
> > > >
> > > > Say:
> > > >
> > > > 1) "device_property_read_u32(dev, "rotation", &rotation);" and
> > > > 2) "!device_property_read_string(dev, "pervasive,thermal-zone",
> > > > &thermal_zone))"
> > > >
> > > >
> > > > For those spi/i2c/platform devices, what we argues are that
> > > > those drivers really should just depend on "OF" before we have
> > > > a reliable fwnode API backend to redirect to.
> > > They are working fine without such restriction now,
> >
> >
> > You still *NOT* answer where the additional devices properties[1][2]
> > can be acquire.
> >
> > [1] device_property_read_u32(dev, "rotation", &rotation)
> >
> > [2] device_property_read_string(dev, "pervasive,thermal-zone",
> > &thermal_zone))
> >
> >
> > > so I see absolutely no reason imposing this restriction.
> >
> > The reason is rigorous.
> >
> > You are acclaiming that works by hardcode or by ignoring the flaws
> > is fine, then all driver are working fine by *your* standard.
> >
> > Your personal standard has nothing to do with this patch.
> >
> > > > Where the additional device_property_read_xxxx() calls redirect to?
> > > >
> > > > What if users want to invoke more device_property_read_xxxx() function?
> > > They are being directed to first the primary FW node instance, which may
> > > be either OF, ACPI, or SW node, and then, if property is not present
> > > there, to the secondary FW node, which can be either again.
> >
> >
> > What I'm asking is, on the non-OF and no-ACPI cases, where's those
> > device_property_read_xxx() calls can be directed to?
> >
> > > At no point ->device_get_match_data() callback in involved in this
> > > process.
> > >
> >
> > The patch is written for people who need it, not for people who don't.
> >
> > It will be involved if the device is associated with software node.
> > Its for fwnode API user to get a consistent experience, that is
> > to get a matching data without introduce extra/duplicated match
> > mechanism.
> >
> > The patch is focus on fixing the undefined behavior, is discussing
> > the correct way to consolidate the fwnode API. Its not going to
> > discuss how does the those *old" and/or how does those non-fwnode
> > systems works.
> >
> > Its NOT discussing how does the driver itself can be probed, a driver
> > can be probed multiple way and is another question. Being probed and
> > extract matching data can two different thing and is perfectly valid.
> >
> > Your problem is that you are not fully understand what other people
> > does before you rush into the discussion. You are putting restrictions
> > onto other people, while leaving the problem itself there unsolved.
> >
> > Its not a place to express your personal value or you personal status,
> > such as, you are "ready" or "not ready" for something. Or persuading
> > somebody should get used to what or teaching people to talks with a
> > whatever tone like a God.
> >
> > None of those junk words are technical, I can not see constructive
> > ideas.
>
> Yes, indeed, it appears that further discussion is pointless at this
> point.
>
> Andy, Heikki, Greg, and others: FWIW this is a NAK from me.

Even though I said I will not discuss this topic, I stumbled across some
of your patches to the DRM subsystem and it looks like what you want is
actually to help push through this set of patches allowing buses (such as
platform) define their own ->get_match_data() callback so that
device_get_match_data() can be used universally:

https://lore.kernel.org/all/20230804161728.394920-2-biju.das.jz@bp.renesas.com/

Thanks.

--
Dmitry