* 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
* [PATCH 2/2] arm64: dts: sunxi: enable EHCI1, OHCI1 and USB PHY nodes in Pine64
From: Icenowy Zheng @ 2016-11-05 14:38 UTC (permalink / raw)
To: Maxime Ripard, Chen-Yu Tsai, Rob Herring, Hans de Goede,
Andre Przywara
Cc: Mark Rutland, devicetree, Catalin Marinas, Will Deacon,
linux-kernel, Icenowy Zheng, linux-arm-kernel
In-Reply-To: <20161105143830.10099-1-icenowy@aosc.xyz>
Pine64 have two USB Type-A ports, which are wired to the two ports of
A64 USB PHY, and the lower port is the EHCI/OHCI1 port.
Enable the necessary nodes to enable the lower USB port to work.
Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
---
arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
index 4709590..d836995 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
@@ -72,3 +72,15 @@
&i2c1_pins {
bias-pull-up;
};
+
+&usbphy {
+ status = "okay";
+};
+
+&ehci1 {
+ status = "okay";
+};
+
+&ohci1 {
+ status = "okay";
+};
--
2.10.1
^ permalink raw reply related
* [PATCH 1/2] arm64: dts: add USB1-related nodes of Allwinner A64
From: Icenowy Zheng @ 2016-11-05 14:38 UTC (permalink / raw)
To: Maxime Ripard, Chen-Yu Tsai, Rob Herring, Hans de Goede,
Andre Przywara
Cc: Mark Rutland, devicetree, Catalin Marinas, Will Deacon,
linux-kernel, Icenowy Zheng, linux-arm-kernel
Allwinner A64 have two HCI USB controllers, a OTG controller and a USB
PHY device which have two ports. One of the port is wired to both a HCI
USB controller and the OTG controller, which is currently not supported.
The another one is only wired to a HCI controller, and the device node of
OHCI/EHCI controller of the port can be added now.
Also the A64 USB PHY device node is also added for the HCI controllers to
work.
Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
---
arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 50 +++++++++++++++++++++++++++
1 file changed, 50 insertions(+)
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index 3d70be3..c2b6dc8 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -259,5 +259,55 @@
interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH>;
};
+
+ usbphy: phy@01c19400 {
+ compatible = "allwinner,sun50i-a64-usb-phy";
+ reg = <0x01c19400 0x14>,
+ <0x01c1b800 0x4>;
+ reg-names = "phy_ctrl",
+ "pmu1";
+ clocks = <&ccu CLK_USB_PHY0>,
+ <&ccu CLK_USB_PHY1>;
+ clock-names = "usb0_phy",
+ "usb1_phy";
+ resets = <&ccu RST_USB_PHY0>,
+ <&ccu RST_USB_PHY1>;
+ reset-names = "usb0_reset",
+ "usb1_reset";
+ status = "disabled";
+ #phy-cells = <1>;
+ };
+
+ ohci1: usb@01c1a400 {
+ compatible = "allwinner,sun50i-a64-ohci", "generic-ohci";
+ reg = <0x01c1b400 0x100>;
+ interrupts = <GIC_SPI 75 IRQ_TYPE_LEVEL_HIGH>;
+ /*
+ * According to the user manual, OHCI1 USB clock
+ * depends on OHCI0 clock.
+ */
+ clocks = <&ccu CLK_BUS_OHCI1>,
+ <&ccu CLK_USB_OHCI0>,
+ <&ccu CLK_USB_OHCI1>;
+ resets = <&ccu RST_BUS_OHCI1>;
+ phys = <&usbphy 1>;
+ phy-names = "usb";
+ status = "disabled";
+ };
+
+ ehci1: usb@01c1a000 {
+ compatible = "allwinner,sun50i-a64-ehci", "generic-ehci";
+ reg = <0x01c1b000 0x100>;
+ interrupts = <GIC_SPI 74 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&ccu CLK_BUS_OHCI1>,
+ <&ccu CLK_BUS_EHCI1>,
+ <&ccu CLK_USB_OHCI0>,
+ <&ccu CLK_USB_OHCI1>;
+ resets = <&ccu RST_BUS_OHCI1>,
+ <&ccu RST_BUS_EHCI1>;
+ phys = <&usbphy 1>;
+ phy-names = "usb";
+ status = "disabled";
+ };
};
};
--
2.10.1
^ permalink raw reply related
* Re: [PATCH] ARM: dts: imx6: Add imx-weim parameters to dtsi's
From: Shawn Guo @ 2016-11-05 11:47 UTC (permalink / raw)
To: Joshua Clayton
Cc: Sascha Hauer, Fabio Estevam, Rob Herring, Mark Rutland,
Russell King,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
In-Reply-To: <1478044305-5666-1-git-send-email-stillcompiling@gmail.com>
On Tue, Nov 01, 2016 at 04:51:45PM -0700, Joshua Clayton wrote:
> imx-weim should always set address-cells to 2,
> and size_cells to 1.
> On imx6, fsl,weim-cs-gpr will always be &gpr
>
> Set these common parameters in the dtsi file,
> rather than in a downstream dts.
>
> Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
Applied, thanks.
^ permalink raw reply
* Re: [REGRESSION] "console: don't prefer first registered if DT specifies stdout-path" breaks console on video outputs of various ARM boards
From: Paul Burton @ 2016-11-05 11:00 UTC (permalink / raw)
To: Hans de Goede
Cc: Linus Torvalds, Andrew Morton, Rob Herring, Frank Rowand,
Thorsten Leemhuis, Greg Kroah-Hartman, Tejun Heo, devicetree,
linux-kernel
In-Reply-To: <d3f0f1f4-d6f6-1e75-499c-789b82b5bb1b@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 4562 bytes --]
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?
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.
Thanks,
Paul
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: [REGRESSION] "console: don't prefer first registered if DT specifies stdout-path" breaks console on video outputs of various ARM boards
From: Paul Burton @ 2016-11-05 10:40 UTC (permalink / raw)
To: Hans de Goede
Cc: Linus Torvalds, Andrew Morton, Rob Herring, Frank Rowand,
Thorsten Leemhuis, Greg Kroah-Hartman, Tejun Heo, devicetree,
linux-kernel
In-Reply-To: <d3f0f1f4-d6f6-1e75-499c-789b82b5bb1b@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 4562 bytes --]
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?
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.
Thanks,
Paul
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: [PATCH v6 7/7] arm64: dts: NS2: add AMAC ethernet support
From: Sergei Shtylyov @ 2016-11-05 10:33 UTC (permalink / raw)
To: Jon Mason
Cc: David Miller, Rob Herring, Mark Rutland, Florian Fainelli, rafal,
bcm-kernel-feedback-list, netdev, devicetree, linux-arm-kernel,
linux-kernel
In-Reply-To: <20161104163024.GA6128@broadcom.com>
On 11/4/2016 7:30 PM, Jon Mason wrote:
>>> Add support for the AMAC ethernet to the Broadcom Northstar2 SoC device
>>> tree
>>>
>>> Signed-off-by: Jon Mason <jon.mason@broadcom.com>
>>> ---
>>> arch/arm64/boot/dts/broadcom/ns2-svk.dts | 5 +++++
>>> arch/arm64/boot/dts/broadcom/ns2.dtsi | 12 ++++++++++++
>>> 2 files changed, 17 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/broadcom/ns2-svk.dts b/arch/arm64/boot/dts/broadcom/ns2-svk.dts
>>> index b09f3bc..c4d5442 100644
>>> --- a/arch/arm64/boot/dts/broadcom/ns2-svk.dts
>>> +++ b/arch/arm64/boot/dts/broadcom/ns2-svk.dts
>>> @@ -56,6 +56,10 @@
>>> };
>>> };
>>>
>>> +&enet {
>>> + status = "ok";
>>
>> The spec dictates it should be "okay" (although "ok" is also recognized).
>
> The rest of the file uses "ok". So, the addition above is consistent
> with the other entries.
>
> Perhaps a patch outside this series to convert the entire file from
> "ok" to "okay" would be acceptable to you.
OK, it would...
>
> Thanks,
> Jon
MBR, Sergei
^ permalink raw reply
* Re: [PATCH 3/3] clk: imx: clk-imx6ul: add clk support for imx6ull
From: Shawn Guo @ 2016-11-05 9:05 UTC (permalink / raw)
To: Peter Chen
Cc: sboyd-sgV2jX0FEOL9JmXXK+q4OQ, mturquette-rdvid1DuHRBWk0Htik3J/w,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
kernel-bIcnvbaLZ9MEGnE8C9+IrQ, devicetree-u79uwXL29TY76Z2rM5mHXA,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, fabio.estevam-3arQi8VN3Tc,
mark.rutland-5wv7dgnIgG8, linux-clk-u79uwXL29TY76Z2rM5mHXA,
Bai Ping, Peng Fan
In-Reply-To: <1477969343-19887-4-git-send-email-peter.chen-3arQi8VN3Tc@public.gmane.org>
On Tue, Nov 01, 2016 at 11:02:23AM +0800, Peter Chen wrote:
> From: Bai Ping <ping.bai-3arQi8VN3Tc@public.gmane.org>
>
> imx6ull is the derived SoC from imx6ul
>
> Cc: Michael Turquette <mturquette-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> Cc: Stephen Boyd <sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> Signed-off-by: Peng Fan <peng.fan-3arQi8VN3Tc@public.gmane.org>
> Signed-off-by: Bai Ping <ping.bai-3arQi8VN3Tc@public.gmane.org>
> Signed-off-by: Peter Chen <peter.chen-3arQi8VN3Tc@public.gmane.org>
> ---
> drivers/clk/imx/clk-imx6ul.c | 74 +++++++++++++++++++++++++++-----
> include/dt-bindings/clock/imx6ul-clock.h | 15 ++++++-
> 2 files changed, 77 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/clk/imx/clk-imx6ul.c b/drivers/clk/imx/clk-imx6ul.c
> index d1d7787..ceb99a7 100644
> --- a/drivers/clk/imx/clk-imx6ul.c
> +++ b/drivers/clk/imx/clk-imx6ul.c
> @@ -64,6 +64,11 @@ static const char *perclk_sels[] = { "ipg", "osc", };
> static const char *lcdif_sels[] = { "lcdif_podf", "ipp_di0", "ipp_di1", "ldb_di0", "ldb_di1", };
> static const char *csi_sels[] = { "osc", "pll2_pfd2_396m", "pll3_120m", "pll3_pfd1_540m", };
> static const char *sim_sels[] = { "sim_podf", "ipp_di0", "ipp_di1", "ldb_di0", "ldb_di1", };
> +/* epdc_pre_sels, epdc_sels, esai_sels only exists on i.MX6ULL */
> +static const char *epdc_pre_sels[] = { "pll2_bus", "pll3_usb_otg", "pll5_video_div", "pll2_pfd0_352m", "pll2_pfd2_396m", "pll3_pfd2_508m", };
> +static const char *esai_sels[] = { "pll4_audio_div", "pll3_pfd2_508m", "pll5_video_div", "pll3_usb_otg", };
> +static const char *epdc_sels[] = { "epdc_podf", "ipp_di0", "ipp_di1", "ldb_di0", "ldb_di1", };
> +
The new line is not needed. There is already one below.
>
> static struct clk *clks[IMX6UL_CLK_END];
> static struct clk_onecell_data clk_data;
<snip>
> diff --git a/include/dt-bindings/clock/imx6ul-clock.h b/include/dt-bindings/clock/imx6ul-clock.h
> index fd8aee8..563fd5b 100644
> --- a/include/dt-bindings/clock/imx6ul-clock.h
> +++ b/include/dt-bindings/clock/imx6ul-clock.h
> @@ -236,6 +236,19 @@
> #define IMX6UL_CLK_PLL3_120M 223
> #define IMX6UL_CLK_KPP 224
>
> -#define IMX6UL_CLK_END 225
> +/* For i.MX6ULL */
> +#define IMX6UL_CLK_ESAI_PRED 225
> +#define IMX6UL_CLK_ESAI_PODF 226
> +#define IMX6UL_CLK_ESAI_EXTAL 227
> +#define IMX6UL_CLK_ESAI_MEM 228
> +#define IMX6UL_CLK_ESAI_IPG 229
> +#define IMX6UL_CLK_DCP_CLK 230
> +#define IMX6UL_CLK_EPDC_PRE_SEL 231
> +#define IMX6UL_CLK_EPDC_SEL 232
> +#define IMX6UL_CLK_EPDC_PODF 233
> +#define IMX6UL_CLK_EPDC_ACLK 234
> +#define IMX6UL_CLK_EPDC_PIX 235
> +#define IMX6UL_CLK_ESAI_SEL 236
Can we have these imx6ull only clocks named after IMX6ULL_xxx?
Shawn
> +#define IMX6UL_CLK_END 237
>
> #endif /* __DT_BINDINGS_CLOCK_IMX6UL_H */
> --
> 2.7.4
>
--
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 1/3] ARM: imx6u: add imx6ull support
From: Shawn Guo @ 2016-11-05 8:59 UTC (permalink / raw)
To: Peter Chen
Cc: sboyd-sgV2jX0FEOL9JmXXK+q4OQ, mturquette-rdvid1DuHRBWk0Htik3J/w,
mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
fabio.estevam-3arQi8VN3Tc, linux-clk-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1477969343-19887-2-git-send-email-peter.chen-3arQi8VN3Tc@public.gmane.org>
On Tue, Nov 01, 2016 at 11:02:21AM +0800, Peter Chen wrote:
> It is the 10th processor in the well-known imx6 series, and derived
> from imx6ul but cost optimized. The more information about imx6ull
> can be found at:
>
> http://www.nxp.com/products/microcontrollers-and-processors/
> arm-processors/i.mx-applications-processors/i.mx-6-processors
> /i.mx6qp/i.mx-6ull-single-core-processor-with-arm-cortex-a7-core
> :i.MX6ULL
>
> In this patch, it moves common dts between imx6ul* and imx6ull* as
> new files named imx6u*, and the specific part for imx6ul and imx6ull
> are individual file.
No. That only helps churn the tree and git history. Please keep using
imx6ul.dtsi and simply have it be included by imx6ull.dtsi.
>
> Signed-off-by: Peter Chen <peter.chen-3arQi8VN3Tc@public.gmane.org>
> ---
> arch/arm/boot/dts/Makefile | 3 +-
> arch/arm/boot/dts/imx6u-14x14-evk.dts | 475 ++++++++++++++++
> arch/arm/boot/dts/imx6u-14x14-evk.dtsi | 487 +++++++++++++++++
> arch/arm/boot/dts/imx6u.dtsi | 942 ++++++++++++++++++++++++++++++++
> arch/arm/boot/dts/imx6ul-14x14-evk.dts | 479 +---------------
> arch/arm/boot/dts/imx6ul.dtsi | 936 +------------------------------
> arch/arm/boot/dts/imx6ull-14x14-evk.dts | 17 +
> arch/arm/boot/dts/imx6ull-pinfunc.h | 57 ++
> arch/arm/boot/dts/imx6ull.dtsi | 10 +
> 9 files changed, 1993 insertions(+), 1413 deletions(-)
> create mode 100644 arch/arm/boot/dts/imx6u-14x14-evk.dts
> create mode 100644 arch/arm/boot/dts/imx6u-14x14-evk.dtsi
> create mode 100644 arch/arm/boot/dts/imx6u.dtsi
> create mode 100644 arch/arm/boot/dts/imx6ull-14x14-evk.dts
> create mode 100644 arch/arm/boot/dts/imx6ull-pinfunc.h
> create mode 100644 arch/arm/boot/dts/imx6ull.dtsi
>
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index befcd26..3d6e199 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -423,7 +423,8 @@ dtb-$(CONFIG_SOC_IMX6UL) += \
> imx6ul-pico-hobbit.dtb \
> imx6ul-tx6ul-0010.dtb \
> imx6ul-tx6ul-0011.dtb \
> - imx6ul-tx6ul-mainboard.dtb
> + imx6ul-tx6ul-mainboard.dtb \
> + imx6ull-14x14-evk.dtb
> dtb-$(CONFIG_SOC_IMX7D) += \
> imx7d-cl-som-imx7.dtb \
> imx7d-colibri-eval-v3.dtb \
> diff --git a/arch/arm/boot/dts/imx6u-14x14-evk.dts b/arch/arm/boot/dts/imx6u-14x14-evk.dts
> new file mode 100644
> index 0000000..ba8614c
> --- /dev/null
> +++ b/arch/arm/boot/dts/imx6u-14x14-evk.dts
> @@ -0,0 +1,475 @@
> +/*
> + * Copyright (C) 2015 Freescale Semiconductor, 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.
> + */
For new DT files, please consider to use GPL/X11 dual license. You
should be able to find a lot of examples in arch/arm/boot/dts.
<snip>
> diff --git a/arch/arm/boot/dts/imx6ull-pinfunc.h b/arch/arm/boot/dts/imx6ull-pinfunc.h
> new file mode 100644
> index 0000000..fca0036
> --- /dev/null
> +++ b/arch/arm/boot/dts/imx6ull-pinfunc.h
> @@ -0,0 +1,57 @@
> +/*
> + * Copyright (C) 2016 Freescale Semiconductor, 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.
> + */
> +
> +#ifndef __DTS_IMX6ULL_PINFUNC_H
> +#define __DTS_IMX6ULL_PINFUNC_H
> +
> +#include "imx6ul-pinfunc.h"
> +/*
> + * The pin function ID is a tuple of
> + * <mux_reg conf_reg input_reg mux_mode input_val>
> + */
> +#define MX6UL_PAD_ENET2_RX_DATA0__EPDC_SDDO08 0x00E4 0x0370 0x0000 0x9 0x0
Can we name these imx6ull specific defines MX6ULL_xxx, so that we know
they should only be used in imx6ull specific dts?
> +#define MX6UL_PAD_ENET2_RX_DATA1__EPDC_SDDO09 0x00E8 0x0374 0x0000 0x9 0x0
> +#define MX6UL_PAD_ENET2_RX_EN__EPDC_SDDO10 0x00EC 0x0378 0x0000 0x9 0x0
> +#define MX6UL_PAD_ENET2_TX_DATA0__EPDC_SDDO11 0x00F0 0x037C 0x0000 0x9 0x0
> +#define MX6UL_PAD_ENET2_TX_DATA1__EPDC_SDDO12 0x00F4 0x0380 0x0000 0x9 0x0
> +#define MX6UL_PAD_ENET2_TX_EN__EPDC_SDDO13 0x00F8 0x0384 0x0000 0x9 0x0
> +#define MX6UL_PAD_ENET2_TX_CLK__EPDC_SDDO14 0x00FC 0x0388 0x0000 0x9 0x0
> +#define MX6UL_PAD_ENET2_RX_ER__EPDC_SDDO15 0x0100 0x038C 0x0000 0x9 0x0
> +#define MX6UL_PAD_LCD_CLK__EPDC_SDCLK 0x0104 0x0390 0x0000 0x9 0x0
> +#define MX6UL_PAD_LCD_ENABLE__EPDC_SDLE 0x0108 0x0394 0x0000 0x9 0x0
> +#define MX6UL_PAD_LCD_HSYNC__EPDC_SDOE 0x010C 0x0398 0x0000 0x9 0x0
> +#define MX6UL_PAD_LCD_VSYNC__EPDC_SDCE0 0x0110 0x039C 0x0000 0x9 0x0
> +#define MX6UL_PAD_LCD_RESET__EPDC_GDOE 0x0114 0x03A0 0x0000 0x9 0x0
> +#define MX6UL_PAD_LCD_DATA00__EPDC_SDDO00 0x0118 0x03A4 0x0000 0x9 0x0
> +#define MX6UL_PAD_LCD_DATA01__EPDC_SDDO01 0x011C 0x03A8 0x0000 0x9 0x0
> +#define MX6UL_PAD_LCD_DATA02__EPDC_SDDO02 0x0120 0x03AC 0x0000 0x9 0x0
> +#define MX6UL_PAD_LCD_DATA03__EPDC_SDDO03 0x0124 0x03B0 0x0000 0x9 0x0
> +#define MX6UL_PAD_LCD_DATA04__EPDC_SDDO04 0x0128 0x03B4 0x0000 0x9 0x0
> +#define MX6UL_PAD_LCD_DATA05__EPDC_SDDO05 0x012C 0x03B8 0x0000 0x9 0x0
> +#define MX6UL_PAD_LCD_DATA06__EPDC_SDDO06 0x0130 0x03BC 0x0000 0x9 0x0
> +#define MX6UL_PAD_LCD_DATA07__EPDC_SDDO07 0x0134 0x03C0 0x0000 0x9 0x0
> +#define MX6UL_PAD_LCD_DATA14__EPDC_SDSHR 0x0150 0x03DC 0x0000 0x9 0x0
> +#define MX6UL_PAD_LCD_DATA15__EPDC_GDRL 0x0154 0x03E0 0x0000 0x9 0x0
> +#define MX6UL_PAD_LCD_DATA16__EPDC_GDCLK 0x0158 0x03E4 0x0000 0x9 0x0
> +#define MX6UL_PAD_LCD_DATA17__EPDC_GDSP 0x015C 0x03E8 0x0000 0x9 0x0
> +#define MX6UL_PAD_LCD_DATA21__EPDC_SDCE1 0x016C 0x03F8 0x0000 0x9 0x0
> +
Why this new line?
Shawn
> +#define MX6UL_PAD_CSI_MCLK__ESAI_TX3_RX2 0x01D4 0x0460 0x0000 0x9 0x0
> +#define MX6UL_PAD_CSI_PIXCLK__ESAI_TX2_RX3 0x01D8 0x0464 0x0000 0x9 0x0
> +#define MX6UL_PAD_CSI_VSYNC__ESAI_TX4_RX1 0x01DC 0x0468 0x0000 0x9 0x0
> +#define MX6UL_PAD_CSI_HSYNC__ESAI_TX1 0x01E0 0x046C 0x0000 0x9 0x0
> +#define MX6UL_PAD_CSI_DATA00__ESAI_TX_HF_CLK 0x01E4 0x0470 0x0000 0x9 0x0
> +#define MX6UL_PAD_CSI_DATA01__ESAI_RX_HF_CLK 0x01E8 0x0474 0x0000 0x9 0x0
> +#define MX6UL_PAD_CSI_DATA02__ESAI_RX_FS 0x01EC 0x0478 0x0000 0x9 0x0
> +#define MX6UL_PAD_CSI_DATA03__ESAI_RX_CLK 0x01F0 0x047C 0x0000 0x9 0x0
> +#define MX6UL_PAD_CSI_DATA04__ESAI_TX_FS 0x01F4 0x0480 0x0000 0x9 0x0
> +#define MX6UL_PAD_CSI_DATA05__ESAI_TX_CLK 0x01F8 0x0484 0x0000 0x9 0x0
> +#define MX6UL_PAD_CSI_DATA06__ESAI_TX5_RX0 0x01FC 0x0488 0x0000 0x9 0x0
> +#define MX6UL_PAD_CSI_DATA07__ESAI_T0 0x0200 0x048C 0x0000 0x9 0x0
> +
> +#endif /* __DTS_IMX6ULL_PINFUNC_H */
> diff --git a/arch/arm/boot/dts/imx6ull.dtsi b/arch/arm/boot/dts/imx6ull.dtsi
> new file mode 100644
> index 0000000..afd9796
> --- /dev/null
> +++ b/arch/arm/boot/dts/imx6ull.dtsi
> @@ -0,0 +1,10 @@
> +/*
> + * Copyright 2016 Freescale Semiconductor, 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.
> + */
> +
> +#include "imx6u.dtsi"
> +#include "imx6ull-pinfunc.h"
> --
> 2.7.4
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
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 1/2] ARM: dts: imx6sx-sdb: update TX D_CAL for USBPHY
From: Shawn Guo @ 2016-11-05 8:31 UTC (permalink / raw)
To: Peter Chen
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
kernel-bIcnvbaLZ9MEGnE8C9+IrQ, devicetree-u79uwXL29TY76Z2rM5mHXA,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, pawel.moll-5wv7dgnIgG8,
mark.rutland-5wv7dgnIgG8
In-Reply-To: <1477882709-2518-1-git-send-email-peter.chen-3arQi8VN3Tc@public.gmane.org>
On Mon, Oct 31, 2016 at 10:58:28AM +0800, Peter Chen wrote:
> We need to change trimming value (as a percentage) of the 17.78mA TX
> reference current for better signal quality. With this change, we
> can patch the eye-diagram test on this board.
s/patch/pass? I just want to confirm this is a typo, and can fix it up
when applying.
Shawn
>
> Signed-off-by: Peter Chen <peter.chen-3arQi8VN3Tc@public.gmane.org>
> ---
> arch/arm/boot/dts/imx6sx-sdb.dtsi | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/arch/arm/boot/dts/imx6sx-sdb.dtsi b/arch/arm/boot/dts/imx6sx-sdb.dtsi
> index 85f56a5..4933c56 100644
> --- a/arch/arm/boot/dts/imx6sx-sdb.dtsi
> +++ b/arch/arm/boot/dts/imx6sx-sdb.dtsi
> @@ -317,6 +317,14 @@
> status = "okay";
> };
>
> +&usbphy1 {
> + fsl,tx-d-cal = <106>;
> +};
> +
> +&usbphy2 {
> + fsl,tx-d-cal = <106>;
> +};
> +
> &usdhc2 {
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_usdhc2>;
> --
> 2.7.4
>
--
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 2/5] drivers: gpio: Add support for multiple IPs
From: Linus Walleij @ 2016-11-05 8:23 UTC (permalink / raw)
To: Grygorii Strashko
Cc: Keerthy, Roger Quadros, Alexandre Courbot, Lokesh Vutla,
Rob Herring, linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux-OMAP
In-Reply-To: <b92f729b-5897-c29a-31ec-97510776d85d-l0cyMroinI0@public.gmane.org>
On Fri, Nov 4, 2016 at 8:59 PM, Grygorii Strashko
<grygorii.strashko-l0cyMroinI0@public.gmane.org> wrote:
> On 11/04/2016 09:28 AM, Linus Walleij wrote:
>> The DT model sort of mandates how the interrupts should be mapped
>> at this point, and as far as I can tell from the binding the example looks
>> like so:
>>
>> gpio: gpio@1e26000 {
>> compatible = "ti,dm6441-gpio";
>> gpio-controller;
>> #gpio-cells = <2>;
>> reg = <0x226000 0x1000>;
>> interrupt-parent = <&intc>;
>> interrupts = <42 IRQ_TYPE_EDGE_BOTH 43 IRQ_TYPE_EDGE_BOTH
>> 44 IRQ_TYPE_EDGE_BOTH 45 IRQ_TYPE_EDGE_BOTH
>> 46 IRQ_TYPE_EDGE_BOTH 47 IRQ_TYPE_EDGE_BOTH
>> 48 IRQ_TYPE_EDGE_BOTH 49 IRQ_TYPE_EDGE_BOTH
>> 50 IRQ_TYPE_EDGE_BOTH>;
>> ti,ngpio = <144>;
>> ti,davinci-gpio-unbanked = <0>;
>> interrupt-controller;
>> #interrupt-cells = <2>;
>> };
>
> Above, DT bindings models Davinci GPIO IP as monolithic GPIO controller
> with N gpio pins, but internally separate GPIO chips are created for each
> banksX&Y register set (32 pins, 2 banked irq -or- 32 direct irqs).
Hm it would be good to get away from that and just have one big gpio
chip.
> Translation from linear GPIO numbering to the proper internal GPIO chip is done
> using chip.of_xlate().
Yeah :/ this could be made simpler with a single chip just spanning all
the banks and the common registers I think.
Yours,
Linus Walleij
--
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