diff mbox series

[v2,2/6] pinctrl: intel: use acpi_dev_uid_match() for matching _UID

Message ID 20231023053530.5525-3-raag.jadav@intel.com
State New
Headers
Series Refine _UID references across kernel |

Commit Message

Raag Jadav Oct. 23, 2023, 5:35 a.m. UTC 
  Convert manual _UID references to use the standard ACPI helper.

Signed-off-by: Raag Jadav <raag.jadav@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pinctrl/intel/pinctrl-intel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Andy Shevchenko Oct. 23, 2023, 11:35 a.m. UTC | #1 
On Mon, Oct 23, 2023 at 11:05:26AM +0530, Raag Jadav wrote:
> Convert manual _UID references to use the standard ACPI helper.

> Signed-off-by: Raag Jadav <raag.jadav@intel.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

As I asked you, please drop this one. It has a hidden logic that is not aligned
with acpi_dev_hid_uid_match(). Or revert to your v1 I assume.
Raag Jadav Oct. 23, 2023, 2:42 p.m. UTC | #2 
On Mon, Oct 23, 2023 at 02:35:13PM +0300, Andy Shevchenko wrote:
> On Mon, Oct 23, 2023 at 11:05:26AM +0530, Raag Jadav wrote:
> > Convert manual _UID references to use the standard ACPI helper.
> 
> > Signed-off-by: Raag Jadav <raag.jadav@intel.com>
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> It has a hidden logic that is not aligned with acpi_dev_hid_uid_match().
> Or revert to your v1 I assume.

I don't see how this has to be aligned with acpi_dev_hid_uid_match() or
if acpi_dev_hid_uid_match() implementation concerns this specific change,
since that's not what we intend to do here.

Also, I think acpi_dev_uid_match() implementation in v2 is actually more
aligned with the previous logic that we're replacing here, since it gives
us a guaranteed match result as originally intended with strcmp in this
case. And the "hidden logic" in v1 implementation (match with @uid2 == NULL)
is what ends up breaking it in my opinion.

Regardless, for any version (v1 or v2) the usage still remains the same
in this case.

> As I asked you, please drop this one.

But okay, as you wish :(

Rafael, should I send a v3 with dropped tags?

Raag
Rafael J. Wysocki Oct. 23, 2023, 6:45 p.m. UTC | #3 
On Mon, Oct 23, 2023 at 4:43 PM Raag Jadav <raag.jadav@intel.com> wrote:
>
> On Mon, Oct 23, 2023 at 02:35:13PM +0300, Andy Shevchenko wrote:
> > On Mon, Oct 23, 2023 at 11:05:26AM +0530, Raag Jadav wrote:
> > > Convert manual _UID references to use the standard ACPI helper.
> >
> > > Signed-off-by: Raag Jadav <raag.jadav@intel.com>
> > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> >
> > It has a hidden logic that is not aligned with acpi_dev_hid_uid_match().
> > Or revert to your v1 I assume.
>
> I don't see how this has to be aligned with acpi_dev_hid_uid_match() or
> if acpi_dev_hid_uid_match() implementation concerns this specific change,
> since that's not what we intend to do here.
>
> Also, I think acpi_dev_uid_match() implementation in v2 is actually more
> aligned with the previous logic that we're replacing here, since it gives
> us a guaranteed match result as originally intended with strcmp in this
> case. And the "hidden logic" in v1 implementation (match with @uid2 == NULL)
> is what ends up breaking it in my opinion.
>
> Regardless, for any version (v1 or v2) the usage still remains the same
> in this case.

Right, so it is a bit unclear what all of the fuss is about.

> > As I asked you, please drop this one.
>
> But okay, as you wish :(
>
> Rafael, should I send a v3 with dropped tags?

No need to resend in general, I can drop tags from the patches just fine.

For this one, though, I'd like to get a maintainer's ACK, so it may be
necessary to resend it without the tag.
diff mbox series

Patch

diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c
index 3be04ab760d3..999f453344d2 100644
--- a/drivers/pinctrl/intel/pinctrl-intel.c
+++ b/drivers/pinctrl/intel/pinctrl-intel.c
@@ -1694,7 +1694,7 @@  const struct intel_pinctrl_soc_data *intel_pinctrl_get_soc_data(struct platform_
 		unsigned int i;
 
 		for (i = 0; table[i]; i++) {
-			if (!strcmp(adev->pnp.unique_id, table[i]->uid)) {
+			if (acpi_dev_uid_match(adev, table[i]->uid)) {
 				data = table[i];
 				break;
 			}