Re: [PATCH v8 3/3] HID: cp2112: Fwnode Support

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cc: Daniel Kaehn <kaehndan@gmail.com>,
	Hans de Goede <hdegoede@redhat.com>,
	robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org,
	jikos@kernel.org, bartosz.golaszewski@linaro.org,
	dmitry.torokhov@gmail.com, devicetree@vger.kernel.org,
	linux-input@vger.kernel.org, ethan.twardy@plexus.com
Subject: Re: [PATCH v8 3/3] HID: cp2112: Fwnode Support
Date: Tue, 7 Mar 2023 20:14:21 +0200	[thread overview]
Message-ID: <ZAd+/fHeQ3zuvoTN@smile.fi.intel.com> (raw)
In-Reply-To: <20230307131706.olnb4qzo4ynu7gce@mail.corp.redhat.com>

On Tue, Mar 07, 2023 at 02:17:06PM +0100, Benjamin Tissoires wrote:
> On Mar 06 2023, Andy Shevchenko wrote:
> > On Mon, Mar 06, 2023 at 01:40:16PM -0600, Daniel Kaehn wrote:

...

> This almost works. Almost because the I2C device is correctly created,
> but I have an issue with the GpioInt call which is not properly set by
> the kernel and which returns -EDEFER. /o\ 

You can try to utilize _DEP method, but I dunno if it implemented that way in
the Linux that helps your case.

> > > With this all said -- I noticed iasl prints this statement when trying
> > > to create a node with a lowercase name:
> > > "At least one lower case letter found in NameSeg, ASL is case
> > > insensitive - converting to upper case (GPIO)"
> > 
> > Yes, because it should be in the upper case.
> > 
> > > I wonder if this suggests that adding a call to toupper() to
> > > acpi_fwnode_get_named_child_node would be
> > > an appropriate solution for the node name casing issue....
> > 
> > I dunno. You need to ask in the linux-acpi@ mailing list.
> > To me this is corner case that can't be easily solved
> > (because two different specifications treat it differently.
> > 
> > You also need to ask DT people about capital letters there.
> > And my guts tell me that it's probably also carved in the spec
> > as "must be lower case" or alike.
> 
> FWIW while trying to enable this, at some point I named the I2C and the
> GPIO entries "I2C0" and "GPI0" (with the number '0', not the letter
> 'o'), and it was not working as you would expect.
> 
> It is commonly accepted in the ACPI world that the names do not carry
> meaning AFAICT, and so I think I agree with Andy's initial comment
> regarding using indexes, not names to also fetch the I2C and GPIO nodes.
> You can probably have a fallback mechanism for when "i2c" is not
> present, or simply check if you are in DT or not and use the names only
> if we are in DT.

The solution is to provide in the main node the list of cell names, that way
you will always know the indices:

  Device (DEV) {
	  _DSD
		  "cell-names" { "i2c", "gpio" } // index of the name is the
						 // index of the cell

	Device (I2C0) {
	}

	Device (GPI0) {
	}
  }

Problem solved.

> Thanks a lot to both of you, this will be tremendously helpful to me.

You're welcome!

> [1] https://www.kernel.org/doc/html/latest/admin-guide/acpi/ssdt-overlays.html#loading-acpi-ssdts-from-initrd

-- 
With Best Regards,
Andy Shevchenko