Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.34 as permitted sender) client-ip=23.128.96.34;
Message-ID: <4bb709ad-c5ca-955b-92eb-2825475b0da5@quicinc.com>
Date:   Mon, 23 Oct 2023 11:38:36 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.10.1
Subject: Re: [PATCH 01/11] iio: adc: Update bindings for ADC7 name used on
 QCOM PMICs
Content-Language: en-US
To:     Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
        <agross@kernel.org>, <devicetree@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <linus.walleij@linaro.org>,
        <Jonathan.Cameron@huawei.com>, <sboyd@kernel.org>,
        <dmitry.baryshkov@linaro.org>, <quic_subbaram@quicinc.com>,
        <quic_collinsd@quicinc.com>, <quic_kamalw@quicinc.com>,
        <quic_jestar@quicinc.com>, <marijn.suijten@somainline.org>,
        <andriy.shevchenko@linux.intel.com>,
        Jonathan Cameron <jic23@kernel.org>,
        Lars-Peter Clausen <lars@metafoo.de>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Conor Dooley <conor+dt@kernel.org>,
        Bjorn Andersson <andersson@kernel.org>,
        Konrad Dybcio <konrad.dybcio@linaro.org>,
        "Rafael J. Wysocki" <rafael@kernel.org>,
        "Daniel Lezcano" <daniel.lezcano@linaro.org>,
        Amit Kucheria <amitk@kernel.org>,
        "Zhang Rui" <rui.zhang@intel.com>, Luca Weiss <luca@z3ntu.xyz>,
        <linux-iio@vger.kernel.org>, <linux-arm-msm@vger.kernel.org>,
        <linux-pm@vger.kernel.org>
CC:     <linux-arm-msm-owner@vger.kernel.org>
References: <20230708072835.3035398-1-quic_jprakash@quicinc.com>
 <20230708072835.3035398-2-quic_jprakash@quicinc.com>
 <935c14fd-e87f-e89b-f641-7fb529bc7e50@linaro.org>
From:   Jishnu Prakash <quic_jprakash@quicinc.com>
In-Reply-To: <935c14fd-e87f-e89b-f641-7fb529bc7e50@linaro.org>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi Krzysztof,

Sorry for the late reply, I could not get back earlier as I got occupied 
with other work till now. I have addressed your comments inline.

On 7/9/2023 10:47 PM, Krzysztof Kozlowski wrote:
> On 08/07/2023 09:28, Jishnu Prakash wrote:
>> The name used initially for this version of Qualcomm Technologies, Inc.
>> PMIC ADC was ADC7, following the convention of calling the PMIC generation
>> PMIC7. However, the names were later amended internally to ADC5 Gen2 and
>> PMIC5 Gen2. In addition, the latest PMIC generation now is known as
>> PMIC5 Gen3 with ADC5 Gen3 supported on it. With this addition, it makes more
>> sense to correct the name for this version of ADCs to ADC5 Gen2 from ADC7.
>> Since this affects ADC devices across some PMICs, update the names accordingly.
>>
>> In order to avoid breaking the existing implementations of ADC7, add
>> support for ADC5 Gen2 first now and remove the ADC7 support in a later
>> patch.
> I don't understand and I do not see it explained, why do you remove
> ADC7. The patch is also doing way too many things at the same time...

In patches 1-5 of this series, I intended to update all existing support 
for ADC7 by renaming it to ADC5 Gen2 to match the correct name used 
internally. In addition, since I am adding support for ADC5 Gen3 in 
patches 6 and 7, I thought it would make sense to rename this older 
peripheral, to make it more obvious to others that this version lies 
between ADC5 and ADC5 Gen3.

I have also left a comment in one earlier upstream change for PMIC5 Gen2 
ADC_TM (closely related to ADC5 Gen2) where I mentioned the same 
briefly, when I was asked why we were not naming it ADC_TM7: 
https://lore.kernel.org/linux-arm-msm/111fcc56-6441-3300-8d96-029ef8600702@quicinc.com/.

Is it fine if we keep the old compatible for adc7, marking it 
deprecated, while adding the new compatible for adc5-gen2 and updating 
the bindings, driver and devicetree completely to use gen2 (while not 
replacing instances of the old compatible)?

Or should we also keep the old bindings with macros using the adc7 name 
instead of directly replacing them with new macros using the adc5 gen2 
name, to avoid the possibility breaking the devicetree for some users?

>
>> Signed-off-by: Jishnu Prakash <quic_jprakash@quicinc.com>
>> ---
>>   .../bindings/iio/adc/qcom,spmi-vadc.yaml      | 21 +++--
>>   .../bindings/thermal/qcom-spmi-adc-tm5.yaml   | 16 ++--
>>   .../iio/qcom,spmi-adc5-gen2-pm8350.h          | 64 +++++++++++++
>>   .../iio/qcom,spmi-adc5-gen2-pm8350b.h         | 89 +++++++++++++++++++
>>   .../iio/qcom,spmi-adc5-gen2-pmk8350.h         | 47 ++++++++++
>>   .../iio/qcom,spmi-adc5-gen2-pmr735a.h         | 29 ++++++
>>   .../iio/qcom,spmi-adc5-gen2-pmr735b.h         | 28 ++++++
>>   include/dt-bindings/iio/qcom,spmi-vadc.h      | 77 ++++++++++++++++
> Bindings are always separate patches. If this is commit for bindings, then:
>
> Please use subject prefixes matching the subsystem. You can get them for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> your patch is touching.

Sure, I'll check this for the next patch series.

Thanks,

Jishnu

>
>
>
> Best regards,
> Krzysztof
>