Re: [PATCH 2/4] firmware: arm_scmi: Add Quirks framework

[Johan Hovold <johan@kernel.org>]

On Wed, Apr 16, 2025 at 05:37:13PM +0100, Cristian Marussi wrote:
> On Wed, Apr 16, 2025 at 06:00:37PM +0200, Johan Hovold wrote:
> > On Tue, Apr 15, 2025 at 03:29:31PM +0100, Cristian Marussi wrote:

> > > +static void scmi_enable_matching_quirks(struct scmi_info *info)
> > > +{
> > > +	struct scmi_revision_info *rev = &info->version;
> > > +	const char *compatible = NULL;
> > > +	struct device_node *root;
> > > +
> > > +	root = of_find_node_by_path("/");
> > > +	if (root) {
> > > +		of_property_read_string(root, "compatible", &compatible);
> > 
> > Looks like you still only allow matching on the most specific compatible
> > string.
> > 
> > As we discussed in the RFC thread, this will result in one quirk entry
> > for each machine in a SoC family in case the issue is not machine
> > specific.
> 
> Well, yes but the solution would be to add multiple compatible on the
> same quirk line, which is definitely less cumbersome than adding
> multiple quirk defs for the same quirk but does NOT scale anyway....
> 
> ...anyway I will add that possibility..or I am missing something more ?

I was referring to the need to match on other compatible strings than
the most specific one. For the ThinkPad T14s the strings are:

	"lenovo,thinkpad-t14s-lcd", "lenovo,thinkpad-t14s",
	"qcom,x1e78100", "qcom,x1e80100"

Here you most certainly would not want to match on
"lenovo,thinkpad-t14s-lcd" but rather on "lenovo,thinkpad-t14s" or one
of the SoC compatibles.

For the FC quirk we may have to match on compatible and then a single
SoC entry could cover tens of machines (and their SKU variants).

of_machine_is_compatible() can be used to match on any compatible
string, but not sure if that fits with your current implementation.
 
> > > +		of_node_put(root);
> > > +	}
> > > +
> > > +	/* Enable applicable quirks */
> > > +	scmi_quirks_enable(info->dev, compatible,
> > > +			   rev->vendor_id, rev->sub_vendor_id, rev->impl_ver);
> > > +}

Johan