lkml.org 
[lkml]   [2020]   [May]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH V5 5/5] iio: adc: Clean up ADC code common to PMIC5 and PMIC7
From
Date
Hi Jonathan,

On 5/24/2020 5:34 PM, Jonathan Cameron wrote:
> On Fri, 22 May 2020 19:54:12 +0530
> Jishnu Prakash <jprakash@codeaurora.org> wrote:
>
>> This commit includes the following changes:
>>
>> Add a common function used for read_raw callback for both PMIC5
>> and PMIC7 ADCs.
>>
>> Add exit function for ADC.
> Hi Jishnu,
>
> I don't understand why one is needed, and if it is you can't do
> what you have here without introducing some nasty races.
> So if you need it clearly explain why in comments in the code
> and also consider how it may race with new requests coming in etc
> as the userspace interfaces are still visible.
>
> Move the eoc_irq addition to the structure here as well as makes
> no sense in earlier patch.
>
> Thanks,
>
> Jonathan
>
>
>> Add info_property under adc_data to more efficiently distinguish
>> PMIC5 and PMIC7 ADCs.
>>
>> Signed-off-by: Jishnu Prakash <jprakash@codeaurora.org>
>> ---
>> drivers/iio/adc/qcom-spmi-adc5.c | 81 +++++++++++++++++++++-----------------
>> drivers/iio/adc/qcom-vadc-common.h | 1 +
>> 2 files changed, 46 insertions(+), 36 deletions(-)
>>
>>
>> +static int adc5_exit(struct platform_device *pdev)
>> +{
>> + struct adc5_chip *adc = platform_get_drvdata(pdev);
>> +
>> + if (adc->irq_eoc >= 0)
>> + disable_irq(adc->irq_eoc);
> So here you are disabling an irq? Why. We should be removing it
> cleanly in the managed flow shortly anyway. If you did do this
> here for some reason I'm not thinking of then you would have
> a race against the userspace being removed on the unwind
> of the iio device register.
>
>> + return 0;
>> +}
>> +
You're right about the exit function, the actions done in it are not
strictly required, so I'll remove it in the next post.

\
 
 \ /
  Last update: 2020-05-28 18:51    [W:0.091 / U:1.036 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site