Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 5/6] regulator: anatop-regulator: make regulator-name using optionally
From: Dong Aisheng @ 2017-04-13  7:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170411203817.5f32em4v5xiwzchl@sirena.org.uk>

On Tue, Apr 11, 2017 at 09:38:18PM +0100, Mark Brown wrote:
> On Wed, Apr 12, 2017 at 09:58:46AM +0800, Dong Aisheng wrote:
> > rdesc->name/regulator-name is optional according to standard regulator
> > binding doc. Use it conditionally to avoid a kernel NULL point crash.
> 
> It is optional in the standard binding because it is used to override
> the name statically provided in the driver for the device.  Since the
> anatop regulator is completely dynamic (there's no static list of
> regulators in the device) it's mandatory for anatop regulators - you
> should improve the error handling instead to detect a missing name.

Got it. Will change in v2.
Thanks for the suggestion.

Regards
Dong Aisheng

^ permalink raw reply

* A question about vtimer interrupt when guest_exit
From: Marc Zyngier @ 2017-04-13  7:32 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <B99C36949BCC8541A564076B659DD970671E7965@nkgeml514-mbs.china.huawei.com>

On 13/04/17 07:14, Wangxuefeng (E) wrote:
>  
> 
> Hi Marc,
> 
> I have a question about the vtimer ppi report when guest exit.
> 
>  
> 
> 1.       The flow: vtimer PPI assert->guest_exit->disable the
> vtimer->msr daifclear, #0x2
> 
>  
> 
> 2.       When disable the vtimer, the nCNTVIRQ will be cleared, then the
> clear request will be sent to GICD, GICD send clear request to GICC then
> clear the irq to CPU. So the clear request indeed be sent to CPU need
> some cycles(for example: N cycles),  
> 
> while the cpu execute from ?disable vtimer?  to ?clear the mask of irq?
> need M cycles, how the software make sure the interrupt won?t be taken
> by CPU before it indeed get the clear request?  Or the ARM architecture
> make sure M<N.

The architecture doesn't guarantee anything like that. If your GIC is
too slow to retire the pending interrupt by the time we unmask
interrupts at the CPU level, you'll get a (harmless) warning that we've
taken a spurious timer interrupt. This isn't a big deal, just the
indication that the HW is a bit imbalanced (fast CPU, slow GIC).

We may revise the way we handle the timer interrupt@some point,
taking the interrupt and keeping it active while injected into the guest.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply

* [PATCH 0/3] Fix mdp device tree
From: Minghsiu Tsai @ 2017-04-13  7:33 UTC (permalink / raw)
  To: linux-arm-kernel

Fix this by moving the mdp component nodes up a level such that they are
siblings of mdp and all other SoC subsystems.  This also simplifies the
device tree.

Daniel Kurtz (2):
  arm64: dts: mt8173: Fix mdp device tree
  media: mtk-mdp: Fix mdp device tree

Minghsiu Tsai (1):
  dt-bindings: mt8173: Fix mdp device tree

 .../devicetree/bindings/media/mediatek-mdp.txt     |  12 +-
 arch/arm64/boot/dts/mediatek/mt8173.dtsi           | 126 ++++++++++-----------
 drivers/media/platform/mtk-mdp/mtk_mdp_core.c      |   2 +-
 3 files changed, 64 insertions(+), 76 deletions(-)

-- 
1.9.1

^ permalink raw reply

* [PATCH 1/3] dt-bindings: mt8173: Fix mdp device tree
From: Minghsiu Tsai @ 2017-04-13  7:33 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1492068787-17838-1-git-send-email-minghsiu.tsai@mediatek.com>

If the mdp_* nodes are under an mdp sub-node, their corresponding
platform device does not automatically get its iommu assigned properly.

Fix this by moving the mdp component nodes up a level such that they are
siblings of mdp and all other SoC subsystems.  This also simplifies the
device tree.

Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
Signed-off-by: Minghsiu Tsai <minghsiu.tsai@mediatek.com>

---
 Documentation/devicetree/bindings/media/mediatek-mdp.txt | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/mediatek-mdp.txt b/Documentation/devicetree/bindings/media/mediatek-mdp.txt
index 4182063..0d03e3a 100644
--- a/Documentation/devicetree/bindings/media/mediatek-mdp.txt
+++ b/Documentation/devicetree/bindings/media/mediatek-mdp.txt
@@ -2,7 +2,7 @@
 
 Media Data Path is used for scaling and color space conversion.
 
-Required properties (controller (parent) node):
+Required properties (controller node):
 - compatible: "mediatek,mt8173-mdp"
 - mediatek,vpu: the node of video processor unit, see
   Documentation/devicetree/bindings/media/mediatek-vpu.txt for details.
@@ -32,21 +32,16 @@ Required properties (DMA function blocks, child node):
   for details.
 
 Example:
-mdp {
-	compatible = "mediatek,mt8173-mdp";
-	#address-cells = <2>;
-	#size-cells = <2>;
-	ranges;
-	mediatek,vpu = <&vpu>;
-
 	mdp_rdma0: rdma at 14001000 {
 		compatible = "mediatek,mt8173-mdp-rdma";
+			     "mediatek,mt8173-mdp";
 		reg = <0 0x14001000 0 0x1000>;
 		clocks = <&mmsys CLK_MM_MDP_RDMA0>,
 			 <&mmsys CLK_MM_MUTEX_32K>;
 		power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
 		iommus = <&iommu M4U_PORT_MDP_RDMA0>;
 		mediatek,larb = <&larb0>;
+		mediatek,vpu = <&vpu>;
 	};
 
 	mdp_rdma1: rdma at 14002000 {
@@ -106,4 +101,3 @@ mdp {
 		iommus = <&iommu M4U_PORT_MDP_WROT1>;
 		mediatek,larb = <&larb4>;
 	};
-};
-- 
1.9.1

^ permalink raw reply related

* [PATCH 2/3] arm64: dts: mt8173: Fix mdp device tree
From: Minghsiu Tsai @ 2017-04-13  7:33 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1492068787-17838-1-git-send-email-minghsiu.tsai@mediatek.com>

From: Daniel Kurtz <djkurtz@chromium.org>

If the mdp_* nodes are under an mdp sub-node, their corresponding
platform device does not automatically get its iommu assigned properly.

Fix this by moving the mdp component nodes up a level such that they are
siblings of mdp and all other SoC subsystems.  This also simplifies the
device tree.

Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
Signed-off-by: Minghsiu Tsai <minghsiu.tsai@mediatek.com>

---
 arch/arm64/boot/dts/mediatek/mt8173.dtsi | 126 +++++++++++++++----------------
 1 file changed, 60 insertions(+), 66 deletions(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
index 6922252..d28a363 100644
--- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
@@ -792,80 +792,74 @@
 			#clock-cells = <1>;
 		};
 
-		mdp {
-			compatible = "mediatek,mt8173-mdp";
-			#address-cells = <2>;
-			#size-cells = <2>;
-			ranges;
+		mdp_rdma0: rdma at 14001000 {
+			compatible = "mediatek,mt8173-mdp-rdma",
+				     "mediatek,mt8173-mdp";
+			reg = <0 0x14001000 0 0x1000>;
+			clocks = <&mmsys CLK_MM_MDP_RDMA0>,
+				 <&mmsys CLK_MM_MUTEX_32K>;
+			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+			iommus = <&iommu M4U_PORT_MDP_RDMA0>;
+			mediatek,larb = <&larb0>;
 			mediatek,vpu = <&vpu>;
+		};
 
-			mdp_rdma0: rdma at 14001000 {
-				compatible = "mediatek,mt8173-mdp-rdma";
-				reg = <0 0x14001000 0 0x1000>;
-				clocks = <&mmsys CLK_MM_MDP_RDMA0>,
-					 <&mmsys CLK_MM_MUTEX_32K>;
-				power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
-				iommus = <&iommu M4U_PORT_MDP_RDMA0>;
-				mediatek,larb = <&larb0>;
-			};
-
-			mdp_rdma1: rdma at 14002000 {
-				compatible = "mediatek,mt8173-mdp-rdma";
-				reg = <0 0x14002000 0 0x1000>;
-				clocks = <&mmsys CLK_MM_MDP_RDMA1>,
-					 <&mmsys CLK_MM_MUTEX_32K>;
-				power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
-				iommus = <&iommu M4U_PORT_MDP_RDMA1>;
-				mediatek,larb = <&larb4>;
-			};
+		mdp_rdma1: rdma at 14002000 {
+			compatible = "mediatek,mt8173-mdp-rdma";
+			reg = <0 0x14002000 0 0x1000>;
+			clocks = <&mmsys CLK_MM_MDP_RDMA1>,
+				 <&mmsys CLK_MM_MUTEX_32K>;
+			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+			iommus = <&iommu M4U_PORT_MDP_RDMA1>;
+			mediatek,larb = <&larb4>;
+		};
 
-			mdp_rsz0: rsz at 14003000 {
-				compatible = "mediatek,mt8173-mdp-rsz";
-				reg = <0 0x14003000 0 0x1000>;
-				clocks = <&mmsys CLK_MM_MDP_RSZ0>;
-				power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
-			};
+		mdp_rsz0: rsz at 14003000 {
+			compatible = "mediatek,mt8173-mdp-rsz";
+			reg = <0 0x14003000 0 0x1000>;
+			clocks = <&mmsys CLK_MM_MDP_RSZ0>;
+			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+		};
 
-			mdp_rsz1: rsz at 14004000 {
-				compatible = "mediatek,mt8173-mdp-rsz";
-				reg = <0 0x14004000 0 0x1000>;
-				clocks = <&mmsys CLK_MM_MDP_RSZ1>;
-				power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
-			};
+		mdp_rsz1: rsz at 14004000 {
+			compatible = "mediatek,mt8173-mdp-rsz";
+			reg = <0 0x14004000 0 0x1000>;
+			clocks = <&mmsys CLK_MM_MDP_RSZ1>;
+			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+		};
 
-			mdp_rsz2: rsz at 14005000 {
-				compatible = "mediatek,mt8173-mdp-rsz";
-				reg = <0 0x14005000 0 0x1000>;
-				clocks = <&mmsys CLK_MM_MDP_RSZ2>;
-				power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
-			};
+		mdp_rsz2: rsz at 14005000 {
+			compatible = "mediatek,mt8173-mdp-rsz";
+			reg = <0 0x14005000 0 0x1000>;
+			clocks = <&mmsys CLK_MM_MDP_RSZ2>;
+			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+		};
 
-			mdp_wdma0: wdma at 14006000 {
-				compatible = "mediatek,mt8173-mdp-wdma";
-				reg = <0 0x14006000 0 0x1000>;
-				clocks = <&mmsys CLK_MM_MDP_WDMA>;
-				power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
-				iommus = <&iommu M4U_PORT_MDP_WDMA>;
-				mediatek,larb = <&larb0>;
-			};
+		mdp_wdma0: wdma at 14006000 {
+			compatible = "mediatek,mt8173-mdp-wdma";
+			reg = <0 0x14006000 0 0x1000>;
+			clocks = <&mmsys CLK_MM_MDP_WDMA>;
+			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+			iommus = <&iommu M4U_PORT_MDP_WDMA>;
+			mediatek,larb = <&larb0>;
+		};
 
-			mdp_wrot0: wrot at 14007000 {
-				compatible = "mediatek,mt8173-mdp-wrot";
-				reg = <0 0x14007000 0 0x1000>;
-				clocks = <&mmsys CLK_MM_MDP_WROT0>;
-				power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
-				iommus = <&iommu M4U_PORT_MDP_WROT0>;
-				mediatek,larb = <&larb0>;
-			};
+		mdp_wrot0: wrot at 14007000 {
+			compatible = "mediatek,mt8173-mdp-wrot";
+			reg = <0 0x14007000 0 0x1000>;
+			clocks = <&mmsys CLK_MM_MDP_WROT0>;
+			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+			iommus = <&iommu M4U_PORT_MDP_WROT0>;
+			mediatek,larb = <&larb0>;
+		};
 
-			mdp_wrot1: wrot at 14008000 {
-				compatible = "mediatek,mt8173-mdp-wrot";
-				reg = <0 0x14008000 0 0x1000>;
-				clocks = <&mmsys CLK_MM_MDP_WROT1>;
-				power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
-				iommus = <&iommu M4U_PORT_MDP_WROT1>;
-				mediatek,larb = <&larb4>;
-			};
+		mdp_wrot1: wrot at 14008000 {
+			compatible = "mediatek,mt8173-mdp-wrot";
+			reg = <0 0x14008000 0 0x1000>;
+			clocks = <&mmsys CLK_MM_MDP_WROT1>;
+			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+			iommus = <&iommu M4U_PORT_MDP_WROT1>;
+			mediatek,larb = <&larb4>;
 		};
 
 		ovl0: ovl at 1400c000 {
-- 
1.9.1

^ permalink raw reply related

* [PATCH 3/3] media: mtk-mdp: Fix mdp device tree
From: Minghsiu Tsai @ 2017-04-13  7:33 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1492068787-17838-1-git-send-email-minghsiu.tsai@mediatek.com>

From: Daniel Kurtz <djkurtz@chromium.org>

If the mdp_* nodes are under an mdp sub-node, their corresponding
platform device does not automatically get its iommu assigned properly.

Fix this by moving the mdp component nodes up a level such that they are
siblings of mdp and all other SoC subsystems.  This also simplifies the
device tree.

Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
Signed-off-by: Minghsiu Tsai <minghsiu.tsai@mediatek.com>

---
 drivers/media/platform/mtk-mdp/mtk_mdp_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_core.c b/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
index 9e4eb7d..a5ad586 100644
--- a/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
+++ b/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
@@ -118,7 +118,7 @@ static int mtk_mdp_probe(struct platform_device *pdev)
 	mutex_init(&mdp->vpulock);
 
 	/* Iterate over sibling MDP function blocks */
-	for_each_child_of_node(dev->of_node, node) {
+	for_each_child_of_node(dev->of_node->parent, node) {
 		const struct of_device_id *of_id;
 		enum mtk_mdp_comp_type comp_type;
 		int comp_id;
-- 
1.9.1

^ permalink raw reply related

* [PATCH 1/3] clk: sunxi-ng: Add clk notifier to gate then ungate PLL clocks
From: Chen-Yu Tsai @ 2017-04-13  7:35 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170413070209.kjpv27fqc4o4znqs@lukather>

On Thu, Apr 13, 2017 at 3:02 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Hi Chen-Yu,
>
> On Thu, Apr 13, 2017 at 10:13:52AM +0800, Chen-Yu Tsai wrote:
>> In common PLL designs, changes to the dividers take effect almost
>> immediately, while changes to the multipliers (implemented as
>> dividers in the feedback loop) take a few cycles to work into
>> the feedback loop for the PLL to stablize.
>>
>> Sometimes when the PLL clock rate is changed, the decrease in the
>> divider is too much for the decrease in the multiplier to catch up.
>> The PLL clock rate will spike, and in some cases, might lock up
>> completely. This is especially the case if the divider changed is
>> the pre-divider, which affects the reference frequency.
>>
>> This patch introduces a clk notifier callback that will gate and
>> then ungate a clk after a rate change, effectively resetting it,
>> so it continues to work, despite any possible lockups. Care must
>> be taken to reparent any consumers to other temporary clocks during
>> the rate change, and that this notifier callback must be the first
>> to be registered.
>>
>> This is intended to fix occasional lockups with cpufreq on newer
>> Allwinner SoCs, such as the A33 and the H3. Previously it was
>> thought that reparenting the cpu clock away from the PLL while
>> it stabilized was enough, as this worked quite well on the A31.
>>
>> On the A33, hangs have been observed after cpufreq was recently
>> introduced. With the H3, a more thorough test [1] showed that
>> reparenting alone isn't enough. The system still locks up unless
>> the dividers are limited to 1.
>>
>> A hunch was if the PLL was stuck in some unknown state, perhaps
>> gating then ungating it would bring it back to normal. Tests
>> done by Icenowy Zheng using Ondrej's test firmware shows this
>> to be a valid solution.
>>
>> [1] http://www.spinics.net/lists/arm-kernel/msg552501.html
>>
>> Reported-by: Ondrej Jirman <megous@megous.com>
>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>> Tested-by: Icenowy Zheng <icenowy@aosc.io>
>> Tested-by: Quentin Schulz <quentin.schulz@free-electrons.com>
>
> Thanks for looking into this, and coming up with a clean solution, and
> a great commit log.
>
> However, I wondering, isn't that notifier just a re-implementation of
> CLK_SET_RATE_GATE?

They are not the same. AFAIK, CLK_SET_RATE_GATE tells the clk framework
that this clk's rate cannot be changed if it is enabled (which means
some one is using it). However the clk framework does nothing to
actually handle it. It just returns an error. Any consumers are
responsible for gating the clock before making changes. This is a nice
thing to have, as it can prevent unintended changes to dot clocks or
audio clocks used with active output streams. We could consider setting
this for the audio and video PLLs.

Here we are dealing with the CPU PLL, which, for practical reasons,
is always enabled as far as the clk framework is concerned. The
reason being the OPPs are never low enough for the CPU clock to
use any other parent. To have it disabled, we would have to kick
consumers (the CPU clock in this case) to use other clocks, so it's
safe, remember which ones we kicked, and then bring them back once
everything is done.

AFAIK, we, samsung, rockchip, meson, do the temporary reparenting
using clk_notifiers to access the mux registers directly. As far
as the clk framework is concerned, nothing has changed.

I'm not saying it's not possible to support this in the core, but
the core already has to do a lot of bookkeeping and recalculation
when anything changes. Adding something transient into the process
isn't helping. And the reparenting might temporarily violate any
downstream requirements.

For now, I think clk notifiers is the easier solution for these
one off requirements that are pretty much contained in a small
part of the system.

Regards
ChenYu

^ permalink raw reply

* [PATCH 1/2] drivers: pci: do not disregard parent resources starting at 0x0
From: Ard Biesheuvel @ 2017-04-13  7:39 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170412132423.GA6584@red-moon>

On 12 April 2017 at 14:24, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote:
> [+Yinghai, Bjorn]
>
> On Tue, Apr 11, 2017 at 05:33:12PM +0100, Ard Biesheuvel wrote:
>> Commit f44116ae8818 ("PCI: Remove pci_find_parent_resource() use for
>> allocation") updated the logic that iterates over all bus resources
>> and compares them to a given resource, in order to decide whether one
>> is the parent of the latter.
>>
>> This change inadvertently causes pci_find_parent_resource() to disregard
>> resources starting at address 0x0, resulting in an error such as the one
>> below on ARM systems whose I/O window starts at 0x0.
>>
>> pci_bus 0000:00: root bus resource [mem 0x10000000-0x3efeffff window]
>> pci_bus 0000:00: root bus resource [io  0x0000-0xffff window]
>> pci_bus 0000:00: root bus resource [mem 0x8000000000-0xffffffffff window]
>> pci_bus 0000:00: root bus resource [bus 00-0f]
>> pci 0000:00:01.0: PCI bridge to [bus 01]
>> pci 0000:00:02.0: PCI bridge to [bus 02]
>> pci 0000:00:03.0: PCI bridge to [bus 03]
>> pci 0000:00:03.0: can't claim BAR 13 [io  0x0000-0x0fff]: no compatible bridge window
>> pci 0000:03:01.0: can't claim BAR 0 [io  0x0000-0x001f]: no compatible bridge window
>>
>> While this never happens on x86, it is perfectly legal in general for a
>> PCI MMIO or IO window to start at address 0x0, and it was supported in
>> the code before commit f44116ae8818.
>>
>> So let's drop the test for res->start != 0; resource_contains() already
>> checks whether [start, end) completely covers the resource, and so it
>> should be redundant.
>>
>> Fixes: f44116ae8818 ("PCI: Remove pci_find_parent_resource() use for allocation")
>
> I know this code fixes IO claiming on ARM/ARM64 (well, it fixes nothing
> because we never claim resources on ARM/ARM64 apart from kvmtool and
> generic host bridge), my _big_ worry is that it can cause endless
> regressions on other arches, in any case I would be really really
> careful about adding a Fixes: tag to it.
>

The patch is only 3 years old, and is obviously a regression given
that the change in behavior described here occurs as a side effect.
But given that my involvement with PCI is much more recent than that,
it is very well possible that I am missing something here.

-- 
Ard.

^ permalink raw reply

* [PATCH 6/6] regulator: anatop: set default voltage selector for pcie
From: Dong Aisheng @ 2017-04-13  7:41 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170411204003.avoe5a6o52kx5on2@sirena.org.uk>

On Tue, Apr 11, 2017 at 09:40:03PM +0100, Mark Brown wrote:
> On Wed, Apr 12, 2017 at 09:58:47AM +0800, Dong Aisheng wrote:
> > Set the initial voltage selector for vddpcie in case it's disabled
> > by default.
> 
> Why is this the only anatop regulator which can have this problem and
> how do we know this is a good value?

Anatop regulator has no separate gate bit.
e.g.
00000 Power gated off
00001 Target core voltage = 0.725V
...
So it may have no valid default voltage in case it's disabled in
bootloader.
e.g. regulator_enable() may not work.

The default voltage 1.100v this patch sets is defined in reference
manual.

Regards
Dong Aisheng

^ permalink raw reply

* [PATCH v5 1/4] gpio: mvebu: Add limited PWM support
From: Ralph Sennhauser @ 2017-04-13  7:45 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170412171121.GB11964@ulmo.ba.sec>

On Wed, 12 Apr 2017 19:11:21 +0200
Thierry Reding <thierry.reding@gmail.com> wrote:

> On Sun, Apr 09, 2017 at 08:09:27PM +0200, Ralph Sennhauser wrote:
> > From: Andrew Lunn <andrew@lunn.ch>
> > 
> > Armada 370/XP devices can 'blink' GPIO lines with a configurable on
> > and off period. This can be modelled as a PWM.
> > 
> > However, there are only two sets of PWM configuration registers for
> > all the GPIO lines. This driver simply allows a single GPIO line per
> > GPIO chip of 32 lines to be used as a PWM. Attempts to use more
> > return EBUSY.
> > 
> > Due to the interleaving of registers it is not simple to separate
> > the PWM driver from the GPIO driver. Thus the GPIO driver has been
> > extended with a PWM driver.
> > 
> > Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> > URL: https://patchwork.ozlabs.org/patch/427287/
> > URL: https://patchwork.ozlabs.org/patch/427295/
> > [Ralph Sennhauser:
> >   * Port forward
> >   * Merge PWM portion into gpio-mvebu.c
> >   * Switch to atomic PWM API
> >   * Add new compatible string marvell,armada-370-xp-gpio
> >   * Update and merge documentation patch
> >   * Update MAINTAINERS]
> > Signed-off-by: Ralph Sennhauser <ralph.sennhauser@gmail.com>
> > Tested-by: Andrew Lunn <andrew@lunn.ch>
> > ---
> >  .../devicetree/bindings/gpio/gpio-mvebu.txt        |  32 ++
> >  MAINTAINERS                                        |   2 +
> >  drivers/gpio/gpio-mvebu.c                          | 324
> > ++++++++++++++++++++- 3 files changed, 346 insertions(+), 12
> > deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/gpio/gpio-mvebu.txt
> > b/Documentation/devicetree/bindings/gpio/gpio-mvebu.txt index
> > a6f3bec..fe49e9d 100644 ---
> > a/Documentation/devicetree/bindings/gpio/gpio-mvebu.txt +++
> > b/Documentation/devicetree/bindings/gpio/gpio-mvebu.txt @@ -38,6
> > +38,24 @@ Required properties:
> >  - #gpio-cells: Should be two. The first cell is the pin number. The
> >    second cell is reserved for flags, unused at the moment.
> >  
> > +Optional properties:
> > +
> > +In order to use the gpio lines in PWM mode, some additional
> > optional +properties are required. Only Armada 370 and XP support
> > these properties. +
> > +- compatible: Must contain "marvell,armada-370-xp-gpio"
> > +
> > +- reg: an additional register set is needed, for the GPIO Blink
> > +  Counter on/off registers.
> > +
> > +- reg-names: Must contain an entry "pwm" corresponding to the
> > +  additional register range needed for pwm operation.
> > +
> > +- #pwm-cells: Should be two. The first cell is the GPIO line
> > number. The
> > +  second cell is the period in nanoseconds.
> > +
> > +- clocks: Must be a phandle to the clock for the gpio controller.
> > +
> >  Example:
> >  
> >  		gpio0: gpio at d0018100 {
> > @@ -51,3 +69,17 @@ Example:
> >  			#interrupt-cells = <2>;
> >  			interrupts = <16>, <17>, <18>, <19>;
> >  		};
> > +
> > +		gpio1: gpio at 18140 {
> > +			compatible = "marvell,armada-370-xp-gpio";
> > +			reg = <0x18140 0x40>, <0x181c8 0x08>;
> > +			reg-names = "gpio", "pwm";
> > +			ngpios = <17>;
> > +			gpio-controller;
> > +			#gpio-cells = <2>;
> > +			#pwm-cells = <2>;
> > +			interrupt-controller;
> > +			#interrupt-cells = <2>;
> > +			interrupts = <87>, <88>, <89>;
> > +			clocks = <&coreclk 0>;
> > +		};  
> 
> This is going to need an Acked-by from one of the device tree
> maintainers. Rob and devicetree at vger.kernel.org are on Cc, but I
> suspect nobody might look for the binding change "hidden" in this
> patch.
> 
> Maybe best to split this off into a separate patch, or explicitly ping
> Rob to look at this patch.

Hi Thierry,

Rob asked for the new compatible string so he did see it presumably. As
you prefer to have an ACK by him I'll see to getting one for the driver
(bindings part). The patch could be split but then one might want to
split it even further. Like this the first patch in the series is a nice
self contained package.

>
<snip/>
> 
> > +
> > +	mvpwm->clk_rate = clk_get_rate(mvchip->clk);
> > +	if (!mvpwm->clk_rate) {
> > +		dev_err(dev, "failed to get clock rate\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	mvpwm->chip.dev = dev;
> > +	mvpwm->chip.ops = &mvebu_pwm_ops;
> > +	mvpwm->chip.base = mvchip->chip.base;
> > +	mvpwm->chip.npwm = mvchip->chip.ngpio;  
> 
> I still would've done this differently. If you use this with a PWM
> user you have to hook it up via DT anyway, so it doesn't matter
> whether you specify the PWM index or the GPIO via some other
> property. The _only_ use-case where this might actually be an
> advantage is if you request a PWM via the sysfs interface.

Let me answer this in the other mail where you bring this up.

> 
<snip/>
> All of my comments are effectively of a bikeshed nature, so from a PWM
> perspective this is:
> 
> Acked-by: Thierry Reding <thierry.reding@gmail.com>

Thanks for the detailed review and the ACK. Will work on all the
mentioned bits for v6.

Ralph

^ permalink raw reply

* [PATCH v2] efi: Config options to assign versions in the PE-COFF header
From: Gary Lin @ 2017-04-13  7:47 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAKv+Gu8ZS1rdDwLrspv_VnYHOfxkdxUpZe_h1Sw3-N2SBpeJ5g@mail.gmail.com>

On Thu, Apr 13, 2017 at 08:26:04AM +0100, Ard Biesheuvel wrote:
> On 13 April 2017 at 04:58, Gary Lin <glin@suse.com> wrote:
> > This commit adds the new config options to allow the user to modify the
> > following fields in the PE-COFF header.
> >
> > UINT16 MajorOperatingSystemVersion
> > UINT16 MinorOperatingSystemVersion
> > UINT16 MajorImageVersion
> > UINT16 MinorImageVersion
> >
> > Those fields are mainly for the executables or libraries in Windows NT
> > or higher to specify the minimum supported Windows version and the
> > version of the image itself.
> >
> > Given the fact that those fields are ignored in UEFI, we can safely reuse
> > those fields for other purposes, e.g. Security Version(*).
> >
> > (*) https://github.com/lcp/shim/wiki/Security-Version
> >
> > v2 changes:
> > - Modify the header direct instead of using an external script as
> >   suggested by Ard Biesheuvel
> > - Include arm and arm64
> >
> 
> Thanks for the update. Could we put the Kconfig changes in
> drivers/firmware/efi/Kconfig, rather than duplicating them 3 times?

Sure. Will send a v3 to update Kconfig.

Thanks,

Gary Lin

> 
> > Cc: Russell King <linux@armlinux.org.uk>
> > Cc: Matt Fleming <matt@codeblueprint.co.uk>
> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Will Deacon <will.deacon@arm.com>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Ingo Molnar <mingo@redhat.com>
> > Cc: "H. Peter Anvin" <hpa@zytor.com>
> > Cc: Joey Lee <jlee@suse.com>
> > Cc: Vojtech Pavlik <vojtech@suse.cz>
> > Signed-off-by: Gary Lin <glin@suse.com>
> > ---
> >  arch/arm/Kconfig                      | 24 ++++++++++++++++++++++++
> >  arch/arm/boot/compressed/efi-header.S |  8 ++++----
> >  arch/arm64/Kconfig                    | 24 ++++++++++++++++++++++++
> >  arch/arm64/kernel/head.S              |  8 ++++----
> >  arch/x86/Kconfig                      | 24 ++++++++++++++++++++++++
> >  arch/x86/boot/header.S                |  8 ++++----
> >  6 files changed, 84 insertions(+), 12 deletions(-)
> >
> > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > index 0d4e71b42c77..4965ad2ccc23 100644
> > --- a/arch/arm/Kconfig
> > +++ b/arch/arm/Kconfig
> > @@ -2090,6 +2090,30 @@ config EFI
> >           is only useful for kernels that may run on systems that have
> >           UEFI firmware.
> >
> > +config EFI_MAJOR_OS
> > +       hex "EFI Major OS Version"
> > +       range 0x0 0xFFFF
> > +       default "0x0"
> > +       depends on EFI_STUB
> > +
> > +config EFI_MINOR_OS
> > +       hex "EFI Minor OS Version"
> > +       range 0x0 0xFFFF
> > +       default "0x0"
> > +       depends on EFI_STUB
> > +
> > +config EFI_MAJOR_IMAGE
> > +       hex "EFI Major Image Version"
> > +       range 0x0 0xFFFF
> > +       default "0x0"
> > +       depends on EFI_STUB
> > +
> > +config EFI_MINOR_IMAGE
> > +       hex "EFI Minor Image Version"
> > +       range 0x0 0xFFFF
> > +       default "0x0"
> > +       depends on EFI_STUB
> > +
> >  endmenu
> >
> >  menu "CPU Power Management"
> > diff --git a/arch/arm/boot/compressed/efi-header.S b/arch/arm/boot/compressed/efi-header.S
> > index 9d5dc4fda3c1..67715472a76f 100644
> > --- a/arch/arm/boot/compressed/efi-header.S
> > +++ b/arch/arm/boot/compressed/efi-header.S
> > @@ -69,10 +69,10 @@ extra_header_fields:
> >                 .long   0                       @ ImageBase
> >                 .long   0x200                   @ SectionAlignment
> >                 .long   0x200                   @ FileAlignment
> > -               .short  0                       @ MajorOperatingSystemVersion
> > -               .short  0                       @ MinorOperatingSystemVersion
> > -               .short  0                       @ MajorImageVersion
> > -               .short  0                       @ MinorImageVersion
> > +               .short  CONFIG_EFI_MAJOR_OS     @ MajorOperatingSystemVersion
> > +               .short  CONFIG_EFI_MINOR_OS     @ MinorOperatingSystemVersion
> > +               .short  CONFIG_EFI_MAJOR_IMAGE  @ MajorImageVersion
> > +               .short  CONFIG_EFI_MINOR_IMAGE  @ MinorImageVersion
> >                 .short  0                       @ MajorSubsystemVersion
> >                 .short  0                       @ MinorSubsystemVersion
> >                 .long   0                       @ Win32VersionValue
> > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> > index 3741859765cf..c782c422e58c 100644
> > --- a/arch/arm64/Kconfig
> > +++ b/arch/arm64/Kconfig
> > @@ -1033,6 +1033,30 @@ config EFI
> >           allow the kernel to be booted as an EFI application. This
> >           is only useful on systems that have UEFI firmware.
> >
> > +config EFI_MAJOR_OS
> > +       hex "EFI Major OS Version"
> > +       range 0x0 0xFFFF
> > +       default "0x0"
> > +       depends on EFI_STUB
> > +
> > +config EFI_MINOR_OS
> > +       hex "EFI Minor OS Version"
> > +       range 0x0 0xFFFF
> > +       default "0x0"
> > +       depends on EFI_STUB
> > +
> > +config EFI_MAJOR_IMAGE
> > +       hex "EFI Major Image Version"
> > +       range 0x0 0xFFFF
> > +       default "0x0"
> > +       depends on EFI_STUB
> > +
> > +config EFI_MINOR_IMAGE
> > +       hex "EFI Minor Image Version"
> > +       range 0x0 0xFFFF
> > +       default "0x0"
> > +       depends on EFI_STUB
> > +
> >  config DMI
> >         bool "Enable support for SMBIOS (DMI) tables"
> >         depends on EFI
> > diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> > index 4fb6ccd886d1..9faa4b04d0ef 100644
> > --- a/arch/arm64/kernel/head.S
> > +++ b/arch/arm64/kernel/head.S
> > @@ -129,10 +129,10 @@ extra_header_fields:
> >         .quad   0                               // ImageBase
> >         .long   0x1000                          // SectionAlignment
> >         .long   PECOFF_FILE_ALIGNMENT           // FileAlignment
> > -       .short  0                               // MajorOperatingSystemVersion
> > -       .short  0                               // MinorOperatingSystemVersion
> > -       .short  0                               // MajorImageVersion
> > -       .short  0                               // MinorImageVersion
> > +       .short  CONFIG_EFI_MAJOR_OS             // MajorOperatingSystemVersion
> > +       .short  CONFIG_EFI_MINOR_OS             // MinorOperatingSystemVersion
> > +       .short  CONFIG_EFI_MAJOR_IMAGE          // MajorImageVersion
> > +       .short  CONFIG_EFI_MINOR_IMAGE          // MinorImageVersion
> >         .short  0                               // MajorSubsystemVersion
> >         .short  0                               // MinorSubsystemVersion
> >         .long   0                               // Win32VersionValue
> > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > index 5bbdef151805..233933fde7dd 100644
> > --- a/arch/x86/Kconfig
> > +++ b/arch/x86/Kconfig
> > @@ -1803,6 +1803,30 @@ config EFI_STUB
> >
> >           See Documentation/efi-stub.txt for more information.
> >
> > +config EFI_MAJOR_OS
> > +       hex "EFI Major OS Version"
> > +       range 0x0 0xFFFF
> > +       default "0x0"
> > +       depends on EFI_STUB
> > +
> > +config EFI_MINOR_OS
> > +       hex "EFI Minor OS Version"
> > +       range 0x0 0xFFFF
> > +       default "0x0"
> > +       depends on EFI_STUB
> > +
> > +config EFI_MAJOR_IMAGE
> > +       hex "EFI Major Image Version"
> > +       range 0x0 0xFFFF
> > +       default "0x0"
> > +       depends on EFI_STUB
> > +
> > +config EFI_MINOR_IMAGE
> > +       hex "EFI Minor Image Version"
> > +       range 0x0 0xFFFF
> > +       default "0x0"
> > +       depends on EFI_STUB
> > +
> >  config EFI_MIXED
> >         bool "EFI mixed-mode support"
> >         depends on EFI_STUB && X86_64
> > diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S
> > index 3dd5be33aaa7..863813007207 100644
> > --- a/arch/x86/boot/header.S
> > +++ b/arch/x86/boot/header.S
> > @@ -156,10 +156,10 @@ extra_header_fields:
> >  #endif
> >         .long   0x20                            # SectionAlignment
> >         .long   0x20                            # FileAlignment
> > -       .word   0                               # MajorOperatingSystemVersion
> > -       .word   0                               # MinorOperatingSystemVersion
> > -       .word   0                               # MajorImageVersion
> > -       .word   0                               # MinorImageVersion
> > +       .word   CONFIG_EFI_MAJOR_OS             # MajorOperatingSystemVersion
> > +       .word   CONFIG_EFI_MINOR_OS             # MinorOperatingSystemVersion
> > +       .word   CONFIG_EFI_MAJOR_IMAGE          # MajorImageVersion
> > +       .word   CONFIG_EFI_MINOR_IMAGE          # MinorImageVersion
> >         .word   0                               # MajorSubsystemVersion
> >         .word   0                               # MinorSubsystemVersion
> >         .long   0                               # Win32VersionValue
> > --
> > 2.12.0
> >
> 

^ permalink raw reply

* [PATCH v5 0/4] gpio: mvebu: Add PWM fan support
From: Ralph Sennhauser @ 2017-04-13  7:49 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170412171656.GC11964@ulmo.ba.sec>

On Wed, 12 Apr 2017 19:16:56 +0200
Thierry Reding <thierry.reding@gmail.com> wrote:

> On Sun, Apr 09, 2017 at 08:09:26PM +0200, Ralph Sennhauser wrote:
> > Hi Therry,
> > 
> > Resending this as v5 with some minor changes since v4. What is
> > missing is an ACK from you so Linus can merge the driver and
> > Gregory the dts changes. For this driver to make it into 4.12 it
> > would be nice to have it in next soon. I hope you can make some
> > room in your schedule to have another look at this series.
> > 
> > Thanks
> > Ralph
> > 
> > ---
> > 
> > Notes:
> > 
> >   About npwm = 1:
> >     The only way I can think of to achieve that requires reading the
> >     GPIO line from the device tree. This would prevent a user to
> >     dynamically choose a line. Which is fine for the fan found on
> > Mamba but let's take some development board with freely accessible
> > GPIOs and suddenly we limit the use of this driver. Given the
> > above, npwm = ngpio with only one usable at a time is a more
> > accurate  
> 
> I think "accurate" is perhaps not the word I'd choose. "npwm" is
> defined as "number of PWMs controlled by this chip", and that's
> effectively just the one. It's implied that all PWMs exposed by a
> chip can be used concurrently.

I'm not native English so some terms might be off a tad in how I use
them, replace accurate with what you think I meant ;).

The "it's implied" sounds like a contract and EBUSY a violation thereof.

> 
> Anyway, I can see how npwm = ngpio might be more convenient, and if
> that is what you want to do, I don't feel strongly enough to object.

The goal is a pwm-fan driver for Mamba. So npwm=1 would work just fine
from that stand point. It's indeed the sysfs case I had in mind which
would be hampered. Whether to consider that one valid / desirable I
don't want to judge. For me it's a hypothetical use case for others it
might be real.

For this series I want just this one device to work and getting the
bindings right to not prevent others extending the driver later when
they need.

The driver which is under rework here is in use for 2 years, so I went
with the feature set provided by it which is more than I need. It's a
one of it's kind driver and I only have this single use case, so really
your call.

Thanks
Ralph

^ permalink raw reply

* [PATCH 1/2] dt-bindings: Document STM32 I2S bindings
From: Olivier MOYSAN @ 2017-04-13  8:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170412153831.r2urwvfseipfsrsi@sirena.org.uk>

Hello Mark,

On 04/12/2017 05:38 PM, Mark Brown wrote:
> On Wed, Apr 12, 2017 at 12:58:16PM +0000, Olivier MOYSAN wrote:
>> On 04/12/2017 01:32 PM, Mark Brown wrote:
>
>>> It is totally normal to just not use one direction in a given system, we
>>> don't normally need to do anything special to handle things.  I'm a bit
>>> confused as to what's different here and needs configuring?
>
>> The IP does not provide an audio channel configured either as rx or tx.
>> I agree, that in such case, the cpu driver does not generally need
>> to worry about direction and there is nothing special required to handle it.
>
> No, that's not what I'm saying - such hardware would be extremely
> unusual.
>
>> Here the IP provides 2 channels, 1 tx and 1 rx,  which may be active or
>> not. The driver has to know which channel is used, and if both channels
>> have to be managed. The affected registers depend on selected channel.
>
> This sounds like essentially every audio controller out there.  The
> overwhelming majority of controllers do exactly as you describe and have
> both directions in the same IP, this really doesn't seem at all unusual.
> Off the top of my head I can only think of one SoC family which combines
> multiple IPs to do bidirectional audio (though I didn't check).
>
> It really feels like there is something different here and I'm just
> missing it.
>

Yes, I think I need to give more details on design choices.

The IP provides 2 channels.
I can see mainly 3 solutions to link dais to these channels:

1) 2 static dais NOT exclusive
	- dai tx
	- dai rx
The IP exhibits a mode register, where you select mode TX, RX or FD.
There are 2 two options to manage this register.
option 1:
	start first channel with mode RX or TX
	when second channel is started, mode has to be changed to FD.
	Transfers have to be stopped before changing configuration
	registers, so this leads to cuts in audio stream.
option 2:
	start a first channel with mode FD.
	In this case, we may have unpredictable behavior for the stream
	which is not already started. probably underrun/overrun.
So, this solution rises problem for full-duplex management.

2) 3 static dais exclusive
	- dai tx
	- dai rx
	- dai rx-tx (fd)
This is the current implementation.
The choice of the dai is done at probe time. It is provided by DT 
through sound-dai parameter.
When dai fd is selected, after starting first stream, we assume that
second stream will be started. In this case we wait for second stream
to be available before enabling IP and starting transfers.

3) 1 dynamic dai
	- dai rx or tx or fd (according to dma conf in IP node)
Here the driver exposes only a single dai constructed from dma 
configuration provided by IP DT node.
This allows to get ride of sound-dai parameter.
This was my suggestion after Rob comments on DT bindings.

Hope this will help clarifying design constraints.

>> Moreover specific processing has to be performed if both channels are used.
>
> Given that this case has to be supported anyway I'd be more inclined to
> ask the question the other way around TBH.
>

Best regards
olivier

^ permalink raw reply

* [PATCH v3] arm64: dts: rk3399: add support for firefly-rk3399 board
From: Heiko Stübner @ 2017-04-13  8:22 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <6d1eb0cc-fe3b-a483-123c-8558096c0846@rock-chips.com>

Am Donnerstag, 13. April 2017, 12:00:19 CEST schrieb Kever Yang:
> Hi Heiko,
> 
> On 04/12/2017 09:29 PM, Heiko Stuebner wrote:
> > Hi Kever,
> > 
> > Am Montag, 10. April 2017, 11:50:13 CEST schrieb Kever Yang:
> >> Firefly-rk3399 is a bord from T-Firefly, you can find detail about
> >> it here:
> >> http://en.t-firefly.com/en/firenow/Firefly_RK3399/
> >> 
> >> This patch add basic node for the board and make it able to bring
> >> up.
> >> 
> >> Peripheral works:
> >> - usb hub which connect to ehci controller;
> >> - UART2 debug
> >> - eMMC
> >> - PCIe
> >> 
> >> Not work:
> >> - USB 3.0 HOST, type-C port
> >> - sdio, sd-card
> >> 
> >> Not test for other peripheral:
> >> - HDMI
> >> - Ethernet
> >> - OPTICAL
> >> - WiFi/BT
> >> - MIPI CSI/DSI
> >> - IR
> >> - EDP/DP
> >> 
> >> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
> > 
> > applied for 4.13, as we're a bit late for 4.12, with the following 
changes:
> Thanks for your help, I'm not familiar with the devices status on upstream
> because not working on kernel upstream for a long time.
> 
> > - commit subject
> > - dropped status from backlight (as there is no disabled common node
> > 
> >    and it's specific to the firefly itself)
> > 
> > - quite some reordering of properties
> > - reordered regulator nodes per their addresses: 0x1b < 0x40
> > - dropped obsolete regulator-compatible properties
> > - fixed gpio-irq on the mpu6500
> > - dropped out-of-tree orientation properties of mpu6500
> > 
> >    --> please provide the optional "mount-matrix" in a follow-up patch
> >    
> >        see bindings/iio/imu/inv_mpu6050.txt
> 
> Maybe we can drop this mpu6050 node first? I can't find binding file for it.

The binding (and driver in the kernel) is the same as for the mpu6050.
So you can find the documentation in
Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt

Someone should add the other ids from 
drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
to the binding.

mount-matrix is optional, so the basic node itself can stay in the dt.


Heiko

^ permalink raw reply

* [PATCH v1 1/1] mtd: mtk-nor: set controller's address width according to nor flash
From: Cyrille Pitchen @ 2017-04-13  8:24 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1492051237.2866.36.camel@mhfsdcap03>

Hi Guochun,

Le 13/04/2017 ? 04:40, Guochun Mao a ?crit :
> Hi Cyrille,
> 
> On Wed, 2017-04-12 at 22:57 +0200, Cyrille Pitchen wrote:
>> Hi Guochun,
>>
>> Le 05/04/2017 ? 10:37, Guochun Mao a ?crit :
>>> When nor's size larger than 16MByte, nor's address width maybe
>>> set to 3 or 4, and controller should change address width according
>>> to nor's setting.
>>>
>>> Signed-off-by: Guochun Mao <guochun.mao@mediatek.com>st

Acked-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>

>>> ---
>>>  drivers/mtd/spi-nor/mtk-quadspi.c |   27 +++++++++++++++++++++++++++
>>>  1 file changed, 27 insertions(+)
>>>
>>> diff --git a/drivers/mtd/spi-nor/mtk-quadspi.c b/drivers/mtd/spi-nor/mtk-quadspi.c
>>> index e661877..b637770 100644
>>> --- a/drivers/mtd/spi-nor/mtk-quadspi.c
>>> +++ b/drivers/mtd/spi-nor/mtk-quadspi.c
>>> @@ -104,6 +104,8 @@
>>>  #define MTK_NOR_MAX_RX_TX_SHIFT		6
>>>  /* can shift up to 56 bits (7 bytes) transfer by MTK_NOR_PRG_CMD */
>>>  #define MTK_NOR_MAX_SHIFT		7
>>> +/* nor controller 4-byte address mode enable bit */
>>> +#define MTK_NOR_4B_ADDR_EN		BIT(4)
>>>  
>>>  /* Helpers for accessing the program data / shift data registers */
>>>  #define MTK_NOR_PRG_REG(n)		(MTK_NOR_PRGDATA0_REG + 4 * (n))
>>> @@ -230,10 +232,35 @@ static int mt8173_nor_write_buffer_disable(struct mt8173_nor *mt8173_nor)
>>>  				  10000);
>>>  }
>>>  
>>> +static void mt8173_nor_set_addr_width(struct mt8173_nor *mt8173_nor)
>>> +{
>>> +	u8 val;
>>> +	struct spi_nor *nor = &mt8173_nor->nor;
>>> +
>>> +	val = readb(mt8173_nor->base + MTK_NOR_DUAL_REG);
>>> +
>>> +	switch (nor->addr_width) {
>>> +	case 3:
>>> +		val &= ~MTK_NOR_4B_ADDR_EN;
>>> +		break;
>>> +	case 4:
>>> +		val |= MTK_NOR_4B_ADDR_EN;
>>> +		break;
>>> +	default:
>>> +		dev_warn(mt8173_nor->dev, "Unexpected address width %u.\n",
>>> +			 nor->addr_width);
>>> +		break;
>>> +	}
>>> +
>>> +	writeb(val, mt8173_nor->base + MTK_NOR_DUAL_REG);
>>> +}
>>> +
>>>  static void mt8173_nor_set_addr(struct mt8173_nor *mt8173_nor, u32 addr)
>>>  {
>>>  	int i;
>>>  
>>> +	mt8173_nor_set_addr_width(mt8173_nor);
>>> +
>>>  	for (i = 0; i < 3; i++) {
>>
>> Should it be 'i < nor->addr_width' instead of 'i < 3' ?
>> Does it work when accessing data after 128Mbit ?
> 
> Yes, it can work.
> 
> Let's see the whole function,
> 
> static void mt8173_nor_set_addr(struct mt8173_nor *mt8173_nor, u32 addr)
> {
>         int i;
> 
>         mt8173_nor_set_addr_width(mt8173_nor);
> 
>         for (i = 0; i < 3; i++) {
>                 writeb(addr & 0xff, mt8173_nor->base + MTK_NOR_RADR0_REG
> + i * 4);
>                 addr >>= 8;
>         }
>         /* Last register is non-contiguous */
>         writeb(addr & 0xff, mt8173_nor->base + MTK_NOR_RADR3_REG);
> }
> 
> The nor controller has 4 registers for address.
> This '3' indicates the number of contiguous address' registers
> base + MTK_NOR_RADR0_REG(0x10)
> base + MTK_NOR_RADR1_REG(0x14)
> base + MTK_NOR_RADR2_REG(0x18),
> but the last address register is non-contiguous,
> it's base + MTK_NOR_RADR3_REG(0xc8)
> 
> mt8173_nor_set_addr will set addr into these 4 registers by Byte.
> The bit MTK_NOR_4B_ADDR_EN will decide whether 3-byte(0x10,0x14,0x18)
> or 4-byte(0x10,0x14,x018,0xc8) been sent to nor device.
> and, it can access data after 128Mbit when sent 4-byte address.

Indeed, you're right. Sorry for the noise!

> 
> Best regards,
> 
> Guochun
> 
>>
>> Best regards,
>>
>> Cyrille
>>
>>>  		writeb(addr & 0xff, mt8173_nor->base + MTK_NOR_RADR0_REG + i * 4);
>>>  		addr >>= 8;
>>>
>>
> 
> 
> 

^ permalink raw reply

* [PATCH 2/2] hwrng: mtk: Add driver for hardware random generator on MT7623 SoC
From: PrasannaKumar Muralidharan @ 2017-04-13  8:39 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1492067108-14748-3-git-send-email-sean.wang@mediatek.com>

Hi Sean,

Mostly looks good, have few minor comments.

On 13 April 2017 at 12:35,  <sean.wang@mediatek.com> wrote:
> +static bool mtk_rng_wait_ready(struct hwrng *rng, bool wait)
> +{
> +       struct mtk_rng *priv = to_mtk_rng(rng);
> +       int ready;
> +
> +       ready = readl(priv->base + RNG_CTRL) & RNG_READY;
> +       if (!ready && wait)
> +               readl_poll_timeout_atomic(priv->base + RNG_CTRL, ready,
> +                                         ready & RNG_READY, USEC_POLL,
> +                                         TIMEOUT_POLL);
> +       return !!ready;
> +}

Use readl_poll_timeout_atomic's return value or -EIO instead of
!!ready. This will simplify mtk_rng_read.

> +static int mtk_rng_read(struct hwrng *rng, void *buf, size_t max, bool wait)
> +{
> +       struct mtk_rng *priv = to_mtk_rng(rng);
> +       int retval = 0;
> +
> +       while (max >= sizeof(u32)) {
> +               if (!mtk_rng_wait_ready(rng, wait))
> +                       break;
> +
> +               *(u32 *)buf = readl(priv->base + RNG_DATA);
> +               retval += sizeof(u32);
> +               buf += sizeof(u32);
> +               max -= sizeof(u32);
> +       }
> +
> +       if (unlikely(wait && max))
> +               dev_warn(priv->dev, "timeout might be not properly set\n");

Is this really necessary? Better to choose proper timeout than
providing this warning message. In rare cases if the timeout could
occur due to some reason (may be a hardware fault) print appropriate
warning message.

> +       return retval || !wait ? retval : -EIO;
> +}

Set retavl to mtk_rng_wait_ready and return retval.

Regards,
Prasanna

^ permalink raw reply

* [PATCH v3 7/8] arm64: exception: handle asynchronous SError interrupt
From: Xiongfeng Wang @ 2017-04-13  8:44 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1490869877-118713-8-git-send-email-xiexiuqi@huawei.com>

Hi Xiuqi,

On 2017/3/30 18:31, Xie XiuQi wrote:
> Error Synchronization Barrier (ESB; part of the ARMv8.2 Extensions)
> is used to synchronize Unrecoverable errors. That is, containable errors
> architecturally consumed by the PE and not silently propagated.
> 
> With ESB it is generally possible to isolate an unrecoverable error
> between two ESB instructions. So, it's possible to recovery from

>  /* ISS field definitions for exceptions taken in to Hyp */
>  #define ESR_ELx_CV		(UL(1) << 24)
>  #define ESR_ELx_COND_SHIFT	(20)
> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index 43512d4..d8a7306 100644
> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -69,7 +69,14 @@
>  #define BAD_FIQ		2
>  #define BAD_ERROR	3
>  
> +	.arch_extension ras
> +
>  	.macro	kernel_entry, el, regsize = 64
> +#ifdef CONFIG_ARM64_ESB
> +	.if	\el == 0
> +	esb
> +	.endif
> +#endif
>  	sub	sp, sp, #S_FRAME_SIZE
>  	.if	\regsize == 32
>  	mov	w0, w0				// zero upper 32 bits of x0
> @@ -208,6 +215,7 @@ alternative_else_nop_endif
>  #endif
>  
>  	.if	\el == 0
> +	msr	daifset, #0xF			// Set flags
>  	ldr	x23, [sp, #S_SP]		// load return stack pointer
>  	msr	sp_el0, x23
>  #ifdef CONFIG_ARM64_ERRATUM_845719
> @@ -226,6 +234,15 @@ alternative_else_nop_endif
>  
>  	msr	elr_el1, x21			// set up the return data
>  	msr	spsr_el1, x22
> +
> +#ifdef CONFIG_ARM64_ESB
> +	.if \el == 0
> +	esb					// Error Synchronization Barrier
> +	mrs	x21, disr_el1			// Check for deferred error
> +	tbnz	x21, #31, el1_sei

We may need to clear disr_el1.A after reading it because the hardware won't clear it.

> +	.endif
> +#endif
> +
>  	ldp	x0, x1, [sp, #16 * 0]
>  	ldp	x2, x3, [sp, #16 * 1]
>  	ldp	x4, x5, [sp, #16 * 2]
> @@ -318,7 +335,7 @@ ENTRY(vectors)
>  	ventry	el1_sync_invalid		// Synchronous EL1t
>  	ventry	el1_irq_invalid			// IRQ EL1t
>  	ventry	el1_fiq_invalid			// FIQ EL1t
> -	ventry	el1_error_invalid		// Error EL1t
> +	ventry	el1_error			// Error EL1t
>  
>  	ventry	el1_sync			// Synchronous EL1h
>  	ventry	el1_irq				// IRQ EL1h
> @@ -328,7 +345,7 @@ ENTRY(vectors)
>  	ventry	el0_sync			// Synchronous 64-bit EL0
>  	ventry	el0_irq				// IRQ 64-bit EL0
>  	ventry	el0_fiq_invalid			// FIQ 64-bit EL0
> -	ventry	el0_error_invalid		// Error 64-bit EL0
> +	ventry	el0_error			// Error 64-bit EL0
>  
>  #ifdef CONFIG_COMPAT
>  	ventry	el0_sync_compat			// Synchronous 32-bit EL0
> @@ -508,12 +525,31 @@ el1_preempt:
>  	ret	x24
>  #endif
>  
> +	.align	6
> +el1_error:
> +	kernel_entry 1
> +el1_sei:
> +	/*
> +	 * asynchronous SError interrupt from kernel
> +	 */
> +	mov	x0, sp
> +	mrs	x1, esr_el1
> +	mov	x2, #1				// exception level of SEI generated
> +	b	do_sei
> +ENDPROC(el1_error)
> +
> +
>  /*
>   * EL0 mode handlers.
>   */
>  	.align	6
>  el0_sync:
>  	kernel_entry 0
> +#ifdef CONFIG_ARM64_ESB
> +	mrs     x26, disr_el1
> +	tbnz    x26, #31, el0_sei		// check DISR.A
> +	msr	daifclr, #0x4			// unmask SEI
> +#endif
>  	mrs	x25, esr_el1			// read the syndrome register
>  	lsr	x24, x25, #ESR_ELx_EC_SHIFT	// exception class
>  	cmp	x24, #ESR_ELx_EC_SVC64		// SVC in 64-bit state
> @@ -688,8 +724,38 @@ el0_inv:
>  ENDPROC(el0_sync)
>  
>  	.align	6
> +el0_error:
> +	kernel_entry 0
> +el0_sei:
> +	/*
> +	 * asynchronous SError interrupt from userspace
> +	 */
> +	ct_user_exit
> +	mov	x0, sp
> +	mrs	x1, esr_el1
> +	mov	x2, #0
> +	bl	do_sei
> +	b	ret_to_user
> +ENDPROC(el0_error)
> +
> +	.align	6
>  el0_irq:
>  	kernel_entry 0
> +#ifdef CONFIG_ARM64_ESB
> +	mrs     x26, disr_el1
> +	tbz     x26, #31, el0_irq_naked          // check DISR.A
> +
> +	mov	x0, sp
> +	mrs	x1, esr_el1
> +	mov	x2, 0
> +
> +	/*
> +	 * The SEI generated at EL0 is not affect this irq context,
> +	 * so after sei handler, we continue process this irq.
> +	 */
> +	bl	do_sei
> +	msr     daifclr, #0x4                   // unmask SEI
> +#endif
>  el0_irq_naked:
>  	enable_dbg
>  #ifdef CONFIG_TRACE_IRQFLAGS

Thanks,
Wang Xiongfeng

^ permalink raw reply

* [PATCH net-next 1/4] ixgbe: sparc: rename the ARCH_WANT_RELAX_ORDER to IXGBE_ALLOW_RELAXED_ORDER
From: Gabriele Paoloni @ 2017-04-13  9:10 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <42428725-89f1-1508-4e3d-723e087b3bbb@huawei.com>

Hi David

> -----Original Message-----
> Subject: Re: [PATCH net-next 1/4] ixgbe: sparc: rename the
> ARCH_WANT_RELAX_ORDER to IXGBE_ALLOW_RELAXED_ORDER
> Date: Sat, 1 Apr 2017 11:26:03 -0700
> From: David Miller <davem@davemloft.net>
> To: dingtianhong at huawei.com
> CC: catalin.marinas at arm.com, will.deacon at arm.com, mark.rutland at arm.com,
> robin.murphy at arm.com, jeffrey.t.kirsher at intel.com,
> alexander.duyck at gmail.com, linux-arm-kernel at lists.infradead.org,
> netdev at vger.kernel.org
> 
> From: Ding Tianhong <dingtianhong@huawei.com>
> Date: Sat, 1 Apr 2017 15:25:51 +0800
> 
> > Till now only the Intel ixgbe could support enable
> > Relaxed ordering in the drivers for special architecture,
> > but the ARCH_WANT_RELAX_ORDER is looks like a general name
> > for all arch, so rename to a specific name for intel
> > card looks more appropriate.
> >
> > Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
> 
> This is not a driver specific facility.
> 
> Any driver can test this symbol and act accordingly.
> 
> Just because IXGBE is the first and only user, doesn't mean
> the facility is driver specific.


Please correct me if I am wrong but my understanding is that the standard
way to enable/disable relaxed ordering is to set/clear bit 4 of the Device
Control Register (PCI_EXP_DEVCTL_RELAX_EN 0x0010 /* Enable relaxed
ordering */).
Now I have looked up for all drivers either enabling or disabling relaxed
ordering and none of them seems to need a symbol to decide whether to
enable it or not.
Also it seems to me enabling/disabling relaxed ordering is never bound to the
host architecture.

So why this should be (or it is expected to be) a generic symbol?
Wouldn't it be more correct to have this as a driver specific symbol now and
move it to a generic one later once we have other drivers requiring it?
  
Many thanks
Gab

> 
> Thank you.
> 
> .
> 

^ permalink raw reply

* kvm/arm64: use-after-free in kvm_unmap_hva_handler/unmap_stage2_pmds
From: Suzuki K Poulose @ 2017-04-13  9:17 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1050c9d8-5813-5df9-29e5-3ab6e61b5de6@arm.com>

On 12/04/17 19:43, Marc Zyngier wrote:
> On 12/04/17 17:19, Andrey Konovalov wrote:
>
> Hi Andrey,
>
>> Apparently this wasn't fixed, I've got this report again on
>> linux-next-c4e7b35a3 (Apr 11), which includes 8b3405e34 "kvm:
>> arm/arm64: Fix locking for kvm_free_stage2_pgd".
>
> This looks like a different bug.
>
>>
>> I now have a way to reproduce it, so I can test proposed patches. I
>> don't have a simple C reproducer though.
>>
>> The bug happens when the following syzkaller program is executed:
>>
>> mmap(&(0x7f0000000000/0xc000)=nil, (0xc000), 0x3, 0x32, 0xffffffffffffffff, 0x0)
>> unshare(0x400)
>> perf_event_open(&(0x7f000002f000-0x78)={0x1, 0x78, 0x0, 0x0, 0x0, 0x0,
>> 0x0, 0x6, 0x0, 0x0, 0xd34, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
>> 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, 0x0, 0xffffffff,
>> 0xffffffffffffffff, 0x0)
>> r0 = openat$kvm(0xffffffffffffff9c,
>> &(0x7f000000c000-0x9)="2f6465762f6b766d00", 0x0, 0x0)
>> ioctl$TIOCSBRK(0xffffffffffffffff, 0x5427)
>> r1 = ioctl$KVM_CREATE_VM(r0, 0xae01, 0x0)
>> syz_kvm_setup_cpu$arm64(r1, 0xffffffffffffffff,
>> &(0x7f0000dc6000/0x18000)=nil, &(0x7f000000c000)=[{0x0,
>> &(0x7f000000c000)="5ba3c16f533efbed09f8221253c73763327fadce2371813b45dd7f7982f84a873e4ae89a6c2bd1af83a6024c36a1ff518318",
>> 0x32}], 0x1, 0x0, &(0x7f000000d000-0x10)=[@featur2={0x1, 0x3}], 0x1)
>
> Is that the only thing the program does? Or is there anything running in
> parallel?
>
>> ==================================================================
>> BUG: KASAN: use-after-free in arch_spin_is_locked
>> include/linux/compiler.h:254 [inline]
>> BUG: KASAN: use-after-free in unmap_stage2_range+0x990/0x9a8
>> arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:295
>> Read of size 8 at addr ffff800004476730 by task syz-executor/13106
>>
>> CPU: 1 PID: 13106 Comm: syz-executor Not tainted
>> 4.11.0-rc6-next-20170411-xc2-11025-gc4e7b35a33d4-dirty #5
>> Hardware name: Hardkernel ODROID-C2 (DT)
>> Call trace:
>> [<ffff20000808fd08>] dump_backtrace+0x0/0x440 arch/arm64/kernel/traps.c:505
>> [<ffff2000080903c0>] show_stack+0x20/0x30 arch/arm64/kernel/traps.c:228
>> [<ffff2000088df030>] __dump_stack lib/dump_stack.c:16 [inline]
>> [<ffff2000088df030>] dump_stack+0x110/0x168 lib/dump_stack.c:52
>> [<ffff200008406db8>] print_address_description+0x60/0x248 mm/kasan/report.c:252
>> [<ffff2000084072c8>] kasan_report_error mm/kasan/report.c:351 [inline]
>> [<ffff2000084072c8>] kasan_report+0x218/0x300 mm/kasan/report.c:408
>> [<ffff200008407428>] __asan_report_load8_noabort+0x18/0x20 mm/kasan/report.c:429
>> [<ffff2000080db1b8>] arch_spin_is_locked include/linux/compiler.h:254 [inline]
>
> This is the assert on the spinlock, and the memory is gone.
>
>> [<ffff2000080db1b8>] unmap_stage2_range+0x990/0x9a8
>> arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:295
>> [<ffff2000080db248>] kvm_free_stage2_pgd.part.16+0x30/0x98
>> arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:842
>> [<ffff2000080ddfb8>] kvm_free_stage2_pgd
>> arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:838 [inline]
>
> But we've taken than lock here. There's only a handful of instructions
> in between, and the memory can only go away if there is something
> messing with us in parallel.
>
>> [<ffff2000080ddfb8>] kvm_arch_flush_shadow_all+0x40/0x58
>> arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:1895
>> [<ffff2000080c379c>] kvm_mmu_notifier_release+0x154/0x1d0
>> arch/arm64/kvm/../../../virt/kvm/kvm_main.c:472
>> [<ffff2000083f2b60>] __mmu_notifier_release+0x1c0/0x3e0 mm/mmu_notifier.c:75
>> [<ffff2000083a1fb4>] mmu_notifier_release
>> include/linux/mmu_notifier.h:235 [inline]
>> [<ffff2000083a1fb4>] exit_mmap+0x21c/0x288 mm/mmap.c:2941
>> [<ffff20000810ecd4>] __mmput kernel/fork.c:888 [inline]
>> [<ffff20000810ecd4>] mmput+0xdc/0x2e0 kernel/fork.c:910
>> [<ffff20000811fda8>] exit_mm kernel/exit.c:557 [inline]
>> [<ffff20000811fda8>] do_exit+0x648/0x2020 kernel/exit.c:865
>> [<ffff2000081218b4>] do_group_exit+0xdc/0x260 kernel/exit.c:982
>> [<ffff20000813adf0>] get_signal+0x358/0xf58 kernel/signal.c:2318
>> [<ffff20000808de98>] do_signal+0x170/0xc10 arch/arm64/kernel/signal.c:370
>> [<ffff20000808edb4>] do_notify_resume+0xe4/0x120 arch/arm64/kernel/signal.c:421
>> [<ffff200008083e68>] work_pending+0x8/0x14
>
> So we're being serviced with a signal. Do you know if this signal is
> generated by your syzkaller program? We could be racing between do_exit
> triggered by a fatal signal (this trace) and the closing of the two file
> descriptors (vcpu and vm).
>
> Paolo: does this look possible to you? I can't see what locking we have
> that could prevent this race.

On a quick look, I see two issues:

1) It looks like the mmu_notifier->ops.release could be called twice for a notifier,
from mmu_notifier_unregister() and exit_mmap()->mmu_notifier_release(), which is
causing the problem as above.

This could possibly be avoided by swapping the order of the following operations
in themmu_notifier_unregister():

  a) Invoke ops->release under src_read_lock()
  b) Delete the notifier from the list.

which can prevent mmu_notifier_release() calling the ops->release() again, before
we reach (b).


2) The core KVM code does an mmgrab()/mmdrop on the current->mm to pin the mm_struct. But
this doesn't prevent the "real_address user space" from being destroyed. Since KVM
actually depends on the user pages and page tables, it should really/also(?) use
mmget()/mmput() (See Documentation/vm/active_mm.txt). I understand that mmget() shouldn't
be used for pinning unbounded amount of time. But since we do it from within the same
process context (like say threads), we should be safe to do so.

Thanks
Suzuki

>
> Thanks,
> 	
> M.
>

^ permalink raw reply

* [PATCH 2/2] mmc: sdhci: provide isr for card-detect interrupts
From: Jisheng Zhang @ 2017-04-13  9:17 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <772c085c-beba-cfe8-23ca-0c46a96145fe@intel.com>

On Tue, 11 Apr 2017 13:31:16 +0300 Adrian Hunter wrote:

> On 11/04/17 13:20, Jisheng Zhang wrote:
> > We found one issue on BG4CT platforms with GPIO for sdcar card detect
> > interrupt: remove sdcard when there's read write access to the sdcard,
> > sometimes the card remove event can't be handled for a long time, so
> > the  system still think the sdcard is still plugged in. It turns out
> > that the sdhci_card_event() is missing in common slot gpio's card
> > detect isr.  
> 
> ->card_event() is called by mmc_rescan().  Why is that too late?  

Indeed, maybe the "too late" here is not accurate. Compared with host's
CD interrupt handling, the GPIO CD interrupt handling differs in when to
call sdhci_card_event(): in previous case, it's called in ISR, in later
it's in workqueue.

Now, I agree it's not necessary to put into ISR. So let's ignore the
two patches.

Thanks for your review,
Jisheng

^ permalink raw reply

* [PATCH 1/3] clk: sunxi-ng: Add clk notifier to gate then ungate PLL clocks
From: Maxime Ripard @ 2017-04-13  9:27 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAGb2v64z9+FyozKf=O4ASFrEgZoeGJVBSNR6NsnyP4dHgYZqfw@mail.gmail.com>

On Thu, Apr 13, 2017 at 03:35:30PM +0800, Chen-Yu Tsai wrote:
> On Thu, Apr 13, 2017 at 3:02 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > Hi Chen-Yu,
> >
> > On Thu, Apr 13, 2017 at 10:13:52AM +0800, Chen-Yu Tsai wrote:
> >> In common PLL designs, changes to the dividers take effect almost
> >> immediately, while changes to the multipliers (implemented as
> >> dividers in the feedback loop) take a few cycles to work into
> >> the feedback loop for the PLL to stablize.
> >>
> >> Sometimes when the PLL clock rate is changed, the decrease in the
> >> divider is too much for the decrease in the multiplier to catch up.
> >> The PLL clock rate will spike, and in some cases, might lock up
> >> completely. This is especially the case if the divider changed is
> >> the pre-divider, which affects the reference frequency.
> >>
> >> This patch introduces a clk notifier callback that will gate and
> >> then ungate a clk after a rate change, effectively resetting it,
> >> so it continues to work, despite any possible lockups. Care must
> >> be taken to reparent any consumers to other temporary clocks during
> >> the rate change, and that this notifier callback must be the first
> >> to be registered.
> >>
> >> This is intended to fix occasional lockups with cpufreq on newer
> >> Allwinner SoCs, such as the A33 and the H3. Previously it was
> >> thought that reparenting the cpu clock away from the PLL while
> >> it stabilized was enough, as this worked quite well on the A31.
> >>
> >> On the A33, hangs have been observed after cpufreq was recently
> >> introduced. With the H3, a more thorough test [1] showed that
> >> reparenting alone isn't enough. The system still locks up unless
> >> the dividers are limited to 1.
> >>
> >> A hunch was if the PLL was stuck in some unknown state, perhaps
> >> gating then ungating it would bring it back to normal. Tests
> >> done by Icenowy Zheng using Ondrej's test firmware shows this
> >> to be a valid solution.
> >>
> >> [1] http://www.spinics.net/lists/arm-kernel/msg552501.html
> >>
> >> Reported-by: Ondrej Jirman <megous@megous.com>
> >> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> >> Tested-by: Icenowy Zheng <icenowy@aosc.io>
> >> Tested-by: Quentin Schulz <quentin.schulz@free-electrons.com>
> >
> > Thanks for looking into this, and coming up with a clean solution, and
> > a great commit log.
> >
> > However, I wondering, isn't that notifier just a re-implementation of
> > CLK_SET_RATE_GATE?
> 
> They are not the same. AFAIK, CLK_SET_RATE_GATE tells the clk framework
> that this clk's rate cannot be changed if it is enabled (which means
> some one is using it). However the clk framework does nothing to
> actually handle it. It just returns an error. Any consumers are
> responsible for gating the clock before making changes. This is a nice
> thing to have, as it can prevent unintended changes to dot clocks or
> audio clocks used with active output streams. We could consider setting
> this for the audio and video PLLs.

Ah, you're right. I merged the two first patches and will send them
for 4.11.

> Here we are dealing with the CPU PLL, which, for practical reasons,
> is always enabled as far as the clk framework is concerned. The
> reason being the OPPs are never low enough for the CPU clock to
> use any other parent. To have it disabled, we would have to kick
> consumers (the CPU clock in this case) to use other clocks, so it's
> safe, remember which ones we kicked, and then bring them back once
> everything is done.
> 
> AFAIK, we, samsung, rockchip, meson, do the temporary reparenting
> using clk_notifiers to access the mux registers directly. As far
> as the clk framework is concerned, nothing has changed.
> 
> I'm not saying it's not possible to support this in the core, but
> the core already has to do a lot of bookkeeping and recalculation
> when anything changes. Adding something transient into the process
> isn't helping. And the reparenting might temporarily violate any
> downstream requirements.
> 
> For now, I think clk notifiers is the easier solution for these
> one off requirements that are pretty much contained in a small
> part of the system.

However, the third one is less urgent, since we don't have H3 cpufreq
support yet, so we won't hit that case, and I'd like to have first a
common function that register the notifiers since the order really
matters, we don't want to have someone getting it wrong.

Since this is 4.13 material, there's no rush on that one though.

Thanks again!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170413/2f3b1b71/attachment-0001.sig>

^ permalink raw reply

* kvm/arm64: use-after-free in kvm_unmap_hva_handler/unmap_stage2_pmds
From: Mark Rutland @ 2017-04-13  9:34 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAAeHK+yzzdQVvGQcuprTHEa7S9CPiAVYodD__jQ8a=CzA6RaZg@mail.gmail.com>

On Wed, Apr 12, 2017 at 08:51:31PM +0200, Andrey Konovalov wrote:
> On Wed, Apr 12, 2017 at 8:43 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> > On 12/04/17 17:19, Andrey Konovalov wrote:

> >> I now have a way to reproduce it, so I can test proposed patches. I
> >> don't have a simple C reproducer though.
> >>
> >> The bug happens when the following syzkaller program is executed:
> >>
> >> mmap(&(0x7f0000000000/0xc000)=nil, (0xc000), 0x3, 0x32, 0xffffffffffffffff, 0x0)
> >> unshare(0x400)
> >> perf_event_open(&(0x7f000002f000-0x78)={0x1, 0x78, 0x0, 0x0, 0x0, 0x0,
> >> 0x0, 0x6, 0x0, 0x0, 0xd34, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
> >> 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, 0x0, 0xffffffff,
> >> 0xffffffffffffffff, 0x0)
> >> r0 = openat$kvm(0xffffffffffffff9c,
> >> &(0x7f000000c000-0x9)="2f6465762f6b766d00", 0x0, 0x0)
> >> ioctl$TIOCSBRK(0xffffffffffffffff, 0x5427)
> >> r1 = ioctl$KVM_CREATE_VM(r0, 0xae01, 0x0)
> >> syz_kvm_setup_cpu$arm64(r1, 0xffffffffffffffff,
> >> &(0x7f0000dc6000/0x18000)=nil, &(0x7f000000c000)=[{0x0,
> >> &(0x7f000000c000)="5ba3c16f533efbed09f8221253c73763327fadce2371813b45dd7f7982f84a873e4ae89a6c2bd1af83a6024c36a1ff518318",
> >> 0x32}], 0x1, 0x0, &(0x7f000000d000-0x10)=[@featur2={0x1, 0x3}], 0x1)
> >
> > Is that the only thing the program does? Or is there anything running in
> > parallel?
> 
> These calls are executed repeatedly and in random order. That's all.
> 
> Except that I'm running the reproducer on a real arm board, so there's
> probably a bunch of stuff going on besides these calls.

I had a go at reproducing this on an arm64 board following [1], but so
far I've had no luck. I've dumped the above into syz-kvm-bug, and I'm
trying to reproduce the issue with:

	PATH=$PATH:$(pwd)/bin syz-execprog \
		-executor $(pwd)/bin/syz-executor \
		-cover=0 -repeat=0 -procs 16 \
		syz-kvm-bug

Just to check, is that the correct way to reproduce the problem with the
above log?

How quickly does that reproduce the problem for you?

Thanks,
Mark.

[1] https://github.com/google/syzkaller/wiki/How-to-execute-syzkaller-programs

^ permalink raw reply

* [PATCH 1/2] drivers: pci: do not disregard parent resources starting at 0x0
From: Lorenzo Pieralisi @ 2017-04-13  9:42 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAKv+Gu9JrU=-SZQcTtFfOP423gVTyM1w9yF=_J1uGd74ntpwLw@mail.gmail.com>

On Thu, Apr 13, 2017 at 08:39:12AM +0100, Ard Biesheuvel wrote:
> On 12 April 2017 at 14:24, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote:
> > [+Yinghai, Bjorn]
> >
> > On Tue, Apr 11, 2017 at 05:33:12PM +0100, Ard Biesheuvel wrote:
> >> Commit f44116ae8818 ("PCI: Remove pci_find_parent_resource() use for
> >> allocation") updated the logic that iterates over all bus resources
> >> and compares them to a given resource, in order to decide whether one
> >> is the parent of the latter.
> >>
> >> This change inadvertently causes pci_find_parent_resource() to disregard
> >> resources starting at address 0x0, resulting in an error such as the one
> >> below on ARM systems whose I/O window starts at 0x0.
> >>
> >> pci_bus 0000:00: root bus resource [mem 0x10000000-0x3efeffff window]
> >> pci_bus 0000:00: root bus resource [io  0x0000-0xffff window]
> >> pci_bus 0000:00: root bus resource [mem 0x8000000000-0xffffffffff window]
> >> pci_bus 0000:00: root bus resource [bus 00-0f]
> >> pci 0000:00:01.0: PCI bridge to [bus 01]
> >> pci 0000:00:02.0: PCI bridge to [bus 02]
> >> pci 0000:00:03.0: PCI bridge to [bus 03]
> >> pci 0000:00:03.0: can't claim BAR 13 [io  0x0000-0x0fff]: no compatible bridge window
> >> pci 0000:03:01.0: can't claim BAR 0 [io  0x0000-0x001f]: no compatible bridge window
> >>
> >> While this never happens on x86, it is perfectly legal in general for a
> >> PCI MMIO or IO window to start at address 0x0, and it was supported in
> >> the code before commit f44116ae8818.
> >>
> >> So let's drop the test for res->start != 0; resource_contains() already
> >> checks whether [start, end) completely covers the resource, and so it
> >> should be redundant.
> >>
> >> Fixes: f44116ae8818 ("PCI: Remove pci_find_parent_resource() use for allocation")
> >
> > I know this code fixes IO claiming on ARM/ARM64 (well, it fixes nothing
> > because we never claim resources on ARM/ARM64 apart from kvmtool and
> > generic host bridge), my _big_ worry is that it can cause endless
> > regressions on other arches, in any case I would be really really
> > careful about adding a Fixes: tag to it.
> >
> 
> The patch is only 3 years old, and is obviously a regression given
> that the change in behavior described here occurs as a side effect.

I agree with you that res->start usage changed with f44116ae8818 but
I am not sure you can call that a regression unless we prove there
was some code relying on the previous behaviour (and it is not just
x86).

Anyway, I am happy to put these two patches (with some tweaks on patch
2) on a branch for testing on ARM64 ACPI platforms to see the best
way forward.

Thanks,
Lorenzo

^ permalink raw reply

* [GIT PULL] Allwinner clock fixes for 4.11, again
From: Maxime Ripard @ 2017-04-13  9:59 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Mike, Stephen,

Here is another PR, without the patches that you didn't want as fixes.

A few more fixes showed up in between, one of them being another build
fix, the other one fixing random CPU crashes happening with CPUfreq
since 4.11

Please merge it for one of 4.11-rc's, thanks!
Maxime

The following changes since commit b467e08a15563dede0d37d3233baa24fb97a7310:

  clk: sunxi-ng: fix recalc_rate formula of NKMP clocks (2017-03-20 10:34:05 +0100)

are available in the git repository at:

  https://git.kernel.org/pub/scm/linux/kernel/git/sunxi/linux.git sunxi-clk-fixes-for-4.11-2-bis

for you to fetch changes up to 372fa10172a2f9e1bfbc70778449628e82b72341:

  clk: sunxi-ng: a33: gate then ungate PLL CPU clk after rate change (2017-04-13 11:22:04 +0200)

----------------------------------------------------------------
Allwinner clock fixes for 4.11

Two build errors fixes for the sunxi-ng drivers.

The two other patches fix random CPU crashes happening on the A33 since
CPUFreq has been enabled in 4.11.

----------------------------------------------------------------
Chen-Yu Tsai (2):
      clk: sunxi-ng: Add clk notifier to gate then ungate PLL clocks
      clk: sunxi-ng: a33: gate then ungate PLL CPU clk after rate change

Tobias Regnery (2):
      clk: sunxi-ng: fix build error without CONFIG_RESET_CONTROLLER
      clk: sunxi-ng: fix build failure in ccu-sun9i-a80 driver

 drivers/clk/sunxi-ng/Kconfig         |  2 ++
 drivers/clk/sunxi-ng/ccu-sun8i-a33.c | 11 ++++++++
 drivers/clk/sunxi-ng/ccu_common.c    | 49 ++++++++++++++++++++++++++++++++++++
 drivers/clk/sunxi-ng/ccu_common.h    | 12 +++++++++
 4 files changed, 74 insertions(+)

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170413/97cfe53c/attachment.sig>

^ permalink raw reply

* [PATCH] [media] imx: csi: retain current field order and colorimetry setting as default
From: Philipp Zabel @ 2017-04-13 10:07 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <6c22519f-64f8-7213-d458-23470bdd5ecd@xs4all.nl>

On Wed, 2017-04-12 at 09:03 +0200, Hans Verkuil wrote:
[...]
> >> Do you have a git tree with this patch? It is really hard to review without
> >> having the full imx-media-csi.c source.
> > 
> > The patch applies on top of
> > 
> >   https://github.com/slongerbeam/mediatree.git imx-media-staging-md-v14
> > 
> > I have uploaded a branch
> > 
> >   git://git.pengutronix.de/git/pza/linux imx-media-staging-md-v14+color
> > 
> > with the patch applied on top.
> > 
> >> I think one problem is that it is not clearly defined how subdevs and colorspace
> >> information should work.
> 
> Ah, having the full source helped.
> 
> Ignore my previous review, it was incorrect.

Ok.

> I'll have to think about this some more. I'll get back to this, but it may take some
> time since my vacation starts tomorrow. The spec is simply unclear about how to handle
> this so we have to come up with some guidelines.

Yes, please. Until then, have a nice vacation.

regards
Philipp

^ permalink raw reply


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