Hi Dimitry,
Sorry for the (long) delay, I did not have time to work on it. I'll
mainly work in my free time now.
On 20/07/2016 19:25, Dmitry Torokhov wrote:
> Hi Quentin,
>
> On Wed, Jul 20, 2016 at 10:29:10AM +0200, Quentin Schulz wrote:
>> This adds support for Allwinner SoCs' (A10, A13 and A31) resistive
>> touchscreen. This driver is probed by the MFD sunxi-gpadc-mfd.
>>
>> This driver uses ADC channels exposed through the IIO framework by
>> sunxi-gpadc-iio to get its data. When opening this input device, it will
>> start buffering in the ADC driver and enable a TP_UP_PENDING irq. The ADC
>> driver will fill in a buffer with all data and call the callback the input
>> device associated with this buffer. The input device will then read the
>> buffer two by two and send X and Y coordinates to the input framework based
>> on what it received from the ADC's buffer. When closing this input device,
>> the buffering is stopped.
>>
>> Note that locations in the first received buffer after an TP_UP_PENDING irq
>> occurred are unreliable, thus dropped.
>>
>> Signed-off-by: Quentin Schulz <quentin.schulz@xxxxxxxxxxxxxxxxxx>
>> ---
[...]
>> + info->buffer = iio_channel_get_all_cb(&pdev->dev,
>> + &sunxi_gpadc_ts_callback,
>> + (void *)info);
>
> Any chance we could introduce devm-variant here? If you do not want to
> wait for IIO to add it you can temporarily add call
> devm_add_action_or_reset() after getting channels and remove it when IIO
> API catches up.
>
Something like:
release_iio_channels(void* data)
{
struct sunxi_gpadc_ts *info = data;
iio_channel_release_all_cb(info->buffer);
}
[...]
info->buffer = iio_channel_get_all_cb(&pdev->dev,
&sunxi_gpadc_ts_callback,
(void *)info);
ret = devm_add_action_or_reset(&pdev->dev,
release_iio_channels,
(void *)info);
if (ret)
return ret;
?
May I know why you prefer that way instead of explicit removing in
remove function of the platform device? I understand for devm-variant
already in the framework but I am curious for this one.
[...]
>> +static int sunxi_gpadc_ts_remove(struct platform_device *pdev)
>> +{
>> + struct sunxi_gpadc_ts *info = platform_get_drvdata(pdev);
>> +
>> + iio_channel_stop_all_cb(info->buffer);
>> + iio_channel_release_all_cb(info->buffer);
>> +
>> + disable_irq(info->tp_up_irq);
>
> You are mixing devm and non-devm so your unwind order is completely out
> of wack. If input device is opened while you are unloading (or
> unbinding) the dirver, then you'll release channels, then input device's
> close() will be called, which will try to stop the IIO channels again
> and disable IRQ yet again.
>
Do you mean that I should be using exclusively devm or non-devm functions?
Do you mean input device's close will always be called when
unloading/unbinding the driver?
[...]
Thanks,
Quentin
--
Quentin Schulz, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
