* Re: [PATCH 09/15] device core: Add ability to handle multiple dma offsets
From: Jim Quinlan @ 2020-05-22 14:31 UTC (permalink / raw)
To: Nicolas Saenz Julienne
Cc: Rob Herring, Frank Rowand, Christoph Hellwig, Marek Szyprowski,
Robin Murphy, Greg Kroah-Hartman, Suzuki K Poulose,
Saravana Kannan, Heikki Krogerus, Rafael J. Wysocki, Dan Williams,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE, open list,
open list:DMA MAPPING HELPERS
In-Reply-To: <2aa6f276085319a5af7a96b3d7bdd0501641a7d7.camel@suse.de>
Hi Nicolas,
On Wed, May 20, 2020 at 7:28 AM Nicolas Saenz Julienne
<nsaenzjulienne@suse.de> wrote:
>
> Hi Jim,
> thanks for having a go at this! My two cents.
>
> On Tue, 2020-05-19 at 16:34 -0400, Jim Quinlan wrote:
> > The device variable 'dma_pfn_offset' is used to do a single
> > linear map between cpu addrs and dma addrs. The variable
> > 'dma_map' is added to struct device to point to an array
> > of multiple offsets which is required for some devices.
> >
> > Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>
> > ---
>
> [...]
>
> > --- a/include/linux/device.h
> > +++ b/include/linux/device.h
> > @@ -493,6 +493,8 @@ struct dev_links_info {
> > * @bus_dma_limit: Limit of an upstream bridge or bus which imposes a smaller
> > * DMA limit than the device itself supports.
> > * @dma_pfn_offset: offset of DMA memory range relatively of RAM
> > + * @dma_map: Like dma_pfn_offset but used when there are multiple
> > + * pfn offsets for multiple dma-ranges.
> > * @dma_parms: A low level driver may set these to teach IOMMU code
> > about
> > * segment limitations.
> > * @dma_pools: Dma pools (if dma'ble device).
> > @@ -578,7 +580,12 @@ struct device {
> > allocations such descriptors. */
> > u64 bus_dma_limit; /* upstream dma constraint */
> > unsigned long dma_pfn_offset;
> > -
> > +#ifdef CONFIG_DMA_PFN_OFFSET_MAP
> > + const void *dma_offset_map; /* Like dma_pfn_offset, but for
> > + * the unlikely case of multiple
> > + * offsets. If non-null, dma_pfn_offset
> > + * will be 0. */
>
> I get a bad feeling about separating the DMA offset handling into two distinct
> variables. Albeit generally frowned upon, there is a fair amount of trickery
> around dev->dma_pfn_offset all over the kernel. usb_alloc_dev() comes to mind
> for example. And this obviously doesn't play well with it.
The trickery should only be present when
CONFIG_DMA_PFN_OFFSET_MAP=y**. Otherwise it does no harm. Also, I
feel that if dev-dma_pfn_offset is valid then so is
dev->dma_pfn_offset_map -- they both use the same mechanism in the
same places. I am merely
extending something that has been in Linux for a long time..
Further, I could have had dma_pfn_offset_map subsume dma_pfn_offset
but I wanted to leave it alone since folks would complain that it
would go from an addition to an if-clause and an inline function. But
if I did go that way there would only be one mechanism that would
cover both cases.
> I feel a potential
> solution to multiple DMA ranges should completely integrate with the current
> device DMA handling code, without special cases, on top of that, be transparent
> to the user.
Having dma_pfn_offset_map subsume dma_pfn_offset would integrate the
current code too. And I am not sure what you mean by being
"transparent to the user" -- the writer of the PCIe endpoint driver is
going to do some DMA calls and they have no idea if this mechanism is
in play or not.
>
> In more concrete terms, I'd repackage dev->bus_dma_limit and
> dev->dma_pfn_offset into a list/array of DMA range structures
This is sort of what I am doing except I defined my own structure.
Using the of_range structure would require one to do the same extra
calculations over and over for a DMA call; this is why I defined my
structure that has all of the needed precomputed variables.
> and adapt/create
> the relevant getter/setter functions so as for DMA users not to have to worry
> about the specifics of a device's DMA constraints.
I'm not sure I understand where these getter/setter functions would
exist or what they would do.
> editing dev->dma_pfn_offset, you'd be passing a DMA range structure to the
> device core, and let it take the relevant decisions on how to handle it
How and where would the device core operate for these getter/setters?
In how many places in the code? The way I see it, any solution has to
adjust the value when doing dma2phys and phys2dma conversions, and the
most efficient place to do that is in the two DMA header files (the
second one is for ARM).
> internally (overwrite, add a new entry, merge them, etc...).
I'm concerned that this would be overkill; I am just trying to get a
driver upstream for some baroque PCIe RC HW I'm not sure if we should
set up something elaborate when the demand is not there.
I'll be posting a v2. ChrisophH has sent me some personal feedback
which I am incorporating; so feel free to discuss your ideas with him
as well because I really want consensus on any large changes in
direction.
Thanks,
Jim
** CONFIG_DMA_OF_PFN_OFFSET_MAP=y only occurs when building for
ARCH_BRCMSTB. However, ARCH_BRCMSTB is set by the ARM64 defconfig and
the ARM multi_v7_defconfig, so it would be activated for those
defconfigs. This may(a) get us kicked out of those defconfigs or
(b) we may have to keep DMA_OF_PFN_OFFSET_MAP off in those defconfigs
and only turn it on solely for Broadcom STB Linux.
>
> Easier said than done. :)
>
> Regards,
> Nicolas
>
^ permalink raw reply
* Re: [PATCH v10 2/5] PCI: Add Loongson PCI Controller support
From: Thomas Bogendoerfer @ 2020-05-22 14:27 UTC (permalink / raw)
To: Jiaxun Yang
Cc: Lorenzo Pieralisi, linux-pci, Rob Herring, Bjorn Helgaas,
Rob Herring, Huacai Chen, Paul Burton, devicetree, linux-kernel,
linux-mips
In-Reply-To: <7FE3D498-D293-407C-A70C-5E763151477C@flygoat.com>
On Fri, May 22, 2020 at 09:32:10PM +0800, Jiaxun Yang wrote:
>
>
> 于 2020年5月22日 GMT+08:00 下午9:10:18, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> 写到:
> >On Wed, May 20, 2020 at 07:57:29PM +0800, Jiaxun Yang wrote:
> >>
> >>
> >> 于 2020年5月14日 GMT+08:00 下午9:16:38, Jiaxun Yang <jiaxun.yang@flygoat.com> 写到:
> >> >This controller can be found on Loongson-2K SoC, Loongson-3
> >> >systems with RS780E/LS7A PCH.
> >> >
> >> >The RS780E part of code was previously located at
> >> >arch/mips/pci/ops-loongson3.c and now it can use generic PCI
> >> >driver implementation.
> >> >
> >> >Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> >> >Reviewed-by: Rob Herring <robh@kernel.org>
> >> >
> >>
> >> Hi there,
> >>
> >> Is it possible to let this series go into next tree soon?
> >>
> >> As LS7A dts patch would depend on this series, and I want to
> >> make the whole LS7A basic support as a part of 5.8 release.
> >
> >I think you have all necessary tags to take this in the MIPS
> >tree, please let me know if that's the way we want this to go
> >upstream - I would not pull MIPS/dts changes into the PCI tree
> >and I don't think it is needed for this series.
>
> Please pull all PCI changes [1~3] into PCI tree.
>
> And Thomas,
> Could you please apply rest changes into MIPS Tree?
this will cause loss of PCI support for loongson in mips-next until
everything is pulled together during merge window. As this isn't too
nice I've acked the remaining patches to get them into the pci/mips
tree, too.
Thomas.
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]
^ permalink raw reply
* Re: [PATCH v10 2/5] PCI: Add Loongson PCI Controller support
From: Thomas Bogendoerfer @ 2020-05-22 14:27 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Jiaxun Yang, linux-pci, Rob Herring, Bjorn Helgaas, Rob Herring,
Huacai Chen, Paul Burton, devicetree, linux-kernel, linux-mips
In-Reply-To: <20200522134048.GG11785@e121166-lin.cambridge.arm.com>
On Fri, May 22, 2020 at 02:40:48PM +0100, Lorenzo Pieralisi wrote:
> On Fri, May 22, 2020 at 09:32:10PM +0800, Jiaxun Yang wrote:
>
> [...]
>
> > >> Is it possible to let this series go into next tree soon?
> > >>
> > >> As LS7A dts patch would depend on this series, and I want to
> > >> make the whole LS7A basic support as a part of 5.8 release.
> > >
> > >I think you have all necessary tags to take this in the MIPS
> > >tree, please let me know if that's the way we want this to go
> > >upstream - I would not pull MIPS/dts changes into the PCI tree
> > >and I don't think it is needed for this series.
> >
> > Please pull all PCI changes [1~3] into PCI tree.
>
> Pulled [1,2,3] into pci/mips, thanks !
could please take the other patches, too ?
Thomas.
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]
^ permalink raw reply
* Re: [PATCH v10 4/5] MIPS: DTS: Loongson64: Add PCI Controller Node
From: Thomas Bogendoerfer @ 2020-05-22 14:25 UTC (permalink / raw)
To: Jiaxun Yang
Cc: linux-pci, Bjorn Helgaas, Rob Herring, Huacai Chen,
Lorenzo Pieralisi, Paul Burton, devicetree, linux-kernel,
linux-mips
In-Reply-To: <20200514131650.3587281-4-jiaxun.yang@flygoat.com>
On Thu, May 14, 2020 at 09:16:40PM +0800, Jiaxun Yang wrote:
> Add PCI Host controller node for Loongson64 with RS780E PCH dts.
> Note that PCI interrupts are probed via legacy way, as different
> machine have different interrupt arrangement, we can't cover all
> of them in dt.
>
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
> arch/mips/boot/dts/loongson/rs780e-pch.dtsi | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
Acked-by: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Thomas.
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]
^ permalink raw reply
* Re: [PATCH v10 5/5] MIPS: Loongson64: Switch to generic PCI driver
From: Thomas Bogendoerfer @ 2020-05-22 14:25 UTC (permalink / raw)
To: Jiaxun Yang
Cc: linux-pci, Bjorn Helgaas, Rob Herring, Huacai Chen,
Lorenzo Pieralisi, Paul Burton, devicetree, linux-kernel,
linux-mips
In-Reply-To: <20200514131650.3587281-5-jiaxun.yang@flygoat.com>
On Thu, May 14, 2020 at 09:16:41PM +0800, Jiaxun Yang wrote:
> We can now enable generic PCI driver in Kconfig, and remove legacy
> PCI driver code.
>
> Radeon vbios quirk is moved to the platform folder to fit the
> new structure.
>
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> --
> v9: Fix licenses tag
> ---
> arch/mips/Kconfig | 1 +
> arch/mips/loongson64/Makefile | 2 +-
> arch/mips/loongson64/vbios_quirk.c | 29 ++++++++
> arch/mips/pci/Makefile | 1 -
> arch/mips/pci/fixup-loongson3.c | 71 ------------------
> arch/mips/pci/ops-loongson3.c | 116 -----------------------------
> 6 files changed, 31 insertions(+), 189 deletions(-)
> create mode 100644 arch/mips/loongson64/vbios_quirk.c
> delete mode 100644 arch/mips/pci/fixup-loongson3.c
> delete mode 100644 arch/mips/pci/ops-loongson3.c
Acked-by: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Thomas.
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]
^ permalink raw reply
* Re: [PATCH v2 1/4] dt-bindings: iio: imu: bmi160: convert txt format to yaml
From: Jonathan Albrieux @ 2020-05-22 14:26 UTC (permalink / raw)
To: Daniel Baluta
Cc: Jonathan Cameron, Jonathan Cameron, linux-kernel,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Hartmut Knaack, Lars-Peter Clausen,
open list:IIO SUBSYSTEM AND DRIVERS, Peter Meerwald-Stadler,
Rob Herring
In-Reply-To: <134f419f-de85-f14e-0331-49b8bb72625c@nxp.com>
On Fri, May 22, 2020 at 01:47:21PM +0300, Daniel Baluta wrote:
>
> > > > > > +
> > > > > > +maintainers:
> > > > > > + - can't find a mantainer, author is Daniel Baluta <daniel.baluta@intel.com>
> > > > > Daniel is still active in the kernel, just not at Intel any more. +CC
> > > > Oh ok thank you! Daniel are you still maintaining this driver?
>
> I can do reviews if requested but I'm not actively maintaining this driver.
> If anyone wants
>
> to take this over, will be more than happy.
>
>
> Other than that we can add my gmail address: Daniel Baluta
> <daniel.baluta@gmail.com>
>
>
>
Well if you'd like to review this patch I'd really appreciate :-)
Forgive me for not having understood your answer regarding the maintainer
field, can I add you to this binding as maintainer or are you saying to
not add you? Thank you and sorry for the repeated question,
Best regards,
Jonathan Albrieux
^ permalink raw reply
* [PATCH V5 4/5] iio: adc: Update debug prints
From: Jishnu Prakash @ 2020-05-22 14:24 UTC (permalink / raw)
To: agross, bjorn.andersson, devicetree, linux-kernel, mka,
linus.walleij, Jonathan.Cameron, andy.shevchenko, amit.kucheria,
smohanad, kgunda, aghayal, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, linux-arm-msm,
linux-iio
Cc: linux-arm-msm-owner, Jishnu Prakash
In-Reply-To: <1590157452-27179-1-git-send-email-jprakash@codeaurora.org>
Change pr_err/pr_debug statements to dev_err/dev_dbg for
increased clarity.
Signed-off-by: Jishnu Prakash <jprakash@codeaurora.org>
---
drivers/iio/adc/qcom-spmi-adc5.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/iio/adc/qcom-spmi-adc5.c b/drivers/iio/adc/qcom-spmi-adc5.c
index e1482fd..11eb97c 100644
--- a/drivers/iio/adc/qcom-spmi-adc5.c
+++ b/drivers/iio/adc/qcom-spmi-adc5.c
@@ -249,11 +249,11 @@ static int adc5_read_voltage_data(struct adc5_chip *adc, u16 *data)
*data = (rslt_msb << 8) | rslt_lsb;
if (*data == ADC5_USR_DATA_CHECK) {
- pr_err("Invalid data:0x%x\n", *data);
+ dev_err(adc->dev, "Invalid data:0x%x\n", *data);
return -EINVAL;
}
- pr_debug("voltage raw code:0x%x\n", *data);
+ dev_dbg(adc->dev, "voltage raw code:0x%x\n", *data);
return 0;
}
@@ -385,24 +385,24 @@ static int adc5_do_conversion(struct adc5_chip *adc,
ret = adc5_configure(adc, prop);
if (ret) {
- pr_err("ADC configure failed with %d\n", ret);
+ dev_err(adc->dev, "ADC configure failed with %d\n", ret);
goto unlock;
}
if (adc->poll_eoc) {
ret = adc5_poll_wait_eoc(adc);
if (ret) {
- pr_err("EOC bit not set\n");
+ dev_err(adc->dev, "EOC bit not set\n");
goto unlock;
}
} else {
ret = wait_for_completion_timeout(&adc->complete,
ADC5_CONV_TIMEOUT);
if (!ret) {
- pr_debug("Did not get completion timeout.\n");
+ dev_dbg(adc->dev, "Did not get completion timeout.\n");
ret = adc5_poll_wait_eoc(adc);
if (ret) {
- pr_err("EOC bit not set\n");
+ dev_err(adc->dev, "EOC bit not set\n");
goto unlock;
}
}
@@ -724,7 +724,7 @@ static int adc5_get_dt_channel_data(struct adc5_chip *adc,
channel_name = of_get_property(node,
"label", NULL) ? : node->name;
if (!channel_name) {
- pr_err("Invalid channel name\n");
+ dev_err(dev, "Invalid channel name\n");
return -EINVAL;
}
prop->datasheet_name = channel_name;
@@ -767,7 +767,7 @@ static int adc5_get_dt_channel_data(struct adc5_chip *adc,
return ret;
}
- pr_debug("dig_ver:minor:%d, major:%d\n", dig_version[0],
+ dev_dbg(dev, "dig_ver:minor:%d, major:%d\n", dig_version[0],
dig_version[1]);
/* Digital controller >= 5.3 have hw_settle_2 option */
if ((dig_version[0] >= ADC5_HW_SETTLE_DIFF_MINOR &&
@@ -971,7 +971,7 @@ static int adc5_probe(struct platform_device *pdev)
ret = adc5_get_dt_data(adc, node);
if (ret) {
- pr_err("adc get dt data failed\n");
+ dev_err(dev, "adc get dt data failed\n");
return ret;
}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related
* [PATCH V5 5/5] iio: adc: Clean up ADC code common to PMIC5 and PMIC7
From: Jishnu Prakash @ 2020-05-22 14:24 UTC (permalink / raw)
To: agross, bjorn.andersson, devicetree, linux-kernel, mka,
linus.walleij, Jonathan.Cameron, andy.shevchenko, amit.kucheria,
smohanad, kgunda, aghayal, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, linux-arm-msm,
linux-iio
Cc: linux-arm-msm-owner, Jishnu Prakash
In-Reply-To: <1590157452-27179-1-git-send-email-jprakash@codeaurora.org>
This commit includes the following changes:
Add a common function used for read_raw callback for both PMIC5
and PMIC7 ADCs.
Add exit function for ADC.
Add info_property under adc_data to more efficiently distinguish
PMIC5 and PMIC7 ADCs.
Signed-off-by: Jishnu Prakash <jprakash@codeaurora.org>
---
drivers/iio/adc/qcom-spmi-adc5.c | 81 +++++++++++++++++++++-----------------
drivers/iio/adc/qcom-vadc-common.h | 1 +
2 files changed, 46 insertions(+), 36 deletions(-)
diff --git a/drivers/iio/adc/qcom-spmi-adc5.c b/drivers/iio/adc/qcom-spmi-adc5.c
index 11eb97c..0208da3 100644
--- a/drivers/iio/adc/qcom-spmi-adc5.c
+++ b/drivers/iio/adc/qcom-spmi-adc5.c
@@ -167,8 +167,6 @@ static const struct vadc_prescale_ratio adc5_prescale_ratios[] = {
{.num = 1, .den = 16}
};
-static const struct adc5_data adc7_data_pmic;
-
static int adc5_read(struct adc5_chip *adc, u16 offset, u8 *data, int len)
{
return regmap_bulk_read(adc->regmap, adc->base + offset, data, len);
@@ -452,6 +450,13 @@ static int adc7_do_conversion(struct adc5_chip *adc,
return ret;
}
+struct adc_do_conversion {
+ int (*adc_do_conversion)(struct adc5_chip *adc,
+ struct adc5_channel_prop *prop,
+ struct iio_chan_spec const *chan,
+ u16 *data_volt, u16 *data_cur);
+};
+
static irqreturn_t adc5_isr(int irq, void *dev_id)
{
struct adc5_chip *adc = dev_id;
@@ -490,9 +495,9 @@ static int adc7_of_xlate(struct iio_dev *indio_dev,
return -EINVAL;
}
-static int adc5_read_raw(struct iio_dev *indio_dev,
+static int adc_read_raw_common(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan, int *val, int *val2,
- long mask)
+ long mask, struct adc_do_conversion do_conv)
{
struct adc5_chip *adc = iio_priv(indio_dev);
struct adc5_channel_prop *prop;
@@ -503,8 +508,8 @@ static int adc5_read_raw(struct iio_dev *indio_dev,
switch (mask) {
case IIO_CHAN_INFO_PROCESSED:
- ret = adc5_do_conversion(adc, prop, chan,
- &adc_code_volt, &adc_code_cur);
+ ret = do_conv.adc_do_conversion(adc, prop, chan,
+ &adc_code_volt, &adc_code_cur);
if (ret)
return ret;
@@ -521,36 +526,26 @@ static int adc5_read_raw(struct iio_dev *indio_dev,
}
}
-static int adc7_read_raw(struct iio_dev *indio_dev,
+static int adc5_read_raw(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan, int *val, int *val2,
long mask)
{
- struct adc5_chip *adc = iio_priv(indio_dev);
- struct adc5_channel_prop *prop;
- u16 adc_code_volt, adc_code_cur;
- int ret;
-
- prop = &adc->chan_props[chan->address];
+ struct adc_do_conversion do_conv;
- switch (mask) {
- case IIO_CHAN_INFO_PROCESSED:
- ret = adc7_do_conversion(adc, prop, chan,
- &adc_code_volt, &adc_code_cur);
- if (ret)
- return ret;
-
- ret = qcom_adc5_hw_scale(prop->scale_fn_type,
- &adc5_prescale_ratios[prop->prescale],
- adc->data,
- adc_code_volt, val);
+ do_conv.adc_do_conversion = adc5_do_conversion;
+ return adc_read_raw_common(indio_dev, chan, val, val2,
+ mask, do_conv);
+}
- if (ret)
- return ret;
+static int adc7_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan, int *val, int *val2,
+ long mask)
+{
+ struct adc_do_conversion do_conv;
- return IIO_VAL_INT;
- default:
- return -EINVAL;
- }
+ do_conv.adc_do_conversion = adc7_do_conversion;
+ return adc_read_raw_common(indio_dev, chan, val, val2,
+ mask, do_conv);
}
static const struct iio_info adc5_info = {
@@ -706,7 +701,7 @@ static int adc5_get_dt_channel_data(struct adc5_chip *adc,
/* virtual channel number = sid << 8 | channel number */
- if (adc->data == &adc7_data_pmic) {
+ if (adc->data->info == &adc7_info) {
sid = chan >> ADC_CHANNEL_OFFSET;
chan = chan & ADC_CHANNEL_MASK;
}
@@ -772,7 +767,7 @@ static int adc5_get_dt_channel_data(struct adc5_chip *adc,
/* Digital controller >= 5.3 have hw_settle_2 option */
if ((dig_version[0] >= ADC5_HW_SETTLE_DIFF_MINOR &&
dig_version[1] >= ADC5_HW_SETTLE_DIFF_MAJOR) ||
- adc->data == &adc7_data_pmic)
+ adc->data->info == &adc7_info)
ret = adc5_hw_settle_time_from_dt(value,
data->hw_settle_2);
else
@@ -822,6 +817,7 @@ static const struct adc5_data adc5_data_pmic = {
.full_scale_code_volt = 0x70e4,
.full_scale_code_cur = 0x2710,
.adc_chans = adc5_chans_pmic,
+ .info = &adc5_info,
.decimation = (unsigned int [ADC5_DECIMATION_SAMPLES_MAX])
{250, 420, 840},
.hw_settle_1 = (unsigned int [VADC_HW_SETTLE_SAMPLES_MAX])
@@ -835,6 +831,7 @@ static const struct adc5_data adc5_data_pmic = {
static const struct adc5_data adc7_data_pmic = {
.full_scale_code_volt = 0x70e4,
.adc_chans = adc7_chans_pmic,
+ .info = &adc7_info,
.decimation = (unsigned int [ADC5_DECIMATION_SAMPLES_MAX])
{85, 340, 1360},
.hw_settle_2 = (unsigned int [VADC_HW_SETTLE_SAMPLES_MAX])
@@ -847,6 +844,7 @@ static const struct adc5_data adc5_data_pmic_rev2 = {
.full_scale_code_volt = 0x4000,
.full_scale_code_cur = 0x1800,
.adc_chans = adc5_chans_rev2,
+ .info = &adc5_info,
.decimation = (unsigned int [ADC5_DECIMATION_SAMPLES_MAX])
{256, 512, 1024},
.hw_settle_1 = (unsigned int [VADC_HW_SETTLE_SAMPLES_MAX])
@@ -961,10 +959,7 @@ static int adc5_probe(struct platform_device *pdev)
adc->dev = dev;
adc->base = reg;
- if (of_device_is_compatible(node, "qcom,spmi-adc7"))
- indio_dev->info = &adc7_info;
- else
- indio_dev->info = &adc5_info;
+ platform_set_drvdata(pdev, adc);
init_completion(&adc->complete);
mutex_init(&adc->lock);
@@ -975,6 +970,8 @@ static int adc5_probe(struct platform_device *pdev)
return ret;
}
+ indio_dev->info = adc->data->info;
+
irq_eoc = platform_get_irq(pdev, 0);
if (irq_eoc < 0) {
if (irq_eoc == -EPROBE_DEFER || irq_eoc == -EINVAL)
@@ -987,6 +984,8 @@ static int adc5_probe(struct platform_device *pdev)
return ret;
}
+ adc->irq_eoc = irq_eoc;
+
indio_dev->dev.parent = dev;
indio_dev->dev.of_node = node;
indio_dev->name = pdev->name;
@@ -997,12 +996,22 @@ static int adc5_probe(struct platform_device *pdev)
return devm_iio_device_register(dev, indio_dev);
}
+static int adc5_exit(struct platform_device *pdev)
+{
+ struct adc5_chip *adc = platform_get_drvdata(pdev);
+
+ if (adc->irq_eoc >= 0)
+ disable_irq(adc->irq_eoc);
+ return 0;
+}
+
static struct platform_driver adc5_driver = {
.driver = {
.name = "qcom-spmi-adc5.c",
.of_match_table = adc5_match_table,
},
.probe = adc5_probe,
+ .remove = adc5_exit,
};
module_platform_driver(adc5_driver);
diff --git a/drivers/iio/adc/qcom-vadc-common.h b/drivers/iio/adc/qcom-vadc-common.h
index f10250b..17b2fc4 100644
--- a/drivers/iio/adc/qcom-vadc-common.h
+++ b/drivers/iio/adc/qcom-vadc-common.h
@@ -150,6 +150,7 @@ struct adc5_data {
const u32 full_scale_code_volt;
const u32 full_scale_code_cur;
const struct adc5_channels *adc_chans;
+ const struct iio_info *info;
unsigned int *decimation;
unsigned int *hw_settle_1;
unsigned int *hw_settle_2;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related
* [PATCH V5 3/5] iio: adc: Add support for PMIC7 ADC
From: Jishnu Prakash @ 2020-05-22 14:24 UTC (permalink / raw)
To: agross, bjorn.andersson, devicetree, linux-kernel, mka,
linus.walleij, Jonathan.Cameron, andy.shevchenko, amit.kucheria,
smohanad, kgunda, aghayal, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, linux-arm-msm,
linux-iio
Cc: linux-arm-msm-owner, Jishnu Prakash
In-Reply-To: <1590157452-27179-1-git-send-email-jprakash@codeaurora.org>
The ADC architecture on PMIC7 is changed as compared to PMIC5. The
major change from PMIC5 is that all SW communication to ADC goes through
PMK8350, which communicates with other PMICs through PBS when the ADC
on PMK8350 works in master mode. The SID register is used to identify the
PMICs with which the PBS needs to communicate. Add support for the same.
Signed-off-by: Jishnu Prakash <jprakash@codeaurora.org>
---
drivers/iio/adc/qcom-spmi-adc5.c | 231 ++++++++++++++++++++++++++++++--
drivers/iio/adc/qcom-vadc-common.c | 260 +++++++++++++++++++++++++++++++++++++
drivers/iio/adc/qcom-vadc-common.h | 14 ++
3 files changed, 496 insertions(+), 9 deletions(-)
diff --git a/drivers/iio/adc/qcom-spmi-adc5.c b/drivers/iio/adc/qcom-spmi-adc5.c
index 21fdcde..e1482fd 100644
--- a/drivers/iio/adc/qcom-spmi-adc5.c
+++ b/drivers/iio/adc/qcom-spmi-adc5.c
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-2.0
/*
- * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2018, 2020, The Linux Foundation. All rights reserved.
*/
#include <linux/bitops.h>
@@ -23,6 +23,7 @@
#define ADC5_USR_REVISION1 0x0
#define ADC5_USR_STATUS1 0x8
+#define ADC5_USR_STATUS1_CONV_FAULT BIT(7)
#define ADC5_USR_STATUS1_REQ_STS BIT(1)
#define ADC5_USR_STATUS1_EOC BIT(0)
#define ADC5_USR_STATUS1_REQ_STS_EOC_MASK 0x3
@@ -65,6 +66,9 @@
#define ADC5_USR_IBAT_DATA1 0x53
+#define ADC_CHANNEL_OFFSET 0x8
+#define ADC_CHANNEL_MASK GENMASK(7, 0)
+
/*
* Conversion time varies based on the decimation, clock rate, fast average
* samples and measurements queued across different VADC peripherals.
@@ -79,6 +83,11 @@
#define ADC5_HW_SETTLE_DIFF_MINOR 3
#define ADC5_HW_SETTLE_DIFF_MAJOR 5
+/* For PMIC7 */
+#define ADC_APP_SID 0x40
+#define ADC_APP_SID_MASK GENMASK(3, 0)
+#define ADC7_CONV_TIMEOUT msecs_to_jiffies(10)
+
enum adc5_cal_method {
ADC5_NO_CAL = 0,
ADC5_RATIOMETRIC_CAL,
@@ -96,6 +105,7 @@ enum adc5_cal_val {
* @cal_method: calibration method.
* @cal_val: calibration value
* @decimation: sampling rate supported for the channel.
+ * @sid: slave id of PMIC owning the channel, for PMIC7.
* @prescale: channel scaling performed on the input signal.
* @hw_settle_time: the time between AMUX being configured and the
* start of conversion.
@@ -110,6 +120,7 @@ struct adc5_channel_prop {
enum adc5_cal_method cal_method;
enum adc5_cal_val cal_val;
unsigned int decimation;
+ unsigned int sid;
unsigned int prescale;
unsigned int hw_settle_time;
unsigned int avg_samples;
@@ -140,6 +151,7 @@ struct adc5_chip {
bool poll_eoc;
struct completion complete;
struct mutex lock;
+ int irq_eoc;
const struct adc5_data *data;
};
@@ -155,6 +167,8 @@ static const struct vadc_prescale_ratio adc5_prescale_ratios[] = {
{.num = 1, .den = 16}
};
+static const struct adc5_data adc7_data_pmic;
+
static int adc5_read(struct adc5_chip *adc, u16 offset, u8 *data, int len)
{
return regmap_bulk_read(adc->regmap, adc->base + offset, data, len);
@@ -165,6 +179,11 @@ static int adc5_write(struct adc5_chip *adc, u16 offset, u8 *data, int len)
return regmap_bulk_write(adc->regmap, adc->base + offset, data, len);
}
+static int adc5_masked_write(struct adc5_chip *adc, u16 offset, u8 mask, u8 val)
+{
+ return regmap_update_bits(adc->regmap, adc->base + offset, mask, val);
+}
+
static int adc5_prescaling_from_dt(u32 num, u32 den)
{
unsigned int pre;
@@ -285,7 +304,7 @@ static int adc5_configure(struct adc5_chip *adc,
/* Read registers 0x42 through 0x46 */
ret = adc5_read(adc, ADC5_USR_DIG_PARAM, buf, sizeof(buf));
- if (ret < 0)
+ if (ret)
return ret;
/* Digital param selection */
@@ -314,6 +333,47 @@ static int adc5_configure(struct adc5_chip *adc,
return adc5_write(adc, ADC5_USR_DIG_PARAM, buf, sizeof(buf));
}
+static int adc7_configure(struct adc5_chip *adc,
+ struct adc5_channel_prop *prop)
+{
+ int ret;
+ u8 conv_req = 0, buf[4];
+
+ ret = adc5_masked_write(adc, ADC_APP_SID, ADC_APP_SID_MASK, prop->sid);
+ if (ret)
+ return ret;
+
+ ret = adc5_read(adc, ADC5_USR_DIG_PARAM, buf, sizeof(buf));
+ if (ret)
+ return ret;
+
+ /* Digital param selection */
+ adc5_update_dig_param(adc, prop, &buf[0]);
+
+ /* Update fast average sample value */
+ buf[1] &= ~ADC5_USR_FAST_AVG_CTL_SAMPLES_MASK;
+ buf[1] |= prop->avg_samples;
+
+ /* Select ADC channel */
+ buf[2] = prop->channel;
+
+ /* Select HW settle delay for channel */
+ buf[3] &= ~ADC5_USR_HW_SETTLE_DELAY_MASK;
+ buf[3] |= prop->hw_settle_time;
+
+ /* Select CONV request */
+ conv_req = ADC5_USR_CONV_REQ_REQ;
+
+ if (!adc->poll_eoc)
+ reinit_completion(&adc->complete);
+
+ ret = adc5_write(adc, ADC5_USR_DIG_PARAM, buf, sizeof(buf));
+ if (ret)
+ return ret;
+
+ return adc5_write(adc, ADC5_USR_CONV_REQ, &conv_req, 1);
+}
+
static int adc5_do_conversion(struct adc5_chip *adc,
struct adc5_channel_prop *prop,
struct iio_chan_spec const *chan,
@@ -331,7 +391,7 @@ static int adc5_do_conversion(struct adc5_chip *adc,
if (adc->poll_eoc) {
ret = adc5_poll_wait_eoc(adc);
- if (ret < 0) {
+ if (ret) {
pr_err("EOC bit not set\n");
goto unlock;
}
@@ -341,7 +401,7 @@ static int adc5_do_conversion(struct adc5_chip *adc,
if (!ret) {
pr_debug("Did not get completion timeout.\n");
ret = adc5_poll_wait_eoc(adc);
- if (ret < 0) {
+ if (ret) {
pr_err("EOC bit not set\n");
goto unlock;
}
@@ -355,6 +415,43 @@ static int adc5_do_conversion(struct adc5_chip *adc,
return ret;
}
+static int adc7_do_conversion(struct adc5_chip *adc,
+ struct adc5_channel_prop *prop,
+ struct iio_chan_spec const *chan,
+ u16 *data_volt, u16 *data_cur)
+{
+ int ret;
+ u8 status;
+
+ mutex_lock(&adc->lock);
+
+ ret = adc7_configure(adc, prop);
+ if (ret) {
+ dev_err(adc->dev, "ADC configure failed with %d\n", ret);
+ goto unlock;
+ }
+
+ /* No support for polling mode at present */
+ wait_for_completion_timeout(&adc->complete, ADC7_CONV_TIMEOUT);
+
+ ret = adc5_read(adc, ADC5_USR_STATUS1, &status, 1);
+ if (ret)
+ goto unlock;
+
+ if (status & ADC5_USR_STATUS1_CONV_FAULT) {
+ dev_err(adc->dev, "Unexpected conversion fault\n");
+ ret = -EIO;
+ goto unlock;
+ }
+
+ ret = adc5_read_voltage_data(adc, data_volt);
+
+unlock:
+ mutex_unlock(&adc->lock);
+
+ return ret;
+}
+
static irqreturn_t adc5_isr(int irq, void *dev_id)
{
struct adc5_chip *adc = dev_id;
@@ -377,6 +474,22 @@ static int adc5_of_xlate(struct iio_dev *indio_dev,
return -EINVAL;
}
+static int adc7_of_xlate(struct iio_dev *indio_dev,
+ const struct of_phandle_args *iiospec)
+{
+ struct adc5_chip *adc = iio_priv(indio_dev);
+ int i, v_channel;
+
+ for (i = 0; i < adc->nchannels; i++) {
+ v_channel = (adc->chan_props[i].sid << ADC_CHANNEL_OFFSET) |
+ adc->chan_props[i].channel;
+ if (v_channel == iiospec->args[0])
+ return i;
+ }
+
+ return -EINVAL;
+}
+
static int adc5_read_raw(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan, int *val, int *val2,
long mask)
@@ -406,8 +519,38 @@ static int adc5_read_raw(struct iio_dev *indio_dev,
default:
return -EINVAL;
}
+}
- return 0;
+static int adc7_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan, int *val, int *val2,
+ long mask)
+{
+ struct adc5_chip *adc = iio_priv(indio_dev);
+ struct adc5_channel_prop *prop;
+ u16 adc_code_volt, adc_code_cur;
+ int ret;
+
+ prop = &adc->chan_props[chan->address];
+
+ switch (mask) {
+ case IIO_CHAN_INFO_PROCESSED:
+ ret = adc7_do_conversion(adc, prop, chan,
+ &adc_code_volt, &adc_code_cur);
+ if (ret)
+ return ret;
+
+ ret = qcom_adc5_hw_scale(prop->scale_fn_type,
+ &adc5_prescale_ratios[prop->prescale],
+ adc->data,
+ adc_code_volt, val);
+
+ if (ret)
+ return ret;
+
+ return IIO_VAL_INT;
+ default:
+ return -EINVAL;
+ }
}
static const struct iio_info adc5_info = {
@@ -415,6 +558,11 @@ static const struct iio_info adc5_info = {
.of_xlate = adc5_of_xlate,
};
+static const struct iio_info adc7_info = {
+ .read_raw = adc7_read_raw,
+ .of_xlate = adc7_of_xlate,
+};
+
struct adc5_channels {
const char *datasheet_name;
unsigned int prescale_index;
@@ -477,6 +625,39 @@ static const struct adc5_channels adc5_chans_pmic[ADC5_MAX_CHANNEL] = {
SCALE_HW_CALIB_PM5_SMB_TEMP)
};
+static const struct adc5_channels adc7_chans_pmic[ADC5_MAX_CHANNEL] = {
+ [ADC7_REF_GND] = ADC5_CHAN_VOLT("ref_gnd", 0,
+ SCALE_HW_CALIB_DEFAULT)
+ [ADC7_1P25VREF] = ADC5_CHAN_VOLT("vref_1p25", 0,
+ SCALE_HW_CALIB_DEFAULT)
+ [ADC7_VPH_PWR] = ADC5_CHAN_VOLT("vph_pwr", 1,
+ SCALE_HW_CALIB_DEFAULT)
+ [ADC7_VBAT_SNS] = ADC5_CHAN_VOLT("vbat_sns", 3,
+ SCALE_HW_CALIB_DEFAULT)
+ [ADC7_DIE_TEMP] = ADC5_CHAN_TEMP("die_temp", 0,
+ SCALE_HW_CALIB_PMIC_THERM_PM7)
+ [ADC7_AMUX_THM1_100K_PU] = ADC5_CHAN_TEMP("amux_thm1_pu2", 0,
+ SCALE_HW_CALIB_THERM_100K_PU_PM7)
+ [ADC7_AMUX_THM2_100K_PU] = ADC5_CHAN_TEMP("amux_thm2_pu2", 0,
+ SCALE_HW_CALIB_THERM_100K_PU_PM7)
+ [ADC7_AMUX_THM3_100K_PU] = ADC5_CHAN_TEMP("amux_thm3_pu2", 0,
+ SCALE_HW_CALIB_THERM_100K_PU_PM7)
+ [ADC7_AMUX_THM4_100K_PU] = ADC5_CHAN_TEMP("amux_thm4_pu2", 0,
+ SCALE_HW_CALIB_THERM_100K_PU_PM7)
+ [ADC7_AMUX_THM5_100K_PU] = ADC5_CHAN_TEMP("amux_thm5_pu2", 0,
+ SCALE_HW_CALIB_THERM_100K_PU_PM7)
+ [ADC7_AMUX_THM6_100K_PU] = ADC5_CHAN_TEMP("amux_thm6_pu2", 0,
+ SCALE_HW_CALIB_THERM_100K_PU_PM7)
+ [ADC7_GPIO1_100K_PU] = ADC5_CHAN_TEMP("gpio1_pu2", 0,
+ SCALE_HW_CALIB_THERM_100K_PU_PM7)
+ [ADC7_GPIO2_100K_PU] = ADC5_CHAN_TEMP("gpio2_pu2", 0,
+ SCALE_HW_CALIB_THERM_100K_PU_PM7)
+ [ADC7_GPIO3_100K_PU] = ADC5_CHAN_TEMP("gpio3_pu2", 0,
+ SCALE_HW_CALIB_THERM_100K_PU_PM7)
+ [ADC7_GPIO4_100K_PU] = ADC5_CHAN_TEMP("gpio4_pu2", 0,
+ SCALE_HW_CALIB_THERM_100K_PU_PM7)
+};
+
static const struct adc5_channels adc5_chans_rev2[ADC5_MAX_CHANNEL] = {
[ADC5_REF_GND] = ADC5_CHAN_VOLT("ref_gnd", 0,
SCALE_HW_CALIB_DEFAULT)
@@ -511,6 +692,7 @@ static int adc5_get_dt_channel_data(struct adc5_chip *adc,
{
const char *name = node->name, *channel_name;
u32 chan, value, varr[2];
+ u32 sid = 0;
int ret;
struct device *dev = adc->dev;
@@ -520,6 +702,15 @@ static int adc5_get_dt_channel_data(struct adc5_chip *adc,
return ret;
}
+ /* Value read from "reg" is virtual channel number */
+
+ /* virtual channel number = sid << 8 | channel number */
+
+ if (adc->data == &adc7_data_pmic) {
+ sid = chan >> ADC_CHANNEL_OFFSET;
+ chan = chan & ADC_CHANNEL_MASK;
+ }
+
if (chan > ADC5_PARALLEL_ISENSE_VBAT_IDATA ||
!data->adc_chans[chan].datasheet_name) {
dev_err(dev, "%s invalid channel number %d\n", name, chan);
@@ -528,6 +719,7 @@ static int adc5_get_dt_channel_data(struct adc5_chip *adc,
/* the channel has DT description */
prop->channel = chan;
+ prop->sid = sid;
channel_name = of_get_property(node,
"label", NULL) ? : node->name;
@@ -570,7 +762,7 @@ static int adc5_get_dt_channel_data(struct adc5_chip *adc,
ret = adc5_read(adc, ADC5_USR_REVISION1, dig_version,
sizeof(dig_version));
- if (ret < 0) {
+ if (ret) {
dev_err(dev, "Invalid dig version read %d\n", ret);
return ret;
}
@@ -578,8 +770,9 @@ static int adc5_get_dt_channel_data(struct adc5_chip *adc,
pr_debug("dig_ver:minor:%d, major:%d\n", dig_version[0],
dig_version[1]);
/* Digital controller >= 5.3 have hw_settle_2 option */
- if (dig_version[0] >= ADC5_HW_SETTLE_DIFF_MINOR &&
- dig_version[1] >= ADC5_HW_SETTLE_DIFF_MAJOR)
+ if ((dig_version[0] >= ADC5_HW_SETTLE_DIFF_MINOR &&
+ dig_version[1] >= ADC5_HW_SETTLE_DIFF_MAJOR) ||
+ adc->data == &adc7_data_pmic)
ret = adc5_hw_settle_time_from_dt(value,
data->hw_settle_2);
else
@@ -639,6 +832,17 @@ static const struct adc5_data adc5_data_pmic = {
1, 2, 4, 8, 16, 32, 64, 128},
};
+static const struct adc5_data adc7_data_pmic = {
+ .full_scale_code_volt = 0x70e4,
+ .adc_chans = adc7_chans_pmic,
+ .decimation = (unsigned int [ADC5_DECIMATION_SAMPLES_MAX])
+ {85, 340, 1360},
+ .hw_settle_2 = (unsigned int [VADC_HW_SETTLE_SAMPLES_MAX])
+ {15, 100, 200, 300, 400, 500, 600, 700,
+ 1000, 2000, 4000, 8000, 16000, 32000,
+ 64000, 128000},
+};
+
static const struct adc5_data adc5_data_pmic_rev2 = {
.full_scale_code_volt = 0x4000,
.full_scale_code_cur = 0x1800,
@@ -659,6 +863,10 @@ static const struct of_device_id adc5_match_table[] = {
.data = &adc5_data_pmic,
},
{
+ .compatible = "qcom,spmi-adc7",
+ .data = &adc7_data_pmic,
+ },
+ {
.compatible = "qcom,spmi-adc-rev2",
.data = &adc5_data_pmic_rev2,
},
@@ -752,6 +960,12 @@ static int adc5_probe(struct platform_device *pdev)
adc->regmap = regmap;
adc->dev = dev;
adc->base = reg;
+
+ if (of_device_is_compatible(node, "qcom,spmi-adc7"))
+ indio_dev->info = &adc7_info;
+ else
+ indio_dev->info = &adc5_info;
+
init_completion(&adc->complete);
mutex_init(&adc->lock);
@@ -777,7 +991,6 @@ static int adc5_probe(struct platform_device *pdev)
indio_dev->dev.of_node = node;
indio_dev->name = pdev->name;
indio_dev->modes = INDIO_DIRECT_MODE;
- indio_dev->info = &adc5_info;
indio_dev->channels = adc->iio_chans;
indio_dev->num_channels = adc->nchannels;
diff --git a/drivers/iio/adc/qcom-vadc-common.c b/drivers/iio/adc/qcom-vadc-common.c
index 2bb78d1..b6915c2 100644
--- a/drivers/iio/adc/qcom-vadc-common.c
+++ b/drivers/iio/adc/qcom-vadc-common.c
@@ -89,6 +89,195 @@ static const struct vadc_map_pt adcmap_100k_104ef_104fb_1875_vref[] = {
{ 46, 125000 },
};
+static const struct vadc_map_pt adcmap7_die_temp[] = {
+ { 433700, 1967},
+ { 473100, 1964},
+ { 512400, 1957},
+ { 551500, 1949},
+ { 590500, 1940},
+ { 629300, 1930},
+ { 667900, 1921},
+ { 706400, 1910},
+ { 744600, 1896},
+ { 782500, 1878},
+ { 820100, 1859},
+ { 857300, 0},
+};
+
+/*
+ * Resistance to temperature table for 100k pull up for NTCG104EF104.
+ */
+static const struct vadc_map_pt adcmap7_100k[] = {
+ { 4250657, -40960 },
+ { 3962085, -39936 },
+ { 3694875, -38912 },
+ { 3447322, -37888 },
+ { 3217867, -36864 },
+ { 3005082, -35840 },
+ { 2807660, -34816 },
+ { 2624405, -33792 },
+ { 2454218, -32768 },
+ { 2296094, -31744 },
+ { 2149108, -30720 },
+ { 2012414, -29696 },
+ { 1885232, -28672 },
+ { 1766846, -27648 },
+ { 1656598, -26624 },
+ { 1553884, -25600 },
+ { 1458147, -24576 },
+ { 1368873, -23552 },
+ { 1285590, -22528 },
+ { 1207863, -21504 },
+ { 1135290, -20480 },
+ { 1067501, -19456 },
+ { 1004155, -18432 },
+ { 944935, -17408 },
+ { 889550, -16384 },
+ { 837731, -15360 },
+ { 789229, -14336 },
+ { 743813, -13312 },
+ { 701271, -12288 },
+ { 661405, -11264 },
+ { 624032, -10240 },
+ { 588982, -9216 },
+ { 556100, -8192 },
+ { 525239, -7168 },
+ { 496264, -6144 },
+ { 469050, -5120 },
+ { 443480, -4096 },
+ { 419448, -3072 },
+ { 396851, -2048 },
+ { 375597, -1024 },
+ { 355598, 0 },
+ { 336775, 1024 },
+ { 319052, 2048 },
+ { 302359, 3072 },
+ { 286630, 4096 },
+ { 271806, 5120 },
+ { 257829, 6144 },
+ { 244646, 7168 },
+ { 232209, 8192 },
+ { 220471, 9216 },
+ { 209390, 10240 },
+ { 198926, 11264 },
+ { 189040, 12288 },
+ { 179698, 13312 },
+ { 170868, 14336 },
+ { 162519, 15360 },
+ { 154622, 16384 },
+ { 147150, 17408 },
+ { 140079, 18432 },
+ { 133385, 19456 },
+ { 127046, 20480 },
+ { 121042, 21504 },
+ { 115352, 22528 },
+ { 109960, 23552 },
+ { 104848, 24576 },
+ { 100000, 25600 },
+ { 95402, 26624 },
+ { 91038, 27648 },
+ { 86897, 28672 },
+ { 82965, 29696 },
+ { 79232, 30720 },
+ { 75686, 31744 },
+ { 72316, 32768 },
+ { 69114, 33792 },
+ { 66070, 34816 },
+ { 63176, 35840 },
+ { 60423, 36864 },
+ { 57804, 37888 },
+ { 55312, 38912 },
+ { 52940, 39936 },
+ { 50681, 40960 },
+ { 48531, 41984 },
+ { 46482, 43008 },
+ { 44530, 44032 },
+ { 42670, 45056 },
+ { 40897, 46080 },
+ { 39207, 47104 },
+ { 37595, 48128 },
+ { 36057, 49152 },
+ { 34590, 50176 },
+ { 33190, 51200 },
+ { 31853, 52224 },
+ { 30577, 53248 },
+ { 29358, 54272 },
+ { 28194, 55296 },
+ { 27082, 56320 },
+ { 26020, 57344 },
+ { 25004, 58368 },
+ { 24033, 59392 },
+ { 23104, 60416 },
+ { 22216, 61440 },
+ { 21367, 62464 },
+ { 20554, 63488 },
+ { 19776, 64512 },
+ { 19031, 65536 },
+ { 18318, 66560 },
+ { 17636, 67584 },
+ { 16982, 68608 },
+ { 16355, 69632 },
+ { 15755, 70656 },
+ { 15180, 71680 },
+ { 14628, 72704 },
+ { 14099, 73728 },
+ { 13592, 74752 },
+ { 13106, 75776 },
+ { 12640, 76800 },
+ { 12192, 77824 },
+ { 11762, 78848 },
+ { 11350, 79872 },
+ { 10954, 80896 },
+ { 10574, 81920 },
+ { 10209, 82944 },
+ { 9858, 83968 },
+ { 9521, 84992 },
+ { 9197, 86016 },
+ { 8886, 87040 },
+ { 8587, 88064 },
+ { 8299, 89088 },
+ { 8023, 90112 },
+ { 7757, 91136 },
+ { 7501, 92160 },
+ { 7254, 93184 },
+ { 7017, 94208 },
+ { 6789, 95232 },
+ { 6570, 96256 },
+ { 6358, 97280 },
+ { 6155, 98304 },
+ { 5959, 99328 },
+ { 5770, 100352 },
+ { 5588, 101376 },
+ { 5412, 102400 },
+ { 5243, 103424 },
+ { 5080, 104448 },
+ { 4923, 105472 },
+ { 4771, 106496 },
+ { 4625, 107520 },
+ { 4484, 108544 },
+ { 4348, 109568 },
+ { 4217, 110592 },
+ { 4090, 111616 },
+ { 3968, 112640 },
+ { 3850, 113664 },
+ { 3736, 114688 },
+ { 3626, 115712 },
+ { 3519, 116736 },
+ { 3417, 117760 },
+ { 3317, 118784 },
+ { 3221, 119808 },
+ { 3129, 120832 },
+ { 3039, 121856 },
+ { 2952, 122880 },
+ { 2868, 123904 },
+ { 2787, 124928 },
+ { 2709, 125952 },
+ { 2633, 126976 },
+ { 2560, 128000 },
+ { 2489, 129024 },
+ { 2420, 130048 }
+};
+
static int qcom_vadc_scale_hw_calib_volt(
const struct vadc_prescale_ratio *prescale,
const struct adc5_data *data,
@@ -97,6 +286,10 @@ static int qcom_vadc_scale_hw_calib_therm(
const struct vadc_prescale_ratio *prescale,
const struct adc5_data *data,
u16 adc_code, int *result_mdec);
+static int qcom_vadc7_scale_hw_calib_therm(
+ const struct vadc_prescale_ratio *prescale,
+ const struct adc5_data *data,
+ u16 adc_code, int *result_mdec);
static int qcom_vadc_scale_hw_smb_temp(
const struct vadc_prescale_ratio *prescale,
const struct adc5_data *data,
@@ -109,12 +302,20 @@ static int qcom_vadc_scale_hw_calib_die_temp(
const struct vadc_prescale_ratio *prescale,
const struct adc5_data *data,
u16 adc_code, int *result_mdec);
+static int qcom_vadc7_scale_hw_calib_die_temp(
+ const struct vadc_prescale_ratio *prescale,
+ const struct adc5_data *data,
+ u16 adc_code, int *result_mdec);
static struct qcom_adc5_scale_type scale_adc5_fn[] = {
[SCALE_HW_CALIB_DEFAULT] = {qcom_vadc_scale_hw_calib_volt},
[SCALE_HW_CALIB_THERM_100K_PULLUP] = {qcom_vadc_scale_hw_calib_therm},
[SCALE_HW_CALIB_XOTHERM] = {qcom_vadc_scale_hw_calib_therm},
+ [SCALE_HW_CALIB_THERM_100K_PU_PM7] = {
+ qcom_vadc7_scale_hw_calib_therm},
[SCALE_HW_CALIB_PMIC_THERM] = {qcom_vadc_scale_hw_calib_die_temp},
+ [SCALE_HW_CALIB_PMIC_THERM_PM7] = {
+ qcom_vadc7_scale_hw_calib_die_temp},
[SCALE_HW_CALIB_PM5_CHG_TEMP] = {qcom_vadc_scale_hw_chg5_temp},
[SCALE_HW_CALIB_PM5_SMB_TEMP] = {qcom_vadc_scale_hw_smb_temp},
};
@@ -291,6 +492,32 @@ static int qcom_vadc_scale_code_voltage_factor(u16 adc_code,
return (int) voltage;
}
+static int qcom_vadc7_scale_hw_calib_therm(
+ const struct vadc_prescale_ratio *prescale,
+ const struct adc5_data *data,
+ u16 adc_code, int *result_mdec)
+{
+ s64 resistance = adc_code;
+ int ret, result;
+
+ if (adc_code >= RATIO_MAX_ADC7)
+ return -EINVAL;
+
+ /* (ADC code * R_PULLUP (100Kohm)) / (full_scale_code - ADC code)*/
+ resistance *= R_PU_100K;
+ resistance = div64_s64(resistance, RATIO_MAX_ADC7 - adc_code);
+
+ ret = qcom_vadc_map_voltage_temp(adcmap7_100k,
+ ARRAY_SIZE(adcmap7_100k),
+ resistance, &result);
+ if (ret)
+ return ret;
+
+ *result_mdec = result;
+
+ return 0;
+}
+
static int qcom_vadc_scale_hw_calib_volt(
const struct vadc_prescale_ratio *prescale,
const struct adc5_data *data,
@@ -330,6 +557,39 @@ static int qcom_vadc_scale_hw_calib_die_temp(
return 0;
}
+static int qcom_vadc7_scale_hw_calib_die_temp(
+ const struct vadc_prescale_ratio *prescale,
+ const struct adc5_data *data,
+ u16 adc_code, int *result_mdec)
+{
+
+ int voltage, vtemp0, temp, i = ARRAY_SIZE(adcmap7_die_temp) - 1;
+
+ voltage = qcom_vadc_scale_code_voltage_factor(adc_code,
+ prescale, data, 1);
+
+ if (adcmap7_die_temp[0].x > voltage) {
+ *result_mdec = DIE_TEMP_ADC7_SCALE_1;
+ return 0;
+ } else if (adcmap7_die_temp[i].x <= voltage) {
+ *result_mdec = DIE_TEMP_ADC7_MAX;
+ return 0;
+ }
+
+ for (i = 0; i < ARRAY_SIZE(adcmap7_die_temp); i++)
+ if (adcmap7_die_temp[i].x > voltage)
+ break;
+
+ vtemp0 = adcmap7_die_temp[i - 1].x;
+ voltage = voltage - vtemp0;
+ temp = div64_s64(voltage * DIE_TEMP_ADC7_SCALE_FACTOR,
+ adcmap7_die_temp[i - 1].y);
+ temp += DIE_TEMP_ADC7_SCALE_1 + (DIE_TEMP_ADC7_SCALE_2 * (i - 1));
+ *result_mdec = temp;
+
+ return 0;
+}
+
static int qcom_vadc_scale_hw_smb_temp(
const struct vadc_prescale_ratio *prescale,
const struct adc5_data *data,
diff --git a/drivers/iio/adc/qcom-vadc-common.h b/drivers/iio/adc/qcom-vadc-common.h
index e074902a..f10250b 100644
--- a/drivers/iio/adc/qcom-vadc-common.h
+++ b/drivers/iio/adc/qcom-vadc-common.h
@@ -49,6 +49,14 @@
#define ADC5_FULL_SCALE_CODE 0x70e4
#define ADC5_USR_DATA_CHECK 0x8000
+#define R_PU_100K 100000
+#define RATIO_MAX_ADC7 BIT(14)
+
+#define DIE_TEMP_ADC7_SCALE_1 -60000
+#define DIE_TEMP_ADC7_SCALE_2 20000
+#define DIE_TEMP_ADC7_SCALE_FACTOR 1000
+#define DIE_TEMP_ADC7_MAX 160000
+
/**
* struct vadc_map_pt - Map the graph representation for ADC channel
* @x: Represent the ADC digitized code.
@@ -110,8 +118,12 @@ struct vadc_prescale_ratio {
* lookup table. The hardware applies offset/slope to adc code.
* SCALE_HW_CALIB_XOTHERM: Returns XO thermistor voltage in millidegC using
* 100k pullup. The hardware applies offset/slope to adc code.
+ * SCALE_HW_CALIB_THERM_100K_PU_PM7: Returns temperature in millidegC using
+ * lookup table for PMIC7. The hardware applies offset/slope to adc code.
* SCALE_HW_CALIB_PMIC_THERM: Returns result in milli degree's Centigrade.
* The hardware applies offset/slope to adc code.
+ * SCALE_HW_CALIB_PMIC_THERM: Returns result in milli degree's Centigrade.
+ * The hardware applies offset/slope to adc code. This is for PMIC7.
* SCALE_HW_CALIB_PM5_CHG_TEMP: Returns result in millidegrees for PMIC5
* charger temperature.
* SCALE_HW_CALIB_PM5_SMB_TEMP: Returns result in millidegrees for PMIC5
@@ -126,7 +138,9 @@ enum vadc_scale_fn_type {
SCALE_HW_CALIB_DEFAULT,
SCALE_HW_CALIB_THERM_100K_PULLUP,
SCALE_HW_CALIB_XOTHERM,
+ SCALE_HW_CALIB_THERM_100K_PU_PM7,
SCALE_HW_CALIB_PMIC_THERM,
+ SCALE_HW_CALIB_PMIC_THERM_PM7,
SCALE_HW_CALIB_PM5_CHG_TEMP,
SCALE_HW_CALIB_PM5_SMB_TEMP,
SCALE_HW_CALIB_INVALID,
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related
* [PATCH V5 2/5] iio: adc: Add PMIC7 ADC bindings
From: Jishnu Prakash @ 2020-05-22 14:24 UTC (permalink / raw)
To: agross, bjorn.andersson, devicetree, linux-kernel, mka,
linus.walleij, Jonathan.Cameron, andy.shevchenko, amit.kucheria,
smohanad, kgunda, aghayal, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, Rob Herring,
Mark Rutland, Jishnu Prakash, Amit Kucheria, linux-iio,
linux-arm-msm
Cc: linux-arm-msm-owner
In-Reply-To: <1590157452-27179-1-git-send-email-jprakash@codeaurora.org>
Add documentation for PMIC7 ADC peripheral.
For the PMIC7-type PMICs, ADC peripheral is present in HW for the
following PMICs: PMK8350, PM8350, PM8350b, PMR735a and PMR735b.
Of these, only the ADC peripheral on PMK8350 is exposed directly to SW.
If SW needs to communicate with ADCs on other PMICs, it specifies the
PMIC to PMK8350 through the newly added SID register and communication
between PMK8350 ADC and other PMIC ADCs is carried out through
PBS(Programmable Boot Sequence) at the firmware level.
In addition, add definitions for ADC channels and virtual channel
definitions (combination of ADC channel number and PMIC SID number)
per PMIC, to be used by ADC clients for PMIC7.
Signed-off-by: Jishnu Prakash <jprakash@codeaurora.org>
Reviewed-by: Amit Kucheria <amit.kucheria@linaro.org>
Reviewed-by: Rob Herring <robh@kernel.org>
---
.../bindings/iio/adc/qcom,spmi-vadc.yaml | 38 ++++++++--
include/dt-bindings/iio/qcom,spmi-adc7-pm8350.h | 67 ++++++++++++++++
include/dt-bindings/iio/qcom,spmi-adc7-pm8350b.h | 88 ++++++++++++++++++++++
include/dt-bindings/iio/qcom,spmi-adc7-pmk8350.h | 46 +++++++++++
include/dt-bindings/iio/qcom,spmi-adc7-pmr735a.h | 28 +++++++
include/dt-bindings/iio/qcom,spmi-adc7-pmr735b.h | 28 +++++++
include/dt-bindings/iio/qcom,spmi-vadc.h | 78 ++++++++++++++++++-
7 files changed, 366 insertions(+), 7 deletions(-)
create mode 100644 include/dt-bindings/iio/qcom,spmi-adc7-pm8350.h
create mode 100644 include/dt-bindings/iio/qcom,spmi-adc7-pm8350b.h
create mode 100644 include/dt-bindings/iio/qcom,spmi-adc7-pmk8350.h
create mode 100644 include/dt-bindings/iio/qcom,spmi-adc7-pmr735a.h
create mode 100644 include/dt-bindings/iio/qcom,spmi-adc7-pmr735b.h
diff --git a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.yaml b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.yaml
index de8d243..e6263b6 100644
--- a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.yaml
@@ -13,7 +13,7 @@ maintainers:
description: |
SPMI PMIC voltage ADC (VADC) provides interface to clients to read
voltage. The VADC is a 15-bit sigma-delta ADC.
- SPMI PMIC5 voltage ADC (ADC) provides interface to clients to read
+ SPMI PMIC5/PMIC7 voltage ADC (ADC) provides interface to clients to read
voltage. The VADC is a 16-bit sigma-delta ADC.
properties:
@@ -28,6 +28,7 @@ properties:
- qcom,spmi-vadc
- qcom,spmi-adc5
- qcom,spmi-adc-rev2
+ - qcom,spmi-adc7
reg:
description: VADC base address in the SPMI PMIC register map
@@ -70,6 +71,8 @@ patternProperties:
description: |
ADC channel number.
See include/dt-bindings/iio/qcom,spmi-vadc.h
+ For PMIC7 ADC, the channel numbers are specified separately per PMIC
+ in the PMIC-specific files in include/dt-bindings/iio/.
label:
$ref: /schemas/types.yaml#/definitions/string
@@ -113,11 +116,11 @@ patternProperties:
channel calibration. If property is not found, channel will be
calibrated with 0.625V and 1.25V reference channels, also
known as absolute calibration.
- - For compatible property "qcom,spmi-adc5" and "qcom,spmi-adc-rev2",
- if this property is specified VADC will use the VDD reference (1.875V)
- and GND for channel calibration. If property is not found, channel
- will be calibrated with 0V and 1.25V reference channels, also known
- as absolute calibration.
+ - For compatible property "qcom,spmi-adc5", "qcom,spmi-adc7" and
+ "qcom,spmi-adc-rev2", if this property is specified VADC will use
+ the VDD reference (1.875V) and GND for channel calibration. If
+ property is not found, channel will be calibrated with 0V and 1.25V
+ reference channels, also known as absolute calibration.
type: boolean
qcom,hw-settle-time:
@@ -208,6 +211,29 @@ allOf:
enum: [ 1, 2, 4, 8, 16 ]
default: 1
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: qcom,spmi-adc7
+
+ then:
+ patternProperties:
+ "^.*@[0-9a-f]+$":
+ properties:
+ qcom,decimation:
+ enum: [ 85, 340, 1360 ]
+ default: 1360
+
+ qcom,hw-settle-time:
+ enum: [ 15, 100, 200, 300, 400, 500, 600, 700, 1000, 2000, 4000,
+ 8000, 16000, 32000, 64000, 128000 ]
+ default: 15
+
+ qcom,avg-samples:
+ enum: [ 1, 2, 4, 8, 16 ]
+ default: 1
+
examples:
- |
spmi_bus {
diff --git a/include/dt-bindings/iio/qcom,spmi-adc7-pm8350.h b/include/dt-bindings/iio/qcom,spmi-adc7-pm8350.h
new file mode 100644
index 0000000..9426f27
--- /dev/null
+++ b/include/dt-bindings/iio/qcom,spmi-adc7-pm8350.h
@@ -0,0 +1,67 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2020, The Linux Foundation. All rights reserved.
+ */
+
+#ifndef _DT_BINDINGS_QCOM_SPMI_VADC_PM8350_H
+#define _DT_BINDINGS_QCOM_SPMI_VADC_PM8350_H
+
+#ifndef PM8350_SID
+#define PM8350_SID 1
+#endif
+
+/* ADC channels for PM8350_ADC for PMIC7 */
+#define PM8350_ADC7_REF_GND (PM8350_SID << 8 | 0x0)
+#define PM8350_ADC7_1P25VREF (PM8350_SID << 8 | 0x01)
+#define PM8350_ADC7_VREF_VADC (PM8350_SID << 8 | 0x02)
+#define PM8350_ADC7_DIE_TEMP (PM8350_SID << 8 | 0x03)
+
+#define PM8350_ADC7_AMUX_THM1 (PM8350_SID << 8 | 0x04)
+#define PM8350_ADC7_AMUX_THM2 (PM8350_SID << 8 | 0x05)
+#define PM8350_ADC7_AMUX_THM3 (PM8350_SID << 8 | 0x06)
+#define PM8350_ADC7_AMUX_THM4 (PM8350_SID << 8 | 0x07)
+#define PM8350_ADC7_AMUX_THM5 (PM8350_SID << 8 | 0x08)
+#define PM8350_ADC7_GPIO1 (PM8350_SID << 8 | 0x0a)
+#define PM8350_ADC7_GPIO2 (PM8350_SID << 8 | 0x0b)
+#define PM8350_ADC7_GPIO3 (PM8350_SID << 8 | 0x0c)
+#define PM8350_ADC7_GPIO4 (PM8350_SID << 8 | 0x0d)
+
+/* 30k pull-up1 */
+#define PM8350_ADC7_AMUX_THM1_30K_PU (PM8350_SID << 8 | 0x24)
+#define PM8350_ADC7_AMUX_THM2_30K_PU (PM8350_SID << 8 | 0x25)
+#define PM8350_ADC7_AMUX_THM3_30K_PU (PM8350_SID << 8 | 0x26)
+#define PM8350_ADC7_AMUX_THM4_30K_PU (PM8350_SID << 8 | 0x27)
+#define PM8350_ADC7_AMUX_THM5_30K_PU (PM8350_SID << 8 | 0x28)
+#define PM8350_ADC7_GPIO1_30K_PU (PM8350_SID << 8 | 0x2a)
+#define PM8350_ADC7_GPIO2_30K_PU (PM8350_SID << 8 | 0x2b)
+#define PM8350_ADC7_GPIO3_30K_PU (PM8350_SID << 8 | 0x2c)
+#define PM8350_ADC7_GPIO4_30K_PU (PM8350_SID << 8 | 0x2d)
+
+/* 100k pull-up2 */
+#define PM8350_ADC7_AMUX_THM1_100K_PU (PM8350_SID << 8 | 0x44)
+#define PM8350_ADC7_AMUX_THM2_100K_PU (PM8350_SID << 8 | 0x45)
+#define PM8350_ADC7_AMUX_THM3_100K_PU (PM8350_SID << 8 | 0x46)
+#define PM8350_ADC7_AMUX_THM4_100K_PU (PM8350_SID << 8 | 0x47)
+#define PM8350_ADC7_AMUX_THM5_100K_PU (PM8350_SID << 8 | 0x48)
+#define PM8350_ADC7_GPIO1_100K_PU (PM8350_SID << 8 | 0x4a)
+#define PM8350_ADC7_GPIO2_100K_PU (PM8350_SID << 8 | 0x4b)
+#define PM8350_ADC7_GPIO3_100K_PU (PM8350_SID << 8 | 0x4c)
+#define PM8350_ADC7_GPIO4_100K_PU (PM8350_SID << 8 | 0x4d)
+
+/* 400k pull-up3 */
+#define PM8350_ADC7_AMUX_THM1_400K_PU (PM8350_SID << 8 | 0x64)
+#define PM8350_ADC7_AMUX_THM2_400K_PU (PM8350_SID << 8 | 0x65)
+#define PM8350_ADC7_AMUX_THM3_400K_PU (PM8350_SID << 8 | 0x66)
+#define PM8350_ADC7_AMUX_THM4_400K_PU (PM8350_SID << 8 | 0x67)
+#define PM8350_ADC7_AMUX_THM5_400K_PU (PM8350_SID << 8 | 0x68)
+#define PM8350_ADC7_GPIO1_400K_PU (PM8350_SID << 8 | 0x6a)
+#define PM8350_ADC7_GPIO2_400K_PU (PM8350_SID << 8 | 0x6b)
+#define PM8350_ADC7_GPIO3_400K_PU (PM8350_SID << 8 | 0x6c)
+#define PM8350_ADC7_GPIO4_400K_PU (PM8350_SID << 8 | 0x6d)
+
+/* 1/3 Divider */
+#define PM8350_ADC7_GPIO4_DIV3 (PM8350_SID << 8 | 0x8d)
+
+#define PM8350_ADC7_VPH_PWR (PM8350_SID << 8 | 0x8e)
+
+#endif /* _DT_BINDINGS_QCOM_SPMI_VADC_PM8350_H */
diff --git a/include/dt-bindings/iio/qcom,spmi-adc7-pm8350b.h b/include/dt-bindings/iio/qcom,spmi-adc7-pm8350b.h
new file mode 100644
index 0000000..dc2497c
--- /dev/null
+++ b/include/dt-bindings/iio/qcom,spmi-adc7-pm8350b.h
@@ -0,0 +1,88 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2020 The Linux Foundation. All rights reserved.
+ */
+
+#ifndef _DT_BINDINGS_QCOM_SPMI_VADC_PM8350B_H
+#define _DT_BINDINGS_QCOM_SPMI_VADC_PM8350B_H
+
+#ifndef PM8350B_SID
+#define PM8350B_SID 3
+#endif
+
+/* ADC channels for PM8350B_ADC for PMIC7 */
+#define PM8350B_ADC7_REF_GND (PM8350B_SID << 8 | 0x0)
+#define PM8350B_ADC7_1P25VREF (PM8350B_SID << 8 | 0x01)
+#define PM8350B_ADC7_VREF_VADC (PM8350B_SID << 8 | 0x02)
+#define PM8350B_ADC7_DIE_TEMP (PM8350B_SID << 8 | 0x03)
+
+#define PM8350B_ADC7_AMUX_THM1 (PM8350B_SID << 8 | 0x04)
+#define PM8350B_ADC7_AMUX_THM2 (PM8350B_SID << 8 | 0x05)
+#define PM8350B_ADC7_AMUX_THM3 (PM8350B_SID << 8 | 0x06)
+#define PM8350B_ADC7_AMUX_THM4 (PM8350B_SID << 8 | 0x07)
+#define PM8350B_ADC7_AMUX_THM5 (PM8350B_SID << 8 | 0x08)
+#define PM8350B_ADC7_AMUX_THM6 (PM8350B_SID << 8 | 0x09)
+#define PM8350B_ADC7_GPIO1 (PM8350B_SID << 8 | 0x0a)
+#define PM8350B_ADC7_GPIO2 (PM8350B_SID << 8 | 0x0b)
+#define PM8350B_ADC7_GPIO3 (PM8350B_SID << 8 | 0x0c)
+#define PM8350B_ADC7_GPIO4 (PM8350B_SID << 8 | 0x0d)
+
+#define PM8350B_ADC7_CHG_TEMP (PM8350B_SID << 8 | 0x10)
+#define PM8350B_ADC7_USB_IN_V_16 (PM8350B_SID << 8 | 0x11)
+#define PM8350B_ADC7_VDC_16 (PM8350B_SID << 8 | 0x12)
+#define PM8350B_ADC7_CC1_ID (PM8350B_SID << 8 | 0x13)
+#define PM8350B_ADC7_VREF_BAT_THERM (PM8350B_SID << 8 | 0x15)
+#define PM8350B_ADC7_IIN_FB (PM8350B_SID << 8 | 0x17)
+
+/* 30k pull-up1 */
+#define PM8350B_ADC7_AMUX_THM1_30K_PU (PM8350B_SID << 8 | 0x24)
+#define PM8350B_ADC7_AMUX_THM2_30K_PU (PM8350B_SID << 8 | 0x25)
+#define PM8350B_ADC7_AMUX_THM3_30K_PU (PM8350B_SID << 8 | 0x26)
+#define PM8350B_ADC7_AMUX_THM4_30K_PU (PM8350B_SID << 8 | 0x27)
+#define PM8350B_ADC7_AMUX_THM5_30K_PU (PM8350B_SID << 8 | 0x28)
+#define PM8350B_ADC7_AMUX_THM6_30K_PU (PM8350B_SID << 8 | 0x29)
+#define PM8350B_ADC7_GPIO1_30K_PU (PM8350B_SID << 8 | 0x2a)
+#define PM8350B_ADC7_GPIO2_30K_PU (PM8350B_SID << 8 | 0x2b)
+#define PM8350B_ADC7_GPIO3_30K_PU (PM8350B_SID << 8 | 0x2c)
+#define PM8350B_ADC7_GPIO4_30K_PU (PM8350B_SID << 8 | 0x2d)
+#define PM8350B_ADC7_CC1_ID_30K_PU (PM8350B_SID << 8 | 0x33)
+
+/* 100k pull-up2 */
+#define PM8350B_ADC7_AMUX_THM1_100K_PU (PM8350B_SID << 8 | 0x44)
+#define PM8350B_ADC7_AMUX_THM2_100K_PU (PM8350B_SID << 8 | 0x45)
+#define PM8350B_ADC7_AMUX_THM3_100K_PU (PM8350B_SID << 8 | 0x46)
+#define PM8350B_ADC7_AMUX_THM4_100K_PU (PM8350B_SID << 8 | 0x47)
+#define PM8350B_ADC7_AMUX_THM5_100K_PU (PM8350B_SID << 8 | 0x48)
+#define PM8350B_ADC7_AMUX_THM6_100K_PU (PM8350B_SID << 8 | 0x49)
+#define PM8350B_ADC7_GPIO1_100K_PU (PM8350B_SID << 8 | 0x4a)
+#define PM8350B_ADC7_GPIO2_100K_PU (PM8350B_SID << 8 | 0x4b)
+#define PM8350B_ADC7_GPIO3_100K_PU (PM8350B_SID << 8 | 0x4c)
+#define PM8350B_ADC7_GPIO4_100K_PU (PM8350B_SID << 8 | 0x4d)
+#define PM8350B_ADC7_CC1_ID_100K_PU (PM8350B_SID << 8 | 0x53)
+
+/* 400k pull-up3 */
+#define PM8350B_ADC7_AMUX_THM1_400K_PU (PM8350B_SID << 8 | 0x64)
+#define PM8350B_ADC7_AMUX_THM2_400K_PU (PM8350B_SID << 8 | 0x65)
+#define PM8350B_ADC7_AMUX_THM3_400K_PU (PM8350B_SID << 8 | 0x66)
+#define PM8350B_ADC7_AMUX_THM4_400K_PU (PM8350B_SID << 8 | 0x67)
+#define PM8350B_ADC7_AMUX_THM5_400K_PU (PM8350B_SID << 8 | 0x68)
+#define PM8350B_ADC7_AMUX_THM6_400K_PU (PM8350B_SID << 8 | 0x69)
+#define PM8350B_ADC7_GPIO1_400K_PU (PM8350B_SID << 8 | 0x6a)
+#define PM8350B_ADC7_GPIO2_400K_PU (PM8350B_SID << 8 | 0x6b)
+#define PM8350B_ADC7_GPIO3_400K_PU (PM8350B_SID << 8 | 0x6c)
+#define PM8350B_ADC7_GPIO4_400K_PU (PM8350B_SID << 8 | 0x6d)
+#define PM8350B_ADC7_CC1_ID_400K_PU (PM8350B_SID << 8 | 0x73)
+
+/* 1/3 Divider */
+#define PM8350B_ADC7_GPIO1_DIV3 (PM8350B_SID << 8 | 0x8a)
+#define PM8350B_ADC7_GPIO2_DIV3 (PM8350B_SID << 8 | 0x8b)
+#define PM8350B_ADC7_GPIO3_DIV3 (PM8350B_SID << 8 | 0x8c)
+#define PM8350B_ADC7_GPIO4_DIV3 (PM8350B_SID << 8 | 0x8d)
+
+#define PM8350B_ADC7_VPH_PWR (PM8350B_SID << 8 | 0x8e)
+#define PM8350B_ADC7_VBAT_SNS (PM8350B_SID << 8 | 0x8f)
+
+#define PM8350B_ADC7_SBUx (PM8350B_SID << 8 | 0x94)
+#define PM8350B_ADC7_VBAT_2S_MID (PM8350B_SID << 8 | 0x96)
+
+#endif /* _DT_BINDINGS_QCOM_SPMI_VADC_PM8350B_H */
diff --git a/include/dt-bindings/iio/qcom,spmi-adc7-pmk8350.h b/include/dt-bindings/iio/qcom,spmi-adc7-pmk8350.h
new file mode 100644
index 0000000..6c29687
--- /dev/null
+++ b/include/dt-bindings/iio/qcom,spmi-adc7-pmk8350.h
@@ -0,0 +1,46 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2020 The Linux Foundation. All rights reserved.
+ */
+
+#ifndef _DT_BINDINGS_QCOM_SPMI_VADC_PMK8350_H
+#define _DT_BINDINGS_QCOM_SPMI_VADC_PMK8350_H
+
+#ifndef PMK8350_SID
+#define PMK8350_SID 0
+#endif
+
+/* ADC channels for PMK8350_ADC for PMIC7 */
+#define PMK8350_ADC7_REF_GND (PMK8350_SID << 8 | 0x0)
+#define PMK8350_ADC7_1P25VREF (PMK8350_SID << 8 | 0x01)
+#define PMK8350_ADC7_VREF_VADC (PMK8350_SID << 8 | 0x02)
+#define PMK8350_ADC7_DIE_TEMP (PMK8350_SID << 8 | 0x03)
+
+#define PMK8350_ADC7_AMUX_THM1 (PMK8350_SID << 8 | 0x04)
+#define PMK8350_ADC7_AMUX_THM2 (PMK8350_SID << 8 | 0x05)
+#define PMK8350_ADC7_AMUX_THM3 (PMK8350_SID << 8 | 0x06)
+#define PMK8350_ADC7_AMUX_THM4 (PMK8350_SID << 8 | 0x07)
+#define PMK8350_ADC7_AMUX_THM5 (PMK8350_SID << 8 | 0x08)
+
+/* 30k pull-up1 */
+#define PMK8350_ADC7_AMUX_THM1_30K_PU (PMK8350_SID << 8 | 0x24)
+#define PMK8350_ADC7_AMUX_THM2_30K_PU (PMK8350_SID << 8 | 0x25)
+#define PMK8350_ADC7_AMUX_THM3_30K_PU (PMK8350_SID << 8 | 0x26)
+#define PMK8350_ADC7_AMUX_THM4_30K_PU (PMK8350_SID << 8 | 0x27)
+#define PMK8350_ADC7_AMUX_THM5_30K_PU (PMK8350_SID << 8 | 0x28)
+
+/* 100k pull-up2 */
+#define PMK8350_ADC7_AMUX_THM1_100K_PU (PMK8350_SID << 8 | 0x44)
+#define PMK8350_ADC7_AMUX_THM2_100K_PU (PMK8350_SID << 8 | 0x45)
+#define PMK8350_ADC7_AMUX_THM3_100K_PU (PMK8350_SID << 8 | 0x46)
+#define PMK8350_ADC7_AMUX_THM4_100K_PU (PMK8350_SID << 8 | 0x47)
+#define PMK8350_ADC7_AMUX_THM5_100K_PU (PMK8350_SID << 8 | 0x48)
+
+/* 400k pull-up3 */
+#define PMK8350_ADC7_AMUX_THM1_400K_PU (PMK8350_SID << 8 | 0x64)
+#define PMK8350_ADC7_AMUX_THM2_400K_PU (PMK8350_SID << 8 | 0x65)
+#define PMK8350_ADC7_AMUX_THM3_400K_PU (PMK8350_SID << 8 | 0x66)
+#define PMK8350_ADC7_AMUX_THM4_400K_PU (PMK8350_SID << 8 | 0x67)
+#define PMK8350_ADC7_AMUX_THM5_400K_PU (PMK8350_SID << 8 | 0x68)
+
+#endif /* _DT_BINDINGS_QCOM_SPMI_VADC_PMK8350_H */
diff --git a/include/dt-bindings/iio/qcom,spmi-adc7-pmr735a.h b/include/dt-bindings/iio/qcom,spmi-adc7-pmr735a.h
new file mode 100644
index 0000000..d6df1b1
--- /dev/null
+++ b/include/dt-bindings/iio/qcom,spmi-adc7-pmr735a.h
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2020 The Linux Foundation. All rights reserved.
+ */
+
+#ifndef _DT_BINDINGS_QCOM_SPMI_VADC_PMR735A_H
+#define _DT_BINDINGS_QCOM_SPMI_VADC_PMR735A_H
+
+#ifndef PMR735A_SID
+#define PMR735A_SID 4
+#endif
+
+/* ADC channels for PMR735A_ADC for PMIC7 */
+#define PMR735A_ADC7_REF_GND (PMR735A_SID << 8 | 0x0)
+#define PMR735A_ADC7_1P25VREF (PMR735A_SID << 8 | 0x01)
+#define PMR735A_ADC7_VREF_VADC (PMR735A_SID << 8 | 0x02)
+#define PMR735A_ADC7_DIE_TEMP (PMR735A_SID << 8 | 0x03)
+
+#define PMR735A_ADC7_GPIO1 (PMR735A_SID << 8 | 0x0a)
+#define PMR735A_ADC7_GPIO2 (PMR735A_SID << 8 | 0x0b)
+#define PMR735A_ADC7_GPIO3 (PMR735A_SID << 8 | 0x0c)
+
+/* 100k pull-up2 */
+#define PMR735A_ADC7_GPIO1_100K_PU (PMR735A_SID << 8 | 0x4a)
+#define PMR735A_ADC7_GPIO2_100K_PU (PMR735A_SID << 8 | 0x4b)
+#define PMR735A_ADC7_GPIO3_100K_PU (PMR735A_SID << 8 | 0x4c)
+
+#endif /* _DT_BINDINGS_QCOM_SPMI_VADC_PMR735A_H */
diff --git a/include/dt-bindings/iio/qcom,spmi-adc7-pmr735b.h b/include/dt-bindings/iio/qcom,spmi-adc7-pmr735b.h
new file mode 100644
index 0000000..8da0e7d
--- /dev/null
+++ b/include/dt-bindings/iio/qcom,spmi-adc7-pmr735b.h
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2020 The Linux Foundation. All rights reserved.
+ */
+
+#ifndef _DT_BINDINGS_QCOM_SPMI_VADC_PMR735B_H
+#define _DT_BINDINGS_QCOM_SPMI_VADC_PMR735B_H
+
+#ifndef PMR735B_SID
+#define PMR735B_SID 5
+#endif
+
+/* ADC channels for PMR735B_ADC for PMIC7 */
+#define PMR735B_ADC7_REF_GND (PMR735B_SID << 8 | 0x0)
+#define PMR735B_ADC7_1P25VREF (PMR735B_SID << 8 | 0x01)
+#define PMR735B_ADC7_VREF_VADC (PMR735B_SID << 8 | 0x02)
+#define PMR735B_ADC7_DIE_TEMP (PMR735B_SID << 8 | 0x03)
+
+#define PMR735B_ADC7_GPIO1 (PMR735B_SID << 8 | 0x0a)
+#define PMR735B_ADC7_GPIO2 (PMR735B_SID << 8 | 0x0b)
+#define PMR735B_ADC7_GPIO3 (PMR735B_SID << 8 | 0x0c)
+
+/* 100k pull-up2 */
+#define PMR735B_ADC7_GPIO1_100K_PU (PMR735B_SID << 8 | 0x4a)
+#define PMR735B_ADC7_GPIO2_100K_PU (PMR735B_SID << 8 | 0x4b)
+#define PMR735B_ADC7_GPIO3_100K_PU (PMR735B_SID << 8 | 0x4c)
+
+#endif /* _DT_BINDINGS_QCOM_SPMI_VADC_PMR735B_H */
diff --git a/include/dt-bindings/iio/qcom,spmi-vadc.h b/include/dt-bindings/iio/qcom,spmi-vadc.h
index 61d556d..08adfe2 100644
--- a/include/dt-bindings/iio/qcom,spmi-vadc.h
+++ b/include/dt-bindings/iio/qcom,spmi-vadc.h
@@ -1,6 +1,6 @@
/* SPDX-License-Identifier: GPL-2.0 */
/*
- * Copyright (c) 2012-2014,2018 The Linux Foundation. All rights reserved.
+ * Copyright (c) 2012-2014,2018,2020 The Linux Foundation. All rights reserved.
*/
#ifndef _DT_BINDINGS_QCOM_SPMI_VADC_H
@@ -221,4 +221,80 @@
#define ADC5_MAX_CHANNEL 0xc0
+/* ADC channels for ADC for PMIC7 */
+
+#define ADC7_REF_GND 0x00
+#define ADC7_1P25VREF 0x01
+#define ADC7_VREF_VADC 0x02
+#define ADC7_DIE_TEMP 0x03
+
+#define ADC7_AMUX_THM1 0x04
+#define ADC7_AMUX_THM2 0x05
+#define ADC7_AMUX_THM3 0x06
+#define ADC7_AMUX_THM4 0x07
+#define ADC7_AMUX_THM5 0x08
+#define ADC7_AMUX_THM6 0x09
+#define ADC7_GPIO1 0x0a
+#define ADC7_GPIO2 0x0b
+#define ADC7_GPIO3 0x0c
+#define ADC7_GPIO4 0x0d
+
+#define ADC7_CHG_TEMP 0x10
+#define ADC7_USB_IN_V_16 0x11
+#define ADC7_VDC_16 0x12
+#define ADC7_CC1_ID 0x13
+#define ADC7_VREF_BAT_THERM 0x15
+#define ADC7_IIN_FB 0x17
+
+/* 30k pull-up1 */
+#define ADC7_AMUX_THM1_30K_PU 0x24
+#define ADC7_AMUX_THM2_30K_PU 0x25
+#define ADC7_AMUX_THM3_30K_PU 0x26
+#define ADC7_AMUX_THM4_30K_PU 0x27
+#define ADC7_AMUX_THM5_30K_PU 0x28
+#define ADC7_AMUX_THM6_30K_PU 0x29
+#define ADC7_GPIO1_30K_PU 0x2a
+#define ADC7_GPIO2_30K_PU 0x2b
+#define ADC7_GPIO3_30K_PU 0x2c
+#define ADC7_GPIO4_30K_PU 0x2d
+#define ADC7_CC1_ID_30K_PU 0x33
+
+/* 100k pull-up2 */
+#define ADC7_AMUX_THM1_100K_PU 0x44
+#define ADC7_AMUX_THM2_100K_PU 0x45
+#define ADC7_AMUX_THM3_100K_PU 0x46
+#define ADC7_AMUX_THM4_100K_PU 0x47
+#define ADC7_AMUX_THM5_100K_PU 0x48
+#define ADC7_AMUX_THM6_100K_PU 0x49
+#define ADC7_GPIO1_100K_PU 0x4a
+#define ADC7_GPIO2_100K_PU 0x4b
+#define ADC7_GPIO3_100K_PU 0x4c
+#define ADC7_GPIO4_100K_PU 0x4d
+#define ADC7_CC1_ID_100K_PU 0x53
+
+/* 400k pull-up3 */
+#define ADC7_AMUX_THM1_400K_PU 0x64
+#define ADC7_AMUX_THM2_400K_PU 0x65
+#define ADC7_AMUX_THM3_400K_PU 0x66
+#define ADC7_AMUX_THM4_400K_PU 0x67
+#define ADC7_AMUX_THM5_400K_PU 0x68
+#define ADC7_AMUX_THM6_400K_PU 0x69
+#define ADC7_GPIO1_400K_PU 0x6a
+#define ADC7_GPIO2_400K_PU 0x6b
+#define ADC7_GPIO3_400K_PU 0x6c
+#define ADC7_GPIO4_400K_PU 0x6d
+#define ADC7_CC1_ID_400K_PU 0x73
+
+/* 1/3 Divider */
+#define ADC7_GPIO1_DIV3 0x8a
+#define ADC7_GPIO2_DIV3 0x8b
+#define ADC7_GPIO3_DIV3 0x8c
+#define ADC7_GPIO4_DIV3 0x8d
+
+#define ADC7_VPH_PWR 0x8e
+#define ADC7_VBAT_SNS 0x8f
+
+#define ADC7_SBUx 0x94
+#define ADC7_VBAT_2S_MID 0x96
+
#endif /* _DT_BINDINGS_QCOM_SPMI_VADC_H */
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related
* [PATCH V5 1/5] iio: adc: Convert the QCOM SPMI ADC bindings to .yaml format
From: Jishnu Prakash @ 2020-05-22 14:24 UTC (permalink / raw)
To: agross, bjorn.andersson, devicetree, linux-kernel, mka,
linus.walleij, Jonathan.Cameron, andy.shevchenko, amit.kucheria,
smohanad, kgunda, aghayal, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, Rob Herring,
Mark Rutland, Amit Kucheria, Jishnu Prakash, linux-iio
Cc: linux-arm-msm, linux-arm-msm-owner
In-Reply-To: <1590157452-27179-1-git-send-email-jprakash@codeaurora.org>
Convert the adc bindings from .txt to .yaml format.
Signed-off-by: Jishnu Prakash <jprakash@codeaurora.org>
Reviewed-by: Amit Kucheria <amit.kucheria@linaro.org>
Reviewed-by: Rob Herring <robh@kernel.org>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
---
.../devicetree/bindings/iio/adc/qcom,spmi-vadc.txt | 173 --------------
.../bindings/iio/adc/qcom,spmi-vadc.yaml | 252 +++++++++++++++++++++
2 files changed, 252 insertions(+), 173 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt
create mode 100644 Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.yaml
diff --git a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt
deleted file mode 100644
index c878768..0000000
--- a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt
+++ /dev/null
@@ -1,173 +0,0 @@
-Qualcomm's SPMI PMIC ADC
-
-- SPMI PMIC voltage ADC (VADC) provides interface to clients to read
- voltage. The VADC is a 15-bit sigma-delta ADC.
-- SPMI PMIC5 voltage ADC (ADC) provides interface to clients to read
- voltage. The VADC is a 16-bit sigma-delta ADC.
-
-VADC node:
-
-- compatible:
- Usage: required
- Value type: <string>
- Definition: Should contain "qcom,spmi-vadc".
- Should contain "qcom,spmi-adc5" for PMIC5 ADC driver.
- Should contain "qcom,spmi-adc-rev2" for PMIC rev2 ADC driver.
- Should contain "qcom,pms405-adc" for PMS405 PMIC
-
-- reg:
- Usage: required
- Value type: <prop-encoded-array>
- Definition: VADC base address in the SPMI PMIC register map.
-
-- #address-cells:
- Usage: required
- Value type: <u32>
- Definition: Must be one. Child node 'reg' property should define ADC
- channel number.
-
-- #size-cells:
- Usage: required
- Value type: <u32>
- Definition: Must be zero.
-
-- #io-channel-cells:
- Usage: required
- Value type: <u32>
- Definition: Must be one. For details about IIO bindings see:
- Documentation/devicetree/bindings/iio/iio-bindings.txt
-
-- interrupts:
- Usage: optional
- Value type: <prop-encoded-array>
- Definition: End of conversion interrupt.
-
-Channel node properties:
-
-- reg:
- Usage: required
- Value type: <u32>
- Definition: ADC channel number.
- See include/dt-bindings/iio/qcom,spmi-vadc.h
-
-- label:
- Usage: required for "qcom,spmi-adc5" and "qcom,spmi-adc-rev2"
- Value type: <empty>
- Definition: ADC input of the platform as seen in the schematics.
- For thermistor inputs connected to generic AMUX or GPIO inputs
- these can vary across platform for the same pins. Hence select
- the platform schematics name for this channel.
-
-- qcom,decimation:
- Usage: optional
- Value type: <u32>
- Definition: This parameter is used to decrease ADC sampling rate.
- Quicker measurements can be made by reducing decimation ratio.
- - For compatible property "qcom,spmi-vadc", valid values are
- 512, 1024, 2048, 4096. If property is not found, default value
- of 512 will be used.
- - For compatible property "qcom,spmi-adc5", valid values are 250, 420
- and 840. If property is not found, default value of 840 is used.
- - For compatible property "qcom,spmi-adc-rev2", valid values are 256,
- 512 and 1024. If property is not present, default value is 1024.
-
-- qcom,pre-scaling:
- Usage: optional
- Value type: <u32 array>
- Definition: Used for scaling the channel input signal before the signal is
- fed to VADC. The configuration for this node is to know the
- pre-determined ratio and use it for post scaling. Select one from
- the following options.
- <1 1>, <1 3>, <1 4>, <1 6>, <1 20>, <1 8>, <10 81>, <1 10>
- If property is not found default value depending on chip will be used.
-
-- qcom,ratiometric:
- Usage: optional
- Value type: <empty>
- Definition: Channel calibration type.
- - For compatible property "qcom,spmi-vadc", if this property is
- specified VADC will use the VDD reference (1.8V) and GND for
- channel calibration. If property is not found, channel will be
- calibrated with 0.625V and 1.25V reference channels, also
- known as absolute calibration.
- - For compatible property "qcom,spmi-adc5" and "qcom,spmi-adc-rev2",
- if this property is specified VADC will use the VDD reference
- (1.875V) and GND for channel calibration. If property is not found,
- channel will be calibrated with 0V and 1.25V reference channels,
- also known as absolute calibration.
-
-- qcom,hw-settle-time:
- Usage: optional
- Value type: <u32>
- Definition: Time between AMUX getting configured and the ADC starting
- conversion. The 'hw_settle_time' is an index used from valid values
- and programmed in hardware to achieve the hardware settling delay.
- - For compatible property "qcom,spmi-vadc" and "qcom,spmi-adc-rev2",
- Delay = 100us * (hw_settle_time) for hw_settle_time < 11,
- and 2ms * (hw_settle_time - 10) otherwise.
- Valid values are: 0, 100, 200, 300, 400, 500, 600, 700, 800,
- 900 us and 1, 2, 4, 6, 8, 10 ms.
- If property is not found, channel will use 0us.
- - For compatible property "qcom,spmi-adc5", delay = 15us for
- value 0, 100us * (value) for values < 11,
- and 2ms * (value - 10) otherwise.
- Valid values are: 15, 100, 200, 300, 400, 500, 600, 700, 800,
- 900 us and 1, 2, 4, 6, 8, 10 ms
- Certain controller digital versions have valid values of
- 15, 100, 200, 300, 400, 500, 600, 700, 1, 2, 4, 8, 16, 32, 64, 128 ms
- If property is not found, channel will use 15us.
-
-- qcom,avg-samples:
- Usage: optional
- Value type: <u32>
- Definition: Number of samples to be used for measurement.
- Averaging provides the option to obtain a single measurement
- from the ADC that is an average of multiple samples. The value
- selected is 2^(value).
- - For compatible property "qcom,spmi-vadc", valid values
- are: 1, 2, 4, 8, 16, 32, 64, 128, 256, 512
- If property is not found, 1 sample will be used.
- - For compatible property "qcom,spmi-adc5" and "qcom,spmi-adc-rev2",
- valid values are: 1, 2, 4, 8, 16
- If property is not found, 1 sample will be used.
-
-NOTE:
-
-For compatible property "qcom,spmi-vadc" following channels, also known as
-reference point channels, are used for result calibration and their channel
-configuration nodes should be defined:
-VADC_REF_625MV and/or VADC_SPARE1(based on PMIC version) VADC_REF_1250MV,
-VADC_GND_REF and VADC_VDD_VADC.
-
-Example:
-
-#include <dt-bindings/iio/qcom,spmi-vadc.h>
-#include <linux/irq.h>
-/* ... */
-
- /* VADC node */
- pmic_vadc: vadc@3100 {
- compatible = "qcom,spmi-vadc";
- reg = <0x3100>;
- interrupts = <0x0 0x31 0x0 IRQ_TYPE_EDGE_RISING>;
- #address-cells = <1>;
- #size-cells = <0>;
- #io-channel-cells = <1>;
- io-channel-ranges;
-
- /* Channel node */
- adc-chan@VADC_LR_MUX10_USB_ID {
- reg = <VADC_LR_MUX10_USB_ID>;
- qcom,decimation = <512>;
- qcom,ratiometric;
- qcom,hw-settle-time = <200>;
- qcom,avg-samples = <1>;
- qcom,pre-scaling = <1 3>;
- };
- };
-
- /* IIO client node */
- usb {
- io-channels = <&pmic_vadc VADC_LR_MUX10_USB_ID>;
- io-channel-names = "vadc";
- };
diff --git a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.yaml b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.yaml
new file mode 100644
index 0000000..de8d243
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.yaml
@@ -0,0 +1,252 @@
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/qcom,spmi-vadc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm's SPMI PMIC ADC
+
+maintainers:
+ - Andy Gross <agross@kernel.org>
+ - Bjorn Andersson <bjorn.andersson@linaro.org>
+
+description: |
+ SPMI PMIC voltage ADC (VADC) provides interface to clients to read
+ voltage. The VADC is a 15-bit sigma-delta ADC.
+ SPMI PMIC5 voltage ADC (ADC) provides interface to clients to read
+ voltage. The VADC is a 16-bit sigma-delta ADC.
+
+properties:
+ compatible:
+ oneOf:
+ - items:
+ - const: qcom,pms405-adc
+ - const: qcom,spmi-adc-rev2
+
+ - items:
+ - enum:
+ - qcom,spmi-vadc
+ - qcom,spmi-adc5
+ - qcom,spmi-adc-rev2
+
+ reg:
+ description: VADC base address in the SPMI PMIC register map
+ maxItems: 1
+
+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 0
+
+ '#io-channel-cells':
+ const: 1
+
+ interrupts:
+ maxItems: 1
+ description:
+ End of conversion interrupt.
+
+required:
+ - compatible
+ - reg
+ - '#address-cells'
+ - '#size-cells'
+ - '#io-channel-cells'
+
+patternProperties:
+ "^.*@[0-9a-f]+$":
+ type: object
+ description: |
+ Represents the external channels which are connected to the ADC.
+ For compatible property "qcom,spmi-vadc" following channels, also known as
+ reference point channels, are used for result calibration and their channel
+ configuration nodes should be defined:
+ VADC_REF_625MV and/or VADC_SPARE1(based on PMIC version) VADC_REF_1250MV,
+ VADC_GND_REF and VADC_VDD_VADC.
+
+ properties:
+ reg:
+ description: |
+ ADC channel number.
+ See include/dt-bindings/iio/qcom,spmi-vadc.h
+
+ label:
+ $ref: /schemas/types.yaml#/definitions/string
+ description: |
+ ADC input of the platform as seen in the schematics.
+ For thermistor inputs connected to generic AMUX or GPIO inputs
+ these can vary across platform for the same pins. Hence select
+ the platform schematics name for this channel.
+
+ qcom,decimation:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: |
+ This parameter is used to decrease ADC sampling rate.
+ Quicker measurements can be made by reducing decimation ratio.
+
+ qcom,pre-scaling:
+ description: |
+ Used for scaling the channel input signal before the signal is
+ fed to VADC. The configuration for this node is to know the
+ pre-determined ratio and use it for post scaling. It is a pair of
+ integers, denoting the numerator and denominator of the fraction by which
+ input signal is multiplied. For example, <1 3> indicates the signal is scaled
+ down to 1/3 of its value before ADC measurement.
+ If property is not found default value depending on chip will be used.
+ allOf:
+ - $ref: /schemas/types.yaml#/definitions/uint32-array
+ oneOf:
+ - items:
+ - const: 1
+ - enum: [ 1, 3, 4, 6, 20, 8, 10 ]
+
+ - items:
+ - const: 10
+ - const: 81
+
+ qcom,ratiometric:
+ description: |
+ Channel calibration type.
+ - For compatible property "qcom,spmi-vadc", if this property is
+ specified VADC will use the VDD reference (1.8V) and GND for
+ channel calibration. If property is not found, channel will be
+ calibrated with 0.625V and 1.25V reference channels, also
+ known as absolute calibration.
+ - For compatible property "qcom,spmi-adc5" and "qcom,spmi-adc-rev2",
+ if this property is specified VADC will use the VDD reference (1.875V)
+ and GND for channel calibration. If property is not found, channel
+ will be calibrated with 0V and 1.25V reference channels, also known
+ as absolute calibration.
+ type: boolean
+
+ qcom,hw-settle-time:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: |
+ Time between AMUX getting configured and the ADC starting
+ conversion. The 'hw_settle_time' is an index used from valid values
+ and programmed in hardware to achieve the hardware settling delay.
+
+ qcom,avg-samples:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: |
+ Number of samples to be used for measurement.
+ Averaging provides the option to obtain a single measurement
+ from the ADC that is an average of multiple samples. The value
+ selected is 2^(value).
+
+ required:
+ - reg
+
+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: qcom,spmi-vadc
+
+ then:
+ patternProperties:
+ "^.*@[0-9a-f]+$":
+ properties:
+ qcom,decimation:
+ enum: [ 512, 1024, 2048, 4096 ]
+ default: 512
+
+ qcom,hw-settle-time:
+ enum: [ 0, 100, 200, 300, 400, 500, 600, 700, 800, 900, 1, 2,
+ 4, 6, 8, 10 ]
+ default: 0
+
+ qcom,avg-samples:
+ enum: [ 1, 2, 4, 8, 16, 32, 64, 128, 256, 512 ]
+ default: 1
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: qcom,spmi-adc-rev2
+
+ then:
+ patternProperties:
+ "^.*@[0-9a-f]+$":
+ properties:
+ qcom,decimation:
+ enum: [ 256, 512, 1024 ]
+ default: 1024
+
+ qcom,hw-settle-time:
+ enum: [ 0, 100, 200, 300, 400, 500, 600, 700, 800, 900, 1, 2,
+ 4, 6, 8, 10 ]
+ default: 0
+
+ qcom,avg-samples:
+ enum: [ 1, 2, 4, 8, 16 ]
+ default: 1
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: qcom,spmi-adc5
+
+ then:
+ patternProperties:
+ "^.*@[0-9a-f]+$":
+ properties:
+ qcom,decimation:
+ enum: [ 250, 420, 840 ]
+ default: 840
+
+ qcom,hw-settle-time:
+ enum: [ 15, 100, 200, 300, 400, 500, 600, 700, 800, 900, 1, 2,
+ 4, 6, 8, 10, 16, 32, 64, 128 ]
+ default: 15
+
+ qcom,avg-samples:
+ enum: [ 1, 2, 4, 8, 16 ]
+ default: 1
+
+examples:
+ - |
+ spmi_bus {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ /* VADC node */
+ pmic_vadc: adc@3100 {
+ compatible = "qcom,spmi-vadc";
+ reg = <0x3100>;
+ interrupts = <0x0 0x31 0x0 0x1>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ #io-channel-cells = <1>;
+ io-channel-ranges;
+
+ /* Channel node */
+ adc-chan@39 {
+ reg = <0x39>;
+ qcom,decimation = <512>;
+ qcom,ratiometric;
+ qcom,hw-settle-time = <200>;
+ qcom,avg-samples = <1>;
+ qcom,pre-scaling = <1 3>;
+ };
+
+ adc-chan@9 {
+ reg = <0x9>;
+ };
+
+ adc-chan@a {
+ reg = <0xa>;
+ };
+
+ adc-chan@e {
+ reg = <0xe>;
+ };
+
+ adc-chan@f {
+ reg = <0xf>;
+ };
+ };
+ };
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related
* [PATCH V5 0/5] iio: adc: Add support for QCOM SPMI PMIC7 ADC
From: Jishnu Prakash @ 2020-05-22 14:24 UTC (permalink / raw)
To: agross, bjorn.andersson, devicetree, linux-kernel, mka,
linus.walleij, Jonathan.Cameron, andy.shevchenko, amit.kucheria,
smohanad, kgunda, aghayal
Cc: linux-arm-msm, linux-arm-msm-owner, Jishnu Prakash
The following changes are made in V5:
Made some recommended minor changes in the third patch and
accordingly moved some return value check corrections From
fourth patch to third.
Cleaned up commit message of fifth patch.
Jishnu Prakash (5):
iio: adc: Convert the QCOM SPMI ADC bindings to .yaml format
iio: adc: Add PMIC7 ADC bindings
iio: adc: Add support for PMIC7 ADC
iio: adc: Update debug prints
iio: adc: Clean up ADC code common to PMIC5 and PMIC7
.../devicetree/bindings/iio/adc/qcom,spmi-vadc.txt | 173 -------------
.../bindings/iio/adc/qcom,spmi-vadc.yaml | 278 +++++++++++++++++++++
drivers/iio/adc/qcom-spmi-adc5.c | 266 ++++++++++++++++++--
drivers/iio/adc/qcom-vadc-common.c | 260 +++++++++++++++++++
drivers/iio/adc/qcom-vadc-common.h | 15 ++
include/dt-bindings/iio/qcom,spmi-adc7-pm8350.h | 67 +++++
include/dt-bindings/iio/qcom,spmi-adc7-pm8350b.h | 88 +++++++
include/dt-bindings/iio/qcom,spmi-adc7-pmk8350.h | 46 ++++
include/dt-bindings/iio/qcom,spmi-adc7-pmr735a.h | 28 +++
include/dt-bindings/iio/qcom,spmi-adc7-pmr735b.h | 28 +++
include/dt-bindings/iio/qcom,spmi-vadc.h | 78 +++++-
11 files changed, 1131 insertions(+), 196 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.txt
create mode 100644 Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.yaml
create mode 100644 include/dt-bindings/iio/qcom,spmi-adc7-pm8350.h
create mode 100644 include/dt-bindings/iio/qcom,spmi-adc7-pm8350b.h
create mode 100644 include/dt-bindings/iio/qcom,spmi-adc7-pmk8350.h
create mode 100644 include/dt-bindings/iio/qcom,spmi-adc7-pmr735a.h
create mode 100644 include/dt-bindings/iio/qcom,spmi-adc7-pmr735b.h
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH v2 2/2] hwmon: Add Baikal-T1 PVT sensor driver
From: Guenter Roeck @ 2020-05-22 14:19 UTC (permalink / raw)
To: Serge Semin
Cc: Jean Delvare, Jonathan Corbet, Serge Semin, Maxim Kaurkin,
Alexey Malahov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle,
Arnd Bergmann, Rob Herring, linux-mips, devicetree,
Jonathan Cameron, Linus Walleij, Nuno Sá, Jisheng Zhang,
Paul Cercueil, Dan Robertson, Eric Tremblay, Beniamin Bia,
Jean Delvare, Dr. David Alan Gilbert, linux-hwmon, linux-doc,
linux-kernel
In-Reply-To: <20200510103211.27905-3-Sergey.Semin@baikalelectronics.ru>
On Sun, May 10, 2020 at 01:32:11PM +0300, Serge Semin wrote:
> Baikal-T1 SoC provides an embedded process, voltage and temperature
> sensor to monitor an internal SoC environment (chip temperature, supply
> voltage and process monitor) and on time detect critical situations,
> which may cause the system instability and even damages. The IP-block
> is based on the Analog Bits PVT sensor, but is equipped with a
> dedicated control wrapper, which provides a MMIO registers-based access
> to the sensor core functionality (APB3-bus based) and exposes an
> additional functions like thresholds/data ready interrupts, its status
> and masks, measurements timeout. All of these is used to create a hwmon
> driver being added to the kernel by this commit.
>
> The driver implements support for the hardware monitoring capabilities
> of Baikal-T1 process, voltage and temperature sensors. PVT IP-core
> consists of one temperature and four voltage sensors, each of which is
> implemented as a dedicated hwmon channel config.
>
> The driver can optionally provide the hwmon alarms for each sensor the
> PVT controller supports. The alarms functionality is made compile-time
> configurable due to the hardware interface implementation peculiarity,
> which is connected with an ability to convert data from only one sensor
> at a time. Additional limitation is that the controller performs the
> thresholds checking synchronously with the data conversion procedure.
> Due to these limitations in order to have the hwmon alarms
> automatically detected the driver code must switch from one sensor to
> another, read converted data and manually check the threshold status
> bits. Depending on the measurements timeout settings this design may
> cause additional burden on the system performance. By default if the
> alarms kernel config is disabled the data conversion is performed by
> the driver on demand when read operation is requested via corresponding
> _input-file.
>
> Co-developed-by: Maxim Kaurkin <maxim.kaurkin@baikalelectronics.ru>
> Signed-off-by: Maxim Kaurkin <maxim.kaurkin@baikalelectronics.ru>
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Paul Burton <paulburton@kernel.org>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: linux-mips@vger.kernel.org
> Cc: devicetree@vger.kernel.org
> ---
>
> Changelog v2:
> - Discard handwritten IO-access wrappers. Use normal readl/writel instead.
> - Use generic FIELD_{GET,PREP} macros instead of handwritten ones.
> - Since the driver depends on the OF config we can remove of_match_ptr()
> macro utilization.
> - Don't print error-message if no platform IRQ found. Just return an error.
> - Remove probe-status info string printout.
> ---
> Documentation/hwmon/bt1-pvt.rst | 113 +++
Needs to be added to index.rst.
> drivers/hwmon/Kconfig | 29 +
> drivers/hwmon/Makefile | 1 +
> drivers/hwmon/bt1-pvt.c | 1154 +++++++++++++++++++++++++++++++
> drivers/hwmon/bt1-pvt.h | 237 +++++++
> 5 files changed, 1534 insertions(+)
> create mode 100644 Documentation/hwmon/bt1-pvt.rst
> create mode 100644 drivers/hwmon/bt1-pvt.c
> create mode 100644 drivers/hwmon/bt1-pvt.h
>
> diff --git a/Documentation/hwmon/bt1-pvt.rst b/Documentation/hwmon/bt1-pvt.rst
> new file mode 100644
> index 000000000000..7ad8574bce98
> --- /dev/null
> +++ b/Documentation/hwmon/bt1-pvt.rst
> @@ -0,0 +1,113 @@
> +.. SPDX-License-Identifier: GPL-2.0-only
> +
> +Kernel driver bt1-pvt
> +=====================
> +
> +Supported chips:
> +
> + * Baikal-T1 PVT sensor (in SoC)
> +
> + Prefix: 'bt1-pvt'
> +
> + Addresses scanned: -
> +
> + Datasheet: Provided by BAIKAL ELECTRONICS upon request and under NDA
> +
> +Authors:
> + Maxim Kaurkin <maxim.kaurkin@baikalelectronics.ru>
> + Serge Semin <Sergey.Semin@baikalelectronics.ru>
> +
> +Description
> +-----------
> +
> +This driver implements support for the hardware monitoring capabilities of the
> +embedded into Baikal-T1 process, voltage and temperature sensors. PVT IP-core
> +consists of one temperature and four voltage sensors, which can be used to
> +monitor the chip internal environment like heating, supply voltage and
> +transistors performance. The driver can optionally provide the hwmon alarms
> +for each sensor the PVT controller supports. The alarms functionality is made
> +compile-time configurable due to the hardware interface implementation
> +peculiarity, which is connected with an ability to convert data from only one
> +sensor at a time. Additional limitation is that the controller performs the
> +thresholds checking synchronously with the data conversion procedure. Due to
> +these in order to have the hwmon alarms automatically detected the driver code
> +must switch from one sensor to another, read converted data and manually check
> +the threshold status bits. Depending on the measurements timeout settings
> +(update_interval sysfs node value) this design may cause additional burden on
> +the system performance. So in case if alarms are unnecessary in your system
> +design it's recommended to have them disabled to prevent the PVT IRQs being
> +periodically raised to get the data cache/alarms status up to date. By default
> +in alarm-less configuration the data conversion is performed by the driver
> +on demand when read operation is requested via corresponding _input-file.
> +
> +Temperature Monitoring
> +----------------------
> +
> +Temperature is measured with 10-bit resolution and reported in millidegree
> +Celsius. The driver performs all the scaling by itself therefore reports true
> +temperatures that don't need any user-space adjustments. While the data
> +translation formulae isn't linear, which gives us non-linear discreteness,
> +it's close to one, but giving a bit better accuracy for higher temperatures.
> +The temperature input is mapped as follows (the last column indicates the input
> +ranges)::
> +
> + temp1: CPU embedded diode -48.38C - +147.438C
> +
> +In case if the alarms kernel config is enabled in the driver the temperature input
> +has associated min and max limits which trigger an alarm when crossed.
> +
> +Voltage Monitoring
> +------------------
> +
> +The voltage inputs are also sampled with 10-bit resolution and reported in
> +millivolts. But in this case the data translation formulae is linear, which
> +provides a constant measurements discreteness. The data scaling is also
> +performed by the driver, so returning true millivolts. The voltage inputs are
> +mapped as follows (the last column indicates the input ranges)::
> +
> + in0: VDD (processor core) 0.62V - 1.168V
> + in1: Low-Vt (low voltage threshold) 0.62V - 1.168V
> + in2: High-Vt (high voltage threshold) 0.62V - 1.168V
> + in3: Standard-Vt (standard voltage threshold) 0.62V - 1.168V
> +
> +In case if the alarms configis enabled in the driver the voltage inputs
> +have associated min and max limits which trigger an alarm when crossed.
> +
> +Sysfs Attributes
> +----------------
> +
> +Following is a list of all sysfs attributes that the driver provides, their
> +permissions and a short description:
> +
> +=============================== ======= =======================================
> +Name Perm Description
> +=============================== ======= =======================================
> +update_interval RW Measurements update interval per
> + sensor.
> +temp1_type RO Sensor type (always 1 as CPU embedded
> + diode).
> +temp1_label RO MIPS P5600 Temperature sensor.
> +temp1_input RO Measured temperature in millidegree
> + Celsius.
> +temp1_min RW Low limit for temp input.
> +temp1_max RW High limit for temp input.
> +temp1_min_alarm RO Temperature input alarm. Returns 1 if
> + temperature input went below min limit,
> + 0 otherwise.
> +temp1_max_alarm RO Temperature input alarm. Returns 1 if
> + temperature input went above max limit,
> + 0 otherwise.
> +temp1_trim RW Temperature sensor trimming factor
> + (specific to the SoC application).
This needs a much better documentation as well as explanation why it needs to be
a sysfs attribute instead of a devicetree node (ie why it needs to be
runtime-controlled).
> +in[0-3]_label RO MIPS P5600 Voltage sensor (either core
> + or low/high/standard thresholds).
> +in[0-3]_input RO Measured voltage in millivolts.
> +in[0-3]_min RW Low limit for voltage input.
> +in[0-3]_max RW High limit for voltage input.
> +in[0-3]_min_alarm RO Voltage input alarm. Returns 1 if
> + voltage input went below min limit,
> + 0 otherwise.
> +in[0-3]_max_alarm RO Voltage input alarm. Returns 1 if
> + voltage input went above max limit,
> + 0 otherwise.
> +=============================== ======= =======================================
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 4c62f900bf7e..b80fc7d398c3 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -404,6 +404,35 @@ config SENSORS_ATXP1
> This driver can also be built as a module. If so, the module
> will be called atxp1.
>
> +config SENSORS_BT1_PVT
> + tristate "Baikal-T1 Process, Voltage, Temperature sensor driver"
> + depends on (MIPS_BAIKAL_T1 && OF) || COMPILE_TEST
> + help
> + If you say yes here you get support for Baikal-T1 PVT sensor
> + embedded into the SoC.
> +
> + This driver can also be built as a module. If so, the module will be
> + called bt1-pvt.
> +
> +if SENSORS_BT1_PVT
> +
> +config SENSORS_BT1_PVT_ALARMS
> + bool "Enable Baikal-T1 PVT sensor alarms"
> + default n
"default n" is unnecessary. Also, instead of "if SENSORS_BT1_PVT", please
use "depends on SENSORS_BT1_PVT".
> + help
> + Baikal-T1 PVT IP-block provides threshold registers for each
> + supported sensor. But the corresponding interrupts might be
> + generated by the thresholds comparator only in synchronization with
> + a data conversion. Additionally there is only one sensor data can
> + be converted at a time. All of these makes the interface impossible
> + to be used for the hwmon alarms implementation without periodic
> + switch between the PVT sensors. By default the data conversion is
> + performed on demand from the user-space. If this config is enabled
> + the data conversion will be periodically performed and the data will be
> + saved in the internal driver cache.
> +
> +endif
> +
> config SENSORS_DRIVETEMP
> tristate "Hard disk drives with temperature sensors"
> depends on SCSI && ATA
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index b0b9c8e57176..408b86a5e71d 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -53,6 +53,7 @@ obj-$(CONFIG_SENSORS_ASC7621) += asc7621.o
> obj-$(CONFIG_SENSORS_ASPEED) += aspeed-pwm-tacho.o
> obj-$(CONFIG_SENSORS_ATXP1) += atxp1.o
> obj-$(CONFIG_SENSORS_AXI_FAN_CONTROL) += axi-fan-control.o
> +obj-$(CONFIG_SENSORS_BT1_PVT) += bt1-pvt.o
> obj-$(CONFIG_SENSORS_CORETEMP) += coretemp.o
> obj-$(CONFIG_SENSORS_DA9052_ADC)+= da9052-hwmon.o
> obj-$(CONFIG_SENSORS_DA9055)+= da9055-hwmon.o
> diff --git a/drivers/hwmon/bt1-pvt.c b/drivers/hwmon/bt1-pvt.c
> new file mode 100644
> index 000000000000..1a29fd01aa2b
> --- /dev/null
> +++ b/drivers/hwmon/bt1-pvt.c
> @@ -0,0 +1,1154 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2020 BAIKAL ELECTRONICS, JSC
> + *
> + * Authors:
> + * Maxim Kaurkin <maxim.kaurkin@baikalelectronics.ru>
> + * Serge Semin <Sergey.Semin@baikalelectronics.ru>
> + *
> + * Baikal-T1 Process, Voltage, Temperature sensor driver
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/types.h>
> +#include <linux/bitfield.h>
> +#include <linux/device.h>
> +#include <linux/platform_device.h>
> +#include <linux/interrupt.h>
> +#include <linux/bitops.h>
> +#include <linux/mutex.h>
> +#include <linux/seqlock.h>
> +#include <linux/completion.h>
> +#include <linux/limits.h>
> +#include <linux/ktime.h>
> +#include <linux/clk.h>
> +#include <linux/sysfs.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
Alphabetic order, please.
> +
> +#include "bt1-pvt.h"
> +
> +/*
> + * For the sake of the code simplification we created the sensors info table
> + * with the sensor names, activation modes, threshold registers base address
> + * and the thresholds bit fields.
> + */
> +static const struct pvt_sensor_info pvt_info[] = {
> + PVT_SENSOR_INFO("MIPS P5600 Temperature", "temp1", TEMP, TTHRES),
> + PVT_SENSOR_INFO("MIPS P5600 Voltage", "in0", VOLT, VTHRES),
> + PVT_SENSOR_INFO("MIPS P5600 Low-Vt", "in1", LVT, LTHRES),
> + PVT_SENSOR_INFO("MIPS P5600 High-Vt", "in2", HVT, HTHRES),
> + PVT_SENSOR_INFO("MIPS P5600 Standard-Vt", "in3", SVT, STHRES)
> +};
> +
> +/*
> + * The original translation formulae of the temperature (in degrees of Celsius)
> + * to PVT data and vice-versa are following:
> + * N = 1.8322e-8*(T^4) + 2.343e-5*(T^3) + 8.7018e-3*(T^2) + 3.9269*(T^1) +
> + * 1.7204e2,
> + * T = -1.6743e-11*(N^4) + 8.1542e-8*(N^3) + -1.8201e-4*(N^2) +
> + * 3.1020e-1*(N^1) - 4.838e1,
> + * where T = [-48.380, 147.438]C and N = [0, 1023].
> + * They must be accordingly altered to be suitable for the integer arithmetics.
> + * The technique is called 'factor redistribution', which just makes sure the
> + * multiplications and divisions are made so to have a result of the operations
> + * within the integer numbers limit. In addition we need to translate the
> + * formulae to accept millidegrees of Celsius. Here what they look like after
> + * the alterations:
> + * N = (18322e-20*(T^4) + 2343e-13*(T^3) + 87018e-9*(T^2) + 39269e-3*T +
> + * 17204e2) / 1e4,
> + * T = -16743e-12*(D^4) + 81542e-9*(D^3) - 182010e-6*(D^2) + 310200e-3*D -
> + * 48380,
> + * where T = [-48380, 147438] mC and N = [0, 1023].
> + */
> +const static struct pvt_poly poly_temp_to_N = {
> + .total_divider = 10000,
> + .terms = {
> + {4, 18322, 10000, 10000},
> + {3, 2343, 10000, 10},
> + {2, 87018, 10000, 10},
> + {1, 39269, 1000, 1},
> + {0, 1720400, 1, 1}
> + }
> +};
> +
> +const static struct pvt_poly poly_N_to_temp = {
> + .total_divider = 1,
> + .terms = {
> + {4, -16743, 1000, 1},
> + {3, 81542, 1000, 1},
> + {2, -182010, 1000, 1},
> + {1, 310200, 1000, 1},
> + {0, -48380, 1, 1}
> + }
> +};
> +
> +/*
> + * Similar alterations are performed for the voltage conversion equations.
> + * The original formulae are:
> + * N = 1.8658e3*V - 1.1572e3,
> + * V = (N + 1.1572e3) / 1.8658e3,
> + * where V = [0.620, 1.168] V and N = [0, 1023].
> + * After the optimization they looks as follows:
> + * N = (18658e-3*V - 11572) / 10,
> + * V = N * 10^5 / 18658 + 11572 * 10^4 / 18658.
> + */
> +const static struct pvt_poly poly_volt_to_N = {
> + .total_divider = 10,
> + .terms = {
> + {1, 18658, 1000, 1},
> + {0, -11572, 1, 1}
> + }
> +};
> +
> +const static struct pvt_poly poly_N_to_volt = {
> + .total_divider = 10,
> + .terms = {
> + {1, 100000, 18658, 1},
> + {0, 115720000, 1, 18658}
> + }
> +};
> +
> +/*
> + * Here is the polynomial calculation function, which performs the
> + * redistributed terms calculations. It's pretty straightforward. We walk
> + * over each degree term up to the free one, and perform the redistributed
> + * multiplication of the term coefficient, its divider (as for the rationale
> + * fraction representation), data power and the rational fraction divider
> + * leftover. Then all of this is collected in a total sum variable, which
> + * value is normalized by the total divider before being returned.
> + */
> +static long pvt_calc_poly(const struct pvt_poly *poly, long data)
> +{
> + const struct pvt_poly_term *term = poly->terms;
> + long tmp, ret = 0;
> + int deg;
> +
> + do {
> + tmp = term->coef;
> + for (deg = 0; deg < term->deg; ++deg)
> + tmp = mult_frac(tmp, data, term->divider);
> + ret += tmp / term->divider_leftover;
> + } while ((term++)->deg);
> +
> + return ret / poly->total_divider;
> +}
> +
> +static inline u32 pvt_update(void __iomem *reg, u32 mask, u32 data)
> +{
> + u32 old;
> +
> + old = readl_relaxed(reg);
> + writel((old & ~mask) | (data & mask), reg);
> +
> + return old & mask;
> +}
> +
> +/*
> + * Baikal-T1 PVT mode can be updated only when the controller is disabled.
> + * So first we disable it, then set the new mode together with the controller
> + * getting back enabled. The same concerns the temperature trim and
> + * measurements timeout. If it is necessary the interface mutex is supposed
> + * to be locked at the time the operations are performed.
> + */
> +static inline void pvt_set_mode(struct pvt_hwmon *pvt, u32 mode)
> +{
> + u32 old;
> +
> + mode = FIELD_PREP(PVT_CTRL_MODE_MASK, mode);
> +
> + old = pvt_update(pvt->regs + PVT_CTRL, PVT_CTRL_EN, 0);
> + pvt_update(pvt->regs + PVT_CTRL, PVT_CTRL_MODE_MASK | PVT_CTRL_EN,
> + mode | old);
> +}
> +
> +static inline void pvt_set_trim(struct pvt_hwmon *pvt, u32 trim)
> +{
> + u32 old;
> +
> + trim = FIELD_PREP(PVT_CTRL_TRIM_MASK, trim);
> +
> + old = pvt_update(pvt->regs + PVT_CTRL, PVT_CTRL_EN, 0);
> + pvt_update(pvt->regs + PVT_CTRL, PVT_CTRL_TRIM_MASK | PVT_CTRL_EN,
> + trim | old);
> +}
> +
> +static inline void pvt_set_tout(struct pvt_hwmon *pvt, u32 tout)
> +{
> + u32 old;
> +
> + old = pvt_update(pvt->regs + PVT_CTRL, PVT_CTRL_EN, 0);
> + writel(tout, pvt->regs + PVT_TTIMEOUT);
> + pvt_update(pvt->regs + PVT_CTRL, PVT_CTRL_EN, old);
> +}
> +
> +/*
> + * This driver can optionally provide the hwmon alarms for each sensor the PVT
> + * controller supports. The alarms functionality is made compile-time
> + * configurable due to the hardware interface implementation peculiarity
> + * described further in this comment. So in case if alarms are unnecessary in
> + * your system design it's recommended to have them disabled to prevent the PVT
> + * IRQs being periodically raised to get the data cache/alarms status up to
> + * date.
> + *
> + * Baikal-T1 PVT embedded controller is based on the Analog Bits PVT sensor,
> + * but is equipped with a dedicated control wrapper. It exposes the PVT
> + * sub-block registers space via the APB3 bus. In addition the wrapper provides
> + * a common interrupt vector of the sensors conversion completion events and
> + * threshold value alarms. Alas the wrapper interface hasn't been fully thought
> + * through. There is only one sensor can be activated at a time, for which the
> + * thresholds comparator is enabled right after the data conversion is
> + * completed. Due to this if alarms need to be implemented for all available
> + * sensors we can't just set the thresholds and enable the interrupts. We need
> + * to enable the sensors one after another and let the controller to detect
> + * the alarms by itself at each conversion. This also makes pointless to handle
> + * the alarms interrupts, since in occasion they happen synchronously with
> + * data conversion completion. The best driver design would be to have the
> + * completion interrupts enabled only and keep the converted value in the
> + * driver data cache. This solution is implemented if hwmon alarms are enabled
> + * in this driver. In case if the alarms are disabled, the conversion is
> + * performed on demand at the time a sensors input file is read.
> + */
> +
> +#if defined(CONFIG_SENSORS_BT1_PVT_ALARMS)
> +
> +#define pvt_hard_isr NULL
> +
> +static irqreturn_t pvt_soft_isr(int irq, void *data)
> +{
> + const struct pvt_sensor_info *info;
> + struct pvt_hwmon *pvt = data;
> + struct pvt_cache *cache;
> + u32 val, thres_sts, old;
> +
> + /*
> + * DVALID bit will be cleared by reading the data. We need to save the
> + * status before the next conversion happens. Threshold events will be
> + * handled a bit later.
> + */
> + thres_sts = readl(pvt->regs + PVT_RAW_INTR_STAT);
> +
> + /*
> + * Then lets recharge the PVT interface with the next sampling mode.
> + * Lock the interface mutex to serialize trim, timeouts and alarm
> + * thresholds settings.
> + */
> + cache = &pvt->cache[pvt->sensor];
> + info = &pvt_info[pvt->sensor];
> + pvt->sensor = (pvt->sensor == PVT_SENSOR_LAST) ?
> + PVT_SENSOR_FIRST : (pvt->sensor + 1);
> +
> + /*
> + * For some reason we have to mask the interrupt before changing the
> + * mode, otherwise sometimes the temperature mode doesn't get
> + * activated even though the actual mode in ctrl register correspond
> + * to one. Then we read the data. By doing so we also recharge the data
> + * conversion. After this the mode corresponding to the next
> + * sensor in the row is set. Finally we enable the interrupts back.
> + */
> + mutex_lock(&pvt->iface_mtx);
> +
> + old = pvt_update(pvt->regs + PVT_INTR_MASK, PVT_INTR_DVALID,
> + PVT_INTR_DVALID);
> +
> + val = readl(pvt->regs + PVT_DATA);
> +
> + pvt_set_mode(pvt, pvt_info[pvt->sensor].mode);
> +
> + pvt_update(pvt->regs + PVT_INTR_MASK, PVT_INTR_DVALID, old);
> +
> + mutex_unlock(&pvt->iface_mtx);
> +
> + /*
> + * We can now update the data cache with data just retrieved from the
> + * sensor. Lock write-seqlock to make sure the reader has a coherent
> + * data.
> + */
> + write_seqlock(&cache->data_seqlock);
> +
> + cache->data = FIELD_GET(PVT_DATA_DATA_MASK, val);
> +
> + write_sequnlock(&cache->data_seqlock);
> +
> + /*
> + * While PVT core is doing the next mode data conversion, we'll check
> + * whether the alarms were triggered for the current sensor. Note that
> + * according to the documentation only one threshold IRQ status can be
> + * set at a time, that's why if-else statement is utilized.
> + */
> + if ((thres_sts & info->thres_sts_lo) ^ cache->thres_sts_lo) {
> + WRITE_ONCE(cache->thres_sts_lo, thres_sts & info->thres_sts_lo);
> + sysfs_notify(&pvt->hwmon->kobj, NULL, info->sysfs_min_alarm);
> + kobject_uevent(&pvt->hwmon->kobj, KOBJ_CHANGE);
> + if (cache->thres_sts_lo) {
> + dev_warn(pvt->dev, "%s is below the lower limit\n",
> + info->label);
> + } else {
> + dev_warn(pvt->dev, "%s is back to normal\n",
> + info->label);
> + }
> + } else if ((thres_sts & info->thres_sts_hi) ^ cache->thres_sts_hi) {
> + WRITE_ONCE(cache->thres_sts_hi, thres_sts & info->thres_sts_hi);
> + sysfs_notify(&pvt->hwmon->kobj, NULL, info->sysfs_max_alarm);
> + kobject_uevent(&pvt->hwmon->kobj, KOBJ_CHANGE);
> + if (cache->thres_sts_hi) {
> + dev_warn(pvt->dev, "%s is above the upper limit\n",
> + info->label);
> + } else {
> + dev_warn(pvt->dev, "%s is back to normal\n",
> + info->label);
> + }
> + }
The point of userspace notification is to let userspace deal with it.
I do not think it is a good idea to clog the kernel log with this
information. It may sound like a good idea, but in a system prone to
exceeding temperature limits all that is left in the log will be
repeated temperature warnings. Let userspace handle it, and let
userspace decide if it wants to log the events somewhere.
Also, please have a look at https://patchwork.kernel.org/patch/11277347/
and check if you can use it instead of creating notifications locally.
> +
> + return IRQ_HANDLED;
> +}
> +
> +inline umode_t pvt_limit_is_visible(enum pvt_sensor_type type)
> +{
> + return 0644;
> +}
> +
> +inline umode_t pvt_alarm_is_visible(enum pvt_sensor_type type)
> +{
> + return 0444;
> +}
> +
> +static int pvt_read_data(struct pvt_hwmon *pvt, enum pvt_sensor_type type,
> + long *val)
> +{
> + struct pvt_cache *cache = &pvt->cache[type];
> + unsigned int seq;
> + u32 data;
> +
> + do {
> + seq = read_seqbegin(&cache->data_seqlock);
> + data = cache->data;
> + } while (read_seqretry(&cache->data_seqlock, seq));
> +
> + if (type == PVT_TEMP)
> + *val = pvt_calc_poly(&poly_N_to_temp, data);
> + else
> + *val = pvt_calc_poly(&poly_N_to_volt, data);
> +
> + return 0;
> +}
> +
> +static int pvt_read_limit(struct pvt_hwmon *pvt, enum pvt_sensor_type type,
> + bool is_low, long *val)
> +{
> + u32 data;
> +
> + /* No need in serialization, since it is just read from MMIO. */
> + data = readl(pvt->regs + pvt_info[type].thres_base);
> +
> + if (is_low)
> + data = FIELD_GET(PVT_THRES_LO_MASK, data);
> + else
> + data = FIELD_GET(PVT_THRES_HI_MASK, data);
> +
> + if (type == PVT_TEMP)
> + *val = pvt_calc_poly(&poly_N_to_temp, data);
> + else
> + *val = pvt_calc_poly(&poly_N_to_volt, data);
> +
> + return 0;
> +}
> +
> +static int pvt_write_limit(struct pvt_hwmon *pvt, enum pvt_sensor_type type,
> + bool is_low, long val)
> +{
> + u32 data, limit, mask;
> + int ret;
> +
> + if (type == PVT_TEMP) {
> + val = clamp(val, PVT_TEMP_MIN, PVT_TEMP_MAX);
> + data = pvt_calc_poly(&poly_temp_to_N, val);
> + } else {
> + val = clamp(val, PVT_VOLT_MIN, PVT_VOLT_MAX);
> + data = pvt_calc_poly(&poly_volt_to_N, val);
> + }
> +
> + /* Serialize limit update, since a part of the register is changed. */
> + ret = mutex_lock_interruptible(&pvt->iface_mtx);
> + if (ret)
> + return ret;
> +
> + /* Make sure the upper and lower ranges don't intersect. */
> + limit = readl(pvt->regs + pvt_info[type].thres_base);
> + if (is_low) {
> + limit = FIELD_GET(PVT_THRES_HI_MASK, limit);
> + data = clamp_val(data, PVT_DATA_MIN, limit);
> + data = FIELD_PREP(PVT_THRES_LO_MASK, data);
> + mask = PVT_THRES_LO_MASK;
> + } else {
> + limit = FIELD_GET(PVT_THRES_LO_MASK, limit);
> + data = clamp_val(data, limit, PVT_DATA_MAX);
> + data = FIELD_PREP(PVT_THRES_HI_MASK, data);
> + mask = PVT_THRES_HI_MASK;
> + }
> +
> + pvt_update(pvt->regs + pvt_info[type].thres_base, mask, data);
> +
> + mutex_unlock(&pvt->iface_mtx);
> +
> + return 0;
> +}
> +
> +static int pvt_read_alarm(struct pvt_hwmon *pvt, enum pvt_sensor_type type,
> + bool is_low, long *val)
> +{
> + if (is_low)
> + *val = !!READ_ONCE(pvt->cache[type].thres_sts_lo);
> + else
> + *val = !!READ_ONCE(pvt->cache[type].thres_sts_hi);
> +
> + return 0;
> +}
> +
> +static const struct hwmon_channel_info *pvt_channel_info[] = {
> + HWMON_CHANNEL_INFO(chip,
> + HWMON_C_REGISTER_TZ | HWMON_C_UPDATE_INTERVAL),
> + HWMON_CHANNEL_INFO(temp,
> + HWMON_T_INPUT | HWMON_T_TYPE | HWMON_T_LABEL |
> + HWMON_T_MIN | HWMON_T_MIN_ALARM |
> + HWMON_T_MAX | HWMON_T_MAX_ALARM),
> + HWMON_CHANNEL_INFO(in,
> + HWMON_I_INPUT | HWMON_I_LABEL |
> + HWMON_I_MIN | HWMON_I_MIN_ALARM |
> + HWMON_I_MAX | HWMON_I_MAX_ALARM,
> + HWMON_I_INPUT | HWMON_I_LABEL |
> + HWMON_I_MIN | HWMON_I_MIN_ALARM |
> + HWMON_I_MAX | HWMON_I_MAX_ALARM,
> + HWMON_I_INPUT | HWMON_I_LABEL |
> + HWMON_I_MIN | HWMON_I_MIN_ALARM |
> + HWMON_I_MAX | HWMON_I_MAX_ALARM,
> + HWMON_I_INPUT | HWMON_I_LABEL |
> + HWMON_I_MIN | HWMON_I_MIN_ALARM |
> + HWMON_I_MAX | HWMON_I_MAX_ALARM),
> + NULL
> +};
> +
> +#else /* !CONFIG_SENSORS_BT1_PVT_ALARMS */
> +
> +static irqreturn_t pvt_hard_isr(int irq, void *data)
> +{
> + struct pvt_hwmon *pvt = data;
> + struct pvt_cache *cache;
> + u32 val;
> +
> + /*
> + * Mask the DVALID interrupt so after exiting from the handler a
> + * repeated conversion wouldn't happen.
> + */
> + pvt_update(pvt->regs + PVT_INTR_MASK, PVT_INTR_DVALID,
> + PVT_INTR_DVALID);
> +
> + /*
> + * Nothing special for alarm-less driver. Just read the data, update
> + * the cache and notify a waiter of this event.
> + */
> + val = readl(pvt->regs + PVT_DATA);
> + if (!(val & PVT_DATA_VALID)) {
> + dev_err(pvt->dev, "Got IRQ when data isn't valid\n");
> + return IRQ_HANDLED;
> + }
> +
> + cache = &pvt->cache[pvt->sensor];
> +
> + WRITE_ONCE(cache->data, FIELD_GET(PVT_DATA_DATA_MASK, val));
> +
> + complete(&cache->conversion);
> +
> + return IRQ_HANDLED;
> +}
> +
> +#define pvt_soft_isr NULL
> +
> +inline umode_t pvt_limit_is_visible(enum pvt_sensor_type type)
> +{
> + return 0;
> +}
> +
> +inline umode_t pvt_alarm_is_visible(enum pvt_sensor_type type)
> +{
> + return 0;
> +}
> +
> +static int pvt_read_data(struct pvt_hwmon *pvt, enum pvt_sensor_type type,
> + long *val)
> +{
> + struct pvt_cache *cache = &pvt->cache[type];
> + u32 data;
> + int ret;
> +
> + /*
> + * Lock PVT conversion interface until data cache is updated. The
> + * data read procedure is following: set the requested PVT sensor
> + * mode, enable IRQ and conversion, wait until conversion is finished,
> + * then disable conversion and IRQ, and read the cached data.
> + */
> + ret = mutex_lock_interruptible(&pvt->iface_mtx);
> + if (ret)
> + return ret;
> +
> + pvt->sensor = type;
> + pvt_set_mode(pvt, pvt_info[type].mode);
> +
> + /*
> + * Unmask the DVALID interrupt and enable the sensors conversions.
> + * Do the reverse procedure when conversion is done.
> + */
> + pvt_update(pvt->regs + PVT_INTR_MASK, PVT_INTR_DVALID, 0);
> + pvt_update(pvt->regs + PVT_CTRL, PVT_CTRL_EN, PVT_CTRL_EN);
> +
> + wait_for_completion(&cache->conversion);
> +
> + pvt_update(pvt->regs + PVT_CTRL, PVT_CTRL_EN, 0);
> + pvt_update(pvt->regs + PVT_INTR_MASK, PVT_INTR_DVALID,
> + PVT_INTR_DVALID);
> +
> + data = READ_ONCE(cache->data);
> +
> + mutex_unlock(&pvt->iface_mtx);
> +
> + if (type == PVT_TEMP)
> + *val = pvt_calc_poly(&poly_N_to_temp, data);
> + else
> + *val = pvt_calc_poly(&poly_N_to_volt, data);
> +
> + return 0;
> +}
> +
> +static int pvt_read_limit(struct pvt_hwmon *pvt, enum pvt_sensor_type type,
> + bool is_low, long *val)
> +{
> + return -EOPNOTSUPP;
> +}
> +
> +static int pvt_write_limit(struct pvt_hwmon *pvt, enum pvt_sensor_type type,
> + bool is_low, long val)
> +{
> + return -EOPNOTSUPP;
> +}
> +
> +static int pvt_read_alarm(struct pvt_hwmon *pvt, enum pvt_sensor_type type,
> + bool is_low, long *val)
> +{
> + return -EOPNOTSUPP;
> +}
> +
> +static const struct hwmon_channel_info *pvt_channel_info[] = {
> + HWMON_CHANNEL_INFO(chip,
> + HWMON_C_REGISTER_TZ | HWMON_C_UPDATE_INTERVAL),
> + HWMON_CHANNEL_INFO(temp,
> + HWMON_T_INPUT | HWMON_T_TYPE | HWMON_T_LABEL),
> + HWMON_CHANNEL_INFO(in,
> + HWMON_I_INPUT | HWMON_I_LABEL,
> + HWMON_I_INPUT | HWMON_I_LABEL,
> + HWMON_I_INPUT | HWMON_I_LABEL,
> + HWMON_I_INPUT | HWMON_I_LABEL),
> + NULL
> +};
> +
> +#endif /* !CONFIG_SENSORS_BT1_PVT_ALARMS */
> +
> +static inline bool pvt_hwmon_channel_is_valid(enum hwmon_sensor_types type,
> + int ch)
> +{
> + switch (type) {
> + case hwmon_temp:
> + if (ch < 0 || ch >= PVT_TEMP_CHS)
> + return false;
> + break;
> + case hwmon_in:
> + if (ch < 0 || ch >= PVT_VOLT_CHS)
> + return false;
> + break;
> + default:
> + break;
> + }
> +
> + /* The rest of the types are independent from the channel number. */
> + return true;
> +}
> +
> +static umode_t pvt_hwmon_is_visible(const void *data,
> + enum hwmon_sensor_types type,
> + u32 attr, int ch)
> +{
> + if (!pvt_hwmon_channel_is_valid(type, ch))
> + return 0;
> +
> + switch (type) {
> + case hwmon_chip:
> + switch (attr) {
> + case hwmon_chip_update_interval:
> + return 0644;
> + }
> + break;
> + case hwmon_temp:
> + switch (attr) {
> + case hwmon_temp_input:
> + case hwmon_temp_type:
> + case hwmon_temp_label:
> + return 0444;
> + case hwmon_temp_min:
> + case hwmon_temp_max:
> + return pvt_limit_is_visible(ch);
> + case hwmon_temp_min_alarm:
> + case hwmon_temp_max_alarm:
> + return pvt_alarm_is_visible(ch);
> + }
> + break;
> + case hwmon_in:
> + switch (attr) {
> + case hwmon_in_input:
> + case hwmon_in_label:
> + return 0444;
> + case hwmon_in_min:
> + case hwmon_in_max:
> + return pvt_limit_is_visible(PVT_VOLT + ch);
> + case hwmon_in_min_alarm:
> + case hwmon_in_max_alarm:
> + return pvt_alarm_is_visible(PVT_VOLT + ch);
> + }
> + break;
> + default:
> + break;
> + }
> +
> + return 0;
> +}
> +
> +static int pvt_read_timeout(struct pvt_hwmon *pvt, long *val)
> +{
> + unsigned long rate;
> + ktime_t kt;
> + u32 data;
> +
> + rate = clk_get_rate(pvt->clks[PVT_CLOCK_REF].clk);
Is this a dynamic clock ? If not, it would be better to read its frequency once
in the probe function
> + if (!rate)
> + return -ENODEV;
... and return -ENODEV there if it is 0.
> +
> + /*
> + * Don't bother with mutex here, since we just read data from MMIO.
> + * We also have to scale the ticks timeout up to compensate the
> + * ms-ns-data translations.
> + */
> + data = readl(pvt->regs + PVT_TTIMEOUT) + 1;
> +
> + /*
> + * Calculate ref-clock based delay (Ttotal) between two consecutive
> + * data samples of the same sensor. So we first must calculate the
> + * delay introduced by the internal ref-clock timer (Tref * Fclk).
> + * Then add the constant timeout cuased by each conversion latency
> + * (Tmin). The basic formulae for each conversion is following:
> + * Ttotal = Tref * Fclk + Tmin
> + * Note if alarms are enabled the sensors are polled one after
> + * another, so in order to have the delay being applicable for each
> + * sensor the requested value must be equally redistirbuted.
> + */
> +#if defined(CONFIG_SENSORS_BT1_PVT_ALARMS)
> + kt = ktime_set(PVT_SENSORS_NUM * (u64)data, 0);
> + kt = ktime_divns(kt, rate);
> + kt = ktime_add_ns(kt, PVT_SENSORS_NUM * PVT_TOUT_MIN);
> +#else
> + kt = ktime_set(data, 0);
> + kt = ktime_divns(kt, rate);
> + kt = ktime_add_ns(kt, PVT_TOUT_MIN);
> +#endif
> +
> + /* Return the result in msec as hwmon sysfs interface requires. */
> + *val = ktime_to_ms(kt);
Personally I am not sure about the value of using ktime for those
calculations, but that is your call. AFAICS ktime_to_ms() rounds down,
so I assume you understand that this can return 0.
> +
> + return 0;
> +}
> +
> +static int pvt_write_timeout(struct pvt_hwmon *pvt, long val)
> +{
> + unsigned long rate;
> + ktime_t kt;
> + u32 data;
> + int ret;
> +
> + rate = clk_get_rate(pvt->clks[PVT_CLOCK_REF].clk);
> + if (!rate)
> + return -ENODEV;
> +
> + /*
> + * If alarms are enabled, the requested timeout must be divided
> + * between all available sensors to have the requested delay
> + * applicable to each individual sensor.
> + */
> + kt = ms_to_ktime(val);
> +#if defined(CONFIG_SENSORS_BT1_PVT_ALARMS)
> + kt = ktime_divns(kt, PVT_SENSORS_NUM);
> +#endif
> +
> + /*
> + * Subtract a constant lag, which always persists due to the limited
> + * PVT sampling rate. Make sure the timeout is non-zero.
> + */
> + kt = ktime_sub_ns(kt, PVT_TOUT_MIN);
> + if (ktime_to_ns(kt) < 0)
> + kt = ktime_set(0, 0);
Doesn't that set the timeout to 0 ? That seems to conflict with the "non-zero"
comment above. Should that be "not negative" ?
> +
> + /*
> + * Finally recalculate the timeout in terms of the reference clock
> + * period.
> + */
> + data = ktime_divns(kt * rate, NSEC_PER_SEC);
> +
> + /*
> + * Update the measurements delay, but lock the interface first, since
> + * we have to disable PVT in order to have the new delay actually
> + * updated.
> + */
> + ret = mutex_lock_interruptible(&pvt->iface_mtx);
> + if (ret)
> + return ret;
> +
> + pvt_set_tout(pvt, data);
> +
> + mutex_unlock(&pvt->iface_mtx);
> +
> + return 0;
> +}
> +
> +static int pvt_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
> + u32 attr, int ch, long *val)
> +{
> + struct pvt_hwmon *pvt = dev_get_drvdata(dev);
> +
> + if (!pvt_hwmon_channel_is_valid(type, ch))
> + return -EINVAL;
> +
> + switch (type) {
> + case hwmon_chip:
> + switch (attr) {
> + case hwmon_chip_update_interval:
> + return pvt_read_timeout(pvt, val);
> + }
> + break;
> + case hwmon_temp:
> + switch (attr) {
> + case hwmon_temp_input:
> + return pvt_read_data(pvt, ch, val);
> + case hwmon_temp_type:
> + *val = 1;
> + return 0;
> + case hwmon_temp_min:
> + return pvt_read_limit(pvt, ch, true, val);
> + case hwmon_temp_max:
> + return pvt_read_limit(pvt, ch, false, val);
> + case hwmon_temp_min_alarm:
> + return pvt_read_alarm(pvt, ch, true, val);
> + case hwmon_temp_max_alarm:
> + return pvt_read_alarm(pvt, ch, false, val);
> + }
> + break;
> + case hwmon_in:
> + switch (attr) {
> + case hwmon_in_input:
> + return pvt_read_data(pvt, PVT_VOLT + ch, val);
> + case hwmon_in_min:
> + return pvt_read_limit(pvt, PVT_VOLT + ch, true, val);
> + case hwmon_in_max:
> + return pvt_read_limit(pvt, PVT_VOLT + ch, false, val);
> + case hwmon_in_min_alarm:
> + return pvt_read_alarm(pvt, PVT_VOLT + ch, true, val);
> + case hwmon_in_max_alarm:
> + return pvt_read_alarm(pvt, PVT_VOLT + ch, false, val);
> + }
> + break;
> + default:
> + break;
> + }
> +
> + return -EOPNOTSUPP;
> +}
> +
> +static int pvt_hwmon_read_string(struct device *dev,
> + enum hwmon_sensor_types type,
> + u32 attr, int ch, const char **str)
> +{
> + if (!pvt_hwmon_channel_is_valid(type, ch))
> + return -EINVAL;
> +
> + switch (type) {
> + case hwmon_temp:
> + switch (attr) {
> + case hwmon_temp_label:
> + *str = pvt_info[ch].label;
> + return 0;
> + }
> + break;
> + case hwmon_in:
> + switch (attr) {
> + case hwmon_in_label:
> + *str = pvt_info[PVT_VOLT + ch].label;
> + return 0;
> + }
> + break;
> + default:
> + break;
> + }
> +
> + return -EOPNOTSUPP;
> +}
> +
> +int pvt_hwmon_write(struct device *dev, enum hwmon_sensor_types type,
> + u32 attr, int ch, long val)
> +{
> + struct pvt_hwmon *pvt = dev_get_drvdata(dev);
> +
> + if (!pvt_hwmon_channel_is_valid(type, ch))
> + return -EINVAL;
> +
> + switch (type) {
> + case hwmon_chip:
> + switch (attr) {
> + case hwmon_chip_update_interval:
> + return pvt_write_timeout(pvt, val);
> + }
> + break;
> + case hwmon_temp:
> + switch (attr) {
> + case hwmon_temp_min:
> + return pvt_write_limit(pvt, ch, true, val);
> + case hwmon_temp_max:
> + return pvt_write_limit(pvt, ch, false, val);
> + }
> + break;
> + case hwmon_in:
> + switch (attr) {
> + case hwmon_in_min:
> + return pvt_write_limit(pvt, PVT_VOLT + ch, true, val);
> + case hwmon_in_max:
> + return pvt_write_limit(pvt, PVT_VOLT + ch, false, val);
> + }
> + break;
> + default:
> + break;
> + }
> +
> + return -EOPNOTSUPP;
> +}
> +
> +static const struct hwmon_ops pvt_hwmon_ops = {
> + .is_visible = pvt_hwmon_is_visible,
> + .read = pvt_hwmon_read,
> + .read_string = pvt_hwmon_read_string,
> + .write = pvt_hwmon_write
> +};
> +
> +static const struct hwmon_chip_info pvt_hwmon_info = {
> + .ops = &pvt_hwmon_ops,
> + .info = pvt_channel_info
> +};
> +
> +static ssize_t pvt_hwmon_temp_trim_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct pvt_hwmon *pvt = dev_get_drvdata(dev);
> + u32 data;
> +
> + data = readl(pvt->regs + PVT_CTRL);
> + data = FIELD_GET(PVT_CTRL_TRIM_MASK, data);
> +
> + return scnprintf(buf, PAGE_SIZE, "%u\n", data);
> +}
> +
> +static ssize_t pvt_hwmon_temp_trim_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct pvt_hwmon *pvt = dev_get_drvdata(dev);
> + u32 data;
> + int ret;
> +
> + ret = kstrtou32(buf, 0, &data);
> + if (ret)
> + return ret;
> +
> + data = clamp_val(data, PVT_TRIM_MIN, PVT_TRIM_MAX);
> +
> + /*
> + * Serialize trim update, since a part of the register is changed and
> + * the controller is supposed to be disabled during this operation.
> + */
> + ret = mutex_lock_interruptible(&pvt->iface_mtx);
> + if (ret)
> + return ret;
> +
> + pvt_set_trim(pvt, data);
> +
> + mutex_unlock(&pvt->iface_mtx);
> +
> + return count;
> +}
> +static SENSOR_DEVICE_ATTR_RW(temp1_trim, pvt_hwmon_temp_trim, 0);
> +
> +static struct attribute *pvt_hwmon_attrs[] = {
> + &sensor_dev_attr_temp1_trim.dev_attr.attr,
> + NULL
> +};
> +ATTRIBUTE_GROUPS(pvt_hwmon);
> +
> +static void pvt_clear_data(void *data)
> +{
> + struct pvt_hwmon *pvt = data;
> +#if !defined(CONFIG_SENSORS_BT1_PVT_ALARMS)
> + int idx;
> +
> + for (idx = 0; idx < PVT_SENSORS_NUM; ++idx)
> + complete_all(&pvt->cache[idx].conversion);
> +#endif
> +
> + mutex_destroy(&pvt->iface_mtx);
> + platform_set_drvdata(to_platform_device(pvt->dev), NULL);
Unnecessary.
> +}
> +
> +static struct pvt_hwmon *pvt_create_data(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct pvt_hwmon *pvt;
> + int ret, idx;
> +
> + pvt = devm_kzalloc(dev, sizeof(*pvt), GFP_KERNEL);
> + if (!pvt)
> + return ERR_PTR(-ENOMEM);
> +
> + ret = devm_add_action(dev, pvt_clear_data, pvt);
> + if (ret) {
> + dev_err(dev, "Can't add PVT data clear action\n");
> + return ERR_PTR(ret);
> + }
> +
> + pvt->dev = dev;
> + pvt->sensor = PVT_SENSOR_FIRST;
> + platform_set_drvdata(pdev, pvt);
I don't immediately see where this is used. Can you point to it ?
> + mutex_init(&pvt->iface_mtx);
> +
> +#if defined(CONFIG_SENSORS_BT1_PVT_ALARMS)
> + for (idx = 0; idx < PVT_SENSORS_NUM; ++idx)
> + seqlock_init(&pvt->cache[idx].data_seqlock);
> +#else
> + for (idx = 0; idx < PVT_SENSORS_NUM; ++idx)
> + init_completion(&pvt->cache[idx].conversion);
> +#endif
> +
> + return pvt;
> +}
> +
> +static int pvt_request_regs(struct pvt_hwmon *pvt)
> +{
> + struct platform_device *pdev = to_platform_device(pvt->dev);
> + struct resource *res;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res) {
> + dev_err(pvt->dev, "Couldn't find PVT memresource\n");
> + return -EINVAL;
> + }
> +
> + pvt->regs = devm_ioremap_resource(pvt->dev, res);
> + if (IS_ERR(pvt->regs)) {
> + dev_err(pvt->dev, "Couldn't map PVT registers\n");
> + return PTR_ERR(pvt->regs);
> + }
> +
> + return 0;
> +}
> +
> +static void pvt_disable_clks(void *data)
> +{
> + struct pvt_hwmon *pvt = data;
> +
> + clk_bulk_disable_unprepare(PVT_CLOCK_NUM, pvt->clks);
> +}
> +
> +static int pvt_request_clks(struct pvt_hwmon *pvt)
> +{
> + int ret;
> +
> + pvt->clks[PVT_CLOCK_APB].id = "pclk";
> + pvt->clks[PVT_CLOCK_REF].id = "ref";
> +
> + ret = devm_clk_bulk_get(pvt->dev, PVT_CLOCK_NUM, pvt->clks);
> + if (ret) {
> + dev_err(pvt->dev, "Couldn't get PVT clocks descriptors\n");
> + return ret;
> + }
> +
> + ret = clk_bulk_prepare_enable(PVT_CLOCK_NUM, pvt->clks);
> + if (ret) {
> + dev_err(pvt->dev, "Couldn't enable the PVT clocks\n");
> + return ret;
> + }
> +
> + ret = devm_add_action_or_reset(pvt->dev, pvt_disable_clks, pvt);
> + if (ret) {
> + dev_err(pvt->dev, "Can't add PVT clocks disable action\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static void pvt_init_iface(struct pvt_hwmon *pvt)
> +{
> + /*
> + * Make sure all interrupts and controller are disabled so not to
> + * accidentally have ISR executed before the driver data is fully
> + * initialized. Clear the IRQ status as well.
> + */
> + pvt_update(pvt->regs + PVT_INTR_MASK, PVT_INTR_ALL, PVT_INTR_ALL);
> + pvt_update(pvt->regs + PVT_CTRL, PVT_CTRL_EN, 0);
> + readl(pvt->regs + PVT_CLR_INTR);
> + readl(pvt->regs + PVT_DATA);
> +
> + /* Setup default sensor mode, timeout and temperature trim. */
> + pvt_set_mode(pvt, pvt_info[pvt->sensor].mode);
> + pvt_set_tout(pvt, PVT_TOUT_DEF);
> + pvt_set_trim(pvt, PVT_TRIM_DEF);
> +}
> +
> +static int pvt_request_irq(struct pvt_hwmon *pvt)
> +{
> + struct platform_device *pdev = to_platform_device(pvt->dev);
> + int ret;
> +
> + pvt->irq = platform_get_irq(pdev, 0);
> + if (pvt->irq < 0)
> + return pvt->irq;
> +
> + ret = devm_request_threaded_irq(pvt->dev, pvt->irq,
> + pvt_hard_isr, pvt_soft_isr,
> +#if defined(CONFIG_SENSORS_BT1_PVT_ALARMS)
> + IRQF_SHARED | IRQF_TRIGGER_HIGH |
> + IRQF_ONESHOT,
> +#else
> + IRQF_SHARED | IRQF_TRIGGER_HIGH,
> +#endif
> + "pvt", pvt);
> + if (ret) {
> + dev_err(pvt->dev, "Couldn't request PVT IRQ\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int pvt_create_hwmon(struct pvt_hwmon *pvt)
> +{
> + pvt->hwmon = devm_hwmon_device_register_with_info(pvt->dev,
> + dev_name(pvt->dev), pvt, &pvt_hwmon_info, pvt_hwmon_groups);
Is the device name guaranteed to be a valid hwmon device name ?
> + if (IS_ERR(pvt->hwmon)) {
> + dev_err(pvt->dev, "Couldn't create hwmon device\n");
> + return PTR_ERR(pvt->hwmon);
> + }
> +
> + return 0;
> +}
> +
> +#if defined(CONFIG_SENSORS_BT1_PVT_ALARMS)
> +
> +static void pvt_disable_iface(void *data)
> +{
> + struct pvt_hwmon *pvt = data;
> +
> + mutex_lock(&pvt->iface_mtx);
> + pvt_update(pvt->regs + PVT_CTRL, PVT_CTRL_EN, 0);
> + pvt_update(pvt->regs + PVT_INTR_MASK, PVT_INTR_DVALID,
> + PVT_INTR_DVALID);
> + mutex_unlock(&pvt->iface_mtx);
> +}
> +
> +static int pvt_enable_iface(struct pvt_hwmon *pvt)
> +{
> + int ret;
> +
> + ret = devm_add_action(pvt->dev, pvt_disable_iface, pvt);
> + if (ret) {
> + dev_err(pvt->dev, "Can't add PVT disable interface action\n");
> + return ret;
> + }
> +
> + /*
> + * Enable sensors data conversion and IRQ. We need to lock the
> + * interface mutex since hwmon has just been created and the
> + * corresponding sysfs files are accessible from user-space,
> + * which theoretically may cause races.
> + */
What if those userspace actions make a request which requires an interrupt ?
> + mutex_lock(&pvt->iface_mtx);
> + pvt_update(pvt->regs + PVT_INTR_MASK, PVT_INTR_DVALID, 0);
> + pvt_update(pvt->regs + PVT_CTRL, PVT_CTRL_EN, PVT_CTRL_EN);
> + mutex_unlock(&pvt->iface_mtx);
> +
> + return 0;
> +}
> +
> +#else /* !CONFIG_SENSORS_BT1_PVT_ALARMS */
> +
> +static int pvt_enable_iface(struct pvt_hwmon *pvt)
> +{
> + return 0;
> +}
> +
> +#endif /* !CONFIG_SENSORS_BT1_PVT_ALARMS */
> +
> +static int pvt_probe(struct platform_device *pdev)
> +{
> + struct pvt_hwmon *pvt;
> + int ret;
> +
> + pvt = pvt_create_data(pdev);
> + if (IS_ERR(pvt))
> + return PTR_ERR(pvt);
> +
> + ret = pvt_request_regs(pvt);
> + if (ret)
> + return ret;
> +
> + ret = pvt_request_clks(pvt);
> + if (ret)
> + return ret;
> +
> + pvt_init_iface(pvt);
> +
> + ret = pvt_request_irq(pvt);
> + if (ret)
> + return ret;
> +
> + ret = pvt_create_hwmon(pvt);
> + if (ret)
> + return ret;
> +
> + ret = pvt_enable_iface(pvt);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +static const struct of_device_id pvt_of_match[] = {
> + { .compatible = "baikal,bt1-pvt" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, pvt_of_match);
> +
> +static struct platform_driver pvt_driver = {
> + .probe = pvt_probe,
> + .driver = {
> + .name = "bt1-pvt",
> + .of_match_table = pvt_of_match
> + }
> +};
> +module_platform_driver(pvt_driver);
> +
> +MODULE_AUTHOR("Maxim Kaurkin <maxim.kaurkin@baikalelectronics.ru>");
> +MODULE_DESCRIPTION("Baikal-T1 PVT driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/hwmon/bt1-pvt.h b/drivers/hwmon/bt1-pvt.h
> new file mode 100644
> index 000000000000..7851ed947877
> --- /dev/null
> +++ b/drivers/hwmon/bt1-pvt.h
> @@ -0,0 +1,237 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2020 BAIKAL ELECTRONICS, JSC
> + *
> + * Baikal-T1 Process, Voltage, Temperature sensor driver
> + */
> +#ifndef __HWMON_BT1_PVT_H__
> +#define __HWMON_BT1_PVT_H__
> +
> +#include <linux/kernel.h>
> +#include <linux/platform_device.h>
> +#include <linux/io.h>
> +#include <linux/mutex.h>
> +#include <linux/seqlock.h>
> +#include <linux/completion.h>
> +#include <linux/sysfs.h>
> +#include <linux/hwmon.h>
Alphabetic order, please, and drop unnecessary includes.
> +
> +/* Baikal-T1 PVT registers and their bitfields */
> +#define PVT_CTRL 0x00
> +#define PVT_CTRL_EN BIT(0)
> +#define PVT_CTRL_MODE_FLD 1
> +#define PVT_CTRL_MODE_MASK GENMASK(3, PVT_CTRL_MODE_FLD)
> +#define PVT_CTRL_MODE_TEMP 0x0
> +#define PVT_CTRL_MODE_VOLT 0x1
> +#define PVT_CTRL_MODE_LVT 0x2
> +#define PVT_CTRL_MODE_HVT 0x4
> +#define PVT_CTRL_MODE_SVT 0x6
> +#define PVT_CTRL_TRIM_FLD 4
> +#define PVT_CTRL_TRIM_MASK GENMASK(8, PVT_CTRL_TRIM_FLD)
> +#define PVT_DATA 0x04
> +#define PVT_DATA_VALID BIT(10)
> +#define PVT_DATA_DATA_FLD 0
> +#define PVT_DATA_DATA_MASK GENMASK(9, PVT_DATA_DATA_FLD)
> +#define PVT_TTHRES 0x08
> +#define PVT_VTHRES 0x0C
> +#define PVT_LTHRES 0x10
> +#define PVT_HTHRES 0x14
> +#define PVT_STHRES 0x18
> +#define PVT_THRES_LO_FLD 0
> +#define PVT_THRES_LO_MASK GENMASK(9, PVT_THRES_LO_FLD)
> +#define PVT_THRES_HI_FLD 10
> +#define PVT_THRES_HI_MASK GENMASK(19, PVT_THRES_HI_FLD)
> +#define PVT_TTIMEOUT 0x1C
> +#define PVT_INTR_STAT 0x20
> +#define PVT_INTR_MASK 0x24
> +#define PVT_RAW_INTR_STAT 0x28
> +#define PVT_INTR_DVALID BIT(0)
> +#define PVT_INTR_TTHRES_LO BIT(1)
> +#define PVT_INTR_TTHRES_HI BIT(2)
> +#define PVT_INTR_VTHRES_LO BIT(3)
> +#define PVT_INTR_VTHRES_HI BIT(4)
> +#define PVT_INTR_LTHRES_LO BIT(5)
> +#define PVT_INTR_LTHRES_HI BIT(6)
> +#define PVT_INTR_HTHRES_LO BIT(7)
> +#define PVT_INTR_HTHRES_HI BIT(8)
> +#define PVT_INTR_STHRES_LO BIT(9)
> +#define PVT_INTR_STHRES_HI BIT(10)
> +#define PVT_INTR_ALL GENMASK(10, 0)
> +#define PVT_CLR_INTR 0x2C
> +
> +/*
> + * PVT sensors-related limits and default values
> + * @PVT_TEMP_MIN: Minimal temperature in millidegrees of Celsius.
> + * @PVT_TEMP_MAX: Maximal temperature in millidegrees of Celsius.
> + * @PVT_TEMP_CHS: Number of temperature hwmon channels.
> + * @PVT_VOLT_MIN: Minimal voltage in mV.
> + * @PVT_VOLT_MAX: Maximal voltage in mV.
> + * @PVT_VOLT_CHS: Number of voltage hwmon channels.
> + * @PVT_DATA_MIN: Minimal PVT raw data value.
> + * @PVT_DATA_MAX: Maximal PVT raw data value.
> + * @PVT_TRIM_MIN: Minimal temperature sensor trim value.
> + * @PVT_TRIM_MAX: Maximal temperature sensor trim value.
> + * @PVT_TRIM_DEF: Default temperature sensor trim value (set a proper value
> + * when one is determined for Baikal-T1 SoC).
> + * @PVT_TOUT_MIN: Minimal timeout between samples in nanoseconds.
> + * @PVT_TOUT_DEF: Default data measurements timeout. In case if alarms are
> + * activated the PVT IRQ is enabled to be raised after each
> + * conversion in order to have the thresholds checked and the
> + * converted value cached. Too frequent conversions may cause
> + * the system CPU overload. Lets set the 50ms delay between
> + * them by default to prevent this.
> + */
> +#define PVT_TEMP_MIN -48380L
> +#define PVT_TEMP_MAX 147438L
> +#define PVT_TEMP_CHS 1
> +#define PVT_VOLT_MIN 620L
> +#define PVT_VOLT_MAX 1168L
> +#define PVT_VOLT_CHS 4
> +#define PVT_DATA_MIN 0
> +#define PVT_DATA_MAX (PVT_DATA_DATA_MASK >> PVT_DATA_DATA_FLD)
> +#define PVT_TRIM_MIN 0
> +#define PVT_TRIM_MAX (PVT_CTRL_TRIM_MASK >> PVT_CTRL_TRIM_FLD)
> +#define PVT_TRIM_DEF 0
> +#define PVT_TOUT_MIN (NSEC_PER_SEC / 3000)
> +#if defined(CONFIG_SENSORS_BT1_PVT_ALARMS)
> +# define PVT_TOUT_DEF 60000
> +#else
> +# define PVT_TOUT_DEF 0
> +#endif
> +
> +/*
> + * enum pvt_sensor_type - Baikal-T1 PVT sensor types (correspond to each PVT
> + * sampling mode)
> + * @PVT_SENSOR*: helpers to traverse the sensors in loops.
> + * @PVT_TEMP: PVT Temperature sensor.
> + * @PVT_VOLT: PVT Voltage sensor.
> + * @PVT_LVT: PVT Low-Voltage threshold sensor.
> + * @PVT_HVT: PVT High-Voltage threshold sensor.
> + * @PVT_SVT: PVT Standard-Voltage threshold sensor.
> + */
> +enum pvt_sensor_type {
> + PVT_SENSOR_FIRST,
> + PVT_TEMP = PVT_SENSOR_FIRST,
> + PVT_VOLT,
> + PVT_LVT,
> + PVT_HVT,
> + PVT_SVT,
> + PVT_SENSOR_LAST = PVT_SVT,
> + PVT_SENSORS_NUM
> +};
> +
> +/*
> + * enum pvt_clock_type - Baikal-T1 PVT clocks.
> + * @PVT_CLOCK_APB: APB clock.
> + * @PVT_CLOCK_REF: PVT reference clock.
> + */
> +enum pvt_clock_type {
> + PVT_CLOCK_APB,
> + PVT_CLOCK_REF,
> + PVT_CLOCK_NUM
> +};
> +
> +/*
> + * struct pvt_sensor_info - Baikal-T1 PVT sensor informational structure
> + * @label: hwmon sensor label.
> + * @mode: PVT mode corresponding to the channel.
> + * @thres_base: upper and lower threshold values of the sensor.
> + * @thres_sts_lo: low threshold status bitfield.
> + * @thres_sts_hi: high threshold status bitfield.
> + * @sysfs_min_alarm: pointer to a min-alarm sysfs filename.
> + * @sysfs_min_alarm: pointer to a max-alarm sysfs filename.
> + */
> +struct pvt_sensor_info {
> + const char *label;
> + u32 mode;
> + unsigned long thres_base;
> + u32 thres_sts_lo;
> + u32 thres_sts_hi;
> + const char *sysfs_min_alarm;
> + const char *sysfs_max_alarm;
> +};
> +
> +#define PVT_SENSOR_INFO(_label, _file, _mode, _thres) \
_file seems to be a global define or variable, or at least checkpatch
complains about its use.
> + { \
> + .label = _label, \
> + .mode = PVT_CTRL_MODE_ ##_mode, \
> + .thres_base = PVT_ ##_thres, \
> + .thres_sts_lo = PVT_INTR_ ##_thres## _LO, \
> + .thres_sts_hi = PVT_INTR_ ##_thres## _HI, \
> + .sysfs_min_alarm = _file "_min_alarm", \
> + .sysfs_max_alarm = _file "_max_alarm" \
> + }
> +
> +/*
> + * struct pvt_cache - PVT sensors data cache
> + * @data: data cache in raw format.
> + * @thres_sts_lo: low threshold status saved on the previous data conversion.
> + * @thres_sts_hi: high threshold status saved on the previous data conversion.
> + * @data_seqlock: cached data seq-lock.
> + * @conversion: data conversion completion.
> + */
> +struct pvt_cache {
> + u32 data;
> +#if defined(CONFIG_SENSORS_BT1_PVT_ALARMS)
> + seqlock_t data_seqlock;
> + u32 thres_sts_lo;
> + u32 thres_sts_hi;
> +#else
> + struct completion conversion;
> +#endif
> +};
> +
> +/*
> + * struct pvt_hwmon - Baikal-T1 PVT private data
> + * @dev: device structure of the PVT platform device.
> + * @hwmon: hwmon device structure.
> + * @regs: pointer to the Baikal-T1 PVT registers region.
> + * @irq: PVT events IRQ number.
> + * @clks: Array of the PVT clocks descriptor (APB/ref clocks).
> + * @ref_clk: Pointer to the reference clocks descriptor.
> + * @iface_mtx: Generic interface mutex (used to lock the alarm registers
> + * when the alarms enabled, or the data conversion interface
> + * if alarms are disabled).
> + * @sensor: current PVT sensor the data conversion is being performed for.
> + * @cache: data cache descriptor.
> + */
> +struct pvt_hwmon {
> + struct device *dev;
> + struct device *hwmon;
> +
> + void __iomem *regs;
> + int irq;
> +
> + struct clk_bulk_data clks[PVT_CLOCK_NUM];
> +
> + struct mutex iface_mtx;
> + enum pvt_sensor_type sensor;
> + struct pvt_cache cache[PVT_SENSORS_NUM];
> +};
> +
> +/*
> + * struct pvt_poly_term - a term descriptor of the PVT data translation
> + * polynomial
> + * @deg: degree of the term.
> + * @coef: multiplication factor of the term.
> + * @divider: distributed divider per each degree.
> + * @divider_leftover: divider leftover, which couldn't be redistributed.
> + */
> +struct pvt_poly_term {
> + unsigned int deg;
> + long coef;
> + long divider;
> + long divider_leftover;
> +};
> +
> +/*
> + * struct pvt_poly - PVT data translation polynomial descriptor
> + * @total_divider: total data divider.
> + * @terms: polynomial terms up to a free one.
> + */
> +struct pvt_poly {
> + long total_divider;
> + struct pvt_poly_term terms[];
> +};
> +
> +#endif /* __HWMON_BT1_PVT_H__ */
^ permalink raw reply
* Re: [RFC PATCH V4 4/4] platform: mtk-isp: Add Mediatek FD driver
From: Jerry-ch Chen @ 2020-05-22 14:10 UTC (permalink / raw)
To: Tomasz Figa
Cc: hverkuil, hans.verkuil, laurent.pinchart+renesas, matthias.bgg,
mchehab, pihsun, yuzhao, zwisler, linux-mediatek,
linux-arm-kernel, Sean.Cheng, sj.huang, christie.yu,
frederic.chen, jungo.lin, Rynn.Wu, linux-media, srv_heupstream,
devicetree, jerry-ch.chen
In-Reply-To: <20200521182825.GA249683@chromium.org>
Hi Tomasz,
On Thu, 2020-05-21 at 18:28 +0000, Tomasz Figa wrote:
> Hi Jerry,
>
> On Wed, Dec 04, 2019 at 08:47:32PM +0800, Jerry-ch Chen wrote:
> > From: Jerry-ch Chen <jerry-ch.chen@mediatek.com>
> >
> > This patch adds the driver of Face Detection (FD) unit in
> > Mediatek camera system, providing face detection function.
> >
> > The mtk-isp directory will contain drivers for multiple IP
> > blocks found in Mediatek ISP system. It will include ISP Pass 1
> > driver (CAM), sensor interface driver, DIP driver and face
> > detection driver.
> >
> > Signed-off-by: Jerry-ch Chen <jerry-ch.chen@mediatek.com>
> > ---
> > drivers/media/platform/Kconfig | 2 +
> > drivers/media/platform/Makefile | 2 +
> > drivers/media/platform/mtk-isp/fd/Kconfig | 19 +
> > drivers/media/platform/mtk-isp/fd/Makefile | 5 +
> > drivers/media/platform/mtk-isp/fd/mtk_fd.h | 149 ++
> > drivers/media/platform/mtk-isp/fd/mtk_fd_40.c | 1279 +++++++++++++++++
> > include/uapi/linux/v4l2-controls.h | 4 +
> > include/uapi/linux/videodev2.h | 3 +
> > 8 files changed, 1463 insertions(+)
> > create mode 100644 drivers/media/platform/mtk-isp/fd/Kconfig
> > create mode 100644 drivers/media/platform/mtk-isp/fd/Makefile
> > create mode 100644 drivers/media/platform/mtk-isp/fd/mtk_fd.h
> > create mode 100644 drivers/media/platform/mtk-isp/fd/mtk_fd_40.c
> >
>
> Thank you for the patch. Please see my comments inline. Really sorry for
> the much delayed review.
>
I appreciate your comments, here's the reply
> [snip]
> > +config VIDEO_MEDIATEK_FD
> > + tristate "Mediatek face detection processing function"
> > + depends on VIDEO_V4L2
> > + depends on ARCH_MEDIATEK
> > + select VIDEOBUF2_DMA_CONTIG
> > + select VIDEOBUF2_CORE
> > + select VIDEOBUF2_V4L2
> > + select VIDEOBUF2_MEMOPS
> > + select MEDIA_CONTROLLER
> > + select MTK_SCP
> > +
> > + default n
> > + help
> > + Support the Face Detection (FD) feature in the Mediatek
> > + mt8183 Soc.
>
> MT8183?
Fixed.
>
> > +
> > + FD driver is a V4L2 memory-to-memory device driver which
> > + provides hardware accelerated face detection function,
> > + it can detect different sizes of faces in a raw image.
>
> A YUV image I guess?
Fixed
>
> [snip]
> > diff --git a/drivers/media/platform/mtk-isp/fd/mtk_fd.h b/drivers/media/platform/mtk-isp/fd/mtk_fd.h
> > new file mode 100644
> > index 000000000000..d85bdcb70d6d
> > --- /dev/null
> > +++ b/drivers/media/platform/mtk-isp/fd/mtk_fd.h
> > @@ -0,0 +1,149 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +//
> > +// Copyright (c) 2018 MediaTek Inc.
> > +
> > +#ifndef __MTK_FD_HW_H__
> > +#define __MTK_FD_HW_H__
> > +
> > +#include <linux/completion.h>
> > +#include <linux/io.h>
> > +#include <linux/types.h>
> > +#include <linux/platform_device.h>
> > +#include <media/v4l2-ctrls.h>
> > +#include <media/v4l2-device.h>
> > +#include <media/videobuf2-v4l2.h>
> > +
> > +#define MTK_FD_OUTPUT_MIN_WIDTH 26U
> > +#define MTK_FD_OUTPUT_MIN_HEIGHT 26U
> > +#define MTK_FD_OUTPUT_MAX_WIDTH 640U
> > +#define MTK_FD_OUTPUT_MAX_HEIGHT 480U
> > +
> > +#define MTK_FD_HW_FMT_VYUY 2
> > +#define MTK_FD_HW_FMT_UYVY 3
> > +#define MTK_FD_HW_FMT_YVYU 4
> > +#define MTK_FD_HW_FMT_YUYV 5
> > +#define MTK_FD_HW_FMT_YVU_2P 6
> > +#define MTK_FD_HW_FMT_YUV_2P 7
> > +#define MTK_FD_HW_FMT_UNKNOWN 8
>
> What is an unknown format?
It's used in translate v4l2 pixel format to the value that FD HW
recognized. But I think the case of UNKNWOWN shouldn't happened.
I will remove it
>
> > +
> > +#define MTK_FD_IPI_CMD_INIT 0
> > +#define MTK_FD_IPI_CMD_INIT_ACK 1
> > +#define MTK_FD_IPI_CMD_ENQUEUE 2
> > +#define MTK_FD_IPI_CMD_ENQ_ACK 3
> > +#define MTK_FD_IPI_CMD_EXIT 4
> > +#define MTK_FD_IPI_CMD_EXIT_ACK 5
> > +#define MTK_FD_IPI_CMD_RESET 6
> > +#define MTK_FD_IPI_CMD_RESET_ACK 7
> > +
> > +#define MTK_FD_REG_OFFSET_HW_ENABLE 0x4
> > +#define MTK_FD_REG_OFFSET_INT_EN 0x15c
> > +#define MTK_FD_REG_OFFSET_INT_VAL 0x168
> > +#define MTK_FD_REG_OFFSET_RESULT 0x178
> > +
> > +#define MTK_FD_SET_HW_ENABLE 0x111
> > +#define MTK_FD_RS_BUF_SIZE 2289664
> > +#define MTK_FD_HW_WORK_BUF_SIZE 0x100000
>
> How about using the SZ_1M?
>
yes, fixed.
> > +#define MTK_FD_MAX_SPEEDUP 7
> > +#define MTK_FD_MAX_RESULT_NUM 1026
>
> Perhaps NUM_RESULTS?
>
fixed as MTK_FD_MAX_NUM_RESULT.
> > +
> > +/* Max scale size counts */
> > +#define MTK_FD_SCALE_ARR_NUM 15
>
> Perhaps NUM_SCALE_SIZES?
>
fixed as MTK_FD_NUM_SCALE_SIZE
> > +
> > +#define MTK_FD_HW_TIMEOUT 1000
>
> What's the unit?
it's in milliseconds, I would like to rename it to
MTK_FD_HW_TIMEOUT_MSEC
>
> > +
> > +enum face_angle {
> > + MTK_FD_FACE_FRONT,
> > + MTK_FD_FACE_RIGHT_50,
> > + MTK_FD_FACE_LEFT_50,
> > + MTK_FD_FACE_RIGHT_90,
> > + MTK_FD_FACE_LEFT_90,
> > + MTK_FD_FACE_ANGLE_NUM,
> > +};
>
> This enum seems to define values for the V4L2_CID_MTK_FD_DETECT_POSE
> control. Considering that this is an enumeration and the values are
> actually integers (-90, -50, 0, 50, 90), perhaps this should be an
> INTEGER_MENU control instead?
>
this ioctl let user select multiple face positions(combination of angles
and directions) to be detected. so I thought I am not able to use the
INTEGER_MENU for this purpose.
A bit-field as following should be used by user.
I consider adding it to uapi.
struct face_direction_def {
__u16 MTK_FD_FACE_DIR_0 : 1,
MTK_FD_FACE_DIR_30 : 1,
MTK_FD_FACE_DIR_60 : 1,
MTK_FD_FACE_DIR_90 : 1,
MTK_FD_FACE_DIR_120 : 1,
MTK_FD_FACE_DIR_150 : 1,
MTK_FD_FACE_DIR_180 : 1,
MTK_FD_FACE_DIR_210 : 1,
MTK_FD_FACE_DIR_240 : 1,
MTK_FD_FACE_DIR_270 : 1,
MTK_FD_FACE_DIR_300 : 1,
MTK_FD_FACE_DIR_330 : 1,
: 4;
};
User can also select some face directions of each face angle in one
ioctl, for example:
/*
* u16 face_directions[MTK_FD_FACE_ANGLE_NUM] = {0};
*
* face_directions[MTK_FD_FACE_FRONT] = 0x7; //angle:0, dir:0,30,60
* face_directions[MTK_FACE_RIGHT_50] = 0x2; //angle:50, dir:30
*
*/
> > +
> > +struct fd_buffer {
> > + __u32 scp_addr; /* used by SCP */
> > + __u32 dma_addr; /* used by DMA HW */
> > +} __packed;
fd buffer is used for scp ipi
> > +
> > +struct fd_face_result {
> > + char data[16];
> > +};
fd_face_result is used for user, so it should be moved to
include/uapi/linux.
In fact, it has bit-field definition for user, so I would like to define
it in include/uapi/linux as following:
struct fd_face_result {
__u64 face_idx : 12,
type : 1,
x0 : 10,
y0 : 10,
x1 : 10,
y1 : 10,
fcv1 : 11;
__u64 fcv2 : 7,
rip_dir : 4,
rop_dir : 3,
det_size : 5;
};
> > +
> > +struct fd_user_output {
> > + struct fd_face_result results[MTK_FD_MAX_RESULT_NUM];
> > + __u16 number;
>
> Is this perhaps the number of results? If so, would num_results be a better
> name?
>
yes, fixed.
> > +};
>
> Since this struct is the meta buffer format, it is a part of the userspace
> interface and should be defined in a header under include/uapi/linux/.
>
Ok, I will create include/uapi/linux/mtk_fd_40.h
which suppose to include structures that userspace will use.
should the private IOCTLs be placed in it together?
> > +
> > +struct user_param {
> > + u8 fd_speedup;
> > + u8 fd_extra_model;
> > + u8 scale_img_num;
> > + u8 src_img_fmt;
> > + __u16 scale_img_width[MTK_FD_SCALE_ARR_NUM];
> > + __u16 scale_img_height[MTK_FD_SCALE_ARR_NUM];
> > + __u16 face_directions[MTK_FD_FACE_ANGLE_NUM];
>
> Is this a user-facing definition or an interface between the kernel driver
> and firmware? If the latter, the __ types shouldn't be used.
>
It's the later, I'll fix it.
> > +} __packed;
> > +
> > +struct fd_init_param {
> > + struct fd_buffer fd_manager;
> > + __u32 rs_dma_addr;
>
> Ditto.
>
done.
> > +} __packed;
> > +
> > +struct fd_enq_param {
> > + __u64 output_vaddr;
>
> Ditto.
done.
>
> > + struct fd_buffer src_img[2];
> > + struct fd_buffer user_result;
> > + struct user_param user_param;
> > +} __packed;
> > +
> > +struct fd_ack_param {
> > + __u32 ret_code;
> > + __u32 ret_msg;
>
> Ditto.
done.
>
> > +} __packed;
> [snip]
> > +/* */
>
> Was there supposed to be a comment here? :)
Ooops, no, it should be removed.
>
> > +static int mtk_fd_hw_alloc_rs_dma_addr(struct mtk_fd_dev *fd)
> > +{
> > + struct device *dev = fd->dev;
> > + void *va;
> > + dma_addr_t dma_handle;
> > +
> > + va = dma_alloc_coherent(dev, MTK_FD_RS_BUF_SIZE, &dma_handle,
> > + GFP_KERNEL);
> > + if (!va) {
> > + dev_err(dev, "dma_alloc null va\n");
>
> No need to print errors for memory allocation failures, because one will be
> printed automatically.
Ok, log removed.
>
> [snip]
> > +static int mtk_fd_hw_connect(struct mtk_fd_dev *fd)
> > +{
> > + int ret;
> > +
> > + ret = rproc_boot(fd->rproc_handle);
> > +
>
> nit: Unnecessary blank line.
>
fixed.
> [snip]
> > +static int mtk_fd_vb2_queue_setup(struct vb2_queue *vq,
> > + unsigned int *num_buffers,
> > + unsigned int *num_planes,
> > + unsigned int sizes[],
> > + struct device *alloc_devs[])
> > +{
> > + struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vq);
> > + unsigned int size[2];
> > + unsigned int plane;
> > +
> > + switch (vq->type) {
> > + case V4L2_BUF_TYPE_META_CAPTURE:
> > + size[0] = ctx->dst_fmt.buffersize;
> > + break;
> > + case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
> > + size[0] = ctx->src_fmt.plane_fmt[0].sizeimage;
> > + if (*num_planes == 2)
> > + size[1] = ctx->src_fmt.plane_fmt[1].sizeimage;
> > + break;
> > + }
>
> Is this code above needed? The code below sets sizes[] and it uses a for loop,
> without opencoded assignment for the second plane.
>
Looks like not really useful here,
it should check sizes and num_planes if num_plane not zero,
and for V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, it will at most have 2
planes, maybe no need for loop as well.
I will refine this function as following:
mtk_fd_vb2_queue_setup(...)
{
struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vq);
if (*num_planes == 0) {
if (vq->type == V4L2_BUF_TYPE_META_CAPTURE) {
sizes[0] = ctx->dst_fmt.buffersize;
*num_planes = 1;
return 0;
} else if (vq->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
*num_planes = ctx->src_fmt.num_planes;
sizes[0] = ctx->src_fmt.plane_fmt[0].sizeimage;
if (*num_planes == 2)
sizes[1] = ctx->src_fmt.plane_fmt[1].sizeimage;
return 0;
}
return -EINVAL;
}
/* If num_plane not zero, check the num_plane and sizes*/
if (vq->type == V4L2_BUF_TYPE_META_CAPTURE) {
if ((*num_planes == 1) &&
(sizes[0] <= ctx->dst_fmt.buffersize))
return 0;
else
return -EINVAL;
}
if (vq->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
if ((*num_planes == 1) &&
(sizes[0] <= ctx->src_fmt.plane_fmt[0].sizeimage))
return 0;
else if ((*num_planes == 2) &&
(sizes[0] <= ctx->src_fmt.plane_fmt[0].sizeimage) &&
(sizes[1] <= ctx->src_fmt.plane_fmt[1].sizeimage))
return 0;
else
return -EINVAL;
}
return 0;
}
> > +
> > + if (*num_planes > 2)
> > + return -EINVAL;
> > + if (*num_planes == 0) {
> > + if (vq->type == V4L2_BUF_TYPE_META_CAPTURE) {
> > + sizes[0] = ctx->dst_fmt.buffersize;
> > + *num_planes = 1;
> > + return 0;
> > + }
> > +
> > + *num_planes = ctx->src_fmt.num_planes;
> > + for (plane = 0; plane < *num_planes; plane++)
> > + sizes[plane] = ctx->src_fmt.plane_fmt[plane].sizeimage;
> > + return 0;
> > + }
> > +
> > + for (plane = 0; plane < *num_planes; plane++) {
> > + if (sizes[plane] < size[plane])
> > + return -EINVAL;
> > + }
> > + return 0;
> > +}
> [snip]
> > +static int mtk_fd_enum_fmt_out_mp(struct file *file, void *fh,
> > + struct v4l2_fmtdesc *f)
> > +{
> > + if (f->index >= NUM_FORMATS)
>
> Please use ARRAY_SIZE().
fixed.
>
> > + return -EINVAL;
> > +
> > + f->pixelformat = mtk_fd_img_fmts[f->index].pixelformat;
> > + return 0;
> > +}
> > +
> > +static void mtk_fd_fill_pixfmt_mp(struct v4l2_pix_format_mplane *dfmt,
> > + const struct v4l2_pix_format_mplane *sfmt)
> > +{
> > + dfmt->field = V4L2_FIELD_NONE;
> > + dfmt->colorspace = V4L2_COLORSPACE_BT2020;
> > + dfmt->num_planes = sfmt->num_planes;
> > + dfmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> > + dfmt->quantization = V4L2_QUANTIZATION_DEFAULT;
> > + dfmt->xfer_func =
> > + V4L2_MAP_XFER_FUNC_DEFAULT(dfmt->colorspace);
> > +
> > + /* Keep user setting as possible */
> > + dfmt->width = clamp(dfmt->width,
> > + MTK_FD_OUTPUT_MIN_WIDTH,
> > + MTK_FD_OUTPUT_MAX_WIDTH);
> > + dfmt->height = clamp(dfmt->height,
> > + MTK_FD_OUTPUT_MIN_HEIGHT,
> > + MTK_FD_OUTPUT_MAX_HEIGHT);
> > +
> > + if (sfmt->num_planes == 2) {
> > + /* NV16M and NV61M has 1 byte per pixel */
> > + dfmt->plane_fmt[0].bytesperline = dfmt->width;
> > + dfmt->plane_fmt[1].bytesperline = dfmt->width;
> > + } else {
> > + /* 2 bytes per pixel */
> > + dfmt->plane_fmt[0].bytesperline = dfmt->width * 2;
> > + }
> > +
> > + dfmt->plane_fmt[0].sizeimage =
> > + dfmt->height * dfmt->plane_fmt[0].bytesperline;
>
> Could some of the code above be replaced with v4l2_fill_pixfmt_mp()?
>
I would like to refine as following
mtk_fd_fill_pixfmt_mp(...){
v4l2_fill_pixfmt_mp(dfmt, sfmt->pixelformat, dfmt->width,
dfmt->height);
dfmt->field = V4L2_FIELD_NONE;
dfmt->colorspace = V4L2_COLORSPACE_BT2020;
dfmt->num_planes = sfmt->num_planes;
dfmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
dfmt->quantization = V4L2_QUANTIZATION_DEFAULT;
dfmt->xfer_func =
V4L2_MAP_XFER_FUNC_DEFAULT(dfmt->colorspace);
}
> > +}
> > +
> > +static const struct v4l2_pix_format_mplane *mtk_fd_find_fmt(u32 format)
> > +{
> > + unsigned int i;
> > + const struct v4l2_pix_format_mplane *dev_fmt;
> > +
> > + for (i = 0; i < NUM_FORMATS; i++) {
>
> Please use ARRAY_SIZE rather than a custom macro.
fixed.
>
> > + dev_fmt = &mtk_fd_img_fmts[i];
> > + if (dev_fmt->pixelformat == format)
> > + return dev_fmt;
> > + }
> > +
> > + return NULL;
> > +}
> > +
> > +static int mtk_fd_try_fmt_out_mp(struct file *file,
> > + void *fh,
> > + struct v4l2_format *f)
> > +{
> > + struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp;
> > + const struct v4l2_pix_format_mplane *fmt;
> > +
> > + fmt = mtk_fd_find_fmt(pix_mp->pixelformat);
> > + if (!fmt)
> > + fmt = &mtk_fd_img_fmts[0]; /* Get default img fmt */
>
> nit: Please move the comment to a separate line and add braces.
>
fixed.
> > +
> > + mtk_fd_fill_pixfmt_mp(pix_mp, fmt);
> > + return 0;
> > +}
> [snip]
> > +static unsigned int get_fd_img_fmt(unsigned int fourcc)
> > +{
> > + switch (fourcc) {
> > + case V4L2_PIX_FMT_VYUY:
> > + return MTK_FD_HW_FMT_VYUY;
> > + case V4L2_PIX_FMT_YUYV:
> > + return MTK_FD_HW_FMT_YUYV;
> > + case V4L2_PIX_FMT_YVYU:
> > + return MTK_FD_HW_FMT_YVYU;
> > + case V4L2_PIX_FMT_UYVY:
> > + return MTK_FD_HW_FMT_UYVY;
> > + case V4L2_PIX_FMT_NV16M:
> > + return MTK_FD_HW_FMT_YUV_2P;
> > + case V4L2_PIX_FMT_NV61M:
> > + return MTK_FD_HW_FMT_YVU_2P;
> > + default:
> > + return MTK_FD_HW_FMT_UNKNOWN;
>
> If we want to be paranoid, we can just add a WARN here and return 0.
> Shouldn't be a need to define an unknown format.
Ok, I will refine as following:
default:
dev_warn(dev, "%s: Unsupported V4L2_PIX_FMT\n", __func__);
return 0;
>
> [snip]
> > +static void mtk_fd_fill_user_param(struct user_param *user_param,
> > + struct v4l2_ctrl_handler *hdl)
> > +{
> > + struct v4l2_ctrl *ctrl;
> > + int i;
> > +
> > + ctrl = v4l2_ctrl_find(hdl, V4L2_CID_MTK_FD_SCALE_DOWN_IMG_WIDTH);
> > + if (ctrl)
> > + for (i = 0; i < ctrl->elems; i++)
> > + user_param->scale_img_width[i] = ctrl->p_new.p_u16[i];
> > + ctrl = v4l2_ctrl_find(hdl, V4L2_CID_MTK_FD_SCALE_DOWN_IMG_HEIGHT);
> > + if (ctrl)
> > + for (i = 0; i < ctrl->elems; i++)
> > + user_param->scale_img_height[i] = ctrl->p_new.p_u16[i];
> > + ctrl = v4l2_ctrl_find(hdl, V4L2_CID_MTK_FD_SCALE_IMG_NUM);
> > + if (ctrl)
> > + user_param->scale_img_num = ctrl->val;
> > +
>
> nit: Either separate the code dealing with all controls from each other, or
> none.
>
Fixed.
> > + ctrl = v4l2_ctrl_find(hdl, V4L2_CID_MTK_FD_DETECT_POSE);
> > + if (ctrl)
> > + for (i = 0; i < ctrl->elems; i++)
> > + user_param->face_directions[i] = ctrl->p_new.p_u16[i];
> > + ctrl = v4l2_ctrl_find(hdl, V4L2_CID_MTK_FD_DETECT_SPEED);
> > + if (ctrl)
> > + user_param->fd_speedup = ctrl->val;
> > + ctrl = v4l2_ctrl_find(hdl, V4L2_CID_MTK_FD_DETECTION_MODEL);
> > + if (ctrl)
> > + user_param->fd_extra_model = ctrl->val;
> > +}
> > +
> > +static void mtk_fd_device_run(void *priv)
> > +{
> > + struct mtk_fd_ctx *ctx = priv;
> > + struct mtk_fd_dev *fd = ctx->fd_dev;
> > + struct vb2_v4l2_buffer *src_buf, *dst_buf;
> > + struct fd_enq_param fd_param;
> > + void *plane_vaddr;
> > +
> > + src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
> > + dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
> > +
> > + fd_param.src_img[0].dma_addr =
> > + vb2_dma_contig_plane_dma_addr(&src_buf->vb2_buf, 0);
> > + fd_param.user_result.dma_addr =
> > + vb2_dma_contig_plane_dma_addr(&dst_buf->vb2_buf, 0);
> > + plane_vaddr = vb2_plane_vaddr(&dst_buf->vb2_buf, 0);
> > + fd_param.output_vaddr = (u64)(unsigned long)plane_vaddr;
>
> Why is the vaddr needed here? Specfically, it could pose a security problem
> if CPU virtual addresses are exchanged with firmware.
>
This should be removed... SCP side as well.
> > + fd_param.user_param.src_img_fmt =
> > + get_fd_img_fmt(ctx->src_fmt.pixelformat);
> > + if (ctx->src_fmt.num_planes == 2)
> > + fd_param.src_img[1].dma_addr =
> > + vb2_dma_contig_plane_dma_addr(&src_buf->vb2_buf, 1);
>
> nit: Could this be moved above, to be just below src_img[0] initialization,
> for readability reasons?
>
Ok, this function will be refined as
static void mtk_fd_device_run(void *priv)
{
struct mtk_fd_ctx *ctx = priv;
struct mtk_fd_dev *fd = ctx->fd_dev;
struct vb2_v4l2_buffer *src_buf, *dst_buf;
struct fd_enq_param fd_param;
src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
fd_param.src_img[0].dma_addr =
vb2_dma_contig_plane_dma_addr(&src_buf->vb2_buf, 0);
if (ctx->src_fmt.num_planes == 2)
fd_param.src_img[1].dma_addr =
vb2_dma_contig_plane_dma_addr(&src_buf->vb2_buf, 1);
fd_param.user_result.dma_addr =
vb2_dma_contig_plane_dma_addr(&dst_buf->vb2_buf, 0);
fd_param.user_param.src_img_fmt =
get_fd_img_fmt(fd->dev, ctx->src_fmt.pixelformat);
mtk_fd_fill_user_param(&fd_param.user_param, &ctx->hdl);
/* Complete request controls if any */
v4l2_ctrl_request_complete(src_buf->vb2_buf.req_obj.req, &ctx->hdl);
fd->output = vb2_plane_vaddr(&dst_buf->vb2_buf, 0);
mtk_fd_hw_job_exec(fd, &fd_param);
}
> > + mtk_fd_fill_user_param(&fd_param.user_param, &ctx->hdl);
> > +
> > + /* Complete request controls if any */
> > + v4l2_ctrl_request_complete(src_buf->vb2_buf.req_obj.req, &ctx->hdl);
> > +
> > + fd->output = plane_vaddr;
> > + mtk_fd_hw_job_exec(fd, &fd_param);
> > +}
> > +
> > +static struct v4l2_m2m_ops fd_m2m_ops = {
> > + .device_run = mtk_fd_device_run,
> > +};
> > +
> > +static const struct media_device_ops fd_m2m_media_ops = {
> > + .req_validate = vb2_request_validate,
> > + .req_queue = v4l2_m2m_request_queue,
> > +};
> > +
> > +static int mtk_fd_video_device_register(struct mtk_fd_dev *fd)
> > +{
> > + struct video_device *vfd = &fd->vfd;
> > + struct v4l2_m2m_dev *m2m_dev = fd->m2m_dev;
> > + struct device *dev = fd->dev;
> > + int ret;
> > +
> > + vfd->fops = &fd_video_fops;
> > + vfd->release = video_device_release;
> > + vfd->lock = &fd->vfd_lock;
> > + vfd->v4l2_dev = &fd->v4l2_dev;
> > + vfd->vfl_dir = VFL_DIR_M2M;
> > + vfd->device_caps = V4L2_CAP_STREAMING | V4L2_CAP_VIDEO_OUTPUT_MPLANE |
> > + V4L2_CAP_META_CAPTURE;
> > + vfd->ioctl_ops = &mtk_fd_v4l2_video_out_ioctl_ops;
> > +
> > + strscpy(vfd->name, dev_driver_string(dev), sizeof(vfd->name));
> > +
> > + video_set_drvdata(vfd, fd);
> > +
> > + ret = video_register_device(vfd, VFL_TYPE_GRABBER, 0);
> > + if (ret) {
> > + dev_err(dev, "Failed to register video device\n");
> > + goto err_free_dev;
> > + }
> > +
> > + ret = v4l2_m2m_register_media_controller(m2m_dev, vfd,
> > + MEDIA_ENT_F_PROC_VIDEO_STATISTICS);
> > + if (ret) {
> > + dev_err(dev, "Failed to init mem2mem media controller\n");
> > + goto err_unreg_video;
> > + }
> > + return 0;
> > +
> > +err_unreg_video:
> > + video_unregister_device(vfd);
> > +err_free_dev:
> > + video_device_release(vfd);
> > + return ret;
> > +}
> > +
> > +static int mtk_fd_dev_v4l2_init(struct mtk_fd_dev *fd)
> > +{
> > + struct media_device *mdev = &fd->mdev;
> > + struct device *dev = fd->dev;
> > + int ret;
> > +
> > + ret = v4l2_device_register(dev, &fd->v4l2_dev);
> > + if (ret) {
> > + dev_err(dev, "Failed to register v4l2 device\n");
> > + return ret;
> > + }
> > +
> > + fd->m2m_dev = v4l2_m2m_init(&fd_m2m_ops);
> > + if (IS_ERR(fd->m2m_dev)) {
> > + dev_err(dev, "Failed to init mem2mem device\n");
> > + ret = PTR_ERR(fd->m2m_dev);
> > + goto err_unreg_v4l2_dev;
> > + }
> > +
> > + mdev->dev = dev;
> > + strscpy(mdev->model, dev_driver_string(dev), sizeof(mdev->model));
> > + snprintf(mdev->bus_info, sizeof(mdev->bus_info),
> > + "platform:%s", dev_name(dev));
> > + media_device_init(mdev);
> > + mdev->ops = &fd_m2m_media_ops;
> > + fd->v4l2_dev.mdev = mdev;
> > +
> > + ret = mtk_fd_video_device_register(fd);
> > + if (ret) {
> > + dev_err(dev, "Failed to register video device\n");
> > + goto err_cleanup_mdev;
> > + }
> > +
> > + ret = media_device_register(mdev);
> > + if (ret) {
> > + dev_err(dev, "Failed to register mem2mem media device\n");
> > + goto err_unreg_vdev;
> > + }
> > +
> > + return 0;
> > +
> > +err_unreg_vdev:
> > + v4l2_m2m_unregister_media_controller(fd->m2m_dev);
> > + video_unregister_device(&fd->vfd);
> > + video_device_release(&fd->vfd);
>
> This is inconsistent with the registration, which registers all of these in
> a function. Perhaps it would be worth to move this cleanup to a
> mtk_fd_video_device_unregister() function?
>
Fixed.
> > +err_cleanup_mdev:
> > + media_device_cleanup(mdev);
> > + v4l2_m2m_release(fd->m2m_dev);
> > +err_unreg_v4l2_dev:
> > + v4l2_device_unregister(&fd->v4l2_dev);
> > + return ret;
> > +}
> > +
> > +static void mtk_fd_dev_v4l2_release(struct mtk_fd_dev *fd)
> > +{
> > + v4l2_m2m_unregister_media_controller(fd->m2m_dev);
> > + video_unregister_device(&fd->vfd);
> > + video_device_release(&fd->vfd);
> > + media_device_cleanup(&fd->mdev);
> > + v4l2_m2m_release(fd->m2m_dev);
> > + v4l2_device_unregister(&fd->v4l2_dev);
> > +}
> > +
> > +static irqreturn_t mtk_fd_irq(int irq, void *data)
> > +{
> > + struct mtk_fd_dev *fd = (struct mtk_fd_dev *)data;
> > +
> > + /* must read this register otherwise HW will keep sending irq */
> > + readl(fd->fd_base + MTK_FD_REG_OFFSET_INT_VAL);
> > + fd->output->number = readl(fd->fd_base + MTK_FD_REG_OFFSET_RESULT);
> > + dev_dbg(fd->dev, "mtk_fd_face_num:%d\n", fd->output->number);
> > +
> > + mtk_fd_hw_done(fd, VB2_BUF_STATE_DONE);
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static int mtk_fd_hw_get_scp_mem(struct mtk_fd_dev *fd)
> > +{
> > + struct device *dev = fd->dev;
> > + dma_addr_t addr;
> > + void *ptr;
> > + u32 ret;
> > +
> > + /*
> > + * Allocate coherent reserved memory for SCP firmware usage.
> > + * The size of SCP composer's memory is fixed to 0x100000
> > + * for the requirement of firmware.
> > + */
> > + ptr = dma_alloc_coherent(&fd->scp_pdev->dev,
> > + MTK_FD_HW_WORK_BUF_SIZE, &addr, GFP_KERNEL);
> > + if (!ptr)
> > + return -ENOMEM;
> > +
> > + fd->scp_mem.scp_addr = addr;
> > + fd->scp_mem_virt_addr = ptr;
> > + dev_info(dev, "scp addr:%pad va:%pK\n", &addr, ptr);
>
> These addresses are nothing a user should be concerned about, so please
> don't use the _info level here. If you think this is an important debugging
> information, please make it dev_dbg(). Otherwise, please just remove.
>
Removed.
> > +
> > + /*
> > + * This reserved memory is also be used by FD HW.
> > + * Need to get iova address for FD DMA.
> > + */
> > + addr = dma_map_resource(dev, addr, MTK_FD_HW_WORK_BUF_SIZE,
> > + DMA_TO_DEVICE, DMA_ATTR_SKIP_CPU_SYNC);
>
> Note that the second argument to dma_map_resource() is phys_addr_t, but the
> code above passes a DMA address. This works only by luck, because both
> physical and SCP DMA address space have the same addresses. To be fully
> correct, dma_map_single() needs to be used, with the kernel virtual address
> passed to it, but currently it doesn't handle the memory from a reserved
> pool. Thus, we can only keep the hack as is, but please add a comment
> explaining it, e.g.
>
> /*
> * FIXME: Assume SCP DMA and physical addresses are the same until
> * dma_map_single() is fixed to handle reserved memory allocations.
> */
>
Ok, I've insert it above the comments there.
as following:
/*
* FIXME: Assume SCP DMA and physical addresses are the same until
* dma_map_single() is fixed to handle reserved memory allocations.
*
* This reserved memory will also be used by FD HW.
* Need to get iova address for FD DMA.
*/
> > + if (dma_mapping_error(dev, addr)) {
> > + dev_err(dev, "Failed to map scp iova\n");
> > + ret = -ENOMEM;
> > + goto fail_free_mem;
> > + }
> > + fd->scp_mem.dma_addr = addr;
> > + dev_info(dev, "scp iova addr:%pad\n", &addr);
>
> Ditto.
fixed.
>
> Best regards,
> Tomasz
Thanks and best regards,
Jerry
^ permalink raw reply
* Re: [PATCH 09/12] dt-bindings: arm: fsl: Add msi-map device-tree binding for fsl-mc bus
From: Rob Herring @ 2020-05-22 14:08 UTC (permalink / raw)
To: Diana Craciun OSS
Cc: Robin Murphy, Lorenzo Pieralisi, devicetree, Catalin Marinas,
Will Deacon, PCI, Sudeep Holla, Rafael J. Wysocki,
Makarand Pawagi, linux-acpi, Linux IOMMU, Marc Zyngier,
Hanjun Guo, Bjorn Helgaas,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <ee7a5c04-814e-215f-ec74-52c2f3b881d0@oss.nxp.com>
On Fri, May 22, 2020 at 3:57 AM Diana Craciun OSS
<diana.craciun@oss.nxp.com> wrote:
>
> On 5/22/2020 12:42 PM, Robin Murphy wrote:
> > On 2020-05-22 00:10, Rob Herring wrote:
> >> On Thu, May 21, 2020 at 7:00 AM Lorenzo Pieralisi
> >> <lorenzo.pieralisi@arm.com> wrote:
> >>>
> >>> From: Laurentiu Tudor <laurentiu.tudor@nxp.com>
> >>>
> >>> The existing bindings cannot be used to specify the relationship
> >>> between fsl-mc devices and GIC ITSes.
> >>>
> >>> Add a generic binding for mapping fsl-mc devices to GIC ITSes, using
> >>> msi-map property.
> >>>
> >>> Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
> >>> Cc: Rob Herring <robh+dt@kernel.org>
> >>> ---
> >>> .../devicetree/bindings/misc/fsl,qoriq-mc.txt | 30
> >>> +++++++++++++++++--
> >>> 1 file changed, 27 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> >>> b/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> >>> index 9134e9bcca56..b0813b2d0493 100644
> >>> --- a/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> >>> +++ b/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> >>> @@ -18,9 +18,9 @@ same hardware "isolation context" and a 10-bit
> >>> value called an ICID
> >>> the requester.
> >>>
> >>> The generic 'iommus' property is insufficient to describe the
> >>> relationship
> >>> -between ICIDs and IOMMUs, so an iommu-map property is used to define
> >>> -the set of possible ICIDs under a root DPRC and how they map to
> >>> -an IOMMU.
> >>> +between ICIDs and IOMMUs, so the iommu-map and msi-map properties
> >>> are used
> >>> +to define the set of possible ICIDs under a root DPRC and how they
> >>> map to
> >>> +an IOMMU and a GIC ITS respectively.
> >>>
> >>> For generic IOMMU bindings, see
> >>> Documentation/devicetree/bindings/iommu/iommu.txt.
> >>> @@ -28,6 +28,9 @@ Documentation/devicetree/bindings/iommu/iommu.txt.
> >>> For arm-smmu binding, see:
> >>> Documentation/devicetree/bindings/iommu/arm,smmu.yaml.
> >>>
> >>> +For GICv3 and GIC ITS bindings, see:
> >>> +Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.yaml.
> >>>
> >>> +
> >>> Required properties:
> >>>
> >>> - compatible
> >>> @@ -119,6 +122,15 @@ Optional properties:
> >>> associated with the listed IOMMU, with the iommu-specifier
> >>> (i - icid-base + iommu-base).
> >>>
> >>> +- msi-map: Maps an ICID to a GIC ITS and associated iommu-specifier
> >>> + data.
> >>> +
> >>> + The property is an arbitrary number of tuples of
> >>> + (icid-base,iommu,iommu-base,length).
> >>
> >> I'm confused because the example has GIC ITS phandle, not an IOMMU.
> >>
> >> What is an iommu-base?
> >
> > Right, I was already halfway through writing a reply to say that all
> > the copy-pasted "iommu" references here should be using the
> > terminology from the pci-msi.txt binding instead.
>
> Right, will change it.
>
> >
> >>> +
> >>> + Any ICID in the interval [icid-base, icid-base + length) is
> >>> + associated with the listed GIC ITS, with the iommu-specifier
> >>> + (i - icid-base + iommu-base).
> >>> Example:
> >>>
> >>> smmu: iommu@5000000 {
> >>> @@ -128,6 +140,16 @@ Example:
> >>> ...
> >>> };
> >>>
> >>> + gic: interrupt-controller@6000000 {
> >>> + compatible = "arm,gic-v3";
> >>> + ...
> >>> + its: gic-its@6020000 {
> >>> + compatible = "arm,gic-v3-its";
> >>> + msi-controller;
> >>> + ...
> >>> + };
> >>> + };
> >>> +
> >>> fsl_mc: fsl-mc@80c000000 {
> >>> compatible = "fsl,qoriq-mc";
> >>> reg = <0x00000008 0x0c000000 0 0x40>, /* MC
> >>> portal base */
> >>> @@ -135,6 +157,8 @@ Example:
> >>> msi-parent = <&its>;
> >
> > Side note: is it right to keep msi-parent here? It rather implies that
> > the MC itself has a 'native' Device ID rather than an ICID, which I
> > believe is not strictly true. Plus it's extra-confusing that it
> > doesn't specify an ID either way, since that makes it look like the
> > legacy PCI case that gets treated implicitly as an identity msi-map,
> > which makes no sense at all to combine with an actual msi-map.
>
> Before adding msi-map, the fsl-mc code assumed that ICID and streamID
> are equal and used msi-parent just to get the reference to the ITS node.
> Removing msi-parent will break the backward compatibility of the already
> existing systems. Maybe we should mention that this is legacy and not to
> be used for newer device trees.
If ids are 1:1, then the DT should use msi-parent. If there is
remapping, then use msi-map. A given system should use one or the
other. I suppose if some ids are 1:1 and the msi-map was added to add
additional support for ids not 1:1, then you could end up with both.
That's fine in dts files, but examples should reflect the 'right' way.
Rob
^ permalink raw reply
* Re: [PATCH 09/12] dt-bindings: arm: fsl: Add msi-map device-tree binding for fsl-mc bus
From: Rob Herring @ 2020-05-22 14:02 UTC (permalink / raw)
To: Robin Murphy
Cc: Lorenzo Pieralisi, devicetree, Catalin Marinas, Will Deacon,
Diana Craciun, PCI, Sudeep Holla, Rafael J. Wysocki,
Makarand Pawagi, linux-acpi, Linux IOMMU, Marc Zyngier,
Hanjun Guo, Bjorn Helgaas,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <abca6ecb-5d93-832f-ff7c-de53bb6203f3@arm.com>
On Fri, May 22, 2020 at 3:42 AM Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 2020-05-22 00:10, Rob Herring wrote:
> > On Thu, May 21, 2020 at 7:00 AM Lorenzo Pieralisi
> > <lorenzo.pieralisi@arm.com> wrote:
> >>
> >> From: Laurentiu Tudor <laurentiu.tudor@nxp.com>
> >>
> >> The existing bindings cannot be used to specify the relationship
> >> between fsl-mc devices and GIC ITSes.
> >>
> >> Add a generic binding for mapping fsl-mc devices to GIC ITSes, using
> >> msi-map property.
> >>
> >> Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
> >> Cc: Rob Herring <robh+dt@kernel.org>
> >> ---
> >> .../devicetree/bindings/misc/fsl,qoriq-mc.txt | 30 +++++++++++++++++--
> >> 1 file changed, 27 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt b/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> >> index 9134e9bcca56..b0813b2d0493 100644
> >> --- a/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> >> +++ b/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> >> @@ -18,9 +18,9 @@ same hardware "isolation context" and a 10-bit value called an ICID
> >> the requester.
> >>
> >> The generic 'iommus' property is insufficient to describe the relationship
> >> -between ICIDs and IOMMUs, so an iommu-map property is used to define
> >> -the set of possible ICIDs under a root DPRC and how they map to
> >> -an IOMMU.
> >> +between ICIDs and IOMMUs, so the iommu-map and msi-map properties are used
> >> +to define the set of possible ICIDs under a root DPRC and how they map to
> >> +an IOMMU and a GIC ITS respectively.
> >>
> >> For generic IOMMU bindings, see
> >> Documentation/devicetree/bindings/iommu/iommu.txt.
> >> @@ -28,6 +28,9 @@ Documentation/devicetree/bindings/iommu/iommu.txt.
> >> For arm-smmu binding, see:
> >> Documentation/devicetree/bindings/iommu/arm,smmu.yaml.
> >>
> >> +For GICv3 and GIC ITS bindings, see:
> >> +Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.yaml.
> >> +
> >> Required properties:
> >>
> >> - compatible
> >> @@ -119,6 +122,15 @@ Optional properties:
> >> associated with the listed IOMMU, with the iommu-specifier
> >> (i - icid-base + iommu-base).
> >>
> >> +- msi-map: Maps an ICID to a GIC ITS and associated iommu-specifier
> >> + data.
> >> +
> >> + The property is an arbitrary number of tuples of
> >> + (icid-base,iommu,iommu-base,length).
> >
> > I'm confused because the example has GIC ITS phandle, not an IOMMU.
> >
> > What is an iommu-base?
>
> Right, I was already halfway through writing a reply to say that all the
> copy-pasted "iommu" references here should be using the terminology from
> the pci-msi.txt binding instead.
>
> >> +
> >> + Any ICID in the interval [icid-base, icid-base + length) is
> >> + associated with the listed GIC ITS, with the iommu-specifier
> >> + (i - icid-base + iommu-base).
> >> Example:
> >>
> >> smmu: iommu@5000000 {
> >> @@ -128,6 +140,16 @@ Example:
> >> ...
> >> };
> >>
> >> + gic: interrupt-controller@6000000 {
> >> + compatible = "arm,gic-v3";
> >> + ...
> >> + its: gic-its@6020000 {
> >> + compatible = "arm,gic-v3-its";
> >> + msi-controller;
> >> + ...
> >> + };
> >> + };
> >> +
> >> fsl_mc: fsl-mc@80c000000 {
> >> compatible = "fsl,qoriq-mc";
> >> reg = <0x00000008 0x0c000000 0 0x40>, /* MC portal base */
> >> @@ -135,6 +157,8 @@ Example:
> >> msi-parent = <&its>;
>
> Side note: is it right to keep msi-parent here? It rather implies that
> the MC itself has a 'native' Device ID rather than an ICID, which I
> believe is not strictly true. Plus it's extra-confusing that it doesn't
> specify an ID either way, since that makes it look like the legacy PCI
> case that gets treated implicitly as an identity msi-map, which makes no
> sense at all to combine with an actual msi-map.
No, it doesn't make sense from a binding perspective.
>
> >> /* define map for ICIDs 23-64 */
> >> iommu-map = <23 &smmu 23 41>;
> >> + /* define msi map for ICIDs 23-64 */
> >> + msi-map = <23 &its 23 41>;
> >
> > Seeing 23 twice is odd. The numbers to the right of 'its' should be an
> > ITS number space.
>
> On about 99% of systems the values in the SMMU Stream ID and ITS Device
> ID spaces are going to be the same. Nobody's going to bother carrying
> *two* sets of sideband data across the interconnect if they don't have to ;)
I'm referring to the 23 on the left and right, not between the msi and
iommu. If the left and right are the same, then what are we remapping
exactly?
Rob
^ permalink raw reply
* Re: [PATCH v10 2/5] PCI: Add Loongson PCI Controller support
From: Lorenzo Pieralisi @ 2020-05-22 13:40 UTC (permalink / raw)
To: Jiaxun Yang
Cc: linux-pci, Rob Herring, Bjorn Helgaas, Rob Herring,
Thomas Bogendoerfer, Huacai Chen, Paul Burton, devicetree,
linux-kernel, linux-mips
In-Reply-To: <7FE3D498-D293-407C-A70C-5E763151477C@flygoat.com>
On Fri, May 22, 2020 at 09:32:10PM +0800, Jiaxun Yang wrote:
[...]
> >> Is it possible to let this series go into next tree soon?
> >>
> >> As LS7A dts patch would depend on this series, and I want to
> >> make the whole LS7A basic support as a part of 5.8 release.
> >
> >I think you have all necessary tags to take this in the MIPS
> >tree, please let me know if that's the way we want this to go
> >upstream - I would not pull MIPS/dts changes into the PCI tree
> >and I don't think it is needed for this series.
>
> Please pull all PCI changes [1~3] into PCI tree.
Pulled [1,2,3] into pci/mips, thanks !
Lorenzo
^ permalink raw reply
* Re: [PATCH v10 2/5] PCI: Add Loongson PCI Controller support
From: Jiaxun Yang @ 2020-05-22 13:32 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: linux-pci, Rob Herring, Bjorn Helgaas, Rob Herring,
Thomas Bogendoerfer, Huacai Chen, Paul Burton, devicetree,
linux-kernel, linux-mips
In-Reply-To: <20200522131018.GE11785@e121166-lin.cambridge.arm.com>
于 2020年5月22日 GMT+08:00 下午9:10:18, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> 写到:
>On Wed, May 20, 2020 at 07:57:29PM +0800, Jiaxun Yang wrote:
>>
>>
>> 于 2020年5月14日 GMT+08:00 下午9:16:38, Jiaxun Yang <jiaxun.yang@flygoat.com> 写到:
>> >This controller can be found on Loongson-2K SoC, Loongson-3
>> >systems with RS780E/LS7A PCH.
>> >
>> >The RS780E part of code was previously located at
>> >arch/mips/pci/ops-loongson3.c and now it can use generic PCI
>> >driver implementation.
>> >
>> >Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>> >Reviewed-by: Rob Herring <robh@kernel.org>
>> >
>>
>> Hi there,
>>
>> Is it possible to let this series go into next tree soon?
>>
>> As LS7A dts patch would depend on this series, and I want to
>> make the whole LS7A basic support as a part of 5.8 release.
>
>I think you have all necessary tags to take this in the MIPS
>tree, please let me know if that's the way we want this to go
>upstream - I would not pull MIPS/dts changes into the PCI tree
>and I don't think it is needed for this series.
Please pull all PCI changes [1~3] into PCI tree.
And Thomas,
Could you please apply rest changes into MIPS Tree?
Thanks a lot!
>
>Thanks,
>Lorenzo
--
Jiaxun Yang
^ permalink raw reply
* [PATCH v5 2/2] mailbox: sprd: Add Spreadtrum mailbox driver
From: Baolin Wang @ 2020-05-22 13:31 UTC (permalink / raw)
To: robh+dt, jassisinghbrar
Cc: orsonzhai, baolin.wang7, zhang.lyra, devicetree, linux-kernel
In-Reply-To: <8d29eba045ef18c5489e122b3668afc20431f15d.1590153779.git.baolin.wang7@gmail.com>
From: Baolin Wang <baolin.wang@unisoc.com>
The Spreadtrum mailbox controller supports 8 channels to communicate
with MCUs, and it contains 2 different parts: inbox and outbox, which
are used to send and receive messages by IRQ mode.
Signed-off-by: Baolin Wang <baolin.wang@unisoc.com>
Signed-off-by: Baolin Wang <baolin.wang7@gmail.com>
---
Changes from v4:
- Implement flush() to make sure the message has been fetched by
remote.
Changes from v3:
- Save the id in mbox_chan.con_priv and remove the 'sprd_mbox_chan'
Changes from v2:
- None.
Changes from v1:
- None
---
drivers/mailbox/Kconfig | 8 +
drivers/mailbox/Makefile | 2 +
drivers/mailbox/sprd-mailbox.c | 361 +++++++++++++++++++++++++++++++++
3 files changed, 371 insertions(+)
create mode 100644 drivers/mailbox/sprd-mailbox.c
diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index 5a577a6734cf..e03f3fb5caed 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -236,4 +236,12 @@ config SUN6I_MSGBOX
various Allwinner SoCs. This mailbox is used for communication
between the application CPUs and the power management coprocessor.
+config SPRD_MBOX
+ tristate "Spreadtrum Mailbox"
+ depends on ARCH_SPRD || COMPILE_TEST
+ help
+ Mailbox driver implementation for the Spreadtrum platform. It is used
+ to send message between application processors and MCU. Say Y here if
+ you want to build the Spreatrum mailbox controller driver.
+
endif
diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
index 2e4364ef5c47..9caf4ede6ce0 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -50,3 +50,5 @@ obj-$(CONFIG_MTK_CMDQ_MBOX) += mtk-cmdq-mailbox.o
obj-$(CONFIG_ZYNQMP_IPI_MBOX) += zynqmp-ipi-mailbox.o
obj-$(CONFIG_SUN6I_MSGBOX) += sun6i-msgbox.o
+
+obj-$(CONFIG_SPRD_MBOX) += sprd-mailbox.o
diff --git a/drivers/mailbox/sprd-mailbox.c b/drivers/mailbox/sprd-mailbox.c
new file mode 100644
index 000000000000..f6fab24ae8a9
--- /dev/null
+++ b/drivers/mailbox/sprd-mailbox.c
@@ -0,0 +1,361 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Spreadtrum mailbox driver
+ *
+ * Copyright (c) 2020 Spreadtrum Communications Inc.
+ */
+
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/mailbox_controller.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/clk.h>
+
+#define SPRD_MBOX_ID 0x0
+#define SPRD_MBOX_MSG_LOW 0x4
+#define SPRD_MBOX_MSG_HIGH 0x8
+#define SPRD_MBOX_TRIGGER 0xc
+#define SPRD_MBOX_FIFO_RST 0x10
+#define SPRD_MBOX_FIFO_STS 0x14
+#define SPRD_MBOX_IRQ_STS 0x18
+#define SPRD_MBOX_IRQ_MSK 0x1c
+#define SPRD_MBOX_LOCK 0x20
+#define SPRD_MBOX_FIFO_DEPTH 0x24
+
+/* Bit and mask definiation for inbox's SPRD_MBOX_FIFO_STS register */
+#define SPRD_INBOX_FIFO_DELIVER_MASK GENMASK(23, 16)
+#define SPRD_INBOX_FIFO_OVERLOW_MASK GENMASK(15, 8)
+#define SPRD_INBOX_FIFO_DELIVER_SHIFT 16
+#define SPRD_INBOX_FIFO_BUSY_MASK GENMASK(7, 0)
+
+/* Bit and mask definiation for SPRD_MBOX_IRQ_STS register */
+#define SPRD_MBOX_IRQ_CLR BIT(0)
+
+/* Bit and mask definiation for outbox's SPRD_MBOX_FIFO_STS register */
+#define SPRD_OUTBOX_FIFO_FULL BIT(0)
+#define SPRD_OUTBOX_FIFO_WR_SHIFT 16
+#define SPRD_OUTBOX_FIFO_RD_SHIFT 24
+#define SPRD_OUTBOX_FIFO_POS_MASK GENMASK(7, 0)
+
+/* Bit and mask definiation for inbox's SPRD_MBOX_IRQ_MSK register */
+#define SPRD_INBOX_FIFO_BLOCK_IRQ BIT(0)
+#define SPRD_INBOX_FIFO_OVERFLOW_IRQ BIT(1)
+#define SPRD_INBOX_FIFO_DELIVER_IRQ BIT(2)
+#define SPRD_INBOX_FIFO_IRQ_MASK GENMASK(2, 0)
+
+/* Bit and mask definiation for outbox's SPRD_MBOX_IRQ_MSK register */
+#define SPRD_OUTBOX_FIFO_NOT_EMPTY_IRQ BIT(0)
+#define SPRD_OUTBOX_FIFO_IRQ_MASK GENMASK(4, 0)
+
+#define SPRD_MBOX_CHAN_MAX 8
+
+struct sprd_mbox_priv {
+ struct mbox_controller mbox;
+ struct device *dev;
+ void __iomem *inbox_base;
+ void __iomem *outbox_base;
+ struct clk *clk;
+ u32 outbox_fifo_depth;
+
+ struct mbox_chan chan[SPRD_MBOX_CHAN_MAX];
+};
+
+static struct sprd_mbox_priv *to_sprd_mbox_priv(struct mbox_controller *mbox)
+{
+ return container_of(mbox, struct sprd_mbox_priv, mbox);
+}
+
+static u32 sprd_mbox_get_fifo_len(struct sprd_mbox_priv *priv, u32 fifo_sts)
+{
+ u32 wr_pos = (fifo_sts >> SPRD_OUTBOX_FIFO_WR_SHIFT) &
+ SPRD_OUTBOX_FIFO_POS_MASK;
+ u32 rd_pos = (fifo_sts >> SPRD_OUTBOX_FIFO_RD_SHIFT) &
+ SPRD_OUTBOX_FIFO_POS_MASK;
+ u32 fifo_len;
+
+ /*
+ * If the read pointer is equal with write pointer, which means the fifo
+ * is full or empty.
+ */
+ if (wr_pos == rd_pos) {
+ if (fifo_sts & SPRD_OUTBOX_FIFO_FULL)
+ fifo_len = priv->outbox_fifo_depth;
+ else
+ fifo_len = 0;
+ } else if (wr_pos > rd_pos) {
+ fifo_len = wr_pos - rd_pos;
+ } else {
+ fifo_len = priv->outbox_fifo_depth - rd_pos + wr_pos;
+ }
+
+ return fifo_len;
+}
+
+static irqreturn_t sprd_mbox_outbox_isr(int irq, void *data)
+{
+ struct sprd_mbox_priv *priv = data;
+ struct mbox_chan *chan;
+ u32 fifo_sts, fifo_len, msg[2];
+ int i, id;
+
+ fifo_sts = readl(priv->outbox_base + SPRD_MBOX_FIFO_STS);
+
+ fifo_len = sprd_mbox_get_fifo_len(priv, fifo_sts);
+ if (!fifo_len) {
+ dev_warn_ratelimited(priv->dev, "spurious outbox interrupt\n");
+ return IRQ_NONE;
+ }
+
+ for (i = 0; i < fifo_len; i++) {
+ msg[0] = readl(priv->outbox_base + SPRD_MBOX_MSG_LOW);
+ msg[1] = readl(priv->outbox_base + SPRD_MBOX_MSG_HIGH);
+ id = readl(priv->outbox_base + SPRD_MBOX_ID);
+
+ chan = &priv->chan[id];
+ mbox_chan_received_data(chan, (void *)msg);
+
+ /* Trigger to update outbox FIFO pointer */
+ writel(0x1, priv->outbox_base + SPRD_MBOX_TRIGGER);
+ }
+
+ /* Clear irq status after reading all message. */
+ writel(SPRD_MBOX_IRQ_CLR, priv->outbox_base + SPRD_MBOX_IRQ_STS);
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t sprd_mbox_inbox_isr(int irq, void *data)
+{
+ struct sprd_mbox_priv *priv = data;
+ struct mbox_chan *chan;
+ u32 fifo_sts, send_sts, busy, id;
+
+ fifo_sts = readl(priv->inbox_base + SPRD_MBOX_FIFO_STS);
+
+ /* Get the inbox data delivery status */
+ send_sts = (fifo_sts & SPRD_INBOX_FIFO_DELIVER_MASK) >>
+ SPRD_INBOX_FIFO_DELIVER_SHIFT;
+ if (!send_sts) {
+ dev_warn_ratelimited(priv->dev, "spurious inbox interrupt\n");
+ return IRQ_NONE;
+ }
+
+ while (send_sts) {
+ id = __ffs(send_sts);
+ send_sts &= (send_sts - 1);
+
+ chan = &priv->chan[id];
+
+ /*
+ * Check if the message was fetched by remote traget, if yes,
+ * that means the transmission has been completed.
+ */
+ busy = fifo_sts & SPRD_INBOX_FIFO_BUSY_MASK;
+ if (!(busy & BIT(id)))
+ mbox_chan_txdone(chan, 0);
+ }
+
+ /* Clear FIFO delivery and overflow status */
+ writel(fifo_sts &
+ (SPRD_INBOX_FIFO_DELIVER_MASK | SPRD_INBOX_FIFO_OVERLOW_MASK),
+ priv->inbox_base + SPRD_MBOX_FIFO_RST);
+
+ /* Clear irq status */
+ writel(SPRD_MBOX_IRQ_CLR, priv->inbox_base + SPRD_MBOX_IRQ_STS);
+
+ return IRQ_HANDLED;
+}
+
+static int sprd_mbox_send_data(struct mbox_chan *chan, void *msg)
+{
+ struct sprd_mbox_priv *priv = to_sprd_mbox_priv(chan->mbox);
+ unsigned long id = (unsigned long)chan->con_priv;
+ u32 *data = msg;
+
+ /* Write data into inbox FIFO, and only support 8 bytes every time */
+ writel(data[0], priv->inbox_base + SPRD_MBOX_MSG_LOW);
+ writel(data[1], priv->inbox_base + SPRD_MBOX_MSG_HIGH);
+
+ /* Set target core id */
+ writel(id, priv->inbox_base + SPRD_MBOX_ID);
+
+ /* Trigger remote request */
+ writel(0x1, priv->inbox_base + SPRD_MBOX_TRIGGER);
+
+ return 0;
+}
+
+static int sprd_mbox_flush(struct mbox_chan *chan, unsigned long timeout)
+{
+ struct sprd_mbox_priv *priv = to_sprd_mbox_priv(chan->mbox);
+ unsigned long id = (unsigned long)chan->con_priv;
+ u32 busy;
+
+ timeout = jiffies + msecs_to_jiffies(timeout);
+
+ while (time_before(jiffies, timeout)) {
+ busy = readl(priv->inbox_base + SPRD_MBOX_FIFO_STS) &
+ SPRD_INBOX_FIFO_BUSY_MASK;
+ if (!(busy & BIT(id))) {
+ mbox_chan_txdone(chan, 0);
+ return 0;
+ }
+
+ udelay(1);
+ }
+
+ return -ETIME;
+}
+
+static int sprd_mbox_startup(struct mbox_chan *chan)
+{
+ struct sprd_mbox_priv *priv = to_sprd_mbox_priv(chan->mbox);
+ u32 val;
+
+ /* Select outbox FIFO mode and reset the outbox FIFO status */
+ writel(0x0, priv->outbox_base + SPRD_MBOX_FIFO_RST);
+
+ /* Enable inbox FIFO overflow and delivery interrupt */
+ val = readl(priv->inbox_base + SPRD_MBOX_IRQ_MSK);
+ val &= ~(SPRD_INBOX_FIFO_OVERFLOW_IRQ | SPRD_INBOX_FIFO_DELIVER_IRQ);
+ writel(val, priv->inbox_base + SPRD_MBOX_IRQ_MSK);
+
+ /* Enable outbox FIFO not empty interrupt */
+ val = readl(priv->outbox_base + SPRD_MBOX_IRQ_MSK);
+ val &= ~SPRD_OUTBOX_FIFO_NOT_EMPTY_IRQ;
+ writel(val, priv->outbox_base + SPRD_MBOX_IRQ_MSK);
+
+ return 0;
+}
+
+static void sprd_mbox_shutdown(struct mbox_chan *chan)
+{
+ struct sprd_mbox_priv *priv = to_sprd_mbox_priv(chan->mbox);
+
+ /* Disable inbox & outbox interrupt */
+ writel(SPRD_INBOX_FIFO_IRQ_MASK, priv->inbox_base + SPRD_MBOX_IRQ_MSK);
+ writel(SPRD_OUTBOX_FIFO_IRQ_MASK, priv->outbox_base + SPRD_MBOX_IRQ_MSK);
+}
+
+static const struct mbox_chan_ops sprd_mbox_ops = {
+ .send_data = sprd_mbox_send_data,
+ .flush = sprd_mbox_flush,
+ .startup = sprd_mbox_startup,
+ .shutdown = sprd_mbox_shutdown,
+};
+
+static void sprd_mbox_disable(void *data)
+{
+ struct sprd_mbox_priv *priv = data;
+
+ clk_disable_unprepare(priv->clk);
+}
+
+static int sprd_mbox_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct sprd_mbox_priv *priv;
+ int ret, inbox_irq, outbox_irq;
+ unsigned long id;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->dev = dev;
+
+ /*
+ * The Spreadtrum mailbox uses an inbox to send messages to the target
+ * core, and uses an outbox to receive messages from other cores.
+ *
+ * Thus the mailbox controller supplies 2 different register addresses
+ * and IRQ numbers for inbox and outbox.
+ */
+ priv->inbox_base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(priv->inbox_base))
+ return PTR_ERR(priv->inbox_base);
+
+ priv->outbox_base = devm_platform_ioremap_resource(pdev, 1);
+ if (IS_ERR(priv->outbox_base))
+ return PTR_ERR(priv->outbox_base);
+
+ priv->clk = devm_clk_get(dev, "enable");
+ if (IS_ERR(priv->clk)) {
+ dev_err(dev, "failed to get mailbox clock\n");
+ return PTR_ERR(priv->clk);
+ }
+
+ ret = clk_prepare_enable(priv->clk);
+ if (ret)
+ return ret;
+
+ ret = devm_add_action_or_reset(dev, sprd_mbox_disable, priv);
+ if (ret) {
+ dev_err(dev, "failed to add mailbox disable action\n");
+ return ret;
+ }
+
+ inbox_irq = platform_get_irq(pdev, 0);
+ if (inbox_irq < 0)
+ return inbox_irq;
+
+ ret = devm_request_irq(dev, inbox_irq, sprd_mbox_inbox_isr,
+ IRQF_NO_SUSPEND, dev_name(dev), priv);
+ if (ret) {
+ dev_err(dev, "failed to request inbox IRQ: %d\n", ret);
+ return ret;
+ }
+
+ outbox_irq = platform_get_irq(pdev, 1);
+ if (outbox_irq < 0)
+ return outbox_irq;
+
+ ret = devm_request_irq(dev, outbox_irq, sprd_mbox_outbox_isr,
+ IRQF_NO_SUSPEND, dev_name(dev), priv);
+ if (ret) {
+ dev_err(dev, "failed to request outbox IRQ: %d\n", ret);
+ return ret;
+ }
+
+ /* Get the default outbox FIFO depth */
+ priv->outbox_fifo_depth =
+ readl(priv->outbox_base + SPRD_MBOX_FIFO_DEPTH) + 1;
+ priv->mbox.dev = dev;
+ priv->mbox.chans = &priv->chan[0];
+ priv->mbox.num_chans = SPRD_MBOX_CHAN_MAX;
+ priv->mbox.ops = &sprd_mbox_ops;
+ priv->mbox.txdone_irq = true;
+
+ for (id = 0; id < SPRD_MBOX_CHAN_MAX; id++)
+ priv->chan[id].con_priv = (void *)id;
+
+ ret = devm_mbox_controller_register(dev, &priv->mbox);
+ if (ret) {
+ dev_err(dev, "failed to register mailbox: %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static const struct of_device_id sprd_mbox_of_match[] = {
+ { .compatible = "sprd,sc9860-mailbox", },
+ { },
+};
+MODULE_DEVICE_TABLE(of, sprd_mbox_of_match);
+
+static struct platform_driver sprd_mbox_driver = {
+ .driver = {
+ .name = "sprd-mailbox",
+ .of_match_table = sprd_mbox_of_match,
+ },
+ .probe = sprd_mbox_probe,
+};
+module_platform_driver(sprd_mbox_driver);
+
+MODULE_AUTHOR("Baolin Wang <baolin.wang@unisoc.com>");
+MODULE_DESCRIPTION("Spreadtrum mailbox driver");
+MODULE_LICENSE("GPL v2");
--
2.17.1
^ permalink raw reply related
* [PATCH v5 1/2] dt-bindings: mailbox: Add the Spreadtrum mailbox documentation
From: Baolin Wang @ 2020-05-22 13:31 UTC (permalink / raw)
To: robh+dt, jassisinghbrar
Cc: orsonzhai, baolin.wang7, zhang.lyra, devicetree, linux-kernel
From: Baolin Wang <baolin.wang@unisoc.com>
Add the Spreadtrum mailbox documentation.
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Baolin Wang <baolin.wang@unisoc.com>
Signed-off-by: Baolin Wang <baolin.wang7@gmail.com>
---
Changes from v4:
-None.
Changes from v3:
- None.
Changes from v2:
- Add reviewed tag from Rob.
- Remove redundant 'minItems'.
Changes from v1:
- Add 'additionalProperties'.
- Split description for each entry.
---
.../bindings/mailbox/sprd-mailbox.yaml | 60 +++++++++++++++++++
1 file changed, 60 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mailbox/sprd-mailbox.yaml
diff --git a/Documentation/devicetree/bindings/mailbox/sprd-mailbox.yaml b/Documentation/devicetree/bindings/mailbox/sprd-mailbox.yaml
new file mode 100644
index 000000000000..0f7451b42d7e
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/sprd-mailbox.yaml
@@ -0,0 +1,60 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/mailbox/sprd-mailbox.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Spreadtrum mailbox controller bindings
+
+maintainers:
+ - Orson Zhai <orsonzhai@gmail.com>
+ - Baolin Wang <baolin.wang7@gmail.com>
+ - Chunyan Zhang <zhang.lyra@gmail.com>
+
+properties:
+ compatible:
+ enum:
+ - sprd,sc9860-mailbox
+
+ reg:
+ items:
+ - description: inbox registers' base address
+ - description: outbox registers' base address
+
+ interrupts:
+ items:
+ - description: inbox interrupt
+ - description: outbox interrupt
+
+ clocks:
+ maxItems: 1
+
+ clock-names:
+ items:
+ - const: enable
+
+ "#mbox-cells":
+ const: 1
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - "#mbox-cells"
+ - clocks
+ - clock-names
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ mailbox: mailbox@400a0000 {
+ compatible = "sprd,sc9860-mailbox";
+ reg = <0 0x400a0000 0 0x8000>, <0 0x400a8000 0 0x8000>;
+ #mbox-cells = <1>;
+ clock-names = "enable";
+ clocks = <&aon_gate 53>;
+ interrupts = <GIC_SPI 28 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 29 IRQ_TYPE_LEVEL_HIGH>;
+ };
+...
--
2.17.1
^ permalink raw reply related
* Re: [PATCH V2 2/3] mmc: sdhci-msm: Use internal voltage control
From: Veerabhadrarao Badiganti @ 2020-05-22 13:27 UTC (permalink / raw)
To: Bjorn Andersson
Cc: adrian.hunter, ulf.hansson, robh+dt, linux-mmc, linux-kernel,
linux-arm-msm, devicetree, Asutosh Das, Vijay Viswanath,
Andy Gross
In-Reply-To: <20200521190739.GC1331782@builder.lan>
Hi Bjorn,
On 5/22/2020 12:37 AM, Bjorn Andersson wrote:
> On Thu 21 May 08:23 PDT 2020, Veerabhadrarao Badiganti wrote:
>
>> On qcom SD host controllers voltage switching be done after the HW
>> is ready for it. The HW informs its readiness through power irq.
>> The voltage switching should happen only then.
>>
>> Use the internal voltage switching and then control the voltage
>> switching using power irq.
>>
>> Set the regulator load as well so that regulator can be configured
>> in LPM mode when in is not being used.
>>
>> Co-developed-by: Asutosh Das <asutoshd@codeaurora.org>
>> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
>> Co-developed-by: Vijay Viswanath <vviswana@codeaurora.org>
>> Signed-off-by: Vijay Viswanath <vviswana@codeaurora.org>
>> Co-developed-by: Veerabhadrarao Badiganti <vbadigan@codeaurora.org>
>> Signed-off-by: Veerabhadrarao Badiganti <vbadigan@codeaurora.org>
> Looks better, thanks.
>
>> ---
>> drivers/mmc/host/sdhci-msm.c | 207 +++++++++++++++++++++++++++++++++++++++++--
>> 1 file changed, 198 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> [..]
>> static const struct sdhci_msm_offset *sdhci_priv_msm_offset(struct sdhci_host *host)
>> @@ -1298,6 +1302,71 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host *host,
>> sdhci_msm_hs400(host, &mmc->ios);
>> }
>>
>> +static int sdhci_msm_set_vmmc(struct mmc_host *mmc)
>> +{
>> + int ret;
>> +
>> + if (IS_ERR(mmc->supply.vmmc))
>> + return 0;
>> +
>> + ret = mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, mmc->ios.vdd);
>> + if (ret)
>> + dev_err(mmc_dev(mmc), "%s: vmmc set ocr with vdd=%d failed: %d\n",
>> + mmc_hostname(mmc), mmc->ios.vdd, ret);
> Missed this one on v1, in the event that mmc_regulator_set_ocr() return
> a non-zero value it has already printed an error message. So please
> replace the tail with just:
>
> return mmc_regulator_set_ocr(...);
>
>> +
>> + return ret;
>> +}
>> +
>> +static int sdhci_msm_set_vqmmc(struct sdhci_msm_host *msm_host,
>> + struct mmc_host *mmc, bool level)
>> +{
>> + int load, ret;
>> + struct mmc_ios ios;
>> +
>> + if (IS_ERR(mmc->supply.vqmmc) ||
>> + (mmc->ios.power_mode == MMC_POWER_UNDEFINED) ||
>> + (msm_host->vqmmc_enabled == level))
>> + return 0;
>> +
>> + if (msm_host->vqmmc_load) {
>> + load = level ? msm_host->vqmmc_load : 0;
>> + ret = regulator_set_load(mmc->supply.vqmmc, load);
> Sorry for the late reply on v1, but please see my explanation regarding
> load and always-on regulators there.
<Merging your comment from V1 here>
>> You should still call regulator_enable()/regulator_disable() on your
>> consumer regulator in this driver. When you do this the regulator core
>> will conclude that the regulator_dev (i.e. the part that represents the
>> hardware) is marked always_on and will not enable/disable the regulator.
>> But it will still invoke _regulator_handle_consumer_enable() and
>> _regulator_handle_consumer_disable(), which will aggregate the "load" of
>> all client regulators and update the regulator's load.
>> So this will apply the load as you expect regardless of it being
>> supplied by a regulator marked as always_on.
Since I'm not turning off this regulator for eMMC, I wanted to keep it
in LPM mode
to save some power.
When the regulator configured in auto mode (RPMH_REGULATOR_MODE_AUTO) it
switches to LPM/HPM mode based on the active load.
So i have to minimize my driver load requirement so that I can let this
regulator
in LPM mode.
So i need to set load every-time I disable/enable the regulator.
>> + if (ret) {
>> + dev_err(mmc_dev(mmc), "%s: vqmmc set load failed: %d\n",
>> + mmc_hostname(mmc), ret);
>> + goto out;
>> + }
>> + }
>> +
>> + if (level) {
>> + /* Set the IO voltage regulator to default voltage level */
>> + if (msm_host->caps_0 & CORE_3_0V_SUPPORT)
>> + ios.signal_voltage = MMC_SIGNAL_VOLTAGE_330;
>> + else if (msm_host->caps_0 & CORE_1_8V_SUPPORT)
>> + ios.signal_voltage = MMC_SIGNAL_VOLTAGE_180;
>> +
>> + if (msm_host->caps_0 & CORE_VOLT_SUPPORT) {
>> + ret = mmc_regulator_set_vqmmc(mmc, &ios);
>> + if (ret < 0) {
>> + dev_err(mmc_dev(mmc), "%s: vqmmc set volgate failed: %d\n",
>> + mmc_hostname(mmc), ret);
>> + goto out;
>> + }
>> + }
>> + ret = regulator_enable(mmc->supply.vqmmc);
>> + } else {
>> + ret = regulator_disable(mmc->supply.vqmmc);
>> + }
>> +
>> + if (ret)
>> + dev_err(mmc_dev(mmc), "%s: vqmm %sable failed: %d\n",
>> + mmc_hostname(mmc), level ? "en":"dis", ret);
>> + else
>> + msm_host->vqmmc_enabled = level;
>> +out:
>> + return ret;
>> +}
> [..]
>> +static int sdhci_msm_start_signal_voltage_switch(struct mmc_host *mmc,
>> + struct mmc_ios *ios)
>> +{
>> + struct sdhci_host *host = mmc_priv(mmc);
>> + u16 ctrl, status;
>> +
>> + /*
>> + * Signal Voltage Switching is only applicable for Host Controllers
>> + * v3.00 and above.
>> + */
>> + if (host->version < SDHCI_SPEC_300)
>> + return 0;
>> +
>> + ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
>> +
>> + switch (ios->signal_voltage) {
>> + case MMC_SIGNAL_VOLTAGE_330:
>> + if (!(host->flags & SDHCI_SIGNALING_330))
>> + return -EINVAL;
>> +
>> + /* Set 1.8V Signal Enable in the Host Control2 register to 0 */
>> + ctrl &= ~SDHCI_CTRL_VDD_180;
>> + break;
>> + case MMC_SIGNAL_VOLTAGE_180:
>> + if (!(host->flags & SDHCI_SIGNALING_180))
>> + return -EINVAL;
>> +
>> + /*
>> + * Enable 1.8V Signal Enable in the Host Control2
>> + * register
>> + */
>> + ctrl |= SDHCI_CTRL_VDD_180;
>> + break;
>> + case MMC_SIGNAL_VOLTAGE_120:
>> + if (!(host->flags & SDHCI_SIGNALING_120))
>> + return -EINVAL;
>> + return 0;
>> + default:
>> + /* No signal voltage switch required */
>> + return 0;
>> + }
>> +
>> + sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
>> +
>> + /* Wait for 5ms */
>> + usleep_range(5000, 5500);
>> +
>> + /* regulator output should be stable within 5 ms */
>> + status = !!(ctrl & SDHCI_CTRL_VDD_180);
>> + ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
>> + if (!!(ctrl & SDHCI_CTRL_VDD_180) == status)
> You should be able to drop the !! both here and when assigning status.
>
> Overall this looks neater, thanks for reworking it.
>
> Regards,
> Bjorn
Thanks
Veera
^ permalink raw reply
* Re: [PATCH v4 03/13] mips: Add MIPS Release 5 support
From: Serge Semin @ 2020-05-22 13:15 UTC (permalink / raw)
To: Thomas Bogendoerfer
Cc: Serge Semin, Alexey Malahov, Paul Burton, Ralf Baechle,
Arnd Bergmann, Rob Herring, devicetree, Jiaxun Yang,
Alexander Lobakin, Huacai Chen, Nathan Chancellor, Ard Biesheuvel,
Cedric Hombourger, Thomas Gleixner, Ingo Molnar,
Sebastian Andrzej Siewior, Philippe Mathieu-Daudé,
Guenter Roeck, Paul Cercueil, Zhou Yanjie, Masahiro Yamada,
Greg Kroah-Hartman, Allison Randal, Liangliang Huang,
周琰杰 (Zhou Yanjie), YunQiang Su, Zou Wei,
Oleksij Rempel, Kamal Dasu, linux-mips, linux-kernel, kvm
In-Reply-To: <20200522072743.GA7331@alpha.franken.de>
On Fri, May 22, 2020 at 09:27:43AM +0200, Thomas Bogendoerfer wrote:
> On Thu, May 21, 2020 at 05:07:14PM +0300, Serge Semin wrote:
> > There are five MIPS32/64 architecture releases currently available:
> > from 1 to 6 except fourth one, which was intentionally skipped.
> > Three of them can be called as major: 1st, 2nd and 6th, that not only
> > have some system level alterations, but also introduced significant
> > core/ISA level updates. The rest of the MIPS architecture releases are
> > minor.
> >
> > Even though they don't have as much ISA/system/core level changes
> > as the major ones with respect to the previous releases, they still
> > provide a set of updates (I'd say they were intended to be the
> > intermediate releases before a major one) that might be useful for the
> > kernel and user-level code, when activated by the kernel or compiler.
> > In particular the following features were introduced or ended up being
> > available at/after MIPS32/64 Release 5 architecture:
> > + the last release of the misaligned memory access instructions,
> > + virtualisation - VZ ASE - is optional component of the arch,
> > + SIMD - MSA ASE - is optional component of the arch,
> > + DSP ASE is optional component of the arch,
> > + CP0.Status.FR=1 for CP1.FIR.F64=1 (pure 64-bit FPU general registers)
> > must be available if FPU is implemented,
> > + CP1.FIR.Has2008 support is required so CP1.FCSR.{ABS2008,NAN2008} bits
> > are available.
> > + UFR/UNFR aliases to access CP0.Status.FR from user-space by means of
> > ctc1/cfc1 instructions (enabled by CP0.Config5.UFR),
> > + CP0.COnfig5.LLB=1 and eretnc instruction are implemented to without
> > accidentally clearing LL-bit when returning from an interrupt,
> > exception, or error trap,
> > + XPA feature together with extended versions of CPx registers is
> > introduced, which needs to have mfhc0/mthc0 instructions available.
> >
> > So due to these changes GNU GCC provides an extended instructions set
> > support for MIPS32/64 Release 5 by default like eretnc/mfhc0/mthc0. Even
> > though the architecture alteration isn't that big, it still worth to be
> > taken into account by the kernel software. Finally we can't deny that
> > some optimization/limitations might be found in future and implemented
> > on some level in kernel or compiler. In this case having even
> > intermediate MIPS architecture releases support would be more than
> > useful.
> >
> > So the most of the changes provided by this commit can be split into
> > either compile- or runtime configs related. The compile-time related
> > changes are caused by adding the new CONFIG_CPU_MIPS32_R5/CONFIG_CPU_MIPSR5
> > configs and concern the code activating MIPSR2 or MIPSR6 already
> > implemented features (like eretnc/LLbit, mthc0/mfhc0). In addition
> > CPU_HAS_MSA can be now freely enabled for MIPS32/64 release 5 based
> > platforms as this is done for CPU_MIPS32_R6 CPUs. The runtime changes
> > concerns the features which are handled with respect to the MIPS ISA
> > revision detected at run-time by means of CP0.Config.{AT,AR} bits. Alas
> > these fields can be used to detect either r1 or r2 or r6 releases.
> > But since we know which CPUs in fact support the R5 arch, we can manually
> > set MIPS_CPU_ISA_M32R5/MIPS_CPU_ISA_M64R5 bit of c->isa_level and then
> > use cpu_has_mips32r5/cpu_has_mips64r5 where it's appropriate.
> >
> > Since XPA/EVA provide too complex alterationss and to have them used with
> > MIPS32 Release 2 charged kernels (for compatibility with current platform
> > configs) they are left to be setup as a separate kernel configs.
> >
> > Co-developed-by: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> > Signed-off-by: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> > Cc: Paul Burton <paulburton@kernel.org>
> > Cc: Ralf Baechle <ralf@linux-mips.org>
> > Cc: Arnd Bergmann <arnd@arndb.de>
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: devicetree@vger.kernel.org
> > ---
> > arch/mips/Kconfig | 56 +++++++++++++++++++++++++---
> > arch/mips/Makefile | 2 +
> > arch/mips/include/asm/asmmacro.h | 18 +++++----
> > arch/mips/include/asm/compiler.h | 5 +++
> > arch/mips/include/asm/cpu-features.h | 27 ++++++++++----
> > arch/mips/include/asm/cpu-info.h | 2 +-
> > arch/mips/include/asm/cpu-type.h | 7 +++-
> > arch/mips/include/asm/cpu.h | 10 +++--
> > arch/mips/include/asm/fpu.h | 4 +-
> > arch/mips/include/asm/hazards.h | 8 ++--
> > arch/mips/include/asm/module.h | 4 ++
> > arch/mips/include/asm/stackframe.h | 2 +-
> > arch/mips/include/asm/switch_to.h | 8 ++--
> > arch/mips/kernel/cpu-probe.c | 17 +++++++++
> > arch/mips/kernel/entry.S | 6 +--
> > arch/mips/kernel/proc.c | 4 ++
> > arch/mips/kernel/r4k_fpu.S | 14 +++----
> > arch/mips/kvm/vz.c | 6 +--
> > arch/mips/lib/csum_partial.S | 6 ++-
> > arch/mips/mm/c-r4k.c | 7 ++--
> > arch/mips/mm/sc-mips.c | 7 ++--
> > 21 files changed, 163 insertions(+), 57 deletions(-)
>
> applied to mips-next. I've changed the two /* fall through */ by fallthrough;
> while appliny. Running checkpatch would have caught that ;-)
Good. Thanks. Actually I've seen that warning, but just didn't know what way to
choose.) So I've decided to leave the comment-based Fall-through fixup seeing
the rest of the file is using the older way. By doing so I've kept the locally
implemented coding style. Though I've heard the explicit attribute "fallthrough;"
utilization is a preferred way of marking combined case statements.
-Sergey
>
> Thomas.
>
> --
> Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
> good idea. [ RFC1925, 2.3 ]
^ permalink raw reply
* Re: [PATCH v10 2/5] PCI: Add Loongson PCI Controller support
From: Lorenzo Pieralisi @ 2020-05-22 13:10 UTC (permalink / raw)
To: Jiaxun Yang
Cc: linux-pci, Rob Herring, Bjorn Helgaas, Rob Herring,
Thomas Bogendoerfer, Huacai Chen, Paul Burton, devicetree,
linux-kernel, linux-mips
In-Reply-To: <AC29D474-D846-41AF-9900-759CE430A744@flygoat.com>
On Wed, May 20, 2020 at 07:57:29PM +0800, Jiaxun Yang wrote:
>
>
> 于 2020年5月14日 GMT+08:00 下午9:16:38, Jiaxun Yang <jiaxun.yang@flygoat.com> 写到:
> >This controller can be found on Loongson-2K SoC, Loongson-3
> >systems with RS780E/LS7A PCH.
> >
> >The RS780E part of code was previously located at
> >arch/mips/pci/ops-loongson3.c and now it can use generic PCI
> >driver implementation.
> >
> >Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> >Reviewed-by: Rob Herring <robh@kernel.org>
> >
>
> Hi there,
>
> Is it possible to let this series go into next tree soon?
>
> As LS7A dts patch would depend on this series, and I want to
> make the whole LS7A basic support as a part of 5.8 release.
I think you have all necessary tags to take this in the MIPS
tree, please let me know if that's the way we want this to go
upstream - I would not pull MIPS/dts changes into the PCI tree
and I don't think it is needed for this series.
Thanks,
Lorenzo
^ permalink raw reply
* Re: [PATCH v4 2/2] mailbox: sprd: Add Spreadtrum mailbox driver
From: Baolin Wang @ 2020-05-22 13:07 UTC (permalink / raw)
To: Jassi Brar; +Cc: Rob Herring, Orson Zhai, Chunyan Zhang, Devicetree List, LKML
In-Reply-To: <CABb+yY244ZCOk5kDtOR0oEYajwUVbXoSZdNiid__UuYbU=yB-Q@mail.gmail.com>
On Fri, May 22, 2020 at 11:48 AM Jassi Brar <jassisinghbrar@gmail.com> wrote:
>
> On Thu, May 21, 2020 at 7:24 AM Baolin Wang <baolin.wang7@gmail.com> wrote:
> >
> > Hi Jassi,
> >
> > On Wed, May 13, 2020 at 2:32 PM Baolin Wang <baolin.wang7@gmail.com> wrote:
> > >
> > > On Wed, May 13, 2020 at 2:05 PM Jassi Brar <jassisinghbrar@gmail.com> wrote:
> > > >
> > > > On Tue, May 12, 2020 at 11:14 PM Baolin Wang <baolin.wang7@gmail.com> wrote:
> > > > >
> > > > > Hi Jassi,
> > > > >
> > > > > On Thu, May 7, 2020 at 11:23 AM Baolin Wang <baolin.wang7@gmail.com> wrote:
> > > > > >
> > > > > > Hi Jassi,
> > > > > >
> > > > > > On Thu, May 7, 2020 at 7:25 AM Jassi Brar <jassisinghbrar@gmail.com> wrote:
> > > > > > >
> > > > > > > On Wed, May 6, 2020 at 8:29 AM Baolin Wang <baolin.wang7@gmail.com> wrote:
> > > > > > > >
> > > > > > > > Hi Jassi,
> > > > > > > >
> > > > > > > > On Tue, Apr 28, 2020 at 11:10 AM Baolin Wang <baolin.wang7@gmail.com> wrote:
> > > > > > > > >
> > > > > > > > > From: Baolin Wang <baolin.wang@unisoc.com>
> > > > > > > > >
> > > > > > > > > The Spreadtrum mailbox controller supports 8 channels to communicate
> > > > > > > > > with MCUs, and it contains 2 different parts: inbox and outbox, which
> > > > > > > > > are used to send and receive messages by IRQ mode.
> > > > > > > > >
> > > > > > > > > Signed-off-by: Baolin Wang <baolin.wang@unisoc.com>
> > > > > > > > > Signed-off-by: Baolin Wang <baolin.wang7@gmail.com>
> > > > > > > > > ---
> > > > > > > > > Changes from v3:
> > > > > > > > > - Save the id in mbox_chan.con_priv and remove the 'sprd_mbox_chan'
> > > > > > > > >
> > > > > > > > > Changes from v2:
> > > > > > > > > - None.
> > > > > > > > >
> > > > > > > > > Changes from v1:
> > > > > > > > > - None
> > > > > > > >
> > > > > > > > Gentle ping, do you have any other comments? Thanks.
> > > > > > > >
> > > > > > > Yea, I am still not sure about the error returned in send_data(). It
> > > > > > > will either never hit or there will be no easy recovery from it. The
> > > > > > > api expects the driver to tell it the last-tx was done only when it
> > > > > > > can send the next message. (There may be case like sending depend on
> > > > > > > remote, which can't be ensured before hand).
> > > > > >
> > > > > > Actually this is an unusual case, suppose the remote target did not
> > > > > > fetch the message as soon as possile, which will cause the FIFO
> > > > > > overflow, so in this case we can not send messages to the remote
> > > > > > target any more, otherwise messages will be lost. Thus we can return
> > > > > > errors to users to indicate that something wrong with the remote
> > > > > > target need to be checked.
> > > > > >
> > > > > > So this validation in send_data() is mostly for debugging for this
> > > > > > abnormal case and we will not trigger this issue if the remote target
> > > > > > works well. So I think it is useful to keep this validation in
> > > > > > send_data(). Thanks.
> > > > >
> > > > > Any comments? Thanks.
> > > > >
> > > > Same as my last post.
> > >
> > > I think I've explained the reason why we need add this validation in
> > > my previous email, I am not sure how do you think? You still want to
> > > remove this validation?
> >
> > Gentle ping.
> >
> > As I explained in previous email, this validation is for an unusual
> > case, suppose the remote target did not fetch the message as soon as
> > possile, which will cause the FIFO overflow, so in this case we can
> > not send messages to the remote
> > target any more, otherwise messages will be lost. Thus we can return
> > errors to users to indicate that something wrong with the remote
> > target need to be checked.
> >
> > So this validation in send_data() is mostly for debugging for this
> > abnormal case and we will not trigger this issue if the remote target
> > works well. So I think it is useful to keep this validation in
> > send_data(). What do you think? Thanks.
> >
> I still think the same as before.
> You should do this check before you call mbox_chan_txdone() and wait
> if busy ... which is exactly the purpose of txdone().
> It seems harmless to be paranoid and place a block of code in
> practically "if 0", but that sets bad precedence for other drivers. So
> please move the check before txdone().
OK. I realized I can implement the flush() to make sure the
transmission has been completed. Thanks.
--
Baolin Wang
^ permalink raw reply
* Re: [PATCH] PM / devfreq: fix odd_ptr_err.cocci warnings
From: Julia Lawall @ 2020-05-22 12:59 UTC (permalink / raw)
To: kbuild-all, Andrew-sh.Cheng, MyungJoo Ham, Kyungmin Park,
Chanwoo Choi, Rob Herring, Mark Rutland, Matthias Brugger,
Rafael J . Wysocki, Viresh Kumar, Nishanth Menon, Stephen Boyd,
Liam Girdwood, Mark Brown, devicetree, Andrew-sh . Cheng,
srv_heupstream, linux-pm, linux-kernel, Saravana Kannan,
linux-mediatek, Sibi Sankar, linux-arm-kernel
In-Reply-To: <20200521160908.GA88022@052716d1a29e>
Hello,
This provides a patch, but it doesn't look like the right one. It looks
like the if test should be testing opp_table,
julia
On Fri, 22 May 2020, kbuild test robot wrote:
> From: kbuild test robot <lkp@intel.com>
>
> drivers/devfreq/governor_passive.c:336:7-13: inconsistent IS_ERR and PTR_ERR on line 337.
>
> PTR_ERR should access the value just tested by IS_ERR
>
> Semantic patch information:
> There can be false positives in the patch case, where it is the call to
> IS_ERR that is wrong.
>
> Generated by: scripts/coccinelle/tests/odd_ptr_err.cocci
>
> CC: Saravana Kannan <skannan@codeaurora.org>
> Signed-off-by: kbuild test robot <lkp@intel.com>
> ---
>
> url: https://github.com/0day-ci/linux/commits/Andrew-sh-Cheng/Add-cpufreq-and-cci-devfreq-for-mt8183-and-SVS-support/20200520-222709
> base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
> :::::: branch date: 26 hours ago
> :::::: commit date: 26 hours ago
>
> Please take the patch only if it's a positive warning. Thanks!
>
> governor_passive.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> --- a/drivers/devfreq/governor_passive.c
> +++ b/drivers/devfreq/governor_passive.c
> @@ -334,7 +334,7 @@ static int cpufreq_passive_register(stru
>
> opp_table = dev_pm_opp_get_opp_table(cpu_dev);
> if (IS_ERR(devfreq->opp_table)) {
> - ret = PTR_ERR(opp_table);
> + ret = PTR_ERR(devfreq->opp_table);
> goto out;
> }
>
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox