* Re: [PATCH 5/10] dt: bindings: Add bindings for Marvell Xenon SD Host Controller
From: Thomas Petazzoni @ 2016-11-24 10:10 UTC (permalink / raw)
To: Marcin Wojtas
Cc: Gregory CLEMENT, Arnd Bergmann,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
Jimmy Xu, Andrew Lunn, Ulf Hansson, Romain Perier, Liuliu Zhao,
Peng Zhu, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Nadav Haklai, Ziji Hu, Victor Gu, Doug Jones, Jisheng Zhang,
Yehuda Yitschak, Xueping Liu, Hilbert Zhang, Shiwu Zhang
In-Reply-To: <CAPv3WKddPHgpRU2_tVoDF=5Z-nqfFPxjgJ-+z9o-1tR2=fFvAA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hello,
On Thu, 24 Nov 2016 10:49:23 +0100, Marcin Wojtas wrote:
> How about to avoid confusion, by simply renaming this number to
> port-id/xenon-id or anything else but slot? I guess this may allow to
> avoid some misunderstandings.
Agreed.
Thomas
--
Thomas Petazzoni, CTO, 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 0/3] arm64: dts: r8a7796: Add CAN/CAN FD support
From: Chris Paterson @ 2016-11-24 10:05 UTC (permalink / raw)
To: Simon Horman, Marc Kleine-Budde
Cc: Wolfgang Grandegger, Magnus Damm, Rob Herring, Mark Rutland,
Ramesh Shanmugasundaram, linux-renesas-soc@vger.kernel.org,
devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-can@vger.kernel.org
In-Reply-To: <20161123142938.GF9057@verge.net.au>
Hello Simon,
From: Simon Horman [mailto:horms@verge.net.au]
Sent: 23 November 2016 14:30
> 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.
Okay, will do.
>
> 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.
Can you give me some guidance here? I'm not sure what you're referring to. As far as I know the DT reg definition here is 64-bit, or are you referring to DMA usage? If the later, neither CAN driver uses DMA.
Kind regards, Chris
^ permalink raw reply
* Re: [PATCH 5/10] dt: bindings: Add bindings for Marvell Xenon SD Host Controller
From: Arnd Bergmann @ 2016-11-24 10:04 UTC (permalink / raw)
To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: Thomas Petazzoni, Gregory CLEMENT, Jimmy Xu, Andrew Lunn,
Ulf Hansson, Romain Perier, Liuliu Zhao, Peng Zhu, Adrian Hunter,
Nadav Haklai, Ziji Hu, Victor Gu, Doug Jones, Jisheng Zhang,
Yehuda Yitschak, Wei(SOCP) Liu, Xueping Liu, Hilbert Zhang,
Shiwu Zhang, Yu Cao, Sebastian
In-Reply-To: <20161124104858.3604c11d-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
On Thursday, November 24, 2016 10:48:58 AM CET Thomas Petazzoni wrote:
> Hello,
>
> On Thu, 24 Nov 2016 10:44:48 +0100, Gregory CLEMENT wrote:
>
> > "A single Xenon IP can support multiple slots.
> > Each slot acts as an independent SDHC. It owns independent resources, such
> > as register sets clock and PHY.
> > Each slot should have an independent device tree node."
>
> I think this wording is still very confusing, and continues to cause
> confusion.
>
> We should just state that each Xenon controller supports a single slot,
> and that's it.
>
> The text still says "a single Xenon IP can support multiple slots",
> which continues to cause confusion.
Agreed. Ideally we'd find out why exactly the slot number must
be used for accessing some of the registers to have a better
explanation to put in there, aside from stating that only one
slot is supported but the number must be set.
Could it be that this is some form of pinmuxing, i.e. that each
controller could in theory be used for any of the slots but you
have to pick one of them?
Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v28 9/9] Documentation: dt: chosen properties for arm64 kdump
From: AKASHI Takahiro @ 2016-11-24 9:59 UTC (permalink / raw)
To: catalin.marinas-5wv7dgnIgG8, will.deacon-5wv7dgnIgG8,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8
Cc: james.morse-5wv7dgnIgG8, geoff-wEGCiKHe2LqWVfeAwA7xHQ,
bauerman-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
dyoung-H+wXaHxf7aLQT0dZR+AlfA,
kexec-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, AKASHI Takahiro
In-Reply-To: <20161124095523.6972-1-takahiro.akashi-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
From: James Morse <james.morse-5wv7dgnIgG8@public.gmane.org>
Add documentation for
linux,crashkernel-base and crashkernel-size,
linux,usable-memory-range
linux,elfcorehdr
used by arm64 kdump to decribe the kdump reserved area, and
the elfcorehdr's location within it.
Signed-off-by: James Morse <james.morse-5wv7dgnIgG8@public.gmane.org>
[takahiro.akashi-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org: added "linux,crashkernel-base" and "-size" ]
Signed-off-by: AKASHI Takahiro <takahiro.akashi-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
---
Documentation/devicetree/bindings/chosen.txt | 50 ++++++++++++++++++++++++++++
1 file changed, 50 insertions(+)
diff --git a/Documentation/devicetree/bindings/chosen.txt b/Documentation/devicetree/bindings/chosen.txt
index 6ae9d82..7b11516 100644
--- a/Documentation/devicetree/bindings/chosen.txt
+++ b/Documentation/devicetree/bindings/chosen.txt
@@ -52,3 +52,53 @@ This property is set (currently only on PowerPC, and only needed on
book3e) by some versions of kexec-tools to tell the new kernel that it
is being booted by kexec, as the booting environment may differ (e.g.
a different secondary CPU release mechanism)
+
+linux,crashkernel-base
+linux,crashkernel-size
+----------------------
+
+These properties (currently used on PowerPC and arm64) indicates
+the base address and the size, respectively, of the reserved memory
+range for crash dump kernel.
+e.g.
+
+/ {
+ chosen {
+ linux,crashkernel-base = <0x9 0xf0000000>;
+ linux,crashkernel-size = <0x0 0x10000000>;
+ };
+};
+
+linux,usable-memory-range
+-------------------------
+
+This property (currently used only on arm64) holds the memory range,
+the base address and the size, which can be used as system ram on
+the *current* kernel. Note that, if this property is present, any memory
+regions under "memory" nodes in DT blob or ones marked as "conventional
+memory" in EFI memory map should be ignored.
+e.g.
+
+/ {
+ chosen {
+ linux,usable-memory-range = <0x9 0xf0000000 0x0 0x10000000>;
+ };
+};
+
+The main usage is for crash dump kernel to identify its own usable
+memory and exclude, at its boot time, any other memory areas that are
+part of the panicked kernel's memory.
+
+linux,elfcorehdr
+----------------
+
+This property (currently used only on arm64) holds the memory range,
+the address and the size, of the elf core header which mainly describes
+the panicked kernel's memory layout as PT_LOAD segments of elf format.
+e.g.
+
+/ {
+ chosen {
+ linux,elfcorehdr = <0x9 0xfffff000 0x0 0x800>;
+ };
+};
--
2.10.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH 7/10] mmc: sdhci-xenon: Add support to PHYs of Marvell Xenon SDHC
From: Arnd Bergmann @ 2016-11-24 9:56 UTC (permalink / raw)
To: Gregory CLEMENT
Cc: Ulf Hansson, Adrian Hunter, linux-mmc-u79uwXL29TY76Z2rM5mHXA,
Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Rob Herring,
devicetree-u79uwXL29TY76Z2rM5mHXA, Thomas Petazzoni,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Ziji Hu,
Jack(SH) Zhu, Jimmy Xu, Jisheng Zhang, Nadav Haklai, Ryan Gao,
Doug Jones, Shiwu Zhang, Victor Gu, Wei(SOCP) Liu, Wilson Ding,
Xueping Liu <xpli>
In-Reply-To: <a05ffd140f4edc02fc3128db8445b2264cf38723.1477911954.git-series.gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
On Monday, October 31, 2016 12:09:56 PM CET Gregory CLEMENT wrote:
> From: Ziji Hu <huziji-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
>
> Marvell Xenon eMMC/SD/SDIO Host Controller contains PHY.
> Three types of PHYs are supported.
>
> Add support to multiple types of PHYs init and configuration.
> Add register definitions of PHYs.
>
> Signed-off-by: Hu Ziji <huziji-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>
Please explain in the changelog why this is not a generic
phy driver (or three of them).
Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFC v2: PATCH 1/2] dt-bindings: Document the hi3660 reset bindings
From: Philipp Zabel @ 2016-11-24 9:50 UTC (permalink / raw)
To: zhangfei
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <ac3c8e65-e4f2-3a9d-452c-f270d245cf9d-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Am Donnerstag, den 24.11.2016, 17:40 +0800 schrieb zhangfei:
>
> On 2016年11月24日 17:26, Philipp Zabel wrote:
> > Am Mittwoch, den 23.11.2016, 16:07 +0800 schrieb Zhangfei Gao:
> >> Add DT bindings documentation for hi3660 SoC reset controller.
> >>
> >> Signed-off-by: Zhangfei Gao <zhangfei.gao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> >> ---
> >> .../bindings/reset/hisilicon,hi3660-reset.txt | 51 ++++++++++++++++++++++
> >> 1 file changed, 51 insertions(+)
> >> create mode 100644 Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt b/Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt
> >> new file mode 100644
> >> index 0000000..250daf2
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt
> >> @@ -0,0 +1,51 @@
> >> +Hisilicon System Reset Controller
> >> +======================================
> >> +
> >> +Please also refer to reset.txt in this directory for common reset
> >> +controller binding usage.
> >> +
> >> +The reset controller registers are part of the system-ctl block on
> >> +hi3660 SoC.
> >> +
> >> +Required properties:
> >> +- compatible: should be
> >> + "hisilicon,hi3660-reset"
> >> +- #reset-cells: 1, see below
> >> +- hisi,rst-syscon: phandle of the reset's syscon.
> >> +- hisi,reset-bits: Contains the reset control register information
> >> + Should contain 2 cells for each reset exposed to
> >> + consumers, defined as:
> >> + Cell #1 : offset from the syscon register base
> >> + Cell #2 : bits position of the control register
> >> +
> >> +Example:
> >> + iomcu: iomcu@ffd7e000 {
> >> + compatible = "hisilicon,hi3660-iomcu", "syscon";
> >> + reg = <0x0 0xffd7e000 0x0 0x1000>;
> >> + };
> >> +
> >> + iomcu_rst: iomcu_rst_controller {
> > This should be
> > iomcu_rst: reset-controller {
> >
> >> + compatible = "hisilicon,hi3660-reset";
> >> + #reset-cells = <1>;
> >> + hisi,rst-syscon = <&iomcu>;
> >> + hisi,reset-bits = <0x20 0x8 /* 0: i2c0 */
> >> + 0x20 0x10 /* 1: i2c1 */
> >> + 0x20 0x20 /* 2: i2c2 */
> >> + 0x20 0x8000000>; /* 3: i2c6 */
> >> + };
> > The reset lines are controlled through iomcu bits, is there a reason not
> > to put the iomcu_rst node inside the iomcu node? That way the
> > hisi,rst-syscon property could be removed and the syscon could be
> > retrieved via the reset-controller parent node.
> iomcu is common registers, controls clock and reset, etc.
> So we use syscon, without mapping the registers everywhere.
> It is common case in hisilicon, same in hi6220.
>
> Also the #clock-cells and #reset-cells can not be put in the same node,
> if they are both using probe, since reset_probe will not be called.
>
> So we use hisi,rst-syscon as a general solution.
What I meant is this:
iomcu: iomcu@ffd7e000 {
compatible = "hisilicon,hi3660-iomcu", "syscon", "simple-mfd";
reg = <0x0 0xffd7e000 0x0 0x1000>;
iomcu_rst: reset-controller {
compatible = "hisilicon,hi3660-reset";
#reset-cells = <1>;
hisi,reset-bits = <0x20 0x8 /* 0: i2c0 */
0x20 0x10 /* 1: i2c1 */
0x20 0x20 /* 2: i2c2 */
0x20 0x8000000>; /* 3: i2c6 */
};
};
regards
Philipp
--
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 5/10] dt: bindings: Add bindings for Marvell Xenon SD Host Controller
From: Marcin Wojtas @ 2016-11-24 9:49 UTC (permalink / raw)
To: Gregory CLEMENT
Cc: Arnd Bergmann, linux-arm-kernel@lists.infradead.org, Jimmy Xu,
Andrew Lunn, Ulf Hansson, Romain Perier, Liuliu Zhao, Peng Zhu,
linux-kernel@vger.kernel.org, Nadav Haklai, Ziji Hu, Victor Gu,
Doug Jones, Jisheng Zhang, Yehuda Yitschak, Xueping Liu,
Hilbert Zhang, Shiwu Zhang, Yu Cao,
Sebastian Hesselbarth <sebastian.he>
In-Reply-To: <8737ihmctr.fsf@free-electrons.com>
Hi Gregory,
2016-11-24 10:44 GMT+01:00 Gregory CLEMENT <gregory.clement@free-electrons.com>:
> Hi Arnd,
>
> On jeu., nov. 24 2016, Arnd Bergmann <arnd@arndb.de> wrote:
>
>> On Thursday, November 24, 2016 10:22:31 AM CET Gregory CLEMENT wrote:
>>>
>>> I don't have an option for mmc in general, but using child node do not
>>> fit at all the xenon controller.
>>>
>>> For this controller each slot has its own set of register, so there is
>>> no common ressource to share so no advantage to use it. Using child node
>>> in our case will just make the code more complex for no benefit.
>>
>> If every slot has its own registers, what is it that makes up the
>> 'controller'? It sounds to me that you just have to adjust the terminology
>> and talk about multiple controllers then, with one slot per controller.
>>
>
> I agree and actually there were some words about in at the begining of
> the binding:
>
> "A single Xenon IP can support multiple slots.
> Each slot acts as an independent SDHC. It owns independent resources, such
> as register sets clock and PHY.
> Each slot should have an independent device tree node."
>
> All the confusion came from the fact that we still need to identify a
> slot ID. For an obscure reason the hardware can't guess the slot ID from
> the address register."
>
How about to avoid confusion, by simply renaming this number to
port-id/xenon-id or anything else but slot? I guess this may allow to
avoid some misunderstandings.
Best regards,
Marcin
^ permalink raw reply
* Re: [PATCH 5/10] dt: bindings: Add bindings for Marvell Xenon SD Host Controller
From: Thomas Petazzoni @ 2016-11-24 9:48 UTC (permalink / raw)
To: Gregory CLEMENT
Cc: Arnd Bergmann, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Jimmy Xu, Andrew Lunn, Ulf Hansson, Romain Perier, Liuliu Zhao,
Peng Zhu, linux-kernel@vger.kernel.org, Nadav Haklai, Ziji Hu,
Victor Gu, Doug Jones, Jisheng Zhang, Yehuda Yitschak,
Marcin Wojtas, Xueping Liu, Hilbert Zhang, Shiwu Zhang, Yu Cao,
Sebastian Hesselbarth <sebastian.hesselbart>
In-Reply-To: <8737ihmctr.fsf-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Hello,
On Thu, 24 Nov 2016 10:44:48 +0100, Gregory CLEMENT wrote:
> "A single Xenon IP can support multiple slots.
> Each slot acts as an independent SDHC. It owns independent resources, such
> as register sets clock and PHY.
> Each slot should have an independent device tree node."
I think this wording is still very confusing, and continues to cause
confusion.
We should just state that each Xenon controller supports a single slot,
and that's it.
The text still says "a single Xenon IP can support multiple slots",
which continues to cause confusion.
Best regards,
Thomas
--
Thomas Petazzoni, CTO, 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: [RFC v2: PATCH 1/2] dt-bindings: Document the hi3660 reset bindings
From: zhangfei @ 2016-11-24 9:40 UTC (permalink / raw)
To: Philipp Zabel; +Cc: devicetree, linux-arm-kernel
In-Reply-To: <1479979605.2472.4.camel@pengutronix.de>
On 2016年11月24日 17:26, Philipp Zabel wrote:
> Am Mittwoch, den 23.11.2016, 16:07 +0800 schrieb Zhangfei Gao:
>> Add DT bindings documentation for hi3660 SoC reset controller.
>>
>> Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
>> ---
>> .../bindings/reset/hisilicon,hi3660-reset.txt | 51 ++++++++++++++++++++++
>> 1 file changed, 51 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt
>>
>> diff --git a/Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt b/Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt
>> new file mode 100644
>> index 0000000..250daf2
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt
>> @@ -0,0 +1,51 @@
>> +Hisilicon System Reset Controller
>> +======================================
>> +
>> +Please also refer to reset.txt in this directory for common reset
>> +controller binding usage.
>> +
>> +The reset controller registers are part of the system-ctl block on
>> +hi3660 SoC.
>> +
>> +Required properties:
>> +- compatible: should be
>> + "hisilicon,hi3660-reset"
>> +- #reset-cells: 1, see below
>> +- hisi,rst-syscon: phandle of the reset's syscon.
>> +- hisi,reset-bits: Contains the reset control register information
>> + Should contain 2 cells for each reset exposed to
>> + consumers, defined as:
>> + Cell #1 : offset from the syscon register base
>> + Cell #2 : bits position of the control register
>> +
>> +Example:
>> + iomcu: iomcu@ffd7e000 {
>> + compatible = "hisilicon,hi3660-iomcu", "syscon";
>> + reg = <0x0 0xffd7e000 0x0 0x1000>;
>> + };
>> +
>> + iomcu_rst: iomcu_rst_controller {
> This should be
> iomcu_rst: reset-controller {
>
>> + compatible = "hisilicon,hi3660-reset";
>> + #reset-cells = <1>;
>> + hisi,rst-syscon = <&iomcu>;
>> + hisi,reset-bits = <0x20 0x8 /* 0: i2c0 */
>> + 0x20 0x10 /* 1: i2c1 */
>> + 0x20 0x20 /* 2: i2c2 */
>> + 0x20 0x8000000>; /* 3: i2c6 */
>> + };
> The reset lines are controlled through iomcu bits, is there a reason not
> to put the iomcu_rst node inside the iomcu node? That way the
> hisi,rst-syscon property could be removed and the syscon could be
> retrieved via the reset-controller parent node.
iomcu is common registers, controls clock and reset, etc.
So we use syscon, without mapping the registers everywhere.
It is common case in hisilicon, same in hi6220.
Also the #clock-cells and #reset-cells can not be put in the same node,
if they are both using probe, since reset_probe will not be called.
So we use hisi,rst-syscon as a general solution.
Thanks
_______________________________________________
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 5/10] dt: bindings: Add bindings for Marvell Xenon SD Host Controller
From: Arnd Bergmann @ 2016-11-24 9:34 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Gregory CLEMENT, Jimmy Xu, Andrew Lunn, Ulf Hansson,
Romain Perier, Liuliu Zhao, Peng Zhu,
linux-kernel@vger.kernel.org, Nadav Haklai, Ziji Hu, Victor Gu,
Doug Jones, Jisheng Zhang, Yehuda Yitschak, Marcin Wojtas,
Xueping Liu, Hilbert Zhang, Shiwu Zhang, Yu Cao,
Sebastian Hesselbarth
In-Reply-To: <877f7tmduw.fsf@free-electrons.com>
On Thursday, November 24, 2016 10:22:31 AM CET Gregory CLEMENT wrote:
>
> I don't have an option for mmc in general, but using child node do not
> fit at all the xenon controller.
>
> For this controller each slot has its own set of register, so there is
> no common ressource to share so no advantage to use it. Using child node
> in our case will just make the code more complex for no benefit.
If every slot has its own registers, what is it that makes up the
'controller'? It sounds to me that you just have to adjust the terminology
and talk about multiple controllers then, with one slot per controller.
Arnd
^ permalink raw reply
* [PATCH] ARM: dts: da850: enable the memctrl and mstpri nodes per board
From: Bartosz Golaszewski @ 2016-11-24 9:31 UTC (permalink / raw)
To: Kevin Hilman, Michael Turquette, Sekhar Nori, Rob Herring,
Frank Rowand, Mark Rutland, Peter Ujfalusi, Russell King
Cc: LKML, arm-soc, linux-drm, linux-devicetree, Jyri Sarha,
Tomi Valkeinen, David Airlie, Laurent Pinchart, Robin Murphy,
Sudeep Holla, Bartosz Golaszewski
Currently the memory controller and master priorities drivers are
enabled in da850.dtsi. For boards for which there are no settings
defined, this makes these drivers emit error messages.
Disable the nodes in da850.dtsi and only enable them for da850-lcdk -
the only board that currently needs them.
Signed-off-by: Bartosz Golaszewski <bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
---
arch/arm/boot/dts/da850-lcdk.dts | 8 ++++++++
arch/arm/boot/dts/da850.dtsi | 2 ++
2 files changed, 10 insertions(+)
diff --git a/arch/arm/boot/dts/da850-lcdk.dts b/arch/arm/boot/dts/da850-lcdk.dts
index 3b99a88..94504c8 100644
--- a/arch/arm/boot/dts/da850-lcdk.dts
+++ b/arch/arm/boot/dts/da850-lcdk.dts
@@ -283,3 +283,11 @@
&display {
status = "okay";
};
+
+&prictrl {
+ status = "okay";
+};
+
+&memctrl {
+ status = "okay";
+};
diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
index 36066fa..4e40187 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -212,6 +212,7 @@
prictrl: priority-controller@14110 {
compatible = "ti,da850-mstpri";
reg = <0x14110 0x0c>;
+ status = "disabled";
};
cfgchip: chip-controller@1417c {
compatible = "ti,da830-cfgchip", "syscon", "simple-mfd";
@@ -457,5 +458,6 @@
memctrl: memory-controller@b0000000 {
compatible = "ti,da850-ddr-controller";
reg = <0xb0000000 0xe8>;
+ status = "disabled";
};
};
--
2.9.3
--
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: [RFC PATCH 2/5] dmaengine: allow sun6i-dma for more SoCs
From: Chen-Yu Tsai @ 2016-11-24 9:30 UTC (permalink / raw)
To: Andre Przywara
Cc: Chen-Yu Tsai, Maxime Ripard, Icenowy Zheng, linux-sunxi,
linux-arm-kernel, Mark Rutland, Rob Herring, devicetree
In-Reply-To: <34b5e50f-a091-9bd8-7a74-96e538a7351d-5wv7dgnIgG8@public.gmane.org>
On Thu, Nov 24, 2016 at 5:16 PM, Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org> wrote:
> Hi,
>
> On 24/11/16 04:16, Chen-Yu Tsai wrote:
>> Hi,
>>
>> On Thu, Nov 24, 2016 at 9:17 AM, Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org> wrote:
>>> The sun6i DMA driver is used in the Allwinner A64 and H5 SoC, which
>>> have arm64 capable cores. Add the generic sunxi config symbol to allow
>>> the driver to be selected by arm64 Kconfigs, which don't feature
>>> SoC specific MACH_xxxx configs.
>>>
>>> Signed-off-by: Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org>
>>> ---
>>> drivers/dma/Kconfig | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
>>> index af63a6b..003c284 100644
>>> --- a/drivers/dma/Kconfig
>>> +++ b/drivers/dma/Kconfig
>>> @@ -157,7 +157,7 @@ config DMA_SUN4I
>>>
>>> config DMA_SUN6I
>>> tristate "Allwinner A31 SoCs DMA support"
>>> - depends on MACH_SUN6I || MACH_SUN8I || COMPILE_TEST
>>> + depends on MACH_SUN6I || MACH_SUN8I || COMPILE_TEST || ARCH_SUNXI
>>
>> AFAIK ARCH_SUNXI encompasses/supersedes MACH_SUN*I.
>> (And I don't have to add MACH_SUN9I later :) )
>
> Sure, admittedly it was just a quick hack to get things going.
> Actually I don't know why we had a *depend* on those MACH_s before. I
> think technically it does not depend on a certain SoC (having the
> COMPILE_TEST in there hints on that). So what about:
It was really because this DMA engine only comes with the later
SoCs. We have dma-sun4i for the older one. But yes, there's no
reason why you can't build it for the earlier SoC. It just doesn't
get used.
>
> depends on ARCH_SUNXI || COMPILE_TEST
>
> and maybe:
>
> default y if MACH_SUN6I || MACH_SUN8I
>
> Though I see that both multi_v7_defconfig and sunxi_defconfig explicitly
> set this, so this wouldn't be needed?
I guess it's just nice to get stuff out of defconfig?
Why not go all the way and just have
default y if ARCH_SUNXI
ChenYu
>
> Cheers,
> Andre.
^ permalink raw reply
* Re: [RFC v2: PATCH 1/2] dt-bindings: Document the hi3660 reset bindings
From: Philipp Zabel @ 2016-11-24 9:26 UTC (permalink / raw)
To: Zhangfei Gao
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1479888476-13138-2-git-send-email-zhangfei.gao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Am Mittwoch, den 23.11.2016, 16:07 +0800 schrieb Zhangfei Gao:
> Add DT bindings documentation for hi3660 SoC reset controller.
>
> Signed-off-by: Zhangfei Gao <zhangfei.gao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
> .../bindings/reset/hisilicon,hi3660-reset.txt | 51 ++++++++++++++++++++++
> 1 file changed, 51 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt
>
> diff --git a/Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt b/Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt
> new file mode 100644
> index 0000000..250daf2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt
> @@ -0,0 +1,51 @@
> +Hisilicon System Reset Controller
> +======================================
> +
> +Please also refer to reset.txt in this directory for common reset
> +controller binding usage.
> +
> +The reset controller registers are part of the system-ctl block on
> +hi3660 SoC.
> +
> +Required properties:
> +- compatible: should be
> + "hisilicon,hi3660-reset"
> +- #reset-cells: 1, see below
> +- hisi,rst-syscon: phandle of the reset's syscon.
> +- hisi,reset-bits: Contains the reset control register information
> + Should contain 2 cells for each reset exposed to
> + consumers, defined as:
> + Cell #1 : offset from the syscon register base
> + Cell #2 : bits position of the control register
> +
> +Example:
> + iomcu: iomcu@ffd7e000 {
> + compatible = "hisilicon,hi3660-iomcu", "syscon";
> + reg = <0x0 0xffd7e000 0x0 0x1000>;
> + };
> +
> + iomcu_rst: iomcu_rst_controller {
This should be
iomcu_rst: reset-controller {
> + compatible = "hisilicon,hi3660-reset";
> + #reset-cells = <1>;
> + hisi,rst-syscon = <&iomcu>;
> + hisi,reset-bits = <0x20 0x8 /* 0: i2c0 */
> + 0x20 0x10 /* 1: i2c1 */
> + 0x20 0x20 /* 2: i2c2 */
> + 0x20 0x8000000>; /* 3: i2c6 */
> + };
The reset lines are controlled through iomcu bits, is there a reason not
to put the iomcu_rst node inside the iomcu node? That way the
hisi,rst-syscon property could be removed and the syscon could be
retrieved via the reset-controller parent node.
> +Specifying reset lines connected to IP modules
> +==============================================
> +example:
> +
> + i2c0: i2c@..... {
> + ...
> + resets = <&iomcu_rst 0>;
> + ...
> + };
> +
> + i2c1: i2c@..... {
> + ...
> + resets = <&iomcu_rst 1>;
> + ...
> + };
regards
Philipp
--
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 5/10] dt: bindings: Add bindings for Marvell Xenon SD Host Controller
From: Gregory CLEMENT @ 2016-11-24 9:22 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Ulf Hansson, Rob Herring, Ziji Hu, Adrian Hunter,
linux-mmc@vger.kernel.org, Jason Cooper, Andrew Lunn,
Sebastian Hesselbarth, devicetree@vger.kernel.org,
Thomas Petazzoni, linux-arm-kernel@lists.infradead.org,
Jack(SH) Zhu, Jimmy Xu, Jisheng Zhang, Nadav Haklai, Ryan Gao,
Doug Jones, Shiwu Zhang, Victor Gu <xi>
In-Reply-To: <4031579.CBE32NHUoW@wuerfel>
Hi Arnd,
On jeu., nov. 24 2016, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
> On Thursday, November 24, 2016 10:05:45 AM CET Ulf Hansson wrote:
>> > You also mentioned other bindings using child nodes, but for this one
>> > we have one controller with only one set of register with multiple slots
>> > (Atmel is an example). Here each slot have it own set of register.
>> >
>> > Actually giving the fact that each slot is controlled by a different set
>> > of register I wonder why the hardware can't also deduce the slot number
>> > from the address register. For me it looks like an hardware bug but we
>> > have to deal with it.
>> >
>> > Do you still think we needchild node here?
>>
>> Using child-nodes for slots like what's done in the atmel case, is
>> currently broken. I would recommend to avoid using child-nodes for
>> slots, if possible.
>>
>> To give you some more background, currently the mmc core treats child
>> nodes as embedded non-removable cards or SDIO funcs. However, we can
>> change to make child-nodes also allowed to describe slots, but it
>> requires a specific compatible for "slots" and of course then we also
>> need to update the DT parsing of the child-nodes in the mmc core.
>>
>> Documentation/devicetree/bindings/mmc/mmc.txt
>> Documentation/devicetree/bindings/mmc/mmc-card.txt
>
> I don't see anything wrong with having child nodes for the slots
> even with the current binding, under one condition:
>
> The mmc.txt binding above must refer only to the child node, while
> the parent node conceptually becomes a plain bus or MFD that
> happens to encapsulate multiple MMC host controllers, and possibly
> provides some shared registers to them.
I don't have an option for mmc in general, but using child node do not
fit at all the xenon controller.
For this controller each slot has its own set of register, so there is
no common ressource to share so no advantage to use it. Using child node
in our case will just make the code more complex for no benefit.
Gregory
>
> Arnd
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
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: [RFC PATCH 2/5] dmaengine: allow sun6i-dma for more SoCs
From: Andre Przywara @ 2016-11-24 9:16 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Maxime Ripard, Icenowy Zheng, linux-sunxi, linux-arm-kernel,
Mark Rutland, Rob Herring, devicetree
In-Reply-To: <CAGb2v67M8DrPaf8GzSPEjekgV6cLcXXzO3tVUc9kjUDcM3BE_w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hi,
On 24/11/16 04:16, Chen-Yu Tsai wrote:
> Hi,
>
> On Thu, Nov 24, 2016 at 9:17 AM, Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org> wrote:
>> The sun6i DMA driver is used in the Allwinner A64 and H5 SoC, which
>> have arm64 capable cores. Add the generic sunxi config symbol to allow
>> the driver to be selected by arm64 Kconfigs, which don't feature
>> SoC specific MACH_xxxx configs.
>>
>> Signed-off-by: Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org>
>> ---
>> drivers/dma/Kconfig | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
>> index af63a6b..003c284 100644
>> --- a/drivers/dma/Kconfig
>> +++ b/drivers/dma/Kconfig
>> @@ -157,7 +157,7 @@ config DMA_SUN4I
>>
>> config DMA_SUN6I
>> tristate "Allwinner A31 SoCs DMA support"
>> - depends on MACH_SUN6I || MACH_SUN8I || COMPILE_TEST
>> + depends on MACH_SUN6I || MACH_SUN8I || COMPILE_TEST || ARCH_SUNXI
>
> AFAIK ARCH_SUNXI encompasses/supersedes MACH_SUN*I.
> (And I don't have to add MACH_SUN9I later :) )
Sure, admittedly it was just a quick hack to get things going.
Actually I don't know why we had a *depend* on those MACH_s before. I
think technically it does not depend on a certain SoC (having the
COMPILE_TEST in there hints on that). So what about:
depends on ARCH_SUNXI || COMPILE_TEST
and maybe:
default y if MACH_SUN6I || MACH_SUN8I
Though I see that both multi_v7_defconfig and sunxi_defconfig explicitly
set this, so this wouldn't be needed?
Cheers,
Andre.
^ permalink raw reply
* Re: [PATCH V5 3/3] ARM64 LPC: LPC driver implementation on Hip06
From: zhichang.yuan @ 2016-11-24 9:12 UTC (permalink / raw)
To: Arnd Bergmann, linux-arm-kernel
Cc: Gabriele Paoloni, 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
In-Reply-To: <4675465.4Qhqy6WU4X@wuerfel>
Hi, Arnd,
Thanks you very much!
To understand your idea more clear, I have some questions on your patch sketch.
Please check it below.
On 2016/11/24 7:23, Arnd Bergmann wrote:
> On Wednesday, November 23, 2016 6:07:11 PM CET Arnd Bergmann wrote:
>> On Wednesday, November 23, 2016 3:22:33 PM CET Gabriele Paoloni wrote:
>>> From: Arnd Bergmann [mailto:arnd@arndb.de]
>>>> On Friday, November 18, 2016 5:03:11 PM CET Gabriele Paoloni wrote:
>>
>> Please don't proliferate the use of
>> pci_pio_to_address/pci_address_to_pio here, computing the physical
>> address from the logical address is trivial, you just need to
>> subtract the start of the range that you already use when matching
>> the port number range.
>>
>> The only thing we need here is to make of_address_to_resource()
>> return the correct logical port number that was registered for
>> a given host device when asked to translate an address that
>> does not have a CPU address associated with it.
>
> Ok, I admit this was a little harder than I expected, but see below
> for a rough outline of how I think it can be done.
>
> This makes it possible to translate bus specific I/O port numbers
> from device nodes into Linux port numbers, and gives a way to register
> them. We could take this further and completely remove pci_pio_to_address
> and pci_address_to_pio if we make the I/O port translation always
> go through the io_range list, looking up up the hostbridge by fwnode,
> but we don't have to do that now.
>
> The patch is completely untested and probably buggy, it just seemed
> easier to put out a prototype than to keep going in circles with the
> discussion.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index bf601d4df8cf..6cadf0501bb0 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -730,7 +730,8 @@ static void acpi_pci_root_validate_resources(struct device *dev,
> }
> }
>
> -static void acpi_pci_root_remap_iospace(struct resource_entry *entry)
> +static void acpi_pci_root_remap_iospace(struct fwnode_handle *node,
> + struct resource_entry *entry)
> {
> #ifdef PCI_IOBASE
> struct resource *res = entry->res;
> @@ -739,11 +740,7 @@ static void acpi_pci_root_remap_iospace(struct resource_entry *entry)
> resource_size_t length = resource_size(res);
> unsigned long port;
>
> - if (pci_register_io_range(cpu_addr, length))
> - goto err;
> -
> - port = pci_address_to_pio(cpu_addr);
> - if (port == (unsigned long)-1)
> + if (pci_register_io_range(node, cpu_addr, length, &port))
> goto err;
>
> res->start = port;
> @@ -781,7 +778,8 @@ int acpi_pci_probe_root_resources(struct acpi_pci_root_info *info)
> else {
> resource_list_for_each_entry_safe(entry, tmp, list) {
> if (entry->res->flags & IORESOURCE_IO)
> - acpi_pci_root_remap_iospace(entry);
> + acpi_pci_root_remap_iospace(&device->fwnode,
> + entry);
>
> if (entry->res->flags & IORESOURCE_DISABLED)
> resource_list_destroy_entry(entry);
I think those changes in pci_root.c is only to match the new definition of
pci_register_io_range() and work for PCI I/O. It doesn't make sense for LPC, is
it right?
> diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
> index a50025a3777f..df96955a43f8 100644
> --- a/drivers/block/nbd.c
> +++ b/drivers/block/nbd.c
> @@ -760,8 +760,10 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
> set_bit(NBD_RUNNING, &nbd->runtime_flags);
> blk_mq_update_nr_hw_queues(&nbd->tag_set, nbd->num_connections);
> args = kcalloc(num_connections, sizeof(*args), GFP_KERNEL);
> - if (!args)
> + if (!args) {
> + error = -ENOMEM;
> goto out_err;
> + }
> nbd->task_recv = current;
> mutex_unlock(&nbd->config_lock);
>
I think change here is none of the business.:)
> diff --git a/drivers/of/address.c b/drivers/of/address.c
> index 02b2903fe9d2..5decaba96eed 100644
> --- a/drivers/of/address.c
> +++ b/drivers/of/address.c
> @@ -2,6 +2,7 @@
> #define pr_fmt(fmt) "OF: " fmt
>
> #include <linux/device.h>
> +#include <linux/fwnode.h>
> #include <linux/io.h>
> #include <linux/ioport.h>
> #include <linux/module.h>
> @@ -323,14 +324,9 @@ int of_pci_range_to_resource(struct of_pci_range *range,
>
> if (res->flags & IORESOURCE_IO) {
> unsigned long port;
> - err = pci_register_io_range(range->cpu_addr, range->size);
> + err = pci_register_io_range(&np->fwnode, range->cpu_addr, range->size, &port);
> if (err)
> goto invalid_range;
> - port = pci_address_to_pio(range->cpu_addr);
> - if (port == (unsigned long)-1) {
> - err = -EINVAL;
> - goto invalid_range;
> - }
> res->start = port;
> } else {
> if ((sizeof(resource_size_t) < 8) &&
> @@ -479,7 +475,7 @@ static int of_empty_ranges_quirk(struct device_node *np)
> return false;
> }
>
> -static int of_translate_one(struct device_node *parent, struct of_bus *bus,
> +static u64 of_translate_one(struct device_node *parent, struct of_bus *bus,
> struct of_bus *pbus, __be32 *addr,
> int na, int ns, int pna, const char *rprop)
> {
> @@ -507,7 +503,7 @@ static int of_translate_one(struct device_node *parent, struct of_bus *bus,
> ranges = of_get_property(parent, rprop, &rlen);
> if (ranges == NULL && !of_empty_ranges_quirk(parent)) {
> pr_debug("no ranges; cannot translate\n");
> - return 1;
> + return OF_BAD_ADDR;
> }
> if (ranges == NULL || rlen == 0) {
> offset = of_read_number(addr, na);
> @@ -528,7 +524,7 @@ static int of_translate_one(struct device_node *parent, struct of_bus *bus,
> }
> if (offset == OF_BAD_ADDR) {
> pr_debug("not found !\n");
> - return 1;
> + return offset;
> }
> memcpy(addr, ranges + na, 4 * pna);
>
> @@ -537,7 +533,10 @@ static int of_translate_one(struct device_node *parent, struct of_bus *bus,
> pr_debug("with offset: %llx\n", (unsigned long long)offset);
>
> /* Translate it into parent bus space */
> - return pbus->translate(addr, offset, pna);
> + if (pbus->translate(addr, offset, pna))
> + return OF_BAD_ADDR;
> +
> + return offset;
> }
>
> /*
> @@ -549,9 +548,14 @@ static int of_translate_one(struct device_node *parent, struct of_bus *bus,
> * that translation is impossible (that is we are not dealing with a value
> * that can be mapped to a cpu physical address). This is not really specified
> * that way, but this is traditionally the way IBM at least do things
> + *
> + * Whenever the translation fails, the *host pointer will be set to the
> + * device that lacks a tranlation, and the return code is relative to
> + * that node.
> */
> static u64 __of_translate_address(struct device_node *dev,
> - const __be32 *in_addr, const char *rprop)
> + const __be32 *in_addr, const char *rprop,
> + struct device_node **host)
> {
> struct device_node *parent = NULL;
> struct of_bus *bus, *pbus;
> @@ -564,6 +568,7 @@ static u64 __of_translate_address(struct device_node *dev,
> /* Increase refcount at current level */
> of_node_get(dev);
>
> + *host = NULL;
> /* Get parent & match bus type */
> parent = of_get_parent(dev);
> if (parent == NULL)
> @@ -600,8 +605,9 @@ static u64 __of_translate_address(struct device_node *dev,
> pbus = of_match_bus(parent);
> pbus->count_cells(dev, &pna, &pns);
> if (!OF_CHECK_COUNTS(pna, pns)) {
> - pr_err("Bad cell count for %s\n",
> - of_node_full_name(dev));
> + pr_debug("Bad cell count for %s\n",
> + of_node_full_name(dev));
> + *host = of_node_get(parent);
> break;
> }
I don't think here is the right place to fill *host. I think you want to return
the parent where the of_translate_one() failed for the 'ranges' property
missing. So, I think this seems better:
if (of_translate_one(dev, bus, pbus, addr, na, ns, pna, rprop)) {
*host = of_node_get(dev);
break;
}
>
> @@ -609,7 +615,9 @@ static u64 __of_translate_address(struct device_node *dev,
> pbus->name, pna, pns, of_node_full_name(parent));
>
> /* Apply bus translation */
> - if (of_translate_one(dev, bus, pbus, addr, na, ns, pna, rprop))
> + result = of_translate_one(dev, bus, pbus, addr, na, ns,
> + pna, rprop);
> + if (result == OF_BAD_ADDR)
> break;
>
> /* Complete the move up one level */
> @@ -628,13 +636,32 @@ static u64 __of_translate_address(struct device_node *dev,
>
> u64 of_translate_address(struct device_node *dev, const __be32 *in_addr)
> {
> - return __of_translate_address(dev, in_addr, "ranges");
> + struct device_node *host;
> + u64 ret;
> +
> + ret = __of_translate_address(dev, in_addr, "ranges", &host);
> + if (host) {
> + of_node_put(host);
> + return OF_BAD_ADDR;
> + }
> +
> + return ret;
> }
> EXPORT_SYMBOL(of_translate_address);
>
> u64 of_translate_dma_address(struct device_node *dev, const __be32 *in_addr)
> {
> - return __of_translate_address(dev, in_addr, "dma-ranges");
> + struct device_node *host;
> + u64 ret;
> +
> + ret = __of_translate_address(dev, in_addr, "dma-ranges", &host);
> +
> + if (host) {
> + of_node_put(host);
> + return OF_BAD_ADDR;
> + }
> +
> + return ret;
> }
> EXPORT_SYMBOL(of_translate_dma_address);
>
> @@ -676,29 +703,48 @@ const __be32 *of_get_address(struct device_node *dev, int index, u64 *size,
> }
> EXPORT_SYMBOL(of_get_address);
>
> +extern unsigned long extio_translate(struct fwnode_handle *node, unsigned long offset);
> +
> +u64 of_translate_ioport(struct device_node *dev, const __be32 *in_addr)
> +{
> + u64 taddr;
> + unsigned long port;
> + struct device_node *host;
> +
> + taddr = __of_translate_address(dev, in_addr, "ranges", &host);
> + if (host) {
> + /* host specific port access */
> + port = extio_translate(&host->fwnode, taddr);
> + of_node_put(host);
> + } else {
> + /* memory mapped I/O range */
> + port = pci_address_to_pio(taddr);
> + if (port == (unsigned long)-1)
> + return OF_BAD_ADDR;
> + }
> +
> + return port;
> +}
> +
> static int __of_address_to_resource(struct device_node *dev,
> const __be32 *addrp, u64 size, unsigned int flags,
> const char *name, struct resource *r)
> {
> u64 taddr;
>
> - if ((flags & (IORESOURCE_IO | IORESOURCE_MEM)) == 0)
> + if (flags & IORESOURCE_MEM)
> + taddr = of_translate_address(dev, addrp);
> + else if (flags & IORESOURCE_IO)
> + taddr = of_translate_ioport(dev, addrp);
> + else
> return -EINVAL;
> - taddr = of_translate_address(dev, addrp);
> +
> if (taddr == OF_BAD_ADDR)
> return -EINVAL;
> memset(r, 0, sizeof(struct resource));
> - if (flags & IORESOURCE_IO) {
> - unsigned long port;
> - port = pci_address_to_pio(taddr);
> - if (port == (unsigned long)-1)
> - return -EINVAL;
> - r->start = port;
> - r->end = port + size - 1;
> - } else {
> - r->start = taddr;
> - r->end = taddr + size - 1;
> - }
> +
> + r->start = taddr;
> + r->end = taddr + size - 1;
> r->flags = flags;
> r->name = name ? name : dev->full_name;
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index eda6a7cf0e54..320ab9fbf6af 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -3249,6 +3249,7 @@ EXPORT_SYMBOL(pci_request_regions_exclusive);
> #ifdef PCI_IOBASE
> struct io_range {
> struct list_head list;
> + struct fwnode_handle *node;
> phys_addr_t start;
> resource_size_t size;
> };
> @@ -3257,11 +3258,14 @@ static LIST_HEAD(io_range_list);
> static DEFINE_SPINLOCK(io_range_lock);
> #endif
>
> +#define IO_RANGE_IOEXT (resource_size_t)(-1ull)
> +
> /*
> * Record the PCI IO range (expressed as CPU physical address + size).
> * Return a negative value if an error has occured, zero otherwise
> */
> -int __weak pci_register_io_range(phys_addr_t addr, resource_size_t size)
> +int __weak pci_register_io_range(struct fwnode_handle *node, phys_addr_t addr,
> + resource_size_t size, unsigned long *port)
> {
> int err = 0;
>
> @@ -3272,7 +3276,12 @@ int __weak pci_register_io_range(phys_addr_t addr, resource_size_t size)
> /* check if the range hasn't been previously recorded */
> spin_lock(&io_range_lock);
> list_for_each_entry(range, &io_range_list, list) {
> - if (addr >= range->start && addr + size <= range->start + size) {
> + if (node == range->node)
> + goto end_register;
> +
I don't think it is safe to only check the node had been registered. For
PCI/PCIE, there is only one I/O windows in bridge, it seems ok. But for non-pci
devices, such as ISA/LPC, I wonder it is possible there are several disjoint I/O
'ranges' entries...
What parameters are necessary for linux PIO allocation?
1) For those bus devices which have no MMIO( that is to say, indirectIO is
using), I think 'addr' is not needed, but 'size' is mandatory;
I am thinking for our LPC, as there is no cpu address, we should not input
'addr' for the io range register. With 'size' as parameter, we implement a new
io range register function where can assign an unique linux PIO for this
register calling. The output linux PIO can allocate from a sub-range of whole
I/O space of [0, IO_SPACE_LIMIT]. This sub-range is specific for indirectIO, I
want to define a new macro, such as EXTIO_LIMIT, to represent the upper limit of
indirect IO space.
#if defined(CONFIG_PCI) && defined(CONFIG_INDIRECT_PIO)
#define EXTIO_LIMIT PCIBIOS_MIN_IO
#elif defined(CONFIG_INDIRECT_PIO)
#define EXTIO_LIMIT 0x1000
#else
#define EXTIO_LIMIT 0x00
#end
We should do some checkings to ensure EXTIO_LIMIT < IO_SPACE_LIMIT.
Then when someone call pci_register_io_range() or a new function for the linux
PIO register, we can allocate linux PIO from [0, EXTIO_LIMIT) for indirectIO
bus, from [EXTIO_LIMIT, IO_SPACE_LIMIT] for MMIO;
But there are issues confused me yet. For example, how to know the IO size for
the indirectIO bus? You known, there is no 'ranges' property for those buses....
2) For PCI MMIO, I think 'addr' is needed
As for the current pci_register_io_range()/pci_address_to_pio(), I have two doubts:
2.1) If there are multiple PCI host bridges which support I/O transaction, I
wonder whether the first host bridge can access the downstream devices with bus
I/O address in [0, PCIBIOS_MIN_IO)
for the first host bridge, pci_address_to_pio() will return a linux PIO range
start from 0.
But when calling __pci_assign_resource() to allocate the linux PIO for PCI/PCIE
devices/buses which are just children of first host bus, it can not allocate
linux PIO less than PCIBIOS_MIN_IO, which means kernel can not called in/out()
with port less than PCIBIOS_MIN_IO. But we had ioremap [PCI_IOBASE + 0,
PCI_IOBASE + size) to [pci_ioadd, pci_ioadd + size) before.
static int __pci_assign_resource(struct pci_bus *bus, struct pci_dev *dev,
int resno, resource_size_t size, resource_size_t align)
{
struct resource *res = dev->resource + resno;
resource_size_t min;
int ret;
min = (res->flags & IORESOURCE_IO) ? PCIBIOS_MIN_IO : PCIBIOS_MIN_MEM;
and in the later function:
static int pci_bus_alloc_from_region(struct pci_bus *bus, struct resource *res,
resource_size_t size, resource_size_t align,
resource_size_t min, unsigned long type_mask,
resource_size_t (*alignf)(void *,
....
pci_bus_for_each_resource(bus, r, i) {
resource_size_t min_used = min;
....
if (avail.start)
min_used = avail.start;
max = avail.end;
/* Ok, try it out.. */
ret = allocate_resource(r, res, size, min_used, max,
align, alignf, alignf_data);
After allocate_resource(), a IO resource is allocated, but whose 'start' is not
less than min_used.( since avail.start is 0, min_used will keep the 'min'
without change to avail.start; Should be PCIBIOS_MIN_IO).
2.2) Is it possible the return linux PIO isn't page-aligned?
When calling pci_remap_iospace(const struct resource *res, phys_addr_t
phys_addr), if res->start is not page-aligned, it seems that
ioremap_page_range() will meet some issues for duplication iorempa for same
virtual page.
of-course, if we always configure the I/O ranges size as page-aligned, it will
be OK.
I found PowerPC will ensure the 'vaddress' and the 'size' are page-aligned
before ioremap, do we need to improve the current handling in
pci_register_io_range/pci_address_to_pio?
Thanks,
Zhichang
> + if (addr != IO_RANGE_IOEXT &&
> + addr >= range->start &&
> + addr + size <= range->start + size) {
> /* range already registered, bail out */
> goto end_register;
> }
> @@ -3298,6 +3307,7 @@ int __weak pci_register_io_range(phys_addr_t addr, resource_size_t size)
> goto end_register;
> }
>
> + range->node = node;
> range->start = addr;
> range->size = size;
>
> @@ -3305,11 +3315,26 @@ int __weak pci_register_io_range(phys_addr_t addr, resource_size_t size)
>
> end_register:
> spin_unlock(&io_range_lock);
> +
> + *port = allocated_size;
> +#else
> + /*
> + * powerpc and microblaze have their own registration,
> + * just look up the value here
> + */
> + *port = pci_address_to_pio(addr);
> #endif
>
> return err;
> }
>
> +#ifdef CONFIG_IOEXT
> +int ioext_register_io_range
> +{
> + return pci_register_io_range(node, IO_RANGE_IOEXT, size, port);
> +}
> +#endif
> +
> phys_addr_t pci_pio_to_address(unsigned long pio)
> {
> phys_addr_t address = (phys_addr_t)OF_BAD_ADDR;
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 6bd94a803e8f..b7a8fa3da3ca 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1192,7 +1192,8 @@ int __must_check pci_bus_alloc_resource(struct pci_bus *bus,
> void *alignf_data);
>
>
> -int pci_register_io_range(phys_addr_t addr, resource_size_t size);
> +int pci_register_io_range(struct fwnode_handle *node, phys_addr_t addr,
> + resource_size_t size, unsigned long *port);
> unsigned long pci_address_to_pio(phys_addr_t addr);
> phys_addr_t pci_pio_to_address(unsigned long pio);
> int pci_remap_iospace(const struct resource *res, phys_addr_t phys_addr);
>
>
> .
>
^ permalink raw reply
* Re: [PATCH 5/10] dt: bindings: Add bindings for Marvell Xenon SD Host Controller
From: Arnd Bergmann @ 2016-11-24 9:11 UTC (permalink / raw)
To: Ulf Hansson
Cc: Gregory CLEMENT, Rob Herring, Ziji Hu, Adrian Hunter,
linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Jason Cooper,
Andrew Lunn, Sebastian Hesselbarth,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Thomas Petazzoni,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
Jack(SH) Zhu, Jimmy Xu, Jisheng Zhang, Nadav Haklai, Ryan Gao,
Doug Jones, Shiwu Zhang, Vi
In-Reply-To: <CAPDyKFpoifsKkse7Fc-bbZAoa=QGT=9QOQ-4D=f60ptx0hzZsA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Thursday, November 24, 2016 10:05:45 AM CET Ulf Hansson wrote:
> > You also mentioned other bindings using child nodes, but for this one
> > we have one controller with only one set of register with multiple slots
> > (Atmel is an example). Here each slot have it own set of register.
> >
> > Actually giving the fact that each slot is controlled by a different set
> > of register I wonder why the hardware can't also deduce the slot number
> > from the address register. For me it looks like an hardware bug but we
> > have to deal with it.
> >
> > Do you still think we needchild node here?
>
> Using child-nodes for slots like what's done in the atmel case, is
> currently broken. I would recommend to avoid using child-nodes for
> slots, if possible.
>
> To give you some more background, currently the mmc core treats child
> nodes as embedded non-removable cards or SDIO funcs. However, we can
> change to make child-nodes also allowed to describe slots, but it
> requires a specific compatible for "slots" and of course then we also
> need to update the DT parsing of the child-nodes in the mmc core.
>
> Documentation/devicetree/bindings/mmc/mmc.txt
> Documentation/devicetree/bindings/mmc/mmc-card.txt
I don't see anything wrong with having child nodes for the slots
even with the current binding, under one condition:
The mmc.txt binding above must refer only to the child node, while
the parent node conceptually becomes a plain bus or MFD that
happens to encapsulate multiple MMC host controllers, and possibly
provides some shared registers to them.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 5/10] dt: bindings: Add bindings for Marvell Xenon SD Host Controller
From: Ulf Hansson @ 2016-11-24 9:05 UTC (permalink / raw)
To: Gregory CLEMENT, Rob Herring
Cc: Ziji Hu, Adrian Hunter, linux-mmc@vger.kernel.org, Jason Cooper,
Andrew Lunn, Sebastian Hesselbarth, devicetree@vger.kernel.org,
Thomas Petazzoni, linux-arm-kernel@lists.infradead.org,
Jack(SH) Zhu, Jimmy Xu, Jisheng Zhang, Nadav Haklai, Ryan Gao,
Doug Jones, Shiwu Zhang, Victor Gu, Wei(SOCP) Liu
In-Reply-To: <87d1hno2d7.fsf@free-electrons.com>
On 22 November 2016 at 18:23, Gregory CLEMENT
<gregory.clement@free-electrons.com> wrote:
> Hi Rob,
>
> On jeu., nov. 10 2016, Ziji Hu <huziji@marvell.com> wrote:
>
> [...]
>
>>>> +
>>>> +- reg:
>>>> + * For "marvell,xenon-sdhci", one register area for Xenon IP.
>>>> +
>>>> + * For "marvell,armada-3700-sdhci", two register areas.
>>>> + The first one for Xenon IP register. The second one for the Armada 3700 SOC
>>>> + PHY PAD Voltage Control register.
>>>> + Please follow the examples with compatible "marvell,armada-3700-sdhci"
>>>> + in below.
>>>> + Please also check property marvell,pad-type in below.
>>>> +
>>>> +Optional Properties:
>>>> +- marvell,xenon-slotno:
>>>
>>> Multiple slots should be represented as child nodes IMO. I think some
>>> other bindings already do this.
>>>
>>
>> All the slots are entirely independent.
>> I prefer to consider it as multiple independent SDHCs placed in
>> a single IP, instead of that a IP contains multiple child slots.
>
> It was indeed what I tried to show in my answer for the 1st version:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2016-October/461860.html
>
> Maybe you missed it.
>
> You also mentioned other bindings using child nodes, but for this one
> we have one controller with only one set of register with multiple slots
> (Atmel is an example). Here each slot have it own set of register.
>
> Actually giving the fact that each slot is controlled by a different set
> of register I wonder why the hardware can't also deduce the slot number
> from the address register. For me it looks like an hardware bug but we
> have to deal with it.
>
> Do you still think we needchild node here?
Using child-nodes for slots like what's done in the atmel case, is
currently broken. I would recommend to avoid using child-nodes for
slots, if possible.
To give you some more background, currently the mmc core treats child
nodes as embedded non-removable cards or SDIO funcs. However, we can
change to make child-nodes also allowed to describe slots, but it
requires a specific compatible for "slots" and of course then we also
need to update the DT parsing of the child-nodes in the mmc core.
Documentation/devicetree/bindings/mmc/mmc.txt
Documentation/devicetree/bindings/mmc/mmc-card.txt
>
>>
>> It is unlike the implementation which put multiple slots behind PCIe EP interface. sdhci-pci.c will handle each slot init one by one.
>> If Xenon SDHC slots are represented as child nodes, there should also be a main entry in Xenon driver to init each child node one by one.
>> In my very own opinion, it is inconvenient and unnecessary.
>
Kind regards
Uffe
^ permalink raw reply
* Re: [PATCH] mfd: cpcap: Add minimal support
From: Lee Jones @ 2016-11-24 8:59 UTC (permalink / raw)
To: Tony Lindgren
Cc: Samuel Ortiz, linux-kernel, linux-omap, devicetree, Marcel Partap,
Mark Rutland, Michael Scott, Rob Herring
In-Reply-To: <20161123152652.GB4082@atomide.com>
On Wed, 23 Nov 2016, Tony Lindgren wrote:
> * 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 :)
If this IP is shared amongst vendors, it usually means it was designed
by someone else? Synopsis perhaps?
> > > + 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.
*sad face*
Does that mean you don't even know what they're for?
> > > + 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.
I'd prefer to stick to the conventions of *this* subsystem.
... and the most common convention used kernel wide:
$ git grep "ret =" | wc -l
117976
$ git grep "err =" | wc -l
56708
$ git grep "error =" | wc -l
14427
> > > + 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
I'm missing context now you've <snip>ed.
These look straight, however is the whole file lined up (as much as
*practically* possible)?
> 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? :)
Yes, that's what it means.
/me likes straight lines. :)
> Anywys thanks for the review, the rest of the comments I will just
> fix and repost.
Welcome.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH 1/7] add binding for stm32 multifunctions timer driver
From: Lee Jones @ 2016-11-24 8:52 UTC (permalink / raw)
To: Benjamin Gaignard
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, Mark Rutland,
alexandre.torgue-qxv4g6HH51o, devicetree-u79uwXL29TY76Z2rM5mHXA,
Linux Kernel Mailing List, Thierry Reding,
linux-pwm-u79uwXL29TY76Z2rM5mHXA, jic23-DgEjT+Ai2ygdnm+yROfE0A,
knaack.h-Mmb7MZpHnFY, Lars-Peter Clausen, Peter Meerwald-Stadler,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Fabrice Gasnier, Gerald Baeza, Arnaud Pouliquen, Linus Walleij,
Linaro Kernel Mailman List, Benjamin Gaignard
In-Reply-To: <CA+M3ks6rnago88JC0C5Uj6JpGZGuwyoOj-W8+r7Oj4s8_GuXyA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Rob,
Would you mind casting an eye on this please?
On Wed, 23 Nov 2016, Benjamin Gaignard wrote:
> 2016-11-23 10:21 GMT+01:00 Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>:
> > On Wed, 23 Nov 2016, Benjamin Gaignard wrote:
> >
> >> 2016-11-22 17:52 GMT+01:00 Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>:
> >> > On Tue, 22 Nov 2016, Benjamin Gaignard wrote:
> >> >
> >> >> Add bindings information for stm32 timer MFD
> >> >>
> >> >> Signed-off-by: Benjamin Gaignard <benjamin.gaignard-qxv4g6HH51o@public.gmane.org>
> >> >> ---
> >> >> .../devicetree/bindings/mfd/stm32-timer.txt | 53 ++++++++++++++++++++++
> >> >> 1 file changed, 53 insertions(+)
> >> >> create mode 100644 Documentation/devicetree/bindings/mfd/stm32-timer.txt
> >> >>
> >> >> diff --git a/Documentation/devicetree/bindings/mfd/stm32-timer.txt b/Documentation/devicetree/bindings/mfd/stm32-timer.txt
> >> >> new file mode 100644
> >> >> index 0000000..3cefce1
> >> >> --- /dev/null
> >> >> +++ b/Documentation/devicetree/bindings/mfd/stm32-timer.txt
> >> >> @@ -0,0 +1,53 @@
> >> >> +STM32 multifunctions timer driver
> >> >
> >> > "STM32 Multi-Function Timer/PWM device bindings"
> >> >
> >> > Doesn't this shared device have a better name?
> >>
> >> In SoC documentation those hardware blocks are named "advanced-control
> >> timers", "general purpose timers" or "basic timers"
> >> "stm32-timer" name is already used for clock source driver, that why I
> >> have prefix it with mfd
> >
> > MFD is a Linuxisum and has no place in hardware description.
> >
> > Please used one of the names you mentioned above.
>
> I will go for "st,stm32-advanced-timer"
>
> >
> > Hopefully the one that best fits.
> >
> >> >> +stm32 timer MFD allow to handle at the same time pwm and IIO timer devices
> >> >
> >> > No need for this sentence.
> >> >
> >> OK
> >>
> >> >> +Required parameters:
> >> >> +- compatible: must be one of the follow value:
> >> >> + "st,stm32-mfd-timer1"
> >> >> + "st,stm32-mfd-timer2"
> >> >> + "st,stm32-mfd-timer3"
> >> >> + "st,stm32-mfd-timer4"
> >> >> + "st,stm32-mfd-timer5"
> >> >> + "st,stm32-mfd-timer6"
> >> >> + "st,stm32-mfd-timer7"
> >> >> + "st,stm32-mfd-timer8"
> >> >> + "st,stm32-mfd-timer9"
> >> >> + "st,stm32-mfd-timer10"
> >> >> + "st,stm32-mfd-timer11"
> >> >> + "st,stm32-mfd-timer12"
> >> >> + "st,stm32-mfd-timer13"
> >> >> + "st,stm32-mfd-timer14"
> >> >
> >> > We don't normally number devices.
> >> >
> >> > What's stopping you from simply doing:
> >> >
> >> > pwm1: pwm1@40010000 {
> >> > compatible = "st,stm32-pwm";
> >> > };
> >> > pwm2: pwm1@40020000 {
> >> > compatible = "st,stm32-pwm";
> >> > };
> >> > pwm3: pwm1@40030000 {
> >> > compatible = "st,stm32-pwm";
> >> > };
> >> >
> >>
> >> Because each instance of the hardware is slightly different: number of
> >> pwm channels, triggers capabilities, etc ..
> >> so I need to distinguish them.
> >> Since it look to be a problem I will follow your suggestion and add a
> >> property this driver to be able to identify each instance.
> >> Do you think that "id" parameter (integer for 1 to 14) is acceptable ?
> >
> > Unfortunately not. IDs aren't allowed in DT.
> >
> > What about "pwm-chans" and "trigger"?
> >
> > pwm-chans : Number of available channels available
>
> For pwm I need those 4 properties:
> st,pwm-number: the number of PWM devices
st,pwm-num-chan is already documented.
Please use that instead of creating new properties.
> st,complementary: if exist have complementary ouput
> st,32bit-counter: if exist have 32 bits counter
> st,breakinput-polarity: if set enable break input feature.
>
> Is it acceptable from pwm maintainer point of view ?
>
> > trigger : Boolean value specifying whether a timer is present
>
> Following our discussion on IRC I will try to code for your proposal:
>
> advanced-timer@40010000 {
> compatible = "st,stm32-advanced-timer";
> reg = <0x40010000 0x400>;
> clocks = <&rcc 0 160>;
> clock-names = "clk_int";
>
> pwm@0 {
> compatible = "st,stm32-pwm";
> st,pwm-number= <4>;
> st,complementary;
> st,breakinput;
> };
>
> timer@0 {
> reg = <1>;
> compatible = "st,stm32-iio-timer";
> interrupts = <27>;
> triggers = <5 2 3 4>;
> };
> };
>
> triggers parameter will be used to know which trigger are valid for
> the IIO device
Except for "st,pwm-number" as mentioned above, this looks good to me.
Rob, would what do you think?
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH v2] ARM: dts: da850: add the mstpri and ddrctl nodes
From: Bartosz Golaszewski @ 2016-11-24 8:48 UTC (permalink / raw)
To: Sekhar Nori
Cc: David Lechner, Kevin Hilman, Michael Turquette, 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: <20fbc946-d56c-31a3-4ae7-cf61df96a3c3-l0cyMroinI0@public.gmane.org>
2016-11-24 6:03 GMT+01:00 Sekhar Nori <nsekhar-l0cyMroinI0@public.gmane.org>:
> On Thursday 24 November 2016 04:18 AM, David Lechner wrote:
>> On 11/23/2016 04:32 PM, Kevin Hilman wrote:
>>> David Lechner <david-nq/r/kbU++upp/zk7JDF2g@public.gmane.org> writes:
>>>
>>>> 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.
>>>
>>> Then the driver is too noisy and should be cleaned up.
>>>
>>>> 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.
>>>
>>> IMO, what exactly are the error messages? Sounds like the driver is
>>> being too verbose, and calling things errors that are not really errors.
>>
>> It is just one line per driver.
>>
>> dev_err(dev, "no master priorities defined for this board\n");
>>
>> and
>>
>> dev_err(dev, "no settings defined for this board\n");
>>
>>
>> Since "ti,da850-lcdk" is the only board supported in these drivers, all
>> other boards will see these error messages.
>
> Thats pretty bad. Sorry about that. The original justification for
> keeping them enabled all the time was that they are in-SoC modules with
> no external dependencies (like IO lines or voltage rails) so they can be
> enabled on all boards that use DA850. While that remains true, the
> configuration itself is board specific.
>
> I think the error messages are still useful, so instead of silencing
> them, I think we should go back to keeping these nodes disabled by
> default and enabling only on boards which have support for it in the driver.
>
> Thanks,
> Sekhar
I'll send a patch.
Thanks,
Bartosz
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 1/5] Documentation: pinctrl: palmas: Add ti,palmas-powerhold-override property definition
From: Lee Jones @ 2016-11-24 8:45 UTC (permalink / raw)
To: Keerthy
Cc: Rob Herring, tony, linux-omap, linux-kernel, devicetree,
linux-gpio, nm, t-kristo
In-Reply-To: <23bf98ee-bcdc-e100-54ce-6f4b1e13ffcf@ti.com>
On Thu, 24 Nov 2016, Keerthy wrote:
>
>
> On Tuesday 15 November 2016 07:13 AM, Rob Herring wrote:
> > On Thu, Nov 10, 2016 at 10:39:16AM +0530, Keerthy wrote:
> > > GPIO7 is configured in POWERHOLD mode which has higher priority
> > > over DEV_ON bit and keeps the PMIC supplies on even after the DEV_ON
> > > bit is turned off. This property enables driver to over ride the
> > > POWERHOLD value to GPIO7 so as to turn off the PMIC in power off
> > > scenarios.
> > >
> > > Signed-off-by: Keerthy <j-keerthy@ti.com>
> > > ---
> > > Documentation/devicetree/bindings/pinctrl/pinctrl-palmas.txt | 9 +++++++++
> > > 1 file changed, 9 insertions(+)
> >
> > Acked-by: Rob Herring <robh@kernel.org>
>
> Tony,
>
> Are you planning to pick this one as well?
This should be taken by LinusW.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* [PATCH V4 2/2] pinctrl: tegra: Add driver to configure voltage and power of io pads
From: Laxman Dewangan @ 2016-11-24 8:38 UTC (permalink / raw)
To: linus.walleij, robh+dt, mark.rutland, swarren, thierry.reding,
gnurou, jonathanh, joe
Cc: yamada.masahiro, linux-gpio, devicetree, linux-tegra,
linux-kernel, Laxman Dewangan
In-Reply-To: <1479976734-30498-1-git-send-email-ldewangan@nvidia.com>
NVIDIA Tegra124 and later SoCs support the multi-voltage level and
low power state of some of its IO pads. The IO pads can work in
the voltage of the 1.8V and 3.3V of IO voltage from IO power rail
sources. When IO interfaces are not used then IO pads can be
configure in low power state to reduce the power consumption from
that IO pads.
On Tegra124, the voltage level of IO power rail source is auto
detected by hardware(SoC) and hence it is only require to configure
in low power mode if IO pads are not used.
On T210 onwards, the auto-detection of voltage level from IO power
rail is removed from SoC and hence SW need to configure the PMC
register explicitly to set proper voltage in IO pads based on
IO rail power source voltage.
This driver adds the IO pad driver to configure the power state and
IO pad voltage based on the usage and power tree via pincontrol
framework. The configuration can be static and dynamic.
Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
Changes from V1:
- Dropped the custom properties to set pad voltage and use regulator.
- Added support for regulator to get vottage in boot and configure IO
pad voltage.
- Add support for callback to handle regulator notification and configure
IO pad voltage based on voltage change.
Changes from V2:
Mostly nit changes per Jon's feedback i.e. use macros for voltage, added
comment on macros, reduce the structure and variable name size, optimise
number of variables, and allocate memory for regulator info when it needed.
Changes from V3:
Use the devm_regulator_get() instead of devm_regulator_get_optional().
drivers/pinctrl/tegra/Kconfig | 12 +
drivers/pinctrl/tegra/Makefile | 1 +
drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c | 530 +++++++++++++++++++++++++++
3 files changed, 543 insertions(+)
create mode 100644 drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c
diff --git a/drivers/pinctrl/tegra/Kconfig b/drivers/pinctrl/tegra/Kconfig
index 24e20cc..6004e5c 100644
--- a/drivers/pinctrl/tegra/Kconfig
+++ b/drivers/pinctrl/tegra/Kconfig
@@ -23,6 +23,18 @@ config PINCTRL_TEGRA210
bool
select PINCTRL_TEGRA
+config PINCTRL_TEGRA_IO_PAD
+ bool "Tegra IO pad Control Driver"
+ depends on ARCH_TEGRA && REGULATOR
+ select PINCONF
+ select PINMUX
+ help
+ NVIDIA Tegra124/210 SoC has IO pads which supports multi-voltage
+ level of interfacing and deep power down mode of IO pads. The
+ voltage of IO pads are SW configurable based on IO rail of that
+ pads on T210. This driver provides the interface to change IO pad
+ voltage and power state via pincontrol interface.
+
config PINCTRL_TEGRA_XUSB
def_bool y if ARCH_TEGRA
select GENERIC_PHY
diff --git a/drivers/pinctrl/tegra/Makefile b/drivers/pinctrl/tegra/Makefile
index d9ea2be..3ebaaa2 100644
--- a/drivers/pinctrl/tegra/Makefile
+++ b/drivers/pinctrl/tegra/Makefile
@@ -4,4 +4,5 @@ obj-$(CONFIG_PINCTRL_TEGRA30) += pinctrl-tegra30.o
obj-$(CONFIG_PINCTRL_TEGRA114) += pinctrl-tegra114.o
obj-$(CONFIG_PINCTRL_TEGRA124) += pinctrl-tegra124.o
obj-$(CONFIG_PINCTRL_TEGRA210) += pinctrl-tegra210.o
+obj-$(CONFIG_PINCTRL_TEGRA_IO_PAD) += pinctrl-tegra-io-pad.o
obj-$(CONFIG_PINCTRL_TEGRA_XUSB) += pinctrl-tegra-xusb.o
diff --git a/drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c b/drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c
new file mode 100644
index 0000000..aab02d0
--- /dev/null
+++ b/drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c
@@ -0,0 +1,530 @@
+/*
+ * pinctrl-tegra-io-pad: IO PAD driver for configuration of IO rail and deep
+ * Power Down mode via pinctrl framework.
+ *
+ * Copyright (C) 2016 NVIDIA CORPORATION. All rights reserved.
+ *
+ * Author: Laxman Dewangan <ldewangan@nvidia.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/delay.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinconf-generic.h>
+#include <linux/pinctrl/pinconf.h>
+#include <linux/pinctrl/pinmux.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
+#include <soc/tegra/pmc.h>
+
+#include "../core.h"
+#include "../pinconf.h"
+#include "../pinctrl-utils.h"
+
+#define TEGRA_IO_RAIL_1800000UV 1800000
+#define TEGRA_IO_RAIL_3300000UV 3300000
+
+/* Covert IO voltage to IO pad voltage enum */
+#define tegra_io_uv_to_io_pads_uv(io_uv) \
+ (((io_uv) == TEGRA_IO_RAIL_1800000UV) ? \
+ TEGRA_IO_PAD_1800000UV : TEGRA_IO_PAD_3300000UV)
+
+#define tegra_io_voltage_is_valid(io_uv) \
+ ({ typeof(io_uv) io_uv_ = (io_uv); \
+ ((io_uv_ == TEGRA_IO_RAIL_1800000UV) || \
+ (io_uv_ == TEGRA_IO_RAIL_3300000UV)); })
+
+struct tegra_io_pads_cfg {
+ const char *name;
+ const unsigned int pins[1];
+ const char *vsupply;
+ enum tegra_io_pad id;
+ bool supports_low_power;
+};
+
+struct tegra_io_pads_soc_data {
+ const struct tegra_io_pads_cfg *cfg;
+ int num_cfg;
+ const struct pinctrl_pin_desc *desc;
+ int num_desc;
+};
+
+struct tegra_io_pads_info {
+ struct device *dev;
+ struct pinctrl_dev *pctl;
+ const struct tegra_io_pads_soc_data *soc_data;
+};
+
+struct tegra_io_pads_regulator_info {
+ struct tegra_io_pads_info *tiopi;
+ const struct tegra_io_pads_cfg *cfg;
+ struct regulator *regulator;
+ struct notifier_block regulator_nb;
+};
+
+static int tegra_io_pads_pinctrl_get_groups_count(struct pinctrl_dev *pctldev)
+{
+ struct tegra_io_pads_info *tiopi = pinctrl_dev_get_drvdata(pctldev);
+
+ return tiopi->soc_data->num_cfg;
+}
+
+static const char *tegra_io_pads_pinctrl_get_group_name(
+ struct pinctrl_dev *pctldev, unsigned int group)
+{
+ struct tegra_io_pads_info *tiopi = pinctrl_dev_get_drvdata(pctldev);
+
+ return tiopi->soc_data->cfg[group].name;
+}
+
+static int tegra_io_pads_pinctrl_get_group_pins(struct pinctrl_dev *pctldev,
+ unsigned int group,
+ const unsigned int **pins,
+ unsigned int *num_pins)
+{
+ struct tegra_io_pads_info *tiopi = pinctrl_dev_get_drvdata(pctldev);
+
+ *pins = tiopi->soc_data->cfg[group].pins;
+ *num_pins = 1;
+
+ return 0;
+}
+
+static const struct pinctrl_ops tegra_io_pads_pinctrl_ops = {
+ .get_groups_count = tegra_io_pads_pinctrl_get_groups_count,
+ .get_group_name = tegra_io_pads_pinctrl_get_group_name,
+ .get_group_pins = tegra_io_pads_pinctrl_get_group_pins,
+ .dt_node_to_map = pinconf_generic_dt_node_to_map_pin,
+ .dt_free_map = pinctrl_utils_free_map,
+};
+
+static int tegra_io_pads_pinconf_get(struct pinctrl_dev *pctldev,
+ unsigned int pin, unsigned long *config)
+{
+ struct tegra_io_pads_info *tiopi = pinctrl_dev_get_drvdata(pctldev);
+ int param = pinconf_to_config_param(*config);
+ const struct tegra_io_pads_cfg *cfg = &tiopi->soc_data->cfg[pin];
+ int arg = 0;
+ int ret;
+
+ switch (param) {
+ case PIN_CONFIG_LOW_POWER_MODE:
+ if (!cfg->supports_low_power) {
+ dev_err(tiopi->dev,
+ "IO pad %s does not support low power\n",
+ cfg->name);
+ return -EINVAL;
+ }
+
+ ret = tegra_io_pad_power_get_status(cfg->id);
+ if (ret < 0)
+ return ret;
+ arg = !ret;
+ break;
+
+ default:
+ dev_err(tiopi->dev, "The parameter %d not supported\n", param);
+ return -EINVAL;
+ }
+
+ *config = pinconf_to_config_packed(param, (u16)arg);
+
+ return 0;
+}
+
+static int tegra_io_pads_pinconf_set(struct pinctrl_dev *pctldev,
+ unsigned int pin, unsigned long *configs,
+ unsigned int num_configs)
+{
+ struct tegra_io_pads_info *tiopi = pinctrl_dev_get_drvdata(pctldev);
+ const struct tegra_io_pads_cfg *cfg = &tiopi->soc_data->cfg[pin];
+ int i;
+
+ for (i = 0; i < num_configs; i++) {
+ int ret;
+ int param = pinconf_to_config_param(configs[i]);
+ u16 param_val = pinconf_to_config_argument(configs[i]);
+
+ switch (param) {
+ case PIN_CONFIG_LOW_POWER_MODE:
+ if (!cfg->supports_low_power) {
+ dev_err(tiopi->dev,
+ "IO pad %s does not support low power\n",
+ cfg->name);
+ return -EINVAL;
+ }
+ if (param_val)
+ ret = tegra_io_pad_power_disable(cfg->id);
+ else
+ ret = tegra_io_pad_power_enable(cfg->id);
+ if (ret < 0) {
+ dev_err(tiopi->dev,
+ "Failed to set DPD %d of io-pad %s: %d\n",
+ param_val, cfg->name, ret);
+ return ret;
+ }
+ break;
+
+ default:
+ dev_err(tiopi->dev, "The parameter %d not supported\n",
+ param);
+ return -EINVAL;
+ }
+ }
+
+ return 0;
+}
+
+static const struct pinconf_ops tegra_io_pads_pinconf_ops = {
+ .pin_config_get = tegra_io_pads_pinconf_get,
+ .pin_config_set = tegra_io_pads_pinconf_set,
+};
+
+static struct pinctrl_desc tegra_io_pads_pinctrl_desc = {
+ .name = "pinctrl-tegra-io-pads",
+ .pctlops = &tegra_io_pads_pinctrl_ops,
+ .confops = &tegra_io_pads_pinconf_ops,
+};
+
+static int tegra_io_pads_rail_change_notify_cb(struct notifier_block *nb,
+ unsigned long event, void *data)
+{
+ struct tegra_io_pads_regulator_info *rinfo;
+ struct pre_voltage_change_data *vdata;
+ unsigned long int io_volt_uv;
+ enum tegra_io_pad_voltage pad_volt;
+ int ret;
+
+ rinfo = container_of(nb, struct tegra_io_pads_regulator_info,
+ regulator_nb);
+
+ switch (event) {
+ case REGULATOR_EVENT_PRE_VOLTAGE_CHANGE:
+ vdata = data;
+
+ if (!tegra_io_voltage_is_valid(vdata->old_uV) ||
+ !tegra_io_voltage_is_valid(vdata->min_uV)) {
+ dev_err(rinfo->tiopi->dev,
+ "IO rail %s voltage is not 1.8/3.3V: %lu:%lu\n",
+ rinfo->cfg->name, vdata->old_uV, vdata->min_uV);
+ return -EINVAL;
+ }
+
+ /**
+ * Change IO pad voltage before changing IO voltage when it
+ * changes from 1.8V to 3.3V
+ */
+ if (vdata->min_uV == TEGRA_IO_RAIL_1800000UV)
+ break;
+
+ ret = tegra_io_pad_set_voltage(rinfo->cfg->id,
+ TEGRA_IO_PAD_3300000UV);
+ if (ret < 0) {
+ dev_err(rinfo->tiopi->dev,
+ "Failed to set voltage %lu of pad %s: %d\n",
+ vdata->min_uV, rinfo->cfg->name, ret);
+ return ret;
+ }
+ break;
+
+ case REGULATOR_EVENT_VOLTAGE_CHANGE:
+ io_volt_uv = (unsigned long)data;
+ ret = tegra_io_pad_get_voltage(rinfo->cfg->id);
+ if (ret < 0) {
+ dev_err(rinfo->tiopi->dev,
+ "Failed to get IO pad voltage: %d\n", ret);
+ return ret;
+ }
+
+ if (!tegra_io_voltage_is_valid(io_volt_uv)) {
+ dev_err(rinfo->tiopi->dev,
+ "IO rail %s voltage is not 1.8/3.3V: %lu\n",
+ rinfo->cfg->name, io_volt_uv);
+ return -EINVAL;
+ }
+
+ /*
+ * If IO pad configuration matching with IO rail voltage then
+ * do nothing.
+ */
+ if (((io_volt_uv == TEGRA_IO_RAIL_1800000UV) &&
+ (ret == TEGRA_IO_PAD_1800000UV)) ||
+ ((io_volt_uv == TEGRA_IO_RAIL_3300000UV) &&
+ (ret == TEGRA_IO_PAD_3300000UV)))
+ break;
+
+ ret = tegra_io_pad_set_voltage(rinfo->cfg->id,
+ TEGRA_IO_PAD_1800000UV);
+ if (ret < 0) {
+ dev_err(rinfo->tiopi->dev,
+ "Failed to set voltage %lu of pad %s: %d\n",
+ vdata->min_uV, rinfo->cfg->name, ret);
+ return ret;
+ }
+ break;
+
+ case REGULATOR_EVENT_ABORT_VOLTAGE_CHANGE:
+ io_volt_uv = (unsigned long)data;
+
+ if (!tegra_io_voltage_is_valid(io_volt_uv)) {
+ dev_err(rinfo->tiopi->dev,
+ "IO rail %s voltage is not 1.8/3.3V: %lu\n",
+ rinfo->cfg->name, io_volt_uv);
+ return -EINVAL;
+ }
+
+ pad_volt = tegra_io_uv_to_io_pads_uv(io_volt_uv);
+ ret = tegra_io_pad_set_voltage(rinfo->cfg->id, pad_volt);
+ if (ret < 0) {
+ dev_err(rinfo->tiopi->dev,
+ "Failed to set voltage %lu of pad %s: %d\n",
+ io_volt_uv, rinfo->cfg->name, ret);
+ return ret;
+ }
+ break;
+
+ default:
+ break;
+ }
+
+ return NOTIFY_OK;
+}
+
+static int tegra_io_pads_pinctrl_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ const struct platform_device_id *id = platform_get_device_id(pdev);
+ const struct tegra_io_pads_soc_data *soc_data =
+ (const struct tegra_io_pads_soc_data *)id->driver_data;
+ struct tegra_io_pads_info *tiopi;
+ int ret, i;
+
+ if (!pdev->dev.parent->of_node) {
+ dev_err(dev, "PMC should be register from DT\n");
+ return -ENODEV;
+ }
+
+ tiopi = devm_kzalloc(dev, sizeof(*tiopi), GFP_KERNEL);
+ if (!tiopi)
+ return -ENOMEM;
+
+ tiopi->dev = &pdev->dev;
+ pdev->dev.of_node = pdev->dev.parent->of_node;
+ tiopi->soc_data = soc_data;
+
+ for (i = 0; i < soc_data->num_cfg; ++i) {
+ struct tegra_io_pads_regulator_info *rinfo;
+ enum tegra_io_pad_voltage pad_volt;
+ int io_volt_uv;
+
+ if (!soc_data->cfg[i].vsupply)
+ continue;
+
+ rinfo = devm_kzalloc(dev, sizeof(*rinfo), GFP_KERNEL);
+ if (!rinfo)
+ return -ENOMEM;
+
+ rinfo->tiopi = tiopi;
+ rinfo->cfg = &soc_data->cfg[i];
+
+ rinfo->regulator = devm_regulator_get(dev,
+ soc_data->cfg[i].vsupply);
+ if (IS_ERR(rinfo->regulator)) {
+ ret = PTR_ERR(rinfo->regulator);
+ if (ret == -EPROBE_DEFER)
+ return ret;
+ continue;
+ }
+
+ io_volt_uv = regulator_get_voltage(rinfo->regulator);
+ if (io_volt_uv < 0) {
+ dev_warn(dev, "Failed to get voltage for rail %s: %d\n",
+ soc_data->cfg[i].vsupply, io_volt_uv);
+ continue;
+ }
+
+ if (!tegra_io_voltage_is_valid(io_volt_uv)) {
+ dev_warn(dev, "IO rail %s voltage is not 1.8/3.3V: %d\n",
+ soc_data->cfg[i].vsupply, io_volt_uv);
+ continue;
+ }
+
+ pad_volt = tegra_io_uv_to_io_pads_uv(io_volt_uv);
+ ret = tegra_io_pad_set_voltage(soc_data->cfg[i].id, pad_volt);
+ if (ret < 0) {
+ dev_err(dev, "Failed to set voltage %d of pad %s: %d\n",
+ io_volt_uv, soc_data->cfg[i].name, ret);
+ return ret;
+ }
+
+ rinfo->regulator_nb.notifier_call =
+ tegra_io_pads_rail_change_notify_cb;
+ ret = devm_regulator_register_notifier(rinfo->regulator,
+ &rinfo->regulator_nb);
+ if (ret < 0) {
+ dev_err(dev, "Failed to register regulator %s notifier: %d\n",
+ soc_data->cfg[i].name, ret);
+ return ret;
+ }
+ }
+
+ tegra_io_pads_pinctrl_desc.pins = tiopi->soc_data->desc;
+ tegra_io_pads_pinctrl_desc.npins = tiopi->soc_data->num_desc;
+ platform_set_drvdata(pdev, tiopi);
+
+ tiopi->pctl = devm_pinctrl_register(dev, &tegra_io_pads_pinctrl_desc,
+ tiopi);
+ if (IS_ERR(tiopi->pctl)) {
+ ret = PTR_ERR(tiopi->pctl);
+ dev_err(dev, "Failed to register io-pad pinctrl driver: %d\n",
+ ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+#define TEGRA124_PAD_INFO_TABLE(_entry_) \
+ _entry_(0, "audio", AUDIO, true, NULL), \
+ _entry_(1, "bb", BB, true, NULL), \
+ _entry_(2, "cam", CAM, true, NULL), \
+ _entry_(3, "comp", COMP, true, NULL), \
+ _entry_(4, "csia", CSIA, true, NULL), \
+ _entry_(5, "csib", CSIB, true, NULL), \
+ _entry_(6, "csie", CSIE, true, NULL), \
+ _entry_(7, "dsi", DSI, true, NULL), \
+ _entry_(8, "dsib", DSIB, true, NULL), \
+ _entry_(9, "dsic", DSIC, true, NULL), \
+ _entry_(10, "dsid", DSID, true, NULL), \
+ _entry_(11, "hdmi", HDMI, true, NULL), \
+ _entry_(12, "hsic", HSIC, true, NULL), \
+ _entry_(13, "hv", HV, true, NULL), \
+ _entry_(14, "lvds", LVDS, true, NULL), \
+ _entry_(15, "mipi-bias", MIPI_BIAS, true, NULL), \
+ _entry_(16, "nand", NAND, true, NULL), \
+ _entry_(17, "pex-bias", PEX_BIAS, true, NULL), \
+ _entry_(18, "pex-clk1", PEX_CLK1, true, NULL), \
+ _entry_(19, "pex-clk2", PEX_CLK2, true, NULL), \
+ _entry_(20, "pex-ctrl", PEX_CNTRL, true, NULL), \
+ _entry_(21, "sdmmc1", SDMMC1, true, NULL), \
+ _entry_(22, "sdmmc3", SDMMC3, true, NULL), \
+ _entry_(23, "sdmmc4", SDMMC4, true, NULL), \
+ _entry_(24, "sys-ddc", SYS_DDC, true, NULL), \
+ _entry_(25, "uart", UART, true, NULL), \
+ _entry_(26, "usb0", USB0, true, NULL), \
+ _entry_(27, "usb1", USB1, true, NULL), \
+ _entry_(28, "usb2", USB2, true, NULL), \
+ _entry_(29, "usb-bias", USB_BIAS, true, NULL)
+
+#define TEGRA210_PAD_INFO_TABLE(_entry_) \
+ _entry_(0, "audio", AUDIO, true, "vddio-audio"), \
+ _entry_(1, "audio-hv", AUDIO_HV, true, "vddio-audio-hv"), \
+ _entry_(2, "cam", CAM, true, "vddio-cam"), \
+ _entry_(3, "csia", CSIA, true, NULL), \
+ _entry_(4, "csib", CSIB, true, NULL), \
+ _entry_(5, "csic", CSIC, true, NULL), \
+ _entry_(6, "csid", CSID, true, NULL), \
+ _entry_(7, "csie", CSIE, true, NULL), \
+ _entry_(8, "csif", CSIF, true, NULL), \
+ _entry_(9, "dbg", DBG, true, "vddio-dbg"), \
+ _entry_(10, "debug-nonao", DEBUG_NONAO, true, NULL), \
+ _entry_(11, "dmic", DMIC, true, "vddio-dmic"), \
+ _entry_(12, "dp", DP, true, NULL), \
+ _entry_(13, "dsi", DSI, true, NULL), \
+ _entry_(14, "dsib", DSIB, true, NULL), \
+ _entry_(15, "dsic", DSIC, true, NULL), \
+ _entry_(16, "dsid", DSID, true, NULL), \
+ _entry_(17, "emmc", SDMMC4, true, NULL), \
+ _entry_(18, "emmc2", EMMC2, true, NULL), \
+ _entry_(19, "gpio", GPIO, true, "vddio-gpio"), \
+ _entry_(20, "hdmi", HDMI, true, NULL), \
+ _entry_(21, "hsic", HSIC, true, NULL), \
+ _entry_(22, "lvds", LVDS, true, NULL), \
+ _entry_(23, "mipi-bias", MIPI_BIAS, true, NULL), \
+ _entry_(24, "pex-bias", PEX_BIAS, true, NULL), \
+ _entry_(25, "pex-clk1", PEX_CLK1, true, NULL), \
+ _entry_(26, "pex-clk2", PEX_CLK2, true, NULL), \
+ _entry_(27, "pex-ctrl", PEX_CNTRL, false, "vddio-pex-ctrl"), \
+ _entry_(28, "sdmmc1", SDMMC1, true, "vddio-sdmmc1"), \
+ _entry_(29, "sdmmc3", SDMMC3, true, "vddio-sdmmc3"), \
+ _entry_(30, "spi", SPI, true, "vddio-spi"), \
+ _entry_(31, "spi-hv", SPI_HV, true, "vddio-spi-hv"), \
+ _entry_(32, "uart", UART, true, "vddio-uart"), \
+ _entry_(33, "usb0", USB0, true, NULL), \
+ _entry_(34, "usb1", USB1, true, NULL), \
+ _entry_(35, "usb2", USB2, true, NULL), \
+ _entry_(36, "usb3", USB3, true, NULL), \
+ _entry_(37, "usb-bias", USB_BIAS, true, NULL)
+
+#define TEGRA_IO_PAD_INFO(_pin, _name, _id, _lpstate, _vsupply) \
+ { \
+ .name = _name, \
+ .pins = {(_pin)}, \
+ .id = TEGRA_IO_PAD_##_id, \
+ .vsupply = (_vsupply), \
+ .supports_low_power = (_lpstate), \
+ }
+
+static const struct tegra_io_pads_cfg tegra124_io_pads_cfg_info[] = {
+ TEGRA124_PAD_INFO_TABLE(TEGRA_IO_PAD_INFO),
+};
+
+static const struct tegra_io_pads_cfg tegra210_io_pads_cfg_info[] = {
+ TEGRA210_PAD_INFO_TABLE(TEGRA_IO_PAD_INFO),
+};
+
+#define TEGRA_IO_PAD_DESC(_pin, _name, _id, _lpstate, _vsupply) \
+ PINCTRL_PIN(_pin, _name)
+
+static const struct pinctrl_pin_desc tegra124_io_pads_pinctrl_desc[] = {
+ TEGRA124_PAD_INFO_TABLE(TEGRA_IO_PAD_DESC),
+};
+
+static const struct pinctrl_pin_desc tegra210_io_pads_pinctrl_desc[] = {
+ TEGRA210_PAD_INFO_TABLE(TEGRA_IO_PAD_DESC),
+};
+
+static const struct tegra_io_pads_soc_data tegra124_io_pad_soc_data = {
+ .desc = tegra124_io_pads_pinctrl_desc,
+ .num_desc = ARRAY_SIZE(tegra124_io_pads_pinctrl_desc),
+ .cfg = tegra124_io_pads_cfg_info,
+ .num_cfg = ARRAY_SIZE(tegra124_io_pads_cfg_info),
+};
+
+static const struct tegra_io_pads_soc_data tegra210_io_pad_soc_data = {
+ .desc = tegra210_io_pads_pinctrl_desc,
+ .num_desc = ARRAY_SIZE(tegra210_io_pads_pinctrl_desc),
+ .cfg = tegra210_io_pads_cfg_info,
+ .num_cfg = ARRAY_SIZE(tegra210_io_pads_cfg_info),
+};
+
+static const struct platform_device_id tegra_io_pads_dev_id[] = {
+ {
+ .name = "pinctrl-t124-io-pad",
+ .driver_data = (kernel_ulong_t)&tegra124_io_pad_soc_data,
+ }, {
+ .name = "pinctrl-t210-io-pad",
+ .driver_data = (kernel_ulong_t)&tegra210_io_pad_soc_data,
+ }, {
+ },
+};
+MODULE_DEVICE_TABLE(platform, tegra_io_pads_dev_id);
+
+static struct platform_driver tegra_io_pads_pinctrl_driver = {
+ .driver = {
+ .name = "pinctrl-tegra-io-pad",
+ },
+ .probe = tegra_io_pads_pinctrl_probe,
+ .id_table = tegra_io_pads_dev_id,
+};
+
+module_platform_driver(tegra_io_pads_pinctrl_driver);
+
+MODULE_DESCRIPTION("NVIDIA TEGRA IO pad Control Driver");
+MODULE_AUTHOR("Laxman Dewangan <ldewangan@nvidia.com>");
+MODULE_LICENSE("GPL v2");
--
2.1.4
^ permalink raw reply related
* [PATCH V4 1/2] pinctrl: tegra: Add DT binding for io pads control
From: Laxman Dewangan @ 2016-11-24 8:38 UTC (permalink / raw)
To: linus.walleij, robh+dt, mark.rutland, swarren, thierry.reding,
gnurou, jonathanh, joe
Cc: yamada.masahiro, linux-gpio, devicetree, linux-tegra,
linux-kernel, Laxman Dewangan
In-Reply-To: <1479976734-30498-1-git-send-email-ldewangan@nvidia.com>
NVIDIA Tegra124 and later SoCs support the multi-voltage level and
low power state of some of its IO pads. The IO pads can work in
the voltage of the 1.8V and 3.3V of IO voltage from IO power rail
sources. When IO interfaces are not used then IO pads can be
configure in low power state to reduce the power consumption from
that IO pads.
On Tegra124, the voltage level of IO power rail source is auto
detected by hardware(SoC) and hence it is only require to configure
in low power mode if IO pads are not used.
On T210 onwards, the auto-detection of voltage level from IO power
rail is removed from SoC and hence SW need to configure the PMC
register explicitly to set proper voltage in IO pads based on
IO rail power source voltage.
Add DT binding document for detailing the DT properties for
configuring IO pads voltage levels and its power state.
Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
Changes from V1:
New in series based on pinctrl driver requirement.
Changes from V2:
Updated the statement to say 1.8V and 3.3V as nominal voltage.
Corrected DT example by adding -supply and taken care of V1 review
from Rob.
.../bindings/pinctrl/nvidia,tegra-io-pad.txt | 126 +++++++++++++++++++++
1 file changed, 126 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pinctrl/nvidia,tegra-io-pad.txt
diff --git a/Documentation/devicetree/bindings/pinctrl/nvidia,tegra-io-pad.txt b/Documentation/devicetree/bindings/pinctrl/nvidia,tegra-io-pad.txt
new file mode 100644
index 0000000..a88c484
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/nvidia,tegra-io-pad.txt
@@ -0,0 +1,126 @@
+NVIDIA Tegra PMC IO pad controller
+
+NVIDIA Tegra124 and later SoCs support the multi-voltage level and low power
+state of some of its IO pads. When IO interface are not used then IO pads can
+be configure in low power state to reduce the power from that IO pads. The IO
+pads can work in the nominal IO voltage of 1.8V and 3.3V from power rail
+sources.
+
+On Tegra124, the voltage of IO power rail source is auto detected by SoC and
+hence it is only require to configure in low power mode if IO pads are not
+used.
+
+On T210 onwards, the HW based auto-detection for IO voltage is removed and
+hence SW need to configure the PMC register explicitly, to set proper voltage
+in IO pads, based on IO rail power source voltage.
+
+The voltage configurations and low power state of IO pads should be done in
+boot if it is not going to change otherwise dynamically based on IO rail
+voltage on that IO pads and usage of IO pads
+
+The DT property of the IO pads must be under the node of pmc i.e.
+pmc@7000e400 for Tegra124 onwards.
+
+Please refer to <pinctrl-bindings.txt> in this directory for details of the
+common pinctrl bindings used by client devices, including the meaning of the
+phrase "pin configuration node".
+
+Tegra's pin configuration nodes act as a container for an arbitrary number of
+subnodes. Each of these subnodes represents some desired configuration for an
+IO pads, or a list of IO pads. This configuration can include the voltage and
+power enable/disable control
+
+The name of each subnode is not important; all subnodes should be enumerated
+and processed purely based on their content. Each subnode only affects those
+parameters that are explicitly listed. Unspecified is represented as an absent
+property,
+
+See the TRM to determine which properties and values apply to each IO pads.
+
+Required subnode-properties:
+==========================
+- pins : An array of strings. Each string contains the name of an IO pads. Valid
+ values for these names are listed below.
+
+Optional subnode-properties:
+==========================
+Following properties are supported from generic pin configuration explained
+in <dt-bindings/pinctrl/pinctrl-binding.txt>.
+low-power-enable: enable low power mode.
+low-power-disable: disable low power mode.
+
+Valid values for pin for T124 are:
+ audio, bb, cam, comp, csia, csib, csie, dsi, dsib, dsic, dsid, hdmi,
+ hsic, hv, lvds, mipi-bias, nand, pex-bias, pex-clk1, pex-clk2,
+ pex-ctrl, sdmmc1, sdmmc3, sdmmc4, sys-ddc, uart, usb0, usb1, usb2,
+ usb-bias
+
+Valid values for pin for T210 are:
+ audio, audio-hv, cam, csia, csib, csic, csid, csie, csif,
+ dbg, debug-nonao, dmic, dp, dsi, dsib, dsic, dsid, emmc, emmc2,
+ gpio, hdmi, hsic, lvds, mipi-bias, pex-bias, pex-clk1, pex-clk2,
+ pex-ctrl, sdmmc1, sdmmc3, spi, spi-hv, uart, usb-bias, usb0,
+ usb1, usb2, usb3.
+
+To find out the IO rail voltage for setting the voltage of IO pad by SW,
+the regulator supply handle must provided from the DT and it is explained
+in the regulator DT binding document
+ <devicetree/bindings/regulator/regulator.txt>.
+For example, for GPIO rail the supply name is vddio-gpio and regulator
+handle is supplied from DT as
+ vddio-gpio-supply = <®ulator_xyz>;
+
+For T210, following IO pads support the 1.8V/3.3V and the corresponding
+IO voltage pin names are as follows:
+ audio -> vddio-audio
+ audio-hv -> vddio-audio-hv
+ cam ->vddio-cam
+ dbg -> vddio-dbg
+ dmic -> vddio-dmic
+ gpio -> vddio-gpio
+ pex-ctrl -> vddio-pex-ctrl
+ sdmmc1 -> vddio-sdmmc1
+ sdmmc3 -> vddio-sdmmc3
+ spi -> vddio-spi
+ spi-hv -> vddio-spi-hv
+ uart -> vddio-uart
+
+Example:
+ i2c@7000d000 {
+ pmic@3c {
+ regulators {
+ vddio_sdmmc1: ldo2 {
+ /* Regulator entries for LDO2 */
+ };
+
+ vdd_cam: ldo3 {
+ /* Regulator entries for LDO3 */
+ };
+ };
+ };
+ };
+
+ pmc@7000e400 {
+ vddio-cam-supply = <&vdd_cam>;
+ vddio-sdmmc1-supply = <&vddio_sdmmc1>;
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&tegra_io_pad_volt_default>;
+ tegra_io_pad_volt_default: common {
+ audio-hv {
+ pins = "audio-hv";
+ low-power-disable;
+ };
+
+ gpio {
+ pins = "gpio";
+ low-power-disable;
+ };
+
+ audio {
+ pins = "audio", "dmic", "sdmmc3";
+ low-power-enable;
+ };
+ };
+
+ };
--
2.1.4
^ permalink raw reply related
* [PATCH V4 0/2] pinctrl: tegra: Add support for IO pad control
From: Laxman Dewangan @ 2016-11-24 8:38 UTC (permalink / raw)
To: linus.walleij-QSEj5FYQhm4dnm+yROfE0A,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
swarren-3lzwWm7+Weoh9ZMKESR00Q,
thierry.reding-Re5JQEeQqe8AvxtiuMwx3w,
gnurou-Re5JQEeQqe8AvxtiuMwx3w, jonathanh-DDmLM1+adcrQT0dZR+AlfA,
joe-6d6DIl74uiNBDgjK7y7TUQ
Cc: yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A,
linux-gpio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-tegra-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Laxman Dewangan
NVIDIA Tegra124 and later SoCs support the multi-voltage level and
low power state of some of its IO pads. The IO pads can work in
the voltage of the 1.8V and 3.3V of IO power rail sources. When IO
interface are not used then IO pads can be configure in low power
state to reduce the power from that IO pads.
This series add the support of configuration of IO pad via pinctrl
framework. The io pad driver uses the tegra PMC interface.
---
This driver was sent earlier for review along with soc/tegra pmc
changes. During review, decided to first conclude in soc/tegra pmc
patches and then review this.
Thierry applied the pmc patches in the private tree
https://github.com/thierryreding/linux/tree/tegra186
and he wanted to have the patches for user of the new APIs so that
it can be pushed to mainline.
Sending the pinctrl driver. This needs Ack/reviewed from pinctrl subsystem
i.e. Linus Welleij to apply in the Thierry's T186 branch along with
PMC patches.
---
Changes from V1:
- use the regulator framework to get the IO voltage instead of table from
DT. The regulator handle is provided from DT.
Changes from V2:
- Nit fixes and variable/allocation optimisation as per review comment from
V2.
Changes from V3:
Use devm_regulator_get() instead of devm_regulator_get_optional().
Laxman Dewangan (2):
pinctrl: tegra: Add DT binding for io pads control
pinctrl: tegra: Add driver to configure voltage and power of io pads
.../bindings/pinctrl/nvidia,tegra-io-pad.txt | 126 +++++
drivers/pinctrl/tegra/Kconfig | 12 +
drivers/pinctrl/tegra/Makefile | 1 +
drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c | 530 +++++++++++++++++++++
4 files changed, 669 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pinctrl/nvidia,tegra-io-pad.txt
create mode 100644 drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c
--
2.1.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
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