| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Sun, 29 May 2016 20:53:01 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Ksenija Stanojević <ksenija.stanojevic@...il.com>
Cc: linux-kernel@...r.kernel.org, Lee Jones <lee.jones@...aro.org>,
Dmitry Torokhov <dmitry.torokhov@...il.com>,
linux-input@...r.kernel.org, Hartmut Knaack <knaack.h@....de>,
Lars-Peter Clausen <lars@...afoo.de>,
Peter Meerwald-Stadler <pmeerw@...erw.net>,
Marek Vašut <marex@...x.de>,
linux-iio@...r.kernel.org, Harald Geyer <harald@...ib.org>
Subject: Re: [PATCH 3/3] input: touchscreen: mxs-lradc: Add support for
touchscreen
<snip>
>>>>> + if (of_property_read_u32(node, "fsl,settling", &adapt)) {
>>>>> + ts->settling_delay = 10;
>>>>> + } else {
>>>>> + if (adapt < 1 || adapt > LRADC_DELAY_DELAY_MASK) {
>>>>> + dev_err(ts->dev, "Invalid settling delay (%u)\n",
>>>>> + adapt);
>>>>> + touch_ret = -EINVAL;
>>>>> + } else {
>>>>> + ts->settling_delay = adapt;
>>>>> + }
>>>>> + }
>>>>> +
>>>>> + mxs_lradc_ts_hw_init(ts);
>>>>> + for (i = 0; i < lradc->irq_count; i++) {
>>>>> + ret = devm_request_irq(dev, lradc->irq[i],
>>>>> + mxs_lradc_ts_handle_irq,
>>>>> + IRQF_SHARED, lradc->irq_name[i], ts);
>>>> As with the adc driver, are we actually using all of these? I'd prefer we
>>>> only grab the ones that are actually relevant.
>>>
>>> Only irq lines relevant for touchscreen are:
>>> mxs-lradc-touchscreen, mxs-lradc-channel6 and mxs-lradc-channel7
>>> But not all interrupts are beiing used even when I enabled all remaining
>>> channels (not used by touchscreen) for bufferd capture via
>>> echo 1 >/sys/bus/iio/devices/iio\:device0/scan_elements/in_voltagexx_en
>>>
>>> So I don't know if it's supposed to work like this...
>>> (It works the same on the original code)
>> Certainly should only grab the relevant ones to touch screen use in here..
>> Original code probably being overly enthusiastic and we never noticed ;)
>
> Sorry for being unclear...
> The point I was trying to make was that even though touchscreen and
> adc don't share irq lines, irqs mxs-lradc-channel(2,3,4,5,6) are never
> being used.
> So touchscreen is only use 2 out of 3 irqs assigned to it:
> mxs-lradc-touchscreen and mxs-lradc-channel7 . And adc
> is only using 2 irq lines mxs-lradc-channel(0,1) during buffered
> capture, even when all virtal channels are enabled.
> I found that odd...
odd indeed. It's odd hardware ;)
>From what I recall this hardware is capable of scheduling things in really
complex ways, but the driver elects not to use that, basically because it became
unmanageably complex and KISS was applied....
If there is an application needing that complexity we can revisit.
Of course there may be a better explanation ;)
Jonathan
>
>>> root@...100xx:~# cat /proc/interrupts
>>> CPU0
>>> 16: 13108 - 48 Edge MXS Timer Tick
>>> 17: 4240 - 82 Edge mxs-dma
>>> 25: 6 - 96 Edge 80010000.ssp
>>> 196: 0 - 68 Edge mxs-dma
>>> 210: 13 - 10 Edge mxs-lradc-touchscreen
>>> 211: 0 - 14 Edge mxs-lradc-thresh0
>>> 212: 0 - 15 Edge mxs-lradc-thresh1
>>> 213: 10 - 16 Edge mxs-lradc-channel0
>>> 214: 10 - 17 Edge mxs-lradc-channel1
>>> 215: 0 - 18 Edge mxs-lradc-channel2
>>> 216: 0 - 19 Edge mxs-lradc-channel3
>>> 217: 0 - 20 Edge mxs-lradc-channel4
>>> 218: 0 - 21 Edge mxs-lradc-channel5
>>> 219: 0 - 22 Edge mxs-lradc-channel6
>>> 220: 412 - 23 Edge mxs-lradc-channel7
>>> 221: 0 - 24 Edge mxs-lradc-button0
>>> 222: 0 - 25 Edge mxs-lradc-button1
>>> 223: 0 - 29 Edge RTC alarm
>>> 224: 0 - 111 Edge 80058000.i2c
>>> 228: 174 - 47 Edge uart-pl011
>>> 229: 439 - 93 Edge 80080000.usb
>>> 230: 0 - 92 Edge 80090000.usb
>>> 231: 3610 - 101 Edge 800f0000.ethernet
>>> 232: 10 80050000.lradc-dev0 Edge
>>> Err: 0
>>>
>>>>> + if (ret)
>>>>> + return ret;
>>>>> + }
>>>>> +
>>>>> + if (!touch_ret) {
>>>>> + ret = mxs_lradc_ts_register(ts);
>>>>> + if (!ret)
>>>>> + goto err_ts_register;
>>>>> + }
>>>>> +
>>>>> + return 0;
>>>>> +
>>>>> +err_ts_register:
>>>>> + mxs_lradc_ts_hw_stop(ts);
>>>>> + return ret;
>>>>> +}
>>>>> +
>>>>> +static int mxs_lradc_ts_remove(struct platform_device *pdev)
>>>>> +{
>>>>> + struct mxs_lradc_ts *ts = platform_get_drvdata(pdev);
>>>>> +
>>>>> + mxs_lradc_ts_hw_stop(ts);
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +static struct platform_driver mxs_lradc_ts_driver = {
>>>>> + .driver = {
>>>>> + .name = DRIVER_NAME_TS,
>>>>> + },
>>>>> + .probe = mxs_lradc_ts_probe,
>>>>> + .remove = mxs_lradc_ts_remove,
>>>>> +};
>>>>> +module_platform_driver(mxs_lradc_ts_driver);
>>>>> +
>>>>> +MODULE_LICENSE("GPL v2");
>>>>>
>>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>>> the body of a message to majordomo@...r.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>
>>
>
Powered by blists - more mailing lists