Devicetree
 help / color / mirror / Atom feed
* [PATCH v2 00/18] Add support for Cirrus Logic CS47L35/L85/L90/L91 codecs
From: Richard Fitzgerald @ 2017-04-24 16:08 UTC (permalink / raw)
  To: lee.jones, broonie, linus.walleij, gnurou, robh+dt, tglx, jason
  Cc: linux-gpio, alsa-devel, patches, linux-kernel, devicetree

The Cirrus Logic CS47L35, CS47L85, CS47L90/91 codecs are complex audio SoC
devices. In addition to the core audio capability they have onboard GPIO,
regulators, DSPs and interrupt controller and a large register map space
accessed over SPI or I2C. This family of codecs is based around common IP
blocks and they are managed by a set of common drivers referred to as "Madera".

Summary of main changes since V1:
- dt bindings moved to separate patches
- pdata struct descriptions fixed to be kerneldoc format
- now re-uses the arizona micsupp and ldo1 regulators
- various fixes and improvements to the pinctrl driver
- gpio driver can now interoperate with the pinctrl driver
- some changes to the way Kconfig options are selected

Richard Fitzgerald (18):
  mfd: madera: Add register definitions for Cirrus Logic Madera codecs
  mfd: madera: Add common support for Cirrus Logic Madera codecs
  dt-bindings: mfd: Add bindings for Cirrus Logic Madera codecs
  mfd: madera: Register map tables for Cirrus Logic CS47L35
  mfd: madera: Register map tables for Cirrus Logic CS47L85
  mfd: madera: Register map tables for Cirrus Logic CS47L90/91
  regulator: arizona-micsupp: Add support for Cirrus Logic Madera codecs
  regulator: arizona-ldo1: Add support for Cirrus Logic Madera codecs
  irqchip: Add driver for Cirrus Logic Madera codecs
  pinctrl: madera: Add driver for Cirrus Logic Madera codecs
  dt-bindings: pinctrl: Add bindings for Cirrus Logic Madera codecs
  gpio: madera: Support Cirrus Logic Madera class codecs
  dt-bindings: gpio: Add bindings for GPIO on Cirrus Logic Madera codecs
  ASoC: madera: Add common support for Cirrus Logic Madera codecs
  dt-bindings: sound: Add bindings for Cirrus Logic Madera codecs
  ASoC: cs47l35: Add codec driver for Cirrus Logic CS47L35
  ASoC: cs47l85: Add codec driver for Cirrus Logic CS47L85
  ASoC: cs47l90: Add codec driver for Cirrus Logic CS47L90

 .../devicetree/bindings/gpio/gpio-madera.txt       |   27 +
 Documentation/devicetree/bindings/mfd/madera.txt   |   96 +
 .../bindings/pinctrl/cirrus,madera-pinctrl.txt     |  102 +
 .../bindings/regulator/arizona-regulator.txt       |    3 +-
 Documentation/devicetree/bindings/sound/madera.txt |   66 +
 MAINTAINERS                                        |   24 +
 drivers/gpio/Kconfig                               |    6 +
 drivers/gpio/Makefile                              |    1 +
 drivers/gpio/gpio-madera.c                         |  196 +
 drivers/irqchip/Kconfig                            |    3 +
 drivers/irqchip/Makefile                           |    1 +
 drivers/irqchip/irq-madera.c                       |  349 +
 drivers/mfd/Kconfig                                |   50 +
 drivers/mfd/Makefile                               |   13 +
 drivers/mfd/cs47l35-tables.c                       | 1688 ++++
 drivers/mfd/cs47l85-tables.c                       | 3169 +++++++
 drivers/mfd/cs47l90-tables.c                       | 2830 +++++++
 drivers/mfd/madera-core.c                          |  635 ++
 drivers/mfd/madera-i2c.c                           |  131 +
 drivers/mfd/madera-spi.c                           |  132 +
 drivers/mfd/madera.h                               |   52 +
 drivers/pinctrl/Kconfig                            |    1 +
 drivers/pinctrl/Makefile                           |    1 +
 drivers/pinctrl/cirrus/Kconfig                     |   15 +
 drivers/pinctrl/cirrus/Makefile                    |   11 +
 drivers/pinctrl/cirrus/pinctrl-cs47l35.c           |   46 +
 drivers/pinctrl/cirrus/pinctrl-cs47l85.c           |   60 +
 drivers/pinctrl/cirrus/pinctrl-cs47l90.c           |   58 +
 drivers/pinctrl/cirrus/pinctrl-madera.c            | 1106 +++
 drivers/pinctrl/cirrus/pinctrl-madera.h            |   40 +
 drivers/regulator/Kconfig                          |   15 +-
 drivers/regulator/arizona-ldo1.c                   |   82 +-
 drivers/regulator/arizona-micsupp.c                |   71 +-
 include/dt-bindings/sound/madera.h                 |   18 +
 include/linux/irqchip/irq-madera-pdata.h           |   19 +
 include/linux/irqchip/irq-madera.h                 |   96 +
 include/linux/mfd/madera/core.h                    |  180 +
 include/linux/mfd/madera/pdata.h                   |   61 +
 include/linux/mfd/madera/registers.h               | 8832 ++++++++++++++++++++
 include/sound/madera-pdata.h                       |   58 +
 sound/soc/codecs/Kconfig                           |   23 +
 sound/soc/codecs/Makefile                          |    8 +
 sound/soc/codecs/cs47l35.c                         | 1747 ++++
 sound/soc/codecs/cs47l85.c                         | 2706 ++++++
 sound/soc/codecs/cs47l90.c                         | 2645 ++++++
 sound/soc/codecs/madera.c                          | 4430 ++++++++++
 sound/soc/codecs/madera.h                          |  470 ++
 47 files changed, 32363 insertions(+), 10 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-madera.txt
 create mode 100644 Documentation/devicetree/bindings/mfd/madera.txt
 create mode 100644 Documentation/devicetree/bindings/pinctrl/cirrus,madera-pinctrl.txt
 create mode 100644 Documentation/devicetree/bindings/sound/madera.txt
 create mode 100644 drivers/gpio/gpio-madera.c
 create mode 100644 drivers/irqchip/irq-madera.c
 create mode 100644 drivers/mfd/cs47l35-tables.c
 create mode 100644 drivers/mfd/cs47l85-tables.c
 create mode 100644 drivers/mfd/cs47l90-tables.c
 create mode 100644 drivers/mfd/madera-core.c
 create mode 100644 drivers/mfd/madera-i2c.c
 create mode 100644 drivers/mfd/madera-spi.c
 create mode 100644 drivers/mfd/madera.h
 create mode 100644 drivers/pinctrl/cirrus/Kconfig
 create mode 100644 drivers/pinctrl/cirrus/Makefile
 create mode 100644 drivers/pinctrl/cirrus/pinctrl-cs47l35.c
 create mode 100644 drivers/pinctrl/cirrus/pinctrl-cs47l85.c
 create mode 100644 drivers/pinctrl/cirrus/pinctrl-cs47l90.c
 create mode 100644 drivers/pinctrl/cirrus/pinctrl-madera.c
 create mode 100644 drivers/pinctrl/cirrus/pinctrl-madera.h
 create mode 100644 include/dt-bindings/sound/madera.h
 create mode 100644 include/linux/irqchip/irq-madera-pdata.h
 create mode 100644 include/linux/irqchip/irq-madera.h
 create mode 100644 include/linux/mfd/madera/core.h
 create mode 100644 include/linux/mfd/madera/pdata.h
 create mode 100644 include/linux/mfd/madera/registers.h
 create mode 100644 include/sound/madera-pdata.h
 create mode 100644 sound/soc/codecs/cs47l35.c
 create mode 100644 sound/soc/codecs/cs47l85.c
 create mode 100644 sound/soc/codecs/cs47l90.c
 create mode 100644 sound/soc/codecs/madera.c
 create mode 100644 sound/soc/codecs/madera.h

-- 
1.9.1

^ permalink raw reply

* Re: [PATCH 3/4] pinctrl: stm32: Implement .get_direction gpio_chip callback
From: Alexandre Torgue @ 2017-04-24 16:07 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Maxime Coquelin, Patrice Chotard, Paul Gortmaker, Rob Herring,
	linux-kernel@vger.kernel.org, linux-gpio@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org
In-Reply-To: <CACRpkdZG-yxMCzBqP2s37QThP7tgWKgd3pVgZGgceB+ihZs3nw@mail.gmail.com>

Hi Linus,

On 04/24/2017 02:37 PM, Linus Walleij wrote:
> On Fri, Apr 7, 2017 at 5:10 PM, Alexandre TORGUE
> <alexandre.torgue@st.com> wrote:
>
>> Add .get_direction() gpiochip callback in STM32 pinctrl driver.
>>
>> Signed-off-by: Alexandre TORGUE <alexandre.torgue@st.com>
>
> (...)
>> +#include <linux/gpio.h>
>
> No this is wrong, drivers should never include this file.
> It is a deprecated consumer header.
>
>> +       if ((alt == 0) && (mode == 0))
>> +               ret = GPIOF_DIR_IN;
>> +       else if ((alt == 0) && (mode == 1))
>> +               ret = GPIOF_DIR_OUT;
>
> Just return 0 or 1, that is the driver-internal API.

Ok. I will fix it in V2.

Thanks
Alex

>
> Yours,
> Linus Walleij
>

^ permalink raw reply

* Re: [PATCH v2 6/7] ARM: Kconfig: Introduce MACH_STM32F469 flag
From: Alexandre Torgue @ 2017-04-24 16:06 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Mark Rutland, devicetree@vger.kernel.org, Arnd Bergmann,
	Jonathan Corbet, Russell King, linux-kernel@vger.kernel.org,
	linux-gpio@vger.kernel.org, Rob Herring, Maxime Coquelin,
	Olof Johansson, Lee Jones, linux-arm-kernel@lists.infradead.org
In-Reply-To: <CACRpkdZmWFKaZWB8qZaH=jB61xdwvJ=TC8idYH9kJdi=HEZRAA@mail.gmail.com>

Hi

On 04/24/2017 02:30 PM, Linus Walleij wrote:
> On Fri, Apr 7, 2017 at 2:43 PM, Alexandre TORGUE
> <alexandre.torgue@st.com> wrote:
>
>> This patch introduces the MACH_STM32F469 to make possible to only select
>> STM32F469 pinctrl driver
>>
>> By default, all the MACH_STM32Fxxx flags will be set with STM32 defconfig.
>>
>> Signed-off-by: Alexandre TORGUE <alexandre.torgue@st.com>
>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
>
> Please funnel this through the ARM SoC tree.

Yes, I'll merge it in future pull request.
Thanks for the series.

Regards
Alex

>
> Yours,
> Linus Walleij
>

^ permalink raw reply

* [PATCH v4 10/10] arm64: allwinner: a64: enable Wi-Fi for Pine64
From: Icenowy Zheng @ 2017-04-24 16:01 UTC (permalink / raw)
  To: Thomas Gleixner, Rob Herring, Maxime Ripard, Chen-Yu Tsai,
	Lee Jones, Liam Girdwood
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Icenowy Zheng
In-Reply-To: <20170424160103.9447-1-icenowy-h8G6r0blFSE@public.gmane.org>

The Wi-Fi module of Pine64 is powered via DLDO4 and ELDO1 (the latter
one provides I/O voltage).

Add device node for it.

Although the Wi-Fi module is an external module which should be inserted
to a header, according to my personal talk with TL Lim, he does not want
this header to be used as GPIO (so it's with 2.0mm pitch, not 2.54mm as
other GPIO headers).

Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
---
 arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
index abc1879e91f2..2e4f44800162 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
@@ -64,6 +64,11 @@
 		regulator-min-microvolt = <3300000>;
 		regulator-max-microvolt = <3300000>;
 	};
+
+	wifi_pwrseq: wifi_pwrseq {
+		compatible = "mmc-pwrseq-simple";
+		reset-gpios = <&r_pio 0 2 GPIO_ACTIVE_LOW>; /* PL2 */
+	};
 };
 
 &ehci1 {
@@ -91,6 +96,17 @@
 	status = "okay";
 };
 
+&mmc1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&mmc1_pins>;
+	vmmc-supply = <&reg_dldo4>;
+	vqmmc-supply = <&reg_eldo1>;
+	mmc-pwrseq = <&wifi_pwrseq>;
+	non-removable;
+	bus-width = <4>;
+	status = "okay";
+};
+
 &ohci1 {
 	status = "okay";
 };
-- 
2.12.2

^ permalink raw reply related

* [PATCH v4 09/10] arm64: allwinner: a64: enable AXP803 regulators for Pine64
From: Icenowy Zheng @ 2017-04-24 16:01 UTC (permalink / raw)
  To: Thomas Gleixner, Rob Herring, Maxime Ripard, Chen-Yu Tsai,
	Lee Jones, Liam Girdwood
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Icenowy Zheng
In-Reply-To: <20170424160103.9447-1-icenowy-h8G6r0blFSE@public.gmane.org>

Add support of AXP803 regulators in the Pine64 device tree, in order to
enable many future functionalities, e.g. Wi-Fi.

Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
---
 .../arm64/boot/dts/allwinner/sun50i-a64-pine64.dts | 109 +++++++++++++++++++++
 1 file changed, 109 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
index 3e1b44292534..abc1879e91f2 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
@@ -106,6 +106,115 @@
 	};
 };
 
+#include "axp803.dtsi"
+
+&reg_aldo1 {
+	regulator-min-microvolt = <2800000>;
+	regulator-max-microvolt = <2800000>;
+	regulator-name = "vcc-csi";
+};
+
+&reg_aldo2 {
+	regulator-always-on;
+	regulator-min-microvolt = <1800000>;
+	regulator-max-microvolt = <3300000>;
+	regulator-name = "vcc-pl";
+};
+
+&reg_aldo3 {
+	regulator-always-on;
+	regulator-min-microvolt = <2700000>;
+	regulator-max-microvolt = <3300000>;
+	regulator-name = "vcc-pll-avcc";
+};
+
+&reg_dc1sw {
+	regulator-name = "vcc-phy";
+};
+
+&reg_dcdc1 {
+	regulator-always-on;
+	regulator-min-microvolt = <3300000>;
+	regulator-max-microvolt = <3300000>;
+	regulator-name = "vcc-3v3";
+};
+
+&reg_dcdc2 {
+	regulator-always-on;
+	regulator-min-microvolt = <1000000>;
+	regulator-max-microvolt = <1300000>;
+	regulator-name = "vdd-cpux";
+};
+
+/* DCDC3 is polyphased with DCDC2 */
+
+&reg_dcdc5 {
+	regulator-always-on;
+	regulator-min-microvolt = <1500000>;
+	regulator-max-microvolt = <1500000>;
+	regulator-name = "vcc-dram";
+};
+
+&reg_dcdc6 {
+	regulator-always-on;
+	regulator-min-microvolt = <1100000>;
+	regulator-max-microvolt = <1100000>;
+	regulator-name = "vdd-sys";
+};
+
+&reg_dldo1 {
+	regulator-min-microvolt = <3300000>;
+	regulator-max-microvolt = <3300000>;
+	regulator-name = "vcc-hdmi";
+};
+
+&reg_dldo2 {
+	regulator-min-microvolt = <3300000>;
+	regulator-max-microvolt = <3300000>;
+	regulator-name = "vcc-mipi";
+};
+
+&reg_dldo3 {
+	regulator-min-microvolt = <3300000>;
+	regulator-max-microvolt = <3300000>;
+	regulator-name = "avdd-csi";
+};
+
+&reg_dldo4 {
+	regulator-min-microvolt = <3300000>;
+	regulator-max-microvolt = <3300000>;
+	regulator-name = "vcc-wifi";
+};
+
+&reg_eldo1 {
+	regulator-min-microvolt = <1800000>;
+	regulator-max-microvolt = <1800000>;
+	regulator-name = "cpvdd";
+};
+
+&reg_eldo3 {
+	regulator-min-microvolt = <1800000>;
+	regulator-max-microvolt = <1800000>;
+	regulator-name = "vdd-1v8-csi";
+};
+
+&reg_fldo1 {
+	regulator-min-microvolt = <1200000>;
+	regulator-max-microvolt = <1200000>;
+	regulator-name = "vcc-1v2-hsic";
+};
+
+&reg_fldo2 {
+	regulator-always-on;
+	regulator-min-microvolt = <1100000>;
+	regulator-max-microvolt = <1100000>;
+	regulator-name = "vdd-cpus";
+};
+
+&reg_rtc_ldo {
+	regulator-name = "vcc-rtc";
+};
+
 &uart0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&uart0_pins_a>;
-- 
2.12.2

^ permalink raw reply related

* [PATCH v4 08/10] arm64: allwinner: a64: add DTSI file for AXP803 PMIC
From: Icenowy Zheng @ 2017-04-24 16:01 UTC (permalink / raw)
  To: Thomas Gleixner, Rob Herring, Maxime Ripard, Chen-Yu Tsai,
	Lee Jones, Liam Girdwood
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Icenowy Zheng
In-Reply-To: <20170424160103.9447-1-icenowy-h8G6r0blFSE@public.gmane.org>

As nearly all A64 boards are using AXP803 PMIC, add a DTSI file for it,
like the old DTSI files for AXP20x/22x, for the common parts of the
PMIC.

Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
---
Changes in v4:
- Re-sorted the nodes.

 arch/arm64/boot/dts/allwinner/axp803.dtsi | 150 ++++++++++++++++++++++++++++++
 1 file changed, 150 insertions(+)
 create mode 100644 arch/arm64/boot/dts/allwinner/axp803.dtsi

diff --git a/arch/arm64/boot/dts/allwinner/axp803.dtsi b/arch/arm64/boot/dts/allwinner/axp803.dtsi
new file mode 100644
index 000000000000..ff8af52743ff
--- /dev/null
+++ b/arch/arm64/boot/dts/allwinner/axp803.dtsi
@@ -0,0 +1,150 @@
+/*
+ * Copyright 2017 Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.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 free software; you can redistribute it and/or
+ *     modify it under the terms of the GNU General Public License as
+ *     published by the Free Software Foundation; either version 2 of the
+ *     License, or (at your option) any later version.
+ *
+ *     This file 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.
+ *
+ * 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.
+ */
+
+/*
+ * AXP803 Integrated Power Management Chip
+ * http://files.pine64.org/doc/datasheet/pine64/AXP803_Datasheet_V1.0.pdf
+ */
+
+&axp803 {
+	interrupt-controller;
+	#interrupt-cells = <1>;
+
+	regulators {
+		/* Default work frequency for buck regulators */
+		x-powers,dcdc-freq = <3000>;
+
+		reg_aldo1: aldo1 {
+			regulator-name = "aldo1";
+		};
+
+		reg_aldo2: aldo2 {
+			regulator-name = "aldo2";
+		};
+
+		reg_aldo3: aldo3 {
+			regulator-name = "aldo3";
+		};
+
+		reg_dc1sw: dc1sw {
+			regulator-name = "dc1sw";
+		};
+
+		reg_dcdc1: dcdc1 {
+			regulator-name = "dcdc1";
+		};
+
+		reg_dcdc2: dcdc2 {
+			regulator-name = "dcdc2";
+		};
+
+		reg_dcdc3: dcdc3 {
+			regulator-name = "dcdc3";
+		};
+
+		reg_dcdc4: dcdc4 {
+			regulator-name = "dcdc4";
+		};
+
+		reg_dcdc5: dcdc5 {
+			regulator-name = "dcdc5";
+		};
+
+		reg_dcdc6: dcdc6 {
+			regulator-name = "dcdc6";
+		};
+
+		reg_dldo1: dldo1 {
+			regulator-name = "dldo1";
+		};
+
+		reg_dldo2: dldo2 {
+			regulator-name = "dldo2";
+		};
+
+		reg_dldo3: dldo3 {
+			regulator-name = "dldo3";
+		};
+
+		reg_dldo4: dldo4 {
+			regulator-name = "dldo4";
+		};
+
+		reg_eldo1: eldo1 {
+			regulator-name = "eldo1";
+		};
+
+		reg_eldo2: eldo2 {
+			regulator-name = "eldo2";
+		};
+
+		reg_eldo3: eldo3 {
+			regulator-name = "eldo3";
+		};
+
+		reg_fldo1: fldo1 {
+			regulator-name = "fldo1";
+		};
+
+		reg_fldo2: fldo2 {
+			regulator-name = "fldo2";
+		};
+
+		reg_ldo_io0: ldo-io0 {
+			regulator-name = "ldo-io0";
+			status = "disabled";
+		};
+
+		reg_ldo_io1: ldo-io1 {
+			regulator-name = "ldo-io1";
+			status = "disabled";
+		};
+
+		reg_rtc_ldo: rtc-ldo {
+			/* RTC_LDO is a fixed, always-on regulator */
+			regulator-always-on;
+			regulator-min-microvolt = <3000000>;
+			regulator-max-microvolt = <3000000>;
+			regulator-name = "rtc-ldo";
+		};
+	};
+};
-- 
2.12.2

^ permalink raw reply related

* [PATCH v4 07/10] mfd: axp20x: add axp20x-regulator cell for AXP803
From: Icenowy Zheng @ 2017-04-24 16:01 UTC (permalink / raw)
  To: Thomas Gleixner, Rob Herring, Maxime Ripard, Chen-Yu Tsai,
	Lee Jones, Liam Girdwood
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Icenowy Zheng
In-Reply-To: <20170424160103.9447-1-icenowy-h8G6r0blFSE@public.gmane.org>

As axp20x-regulator now supports AXP803, add a cell for it.

Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
Acked-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
---
Changes in v4:
- Added a trailing comma for new cell, for easier further cell addition.
Changes in v3:
- Make the new cell one-liner.

 drivers/mfd/axp20x.c                 | 3 ++-
 drivers/regulator/axp20x-regulator.c | 6 +++---
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
index 1dc6235778eb..917b6ddc4f15 100644
--- a/drivers/mfd/axp20x.c
+++ b/drivers/mfd/axp20x.c
@@ -848,7 +848,8 @@ static struct mfd_cell axp803_cells[] = {
 		.name			= "axp20x-pek",
 		.num_resources		= ARRAY_SIZE(axp803_pek_resources),
 		.resources		= axp803_pek_resources,
-	}
+	},
+	{	.name			= "axp20x-regulator" },
 };
 
 static struct mfd_cell axp806_cells[] = {
diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
index 9356ec8a9a1f..e2608fe770b9 100644
--- a/drivers/regulator/axp20x-regulator.c
+++ b/drivers/regulator/axp20x-regulator.c
@@ -311,13 +311,13 @@ static const struct regulator_desc axp803_regulators[] = {
 		 AXP803_FLDO1_V_OUT, 0x0f, AXP22X_PWR_OUT_CTRL3, BIT(2)),
 	AXP_DESC(AXP803, FLDO2, "fldo2", "fldoin", 700, 1450, 50,
 		 AXP803_FLDO2_V_OUT, 0x0f, AXP22X_PWR_OUT_CTRL3, BIT(3)),
-	AXP_DESC_IO(AXP803, LDO_IO0, "ldo_io0", "ips", 700, 3300, 100,
+	AXP_DESC_IO(AXP803, LDO_IO0, "ldo-io0", "ips", 700, 3300, 100,
 		    AXP22X_LDO_IO0_V_OUT, 0x1f, AXP20X_GPIO0_CTRL, 0x07,
 		    AXP22X_IO_ENABLED, AXP22X_IO_DISABLED),
-	AXP_DESC_IO(AXP803, LDO_IO1, "ldo_io1", "ips", 700, 3300, 100,
+	AXP_DESC_IO(AXP803, LDO_IO1, "ldo-io1", "ips", 700, 3300, 100,
 		    AXP22X_LDO_IO1_V_OUT, 0x1f, AXP20X_GPIO1_CTRL, 0x07,
 		    AXP22X_IO_ENABLED, AXP22X_IO_DISABLED),
-	AXP_DESC_FIXED(AXP803, RTC_LDO, "rtc_ldo", "ips", 3000),
+	AXP_DESC_FIXED(AXP803, RTC_LDO, "rtc-ldo", "ips", 3000),
 };
 
 static const struct regulator_linear_range axp806_dcdca_ranges[] = {
-- 
2.12.2

^ permalink raw reply related

* [PATCH v4 06/10] regulator: axp20x-regulator: add support for AXP803
From: Icenowy Zheng @ 2017-04-24 16:00 UTC (permalink / raw)
  To: Thomas Gleixner, Rob Herring, Maxime Ripard, Chen-Yu Tsai,
	Lee Jones, Liam Girdwood
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Icenowy Zheng
In-Reply-To: <20170424160103.9447-1-icenowy-h8G6r0blFSE@public.gmane.org>

AXP803 PMIC also have a series of regulators (DCDCs and LDOs)
controllable via I2C/RSB bus.

Add support for them.

Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
Acked-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
---
Changes in v4:
- Fixed somewhere which mention AXP806 before 803.
Changes in v2:
- Place AXP803 codes before AXP806/809 ones.
- Fixed some errors in regulator description.
- Reuse AXP803 DLDO2 range for AXP806 CLDO2 & AXP809 DLDO1.

 drivers/regulator/axp20x-regulator.c | 153 ++++++++++++++++++++++++++++++-----
 include/linux/mfd/axp20x.h           |  37 +++++++++
 2 files changed, 168 insertions(+), 22 deletions(-)

diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
index 0b9d4e3e52c7..9356ec8a9a1f 100644
--- a/drivers/regulator/axp20x-regulator.c
+++ b/drivers/regulator/axp20x-regulator.c
@@ -244,6 +244,82 @@ static const struct regulator_desc axp22x_drivevbus_regulator = {
 	.ops		= &axp20x_ops_sw,
 };
 
+static const struct regulator_linear_range axp803_dcdc234_ranges[] = {
+	REGULATOR_LINEAR_RANGE(500000, 0x0, 0x46, 10000),
+	REGULATOR_LINEAR_RANGE(1220000, 0x47, 0x4b, 20000),
+};
+
+static const struct regulator_linear_range axp803_dcdc5_ranges[] = {
+	REGULATOR_LINEAR_RANGE(800000, 0x0, 0x20, 10000),
+	REGULATOR_LINEAR_RANGE(1140000, 0x21, 0x44, 20000),
+};
+
+static const struct regulator_linear_range axp803_dcdc6_ranges[] = {
+	REGULATOR_LINEAR_RANGE(600000, 0x0, 0x32, 10000),
+	REGULATOR_LINEAR_RANGE(1120000, 0x33, 0x47, 20000),
+};
+
+/* AXP806's CLDO2 and AXP809's DLDO1 shares the same range */
+static const struct regulator_linear_range axp803_dldo2_ranges[] = {
+	REGULATOR_LINEAR_RANGE(700000, 0x0, 0x1a, 100000),
+	REGULATOR_LINEAR_RANGE(3400000, 0x1b, 0x1f, 200000),
+};
+
+static const struct regulator_desc axp803_regulators[] = {
+	AXP_DESC(AXP803, DCDC1, "dcdc1", "vin1", 1600, 3400, 100,
+		 AXP803_DCDC1_V_OUT, 0x1f, AXP22X_PWR_OUT_CTRL1, BIT(0)),
+	AXP_DESC_RANGES(AXP803, DCDC2, "dcdc2", "vin2", axp803_dcdc234_ranges,
+			76, AXP803_DCDC2_V_OUT, 0x7f, AXP22X_PWR_OUT_CTRL1,
+			BIT(1)),
+	AXP_DESC_RANGES(AXP803, DCDC3, "dcdc3", "vin3", axp803_dcdc234_ranges,
+			76, AXP803_DCDC3_V_OUT, 0x7f, AXP22X_PWR_OUT_CTRL1,
+			BIT(2)),
+	AXP_DESC_RANGES(AXP803, DCDC4, "dcdc4", "vin4", axp803_dcdc234_ranges,
+			76, AXP803_DCDC4_V_OUT, 0x7f, AXP22X_PWR_OUT_CTRL1,
+			BIT(3)),
+	AXP_DESC_RANGES(AXP803, DCDC5, "dcdc5", "vin5", axp803_dcdc5_ranges,
+			68, AXP803_DCDC5_V_OUT, 0x7f, AXP22X_PWR_OUT_CTRL1,
+			BIT(4)),
+	AXP_DESC_RANGES(AXP803, DCDC6, "dcdc6", "vin6", axp803_dcdc6_ranges,
+			72, AXP803_DCDC6_V_OUT, 0x7f, AXP22X_PWR_OUT_CTRL1,
+			BIT(5)),
+	/* secondary switchable output of DCDC1 */
+	AXP_DESC_SW(AXP803, DC1SW, "dc1sw", NULL, AXP22X_PWR_OUT_CTRL2,
+		    BIT(7)),
+	AXP_DESC(AXP803, ALDO1, "aldo1", "aldoin", 700, 3300, 100,
+		 AXP22X_ALDO1_V_OUT, 0x1f, AXP22X_PWR_OUT_CTRL3, BIT(5)),
+	AXP_DESC(AXP803, ALDO2, "aldo2", "aldoin", 700, 3300, 100,
+		 AXP22X_ALDO2_V_OUT, 0x1f, AXP22X_PWR_OUT_CTRL3, BIT(6)),
+	AXP_DESC(AXP803, ALDO3, "aldo3", "aldoin", 700, 3300, 100,
+		 AXP22X_ALDO3_V_OUT, 0x1f, AXP22X_PWR_OUT_CTRL3, BIT(7)),
+	AXP_DESC(AXP803, DLDO1, "dldo1", "dldoin", 700, 3300, 100,
+		 AXP22X_DLDO1_V_OUT, 0x1f, AXP22X_PWR_OUT_CTRL2, BIT(3)),
+	AXP_DESC_RANGES(AXP803, DLDO2, "dldo2", "dldoin", axp803_dldo2_ranges,
+			32, AXP22X_DLDO2_V_OUT, 0x1f, AXP22X_PWR_OUT_CTRL2,
+			BIT(4)),
+	AXP_DESC(AXP803, DLDO3, "dldo3", "dldoin", 700, 3300, 100,
+		 AXP22X_DLDO3_V_OUT, 0x1f, AXP22X_PWR_OUT_CTRL2, BIT(5)),
+	AXP_DESC(AXP803, DLDO4, "dldo4", "dldoin", 700, 3300, 100,
+		 AXP22X_DLDO4_V_OUT, 0x1f, AXP22X_PWR_OUT_CTRL2, BIT(6)),
+	AXP_DESC(AXP803, ELDO1, "eldo1", "eldoin", 700, 1900, 50,
+		 AXP22X_ELDO1_V_OUT, 0x1f, AXP22X_PWR_OUT_CTRL2, BIT(0)),
+	AXP_DESC(AXP803, ELDO2, "eldo2", "eldoin", 700, 1900, 50,
+		 AXP22X_ELDO2_V_OUT, 0x1f, AXP22X_PWR_OUT_CTRL2, BIT(1)),
+	AXP_DESC(AXP803, ELDO3, "eldo3", "eldoin", 700, 1900, 50,
+		 AXP22X_ELDO3_V_OUT, 0x1f, AXP22X_PWR_OUT_CTRL2, BIT(2)),
+	AXP_DESC(AXP803, FLDO1, "fldo1", "fldoin", 700, 1450, 50,
+		 AXP803_FLDO1_V_OUT, 0x0f, AXP22X_PWR_OUT_CTRL3, BIT(2)),
+	AXP_DESC(AXP803, FLDO2, "fldo2", "fldoin", 700, 1450, 50,
+		 AXP803_FLDO2_V_OUT, 0x0f, AXP22X_PWR_OUT_CTRL3, BIT(3)),
+	AXP_DESC_IO(AXP803, LDO_IO0, "ldo_io0", "ips", 700, 3300, 100,
+		    AXP22X_LDO_IO0_V_OUT, 0x1f, AXP20X_GPIO0_CTRL, 0x07,
+		    AXP22X_IO_ENABLED, AXP22X_IO_DISABLED),
+	AXP_DESC_IO(AXP803, LDO_IO1, "ldo_io1", "ips", 700, 3300, 100,
+		    AXP22X_LDO_IO1_V_OUT, 0x1f, AXP20X_GPIO1_CTRL, 0x07,
+		    AXP22X_IO_ENABLED, AXP22X_IO_DISABLED),
+	AXP_DESC_FIXED(AXP803, RTC_LDO, "rtc_ldo", "ips", 3000),
+};
+
 static const struct regulator_linear_range axp806_dcdca_ranges[] = {
 	REGULATOR_LINEAR_RANGE(600000, 0x0, 0x32, 10000),
 	REGULATOR_LINEAR_RANGE(1120000, 0x33, 0x47, 20000),
@@ -254,11 +330,6 @@ static const struct regulator_linear_range axp806_dcdcd_ranges[] = {
 	REGULATOR_LINEAR_RANGE(1600000, 0x2e, 0x3f, 100000),
 };
 
-static const struct regulator_linear_range axp806_cldo2_ranges[] = {
-	REGULATOR_LINEAR_RANGE(700000, 0x0, 0x1a, 100000),
-	REGULATOR_LINEAR_RANGE(3400000, 0x1b, 0x1f, 200000),
-};
-
 static const struct regulator_desc axp806_regulators[] = {
 	AXP_DESC_RANGES(AXP806, DCDCA, "dcdca", "vina", axp806_dcdca_ranges,
 			72, AXP806_DCDCA_V_CTRL, 0x7f, AXP806_PWR_OUT_CTRL1,
@@ -289,7 +360,7 @@ static const struct regulator_desc axp806_regulators[] = {
 		 AXP806_BLDO4_V_CTRL, 0x0f, AXP806_PWR_OUT_CTRL2, BIT(3)),
 	AXP_DESC(AXP806, CLDO1, "cldo1", "cldoin", 700, 3300, 100,
 		 AXP806_CLDO1_V_CTRL, 0x1f, AXP806_PWR_OUT_CTRL2, BIT(4)),
-	AXP_DESC_RANGES(AXP806, CLDO2, "cldo2", "cldoin", axp806_cldo2_ranges,
+	AXP_DESC_RANGES(AXP806, CLDO2, "cldo2", "cldoin", axp803_dldo2_ranges,
 			32, AXP806_CLDO2_V_CTRL, 0x1f, AXP806_PWR_OUT_CTRL2,
 			BIT(5)),
 	AXP_DESC(AXP806, CLDO3, "cldo3", "cldoin", 700, 3300, 100,
@@ -326,7 +397,7 @@ static const struct regulator_desc axp809_regulators[] = {
 		 AXP22X_ALDO2_V_OUT, 0x1f, AXP22X_PWR_OUT_CTRL1, BIT(7)),
 	AXP_DESC(AXP809, ALDO3, "aldo3", "aldoin", 700, 3300, 100,
 		 AXP22X_ALDO3_V_OUT, 0x1f, AXP22X_PWR_OUT_CTRL2, BIT(5)),
-	AXP_DESC_RANGES(AXP809, DLDO1, "dldo1", "dldoin", axp806_cldo2_ranges,
+	AXP_DESC_RANGES(AXP809, DLDO1, "dldo1", "dldoin", axp803_dldo2_ranges,
 			32, AXP22X_DLDO1_V_OUT, 0x1f, AXP22X_PWR_OUT_CTRL2,
 			BIT(3)),
 	AXP_DESC(AXP809, DLDO2, "dldo2", "dldoin", 700, 3300, 100,
@@ -369,14 +440,21 @@ static int axp20x_set_dcdc_freq(struct platform_device *pdev, u32 dcdcfreq)
 		def = 1500;
 		step = 75;
 		break;
-	case AXP806_ID:
+	case AXP803_ID:
 		/*
-		 * AXP806 DCDC work frequency setting has the same range and
+		 * AXP803 DCDC work frequency setting has the same range and
 		 * step as AXP22X, but at a different register.
 		 * Fall through to the check below.
 		 * (See include/linux/mfd/axp20x.h)
 		 */
-		reg = AXP806_DCDC_FREQ_CTRL;
+		reg = AXP803_DCDC_FREQ_CTRL;
+	case AXP806_ID:
+		/*
+		 * AXP806 also have DCDC work frequency setting register at a
+		 * different position.
+		 */
+		if (axp20x->variant == AXP806_ID)
+			reg = AXP806_DCDC_FREQ_CTRL;
 	case AXP221_ID:
 	case AXP223_ID:
 	case AXP809_ID:
@@ -475,6 +553,14 @@ static int axp20x_set_dcdc_workmode(struct regulator_dev *rdev, int id, u32 work
 		workmode <<= id - AXP22X_DCDC1;
 		break;
 
+	case AXP803_ID:
+		if (id < AXP803_DCDC1 || id > AXP803_DCDC6)
+			return -EINVAL;
+
+		mask = AXP22X_WORKMODE_DCDCX_MASK(id - AXP803_DCDC1);
+		workmode <<= id - AXP803_DCDC1;
+		break;
+
 	default:
 		/* should not happen */
 		WARN_ON(1);
@@ -492,20 +578,38 @@ static bool axp20x_is_polyphase_slave(struct axp20x_dev *axp20x, int id)
 {
 	u32 reg = 0;
 
-	/* Only AXP806 has poly-phase outputs */
-	if (axp20x->variant != AXP806_ID)
-		return false;
+	/*
+	 * Currently in our supported AXP variants, only AXP803 and AXP806
+	 * have polyphase regulators.
+	 */
+	switch (axp20x->variant) {
+	case AXP803_ID:
+		regmap_read(axp20x->regmap, AXP803_POLYPHASE_CTRL, &reg);
+
+		switch (id) {
+		case AXP803_DCDC3:
+			return !!(reg & BIT(6));
+		case AXP803_DCDC6:
+			return !!(reg & BIT(7));
+		}
+		break;
 
-	regmap_read(axp20x->regmap, AXP806_DCDC_MODE_CTRL2, &reg);
+	case AXP806_ID:
+		regmap_read(axp20x->regmap, AXP806_DCDC_MODE_CTRL2, &reg);
+
+		switch (id) {
+		case AXP806_DCDCB:
+			return (((reg & GENMASK(7, 6)) == BIT(6)) ||
+				((reg & GENMASK(7, 6)) == BIT(7)));
+		case AXP806_DCDCC:
+			return ((reg & GENMASK(7, 6)) == BIT(7));
+		case AXP806_DCDCE:
+			return !!(reg & BIT(5));
+		}
+		break;
 
-	switch (id) {
-	case AXP806_DCDCB:
-		return (((reg & GENMASK(7, 6)) == BIT(6)) ||
-			((reg & GENMASK(7, 6)) == BIT(7)));
-	case AXP806_DCDCC:
-		return ((reg & GENMASK(7, 6)) == BIT(7));
-	case AXP806_DCDCE:
-		return !!(reg & BIT(5));
+	default:
+		return false;
 	}
 
 	return false;
@@ -540,6 +644,10 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
 		drivevbus = of_property_read_bool(pdev->dev.parent->of_node,
 						  "x-powers,drive-vbus-en");
 		break;
+	case AXP803_ID:
+		regulators = axp803_regulators;
+		nregulators = AXP803_REG_ID_MAX;
+		break;
 	case AXP806_ID:
 		regulators = axp806_regulators;
 		nregulators = AXP806_REG_ID_MAX;
@@ -579,6 +687,7 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
 		 * name.
 		 */
 		if ((regulators == axp22x_regulators && i == AXP22X_DC1SW) ||
+		    (regulators == axp803_regulators && i == AXP803_DC1SW) ||
 		    (regulators == axp809_regulators && i == AXP809_DC1SW)) {
 			new_desc = devm_kzalloc(&pdev->dev, sizeof(*desc),
 						GFP_KERNEL);
diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
index cde56cfe8446..965b027e31b3 100644
--- a/include/linux/mfd/axp20x.h
+++ b/include/linux/mfd/axp20x.h
@@ -119,6 +119,17 @@ enum axp20x_variants {
 #define AXP806_BUS_ADDR_EXT		0xfe
 #define AXP806_REG_ADDR_EXT		0xff
 
+#define AXP803_POLYPHASE_CTRL		0x14
+#define AXP803_FLDO1_V_OUT		0x1c
+#define AXP803_FLDO2_V_OUT		0x1d
+#define AXP803_DCDC1_V_OUT		0x20
+#define AXP803_DCDC2_V_OUT		0x21
+#define AXP803_DCDC3_V_OUT		0x22
+#define AXP803_DCDC4_V_OUT		0x23
+#define AXP803_DCDC5_V_OUT		0x24
+#define AXP803_DCDC6_V_OUT		0x25
+#define AXP803_DCDC_FREQ_CTRL		0x3b
+
 /* Interrupt */
 #define AXP152_IRQ1_EN			0x40
 #define AXP152_IRQ2_EN			0x41
@@ -350,6 +361,32 @@ enum {
 	AXP809_REG_ID_MAX,
 };
 
+enum {
+	AXP803_DCDC1 = 0,
+	AXP803_DCDC2,
+	AXP803_DCDC3,
+	AXP803_DCDC4,
+	AXP803_DCDC5,
+	AXP803_DCDC6,
+	AXP803_DC1SW,
+	AXP803_ALDO1,
+	AXP803_ALDO2,
+	AXP803_ALDO3,
+	AXP803_DLDO1,
+	AXP803_DLDO2,
+	AXP803_DLDO3,
+	AXP803_DLDO4,
+	AXP803_ELDO1,
+	AXP803_ELDO2,
+	AXP803_ELDO3,
+	AXP803_FLDO1,
+	AXP803_FLDO2,
+	AXP803_RTC_LDO,
+	AXP803_LDO_IO0,
+	AXP803_LDO_IO1,
+	AXP803_REG_ID_MAX,
+};
+
 /* IRQs */
 enum {
 	AXP152_IRQ_LDO0IN_CONNECT = 1,
-- 
2.12.2

^ permalink raw reply related

* [PATCH v4 05/10] arm64: allwinner: a64: add AXP803 node to Pine64 device tree
From: Icenowy Zheng @ 2017-04-24 16:00 UTC (permalink / raw)
  To: Thomas Gleixner, Rob Herring, Maxime Ripard, Chen-Yu Tsai,
	Lee Jones, Liam Girdwood
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Icenowy Zheng
In-Reply-To: <20170424160103.9447-1-icenowy-h8G6r0blFSE@public.gmane.org>

The Pine64 (including Pine64+) boards have an AXP803 as its main PMIC.

Add its device node.

Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
---
 arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
index c680ed385da3..3e1b44292534 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
@@ -95,6 +95,17 @@
 	status = "okay";
 };
 
+&r_rsb {
+	status = "okay";
+
+	axp803: pmic@3a3 {
+		compatible = "x-powers,axp803";
+		reg = <0x3a3>;
+		interrupt-parent = <&r_intc>;
+		interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
+	};
+};
+
 &uart0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&uart0_pins_a>;
-- 
2.12.2

^ permalink raw reply related

* [PATCH v4 04/10] arm64: allwinner: a64: add NMI (R_INTC) controller on A64
From: Icenowy Zheng @ 2017-04-24 16:00 UTC (permalink / raw)
  To: Thomas Gleixner, Rob Herring, Maxime Ripard, Chen-Yu Tsai,
	Lee Jones, Liam Girdwood
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Icenowy Zheng
In-Reply-To: <20170424160103.9447-1-icenowy-h8G6r0blFSE@public.gmane.org>

Allwinner A64 SoC features a R_INTC controller, which controls the NMI
line, and this interrupt line is usually connected to the AXP PMIC.

Add support for it.

Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
---
Changes in v4:
- Changes it to use R_INTC binding and change node label to r_intc.
- Fixed MMIO region.
- Dropped Chen-Yu's ACK due to big change.
Changes in v2:
- Added Chen-Yu's ACK.

 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index 05ec9fc5e81f..a6566d292934 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -403,6 +403,14 @@
 				     <GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH>;
 		};
 
+		r_intc: interrupt-controller@1f00c00 {
+			compatible = "allwinner,sun50i-a64-r-intc";
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			reg = <0x01f00c00 0x400>;
+			interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
+		};
+
 		r_ccu: clock@1f01400 {
 			compatible = "allwinner,sun50i-a64-r-ccu";
 			reg = <0x01f01400 0x100>;
-- 
2.12.2

^ permalink raw reply related

* [PATCH v4 03/10] irqchip/sunxi-nmi: add support for the NMI in A64 R_INTC
From: Icenowy Zheng @ 2017-04-24 16:00 UTC (permalink / raw)
  To: Thomas Gleixner, Rob Herring, Maxime Ripard, Chen-Yu Tsai,
	Lee Jones, Liam Girdwood
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Icenowy Zheng
In-Reply-To: <20170424160103.9447-1-icenowy-h8G6r0blFSE@public.gmane.org>

Add support for the newly imported compatible for the A64 R_INTC in
irq-sunxi-nmi driver

Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
---
New patch in v4, which is part of NMI refactor.

 drivers/irqchip/irq-sunxi-nmi.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/irqchip/irq-sunxi-nmi.c b/drivers/irqchip/irq-sunxi-nmi.c
index 668730c5cb66..5f7408b1cd4d 100644
--- a/drivers/irqchip/irq-sunxi-nmi.c
+++ b/drivers/irqchip/irq-sunxi-nmi.c
@@ -56,6 +56,12 @@ static struct sunxi_sc_nmi_reg_offs sun9i_reg_offs = {
 	.enable	= 0x04,
 };
 
+static struct sunxi_sc_nmi_reg_offs sun50i_reg_offs = {
+	.ctrl	= 0x0c,
+	.pend	= 0x10,
+	.enable	= 0x40,
+};
+
 static inline void sunxi_sc_nmi_write(struct irq_chip_generic *gc, u32 off,
 				      u32 val)
 {
@@ -220,3 +226,10 @@ static int __init sun9i_nmi_irq_init(struct device_node *node,
 	return sunxi_sc_nmi_irq_init(node, &sun9i_reg_offs);
 }
 IRQCHIP_DECLARE(sun9i_nmi, "allwinner,sun9i-a80-nmi", sun9i_nmi_irq_init);
+
+static int __init sun50i_nmi_irq_init(struct device_node *node,
+				     struct device_node *parent)
+{
+	return sunxi_sc_nmi_irq_init(node, &sun50i_reg_offs);
+}
+IRQCHIP_DECLARE(sun50i_nmi, "allwinner,sun50i-a80-nmi", sun50i_nmi_irq_init);
-- 
2.12.2

^ permalink raw reply related

* [PATCH v4 02/10] irqchip/sunxi-nmi: add A64 R_INTC to the binding doc
From: Icenowy Zheng @ 2017-04-24 16:00 UTC (permalink / raw)
  To: Thomas Gleixner, Rob Herring, Maxime Ripard, Chen-Yu Tsai,
	Lee Jones, Liam Girdwood
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Icenowy Zheng
In-Reply-To: <20170424160103.9447-1-icenowy-h8G6r0blFSE@public.gmane.org>

The A31 NMI driver seems to be using wrong base address.

As we're going to convert to use a correct NMI base address (and
correctly name it to R_INTC as the datasheet suggests), add a new
compatible string for the "correct" R_INTC, which we will use for A64
SoC.

Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
---
New patch in v4, which is part of NMI refactor.

 .../bindings/interrupt-controller/allwinner,sunxi-nmi.txt          | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/allwinner,sunxi-nmi.txt b/Documentation/devicetree/bindings/interrupt-controller/allwinner,sunxi-nmi.txt
index 81cd3692405e..fea0c6a6211f 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/allwinner,sunxi-nmi.txt
+++ b/Documentation/devicetree/bindings/interrupt-controller/allwinner,sunxi-nmi.txt
@@ -3,8 +3,11 @@ Allwinner Sunxi NMI Controller
 
 Required properties:
 
-- compatible : should be "allwinner,sun7i-a20-sc-nmi" or
-  "allwinner,sun6i-a31-sc-nmi" or "allwinner,sun9i-a80-nmi"
+- compatible : should be one of:
+	"allwinner,sun6i-a31-sc-nmi"
+	"allwinner,sun7i-a20-sc-nmi"
+	"allwinner,sun9i-a80-nmi"
+	"allwinner,sun50i-a64-r-intc"
 - reg : Specifies base physical address and size of the registers.
 - interrupt-controller : Identifies the node as an interrupt controller
 - #interrupt-cells : Specifies the number of cells needed to encode an
-- 
2.12.2

^ permalink raw reply related

* [PATCH v4 01/10] arm64: allwinner: a64: enable RSB on A64
From: Icenowy Zheng @ 2017-04-24 16:00 UTC (permalink / raw)
  To: Thomas Gleixner, Rob Herring, Maxime Ripard, Chen-Yu Tsai,
	Lee Jones, Liam Girdwood
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Icenowy Zheng
In-Reply-To: <20170424160103.9447-1-icenowy-h8G6r0blFSE@public.gmane.org>

Allwinner A64 have a RSB controller like the one on A23/A33 SoCs.

Add it and its pinmux.

Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
Acked-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
---
Changes in v2:
- Removed bonus properties in pio node.
- Added Chen-Yu's ACK.

 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index c7f669f5884f..05ec9fc5e81f 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -422,6 +422,25 @@
 			#gpio-cells = <3>;
 			interrupt-controller;
 			#interrupt-cells = <3>;
+
+			r_rsb_pins: rsb@0 {
+				pins = "PL0", "PL1";
+				function = "s_rsb";
+			};
+		};
+
+		r_rsb: rsb@1f03400 {
+			compatible = "allwinner,sun8i-a23-rsb";
+			reg = <0x01f03400 0x400>;
+			interrupts = <GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&r_ccu 6>;
+			clock-frequency = <3000000>;
+			resets = <&r_ccu 2>;
+			pinctrl-names = "default";
+			pinctrl-0 = <&r_rsb_pins>;
+			status = "disabled";
+			#address-cells = <1>;
+			#size-cells = <0>;
 		};
 	};
 };
-- 
2.12.2

^ permalink raw reply related

* [PATCH v4 00/10] AXP803 PMIC support for Pine64
From: Icenowy Zheng @ 2017-04-24 16:00 UTC (permalink / raw)
  To: Thomas Gleixner, Rob Herring, Maxime Ripard, Chen-Yu Tsai,
	Lee Jones, Liam Girdwood
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Icenowy Zheng

The Pine64 (including Pine64+) boards have an AXP803 PMIC, which is a PMIC
similar to AXP288, but tweaked to use with Allwinner SoCs rather than Intel
tablets (with DCIN and Vbus re-splitted like other AXP PMICs, and RSB bus
support added).

This patchset adds support for it and enabled it in Pine64 device tree.

The basical part of AXP803 MFD driver is already applied, according to Lee.

Thus this patchset is now still two parts, but a bit different to older
revisions:

- Part1: from PATCH 1/10 to PATCH 5/10, which focus on enabling AXP803 in
  the device tree: the RSB bus, the R_INTC interrupt controller (for the
  NMI line, which is connected to AXP803 on Pine64), and finally the basical
  AXP803 node.
- Part2: from PATCH 5/10 to PATCH 10/10, which are enabling the regulator
  function of the AXP803 PMIC. Finally Wi-Fi function is added
  as a usage of regulators function.

PATCH 1 adds RSB device nodes, which is used for the communication between
A64 and AXP803.

PATCH 2 adds device tree binding of A64 R_INTC.

PATCH 3 really adds support for A64 R_INTC in NMI driver.

PATCH 4 adds R_INTC node in A64 device tree.

PATCH 5 adds AXP803 node to the Pine64 device tree by using already
applied drivers/bindings.

PATCH 6 adds support for AXP803 regulators in AXP20x regulatoe driver.
(The binding is already applied)

PATCH 7 enables the AXP803 regulator cell in MFD driver.

PATCH 8 adds a DTSI file for AXP803, like other older AXP PMICs.

PATCH 9 enables AXP803 regulators in Pine64 device tree.

PATCH 10 enables Wi-Fi for Pine64.

Icenowy Zheng (10):
  arm64: allwinner: a64: enable RSB on A64
  irqchip/sunxi-nmi: add A64 R_INTC to the binding doc
  irqchip/sunxi-nmi: add support for the NMI in A64 R_INTC
  arm64: allwinner: a64: add NMI (R_INTC) controller on A64
  arm64: allwinner: a64: add AXP803 node to Pine64 device tree
  regulator: axp20x-regulator: add support for AXP803
  mfd: axp20x: add axp20x-regulator cell for AXP803
  arm64: allwinner: a64: add DTSI file for AXP803 PMIC
  arm64: allwinner: a64: enable AXP803 regulators for Pine64
  arm64: allwinner: a64: enable Wi-Fi for Pine64

 .../interrupt-controller/allwinner,sunxi-nmi.txt   |   7 +-
 arch/arm64/boot/dts/allwinner/axp803.dtsi          | 150 ++++++++++++++++++++
 .../arm64/boot/dts/allwinner/sun50i-a64-pine64.dts | 136 ++++++++++++++++++
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi      |  27 ++++
 drivers/irqchip/irq-sunxi-nmi.c                    |  13 ++
 drivers/mfd/axp20x.c                               |   3 +-
 drivers/regulator/axp20x-regulator.c               | 153 ++++++++++++++++++---
 include/linux/mfd/axp20x.h                         |  37 +++++
 8 files changed, 501 insertions(+), 25 deletions(-)
 create mode 100644 arch/arm64/boot/dts/allwinner/axp803.dtsi

-- 
2.12.2

^ permalink raw reply

* Re: [PATCH v2 2/2] dmaengine: Add DW AXI DMAC driver
From: Eugeniy Paltsev @ 2017-04-24 15:55 UTC (permalink / raw)
  To: andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org
  Cc: vinod.koul-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	Alexey.Brodkin-HKixBCOQz3hWk0Htik3J/w@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Eugeniy.Paltsev-HKixBCOQz3hWk0Htik3J/w@public.gmane.org,
	linux-snps-arc-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	dan.j.williams-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org,
	dmaengine-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <1492787583.24567.120.camel-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

Hi,
On Fri, 2017-04-21 at 18:13 +0300, Andy Shevchenko wrote:
> On Fri, 2017-04-21 at 14:29 +0000, Eugeniy Paltsev wrote:
> > On Tue, 2017-04-18 at 15:31 +0300, Andy Shevchenko wrote:
> > > On Fri, 2017-04-07 at 17:04 +0300, Eugeniy Paltsev wrote:
> > > > This patch adds support for the DW AXI DMAC controller.
> > > > +#define AXI_DMA_BUSWIDTHS		  \
> > > > +	(DMA_SLAVE_BUSWIDTH_1_BYTE	| \
> > > > +	DMA_SLAVE_BUSWIDTH_2_BYTES	| \
> > > > +	DMA_SLAVE_BUSWIDTH_4_BYTES	| \
> > > > +	DMA_SLAVE_BUSWIDTH_8_BYTES	| \
> > > > +	DMA_SLAVE_BUSWIDTH_16_BYTES	| \
> > > > +	DMA_SLAVE_BUSWIDTH_32_BYTES	| \
> > > > +	DMA_SLAVE_BUSWIDTH_64_BYTES)
> > > > +/* TODO: check: do we need to use BIT() macro here? */
> > >
> > > Still TODO? I remember I answered to this on the first round.
> >
> > Yes, I remember it.
> > I left this TODO as a reminder because src_addr_widths and
> > dst_addr_widths are
> > not used anywhere and they are set differently in different drivers
> > (with or without BIT macro).
>
> Strange. AFAIK they are representing bits (which is not the best
> idea) in the resulting u64 field. So, anything bigger than 63 doesn't
> make sense.
They are u32 fields!
From dmaengine.h :
struct dma_device {
...
    u32 src_addr_widths;
    u32 dst_addr_widths;
};

> In drivers where they are not bits quite likely a bug is hidden.
For example (from pxa_dma.c):
const enum dma_slave_buswidth widths = DMA_SLAVE_BUSWIDTH_1_BYTE |
DMA_SLAVE_BUSWIDTH_2_BYTES | DMA_SLAVE_BUSWIDTH_4_BYTES;

And there are a lot of drivers, which don't use BIT for this fields.
sh/shdmac.c
sh/rcar-dmac.c
qcom/bam_dma.c
mmp_pdma.c
ste_dma40.c
And many others...

> >
> > > > +
> > > > +static inline void
> > > > +axi_dma_iowrite32(struct axi_dma_chip *chip, u32 reg, u32 val)
> > > > +{
> > > > +	iowrite32(val, chip->regs + reg);
> > >
> > > Are you going to use IO ports for this IP? I don't think so.
> > > Wouldn't be better to call readl()/writel() instead?
> >
> > As I understand, it's better to use ioread/iowrite as more
> > universal
> > IO
> > access way. Am I wrong?
>
> As I said above the ioreadX/iowriteX makes only sense when your IP
> would be accessed via IO region or MMIO. I'm pretty sure IO is not
> the case at all for this IP.
MMIO? This IP works exactly via memory-mapped I/O.

> > > > +static inline void axi_chan_irq_disable(struct axi_dma_chan
> > > > *chan,
> > > > u32 irq_mask)
> > > > +{
> > > > +	u32 val;
> > > > +
> > > > +	if (likely(irq_mask == DWAXIDMAC_IRQ_ALL)) {
> > > > +		axi_chan_iowrite32(chan, CH_INTSTATUS_ENA,
> > > > DWAXIDMAC_IRQ_NONE);
> > > > +	} else {
> > >
> > > I don't see the benefit. (Yes, I see one read-less path, I think
> > > it
> > > makes perplexity for nothing here)
> >
> > This function is called mostly with irq_mask = DWAXIDMAC_IRQ_ALL.
> > Actually it is called only with irq_mask = DWAXIDMAC_IRQ_ALL until
> > I add DMA_SLAVE support.
> > But I can cut off this 'if' statment, if it is necessery.
>
> It's not necessary, but from my point of view increases readability
> of the code a lot for the price of an additional readl().
Ok.

> >
> > > > +		val = axi_chan_ioread32(chan,
> > > > CH_INTSTATUS_ENA);
> > > > +		val &= ~irq_mask;
> > > > +		axi_chan_iowrite32(chan, CH_INTSTATUS_ENA,
> > > > val);
> > > > +	}
> > > > +
> > > > +	return min_t(size_t, __ffs(sdl), max_width);
> > > > +}
> > > > +static void axi_desc_put(struct axi_dma_desc *desc)
> > > > +{
> > > > +	struct axi_dma_chan *chan = desc->chan;
> > > > +	struct dw_axi_dma *dw = chan->chip->dw;
> > > > +	struct axi_dma_desc *child, *_next;
> > > > +	unsigned int descs_put = 0;
> > > > +	list_for_each_entry_safe(child, _next, &desc-
> > > > >xfer_list,
> > > > xfer_list) {
> > >
> > > xfer_list looks redundant.
> > > Can you elaborate why virtual channel management is not working
> > > for
> > > you?
> >
> > Each virtual descriptor encapsulates several hardware descriptors,
> > which belong to same transfer.
> > This list (xfer_list) is used only for allocating/freeing these
> > descriptors and it doesn't affect on virtual dma work logic.
> > I can see this approach in several drivers with VirtDMA (but they
> > mostly use array instead of list)
>
> You described how most of the DMA drivers are implemented, though
> they
> are using just sg_list directly. I would recommend to do the same and
> get rid of this list.
This IP can be (ans is) configured with small block size.
(note, that I am not saying about runtime HW configuration)

And there is opportunity what we can't use sg_list directly and need to
split sg_list to a smaller chunks.

> > > Btw, are you planning to use priority at all? For now on I didn't
> > > see
> > > a single driver (from the set I have checked, like 4-5 of them)
> > > that
> > > uses priority anyhow. It makes driver more complex for nothing.
> >
> > Only for dma slave operations.
>
> So, in other words you *have* an actual two or more users that *need*
> prioritization?
As I remember there was an idea to give higher priority to audio dma
chanels.

> > > > +	if (unlikely(dst_nents == 0 || src_nents == 0))
> > > > +		return NULL;
> > > > +
> > > > +	if (unlikely(dst_sg == NULL || src_sg == NULL))
> > > > +		return NULL;
> > >
> > > If we need those checks they should go to
> > > dmaengine.h/dmaengine.c.
> >
> > I checked several drivers, which implements device_prep_dma_sg and
> > they
> > implements this checkers.
> > Should I add something like "dma_sg_desc_invalid" function to
> > dmaengine.h ?
>
> I suppose it's better to implement them in the corresponding helpers
> in dmaengine.h.
Ok.

> > > > +static void axi_chan_list_dump_lli(struct axi_dma_chan *chan,
> > > > +				   struct axi_dma_desc
> > > > *desc_head)
> > > > +{
> > > > +	struct axi_dma_desc *desc;
> > > > +
> > > > +	axi_chan_dump_lli(chan, desc_head);
> > > > +	list_for_each_entry(desc, &desc_head->xfer_list,
> > > > xfer_list)
> > > > +		axi_chan_dump_lli(chan, desc);
> > > > +}
> > > > +
> > > > +static void axi_chan_handle_err(struct axi_dma_chan *chan, u32
> > > > status)
> > > > +{
> > > > +	/* WARN about bad descriptor */
> > > >
> > > > +	dev_err(chan2dev(chan),
> > > > +		"Bad descriptor submitted for %s, cookie: %d,
> > > > irq:
> > > > 0x%08x\n",
> > > > +		axi_chan_name(chan), vd->tx.cookie, status);
> > > > +	axi_chan_list_dump_lli(chan, vd_to_axi_desc(vd));
> > >
> > > As I said earlier dw_dmac is *bad* example of the (virtual
> > > channel
> > > based) DMA driver.
> > >
> > > I guess you may just fail the descriptor and don't pretend it has
> > > been processed successfully.
> >
> > What do you mean by saying "fail the descriptor"?
> > After I get error I cancel current transfer and free all
> > descriptors
> > from it (by calling vchan_cookie_complete).
> > I can't store error status in descriptor structure because it will
> > be
> > freed by vchan_cookie_complete.
> > I can't store error status in channel structure because it will be
> > overwritten by next transfer.
>
> Better not to pretend that it has been processed successfully. Don't
> call callback on it and set its status to DMA_ERROR (that's why
> descriptors in many drivers have dma_status field). When user asks
> for
> status (using cookie) the saved value would be returned until
> descriptor
> is active.
>
> Do you have some other workflow in mind?

Hmm...
Do you mean I should left error descriptors in desc_issued list
or I should create another list (like desc_error) in my driver and move
error descriptors to desc_error list?

And when exactly should I free error descriptors?

I checked hsu/hsu.c dma driver implementation:
  vdma descriptor is deleted from desc_issued list when transfer
  starts. When descriptor marked as error descriptor
  vchan_cookie_complete isn't called for this descriptor. And this
  descriptor isn't placed in any list. So error descriptors *never*
  will be freed.
  I don't actually like this approach.

> > > > +
> > > > +static const struct dev_pm_ops dw_axi_dma_pm_ops = {
> > > > +	SET_RUNTIME_PM_OPS(axi_dma_runtime_suspend,
> > > > axi_dma_runtime_resume, NULL)
> > > > +};
> > >
> > > Have you tried to build with CONFIG_PM disabled?
> >
> > Yes.
> >
> > > I'm pretty sure you need __maybe_unused applied to your PM ops.
> >
> > I call axi_dma_runtime_suspend / axi_dma_runtime_resume even I
> > dont't
> > use PM.
> > (I call them in probe / remove function.)
>
> Hmm... I didn't check your ->probe() and ->remove(). Do you mean you
> call them explicitly by those names?
>
> If so, please don't do that. Use pm_runtime_*() instead. And...
>
> > So I don't need to declare them with __maybe_unused.
>
> ...in that case it's possible you have them defined but not used.
>
From my ->probe() function:

pm_runtime_get_noresume(chip->dev);
ret = axi_dma_runtime_resume(chip->dev);

Firstly I only incrememt counter.
Secondly explicitly call my resume function.

I call them explicitly because I need driver to work also without
Runtime PM. So I can't just call pm_runtime_get here instead of
pm_runtime_get_noresume + axi_dma_runtime_resume.

Of course I can copy *all* code from axi_dma_runtime_resume
to ->probe() function, but I don't really like this idea.

> > > > +struct axi_dma_chan {
> > > > +	struct axi_dma_chip		*chip;
> > > > +	void __iomem			*chan_regs;
> > > > +	u8				id;
> > > > +	atomic_t			descs_allocated;
> > > > +
> > > > +	struct virt_dma_chan		vc;
> > > > +
> > > > +	/* these other elements are all protected by vc.lock
> > > > */
> > > > +	bool				is_paused;
> > >
> > > I still didn't get (already forgot) why you can't use dma_status
> > > instead for the active descriptor?
> >
> > As I said before, I checked several driver, which have status
> > variable
> > in their channel structure - it is used *only* for determinating is
> > channel paused or not. So there is no much sense in replacing
> > "is_paused" to "status" and I left "is_paused" variable untouched.
>
> Not only (see above), the errored descriptor keeps that status.
>
> > (I described above why we can't use status in channel structure for
> > error handling)
>
> Ah, I'm talking about descriptor.

Again - PAUSED is per-channel flag. So, even if we have status field in
each descriptor, it is simpler to use one per-channel flag instead of
plenty per-descriptor flags.
When we pausing/resuming dma channel it is simpler to set only one flag
instead of writing DMA_PAUSED to *each* descriptor status field.

> > > > Status Fetch Addr */
> > > > +#define CH_INTSTATUS_ENA	0x080 /* R/W Chan Interrupt
> > > > Status
> > > > Enable */
> > > > +#define CH_INTSTATUS		0x088 /* R/W Chan
> > > > Interrupt
> > > > Status */
> > > > +#define CH_INTSIGNAL_ENA	0x090 /* R/W Chan Interrupt
> > > > Signal
> > > > Enable */
> > > > +#define CH_INTCLEAR		0x098 /* W Chan Interrupt
> > > > Clear
> > > > */
> > >
> > > I'm wondering if you can use regmap API instead.
> >
> > Is it really necessary? It'll make driver more complex for nothing.
>
> That's why I'm not suggesting but wondering.
> >
> > > > +};
> > >
> > > Hmm... do you need them in the header?
> >
> > I use some of these definitions in my code so I guess yes.
> > /* Maybe I misunderstood your question... */
>
> I mean, who are the users of them? If it's only one module, there is
> no need to put them in header.
Yes, only one module.
Should I move all this definitions to axi_dma_platform.c file and rid
of both axi_dma_platform_reg.h and axi_dma_platform.h headers?

> >
> > > > +#define CH_CFG_H_TT_FC_POS	0
> > > > +enum {
> > > > +	DWAXIDMAC_TT_FC_MEM_TO_MEM_DMAC	= 0x0,
> > > > +	DWAXIDMAC_TT_FC_MEM_TO_PER_DMAC,
> > > > +	DWAXIDMAC_TT_FC_PER_TO_MEM_DMAC,
> > > > +	DWAXIDMAC_TT_FC_PER_TO_PER_DMAC,
> > > > +	DWAXIDMAC_TT_FC_PER_TO_MEM_SRC,
> > > > +	DWAXIDMAC_TT_FC_PER_TO_PER_SRC,
> > > > +	DWAXIDMAC_TT_FC_MEM_TO_PER_DST,
> > > > +	DWAXIDMAC_TT_FC_PER_TO_PER_DST
> > > > +};
> > >
> > > Some of definitions are the same as for dw_dmac, right? We might
> > > split them to a common header, though I have no strong opinion
> > > about
> >
> > it.
> > > Vinod?
> >
> > APB DMAC and AXI DMAC have completely different regmap. So there is
> > no
> > much sense to do that.
>
> I'm not talking about registers, I'm talking about definitions like
> above. Though it would be indeed little sense to split some common
> code between those two.
This definitions are the fields of some registers. So they are also
different.

--
 Eugeniy Paltsev

^ permalink raw reply

* Re: [PATCH V2 3/4] mtd: partitions: add of_match_table parser matching
From: Jonas Gorski @ 2017-04-24 15:31 UTC (permalink / raw)
  To: Rafał Miłecki
  Cc: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut,
	Richard Weinberger, Cyrille Pitchen, Rob Herring, Mark Rutland,
	Frank Rowand, Linus Walleij, MTD Maling List,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Geert Uytterhoeven, Rafał Miłecki
In-Reply-To: <20170424124120.31613-4-zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Hi,

On 24 April 2017 at 14:41, Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> From: Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>
> Partition parsers can now provide an of_match_table to enable
> flash<-->parser matching via device tree.
>
> This support is currently limited to built-in parsers as it uses
> request_module() and friends. This should be sufficient for most cases
> though as compiling parsers as modules isn't a common choice.
>
> Signed-off-by: Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
> Acked-by: Brian Norris <computersforpeac-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> This is based on Brian's patches:
> [RFC PATCH 4/7] mtd: add of_match_mtd_parser() and of_mtd_match_mtd_parser() helpers
> [RFC PATCH 6/7] RFC: mtd: partitions: enable of_match_table matching
>
> V1: Put helpers in mtdpart.c instead of drivers/of/of_mtd.c
>     Merge helpers into a single of_mtd_match_mtd_parser
> ---
>  drivers/mtd/mtdpart.c          | 47 ++++++++++++++++++++++++++++++++++++++++++
>  include/linux/mtd/partitions.h |  1 +
>  2 files changed, 48 insertions(+)
>
> diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
> index 73c52f1a2e4c..d0cb1a892ed2 100644
> --- a/drivers/mtd/mtdpart.c
> +++ b/drivers/mtd/mtdpart.c
> @@ -861,6 +861,41 @@ static int mtd_part_do_parse(struct mtd_part_parser *parser,
>         return ret;
>  }
>
> +static bool of_mtd_match_mtd_parser(struct mtd_info *mtd,
> +                                   struct mtd_part_parser *parser)
> +{
> +       struct device_node *np;
> +       bool ret;
> +
> +       np = mtd_get_of_node(mtd);
> +       np = of_get_child_by_name(np, "partitions");
> +
> +       ret = !!of_match_node(parser->of_match_table, np);
> +
> +       of_node_put(np);
> +
> +       return ret;
> +}
> +
> +static struct mtd_part_parser *mtd_part_get_parser_by_of(struct mtd_info *mtd)
> +{
> +       struct mtd_part_parser *p, *ret = NULL;
> +
> +       spin_lock(&part_parser_lock);
> +
> +       list_for_each_entry(p, &part_parsers, list) {
> +               if (of_mtd_match_mtd_parser(mtd, p) &&
> +                               try_module_get(p->owner)) {
> +                       ret = p;
> +                       break;
> +               }
> +       }


Hm, maybe iterate over the compatibles, so parsers matching the most
specific compatible get precedence in case there is more than one
compatible? Currently it will match the first one that matches any
compatible, and registration order of parsers can change that. I'm
thinking of parsers that partially rely on fixed, unprobable layouts,
so can use "fixed-partitions" as a fallback compatible.

E.g. having something like this

partitions {
        compatible = "sample,custom-layout", "fixed-partitions";

        bootloader@0 { ...  };

        firmware@10000 { .... }; /* will be split by the parser */

        extra@780000 { .... }; /* partition the on-flash format can't specify */
};

Where you will still be able to write an image raw to the image
partition even if the "custom-layout"-parser isn't present/enabled,
but if it is present, it should always be used.


Regards
Jonas
--
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 V2 1/4] dt-bindings: mtd: make partitions doc a bit more generic
From: Jonas Gorski @ 2017-04-24 15:28 UTC (permalink / raw)
  To: Rafał Miłecki
  Cc: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut,
	Richard Weinberger, Cyrille Pitchen, Rob Herring, Mark Rutland,
	Frank Rowand, Linus Walleij, MTD Maling List,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Geert Uytterhoeven, Rafał Miłecki
In-Reply-To: <20170424124120.31613-2-zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Hi,

On 24 April 2017 at 14:41, Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> From: Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>
> Currently the only documented partitioning is "fixed-partitions" but
> there are more methods in use that we may want to support in the future.
> Mention them and make it clear Fixed Partitions are just a single case.
>
> Signed-off-by: Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
> ---
> This is based on Brian's patch:
> [RFC PATCH 3/7] doc: dt: mtd: partition: add on-flash format binding
>
> V1: Dropped "Section B: On-Flash Partition Tables" with Google's FMAP as this
>     patchset doesn't add that new parser.
> V2: Add "We currently only document a binding for fixed layouts." part
> ---
>  .../devicetree/bindings/mtd/partition.txt          | 30 +++++++++++++++++-----
>  1 file changed, 24 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mtd/partition.txt b/Documentation/devicetree/bindings/mtd/partition.txt
> index 81a224da63be..b5de311b967a 100644
> --- a/Documentation/devicetree/bindings/mtd/partition.txt
> +++ b/Documentation/devicetree/bindings/mtd/partition.txt
> @@ -1,29 +1,47 @@
> -Representing flash partitions in devicetree
> +Flash partitions in device tree
> +===============================
>
> -Partitions can be represented by sub-nodes of an mtd device. This can be used
> +Flash devices can be partitioned into one or more functional ranges (e.g. "boot
> +code", "nvram", "kernel").
> +
> +Different devices may be partitioned in a different ways. Some may use a fixed
> +flash layout set at production time. Some may use on-flash table that describes
> +the geometry and naming/purpose of each functional region. It is also possible
> +to see these methods mixed.
> +
> +To assist system software in locating partitions, we provide a binding to
> +describe which method is used for a given flash.

Since patch 3 adds specifying methods through the compatible of the
partitions subnode, maybe we should document that here? Something
along

"To assist system software in locating partitions, we allow describing
which method is used for a given flash device. To describe the method
there should be a subnode of the flash device that is named
'partitions'. It must have a 'compatible' property, which is used to
identify the method to use."


Regards
Jonas
--
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 V2 1/4] mfd: Add ROHM BD9571MWV-M PMIC DT bindings
From: Marek Vasut @ 2017-04-24 15:21 UTC (permalink / raw)
  To: linux-renesas-soc
  Cc: Marek Vasut, devicetree, Rob Herring, Geert Uytterhoeven

Add DT bindings for the ROHM BD9571MWV-M PMIC. This PMIC has
the following features:
- multiple voltage monitors for 1V8, 2V5, 3V3 voltage rail
- one voltage regulator for DVFS
- two GPIOs

Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
Cc: devicetree@vger.kernel.org
Cc: Rob Herring <robh@kernel.org>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: linux-renesas-soc@vger.kernel.org
---
V2: - Drop the compatible = "regulator-fixed" from the binding example,
      it should not be there.
    - List the VD09 regulator
---
 .../devicetree/bindings/mfd/bd9571mwv.txt          | 49 ++++++++++++++++++++++
 1 file changed, 49 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/bd9571mwv.txt

diff --git a/Documentation/devicetree/bindings/mfd/bd9571mwv.txt b/Documentation/devicetree/bindings/mfd/bd9571mwv.txt
new file mode 100644
index 000000000000..ce24231edd7d
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/bd9571mwv.txt
@@ -0,0 +1,49 @@
+* ROHM BD9571MWV Power Management Integrated Circuit (PMIC) bindings
+
+Required properties:
+ - compatible		: Should be "rohm,bd9571mwv".
+ - reg			: I2C slave address.
+ - interrupt-parent	: Phandle to the parent interrupt controller.
+ - interrupts		: The interrupt line the device is connected to.
+ - interrupt-controller	: Marks the device node as an interrupt controller.
+ - #interrupt-cells	: The number of cells to describe an IRQ, should be 2.
+			    The first cell is the IRQ number.
+			    The second cell is the flags, encoded as trigger
+			    masks from ../interrupt-controller/interrupts.txt.
+ - gpio-controller      : Marks the device node as a GPIO Controller.
+ - #gpio-cells          : Should be two.  The first cell is the pin number and
+                            the second cell is used to specify flags.
+                            See ../gpio/gpio.txt for more information.
+ - regulators:          : List of child nodes that specify the regulator
+                            initialization data. Child nodes must be named
+                            after their hardware counterparts:
+			     - vd09
+			     - vd18
+			     - vd25
+			     - vd33
+			     - dvfs
+			    Each child node is defined using the standard
+			    binding for regulators.
+
+Example:
+
+	pmic: bd9571mwv@30 {
+		compatible = "rohm,bd9571mwv";
+		reg = <0x30>;
+		interrupt-parent = <&gpio2>;
+		interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
+		interrupt-controller;
+		#interrupt-cells = <2>;
+		gpio-controller;
+		#gpio-cells = <2>;
+
+		regulators {
+			dvfs: dvfs {
+				regulator-name = "dvfs";
+				regulator-min-microvolt = <750000>;
+				regulator-max-microvolt = <1030000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+		};
+	};
-- 
2.11.0

^ permalink raw reply related

* Re: [PATCH v8 1/3] backlight arcxcnn add arc to vendor prefix
From: Rob Herring @ 2017-04-24 15:09 UTC (permalink / raw)
  To: Olimpiu Dejeu
  Cc: Lee Jones, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Jingoo Han,
	bdodge-eV7fy4qpoLhpLGFMi4vTTA, Joe Perches,
	medasaro-eV7fy4qpoLhpLGFMi4vTTA, Daniel Thompson
In-Reply-To: <1489607133-7870-1-git-send-email-olimpiu-eV7fy4qpoLhpLGFMi4vTTA@public.gmane.org>

On Wed, Mar 15, 2017 at 2:45 PM, Olimpiu Dejeu <olimpiu-eV7fy4qpoLhpLGFMi4vTTA@public.gmane.org> wrote:
> backlight: Add arc to vendor prefixes
> Signed-off-by: Olimpiu Dejeu <olimpiu-eV7fy4qpoLhpLGFMi4vTTA@public.gmane.org>
> ---
> v8:
> - Version to match other patches in set
>
>  Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
> index 16d3b5e..6f33a4b 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
> @@ -28,6 +28,7 @@ andestech     Andes Technology Corporation
>  apm    Applied Micro Circuits Corporation (APM)
>  aptina Aptina Imaging
>  arasan Arasan Chip Systems
> +arc    Arctic Sand

arc is also a cpu arch. While not a vendor, it could be confusing. How
about "arctic" instead?

BTW, some reason your patches are not going to the DT list.

Rob
--
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 v14 00/11] mux controller abstraction and iio/i2c muxes
From: Philipp Zabel @ 2017-04-24 14:59 UTC (permalink / raw)
  To: Peter Rosin
  Cc: Jonathan Cameron, linux-kernel, Greg Kroah-Hartman, Wolfram Sang,
	Rob Herring, Mark Rutland, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Jonathan Corbet, linux-i2c, devicetree,
	linux-iio, linux-doc, Andrew Morton, Colin Ian King,
	Paul Gortmaker, kernel
In-Reply-To: <6fb34ea5-edc8-c7aa-1d49-f6ce1d33a2d4@axentia.se>

On Mon, 2017-04-24 at 16:36 +0200, Peter Rosin wrote:
[...]
> > How about an atomic use_count on the mux_control, a bool shared that is
> > only set by the first consumer, and controls whether selecting locks?
> 
> That has the drawback that it is hard to restore the mux-control in a safe
> way so that exclusive consumers are allowed after the last shared consumer
> puts the mux away.

True.

> Agreed, it's a corner case, but I had this very similar
> patch going through the compiler when I got this mail. Does it work as well
> as what you suggested?

Yes, this patch works just as well.

regards
Philipp

^ permalink raw reply

* Re: [PATCH v14 00/11] mux controller abstraction and iio/i2c muxes
From: Peter Rosin @ 2017-04-24 14:36 UTC (permalink / raw)
  To: Philipp Zabel
  Cc: Jonathan Cameron, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	Greg Kroah-Hartman, Wolfram Sang, Rob Herring, Mark Rutland,
	Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
	Jonathan Corbet, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	linux-doc-u79uwXL29TY76Z2rM5mHXA, Andrew Morton, Colin Ian King,
	Paul Gortmaker, kernel-bIcnvbaLZ9MEGnE8C9+IrQ
In-Reply-To: <1493043046.2446.37.camel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

On 2017-04-24 16:10, Philipp Zabel wrote:
> On Mon, 2017-04-24 at 13:37 +0200, Peter Rosin wrote:
>> On 2017-04-24 12:52, Philipp Zabel wrote:
>>> On Mon, 2017-04-24 at 10:36 +0200, Peter Rosin wrote:
>>>> Hi!
>>>>
>>>> The big change since v13 is that the mux state is now locked with a mutex
>>>> instead of an rwsem. Other that that, it is mostly restructuring and doc
>>>> changes. There are a few other "real" changes as well, but those changes
>>>> feel kind of minor. I guess what I'm trying to say is that although the
>>>> list of changes for v14 is longish, it's still basically the same as last
>>>> time.
>>>
>>> I have hooked this up to the video-multiplexer and now I trigger
>>> the lockdep debug_check_no_locks_held error message when selecting the
>>> mux input from userspace:
>>>
>>> $ media-ctl --links "'imx6-mipi-csi2':1->'ipu1_csi0_mux':0[1]"
>>> [   66.258368] 
>>> [   66.259919] =====================================
>>> [   66.265369] [ BUG: media-ctl/258 still has locks held! ]
>>> [   66.270810] 4.11.0-rc8-20170424-1+ #1305 Not tainted
>>> [   66.275863] -------------------------------------
>>> [   66.282158] 1 lock held by media-ctl/258:
>>> [   66.286464]  #0:  (&mux->lock){+.+.+.}, at: [<8074a6c0>] mux_control_select+0x24/0x50
>>> [   66.294424] 
>>> [   66.294424] stack backtrace:
>>> [   66.298980] CPU: 0 PID: 258 Comm: media-ctl Not tainted 4.11.0-rc8+ #1305
>>> [   66.306771] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
>>> [   66.313334] Backtrace: 
>>> [   66.315858] [<8010e5a4>] (dump_backtrace) from [<8010e8ac>] (show_stack+0x20/0x24)
>>> [   66.323473]  r7:80e518d0 r6:80e518d0 r5:600d0013 r4:00000000
>>> [   66.329190] [<8010e88c>] (show_stack) from [<80496cf4>] (dump_stack+0xb0/0xdc)
>>> [   66.336470] [<80496c44>] (dump_stack) from [<8017e404>] (debug_check_no_locks_held+0xb8/0xbc)
>>> [   66.345043]  r9:ae8566b8 r8:ad88dc84 r7:ad88df58 r6:ad88dc84 r5:ad88df58 r4:ae856400
>>> [   66.352837] [<8017e34c>] (debug_check_no_locks_held) from [<8012b258>] (do_exit+0x79c/0xcc8)
>>> [   66.361321] [<8012aabc>] (do_exit) from [<8012d25c>] (do_group_exit+0x4c/0xcc)
>>> [   66.368581]  r7:000000f8
>>> [   66.371161] [<8012d210>] (do_group_exit) from [<8012d2fc>] (__wake_up_parent+0x0/0x30)
>>> [   66.379120]  r7:000000f8 r6:76f71798 r5:00000000 r4:00000001
>>> [   66.384837] [<8012d2dc>] (SyS_exit_group) from [<80109380>] (ret_fast_syscall+0x0/0x1c)
>>>
>>> That correctly warns that the media-ctl process caused the mux->lock to
>>> be locked and still held when the process exited. Do we need a usage
>>> counter based mechanism for muxes that are (indirectly) controlled from
>>> userspace?
> [...]
>> The question then becomes how to best tell the mux core that you want
>> an exclusive mux. I see two options. Either you declare a mux controller
>> as exclusive up front somehow (in the device tree presumably), or you
>> add a mux_control_get_exclusive call that makes further calls to
>> mux_control_get{,_exclusive} fail with -EBUSY. I think I like the
>> latter better, if that can be made to work...
> 
> How about an atomic use_count on the mux_control, a bool shared that is
> only set by the first consumer, and controls whether selecting locks?

That has the drawback that it is hard to restore the mux-control in a safe
way so that exclusive consumers are allowed after the last shared consumer
puts the mux away. Agreed, it's a corner case, but I had this very similar
patch going through the compiler when I got this mail. Does it work as well
as what you suggested?

Cheers,
peda

diff --git a/Documentation/driver-model/devres.txt b/Documentation/driver-model/devres.txt
index e2343d9cbec7..5a7352a83124 100644
--- a/Documentation/driver-model/devres.txt
+++ b/Documentation/driver-model/devres.txt
@@ -342,7 +342,8 @@ MUX
   devm_mux_chip_free()
   devm_mux_chip_register()
   devm_mux_chip_unregister()
-  devm_mux_control_get()
+  devm_mux_control_get_shared()
+  devm_mux_control_get_exclusive()
   devm_mux_control_put()
 
 PER-CPU MEM
diff --git a/drivers/i2c/muxes/i2c-mux-gpmux.c b/drivers/i2c/muxes/i2c-mux-gpmux.c
index 92cf5f48afe6..135d6baaebe5 100644
--- a/drivers/i2c/muxes/i2c-mux-gpmux.c
+++ b/drivers/i2c/muxes/i2c-mux-gpmux.c
@@ -87,7 +87,7 @@ static int i2c_mux_probe(struct platform_device *pdev)
 	if (!mux)
 		return -ENOMEM;
 
-	mux->control = devm_mux_control_get(dev, NULL);
+	mux->control = devm_mux_control_get_shared(dev, NULL);
 	if (IS_ERR(mux->control)) {
 		if (PTR_ERR(mux->control) != -EPROBE_DEFER)
 			dev_err(dev, "failed to get control-mux\n");
diff --git a/drivers/iio/multiplexer/iio-mux.c b/drivers/iio/multiplexer/iio-mux.c
index 37ba007f8dca..fc41e9d10737 100644
--- a/drivers/iio/multiplexer/iio-mux.c
+++ b/drivers/iio/multiplexer/iio-mux.c
@@ -413,7 +413,7 @@ static int mux_probe(struct platform_device *pdev)
 		}
 	}
 
-	mux->control = devm_mux_control_get(dev, NULL);
+	mux->control = devm_mux_control_get_shared(dev, NULL);
 	if (IS_ERR(mux->control)) {
 		if (PTR_ERR(mux->control) != -EPROBE_DEFER)
 			dev_err(dev, "failed to get control-mux\n");
diff --git a/drivers/mux/mux-core.c b/drivers/mux/mux-core.c
index c02fa4dd2d09..6055e7c3ad1c 100644
--- a/drivers/mux/mux-core.c
+++ b/drivers/mux/mux-core.c
@@ -37,6 +37,7 @@ static struct class mux_class = {
 };
 
 static DEFINE_IDA(mux_ida);
+static DEFINE_MUTEX(mux_lock);
 
 static int __init mux_init(void)
 {
@@ -333,7 +334,8 @@ unsigned int mux_control_states(struct mux_control *mux)
 EXPORT_SYMBOL_GPL(mux_control_states);
 
 /*
- * The mux->lock must be held when calling this function.
+ * The mux->lock must be held when calling this function if the
+ * mux-control is shared.
  */
 static int __mux_control_select(struct mux_control *mux, int state)
 {
@@ -372,11 +374,12 @@ int mux_control_select(struct mux_control *mux, unsigned int state)
 {
 	int ret;
 
-	mutex_lock(&mux->lock);
+	if (mux->shared >= 0)
+		mutex_lock(&mux->lock);
 
 	ret = __mux_control_select(mux, state);
 
-	if (ret < 0)
+	if (ret < 0 && mux->shared >= 0)
 		mutex_unlock(&mux->lock);
 
 	return ret;
@@ -399,12 +402,12 @@ int mux_control_try_select(struct mux_control *mux, unsigned int state)
 {
 	int ret;
 
-	if (!mutex_trylock(&mux->lock))
+	if (mux->shared >= 0 && !mutex_trylock(&mux->lock))
 		return -EBUSY;
 
 	ret = __mux_control_select(mux, state);
 
-	if (ret < 0)
+	if (ret < 0 && mux->shared >= 0)
 		mutex_unlock(&mux->lock);
 
 	return ret;
@@ -427,7 +430,8 @@ int mux_control_deselect(struct mux_control *mux)
 	    mux->idle_state != mux->cached_state)
 		ret = mux_control_set(mux, mux->idle_state);
 
-	mutex_unlock(&mux->lock);
+	if (mux->shared >= 0)
+		mutex_unlock(&mux->lock);
 
 	return ret;
 }
@@ -447,18 +451,13 @@ static struct mux_chip *of_find_mux_chip_by_node(struct device_node *np)
 	return dev ? to_mux_chip(dev) : NULL;
 }
 
-/**
- * mux_control_get() - Get the mux-control for a device.
- * @dev: The device that needs a mux-control.
- * @mux_name: The name identifying the mux-control.
- *
- * Return: A pointer to the mux-control, or an ERR_PTR with a negative errno.
- */
-struct mux_control *mux_control_get(struct device *dev, const char *mux_name)
+struct mux_control *__mux_control_get(struct device *dev, const char *mux_name,
+	bool shared)
 {
 	struct device_node *np = dev->of_node;
 	struct of_phandle_args args;
 	struct mux_chip *mux_chip;
+	struct mux_control *mux;
 	unsigned int controller;
 	int index = 0;
 	int ret;
@@ -504,10 +503,20 @@ struct mux_control *mux_control_get(struct device *dev, const char *mux_name)
 		return ERR_PTR(-EINVAL);
 	}
 
+	mux = &mux_chip->mux[controller];
+
+	mutex_lock(&mux_lock);
+	if (WARN_ON(mux->shared < 0 || (!shared && mux->shared))) {
+		mutex_unlock(&mux_lock);
+		return ERR_PTR(-EBUSY);
+	}
+	mux->shared = shared ? mux->shared + 1 : -1;
+	mutex_unlock(&mux_lock);
+
 	get_device(&mux_chip->dev);
-	return &mux_chip->mux[controller];
+	return mux;
 }
-EXPORT_SYMBOL_GPL(mux_control_get);
+EXPORT_SYMBOL_GPL(__mux_control_get);
 
 /**
  * mux_control_put() - Put away the mux-control for good.
@@ -517,6 +526,14 @@ EXPORT_SYMBOL_GPL(mux_control_get);
  */
 void mux_control_put(struct mux_control *mux)
 {
+	mutex_lock(&mux_lock);
+	WARN_ON(!mux->shared);
+	if (mux->shared > 0)
+		--mux->shared;
+	else
+		mux->shared = 0;
+	mutex_unlock(&mux_lock);
+
 	put_device(&mux->chip->dev);
 }
 EXPORT_SYMBOL_GPL(mux_control_put);
@@ -528,16 +545,9 @@ static void devm_mux_control_release(struct device *dev, void *res)
 	mux_control_put(mux);
 }
 
-/**
- * devm_mux_control_get() - Get the mux-control for a device, with resource
- *			    management.
- * @dev: The device that needs a mux-control.
- * @mux_name: The name identifying the mux-control.
- *
- * Return: Pointer to the mux-control, or an ERR_PTR with a negative errno.
- */
-struct mux_control *devm_mux_control_get(struct device *dev,
-					 const char *mux_name)
+struct mux_control *__devm_mux_control_get(struct device *dev,
+					   const char *mux_name,
+					   bool shared)
 {
 	struct mux_control **ptr, *mux;
 
@@ -545,7 +555,7 @@ struct mux_control *devm_mux_control_get(struct device *dev,
 	if (!ptr)
 		return ERR_PTR(-ENOMEM);
 
-	mux = mux_control_get(dev, mux_name);
+	mux = __mux_control_get(dev, mux_name, shared);
 	if (IS_ERR(mux)) {
 		devres_free(ptr);
 		return mux;
@@ -556,7 +566,7 @@ struct mux_control *devm_mux_control_get(struct device *dev,
 
 	return mux;
 }
-EXPORT_SYMBOL_GPL(devm_mux_control_get);
+EXPORT_SYMBOL_GPL(__devm_mux_control_get);
 
 static int devm_mux_control_match(struct device *dev, void *res, void *data)
 {
diff --git a/include/linux/mux/consumer.h b/include/linux/mux/consumer.h
index a2a61834bd3a..611dcec7fa3a 100644
--- a/include/linux/mux/consumer.h
+++ b/include/linux/mux/consumer.h
@@ -23,11 +23,81 @@ int __must_check mux_control_try_select(struct mux_control *mux,
 					unsigned int state);
 int mux_control_deselect(struct mux_control *mux);
 
-struct mux_control *mux_control_get(struct device *dev, const char *mux_name);
+struct mux_control *__mux_control_get(struct device *dev,
+				      const char *mux_name, bool shared);
 void mux_control_put(struct mux_control *mux);
-
-struct mux_control *devm_mux_control_get(struct device *dev,
-					 const char *mux_name);
+struct mux_control *__devm_mux_control_get(struct device *dev,
+					   const char *mux_name, bool shared);
 void devm_mux_control_put(struct device *dev, struct mux_control *mux);
 
+/**
+ * mux_control_get_shared() - Get the mux-control for a device.
+ * @dev: The device that needs a mux-control.
+ * @mux_name: The name identifying the mux-control.
+ *
+ * A shared mux-control can be operated by several independent consumers.
+ * The mux core will coordinate access by grabbing a lock when the mux-control
+ * is selected and releasing it when it is deselected.
+ *
+ * Return: A pointer to the mux-control, or an ERR_PTR with a negative errno.
+ */
+static inline struct mux_control *mux_control_get_shared(
+					struct device *dev,
+					const char *mux_name)
+{
+	return __mux_control_get(dev, mux_name, true);
+}
+
+/**
+ * mux_control_get_exclusive() - Get the mux-control for a device.
+ * @dev: The device that needs a mux-control.
+ * @mux_name: The name identifying the mux-control.
+ *
+ * An exclusive mux-control can only be operated by a single consumer. The
+ * mux core will return EBUSY for any further attempts to get a mux-control
+ * that already has an exclusive consumer. The mux core will also not lock
+ * the mux-control on mux_control_select, and the consumer is free to call
+ * repeatedly call select without calling mux_control_deselect. The consumer
+ * may however still call mux_control_deselect in order to activate the idle
+ * state.
+ *
+ * Return: A pointer to the mux-control, or an ERR_PTR with a negative errno.
+ */
+static inline struct mux_control *mux_control_get_exclusive(
+					struct device *dev,
+					const char *mux_name)
+{
+	return __mux_control_get(dev, mux_name, false);
+}
+
+/**
+ * devm_mux_control_get_shared() - Get a shared mux-control for a device, with
+ *				   resource management.
+ * @dev: The device that needs a mux-control.
+ * @mux_name: The name identifying the mux-control.
+ *
+ * Return: Pointer to the mux-control, or an ERR_PTR with a negative errno.
+ */
+static inline struct mux_control *devm_mux_control_get_shared(
+					struct device *dev,
+					const char *mux_name)
+{
+	return __devm_mux_control_get(dev, mux_name, true);
+}
+
+/**
+ * devm_mux_control_get_exclusive() - Get an exclusive mux-control for a
+ *				      device, with resource management.
+ * @dev: The device that needs a mux-control.
+ * @mux_name: The name identifying the mux-control.
+ *
+ * Return: Pointer to the mux-control, or an ERR_PTR with a negative errno.
+ */
+static inline struct mux_control *devm_mux_control_get_exclusive(
+					struct device *dev,
+					const char *mux_name)
+{
+	return __devm_mux_control_get(dev, mux_name, false);
+}
+
 #endif /* _LINUX_MUX_CONSUMER_H */
diff --git a/include/linux/mux/driver.h b/include/linux/mux/driver.h
index 95269f40670a..882e9be3d185 100644
--- a/include/linux/mux/driver.h
+++ b/include/linux/mux/driver.h
@@ -33,6 +33,8 @@ struct mux_control_ops {
  * struct mux_control -	Represents a mux controller.
  * @lock:		Protects the mux controller state.
  * @chip:		The mux chip that is handling this mux controller.
+ * @shared:		The shared state, -1 if exclusive, 0 if no consumer
+ *			and if positive the number of sharing consumers.
  * @cached_state:	The current mux controller state, or -1 if none.
  * @states:		The number of mux controller states.
  * @idle_state:		The mux controller state to use when inactive, or one
@@ -40,13 +42,14 @@ struct mux_control_ops {
  *
  * Mux drivers may only change @states and @idle_state, and may only do so
  * between allocation and registration of the mux controller. Specifically,
- * @cached_state is internal to the mux core and should never be written by
- * mux drivers.
+ * @cached_state and @shared are internal to the mux core and should never be
+ * written by mux drivers.
  */
 struct mux_control {
 	struct mutex lock; /* protects the state of the mux */
 
 	struct mux_chip *chip;
+	int shared;
 	int cached_state;
 
 	unsigned int states;

--
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

* Re: [PATCH 2/3] drm/vc4: Don't try to initialize FBDEV if we're only bound to V3D.
From: Alex Deucher @ 2017-04-24 14:26 UTC (permalink / raw)
  To: Eric Anholt
  Cc: Mark Rutland, devicetree@vger.kernel.org,
	Linux Kernel Mailing List, dri-devel, Rob Herring
In-Reply-To: <87wpadtlsl.fsf@eliezer.anholt.net>

On Fri, Apr 21, 2017 at 6:53 PM, Eric Anholt <eric@anholt.net> wrote:
> Daniel Vetter <daniel@ffwll.ch> writes:
>
>> On Wed, Apr 19, 2017 at 7:55 PM, Eric Anholt <eric@anholt.net> wrote:
>>> Daniel Vetter <daniel@ffwll.ch> writes:
>>>> On Tue, Apr 18, 2017 at 9:11 PM, Eric Anholt <eric@anholt.net> wrote:
>>>>> The FBDEV initialization would throw an error in dmesg, when we just
>>>>> want to silently not initialize fbdev on a V3D-only VC4 instance.
>>>>>
>>>>> Signed-off-by: Eric Anholt <eric@anholt.net>
>>>>
>>>> Hm, this shouldn't be an error really, you might want to hotplug more
>>>> connectors later on. What exactly complains?
>>>
>>> drm_fb_helper_init() throws an error if the passed in connector count is
>>> 0, so drm_fb_cma_helper() printks an error.
>>
>> Oh, _that_ thing. The error in there is correct, but (almost) everyone
>> gets this parameter wrong. This isn't the max number of connectors the
>> fb helper will light up, but just the max number of connectors _per_
>> crtc when driving in hw clone mode. There's two problems with that:
>> - fb helpers don't support hw clone mode, we select 1:1 crtcs for each
>> active connector
>> - I mentioned that everyone gets this wrong?
>>
>> If you're moderately bored it'd be great to nuke the max_connector
>> argument from drm_fb_helper_init, and hard-code it to 1 (with a big
>> comment explaining that this needs to be changed, probably with
>> dynamic reallocation, once someone gets around to implementing hw
>> clone mode).
>>
>> If you're less bored, just hardcode this to 1 in vc4 and done. Plus a
>> TODO.rst entry would be great in that case.
>
> If I'm driving a GPU with no display subsystem at all, it seems like I
> shouldn't initialize fbdev for it, right?

That's what we do for radeon/amdgpu on hw without display blocks.

Alex
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* Re: [RFC PATH] of/pci/dma: fix DMA configruation for PCI masters
From: Rob Herring @ 2017-04-24 14:20 UTC (permalink / raw)
  To: Oza Pawandeep
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Oza Pawandeep,
	linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux IOMMU,
	bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <1492848487-11768-1-git-send-email-oza.oza-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>

On Sat, Apr 22, 2017 at 3:08 AM, Oza Pawandeep <oza.oza-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> wrote:
> current device frmework and of framework integration assumes dma-ranges
> in a way where memory-mapped devices define their dma-ranges.
> dma-ranges: (child-bus-address, parent-bus-address, length).
>
> but iproc based SOCs and other SOCs(suc as rcar) have PCI world dma-ranges.
> dma-ranges = <0x43000000 0x00 0x00 0x00 0x00 0x80 0x00>;
>
> of_dma_configure is specifically witten to take care of memory mapped devices.
> but no implementation exists for pci to take care of pcie based memory ranges.
> in fact pci world doesnt seem to define standard dma-ranges
>
> this patch served following purposes
>
> 1) exposes intrface to the pci host driver for thir inbound memory ranges
>
> 2) provide an interface to callers such as of_dma_get_ranges.
> so then the returned size get best possible (largest) dma_mask.
> for e.g.
> dma-ranges = <0x43000000 0x00 0x00 0x00 0x00 0x80 0x00>;
> we should get dev->coherent_dma_mask=0x7fffffffff.
>
> 3) this patch handles multiple inbound windows and dma-ranges.
> it is left to the caller, how it wants to use them.
> the new function returns the resources in a standard and unform way
>
> 4) this way the callers of of_dma_get_ranges does not need to change.
> and
>
> 5) leaves scope of adding PCI flag handling for inbound memory
> by the new function.
>
> Signed-off-by: Oza Pawandeep <oza.oza-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>
> diff --git a/drivers/of/address.c b/drivers/of/address.c
> index 02b2903..ec21191 100644
> --- a/drivers/of/address.c
> +++ b/drivers/of/address.c
> @@ -6,6 +6,7 @@
>  #include <linux/ioport.h>
>  #include <linux/module.h>
>  #include <linux/of_address.h>
> +#include <linux/of_pci.h>
>  #include <linux/pci.h>
>  #include <linux/pci_regs.h>
>  #include <linux/sizes.h>
> @@ -829,10 +830,30 @@ int of_dma_get_range(struct device_node *np, u64 *dma_addr, u64 *paddr, u64 *siz
>         int len, naddr, nsize, pna;
>         int ret = 0;
>         u64 dmaaddr;
> +       struct resource_entry *window;
> +       LIST_HEAD(res);
>
>         if (!node)
>                 return -EINVAL;
>
> +       if (strcmp(np->name, "pci")) {

Using the name is not reliable though I did recently add a dtc check
for this. Of course, 'pcie' is valid too (and probably should be used
for what you are testing). type is what you want to use here. We
already have bus matching function and bus specific handlers in
address.c. Whatever solution you come up with should be integrated
with the existing bus specific handlers.

Rob

^ permalink raw reply

* Re: [PATCH 08/13] OF/PCI: Add IORESOURCE_MEM_64 for 64-bit resource
From: Rob Herring @ 2017-04-24 14:12 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: Bjorn Helgaas, David Miller, Benjamin Herrenschmidt, Wei Yang,
	Khalid Aziz, linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Grant Likely, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20170421050500.13957-9-yinghai-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

On Fri, Apr 21, 2017 at 12:04 AM, Yinghai Lu <yinghai-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> For device resource PREF bit setting under bridge 64-bit pref resource,
> we need to make sure only set PREF for 64bit resource.
>
> This patch set IORESOUCE_MEM_64 for 64bit resource during OF device
> resource flags parsing.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=96261
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=96241
> Signed-off-by: Yinghai Lu <yinghai-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Tested-by: Khalid Aziz <khalid.aziz-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> ---
>  drivers/of/address.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Rob
--
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 v14 00/11] mux controller abstraction and iio/i2c muxes
From: Philipp Zabel @ 2017-04-24 14:10 UTC (permalink / raw)
  To: Peter Rosin
  Cc: Jonathan Cameron, linux-kernel, Greg Kroah-Hartman, Wolfram Sang,
	Rob Herring, Mark Rutland, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Jonathan Corbet, linux-i2c, devicetree,
	linux-iio, linux-doc, Andrew Morton, Colin Ian King,
	Paul Gortmaker, kernel
In-Reply-To: <2d978956-d247-917d-4150-a6723917a733@axentia.se>

On Mon, 2017-04-24 at 13:37 +0200, Peter Rosin wrote:
> On 2017-04-24 12:52, Philipp Zabel wrote:
> > On Mon, 2017-04-24 at 10:36 +0200, Peter Rosin wrote:
> >> Hi!
> >>
> >> The big change since v13 is that the mux state is now locked with a mutex
> >> instead of an rwsem. Other that that, it is mostly restructuring and doc
> >> changes. There are a few other "real" changes as well, but those changes
> >> feel kind of minor. I guess what I'm trying to say is that although the
> >> list of changes for v14 is longish, it's still basically the same as last
> >> time.
> > 
> > I have hooked this up to the video-multiplexer and now I trigger
> > the lockdep debug_check_no_locks_held error message when selecting the
> > mux input from userspace:
> > 
> > $ media-ctl --links "'imx6-mipi-csi2':1->'ipu1_csi0_mux':0[1]"
> > [   66.258368] 
> > [   66.259919] =====================================
> > [   66.265369] [ BUG: media-ctl/258 still has locks held! ]
> > [   66.270810] 4.11.0-rc8-20170424-1+ #1305 Not tainted
> > [   66.275863] -------------------------------------
> > [   66.282158] 1 lock held by media-ctl/258:
> > [   66.286464]  #0:  (&mux->lock){+.+.+.}, at: [<8074a6c0>] mux_control_select+0x24/0x50
> > [   66.294424] 
> > [   66.294424] stack backtrace:
> > [   66.298980] CPU: 0 PID: 258 Comm: media-ctl Not tainted 4.11.0-rc8+ #1305
> > [   66.306771] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
> > [   66.313334] Backtrace: 
> > [   66.315858] [<8010e5a4>] (dump_backtrace) from [<8010e8ac>] (show_stack+0x20/0x24)
> > [   66.323473]  r7:80e518d0 r6:80e518d0 r5:600d0013 r4:00000000
> > [   66.329190] [<8010e88c>] (show_stack) from [<80496cf4>] (dump_stack+0xb0/0xdc)
> > [   66.336470] [<80496c44>] (dump_stack) from [<8017e404>] (debug_check_no_locks_held+0xb8/0xbc)
> > [   66.345043]  r9:ae8566b8 r8:ad88dc84 r7:ad88df58 r6:ad88dc84 r5:ad88df58 r4:ae856400
> > [   66.352837] [<8017e34c>] (debug_check_no_locks_held) from [<8012b258>] (do_exit+0x79c/0xcc8)
> > [   66.361321] [<8012aabc>] (do_exit) from [<8012d25c>] (do_group_exit+0x4c/0xcc)
> > [   66.368581]  r7:000000f8
> > [   66.371161] [<8012d210>] (do_group_exit) from [<8012d2fc>] (__wake_up_parent+0x0/0x30)
> > [   66.379120]  r7:000000f8 r6:76f71798 r5:00000000 r4:00000001
> > [   66.384837] [<8012d2dc>] (SyS_exit_group) from [<80109380>] (ret_fast_syscall+0x0/0x1c)
> > 
> > That correctly warns that the media-ctl process caused the mux->lock to
> > be locked and still held when the process exited. Do we need a usage
> > counter based mechanism for muxes that are (indirectly) controlled from
> > userspace?
[...]
> The question then becomes how to best tell the mux core that you want
> an exclusive mux. I see two options. Either you declare a mux controller
> as exclusive up front somehow (in the device tree presumably), or you
> add a mux_control_get_exclusive call that makes further calls to
> mux_control_get{,_exclusive} fail with -EBUSY. I think I like the
> latter better, if that can be made to work...

How about an atomic use_count on the mux_control, a bool shared that is
only set by the first consumer, and controls whether selecting locks?

----------8<----------
diff --git a/drivers/mux/mux-core.c b/drivers/mux/mux-core.c
index c02fa4dd2d099..1d18bb7f15e07 100644
--- a/drivers/mux/mux-core.c
+++ b/drivers/mux/mux-core.c
@@ -372,11 +372,12 @@ int mux_control_select(struct mux_control *mux, unsigned int state)
 {
 	int ret;
 
-	mutex_lock(&mux->lock);
+	if (mux->shared)
+		mutex_lock(&mux->lock);
 
 	ret = __mux_control_select(mux, state);
 
-	if (ret < 0)
+	if (ret < 0 && mux->shared)
 		mutex_unlock(&mux->lock);
 
 	return ret;
@@ -399,12 +400,12 @@ int mux_control_try_select(struct mux_control *mux, unsigned int state)
 {
 	int ret;
 
-	if (!mutex_trylock(&mux->lock))
+	if (mux->shared && !mutex_trylock(&mux->lock))
 		return -EBUSY;
 
 	ret = __mux_control_select(mux, state);
 
-	if (ret < 0)
+	if (ret < 0 && mux->shared)
 		mutex_unlock(&mux->lock);
 
 	return ret;
@@ -427,7 +428,8 @@ int mux_control_deselect(struct mux_control *mux)
 	    mux->idle_state != mux->cached_state)
 		ret = mux_control_set(mux, mux->idle_state);
 
-	mutex_unlock(&mux->lock);
+	if (mux->shared)
+		mutex_unlock(&mux->lock);
 
 	return ret;
 }
@@ -447,18 +449,13 @@ static struct mux_chip *of_find_mux_chip_by_node(struct device_node *np)
 	return dev ? to_mux_chip(dev) : NULL;
 }
 
-/**
- * mux_control_get() - Get the mux-control for a device.
- * @dev: The device that needs a mux-control.
- * @mux_name: The name identifying the mux-control.
- *
- * Return: A pointer to the mux-control, or an ERR_PTR with a negative errno.
- */
-struct mux_control *mux_control_get(struct device *dev, const char *mux_name)
+static struct mux_control *__mux_control_get(struct device *dev,
+					     const char *mux_name, bool shared)
 {
 	struct device_node *np = dev->of_node;
 	struct of_phandle_args args;
 	struct mux_chip *mux_chip;
+	struct mux_control *mux;
 	unsigned int controller;
 	int index = 0;
 	int ret;
@@ -505,9 +502,56 @@ struct mux_control *mux_control_get(struct device *dev, const char *mux_name)
 	}
 
 	get_device(&mux_chip->dev);
-	return &mux_chip->mux[controller];
+
+	mux = &mux_chip->mux[controller];
+
+	ret = atomic_inc_return(&mux->use_count);
+	if (ret == 1 && shared)
+		mux->shared = true;
+	if (ret > 1 && (!shared || !mux->shared)) {
+		atomic_dec(&mux->use_count);
+		put_device(&mux_chip->dev);
+		return ERR_PTR(-EBUSY);
+	}
+
+	return mux;
+}
+
+/**
+ * mux_control_get_shared() - Get the mux-control for a device.
+ * @dev: The device that needs a mux-control.
+ * @mux_name: The name identifying the mux-control.
+ *
+ * The returned mux control is in shared mode and can not be controlled from
+ * userspace. The mux control lock is taken when the mux is selected and
+ * released when the mux is deselected.
+ *
+ * Return: A pointer to the mux-control, or an ERR_PTR with a negative errno.
+ */
+struct mux_control *mux_control_get_shared(struct device *dev,
+					   const char *mux_name)
+{
+	return __mux_control_get(dev, mux_name, true);
+}
+EXPORT_SYMBOL_GPL(mux_control_get_shared);
+
+/**
+ * mux_control_get_exclusive() - Get the mux-control for a device.
+ * @dev: The device that needs a mux-control.
+ * @mux_name: The name identifying the mux-control.
+ *
+ * The returned mux control is in exclusive mode and does not lock the mux
+ * control lock when the mux is selected. The exclusive consumer has to take
+ * care of locking, if necessary.
+ *
+ * Return: A pointer to the mux-control, or an ERR_PTR with a negative errno.
+ */
+struct mux_control *mux_control_get_exclusive(struct device *dev,
+					      const char *mux_name)
+{
+	return __mux_control_get(dev, mux_name, false);
 }
-EXPORT_SYMBOL_GPL(mux_control_get);
+EXPORT_SYMBOL_GPL(mux_control_get_exclusive);
 
 /**
  * mux_control_put() - Put away the mux-control for good.
@@ -517,6 +561,7 @@ EXPORT_SYMBOL_GPL(mux_control_get);
  */
 void mux_control_put(struct mux_control *mux)
 {
+	atomic_dec(&mux->use_count);
 	put_device(&mux->chip->dev);
 }
 EXPORT_SYMBOL_GPL(mux_control_put);
@@ -528,16 +573,9 @@ static void devm_mux_control_release(struct device *dev, void *res)
 	mux_control_put(mux);
 }
 
-/**
- * devm_mux_control_get() - Get the mux-control for a device, with resource
- *			    management.
- * @dev: The device that needs a mux-control.
- * @mux_name: The name identifying the mux-control.
- *
- * Return: Pointer to the mux-control, or an ERR_PTR with a negative errno.
- */
-struct mux_control *devm_mux_control_get(struct device *dev,
-					 const char *mux_name)
+static struct mux_control *__devm_mux_control_get(struct device *dev,
+						  const char *mux_name,
+						  bool shared)
 {
 	struct mux_control **ptr, *mux;
 
@@ -545,7 +583,7 @@ struct mux_control *devm_mux_control_get(struct device *dev,
 	if (!ptr)
 		return ERR_PTR(-ENOMEM);
 
-	mux = mux_control_get(dev, mux_name);
+	mux = __mux_control_get(dev, mux_name, shared);
 	if (IS_ERR(mux)) {
 		devres_free(ptr);
 		return mux;
@@ -556,7 +594,36 @@ struct mux_control *devm_mux_control_get(struct device *dev,
 
 	return mux;
 }
-EXPORT_SYMBOL_GPL(devm_mux_control_get);
+
+/**
+ * devm_mux_control_get_shared() - Get the mux-control for a device, with
+ *				   resource management.
+ * @dev: The device that needs a mux-control.
+ * @mux_name: The name identifying the mux-control.
+ *
+ * Return: Pointer to the mux-control, or an ERR_PTR with a negative errno.
+ */
+struct mux_control *devm_mux_control_get_shared(struct device *dev,
+						const char *mux_name)
+{
+	return __devm_mux_control_get(dev, mux_name, true);
+}
+EXPORT_SYMBOL_GPL(devm_mux_control_get_shared);
+
+/**
+ * devm_mux_control_get_exclusive() - Get the mux-control for a device, with
+ *				      resource management.
+ * @dev: The device that needs a mux-control.
+ * @mux_name: The name identifying the mux-control.
+ *
+ * Return: Pointer to the mux-control, or an ERR_PTR with a negative errno.
+ */
+struct mux_control *devm_mux_control_get_exclusive(struct device *dev,
+						   const char *mux_name)
+{
+	return __devm_mux_control_get(dev, mux_name, false);
+}
+EXPORT_SYMBOL_GPL(devm_mux_control_get_exclusive);
 
 static int devm_mux_control_match(struct device *dev, void *res, void *data)
 {
diff --git a/include/linux/mux/consumer.h b/include/linux/mux/consumer.h
index a2a61834bd3ae..26634c3dda124 100644
--- a/include/linux/mux/consumer.h
+++ b/include/linux/mux/consumer.h
@@ -23,11 +23,16 @@ int __must_check mux_control_try_select(struct mux_control *mux,
 					unsigned int state);
 int mux_control_deselect(struct mux_control *mux);
 
-struct mux_control *mux_control_get(struct device *dev, const char *mux_name);
+struct mux_control *mux_control_get_exclusive(struct device *dev,
+					      const char *mux_name);
+struct mux_control *mux_control_get_shared(struct device *dev,
+					   const char *mux_name);
 void mux_control_put(struct mux_control *mux);
 
-struct mux_control *devm_mux_control_get(struct device *dev,
-					 const char *mux_name);
+struct mux_control *devm_mux_control_get_exclusive(struct device *dev,
+						   const char *mux_name);
+struct mux_control *devm_mux_control_get_shared(struct device *dev,
+						const char *mux_name);
 void devm_mux_control_put(struct device *dev, struct mux_control *mux);
 
 #endif /* _LINUX_MUX_CONSUMER_H */
diff --git a/include/linux/mux/driver.h b/include/linux/mux/driver.h
index 95269f40670a7..0ac5d3db9cd3e 100644
--- a/include/linux/mux/driver.h
+++ b/include/linux/mux/driver.h
@@ -31,17 +31,20 @@ struct mux_control_ops {
 
 /**
  * struct mux_control -	Represents a mux controller.
- * @lock:		Protects the mux controller state.
+ * @lock:		Protects the mux controller state, if shared.
  * @chip:		The mux chip that is handling this mux controller.
  * @cached_state:	The current mux controller state, or -1 if none.
  * @states:		The number of mux controller states.
  * @idle_state:		The mux controller state to use when inactive, or one
  *			of MUX_IDLE_AS_IS and MUX_IDLE_DISCONNECT.
+ * @use_count:		Number of consumers that have requested this mux.
+ * @shared:		True if the mux control was requested as shared and
+ *			must therefore be locked.
  *
  * Mux drivers may only change @states and @idle_state, and may only do so
  * between allocation and registration of the mux controller. Specifically,
- * @cached_state is internal to the mux core and should never be written by
- * mux drivers.
+ * @cached_state, @use_count, and @shared are internal to the mux core and
+ * should never be written by mux drivers.
  */
 struct mux_control {
 	struct mutex lock; /* protects the state of the mux */
@@ -51,6 +54,9 @@ struct mux_control {
 
 	unsigned int states;
 	int idle_state;
+
+	atomic_t use_count;
+	bool shared;
 };
 
 /**
---------->8----------

regards
Philipp

^ permalink raw reply related


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