Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org 54FB0C433C9
Subject: Re: [PATCH V5 5/5] iio: adc: Clean up ADC code common to PMIC5 and
 PMIC7
To:     Jonathan Cameron <jic23@kernel.org>
Cc:     agross@kernel.org, bjorn.andersson@linaro.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
        mka@chromium.org, linus.walleij@linaro.org,
        Jonathan.Cameron@huawei.com, andy.shevchenko@gmail.com,
        amit.kucheria@verdurent.com, smohanad@codeaurora.org,
        kgunda@codeaurora.org, aghayal@codeaurora.org,
        Hartmut Knaack <knaack.h@gmx.de>,
        Lars-Peter Clausen <lars@metafoo.de>,
        Peter Meerwald-Stadler <pmeerw@pmeerw.net>,
        linux-arm-msm@vger.kernel.org, linux-iio@vger.kernel.org,
        linux-arm-msm-owner@vger.kernel.org
References: <1590157452-27179-1-git-send-email-jprakash@codeaurora.org>
 <1590157452-27179-6-git-send-email-jprakash@codeaurora.org>
 <20200524130440.250edb2e@archlinux>
From:   Jishnu Prakash <jprakash@codeaurora.org>
Message-ID: <0078a610-fed8-7a18-ecd1-27b8eb5a8feb@codeaurora.org>
Date:   Thu, 28 May 2020 22:20:02 +0530
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101
 Thunderbird/68.8.0
MIME-Version: 1.0
In-Reply-To: <20200524130440.250edb2e@archlinux>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Content-Language: en-US
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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.