Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 02/16] dt/bindings: Update binding for PM domain idle states
From: Lina Iyer @ 2016-09-20 16:17 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <874m5cx6y8.fsf@arm.com>

On Mon, Sep 19 2016 at 09:09 -0600, Brendan Jackman wrote:
>
>On Fri, Sep 16 2016 at 18:39, Sudeep Holla <sudeep.holla@arm.com> wrote:
>> Hi Kevin,
>>
>> Thanks for looking at this and simplifying various discussions we had so
>> far. I was thinking of summarizing something very similar. I couldn't
>> due to lack of time.
>>
>> On 16/09/16 18:13, Kevin Hilman wrote:
>>
>> [...]
>>
>>> I think we're having some terminology issues...
>>>
>>> FWIW, the kernel terminolgy is actually "PM domain", not power domain.
>>> This was intentional because the goal of the PM domain was to group
>>> devices that some PM features.  To be very specific to the kernel, they
>>> us the same set of PM callbacks.  Today, this is most commonly used to
>>> model power domains, where a group of devices share a power rail, but it
>>> does not need to be limited to that.
>>>
>>
>> Agreed/Understood.
>>
>>> That being said, I'm having a hard time understanding the root of the
>>> disagreement.
>>>
>>
>> Yes. I tried to convey the same earlier, but have failed. The only
>> disagreement is about a small part of this DT bindings. We would like to
>> make it completely hierarchical up to CPU nodes. More comments on that
>> below.
>>
>>> It seems that you and Sudeep would like to use domain-idle-states to
>>> replace/superceed cpu-idle-states with the primary goal (and benefit)
>>> being that it simplifies the DT bindings.  Is that correct?
>>>
>>
>> Correct, we want to deprecate cpu-idle-states with the introduction of
>> this hierarchical PM bindings. Yes IMO, it simplifies things and avoids
>> any ABI break we might trigger if we miss to consider some use-case now.
>>
>>> The objections have come in because that means that implies that CPUs
>>> become their own domains, which may not be the case in hardware in the
>>> sense that they share a power rail.
>>>
>>
>> Agreed.
>>
>>> However, IMO, thinking of a CPU as it's own "PM domain" may make some
>>> sense based on the terminology above.
>>>
>>
>> Thanks for that, we do understand that it may not be 100% correct when
>> we strictly considers hardware terminologies instead of above ones.
>> As along as we see no issues with the above terminologies it should be fine.
>>
>>> I think the other objection may be that using a genpd to model domain
>>> with only a single device in it may be overkill, and I agree with that.
>>
>> I too agree with that. Just because we represent that in DT in that way
>> doesn't mean we need to create a genpd to model domain. We can always
>> skip that if not required. That's pure implementation specifics and I
>> have tried to convey the same in my previous emails. I must say you have
>> summarized it very clearly in this email. Thanks again for that.
>>
>>> But, I'm not sure if making CPUs use domain-idle-states implies that
>>> they necessarily have to use genpd is what you are proposing.  Maybe
>>> someone could clarify that?
>>>
>>
>> No, I have not proposing anything around implementation in the whole
>> discussion so far. I have constrained myself just to DT bindings so far.
>> That's the main reason why I was opposed to mentions of OS vs platform
>> co-ordinated modes of CPU suspend in this discussion. IMO that's
>> completely out of scope of this DT binding we are defining here.
>>
Fair. But understand the PM Domain bindings do not impose any
requirements of hierarchy. Domain idle states are defined by the
property domain-idle-states in the domain node. How the DT bindings are
organized is immaterial to the PM Domain core.

It is a different exercise all together to look at CPU PSCI modes and
have a unified way of representing them in DT. The current set of
patches does not dictate where the domain idle states be located (pardon
my example in the patch, which was not updated to reflect that). That
said, I do require that domains that are controlled by the PSCI f/w be
defined under the 'psci' node in DT, which is fair. All the domain needs
are phandles to the idle state definitions; how the nodes are arranged
in DT is not of consequence to the driver.

In my mind providing a structure to CPU PM domains that can be used for
both OSI and PC is a separate effort. It may also club what Brendan
mentions below as part of the effort. The hierarchy that is presented in
[1] is inherent in the PM domain hierarchy and idle states don't have to
duplicate that information.

>> Hope that helps/clarifies the misunderstanding/disagreement.
>
>Indeed. My intention was that the proposal would result in the exact
>same kernel behaviour as Lina's current patchset, i.e. there is one
>genpd per cluster, and CPU-level idle states are still handled by
>cpuidle.
>
>The only change from the current patchset would be in initialisation
>code: some coordination would need to be done to determine which idle
>states go into cpuidle and which go into the genpds (whereas with the
>current bindings, states from cpu-idle-states go into cpuidle and states
>from domain-idle-states go into genpd). So you could say that this would
>be a trade-off between binding simplicity and implementation simplicity.
>
I would not oppose the idea of virtual domains around CPUs (I admit I am
not comfortable with the idea though), if that is the right thing to do.
But the scope of that work is extensive and should not be clubbed as
part of this proposal. It is an extensive code rework spanning cpuidle
drivers and PSCI and there are hooks in this code to help you achieve
that.

Thanks,
Lina

[1]. https://patchwork.kernel.org/patch/9264507/

^ permalink raw reply

* [PATCH] clk: mvebu: Add clk support for the orion5x SoC mv88f5181
From: Gregory CLEMENT @ 2016-09-20 16:24 UTC (permalink / raw)
  To: linux-arm-kernel

From: Jamie Lentin <jm@lentin.co.uk>

Referring to the u-boot sources for the Netgear WNR854T, add support
for the mv88f5181.

[gregory.clement at free-electrons.com: fix commit title]
Signed-off-by: Jamie Lentin <jm@lentin.co.uk>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---

Hi Stephen and Mike,

do you agree to give your acked-by on this patch. It is part of a
convertion of old orion5x Socv to the device tree. If you acked-by
this one, then I will be able to take it in my tree and avoiding
breaking the git bisect.

This patch is really about adding a new variant, teh logic remains the
same and also it won't produce any conflict.

Sorry to not have asked this before.

Thanks,

Gregory


 .../devicetree/bindings/clock/mvebu-core-clock.txt |  1 +
 drivers/clk/mvebu/orion.c                          | 70 ++++++++++++++++++++++
 2 files changed, 71 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/mvebu-core-clock.txt b/Documentation/devicetree/bindings/clock/mvebu-core-clock.txt
index 670c2af3e931..eb985a633d59 100644
--- a/Documentation/devicetree/bindings/clock/mvebu-core-clock.txt
+++ b/Documentation/devicetree/bindings/clock/mvebu-core-clock.txt
@@ -52,6 +52,7 @@ Required properties:
 	"marvell,dove-core-clock" - for Dove SoC core clocks
 	"marvell,kirkwood-core-clock" - for Kirkwood SoC (except mv88f6180)
 	"marvell,mv88f6180-core-clock" - for Kirkwood MV88f6180 SoC
+	"marvell,mv88f5181-core-clock" - for Orion MV88F5181 SoC
 	"marvell,mv88f5182-core-clock" - for Orion MV88F5182 SoC
 	"marvell,mv88f5281-core-clock" - for Orion MV88F5281 SoC
 	"marvell,mv88f6183-core-clock" - for Orion MV88F6183 SoC
diff --git a/drivers/clk/mvebu/orion.c b/drivers/clk/mvebu/orion.c
index fd129566c1ce..a6e5bee23385 100644
--- a/drivers/clk/mvebu/orion.c
+++ b/drivers/clk/mvebu/orion.c
@@ -21,6 +21,76 @@ static const struct coreclk_ratio orion_coreclk_ratios[] __initconst = {
 };
 
 /*
+ * Orion 5181
+ */
+
+#define SAR_MV88F5181_TCLK_FREQ      8
+#define SAR_MV88F5181_TCLK_FREQ_MASK 0x3
+
+static u32 __init mv88f5181_get_tclk_freq(void __iomem *sar)
+{
+	u32 opt = (readl(sar) >> SAR_MV88F5181_TCLK_FREQ) &
+		SAR_MV88F5181_TCLK_FREQ_MASK;
+	if (opt == 0)
+		return 133333333;
+	else if (opt == 1)
+		return 150000000;
+	else if (opt == 2)
+		return 166666667;
+	else
+		return 0;
+}
+
+#define SAR_MV88F5181_CPU_FREQ       4
+#define SAR_MV88F5181_CPU_FREQ_MASK  0xf
+
+static u32 __init mv88f5181_get_cpu_freq(void __iomem *sar)
+{
+	u32 opt = (readl(sar) >> SAR_MV88F5181_CPU_FREQ) &
+		SAR_MV88F5181_CPU_FREQ_MASK;
+	if (opt == 0)
+		return 333333333;
+	else if (opt == 1 || opt == 2)
+		return 400000000;
+	else if (opt == 3)
+		return 500000000;
+	else
+		return 0;
+}
+
+static void __init mv88f5181_get_clk_ratio(void __iomem *sar, int id,
+					   int *mult, int *div)
+{
+	u32 opt = (readl(sar) >> SAR_MV88F5181_CPU_FREQ) &
+		SAR_MV88F5181_CPU_FREQ_MASK;
+	if (opt == 0 || opt == 1) {
+		*mult = 1;
+		*div  = 2;
+	} else if (opt == 2 || opt == 3) {
+		*mult = 1;
+		*div  = 3;
+	} else {
+		*mult = 0;
+		*div  = 1;
+	}
+}
+
+static const struct coreclk_soc_desc mv88f5181_coreclks = {
+	.get_tclk_freq = mv88f5181_get_tclk_freq,
+	.get_cpu_freq = mv88f5181_get_cpu_freq,
+	.get_clk_ratio = mv88f5181_get_clk_ratio,
+	.ratios = orion_coreclk_ratios,
+	.num_ratios = ARRAY_SIZE(orion_coreclk_ratios),
+};
+
+static void __init mv88f5181_clk_init(struct device_node *np)
+{
+	return mvebu_coreclk_setup(np, &mv88f5181_coreclks);
+}
+
+CLK_OF_DECLARE(mv88f5181_clk, "marvell,mv88f5181-core-clock", mv88f5181_clk_init);
+
+/*
  * Orion 5182
  */
 
-- 
2.9.3

^ permalink raw reply related

* [PATCH] pinctrl: mvebu: orion5x: Generalise mv88f5181l support for 88f5181
From: Gregory CLEMENT @ 2016-09-20 16:26 UTC (permalink / raw)
  To: linux-arm-kernel

From: Jamie Lentin <jm@lentin.co.uk>

As far as I'm aware the mv88f5181-b1 and mv88f5181l are the same at the
pinctrl level, so re-use the definitions for both.

[gregory.clement at free-electrons.com: fix commit title]
Signed-off-by: Jamie Lentin <jm@lentin.co.uk>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 .../bindings/pinctrl/marvell,orion-pinctrl.txt     |  4 +++-
 drivers/pinctrl/mvebu/pinctrl-orion.c              | 23 +++++++++++-----------
 2 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/Documentation/devicetree/bindings/pinctrl/marvell,orion-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/marvell,orion-pinctrl.txt
index 27570a3a1741..ec8aa3c6936b 100644
--- a/Documentation/devicetree/bindings/pinctrl/marvell,orion-pinctrl.txt
+++ b/Documentation/devicetree/bindings/pinctrl/marvell,orion-pinctrl.txt
@@ -4,7 +4,9 @@ Please refer to marvell,mvebu-pinctrl.txt in this directory for common binding
 part and usage.
 
 Required properties:
-- compatible: "marvell,88f5181l-pinctrl", "marvell,88f5182-pinctrl",
+- compatible: "marvell,88f5181-pinctrl",
+              "marvell,88f5181l-pinctrl",
+              "marvell,88f5182-pinctrl",
               "marvell,88f5281-pinctrl"
 
 - reg: two register areas, the first one describing the first two
diff --git a/drivers/pinctrl/mvebu/pinctrl-orion.c b/drivers/pinctrl/mvebu/pinctrl-orion.c
index 345c3df669a0..84e144167b44 100644
--- a/drivers/pinctrl/mvebu/pinctrl-orion.c
+++ b/drivers/pinctrl/mvebu/pinctrl-orion.c
@@ -64,11 +64,11 @@ static int orion_mpp_ctrl_set(unsigned pid, unsigned long config)
 	return 0;
 }
 
-#define V(f5181l, f5182, f5281) \
-	((f5181l << 0) | (f5182 << 1) | (f5281 << 2))
+#define V(f5181, f5182, f5281) \
+	((f5181 << 0) | (f5182 << 1) | (f5281 << 2))
 
 enum orion_variant {
-	V_5181L = V(1, 0, 0),
+	V_5181  = V(1, 0, 0),
 	V_5182  = V(0, 1, 0),
 	V_5281  = V(0, 0, 1),
 	V_ALL   = V(1, 1, 1),
@@ -103,13 +103,13 @@ static struct mvebu_mpp_mode orion_mpp_modes[] = {
 		 MPP_VAR_FUNCTION(0x0, "gpio", NULL,        V_ALL),
 		 MPP_VAR_FUNCTION(0x2, "pci", "req5",       V_ALL),
 		 MPP_VAR_FUNCTION(0x4, "nand", "re0",       V_5182 | V_5281),
-		 MPP_VAR_FUNCTION(0x5, "pci-1", "clk",      V_5181L),
+		 MPP_VAR_FUNCTION(0x5, "pci-1", "clk",      V_5181),
 		 MPP_VAR_FUNCTION(0x5, "sata0", "act",      V_5182)),
 	MPP_MODE(7,
 		 MPP_VAR_FUNCTION(0x0, "gpio", NULL,        V_ALL),
 		 MPP_VAR_FUNCTION(0x2, "pci", "gnt5",       V_ALL),
 		 MPP_VAR_FUNCTION(0x4, "nand", "we0",       V_5182 | V_5281),
-		 MPP_VAR_FUNCTION(0x5, "pci-1", "clk",      V_5181L),
+		 MPP_VAR_FUNCTION(0x5, "pci-1", "clk",      V_5181),
 		 MPP_VAR_FUNCTION(0x5, "sata1", "act",      V_5182)),
 	MPP_MODE(8,
 		 MPP_VAR_FUNCTION(0x0, "gpio", NULL,        V_ALL),
@@ -165,7 +165,7 @@ static struct mvebu_mpp_ctrl orion_mpp_controls[] = {
 	MPP_FUNC_CTRL(0, 19, NULL, orion_mpp_ctrl),
 };
 
-static struct pinctrl_gpio_range mv88f5181l_gpio_ranges[] = {
+static struct pinctrl_gpio_range mv88f5181_gpio_ranges[] = {
 	MPP_GPIO_RANGE(0, 0, 0, 16),
 };
 
@@ -177,14 +177,14 @@ static struct pinctrl_gpio_range mv88f5281_gpio_ranges[] = {
 	MPP_GPIO_RANGE(0, 0, 0, 16),
 };
 
-static struct mvebu_pinctrl_soc_info mv88f5181l_info = {
-	.variant = V_5181L,
+static struct mvebu_pinctrl_soc_info mv88f5181_info = {
+	.variant = V_5181,
 	.controls = orion_mpp_controls,
 	.ncontrols = ARRAY_SIZE(orion_mpp_controls),
 	.modes = orion_mpp_modes,
 	.nmodes = ARRAY_SIZE(orion_mpp_modes),
-	.gpioranges = mv88f5181l_gpio_ranges,
-	.ngpioranges = ARRAY_SIZE(mv88f5181l_gpio_ranges),
+	.gpioranges = mv88f5181_gpio_ranges,
+	.ngpioranges = ARRAY_SIZE(mv88f5181_gpio_ranges),
 };
 
 static struct mvebu_pinctrl_soc_info mv88f5182_info = {
@@ -212,7 +212,8 @@ static struct mvebu_pinctrl_soc_info mv88f5281_info = {
  * muxing, they are identical.
  */
 static const struct of_device_id orion_pinctrl_of_match[] = {
-	{ .compatible = "marvell,88f5181l-pinctrl", .data = &mv88f5181l_info },
+	{ .compatible = "marvell,88f5181-pinctrl", .data = &mv88f5181_info },
+	{ .compatible = "marvell,88f5181l-pinctrl", .data = &mv88f5181_info },
 	{ .compatible = "marvell,88f5182-pinctrl", .data = &mv88f5182_info },
 	{ .compatible = "marvell,88f5281-pinctrl", .data = &mv88f5281_info },
 	{ }
-- 
2.9.3

^ permalink raw reply related

* [PATCH 2/3] regulator: axp20x: simplify the treatment of linked regulators
From: Jean-Francois Moine @ 2016-09-20 16:38 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <cover.1474616699.git.moinejf@free.fr>

Using ancillary variables for handling the linked regulators simplifies
the loop of regulator creation and makes easier the addition of new
regulator types.

Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
---
 drivers/regulator/axp20x-regulator.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
index 4e5e7c8..7405f5b 100644
--- a/drivers/regulator/axp20x-regulator.c
+++ b/drivers/regulator/axp20x-regulator.c
@@ -511,6 +511,10 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
 	u32 workmode;
 	const char *dcdc1_name = axp22x_regulators[AXP22X_DCDC1].name;
 	const char *dcdc5_name = axp22x_regulators[AXP22X_DCDC5].name;
+	s8 dcdc1_ix = -1;
+	s8 dcdc5_ix = -1;
+	s8 dc1sw_ix = -1;
+	s8 dc5ldo_ix = -1;
 	bool drivevbus = false;
 	u32 skip_bitmap = 0;
 
@@ -524,6 +528,10 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
 	case AXP223_ID:
 		regulators = axp22x_regulators;
 		nregulators = AXP22X_REG_ID_MAX;
+		dcdc1_ix = AXP22X_DCDC1;
+		dcdc5_ix = AXP22X_DCDC5;
+		dc1sw_ix = AXP22X_DC1SW;
+		dc5ldo_ix = AXP22X_DC5LDO;
 		drivevbus = of_property_read_bool(pdev->dev.parent->of_node,
 						  "x-powers,drive-vbus-en");
 		break;
@@ -541,6 +549,10 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
 	case AXP809_ID:
 		regulators = axp809_regulators;
 		nregulators = AXP809_REG_ID_MAX;
+		dcdc1_ix = AXP809_DCDC1;
+		dcdc5_ix = AXP809_DCDC5;
+		dc1sw_ix = AXP809_DC1SW;
+		dc5ldo_ix = AXP809_DC5LDO;
 		break;
 	default:
 		dev_err(&pdev->dev, "Unsupported AXP variant: %ld\n",
@@ -567,8 +579,7 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
 		 * part of this loop to see where we save the DT defined
 		 * name.
 		 */
-		if ((regulators == axp22x_regulators && i == AXP22X_DC1SW) ||
-		    (regulators == axp809_regulators && i == AXP809_DC1SW)) {
+		if (i == dc1sw_ix && dcdc1_name) {
 			new_desc = devm_kzalloc(&pdev->dev, sizeof(*desc),
 						GFP_KERNEL);
 			*new_desc = regulators[i];
@@ -576,8 +587,7 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
 			desc = new_desc;
 		}
 
-		if ((regulators == axp22x_regulators && i == AXP22X_DC5LDO) ||
-		    (regulators == axp809_regulators && i == AXP809_DC5LDO)) {
+		if (i == dc5ldo_ix && dcdc5_name) {
 			new_desc = devm_kzalloc(&pdev->dev, sizeof(*desc),
 						GFP_KERNEL);
 			*new_desc = regulators[i];
@@ -605,14 +615,12 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
 		/*
 		 * Save AXP22X DCDC1 / DCDC5 regulator names for later.
 		 */
-		if ((regulators == axp22x_regulators && i == AXP22X_DCDC1) ||
-		    (regulators == axp809_regulators && i == AXP809_DCDC1))
+		if (i == dcdc1_ix)
 			of_property_read_string(rdev->dev.of_node,
 						"regulator-name",
 						&dcdc1_name);
 
-		if ((regulators == axp22x_regulators && i == AXP22X_DCDC5) ||
-		    (regulators == axp809_regulators && i == AXP809_DCDC5))
+		if (i == dcdc5_ix)
 			of_property_read_string(rdev->dev.of_node,
 						"regulator-name",
 						&dcdc5_name);
-- 
2.10.0

^ permalink raw reply related

* [PATCH v4 2/2] KVM: arm/arm64: Route vtimer events to user space
From: Marc Zyngier @ 2016-09-20 16:42 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <57E14829.3060406@suse.de>

On 20/09/16 15:31, Alexander Graf wrote:
> On 09/20/2016 02:37 PM, Marc Zyngier wrote:

[...]

>>>>> We also need to know "timer line low + timer line masked", as otherwise
>>>>> we might get spurious interrupts in the guest, no?
>>>> Yes. Though you can't really know about this one, and you'll have to
>>>> wait until the next natural exit to find out. As long as the spurious is
>>> We can provoke a special exit for it, no?
>> How? The guest decides to disable its timer. That doesn't trigger any
>> exit whatsoever. You'll have to wait until the next exit from the guest
>> to notice it.
> 
> Before we inject a timer interrupt, we can check whether the pending 
> semantics of user space / kernel space match. If they don't match, we 
> can exit before we inject the interrupt and allow user space to disable 
> the pending state again.

Let's rewind a bit, because I've long lost track of what you're trying
to do to handle what.

You need two signals:

(1) TIMER_LEVEL: the output of the timer line, having accounted for the
IMASK bit. This is conveniently the value of timer->irq.level.

(2) TIMER_IRQ_MASK: an indication from userspace that a timer interrupt
is pending, and that the physical line should be masked.

You need a number of rules:

(a) On exit to userspace, the kernel always exposes the value of
TIMER_LEVEL.

(b) On kernel entry, userspace always exposes the required
TIMER_IRQ_MASK, depending on what has been exposed to it by TIMER_LEVEL.

(c) If on guest exit, TIMER_LEVEL==1 and TIMER_IRQ_MASK==0, perform a
userspace exit, because the emulated GIC needs to make the interrupt
pending.

(d) If on guest exit, TIMER_LEVEL==0 and TIMER_IRQ_MASK==1, perform a
userspace exit, because the guest has disabled its timer before taking
the interrupt, and the emulated GIC needs to retire the pending state.

and that's it. Nothing else. The kernel tells userspace the state of the
timer, and userspace drives the masking of the physical interrupt.
Conveniently, this matches what the current code does.

> 
>>
>>>> "spurious in the GIC sense" (hence returning 0x3ff) and not a spurious
>>>> timer interrupt being presented, you're fine.
>>> Imagine
>>>
>>>     * guest configures timer to fire
>>>     * kvm exits to user space because lines mismatch now
>> I suppose you mean timer fired + physical interrupt for the virtual enabled?
> 
> In this scheme, I would simply say "user space thinks it's pending" == 
> "host vtimer irq is masked". So because user space thinks that no timer 
> is pending and the guest managed to trigger a vtimer irq on the host, 
> the guest exited on a physical vtimer irq. Kvm then checks whether the 
> timer expired and sets the vcpu timer pending status to 1. User space 
> status is still 0, so kvm exits to user space to update the status.
> 
>>
>>>     * user space sets gic line up, setting vcpu irq line high
>>>     * guest runs, handles IRQ, calls EOI with IRQs disabled on the core
>> Same thing: you mean physical interrupt for the virtual timer disabled?
>> or something else?
> 
> I mean the guest sets pstate.I = 0.

The I bit is a mask, just like any other exception bit in PSTATE. So
you're *enabling* interrupts here. But let's assume the interrupts are
disabled, as they are in an interrupt handler.

> 
>>
>>>     * user space handles EOI, but still leaves the vcpu line high
>> If there is another pending interrupt, sure. Otherwise, that's a bug.
> 
> Why is that a bug? The line really is still pending at this point.

So what is this "vcpu line"? HCR_EL2.VI? The timer interrupt through the
GIC? The timer output line?

> 
>>
>>>     * guest runs, sets cval, enables IRQs on the core
>> What is that interrupt? Please name things, because that's very confusing.
> 
> The guest sets pstate.I=1.

And?

> 
>>
>>>     * kvm injects a timer interrupt into the guest
>> Why? Has the timer fired?
> 
> cntv.ctl.imask = 0 here, but user space doesn't know that yet. So the 
> "pending" state is still active.

Rule (a) should apply, always. Whether it is pending or not depends on
what userspace decides to do. If userspace doesn't play ball, the guest
will keep exiting.

> 
>>
>>> At this point, the pending state from user space is bogus, as the line
>>> really isn't pending anymore. However, when the guest asks the GIC at
>>> that point what interrupt is pending, the line down state will have
>>> populated into user space before the mmio request gets processed. So you
>>> will see a 0x3ff return I guess.
>>>
>>> Is that reasonable?
>> I'm sorry, but you need to put names on things and precisely describe
>> the various events. because from the above script, I can derive twenty
>> different scenarios...
> 
> I'm sure one of them has to be racy? :)
> 
> (in fact, there are probably more than what I came up with that really 
> are...)

At this stage, I don't have any understanding of what you're trying to
do. I've given you what I think needs to be implemented, please let me
know if that matches your understanding.

Thanks,

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

^ permalink raw reply

* [GIT PULL] ARM: mvebu: soc for v4.9 (#1)
From: Gregory CLEMENT @ 2016-09-20 16:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <5238448.m4YaC7vVme@wuerfel>

Hi Arnd,
 
 On mar., sept. 20 2016, Arnd Bergmann <arnd@arndb.de> wrote:

> On Tuesday, September 20, 2016 9:25:51 AM CEST Gregory CLEMENT wrote:
>> Hi Arnd,
>>  
>>  On lun., sept. 19 2016, Arnd Bergmann <arnd@arndb.de> wrote:
>> 
>> > On Monday, September 19, 2016 11:46:22 PM CEST Arnd Bergmann wrote:
>> >> On Wednesday, September 14, 2016 5:34:37 PM CEST Gregory CLEMENT wrote:
>> >> > mvebu soc for 4.9 (part 1)
>> >> > 
>> >> > - irq cleanup for old mvebu SoC
>> >> > - Convert orion5x based SoC Netgear WNR854T to devicetree
>> >> > 
>> >> 
>> >> Pulled into next/soc, thanks!
>> >
>> > Sorry, backed out again after seeing the PCI stuff on the WNR854T in
>> > there. I thought the plan was to leave out PCI support from the DT
>> > based machine file, and leave the old board in place, or am I
>> > missing something?
>> 
>> I might have overlooked the thread, I thouhgt the state of the patch was
>> OK as is, and further changes can be done later.
>> 
>> So it seems that it will be 4.10 material.
>> 
>
> We have gained a little more time since Linus delayed the merge window
> by another week, so I think there is still a chance to respin this.
>
> I also really want the NO_IRQ changes to get merged ;-)
>
> Just drop the patch removing wnr854t-setup.c, and add another patch
> on top to remove the PCI initialization from wrt350n-v2-setup.c,
> and I'll take it.

Do you mean board-wnr854t.c?

If it is the case then I can just drop the patch adding this file because
the only things done in this file are about the PCI.

Gregory

>
> 	Arnd

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply

* [PATCH 5/5] arm64: Add uprobe support
From: Catalin Marinas @ 2016-09-20 16:59 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <abaac6ebaa5dd63889803ec568663bc8c8c65b02.1470114993.git.panand@redhat.com>

Hi Pratyush,

On Tue, Aug 02, 2016 at 11:00:09AM +0530, Pratyush Anand wrote:
> --- a/arch/arm64/include/asm/debug-monitors.h
> +++ b/arch/arm64/include/asm/debug-monitors.h
> @@ -70,6 +70,9 @@
>  #define BRK64_ESR_MASK		0xFFFF
>  #define BRK64_ESR_KPROBES	0x0004
>  #define BRK64_OPCODE_KPROBES	(AARCH64_BREAK_MON | (BRK64_ESR_KPROBES << 5))
> +/* uprobes BRK opcodes with ESR encoding  */
> +#define BRK64_ESR_UPROBES	0x0008
> +#define BRK64_OPCODE_UPROBES	(AARCH64_BREAK_MON | (BRK64_ESR_UPROBES << 5))

You can use 0x0005 as immediate, we don't need individual bits here.

> --- a/arch/arm64/include/asm/probes.h
> +++ b/arch/arm64/include/asm/probes.h
> @@ -35,4 +35,8 @@ struct arch_specific_insn {
>  };
>  #endif
>  
> +#ifdef CONFIG_UPROBES
> +typedef u32 uprobe_opcode_t;
> +#endif

We don't need #ifdef around this typedef. Also, all the other
architectures seem to have this typedef in asm/uprobes.h.

> --- a/arch/arm64/include/asm/ptrace.h
> +++ b/arch/arm64/include/asm/ptrace.h
> @@ -217,6 +217,14 @@ int valid_user_regs(struct user_pt_regs *regs, struct task_struct *task);
>  
>  #include <asm-generic/ptrace.h>
>  
> +#define procedure_link_pointer(regs)	((regs)->regs[30])
> +
> +static inline void procedure_link_pointer_set(struct pt_regs *regs,
> +					   unsigned long val)
> +{
> +	procedure_link_pointer(regs) = val;
> +}

Do you need these macro and function here? It seems that they are only
used in arch_uretprobe_hijack_return_addr(), so you can just expand them
in there, no need for additional definitions.

> --- a/arch/arm64/include/asm/thread_info.h
> +++ b/arch/arm64/include/asm/thread_info.h
> @@ -109,6 +109,7 @@ static inline struct thread_info *current_thread_info(void)
>  #define TIF_NEED_RESCHED	1
>  #define TIF_NOTIFY_RESUME	2	/* callback before returning to user */
>  #define TIF_FOREIGN_FPSTATE	3	/* CPU's FP state is not current's */
> +#define TIF_UPROBE		5	/* uprobe breakpoint or singlestep */

Nitpick: you can just use 4 until we cover this gap.

> --- /dev/null
> +++ b/arch/arm64/include/asm/uprobes.h
> @@ -0,0 +1,37 @@
> +/*
> + * Copyright (C) 2014-2015 Pratyush Anand <panand@redhat.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.
> + */
> +
> +#ifndef _ASM_UPROBES_H
> +#define _ASM_UPROBES_H
> +
> +#include <asm/debug-monitors.h>
> +#include <asm/insn.h>
> +#include <asm/probes.h>
> +
> +#define MAX_UINSN_BYTES		AARCH64_INSN_SIZE
> +
> +#define UPROBE_SWBP_INSN	BRK64_OPCODE_UPROBES
> +#define UPROBE_SWBP_INSN_SIZE	4

Nitpick: for consistency, just use AARCH64_INSN_SIZE.

> +#define UPROBE_XOL_SLOT_BYTES	MAX_UINSN_BYTES
> +
> +struct arch_uprobe_task {
> +	unsigned long saved_fault_code;
> +};
> +
> +struct arch_uprobe {
> +	union {
> +		u8 insn[MAX_UINSN_BYTES];
> +		u8 ixol[MAX_UINSN_BYTES];
> +	};
> +	struct arch_probe_insn api;
> +	bool simulate;
> +};
> +
> +extern void flush_uprobe_xol_access(struct page *page, unsigned long uaddr,
> +		void *kaddr, unsigned long len);

I don't think we need this. It doesn't seem to be used anywhere other
than the arm64 uprobes.c file. So I would rather expose
sync_icache_aliases(), similarly to __sync_icache_dcache().

> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -688,7 +688,8 @@ ret_fast_syscall:
>  	ldr	x1, [tsk, #TI_FLAGS]		// re-check for syscall tracing
>  	and	x2, x1, #_TIF_SYSCALL_WORK
>  	cbnz	x2, ret_fast_syscall_trace
> -	and	x2, x1, #_TIF_WORK_MASK
> +	mov     x2, #_TIF_WORK_MASK
> +	and     x2, x1, x2

Is this needed because _TIF_WORK_MASK cannot work as an immediate value
to 'and'? We could reorder the TIF bits, they are not exposed to user to
have ABI implications.

>  	cbnz	x2, work_pending
>  	enable_step_tsk x1, x2
>  	kernel_exit 0
> @@ -718,7 +719,8 @@ work_resched:
>  ret_to_user:
>  	disable_irq				// disable interrupts
>  	ldr	x1, [tsk, #TI_FLAGS]
> -	and	x2, x1, #_TIF_WORK_MASK
> +	mov     x2, #_TIF_WORK_MASK
> +	and     x2, x1, x2
>  	cbnz	x2, work_pending
>  	enable_step_tsk x1, x2
>  	kernel_exit 0

Same here.

> --- /dev/null
> +++ b/arch/arm64/kernel/probes/uprobes.c
> @@ -0,0 +1,227 @@
> +/*
> + * Copyright (C) 2014-2015 Pratyush Anand <panand@redhat.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/highmem.h>
> +#include <linux/ptrace.h>
> +#include <linux/uprobes.h>
> +
> +#include "decode-insn.h"
> +
> +#define UPROBE_INV_FAULT_CODE	UINT_MAX
> +
> +void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
> +		void *src, unsigned long len)
> +{
> +	void *xol_page_kaddr = kmap_atomic(page);
> +	void *dst = xol_page_kaddr + (vaddr & ~PAGE_MASK);
> +
> +	preempt_disable();

kmap_atomic() already disabled preemption.

> +
> +	/* Initialize the slot */
> +	memcpy(dst, src, len);
> +
> +	/* flush caches (dcache/icache) */
> +	flush_uprobe_xol_access(page, vaddr, dst, len);

Just use sync_icache_aliases() here (once exposed in an arm64 header).

> +
> +	preempt_enable();
> +
> +	kunmap_atomic(xol_page_kaddr);
> +}
> +
> +unsigned long uprobe_get_swbp_addr(struct pt_regs *regs)
> +{
> +	return instruction_pointer(regs);
> +}
> +
> +int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe, struct mm_struct *mm,
> +		unsigned long addr)
> +{
> +	probe_opcode_t insn;
> +
> +	/* TODO: Currently we do not support AARCH32 instruction probing */

Is there a way to check (not necessarily in this file) that we don't
probe 32-bit tasks?

> +	if (!IS_ALIGNED(addr, AARCH64_INSN_SIZE))
> +		return -EINVAL;
> +
> +	insn = *(probe_opcode_t *)(&auprobe->insn[0]);
> +
> +	switch (arm_probe_decode_insn(insn, &auprobe->api)) {
> +	case INSN_REJECTED:
> +		return -EINVAL;
> +
> +	case INSN_GOOD_NO_SLOT:
> +		auprobe->simulate = true;
> +		break;
> +
> +	case INSN_GOOD:
> +	default:
> +		break;

Nitpick, we don't need case INSN_GOOD as well since default would cover
it (that's unless you want to change default to BUG()).

> +	}
> +
> +	return 0;
> +}
> +
> +int arch_uprobe_pre_xol(struct arch_uprobe *auprobe, struct pt_regs *regs)
> +{
> +	struct uprobe_task *utask = current->utask;
> +
> +	/* saved fault code is restored in post_xol */
> +	utask->autask.saved_fault_code = current->thread.fault_code;

Does the current->thread.fault_code has any meaning here? We use it in
the arm64 code when delivering a signal to user but I don't think that's
the case here, we are handling a breakpoint instruction and there isn't
anything that set the fault_code.

> +
> +	/* An invalid fault code between pre/post xol event */
> +	current->thread.fault_code = UPROBE_INV_FAULT_CODE;
> +
> +	/* Instruction point to execute ol */
> +	instruction_pointer_set(regs, utask->xol_vaddr);
> +
> +	user_enable_single_step(current);
> +
> +	return 0;
> +}
> +
> +int arch_uprobe_post_xol(struct arch_uprobe *auprobe, struct pt_regs *regs)
> +{
> +	struct uprobe_task *utask = current->utask;
> +
> +	WARN_ON_ONCE(current->thread.fault_code != UPROBE_INV_FAULT_CODE);
> +
> +	/* restore fault code */
> +	current->thread.fault_code = utask->autask.saved_fault_code;

Same here, I don't think this needs restoring if it wasn't meaningful in
the first place.


-- 
Catalin

^ permalink raw reply

* [PATCH 1/4] ARM: tegra: nyan: Use proper IRQ type definitions
From: Jon Hunter @ 2016-09-20 17:15 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160828173246.32621-1-contact@paulk.fr>


On 28/08/16 18:32, Paul Kocialkowski wrote:
> This switches a few interrupt definitions that were using
> GPIO_ACTIVE_HIGH as IRQ type, which is invalid.

May be you are right, but this does not describe why this is invalid.
Can you elaborate?

> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> ---
>  arch/arm/boot/dts/tegra124-nyan.dtsi | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/tegra124-nyan.dtsi b/arch/arm/boot/dts/tegra124-nyan.dtsi
> index 271505e..30a77ec 100644
> --- a/arch/arm/boot/dts/tegra124-nyan.dtsi
> +++ b/arch/arm/boot/dts/tegra124-nyan.dtsi
> @@ -59,7 +59,7 @@
>  			compatible = "maxim,max98090";
>  			reg = <0x10>;
>  			interrupt-parent = <&gpio>;
> -			interrupts = <TEGRA_GPIO(H, 4) GPIO_ACTIVE_HIGH>;
> +			interrupts = <TEGRA_GPIO(H, 4) IRQ_TYPE_EDGE_FALLING>;

If this in invalid then the DT binding doc for the max98090 should be
updated as well.

>  		};
>  
>  		temperature-sensor at 4c {
> @@ -325,7 +325,7 @@
>  					reg = <0x9>;
>  					interrupt-parent = <&gpio>;
>  					interrupts = <TEGRA_GPIO(J, 0)
> -							GPIO_ACTIVE_HIGH>;
> +							IRQ_TYPE_EDGE_BOTH>;
>  					ti,ac-detect-gpios = <&gpio
>  							TEGRA_GPIO(J, 0)
>  							GPIO_ACTIVE_HIGH>;
> 

Cheers
Jon

-- 
nvpublic

^ permalink raw reply

* [PATCH v3] ASoC: exynos: organize the asoc audio into a menu
From: Krzysztof Kozlowski @ 2016-09-20 17:26 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474311470-23175-1-git-send-email-ayaka@soulik.info>

On Tue, Sep 20, 2016 at 02:57:50AM +0800, Randy Li wrote:
> It is simple sound card time, we could assign different codec
> to a interface without making a specific driver for it. The SPDIF
> and I2S interface for Samsung would be possible used by
> simple-sound-card, but not sure about the PCM.
> 
> Those S3C time entries are left alone as I don't think any new board
> would need them.
> 
> Signed-off-by: Randy Li <ayaka@soulik.info>
> ---
>  sound/soc/samsung/Kconfig | 57 ++++++++++++++++++++++++-----------------------
>  1 file changed, 29 insertions(+), 28 deletions(-)
> 

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

^ permalink raw reply

* [PATCH] KVM: arm/arm64: timer: Fix hw sync for user space irqchip path
From: Marc Zyngier @ 2016-09-20 17:28 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474002971-31434-1-git-send-email-agraf@suse.de>

Alex,

On 16/09/16 06:16, Alexander Graf wrote:
> While adding the new vgic implementation, apparently nobody tested
> the non-vgic path where user space controls the vgic, so two functions
> slipped through the cracks that get called in generic code but don't
> check whether hardware support is enabled.
> 
> This patch guards them with proper checks to ensure we only try to
> use vgic data structures if they are available. Without this, I get
> a stack trace:
> 
> [   74.363037] Unable to handle kernel paging request at virtual address ffffffffffffffe8
> [...]
> [   74.929654] [<ffff000008824bcc>] _raw_spin_lock+0x1c/0x58
> [   74.935133] [<ffff0000080b7f20>] kvm_vgic_flush_hwstate+0x88/0x288
> [   74.941406] [<ffff0000080ab0b4>] kvm_arch_vcpu_ioctl_run+0xfc/0x630
> [   74.947766] [<ffff0000080a15bc>] kvm_vcpu_ioctl+0x2f4/0x710
> [   74.953420] [<ffff0000082788a8>] do_vfs_ioctl+0xb0/0x728
> [   74.958807] [<ffff000008278fb4>] SyS_ioctl+0x94/0xa8
> [   74.963844] [<ffff000008083744>] el0_svc_naked+0x38/0x3c
> 
> Fixes: 0919e84c0
> Cc: stable at vger.kernel.org
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  virt/kvm/arm/vgic/vgic.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c
> index e83b7fe..9f312ba 100644
> --- a/virt/kvm/arm/vgic/vgic.c
> +++ b/virt/kvm/arm/vgic/vgic.c
> @@ -645,6 +645,9 @@ next:
>  /* Sync back the hardware VGIC state into our emulation after a guest's run. */
>  void kvm_vgic_sync_hwstate(struct kvm_vcpu *vcpu)
>  {
> +	if (!vcpu->kvm->arch.vgic.enabled)
> +		return;
> +
>  	vgic_process_maintenance_interrupt(vcpu);
>  	vgic_fold_lr_state(vcpu);
>  	vgic_prune_ap_list(vcpu);
> @@ -653,6 +656,9 @@ void kvm_vgic_sync_hwstate(struct kvm_vcpu *vcpu)
>  /* Flush our emulation state into the GIC hardware before entering the guest. */
>  void kvm_vgic_flush_hwstate(struct kvm_vcpu *vcpu)
>  {
> +	if (!vcpu->kvm->arch.vgic.enabled)
> +		return;
> +
>  	spin_lock(&vcpu->arch.vgic_cpu.ap_list_lock);
>  	vgic_flush_lr_state(vcpu);
>  	spin_unlock(&vcpu->arch.vgic_cpu.ap_list_lock);
> 

I hate that fix, because it papers over the fact that we have uninitialized
structures all over the shop, and that's not exactly great.

How about the following instead:

diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index c94b90d..0961128 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -472,6 +472,9 @@ static int kvm_vcpu_first_run_init(struct kvm_vcpu *vcpu)
 			return ret;
 	}
 
+	if (unlikely(!irqchip_in_kernel(kvm)))
+		kvm_no_vgic_init(kvm);
+
 	/*
 	 * Enable the arch timers only if we have an in-kernel VGIC
 	 * and it has been properly initialized, since we cannot handle
diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index bb46c03..1b70b1e 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -327,4 +327,6 @@ int kvm_send_userspace_msi(struct kvm *kvm, struct kvm_msi *msi);
  */
 int kvm_vgic_setup_default_irq_routing(struct kvm *kvm);
 
+void kvm_no_vgic_init(struct kvm *kvm);
+
 #endif /* __KVM_ARM_VGIC_H */
diff --git a/virt/kvm/arm/vgic/vgic-init.c b/virt/kvm/arm/vgic/vgic-init.c
index 83777c1..7b8f12b 100644
--- a/virt/kvm/arm/vgic/vgic-init.c
+++ b/virt/kvm/arm/vgic/vgic-init.c
@@ -151,9 +151,11 @@ static int kvm_vgic_dist_init(struct kvm *kvm, unsigned int nr_spis)
 	INIT_LIST_HEAD(&dist->lpi_list_head);
 	spin_lock_init(&dist->lpi_list_lock);
 
-	dist->spis = kcalloc(nr_spis, sizeof(struct vgic_irq), GFP_KERNEL);
-	if (!dist->spis)
-		return  -ENOMEM;
+	if (nr_spis) {
+		dist->spis = kcalloc(nr_spis, sizeof(struct vgic_irq), GFP_KERNEL);
+		if (!dist->spis)
+			return  -ENOMEM;
+	}
 
 	/*
 	 * In the following code we do not take the irq struct lock since
@@ -325,6 +327,21 @@ int vgic_lazy_init(struct kvm *kvm)
 	return ret;
 }
 
+void kvm_no_vgic_init(struct kvm *kvm)
+{
+	mutex_lock(&kvm->lock);
+	if (unlikely(!vgic_initialized(kvm))) {
+		struct kvm_vcpu *vcpu;
+		int i;
+
+		kvm_vgic_dist_init(kvm, 0);
+		kvm_for_each_vcpu(i, vcpu, kvm)
+			kvm_vgic_vcpu_init(vcpu);
+		kvm->arch.vgic.initialized = true;
+	}
+	mutex_unlock(&kvm->lock);
+}
+
 /* RESOURCE MAPPING */
 
 /**

Thanks,

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

^ permalink raw reply related

* [PATCH 0/2] ARM: imx31/of: clk: init clock controller with CLK_OF_DECLARE
From: Stephen Boyd @ 2016-09-20 17:32 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160919055802.dkf2rxcou44kbya5@pengutronix.de>

On 09/19, Uwe Kleine-K?nig wrote:
> Hello,
> 
> On Mon, Sep 19, 2016 at 05:42:17AM +0300, Vladimir Zapolskiy wrote:
> > The change is based on v4.8.0-rc1 and plus it depends on this fixup:
> > 
> >   ARM: dts: imx31: fix clock control module interrupts description
> > 
> > The change is tested on qemu kzm target and mx31lite board, while both
> > targets don't have DTS in upstream, I had to write simple DTS files for
> > them, because the proposed change is for i.MX31 targets with OF support.
> > 
> > The second change in the series compilation time dependent on the first
> > one, while the first change apparently should break runtime execution
> > of i.MX31 boards with OF support, if both changes are applied everything
> > should be fine, regression testing on the same legacy targets does not
> 
> that means this should be done in a single patch.

Yes, please combine the patches. I can ack it then so it can be
routed through arm-soc.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply

* [PATCH 2/2] clk: imx31: properly init clocks for machines with DT
From: Stephen Boyd @ 2016-09-20 17:36 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474252939-12579-3-git-send-email-vz@mleia.com>

On 09/19, Vladimir Zapolskiy wrote:
> @@ -222,22 +215,31 @@ int __init mx31_clocks_init(unsigned long fref)
>  	return 0;
>  }
>  
> -int __init mx31_clocks_init_dt(void)
> +static void __init mx31_clocks_init_dt(struct device_node *np)

Alternatively we can leave mx31_clocks_init_dt() around as an
exported API, but make it return 0 immediately in the same patch
to the clk driver here. Then after rc1 we can delete the "unused"
function in the clk tree assuming the last caller has been
deleted through arm-soc.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply

* [PATCH 2/4] ARM: tegra: nyan: Use external control for bq24735 charger
From: Jon Hunter @ 2016-09-20 17:40 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160828173246.32621-2-contact@paulk.fr>


On 28/08/16 18:32, Paul Kocialkowski wrote:
> Nyan boards come with an embedded controller that controls when to
> enable and disable the charge. Thus, it should not be left up to the
> kernel to handle that.
> 
> Using the ti,external-control property allows specifying this use-case.

So the bq24735 is populated under the EC's 'i2c-tunnel' property which
is there to specifically interface it's child devices to the host. So I
am a bit confused why this is expose to the host if it should not be used?

Again you may right and I did find the original series [0] for this
which specifically references the Acer Chromebook that needs this.
However, I am not sure why this was never populated? Is there any other
history here?

What is the actual problem you see without making this change? The
original series states ...

"On Acer Chromebook 13 (CB5-311) this module fails to load if the
charger is not inserted, and will error when it is removed."

Cheers
Jon

[0] http://marc.info/?l=linux-pm&m=145447948705686&w=2

-- 
nvpublic

^ permalink raw reply

* [PATCH 3/4] ARM: tegra: nyan-big: Include compatible revisions for proper detection
From: Jon Hunter @ 2016-09-20 17:41 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160828173246.32621-3-contact@paulk.fr>


On 28/08/16 18:32, Paul Kocialkowski wrote:
> Depthcharge (the payload used with cros devices) will attempt to detect
> boards using their revision. This includes all the known revisions for
> the nyan-big board so that the dtb can be selected preferably.

May be I am missing something here, but for the mainline there is only
one dtb available and so why is this needed for the mainline?

I understand for the downstream kernels that the chromebooks ship with
they may include multiple dtbs, but for the mainline we only have one.

Cheers
Jon

-- 
nvpublic

^ permalink raw reply

* [PATCH 4/4] ARM: tegra: nyan-blaze: Include compatible revisions for proper detection
From: Jon Hunter @ 2016-09-20 17:42 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160828173246.32621-4-contact@paulk.fr>


On 28/08/16 18:32, Paul Kocialkowski wrote:
> Depthcharge (the payload used with cros devices) will attempt to detect
> boards using their revision. This includes all the known revisions for
> the nyan-blaze board so that the dtb can be selected preferably.

Again I am not sure what the benefit/need for this is.

Jon

-- 
nvpublic

^ permalink raw reply

* [PATCH V6 2/6] thermal: bcm2835: add thermal driver for bcm2835 soc
From: Stefan Wahren @ 2016-09-20 17:49 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474275600-19378-3-git-send-email-kernel@martin.sperl.org>

Hi Martin,

> kernel at martin.sperl.org hat am 19. September 2016 um 10:59 geschrieben:
> 
> 
> From: Martin Sperl <kernel@martin.sperl.org>
> 
> Add basic thermal driver for bcm2835 SOC.
> 
> This driver currently relies on the firmware setting up the
> tsense HW block and does not set it up itself.
> 
> Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
> Acked-by: Eric Anholt <eric@anholt.net>
> 
> ChangeLog:
>  V1 -> V2: added specific settings depending on compatiblity
> 	   added trip point based on register
> 	   setting up ctrl-register if HW is not enabled by firmware
> 	     as per recommendation of Eric (untested)
> 	   check that clock frequency is in range
> 	     (1.9 - 5MHz - as per comment in clk-bcm2835.c)
>  V2 -> V4: moved back to thermal (not using bcm sub-directory)
>        	   set polling interval to 1second (was 0ms, so interrupt driven)
>  V5 -> V6: added correct depends in KConfig
> 	   removed defined default for RESET_DELAY
> 	   removed obvious comments
> 	   clarify HW setup comments if not set up by FW already
> 	   move clk_prepare_enable to an earlier stage and add error handling
> 	   clarify warning when TS-clock runs out of recommended range
> 	   clk_disable_unprepare added in bcm2835_thermal_remove
> 	   added comment on recommended temperature ranges for SOC
> ---
>  drivers/thermal/Kconfig           |   8 +
>  drivers/thermal/Makefile          |   1 +
>  drivers/thermal/bcm2835_thermal.c | 340
> ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 349 insertions(+)
>  create mode 100644 drivers/thermal/bcm2835_thermal.c
> 
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index 2d702ca..6489723 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -399,4 +399,12 @@ config GENERIC_ADC_THERMAL
>  	  to this driver. This driver reports the temperature by reading ADC
>  	  channel and converts it to temperature based on lookup table.
>  
> +config BCM2835_THERMAL
> +	tristate "Thermal sensors on bcm2835 SoC"
> +	depends on ARCH_BCM2835 || ARCH_BCM2836 || ARCH_BCM2837 || COMPILE_TEST

AFAIK there are no ARCH_BCM2836 or ARCH_BCM2837

After fixing that, you can add my ACK.

> +	depends on OF
> +	depends on HAS_IOMEM
> +	help
> +	  Support for thermal sensors on Broadcom bcm2835 SoCs.
> +
> ...
> +
> +static const struct of_device_id bcm2835_thermal_of_match_table[];
> +static int bcm2835_thermal_probe(struct platform_device *pdev)
> +{
> +	const struct of_device_id *match;
> +	struct thermal_zone_device *tz;
> +	struct bcm2835_thermal_data *data;
> +	struct resource *res;
> +	int err;
> +	u32 val;
> +	unsigned long rate;
> +
> +	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	match = of_match_device(bcm2835_thermal_of_match_table,
> +				&pdev->dev);
> +	if (!match)
> +		return -EINVAL;
> +	data->info = match->data;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	data->regs = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(data->regs)) {
> +		err = PTR_ERR(data->regs);
> +		dev_err(&pdev->dev, "Could not get registers: %d\n", err);
> +		return err;
> +	}
> +
> +	data->clk = devm_clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(data->clk)) {
> +		err = PTR_ERR(data->clk);
> +		if (err != -EPROBE_DEFER)
> +			dev_err(&pdev->dev, "Could not get clk: %d\n", err);
> +		return err;
> +	}
> +
> +	err = clk_prepare_enable(data->clk);
> +	if (err)
> +		return err;
> +
> +	rate = clk_get_rate(data->clk);
> +	if ((rate < 1920000) || (rate > 5000000))
> +		dev_warn(&pdev->dev,
> +			 "Clock %pCn running at %pCr Hz is outside of the recommended range: 1.92
> to 5MHz\n",
> +			 data->clk, data->clk);
> +
> +	/*
> +	 * right now the FW does set up the HW-block, so we are not
> +	 * touching the configuration registers.
> +	 * But if the HW is not enabled, then set it up
> +	 * using "sane" values used by the firmware right now.
> +	 */
> +	val = readl(data->regs + BCM2835_TS_TSENSCTL);
> +	if (!(val & BCM2835_TS_TSENSCTL_RSTB)) {
> +		/* the basic required flags */
> +		val = (BCM2835_TS_TSENSCTL_CTRL_DEFAULT <<
> +		       BCM2835_TS_TSENSCTL_CTRL_SHIFT) |
> +		      BCM2835_TS_TSENSCTL_REGULEN;
> +
> +		/*
> +		 * reset delay using the current firmware value of 14
> +		 * - units of time are unknown.
> +		 */

I've have open a thread in the Raspberry Pi Forum regards to this. If i get a
positive feedback then i will send an incremental patch.

Stefan

> +		val |= (14 << BCM2835_TS_TSENSCTL_RSTDELAY_SHIFT);
> +
> +		/*  trip_adc value from info */
> +		val |= bcm2835_thermal_temp2adc(data->info,
> +						data->info->trip_temp) <<
> +			BCM2835_TS_TSENSCTL_THOLD_SHIFT;
> +
> +		/* write the value back to the register as 2 steps */
> +		writel(val, data->regs + BCM2835_TS_TSENSCTL);
> +		val |= BCM2835_TS_TSENSCTL_RSTB;
> +		writel(val, data->regs + BCM2835_TS_TSENSCTL);
> +	}
> +

^ permalink raw reply

* [PATCH] arm64, numa: Add cpu_to_node() implementation.
From: David Daney @ 2016-09-20 17:53 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160920104348.GP25086@rric.localdomain>

On 09/20/2016 03:43 AM, Robert Richter wrote:
[...]
>
> Instead we need to make sure the set_*numa_node() functions are called
> earlier before secondary cpus are booted. My suggested change for that
> is this:
>
>
> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
> index d93d43352504..952365c2f100 100644
> --- a/arch/arm64/kernel/smp.c
> +++ b/arch/arm64/kernel/smp.c
> @@ -204,7 +204,6 @@ int __cpu_up(unsigned int cpu, struct task_struct *idle)
>   static void smp_store_cpu_info(unsigned int cpuid)
>   {
>   	store_cpu_topology(cpuid);
> -	numa_store_cpu_info(cpuid);
>   }
>
>   /*
> @@ -719,6 +718,7 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
>   			continue;
>
>   		set_cpu_present(cpu, true);
> +		numa_store_cpu_info(cpu);
>   	}
>   }
>
>
> I have tested the code and it properly sets up all per-cpu workqueues.
>

Thanks Robert,

I have tested a slightly modified version of that, and it seems to also 
fix the problem for me.

I will submit a cleaned up patch.

David Daney



> Unfortunately either your nor my code does fix the BUG_ON() I see with
> the numa kernel:
>
>   kernel BUG at mm/page_alloc.c:1848!
>
> See below for the core dump. It looks like this happens due to moving
> a mem block where first and last page are mapped to different numa
> nodes, thus, triggering the BUG_ON().
>
> Continuing with my investigations...
>
> -Robert
>
>
>
> [    9.674272] ------------[ cut here ]------------
> [    9.678881] kernel BUG at mm/page_alloc.c:1848!
> [    9.683406] Internal error: Oops - BUG: 0 [#1] SMP
> [    9.688190] Modules linked in:
> [    9.691247] CPU: 77 PID: 1 Comm: swapper/0 Tainted: G        W       4.8.0-rc5.vanilla5-00030-ga2b86cb3ce72 #38
> [    9.701322] Hardware name: www.cavium.com ThunderX CRB-2S/ThunderX CRB-2S, BIOS 0.3 Aug 24 2016
> [    9.710008] task: ffff800fe4561400 task.stack: ffff800ffbe0c000
> [    9.715939] PC is at move_freepages+0x160/0x168
> [    9.720460] LR is at move_freepages+0x160/0x168
> [    9.724979] pc : [<ffff0000081ec7d0>] lr : [<ffff0000081ec7d0>] pstate: 600000c5
> [    9.732362] sp : ffff800ffbe0f510
> [    9.735666] x29: ffff800ffbe0f510 x28: ffff7fe043f80020
> [    9.740975] x27: ffff7fe043f80000 x26: 000000000000000c
> [    9.746283] x25: 000000000000000c x24: ffff810ffffaf0e0
> [    9.751591] x23: 0000000000000001 x22: 0000000000000000
> [    9.756898] x21: ffff7fe043ffffc0 x20: ffff810ffffaeb00
> [    9.762206] x19: ffff7fe043f80000 x18: 0000000000000010
> [    9.767513] x17: 0000000000000000 x16: 0000000100000000
> [    9.772821] x15: ffff000088f03f37 x14: 6e2c303d64696e2c
> [    9.778128] x13: 3038336566666666 x12: 6630303866666666
> [    9.783436] x11: 3d656e6f7a203a64 x10: 0000000000000536
> [    9.788744] x9 : 0000000000000060 x8 : 3030626561666666
> [    9.794051] x7 : 6630313866666666 x6 : ffff000008f03f97
> [    9.799359] x5 : 0000000000000006 x4 : 000000000000000c
> [    9.804667] x3 : 0000000000010000 x2 : 0000000000010000
> [    9.809975] x1 : ffff000008da7be0 x0 : 0000000000000050
>
> [   10.517213] Call trace:
> [   10.519651] Exception stack(0xffff800ffbe0f340 to 0xffff800ffbe0f470)
> [   10.526081] f340: ffff7fe043f80000 0001000000000000 ffff800ffbe0f510 ffff0000081ec7d0
> [   10.533900] f360: ffff000008f03988 0000000008da7bc8 ffff800ffbe0f410 ffff0000081275fc
> [   10.541718] f380: ffff800ffbe0f470 ffff000008ac5a00 ffff7fe043ffffc0 0000000000000000
> [   10.549536] f3a0: 0000000000000001 ffff810ffffaf0e0 000000000000000c 000000000000000c
> [   10.557355] f3c0: ffff7fe043f80000 ffff7fe043f80020 0000000000000030 0000000000000000
> [   10.565173] f3e0: 0000000000000050 ffff000008da7be0 0000000000010000 0000000000010000
> [   10.572991] f400: 000000000000000c 0000000000000006 ffff000008f03f97 6630313866666666
> [   10.580809] f420: 3030626561666666 0000000000000060 0000000000000536 3d656e6f7a203a64
> [   10.588628] f440: 6630303866666666 3038336566666666 6e2c303d64696e2c ffff000088f03f37
> [   10.596446] f460: 0000000100000000 0000000000000000
> [   10.601316] [<ffff0000081ec7d0>] move_freepages+0x160/0x168
> [   10.606879] [<ffff0000081ec880>] move_freepages_block+0xa8/0xb8
> [   10.612788] [<ffff0000081ecf80>] __rmqueue+0x610/0x670
> [   10.617918] [<ffff0000081ee2e4>] get_page_from_freelist+0x3cc/0xb40
> [   10.624174] [<ffff0000081ef05c>] __alloc_pages_nodemask+0x12c/0xd40
> [   10.630438] [<ffff000008244cd0>] alloc_page_interleave+0x60/0xb0
> [   10.636434] [<ffff000008245398>] alloc_pages_current+0x108/0x168
> [   10.642430] [<ffff0000081e49ac>] __page_cache_alloc+0x104/0x140
> [   10.648339] [<ffff0000081e4b00>] pagecache_get_page+0x118/0x2e8
> [   10.654248] [<ffff0000081e4d18>] grab_cache_page_write_begin+0x48/0x68
> [   10.660769] [<ffff000008298c08>] simple_write_begin+0x40/0x150
> [   10.666591] [<ffff0000081e47c0>] generic_perform_write+0xb8/0x1a0
> [   10.672674] [<ffff0000081e6228>] __generic_file_write_iter+0x178/0x1c8
> [   10.679191] [<ffff0000081e6344>] generic_file_write_iter+0xcc/0x1c8
> [   10.685448] [<ffff00000826d12c>] __vfs_write+0xcc/0x140
> [   10.690663] [<ffff00000826de08>] vfs_write+0xa8/0x1c0
> [   10.695704] [<ffff00000826ee34>] SyS_write+0x54/0xb0
> [   10.700666] [<ffff000008bf2008>] xwrite+0x34/0x7c
> [   10.705359] [<ffff000008bf20ec>] do_copy+0x9c/0xf4
> [   10.710140] [<ffff000008bf1dc4>] write_buffer+0x34/0x50
> [   10.715354] [<ffff000008bf1e28>] flush_buffer+0x48/0xb8
> [   10.720579] [<ffff000008c1faa0>] __gunzip+0x27c/0x324
> [   10.725620] [<ffff000008c1fb60>] gunzip+0x18/0x20
> [   10.730314] [<ffff000008bf26dc>] unpack_to_rootfs+0x168/0x280
> [   10.736049] [<ffff000008bf2864>] populate_rootfs+0x70/0x138
> [   10.741615] [<ffff000008082ff4>] do_one_initcall+0x44/0x138
> [   10.747179] [<ffff000008bf0d0c>] kernel_init_freeable+0x1ac/0x24c
> [   10.753267] [<ffff000008859f78>] kernel_init+0x20/0xf8
> [   10.758395] [<ffff000008082b80>] ret_from_fork+0x10/0x50
> [   10.763698] Code: 17fffff2 b00046c0 91280000 97ffd47d (d4210000)
> [   10.769834] ---[ end trace 972d622f64fd69c0 ]---
>

^ permalink raw reply

* [PATCH 3/4] ARM: tegra: nyan-big: Include compatible revisions for proper detection
From: Paul Kocialkowski @ 2016-09-20 17:53 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <a9485a88-55a5-9dd1-1a1b-a6c0a78ff476@nvidia.com>

Le mardi 20 septembre 2016 ? 18:41 +0100, Jon Hunter a ?crit?:
> On 28/08/16 18:32, Paul Kocialkowski wrote:
> > 
> > Depthcharge (the payload used with cros devices) will attempt to detect
> > boards using their revision. This includes all the known revisions for
> > the nyan-big board so that the dtb can be selected preferably.
> 
> May be I am missing something here, but for the mainline there is only
> one dtb available and so why is this needed for the mainline?

There is indeed a single dts in mainline, but depthcharge will use the revision
to match the compatible string (e.g. it will look for google,nyan-big-rev5, not
google,nyan-big), so we need to list them all in that single dts. Otherwise,
depthcharge will fall back to the default config, which may or may not be
suitable for nyan.

-- 
Paul Kocialkowski, developer of low-level free software for embedded devices

Website: https://www.paulk.fr/
Coding blog: https://code.paulk.fr/
Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160920/30d1d305/attachment-0001.sig>

^ permalink raw reply

* [PATCH 3/4] ARM: tegra: nyan-big: Include compatible revisions for proper detection
From: Jon Hunter @ 2016-09-20 17:56 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474394004.1215.2.camel@paulk.fr>


On 20/09/16 18:53, Paul Kocialkowski wrote:
> * PGP Signed by an unknown key
> 
> Le mardi 20 septembre 2016 ? 18:41 +0100, Jon Hunter a ?crit :
>> On 28/08/16 18:32, Paul Kocialkowski wrote:
>>>
>>> Depthcharge (the payload used with cros devices) will attempt to detect
>>> boards using their revision. This includes all the known revisions for
>>> the nyan-big board so that the dtb can be selected preferably.
>>
>> May be I am missing something here, but for the mainline there is only
>> one dtb available and so why is this needed for the mainline?
> 
> There is indeed a single dts in mainline, but depthcharge will use the revision
> to match the compatible string (e.g. it will look for google,nyan-big-rev5, not
> google,nyan-big), so we need to list them all in that single dts. Otherwise,
> depthcharge will fall back to the default config, which may or may not be
> suitable for nyan.

Is tegra124-nyan-big.dtb not the default?

Jon

-- 
nvpublic

^ permalink raw reply

* [PATCH 3/4] ARM: tegra: nyan-big: Include compatible revisions for proper detection
From: Paul Kocialkowski @ 2016-09-20 18:02 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <d05b8e5a-adf7-9057-b42a-0da1fde84953@nvidia.com>

Le mardi 20 septembre 2016 ? 18:56 +0100, Jon Hunter a ?crit?:
> On 20/09/16 18:53, Paul Kocialkowski wrote:
> > 
> > * PGP Signed by an unknown key
> > 
> > Le mardi 20 septembre 2016 ? 18:41 +0100, Jon Hunter a ?crit :
> > > 
> > > On 28/08/16 18:32, Paul Kocialkowski wrote:
> > > > 
> > > > 
> > > > Depthcharge (the payload used with cros devices) will attempt to detect
> > > > boards using their revision. This includes all the known revisions for
> > > > the nyan-big board so that the dtb can be selected preferably.
> > > 
> > > May be I am missing something here, but for the mainline there is only
> > > one dtb available and so why is this needed for the mainline?
> > 
> > There is indeed a single dts in mainline, but depthcharge will use the
> > revision
> > to match the compatible string (e.g. it will look for google,nyan-big-rev5,
> > not
> > google,nyan-big), so we need to list them all in that single dts. Otherwise,
> > depthcharge will fall back to the default config, which may or may not be
> > suitable for nyan.
> 
> Is tegra124-nyan-big.dtb not the default?

You can't expect that to always be the case. The image format allows many
different dts to be provided, so I could easily build with multi_v7_defconfig
and have various dts for various devices in the same image, and just select a
random one as default.

Here, default is really a fallback, the right one is expected to be detected by
this mechanism. And it really doesn't hurt to provide that information for
proper detection.

Note that this is done with many other cros devices in mainline (such as rk3288
veyrons).

-- 
Paul Kocialkowski, developer of low-level free software for embedded devices

Website: https://www.paulk.fr/
Coding blog: https://code.paulk.fr/
Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160920/82f5dcd1/attachment.sig>

^ permalink raw reply

* [PATCH 2/4] ARM: tegra: nyan: Use external control for bq24735 charger
From: Paul Kocialkowski @ 2016-09-20 18:02 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <00697eef-0b36-1ff5-9c7a-43c1b53b52bc@nvidia.com>

Le mardi 20 septembre 2016 ? 18:40 +0100, Jon Hunter a ?crit :
> On 28/08/16 18:32, Paul Kocialkowski wrote:
> >?
> > Nyan boards come with an embedded controller that controls when to
> > enable and disable the charge. Thus, it should not be left up to the
> > kernel to handle that.
> >?
> > Using the ti,external-control property allows specifying this use-case.
>?
> So the bq24735 is populated under the EC's 'i2c-tunnel' property which
> is there to specifically interface it's child devices to the host. So I
> am a bit confused why this is expose to the host if it should not be used?

Well, it needs to access the information in the read-only registers provided by
the chip, which is allowed by the setup in place that you described.

However, the EC has its internal state machine that decides when to start
charging, etc and so should be the only one to write registers, to avoid
conflicts.

> Again you may right and I did find the original series [0] for this
> which specifically references the Acer Chromebook that needs this.
> However, I am not sure why this was never populated? Is there any other
> history here?

I am also confused about why it wasn't applied earlier. However, the cros kernel
is using the very same scheme.

> What is the actual problem you see without making this change?

There is a risk of conflict (even though it's probably not that significant),
given the low variety of possible cases here. The idea is simply to say that the
EC is in charge and to let it do its job without interfering.

> The original series states ...
>?
> "On Acer Chromebook 13 (CB5-311) this module fails to load if the
> charger is not inserted, and will error when it is removed."

I'm confused about that comment. At this point (and with this patch), it works
normally.

> Cheers
> Jon
>?
> [0] http://marc.info/?l=linux-pm&m=145447948705686&w=2

Cheers,

--?
Paul Kocialkowski, developer of low-level free software for embedded devices

Website: https://www.paulk.fr/
Coding blog: https://code.paulk.fr/
Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160920/19d6ecea/attachment.sig>

^ permalink raw reply

* [PATCH 1/4] ARM: tegra: nyan: Use proper IRQ type definitions
From: Paul Kocialkowski @ 2016-09-20 18:14 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <ef69b6d2-ce6b-ff76-5c04-ab2176eb6ecc@nvidia.com>

Le mardi 20 septembre 2016 ? 18:15 +0100, Jon Hunter a ?crit?:
> On 28/08/16 18:32, Paul Kocialkowski wrote:
> > 
> > This switches a few interrupt definitions that were using
> > GPIO_ACTIVE_HIGH as IRQ type, which is invalid.
> 
> May be you are right, but this does not describe why this is invalid.
> Can you elaborate?

GPIO_ACTIVE_HIGH is simply not the right kind of define to use in the
"interrupts" devicetree property. Values provided there are understood as
IRQ_TYPE_ defines.

Also, a similar commit[0] was pushed to the cros kernel tree.

Cheers,

[0]:?https://chromium.googlesource.com/chromiumos/third_party/kernel/+/f77288938d571b48c9736ac3703cea370c002434

> > Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> > ---
> > ?arch/arm/boot/dts/tegra124-nyan.dtsi | 4 ++--
> > ?1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/tegra124-nyan.dtsi
> > b/arch/arm/boot/dts/tegra124-nyan.dtsi
> > index 271505e..30a77ec 100644
> > --- a/arch/arm/boot/dts/tegra124-nyan.dtsi
> > +++ b/arch/arm/boot/dts/tegra124-nyan.dtsi
> > @@ -59,7 +59,7 @@
> > ?			compatible = "maxim,max98090";
> > ?			reg = <0x10>;
> > ?			interrupt-parent = <&gpio>;
> > -			interrupts = <TEGRA_GPIO(H, 4) GPIO_ACTIVE_HIGH>;
> > +			interrupts = <TEGRA_GPIO(H, 4)
> > IRQ_TYPE_EDGE_FALLING>;
> 
> If this in invalid then the DT binding doc for the max98090 should be
> updated as well.
> 
> > 
> > ?		};
> > ?
> > ?		temperature-sensor at 4c {
> > @@ -325,7 +325,7 @@
> > ?					reg = <0x9>;
> > ?					interrupt-parent = <&gpio>;
> > ?					interrupts = <TEGRA_GPIO(J, 0)
> > -							GPIO_ACTIVE_HIGH>;
> > +							IRQ_TYPE_EDGE_BOTH>
> > ;
> > ?					ti,ac-detect-gpios = <&gpio
> > ?							TEGRA_GPIO(J, 0)
> > ?							GPIO_ACTIVE_HIGH>;
> > 
> 
> Cheers
> Jon
> 
-- 
Paul Kocialkowski, developer of low-level free software for embedded devices

Website: https://www.paulk.fr/
Coding blog: https://code.paulk.fr/
Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160920/cc2229e4/attachment.sig>

^ permalink raw reply

* [PATCH] ARM: tegra: nyan: Enable GPU node and related supply
From: Paul Kocialkowski @ 2016-09-20 18:17 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <f2489bf5-bb08-11f5-1b53-7a1568f20b6b@nvidia.com>

Le mardi 20 septembre 2016 ? 13:24 +0100, Jon Hunter a ?crit?:
> On 18/09/16 15:13, Paul Kocialkowski wrote:
> > 
> > This enables the GPU node for tegra124 nyan boards, which is required to
> > get graphics acceleration with nouveau on these devices.
> > 
> > Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> > ---
> > ?arch/arm/boot/dts/tegra124-nyan.dtsi | 8 +++++++-
> > ?1 file changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm/boot/dts/tegra124-nyan.dtsi
> > b/arch/arm/boot/dts/tegra124-nyan.dtsi
> > index dab9509..225ca77 100644
> > --- a/arch/arm/boot/dts/tegra124-nyan.dtsi
> > +++ b/arch/arm/boot/dts/tegra124-nyan.dtsi
> > @@ -42,6 +42,12 @@
> > ?		};
> > ?	};
> > ?
> > +	gpu at 0,57000000 {
> > +		status = "okay";
> > +
> > +		vdd-supply = <&vdd_gpu>;
> > +	};
> > +
> > ?	serial at 70006000 {
> > ?		/* Debug connector on the bottom of the board near SD card.
> > */
> > ?		status = "okay";
> > @@ -214,7 +220,7 @@
> > ?					regulator-always-on;
> > ?				};
> > ?
> > -				sd6 {
> > +				vdd_gpu: sd6 {
> > ?					regulator-name = "+VDD_GPU_AP";
> > ?					regulator-min-microvolt = <650000>;
> > ?					regulator-max-microvolt =
> > <1200000>;
> > 
> 
> Looks good to me. I see the following error when booting but looking at the
> code appears to be benign. Thierry, Alex, is this normal/okay?

I have the same messages and asked?Alexandre about them the other day. He told
me that it looks normal.

> [????5.715181] nouveau 57000000.gpu: NVIDIA GK20A
> (0ea000a1)????????????????????????????????????????????????????????????????????
> ????????????????????????
> [????5.720625] nouveau 57000000.gpu: imem: using
> IOMMU?????????????????????????????????????????????????????????????????????????
> ?????????????????????????
> [????5.803694] nouveau 57000000.gpu: DRM: VRAM: 0
> MiB???????????????????????????????????????????????????????????????????????????
> ????????????????????????
> [????5.808501] nouveau 57000000.gpu: DRM: GART: 1048576
> MiB???????????????????????????????????????????????????????????????????????????
> ??????????????????
> [????5.816000] nouveau 57000000.gpu: DRM: failed to create ce channel,
> -22???????????????????????????????????????????????????????????????????????????
> ???
> [????5.924140] nouveau 57000000.gpu: DRM: MM: using GRCE for buffer copies??
> 
> Cheers
> Jon
> 
-- 
Paul Kocialkowski, developer of low-level free software for embedded devices

Website: https://www.paulk.fr/
Coding blog: https://code.paulk.fr/
Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160920/8b5aba3b/attachment-0001.sig>

^ permalink raw reply

* [PATCH] ARM: dts: Add power button support for igepv5
From: Tony Lindgren @ 2016-09-20 18:39 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160920121521.GA32519@localhost.localdomain>

* Ladislav Michl <ladis@linux-mips.org> [160920 05:15]:
> On Mon, Sep 19, 2016 at 04:08:26PM -0700, Tony Lindgren wrote:
> > * Pau Pajuel <ppajuel@gmail.com> [160919 01:13]:
> > > Hello Tony,
> > > 
> > > 2016-09-14 3:26 GMT+02:00 Javier Martinez Canillas <javier@osg.samsung.com>:
> > > > Hello Tony,
> > > >
> > > > On 09/09/2016 05:07 PM, Tony Lindgren wrote:
> > > >> Add power button support for igepv5.
> > > >>
> > > >> Cc: Agust? Fontquerni i Gorchs <afontquerni@iseebcn.com>
> > > >> Cc: Enric Balletbo Serra <eballetbo@gmail.com>
> > > >> Cc: Javier Martinez Canillas <javier@osg.samsung.com>
> > > >> Cc: Pau Pajuel <ppajuel@gmail.com>
> > > >> Signed-off-by: Tony Lindgren <tony@atomide.com>
> > > >> ---
> > > >
> > > > I don't have a schematics for this board, but the patch looks good to me.
> > > Patch has an errata. Power button is GPIO_ACTIVE_LOW instead GPIO_ACTIVE_HIGH.
> > 
> > Oops sorry about that, here's an incremental fix.
> 
> And here's another nitpick ;-)

Heh OK thanks, applying both into omap-for-v4.9/dt-v2.

Regards,

Tony

^ permalink raw reply

* [PATCH] arm64: Call numa_store_cpu_info() earlier.
From: David Daney @ 2016-09-20 18:46 UTC (permalink / raw)
  To: linux-arm-kernel

From: David Daney <david.daney@cavium.com>

The wq_numa_init() function makes a private CPU to node map by calling
cpu_to_node() early in the boot process, before the non-boot CPUs are
brought online.  Since the default implementation of cpu_to_node()
returns zero for CPUs that have never been brought online, the
workqueue system's view is that *all* CPUs are on node zero.

When the unbound workqueue for a non-zero node is created, the
tsk_cpus_allowed() for the worker threads is the empty set because
there are, in the view of the workqueue system, no CPUs on non-zero
nodes.  The code in try_to_wake_up() using this empty cpumask ends up
using the cpumask empty set value of NR_CPUS as an index into the
per-CPU area pointer array, and gets garbage as it is one past the end
of the array.  This results in:

[    0.881970] Unable to handle kernel paging request at virtual address fffffb1008b926a4
[    1.970095] pgd = fffffc00094b0000
[    1.973530] [fffffb1008b926a4] *pgd=0000000000000000, *pud=0000000000000000, *pmd=0000000000000000
[    1.982610] Internal error: Oops: 96000004 [#1] SMP
[    1.987541] Modules linked in:
[    1.990631] CPU: 48 PID: 295 Comm: cpuhp/48 Tainted: G        W       4.8.0-rc6-preempt-vol+ #9
[    1.999435] Hardware name: Cavium ThunderX CN88XX board (DT)
[    2.005159] task: fffffe0fe89cc300 task.stack: fffffe0fe8b8c000
[    2.011158] PC is at try_to_wake_up+0x194/0x34c
[    2.015737] LR is at try_to_wake_up+0x150/0x34c
[    2.020318] pc : [<fffffc00080e7468>] lr : [<fffffc00080e7424>] pstate: 600000c5
[    2.027803] sp : fffffe0fe8b8fb10
[    2.031149] x29: fffffe0fe8b8fb10 x28: 0000000000000000
[    2.036522] x27: fffffc0008c63bc8 x26: 0000000000001000
[    2.041896] x25: fffffc0008c63c80 x24: fffffc0008bfb200
[    2.047270] x23: 00000000000000c0 x22: 0000000000000004
[    2.052642] x21: fffffe0fe89d25bc x20: 0000000000001000
[    2.058014] x19: fffffe0fe89d1d00 x18: 0000000000000000
[    2.063386] x17: 0000000000000000 x16: 0000000000000000
[    2.068760] x15: 0000000000000018 x14: 0000000000000000
[    2.074133] x13: 0000000000000000 x12: 0000000000000000
[    2.079505] x11: 0000000000000000 x10: 0000000000000000
[    2.084879] x9 : 0000000000000000 x8 : 0000000000000000
[    2.090251] x7 : 0000000000000040 x6 : 0000000000000000
[    2.095621] x5 : ffffffffffffffff x4 : 0000000000000000
[    2.100991] x3 : 0000000000000000 x2 : 0000000000000000
[    2.106364] x1 : fffffc0008be4c24 x0 : ffffff0ffffada80
[    2.111737]
[    2.113236] Process cpuhp/48 (pid: 295, stack limit = 0xfffffe0fe8b8c020)
[    2.120102] Stack: (0xfffffe0fe8b8fb10 to 0xfffffe0fe8b90000)
[    2.125914] fb00:                                   fffffe0fe8b8fb80 fffffc00080e7648
.
.
.
[    2.442859] Call trace:
[    2.445327] Exception stack(0xfffffe0fe8b8f940 to 0xfffffe0fe8b8fa70)
[    2.451843] f940: fffffe0fe89d1d00 0000040000000000 fffffe0fe8b8fb10 fffffc00080e7468
[    2.459767] f960: fffffe0fe8b8f980 fffffc00080e4958 ffffff0ff91ab200 fffffc00080e4b64
[    2.467690] f980: fffffe0fe8b8f9d0 fffffc00080e515c fffffe0fe8b8fa80 0000000000000000
[    2.475614] f9a0: fffffe0fe8b8f9d0 fffffc00080e58e4 fffffe0fe8b8fa80 0000000000000000
[    2.483540] f9c0: fffffe0fe8d10000 0000000000000040 fffffe0fe8b8fa50 fffffc00080e5ac4
[    2.491465] f9e0: ffffff0ffffada80 fffffc0008be4c24 0000000000000000 0000000000000000
[    2.499387] fa00: 0000000000000000 ffffffffffffffff 0000000000000000 0000000000000040
[    2.507309] fa20: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[    2.515233] fa40: 0000000000000000 0000000000000000 0000000000000000 0000000000000018
[    2.523156] fa60: 0000000000000000 0000000000000000
[    2.528089] [<fffffc00080e7468>] try_to_wake_up+0x194/0x34c
[    2.533723] [<fffffc00080e7648>] wake_up_process+0x28/0x34
[    2.539275] [<fffffc00080d3764>] create_worker+0x110/0x19c
[    2.544824] [<fffffc00080d69dc>] alloc_unbound_pwq+0x3cc/0x4b0
[    2.550724] [<fffffc00080d6bcc>] wq_update_unbound_numa+0x10c/0x1e4
[    2.557066] [<fffffc00080d7d78>] workqueue_online_cpu+0x220/0x28c
[    2.563234] [<fffffc00080bd288>] cpuhp_invoke_callback+0x6c/0x168
[    2.569398] [<fffffc00080bdf74>] cpuhp_up_callbacks+0x44/0xe4
[    2.575210] [<fffffc00080be194>] cpuhp_thread_fun+0x13c/0x148
[    2.581027] [<fffffc00080dfbac>] smpboot_thread_fn+0x19c/0x1a8
[    2.586929] [<fffffc00080dbd64>] kthread+0xdc/0xf0
[    2.591776] [<fffffc0008083380>] ret_from_fork+0x10/0x50
[    2.597147] Code: b00057e1 91304021 91005021 b8626822 (b8606821)
[    2.603464] ---[ end trace 58c0cd36b88802bc ]---
[    2.608138] Kernel panic - not syncing: Fatal exception

Fix by moving call to numa_store_cpu_info() for all CPUs into
smp_prepare_cpus(), which happens before wq_numa_init().  Since
smp_store_cpu_info() now contains only a single function call,
simplify by removing the function and out-lining its contents.

Suggested-by: Robert Richter <rric@kernel.org>
fixes: 1a2db300348b ("arm64, numa: Add NUMA support for arm64 platforms.")
Cc: <stable@vger.kernel.org> # 4.7.x-
Signed-off-by: David Daney <david.daney@cavium.com>
---
 arch/arm64/kernel/smp.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index d93d433..3ff173e 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -201,12 +201,6 @@ int __cpu_up(unsigned int cpu, struct task_struct *idle)
 	return ret;
 }
 
-static void smp_store_cpu_info(unsigned int cpuid)
-{
-	store_cpu_topology(cpuid);
-	numa_store_cpu_info(cpuid);
-}
-
 /*
  * This is the secondary CPU boot entry.  We're using this CPUs
  * idle thread stack, but a set of temporary page tables.
@@ -254,7 +248,7 @@ asmlinkage void secondary_start_kernel(void)
 	 */
 	notify_cpu_starting(cpu);
 
-	smp_store_cpu_info(cpu);
+	store_cpu_topology(cpu);
 
 	/*
 	 * OK, now it's safe to let the boot CPU continue.  Wait for
@@ -689,10 +683,13 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
 {
 	int err;
 	unsigned int cpu;
+	unsigned int this_cpu;
 
 	init_cpu_topology();
 
-	smp_store_cpu_info(smp_processor_id());
+	this_cpu = smp_processor_id();
+	store_cpu_topology(this_cpu);
+	numa_store_cpu_info(this_cpu);
 
 	/*
 	 * If UP is mandated by "nosmp" (which implies "maxcpus=0"), don't set
@@ -719,6 +716,7 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
 			continue;
 
 		set_cpu_present(cpu, true);
+		numa_store_cpu_info(cpu);
 	}
 }
 
-- 
1.8.3.1

^ permalink raw reply related


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