| Message ID | 1422291516-24895-1-git-send-email-plaes@plaes.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On Mon, Jan 26, 2015 at 06:58:32PM +0200, Priit Laes wrote: > --- Like Hans was pointing out, commit log and signed-off-by please > .../ABI/testing/sysfs-driver-input-sun4i-lradc | 4 ++ > drivers/input/keyboard/sun4i-lradc-keys.c | 49 +++++++++++++++++----- > 2 files changed, 43 insertions(+), 10 deletions(-) > create mode 100644 Documentation/ABI/testing/sysfs-driver-input-sun4i-lradc > > diff --git a/Documentation/ABI/testing/sysfs-driver-input-sun4i-lradc b/Documentation/ABI/testing/sysfs-driver-input-sun4i-lradc > new file mode 100644 > index 0000000..e4e6448 > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-driver-input-sun4i-lradc > @@ -0,0 +1,4 @@ > +What: /sys/class/input/input(x)/device/voltage > +Date: February 2015 > +Contact: Priit Laes <plaes@plaes.org> > +Description: ADC output voltage in microvolts or 0 if device is not opened. Why is it returning 0 when "device is not opened" ? What does that even mean? You can't read that file without opening it. > diff --git a/drivers/input/keyboard/sun4i-lradc-keys.c b/drivers/input/keyboard/sun4i-lradc-keys.c > index cc8f7dd..c0ab8ec 100644 > --- a/drivers/input/keyboard/sun4i-lradc-keys.c > +++ b/drivers/input/keyboard/sun4i-lradc-keys.c > @@ -79,10 +79,27 @@ struct sun4i_lradc_data { > u32 vref; > }; > > +static u32 sun4i_lradc_read_voltage(struct sun4i_lradc_data *lradc) > +{ > + u32 val = readl(lradc->base + LRADC_DATA0) & 0x3f; > + return val * lradc->vref / 63; > +}; > + > +static ssize_t > +sun4i_lradc_dev_voltage_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct sun4i_lradc_data *lradc = dev_get_drvdata(dev); > + > + return sprintf(buf, "%u\n", sun4i_lradc_read_voltage(lradc)); > +} > + > +static const DEVICE_ATTR(voltage, S_IRUGO, sun4i_lradc_dev_voltage_show, NULL); > + > static irqreturn_t sun4i_lradc_irq(int irq, void *dev_id) > { > struct sun4i_lradc_data *lradc = dev_id; > - u32 i, ints, val, voltage, diff, keycode = 0, closest = 0xffffffff; > + u32 i, ints, voltage, diff, keycode = 0, closest = 0xffffffff; > > ints = readl(lradc->base + LRADC_INTS); > > @@ -97,8 +114,7 @@ static irqreturn_t sun4i_lradc_irq(int irq, void *dev_id) > } > > if ((ints & CHAN0_KEYDOWN_IRQ) && lradc->chan0_keycode == 0) { > - val = readl(lradc->base + LRADC_DATA0) & 0x3f; > - voltage = val * lradc->vref / 63; > + voltage = sun4i_lradc_read_voltage(lradc); > > for (i = 0; i < lradc->chan0_map_count; i++) { > diff = abs(lradc->chan0_map[i].voltage - voltage); > @@ -156,7 +172,7 @@ static void sun4i_lradc_close(struct input_dev *dev) > } > > static int sun4i_lradc_load_dt_keymap(struct device *dev, > - struct sun4i_lradc_data *lradc) > + struct sun4i_lradc_data *lradc) > { > struct device_node *np, *pp; > int i; > @@ -168,8 +184,8 @@ static int sun4i_lradc_load_dt_keymap(struct device *dev, > > lradc->chan0_map_count = of_get_child_count(np); > if (lradc->chan0_map_count == 0) { > - dev_err(dev, "keymap is missing in device tree\n"); > - return -EINVAL; > + dev_info(dev, "keymap is missing in device tree\n"); > + return 0; > } > > lradc->chan0_map = devm_kmalloc_array(dev, lradc->chan0_map_count, > @@ -185,19 +201,19 @@ static int sun4i_lradc_load_dt_keymap(struct device *dev, > > error = of_property_read_u32(pp, "channel", &channel); > if (error || channel != 0) { > - dev_err(dev, "%s: Inval channel prop\n", pp->name); > + dev_err(dev, "%s: Invalid 'channel' property\n", pp->name); > return -EINVAL; > } > > error = of_property_read_u32(pp, "voltage", &map->voltage); > if (error) { > - dev_err(dev, "%s: Inval voltage prop\n", pp->name); > + dev_err(dev, "%s: Invalid 'voltage' property\n", pp->name); > return -EINVAL; > } > > error = of_property_read_u32(pp, "linux,code", &map->keycode); > if (error) { > - dev_err(dev, "%s: Inval linux,code prop\n", pp->name); > + dev_err(dev, "%s: Invalid 'linux,code' property\n", pp->name); > return -EINVAL; > } > > @@ -257,14 +273,26 @@ static int sun4i_lradc_probe(struct platform_device *pdev) > if (error) > return error; > > - error = input_register_device(lradc->input); > + error = device_create_file(dev, &dev_attr_voltage); As I told you already, if you're going to expose this an ADC in the end, the proper solution is to use the IIO framework, not adding a custom sysfs file. Maxime
On Tue, 2015-01-27 at 10:18 +0100, Maxime Ripard wrote: > Hi, > > On Mon, Jan 26, 2015 at 06:58:32PM +0200, Priit Laes wrote: > > --- > > Like Hans was pointing out, commit log and signed-off-by please > > > .../ABI/testing/sysfs-driver-input-sun4i-lradc | 4 ++ > > drivers/input/keyboard/sun4i-lradc-keys.c | 49 > > +++++++++++++++++----- > > 2 files changed, 43 insertions(+), 10 deletions(-) > > create mode 100644 Documentation/ABI/testing/sysfs-driver-input- > > sun4i-lradc > > > > diff --git a/Documentation/ABI/testing/sysfs-driver-input-sun4i- > > lradc b/Documentation/ABI/testing/sysfs-driver-input-sun4i-lradc > > new file mode 100644 > > index 0000000..e4e6448 > > --- /dev/null > > +++ b/Documentation/ABI/testing/sysfs-driver-input-sun4i-lradc > > @@ -0,0 +1,4 @@ > > +What: /sys/class/input/input(x)/device/voltage > > +Date: February 2015 > > +Contact: Priit Laes <plaes@plaes.org> > > +Description: ADC output voltage in microvolts or 0 if device is > > not opened. > > Why is it returning 0 when "device is not opened" ? What does that > even mean? You can't read that file without opening it. It means that something has to open the /dev/input/inputX device which sets up the ADC before the voltage can be read from the sysfs file. [...] > > As I told you already, if you're going to expose this an ADC in the > end, the proper solution is to use the IIO framework, not adding a > custom sysfs file. My intention was to expose just a simple debug output, so one can press the buttons and read the voltages for devicetree keymap. If anyone can suggest a simpler approach than current sysfs based one, I would do it. But full blown iio driver is currently out of scope. Also, Carlo's (ccaione) initially submitted (?) driver for lradc utilized iio subsystem. Päikest, Priit :) -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On 27-01-15 10:49, Priit Laes wrote: > > On Tue, 2015-01-27 at 10:18 +0100, Maxime Ripard wrote: >> Hi, >> >> On Mon, Jan 26, 2015 at 06:58:32PM +0200, Priit Laes wrote: >>> --- >> >> Like Hans was pointing out, commit log and signed-off-by please >> >>> .../ABI/testing/sysfs-driver-input-sun4i-lradc | 4 ++ >>> drivers/input/keyboard/sun4i-lradc-keys.c | 49 >>> +++++++++++++++++----- >>> 2 files changed, 43 insertions(+), 10 deletions(-) >>> create mode 100644 Documentation/ABI/testing/sysfs-driver-input- >>> sun4i-lradc >>> >>> diff --git a/Documentation/ABI/testing/sysfs-driver-input-sun4i- >>> lradc b/Documentation/ABI/testing/sysfs-driver-input-sun4i-lradc >>> new file mode 100644 >>> index 0000000..e4e6448 >>> --- /dev/null >>> +++ b/Documentation/ABI/testing/sysfs-driver-input-sun4i-lradc >>> @@ -0,0 +1,4 @@ >>> +What: /sys/class/input/input(x)/device/voltage >>> +Date: February 2015 >>> +Contact: Priit Laes <plaes@plaes.org> >>> +Description: ADC output voltage in microvolts or 0 if device is >>> not opened. >> >> Why is it returning 0 when "device is not opened" ? What does that >> even mean? You can't read that file without opening it. > > It means that something has to open the /dev/input/inputX device which > sets up the ADC before the voltage can be read from the sysfs file. > > [...] > > >> >> As I told you already, if you're going to expose this an ADC in the >> end, the proper solution is to use the IIO framework, not adding a >> custom sysfs file. > > My intention was to expose just a simple debug output, so one can > press the buttons and read the voltages for devicetree keymap. > > If anyone can suggest a simpler approach than current sysfs based one, > I would do it. The android driver always uses 0.2V / 200mV steps, so what I do is simply create a mapping with 200mV mapped to KEY_VOLUMEUP, 400mV mapped to KEY_VOLUMEDOWN, etc. following the hardcoded android driver mapping: https://github.com/linux-sunxi/linux-sunxi/blob/sunxi-3.4/drivers/input/keyboard/sun4i-keyboard.c#L136 Usually this will be correct in one go, after testing one can shuffle key codes as needed (usually not needed) and/or remove unused entries. With that said I do think that a sysfs file to see the actual voltages, or a kernel parameter to printk them on keypress interrupt would be useful. I guess the printk option would be better as it would show the actual keypress value read, not some semi-random sample. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jan 27, 2015 at 11:49:49AM +0200, Priit Laes wrote: > > On Tue, 2015-01-27 at 10:18 +0100, Maxime Ripard wrote: > > Hi, > > > > On Mon, Jan 26, 2015 at 06:58:32PM +0200, Priit Laes wrote: > > > --- > > > > Like Hans was pointing out, commit log and signed-off-by please > > > > > .../ABI/testing/sysfs-driver-input-sun4i-lradc | 4 ++ > > > drivers/input/keyboard/sun4i-lradc-keys.c | 49 > > > +++++++++++++++++----- > > > 2 files changed, 43 insertions(+), 10 deletions(-) > > > create mode 100644 Documentation/ABI/testing/sysfs-driver-input- > > > sun4i-lradc > > > > > > diff --git a/Documentation/ABI/testing/sysfs-driver-input-sun4i- > > > lradc b/Documentation/ABI/testing/sysfs-driver-input-sun4i-lradc > > > new file mode 100644 > > > index 0000000..e4e6448 > > > --- /dev/null > > > +++ b/Documentation/ABI/testing/sysfs-driver-input-sun4i-lradc > > > @@ -0,0 +1,4 @@ > > > +What: /sys/class/input/input(x)/device/voltage > > > +Date: February 2015 > > > +Contact: Priit Laes <plaes@plaes.org> > > > +Description: ADC output voltage in microvolts or 0 if device is > > > not opened. > > > > Why is it returning 0 when "device is not opened" ? What does that > > even mean? You can't read that file without opening it. > > It means that something has to open the /dev/input/inputX device which > sets up the ADC before the voltage can be read from the sysfs file. I'd consider this a bug. Why someone has to use an unrelated interface for another interface to work correctly? > > [...] > > > > > > As I told you already, if you're going to expose this an ADC in the > > end, the proper solution is to use the IIO framework, not adding a > > custom sysfs file. > > My intention was to expose just a simple debug output, so one can > press the buttons and read the voltages for devicetree keymap. > > If anyone can suggest a simpler approach than current sysfs based one, > I would do it. But full blown iio driver is currently out of scope. > Also, Carlo's (ccaione) initially submitted (?) driver for lradc > utilized iio subsystem. For stuff like this debugfs (AKA dumping ground) is better suited as it does not establish ABI. i Thanks.
On Tue, Jan 27, 2015 at 11:49:49AM +0200, Priit Laes wrote: > > On Tue, 2015-01-27 at 10:18 +0100, Maxime Ripard wrote: > > Hi, > > > > On Mon, Jan 26, 2015 at 06:58:32PM +0200, Priit Laes wrote: > > > --- > > > > Like Hans was pointing out, commit log and signed-off-by please > > > > > .../ABI/testing/sysfs-driver-input-sun4i-lradc | 4 ++ > > > drivers/input/keyboard/sun4i-lradc-keys.c | 49 > > > +++++++++++++++++----- > > > 2 files changed, 43 insertions(+), 10 deletions(-) > > > create mode 100644 Documentation/ABI/testing/sysfs-driver-input- > > > sun4i-lradc > > > > > > diff --git a/Documentation/ABI/testing/sysfs-driver-input-sun4i- > > > lradc b/Documentation/ABI/testing/sysfs-driver-input-sun4i-lradc > > > new file mode 100644 > > > index 0000000..e4e6448 > > > --- /dev/null > > > +++ b/Documentation/ABI/testing/sysfs-driver-input-sun4i-lradc > > > @@ -0,0 +1,4 @@ > > > +What: /sys/class/input/input(x)/device/voltage > > > +Date: February 2015 > > > +Contact: Priit Laes <plaes@plaes.org> > > > +Description: ADC output voltage in microvolts or 0 if device is > > > not opened. > > > > Why is it returning 0 when "device is not opened" ? What does that > > even mean? You can't read that file without opening it. > > It means that something has to open the /dev/input/inputX device which > sets up the ADC before the voltage can be read from the sysfs file. It's a bug. > > As I told you already, if you're going to expose this an ADC in the > > end, the proper solution is to use the IIO framework, not adding a > > custom sysfs file. > > My intention was to expose just a simple debug output, so one can > press the buttons and read the voltages for devicetree keymap. > > If anyone can suggest a simpler approach than current sysfs based one, > I would do it. But full blown iio driver is currently out of scope. For this kind of ADCs, it's really not that hard, and provide more or less the same interface. > Also, Carlo's (ccaione) initially submitted (?) driver for lradc > utilized iio subsystem. And it was dropped because no-one-would-use-this-to-retrieve-the-voltage-ever :) Maxime
On Tue, Jan 27, 2015 at 11:52:34AM +0100, Hans de Goede wrote: > Hi, > > On 27-01-15 10:49, Priit Laes wrote: > > > >On Tue, 2015-01-27 at 10:18 +0100, Maxime Ripard wrote: > >>Hi, > >> > >>On Mon, Jan 26, 2015 at 06:58:32PM +0200, Priit Laes wrote: > >>>--- > >> > >>Like Hans was pointing out, commit log and signed-off-by please > >> > >>> .../ABI/testing/sysfs-driver-input-sun4i-lradc | 4 ++ > >>> drivers/input/keyboard/sun4i-lradc-keys.c | 49 > >>>+++++++++++++++++----- > >>> 2 files changed, 43 insertions(+), 10 deletions(-) > >>> create mode 100644 Documentation/ABI/testing/sysfs-driver-input- > >>>sun4i-lradc > >>> > >>>diff --git a/Documentation/ABI/testing/sysfs-driver-input-sun4i- > >>>lradc b/Documentation/ABI/testing/sysfs-driver-input-sun4i-lradc > >>>new file mode 100644 > >>>index 0000000..e4e6448 > >>>--- /dev/null > >>>+++ b/Documentation/ABI/testing/sysfs-driver-input-sun4i-lradc > >>>@@ -0,0 +1,4 @@ > >>>+What: /sys/class/input/input(x)/device/voltage > >>>+Date: February 2015 > >>>+Contact: Priit Laes <plaes@plaes.org> > >>>+Description: ADC output voltage in microvolts or 0 if device is > >>>not opened. > >> > >>Why is it returning 0 when "device is not opened" ? What does that > >>even mean? You can't read that file without opening it. > > > >It means that something has to open the /dev/input/inputX device which > >sets up the ADC before the voltage can be read from the sysfs file. > > > >[...] > > > > > >> > >>As I told you already, if you're going to expose this an ADC in the > >>end, the proper solution is to use the IIO framework, not adding a > >>custom sysfs file. > > > >My intention was to expose just a simple debug output, so one can > >press the buttons and read the voltages for devicetree keymap. > > > >If anyone can suggest a simpler approach than current sysfs based one, > >I would do it. > > The android driver always uses 0.2V / 200mV steps, so what I do is > simply create a mapping with 200mV mapped to KEY_VOLUMEUP, 400mV mapped > to KEY_VOLUMEDOWN, etc. following the hardcoded android driver mapping: > > https://github.com/linux-sunxi/linux-sunxi/blob/sunxi-3.4/drivers/input/keyboard/sun4i-keyboard.c#L136 > > Usually this will be correct in one go, after testing one can shuffle > key codes as needed (usually not needed) and/or remove unused entries. > > With that said I do think that a sysfs file to see the actual voltages, > or a kernel parameter to printk them on keypress interrupt would be useful. > > I guess the printk option would be better as it would show the actual > keypress value read, not some semi-random sample. That wouldn't require that much code actually. Either using dev_dbg, or debugfs like Dmitry was suggesting would be two nice solutions I guess. Maxime
On Tue, Jan 27, 2015 at 08:44:47PM +0100, Maxime Ripard wrote: > On Tue, Jan 27, 2015 at 11:52:34AM +0100, Hans de Goede wrote: > > Hi, > > > > On 27-01-15 10:49, Priit Laes wrote: > > > > > >On Tue, 2015-01-27 at 10:18 +0100, Maxime Ripard wrote: > > >>Hi, > > >> > > >>On Mon, Jan 26, 2015 at 06:58:32PM +0200, Priit Laes wrote: > > >>>--- > > >> > > >>Like Hans was pointing out, commit log and signed-off-by please > > >> > > >>> .../ABI/testing/sysfs-driver-input-sun4i-lradc | 4 ++ > > >>> drivers/input/keyboard/sun4i-lradc-keys.c | 49 > > >>>+++++++++++++++++----- > > >>> 2 files changed, 43 insertions(+), 10 deletions(-) > > >>> create mode 100644 Documentation/ABI/testing/sysfs-driver-input- > > >>>sun4i-lradc > > >>> > > >>>diff --git a/Documentation/ABI/testing/sysfs-driver-input-sun4i- > > >>>lradc b/Documentation/ABI/testing/sysfs-driver-input-sun4i-lradc > > >>>new file mode 100644 > > >>>index 0000000..e4e6448 > > >>>--- /dev/null > > >>>+++ b/Documentation/ABI/testing/sysfs-driver-input-sun4i-lradc > > >>>@@ -0,0 +1,4 @@ > > >>>+What: /sys/class/input/input(x)/device/voltage > > >>>+Date: February 2015 > > >>>+Contact: Priit Laes <plaes@plaes.org> > > >>>+Description: ADC output voltage in microvolts or 0 if device is > > >>>not opened. > > >> > > >>Why is it returning 0 when "device is not opened" ? What does that > > >>even mean? You can't read that file without opening it. > > > > > >It means that something has to open the /dev/input/inputX device which > > >sets up the ADC before the voltage can be read from the sysfs file. > > > > > >[...] > > > > > > > > >> > > >>As I told you already, if you're going to expose this an ADC in the > > >>end, the proper solution is to use the IIO framework, not adding a > > >>custom sysfs file. > > > > > >My intention was to expose just a simple debug output, so one can > > >press the buttons and read the voltages for devicetree keymap. > > > > > >If anyone can suggest a simpler approach than current sysfs based one, > > >I would do it. > > > > The android driver always uses 0.2V / 200mV steps, so what I do is > > simply create a mapping with 200mV mapped to KEY_VOLUMEUP, 400mV mapped > > to KEY_VOLUMEDOWN, etc. following the hardcoded android driver mapping: > > > > https://github.com/linux-sunxi/linux-sunxi/blob/sunxi-3.4/drivers/input/keyboard/sun4i-keyboard.c#L136 > > > > Usually this will be correct in one go, after testing one can shuffle > > key codes as needed (usually not needed) and/or remove unused entries. > > > > With that said I do think that a sysfs file to see the actual voltages, > > or a kernel parameter to printk them on keypress interrupt would be useful. > > > > I guess the printk option would be better as it would show the actual > > keypress value read, not some semi-random sample. > > That wouldn't require that much code actually. Either using dev_dbg, > or debugfs like Dmitry was suggesting would be two nice solutions I > guess. Given the stated purpose I'd say dev_dbg() and call it a day. Thanks.
diff --git a/Documentation/ABI/testing/sysfs-driver-input-sun4i-lradc b/Documentation/ABI/testing/sysfs-driver-input-sun4i-lradc new file mode 100644 index 0000000..e4e6448 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-driver-input-sun4i-lradc @@ -0,0 +1,4 @@ +What: /sys/class/input/input(x)/device/voltage +Date: February 2015 +Contact: Priit Laes <plaes@plaes.org> +Description: ADC output voltage in microvolts or 0 if device is not opened. diff --git a/drivers/input/keyboard/sun4i-lradc-keys.c b/drivers/input/keyboard/sun4i-lradc-keys.c index cc8f7dd..c0ab8ec 100644 --- a/drivers/input/keyboard/sun4i-lradc-keys.c +++ b/drivers/input/keyboard/sun4i-lradc-keys.c @@ -79,10 +79,27 @@ struct sun4i_lradc_data { u32 vref; }; +static u32 sun4i_lradc_read_voltage(struct sun4i_lradc_data *lradc) +{ + u32 val = readl(lradc->base + LRADC_DATA0) & 0x3f; + return val * lradc->vref / 63; +}; + +static ssize_t +sun4i_lradc_dev_voltage_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct sun4i_lradc_data *lradc = dev_get_drvdata(dev); + + return sprintf(buf, "%u\n", sun4i_lradc_read_voltage(lradc)); +} + +static const DEVICE_ATTR(voltage, S_IRUGO, sun4i_lradc_dev_voltage_show, NULL); + static irqreturn_t sun4i_lradc_irq(int irq, void *dev_id) { struct sun4i_lradc_data *lradc = dev_id; - u32 i, ints, val, voltage, diff, keycode = 0, closest = 0xffffffff; + u32 i, ints, voltage, diff, keycode = 0, closest = 0xffffffff; ints = readl(lradc->base + LRADC_INTS); @@ -97,8 +114,7 @@ static irqreturn_t sun4i_lradc_irq(int irq, void *dev_id) } if ((ints & CHAN0_KEYDOWN_IRQ) && lradc->chan0_keycode == 0) { - val = readl(lradc->base + LRADC_DATA0) & 0x3f; - voltage = val * lradc->vref / 63; + voltage = sun4i_lradc_read_voltage(lradc); for (i = 0; i < lradc->chan0_map_count; i++) { diff = abs(lradc->chan0_map[i].voltage - voltage); @@ -156,7 +172,7 @@ static void sun4i_lradc_close(struct input_dev *dev) } static int sun4i_lradc_load_dt_keymap(struct device *dev, - struct sun4i_lradc_data *lradc) + struct sun4i_lradc_data *lradc) { struct device_node *np, *pp; int i; @@ -168,8 +184,8 @@ static int sun4i_lradc_load_dt_keymap(struct device *dev, lradc->chan0_map_count = of_get_child_count(np); if (lradc->chan0_map_count == 0) { - dev_err(dev, "keymap is missing in device tree\n"); - return -EINVAL; + dev_info(dev, "keymap is missing in device tree\n"); + return 0; } lradc->chan0_map = devm_kmalloc_array(dev, lradc->chan0_map_count, @@ -185,19 +201,19 @@ static int sun4i_lradc_load_dt_keymap(struct device *dev, error = of_property_read_u32(pp, "channel", &channel); if (error || channel != 0) { - dev_err(dev, "%s: Inval channel prop\n", pp->name); + dev_err(dev, "%s: Invalid 'channel' property\n", pp->name); return -EINVAL; } error = of_property_read_u32(pp, "voltage", &map->voltage); if (error) { - dev_err(dev, "%s: Inval voltage prop\n", pp->name); + dev_err(dev, "%s: Invalid 'voltage' property\n", pp->name); return -EINVAL; } error = of_property_read_u32(pp, "linux,code", &map->keycode); if (error) { - dev_err(dev, "%s: Inval linux,code prop\n", pp->name); + dev_err(dev, "%s: Invalid 'linux,code' property\n", pp->name); return -EINVAL; } @@ -257,14 +273,26 @@ static int sun4i_lradc_probe(struct platform_device *pdev) if (error) return error; - error = input_register_device(lradc->input); + error = device_create_file(dev, &dev_attr_voltage); if (error) return error; + error = input_register_device(lradc->input); + if (error) { + device_remove_file(&pdev->dev, &dev_attr_voltage); + return error; + } + platform_set_drvdata(pdev, lradc); return 0; } +static int sun4i_lradc_remove(struct platform_device *pdev) +{ + device_remove_file(&pdev->dev, &dev_attr_voltage); + return 0; +} + static const struct of_device_id sun4i_lradc_of_match[] = { { .compatible = "allwinner,sun4i-a10-lradc-keys", }, { /* sentinel */ } @@ -277,6 +305,7 @@ static struct platform_driver sun4i_lradc_driver = { .of_match_table = of_match_ptr(sun4i_lradc_of_match), }, .probe = sun4i_lradc_probe, + .remove = sun4i_lradc_remove, }; module_platform_driver(sun4i_lradc_driver);