linux-kernel - Re: [PATCH V5 5/5] iio: adc: Clean up ADC code common to PMIC5 and PMIC7

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <0078a610-fed8-7a18-ecd1-27b8eb5a8feb@codeaurora.org>
Date:   Thu, 28 May 2020 22:20:02 +0530
From:   Jishnu Prakash <jprakash@...eaurora.org>
To:     Jonathan Cameron <jic23@...nel.org>
Cc:     agross@...nel.org, bjorn.andersson@...aro.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        mka@...omium.org, linus.walleij@...aro.org,
        Jonathan.Cameron@...wei.com, andy.shevchenko@...il.com,
        amit.kucheria@...durent.com, smohanad@...eaurora.org,
        kgunda@...eaurora.org, aghayal@...eaurora.org,
        Hartmut Knaack <knaack.h@....de>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Peter Meerwald-Stadler <pmeerw@...erw.net>,
        linux-arm-msm@...r.kernel.org, linux-iio@...r.kernel.org,
        linux-arm-msm-owner@...r.kernel.org
Subject: Re: [PATCH V5 5/5] iio: adc: Clean up ADC code common to PMIC5 and
 PMIC7

Hi Jonathan,

On 5/24/2020 5:34 PM, Jonathan Cameron wrote:
> On Fri, 22 May 2020 19:54:12 +0530
> Jishnu Prakash <jprakash@...eaurora.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@...eaurora.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.