* Applied "regulator: max77620: add documentation for MPOK property" to the regulator tree
From: Mark Brown @ 2016-11-23 16:31 UTC (permalink / raw)
Cc: Rob Herring, Lee Jones, Mark Brown
In-Reply-To: <1479297161-7705-2-git-send-email-vreddytalla-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
The patch
regulator: max77620: add documentation for MPOK property
has been applied to the regulator tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
>From 983779235a4d08f94e8cda073200423e0ff01d2e Mon Sep 17 00:00:00 2001
From: Venkat Reddy Talla <vreddytalla-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Date: Thu, 17 Nov 2016 23:24:36 +0530
Subject: [PATCH] regulator: max77620: add documentation for MPOK property
Adding documentation for maxim,power-ok-control dts property
Signed-off-by: Venkat Reddy Talla <vreddytalla-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Acked-by: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Signed-off-by: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
Documentation/devicetree/bindings/mfd/max77620.txt | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/Documentation/devicetree/bindings/mfd/max77620.txt b/Documentation/devicetree/bindings/mfd/max77620.txt
index 2ad44f7e4880..9c16d51cc15b 100644
--- a/Documentation/devicetree/bindings/mfd/max77620.txt
+++ b/Documentation/devicetree/bindings/mfd/max77620.txt
@@ -106,6 +106,18 @@ Here supported time periods by device in microseconds are as follows:
MAX77620 supports 40, 80, 160, 320, 640, 1280, 2560 and 5120 microseconds.
MAX20024 supports 20, 40, 80, 160, 320, 640, 1280 and 2540 microseconds.
+-maxim,power-ok-control: configure map power ok bit
+ 1: Enables POK(Power OK) to control nRST_IO and GPIO1
+ POK function.
+ 0: Disables POK control.
+ if property missing, do not configure MPOK bit.
+ If POK mapping is enabled for GPIO1/nRST_IO then,
+ GPIO1/nRST_IO pins are HIGH only if all rails
+ that have POK control enabled are HIGH.
+ If any of the rails goes down(which are enabled for POK
+ control) then, GPIO1/nRST_IO goes LOW.
+ this property is valid for max20024 only.
+
For DT binding details of different sub modules like GPIO, pincontrol,
regulator, power, please refer respective device-tree binding document
under their respective sub-system directories.
--
2.10.2
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH 3/3] ARM: dts: da850: Add node for pullup/pulldown pinconf
From: David Lechner @ 2016-11-23 16:24 UTC (permalink / raw)
To: Sekhar Nori, Linus Walleij, Rob Herring, Mark Rutland,
Kevin Hilman
Cc: devicetree, Axel Haslam, Bartosz Gołaszewski, linux-kernel,
linux-gpio, Alexandre Bailon, linux-arm-kernel
In-Reply-To: <8c3e6535-4b79-9731-f801-c13f007e48ab@ti.com>
On 11/23/2016 05:12 AM, Sekhar Nori wrote:
> On Wednesday 23 November 2016 08:59 AM, David Lechner wrote:
>> This SoC has a separate pin controller for configuring pullup/pulldown
>> bias on groups of pins.
>>
>> Signed-off-by: David Lechner <david@lechnology.com>
>> ---
>> arch/arm/boot/dts/da850.dtsi | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
>> index 8945815..1c0224c 100644
>> --- a/arch/arm/boot/dts/da850.dtsi
>> +++ b/arch/arm/boot/dts/da850.dtsi
>> @@ -210,6 +210,11 @@
>> };
>>
>> };
>> + pinconf: pin-controller@22c00c {
>> + compatible = "ti,da850-pupd";
>> + reg = <0x22c00c 0x8>;
>> + status = "disabled";
>> + };
>
> Can you please place this below the i2c1 node. I am trying to keep the
> nodes sorted by unit address. I know thats broken in many places today,
> but lets add the new ones where they should eventually end up.
I can do this, but it seems that the predominant sorting pattern here is
to keep subsystems together (e.g. all i2c are together, all uart are
together, etc.)
Would a separate patch to sort everything by unit address to get this
cleaned up be acceptable?
>
> Thanks,
> Sekhar
>
^ permalink raw reply
* Re: [PATCH 2/3] pinctrl: New driver for TI DA8XX/OMAP-L138/AM18XX pinconf
From: David Lechner @ 2016-11-23 16:21 UTC (permalink / raw)
To: Sekhar Nori, Linus Walleij, Rob Herring, Mark Rutland,
Kevin Hilman
Cc: linux-gpio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Axel Haslam,
Alexandre Bailon, Bartosz Gołaszewski
In-Reply-To: <88b3d16b-6e5b-ea75-d770-35d9adc6c677-l0cyMroinI0@public.gmane.org>
On 11/23/2016 05:04 AM, Sekhar Nori wrote:
> On Wednesday 23 November 2016 08:59 AM, David Lechner wrote:
>> This adds a new driver for pinconf on TI DA8XX/OMAP-L138/AM18XX. These
>
> s/DA8XX/DA850/
>
>> SoCs have a separate controller for controlling pullup/pulldown groups.
>>
>> Signed-off-by: David Lechner <david-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>
>
>> +static const char *da850_pupd_get_get_group_name(struct pinctrl_dev *pctldev,
>> + unsigned int selector)
>> +{
>> + return da850_pupd_group_names[selector];
>> +}
>> +
>> +static int da850_pupd_get_get_group_pins(struct pinctrl_dev *pctldev,
>> + unsigned int selector,
>> + const unsigned int **pins,
>> + unsigned int *num_pins)
>> +{
>> + *num_pins = 0;
>> +
>> + return 0;
>> +}
>
> usage of get_get_ in the function names above is odd.
Will fix (copy/paste error)
>
> Thanks,
> Sekhar
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH 2/2] ARM64: dts: meson-gxm: add SCPI configuration for GXM
From: Martin Blumenstingl @ 2016-11-23 16:20 UTC (permalink / raw)
To: linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
khilman-rdvid1DuHRBWk0Htik3J/w, carlo-KA+7E9HrN00dnm+yROfE0A
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, will.deacon-5wv7dgnIgG8,
catalin.marinas-5wv7dgnIgG8, mark.rutland-5wv7dgnIgG8,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, Martin Blumenstingl
In-Reply-To: <20161123162040.24843-1-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
This adds the SCPI DVFS clock index and configures the CPU cores
accordingly.
Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
arch/arm64/boot/dts/amlogic/meson-gxm.dtsi | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxm.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxm.dtsi
index c1974bb..2b1d276e 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxm.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxm.dtsi
@@ -85,6 +85,7 @@
reg = <0x0 0x100>;
enable-method = "psci";
next-level-cache = <&l2>;
+ clocks = <&scpi_dvfs 1>;
};
cpu5: cpu@101 {
@@ -93,6 +94,7 @@
reg = <0x0 0x101>;
enable-method = "psci";
next-level-cache = <&l2>;
+ clocks = <&scpi_dvfs 1>;
};
cpu6: cpu@102 {
@@ -101,6 +103,7 @@
reg = <0x0 0x102>;
enable-method = "psci";
next-level-cache = <&l2>;
+ clocks = <&scpi_dvfs 1>;
};
cpu7: cpu@103 {
@@ -109,6 +112,12 @@
reg = <0x0 0x103>;
enable-method = "psci";
next-level-cache = <&l2>;
+ clocks = <&scpi_dvfs 1>;
};
};
};
+
+&scpi_dvfs {
+ clock-indices = <0 1>;
+ clock-output-names = "vbig", "vlittle";
+};
--
2.10.2
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 1/2] ARM64: dts: meson-gx: move the SCPI and SRAM nodes to meson-gx
From: Martin Blumenstingl @ 2016-11-23 16:20 UTC (permalink / raw)
To: linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
khilman-rdvid1DuHRBWk0Htik3J/w, carlo-KA+7E9HrN00dnm+yROfE0A
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, will.deacon-5wv7dgnIgG8,
catalin.marinas-5wv7dgnIgG8, mark.rutland-5wv7dgnIgG8,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, Martin Blumenstingl
In-Reply-To: <20161123162040.24843-1-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
SCPI and SRAM are identical on GXBB and GXL. Moving the corresponding
nodes to meson-gx adds support for the thermal sensor on GXL based
devices.
While here, also rename the second compatible string because
"arm,legacy-scpi" was replaced by "arm,scpi-pre-1.0".
Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
arch/arm64/boot/dts/amlogic/meson-gx.dtsi | 45 +++++++++++++++++++++++
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 57 -----------------------------
2 files changed, 45 insertions(+), 57 deletions(-)
diff --git a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
index fc033c0..47ab306 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
@@ -65,6 +65,7 @@
reg = <0x0 0x0>;
enable-method = "psci";
next-level-cache = <&l2>;
+ clocks = <&scpi_dvfs 0>;
};
cpu1: cpu@1 {
@@ -73,6 +74,7 @@
reg = <0x0 0x1>;
enable-method = "psci";
next-level-cache = <&l2>;
+ clocks = <&scpi_dvfs 0>;
};
cpu2: cpu@2 {
@@ -81,6 +83,7 @@
reg = <0x0 0x2>;
enable-method = "psci";
next-level-cache = <&l2>;
+ clocks = <&scpi_dvfs 0>;
};
cpu3: cpu@3 {
@@ -89,6 +92,7 @@
reg = <0x0 0x3>;
enable-method = "psci";
next-level-cache = <&l2>;
+ clocks = <&scpi_dvfs 0>;
};
l2: l2-cache0 {
@@ -153,6 +157,28 @@
};
};
+ scpi {
+ compatible = "amlogic,meson-gxbb-scpi", "arm,scpi-pre-1.0";
+ mboxes = <&mailbox 1 &mailbox 2>;
+ shmem = <&cpu_scp_lpri &cpu_scp_hpri>;
+
+ clocks {
+ compatible = "arm,scpi-clocks";
+
+ scpi_dvfs: scpi_clocks@0 {
+ compatible = "arm,scpi-dvfs-clocks";
+ #clock-cells = <1>;
+ clock-indices = <0>;
+ clock-output-names = "vcpu";
+ };
+ };
+
+ scpi_sensors: sensors {
+ compatible = "arm,scpi-sensors";
+ #thermal-sensor-cells = <1>;
+ };
+ };
+
soc {
compatible = "simple-bus";
#address-cells = <2>;
@@ -264,6 +290,25 @@
#address-cells = <0>;
};
+ sram: sram@c8000000 {
+ compatible = "amlogic,meson-gxbb-sram", "mmio-sram";
+ reg = <0x0 0xc8000000 0x0 0x14000>;
+
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0 0x0 0xc8000000 0x14000>;
+
+ cpu_scp_lpri: scp-shmem@0 {
+ compatible = "amlogic,meson-gxbb-scp-shmem";
+ reg = <0x13000 0x400>;
+ };
+
+ cpu_scp_hpri: scp-shmem@200 {
+ compatible = "amlogic,meson-gxbb-scp-shmem";
+ reg = <0x13400 0x400>;
+ };
+ };
+
aobus: aobus@c8100000 {
compatible = "simple-bus";
reg = <0x0 0xc8100000 0x0 0x100000>;
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
index ac5ad3b..76465e7 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
@@ -50,28 +50,6 @@
/ {
compatible = "amlogic,meson-gxbb";
- scpi {
- compatible = "amlogic,meson-gxbb-scpi";
- mboxes = <&mailbox 1 &mailbox 2>;
- shmem = <&cpu_scp_lpri &cpu_scp_hpri>;
-
- clocks {
- compatible = "arm,scpi-clocks";
-
- scpi_dvfs: scpi_clocks@0 {
- compatible = "arm,scpi-dvfs-clocks";
- #clock-cells = <1>;
- clock-indices = <0>;
- clock-output-names = "vcpu";
- };
- };
-
- scpi_sensors: sensors {
- compatible = "arm,scpi-sensors";
- #thermal-sensor-cells = <1>;
- };
- };
-
soc {
usb0_phy: phy@c0000000 {
compatible = "amlogic,meson-gxbb-usb2-phy";
@@ -93,25 +71,6 @@
status = "disabled";
};
- sram: sram@c8000000 {
- compatible = "amlogic,meson-gxbb-sram", "mmio-sram";
- reg = <0x0 0xc8000000 0x0 0x14000>;
-
- #address-cells = <1>;
- #size-cells = <1>;
- ranges = <0 0x0 0xc8000000 0x14000>;
-
- cpu_scp_lpri: scp-shmem@0 {
- compatible = "amlogic,meson-gxbb-scp-shmem";
- reg = <0x13000 0x400>;
- };
-
- cpu_scp_hpri: scp-shmem@200 {
- compatible = "amlogic,meson-gxbb-scp-shmem";
- reg = <0x13400 0x400>;
- };
- };
-
usb0: usb@c9000000 {
compatible = "amlogic,meson-gxbb-usb", "snps,dwc2";
reg = <0x0 0xc9000000 0x0 0x40000>;
@@ -138,22 +97,6 @@
};
};
-&cpu0 {
- clocks = <&scpi_dvfs 0>;
-};
-
-&cpu1 {
- clocks = <&scpi_dvfs 0>;
-};
-
-&cpu2 {
- clocks = <&scpi_dvfs 0>;
-};
-
-&cpu3 {
- clocks = <&scpi_dvfs 0>;
-};
-
&cbus {
spifc: spi@8c80 {
compatible = "amlogic,meson-gxbb-spifc";
--
2.10.2
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 0/2] minor GXL and GXM improvements
From: Martin Blumenstingl @ 2016-11-23 16:20 UTC (permalink / raw)
To: linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
khilman-rdvid1DuHRBWk0Htik3J/w, carlo-KA+7E9HrN00dnm+yROfE0A
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, will.deacon-5wv7dgnIgG8,
catalin.marinas-5wv7dgnIgG8, mark.rutland-5wv7dgnIgG8,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, Martin Blumenstingl
This series adds SCPI support to GXL and GXM SoCs by moving the nodes
to meson-gx.dtsi. Additionally this updates the compatible string to
match the recent changes, see [0]
Now that we have SCPI support for GXM we can also use it to configure
the CPU cores using the SCPI DVFS clocks.
[0] http://lists.infradead.org/pipermail/linux-amlogic/2016-November/001632.html
Martin Blumenstingl (2):
ARM64: dts: meson-gx: move the SCPI and SRAM nodes to meson-gx
ARM64: dts: meson-gxm: add SCPI configuration for GXM
arch/arm64/boot/dts/amlogic/meson-gx.dtsi | 45 +++++++++++++++++++++++
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 57 -----------------------------
arch/arm64/boot/dts/amlogic/meson-gxm.dtsi | 9 +++++
3 files changed, 54 insertions(+), 57 deletions(-)
--
2.10.2
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2] ARM: dts: da850: add the mstpri and ddrctl nodes
From: David Lechner @ 2016-11-23 16:19 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Kevin Hilman, Michael Turquette, Sekhar Nori, Rob Herring,
Frank Rowand, Mark Rutland, Peter Ujfalusi, Russell King,
linux-devicetree, David Airlie, LKML, linux-drm, Tomi Valkeinen,
Jyri Sarha, arm-soc, Laurent Pinchart
In-Reply-To: <CAMpxmJVqog0Fno_DO0NQKmVNvzu5d=3GOLk145wBmB-V70BLeg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 11/23/2016 04:27 AM, Bartosz Golaszewski wrote:
> 2016-11-22 23:23 GMT+01:00 David Lechner <david-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>:
>> On 11/15/2016 05:00 AM, Bartosz Golaszewski wrote:
>>>
>>> Add the nodes for the MSTPRI configuration and DDR2/mDDR memory
>>> controller drivers to da850.dtsi.
>>>
>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
>>> ---
>>> v1 -> v2:
>>> - moved the priority controller node above the cfgchip node
>>> - renamed added nodes to better reflect their purpose
>>>
>>> arch/arm/boot/dts/da850.dtsi | 8 ++++++++
>>> 1 file changed, 8 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
>>> index 1bb1f6d..412eec6 100644
>>> --- a/arch/arm/boot/dts/da850.dtsi
>>> +++ b/arch/arm/boot/dts/da850.dtsi
>>> @@ -210,6 +210,10 @@
>>> };
>>>
>>> };
>>> + prictrl: priority-controller@14110 {
>>> + compatible = "ti,da850-mstpri";
>>> + reg = <0x14110 0x0c>;
>>
>>
>> I think we should add status = "disabled"; here and let boards opt in.
>>
>>> + };
>>> cfgchip: chip-controller@1417c {
>>> compatible = "ti,da830-cfgchip", "syscon",
>>> "simple-mfd";
>>> reg = <0x1417c 0x14>;
>>> @@ -451,4 +455,8 @@
>>> 1 0 0x68000000 0x00008000>;
>>> status = "disabled";
>>> };
>>> + memctrl: memory-controller@b0000000 {
>>> + compatible = "ti,da850-ddr-controller";
>>> + reg = <0xb0000000 0xe8>;
>>
>>
>> same here. status = "disabled";
>>
>>> + };
>>> };
>>>
>
> Hi David,
>
> I did that initially[1][2] and it was rejected by Kevin[3] and Laurent[4].
>
> FYI this patch has already been queued by Sekhar.
Thanks. I did not see those threads.
FYI to maintainers, having these enabled by default causes error
messages in the kernel log for other boards that are not supported by
the drivers. Since there is only one board that is supported and soon to
be 2 that are not, I would rather have this disabled by default to avoid
the error messages.
>
> Best regards,
> Bartosz Golaszewski
>
> [1] https://www.spinics.net/lists/arm-kernel/msg539638.html
> [2] http://www.spinics.net/lists/devicetree/msg148575.html
> [3] http://www.spinics.net/lists/devicetree/msg148667.html
> [4] http://www.spinics.net/lists/devicetree/msg148655.html
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] v4l: async: make v4l2 coexists with devicetree nodes in a dt overlay
From: Javi Merino @ 2016-11-23 16:15 UTC (permalink / raw)
To: Sakari Ailus
Cc: linux-media, linux-kernel, devicetree, Pantelis Antoniou,
Mauro Carvalho Chehab, Javier Martinez Canillas, Sakari Ailus
In-Reply-To: <20161123151042.GD16630@valkosipuli.retiisi.org.uk>
On Wed, Nov 23, 2016 at 05:10:42PM +0200, Sakari Ailus wrote:
> Hi Javi,
Hi Sakari,
> On Wed, Nov 23, 2016 at 10:09:57AM +0000, Javi Merino wrote:
> > In asd's configured with V4L2_ASYNC_MATCH_OF, if the v4l2 subdev is in
> > a devicetree overlay, its of_node pointer will be different each time
> > the overlay is applied. We are not interested in matching the
> > pointer, what we want to match is that the path is the one we are
> > expecting. Change to use of_node_cmp() so that we continue matching
> > after the overlay has been removed and reapplied.
> >
> > Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
> > Cc: Javier Martinez Canillas <javier@osg.samsung.com>
> > Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> > Signed-off-by: Javi Merino <javi.merino@kernel.org>
> > ---
> > Hi,
> >
> > I feel it is a bit of a hack, but I couldn't think of anything better.
> > I'm ccing devicetree@ and Pantelis because there may be a simpler
> > solution.
>
> First I have to admit that I'm not an expert when it comes to DT overlays.
>
> That said, my understanding is that the sub-device and the async sub-device
> are supposed to point to the exactly same DT node. I wonder if there's
> actually anything wrong in the current code.
>
> If the overlay has changed between probing the driver for the async notifier
> and the async sub-device, there should be no match here, should there? The
> two nodes actually point to a node in a different overlay in that case.
Overlays are parts of the devicetree that can be added and removed.
When the overlay is applied, the camera driver is probed and does
v4l2_async_register_subdev(). However, v4l2_async_belongs() fails.
The problem is with comparing pointers. I haven't looked at the
implementation of overlays in detail, but what I see is that the
of_node pointer changes when you remove and reapply an overlay (I
guess it's dynamically allocated and when you remove the overlay, it's
freed).
Cheers,
Javi
> >
> > drivers/media/v4l2-core/v4l2-async.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
> > index 5bada20..d33a17c 100644
> > --- a/drivers/media/v4l2-core/v4l2-async.c
> > +++ b/drivers/media/v4l2-core/v4l2-async.c
> > @@ -42,7 +42,8 @@ static bool match_devname(struct v4l2_subdev *sd,
> >
> > static bool match_of(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)
> > {
> > - return sd->of_node == asd->match.of.node;
> > + return !of_node_cmp(of_node_full_name(sd->of_node),
> > + of_node_full_name(asd->match.of.node));
> > }
> >
> > static bool match_custom(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)
>
> --
> Kind regards,
>
> Sakari Ailus
> e-mail: sakari.ailus@iki.fi XMPP: sailus@retiisi.org.uk
^ permalink raw reply
* Re: [PATCH v2 4/5] arm: dts: am57xx-beagle-x15-common: Add overide powerhold property
From: Tony Lindgren @ 2016-11-23 16:08 UTC (permalink / raw)
To: Keerthy
Cc: Lee Jones, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-gpio-u79uwXL29TY76Z2rM5mHXA, nm-l0cyMroinI0,
t-kristo-l0cyMroinI0
In-Reply-To: <3a05871c-0512-8a84-4ac3-889c661c8638-l0cyMroinI0@public.gmane.org>
* Keerthy <j-keerthy-l0cyMroinI0@public.gmane.org> [161123 00:33]:
> On Wednesday 23 November 2016 02:03 PM, Lee Jones wrote:
> > On Wed, 23 Nov 2016, Keerthy wrote:
> > > On Tuesday 15 November 2016 05:38 AM, Tony Lindgren wrote:
> > > > * Keerthy <j-keerthy-l0cyMroinI0@public.gmane.org> [161109 21:10]:
> > > > > The PMICs have POWERHOLD set by default which prevents PMIC shutdown
> > > > > even on DEV_CTRL On bit set to 0 as the Powerhold has higher priority.
> > > > > So to enable pmic power off this property lets one over ride the default
> > > > > value and enable pmic power off.
> > > >
> > > > This should not cause merge conflicts so probably best to merge along
> > > > with the driver changes:
> > > >
> > > > Acked-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
> > > >
> > > > If you guys want me to pick up this separately let me know.
> > >
> > > Hi Lee Jones,
> > >
> > > Are you planning to pull DT and Documentation patches as well?
> >
> > No need. They can be safely applied to their own subsystems.
>
> Okay. Thanks for the response.
>
> Tony,
>
> Hope you can pull the DT patches.
Applying both into omap-for-v4.10/dt thanks. Please send dts changes
seprately next time if there are no dependencies. This leaves out
the second guessing who should apply what.
Regards,
Tony
--
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] v4l: async: make v4l2 coexists with devicetree nodes in a dt overlay
From: Javi Merino @ 2016-11-23 16:03 UTC (permalink / raw)
To: Javier Martinez Canillas
Cc: linux-media, linux-kernel, devicetree, Pantelis Antoniou,
Mauro Carvalho Chehab, Sakari Ailus
In-Reply-To: <cf31105b-e8c1-4379-cd03-0bdcbdea64d6@osg.samsung.com>
On Wed, Nov 23, 2016 at 11:25:39AM -0300, Javier Martinez Canillas wrote:
> Hello Javi,
>
> On 11/23/2016 07:09 AM, Javi Merino wrote:
> > In asd's configured with V4L2_ASYNC_MATCH_OF, if the v4l2 subdev is in
> > a devicetree overlay, its of_node pointer will be different each time
> > the overlay is applied. We are not interested in matching the
> > pointer, what we want to match is that the path is the one we are
> > expecting. Change to use of_node_cmp() so that we continue matching
> > after the overlay has been removed and reapplied.
> >
>
> I'm still not that familiar with DT overlays (and I guess others aren't
> either) so I think that including an example of a base tree and overlay
> DTS where this is an issue, could make things more clear in the commit.
>
> IIUC, it should be something like this?
>
> -- base tree --
>
> &i2c1 {
> camera: camera@10 {
> reg = <0x10>;
> port {
> cam_ep: endpoint {
> ...
> };
> };
> };
> };
>
> &media_bridge {
> ...
> ports {
> port@0 {
> reg = <0>;
> ep: endpoint {
> remote-endpoint = <&cam_ep>;
> };
> };
> };
> };
>
> -- overlay --
>
> /plugin/;
> / {
> ...
> fragment@0 {
> target = <&camera>;
> __overlay__ {
> compatible = "foo,bar";
> ...
> port {
> cam_ep: endpoint {
> ...
> };
> };
> };
> }
> }
Yes, that's right. What I have is that the whole camera can be
plugged or unplugged, so the overlay adds/removes the camera node:
/ {
fragment@0 {
target-path = "/i2c0";
__overlay__ {
my_cam {
compatible = "foo,bar";
port {
camera0: endpoint {
remote-endpoint = <&vin2a>;
...
};
};
};
};
};
I will add it to the commit message.
> > Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
> > Cc: Javier Martinez Canillas <javier@osg.samsung.com>
> > Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> > Signed-off-by: Javi Merino <javi.merino@kernel.org>
> > ---
> > Hi,
> >
> > I feel it is a bit of a hack, but I couldn't think of anything better.
> > I'm ccing devicetree@ and Pantelis because there may be a simpler
> > solution.
> >
>
> I also couldn't think a better way to do this, since IIUC the node's name is
> the only thing that doesn't change, and is available at the time the bridge
> driver calls v4l2_async_notifier_register() when parsing the base tree.
>
> > drivers/media/v4l2-core/v4l2-async.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
> > index 5bada20..d33a17c 100644
> > --- a/drivers/media/v4l2-core/v4l2-async.c
> > +++ b/drivers/media/v4l2-core/v4l2-async.c
> > @@ -42,7 +42,8 @@ static bool match_devname(struct v4l2_subdev *sd,
> >
> > static bool match_of(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)
> > {
> > - return sd->of_node == asd->match.of.node;
> > + return !of_node_cmp(of_node_full_name(sd->of_node),
> > + of_node_full_name(asd->match.of.node));
> > }
> >
> > static bool match_custom(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)
> >
>
> Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>
>
> Best regards,
> --
> Javier Martinez Canillas
> Open Source Group
> Samsung Research America
^ permalink raw reply
* Re: [PATCH v2 1/2] ARM64: dts: Add support for Meson GXM
From: Kevin Hilman @ 2016-11-23 15:58 UTC (permalink / raw)
To: Martin Blumenstingl
Cc: Neil Armstrong, carlo, linux-amlogic, devicetree, linux-kernel,
linux-arm-kernel
In-Reply-To: <CAFBinCAbUK-6uLc_ceJ5d-oD4N_HGz9rbtCvivTp1N8OOVBNBg@mail.gmail.com>
Martin Blumenstingl <martin.blumenstingl@googlemail.com> writes:
> On Tue, Nov 22, 2016 at 11:00 AM, Neil Armstrong
> <narmstrong@baylibre.com> wrote:
>> Following the Amlogic Linux kernel, it seem the only differences
>> between the GXL and GXM SoCs are the CPU Clusters.
>>
>> This commit renames the gxl-s905d-p23x DTSI in a common file for
>> S905D p23x and S912 q20x boards.
>>
>> Then adds a meson-gxm dtsi and reproduce the P23x to Q20x boards
>> dts files since the S905D and S912 SoCs shares the same pinout
>> and the P23x and Q20x boards are identical.
>>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> Tested-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>
> I have tested this successfully on a Mecool BB2 (similar to the Q200
> reference board).
Thanks for testing (and reporting),
Kevin
^ permalink raw reply
* Re: [PATCH 1/2] PM / Domains: Introduce domain-performance-state binding
From: Vincent Guittot @ 2016-11-23 15:55 UTC (permalink / raw)
To: Kevin Hilman
Cc: Viresh Kumar, Rob Herring, Rafael Wysocki,
linaro-kernel-cunTk1MwBs8s++Sfvej+rw@public.gmane.org,
linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel,
Mark Rutland, Ulf Hansson, Lina Iyer,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Stephen Boyd,
Nayak Rajendra
In-Reply-To: <m2wpfuw5wq.fsf-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
On 23 November 2016 at 16:51, Kevin Hilman <khilman-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> wrote:
> Vincent Guittot <vincent.guittot-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> writes:
>
>> On 22 November 2016 at 19:12, Kevin Hilman <khilman-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> wrote:
>>> Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> writes:
>>>
>>>> On 21-11-16, 09:07, Rob Herring wrote:
>>>>> On Fri, Nov 18, 2016 at 02:53:12PM +0530, Viresh Kumar wrote:
>>>>> > Some platforms have the capability to configure the performance state of
>>>>> > their Power Domains. The performance levels are represented by positive
>>>>> > integer values, a lower value represents lower performance state.
>>>>> >
>>>>> > The power-domains until now were only concentrating on the idle state
>>>>> > management of the device and this needs to change in order to reuse the
>>>>> > infrastructure of power domains for active state management.
>>>>> >
>>>>> > This patch introduces a new optional property for the consumers of the
>>>>> > power-domains: domain-performance-state.
>>>>> >
>>>>> > If the consumers don't need the capability of switching to different
>>>>> > domain performance states at runtime, then they can simply define their
>>>>> > required domain performance state in their node directly. Otherwise the
>>>>> > consumers can define their requirements with help of other
>>>>> > infrastructure, for example the OPP table.
>>>>> >
>>>>> > Signed-off-by: Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>>>> > ---
>>>>> > Documentation/devicetree/bindings/power/power_domain.txt | 6 ++++++
>>>>> > 1 file changed, 6 insertions(+)
>>>>> >
>>>>> > diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
>>>>> > index e1650364b296..db42eacf8b5c 100644
>>>>> > --- a/Documentation/devicetree/bindings/power/power_domain.txt
>>>>> > +++ b/Documentation/devicetree/bindings/power/power_domain.txt
>>>>> > @@ -106,6 +106,12 @@ domain provided by the 'parent' power controller.
>>>>> > - power-domains : A phandle and PM domain specifier as defined by bindings of
>>>>> > the power controller specified by phandle.
>>>>> >
>>>>> > +Optional properties:
>>>>> > +- domain-performance-state: A positive integer value representing the minimum
>>>>> > + performance level (of the parent domain) required by the consumer for its
>>>>> > + working. The integer value '1' represents the lowest performance level and the
>>>>> > + highest value represents the highest performance level.
>>>>>
>>>>> How does one come up with the range of values?
>>>>
>>>> Why would we need a range here? The value here represents the minimum 'state'
>>>> and the assumption is that everything above that level would be fine. So the
>>>> range is automatically: domain-performance-state -> MAX.
>>>>
>>>>> It seems like you are
>>>>> just making up numbers. Couldn't the domain performance level be an OPP
>>>>> in the sense that it is a collection of clock frequencies and voltage
>>>>> settings?
>>>>
>>>> The clock is going to be handled by the device itself (at least for the case we
>>>> have today) and the performance-state lies with the power-domain which is
>>>> configured separately. If the performance level includes both clk and voltage,
>>>> then why would we need to show the clock rates in the DT ? Wouldn't a
>>>> performance level be enough in such cases?
>>>
>>> I think the question is: what does the performance-level of a domain
>>> actually mean? Or, what are the units?
>>>
>>> Depending on the SoC, there's probably a few things this could mean. It
>>> might mean is that an underlying bus/interconnect can be configured to
>>> guarantee a specific bandwidth or throughput. That in turn might mean
>>> that that bus/interconnect might have to be set at a specific
>>> frequency/voltage.
>>>
>>> In your case, IIUC, you're just passing some magic value to some
>>> firmware running on a micro-controller, but under the hood that uC is
>>> probably configuring a frequency/voltage someplace.
>>
>> In the case described by Viresh, it's only about setting the voltage
>> of a power domain that is shared between different devices. these
>> devices wants to run at different frequency (set by the devices) but
>> we have to select a Volateg value that will match with the constraint
>> of all devices (in this case the highest voltage)
>
> Then, at least for this use case, we're talking about voltage, not some
> unspecified units.
>
> But that makes me wonder, this performance state sounds like something
> that is changing dynamically at runtime, so why do you want to describe
> this statically in DT?
>
> This sounds to me like the job of the genpd. When any device in the
> domain does its pm_runtime_get(), the domain could check the device
> frequency and see if it needs to change the domain voltage in order for
> that device to operate at that frequency. When the device goes away
> (using pm_runtime_put()) the domain can check again if it could lower
> the voltage and still meet the requirements of the remaining devices.
That's only part of the job. The device can change its frequency and
as a result ask for a new voltage index while it is already running
Vincent
>
> Kevin
>
>
>
--
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] PM / Domains: Introduce domain-performance-state binding
From: Kevin Hilman @ 2016-11-23 15:51 UTC (permalink / raw)
To: Vincent Guittot
Cc: Viresh Kumar, Rob Herring, Rafael Wysocki,
linaro-kernel@lists.linaro.org, linux-pm@vger.kernel.org,
linux-kernel, Mark Rutland, Ulf Hansson, Lina Iyer,
devicetree@vger.kernel.org, Stephen Boyd, Nayak Rajendra
In-Reply-To: <CAKfTPtDR6Y5UgaqJ+D5T0yBeFRSYWm6OT1+r4ZABrtqtvF2D0w@mail.gmail.com>
Vincent Guittot <vincent.guittot@linaro.org> writes:
> On 22 November 2016 at 19:12, Kevin Hilman <khilman@baylibre.com> wrote:
>> Viresh Kumar <viresh.kumar@linaro.org> writes:
>>
>>> On 21-11-16, 09:07, Rob Herring wrote:
>>>> On Fri, Nov 18, 2016 at 02:53:12PM +0530, Viresh Kumar wrote:
>>>> > Some platforms have the capability to configure the performance state of
>>>> > their Power Domains. The performance levels are represented by positive
>>>> > integer values, a lower value represents lower performance state.
>>>> >
>>>> > The power-domains until now were only concentrating on the idle state
>>>> > management of the device and this needs to change in order to reuse the
>>>> > infrastructure of power domains for active state management.
>>>> >
>>>> > This patch introduces a new optional property for the consumers of the
>>>> > power-domains: domain-performance-state.
>>>> >
>>>> > If the consumers don't need the capability of switching to different
>>>> > domain performance states at runtime, then they can simply define their
>>>> > required domain performance state in their node directly. Otherwise the
>>>> > consumers can define their requirements with help of other
>>>> > infrastructure, for example the OPP table.
>>>> >
>>>> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
>>>> > ---
>>>> > Documentation/devicetree/bindings/power/power_domain.txt | 6 ++++++
>>>> > 1 file changed, 6 insertions(+)
>>>> >
>>>> > diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
>>>> > index e1650364b296..db42eacf8b5c 100644
>>>> > --- a/Documentation/devicetree/bindings/power/power_domain.txt
>>>> > +++ b/Documentation/devicetree/bindings/power/power_domain.txt
>>>> > @@ -106,6 +106,12 @@ domain provided by the 'parent' power controller.
>>>> > - power-domains : A phandle and PM domain specifier as defined by bindings of
>>>> > the power controller specified by phandle.
>>>> >
>>>> > +Optional properties:
>>>> > +- domain-performance-state: A positive integer value representing the minimum
>>>> > + performance level (of the parent domain) required by the consumer for its
>>>> > + working. The integer value '1' represents the lowest performance level and the
>>>> > + highest value represents the highest performance level.
>>>>
>>>> How does one come up with the range of values?
>>>
>>> Why would we need a range here? The value here represents the minimum 'state'
>>> and the assumption is that everything above that level would be fine. So the
>>> range is automatically: domain-performance-state -> MAX.
>>>
>>>> It seems like you are
>>>> just making up numbers. Couldn't the domain performance level be an OPP
>>>> in the sense that it is a collection of clock frequencies and voltage
>>>> settings?
>>>
>>> The clock is going to be handled by the device itself (at least for the case we
>>> have today) and the performance-state lies with the power-domain which is
>>> configured separately. If the performance level includes both clk and voltage,
>>> then why would we need to show the clock rates in the DT ? Wouldn't a
>>> performance level be enough in such cases?
>>
>> I think the question is: what does the performance-level of a domain
>> actually mean? Or, what are the units?
>>
>> Depending on the SoC, there's probably a few things this could mean. It
>> might mean is that an underlying bus/interconnect can be configured to
>> guarantee a specific bandwidth or throughput. That in turn might mean
>> that that bus/interconnect might have to be set at a specific
>> frequency/voltage.
>>
>> In your case, IIUC, you're just passing some magic value to some
>> firmware running on a micro-controller, but under the hood that uC is
>> probably configuring a frequency/voltage someplace.
>
> In the case described by Viresh, it's only about setting the voltage
> of a power domain that is shared between different devices. these
> devices wants to run at different frequency (set by the devices) but
> we have to select a Volateg value that will match with the constraint
> of all devices (in this case the highest voltage)
Then, at least for this use case, we're talking about voltage, not some
unspecified units.
But that makes me wonder, this performance state sounds like something
that is changing dynamically at runtime, so why do you want to describe
this statically in DT?
This sounds to me like the job of the genpd. When any device in the
domain does its pm_runtime_get(), the domain could check the device
frequency and see if it needs to change the domain voltage in order for
that device to operate at that frequency. When the device goes away
(using pm_runtime_put()) the domain can check again if it could lower
the voltage and still meet the requirements of the remaining devices.
Kevin
^ permalink raw reply
* Re: [PATCH] ALSA SoC MAX98927 driver - Initial release
From: Mark Brown @ 2016-11-23 15:46 UTC (permalink / raw)
To: Ryan Lee
Cc: lgirdwood, robh+dt, mark.rutland, perex, tiwai, arnd, michael,
oder_chiou, yesanishhere, jacob, Damien.Horsley, bardliao,
kuninori.morimoto.gx, petr, lars, nh6z, alsa-devel, devicetree,
linux-kernel
In-Reply-To: <1479877026-5172-1-git-send-email-RyanS.Lee@maximintegrated.com>
[-- Attachment #1: Type: text/plain, Size: 4634 bytes --]
On Wed, Nov 23, 2016 at 01:57:06PM +0900, Ryan Lee wrote:
> +static struct reg_default max98927_reg_map[] = {
> + {0x0014, 0x78},
> + {0x0015, 0xFF},
> + {0x0043, 0x04},
> + {0x0017, 0x55},
> + /* For mono driver we are just enabling one channel*/
If this table contains anything other than the physical defaults the
device has it is broken.
> + {MAX98927_PCM_Rx_Enables_A, 0x03},
> + {MAX98927_PCM_Tx_HiZ_Control_A, 0xfc},
> + {MAX98927_PCM_Tx_HiZ_Control_B, 0xff},
> + {MAX98927_PCM_Tx_Channel_Sources_A, 0x01},
> + {MAX98927_PCM_Tx_Channel_Sources_B, 0x01},
> + {MAX98927_Measurement_DSP_Config, 0xf7},
> + {0x0025, 0x80},
This random mix of strangely formatted #defines and numbers isn't great
- can we please be consistent and ideally use normal style defines?
> +void max98927_wrapper_write(struct max98927_priv *max98927,
> + unsigned int reg, unsigned int val)
> +{
> + if (max98927->regmap)
> + regmap_write(max98927->regmap, reg, val);
> + if (max98927->sub_regmap)
> + regmap_write(max98927->sub_regmap, reg, val);
> +}
I don't really know what this is doing but it looks very confused.
Having multiple regmaps is a bit worrying but even more so is having
some of those regmaps be optional. If the device does sensibly have
multiple register maps I'd really not expect to see them appearing and
disappearing at runtime. Whatever this is doing it at least needs to be
documented.
> + switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
> + case SND_SOC_DAIFMT_CBS_CFS:
> + max98927_wrap_update_bits(max98927, MAX98927_PCM_Master_Mode,
> + MAX98927_PCM_Master_Mode_PCM_MSTR_MODE_Mask,
> + MAX98927_PCM_Master_Mode_PCM_MSTR_MODE_SLAVE);
> + break;
Please use a normal kernel coding style, I can't think of any coding
style where it's normal to indent continuation lines in a multi line
statement like this. There are severe coding style problems throughout
the driver which make it hard to read, it doesn't visually resemble
normal Linux kernel code.
> + case SND_SOC_DAIFMT_IB_NF:
> + invert = MAX98927_PCM_Mode_Config_PCM_BCLKEDGE;
> + break;
> + switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
> + case SND_SOC_DAIFMT_I2S:
> + max98927->iface |= SND_SOC_DAIFMT_I2S;
> + max98927_wrap_update_bits(max98927,
> + MAX98927_PCM_Mode_Config,
> + max98927->iface, max98927->iface);
> + break;
The indentation of the break statements isn't consistent within this
function :( The former is the normal kernel coding style.
> + if (i == ARRAY_SIZE(rate_table)) {
> + pr_err("%s couldn't get the MCLK to match codec\n",
> + __func__);
dev_err() and I'm not sure anyone's going to be able to understand that
error message...
> +static void max98927_handle_pdata(struct snd_soc_codec *codec)
> +{
> + struct max98927_priv *max98927 = snd_soc_codec_get_drvdata(codec);
> + struct max98927_reg_default *regInfo;
> + int cfg_size = 0;
> + int x;
> +
> + if (max98927->regcfg != NULL)
> + cfg_size = max98927->regcfg_sz / sizeof(uint32_t);
> +
> + if (cfg_size <= 0) {
> + dev_dbg(codec->dev,
> + "Register configuration is not required.\n");
> + return;
> + }
> +
> + /* direct configuration from device tree */
> + for (x = 0; x < cfg_size; x += 3) {
> + regInfo = (struct max98927_reg_default *)&max98927->regcfg[x];
> + dev_info(codec->dev, "CH:%d, reg:0x%02x, value:0x%02x\n",
> + be32_to_cpu(regInfo->ch),
> + be32_to_cpu(regInfo->reg),
> + be32_to_cpu(regInfo->def));
> + if (be32_to_cpu(regInfo->ch) == PRI_MAX98927
> + && max98927->regmap)
> + regmap_write(max98927->regmap,
> + be32_to_cpu(regInfo->reg),
> + be32_to_cpu(regInfo->def));
> + else if (be32_to_cpu(regInfo->ch) == SEC_MAX98927
> + && max98927->sub_regmap)
> + regmap_write(max98927->sub_regmap,
> + be32_to_cpu(regInfo->reg),
> + be32_to_cpu(regInfo->def));
> + }
> +}
This also looks like it probably shouldn't be doing whatever it is doing
but needs some documentation.
I've stopped here. In general it seems like this driver needs a *lot*
of work to work with the kernel interfaces in a normal style. Aside
from the coding style issues (which really get in the way) the bulk of
the code appears to be coming from unusual and undocumented ways of
working with kernel APIs. I'd strongly recommend taking a look at other
drivers for similar hardware and making sure that your driver looks like
them textually and structurally. If there are things about your
hardware that mean it needs something unusual then it should be clear to
someone reading the code what's going on.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply
* Re: [PATCH v3 3/4] [media] davinci: vpif_capture: get subdevs from DT
From: Sakari Ailus @ 2016-11-23 15:37 UTC (permalink / raw)
To: Kevin Hilman
Cc: linux-media-u79uwXL29TY76Z2rM5mHXA, Hans Verkuil,
devicetree-u79uwXL29TY76Z2rM5mHXA, Sekhar Nori, Axel Haslam,
Bartosz Gołaszewski, Alexandre Bailon, David Lechner
In-Reply-To: <20161122155244.802-4-khilman-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
Hi Kevin,
On Tue, Nov 22, 2016 at 07:52:43AM -0800, Kevin Hilman wrote:
> Allow getting of subdevs from DT ports and endpoints.
>
> The _get_pdata() function was larely inspired by (i.e. stolen from)
vpif_capture_get_pdata and "largely"?
> am437x-vpfe.c
>
> Signed-off-by: Kevin Hilman <khilman-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> ---
> drivers/media/platform/davinci/vpif_capture.c | 130 +++++++++++++++++++++++++-
> include/media/davinci/vpif_types.h | 9 +-
> 2 files changed, 133 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
> index 94ee6cf03f02..47a4699157e7 100644
> --- a/drivers/media/platform/davinci/vpif_capture.c
> +++ b/drivers/media/platform/davinci/vpif_capture.c
> @@ -26,6 +26,8 @@
> #include <linux/slab.h>
>
> #include <media/v4l2-ioctl.h>
> +#include <media/v4l2-of.h>
> +#include <media/i2c/tvp514x.h>
Do you need this header?
>
> #include "vpif.h"
> #include "vpif_capture.h"
> @@ -650,6 +652,10 @@ static int vpif_input_to_subdev(
>
> vpif_dbg(2, debug, "vpif_input_to_subdev\n");
>
> + if (!chan_cfg)
> + return -1;
> + if (input_index >= chan_cfg->input_count)
> + return -1;
> subdev_name = chan_cfg->inputs[input_index].subdev_name;
> if (subdev_name == NULL)
> return -1;
> @@ -657,7 +663,7 @@ static int vpif_input_to_subdev(
> /* loop through the sub device list to get the sub device info */
> for (i = 0; i < vpif_cfg->subdev_count; i++) {
> subdev_info = &vpif_cfg->subdev_info[i];
> - if (!strcmp(subdev_info->name, subdev_name))
> + if (subdev_info && !strcmp(subdev_info->name, subdev_name))
> return i;
> }
> return -1;
> @@ -1327,6 +1333,21 @@ static int vpif_async_bound(struct v4l2_async_notifier *notifier,
> {
> int i;
>
> + for (i = 0; i < vpif_obj.config->asd_sizes[0]; i++) {
> + struct v4l2_async_subdev *_asd = vpif_obj.config->asd[i];
> + const struct device_node *node = _asd->match.of.node;
> +
> + if (node == subdev->of_node) {
> + vpif_obj.sd[i] = subdev;
> + vpif_obj.config->chan_config->inputs[i].subdev_name =
> + (char *)subdev->of_node->full_name;
> + vpif_dbg(2, debug,
> + "%s: setting input %d subdev_name = %s\n",
> + __func__, i, subdev->of_node->full_name);
> + return 0;
> + }
> + }
> +
> for (i = 0; i < vpif_obj.config->subdev_count; i++)
> if (!strcmp(vpif_obj.config->subdev_info[i].name,
> subdev->name)) {
> @@ -1422,6 +1443,110 @@ static int vpif_async_complete(struct v4l2_async_notifier *notifier)
> return vpif_probe_complete();
> }
>
> +static struct vpif_capture_config *
> +vpif_capture_get_pdata(struct platform_device *pdev)
> +{
> + struct device_node *endpoint = NULL;
> + struct v4l2_of_endpoint bus_cfg;
> + struct vpif_capture_config *pdata;
> + struct vpif_subdev_info *sdinfo;
> + struct vpif_capture_chan_config *chan;
> + unsigned int i;
> +
> + dev_dbg(&pdev->dev, "vpif_get_pdata\n");
> +
> + if (!IS_ENABLED(CONFIG_OF) || !pdev->dev.of_node)
> + return pdev->dev.platform_data;
> +
> + pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
> + if (!pdata)
> + return NULL;
> + pdata->subdev_info =
> + devm_kzalloc(&pdev->dev, sizeof(*pdata->subdev_info) *
> + VPIF_CAPTURE_MAX_CHANNELS, GFP_KERNEL);
> +
> + if (!pdata->subdev_info)
> + return NULL;
> + dev_dbg(&pdev->dev, "%s\n", __func__);
> +
> + for (i = 0; ; i++) {
> + struct device_node *rem;
> + unsigned int flags;
> + int err;
> +
> + endpoint = of_graph_get_next_endpoint(pdev->dev.of_node,
> + endpoint);
> + if (!endpoint)
> + break;
> +
> + sdinfo = &pdata->subdev_info[i];
subdev_info[] has got VPIF_CAPTURE_MAX_CHANNELS entries only.
> + chan = &pdata->chan_config[i];
> + chan->inputs = devm_kzalloc(&pdev->dev,
> + sizeof(*chan->inputs) *
> + VPIF_DISPLAY_MAX_CHANNELS,
> + GFP_KERNEL);
> +
> + chan->input_count++;
> + chan->inputs[i].input.type = V4L2_INPUT_TYPE_CAMERA;
I wonder what's the purpose of using index i on this array as well.
If you use that to access a corresponding entry in a different array, I'd
just create a struct that contains the port configuration and the async
sub-device. The omap3isp driver does that, for instance; see
isp_of_parse_nodes() in drivers/media/platform/omap3isp/isp.c if you're
interested. Up to you.
> + chan->inputs[i].input.std = V4L2_STD_ALL;
> + chan->inputs[i].input.capabilities = V4L2_IN_CAP_STD;
> +
> + /* FIXME: need a new property? ch0:composite ch1: s-video */
> + if (i == 0)
Can you assume that the first endopoint has got a particular kind of input?
What if it's not connected?
If this is a different physical port (not in the meaning another) in the
device, I'd use the reg property for this. Please see
Documentation/devicetree/bindings/media/video-interfaces.txt .
> + chan->inputs[i].input_route = INPUT_CVBS_VI2B;
> + else
> + chan->inputs[i].input_route = INPUT_SVIDEO_VI2C_VI1C;
> + chan->inputs[i].output_route = OUTPUT_10BIT_422_EMBEDDED_SYNC;
> +
> + err = v4l2_of_parse_endpoint(endpoint, &bus_cfg);
> + if (err) {
> + dev_err(&pdev->dev, "Could not parse the endpoint\n");
> + goto done;
> + }
> + dev_dbg(&pdev->dev, "Endpoint %s, bus_width = %d\n",
> + endpoint->full_name, bus_cfg.bus.parallel.bus_width);
> + flags = bus_cfg.bus.parallel.flags;
> +
> + if (flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH)
> + chan->vpif_if.hd_pol = 1;
> +
> + if (flags & V4L2_MBUS_VSYNC_ACTIVE_HIGH)
> + chan->vpif_if.vd_pol = 1;
> +
> + chan->vpif_if.if_type = VPIF_IF_BT656;
> + rem = of_graph_get_remote_port_parent(endpoint);
> + if (!rem) {
> + dev_dbg(&pdev->dev, "Remote device at %s not found\n",
> + endpoint->full_name);
> + goto done;
> + }
> +
> + dev_dbg(&pdev->dev, "Remote device %s, %s found\n",
> + rem->name, rem->full_name);
> + sdinfo->name = rem->full_name;
> +
> + pdata->asd[i] = devm_kzalloc(&pdev->dev,
> + sizeof(struct v4l2_async_subdev),
> + GFP_KERNEL);
Do you ensure somewhere that i isn't overrunning the pdata->asd[] array?
It's got VPIF_CAPTURE_MAX_CHANNELS entries.
> + if (!pdata->asd[i]) {
> + of_node_put(rem);
> + pdata = NULL;
> + goto done;
> + }
> +
> + pdata->asd[i]->match_type = V4L2_ASYNC_MATCH_OF;
> + pdata->asd[i]->match.of.node = rem;
> + of_node_put(rem);
> + }
> +
> +done:
> + pdata->asd_sizes[0] = i;
> + pdata->subdev_count = i;
> + pdata->card_name = "DA850/OMAP-L138 Video Capture";
> +
> + return pdata;
> +}
> +
> /**
> * vpif_probe : This function probes the vpif capture driver
> * @pdev: platform device pointer
> @@ -1438,6 +1563,7 @@ static __init int vpif_probe(struct platform_device *pdev)
> int res_idx = 0;
> int i, err;
>
> + pdev->dev.platform_data = vpif_capture_get_pdata(pdev);
> if (!pdev->dev.platform_data) {
> dev_warn(&pdev->dev, "Missing platform data. Giving up.\n");
> return -EINVAL;
> @@ -1480,7 +1606,7 @@ static __init int vpif_probe(struct platform_device *pdev)
> goto vpif_unregister;
> }
>
> - if (!vpif_obj.config->asd_sizes) {
> + if (!vpif_obj.config->asd_sizes[0]) {
> i2c_adap = i2c_get_adapter(1);
> for (i = 0; i < subdev_count; i++) {
> subdevdata = &vpif_obj.config->subdev_info[i];
> diff --git a/include/media/davinci/vpif_types.h b/include/media/davinci/vpif_types.h
> index 3cb1704a0650..4ee3b41975db 100644
> --- a/include/media/davinci/vpif_types.h
> +++ b/include/media/davinci/vpif_types.h
> @@ -65,14 +65,14 @@ struct vpif_display_config {
>
> struct vpif_input {
> struct v4l2_input input;
> - const char *subdev_name;
> + char *subdev_name;
> u32 input_route;
> u32 output_route;
> };
>
> struct vpif_capture_chan_config {
> struct vpif_interface vpif_if;
> - const struct vpif_input *inputs;
> + struct vpif_input *inputs;
> int input_count;
> };
>
> @@ -83,7 +83,8 @@ struct vpif_capture_config {
> struct vpif_subdev_info *subdev_info;
> int subdev_count;
> const char *card_name;
> - struct v4l2_async_subdev **asd; /* Flat array, arranged in groups */
> - int *asd_sizes; /* 0-terminated array of asd group sizes */
> +
> + struct v4l2_async_subdev *asd[VPIF_CAPTURE_MAX_CHANNELS];
> + int asd_sizes[VPIF_CAPTURE_MAX_CHANNELS];
> };
> #endif /* _VPIF_TYPES_H */
--
Kind regards,
Sakari Ailus
e-mail: sakari.ailus-X3B1VOXEql0@public.gmane.org XMPP: sailus-PCDdDYkjdNMDXYZnReoRVg@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] mfd: cpcap: Add minimal support
From: Tony Lindgren @ 2016-11-23 15:26 UTC (permalink / raw)
To: Lee Jones
Cc: Samuel Ortiz, linux-kernel, linux-omap, devicetree, Marcel Partap,
Mark Rutland, Michael Scott, Rob Herring
In-Reply-To: <20161121114558.GJ32509@dell>
* Lee Jones <lee.jones@linaro.org> [161121 03:43]:
> On Fri, 18 Nov 2016, Tony Lindgren wrote:
> > --- a/drivers/mfd/Makefile
> > +++ b/drivers/mfd/Makefile
> > @@ -97,6 +97,7 @@ obj-$(CONFIG_MFD_MC13XXX_I2C) += mc13xxx-i2c.o
> > obj-$(CONFIG_MFD_CORE) += mfd-core.o
> >
> > obj-$(CONFIG_EZX_PCAP) += ezx-pcap.o
> > +obj-$(CONFIG_MFD_CPCAP) += cpcap.o
>
> Who is the manufacturer?
Hmm that I don't know. There seems to be both ST and TI versions
of this chip manufactured for Motorola. So my guess is that it
should be Motorola unless there's some similar catalog part
available from ST used by others. If anybody has more info
on this please let me know :)
> > + cpcap->vendor = (val >> 6) & 0x0007;
> > + cpcap->revision = ((val >> 3) & 0x0007) | ((val << 3) & 0x0038);
>
> Lots of magic numbers here. I suggest you define them.
I'll check if some earlier code has these defined. Otherwise I'll
just add a comment on the lack of available documentation.
> > + error = cpcap_init_irq_bank(cpcap, 0, 0, 16);
>
> 'ret' is more traditional.
FYI error seems to be preferred over ret as it's meaning is
clear, git grep "error =" drivers/input for example.
I can of course change it if you prefer ret over error.
> > + error = cpcap_init_irq_bank(cpcap, 2, 32, 64);
> > + if (error)
> > + return error;
>
> I don't think I've seen this method of adding bulk IRQ chips before.
> Isn't there a cleaner or generic way to do this?
I'll check.
...
> > +#define CPCAP_REG_LDEB 0x1270 /* LMR Debounce Settings */
> > +#define CPCAP_REG_LGDET 0x1274 /* LMR GCAI Detach Detect */
> > +#define CPCAP_REG_LMISC 0x1278 /* LMR Misc Bits */
> > +#define CPCAP_REG_LMACE 0x127c /* LMR Mace IC Support */
> > +
> > +#define CPCAP_REG_TEST 0x7c00 /* Test */
> > +
> > +#define CPCAP_REG_ST_TEST1 0x7d08 /* ST Test1 */
> > +
> > +#define CPCAP_REG_ST_TEST2 0x7d18 /* ST Test2 */
>
> It would be nice to line up the entire file. #OCD
Hmm care to clarify what you mean here? I think it's lined up with
tabs to line up. I left empty lines where the registers are not
contiguous. What does #OCD mean, Obsessive Compulsive Disorder over
header files maybe? :)
Anywys thanks for the review, the rest of the comments I will just
fix and repost.
Regards,
Tony
^ permalink raw reply
* RE: [PATCH V5 3/3] ARM64 LPC: LPC driver implementation on Hip06
From: Gabriele Paoloni @ 2016-11-23 15:22 UTC (permalink / raw)
To: Arnd Bergmann
Cc: mark.rutland@arm.com, catalin.marinas@arm.com,
linux-pci@vger.kernel.org, liviu.dudau@arm.com, Linuxarm,
lorenzo.pieralisi@arm.com, xuwei (O), Jason Gunthorpe,
T homas Petazzoni, linux-serial@vger.kernel.org,
benh@kernel.crashing.org, devicetree@vger.kernel.org,
minyard@acm.org, will.deacon@arm.com, John Garry, olof@lixom.net,
robh+dt@kernel.org, bhelgaas@go og le.com,
"kantyzc@163.com" <kan>
In-Reply-To: <2359248.XjnRfPbj1B@wuerfel>
Hi Arnd
> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: 23 November 2016 14:16
> To: Gabriele Paoloni
> Cc: linux-arm-kernel@lists.infradead.org; mark.rutland@arm.com;
> benh@kernel.crashing.org; catalin.marinas@arm.com; liviu.dudau@arm.com;
> Linuxarm; lorenzo.pieralisi@arm.com; xuwei (O); Jason Gunthorpe; linux-
> serial@vger.kernel.org; linux-pci@vger.kernel.org;
> devicetree@vger.kernel.org; minyard@acm.org; will.deacon@arm.com; John
> Garry; zourongrong@gmail.com; robh+dt@kernel.org; bhelgaas@go og
> le.com; kantyzc@163.com; zhichang.yuan02@gmail.com; T homas Petazzoni;
> linux-kernel@vger.kernel.org; Yuanzhichang; olof@lixom.net
> Subject: Re: [PATCH V5 3/3] ARM64 LPC: LPC driver implementation on
> Hip06
>
> On Friday, November 18, 2016 5:03:11 PM CET Gabriele Paoloni wrote:
> > > On Friday, November 18, 2016 4:18:07 PM CET Gabriele Paoloni wrote:
> > > > From: Arnd Bergmann [mailto:arnd@arndb.de]
> > > > > On Friday, November 18, 2016 12:53:08 PM CET Gabriele Paoloni
> > > wrote:
> > > > > For the ISA/LPC spaces there are only 4k of addresses, they
> > > > > the bus addresses always overlap, but we can trivially
> > > > > figure out the bus address from Linux I/O port number
> > > > > by subtracting the start of the range.
> > > >
> > > > Are you saying that our LPC controller should specify a
> > > > range property to map bus addresses into a cpu address range?
> > >
> > > No. There is not CPU address associated with it, because it's
> > > not memory mapped.
> > >
> > > Instead, we need to associate a bus address with a logical
> > > Linux port number, both in of_address_to_resource and
> > > in inb()/outb().
> >
> > I think this is effectively what we are doing so far with patch 2/3.
> > The problem with this patch is that we are carving out a "forbidden"
> > IO tokens range that goes from 0 to PCIBIOS_MIN_IO.
> >
> > I think that the proper solution would be to have the LPC driver to
> > set the carveout threshold used in pci_register_io_range(),
> > pci_pio_to_address(), pci_address_to_pio(), but this would impose
> > a probe dependency on the LPC itself that should be probed before
> > the PCI controller (or before any other devices calling these
> > functions...)
>
> Why do you think the order matters? My point was that we should
> be able to register any region of logical port numbers for any
> bus here.
Maybe I have not followed well so let's roll back to your previous
comment...
"we need to associate a bus address with a logical Linux port number,
both in of_address_to_resource and in inb()/outb()"
Actually of_address_to_resource() returns the port number to used
in inb/outb(); inb() and outb() add the port number to PCI_IOBASE
to rd/wr to the right virtual address.
Our LPC cannot operate on the virtual address and it operates on
a bus address range that for LPC is also equal to the cpu address
range and goes from 0 to 0x1000.
Now as I understand it is risky and not appropriate to reserve
the logical port numbers from 0 to 0x1000 or to whatever other
upper bound because existing systems may rely on these port numbers
retrieved by __of_address_to_resource().
In this scenario I think the best thing to do would be
in the probe function of the LPC driver:
1) call pci_register_io_range() passing [0, 0x1000] (that is the
range for LPC)
2) retrieve the logical port numbers associated to the LPC range
by calling pci_address_to_pio() for 0 and 0x1000 and assign
them to extio_ops_node->start and extio_ops_node->end
3) implement the LPC accessors to operate on the logical ports
associated to the LPC range (in practice in the accessors
implementation we will call pci_pio_to_address to retrieve
the cpu address to operate on)
What do you think?
Thanks
Gab
>
>
> > > > > > To be honest with you I would keep things simple for this
> > > > > > LPC and introduce more complex reworks later if more devices
> > > > > > need to be introduced.
> > > > > >
> > > > > > What if we stick on a single domain now where we introduce a
> > > > > > reserved threshold for the IO space (say INDIRECT_MAX_IO).
> > > > >
> > > > > I said having a single domain is fine, but I still don't
> > > > > like the idea of reserving low port numbers for this hack,
> > > > > it would mean that the numbers change for everyone else.
> > > >
> > > > I don't get this much...I/O tokens that are passed to the I/O
> > > > accessors are not fixed anyway and they vary depending on the
> order
> > > > of adding ranges to io_range_list...so I don't see a big issue
> > > > with this...
> > >
> > > On machines with a legacy devices behind the PCI bridge,
> > > there may still be a reason to have the low I/O port range
> > > reserved for the primary bus, e.g. to get a VGA text console
> > > to work.
> > >
> > > On powerpc, this is called the "primary" PCI host, i.e. the
> > > only one that is allowed to have an ISA bridge.
> >
> > Yes but
> > 1) isn't the PCI controller range property that defines how IO bus
> address
> > map into physical CPU addresses?
>
> Correct, but the DT knows nothing about logical port numbers in Linux.
>
> > 2) How can you guarantee that the cpu range associated with this
> > IO bus range is the first to be registered in
> pci_register_io_range()?
> > ( i.e. are you saying that they are just relying on the fact that
> it is the
> > only IO range in the system and by chance the IO tokens and
> corresponding
> > bus addresses are the same? )
>
> To clarify: the special properties of having the first 0x1000 logical
> port numbers go to a particular physical bus are very obscure. I think
> it's more important to not change the behavior for existing systems
> that might rely on it than for new systems that have no such legacy.
>
> The ipmi and uart drivers in particular will get the port numbers
> filled
> in their platform device from the DT bus scanning, so they don't care
> at all about having the same numeric value for port numbers on the bus
> and logical numbers, but other drivers might rely on particular ports
> to be mapped on a specific PCI host, especially when those drivers
> are used only on systems that don't have more than one PCI domain.
>
> Arnd
^ permalink raw reply
* Re: [PATCH] v4l: async: make v4l2 coexists with devicetree nodes in a dt overlay
From: Sakari Ailus @ 2016-11-23 15:10 UTC (permalink / raw)
To: Javi Merino
Cc: linux-media, linux-kernel, devicetree, Pantelis Antoniou,
Mauro Carvalho Chehab, Javier Martinez Canillas, Sakari Ailus
In-Reply-To: <1479895797-7946-1-git-send-email-javi.merino@kernel.org>
Hi Javi,
On Wed, Nov 23, 2016 at 10:09:57AM +0000, Javi Merino wrote:
> In asd's configured with V4L2_ASYNC_MATCH_OF, if the v4l2 subdev is in
> a devicetree overlay, its of_node pointer will be different each time
> the overlay is applied. We are not interested in matching the
> pointer, what we want to match is that the path is the one we are
> expecting. Change to use of_node_cmp() so that we continue matching
> after the overlay has been removed and reapplied.
>
> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
> Cc: Javier Martinez Canillas <javier@osg.samsung.com>
> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> Signed-off-by: Javi Merino <javi.merino@kernel.org>
> ---
> Hi,
>
> I feel it is a bit of a hack, but I couldn't think of anything better.
> I'm ccing devicetree@ and Pantelis because there may be a simpler
> solution.
First I have to admit that I'm not an expert when it comes to DT overlays.
That said, my understanding is that the sub-device and the async sub-device
are supposed to point to the exactly same DT node. I wonder if there's
actually anything wrong in the current code.
If the overlay has changed between probing the driver for the async notifier
and the async sub-device, there should be no match here, should there? The
two nodes actually point to a node in a different overlay in that case.
>
> drivers/media/v4l2-core/v4l2-async.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
> index 5bada20..d33a17c 100644
> --- a/drivers/media/v4l2-core/v4l2-async.c
> +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -42,7 +42,8 @@ static bool match_devname(struct v4l2_subdev *sd,
>
> static bool match_of(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)
> {
> - return sd->of_node == asd->match.of.node;
> + return !of_node_cmp(of_node_full_name(sd->of_node),
> + of_node_full_name(asd->match.of.node));
> }
>
> static bool match_custom(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)
--
Kind regards,
Sakari Ailus
e-mail: sakari.ailus@iki.fi XMPP: sailus@retiisi.org.uk
^ permalink raw reply
* Re: [PATCH] mfd: cpcap: Add minimal support
From: Tony Lindgren @ 2016-11-23 15:03 UTC (permalink / raw)
To: Rob Herring
Cc: Lee Jones, Samuel Ortiz, linux-kernel, linux-omap, devicetree,
Marcel Partap, Mark Rutland, Michael Scott
In-Reply-To: <20161121163439.22ztjxnq535lo6dd@rob-hp-laptop>
* Rob Herring <robh@kernel.org> [161121 08:34]:
> On Mon, Nov 21, 2016 at 11:45:58AM +0000, Lee Jones wrote:
> > On Fri, 18 Nov 2016, Tony Lindgren wrote:
> > > +Example:
> > > +
> > > +&mcspi1 {
> > > + #address-cells = <1>;
> > > + #size-cells = <1>;
> > > + ranges;
> > > + cpcap: pmic@0 {
> > > + compatible = "motorola,cpcap", "st,6556002";
> > > + reg = <0 0>; /* cs0, size 0 */
> >
> > Is this really correct?
>
> No, SPI devices are 1 cell and there shouldn't be a ranges prop.
>
> >
> > How can ranges have a size of 0x8000 and this 0?
>
> reg here doesn't affect ranges and address translation.
>
> Perhaps this is trying to make address translation work, but if that
> does, it is by chance. Children of pmic addresses in the range of
> 0-0x8000 would get translated to "cpu address" 0-0x8000 as long as the
> DT has empty ranges up to the root. If the parent bus (i.e. SoC bus) has
> any base address, then that is going to get added which would not be
> good.
Yes I was thinking we can just get the register offset from the
cpcap base from dts for children. So a range from 0-0x8000.
Rob, do you have any better suggestions for doing that?
Anyways, the children can just rely on regmap too, so I'll just
drop the ranges here. If we ever come up with suitable ranges for
cases like this we can change it later.
Regards,
Tony
^ permalink raw reply
* Re: [PATCH RFC] ARM: dts: add support for Turris Omnia
From: Andrew Lunn @ 2016-11-23 14:59 UTC (permalink / raw)
To: tomas.hlavacek
Cc: Mark Rutland, marex, Jason Cooper, Uwe Kleine-König,
devicetree, Rob Herring, Gregory Clement, linux-arm-kernel,
Sebastian Hesselbarth
In-Reply-To: <1479851991.26813.2@smtp.gmail.com>
> >CZ11NIC12 is indicated on my board.
>
> :-( Well, this board version has wrongly matched length of some
> differential pairs, IRQ from 88E1514 is connected differently, there
> are slight differences in power supplies and (if I am not mistaken)
> something changed in RTC support circuitry. It looks like a huge
> mistake on our side.
Hi Tomas
Would these problems also explain why the Ethernet links to the switch
don't work? Maybe the differential pairs?
> It seems that libphy is probed before pca9538 and we end up with:
> [ 4.217550] libphy: orion_mdio_bus: probed
> [ 4.221777] irq: no irq domain found for
> /soc/internal-regs/i2c@11000/i2cmux@70/i2c@7/gpio@71 !
>
> Any clue where to look in order to defer probing libphy or at least
> orion_mdio_bus?
I think there is a known phylib problem here. Somewhere in the call
chain there is a void function, so the EPROBE_DEFFER gets
discarded. But i could be remembering this wrongly.
Andrew
^ permalink raw reply
* [PATCH v3] iommu/ipmmu-vmsa: Add r8a7796 DT binding
From: Magnus Damm @ 2016-11-23 14:40 UTC (permalink / raw)
To: devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: mark.rutland-5wv7dgnIgG8,
laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw,
geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ, Magnus Damm,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
horms+renesas-/R6kz+dDXgpPR4JQBCEnsQ
From: Magnus Damm <damm+renesas-yzvPICuk2ACczHhG9Qg4qA@public.gmane.org>
Update the IPMMU DT binding documentation to include the r8a7796 compat
string for R-Car M3-W.
Signed-off-by: Magnus Damm <damm+renesas-yzvPICuk2ACczHhG9Qg4qA@public.gmane.org>
Acked-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Acked-by: Simon Horman <horms+renesas-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org>
---
This particular patch seems ready to merge IMO. How to proceed?
Changes since V2:
- Added Acked-by from Rob Herring and Simon Horman - thanks!
Changes since V1:
- Added Acked-by from Laurent - thanks!
Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt | 1 +
1 file changed, 1 insertion(+)
--- 0001/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt
+++ work/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt 2016-06-06 11:27:37.560607110 +0900
@@ -16,6 +16,7 @@ Required Properties:
- "renesas,ipmmu-r8a7793" for the R8A7793 (R-Car M2-N) IPMMU.
- "renesas,ipmmu-r8a7794" for the R8A7794 (R-Car E2) IPMMU.
- "renesas,ipmmu-r8a7795" for the R8A7795 (R-Car H3) IPMMU.
+ - "renesas,ipmmu-r8a7796" for the R8A7796 (R-Car M3-W) IPMMU.
- "renesas,ipmmu-vmsa" for generic R-Car Gen2 VMSA-compatible IPMMU.
- reg: Base address and size of the IPMMU registers.
^ permalink raw reply
* Re: [PATCH v3 3/5] i2c: designware: Add slave definitions
From: Luis Oliveira @ 2016-11-23 14:36 UTC (permalink / raw)
To: Rob Herring, Andy Shevchenko
Cc: Luis Oliveira, wsa, mark.rutland, jarkko.nikula, mika.westerberg,
linux-i2c, devicetree, linux-kernel, Ramiro.Oliveira, Joao.Pinto,
CARLOS.PALMINHA
In-Reply-To: <20161118170155.chrdpguohgl6vo5f@rob-hp-laptop>
OK, I will create a "mode" string property in the devicetree that can be
"master" or "slave".
Thank you all,
Luis
On 18-Nov-16 17:01, Rob Herring wrote:
> On Fri, Nov 18, 2016 at 02:35:52PM +0200, Andy Shevchenko wrote:
>> On Fri, 2016-11-18 at 11:19 +0000, Luis Oliveira wrote:
>>> - Add slave defintitions to i2c-designware-core
>>> - Changes in Kconfig to auto-enable I2C_SLAVE when compiling the
>>> modules
>>> - Add compatible string to designware-core.txt explaining the
>>> devicetree bindings
>>>
>>
>>> --- a/Documentation/devicetree/bindings/i2c/i2c-designware.txt
>>> +++ b/Documentation/devicetree/bindings/i2c/i2c-designware.txt
>>> @@ -2,7 +2,9 @@
>>>
>>> Required properties :
>>>
>>> - - compatible : should be "snps,designware-i2c"
>>> + - compatible : should be:
>>> + - "snps,designware-i2c" to setup the hardware block as I2C master.
>>> + - "snps,designware-i2c-slave" to setup the hardware block as I2C
>>> slave.
>> Not sure about this one.
>>
>> Compatible string is more generic than list of modes. Basically you have
>> to add a property which selects mode.
> Yes, agreed. And come up with a common property.
>
>> DT people's ACK is a must for this change.
>>
>>
>> --- a/drivers/i2c/busses/Kconfig
>>> +++ b/drivers/i2c/busses/Kconfig
>>> @@ -470,6 +470,7 @@ config I2C_DESIGNWARE_CORE
>>> config I2C_DESIGNWARE_PLATFORM
>>> tristate "Synopsys DesignWare Platform"
>>> select I2C_DESIGNWARE_CORE
>>> + select I2C_SLAVE
>>>
>> Common rule, generic dependencies usually go first
>>
>> select I2C_SLAVE
>> select I2C_DESIGNWARE_CORE
>>
>> --
>> Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> Intel Finland Oy
--
Best regards,
Luis
^ permalink raw reply
* Re: [PATCH 0/3] arm64: dts: r8a7796: Add CAN/CAN FD support
From: Marc Kleine-Budde @ 2016-11-23 14:31 UTC (permalink / raw)
To: Simon Horman
Cc: Mark Rutland, devicetree, Chris Paterson, Magnus Damm, linux-can,
linux-renesas-soc, Rob Herring, linux-arm-kernel,
Ramesh Shanmugasundaram, Wolfgang Grandegger
In-Reply-To: <20161123142938.GF9057@verge.net.au>
[-- Attachment #1.1.1: Type: text/plain, Size: 1519 bytes --]
On 11/23/2016 03:29 PM, Simon Horman wrote:
> On Wed, Nov 23, 2016 at 02:18:13PM +0100, Marc Kleine-Budde wrote:
>> On 11/23/2016 01:14 PM, Chris Paterson wrote:
>>> This patch series adds CAN and CAN FD support to the r8a7796.
>>>
>>> Based on renesas-devel-20161122-v4.9-rc6.
>>>
>>> Chris Paterson (3):
>>> arm64: dts: r8a7796: Add CAN external clock support
>>> arm64: dts: r8a7796: Add CAN support
>>> arm64: dts: r8a7796: Add CAN FD support
>>>
>>> .../devicetree/bindings/net/can/rcar_can.txt | 12 +++--
>>> .../devicetree/bindings/net/can/rcar_canfd.txt | 12 +++--
>>> arch/arm64/boot/dts/renesas/r8a7796.dtsi | 61 ++++++++++++++++++++++
>>> 3 files changed, 75 insertions(+), 10 deletions(-)
>>
>> For all three:
>>
>> Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>
>>
>> Who takes this series?
>
> I would like to see these patches split up so that the
> .../devicetree/bindings/ portions can go through you whole
> the arch/arm64/boot/dts/renesas/ portions go thorugh my renesas tree.
Ok
> Regarding the arch/arm64/boot/dts/renesas/ portion, I would like
> some consideration given to what effect enabling memory above 4Gb
> (64bit addressing) would have.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 0/3] arm64: dts: r8a7796: Add CAN/CAN FD support
From: Simon Horman @ 2016-11-23 14:29 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: Mark Rutland, devicetree, Chris Paterson, Magnus Damm, linux-can,
linux-renesas-soc, Rob Herring, linux-arm-kernel,
Ramesh Shanmugasundaram, Wolfgang Grandegger
In-Reply-To: <f358442c-8373-328c-1270-709498c133f5@pengutronix.de>
On Wed, Nov 23, 2016 at 02:18:13PM +0100, Marc Kleine-Budde wrote:
> On 11/23/2016 01:14 PM, Chris Paterson wrote:
> > This patch series adds CAN and CAN FD support to the r8a7796.
> >
> > Based on renesas-devel-20161122-v4.9-rc6.
> >
> > Chris Paterson (3):
> > arm64: dts: r8a7796: Add CAN external clock support
> > arm64: dts: r8a7796: Add CAN support
> > arm64: dts: r8a7796: Add CAN FD support
> >
> > .../devicetree/bindings/net/can/rcar_can.txt | 12 +++--
> > .../devicetree/bindings/net/can/rcar_canfd.txt | 12 +++--
> > arch/arm64/boot/dts/renesas/r8a7796.dtsi | 61 ++++++++++++++++++++++
> > 3 files changed, 75 insertions(+), 10 deletions(-)
>
> For all three:
>
> Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>
>
> Who takes this series?
I would like to see these patches split up so that the
.../devicetree/bindings/ portions can go through you whole
the arch/arm64/boot/dts/renesas/ portions go thorugh my renesas tree.
Regarding the arch/arm64/boot/dts/renesas/ portion, I would like
some consideration given to what effect enabling memory above 4Gb
(64bit addressing) would have.
^ permalink raw reply
* Re: [PATCH 3/3] ARM: dts: sunxi: enable SDIO Wi-Fi on Orange Pi Zero
From: Hans de Goede @ 2016-11-23 14:29 UTC (permalink / raw)
To: Chen-Yu Tsai, Maxime Ripard
Cc: Mark Rutland, devicetree, Vishnu Patekar, Arnd Bergmann,
Jonathan Corbet, Andre Przywara, linux-doc@vger.kernel.org,
Russell King, linux-kernel, Icenowy Zheng, linux-arm-kernel
In-Reply-To: <CAGb2v64taF9x9MDYW+KUEEUUoSx0bF68QNc7uZXQoNrsozMGtg@mail.gmail.com>
Hi,
On 23-11-16 15:25, Chen-Yu Tsai wrote:
> On Wed, Nov 23, 2016 at 3:59 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
>> Hi,
>>
>> On Tue, Nov 22, 2016 at 12:24:21AM +0800, Icenowy Zheng wrote:
>>> There's a Allwinner's XR819 SDIO Wi-Fi module soldered on the board of
>>> Orange Pi Zero, which used a dedicated regulator to power.
>>>
>>> Add the device tree node of the regulator, the enable gpio (with
>>> mmc-pwrseq) and the sdio controller.
>>>
>>> There's a out-of-tree driver tested to work with this device tree.
>>>
>>> Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
>>> ---
>>> New patch in the patchset, since a out-of-tree working xradio driver is done.
>>>
>>> If there is any problem in this patch, it can be omitted.
>>
>> No particular problem with this one, however it can and should be
>> merged with the previous one.
>>
>> Minor comments below though.
>>
>>>
>>> arch/arm/boot/dts/sun8i-h2plus-orangepi-zero.dts | 42 ++++++++++++++++++++++++
>>> 1 file changed, 42 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/sun8i-h2plus-orangepi-zero.dts b/arch/arm/boot/dts/sun8i-h2plus-orangepi-zero.dts
>>> index b428e47..39cac26 100644
>>> --- a/arch/arm/boot/dts/sun8i-h2plus-orangepi-zero.dts
>>> +++ b/arch/arm/boot/dts/sun8i-h2plus-orangepi-zero.dts
>>> @@ -79,6 +79,24 @@
>>> gpios = <&pio 0 17 GPIO_ACTIVE_HIGH>;
>>> };
>>> };
>>> +
>>> + reg_vcc_wifi: reg_vcc_wifi {
>>> + compatible = "regulator-fixed";
>>> + pinctrl-names = "default";
>>> + pinctrl-0 = <&vcc_wifi_pin_opi0>;
>>> + regulator-min-microvolt = <3300000>;
>>> + regulator-max-microvolt = <3300000>;
>>> + regulator-name = "vcc-wifi";
>>> + enable-active-high;
>>> + gpio = <&pio 0 20 GPIO_ACTIVE_HIGH>;
>>> + };
>>> +
>>> + wifi_pwrseq: wifi_pwrseq {
>>> + compatible = "mmc-pwrseq-simple";
>>> + pinctrl-names = "default";
>>> + pinctrl-0 = <&wifi_pwrseq_pin_opi0>;
>>> + reset-gpios = <&r_pio 0 7 GPIO_ACTIVE_LOW>;
>>> + };
>>> };
>>>
>>> &ehci1 {
>>> @@ -95,6 +113,20 @@
>>> status = "okay";
>>> };
>>>
>>> +&mmc1 {
>>> + pinctrl-names = "default";
>>> + pinctrl-0 = <&mmc1_pins_a>;
>>> + vmmc-supply = <®_vcc_wifi>;
>>> + mmc-pwrseq = <&wifi_pwrseq>;
>>> + bus-width = <4>;
>>> + non-removable;
>>> + status = "okay";
>>> +};
>>> +
>>> +&mmc1_pins_a {
>>> + allwinner,pull = <SUN4I_PINCTRL_PULL_UP>;
>>
>> This should be bias-pull-up.
>
> IIRC I already added this for _all_ existing mmc pinmux settings?
>
>>
>>> +};
>>> +
>>> &ohci1 {
>>> status = "okay";
>>> };
>>> @@ -104,6 +136,11 @@
>>> pins = "PA17";
>>> function = "gpio_out";
>>> };
>>> +
>>> + vcc_wifi_pin_opi0: vcc_wifi_pin@0 {
>>> + allwinner,pins = "PA20";
>>
>> This should be pins
>>
>>> + allwinner,function = "gpio_out";
>>
>> This should be function
>>
>>> + };
>>> };
>>>
>>> &r_pio {
>>> @@ -111,6 +148,11 @@
>>> pins = "PL10";
>>> function = "gpio_out";
>>> };
>>> +
>>> + wifi_pwrseq_pin_opi0: wifi_pwrseq_pin@0 {
>>> + allwinner,pins = "PL7";
>>> + allwinner,function = "gpio_out";
>>
>> And same thing here.
>
> Might we do away with the pinmux for gpio pins tradition?
> Recent patches I've sent all omit them.
I'm in favor of doing away with them, except there were
we need to configure bias / strength.
Regards,
Hans
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox