Re: [PATCH 01/11] iio: adc: Update bindings for ADC7 name used on QCOM PMICs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxx/.

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@xxxxxxxxxxx>
---
  .../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




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux