Message ID | 20200417202859.35427-5-contact@artur-rojek.eu (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [RESEND,v5,1/5] IIO: Ingenic JZ47xx: Add xlate cb to retrieve correct channel idx | expand |
On Fri, Apr 17, 2020 at 11:21 PM Artur Rojek <contact@artur-rojek.eu> wrote: > > Add a driver for joystick devices connected to ADC controllers > supporting the Industrial I/O subsystem. ... > +#include <linux/of.h> Do you really need this? (See below as well) ... > + sign = (tolower(joy->chans[i].channel->scan_type.sign) == 's'); Too many parentheses. But here it's up to you, ... > + case 2: > + val = ((const u16 *)data)[i]; Can't you do this in each branch below? > + if (endianness == IIO_BE) > + val = be16_to_cpu(val); > + else if (endianness == IIO_LE) > + val = le16_to_cpu(val); > + break; ... > + device_for_each_child_node(dev, child) { > + ret = fwnode_property_read_u32(child, "reg", &i); > + if (ret || i >= num_axes) { > + dev_err(dev, "reg invalid or missing"); > + goto err; > + } > + > + if (fwnode_property_read_u32(child, "linux,code", > + &axes[i].code)) { > + dev_err(dev, "linux,code invalid or missing"); > + goto err; > + } > + > + if (fwnode_property_read_u32_array(child, "abs-range", > + axes[i].range, 2)) { > + dev_err(dev, "abs-range invalid or missing"); > + goto err; > + } > + } > + > + joy->axes = axes; > + > + return 0; > + > +err: > + fwnode_handle_put(child); > + return -EINVAL; Can we avoid shadowing the actual error code? ... > + bits = joy->chans[0].channel->scan_type.storagebits; > + if (!bits || (bits >> 3) > 2) { Wouldn't be clear to use simple 'bits > 16'? > + dev_err(dev, "Unsupported channel storage size"); > + return -EINVAL; > + } ... > +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. On top of that, you are using device property API, OF use in this case is contradictory (at lest to some extend). > + }, > + .probe = adc_joystick_probe, > +};
Hi Andy, Le sam. 18 avril 2020 à 0:10, Andy Shevchenko <andy.shevchenko@gmail.com> a écrit : > On Fri, Apr 17, 2020 at 11:21 PM Artur Rojek <contact@artur-rojek.eu> > wrote: >> >> Add a driver for joystick devices connected to ADC controllers >> supporting the Industrial I/O subsystem. > > ... > >> +#include <linux/of.h> > > Do you really need this? (See below as well) > > ... > >> + sign = >> (tolower(joy->chans[i].channel->scan_type.sign) == 's'); > > Too many parentheses. But here it's up to you, > > ... > >> + case 2: > >> + val = ((const u16 *)data)[i]; > > Can't you do this in each branch below? > >> + if (endianness == IIO_BE) >> + val = be16_to_cpu(val); >> + else if (endianness == IIO_LE) >> + val = le16_to_cpu(val); >> + break; > > ... > >> + device_for_each_child_node(dev, child) { >> + ret = fwnode_property_read_u32(child, "reg", &i); >> + if (ret || i >= num_axes) { >> + dev_err(dev, "reg invalid or missing"); >> + goto err; >> + } >> + >> + if (fwnode_property_read_u32(child, "linux,code", >> + &axes[i].code)) { >> + dev_err(dev, "linux,code invalid or >> missing"); >> + goto err; >> + } >> + >> + if (fwnode_property_read_u32_array(child, >> "abs-range", >> + axes[i].range, >> 2)) { >> + dev_err(dev, "abs-range invalid or >> missing"); >> + goto err; >> + } > >> + } >> + >> + joy->axes = axes; >> + >> + return 0; >> + >> +err: >> + fwnode_handle_put(child); > >> + return -EINVAL; > > Can we avoid shadowing the actual error code? > > ... > >> + bits = joy->chans[0].channel->scan_type.storagebits; > >> + if (!bits || (bits >> 3) > 2) { > > Wouldn't be clear to use simple 'bits > 16'? > >> + dev_err(dev, "Unsupported channel storage size"); >> + return -EINVAL; >> + } > > ... > >> +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? > 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, doesn't mean that it shouldn't have a table to probe from devicetree. -Paul > >> + }, >> + .probe = adc_joystick_probe, >> +}; > > -- > With Best Regards, > Andy Shevchenko
On Sat, Apr 18, 2020 at 12:24 AM Paul Cercueil <paul@crapouillou.net> wrote: > Le sam. 18 avril 2020 à 0:10, Andy Shevchenko > <andy.shevchenko@gmail.com> a écrit : > > On Fri, Apr 17, 2020 at 11:21 PM Artur Rojek <contact@artur-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. > 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).
Le sam. 18 avril 2020 à 0:49, Andy Shevchenko <andy.shevchenko@gmail.com> a écrit : > On Sat, Apr 18, 2020 at 12:24 AM Paul Cercueil <paul@crapouillou.net> > wrote: >> Le sam. 18 avril 2020 à 0:10, Andy Shevchenko >> <andy.shevchenko@gmail.com> a écrit : >> > On Fri, Apr 17, 2020 at 11:21 PM Artur Rojek >> <contact@artur-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
On Sat, Apr 18, 2020 at 1:48 AM Paul Cercueil <paul@crapouillou.net> wrote: > > > > Le sam. 18 avril 2020 à 0:49, Andy Shevchenko > <andy.shevchenko@gmail.com> a écrit : > > On Sat, Apr 18, 2020 at 12:24 AM Paul Cercueil <paul@crapouillou.net> > > wrote: > >> Le sam. 18 avril 2020 à 0:10, Andy Shevchenko > >> <andy.shevchenko@gmail.com> a écrit : > >> > On Fri, Apr 17, 2020 at 11:21 PM Artur Rojek > >> <contact@artur-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. I think above has no relation to what I'm talking about. How *this* driver can work as a platform instantiated one? We seems have a conceptual misunderstanding here. For example, how can probe of this driver not fail, if it is not backed by a DT/ACPI properties? > >> 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. Table is fine(JFYI the types and sections are defined outside of OF stuff, though being [heavily] used by it) , API (of_match_ptr() macro use) is not.
Le sam. 18 avril 2020 à 14:57, Andy Shevchenko <andy.shevchenko@gmail.com> a écrit : > On Sat, Apr 18, 2020 at 1:48 AM Paul Cercueil <paul@crapouillou.net> > wrote: >> >> >> >> Le sam. 18 avril 2020 à 0:49, Andy Shevchenko >> <andy.shevchenko@gmail.com> a écrit : >> > On Sat, Apr 18, 2020 at 12:24 AM Paul Cercueil >> <paul@crapouillou.net> >> > wrote: >> >> Le sam. 18 avril 2020 à 0:10, Andy Shevchenko >> >> <andy.shevchenko@gmail.com> a écrit : >> >> > On Fri, Apr 17, 2020 at 11:21 PM Artur Rojek >> >> <contact@artur-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. > > I think above has no relation to what I'm talking about. Yes it does. It allows you to map the IIO channels, set the pinctrl configurations and register a device from platform code instead of devicetree. > How *this* driver can work as a platform instantiated one? > We seems have a conceptual misunderstanding here. > > For example, how can probe of this driver not fail, if it is not > backed by a DT/ACPI properties? platform_device_add_properties(). > >> >> 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. > > Table is fine(JFYI the types and sections are defined outside of OF > stuff, though being [heavily] used by it) , API (of_match_ptr() macro > use) is not. Sorry, but that's just stupid. Please have a look at how of_match_ptr() macro is defined in <linux/of.h>. -Paul
On Sat, Apr 18, 2020 at 3:10 PM Paul Cercueil <paul@crapouillou.net> wrote: > Le sam. 18 avril 2020 à 14:57, Andy Shevchenko > <andy.shevchenko@gmail.com> a écrit : > > On Sat, Apr 18, 2020 at 1:48 AM Paul Cercueil <paul@crapouillou.net> > > wrote: > >> Le sam. 18 avril 2020 à 0:49, Andy Shevchenko > >> <andy.shevchenko@gmail.com> a écrit : > >> > On Sat, Apr 18, 2020 at 12:24 AM Paul Cercueil > >> <paul@crapouillou.net> > >> > wrote: > >> >> Le sam. 18 avril 2020 à 0:10, Andy Shevchenko > >> >> <andy.shevchenko@gmail.com> a écrit : > >> >> > On Fri, Apr 17, 2020 at 11:21 PM Artur Rojek > >> >> <contact@artur-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. > > > > I think above has no relation to what I'm talking about. > > Yes it does. It allows you to map the IIO channels, set the pinctrl > configurations and register a device from platform code instead of > devicetree. I'm not talking about other drivers, I'm talking about this driver and how it will be instantiated. Above, according to the code, can't be comprehensive to fulfill this. > > How *this* driver can work as a platform instantiated one? > > We seems have a conceptual misunderstanding here. > > > > For example, how can probe of this driver not fail, if it is not > > backed by a DT/ACPI properties? > > platform_device_add_properties(). Yes, I waited for this. And seems you don't understand the (scope of) API, you are trying to insist this driver can be used as a platform one. Sorry, I must to disappoint you, it can't. Above interface is created solely for quirks to support (broken) DT/ACPI tables. It's not supposed to be used as a main source for the device properties. > >> >> 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. > > > > Table is fine(JFYI the types and sections are defined outside of OF > > stuff, though being [heavily] used by it) , API (of_match_ptr() macro > > use) is not. > > Sorry, but that's just stupid. Please have a look at how of_match_ptr() > macro is defined in <linux/of.h>. Call it whatever you want, but above code is broken. It needs either of: - ugly ifdeffery - dropping of_match_ptr() - explicit dependence to OF My choice is second one. Because it makes code better and allows also ACPI to use this driver (usually) without changes.
Le sam. 18 avril 2020 à 15:42, Andy Shevchenko <andy.shevchenko@gmail.com> a écrit : > On Sat, Apr 18, 2020 at 3:10 PM Paul Cercueil <paul@crapouillou.net> > wrote: >> Le sam. 18 avril 2020 à 14:57, Andy Shevchenko >> <andy.shevchenko@gmail.com> a écrit : >> > On Sat, Apr 18, 2020 at 1:48 AM Paul Cercueil >> <paul@crapouillou.net> >> > wrote: >> >> Le sam. 18 avril 2020 à 0:49, Andy Shevchenko >> >> <andy.shevchenko@gmail.com> a écrit : >> >> > On Sat, Apr 18, 2020 at 12:24 AM Paul Cercueil >> >> <paul@crapouillou.net> >> >> > wrote: >> >> >> Le sam. 18 avril 2020 à 0:10, Andy Shevchenko >> >> >> <andy.shevchenko@gmail.com> a écrit : >> >> >> > On Fri, Apr 17, 2020 at 11:21 PM Artur Rojek >> >> >> <contact@artur-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. >> > >> > I think above has no relation to what I'm talking about. >> >> Yes it does. It allows you to map the IIO channels, set the pinctrl >> configurations and register a device from platform code instead of >> devicetree. > > I'm not talking about other drivers, I'm talking about this driver and > how it will be instantiated. Above, according to the code, can't be > comprehensive to fulfill this. This is how the platform devices were instanciated on JZ4740 before we switched everything to devicetree. >> > How *this* driver can work as a platform instantiated one? >> > We seems have a conceptual misunderstanding here. >> > >> > For example, how can probe of this driver not fail, if it is not >> > backed by a DT/ACPI properties? >> >> platform_device_add_properties(). > > Yes, I waited for this. And seems you don't understand the (scope of) > API, you are trying to insist this driver can be used as a platform > one. > Sorry, I must to disappoint you, it can't. Above interface is created > solely for quirks to support (broken) DT/ACPI tables. It's not > supposed to be used as a main source for the device properties. The fact that it was designed for something else doesn't mean it can't be used. Anyway, this discussion is pointless. I don't think anybody would want to do that. >> >> >> 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. >> > >> > Table is fine(JFYI the types and sections are defined outside of >> OF >> > stuff, though being [heavily] used by it) , API (of_match_ptr() >> macro >> > use) is not. >> >> Sorry, but that's just stupid. Please have a look at how >> of_match_ptr() >> macro is defined in <linux/of.h>. > > Call it whatever you want, but above code is broken. of_match_ptr() is basically defined like this: #ifdef CONFIG_OF #define of_match_ptr(x) (x) #else #define of_match_ptr(x) NULL #endif So please, enlighten me, tell me what is so wrong about what's being done here. > It needs either of: > - ugly ifdeffery > - dropping of_match_ptr() > - explicit dependence to OF > > My choice is second one. Because it makes code better and allows also > ACPI to use this driver (usually) without changes. And how is unconditionally compiling the of_match_table make it magically probe from ACPI, without a acpi_match_table? -Paul
On Sat, 18 Apr 2020 15:24:58 +0200 Paul Cercueil <paul@crapouillou.net> wrote: > Le sam. 18 avril 2020 à 15:42, Andy Shevchenko > <andy.shevchenko@gmail.com> a écrit : > > On Sat, Apr 18, 2020 at 3:10 PM Paul Cercueil <paul@crapouillou.net> > > wrote: > >> Le sam. 18 avril 2020 à 14:57, Andy Shevchenko > >> <andy.shevchenko@gmail.com> a écrit : > >> > On Sat, Apr 18, 2020 at 1:48 AM Paul Cercueil > >> <paul@crapouillou.net> > >> > wrote: > >> >> Le sam. 18 avril 2020 à 0:49, Andy Shevchenko > >> >> <andy.shevchenko@gmail.com> a écrit : > >> >> > On Sat, Apr 18, 2020 at 12:24 AM Paul Cercueil > >> >> <paul@crapouillou.net> > >> >> > wrote: > >> >> >> Le sam. 18 avril 2020 à 0:10, Andy Shevchenko > >> >> >> <andy.shevchenko@gmail.com> a écrit : > >> >> >> > On Fri, Apr 17, 2020 at 11:21 PM Artur Rojek > >> >> >> <contact@artur-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. > >> > > >> > I think above has no relation to what I'm talking about. > >> > >> Yes it does. It allows you to map the IIO channels, set the pinctrl > >> configurations and register a device from platform code instead of > >> devicetree. > > > > I'm not talking about other drivers, I'm talking about this driver and > > how it will be instantiated. Above, according to the code, can't be > > comprehensive to fulfill this. > > This is how the platform devices were instanciated on JZ4740 before we > switched everything to devicetree. > > >> > How *this* driver can work as a platform instantiated one? > >> > We seems have a conceptual misunderstanding here. > >> > > >> > For example, how can probe of this driver not fail, if it is not > >> > backed by a DT/ACPI properties? > >> > >> platform_device_add_properties(). > > > > Yes, I waited for this. And seems you don't understand the (scope of) > > API, you are trying to insist this driver can be used as a platform > > one. > > Sorry, I must to disappoint you, it can't. Above interface is created > > solely for quirks to support (broken) DT/ACPI tables. It's not > > supposed to be used as a main source for the device properties. > > The fact that it was designed for something else doesn't mean it can't > be used. > > Anyway, this discussion is pointless. I don't think anybody would want > to do that. > > >> >> >> 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. > >> > > >> > Table is fine(JFYI the types and sections are defined outside of > >> OF > >> > stuff, though being [heavily] used by it) , API (of_match_ptr() > >> macro > >> > use) is not. > >> > >> Sorry, but that's just stupid. Please have a look at how > >> of_match_ptr() > >> macro is defined in <linux/of.h>. > > > > Call it whatever you want, but above code is broken. > > of_match_ptr() is basically defined like this: > > #ifdef CONFIG_OF > #define of_match_ptr(x) (x) > #else > #define of_match_ptr(x) NULL > #endif > > So please, enlighten me, tell me what is so wrong about what's being > done here. > > > It needs either of: > > - ugly ifdeffery > > - dropping of_match_ptr() > > - explicit dependence to OF > > > > My choice is second one. Because it makes code better and allows also > > ACPI to use this driver (usually) without changes. > > And how is unconditionally compiling the of_match_table make it > magically probe from ACPI, without a acpi_match_table? > > -Paul Look up PRP0001 ACPI ID. Magic trick ;) https://www.kernel.org/doc/html/latest/firmware-guide/acpi/enumeration.html?highlight=PRP0001 It allows you to define an ACPI device in DSDT that is instantiated from what is effectively the DT binding including the id table. Jonathan > >
Hi Jonathan, Le sam. 18 avril 2020 à 15:22, Jonathan Cameron <jic23@kernel.org> a écrit : > On Sat, 18 Apr 2020 15:24:58 +0200 > Paul Cercueil <paul@crapouillou.net> wrote: > >> Le sam. 18 avril 2020 à 15:42, Andy Shevchenko >> <andy.shevchenko@gmail.com> a écrit : >> > On Sat, Apr 18, 2020 at 3:10 PM Paul Cercueil >> <paul@crapouillou.net> >> > wrote: >> >> Le sam. 18 avril 2020 à 14:57, Andy Shevchenko >> >> <andy.shevchenko@gmail.com> a écrit : >> >> > On Sat, Apr 18, 2020 at 1:48 AM Paul Cercueil >> >> <paul@crapouillou.net> >> >> > wrote: >> >> >> Le sam. 18 avril 2020 à 0:49, Andy Shevchenko >> >> >> <andy.shevchenko@gmail.com> a écrit : >> >> >> > On Sat, Apr 18, 2020 at 12:24 AM Paul Cercueil >> >> >> <paul@crapouillou.net> >> >> >> > wrote: >> >> >> >> Le sam. 18 avril 2020 à 0:10, Andy Shevchenko >> >> >> >> <andy.shevchenko@gmail.com> a écrit : >> >> >> >> > On Fri, Apr 17, 2020 at 11:21 PM Artur Rojek >> >> >> >> <contact@artur-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. >> >> > >> >> > I think above has no relation to what I'm talking about. >> >> >> >> Yes it does. It allows you to map the IIO channels, set the >> pinctrl >> >> configurations and register a device from platform code instead >> of >> >> devicetree. >> > >> > I'm not talking about other drivers, I'm talking about this >> driver and >> > how it will be instantiated. Above, according to the code, can't >> be >> > comprehensive to fulfill this. >> >> This is how the platform devices were instanciated on JZ4740 before >> we >> switched everything to devicetree. >> >> >> > How *this* driver can work as a platform instantiated one? >> >> > We seems have a conceptual misunderstanding here. >> >> > >> >> > For example, how can probe of this driver not fail, if it is >> not >> >> > backed by a DT/ACPI properties? >> >> >> >> platform_device_add_properties(). >> > >> > Yes, I waited for this. And seems you don't understand the (scope >> of) >> > API, you are trying to insist this driver can be used as a >> platform >> > one. >> > Sorry, I must to disappoint you, it can't. Above interface is >> created >> > solely for quirks to support (broken) DT/ACPI tables. It's not >> > supposed to be used as a main source for the device properties. >> >> The fact that it was designed for something else doesn't mean it >> can't >> be used. >> >> Anyway, this discussion is pointless. I don't think anybody would >> want >> to do that. >> >> >> >> >> 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. >> >> > >> >> > Table is fine(JFYI the types and sections are defined outside >> of >> >> OF >> >> > stuff, though being [heavily] used by it) , API >> (of_match_ptr() >> >> macro >> >> > use) is not. >> >> >> >> Sorry, but that's just stupid. Please have a look at how >> >> of_match_ptr() >> >> macro is defined in <linux/of.h>. >> > >> > Call it whatever you want, but above code is broken. >> >> of_match_ptr() is basically defined like this: >> >> #ifdef CONFIG_OF >> #define of_match_ptr(x) (x) >> #else >> #define of_match_ptr(x) NULL >> #endif >> >> So please, enlighten me, tell me what is so wrong about what's being >> done here. >> >> > It needs either of: >> > - ugly ifdeffery >> > - dropping of_match_ptr() >> > - explicit dependence to OF >> > >> > My choice is second one. Because it makes code better and allows >> also >> > ACPI to use this driver (usually) without changes. >> >> And how is unconditionally compiling the of_match_table make it >> magically probe from ACPI, without a acpi_match_table? >> >> -Paul > > Look up PRP0001 ACPI ID. Magic trick ;) > > https://www.kernel.org/doc/html/latest/firmware-guide/acpi/enumeration.html?highlight=PRP0001 > > It allows you to define an ACPI device in DSDT that is instantiated > from what is effectively the DT binding including the id table. So what you're saying, is that the OF table should be present, even though CONFIG_OF is not set, just in case it is probed from ACPI? -Paul
On Sat, 18 Apr 2020 19:25:15 +0200 Paul Cercueil <paul@crapouillou.net> wrote: > Hi Jonathan, > > Le sam. 18 avril 2020 à 15:22, Jonathan Cameron <jic23@kernel.org> a > écrit : > > On Sat, 18 Apr 2020 15:24:58 +0200 > > Paul Cercueil <paul@crapouillou.net> wrote: > > > >> Le sam. 18 avril 2020 à 15:42, Andy Shevchenko > >> <andy.shevchenko@gmail.com> a écrit : > >> > On Sat, Apr 18, 2020 at 3:10 PM Paul Cercueil > >> <paul@crapouillou.net> > >> > wrote: > >> >> Le sam. 18 avril 2020 à 14:57, Andy Shevchenko > >> >> <andy.shevchenko@gmail.com> a écrit : > >> >> > On Sat, Apr 18, 2020 at 1:48 AM Paul Cercueil > >> >> <paul@crapouillou.net> > >> >> > wrote: > >> >> >> Le sam. 18 avril 2020 à 0:49, Andy Shevchenko > >> >> >> <andy.shevchenko@gmail.com> a écrit : > >> >> >> > On Sat, Apr 18, 2020 at 12:24 AM Paul Cercueil > >> >> >> <paul@crapouillou.net> > >> >> >> > wrote: > >> >> >> >> Le sam. 18 avril 2020 à 0:10, Andy Shevchenko > >> >> >> >> <andy.shevchenko@gmail.com> a écrit : > >> >> >> >> > On Fri, Apr 17, 2020 at 11:21 PM Artur Rojek > >> >> >> >> <contact@artur-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. > >> >> > > >> >> > I think above has no relation to what I'm talking about. > >> >> > >> >> Yes it does. It allows you to map the IIO channels, set the > >> pinctrl > >> >> configurations and register a device from platform code instead > >> of > >> >> devicetree. > >> > > >> > I'm not talking about other drivers, I'm talking about this > >> driver and > >> > how it will be instantiated. Above, according to the code, can't > >> be > >> > comprehensive to fulfill this. > >> > >> This is how the platform devices were instanciated on JZ4740 before > >> we > >> switched everything to devicetree. > >> > >> >> > How *this* driver can work as a platform instantiated one? > >> >> > We seems have a conceptual misunderstanding here. > >> >> > > >> >> > For example, how can probe of this driver not fail, if it is > >> not > >> >> > backed by a DT/ACPI properties? > >> >> > >> >> platform_device_add_properties(). > >> > > >> > Yes, I waited for this. And seems you don't understand the (scope > >> of) > >> > API, you are trying to insist this driver can be used as a > >> platform > >> > one. > >> > Sorry, I must to disappoint you, it can't. Above interface is > >> created > >> > solely for quirks to support (broken) DT/ACPI tables. It's not > >> > supposed to be used as a main source for the device properties. > >> > >> The fact that it was designed for something else doesn't mean it > >> can't > >> be used. > >> > >> Anyway, this discussion is pointless. I don't think anybody would > >> want > >> to do that. > >> > >> >> >> >> 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. > >> >> > > >> >> > Table is fine(JFYI the types and sections are defined outside > >> of > >> >> OF > >> >> > stuff, though being [heavily] used by it) , API > >> (of_match_ptr() > >> >> macro > >> >> > use) is not. > >> >> > >> >> Sorry, but that's just stupid. Please have a look at how > >> >> of_match_ptr() > >> >> macro is defined in <linux/of.h>. > >> > > >> > Call it whatever you want, but above code is broken. > >> > >> of_match_ptr() is basically defined like this: > >> > >> #ifdef CONFIG_OF > >> #define of_match_ptr(x) (x) > >> #else > >> #define of_match_ptr(x) NULL > >> #endif > >> > >> So please, enlighten me, tell me what is so wrong about what's being > >> done here. > >> > >> > It needs either of: > >> > - ugly ifdeffery > >> > - dropping of_match_ptr() > >> > - explicit dependence to OF > >> > > >> > My choice is second one. Because it makes code better and allows > >> also > >> > ACPI to use this driver (usually) without changes. > >> > >> And how is unconditionally compiling the of_match_table make it > >> magically probe from ACPI, without a acpi_match_table? > >> > >> -Paul > > > > Look up PRP0001 ACPI ID. Magic trick ;) > > > > https://www.kernel.org/doc/html/latest/firmware-guide/acpi/enumeration.html?highlight=PRP0001 > > > > It allows you to define an ACPI device in DSDT that is instantiated > > from what is effectively the DT binding including the id table. > > So what you're saying, is that the OF table should be present, even > though CONFIG_OF is not set, just in case it is probed from ACPI? Exactly. Weird isn't it :) > > -Paul > >
diff --git a/drivers/input/joystick/Kconfig b/drivers/input/joystick/Kconfig index 940b744639c7..efbc20ec5099 100644 --- a/drivers/input/joystick/Kconfig +++ b/drivers/input/joystick/Kconfig @@ -42,6 +42,16 @@ config JOYSTICK_A3D To compile this driver as a module, choose M here: the module will be called a3d. +config JOYSTICK_ADC + tristate "Simple joystick connected over ADC" + depends on IIO + select IIO_BUFFER_CB + help + Say Y here if you have a simple joystick connected over ADC. + + To compile this driver as a module, choose M here: the + module will be called adc-joystick. + config JOYSTICK_ADI tristate "Logitech ADI digital joysticks and gamepads" select GAMEPORT diff --git a/drivers/input/joystick/Makefile b/drivers/input/joystick/Makefile index 8656023f6ef5..58232b3057d3 100644 --- a/drivers/input/joystick/Makefile +++ b/drivers/input/joystick/Makefile @@ -6,6 +6,7 @@ # Each configuration option enables a list of files. obj-$(CONFIG_JOYSTICK_A3D) += a3d.o +obj-$(CONFIG_JOYSTICK_ADC) += adc-joystick.o obj-$(CONFIG_JOYSTICK_ADI) += adi.o obj-$(CONFIG_JOYSTICK_AMIGA) += amijoy.o obj-$(CONFIG_JOYSTICK_AS5011) += as5011.o diff --git a/drivers/input/joystick/adc-joystick.c b/drivers/input/joystick/adc-joystick.c new file mode 100644 index 000000000000..9cb9896da26e --- /dev/null +++ b/drivers/input/joystick/adc-joystick.c @@ -0,0 +1,245 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Input driver for joysticks connected over ADC. + * Copyright (c) 2019-2020 Artur Rojek <contact@artur-rojek.eu> + */ +#include <linux/ctype.h> +#include <linux/input.h> +#include <linux/iio/iio.h> +#include <linux/iio/consumer.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/platform_device.h> +#include <linux/property.h> + +struct adc_joystick_axis { + u32 code; + s32 range[2]; + s32 fuzz; + s32 flat; +}; + +struct adc_joystick { + struct input_dev *input; + struct iio_cb_buffer *buffer; + struct adc_joystick_axis *axes; + struct iio_channel *chans; + int num_chans; +}; + +static int adc_joystick_handle(const void *data, void *private) +{ + struct adc_joystick *joy = private; + enum iio_endian endianness; + int bytes, msb, val, i; + bool sign; + + bytes = joy->chans[0].channel->scan_type.storagebits >> 3; + + for (i = 0; i < joy->num_chans; ++i) { + endianness = joy->chans[i].channel->scan_type.endianness; + msb = joy->chans[i].channel->scan_type.realbits - 1; + sign = (tolower(joy->chans[i].channel->scan_type.sign) == 's'); + + switch (bytes) { + case 1: + val = ((const u8 *)data)[i]; + break; + case 2: + val = ((const u16 *)data)[i]; + if (endianness == IIO_BE) + val = be16_to_cpu(val); + else if (endianness == IIO_LE) + val = le16_to_cpu(val); + break; + default: + return -EINVAL; + } + + val >>= joy->chans[i].channel->scan_type.shift; + if (sign) + val = sign_extend32(val, msb); + else + val &= GENMASK(msb, 0); + input_report_abs(joy->input, joy->axes[i].code, val); + } + + input_sync(joy->input); + + return 0; +} + +static int adc_joystick_open(struct input_dev *dev) +{ + struct adc_joystick *joy = input_get_drvdata(dev); + int ret; + + ret = iio_channel_start_all_cb(joy->buffer); + if (ret) + dev_err(dev->dev.parent, "Unable to start callback buffer"); + + return ret; +} + +static void adc_joystick_close(struct input_dev *dev) +{ + struct adc_joystick *joy = input_get_drvdata(dev); + + iio_channel_stop_all_cb(joy->buffer); +} + +static void adc_joystick_cleanup(void *data) +{ + iio_channel_release_all_cb(data); +} + +static int adc_joystick_set_axes(struct device *dev, struct adc_joystick *joy) +{ + struct adc_joystick_axis *axes; + struct fwnode_handle *child; + int num_axes, ret, i; + + num_axes = device_get_child_node_count(dev); + if (!num_axes) { + dev_err(dev, "Unable to find child nodes"); + return -EINVAL; + } + + if (num_axes != joy->num_chans) { + dev_err(dev, "Got %d child nodes for %d channels", + num_axes, joy->num_chans); + return -EINVAL; + } + + axes = devm_kmalloc_array(dev, num_axes, sizeof(*axes), GFP_KERNEL); + if (!axes) + return -ENOMEM; + + device_for_each_child_node(dev, child) { + ret = fwnode_property_read_u32(child, "reg", &i); + if (ret || i >= num_axes) { + dev_err(dev, "reg invalid or missing"); + goto err; + } + + if (fwnode_property_read_u32(child, "linux,code", + &axes[i].code)) { + dev_err(dev, "linux,code invalid or missing"); + goto err; + } + + if (fwnode_property_read_u32_array(child, "abs-range", + axes[i].range, 2)) { + dev_err(dev, "abs-range invalid or missing"); + goto err; + } + + fwnode_property_read_u32(child, "abs-fuzz", + &axes[i].fuzz); + fwnode_property_read_u32(child, "abs-flat", + &axes[i].flat); + + input_set_abs_params(joy->input, axes[i].code, + axes[i].range[0], axes[i].range[1], + axes[i].fuzz, + axes[i].flat); + input_set_capability(joy->input, EV_ABS, axes[i].code); + } + + joy->axes = axes; + + return 0; + +err: + fwnode_handle_put(child); + return -EINVAL; +} + +static int adc_joystick_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct adc_joystick *joy; + struct input_dev *input; + int bits, ret, i; + + joy = devm_kzalloc(dev, sizeof(*joy), GFP_KERNEL); + if (!joy) + return -ENOMEM; + + joy->chans = devm_iio_channel_get_all(dev); + if (IS_ERR(joy->chans)) { + ret = PTR_ERR(joy->chans); + if (ret != -EPROBE_DEFER) + dev_err(dev, "Unable to get IIO channels"); + return ret; + } + + /* Count how many channels we got. NULL terminated. */ + while (joy->chans[joy->num_chans].indio_dev) + joy->num_chans++; + + bits = joy->chans[0].channel->scan_type.storagebits; + if (!bits || (bits >> 3) > 2) { + dev_err(dev, "Unsupported channel storage size"); + return -EINVAL; + } + for (i = 1; i < joy->num_chans; ++i) + if (joy->chans[i].channel->scan_type.storagebits != bits) { + dev_err(dev, "Channels must have equal storage size"); + return -EINVAL; + } + + input = devm_input_allocate_device(dev); + if (!input) { + dev_err(dev, "Unable to allocate input device"); + return -ENOMEM; + } + + joy->input = input; + input->name = pdev->name; + input->id.bustype = BUS_HOST; + input->open = adc_joystick_open; + input->close = adc_joystick_close; + + ret = adc_joystick_set_axes(dev, joy); + if (ret) + return ret; + + input_set_drvdata(input, joy); + ret = input_register_device(input); + if (ret) { + dev_err(dev, "Unable to register input device: %d", ret); + return ret; + } + + joy->buffer = iio_channel_get_all_cb(dev, adc_joystick_handle, joy); + if (IS_ERR(joy->buffer)) { + dev_err(dev, "Unable to allocate callback buffer"); + return PTR_ERR(joy->buffer); + } + + ret = devm_add_action_or_reset(dev, adc_joystick_cleanup, joy->buffer); + if (ret) + dev_err(dev, "Unable to add action"); + + return ret; +} + +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), + }, + .probe = adc_joystick_probe, +}; +module_platform_driver(adc_joystick_driver); + +MODULE_DESCRIPTION("Input driver for joysticks connected over ADC"); +MODULE_AUTHOR("Artur Rojek <contact@artur-rojek.eu>"); +MODULE_LICENSE("GPL");