lkml.org 
[lkml]   [2023]   [Oct]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH 01/11] iio: adc: Update bindings for ADC7 name used on QCOM PMICs
From
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
>

\
 
 \ /
  Last update: 2023-10-23 08:10    [W:0.612 / U:0.048 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site