mbox series

[V2,0/3] iio: adc: Add support for QCOM SPMI PMIC5 Gen3 ADC

Message ID 20231116032530.753192-1-quic_jprakash@quicinc.com (mailing list archive)
Headers show
Series iio: adc: Add support for QCOM SPMI PMIC5 Gen3 ADC | expand

Message

Jishnu Prakash Nov. 16, 2023, 3:25 a.m. UTC
PMIC5 Gen3 has a similar ADC architecture to that on PMIC5 Gen2,
with all SW communication to ADC going through PMK8550 which
communicates with other PMICs through PBS. The major difference is
that the register interface used here is that of an SDAM present on
PMK8550, rather than a dedicated ADC peripheral. There may be more than one
SDAM used for ADC5 Gen3. Each ADC SDAM has eight channels, each of which may
be used for either immediate reads (same functionality as previous PMIC5 and
PMIC5 Gen2 ADC peripherals) or recurring measurements (same as PMIC5 and PMIC5
Gen2 ADC_TM functionality). In this case, we have VADC and ADC_TM functionality
combined into the same driver.

Patches 1 adds bindings for ADC5 Gen3 peripheral.

Patches 2 adds driver support for ADC5 Gen3.

Patch 3 is a cleanup, to move the QCOM ADC dt-bindings files from
dt-bindings/iio to dt-bindings/iio/adc folder, as they are
specifically for ADC devices. It also fixes all compilation errors
with this change in driver and devicetree files and similar errors
in documentation for dtbinding check.

Changes since v1:
- Dropped patches 1-5 for changing 'ADC7' peripheral name to 'ADC5 Gen2'.
- Addressed reviewer comments for binding and driver patches for ADC5 Gen3.
- Combined patches 8-11 into a single patch as requested by reviewers to make
  the change clearer and made all fixes required in same patch.

Jishnu Prakash (3):
  dt-bindings: iio: adc: Add QCOM PMIC5 Gen3 ADC bindings
  iio: adc: Add support for QCOM PMIC5 Gen3 ADC
  dt-bindings: iio/adc: Move QCOM ADC bindings to iio/adc folder

 .../bindings/iio/adc/qcom,spmi-vadc.yaml      |  185 ++-
 .../bindings/mfd/qcom,spmi-pmic.yaml          |    2 +-
 .../bindings/thermal/qcom-spmi-adc-tm-hc.yaml |    2 +-
 .../bindings/thermal/qcom-spmi-adc-tm5.yaml   |    6 +-
 arch/arm64/boot/dts/qcom/pm2250.dtsi          |    2 +-
 arch/arm64/boot/dts/qcom/pm6125.dtsi          |    2 +-
 arch/arm64/boot/dts/qcom/pm6150.dtsi          |    2 +-
 arch/arm64/boot/dts/qcom/pm6150l.dtsi         |    2 +-
 arch/arm64/boot/dts/qcom/pm660.dtsi           |    2 +-
 arch/arm64/boot/dts/qcom/pm660l.dtsi          |    2 +-
 arch/arm64/boot/dts/qcom/pm7250b.dtsi         |    2 +-
 arch/arm64/boot/dts/qcom/pm8150.dtsi          |    2 +-
 arch/arm64/boot/dts/qcom/pm8150b.dtsi         |    2 +-
 arch/arm64/boot/dts/qcom/pm8150l.dtsi         |    2 +-
 arch/arm64/boot/dts/qcom/pm8916.dtsi          |    2 +-
 arch/arm64/boot/dts/qcom/pm8950.dtsi          |    2 +-
 arch/arm64/boot/dts/qcom/pm8953.dtsi          |    2 +-
 arch/arm64/boot/dts/qcom/pm8994.dtsi          |    2 +-
 arch/arm64/boot/dts/qcom/pm8998.dtsi          |    2 +-
 arch/arm64/boot/dts/qcom/pmi632.dtsi          |    2 +-
 arch/arm64/boot/dts/qcom/pmi8950.dtsi         |    2 +-
 arch/arm64/boot/dts/qcom/pmm8155au_1.dtsi     |    2 +-
 arch/arm64/boot/dts/qcom/pmp8074.dtsi         |    2 +-
 arch/arm64/boot/dts/qcom/pms405.dtsi          |    2 +-
 arch/arm64/boot/dts/qcom/sc7280-idp.dts       |    2 +-
 arch/arm64/boot/dts/qcom/sc7280-idp.dtsi      |    2 +-
 arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi    |    4 +-
 arch/arm64/boot/dts/qcom/sc8180x-pmics.dtsi   |    2 +-
 .../qcom/sc8280xp-lenovo-thinkpad-x13s.dts    |    6 +-
 .../boot/dts/qcom/sm7225-fairphone-fp4.dts    |    2 +-
 arch/arm64/boot/dts/qcom/sm8450-hdk.dts       |    8 +-
 drivers/iio/adc/Kconfig                       |   25 +
 drivers/iio/adc/Makefile                      |    1 +
 drivers/iio/adc/qcom-spmi-adc5-gen3.c         | 1189 +++++++++++++++++
 drivers/iio/adc/qcom-spmi-adc5.c              |    2 +-
 drivers/iio/adc/qcom-spmi-vadc.c              |    2 +-
 .../iio/adc/qcom,spmi-adc5-gen3-pm8550.h      |   50 +
 .../iio/adc/qcom,spmi-adc5-gen3-pm8550b.h     |   89 ++
 .../iio/adc/qcom,spmi-adc5-gen3-pm8550vx.h    |   22 +
 .../iio/adc/qcom,spmi-adc5-gen3-pmk8550.h     |   56 +
 .../iio/{ => adc}/qcom,spmi-adc7-pm8350.h     |    2 +-
 .../iio/{ => adc}/qcom,spmi-adc7-pm8350b.h    |    2 +-
 .../iio/{ => adc}/qcom,spmi-adc7-pmk8350.h    |    2 +-
 .../iio/{ => adc}/qcom,spmi-adc7-pmr735a.h    |    2 +-
 .../iio/{ => adc}/qcom,spmi-adc7-pmr735b.h    |    0
 .../iio/{ => adc}/qcom,spmi-vadc.h            |   81 ++
 46 files changed, 1725 insertions(+), 61 deletions(-)
 create mode 100644 drivers/iio/adc/qcom-spmi-adc5-gen3.c
 create mode 100644 include/dt-bindings/iio/adc/qcom,spmi-adc5-gen3-pm8550.h
 create mode 100644 include/dt-bindings/iio/adc/qcom,spmi-adc5-gen3-pm8550b.h
 create mode 100644 include/dt-bindings/iio/adc/qcom,spmi-adc5-gen3-pm8550vx.h
 create mode 100644 include/dt-bindings/iio/adc/qcom,spmi-adc5-gen3-pmk8550.h
 rename include/dt-bindings/iio/{ => adc}/qcom,spmi-adc7-pm8350.h (98%)
 rename include/dt-bindings/iio/{ => adc}/qcom,spmi-adc7-pm8350b.h (99%)
 rename include/dt-bindings/iio/{ => adc}/qcom,spmi-adc7-pmk8350.h (97%)
 rename include/dt-bindings/iio/{ => adc}/qcom,spmi-adc7-pmr735a.h (95%)
 rename include/dt-bindings/iio/{ => adc}/qcom,spmi-adc7-pmr735b.h (100%)
 rename include/dt-bindings/iio/{ => adc}/qcom,spmi-vadc.h (77%)

Comments

Dmitry Baryshkov Nov. 16, 2023, 5:22 a.m. UTC | #1
On Thu, 16 Nov 2023 at 05:26, Jishnu Prakash <quic_jprakash@quicinc.com> wrote:
>
> PMIC5 Gen3 has a similar ADC architecture to that on PMIC5 Gen2,
> with all SW communication to ADC going through PMK8550 which
> communicates with other PMICs through PBS. The major difference is
> that the register interface used here is that of an SDAM present on
> PMK8550, rather than a dedicated ADC peripheral. There may be more than one
> SDAM used for ADC5 Gen3. Each ADC SDAM has eight channels, each of which may
> be used for either immediate reads (same functionality as previous PMIC5 and
> PMIC5 Gen2 ADC peripherals) or recurring measurements (same as PMIC5 and PMIC5
> Gen2 ADC_TM functionality). In this case, we have VADC and ADC_TM functionality
> combined into the same driver.
>
> Patches 1 adds bindings for ADC5 Gen3 peripheral.
>
> Patches 2 adds driver support for ADC5 Gen3.

For some reason I don't see this patch in my inbox. Maybe it will
arrive later. Immediate response: please add
devm_thermal_add_hwmon_sysfs().

>
> Patch 3 is a cleanup, to move the QCOM ADC dt-bindings files from
> dt-bindings/iio to dt-bindings/iio/adc folder, as they are
> specifically for ADC devices. It also fixes all compilation errors
> with this change in driver and devicetree files and similar errors
> in documentation for dtbinding check.

NAK. The kernel is expected to build and work after each commit.
Otherwise git-bisecting the kernel becomes impossible.
So, please rework your series in a way that there are no compilation
errors after any of the patches. The easiest way would be to rearrange
your patches in 3-1-2 order.


>
> Changes since v1:
> - Dropped patches 1-5 for changing 'ADC7' peripheral name to 'ADC5 Gen2'.
> - Addressed reviewer comments for binding and driver patches for ADC5 Gen3.
> - Combined patches 8-11 into a single patch as requested by reviewers to make
>   the change clearer and made all fixes required in same patch.
>
> Jishnu Prakash (3):
>   dt-bindings: iio: adc: Add QCOM PMIC5 Gen3 ADC bindings
>   iio: adc: Add support for QCOM PMIC5 Gen3 ADC
>   dt-bindings: iio/adc: Move QCOM ADC bindings to iio/adc folder
>
>  .../bindings/iio/adc/qcom,spmi-vadc.yaml      |  185 ++-
>  .../bindings/mfd/qcom,spmi-pmic.yaml          |    2 +-
>  .../bindings/thermal/qcom-spmi-adc-tm-hc.yaml |    2 +-
>  .../bindings/thermal/qcom-spmi-adc-tm5.yaml   |    6 +-
>  arch/arm64/boot/dts/qcom/pm2250.dtsi          |    2 +-
>  arch/arm64/boot/dts/qcom/pm6125.dtsi          |    2 +-
>  arch/arm64/boot/dts/qcom/pm6150.dtsi          |    2 +-
>  arch/arm64/boot/dts/qcom/pm6150l.dtsi         |    2 +-
>  arch/arm64/boot/dts/qcom/pm660.dtsi           |    2 +-
>  arch/arm64/boot/dts/qcom/pm660l.dtsi          |    2 +-
>  arch/arm64/boot/dts/qcom/pm7250b.dtsi         |    2 +-
>  arch/arm64/boot/dts/qcom/pm8150.dtsi          |    2 +-
>  arch/arm64/boot/dts/qcom/pm8150b.dtsi         |    2 +-
>  arch/arm64/boot/dts/qcom/pm8150l.dtsi         |    2 +-
>  arch/arm64/boot/dts/qcom/pm8916.dtsi          |    2 +-
>  arch/arm64/boot/dts/qcom/pm8950.dtsi          |    2 +-
>  arch/arm64/boot/dts/qcom/pm8953.dtsi          |    2 +-
>  arch/arm64/boot/dts/qcom/pm8994.dtsi          |    2 +-
>  arch/arm64/boot/dts/qcom/pm8998.dtsi          |    2 +-
>  arch/arm64/boot/dts/qcom/pmi632.dtsi          |    2 +-
>  arch/arm64/boot/dts/qcom/pmi8950.dtsi         |    2 +-
>  arch/arm64/boot/dts/qcom/pmm8155au_1.dtsi     |    2 +-
>  arch/arm64/boot/dts/qcom/pmp8074.dtsi         |    2 +-
>  arch/arm64/boot/dts/qcom/pms405.dtsi          |    2 +-
>  arch/arm64/boot/dts/qcom/sc7280-idp.dts       |    2 +-
>  arch/arm64/boot/dts/qcom/sc7280-idp.dtsi      |    2 +-
>  arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi    |    4 +-
>  arch/arm64/boot/dts/qcom/sc8180x-pmics.dtsi   |    2 +-
>  .../qcom/sc8280xp-lenovo-thinkpad-x13s.dts    |    6 +-
>  .../boot/dts/qcom/sm7225-fairphone-fp4.dts    |    2 +-
>  arch/arm64/boot/dts/qcom/sm8450-hdk.dts       |    8 +-
>  drivers/iio/adc/Kconfig                       |   25 +
>  drivers/iio/adc/Makefile                      |    1 +
>  drivers/iio/adc/qcom-spmi-adc5-gen3.c         | 1189 +++++++++++++++++
>  drivers/iio/adc/qcom-spmi-adc5.c              |    2 +-
>  drivers/iio/adc/qcom-spmi-vadc.c              |    2 +-
>  .../iio/adc/qcom,spmi-adc5-gen3-pm8550.h      |   50 +
>  .../iio/adc/qcom,spmi-adc5-gen3-pm8550b.h     |   89 ++
>  .../iio/adc/qcom,spmi-adc5-gen3-pm8550vx.h    |   22 +
>  .../iio/adc/qcom,spmi-adc5-gen3-pmk8550.h     |   56 +
>  .../iio/{ => adc}/qcom,spmi-adc7-pm8350.h     |    2 +-
>  .../iio/{ => adc}/qcom,spmi-adc7-pm8350b.h    |    2 +-
>  .../iio/{ => adc}/qcom,spmi-adc7-pmk8350.h    |    2 +-
>  .../iio/{ => adc}/qcom,spmi-adc7-pmr735a.h    |    2 +-
>  .../iio/{ => adc}/qcom,spmi-adc7-pmr735b.h    |    0
>  .../iio/{ => adc}/qcom,spmi-vadc.h            |   81 ++
>  46 files changed, 1725 insertions(+), 61 deletions(-)
>  create mode 100644 drivers/iio/adc/qcom-spmi-adc5-gen3.c
>  create mode 100644 include/dt-bindings/iio/adc/qcom,spmi-adc5-gen3-pm8550.h
>  create mode 100644 include/dt-bindings/iio/adc/qcom,spmi-adc5-gen3-pm8550b.h
>  create mode 100644 include/dt-bindings/iio/adc/qcom,spmi-adc5-gen3-pm8550vx.h
>  create mode 100644 include/dt-bindings/iio/adc/qcom,spmi-adc5-gen3-pmk8550.h
>  rename include/dt-bindings/iio/{ => adc}/qcom,spmi-adc7-pm8350.h (98%)
>  rename include/dt-bindings/iio/{ => adc}/qcom,spmi-adc7-pm8350b.h (99%)
>  rename include/dt-bindings/iio/{ => adc}/qcom,spmi-adc7-pmk8350.h (97%)
>  rename include/dt-bindings/iio/{ => adc}/qcom,spmi-adc7-pmr735a.h (95%)
>  rename include/dt-bindings/iio/{ => adc}/qcom,spmi-adc7-pmr735b.h (100%)
>  rename include/dt-bindings/iio/{ => adc}/qcom,spmi-vadc.h (77%)
>
> --
> 2.25.1
>


--
With best wishes
Dmitry
Jishnu Prakash Nov. 16, 2023, 6:29 a.m. UTC | #2
Hi Dmitry,

On 11/16/2023 10:52 AM, Dmitry Baryshkov wrote:
> On Thu, 16 Nov 2023 at 05:26, Jishnu Prakash <quic_jprakash@quicinc.com> wrote:
>> PMIC5 Gen3 has a similar ADC architecture to that on PMIC5 Gen2,
>> with all SW communication to ADC going through PMK8550 which
>> communicates with other PMICs through PBS. The major difference is
>> that the register interface used here is that of an SDAM present on
>> PMK8550, rather than a dedicated ADC peripheral. There may be more than one
>> SDAM used for ADC5 Gen3. Each ADC SDAM has eight channels, each of which may
>> be used for either immediate reads (same functionality as previous PMIC5 and
>> PMIC5 Gen2 ADC peripherals) or recurring measurements (same as PMIC5 and PMIC5
>> Gen2 ADC_TM functionality). In this case, we have VADC and ADC_TM functionality
>> combined into the same driver.
>>
>> Patches 1 adds bindings for ADC5 Gen3 peripheral.
>>
>> Patches 2 adds driver support for ADC5 Gen3.
> For some reason I don't see this patch in my inbox. Maybe it will
> arrive later. Immediate response: please add
> devm_thermal_add_hwmon_sysfs().


Yes, I'll check and add this in the next patch series, I'll wait for 
some more comments on the existing patches for now.

I ran into some error after sending the first two mails (cover letter 
and patch 1), so I sent patches 2 and 3 separately after it, I think you 
may have received them separately.


>
>> Patch 3 is a cleanup, to move the QCOM ADC dt-bindings files from
>> dt-bindings/iio to dt-bindings/iio/adc folder, as they are
>> specifically for ADC devices. It also fixes all compilation errors
>> with this change in driver and devicetree files and similar errors
>> in documentation for dtbinding check.
> NAK. The kernel is expected to build and work after each commit.
> Otherwise git-bisecting the kernel becomes impossible.
> So, please rework your series in a way that there are no compilation
> errors after any of the patches. The easiest way would be to rearrange
> your patches in 3-1-2 order.


I think you may have misunderstood the meaning here, I had verified 
compilation works each time after applying each of the three patches in 
this series. It's not that this last patch fixes compilation errors 
caused by the first two, this is a completely separate patch which 
affects existing QCOM ADC code (driver and devicetree) including ADC5 Gen3.


This patch does two things mainly:

Move the ADC binding files from dt-bindings/iio folder to 
dt-bindings/iio/adc folder (this would naturally cause some errors in 
driver and devicetree code due to path update)

Fix all compilation and dtbinding errors generated by the move


I added this change at the end of the series as I was not completely 
sure if it could get picked, just wanted to make it easier to drop if 
that is the final decision.


Thanks,

Jishnu


>
>
>> Changes since v1:
>> - Dropped patches 1-5 for changing 'ADC7' peripheral name to 'ADC5 Gen2'.
>> - Addressed reviewer comments for binding and driver patches for ADC5 Gen3.
>> - Combined patches 8-11 into a single patch as requested by reviewers to make
>>    the change clearer and made all fixes required in same patch.
>>
>>   .../iio/{ => adc}/qcom,spmi-adc7-pm8350b.h    |    2 +-
>>   .../iio/{ => adc}/qcom,spmi-adc7-pmk8350.h    |    2 +-
>>   .../iio/{ => adc}/qcom,spmi-adc7-pmr735a.h    |    2 +-
>>   .../iio/{ => adc}/qcom,spmi-adc7-pmr735b.h    |    0
>>   .../iio/{ => adc}/qcom,spmi-vadc.h            |   81 ++
>>   46 files changed, 1725 insertions(+), 61 deletions(-)
>>   create mode 100644 drivers/iio/adc/qcom-spmi-adc5-gen3.c
>>   create mode 100644 include/dt-bindings/iio/adc/qcom,spmi-adc5-gen3-pm8550.h
>>   create mode 100644 include/dt-bindings/iio/adc/qcom,spmi-adc5-gen3-pm8550b.h
>>   create mode 100644 include/dt-bindings/iio/adc/qcom,spmi-adc5-gen3-pm8550vx.h
>>   create mode 100644 include/dt-bindings/iio/adc/qcom,spmi-adc5-gen3-pmk8550.h
>>   rename include/dt-bindings/iio/{ => adc}/qcom,spmi-adc7-pm8350.h (98%)
>>   rename include/dt-bindings/iio/{ => adc}/qcom,spmi-adc7-pm8350b.h (99%)
>>   rename include/dt-bindings/iio/{ => adc}/qcom,spmi-adc7-pmk8350.h (97%)
>>   rename include/dt-bindings/iio/{ => adc}/qcom,spmi-adc7-pmr735a.h (95%)
>>   rename include/dt-bindings/iio/{ => adc}/qcom,spmi-adc7-pmr735b.h (100%)
>>   rename include/dt-bindings/iio/{ => adc}/qcom,spmi-vadc.h (77%)
>>
>> --
>> 2.25.1
>>
>
> --
> With best wishes
> Dmitry
Dmitry Baryshkov Nov. 16, 2023, 6:58 a.m. UTC | #3
On Thu, 16 Nov 2023 at 08:30, Jishnu Prakash <quic_jprakash@quicinc.com> wrote:
>
> Hi Dmitry,
>
> On 11/16/2023 10:52 AM, Dmitry Baryshkov wrote:
> > On Thu, 16 Nov 2023 at 05:26, Jishnu Prakash <quic_jprakash@quicinc.com> wrote:
> >> PMIC5 Gen3 has a similar ADC architecture to that on PMIC5 Gen2,
> >> with all SW communication to ADC going through PMK8550 which
> >> communicates with other PMICs through PBS. The major difference is
> >> that the register interface used here is that of an SDAM present on
> >> PMK8550, rather than a dedicated ADC peripheral. There may be more than one
> >> SDAM used for ADC5 Gen3. Each ADC SDAM has eight channels, each of which may
> >> be used for either immediate reads (same functionality as previous PMIC5 and
> >> PMIC5 Gen2 ADC peripherals) or recurring measurements (same as PMIC5 and PMIC5
> >> Gen2 ADC_TM functionality). In this case, we have VADC and ADC_TM functionality
> >> combined into the same driver.
> >>
> >> Patches 1 adds bindings for ADC5 Gen3 peripheral.
> >>
> >> Patches 2 adds driver support for ADC5 Gen3.
> > For some reason I don't see this patch in my inbox. Maybe it will
> > arrive later. Immediate response: please add
> > devm_thermal_add_hwmon_sysfs().
>
>
> Yes, I'll check and add this in the next patch series, I'll wait for
> some more comments on the existing patches for now.
>
> I ran into some error after sending the first two mails (cover letter
> and patch 1), so I sent patches 2 and 3 separately after it, I think you
> may have received them separately.
>
>
> >
> >> Patch 3 is a cleanup, to move the QCOM ADC dt-bindings files from
> >> dt-bindings/iio to dt-bindings/iio/adc folder, as they are
> >> specifically for ADC devices. It also fixes all compilation errors
> >> with this change in driver and devicetree files and similar errors
> >> in documentation for dtbinding check.
> > NAK. The kernel is expected to build and work after each commit.
> > Otherwise git-bisecting the kernel becomes impossible.
> > So, please rework your series in a way that there are no compilation
> > errors after any of the patches. The easiest way would be to rearrange
> > your patches in 3-1-2 order.
>
>
> I think you may have misunderstood the meaning here, I had verified
> compilation works each time after applying each of the three patches in
> this series. It's not that this last patch fixes compilation errors
> caused by the first two, this is a completely separate patch which
> affects existing QCOM ADC code (driver and devicetree) including ADC5 Gen3.
>
>
> This patch does two things mainly:
>
> Move the ADC binding files from dt-bindings/iio folder to
> dt-bindings/iio/adc folder (this would naturally cause some errors in
> driver and devicetree code due to path update)
>
> Fix all compilation and dtbinding errors generated by the move
>
>
> I added this change at the end of the series as I was not completely
> sure if it could get picked, just wanted to make it easier to drop if
> that is the final decision.

Ah, so patch 1 adds new files to <dt-bindings/iio/adc>, while
retaining old files in the old directory. I'd say, this is
counterintuitive.
Please reorder patches into 3-1-2 order. dt-binding changes anyway
should come first.

>
>
> Thanks,
>
> Jishnu
>
>
> >
> >
> >> Changes since v1:
> >> - Dropped patches 1-5 for changing 'ADC7' peripheral name to 'ADC5 Gen2'.
> >> - Addressed reviewer comments for binding and driver patches for ADC5 Gen3.
> >> - Combined patches 8-11 into a single patch as requested by reviewers to make
> >>    the change clearer and made all fixes required in same patch.
> >>
> >>   .../iio/{ => adc}/qcom,spmi-adc7-pm8350b.h    |    2 +-
> >>   .../iio/{ => adc}/qcom,spmi-adc7-pmk8350.h    |    2 +-
> >>   .../iio/{ => adc}/qcom,spmi-adc7-pmr735a.h    |    2 +-
> >>   .../iio/{ => adc}/qcom,spmi-adc7-pmr735b.h    |    0
> >>   .../iio/{ => adc}/qcom,spmi-vadc.h            |   81 ++
> >>   46 files changed, 1725 insertions(+), 61 deletions(-)
> >>   create mode 100644 drivers/iio/adc/qcom-spmi-adc5-gen3.c
> >>   create mode 100644 include/dt-bindings/iio/adc/qcom,spmi-adc5-gen3-pm8550.h
> >>   create mode 100644 include/dt-bindings/iio/adc/qcom,spmi-adc5-gen3-pm8550b.h
> >>   create mode 100644 include/dt-bindings/iio/adc/qcom,spmi-adc5-gen3-pm8550vx.h
> >>   create mode 100644 include/dt-bindings/iio/adc/qcom,spmi-adc5-gen3-pmk8550.h
> >>   rename include/dt-bindings/iio/{ => adc}/qcom,spmi-adc7-pm8350.h (98%)
> >>   rename include/dt-bindings/iio/{ => adc}/qcom,spmi-adc7-pm8350b.h (99%)
> >>   rename include/dt-bindings/iio/{ => adc}/qcom,spmi-adc7-pmk8350.h (97%)
> >>   rename include/dt-bindings/iio/{ => adc}/qcom,spmi-adc7-pmr735a.h (95%)
> >>   rename include/dt-bindings/iio/{ => adc}/qcom,spmi-adc7-pmr735b.h (100%)
> >>   rename include/dt-bindings/iio/{ => adc}/qcom,spmi-vadc.h (77%)
> >>
> >> --
> >> 2.25.1
> >>
> >
> > --
> > With best wishes
> > Dmitry
Andy Shevchenko Nov. 20, 2023, 11:31 a.m. UTC | #4
On Thu, Nov 16, 2023 at 08:55:27AM +0530, Jishnu Prakash wrote:
> PMIC5 Gen3 has a similar ADC architecture to that on PMIC5 Gen2,
> with all SW communication to ADC going through PMK8550 which
> communicates with other PMICs through PBS. The major difference is
> that the register interface used here is that of an SDAM present on
> PMK8550, rather than a dedicated ADC peripheral. There may be more than one
> SDAM used for ADC5 Gen3. Each ADC SDAM has eight channels, each of which may
> be used for either immediate reads (same functionality as previous PMIC5 and
> PMIC5 Gen2 ADC peripherals) or recurring measurements (same as PMIC5 and PMIC5
> Gen2 ADC_TM functionality). In this case, we have VADC and ADC_TM functionality
> combined into the same driver.
> 
> Patches 1 adds bindings for ADC5 Gen3 peripheral.
> 
> Patches 2 adds driver support for ADC5 Gen3.
> 
> Patch 3 is a cleanup, to move the QCOM ADC dt-bindings files from
> dt-bindings/iio to dt-bindings/iio/adc folder, as they are
> specifically for ADC devices. It also fixes all compilation errors
> with this change in driver and devicetree files and similar errors
> in documentation for dtbinding check.

Something wrong with the email chaining.
Please be sure you are using --thread when preparing emails.
Jonathan Cameron Nov. 25, 2023, 7:35 p.m. UTC | #5
On Thu, 16 Nov 2023 08:58:03 +0200
Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote:

> On Thu, 16 Nov 2023 at 08:30, Jishnu Prakash <quic_jprakash@quicinc.com> wrote:
> >
> > Hi Dmitry,
> >
> > On 11/16/2023 10:52 AM, Dmitry Baryshkov wrote:  
> > > On Thu, 16 Nov 2023 at 05:26, Jishnu Prakash <quic_jprakash@quicinc.com> wrote:  
> > >> PMIC5 Gen3 has a similar ADC architecture to that on PMIC5 Gen2,
> > >> with all SW communication to ADC going through PMK8550 which
> > >> communicates with other PMICs through PBS. The major difference is
> > >> that the register interface used here is that of an SDAM present on
> > >> PMK8550, rather than a dedicated ADC peripheral. There may be more than one
> > >> SDAM used for ADC5 Gen3. Each ADC SDAM has eight channels, each of which may
> > >> be used for either immediate reads (same functionality as previous PMIC5 and
> > >> PMIC5 Gen2 ADC peripherals) or recurring measurements (same as PMIC5 and PMIC5
> > >> Gen2 ADC_TM functionality). In this case, we have VADC and ADC_TM functionality
> > >> combined into the same driver.
> > >>
> > >> Patches 1 adds bindings for ADC5 Gen3 peripheral.
> > >>
> > >> Patches 2 adds driver support for ADC5 Gen3.  
> > > For some reason I don't see this patch in my inbox. Maybe it will
> > > arrive later. Immediate response: please add
> > > devm_thermal_add_hwmon_sysfs().  
> >
> >
> > Yes, I'll check and add this in the next patch series, I'll wait for
> > some more comments on the existing patches for now.
> >
> > I ran into some error after sending the first two mails (cover letter
> > and patch 1), so I sent patches 2 and 3 separately after it, I think you
> > may have received them separately.
> >
> >  
> > >  
> > >> Patch 3 is a cleanup, to move the QCOM ADC dt-bindings files from
> > >> dt-bindings/iio to dt-bindings/iio/adc folder, as they are
> > >> specifically for ADC devices. It also fixes all compilation errors
> > >> with this change in driver and devicetree files and similar errors
> > >> in documentation for dtbinding check.  
> > > NAK. The kernel is expected to build and work after each commit.
> > > Otherwise git-bisecting the kernel becomes impossible.
> > > So, please rework your series in a way that there are no compilation
> > > errors after any of the patches. The easiest way would be to rearrange
> > > your patches in 3-1-2 order.  
> >
> >
> > I think you may have misunderstood the meaning here, I had verified
> > compilation works each time after applying each of the three patches in
> > this series. It's not that this last patch fixes compilation errors
> > caused by the first two, this is a completely separate patch which
> > affects existing QCOM ADC code (driver and devicetree) including ADC5 Gen3.
> >
> >
> > This patch does two things mainly:
> >
> > Move the ADC binding files from dt-bindings/iio folder to
> > dt-bindings/iio/adc folder (this would naturally cause some errors in
> > driver and devicetree code due to path update)
> >
> > Fix all compilation and dtbinding errors generated by the move
> >
> >
> > I added this change at the end of the series as I was not completely
> > sure if it could get picked, just wanted to make it easier to drop if
> > that is the final decision.  
> 
> Ah, so patch 1 adds new files to <dt-bindings/iio/adc>, while
> retaining old files in the old directory. I'd say, this is
> counterintuitive.
> Please reorder patches into 3-1-2 order. dt-binding changes anyway
> should come first.

Absolutely agree.  Refactors, cleanup etc should precede the new stuff
in a series.  That way they can get picked up by anyone who wants to backport
without having to first figure out if they want the new stuff.

Jonathan

> 
> >
> >
> > Thanks,
> >
> > Jishnu
> >
> >  
> > >
> > >  
> > >> Changes since v1:
> > >> - Dropped patches 1-5 for changing 'ADC7' peripheral name to 'ADC5 Gen2'.
> > >> - Addressed reviewer comments for binding and driver patches for ADC5 Gen3.
> > >> - Combined patches 8-11 into a single patch as requested by reviewers to make
> > >>    the change clearer and made all fixes required in same patch.
> > >>
> > >>   .../iio/{ => adc}/qcom,spmi-adc7-pm8350b.h    |    2 +-
> > >>   .../iio/{ => adc}/qcom,spmi-adc7-pmk8350.h    |    2 +-
> > >>   .../iio/{ => adc}/qcom,spmi-adc7-pmr735a.h    |    2 +-
> > >>   .../iio/{ => adc}/qcom,spmi-adc7-pmr735b.h    |    0
> > >>   .../iio/{ => adc}/qcom,spmi-vadc.h            |   81 ++
> > >>   46 files changed, 1725 insertions(+), 61 deletions(-)
> > >>   create mode 100644 drivers/iio/adc/qcom-spmi-adc5-gen3.c
> > >>   create mode 100644 include/dt-bindings/iio/adc/qcom,spmi-adc5-gen3-pm8550.h
> > >>   create mode 100644 include/dt-bindings/iio/adc/qcom,spmi-adc5-gen3-pm8550b.h
> > >>   create mode 100644 include/dt-bindings/iio/adc/qcom,spmi-adc5-gen3-pm8550vx.h
> > >>   create mode 100644 include/dt-bindings/iio/adc/qcom,spmi-adc5-gen3-pmk8550.h
> > >>   rename include/dt-bindings/iio/{ => adc}/qcom,spmi-adc7-pm8350.h (98%)
> > >>   rename include/dt-bindings/iio/{ => adc}/qcom,spmi-adc7-pm8350b.h (99%)
> > >>   rename include/dt-bindings/iio/{ => adc}/qcom,spmi-adc7-pmk8350.h (97%)
> > >>   rename include/dt-bindings/iio/{ => adc}/qcom,spmi-adc7-pmr735a.h (95%)
> > >>   rename include/dt-bindings/iio/{ => adc}/qcom,spmi-adc7-pmr735b.h (100%)
> > >>   rename include/dt-bindings/iio/{ => adc}/qcom,spmi-vadc.h (77%)
> > >>
> > >> --
> > >> 2.25.1
> > >>  
> > >
> > > --
> > > With best wishes
> > > Dmitry  
> 
> 
>