linux-kernel - Re: [PATCH V2 0/3] iio: adc: Add support for QCOM SPMI PMIC5 Gen3 ADC

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Sat, 25 Nov 2023 19:35:37 +0000
From:   Jonathan Cameron <jic23@...nel.org>
To:     Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
Cc:     Jishnu Prakash <quic_jprakash@...cinc.com>, robh+dt@...nel.org,
        krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org,
        agross@...nel.org, andersson@...nel.org, konrad.dybcio@...aro.org,
        daniel.lezcano@...aro.org, linus.walleij@...aro.org,
        linux-arm-msm@...r.kernel.org, andriy.shevchenko@...ux.intel.com,
        quic_subbaram@...cinc.com, quic_collinsd@...cinc.com,
        quic_amelende@...cinc.com, quic_kamalw@...cinc.com,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        marijn.suijten@...ainline.org, lars@...afoo.de, luca@...tu.xyz,
        linux-iio@...r.kernel.org, lee@...nel.org, rafael@...nel.org,
        rui.zhang@...el.com, lukasz.luba@....com,
        cros-qcom-dts-watchers@...omium.org, sboyd@...nel.org,
        linux-pm@...r.kernel.org, linux-arm-msm-owner@...r.kernel.org,
        kernel@...cinc.com
Subject: Re: [PATCH V2 0/3] iio: adc: Add support for QCOM SPMI PMIC5 Gen3
 ADC

On Thu, 16 Nov 2023 08:58:03 +0200
Dmitry Baryshkov <dmitry.baryshkov@...aro.org> wrote:

> On Thu, 16 Nov 2023 at 08:30, Jishnu Prakash <quic_jprakash@...cinc.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@...cinc.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  
> 
> 
>