* Re: [REGRESSION] "console: don't prefer first registered if DT specifies stdout-path" breaks console on video outputs of various ARM boards
From: Hans de Goede @ 2016-11-06 10:54 UTC (permalink / raw)
To: Paul Burton
Cc: Linus Torvalds, Andrew Morton, Rob Herring, Frank Rowand,
Thorsten Leemhuis, Greg Kroah-Hartman, Tejun Heo, devicetree,
linux-kernel
In-Reply-To: <1573891.u0IFOelkNI@np-p-burton>
Hi,
On 05-11-16 11:40, Paul Burton wrote:
> On Friday, 4 November 2016 14:22:17 GMT Hans de Goede wrote:
>> Hi,
>>
>> On 04-11-16 13:30, Paul Burton wrote:
>>> Hi Hans,
>>>
>>> On Friday, 4 November 2016 13:11:34 GMT Hans de Goede wrote:
>>>> Hi All,
>>>>
>>>> While booting 4.9-rc# for the first time on an Allwinner A33 tablet,
>>>> I noticed that after u-boot the LCD display stayed black. It turns out
>>>> that there was an issue which caused X to never get up, and all kernel
>>>> (and other startup) messages prior to that only went to ttyS0 which
>>>> consists of 2 tiny testpads on the PCB with this tablet.
>>>>
>>>> The same issue will also happen on any ARM boards which have a HDMI or
>>>> composite video output and which use a stdout-path pointing to their
>>>> serial console. I think this will e.g. also impact the Raspberry Pi,
>>>> I know for certain that this will impact the 99 different Allwinnner
>>>> boards currently supported by mainline u-boot + the mainline kernel.
>>>>
>>>> This is a behavior changes from previous kernels and I consider this
>>>> a regression. Thus I propose to revert the commit in question, for more
>>>> info here is a partial copy of the commit message of the proposed revert:
>>>>
>>>> The reverted commit changes existing behavior on which many ARM boards
>>>> rely. Many ARM small-board-computers, like e.g. the Raspberry Pi have
>>>> both a video output and a serial console. Depending on whether the user
>>>> is using the device as a more regular computer; or as a headless device
>>>> we need to have the console on either one or the other.
>>>>
>>>> Many users rely on the kernel behavior of the console being present on
>>>> both outputs, before the reverted commit the console setup with no
>>>> console= kernel arguments on an ARM board which sets stdout-path in dt
>>>> would look like this:
>>>>
>>>> [root@localhost ~]# cat /proc/consoles
>>>> ttyS0 -W- (EC p a) 4:64
>>>> tty0 -WU (E p ) 4:1
>>>>
>>>> Where as after the reverted commit, it looks like this:
>>>>
>>>> [root@localhost ~]# cat /proc/consoles
>>>> ttyS0 -W- (EC p a) 4:64
>>>>
>>>> This commit reverts commit 05fd007e4629 ("console: don't prefer first
>>>> registered if DT specifies stdout-path") restoring the original behavior.
>>>>
>>>> Regards,
>>>>
>>>> Hans
>>>
>>> Ugh... so the devices you're talking about rely upon set stdout-path in
>>> their device tree but effectively rely upon us ignoring it?
>>
>> No they rely on the kernel using stdout-path as an extra console while
>> keeping tty0 as console, not ignoring it. This how stdout-path has always
>> worked (at least as long as the Allwinner boards have used it, which has
>> been 2 - 3 years now).
>>
>> If you want only the console specified by stdout-path you can get this by
>> specifying it with console=... on the kernel cmdline.
>>
>>> If that's the case then I guess reverting is probably the best option, but
>>> it does restore us to a position where we honor stdout-path for earlycon
>>> & then essentially ignore it for the proper kernel console. That seems
>>> pretty bust to me...
>>
>> We do not ignore it, we use both the tty pointed to by stdout-path and tty0.
>>
>> Regards,
>>
>> Hans
>
> Hi Hans,
>
> Could you walk me though how you're getting that behaviour from the current
> code? I don't see how that would happen besides perhaps if drivers are probed
> in a fortunate order. Is that what you're relying upon?
I guess so, I never looked carefully at this, it has just always worked
until your patch.
> What I see in my systems, and what 05fd007e4629 ("console: don't prefer first
> registered if DT specifies stdout-path") addressed, is that if there are for
> example 2 UARTs uart0 & uart1 that are probed in that order and stdout-path
> indicates that we should use uart1 we wind up essentially ignoring it because
> the ordering of the relevant calls goes:
>
> - of_console_check() for uart0
> - add_preferred_console() for uart0
> - register_console() for uart0
> - of_console_check() for uart1
> - add_preferred_console() for uart1
> - register_console() for uart1
>
> Since of_check_console() doesn't get called for uart1 until after uart0 has
> been probed, we don't add an entry for it to the console_cmdline array until
> after register_console() has already decided to enable uart0 because
> preferred_console == -1.
>
> I'm not the only one seeing this oddity either, for example see the discussion
> on this patch:
>
> https://patchwork.kernel.org/patch/9263753/
>
> By simply reverting my patch you restore us to a position where so far as I
> can see we simply do not honor stdout-path for the real kernel console.
As said before, we do still honor it, but in your probe example we also get
a (second) serial console on uart0, where as you only want one on uart1.
So I see a few possible solutions here:
1) Do a new version of your patch which changes the "&& !of_specified_console"
check to "&& (newcon == tty0 || !of_specified_console)", then we would still
always register tty0 (as long as it gets registered first, just like now) and
we would not register uart0 in your above example, note the "newcon == tty0"
check in my example is pseudo-code. I would be happy to try out such a patch
2) Add a new dt property to enable the new behavior you seek
I'm myself tending towards 1 as a better solution: treat tty0 special, because
some existing setups rely on it always being registered as a console even if
stdout-path is specified and otherwise always honor stdout-path.
Regards,
Hans
^ permalink raw reply
* Re: [PATCH RFC] ARM: dts: add support for Turris Omnia
From: Andrew Lunn @ 2016-11-06 10:19 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Jason Cooper, Gregory Clement, Sebastian Hesselbarth,
Martin Strba??ka, Tomas Hlavacek, Rob Herring, Mark Rutland,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20161105220848.k6rrjmvvhdaeduma-jgopVnDzZD+b0XQX99//ntPVjbGH4+40kFgPdswSElo@public.gmane.org>
> Will do for v2. arch/arm/boot/dts/keystone-* needs fixing in this
> regard, too.
There is a whitelist of compatible strings which are ignored, for
backwards compatibility. See of_mdiobus_child_is_phy(). It would be
nice to fix keystone, but it is not required.
Andrew
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: Issue with dt_to_config with dts files that include headers from dt-bindings
From: Gaurav Minocha @ 2016-11-06 1:01 UTC (permalink / raw)
To: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CA+rpMbK=T+Z=-pML4RHLJpLziy-UuANpUcgKNbq06tMNrdAPKw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Sat, Nov 5, 2016 at 2:56 PM, Gaurav Minocha
<gaurav.minocha.os-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Nov 4, 2016 1:19 PM, "Peter Robinson" <pbrobinson-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>
>> Hi Gaurav, Rob and Frank,
>>
>> I've just discovered the dt_to_config script (awesome thanks!) but it
>> errors out when a dts(i) includes a header file from dt-bindings. EG
>>
>> ./scripts/dtc/dt_to_config arch/arm/boot/dts/am335x-boneblack.dts
>> In file included from arch/arm/boot/dts/am335x-boneblack.dts:10:0:
>> arch/arm/boot/dts/am33xx.dtsi:11:35: error: no include path in which
>> to search for dt-bindings/gpio/gpio.h
>> #include <dt-bindings/gpio/gpio.h>
>> ^
>> arch/arm/boot/dts/am33xx.dtsi:12:40: error: no include path in which
>> to search for dt-bindings/pinctrl/am33xx.h
>> #include <dt-bindings/pinctrl/am33xx.h>
>> ^
>> arch/arm/boot/dts/am335x-boneblack.dts:12:41: error: no include path
>> in which to search for dt-bindings/display/tda998x.h
>> #include <dt-bindings/display/tda998x.h>
>> ^
>> Error: arch/arm/boot/dts/am335x-bone-common.dtsi:33.23-24 syntax error
>> FATAL ERROR: Unable to parse input tree
>>
>
> In bash do, export ARCH=arm
>
> Works for me!
>
>>
>> Regards,
>> Peter
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH] usb: dwc2: add amcc,dwc-otg device tree definition
From: Christian Lamparter @ 2016-11-06 0:56 UTC (permalink / raw)
To: linux-usb-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: John Youn, Mark Rutland, Rob Herring, Greg Kroah-Hartman
This patch adds support for the "amcc,usb-otg" device
which is found in the PowerPC Canyonlands' dts.
The device definition was added by:
commit c89b3458d8cc ("powerpc/44x: Add USB DWC DTS entry to Canyonlands board")'.
AMCC produced a standalone driver that was sent to the
linuxppc-dev at the time. However, it was never integrated.
Signed-off-by: Christian Lamparter <chunkeey-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
For anyone interested: the driver was sent to the ML multiple times back
in 2012 [0], [1].
[0] <https://lists.ozlabs.org/pipermail/linuxppc-dev/2012-May/097847.html>
[1] <https://lists.ozlabs.org/pipermail/linuxppc-dev/2012-May/097938.html>
---
Documentation/devicetree/bindings/usb/dwc2.txt | 1 +
drivers/usb/dwc2/platform.c | 33 ++++++++++++++++++++++++++
2 files changed, 34 insertions(+)
diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
index 2c30a54..73c4dc5 100644
--- a/Documentation/devicetree/bindings/usb/dwc2.txt
+++ b/Documentation/devicetree/bindings/usb/dwc2.txt
@@ -12,6 +12,7 @@ Required properties:
- "lantiq,xrx200-usb": The DWC2 USB controller instance in Lantiq XRX SoCs;
- "amlogic,meson8b-usb": The DWC2 USB controller instance in Amlogic Meson8b SoCs;
- "amlogic,meson-gxbb-usb": The DWC2 USB controller instance in Amlogic S905 SoCs;
+ - "amcc,usb-otg": The DWC2 USB controller instance in AMCC Canyonlands 460EX SoCs;
- snps,dwc2: A generic DWC2 USB controller with default parameters.
- reg : Should contain 1 register range (address and length)
- interrupts : Should contain 1 interrupt
diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index dde08d5..8230037 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -214,6 +214,38 @@ static const struct dwc2_core_params params_amlogic = {
.hibernation = -1,
};
+static const struct dwc2_core_params params_amcc_dwc_otg = {
+ .otg_cap = DWC2_CAP_PARAM_HNP_SRP_CAPABLE,
+ .otg_ver = -1,
+ .dma_enable = -1,
+ .dma_desc_enable = -1,
+ .speed = -1,
+ .enable_dynamic_fifo = -1,
+ .en_multiple_tx_fifo = -1,
+ .host_rx_fifo_size = -1,
+ .host_nperio_tx_fifo_size = -1,
+ .host_perio_tx_fifo_size = -1,
+ .max_transfer_size = -1,
+ .max_packet_count = -1,
+ .host_channels = -1,
+ .phy_type = -1,
+ .phy_utmi_width = -1,
+ .phy_ulpi_ddr = -1,
+ .phy_ulpi_ext_vbus = -1,
+ .i2c_enable = -1,
+ .ulpi_fs_ls = -1,
+ .host_support_fs_ls_low_power = -1,
+ .host_ls_low_power_phy_clk = -1,
+ .ts_dline = -1,
+ .reload_ctl = -1,
+ /* Avoid system hang during concurrently using USB and SATA */
+ .ahbcfg = GAHBCFG_HBSTLEN_INCR16 <<
+ GAHBCFG_HBSTLEN_SHIFT,
+ .uframe_sched = -1,
+ .external_id_pin_ctl = -1,
+ .hibernation = -1,
+};
+
/*
* Check the dr_mode against the module configuration and hardware
* capabilities.
@@ -521,6 +553,7 @@ static const struct of_device_id dwc2_of_match_table[] = {
{ .compatible = "samsung,s3c6400-hsotg", .data = NULL},
{ .compatible = "amlogic,meson8b-usb", .data = ¶ms_amlogic },
{ .compatible = "amlogic,meson-gxbb-usb", .data = ¶ms_amlogic },
+ { .compatible = "amcc,dwc-otg", .data = ¶ms_amcc_dwc_otg },
{},
};
MODULE_DEVICE_TABLE(of, dwc2_of_match_table);
--
2.10.2
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v3 2/2] clk: renesas: cpg-mssr: add R8A7745 support
From: Sergei Shtylyov @ 2016-11-05 22:34 UTC (permalink / raw)
To: mturquette, linux-clk, sboyd, robh+dt, mark.rutland, devicetree
Cc: linux-renesas-soc, horms+renesas
In-Reply-To: <3539878.XFtRJDB9iq@wasted.cogentembedded.com>
Add RZ/G1E (R8A7745) Clock Pulse Generator / Module Standby and Software
Reset support, using the CPG/MSSR driver core and the common R-Car Gen2
(and RZ/G) code.
Based on the proof-of-concept R8A7791 CPG/MSSR patch by Geert Uytterhoeven
<geert+renesas@glider.be>.
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Changes in version 3:
- removed the FDP1-1 module clock;
- added Geert's tag.
Changes in version 2:
- changed the Z2 clock's divisor to 3.
Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt | 5
drivers/clk/renesas/Kconfig | 1
drivers/clk/renesas/Makefile | 1
drivers/clk/renesas/r8a7745-cpg-mssr.c | 260 +++++++++++
drivers/clk/renesas/renesas-cpg-mssr.c | 6
drivers/clk/renesas/renesas-cpg-mssr.h | 1
6 files changed, 272 insertions(+), 2 deletions(-)
Index: linux/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt
===================================================================
--- linux.orig/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt
+++ linux/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt
@@ -14,6 +14,7 @@ They provide the following functionaliti
Required Properties:
- compatible: Must be one of:
- "renesas,r8a7743-cpg-mssr" for the r8a7743 SoC (RZ/G1M)
+ - "renesas,r8a7745-cpg-mssr" for the r8a7745 SoC (RZ/G1E)
- "renesas,r8a7795-cpg-mssr" for the r8a7795 SoC (R-Car H3)
- "renesas,r8a7796-cpg-mssr" for the r8a7796 SoC (R-Car M3-W)
@@ -23,8 +24,8 @@ Required Properties:
- clocks: References to external parent clocks, one entry for each entry in
clock-names
- clock-names: List of external parent clock names. Valid names are:
- - "extal" (r8a7743, r8a7795, r8a7796)
- - "usb_extal" (r8a7743)
+ - "extal" (r8a7743, r8a7745, r8a7795, r8a7796)
+ - "usb_extal" (r8a7743, r8a7745)
- "extalr" (r8a7795, r8a7796)
- #clock-cells: Must be 2
Index: linux/drivers/clk/renesas/Kconfig
===================================================================
--- linux.orig/drivers/clk/renesas/Kconfig
+++ linux/drivers/clk/renesas/Kconfig
@@ -1,6 +1,7 @@
config CLK_RENESAS_CPG_MSSR
bool
default y if ARCH_R8A7743
+ default y if ARCH_R8A7745
default y if ARCH_R8A7795
default y if ARCH_R8A7796
Index: linux/drivers/clk/renesas/Makefile
===================================================================
--- linux.orig/drivers/clk/renesas/Makefile
+++ linux/drivers/clk/renesas/Makefile
@@ -3,6 +3,7 @@ obj-$(CONFIG_ARCH_R7S72100) += clk-rz.o
obj-$(CONFIG_ARCH_R8A73A4) += clk-r8a73a4.o clk-div6.o
obj-$(CONFIG_ARCH_R8A7740) += clk-r8a7740.o clk-div6.o
obj-$(CONFIG_ARCH_R8A7743) += r8a7743-cpg-mssr.o rcar-gen2-cpg.o
+obj-$(CONFIG_ARCH_R8A7745) += r8a7745-cpg-mssr.o rcar-gen2-cpg.o
obj-$(CONFIG_ARCH_R8A7778) += clk-r8a7778.o
obj-$(CONFIG_ARCH_R8A7779) += clk-r8a7779.o
obj-$(CONFIG_ARCH_R8A7790) += clk-rcar-gen2.o clk-div6.o
Index: linux/drivers/clk/renesas/r8a7745-cpg-mssr.c
===================================================================
--- /dev/null
+++ linux/drivers/clk/renesas/r8a7745-cpg-mssr.c
@@ -0,0 +1,260 @@
+/*
+ * r8a7745 Clock Pulse Generator / Module Standby and Software Reset
+ *
+ * Copyright (C) 2016 Cogent Embedded Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published
+ * by the Free Software Foundation; of the License.
+ */
+
+#include <linux/device.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/soc/renesas/rcar-rst.h>
+
+#include <dt-bindings/clock/r8a7745-cpg-mssr.h>
+
+#include "renesas-cpg-mssr.h"
+#include "rcar-gen2-cpg.h"
+
+enum clk_ids {
+ /* Core Clock Outputs exported to DT */
+ LAST_DT_CORE_CLK = R8A7745_CLK_OSC,
+
+ /* External Input Clocks */
+ CLK_EXTAL,
+ CLK_USB_EXTAL,
+
+ /* Internal Core Clocks */
+ CLK_MAIN,
+ CLK_PLL0,
+ CLK_PLL1,
+ CLK_PLL3,
+ CLK_PLL1_DIV2,
+
+ /* Module Clocks */
+ MOD_CLK_BASE
+};
+
+static const struct cpg_core_clk r8a7745_core_clks[] __initconst = {
+ /* External Clock Inputs */
+ DEF_INPUT("extal", CLK_EXTAL),
+ DEF_INPUT("usb_extal", CLK_USB_EXTAL),
+
+ /* Internal Core Clocks */
+ DEF_BASE(".main", CLK_MAIN, CLK_TYPE_GEN2_MAIN, CLK_EXTAL),
+ DEF_BASE(".pll0", CLK_PLL0, CLK_TYPE_GEN2_PLL0, CLK_MAIN),
+ DEF_BASE(".pll1", CLK_PLL1, CLK_TYPE_GEN2_PLL1, CLK_MAIN),
+ DEF_BASE(".pll3", CLK_PLL3, CLK_TYPE_GEN2_PLL3, CLK_MAIN),
+
+ DEF_FIXED(".pll1_div2", CLK_PLL1_DIV2, CLK_PLL1, 2, 1),
+
+ /* Core Clock Outputs */
+ DEF_BASE("lb", R8A7745_CLK_LB, CLK_TYPE_GEN2_LB, CLK_PLL1),
+ DEF_BASE("sdh", R8A7745_CLK_SDH, CLK_TYPE_GEN2_SDH, CLK_PLL1),
+ DEF_BASE("sd0", R8A7745_CLK_SD0, CLK_TYPE_GEN2_SD0, CLK_PLL1),
+ DEF_BASE("qspi", R8A7745_CLK_QSPI, CLK_TYPE_GEN2_QSPI, CLK_PLL1_DIV2),
+ DEF_BASE("rcan", R8A7745_CLK_RCAN, CLK_TYPE_GEN2_RCAN, CLK_USB_EXTAL),
+
+ DEF_FIXED("z2", R8A7745_CLK_Z2, CLK_PLL0, 3, 1),
+ DEF_FIXED("zg", R8A7745_CLK_ZG, CLK_PLL1, 6, 1),
+ DEF_FIXED("zx", R8A7745_CLK_ZX, CLK_PLL1, 3, 1),
+ DEF_FIXED("zs", R8A7745_CLK_ZS, CLK_PLL1, 6, 1),
+ DEF_FIXED("hp", R8A7745_CLK_HP, CLK_PLL1, 12, 1),
+ DEF_FIXED("b", R8A7745_CLK_B, CLK_PLL1, 12, 1),
+ DEF_FIXED("p", R8A7745_CLK_P, CLK_PLL1, 24, 1),
+ DEF_FIXED("cl", R8A7745_CLK_CL, CLK_PLL1, 48, 1),
+ DEF_FIXED("cp", R8A7745_CLK_CP, CLK_PLL1, 48, 1),
+ DEF_FIXED("m2", R8A7745_CLK_M2, CLK_PLL1, 8, 1),
+ DEF_FIXED("zb3", R8A7745_CLK_ZB3, CLK_PLL3, 4, 1),
+ DEF_FIXED("zb3d2", R8A7745_CLK_ZB3D2, CLK_PLL3, 8, 1),
+ DEF_FIXED("ddr", R8A7745_CLK_DDR, CLK_PLL3, 8, 1),
+ DEF_FIXED("mp", R8A7745_CLK_MP, CLK_PLL1_DIV2, 15, 1),
+ DEF_FIXED("acp", R8A7745_CLK_ACP, CLK_EXTAL, 2, 1),
+ DEF_FIXED("r", R8A7745_CLK_R, CLK_PLL1, 49152, 1),
+ DEF_FIXED("osc", R8A7745_CLK_OSC, CLK_PLL1, 12288, 1),
+
+ DEF_DIV6P1("sd2", R8A7745_CLK_SD2, CLK_PLL1_DIV2, 0x078),
+ DEF_DIV6P1("sd3", R8A7745_CLK_SD3, CLK_PLL1_DIV2, 0x26c),
+ DEF_DIV6P1("mmc0", R8A7745_CLK_MMC0, CLK_PLL1_DIV2, 0x240),
+};
+
+static const struct mssr_mod_clk r8a7745_mod_clks[] __initconst = {
+ DEF_MOD("msiof0", 0, R8A7745_CLK_MP),
+ DEF_MOD("vcp0", 101, R8A7745_CLK_ZS),
+ DEF_MOD("vpc0", 103, R8A7745_CLK_ZS),
+ DEF_MOD("tmu1", 111, R8A7745_CLK_P),
+ DEF_MOD("3dg", 112, R8A7745_CLK_ZG),
+ DEF_MOD("2d-dmac", 115, R8A7745_CLK_ZS),
+ DEF_MOD("fdp1-0", 119, R8A7745_CLK_ZS),
+ DEF_MOD("tmu3", 121, R8A7745_CLK_P),
+ DEF_MOD("tmu2", 122, R8A7745_CLK_P),
+ DEF_MOD("cmt0", 124, R8A7745_CLK_R),
+ DEF_MOD("tmu0", 125, R8A7745_CLK_CP),
+ DEF_MOD("vsp1du0", 128, R8A7745_CLK_ZS),
+ DEF_MOD("vsp1-sy", 131, R8A7745_CLK_ZS),
+ DEF_MOD("scifa2", 202, R8A7745_CLK_MP),
+ DEF_MOD("scifa1", 203, R8A7745_CLK_MP),
+ DEF_MOD("scifa0", 204, R8A7745_CLK_MP),
+ DEF_MOD("msiof2", 205, R8A7745_CLK_MP),
+ DEF_MOD("scifb0", 206, R8A7745_CLK_MP),
+ DEF_MOD("scifb1", 207, R8A7745_CLK_MP),
+ DEF_MOD("msiof1", 208, R8A7745_CLK_MP),
+ DEF_MOD("scifb2", 216, R8A7745_CLK_MP),
+ DEF_MOD("sys-dmac1", 218, R8A7745_CLK_ZS),
+ DEF_MOD("sys-dmac0", 219, R8A7745_CLK_ZS),
+ DEF_MOD("tpu0", 304, R8A7745_CLK_CP),
+ DEF_MOD("sdhi3", 311, R8A7745_CLK_SD3),
+ DEF_MOD("sdhi2", 312, R8A7745_CLK_SD2),
+ DEF_MOD("sdhi0", 314, R8A7745_CLK_SD0),
+ DEF_MOD("mmcif0", 315, R8A7745_CLK_MMC0),
+ DEF_MOD("iic0", 318, R8A7745_CLK_HP),
+ DEF_MOD("iic1", 323, R8A7745_CLK_HP),
+ DEF_MOD("cmt1", 329, R8A7745_CLK_R),
+ DEF_MOD("usbhs-dmac0", 330, R8A7745_CLK_HP),
+ DEF_MOD("usbhs-dmac1", 331, R8A7745_CLK_HP),
+ DEF_MOD("irqc", 407, R8A7745_CLK_CP),
+ DEF_MOD("intc-sys", 408, R8A7745_CLK_ZS),
+ DEF_MOD("audio-dmac0", 502, R8A7745_CLK_HP),
+ DEF_MOD("thermal", 522, CLK_EXTAL),
+ DEF_MOD("pwm", 523, R8A7745_CLK_P),
+ DEF_MOD("usb-ehci", 703, R8A7745_CLK_MP),
+ DEF_MOD("usbhs", 704, R8A7745_CLK_HP),
+ DEF_MOD("hscif2", 713, R8A7745_CLK_ZS),
+ DEF_MOD("scif5", 714, R8A7745_CLK_P),
+ DEF_MOD("scif4", 715, R8A7745_CLK_P),
+ DEF_MOD("hscif1", 716, R8A7745_CLK_ZS),
+ DEF_MOD("hscif0", 717, R8A7745_CLK_ZS),
+ DEF_MOD("scif3", 718, R8A7745_CLK_P),
+ DEF_MOD("scif2", 719, R8A7745_CLK_P),
+ DEF_MOD("scif1", 720, R8A7745_CLK_P),
+ DEF_MOD("scif0", 721, R8A7745_CLK_P),
+ DEF_MOD("du0", 724, R8A7745_CLK_ZX),
+ DEF_MOD("ipmmu-sgx", 800, R8A7745_CLK_ZX),
+ DEF_MOD("vin1", 810, R8A7745_CLK_ZG),
+ DEF_MOD("vin0", 811, R8A7745_CLK_ZG),
+ DEF_MOD("etheravb", 812, R8A7745_CLK_HP),
+ DEF_MOD("ether", 813, R8A7745_CLK_P),
+ DEF_MOD("gpio6", 905, R8A7745_CLK_CP),
+ DEF_MOD("gpio5", 907, R8A7745_CLK_CP),
+ DEF_MOD("gpio4", 908, R8A7745_CLK_CP),
+ DEF_MOD("gpio3", 909, R8A7745_CLK_CP),
+ DEF_MOD("gpio2", 910, R8A7745_CLK_CP),
+ DEF_MOD("gpio1", 911, R8A7745_CLK_CP),
+ DEF_MOD("gpio0", 912, R8A7745_CLK_CP),
+ DEF_MOD("can1", 915, R8A7745_CLK_P),
+ DEF_MOD("can0", 916, R8A7745_CLK_P),
+ DEF_MOD("qspi_mod", 917, R8A7745_CLK_QSPI),
+ DEF_MOD("i2c5", 925, R8A7745_CLK_HP),
+ DEF_MOD("i2c4", 927, R8A7745_CLK_HP),
+ DEF_MOD("i2c3", 928, R8A7745_CLK_HP),
+ DEF_MOD("i2c2", 929, R8A7745_CLK_HP),
+ DEF_MOD("i2c1", 930, R8A7745_CLK_HP),
+ DEF_MOD("i2c0", 931, R8A7745_CLK_HP),
+ DEF_MOD("ssi-all", 1005, R8A7745_CLK_P),
+ DEF_MOD("ssi9", 1006, MOD_CLK_ID(1005)),
+ DEF_MOD("ssi8", 1007, MOD_CLK_ID(1005)),
+ DEF_MOD("ssi7", 1008, MOD_CLK_ID(1005)),
+ DEF_MOD("ssi6", 1009, MOD_CLK_ID(1005)),
+ DEF_MOD("ssi5", 1010, MOD_CLK_ID(1005)),
+ DEF_MOD("ssi4", 1011, MOD_CLK_ID(1005)),
+ DEF_MOD("ssi3", 1012, MOD_CLK_ID(1005)),
+ DEF_MOD("ssi2", 1013, MOD_CLK_ID(1005)),
+ DEF_MOD("ssi1", 1014, MOD_CLK_ID(1005)),
+ DEF_MOD("ssi0", 1015, MOD_CLK_ID(1005)),
+ DEF_MOD("scu-all", 1017, R8A7745_CLK_P),
+ DEF_MOD("scu-dvc1", 1018, MOD_CLK_ID(1017)),
+ DEF_MOD("scu-dvc0", 1019, MOD_CLK_ID(1017)),
+ DEF_MOD("scu-ctu1-mix1", 1020, MOD_CLK_ID(1017)),
+ DEF_MOD("scu-ctu0-mix0", 1021, MOD_CLK_ID(1017)),
+ DEF_MOD("scu-src9", 1022, MOD_CLK_ID(1017)),
+ DEF_MOD("scu-src8", 1023, MOD_CLK_ID(1017)),
+ DEF_MOD("scu-src7", 1024, MOD_CLK_ID(1017)),
+ DEF_MOD("scu-src6", 1025, MOD_CLK_ID(1017)),
+ DEF_MOD("scu-src5", 1026, MOD_CLK_ID(1017)),
+ DEF_MOD("scu-src4", 1027, MOD_CLK_ID(1017)),
+ DEF_MOD("scu-src3", 1028, MOD_CLK_ID(1017)),
+ DEF_MOD("scu-src2", 1029, MOD_CLK_ID(1017)),
+ DEF_MOD("scu-src1", 1030, MOD_CLK_ID(1017)),
+ DEF_MOD("scu-src0", 1031, MOD_CLK_ID(1017)),
+ DEF_MOD("scifa3", 1106, R8A7745_CLK_MP),
+ DEF_MOD("scifa4", 1107, R8A7745_CLK_MP),
+ DEF_MOD("scifa5", 1108, R8A7745_CLK_MP),
+};
+
+static const unsigned int r8a7745_crit_mod_clks[] __initconst = {
+ MOD_CLK_ID(408), /* INTC-SYS (GIC) */
+};
+
+/*
+ * CPG Clock Data
+ */
+
+/*
+ * MD EXTAL PLL0 PLL1 PLL3
+ * 14 13 19 (MHz) *1 *2
+ *---------------------------------------------------
+ * 0 0 0 15 x200/3 x208/2 x106
+ * 0 0 1 15 x200/3 x208/2 x88
+ * 0 1 0 20 x150/3 x156/2 x80
+ * 0 1 1 20 x150/3 x156/2 x66
+ * 1 0 0 26 / 2 x230/3 x240/2 x122
+ * 1 0 1 26 / 2 x230/3 x240/2 x102
+ * 1 1 0 30 / 2 x200/3 x208/2 x106
+ * 1 1 1 30 / 2 x200/3 x208/2 x88
+ *
+ * *1 : Table 7.5b indicates VCO output (PLL0 = VCO/3)
+ * *2 : Table 7.5b indicates VCO output (PLL1 = VCO/2)
+ */
+#define CPG_PLL_CONFIG_INDEX(md) ((((md) & BIT(14)) >> 12) | \
+ (((md) & BIT(13)) >> 12) | \
+ (((md) & BIT(19)) >> 19))
+
+static const struct rcar_gen2_cpg_pll_config cpg_pll_configs[8] __initconst = {
+ /* EXTAL div PLL1 mult PLL3 mult PLL0 mult */
+ { 1, 208, 106, 200 },
+ { 1, 208, 88, 200 },
+ { 1, 156, 80, 150 },
+ { 1, 156, 66, 150 },
+ { 2, 240, 122, 230 },
+ { 2, 240, 102, 230 },
+ { 2, 208, 106, 200 },
+ { 2, 208, 88, 200 },
+};
+
+static int __init r8a7745_cpg_mssr_init(struct device *dev)
+{
+ const struct rcar_gen2_cpg_pll_config *cpg_pll_config;
+ u32 cpg_mode;
+ int error;
+
+ error = rcar_rst_read_mode_pins(&cpg_mode);
+ if (error)
+ return error;
+
+ cpg_pll_config = &cpg_pll_configs[CPG_PLL_CONFIG_INDEX(cpg_mode)];
+
+ return rcar_gen2_cpg_init(cpg_pll_config);
+}
+
+const struct cpg_mssr_info r8a7745_cpg_mssr_info __initconst = {
+ /* Core Clocks */
+ .core_clks = r8a7745_core_clks,
+ .num_core_clks = ARRAY_SIZE(r8a7745_core_clks),
+ .last_dt_core_clk = LAST_DT_CORE_CLK,
+ .num_total_core_clks = MOD_CLK_BASE,
+
+ /* Module Clocks */
+ .mod_clks = r8a7745_mod_clks,
+ .num_mod_clks = ARRAY_SIZE(r8a7745_mod_clks),
+ .num_hw_mod_clks = 12 * 32,
+
+ /* Critical Module Clocks */
+ .crit_mod_clks = r8a7745_crit_mod_clks,
+ .num_crit_mod_clks = ARRAY_SIZE(r8a7745_crit_mod_clks),
+
+ /* Callbacks */
+ .init = r8a7745_cpg_mssr_init,
+ .cpg_clk_register = rcar_gen2_cpg_clk_register,
+};
Index: linux/drivers/clk/renesas/renesas-cpg-mssr.c
===================================================================
--- linux.orig/drivers/clk/renesas/renesas-cpg-mssr.c
+++ linux/drivers/clk/renesas/renesas-cpg-mssr.c
@@ -509,6 +509,12 @@ static const struct of_device_id cpg_mss
.data = &r8a7743_cpg_mssr_info,
},
#endif
+#ifdef CONFIG_ARCH_R8A7745
+ {
+ .compatible = "renesas,r8a7745-cpg-mssr",
+ .data = &r8a7745_cpg_mssr_info,
+ },
+#endif
#ifdef CONFIG_ARCH_R8A7795
{
.compatible = "renesas,r8a7795-cpg-mssr",
Index: linux/drivers/clk/renesas/renesas-cpg-mssr.h
===================================================================
--- linux.orig/drivers/clk/renesas/renesas-cpg-mssr.h
+++ linux/drivers/clk/renesas/renesas-cpg-mssr.h
@@ -131,6 +131,7 @@ struct cpg_mssr_info {
};
extern const struct cpg_mssr_info r8a7743_cpg_mssr_info;
+extern const struct cpg_mssr_info r8a7745_cpg_mssr_info;
extern const struct cpg_mssr_info r8a7795_cpg_mssr_info;
extern const struct cpg_mssr_info r8a7796_cpg_mssr_info;
#endif
^ permalink raw reply
* [PATCH v3 1/2] ARM: shmobile: r8a7745: add CPG clock index macros
From: Sergei Shtylyov @ 2016-11-05 22:31 UTC (permalink / raw)
To: robh+dt, mark.rutland
Cc: mturquette, linux-clk, sboyd, devicetree, linux-renesas-soc,
horms+renesas
In-Reply-To: <3539878.XFtRJDB9iq@wasted.cogentembedded.com>
Add macros usable by the device tree sources to reference the R8A7745 CPG
clocks by index. The data comes from the table 7.2c in the revision 0.50
of the RZ/G Series User's Manual.
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Changes in version 2:
- added Geert's tag.
include/dt-bindings/clock/r8a7745-cpg-mssr.h | 44 +++++++++++++++++++++++++++
1 file changed, 44 insertions(+)
Index: linux/include/dt-bindings/clock/r8a7745-cpg-mssr.h
===================================================================
--- /dev/null
+++ linux/include/dt-bindings/clock/r8a7745-cpg-mssr.h
@@ -0,0 +1,44 @@
+/*
+ * Copyright (C) 2016 Cogent Embedded Inc.
+ *
+ * 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.
+ */
+#ifndef __DT_BINDINGS_CLOCK_R8A7745_CPG_MSSR_H__
+#define __DT_BINDINGS_CLOCK_R8A7745_CPG_MSSR_H__
+
+#include <dt-bindings/clock/renesas-cpg-mssr.h>
+
+/* r8a7745 CPG Core Clocks */
+#define R8A7745_CLK_Z2 0
+#define R8A7745_CLK_ZG 1
+#define R8A7745_CLK_ZTR 2
+#define R8A7745_CLK_ZTRD2 3
+#define R8A7745_CLK_ZT 4
+#define R8A7745_CLK_ZX 5
+#define R8A7745_CLK_ZS 6
+#define R8A7745_CLK_HP 7
+#define R8A7745_CLK_B 9
+#define R8A7745_CLK_LB 10
+#define R8A7745_CLK_P 11
+#define R8A7745_CLK_CL 12
+#define R8A7745_CLK_CP 13
+#define R8A7745_CLK_M2 14
+#define R8A7745_CLK_ZB3 16
+#define R8A7745_CLK_ZB3D2 17
+#define R8A7745_CLK_DDR 18
+#define R8A7745_CLK_SDH 19
+#define R8A7745_CLK_SD0 20
+#define R8A7745_CLK_SD2 21
+#define R8A7745_CLK_SD3 22
+#define R8A7745_CLK_MMC0 23
+#define R8A7745_CLK_MP 24
+#define R8A7745_CLK_QSPI 25
+#define R8A7745_CLK_ACP 26
+#define R8A7745_CLK_RCAN 27
+#define R8A7745_CLK_R 28
+#define R8A7745_CLK_OSC 29
+
+#endif /* __DT_BINDINGS_CLOCK_R8A7745_CPG_MSSR_H__ */
^ permalink raw reply
* [PATCH v3 0/2] Renesas R8A7745 CPG/MSSR clock support
From: Sergei Shtylyov @ 2016-11-05 22:28 UTC (permalink / raw)
To: mturquette, linux-clk, sboyd, robh+dt, mark.rutland, devicetree
Cc: linux-renesas-soc, horms+renesas
Hello.
Here's the set of 2 patches against the 'clk-next' branch of CLK group's
'linux.git' repo plus the R8A7743 CPG/MSSR patches just re-posted. They also
depend on the common R-Car Gen2 support patch. As the DT patches in the
R8A7745/SK-RZG1E board support series depend on the patch #1 of this series,
it might make sense to merge this patch to the 'renesas.git' repo as well...
[1/2] ARM: shmobile: r8a7745: add CPG clock index macros
[2/2] clk: renesas: cpg-mssr: add R8A7745 support
MBR, Sergei
^ permalink raw reply
* Re: [PATCH 7/8] ARM64: dts: meson-gxl-p23x: Add SD/SDIO/MMC and PWM nodes
From: Kevin Hilman @ 2016-11-05 22:10 UTC (permalink / raw)
To: Neil Armstrong
Cc: carlo-KA+7E9HrN00dnm+yROfE0A,
linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1477932286-27482-8-git-send-email-narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
Neil Armstrong <narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> writes:
> Add SD/SDIO/MMC nodes and PWM 32768Hz clock configuration to provide
> storage and WiFi functionality on the p23x boards.
Just curious... what storage functionality are you referring to here?
Kevin
> Signed-off-by: Neil Armstrong <narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> ---
> .../boot/dts/amlogic/meson-gxl-s905d-p23x.dtsi | 112 +++++++++++++++++++++
> 1 file changed, 112 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p23x.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p23x.dtsi
> index 666fe2b..7830809 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p23x.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p23x.dtsi
> @@ -56,6 +56,46 @@
> device_type = "memory";
> reg = <0x0 0x0 0x0 0x80000000>;
> };
> +
> + vddio_boot: regulator-vddio_boot {
> + compatible = "regulator-fixed";
> + regulator-name = "VDDIO_BOOT";
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + };
> +
> + vddao_3v3: regulator-vddao_3v3 {
> + compatible = "regulator-fixed";
> + regulator-name = "VDDAO_3V3";
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + };
> +
> + vcc_3v3: regulator-vcc_3v3 {
> + compatible = "regulator-fixed";
> + regulator-name = "VCC_3V3";
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + };
> +
> + emmc_pwrseq: emmc-pwrseq {
> + compatible = "mmc-pwrseq-emmc";
> + reset-gpios = <&gpio BOOT_9 GPIO_ACTIVE_LOW>;
> + };
> +
> + wifi32k: wifi32k {
> + compatible = "pwm-clock";
> + #clock-cells = <0>;
> + clock-frequency = <32768>;
> + pwms = <&pwm_ef 0 30518 0>; /* PWM_E at 32.768KHz */
> + };
> +
> + sdio_pwrseq: sdio-pwrseq {
> + compatible = "mmc-pwrseq-simple";
> + reset-gpios = <&gpio GPIOX_6 GPIO_ACTIVE_LOW>;
> + clocks = <&wifi32k>;
> + clock-names = "ext_clock";
> + };
> };
>
> /* This UART is brought out to the DB9 connector */
> @@ -64,3 +104,75 @@
> pinctrl-0 = <&uart_ao_a_pins>;
> pinctrl-names = "default";
> };
> +
> +/* Wireless SDIO Module */
> +&sd_emmc_a {
> + status = "okay";
> + pinctrl-0 = <&sdio_pins>;
> + pinctrl-names = "default";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + bus-width = <4>;
> + cap-sd-highspeed;
> + max-frequency = <100000000>;
> +
> + non-removable;
> + disable-wp;
> +
> + mmc-pwrseq = <&sdio_pwrseq>;
> +
> + vmmc-supply = <&vddao_3v3>;
> + vqmmc-supply = <&vddio_boot>;
> +
> + brcmf: bcrmf@1 {
> + reg = <1>;
> + compatible = "brcm,bcm4329-fmac";
> + };
> +};
> +
> +/* SD card */
> +&sd_emmc_b {
> + status = "okay";
> + pinctrl-0 = <&sdcard_pins>;
> + pinctrl-names = "default";
> +
> + bus-width = <4>;
> + cap-sd-highspeed;
> + max-frequency = <100000000>;
> + disable-wp;
> +
> + cd-gpios = <&gpio CARD_6 GPIO_ACTIVE_HIGH>;
> + cd-inverted;
> +
> + vmmc-supply = <&vddao_3v3>;
> + vqmmc-supply = <&vddio_boot>;
> +};
> +
> +/* eMMC */
> +&sd_emmc_c {
> + status = "okay";
> + pinctrl-0 = <&emmc_pins>;
> + pinctrl-names = "default";
> +
> + bus-width = <8>;
> + cap-sd-highspeed;
> + cap-mmc-highspeed;
> + max-frequency = <200000000>;
> + non-removable;
> + disable-wp;
> + mmc-ddr-1_8v;
> + mmc-hs200-1_8v;
> +
> + mmc-pwrseq = <&emmc_pwrseq>;
> + vmmc-supply = <&vcc_3v3>;
> + vqmmc-supply = <&vddio_boot>;
> +};
> +
> +&pwm_ef {
> + status = "okay";
> + pinctrl-0 = <&pwm_e_pins>;
> + pinctrl-names = "default";
> + clocks = <&clkc CLKID_FCLK_DIV4>;
> + clock-names = "clkin0";
> +};
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH RFC] ARM: dts: add support for Turris Omnia
From: Uwe Kleine-König @ 2016-11-05 22:08 UTC (permalink / raw)
To: Andrew Lunn
Cc: Jason Cooper, Gregory Clement, Sebastian Hesselbarth,
Martin Strba??ka, Tomas Hlavacek, Rob Herring, Mark Rutland,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20161105210441.GB1216-g2DYL2Zd6BY@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1346 bytes --]
On Sat, Nov 05, 2016 at 10:04:41PM +0100, Andrew Lunn wrote:
> > +&mdio {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&mdio_pins>;
> > + status = "okay";
> > +
> > + phy1: phy@1 {
> > + status = "okay";
> > + compatible = "marvell,88E1514", "marvell,88E1510", "ethernet-phy-ieee802.3-c22";
> > + reg = <1>;
> > + };
>
> phy.txt says:
>
> - compatible: Compatible list, may contain
> "ethernet-phy-ieee802.3-c22" or "ethernet-phy-ieee802.3-c45" for
> PHYs that implement IEEE802.3 clause 22 or IEEE802.3 clause 45
> specifications. If neither of these are specified, the default is to
> assume clause 22.
>
> If the phy's identifier is known then the list may contain an entry
> of the form: "ethernet-phy-idAAAA.BBBB" where
> AAAA - The value of the 16 bit Phy Identifier 1 register as
> 4 hex digits. This is the chip vendor OUI bits 3:18
> BBBB - The value of the 16 bit Phy Identifier 2 register as
> 4 hex digits. This is the chip vendor OUI bits 19:24,
> followed by 10 bits of a vendor specific ID.
>
> The compatible list should not contain other values than those
> listed here.
>
> Please don't list the "marvell,*" names.
Will do for v2. arch/arm/boot/dts/keystone-* needs fixing in this
regard, too.
Best regards
Uwe
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply
* Re: [PATCH RFC] ARM: dts: add support for Turris Omnia
From: Andrew Lunn @ 2016-11-05 21:37 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Mark Rutland, Martin Strba??ka, Jason Cooper,
devicetree-u79uwXL29TY76Z2rM5mHXA, Tomas Hlavacek, Rob Herring,
Gregory Clement,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Sebastian Hesselbarth
In-Reply-To: <20161105212748.vtdprlxxismy5xmk-jgopVnDzZD+b0XQX99//ntPVjbGH4+40kFgPdswSElo@public.gmane.org>
On Sat, Nov 05, 2016 at 10:27:49PM +0100, Uwe Kleine-König wrote:
> Hello Andrew,
>
> On Sat, Nov 05, 2016 at 10:23:26PM +0100, Andrew Lunn wrote:
> > > Still missing is support for the switch.
> >
> > Is it a Marvell Switch? armada-370-rd.dts would be a good start for
> > the old binding? vf610-zii-dev-rev-b.dts uses the new binding.
>
> Yeah, a 88E6176. I already try to understand vf610-zii-dev-rev-b.dts. Do
> you know if this driver works for the 88E6176?
Yes it does.
The vf610-zii-dev-rev-b is a bit complex because it has three
switches, and an mdio mux. You should be able to transplant the
switch0: switch0@0 part into the mdio node.
Andrew
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH RFC] ARM: dts: add support for Turris Omnia
From: Uwe Kleine-König @ 2016-11-05 21:27 UTC (permalink / raw)
To: Andrew Lunn
Cc: Jason Cooper, Gregory Clement, Sebastian Hesselbarth,
Martin Strba??ka, Tomas Hlavacek, Rob Herring, Mark Rutland,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20161105212326.GC1216-g2DYL2Zd6BY@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 756 bytes --]
Hello Andrew,
On Sat, Nov 05, 2016 at 10:23:26PM +0100, Andrew Lunn wrote:
> > Still missing is support for the switch.
>
> Is it a Marvell Switch? armada-370-rd.dts would be a good start for
> the old binding? vf610-zii-dev-rev-b.dts uses the new binding.
Yeah, a 88E6176. I already try to understand vf610-zii-dev-rev-b.dts. Do
you know if this driver works for the 88E6176?
> > SFP is untested as is UART1.
>
> UART would be unusual. They are normally i2c.
I wanted to say: SFP is untested, and UART1 is untested too. Yes, SFP is
connected via i2c.
> > Do I need to "register" turris in vendor-prefixes.txt for that?
>
> Yes please.
OK, will wait for Martin to comment what we want there. cznic or turris.
Thanks
Uwe
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply
* Re: [PATCH 2/2] clk: renesas: cpg-mssr: add R8A7745 support
From: Sergei Shtylyov @ 2016-11-05 21:25 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Michael Turquette, linux-clk, Stephen Boyd, Rob Herring,
Mark Rutland, devicetree@vger.kernel.org, Linux-Renesas,
Simon Horman
In-Reply-To: <CAMuHMdUD_U5YqOnWv3Oo8P_Oxbh=qMP9wAtJmEgt-2eW4y3XqQ@mail.gmail.com>
On 11/02/2016 06:47 PM, Geert Uytterhoeven wrote:
>> Add RZ/G1E (R8A7745) Clock Pulse Generator / Module Standby and Software
>> Reset support, using the CPG/MSSR driver core and the common R-Car Gen2
>> (and RZ/G) code.
>>
>> Based on the proof-of-concept R8A7791 CPG/MSSR patch by Geert Uytterhoeven
>> <geert+renesas@glider.be>.
>>
>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>
> Thanks a lot for your patch!
>
>> --- /dev/null
>> +++ linux/drivers/clk/renesas/r8a7745-cpg-mssr.c
>> @@ -0,0 +1,261 @@
[...]
>> +static const struct mssr_mod_clk r8a7745_mod_clks[] __initconst = {
>
>> + DEF_MOD("fdp1-1", 118, R8A7745_CLK_ZS),
>
> FDP1-1 does not exist on RZ/G1E.
Sorry, I somehow forgot about this one... will respin!
MBR, Sergei
^ permalink raw reply
* Re: [PATCH RFC] ARM: dts: add support for Turris Omnia
From: Andrew Lunn @ 2016-11-05 21:23 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Jason Cooper, Gregory Clement, Sebastian Hesselbarth,
Martin Strba??ka, Tomas Hlavacek, Rob Herring, Mark Rutland,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20161105203841.9661-1-uwe-rXY34ruvC2xidJT2blvkqNi2O/JbrIOy@public.gmane.org>
> Still missing is support for the switch.
Is it a Marvell Switch? armada-370-rd.dts would be a good start for
the old binding? vf610-zii-dev-rev-b.dts uses the new binding.
> SFP is untested as is UART1.
UART would be unusual. They are normally i2c.
> Do I need to "register" turris in vendor-prefixes.txt for that?
Yes please.
Andrew
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH RFC] ARM: dts: add support for Turris Omnia
From: Andrew Lunn @ 2016-11-05 21:04 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Jason Cooper, Gregory Clement, Sebastian Hesselbarth,
Martin Strba??ka, Tomas Hlavacek, Rob Herring, Mark Rutland,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20161105203841.9661-1-uwe-rXY34ruvC2xidJT2blvkqNi2O/JbrIOy@public.gmane.org>
> +&mdio {
> + pinctrl-names = "default";
> + pinctrl-0 = <&mdio_pins>;
> + status = "okay";
> +
> + phy1: phy@1 {
> + status = "okay";
> + compatible = "marvell,88E1514", "marvell,88E1510", "ethernet-phy-ieee802.3-c22";
> + reg = <1>;
> + };
phy.txt says:
- compatible: Compatible list, may contain
"ethernet-phy-ieee802.3-c22" or "ethernet-phy-ieee802.3-c45" for
PHYs that implement IEEE802.3 clause 22 or IEEE802.3 clause 45
specifications. If neither of these are specified, the default is to
assume clause 22.
If the phy's identifier is known then the list may contain an entry
of the form: "ethernet-phy-idAAAA.BBBB" where
AAAA - The value of the 16 bit Phy Identifier 1 register as
4 hex digits. This is the chip vendor OUI bits 3:18
BBBB - The value of the 16 bit Phy Identifier 2 register as
4 hex digits. This is the chip vendor OUI bits 19:24,
followed by 10 bits of a vendor specific ID.
The compatible list should not contain other values than those
listed here.
Please don't list the "marvell,*" names.
Andrew
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 0/2] Add TI SCI Reset Driver
From: santosh.shilimkar-QHcLZuEGTsvQT0dZR+AlfA @ 2016-11-05 21:01 UTC (permalink / raw)
To: Andrew F. Davis, Nishanth Menon, Tero Kristo, Santosh Shilimkar,
Philipp Zabel, Rob Herring, Mark Rutland, Suman Anna
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161104174240.9688-1-afd-l0cyMroinI0@public.gmane.org>
On 11/4/16 11:42 AM, Andrew F. Davis wrote:
> Hello all,
>
> This series adds a reset controller driver that uses the TI SCI
> protocol to manage resets.
>
> The TI SCI protocol is used to communicate with power management
> controllers used by some SoCs. These controllers manage the various
> power domains, clocks, and resets available on a SoC.
>
> This series is based on drivers for TI SCI and the first two controlled
> elements above, these series can be found here:
>
> TI-SCI: http://www.spinics.net/lists/arm-kernel/msg536851.html
> PM Domains: http://www.spinics.net/lists/devicetree/msg146621.html
> Clocks: https://www.spinics.net/lists/linux-clk/msg12785.html
>
> Thanks,
> Andrew
>
> Changes from v2:
> - Merged DT binding patch and reset header patch
> - Added locking for reset bit mask
>
> Changes from v1:
> - Revised dt binding
> - CC Linux ARM list
>
> Andrew F. Davis (2):
> Documentation: dt: reset: Add TI SCI reset binding
> reset: Add the TI SCI reset driver
>
Acked-by: Santosh Shilimkar <ssantosh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: Issue with dt_to_config with dts files that include headers from dt-bindings
From: Gaurav Minocha @ 2016-11-05 20:56 UTC (permalink / raw)
To: Peter Robinson; +Cc: Frank Rowand, Rob Herring, devicetree
In-Reply-To: <CALeDE9NVuztzMe8iO+OhOrwg3YxCr9VbEwh24-3MLK+5U82fYg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1174 bytes --]
On Nov 4, 2016 1:19 PM, "Peter Robinson" <pbrobinson@gmail.com> wrote:
>
> Hi Gaurav, Rob and Frank,
>
> I've just discovered the dt_to_config script (awesome thanks!) but it
> errors out when a dts(i) includes a header file from dt-bindings. EG
>
> ./scripts/dtc/dt_to_config arch/arm/boot/dts/am335x-boneblack.dts
> In file included from arch/arm/boot/dts/am335x-boneblack.dts:10:0:
> arch/arm/boot/dts/am33xx.dtsi:11:35: error: no include path in which
> to search for dt-bindings/gpio/gpio.h
> #include <dt-bindings/gpio/gpio.h>
> ^
> arch/arm/boot/dts/am33xx.dtsi:12:40: error: no include path in which
> to search for dt-bindings/pinctrl/am33xx.h
> #include <dt-bindings/pinctrl/am33xx.h>
> ^
> arch/arm/boot/dts/am335x-boneblack.dts:12:41: error: no include path
> in which to search for dt-bindings/display/tda998x.h
> #include <dt-bindings/display/tda998x.h>
> ^
> Error: arch/arm/boot/dts/am335x-bone-common.dtsi:33.23-24 syntax error
> FATAL ERROR: Unable to parse input tree
>
In bash do, export ARCH=arm
Works for me!
>
> Regards,
> Peter
[-- Attachment #2: Type: text/html, Size: 1611 bytes --]
^ permalink raw reply
* [PATCH RFC] ARM: dts: add support for Turris Omnia
From: Uwe Kleine-König @ 2016-11-05 20:38 UTC (permalink / raw)
To: Jason Cooper, Andrew Lunn, Gregory Clement, Sebastian Hesselbarth
Cc: Mark Rutland, Martin Strbačka, devicetree, Tomas Hlavacek,
Rob Herring, linux-arm-kernel
This machine is an open hardware router by cz.nic driven by a
Marvell Armada 385.
Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org>
---
Hello,
the following components are working:
- WAN port
- eMMC
- UART0
- USB
Still missing is support for the switch. Wireless fails to probe, didn't
debug this up to now. SFP is untested as is UART1.
The device tree on the device doesn't specify a board compatible, I added
"turris,omnia". Do I need to "register" turris in vendor-prefixes.txt for that?
@Tomas+Martin: Is this correct at all, or should I better reference cz.nic?
Best regards
Uwe
---
arch/arm/boot/dts/Makefile | 1 +
arch/arm/boot/dts/armada-385-turris-omnia.dts | 246 ++++++++++++++++++++++++++
2 files changed, 247 insertions(+)
create mode 100644 arch/arm/boot/dts/armada-385-turris-omnia.dts
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index befcd2619902..f1d3b9ff257e 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -920,6 +920,7 @@ dtb-$(CONFIG_MACH_ARMADA_38X) += \
armada-385-db-ap.dtb \
armada-385-linksys-caiman.dtb \
armada-385-linksys-cobra.dtb \
+ armada-385-turris-omnia.dtb \
armada-388-clearfog.dtb \
armada-388-db.dtb \
armada-388-gp.dtb \
diff --git a/arch/arm/boot/dts/armada-385-turris-omnia.dts b/arch/arm/boot/dts/armada-385-turris-omnia.dts
new file mode 100644
index 000000000000..d3cd8a4d713d
--- /dev/null
+++ b/arch/arm/boot/dts/armada-385-turris-omnia.dts
@@ -0,0 +1,246 @@
+/*
+ * Device Tree file for the Turris Omnia
+ * Schematic available at https://www.turris.cz/doc/_media/rtrom01-schema.pdf
+ *
+ * Copyright (C) 2016 Uwe Kleine-König <uwe@kleine-koenig.org>
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ * a) This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without
+ * any warranty of any kind, whether express or implied.
+ *
+ * Or, alternatively,
+ *
+ * b) Permission is hereby granted, free of charge, to any person
+ * obtaining a copy of this software and associated documentation
+ * files (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use,
+ * copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following
+ * conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+/dts-v1/;
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/input.h>
+#include "armada-385.dtsi"
+
+/ {
+ model = "Turris Omnia";
+ compatible = "turris,omnia", "marvell,armada385", "marvell,armada380";
+
+ chosen {
+ stdout-path = &uart0;
+ };
+
+ memory {
+ device_type = "memory";
+ reg = <0x00000000 0x40000000>; /* 1024 MB */
+ };
+
+ soc {
+ ranges = <MBUS_ID(0xf0, 0x01) 0 0xf1000000 0x100000
+ MBUS_ID(0x01, 0x1d) 0 0xfff00000 0x100000>;
+
+ internal-regs {
+
+ /* USB part of the eSATA/USB 2.0 port */
+ usb@58000 {
+ status = "okay";
+ };
+
+ sata@a8000 {
+ status = "okay";
+ };
+
+ sdhci@d8000 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&sdhci_pins>;
+ status = "okay";
+
+ bus-width = <8>;
+ no-1-8-v;
+ non-removable;
+ };
+
+ usb3@f0000 {
+ status = "okay";
+ };
+
+ usb3@f8000 {
+ status = "okay";
+ };
+ };
+
+ pcie-controller {
+ status = "okay";
+
+ pcie@1,0 {
+ /* Port 0, Lane 0 */
+ status = "okay";
+ };
+
+ pcie@2,0 {
+ /* Port 2, Lane 0 */
+ status = "okay";
+ };
+
+ pcie@3,0 {
+ /* Port 3, Lane 0 */
+ status = "okay";
+ };
+ };
+ };
+};
+
+ð0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&ge0_rgmii_pins>;
+ status = "okay";
+ phy-mode = "rgmii-id";
+
+ fixed-link {
+ speed = <1000>;
+ full-duplex;
+ };
+};
+
+ð1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&ge1_rgmii_pins>;
+ status = "okay";
+ phy-mode = "rgmii-id";
+
+ fixed-link {
+ speed = <1000>;
+ full-duplex;
+ };
+};
+
+/* WAN port */
+ð2 {
+ status = "okay";
+ phy-mode = "sgmii";
+ phy = <&phy1>;
+};
+
+&i2c0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&i2c0_pins>;
+ status = "okay";
+
+ i2cmux@70 {
+ compatible = "nxp,pca9547";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x70>;
+ status = "okay";
+
+ i2c@0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0>;
+ status = "okay";
+
+ /* STM32F0 at address 0x2a */
+ /* leds device at address 0x2b */
+
+ eeprom@54 {
+ /* holds configuration about RAM, evaluated by bootloader */
+ compatible = "at,24c64";
+ reg = <0x54>;
+ };
+ };
+
+ i2c@5 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <5>;
+
+ /* ATSHA204A at address 0x64 */
+ };
+
+ i2c@6 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <6>;
+
+ /* exposed on pin header */
+ };
+ };
+};
+
+&mdio {
+ pinctrl-names = "default";
+ pinctrl-0 = <&mdio_pins>;
+ status = "okay";
+
+ phy1: phy@1 {
+ status = "okay";
+ compatible = "marvell,88E1514", "marvell,88E1510", "ethernet-phy-ieee802.3-c22";
+ reg = <1>;
+ };
+};
+
+&pinctrl {
+ spi0cs1_pins: spi0-pins-0cs1 {
+ marvell,pins = "mpp26";
+ marvell,function = "spi0";
+ };
+};
+
+&spi0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&spi0_pins &spi0cs1_pins>;
+ status = "okay";
+
+ spi-nor@0 {
+ compatible = "spansion,s25fl164k", "jedec,spi-nor";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ reg = <0>;
+ spi-max-frequency = <40000000>;
+
+ partition@0 {
+ reg = <0x0 0x00100000>;
+ label = "U-Boot";
+ };
+
+ partition@1 {
+ reg = <0x00100000 0x00700000>;
+ label = "Rescue system";
+ };
+ };
+
+ /* @1 is on pin header */
+};
+
+&uart0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&uart0_pins>;
+ status = "okay";
+};
+
+&uart1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&uart1_pins>;
+ status = "okay";
+};
--
2.10.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH 1/4] mfd: ti_am335x_tscadc: store physical address
From: Jonathan Cameron @ 2016-11-05 17:45 UTC (permalink / raw)
To: Lee Jones
Cc: Mugunthan V N, linux-iio-u79uwXL29TY76Z2rM5mHXA, Tony Lindgren,
Rob Herring, Mark Rutland, Russell King, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, Vignesh R,
Andrew F . Davis, linux-omap-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Sekhar Nori, Peter Ujfalusi
In-Reply-To: <20161031081643.GN13127@dell>
On 31/10/16 08:16, Lee Jones wrote:
> On Sun, 30 Oct 2016, Jonathan Cameron wrote:
>
>> On 26/10/16 13:17, Lee Jones wrote:
>>> On Fri, 30 Sep 2016, Mugunthan V N wrote:
>>>
>>>> On Wednesday 28 September 2016 01:10 AM, Lee Jones wrote:
>>>>> On Wed, 21 Sep 2016, Mugunthan V N wrote:
>>>>>
>>>>>> store the physical address of the device in its priv to use it
>>>>>> for DMA addressing in the client drivers.
>>>>>>
>>>>>> Signed-off-by: Mugunthan V N <mugunthanvnm-l0cyMroinI0@public.gmane.org>
>>>>>> ---
>>>>>> drivers/mfd/ti_am335x_tscadc.c | 1 +
>>>>>> include/linux/mfd/ti_am335x_tscadc.h | 1 +
>>>>>> 2 files changed, 2 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
>>>>>> index c8f027b..0f3fab4 100644
>>>>>> --- a/drivers/mfd/ti_am335x_tscadc.c
>>>>>> +++ b/drivers/mfd/ti_am335x_tscadc.c
>>>>>> @@ -183,6 +183,7 @@ static int ti_tscadc_probe(struct platform_device *pdev)
>>>>>> tscadc->irq = err;
>>>>>>
>>>>>> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>>>> + tscadc->tscadc_phys_base = res->start;
>>>>>
>>>>> This is unusual. Can't you use a virt_to_phys() variant instead?
>>>>>
>>>>
>>>> I tried using virt_to_phys(), but its not working for me.
>>>> Also saw many drivers uses like this to get physical address
>>>> ("git grep -n " res->start;" drivers/*").
>>>
>>> Very well:
>>>
>>> For my own reference:
>>> Acked-for-MFD-by: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>>
>>> Let me know how you wish this set to be handled.
>> I'm happy to pick up the whole series. There are some more mfd
>> header changes in patch 2 but as they only add defines, I
>> don't mind that much if I don't an Ack from you on those
>> (btw this got to V3 but as patch 1 didn't change I'll carry
>> your ack forwards).
>>
>> Do you want an immutable branch? Seems unlikely to cause
>> much trouble even if there is a merge issue on all 10ish
>> lines of mfd code in the next merge window.
>
> Not at the moment, but if you could set things up so it's possible to
> create one at a later date if things go Pete Tong, that would be
> great.
Couldn't think of an easy way to do this without creating a branch
and merging it into my normal branch. I'll not push it out to
kernel.org though unless you tell me you need it.
Applied to the togreg branch (indirectly ;) of iio.git pushed out
as testing for the autobuilders to play with it.
Thanks,
Jonathan
>
^ permalink raw reply
* Re: [PATCH v3 3/4] ARM: dts: am33xx: add DMA properties for tscadc
From: Jonathan Cameron @ 2016-11-05 17:35 UTC (permalink / raw)
To: Mugunthan V N, linux-iio-u79uwXL29TY76Z2rM5mHXA
Cc: Tony Lindgren, Rob Herring, Mark Rutland, Russell King,
Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
Lee Jones, Vignesh R, Andrew F . Davis,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Sekhar Nori, Peter Ujfalusi,
John Syne
In-Reply-To: <20161005090443.24576-4-mugunthanvnm-l0cyMroinI0@public.gmane.org>
On 05/10/16 10:04, Mugunthan V N wrote:
> Add DMA properties for tscadc
>
> Signed-off-by: Mugunthan V N <mugunthanvnm-l0cyMroinI0@public.gmane.org>
The support in the driver is now working it's way through iio.git towards
linux-next. I'm guessing this and the next patch will ultimately go through
arm-soc.
Shout if you'd rather I took them through the iio tree.
Thanks,
Jonathan
> ---
> arch/arm/boot/dts/am33xx.dtsi | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
> index 98748c6..6d607b8 100644
> --- a/arch/arm/boot/dts/am33xx.dtsi
> +++ b/arch/arm/boot/dts/am33xx.dtsi
> @@ -917,6 +917,8 @@
> interrupts = <16>;
> ti,hwmods = "adc_tsc";
> status = "disabled";
> + dmas = <&edma 53 0>, <&edma 57 0>;
> + dma-names = "fifo0", "fifo1";
>
> tsc {
> compatible = "ti,am3359-tsc";
>
^ permalink raw reply
* Re: [PATCH v3 2/4] drivers: iio: ti_am335x_adc: add dma support
From: Jonathan Cameron @ 2016-11-05 17:33 UTC (permalink / raw)
To: Peter Ujfalusi, Mugunthan V N, linux-iio-u79uwXL29TY76Z2rM5mHXA
Cc: Tony Lindgren, Rob Herring, Mark Rutland, Russell King,
Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
Lee Jones, Vignesh R, Andrew F . Davis,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Sekhar Nori, John Syne
In-Reply-To: <8ff4af23-9e90-535c-0e25-9373d845caab-l0cyMroinI0@public.gmane.org>
On 05/10/16 10:26, Peter Ujfalusi wrote:
> On 10/05/16 12:04, Mugunthan V N wrote:
>> This patch adds the required pieces to ti_am335x_adc driver for
>> DMA support
>>
>> Signed-off-by: Mugunthan V N <mugunthanvnm-l0cyMroinI0@public.gmane.org>
Applied to a special branch in iio.git that I'll pull into togreg and then
push out as testing in a few minutes for the autobuilders to play with it.
(the special branch is incase Lee needs to later pull this into mfd as well).
Jonathan
>> ---
>> drivers/iio/adc/ti_am335x_adc.c | 148 ++++++++++++++++++++++++++++++++++-
>> include/linux/mfd/ti_am335x_tscadc.h | 7 ++
>> 2 files changed, 152 insertions(+), 3 deletions(-)
>
> Reviewed-by: Peter Ujfalusi <peter.ujfalusi-l0cyMroinI0@public.gmane.org>
>
>>
>> diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
>> index c3cfacca..ad9dec3 100644
>> --- a/drivers/iio/adc/ti_am335x_adc.c
>> +++ b/drivers/iio/adc/ti_am335x_adc.c
>> @@ -30,10 +30,28 @@
>> #include <linux/iio/buffer.h>
>> #include <linux/iio/kfifo_buf.h>
>>
>> +#include <linux/dmaengine.h>
>> +#include <linux/dma-mapping.h>
>> +
>> +#define DMA_BUFFER_SIZE SZ_2K
>> +
>> +struct tiadc_dma {
>> + struct dma_slave_config conf;
>> + struct dma_chan *chan;
>> + dma_addr_t addr;
>> + dma_cookie_t cookie;
>> + u8 *buf;
>> + int current_period;
>> + int period_size;
>> + u8 fifo_thresh;
>> +};
>> +
>> struct tiadc_device {
>> struct ti_tscadc_dev *mfd_tscadc;
>> + struct tiadc_dma dma;
>> struct mutex fifo1_lock; /* to protect fifo access */
>> int channels;
>> + int total_ch_enabled;
>> u8 channel_line[8];
>> u8 channel_step[8];
>> int buffer_en_ch_steps;
>> @@ -198,6 +216,67 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
>> return IRQ_HANDLED;
>> }
>>
>> +static void tiadc_dma_rx_complete(void *param)
>> +{
>> + struct iio_dev *indio_dev = param;
>> + struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> + u8 *data;
>> + int i;
>> +
>> + data = dma->buf + dma->current_period * dma->period_size;
>> + dma->current_period = 1 - dma->current_period; /* swap the buffer ID */
>> +
>> + for (i = 0; i < dma->period_size; i += indio_dev->scan_bytes) {
>> + iio_push_to_buffers(indio_dev, data);
>> + data += indio_dev->scan_bytes;
>> + }
>> +}
>> +
>> +static int tiadc_start_dma(struct iio_dev *indio_dev)
>> +{
>> + struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> + struct dma_async_tx_descriptor *desc;
>> +
>> + dma->current_period = 0; /* We start to fill period 0 */
>> + /*
>> + * Make the fifo thresh as the multiple of total number of
>> + * channels enabled, so make sure that cyclic DMA period
>> + * length is also a multiple of total number of channels
>> + * enabled. This ensures that no invalid data is reported
>> + * to the stack via iio_push_to_buffers().
>> + */
>> + dma->fifo_thresh = rounddown(FIFO1_THRESHOLD + 1,
>> + adc_dev->total_ch_enabled) - 1;
>> + /* Make sure that period length is multiple of fifo thresh level */
>> + dma->period_size = rounddown(DMA_BUFFER_SIZE / 2,
>> + (dma->fifo_thresh + 1) * sizeof(u16));
>> +
>> + dma->conf.src_maxburst = dma->fifo_thresh + 1;
>> + dmaengine_slave_config(dma->chan, &dma->conf);
>> +
>> + desc = dmaengine_prep_dma_cyclic(dma->chan, dma->addr,
>> + dma->period_size * 2,
>> + dma->period_size, DMA_DEV_TO_MEM,
>> + DMA_PREP_INTERRUPT);
>> + if (!desc)
>> + return -EBUSY;
>> +
>> + desc->callback = tiadc_dma_rx_complete;
>> + desc->callback_param = indio_dev;
>> +
>> + dma->cookie = dmaengine_submit(desc);
>> +
>> + dma_async_issue_pending(dma->chan);
>> +
>> + tiadc_writel(adc_dev, REG_FIFO1THR, dma->fifo_thresh);
>> + tiadc_writel(adc_dev, REG_DMA1REQ, dma->fifo_thresh);
>> + tiadc_writel(adc_dev, REG_DMAENABLE_SET, DMA_FIFO1);
>> +
>> + return 0;
>> +}
>> +
>> static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
>> {
>> struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> @@ -218,20 +297,30 @@ static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
>> static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
>> {
>> struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> + unsigned int irq_enable;
>> unsigned int enb = 0;
>> u8 bit;
>>
>> tiadc_step_config(indio_dev);
>> - for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels)
>> + for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels) {
>> enb |= (get_adc_step_bit(adc_dev, bit) << 1);
>> + adc_dev->total_ch_enabled++;
>> + }
>> adc_dev->buffer_en_ch_steps = enb;
>>
>> + if (dma->chan)
>> + tiadc_start_dma(indio_dev);
>> +
>> am335x_tsc_se_set_cache(adc_dev->mfd_tscadc, enb);
>>
>> tiadc_writel(adc_dev, REG_IRQSTATUS, IRQENB_FIFO1THRES
>> | IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW);
>> - tiadc_writel(adc_dev, REG_IRQENABLE, IRQENB_FIFO1THRES
>> - | IRQENB_FIFO1OVRRUN);
>> +
>> + irq_enable = IRQENB_FIFO1OVRRUN;
>> + if (!dma->chan)
>> + irq_enable |= IRQENB_FIFO1THRES;
>> + tiadc_writel(adc_dev, REG_IRQENABLE, irq_enable);
>>
>> return 0;
>> }
>> @@ -239,12 +328,18 @@ static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
>> static int tiadc_buffer_predisable(struct iio_dev *indio_dev)
>> {
>> struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> int fifo1count, i, read;
>>
>> tiadc_writel(adc_dev, REG_IRQCLR, (IRQENB_FIFO1THRES |
>> IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW));
>> am335x_tsc_se_clr(adc_dev->mfd_tscadc, adc_dev->buffer_en_ch_steps);
>> adc_dev->buffer_en_ch_steps = 0;
>> + adc_dev->total_ch_enabled = 0;
>> + if (dma->chan) {
>> + tiadc_writel(adc_dev, REG_DMAENABLE_CLEAR, 0x2);
>> + dmaengine_terminate_async(dma->chan);
>> + }
>>
>> /* Flush FIFO of leftover data in the time it takes to disable adc */
>> fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
>> @@ -430,6 +525,41 @@ static const struct iio_info tiadc_info = {
>> .driver_module = THIS_MODULE,
>> };
>>
>> +static int tiadc_request_dma(struct platform_device *pdev,
>> + struct tiadc_device *adc_dev)
>> +{
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> + dma_cap_mask_t mask;
>> +
>> + /* Default slave configuration parameters */
>> + dma->conf.direction = DMA_DEV_TO_MEM;
>> + dma->conf.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
>> + dma->conf.src_addr = adc_dev->mfd_tscadc->tscadc_phys_base + REG_FIFO1;
>> +
>> + dma_cap_zero(mask);
>> + dma_cap_set(DMA_CYCLIC, mask);
>> +
>> + /* Get a channel for RX */
>> + dma->chan = dma_request_chan(adc_dev->mfd_tscadc->dev, "fifo1");
>> + if (IS_ERR(dma->chan)) {
>> + int ret = PTR_ERR(dma->chan);
>> +
>> + dma->chan = NULL;
>> + return ret;
>> + }
>> +
>> + /* RX buffer */
>> + dma->buf = dma_alloc_coherent(dma->chan->device->dev, DMA_BUFFER_SIZE,
>> + &dma->addr, GFP_KERNEL);
>> + if (!dma->buf)
>> + goto err;
>> +
>> + return 0;
>> +err:
>> + dma_release_channel(dma->chan);
>> + return -ENOMEM;
>> +}
>> +
>> static int tiadc_parse_dt(struct platform_device *pdev,
>> struct tiadc_device *adc_dev)
>> {
>> @@ -512,8 +642,14 @@ static int tiadc_probe(struct platform_device *pdev)
>>
>> platform_set_drvdata(pdev, indio_dev);
>>
>> + err = tiadc_request_dma(pdev, adc_dev);
>> + if (err && err == -EPROBE_DEFER)
>> + goto err_dma;
>> +
>> return 0;
>>
>> +err_dma:
>> + iio_device_unregister(indio_dev);
>> err_buffer_unregister:
>> tiadc_iio_buffered_hardware_remove(indio_dev);
>> err_free_channels:
>> @@ -525,8 +661,14 @@ static int tiadc_remove(struct platform_device *pdev)
>> {
>> struct iio_dev *indio_dev = platform_get_drvdata(pdev);
>> struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> u32 step_en;
>>
>> + if (dma->chan) {
>> + dma_free_coherent(dma->chan->device->dev, DMA_BUFFER_SIZE,
>> + dma->buf, dma->addr);
>> + dma_release_channel(dma->chan);
>> + }
>> iio_device_unregister(indio_dev);
>> tiadc_iio_buffered_hardware_remove(indio_dev);
>> tiadc_channels_remove(indio_dev);
>> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
>> index e45a208..b9a53e0 100644
>> --- a/include/linux/mfd/ti_am335x_tscadc.h
>> +++ b/include/linux/mfd/ti_am335x_tscadc.h
>> @@ -23,6 +23,8 @@
>> #define REG_IRQENABLE 0x02C
>> #define REG_IRQCLR 0x030
>> #define REG_IRQWAKEUP 0x034
>> +#define REG_DMAENABLE_SET 0x038
>> +#define REG_DMAENABLE_CLEAR 0x03c
>> #define REG_CTRL 0x040
>> #define REG_ADCFSM 0x044
>> #define REG_CLKDIV 0x04C
>> @@ -36,6 +38,7 @@
>> #define REG_FIFO0THR 0xE8
>> #define REG_FIFO1CNT 0xF0
>> #define REG_FIFO1THR 0xF4
>> +#define REG_DMA1REQ 0xF8
>> #define REG_FIFO0 0x100
>> #define REG_FIFO1 0x200
>>
>> @@ -126,6 +129,10 @@
>> #define FIFOREAD_DATA_MASK (0xfff << 0)
>> #define FIFOREAD_CHNLID_MASK (0xf << 16)
>>
>> +/* DMA ENABLE/CLEAR Register */
>> +#define DMA_FIFO0 BIT(0)
>> +#define DMA_FIFO1 BIT(1)
>> +
>> /* Sequencer Status */
>> #define SEQ_STATUS BIT(5)
>> #define CHARGE_STEP 0x11
>>
>
>
^ permalink raw reply
* [PATCH 2/2] regulator: rn5t618: add RC5T619 PMIC support
From: Pierre-Hugues Husson @ 2016-11-05 16:19 UTC (permalink / raw)
To: lee.jones, robh+dt, mark.rutland, lgirdwood, broonie
Cc: devicetree, linux-kernel, Pierre-Hugues Husson
In-Reply-To: <20161105161925.14910-1-phh@phh.me>
Extend the driver to support Ricoh RC5T619.
Support the additional regulators and slightly different voltage ranges.
Signed-off-by: Pierre-Hugues Husson <phh@phh.me>
---
drivers/regulator/Kconfig | 4 ++--
drivers/regulator/rn5t618-regulator.c | 35 +++++++++++++++++++++++++++++++----
include/linux/mfd/rn5t618.h | 6 ++++++
3 files changed, 39 insertions(+), 6 deletions(-)
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 936f7cc..7b48b1a 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -656,8 +656,8 @@ config REGULATOR_RN5T618
tristate "Ricoh RN5T567/618 voltage regulators"
depends on MFD_RN5T618
help
- Say y here to support the regulators found on Ricoh RN5T567 or
- RN5T618 PMIC.
+ Say y here to support the regulators found on Ricoh RN5T567,
+ RN5T618 or RC5T619 PMIC.
config REGULATOR_RT5033
tristate "Richtek RT5033 Regulators"
diff --git a/drivers/regulator/rn5t618-regulator.c b/drivers/regulator/rn5t618-regulator.c
index 9c930eb..e7bc15b 100644
--- a/drivers/regulator/rn5t618-regulator.c
+++ b/drivers/regulator/rn5t618-regulator.c
@@ -79,6 +79,29 @@ static struct regulator_desc rn5t618_regulators[] = {
REG(LDORTC2, LDOEN2, BIT(5), LDORTC2DAC, 0x7f, 900000, 3500000, 25000),
};
+static struct regulator_desc rc5t619_regulators[] = {
+ /* DCDC */
+ REG(DCDC1, DC1CTL, BIT(0), DC1DAC, 0xff, 600000, 3500000, 12500),
+ REG(DCDC2, DC2CTL, BIT(0), DC2DAC, 0xff, 600000, 3500000, 12500),
+ REG(DCDC3, DC3CTL, BIT(0), DC3DAC, 0xff, 600000, 3500000, 12500),
+ REG(DCDC4, DC4CTL, BIT(0), DC4DAC, 0xff, 600000, 3500000, 12500),
+ REG(DCDC5, DC5CTL, BIT(0), DC5DAC, 0xff, 600000, 3500000, 12500),
+ /* LDO */
+ REG(LDO1, LDOEN1, BIT(0), LDO1DAC, 0x7f, 900000, 3500000, 25000),
+ REG(LDO2, LDOEN1, BIT(1), LDO2DAC, 0x7f, 900000, 3500000, 25000),
+ REG(LDO3, LDOEN1, BIT(2), LDO3DAC, 0x7f, 900000, 3500000, 25000),
+ REG(LDO4, LDOEN1, BIT(3), LDO4DAC, 0x7f, 900000, 3500000, 25000),
+ REG(LDO5, LDOEN1, BIT(4), LDO5DAC, 0x7f, 600000, 3500000, 25000),
+ REG(LDO6, LDOEN1, BIT(5), LDO6DAC, 0x7f, 600000, 3500000, 25000),
+ REG(LDO7, LDOEN1, BIT(6), LDO7DAC, 0x7f, 900000, 3500000, 25000),
+ REG(LDO8, LDOEN1, BIT(7), LDO8DAC, 0x7f, 900000, 3500000, 25000),
+ REG(LDO9, LDOEN2, BIT(0), LDO9DAC, 0x7f, 900000, 3500000, 25000),
+ REG(LDO10, LDOEN2, BIT(0), LDO10DAC, 0x7f, 900000, 3500000, 25000),
+ /* LDO RTC */
+ REG(LDORTC1, LDOEN2, BIT(4), LDORTCDAC, 0x7f, 1700000, 3500000, 25000),
+ REG(LDORTC2, LDOEN2, BIT(5), LDORTC2DAC, 0x7f, 900000, 3500000, 25000),
+};
+
static int rn5t618_regulator_probe(struct platform_device *pdev)
{
struct rn5t618 *rn5t618 = dev_get_drvdata(pdev->dev.parent);
@@ -86,13 +109,20 @@ static int rn5t618_regulator_probe(struct platform_device *pdev)
struct regulator_dev *rdev;
struct regulator_desc *regulators;
int i;
+ int num_regulators = 0;
switch (rn5t618->variant) {
case RN5T567:
regulators = rn5t567_regulators;
+ num_regulators = ARRAY_SIZE(rn5t567_regulators);
break;
case RN5T618:
regulators = rn5t618_regulators;
+ num_regulators = ARRAY_SIZE(rn5t618_regulators);
+ break;
+ case RC5T619:
+ regulators = rc5t619_regulators;
+ num_regulators = ARRAY_SIZE(rc5t619_regulators);
break;
default:
return -EINVAL;
@@ -101,10 +131,7 @@ static int rn5t618_regulator_probe(struct platform_device *pdev)
config.dev = pdev->dev.parent;
config.regmap = rn5t618->regmap;
- for (i = 0; i < RN5T618_REG_NUM; i++) {
- if (!regulators[i].name)
- continue;
-
+ for (i = 0; i < num_regulators; i++) {
rdev = devm_regulator_register(&pdev->dev,
®ulators[i],
&config);
diff --git a/include/linux/mfd/rn5t618.h b/include/linux/mfd/rn5t618.h
index e5a6cde..d61bc58 100644
--- a/include/linux/mfd/rn5t618.h
+++ b/include/linux/mfd/rn5t618.h
@@ -226,11 +226,17 @@ enum {
RN5T618_DCDC2,
RN5T618_DCDC3,
RN5T618_DCDC4,
+ RN5T618_DCDC5,
RN5T618_LDO1,
RN5T618_LDO2,
RN5T618_LDO3,
RN5T618_LDO4,
RN5T618_LDO5,
+ RN5T618_LDO6,
+ RN5T618_LDO7,
+ RN5T618_LDO8,
+ RN5T618_LDO9,
+ RN5T618_LDO10,
RN5T618_LDORTC1,
RN5T618_LDORTC2,
RN5T618_REG_NUM,
--
2.10.1
^ permalink raw reply related
* [PATCH 1/2] mfd: rn5t618: Add Ricoh RC5T619 PMIC support
From: Pierre-Hugues Husson @ 2016-11-05 16:19 UTC (permalink / raw)
To: lee.jones-QSEj5FYQhm4dnm+yROfE0A, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, lgirdwood-Re5JQEeQqe8AvxtiuMwx3w,
broonie-DgEjT+Ai2ygdnm+yROfE0A
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Pierre-Hugues Husson
In-Reply-To: <20161105161925.14910-1-phh-8tEavu1zA38@public.gmane.org>
The Ricoh RN5T567 is from the same family as the Ricoh RN5T618 is,
the differences are:
+ DCDC4/DCDC5
+ LDO7-10
+ Slightly different output voltage/currents
+ 32kHz Output
+ RTC
+ USB Charger detection
Signed-off-by: Pierre-Hugues Husson <phh-8tEavu1zA38@public.gmane.org>
---
Documentation/devicetree/bindings/mfd/rn5t618.txt | 16 ++++++++++------
drivers/mfd/Kconfig | 3 ++-
drivers/mfd/rn5t618.c | 1 +
include/linux/mfd/rn5t618.h | 9 +++++++++
4 files changed, 22 insertions(+), 7 deletions(-)
diff --git a/Documentation/devicetree/bindings/mfd/rn5t618.txt b/Documentation/devicetree/bindings/mfd/rn5t618.txt
index 9e6770b..65c2326 100644
--- a/Documentation/devicetree/bindings/mfd/rn5t618.txt
+++ b/Documentation/devicetree/bindings/mfd/rn5t618.txt
@@ -1,21 +1,25 @@
* Ricoh RN5T567/RN5T618 PMIC
-Ricoh RN5T567/RN5T618 is a power management IC family which integrates
-3 to 4 step-down DCDC converters, 7 low-dropout regulators, GPIOs and
-a watchdog timer. The RN5T618 provides additionally a Li-ion battery
-charger, fuel gauge and an ADC. It can be controlled through an I2C
-interface.
+Ricoh RN5T567/RN5T618/RC5T619 is a power management IC family which
+integrates 3 to 5 step-down DCDC converters, 7 to 10 low-dropout regulators,
+GPIOs, and a watchdog timer. It can be controlled through an I2C interface.
+The RN5T618/RC5T619 provides additionally a Li-ion battery charger,
+fuel gauge, and an ADC.
+The RC5T619 additionnally includes USB charger detection and an RTC.
Required properties:
- compatible: must be one of
"ricoh,rn5t567"
"ricoh,rn5t618"
+ "ricoh,rc5t619"
- reg: the I2C slave address of the device
Sub-nodes:
- regulators: the node is required if the regulator functionality is
needed. The valid regulator names are: DCDC1, DCDC2, DCDC3, DCDC4
- (RN5T567), LDO1, LDO2, LDO3, LDO4, LDO5, LDORTC1 and LDORTC2.
+ (RN5T567/RC5T619), LDO1, LDO2, LDO3, LDO4, LDO5, LDO6, LDO7, LDO8,
+ LDO9, LDO10, LDORTC1 and LDORTC2.
+ LDO7-10 are specific to RC5T619.
The common bindings for each individual regulator can be found in:
Documentation/devicetree/bindings/regulator/regulator.txt
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index c6df644..65d0a65 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -881,7 +881,8 @@ config MFD_RN5T618
select MFD_CORE
select REGMAP_I2C
help
- Say yes here to add support for the Ricoh RN5T567 or R5T618 PMIC.
+ Say yes here to add support for the Ricoh RN5T567,
+ RN5T618, RC5T619 PMIC.
This driver provides common support for accessing the device,
additional drivers must be enabled in order to use the
functionality of the device.
diff --git a/drivers/mfd/rn5t618.c b/drivers/mfd/rn5t618.c
index 9d377d6..0b5a2a1 100644
--- a/drivers/mfd/rn5t618.c
+++ b/drivers/mfd/rn5t618.c
@@ -87,6 +87,7 @@ static int rn5t618_restart(struct notifier_block *this,
static const struct of_device_id rn5t618_of_match[] = {
{ .compatible = "ricoh,rn5t567", .data = (void *)RN5T567 },
{ .compatible = "ricoh,rn5t618", .data = (void *)RN5T618 },
+ { .compatible = "ricoh,rc5t619", .data = (void *)RC5T619 },
{ }
};
MODULE_DEVICE_TABLE(of, rn5t618_of_match);
diff --git a/include/linux/mfd/rn5t618.h b/include/linux/mfd/rn5t618.h
index cadc654..e5a6cde 100644
--- a/include/linux/mfd/rn5t618.h
+++ b/include/linux/mfd/rn5t618.h
@@ -58,10 +58,13 @@
#define RN5T618_DC3CTL2 0x31
#define RN5T618_DC4CTL 0x32
#define RN5T618_DC4CTL2 0x33
+#define RN5T618_DC5CTL 0x34
+#define RN5T618_DC5CTL2 0x35
#define RN5T618_DC1DAC 0x36
#define RN5T618_DC2DAC 0x37
#define RN5T618_DC3DAC 0x38
#define RN5T618_DC4DAC 0x39
+#define RN5T618_DC5DAC 0x3a
#define RN5T618_DC1DAC_SLP 0x3b
#define RN5T618_DC2DAC_SLP 0x3c
#define RN5T618_DC3DAC_SLP 0x3d
@@ -77,6 +80,11 @@
#define RN5T618_LDO3DAC 0x4e
#define RN5T618_LDO4DAC 0x4f
#define RN5T618_LDO5DAC 0x50
+#define RN5T618_LDO6DAC 0x51
+#define RN5T618_LDO7DAC 0x52
+#define RN5T618_LDO8DAC 0x53
+#define RN5T618_LDO9DAC 0x54
+#define RN5T618_LDO10DAC 0x55
#define RN5T618_LDORTCDAC 0x56
#define RN5T618_LDORTC2DAC 0x57
#define RN5T618_LDO1DAC_SLP 0x58
@@ -231,6 +239,7 @@ enum {
enum {
RN5T567 = 0,
RN5T618,
+ RC5T619,
};
struct rn5t618 {
--
2.10.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 0/2] mfd: add Ricoh RC5T619 PMIC support
From: Pierre-Hugues Husson @ 2016-11-05 16:19 UTC (permalink / raw)
To: lee.jones-QSEj5FYQhm4dnm+yROfE0A, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, lgirdwood-Re5JQEeQqe8AvxtiuMwx3w,
broonie-DgEjT+Ai2ygdnm+yROfE0A
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
This patchset adds RC5T619 support. This PMIC is used on many
rockchip-based devices, for instance the GPD XD, or the Archos 101 Oxygen
---
Documentation/devicetree/bindings/mfd/rn5t618.txt | 16 +++--
drivers/mfd/Kconfig | 3 -
drivers/mfd/rn5t618.c | 1
drivers/regulator/Kconfig | 4 -
drivers/regulator/rn5t618-regulator.c | 35 ++++++++++--
include/linux/mfd/rn5t618.h | 15 +++++
6 files changed, 61 insertions(+), 13 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH V3 1/2] iio: adc: spmi-vadc: Update changes to support reporting of Raw adc code.
From: Jonathan Cameron @ 2016-11-05 15:59 UTC (permalink / raw)
To: Phani A, Rama Krishna, linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: robh-DgEjT+Ai2ygdnm+yROfE0A, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
smohanad-sgV2jX0FEOL9JmXXK+q4OQ, mgautam-sgV2jX0FEOL9JmXXK+q4OQ,
sivaa-sgV2jX0FEOL9JmXXK+q4OQ, knaack.h-Mmb7MZpHnFY,
lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg,
Julia.Lawall-L2FTfq7BK8M
In-Reply-To: <a36d7f9c-7716-19bb-2710-7af90f8e0787-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
On 02/11/16 13:12, Phani A, Rama Krishna wrote:
> Hi Jonathan,
>
> On 02-Nov-16 12:00 AM, Jonathan Cameron wrote:
>> On 31/10/16 08:48, Rama Krishna Phani A wrote:
>>> Hi Jonathan,
>>>
>>> On 30-Oct-16 11:05 PM, Jonathan Cameron wrote:
>>>> On 26/10/16 15:41, Rama Krishna Phani A wrote:
>>>>> Logic to convert adc code to voltage is generic across all channels.
>>>>> Different scaling can be done on the obtained voltage to report in physical
>>>>> units. Implement separate function for generic conversion logic.
>>>>> Scaling functionality can be changed per channel. Update changes to support
>>>>> reporting of Raw adc code.
>>>> Pleas rewrite this description. Perhaps give examples of the changes
>>>> it makes to what is read from the various attributes?
>>> There are several channels in the ADC of PMIC which can be used to
>>> measure voltage, temperature, current etc., Hardware provides
>>> readings for all channels in adc code. That adc code needs to be
>>> converted to voltage. The logic for conversion of adc code to voltage
>>> is common for all ADC channels(voltage, temperature and current
>>> .,etc). Once voltage is obtained ., scaling is done on that voltage.
>>>
>>> For Ex., Thermal SW wants to know the temperature of thermistor on
>>> PMIC and it expects the temperature to be reported in millidegC. ADC
>>> channel is used to read the adc code and convert it to voltage. Once
>>> the voltage is available based on the thermistor spec that voltage is
>>> mapped to a temperature and then that value is reported to Thermal
>>> SW.
>>>
>>> Mapping of voltage to temperature is called scaling for that channel
>>> and scaling function can be different per channel based on how the
>>> voltage is reported.
>> Is the thermistor always part of the device? (i.e. in the chip) in which
>> case this might be fine. If it's external then it needs to be described
>> by a separate device which acts as a consumer of the IIO channel and
>> in turn provides the scaled output to thermal.
>>
>> The thermistor should really be separately described. This is already
>> done in drivers/hwmon/ntc_thermistor
>>
>> Are any of these scalings characteristics of the chip supported by
>> this driver, or are they the result of external hardware?
>
> All the VADC channels i.e., Voltage, temperature(thermistors and
> other channels) are part of PMIC chip. The scaling functionalities
> supported in this driver are for the adc channels which are part of
> PMIC chip.
>>>>
>>>> I haven't immediately followed what this change is actually doing.
>>>>
>>>> I 'think' the point here is to not apply the calibration to
>>>> the raw adc counts when a true raw read is requested?
>>>>
>>> When a true raw read is requested .,Scaling is not applied.
>>>> There are several unconnected looking changes in here...
>>>>>
>>>>> Signed-off-by: Rama Krishna Phani A <rphani-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>>>>> ---
>>>>> drivers/iio/adc/qcom-spmi-vadc.c | 54 +++++++++++++++++++++-------------------
>>>>> 1 file changed, 28 insertions(+), 26 deletions(-)
>>>>>
>>>>> diff --git a/drivers/iio/adc/qcom-spmi-vadc.c b/drivers/iio/adc/qcom-spmi-vadc.c
>>>>> index c2babe5..ff4d549 100644
>>>>> --- a/drivers/iio/adc/qcom-spmi-vadc.c
>>>>> +++ b/drivers/iio/adc/qcom-spmi-vadc.c
>>>>> @@ -1,5 +1,5 @@
>>>>> /*
>>>>> - * Copyright (c) 2012-2014, The Linux Foundation. All rights reserved.
>>>>> + * Copyright (c) 2012-2016, 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
>>>>> @@ -84,7 +84,7 @@
>>>>> #define VADC_MAX_ADC_CODE 0xa800
>>>>>
>>>>> #define VADC_ABSOLUTE_RANGE_UV 625000
>>>>> -#define VADC_RATIOMETRIC_RANGE_UV 1800000
>>>>> +#define VADC_RATIOMETRIC_RANGE 1800
>>>>>
>>>>> #define VADC_DEF_PRESCALING 0 /* 1:1 */
>>>>> #define VADC_DEF_DECIMATION 0 /* 512 */
>>>>> @@ -418,7 +418,7 @@ static int vadc_measure_ref_points(struct vadc_priv *vadc)
>>>>> u16 read_1, read_2;
>>>>> int ret;
>>>>>
>>>>> - vadc->graph[VADC_CALIB_RATIOMETRIC].dx = VADC_RATIOMETRIC_RANGE_UV;
>>>>> + vadc->graph[VADC_CALIB_RATIOMETRIC].dx = VADC_RATIOMETRIC_RANGE;
>>>>> vadc->graph[VADC_CALIB_ABSOLUTE].dx = VADC_ABSOLUTE_RANGE_UV;
>>>>>
>>>>> prop = vadc_get_channel(vadc, VADC_REF_1250MV);
>>>>> @@ -468,21 +468,30 @@ static int vadc_measure_ref_points(struct vadc_priv *vadc)
>>>>> return ret;
>>>>> }
>>>>>
>>>>> -static s32 vadc_calibrate(struct vadc_priv *vadc,
>>>>> - const struct vadc_channel_prop *prop, u16 adc_code)
>>>>> +static void vadc_scale_calib(struct vadc_priv *vadc, u16 adc_code,
>>>>> + const struct vadc_channel_prop *prop,
>>>>> + s64 *scale_voltage)
>>>>> {
>>>>> - const struct vadc_prescale_ratio *prescale;
>>>>> - s64 voltage;
>>>>> + *scale_voltage = (adc_code -
>>>>> + vadc->graph[prop->calibration].gnd);
>>>>> + *scale_voltage *= vadc->graph[prop->calibration].dx;
>>>>> + *scale_voltage = div64_s64(*scale_voltage,
>>>>> + vadc->graph[prop->calibration].dy);
>>>>> + if (prop->calibration == VADC_CALIB_ABSOLUTE)
>>>>> + *scale_voltage +=
>>>>> + vadc->graph[prop->calibration].dx;
>>>>>
>>>>> - voltage = adc_code - vadc->graph[prop->calibration].gnd;
>>>>> - voltage *= vadc->graph[prop->calibration].dx;
>>>>> - voltage = div64_s64(voltage, vadc->graph[prop->calibration].dy);
>>>>> + if (*scale_voltage < 0)
>>>>> + *scale_voltage = 0;
>>>>> +}
>>>>>
>>>>> - if (prop->calibration == VADC_CALIB_ABSOLUTE)
>>>>> - voltage += vadc->graph[prop->calibration].dx;
>>>>> +static s64 vadc_scale_fn(struct vadc_priv *vadc,
>>>>> + const struct vadc_channel_prop *prop, u16 adc_code)
>>>>> +{
>>>>> + const struct vadc_prescale_ratio *prescale;
>>>>> + s64 voltage = 0;
>>>>>
>>>>> - if (voltage < 0)
>>>>> - voltage = 0;
>>>>> + vadc_scale_calib(vadc, adc_code, prop, &voltage);
>>>>>
>>>>> prescale = &vadc_prescale_ratios[prop->prescale];
>>>>>
>>>>> @@ -552,11 +561,8 @@ static int vadc_read_raw(struct iio_dev *indio_dev,
>>>>> if (ret)
>>>>> break;
>>>>>
>>>>> - *val = vadc_calibrate(vadc, prop, adc_code);
>>>>> + *val = vadc_scale_fn(vadc, prop, adc_code);
>>>>>
>>>>> - /* 2mV/K, return milli Celsius */
>>>>> - *val /= 2;
>>>>> - *val -= KELVINMIL_CELSIUSMIL;
>>>>> return IIO_VAL_INT;
>>>>> case IIO_CHAN_INFO_RAW:
>>>>> prop = &vadc->chan_props[chan->address];
>>>>> @@ -564,12 +570,8 @@ static int vadc_read_raw(struct iio_dev *indio_dev,
>>>>> if (ret)
>>>>> break;
>>>>>
>>>>> - *val = vadc_calibrate(vadc, prop, adc_code);
>>>>> + *val = (int)adc_code;
>>>>> return IIO_VAL_INT;
>>>> So this is 'more raw'.
>>> Yes., its raw value.
>>>>> - case IIO_CHAN_INFO_SCALE:
>>>>> - *val = 0;
>>>>> - *val2 = 1000;
>>>>> - return IIO_VAL_INT_PLUS_MICRO;
>>>>> default:
>>>>> ret = -EINVAL;
>>>>> break;
>>>>> @@ -616,8 +618,8 @@ struct vadc_channels {
>>>>> VADC_CHAN(_dname, IIO_TEMP, BIT(IIO_CHAN_INFO_PROCESSED), _pre) \
>>>>>
>>>>> #define VADC_CHAN_VOLT(_dname, _pre) \
>>>>> - VADC_CHAN(_dname, IIO_VOLTAGE, \
>>>>> - BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE), \
>>>>> + VADC_CHAN(_dname, IIO_VOLTAGE, \
>>>>> + BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_PROCESSED),\
>>>>> _pre) \
>>>> It very unusual to report both raw and processed values. Please explain
>>>> why that is needed here? It may be valid to maintain backwards compatibility
>>>> of ABI. Which would be fine. However if I read the above correctly you are
>>>> changing what comes out of reading the raw value so the ABI just changed...
>>>>
>>> With the help of IIO sysfs ., we can read the ADC channel readings
>>> either in RAW format or in processed format. There are two separate
>>> individual entries to read the ADC channel either in Raw format or in
>>> processed format. Most of the clients for ADC expect the readings in
>>> processed format.
>> If we are talking in kernel, that is worked out by the application of
>> scale. The IIO in kernel interfaces will do this automatically.
>>
>> I we are talking in userspace, then the userspace needs to be
>> extended to support raw and scale reading.
>
> Every channel present in adc has an unique conversion formula for
> obtained voltage, suggested by Hardware designers.
>
> Ex: For die_temp channel., Temp = 2mv/Kelvin Above formula has to be
> applied on obtained voltage in order for the channel to report the
> temperature in milldegC.
>
> Like wise every channel has unique way of conversion logic suggested
> by HW folks. That conversion logic is done in ADC driver.
If it is linear then ideally expose it as a raw channel and offset + scale.
If non linear then a processed channel with the conversion logic in kernel
as we have no means of describing it to userspace.
>
>>>>
>>
>>>>> /*
>>>>> @@ -850,9 +852,9 @@ static int vadc_get_dt_data(struct vadc_priv *vadc, struct device_node *node)
>>>>>
>>>>> iio_chan->channel = prop.channel;
>>>>> iio_chan->datasheet_name = vadc_chan->datasheet_name;
>>>>> + iio_chan->extend_name = child->name;
>>>> What's this change?
>>> We can choose how we want to display our adc channel entries in sysfs. Am using the child node name to be displayed as the sysfs entry rather than channel number for easy interpretation.
>>>
>>> For ex: for vcoin(coin battery voltage channel.,) with this change it appears like below in iio adc sysfs
>>>
>>> "in_voltage_vcoin_input"
>> No. This introduces a mass of undocumented (and uncontrolled) ABI.
>> If there are reasons to add such a label then it should not be done
>> in the file name.
>
> "extended_name" is an existing field in "iio_chan_spec" structure,
> present in iio.h(include\linux\iio) and has documentation regarding
> the functionality. Pasting it here for quick reference.>
> * @extend_name:Allows labeling of channel attributes with an
> * informative name. Note this has no effect codes etc,
> * unlike modifiers.
>
> Am trying to use the existing field here., initializing it with a
> value which is easy for interpretation of channel attributes.
OK. I'd misunderstood what was going on here. If and only if these
channels are internally linked to a particular voltage / temperature
sensor etc it 'may' make sense.
If they are linked to a hardware monitoring channel then ideally we
would also be mapping them across to hwmon through the iio_hwmon
bridge.
>>>
>>>>> iio_chan->info_mask_separate = vadc_chan->info_mask;
>>>>> iio_chan->type = vadc_chan->type;
>>>>> - iio_chan->indexed = 1;
>>>> Or for that matter this one...
>>> reason explained above.
No they still need to be indexed. If we ever have events etc
on these channels or want to 'consume' them in other kernel drivers
they need to be indexed. The strings are not available. They are
just an convenience in naming of channels (and one I'm kind of
wishing we had never provided as it leads to uncontrolled ABI
explosion like here).
Anyhow, the key thing that wasn't clear in this patch description
and left me confused is that the scaling previously reported was
simply wrong and this was fixing it!
You do have several overlapping changes here which confused matters
further. This extend_name stuff for example is a different
issue. Your big problem is that it is an ABI change and hence
a non starter at this point. Fixing wrong scaling is one thing
but changing the naming of sysfs files like this is not going to
be possible now the driver has been out there for a while.
Jonathan
>>>>> iio_chan->address = index++;
>>>>>
>>>>> iio_chan++;
>>>>>
>>>>
>>>
>>> Thanks,
>>> Ramakrishna
>>>
>>> ---
>>> This email has been checked for viruses by Avast antivirus software.
>>> https://www.avast.com/antivirus
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
> Thanks,
> Ramakrishna
^ permalink raw reply
* Re: [PATCH 02/10] iio: adc: Add stm32 support
From: Jonathan Cameron @ 2016-11-05 15:44 UTC (permalink / raw)
To: Fabrice Gasnier, linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: linux-I+IVW8TIWO2tmTQ+vhA3Yw, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, mcoquelin.stm32-Re5JQEeQqe8AvxtiuMwx3w,
alexandre.torgue-qxv4g6HH51o, lars-Qo5EllUWu/uELgA04lAiVw,
knaack.h-Mmb7MZpHnFY, pmeerw-jW+XmwGofnusTnJN9+BGXg,
Benjamin GAIGNARD
In-Reply-To: <4687bde5-1e1e-83bc-0aca-3224399f0fa2-qxv4g6HH51o@public.gmane.org>
On 03/11/16 08:20, Fabrice Gasnier wrote:
> On 10/30/2016 04:27 PM, Jonathan Cameron wrote:
>> On 25/10/16 17:25, Fabrice Gasnier wrote:
>>> This patch adds support for STMicroelectronics STM32 MCU's analog to
>>> digital converter.
>>>
>>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>
>> Hi Fabrice,
>>
>> Sometimes I hate SoC ADCs. For some reason the hardware designers seem to
>> try and throw everything and the kitchen sink at them. Discontinuous mode
>> as an example in this device. Not seen that particular piece of fun before
>> and glad to see you haven't 'yet' tried to support it!
>>
>> Anyhow, the complexity of the hardware leads to an initially complex driver.
>> My first thought it that this would be easier to follow / review if we
>> built it up in smaller steps. Perhaps ditch the injected channel support
>> entirely in the first instance. I also wonder if you don't need to support
>> that whole thing (injected sampling) as another iio device entirely using the
>> same channels. That's kind of what it is from a data flow point of view
>> (we've had arbitary sequencers before with priorities - don't think anyone
>> ever decided the pain was worth supporting the complexity, but right answer
>> has always been multiple IIO devices).
> Hi Jonathan,
>
> First, many thanks for your review. I agree with you, most reasonable approach is to remove some
> complexity to ease the review. Regarding injected support, basically, bellow approach is to use
> separate IIO devices for regular and injected. But, I'll remove this, at least for now, in next patch set.
>
>> You also have at least one layer of abstraction in here that serves no
>> current purpose. Please clear that out for now. It'll make the code
>> shorter and easier to follow. If/when other parts are introduced then
>> is the time to do that transistion to having the abstraction.
>
> From your suggestion, this may end-up in a single driver file in drivers/iio.
> I think I'll try to keep simple routines like start, stop, conf_scan and so on, but
> remove indirection routines from stm32-adc.h file (e.g. stm32_adc_ops).
> Is it in line with your suggestions ?
Sure, some of those will want to be in their own functions so it sounds
about right.
>
>>
>> My first thought on the double / tripple adc handling is that you'd be better
>> off handling them as 3 separate devices then doing some 'unusual' trigger
>> handling to support the weird sequencing. Guessing you thought about that?
>> If so could you lay out your reasoning for the single driver instance approach.
>> I'm not arguing against it btw, merely want to understand your reasoning!
>
> I mainly came up with a single driver instance approach because there are basically
> 3 identical ADC instances 'mapped' in a single IP with few common resources.
> I usually see mfd are more heterogeneous and declare cells for various subsystem drivers.
> But I can try to move to mfd as you're suggesting.
> I just hope this will not bring more complexity.
If anything I suspect it'll end up simpler to read (be it a tiny bit longer
in terms of lines of code).
>
>>
>> It would be tricky given one set of channels are selectable over 3 devices
>> and there are constraints to enforce (not sampling same channel on two ADCs
>> at the same time) but not impossible... Perhaps what you have here is
>> indeed simpler!
>>
>> Whilst it's been a nasty job to review, I'm guessing writing it was
>> much worse ;) Pretty good starting point though might take a little while
>> to pin down the remaining questions on how best to handle this particular
>> monster.
> My apologies... I hope you didn't had much of a headache :-) by reading me.
> More questions bellow.
>
>> Jonathan
>>> ---
>>> drivers/iio/adc/Kconfig | 2 +
>>> drivers/iio/adc/Makefile | 1 +
>>> drivers/iio/adc/stm32/Kconfig | 34 ++
>>> drivers/iio/adc/stm32/Makefile | 4 +
>>> drivers/iio/adc/stm32/stm32-adc.c | 999 ++++++++++++++++++++++++++++++++++++
>>> drivers/iio/adc/stm32/stm32-adc.h | 442 ++++++++++++++++
>>> drivers/iio/adc/stm32/stm32f4-adc.c | 574 +++++++++++++++++++++
>>> 7 files changed, 2056 insertions(+)
>>> create mode 100644 drivers/iio/adc/stm32/Kconfig
>>> create mode 100644 drivers/iio/adc/stm32/Makefile
>>> create mode 100644 drivers/iio/adc/stm32/stm32-adc.c
>>> create mode 100644 drivers/iio/adc/stm32/stm32-adc.h
>>> create mode 100644 drivers/iio/adc/stm32/stm32f4-adc.c
>>>
>>> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
>>> index 7edcf32..5c96a55 100644
>>> --- a/drivers/iio/adc/Kconfig
>>> +++ b/drivers/iio/adc/Kconfig
>>> @@ -583,4 +583,6 @@ config XILINX_XADC
>>> The driver can also be build as a module. If so, the module will be called
>>> xilinx-xadc.
>>> +source "drivers/iio/adc/stm32/Kconfig"
>>> +
>>> endmenu
>>> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
>>> index 7a40c04..a9dbf3a 100644
>>> --- a/drivers/iio/adc/Makefile
>>> +++ b/drivers/iio/adc/Makefile
>>> @@ -13,6 +13,7 @@ obj-$(CONFIG_AD7791) += ad7791.o
>>> obj-$(CONFIG_AD7793) += ad7793.o
>>> obj-$(CONFIG_AD7887) += ad7887.o
>>> obj-$(CONFIG_AD799X) += ad799x.o
>>> +obj-$(CONFIG_ARCH_STM32) += stm32/
>>> obj-$(CONFIG_AT91_ADC) += at91_adc.o
>>> obj-$(CONFIG_AT91_SAMA5D2_ADC) += at91-sama5d2_adc.o
>>> obj-$(CONFIG_AXP288_ADC) += axp288_adc.o
>>> diff --git a/drivers/iio/adc/stm32/Kconfig b/drivers/iio/adc/stm32/Kconfig
>>> new file mode 100644
>>> index 0000000..245d037
>>> --- /dev/null
>>> +++ b/drivers/iio/adc/stm32/Kconfig
>>> @@ -0,0 +1,34 @@
>>> +#
>>> +# STM32 familly ADC drivers
>>> +#
>>> +
>>> +config STM32_ADC
>>> + tristate
>>> + select REGULATOR
>>> + select REGULATOR_FIXED_VOLTAGE
>>> + select IIO_BUFFER
>>> + select IIO_TRIGGERED_BUFFER
>>> + help
>>> + Say yes here to build the driver for the STMicroelectronics
>>> + STM32 analog-to-digital converter (ADC).
>>> +
>>> + This driver can also be built as a module. If so, the module
>>> + will be called stm32-adc.
>>> +
>>> +config STM32F4_ADC
>>> + tristate "STMicroelectronics STM32F4 adc"
>>> + depends on ARCH_STM32 || COMPILE_TEST
>>> + depends on OF
>>> + select STM32_ADC
>>> + help
>>> + Say yes here to build support for STMicroelectronics stm32f4 Analog
>>> + to Digital Converter (ADC).
>>> +
>>> + This driver can also be built as a module. If so, the module
>>> + will be called stm32f4-adc.
>>> +
>>> +config STM32_ADC_DEBUG
>>> + bool "Enable debug for stm32 ADC drivers"
>>> + depends on STM32_ADC
>>> + help
>>> + Say "yes" to enable debug messages, on stm32 ADC drivers.
>>> diff --git a/drivers/iio/adc/stm32/Makefile b/drivers/iio/adc/stm32/Makefile
>>> new file mode 100644
>>> index 0000000..83e8154
>>> --- /dev/null
>>> +++ b/drivers/iio/adc/stm32/Makefile
>>> @@ -0,0 +1,4 @@
>>> +# Core
>>> +subdir-ccflags-$(CONFIG_STM32_ADC_DEBUG) := -DDEBUG
>>> +obj-$(CONFIG_STM32_ADC) += stm32-adc.o
>>> +obj-$(CONFIG_STM32F4_ADC) += stm32f4-adc.o
>>> diff --git a/drivers/iio/adc/stm32/stm32-adc.c b/drivers/iio/adc/stm32/stm32-adc.c
>>> new file mode 100644
>>> index 0000000..1e0850d
>>> --- /dev/null
>>> +++ b/drivers/iio/adc/stm32/stm32-adc.c
>>> @@ -0,0 +1,999 @@
> [snip]
>
>>> +
>>> +static int stm32_adc_conf_scan(struct iio_dev *indio_dev,
>>> + const unsigned long *scan_mask)
>>> +{
>>> + struct stm32_adc *adc = iio_priv(indio_dev);
>>> + int ret;
>>> +
>>> + ret = stm32_adc_clk_sel(adc);
>>> + if (ret) {
>>> + dev_err(&indio_dev->dev, "Clock sel failed\n");
>>> + return ret;
>>> + }
>>> +
>>> + ret = stm32_adc_enable(adc);
>>> + if (ret) {
>>> + dev_err(&indio_dev->dev, "Failed to enable adc\n");
>>> + return ret;
>>> + }
>>> +
>>> + ret = stm32_adc_conf_scan_seq(indio_dev, scan_mask);
>>> + if (ret) {
>>> + dev_err(&indio_dev->dev, "Failed to configure sequence\n");
>>> + goto err_dis;
>>> + }
>> It's horrible but to end up in the 'obvious' state I'd disable the adc
>> again assuming that doesn't kill the stuff that is configured.
> I'll check this and try to come up with something.
>
>>> +
>>> + return 0;
>>> +
>>> +err_dis:
>>> + stm32_adc_disable(adc);
>>> +
>>> + return ret;
>>> +}
>>> +
> [snip]
>>> +/**
>>> + * stm32_adc_single_conv() - perform a single conversion
>>> + * @indio_dev: IIO device
>>> + * @chan: IIO channel
>>> + * @result: conversion result
>>> + *
>>> + * The function performs a single conversion on a given channel, by
>>> + * by:
>>> + * - creating scan mask with only one channel
>>> + * - using SW trigger
>>> + * - then start single conv
>>> + */
>>> +static int stm32_adc_single_conv(struct iio_dev *indio_dev,
>>> + const struct iio_chan_spec *chan,
>>> + int *val)
>>> +{
>>> + struct stm32_adc *adc = iio_priv(indio_dev);
>>> + unsigned long *scan_mask;
>>> + long timeout;
>>> + u16 result;
>>> + int ret;
>>> +
>>> + scan_mask = kcalloc(BITS_TO_LONGS(indio_dev->masklength), sizeof(long),
>>> + GFP_KERNEL);
>> This is known maximum length... I'd just avoid the complexity of allocating
>> it like this - a comment would do the job to say it is the right length.
> Do you suggest to use a predefined variable (like unsigned long scan_mask) directly ?
> And add a more basic test on 'masklength', to be sure ?
A test would make sense as would prevent any problems if this driver
is extended to support much larger devices in future.
>
>>> + if (!scan_mask)
>>> + return -ENOMEM;
>>> +
>>> + set_bit(chan->scan_index, scan_mask);
>>> +
>>> + reinit_completion(&adc->completion);
>>> +
>>> + adc->bufi = 0;
>>> + adc->num_conv = 1;
>>> + adc->buffer = &result;
>>> +
>>> + ret = stm32_adc_conf_scan(indio_dev, scan_mask);
>>> + if (ret)
>>> + goto free;
>>> +
>>> + /* No HW trigger: conversion can be launched in SW */
>>> + ret = stm32_adc_set_trig(indio_dev, NULL);
>> Put it back again afterwards? Otherwise some nasty race conditions look
>> likely to me.. (userspace sets trigger and is about to enable the buffer
>> when along comes this code and changes it underneath).
> I'll fix this.
>
>>> + if (ret) {
>>> + dev_err(&indio_dev->dev, "Can't set SW trigger\n");
>>> + goto adc_disable;
>>> + }
>>> +
>>> + stm32_adc_conv_irq_enable(adc);
>>> +
>>> + ret = stm32_adc_start_conv(adc);
>>> + if (ret) {
>>> + dev_err(&indio_dev->dev, "Failed to start single conv\n");
>>> + goto irq_disable;
>>> + }
>>> +
>>> + timeout = wait_for_completion_interruptible_timeout(
>>> + &adc->completion, STM32_ADC_TIMEOUT);
>>> + if (timeout == 0) {
>>> + dev_warn(&indio_dev->dev, "Conversion timed out!\n");
>>> + ret = -ETIMEDOUT;
>>> + } else if (timeout < 0) {
>>> + dev_warn(&indio_dev->dev, "Interrupted conversion!\n");
>>> + ret = -EINTR;
>>> + } else {
>>> + *val = result & STM32_RESULT_MASK;
>>> + ret = IIO_VAL_INT;
>>> + }
>>> +
>>> + if (stm32_adc_stop_conv(adc))
>>> + dev_err(&indio_dev->dev, "stop failed\n");
>>> +
>>> +irq_disable:
>>> + stm32_adc_conv_irq_disable(adc);
>>> +
>>> +adc_disable:
>>> + stm32_adc_disable(adc);
>>> +
>>> +free:
>>> + kfree(scan_mask);
>>> + adc->buffer = NULL;
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static int stm32_adc_read_raw(struct iio_dev *indio_dev,
>>> + struct iio_chan_spec const *chan,
>>> + int *val, int *val2, long mask)
>>> +{
>>> + struct stm32_adc *adc = iio_priv(indio_dev);
>>> + int ret = -EINVAL;
>>> +
>>> + switch (mask) {
>>> + case IIO_CHAN_INFO_RAW:
>>> + ret = iio_device_claim_direct_mode(indio_dev);
>>> + if (ret)
>>> + return ret;
>>> + if (chan->type == IIO_VOLTAGE)
>>> + ret = stm32_adc_single_conv(indio_dev, chan, val);
>>> + iio_device_release_direct_mode(indio_dev);
>>> + break;
>>> + case IIO_CHAN_INFO_SCALE:
>>> + *val = adc->common->vref_mv;
>>> + *val2 = chan->scan_type.realbits;
>>> + ret = IIO_VAL_FRACTIONAL_LOG2;
>>> + break;
>>> + default:
>>> + break;
>>> + }
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +/**
>>> + * stm32_adc_isr() - Treat interrupt for one ADC instance within ADC block
>> As this is kernel doc please document the parameter as well. Otherwise
>> we'll get a pile of warnings!
> Sure.
>>> + */
>>> +static irqreturn_t stm32_adc_isr(struct stm32_adc *adc)
>>> +{
>>> + struct iio_dev *indio_dev = iio_priv_to_dev(adc);
>>> + const struct stm32_adc_reginfo *reginfo =
>>> + adc->common->data->adc_reginfo;
>>> + u32 mask, clr_mask, status = stm32_adc_readl(adc, reginfo->isr);
>>> +
>>> + if (adc->injected) {
>>> + mask = reginfo->jeoc;
>>> + clr_mask = mask;
>>> + } else {
>>> + mask = reginfo->eoc;
>>> + /* don't clear 'eoc' as it is cleared when reading 'dr' */
>>> + clr_mask = 0;
>>> + }
>>> +
>>> + /* clear irq */
>>> + stm32_adc_writel(adc, reginfo->isr, status & ~clr_mask);
>> Want to do this in the non injected case? it's a noop isn't it?
> I'll rework this.
>
>>
>>> + status &= mask;
>>> +
>>> + /* Regular data */
>>> + if (status & reginfo->eoc) {
>> Hmm.. this is a little bit of 'missuse' of the standard trigger architecture
>> but as long as it's restricted to just this device I don't suppose we need
>> to care. Only reason we need it is to provide control of 'which' hardware
>> trigger is being used.
>>
>> Guessing the DMA will almost always be turned on and will make this oddity
>> effectively disappear.
> I'm not sure to understand your remark. Above test checks end of conversion status flag.
> Or do you talk about bellow lines ? Can you please clarify ?
Took me a while to figure this out. (i.e. I wasn't sure what I meant either!)
This setup corresponds (more or less) to having an external trigger fire
off a software based sequencer. So we'd expect the 'loop' element of this
to run in the buffer handler rather than the trigger handler. In theory
that would allow other triggers to be used as well as the ones supported
in hardware.
Here that is somewhat of a pain however. If there weren't multiple triggers
to select between I'd just suggest dropping the trigger interface entirely
(it's optional) but then we'd have to do something custom to select
which of the device supplied triggers to use.
Hence probably best plan is leave it as it is. Sometimes hardware
just doesn't fit the conceptual model we have for it!
>
>>> + adc->buffer[adc->bufi] = stm32_adc_readl(adc, reginfo->dr);
>>> + if (iio_buffer_enabled(indio_dev)) {
>>> + adc->bufi++;
>>> + if (adc->bufi >= adc->num_conv) {
>>> + stm32_adc_conv_irq_disable(adc);
>>> + iio_trigger_poll(indio_dev->trig);
>>> + }
>>> + } else {
>>> + complete(&adc->completion);
>>> + }
>>> + }
>>> +
>>> + /* Injected data */
>>> + if (status & reginfo->jeoc) {
>>> + int i;
>>> +
>>> + for (i = 0; i < adc->num_conv; i++) {
>>> + adc->buffer[i] = stm32_adc_readl(adc, reginfo->jdr[i]);
>>> + adc->bufi++;
>>> + }
>>> +
>>> + if (iio_buffer_enabled(indio_dev)) {
>>> + stm32_adc_conv_irq_disable(adc);
>>> + iio_trigger_poll(indio_dev->trig);
>>> + } else {
>>> + complete(&adc->completion);
>>> + }
>>> + }
>>> +
>>> + /*
>>> + * In case end of conversion flags have been handled, this has been
>>> + * handled for this ADC instance
>>> + */
>>> + if (status)
>>> + return IRQ_HANDLED;
>>> +
>>> + /* This adc instance didn't trigger this interrupt */
>>> + return IRQ_NONE;
>>> +}
>>> +
>>> +/**
>>> + * stm32_adc_common_isr() - Common isr for the whole ADC block
>>> + *
>>> + * There is one IRQ for all ADCs in ADC block, check all instances.
>>> + */
>>> +static irqreturn_t stm32_adc_common_isr(int irq, void *data)
>>> +{
>>> + struct stm32_adc_common *common = data;
>>> + irqreturn_t ret = IRQ_NONE;
>>> + struct stm32_adc *adc;
>>> +
>>> + list_for_each_entry(adc, &common->adc_list, adc_list)
>>> + ret |= stm32_adc_isr(adc);
>> Hmm.. ret |= is rather fragile. Preferable to make the handling of NONE
>> vs IRQ_HANDLED explicit.
>>
>> If you were to split the driver up as I suggested might make sense above,
>> then this would be done with an irq chip in a top level device (effectively
>> a very simple mfd).
> I'll look into it.
>
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +/**
>>> + * stm32_adc_validate_trigger() - validate trigger for stm32 adc
>>> + * @indio_dev: IIO device
>>> + * @trig: new trigger
>>> + *
>>> + * Returns: 0 if trig matches one of the triggers registered by stm32 adc
>>> + * driver, -EINVAL otherwise.
>>> + */
>>> +static int stm32_adc_validate_trigger(struct iio_dev *indio_dev,
>>> + struct iio_trigger *trig)
>>> +{
>>> + return stm32_adc_get_trig_index(indio_dev, trig) < 0 ? -EINVAL : 0;
>>> +}
>>> +
>>> +static int stm32_adc_update_scan_mode(struct iio_dev *indio_dev,
>>> + const unsigned long *scan_mask)
>> I'm glad you kept this relatively simple compared to some of the
>> 'fun' the hardware is capable of. Very wise!
>>> +{
>>> + struct stm32_adc *adc = iio_priv(indio_dev);
>>> + int ret;
>>> + u32 bit;
>>> +
>>> + adc->num_conv = 0;
>>> + for_each_set_bit(bit, scan_mask, indio_dev->masklength)
>>> + adc->num_conv++;
>>> +
>>> + ret = stm32_adc_conf_scan(indio_dev, scan_mask);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int stm32_adc_of_xlate(struct iio_dev *indio_dev,
>>> + const struct of_phandle_args *iiospec)
>>> +{
>>> + int i;
>>> +
>>> + for (i = 0; i < indio_dev->num_channels; i++)
>>> + if (indio_dev->channels[i].channel == iiospec->args[0])
>>> + return i;
>>> +
>>> + return -EINVAL;
>>> +}
>>> +
>>> +/**
>>> + * stm32_adc_debugfs_reg_access - read or write register value
>>> + *
>>> + * To read a value from an ADC register:
>>> + * echo [ADC reg offset] > direct_reg_access
>>> + * cat direct_reg_access
>>> + *
>>> + * To write a value in a ADC register:
>>> + * echo [ADC_reg_offset] [value] > direct_reg_access
>>> + */
>>> +static int stm32_adc_debugfs_reg_access(struct iio_dev *indio_dev,
>>> + unsigned reg, unsigned writeval,
>>> + unsigned *readval)
>>> +{
>>> + struct stm32_adc *adc = iio_priv(indio_dev);
>>> +
>>> + if (!readval)
>>> + stm32_adc_writel(adc, reg, writeval);
>>> + else
>>> + *readval = stm32_adc_readl(adc, reg);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static const struct iio_info stm32_adc_iio_info = {
>>> + .read_raw = stm32_adc_read_raw,
>>> + .validate_trigger = stm32_adc_validate_trigger,
>>> + .update_scan_mode = stm32_adc_update_scan_mode,
>>> + .debugfs_reg_access = stm32_adc_debugfs_reg_access,
>>> + .of_xlate = stm32_adc_of_xlate,
>>> + .driver_module = THIS_MODULE,
>>> +};
>>> +
>>> +static int stm32_adc_buffer_postdisable(struct iio_dev *indio_dev)
>>> +{
>>> + struct stm32_adc *adc = iio_priv(indio_dev);
>>> +
>>> + stm32_adc_disable(adc);
>> This is a surprise as postdisbale should balance preenable...
>> Ah, you have update scan mode enabling the adc. If you can balance it
>> better by moving that to preenable please do as it is more 'obviously' correct.
> I'll try to rework this.
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static const struct iio_buffer_setup_ops iio_triggered_buffer_setup_ops = {
>>> + .postenable = &iio_triggered_buffer_postenable,
>>> + .predisable = &iio_triggered_buffer_predisable,
>>> + .postdisable = &stm32_adc_buffer_postdisable,
>>> +};
>>> +
>>> +static int stm32_adc_validate_device(struct iio_trigger *trig,
>>> + struct iio_dev *indio_dev)
>>> +{
>>> + struct iio_dev *indio = iio_trigger_get_drvdata(trig);
>>> +
>>> + return indio != indio_dev ? -EINVAL : 0;
>>> +}
>>> +
>>> +static int stm32_adc_set_trigger_state(struct iio_trigger *trig,
>>> + bool state)
>>> +{
>>> + struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
>>> + struct stm32_adc *adc = iio_priv(indio_dev);
>>> + int ret;
>>> +
>>> + if (state) {
>>> + /* Reset adc buffer index */
>>> + adc->bufi = 0;
>>> +
>>> + /* Allocate adc buffer */
>>> + adc->buffer = kzalloc(indio_dev->scan_bytes, GFP_KERNEL);
>> I'd be more cynical. It's not that big a memory allocation at worst.
>> Just put a big enough buffer in your adc structure and don't bother doing
>> it dynamically.
>>
>> If you didn't want to do it, it should be in the preenable callback rather
>> than the trigger state one (for semantic reasons rather than because it's a
>> bug)
> I'll fix this.
>>> + if (!adc->buffer)
>>> + return -ENOMEM;
>>> +
>>> + ret = stm32_adc_set_trig(indio_dev, trig);
>>> + if (ret) {
>>> + dev_err(&indio_dev->dev, "Can't set trigger\n");
>>> + goto err_buffer_free;
>>> + }
>>> +
>>> + stm32_adc_conv_irq_enable(adc);
>>> +
>>> + ret = stm32_adc_start_conv(adc);
>>> + if (ret) {
>>> + dev_err(&indio_dev->dev, "Failed to start\n");
>>> + goto err_irq_trig_disable;
>>> + }
>>> + } else {
>>> + ret = stm32_adc_stop_conv(adc);
>>> + if (ret < 0) {
>>> + dev_err(&indio_dev->dev, "Failed to stop\n");
>>> + return ret;
>>> + }
>>> +
>>> + stm32_adc_conv_irq_disable(adc);
>>> +
>>> + ret = stm32_adc_set_trig(indio_dev, NULL);
>>> + if (ret)
>>> + dev_warn(&indio_dev->dev, "Can't clear trigger\n");
>>> +
>>> + kfree(adc->buffer);
>>> + adc->buffer = NULL;
>>> + }
>>> +
>>> + return 0;
>>> +
>>> +err_irq_trig_disable:
>>> + stm32_adc_conv_irq_disable(adc);
>>> + stm32_adc_set_trig(indio_dev, NULL);
>>> +
>>> +err_buffer_free:
>>> + kfree(adc->buffer);
>>> + adc->buffer = NULL;
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static const struct iio_trigger_ops stm32_adc_trigger_ops = {
>>> + .owner = THIS_MODULE,
>>> + .validate_device = stm32_adc_validate_device,
>>> + .set_trigger_state = stm32_adc_set_trigger_state,
>>> +};
>>> +
>>> +static irqreturn_t stm32_adc_trigger_handler(int irq, void *p)
>>> +{
>>> + struct iio_poll_func *pf = p;
>>> + struct iio_dev *indio_dev = pf->indio_dev;
>>> + struct stm32_adc *adc = iio_priv(indio_dev);
>>> +
>>> + dev_dbg(&indio_dev->dev, "%s bufi=%d\n", __func__, adc->bufi);
>>> +
>>> + /* reset buffer index */
>>> + adc->bufi = 0;
>>> + iio_push_to_buffers_with_timestamp(indio_dev, adc->buffer,
>>> + pf->timestamp);
>>> +
>>> + iio_trigger_notify_done(indio_dev->trig);
>>> +
>>> + /* re-enable eoc irq */
>>> + stm32_adc_conv_irq_enable(adc);
>>> +
>>> + return IRQ_HANDLED;
>>> +}
>>> +
>>> +static void stm32_adc_trig_unregister(struct iio_dev *indio_dev)
>>> +{
>>> + struct stm32_adc *adc = iio_priv(indio_dev);
>>> + struct iio_trigger *trig, *_t;
>>> +
>>> + list_for_each_entry_safe(trig, _t, &adc->extrig_list, alloc_list) {
>>> + iio_trigger_unregister(trig);
>>> + list_del(&trig->alloc_list);
>>> + }
>>> +}
>>> +
>> I'd like a bit of documentation on this and a few of the other more
>> complex functions. Here it wasn't immediately obvious to me that it
>> was registering a large set of triggers. Also, silly question but
>> do you have any means of controlling the various timer setups from userspace?
> Sorry about this, I'll try to comment about trigger list to make it more obvious.
> There is no mean to setup timers via userspace, yet...
> I can remove them from the list for now, until this is supported.
> BTW I have some questions on trigger...
>
>>
>> There have been numerous discussions over the years on having a generic
>> timer subsystem, but if anything got written it passed me by. I have a couple
>> of boards where it would be handy but never had the time to do more than
>> talk about it ;)
>
> This is interesting... I'd be glad to hear more about it. Can you
> point some discussions if you have it in mind?
Err. Not sure I'll actually find an email thread on this. I can find
a reference to earlier discussions on it in the original IIO submission
over 8 years ago... So it was before we had an IIO list which means
it must have been on lkml.
As far as I know, no one ever took this futher though...
>
> In this driver, validate_trigger routine enforces that only triggers
> allocated for current indio_dev can be used. What if all timer
> triggers are put in a separate driver ? (e.g. like hrtimer in
> drivers/iio/trigger/) ? Purpose would be to tune 'sampling_frequency'
> and so on, on similar model, and have it configured basically when
> using it (e.g. cat trigger/name>trigger/current_trigger.).
I think we'd be closer to having a timer subsystem offer some
'services' to the drivers and then have your driver make use of those.
As the timer is hard wired to the actual adc timing here (rather than
a signal passing through kernel space like the high resolution timer
triggers are) I think the ADC driver will need to know about it
directly in some fashion.
>
> Is it a viable option, not to declare timer triggers in stm32-adc.c,
> but use pre-defined list of triggers, and separate trigger driver ?
> I'm thinking then, of simple string based list... But maybe you
> already though about this king of things ?
A simple string based identification might be prone to problems as
any driver could define it's own naming.
So I think the triggers need to be supplied by stm32-adc.c as the
driver needs to know about the 'hard wired' nature of which timers
can be used. The underlying handling of timer configuring etc
might be provided by a separate 'provider' module - similar to we
do for clock sources or regulators for example. Both of these
have the same characteristic of being separate 'hardware blocks'
that can be connected to lots of things, but are in this case
directly feeding the devices using them.
(we do this with IIO consumers too but that is a different game
and not relevant here + harder to follow than simple regs and
clocks).
>
> Please kindly share you view on this.
It's a non trivial job, but if you ultimately want to be able
to use those periodic timers for multiple possible purposes then
you'll need to do a fair bit of the work towards a generic
subsystem for timers anyway (callbacks etc).
Whether it is worth supporting the more 'soft' connected
equivalents (blackfin timer which is in staging as an IIO trigger
or the pxa271 periodic timer driver I wrote years ago - which
cheated and created a pile of RTCs to expose the interface)
is unclear. A lot of those usecases are well handled by
the High Resolution Timer trigger.
Jonathan
>
> Thanks again for your review.
> Best Regards,
> Fabrice
>
>>> +static int stm32_adc_trig_register(struct iio_dev *indio_dev)
>>> +{
>>> + struct stm32_adc *adc = iio_priv(indio_dev);
>>> + struct stm32_adc_common *common = adc->common;
>>> + const struct stm32_adc_trig_info *ext = common->data->ext_triggers;
>>> + struct iio_trigger *trig;
>>> + int i, ret = 0;
>>> +
>>> + if (adc->injected)
>>> + ext = common->data->jext_triggers;
>>> + else
>>> + ext = common->data->ext_triggers;
>>> +
>>> + for (i = 0; ext && ext[i].name; i++) {
>>> + trig = devm_iio_trigger_alloc(common->dev, "%s_%s%d_%s",
>>> + indio_dev->name,
>>> + adc->injected ? "jext" : "ext",
>>> + ext[i].extsel, ext[i].name);
>>> + if (!trig) {
>>> + dev_err(common->dev, "trig %s_%s%d_%s alloc failed\n",
>>> + indio_dev->name,
>>> + adc->injected ? "jext" : "ext",
>>> + ext[i].extsel, ext[i].name);
>>> + ret = -ENOMEM;
>>> + goto err;
>>> + }
>>> +
>>> + trig->dev.parent = common->dev;
>>> + trig->ops = &stm32_adc_trigger_ops;
>>> + iio_trigger_set_drvdata(trig, indio_dev);
>>> +
>>> + ret = iio_trigger_register(trig);
>>> + if (ret) {
>>> + dev_err(common->dev,
>>> + "trig %s_%s%d_%s register failed\n",
>>> + indio_dev->name,
>>> + adc->injected ? "jext" : "ext",
>>> + ext[i].extsel, ext[i].name);
>>> + goto err;
>>> + }
>>> +
>>> + list_add_tail(&trig->alloc_list, &adc->extrig_list);
>>> + }
>>> +
>>> + return 0;
>>> +err:
>>> + stm32_adc_trig_unregister(indio_dev);
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
>>> + struct iio_chan_spec *chan,
>>> + const struct stm32_adc_chan_spec *channel,
>>> + int scan_index)
>>> +{
>>> + struct stm32_adc *adc = iio_priv(indio_dev);
>>> +
>>> + chan->type = channel->type;
>>> + chan->channel = channel->channel;
>>> + chan->datasheet_name = channel->name;
>>> + chan->extend_name = channel->name;
>>> + chan->scan_index = scan_index;
>>> + chan->indexed = 1;
>>> + chan->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
>>> + chan->info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE);
>>> + chan->scan_type.sign = 'u';
>>> + chan->scan_type.realbits = adc->common->data->highres;
>>> + chan->scan_type.storagebits = STM32_STORAGEBITS;
>> This is one of those cases where actually I'd argue just having the number
>> here and not under a define would be clearer! So just put 16 here.
>>> + chan->scan_type.shift = 0;
>> Should be unneeded. Shift of 0 is the obvious default so no info provided
>> to readers of the code either really.
> I'll fix this
>>> +}
>>> +
>>> +static int stm32_adc_chan_of_init(struct iio_dev *indio_dev,
>>> + const struct stm32_adc_info *adc_info)
>>> +{
>>> + struct stm32_adc *adc = iio_priv(indio_dev);
>>> + struct device_node *node = indio_dev->dev.of_node;
>>> + struct property *prop;
>>> + const __be32 *cur;
>>> + struct iio_chan_spec *channels;
>>> + int scan_index = 0, num_channels = 0;
>>> + u32 val;
>>> +
>>> + of_property_for_each_u32(node, "st,adc-channels", prop, cur, val)
>>> + num_channels++;
>>> +
>>> + channels = devm_kcalloc(&indio_dev->dev, num_channels,
>>> + sizeof(struct iio_chan_spec), GFP_KERNEL);
>>> + if (!channels)
>>> + return -ENOMEM;
>>> +
>>> + of_property_for_each_u32(node, "st,adc-channels", prop, cur, val) {
>>> + stm32_adc_chan_init_one(indio_dev, &channels[scan_index],
>>> + &adc_info->channels[val],
>>> + scan_index);
>>> + scan_index++;
>>> + }
>>> +
>>> + adc->max_channels = adc_info->max_channels;
>>> + indio_dev->num_channels = scan_index;
>>> + indio_dev->channels = channels;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int stm32_adc_register(struct stm32_adc_common *common,
>>> + struct device_node *child)
>>> +{
>>> + struct iio_dev *indio_dev;
>>> + struct stm32_adc *adc;
>>> + int i, ret;
>>> + u32 reg;
>>> +
>>> + ret = of_property_read_u32(child, "reg", ®);
>>> + if (ret != 0) {
>>> + dev_err(common->dev, "missing reg property\n");
>>> + return -EINVAL;
>>> + }
>>> +
>>> + for (i = 0; common->data->adc_info[i].channels; i++)
>>> + if (common->data->adc_info[i].reg == reg)
>>> + break;
>>> +
>>> + if (i >= STM32_ADC_ID_MAX || !common->data->adc_info[i].channels) {
>>> + dev_err(common->dev, "bad adc reg offset\n");
>>> + return -ENOENT;
>>> + }
>>> +
>>> + indio_dev = devm_iio_device_alloc(common->dev, sizeof(*adc));
>>> + if (!indio_dev) {
>>> + dev_err(common->dev, "iio device allocation failed\n");
>>> + return -ENOMEM;
>>> + }
>>> +
>>> + adc = iio_priv(indio_dev);
>>> + adc->id = i;
>>> + adc->offset = reg;
>>> + adc->common = common;
>>> + INIT_LIST_HEAD(&adc->extrig_list);
>>> + spin_lock_init(&adc->lock);
>>> + init_completion(&adc->completion);
>>> +
>>> + if (child->name)
>>> + indio_dev->name = child->name;
>>> + else
>>> + indio_dev->name = common->data->adc_info[i].name;
>>> + indio_dev->dev.parent = common->dev;
>>> + indio_dev->dev.of_node = child;
>>> + indio_dev->info = &stm32_adc_iio_info;
>>> + indio_dev->modes = INDIO_DIRECT_MODE;
>>> +
>>> + if (of_property_read_bool(child, "st,injected")) {
>>> + dev_dbg(common->dev, "%s Configured to use injected\n",
>>> + indio_dev->name);
>>> + adc->injected = true;
>>> + }
>>> +
>>> + adc->clk = of_clk_get(child, 0);
>>> + if (IS_ERR(adc->clk)) {
>>> + adc->clk = NULL;
>>> + dev_dbg(common->dev, "No child clk found\n");
>>> + } else {
>>> + ret = clk_prepare_enable(adc->clk);
>>> + if (ret < 0)
>>> + goto err_clk_put;
>>> + }
>>> +
>>> + ret = stm32_adc_chan_of_init(indio_dev, &common->data->adc_info[i]);
>>> + if (ret < 0) {
>>> + dev_err(common->dev, "iio channels init failed\n");
>>> + goto err_clk_disable;
>>> + }
>>> +
>>> + ret = stm32_adc_trig_register(indio_dev);
>>> + if (ret)
>>> + goto err_clk_disable;
>>> +
>>> + ret = iio_triggered_buffer_setup(indio_dev,
>>> + &iio_pollfunc_store_time,
>>> + &stm32_adc_trigger_handler,
>>> + &iio_triggered_buffer_setup_ops);
>>> + if (ret) {
>>> + dev_err(common->dev, "buffer setup failed\n");
>>> + goto err_trig_unregister;
>>> + }
>>> +
>>> + ret = iio_device_register(indio_dev);
>>> + if (ret) {
>>> + dev_err(common->dev, "iio dev register failed\n");
>>> + goto err_buffer_cleanup;
>>> + }
>>> +
>>> + list_add_tail(&adc->adc_list, &common->adc_list);
>>> +
>>> + return 0;
>>> +
>>> +err_buffer_cleanup:
>>> + iio_triggered_buffer_cleanup(indio_dev);
>>> +
>>> +err_trig_unregister:
>>> + stm32_adc_trig_unregister(indio_dev);
>>> +
>>> +err_clk_disable:
>>> + if (adc->clk)
>>> + clk_disable_unprepare(adc->clk);
>>> +
>>> +err_clk_put:
>>> + if (adc->clk)
>>> + clk_put(adc->clk);
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static void stm32_adc_unregister(struct stm32_adc *adc)
>>> +{
>>> + struct iio_dev *indio_dev = iio_priv_to_dev(adc);
>>> +
>>> + iio_device_unregister(indio_dev);
>>> + iio_triggered_buffer_cleanup(indio_dev);
>>> + stm32_adc_trig_unregister(indio_dev);
>>> + if (adc->clk) {
>>> + clk_disable_unprepare(adc->clk);
>>> + clk_put(adc->clk);
>>> + }
>>> +}
>>> +
>>> +int stm32_adc_probe(struct platform_device *pdev)
>>> +{
>>> + struct device_node *np = pdev->dev.of_node, *child;
>>> + struct device *dev = &pdev->dev;
>>> + const struct of_device_id *match;
>>> + struct stm32_adc_common *common;
>>> + struct stm32_adc *adc;
>>> + struct resource *res;
>>> + int ret;
>>> +
>>> + match = of_match_device(dev->driver->of_match_table, &pdev->dev);
>>> + if (!match || !match->data) {
>>> + dev_err(&pdev->dev, "compatible data not provided\n");
>> How would we have instantiated this if there was not a suitable match?
>> As such what does this check give us? (confused!)
> I'll fix this.
>>> + return -EINVAL;
>>> + }
>>> +
>>> + common = devm_kzalloc(&pdev->dev, sizeof(*common), GFP_KERNEL);
>>> + if (!common)
>>> + return -ENOMEM;
>>> +
>>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> + common->base = devm_ioremap_resource(&pdev->dev, res);
>>> + if (IS_ERR(common->base))
>>> + return PTR_ERR(common->base);
>>> +
>>> + common->data = match->data;
>>> + common->dev = &pdev->dev;
>>> + platform_set_drvdata(pdev, common);
>>> + mutex_init(&common->lock);
>>> + INIT_LIST_HEAD(&common->adc_list);
>>> +
>>> + common->vref = devm_regulator_get(&pdev->dev, "vref");
>>> + if (IS_ERR(common->vref)) {
>>> + ret = PTR_ERR(common->vref);
>>> + dev_err(&pdev->dev, "vref get failed, %d\n", ret);
>>> + return ret;
>>> + }
>>> +
>>> + ret = regulator_enable(common->vref);
>>> + if (ret < 0) {
>>> + dev_err(&pdev->dev, "vref enable failed\n");
>>> + return ret;
>>> + }
>>> +
>>> + ret = regulator_get_voltage(common->vref);
>>> + if (ret < 0) {
>>> + dev_err(&pdev->dev, "vref get voltage failed, %d\n", ret);
>>> + goto err_regulator_disable;
>>> + }
>>> + common->vref_mv = ret / 1000;
>>> + dev_dbg(&pdev->dev, "vref+=%dmV\n", common->vref_mv);
>>> +
>>> + common->aclk = devm_clk_get(&pdev->dev, "adc");
>>> + if (IS_ERR(common->aclk)) {
>>> + ret = PTR_ERR(common->aclk);
>>> + dev_err(&pdev->dev, "Can't get 'adc' clock\n");
>>> + goto err_regulator_disable;
>>> + }
>>> +
>>> + ret = clk_prepare_enable(common->aclk);
>>> + if (ret < 0) {
>>> + dev_err(common->dev, "adc clk enable failed\n");
>>> + goto err_regulator_disable;
>>> + }
>>> +
>>> + common->irq = platform_get_irq(pdev, 0);
>>> + if (common->irq < 0) {
>>> + dev_err(&pdev->dev, "failed to get irq\n");
>>> + ret = common->irq;
>>> + goto err_clk_disable;
>>> + }
>>> +
>>> + ret = devm_request_irq(&pdev->dev, common->irq, stm32_adc_common_isr,
>>> + 0, pdev->name, common);
>>> + if (ret) {
>>> + dev_err(&pdev->dev, "failed to request irq\n");
>>> + goto err_clk_disable;
>>> + }
>>> +
>>> + /* Parse adc child nodes to retrieve master/slave instances data */
>>> + for_each_available_child_of_node(np, child) {
>>> + ret = stm32_adc_register(common, child);
>>> + if (ret)
>>> + goto err_unregister;
>>> + }
>>> +
>>> + dev_info(&pdev->dev, "registered\n");
>> No benefit in this info being provided (it's obvious, device just turned up
>> in sysfs :) So drop it.
> I'll fix this.
>>> +
>>> + return 0;
>>> +
>>> +err_unregister:
>>> + list_for_each_entry(adc, &common->adc_list, adc_list)
>>> + stm32_adc_unregister(adc);
>>> +
>>> +err_clk_disable:
>>> + clk_disable_unprepare(common->aclk);
>>> +
>>> +err_regulator_disable:
>>> + regulator_disable(common->vref);
>>> +
>>> + return ret;
>>> +}
>>> +EXPORT_SYMBOL_GPL(stm32_adc_probe);
>>> +
>>> +int stm32_adc_remove(struct platform_device *pdev)
>>> +{
>>> + struct stm32_adc_common *common = platform_get_drvdata(pdev);
>>> + struct stm32_adc *adc;
>>> +
>>> + list_for_each_entry(adc, &common->adc_list, adc_list)
>>> + stm32_adc_unregister(adc);
>>> + clk_disable_unprepare(common->aclk);
>>> + regulator_disable(common->vref);
>>> +
>>> + return 0;
>>> +}
>>> +EXPORT_SYMBOL_GPL(stm32_adc_remove);
>>> +
>>> +MODULE_AUTHOR("Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>");
>>> +MODULE_DESCRIPTION("STMicroelectronics STM32 ADC driver");
>>> +MODULE_LICENSE("GPL v2");
>>> diff --git a/drivers/iio/adc/stm32/stm32-adc.h b/drivers/iio/adc/stm32/stm32-adc.h
>>> new file mode 100644
>>> index 0000000..0be603c
>>> --- /dev/null
>>> +++ b/drivers/iio/adc/stm32/stm32-adc.h
>>> @@ -0,0 +1,442 @@
>>> +/*
>>> + * This file is part of STM32 ADC driver
>>> + *
>>> + * Copyright (C) 2016, STMicroelectronics - All Rights Reserved
>>> + * Author: Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>.
>>> + *
>>> + * License type: GPLv2
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify it
>>> + * under the terms of the GNU General Public License 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.
>>> + *
>>> + * You should have received a copy of the GNU General Public License along with
>>> + * this program. If not, see <http://www.gnu.org/licenses/>.
>>> + */
>>> +
>>> +#ifndef __STM32_ADC_H
>>> +#define __STM32_ADC_H
>>> +
>>> +/*
>>> + * STM32 - ADC global register map
>>> + * ________________________________________________________
>>> + * | Offset | Register |
>>> + * --------------------------------------------------------
>>> + * | 0x000 | Master ADC1 |
>>> + * --------------------------------------------------------
>>> + * | 0x100 | Slave ADC2 |
>>> + * --------------------------------------------------------
>>> + * | 0x200 | Slave ADC3 |
>>> + * --------------------------------------------------------
>>> + * | 0x300 | Master & Slave common regs |
>>> + * --------------------------------------------------------
>>> + */
>>> +#define STM32_ADCX_COMN_OFFSET 0x300
>>> +#define STM32_ADC_ID_MAX 3
>>> +#define STM32_ADC_MAX_SQ 16 /* SQ1..SQ16 */
>>> +#define STM32_ADC_MAX_JSQ 4 /* JSQ1..JSQ4 */
>>> +
>>> +/* STM32 value masks */
>>> +#define STM32_RESULT_MASK GENMASK(15, 0)
>>> +#define STM32_STORAGEBITS 16
>>> +
>>> +/* External trigger enable for regular or injected channels (exten/jexten) */
>>> +enum stm32_adc_exten {
>>> + STM32_EXTEN_SWTRIG,
>>> + STM32_EXTEN_HWTRIG_RISING_EDGE,
>>> + STM32_EXTEN_HWTRIG_FALLING_EDGE,
>>> + STM32_EXTEN_HWTRIG_BOTH_EDGES,
>>> +};
>>> +
>>> +enum stm32_adc_extsel {
>>> + STM32_EXT0,
>>> + STM32_EXT1,
>>> + STM32_EXT2,
>>> + STM32_EXT3,
>>> + STM32_EXT4,
>>> + STM32_EXT5,
>>> + STM32_EXT6,
>>> + STM32_EXT7,
>>> + STM32_EXT8,
>>> + STM32_EXT9,
>>> + STM32_EXT10,
>>> + STM32_EXT11,
>>> + STM32_EXT12,
>>> + STM32_EXT13,
>>> + STM32_EXT14,
>>> + STM32_EXT15,
>>> + STM32_EXT16,
>>> + STM32_EXT17,
>>> + STM32_EXT18,
>>> + STM32_EXT19,
>>> + STM32_EXT20,
>>> + STM32_EXT21,
>>> + STM32_EXT22,
>>> + STM32_EXT23,
>>> + STM32_EXT24,
>>> + STM32_EXT25,
>>> + STM32_EXT26,
>>> + STM32_EXT27,
>>> + STM32_EXT28,
>>> + STM32_EXT29,
>>> + STM32_EXT30,
>>> + STM32_EXT31,
>>> +};
>>> +
>>> +enum stm32_adc_jextsel {
>>> + STM32_JEXT0,
>>> + STM32_JEXT1,
>>> + STM32_JEXT2,
>>> + STM32_JEXT3,
>>> + STM32_JEXT4,
>>> + STM32_JEXT5,
>>> + STM32_JEXT6,
>>> + STM32_JEXT7,
>>> + STM32_JEXT8,
>>> + STM32_JEXT9,
>>> + STM32_JEXT10,
>>> + STM32_JEXT11,
>>> + STM32_JEXT12,
>>> + STM32_JEXT13,
>>> + STM32_JEXT14,
>>> + STM32_JEXT15,
>>> + STM32_JEXT16,
>>> + STM32_JEXT17,
>>> + STM32_JEXT18,
>>> + STM32_JEXT19,
>>> + STM32_JEXT20,
>>> + STM32_JEXT21,
>>> + STM32_JEXT22,
>>> + STM32_JEXT23,
>>> + STM32_JEXT24,
>>> + STM32_JEXT25,
>>> + STM32_JEXT26,
>>> + STM32_JEXT27,
>>> + STM32_JEXT28,
>>> + STM32_JEXT29,
>>> + STM32_JEXT30,
>>> + STM32_JEXT31,
>>> +};
>>> +
>>> +#define STM32_ADC_TIMEOUT_US 100000
>>> +#define STM32_ADC_TIMEOUT (msecs_to_jiffies(STM32_ADC_TIMEOUT_US / 1000))
>>> +
>>> +/**
>>> + * struct stm32_adc_chan_spec - specification of stm32 adc channel
>>> + * @type: IIO channel type
>>> + * @channel: channel number (single ended)
>>> + * @name: channel name (single ended)
>>> + */
>>> +struct stm32_adc_chan_spec {
>>> + enum iio_chan_type type;
>>> + int channel;
>>> + const char *name;
>>> +};
>>> +
>>> +/**
>>> + * struct stm32_adc_trig_info - ADC trigger info
>>> + * @extsel: trigger selection for regular or injected
>>> + * @name: name of the trigger, corresponding to its source
>>> + */
>>> +struct stm32_adc_trig_info {
>>> + u32 extsel;
>>> + const char *name;
>>> +};
>>> +
>>> +/**
>>> + * struct stm32_adc_info - stm32 ADC, per instance config data
>>> + * @name: default name for this instance (like "adc1")
>>> + * @reg: reg offset for this instance (e.g. 0x0 for adc1...)
>>> + * @channels: Reference to stm32 channels spec
>>> + * @max_channels: Number of single ended channels
>>> + */
>>> +struct stm32_adc_info {
>>> + const char *name;
>>> + u32 reg;
>>> + const struct stm32_adc_chan_spec *channels;
>>> + int max_channels;
>>> +};
>>> +
>>> +/**
>>> + * stm32_adc_regs - stm32 ADC misc registers & bitfield desc
>>> + * @reg: register offset
>>> + * @mask: bitfield mask
>>> + * @shift: left shift
>>> + */
>>> +struct stm32_adc_regs {
>>> + int reg;
>>> + int mask;
>>> + int shift;
>>> +};
>>> +
>>> +/**
>>> + * stm32_adc_trig_reginfo - stm32 ADC trigger control registers description
>>> + * @reg: trigger control register offset (exten/jexten)
>>> + * @exten_mask: external trigger en/polarity mask in @reg
>>> + * @exten_shift: external trigger en/polarity shift in @reg
>>> + * @extsel_mask: external trigger source mask in @reg
>>> + * @extsel_shift: external trigger source shift in @reg
>>> + */
>>> +struct stm32_adc_trig_reginfo {
>>> + u32 reg;
>>> + u32 exten_mask;
>>> + u32 exten_shift;
>>> + u32 extsel_mask;
>>> + u32 extsel_shift;
>>> +};
>>> +
>>> +/**
>>> + * struct stm32_adc_reginfo - stm32 ADC registers description
>>> + * @isr: interrupt status register offset
>>> + * @eoc: end of conversion mask in @isr
>>> + * @jeoc: end of injected conversion sequence mask in @isr
>>> + * @ier: interrupt enable register offset
>>> + * @eocie: end of conversion interrupt enable mask in @ier
>>> + * @jeocie: end of injected conversion sequence interrupt en mask
>>> + * @dr: data register offset
>>> + * @jdr: injected data registers offsets
>>> + * @sqr_regs: Regular sequence registers description
>>> + * @jsqr_reg: Injected sequence register description
>>> + * @trig_reginfo: regular trigger control registers description
>>> + * @jtrig_reginfo: injected trigger control registers description
>>> + */
>>> +struct stm32_adc_reginfo {
>>> + u32 isr;
>>> + u32 eoc;
>>> + u32 jeoc;
>>> + u32 ier;
>>> + u32 eocie;
>>> + u32 jeocie;
>>> + u32 dr;
>>> + u32 jdr[4];
>>> + const struct stm32_adc_regs *sqr_regs;
>>> + const struct stm32_adc_regs *jsqr_reg;
>>> + const struct stm32_adc_trig_reginfo *trig_reginfo;
>>> + const struct stm32_adc_trig_reginfo *jtrig_reginfo;
>>> +};
>>> +
>>> +struct stm32_adc;
>>> +
>>> +/**
>>> + * struct stm32_adc_ops - stm32 ADC, compatible dependent data
>>> + * - stm32 ADC may work as single ADC, or as tightly coupled master/slave ADCs.
>>> + *
>>> + * @adc_info: Array spec for stm32 adc master/slaves instances
>>> + * @ext_triggers: Reference to trigger info for regular channels
>>> + * @jext_triggers: Reference to trigger info for injected channels
>>> + * @adc_reginfo: stm32 ADC registers description
>>> + * @highres: Max resolution
>>> + * @max_clock_rate: Max input clock rate
>>> + * @clk_sel: routine to select common clock and prescaler
>>> + * @start_conv: routine to start conversions
>>> + * @stop_conv: routine to stop conversions
>>> + * @is_started: routine to get adc 'started' state
>>> + * @regular_started routine to check regular conversions status
>>> + * @injected_started routine to check injected conversions status
>>> + * @enable: optional routine to enable stm32 adc
>>> + * @disable: optional routine to disable stm32 adc
>>> + * @is_enabled reports enabled state
>>> + */
>> This is a big chunk of abstraction that seems excessive at the moment.
>> I'd rather see it introduced only just before it's actually used..
>> (I'm guessing it's intended for support of similar parts?)
>>
>> Right now it just makes the driver harder to review.
>>> +struct stm32_adc_ops {
>>> + const struct stm32_adc_info *adc_info;
>>> + const struct stm32_adc_trig_info *ext_triggers;
>>> + const struct stm32_adc_trig_info *jext_triggers;
>>> + const struct stm32_adc_reginfo *adc_reginfo;
>>> + int highres;
>>> + unsigned long max_clock_rate;
>>> + int (*clk_sel)(struct stm32_adc *adc);
>>> + int (*start_conv)(struct stm32_adc *adc);
>>> + int (*stop_conv)(struct stm32_adc *adc);
>>> + bool (*is_started)(struct stm32_adc *adc);
>>> + bool (*regular_started)(struct stm32_adc *adc);
>>> + bool (*injected_started)(struct stm32_adc *adc);
>>> + int (*enable)(struct stm32_adc *adc);
>>> + void (*disable)(struct stm32_adc *adc);
>>> + bool (*is_enabled)(struct stm32_adc *adc);
>>> +};
>>> +
>>> +struct stm32_adc_common;
>>> +
>>> +/**
>>> + * struct stm32_adc - private data of each ADC IIO instance
>>> + * @common: reference to ADC block common data
>>> + * @adc_list: current ADC entry in common ADC list
>>> + * @id: ADC instance number (e.g. adc 1, 2 or 3)
>>> + * @offset: ADC instance register offset in ADC block
>>> + * @max_channels: Max channels number for this ADC.
>>> + * @extrig_list: External trigger list (for regular channel)
>>> + * @completion: end of single conversion completion
>>> + * @buffer: data buffer
>>> + * @bufi: data buffer index
>>> + * @num_conv: expected number of scan conversions
>>> + * @injected: use injected channels on this adc
>>> + * @lock: spinlock
>>> + * @clk: optional adc clock, for this adc instance
>>> + * @calib: optional calibration data
>>> + * @en: emulates enabled state on some stm32 adc
>>> + */
>>> +struct stm32_adc {
>>> + struct stm32_adc_common *common;
>>> + struct list_head adc_list;
>>> + int id;
>>> + int offset;
>>> + int max_channels;
>>> + struct list_head extrig_list;
>>> + struct completion completion;
>>> + u16 *buffer;
>>> + int bufi;
>>> + int num_conv;
>>> + bool injected;
>>> + spinlock_t lock; /* interrupt lock */
>>> + struct clk *clk;
>>> + void *calib;
>>> + bool en;
>>> +};
>>> +
>>> +/**
>>> + * struct stm32_adc_common - private data of ADC driver, common to all
>>> + * ADC instances (ADC block)
>>> + * @dev: device for this controller
>>> + * @base: control registers base cpu addr
>>> + * @irq: Common irq line for all adc instances
>>> + * @data: STM32 dependent data from compatible
>>> + * @adc_list: list of all stm32 ADC in this ADC block
>>> + * @aclk: common clock for the analog circuitry
>>> + * @vref: regulator reference
>>> + * @vref_mv: vref voltage (mv)
>>> + * @lock: mutex
>>> + */
>>> +struct stm32_adc_common {
>>> + struct device *dev;
>>> + void __iomem *base;
>>> + int irq;
>>> + const struct stm32_adc_ops *data;
>>> + struct list_head adc_list;
>>> + struct clk *aclk;
>>> + struct regulator *vref;
>>> + int vref_mv;
>>> + struct mutex lock; /* read_raw lock */
>>> +};
>>> +
>>> +/* Helper routines */
>>> +static inline int stm32_adc_start_conv(struct stm32_adc *adc)
>>> +{
>>> + return adc->common->data->start_conv(adc);
>>> +}
>>> +
>>> +static inline int stm32_adc_stop_conv(struct stm32_adc *adc)
>>> +{
>>> + return adc->common->data->stop_conv(adc);
>>> +}
>>> +
>>> +static inline bool stm32_adc_is_started(struct stm32_adc *adc)
>>> +{
>>> + return adc->common->data->is_started(adc);
>>> +}
>>> +
>>> +static inline bool stm32_adc_regular_started(struct stm32_adc *adc)
>>> +{
>>> + return adc->common->data->regular_started(adc);
>>> +}
>>> +
>>> +static inline bool stm32_adc_injected_started(struct stm32_adc *adc)
>>> +{
>>> + return adc->common->data->injected_started(adc);
>>> +}
>>> +
>>> +static inline bool stm32_adc_clk_sel(struct stm32_adc *adc)
>>> +{
>>> + return adc->common->data->clk_sel(adc);
>>> +}
>>> +
>>> +static inline int stm32_adc_enable(struct stm32_adc *adc)
>>> +{
>>> + if (adc->common->data->enable)
>>> + return adc->common->data->enable(adc);
>>> +
>>> + adc->en = true;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static inline bool stm32_adc_is_enabled(struct stm32_adc *adc)
>>> +{
>>> + if (adc->common->data->is_enabled)
>>> + return adc->common->data->is_enabled(adc);
>>> + else
>>> + return adc->en;
>>> +}
>>> +
>>> +static inline void stm32_adc_disable(struct stm32_adc *adc)
>>> +{
>>> + /* Check there is no regular or injected on-going conversions */
>>> + if (stm32_adc_is_started(adc))
>>> + return;
>>> +
>>> + if (adc->common->data->disable)
>>> + adc->common->data->disable(adc);
>>> + else
>>> + adc->en = false;
>>> +}
>>> +
>>> +/* STM32 ADC registers access routines */
>>> +static inline u32 stm32_adc_common_readl(struct stm32_adc_common *com, u32 reg)
>>> +{
>>> + u32 val = readl_relaxed(com->base + reg);
>>> +
>>> + return val;
>>> +}
>>> +
>>> +static inline void stm32_adc_common_writel(struct stm32_adc_common *com,
>>> + u32 reg, u32 val)
>>> +{
>>> + writel_relaxed(val, com->base + reg);
>>> +}
>>> +
>>> +static inline u32 stm32_adc_readl(struct stm32_adc *adc, u32 reg)
>>> +{
>>> + u32 val = readl_relaxed(adc->common->base + adc->offset + reg);
>>> +
>>> + return val;
>>> +}
>>> +
>>> +#define stm32_adc_readl_addr(addr) stm32_adc_readl(adc, addr)
>>> +
>>> +#define stm32_adc_readl_poll_timeout(reg, val, cond, sleep_us, timeout_us) \
>>> + readx_poll_timeout(stm32_adc_readl_addr, reg, val, \
>>> + cond, sleep_us, timeout_us)
>>> +
>>> +static inline void stm32_adc_writel(struct stm32_adc *adc, u32 reg, u32 val)
>>> +{
>>> + writel_relaxed(val, adc->common->base + adc->offset + reg);
>>> +}
>>> +
>>> +static inline void stm32_adc_set_bits(struct stm32_adc *adc, u32 reg, u32 bits)
>>> +{
>>> + unsigned long flags;
>>> +
>>> + spin_lock_irqsave(&adc->lock, flags);
>>> + stm32_adc_writel(adc, reg, stm32_adc_readl(adc, reg) | bits);
>>> + spin_unlock_irqrestore(&adc->lock, flags);
>>> +}
>>> +
>>> +static inline void stm32_adc_clr_bits(struct stm32_adc *adc, u32 reg, u32 bits)
>>> +{
>>> + unsigned long flags;
>>> +
>>> + spin_lock_irqsave(&adc->lock, flags);
>>> + stm32_adc_writel(adc, reg, stm32_adc_readl(adc, reg) & ~bits);
>>> + spin_unlock_irqrestore(&adc->lock, flags);
>>> +}
>>> +
>>> +/* STM32 common extended attributes */
>>> +extern const struct iio_enum stm32_adc_trig_pol;
>>> +int stm32_adc_probe(struct platform_device *pdev);
>>> +int stm32_adc_remove(struct platform_device *pdev);
>>> +
>>> +#endif
>>> diff --git a/drivers/iio/adc/stm32/stm32f4-adc.c b/drivers/iio/adc/stm32/stm32f4-adc.c
>>> new file mode 100644
>>> index 0000000..147fe9c
>>> --- /dev/null
>>> +++ b/drivers/iio/adc/stm32/stm32f4-adc.c
>>> @@ -0,0 +1,574 @@
>>> +/*
>>> + * This file is part of STM32F4 ADC driver
>>> + *
>>> + * Copyright (C) 2016, STMicroelectronics - All Rights Reserved
>>> + * Author: Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>.
>>> + *
>>> + * License type: GPLv2
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify it
>>> + * under the terms of the GNU General Public License 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.
>>> + *
>>> + * You should have received a copy of the GNU General Public License along with
>>> + * this program. If not, see <http://www.gnu.org/licenses/>.
>>> + */
>>> +
>>> +#include <linux/clk.h>
>>> +#include <linux/delay.h>
>>> +#include <linux/iio/iio.h>
>>> +#include <linux/iio/trigger.h>
>>> +#include <linux/platform_device.h>
>>> +#include "stm32-adc.h"
>>> +
>>> +/*
>>> + * STM32F4 - ADC global register map
>>> + * ________________________________________________________
>>> + * | Offset | Register |
>>> + * --------------------------------------------------------
>>> + * | 0x000 | Master ADC1 |
>>> + * --------------------------------------------------------
>>> + * | 0x100 | Slave ADC2 |
>>> + * --------------------------------------------------------
>>> + * | 0x200 | Slave ADC3 |
>>> + * --------------------------------------------------------
>>> + * | 0x300 | Master & Slave common regs |
>>> + * --------------------------------------------------------
>>> + */
>>> +
>>> +/* STM32F4 - Registers for each ADC instance */
>>> +#define STM32F4_ADCX_SR 0x00
>>> +#define STM32F4_ADCX_CR1 0x04
>>> +#define STM32F4_ADCX_CR2 0x08
>>> +#define STM32F4_ADCX_SMPR1 0x0C
>>> +#define STM32F4_ADCX_SMPR2 0x10
>>> +#define STM32F4_ADCX_HTR 0x24
>>> +#define STM32F4_ADCX_LTR 0x28
>>> +#define STM32F4_ADCX_SQR1 0x2C
>>> +#define STM32F4_ADCX_SQR2 0x30
>>> +#define STM32F4_ADCX_SQR3 0x34
>>> +#define STM32F4_ADCX_JSQR 0x38
>>> +#define STM32F4_ADCX_JDR1 0x3C
>>> +#define STM32F4_ADCX_JDR2 0x40
>>> +#define STM32F4_ADCX_JDR3 0x44
>>> +#define STM32F4_ADCX_JDR4 0x48
>>> +#define STM32F4_ADCX_DR 0x4C
>>> +
>>> +/* STM32 - Master & slave registers (common for all instances: 1, 2 & 3) */
>>> +#define STM32F4_ADC_CSR (STM32_ADCX_COMN_OFFSET + 0x00)
>>> +#define STM32F4_ADC_CCR (STM32_ADCX_COMN_OFFSET + 0x04)
>>> +#define STM32F4_ADC_CDR (STM32_ADCX_COMN_OFFSET + 0x08)
>>> +
>>> +/* STM32F4_ADCX_SR - bit fields */
>>> +#define STM32F4_OVR BIT(5)
>>> +#define STM32F4_STRT BIT(4)
>>> +#define STM32F4_JSTRT BIT(3)
>>> +#define STM32F4_JEOC BIT(2)
>>> +#define STM32F4_EOC BIT(1)
>>> +#define STM32F4_AWD BIT(0)
>>> +
>>> +/* STM32F4_ADCX_CR1 - bit fields */
>>> +#define STM32F4_OVRIE BIT(26)
>>> +#define STM32F4_RES_SHIFT 24
>>> +#define STM32F4_RES_MASK GENMASK(25, 24)
>>> +#define STM32F4_AWDEN BIT(23)
>>> +#define STM32F4_JAWDEN BIT(22)
>>> +#define STM32F4_DISCNUM_SHIFT 13
>>> +#define STM32F4_DISCNUM_MASK GENMASK(15, 13)
>>> +#define STM32F4_JDISCEN BIT(12)
>>> +#define STM32F4_DISCEN BIT(11)
>>> +#define STM32F4_JAUTO BIT(10)
>>> +#define STM32F4_AWDSGL BIT(9)
>>> +#define STM32F4_SCAN BIT(8)
>>> +#define STM32F4_JEOCIE BIT(7)
>>> +#define STM32F4_AWDIE BIT(6)
>>> +#define STM32F4_EOCIE BIT(5)
>>> +#define STM32F4_AWDCH_SHIFT 0
>>> +#define STM32F4_AWDCH_MASK GENMASK(4, 0)
>>> +
>>> +/* STM32F4_ADCX_CR2 - bit fields */
>>> +#define STM32F4_SWSTART BIT(30)
>>> +#define STM32F4_EXTEN_SHIFT 28
>>> +#define STM32F4_EXTEN_MASK GENMASK(29, 28)
>>> +#define STM32F4_EXTSEL_SHIFT 24
>>> +#define STM32F4_EXTSEL_MASK GENMASK(27, 24)
>>> +#define STM32F4_JSWSTART BIT(22)
>>> +#define STM32F4_JEXTEN_SHIFT 20
>>> +#define STM32F4_JEXTEN_MASK GENMASK(21, 20)
>>> +#define STM32F4_JEXTSEL_SHIFT 16
>>> +#define STM32F4_JEXTSEL_MASK GENMASK(19, 16)
>>> +#define STM32F4_ALIGN BIT(11)
>>> +#define STM32F4_EOCS BIT(10)
>>> +#define STM32F4_DDS BIT(9)
>>> +#define STM32F4_DMA BIT(8)
>>> +#define STM32F4_CONT BIT(1)
>>> +#define STM32F4_ADON BIT(0)
>>> +
>>> +/* STM32F4_ADCX_SMPR1 - bit fields */
>>> +#define STM32F4_SMP18_SHIFT 24
>>> +#define STM32F4_SMP18_MASK GENMASK(26, 24)
>>> +#define STM32F4_SMP17_SHIFT 21
>>> +#define STM32F4_SMP17_MASK GENMASK(23, 21)
>>> +#define STM32F4_SMP16_SHIFT 18
>>> +#define STM32F4_SMP16_MASK GENMASK(20, 18)
>>> +#define STM32F4_SMP15_SHIFT 15
>>> +#define STM32F4_SMP15_MASK GENMASK(17, 15)
>>> +#define STM32F4_SMP14_SHIFT 12
>>> +#define STM32F4_SMP14_MASK GENMASK(14, 12)
>>> +#define STM32F4_SMP13_SHIFT 9
>>> +#define STM32F4_SMP13_MASK GENMASK(11, 9)
>>> +#define STM32F4_SMP12_SHIFT 6
>>> +#define STM32F4_SMP12_MASK GENMASK(8, 6)
>>> +#define STM32F4_SMP11_SHIFT 3
>>> +#define STM32F4_SMP11_MASK GENMASK(5, 3)
>>> +#define STM32F4_SMP10_SHIFT 0
>>> +#define STM32F4_SMP10_MASK GENMASK(2, 0)
>>> +
>>> +/* STM32F4_ADCX_SMPR2 - bit fields */
>>> +#define STM32F4_SMP9_SHIFT 27
>>> +#define STM32F4_SMP9_MASK GENMASK(29, 27)
>>> +#define STM32F4_SMP8_SHIFT 24
>>> +#define STM32F4_SMP8_MASK GENMASK(26, 24)
>>> +#define STM32F4_SMP7_SHIFT 21
>>> +#define STM32F4_SMP7_MASK GENMASK(23, 21)
>>> +#define STM32F4_SMP6_SHIFT 18
>>> +#define STM32F4_SMP6_MASK GENMASK(20, 18)
>>> +#define STM32F4_SMP5_SHIFT 15
>>> +#define STM32F4_SMP5_MASK GENMASK(17, 15)
>>> +#define STM32F4_SMP4_SHIFT 12
>>> +#define STM32F4_SMP4_MASK GENMASK(14, 12)
>>> +#define STM32F4_SMP3_SHIFT 9
>>> +#define STM32F4_SMP3_MASK GENMASK(11, 9)
>>> +#define STM32F4_SMP2_SHIFT 6
>>> +#define STM32F4_SMP2_MASK GENMASK(8, 6)
>>> +#define STM32F4_SMP1_SHIFT 3
>>> +#define STM32F4_SMP1_MASK GENMASK(5, 3)
>>> +#define STM32F4_SMP0_SHIFT 0
>>> +#define STM32F4_SMP0_MASK GENMASK(2, 0)
>>> +enum stm32f4_adc_smpr {
>>> + STM32F4_SMPR_3_CK_CYCLES,
>>> + STM32F4_SMPR_15_CK_CYCLES,
>>> + STM32F4_SMPR_28_CK_CYCLES,
>>> + STM32F4_SMPR_56_CK_CYCLES,
>>> + STM32F4_SMPR_84_CK_CYCLES,
>>> + STM32F4_SMPR_112_CK_CYCLES,
>>> + STM32F4_SMPR_144_CK_CYCLES,
>>> + STM32F4_SMPR_480_CK_CYCLES,
>>> +};
>>> +
>>> +/* STM32F4_ADCX_SQR1 - bit fields */
>>> +#define STM32F4_L_SHIFT 20
>>> +#define STM32F4_L_MASK GENMASK(23, 20)
>>> +#define STM32F4_SQ16_SHIFT 15
>>> +#define STM32F4_SQ16_MASK GENMASK(19, 15)
>>> +#define STM32F4_SQ15_SHIFT 10
>>> +#define STM32F4_SQ15_MASK GENMASK(14, 10)
>>> +#define STM32F4_SQ14_SHIFT 5
>>> +#define STM32F4_SQ14_MASK GENMASK(9, 5)
>>> +#define STM32F4_SQ13_SHIFT 0
>>> +#define STM32F4_SQ13_MASK GENMASK(4, 0)
>>> +
>>> +/* STM32F4_ADCX_SQR2 - bit fields */
>>> +#define STM32F4_SQ12_SHIFT 25
>>> +#define STM32F4_SQ12_MASK GENMASK(29, 25)
>>> +#define STM32F4_SQ11_SHIFT 20
>>> +#define STM32F4_SQ11_MASK GENMASK(24, 20)
>>> +#define STM32F4_SQ10_SHIFT 15
>>> +#define STM32F4_SQ10_MASK GENMASK(19, 15)
>>> +#define STM32F4_SQ9_SHIFT 10
>>> +#define STM32F4_SQ9_MASK GENMASK(14, 10)
>>> +#define STM32F4_SQ8_SHIFT 5
>>> +#define STM32F4_SQ8_MASK GENMASK(9, 5)
>>> +#define STM32F4_SQ7_SHIFT 0
>>> +#define STM32F4_SQ7_MASK GENMASK(4, 0)
>>> +
>>> +/* STM32F4_ADCX_SQR3 - bit fields */
>>> +#define STM32F4_SQ6_SHIFT 25
>>> +#define STM32F4_SQ6_MASK GENMASK(29, 25)
>>> +#define STM32F4_SQ5_SHIFT 20
>>> +#define STM32F4_SQ5_MASK GENMASK(24, 20)
>>> +#define STM32F4_SQ4_SHIFT 15
>>> +#define STM32F4_SQ4_MASK GENMASK(19, 15)
>>> +#define STM32F4_SQ3_SHIFT 10
>>> +#define STM32F4_SQ3_MASK GENMASK(14, 10)
>>> +#define STM32F4_SQ2_SHIFT 5
>>> +#define STM32F4_SQ2_MASK GENMASK(9, 5)
>>> +#define STM32F4_SQ1_SHIFT 0
>>> +#define STM32F4_SQ1_MASK GENMASK(4, 0)
>>> +
>>> +/* STM32F4_ADCX_JSQR - bit fields */
>>> +#define STM32F4_JL_SHIFT 20
>>> +#define STM32F4_JL_MASK GENMASK(21, 20)
>>> +#define STM32F4_JSQ4_SHIFT 15
>>> +#define STM32F4_JSQ4_MASK GENMASK(19, 15)
>>> +#define STM32F4_JSQ3_SHIFT 10
>>> +#define STM32F4_JSQ3_MASK GENMASK(14, 10)
>>> +#define STM32F4_JSQ2_SHIFT 5
>>> +#define STM32F4_JSQ2_MASK GENMASK(9, 5)
>>> +#define STM32F4_JSQ1_SHIFT 0
>>> +#define STM32F4_JSQ1_MASK GENMASK(4, 0)
>>> +
>>> +/* STM32F4_ADC_CCR - bit fields */
>>> +#define STM32F4_ADC_ADCPRE_SHIFT 16
>>> +#define STM32F4_ADC_ADCPRE_MASK GENMASK(17, 16)
>>> +
>>> +/*
>>> + * stm32 ADC1, ADC2 & ADC3 are tightly coupled and may be used in multi mode
>>> + * Define here all inputs for all ADC instances
>>> + */
>>> +static const struct stm32_adc_chan_spec stm32f4_adc1_channels[] = {
>>> + /* master ADC1 */
>>> + { IIO_VOLTAGE, 0, "in0" },
>>> + { IIO_VOLTAGE, 1, "in1" },
>>> + { IIO_VOLTAGE, 2, "in2" },
>>> + { IIO_VOLTAGE, 3, "in3" },
>>> + { IIO_VOLTAGE, 4, "in4" },
>>> + { IIO_VOLTAGE, 5, "in5" },
>>> + { IIO_VOLTAGE, 6, "in6" },
>>> + { IIO_VOLTAGE, 7, "in7" },
>>> + { IIO_VOLTAGE, 8, "in8" },
>>> + { IIO_VOLTAGE, 9, "in9" },
>>> + { IIO_VOLTAGE, 10, "in10" },
>>> + { IIO_VOLTAGE, 11, "in11" },
>>> + { IIO_VOLTAGE, 12, "in12" },
>>> + { IIO_VOLTAGE, 13, "in13" },
>>> + { IIO_VOLTAGE, 14, "in14" },
>>> + { IIO_VOLTAGE, 15, "in15" },
>>> + /* internal analog sources available on input 16 to 18 */
>>> + { IIO_VOLTAGE, 16, "in16" },
>>> + { IIO_VOLTAGE, 17, "in17" },
>>> + { IIO_VOLTAGE, 18, "in18" },
>>> +};
>>> +
>>> +static const struct stm32_adc_chan_spec stm32f4_adc23_channels[] = {
>>> + /* slave ADC2 / ADC3 */
>>> + { IIO_VOLTAGE, 0, "in0" },
>>> + { IIO_VOLTAGE, 1, "in1" },
>>> + { IIO_VOLTAGE, 2, "in2" },
>>> + { IIO_VOLTAGE, 3, "in3" },
>>> + { IIO_VOLTAGE, 4, "in4" },
>>> + { IIO_VOLTAGE, 5, "in5" },
>>> + { IIO_VOLTAGE, 6, "in6" },
>>> + { IIO_VOLTAGE, 7, "in7" },
>>> + { IIO_VOLTAGE, 8, "in8" },
>>> + { IIO_VOLTAGE, 9, "in9" },
>>> + { IIO_VOLTAGE, 10, "in10" },
>>> + { IIO_VOLTAGE, 11, "in11" },
>>> + { IIO_VOLTAGE, 12, "in12" },
>>> + { IIO_VOLTAGE, 13, "in13" },
>>> + { IIO_VOLTAGE, 14, "in14" },
>>> + { IIO_VOLTAGE, 15, "in15" },
>>> +};
>>> +
>>> +/* Triggers for regular channels */
>>> +static const struct stm32_adc_trig_info stm32f4_adc_ext_triggers[] = {
>>> + { STM32_EXT0, "TIM1_CH1" },
>>> + { STM32_EXT1, "TIM1_CH2" },
>>> + { STM32_EXT2, "TIM1_CH3" },
>>> + { STM32_EXT3, "TIM2_CH2" },
>>> + { STM32_EXT4, "TIM2_CH3" },
>>> + { STM32_EXT5, "TIM2_CH4" },
>>> + { STM32_EXT6, "TIM2_TRGO" },
>>> + { STM32_EXT7, "TIM3_CH1" },
>>> + { STM32_EXT8, "TIM3_TRGO" },
>>> + { STM32_EXT9, "TIM4_CH4" },
>>> + { STM32_EXT10, "TIM5_CH1" },
>>> + { STM32_EXT11, "TIM5_CH2" },
>>> + { STM32_EXT12, "TIM5_CH3" },
>>> + { STM32_EXT13, "TIM8_CH1" },
>>> + { STM32_EXT14, "TIM8_TRGO" },
>>> + { STM32_EXT15, "EXTI_11" },
>>> + {},
>>> +};
>>> +
>>> +/* Triggers for injected channels */
>>> +static const struct stm32_adc_trig_info stm32f4_adc_jext_triggers[] = {
>>> + { STM32_JEXT0, "TIM1_CH4" },
>>> + { STM32_JEXT1, "TIM1_TRGO" },
>>> + { STM32_JEXT2, "TIM2_CH1" },
>>> + { STM32_JEXT3, "TIM2_TRGO" },
>>> + { STM32_JEXT4, "TIM3_CH2" },
>>> + { STM32_JEXT5, "TIM3_CH4" },
>>> + { STM32_JEXT6, "TIM4_CH1" },
>>> + { STM32_JEXT7, "TIM4_CH2" },
>>> + { STM32_JEXT8, "TIM4_CH3" },
>>> + { STM32_JEXT9, "TIM4_TRGO" },
>>> + { STM32_JEXT10, "TIM5_CH4" },
>>> + { STM32_JEXT11, "TIM5_TRGO" },
>>> + { STM32_JEXT12, "TIM8_CH2" },
>>> + { STM32_JEXT13, "TIM8_CH3" },
>>> + { STM32_JEXT14, "TIM8_CH4" },
>>> + { STM32_JEXT15, "EXTI_15" },
>>> + {},
>>> +};
>>> +
>>> +static const struct stm32_adc_info stm32f4_adc_info[] = {
>>> + {
>>> + .name = "adc1-master",
>>> + .reg = 0x0,
>>> + .channels = stm32f4_adc1_channels,
>>> + .max_channels = ARRAY_SIZE(stm32f4_adc1_channels),
>>> + },
>>> + {
>>> + .name = "adc2-slave",
>>> + .reg = 0x100,
>>> + .channels = stm32f4_adc23_channels,
>>> + .max_channels = ARRAY_SIZE(stm32f4_adc23_channels),
>>> + },
>>> + {
>>> + .name = "adc3-slave",
>>> + .reg = 0x200,
>>> + .channels = stm32f4_adc23_channels,
>>> + .max_channels = ARRAY_SIZE(stm32f4_adc23_channels),
>>> + },
>>> + {},
>>> +};
>>> +
>>> +/**
>>> + * stm32f4_sqr_regs - describe regular sequence registers
>>> + * - L: sequence len (register & bit field)
>>> + * - SQ1..SQ16: sequence entries (register & bit field)
>>> + */
>>> +static const struct stm32_adc_regs stm32f4_sqr_regs[STM32_ADC_MAX_SQ + 1] = {
>>> + /* L: len bit field description to be kept as first element */
>>> + { STM32F4_ADCX_SQR1, STM32F4_L_MASK, STM32F4_L_SHIFT },
>>> + /* SQ1..SQ16 registers & bit fields */
>>> + { STM32F4_ADCX_SQR3, STM32F4_SQ1_MASK, STM32F4_SQ1_SHIFT },
>>> + { STM32F4_ADCX_SQR3, STM32F4_SQ2_MASK, STM32F4_SQ2_SHIFT },
>>> + { STM32F4_ADCX_SQR3, STM32F4_SQ3_MASK, STM32F4_SQ3_SHIFT },
>>> + { STM32F4_ADCX_SQR3, STM32F4_SQ4_MASK, STM32F4_SQ4_SHIFT },
>>> + { STM32F4_ADCX_SQR3, STM32F4_SQ5_MASK, STM32F4_SQ5_SHIFT },
>>> + { STM32F4_ADCX_SQR3, STM32F4_SQ6_MASK, STM32F4_SQ6_SHIFT },
>>> + { STM32F4_ADCX_SQR2, STM32F4_SQ7_MASK, STM32F4_SQ7_SHIFT },
>>> + { STM32F4_ADCX_SQR2, STM32F4_SQ8_MASK, STM32F4_SQ8_SHIFT },
>>> + { STM32F4_ADCX_SQR2, STM32F4_SQ9_MASK, STM32F4_SQ9_SHIFT },
>>> + { STM32F4_ADCX_SQR2, STM32F4_SQ10_MASK, STM32F4_SQ10_SHIFT },
>>> + { STM32F4_ADCX_SQR2, STM32F4_SQ11_MASK, STM32F4_SQ11_SHIFT },
>>> + { STM32F4_ADCX_SQR2, STM32F4_SQ12_MASK, STM32F4_SQ12_SHIFT },
>>> + { STM32F4_ADCX_SQR1, STM32F4_SQ13_MASK, STM32F4_SQ13_SHIFT },
>>> + { STM32F4_ADCX_SQR1, STM32F4_SQ14_MASK, STM32F4_SQ14_SHIFT },
>>> + { STM32F4_ADCX_SQR1, STM32F4_SQ15_MASK, STM32F4_SQ15_SHIFT },
>>> + { STM32F4_ADCX_SQR1, STM32F4_SQ16_MASK, STM32F4_SQ16_SHIFT },
>>> +};
>>> +
>>> +/**
>>> + * stm32f4_jsqr_reg - describe injected sequence register:
>>> + * - JL: injected sequence len
>>> + * - JSQ4..SQ1: sequence entries
>>> + * When JL == 3, ADC converts JSQ1, JSQ2, JSQ3, JSQ4
>>> + * When JL == 2, ADC converts JSQ2, JSQ3, JSQ4
>>> + * When JL == 1, ADC converts JSQ3, JSQ4
>>> + * When JL == 0, ADC converts JSQ4
>>> + */
>>> +static const struct stm32_adc_regs stm32f4_jsqr_reg[STM32_ADC_MAX_JSQ + 1] = {
>>> + /* JL: len bit field description to be kept as first element */
>>> + {STM32F4_ADCX_JSQR, STM32F4_JL_MASK, STM32F4_JL_SHIFT},
>>> + /* JSQ4..JSQ1 registers & bit fields */
>>> + {STM32F4_ADCX_JSQR, STM32F4_JSQ4_MASK, STM32F4_JSQ4_SHIFT},
>>> + {STM32F4_ADCX_JSQR, STM32F4_JSQ3_MASK, STM32F4_JSQ3_SHIFT},
>>> + {STM32F4_ADCX_JSQR, STM32F4_JSQ2_MASK, STM32F4_JSQ2_SHIFT},
>>> + {STM32F4_ADCX_JSQR, STM32F4_JSQ1_MASK, STM32F4_JSQ1_SHIFT},
>>> +};
>>> +
>>> +static const struct stm32_adc_trig_reginfo stm32f4_adc_trig_reginfo = {
>>> + .reg = STM32F4_ADCX_CR2,
>>> + .exten_mask = STM32F4_EXTEN_MASK,
>>> + .exten_shift = STM32F4_EXTEN_SHIFT,
>>> + .extsel_mask = STM32F4_EXTSEL_MASK,
>>> + .extsel_shift = STM32F4_EXTSEL_SHIFT,
>>> +};
>>> +
>>> +static const struct stm32_adc_trig_reginfo stm32f4_adc_jtrig_reginfo = {
>>> + .reg = STM32F4_ADCX_CR2,
>>> + .exten_mask = STM32F4_JEXTEN_MASK,
>>> + .exten_shift = STM32F4_JEXTEN_SHIFT,
>>> + .extsel_mask = STM32F4_JEXTSEL_MASK,
>>> + .extsel_shift = STM32F4_JEXTSEL_SHIFT,
>>> +};
>>> +
>>> +static const struct stm32_adc_reginfo stm32f4_adc_reginfo = {
>>> + .isr = STM32F4_ADCX_SR,
>>> + .eoc = STM32F4_EOC,
>>> + .jeoc = STM32F4_JEOC,
>>> + .ier = STM32F4_ADCX_CR1,
>>> + .eocie = STM32F4_EOCIE,
>>> + .jeocie = STM32F4_JEOCIE,
>>> + .dr = STM32F4_ADCX_DR,
>>> + .jdr = {
>>> + STM32F4_ADCX_JDR1,
>>> + STM32F4_ADCX_JDR2,
>>> + STM32F4_ADCX_JDR3,
>>> + STM32F4_ADCX_JDR4,
>>> + },
>>> + .sqr_regs = stm32f4_sqr_regs,
>>> + .jsqr_reg = stm32f4_jsqr_reg,
>>> + .trig_reginfo = &stm32f4_adc_trig_reginfo,
>>> + .jtrig_reginfo = &stm32f4_adc_jtrig_reginfo,
>>> +};
>>> +
>>> +static bool stm32f4_adc_is_started(struct stm32_adc *adc)
>>> +{
>>> + u32 val = stm32_adc_readl(adc, STM32F4_ADCX_CR2) & STM32F4_ADON;
>>> +
>>> + return !!val;
>>> +}
>>> +
>>> +static bool stm32f4_adc_regular_started(struct stm32_adc *adc)
>>> +{
>>> + u32 val = stm32_adc_readl(adc, STM32F4_ADCX_SR) & STM32F4_STRT;
>>> +
>>> + return !!val;
>>> +}
>>> +
>>> +static bool stm32f4_adc_injected_started(struct stm32_adc *adc)
>>> +{
>>> + u32 val = stm32_adc_readl(adc, STM32F4_ADCX_SR) & STM32F4_JSTRT;
>>> +
>>> + return !!val;
>>> +}
>>> +
>>> +/**
>>> + * stm32f4_adc_start_conv() - Start regular or injected conversions
>>> + * @adc: stm32 adc instance
>>> + *
>>> + * Start single conversions for regular or injected channels.
>>> + */
>>> +static int stm32f4_adc_start_conv(struct stm32_adc *adc)
>>> +{
>>> + u32 trig_msk, start_msk;
>>> +
>>> + dev_dbg(adc->common->dev, "%s %s\n", __func__,
>>> + adc->injected ? "injected" : "regular");
>>> +
>>> + stm32_adc_set_bits(adc, STM32F4_ADCX_CR1, STM32F4_SCAN);
>>> +
>>> + if (!stm32f4_adc_is_started(adc)) {
>>> + stm32_adc_set_bits(adc, STM32F4_ADCX_CR2,
>>> + STM32F4_EOCS | STM32F4_ADON);
>>> +
>>> + /* Wait for Power-up time (tSTAB from datasheet) */
>>> + usleep_range(2, 3);
>>> + }
>>> +
>>> + if (adc->injected) {
>>> + trig_msk = STM32F4_JEXTEN_MASK;
>>> + start_msk = STM32F4_JSWSTART;
>>> + } else {
>>> + trig_msk = STM32F4_EXTEN_MASK;
>>> + start_msk = STM32F4_SWSTART;
>>> + }
>>> +
>>> + /* Software start ? (e.g. trigger detection disabled ?) */
>>> + if (!(stm32_adc_readl(adc, STM32F4_ADCX_CR2) & trig_msk))
>>> + stm32_adc_set_bits(adc, STM32F4_ADCX_CR2, start_msk);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int stm32f4_adc_stop_conv(struct stm32_adc *adc)
>>> +{
>>> + u32 val;
>>> +
>>> + dev_dbg(adc->common->dev, "%s %s\n", __func__,
>>> + adc->injected ? "injected" : "regular");
>>> +
>>> + /* First disable trigger for either regular or injected channels */
>>> + if (adc->injected) {
>>> + stm32_adc_clr_bits(adc, STM32F4_ADCX_CR2, STM32F4_JEXTEN_MASK);
>>> + stm32_adc_clr_bits(adc, STM32F4_ADCX_SR, STM32F4_JSTRT);
>>> + } else {
>>> + stm32_adc_clr_bits(adc, STM32F4_ADCX_CR2, STM32F4_EXTEN_MASK);
>>> + stm32_adc_clr_bits(adc, STM32F4_ADCX_SR, STM32F4_STRT);
>>> + }
>>> +
>>> + /* Disable adc when all triggered conversion have been disabled */
>>> + val = stm32_adc_readl(adc, STM32F4_ADCX_CR2);
>>> + val &= STM32F4_EXTEN_MASK | STM32F4_JEXTEN_MASK;
>>> + if (!val) {
>>> + stm32_adc_clr_bits(adc, STM32F4_ADCX_CR1, STM32F4_SCAN);
>>> + stm32_adc_clr_bits(adc, STM32F4_ADCX_CR2, STM32F4_ADON);
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +/* ADC internal common clock prescaler division ratios */
>>> +static int stm32f4_pclk_div[] = {2, 4, 6, 8};
>>> +
>>> +/**
>>> + * stm32f4_adc_clk_sel() - Select ADC common clock prescaler
>>> + * @adc: stm32 adc instance
>>> + * Select clock prescaler used for analog conversions.
>>> + */
>>> +static int stm32f4_adc_clk_sel(struct stm32_adc *adc)
>>> +{
>>> + struct stm32_adc_common *common = adc->common;
>>> + unsigned long rate;
>>> + u32 val;
>>> + int i;
>>> +
>>> + /* Common prescaler is set only once, when 1st ADC instance starts */
>>> + list_for_each_entry(adc, &common->adc_list, adc_list)
>>> + if (stm32f4_adc_is_started(adc))
>>> + return 0;
>>> +
>>> + rate = clk_get_rate(common->aclk);
>>> + for (i = 0; i < ARRAY_SIZE(stm32f4_pclk_div); i++) {
>>> + if ((rate / stm32f4_pclk_div[i]) <=
>>> + common->data->max_clock_rate)
>>> + break;
>>> + }
>>> + if (i >= ARRAY_SIZE(stm32f4_pclk_div))
>>> + return -EINVAL;
>>> +
>>> + val = stm32_adc_common_readl(common, STM32F4_ADC_CCR);
>>> + val &= ~STM32F4_ADC_ADCPRE_MASK;
>>> + val |= i << STM32F4_ADC_ADCPRE_SHIFT;
>>> + stm32_adc_common_writel(common, STM32F4_ADC_CCR, val);
>>> +
>>> + dev_dbg(common->dev, "Using analog clock source at %ld kHz\n",
>>> + rate / (stm32f4_pclk_div[i] * 1000));
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static const struct stm32_adc_ops stm32f4_adc_ops = {
>>> + .adc_info = stm32f4_adc_info,
>>> + .ext_triggers = stm32f4_adc_ext_triggers,
>>> + .jext_triggers = stm32f4_adc_jext_triggers,
>>> + .adc_reginfo = &stm32f4_adc_reginfo,
>>> + .highres = 12,
>>> + .max_clock_rate = 36000000,
>>> + .clk_sel = stm32f4_adc_clk_sel,
>>> + .start_conv = stm32f4_adc_start_conv,
>>> + .stop_conv = stm32f4_adc_stop_conv,
>>> + .is_started = stm32f4_adc_is_started,
>>> + .regular_started = stm32f4_adc_regular_started,
>>> + .injected_started = stm32f4_adc_injected_started,
>>> +};
>>> +
>>> +static const struct of_device_id stm32f4_adc_of_match[] = {
>>> + { .compatible = "st,stm32f4-adc", .data = (void *)&stm32f4_adc_ops},
>>> + {},
>>> +};
>>> +MODULE_DEVICE_TABLE(of, stm32f4_adc_of_match);
>>> +
>>> +static struct platform_driver stm32f4_adc_driver = {
>>> + .probe = stm32_adc_probe,
>>> + .remove = stm32_adc_remove,
>>> + .driver = {
>>> + .name = "stm32f4-adc",
>>> + .of_match_table = stm32f4_adc_of_match,
>>> + },
>>> +};
>>> +
>>> +module_platform_driver(stm32f4_adc_driver);
>>> +
>>> +MODULE_AUTHOR("Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>");
>>> +MODULE_DESCRIPTION("STMicroelectronics STM32F4 ADC driver");
>>> +MODULE_LICENSE("GPL v2");
>>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox