[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <7CFY8Q.68YMS0V08F992@crapouillou.net>
Date: Sat, 18 Apr 2020 00:48:07 +0200
From: Paul Cercueil <paul@...pouillou.net>
To: Andy Shevchenko <andy.shevchenko@...il.com>
Cc: Artur Rojek <contact@...ur-rojek.eu>,
Dmitry Torokhov <dmitry.torokhov@...il.com>,
Rob Herring <robh+dt@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Jonathan Cameron <jic23@...nel.org>,
Heiko Stuebner <heiko@...ech.de>,
linux-input <linux-input@...r.kernel.org>,
devicetree <devicetree@...r.kernel.org>,
linux-iio <linux-iio@...r.kernel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [RESEND PATCH v5 5/5] input: joystick: Add ADC attached joystick
driver.
Le sam. 18 avril 2020 à 0:49, Andy Shevchenko
<andy.shevchenko@...il.com> a écrit :
> On Sat, Apr 18, 2020 at 12:24 AM Paul Cercueil <paul@...pouillou.net>
> wrote:
>> Le sam. 18 avril 2020 à 0:10, Andy Shevchenko
>> <andy.shevchenko@...il.com> a écrit :
>> > On Fri, Apr 17, 2020 at 11:21 PM Artur Rojek
>> <contact@...ur-rojek.eu>
>> > wrote:
>
> ...
>
>> >> +#include <linux/of.h>
>> >
>> > Do you really need this? (See below as well)
>
>> >> +static const struct of_device_id adc_joystick_of_match[] = {
>> >> + { .compatible = "adc-joystick", },
>> >> + { },
>> >> +};
>> >> +MODULE_DEVICE_TABLE(of, adc_joystick_of_match);
>> >> +
>> >> +static struct platform_driver adc_joystick_driver = {
>> >> + .driver = {
>> >> + .name = "adc-joystick",
>> >
>> >> + .of_match_table =
>> >> of_match_ptr(adc_joystick_of_match),
>> >
>> > Drop this a bit harmful of_match_ptr() macro. It should go with
>> ugly
>> > #ifdeffery. Here you simple introduced a compiler warning.
>>
>> I assume you mean #ifdef around the of_device_id + module table
>> macro?
>
> Yes.
>
>> > On top of that, you are using device property API, OF use in this
>> case
>> > is contradictory (at lest to some extend).
>>
>> I don't see why. The fact that the driver can work when probed from
>> platform code
>
> Ha-ha, tell me how. I would like to be very surprised.
iio_map_array_register(),
pinctrl_register_mappings(),
platform_add_devices(),
you're welcome.
>> doesn't mean that it shouldn't have a table to probe
>> from devicetree.
>
> I didn't get what you are talking about here. The idea of _unified_
> device property API is to get rid of OF-centric code in favour of more
> generic approach. Mixing those two can be done only in specific cases
> (here is not the one).
And how are we mixing those two here? The only OF-centric thing here is
the device table, which is required if we want the driver to probe from
devicetree.
-Paul
>
> --
> With Best Regards,
> Andy Shevchenko
Powered by blists - more mailing lists