All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jishnu Prakash <quic_jprakash@quicinc.com>
To: Jonathan Cameron <jic23@kernel.org>
Cc: <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>,
	<krzysztof.kozlowski@linaro.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>, <linux-arm-msm-owner@vger.kernel.org>
Subject: Re: [PATCH 01/11] iio: adc: Update bindings for ADC7 name used on QCOM PMICs
Date: Mon, 23 Oct 2023 11:35:43 +0530	[thread overview]
Message-ID: <0401d8fc-1162-ea60-bd91-ad18afece344@quicinc.com> (raw)
In-Reply-To: <20230708155844.31c55ca0@jic23-huawei>

Hi Jonathan,

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/8/2023 8:28 PM, Jonathan Cameron wrote:
> On Sat, 8 Jul 2023 12:58:25 +0530
> Jishnu Prakash <quic_jprakash@quicinc.com> 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.
>>
>> Signed-off-by: Jishnu Prakash <quic_jprakash@quicinc.com>
> Hi Jishnu.
>
> Whilst I can appreciate why you've picked this particular approach to
> deal with the renames I'm not sure it's the smoothest path - or the
> easiest to review.
>
> If doing a single patch for the complete rename was too much, perhaps
> doing one header (or if it makes sense set of headers)
> at a time would be easier to read?  With a final patch doing the compatible
> addition.  Maybe let's see what other reviewers think though.


I don't completely understand what you mean here - but first let me 
briefly recap what I was trying to do here.

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 everyone that this version lies 
between ADC5 and ADC5 Gen3.

The patches were organized like  this:

Patch 1 - Update documentation to add gen2 compatible and update 
examples(without removing older compatible). Add new binding files 
equivalent to existing ADC7 files, just with macros and file names 
updated to use "adc5_gen2" instead of "adc7"

Patch 2 - Update driver files to replace usage of "adc7" with "adc5 
gen2", adding new compatible for adc5 gen2 without removing exsiting one 
for adc7.

Patch 3 - Update compatible, macros and binding files included in all 
devicetree files, based on the earlier two changes.

Patch 4 - Delete all instances of adc7 compatible from documentation 
files. Delete all older binding files

Patch 5 - Delete the adc7 compatible from the driver


Based on the comments I got, I understand I cannot proceed as such with 
patches 4 and 5, I can amend/drop them. But to get back to your above 
point about my overall approach, how exactly would you like me to 
structure my patch series?

Should I make one big patch for documentation, bindings, driver and 
devicetree changes where I update the naming and deprecate adc7 usage? 
This may be straightforward but also hard to review.


Or a patch series like this:

One patch to update documentation

One patch to update the bindings (headers) (Or one patch per header file?)

One patch to update driver file (adding new compatible and comment to 
deprecate old one)

One patch to update all devicetree files (or separate patches?)

Please let me know what you think.

> A few other comments inline,
>
> Jonathan
>
>
>>   
>>   properties:
>> @@ -27,6 +27,7 @@ properties:
>>             - qcom,spmi-adc5
>>             - qcom,spmi-adc-rev2
>>             - qcom,spmi-adc7
>> +          - qcom,spmi-adc5-gen2
> Alphabetical order (roughly given currently list). So I'd stick
> this after qcom,spmi-adc5


Will reorder them in the next patchset.


>>   
>>     reg:
>>       description: VADC base address in the SPMI PMIC register map
>> @@ -71,7 +72,7 @@ patternProperties:
>>           description: |
>>             ADC channel number.
>>             See include/dt-bindings/iio/qcom,spmi-vadc.h
>> -          For PMIC7 ADC, the channel numbers are specified separately per PMIC
>> +          For PMIC5 Gen2 ADC, the channel numbers are specified separately per PMIC
>>             in the PMIC-specific files in include/dt-bindings/iio/.
>>   
>>         label:
>> @@ -114,7 +115,7 @@ patternProperties:
>>                 channel calibration. If property is not found, channel will be
>>                 calibrated with 0.625V and 1.25V reference channels, also
>>                 known as absolute calibration.
>> -            - For compatible property "qcom,spmi-adc5", "qcom,spmi-adc7" and
>> +            - For compatible property "qcom,spmi-adc5", "qcom,spmi-adc5-gen2" and
>>                 "qcom,spmi-adc-rev2", if this property is specified VADC will use
>>                 the VDD reference (1.875V) and GND for channel calibration. If
>>                 property is not found, channel will be calibrated with 0V and 1.25V
>> @@ -213,7 +214,9 @@ allOf:
>>         properties:
>>           compatible:
>>             contains:
>> -            const: qcom,spmi-adc7
>> +            enum :
>> +                - qcom,spmi-adc7
> There is a deprecated marking for dt-bindings. Might be good to use it here.


Thanks for your suggestion, I'll do this in the next patchset.


>
>> +                - qcom,spmi-adc5-gen2
>>   
>>       then:

Thanks,

Jishnu

>>

  reply	other threads:[~2023-10-23  6:06 UTC|newest]

Thread overview: 68+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-08  7:28 [PATCH 00/11] iio: adc: Add support for QCOM SPMI PMIC5 Gen3 ADC Jishnu Prakash
2023-07-08  7:28 ` [PATCH 01/11] iio: adc: Update bindings for ADC7 name used on QCOM PMICs Jishnu Prakash
2023-07-08 14:58   ` Jonathan Cameron
2023-10-23  6:05     ` Jishnu Prakash [this message]
2023-10-23  9:56       ` Jonathan Cameron
2023-07-09 17:17   ` Krzysztof Kozlowski
2023-10-23  6:08     ` Jishnu Prakash
2023-07-08  7:28 ` [PATCH 02/11] iio: adc: Update driver files for ADC7 rename for " Jishnu Prakash
2023-07-08  7:28 ` [PATCH 03/11] ARM: dts: qcom: Update devicetree " Jishnu Prakash
2023-07-09 17:18   ` Krzysztof Kozlowski
2023-10-23  6:09     ` Jishnu Prakash
2023-10-23  6:32       ` Krzysztof Kozlowski
2023-11-09  8:22         ` Jishnu Prakash
2023-11-10 10:59           ` Krzysztof Kozlowski
2023-11-16  3:23             ` Jishnu Prakash
2023-07-08  7:28 ` [PATCH 04/11] iio: adc: Update bindings to remove support for ADC7 name used on " Jishnu Prakash
2023-07-08 15:02   ` Jonathan Cameron
2023-10-23  6:10     ` Jishnu Prakash
2023-07-09 17:19   ` Krzysztof Kozlowski
2023-10-23  6:11     ` Jishnu Prakash
2023-10-23  6:33       ` Krzysztof Kozlowski
2023-07-08  7:28 ` [PATCH 05/11] iio: adc: qcom-spmi-adc5: remove support for ADC7 compatible string Jishnu Prakash
2023-07-08 15:00   ` Jonathan Cameron
2023-10-23  6:11     ` Jishnu Prakash
2023-07-09 17:38   ` Krzysztof Kozlowski
2023-10-23  6:12     ` Jishnu Prakash
2023-07-08  7:28 ` [PATCH 06/11] iio: adc: Add QCOM PMIC5 Gen3 ADC bindings Jishnu Prakash
2023-07-08 15:12   ` Jonathan Cameron
2023-07-08 15:25     ` Jonathan Cameron
2023-10-23  6:13       ` Jishnu Prakash
2023-10-23  6:13     ` Jishnu Prakash
2023-07-09 17:23   ` Krzysztof Kozlowski
2023-10-23  6:14     ` Jishnu Prakash
2023-10-23  6:36       ` Krzysztof Kozlowski
2023-11-16  3:23         ` Jishnu Prakash
2023-11-16 11:40           ` Krzysztof Kozlowski
2023-11-16 11:46           ` Krzysztof Kozlowski
2023-12-21  8:01             ` Jishnu Prakash
2023-07-08  7:28 ` [PATCH 07/11] iio: adc: Add support for QCOM PMIC5 Gen3 ADC Jishnu Prakash
2023-07-08 15:59   ` Jonathan Cameron
2023-10-23  6:15     ` Jishnu Prakash
2023-10-23  8:03       ` Dmitry Baryshkov
2023-11-16  3:24         ` Jishnu Prakash
2023-10-27 13:26       ` Jonathan Cameron
2023-07-09 17:41   ` Krzysztof Kozlowski
2023-10-23  6:15     ` Jishnu Prakash
2023-07-08  7:28 ` [PATCH 08/11] dt-bindings: iio: adc: Copy QCOM ADC bindings files Jishnu Prakash
2023-07-09 17:25   ` Krzysztof Kozlowski
2023-10-23  6:16     ` Jishnu Prakash
2023-10-23  6:19       ` Krzysztof Kozlowski
2023-07-08  7:28 ` [PATCH 09/11] iio: adc: Update QCOM ADC drivers for bindings path change Jishnu Prakash
2023-07-08 15:23   ` Jonathan Cameron
2023-10-23  6:17     ` Jishnu Prakash
2023-10-23  7:58       ` Dmitry Baryshkov
2023-11-16  3:24         ` Jishnu Prakash
2023-07-08  7:28 ` [PATCH 10/11] ARM: dts: qcom: Update devicetree for QCOM ADC " Jishnu Prakash
2023-07-09 17:26   ` Krzysztof Kozlowski
2023-10-23  6:18     ` Jishnu Prakash
2023-10-23  6:38       ` Krzysztof Kozlowski
2023-11-10 23:47       ` Bjorn Andersson
2023-11-16  3:25         ` Jishnu Prakash
2023-07-09 17:26   ` Krzysztof Kozlowski
2023-10-23  6:18     ` Jishnu Prakash
2023-07-08  7:28 ` [PATCH 11/11] dt-bindings: iio: remove QCOM ADC files from iio folder Jishnu Prakash
2023-07-09 17:28   ` Krzysztof Kozlowski
2023-10-23  6:19     ` Jishnu Prakash
2023-10-23  6:40       ` Krzysztof Kozlowski
2023-07-08 15:13 ` [PATCH 00/11] iio: adc: Add support for QCOM SPMI PMIC5 Gen3 ADC Jonathan Cameron

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0401d8fc-1162-ea60-bd91-ad18afece344@quicinc.com \
    --to=quic_jprakash@quicinc.com \
    --cc=Jonathan.Cameron@huawei.com \
    --cc=agross@kernel.org \
    --cc=amitk@kernel.org \
    --cc=andersson@kernel.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=conor+dt@kernel.org \
    --cc=daniel.lezcano@linaro.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dmitry.baryshkov@linaro.org \
    --cc=jic23@kernel.org \
    --cc=konrad.dybcio@linaro.org \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=krzysztof.kozlowski@linaro.org \
    --cc=lars@metafoo.de \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-msm-owner@vger.kernel.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=luca@z3ntu.xyz \
    --cc=marijn.suijten@somainline.org \
    --cc=quic_collinsd@quicinc.com \
    --cc=quic_jestar@quicinc.com \
    --cc=quic_kamalw@quicinc.com \
    --cc=quic_subbaram@quicinc.com \
    --cc=rafael@kernel.org \
    --cc=robh+dt@kernel.org \
    --cc=rui.zhang@intel.com \
    --cc=sboyd@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.