Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH v10 00/27] ARM: davinci: convert to common clock framework​
From: David Lechner @ 2018-05-12 21:11 UTC (permalink / raw)
  To: Sekhar Nori, linux-clk, devicetree, linux-arm-kernel
  Cc: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland,
	Kevin Hilman, Bartosz Golaszewski, Adam Ford, linux-kernel
In-Reply-To: <45ce3129-4a9e-54d2-e594-49b451571842@ti.com>

On 05/11/2018 10:26 AM, Sekhar Nori wrote:
> Hi David,
> 
> On Wednesday 09 May 2018 10:55 PM, David Lechner wrote:
>> This series converts mach-davinci to use the common clock framework.
>>
>> The series works like this, the first 3 patches fix some issues with the clock
>> drivers that have already been accepted into the mainline kernel.
> 
> I have not yet looked at the patches, but I got a bunch of W=1 warnings
> and some sparse warnings when building your branch. Please take a look
> at these. Unfortunately the output is mixed between sparse and compiler.
> The "expression using sizeof(void)" can be ignored as its a known issue
> with sparse, I believe.
> 

I've started a common-clk-v11 branch on my GitHub that fixes most of these.
Also submitted "clk: davinci: psc-dm355: fix ASP0/1 clkdev lookups" that
fixes a couple more. I've purposely not fixed the davinci_clk_reset_* functions
since there is already a patch that will remove those functions in the future.

I'll wait a bit longer for DT review before re-sending v11 of this series.

^ permalink raw reply

* Re: [PATCH v2 2/2] leds: Add Spreadtrum SC27xx breathing light controller driver
From: Jacek Anaszewski @ 2018-05-12 20:44 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Baolin Wang, robh+dt, mark.rutland, xiaotong.lu, broonie,
	linux-leds, devicetree, linux-kernel
In-Reply-To: <20180512083559.GB8944@amd>

Hi Pavel,

On 05/12/2018 10:35 AM, Pavel Machek wrote:
> Hi!
>
>>>> I disagree here. We already had the same discussion at the occasion
>>>> of the patch [0] and it turned out to be a dead-end [1]. Now we have
>>>> neither the driver nor the generic pattern interface.
>>>>
>>>> We also already have some older LED class drivers that implement custom
>>>> pattern interfaces (e.g. drivers/leds/leds-lm3533.c) and the same
>>>> approach can be applied in this case.
>>>
>>> Please don't. It was mistake to implement custom pattern interfaces
>>> back then, it is still mistake now.
>>
>> It turned out to be really hard to cover all known pattern generator
>> implementations with generic interface. Sure, it would be nice to have
>> one, but the whole discussion around [0] only unveiled the diversity of
>> parameters to cover. And still new devices appear on the market.
>>
>> We would have to propose a set of pattern schemes and allow to
>> add new ones to it.
>
> I believe that what I'm proposing below is close enough to universal.
>
>>> If we really need solution now, I'd recommend "pattern" file with
>>>
>>> "<delta time> <brightness> <delta time> <brightness>".
>>>
>>> In this specific case, hardware only supports patterns in this format:
>>>
>>> low_time 0 rise_time 255 high_time 255 fall_time 0
>>>
>>> so driver would simply -EINVAL on anything else.
>>
>> I'm fine with the pattern file, but the pattern format would have
>> to be defined in the per-driver ABI documentation. It wouldn't much
>> differ from the custom pattern approach though, unless I'm missing some
>> gain of having pattern setting in a uniformly named single sysfs file
>> (with semantics differing from driver to driver).
>
> I'm proposing "<delta time> <brightness> ..." sysfs file. It certainly
> covers this hardware, it would be enough to cover the Qualcomm Pulse
> generator (IIRC), and it would cover most uses cases of Nokia N900's
> LED.
>
> Yes, we would need to document limitations of each chip. But it should
> be easily possible to run pattern designed for Spreadtrum on N900,
> even if it would not work the other way around.
>
> (If someone really wants to run complex patterns on simple hardware,
> we can provide software emulation using same file format. I believe I
> still have that patch somewhere.)

OK, I've revised the discussion under Qualcomm LPG patch set and
it seems that we have almost ready solution in [0], except the
pattern_repeat file you mention in [1]. So probably Baolin could
address your remarks from [1] and add pattern_repeat file to the
patch that begins thread [0].

[0] https://lkml.org/lkml/2017/11/15/27
[1] https://lkml.org/lkml/2017/12/8/470

-- 
Best regards,
Jacek Anaszewski

^ permalink raw reply

* Re: [PATCH] mtd: nand: Add support for reading ooblayout from device tree
From: Paul Cercueil @ 2018-05-12 18:30 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Mark Rutland, David Woodhouse, linux-mtd, Rob Herring,
	linux-kernel, devicetree, Brian Norris, Richard Weinberger,
	Boris Brezillon, Marek Vasut
In-Reply-To: <20180512200219.6ded51a6@bbrezillon>



Le sam. 12 mai 2018 à 20:02, Boris Brezillon 
<boris.brezillon@bootlin.com> a écrit :
> On Sat, 12 May 2018 19:42:49 +0200
> Paul Cercueil <paul@crapouillou.net> wrote:
> 
>>  >>  >>  My motivation is to get rid of this (move it to devicetree):
>>  >>  >>
>>  >>  >>
>>  >> 
>> https://elixir.bootlin.com/linux/latest/source/arch/mips/jz4740/board-qi_lb60.c#L93
>>  >>  >>  And enable the support of other boards with custom OOB 
>> layouts.
>>  >>  >
>>  >>  > Can you list the different layouts you have? I'm pretty sure
>>  >> there's a
>>  >>  > pattern. Maybe we can even deduce the layout from the page 
>> size
>>  >> or OOB
>>  >>  > size.
>>  >>
>>  >>  This is the other layout I have for another ingenic device:
>>  >>
>>  >> 
>> http://projects.qi-hardware.com/index.php/p/qi-kernel/source/tree/od-2011-09-18/arch/mips/jz4740/board-a320.c#L125
>>  >>
>>  >>  Page size and OOB size are the same between these two devices.
>>  >
>>  > Indeed. Do you know if there are other kind of layouts in the 
>> wild?
>> 
>>  I'm getting a new board in a few weeks, I'll be able to check that 
>> out.
>> 
>>  > Note that <layout-id> can be a string, so if each each board is
>>  > defining its own layout, you could specify the board name here.
>>  > Otherwise, if you just have those 2 patterns, you can just name 
>> them
>>  > "contiguous" and "interleaved".
>> 
>>  I don't like the idea of adding board-specific data inside the 
>> driver...
>>  I'd prefer to use the method I used in this patch, but inside the
>>  jz4740-nand driver, if you're OK with it.
> 
> Please don't. Encoding such detailed description in the DT has almost
> always proven to be poor choice. Also, people are likely to get it
> wrong, and then you'll have to fix all DTs, while, with a single 
> unique
> ID representing the layout, you can
> - re-use existing layouts easily without having to describe everything
>   again in the DT
> - fix the driver without getting in trouble with people who claim
>   that DT is a stable ABI and don't want to update their DT
> 
> So, please just pick user-friendly IDs and add layout definitions in
> the driver. If you don't want to leak board info in the driver, then
> don't name the layout with the board name. BTW, I still hope you'll 
> only
> have 2 kind of layouts (contiguous and interleaved).

Alright. Thanks for the insights.

-Paul

^ permalink raw reply

* Re: [PATCH] mtd: nand: Add support for reading ooblayout from device tree
From: Boris Brezillon @ 2018-05-12 18:02 UTC (permalink / raw)
  To: Paul Cercueil
  Cc: Mark Rutland, David Woodhouse, linux-mtd, Rob Herring,
	linux-kernel, devicetree, Brian Norris, Richard Weinberger,
	Boris Brezillon, Marek Vasut
In-Reply-To: <1526146969.1594.0@smtp.crapouillou.net>

On Sat, 12 May 2018 19:42:49 +0200
Paul Cercueil <paul@crapouillou.net> wrote:

> >>  >>  My motivation is to get rid of this (move it to devicetree):
> >>  >>
> >>  >>   
> >> https://elixir.bootlin.com/linux/latest/source/arch/mips/jz4740/board-qi_lb60.c#L93  
> >>  >>  And enable the support of other boards with custom OOB layouts.  
> >>  >
> >>  > Can you list the different layouts you have? I'm pretty sure   
> >> there's a  
> >>  > pattern. Maybe we can even deduce the layout from the page size   
> >> or OOB  
> >>  > size.  
> >> 
> >>  This is the other layout I have for another ingenic device:
> >>  
> >> http://projects.qi-hardware.com/index.php/p/qi-kernel/source/tree/od-2011-09-18/arch/mips/jz4740/board-a320.c#L125
> >> 
> >>  Page size and OOB size are the same between these two devices.  
> > 
> > Indeed. Do you know if there are other kind of layouts in the wild?  
> 
> I'm getting a new board in a few weeks, I'll be able to check that out.
> 
> > Note that <layout-id> can be a string, so if each each board is
> > defining its own layout, you could specify the board name here.
> > Otherwise, if you just have those 2 patterns, you can just name them
> > "contiguous" and "interleaved".  
> 
> I don't like the idea of adding board-specific data inside the driver...
> I'd prefer to use the method I used in this patch, but inside the
> jz4740-nand driver, if you're OK with it.

Please don't. Encoding such detailed description in the DT has almost
always proven to be poor choice. Also, people are likely to get it
wrong, and then you'll have to fix all DTs, while, with a single unique
ID representing the layout, you can
- re-use existing layouts easily without having to describe everything
  again in the DT
- fix the driver without getting in trouble with people who claim
  that DT is a stable ABI and don't want to update their DT

So, please just pick user-friendly IDs and add layout definitions in
the driver. If you don't want to leak board info in the driver, then
don't name the layout with the board name. BTW, I still hope you'll only
have 2 kind of layouts (contiguous and interleaved).

^ permalink raw reply

* Re: [PATCH] mtd: nand: Add support for reading ooblayout from device tree
From: Paul Cercueil @ 2018-05-12 17:42 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Mark Rutland, David Woodhouse, linux-mtd, Rob Herring,
	linux-kernel, devicetree, Brian Norris, Richard Weinberger,
	Boris Brezillon, Marek Vasut
In-Reply-To: <20180512170034.12779313@bbrezillon>



>>  >>  My motivation is to get rid of this (move it to devicetree):
>>  >>
>>  >> 
>> https://elixir.bootlin.com/linux/latest/source/arch/mips/jz4740/board-qi_lb60.c#L93
>>  >>  And enable the support of other boards with custom OOB layouts.
>>  >
>>  > Can you list the different layouts you have? I'm pretty sure 
>> there's a
>>  > pattern. Maybe we can even deduce the layout from the page size 
>> or OOB
>>  > size.
>> 
>>  This is the other layout I have for another ingenic device:
>>  
>> http://projects.qi-hardware.com/index.php/p/qi-kernel/source/tree/od-2011-09-18/arch/mips/jz4740/board-a320.c#L125
>> 
>>  Page size and OOB size are the same between these two devices.
> 
> Indeed. Do you know if there are other kind of layouts in the wild?

I'm getting a new board in a few weeks, I'll be able to check that out.

> Note that <layout-id> can be a string, so if each each board is
> defining its own layout, you could specify the board name here.
> Otherwise, if you just have those 2 patterns, you can just name them
> "contiguous" and "interleaved".

I don't like the idea of adding board-specific data inside the driver...
I'd prefer to use the method I used in this patch, but inside the
jz4740-nand driver, if you're OK with it.

Thanks,
-Paul

^ permalink raw reply

* Re: [PATCH] mtd: nand: Add support for reading ooblayout from device tree
From: Boris Brezillon @ 2018-05-12 15:00 UTC (permalink / raw)
  To: Paul Cercueil
  Cc: Mark Rutland, David Woodhouse, linux-mtd, Rob Herring,
	linux-kernel, devicetree, Brian Norris, Richard Weinberger,
	Boris Brezillon, Marek Vasut
In-Reply-To: <1526135906.4971.0@smtp.crapouillou.net>

On Sat, 12 May 2018 11:38:26 -0300
Paul Cercueil <paul@crapouillou.net> wrote:

> Le sam. 12 mai 2018 à 10:42, Boris Brezillon 
> <boris.brezillon@bootlin.com> a écrit :
> > On Sat, 12 May 2018 08:55:40 -0300
> > Paul Cercueil <paul@crapouillou.net> wrote:
> >   
> >>  Hi Boris,
> >> 
> >>  Le 12 mai 2018 02:55, Boris Brezillon <boris.brezillon@bootlin.com> 
> >> a écrit :  
> >>  >
> >>  > Hi Paul,
> >>  >
> >>  > On Fri, 11 May 2018 23:29:12 +0200
> >>  > Paul Cercueil <paul@crapouillou.net> wrote:
> >>  >  
> >>  > > By specifying the properties "mtd-oob-ecc" and "mtd-oob-free",   
> >> it is  
> >>  > > now possible to specify from devicetree where the ECC data is   
> >> located  
> >>  > > inside the OOB region.  
> >>  >
> >>  > Why would we want to do that? I mean, ECC/free regions are ECC
> >>  > controller dependent (and NAND chip dependent for the OOB size   
> >> part),  
> >>  > so there's no reason to describe it in the DT. And more   
> >> importantly,  
> >>  > people are likely to get it wrong.
> >>  >
> >>  > I'm curious, why do you need that?  
> >> 
> >>  Good question.
> >> 
> >>  The reason is that some SoCs have no ECC controller.
> >>  The various boards for these SoCs then all use a different layout.  
> > 
> > Okay. Still think defining the layouts in the DT is a bad idea. We
> > can add a jz4740 specific property to define the layout id
> > (ingenic,nand-oob-layout = <layout-id>), but not a generic way to
> > define custom layouts for all kind of NAND controller.  
> 
> Okay.
> 
> >> 
> >>  My motivation is to get rid of this (move it to devicetree):
> >>  
> >> https://elixir.bootlin.com/linux/latest/source/arch/mips/jz4740/board-qi_lb60.c#L93
> >>  And enable the support of other boards with custom OOB layouts.  
> > 
> > Can you list the different layouts you have? I'm pretty sure there's a
> > pattern. Maybe we can even deduce the layout from the page size or OOB
> > size.  
> 
> This is the other layout I have for another ingenic device:
> http://projects.qi-hardware.com/index.php/p/qi-kernel/source/tree/od-2011-09-18/arch/mips/jz4740/board-a320.c#L125
> 
> Page size and OOB size are the same between these two devices.

Indeed. Do you know if there are other kind of layouts in the wild?
Note that <layout-id> can be a string, so if each each board is
defining its own layout, you could specify the board name here.
Otherwise, if you just have those 2 patterns, you can just name them
"contiguous" and "interleaved".

^ permalink raw reply

* Re: [PATCH] mtd: nand: Add support for reading ooblayout from device tree
From: Paul Cercueil @ 2018-05-12 14:38 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Mark Rutland, David Woodhouse, linux-mtd, Rob Herring,
	linux-kernel, devicetree, Brian Norris, Richard Weinberger,
	Boris Brezillon, Marek Vasut
In-Reply-To: <20180512154223.3b81c7f7@bbrezillon>



Le sam. 12 mai 2018 à 10:42, Boris Brezillon 
<boris.brezillon@bootlin.com> a écrit :
> On Sat, 12 May 2018 08:55:40 -0300
> Paul Cercueil <paul@crapouillou.net> wrote:
> 
>>  Hi Boris,
>> 
>>  Le 12 mai 2018 02:55, Boris Brezillon <boris.brezillon@bootlin.com> 
>> a écrit :
>>  >
>>  > Hi Paul,
>>  >
>>  > On Fri, 11 May 2018 23:29:12 +0200
>>  > Paul Cercueil <paul@crapouillou.net> wrote:
>>  >
>>  > > By specifying the properties "mtd-oob-ecc" and "mtd-oob-free", 
>> it is
>>  > > now possible to specify from devicetree where the ECC data is 
>> located
>>  > > inside the OOB region.
>>  >
>>  > Why would we want to do that? I mean, ECC/free regions are ECC
>>  > controller dependent (and NAND chip dependent for the OOB size 
>> part),
>>  > so there's no reason to describe it in the DT. And more 
>> importantly,
>>  > people are likely to get it wrong.
>>  >
>>  > I'm curious, why do you need that?
>> 
>>  Good question.
>> 
>>  The reason is that some SoCs have no ECC controller.
>>  The various boards for these SoCs then all use a different layout.
> 
> Okay. Still think defining the layouts in the DT is a bad idea. We
> can add a jz4740 specific property to define the layout id
> (ingenic,nand-oob-layout = <layout-id>), but not a generic way to
> define custom layouts for all kind of NAND controller.

Okay.

>> 
>>  My motivation is to get rid of this (move it to devicetree):
>>  
>> https://elixir.bootlin.com/linux/latest/source/arch/mips/jz4740/board-qi_lb60.c#L93
>>  And enable the support of other boards with custom OOB layouts.
> 
> Can you list the different layouts you have? I'm pretty sure there's a
> pattern. Maybe we can even deduce the layout from the page size or OOB
> size.

This is the other layout I have for another ingenic device:
http://projects.qi-hardware.com/index.php/p/qi-kernel/source/tree/od-2011-09-18/arch/mips/jz4740/board-a320.c#L125

Page size and OOB size are the same between these two devices.

-Paul

^ permalink raw reply

* [PATCH] clk: rockchip: remove deprecated gate-clk code and dt-binding
From: Heiko Stuebner @ 2018-05-12 14:30 UTC (permalink / raw)
  To: mturquette-rdvid1DuHRBWk0Htik3J/w, sboyd-DgEjT+Ai2ygdnm+yROfE0A
  Cc: mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
	Heiko Stuebner, linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, linux-clk-u79uwXL29TY76Z2rM5mHXA

Initially we tried modeling clocks via the devicetree before switching
to clocks declared in the clock drivers and only exporting specific
ids to the devicetree.

As the old code was in the kernel for 1-2 releases when the new mode
of operation was added we kept it for backwards compatibility.

That deprecation notice is in the binding since july 2014, so nearly
4 years now and I think it's time to drop the old cruft.

Especially as at the time using the mainline kernel on Rockchip devices
was not really possible, except for experiments on the really old socs of
the rk3066 + rk3188 line, so there shouldn't be any devicetrees still
around that rely on that code.

Signed-off-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
---
 .../devicetree/bindings/clock/rockchip.txt    | 77 ---------------
 drivers/clk/rockchip/Makefile                 |  1 -
 drivers/clk/rockchip/clk-rockchip.c           | 98 -------------------
 3 files changed, 176 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/clock/rockchip.txt
 delete mode 100644 drivers/clk/rockchip/clk-rockchip.c

diff --git a/Documentation/devicetree/bindings/clock/rockchip.txt b/Documentation/devicetree/bindings/clock/rockchip.txt
deleted file mode 100644
index 22f6769e5d4a..000000000000
--- a/Documentation/devicetree/bindings/clock/rockchip.txt
+++ /dev/null
@@ -1,77 +0,0 @@
-Device Tree Clock bindings for arch-rockchip
-
-This binding uses the common clock binding[1].
-
-[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
-
-== Gate clocks ==
-
-These bindings are deprecated!
-Please use the soc specific CRU bindings instead.
-
-The gate registers form a continuos block which makes the dt node
-structure a matter of taste, as either all gates can be put into
-one gate clock spanning all registers or they can be divided into
-the 10 individual gates containing 16 clocks each.
-The code supports both approaches.
-
-Required properties:
-- compatible : "rockchip,rk2928-gate-clk"
-- reg : shall be the control register address(es) for the clock.
-- #clock-cells : from common clock binding; shall be set to 1
-- clock-output-names : the corresponding gate names that the clock controls
-- clocks : should contain the parent clock for each individual gate,
-  therefore the number of clocks elements should match the number of
-  clock-output-names
-
-Example using multiple gate clocks:
-
-		clk_gates0: gate-clk@200000d0 {
-			compatible = "rockchip,rk2928-gate-clk";
-			reg = <0x200000d0 0x4>;
-			clocks = <&dummy>, <&dummy>,
-				 <&dummy>, <&dummy>,
-				 <&dummy>, <&dummy>,
-				 <&dummy>, <&dummy>,
-				 <&dummy>, <&dummy>,
-				 <&dummy>, <&dummy>,
-				 <&dummy>, <&dummy>,
-				 <&dummy>, <&dummy>;
-
-			clock-output-names =
-				"gate_core_periph", "gate_cpu_gpll",
-				"gate_ddrphy", "gate_aclk_cpu",
-				"gate_hclk_cpu", "gate_pclk_cpu",
-				"gate_atclk_cpu", "gate_i2s0",
-				"gate_i2s0_frac", "gate_i2s1",
-				"gate_i2s1_frac", "gate_i2s2",
-				"gate_i2s2_frac", "gate_spdif",
-				"gate_spdif_frac", "gate_testclk";
-
-			#clock-cells = <1>;
-		};
-
-		clk_gates1: gate-clk@200000d4 {
-			compatible = "rockchip,rk2928-gate-clk";
-			reg = <0x200000d4 0x4>;
-			clocks = <&xin24m>, <&xin24m>,
-				 <&xin24m>, <&dummy>,
-				 <&dummy>, <&xin24m>,
-				 <&xin24m>, <&dummy>,
-				 <&xin24m>, <&dummy>,
-				 <&xin24m>, <&dummy>,
-				 <&xin24m>, <&dummy>,
-				 <&xin24m>, <&dummy>;
-
-			clock-output-names =
-				"gate_timer0", "gate_timer1",
-				"gate_timer2", "gate_jtag",
-				"gate_aclk_lcdc1_src", "gate_otgphy0",
-				"gate_otgphy1", "gate_ddr_gpll",
-				"gate_uart0", "gate_frac_uart0",
-				"gate_uart1", "gate_frac_uart1",
-				"gate_uart2", "gate_frac_uart2",
-				"gate_uart3", "gate_frac_uart3";
-
-			#clock-cells = <1>;
-		};
diff --git a/drivers/clk/rockchip/Makefile b/drivers/clk/rockchip/Makefile
index 59b8d320960a..98e7b9429b83 100644
--- a/drivers/clk/rockchip/Makefile
+++ b/drivers/clk/rockchip/Makefile
@@ -3,7 +3,6 @@
 # Rockchip Clock specific Makefile
 #
 
-obj-y	+= clk-rockchip.o
 obj-y	+= clk.o
 obj-y	+= clk-pll.o
 obj-y	+= clk-cpu.o
diff --git a/drivers/clk/rockchip/clk-rockchip.c b/drivers/clk/rockchip/clk-rockchip.c
deleted file mode 100644
index 2c9bb81144c9..000000000000
--- a/drivers/clk/rockchip/clk-rockchip.c
+++ /dev/null
@@ -1,98 +0,0 @@
-/*
- * Copyright (c) 2013 MundoReader S.L.
- * Author: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- */
-
-#include <linux/clk-provider.h>
-#include <linux/clkdev.h>
-#include <linux/of.h>
-#include <linux/of_address.h>
-
-static DEFINE_SPINLOCK(clk_lock);
-
-/*
- * Gate clocks
- */
-
-static void __init rk2928_gate_clk_init(struct device_node *node)
-{
-	struct clk_onecell_data *clk_data;
-	const char *clk_parent;
-	const char *clk_name;
-	void __iomem *reg;
-	void __iomem *reg_idx;
-	int flags;
-	int qty;
-	int reg_bit;
-	int clkflags = CLK_SET_RATE_PARENT;
-	int i;
-
-	qty = of_property_count_strings(node, "clock-output-names");
-	if (qty < 0) {
-		pr_err("%s: error in clock-output-names %d\n", __func__, qty);
-		return;
-	}
-
-	if (qty == 0) {
-		pr_info("%s: nothing to do\n", __func__);
-		return;
-	}
-
-	reg = of_iomap(node, 0);
-	if (!reg)
-		return;
-
-	clk_data = kzalloc(sizeof(struct clk_onecell_data), GFP_KERNEL);
-	if (!clk_data) {
-		iounmap(reg);
-		return;
-	}
-
-	clk_data->clks = kzalloc(qty * sizeof(struct clk *), GFP_KERNEL);
-	if (!clk_data->clks) {
-		kfree(clk_data);
-		iounmap(reg);
-		return;
-	}
-
-	flags = CLK_GATE_HIWORD_MASK | CLK_GATE_SET_TO_DISABLE;
-
-	for (i = 0; i < qty; i++) {
-		of_property_read_string_index(node, "clock-output-names",
-					      i, &clk_name);
-
-		/* ignore empty slots */
-		if (!strcmp("reserved", clk_name))
-			continue;
-
-		clk_parent = of_clk_get_parent_name(node, i);
-
-		/* keep all gates untouched for now */
-		clkflags |= CLK_IGNORE_UNUSED;
-
-		reg_idx = reg + (4 * (i / 16));
-		reg_bit = (i % 16);
-
-		clk_data->clks[i] = clk_register_gate(NULL, clk_name,
-						      clk_parent, clkflags,
-						      reg_idx, reg_bit,
-						      flags,
-						      &clk_lock);
-		WARN_ON(IS_ERR(clk_data->clks[i]));
-	}
-
-	clk_data->clk_num = qty;
-
-	of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
-}
-CLK_OF_DECLARE(rk2928_gate, "rockchip,rk2928-gate-clk", rk2928_gate_clk_init);
-- 
2.17.0

^ permalink raw reply related

* Re: [PATCH 1/2] dt-bindings: arm: document Renesas V3HSK board bindings
From: Simon Horman @ 2018-05-12 14:12 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: Rob Herring, linux-renesas-soc, devicetree, Magnus Damm,
	Mark Rutland
In-Reply-To: <24039c74-d6d3-cac3-6c68-5dcd88ef8ecc@cogentembedded.com>

On Thu, May 10, 2018 at 09:09:40PM +0300, Sergei Shtylyov wrote:
> Document the V3H Starter Kit device tree bindings, listing it as
> a supported board.
> 
> This allows to use checkpatch.pl to validate .dts files referring to
> the V3HSK board.
> 
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

This looks fine but I will wait to see if there are other reviews before
applying.

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

> 
> ---
>  Documentation/devicetree/bindings/arm/shmobile.txt |    2 ++
>  1 file changed, 2 insertions(+)
> 
> Index: renesas/Documentation/devicetree/bindings/arm/shmobile.txt
> ===================================================================
> --- renesas.orig/Documentation/devicetree/bindings/arm/shmobile.txt
> +++ renesas/Documentation/devicetree/bindings/arm/shmobile.txt
> @@ -132,6 +132,8 @@ Boards:
>      compatible = "renesas,sk-rzg1m", "renesas,r8a7743"
>    - Stout (ADAS Starterkit, Y-R-CAR-ADAS-SKH2-BOARD)
>      compatible = "renesas,stout", "renesas,r8a7790"
> +  - V3HSK (Y-ASK-RCAR-V3H-WS10)
> +    compatible = "renesas,v3hsk", "renesas,r8a77980"
>    - V3MSK (Y-ASK-RCAR-V3M-WS10)
>      compatible = "renesas,v3msk", "renesas,r8a77970"
>    - Wheat (RTP0RC7792ASKB0000JE)
> 

^ permalink raw reply

* Re: [PATCH v1 04/13] soc: rockchip: power-domain: Fix wrong value when power up pd
From: Heiko Stuebner @ 2018-05-12 14:11 UTC (permalink / raw)
  To: Elaine Zhang
  Cc: robh+dt, mark.rutland, devicetree, rjw, khilman, ulf.hansson,
	linux-pm, linux-arm-kernel, linux-rockchip, linux-kernel, wxt,
	xxx, xf, huangtao, Finley Xiao
In-Reply-To: <1526009458-28579-5-git-send-email-zhangqing@rock-chips.com>

Hi Elaine,

Am Freitag, 11. Mai 2018, 05:30:49 CEST schrieb Elaine Zhang:

please provide a patch description. This seems to affect rk3328
alone right now, but seems that rk3328 could only ever turn off
power-domains but never turn them on again, right?

Authorship comment from my previous comment applies here
as well.

And please add a

Fixes: 9bb17ce8edb3 ("soc: rockchip: power-domain: Support domain control in hiword-registers")
Cc: stable@vger.kernel.org

to the patch description.


Thanks
Heiko

> Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
> Signed-off-by: Finley Xiao <finley.xiao@rock-chips.com>
> ---
>  drivers/soc/rockchip/pm_domains.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/soc/rockchip/pm_domains.c b/drivers/soc/rockchip/pm_domains.c
> index ebd7c41898c0..01d4ba26a054 100644
> --- a/drivers/soc/rockchip/pm_domains.c
> +++ b/drivers/soc/rockchip/pm_domains.c
> @@ -264,7 +264,7 @@ static void rockchip_do_pmu_set_power_domain(struct rockchip_pm_domain *pd,
>  		return;
>  	else if (pd->info->pwr_w_mask)
>  		regmap_write(pmu->regmap, pmu->info->pwr_offset,
> -			     on ? pd->info->pwr_mask :
> +			     on ? pd->info->pwr_w_mask :
>  			     (pd->info->pwr_mask | pd->info->pwr_w_mask));
>  	else
>  		regmap_update_bits(pmu->regmap, pmu->info->pwr_offset,
> 

^ permalink raw reply

* Re: [PATCH] arm64: dts: renesas: r8a77970: add SMP support
From: Simon Horman @ 2018-05-12 14:08 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: Mark Rutland,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Magnus Damm, Catalin Marinas, Will Deacon, Linux-Renesas,
	Rob Herring, Geert Uytterhoeven, Linux ARM
In-Reply-To: <110ba99c-1a8d-98a1-3ca0-5fc99e79572b@cogentembedded.com>

On Thu, May 10, 2018 at 07:43:03PM +0300, Sergei Shtylyov wrote:
> Hello!
> 
> On 05/09/2018 10:05 PM, Simon Horman wrote:
> 
> >>>> Add the device node for the second Cortex-A53 CPU core.
> >>>>
> >>>> Based on the original (and large) patch by Daisuke Matsushita
> >>>> <daisuke.matsushita.ns@hitachi.com>.
> >>>>
> >>>> Signed-off-by: Vladimir Barinov <vladimir.barinov@cogentembedded.com>
> >>>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> >>>
> >>> Dupe of https://patchwork.kernel.org/patch/10032875/
> >>
> >>    Sorry!
> >>    Not an exact dupe, though -- mine has clock/power #define's used,
> >> yours -- only bare #s. :-)
> >>
> >>> From series "[PATCH 0/2] arm64: dts: renesas: r8a77970: Add SMP Support"
> >>> (https://www.spinics.net/lists/linux-renesas-soc/msg19681.html)
> >>
> >>    Hmm... what's the fate of this series?
> > 
> > There is now a v2 of Geert's series which incorporates your enhancements.
> 
>    I suggested to respin it. :-)

Thanks, that would have been my suggestion too :)

> > I will apply that.
> 
>    Thank you.
>    For the record, I had better luck than Geert testing SMP on Eagle:
>    only CPU0 couldn't be offlined (and I was unable to find a
>    workaround), others could be on/ offlined w/o issues. As for
>    suspend/resume -- it did work but I could only test s2idle
>    (/sys/power/mem_sleep had no other variants)...

Thanks. Perhaps you have a more recent firmware than Geert.
I would expect the CPU0 issue you describe will be resolved in time in
the firmware.

^ permalink raw reply

* Re: [PATCH] mtd: nand: Add support for reading ooblayout from device tree
From: Boris Brezillon @ 2018-05-12 13:42 UTC (permalink / raw)
  To: Paul Cercueil
  Cc: Mark Rutland, David Woodhouse, linux-mtd, Rob Herring,
	linux-kernel, devicetree, Brian Norris, Richard Weinberger,
	Boris Brezillon, Marek Vasut
In-Reply-To: <20180512115551.56C6E20787@mail.bootlin.com>

On Sat, 12 May 2018 08:55:40 -0300
Paul Cercueil <paul@crapouillou.net> wrote:

> Hi Boris,
> 
> Le 12 mai 2018 02:55, Boris Brezillon <boris.brezillon@bootlin.com> a écrit :
> >
> > Hi Paul, 
> >
> > On Fri, 11 May 2018 23:29:12 +0200 
> > Paul Cercueil <paul@crapouillou.net> wrote: 
> >  
> > > By specifying the properties "mtd-oob-ecc" and "mtd-oob-free", it is 
> > > now possible to specify from devicetree where the ECC data is located 
> > > inside the OOB region.   
> >
> > Why would we want to do that? I mean, ECC/free regions are ECC 
> > controller dependent (and NAND chip dependent for the OOB size part), 
> > so there's no reason to describe it in the DT. And more importantly, 
> > people are likely to get it wrong. 
> >
> > I'm curious, why do you need that?   
> 
> Good question.
> 
> The reason is that some SoCs have no ECC controller.
> The various boards for these SoCs then all use a different layout.

Okay. Still think defining the layouts in the DT is a bad idea. We
can add a jz4740 specific property to define the layout id
(ingenic,nand-oob-layout = <layout-id>), but not a generic way to
define custom layouts for all kind of NAND controller.

> 
> My motivation is to get rid of this (move it to devicetree):
> https://elixir.bootlin.com/linux/latest/source/arch/mips/jz4740/board-qi_lb60.c#L93
> And enable the support of other boards with custom OOB layouts.

Can you list the different layouts you have? I'm pretty sure there's a
pattern. Maybe we can even deduce the layout from the page size or OOB
size.

^ permalink raw reply

* [PATCH V3] ARM: dts: da850-evm: add WP and CD to MMC
From: Adam Ford @ 2018-05-12 12:23 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: Adam Ford, devicetree, nsekhar, khilman

The legacy mode uses the write protect and card detect but DT does not.
This patch enables Write-Protect and Card-Detect pins for the MMC card, and
the gpio-ranges property sets the gpio pinmuxing for those respective pins.

Signed-off-by: Adam Ford <aford173@gmail.com>
--

V3: Update description of the patch. Rebase on davinci 4.18/dt
V2: Rebase on davinci master and remove GPIO pinmuxing

diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts
index e91f81f21b22..2e817da37fdb 100644
--- a/arch/arm/boot/dts/da850-evm.dts
+++ b/arch/arm/boot/dts/da850-evm.dts
@@ -149,6 +149,8 @@
 	status = "okay";
 	pinctrl-names = "default";
 	pinctrl-0 = <&mmc0_pins>;
+	cd-gpios = <&gpio 64 GPIO_ACTIVE_LOW>;
+	wp-gpios = <&gpio 65 GPIO_ACTIVE_HIGH>;
 };
 
 &spi1 {
-- 
2.17.0

^ permalink raw reply related

* Re: [PATCH] mtd: nand: Add support for reading ooblayout from device tree
From: Paul Cercueil @ 2018-05-12 11:55 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Mark Rutland, David Woodhouse, linux-mtd, Rob Herring,
	linux-kernel, devicetree, Brian Norris, Richard Weinberger,
	Boris Brezillon, Marek Vasut

Hi Boris,

Le 12 mai 2018 02:55, Boris Brezillon <boris.brezillon@bootlin.com> a écrit :
>
> Hi Paul, 
>
> On Fri, 11 May 2018 23:29:12 +0200 
> Paul Cercueil <paul@crapouillou.net> wrote: 
>
> > By specifying the properties "mtd-oob-ecc" and "mtd-oob-free", it is 
> > now possible to specify from devicetree where the ECC data is located 
> > inside the OOB region. 
>
> Why would we want to do that? I mean, ECC/free regions are ECC 
> controller dependent (and NAND chip dependent for the OOB size part), 
> so there's no reason to describe it in the DT. And more importantly, 
> people are likely to get it wrong. 
>
> I'm curious, why do you need that? 

Good question.

The reason is that some SoCs have no ECC controller.
The various boards for these SoCs then all use a different layout.

My motivation is to get rid of this (move it to devicetree):
https://elixir.bootlin.com/linux/latest/source/arch/mips/jz4740/board-qi_lb60.c#L93
And enable the support of other boards with custom OOB layouts.

> Regards, 
>
> Boris 

Thanks,
-Paul

> > 
> > Signed-off-by: Paul Cercueil <paul@crapouillou.net> 
> > --- 
> >  Documentation/devicetree/bindings/mtd/nand.txt |  7 +++++ 
> >  drivers/mtd/nand/raw/nand_base.c               | 42 ++++++++++++++++++++++++++ 
> >  2 files changed, 49 insertions(+) 
> > 
> > diff --git a/Documentation/devicetree/bindings/mtd/nand.txt b/Documentation/devicetree/bindings/mtd/nand.txt 
> > index 8bb11d809429..118ea92787cb 100644 
> > --- a/Documentation/devicetree/bindings/mtd/nand.txt 
> > +++ b/Documentation/devicetree/bindings/mtd/nand.txt 
> > @@ -45,6 +45,13 @@ Optional NAND chip properties: 
> >       as reliable as possible. 
> >  - nand-rb: shall contain the native Ready/Busy ids. 
> >  
> > +- nand-oob-ecc: <offset, length> couples of integers, specifying the offset 
> > +      and length of the ECC data in the OOB region. There can be more 
> > +      than one couple. 
> > +- nand-oob-free: <offset, length> couples of integers, specifying the offset 
> > +      and length of a free-to-use area in the OOB region. There can be 
> > +      more than one couple. 
> > + 
> >  The ECC strength and ECC step size properties define the correction capability 
> >  of a controller. Together, they say a controller can correct "{strength} bit 
> >  errors per {size} bytes". 
> > diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c 
> > index 72f3a89da513..c905531effb0 100644 
> > --- a/drivers/mtd/nand/raw/nand_base.c 
> > +++ b/drivers/mtd/nand/raw/nand_base.c 
> > @@ -213,6 +213,43 @@ static const struct mtd_ooblayout_ops nand_ooblayout_lp_hamming_ops = { 
> >  .free = nand_ooblayout_free_lp_hamming, 
> >  }; 
> >  
> > +static int nand_oob_of(struct device_node *np, int section, 
> > +        struct mtd_oob_region *oobregion, const char *prop) 
> > +{ 
> > + int ret = of_property_read_u32_index(np, prop, 
> > + section * 2, &oobregion->offset); 
> > + if (ret == -EOVERFLOW) 
> > + return -ERANGE; /* We're done */ 
> > + if (ret) 
> > + return ret; 
> > + 
> > + ret = of_property_read_u32_index(np, prop, 
> > + section * 2 + 1, &oobregion->length); 
> > + if (ret == -EOVERFLOW) 
> > + return -EINVAL; /* We must have an even number of integers */ 
> > + 
> > + return ret; 
> > +} 
> > + 
> > +static int nand_ooblayout_ecc_of(struct mtd_info *mtd, int section, 
> > + struct mtd_oob_region *oobregion) 
> > +{ 
> > + return nand_oob_of(mtd->dev.of_node, section, 
> > + oobregion, "nand-oob-ecc"); 
> > +} 
> > + 
> > +static int nand_ooblayout_free_of(struct mtd_info *mtd, int section, 
> > + struct mtd_oob_region *oobregion) 
> > +{ 
> > + return nand_oob_of(mtd->dev.of_node, section, 
> > + oobregion, "nand-oob-free"); 
> > +} 
> > + 
> > +static const struct mtd_ooblayout_ops nand_ooblayout_of_ops = { 
> > + .ecc = nand_ooblayout_ecc_of, 
> > + .free = nand_ooblayout_free_of, 
> > +}; 
> > + 
> >  static int check_offs_len(struct mtd_info *mtd, 
> >  loff_t ofs, uint64_t len) 
> >  { 
> > @@ -5843,6 +5880,11 @@ static int nand_dt_init(struct nand_chip *chip) 
> >  if (of_property_read_bool(dn, "nand-ecc-maximize")) 
> >  chip->ecc.options |= NAND_ECC_MAXIMIZE; 
> >  
> > + if (!chip->mtd.ooblayout && 
> > + of_property_read_bool(dn, "nand-oob-ecc") && 
> > + of_property_read_bool(dn, "nand-oob-free")) 
> > + chip->mtd.ooblayout = &nand_ooblayout_of_ops; 
> > + 
> >  return 0; 
> >  } 
> >  
>

^ permalink raw reply

* Re: [PATCH v6 3/6] kernel/reboot.c: export pm_power_off_prepare
From: Rafael J. Wysocki @ 2018-05-12 11:13 UTC (permalink / raw)
  To: Oleksij Rempel
  Cc: Andrew Morton, kernel, devicetree, linux-arm-kernel, linux-clk,
	linux-kernel, Liam Girdwood, Leonard Crestez, Rob Herring,
	Mark Rutland, Michael Turquette, Stephen Boyd, Fabio Estevam,
	Russell King, Shawn Guo, Mark Brown
In-Reply-To: <20180504185052.bjhfkvpgrblc7tfc@pengutronix.de>

On Friday, May 4, 2018 8:50:52 PM CEST Oleksij Rempel wrote:
> Hallo Andrew,
> I need your ACK or NACK for this patch.
> 
> This function is used to configure external PMIC to interpret
> signal which will be triggered by pm_power_off as power off.
> Since same signal can be used for stand by, I linked PMIC configuration
> with pm_power_off_prepare to avoid possible conflicts.
> 
> On Mon, Mar 05, 2018 at 11:25:20AM +0100, Oleksij Rempel wrote:
> > Export pm_power_off_prepare. It is needed to implement power off on
> > Freescale/NXP iMX6 based boards with external power management
> > integrated circuit (PMIC).
> > 
> > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > ---
> >  kernel/reboot.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/kernel/reboot.c b/kernel/reboot.c
> > index e4ced883d8de..350be6baa60d 100644
> > --- a/kernel/reboot.c
> > +++ b/kernel/reboot.c
> > @@ -49,6 +49,7 @@ int reboot_force;
> >   */
> >  
> >  void (*pm_power_off_prepare)(void);
> > +EXPORT_SYMBOL(pm_power_off_prepare);

Why not EXPORT_SYMBOL_GPL() ?

> >  
> >  /**
> >   *	emergency_restart - reboot the system
> 
> 

^ permalink raw reply

* Re: [PATCH 2/2] iio: adc: Add Qualcomm SPMI PMIC5 ADC driver
From: Jonathan Cameron @ 2018-05-12 10:56 UTC (permalink / raw)
  To: Siddartha Mohanadoss
  Cc: linux-iio, linux-arm-msm, devicetree, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Rob Herring, cdevired,
	rphani, sivaa
In-Reply-To: <1525815536-27781-1-git-send-email-smohanad@codeaurora.org>

On Tue,  8 May 2018 14:38:56 -0700
Siddartha Mohanadoss <smohanad@codeaurora.org> wrote:

> This patch adds support for Qualcomm SPMI PMIC5 family
> of ADC driver that supports hardware based offset and
> gain compensation. The ADC peripheral can measure both
> voltage and current channels whose input signal is
> connected to the PMIC.
> 
> The register set and configuration has been refreshed
> compared to the prior Qualcomm PMIC ADC family. Register
> ADC5 as part of the IIO framework.
> 
> Signed-off-by: Siddartha Mohanadoss <smohanad@codeaurora.org>

Various minor comments inline.

Jonathan

> ---
>  drivers/iio/adc/Kconfig                  |  18 +
>  drivers/iio/adc/Makefile                 |   1 +
>  drivers/iio/adc/qcom-spmi-adc5.c         | 818 +++++++++++++++++++++++++++++++
>  drivers/iio/adc/qcom-vadc-common.c       | 241 +++++++++
>  drivers/iio/adc/qcom-vadc-common.h       |  51 ++
>  include/dt-bindings/iio/qcom,spmi-vadc.h | 115 ++++-
>  6 files changed, 1243 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/iio/adc/qcom-spmi-adc5.c
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 15606f2..ba861a1 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -567,6 +567,24 @@ config QCOM_PM8XXX_XOADC
>  	  To compile this driver as a module, choose M here: the module
>  	  will be called qcom-pm8xxx-xoadc.
>  
> +config QCOM_SPMI_ADC5
> +	tristate "Qualcomm Technologies Inc. SPMI PMIC5 ADC"
> +	depends on SPMI
> +	select REGMAP_SPMI
> +	select QCOM_VADC_COMMON
> +	help
> +	  This is the IIO Voltage PMIC5 ADC driver for Qualcomm Technologies Inc.
> +
> +	  The driver supports multiple channels read. The ADC is a 16-bit
> +	  sigma-delta ADC. The hardware supports calibrated results for
> +	  conversion requests and clients include reading voltage phone
> +	  power, on board system thermistors connected to the PMIC ADC,
> +	  PMIC die temperature, charger temperature, battery current, USB voltage
> +	  input, voltage signals connected to supported PMIC GPIO inputs.
> +
> +	  To compile this driver as a module, choose M here: the module will
> +	  be called qcom-spmi-adc5.
> +
>  config QCOM_SPMI_IADC
>  	tristate "Qualcomm SPMI PMIC current ADC"
>  	depends on SPMI
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index 28a9423..30ceb65 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -55,6 +55,7 @@ obj-$(CONFIG_NAU7802) += nau7802.o
>  obj-$(CONFIG_PALMAS_GPADC) += palmas_gpadc.o
>  obj-$(CONFIG_QCOM_SPMI_IADC) += qcom-spmi-iadc.o
>  obj-$(CONFIG_QCOM_VADC_COMMON) += qcom-vadc-common.o
> +obj-$(CONFIG_QCOM_SPMI_ADC5) += qcom-spmi-adc5.o
>  obj-$(CONFIG_QCOM_SPMI_VADC) += qcom-spmi-vadc.o
>  obj-$(CONFIG_QCOM_PM8XXX_XOADC) += qcom-pm8xxx-xoadc.o
>  obj-$(CONFIG_RCAR_GYRO_ADC) += rcar-gyroadc.o
> diff --git a/drivers/iio/adc/qcom-spmi-adc5.c b/drivers/iio/adc/qcom-spmi-adc5.c
> new file mode 100644
> index 0000000..e6bc584
> --- /dev/null
> +++ b/drivers/iio/adc/qcom-spmi-adc5.c
> @@ -0,0 +1,818 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*

Don't normally have both SPDX and a licence statement...

> + * Copyright (c) 2018, The Linux Foundation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/completion.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/iio/iio.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/math64.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +#include <linux/log2.h>
> +
> +#include <dt-bindings/iio/qcom,spmi-vadc.h>
> +
> +#include "qcom-vadc-common.h"
> +
> +#define ADC_USR_STATUS1				0x8

I'd prefer a more unique prefix on these. Even ADC5 as you
have used elsewhere would be an improvement an cut down
on potential clashes some tiem in teh future.

> +#define ADC_USR_STATUS1_REQ_STS			BIT(1)
> +#define ADC_USR_STATUS1_EOC			BIT(0)
> +#define ADC_USR_STATUS1_REQ_STS_EOC_MASK	0x3
> +
> +#define ADC_USR_STATUS2				0x9
> +#define ADC_USR_STATUS2_CONV_SEQ_MASK		0x70
> +#define ADC_USR_STATUS2_CONV_SEQ_MASK_SHIFT	0x5
> +
> +#define ADC_USR_IBAT_MEAS			0xf
> +#define ADC_USR_IBAT_MEAS_SUPPORTED		BIT(0)
> +
> +#define ADC_USR_DIG_PARAM			0x42
> +#define ADC_USR_DIG_PARAM_CAL_VAL		BIT(6)
> +#define ADC_USR_DIG_PARAM_CAL_VAL_SHIFT		6
> +#define ADC_USR_DIG_PARAM_CAL_SEL		0x30
> +#define ADC_USR_DIG_PARAM_CAL_SEL_SHIFT		4
> +#define ADC_USR_DIG_PARAM_DEC_RATIO_SEL		0xc
> +#define ADC_USR_DIG_PARAM_DEC_RATIO_SEL_SHIFT	2
> +
> +#define ADC_USR_FAST_AVG_CTL			0x43
> +#define ADC_USR_FAST_AVG_CTL_EN			BIT(7)
> +#define ADC_USR_FAST_AVG_CTL_SAMPLES_MASK	0x7
> +
> +#define ADC_USR_CH_SEL_CTL			0x44
> +
> +#define ADC_USR_DELAY_CTL			0x45
> +#define ADC_USR_HW_SETTLE_DELAY_MASK		0xf
> +
> +#define ADC_USR_EN_CTL1				0x46
> +#define ADC_USR_EN_CTL1_ADC_EN			BIT(7)
> +
> +#define ADC_USR_CONV_REQ			0x47
> +#define ADC_USR_CONV_REQ_REQ			BIT(7)
> +
> +#define ADC_USR_DATA0				0x50
> +
> +#define ADC_USR_DATA1				0x51
> +
> +#define ADC_USR_IBAT_DATA0			0x52
> +
> +#define ADC_USR_IBAT_DATA1			0x53
> +
> +/*
> + * Conversion time varies based on the decimation, clock rate, fast average
> + * samples and measurements queued across different VADC peripherals.
> + * Set the timeout to a max of 100ms.
> + */
> +#define ADC_CONV_TIME_MIN_US			263
> +#define ADC_CONV_TIME_MAX_US			264
> +#define ADC_CONV_TIME_RETRY			400
> +#define ADC_CONV_TIMEOUT			msecs_to_jiffies(100)
> +
> +enum adc_cal_method {
> +	ADC_NO_CAL = 0,
> +	ADC_RATIOMETRIC_CAL,
> +	ADC_ABSOLUTE_CAL
> +};
> +
> +enum adc_cal_val {
> +	ADC_TIMER_CAL = 0,
> +	ADC_NEW_CAL
> +};
> +
> +/**
> + * struct adc_channel_prop - ADC channel property.
> + * @channel: channel number, refer to the channel list.
> + * @cal_method: calibration method.
> + * @cal_val: calibration value
> + * @decimation: sampling rate supported for the channel.
> + * @prescale: channel scaling performed on the input signal.
> + * @hw_settle_time: the time between AMUX being configured and the
> + *	start of conversion.
> + * @avg_samples: ability to provide single result from the ADC
> + *	that is an average of multiple measurements.
> + * @scale_fn_type: Represents the scaling function to convert voltage
> + *	physical units desired by the client for the channel.
> + * @datasheet_name: Channel name used in device tree.
> + */
> +struct adc_channel_prop {
> +	unsigned int		channel;
> +	enum adc_cal_method	cal_method;
> +	enum adc_cal_val	cal_val;
> +	unsigned int		decimation;
> +	unsigned int		prescale;
> +	unsigned int		hw_settle_time;
> +	unsigned int		avg_samples;
> +	enum vadc_scale_fn_type	scale_fn_type;
> +	const char		*datasheet_name;
> +};
> +
> +/**
> + * struct adc_chip - ADC private structure.
> + * @regmap: pointer to struct regmap.
> + * @dev: pointer to struct device.

The two above aren't really all that informative, anything
more relevant you could put?

> + * @base: base address for the ADC peripheral.
> + * @nchannels: number of ADC channels.
> + * @chan_props: array of ADC channel properties.
> + * @iio_chans: array of IIO channels specification.
> + * @poll_eoc: use polling instead of interrupt.
> + * @complete: ADC result notification after interrupt is received.
> + * @lock: ADC lock for access to the peripheral.
> + * @data: software configuration data.
> + */
> +struct adc_chip {
> +	struct regmap		*regmap;
> +	struct device		*dev;
> +	u16			base;
> +	unsigned int		nchannels;
> +	struct adc_channel_prop	*chan_props;
> +	struct iio_chan_spec	*iio_chans;
> +	bool			poll_eoc;
> +	struct completion	complete;
> +	struct mutex		lock;
> +	const struct adc_data	*data;
> +};
> +
> +static const struct vadc_prescale_ratio adc_prescale_ratios[] = {
> +	{.num =  1, .den =  1},
> +	{.num =  1, .den =  3},
> +	{.num =  1, .den =  4},
> +	{.num =  1, .den =  6},
> +	{.num =  1, .den = 20},
> +	{.num =  1, .den =  8},
> +	{.num = 10, .den = 81},
> +	{.num =  1, .den = 10},
> +	{.num =  1, .den = 16}
> +};
> +
> +static int adc_read(struct adc_chip *adc, u16 offset, u8 *data, int len)
> +{
> +	return regmap_bulk_read(adc->regmap, adc->base + offset, data, len);
> +}
> +
> +static int adc_write(struct adc_chip *adc, u16 offset, u8 *data, int len)
> +{
> +	return regmap_bulk_write(adc->regmap, adc->base + offset, data, len);
> +}
> +
> +static int adc_prescaling_from_dt(u32 num, u32 den)
> +{
> +	unsigned int pre;
> +
> +	for (pre = 0; pre < ARRAY_SIZE(adc_prescale_ratios); pre++)
> +		if (adc_prescale_ratios[pre].num == num &&
> +		    adc_prescale_ratios[pre].den == den)
> +			break;
> +
> +	if (pre == ARRAY_SIZE(adc_prescale_ratios))
> +		return -EINVAL;
> +
> +	return pre;
> +}
> +
> +static int adc_hw_settle_time_from_dt(u32 value,
> +					const unsigned int *hw_settle)
> +{
> +	uint32_t i;
> +
> +	for (i = 0; i < VADC_HW_SETTLE_SAMPLES_MAX; i++) {
> +		if (value == hw_settle[i])
> +			return i;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int adc_avg_samples_from_dt(u32 value)
> +{
> +	if (!is_power_of_2(value) || value > ADC5_AVG_SAMPLES_MAX)
> +		return -EINVAL;
> +
> +	return __ffs64(value);
> +}
> +
> +static int adc_read_current_data(struct adc_chip *adc, u16 *data)
> +{
> +	int ret;
> +	u8 rslt_lsb = 0, rslt_msb = 0;
> +
> +	ret = adc_read(adc, ADC_USR_IBAT_DATA0, &rslt_lsb, 1);
> +	if (ret)
> +		return ret;
> +
> +	ret = adc_read(adc, ADC_USR_IBAT_DATA1, &rslt_msb, 1);
> +	if (ret)
> +		return ret;
> +
> +	*data = (rslt_msb << 8) | rslt_lsb;
> +
> +	if (*data == ADC_USR_DATA_CHECK) {
> +		pr_err("Invalid data:0x%x\n", *data);
> +		return -EINVAL;
> +	}
> +
> +	return ret;
> +}
> +
> +static int adc_read_voltage_data(struct adc_chip *adc, u16 *data)
> +{
> +	int ret;
> +	u8 rslt_lsb = 0, rslt_msb = 0;

Seems odd that you need to assign these for any path?

> +
> +	ret = adc_read(adc, ADC_USR_DATA0, &rslt_lsb, 1);
> +	if (ret)
> +		return ret;
> +
> +	ret = adc_read(adc, ADC_USR_DATA1, &rslt_msb, 1);
> +	if (ret)
> +		return ret;
> +
> +	*data = (rslt_msb << 8) | rslt_lsb;
> +
> +	if (*data == ADC_USR_DATA_CHECK) {
> +		pr_err("Invalid data:0x%x\n", *data);
> +		return -EINVAL;
> +	}
> +
> +	return ret;
> +}
> +
> +static int adc_poll_wait_eoc(struct adc_chip *adc)
> +{
> +	unsigned int count, retry;
> +	u8 status1;
> +	int ret;
> +
> +	retry = ADC_CONV_TIME_RETRY;

unsigned int retry = ADC_CONV_TIME_RETRY;

> +
> +	for (count = 0; count < retry; count++) {
> +		ret = adc_read(adc, ADC_USR_STATUS1, &status1, 1);
> +		if (ret)
> +			return ret;
> +
> +		status1 &= ADC_USR_STATUS1_REQ_STS_EOC_MASK;
> +		if (status1 == ADC_USR_STATUS1_EOC)
> +			return 0;
> +		usleep_range(ADC_CONV_TIME_MIN_US, ADC_CONV_TIME_MAX_US);
> +	}
> +
> +	return -ETIMEDOUT;
> +}
> +
> +static void adc_update_dig_param(struct adc_chip *adc,
> +			struct adc_channel_prop *prop, u8 *data)
> +{
> +	/* Update calibration value */
> +	*data &= ~ADC_USR_DIG_PARAM_CAL_VAL;
> +	*data |= (prop->cal_val << ADC_USR_DIG_PARAM_CAL_VAL_SHIFT);
> +
> +	/* Update calibration select */
> +	*data &= ~ADC_USR_DIG_PARAM_CAL_SEL;
> +	*data |= (prop->cal_method << ADC_USR_DIG_PARAM_CAL_SEL_SHIFT);
> +
> +	/* Update decimation ratio select */
> +	*data &= ~ADC_USR_DIG_PARAM_DEC_RATIO_SEL;
> +	*data |= (prop->decimation << ADC_USR_DIG_PARAM_DEC_RATIO_SEL_SHIFT);
> +}
> +
> +static int adc_configure(struct adc_chip *adc,
> +			struct adc_channel_prop *prop)
> +{
> +	int ret;
> +	u8 buf[6];
> +
> +	/* Read registers 0x42 through 0x46 */
> +	ret = adc_read(adc, ADC_USR_DIG_PARAM, buf, 6);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Digital param selection */
> +	adc_update_dig_param(adc, prop, &buf[0]);
> +
> +	/* Update fast average sample value */
> +	buf[1] &= (u8) ~ADC_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] &= (u8) ~ADC_USR_HW_SETTLE_DELAY_MASK;
> +	buf[3] |= prop->hw_settle_time;
> +
> +	/* Select ADC enable */
> +	buf[4] |= ADC_USR_EN_CTL1_ADC_EN;
> +
> +	/* Select CONV request */
> +	buf[5] |= ADC_USR_CONV_REQ_REQ;
> +
> +	if (!adc->poll_eoc)
> +		reinit_completion(&adc->complete);
> +
> +	ret = adc_write(adc, ADC_USR_DIG_PARAM, buf, 6);
> +
> +	return ret;

return adc_write...

> +}
> +
> +static int adc_do_conversion(struct adc_chip *adc,
> +			struct adc_channel_prop *prop,
> +			struct iio_chan_spec const *chan,
> +			u16 *data_volt, u16 *data_cur)
> +{
> +	int ret;
> +
> +	mutex_lock(&adc->lock);
> +
> +	ret = adc_configure(adc, prop);
> +	if (ret) {
> +		pr_err("ADC configure failed with %d\n", ret);
> +		goto unlock;
> +	}
> +
> +	if (adc->poll_eoc) {
> +		ret = adc_poll_wait_eoc(adc);
> +		if (ret < 0) {
> +			pr_err("EOC bit not set\n");
> +			goto unlock;
> +		}
> +	} else {
> +		ret = wait_for_completion_timeout(&adc->complete,
> +							ADC_CONV_TIMEOUT);
> +		if (!ret) {
> +			pr_debug("Did not get completion timeout.\n");
> +			ret = adc_poll_wait_eoc(adc);
> +			if (ret < 0) {
> +				pr_err("EOC bit not set\n");
> +				goto unlock;
> +			}
> +		}
> +	}
> +
> +	if ((chan->type == IIO_VOLTAGE) || (chan->type == IIO_TEMP))
> +		ret = adc_read_voltage_data(adc, data_volt);
> +	else if (chan->type == IIO_POWER) {
> +		ret = adc_read_voltage_data(adc, data_volt);
> +		if (ret)
> +			goto unlock;
> +
> +		ret = adc_read_current_data(adc, data_cur);
> +	}
> +unlock:
> +	mutex_unlock(&adc->lock);
> +
> +	return ret;
> +}
> +
> +static irqreturn_t adc_isr(int irq, void *dev_id)
> +{
> +	struct adc_chip *adc = dev_id;
> +
> +	complete(&adc->complete);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int adc_of_xlate(struct iio_dev *indio_dev,
> +				const struct of_phandle_args *iiospec)
> +{
> +	struct adc_chip *adc = iio_priv(indio_dev);
> +	int i;
> +
> +	for (i = 0; i < adc->nchannels; i++)
> +		if (adc->chan_props[i].channel == iiospec->args[0])
> +			return i;
> +
> +	return -EINVAL;
> +}
> +
> +static int adc_read_raw(struct iio_dev *indio_dev,
> +			 struct iio_chan_spec const *chan, int *val, int *val2,
> +			 long mask)
> +{
> +	struct adc_chip *adc = iio_priv(indio_dev);
> +	struct adc_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 = adc_do_conversion(adc, prop, chan,
> +				&adc_code_volt, &adc_code_cur);
> +		if (ret)
> +			break;
> +
> +		if ((chan->type == IIO_VOLTAGE) || (chan->type == IIO_TEMP))
> +			ret = qcom_vadc_hw_scale(prop->scale_fn_type,
> +				&adc_prescale_ratios[prop->prescale],
> +				adc->data,
> +				adc_code_volt, val);
> +		if (ret)
> +			break;
> +
> +		if (chan->type == IIO_POWER) {
> +			ret = qcom_vadc_hw_scale(SCALE_HW_CALIB_DEFAULT,
> +				&adc_prescale_ratios[VADC_DEF_VBAT_PRESCALING],
> +				adc->data,
> +				adc_code_volt, val);
> +			if (ret)
> +				break;
> +
> +			ret = qcom_vadc_hw_scale(prop->scale_fn_type,
> +				&adc_prescale_ratios[prop->prescale],
> +				adc->data,
> +				adc_code_cur, val2);
> +			if (ret)
> +				break;
> +		}
> +
> +		if (chan->type == IIO_POWER)
> +			return IIO_VAL_INT_MULTIPLE;
> +		else
> +			return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_RAW:
> +		ret = adc_do_conversion(adc, prop, chan,
> +				&adc_code_volt, &adc_code_cur);
> +		if (ret)
> +			break;
> +
> +		*val = (int)adc_code_volt;
> +		*val2 = (int)adc_code_cur;
> +		if (chan->type == IIO_POWER)
> +			return IIO_VAL_INT_MULTIPLE;
> +		else
> +			return IIO_VAL_INT;
> +	default:
> +		ret = -EINVAL;
> +		break;
> +	}
> +
> +	return ret;
> +}
> +
> +static const struct iio_info adc_info = {
> +	.read_raw = adc_read_raw,
> +	.of_xlate = adc_of_xlate,
> +};
> +
> +struct adc_channels {
> +	const char *datasheet_name;
> +	unsigned int prescale_index;
> +	enum iio_chan_type type;
> +	long info_mask;
> +	enum vadc_scale_fn_type scale_fn_type;
> +};
> +
> +#define ADC_CHAN(_dname, _type, _mask, _pre, _scale)			\
> +	{								\
> +		.datasheet_name = (_dname),				\
> +		.prescale_index = _pre,					\
> +		.type = _type,						\
> +		.info_mask = _mask,					\
> +		.scale_fn_type = _scale,				\
> +	},								\
> +
> +#define ADC_CHAN_TEMP(_dname, _pre, _scale)				\
> +	ADC_CHAN(_dname, IIO_TEMP,					\
> +		BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_PROCESSED),	\
> +		_pre, _scale)						\
> +
> +#define ADC_CHAN_VOLT(_dname, _pre, _scale)				\
> +	ADC_CHAN(_dname, IIO_VOLTAGE,					\
> +		  BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_PROCESSED),\
> +		  _pre, _scale)						\
> +
> +#define ADC_CHAN_POWER(_dname, _pre, _scale)				\
> +	ADC_CHAN(_dname, IIO_POWER,					\
> +		  BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_PROCESSED),\
> +		  _pre, _scale)						\
> +
> +static const struct adc_channels adc_chans_pmic5[ADC_MAX_CHANNEL] = {
> +	[ADC_REF_GND]		= ADC_CHAN_VOLT("ref_gnd", 1,
> +					SCALE_HW_CALIB_DEFAULT)
> +	[ADC_1P25VREF]		= ADC_CHAN_VOLT("vref_1p25", 1,
> +					SCALE_HW_CALIB_DEFAULT)
> +	[ADC_VPH_PWR]		= ADC_CHAN_VOLT("vph_pwr", 3,
> +					SCALE_HW_CALIB_DEFAULT)
> +	[ADC_VBAT_SNS]		= ADC_CHAN_VOLT("vbat_sns", 3,
> +					SCALE_HW_CALIB_DEFAULT)
> +	[ADC_DIE_TEMP]		= ADC_CHAN_TEMP("die_temp", 1,
> +					SCALE_HW_CALIB_PMIC_THERM)
> +	[ADC_USB_IN_I]		= ADC_CHAN_VOLT("usb_in_i_uv", 1,
> +					SCALE_HW_CALIB_DEFAULT)
> +	[ADC_USB_IN_V_16]	= ADC_CHAN_VOLT("usb_in_v_div_16", 16,
> +					SCALE_HW_CALIB_DEFAULT)
> +	[ADC_CHG_TEMP]		= ADC_CHAN_TEMP("chg_temp", 1,
> +					SCALE_HW_CALIB_PM5_CHG_TEMP)
> +	/* Charger prescales SBUx and MID_CHG to fit within 1.8V upper unit */
> +	[ADC_SBUx]		= ADC_CHAN_VOLT("chg_sbux", 3,
> +					SCALE_HW_CALIB_DEFAULT)
> +	[ADC_MID_CHG_DIV6]	= ADC_CHAN_VOLT("chg_mid_chg", 6,
> +					SCALE_HW_CALIB_DEFAULT)
> +	[ADC_XO_THERM_PU2]	= ADC_CHAN_TEMP("xo_therm", 1,
> +					SCALE_HW_CALIB_XOTHERM)
> +	[ADC_AMUX_THM1_PU2]	= ADC_CHAN_TEMP("amux_thm1_pu2", 1,
> +					SCALE_HW_CALIB_THERM_100K_PULLUP)
> +	[ADC_AMUX_THM2_PU2]	= ADC_CHAN_TEMP("amux_thm2_pu2", 1,
> +					SCALE_HW_CALIB_THERM_100K_PULLUP)
> +	[ADC_AMUX_THM3_PU2]	= ADC_CHAN_TEMP("amux_thm3_pu2", 1,
> +					SCALE_HW_CALIB_THERM_100K_PULLUP)
> +	[ADC_INT_EXT_ISENSE_VBAT_VDATA]	= ADC_CHAN_POWER("int_ext_isense", 1,
> +					SCALE_HW_CALIB_CUR)
> +	[ADC_EXT_ISENSE_VBAT_VDATA]	= ADC_CHAN_POWER("ext_isense", 1,
> +					SCALE_HW_CALIB_CUR)
> +	[ADC_PARALLEL_ISENSE_VBAT_VDATA] = ADC_CHAN_POWER("parallel_isense", 1,
> +					SCALE_HW_CALIB_CUR)
> +	[ADC_AMUX_THM2]			= ADC_CHAN_TEMP("amux_thm2", 1,
> +					SCALE_HW_CALIB_PM5_SMB_TEMP)
> +};
> +
> +static const struct adc_channels adc_chans_rev2[ADC_MAX_CHANNEL] = {
> +	[ADC_REF_GND]		= ADC_CHAN_VOLT("ref_gnd", 1,
> +					SCALE_HW_CALIB_DEFAULT)
> +	[ADC_1P25VREF]		= ADC_CHAN_VOLT("vref_1p25", 1,
> +					SCALE_HW_CALIB_DEFAULT)
> +	[ADC_VPH_PWR]		= ADC_CHAN_VOLT("vph_pwr", 3,
> +					SCALE_HW_CALIB_DEFAULT)
> +	[ADC_VBAT_SNS]		= ADC_CHAN_VOLT("vbat_sns", 3,
> +					SCALE_HW_CALIB_DEFAULT)
> +	[ADC_VCOIN]		= ADC_CHAN_VOLT("vcoin", 3,
> +					SCALE_HW_CALIB_DEFAULT)
> +	[ADC_DIE_TEMP]		= ADC_CHAN_TEMP("die_temp", 1,
> +					SCALE_HW_CALIB_PMIC_THERM)
> +	[ADC_AMUX_THM1_PU2]	= ADC_CHAN_TEMP("amux_thm1_pu2", 1,
> +					SCALE_HW_CALIB_THERM_100K_PULLUP)
> +	[ADC_AMUX_THM3_PU2]	= ADC_CHAN_TEMP("amux_thm3_pu2", 1,
> +					SCALE_HW_CALIB_THERM_100K_PULLUP)
> +	[ADC_AMUX_THM5_PU2]	= ADC_CHAN_TEMP("amux_thm5_pu2", 1,
> +					SCALE_HW_CALIB_THERM_100K_PULLUP)
> +	[ADC_XO_THERM_PU2]	= ADC_CHAN_TEMP("xo_therm", 1,
> +					SCALE_HW_CALIB_THERM_100K_PULLUP)
> +};
> +
> +static int adc_get_dt_channel_data(struct device *dev,
> +				    struct adc_channel_prop *prop,
> +				    struct device_node *node,
> +				    const struct adc_data *data)
> +{
> +	const char *name = node->name, *channel_name;
> +	u32 chan, value, varr[2];
> +	int ret;
> +
> +	ret = of_property_read_u32(node, "reg", &chan);
> +	if (ret) {
> +		dev_err(dev, "invalid channel number %s\n", name);
> +		return ret;
> +	}
> +
> +	if (chan > ADC_PARALLEL_ISENSE_VBAT_IDATA) {
> +		dev_err(dev, "%s invalid channel number %d\n", name, chan);
> +		return -EINVAL;
> +	}
> +
> +	/* the channel has DT description */
> +	prop->channel = chan;
> +
> +	channel_name = of_get_property(node,
> +				"label", NULL) ? : node->name;
> +	if (!channel_name) {
> +		pr_err("Invalid channel name\n");
> +		return -EINVAL;
> +	}
> +	prop->datasheet_name = channel_name;
> +
> +	ret = of_property_read_u32(node, "qcom,decimation", &value);
> +	if (!ret) {
> +		ret = qcom_adc5_decimation_from_dt(value, data->decimation);
> +		if (ret < 0) {
> +			dev_err(dev, "%02x invalid decimation %d\n",
> +				chan, value);
> +			return ret;
> +		}
> +		prop->decimation = ret;
> +	} else {
> +		prop->decimation = ADC_DECIMATION_DEFAULT;
> +	}
> +
> +	ret = of_property_read_u32_array(node, "qcom,pre-scaling", varr, 2);
> +	if (!ret) {
> +		ret = adc_prescaling_from_dt(varr[0], varr[1]);
> +		if (ret < 0) {
> +			dev_err(dev, "%02x invalid pre-scaling <%d %d>\n",
> +				chan, varr[0], varr[1]);
> +			return ret;
> +		}
> +		prop->prescale = ret;
> +	}
> +
> +	ret = of_property_read_u32(node, "qcom,hw-settle-time", &value);
> +	if (!ret) {
> +		ret = adc_hw_settle_time_from_dt(value, data->hw_settle);
> +		if (ret < 0) {
> +			dev_err(dev, "%02x invalid hw-settle-time %d us\n",
> +				chan, value);
> +			return ret;
> +		}
> +		prop->hw_settle_time = ret;
> +	} else {
> +		prop->hw_settle_time = VADC_DEF_HW_SETTLE_TIME;
> +	}
> +
> +	ret = of_property_read_u32(node, "qcom,avg-samples", &value);
> +	if (!ret) {
> +		ret = adc_avg_samples_from_dt(value);
> +		if (ret < 0) {
> +			dev_err(dev, "%02x invalid avg-samples %d\n",
> +				chan, value);
> +			return ret;
> +		}
> +		prop->avg_samples = ret;
> +	} else {
> +		prop->avg_samples = VADC_DEF_AVG_SAMPLES;
> +	}
> +
> +	if (of_property_read_bool(node, "qcom,ratiometric"))
> +		prop->cal_method = ADC_RATIOMETRIC_CAL;
> +	else
> +		prop->cal_method = ADC_ABSOLUTE_CAL;
> +
> +	dev_dbg(dev, "%02x name %s\n", chan, name);
> +
> +	return 0;
> +}
> +
> +const struct adc_data data_pmic5 = {
> +	.full_scale_code_volt = 0x70e4,
> +	.full_scale_code_cur = 10000,

Odd hex / decimal mix.

> +	.adc_chans = adc_chans_pmic5,
> +	.decimation = (unsigned int []) {250, 420, 840},
> +	.hw_settle = (unsigned int []) {15, 100, 200, 300, 400, 500, 600, 700,
> +					800, 900, 1, 2, 4, 6, 8, 10},
> +};
> +
> +const struct adc_data data_pmic_rev2 = {
> +	.full_scale_code_volt = 0x4000,
> +	.full_scale_code_cur = 0x1800,
> +	.adc_chans = adc_chans_rev2,
> +	.decimation = (unsigned int []) {256, 512, 1024},

As I mention further down I'd rather see these given a specific length so
that we ensure the numbers are consistent with those used in the relevant
for loops.  Right now we have to enforce that by review which is more
fragile than making it explicit.

> +	.hw_settle = (unsigned int []) {0, 100, 200, 300, 400, 500, 600, 700,
> +					800, 900, 1, 2, 4, 6, 8, 10},
> +};
> +
> +static const struct of_device_id adc_match_table[] = {
> +	{
> +		.compatible = "qcom,spmi-adc5",
> +		.data = &data_pmic5,
> +	},
> +	{
> +		.compatible = "qcom,spmi-adc-rev2",
> +		.data = &data_pmic_rev2,
> +	},
> +	{ }
> +};
> +
> +static int adc_get_dt_data(struct adc_chip *adc, struct device_node *node)
> +{
> +	const struct adc_channels *adc_chan;
> +	struct iio_chan_spec *iio_chan;
> +	struct adc_channel_prop prop;
> +	struct device_node *child;
> +	unsigned int index = 0;
> +	const struct of_device_id *id;
> +	const struct adc_data *data;
> +	int ret;
> +
> +	adc->nchannels = of_get_available_child_count(node);
> +	if (!adc->nchannels)
> +		return -EINVAL;
> +
> +	adc->iio_chans = devm_kcalloc(adc->dev, adc->nchannels,
> +				       sizeof(*adc->iio_chans), GFP_KERNEL);
> +	if (!adc->iio_chans)
> +		return -ENOMEM;
> +
> +	adc->chan_props = devm_kcalloc(adc->dev, adc->nchannels,
> +					sizeof(*adc->chan_props), GFP_KERNEL);
> +	if (!adc->chan_props)
> +		return -ENOMEM;
> +
> +	iio_chan = adc->iio_chans;
> +	id = of_match_node(adc_match_table, node);
> +	if (id)
> +		data = id->data;
> +	else
> +		data = &data_pmic5;
> +	adc->data = data;
> +
> +	for_each_available_child_of_node(node, child) {
> +		ret = adc_get_dt_channel_data(adc->dev, &prop, child, data);
> +		if (ret) {
> +			of_node_put(child);
> +			return ret;
> +		}
> +
> +		prop.scale_fn_type =
> +			data->adc_chans[prop.channel].scale_fn_type;
> +		adc->chan_props[index] = prop;

It seems a little inconsistent to use an array access for chan_props
and pointer arithmetic for iio_chan.   I don't mind which style
you want to use, but I can see a reason not to be consistent.

> +
> +		adc_chan = &data->adc_chans[prop.channel];
> +
> +		iio_chan->channel = prop.channel;
> +		iio_chan->datasheet_name = prop.datasheet_name;
> +		iio_chan->extend_name = prop.datasheet_name;
> +		iio_chan->info_mask_separate = adc_chan->info_mask;
> +		iio_chan->type = adc_chan->type;
> +		iio_chan->address = index;
> +		iio_chan++;
> +		index++;
> +	}
> +
> +	return 0;
> +}
> +
> +static int adc_probe(struct platform_device *pdev)
> +{
> +	struct device_node *node = pdev->dev.of_node;
> +	struct device *dev = &pdev->dev;
> +	struct iio_dev *indio_dev;
> +	struct adc_chip *adc;
> +	struct regmap *regmap;
> +	int ret, irq_eoc;
> +	u32 reg;
> +
> +	regmap = dev_get_regmap(dev->parent, NULL);
> +	if (!regmap)
> +		return -ENODEV;
> +
> +	ret = of_property_read_u32(node, "reg", &reg);
> +	if (ret < 0)
> +		return ret;
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*adc));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	adc = iio_priv(indio_dev);
> +	adc->regmap = regmap;
> +	adc->dev = dev;
> +	adc->base = reg;
> +	init_completion(&adc->complete);
> +	mutex_init(&adc->lock);
> +
> +	ret = adc_get_dt_data(adc, node);
> +	if (ret) {
> +		pr_err("adc get dt data failed\n");
> +		return ret;
> +	}
> +
> +	irq_eoc = platform_get_irq(pdev, 0);
> +	if (irq_eoc < 0) {
> +		if (irq_eoc == -EPROBE_DEFER || irq_eoc == -EINVAL)
> +			return irq_eoc;
> +		adc->poll_eoc = true;
> +	} else {
> +		ret = devm_request_irq(dev, irq_eoc, adc_isr, 0,
> +				       "pm-adc5", adc);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	indio_dev->dev.parent = dev;
> +	indio_dev->dev.of_node = node;
> +	indio_dev->name = pdev->name;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->info = &adc_info;
> +	indio_dev->channels = adc->iio_chans;
> +	indio_dev->num_channels = adc->nchannels;
> +
> +	return devm_iio_device_register(dev, indio_dev);
> +}
> +
> +static struct platform_driver adc_driver = {
> +	.driver = {
> +		.name = "qcom-spmi-adc5.c",
> +		.of_match_table = adc_match_table,
> +	},
> +	.probe = adc_probe,
> +};
> +module_platform_driver(adc_driver);
> +
> +MODULE_ALIAS("platform:qcom-spmi-adc5");
> +MODULE_DESCRIPTION("Qualcomm Technologies Inc. PMIC5 ADC driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iio/adc/qcom-vadc-common.c b/drivers/iio/adc/qcom-vadc-common.c
> index fe3d782..7a37035 100644
> --- a/drivers/iio/adc/qcom-vadc-common.c
> +++ b/drivers/iio/adc/qcom-vadc-common.c
> @@ -47,6 +47,47 @@
>  	{44,	125}
>  };
>  
> +/*
> + * Voltage to temperature table for 100k pull up for NTCG104EF104 with
> + * 1.875V reference.
> + */
> +static const struct vadc_map_pt adcmap_100k_104ef_104fb_1875_vref[] = {
> +	{ 1831,	-40000 },
> +	{ 1814,	-35000 },
> +	{ 1791,	-30000 },
> +	{ 1761,	-25000 },
> +	{ 1723,	-20000 },
> +	{ 1675,	-15000 },
> +	{ 1616,	-10000 },
> +	{ 1545,	-5000 },
> +	{ 1463,	0 },
> +	{ 1370,	5000 },
> +	{ 1268,	10000 },
> +	{ 1160,	15000 },
> +	{ 1049,	20000 },
> +	{ 937,	25000 },
> +	{ 828,	30000 },
> +	{ 726,	35000 },
> +	{ 630,	40000 },
> +	{ 544,	45000 },
> +	{ 467,	50000 },
> +	{ 399,	55000 },
> +	{ 340,	60000 },
> +	{ 290,	65000 },
> +	{ 247,	70000 },
> +	{ 209,	75000 },
> +	{ 179,	80000 },
> +	{ 153,	85000 },
> +	{ 130,	90000 },
> +	{ 112,	95000 },
> +	{ 96,	100000 },
> +	{ 82,	105000 },
> +	{ 71,	110000 },
> +	{ 62,	115000 },
> +	{ 53,	120000 },
> +	{ 46,	125000 },
> +};
> +
>  static int qcom_vadc_map_voltage_temp(const struct vadc_map_pt *pts,
>  				      u32 tablesize, s32 input, s64 *output)
>  {
> @@ -191,6 +232,163 @@ static int qcom_vadc_scale_chg_temp(const struct vadc_linear_graph *calib_graph,
>  	return 0;
>  }
>  
> +static int qcom_vadc_scale_hw_calib_volt(
> +				const struct vadc_prescale_ratio *prescale,
> +				const struct adc_data *data,
> +				u16 adc_code, int *result_uv)
> +{
> +	s64 voltage = 0, result = 0, adc_vdd_ref_mv = 1875;
> +
> +	if (adc_code > VADC5_MAX_CODE)
> +		adc_code = 0;
> +
> +	/* (ADC code * vref_vadc (1.875V)) / full_scale_code */
> +	voltage = (s64) adc_code * adc_vdd_ref_mv * 1000;
> +	voltage = div64_s64(voltage, data->full_scale_code_volt);
> +	voltage = voltage * prescale->den;
> +	result = div64_s64(voltage, prescale->num);
> +	*result_uv = result;
> +
> +	return 0;
> +}
> +
> +static int qcom_vadc_scale_hw_calib_therm(
> +				const struct vadc_prescale_ratio *prescale,
> +				const struct adc_data *data,
> +				u16 adc_code, int *result_mdec)
> +{
> +	s64 voltage = 0, result = 0, adc_vdd_ref_mv = 1875;
> +	int ret;
> +
> +	if (adc_code > VADC5_MAX_CODE)
> +		adc_code = 0;
> +
> +	/* (ADC code * vref_vadc (1.875V)) / full_scale_code */
> +	voltage = (s64) adc_code * adc_vdd_ref_mv * 1000;
> +	voltage = div64_s64(voltage, (data->full_scale_code_volt
> +								* 1000));
> +	ret = qcom_vadc_map_voltage_temp(adcmap_100k_104ef_104fb_1875_vref,
> +				 ARRAY_SIZE(adcmap_100k_104ef_104fb_1875_vref),
> +				 voltage, &result);
> +	if (ret)
> +		return ret;
> +
> +	*result_mdec = result;
> +
> +	return 0;
> +}
> +
> +static int qcom_vadc_scale_hw_calib_die_temp(
> +				const struct vadc_prescale_ratio *prescale,
> +				const struct adc_data *data,
> +				u16 adc_code, int *result_mdec)
> +{
> +	s64 voltage = 0, adc_vdd_ref_mv = 1875;
> +	u64 temp; /* Temporary variable for do_div */
> +
> +	if (adc_code > VADC5_MAX_CODE)
> +		adc_code = 0;
> +
> +	/* (ADC code * vref_vadc (1.875V)) / full_scale_code */
> +	voltage = (s64) adc_code * adc_vdd_ref_mv * 1000;
> +	voltage = div64_s64(voltage, data->full_scale_code_volt);
> +	if (voltage > 0) {
> +		temp = voltage * prescale->den;
> +		do_div(temp, prescale->num * 2);
> +		voltage = temp;
> +	} else {
> +		voltage = 0;
> +	}
> +
> +	voltage -= KELVINMIL_CELSIUSMIL;
> +	*result_mdec = voltage;
> +
> +	return 0;
> +}
> +
> +static int qcom_vadc_scale_hw_smb_temp(
> +				const struct vadc_prescale_ratio *prescale,
> +				const struct adc_data *data,
> +				u16 adc_code, int *result_mdec)
> +{
This is very close to the function below, can we not create a utility function
that does both and save on a lot of code repetition?

> +	s64 voltage = 0, adc_vdd_ref_mv = 1875;
> +	u64 temp;
> +
> +	if (adc_code > VADC5_MAX_CODE)
> +		adc_code = 0;
> +
> +	/* (ADC code * vref_vadc (1.875V)) / full_scale_code */
> +	voltage = (s64) adc_code * adc_vdd_ref_mv * 1000;
> +	voltage = div64_s64(voltage, data->full_scale_code_volt);
> +	if (voltage > 0) {
> +		temp = voltage * prescale->den;
> +		temp *= 100;
> +		do_div(temp, prescale->num * PMIC5_SMB_TEMP_SCALE_FACTOR);
> +		voltage = temp;
> +	} else {
> +		voltage = 0;
> +	}
> +
> +	voltage = PMIC5_SMB_TEMP_CONSTANT - voltage;
> +	*result_mdec = voltage;
> +
> +	return 0;
> +}
> +
> +static int qcom_vadc_scale_hw_chg5_temp(
> +				const struct vadc_prescale_ratio *prescale,
> +				const struct adc_data *data,
> +				u16 adc_code, int *result_mdec)
> +{
> +	s64 voltage = 0, adc_vdd_ref_mv = 1875;
> +	u64 temp;
> +
> +	if (adc_code > VADC5_MAX_CODE)
> +		adc_code = 0;

That's unusual.  Would normally expect to clamp or return an
error, why does setting to 0 make sense here? Add a comment.

> +
> +	/* (ADC code * vref_vadc (1.875V)) / full_scale_code */
> +	voltage = (s64) adc_code * adc_vdd_ref_mv * 1000;
> +	voltage = div64_s64(voltage, data->full_scale_code_volt);
> +	if (voltage > 0) {
> +		temp = voltage * prescale->den;
> +		do_div(temp, prescale->num * 4);
> +		voltage = temp;
> +	} else {
> +		voltage = 0;
> +	}
> +
> +	voltage = PMIC5_CHG_TEMP_SCALE_FACTOR - voltage;
> +	*result_mdec = voltage;

Not sure why these last two lines aren't combined?

> +
> +	return 0;
> +}
> +
> +static int qcom_adc_scale_hw_calib_cur(
> +				const struct vadc_prescale_ratio *prescale,
> +				const struct adc_data *data,
> +				u16 adc_code, int *result_uamps)
> +{
> +	s64 voltage = 0, result = 0;

Both are set in both paths, so don't give them values here.
I would have expected sparse or similar to have picked up on this.
Please run them on patches before sending out.

> +
This code is 'unusual', please provide some docs on what the two options
actually are and why they are different.

> +	if ((adc_code & ADC_USR_DATA_CHECK) == 0) {
> +		voltage = (s64) adc_code * data->full_scale_code_cur * 1000;
> +		voltage = div64_s64(voltage, VADC5_MAX_CODE);
> +		voltage = voltage * prescale->den;
> +		result = div64_s64(voltage, prescale->num);
> +		*result_uamps = result;
> +	} else {
> +		adc_code = ~adc_code + 1;

2's comp negation? Followed at the end by negation - my immediate
thought is that this would give exactly the same answer as the
above path.. What am I missing?

> +		voltage = (s64) adc_code;

Huh?  You assign voltage then wipe it out on the next line.

There is a lot of sharing between the two paths here.  Only the first and last
statements are different.  Hmm. I'm torn on whether it would improve
or hurt readability to have two if blocks (at start and end) rather than
the bif if else you have currently.


> +		voltage = (s64) adc_code * data->full_scale_code_cur * 1000;
> +		voltage = div64_s64(voltage, VADC5_MAX_CODE);
> +		voltage = voltage * prescale->den;
> +		result = div64_s64(voltage, prescale->num);
> +		*result_uamps = -result;
What does the local variable result give you?  just assign directly.

> +	}
> +
> +	return 0;
> +}
> +
>  int qcom_vadc_scale(enum vadc_scale_fn_type scaletype,
>  		    const struct vadc_linear_graph *calib_graph,
>  		    const struct vadc_prescale_ratio *prescale,
> @@ -221,6 +419,37 @@ int qcom_vadc_scale(enum vadc_scale_fn_type scaletype,
>  }
>  EXPORT_SYMBOL(qcom_vadc_scale);
>  
> +int qcom_vadc_hw_scale(enum vadc_scale_fn_type scaletype,
> +		    const struct vadc_prescale_ratio *prescale,
> +		    const struct adc_data *data,
> +		    u16 adc_code, int *result)
> +{
> +	switch (scaletype) {

Superficially this feels like a case for a function pointer look up
table. Table will be a little sparse but that shouldn't
matter and it'll clean the code up here nicely.

> +	case SCALE_HW_CALIB_DEFAULT:
> +		return qcom_vadc_scale_hw_calib_volt(prescale, data,
> +						adc_code, result);
> +	case SCALE_HW_CALIB_THERM_100K_PULLUP:
> +	case SCALE_HW_CALIB_XOTHERM:
> +		return qcom_vadc_scale_hw_calib_therm(prescale, data,
> +						adc_code, result);
> +	case SCALE_HW_CALIB_PMIC_THERM:
> +		return qcom_vadc_scale_hw_calib_die_temp(prescale, data,
> +						adc_code, result);
> +	case SCALE_HW_CALIB_CUR:
> +		return qcom_adc_scale_hw_calib_cur(prescale, data,
> +						adc_code, result);
> +	case SCALE_HW_CALIB_PM5_CHG_TEMP:
> +		return qcom_vadc_scale_hw_chg5_temp(prescale, data,
> +						adc_code, result);
> +	case SCALE_HW_CALIB_PM5_SMB_TEMP:
> +		return qcom_vadc_scale_hw_smb_temp(prescale, data,
> +						adc_code, result);
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +EXPORT_SYMBOL(qcom_vadc_hw_scale);
> +
>  int qcom_vadc_decimation_from_dt(u32 value)
>  {
>  	if (!is_power_of_2(value) || value < VADC_DECIMATION_MIN ||
> @@ -231,5 +460,17 @@ int qcom_vadc_decimation_from_dt(u32 value)
>  }
>  EXPORT_SYMBOL(qcom_vadc_decimation_from_dt);
>  
> +int qcom_adc5_decimation_from_dt(u32 value, const unsigned int *decimation)
> +{
> +	uint32_t i;
> +
> +	for (i = 0; i < ADC_DECIMATION_SAMPLES_MAX; i++) {

Hmm. I would like to see the arrays fed to this clearly using the
same limits as here.  Right now the code looks fragile to any 
new device where that number is not 3.

> +		if (value == decimation[i])
> +			return i;
> +	}
> +
> +	return -EINVAL;
> +}
> +EXPORT_SYMBOL(qcom_adc5_decimation_from_dt);

Blank line here would help readability as the next line is not really connected
to the previous one.

>  MODULE_LICENSE("GPL v2");
>  MODULE_DESCRIPTION("Qualcomm ADC common functionality");
> diff --git a/drivers/iio/adc/qcom-vadc-common.h b/drivers/iio/adc/qcom-vadc-common.h
> index 1d5354f..7391bcd 100644
> --- a/drivers/iio/adc/qcom-vadc-common.h
> +++ b/drivers/iio/adc/qcom-vadc-common.h
> @@ -22,18 +22,34 @@
>  #define VADC_DEF_HW_SETTLE_TIME			0 /* 0 us */
>  #define VADC_DEF_AVG_SAMPLES			0 /* 1 sample */
>  #define VADC_DEF_CALIB_TYPE			VADC_CALIB_ABSOLUTE
> +#define VADC_DEF_VBAT_PRESCALING		1 /* 1:3 */
>  
>  #define VADC_DECIMATION_MIN			512
>  #define VADC_DECIMATION_MAX			4096
> +#define ADC5_DECIMATION_SHORT			250
> +#define ADC5_DECIMATION_MEDIUM			420
> +#define ADC5_DECIMATION_LONG			840
> +/* Default decimation - 1024 for rev2, 840 for pmic5 */
> +#define ADC_DECIMATION_DEFAULT			2
> +#define ADC_DECIMATION_SAMPLES_MAX		3
>  
>  #define VADC_HW_SETTLE_DELAY_MAX		10000
> +#define VADC_HW_SETTLE_SAMPLES_MAX		16
>  #define VADC_AVG_SAMPLES_MAX			512
> +#define ADC5_AVG_SAMPLES_MAX			16
>  
>  #define KELVINMIL_CELSIUSMIL			273150
> +#define PMIC5_CHG_TEMP_SCALE_FACTOR		377500
> +#define PMIC5_SMB_TEMP_CONSTANT			419400
> +#define PMIC5_SMB_TEMP_SCALE_FACTOR		356
>  
>  #define PMI_CHG_SCALE_1				-138890
>  #define PMI_CHG_SCALE_2				391750000000LL
>  
> +#define VADC5_MAX_CODE				0x7fff
> +#define VADC5_FULL_SCALE_CODE			0x70e4
> +#define ADC_USR_DATA_CHECK			0x8000
> +
>  /**
>   * struct vadc_map_pt - Map the graph representation for ADC channel
>   * @x: Represent the ADC digitized code.
> @@ -89,6 +105,19 @@ struct vadc_prescale_ratio {
>   * SCALE_PMIC_THERM: Returns result in milli degree's Centigrade.
>   * SCALE_XOTHERM: Returns XO thermistor voltage in millidegC.
>   * SCALE_PMI_CHG_TEMP: Conversion for PMI CHG temp
> + * SCALE_HW_CALIB_DEFAULT: Default scaling to convert raw adc code to
> + *	voltage (uV) with hardware applied offset/slope values to adc code.
> + * SCALE_HW_CALIB_THERM_100K_PULLUP: Returns temperature in millidegC using
> + *	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_PMIC_THERM: Returns result in milli degree's Centigrade.
> + *	The hardware applies offset/slope to adc code.
> + * SCALE_HW_CALIB_CUR: Returns result in uA for PMIC5.
> + * 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
> + *	SMB1390 temperature.
>   */
>  enum vadc_scale_fn_type {
>  	SCALE_DEFAULT = 0,
> @@ -96,6 +125,21 @@ enum vadc_scale_fn_type {
>  	SCALE_PMIC_THERM,
>  	SCALE_XOTHERM,
>  	SCALE_PMI_CHG_TEMP,
> +	SCALE_HW_CALIB_DEFAULT,
> +	SCALE_HW_CALIB_THERM_100K_PULLUP,
> +	SCALE_HW_CALIB_XOTHERM,
> +	SCALE_HW_CALIB_PMIC_THERM,
> +	SCALE_HW_CALIB_CUR,
> +	SCALE_HW_CALIB_PM5_CHG_TEMP,
> +	SCALE_HW_CALIB_PM5_SMB_TEMP,
> +};
> +
> +struct adc_data {
> +	const u32	full_scale_code_volt;
> +	const u32	full_scale_code_cur;
> +	const struct adc_channels *adc_chans;
> +	unsigned int	*decimation;
> +	unsigned int	*hw_settle;
>  };
>  
>  int qcom_vadc_scale(enum vadc_scale_fn_type scaletype,
> @@ -104,6 +148,13 @@ int qcom_vadc_scale(enum vadc_scale_fn_type scaletype,
>  		    bool absolute,
>  		    u16 adc_code, int *result_mdec);
>  
> +int qcom_vadc_hw_scale(enum vadc_scale_fn_type scaletype,
> +		    const struct vadc_prescale_ratio *prescale,
> +		    const struct adc_data *data,
> +		    u16 adc_code, int *result_mdec);
> +
>  int qcom_vadc_decimation_from_dt(u32 value);
>  
> +int qcom_adc5_decimation_from_dt(u32 value, const unsigned int *decimation);
> +
>  #endif /* QCOM_VADC_COMMON_H */
> diff --git a/include/dt-bindings/iio/qcom,spmi-vadc.h b/include/dt-bindings/iio/qcom,spmi-vadc.h
> index 42121fa..4aa35de 100644
> --- a/include/dt-bindings/iio/qcom,spmi-vadc.h
> +++ b/include/dt-bindings/iio/qcom,spmi-vadc.h
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2012-2014, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2012-2014,2018 The Linux Foundation. All rights reserved.
>   *
>   * This program is free software; you can redistribute it and/or modify
>   * it under the terms of the GNU General Public License version 2 and
> @@ -116,4 +116,117 @@
>  #define VADC_LR_MUX10_PU1_PU2_AMUX_USB_ID	0xf9
>  #define VADC_LR_MUX3_BUF_PU1_PU2_XO_THERM	0xfc
>  
> +/* ADC channels for SPMI VADC5*/

Check general comment formatting.  Should be a space before the */

> +
> +#define ADC_REF_GND				0x00

Given these are all being added to a generic(ish) header and only
apply to this new part, we should give them a prefix that makes this
clear.

> +#define ADC_1P25VREF				0x01
> +#define ADC_VREF_VADC				0x02
> +#define ADC_VREF_VADC_DIV_3			0x82
> +#define ADC_VPH_PWR				0x83
> +#define ADC_VBAT_SNS				0x84
> +#define ADC_VCOIN				0x85
> +#define ADC_DIE_TEMP				0x06
> +#define ADC_USB_IN_I				0x07
> +#define ADC_USB_IN_V_16				0x08
> +#define ADC_CHG_TEMP				0x09
> +#define ADC_BAT_THERM				0x0a
> +#define ADC_BAT_ID				0x0b
> +#define ADC_XO_THERM				0x0c
> +#define ADC_AMUX_THM1				0x0d
> +#define ADC_AMUX_THM2				0x0e
> +#define ADC_AMUX_THM3				0x0f
> +#define ADC_AMUX_THM4				0x10
> +#define ADC_AMUX_THM5				0x11
> +#define ADC_GPIO1				0x12
> +#define ADC_GPIO2				0x13
> +#define ADC_GPIO3				0x14
> +#define ADC_GPIO4				0x15
> +#define ADC_GPIO5				0x16
> +#define ADC_GPIO6				0x17
> +#define ADC_GPIO7				0x18
> +#define ADC_SBUx				0x99
> +#define ADC_MID_CHG_DIV6			0x1e
> +#define ADC_OFF					0xff
> +
> +/* 30k pull-up1 */
I'd prefer to see the 30k value in the names.

XXX_ADC_BAT_THERM_30K_PU perhaps or something like that?

> +#define ADC_BAT_THERM_PU1			0x2a
> +#define ADC_BAT_ID_PU1				0x2b
> +#define ADC_XO_THERM_PU1			0x2c
> +#define ADC_AMUX_THM1_PU1			0x2d
> +#define ADC_AMUX_THM2_PU1			0x2e
> +#define ADC_AMUX_THM3_PU1			0x2f
> +#define ADC_AMUX_THM4_PU1			0x30
> +#define ADC_AMUX_THM5_PU1			0x31
> +#define ADC_GPIO1_PU1				0x32
> +#define ADC_GPIO2_PU1				0x33
> +#define ADC_GPIO3_PU1				0x34
> +#define ADC_GPIO4_PU1				0x35
> +#define ADC_GPIO5_PU1				0x36
> +#define ADC_GPIO6_PU1				0x37
> +#define ADC_GPIO7_PU1				0x38
> +#define ADC_SBUx_PU1				0x39
> +
> +/* 100k pull-up2 */
> +#define ADC_BAT_THERM_PU2			0x4a
> +#define ADC_BAT_ID_PU2				0x4b
> +#define ADC_XO_THERM_PU2			0x4c
> +#define ADC_AMUX_THM1_PU2			0x4d
> +#define ADC_AMUX_THM2_PU2			0x4e
> +#define ADC_AMUX_THM3_PU2			0x4f
> +#define ADC_AMUX_THM4_PU2			0x50
> +#define ADC_AMUX_THM5_PU2			0x51
> +#define ADC_GPIO1_PU2				0x52
> +#define ADC_GPIO2_PU2				0x53
> +#define ADC_GPIO3_PU2				0x54
> +#define ADC_GPIO4_PU2				0x55
> +#define ADC_GPIO5_PU2				0x56
> +#define ADC_GPIO6_PU2				0x57
> +#define ADC_GPIO7_PU2				0x58
> +#define ADC_SBUx_PU2				0x59
> +
> +/* 400k pull-up3 */
> +#define ADC_BAT_THERM_PU3			0x6a
> +#define ADC_BAT_ID_PU3				0x6b
> +#define ADC_XO_THERM_PU3			0x6c
> +#define ADC_AMUX_THM1_PU3			0x6d
> +#define ADC_AMUX_THM2_PU3			0x6e
> +#define ADC_AMUX_THM3_PU3			0x6f
> +#define ADC_AMUX_THM4_PU3			0x70
> +#define ADC_AMUX_THM5_PU3			0x71
> +#define ADC_GPIO1_PU3				0x72
> +#define ADC_GPIO2_PU3				0x73
> +#define ADC_GPIO3_PU3				0x74
> +#define ADC_GPIO4_PU3				0x75
> +#define ADC_GPIO5_PU3				0x76
> +#define ADC_GPIO6_PU3				0x77
> +#define ADC_GPIO7_PU3				0x78
> +#define ADC_SBUx_PU3				0x79
> +
> +/* 1/3 Divider */
> +#define ADC_GPIO1_DIV3				0x92
> +#define ADC_GPIO2_DIV3				0x93
> +#define ADC_GPIO3_DIV3				0x94
> +#define ADC_GPIO4_DIV3				0x95
> +#define ADC_GPIO5_DIV3				0x96
> +#define ADC_GPIO6_DIV3				0x97
> +#define ADC_GPIO7_DIV3				0x98
> +#define ADC_SBUx_DIV3				0x99
> +
> +/* Current and combined current/voltage channels */
> +#define ADC_INT_EXT_ISENSE			0xa1
> +#define ADC_PARALLEL_ISENSE			0xa5
> +#define ADC_CUR_REPLICA_VDS			0xa7
> +#define ADC_CUR_SENS_BATFET_VDS_OFFSET		0xa9
> +#define ADC_CUR_SENS_REPLICA_VDS_OFFSET		0xab
> +#define ADC_EXT_SENS_OFFSET			0xad
> +
> +#define ADC_INT_EXT_ISENSE_VBAT_VDATA		0xb0
> +#define ADC_INT_EXT_ISENSE_VBAT_IDATA		0xb1
> +#define ADC_EXT_ISENSE_VBAT_VDATA		0xb2
> +#define ADC_EXT_ISENSE_VBAT_IDATA		0xb3
> +#define ADC_PARALLEL_ISENSE_VBAT_VDATA		0xb4
> +#define ADC_PARALLEL_ISENSE_VBAT_IDATA		0xb5
> +
> +#define ADC_MAX_CHANNEL				0xc0
> +
>  #endif /* _DT_BINDINGS_QCOM_SPMI_VADC_H */

^ permalink raw reply

* Re: [PATCH 1/2] dt-bindings: iio: adc: Add DT binding document for PMIC5 ADC
From: Jonathan Cameron @ 2018-05-12 10:15 UTC (permalink / raw)
  To: Siddartha Mohanadoss
  Cc: linux-iio, devicetree, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Rob Herring, cdevired, rphani, sivaa
In-Reply-To: <1525815501-27588-1-git-send-email-smohanad@codeaurora.org>

On Tue,  8 May 2018 14:38:21 -0700
Siddartha Mohanadoss <smohanad@codeaurora.org> wrote:

> PMIC5 ADC has support for clients to measure voltage and current
> on inputs connected to the PMIC. Clients include reading voltage
> phone power and on board system thermistors for thermal management.
> ADC5 on certain PMIC has support to read battery current.
> 
> This change adds documentation.
> 
> Signed-off-by: Siddartha Mohanadoss <smohanad@codeaurora.org>
Hi Siddartha,

Some complexity in here!  Anyhow, a few comments inline and we will definitely
be wanting guidance from the devicetree people for this one.

Jonathan

> ---
>  .../devicetree/bindings/iio/adc/qcom,spmi-adc5.txt | 137 +++++++++++++++++++++
>  1 file changed, 137 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt
> new file mode 100644
> index 0000000..c9268ba
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt
> @@ -0,0 +1,137 @@
> +Qualcomm Technologies Inc. SPMI PMIC5 voltage and current ADC
> +
> +SPMI PMIC5 voltage ADC (ADC) provides interface to clients to read
> +voltage. The VADC is a 16-bit sigma-delta ADC.
> +
> +ADC node:
> +
> +- compatible:
> +    Usage: required
> +    Value type: <string>
> +    Definition: Should contain "qcom,spmi-adc5" for PMIC5 ADC driver.
> +		Should contain "qcom,spmi-adc-rev2" for PMIC refresh ADC driver.
> +
> +- reg:
> +    Usage: required
> +    Value type: <prop-encoded-array>
> +    Definition: VADC base address and length 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
> +    Value type: <empty>
> +    Definition: ADC datasheet channel name.
> +            For thermistor inputs connected to generic AMUX or GPIO inputs
> +            these can vary across platform for the same pins. Hence select
> +            the datasheet name for this channel.
> +
> +- qcom,pre-scaling:
> +    Usage: required
> +    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,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 PMIC5 ADC, combined two step decimation values are 250, 420 and 840.
> +            If property is not found, default value of 840 will be used.

The odd indenting here needs sorting.  Mixture of spaces and tabs at the moment.

Hmm. In someways this is a policy decision so should be pushed up to userspace,
but given the 'right' value will be somewhat dependent on what you are doing
with the channel and what is wired to it, it could arguably have a 'right' value
for a given circuit.  This is really just the sampling frequency wrapped
up in decimation of something, I'm guessing some input clock?

Let's see what the Device-tree people think on this one!  Personally I have
never really minded devicetree providing sensible defaults.  We can put
control on these things later, if there is a usecase for changing them.

> +	    For PMIC refresh ADC, supported decimation values are 256, 512, 1024.
> +	    If property is not found, default value of 1024 will be used.
> +
> +- qcom,ratiometric:
> +    Usage: optional
> +    Value type: <empty>
> +    Definition: Channel calibration type. 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.
> +	    For PMIC5, delay = 15us for value 0,
> +			100us * (value) for values 0 < value < 11, and
> +            		2ms * (value - 10) otherwise.
> +            Valid values are: 15, 100, 200, 300, 400, 500, 600, 700, 800,

This description is very confusing given the different uses of 'value'
None of the values you have allowed is less than 11 so the first condition
doesn't apply.

> +            900 us and 1, 2, 4, 6, 8, 10 ms
> +            If property is not found, channel will use 15us.
> +	    For PMIC rev2, delay = 100us * (value) for values 0 < value < 11, and
> +			2ms * (value - 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 0 us.
> +
> +- 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).
> +            Valid values are: 1, 2, 4, 8, 16
> +            If property is not found, 1 sample will be used.

As with decimation, this is arguably not a feature of the hardware, but
a software decision...

> +
> +Example:
> +
> +        /* VADC node */
> +        pmic_vadc: vadc@3100 {
> +                compatible = "qcom,spmi-adc5";
> +                reg = <0x3100 0x100>;
> +                interrupts = <0x0 0x31 0x0 IRQ_TYPE_EDGE_RISING>;
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +                #io-channel-cells = <1>;
> +                io-channel-ranges;
> +
> +                /* Channel node */
> +                vph_pwr {
> +                        reg = <ADC_VPH_PWR>;
> +                        label = "vph_pwr";
> +                        qcom,pre-scaling = <1 3>;
> +                };
> +        };
> +
> +        /* IIO client node */
> +        usb {
> +                io-channels = <&pmic_vadc ADC_VPH_PWR>;
> +                io-channel-names = "vadc";
> +        };

^ permalink raw reply

* Re: [PATCH v2] iio: potentiostat: lmp91000: add LMP91002 support
From: Jonathan Cameron @ 2018-05-12 10:03 UTC (permalink / raw)
  To: Rob Herring; +Cc: Matt Ranostay, linux-iio, devicetree
In-Reply-To: <20180508154256.GA28095@rob-hp-laptop>

On Tue, 8 May 2018 10:42:56 -0500
Rob Herring <robh@kernel.org> wrote:

> On Mon, May 07, 2018 at 10:34:01PM -0700, Matt Ranostay wrote:
> > LMP91002 is register compatible so add devicetree and i2c client ids
> > 
> > Cc: devicetree@vger.kernel.org
> > Signed-off-by: Matt Ranostay <matt.ranostay@konsulko.com>
> > ---
> > 
> > Changes from v1:
> > * Split out compatible options in multiple lines for the lmp91000.txt documentation
> > 
> >  Documentation/devicetree/bindings/iio/potentiostat/lmp91000.txt | 9 ++++++---
> >  drivers/iio/potentiostat/lmp91000.c                             | 2 ++
> >  2 files changed, 8 insertions(+), 3 deletions(-)  
> 
> Reviewed-by: Rob Herring <robh@kernel.org>

Applied to the togreg branch of iio.git and pushed out as testing for the
autobuilders to play with it.

Thanks,

Jonathan

^ permalink raw reply

* Re: [PATCH] iio: adc: stm32-dfsdm: Add support for stm32mp1
From: Jonathan Cameron @ 2018-05-12  9:33 UTC (permalink / raw)
  To: Rob Herring
  Cc: Fabrice Gasnier, alexandre.torgue, mark.rutland, mcoquelin.stm32,
	lars, knaack.h, pmeerw, linux-iio, devicetree, linux-arm-kernel,
	linux-kernel, benjamin.gaignard, arnaud.pouliquen
In-Reply-To: <20180507205151.GA25321@rob-hp-laptop>

On Mon, 7 May 2018 15:51:51 -0500
Rob Herring <robh@kernel.org> wrote:

> On Wed, May 02, 2018 at 03:05:23PM +0200, Fabrice Gasnier wrote:
> > Add support for DFSDM (Digital Filter For Sigma Delta Modulators)
> > to STM32MP1. This variant is close to STM32H7 DFSDM, it implements
> > 6 filter instances. Registers map is also increased.
> > 
> > Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> > ---
> >  .../bindings/iio/adc/st,stm32-dfsdm-adc.txt         |  7 +++++--
> >  drivers/iio/adc/stm32-dfsdm-core.c                  | 21 +++++++++++++++++++++
> >  2 files changed, 26 insertions(+), 2 deletions(-)  
> 
> Reviewed-by: Rob Herring <robh@kernel.org>
Applied.   Whilst doing this I note that I'm getting a few sparse
warnings for this driver that would be nice to clean up...

  CHECK   drivers/iio/adc/stm32-dfsdm-adc.c
drivers/iio/adc/stm32-dfsdm-adc.c:503:21: warning: expression using sizeof(void)
drivers/iio/adc/stm32-dfsdm-adc.c:503:21: warning: expression using sizeof(void)

Which is mystifying at first glance... Something deep and dirty in the min macro
but I can't see what..

drivers/iio/adc/stm32-dfsdm-adc.c:680:5: warning: symbol 'stm32_dfsdm_get_buff_cb' was not declared. Should it be static?
drivers/iio/adc/stm32-dfsdm-adc.c:703:5: warning: symbol 'stm32_dfsdm_release_buff_cb' was not declared. Should it be static?

Are rather more obvious

^ permalink raw reply

* Re: [PATCH v7 2/3] pinctrl: bcm2835: Add support for generic pinctrl binding
From: Stefan Wahren @ 2018-05-12  9:08 UTC (permalink / raw)
  To: linus.walleij, eric, Matheus Castello
  Cc: swarren, robh+dt, mark.rutland, devicetree, linux-kernel
In-Reply-To: <1763017303.92936.1525369119182@email.1und1.de>

Hi Linus,

> Stefan Wahren <stefan.wahren@i2se.com> hat am 3. Mai 2018 um 19:38 geschrieben:
> 
> 
> 
> > Matheus Castello <matheus@castello.eng.br> hat am 1. Mai 2018 um 02:42 geschrieben:
> > 
> > 
> > To keep driver up to date we add generic pinctrl binding support, which
> > covers the features used in this driver and has additional node properties
> > that this SoC has compatibility, so enabling future implementations of
> > these properties without the need to create new node properties in the
> > device trees.
> > 
> > The logic of this change maintain the old brcm legacy binding support in
> > order to keep the ABI stable.
> > 
> > Signed-off-by: Matheus Castello <matheus@castello.eng.br>
> > Reviewed-by: Eric Anholt <eric@anholt.net>
> 
> Patches 2 and 3 are
> 
> Acked-by: Stefan Wahren <stefan.wahren@i2se.com>
> 
> Thanks
> Stefan

gentle ping ...
> 
> Btw Stephen isn't BCM2835 maintainer anymore

^ permalink raw reply

* Re: [PATCH v2 2/2] leds: Add Spreadtrum SC27xx breathing light controller driver
From: Pavel Machek @ 2018-05-12  8:35 UTC (permalink / raw)
  To: Jacek Anaszewski
  Cc: Baolin Wang, robh+dt, mark.rutland, xiaotong.lu, broonie,
	linux-leds, devicetree, linux-kernel
In-Reply-To: <6289571e-7224-ca5e-1acf-5b099be57302@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2411 bytes --]

Hi!

> >>I disagree here. We already had the same discussion at the occasion
> >>of the patch [0] and it turned out to be a dead-end [1]. Now we have
> >>neither the driver nor the generic pattern interface.
> >>
> >>We also already have some older LED class drivers that implement custom
> >>pattern interfaces (e.g. drivers/leds/leds-lm3533.c) and the same
> >>approach can be applied in this case.
> >
> >Please don't. It was mistake to implement custom pattern interfaces
> >back then, it is still mistake now.
> 
> It turned out to be really hard to cover all known pattern generator
> implementations with generic interface. Sure, it would be nice to have
> one, but the whole discussion around [0] only unveiled the diversity of
> parameters to cover. And still new devices appear on the market.
> 
> We would have to propose a set of pattern schemes and allow to
> add new ones to it.

I believe that what I'm proposing below is close enough to universal.

> >If we really need solution now, I'd recommend "pattern" file with
> >
> >"<delta time> <brightness> <delta time> <brightness>".
> >
> >In this specific case, hardware only supports patterns in this format:
> >
> >low_time 0 rise_time 255 high_time 255 fall_time 0
> >
> >so driver would simply -EINVAL on anything else.
> 
> I'm fine with the pattern file, but the pattern format would have
> to be defined in the per-driver ABI documentation. It wouldn't much
> differ from the custom pattern approach though, unless I'm missing some
> gain of having pattern setting in a uniformly named single sysfs file
> (with semantics differing from driver to driver).

I'm proposing "<delta time> <brightness> ..." sysfs file. It certainly
covers this hardware, it would be enough to cover the Qualcomm Pulse
generator (IIRC), and it would cover most uses cases of Nokia N900's
LED.

Yes, we would need to document limitations of each chip. But it should
be easily possible to run pattern designed for Spreadtrum on N900,
even if it would not work the other way around.

(If someone really wants to run complex patterns on simple hardware,
we can provide software emulation using same file format. I believe I
still have that patch somewhere.)

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply

* Re: [PATCH] mtd: nand: Add support for reading ooblayout from device tree
From: Boris Brezillon @ 2018-05-12  5:55 UTC (permalink / raw)
  To: Paul Cercueil
  Cc: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut,
	Richard Weinberger, Rob Herring, Mark Rutland, devicetree,
	linux-mtd, linux-kernel
In-Reply-To: <20180511212912.1426-1-paul@crapouillou.net>

Hi Paul,

On Fri, 11 May 2018 23:29:12 +0200
Paul Cercueil <paul@crapouillou.net> wrote:

> By specifying the properties "mtd-oob-ecc" and "mtd-oob-free", it is
> now possible to specify from devicetree where the ECC data is located
> inside the OOB region.

Why would we want to do that? I mean, ECC/free regions are ECC
controller dependent (and NAND chip dependent for the OOB size part),
so there's no reason to describe it in the DT. And more importantly,
people are likely to get it wrong.

I'm curious, why do you need that?

Regards,

Boris

> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
>  Documentation/devicetree/bindings/mtd/nand.txt |  7 +++++
>  drivers/mtd/nand/raw/nand_base.c               | 42 ++++++++++++++++++++++++++
>  2 files changed, 49 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/nand.txt b/Documentation/devicetree/bindings/mtd/nand.txt
> index 8bb11d809429..118ea92787cb 100644
> --- a/Documentation/devicetree/bindings/mtd/nand.txt
> +++ b/Documentation/devicetree/bindings/mtd/nand.txt
> @@ -45,6 +45,13 @@ Optional NAND chip properties:
>  		     as reliable as possible.
>  - nand-rb: shall contain the native Ready/Busy ids.
>  
> +- nand-oob-ecc: <offset, length> couples of integers, specifying the offset
> +		     and length of the ECC data in the OOB region. There can be more
> +		     than one couple.
> +- nand-oob-free: <offset, length> couples of integers, specifying the offset
> +		     and length of a free-to-use area in the OOB region. There can be
> +		     more than one couple.
> +
>  The ECC strength and ECC step size properties define the correction capability
>  of a controller. Together, they say a controller can correct "{strength} bit
>  errors per {size} bytes".
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index 72f3a89da513..c905531effb0 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -213,6 +213,43 @@ static const struct mtd_ooblayout_ops nand_ooblayout_lp_hamming_ops = {
>  	.free = nand_ooblayout_free_lp_hamming,
>  };
>  
> +static int nand_oob_of(struct device_node *np, int section,
> +		       struct mtd_oob_region *oobregion, const char *prop)
> +{
> +	int ret = of_property_read_u32_index(np, prop,
> +			section * 2, &oobregion->offset);
> +	if (ret == -EOVERFLOW)
> +		return -ERANGE; /* We're done */
> +	if (ret)
> +		return ret;
> +
> +	ret = of_property_read_u32_index(np, prop,
> +			section * 2 + 1, &oobregion->length);
> +	if (ret == -EOVERFLOW)
> +		return -EINVAL; /* We must have an even number of integers */
> +
> +	return ret;
> +}
> +
> +static int nand_ooblayout_ecc_of(struct mtd_info *mtd, int section,
> +				 struct mtd_oob_region *oobregion)
> +{
> +	return nand_oob_of(mtd->dev.of_node, section,
> +			oobregion, "nand-oob-ecc");
> +}
> +
> +static int nand_ooblayout_free_of(struct mtd_info *mtd, int section,
> +				 struct mtd_oob_region *oobregion)
> +{
> +	return nand_oob_of(mtd->dev.of_node, section,
> +			oobregion, "nand-oob-free");
> +}
> +
> +static const struct mtd_ooblayout_ops nand_ooblayout_of_ops = {
> +	.ecc = nand_ooblayout_ecc_of,
> +	.free = nand_ooblayout_free_of,
> +};
> +
>  static int check_offs_len(struct mtd_info *mtd,
>  					loff_t ofs, uint64_t len)
>  {
> @@ -5843,6 +5880,11 @@ static int nand_dt_init(struct nand_chip *chip)
>  	if (of_property_read_bool(dn, "nand-ecc-maximize"))
>  		chip->ecc.options |= NAND_ECC_MAXIMIZE;
>  
> +	if (!chip->mtd.ooblayout &&
> +				of_property_read_bool(dn, "nand-oob-ecc") &&
> +				of_property_read_bool(dn, "nand-oob-free"))
> +		chip->mtd.ooblayout = &nand_ooblayout_of_ops;
> +
>  	return 0;
>  }
>  

^ permalink raw reply

* Re: [PATCH v2 03/12] arm: dts: mt7623: fix invalid memory node being generated
From: Sean Wang @ 2018-05-12  5:28 UTC (permalink / raw)
  To: Matthias Brugger
  Cc: robh+dt, mark.rutland, devicetree, linux-mediatek,
	linux-arm-kernel, linux-kernel, stable
In-Reply-To: <4dcb0435-1f98-0753-0feb-d17ad02077ae@gmail.com>

On Fri, 2018-05-11 at 17:03 +0200, Matthias Brugger wrote:
> 
> On 04/11/2018 10:53 AM, sean.wang@mediatek.com wrote:
> > From: Sean Wang <sean.wang@mediatek.com>
> > 
> > Below two wrong nodes in existing DTS files would cause a fail boot since
> > in fact the address 0 is not the correct place the memory device locates
> > at.
> > 
> > memory {
> >         device_type = "memory";
> >         reg = <0x0 0x0 0x0 0x0>;
> > };
> > 
> > memory@80000000 {
> >         reg = <0x0 0x80000000 0x0 0x40000000>;
> > };
> > 
> > In order to avoid having a memory node starting at address 0, we can't
> > include file skeleton64.dtsi and instead need to explicitly manually
> > define a few of properties the DTS relies on such as #address-cells
> > and #size-cells in root node and device_type in the node memory@80000000.
> > 
> > Cc: stable@vger.kernel.org
> > Fixes: 31ac0d69a1d4 ("ARM: dts: mediatek: add MT7623 basic support")
> > Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> > Cc: Rob Herring <robh+dt@kernel.org>
> > ---
> >  arch/arm/boot/dts/mt7623.dtsi                 | 3 ++-
> >  arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts | 1 +
> >  arch/arm/boot/dts/mt7623n-rfb.dtsi            | 1 +
> >  3 files changed, 4 insertions(+), 1 deletion(-)
> > 
> 
> merged. We would need this at least for mt2701 as well, correct?
> Would you mind to provide a patch.
> 
> Regards,
> Matthias
> 

Thanks! I totally think the same problem could happen on mt2701, so I'm 
happy to come up with a patch for that.

	Sean

> > diff --git a/arch/arm/boot/dts/mt7623.dtsi b/arch/arm/boot/dts/mt7623.dtsi
> > index fec4715..406a9f3 100644
> > --- a/arch/arm/boot/dts/mt7623.dtsi
> > +++ b/arch/arm/boot/dts/mt7623.dtsi
> > @@ -15,11 +15,12 @@
> >  #include <dt-bindings/phy/phy.h>
> >  #include <dt-bindings/reset/mt2701-resets.h>
> >  #include <dt-bindings/thermal/thermal.h>
> > -#include "skeleton64.dtsi"
> >  
> >  / {
> >  	compatible = "mediatek,mt7623";
> >  	interrupt-parent = <&sysirq>;
> > +	#address-cells = <2>;
> > +	#size-cells = <2>;
> >  
> >  	cpu_opp_table: opp-table {
> >  		compatible = "operating-points-v2";
> > diff --git a/arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts b/arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts
> > index bbf56f8..5938e4c 100644
> > --- a/arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts
> > +++ b/arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts
> > @@ -109,6 +109,7 @@
> >  	};
> >  
> >  	memory@80000000 {
> > +		device_type = "memory";
> >  		reg = <0 0x80000000 0 0x40000000>;
> >  	};
> >  };
> > diff --git a/arch/arm/boot/dts/mt7623n-rfb.dtsi b/arch/arm/boot/dts/mt7623n-rfb.dtsi
> > index a199ae7..343e8ef 100644
> > --- a/arch/arm/boot/dts/mt7623n-rfb.dtsi
> > +++ b/arch/arm/boot/dts/mt7623n-rfb.dtsi
> > @@ -40,6 +40,7 @@
> >  	};
> >  
> >  	memory@80000000 {
> > +		device_type = "memory";
> >  		reg = <0 0x80000000 0 0x40000000>;
> >  	};
> >  
> > 

^ permalink raw reply

* [PATCH v3 2/2] regulator: add QCOM RPMh regulator driver
From: David Collins @ 2018-05-12  2:28 UTC (permalink / raw)
  To: broonie, lgirdwood, robh+dt, mark.rutland
  Cc: David Collins, linux-arm-msm, linux-arm-kernel, devicetree,
	linux-kernel, rnayak, sboyd, dianders
In-Reply-To: <cover.1526088081.git.collinsd@codeaurora.org>

Add the QCOM RPMh regulator driver to manage PMIC regulators
which are controlled via RPMh on some Qualcomm Technologies, Inc.
SoCs.  RPMh is a hardware block which contains several
accelerators which are used to manage various hardware resources
that are shared between the processors of the SoC.  The final
hardware state of a regulator is determined within RPMh by
performing max aggregation of the requests made by all of the
processors.

Add support for PMIC regulator control via the voltage regulator
manager (VRM) and oscillator buffer (XOB) RPMh accelerators.  VRM
supports manipulation of enable state, voltage, mode, and
headroom voltage.  XOB supports manipulation of enable state.

Signed-off-by: David Collins <collinsd@codeaurora.org>
---
 drivers/regulator/Kconfig               |   9 +
 drivers/regulator/Makefile              |   1 +
 drivers/regulator/qcom-rpmh-regulator.c | 925 ++++++++++++++++++++++++++++++++
 3 files changed, 935 insertions(+)
 create mode 100644 drivers/regulator/qcom-rpmh-regulator.c

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 4efae3b..1a69bdc 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -671,6 +671,15 @@ config REGULATOR_QCOM_RPM
 	  Qualcomm RPM as a module. The module will be named
 	  "qcom_rpm-regulator".
 
+config REGULATOR_QCOM_RPMH
+	tristate "Qualcomm Technologies, Inc. RPMh regulator driver"
+	depends on QCOM_RPMH || COMPILE_TEST
+	help
+	  This driver supports control of PMIC regulators via the RPMh hardware
+	  block found on Qualcomm Technologies Inc. SoCs.  RPMh regulator
+	  control allows for voting on regulator state between multiple
+	  processors within the SoC.
+
 config REGULATOR_QCOM_SMD_RPM
 	tristate "Qualcomm SMD based RPM regulator driver"
 	depends on QCOM_SMD_RPM
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index d81fb02..906f048 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -77,6 +77,7 @@ obj-$(CONFIG_REGULATOR_MT6323)	+= mt6323-regulator.o
 obj-$(CONFIG_REGULATOR_MT6380)	+= mt6380-regulator.o
 obj-$(CONFIG_REGULATOR_MT6397)	+= mt6397-regulator.o
 obj-$(CONFIG_REGULATOR_QCOM_RPM) += qcom_rpm-regulator.o
+obj-$(CONFIG_REGULATOR_QCOM_RPMH) += qcom-rpmh-regulator.o
 obj-$(CONFIG_REGULATOR_QCOM_SMD_RPM) += qcom_smd-regulator.o
 obj-$(CONFIG_REGULATOR_QCOM_SPMI) += qcom_spmi-regulator.o
 obj-$(CONFIG_REGULATOR_PALMAS) += palmas-regulator.o
diff --git a/drivers/regulator/qcom-rpmh-regulator.c b/drivers/regulator/qcom-rpmh-regulator.c
new file mode 100644
index 0000000..991ecc1
--- /dev/null
+++ b/drivers/regulator/qcom-rpmh-regulator.c
@@ -0,0 +1,925 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2018, The Linux Foundation. All rights reserved. */
+
+#define pr_fmt(fmt) "%s: " fmt, __func__
+
+#include <linux/err.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+#include <linux/regulator/of_regulator.h>
+
+#include <soc/qcom/cmd-db.h>
+#include <soc/qcom/rpmh.h>
+
+#include <dt-bindings/regulator/qcom,rpmh-regulator.h>
+
+/**
+ * enum rpmh_regulator_type - supported RPMh accelerator types
+ * %VRM:	RPMh VRM accelerator which supports voting on enable, voltage,
+ *		mode, and headroom voltage of LDO, SMPS, and BOB type PMIC
+ *		regulators.
+ * %XOB:	RPMh XOB accelerator which supports voting on the enable state
+ *		of PMIC regulators.
+ */
+enum rpmh_regulator_type {
+	VRM,
+	XOB,
+};
+
+#define RPMH_VRM_HEADROOM_MAX_UV		511000
+
+#define RPMH_REGULATOR_REG_VRM_VOLTAGE		0x0
+#define RPMH_REGULATOR_REG_ENABLE		0x4
+#define RPMH_REGULATOR_REG_VRM_MODE		0x8
+#define RPMH_REGULATOR_REG_VRM_HEADROOM		0xC
+
+#define RPMH_REGULATOR_MODE_COUNT		4
+
+#define PMIC4_LDO_MODE_RETENTION		4
+#define PMIC4_LDO_MODE_LPM			5
+#define PMIC4_LDO_MODE_HPM			7
+
+#define PMIC4_SMPS_MODE_RETENTION		4
+#define PMIC4_SMPS_MODE_PFM			5
+#define PMIC4_SMPS_MODE_AUTO			6
+#define PMIC4_SMPS_MODE_PWM			7
+
+#define PMIC4_BOB_MODE_PASS			0
+#define PMIC4_BOB_MODE_PFM			1
+#define PMIC4_BOB_MODE_AUTO			2
+#define PMIC4_BOB_MODE_PWM			3
+
+/**
+ * struct rpmh_vreg_hw_data - RPMh regulator hardware configurations
+ * @regulator_type:		RPMh accelerator type used to manage this
+ *				regulator
+ * @ops:			Pointer to regulator ops callback structure
+ * @voltage_range:		The single range of voltages supported by this
+ *				PMIC regulator type
+ * @n_voltages:			The number of unique voltage set points defined
+ *				by voltage_range
+ * @pmic_mode_map:		Array indexed by regulator framework mode
+ *				containing PMIC hardware modes.  Must be large
+ *				enough to index all framework modes supported
+ *				by this regulator hardware type.
+ * @of_map_mode:		Maps an RPMH_REGULATOR_MODE_* mode value defined
+ *				in device tree to a regulator framework mode
+ */
+struct rpmh_vreg_hw_data {
+	enum rpmh_regulator_type		regulator_type;
+	const struct regulator_ops		*ops;
+	const struct regulator_linear_range	voltage_range;
+	int					n_voltages;
+	const int				*pmic_mode_map;
+	unsigned int			      (*of_map_mode)(unsigned int mode);
+};
+
+/**
+ * struct rpmh_vreg - individual RPMh regulator data structure encapsulating a
+ *		single regulator device
+ * @dev:			Device pointer for the top-level PMIC RPMh
+ *				regulator parent device.  This is used as a
+ *				handle in RPMh write requests.
+ * @addr:			Base address of the regulator resource within
+ *				an RPMh accelerator
+ * @rdesc:			Regulator descriptor
+ * @hw_data:			PMIC regulator configuration data for this RPMh
+ *				regulator
+ * @always_wait_for_ack:	Boolean flag indicating if a request must always
+ *				wait for an ACK from RPMh before continuing even
+ *				if it corresponds to a strictly lower power
+ *				state (e.g. enabled --> disabled).
+ * @drms_modes:			Array of regulator framework modes which can
+ *				be configured dynamically for this regulator
+ *				via the set_load() callback.
+ * @drms_mode_max_uAs:		Array of maximum load currents in microamps
+ *				supported by the corresponding modes in
+ *				drms_mode.  Elements must be specified in
+ *				strictly increasing order.
+ * @drms_mode_count:		The number of elements in drms_mode array.
+ * @enabled:			Boolean indicating if the regulator is enabled
+ *				or not
+ * @bypassed:			Boolean indicating if the regulator is in
+ *				bypass (pass-through) mode or not.  This is
+ *				only used by BOB rpmh-regulator resources.
+ * @voltage_selector:		Selector used for get_voltage_sel() and
+ *				set_voltage_sel() callbacks
+ * @mode:			RPMh VRM regulator current framework mode
+ */
+struct rpmh_vreg {
+	struct device			*dev;
+	u32				addr;
+	struct regulator_desc		rdesc;
+	const struct rpmh_vreg_hw_data	*hw_data;
+	bool				always_wait_for_ack;
+	unsigned int			*drms_modes;
+	int				*drms_mode_max_uAs;
+	size_t				drms_mode_count;
+
+	bool				enabled;
+	bool				bypassed;
+	int				voltage_selector;
+	unsigned int			mode;
+};
+
+/**
+ * struct rpmh_vreg_init_data - initialization data for an RPMh regulator
+ * @name:			Name for the regulator which also corresponds
+ *				to the device tree subnode name of the regulator
+ * @resource_name:		RPMh regulator resource name format string.
+ *				This must include exactly one field: '%s' which
+ *				is filled at run-time with the PMIC ID provided
+ *				by device tree property qcom,pmic-id.  Example:
+ *				"ldo%s1" for RPMh resource "ldoa1".
+ * @supply_name:		Parent supply regulator name
+ * @hw_data:			Configuration data for this PMIC regulator type
+ */
+struct rpmh_vreg_init_data {
+	const char			*name;
+	const char			*resource_name;
+	const char			*supply_name;
+	const struct rpmh_vreg_hw_data	*hw_data;
+};
+
+/**
+ * rpmh_regulator_send_request() - send the request to RPMh
+ * @vreg:		Pointer to the RPMh regulator
+ * @cmd:		RPMh commands to send
+ * @count:		Size of cmd array
+ * @wait_for_ack:	Boolean indicating if execution must wait until the
+ *			request has been acknowledged as complete
+ *
+ * Return: 0 on success, errno on failure
+ */
+static int rpmh_regulator_send_request(struct rpmh_vreg *vreg,
+			struct tcs_cmd *cmd, int count, bool wait_for_ack)
+{
+	int ret;
+
+	if (wait_for_ack || vreg->always_wait_for_ack)
+		ret = rpmh_write(vreg->dev, RPMH_ACTIVE_ONLY_STATE, cmd, count);
+	else
+		ret = rpmh_write_async(vreg->dev, RPMH_ACTIVE_ONLY_STATE, cmd,
+					count);
+
+	return ret;
+}
+
+static int rpmh_regulator_is_enabled(struct regulator_dev *rdev)
+{
+	struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
+
+	return vreg->enabled;
+}
+
+static int rpmh_regulator_enable(struct regulator_dev *rdev)
+{
+	struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
+	struct tcs_cmd cmd = {
+		.addr = vreg->addr + RPMH_REGULATOR_REG_ENABLE,
+		.data = 1,
+	};
+	int ret;
+
+	ret = rpmh_regulator_send_request(vreg, &cmd, 1, true);
+
+	if (!ret)
+		vreg->enabled = true;
+
+	return ret;
+}
+
+static int rpmh_regulator_disable(struct regulator_dev *rdev)
+{
+	struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
+	struct tcs_cmd cmd = {
+		.addr = vreg->addr + RPMH_REGULATOR_REG_ENABLE,
+		.data = 0,
+	};
+	int ret;
+
+	ret = rpmh_regulator_send_request(vreg, &cmd, 1, false);
+
+	if (!ret)
+		vreg->enabled = false;
+
+	return ret;
+}
+
+static int rpmh_regulator_vrm_set_voltage_sel(struct regulator_dev *rdev,
+				unsigned int selector)
+{
+	struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
+	struct tcs_cmd cmd = {
+		.addr = vreg->addr + RPMH_REGULATOR_REG_VRM_VOLTAGE,
+	};
+	int ret;
+
+	/* VRM voltage control register is set with voltage in millivolts. */
+	cmd.data = DIV_ROUND_UP(regulator_list_voltage_linear_range(rdev,
+							selector), 1000);
+
+	ret = rpmh_regulator_send_request(vreg, &cmd, 1,
+					  selector > vreg->voltage_selector);
+	if (!ret)
+		vreg->voltage_selector = selector;
+
+	return 0;
+}
+
+static int rpmh_regulator_vrm_get_voltage_sel(struct regulator_dev *rdev)
+{
+	struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
+
+	return vreg->voltage_selector;
+}
+
+static int rpmh_regulator_vrm_set_mode_bypass(struct rpmh_vreg *vreg,
+					unsigned int mode, bool bypassed)
+{
+	struct tcs_cmd cmd = {
+		.addr = vreg->addr + RPMH_REGULATOR_REG_VRM_MODE,
+	};
+	int pmic_mode;
+
+	if (mode > REGULATOR_MODE_STANDBY)
+		return -EINVAL;
+
+	pmic_mode = vreg->hw_data->pmic_mode_map[mode];
+	if (pmic_mode < 0)
+		return pmic_mode;
+
+	cmd.data = bypassed ? PMIC4_BOB_MODE_PASS : pmic_mode;
+
+	return rpmh_regulator_send_request(vreg, &cmd, 1, true);
+}
+
+static int rpmh_regulator_vrm_set_mode(struct regulator_dev *rdev,
+					unsigned int mode)
+{
+	struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
+	int ret;
+
+	if (mode == vreg->mode)
+		return 0;
+
+	ret = rpmh_regulator_vrm_set_mode_bypass(vreg, mode, vreg->bypassed);
+	if (!ret)
+		vreg->mode = mode;
+
+	return ret;
+}
+
+static unsigned int rpmh_regulator_vrm_get_mode(struct regulator_dev *rdev)
+{
+	struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
+
+	return vreg->mode;
+}
+
+/**
+ * rpmh_regulator_vrm_set_load() - set the regulator mode based upon the load
+ *		current requested
+ * @rdev:		Regulator device pointer for the rpmh-regulator
+ * @load_uA:		Aggregated load current in microamps
+ *
+ * This function is used in the regulator_ops for VRM type RPMh regulator
+ * devices.  It updates the mode of the regulator using a table of maximum
+ * load currents per mode specified in device tree properties.
+ *
+ * Return: 0 on success, errno on failure
+ */
+static int rpmh_regulator_vrm_set_load(struct regulator_dev *rdev, int load_uA)
+{
+	struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
+	int i;
+
+	for (i = 0; i < vreg->drms_mode_count - 1; i++)
+		if (vreg->drms_mode_max_uAs[i] >= load_uA)
+			break;
+	if (load_uA > vreg->drms_mode_max_uAs[vreg->drms_mode_count - 1])
+		dev_warn(vreg->dev, "%s: requested load=%d uA greater than max=%d uA\n",
+			vreg->rdesc.name, load_uA,
+			vreg->drms_mode_max_uAs[vreg->drms_mode_count - 1]);
+
+	return rpmh_regulator_vrm_set_mode(rdev, vreg->drms_modes[i]);
+}
+
+static int rpmh_regulator_vrm_set_bypass(struct regulator_dev *rdev,
+				bool enable)
+{
+	struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
+	int ret;
+
+	if (vreg->bypassed == enable)
+		return 0;
+
+	ret = rpmh_regulator_vrm_set_mode_bypass(vreg, vreg->mode, enable);
+	if (!ret)
+		vreg->bypassed = enable;
+
+	return ret;
+}
+
+static int rpmh_regulator_vrm_get_bypass(struct regulator_dev *rdev,
+				bool *enable)
+{
+	struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
+
+	*enable = vreg->bypassed;
+
+	return 0;
+}
+
+static const struct regulator_ops rpmh_regulator_vrm_ops = {
+	.enable			= rpmh_regulator_enable,
+	.disable		= rpmh_regulator_disable,
+	.is_enabled		= rpmh_regulator_is_enabled,
+	.set_voltage_sel	= rpmh_regulator_vrm_set_voltage_sel,
+	.get_voltage_sel	= rpmh_regulator_vrm_get_voltage_sel,
+	.list_voltage		= regulator_list_voltage_linear_range,
+	.set_mode		= rpmh_regulator_vrm_set_mode,
+	.get_mode		= rpmh_regulator_vrm_get_mode,
+	.set_load		= rpmh_regulator_vrm_set_load,
+};
+
+static const struct regulator_ops rpmh_regulator_vrm_bypass_ops = {
+	.enable			= rpmh_regulator_enable,
+	.disable		= rpmh_regulator_disable,
+	.is_enabled		= rpmh_regulator_is_enabled,
+	.set_voltage_sel	= rpmh_regulator_vrm_set_voltage_sel,
+	.get_voltage_sel	= rpmh_regulator_vrm_get_voltage_sel,
+	.list_voltage		= regulator_list_voltage_linear_range,
+	.set_mode		= rpmh_regulator_vrm_set_mode,
+	.get_mode		= rpmh_regulator_vrm_get_mode,
+	.set_load		= rpmh_regulator_vrm_set_load,
+	.set_bypass		= rpmh_regulator_vrm_set_bypass,
+	.get_bypass		= rpmh_regulator_vrm_get_bypass,
+};
+
+static const struct regulator_ops rpmh_regulator_xob_ops = {
+	.enable			= rpmh_regulator_enable,
+	.disable		= rpmh_regulator_disable,
+	.is_enabled		= rpmh_regulator_is_enabled,
+};
+
+/**
+ * rpmh_regulator_parse_vrm_modes() - parse the supported mode configurations
+ *		for a VRM RPMh resource from device tree
+ * vreg:		Pointer to the individual rpmh-regulator resource
+ * dev:			Pointer to the top level rpmh-regulator PMIC device
+ * node:		Pointer to the individual rpmh-regulator resource
+ *			device node
+ *
+ * This function initializes the drms_modes[] and drms_mode_max_uAs[] arrays of
+ * vreg based upon the values of optional device tree properties.
+ *
+ * Return: 0 on success, errno on failure
+ */
+static int rpmh_regulator_parse_vrm_modes(struct rpmh_vreg *vreg,
+				struct device *dev, struct device_node *node)
+{
+	const char *prop;
+	int i, len, ret, mode;
+	u32 *buf;
+
+	/* qcom,allowed-drms-modes is optional */
+	prop = "qcom,allowed-drms-modes";
+	len = of_property_count_elems_of_size(node, prop, sizeof(u32));
+	if (len < 0)
+		return 0;
+
+	vreg->drms_modes = devm_kcalloc(dev, len, sizeof(*vreg->drms_modes),
+					GFP_KERNEL);
+	vreg->drms_mode_max_uAs = devm_kcalloc(dev, len,
+					       sizeof(*vreg->drms_mode_max_uAs),
+					       GFP_KERNEL);
+	if (!vreg->drms_modes || !vreg->drms_mode_max_uAs)
+		return -ENOMEM;
+	vreg->drms_mode_count = len;
+
+	buf = kcalloc(len, sizeof(*buf), GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	ret = of_property_read_u32_array(node, prop, buf, len);
+	if (ret < 0) {
+		dev_err(dev, "%s: unable to read %s, ret=%d\n",
+			node->name, prop, ret);
+		goto done;
+	}
+
+	for (i = 0; i < len; i++) {
+		mode = vreg->hw_data->of_map_mode(buf[i]);
+		if (mode == REGULATOR_MODE_INVALID) {
+			dev_err(dev, "%s: element %d of %s = %u is invalid for this regulator\n",
+				node->name, i, prop, buf[i]);
+			ret = -EINVAL;
+			goto done;
+		}
+
+		vreg->drms_modes[i] = mode;
+	}
+
+	prop = "qcom,drms-mode-max-microamps";
+	len = of_property_count_elems_of_size(node, prop, sizeof(u32));
+	if (len != vreg->drms_mode_count) {
+		dev_err(dev, "%s: invalid element count=%d for %s\n",
+			node->name, len, prop);
+		ret = -EINVAL;
+		goto done;
+	}
+
+	ret = of_property_read_u32_array(node, prop, buf, len);
+	if (ret < 0) {
+		dev_err(dev, "%s: unable to read %s, ret=%d\n",
+			node->name, prop, ret);
+		goto done;
+	}
+
+	for (i = 0; i < len; i++) {
+		vreg->drms_mode_max_uAs[i] = buf[i];
+
+		if (i > 0 && vreg->drms_mode_max_uAs[i]
+				<= vreg->drms_mode_max_uAs[i - 1]) {
+			dev_err(dev, "%s: %s elements are not in ascending order\n",
+				node->name, prop);
+			ret = -EINVAL;
+			goto done;
+		}
+	}
+
+done:
+	kfree(buf);
+	return ret;
+}
+
+/**
+ * rpmh_regulator_load_default_parameters() - initialize the RPMh resource
+ *		request for this regulator based on optional device tree
+ *		properties
+ * vreg:		Pointer to the individual rpmh-regulator resource
+ * dev:			Pointer to the top level rpmh-regulator PMIC device
+ * node:		Pointer to the individual rpmh-regulator resource
+ *			device node
+ *
+ * Return: 0 on success, errno on failure
+ */
+static int rpmh_regulator_load_default_parameters(struct rpmh_vreg *vreg,
+				struct device *dev, struct device_node *node)
+{
+	struct tcs_cmd cmd[2] = {};
+	const struct regulator_linear_range *range;
+	const char *prop;
+	int cmd_count = 0;
+	int ret, selector;
+	u32 uV;
+
+	if (vreg->hw_data->regulator_type == VRM) {
+		/*
+		 * This will be set when devm_regulator_register() is called
+		 * if regulator-initial-mode is specified in device tree.
+		 */
+		vreg->mode = REGULATOR_MODE_INVALID;
+
+		prop = "qcom,headroom-microvolt";
+		ret = of_property_read_u32(node, prop, &uV);
+		if (!ret) {
+			if (uV > RPMH_VRM_HEADROOM_MAX_UV) {
+				dev_err(dev, "%s: %s=%u is invalid\n",
+					node->name, prop, uV);
+				return -EINVAL;
+			}
+
+			cmd[cmd_count].addr
+				= vreg->addr + RPMH_REGULATOR_REG_VRM_HEADROOM;
+			cmd[cmd_count++].data = DIV_ROUND_UP(uV, 1000);
+		}
+
+		prop = "qcom,regulator-initial-microvolt";
+		ret = of_property_read_u32(node, prop, &uV);
+		if (!ret) {
+			range = &vreg->hw_data->voltage_range;
+			selector = DIV_ROUND_UP(uV - range->min_uV,
+					range->uV_step) + range->min_sel;
+			if (uV < range->min_uV || selector > range->max_sel) {
+				dev_err(dev, "%s: %s=%u is invalid\n",
+					node->name, prop, uV);
+				return -EINVAL;
+			}
+
+			vreg->voltage_selector = selector;
+
+			cmd[cmd_count].addr
+				= vreg->addr + RPMH_REGULATOR_REG_VRM_VOLTAGE;
+			cmd[cmd_count++].data
+				= DIV_ROUND_UP(selector * range->uV_step
+						+ range->min_uV, 1000);
+		} else {
+			/*
+			 * Default the voltage selector to an error value in the
+			 * case that qcom,regulator-initial-microvolt is not
+			 * specified in device tree since the true voltage is
+			 * not known.  Note that this value causes
+			 * devm_regulator_register() to fail in the case that
+			 * regulator-min-microvolt and regulator-max-microvolt
+			 * are specified in device tree due to
+			 * machine_constraints_voltage() bailing when the
+			 * get_voltage_sel() callback returns this error value.
+			 */
+			vreg->voltage_selector = -EINVAL;
+		}
+	}
+
+	if (cmd_count) {
+		ret = rpmh_regulator_send_request(vreg, cmd, cmd_count, true);
+		if (ret < 0) {
+			dev_err(dev, "%s: could not send default config, ret=%d\n",
+				node->name, ret);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+/**
+ * rpmh_regulator_init_vreg() - initialize all attributes of an rpmh-regulator
+ * vreg:		Pointer to the individual rpmh-regulator resource
+ * dev:			Pointer to the top level rpmh-regulator PMIC device
+ * node:		Pointer to the individual rpmh-regulator resource
+ *			device node
+ * pmic_id:		String used to identify the top level rpmh-regulator
+ *			PMIC device on the board
+ * rpmh_data:		Pointer to a null-terminated array of rpmh-regulator
+ *			resources defined for the top level PMIC device
+ *
+ * Return: 0 on success, errno on failure
+ */
+static int rpmh_regulator_init_vreg(struct rpmh_vreg *vreg, struct device *dev,
+				struct device_node *node, const char *pmic_id,
+				const struct rpmh_vreg_init_data *rpmh_data)
+{
+	struct regulator_config reg_config = {};
+	char rpmh_resource_name[20] = "";
+	struct regulator_dev *rdev;
+	enum rpmh_regulator_type type;
+	struct regulator_init_data *init_data;
+	int ret;
+
+	vreg->dev = dev;
+
+	for (; rpmh_data->name; rpmh_data++)
+		if (!strcmp(rpmh_data->name, node->name))
+			break;
+
+	if (!rpmh_data->name) {
+		dev_err(dev, "Unknown regulator %s\n", node->name);
+		return -EINVAL;
+	}
+
+	scnprintf(rpmh_resource_name, sizeof(rpmh_resource_name),
+		rpmh_data->resource_name, pmic_id);
+
+	vreg->addr = cmd_db_read_addr(rpmh_resource_name);
+	if (!vreg->addr) {
+		dev_err(dev, "%s: could not find RPMh address for resource %s\n",
+			node->name, rpmh_resource_name);
+		return -ENODEV;
+	}
+
+	vreg->rdesc.name = rpmh_data->name;
+	vreg->rdesc.supply_name = rpmh_data->supply_name;
+	vreg->hw_data = rpmh_data->hw_data;
+
+	if (rpmh_data->hw_data->n_voltages) {
+		vreg->rdesc.linear_ranges = &rpmh_data->hw_data->voltage_range;
+		vreg->rdesc.n_linear_ranges = 1;
+		vreg->rdesc.n_voltages = rpmh_data->hw_data->n_voltages;
+	}
+
+	type = rpmh_data->hw_data->regulator_type;
+	if (type == VRM) {
+		ret = rpmh_regulator_parse_vrm_modes(vreg, dev, node);
+		if (ret < 0)
+			return ret;
+	}
+
+	vreg->always_wait_for_ack = of_property_read_bool(node,
+						"qcom,always-wait-for-ack");
+
+	vreg->rdesc.owner	= THIS_MODULE;
+	vreg->rdesc.type	= REGULATOR_VOLTAGE;
+	vreg->rdesc.ops		= vreg->hw_data->ops;
+	vreg->rdesc.of_map_mode	= vreg->hw_data->of_map_mode;
+
+	init_data = of_get_regulator_init_data(dev, node, &vreg->rdesc);
+	if (!init_data)
+		return -ENOMEM;
+
+	if (type == XOB && init_data->constraints.min_uV &&
+	    init_data->constraints.min_uV == init_data->constraints.max_uV) {
+		vreg->rdesc.fixed_uV = init_data->constraints.min_uV;
+		vreg->rdesc.n_voltages = 1;
+	}
+
+	reg_config.dev		= dev;
+	reg_config.init_data	= init_data;
+	reg_config.of_node	= node;
+	reg_config.driver_data	= vreg;
+
+	ret = rpmh_regulator_load_default_parameters(vreg, dev, node);
+	if (ret < 0)
+		return ret;
+
+	rdev = devm_regulator_register(dev, &vreg->rdesc, &reg_config);
+	if (IS_ERR(rdev)) {
+		ret = PTR_ERR(rdev);
+		rdev = NULL;
+		dev_err(dev, "%s: devm_regulator_register() failed, ret=%d\n",
+			node->name, ret);
+		return ret;
+	}
+
+	dev_dbg(dev, "%s regulator registered for RPMh resource %s @ 0x%05X\n",
+		node->name, rpmh_resource_name, vreg->addr);
+
+	return ret;
+}
+
+static const int pmic_mode_map_pmic4_ldo[REGULATOR_MODE_STANDBY + 1] = {
+	[REGULATOR_MODE_INVALID] = -EINVAL,
+	[REGULATOR_MODE_STANDBY] = PMIC4_LDO_MODE_RETENTION,
+	[REGULATOR_MODE_IDLE]    = PMIC4_LDO_MODE_LPM,
+	[REGULATOR_MODE_NORMAL]  = -EINVAL,
+	[REGULATOR_MODE_FAST]    = PMIC4_LDO_MODE_HPM,
+};
+
+static unsigned int rpmh_regulator_pmic4_ldo_of_map_mode(unsigned int mode)
+{
+	static const unsigned int of_mode_map[RPMH_REGULATOR_MODE_COUNT] = {
+		[RPMH_REGULATOR_MODE_RET]  = REGULATOR_MODE_STANDBY,
+		[RPMH_REGULATOR_MODE_LPM]  = REGULATOR_MODE_IDLE,
+		[RPMH_REGULATOR_MODE_AUTO] = REGULATOR_MODE_INVALID,
+		[RPMH_REGULATOR_MODE_HPM]  = REGULATOR_MODE_FAST,
+	};
+
+	if (mode >= RPMH_REGULATOR_MODE_COUNT)
+		return -EINVAL;
+
+	return of_mode_map[mode];
+}
+
+static const int pmic_mode_map_pmic4_smps[REGULATOR_MODE_STANDBY + 1] = {
+	[REGULATOR_MODE_INVALID] = -EINVAL,
+	[REGULATOR_MODE_STANDBY] = PMIC4_SMPS_MODE_RETENTION,
+	[REGULATOR_MODE_IDLE]    = PMIC4_SMPS_MODE_PFM,
+	[REGULATOR_MODE_NORMAL]  = PMIC4_SMPS_MODE_AUTO,
+	[REGULATOR_MODE_FAST]    = PMIC4_SMPS_MODE_PWM,
+};
+
+static unsigned int rpmh_regulator_pmic4_smps_of_map_mode(unsigned int mode)
+{
+	static const unsigned int of_mode_map[RPMH_REGULATOR_MODE_COUNT] = {
+		[RPMH_REGULATOR_MODE_RET]  = REGULATOR_MODE_STANDBY,
+		[RPMH_REGULATOR_MODE_LPM]  = REGULATOR_MODE_IDLE,
+		[RPMH_REGULATOR_MODE_AUTO] = REGULATOR_MODE_NORMAL,
+		[RPMH_REGULATOR_MODE_HPM]  = REGULATOR_MODE_FAST,
+	};
+
+	if (mode >= RPMH_REGULATOR_MODE_COUNT)
+		return -EINVAL;
+
+	return of_mode_map[mode];
+}
+
+static const int pmic_mode_map_pmic4_bob[REGULATOR_MODE_STANDBY + 1] = {
+	[REGULATOR_MODE_INVALID] = -EINVAL,
+	[REGULATOR_MODE_STANDBY] = -EINVAL,
+	[REGULATOR_MODE_IDLE]    = PMIC4_BOB_MODE_PFM,
+	[REGULATOR_MODE_NORMAL]  = PMIC4_BOB_MODE_AUTO,
+	[REGULATOR_MODE_FAST]    = PMIC4_BOB_MODE_PWM,
+};
+
+static unsigned int rpmh_regulator_pmic4_bob_of_map_mode(unsigned int mode)
+{
+	static const unsigned int of_mode_map[RPMH_REGULATOR_MODE_COUNT] = {
+		[RPMH_REGULATOR_MODE_RET]  = REGULATOR_MODE_INVALID,
+		[RPMH_REGULATOR_MODE_LPM]  = REGULATOR_MODE_IDLE,
+		[RPMH_REGULATOR_MODE_AUTO] = REGULATOR_MODE_NORMAL,
+		[RPMH_REGULATOR_MODE_HPM]  = REGULATOR_MODE_FAST,
+	};
+
+	if (mode >= RPMH_REGULATOR_MODE_COUNT)
+		return -EINVAL;
+
+	return of_mode_map[mode];
+}
+
+static const struct rpmh_vreg_hw_data pmic4_pldo = {
+	.regulator_type = VRM,
+	.ops = &rpmh_regulator_vrm_ops,
+	.voltage_range = REGULATOR_LINEAR_RANGE(1664000, 0, 255, 8000),
+	.n_voltages = 256,
+	.pmic_mode_map = pmic_mode_map_pmic4_ldo,
+	.of_map_mode = rpmh_regulator_pmic4_ldo_of_map_mode,
+};
+
+static const struct rpmh_vreg_hw_data pmic4_pldo_lv = {
+	.regulator_type = VRM,
+	.ops = &rpmh_regulator_vrm_ops,
+	.voltage_range = REGULATOR_LINEAR_RANGE(1256000, 0, 127, 8000),
+	.n_voltages = 128,
+	.pmic_mode_map = pmic_mode_map_pmic4_ldo,
+	.of_map_mode = rpmh_regulator_pmic4_ldo_of_map_mode,
+};
+
+static const struct rpmh_vreg_hw_data pmic4_nldo = {
+	.regulator_type = VRM,
+	.ops = &rpmh_regulator_vrm_ops,
+	.voltage_range = REGULATOR_LINEAR_RANGE(312000, 0, 127, 8000),
+	.n_voltages = 128,
+	.pmic_mode_map = pmic_mode_map_pmic4_ldo,
+	.of_map_mode = rpmh_regulator_pmic4_ldo_of_map_mode,
+};
+
+static const struct rpmh_vreg_hw_data pmic4_hfsmps3 = {
+	.regulator_type = VRM,
+	.ops = &rpmh_regulator_vrm_ops,
+	.voltage_range = REGULATOR_LINEAR_RANGE(320000, 0, 215, 8000),
+	.n_voltages = 216,
+	.pmic_mode_map = pmic_mode_map_pmic4_smps,
+	.of_map_mode = rpmh_regulator_pmic4_smps_of_map_mode,
+};
+
+static const struct rpmh_vreg_hw_data pmic4_ftsmps426 = {
+	.regulator_type = VRM,
+	.ops = &rpmh_regulator_vrm_ops,
+	.voltage_range = REGULATOR_LINEAR_RANGE(320000, 0, 258, 4000),
+	.n_voltages = 259,
+	.pmic_mode_map = pmic_mode_map_pmic4_smps,
+	.of_map_mode = rpmh_regulator_pmic4_smps_of_map_mode,
+};
+
+static const struct rpmh_vreg_hw_data pmic4_bob = {
+	.regulator_type = VRM,
+	.ops = &rpmh_regulator_vrm_bypass_ops,
+	.voltage_range = REGULATOR_LINEAR_RANGE(1824000, 0, 83, 32000),
+	.n_voltages = 84,
+	.pmic_mode_map = pmic_mode_map_pmic4_bob,
+	.of_map_mode = rpmh_regulator_pmic4_bob_of_map_mode,
+};
+
+static const struct rpmh_vreg_hw_data pmic4_lvs = {
+	.regulator_type = XOB,
+	.ops = &rpmh_regulator_xob_ops,
+	/* LVS hardware does not support voltage or mode configuration. */
+};
+
+#define RPMH_VREG(_name, _resource_name, _hw_data, _supply_name) \
+{ \
+	.name		= _name, \
+	.resource_name	= _resource_name, \
+	.hw_data	= _hw_data, \
+	.supply_name	= _supply_name, \
+}
+
+static const struct rpmh_vreg_init_data pm8998_vreg_data[] = {
+	RPMH_VREG("smps1",  "smp%s1",  &pmic4_ftsmps426, "vdd-s1"),
+	RPMH_VREG("smps2",  "smp%s2",  &pmic4_ftsmps426, "vdd-s2"),
+	RPMH_VREG("smps3",  "smp%s3",  &pmic4_hfsmps3,   "vdd-s3"),
+	RPMH_VREG("smps4",  "smp%s4",  &pmic4_hfsmps3,   "vdd-s4"),
+	RPMH_VREG("smps5",  "smp%s5",  &pmic4_hfsmps3,   "vdd-s5"),
+	RPMH_VREG("smps6",  "smp%s6",  &pmic4_ftsmps426, "vdd-s6"),
+	RPMH_VREG("smps7",  "smp%s7",  &pmic4_ftsmps426, "vdd-s7"),
+	RPMH_VREG("smps8",  "smp%s8",  &pmic4_ftsmps426, "vdd-s8"),
+	RPMH_VREG("smps9",  "smp%s9",  &pmic4_ftsmps426, "vdd-s9"),
+	RPMH_VREG("smps10", "smp%s10", &pmic4_ftsmps426, "vdd-s10"),
+	RPMH_VREG("smps11", "smp%s11", &pmic4_ftsmps426, "vdd-s11"),
+	RPMH_VREG("smps12", "smp%s12", &pmic4_ftsmps426, "vdd-s12"),
+	RPMH_VREG("smps13", "smp%s13", &pmic4_ftsmps426, "vdd-s13"),
+	RPMH_VREG("ldo1",   "ldo%s1",  &pmic4_nldo,      "vdd-l1-l27"),
+	RPMH_VREG("ldo2",   "ldo%s2",  &pmic4_nldo,      "vdd-l2-l8-l17"),
+	RPMH_VREG("ldo3",   "ldo%s3",  &pmic4_nldo,      "vdd-l3-l11"),
+	RPMH_VREG("ldo4",   "ldo%s4",  &pmic4_nldo,      "vdd-l4-l5"),
+	RPMH_VREG("ldo5",   "ldo%s5",  &pmic4_nldo,      "vdd-l4-l5"),
+	RPMH_VREG("ldo6",   "ldo%s6",  &pmic4_pldo,      "vdd-l6"),
+	RPMH_VREG("ldo7",   "ldo%s7",  &pmic4_pldo_lv,   "vdd-l7-l12-l14-l15"),
+	RPMH_VREG("ldo8",   "ldo%s8",  &pmic4_nldo,      "vdd-l2-l8-l17"),
+	RPMH_VREG("ldo9",   "ldo%s9",  &pmic4_pldo,      "vdd-l9"),
+	RPMH_VREG("ldo10",  "ldo%s10", &pmic4_pldo,      "vdd-l10-l23-l25"),
+	RPMH_VREG("ldo11",  "ldo%s11", &pmic4_nldo,      "vdd-l3-l11"),
+	RPMH_VREG("ldo12",  "ldo%s12", &pmic4_pldo_lv,   "vdd-l7-l12-l14-l15"),
+	RPMH_VREG("ldo13",  "ldo%s13", &pmic4_pldo,      "vdd-l13-l19-l21"),
+	RPMH_VREG("ldo14",  "ldo%s14", &pmic4_pldo_lv,   "vdd-l7-l12-l14-l15"),
+	RPMH_VREG("ldo15",  "ldo%s15", &pmic4_pldo_lv,   "vdd-l7-l12-l14-l15"),
+	RPMH_VREG("ldo16",  "ldo%s16", &pmic4_pldo,      "vdd-l16-l28"),
+	RPMH_VREG("ldo17",  "ldo%s17", &pmic4_nldo,      "vdd-l2-l8-l17"),
+	RPMH_VREG("ldo18",  "ldo%s18", &pmic4_pldo,      "vdd-l18-l22"),
+	RPMH_VREG("ldo19",  "ldo%s19", &pmic4_pldo,      "vdd-l13-l19-l21"),
+	RPMH_VREG("ldo20",  "ldo%s20", &pmic4_pldo,      "vdd-l20-l24"),
+	RPMH_VREG("ldo21",  "ldo%s21", &pmic4_pldo,      "vdd-l13-l19-l21"),
+	RPMH_VREG("ldo22",  "ldo%s22", &pmic4_pldo,      "vdd-l18-l22"),
+	RPMH_VREG("ldo23",  "ldo%s23", &pmic4_pldo,      "vdd-l10-l23-l25"),
+	RPMH_VREG("ldo24",  "ldo%s24", &pmic4_pldo,      "vdd-l20-l24"),
+	RPMH_VREG("ldo25",  "ldo%s25", &pmic4_pldo,      "vdd-l10-l23-l25"),
+	RPMH_VREG("ldo26",  "ldo%s26", &pmic4_nldo,      "vdd-l26"),
+	RPMH_VREG("ldo27",  "ldo%s27", &pmic4_nldo,      "vdd-l1-l27"),
+	RPMH_VREG("ldo28",  "ldo%s28", &pmic4_pldo,      "vdd-l16-l28"),
+	RPMH_VREG("lvs1",   "vs%s1",   &pmic4_lvs,       "vin-lvs-1-2"),
+	RPMH_VREG("lvs2",   "vs%s2",   &pmic4_lvs,       "vin-lvs-1-2"),
+	{},
+};
+
+static const struct rpmh_vreg_init_data pmi8998_vreg_data[] = {
+	RPMH_VREG("bob",    "bob%s1",  &pmic4_bob,       "vdd-bob"),
+	{},
+};
+
+static const struct rpmh_vreg_init_data pm8005_vreg_data[] = {
+	RPMH_VREG("smps1",  "smp%s1",  &pmic4_ftsmps426, "vdd-s1"),
+	RPMH_VREG("smps2",  "smp%s2",  &pmic4_ftsmps426, "vdd-s2"),
+	RPMH_VREG("smps3",  "smp%s3",  &pmic4_ftsmps426, "vdd-s3"),
+	RPMH_VREG("smps4",  "smp%s4",  &pmic4_ftsmps426, "vdd-s4"),
+	{},
+};
+
+static int rpmh_regulator_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	const struct rpmh_vreg_init_data *vreg_data;
+	struct device_node *node;
+	struct rpmh_vreg *vreg;
+	const char *pmic_id;
+	int ret;
+
+	ret = cmd_db_ready();
+	if (ret < 0) {
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "Command DB not available, ret=%d\n", ret);
+		return ret;
+	}
+
+	vreg_data = of_device_get_match_data(dev);
+	if (!vreg_data)
+		return -ENODEV;
+
+	ret = of_property_read_string(dev->of_node, "qcom,pmic-id", &pmic_id);
+	if (ret < 0) {
+		dev_err(dev, "qcom,pmic-id missing in DT node\n");
+		return ret;
+	}
+
+	for_each_available_child_of_node(dev->of_node, node) {
+		vreg = devm_kzalloc(dev, sizeof(*vreg), GFP_KERNEL);
+		if (!vreg) {
+			of_node_put(node);
+			return -ENOMEM;
+		}
+
+		ret = rpmh_regulator_init_vreg(vreg, dev, node, pmic_id,
+						vreg_data);
+		if (ret < 0) {
+			of_node_put(node);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static const struct of_device_id rpmh_regulator_match_table[] = {
+	{
+		.compatible = "qcom,pm8998-rpmh-regulators",
+		.data = pm8998_vreg_data,
+	},
+	{
+		.compatible = "qcom,pmi8998-rpmh-regulators",
+		.data = pmi8998_vreg_data,
+	},
+	{
+		.compatible = "qcom,pm8005-rpmh-regulators",
+		.data = pm8005_vreg_data,
+	},
+	{}
+};
+MODULE_DEVICE_TABLE(of, rpmh_regulator_match_table);
+
+static struct platform_driver rpmh_regulator_driver = {
+	.driver = {
+		.name = "qcom-rpmh-regulator",
+		.of_match_table	= of_match_ptr(rpmh_regulator_match_table),
+	},
+	.probe = rpmh_regulator_probe,
+};
+module_platform_driver(rpmh_regulator_driver);
+
+MODULE_DESCRIPTION("Qualcomm RPMh regulator driver");
+MODULE_LICENSE("GPL v2");
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply related

* [PATCH v3 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings
From: David Collins @ 2018-05-12  2:28 UTC (permalink / raw)
  To: broonie, lgirdwood, robh+dt, mark.rutland
  Cc: David Collins, linux-arm-msm, linux-arm-kernel, devicetree,
	linux-kernel, rnayak, sboyd, dianders
In-Reply-To: <cover.1526088081.git.collinsd@codeaurora.org>

Introduce bindings for RPMh regulator devices found on some
Qualcomm Technlogies, Inc. SoCs.  These devices allow a given
processor within the SoC to make PMIC regulator requests which
are aggregated within the RPMh hardware block along with requests
from other processors in the SoC to determine the final PMIC
regulator hardware state.

Signed-off-by: David Collins <collinsd@codeaurora.org>
---
 .../bindings/regulator/qcom,rpmh-regulator.txt     | 208 +++++++++++++++++++++
 .../dt-bindings/regulator/qcom,rpmh-regulator.h    |  36 ++++
 2 files changed, 244 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.txt
 create mode 100644 include/dt-bindings/regulator/qcom,rpmh-regulator.h

diff --git a/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.txt b/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.txt
new file mode 100644
index 0000000..ad2185e
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.txt
@@ -0,0 +1,208 @@
+Qualcomm Technologies, Inc. RPMh Regulators
+
+rpmh-regulator devices support PMIC regulator management via the Voltage
+Regulator Manager (VRM) and Oscillator Buffer (XOB) RPMh accelerators.  The APPS
+processor communicates with these hardware blocks via a Resource State
+Coordinator (RSC) using command packets.  The VRM allows changing four
+parameters for a given regulator: enable state, output voltage, operating mode,
+and minimum headroom voltage.  The XOB allows changing only a single parameter
+for a given regulator: its enable state.  Despite its name, the XOB is capable
+of controlling the enable state of any PMIC peripheral.  It is used for clock
+buffers, low-voltage switches, and LDO/SMPS regulators which have a fixed
+voltage and mode.
+
+=======================
+Required Node Structure
+=======================
+
+RPMh regulators must be described in two levels of device nodes.  The first
+level describes the PMIC containing the regulators and must reside within an
+RPMh device node.  The second level describes each regulator within the PMIC
+which is to be used on the board.  Each of these regulators maps to a single
+RPMh resource.
+
+The names used for regulator nodes must match those supported by a given PMIC.
+Supported regulator node names:
+	PM8998:		smps1 - smps13, ldo1 - ldo28, lvs1 - lvs2
+	PMI8998:	bob
+	PM8005:		smps1 - smps4
+
+========================
+First Level Nodes - PMIC
+========================
+
+- compatible
+	Usage:      required
+	Value type: <string>
+	Definition: Must be one of: "qcom,pm8998-rpmh-regulators",
+		    "qcom,pmi8998-rpmh-regulators" or
+		    "qcom,pm8005-rpmh-regulators".
+
+- qcom,pmic-id
+	Usage:      required
+	Value type: <string>
+	Definition: RPMh resource name suffix used for the regulators found on
+		    this PMIC.  Typical values: "a", "b", "c", "d", "e", "f".
+
+- vdd-s1-supply
+- vdd-s2-supply
+- vdd-s3-supply
+- vdd-s4-supply
+	Usage:      optional (PM8998 and PM8005 only)
+	Value type: <phandle>
+	Definition: phandle of the parent supply regulator of one or more of the
+		    regulators for this PMIC.
+
+- vdd-s5-supply
+- vdd-s6-supply
+- vdd-s7-supply
+- vdd-s8-supply
+- vdd-s9-supply
+- vdd-s10-supply
+- vdd-s11-supply
+- vdd-s12-supply
+- vdd-s13-supply
+- vdd-l1-l27-supply
+- vdd-l2-l8-l17-supply
+- vdd-l3-l11-supply
+- vdd-l4-l5-supply
+- vdd-l6-supply
+- vdd-l7-l12-l14-l15-supply
+- vdd-l9-supply
+- vdd-l10-l23-l25-supply
+- vdd-l13-l19-l21-supply
+- vdd-l16-l28-supply
+- vdd-l18-l22-supply
+- vdd-l20-l24-supply
+- vdd-l26-supply
+- vin-lvs-1-2-supply
+	Usage:      optional (PM8998 only)
+	Value type: <phandle>
+	Definition: phandle of the parent supply regulator of one or more of the
+		    regulators for this PMIC.
+
+- vdd-bob-supply
+	Usage:      optional (PMI8998 only)
+	Value type: <phandle>
+	Definition: BOB regulator parent supply phandle
+
+===============================
+Second Level Nodes - Regulators
+===============================
+
+- qcom,regulator-initial-microvolt
+	Usage:      optional; VRM regulators only
+	Value type: <u32>
+	Definition: Specifies the initial voltage in microvolts to request for a
+		    VRM regulator.
+
+- regulator-initial-mode
+	Usage:      optional; VRM regulators only
+	Value type: <u32>
+	Definition: Specifies the initial mode to request for a VRM regulator.
+		    Supported values are RPMH_REGULATOR_MODE_* which are defined
+		    in [1] (i.e. 0 to 3).  This property may be specified even
+		    if the regulator-allow-set-load property is not specified.
+
+- qcom,allowed-drms-modes
+	Usage:      required if regulator-allow-set-load is specified;
+		    VRM regulators only
+	Value type: <prop-encoded-array>
+	Definition: A list of integers specifying the PMIC regulator modes which
+		    can be configured at runtime based upon consumer load needs.
+		    Supported values are RPMH_REGULATOR_MODE_* which are defined
+		    in [1] (i.e. 0 to 3).
+
+- qcom,drms-mode-max-microamps
+	Usage:      required if regulator-allow-set-load is specified;
+		    VRM regulators only
+	Value type: <prop-encoded-array>
+	Definition: A list of integers specifying the maximum allowed load
+		    current in microamps for each of the modes listed in
+		    qcom,allowed-drms-modes (matched 1-to-1 in order).  Elements
+		    must be specified in order from lowest to highest value.
+
+- qcom,headroom-microvolt
+	Usage:      optional; VRM regulators only
+	Value type: <u32>
+	Definition: Specifies the headroom voltage in microvolts to request for
+		    a VRM regulator.  RPMh hardware automatically ensures that
+		    the parent of this regulator outputs a voltage high enough
+		    to satisfy the requested headroom.  Supported values are
+		    0 to 511000.
+
+- qcom,always-wait-for-ack
+	Usage:      optional
+	Value type: <empty>
+	Definition: Boolean flag which indicates that the application processor
+		    must wait for an ACK or a NACK from RPMh for every request
+		    sent for this regulator including those which are for a
+		    strictly lower power state.
+
+Other properties defined in Documentation/devicetree/bindings/regulator.txt
+may also be used.
+
+[1] include/dt-bindings/regulator/qcom,rpmh-regulator.h
+
+========
+Examples
+========
+
+#include <dt-bindings/regulator/qcom,rpmh-regulator.h>
+
+&apps_rsc {
+	pm8998-rpmh-regulators {
+		compatible = "qcom,pm8998-rpmh-regulators";
+		qcom,pmic-id = "a";
+
+		vdd-l7-l12-l14-l15-supply = <&pm8998_s5>;
+
+		smps2 {
+			regulator-min-microvolt = <1100000>;
+			regulator-max-microvolt = <1100000>;
+			qcom,regulator-initial-microvolt = <1100000>;
+		};
+
+		pm8998_s5: smps5 {
+			regulator-min-microvolt = <1904000>;
+			regulator-max-microvolt = <2040000>;
+			qcom,regulator-initial-microvolt = <1904000>;
+		};
+
+		ldo7 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+			qcom,regulator-initial-microvolt = <1800000>;
+			qcom,headroom-microvolt = <56000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_LPM>;
+			regulator-allowed-modes =
+				<RPMH_REGULATOR_MODE_LPM
+				 RPMH_REGULATOR_MODE_HPM>;
+			regulator-allow-set-load;
+			qcom,allowed-drms-modes =
+				<RPMH_REGULATOR_MODE_LPM
+				 RPMH_REGULATOR_MODE_HPM>;
+			qcom,drms-mode-max-microamps = <10000 1000000>;
+		};
+
+		lvs1 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+		};
+	};
+
+	pmi8998-rpmh-regulators {
+		compatible = "qcom,pmi8998-rpmh-regulators";
+		qcom,pmic-id = "b";
+
+		bob {
+			regulator-min-microvolt = <3312000>;
+			regulator-max-microvolt = <3600000>;
+			qcom,regulator-initial-microvolt = <3312000>;
+			regulator-allowed-modes =
+				<RPMH_REGULATOR_MODE_AUTO
+				 RPMH_REGULATOR_MODE_HPM>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_AUTO>;
+		};
+	};
+};
diff --git a/include/dt-bindings/regulator/qcom,rpmh-regulator.h b/include/dt-bindings/regulator/qcom,rpmh-regulator.h
new file mode 100644
index 0000000..4378c4b
--- /dev/null
+++ b/include/dt-bindings/regulator/qcom,rpmh-regulator.h
@@ -0,0 +1,36 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (c) 2018, The Linux Foundation. All rights reserved. */
+
+#ifndef __QCOM_RPMH_REGULATOR_H
+#define __QCOM_RPMH_REGULATOR_H
+
+/*
+ * These mode constants may be used for regulator-initial-mode and
+ * qcom,allowed-drms-modes properties of an RPMh regulator device tree node.
+ * Each type of regulator supports a subset of the possible modes.
+ *
+ * %RPMH_REGULATOR_MODE_RET:	Retention mode in which only an extremely small
+ *				load current is allowed.  This mode is supported
+ *				by LDO and SMPS type regulators.
+ * %RPMH_REGULATOR_MODE_LPM:	Low power mode in which a small load current is
+ *				allowed.  This mode corresponds to PFM for SMPS
+ *				and BOB type regulators.  This mode is supported
+ *				by LDO, HFSMPS, BOB, and PMIC4 FTSMPS type
+ *				regulators.
+ * %RPMH_REGULATOR_MODE_AUTO:	Auto mode in which the regulator hardware
+ *				automatically switches between LPM and HPM based
+ *				upon the real-time load current.  This mode is
+ *				supported by HFSMPS, BOB, and PMIC4 FTSMPS type
+ *				regulators.
+ * %RPMH_REGULATOR_MODE_HPM:	High power mode in which the full rated current
+ *				of the regulator is allowed.  This mode
+ *				corresponds to PWM for SMPS and BOB type
+ *				regulators.  This mode is supported by all types
+ *				of regulators.
+ */
+#define RPMH_REGULATOR_MODE_RET		0
+#define RPMH_REGULATOR_MODE_LPM		1
+#define RPMH_REGULATOR_MODE_AUTO	2
+#define RPMH_REGULATOR_MODE_HPM		3
+
+#endif
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox