diff mbox

[2/2] input: cros_ec_keyb: Add of match table

Message ID 1408748582-29029-3-git-send-email-sjoerd.simons@collabora.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Sjoerd Simons Aug. 22, 2014, 11:03 p.m. UTC
To enable the cros_ec_keyb driver to be auto-loaded when build as
module add an of match table (and export it) to match the modalias
information passed on to userspace as the Cros EC MFD driver registers
the MFD subdevices with an of_compatibility string.

Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
---
 drivers/input/keyboard/cros_ec_keyb.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Sergei Shtylyov Aug. 23, 2014, 1:42 p.m. UTC | #1
Hello.

On 8/23/2014 3:03 AM, Sjoerd Simons wrote:

> To enable the cros_ec_keyb driver to be auto-loaded when build as
> module add an of match table (and export it) to match the modalias
> information passed on to userspace as the Cros EC MFD driver registers
> the MFD subdevices with an of_compatibility string.

> Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
> ---
>   drivers/input/keyboard/cros_ec_keyb.c | 10 ++++++++++
>   1 file changed, 10 insertions(+)

> diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
> index 791781a..0bdbf2d 100644
> --- a/drivers/input/keyboard/cros_ec_keyb.c
> +++ b/drivers/input/keyboard/cros_ec_keyb.c
> @@ -342,10 +342,20 @@ static int cros_ec_keyb_resume(struct device *dev)
>
>   static SIMPLE_DEV_PM_OPS(cros_ec_keyb_pm_ops, NULL, cros_ec_keyb_resume);
>
> +#ifdef CONFIG_OF
> +static const struct of_device_id cros_ec_keyb_of_match[] = {

    Perhaps better to use '__maybe_unused' instead of #ifdef...

> +	{ .compatible = "google,cros-ec-keyb" },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, cros_ec_keyb_of_match);
> +#endif
> +
> +

    Too many empty lines.

>   static struct platform_driver cros_ec_keyb_driver = {
>   	.probe = cros_ec_keyb_probe,
>   	.driver = {
>   		.name = "cros-ec-keyb",
> +		.of_match_table = of_match_ptr (cros_ec_keyb_of_match),

    There shouldn't be space before (.

>   		.pm	= &cros_ec_keyb_pm_ops,
>   	},
>   };

WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sjoerd Simons Aug. 25, 2014, 7:34 a.m. UTC | #2
Hey,

On Sat, 2014-08-23 at 17:42 +0400, Sergei Shtylyov wrote:
> Hello.
> 
> On 8/23/2014 3:03 AM, Sjoerd Simons wrote:
> 
> > To enable the cros_ec_keyb driver to be auto-loaded when build as
> > module add an of match table (and export it) to match the modalias
> > information passed on to userspace as the Cros EC MFD driver registers
> > the MFD subdevices with an of_compatibility string.
> 
> > Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
> > ---
> >   drivers/input/keyboard/cros_ec_keyb.c | 10 ++++++++++
> >   1 file changed, 10 insertions(+)
> 
> > diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
> > index 791781a..0bdbf2d 100644
> > --- a/drivers/input/keyboard/cros_ec_keyb.c
> > +++ b/drivers/input/keyboard/cros_ec_keyb.c
> > @@ -342,10 +342,20 @@ static int cros_ec_keyb_resume(struct device *dev)
> >
> >   static SIMPLE_DEV_PM_OPS(cros_ec_keyb_pm_ops, NULL, cros_ec_keyb_resume);
> >
> > +#ifdef CONFIG_OF
> > +static const struct of_device_id cros_ec_keyb_of_match[] = {

>     Perhaps better to use '__maybe_unused' instead of #ifdef...

Hmm, looks like the rtc-ds1742.c driver is the only one in the kernel
tree using that strategy, while all others use #ifdef CONFIG_OF. So i'm
inclined to keep the #ifdef here, ooi what is your rationale behind
suggesting __maybe_unused?


> > +	{ .compatible = "google,cros-ec-keyb" },
> > +	{},
> > +};
> > +MODULE_DEVICE_TABLE(of, cros_ec_keyb_of_match);
> > +#endif
> > +
> > +
> 
>     Too many empty lines.



> >   static struct platform_driver cros_ec_keyb_driver = {
> >   	.probe = cros_ec_keyb_probe,
> >   	.driver = {
> >   		.name = "cros-ec-keyb",
> > +		.of_match_table = of_match_ptr (cros_ec_keyb_of_match),
> 
>     There shouldn't be space before (.

Will fix the identation issues in a v2.

Thanks for the review,
 Sjoerd
Javier Martinez Canillas Aug. 25, 2014, 9:33 a.m. UTC | #3
Hello,

On Mon, Aug 25, 2014 at 9:34 AM, Sjoerd Simons
<sjoerd.simons@collabora.co.uk> wrote:
>> >
>> >   static SIMPLE_DEV_PM_OPS(cros_ec_keyb_pm_ops, NULL, cros_ec_keyb_resume);
>> >
>> > +#ifdef CONFIG_OF
>> > +static const struct of_device_id cros_ec_keyb_of_match[] = {
>
>>     Perhaps better to use '__maybe_unused' instead of #ifdef...
>
> Hmm, looks like the rtc-ds1742.c driver is the only one in the kernel
> tree using that strategy, while all others use #ifdef CONFIG_OF. So i'm
> inclined to keep the #ifdef here, ooi what is your rationale behind
> suggesting __maybe_unused?
>

I agree with Sjoerd on this. Not only using the #ifdef guards makes it
more evident when reading the code that this depends on OF being
enabled but also if using __maybe_unused an entry in the struct
of_device_id table will be added for no reason.

>
>> > +   { .compatible = "google,cros-ec-keyb" },
>> > +   {},
>> > +};
>> > +MODULE_DEVICE_TABLE(of, cros_ec_keyb_of_match);
>> > +#endif
>> > +
>> > +
>>
>>     Too many empty lines.
>
>
>
>> >   static struct platform_driver cros_ec_keyb_driver = {
>> >     .probe = cros_ec_keyb_probe,
>> >     .driver = {
>> >             .name = "cros-ec-keyb",
>> > +           .of_match_table = of_match_ptr (cros_ec_keyb_of_match),
>>
>>     There shouldn't be space before (.
>
> Will fix the identation issues in a v2.
>
> Thanks for the review,
>  Sjoerd

After fixing the empty lines:

Reviewed-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>

Best regards,
Javier
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
index 791781a..0bdbf2d 100644
--- a/drivers/input/keyboard/cros_ec_keyb.c
+++ b/drivers/input/keyboard/cros_ec_keyb.c
@@ -342,10 +342,20 @@  static int cros_ec_keyb_resume(struct device *dev)
 
 static SIMPLE_DEV_PM_OPS(cros_ec_keyb_pm_ops, NULL, cros_ec_keyb_resume);
 
+#ifdef CONFIG_OF
+static const struct of_device_id cros_ec_keyb_of_match[] = {
+	{ .compatible = "google,cros-ec-keyb" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, cros_ec_keyb_of_match);
+#endif
+
+
 static struct platform_driver cros_ec_keyb_driver = {
 	.probe = cros_ec_keyb_probe,
 	.driver = {
 		.name = "cros-ec-keyb",
+		.of_match_table = of_match_ptr (cros_ec_keyb_of_match),
 		.pm	= &cros_ec_keyb_pm_ops,
 	},
 };