Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Date:   Wed, 16 Feb 2022 10:31:32 +0800
From:   Tzung-Bi Shih <tzungbi@google.com>
To:     Prashant Malani <pmalani@chromium.org>
Cc:     bleung@chromium.org, groeck@chromium.org, robh+dt@kernel.org,
        chrome-platform@lists.linux.dev, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH 4/5] platform/chrome: cros_kbd_led_backlight: support OF
 match
Message-ID: <YgxiBO7jDtyhWwlb@google.com>
References: <20220214053646.3088298-1-tzungbi@google.com>
 <20220214053646.3088298-5-tzungbi@google.com>
 <CACeCKac+b1Rp6bZFALn=sbh8gkJbWeKTC=8bdzp+-90pgq=wSw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CACeCKac+b1Rp6bZFALn=sbh8gkJbWeKTC=8bdzp+-90pgq=wSw@mail.gmail.com>
Precedence: bulk

On Tue, Feb 15, 2022 at 05:10:04PM -0800, Prashant Malani wrote:
> On Sun, Feb 13, 2022 at 9:37 PM Tzung-Bi Shih <tzungbi@google.com> wrote:
> > diff --git a/drivers/platform/chrome/cros_kbd_led_backlight.c b/drivers/platform/chrome/cros_kbd_led_backlight.c
> > index 814f2b74c602..ba853e55d29a 100644
> > --- a/drivers/platform/chrome/cros_kbd_led_backlight.c
> > +++ b/drivers/platform/chrome/cros_kbd_led_backlight.c
> > @@ -10,6 +10,7 @@
> >  #include <linux/kernel.h>
> >  #include <linux/leds.h>
> >  #include <linux/module.h>
> > +#include <linux/of_device.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/slab.h>
> >
> > @@ -128,8 +129,11 @@ static int keyboard_led_probe(struct platform_device *pdev)
> >         int error;
> >
> >         drvdata = acpi_device_get_match_data(&pdev->dev);
> > -       if (!drvdata)
> > -               return -EINVAL;
> > +       if (!drvdata) {
> > +               drvdata = of_device_get_match_data(&pdev->dev);
> > +               if (!drvdata)
> > +                       return -EINVAL;
> > +       }
> 
> I'm not familiar with this driver, so can't do a full review, but
> shouldn't device_get_match_data()
> from property.h [1] be able to handle both DT and ACPI cases?
> 
> [1]: https://elixir.bootlin.com/linux/v5.17-rc4/source/include/linux/property.h

Yes, it does[2][3].  Thanks for the feedback, will fix it in next version.

[2]: https://elixir.bootlin.com/linux/v5.17-rc4/source/drivers/of/property.c#L1474
[3]: https://elixir.bootlin.com/linux/v5.17-rc4/source/drivers/acpi/property.c#L1386