Re: [RESEND PATCH v5 5/5] input: joystick: Add ADC attached joystick driver.

[Paul Cercueil]

Le sam. 18 avril 2020 à 0:49, Andy Shevchenko 
<andy.shevchenko@xxxxxxxxx> a écrit :
> On Sat, Apr 18, 2020 at 12:24 AM Paul Cercueil <paul@xxxxxxxxxxxxxxx> 
> wrote:
> >  Le sam. 18 avril 2020 à 0:10, Andy Shevchenko
> >  <andy.shevchenko@xxxxxxxxx> a écrit :
> >  > On Fri, Apr 17, 2020 at 11:21 PM Artur Rojek 
> > <contact@xxxxxxxxxxxxxx>
> >  > 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