Devicetree
 help / color / mirror / Atom feed
* RE: [PATCH V3 8/9] thermal: da9062/61: Thermal junction temperature monitoring driver
From: Steve Twiss @ 2016-11-09 18:20 UTC (permalink / raw)
  To: Lukasz Luba, Eduardo Valentin, LINUX-KERNEL, LINUX-PM, Zhang Rui
  Cc: DEVICETREE, Dmitry Torokhov, Guenter Roeck, LINUX-INPUT,
	LINUX-WATCHDOG, Lee Jones, Liam Girdwood, Mark Brown,
	Mark Rutland, Rob Herring, Support Opensource, Wim Van Sebroeck
In-Reply-To: <b5789f09-0053-f0fc-ee35-c8c32969852b@arm.com>

On 02 November 2016 13:29, Lukasz Luba wrote:
[...]

> Apart from these 2 comments, 10sec is not to long
> (waiting for the temperature change)?

Hi Lukasz,

Are you saying the maximum polling time is too long or too short if it
is fixed in the driver at 10 seconds?

Certainly 10 seconds can be seen as either too long or too short a time
when waiting for the temperature to fall-back below a threshold.
But, this maximum polling time will be application dependent I think.

However, this is a repeated polling event notifying of a warning
over-temperature condition, so, it is already known that the
temperature is above the threshold and action should already be
in progress to reduce the temperature.

#define DA9062_DEFAULT_POLLING_MS_PERIOD	3000
#define DA9062_MAX_POLLING_MS_PERIOD		10000
#define DA9062_MIN_POLLING_MS_PERIOD		1000

The TEMP_WARN first level temperature supervision is intended for
non-invasive temperature controlling measures for cooling the system
and are left to the host software. This first level temperature
TEMP_WARN (125 degC) is only +15degC off the next TEMP_CRIT
(140 degC) temperature threshold. And this TEMP_CRIT is where
the hardware will automatically shutdown.

I suppose it all depends on how fast the temperature is expected to
rise and fall.

In any case, this 10 second polling maximum value was provided as part
of guidance from a specific solution with this hardware. It would be expected
that any final implementation will also include a notify() function and any
of these settings could be altered to match the application where
appropriate.

I've added a comment above these defined variables for the next code
patch.

> On 31/10/16 16:02, Steve Twiss wrote:
> > From: Steve Twiss <stwiss.opensource@diasemi.com>
> >
> > +static int da9062_thermal_probe(struct platform_device *pdev)
> > +{
> > +	struct da9062 *chip = dev_get_drvdata(pdev->dev.parent);
> > +	struct da9062_thermal *thermal;
> > +	unsigned int pp_tmp = DA9062_DEFAULT_POLLING_MS_PERIOD;
> > +	const struct of_device_id *match;
> > +	int ret = 0;
> > +
> > +	match = of_match_node(da9062_compatible_reg_id_table,
> > +			      pdev->dev.of_node);
> > +	if (!match)
> > +		return -ENXIO;
> > +
> > +	if (pdev->dev.of_node) {
> > +		if (!of_property_read_u32(pdev->dev.of_node,
> > +					"dlg,tjunc-temp-polling-period-ms",
> > +					&pp_tmp)) {
> > +			if (pp_tmp < DA9062_MIN_POLLING_MS_PERIOD ||
> > +				pp_tmp > DA9062_MAX_POLLING_MS_PERIOD)
> > +				pp_tmp = DA9062_DEFAULT_POLLING_MS_PERIOD;
>
> Maybe it's worth to add some print here just to mention about
> the DT value out of range. When you saw a dmesg with
> this print on some bug report, you would know about wrong DT entry
> (even if debug was not set).

I can add a dev_warn() here explaining the invalid configuration.

[...]

> > +static int da9062_thermal_remove(struct platform_device *pdev)
> > +{
> > +	struct	da9062_thermal *thermal = platform_get_drvdata(pdev);
> > +
> > +	free_irq(thermal->irq, thermal);
> > +	thermal_zone_device_unregister(thermal->zone);
> > +	cancel_delayed_work_sync(&thermal->work);
>
> You should change the order for these two functions
> and cancel the work before unregistering thermal zone device.

ok

Regards,
Steve

^ permalink raw reply

* Re: [PATCH v6 4/4] of/fdt: mark hotpluggable memory
From: Rob Herring @ 2016-11-09 18:12 UTC (permalink / raw)
  To: Reza Arbab
  Cc: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
	Andrew Morton, Frank Rowand, Thomas Gleixner, Ingo Molnar,
	H. Peter Anvin, linuxppc-dev, linux-mm@kvack.org,
	devicetree@vger.kernel.org, Bharata B Rao, Nathan Fontenot,
	Stewart Smith, Alistair Popple, Balbir Singh, Aneesh Kumar K.V
In-Reply-To: <1478562276-25539-5-git-send-email-arbab@linux.vnet.ibm.com>

On Mon, Nov 7, 2016 at 5:44 PM, Reza Arbab <arbab@linux.vnet.ibm.com> wrote:
> When movable nodes are enabled, any node containing only hotpluggable
> memory is made movable at boot time.
>
> On x86, hotpluggable memory is discovered by parsing the ACPI SRAT,
> making corresponding calls to memblock_mark_hotplug().
>
> If we introduce a dt property to describe memory as hotpluggable,
> configs supporting early fdt may then also do this marking and use
> movable nodes.
>
> Signed-off-by: Reza Arbab <arbab@linux.vnet.ibm.com>
> ---
>  drivers/of/fdt.c | 6 ++++++
>  mm/Kconfig       | 2 +-
>  2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index c89d5d2..2cf1d66 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -1015,6 +1015,7 @@ int __init early_init_dt_scan_memory(unsigned long node, const char *uname,
>         const char *type = of_get_flat_dt_prop(node, "device_type", NULL);
>         const __be32 *reg, *endp;
>         int l;
> +       bool hotpluggable;
>
>         /* We are scanning "memory" nodes only */
>         if (type == NULL) {
> @@ -1034,6 +1035,7 @@ int __init early_init_dt_scan_memory(unsigned long node, const char *uname,
>                 return 0;
>
>         endp = reg + (l / sizeof(__be32));
> +       hotpluggable = of_get_flat_dt_prop(node, "linux,hotpluggable", NULL);

Memory being hotpluggable doesn't seem like a linux property to me.
I'd drop the linux prefix. Also, this needs to be documented.

Rob

^ permalink raw reply

* [PATCH] arm64: Add DTS support for FSL's LS1012A SoC
From: Harninder Rai @ 2016-11-09 18:11 UTC (permalink / raw)
  To: devicetree, shawnguo, robh+dt, mark.rutland
  Cc: oss, Harninder Rai, Bhaskar Upadhaya, linux-arm-kernel

Add the device tree support for FSL LS1012A SoC.
    Following levels of DTSI/DTS files have been created for the LS1012A
    SoC family:

            - fsl-ls1012a.dtsi:
                    DTS-Include file for FSL LS1012A SoC.

            - fsl-ls1012a-frdm.dts:
                    DTS file for FSL LS1012A FRDM board.

            - fsl-ls1012a-qds.dts:
                    DTS file for FSL LS1012A QDS board.

            - fsl-ls1012a-rdb.dts:
                    DTS file for FSL LS1012A RDB board.

Signed-off-by: Harninder Rai <harninder.rai@nxp.com>
Signed-off-by: Bhaskar Upadhaya <Bhaskar.Upadhaya@nxp.com>
---
 arch/arm64/boot/dts/freescale/Makefile             |   3 +
 arch/arm64/boot/dts/freescale/fsl-ls1012a-frdm.dts | 115 ++++++++++
 arch/arm64/boot/dts/freescale/fsl-ls1012a-qds.dts  | 128 +++++++++++
 arch/arm64/boot/dts/freescale/fsl-ls1012a-rdb.dts  |  59 +++++
 arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi     | 248 +++++++++++++++++++++
 5 files changed, 553 insertions(+)
 create mode 100644 arch/arm64/boot/dts/freescale/fsl-ls1012a-frdm.dts
 create mode 100644 arch/arm64/boot/dts/freescale/fsl-ls1012a-qds.dts
 create mode 100644 arch/arm64/boot/dts/freescale/fsl-ls1012a-rdb.dts
 create mode 100644 arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi

diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
index 6602718..39db645 100644
--- a/arch/arm64/boot/dts/freescale/Makefile
+++ b/arch/arm64/boot/dts/freescale/Makefile
@@ -1,3 +1,6 @@
+dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-ls1012a-frdm.dtb
+dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-ls1012a-qds.dtb
+dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-ls1012a-rdb.dtb
 dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-ls1043a-qds.dtb
 dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-ls1043a-rdb.dtb
 dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-ls1046a-qds.dtb
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1012a-frdm.dts b/arch/arm64/boot/dts/freescale/fsl-ls1012a-frdm.dts
new file mode 100644
index 0000000..1f2da79
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1012a-frdm.dts
@@ -0,0 +1,115 @@
+/*
+ * Device Tree file for Freescale LS1012A Freedom Board.
+ *
+ * Copyright 2016, Freescale Semiconductor
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPLv2 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 library 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 library 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.
+ */
+/dts-v1/;
+
+#include "fsl-ls1012a.dtsi"
+
+/ {
+	model = "LS1012A Freedom Board";
+	compatible = "fsl,ls1012a-frdm", "fsl,ls1012a";
+
+	sys_mclk: clock-mclk {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <25000000>;
+	};
+
+	reg_1p8v: regulator@0 {
+		compatible = "regulator-fixed";
+		regulator-name = "1P8V";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <1800000>;
+		regulator-always-on;
+	};
+
+	sound {
+		compatible = "simple-audio-card";
+		simple-audio-card,format = "i2s";
+		simple-audio-card,widgets =
+			"Microphone", "Microphone Jack",
+			"Headphone", "Headphone Jack",
+			"Speaker", "Speaker Ext",
+			"Line", "Line In Jack";
+		simple-audio-card,routing =
+			"MIC_IN", "Microphone Jack",
+			"Microphone Jack", "Mic Bias",
+			"LINE_IN", "Line In Jack",
+			"Headphone Jack", "HP_OUT",
+			"Speaker Ext", "LINE_OUT";
+
+		simple-audio-card,cpu {
+			sound-dai = <&sai2>;
+			frame-master;
+			bitclock-master;
+		};
+
+		simple-audio-card,codec {
+			sound-dai = <&codec>;
+			frame-master;
+			bitclock-master;
+			system-clock-frequency = <25000000>;
+		};
+	};
+};
+
+&i2c0 {
+	status = "okay";
+
+	codec: sgtl5000@a {
+		#sound-dai-cells = <0>;
+		compatible = "fsl,sgtl5000";
+		reg = <0xa>;
+		VDDA-supply = <&reg_1p8v>;
+		VDDIO-supply = <&reg_1p8v>;
+		clocks = <&sys_mclk>;
+	};
+};
+
+&duart0 {
+	status = "okay";
+};
+
+&sai2 {
+	status = "okay";
+};
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1012a-qds.dts b/arch/arm64/boot/dts/freescale/fsl-ls1012a-qds.dts
new file mode 100644
index 0000000..ca680a7
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1012a-qds.dts
@@ -0,0 +1,128 @@
+/*
+ * Device Tree file for Freescale LS1012A QDS Board.
+ *
+ * Copyright 2016, Freescale Semiconductor
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPLv2 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 library 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 library 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.
+ */
+/dts-v1/;
+
+#include "fsl-ls1012a.dtsi"
+
+/ {
+	model = "LS1012A QDS Board";
+	compatible = "fsl,ls1012a-qds", "fsl,ls1012a";
+
+	sys_mclk: clock-mclk {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <24576000>;
+	};
+
+	reg_3p3v: regulator@0 {
+		compatible = "regulator-fixed";
+		regulator-name = "3P3V";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		regulator-always-on;
+	};
+
+	sound {
+		compatible = "simple-audio-card";
+		simple-audio-card,format = "i2s";
+		simple-audio-card,widgets =
+			"Microphone", "Microphone Jack",
+			"Headphone", "Headphone Jack",
+			"Speaker", "Speaker Ext",
+			"Line", "Line In Jack";
+		simple-audio-card,routing =
+			"MIC_IN", "Microphone Jack",
+			"Microphone Jack", "Mic Bias",
+			"LINE_IN", "Line In Jack",
+			"Headphone Jack", "HP_OUT",
+			"Speaker Ext", "LINE_OUT";
+
+		simple-audio-card,cpu {
+			sound-dai = <&sai2>;
+			frame-master;
+			bitclock-master;
+		};
+
+		simple-audio-card,codec {
+			sound-dai = <&codec>;
+			frame-master;
+			bitclock-master;
+			system-clock-frequency = <24576000>;
+		};
+	};
+};
+
+&i2c0 {
+	status = "okay";
+
+	pca9547@77 {
+		compatible = "nxp,pca9547";
+		reg = <0x77>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		i2c@4 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <0x4>;
+
+			codec: sgtl5000@a {
+				#sound-dai-cells = <0>;
+				compatible = "fsl,sgtl5000";
+				reg = <0xa>;
+				VDDA-supply = <&reg_3p3v>;
+				VDDIO-supply = <&reg_3p3v>;
+				clocks = <&sys_mclk>;
+			};
+		};
+	};
+};
+
+&duart0 {
+	status = "okay";
+};
+
+&sai2 {
+	status = "okay";
+};
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1012a-rdb.dts b/arch/arm64/boot/dts/freescale/fsl-ls1012a-rdb.dts
new file mode 100644
index 0000000..924dad6
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1012a-rdb.dts
@@ -0,0 +1,59 @@
+/*
+ * Device Tree file for Freescale LS1012A RDB Board.
+ *
+ * Copyright 2016, Freescale Semiconductor
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPLv2 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 library 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 library 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.
+ */
+/dts-v1/;
+
+#include "fsl-ls1012a.dtsi"
+
+/ {
+	model = "LS1012A RDB Board";
+	compatible = "fsl,ls1012a-rdb", "fsl,ls1012a";
+};
+
+&i2c0 {
+	status = "okay";
+};
+
+&duart0 {
+	status = "okay";
+};
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi
new file mode 100644
index 0000000..0bf5b64
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi
@@ -0,0 +1,248 @@
+/*
+ * Device Tree Include file for Freescale Layerscape-1012A family SoC.
+ *
+ * Copyright 2016, Freescale Semiconductor
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPLv2 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 library 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 library 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.
+ */
+
+#include <dt-bindings/interrupt-controller/irq.h>
+
+/ {
+	compatible = "fsl,ls1012a";
+	interrupt-parent = <&gic>;
+	#address-cells = <2>;
+	#size-cells = <2>;
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		cpu0: cpu@0 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a53";
+			reg = <0x0>;
+			clocks = <&clockgen 1 0>;
+			#cooling-cells = <2>;
+		};
+	};
+
+	sysclk: sysclk {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <100000000>;
+		clock-output-names = "sysclk";
+	};
+
+	timer {
+		compatible = "arm,armv8-timer";
+			     /* Physical Secure PPI */
+		interrupts = <1 13 IRQ_TYPE_LEVEL_LOW>,
+			     /* Physical Non-Secure PPI */
+			     <1 14 IRQ_TYPE_LEVEL_LOW>,
+			     /* Virtual PPI */
+			     <1 11 IRQ_TYPE_LEVEL_LOW>,
+			     /* Hypervisor PPI */
+			     <1 10 IRQ_TYPE_LEVEL_LOW>;
+	};
+
+	pmu {
+		compatible = "arm,armv8-pmuv3";
+		interrupts = <0 106 IRQ_TYPE_LEVEL_LOW>;
+	};
+
+	gic: interrupt-controller@1400000 {
+		compatible = "arm,gic-400";
+		#interrupt-cells = <3>;
+		interrupt-controller;
+		reg = <0x0 0x1401000 0 0x1000>, /* GICD */
+		      <0x0 0x1402000 0 0x2000>, /* GICC */
+		      <0x0 0x1404000 0 0x2000>, /* GICH */
+		      <0x0 0x1406000 0 0x2000>; /* GICV */
+		interrupts = <1 9 IRQ_TYPE_LEVEL_LOW>;
+	};
+
+	reboot {
+		compatible = "syscon-reboot";
+		regmap = <&dcfg>;
+		offset = <0xb0>;
+		mask = <0x02>;
+	};
+
+	soc {
+		compatible = "simple-bus";
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		clockgen: clocking@1ee1000 {
+			compatible = "fsl,ls1012a-clockgen";
+			reg = <0x0 0x1ee1000 0x0 0x1000>;
+			#clock-cells = <2>;
+			clocks = <&sysclk>;
+		};
+
+		scfg: scfg@1570000 {
+			compatible = "fsl,ls1012a-scfg", "syscon";
+			reg = <0x0 0x1570000 0x0 0x10000>;
+			big-endian;
+		};
+
+		dcfg: dcfg@1ee0000 {
+			compatible = "fsl,ls1012a-dcfg",
+				     "syscon";
+			reg = <0x0 0x1ee0000 0x0 0x10000>;
+			big-endian;
+		};
+
+		i2c0: i2c@2180000 {
+			compatible = "fsl,vf610-i2c";
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <0x0 0x2180000 0x0 0x10000>;
+			interrupts = <0 56 IRQ_TYPE_LEVEL_LOW>;
+			clocks = <&clockgen 4 0>;
+			status = "disabled";
+		};
+
+		i2c1: i2c@2190000 {
+			compatible = "fsl,vf610-i2c";
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <0x0 0x2190000 0x0 0x10000>;
+			interrupts = <0 57 IRQ_TYPE_LEVEL_LOW>;
+			clocks = <&clockgen 4 0>;
+			status = "disabled";
+		};
+
+		duart0: serial@21c0500 {
+			compatible = "fsl,ns16550", "ns16550a";
+			reg = <0x00 0x21c0500 0x0 0x100>;
+			interrupts = <0 54 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&clockgen 4 0>;
+		};
+
+		duart1: serial@21c0600 {
+			compatible = "fsl,ns16550", "ns16550a";
+			reg = <0x00 0x21c0600 0x0 0x100>;
+			interrupts = <0 54 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&clockgen 4 0>;
+		};
+
+		gpio0: gpio@2300000 {
+			compatible = "fsl,qoriq-gpio";
+			reg = <0x0 0x2300000 0x0 0x10000>;
+			interrupts = <0 66 IRQ_TYPE_LEVEL_LOW>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+		};
+
+		gpio1: gpio@2310000 {
+			compatible = "fsl,qoriq-gpio";
+			reg = <0x0 0x2310000 0x0 0x10000>;
+			interrupts = <0 67 IRQ_TYPE_LEVEL_LOW>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+		};
+
+		wdog0: wdog@2ad0000 {
+			compatible = "fsl,ls1012a-wdt",
+				     "fsl,imx21-wdt";
+			reg = <0x0 0x2ad0000 0x0 0x10000>;
+			interrupts = <0 83 IRQ_TYPE_LEVEL_LOW>;
+			clocks = <&clockgen 4 0>;
+			big-endian;
+		};
+
+		sai1: sai@2b50000 {
+			#sound-dai-cells = <0>;
+			compatible = "fsl,vf610-sai";
+			reg = <0x0 0x2b50000 0x0 0x10000>;
+			interrupts = <0 148 IRQ_TYPE_LEVEL_LOW>;
+			clocks = <&clockgen 4 3>, <&clockgen 4 3>,
+				 <&clockgen 4 3>, <&clockgen 4 3>;
+			clock-names = "bus", "mclk1", "mclk2", "mclk3";
+			dma-names = "tx", "rx";
+			dmas = <&edma0 1 47>,
+			       <&edma0 1 46>;
+			status = "disabled";
+		};
+
+		sai2: sai@2b60000 {
+			#sound-dai-cells = <0>;
+			compatible = "fsl,vf610-sai";
+			reg = <0x0 0x2b60000 0x0 0x10000>;
+			interrupts = <0 149 IRQ_TYPE_LEVEL_LOW>;
+			clocks = <&clockgen 4 3>, <&clockgen 4 3>,
+				 <&clockgen 4 3>, <&clockgen 4 3>;
+			clock-names = "bus", "mclk1", "mclk2", "mclk3";
+			dma-names = "tx", "rx";
+			dmas = <&edma0 1 45>,
+			       <&edma0 1 44>;
+			status = "disabled";
+		};
+
+		edma0: edma@2c00000 {
+			#dma-cells = <2>;
+			compatible = "fsl,vf610-edma";
+			reg = <0x0 0x2c00000 0x0 0x10000>,
+			      <0x0 0x2c10000 0x0 0x10000>,
+			      <0x0 0x2c20000 0x0 0x10000>;
+			interrupts = <0 103 IRQ_TYPE_LEVEL_LOW>,
+				     <0 103 IRQ_TYPE_LEVEL_LOW>;
+			interrupt-names = "edma-tx", "edma-err";
+			dma-channels = <32>;
+			big-endian;
+			clock-names = "dmamux0", "dmamux1";
+			clocks = <&clockgen 4 3>,
+				 <&clockgen 4 3>;
+		};
+
+		sata: sata@3200000 {
+			compatible = "fsl,ls1012a-ahci";
+			reg = <0x0 0x3200000 0x0 0x10000>;
+			interrupts = <0 69 IRQ_TYPE_LEVEL_LOW>;
+			clocks = <&clockgen 4 0>;
+		};
+	};
+};
-- 
2.7.4

^ permalink raw reply related

* [PATCH 3/3] dt-bindings: clockgen: Add compatible string for LS1012A
From: Harninder Rai @ 2016-11-09 18:10 UTC (permalink / raw)
  To: devicetree, shawnguo, robh+dt, mark.rutland
  Cc: oss, Harninder Rai, Bhaskar Upadhaya, linux-arm-kernel

Signed-off-by: Harninder Rai <harninder.rai@nxp.com>
Signed-off-by: Bhaskar Upadhaya <Bhaskar.Upadhaya@nxp.com>
---
 Documentation/devicetree/bindings/clock/qoriq-clock.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/clock/qoriq-clock.txt b/Documentation/devicetree/bindings/clock/qoriq-clock.txt
index df9cb5a..aa3526f 100644
--- a/Documentation/devicetree/bindings/clock/qoriq-clock.txt
+++ b/Documentation/devicetree/bindings/clock/qoriq-clock.txt
@@ -31,6 +31,7 @@ Required properties:
 	* "fsl,t4240-clockgen"
 	* "fsl,b4420-clockgen"
 	* "fsl,b4860-clockgen"
+	* "fsl,ls1012a-clockgen"
 	* "fsl,ls1021a-clockgen"
 	* "fsl,ls1043a-clockgen"
 	* "fsl,ls1046a-clockgen"
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH] of/irq: improve error message on irq discovery process failure
From: Rob Herring @ 2016-11-09 18:05 UTC (permalink / raw)
  To: Guilherme G. Piccoli
  Cc: devicetree@vger.kernel.org, linuxppc-dev,
	linux-pci@vger.kernel.org, Frank Rowand
In-Reply-To: <1478700308-25481-1-git-send-email-gpiccoli@linux.vnet.ibm.com>

On Wed, Nov 9, 2016 at 8:05 AM, Guilherme G. Piccoli
<gpiccoli@linux.vnet.ibm.com> wrote:
> On PowerPC machines some PCI slots might not have Level-triggered
> interrupts capability (also know as Level Signaled Interrupts - LSI),
> leading of_irq_parse_pci() to complain by presenting error messages
> on the kernel log - in this case, the properties "interrupt-map" and
> "interrupt-map-mask" are not present on the device's node on device
> tree.
>
> This patch introduces a different message for this specific case,
> and it also reduces the level of the message from error to warning.
> Before this patch, when an adapter was plugged in a slot without Level
> interrupts capabilities, we saw generic error messages like this:
>
>     [54.239] pci 002d:70:00.0: of_irq_parse_pci() failed with rc=-22
>
> Now, with this applied, we see the following specific message:
>
>     [19.947] pci 0014:60:00.0: of_irq_parse_pci() gave up. The slot of this
>     device has no Level-triggered Interrupts capability.
>
> No functional changes were introduced.
>
> Signed-off-by: Guilherme G. Piccoli <gpiccoli@linux.vnet.ibm.com>
> ---
>  drivers/of/irq.c        | 5 ++++-
>  drivers/of/of_pci_irq.c | 8 +++++++-
>  2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/of/irq.c b/drivers/of/irq.c
> index 393fea8..1ad6882 100644
> --- a/drivers/of/irq.c
> +++ b/drivers/of/irq.c
> @@ -275,7 +275,10 @@ int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq)
>         of_node_put(ipar);
>         of_node_put(newpar);
>
> -       return -EINVAL;
> +       /* Positive non-zero return means no Level-triggered Interrupts
> +        * capability was found.
> +        */
> +       return ENOENT;

It's not really a normal pattern to return positive errno values. You
should return a negative value and check for that specific error value
or perhaps move the print statement into this function.

Rob

^ permalink raw reply

* [PATCH 2/3] Documentation: DT: add LS1012A compatible for SCFG and DCFG
From: Harninder Rai @ 2016-11-09 18:04 UTC (permalink / raw)
  To: devicetree, shawnguo, robh+dt, mark.rutland
  Cc: oss, Harninder Rai, Bhaskar Upadhaya, linux-arm-kernel

Signed-off-by: Harninder Rai <harninder.rai@nxp.com>
Signed-off-by: Bhaskar Upadhaya <Bhaskar.Upadhaya@nxp.com>
---
 Documentation/devicetree/bindings/arm/fsl.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/fsl.txt b/Documentation/devicetree/bindings/arm/fsl.txt
index 3b01338..c9c567a 100644
--- a/Documentation/devicetree/bindings/arm/fsl.txt
+++ b/Documentation/devicetree/bindings/arm/fsl.txt
@@ -108,7 +108,7 @@ status.
   - compatible: Should contain a chip-specific compatible string,
 	Chip-specific strings are of the form "fsl,<chip>-scfg",
 	The following <chip>s are known to be supported:
-	ls1021a, ls1043a, ls1046a, ls2080a.
+	ls1012a, ls1021a, ls1043a, ls1046a, ls2080a.
 
   - reg: should contain base address and length of SCFG memory-mapped registers
 
@@ -126,7 +126,7 @@ core start address and release the secondary core from holdoff and startup.
   - compatible: Should contain a chip-specific compatible string,
 	Chip-specific strings are of the form "fsl,<chip>-dcfg",
 	The following <chip>s are known to be supported:
-	ls1021a, ls1043a, ls1046a, ls2080a.
+	ls1012a, ls1021a, ls1043a, ls1046a, ls2080a.
 
   - reg : should contain base address and length of DCFG memory-mapped registers
 
-- 
2.7.4

^ permalink raw reply related

* [PATCH 1/3] Documentation: DT: Add entry for FSL LS1012A RDB, FRDM, QDS boards
From: Harninder Rai @ 2016-11-09 18:04 UTC (permalink / raw)
  To: devicetree, shawnguo, robh+dt, mark.rutland
  Cc: oss, Harninder Rai, Bhaskar Upadhaya, linux-arm-kernel

Signed-off-by: Harninder Rai <harninder.rai@nxp.com>
Signed-off-by: Bhaskar Upadhaya <Bhaskar.Upadhaya@nxp.com>
---
 Documentation/devicetree/bindings/arm/fsl.txt | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/fsl.txt b/Documentation/devicetree/bindings/arm/fsl.txt
index d6ee9c6..3b01338 100644
--- a/Documentation/devicetree/bindings/arm/fsl.txt
+++ b/Documentation/devicetree/bindings/arm/fsl.txt
@@ -139,6 +139,22 @@ Example:
 Freescale ARMv8 based Layerscape SoC family Device Tree Bindings
 ----------------------------------------------------------------
 
+LS1012A SoC
+Required root node properties:
+    - compatible = "fsl,ls1012a";
+
+LS1012A ARMv8 based RDB Board
+Required root node properties:
+    - compatible = "fsl,ls1012a-rdb", "fsl,ls1012a";
+
+LS1012A ARMv8 based FRDM Board
+Required root node properties:
+    - compatible = "fsl,ls1012a-frdm", "fsl,ls1012a";
+
+LS1012A ARMv8 based QDS Board
+Required root node properties:
+    - compatible = "fsl,ls1012a-qds", "fsl,ls1012a";
+
 LS1043A SoC
 Required root node properties:
     - compatible = "fsl,ls1043a";
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH v15 4/4] arm: dts: mt2701: Use real clock for UARTs
From: Matthias Brugger @ 2016-11-09 17:20 UTC (permalink / raw)
  To: Erin Lo, Mike Turquette, Stephen Boyd, Rob Herring
  Cc: Arnd Bergmann, Sascha Hauer, Daniel Kurtz, Philipp Zabel,
	devicetree, linux-arm-kernel, linux-kernel, linux-mediatek,
	linux-clk, srv_heupstream, ms.chen, robert.chou
In-Reply-To: <1478245388-1412-5-git-send-email-erin.lo@mediatek.com>



On 11/04/2016 08:43 AM, Erin Lo wrote:
> We used to use a fixed rate clock for the UARTs. Now that we have clock
> support we can associate the correct clocks to the UARTs and drop the
> 26MHz fixed rate UART clock.
>
> Signed-off-by: Erin Lo <erin.lo@mediatek.com>

Applied, thanks.

> ---
>  arch/arm/boot/dts/mt2701.dtsi | 18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/arch/arm/boot/dts/mt2701.dtsi b/arch/arm/boot/dts/mt2701.dtsi
> index c9a8dbf..7eab6f4 100644
> --- a/arch/arm/boot/dts/mt2701.dtsi
> +++ b/arch/arm/boot/dts/mt2701.dtsi
> @@ -73,12 +73,6 @@
>  		#clock-cells = <0>;
>  	};
>
> -	uart_clk: dummy26m {
> -		compatible = "fixed-clock";
> -		clock-frequency = <26000000>;
> -		#clock-cells = <0>;
> -	};
> -
>  	clk26m: oscillator@0 {
>  		compatible = "fixed-clock";
>  		#clock-cells = <0>;
> @@ -186,7 +180,8 @@
>  			     "mediatek,mt6577-uart";
>  		reg = <0 0x11002000 0 0x400>;
>  		interrupts = <GIC_SPI 51 IRQ_TYPE_LEVEL_LOW>;
> -		clocks = <&uart_clk>;
> +		clocks = <&pericfg CLK_PERI_UART0_SEL>, <&pericfg CLK_PERI_UART0>;
> +		clock-names = "baud", "bus";
>  		status = "disabled";
>  	};
>
> @@ -195,7 +190,8 @@
>  			     "mediatek,mt6577-uart";
>  		reg = <0 0x11003000 0 0x400>;
>  		interrupts = <GIC_SPI 52 IRQ_TYPE_LEVEL_LOW>;
> -		clocks = <&uart_clk>;
> +		clocks = <&pericfg CLK_PERI_UART1_SEL>, <&pericfg CLK_PERI_UART1>;
> +		clock-names = "baud", "bus";
>  		status = "disabled";
>  	};
>
> @@ -204,7 +200,8 @@
>  			     "mediatek,mt6577-uart";
>  		reg = <0 0x11004000 0 0x400>;
>  		interrupts = <GIC_SPI 53 IRQ_TYPE_LEVEL_LOW>;
> -		clocks = <&uart_clk>;
> +		clocks = <&pericfg CLK_PERI_UART2_SEL>, <&pericfg CLK_PERI_UART2>;
> +		clock-names = "baud", "bus";
>  		status = "disabled";
>  	};
>
> @@ -213,7 +210,8 @@
>  			     "mediatek,mt6577-uart";
>  		reg = <0 0x11005000 0 0x400>;
>  		interrupts = <GIC_SPI 54 IRQ_TYPE_LEVEL_LOW>;
> -		clocks = <&uart_clk>;
> +		clocks = <&pericfg CLK_PERI_UART3_SEL>, <&pericfg CLK_PERI_UART3>;
> +		clock-names = "baud", "bus";
>  		status = "disabled";
>  	};
>  };
>

^ permalink raw reply

* Re: [PATCH v2 0/7] soc: renesas: Identify SoC and register with the SoC bus
From: Geert Uytterhoeven @ 2016-11-09 17:19 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Greg Kroah-Hartman, Yangbo Lu, Simon Horman, Magnus Damm,
	Rob Herring, Mark Rutland, Dirk Behme, Linux-Renesas,
	linux-arm-kernel@lists.infradead.org,
	linuxppc-dev@lists.ozlabs.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, Pankaj Dubey,
	linux-samsung-soc@vger.kernel.org
In-Reply-To: <1626072.dpWPnWgGl9@wuerfel>

Hi Arnd,

On Wed, Nov 9, 2016 at 5:56 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Wednesday, November 9, 2016 2:34:33 PM CET Geert Uytterhoeven wrote:
>> > And Samsung.
>> > Shall I create the immutable branch now?
>>
>> Arnd: are you happy with the new patches and changes?
>
> I still had some comments for patch 7, but that shouldn't stop
> you from creating a branch for the first three so everyone can
> build on top of that.

Thanks!

What about patch [4/7]?
Haven't you received it? Your address was in the To-line for all 7 patches.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCHv2 2/2] gpio: xilinx: Add clock support
From: Sören Brinkmann @ 2016-11-09 17:18 UTC (permalink / raw)
  To: Shubhrajyoti Datta
  Cc: Shubhrajyoti Datta, linux-gpio, devicetree, Michal Simek
In-Reply-To: <CAKfKVtGxaWYEVPaU_G0Z7HJm4_ZAcTisZBj-BtSmAdL1+w+7xw@mail.gmail.com>

On Wed, 2016-11-09 at 12:16:58 +0530, Shubhrajyoti Datta wrote:
> On Wed, Nov 9, 2016 at 11:57 AM, Sören Brinkmann
> <soren.brinkmann@xilinx.com> wrote:
> > On Wed, 2016-11-09 at 10:39:51 +0530, Shubhrajyoti Datta wrote:
> >> Add basic clock support for xilinx gpio.
> >>
> >> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> >> ---
> >> v2 :
> >> no change
> >>
> >>  drivers/gpio/gpio-xilinx.c |   22 ++++++++++++++++++++++
> >>  1 files changed, 22 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/drivers/gpio/gpio-xilinx.c b/drivers/gpio/gpio-xilinx.c
> >> index 14b2a62..923cab8 100644
> >> --- a/drivers/gpio/gpio-xilinx.c
> >> +++ b/drivers/gpio/gpio-xilinx.c
> >> @@ -13,6 +13,7 @@
> >>   */
> >>
> >>  #include <linux/bitops.h>
> >> +#include <linux/clk.h>
> >>  #include <linux/init.h>
> >>  #include <linux/errno.h>
> >>  #include <linux/module.h>
> >> @@ -45,6 +46,7 @@
> >>   * @gpio_state: GPIO state shadow register
> >>   * @gpio_dir: GPIO direction shadow register
> >>   * @gpio_lock: Lock used for synchronization
> >> + * @clk: Clock resource for this controller
> >>   */
> >>  struct xgpio_instance {
> >>       struct of_mm_gpio_chip mmchip;
> >> @@ -52,6 +54,7 @@ struct xgpio_instance {
> >>       u32 gpio_state[2];
> >>       u32 gpio_dir[2];
> >>       spinlock_t gpio_lock[2];
> >> +     struct clk *clk;
> >>  };
> >>
> >>  static inline int xgpio_index(struct xgpio_instance *chip, int gpio)
> >> @@ -282,6 +285,7 @@ static int xgpio_remove(struct platform_device *pdev)
> >>       struct xgpio_instance *chip = platform_get_drvdata(pdev);
> >>
> >>       of_mm_gpiochip_remove(&chip->mmchip);
> >> +     clk_disable_unprepare(chip->clk);
> >>
> >>       return 0;
> >>  }
> >> @@ -307,6 +311,23 @@ static int xgpio_probe(struct platform_device *pdev)
> >>
> >>       platform_set_drvdata(pdev, chip);
> >>
> >> +     /* Retrieve GPIO clock */
> >> +     chip->clk = devm_clk_get(&pdev->dev, NULL);
> >
> > The driver should use the clock-name documented in the binding to do the
> > clock lookup.
> 
> My idea was to keep the clk name optional since there is only one clock.
> Or do you think we should mandate the name if clk is provided.

I'd make 'clock-names' mandatory if 'clocks' is present. That way there
won't be any trouble if this IP ever consumed additional clocks in the
future.

	Sören

^ permalink raw reply

* Re: [PATCH v2 0/7] soc: renesas: Identify SoC and register with the SoC bus
From: Arnd Bergmann @ 2016-11-09 16:56 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Greg Kroah-Hartman, Yangbo Lu, Simon Horman, Magnus Damm,
	Rob Herring, Mark Rutland, Dirk Behme, Linux-Renesas,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Pankaj Dubey,
	linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAMuHMdUmpMpizZpq1V-sLA8Cf2q5oOgOVxGOvKXqTHvn+Mj7Tg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Wednesday, November 9, 2016 2:34:33 PM CET Geert Uytterhoeven wrote:
> >
> > And Samsung.
> > Shall I create the immutable branch now?
> 
> Arnd: are you happy with the new patches and changes?

I still had some comments for patch 7, but that shouldn't stop
you from creating a branch for the first three so everyone can
build on top of that.

	Arnd
--
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 7/7] soc: renesas: Identify SoC and register with the SoC bus
From: Arnd Bergmann @ 2016-11-09 16:55 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Geert Uytterhoeven, Greg Kroah-Hartman, Yangbo Lu, Simon Horman,
	Magnus Damm, Rob Herring, Mark Rutland, devicetree, Dirk Behme,
	linux-kernel, linux-renesas-soc, linux-arm-kernel
In-Reply-To: <1477913455-5314-8-git-send-email-geert+renesas@glider.be>

On Monday, October 31, 2016 12:30:55 PM CET Geert Uytterhoeven wrote:

> v2:
>   - Drop SoC families and family names; use fixed "Renesas" instead,

I think I'd rather have seen the family names left in there, but it's
not important, so up to you.

>   - Use "renesas,prr" and "renesas,cccr" device nodes in DT if
>     available, else fall back to hardcoded addresses for compatibility
>     with existing DTBs,

I only see patches 2, 3, 5, and 7 in my inbox, so I'm lacking the DT
binding for these, among other things.

It does seem wrong to have a device node for a specific register though.
Shouldn't the node be for the block of registers that these are inside
of?

>   - Don't register the SoC bus if the chip ID register is missing,

Why? My objection was to hardcoding the register, not to registering
the device? I think I'd rather see the device registered with an
empty revision string.

> +#define CCCR   0xe600101c      /* Common Chip Code Register */
> +#define PRR    0xff000044      /* Product Register */
> +#define PRR3   0xfff00044      /* Product Register on R-Car Gen3 */
> +
> +static const struct of_device_id renesas_socs[] __initconst = {
> +#ifdef CONFIG_ARCH_R8A73A4
> +       { .compatible = "renesas,r8a73a4",      .data = (void *)PRR, },
> +#endif
> +#ifdef CONFIG_ARCH_R8A7740
> +       { .compatible = "renesas,r8a7740",      .data = (void *)CCCR, },
> +#endif

My preference here would be to list the register address only for
SoCs that are known to need them, while also having .dtb files that
don't have the nodes.

> +static int __init renesas_soc_init(void)
> +{
> +       struct soc_device_attribute *soc_dev_attr;
> +       const struct of_device_id *match;
> +       void __iomem *chipid = NULL;
> +       struct soc_device *soc_dev;
> +       struct device_node *np;
> +       unsigned int product;
> +
> +       np = of_find_matching_node_and_match(NULL, renesas_socs, &match);
> +       if (!np)
> +               return -ENODEV;
> +
> +       of_node_put(np);
> +
> +       /* Try PRR first, then CCCR, then hardcoded fallback */
> +       np = of_find_compatible_node(NULL, NULL, "renesas,prr");
> +       if (!np)
> +               np = of_find_compatible_node(NULL, NULL, "renesas,cccr");
> +       if (np) {
> +               chipid = of_iomap(np, 0);
> +               of_node_put(np);
> +       } else if (match->data) {
> +               chipid = ioremap((uintptr_t)match->data, 4);
> +       }
> +       if (!chipid)
> 

Here, I'd turn the order around and look for the DT nodes of the
devices first. Only if they are not found, look at the compatible
string of the root node. No need to search for a node though,
you know which one it is when you look for a compatible =
"renesas,r8a73a4".

	Arnd

^ permalink raw reply

* Re: [PATCH V5 2/3] ARM64 LPC: Add missing range exception for special ISA
From: liviu.dudau-5wv7dgnIgG8 @ 2016-11-09 16:50 UTC (permalink / raw)
  To: Gabriele Paoloni
  Cc: Yuanzhichang, catalin.marinas-5wv7dgnIgG8@public.gmane.org,
	will.deacon-5wv7dgnIgG8@public.gmane.org,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	bhelgaas-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org,
	mark.rutland-5wv7dgnIgG8@public.gmane.org,
	olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org,
	arnd-r2nGTMty4D4@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linuxarm,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	"minyard-HInyCGIudOg@public.gmane.org" <mi>
In-Reply-To: <EE11001F9E5DDD47B7634E2F8A612F2E1F8F27C0@lhreml507-mbx>

On Wed, Nov 09, 2016 at 04:16:17PM +0000, Gabriele Paoloni wrote:
> Hi Liviu
> 
> Thanks for reviewing
> 

[removed some irrelevant part of discussion, avoid crazy formatting]

> > > +/**
> > > + * addr_is_indirect_io - check whether the input taddr is for
> > indirectIO.
> > > + * @taddr: the io address to be checked.
> > > + *
> > > + * Returns 1 when taddr is in the range; otherwise return 0.
> > > + */
> > > +int addr_is_indirect_io(u64 taddr)
> > > +{
> > > +	if (arm64_extio_ops->start > taddr || arm64_extio_ops->end <
> > taddr)
> > 
> > start >= taddr ?
> 
> Nope... if  (taddr < arm64_extio_ops->start || taddr > arm64_extio_ops->end)
> then taddr is outside the range [start; end] and will return 0; otherwise
> it will return 1...

Oops, sorry, did not pay attention to the returned value. The check is
correct as it is, no need to change then.

> 
> > 
> > > +		return 0;
> > > +
> > > +	return 1;
> > > +}
> > >
> > >  BUILD_EXTIO(b, u8)
> > >
> > > diff --git a/drivers/of/address.c b/drivers/of/address.c
> > > index 02b2903..cc2a05d 100644
> > > --- a/drivers/of/address.c
> > > +++ b/drivers/of/address.c
> > > @@ -479,6 +479,50 @@ static int of_empty_ranges_quirk(struct
> > device_node *np)
> > >  	return false;
> > >  }
> > >
> > > +
> > > +/*
> > > + * of_isa_indirect_io - get the IO address from some isa reg
> > property value.
> > > + *	For some isa/lpc devices, no ranges property in ancestor node.
> > > + *	The device addresses are described directly in their regs
> > property.
> > > + *	This fixup function will be called to get the IO address of
> > isa/lpc
> > > + *	devices when the normal of_translation failed.
> > > + *
> > > + * @parent:	points to the parent dts node;
> > > + * @bus:		points to the of_bus which can be used to parse
> > address;
> > > + * @addr:	the address from reg property;
> > > + * @na:		the address cell counter of @addr;
> > > + * @presult:	store the address paresed from @addr;
> > > + *
> > > + * return 1 when successfully get the I/O address;
> > > + * 0 will return for some failures.
> > 
> > Bah, you are returning a signed int, why 0 for failure? Return a
> > negative value with
> > error codes. Otherwise change the return value into a bool.
> 
> Yes we'll move to bool
> 
> > 
> > > + */
> > > +static int of_get_isa_indirect_io(struct device_node *parent,
> > > +				struct of_bus *bus, __be32 *addr,
> > > +				int na, u64 *presult)
> > > +{
> > > +	unsigned int flags;
> > > +	unsigned int rlen;
> > > +
> > > +	/* whether support indirectIO */
> > > +	if (!indirect_io_enabled())
> > > +		return 0;
> > > +
> > > +	if (!of_bus_isa_match(parent))
> > > +		return 0;
> > > +
> > > +	flags = bus->get_flags(addr);
> > > +	if (!(flags & IORESOURCE_IO))
> > > +		return 0;
> > > +
> > > +	/* there is ranges property, apply the normal translation
> > directly. */
> > 
> > s/there is ranges/if we have a 'ranges'/
> 
> Thanks for spotting this
> 
> > 
> > > +	if (of_get_property(parent, "ranges", &rlen))
> > > +		return 0;
> > > +
> > > +	*presult = of_read_number(addr + 1, na - 1);
> > > +	/* this fixup is only valid for specific I/O range. */
> > > +	return addr_is_indirect_io(*presult);
> > > +}
> > > +
> > >  static int of_translate_one(struct device_node *parent, struct
> > of_bus *bus,
> > >  			    struct of_bus *pbus, __be32 *addr,
> > >  			    int na, int ns, int pna, const char *rprop)
> > > @@ -595,6 +639,15 @@ static u64 __of_translate_address(struct
> > device_node *dev,
> > >  			result = of_read_number(addr, na);
> > >  			break;
> > >  		}
> > > +		/*
> > > +		 * For indirectIO device which has no ranges property, get
> > > +		 * the address from reg directly.
> > > +		 */
> > > +		if (of_get_isa_indirect_io(dev, bus, addr, na, &result)) {
> > > +			pr_debug("isa indirectIO matched(%s)..addr =
> > 0x%llx\n",
> > > +				of_node_full_name(dev), result);
> > > +			break;
> > > +		}
> > >
> > >  		/* Get new parent bus and counts */
> > >  		pbus = of_match_bus(parent);
> > > @@ -688,8 +741,9 @@ static int __of_address_to_resource(struct
> > device_node *dev,
> > >  	if (taddr == OF_BAD_ADDR)
> > >  		return -EINVAL;
> > >  	memset(r, 0, sizeof(struct resource));
> > > -	if (flags & IORESOURCE_IO) {
> > > +	if (flags & IORESOURCE_IO && taddr >= PCIBIOS_MIN_IO) {
> > >  		unsigned long port;
> > > +
> > >  		port = pci_address_to_pio(taddr);
> > >  		if (port == (unsigned long)-1)
> > >  			return -EINVAL;
> > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > > index ba34907..1a08511 100644
> > > --- a/drivers/pci/pci.c
> > > +++ b/drivers/pci/pci.c
> > > @@ -3263,7 +3263,7 @@ int __weak pci_register_io_range(phys_addr_t
> > addr, resource_size_t size)
> > >
> > >  #ifdef PCI_IOBASE
> > >  	struct io_range *range;
> > > -	resource_size_t allocated_size = 0;
> > > +	resource_size_t allocated_size = PCIBIOS_MIN_IO;
> > >
> > >  	/* check if the range hasn't been previously recorded */
> > >  	spin_lock(&io_range_lock);
> > > @@ -3312,7 +3312,7 @@ phys_addr_t pci_pio_to_address(unsigned long
> > pio)
> > >
> > >  #ifdef PCI_IOBASE
> > >  	struct io_range *range;
> > > -	resource_size_t allocated_size = 0;
> > > +	resource_size_t allocated_size = PCIBIOS_MIN_IO;
> > 
> > Have you checked that pci_pio_to_address still returns valid values
> > after this? I know that
> > you are trying to take into account PCIBIOS_MIN_IO limit when
> > allocating reserving the IO ranges,
> > but the values added in the io_range_list are still starting from zero,
> > no from PCIBIOS_MIN_IO,
> 
> I think you're wrong here as in pci_address_to_pio we have:
> +	resource_size_t offset = PCIBIOS_MIN_IO;
> 
> This should be enough to guarantee that the PIOs start at
> PCIBIOS_MIN_IO...right?

I don't think you can guarantee that the pio value that gets passed into
pci_pio_to_address() always comes from a previously returned value by
pci_address_to_pio(). Maybe you can add a check in pci_pio_to_address()

	if (pio < PCIBIOS_MIN_IO)
		return address;

to avoid adding more checks in the list_for_each_entry() loop.

Best regards,
Liviu

> 
> 
> > so the calculation of the address in this function could return
> > negative values casted to pci_addr_t.
> > 
> > Maybe you want to adjust the range->start value in
> > pci_register_io_range() as well to have it
> > offset by PCIBIOS_MIN_IO as well.
> > 
> > Best regards,
> > Liviu
> > 
> > >
> > >  	if (pio > IO_SPACE_LIMIT)
> > >  		return address;
> > > @@ -3335,7 +3335,7 @@ unsigned long __weak
> > pci_address_to_pio(phys_addr_t address)
> > >  {
> > >  #ifdef PCI_IOBASE
> > >  	struct io_range *res;
> > > -	resource_size_t offset = 0;
> > > +	resource_size_t offset = PCIBIOS_MIN_IO;
> > >  	unsigned long addr = -1;
> > >
> > >  	spin_lock(&io_range_lock);
> > > diff --git a/include/linux/of_address.h b/include/linux/of_address.h
> > > index 3786473..deec469 100644
> > > --- a/include/linux/of_address.h
> > > +++ b/include/linux/of_address.h
> > > @@ -24,6 +24,23 @@ struct of_pci_range {
> > >  #define for_each_of_pci_range(parser, range) \
> > >  	for (; of_pci_range_parser_one(parser, range);)
> > >
> > > +
> > > +#ifndef indirect_io_enabled
> > > +#define indirect_io_enabled indirect_io_enabled
> > > +static inline bool indirect_io_enabled(void)
> > > +{
> > > +	return false;
> > > +}
> > > +#endif
> > > +
> > > +#ifndef addr_is_indirect_io
> > > +#define addr_is_indirect_io addr_is_indirect_io
> > > +static inline int addr_is_indirect_io(u64 taddr)
> > > +{
> > > +	return 0;
> > > +}
> > > +#endif
> > > +
> > >  /* Translate a DMA address from device space to CPU space */
> > >  extern u64 of_translate_dma_address(struct device_node *dev,
> > >  				    const __be32 *in_addr);
> > > diff --git a/include/linux/pci.h b/include/linux/pci.h
> > > index 0e49f70..7f6bbb6 100644
> > > --- a/include/linux/pci.h
> > > +++ b/include/linux/pci.h
> > > @@ -2130,4 +2130,12 @@ static inline bool pci_ari_enabled(struct
> > pci_bus *bus)
> > >  /* provide the legacy pci_dma_* API */
> > >  #include <linux/pci-dma-compat.h>
> > >
> > > +/*
> > > + * define this macro here to refrain from compilation error for some
> > > + * platforms. Please keep this macro at the end of this header file.
> > > + */
> > > +#ifndef PCIBIOS_MIN_IO
> > > +#define PCIBIOS_MIN_IO		0
> > > +#endif
> > > +
> > >  #endif /* LINUX_PCI_H */
> > > --
> > > 1.9.1
> > >
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-pci"
> > in
> > > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯
--
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] ata: xgene: Enable NCQ support for APM X-Gene SATA controller hardware v1.1
From: Tejun Heo @ 2016-11-09 16:45 UTC (permalink / raw)
  To: Rameshwar Sahu
  Cc: Olof Johansson, linux-scsi, linux-ide, Devicetree List, linux-arm,
	ddutile, Jon Masters, patches
In-Reply-To: <CAFd313x-mXpy0N3aKx2rfoag1FNOrN-LzKb5UHkY2L+GGc0B-A@mail.gmail.com>

Hello,

On Wed, Sep 14, 2016 at 04:15:00PM +0530, Rameshwar Sahu wrote:
> > @@ -821,8 +823,6 @@ static int xgene_ahci_probe(struct platform_device
> > *pdev)
> >                                 dev_warn(&pdev->dev, "%s: Error reading
> > device info. Assume version1\n",
> >                                         __func__);
> >                                 version = XGENE_AHCI_V1;
> > -                       } else if (info->valid & ACPI_VALID_CID) {
> > -                               version = XGENE_AHCI_V2;

Can you please explain this part a bit?  Everything else looks good to
me.

Thanks.

-- 
tejun

^ permalink raw reply

* Re: [PATCH 2/2] ASoC: axentia: tse850: add ASoC driver for the Axentia TSE-850
From: Peter Rosin @ 2016-11-09 16:27 UTC (permalink / raw)
  To: Mark Brown
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Liam Girdwood, Rob Herring,
	Mark Rutland, Jaroslav Kysela, Takashi Iwai,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Nicolas Ferre
In-Reply-To: <20161109133852.nvlyiqow2xpbc6ga-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>

On 2016-11-09 14:38, Mark Brown wrote:
> On Tue, Nov 08, 2016 at 05:20:57PM +0100, Peter Rosin wrote:
> 
>> +++ b/sound/soc/axentia/Kconfig
>> @@ -0,0 +1,10 @@
>> +config SND_SOC_AXENTIA_TSE850_PCM5142
>> +	tristate "ASoC driver for the Axentia TSE-850"
>> +	depends on ARCH_AT91 && OF
>> +	select ATMEL_SSC
>> +	select SND_ATMEL_SOC
>> +	select SND_ATMEL_SOC_SSC_DMA
>> +	select SND_SOC_PCM512x_I2C
>> +	help
>> +	  Say Y if you want to add support for the ASoC driver for the
>> +	  Axentia TSE-850 with a PCM5142 codec.
> 
> This just looks like a normal machine driver for an Atmel system which
> would usually go in the atemel directory - why is a new directory being
> created?

I thought atmel in this context meant that Atmel made the board, not that
the board was based on an Atmel cpu.

I'll move it for v2.

>> +static int tse850_get_mux2(struct snd_kcontrol *kctrl,
>> +			   struct snd_ctl_elem_value *ucontrol)
>> +{
>> +	struct snd_soc_dapm_context *dapm = snd_soc_dapm_kcontrol_dapm(kctrl);
>> +	struct snd_soc_card *card = dapm->card;
>> +	struct tse850_priv *tse850 = snd_soc_card_get_drvdata(card);
>> +	int ret;
>> +
>> +	ret = gpiod_get_value(tse850->loop2);
>> +	if (ret < 0)
>> +		return ret;
> 
> We can't reliably read the value of output GPIOs (though in practice the
> majority do support it) so it'd be better practice to use a state
> variable to remember what we set.  I'd also expect this to use the
> _cansleep() GPIO calls as it's not in a context where sleeping would be
> a problem.

Ok, I'll add _cansleep and cached values for v2.

>> +int tse850_get_ana(struct snd_kcontrol *kctrl,
>> +		   struct snd_ctl_elem_value *ucontrol)
>> +{
>> +	struct snd_soc_dapm_context *dapm = snd_soc_dapm_kcontrol_dapm(kctrl);
>> +	struct snd_soc_card *card = dapm->card;
>> +	struct tse850_priv *tse850 = snd_soc_card_get_drvdata(card);
>> +	int ret;
>> +
>> +	ret = regulator_get_voltage(tse850->ana);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	if (ret < 11000000)
>> +		ret = 11000000;
>> +	else if (ret > 20000000)
>> +		ret = 20000000;
> 
> This needs some comments...

Ok, I'll add some words...

>> +	struct snd_soc_pcm_runtime *rtd = substream->private_data;
>> +	struct device *dev = rtd->dev;
>> +	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
>> +	int dir = substream->stream != SNDRV_PCM_STREAM_PLAYBACK;
>> +	int div_id = dir ? ATMEL_SSC_RCMR_PERIOD : ATMEL_SSC_TCMR_PERIOD;
>> +	int period = snd_soc_params_to_frame_size(params) / 2 - 1;
> 
> Please write the logic out as normal if statements for legibility.  It's
> a bit concerning that we even need this function, it looks like pretty
> basic stuff that I'd expect the CPU DAI to just be doing - why can't
> this be the default behaviour of the CPU DAI?

I don't know and obviously don't have all the relevant HW to test
changes. Do you want me to attempt such a change anyway?
Adding Cc: Nicolas Ferre

>> +static int tse850_init(struct snd_soc_pcm_runtime *rtd)
>> +{
>> +	struct snd_soc_dapm_context *dapm = &rtd->card->dapm;
>> +
>> +	return snd_soc_dapm_add_routes(dapm, tse850_intercon,
>> +				       ARRAY_SIZE(tse850_intercon));
> 
> Set this up in the card data structure rather than open coding the call,
> you can register DAPM routes there too.

Right.

Thanks for looking!

Cheers,
Peter

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* RE: [PATCH V5 2/3] ARM64 LPC: Add missing range exception for special ISA
From: Gabriele Paoloni @ 2016-11-09 16:16 UTC (permalink / raw)
  To: liviu.dudau@arm.com, Yuanzhichang
  Cc: catalin.marinas@arm.com, will.deacon@arm.com, robh+dt@kernel.org,
	bhelgaas@google.com, mark.rutland@arm.com, olof@lixom.net,
	arnd@arndb.de, linux-arm-kernel@lists.infradead.org,
	lorenzo.pieralisi@arm.com, linux-kernel@vger.kernel.org, Linuxarm,
	devicetree@vger.kernel.org, linux-pci@vger.kernel.org,
	linux-serial@vger.kernel.org, minyard@acm.org, benh
In-Reply-To: <20161109113955.GD10219@e106497-lin.cambridge.arm.com>

Hi Liviu

Thanks for reviewing

> -----Original Message-----
> From: liviu.dudau@arm.com [mailto:liviu.dudau@arm.com]
> Sent: 09 November 2016 11:40
> To: Yuanzhichang
> Cc: catalin.marinas@arm.com; will.deacon@arm.com; robh+dt@kernel.org;
> bhelgaas@google.com; mark.rutland@arm.com; olof@lixom.net;
> arnd@arndb.de; linux-arm-kernel@lists.infradead.org;
> lorenzo.pieralisi@arm.com; linux-kernel@vger.kernel.org; Linuxarm;
> devicetree@vger.kernel.org; linux-pci@vger.kernel.org; linux-
> serial@vger.kernel.org; minyard@acm.org; benh@kernel.crashing.org;
> zourongrong@gmail.com; John Garry; Gabriele Paoloni;
> zhichang.yuan02@gmail.com; kantyzc@163.com; xuwei (O)
> Subject: Re: [PATCH V5 2/3] ARM64 LPC: Add missing range exception for
> special ISA
> 
> On Tue, Nov 08, 2016 at 11:47:08AM +0800, zhichang.yuan wrote:
> > This patch solves two issues:
> > 1) parse and get the right I/O range from DTS node whose parent does
> not
> > define the corresponding ranges property;
> >
> > There are some special ISA/LPC devices that work on a specific I/O
> range where
> > it is not correct to specify a ranges property in DTS parent node as
> cpu
> > addresses translated from DTS node are only for memory space on some
> > architectures, such as Arm64. Without the parent 'ranges' property,
> current
> > of_translate_address() return an error.
> > Here we add a fixup function, of_get_isa_indirect_io(). During the OF
> address
> > translation, this fixup will be called to check the 'reg' address to
> be
> > translating is for those sepcial ISA/LPC devices and get the I/O
> range
> > directly from the 'reg' property.
> >
> > 2) eliminate the I/O range conflict risk with PCI/PCIE leagecy I/O
> device;
> >
> > The current __of_address_to_resource() always translates the I/O
> range to PIO.
> > But this processing is not suitable for our ISA/LPC devices whose I/O
> range is
> > not cpu address(Arnd had stressed this in his comments on V2,V3
> patch-set).
> > Here, we bypass the mapping between cpu address and PIO for the
> special
> > ISA/LPC devices. But to drive these ISA/LPC devices, a I/O port
> address below
> > PCIBIOS_MIN_IO is needed by in*/out*(). Which means there is conflict
> risk
> > between I/O range of [0, PCIBIOS_MIN_IO) and PCI/PCIE legacy I/O
> range of [0,
> > IO_SPACE_LIMIT).
> > To avoid the I/O conflict, this patch reserve the I/O range below
> > PCIBIOS_MIN_IO.
> >
> > Signed-off-by: zhichang.yuan <yuanzhichang@hisilicon.com>
> > Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
> > ---
> >  .../arm/hisilicon/hisilicon-low-pin-count.txt      | 31 ++++++++++++
> >  arch/arm64/include/asm/io.h                        |  6 +++
> >  arch/arm64/kernel/extio.c                          | 25 ++++++++++
> >  drivers/of/address.c                               | 56
> +++++++++++++++++++++-
> >  drivers/pci/pci.c                                  |  6 +--
> >  include/linux/of_address.h                         | 17 +++++++
> >  include/linux/pci.h                                |  8 ++++
> >  7 files changed, 145 insertions(+), 4 deletions(-)
> >  create mode 100644
> Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low-pin-
> count.txt
> >
> > diff --git
> a/Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low-pin-
> count.txt b/Documentation/devicetree/bindings/arm/hisilicon/hisilicon-
> low-pin-count.txt
> > new file mode 100644
> > index 0000000..13c8ddd
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low-
> pin-count.txt
> > @@ -0,0 +1,31 @@
> > +Hisilicon Hip06 low-pin-count device
> > +  Usually LPC controller is part of PCI host bridge, so the legacy
> ISA ports
> > +  locate on LPC bus can be accessed direclty. But some SoCs have
> independent
> > +  LPC controller, and access the legacy ports by triggering LPC I/O
> cycles.
> > +  Hisilicon Hip06 implements this LPC device.
> > +
> > +Required properties:
> > +- compatible: should be "hisilicon,low-pin-count"
> > +- #address-cells: must be 2 which stick to the ISA/EISA binding doc.
> > +- #size-cells: must be 1 which stick to the ISA/EISA binding doc.
> > +- reg: base memory range where the register set of this device is
> mapped.
> > +
> > +Note:
> > +  The node name before '@' must be "isa" to represent the binding
> stick to the
> > +  ISA/EISA binding specification.
> > +
> > +Example:
> > +
> > +isa@a01b0000 {
> > +	compatible = "hisilicom,low-pin-count";
> > +	#address-cells = <2>;
> > +	#size-cells = <1>;
> > +	reg = <0x0 0xa01b0000 0x0 0x1000>;
> > +
> > +	ipmi0: bt@e4 {
> > +		compatible = "ipmi-bt";
> > +		device_type = "ipmi";
> > +		reg = <0x01 0xe4 0x04>;
> > +		status = "disabled";
> > +	};
> > +};
> 
> 
> This documentation file needs to be part of the next patch. It has
> nothing to do with
> what you are trying to fix here.

Yes you're right...we'll move it to next one

> 
> 
> > diff --git a/arch/arm64/include/asm/io.h
> b/arch/arm64/include/asm/io.h
> > index 136735d..c26b7cc 100644
> > --- a/arch/arm64/include/asm/io.h
> > +++ b/arch/arm64/include/asm/io.h
> > @@ -175,6 +175,12 @@ static inline u64 __raw_readq(const volatile
> void __iomem *addr)
> >  #define outsl outsl
> >
> >  DECLARE_EXTIO(l, u32)
> > +
> > +#define indirect_io_enabled indirect_io_enabled
> > +extern bool indirect_io_enabled(void);
> > +
> > +#define addr_is_indirect_io addr_is_indirect_io
> > +extern int addr_is_indirect_io(u64 taddr);
> >  #endif
> >
> >
> > diff --git a/arch/arm64/kernel/extio.c b/arch/arm64/kernel/extio.c
> > index 647b3fa..3d45fa8 100644
> > --- a/arch/arm64/kernel/extio.c
> > +++ b/arch/arm64/kernel/extio.c
> > @@ -19,6 +19,31 @@
> >
> >  struct extio_ops *arm64_extio_ops;
> >
> > +/**
> > + * indirect_io_enabled - check whether indirectIO is enabled.
> > + *	arm64_extio_ops will be set only when indirectIO mechanism had
> been
> > + *	initialized.
> > + *
> > + * Returns true when indirectIO is enabled.
> > + */
> > +bool indirect_io_enabled(void)
> > +{
> > +	return arm64_extio_ops ? true : false;
> > +}
> > +
> > +/**
> > + * addr_is_indirect_io - check whether the input taddr is for
> indirectIO.
> > + * @taddr: the io address to be checked.
> > + *
> > + * Returns 1 when taddr is in the range; otherwise return 0.
> > + */
> > +int addr_is_indirect_io(u64 taddr)
> > +{
> > +	if (arm64_extio_ops->start > taddr || arm64_extio_ops->end <
> taddr)
> 
> start >= taddr ?

Nope... if  (taddr < arm64_extio_ops->start || taddr > arm64_extio_ops->end)
then taddr is outside the range [start; end] and will return 0; otherwise
it will return 1...

> 
> > +		return 0;
> > +
> > +	return 1;
> > +}
> >
> >  BUILD_EXTIO(b, u8)
> >
> > diff --git a/drivers/of/address.c b/drivers/of/address.c
> > index 02b2903..cc2a05d 100644
> > --- a/drivers/of/address.c
> > +++ b/drivers/of/address.c
> > @@ -479,6 +479,50 @@ static int of_empty_ranges_quirk(struct
> device_node *np)
> >  	return false;
> >  }
> >
> > +
> > +/*
> > + * of_isa_indirect_io - get the IO address from some isa reg
> property value.
> > + *	For some isa/lpc devices, no ranges property in ancestor node.
> > + *	The device addresses are described directly in their regs
> property.
> > + *	This fixup function will be called to get the IO address of
> isa/lpc
> > + *	devices when the normal of_translation failed.
> > + *
> > + * @parent:	points to the parent dts node;
> > + * @bus:		points to the of_bus which can be used to parse
> address;
> > + * @addr:	the address from reg property;
> > + * @na:		the address cell counter of @addr;
> > + * @presult:	store the address paresed from @addr;
> > + *
> > + * return 1 when successfully get the I/O address;
> > + * 0 will return for some failures.
> 
> Bah, you are returning a signed int, why 0 for failure? Return a
> negative value with
> error codes. Otherwise change the return value into a bool.

Yes we'll move to bool

> 
> > + */
> > +static int of_get_isa_indirect_io(struct device_node *parent,
> > +				struct of_bus *bus, __be32 *addr,
> > +				int na, u64 *presult)
> > +{
> > +	unsigned int flags;
> > +	unsigned int rlen;
> > +
> > +	/* whether support indirectIO */
> > +	if (!indirect_io_enabled())
> > +		return 0;
> > +
> > +	if (!of_bus_isa_match(parent))
> > +		return 0;
> > +
> > +	flags = bus->get_flags(addr);
> > +	if (!(flags & IORESOURCE_IO))
> > +		return 0;
> > +
> > +	/* there is ranges property, apply the normal translation
> directly. */
> 
> s/there is ranges/if we have a 'ranges'/

Thanks for spotting this

> 
> > +	if (of_get_property(parent, "ranges", &rlen))
> > +		return 0;
> > +
> > +	*presult = of_read_number(addr + 1, na - 1);
> > +	/* this fixup is only valid for specific I/O range. */
> > +	return addr_is_indirect_io(*presult);
> > +}
> > +
> >  static int of_translate_one(struct device_node *parent, struct
> of_bus *bus,
> >  			    struct of_bus *pbus, __be32 *addr,
> >  			    int na, int ns, int pna, const char *rprop)
> > @@ -595,6 +639,15 @@ static u64 __of_translate_address(struct
> device_node *dev,
> >  			result = of_read_number(addr, na);
> >  			break;
> >  		}
> > +		/*
> > +		 * For indirectIO device which has no ranges property, get
> > +		 * the address from reg directly.
> > +		 */
> > +		if (of_get_isa_indirect_io(dev, bus, addr, na, &result)) {
> > +			pr_debug("isa indirectIO matched(%s)..addr =
> 0x%llx\n",
> > +				of_node_full_name(dev), result);
> > +			break;
> > +		}
> >
> >  		/* Get new parent bus and counts */
> >  		pbus = of_match_bus(parent);
> > @@ -688,8 +741,9 @@ static int __of_address_to_resource(struct
> device_node *dev,
> >  	if (taddr == OF_BAD_ADDR)
> >  		return -EINVAL;
> >  	memset(r, 0, sizeof(struct resource));
> > -	if (flags & IORESOURCE_IO) {
> > +	if (flags & IORESOURCE_IO && taddr >= PCIBIOS_MIN_IO) {
> >  		unsigned long port;
> > +
> >  		port = pci_address_to_pio(taddr);
> >  		if (port == (unsigned long)-1)
> >  			return -EINVAL;
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index ba34907..1a08511 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -3263,7 +3263,7 @@ int __weak pci_register_io_range(phys_addr_t
> addr, resource_size_t size)
> >
> >  #ifdef PCI_IOBASE
> >  	struct io_range *range;
> > -	resource_size_t allocated_size = 0;
> > +	resource_size_t allocated_size = PCIBIOS_MIN_IO;
> >
> >  	/* check if the range hasn't been previously recorded */
> >  	spin_lock(&io_range_lock);
> > @@ -3312,7 +3312,7 @@ phys_addr_t pci_pio_to_address(unsigned long
> pio)
> >
> >  #ifdef PCI_IOBASE
> >  	struct io_range *range;
> > -	resource_size_t allocated_size = 0;
> > +	resource_size_t allocated_size = PCIBIOS_MIN_IO;
> 
> Have you checked that pci_pio_to_address still returns valid values
> after this? I know that
> you are trying to take into account PCIBIOS_MIN_IO limit when
> allocating reserving the IO ranges,
> but the values added in the io_range_list are still starting from zero,
> no from PCIBIOS_MIN_IO,

I think you're wrong here as in pci_address_to_pio we have:
+	resource_size_t offset = PCIBIOS_MIN_IO;

This should be enough to guarantee that the PIOs start at
PCIBIOS_MIN_IO...right?


> so the calculation of the address in this function could return
> negative values casted to pci_addr_t.
> 
> Maybe you want to adjust the range->start value in
> pci_register_io_range() as well to have it
> offset by PCIBIOS_MIN_IO as well.
> 
> Best regards,
> Liviu
> 
> >
> >  	if (pio > IO_SPACE_LIMIT)
> >  		return address;
> > @@ -3335,7 +3335,7 @@ unsigned long __weak
> pci_address_to_pio(phys_addr_t address)
> >  {
> >  #ifdef PCI_IOBASE
> >  	struct io_range *res;
> > -	resource_size_t offset = 0;
> > +	resource_size_t offset = PCIBIOS_MIN_IO;
> >  	unsigned long addr = -1;
> >
> >  	spin_lock(&io_range_lock);
> > diff --git a/include/linux/of_address.h b/include/linux/of_address.h
> > index 3786473..deec469 100644
> > --- a/include/linux/of_address.h
> > +++ b/include/linux/of_address.h
> > @@ -24,6 +24,23 @@ struct of_pci_range {
> >  #define for_each_of_pci_range(parser, range) \
> >  	for (; of_pci_range_parser_one(parser, range);)
> >
> > +
> > +#ifndef indirect_io_enabled
> > +#define indirect_io_enabled indirect_io_enabled
> > +static inline bool indirect_io_enabled(void)
> > +{
> > +	return false;
> > +}
> > +#endif
> > +
> > +#ifndef addr_is_indirect_io
> > +#define addr_is_indirect_io addr_is_indirect_io
> > +static inline int addr_is_indirect_io(u64 taddr)
> > +{
> > +	return 0;
> > +}
> > +#endif
> > +
> >  /* Translate a DMA address from device space to CPU space */
> >  extern u64 of_translate_dma_address(struct device_node *dev,
> >  				    const __be32 *in_addr);
> > diff --git a/include/linux/pci.h b/include/linux/pci.h
> > index 0e49f70..7f6bbb6 100644
> > --- a/include/linux/pci.h
> > +++ b/include/linux/pci.h
> > @@ -2130,4 +2130,12 @@ static inline bool pci_ari_enabled(struct
> pci_bus *bus)
> >  /* provide the legacy pci_dma_* API */
> >  #include <linux/pci-dma-compat.h>
> >
> > +/*
> > + * define this macro here to refrain from compilation error for some
> > + * platforms. Please keep this macro at the end of this header file.
> > + */
> > +#ifndef PCIBIOS_MIN_IO
> > +#define PCIBIOS_MIN_IO		0
> > +#endif
> > +
> >  #endif /* LINUX_PCI_H */
> > --
> > 1.9.1
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-pci"
> in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> --
> ====================
> | I would like to |
> | fix the world,  |
> | but they're not |
> | giving me the   |
>  \ source code!  /
>   ---------------
>     ¯\_(ツ)_/¯

^ permalink raw reply

* Re: [PATCH 2/4] mfd: palmas: Reset the POWERHOLD mux during power off
From: Lee Jones @ 2016-11-09 16:14 UTC (permalink / raw)
  To: Keerthy
  Cc: tony-4v6yS6AI5VpBDgjK7y7TUQ, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA, nm-l0cyMroinI0,
	t-kristo-l0cyMroinI0
In-Reply-To: <1477559414-12520-3-git-send-email-j-keerthy-l0cyMroinI0@public.gmane.org>

On Thu, 27 Oct 2016, Keerthy wrote:

> POWERHOLD signal has higher priority  over the DEV_ON bit.
> So power off will not happen if the POWERHOLD is held high.
> Hence reset the MUX to GPIO_7 mode to release the POWERHOLD
> and the DEV_ON bit to take effect to power off the PMIC.
> 
> PMIC Power off happens in dire situations like thermal shutdown
> so irrespective of the POWERHOLD setting go ahead and turn off
> the powerhold.  Currently poweroff is broken on boards that have
> powerhold enabled. This fixes poweroff on those boards.
> 
> Signed-off-by: Keerthy <j-keerthy-l0cyMroinI0@public.gmane.org>
> ---
>  drivers/mfd/palmas.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/mfd/palmas.c b/drivers/mfd/palmas.c
> index 8f8bacb..8fbc5e0 100644
> --- a/drivers/mfd/palmas.c
> +++ b/drivers/mfd/palmas.c
> @@ -430,10 +430,28 @@ static void palmas_power_off(void)
>  {
>  	unsigned int addr;
>  	int ret, slave;
> +	struct device_node *node;
> +	bool override_powerhold;
>  
>  	if (!palmas_dev)

Can this happen?

>  		return;
>  
> +	node = palmas_dev->dev->of_node;

Just do:

struct device_node *np = palmas_dev->dev->of_node;

> +	override_powerhold = of_property_read_bool(node,
> +					"ti,palmas-override-powerhold");

Break the line after the '=' instead.

> +	if (override_powerhold) {

if (of_property_read_bool(node,	"ti,palmas-override-powerhold"))

Then remove 'override_powerhold'.

> +		addr = PALMAS_BASE_TO_REG(PALMAS_PU_PD_OD_BASE,
> +					  PALMAS_PRIMARY_SECONDARY_PAD2);
> +		slave = PALMAS_BASE_TO_SLAVE(PALMAS_PU_PD_OD_BASE);
> +
> +		ret = regmap_update_bits(palmas_dev->regmap[slave], addr,
> +					 PALMAS_PRIMARY_SECONDARY_PAD2_GPIO_7_MASK, 0);
> +		if (ret)
> +			pr_err("%s: Unable to write PALMAS_PRIMARY_SECONDARY_PAD2 %d\n",
> +			       __func__, ret);

Don't use __func__ in live code.

And use dev_err();


> +	}
> +
>  	slave = PALMAS_BASE_TO_SLAVE(PALMAS_PMU_CONTROL_BASE);
>  	addr = PALMAS_BASE_TO_REG(PALMAS_PMU_CONTROL_BASE, PALMAS_DEV_CTRL);
>  

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 1/4] Documentation: pinctrl: palmas: Add ti,palmas-powerhold-override property definition
From: Lee Jones @ 2016-11-09 16:10 UTC (permalink / raw)
  To: Keerthy
  Cc: tony, robh+dt, linux-omap, linux-kernel, devicetree, linux-gpio,
	nm, t-kristo
In-Reply-To: <1477559414-12520-2-git-send-email-j-keerthy@ti.com>

On Thu, 27 Oct 2016, Keerthy wrote:

> GPIO7 is configured in POWERHOLD mode which has higher priority
> over DEV_ON bit and keeps the PMIC supplies on even after the DEV_ON
> bit is turned off. This property enables driver to over ride the
> POWERHOLD value to GPIO7 so as to turn off the PMIC in power off
> scenarios.
> 
> Signed-off-by: Keerthy <j-keerthy@ti.com>
> ---
>  Documentation/devicetree/bindings/pinctrl/pinctrl-palmas.txt | 9 +++++++++
>  1 file changed, 9 insertions(+)

This requires a DT Ack.

> diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-palmas.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-palmas.txt
> index caf297b..c28d4eb8 100644
> --- a/Documentation/devicetree/bindings/pinctrl/pinctrl-palmas.txt
> +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-palmas.txt
> @@ -35,6 +35,15 @@ Optional properties:
>  - ti,palmas-enable-dvfs2: Enable DVFS2. Configure pins for DVFS2 mode.
>  	Selection primary or secondary function associated to GPADC_START
>  	and SYSEN2 pin/pad for DVFS2 interface
> +- ti,palmas-override-powerhold: This is applicable for PMICs for which
> +	GPIO7 is configured in POWERHOLD mode which has higher priority
> +	over DEV_ON bit and keeps the PMIC supplies on even after the DEV_ON
> +	bit is turned off. This property enables driver to over ride the
> +	POWERHOLD value to GPIO7 so as to turn off the PMIC in power off
> +	scenarios. So for GPIO7 if ti,palmas-override-powerhold is set
> +	then the GPIO_7 field should never be muxed to anything else.
> +	It should be set to POWERHOLD by default and only in case of
> +	power off scenarios the driver will over ride the mux value.
>  
>  This binding uses the following generic properties as defined in
>  pinctrl-bindings.txt:

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* Re: [PATCH 1/3] ipmi/bt-bmc: change compatible node to 'aspeed, ast2400-ibt-bmc'
From: Arnd Bergmann @ 2016-11-09 16:09 UTC (permalink / raw)
  To: minyard
  Cc: devicetree, arm, Benjamin Herrenschmidt, Rob Herring, olof,
	openipmi-developer, linux-arm-kernel, Joel Stanley
In-Reply-To: <c749ff0a-bc71-bc9f-0e8c-326db2cea0fb@acm.org>

On Tuesday, November 8, 2016 12:15:57 PM CET Corey Minyard wrote:
> On 11/08/2016 09:52 AM, Cédric Le Goater wrote:
> > O
> snip
> 
> >>>> While we're modifying the binding, should we add a compat string for
> >>>> the ast2500?
> >>> Well, if the change in this patch is fine for all, may be we can add
> >>> the ast2500 compat string in a followup patch ?
> >> Sounds good to me.
> > OK. So, how do we proceed with this patch ? Who would include it in its
> > tree ?
> 
> I don't have anything for 4.9 at the moment.  Arnd, if you have 
> something, can
> you take this?  Otherwise I will.
> 
> And I guess I should add:
> 
> Acked-by: Corey Minyard <cminyard@mvista.com>

Thanks, I've added it to my todo folder.

Olof, if you do fixes before I do, please pick up this patch with
Corey's Ack.

	Arnd

------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi

^ permalink raw reply

* Re: [PATCH 3/3] ipmi/bt-bmc: add a sysfs file to configure a maximum response time
From: Corey Minyard @ 2016-11-09 16:04 UTC (permalink / raw)
  To: Cédric Le Goater,
	openipmi-developer-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Joel Stanley
  Cc: Rob Herring, Arnd Bergmann, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <aa4804b0-4084-6d34-c1db-68bdb5efb20a-Bxea+6Xhats@public.gmane.org>

On 11/09/2016 08:42 AM, Cédric Le Goater wrote:
> On 11/07/2016 07:37 PM, Corey Minyard wrote:
>> Sorry, I was at Plumbers and extra busy with other stuff.  Just getting around to reviewing this.
>>
>> On 11/02/2016 02:57 AM, Cédric Le Goater wrote:
>>> We could also use an ioctl for that purpose. sysfs seems a better
>>> approach.
>>>
>>> Signed-off-by: Cédric Le Goater <clg-Bxea+6Xhats@public.gmane.org>
>>> ---
>>>    drivers/char/ipmi/bt-bmc.c | 31 +++++++++++++++++++++++++++++++
>>>    1 file changed, 31 insertions(+)
>>>
>>> diff --git a/drivers/char/ipmi/bt-bmc.c b/drivers/char/ipmi/bt-bmc.c
>>> index e751e4a754b7..d7146f0e900e 100644
>>> --- a/drivers/char/ipmi/bt-bmc.c
>>> +++ b/drivers/char/ipmi/bt-bmc.c
>>> @@ -84,6 +84,33 @@ struct bt_bmc {
>>>      static atomic_t open_count = ATOMIC_INIT(0);
>>>    +static ssize_t bt_bmc_show_response_time(struct device *dev,
>>> +                     struct device_attribute *attr,
>>> +                     char *buf)
>>> +{
>>> +    struct bt_bmc *bt_bmc = dev_get_drvdata(dev);
>>> +
>>> +    return snprintf(buf, PAGE_SIZE - 1, "%d\n", bt_bmc->response_time);
>>> +}
>>> +
>>> +static ssize_t bt_bmc_set_response_time(struct device *dev,
>>> +                    struct device_attribute *attr,
>>> +                    const char *buf, size_t count)
>>> +{
>>> +    struct bt_bmc *bt_bmc = dev_get_drvdata(dev);
>>> +    unsigned long val;
>>> +    int err;
>>> +
>>> +    err = kstrtoul(buf, 0, &val);
>>> +    if (err)
>>> +        return err;
>>> +    bt_bmc->response_time = val;
>>> +    return count;
>>> +}
>>> +
>>> +static DEVICE_ATTR(response_time, 0644,
>>> +           bt_bmc_show_response_time, bt_bmc_set_response_time);
>>> +
>>>    static u8 bt_inb(struct bt_bmc *bt_bmc, int reg)
>>>    {
>>>        return ioread8(bt_bmc->base + reg);
>>> @@ -572,6 +599,10 @@ static int bt_bmc_probe(struct platform_device *pdev)
>>>        bt_bmc_config_irq(bt_bmc, pdev);
>>>          bt_bmc->response_time = BT_BMC_RESPONSE_TIME;
>>> +    rc = device_create_file(&pdev->dev, &dev_attr_response_time);
>>> +    if (rc)
>>> +        dev_warn(&pdev->dev, "can't create response_time file\n");
>>> +
>> You added an extra space here.
> yes. I will remove it in the next version.
>
> The patch raises a few questions on the "BT Interface Capabilities" :
>
>   - should we use sysfs or a specific ioctl to configure the driver ?

I should probably say sysfs, but really I don't care.  The hard part about
ioctls is the compat, and there shouldn't be any compat issues with this
interface.  An ioctl is probably easier, especially if you add an ioctl for
the header size thing I talked about in the previous email.

The only thing that matters to the driver is the timeout.  If you want to
make the timeout per fd, then it will have to be an ioctl.

>   - should the driver handle directly the response to the "Get BT
>     Interface Capabilities" command ?

That probably belongs in userspace.  The only reason I can think of
to have it in the driver would be so the host driver can fetch it if the
BMC userspace is down, but I can't see how that's a good idea.

Hope my wishy-washy answer helps :-).

-corey

> What is your opinion ?
>
> Thanks,
>
> C.
>
>>>          if (bt_bmc->irq) {
>>>            dev_info(dev, "Using IRQ %d\n", bt_bmc->irq);
>>

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 1/2] backlight: arcxcnn: add support for ArticSand devices
From: Lee Jones @ 2016-11-09 15:53 UTC (permalink / raw)
  To: Olimpiu Dejeu
  Cc: robh-DgEjT+Ai2ygdnm+yROfE0A, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, jg1.han-Sze3O3UU22JBDgjK7y7TUQ
In-Reply-To: <1477513778-31297-1-git-send-email-olimpiu-eV7fy4qpoLhpLGFMi4vTTA@public.gmane.org>

Jingoo?

On Wed, 26 Oct 2016, Olimpiu Dejeu wrote:

> Resubmition of arcxcnn backliught driver adding devicetree entries
>  for all registers
> 
> Signed-off-by: Olimpiu Dejeu <olimpiu-eV7fy4qpoLhpLGFMi4vTTA@public.gmane.org>
> 
> ---
>  drivers/video/backlight/Kconfig      |   7 +
>  drivers/video/backlight/Makefile     |   1 +
>  drivers/video/backlight/arcxcnn_bl.c | 541 +++++++++++++++++++++++++++++++++++
>  include/linux/i2c/arcxcnn.h          |  67 +++++
>  4 files changed, 616 insertions(+)
>  create mode 100644 drivers/video/backlight/arcxcnn_bl.c
>  create mode 100644 include/linux/i2c/arcxcnn.h
> 
> diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
> index 5ffa4b4..4e1d2ad 100644
> --- a/drivers/video/backlight/Kconfig
> +++ b/drivers/video/backlight/Kconfig
> @@ -460,6 +460,13 @@ config BACKLIGHT_BD6107
>  	help
>  	  If you have a Rohm BD6107 say Y to enable the backlight driver.
>  
> +config BACKLIGHT_ARCXCNN
> +	tristate "Backlight driver for the Arctic Sands ARCxCnnnn family"
> +	depends on I2C
> +	help
> +	  If you have an ARCxCnnnn family backlight say Y to enable
> +	  the backlight driver.
> +
>  endif # BACKLIGHT_CLASS_DEVICE
>  
>  endif # BACKLIGHT_LCD_SUPPORT
> diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
> index 16ec534..8905129 100644
> --- a/drivers/video/backlight/Makefile
> +++ b/drivers/video/backlight/Makefile
> @@ -55,3 +55,4 @@ obj-$(CONFIG_BACKLIGHT_SKY81452)	+= sky81452-backlight.o
>  obj-$(CONFIG_BACKLIGHT_TOSA)		+= tosa_bl.o
>  obj-$(CONFIG_BACKLIGHT_TPS65217)	+= tps65217_bl.o
>  obj-$(CONFIG_BACKLIGHT_WM831X)		+= wm831x_bl.o
> +obj-$(CONFIG_BACKLIGHT_ARCXCNN) 	+= arcxcnn_bl.o
> diff --git a/drivers/video/backlight/arcxcnn_bl.c b/drivers/video/backlight/arcxcnn_bl.c
> new file mode 100644
> index 0000000..1dad680
> --- /dev/null
> +++ b/drivers/video/backlight/arcxcnn_bl.c
> @@ -0,0 +1,541 @@
> +/*
> + * Backlight driver for ArcticSand ARC_X_C_0N_0N Devices
> + *
> + * Copyright 2016 ArcticSand, Inc.
> + *
> + * Licensed under the GPL-2 or later.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/i2c.h>
> +#include <linux/backlight.h>
> +#include <linux/err.h>
> +#include <linux/of.h>
> +#include <linux/pwm.h>
> +#include <linux/regulator/consumer.h>
> +
> +#include "linux/i2c/arcxcnn.h"
> +
> +#define ARCXCNN_CMD		(0x00)  /* Command Register */
> +#define ARCXCNN_CMD_STDBY	(0x80)	/* I2C Standby */
> +#define ARCXCNN_CMD_RESET	(0x40)	/* Reset */
> +#define ARCXCNN_CMD_BOOST	(0x10)	/* Boost */
> +#define ARCXCNN_CMD_OVP_MASK	(0x0C)	/* --- Over Voltage Threshold */
> +#define ARCXCNN_CMD_OVP_XXV	(0x0C)	/* <rsvrd> Over Voltage Threshold */
> +#define ARCXCNN_CMD_OVP_20V	(0x08)	/* 20v Over Voltage Threshold */
> +#define ARCXCNN_CMD_OVP_24V	(0x04)	/* 24v Over Voltage Threshold */
> +#define ARCXCNN_CMD_OVP_31V	(0x00)	/* 31.4v Over Voltage Threshold */
> +#define ARCXCNN_CMD_EXT_COMP	(0x01)	/* part (0) or full (1) external comp */
> +
> +#define ARCXCNN_CONFIG	(0x01)  /* Configuration */
> +#define ARCXCNN_STATUS1	(0x02)  /* Status 1 */
> +#define ARCXCNN_STATUS2	(0x03)  /* Status 2 */
> +#define ARCXCNN_FADECTRL	(0x04)  /* Fading Control */
> +#define ARCXCNN_ILED_CONFIG	(0x05)  /* ILED Configuration */
> +
> +#define ARCXCNN_LEDEN		(0x06)  /* LED Enable Register */
> +#define ARCXCNN_LEDEN_ISETEXT	(0x80)	/* Full-scale current set externally */
> +#define ARCXCNN_LEDEN_MASK	(0x3F)	/* LED string enables */
> +#define ARCXCNN_LEDEN_LED1	(0x01)
> +#define ARCXCNN_LEDEN_LED2	(0x02)
> +#define ARCXCNN_LEDEN_LED3	(0x04)
> +#define ARCXCNN_LEDEN_LED4	(0x08)
> +#define ARCXCNN_LEDEN_LED5	(0x10)
> +#define ARCXCNN_LEDEN_LED6	(0x20)
> +
> +#define ARCXCNN_WLED_ISET_LSB	(0x07)  /* LED ISET LSB (in upper nibble) */
> +#define ARCXCNN_WLED_ISET_MSB	(0x08)  /* LED ISET MSB (8 bits) */
> +
> +#define ARCXCNN_DIMFREQ		(0x09)
> +#define ARCXCNN_COMP_CONFIG	(0x0A)
> +#define ARCXCNN_FILT_CONFIG	(0x0B)
> +#define ARCXCNN_IMAXTUNE	(0x0C)
> +
> +#define DEFAULT_BL_NAME		"arctic_bl"
> +#define MAX_BRIGHTNESS		4095
> +
> +static int s_no_reset_on_remove;
> +module_param_named(noreset, s_no_reset_on_remove, int, 0644);
> +MODULE_PARM_DESC(noreset, "No reset on module removal");
> +
> +static int s_ibright = 60;
> +module_param_named(ibright, s_ibright, int, 0644);
> +MODULE_PARM_DESC(ibright, "Initial brightness (when no plat data)");
> +
> +static int s_iledstr = 0x3F;
> +module_param_named(iledstr, s_iledstr, int, 0644);
> +MODULE_PARM_DESC(iledstr, "Initial LED String (when no plat data)");
> +
> +static int s_retries = 2; /* 1 == only one try */
> +module_param_named(retries, s_retries, int, 0644);
> +MODULE_PARM_DESC(retries, "I2C retries attempted");
> +
> +enum arcxcnn_brightness_ctrl_mode {
> +	PWM_BASED = 1,
> +	REGISTER_BASED,
> +};
> +
> +struct arcxcnn;
> +
> +struct arcxcnn {
> +	char chipname[64];
> +	enum arcxcnn_chip_id chip_id;
> +	enum arcxcnn_brightness_ctrl_mode mode;
> +	struct i2c_client *client;
> +	struct backlight_device *bl;
> +	struct device *dev;
> +	struct arcxcnn_platform_data *pdata;
> +	struct pwm_device *pwm;
> +	struct regulator *supply;	/* regulator for VDD input */
> +};
> +
> +static int arcxcnn_write_byte(struct arcxcnn *lp, u8 reg, u8 data)
> +{
> +	s32 ret = -1;
> +	int att;
> +
> +	for (att = 0; att < s_retries; att++) {
> +		ret = i2c_smbus_write_byte_data(lp->client, reg, data);
> +		if (ret >= 0)
> +			return 0;
> +	}
> +	return ret;
> +}
> +
> +static u8 arcxcnn_read_byte(struct arcxcnn *lp, u8 reg)
> +{
> +	int val;
> +	int att;
> +
> +	for (att = 0; att < s_retries; att++) {
> +		val = i2c_smbus_read_byte_data(lp->client, reg);
> +		if (val >= 0)
> +			return (u8)val;
> +	}
> +	return 0;
> +}
> +
> +static int arcxcnn_update_bit(struct arcxcnn *lp, u8 reg, u8 mask, u8 data)
> +{
> +	int ret, att;
> +	u8 tmp;
> +
> +	for (att = 0, ret = -1; att < s_retries; att++) {
> +		ret = i2c_smbus_read_byte_data(lp->client, reg);
> +		if (ret >= 0)
> +			break;
> +	}
> +	if (ret < 0) {
> +		dev_err(lp->dev, "failed to read 0x%.2x\n", reg);
> +		return ret;
> +	}
> +
> +	tmp = (u8)ret;
> +	tmp &= ~mask;
> +	tmp |= data & mask;
> +
> +	return arcxcnn_write_byte(lp, reg, tmp);
> +}
> +
> +static int arcxcnn_set_brightness(struct arcxcnn *lp, u32 brightness)
> +{
> +	int ret;
> +	u8 val;
> +
> +	val = (brightness & 0xF) << 4;
> +	ret = arcxcnn_write_byte(lp, ARCXCNN_WLED_ISET_LSB, val);
> +	if (ret < 0)
> +		return ret;
> +	val = (brightness >> 4);
> +	ret = arcxcnn_write_byte(lp, ARCXCNN_WLED_ISET_MSB, val);
> +	return ret;
> +}
> +
> +static int arcxcnn_bl_update_status(struct backlight_device *bl)
> +{
> +	struct arcxcnn *lp = bl_get_data(bl);
> +	u32 brightness = bl->props.brightness;
> +
> +	if (bl->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK))
> +		brightness = 0;
> +
> +	/* set brightness */
> +	if (lp->mode == PWM_BASED)
> +		; /* via pwm */
> +	else if (lp->mode == REGISTER_BASED)
> +		arcxcnn_set_brightness(lp, brightness);
> +
> +	/* set power-on/off/save modes */
> +	if (bl->props.power == 0)
> +		/* take out of standby */
> +		arcxcnn_update_bit(lp, ARCXCNN_CMD, ARCXCNN_CMD_STDBY, 0);
> +	else
> +		/* 1-3 == power save, 4 = off
> +		 * place in low-power standby mode
> +		 */
> +		arcxcnn_update_bit(lp, ARCXCNN_CMD,
> +				ARCXCNN_CMD_STDBY, ARCXCNN_CMD_STDBY);
> +	return 0;
> +}
> +
> +static const struct backlight_ops arcxcnn_bl_ops = {
> +	.options = BL_CORE_SUSPENDRESUME,
> +	.update_status = arcxcnn_bl_update_status,
> +};
> +
> +static int arcxcnn_backlight_register(struct arcxcnn *lp)
> +{
> +	struct backlight_device *bl;
> +	struct backlight_properties props;
> +	struct arcxcnn_platform_data *pdata = lp->pdata;
> +	const char *name = pdata->name ? : DEFAULT_BL_NAME;
> +
> +	memset(&props, 0, sizeof(props));
> +	props.type = BACKLIGHT_PLATFORM;
> +	props.max_brightness = MAX_BRIGHTNESS;
> +
> +	if (pdata->initial_brightness > props.max_brightness)
> +		pdata->initial_brightness = props.max_brightness;
> +
> +	props.brightness = pdata->initial_brightness;
> +
> +	bl = devm_backlight_device_register(lp->dev, name, lp->dev, lp,
> +				       &arcxcnn_bl_ops, &props);
> +	if (IS_ERR(bl))
> +		return PTR_ERR(bl);
> +
> +	lp->bl = bl;
> +
> +	return 0;
> +}
> +
> +static ssize_t arcxcnn_get_chip_id(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	struct arcxcnn *lp = dev_get_drvdata(dev);
> +
> +	return scnprintf(buf, PAGE_SIZE, "%s\n", lp->chipname);
> +}
> +
> +static ssize_t arcxcnn_get_led_str(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	struct arcxcnn *lp = dev_get_drvdata(dev);
> +
> +	return scnprintf(buf, PAGE_SIZE, "%02X\n", lp->pdata->led_str);
> +}
> +
> +static ssize_t arcxcnn_set_led_str(struct device *dev,
> +		struct device_attribute *attr, const char *buf, size_t len)
> +{
> +	struct arcxcnn *lp = dev_get_drvdata(dev);
> +	unsigned long ledstr;
> +
> +	if (kstrtoul(buf, 0, &ledstr))
> +		return 0;
> +
> +	if (ledstr != lp->pdata->led_str) {
> +		/* don't allow 0 for ledstr, use power to turn all off */
> +		if (ledstr == 0)
> +			return 0;
> +		lp->pdata->led_str = ledstr & 0x3F;
> +		arcxcnn_update_bit(lp, ARCXCNN_LEDEN,
> +			ARCXCNN_LEDEN_MASK, lp->pdata->led_str);
> +	}
> +	return len;
> +}
> +
> +static ssize_t arcxcnn_get_bl_ctl_mode(struct device *dev,
> +	     struct device_attribute *attr, char *buf)
> +{
> +	struct arcxcnn *lp = dev_get_drvdata(dev);
> +	char *strmode = NULL;
> +
> +	if (lp->mode == PWM_BASED)
> +		strmode = "pwm based";
> +	else if (lp->mode == REGISTER_BASED)
> +		strmode = "register based";
> +
> +	return scnprintf(buf, PAGE_SIZE, "%s\n", strmode);
> +}
> +
> +static DEVICE_ATTR(chip_id, 0444, arcxcnn_get_chip_id, NULL);
> +static DEVICE_ATTR(led_str, 0664, arcxcnn_get_led_str, arcxcnn_set_led_str);
> +static DEVICE_ATTR(bl_ctl_mode, 0444, arcxcnn_get_bl_ctl_mode, NULL);
> +
> +static struct attribute *arcxcnn_attributes[] = {
> +	&dev_attr_chip_id.attr,
> +	&dev_attr_led_str.attr,
> +	&dev_attr_bl_ctl_mode.attr,
> +	NULL,
> +};
> +
> +static const struct attribute_group arcxcnn_attr_group = {
> +	.attrs = arcxcnn_attributes,
> +};
> +
> +#ifdef CONFIG_OF
> +static int arcxcnn_parse_dt(struct arcxcnn *lp)
> +{
> +	struct device *dev = lp->dev;
> +	struct device_node *node = dev->of_node;
> +	u32 prog_val, num_entry, sources[6];
> +	int ret;
> +
> +	if (!node) {
> +		dev_err(dev, "no platform data.\n");
> +		return -EINVAL;
> +	}
> +	lp->pdata->led_config_0_set = false;
> +	lp->pdata->led_config_1_set = false;
> +	lp->pdata->dim_freq_set = false;
> +	lp->pdata->comp_config_set = false;
> +	lp->pdata->filter_config_set = false;
> +	lp->pdata->trim_config_set = false;
> +
> +	ret = of_property_read_string(node, "label", &lp->pdata->name);
> +	if (ret < 0)
> +		lp->pdata->name = NULL;
> +
> +	ret = of_property_read_u32(node, "default-brightness", &prog_val);
> +	if (ret < 0)
> +		prog_val = s_ibright;
> +	lp->pdata->initial_brightness = prog_val;
> +	if (lp->pdata->initial_brightness > MAX_BRIGHTNESS)
> +		lp->pdata->initial_brightness = MAX_BRIGHTNESS;
> +
> +	ret = of_property_read_u32(node, "arcticsand,led-config-0", &prog_val);
> +	if (ret == 0) {
> +		lp->pdata->led_config_0 = (u8)prog_val;
> +		lp->pdata->led_config_0_set = true;
> +	}
> +	ret = of_property_read_u32(node, "arcticsand,led-config-1", &prog_val);
> +	if (ret == 0) {
> +		lp->pdata->led_config_1 = (u8)prog_val;
> +		lp->pdata->led_config_1_set = true;
> +	}
> +	ret = of_property_read_u32(node, "arcticsand,dim-freq", &prog_val);
> +	if (ret == 0) {
> +		lp->pdata->dim_freq = (u8)prog_val;
> +		lp->pdata->dim_freq_set = true;
> +	}
> +	ret = of_property_read_u32(node, "arcticsand,comp-config", &prog_val);
> +	if (ret == 0) {
> +		lp->pdata->comp_config = (u8)prog_val;
> +		lp->pdata->comp_config_set = true;
> +	}
> +	ret = of_property_read_u32(node, "arcticsand,filter-config", &prog_val);
> +	if (ret == 0) {
> +		lp->pdata->filter_config = (u8)prog_val;
> +		lp->pdata->filter_config_set = true;
> +	}
> +	ret = of_property_read_u32(node, "arcticsand,trim-config", &prog_val);
> +	if (ret == 0) {
> +		lp->pdata->trim_config = (u8)prog_val;
> +		lp->pdata->trim_config_set = true;
> +	}
> +	ret = of_property_count_u32_elems(node, "led-sources");
> +	if (ret < 0)
> +		lp->pdata->led_str = 0x3F;
> +	else {
> +		num_entry = ret;
> +		if (num_entry > 6)
> +			num_entry = 6;
> +
> +		ret = of_property_read_u32_array(node, "led-sources", sources,
> +					num_entry);
> +		if (ret < 0) {
> +			dev_err(dev, "led-sources node is invalid.\n");
> +			return -EINVAL;
> +		}
> +
> +		lp->pdata->led_str = 0;
> +		while (num_entry > 0)
> +			lp->pdata->led_str |= (1 << sources[--num_entry]);
> +	}
> +	return 0;
> +}
> +#else
> +static int arcxcnn_parse_dt(struct arcxcnn *lp)
> +{
> +	return -EINVAL;
> +}
> +#endif
> +
> +static int arcxcnn_probe(struct i2c_client *cl, const struct i2c_device_id *id)
> +{
> +	struct arcxcnn *lp;
> +	int ret;
> +	u8 regval;
> +	u16 chipid;
> +
> +	if (!i2c_check_functionality(cl->adapter, I2C_FUNC_SMBUS_BYTE_DATA))
> +		return -EIO;
> +
> +	lp = devm_kzalloc(&cl->dev, sizeof(*lp), GFP_KERNEL);
> +	if (!lp)
> +		return -ENOMEM;
> +
> +	lp->client = cl;
> +	lp->dev = &cl->dev;
> +	lp->chip_id = id->driver_data;
> +	lp->pdata = dev_get_platdata(&cl->dev);
> +
> +	if (!lp->pdata) {
> +		lp->pdata = devm_kzalloc(lp->dev,
> +				sizeof(*lp->pdata), GFP_KERNEL);
> +		if (!lp->pdata)
> +			return -ENOMEM;
> +
> +		/* no platform data, parse the device-tree for info.  if there
> +		 * is no device tree entry, we are being told we exist because
> +		 * user-land said so, so make up the info we need
> +		 */
> +		ret = arcxcnn_parse_dt(lp);
> +		if (ret < 0) {
> +			/* no device tree, use defaults based on module params
> +			 */
> +			lp->pdata->led_config_0_set = false;
> +			lp->pdata->led_config_1_set = false;
> +			lp->pdata->dim_freq_set = false;
> +			lp->pdata->comp_config_set = false;
> +			lp->pdata->filter_config_set = false;
> +			lp->pdata->trim_config_set = false;
> +
> +			lp->pdata->name = NULL;
> +			lp->pdata->initial_brightness = s_ibright;
> +			lp->pdata->led_str = s_iledstr;
> +		}
> +	}
> +
> +	if (lp->pdata->dim_freq_set)
> +		lp->mode = PWM_BASED;
> +	else
> +		lp->mode = REGISTER_BASED;
> +
> +	i2c_set_clientdata(cl, lp);
> +
> +	/* read device ID */
> +	regval = arcxcnn_read_byte(lp, 0x1E);
> +	chipid = regval;
> +	chipid <<= 8;
> +	regval = arcxcnn_read_byte(lp, 0x1F);
> +	chipid |= regval;
> +
> +	/* make sure it belongs to this driver
> +	 * TODO - handle specific ids
> +	 */
> +	if (chipid != 0x02A5) {
> +		#if 1
> +		dev_info(&cl->dev, "Chip Id is %04X\n", chipid);
> +		#else
> +		dev_err(&cl->dev, "%04X is not ARC2C\n", chipid);
> +		return -ENODEV;
> +		#endif
> +	}
> +	/* reset the device */
> +	arcxcnn_write_byte(lp, ARCXCNN_CMD, ARCXCNN_CMD_RESET);
> +
> +	/* set initial brightness */
> +	arcxcnn_set_brightness(lp, lp->pdata->initial_brightness);
> +
> +	/* if fadectrl set in DT, set the value directly, else leave default */
> +	if (lp->pdata->led_config_0_set)
> +		arcxcnn_write_byte(lp, ARCXCNN_FADECTRL,
> +			lp->pdata->led_config_0);
> +
> +	/* if iled config set in DT, set the value, else internal mode */
> +	if (lp->pdata->led_config_1_set)
> +		arcxcnn_write_byte(lp, ARCXCNN_ILED_CONFIG,
> +			lp->pdata->led_config_1);
> +	else
> +		arcxcnn_write_byte(lp, ARCXCNN_ILED_CONFIG, 0x57);
> +
> +	/* other misc DT settings */
> +	if (lp->pdata->dim_freq_set)
> +		arcxcnn_write_byte(lp, ARCXCNN_FADECTRL, lp->pdata->dim_freq);
> +	if (lp->pdata->comp_config_set)
> +		arcxcnn_write_byte(lp, ARCXCNN_COMP_CONFIG,
> +			lp->pdata->comp_config);
> +	if (lp->pdata->filter_config_set)
> +		arcxcnn_write_byte(lp, ARCXCNN_FILT_CONFIG,
> +			lp->pdata->filter_config);
> +	if (lp->pdata->trim_config_set)
> +		arcxcnn_write_byte(lp, ARCXCNN_IMAXTUNE,
> +			lp->pdata->trim_config);
> +
> +	/* set initial LED Strings */
> +	arcxcnn_update_bit(lp, ARCXCNN_LEDEN,
> +		ARCXCNN_LEDEN_MASK, lp->pdata->led_str);
> +
> +	snprintf(lp->chipname, sizeof(lp->chipname),
> +		"%s-%04X", id->name, chipid);
> +
> +	ret = arcxcnn_backlight_register(lp);
> +	if (ret) {
> +		dev_err(lp->dev,
> +			"failed to register backlight. err: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = sysfs_create_group(&lp->dev->kobj, &arcxcnn_attr_group);
> +	if (ret) {
> +		dev_err(lp->dev, "failed to register sysfs. err: %d\n", ret);
> +		return ret;
> +	}
> +
> +	backlight_update_status(lp->bl);
> +	return 0;
> +}
> +
> +static int arcxcnn_remove(struct i2c_client *cl)
> +{
> +	struct arcxcnn *lp = i2c_get_clientdata(cl);
> +
> +	if (!s_no_reset_on_remove) {
> +		/* disable all strings */
> +		arcxcnn_write_byte(lp, ARCXCNN_LEDEN, 0x00);
> +		/* reset the device */
> +		arcxcnn_write_byte(lp, ARCXCNN_CMD, ARCXCNN_CMD_RESET);
> +	}
> +	lp->bl->props.brightness = 0;
> +	backlight_update_status(lp->bl);
> +	if (lp->supply)
> +		regulator_disable(lp->supply);
> +	sysfs_remove_group(&lp->dev->kobj, &arcxcnn_attr_group);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id arcxcnn_dt_ids[] = {
> +	{ .compatible = "arc,arc2c0608" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, arcxcnn_dt_ids);
> +
> +/* Note that the device/chip ID is not fixed in silicon so
> + * auto-probing of these devices on the bus is most likely
> + * not possible, use device tree to set i2c bus address
> + */
> +static const struct i2c_device_id arcxcnn_ids[] = {
> +	{"arc2c0608", ARC2C0608},
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, arcxcnn_ids);
> +
> +static struct i2c_driver arcxcnn_driver = {
> +	.driver = {
> +		   .name = "arcxcnn_bl",
> +		   .of_match_table = of_match_ptr(arcxcnn_dt_ids),
> +		   },
> +	.probe = arcxcnn_probe,
> +	.remove = arcxcnn_remove,
> +	.id_table = arcxcnn_ids,
> +};
> +
> +module_i2c_driver(arcxcnn_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Brian Dodge <bdodge09-1ViLX0X+lBJBDgjK7y7TUQ@public.gmane.org>");
> +MODULE_DESCRIPTION("ARCXCNN Backlight driver");
> diff --git a/include/linux/i2c/arcxcnn.h b/include/linux/i2c/arcxcnn.h
> new file mode 100644
> index 0000000..1c681dd
> --- /dev/null
> +++ b/include/linux/i2c/arcxcnn.h
> @@ -0,0 +1,67 @@
> +/*
> + * Backlight driver for ArcticSand ARC2C0608 Backlight Devices
> + *
> + * Copyright 2016 ArcticSand, Inc.
> + *
> + * Licensed under the GPL-2 or later.
> + */
> +
> +#ifndef _ARCXCNN_H
> +#define _ARCXCNN_H
> +
> +enum arcxcnn_chip_id {
> +	ARC2C0608
> +};
> +
> +enum arcxcnn_brightness_source {
> +	ARCXCNN_PWM_ONLY,
> +	ARCXCNN_I2C_ONLY = 2,
> +};
> +
> +#define ARCXCNN_MAX_PROGENTRIES	48	/* max a/v pairs for custom */
> +
> +/**
> + * struct arcxcnn_platform_data
> + * @name : Backlight driver name. If it is not defined, default name is set.
> + * @initial_brightness : initial value of backlight brightness
> + * @led_str	 : initial LED string enables, upper bit is global on/off
> + * @led_config_0 : fading speed (period between intensity steps)
> + * @led_config_1 : misc settings, see datasheet
> + * @dim_freq	 : pwm dimming frequency if in pwm mode
> + * @comp_config	 : misc config, see datasheet
> + * @filter_config: RC/PWM filter config, see datasheet
> + * @trim_config	 : full scale current trim, see datasheet
> + * @led_config_0_set	: the value in led_config_0 is valid
> + * @led_config_1_set	: the value in led_config_1 is valid
> + * @dim_freq_set	: the value in dim_freq is valid
> + * @comp_config_set	: the value in comp_config is valid
> + * @filter_config_set	: the value in filter_config is valid
> + * @trim_config_set	: the value in trim_config is valid
> + *
> + * the _set flags are used to indicate that the value was explicitly set
> + * in the device tree or platform data. settings not set are left as default
> + * power-on default values of the chip except for led_str and led_config_1
> + * which are set by the driver (led_str is specified indirectly in the
> + * device tree via "led-sources")
> + */
> +struct arcxcnn_platform_data {
> +	const char *name;
> +	u16 initial_brightness;
> +	u8	led_str;
> +
> +	u8	led_config_0;
> +	u8	led_config_1;
> +	u8	dim_freq;
> +	u8	comp_config;
> +	u8	filter_config;
> +	u8	trim_config;
> +
> +	bool	led_config_0_set;
> +	bool	led_config_1_set;
> +	bool	dim_freq_set;
> +	bool	comp_config_set;
> +	bool	filter_config_set;
> +	bool	trim_config_set;
> +};
> +
> +#endif

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 2/3] ipmi/bt-bmc: maintain a request expiry list
From: Corey Minyard @ 2016-11-09 15:52 UTC (permalink / raw)
  To: Cédric Le Goater,
	openipmi-developer-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Joel Stanley
  Cc: Rob Herring, Arnd Bergmann, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Brendan Higgins
In-Reply-To: <6117c1fd-b969-9394-0be5-d46f64269cac-Bxea+6Xhats@public.gmane.org>

On 11/09/2016 08:30 AM, Cédric Le Goater wrote:
> On 11/07/2016 08:04 PM, Corey Minyard wrote:
>> On 11/02/2016 02:57 AM, Cédric Le Goater wrote:
>>> Regarding the response expiration handling, the IPMI spec says :
>>>
>>>      The BMC must not return a given response once the corresponding
>>>      Request-to-Response interval has passed. The BMC can ensure this
>>>      by maintaining its own internal list of outstanding requests through
>>>      the interface. The BMC could age and expire the entries in the list
>>>      by expiring the entries at an interval that is somewhat shorter than
>>>      the specified Request-to-Response interval....
>>>
>>> To handle such case, we maintain list of received requests using the
>>> seq number of the BT message to identify them. The list is updated
>>> each time a request is received and a response is sent. The expiration
>>> of the reponses is handled at each updates but also with a timer.
>> This looks correct, but it seems awfully complicated.
>>
>> Why can't you get the current time before the wait_event_interruptible()
>> and then compare the time before you do the write?  That would seem to
>> accomplish the same thing without any lists or extra locks.
> Well, the expiry list needs a request identifier and it is currently using
> the Seq byte for this purpose. So the BT message needs to be read to grab
> that byte. The request is added to a list and that involves some locking.
>
> When the response is written, the first matching request is removed from
> the list and a garbage collector loop is also run. Then, as we might not
> get any responses to run that loop, we use a timer to empty the list from
> any expired requests.
>
> The read/write ops of the driver are protected with a mutex, the list and
> the timer add their share of locking. That could have been done with RCU
> surely but we don't really need performance in this driver.
>
> Caveats :
>
> bt_bmc_remove_request() should not be done in the writing loop though.
> It needs a fix.
>
> The request identifier is currently Seq but the spec say we should use
> Seq, NetFn, and Command or an internal Seq value as a request identifier.
> Google is also working on an OEM/Group extension (Brendan in CC: )
> which has a different message format. I need to look closer at what
> should be done in this case.

I'm still not sure why the list is necessary.  You have a separate
thread of execution for each writer, why not just time it in that
thread?

What about the following, not even compile-tested, patch?  I'm
sure my mailer will munge this up, I can send you a clean version
if you like.

 From 1a73585a9c1c74ac1d59d82f22e05b30447619a6 Mon Sep 17 00:00:00 2001
From: Corey Minyard <cminyard-Igf4POYTYCDQT0dZR+AlfA@public.gmane.org>
Date: Wed, 9 Nov 2016 09:07:48 -0600
Subject: [PATCH] ipmi:bt-bmc: Fix a multi-user race, time out responses

The IPMI spec says to time out responses after a given amount of
time, so don't let a writer send something after an amount of time
has elapsed.

Also, fix a race condition in the same area where if you have two
writers at the same time, one can get a EIO return when it should
still be waiting its turn to send.  A mutex_lock_interruptible_timeout()
would be handy here, but it doesn't exist.

Signed-off-by: Corey Minyard <cminyard-Igf4POYTYCDQT0dZR+AlfA@public.gmane.org>
---
  drivers/char/ipmi/bt-bmc.c | 39 ++++++++++++++++++++++++---------------
  1 file changed, 24 insertions(+), 15 deletions(-)

diff --git a/drivers/char/ipmi/bt-bmc.c b/drivers/char/ipmi/bt-bmc.c
index b49e613..5be94cf 100644
--- a/drivers/char/ipmi/bt-bmc.c
+++ b/drivers/char/ipmi/bt-bmc.c
@@ -57,6 +57,8 @@

  #define BT_BMC_BUFFER_SIZE 256

+#define BT_BMC_RESPONSE_JIFFIES    (5 * HZ)
+
  struct bt_bmc {
      struct device        dev;
      struct miscdevice    miscdev;
@@ -190,14 +192,12 @@ static ssize_t bt_bmc_read(struct file *file, char 
__user *buf,

      WARN_ON(*ppos);

-    if (wait_event_interruptible(bt_bmc->queue,
-                     bt_inb(bt_bmc, BT_CTRL) & BT_CTRL_H2B_ATN))
+    if (mutex_lock_interruptible(&bt_bmc->mutex))
          return -ERESTARTSYS;

-    mutex_lock(&bt_bmc->mutex);
-
-    if (unlikely(!(bt_inb(bt_bmc, BT_CTRL) & BT_CTRL_H2B_ATN))) {
-        ret = -EIO;
+    if (wait_event_interruptible(bt_bmc->queue,
+                bt_inb(bt_bmc, BT_CTRL) & BT_CTRL_H2B_ATN)) {
+        ret = -ERESTARTSYS;
          goto out_unlock;
      }

@@ -251,6 +251,7 @@ static ssize_t bt_bmc_write(struct file *file, const 
char __user *buf,
      u8 kbuffer[BT_BMC_BUFFER_SIZE];
      ssize_t ret = 0;
      ssize_t nwritten;
+    unsigned long start_jiffies = jiffies, wait_time;

      /*
       * send a minimum response size
@@ -263,23 +264,31 @@ static ssize_t bt_bmc_write(struct file *file, 
const char __user *buf,

      WARN_ON(*ppos);

+    if (mutex_lock_interruptible(&bt_bmc->mutex))
+        return -ERESTARTSYS;
+
+    wait_time = jiffies - start_jiffies;
+    if (wait_time >= BT_BMC_RESPONSE_TIME_JIFFIES) {
+        ret = -ETIMEDOUT;
+        goto out_unlock;
+    }
+    wait_time = BT_BMC_RESPONSE_TIME_JIFFIES - wait_time;
+
      /*
       * There's no interrupt for clearing bmc busy so we have to
       * poll
       */
-    if (wait_event_interruptible(bt_bmc->queue,
+    ret = wait_event_interruptible_timeout(bt_bmc->queue,
                       !(bt_inb(bt_bmc, BT_CTRL) &
-                       (BT_CTRL_H_BUSY | BT_CTRL_B2H_ATN))))
-        return -ERESTARTSYS;
-
-    mutex_lock(&bt_bmc->mutex);
-
-    if (unlikely(bt_inb(bt_bmc, BT_CTRL) &
-             (BT_CTRL_H_BUSY | BT_CTRL_B2H_ATN))) {
-        ret = -EIO;
+                       (BT_CTRL_H_BUSY | BT_CTRL_B2H_ATN)),
+                     wait_time);
+    if (ret <= 0) {
+        if (ret == 0)
+            ret = -ETIMEDOUT;
          goto out_unlock;
      }

+    ret = 0;
      clr_wr_ptr(bt_bmc);

      while (count) {
-- 
2.7.4



>> Also, if you are going to have multiple writers on this interface, I don't
>> think the interface will work correctly.  I think you need to claim the
>> mutex first.  Otherwise you might get into a situation where two writers
>> will wake up at the same time, the first writes and releases the mutex,
>> then the second will find that the interface is busy and fail.
> yes. that is a current problem in the driver and it is not really an
> elegant way to handle concurrency. We are fine for the moment as we
> only have one single threaded process using the device.
>
>> If I am correct, the mutex will need to become interruptible and come
>> first, I think.  (And the timing would have to start before the mutex,
>> of course.)  This applies to both the read and write interface.
> OK. I will look into fixing this problem first.
>
>> Another thing is that this is request-to-release time.  If a request takes
>> a long time to process (say, a write to a flash device) the timeout would
>> need to be decreased by the processing time.
> Hmm, how would that fit with the "BT Interface Capabilities" which
> defines :
>
>    BMC Request-to-Response time, in seconds, 1 based. 30 seconds, maximum.
>
> This is a fixed value. And the spec only say :
>
>    The BMC could age and expire the entries in the list by expiring
>    the entries at an interval that is somewhat shorter than the
>    specified Request-to-Response interval.
>
> May be I am misunderstanding.

Yeah, as usual, the IPMI spec is kind of vague about this.  You have
to think about the problem from the host driver's point of view to
know what this means.

 From a host driver point of view, it's going to send a request and wait
for a response with the same sequence number.  It should time out the
request in "BMC Request-to-Response time" seconds.  After that point,
it's free to re-use the sequence number.  So what a BMC cannot do is
send that response after the timeout.

If the timeout is 5 seconds, and the BMC gets a flash write request that
takes 3 seconds then sits waiting for 2 seconds, it should not send the
response because the host driver may mistake it for a request it just
sent.  So the timeout should be based on when the BMC got the request,
not when the response was queued for write, and that's the reason that
the BMC timer should be somewhat shorter than the host timer, as it
says.

I don't see an easy way to do this.  Some possibilities I can think of are:

  * Switch to using an ioctl to do the read/write operation. That's
    kind of ugly.  It's what the IPMI driver interface does, and it
    has been somewhat of a pain.
  * You could define a "header" that is put into the read message
    and write message.  You could add an ioctl to query the header
    size; if the ioctl fails then the driver doesn't have a header.
    Add a jiffie to the header of the read message that the
    BMC userland must put into the header of the write message.
    You could even have the ioctl enable the header, to preserve
    backwards compatibility.
  * Define some sort of structure that you read/write.  This has all
    sorts of issues.

I think my preference is the second, it allows for backwards
compatibility and lets the kernel do basically whatever it wants with
the data.


>> It's probably ok to not do that for the moment, but you may want to add
>> a note.  Fixing that would require adding a timeout for each message.
> Thanks,
>
> C.
>
>> -corey
>>
>>
>>> Signed-off-by: Cédric Le Goater <clg-Bxea+6Xhats@public.gmane.org>
>>> ---
>>>    drivers/char/ipmi/bt-bmc.c | 132 +++++++++++++++++++++++++++++++++++++++++++++
>>>    1 file changed, 132 insertions(+)
>>>
>>> diff --git a/drivers/char/ipmi/bt-bmc.c b/drivers/char/ipmi/bt-bmc.c
>>> index fc9e8891eae3..e751e4a754b7 100644
>>> --- a/drivers/char/ipmi/bt-bmc.c
>>> +++ b/drivers/char/ipmi/bt-bmc.c
>>> @@ -17,6 +17,7 @@
>>>    #include <linux/platform_device.h>
>>>    #include <linux/poll.h>
>>>    #include <linux/sched.h>
>>> +#include <linux/slab.h>
>>>    #include <linux/timer.h>
>>>      /*
>>> @@ -57,6 +58,15 @@
>>>      #define BT_BMC_BUFFER_SIZE 256
>>>    +#define BT_BMC_RESPONSE_TIME    5 /* seconds */
>>> +
>>> +struct ipmi_request {
>>> +    struct list_head list;
>>> +
>>> +    u8 seq;
>>> +    unsigned long expires;
>>> +};
>>> +
>>>    struct bt_bmc {
>>>        struct device        dev;
>>>        struct miscdevice    miscdev;
>>> @@ -65,6 +75,11 @@ struct bt_bmc {
>>>        wait_queue_head_t    queue;
>>>        struct timer_list    poll_timer;
>>>        struct mutex        mutex;
>>> +
>>> +    unsigned int        response_time;
>>> +    struct timer_list    requests_timer;
>>> +    spinlock_t              requests_lock;
>>> +    struct list_head        requests;
>>>    };
>>>      static atomic_t open_count = ATOMIC_INIT(0);
>>> @@ -163,6 +178,94 @@ static int bt_bmc_open(struct inode *inode, struct file *file)
>>>    }
>>>      /*
>>> + * lock should be held
>>> + */
>>> +static void drop_expired_requests(struct bt_bmc *bt_bmc)
>>> +{
>>> +    unsigned long flags = 0;
>>> +    struct ipmi_request *req, *next;
>>> +    unsigned long next_expires = 0;
>>> +    int start_timer = 0;
>>> +
>>> +    spin_lock_irqsave(&bt_bmc->requests_lock, flags);
>>> +    list_for_each_entry_safe(req, next, &bt_bmc->requests, list) {
>>> +        if (time_after_eq(jiffies, req->expires)) {
>>> +            pr_warn("BT: request seq:%d has expired. dropping\n",
>>> +                req->seq);
>>> +            list_del(&req->list);
>>> +            kfree(req);
>>> +            continue;
>>> +        }
>>> +        if (!start_timer) {
>>> +            start_timer = 1;
>>> +            next_expires = req->expires;
>>> +        } else {
>>> +            next_expires = min(next_expires, req->expires);
>>> +        }
>>> +    }
>>> +    spin_unlock_irqrestore(&bt_bmc->requests_lock, flags);
>>> +
>>> +    /* and possibly restart */
>>> +    if (start_timer)
>>> +        mod_timer(&bt_bmc->requests_timer, next_expires);
>>> +}
>>> +
>>> +static void requests_timer(unsigned long data)
>>> +{
>>> +    struct bt_bmc *bt_bmc = (void *)data;
>>> +
>>> +    drop_expired_requests(bt_bmc);
>>> +}
>>> +
>>> +static int bt_bmc_add_request(struct bt_bmc *bt_bmc, u8 seq)
>>> +{
>>> +    struct ipmi_request *req;
>>> +
>>> +    req = kmalloc(sizeof(*req), GFP_KERNEL);
>>> +    if (!req)
>>> +        return -ENOMEM;
>>> +
>>> +    req->seq = seq;
>>> +    req->expires = jiffies +
>>> +        msecs_to_jiffies(bt_bmc->response_time * 1000);
>>> +
>>> +    spin_lock(&bt_bmc->requests_lock);
>>> +    list_add(&req->list, &bt_bmc->requests);
>>> +    spin_unlock(&bt_bmc->requests_lock);
>>> +
>>> +    drop_expired_requests(bt_bmc);
>>> +    return 0;
>>> +}
>>> +
>>> +static int bt_bmc_remove_request(struct bt_bmc *bt_bmc, u8 seq)
>>> +{
>>> +    struct ipmi_request *req, *next;
>>> +    int ret = -EBADRQC; /* Invalid request code */
>>> +
>>> +    spin_lock(&bt_bmc->requests_lock);
>>> +    list_for_each_entry_safe(req, next, &bt_bmc->requests, list) {
>>> +        /*
>>> +         * The sequence number should be unique, so remove the
>>> +         * first matching request found. If there are others,
>>> +         * they should expire
>>> +         */
>>> +        if (req->seq == seq) {
>>> +            list_del(&req->list);
>>> +            kfree(req);
>>> +            ret = 0;
>>> +            break;
>>> +        }
>>> +    }
>>> +    spin_unlock(&bt_bmc->requests_lock);
>>> +
>>> +    if (ret)
>>> +        pr_warn("BT: request seq:%d is invalid\n", seq);
>>> +
>>> +    drop_expired_requests(bt_bmc);
>>> +    return ret;
>>> +}
>>> +
>>> +/*
>>>     * The BT (Block Transfer) interface means that entire messages are
>>>     * buffered by the host before a notification is sent to the BMC that
>>>     * there is data to be read. The first byte is the length and the
>>> @@ -231,6 +334,13 @@ static ssize_t bt_bmc_read(struct file *file, char __user *buf,
>>>            len_byte = 0;
>>>        }
>>>    +    if (ret > 0) {
>>> +        int ret2 = bt_bmc_add_request(bt_bmc, kbuffer[2]);
>>> +
>>> +        if (ret2)
>>> +            ret = ret2;
>>> +    }
>>> +
>>>        clr_b_busy(bt_bmc);
>>>      out_unlock:
>>> @@ -283,12 +393,20 @@ static ssize_t bt_bmc_write(struct file *file, const char __user *buf,
>>>        clr_wr_ptr(bt_bmc);
>>>          while (count) {
>>> +        int ret2;
>>> +
>>>            nwritten = min_t(ssize_t, count, sizeof(kbuffer));
>>>            if (copy_from_user(&kbuffer, buf, nwritten)) {
>>>                ret = -EFAULT;
>>>                break;
>>>            }
>>>    +        ret2 = bt_bmc_remove_request(bt_bmc, kbuffer[2]);
>>> +        if (ret2) {
>>> +            ret = ret2;
>>> +            break;
>>> +        }
>>> +
>>>            bt_writen(bt_bmc, kbuffer, nwritten);
>>>              count -= nwritten;
>>> @@ -438,6 +556,8 @@ static int bt_bmc_probe(struct platform_device *pdev)
>>>          mutex_init(&bt_bmc->mutex);
>>>        init_waitqueue_head(&bt_bmc->queue);
>>> +    INIT_LIST_HEAD(&bt_bmc->requests);
>>> +    spin_lock_init(&bt_bmc->requests_lock);
>>>          bt_bmc->miscdev.minor    = MISC_DYNAMIC_MINOR,
>>>            bt_bmc->miscdev.name    = DEVICE_NAME,
>>> @@ -451,6 +571,8 @@ static int bt_bmc_probe(struct platform_device *pdev)
>>>          bt_bmc_config_irq(bt_bmc, pdev);
>>>    +    bt_bmc->response_time = BT_BMC_RESPONSE_TIME;
>>> +
>>>        if (bt_bmc->irq) {
>>>            dev_info(dev, "Using IRQ %d\n", bt_bmc->irq);
>>>        } else {
>>> @@ -468,6 +590,9 @@ static int bt_bmc_probe(struct platform_device *pdev)
>>>              BT_CR0_ENABLE_IBT,
>>>              bt_bmc->base + BT_CR0);
>>>    +    setup_timer(&bt_bmc->requests_timer, requests_timer,
>>> +            (unsigned long)bt_bmc);
>>> +
>>>        clr_b_busy(bt_bmc);
>>>          return 0;
>>> @@ -476,10 +601,17 @@ static int bt_bmc_probe(struct platform_device *pdev)
>>>    static int bt_bmc_remove(struct platform_device *pdev)
>>>    {
>>>        struct bt_bmc *bt_bmc = dev_get_drvdata(&pdev->dev);
>>> +    struct ipmi_request *req, *next;
>>>          misc_deregister(&bt_bmc->miscdev);
>>>        if (!bt_bmc->irq)
>>>            del_timer_sync(&bt_bmc->poll_timer);
>>> +
>>> +    del_timer_sync(&bt_bmc->requests_timer);
>>> +    list_for_each_entry_safe(req, next, &bt_bmc->requests, list) {
>>> +        list_del(&req->list);
>>> +        kfree(req);
>>> +    }
>>>        return 0;
>>>    }
>>>    
>>

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH 5/5] media: platform: rcar_drif: Add DRIF support
From: Ramesh Shanmugasundaram @ 2016-11-09 15:44 UTC (permalink / raw)
  To: robh+dt, mark.rutland, mchehab, hverkuil, sakari.ailus, crope
  Cc: chris.paterson2, laurent.pinchart, geert+renesas, linux-media,
	devicetree, linux-renesas-soc, Ramesh Shanmugasundaram
In-Reply-To: <1478706284-59134-1-git-send-email-ramesh.shanmugasundaram@bp.renesas.com>

This patch adds Digital Radio Interface (DRIF) support to R-Car Gen3 SoCs.
The driver exposes each instance of DRIF as a V4L2 SDR device. A DRIF
device represents a channel and each channel can have one or two
sub-channels respectively depending on the target board.

DRIF supports only Rx functionality. It receives samples from a RF
frontend tuner chip it is interfaced with. The combination of DRIF and the
tuner device, which is registered as a sub-device, determines the receive
sample rate and format.

In order to be compliant as a V4L2 SDR device, DRIF needs to bind with
the tuner device, which can be provided by a third party vendor. DRIF acts
as a slave device and the tuner device acts as a master transmitting the
samples. The driver allows asynchronous binding of a tuner device that
is registered as a v4l2 sub-device. The driver can learn about the tuner
it is interfaced with based on port endpoint properties of the device in
device tree. The V4L2 SDR device inherits the controls exposed by the
tuner device.

The device can also be configured to use either one or both of the data
pins at runtime based on the master (tuner) configuration.

Signed-off-by: Ramesh Shanmugasundaram <ramesh.shanmugasundaram@bp.renesas.com>
---
 .../devicetree/bindings/media/renesas,drif.txt     |  136 ++
 drivers/media/platform/Kconfig                     |   25 +
 drivers/media/platform/Makefile                    |    1 +
 drivers/media/platform/rcar_drif.c                 | 1574 ++++++++++++++++++++
 4 files changed, 1736 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/renesas,drif.txt
 create mode 100644 drivers/media/platform/rcar_drif.c

diff --git a/Documentation/devicetree/bindings/media/renesas,drif.txt b/Documentation/devicetree/bindings/media/renesas,drif.txt
new file mode 100644
index 0000000..d65368a
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/renesas,drif.txt
@@ -0,0 +1,136 @@
+Renesas R-Car Gen3 Digital Radio Interface controller (DRIF)
+------------------------------------------------------------
+
+R-Car Gen3 DRIF is a serial slave device. It interfaces with a master
+device as shown below
+
++---------------------+                +---------------------+
+|                     |-----SCK------->|CLK                  |
+|       Master        |-----SS-------->|SYNC  DRIFn (slave)  |
+|                     |-----SD0------->|D0                   |
+|                     |-----SD1------->|D1                   |
++---------------------+                +---------------------+
+
+Each DRIF channel (drifn) consists of two sub-channels (drifn0 & drifn1).
+The sub-channels are like two individual channels in itself that share the
+common CLK & SYNC. Each sub-channel has it's own dedicated resources like
+irq, dma channels, address space & clock.
+
+The device tree model represents the channel and each of it's sub-channel
+as a separate node. The parent channel ties the sub-channels together with
+their phandles.
+
+Required properties of a sub-channel:
+-------------------------------------
+- compatible: "renesas,r8a7795-drif" if DRIF controller is a part of R8A7795 SoC.
+	      "renesas,rcar-gen3-drif" for a generic R-Car Gen3 compatible device.
+	      When compatible with the generic version, nodes must list the
+	      SoC-specific version corresponding to the platform first
+	      followed by the generic version.
+- reg: offset and length of that sub-channel.
+- interrupts: associated with that sub-channel.
+- clocks: phandle and clock specifier of that sub-channel.
+- clock-names: clock input name string: "fck".
+- dmas: phandles to the DMA channel of that sub-channel.
+- dma-names: names of the DMA channel: "rx".
+
+Optional properties of a sub-channel:
+-------------------------------------
+- power-domains: phandle to the respective power domain.
+
+Required properties of a channel:
+---------------------------------
+- pinctrl-0: pin control group to be used for this channel.
+- pinctrl-names: must be "default".
+- sub-channels : phandles to the two sub-channels.
+
+Optional properties of a channel:
+---------------------------------
+- port: child port node of a channel that defines the local and remote
+        endpoints. The remote endpoint is assumed to be a tuner subdevice
+	endpoint.
+- renesas,syncmd       : sync mode
+			 0 (Frame start sync pulse mode. 1-bit width pulse
+			    indicates start of a frame)
+			 1 (L/R sync or I2S mode) (default)
+- renesas,lsb-first    : empty property indicates lsb bit is received first.
+			 When not defined msb bit is received first (default)
+- renesas,syncac-pol-high  : empty property indicates sync signal polarity.
+			 When defined, active high or high->low sync signal.
+			 When not defined, active low or low->high sync signal
+			 (default)
+- renesas,dtdl         : delay between sync signal and start of reception.
+			 Must contain one of the following values:
+			 0   (no bit delay)
+			 50  (0.5-clock-cycle delay)
+			 100 (1-clock-cycle delay) (default)
+			 150 (1.5-clock-cycle delay)
+			 200 (2-clock-cycle delay)
+- renesas,syncdl       : delay between end of reception and sync signal edge.
+			 Must contain one of the following values:
+			 0   (no bit delay) (default)
+			 50  (0.5-clock-cycle delay)
+			 100 (1-clock-cycle delay)
+			 150 (1.5-clock-cycle delay)
+			 200 (2-clock-cycle delay)
+			 300 (3-clock-cycle delay)
+
+Example
+--------
+
+SoC common dtsi file
+
+		drif00: rif@e6f40000 {
+			compatible = "renesas,r8a7795-drif",
+				     "renesas,rcar-gen3-drif";
+			reg = <0 0xe6f40000 0 0x64>;
+			interrupts = <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 515>;
+			clock-names = "fck";
+			dmas = <&dmac1 0x20>, <&dmac2 0x20>;
+			dma-names = "rx", "rx";
+			power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;
+			status = "disabled";
+		};
+
+		drif01: rif@e6f50000 {
+			compatible = "renesas,r8a7795-drif",
+				     "renesas,rcar-gen3-drif";
+			reg = <0 0xe6f50000 0 0x64>;
+			interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 514>;
+			clock-names = "fck";
+			dmas = <&dmac1 0x22>, <&dmac2 0x22>;
+			dma-names = "rx", "rx";
+			power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;
+			status = "disabled";
+		};
+
+		drif0: rif@0 {
+			compatible = "renesas,r8a7795-drif",
+				     "renesas,rcar-gen3-drif";
+			sub-channels = <&drif00>, <&drif01>;
+			status = "disabled";
+		};
+
+Board specific dts file
+
+&drif00 {
+	status = "okay";
+};
+
+&drif01 {
+	status = "okay";
+};
+
+&drif0 {
+	pinctrl-0 = <&drif0_pins>;
+	pinctrl-names = "default";
+	renesas,syncac-pol-high;
+	status = "okay";
+	port {
+		drif0_ep: endpoint {
+		     remote-endpoint = <&tuner_subdev_ep>;
+		};
+	};
+};
diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index 754edbf1..0ae83a8 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -393,3 +393,28 @@ menuconfig DVB_PLATFORM_DRIVERS
 if DVB_PLATFORM_DRIVERS
 source "drivers/media/platform/sti/c8sectpfe/Kconfig"
 endif #DVB_PLATFORM_DRIVERS
+
+menuconfig SDR_PLATFORM_DRIVERS
+	bool "SDR platform devices"
+	depends on MEDIA_SDR_SUPPORT
+	default n
+	---help---
+	  Say Y here to enable support for platform-specific SDR Drivers.
+
+if SDR_PLATFORM_DRIVERS
+
+config VIDEO_RCAR_DRIF
+	tristate "Renesas Digitial Radio Interface (DRIF)"
+	depends on VIDEO_V4L2 && HAS_DMA
+	depends on ARCH_RENESAS
+	select VIDEOBUF2_VMALLOC
+	---help---
+	  Say Y if you want to enable R-Car Gen3 DRIF support. DRIF is Digital
+	  Radio Interface that interfaces with an RF front end chip. It is a
+	  receiver of digital data which uses DMA to transfer received data to
+	  a configured location for an application to use.
+
+	  To compile this driver as a module, choose M here; the module
+	  will be called rcar_drif.
+
+endif # SDR_PLATFORM_DRIVERS
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index f842933..49ce238 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -49,6 +49,7 @@ obj-$(CONFIG_SOC_CAMERA)		+= soc_camera/
 
 obj-$(CONFIG_VIDEO_RENESAS_FCP) 	+= rcar-fcp.o
 obj-$(CONFIG_VIDEO_RENESAS_JPU) 	+= rcar_jpu.o
+obj-$(CONFIG_VIDEO_RCAR_DRIF)		+= rcar_drif.o
 obj-$(CONFIG_VIDEO_RENESAS_VSP1)	+= vsp1/
 
 obj-y	+= omap/
diff --git a/drivers/media/platform/rcar_drif.c b/drivers/media/platform/rcar_drif.c
new file mode 100644
index 0000000..34dc282
--- /dev/null
+++ b/drivers/media/platform/rcar_drif.c
@@ -0,0 +1,1574 @@
+/*
+ * R-Car Gen3 Digital Radio Interface (DRIF) driver
+ *
+ * Copyright (C) 2016 Renesas Electronics Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+/*
+ * The R-Car DRIF is a receive only MSIOF like controller with an
+ * external master device driving the SCK. It receives data into a FIFO,
+ * then this driver uses the SYS-DMAC engine to move the data from
+ * the device to memory.
+ *
+ * Each DRIF channel supports two sub-channels with their own resources
+ * like module clk, register set and dma.
+ *
+ * Depending on the master device, a DRIF channel can use
+ *  (1) both sub-channels (D0 & D1 pins) to receive data in parallel (or)
+ *  (2) one sub-channel (D0 or D1) to receive data
+ *
+ * The primary design goal of this controller is to act as Digitial Radio
+ * Interface that receives digital samples from a tuner device. Hence the
+ * driver exposes the device as a V4L2 SDR device. In order to qualify as
+ * a V4L2 SDR device, it should possess tuner interface as mandated by the
+ * framework. This driver expects a tuner driver (sub-device) to bind
+ * asynchronously with this device and the combined drivers shall expose
+ * a V4L2 compliant SDR device. The DRIF driver is independent of the
+ * tuner vendor.
+ */
+#include <linux/bitops.h>
+#include <linux/clk.h>
+#include <linux/dma-mapping.h>
+#include <linux/dmaengine.h>
+#include <linux/ioctl.h>
+#include <linux/iopoll.h>
+#include <linux/module.h>
+#include <linux/of_graph.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/sched.h>
+#include <media/v4l2-async.h>
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-event.h>
+#include <media/v4l2-fh.h>
+#include <media/v4l2-ioctl.h>
+#include <media/videobuf2-v4l2.h>
+#include <media/videobuf2-vmalloc.h>
+
+/* DRIF register offsets */
+#define RCAR_DRIF_SITMDR1			0x00
+#define RCAR_DRIF_SITMDR2			0x04
+#define RCAR_DRIF_SITMDR3			0x08
+#define RCAR_DRIF_SIRMDR1			0x10
+#define RCAR_DRIF_SIRMDR2			0x14
+#define RCAR_DRIF_SIRMDR3			0x18
+#define RCAR_DRIF_SICTR				0x28
+#define RCAR_DRIF_SIFCTR			0x30
+#define RCAR_DRIF_SISTR				0x40
+#define RCAR_DRIF_SIIER				0x44
+#define RCAR_DRIF_SIRFDR			0x60
+
+#define RCAR_DRIF_RFOVF			BIT(3)	/* Receive FIFO overflow */
+#define RCAR_DRIF_RFUDF			BIT(4)	/* Receive FIFO underflow */
+#define RCAR_DRIF_RFSERR		BIT(5)	/* Receive frame sync error */
+#define RCAR_DRIF_REOF			BIT(7)	/* Frame reception end */
+#define RCAR_DRIF_RDREQ			BIT(12) /* Receive data xfer req */
+#define RCAR_DRIF_RFFUL			BIT(13)	/* Receive FIFO full */
+
+/* SIRMDR1 */
+#define RCAR_DRIF_SIRMDR1_SYNCMD_FRAME		(0 << 28)
+#define RCAR_DRIF_SIRMDR1_SYNCMD_LR		(3 << 28)
+
+#define RCAR_DRIF_SIRMDR1_SYNCAC_POL_HIGH	(0 << 25)
+#define RCAR_DRIF_SIRMDR1_SYNCAC_POL_LOW	(1 << 25)
+
+#define RCAR_DRIF_SIRMDR1_MSB_FIRST		(0 << 24)
+#define RCAR_DRIF_SIRMDR1_LSB_FIRST		(1 << 24)
+
+#define RCAR_DRIF_SIRMDR1_DTDL_0		(0 << 20)
+#define RCAR_DRIF_SIRMDR1_DTDL_1		(1 << 20)
+#define RCAR_DRIF_SIRMDR1_DTDL_2		(2 << 20)
+#define RCAR_DRIF_SIRMDR1_DTDL_0PT5		(5 << 20)
+#define RCAR_DRIF_SIRMDR1_DTDL_1PT5		(6 << 20)
+
+#define RCAR_DRIF_SIRMDR1_SYNCDL_0		(0 << 20)
+#define RCAR_DRIF_SIRMDR1_SYNCDL_1		(1 << 20)
+#define RCAR_DRIF_SIRMDR1_SYNCDL_2		(2 << 20)
+#define RCAR_DRIF_SIRMDR1_SYNCDL_3		(3 << 20)
+#define RCAR_DRIF_SIRMDR1_SYNCDL_0PT5		(5 << 20)
+#define RCAR_DRIF_SIRMDR1_SYNCDL_1PT5		(6 << 20)
+
+#define RCAR_DRIF_MDR_GRPCNT(n)			(((n) - 1) << 30)
+#define RCAR_DRIF_MDR_BITLEN(n)			(((n) - 1) << 24)
+#define RCAR_DRIF_MDR_WDCNT(n)			(((n) - 1) << 16)
+
+/* Hidden Transmit register that controls CLK & SYNC */
+#define RCAR_DRIF_SITMDR1_PCON			BIT(30)
+
+#define RCAR_DRIF_SICTR_RX_RISING_EDGE		BIT(26)
+#define RCAR_DRIF_SICTR_RX_EN			BIT(8)
+#define RCAR_DRIF_SICTR_RESET			BIT(0)
+
+/* Constants */
+#define RCAR_DRIF_MAX_NUM_HWBUFS		32
+#define RCAR_DRIF_MAX_DEVS			4
+#define RCAR_DRIF_DEFAULT_NUM_HWBUFS		16
+#define RCAR_DRIF_DEFAULT_HWBUF_SIZE		(4 * PAGE_SIZE)
+#define RCAR_DRIF_MAX_SUBCHANS			2
+#define RCAR_SDR_BUFFER_SIZE			SZ_64K
+
+/* Internal buffer status flags */
+#define RCAR_DRIF_BUF_DONE			BIT(0)	/* DMA completed */
+#define RCAR_DRIF_BUF_OVERFLOW			BIT(1)	/* Overflow detected */
+
+#define to_rcar_drif_buf_pair(p, ch, idx)	(p->sch[!(ch)]->buf[idx])
+
+#define for_each_rcar_drif_subchannel(sch, schans_mask)			\
+	for_each_set_bit(sch, schans_mask, RCAR_DRIF_MAX_SUBCHANS)
+
+#define for_each_rcar_drif_subdev(sd, tmp, ch)				\
+	list_for_each_entry_safe(sd, tmp, &ch->v4l2_dev.subdevs, list)
+
+/* Debug */
+static unsigned int debug;
+module_param(debug, uint, 0644);
+MODULE_PARM_DESC(debug, "activate debug info");
+
+#define rdrif_dbg(level, ch, fmt, arg...)				\
+	v4l2_dbg(level, debug, &ch->v4l2_dev, fmt, ## arg)
+
+#define rdrif_err(ch, fmt, arg...)					\
+	v4l2_err(&ch->v4l2_dev, fmt, ## arg)
+
+/* Stream formats */
+struct rcar_drif_format {
+	char	*name;
+	u32	pixelformat;
+	u32	buffersize;
+	u32	wdlen;
+	u32	num_schans;
+};
+
+/* Format descriptions for capture and preview */
+static const struct rcar_drif_format formats[] = {
+	{
+		.name		= "Sliced Complex U16BE",
+		.pixelformat	= V4L2_SDR_FMT_SCU16BE,
+		.buffersize	= RCAR_SDR_BUFFER_SIZE,
+		.wdlen		= 16,
+		.num_schans	= 2,
+	},
+	{
+		.name		= "Sliced Complex U18BE",
+		.pixelformat	= V4L2_SDR_FMT_SCU18BE,
+		.buffersize	= RCAR_SDR_BUFFER_SIZE,
+		.wdlen		= 18,
+		.num_schans	= 2,
+	},
+	{
+		.name		= "Sliced Complex U20BE",
+		.pixelformat	= V4L2_SDR_FMT_SCU20BE,
+		.buffersize	= RCAR_SDR_BUFFER_SIZE,
+		.wdlen		= 20,
+		.num_schans	= 2,
+	},
+};
+
+static const unsigned int NUM_FORMATS = ARRAY_SIZE(formats);
+
+/* Buffer for a received frame from one or both sub-channels */
+struct rcar_drif_frame_buf {
+	/* Common v4l buffer stuff -- must be first */
+	struct vb2_v4l2_buffer vb;
+	struct list_head list;
+};
+
+struct rcar_drif_async_subdev {
+	struct v4l2_subdev *sd;
+	struct v4l2_async_subdev asd;
+};
+
+/* DMA buffer */
+struct rcar_drif_hwbuf {
+	void *addr;			/* Cpu-side address */
+	unsigned int status;		/* Buffer status flags */
+};
+
+/* Sub-channel */
+struct rcar_drif_subchan {
+	struct rcar_drif_chan *parent;	/* Parent channel */
+	struct platform_device *pdev;	/* Sub-channel's pdev */
+	void __iomem *base;		/* Base register address */
+	resource_size_t start;		/* I/O resource offset */
+	struct dma_chan *dmach;		/* Reserved DMA channel */
+	struct clk *clkp;		/* Module clock */
+	struct rcar_drif_hwbuf *buf[RCAR_DRIF_MAX_NUM_HWBUFS];
+	dma_addr_t dma_handle;		/* Handle for all bufs */
+	unsigned int num;		/* Sub-channel number */
+};
+
+/* Channel */
+struct rcar_drif_chan {
+	struct device *dev;		/* Platform device */
+	struct video_device vdev;	/* SDR device */
+	struct v4l2_device v4l2_dev;	/* V4l2 device */
+
+	/* Videobuf2 queue and queued buffers list */
+	struct vb2_queue vb_queue;
+	struct list_head queued_bufs;
+	spinlock_t queued_bufs_lock;	/* Protects queued_bufs */
+
+	struct mutex v4l2_mutex;	/* To serialize ioctls */
+	struct mutex vb_queue_mutex;	/* To serialize streaming ioctls */
+	struct v4l2_ctrl_handler ctrl_hdl;	/* SDR control handler */
+	struct v4l2_async_notifier notifier;	/* For subdev (tuner) */
+
+	/* Current V4L2 SDR format array index */
+	unsigned int fmt_idx;
+
+	/* Device tree SYNC properties */
+	u32 mdr1;
+
+	/* Internals */
+	struct rcar_drif_subchan *sch[2]; /* DRIFx0 and DRIFx1 */
+	unsigned long hw_schans_mask;	/* Enabled sub-channels per DT */
+	unsigned long cur_schans_mask;	/* Used sub-channels for an SDR FMT */
+	u32 num_hw_schans;		/* Num of DT enabled sub-channels */
+	u32 num_cur_schans;		/* Num of used sub-channels */
+	u32 num_hwbufs;			/* Num of DMA buffers */
+	u32 hwbuf_size;			/* Each DMA buffer size */
+	u32 produced;			/* Buffers produced by dev */
+};
+
+/* Allocate buffer context */
+static int rcar_drif_alloc_bufctxt(struct rcar_drif_chan *ch)
+{
+	struct rcar_drif_hwbuf *bufctx;
+	unsigned int i, idx;
+
+	for_each_rcar_drif_subchannel(i, &ch->cur_schans_mask) {
+		bufctx = kcalloc(ch->num_hwbufs, sizeof(*bufctx), GFP_KERNEL);
+		if (!bufctx)
+			return -ENOMEM;
+
+		for (idx = 0; idx < ch->num_hwbufs; idx++)
+			ch->sch[i]->buf[idx] = bufctx + idx;
+	}
+	return 0;
+}
+
+/* Release buffer context */
+static void rcar_drif_release_bufctxt(struct rcar_drif_chan *ch)
+{
+	unsigned int i;
+
+	for_each_rcar_drif_subchannel(i, &ch->cur_schans_mask) {
+		kfree(ch->sch[i]->buf[0]);
+		ch->sch[i]->buf[0] = NULL;
+	}
+}
+
+/* Release DMA channel */
+static void rcar_drif_release_dmachannel(struct rcar_drif_chan *ch)
+{
+	unsigned int i;
+
+	for_each_rcar_drif_subchannel(i, &ch->cur_schans_mask)
+		if (ch->sch[i]->dmach) {
+			dma_release_channel(ch->sch[i]->dmach);
+			ch->sch[i]->dmach = NULL;
+		}
+}
+
+/* Allocate DMA channel */
+static int rcar_drif_alloc_dmachannel(struct rcar_drif_chan *ch)
+{
+	struct dma_slave_config dma_cfg;
+	unsigned int i;
+	int ret = -ENODEV;
+
+	for_each_rcar_drif_subchannel(i, &ch->cur_schans_mask) {
+		struct rcar_drif_subchan *sch = ch->sch[i];
+
+		sch->dmach = dma_request_slave_channel(&sch->pdev->dev, "rx");
+		if (!sch->dmach) {
+			rdrif_err(ch, "schan%u: dma channel req failed\n", i);
+			goto dmach_error;
+		}
+
+		/* Configure slave */
+		memset(&dma_cfg, 0, sizeof(dma_cfg));
+		dma_cfg.src_addr = (phys_addr_t)(sch->start + RCAR_DRIF_SIRFDR);
+		dma_cfg.dst_addr = 0;
+		dma_cfg.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+		ret = dmaengine_slave_config(sch->dmach, &dma_cfg);
+		if (ret) {
+			rdrif_err(ch, "schan%u: dma slave config failed\n", i);
+			goto dmach_error;
+		}
+	}
+	return 0;
+
+dmach_error:
+	rcar_drif_release_dmachannel(ch);
+	return ret;
+}
+
+/* Release queued vb2 buffers */
+static void rcar_drif_release_queued_bufs(struct rcar_drif_chan *ch,
+					  enum vb2_buffer_state state)
+{
+	struct rcar_drif_frame_buf *fbuf, *tmp;
+	unsigned long flags;
+
+	spin_lock_irqsave(&ch->queued_bufs_lock, flags);
+	list_for_each_entry_safe(fbuf, tmp, &ch->queued_bufs, list) {
+		list_del(&fbuf->list);
+		vb2_buffer_done(&fbuf->vb.vb2_buf, state);
+	}
+	spin_unlock_irqrestore(&ch->queued_bufs_lock, flags);
+}
+
+/* Set MDR defaults */
+static inline void rcar_drif_set_mdr1(struct rcar_drif_chan *ch)
+{
+	unsigned int i;
+
+	/* Set defaults for both sub-channels */
+	for_each_rcar_drif_subchannel(i, &ch->cur_schans_mask) {
+		/* Refer MSIOF section in manual for this register setting */
+		writel(RCAR_DRIF_SITMDR1_PCON,
+		       ch->sch[i]->base + RCAR_DRIF_SITMDR1);
+
+		/* Setup MDR1 value */
+		writel(ch->mdr1, ch->sch[i]->base + RCAR_DRIF_SIRMDR1);
+
+		rdrif_dbg(2, ch, "schan%u: mdr1 = 0x%08x",
+			  i, readl(ch->sch[i]->base + RCAR_DRIF_SIRMDR1));
+	}
+}
+
+/* Extract bitlen and wdcnt from given word length */
+static int rcar_drif_convert_wdlen(struct rcar_drif_chan *ch,
+				   u32 wdlen, u32 *bitlen, u32 *wdcnt)
+{
+	unsigned int i, nr_wds;
+
+	/* FIFO register size is 32 bits */
+	for (i = 0; i < 32; i++) {
+		nr_wds = wdlen % (32 - i);
+		if (nr_wds == 0) {
+			*bitlen = (32 - i);
+			*wdcnt = (wdlen / *bitlen);
+			break;
+		}
+	}
+
+	/* Sanity check range */
+	if ((i == 32) || !((*bitlen >= 8) && (*bitlen <= 32)) ||
+	    !((*wdcnt >= 1) && (*wdcnt <= 64))) {
+		rdrif_err(ch, "invalid wdlen %u configured\n", wdlen);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+/* Set DRIF receive format */
+static int rcar_drif_set_format(struct rcar_drif_chan *ch)
+{
+	u32 bitlen, wdcnt, wdlen;
+	unsigned int i;
+	int ret = -EINVAL;
+
+	wdlen = formats[ch->fmt_idx].wdlen;
+	rdrif_dbg(2, ch, "setfmt: idx %u, wdlen %u, num_schans %u\n",
+		  ch->fmt_idx, wdlen, formats[ch->fmt_idx].num_schans);
+
+	/* Sanity check */
+	if (formats[ch->fmt_idx].num_schans > ch->num_cur_schans) {
+		rdrif_err(ch, "fmt idx %u current schans %u mismatch\n",
+			  ch->fmt_idx, ch->num_cur_schans);
+		return ret;
+	}
+
+	/* Get bitlen & wdcnt from wdlen */
+	ret = rcar_drif_convert_wdlen(ch, wdlen, &bitlen, &wdcnt);
+	if (ret)
+		return ret;
+
+	/* Setup group, bitlen & wdcnt */
+	for_each_rcar_drif_subchannel(i, &ch->cur_schans_mask) {
+		u32 mdr;
+
+		/* Two groups */
+		mdr = RCAR_DRIF_MDR_GRPCNT(2) | RCAR_DRIF_MDR_BITLEN(bitlen) |
+		       RCAR_DRIF_MDR_WDCNT(wdcnt);
+		writel(mdr, ch->sch[i]->base + RCAR_DRIF_SIRMDR2);
+
+		mdr = RCAR_DRIF_MDR_BITLEN(bitlen) | RCAR_DRIF_MDR_WDCNT(wdcnt);
+		writel(mdr, ch->sch[i]->base + RCAR_DRIF_SIRMDR3);
+
+		rdrif_dbg(2, ch, "schan%u: new mdr[2,3] = 0x%08x, 0x%08x\n",
+			  i, readl(ch->sch[i]->base + RCAR_DRIF_SIRMDR2),
+			  readl(ch->sch[i]->base + RCAR_DRIF_SIRMDR3));
+	}
+	return ret;
+}
+
+/* Release DMA buffers */
+static void rcar_drif_release_buf(struct rcar_drif_chan *ch)
+{
+	unsigned int i;
+
+	for_each_rcar_drif_subchannel(i, &ch->cur_schans_mask) {
+		struct rcar_drif_subchan *sch = ch->sch[i];
+
+		/* First entry of the sub-channel contains the dma buf ptr */
+		if (sch->buf[0] && sch->buf[0]->addr) {
+			dma_free_coherent(&sch->pdev->dev,
+					  ch->hwbuf_size * ch->num_hwbufs,
+					  sch->buf[0]->addr, sch->dma_handle);
+			sch->buf[0]->addr = NULL;
+		}
+	}
+}
+
+/* Request DMA buffers */
+static int rcar_drif_request_buf(struct rcar_drif_chan *ch)
+{
+	int ret = -ENOMEM;
+	unsigned int i, j;
+	void *addr;
+
+	for_each_rcar_drif_subchannel(i, &ch->cur_schans_mask) {
+		struct rcar_drif_subchan *sch = ch->sch[i];
+
+		/* Allocate DMA buffers */
+		addr = dma_alloc_coherent(&sch->pdev->dev,
+					  ch->hwbuf_size * ch->num_hwbufs,
+					  &sch->dma_handle, GFP_KERNEL);
+		if (!addr) {
+			rdrif_err(ch,
+			"schan%u: dma alloc failed. num_hwbufs %u size %u\n",
+			i, ch->num_hwbufs, ch->hwbuf_size);
+			goto alloc_error;
+		}
+
+		/* Split the chunk and populate bufctxt */
+		for (j = 0; j < ch->num_hwbufs; j++) {
+			sch->buf[j]->addr = addr + (j * ch->hwbuf_size);
+			sch->buf[j]->status = 0;
+		}
+	}
+
+	return 0;
+
+alloc_error:
+	return ret;
+}
+
+/* Setup vb_queue minimum buffer requirements */
+static int rcar_drif_queue_setup(struct vb2_queue *vq,
+			unsigned int *num_buffers, unsigned int *num_planes,
+			unsigned int sizes[], struct device *alloc_devs[])
+{
+	struct rcar_drif_chan *ch = vb2_get_drv_priv(vq);
+
+	/* Need at least 16 buffers */
+	if (vq->num_buffers + *num_buffers < 16)
+		*num_buffers = 16 - vq->num_buffers;
+
+	*num_planes = 1;
+	sizes[0] = PAGE_ALIGN(formats[ch->fmt_idx].buffersize);
+
+	rdrif_dbg(2, ch, "num_bufs %d sizes[0] %d\n", *num_buffers, sizes[0]);
+	return 0;
+}
+
+/* Enqueue buffer */
+static void rcar_drif_buf_queue(struct vb2_buffer *vb)
+{
+	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
+	struct rcar_drif_chan *ch = vb2_get_drv_priv(vb->vb2_queue);
+	struct rcar_drif_frame_buf *fbuf =
+			container_of(vbuf, struct rcar_drif_frame_buf, vb);
+	unsigned long flags;
+
+	rdrif_dbg(2, ch, "buf_queue idx %u\n", vb->index);
+	spin_lock_irqsave(&ch->queued_bufs_lock, flags);
+	list_add_tail(&fbuf->list, &ch->queued_bufs);
+	spin_unlock_irqrestore(&ch->queued_bufs_lock, flags);
+}
+
+/* Deliver buffer to user space */
+static void rcar_drif_deliver_buf(struct rcar_drif_subchan *sch, u32 idx)
+{
+	struct rcar_drif_chan *ch = sch->parent;
+	struct rcar_drif_frame_buf *fbuf;
+	unsigned long flags;
+	bool overflow = false;
+
+	spin_lock_irqsave(&ch->queued_bufs_lock, flags);
+	fbuf = list_first_entry_or_null(&ch->queued_bufs, struct
+					rcar_drif_frame_buf, list);
+	if (!fbuf) {
+		/*
+		 * App is late in enqueing buffers. Samples lost & there will
+		 * be a gap in sequence number when app recovers
+		 */
+		rdrif_dbg(1, ch, "\napp late: schan%u: prod %u\n",
+			  sch->num, ch->produced);
+		ch->produced++;	/* Increment the produced count anyway */
+		spin_unlock_irqrestore(&ch->queued_bufs_lock, flags);
+		return;
+	}
+	list_del(&fbuf->list);
+	spin_unlock_irqrestore(&ch->queued_bufs_lock, flags);
+
+	if (ch->num_cur_schans == RCAR_DRIF_MAX_SUBCHANS) {
+		struct rcar_drif_hwbuf *buf_i, *buf_q;
+
+		if (sch->num) {
+			buf_i = to_rcar_drif_buf_pair(ch, sch->num, idx);
+			buf_q = sch->buf[idx];
+		} else {
+			buf_i = sch->buf[idx];
+			buf_q = to_rcar_drif_buf_pair(ch, sch->num, idx);
+		}
+		memcpy(vb2_plane_vaddr(&fbuf->vb.vb2_buf, 0),
+		       buf_i->addr, ch->hwbuf_size);
+		memcpy(vb2_plane_vaddr(&fbuf->vb.vb2_buf, 0) + ch->hwbuf_size,
+		       buf_q->addr, ch->hwbuf_size);
+
+		if ((buf_i->status | buf_q->status) & RCAR_DRIF_BUF_OVERFLOW) {
+			overflow = true;
+			/* Clear the flag in status */
+			buf_i->status &= ~RCAR_DRIF_BUF_OVERFLOW;
+			buf_q->status &= ~RCAR_DRIF_BUF_OVERFLOW;
+		}
+	} else {
+		struct rcar_drif_hwbuf *buf_iq = sch->buf[idx];
+
+		memcpy(vb2_plane_vaddr(&fbuf->vb.vb2_buf, 0),
+		       buf_iq->addr, ch->hwbuf_size);
+
+		if (buf_iq->status & RCAR_DRIF_BUF_OVERFLOW) {
+			overflow = true;
+			/* Clear the flag in status */
+			buf_iq->status &= ~RCAR_DRIF_BUF_OVERFLOW;
+		}
+	}
+
+	rdrif_dbg(2, ch, "schan%u: prod %u\n", sch->num, ch->produced);
+
+	fbuf->vb.field = V4L2_FIELD_NONE;
+	fbuf->vb.sequence = ch->produced++;
+	fbuf->vb.vb2_buf.timestamp = ktime_get_ns();
+	vb2_set_plane_payload(&fbuf->vb.vb2_buf, 0,
+			      formats[ch->fmt_idx].buffersize);
+
+	/* Set error state on overflow */
+	if (overflow)
+		vb2_buffer_done(&fbuf->vb.vb2_buf, VB2_BUF_STATE_ERROR);
+	else
+		vb2_buffer_done(&fbuf->vb.vb2_buf, VB2_BUF_STATE_DONE);
+}
+
+static inline bool rcar_drif_buf_pairs_done(struct rcar_drif_hwbuf *buf1,
+					    struct rcar_drif_hwbuf *buf2)
+{
+	return (buf1->status & buf2->status & RCAR_DRIF_BUF_DONE);
+}
+
+/* Check if DMA of the paired sub-channel is also complete */
+static void rcar_drif_check_dmapair(struct rcar_drif_subchan *sch, u32 idx)
+{
+	struct rcar_drif_chan *ch = sch->parent;
+	struct rcar_drif_hwbuf *buf, *pair;
+
+	buf = sch->buf[idx];
+	pair = to_rcar_drif_buf_pair(ch, sch->num, idx);
+	buf->status |= RCAR_DRIF_BUF_DONE;
+
+	rdrif_dbg(2, ch, "schan%u: status: buf %u pair %u\n",
+		  sch->num, buf->status, pair->status);
+
+	/* Check if both DMA buffers are done */
+	if (rcar_drif_buf_pairs_done(buf, pair)) {
+		/* Clear status flag */
+		buf->status &= ~RCAR_DRIF_BUF_DONE;
+		pair->status &= ~RCAR_DRIF_BUF_DONE;
+
+		/* Deliver buffer to user */
+		rcar_drif_deliver_buf(sch, idx);
+	}
+}
+
+/* DMA callback for each stage */
+static void rcar_drif_dma_complete(void *dma_async_param)
+{
+	struct rcar_drif_subchan *sch =
+		(struct rcar_drif_subchan *)dma_async_param;
+	struct rcar_drif_chan *ch = sch->parent;
+	struct rcar_drif_hwbuf *buf;
+	u32 str, idx;
+
+	mutex_lock(&ch->vb_queue_mutex);
+
+	/* DMA can be terminated while the callback was waiting on lock */
+	if (!vb2_is_streaming(&ch->vb_queue)) {
+		mutex_unlock(&ch->vb_queue_mutex);
+		return;
+	}
+
+	idx = ch->produced % ch->num_hwbufs;
+	buf = sch->buf[idx];
+
+	/* Check for DRIF errors */
+	str = readl(sch->base + RCAR_DRIF_SISTR);
+	if (unlikely(str & RCAR_DRIF_RFOVF)) {
+		/* Writing the same clears it */
+		writel(str, sch->base + RCAR_DRIF_SISTR);
+
+		/* Overflow: some samples are lost */
+		buf->status |= RCAR_DRIF_BUF_OVERFLOW;
+		rdrif_dbg(1, ch, "\nfifo overflow: schan%u: prod %d\n",
+			  sch->num, ch->produced);
+	}
+
+	/* Check if the rx'ed data can be delivered to user */
+	if (ch->num_cur_schans == RCAR_DRIF_MAX_SUBCHANS)
+		rcar_drif_check_dmapair(sch, idx);
+	else
+		rcar_drif_deliver_buf(sch, idx);
+
+	mutex_unlock(&ch->vb_queue_mutex);
+}
+
+static int rcar_drif_qbuf(struct rcar_drif_subchan *sch)
+{
+	struct rcar_drif_chan *ch = sch->parent;
+	dma_addr_t addr = sch->dma_handle;
+	struct dma_async_tx_descriptor *rxd;
+	dma_cookie_t cookie;
+	int ret = -EIO;
+
+	/* Setup cyclic DMA with given buffers */
+	rxd = dmaengine_prep_dma_cyclic(sch->dmach, addr,
+					ch->hwbuf_size * ch->num_hwbufs,
+					ch->hwbuf_size, DMA_DEV_TO_MEM,
+					DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+	if (!rxd) {
+		rdrif_err(ch, "schan%u: prep dma cyclic failed\n", sch->num);
+		return ret;
+	}
+
+	/* Submit descriptor */
+	rxd->callback = rcar_drif_dma_complete;
+	rxd->callback_param = sch;
+	cookie = dmaengine_submit(rxd);
+	if (dma_submit_error(cookie)) {
+		rdrif_err(ch, "schan%u: dma submit failed\n", sch->num);
+		return ret;
+	}
+
+	dma_async_issue_pending(sch->dmach);
+	return 0;
+}
+
+/* Enable reception */
+static int rcar_drif_enable_rx(struct rcar_drif_chan *ch)
+{
+	unsigned int i;
+	u32 ctr;
+	int ret;
+
+	/*
+	 * When both sub-channels are enabled, they can be syncronized only by
+	 * the master
+	 */
+
+	/* Enable receive */
+	for_each_rcar_drif_subchannel(i, &ch->cur_schans_mask) {
+		ctr = readl(ch->sch[i]->base + RCAR_DRIF_SICTR);
+		ctr |= (RCAR_DRIF_SICTR_RX_RISING_EDGE |
+			 RCAR_DRIF_SICTR_RX_EN);
+		writel(ctr, ch->sch[i]->base + RCAR_DRIF_SICTR);
+	}
+
+	/* Check receive enabled */
+	for_each_rcar_drif_subchannel(i, &ch->cur_schans_mask) {
+		ret = readl_poll_timeout(ch->sch[i]->base + RCAR_DRIF_SICTR,
+					 ctr, ctr & RCAR_DRIF_SICTR_RX_EN,
+					 2, 500000);
+		if (ret) {
+			rdrif_err(ch, "schan%u: rx en failed. ctr 0x%08x\n",
+				  i, readl(ch->sch[i]->base + RCAR_DRIF_SICTR));
+			break;
+		}
+	}
+	return ret;
+}
+
+/* Disable reception */
+static void rcar_drif_disable_rx(struct rcar_drif_chan *ch)
+{
+	unsigned int i;
+	u32 ctr;
+	int ret;
+
+	/* Disable receive */
+	for_each_rcar_drif_subchannel(i, &ch->cur_schans_mask) {
+		ctr = readl(ch->sch[i]->base + RCAR_DRIF_SICTR);
+		ctr &= ~RCAR_DRIF_SICTR_RX_EN;
+		writel(ctr, ch->sch[i]->base + RCAR_DRIF_SICTR);
+	}
+
+	/* Check receive disabled */
+	for_each_rcar_drif_subchannel(i, &ch->cur_schans_mask) {
+		ret = readl_poll_timeout(ch->sch[i]->base + RCAR_DRIF_SICTR,
+					 ctr, !(ctr & RCAR_DRIF_SICTR_RX_EN),
+					 2, 500000);
+		if (ret)
+			dev_warn(&ch->vdev.dev,
+			"schan%u: failed to disable rx. ctr 0x%08x\n",
+			i, readl(ch->sch[i]->base + RCAR_DRIF_SICTR));
+	}
+}
+
+/* Start sub-channel */
+static int rcar_drif_start_subchan(struct rcar_drif_subchan *sch)
+{
+	struct rcar_drif_chan *ch = sch->parent;
+	u32 ctr, str;
+	int ret;
+
+	/* Reset receive */
+	writel(RCAR_DRIF_SICTR_RESET, sch->base + RCAR_DRIF_SICTR);
+	ret = readl_poll_timeout(sch->base + RCAR_DRIF_SICTR,
+					 ctr, !(ctr & RCAR_DRIF_SICTR_RESET),
+					 2, 500000);
+	if (ret) {
+		rdrif_err(ch, "schan%u: failed to reset rx. ctr 0x%08x\n",
+			  sch->num, readl(sch->base + RCAR_DRIF_SICTR));
+		return ret;
+	}
+
+	/* Queue buffers for DMA */
+	ret = rcar_drif_qbuf(sch);
+	if (ret)
+		return ret;
+
+	/* Clear status register flags */
+	str = RCAR_DRIF_RFFUL | RCAR_DRIF_REOF | RCAR_DRIF_RFSERR |
+		RCAR_DRIF_RFUDF | RCAR_DRIF_RFOVF;
+	writel(str, sch->base + RCAR_DRIF_SISTR);
+
+	/* Enable DMA receive interrupt */
+	writel(0x00009000, sch->base + RCAR_DRIF_SIIER);
+
+	return ret;
+}
+
+/* Start receive operation */
+static int rcar_drif_start(struct rcar_drif_chan *ch)
+{
+	unsigned int i;
+	int ret;
+
+	for_each_rcar_drif_subchannel(i, &ch->cur_schans_mask) {
+		ret = rcar_drif_start_subchan(ch->sch[i]);
+		if (ret)
+			goto start_error;
+	}
+
+	ch->produced = 0;
+
+	ret = rcar_drif_enable_rx(ch);
+	if (ret)
+		goto start_error;
+
+	rdrif_dbg(1, ch, "started\n");
+
+start_error:
+	return ret;
+}
+
+/* Stop sub-channel */
+static void rcar_drif_stop_subchan(struct rcar_drif_subchan *sch)
+{
+	struct rcar_drif_chan *ch = sch->parent;
+	int ret, retries = 3;
+
+	/* Disable DMA receive interrupt */
+	writel(0x00000000, sch->base + RCAR_DRIF_SIIER);
+
+	do {
+		/* Terminate all DMA transfers */
+		ret = dmaengine_terminate_sync(sch->dmach);
+		if (!ret)
+			break;
+		rdrif_dbg(2, ch, "stop retry\n");
+	} while (--retries);
+
+	WARN_ON(!retries);
+}
+
+/* Stop receive operation */
+static void rcar_drif_stop(struct rcar_drif_chan *ch)
+{
+	unsigned int i;
+
+	/* Disable Rx */
+	rcar_drif_disable_rx(ch);
+
+	for_each_rcar_drif_subchannel(i, &ch->cur_schans_mask)
+		rcar_drif_stop_subchan(ch->sch[i]);
+
+	rdrif_dbg(1, ch, "stopped: prod %u\n", ch->produced);
+}
+
+/* Start streaming */
+static int rcar_drif_start_streaming(struct vb2_queue *vq, unsigned int count)
+{
+	struct rcar_drif_chan *ch = vb2_get_drv_priv(vq);
+	unsigned int i, j;
+	int ret;
+
+	rdrif_dbg(1, ch, "start streaming\n");
+	mutex_lock(&ch->v4l2_mutex);
+
+	for_each_rcar_drif_subchannel(i, &ch->cur_schans_mask) {
+		ret = clk_prepare_enable(ch->sch[i]->clkp);
+		if (ret)
+			goto start_error;
+	}
+
+	/* Set default MDRx settings */
+	rcar_drif_set_mdr1(ch);
+
+	/* Set new format */
+	ret = rcar_drif_set_format(ch);
+	if (ret)
+		goto start_error;
+
+	if (ch->num_cur_schans == RCAR_DRIF_MAX_SUBCHANS)
+		ch->hwbuf_size = formats[ch->fmt_idx].buffersize/2;
+	else
+		ch->hwbuf_size = formats[ch->fmt_idx].buffersize;
+
+	rdrif_dbg(1, ch, "num_hwbufs %u, hwbuf_size %u\n",
+		ch->num_hwbufs, ch->hwbuf_size);
+
+	/* Alloc DMA channel */
+	ret = rcar_drif_alloc_dmachannel(ch);
+	if (ret)
+		goto start_error;
+
+	/* Alloc buf context */
+	ret = rcar_drif_alloc_bufctxt(ch);
+	if (ret)
+		goto start_error;
+
+	/* Request buffers */
+	ret = rcar_drif_request_buf(ch);
+	if (ret)
+		goto start_error;
+
+	/* Start Rx */
+	ret = rcar_drif_start(ch);
+	if (ret)
+		goto start_error;
+
+	mutex_unlock(&ch->v4l2_mutex);
+	return ret;
+
+start_error:
+	rcar_drif_release_queued_bufs(ch, VB2_BUF_STATE_QUEUED);
+	rcar_drif_release_buf(ch);
+	rcar_drif_release_bufctxt(ch);
+	rcar_drif_release_dmachannel(ch);
+	for (j = 0; j < i; j++)
+		clk_disable_unprepare(ch->sch[j]->clkp);
+
+	mutex_unlock(&ch->v4l2_mutex);
+	return ret;
+}
+
+/* Stop streaming */
+static void rcar_drif_stop_streaming(struct vb2_queue *vq)
+{
+	struct rcar_drif_chan *ch = vb2_get_drv_priv(vq);
+	unsigned int i;
+
+	mutex_lock(&ch->v4l2_mutex);
+
+	/* Stop hardware streaming */
+	rcar_drif_stop(ch);
+
+	/* Return all queued buffers to vb2 */
+	rcar_drif_release_queued_bufs(ch, VB2_BUF_STATE_ERROR);
+
+	/* Release buf & buf context */
+	rcar_drif_release_buf(ch);
+	rcar_drif_release_bufctxt(ch);
+
+	/* Release DMA channel resources */
+	rcar_drif_release_dmachannel(ch);
+
+	for (i = 0; i < RCAR_DRIF_MAX_SUBCHANS; i++)
+		clk_disable_unprepare(ch->sch[i]->clkp);
+
+	mutex_unlock(&ch->v4l2_mutex);
+	rdrif_dbg(1, ch, "stopped streaming\n");
+}
+
+/* Vb2 ops */
+static struct vb2_ops rcar_drif_vb2_ops = {
+	.queue_setup            = rcar_drif_queue_setup,
+	.buf_queue              = rcar_drif_buf_queue,
+	.start_streaming        = rcar_drif_start_streaming,
+	.stop_streaming         = rcar_drif_stop_streaming,
+	.wait_prepare		= vb2_ops_wait_prepare,
+	.wait_finish		= vb2_ops_wait_finish,
+};
+
+static int rcar_drif_querycap(struct file *file, void *fh,
+			      struct v4l2_capability *cap)
+{
+	struct rcar_drif_chan *ch = video_drvdata(file);
+
+	strlcpy(cap->driver, KBUILD_MODNAME, sizeof(cap->driver));
+	strlcpy(cap->card, ch->vdev.name, sizeof(cap->card));
+	cap->device_caps = V4L2_CAP_SDR_CAPTURE | V4L2_CAP_TUNER |
+				   V4L2_CAP_STREAMING | V4L2_CAP_READWRITE;
+	cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
+	snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:%s",
+		 ch->vdev.name);
+	return 0;
+}
+
+static int rcar_drif_set_default_format(struct rcar_drif_chan *ch)
+{
+	unsigned int i;
+
+	for (i = 0; i < NUM_FORMATS; i++) {
+		/* Find any matching fmt and set it as default */
+		if (ch->num_hw_schans == formats[i].num_schans) {
+			ch->fmt_idx = i;
+			ch->cur_schans_mask = ch->hw_schans_mask;
+			ch->num_cur_schans = ch->num_hw_schans;
+			dev_dbg(ch->dev, "default fmt[%u]: mask %lu num %u\n",
+				i, ch->cur_schans_mask, ch->num_cur_schans);
+			return 0;
+		}
+	}
+	dev_err(ch->dev, "no matching sdr fmt found\n");
+	return -EINVAL;
+}
+
+static int rcar_drif_enum_fmt_sdr_cap(struct file *file, void *priv,
+				      struct v4l2_fmtdesc *f)
+{
+	if (f->index >= NUM_FORMATS)
+		return -EINVAL;
+
+	strlcpy(f->description, formats[f->index].name, sizeof(f->description));
+	f->pixelformat = formats[f->index].pixelformat;
+	return 0;
+}
+
+static int rcar_drif_g_fmt_sdr_cap(struct file *file, void *priv,
+				   struct v4l2_format *f)
+{
+	struct rcar_drif_chan *ch = video_drvdata(file);
+
+	f->fmt.sdr.pixelformat = formats[ch->fmt_idx].pixelformat;
+	f->fmt.sdr.buffersize = formats[ch->fmt_idx].buffersize;
+	memset(f->fmt.sdr.reserved, 0, sizeof(f->fmt.sdr.reserved));
+	return 0;
+}
+
+static int rcar_drif_s_fmt_sdr_cap(struct file *file, void *priv,
+				   struct v4l2_format *f)
+{
+	struct rcar_drif_chan *ch = video_drvdata(file);
+	struct vb2_queue *q = &ch->vb_queue;
+	unsigned int i;
+
+	if (vb2_is_busy(q))
+		return -EBUSY;
+
+	memset(f->fmt.sdr.reserved, 0, sizeof(f->fmt.sdr.reserved));
+	for (i = 0; i < NUM_FORMATS; i++) {
+		if (formats[i].pixelformat == f->fmt.sdr.pixelformat) {
+			ch->fmt_idx  = i;
+			f->fmt.sdr.buffersize = formats[i].buffersize;
+
+			/*
+			 * If a format demands one sub-channel only out of two
+			 * enabled sub-channels then pick the 0th sub-channel
+			 */
+			if (formats[i].num_schans < ch->num_hw_schans) {
+				ch->cur_schans_mask = BIT(0);	/* Enable D0 */
+				ch->num_cur_schans = formats[i].num_schans;
+			} else {
+				ch->cur_schans_mask = ch->hw_schans_mask;
+				ch->num_cur_schans = ch->num_hw_schans;
+			}
+
+			rdrif_dbg(1, ch, "cur: idx %u mask %lu num %u\n",
+				  i, ch->cur_schans_mask, ch->num_cur_schans);
+			return 0;
+		}
+	}
+
+	if (rcar_drif_set_default_format(ch))
+		return -EINVAL;
+
+	f->fmt.sdr.pixelformat = formats[ch->fmt_idx].pixelformat;
+	f->fmt.sdr.buffersize = formats[ch->fmt_idx].buffersize;
+	return 0;
+}
+
+static int rcar_drif_try_fmt_sdr_cap(struct file *file, void *priv,
+				     struct v4l2_format *f)
+{
+	struct rcar_drif_chan *ch = video_drvdata(file);
+	unsigned int i;
+
+	memset(f->fmt.sdr.reserved, 0, sizeof(f->fmt.sdr.reserved));
+	for (i = 0; i < NUM_FORMATS; i++) {
+		if (formats[i].pixelformat == f->fmt.sdr.pixelformat) {
+			f->fmt.sdr.buffersize = formats[i].buffersize;
+			return 0;
+		}
+	}
+
+	f->fmt.sdr.pixelformat = formats[ch->fmt_idx].pixelformat;
+	f->fmt.sdr.buffersize = formats[ch->fmt_idx].buffersize;
+	return 0;
+}
+
+/* Tuner subdev ioctls */
+static int rcar_drif_enum_freq_bands(struct file *file, void *priv,
+				     struct v4l2_frequency_band *band)
+{
+	struct rcar_drif_chan *ch = video_drvdata(file);
+	struct v4l2_subdev *sd, *tmp;
+	int ret = 0;
+
+	for_each_rcar_drif_subdev(sd, tmp, ch) {
+		ret = v4l2_subdev_call(sd, tuner, enum_freq_bands, band);
+		if (ret)
+			break;
+	}
+	return ret;
+}
+
+static int rcar_drif_g_frequency(struct file *file, void *priv,
+				 struct v4l2_frequency *f)
+{
+	struct rcar_drif_chan *ch = video_drvdata(file);
+	struct v4l2_subdev *sd, *tmp;
+	int ret = 0;
+
+	for_each_rcar_drif_subdev(sd, tmp, ch) {
+		ret = v4l2_subdev_call(sd, tuner, g_frequency, f);
+		if (ret)
+			break;
+	}
+	return ret;
+}
+
+static int rcar_drif_s_frequency(struct file *file, void *priv,
+				 const struct v4l2_frequency *f)
+{
+	struct rcar_drif_chan *ch = video_drvdata(file);
+	struct v4l2_subdev *sd, *tmp;
+	int ret = 0;
+
+	for_each_rcar_drif_subdev(sd, tmp, ch) {
+		ret = v4l2_subdev_call(sd, tuner, s_frequency, f);
+		if (ret)
+			break;
+	}
+	return ret;
+}
+
+static int rcar_drif_g_tuner(struct file *file, void *priv,
+			     struct v4l2_tuner *vt)
+{
+	struct rcar_drif_chan *ch = video_drvdata(file);
+	struct v4l2_subdev *sd, *tmp;
+	int ret = 0;
+
+	for_each_rcar_drif_subdev(sd, tmp, ch) {
+		ret = v4l2_subdev_call(sd, tuner, g_tuner, vt);
+		if (ret)
+			break;
+	}
+	return ret;
+}
+
+static int rcar_drif_s_tuner(struct file *file, void *priv,
+			     const struct v4l2_tuner *vt)
+{
+	struct rcar_drif_chan *ch = video_drvdata(file);
+	struct v4l2_subdev *sd, *tmp;
+	int ret = 0;
+
+	for_each_rcar_drif_subdev(sd, tmp, ch) {
+		ret = v4l2_subdev_call(sd, tuner, s_tuner, vt);
+		if (ret)
+			break;
+	}
+	return ret;
+}
+
+static const struct v4l2_ioctl_ops rcar_drif_ioctl_ops = {
+	.vidioc_querycap          = rcar_drif_querycap,
+
+	.vidioc_enum_fmt_sdr_cap  = rcar_drif_enum_fmt_sdr_cap,
+	.vidioc_g_fmt_sdr_cap     = rcar_drif_g_fmt_sdr_cap,
+	.vidioc_s_fmt_sdr_cap     = rcar_drif_s_fmt_sdr_cap,
+	.vidioc_try_fmt_sdr_cap   = rcar_drif_try_fmt_sdr_cap,
+
+	.vidioc_reqbufs           = vb2_ioctl_reqbufs,
+	.vidioc_create_bufs       = vb2_ioctl_create_bufs,
+	.vidioc_prepare_buf       = vb2_ioctl_prepare_buf,
+	.vidioc_querybuf          = vb2_ioctl_querybuf,
+	.vidioc_qbuf              = vb2_ioctl_qbuf,
+	.vidioc_dqbuf             = vb2_ioctl_dqbuf,
+
+	.vidioc_streamon          = vb2_ioctl_streamon,
+	.vidioc_streamoff         = vb2_ioctl_streamoff,
+
+	.vidioc_s_frequency       = rcar_drif_s_frequency,
+	.vidioc_g_frequency       = rcar_drif_g_frequency,
+	.vidioc_s_tuner		  = rcar_drif_s_tuner,
+	.vidioc_g_tuner		  = rcar_drif_g_tuner,
+	.vidioc_enum_freq_bands   = rcar_drif_enum_freq_bands,
+	.vidioc_subscribe_event   = v4l2_ctrl_subscribe_event,
+	.vidioc_unsubscribe_event = v4l2_event_unsubscribe,
+	.vidioc_log_status        = v4l2_ctrl_log_status,
+};
+
+static const struct v4l2_file_operations rcar_drif_fops = {
+	.owner                    = THIS_MODULE,
+	.open                     = v4l2_fh_open,
+	.release                  = vb2_fop_release,
+	.read                     = vb2_fop_read,
+	.poll                     = vb2_fop_poll,
+	.mmap                     = vb2_fop_mmap,
+	.unlocked_ioctl           = video_ioctl2,
+};
+
+static struct video_device rcar_drif_vdev = {
+	.name                     = "R-Car DRIF",
+	.release                  = video_device_release_empty,
+	.fops                     = &rcar_drif_fops,
+	.ioctl_ops                = &rcar_drif_ioctl_ops,
+};
+
+static int rcar_drif_notify_bound(struct v4l2_async_notifier *notifier,
+				   struct v4l2_subdev *subdev,
+				   struct v4l2_async_subdev *asd)
+{
+	struct rcar_drif_chan *ch =
+		container_of(notifier, struct rcar_drif_chan, notifier);
+
+	/* Nothing to do at this point */
+	rdrif_dbg(2, ch, "bound asd: %s\n", asd->match.of.node->name);
+	return 0;
+}
+
+/* Sub-device registered notification callback */
+static int rcar_drif_notify_complete(struct v4l2_async_notifier *notifier)
+{
+	struct rcar_drif_chan *ch =
+		container_of(notifier, struct rcar_drif_chan, notifier);
+	struct v4l2_subdev *sd, *tmp;
+	int ret;
+
+	v4l2_ctrl_handler_init(&ch->ctrl_hdl, 10);
+	ch->v4l2_dev.ctrl_handler = &ch->ctrl_hdl;
+
+	ret = v4l2_device_register_subdev_nodes(&ch->v4l2_dev);
+	if (ret) {
+		rdrif_err(ch, "failed register subdev nodes ret %d\n", ret);
+		return ret;
+	}
+
+	for_each_rcar_drif_subdev(sd, tmp, ch) {
+		ret = v4l2_ctrl_add_handler(ch->v4l2_dev.ctrl_handler,
+					    sd->ctrl_handler, NULL);
+		if (ret) {
+			rdrif_err(ch, "failed ctrl add hdlr ret %d\n", ret);
+			return ret;
+		}
+	}
+	rdrif_dbg(2, ch, "notify complete\n");
+	return 0;
+}
+
+/* Parse sub-devs (tuner) to find a matching device */
+static int rcar_drif_parse_subdevs(struct device *dev,
+				   struct v4l2_async_notifier *notifier)
+{
+	struct device_node *node = NULL;
+
+	notifier->subdevs = devm_kzalloc(dev, sizeof(*notifier->subdevs),
+					 GFP_KERNEL);
+	if (!notifier->subdevs)
+		return -ENOMEM;
+
+	node = of_graph_get_next_endpoint(dev->of_node, node);
+	if (node) {
+		struct rcar_drif_async_subdev *rsd;
+
+		rsd = devm_kzalloc(dev, sizeof(*rsd), GFP_KERNEL);
+		if (!rsd) {
+			of_node_put(node);
+			return -ENOMEM;
+		}
+
+		notifier->subdevs[notifier->num_subdevs] = &rsd->asd;
+		rsd->asd.match.of.node = of_graph_get_remote_port_parent(node);
+		of_node_put(node);
+		if (!rsd->asd.match.of.node) {
+			dev_warn(dev, "bad remote port parent\n");
+			return -EINVAL;
+		}
+
+		rsd->asd.match_type = V4L2_ASYNC_MATCH_OF;
+		notifier->num_subdevs++;
+	}
+	return 0;
+}
+
+/* SIRMDR1 configuration */
+static int rcar_drif_validate_syncmd(struct rcar_drif_chan *ch, u32 val)
+{
+	if (val > 1) {
+		dev_err(ch->dev, "invalid syncmd %u using L/R mode\n", val);
+		return -EINVAL;
+	}
+
+	ch->mdr1 &= ~(3 << 28);	/* Clear current settings */
+	if (val == 0)
+		ch->mdr1 |= RCAR_DRIF_SIRMDR1_SYNCMD_FRAME;
+	else
+		ch->mdr1 |= RCAR_DRIF_SIRMDR1_SYNCMD_LR;
+	return 0;
+}
+
+/* Get the dtdl or syncdl bits as in MSIOF */
+static u32 rcar_drif_get_dtdl_or_syncdl_bits(u32 dtdl_or_syncdl)
+{
+	/*
+	 * DTDL/SYNCDL bit	: dtdl/syncdl
+	 * b'000		: 0
+	 * b'001		: 100
+	 * b'010		: 200
+	 * b'011 (SYNCDL only)	: 300
+	 * b'101		: 50
+	 * b'110		: 150
+	 */
+	if (dtdl_or_syncdl % 100)
+		return dtdl_or_syncdl / 100 + 5;
+	else
+		return dtdl_or_syncdl / 100;
+}
+
+static int rcar_drif_validate_dtdl_syncdl(struct rcar_drif_chan *ch)
+{
+	struct device_node *np = ch->dev->of_node;
+	u32 dtdl = 100, syncdl = 0;
+
+	ch->mdr1 |= RCAR_DRIF_SIRMDR1_DTDL_1 | RCAR_DRIF_SIRMDR1_SYNCDL_0;
+	of_property_read_u32(np, "renesas,dtdl", &dtdl);
+	of_property_read_u32(np, "renesas,syncdl", &syncdl);
+
+	/* Sanity checks */
+	if (dtdl > 200 || syncdl > 300) {
+		dev_err(ch->dev, "invalid dtdl %u/syncdl %u\n", dtdl, syncdl);
+		return -EINVAL;
+	}
+	if ((dtdl + syncdl) % 100) {
+		dev_err(ch->dev, "sum of dtdl %u & syncdl %u not OK\n",
+			dtdl, syncdl);
+		return -EINVAL;
+	}
+	ch->mdr1 &= ~(7 << 20) & ~(7 << 16);	/* Clear current settings */
+	ch->mdr1 |= rcar_drif_get_dtdl_or_syncdl_bits(dtdl) << 20;
+	ch->mdr1 |= rcar_drif_get_dtdl_or_syncdl_bits(syncdl) << 16;
+	return 0;
+}
+
+static int rcar_drif_parse_properties(struct rcar_drif_chan *ch)
+{
+	struct device_node *np = ch->dev->of_node;
+	u32 syncmd;
+	int ret;
+
+	/* Set the defaults and check for overrides */
+	ch->mdr1 = RCAR_DRIF_SIRMDR1_SYNCMD_LR;
+	if (!of_property_read_u32(np, "renesas,syncmd", &syncmd)) {
+		ret = rcar_drif_validate_syncmd(ch, syncmd);
+		if (ret)
+			return ret;
+	}
+
+	if (of_find_property(np, "renesas,lsb-first", NULL))
+		ch->mdr1 |= RCAR_DRIF_SIRMDR1_LSB_FIRST;
+	else
+		ch->mdr1 |= RCAR_DRIF_SIRMDR1_MSB_FIRST;
+
+	if (of_find_property(np, "renesas,syncac-pol-high", NULL))
+		ch->mdr1 |= RCAR_DRIF_SIRMDR1_SYNCAC_POL_HIGH;
+	else
+		ch->mdr1 |= RCAR_DRIF_SIRMDR1_SYNCAC_POL_LOW;
+
+	return rcar_drif_validate_dtdl_syncdl(ch);
+}
+
+static u32 rcar_drif_enum_sub_channels(struct platform_device *pdev,
+					struct platform_device *s_pdev[])
+{
+	struct device_node *s_np;
+	u32 hw_schans_mask = 0;
+	unsigned int i;
+
+	for (i = 0; i < RCAR_DRIF_MAX_SUBCHANS; i++) {
+		s_np = of_parse_phandle(pdev->dev.of_node, "sub-channels", i);
+		if (s_np && of_device_is_available(s_np)) {
+			s_pdev[i] = of_find_device_by_node(s_np);
+			if (s_pdev[i]) {
+				hw_schans_mask |= BIT(i);
+				dev_dbg(&s_pdev[i]->dev, "schan%u ok\n", i);
+			}
+		}
+	}
+	return hw_schans_mask;
+}
+
+static int rcar_drif_probe(struct platform_device *pdev)
+{
+	struct platform_device *s_pdev[RCAR_DRIF_MAX_SUBCHANS];
+	unsigned long hw_schans_mask;
+	struct rcar_drif_chan *ch;
+	unsigned int i;
+	int ret;
+
+	/*
+	 * Sub-channel resources are managed by the parent channel instance.
+	 * The sub-channel instance helps only in registering with power domain
+	 * to aid in run-time pm support
+	 */
+	if (!of_find_property(pdev->dev.of_node, "sub-channels", NULL))
+		return 0;
+
+	/* Parent channel instance */
+	hw_schans_mask = rcar_drif_enum_sub_channels(pdev, s_pdev);
+	if (!hw_schans_mask) {
+		dev_err(&pdev->dev, "no sub-channels enabled\n");
+		return -ENODEV;
+	}
+
+
+	/* Reserve memory for driver structure */
+	ch = devm_kzalloc(&pdev->dev, sizeof(*ch), GFP_KERNEL);
+	if (!ch) {
+		ret = PTR_ERR(ch);
+		dev_err(&pdev->dev, "failed alloc drif context\n");
+		return ret;
+	}
+	ch->dev = &pdev->dev;
+
+	/* Parse device tree optional properties */
+	ret = rcar_drif_parse_properties(ch);
+	if (ret)
+		return ret;
+
+	dev_dbg(ch->dev, "parsed mdr1 0x%08x\n", ch->mdr1);
+
+	/* Setup enabled sub-channels */
+	for_each_rcar_drif_subchannel(i, &hw_schans_mask) {
+		struct clk *clkp;
+		struct resource	*res;
+		void __iomem *base;
+
+		/* Peripheral clock */
+		clkp = devm_clk_get(&s_pdev[i]->dev, "fck");
+		if (IS_ERR(clkp)) {
+			ret = PTR_ERR(clkp);
+			dev_err(&s_pdev[i]->dev, "clk get failed (%d)\n", ret);
+			return ret;
+		}
+
+		/* Register map */
+		res = platform_get_resource(s_pdev[i], IORESOURCE_MEM, 0);
+		base = devm_ioremap_resource(&s_pdev[i]->dev, res);
+		if (IS_ERR(base)) {
+			ret = PTR_ERR(base);
+			dev_err(&s_pdev[i]->dev, "ioremap failed (%d)\n", ret);
+			return ret;
+		}
+
+		/* Reserve memory for enabled sub-channel */
+		ch->sch[i] = devm_kzalloc(&pdev->dev, sizeof(*ch->sch[i]),
+					  GFP_KERNEL);
+		if (!ch->sch[i]) {
+			ret = PTR_ERR(ch);
+			dev_err(&s_pdev[i]->dev, "failed alloc sub-channel\n");
+			return ret;
+		}
+		ch->sch[i]->pdev = s_pdev[i];
+		ch->sch[i]->clkp = clkp;
+		ch->sch[i]->base = base;
+		ch->sch[i]->num = i;
+		ch->sch[i]->start = res->start;
+		ch->sch[i]->parent = ch;
+		ch->num_hw_schans++;
+	}
+	ch->hw_schans_mask = hw_schans_mask;
+
+	/* Validate any supported format for enabled sub-channels */
+	ret = rcar_drif_set_default_format(ch);
+	if (ret)
+		return ret;
+
+	/* Set defaults */
+	ch->num_hwbufs = RCAR_DRIF_DEFAULT_NUM_HWBUFS;
+	ch->hwbuf_size = RCAR_DRIF_DEFAULT_HWBUF_SIZE;
+
+	mutex_init(&ch->v4l2_mutex);
+	mutex_init(&ch->vb_queue_mutex);
+	spin_lock_init(&ch->queued_bufs_lock);
+	INIT_LIST_HEAD(&ch->queued_bufs);
+
+	/* Init videobuf2 queue structure */
+	ch->vb_queue.type = V4L2_BUF_TYPE_SDR_CAPTURE;
+	ch->vb_queue.io_modes = VB2_READ | VB2_MMAP | VB2_DMABUF;
+	ch->vb_queue.drv_priv = ch;
+	ch->vb_queue.buf_struct_size = sizeof(struct rcar_drif_frame_buf);
+	ch->vb_queue.ops = &rcar_drif_vb2_ops;
+	ch->vb_queue.mem_ops = &vb2_vmalloc_memops;
+	ch->vb_queue.timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
+
+	/* Init videobuf2 queue */
+	ret = vb2_queue_init(&ch->vb_queue);
+	if (ret) {
+		dev_err(ch->dev, "could not initialize vb2 queue\n");
+		return ret;
+	}
+
+	/* Init video_device structure */
+	ch->vdev = rcar_drif_vdev;
+	ch->vdev.lock = &ch->v4l2_mutex;
+	ch->vdev.queue = &ch->vb_queue;
+	ch->vdev.queue->lock = &ch->vb_queue_mutex;
+	ch->vdev.ctrl_handler = &ch->ctrl_hdl;
+	video_set_drvdata(&ch->vdev, ch);
+
+	/* Register the v4l2_device */
+	ret = v4l2_device_register(&pdev->dev, &ch->v4l2_dev);
+	if (ret) {
+		dev_err(ch->dev, "failed v4l2_device_register. ret %d\n", ret);
+		return ret;
+	}
+
+	ch->vdev.v4l2_dev = &ch->v4l2_dev;
+
+	/*
+	 * Parse subdevs after v4l2_device_register because if the subdev
+	 * is already probed, bound and complete will be called immediately
+	 */
+	ret = rcar_drif_parse_subdevs(&pdev->dev, &ch->notifier);
+	if (ret)
+		goto err_unreg_v4l2;
+
+	ch->notifier.bound = rcar_drif_notify_bound;
+	ch->notifier.complete = rcar_drif_notify_complete;
+
+	/* Register notifier */
+	ret = v4l2_async_notifier_register(&ch->v4l2_dev, &ch->notifier);
+	if (ret < 0) {
+		dev_err(ch->dev, "notifier registration failed\n");
+		goto err_unreg_v4l2;
+	}
+
+	/* Register SDR device */
+	ret = video_register_device(&ch->vdev, VFL_TYPE_SDR, -1);
+	if (ret) {
+		dev_err(ch->dev, "failed video_register_device. ret %d\n", ret);
+		goto err_unreg_notif;
+	}
+
+	platform_set_drvdata(pdev, ch);
+	dev_notice(ch->dev, "probed\n");
+	return 0;
+
+err_unreg_notif:
+	v4l2_async_notifier_unregister(&ch->notifier);
+err_unreg_v4l2:
+	v4l2_device_unregister(&ch->v4l2_dev);
+	return ret;
+}
+
+static int rcar_drif_remove(struct platform_device *pdev)
+{
+	struct rcar_drif_chan *ch = platform_get_drvdata(pdev);
+
+	if (!ch)
+		return 0;
+
+	/* Parent channel instance */
+	ch = platform_get_drvdata(pdev);
+	v4l2_ctrl_handler_free(ch->v4l2_dev.ctrl_handler);
+	v4l2_async_notifier_unregister(&ch->notifier);
+	v4l2_device_unregister(&ch->v4l2_dev);
+	video_unregister_device(&ch->vdev);
+	dev_notice(ch->dev, "removed\n");
+	return 0;
+}
+
+static int __maybe_unused rcar_drif_suspend(struct device *dev)
+{
+	return 0;
+}
+
+static int __maybe_unused rcar_drif_resume(struct device *dev)
+{
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(rcar_drif_pm_ops, rcar_drif_suspend,
+			 rcar_drif_resume);
+
+static const struct of_device_id rcar_drif_of_table[] = {
+	{ .compatible = "renesas,rcar-gen3-drif" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, rcar_drif_of_table);
+
+#define RCAR_DRIF_DRV_NAME "rcar_drif"
+static struct platform_driver rcar_drif_driver = {
+	.driver = {
+		.name = RCAR_DRIF_DRV_NAME,
+		.of_match_table = of_match_ptr(rcar_drif_of_table),
+		.pm = &rcar_drif_pm_ops,
+		},
+	.probe = rcar_drif_probe,
+	.remove = rcar_drif_remove,
+};
+
+module_platform_driver(rcar_drif_driver);
+
+MODULE_DESCRIPTION("Renesas R-Car Gen3 DRIF driver");
+MODULE_ALIAS("platform:" RCAR_DRIF_DRV_NAME);
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Ramesh Shanmugasundaram <ramesh.shanmugasundaram@bp.renesas.com>");
-- 
1.9.1

^ permalink raw reply related

* [PATCH 4/5] doc_rst: media: New SDR formats SC16, SC18 & SC20
From: Ramesh Shanmugasundaram @ 2016-11-09 15:44 UTC (permalink / raw)
  To: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	mchehab-DgEjT+Ai2ygdnm+yROfE0A, hverkuil-qWit8jRvyhVmR6Xm/wNWPw,
	sakari.ailus-VuQAYsv1563Yd54FQh9/CA, crope-X3B1VOXEql0
  Cc: chris.paterson2-zM6kxYcvzFBBDgjK7y7TUQ,
	laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw,
	geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ,
	linux-media-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA, Ramesh Shanmugasundaram
In-Reply-To: <1478706284-59134-1-git-send-email-ramesh.shanmugasundaram-kTT6dE0pTRh9uiUsa/gSgQ@public.gmane.org>

This patch adds documentation for the three new SDR formats

V4L2_SDR_FMT_SCU16BE
V4L2_SDR_FMT_SCU18BE
V4L2_SDR_FMT_SCU20BE

Signed-off-by: Ramesh Shanmugasundaram <ramesh.shanmugasundaram-kTT6dE0pTRh9uiUsa/gSgQ@public.gmane.org>
---
 .../media/uapi/v4l/pixfmt-sdr-scu16be.rst          | 80 ++++++++++++++++++++++
 .../media/uapi/v4l/pixfmt-sdr-scu18be.rst          | 80 ++++++++++++++++++++++
 .../media/uapi/v4l/pixfmt-sdr-scu20be.rst          | 80 ++++++++++++++++++++++
 Documentation/media/uapi/v4l/sdr-formats.rst       |  3 +
 4 files changed, 243 insertions(+)
 create mode 100644 Documentation/media/uapi/v4l/pixfmt-sdr-scu16be.rst
 create mode 100644 Documentation/media/uapi/v4l/pixfmt-sdr-scu18be.rst
 create mode 100644 Documentation/media/uapi/v4l/pixfmt-sdr-scu20be.rst

diff --git a/Documentation/media/uapi/v4l/pixfmt-sdr-scu16be.rst b/Documentation/media/uapi/v4l/pixfmt-sdr-scu16be.rst
new file mode 100644
index 0000000..7525378
--- /dev/null
+++ b/Documentation/media/uapi/v4l/pixfmt-sdr-scu16be.rst
@@ -0,0 +1,80 @@
+.. -*- coding: utf-8; mode: rst -*-
+
+.. _V4L2-SDR-FMT-SCU16BE:
+
+******************************
+V4L2_SDR_FMT_SCU16BE ('SC16')
+******************************
+
+Sliced complex unsigned 16-bit big endian IQ sample
+
+Description
+===========
+
+This format contains a sequence of complex number samples. Each complex
+number consist of two parts called In-phase and Quadrature (IQ). Both I
+and Q are represented as a 16 bit unsigned big endian number stored in
+32 bit space. The remaining unused bits within the 32 bit space will be
+padded with 0. I value starts first and Q value starts at an offset
+equalling half of the buffer size (i.e.) offset = buffersize/2. Out of
+the 16 bits, bit 15:2 (14 bit) is data and bit 1:0 (2 bit) can be any
+value.
+
+**Byte Order.**
+Each cell is one byte.
+
+.. flat-table::
+    :header-rows:  1
+    :stub-columns: 0
+
+    * -  Offset:
+
+      -  Byte B0
+
+      -  Byte B1
+
+      -  Byte B2
+
+      -  Byte B3
+
+    * -  start + 0:
+
+      -  I'\ :sub:`0[13:6]`
+
+      -  I'\ :sub:`0[5:0]; B1[1:0]=pad`
+
+      -  pad
+
+      -  pad
+
+    * -  start + 4:
+
+      -  I'\ :sub:`1[13:6]`
+
+      -  I'\ :sub:`1[5:0]; B1[1:0]=pad`
+
+      -  pad
+
+      -  pad
+
+    * -  ...
+
+    * - start + offset:
+
+      -  Q'\ :sub:`0[13:6]`
+
+      -  Q'\ :sub:`0[5:0]; B1[1:0]=pad`
+
+      -  pad
+
+      -  pad
+
+    * - start + offset + 4:
+
+      -  Q'\ :sub:`1[13:6]`
+
+      -  Q'\ :sub:`1[5:0]; B1[1:0]=pad`
+
+      -  pad
+
+      -  pad
diff --git a/Documentation/media/uapi/v4l/pixfmt-sdr-scu18be.rst b/Documentation/media/uapi/v4l/pixfmt-sdr-scu18be.rst
new file mode 100644
index 0000000..0ce714d
--- /dev/null
+++ b/Documentation/media/uapi/v4l/pixfmt-sdr-scu18be.rst
@@ -0,0 +1,80 @@
+.. -*- coding: utf-8; mode: rst -*-
+
+.. _V4L2-SDR-FMT-SCU18BE:
+
+******************************
+V4L2_SDR_FMT_SCU18BE ('SC18')
+******************************
+
+Sliced complex unsigned 18-bit big endian IQ sample
+
+Description
+===========
+
+This format contains a sequence of complex number samples. Each complex
+number consist of two parts called In-phase and Quadrature (IQ). Both I
+and Q are represented as a 18 bit unsigned big endian number stored in
+32 bit space. The remaining unused bits within the 32 bit space will be
+padded with 0. I value starts first and Q value starts at an offset
+equalling half of the buffer size (i.e.) offset = buffersize/2. Out of
+the 18 bits, bit 17:2 (16 bit) is data and bit 1:0 (2 bit) can be any
+value.
+
+**Byte Order.**
+Each cell is one byte.
+
+.. flat-table::
+    :header-rows:  1
+    :stub-columns: 0
+
+    * -  Offset:
+
+      -  Byte B0
+
+      -  Byte B1
+
+      -  Byte B2
+
+      -  Byte B3
+
+    * -  start + 0:
+
+      -  I'\ :sub:`0[17:10]`
+
+      -  I'\ :sub:`0[9:2]`
+
+      -  I'\ :sub:`0[1:0]; B2[5:0]=pad`
+
+      -  pad
+
+    * -  start + 4:
+
+      -  I'\ :sub:`1[17:10]`
+
+      -  I'\ :sub:`1[9:2]`
+
+      -  I'\ :sub:`1[1:0]; B2[5:0]=pad`
+
+      -  pad
+
+    * -  ...
+
+    * - start + offset:
+
+      -  Q'\ :sub:`0[17:10]`
+
+      -  Q'\ :sub:`0[9:2]`
+
+      -  Q'\ :sub:`0[1:0]; B2[5:0]=pad`
+
+      -  pad
+
+    * - start + offset + 4:
+
+      -  Q'\ :sub:`1[17:10]`
+
+      -  Q'\ :sub:`1[9:2]`
+
+      -  Q'\ :sub:`1[1:0]; B2[5:0]=pad`
+
+      -  pad
diff --git a/Documentation/media/uapi/v4l/pixfmt-sdr-scu20be.rst b/Documentation/media/uapi/v4l/pixfmt-sdr-scu20be.rst
new file mode 100644
index 0000000..ff2fe51
--- /dev/null
+++ b/Documentation/media/uapi/v4l/pixfmt-sdr-scu20be.rst
@@ -0,0 +1,80 @@
+.. -*- coding: utf-8; mode: rst -*-
+
+.. _V4L2-SDR-FMT-SCU20BE:
+
+******************************
+V4L2_SDR_FMT_SCU20BE ('SC20')
+******************************
+
+Sliced complex unsigned 20-bit big endian IQ sample
+
+Description
+===========
+
+This format contains a sequence of complex number samples. Each complex
+number consist of two parts called In-phase and Quadrature (IQ). Both I
+and Q are represented as a 20 bit unsigned big endian number stored in
+32 bit space. The remaining unused bits within the 32 bit space will be
+padded with 0. I value starts first and Q value starts at an offset
+equalling half of the buffer size (i.e.) offset = buffersize/2. Out of
+the 20 bits, bit 19:2 (18 bit) is data and bit 1:0 (2 bit) can be any
+value.
+
+**Byte Order.**
+Each cell is one byte.
+
+.. flat-table::
+    :header-rows:  1
+    :stub-columns: 0
+
+    * -  Offset:
+
+      -  Byte B0
+
+      -  Byte B1
+
+      -  Byte B2
+
+      -  Byte B3
+
+    * -  start + 0:
+
+      -  I'\ :sub:`0[19:12]`
+
+      -  I'\ :sub:`0[11:4]`
+
+      -  I'\ :sub:`0[3:0]; B2[3:0]=pad`
+
+      -  pad
+
+    * -  start + 4:
+
+      -  I'\ :sub:`1[19:12]`
+
+      -  I'\ :sub:`1[11:4]`
+
+      -  I'\ :sub:`1[3:0]; B2[3:0]=pad`
+
+      -  pad
+
+    * -  ...
+
+    * - start + offset:
+
+      -  Q'\ :sub:`0[19:12]`
+
+      -  Q'\ :sub:`0[11:4]`
+
+      -  Q'\ :sub:`0[3:0]; B2[3:0]=pad`
+
+      -  pad
+
+    * - start + offset + 4:
+
+      -  Q'\ :sub:`1[19:12]`
+
+      -  Q'\ :sub:`1[11:4]`
+
+      -  Q'\ :sub:`1[3:0]; B2[3:0]=pad`
+
+      -  pad
diff --git a/Documentation/media/uapi/v4l/sdr-formats.rst b/Documentation/media/uapi/v4l/sdr-formats.rst
index f863c08..4c01cf9 100644
--- a/Documentation/media/uapi/v4l/sdr-formats.rst
+++ b/Documentation/media/uapi/v4l/sdr-formats.rst
@@ -17,3 +17,6 @@ These formats are used for :ref:`SDR <sdr>` interface only.
     pixfmt-sdr-cs08
     pixfmt-sdr-cs14le
     pixfmt-sdr-ru12le
+    pixfmt-sdr-scu16be
+    pixfmt-sdr-scu18be
+    pixfmt-sdr-scu20be
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH 3/5] media: Add new SDR formats SC16, SC18 & SC20
From: Ramesh Shanmugasundaram @ 2016-11-09 15:44 UTC (permalink / raw)
  To: robh+dt, mark.rutland, mchehab, hverkuil, sakari.ailus, crope
  Cc: chris.paterson2, laurent.pinchart, geert+renesas, linux-media,
	devicetree, linux-renesas-soc, Ramesh Shanmugasundaram
In-Reply-To: <1478706284-59134-1-git-send-email-ramesh.shanmugasundaram@bp.renesas.com>

This patch adds support for the three new SDR formats. These formats
were prefixed with "sliced" indicating I data constitutes the top half and
Q data constitutes the bottom half of the received buffer.

V4L2_SDR_FMT_SCU16BE - 14-bit complex (I & Q) unsigned big-endian sample
inside 16-bit. V4L2 FourCC: SC16

V4L2_SDR_FMT_SCU18BE - 16-bit complex (I & Q) unsigned big-endian sample
inside 18-bit. V4L2 FourCC: SC18

V4L2_SDR_FMT_SCU20BE - 18-bit complex (I & Q) unsigned big-endian sample
inside 20-bit. V4L2 FourCC: SC20

Signed-off-by: Ramesh Shanmugasundaram <ramesh.shanmugasundaram@bp.renesas.com>
---
 drivers/media/v4l2-core/v4l2-ioctl.c | 3 +++
 include/uapi/linux/videodev2.h       | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index 181381d..d36b386 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -1207,6 +1207,9 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
 	case V4L2_SDR_FMT_CS8:		descr = "Complex S8"; break;
 	case V4L2_SDR_FMT_CS14LE:	descr = "Complex S14LE"; break;
 	case V4L2_SDR_FMT_RU12LE:	descr = "Real U12LE"; break;
+	case V4L2_SDR_FMT_SCU16BE:	descr = "Sliced Complex U16BE"; break;
+	case V4L2_SDR_FMT_SCU18BE:	descr = "Sliced Complex U18BE"; break;
+	case V4L2_SDR_FMT_SCU20BE:	descr = "Sliced Complex U20BE"; break;
 	case V4L2_TCH_FMT_DELTA_TD16:	descr = "16-bit signed deltas"; break;
 	case V4L2_TCH_FMT_DELTA_TD08:	descr = "8-bit signed deltas"; break;
 	case V4L2_TCH_FMT_TU16:		descr = "16-bit unsigned touch data"; break;
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 4364ce6..34a9c30 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -666,6 +666,9 @@ struct v4l2_pix_format {
 #define V4L2_SDR_FMT_CS8          v4l2_fourcc('C', 'S', '0', '8') /* complex s8 */
 #define V4L2_SDR_FMT_CS14LE       v4l2_fourcc('C', 'S', '1', '4') /* complex s14le */
 #define V4L2_SDR_FMT_RU12LE       v4l2_fourcc('R', 'U', '1', '2') /* real u12le */
+#define V4L2_SDR_FMT_SCU16BE	  v4l2_fourcc('S', 'C', '1', '6') /* sliced complex u16be */
+#define V4L2_SDR_FMT_SCU18BE	  v4l2_fourcc('S', 'C', '1', '8') /* sliced complex u18be */
+#define V4L2_SDR_FMT_SCU20BE	  v4l2_fourcc('S', 'C', '2', '0') /* sliced complex u20be */
 
 /* Touch formats - used for Touch devices */
 #define V4L2_TCH_FMT_DELTA_TD16	v4l2_fourcc('T', 'D', '1', '6') /* 16-bit signed deltas */
-- 
1.9.1

^ 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