[PATCH v1 1/1] pwm: core: Replace custom implementation of device_match_fwnode()

[PATCH v1 1/1] pwm: core: Replace custom implementation of device_match_fwnode()

[Andy Shevchenko]

Replace custom implementation of the device_match_fwnode().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pwm/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index cfe3a0327471..d333e7422f4a 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -678,7 +678,7 @@ static struct pwm_chip *fwnode_to_pwmchip(struct fwnode_handle *fwnode)
 	mutex_lock(&pwm_lock);
 
 	list_for_each_entry(chip, &pwm_chips, list)
-		if (chip->dev && dev_fwnode(chip->dev) == fwnode) {
+		if (chip->dev && device_match_fwnode(chip->dev, fwnode)) {
 			mutex_unlock(&pwm_lock);
 			return chip;
 		}
-- 
2.35.1

Re: [PATCH v1 1/1] pwm: core: Replace custom implementation of device_match_fwnode()

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 710 bytes --]

On Tue, Sep 27, 2022 at 08:22:58PM +0300, Andy Shevchenko wrote:
> Replace custom implementation of the device_match_fwnode().
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/pwm/core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

I really don't see the point in having an exported symbol for this. It's
a simple comparison and the result is even longer than the original. The
*only* reason why this helper exists seems to be because it is getting
used in *_find_device() callbacks.

Honestly, I don't see a reason why this should be applied. And frankly,
why bother making all these changes. It's a waste of time, in my
opinion.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v1 1/1] pwm: core: Replace custom implementation of device_match_fwnode()

[Andy Shevchenko]

On Wed, Sep 28, 2022 at 04:26:47PM +0200, Thierry Reding wrote:
> On Tue, Sep 27, 2022 at 08:22:58PM +0300, Andy Shevchenko wrote:
> > Replace custom implementation of the device_match_fwnode().

> I really don't see the point in having an exported symbol for this. It's
> a simple comparison and the result is even longer than the original.

Longer doesn't always mean worse.

> The
> *only* reason why this helper exists seems to be because it is getting
> used in *_find_device() callbacks.

Yes and no. Initially for the purpose to be a callback it can be reused.
The point is that it hides the dev_fwnode() machinery behind and taking
into account ongoing discussion about constification of the dev_fwnode()
we might need to touch this or similar places to avoid problems with
compiler.

> Honestly, I don't see a reason why this should be applied. And frankly,
> why bother making all these changes. It's a waste of time, in my
> opinion.

Obviously I will not do it if I be with you on the same page.
But okay, not a big deal in this case.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/1] pwm: core: Replace custom implementation of device_match_fwnode()

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1207 bytes --]

On Wed, Sep 28, 2022 at 05:49:36PM +0300, Andy Shevchenko wrote:
> On Wed, Sep 28, 2022 at 04:26:47PM +0200, Thierry Reding wrote:
> > On Tue, Sep 27, 2022 at 08:22:58PM +0300, Andy Shevchenko wrote:
> > > Replace custom implementation of the device_match_fwnode().
> 
> > I really don't see the point in having an exported symbol for this. It's
> > a simple comparison and the result is even longer than the original.
> 
> Longer doesn't always mean worse.
> 
> > The
> > *only* reason why this helper exists seems to be because it is getting
> > used in *_find_device() callbacks.
> 
> Yes and no. Initially for the purpose to be a callback it can be reused.
> The point is that it hides the dev_fwnode() machinery behind and taking
> into account ongoing discussion about constification of the dev_fwnode()
> we might need to touch this or similar places to avoid problems with
> compiler.

Maybe next time use that argument in the commit message. That's much
more convincing than a useless "replace custom implementation" because
that just makes it look like you're doing this to pass the time or
something.

Applied, with a slightly updated commit message, thanks.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v1 1/1] pwm: core: Replace custom implementation of device_match_fwnode()

[Andy Shevchenko]

On Wed, Sep 28, 2022 at 04:57:57PM +0200, Thierry Reding wrote:
> On Wed, Sep 28, 2022 at 05:49:36PM +0300, Andy Shevchenko wrote:
> > On Wed, Sep 28, 2022 at 04:26:47PM +0200, Thierry Reding wrote:
> > > On Tue, Sep 27, 2022 at 08:22:58PM +0300, Andy Shevchenko wrote:
> > > > Replace custom implementation of the device_match_fwnode().
> > 
> > > I really don't see the point in having an exported symbol for this. It's
> > > a simple comparison and the result is even longer than the original.
> > 
> > Longer doesn't always mean worse.
> > 
> > > The
> > > *only* reason why this helper exists seems to be because it is getting
> > > used in *_find_device() callbacks.
> > 
> > Yes and no. Initially for the purpose to be a callback it can be reused.
> > The point is that it hides the dev_fwnode() machinery behind and taking
> > into account ongoing discussion about constification of the dev_fwnode()
> > we might need to touch this or similar places to avoid problems with
> > compiler.
> 
> Maybe next time use that argument in the commit message. That's much
> more convincing than a useless "replace custom implementation" because
> that just makes it look like you're doing this to pass the time or
> something.

Noted.

> Applied, with a slightly updated commit message, thanks.

Thank you!

-- 
With Best Regards,
Andy Shevchenko