* Re: [PATCH V6] clk: qcom: Add spmi_pmic clock divider support
From: Tirupathi Reddy T @ 2017-11-21 9:12 UTC (permalink / raw)
To: Rob Herring
Cc: sboyd, mturquette, mark.rutland, andy.gross, david.brown,
linux-clk, devicetree, linux-kernel, linux-arm-msm, linux-soc
In-Reply-To: <20171117202243.you76ej4qvk3ul3g@rob-hp-laptop>
On 11/18/2017 1:52 AM, Rob Herring wrote:
> On Fri, Nov 17, 2017 at 03:18:47PM +0530, Tirupathi Reddy wrote:
>> Clkdiv module provides a clock output on the PMIC with CXO as
>> the source. This clock can be routed through PMIC GPIOs. Add
>> a device driver to configure this clkdiv module.
>>
>> Signed-off-by: Tirupathi Reddy <tirupath@codeaurora.org>
>> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> Normally your S-o-B would be last.
Addressed in next patch version [7]
>
>> ---
>> .../bindings/clock/clk-spmi-pmic-div.txt | 59 ++++
> Please split bindings to a separate patch.
Addressed in next patch version [7]
>
> Otherwise,
>
> Acked-by: Rob Herring <robh@kernel.org>
>
>> drivers/clk/qcom/Kconfig | 9 +
>> drivers/clk/qcom/Makefile | 1 +
>> drivers/clk/qcom/clk-spmi-pmic-div.c | 308 +++++++++++++++++++++
>> 4 files changed, 377 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/clock/clk-spmi-pmic-div.txt
>> create mode 100644 drivers/clk/qcom/clk-spmi-pmic-div.c
^ permalink raw reply
* [PATCH v3 08/16] dt-bindings: phy-qcom-qusb2: Update binding for QUSB2 V2 version
From: Manu Gautam @ 2017-11-21 9:23 UTC (permalink / raw)
To: Kishon Vijay Abraham I
Cc: linux-arm-msm, linux-usb, Manu Gautam, Rob Herring, Mark Rutland,
Stephen Boyd, Vivek Gautam,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
In-Reply-To: <1511256206-1587-1-git-send-email-mgautam@codeaurora.org>
Update generic compatible string for QUSB2 V2 PHY. This will allow
all targets using QUSB2 V2 use same string.
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Manu Gautam <mgautam@codeaurora.org>
---
Documentation/devicetree/bindings/phy/qcom-qusb2-phy.txt | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/phy/qcom-qusb2-phy.txt b/Documentation/devicetree/bindings/phy/qcom-qusb2-phy.txt
index aa0fcb0..42c9742 100644
--- a/Documentation/devicetree/bindings/phy/qcom-qusb2-phy.txt
+++ b/Documentation/devicetree/bindings/phy/qcom-qusb2-phy.txt
@@ -4,7 +4,10 @@ Qualcomm QUSB2 phy controller
QUSB2 controller supports LS/FS/HS usb connectivity on Qualcomm chipsets.
Required properties:
- - compatible: compatible list, contains "qcom,msm8996-qusb2-phy".
+ - compatible: compatible list, contains
+ "qcom,msm8996-qusb2-phy" for 14nm PHY on msm8996,
+ "qcom,qusb2-v2-phy" for QUSB2 V2 PHY.
+
- reg: offset and length of the PHY register set.
- #phy-cells: must be 0.
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related
* [PATCH v3 12/16] dt-bindings: phy-qcom-qmp: Update bindings for QMP V3 USB PHY
From: Manu Gautam @ 2017-11-21 9:23 UTC (permalink / raw)
To: Kishon Vijay Abraham I
Cc: linux-arm-msm, linux-usb, Manu Gautam, Rob Herring, Mark Rutland,
Varadarajan Narayanan, Vivek Gautam, Stephen Boyd,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
In-Reply-To: <1511256206-1587-1-git-send-email-mgautam@codeaurora.org>
Update compatible string and clock names for QMP version V3
USB PHY.
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Manu Gautam <mgautam@codeaurora.org>
---
Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt b/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
index b6a9f2b..dcf1b8f 100644
--- a/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
+++ b/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
@@ -8,7 +8,8 @@ Required properties:
- compatible: compatible list, contains:
"qcom,ipq8074-qmp-pcie-phy" for PCIe phy on IPQ8074
"qcom,msm8996-qmp-pcie-phy" for 14nm PCIe phy on msm8996,
- "qcom,msm8996-qmp-usb3-phy" for 14nm USB3 phy on msm8996.
+ "qcom,msm8996-qmp-usb3-phy" for 14nm USB3 phy on msm8996,
+ "qcom,qmp-v3-usb3-phy" for USB3 QMP V3 phy.
- reg: offset and length of register set for PHY's common serdes block.
@@ -25,10 +26,13 @@ Required properties:
- clock-names: "cfg_ahb" for phy config clock,
"aux" for phy aux clock,
"ref" for 19.2 MHz ref clk,
+ "com_aux" for phy common block aux clock,
For "qcom,msm8996-qmp-pcie-phy" must contain:
"aux", "cfg_ahb", "ref".
For "qcom,msm8996-qmp-usb3-phy" must contain:
"aux", "cfg_ahb", "ref".
+ For "qcom,qmp-v3-usb3-phy" must contain:
+ "aux", "cfg_ahb", "ref", "com_aux".
- resets: a list of phandles and reset controller specifier pairs,
one for each entry in reset-names.
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related
* [PATCH 1/1] arm64: dts: ls1012a: Add LS1012A-2G5RDB board support
From: Bhaskar Upadhaya @ 2017-11-21 9:45 UTC (permalink / raw)
To: devicetree-u79uwXL29TY76Z2rM5mHXA,
shawnguo-DgEjT+Ai2ygdnm+yROfE0A
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Bhaskar Upadhaya
LS1012A-2G5RDB is similar to LS1012ARDB with below features
2 2.5G SGMII PFE Block, SATA, USB 2.0/3.0, WiFi, DDR, eMMC
QuadSPI, UART
Signed-off-by: Bhaskar Upadhaya <Bhaskar.Upadhaya-3arQi8VN3Tc@public.gmane.org>
---
arch/arm64/boot/dts/freescale/Makefile | 1 +
.../boot/dts/freescale/fsl-ls1012a-2g5rdb.dts | 66 ++++++++++++++++++++++
2 files changed, 67 insertions(+)
create mode 100644 arch/arm64/boot/dts/freescale/fsl-ls1012a-2g5rdb.dts
diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
index 72c4b52..b3a0aec 100644
--- a/arch/arm64/boot/dts/freescale/Makefile
+++ b/arch/arm64/boot/dts/freescale/Makefile
@@ -1,6 +1,7 @@
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-ls1012a-2g5rdb.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-2g5rdb.dts b/arch/arm64/boot/dts/freescale/fsl-ls1012a-2g5rdb.dts
new file mode 100644
index 0000000..99b1835
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1012a-2g5rdb.dts
@@ -0,0 +1,66 @@
+/*
+ * Device Tree file for NXP LS1012A 2G5RDB Board.
+ *
+ * Copyright 2017 NXP
+ *
+ * Bhaskar Upadhaya <bhaskar.upadhaya-3arQi8VN3Tc@public.gmane.org>
+ *
+ * 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 2G5RDB Board";
+ compatible = "fsl,ls1012a-rdb", "fsl,ls1012a";
+
+};
+
+&duart0 {
+ status = "okay";
+};
+
+&i2c0 {
+ status = "okay";
+};
+
+&sata {
+ status = "okay";
+};
--
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
* Re: [PATCH v2 4/4] drm: omapdrm: Fix DPI on platforms using the DSI VDDS
From: Tomi Valkeinen @ 2017-11-21 10:25 UTC (permalink / raw)
To: H. Nikolaus Schaller, Thierry Reding, David Airlie, Rob Herring,
Mark Rutland, Benoît Cousson, Tony Lindgren, Russell King,
Bartlomiej Zolnierkiewicz, Laurent Pinchart, Julia Lawall,
Sean Paul
Cc: devicetree, linux-fbdev, letux-kernel, linux-kernel, dri-devel,
kernel, linux-omap, linux-arm-kernel
In-Reply-To: <dd2427920427440e051951071377265fc0a830d4.1510822218.git.hns@goldelico.com>
On 16/11/17 10:50, H. Nikolaus Schaller wrote:
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> Commit d178e034d565 ("drm: omapdrm: Move FEAT_DPI_USES_VDDS_DSI feature
> to dpi code") replaced usage of platform data version with SoC matching
> to configure DPI VDDS. The SoC match entries were incorrect, they should
> have matched on the machine name instead of the SoC family. Fix it.
>
> The result was observed on OpenPandora with OMAP3530 where the panel only
> had the Blue channel and Red&Green were missing. It was not observed on
> GTA04 with DM3730.
>
> Fixes: d178e034d565 ("drm: omapdrm: Move FEAT_DPI_USES_VDDS_DSI feature to dpi code")
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reported-by: H. Nikolaus Schaller <hns@goldelico.com>
> Tested-by: H. Nikolaus Schaller <hns@goldelico.com>
> ---
> drivers/gpu/drm/omapdrm/dss/dpi.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/dss/dpi.c b/drivers/gpu/drm/omapdrm/dss/dpi.c
> index 4ed5fde11313..a91e5f1a0490 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dpi.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dpi.c
> @@ -566,8 +566,8 @@ static int dpi_verify_pll(struct dss_pll *pll)
> }
>
> static const struct soc_device_attribute dpi_soc_devices[] = {
> - { .family = "OMAP3[456]*" },
> - { .family = "[AD]M37*" },
> + { .machine = "OMAP3[456]*" },
> + { .machine = "[AD]M37*" },
> { /* sentinel */ }
> };
>
>
I have picked this one. I think the rest of the patches are more of a
cleanup, right? And you'll be sending v3 at some point.
Tomi
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH v2 4/4] drm: omapdrm: Fix DPI on platforms using the DSI VDDS
From: H. Nikolaus Schaller @ 2017-11-21 10:38 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Thierry Reding, David Airlie, Rob Herring, Mark Rutland,
Benoît Cousson, Tony Lindgren, Russell King,
Bartlomiej Zolnierkiewicz, Laurent Pinchart, Julia Lawall,
Sean Paul, dri-devel, devicetree, linux-kernel, linux-omap,
linux-arm-kernel, linux-fbdev, letux-kernel, kernel
In-Reply-To: <7f34033e-4af7-f5f5-47ef-759249ddfe39@ti.com>
Hi,
> Am 21.11.2017 um 11:25 schrieb Tomi Valkeinen <tomi.valkeinen@ti.com>:
>
> On 16/11/17 10:50, H. Nikolaus Schaller wrote:
>> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>
>> Commit d178e034d565 ("drm: omapdrm: Move FEAT_DPI_USES_VDDS_DSI feature
>> to dpi code") replaced usage of platform data version with SoC matching
>> to configure DPI VDDS. The SoC match entries were incorrect, they should
>> have matched on the machine name instead of the SoC family. Fix it.
>>
>> The result was observed on OpenPandora with OMAP3530 where the panel only
>> had the Blue channel and Red&Green were missing. It was not observed on
>> GTA04 with DM3730.
>>
>> Fixes: d178e034d565 ("drm: omapdrm: Move FEAT_DPI_USES_VDDS_DSI feature to dpi code")
>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> Reported-by: H. Nikolaus Schaller <hns@goldelico.com>
>> Tested-by: H. Nikolaus Schaller <hns@goldelico.com>
>> ---
>> drivers/gpu/drm/omapdrm/dss/dpi.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/omapdrm/dss/dpi.c b/drivers/gpu/drm/omapdrm/dss/dpi.c
>> index 4ed5fde11313..a91e5f1a0490 100644
>> --- a/drivers/gpu/drm/omapdrm/dss/dpi.c
>> +++ b/drivers/gpu/drm/omapdrm/dss/dpi.c
>> @@ -566,8 +566,8 @@ static int dpi_verify_pll(struct dss_pll *pll)
>> }
>>
>> static const struct soc_device_attribute dpi_soc_devices[] = {
>> - { .family = "OMAP3[456]*" },
>> - { .family = "[AD]M37*" },
>> + { .machine = "OMAP3[456]*" },
>> + { .machine = "[AD]M37*" },
>> { /* sentinel */ }
>> };
>>
>>
>
> I have picked this one.
Fine.
> I think the rest of the patches are more of a
> cleanup, right? And you'll be sending v3 at some point.
Yes. Should we wait for more comments or should I send now?
BR and thanks,
Nikolaus Schaller
>
> Tomi
>
> --
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply
* Re: [patches] Re: [PATCH] dt-bindings: Add a RISC-V SBI firmware node
From: Mark Rutland @ 2017-11-21 10:43 UTC (permalink / raw)
To: Palmer Dabbelt
Cc: j.neuschaefer-hi6Y0CQ0nG0, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
devicetree-u79uwXL29TY76Z2rM5mHXA, patches-q3qR2WxjNRFS9aJRtSZj7A,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <mhng-4a3ed3c5-a562-427c-817e-cd897789d5c0@palmer-si-x1c4>
Hi Palmer,
On Mon, Nov 20, 2017 at 01:28:01PM -0800, Palmer Dabbelt wrote:
> On Mon, 20 Nov 2017 12:28:56 PST (-0800), j.neuschaefer-hi6Y0CQ0nG0@public.gmane.org wrote:
> > On Mon, Nov 20, 2017 at 11:50:00AM -0800, Palmer Dabbelt wrote:
> > > +RISC-V Supervisor Binary Interface (SBI)
> > > +
> > > +The RISC-V privileged ISA specification mandates the presence of a supervisor
> > > +binary interface that performs some operations which might otherwise require
> > > +particularly complicated instructions. This interface includes
> > > +inter-processor interrupts, TLB flushes, i-cache and TLB shootdowns, a
> > > +console, and power management.
> > > +
> > > +Required properties:
> > > +- compatible: must contain one of the following
> > > + * "riscv,sbi" for the SBI defined by the privileged specification of the
> > > + system.
> >
> > "of the system" seems to imply that different RISC-V systems (different
> > RISC-V implementations) can have different privileged specifications.
>
> Actually, that was intentional -- I wrote it this way because different
> RISC-V systems do have different privileged specifications. The RISC-V
> specifications aren't frozen in time, they're just guaranteed to be
> compatible in the future.
If that's the case, then you can define a version of the document that
is a baseline. e.g.
* "riscv,sbi" for an SBI implementation compatible with that defined
in $XYZ_DOCUMENT version $N
If every new feature can be probed from that point onwards, then that's
all you'll ever need. Otherwise, if there are backwards-incompatible
changes or non-probeable features, you can add additional strings, and
there's no ambiguity.
See Documentation/devicetree/bindings/arm/psci.txt for an similar
example on ARM systems. That's explicitly versioned, though we don't
list each and every document number, and we probably should.
Thanks,
Mark.
--
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] dt-bindings: Add an enable method to RISC-V
From: Mark Rutland @ 2017-11-21 11:04 UTC (permalink / raw)
To: Palmer Dabbelt
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA,
patches-q3qR2WxjNRFS9aJRtSZj7A,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171120195022.2307-1-palmer-SpMDHPYPyPbQT0dZR+AlfA@public.gmane.org>
Hi Palmer,
On Mon, Nov 20, 2017 at 11:50:22AM -0800, Palmer Dabbelt wrote:
> RISC-V doesn't currently specify a mechanism for enabling or disabling
> CPUs. Instead, we assume that all CPUs are enabled on boot, and if
> someone wants to save power we instead put a CPU to sleep via a WFI
> loop.
>
> This patch adds "enable-method" to the RISC-V CPU binding, which
> currently only has the value "none". This allows us to change the
> enable method in the future.
I think you might want to be a bit more explicit about what this means,
and this could do with a better name, as "none" sounds like the CPU is
unusable, rather than it having been placed within the kernel already by
the FW/bootloader (which IIUC is what happens currently).
As previosuly commented, I also really think you'll want to define a
simple boot protocol (like PPC spin-table) whereby the kernel can bring
each CPU into the kernel independently. That will save you a lot of pain
in future with things like kexec, suspend/resume, etc.
For arm64 we had a spin-table clone (implemented in our boot-wrapper
firmware) that allowed us to bring CPUs into the kernel explicitly.
However, we made the mistake of allowing CPUs to share a mailbox, and we
couldn't tell how many CPUs were stuck in the kernel at any point in
time (rendering kexec, suspend, etc impossible).
Thanks,
Mark.
> CC: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> Signed-off-by: Palmer Dabbelt <palmer-SpMDHPYPyPbQT0dZR+AlfA@public.gmane.org>
> ---
> Documentation/devicetree/bindings/riscv/cpus.txt | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/riscv/cpus.txt b/Documentation/devicetree/bindings/riscv/cpus.txt
> index adf7b7af5dc3..dd9e1ae197e2 100644
> --- a/Documentation/devicetree/bindings/riscv/cpus.txt
> +++ b/Documentation/devicetree/bindings/riscv/cpus.txt
> @@ -82,6 +82,11 @@ described below.
> Value type: <string>
> Definition: Contains the RISC-V ISA string of this hart. These
> ISA strings are defined by the RISC-V ISA manual.
> + - cpu-enable-method:
> + Usage: required
> + Value type: <stringlist>
> + Definition: Must be one of
> + "none": This CPU's state cannot be changed.
>
> Example: SiFive Freedom U540G Development Kit
> ---------------------------------------------
> @@ -105,6 +110,7 @@ Linux is allowed to run on.
> reg = <0>;
> riscv,isa = "rv64imac";
> status = "disabled";
> + enable-method = "none";
> L10: interrupt-controller {
> #interrupt-cells = <1>;
> compatible = "riscv,cpu-intc";
> @@ -130,6 +136,7 @@ Linux is allowed to run on.
> reg = <1>;
> riscv,isa = "rv64imafdc";
> status = "okay";
> + enable-method = "none";
> tlb-split;
> L13: interrupt-controller {
> #interrupt-cells = <1>;
> --
> 2.13.6
>
--
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 3/3] iio: light: isl76683 add way to adjust irq threshold
From: Christoph Fritz @ 2017-11-21 11:10 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Peter Meerwald-Stadler, Rob Herring,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171119112625.073f5d66@archlinux>
Hi Jonathan,
thanks for your input. Please see my questions and answers below.
On Sun, 2017-11-19 at 11:26 +0000, Jonathan Cameron wrote:
> On Sun, 19 Nov 2017 00:20:30 +0100
> Christoph Fritz <chf.fritz-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> wrote:
>
> > This patch adds sysfs read/write support for upper and lower irq
> > thresholds. So it's possible that only on certain lux ranges the
> > irq triggered measurement happens.
> >
> > Signed-off-by: Christoph Fritz <chf.fritz-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
>
> hmm, this is 'unusual' to say the least...
>
> From the datasheet it initially looks like a straight forward threshold
> interrupt - which should be supported as an IIO event.
>
> However, there seems to not be a generic monitoring mode, but rather the
> device has to be polled? (which makes this a 'funny' sort of interrupt..)
It's not polling, it's just that in this "external timing mode" host has
to do one part of adc integration timing (accurate waiting) on its own.
This is suboptimal because doing timing in the driver cannot be that
accurate as the external osc beside the chip with its "internal timing
mode". To compensate this inaccuracy there are Timing-Registers which
would then need to be red and finally calculated too.
So I prefer "internal timing mode" (with IRQ) because I do get data as
accurate and as fast as possible (especially in buffered mode) without
the hassle of compensation.
>
> So I think you are ultimately using this threshold interrupt to provide
> a dataready signal when there isn't a real one provided?
I don't get this point. Why shouldn't the IRQ be a real data-ready
signal?
The usage is this:
set threshold,
in a loop for example(
clear IRQ isl76683_start_measurement()
now wait for IRQ which triggers when sensordata passes threshold
read the data isl76683_get_sensordata()
)
What may confuse is that this chip needs to get the IRQ cleared before
a next "data-ready-because-it-passed-threshold-IRQ"?
By the way, without this patch sensordata always passes threshold and
the IRQ is a real "data-ready-IRQ".
>
> That's horrible and makes it very hard to fit this device into standard
> frameworks. My gut feeling would be to:
If you don't like the adjustable threshold because you really feel it
doesn't fit into iio-framework, you can purge patch 3 and I'll keep it
away from mainline. And it would be great if you could
reconsider :-) ...
>
> * stop using the interrupt for data ready at all, but dead reckon
> that with a timer delay.
Please see my points above why using "internal timing mode" adds
complexity.
> * use this 'interrupt' (actually a hardware threshold signal rather than
> an interrupt really)
Please see above: Why shouldn't the IRQ be a real data-ready signal?
> for event detection and handle it
> as an event with all the standard infrastructure that is in place
> for that.
>
> I can see the hardware designers logic that you might only want to read
> the values back when the light level has changed from your expected value,
> but given you have to manually trigger readings, the utility of this is
> somewhat limited...
No, adc readings are done continuously inside the chip if sensor value
fails threshold test. You get an IRQ if light changes so that threshold
test gets passed.
What do you think?
Thanks
-- Christoph
^ permalink raw reply
* Re: [PATCH v2 1/3] clk: hisilicon: add CRG driver Hi3521A SoC
From: Marty E. Plummer @ 2017-11-21 11:56 UTC (permalink / raw)
To: Rob Herring
Cc: linux-arm-kernel, mturquette, sboyd, mark.rutland, xuejiancheng,
zhangfei.gao, wnpan, linux-kernel, linux-clk, devicetree, xuwei5,
linux
In-Reply-To: <20171024184250.d4wmzr6osd4o53oz@rob-hp-laptop>
On Tue, Oct 24, 2017 at 01:42:50PM -0500, Rob Herring wrote:
> On Tue, Oct 17, 2017 at 05:38:52PM -0500, Marty E. Plummer wrote:
> > Add CRG driver for Hi3521A SoC. CRG (Clock and Reset Generator) module
> > generates clock and reset signals used by other module blocks on SoC.
> >
> > Signed-off-by: Marty E. Plummer <hanetzer@startmail.com>
> > ---
> > Changes in v2:
> > - Switched to SPDX tags and GPL-2.0+
> >
> > drivers/clk/hisilicon/Kconfig | 7 ++
> > drivers/clk/hisilicon/Makefile | 1 +
> > drivers/clk/hisilicon/crg-hi3521a.c | 196 ++++++++++++++++++++++++++++++
>
> > include/dt-bindings/clock/hi3521a-clock.h | 23 ++++
>
> Acked-by: Rob Herring <robh@kernel.org>
Actually nack this for now. I need to change some stuff over to use a
different clock for the sp804 timer@12000000, apparently I'm going to
need to use CLK_OF_DECLARE to get the clock in question working that
early in boot.
^ permalink raw reply
* Re: RFC: Copying Device Tree File into reserved area of VMLINUX before deployment
From: Ulf Samuelsson @ 2017-11-21 12:02 UTC (permalink / raw)
To: Frank Rowand, LKML,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Rob Herring
In-Reply-To: <4d2f0bf4-d49a-27bc-75aa-11c78bc074fa-AoFPY8dbyRPQT0dZR+AlfA@public.gmane.org>
On 2017-11-21 07:19, Ulf Samuelsson wrote:
>
>
> On 2017-11-21 00:09, Ulf Samuelsson wrote:
>>
>>
>> On 2017-11-20 22:39, Frank Rowand wrote:
>>> Hi Ulf, Rob,
>>>
>>> On 11/20/17 15:19, Ulf Samuelsson wrote:
>>>>
>>>>
>>>> On 2017-11-20 05:32, Frank Rowand wrote:
>>>>> Hi Ulf,
>>>>>
>>>>>
>>>>> On 11/19/17 23:23, Frank Rowand wrote:
>>>>>> adding devicetree list, devicetree maintainers
>>>>>>
>>>>>> On 11/18/17 12:59, Ulf Samuelsson wrote:
>>>>>>> I noticed when checking out the OpenWRT support for the board
>>>>>>> that they have a method to avoid having to pass the device tree
>>>>>>> address to the kernel, and can thus boot device tree based
>>>>>>> kernels with U-boots that
>>>>>>> does not support device trees.
>>>>>>>
>>>>>>> Is this something that would be considered useful for including
>>>>>>> in mainstream:
>>>>>>>
>>>>>>> BACKGROUND:
>>>>>>> Trying to load a yocto kernel into a MIPS target (MT7620A based),
>>>>>>> and the U-Boot is more than stupid.
>>>>>>> Does not support the "run" command as an example.
>>>>>>> They modified the U-Boot MAGIC Word to complicate things.
>>>>>>> The U-Boot is not configured to use device tree files.
>>>>>>> The board runs a 2.6 kernel right now.
>>>>>>>
>>>>>>> Several attempts by me a and others to rebuild U-Boot according to
>>>>>>> the H/W vendors source code and build instructions results in a
>>>>>>> bricked unit. Bricked units cannot be recovered.
>>>>>
>>>>> Hopefully you have brought this to the attention of the vendor.
>>>>> U-Boot
>>>>> is GPL v2 (or in some ways possibly GPL v2 or later), so if you can
>>>>> not
>>>>> build U-Boot that is equivalent to the binary U-Boot they shipped, the
>>>>> vendor may want to ensure that they are shipping the proper source and
>>>>> build instructions.
>>>>>
>>>>
>>>> I am not the one in contact with the H/W vendor.
>>>> The U-boot is pretty old, and from comments from those
>>>> in contact with them, the U-Boot knowledge at the H/W vendor
>>>> is minimal at best.
>>>> It might even be that they program an U-boot where the upgrade of
>>>> the U-boot is broken...
>>>>
>>>>
>>>>>
>>>>>>> Not my choice of H/W, so I cannot change it.
>>>>>>>
>>>>>>>
>>>>>>> ===================================================================
>>>>>>> OPENWRT:
>>>>>>> I noticed when checking out the OpenWRT support for the board that
>>>>>>> they have a method to avoid having to pass the device tree address
>>>>>>> to the kernel, and can thus boot device tree based kernels with
>>>>>>> U-boots that does not support device trees.
>>>>>>>
>>>>>>> What they do is to reserve 16 kB of kernel space, and tag it with
>>>>>>> an ASCII string "OWRTDTB:". After the kernel and dtb is built, a
>>>>>>> utility "patch-dtb" will update the vmlinux binary, copying in the
>>>>>>> device tree file.
>>>>>>>
>>>>>>> ===================================================================
>>>>>>> It would be useful to me, and I could of course patch the
>>>>>>> mainstream kernel, but first I would like to check if this is of
>>>>>>> interest for mainstream.
>>>>>
>>>>> Not in this form. Hard coding a fixed size area in the boot image
>>>>> to contain the FDT (aka DTB) is a non-starter.
>>>>
>>>> OK, Is it the fixed size, which is a problem?
>>>
>>> Yes, it is the fixed size which is a problem.
>>
>> The size can of course be changed, by setting the size configuration
>> option (DTB_SIZE).
>> OpenWRT does not support that, but I think it needs to be there for a
>> generic option (but You have to recompile the kernel to increase the
>> size).
>>
>> One problem is that you normally compile and link the kernel before you
>> compile the dtbs, so you do not know what size is until afterwards.
>>
> Found this link: https://csl.name/post/embedding-binary-data/
>
> =======================================
> ...
> Let's say you have an image target.dtb and want to embed it into your
> application. You can create an object file with
>
>
> image_dtb.o: <target dtb>
> mv <target dtb> image_dtb
> ld -r -b binary image_dtb -o image_dtb.o
>
> The object file will have three symbols in it,
>
> $ nm cat.o
> 0000000000000512 D _binary_image_dtb_end
> 0000000000000512 A _binary_image_dtb_size
> 0000000000000000 D _binary_image_dtb_start
> =======================================
>
> This assumes that the dtbs are built before the kernel is linked.
> The copy step is neccessary, since the generated names are
> taken from the name of the "in file".
> (Would have been better, if they used the "out file")
>
> Otherwise you can create an assembler file which "incbin's" the dtb file.
>
Just checked the kernel source, and it appears that the discussion is
somewhat redundant, since the support is already in the linux kernel
for some MIPS boards
arch/mips/Kconfig:
config MIPS_ELF_APPENDED_DTB
bool "vmlinux"
help
With this option, the boot code will look for a device tree binary
DTB) included in the vmlinux ELF section .appended_dtb. By default
it is empty and the DTB can be appended using binutils command
objcopy:
objcopy --update-section .appended_dtb=<filename>.dtb vmlinux
This is meant as a backward compatiblity convenience for those
systems with a bootloader that can't be upgraded to accommodate
the documented boot protocol using a device tree.
arch/mips/kernel/setup.c:
#ifdef CONFIG_MIPS_ELF_APPENDED_DTB
const char __section(.appended_dtb) __appended_dtb[0x100000];
#endif /* CONFIG_MIPS_ELF_APPENDED_DTB */
arch/mips/bmips/setup.c
#ifdef CONFIG_MIPS_ELF_APPENDED_DTB
if (!fdt_check_header(&__appended_dtb))
dtb = (void *)&__appended_dtb;
else
#endif
>
>
>
>>
>>>
>>>> Is generally combining an image with a DTB into a single file also a
>>>> non-starter?
>>>
>>> Can you jump in here Rob? My understanding is that
>>> CONFIG_ARM_APPENDED_DTB,
>>> which is the ARM based solution that Mark mentioned, was envisioned as a
>>> temporary stop gap until boot loaders could add devicetree support.
>>> I don't
>>> know if there is a desire to limit this approach or to remove it in the
>>> future.
>>>
>>> I'm not sure why this feature should not be permanently supported.
>>> I'm being
>>> cautious, just in case I'm overlooking or missing an important issue,
>>> thus
>>> asking for Rob's input. I do know that this feature does not advance
>>> the
>>> desires of people who want a single kernel (single boot image?) that
>>> runs on
>>> many different systems, instead of a boot image that is unique to each
>>> target platform. But I don't see why that desire precludes also having
>>> an option to have a target specific boot image.
>> The main reason to keep it is when you are really constrained for memory.
>> The U-Boot on the board is 96 kB, which is just a fraction of a more
>> normal U-Boot.
>> Also, the u-boot is old.
>>
>>
>>>
>>> -Frank
>>>
>>>
>>>>>
>>>>> And again, I would first approach the H/W vendor before trying to
>>>>> come up with a work around like this.
>>>>>
>>>>>
>>>>>>> I envisage the support would look something like:
>>>>>>>
>>>>>>> ============
>>>>>>> Kconfig.
>>>>>>> config MIPS
>>>>>>> select HAVE_IMAGE_DTB
>>>>>>>
>>>>>>> config HAVE_IMAGE_DTB
>>>>>>> bool
>>>>>>>
>>>>>>> if HAVE_IMAGE_DTB
>>>>>>> config IMAGE_DTB
>>>>>>> bool "Allocated space for DTB within image
>>>>>>>
>>>>>>> config DTB_SIZE
>>>>>>> int "DTB space (kB)
>>>>>>>
>>>>>>> config DTB_TAG
>>>>>>> string "DTB space tag"
>>>>>>> default "OWRTDTB:"
>>>>>>> endif
>>>>>>>
>>>>>>> ============
>>>>>>> Some Makefile
>>>>>>> obj-$(CONFIG_INCLUDE_DTB) += image_dtb.o
>>>>>>>
>>>>>>> ============
>>>>>>> image_dtb.S:
>>>>>>> .text
>>>>>>> .align 5
>>>>>>> .ascii CONFIG_DTB_TAG
>>>>>>> EXPORT(__image_dtb)
>>>>>>> .fill DTB_SIZE * 1024
>>>>>>>
>>>>>>> ===================
>>>>>>> arch/mips/xxx/of.c:
>>>>>>>
>>>>>>> #if defined(CONFIG_IMAGE_DTB)
>>>>>>> if (<conditions to boot from dtb_space>)
>>>>>>> __dt_setup_arch(__dtb_start);
>>>>>>> else
>>>>>>> __dt_setup_arch(&__image_dtb);
>>>>>>> #else
>>>>>>> __dt_setup_arch(__dtb_start);
>>>>>>> #endif
>>>>>>>
>>>>>>> I imagine that if the support is enabled for a target, it should
>>>>>>> be possible to override it with a CMDLINE argument
>>>>>>> They do something similar for the CMDLINE; copying it
>>>>>>> into the vmlinux, to allow a smaller boot
>>>>
>>>
>>
>
--
Best Regards
Ulf Samuelsson
--
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] ARM: dts: bcm283x: Fix fifo size for EP 6,7
From: Minas Harutyunyan @ 2017-11-21 12:02 UTC (permalink / raw)
To: Stefan Wahren, Minas Harutyunyan
Cc: John Youn, Eric Anholt, Phil Elwell,
linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Rob Herring,
Florian Fainelli, Mark Rutland,
linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <56581c3c-9159-8e07-288b-5ccc793e1fa1@i2se.com>
[-- Attachment #1: Type: text/plain, Size: 831 bytes --]
On 11/20/2017 4:48 PM, Stefan Wahren wrote:
> Hi Minas,
>
> Am 20.11.2017 um 12:59 schrieb Minas Harutyunyan:
>> Hi Stefan,
>> Looks like I know cause of issue... but I'm overloaded today and will able to check it tomorrow. Sorry for delay.
>
> thanks for your reply. There is no need to hurry, because the merge
> window isn't open yet, but i like to get this fixed for the next kernel
> release.
>
> I have the suspicion this is related to the fact that u-boot already
> initialize the USB IP, before dwc2 driver is loaded.
>
> Regards
> Stefan
>
>>
>> Thanks,
>> Minas
>>
>
>
Hi Stefan,
We have prepared patch for this issue in July-August'17.
Find attached 2 patch files. Please apply patches and test. If issue
gone, we will send these patches to LKML by regular flow.
Thanks,
Minas
[-- Attachment #2: 8eff278.diff --]
[-- Type: text/plain, Size: 2553 bytes --]
From 8eff278552aaf248161df9af04f8b93f03526bcd Mon Sep 17 00:00:00 2001
From: Gevorg Sahakyan <sahakyan@synopsys.com>
Date: Mon, 31 Jul 2017 17:19:51 +0400
Subject: [PATCH] usb: dwc2: Fix TxFIFO setup issue
In host mode reading from DPTXSIZn returning invalid value(0) in
dwc2_check_param_tx_fifo_sizes function.
Added g_tx_fifo_size array in dwc2_hw_params structure in which stored
power on reset valus of DPTXSIZn registers in device mode (forced to
device).
Updated dwc2_get_hwparams function to write DPTXFSIZn to array.
Modyfied dwc2_check_param_tx_fifo_sizes function accordingly.
Change-Id: I61d3db753b1bc06f0f2caf40df350a09655f18fd
Signed-off-by: Gevorg Sahakyan <sahakyan@synopsys.com>
---
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 5029dde..3b71b49 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -533,6 +533,7 @@
unsigned utmi_phy_data_width:2;
u32 snpsid;
u32 dev_ep_dirs;
+ u32 g_tx_fifo_size[MAX_EPS_CHANNELS];
};
/* Size of control and EP0 buffers */
diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index 701516e..79080a9 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -452,7 +452,6 @@
int fifo;
int min;
u32 total = 0;
- u32 dptxfszn;
fifo_count = dwc2_hsotg_tx_fifo_count(hsotg);
min = hsotg->hw_params.en_multiple_tx_fifo ? 16 : 4;
@@ -467,15 +466,15 @@
}
for (fifo = 1; fifo <= fifo_count; fifo++) {
- dptxfszn = (dwc2_readl(hsotg, DPTXFSIZN(fifo)) &
- FIFOSIZE_DEPTH_MASK) >> FIFOSIZE_DEPTH_SHIFT;
if (hsotg->params.g_tx_fifo_size[fifo] < min ||
- hsotg->params.g_tx_fifo_size[fifo] > dptxfszn) {
+ hsotg->params.g_tx_fifo_size[fifo] >
+ hsotg->hw_params.g_tx_fifo_size[fifo]) {
dev_warn(hsotg->dev, "%s: Invalid parameter g_tx_fifo_size[%d]=%d\n",
__func__, fifo,
hsotg->params.g_tx_fifo_size[fifo]);
- hsotg->params.g_tx_fifo_size[fifo] = dptxfszn;
+ hsotg->params.g_tx_fifo_size[fifo] =
+ hsotg->hw_params.g_tx_fifo_size[fifo];
}
}
}
@@ -607,6 +606,7 @@
{
struct dwc2_hw_params *hw = &hsotg->hw_params;
unsigned int width;
+ int fifo, fifo_count;
u32 hwcfg1, hwcfg2, hwcfg3, hwcfg4;
u32 grxfsiz;
@@ -675,6 +675,12 @@
hw->rx_fifo_size = (grxfsiz & GRXFSIZ_DEPTH_MASK) >>
GRXFSIZ_DEPTH_SHIFT;
+ fifo_count = dwc2_hsotg_tx_fifo_count(hsotg);
+
+ for (fifo = 1; fifo <= fifo_count; fifo++) {
+ hw->g_tx_fifo_size[fifo] = (dwc2_readl(hsotg, DPTXFSIZN(fifo)) &
+ FIFOSIZE_DEPTH_MASK) >> FIFOSIZE_DEPTH_SHIFT;
+ }
return 0;
}
[-- Attachment #3: c6f7e1c.diff --]
[-- Type: text/plain, Size: 2138 bytes --]
From c6f7e1c790c4ef30ba818b119a177fdcd9ca7b34 Mon Sep 17 00:00:00 2001
From: Gevorg Sahakyan <sahakyan@synopsys.com>
Date: Tue, 08 Aug 2017 15:45:32 +0400
Subject: [PATCH] usb: dwc2: Fix tx_fifo_total_depth calculation
Removed ep_info subtraction during calculation tx_addr_max in
dwc2_hsotg_tx_fifo_total_depth function, because its already
done in hardware.
Also removed dwc2_hsotg_ep_info_size function as no more need.
Change-Id: If9a9f8ab115a6e998736ab991056f374eab9f747
Signed-off-by: Gevorg Sahakyan <sahakyan@synopsys.com>
---
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 98a4a79..3c52f46 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -206,47 +206,11 @@
}
/**
- * dwc2_hsotg_ep_info_size - return Endpoint Info Control block size in DWORDs
- */
-static int dwc2_hsotg_ep_info_size(struct dwc2_hsotg *hsotg)
-{
- int val = 0;
- int i;
- u32 ep_dirs;
-
- /*
- * Don't need additional space for ep info control registers in
- * slave mode.
- */
- if (!using_dma(hsotg)) {
- dev_dbg(hsotg->dev, "Buffer DMA ep info size 0\n");
- return 0;
- }
-
- /*
- * Buffer DMA mode - 1 location per endpoit
- * Descriptor DMA mode - 4 locations per endpoint
- */
- ep_dirs = hsotg->hw_params.dev_ep_dirs;
-
- for (i = 0; i <= hsotg->hw_params.num_dev_ep; i++) {
- val += ep_dirs & 3 ? 1 : 2;
- ep_dirs >>= 2;
- }
-
- if (using_desc_dma(hsotg))
- val = val * 4;
-
- return val;
-}
-
-/**
* dwc2_hsotg_tx_fifo_total_depth - return total FIFO depth available for
* device mode TX FIFOs
*/
int dwc2_hsotg_tx_fifo_total_depth(struct dwc2_hsotg *hsotg)
{
- int ep_info_size;
int addr;
int tx_addr_max;
u32 np_tx_fifo_size;
@@ -254,9 +218,7 @@
np_tx_fifo_size = min_t(u32, hsotg->hw_params.dev_nperio_tx_fifo_size,
hsotg->params.g_np_tx_fifo_size);
- /* Get Endpoint Info Control block size in DWORDs. */
- ep_info_size = dwc2_hsotg_ep_info_size(hsotg);
- tx_addr_max = hsotg->hw_params.total_fifo_size - ep_info_size;
+ tx_addr_max = hsotg->hw_params.total_fifo_size;
addr = hsotg->params.g_rx_fifo_size + np_tx_fifo_size;
if (tx_addr_max <= addr)
^ permalink raw reply related
* Re: [PATCH v3 2/5] dt-bindings: pinctrl: mcp23s08: add documentation for drive-open-drain
From: Sebastian Reichel @ 2017-11-21 12:56 UTC (permalink / raw)
To: Phil Reid; +Cc: linus.walleij, robh+dt, mark.rutland, linux-gpio, devicetree
In-Reply-To: <1511252491-79952-3-git-send-email-preid@electromag.com.au>
[-- Attachment #1: Type: text/plain, Size: 1211 bytes --]
Hi,
On Tue, Nov 21, 2017 at 04:21:28PM +0800, Phil Reid wrote:
> This flag set the mcp23s08 device irq type to open drain active low.
>
> Signed-off-by: Phil Reid <preid@electromag.com.au>
> ---
Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
-- Sebastian
> Documentation/devicetree/bindings/pinctrl/pinctrl-mcp23s08.txt | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-mcp23s08.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-mcp23s08.txt
> index 9c451c2..a5a8322 100644
> --- a/Documentation/devicetree/bindings/pinctrl/pinctrl-mcp23s08.txt
> +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-mcp23s08.txt
> @@ -45,6 +45,8 @@ Optional properties:
> - first cell is the pin number
> - second cell is used to specify flags.
> - interrupt-controller: Marks the device node as a interrupt controller.
> +- drive-open-drain: Sets the ODR flag in the IOCON register. This configures
> + the IRQ output as open drain active low.
>
> Optional device specific properties:
> - microchip,irq-mirror: Sets the mirror flag in the IOCON register. Devices
> --
> 1.8.3.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v3 3/5] pinctrl: mcp23s08: add open drain configuration for irq output
From: Sebastian Reichel @ 2017-11-21 12:57 UTC (permalink / raw)
To: Phil Reid; +Cc: linus.walleij, robh+dt, mark.rutland, linux-gpio, devicetree
In-Reply-To: <1511252491-79952-4-git-send-email-preid@electromag.com.au>
[-- Attachment #1: Type: text/plain, Size: 1969 bytes --]
Hi,
On Tue, Nov 21, 2017 at 04:21:29PM +0800, Phil Reid wrote:
> The mcp23s08 series device can be configured for wired and interrupts
> using an external pull-up and open drain output via the IOCON_ODR bit.
> And "drive-open-drain" property to enable this.
>
> Signed-off-by: Phil Reid <preid@electromag.com.au>
> ---
Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
-- Sebastian
> drivers/pinctrl/pinctrl-mcp23s08.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
> index cc1f9f6..8ff9b77 100644
> --- a/drivers/pinctrl/pinctrl-mcp23s08.c
> +++ b/drivers/pinctrl/pinctrl-mcp23s08.c
> @@ -772,6 +772,7 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
> int status, ret;
> bool mirror = false;
> bool irq_active_high = false;
> + bool open_drain = false;
>
> mutex_init(&mcp->lock);
>
> @@ -867,10 +868,11 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
> "microchip,irq-active-high");
>
> mirror = device_property_read_bool(dev, "microchip,irq-mirror");
> + open_drain = device_property_read_bool(dev, "drive-open-drain");
> }
>
> if ((status & IOCON_SEQOP) || !(status & IOCON_HAEN) || mirror ||
> - irq_active_high) {
> + irq_active_high || open_drain) {
> /* mcp23s17 has IOCON twice, make sure they are in sync */
> status &= ~(IOCON_SEQOP | (IOCON_SEQOP << 8));
> status |= IOCON_HAEN | (IOCON_HAEN << 8);
> @@ -882,6 +884,9 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
> if (mirror)
> status |= IOCON_MIRROR | (IOCON_MIRROR << 8);
>
> + if (open_drain)
> + status |= IOCON_ODR | (IOCON_ODR << 8);
> +
> if (type == MCP_TYPE_S18 || type == MCP_TYPE_018)
> status |= IOCON_INTCC | (IOCON_INTCC << 8);
>
> --
> 1.8.3.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v3 1/5] pinctrl: mcp23s08: remove hard coded irq polarity in irq_setup
From: Sebastian Reichel @ 2017-11-21 13:17 UTC (permalink / raw)
To: Phil Reid; +Cc: linus.walleij, robh+dt, mark.rutland, linux-gpio, devicetree
In-Reply-To: <1511252491-79952-2-git-send-email-preid@electromag.com.au>
[-- Attachment #1: Type: text/plain, Size: 2932 bytes --]
Hi,
On Tue, Nov 21, 2017 at 04:21:27PM +0800, Phil Reid wrote:
> The polarity of the irq should be defined in the configuration
> for the irq. eg The device tree bind already allows for either
> active high / low interrupt configuration.
>
> Signed-off-by: Phil Reid <preid@electromag.com.au>
> ---
I think the patch is right, but the long patch description is not.
I would expect something like this:
This changes the driver, so that the "microchip,irq-active-high"
property only configures the mcp23017 interrupt output, but not
the host interrupt input. The host interrupt should be configured
using the standard interrupt flags.
-- Sebastian
> drivers/pinctrl/pinctrl-mcp23s08.c | 14 ++++----------
> 1 file changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
> index 0aef30e..cc1f9f6 100644
> --- a/drivers/pinctrl/pinctrl-mcp23s08.c
> +++ b/drivers/pinctrl/pinctrl-mcp23s08.c
> @@ -56,7 +56,6 @@
>
> struct mcp23s08 {
> u8 addr;
> - bool irq_active_high;
> bool reg_shift;
>
> u16 irq_rise;
> @@ -627,11 +626,6 @@ static int mcp23s08_irq_setup(struct mcp23s08 *mcp)
> int err;
> unsigned long irqflags = IRQF_ONESHOT | IRQF_SHARED;
>
> - if (mcp->irq_active_high)
> - irqflags |= IRQF_TRIGGER_HIGH;
> - else
> - irqflags |= IRQF_TRIGGER_LOW;
> -
> err = devm_request_threaded_irq(chip->parent, mcp->irq, NULL,
> mcp23s08_irq,
> irqflags, dev_name(chip->parent), mcp);
> @@ -777,12 +771,12 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
> {
> int status, ret;
> bool mirror = false;
> + bool irq_active_high = false;
>
> mutex_init(&mcp->lock);
>
> mcp->dev = dev;
> mcp->addr = addr;
> - mcp->irq_active_high = false;
>
> mcp->chip.direction_input = mcp23s08_direction_input;
> mcp->chip.get = mcp23s08_get;
> @@ -868,7 +862,7 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
> mcp->irq_controller =
> device_property_read_bool(dev, "interrupt-controller");
> if (mcp->irq && mcp->irq_controller) {
> - mcp->irq_active_high =
> + irq_active_high =
> device_property_read_bool(dev,
> "microchip,irq-active-high");
>
> @@ -876,11 +870,11 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
> }
>
> if ((status & IOCON_SEQOP) || !(status & IOCON_HAEN) || mirror ||
> - mcp->irq_active_high) {
> + irq_active_high) {
> /* mcp23s17 has IOCON twice, make sure they are in sync */
> status &= ~(IOCON_SEQOP | (IOCON_SEQOP << 8));
> status |= IOCON_HAEN | (IOCON_HAEN << 8);
> - if (mcp->irq_active_high)
> + if (irq_active_high)
> status |= IOCON_INTPOL | (IOCON_INTPOL << 8);
> else
> status &= ~(IOCON_INTPOL | (IOCON_INTPOL << 8));
> --
> 1.8.3.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v3 4/5] pinctrl: mcp23s08: configure irq polarity using irq data
From: Sebastian Reichel @ 2017-11-21 13:34 UTC (permalink / raw)
To: Phil Reid; +Cc: linus.walleij, robh+dt, mark.rutland, linux-gpio, devicetree
In-Reply-To: <1511252491-79952-5-git-send-email-preid@electromag.com.au>
[-- Attachment #1: Type: text/plain, Size: 2320 bytes --]
Hi,
On Tue, Nov 21, 2017 at 04:21:30PM +0800, Phil Reid wrote:
> The irq polarity is already encoded in the irq config. Use that to
> determine the polarity for the mcp32s08 irq output instead of the
> custom microchip,irq-active-high property.
>
> Signed-off-by: Phil Reid <preid@electromag.com.au>
> ---
I don't like, that we use the flags for configuring the host
interrupt input and the mcp23xxx interrupt output. Usually
when the interrupt line has an inverter on it, board DTS files
just toggle the interrupts polarity. This will not work with
this patch applied. We would need to explicitly add an inverter
in the interrupt line, which is completely different to how its
implemented everywhere else (I know at least some Tegra devices
have implicit inverters on interrupt lines).
In case this is really wanted, this patch and the first patch
should be merged to avoid temporarily exposing the splitted
logic.
-- Sebastian
> drivers/pinctrl/pinctrl-mcp23s08.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
> index 8ff9b77..6b3f810 100644
> --- a/drivers/pinctrl/pinctrl-mcp23s08.c
> +++ b/drivers/pinctrl/pinctrl-mcp23s08.c
> @@ -773,6 +773,7 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
> bool mirror = false;
> bool irq_active_high = false;
> bool open_drain = false;
> + u32 irq_trig;
>
> mutex_init(&mcp->lock);
>
> @@ -863,9 +864,13 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
> mcp->irq_controller =
> device_property_read_bool(dev, "interrupt-controller");
> if (mcp->irq && mcp->irq_controller) {
> - irq_active_high =
> - device_property_read_bool(dev,
> - "microchip,irq-active-high");
> + if (device_property_present(dev, "microchip,irq-active-high"))
> + dev_warn(dev,
> + "microchip,irq-active-high is deprecated\n");
> +
> + irq_trig = irqd_get_trigger_type(irq_get_irq_data(mcp->irq));
> + if (irq_trig == IRQF_TRIGGER_HIGH)
> + irq_active_high = true;
>
> mirror = device_property_read_bool(dev, "microchip,irq-mirror");
> open_drain = device_property_read_bool(dev, "drive-open-drain");
> --
> 1.8.3.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 1/2] arm64: dts: orange-pi-zero-plus2: fix sdcard detect
From: Jagan Teki @ 2017-11-21 13:41 UTC (permalink / raw)
To: Sergey Matyukevich
Cc: Maxime Ripard, Chen-Yu Tsai, Rob Herring, Mark Rutland,
devicetree, linux-arm-kernel
In-Reply-To: <20171114185315.gwyacchfct4zvyif-tVm4GcgtBpsfMUw/CLfKLg@public.gmane.org>
On Wed, Nov 15, 2017 at 12:23 AM, Sergey Matyukevich <geomatsi-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Hello Maxime, Jagan,
>
>> > > >> >>> > Did you try to boot from sdcard ? I am not able to boot my board from
>> > > >> >>> > sd-card without this change. As I mentioned earlier in my email
>> > > >> >>> > to buildroot mailing list, with mmc debug enabled I see that mmc
>> > > >> >>> > tries to init sd-card when I remove it from the slot.
>> > > >> >>> >
>> > > >> >>> > Maybe there is a minor differences between h/w versions or batches ?
>> > > >> >>> > What is you board version ? I tested on board v1.0.
>> > > >> >>>
>> > > >> >>> Looking at the schematics, it definitely looks like it's active low.
>> > > >> >>
>> > > >> >> Is it ok to merge it then ? Or using 'cd-inverted' property
>> > > >> >> is the preferable option ?
>> > > >> >
>> > > >> > W/o any change mainline works unstable for me, it wasn't booting
>> > > >> > at-all[1] did you find the same?
>> > > >> >
>> > > >> > Even tried with active LOW and cd-inverted.
>> > > >> >
>> > > >> > [1] https://paste.ubuntu.com/25909064/
>> > > >>
>> > > >> Look like something broken for H5 and A64 between v4.14-rc8 to latest
>> > > >
>> > > > Both 4.13.7 and 4.14-rc8 (synched today) kernels worked fine for me.
>> > > > DTS behavior is all the same:
>> > >
>> > > I've tried fresh sync [2] but still see the issue, can you check the
>> > > Image size of log(suspecting on that area)
>> >
>> > Do you plan to accept this patch as well ? Or you would prefer to wait
>> > for the confirmation from Jagan as well ?
>>
>> I'm happy with the patch, but I was under the impression that the
>> discussion had not settled yet. If it did, then yeah I'll merge it :)
>
> Both schematics and my tests on v1.0 board confirm that this fix is ok.
> However we haven't yet got the ACK from Jagan, the original submitter
> of this dts file. FWIW discussion was mostly about the problems with
> his setup and not about the fix itself.
>
> Jagan,
> did you have a chance to resolve the issues with your setup and verify
> that boot from sd-card is fixed by this patch ?
Acked-by: Jagan Teki <jagan-oRp2ZoJdM/RWk0Htik3J/w@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] dt-bindings: rtc: imxdi: Improve the bindings text
From: Alexandre Belloni @ 2017-11-21 13:46 UTC (permalink / raw)
To: Fabio Estevam
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA,
jbe-bIcnvbaLZ9MEGnE8C9+IrQ, Fabio Estevam
In-Reply-To: <1510750793-25564-1-git-send-email-festevam-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On 15/11/2017 at 10:59:53 -0200, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam-3arQi8VN3Tc@public.gmane.org>
>
> Improve the bindings text by doing the following changes:
>
> - Remove the i.MX53 reference, as the RTC on i.MX53 is a different hardware
> - Add 'clocks' to the list of required properties
> - Explain that the optional security violation irq is the second entry
> - Use the real unit address and irq numbers for i.MX25
>
> Signed-off-by: Fabio Estevam <fabio.estevam-3arQi8VN3Tc@public.gmane.org>
> ---
> Documentation/devicetree/bindings/rtc/imxdi-rtc.txt | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
Applied, thanks.
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 4/5] pinctrl: mcp23s08: configure irq polarity using irq data
From: Phil Reid @ 2017-11-21 14:46 UTC (permalink / raw)
To: Sebastian Reichel
Cc: linus.walleij, robh+dt, mark.rutland, linux-gpio, devicetree
In-Reply-To: <20171121133434.tuvxtrrbabjk4zeg@earth>
G'day Sebastian,
On 21/11/2017 21:34, Sebastian Reichel wrote:
> Hi,
>
> On Tue, Nov 21, 2017 at 04:21:30PM +0800, Phil Reid wrote:
>> The irq polarity is already encoded in the irq config. Use that to
>> determine the polarity for the mcp32s08 irq output instead of the
>> custom microchip,irq-active-high property.
>>
>> Signed-off-by: Phil Reid <preid@electromag.com.au>
>> ---
>
> I don't like, that we use the flags for configuring the host
> interrupt input and the mcp23xxx interrupt output. Usually
> when the interrupt line has an inverter on it, board DTS files
> just toggle the interrupts polarity. This will not work with
> this patch applied. We would need to explicitly add an inverter
> in the interrupt line, which is completely different to how its
> implemented everywhere else (I know at least some Tegra devices
> have implicit inverters on interrupt lines).
>
> In case this is really wanted, this patch and the first patch
> should be merged to avoid temporarily exposing the splitted
> logic.
>
Thanks for looking at the series.
Yes I understand where your coming from. And that's exactly what I
was trying to do in v2. I have 2 of these devices with open drain output
that is feed to an inverter. So active low output from devices and irq
consumer is active high input.
However Linux wasn't a fan of the property and wanted it gone.
He suggested we need a "inverter" device to allow for that in the
device tree. I haven't got my head around how to do that thou.
And if someone is relying on that implicit behaviour are we allowed
to break things? Probably ok with this one as it's currently not possible
due to code patch 1 removes.
If we need to model the invert to get the patches accepted I look into that.
I don't actually need it for my system as I can set open-drain with overrides
the active-high control on this device, while have active high irq consumer.
:)
>
>> drivers/pinctrl/pinctrl-mcp23s08.c | 11 ++++++++---
>> 1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
>> index 8ff9b77..6b3f810 100644
>> --- a/drivers/pinctrl/pinctrl-mcp23s08.c
>> +++ b/drivers/pinctrl/pinctrl-mcp23s08.c
>> @@ -773,6 +773,7 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
>> bool mirror = false;
>> bool irq_active_high = false;
>> bool open_drain = false;
>> + u32 irq_trig;
>>
>> mutex_init(&mcp->lock);
>>
>> @@ -863,9 +864,13 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
>> mcp->irq_controller =
>> device_property_read_bool(dev, "interrupt-controller");
>> if (mcp->irq && mcp->irq_controller) {
>> - irq_active_high =
>> - device_property_read_bool(dev,
>> - "microchip,irq-active-high");
>> + if (device_property_present(dev, "microchip,irq-active-high"))
>> + dev_warn(dev,
>> + "microchip,irq-active-high is deprecated\n");
>> +
>> + irq_trig = irqd_get_trigger_type(irq_get_irq_data(mcp->irq));
>> + if (irq_trig == IRQF_TRIGGER_HIGH)
>> + irq_active_high = true;
>>
>> mirror = device_property_read_bool(dev, "microchip,irq-mirror");
>> open_drain = device_property_read_bool(dev, "drive-open-drain");
>> --
>> 1.8.3.1
>>
--
Regards
Phil Reid
ElectroMagnetic Imaging Technology Pty Ltd
Development of Geophysical Instrumentation & Software
www.electromag.com.au
3 The Avenue, Midland WA 6056, AUSTRALIA
Ph: +61 8 9250 8100
Fax: +61 8 9250 7100
Email: preid@electromag.com.au
^ permalink raw reply
* Re: [PATCH v3 4/5] pinctrl: mcp23s08: configure irq polarity using irq data
From: Sebastian Reichel @ 2017-11-21 15:21 UTC (permalink / raw)
To: Phil Reid, robh+dt-DgEjT+Ai2ygdnm+yROfE0A
Cc: linus.walleij-QSEj5FYQhm4dnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
linux-gpio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <c191a9d6-3ebb-e95b-7342-aa18598ddf2b-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 5292 bytes --]
Hi,
On Tue, Nov 21, 2017 at 10:46:29PM +0800, Phil Reid wrote:
> G'day Sebastian,
>
> On 21/11/2017 21:34, Sebastian Reichel wrote:
> > Hi,
> >
> > On Tue, Nov 21, 2017 at 04:21:30PM +0800, Phil Reid wrote:
> > > The irq polarity is already encoded in the irq config. Use that to
> > > determine the polarity for the mcp32s08 irq output instead of the
> > > custom microchip,irq-active-high property.
> > >
> > > Signed-off-by: Phil Reid <preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>
> > > ---
> >
> > I don't like, that we use the flags for configuring the host
> > interrupt input and the mcp23xxx interrupt output. Usually
> > when the interrupt line has an inverter on it, board DTS files
> > just toggle the interrupts polarity. This will not work with
> > this patch applied. We would need to explicitly add an inverter
> > in the interrupt line, which is completely different to how its
> > implemented everywhere else (I know at least some Tegra devices
> > have implicit inverters on interrupt lines).
> >
> > In case this is really wanted, this patch and the first patch
> > should be merged to avoid temporarily exposing the splitted
> > logic.
> >
> Thanks for looking at the series.
>
> Yes I understand where your coming from. And that's exactly what I
> was trying to do in v2. I have 2 of these devices with open drain output
> that is feed to an inverter. So active low output from devices and irq
> consumer is active high input.
>
> However Linux wasn't a fan of the property and wanted it gone.
I guess s/Linux/Linus Walleij/?
> He suggested we need a "inverter" device to allow for that in the
> device tree. I haven't got my head around how to do that thou.
Just to be on the same term, we are talking about these two variants:
--------------------------------------------
gpio: host-gpio {
random-properties;
}
inv: line-inverter {
/*
* configure the gpio controller input to be active low
* and the inverter interrupt output to be active low
*/
interrupts = <&gpio ACTIVE_LOW>;
};
mcp23xxx {
random-properties;
/*
* configure the chip interrupt output to be active high
* and the inverter interrupt input to be active high
*/
interrupts = <&inv ACTIVE_HIGH>;
}
--------------------------------------------
versus
--------------------------------------------
gpio: host-gpio {
random-properties;
}
mcp23xxx {
random-properties;
/* configure host interrupt input pin to be active low */
interrupts = <&gpio ACTIVE_LOW>;
/* configure chip interrupt output pin to be active high */
microchip,irq-active-high;
}
--------------------------------------------
I think this is something, that Rob should comment on. Obviously at
least in the mainline kernel nobody implemented the first solution
(since there is no fitting interrupt-invert driver), but there are
a few instances of the second variant. On the other hand the first
solution describes the hardware more detailed.
> And if someone is relying on that implicit behaviour are we allowed
> to break things? Probably ok with this one as it's currently not possible
> due to code patch 1 removes.
>
> If we need to model the invert to get the patches accepted I look into that.
> I don't actually need it for my system as I can set open-drain with overrides
> the active-high control on this device, while have active high irq consumer.
> :)
IMHO the explicit line-inverter is a bit over-engineered and
implicit line-inverter is enough, but I'm fine with both solutions.
I think the DT binding maintainers should comment on this though,
since it's pretty much a core decision about interrupt specifiers.
-- Sebastian
> > > drivers/pinctrl/pinctrl-mcp23s08.c | 11 ++++++++---
> > > 1 file changed, 8 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
> > > index 8ff9b77..6b3f810 100644
> > > --- a/drivers/pinctrl/pinctrl-mcp23s08.c
> > > +++ b/drivers/pinctrl/pinctrl-mcp23s08.c
> > > @@ -773,6 +773,7 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
> > > bool mirror = false;
> > > bool irq_active_high = false;
> > > bool open_drain = false;
> > > + u32 irq_trig;
> > > mutex_init(&mcp->lock);
> > > @@ -863,9 +864,13 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
> > > mcp->irq_controller =
> > > device_property_read_bool(dev, "interrupt-controller");
> > > if (mcp->irq && mcp->irq_controller) {
> > > - irq_active_high =
> > > - device_property_read_bool(dev,
> > > - "microchip,irq-active-high");
> > > + if (device_property_present(dev, "microchip,irq-active-high"))
> > > + dev_warn(dev,
> > > + "microchip,irq-active-high is deprecated\n");
> > > +
> > > + irq_trig = irqd_get_trigger_type(irq_get_irq_data(mcp->irq));
> > > + if (irq_trig == IRQF_TRIGGER_HIGH)
> > > + irq_active_high = true;
> > > mirror = device_property_read_bool(dev, "microchip,irq-mirror");
> > > open_drain = device_property_read_bool(dev, "drive-open-drain");
> > > --
> > > 1.8.3.1
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v3 4/5] pinctrl: mcp23s08: configure irq polarity using irq data
From: Phil Reid @ 2017-11-21 15:38 UTC (permalink / raw)
To: Sebastian Reichel, robh+dt
Cc: linus.walleij, mark.rutland, linux-gpio, devicetree
In-Reply-To: <20171121152142.zg3r57gl2kenjwgi@earth>
On 21/11/2017 23:21, Sebastian Reichel wrote:
> Hi,
>
> On Tue, Nov 21, 2017 at 10:46:29PM +0800, Phil Reid wrote:
>> G'day Sebastian,
>>
>> On 21/11/2017 21:34, Sebastian Reichel wrote:
>>> Hi,
>>>
>>> On Tue, Nov 21, 2017 at 04:21:30PM +0800, Phil Reid wrote:
>>>> The irq polarity is already encoded in the irq config. Use that to
>>>> determine the polarity for the mcp32s08 irq output instead of the
>>>> custom microchip,irq-active-high property.
>>>>
>>>> Signed-off-by: Phil Reid <preid@electromag.com.au>
>>>> ---
>>>
>>> I don't like, that we use the flags for configuring the host
>>> interrupt input and the mcp23xxx interrupt output. Usually
>>> when the interrupt line has an inverter on it, board DTS files
>>> just toggle the interrupts polarity. This will not work with
>>> this patch applied. We would need to explicitly add an inverter
>>> in the interrupt line, which is completely different to how its
>>> implemented everywhere else (I know at least some Tegra devices
>>> have implicit inverters on interrupt lines).
>>>
>>> In case this is really wanted, this patch and the first patch
>>> should be merged to avoid temporarily exposing the splitted
>>> logic.
>>>
>> Thanks for looking at the series.
>>
>> Yes I understand where your coming from. And that's exactly what I
>> was trying to do in v2. I have 2 of these devices with open drain output
>> that is feed to an inverter. So active low output from devices and irq
>> consumer is active high input.
>>
>> However Linux wasn't a fan of the property and wanted it gone.
>
> I guess s/Linux/Linus Walleij/?
oops, yes.
>
>> He suggested we need a "inverter" device to allow for that in the
>> device tree. I haven't got my head around how to do that thou.
>
> Just to be on the same term, we are talking about these two variants:
>
> --------------------------------------------
> gpio: host-gpio {
> random-properties;
> }
>
> inv: line-inverter {
> /*
> * configure the gpio controller input to be active low
> * and the inverter interrupt output to be active low
> */
> interrupts = <&gpio ACTIVE_LOW>;
> };
>
> mcp23xxx {
> random-properties;
>
> /*
> * configure the chip interrupt output to be active high
> * and the inverter interrupt input to be active high
> */
> interrupts = <&inv ACTIVE_HIGH>;
> }
> --------------------------------------------
>
> versus
>
> --------------------------------------------
> gpio: host-gpio {
> random-properties;
> }
>
> mcp23xxx {
> random-properties;
>
> /* configure host interrupt input pin to be active low */
> interrupts = <&gpio ACTIVE_LOW>;
>
> /* configure chip interrupt output pin to be active high */
> microchip,irq-active-high;
> }
> --------------------------------------------
>
> I think this is something, that Rob should comment on. Obviously at
> least in the mainline kernel nobody implemented the first solution
> (since there is no fitting interrupt-invert driver), but there are
> a few instances of the second variant. On the other hand the first
> solution describes the hardware more detailed.
Yes that was my understanding of the options, with option 1 being favoured.
Nice summary of the options.
>
>> And if someone is relying on that implicit behaviour are we allowed
>> to break things? Probably ok with this one as it's currently not possible
>> due to code patch 1 removes.
>>
>> If we need to model the invert to get the patches accepted I look into that.
>> I don't actually need it for my system as I can set open-drain with overrides
>> the active-high control on this device, while have active high irq consumer.
>> :)
>
> IMHO the explicit line-inverter is a bit over-engineered and
> implicit line-inverter is enough, but I'm fine with both solutions.
> I think the DT binding maintainers should comment on this though,
> since it's pretty much a core decision about interrupt specifiers.
Thanks again, I'll await further feedback on the preferred direction.>
> -- Sebastian
>
>>>> drivers/pinctrl/pinctrl-mcp23s08.c | 11 ++++++++---
>>>> 1 file changed, 8 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
>>>> index 8ff9b77..6b3f810 100644
>>>> --- a/drivers/pinctrl/pinctrl-mcp23s08.c
>>>> +++ b/drivers/pinctrl/pinctrl-mcp23s08.c
>>>> @@ -773,6 +773,7 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
>>>> bool mirror = false;
>>>> bool irq_active_high = false;
>>>> bool open_drain = false;
>>>> + u32 irq_trig;
>>>> mutex_init(&mcp->lock);
>>>> @@ -863,9 +864,13 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
>>>> mcp->irq_controller =
>>>> device_property_read_bool(dev, "interrupt-controller");
>>>> if (mcp->irq && mcp->irq_controller) {
>>>> - irq_active_high =
>>>> - device_property_read_bool(dev,
>>>> - "microchip,irq-active-high");
>>>> + if (device_property_present(dev, "microchip,irq-active-high"))
>>>> + dev_warn(dev,
>>>> + "microchip,irq-active-high is deprecated\n");
>>>> +
>>>> + irq_trig = irqd_get_trigger_type(irq_get_irq_data(mcp->irq));
>>>> + if (irq_trig == IRQF_TRIGGER_HIGH)
>>>> + irq_active_high = true;
>>>> mirror = device_property_read_bool(dev, "microchip,irq-mirror");
>>>> open_drain = device_property_read_bool(dev, "drive-open-drain");
>>>> --
>>>> 1.8.3.1
--
Regards
Phil Reid
ElectroMagnetic Imaging Technology Pty Ltd
Development of Geophysical Instrumentation & Software
www.electromag.com.au
3 The Avenue, Midland WA 6056, AUSTRALIA
Ph: +61 8 9250 8100
Fax: +61 8 9250 7100
Email: preid@electromag.com.au
^ permalink raw reply
* [PATCH v10 1/3] Documentation: Add device tree binding for Goldfish PIC driver
From: Aleksandar Markovic @ 2017-11-21 15:44 UTC (permalink / raw)
To: linux-mips-6z/3iImG2C8G8FEW9MqTrA
Cc: Miodrag Dinic, Goran Ferenc, Aleksandar Markovic, David S. Miller,
devicetree-u79uwXL29TY76Z2rM5mHXA, Douglas Leung,
Greg Kroah-Hartman, James Hogan, Jason Cooper,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Marc Zyngier, Mark Rutland,
Mauro Carvalho Chehab, Paul Burton, Petar Jovanovic,
Raghu Gandham, Randy Dunlap, Rob Herring, Thomas Gleixner
In-Reply-To: <1511279122-12916-1-git-send-email-aleksandar.markovic-FblTVreYubkAvxtiuMwx3w@public.gmane.org>
From: Miodrag Dinic <miodrag.dinic-8NJIiSa5LzA@public.gmane.org>
Add documentation for DT binding of Goldfish PIC driver. The compatible
string used by OS for binding the driver is "google,goldfish-pic".
Signed-off-by: Miodrag Dinic <miodrag.dinic-8NJIiSa5LzA@public.gmane.org>
Signed-off-by: Goran Ferenc <goran.ferenc-8NJIiSa5LzA@public.gmane.org>
Signed-off-by: Aleksandar Markovic <aleksandar.markovic-8NJIiSa5LzA@public.gmane.org>
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
.../interrupt-controller/google,goldfish-pic.txt | 30 ++++++++++++++++++++++
MAINTAINERS | 5 ++++
2 files changed, 35 insertions(+)
create mode 100644 Documentation/devicetree/bindings/interrupt-controller/google,goldfish-pic.txt
diff --git a/Documentation/devicetree/bindings/interrupt-controller/google,goldfish-pic.txt b/Documentation/devicetree/bindings/interrupt-controller/google,goldfish-pic.txt
new file mode 100644
index 0000000..35f7527
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/google,goldfish-pic.txt
@@ -0,0 +1,30 @@
+Android Goldfish PIC
+
+Android Goldfish programmable interrupt device used by Android
+emulator.
+
+Required properties:
+
+- compatible : should contain "google,goldfish-pic"
+- reg : <registers mapping>
+- interrupts : <interrupt mapping>
+
+Example for mips when used in cascade mode:
+
+ cpuintc {
+ #interrupt-cells = <0x1>;
+ #address-cells = <0>;
+ interrupt-controller;
+ compatible = "mti,cpu-interrupt-controller";
+ };
+
+ interrupt-controller@1f000000 {
+ compatible = "google,goldfish-pic";
+ reg = <0x1f000000 0x1000>;
+
+ interrupt-controller;
+ #interrupt-cells = <0x1>;
+
+ interrupt-parent = <&cpuintc>;
+ interrupts = <0x2>;
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index 650aa0e..998e705 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -872,6 +872,11 @@ S: Supported
F: drivers/android/
F: drivers/staging/android/
+ANDROID GOLDFISH PIC DRIVER
+M: Miodrag Dinic <miodrag.dinic-8NJIiSa5LzA@public.gmane.org>
+S: Supported
+F: Documentation/devicetree/bindings/interrupt-controller/google,goldfish-pic.txt
+
ANDROID GOLDFISH RTC DRIVER
M: Miodrag Dinic <miodrag.dinic-8NJIiSa5LzA@public.gmane.org>
S: Supported
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH v2 1/3] media: V3s: Add support for Allwinner CSI.
From: Maxime Ripard @ 2017-11-21 15:48 UTC (permalink / raw)
To: Yong Deng
Cc: Mauro Carvalho Chehab, Rob Herring, Mark Rutland, Chen-Yu Tsai,
Greg Kroah-Hartman, David S. Miller, Hans Verkuil, Arnd Bergmann,
Hugues Fruchet, Yannick Fertre, Philipp Zabel, Benoit Parrot,
Benjamin Gaignard, Jean-Christophe Trotin,
Ramesh Shanmugasundaram, Minghsiu Tsai, Krzysztof Kozlowski,
Robert Jarzmik, linux-media-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1501131697-1359-2-git-send-email-yong.deng-+3dxTMOEIRNWk0Htik3J/w@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1684 bytes --]
Hi,
On Thu, Jul 27, 2017 at 01:01:35PM +0800, Yong Deng wrote:
> Allwinner V3s SoC have two CSI module. CSI0 is used for MIPI interface
> and CSI1 is used for parallel interface. This is not documented in
> datasheet but by testing and guess.
>
> This patch implement a v4l2 framework driver for it.
>
> Currently, the driver only support the parallel interface. MIPI-CSI2,
> ISP's support are not included in this patch.
>
> Signed-off-by: Yong Deng <yong.deng-+3dxTMOEIRNWk0Htik3J/w@public.gmane.org>
Thanks again for this driver.
It seems like at least this iteration is behaving in a weird way with
DMA transfers for at least YU12 and NV12 (and I would assume YV12).
Starting a transfer of multiple frames in either of these formats,
using either ffmpeg (ffmpeg -f v4l2 -video_size 640x480 -framerate 30
-i /dev/video0 output.mkv) or yavta (yavta -c80 -p -F --skip 0 -f NV12
-s 640x480 $(media-c tl -e 'sun6i-csi')) will end up in a panic.
The panic seems to be generated with random data going into parts of
the kernel memory, the pattern being in my case something like
0x8287868a which is very odd (always around 0x88)
It turns out that when you cover the sensor, the values change to
around 0x28, so it really seems like it's pixels that have been copied
there.
I've looked quickly at the DMA setup, and it seems reasonable to
me. Do you have the same issue on your side? Have you been able to
test those formats using your hardware?
Given that they all are planar formats and YUYV and the likes work
just fine, maybe we can leave them aside for now?
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH v3 4/5] pinctrl: mcp23s08: configure irq polarity using irq data
From: Alexander Stein @ 2017-11-21 16:04 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Phil Reid, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
linus.walleij-QSEj5FYQhm4dnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
linux-gpio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171121152142.zg3r57gl2kenjwgi@earth>
Hi,
On Tuesday, November 21, 2017, 4:21:42 PM CET Sebastian Reichel wrote:
>[...]
> --------------------------------------------
> gpio: host-gpio {
> random-properties;
> }
>
> inv: line-inverter {
> /*
> * configure the gpio controller input to be active low
> * and the inverter interrupt output to be active low
> */
> interrupts = <&gpio ACTIVE_LOW>;
> };
>
> mcp23xxx {
> random-properties;
>
> /*
> * configure the chip interrupt output to be active high
> * and the inverter interrupt input to be active high
> */
> interrupts = <&inv ACTIVE_HIGH>;
> }
> --------------------------------------------
>
> versus
>
> --------------------------------------------
> gpio: host-gpio {
> random-properties;
> }
>
> mcp23xxx {
> random-properties;
>
> /* configure host interrupt input pin to be active low */
> interrupts = <&gpio ACTIVE_LOW>;
>
> /* configure chip interrupt output pin to be active high */
> microchip,irq-active-high;
> }
> --------------------------------------------
>
> I think this is something, that Rob should comment on. Obviously at
> least in the mainline kernel nobody implemented the first solution
> (since there is no fitting interrupt-invert driver), but there are
> a few instances of the second variant. On the other hand the first
> solution describes the hardware more detailed.
>
> > And if someone is relying on that implicit behaviour are we allowed
> > to break things? Probably ok with this one as it's currently not possible
> > due to code patch 1 removes.
> >
> > If we need to model the invert to get the patches accepted I look into that.
> > I don't actually need it for my system as I can set open-drain with overrides
> > the active-high control on this device, while have active high irq consumer.
> > :)
>
> IMHO the explicit line-inverter is a bit over-engineered and
> implicit line-inverter is enough, but I'm fine with both solutions.
> I think the DT binding maintainers should comment on this though,
> since it's pretty much a core decision about interrupt specifiers.
Once you have a hardware, where one of several IRQ users is not attached to the
inverter you need this inverter node anyway, caused by the mixed polarities.
Also some IRQ controllers, like ARM GIC, only support rising edge interrupts
(also level high).
This might get important if there are dedicated IRQ pads connected to several
users.
Just my 2 cents.
Alexander
--
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: [v5 RESEND 02/17] mfd: madera: Add DT bindings for Cirrus Logic Madera codecs
From: Richard Fitzgerald @ 2017-11-21 16:14 UTC (permalink / raw)
To: Rob Herring, Richard Fitzgerald
Cc: gnurou, alsa-devel, jason, devicetree, linus.walleij, patches,
linux-kernel, linux-gpio, broonie, tglx, lee.jones
In-Reply-To: <20171120184622.yor2xemxbbvicmmi@rob-hp-laptop>
Sorry all, I don't know what's gone wrong here. The acks are there on my
computer and I double-checked before I sent but somewhere all the acks
have been stripped.
When I've figured out what went wrong I'll re-send the chain with the acks.
On 20/11/17 18:46, Rob Herring wrote:
> On Mon, Nov 20, 2017 at 02:10:36PM +0000, Richard Fitzgerald wrote:
>> From: Richard Fitzgerald <rf@opensource.wolfsonmicro.com>
>>
>> Specification of the bindings for the parent MFD driver component
>> of the Cirrus Logic Madera codec drivers.
>>
>> Note that although the interrupt controller and GPIO are child
>> drivers their required bindings are trivial, mandatory, and exist
>> within the parent MFD node so are documented here.
>>
>> Signed-off-by: Richard Fitzgerald <rf@opensource.wolfsonmicro.com>
>> ---
>> Documentation/devicetree/bindings/mfd/madera.txt | 102 +++++++++++++++++++++++
>> 1 file changed, 102 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/mfd/madera.txt
> Please add acks when posting new versions.
>
> Rob
^ 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