Message-ID: <20260207165638.0ed27302@jic23-huawei>
Date: Sat, 7 Feb 2026 16:56:38 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: Jishnu Prakash <jishnu.prakash@....qualcomm.com>
Cc: robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
 agross@...nel.org, andersson@...nel.org, lumag@...nel.org,
 dmitry.baryshkov@....qualcomm.com, konradybcio@...nel.org,
 daniel.lezcano@...aro.org, sboyd@...nel.org, amitk@...nel.org,
 thara.gopinath@...il.com, lee@...nel.org, rafael@...nel.org,
 subbaraman.narayanamurthy@....qualcomm.com, david.collins@....qualcomm.com,
 anjelique.melendez@....qualcomm.com, kamal.wadhwa@....qualcomm.com,
 rui.zhang@...el.com, lukasz.luba@....com, devicetree@...r.kernel.org,
 linux-arm-msm@...r.kernel.org, linux-iio@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org,
 cros-qcom-dts-watchers@...omium.org, quic_kotarake@...cinc.com,
 neil.armstrong@...aro.org, stephan.gerhold@...aro.org
Subject: Re: [PATCH V10 3/4] iio: adc: Add support for QCOM PMIC5 Gen3 ADC

On Fri, 6 Feb 2026 18:45:02 +0530
Jishnu Prakash <jishnu.prakash@....qualcomm.com> wrote:

> Hi Jonathan,
> 
> On 1/31/2026 11:09 PM, Jonathan Cameron wrote:
> > On Fri, 30 Jan 2026 17:24:20 +0530
> > Jishnu Prakash <jishnu.prakash@....qualcomm.com> wrote:
> >   
> >> The ADC architecture on PMIC5 Gen3 is similar to that on PMIC5 Gen2,
> >> with all SW communication to ADC going through PMK8550 which
> >> communicates with other PMICs through PBS.
> >>
> >> One major difference is that the register interface used here is that
> >> of an SDAM (Shared Direct Access Memory) peripheral present on PMK8550.
> >> There may be more than one SDAM used for ADC5 Gen3 and each has eight
> >> channels, which may be used for either immediate reads (same functionality
> >> as previous PMIC5 and PMIC5 Gen2 ADC peripherals) or recurring measurements
> >> (same as ADC_TM functionality).
> >>
> >> By convention, we reserve the first channel of the first SDAM for all
> >> immediate reads and use the remaining channels across all SDAMs for
> >> ADC_TM monitoring functionality.
> >>
> >> Add support for PMIC5 Gen3 ADC driver for immediate read functionality.
> >> ADC_TM is implemented as an auxiliary thermal driver under this ADC
> >> driver.
> >>
> >> Signed-off-by: Jishnu Prakash <jishnu.prakash@....qualcomm.com>  
> > Hi Jishnu
> > 
> > Whilst there are a couple of comments below, I think this is ready to go.
> > Unfortunately this is just a few days too late to merge for this coming
> > cycle as I need to send the IIO pull request today or tomorrow (due to going
> > through char-misc) so this would get no soak time in next.
> > 
> > Also, I'm not sure how we actually want to merge this given close coupling with
> > the thermal driver.  Perhaps best bet is I do an immutable branch of next rc1
> > once available that we pull into both trees. That would have the first 3 patches
> > on it. 
> > 
> > Jonathan
> >   
> >> diff --git a/drivers/iio/adc/qcom-spmi-adc5-gen3.c b/drivers/iio/adc/qcom-spmi-adc5-gen3.c
> >> new file mode 100644
> >> index 000000000000..f8168a14b907
> >> --- /dev/null
> >> +++ b/drivers/iio/adc/qcom-spmi-adc5-gen3.c
> >> @@ -0,0 +1,860 @@
> >> +// SPDX-License-Identifier: GPL-2.0-only
> >> +/*
> >> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> >> + */
> >> +
> >> +#include <linux/auxiliary_bus.h>
> >> +#include <linux/bitfield.h>
> >> +#include <linux/bits.h>
> >> +#include <linux/cleanup.h>
> >> +#include <linux/completion.h>
> >> +#include <linux/container_of.h>
> >> +#include <linux/delay.h>
> >> +#include <linux/device.h>  
> > If you happen to be spinning again for some reason, I think you might be able to replace
> > this device.h include with a forwards declaration of struct device;
> > 
> > If you can that would be good as we are trying to reduce includes of these
> > mega headers.  
> 
> I have a query about this. From what I understand, having a forwards
> declaration of struct device may be valid if we only use the device struct
> definition directly, but not if we try to dereference any of the struct's members.
Ah my mistake. I missed the dereferences!  You are correct to include
device.h.


> 
> I see that at some places in this file(qcom-spmi-adc5-gen3.c), we dereference
> the device's parent:
> 
> 
> One example:
> void adc5_gen3_mutex_lock(struct device *dev)
> {
>     struct iio_dev *indio_dev = dev_get_drvdata(dev->parent);
>     ....
> 
> 
> Another example:
> static int adc5_gen3_probe(struct platform_device *pdev)
> {
>     struct device *dev = &pdev->dev;
>     regmap = dev_get_regmap(dev->parent, NULL);
>     ....
> 
> We do not dereference the device's parent in the other new files
> (qcom-adc5-gen3-common.h and qcom-spmi-adc-tm5-gen3.c), so
> I can drop the device.h inclusion and use a forward declaration
> of struct device in them.
> 
> 
> In this file, what would you suggest I do, keep or drop the
> device.h inclusion? 

Keep it here given the dereferences you pointed out.

> 
> I do see that device.h is included in auxiliary_bus.h,
> iio.h and platform_device.h, so chances may be low that
> it is somehow dropped later from all of those files and
> will need to be explicitly included here.
> 
> 
> Also, is it alright if I push the next series to address
> your comments immediately? Would you be able to include patches 1-3
> in the immutable branch you mentioned, once it's available?
It's fine to post a v11.  I'll only be building that immutable tree
after 7.0-rc1 is out (or 6.20-rc1 depending on Linus' decision on naming
for the cycle that is about to start)

Thanks,

Jonathan

Powered by blists - more mailing lists