Re: [PATCH v2 0/3] mach-omap2: handle autoidle denial

[Stephen Boyd]

Quoting Tero Kristo (2018-11-29 23:37:35)
> On 30/11/2018 02:26, Stephen Boyd wrote:
> > Quoting Andreas Kemnade (2018-11-10 12:31:12)
> >> On the gta04 with a dm3730 omap_hdq does not work properly when the
> >> device enters lower power states. Idling uart1 and 2 is enough
> >> to show up that problem, if there are no other things enabled.
> >> Further research reveals that hdq iclk must not be turned off during
> >> transfers, also according to the TRM. That fact is also correctly described
> >> in the flags but the code to handle that is incomplete.
> >>
> >> To handle multiple users of a single ick, autoidle is disabled
> >> when a user of that ick requires that (has the OCPIF_SWSUP_IDLE))
> >>
> >> Changes since v1:
> >> - uses spinlocks instead of mutexes
> >> - invert counter logic
> >> - check whether clock type is basic
> >>
> > 
> > I'm expecting someone like Tero or Tony to review this.
> > 
> 
> Rest of it looks fine to me, except for the discussion under the 
> CLK_IS_BASIC flag, which might trigger a bigger rework of the code.
> 

Is that a Reviewed-by tag?

Re: [PATCH v2 0/3] mach-omap2: handle autoidle denial

[Stephen Boyd]

Quoting Tero Kristo (2018-11-29 23:37:35)
> On 30/11/2018 02:26, Stephen Boyd wrote:
> > Quoting Andreas Kemnade (2018-11-10 12:31:12)
> >> On the gta04 with a dm3730 omap_hdq does not work properly when the
> >> device enters lower power states. Idling uart1 and 2 is enough
> >> to show up that problem, if there are no other things enabled.
> >> Further research reveals that hdq iclk must not be turned off during
> >> transfers, also according to the TRM. That fact is also correctly described
> >> in the flags but the code to handle that is incomplete.
> >>
> >> To handle multiple users of a single ick, autoidle is disabled
> >> when a user of that ick requires that (has the OCPIF_SWSUP_IDLE))
> >>
> >> Changes since v1:
> >> - uses spinlocks instead of mutexes
> >> - invert counter logic
> >> - check whether clock type is basic
> >>
> > 
> > I'm expecting someone like Tero or Tony to review this.
> > 
> 
> Rest of it looks fine to me, except for the discussion under the 
> CLK_IS_BASIC flag, which might trigger a bigger rework of the code.
> 

Is that a Reviewed-by tag?

Re: [PATCH v2 2/3] clk: ti: check clock type before doing autoidle ops

[Stephen Boyd]

Quoting Tero Kristo (2018-11-29 23:35:35)
> On 30/11/2018 09:20, Stephen Boyd wrote:
> > Quoting Andreas Kemnade (2018-11-29 22:15:34)
> >> Hi Stephen,
> >>
> >> On Thu, 29 Nov 2018 16:25:05 -0800
> >> Stephen Boyd  wrote:
> >>
> >>> Quoting Andreas Kemnade (2018-11-10 12:31:14)
>  Code might use autoidle api with clocks not being omap2 clocks,
>  so check if clock type is not basic
> 
>  Signed-off-by: Andreas Kemnade 
>  ---
>  New in v2
>  ---
>    drivers/clk/ti/autoidle.c | 12 ++--
>    1 file changed, 10 insertions(+), 2 deletions(-)
> 
>  diff --git a/drivers/clk/ti/autoidle.c b/drivers/clk/ti/autoidle.c
>  index 161f67850393..5bdae5552d38 100644
>  --- a/drivers/clk/ti/autoidle.c
>  +++ b/drivers/clk/ti/autoidle.c
>  @@ -54,8 +54,12 @@ static DEFINE_SPINLOCK(autoidle_spinlock);
>    int omap2_clk_deny_idle(struct clk *clk)
>    {
>   struct clk_hw_omap *c;
>  +   struct clk_hw *hw = __clk_get_hw(clk);
>    
>  -   c = to_clk_hw_omap(__clk_get_hw(clk));
>  +   if (clk_hw_get_flags(hw) & CLK_IS_BASIC)
> >>>
> >>> Please try to avoid using CLK_IS_BASIC if at all possible. Can you?
> >>> Maybe add some flag in clk_hw_omap() instead?
> >>>
> >> hmm, Tero suggested that.
> >> But to check flags in clk_hw_omap I first need to know that there is a
> >> clk_hw_omap behind clk_hw. And for that I either need to check flags in
> >> clk_hw or do more changes in the omap_hwmod code.
> > 
> > Can you do it? The omap code is the only user of CLK_IS_BASIC. All the
> > other users are marking clks with this but there is no reason to do so.
> > I'll go make another pass over the tree and nuke those ones from orbit.
> 
> The reason for using this flag is because OMAP uses two clock types 
> around, the basic clocks like fixed-factor-clock/fixed-clock, and then 
> all the omap derivatives, which can be cast to clk_hw_omap. If we want 
> to avoid usage of CLK_IS_BASIC, we need to copy paste the remaining 
> basic code under drivers/clk/ti/ and convert them to use clk_hw_omap as 
> internal datatype. Is this preferred?
> 

No that is not preferred. Can the omap2_clk_deny_idle() function be
integrated closer into the clk framework in some way that allows it to
be part of the clk_ops structure? And then have that take a clk_hw
structure instead of a struct clk? I haven't looked at this in any
detail whatsoever so I may be way off right now.

Re: [PATCH v2 2/3] clk: ti: check clock type before doing autoidle ops

[Stephen Boyd]

Quoting Tero Kristo (2018-11-29 23:35:35)
> On 30/11/2018 09:20, Stephen Boyd wrote:
> > Quoting Andreas Kemnade (2018-11-29 22:15:34)
> >> Hi Stephen,
> >>
> >> On Thu, 29 Nov 2018 16:25:05 -0800
> >> Stephen Boyd  wrote:
> >>
> >>> Quoting Andreas Kemnade (2018-11-10 12:31:14)
>  Code might use autoidle api with clocks not being omap2 clocks,
>  so check if clock type is not basic
> 
>  Signed-off-by: Andreas Kemnade 
>  ---
>  New in v2
>  ---
>    drivers/clk/ti/autoidle.c | 12 ++--
>    1 file changed, 10 insertions(+), 2 deletions(-)
> 
>  diff --git a/drivers/clk/ti/autoidle.c b/drivers/clk/ti/autoidle.c
>  index 161f67850393..5bdae5552d38 100644
>  --- a/drivers/clk/ti/autoidle.c
>  +++ b/drivers/clk/ti/autoidle.c
>  @@ -54,8 +54,12 @@ static DEFINE_SPINLOCK(autoidle_spinlock);
>    int omap2_clk_deny_idle(struct clk *clk)
>    {
>   struct clk_hw_omap *c;
>  +   struct clk_hw *hw = __clk_get_hw(clk);
>    
>  -   c = to_clk_hw_omap(__clk_get_hw(clk));
>  +   if (clk_hw_get_flags(hw) & CLK_IS_BASIC)
> >>>
> >>> Please try to avoid using CLK_IS_BASIC if at all possible. Can you?
> >>> Maybe add some flag in clk_hw_omap() instead?
> >>>
> >> hmm, Tero suggested that.
> >> But to check flags in clk_hw_omap I first need to know that there is a
> >> clk_hw_omap behind clk_hw. And for that I either need to check flags in
> >> clk_hw or do more changes in the omap_hwmod code.
> > 
> > Can you do it? The omap code is the only user of CLK_IS_BASIC. All the
> > other users are marking clks with this but there is no reason to do so.
> > I'll go make another pass over the tree and nuke those ones from orbit.
> 
> The reason for using this flag is because OMAP uses two clock types 
> around, the basic clocks like fixed-factor-clock/fixed-clock, and then 
> all the omap derivatives, which can be cast to clk_hw_omap. If we want 
> to avoid usage of CLK_IS_BASIC, we need to copy paste the remaining 
> basic code under drivers/clk/ti/ and convert them to use clk_hw_omap as 
> internal datatype. Is this preferred?
> 

No that is not preferred. Can the omap2_clk_deny_idle() function be
integrated closer into the clk framework in some way that allows it to
be part of the clk_ops structure? And then have that take a clk_hw
structure instead of a struct clk? I haven't looked at this in any
detail whatsoever so I may be way off right now.

Re: [PATCH V2] clk: zynq: do not allow kmalloc failure

[Nicholas Mc Guire]

On Thu, Nov 29, 2018 at 03:45:23PM -0800, Stephen Boyd wrote:
> Quoting Nicholas Mc Guire (2018-11-21 04:28:30)
> > The kmalloc here is small (< 16 bytes) and occurs during initialization
> > during system startup here (can not be built as module) thus if this
> > kmalloc failed it is an indication of something more serious going on
> > and it is fine to hang the system here rather than cause some harder
> > to understand error by dereferencing NULL.
> > 
> > Explicitly checking would not make that much sense here as the only
> > possible reaction would be would BUG() here anyway.
> > 
> > Signed-off-by: Nicholas Mc Guire 
> > Fixes: 0ee52b157b8e ("clk: zynq: Add clock controller driver")
> > Acked-by: Michal Simek 
> > ---
> 
> Nak. We don't have any __GFP_NOFAIL in drivers/clk and I don't see a
> reason why we would want it here either. Just handle the failure, or
> don't care if this is so critical to system boot.
>
It was not motivated by the criticality but by the low probability
and cluttering the code for this case did not seem good to me.
Effectively handling it here means BUG() - so more or less
the same result that hanging it on __GFP_NOFAIL if allocation
was not possible would cause.

Not clear what the objection to __GFP_NOFAIL here is - my understanding
was that it is intended precisely for cases like this - but
I´ll send a V2 handling it with BUG_ON(!clk_name) if that is prefered.


thx!
hofrat

Re: [PATCH V2] clk: zynq: do not allow kmalloc failure

[Nicholas Mc Guire]

On Thu, Nov 29, 2018 at 03:45:23PM -0800, Stephen Boyd wrote:
> Quoting Nicholas Mc Guire (2018-11-21 04:28:30)
> > The kmalloc here is small (< 16 bytes) and occurs during initialization
> > during system startup here (can not be built as module) thus if this
> > kmalloc failed it is an indication of something more serious going on
> > and it is fine to hang the system here rather than cause some harder
> > to understand error by dereferencing NULL.
> > 
> > Explicitly checking would not make that much sense here as the only
> > possible reaction would be would BUG() here anyway.
> > 
> > Signed-off-by: Nicholas Mc Guire 
> > Fixes: 0ee52b157b8e ("clk: zynq: Add clock controller driver")
> > Acked-by: Michal Simek 
> > ---
> 
> Nak. We don't have any __GFP_NOFAIL in drivers/clk and I don't see a
> reason why we would want it here either. Just handle the failure, or
> don't care if this is so critical to system boot.
>
It was not motivated by the criticality but by the low probability
and cluttering the code for this case did not seem good to me.
Effectively handling it here means BUG() - so more or less
the same result that hanging it on __GFP_NOFAIL if allocation
was not possible would cause.

Not clear what the objection to __GFP_NOFAIL here is - my understanding
was that it is intended precisely for cases like this - but
I´ll send a V2 handling it with BUG_ON(!clk_name) if that is prefered.


thx!
hofrat

Re: [PATCH v2 4/4] irqchip: sifive-plic: Implement irq_set_affinity() for SMP host

[Atish Patra]

On 11/29/18 9:59 PM, Atish Patra wrote:

On 11/27/18 2:04 AM, Anup Patel wrote:

Currently on SMP host, all CPUs take external interrupts routed via
PLIC. All CPUs will try to claim a given external interrupt but only
one of them will succeed while other CPUs would simply resume whatever
they were doing before. This means if we have N CPUs then for every
external interrupt N-1 CPUs will always fail to claim it and waste
their CPU time.

Instead of above, external interrupts should be taken by only one CPU
and we should have provision to explicity specify IRQ affinity from

s/explicity/explicitly


kernel-space or user-space.

This patch provides irq_set_affinity() implementation for PLIC driver.
It also updates irq_enable() such that PLIC interrupts are only enabled
for one of CPUs specified in IRQ affinity mask.

With this patch in-place, we can change IRQ affinity at any-time from
user-space using procfs.

Example:

/ # cat /proc/interrupts
 CPU0   CPU1   CPU2   CPU3
8: 44  0  0  0  SiFive PLIC   8  virtio0
   10: 48  0  0  0  SiFive PLIC  10  ttyS0
IPI0:55663 58363  Rescheduling interrupts
IPI1: 0  1  3 16  Function call interrupts
/ #
/ #
/ # echo 4 > /proc/irq/10/smp_affinity
/ #
/ # cat /proc/interrupts
 CPU0   CPU1   CPU2   CPU3
8: 45  0  0  0  SiFive PLIC   8  virtio0
   10:160  0 17  0  SiFive PLIC  10  ttyS0
IPI0:68693 77410  Rescheduling interrupts
IPI1: 0  2  3 16  Function call interrupts

Signed-off-by: Anup Patel 
---
   drivers/irqchip/irq-sifive-plic.c | 35 +--
   1 file changed, 33 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-sifive-plic.c 
b/drivers/irqchip/irq-sifive-plic.c
index ffd4deaca057..fec7da3797fa 100644
--- a/drivers/irqchip/irq-sifive-plic.c
+++ b/drivers/irqchip/irq-sifive-plic.c
@@ -98,14 +98,42 @@ static void plic_irq_toggle(const struct cpumask *mask, int 
hwirq, int enable)
   
   static void plic_irq_enable(struct irq_data *d)

   {
-   plic_irq_toggle(irq_data_get_affinity_mask(d), d->hwirq, 1);
+   unsigned int cpu = cpumask_any_and(irq_data_get_affinity_mask(d),
+  cpu_online_mask);
+   WARN_ON(cpu >= nr_cpu_ids);
+   plic_irq_toggle(cpumask_of(cpu), d->hwirq, 1);
   }
   
   static void plic_irq_disable(struct irq_data *d)

   {
-   plic_irq_toggle(irq_data_get_affinity_mask(d), d->hwirq, 0);
+   plic_irq_toggle(cpu_possible_mask, d->hwirq, 0);
   }
   
+#ifdef CONFIG_SMP

+static int plic_set_affinity(struct irq_data *d, const struct cpumask 
*mask_val,
+   bool force)
+{
+   unsigned int cpu;
+
+   if (!force)
+   cpu = cpumask_any_and(mask_val, cpu_online_mask);
+   else
+   cpu = cpumask_first(mask_val);
+
+   if (cpu >= nr_cpu_ids)
+   return -EINVAL;
+
+   if (!irqd_irq_disabled(d)) {
+   plic_irq_toggle(cpu_possible_mask, d->hwirq, 0);
+   plic_irq_toggle(cpumask_of(cpu), d->hwirq, 1);


irq is disabled for a fraction of time for cpu as well.
You can use cpumask_andnot to avoid that.


Moreover, something is weird here. I tested the patch in Unleashed with
a debug statement.

Here are the cpumask plic_set_affinity receives.

# echo 0 > /proc[  280.81] plic: plic_set_affinity: set affinity [0-1]
[  280.81] plic: plic_set_affinity: cpu = [0] irq = 4
# echo 1 > /proc[  286.29] plic: plic_set_affinity: set affinity [0]
[  286.29] plic: plic_set_affinity: cpu = [0] irq = 4
# echo 2 > /proc[  292.13] plic: plic_set_affinity: set affinity [1]
[  292.13] plic: plic_set_affinity: cpu = [1] irq = 4
# echo 3 > /proc[  297.75] plic: plic_set_affinity: set affinity [0-1]
[  297.75] plic: plic_set_affinity: cpu = [0] irq = 4

# echo 2 > /proc/irq/4/smp_affinity
[  322.85] plic: plic_set_affinity: set affinity [1]
[  322.85] plic: plic_set_affinity: cpu = [1] irq = 4

I have not figured out why it receive cpu mask for 0 & 3.
Not sure if logical cpu id to hart id mapping is responsible for other
two case. I will continue to test tomorrow.



Never mind.
The input is in hex which explains the cpumask which explains all three 
cases except 0.

If we pass zero, it just passing the previous affinity mask.

Regards,
Atish

Regards,
Atish

+   }
+



+   irq_data_update_effective_affinity(d, cpumask_of(cpu));
+
+   return IRQ_SET_MASK_OK_DONE;
+}
+#endif
+
   static struct irq_chip plic_chip = {
.name   = "SiFive PLIC",
/*
@@ -114,6 +142,9 @@ static struct irq_chip plic_chip = {
 */
.irq_enable = plic_irq_enable,
.irq_disable= plic_irq_disable,
+#ifdef 

Re: [PATCH v2 4/4] irqchip: sifive-plic: Implement irq_set_affinity() for SMP host

[Atish Patra]

On 11/29/18 9:59 PM, Atish Patra wrote:

On 11/27/18 2:04 AM, Anup Patel wrote:

Currently on SMP host, all CPUs take external interrupts routed via
PLIC. All CPUs will try to claim a given external interrupt but only
one of them will succeed while other CPUs would simply resume whatever
they were doing before. This means if we have N CPUs then for every
external interrupt N-1 CPUs will always fail to claim it and waste
their CPU time.

Instead of above, external interrupts should be taken by only one CPU
and we should have provision to explicity specify IRQ affinity from

s/explicity/explicitly


kernel-space or user-space.

This patch provides irq_set_affinity() implementation for PLIC driver.
It also updates irq_enable() such that PLIC interrupts are only enabled
for one of CPUs specified in IRQ affinity mask.

With this patch in-place, we can change IRQ affinity at any-time from
user-space using procfs.

Example:

/ # cat /proc/interrupts
 CPU0   CPU1   CPU2   CPU3
8: 44  0  0  0  SiFive PLIC   8  virtio0
   10: 48  0  0  0  SiFive PLIC  10  ttyS0
IPI0:55663 58363  Rescheduling interrupts
IPI1: 0  1  3 16  Function call interrupts
/ #
/ #
/ # echo 4 > /proc/irq/10/smp_affinity
/ #
/ # cat /proc/interrupts
 CPU0   CPU1   CPU2   CPU3
8: 45  0  0  0  SiFive PLIC   8  virtio0
   10:160  0 17  0  SiFive PLIC  10  ttyS0
IPI0:68693 77410  Rescheduling interrupts
IPI1: 0  2  3 16  Function call interrupts

Signed-off-by: Anup Patel 
---
   drivers/irqchip/irq-sifive-plic.c | 35 +--
   1 file changed, 33 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-sifive-plic.c 
b/drivers/irqchip/irq-sifive-plic.c
index ffd4deaca057..fec7da3797fa 100644
--- a/drivers/irqchip/irq-sifive-plic.c
+++ b/drivers/irqchip/irq-sifive-plic.c
@@ -98,14 +98,42 @@ static void plic_irq_toggle(const struct cpumask *mask, int 
hwirq, int enable)
   
   static void plic_irq_enable(struct irq_data *d)

   {
-   plic_irq_toggle(irq_data_get_affinity_mask(d), d->hwirq, 1);
+   unsigned int cpu = cpumask_any_and(irq_data_get_affinity_mask(d),
+  cpu_online_mask);
+   WARN_ON(cpu >= nr_cpu_ids);
+   plic_irq_toggle(cpumask_of(cpu), d->hwirq, 1);
   }
   
   static void plic_irq_disable(struct irq_data *d)

   {
-   plic_irq_toggle(irq_data_get_affinity_mask(d), d->hwirq, 0);
+   plic_irq_toggle(cpu_possible_mask, d->hwirq, 0);
   }
   
+#ifdef CONFIG_SMP

+static int plic_set_affinity(struct irq_data *d, const struct cpumask 
*mask_val,
+   bool force)
+{
+   unsigned int cpu;
+
+   if (!force)
+   cpu = cpumask_any_and(mask_val, cpu_online_mask);
+   else
+   cpu = cpumask_first(mask_val);
+
+   if (cpu >= nr_cpu_ids)
+   return -EINVAL;
+
+   if (!irqd_irq_disabled(d)) {
+   plic_irq_toggle(cpu_possible_mask, d->hwirq, 0);
+   plic_irq_toggle(cpumask_of(cpu), d->hwirq, 1);


irq is disabled for a fraction of time for cpu as well.
You can use cpumask_andnot to avoid that.


Moreover, something is weird here. I tested the patch in Unleashed with
a debug statement.

Here are the cpumask plic_set_affinity receives.

# echo 0 > /proc[  280.81] plic: plic_set_affinity: set affinity [0-1]
[  280.81] plic: plic_set_affinity: cpu = [0] irq = 4
# echo 1 > /proc[  286.29] plic: plic_set_affinity: set affinity [0]
[  286.29] plic: plic_set_affinity: cpu = [0] irq = 4
# echo 2 > /proc[  292.13] plic: plic_set_affinity: set affinity [1]
[  292.13] plic: plic_set_affinity: cpu = [1] irq = 4
# echo 3 > /proc[  297.75] plic: plic_set_affinity: set affinity [0-1]
[  297.75] plic: plic_set_affinity: cpu = [0] irq = 4

# echo 2 > /proc/irq/4/smp_affinity
[  322.85] plic: plic_set_affinity: set affinity [1]
[  322.85] plic: plic_set_affinity: cpu = [1] irq = 4

I have not figured out why it receive cpu mask for 0 & 3.
Not sure if logical cpu id to hart id mapping is responsible for other
two case. I will continue to test tomorrow.



Never mind.
The input is in hex which explains the cpumask which explains all three 
cases except 0.

If we pass zero, it just passing the previous affinity mask.

Regards,
Atish

Regards,
Atish

+   }
+



+   irq_data_update_effective_affinity(d, cpumask_of(cpu));
+
+   return IRQ_SET_MASK_OK_DONE;
+}
+#endif
+
   static struct irq_chip plic_chip = {
.name   = "SiFive PLIC",
/*
@@ -114,6 +142,9 @@ static struct irq_chip plic_chip = {
 */
.irq_enable = plic_irq_enable,
.irq_disable= plic_irq_disable,
+#ifdef 

Re: [PATCH v5 7/8] clk: lochnagar: Add support for the Cirrus Logic Lochnagar

[Stephen Boyd]

Quoting Charles Keepax (2018-11-20 06:16:30)
> diff --git a/drivers/clk/clk-lochnagar.c b/drivers/clk/clk-lochnagar.c
> new file mode 100644
> index ..8b2a78689715
> --- /dev/null
> +++ b/drivers/clk/clk-lochnagar.c
> @@ -0,0 +1,360 @@
[...]
> +
> +static int lochnagar_regmap_set_parent(struct clk_hw *hw, u8 index)
> +{
> +   struct lochnagar_clk *lclk = lochnagar_hw_to_lclk(hw);
> +   struct lochnagar_clk_priv *priv = lclk->priv;
> +   struct regmap *regmap = priv->regmap;
> +   int ret;
> +
> +   /*
> +* Some clocks on Lochnagar can either generate a clock themselves
> +* or accept an external clock, these default to generating the clock
> +* themselves. If we set a parent however we should update the 
> dir_mask
> +* to indicate to the hardware that this clock will now be receiving 
> an
> +* external clock.

Hmm ok. So the plan is to configure parents in DT or from driver code if
the configuration is to accept an external clk? I guess this works.

> +*/
> +   if (lclk->dir_mask) {
> +   ret = regmap_update_bits(regmap, lclk->cfg_reg,
> +lclk->dir_mask, lclk->dir_mask);
> +   if (ret < 0) {
> +   dev_err(priv->dev, "Failed to set %s direction: %d\n",
> +   lclk->name, ret);
> +   return ret;
> +   }
> +   }
> +
> +   ret = regmap_update_bits(regmap, lclk->src_reg, lclk->src_mask, 
> index);
> +   if (ret < 0)
> +   dev_err(priv->dev, "Failed to reparent %s: %d\n",
> +   lclk->name, ret);
> +
> +   return ret;
> +}
> +
> +static u8 lochnagar_regmap_get_parent(struct clk_hw *hw)
> +{
> +   struct lochnagar_clk *lclk = lochnagar_hw_to_lclk(hw);
> +   struct lochnagar_clk_priv *priv = lclk->priv;
> +   struct regmap *regmap = priv->regmap;
> +   unsigned int val;
> +   int ret;
> +
> +   ret = regmap_read(regmap, lclk->src_reg, );
> +   if (ret < 0) {
> +   dev_err(priv->dev, "Failed to read parent of %s: %d\n",
> +   lclk->name, ret);

The error messages in the above functions could be spammy. Just let
drivers who fail when using these clks ops print errors and maybe
downgrade these to debug? If you don't agree with this it's fine, I'll
just hope to never see these prints change to debug in the future.

> +   return priv->nparents;
> +   }
> +
> +   val &= lclk->src_mask;
> +
> +   return val;
> +}
> +
> +static const struct clk_ops lochnagar_clk_regmap_ops = {
> +   .prepare = lochnagar_regmap_prepare,
> +   .unprepare = lochnagar_regmap_unprepare,
> +   .set_parent = lochnagar_regmap_set_parent,
> +   .get_parent = lochnagar_regmap_get_parent,

Is regmap important to have in the name of these functions and struct?
I'd prefer it was just clk instead of regmap.

> +};
> +
> +static int lochnagar_init_parents(struct lochnagar_clk_priv *priv)
> +{
> +   struct device_node *np = priv->dev->of_node;
> +   int i, j;
> +
> +   switch (priv->type) {
> +   case LOCHNAGAR1:
> +   memcpy(priv->lclks, lochnagar1_clks, sizeof(lochnagar1_clks));
> +
> +   priv->nparents = ARRAY_SIZE(lochnagar1_clk_parents);
> +   priv->parents = devm_kmemdup(priv->dev, 
> lochnagar1_clk_parents,
> +sizeof(lochnagar1_clk_parents),
> +GFP_KERNEL);
> +   break;
> +   case LOCHNAGAR2:
> +   memcpy(priv->lclks, lochnagar2_clks, sizeof(lochnagar2_clks));
> +
> +   priv->nparents = ARRAY_SIZE(lochnagar2_clk_parents);
> +   priv->parents = devm_kmemdup(priv->dev, 
> lochnagar2_clk_parents,
> +sizeof(lochnagar2_clk_parents),
> +GFP_KERNEL);

Why do we need to kmemdup it? The clk framework already deep copies
everything from clk_init structure.

> +   break;
> +   default:
> +   dev_err(priv->dev, "Unknown Lochnagar type: %d\n", 
> priv->type);
> +   return -EINVAL;
> +   }
> +
> +   if (!priv->parents)
> +   return -ENOMEM;
> +
> +   for (i = 0; i < priv->nparents; i++) {
> +   j = of_property_match_string(np, "clock-names",
> +priv->parents[i]);
> +   if (j >= 0)
> +   priv->parents[i] = of_clk_get_parent_name(np, j);

Isn't this of_clk_parent_fill()? But there are holes or something?

> +   }
> +
> +   return 0;
> +}
> +
> +static int lochnagar_init_clks(struct lochnagar_clk_priv *priv)
> +{
> +   struct clk_init_data clk_init = {
> +   .ops = _clk_regmap_ops,
> +   .parent_names = priv->parents,
> +   

Re: [PATCH v5 7/8] clk: lochnagar: Add support for the Cirrus Logic Lochnagar

[Stephen Boyd]

Quoting Charles Keepax (2018-11-20 06:16:30)
> diff --git a/drivers/clk/clk-lochnagar.c b/drivers/clk/clk-lochnagar.c
> new file mode 100644
> index ..8b2a78689715
> --- /dev/null
> +++ b/drivers/clk/clk-lochnagar.c
> @@ -0,0 +1,360 @@
[...]
> +
> +static int lochnagar_regmap_set_parent(struct clk_hw *hw, u8 index)
> +{
> +   struct lochnagar_clk *lclk = lochnagar_hw_to_lclk(hw);
> +   struct lochnagar_clk_priv *priv = lclk->priv;
> +   struct regmap *regmap = priv->regmap;
> +   int ret;
> +
> +   /*
> +* Some clocks on Lochnagar can either generate a clock themselves
> +* or accept an external clock, these default to generating the clock
> +* themselves. If we set a parent however we should update the 
> dir_mask
> +* to indicate to the hardware that this clock will now be receiving 
> an
> +* external clock.

Hmm ok. So the plan is to configure parents in DT or from driver code if
the configuration is to accept an external clk? I guess this works.

> +*/
> +   if (lclk->dir_mask) {
> +   ret = regmap_update_bits(regmap, lclk->cfg_reg,
> +lclk->dir_mask, lclk->dir_mask);
> +   if (ret < 0) {
> +   dev_err(priv->dev, "Failed to set %s direction: %d\n",
> +   lclk->name, ret);
> +   return ret;
> +   }
> +   }
> +
> +   ret = regmap_update_bits(regmap, lclk->src_reg, lclk->src_mask, 
> index);
> +   if (ret < 0)
> +   dev_err(priv->dev, "Failed to reparent %s: %d\n",
> +   lclk->name, ret);
> +
> +   return ret;
> +}
> +
> +static u8 lochnagar_regmap_get_parent(struct clk_hw *hw)
> +{
> +   struct lochnagar_clk *lclk = lochnagar_hw_to_lclk(hw);
> +   struct lochnagar_clk_priv *priv = lclk->priv;
> +   struct regmap *regmap = priv->regmap;
> +   unsigned int val;
> +   int ret;
> +
> +   ret = regmap_read(regmap, lclk->src_reg, );
> +   if (ret < 0) {
> +   dev_err(priv->dev, "Failed to read parent of %s: %d\n",
> +   lclk->name, ret);

The error messages in the above functions could be spammy. Just let
drivers who fail when using these clks ops print errors and maybe
downgrade these to debug? If you don't agree with this it's fine, I'll
just hope to never see these prints change to debug in the future.

> +   return priv->nparents;
> +   }
> +
> +   val &= lclk->src_mask;
> +
> +   return val;
> +}
> +
> +static const struct clk_ops lochnagar_clk_regmap_ops = {
> +   .prepare = lochnagar_regmap_prepare,
> +   .unprepare = lochnagar_regmap_unprepare,
> +   .set_parent = lochnagar_regmap_set_parent,
> +   .get_parent = lochnagar_regmap_get_parent,

Is regmap important to have in the name of these functions and struct?
I'd prefer it was just clk instead of regmap.

> +};
> +
> +static int lochnagar_init_parents(struct lochnagar_clk_priv *priv)
> +{
> +   struct device_node *np = priv->dev->of_node;
> +   int i, j;
> +
> +   switch (priv->type) {
> +   case LOCHNAGAR1:
> +   memcpy(priv->lclks, lochnagar1_clks, sizeof(lochnagar1_clks));
> +
> +   priv->nparents = ARRAY_SIZE(lochnagar1_clk_parents);
> +   priv->parents = devm_kmemdup(priv->dev, 
> lochnagar1_clk_parents,
> +sizeof(lochnagar1_clk_parents),
> +GFP_KERNEL);
> +   break;
> +   case LOCHNAGAR2:
> +   memcpy(priv->lclks, lochnagar2_clks, sizeof(lochnagar2_clks));
> +
> +   priv->nparents = ARRAY_SIZE(lochnagar2_clk_parents);
> +   priv->parents = devm_kmemdup(priv->dev, 
> lochnagar2_clk_parents,
> +sizeof(lochnagar2_clk_parents),
> +GFP_KERNEL);

Why do we need to kmemdup it? The clk framework already deep copies
everything from clk_init structure.

> +   break;
> +   default:
> +   dev_err(priv->dev, "Unknown Lochnagar type: %d\n", 
> priv->type);
> +   return -EINVAL;
> +   }
> +
> +   if (!priv->parents)
> +   return -ENOMEM;
> +
> +   for (i = 0; i < priv->nparents; i++) {
> +   j = of_property_match_string(np, "clock-names",
> +priv->parents[i]);
> +   if (j >= 0)
> +   priv->parents[i] = of_clk_get_parent_name(np, j);

Isn't this of_clk_parent_fill()? But there are holes or something?

> +   }
> +
> +   return 0;
> +}
> +
> +static int lochnagar_init_clks(struct lochnagar_clk_priv *priv)
> +{
> +   struct clk_init_data clk_init = {
> +   .ops = _clk_regmap_ops,
> +   .parent_names = priv->parents,
> +   

[RFC PATCH V2 00/11] Intel EPT-Based Sub-page Protection Support

[Zhang Yi]

Here is a patch-series which adding EPT-Based Sub-page Write Protection Support.

Introduction:

EPT-Based Sub-page Write Protection referred to as SPP, it is a capability which
allow Virtual Machine Monitors(VMM) to specify write-permission for guest
physical memory at a sub-page(128 byte) granularity.  When this capability is
utilized, the CPU enforces write-access permissions for sub-page regions of 4K
pages as specified by the VMM. EPT-based sub-page permissions is intended to
enable fine-grained memory write enforcement by a VMM for security(guest OS
monitoring) and usages such as device virtualization and memory check-point.

SPPT is active when the "sub-page write protection" VM-execution control is 1.
SPPT looks up the guest physical addresses to derive a 64 bit "sub-page
permission" value containing sub-page write permissions. The lookup from
guest-physical addresses to the sub-page region permissions is determined by a
set of SPPT paging structures.

When the "sub-page write protection" VM-execution control is 1, the SPPT is used
to lookup write permission bits for the 128 byte sub-page regions containing in
the 4KB guest physical page. EPT specifies the 4KB page level privileges that
software is allowed when accessing the guest physical address, whereas SPPT
defines the write permissions for software at the 128 byte granularity regions
within a 4KB page. Write accesses prevented due to sub-page permissions looked
up via SPPT are reported as EPT violation VM exits. Similar to EPT, a logical
processor uses SPPT to lookup sub-page region write permissions for
guest-physical addresses only when those addresses are used to access memory.

__

How SPP hardware works:
__


Guest write access --> GPA --> Walk EPT --> EPT leaf entry -┐
┌---┘
└-> if VMexec_control.spp && ept_leaf_entry.spp_bit (bit 61)
 |
 └->  --> EPT legacy behavior
 |
 |
 └->   --> if ept_leaf_entry.writable
  |
  └->   --> Ignore SPP
  |
  └->  --> GPA --> Walk SPP 4-level table--┐
  |
┌<--get-the-SPPT-point-from-VMCS-filed-<--┘
|
Walk SPP L4E table
|
└┐--> entry misconfiguration >--┐<┐
 |  | |
else| |
 |  | |
 |   ┌--SPP VMexit<-┘ |
 |   ||
 |   └-> exit_qualification & sppt_misconfig --> sppt misconfig   |
 |   ||
 |   └-> exit_qualification & sppt_miss --> sppt miss |
 └--┐ |
| |
walk SPPT L3E--┐--> if-entry-misconfiguration>┘
   |  |
  else|
   |  |
   |  |
walk SPPT L2E --┐--> if-entry-misconfiguration>---┘
| |
   else   |
| |
| |
 walk SPPT L1E --┐-> if-entry-misconfiguration--->┘
 |
else
 |
 └-> if sub-page writable
  └->   allow, write access
  └->  disallow, EPT violation

Patch description:

Patch 1: The design Doc of EPT-Based Sub-page Write Protection(SPP)

Patch 2: this patch adds reporting SPP capability from VMX Procbased MSR,
according to the definition of hardware spec, bit 23 is the control of the SPP
capability.

Patch 3: Add new secondary processor-based VM-execution control bit which
defined as "sub-page write permission", same as VMX Procbased MSR, bit 23 is
the enable bit of SPP. Also we introduced a kernel parameter "enable_ept_spp",
now SPP is active when the "Sub-page Write Protection" in Secondary VM-Execution
Control is set and enable the kernel parameter by "spp=1".

Patch 4: Introduced the 

[RFC PATCH V2 00/11] Intel EPT-Based Sub-page Protection Support

[Zhang Yi]

Here is a patch-series which adding EPT-Based Sub-page Write Protection Support.

Introduction:

EPT-Based Sub-page Write Protection referred to as SPP, it is a capability which
allow Virtual Machine Monitors(VMM) to specify write-permission for guest
physical memory at a sub-page(128 byte) granularity.  When this capability is
utilized, the CPU enforces write-access permissions for sub-page regions of 4K
pages as specified by the VMM. EPT-based sub-page permissions is intended to
enable fine-grained memory write enforcement by a VMM for security(guest OS
monitoring) and usages such as device virtualization and memory check-point.

SPPT is active when the "sub-page write protection" VM-execution control is 1.
SPPT looks up the guest physical addresses to derive a 64 bit "sub-page
permission" value containing sub-page write permissions. The lookup from
guest-physical addresses to the sub-page region permissions is determined by a
set of SPPT paging structures.

When the "sub-page write protection" VM-execution control is 1, the SPPT is used
to lookup write permission bits for the 128 byte sub-page regions containing in
the 4KB guest physical page. EPT specifies the 4KB page level privileges that
software is allowed when accessing the guest physical address, whereas SPPT
defines the write permissions for software at the 128 byte granularity regions
within a 4KB page. Write accesses prevented due to sub-page permissions looked
up via SPPT are reported as EPT violation VM exits. Similar to EPT, a logical
processor uses SPPT to lookup sub-page region write permissions for
guest-physical addresses only when those addresses are used to access memory.

__

How SPP hardware works:
__


Guest write access --> GPA --> Walk EPT --> EPT leaf entry -┐
┌---┘
└-> if VMexec_control.spp && ept_leaf_entry.spp_bit (bit 61)
 |
 └->  --> EPT legacy behavior
 |
 |
 └->   --> if ept_leaf_entry.writable
  |
  └->   --> Ignore SPP
  |
  └->  --> GPA --> Walk SPP 4-level table--┐
  |
┌<--get-the-SPPT-point-from-VMCS-filed-<--┘
|
Walk SPP L4E table
|
└┐--> entry misconfiguration >--┐<┐
 |  | |
else| |
 |  | |
 |   ┌--SPP VMexit<-┘ |
 |   ||
 |   └-> exit_qualification & sppt_misconfig --> sppt misconfig   |
 |   ||
 |   └-> exit_qualification & sppt_miss --> sppt miss |
 └--┐ |
| |
walk SPPT L3E--┐--> if-entry-misconfiguration>┘
   |  |
  else|
   |  |
   |  |
walk SPPT L2E --┐--> if-entry-misconfiguration>---┘
| |
   else   |
| |
| |
 walk SPPT L1E --┐-> if-entry-misconfiguration--->┘
 |
else
 |
 └-> if sub-page writable
  └->   allow, write access
  └->  disallow, EPT violation

Patch description:

Patch 1: The design Doc of EPT-Based Sub-page Write Protection(SPP)

Patch 2: this patch adds reporting SPP capability from VMX Procbased MSR,
according to the definition of hardware spec, bit 23 is the control of the SPP
capability.

Patch 3: Add new secondary processor-based VM-execution control bit which
defined as "sub-page write permission", same as VMX Procbased MSR, bit 23 is
the enable bit of SPP. Also we introduced a kernel parameter "enable_ept_spp",
now SPP is active when the "Sub-page Write Protection" in Secondary VM-Execution
Control is set and enable the kernel parameter by "spp=1".

Patch 4: Introduced the 

Re: [PATCH v2 4/4] irqchip: sifive-plic: Implement irq_set_affinity() for SMP host

[Anup Patel]

On Fri, Nov 30, 2018 at 11:29 AM Atish Patra  wrote:
>
> On 11/27/18 2:04 AM, Anup Patel wrote:
> > Currently on SMP host, all CPUs take external interrupts routed via
> > PLIC. All CPUs will try to claim a given external interrupt but only
> > one of them will succeed while other CPUs would simply resume whatever
> > they were doing before. This means if we have N CPUs then for every
> > external interrupt N-1 CPUs will always fail to claim it and waste
> > their CPU time.
> >
> > Instead of above, external interrupts should be taken by only one CPU
> > and we should have provision to explicity specify IRQ affinity from
> s/explicity/explicitly

Sure, I will update it.

>
> > kernel-space or user-space.
> >
> > This patch provides irq_set_affinity() implementation for PLIC driver.
> > It also updates irq_enable() such that PLIC interrupts are only enabled
> > for one of CPUs specified in IRQ affinity mask.
> >
> > With this patch in-place, we can change IRQ affinity at any-time from
> > user-space using procfs.
> >
> > Example:
> >
> > / # cat /proc/interrupts
> > CPU0   CPU1   CPU2   CPU3
> >8: 44  0  0  0  SiFive PLIC   8  virtio0
> >   10: 48  0  0  0  SiFive PLIC  10  ttyS0
> > IPI0:55663 58363  Rescheduling interrupts
> > IPI1: 0  1  3 16  Function call interrupts
> > / #
> > / #
> > / # echo 4 > /proc/irq/10/smp_affinity
> > / #
> > / # cat /proc/interrupts
> > CPU0   CPU1   CPU2   CPU3
> >8: 45  0  0  0  SiFive PLIC   8  virtio0
> >   10:160  0 17  0  SiFive PLIC  10  ttyS0
> > IPI0:68693 77410  Rescheduling interrupts
> > IPI1: 0  2  3 16  Function call interrupts
> >
> > Signed-off-by: Anup Patel 
> > ---
> >   drivers/irqchip/irq-sifive-plic.c | 35 +--
> >   1 file changed, 33 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/irqchip/irq-sifive-plic.c 
> > b/drivers/irqchip/irq-sifive-plic.c
> > index ffd4deaca057..fec7da3797fa 100644
> > --- a/drivers/irqchip/irq-sifive-plic.c
> > +++ b/drivers/irqchip/irq-sifive-plic.c
> > @@ -98,14 +98,42 @@ static void plic_irq_toggle(const struct cpumask *mask, 
> > int hwirq, int enable)
> >
> >   static void plic_irq_enable(struct irq_data *d)
> >   {
> > - plic_irq_toggle(irq_data_get_affinity_mask(d), d->hwirq, 1);
> > + unsigned int cpu = cpumask_any_and(irq_data_get_affinity_mask(d),
> > +cpu_online_mask);
> > + WARN_ON(cpu >= nr_cpu_ids);
> > + plic_irq_toggle(cpumask_of(cpu), d->hwirq, 1);
> >   }
> >
> >   static void plic_irq_disable(struct irq_data *d)
> >   {
> > - plic_irq_toggle(irq_data_get_affinity_mask(d), d->hwirq, 0);
> > + plic_irq_toggle(cpu_possible_mask, d->hwirq, 0);
> >   }
> >
> > +#ifdef CONFIG_SMP
> > +static int plic_set_affinity(struct irq_data *d, const struct cpumask 
> > *mask_val,
> > + bool force)
> > +{
> > + unsigned int cpu;
> > +
> > + if (!force)
> > + cpu = cpumask_any_and(mask_val, cpu_online_mask);
> > + else
> > + cpu = cpumask_first(mask_val);
> > +
> > + if (cpu >= nr_cpu_ids)
> > + return -EINVAL;
> > +
> > + if (!irqd_irq_disabled(d)) {
> > + plic_irq_toggle(cpu_possible_mask, d->hwirq, 0);
> > + plic_irq_toggle(cpumask_of(cpu), d->hwirq, 1);
>
> irq is disabled for a fraction of time for cpu as well.
> You can use cpumask_andnot to avoid that.
>
>
> Moreover, something is weird here. I tested the patch in Unleashed with
> a debug statement.
>
> Here are the cpumask plic_set_affinity receives.

The smp_affinity in procfs takes hex values as input.

1 = CPU0
2 = CPU1
3 = CPU0-1
4 = CPU2
... and so on ...

>
> # echo 0 > /proc[  280.81] plic: plic_set_affinity: set affinity [0-1]
> [  280.81] plic: plic_set_affinity: cpu = [0] irq = 4

OK, this is strange.

> # echo 1 > /proc[  286.29] plic: plic_set_affinity: set affinity [0]
> [  286.29] plic: plic_set_affinity: cpu = [0] irq = 4

This is correct.

> # echo 2 > /proc[  292.13] plic: plic_set_affinity: set affinity [1]
> [  292.13] plic: plic_set_affinity: cpu = [1] irq = 4

This is correct.

> # echo 3 > /proc[  297.75] plic: plic_set_affinity: set affinity [0-1]
> [  297.75] plic: plic_set_affinity: cpu = [0] irq = 4

This is correct.

>
> # echo 2 > /proc/irq/4/smp_affinity
> [  322.85] plic: plic_set_affinity: set affinity [1]
> [  322.85] plic: plic_set_affinity: cpu = [1] irq = 4

This is correct.

>
> I have not figured out why it receive cpu mask for 0 & 3.
> Not sure if logical cpu id to hart id mapping is responsible for other
> two case. I will continue to test tomorrow.

Except value '0', all cases are 

Re: [PATCH v2 4/4] irqchip: sifive-plic: Implement irq_set_affinity() for SMP host

[Anup Patel]

On Fri, Nov 30, 2018 at 11:29 AM Atish Patra  wrote:
>
> On 11/27/18 2:04 AM, Anup Patel wrote:
> > Currently on SMP host, all CPUs take external interrupts routed via
> > PLIC. All CPUs will try to claim a given external interrupt but only
> > one of them will succeed while other CPUs would simply resume whatever
> > they were doing before. This means if we have N CPUs then for every
> > external interrupt N-1 CPUs will always fail to claim it and waste
> > their CPU time.
> >
> > Instead of above, external interrupts should be taken by only one CPU
> > and we should have provision to explicity specify IRQ affinity from
> s/explicity/explicitly

Sure, I will update it.

>
> > kernel-space or user-space.
> >
> > This patch provides irq_set_affinity() implementation for PLIC driver.
> > It also updates irq_enable() such that PLIC interrupts are only enabled
> > for one of CPUs specified in IRQ affinity mask.
> >
> > With this patch in-place, we can change IRQ affinity at any-time from
> > user-space using procfs.
> >
> > Example:
> >
> > / # cat /proc/interrupts
> > CPU0   CPU1   CPU2   CPU3
> >8: 44  0  0  0  SiFive PLIC   8  virtio0
> >   10: 48  0  0  0  SiFive PLIC  10  ttyS0
> > IPI0:55663 58363  Rescheduling interrupts
> > IPI1: 0  1  3 16  Function call interrupts
> > / #
> > / #
> > / # echo 4 > /proc/irq/10/smp_affinity
> > / #
> > / # cat /proc/interrupts
> > CPU0   CPU1   CPU2   CPU3
> >8: 45  0  0  0  SiFive PLIC   8  virtio0
> >   10:160  0 17  0  SiFive PLIC  10  ttyS0
> > IPI0:68693 77410  Rescheduling interrupts
> > IPI1: 0  2  3 16  Function call interrupts
> >
> > Signed-off-by: Anup Patel 
> > ---
> >   drivers/irqchip/irq-sifive-plic.c | 35 +--
> >   1 file changed, 33 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/irqchip/irq-sifive-plic.c 
> > b/drivers/irqchip/irq-sifive-plic.c
> > index ffd4deaca057..fec7da3797fa 100644
> > --- a/drivers/irqchip/irq-sifive-plic.c
> > +++ b/drivers/irqchip/irq-sifive-plic.c
> > @@ -98,14 +98,42 @@ static void plic_irq_toggle(const struct cpumask *mask, 
> > int hwirq, int enable)
> >
> >   static void plic_irq_enable(struct irq_data *d)
> >   {
> > - plic_irq_toggle(irq_data_get_affinity_mask(d), d->hwirq, 1);
> > + unsigned int cpu = cpumask_any_and(irq_data_get_affinity_mask(d),
> > +cpu_online_mask);
> > + WARN_ON(cpu >= nr_cpu_ids);
> > + plic_irq_toggle(cpumask_of(cpu), d->hwirq, 1);
> >   }
> >
> >   static void plic_irq_disable(struct irq_data *d)
> >   {
> > - plic_irq_toggle(irq_data_get_affinity_mask(d), d->hwirq, 0);
> > + plic_irq_toggle(cpu_possible_mask, d->hwirq, 0);
> >   }
> >
> > +#ifdef CONFIG_SMP
> > +static int plic_set_affinity(struct irq_data *d, const struct cpumask 
> > *mask_val,
> > + bool force)
> > +{
> > + unsigned int cpu;
> > +
> > + if (!force)
> > + cpu = cpumask_any_and(mask_val, cpu_online_mask);
> > + else
> > + cpu = cpumask_first(mask_val);
> > +
> > + if (cpu >= nr_cpu_ids)
> > + return -EINVAL;
> > +
> > + if (!irqd_irq_disabled(d)) {
> > + plic_irq_toggle(cpu_possible_mask, d->hwirq, 0);
> > + plic_irq_toggle(cpumask_of(cpu), d->hwirq, 1);
>
> irq is disabled for a fraction of time for cpu as well.
> You can use cpumask_andnot to avoid that.
>
>
> Moreover, something is weird here. I tested the patch in Unleashed with
> a debug statement.
>
> Here are the cpumask plic_set_affinity receives.

The smp_affinity in procfs takes hex values as input.

1 = CPU0
2 = CPU1
3 = CPU0-1
4 = CPU2
... and so on ...

>
> # echo 0 > /proc[  280.81] plic: plic_set_affinity: set affinity [0-1]
> [  280.81] plic: plic_set_affinity: cpu = [0] irq = 4

OK, this is strange.

> # echo 1 > /proc[  286.29] plic: plic_set_affinity: set affinity [0]
> [  286.29] plic: plic_set_affinity: cpu = [0] irq = 4

This is correct.

> # echo 2 > /proc[  292.13] plic: plic_set_affinity: set affinity [1]
> [  292.13] plic: plic_set_affinity: cpu = [1] irq = 4

This is correct.

> # echo 3 > /proc[  297.75] plic: plic_set_affinity: set affinity [0-1]
> [  297.75] plic: plic_set_affinity: cpu = [0] irq = 4

This is correct.

>
> # echo 2 > /proc/irq/4/smp_affinity
> [  322.85] plic: plic_set_affinity: set affinity [1]
> [  322.85] plic: plic_set_affinity: cpu = [1] irq = 4

This is correct.

>
> I have not figured out why it receive cpu mask for 0 & 3.
> Not sure if logical cpu id to hart id mapping is responsible for other
> two case. I will continue to test tomorrow.

Except value '0', all cases are 

RE: [RFC/RFT][PATCH v6] cpuidle: New timer events oriented governor for tickless systems

[Doug Smythies]

Hi Rafael,

On 2018.11.23 02:36 Rafael J. Wysocki wrote:

... [snip]...

> +/**
> + * teo_find_shallower_state - Find shallower idle state matching given 
> duration.
> + * @drv: cpuidle driver containing state data.
> + * @dev: Target CPU.
> + * @state_idx: Index of the capping idle state.
> + * @duration_us: Idle duration value to match.
> + */
> +static int teo_find_shallower_state(struct cpuidle_driver *drv,
> + struct cpuidle_device *dev, int state_idx,
> + unsigned int duration_us)
> +{
> + int i;
> +
> + for (i = state_idx - 1; i > 0; i--) {
> + if (drv->states[i].disabled || dev->states_usage[i].disable)
> + continue;
> +
> + if (drv->states[i].target_residency <= duration_us)
> + break;
> + }
> + return i;
> +}

I think this subroutine has a problem when idle state 0
is disabled.

Perhaps something like this might help:

diff --git a/drivers/cpuidle/governors/teo.c b/drivers/cpuidle/governors/teo.c
index bc1c9a2..5b97639 100644
--- a/drivers/cpuidle/governors/teo.c
+++ b/drivers/cpuidle/governors/teo.c
@@ -196,7 +196,8 @@ static void teo_update(struct cpuidle_driver *drv, struct 
cpuidle_device *dev)
 }

 /**
- * teo_find_shallower_state - Find shallower idle state matching given 
duration.
+ * teo_find_shallower_state - Find shallower idle state matching given
+ * duration, if possible.
  * @drv: cpuidle driver containing state data.
  * @dev: Target CPU.
  * @state_idx: Index of the capping idle state.
@@ -208,13 +209,15 @@ static int teo_find_shallower_state(struct cpuidle_driver 
*drv,
 {
int i;

-   for (i = state_idx - 1; i > 0; i--) {
+   for (i = state_idx - 1; i >= 0; i--) {
if (drv->states[i].disabled || dev->states_usage[i].disable)
continue;

if (drv->states[i].target_residency <= duration_us)
break;
}
+   if (i < 0)
+   i = state_idx;
return i;
 }

@@ -264,7 +267,6 @@ static int teo_select(struct cpuidle_driver *drv, struct 
cpuidle_device *dev,
if (max_early_idx >= 0 &&
count < cpu_data->states[i].early_hits)
count = cpu_data->states[i].early_hits;
-
continue;
}

There is an additional issue where if idle state 0 is disabled (with the above 
suggested code patch),
idle state usage seems to fall to deeper states than idle state 1.
This is not the expected behaviour.
Kernel 4.20-rc3 works as expected.
I have not figured this issue out yet, in the code.

Example (1 minute per sample. Number of entries/exits per state):
State 0 State 1 State 2 State 3 State 4Watts
   28235143, 83, 26, 17,837,  64.900
5583238, 657079,5884941,8498552,   30986831,  62.433 << 
Transition sample, after idle state 0 disabled
  0, 793517,7186099,   10559878,   38485721,  61.900 << ?? 
should have all gone into Idle state 1
  0, 795414,7340703,   10553117,   38513456,  62.050
  0, 807028,7288195,   10574113,   38523524,  62.167
  0, 814983,7403534,   10575108,   38571228,  62.167
  0, 838302,7747127,   10552289,   38556054,  62.183
9664999, 544473,4914512,6942037,   25295361,  63.633 << 
Transition sample, after idle state 0 enabled
   27893504, 96, 40,  9,912,  66.500
   26556343, 83, 29,  7,814,  66.683
   27929227, 64, 20, 10,931,  66.683
 
... Doug

RE: [RFC/RFT][PATCH v6] cpuidle: New timer events oriented governor for tickless systems

[Doug Smythies]

Hi Rafael,

On 2018.11.23 02:36 Rafael J. Wysocki wrote:

... [snip]...

> +/**
> + * teo_find_shallower_state - Find shallower idle state matching given 
> duration.
> + * @drv: cpuidle driver containing state data.
> + * @dev: Target CPU.
> + * @state_idx: Index of the capping idle state.
> + * @duration_us: Idle duration value to match.
> + */
> +static int teo_find_shallower_state(struct cpuidle_driver *drv,
> + struct cpuidle_device *dev, int state_idx,
> + unsigned int duration_us)
> +{
> + int i;
> +
> + for (i = state_idx - 1; i > 0; i--) {
> + if (drv->states[i].disabled || dev->states_usage[i].disable)
> + continue;
> +
> + if (drv->states[i].target_residency <= duration_us)
> + break;
> + }
> + return i;
> +}

I think this subroutine has a problem when idle state 0
is disabled.

Perhaps something like this might help:

diff --git a/drivers/cpuidle/governors/teo.c b/drivers/cpuidle/governors/teo.c
index bc1c9a2..5b97639 100644
--- a/drivers/cpuidle/governors/teo.c
+++ b/drivers/cpuidle/governors/teo.c
@@ -196,7 +196,8 @@ static void teo_update(struct cpuidle_driver *drv, struct 
cpuidle_device *dev)
 }

 /**
- * teo_find_shallower_state - Find shallower idle state matching given 
duration.
+ * teo_find_shallower_state - Find shallower idle state matching given
+ * duration, if possible.
  * @drv: cpuidle driver containing state data.
  * @dev: Target CPU.
  * @state_idx: Index of the capping idle state.
@@ -208,13 +209,15 @@ static int teo_find_shallower_state(struct cpuidle_driver 
*drv,
 {
int i;

-   for (i = state_idx - 1; i > 0; i--) {
+   for (i = state_idx - 1; i >= 0; i--) {
if (drv->states[i].disabled || dev->states_usage[i].disable)
continue;

if (drv->states[i].target_residency <= duration_us)
break;
}
+   if (i < 0)
+   i = state_idx;
return i;
 }

@@ -264,7 +267,6 @@ static int teo_select(struct cpuidle_driver *drv, struct 
cpuidle_device *dev,
if (max_early_idx >= 0 &&
count < cpu_data->states[i].early_hits)
count = cpu_data->states[i].early_hits;
-
continue;
}

There is an additional issue where if idle state 0 is disabled (with the above 
suggested code patch),
idle state usage seems to fall to deeper states than idle state 1.
This is not the expected behaviour.
Kernel 4.20-rc3 works as expected.
I have not figured this issue out yet, in the code.

Example (1 minute per sample. Number of entries/exits per state):
State 0 State 1 State 2 State 3 State 4Watts
   28235143, 83, 26, 17,837,  64.900
5583238, 657079,5884941,8498552,   30986831,  62.433 << 
Transition sample, after idle state 0 disabled
  0, 793517,7186099,   10559878,   38485721,  61.900 << ?? 
should have all gone into Idle state 1
  0, 795414,7340703,   10553117,   38513456,  62.050
  0, 807028,7288195,   10574113,   38523524,  62.167
  0, 814983,7403534,   10575108,   38571228,  62.167
  0, 838302,7747127,   10552289,   38556054,  62.183
9664999, 544473,4914512,6942037,   25295361,  63.633 << 
Transition sample, after idle state 0 enabled
   27893504, 96, 40,  9,912,  66.500
   26556343, 83, 29,  7,814,  66.683
   27929227, 64, 20, 10,931,  66.683
 
... Doug

Re: [PATCH v4 4/6] coresight: Use PMU driver configuration for sink selection

[Greg KH]

On Thu, Nov 29, 2018 at 04:09:15PM -0700, Mathieu Poirier wrote:
> Hi Greg,
> 
> On Thu, Nov 29, 2018 at 08:49:36AM +0100, Greg KH wrote:
> > On Wed, Nov 28, 2018 at 03:01:16PM -0700, Mathieu Poirier wrote:
> > > This patch uses the PMU driver configuration held in event::hw::drv_config
> > > to select a sink for each event that is created (the old sysFS way of
> > > working is kept around for backward compatibility).
> > 
> > It is "sysfs", no InterCaps please, I've never called it that in the
> > past.
> > 
> > And just use sysfs, if that does not work properly, then fix that, don't
> > create yet-another-way-to-configure-this-thing to just confuse people.
> 
> Thanks for the review, you've provided usefull comments.
> 
> Regarding the "char *" argument for the ioctl, I followed an example that
> currently exist but I can proceed differently.  

What driver currently uses a char * on an ioctl to parse arbritrary
userspace information to set its configuration?  That should be fixed...

> My goal with this patchset was specifically to fix what is wrong with sysfs 
> and
> completely take it out of the equation.  The only reason to keep the kernel 
> interface alive was to prevent braking older user space perf tools currently
> using it.  

That's fine, just don't create a new syscall that takes arbritrary data
and parses it in the kernel, that's not ok.

> I chose to use an ioctl() because it is flexible and well suited for the 
> dynamic
> nature of perf events.  It is also currently used to set various event 
> specific 
> configuration so doing the same adds to the established pattern and avoids
> creating a new way of doing things, something the perf crew would have been
> quick to point out.
> 
> Was my approach wrong?

I don't know how the perf interface works, so perhaps work with those
developers to sync up and match what they use today?

But step back, what exactly are you trying to do here?  You have an
implementation of a solution but I don't see the problem stated anywhere
here.

thanks,

greg k-h

Re: [PATCH v4 4/6] coresight: Use PMU driver configuration for sink selection

[Greg KH]

On Thu, Nov 29, 2018 at 04:09:15PM -0700, Mathieu Poirier wrote:
> Hi Greg,
> 
> On Thu, Nov 29, 2018 at 08:49:36AM +0100, Greg KH wrote:
> > On Wed, Nov 28, 2018 at 03:01:16PM -0700, Mathieu Poirier wrote:
> > > This patch uses the PMU driver configuration held in event::hw::drv_config
> > > to select a sink for each event that is created (the old sysFS way of
> > > working is kept around for backward compatibility).
> > 
> > It is "sysfs", no InterCaps please, I've never called it that in the
> > past.
> > 
> > And just use sysfs, if that does not work properly, then fix that, don't
> > create yet-another-way-to-configure-this-thing to just confuse people.
> 
> Thanks for the review, you've provided usefull comments.
> 
> Regarding the "char *" argument for the ioctl, I followed an example that
> currently exist but I can proceed differently.  

What driver currently uses a char * on an ioctl to parse arbritrary
userspace information to set its configuration?  That should be fixed...

> My goal with this patchset was specifically to fix what is wrong with sysfs 
> and
> completely take it out of the equation.  The only reason to keep the kernel 
> interface alive was to prevent braking older user space perf tools currently
> using it.  

That's fine, just don't create a new syscall that takes arbritrary data
and parses it in the kernel, that's not ok.

> I chose to use an ioctl() because it is flexible and well suited for the 
> dynamic
> nature of perf events.  It is also currently used to set various event 
> specific 
> configuration so doing the same adds to the established pattern and avoids
> creating a new way of doing things, something the perf crew would have been
> quick to point out.
> 
> Was my approach wrong?

I don't know how the perf interface works, so perhaps work with those
developers to sync up and match what they use today?

But step back, what exactly are you trying to do here?  You have an
implementation of a solution but I don't see the problem stated anywhere
here.

thanks,

greg k-h

RE: [PATCH] clk: imx6sl: ensure MMDC CH0 handshake is bypassed

[Anson Huang]

Hi, Stephen

Best Regards!
Anson Huang

> -Original Message-
> From: Stephen Boyd [mailto:sb...@kernel.org]
> Sent: 2018年11月30日 15:25
> To: ker...@pengutronix.de; linux-arm-ker...@lists.infradead.org;
> linux-...@vger.kernel.org; linux-kernel@vger.kernel.org;
> mturque...@baylibre.com; s.ha...@pengutronix.de; shawn...@kernel.org;
> Anson Huang ; Fabio Estevam
> 
> Cc: dl-linux-imx 
> Subject: Re: [PATCH] clk: imx6sl: ensure MMDC CH0 handshake is bypassed
> 
> Quoting Anson Huang (2018-11-29 23:23:47)
> > Same as other i.MX6 SoCs, ensure unused MMDC channel's handshake is
> > bypassed, this is to make sure no request signal will be generated
> > when periphe_clk_sel is changed or SRC warm reset is triggered.
> >
> > Signed-off-by: Anson Huang 
> 
> Does this need a fixes tag?

Normally this bit is 1b'1 by reset, but during our development, it used to be
overwritten in u-boot and lead to some clock operation fail in Linux kernel, 
that is why we ensure it
in clock driver early phase. IMO, it should be OK to not add a fix tag, since 
it is just
to make sure the setting is correct and NOT depends on the hardware reset value 
which
could be changed in u-boot.

Anson.

RE: [PATCH] clk: imx6sl: ensure MMDC CH0 handshake is bypassed

[Anson Huang]

Hi, Stephen

Best Regards!
Anson Huang

> -Original Message-
> From: Stephen Boyd [mailto:sb...@kernel.org]
> Sent: 2018年11月30日 15:25
> To: ker...@pengutronix.de; linux-arm-ker...@lists.infradead.org;
> linux-...@vger.kernel.org; linux-kernel@vger.kernel.org;
> mturque...@baylibre.com; s.ha...@pengutronix.de; shawn...@kernel.org;
> Anson Huang ; Fabio Estevam
> 
> Cc: dl-linux-imx 
> Subject: Re: [PATCH] clk: imx6sl: ensure MMDC CH0 handshake is bypassed
> 
> Quoting Anson Huang (2018-11-29 23:23:47)
> > Same as other i.MX6 SoCs, ensure unused MMDC channel's handshake is
> > bypassed, this is to make sure no request signal will be generated
> > when periphe_clk_sel is changed or SRC warm reset is triggered.
> >
> > Signed-off-by: Anson Huang 
> 
> Does this need a fixes tag?

Normally this bit is 1b'1 by reset, but during our development, it used to be
overwritten in u-boot and lead to some clock operation fail in Linux kernel, 
that is why we ensure it
in clock driver early phase. IMO, it should be OK to not add a fix tag, since 
it is just
to make sure the setting is correct and NOT depends on the hardware reset value 
which
could be changed in u-boot.

Anson.

Re: [PATCH v2 0/3] mach-omap2: handle autoidle denial

[Tero Kristo]

On 30/11/2018 02:26, Stephen Boyd wrote:

Quoting Andreas Kemnade (2018-11-10 12:31:12)

On the gta04 with a dm3730 omap_hdq does not work properly when the
device enters lower power states. Idling uart1 and 2 is enough
to show up that problem, if there are no other things enabled.
Further research reveals that hdq iclk must not be turned off during
transfers, also according to the TRM. That fact is also correctly described
in the flags but the code to handle that is incomplete.

To handle multiple users of a single ick, autoidle is disabled
when a user of that ick requires that (has the OCPIF_SWSUP_IDLE))

Changes since v1:
- uses spinlocks instead of mutexes
- invert counter logic
- check whether clock type is basic



I'm expecting someone like Tero or Tony to review this.



Rest of it looks fine to me, except for the discussion under the 
CLK_IS_BASIC flag, which might trigger a bigger rework of the code.


-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Re: [PATCH v2 0/3] mach-omap2: handle autoidle denial

[Tero Kristo]

On 30/11/2018 02:26, Stephen Boyd wrote:

Quoting Andreas Kemnade (2018-11-10 12:31:12)

On the gta04 with a dm3730 omap_hdq does not work properly when the
device enters lower power states. Idling uart1 and 2 is enough
to show up that problem, if there are no other things enabled.
Further research reveals that hdq iclk must not be turned off during
transfers, also according to the TRM. That fact is also correctly described
in the flags but the code to handle that is incomplete.

To handle multiple users of a single ick, autoidle is disabled
when a user of that ick requires that (has the OCPIF_SWSUP_IDLE))

Changes since v1:
- uses spinlocks instead of mutexes
- invert counter logic
- check whether clock type is basic



I'm expecting someone like Tero or Tony to review this.



Rest of it looks fine to me, except for the discussion under the 
CLK_IS_BASIC flag, which might trigger a bigger rework of the code.


-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Re: [PATCH v2 2/3] clk: ti: check clock type before doing autoidle ops

[Tero Kristo]

On 30/11/2018 09:20, Stephen Boyd wrote:

Quoting Andreas Kemnade (2018-11-29 22:15:34)

Hi Stephen,

On Thu, 29 Nov 2018 16:25:05 -0800
Stephen Boyd  wrote:


Quoting Andreas Kemnade (2018-11-10 12:31:14)

Code might use autoidle api with clocks not being omap2 clocks,
so check if clock type is not basic

Signed-off-by: Andreas Kemnade 
---
New in v2
---
  drivers/clk/ti/autoidle.c | 12 ++--
  1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/ti/autoidle.c b/drivers/clk/ti/autoidle.c
index 161f67850393..5bdae5552d38 100644
--- a/drivers/clk/ti/autoidle.c
+++ b/drivers/clk/ti/autoidle.c
@@ -54,8 +54,12 @@ static DEFINE_SPINLOCK(autoidle_spinlock);
  int omap2_clk_deny_idle(struct clk *clk)
  {
 struct clk_hw_omap *c;
+   struct clk_hw *hw = __clk_get_hw(clk);
  
-   c = to_clk_hw_omap(__clk_get_hw(clk));

+   if (clk_hw_get_flags(hw) & CLK_IS_BASIC)


Please try to avoid using CLK_IS_BASIC if at all possible. Can you?
Maybe add some flag in clk_hw_omap() instead?


hmm, Tero suggested that.
But to check flags in clk_hw_omap I first need to know that there is a
clk_hw_omap behind clk_hw. And for that I either need to check flags in
clk_hw or do more changes in the omap_hwmod code.


Can you do it? The omap code is the only user of CLK_IS_BASIC. All the
other users are marking clks with this but there is no reason to do so.
I'll go make another pass over the tree and nuke those ones from orbit.


The reason for using this flag is because OMAP uses two clock types 
around, the basic clocks like fixed-factor-clock/fixed-clock, and then 
all the omap derivatives, which can be cast to clk_hw_omap. If we want 
to avoid usage of CLK_IS_BASIC, we need to copy paste the remaining 
basic code under drivers/clk/ti/ and convert them to use clk_hw_omap as 
internal datatype. Is this preferred?


-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Re: [PATCH v2 2/3] clk: ti: check clock type before doing autoidle ops

[Tero Kristo]

On 30/11/2018 09:20, Stephen Boyd wrote:

Quoting Andreas Kemnade (2018-11-29 22:15:34)

Hi Stephen,

On Thu, 29 Nov 2018 16:25:05 -0800
Stephen Boyd  wrote:


Quoting Andreas Kemnade (2018-11-10 12:31:14)

Code might use autoidle api with clocks not being omap2 clocks,
so check if clock type is not basic

Signed-off-by: Andreas Kemnade 
---
New in v2
---
  drivers/clk/ti/autoidle.c | 12 ++--
  1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/ti/autoidle.c b/drivers/clk/ti/autoidle.c
index 161f67850393..5bdae5552d38 100644
--- a/drivers/clk/ti/autoidle.c
+++ b/drivers/clk/ti/autoidle.c
@@ -54,8 +54,12 @@ static DEFINE_SPINLOCK(autoidle_spinlock);
  int omap2_clk_deny_idle(struct clk *clk)
  {
 struct clk_hw_omap *c;
+   struct clk_hw *hw = __clk_get_hw(clk);
  
-   c = to_clk_hw_omap(__clk_get_hw(clk));

+   if (clk_hw_get_flags(hw) & CLK_IS_BASIC)


Please try to avoid using CLK_IS_BASIC if at all possible. Can you?
Maybe add some flag in clk_hw_omap() instead?


hmm, Tero suggested that.
But to check flags in clk_hw_omap I first need to know that there is a
clk_hw_omap behind clk_hw. And for that I either need to check flags in
clk_hw or do more changes in the omap_hwmod code.


Can you do it? The omap code is the only user of CLK_IS_BASIC. All the
other users are marking clks with this but there is no reason to do so.
I'll go make another pass over the tree and nuke those ones from orbit.


The reason for using this flag is because OMAP uses two clock types 
around, the basic clocks like fixed-factor-clock/fixed-clock, and then 
all the omap derivatives, which can be cast to clk_hw_omap. If we want 
to avoid usage of CLK_IS_BASIC, we need to copy paste the remaining 
basic code under drivers/clk/ti/ and convert them to use clk_hw_omap as 
internal datatype. Is this preferred?


-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

[PATCH] arm64: dts: qcom: qcs404: Add pshold node

[Bjorn Andersson]

The pshold block is used to drive pshold towards the PMIC, which is used
to trigger a configurable event, such as reboot or poweroff of the
QCS404 platform. Add the necessary node to enable this functionality.

Signed-off-by: Bjorn Andersson 
---
 arch/arm64/boot/dts/qcom/qcs404.dtsi | 5 +
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/qcs404.dtsi 
b/arch/arm64/boot/dts/qcom/qcs404.dtsi
index 9b5c16562bbe..ce00fe678e5b 100644
--- a/arch/arm64/boot/dts/qcom/qcs404.dtsi
+++ b/arch/arm64/boot/dts/qcom/qcs404.dtsi
@@ -260,6 +260,11 @@
clock-names = "core";
};
 
+   pshold@4ab000 {
+   compatible = "qcom,pshold";
+   reg = <0x004ab000 0x1000>;
+   };
+
tlmm: pinctrl@100 {
compatible = "qcom,qcs404-pinctrl";
reg = <0x0100 0x20>,
-- 
2.18.0

[PATCH] arm64: dts: qcom: qcs404: Add pshold node

[Bjorn Andersson]

The pshold block is used to drive pshold towards the PMIC, which is used
to trigger a configurable event, such as reboot or poweroff of the
QCS404 platform. Add the necessary node to enable this functionality.

Signed-off-by: Bjorn Andersson 
---
 arch/arm64/boot/dts/qcom/qcs404.dtsi | 5 +
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/qcs404.dtsi 
b/arch/arm64/boot/dts/qcom/qcs404.dtsi
index 9b5c16562bbe..ce00fe678e5b 100644
--- a/arch/arm64/boot/dts/qcom/qcs404.dtsi
+++ b/arch/arm64/boot/dts/qcom/qcs404.dtsi
@@ -260,6 +260,11 @@
clock-names = "core";
};
 
+   pshold@4ab000 {
+   compatible = "qcom,pshold";
+   reg = <0x004ab000 0x1000>;
+   };
+
tlmm: pinctrl@100 {
compatible = "qcom,qcs404-pinctrl";
reg = <0x0100 0x20>,
-- 
2.18.0

Re: > [PATCH] Security: Handle hidepid option correctly

[程洋]

Andrew's question makes me think if this fix is superficial. Actually
i have had same question. But when i saw a smilar patch in kernel-4.4
was already merged in 2012, i decided to submit this patch first.

Here is the call stack i got:
[0.003450] [] proc_mount+0x2c/0x98
[0.003459] [] mount_fs+0x164/0x190
[0.003465] [] vfs_kern_mount+0x74/0x168
[0.003469] [] kern_mount_data+0x18/0x30
[0.003474] [] pid_ns_prepare_proc+0x24/0x40
[0.003484] [] alloc_pid+0x498/0x4b4
[0.003492] [] copy_process.isra.73.part.74+0xed0/0x1708
[0.003496] [] _do_fork+0xdc/0x3f8
[0.003501] [] kernel_thread+0x34/0x3c
[0.003511] [] rest_init+0x20/0x80
[0.003522] [] start_kernel+0x3e4/0x43c
[0.003527] [] __primary_switched+0x64/0x90
I notice only proc filesystem has function "pid_ns_prepare_proc".
there is no other "pid_ns_prepare_xxx" function in other filesystem.
Take the position of proc filesystem of kernel into consideration, the
answer of question "Other filesystems parse the options from
fill_super().  Is proc special in some fashion" could be "Yes, it is.
Because proc filesystem is special indeed. It's a filesystem kernel
will mount when it's booting".

But is it enough? Is anyone responsible for deinitialize sb->sroot?
Well actually i'm not an expert of filesystem, and don't unserstand
what does sb->s_root represent for. But i'm sure no one call
"pid_ns_release_proc" in the runtime(by add some logs). And even it is
called, it doesn't clean sb->s_root. Until now, i didn't see any
deeper issue. Maybe it's true that we should handle proc filesystem
specially.

If anyone who is sure about the functionality of sb->s_root and think
it should be handled in another way, feel free to correct me.


程洋  于2018年11月30日周五 上午10:34写道：
>> Here is an article illustrates the details.
> https://medium.com/@topjohnwu/from-anime-game-to-android-system-security-vulnerability-9b955a182f20
>
> And There is a similar fix on kernel-4.4:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=99663be772c827b8f5f594fe87eb4807be1994e5
>
> Q: Other filesystems parse the options from fill_super().  Is proc special in 
> some fashion?
> A: According to my research, start_kernel will call proc_mount first, and 
> initialize sb->s_root before any userspace process runs. If others want to 
> mount it, all options will be ignored.
>  AOSP change here: 
> https://android-review.googlesource.com/c/platform/system/core/+/181345/4/init/init.cpp
>  At first I though we should mount it with MS_REMOUNT flag. But kernel 
> will crash if we did this.
>
> Q:  Why is this considered to be security sensitive?  I can guess, but I'd 
> like to know your reasoning.
> A: See the article above. It's part of Android sanbox.
>
>
> > [PATCH] Security: Handle hidepid option correctly
>
> Why is this considered to be security sensitive?  I can guess, but I'd like 
> to know your reasoning.
>
> On Thu, 29 Nov 2018 19:08:21 +0800 mailto:d17103...@gmail.com wrote:
>
> > From: Cheng Yang 
> >
> > The proc_parse_options() call from proc_mount() runs only once at boot
> > time.  So on any later mount attempt, any mount options are ignored
> > because ->s_root is already initialized.
> > As a consequence, "mount -o " will ignore the options.  The
> > only way to change mount options is "mount -o remount,".
> > To fix this, parse the mount options unconditionally.
> >
> > --- a/fs/proc/inode.c
> > +++ b/fs/proc/inode.c
> > @@ -493,13 +493,9 @@ struct inode *proc_get_inode(struct super_block
> > *sb, struct proc_dir_entry *de)
> >
> >  int proc_fill_super(struct super_block *s, void *data, int silent)  {
> > -struct pid_namespace *ns = get_pid_ns(s->s_fs_info);
> >  struct inode *root_inode;
> >  int ret;
> >
> > -if (!proc_parse_options(data, ns))
> > -return -EINVAL;
> > -
> >  /* User space would break if executables or devices appear on proc */
> >  s->s_iflags |= SB_I_USERNS_VISIBLE | SB_I_NOEXEC | SB_I_NODEV;
> >  s->s_flags |= SB_NODIRATIME | SB_NOSUID | SB_NOEXEC; diff --git
> > a/fs/proc/root.c b/fs/proc/root.c index f4b1a9d..f5f3bf3 100644
> > --- a/fs/proc/root.c
> > +++ b/fs/proc/root.c
> > @@ -98,6 +98,9 @@ static struct dentry *proc_mount(struct file_system_type 
> > *fs_type,
> >  ns = task_active_pid_ns(current);
> >  }
> >
> > +if (!proc_parse_options(data, ns))
> > +return ERR_PTR(-EINVAL);
> > +
> >  return mount_ns(fs_type, flags, data, ns, ns->user_ns,
> > proc_fill_super);  }
>
> Other filesystems parse the options from fill_super().  Is proc special in 
> some fashion?
>
> #/**本邮件及其附件含有小米公司的保密信息，仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形式使用（包括但不限于全部或部分地泄露、复制、或散发）本邮件中的信息。如果您错收了本邮件，请您立即电话或邮件通知发件人并删除本邮件！
>  This e-mail and its attachments contain confidential information from 
> XIAOMI, which is intended only for the person or entity whose address is 
> listed above. Any use of the information contained herein in any way 
> (including, but not limited to, total or partial 

Re: > [PATCH] Security: Handle hidepid option correctly

[程洋]

Andrew's question makes me think if this fix is superficial. Actually
i have had same question. But when i saw a smilar patch in kernel-4.4
was already merged in 2012, i decided to submit this patch first.

Here is the call stack i got:
[0.003450] [] proc_mount+0x2c/0x98
[0.003459] [] mount_fs+0x164/0x190
[0.003465] [] vfs_kern_mount+0x74/0x168
[0.003469] [] kern_mount_data+0x18/0x30
[0.003474] [] pid_ns_prepare_proc+0x24/0x40
[0.003484] [] alloc_pid+0x498/0x4b4
[0.003492] [] copy_process.isra.73.part.74+0xed0/0x1708
[0.003496] [] _do_fork+0xdc/0x3f8
[0.003501] [] kernel_thread+0x34/0x3c
[0.003511] [] rest_init+0x20/0x80
[0.003522] [] start_kernel+0x3e4/0x43c
[0.003527] [] __primary_switched+0x64/0x90
I notice only proc filesystem has function "pid_ns_prepare_proc".
there is no other "pid_ns_prepare_xxx" function in other filesystem.
Take the position of proc filesystem of kernel into consideration, the
answer of question "Other filesystems parse the options from
fill_super().  Is proc special in some fashion" could be "Yes, it is.
Because proc filesystem is special indeed. It's a filesystem kernel
will mount when it's booting".

But is it enough? Is anyone responsible for deinitialize sb->sroot?
Well actually i'm not an expert of filesystem, and don't unserstand
what does sb->s_root represent for. But i'm sure no one call
"pid_ns_release_proc" in the runtime(by add some logs). And even it is
called, it doesn't clean sb->s_root. Until now, i didn't see any
deeper issue. Maybe it's true that we should handle proc filesystem
specially.

If anyone who is sure about the functionality of sb->s_root and think
it should be handled in another way, feel free to correct me.


程洋  于2018年11月30日周五 上午10:34写道：
>> Here is an article illustrates the details.
> https://medium.com/@topjohnwu/from-anime-game-to-android-system-security-vulnerability-9b955a182f20
>
> And There is a similar fix on kernel-4.4:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=99663be772c827b8f5f594fe87eb4807be1994e5
>
> Q: Other filesystems parse the options from fill_super().  Is proc special in 
> some fashion?
> A: According to my research, start_kernel will call proc_mount first, and 
> initialize sb->s_root before any userspace process runs. If others want to 
> mount it, all options will be ignored.
>  AOSP change here: 
> https://android-review.googlesource.com/c/platform/system/core/+/181345/4/init/init.cpp
>  At first I though we should mount it with MS_REMOUNT flag. But kernel 
> will crash if we did this.
>
> Q:  Why is this considered to be security sensitive?  I can guess, but I'd 
> like to know your reasoning.
> A: See the article above. It's part of Android sanbox.
>
>
> > [PATCH] Security: Handle hidepid option correctly
>
> Why is this considered to be security sensitive?  I can guess, but I'd like 
> to know your reasoning.
>
> On Thu, 29 Nov 2018 19:08:21 +0800 mailto:d17103...@gmail.com wrote:
>
> > From: Cheng Yang 
> >
> > The proc_parse_options() call from proc_mount() runs only once at boot
> > time.  So on any later mount attempt, any mount options are ignored
> > because ->s_root is already initialized.
> > As a consequence, "mount -o " will ignore the options.  The
> > only way to change mount options is "mount -o remount,".
> > To fix this, parse the mount options unconditionally.
> >
> > --- a/fs/proc/inode.c
> > +++ b/fs/proc/inode.c
> > @@ -493,13 +493,9 @@ struct inode *proc_get_inode(struct super_block
> > *sb, struct proc_dir_entry *de)
> >
> >  int proc_fill_super(struct super_block *s, void *data, int silent)  {
> > -struct pid_namespace *ns = get_pid_ns(s->s_fs_info);
> >  struct inode *root_inode;
> >  int ret;
> >
> > -if (!proc_parse_options(data, ns))
> > -return -EINVAL;
> > -
> >  /* User space would break if executables or devices appear on proc */
> >  s->s_iflags |= SB_I_USERNS_VISIBLE | SB_I_NOEXEC | SB_I_NODEV;
> >  s->s_flags |= SB_NODIRATIME | SB_NOSUID | SB_NOEXEC; diff --git
> > a/fs/proc/root.c b/fs/proc/root.c index f4b1a9d..f5f3bf3 100644
> > --- a/fs/proc/root.c
> > +++ b/fs/proc/root.c
> > @@ -98,6 +98,9 @@ static struct dentry *proc_mount(struct file_system_type 
> > *fs_type,
> >  ns = task_active_pid_ns(current);
> >  }
> >
> > +if (!proc_parse_options(data, ns))
> > +return ERR_PTR(-EINVAL);
> > +
> >  return mount_ns(fs_type, flags, data, ns, ns->user_ns,
> > proc_fill_super);  }
>
> Other filesystems parse the options from fill_super().  Is proc special in 
> some fashion?
>
> #/**本邮件及其附件含有小米公司的保密信息，仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形式使用（包括但不限于全部或部分地泄露、复制、或散发）本邮件中的信息。如果您错收了本邮件，请您立即电话或邮件通知发件人并删除本邮件！
>  This e-mail and its attachments contain confidential information from 
> XIAOMI, which is intended only for the person or entity whose address is 
> listed above. Any use of the information contained herein in any way 
> (including, but not limited to, total or partial 

[PATCH 1/2] dt-binding: remoteproc: Remove lpass_aon clock from adsp pil clock list

[Rohit kumar]

LPASS_Audio_Wrapper_AON clock is on by default. Remove
it from lpass clock list to avoid voting for it.

Signed-off-by: Rohit kumar 
---
 Documentation/devicetree/bindings/remoteproc/qcom,adsp-pil.txt | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,adsp-pil.txt 
b/Documentation/devicetree/bindings/remoteproc/qcom,adsp-pil.txt
index a842a78..66af2c3 100644
--- a/Documentation/devicetree/bindings/remoteproc/qcom,adsp-pil.txt
+++ b/Documentation/devicetree/bindings/remoteproc/qcom,adsp-pil.txt
@@ -35,7 +35,7 @@ on the Qualcomm Technology Inc. ADSP Hexagon core.
Value type: 
Definition: List of clock input name strings sorted in the same
order as the clocks property. Definition must have
-   "xo", "sway_cbcr", "lpass_aon", "lpass_ahbs_aon_cbcr",
+   "xo", "sway_cbcr", "lpass_ahbs_aon_cbcr",
"lpass_ahbm_aon_cbcr", "qdsp6ss_xo", "qdsp6ss_sleep"
and "qdsp6ss_core".
 
@@ -100,13 +100,12 @@ ADSP, as it is found on SDM845 boards.
 
clocks = < RPMH_CXO_CLK>,
< GCC_LPASS_SWAY_CLK>,
-   < LPASS_AUDIO_WRAPPER_AON_CLK>,
< LPASS_Q6SS_AHBS_AON_CLK>,
< LPASS_Q6SS_AHBM_AON_CLK>,
< LPASS_QDSP6SS_XO_CLK>,
< LPASS_QDSP6SS_SLEEP_CLK>,
< LPASS_QDSP6SS_CORE_CLK>;
-   clock-names = "xo", "sway_cbcr", "lpass_aon",
+   clock-names = "xo", "sway_cbcr",
"lpass_ahbs_aon_cbcr",
"lpass_ahbm_aon_cbcr", "qdsp6ss_xo",
"qdsp6ss_sleep", "qdsp6ss_core";
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH 2/2] remoteproc: q6v5_adsp: Remove voting for lpass_aon clock

[Rohit kumar]

Lpass_aon clock is on by default. Remove it from lpass
clock list to avoid voting for it.

Signed-off-by: Rohit kumar 
---
 drivers/remoteproc/qcom_q6v5_adsp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c 
b/drivers/remoteproc/qcom_q6v5_adsp.c
index 79374d1..4829173 100644
--- a/drivers/remoteproc/qcom_q6v5_adsp.c
+++ b/drivers/remoteproc/qcom_q6v5_adsp.c
@@ -48,7 +48,7 @@
 
 /* list of clocks required by ADSP PIL */
 static const char * const adsp_clk_id[] = {
-   "sway_cbcr", "lpass_aon", "lpass_ahbs_aon_cbcr", "lpass_ahbm_aon_cbcr",
+   "sway_cbcr", "lpass_ahbs_aon_cbcr", "lpass_ahbm_aon_cbcr",
"qdsp6ss_xo", "qdsp6ss_sleep", "qdsp6ss_core",
 };
 
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH 1/2] dt-binding: remoteproc: Remove lpass_aon clock from adsp pil clock list

[Rohit kumar]

LPASS_Audio_Wrapper_AON clock is on by default. Remove
it from lpass clock list to avoid voting for it.

Signed-off-by: Rohit kumar 
---
 Documentation/devicetree/bindings/remoteproc/qcom,adsp-pil.txt | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,adsp-pil.txt 
b/Documentation/devicetree/bindings/remoteproc/qcom,adsp-pil.txt
index a842a78..66af2c3 100644
--- a/Documentation/devicetree/bindings/remoteproc/qcom,adsp-pil.txt
+++ b/Documentation/devicetree/bindings/remoteproc/qcom,adsp-pil.txt
@@ -35,7 +35,7 @@ on the Qualcomm Technology Inc. ADSP Hexagon core.
Value type: 
Definition: List of clock input name strings sorted in the same
order as the clocks property. Definition must have
-   "xo", "sway_cbcr", "lpass_aon", "lpass_ahbs_aon_cbcr",
+   "xo", "sway_cbcr", "lpass_ahbs_aon_cbcr",
"lpass_ahbm_aon_cbcr", "qdsp6ss_xo", "qdsp6ss_sleep"
and "qdsp6ss_core".
 
@@ -100,13 +100,12 @@ ADSP, as it is found on SDM845 boards.
 
clocks = < RPMH_CXO_CLK>,
< GCC_LPASS_SWAY_CLK>,
-   < LPASS_AUDIO_WRAPPER_AON_CLK>,
< LPASS_Q6SS_AHBS_AON_CLK>,
< LPASS_Q6SS_AHBM_AON_CLK>,
< LPASS_QDSP6SS_XO_CLK>,
< LPASS_QDSP6SS_SLEEP_CLK>,
< LPASS_QDSP6SS_CORE_CLK>;
-   clock-names = "xo", "sway_cbcr", "lpass_aon",
+   clock-names = "xo", "sway_cbcr",
"lpass_ahbs_aon_cbcr",
"lpass_ahbm_aon_cbcr", "qdsp6ss_xo",
"qdsp6ss_sleep", "qdsp6ss_core";
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH 2/2] remoteproc: q6v5_adsp: Remove voting for lpass_aon clock

[Rohit kumar]

Lpass_aon clock is on by default. Remove it from lpass
clock list to avoid voting for it.

Signed-off-by: Rohit kumar 
---
 drivers/remoteproc/qcom_q6v5_adsp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c 
b/drivers/remoteproc/qcom_q6v5_adsp.c
index 79374d1..4829173 100644
--- a/drivers/remoteproc/qcom_q6v5_adsp.c
+++ b/drivers/remoteproc/qcom_q6v5_adsp.c
@@ -48,7 +48,7 @@
 
 /* list of clocks required by ADSP PIL */
 static const char * const adsp_clk_id[] = {
-   "sway_cbcr", "lpass_aon", "lpass_ahbs_aon_cbcr", "lpass_ahbm_aon_cbcr",
+   "sway_cbcr", "lpass_ahbs_aon_cbcr", "lpass_ahbm_aon_cbcr",
"qdsp6ss_xo", "qdsp6ss_sleep", "qdsp6ss_core",
 };
 
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH 0/2] qcom_adsp_pil: Remove voting for lpass_aon clock

[Rohit kumar]

LPASS Audio Wrapper AON clock is on by default. Remove
voting for it.

Rohit kumar (2):
  dt-binding: remoteproc: Remove lpass_aon clock from adsp pil clock
list
  remoteproc: q6v5_adsp: Remove voting for lpass_aon clock

 Documentation/devicetree/bindings/remoteproc/qcom,adsp-pil.txt | 5 ++---
 drivers/remoteproc/qcom_q6v5_adsp.c| 2 +-
 2 files changed, 3 insertions(+), 4 deletions(-)

-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH 0/2] qcom_adsp_pil: Remove voting for lpass_aon clock

[Rohit kumar]

LPASS Audio Wrapper AON clock is on by default. Remove
voting for it.

Rohit kumar (2):
  dt-binding: remoteproc: Remove lpass_aon clock from adsp pil clock
list
  remoteproc: q6v5_adsp: Remove voting for lpass_aon clock

 Documentation/devicetree/bindings/remoteproc/qcom,adsp-pil.txt | 5 ++---
 drivers/remoteproc/qcom_q6v5_adsp.c| 2 +-
 2 files changed, 3 insertions(+), 4 deletions(-)

-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH 2/3] clk: qcom: gcc-msm8998: Disable halt check of UFS clocks

[Bjorn Andersson]

On Thu 29 Nov 23:06 PST 2018, Stephen Boyd wrote:

> Quoting Bjorn Andersson (2018-11-29 22:52:58)
> > Drop the halt check of the UFS symbol clocks, in accordance with other
> > platforms. This makes clk_disable_unused() happy and makes it possible
> > to turn the clocks on again without an error.
> > 
> > Signed-off-by: Bjorn Andersson 
> 
> Someone was supposed to figure out why we needed to SKIP here instead of
> doing things in the proper order. Is anyone attempting to figure that
> out?
> 

I'm not aware of any such efforts, but looping in Vivek here.

I would be happy to revert this change in 8996, 8998 and 845 once such
fix arrives. But as this is needed to make progress on getting UFS up on
8998 it would be nice to get it merged for now...

Regards,
Bjorn

Re: [PATCH 2/3] clk: qcom: gcc-msm8998: Disable halt check of UFS clocks

[Bjorn Andersson]

On Thu 29 Nov 23:06 PST 2018, Stephen Boyd wrote:

> Quoting Bjorn Andersson (2018-11-29 22:52:58)
> > Drop the halt check of the UFS symbol clocks, in accordance with other
> > platforms. This makes clk_disable_unused() happy and makes it possible
> > to turn the clocks on again without an error.
> > 
> > Signed-off-by: Bjorn Andersson 
> 
> Someone was supposed to figure out why we needed to SKIP here instead of
> doing things in the proper order. Is anyone attempting to figure that
> out?
> 

I'm not aware of any such efforts, but looping in Vivek here.

I would be happy to revert this change in 8996, 8998 and 845 once such
fix arrives. But as this is needed to make progress on getting UFS up on
8998 it would be nice to get it merged for now...

Regards,
Bjorn

Re: [PATCH] clk: imx6sl: ensure MMDC CH0 handshake is bypassed

[Stephen Boyd]

Quoting Anson Huang (2018-11-29 23:23:47)
> Same as other i.MX6 SoCs, ensure unused MMDC channel's
> handshake is bypassed, this is to make sure no request
> signal will be generated when periphe_clk_sel is changed
> or SRC warm reset is triggered.
> 
> Signed-off-by: Anson Huang 

Does this need a fixes tag?

Re: [PATCH] clk: imx6sl: ensure MMDC CH0 handshake is bypassed

[Stephen Boyd]

Quoting Anson Huang (2018-11-29 23:23:47)
> Same as other i.MX6 SoCs, ensure unused MMDC channel's
> handshake is bypassed, this is to make sure no request
> signal will be generated when periphe_clk_sel is changed
> or SRC warm reset is triggered.
> 
> Signed-off-by: Anson Huang 

Does this need a fixes tag?

Re: [PATCH 1/3] clk: qcom: gcc-msm8998: Mark hmss_dvm and lpass_at critical

[Bjorn Andersson]

On Thu 29 Nov 23:05 PST 2018, Stephen Boyd wrote:

> Quoting Bjorn Andersson (2018-11-29 22:52:57)
> > Keep the two clocks enabled, so that the platform passes
> > clk_disable_unused().
> > 
> > Signed-off-by: Bjorn Andersson 
> > ---
> >  drivers/clk/qcom/gcc-msm8998.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/clk/qcom/gcc-msm8998.c b/drivers/clk/qcom/gcc-msm8998.c
> > index 9f0ae403d5f5..d89f8e7c2a59 100644
> > --- a/drivers/clk/qcom/gcc-msm8998.c
> > +++ b/drivers/clk/qcom/gcc-msm8998.c
> > @@ -1972,6 +1972,7 @@ static struct clk_branch gcc_hmss_dvm_bus_clk = {
> > .enable_mask = BIT(0),
> > .hw.init = &(struct clk_init_data){
> > .name = "gcc_hmss_dvm_bus_clk",
> > +   .flags = CLK_IS_CRITICAL,
> 
> Please add a comment about why they're critical. This is a temporary
> solution?
> 

Disabling them in clk_disable_unused() are bad, mkay...


SDM845 marks the equivalent clocks as critical with a comment that they
must be on for system operation... I'm uncertain what the exact purpose
of these two clocks are, so I don't have a better explanation right now.

Regards,
Bjorn

> > .ops = _branch2_ops,
> > },
> > },

Re: [PATCH 1/3] clk: qcom: gcc-msm8998: Mark hmss_dvm and lpass_at critical

[Bjorn Andersson]

On Thu 29 Nov 23:05 PST 2018, Stephen Boyd wrote:

> Quoting Bjorn Andersson (2018-11-29 22:52:57)
> > Keep the two clocks enabled, so that the platform passes
> > clk_disable_unused().
> > 
> > Signed-off-by: Bjorn Andersson 
> > ---
> >  drivers/clk/qcom/gcc-msm8998.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/clk/qcom/gcc-msm8998.c b/drivers/clk/qcom/gcc-msm8998.c
> > index 9f0ae403d5f5..d89f8e7c2a59 100644
> > --- a/drivers/clk/qcom/gcc-msm8998.c
> > +++ b/drivers/clk/qcom/gcc-msm8998.c
> > @@ -1972,6 +1972,7 @@ static struct clk_branch gcc_hmss_dvm_bus_clk = {
> > .enable_mask = BIT(0),
> > .hw.init = &(struct clk_init_data){
> > .name = "gcc_hmss_dvm_bus_clk",
> > +   .flags = CLK_IS_CRITICAL,
> 
> Please add a comment about why they're critical. This is a temporary
> solution?
> 

Disabling them in clk_disable_unused() are bad, mkay...


SDM845 marks the equivalent clocks as critical with a comment that they
must be on for system operation... I'm uncertain what the exact purpose
of these two clocks are, so I don't have a better explanation right now.

Regards,
Bjorn

> > .ops = _branch2_ops,
> > },
> > },

[PATCH] clk: imx6sl: ensure MMDC CH0 handshake is bypassed

[Anson Huang]

Same as other i.MX6 SoCs, ensure unused MMDC channel's
handshake is bypassed, this is to make sure no request
signal will be generated when periphe_clk_sel is changed
or SRC warm reset is triggered.

Signed-off-by: Anson Huang 
---
 drivers/clk/imx/clk-imx6sl.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/clk/imx/clk-imx6sl.c b/drivers/clk/imx/clk-imx6sl.c
index 6fcfbbd..e13d881 100644
--- a/drivers/clk/imx/clk-imx6sl.c
+++ b/drivers/clk/imx/clk-imx6sl.c
@@ -17,6 +17,8 @@
 
 #include "clk.h"
 
+#define CCDR   0x4
+#define BM_CCM_CCDR_MMDC_CH0_MASK  (1 << 17)
 #define CCSR   0xc
 #define BM_CCSR_PLL1_SW_CLK_SEL(1 << 2)
 #define CACRR  0x10
@@ -411,6 +413,10 @@ static void __init imx6sl_clocks_init(struct device_node 
*ccm_node)
clks[IMX6SL_CLK_USDHC3]   = imx_clk_gate2("usdhc3",   
"usdhc3_podf",   base + 0x80, 6);
clks[IMX6SL_CLK_USDHC4]   = imx_clk_gate2("usdhc4",   
"usdhc4_podf",   base + 0x80, 8);
 
+   /* Ensure the MMDC CH0 handshake is bypassed */
+   writel_relaxed(readl_relaxed(base + CCDR) |
+   BM_CCM_CCDR_MMDC_CH0_MASK, base + CCDR);
+
imx_check_clocks(clks, ARRAY_SIZE(clks));
 
clk_data.clks = clks;
-- 
2.7.4

[PATCH] clk: imx6sl: ensure MMDC CH0 handshake is bypassed

[Anson Huang]

Same as other i.MX6 SoCs, ensure unused MMDC channel's
handshake is bypassed, this is to make sure no request
signal will be generated when periphe_clk_sel is changed
or SRC warm reset is triggered.

Signed-off-by: Anson Huang 
---
 drivers/clk/imx/clk-imx6sl.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/clk/imx/clk-imx6sl.c b/drivers/clk/imx/clk-imx6sl.c
index 6fcfbbd..e13d881 100644
--- a/drivers/clk/imx/clk-imx6sl.c
+++ b/drivers/clk/imx/clk-imx6sl.c
@@ -17,6 +17,8 @@
 
 #include "clk.h"
 
+#define CCDR   0x4
+#define BM_CCM_CCDR_MMDC_CH0_MASK  (1 << 17)
 #define CCSR   0xc
 #define BM_CCSR_PLL1_SW_CLK_SEL(1 << 2)
 #define CACRR  0x10
@@ -411,6 +413,10 @@ static void __init imx6sl_clocks_init(struct device_node 
*ccm_node)
clks[IMX6SL_CLK_USDHC3]   = imx_clk_gate2("usdhc3",   
"usdhc3_podf",   base + 0x80, 6);
clks[IMX6SL_CLK_USDHC4]   = imx_clk_gate2("usdhc4",   
"usdhc4_podf",   base + 0x80, 8);
 
+   /* Ensure the MMDC CH0 handshake is bypassed */
+   writel_relaxed(readl_relaxed(base + CCDR) |
+   BM_CCM_CCDR_MMDC_CH0_MASK, base + CCDR);
+
imx_check_clocks(clks, ARRAY_SIZE(clks));
 
clk_data.clks = clks;
-- 
2.7.4

Re: [PATCH v2 2/3] clk: ti: check clock type before doing autoidle ops

[Stephen Boyd]

Quoting Andreas Kemnade (2018-11-29 22:15:34)
> Hi Stephen,
> 
> On Thu, 29 Nov 2018 16:25:05 -0800
> Stephen Boyd  wrote:
> 
> > Quoting Andreas Kemnade (2018-11-10 12:31:14)
> > > Code might use autoidle api with clocks not being omap2 clocks,
> > > so check if clock type is not basic
> > > 
> > > Signed-off-by: Andreas Kemnade 
> > > ---
> > > New in v2
> > > ---
> > >  drivers/clk/ti/autoidle.c | 12 ++--
> > >  1 file changed, 10 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/clk/ti/autoidle.c b/drivers/clk/ti/autoidle.c
> > > index 161f67850393..5bdae5552d38 100644
> > > --- a/drivers/clk/ti/autoidle.c
> > > +++ b/drivers/clk/ti/autoidle.c
> > > @@ -54,8 +54,12 @@ static DEFINE_SPINLOCK(autoidle_spinlock);
> > >  int omap2_clk_deny_idle(struct clk *clk)
> > >  {
> > > struct clk_hw_omap *c;
> > > +   struct clk_hw *hw = __clk_get_hw(clk);
> > >  
> > > -   c = to_clk_hw_omap(__clk_get_hw(clk));
> > > +   if (clk_hw_get_flags(hw) & CLK_IS_BASIC)  
> > 
> > Please try to avoid using CLK_IS_BASIC if at all possible. Can you?
> > Maybe add some flag in clk_hw_omap() instead?
> > 
> hmm, Tero suggested that.
> But to check flags in clk_hw_omap I first need to know that there is a
> clk_hw_omap behind clk_hw. And for that I either need to check flags in
> clk_hw or do more changes in the omap_hwmod code.

Can you do it? The omap code is the only user of CLK_IS_BASIC. All the
other users are marking clks with this but there is no reason to do so.
I'll go make another pass over the tree and nuke those ones from orbit.

Re: [PATCH v2 2/3] clk: ti: check clock type before doing autoidle ops

[Stephen Boyd]

Quoting Andreas Kemnade (2018-11-29 22:15:34)
> Hi Stephen,
> 
> On Thu, 29 Nov 2018 16:25:05 -0800
> Stephen Boyd  wrote:
> 
> > Quoting Andreas Kemnade (2018-11-10 12:31:14)
> > > Code might use autoidle api with clocks not being omap2 clocks,
> > > so check if clock type is not basic
> > > 
> > > Signed-off-by: Andreas Kemnade 
> > > ---
> > > New in v2
> > > ---
> > >  drivers/clk/ti/autoidle.c | 12 ++--
> > >  1 file changed, 10 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/clk/ti/autoidle.c b/drivers/clk/ti/autoidle.c
> > > index 161f67850393..5bdae5552d38 100644
> > > --- a/drivers/clk/ti/autoidle.c
> > > +++ b/drivers/clk/ti/autoidle.c
> > > @@ -54,8 +54,12 @@ static DEFINE_SPINLOCK(autoidle_spinlock);
> > >  int omap2_clk_deny_idle(struct clk *clk)
> > >  {
> > > struct clk_hw_omap *c;
> > > +   struct clk_hw *hw = __clk_get_hw(clk);
> > >  
> > > -   c = to_clk_hw_omap(__clk_get_hw(clk));
> > > +   if (clk_hw_get_flags(hw) & CLK_IS_BASIC)  
> > 
> > Please try to avoid using CLK_IS_BASIC if at all possible. Can you?
> > Maybe add some flag in clk_hw_omap() instead?
> > 
> hmm, Tero suggested that.
> But to check flags in clk_hw_omap I first need to know that there is a
> clk_hw_omap behind clk_hw. And for that I either need to check flags in
> clk_hw or do more changes in the omap_hwmod code.

Can you do it? The omap code is the only user of CLK_IS_BASIC. All the
other users are marking clks with this but there is no reason to do so.
I'll go make another pass over the tree and nuke those ones from orbit.

Re: [PATCH 2/3] dt-bindings: mmc: sdhci-of-arasan: Add deprecated message for am654

[Michal Simek]

On 29. 11. 18 17:15, Faiz Abbas wrote:
> The "ti,am654-sdhci-5.1" binding has been moved to a new driver. Indicate
> this by a deprecated message.
> 
> Signed-off-by: Faiz Abbas 
> ---
>  Documentation/devicetree/bindings/mmc/arasan,sdhci.txt | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt 
> b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> index e2effe17f05e..1edbb049cccb 100644
> --- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> +++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> @@ -16,6 +16,10 @@ Required Properties:
>  - "rockchip,rk3399-sdhci-5.1", "arasan,sdhci-5.1": rk3399 eMMC PHY
>For this device it is strongly suggested to include 
> arasan,soc-ctl-syscon.
>  - "ti,am654-sdhci-5.1", "arasan,sdhci-5.1": TI AM654 MMC PHY
> + Note: This binding has been deprecated and moved to [5].
> +
> +  [5] Documentation/devicetree/bindings/mmc/sdhci-am654.txt
> +
>- reg: From mmc bindings: Register location and length.
>- clocks: From clock bindings: Handles to clock inputs.
>- clock-names: From clock bindings: Tuple including "clk_xin" and "clk_ahb"
> 

I would prefer if you can extend commit message by description you have
in cover letter which explains reasons. When this is applied and someone
look at git history none will understand from first read why you have
done that.

Thanks,
Michal

Re: [PATCH 2/3] dt-bindings: mmc: sdhci-of-arasan: Add deprecated message for am654

[Michal Simek]

On 29. 11. 18 17:15, Faiz Abbas wrote:
> The "ti,am654-sdhci-5.1" binding has been moved to a new driver. Indicate
> this by a deprecated message.
> 
> Signed-off-by: Faiz Abbas 
> ---
>  Documentation/devicetree/bindings/mmc/arasan,sdhci.txt | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt 
> b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> index e2effe17f05e..1edbb049cccb 100644
> --- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> +++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> @@ -16,6 +16,10 @@ Required Properties:
>  - "rockchip,rk3399-sdhci-5.1", "arasan,sdhci-5.1": rk3399 eMMC PHY
>For this device it is strongly suggested to include 
> arasan,soc-ctl-syscon.
>  - "ti,am654-sdhci-5.1", "arasan,sdhci-5.1": TI AM654 MMC PHY
> + Note: This binding has been deprecated and moved to [5].
> +
> +  [5] Documentation/devicetree/bindings/mmc/sdhci-am654.txt
> +
>- reg: From mmc bindings: Register location and length.
>- clocks: From clock bindings: Handles to clock inputs.
>- clock-names: From clock bindings: Tuple including "clk_xin" and "clk_ahb"
> 

I would prefer if you can extend commit message by description you have
in cover letter which explains reasons. When this is applied and someone
look at git history none will understand from first read why you have
done that.

Thanks,
Michal

Re: [PATCH 4.19 000/110] 4.19.6-stable review

[Greg Kroah-Hartman]

On Fri, Nov 30, 2018 at 03:54:48AM +0530, Harsh Shandilya wrote:
> On 29 November 2018 7:41:31 PM IST, Greg Kroah-Hartman 
>  wrote:
> >This is the start of the stable review cycle for the 4.19.6 release.
> >There are 110 patches in this series, all will be posted as a response
> >to this one.  If anyone has any issues with these being applied, please
> >let me know.
> >
> >Responses should be made by Sat Dec  1 13:58:54 UTC 2018.
> >Anything received after that time might be too late.
> >
> >The whole patch series can be found in one patch at:
> > 
> > https://www.kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.19.6-rc1.gz
> >or in the git tree and branch at:
> > git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git
> >linux-4.19.y
> >and the diffstat can be found below.
> >
> >thanks,
> >
> >greg k-h
> Built and booted on the Lenovo IdeaPad 330-15ARR, no dmesg regressions.

Thanks for testing these and letting me know.

> P.S. My GCC loving friend Gabe said hi :p

People love gcc?  :)

greg k-h

Re: [PATCH 4.19 000/110] 4.19.6-stable review

[Greg Kroah-Hartman]

On Fri, Nov 30, 2018 at 03:54:48AM +0530, Harsh Shandilya wrote:
> On 29 November 2018 7:41:31 PM IST, Greg Kroah-Hartman 
>  wrote:
> >This is the start of the stable review cycle for the 4.19.6 release.
> >There are 110 patches in this series, all will be posted as a response
> >to this one.  If anyone has any issues with these being applied, please
> >let me know.
> >
> >Responses should be made by Sat Dec  1 13:58:54 UTC 2018.
> >Anything received after that time might be too late.
> >
> >The whole patch series can be found in one patch at:
> > 
> > https://www.kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.19.6-rc1.gz
> >or in the git tree and branch at:
> > git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git
> >linux-4.19.y
> >and the diffstat can be found below.
> >
> >thanks,
> >
> >greg k-h
> Built and booted on the Lenovo IdeaPad 330-15ARR, no dmesg regressions.

Thanks for testing these and letting me know.

> P.S. My GCC loving friend Gabe said hi :p

People love gcc?  :)

greg k-h

Re: [PATCH 4.19 000/110] 4.19.6-stable review

[Greg Kroah-Hartman]

On Thu, Nov 29, 2018 at 01:36:14PM -0700, shuah wrote:
> On 11/29/18 7:11 AM, Greg Kroah-Hartman wrote:
> > This is the start of the stable review cycle for the 4.19.6 release.
> > There are 110 patches in this series, all will be posted as a response
> > to this one.  If anyone has any issues with these being applied, please
> > let me know.
> > 
> > Responses should be made by Sat Dec  1 13:58:54 UTC 2018.
> > Anything received after that time might be too late.
> > 
> > The whole patch series can be found in one patch at:
> > 
> > https://www.kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.19.6-rc1.gz
> > or in the git tree and branch at:
> > 
> > git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git 
> > linux-4.19.y
> > and the diffstat can be found below.
> > 
> > thanks,
> > 
> > greg k-h
> > 
> 
> Compiled and booted on my test system. No dmesg regressions.

Thanks for testing all of these and letting me know.

greg k-h

Re: [PATCH 4.19 000/110] 4.19.6-stable review

[Greg Kroah-Hartman]

On Thu, Nov 29, 2018 at 01:36:14PM -0700, shuah wrote:
> On 11/29/18 7:11 AM, Greg Kroah-Hartman wrote:
> > This is the start of the stable review cycle for the 4.19.6 release.
> > There are 110 patches in this series, all will be posted as a response
> > to this one.  If anyone has any issues with these being applied, please
> > let me know.
> > 
> > Responses should be made by Sat Dec  1 13:58:54 UTC 2018.
> > Anything received after that time might be too late.
> > 
> > The whole patch series can be found in one patch at:
> > 
> > https://www.kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.19.6-rc1.gz
> > or in the git tree and branch at:
> > 
> > git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git 
> > linux-4.19.y
> > and the diffstat can be found below.
> > 
> > thanks,
> > 
> > greg k-h
> > 
> 
> Compiled and booted on my test system. No dmesg regressions.

Thanks for testing all of these and letting me know.

greg k-h

Re: [PATCH 4.14 000/100] 4.14.85-stable review

[Naresh Kamboju]

On Thu, 29 Nov 2018 at 19:56, Greg Kroah-Hartman
 wrote:
>
> This is the start of the stable review cycle for the 4.14.85 release.
> There are 100 patches in this series, all will be posted as a response
> to this one.  If anyone has any issues with these being applied, please
> let me know.
>
> Responses should be made by Sat Dec  1 14:00:29 UTC 2018.
> Anything received after that time might be too late.
>
> The whole patch series can be found in one patch at:
> 
> https://www.kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.14.85-rc1.gz
> or in the git tree and branch at:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git 
> linux-4.14.y
> and the diffstat can be found below.
>
> thanks,
>
> greg k-h

Results from Linaro’s test farm.
No regressions on arm64, arm, x86_64, and i386.

Summary


kernel: 4.14.85-rc1
git repo: 
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git
git branch: linux-4.14.y
git commit: ae0375de4a2b34514e4e7934342b476788d9e4f6
git describe: v4.14.84-100-gae0375de4a2b
Test details: 
https://qa-reports.linaro.org/lkft/linux-stable-rc-4.14-oe/build/v4.14.84-100-gae0375de4a2b

No regressions (compared to build v4.14.84)

No fixes (compared to build v4.14.84)

Ran 21425 total tests in the following environments and test suites.

Environments
--
- dragonboard-410c - arm64
- hi6220-hikey - arm64
- i386
- juno-r2 - arm64
- qemu_arm
- qemu_arm64
- qemu_i386
- qemu_x86_64
- x15 - arm
- x86_64

Test Suites
---
* boot
* install-android-platform-tools-r2600
* kselftest
* libhugetlbfs
* ltp-containers-tests
* ltp-cve-tests
* ltp-fcntl-locktests-tests
* ltp-fs-tests
* ltp-fs_bind-tests
* ltp-fs_perms_simple-tests
* ltp-fsx-tests
* ltp-hugetlb-tests
* ltp-io-tests
* ltp-ipc-tests
* ltp-math-tests
* ltp-nptl-tests
* ltp-pty-tests
* ltp-sched-tests
* ltp-securebits-tests
* ltp-syscalls-tests
* ltp-cap_bounds-tests
* ltp-filecaps-tests
* ltp-timers-tests
* ltp-open-posix-tests
* kselftest-vsyscall-mode-native
* kselftest-vsyscall-mode-none

-- 
Linaro LKFT
https://lkft.linaro.org

Re: [PATCH 4.14 000/100] 4.14.85-stable review

[Naresh Kamboju]

On Thu, 29 Nov 2018 at 19:56, Greg Kroah-Hartman
 wrote:
>
> This is the start of the stable review cycle for the 4.14.85 release.
> There are 100 patches in this series, all will be posted as a response
> to this one.  If anyone has any issues with these being applied, please
> let me know.
>
> Responses should be made by Sat Dec  1 14:00:29 UTC 2018.
> Anything received after that time might be too late.
>
> The whole patch series can be found in one patch at:
> 
> https://www.kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.14.85-rc1.gz
> or in the git tree and branch at:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git 
> linux-4.14.y
> and the diffstat can be found below.
>
> thanks,
>
> greg k-h

Results from Linaro’s test farm.
No regressions on arm64, arm, x86_64, and i386.

Summary


kernel: 4.14.85-rc1
git repo: 
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git
git branch: linux-4.14.y
git commit: ae0375de4a2b34514e4e7934342b476788d9e4f6
git describe: v4.14.84-100-gae0375de4a2b
Test details: 
https://qa-reports.linaro.org/lkft/linux-stable-rc-4.14-oe/build/v4.14.84-100-gae0375de4a2b

No regressions (compared to build v4.14.84)

No fixes (compared to build v4.14.84)

Ran 21425 total tests in the following environments and test suites.

Environments
--
- dragonboard-410c - arm64
- hi6220-hikey - arm64
- i386
- juno-r2 - arm64
- qemu_arm
- qemu_arm64
- qemu_i386
- qemu_x86_64
- x15 - arm
- x86_64

Test Suites
---
* boot
* install-android-platform-tools-r2600
* kselftest
* libhugetlbfs
* ltp-containers-tests
* ltp-cve-tests
* ltp-fcntl-locktests-tests
* ltp-fs-tests
* ltp-fs_bind-tests
* ltp-fs_perms_simple-tests
* ltp-fsx-tests
* ltp-hugetlb-tests
* ltp-io-tests
* ltp-ipc-tests
* ltp-math-tests
* ltp-nptl-tests
* ltp-pty-tests
* ltp-sched-tests
* ltp-securebits-tests
* ltp-syscalls-tests
* ltp-cap_bounds-tests
* ltp-filecaps-tests
* ltp-timers-tests
* ltp-open-posix-tests
* kselftest-vsyscall-mode-native
* kselftest-vsyscall-mode-none

-- 
Linaro LKFT
https://lkft.linaro.org

Re: [RFC v3 2/3] dt-bindings: sdm845-pinctrl: add wakeup interrupt parent for GPIO

[Bjorn Andersson]

On Thu 29 Nov 13:45 PST 2018, Lina Iyer wrote:

> On Wed, Nov 28 2018 at 17:25 -0700, Bjorn Andersson wrote:
> > On Wed 28 Nov 09:39 PST 2018, Lina Iyer wrote:
> > 
> > > On Tue, Nov 27 2018 at 14:45 -0700, Stephen Boyd wrote:
> > > > Quoting Lina Iyer (2018-11-27 10:21:23)
> > > > > On Tue, Nov 27 2018 at 02:12 -0700, Stephen Boyd wrote:
> > > > > >
> > > > > >Two reasons. First, simplicity. The TLMM driver just needs to pass 
> > > > > >the
> > > > > >gpio number up to the PDC gpio domain and then that domain can figure
> > > > > >out what hwirq it maps to within the PDC hw irq space. I don't see 
> > > > > >any
> > > > > >reason why we have to know the hwirq of PDC within the TLMM driver
> > > > > >besides "let's not be different".
> > > > > >
> > > > > >And second, it makes it easier for us to implement the MPM case in 
> > > > > >the
> > > > > >TLMM driver by letting the TLMM code just ask "should I mask the irq
> > > > > >here or not?" by passing that with a wrapper struct around the fwspec
> > > > > >and a dedicated domain in the PDC/MPM driver. Keeping less things in 
> > > > > >the
> > > > > >TLMM driver and not driving the decision from DT but from tables in 
> > > > > >the
> > > > > >PDC driver also keeps things simple and reduces DT parsing code/time.
> > > > > >
> > > > > Couldn't this be simply achieved by matching the compatible flags for
> > > > > PDC/MPM bindings for the wakeup-parent in the TLMM driver?
> > > > >
> > > >
> > > > It could be, but then we would be making TLMM highly aware of the wakeup
> > > > parent
> > > It is is not aware of anything about the wakeup parent, just the
> > > compatible that indicates whether it needs to be mask locally or not.
> > > 
> > 
> > But the information related to how the PDC is wired to GPIO pins is a
> > property related to the PDC not of the TLMM, so it does make sense to
> > represent this information there.
> > 
> Hmm.. But we are inconsistent in what we provide as input into the PDC
> driver.
> From the PDC driver perspective, it gets a hwirq number either when
> driver requests a wakeup interrupt specified in DT as
>   < 5 IRQ_TYPE_LEVEL_HIGH>
> or from GPIO, which translates the GPIO to the PDC hwirq and requests
> that.
> 

I see what you're saying, but need to think some more about this...

> > And iiuc it also makes sense to not encode it in DT because it's
> > physical wiring, not configuration.
> > 
> I would agree.
> 
> > > > and have to do compatible string matching in two places, instead
> > > > of making TLMM more abstractly aware that it needs to keep things masked
> > > > while irq parent deals with the interrupts.
> > > >
> > > Your approach seems too complex just to circumvent a simple match node.
> > > I think we are stretching too much to support something that is not a
> > > priority.
> > > 
> > 
> > What "something" are you referring to here?
> > 
> MPM :)
> 

There are still new platforms coming out with MPM, so there's even a
business case to care about it.

> BTW, I am discussing with the internal folks here on if we need to mask
> TLMM when the wakeup-parent is MPM. If we don't have to, we should be
> able to follow the same model as we done in this patch and don't even
> have to check the compatible or use the approach suggested by Stephen.
> 

Looking forward to the result of that discussion.

Regards,
Bjorn

Re: [RFC v3 2/3] dt-bindings: sdm845-pinctrl: add wakeup interrupt parent for GPIO

[Bjorn Andersson]

On Thu 29 Nov 13:45 PST 2018, Lina Iyer wrote:

> On Wed, Nov 28 2018 at 17:25 -0700, Bjorn Andersson wrote:
> > On Wed 28 Nov 09:39 PST 2018, Lina Iyer wrote:
> > 
> > > On Tue, Nov 27 2018 at 14:45 -0700, Stephen Boyd wrote:
> > > > Quoting Lina Iyer (2018-11-27 10:21:23)
> > > > > On Tue, Nov 27 2018 at 02:12 -0700, Stephen Boyd wrote:
> > > > > >
> > > > > >Two reasons. First, simplicity. The TLMM driver just needs to pass 
> > > > > >the
> > > > > >gpio number up to the PDC gpio domain and then that domain can figure
> > > > > >out what hwirq it maps to within the PDC hw irq space. I don't see 
> > > > > >any
> > > > > >reason why we have to know the hwirq of PDC within the TLMM driver
> > > > > >besides "let's not be different".
> > > > > >
> > > > > >And second, it makes it easier for us to implement the MPM case in 
> > > > > >the
> > > > > >TLMM driver by letting the TLMM code just ask "should I mask the irq
> > > > > >here or not?" by passing that with a wrapper struct around the fwspec
> > > > > >and a dedicated domain in the PDC/MPM driver. Keeping less things in 
> > > > > >the
> > > > > >TLMM driver and not driving the decision from DT but from tables in 
> > > > > >the
> > > > > >PDC driver also keeps things simple and reduces DT parsing code/time.
> > > > > >
> > > > > Couldn't this be simply achieved by matching the compatible flags for
> > > > > PDC/MPM bindings for the wakeup-parent in the TLMM driver?
> > > > >
> > > >
> > > > It could be, but then we would be making TLMM highly aware of the wakeup
> > > > parent
> > > It is is not aware of anything about the wakeup parent, just the
> > > compatible that indicates whether it needs to be mask locally or not.
> > > 
> > 
> > But the information related to how the PDC is wired to GPIO pins is a
> > property related to the PDC not of the TLMM, so it does make sense to
> > represent this information there.
> > 
> Hmm.. But we are inconsistent in what we provide as input into the PDC
> driver.
> From the PDC driver perspective, it gets a hwirq number either when
> driver requests a wakeup interrupt specified in DT as
>   < 5 IRQ_TYPE_LEVEL_HIGH>
> or from GPIO, which translates the GPIO to the PDC hwirq and requests
> that.
> 

I see what you're saying, but need to think some more about this...

> > And iiuc it also makes sense to not encode it in DT because it's
> > physical wiring, not configuration.
> > 
> I would agree.
> 
> > > > and have to do compatible string matching in two places, instead
> > > > of making TLMM more abstractly aware that it needs to keep things masked
> > > > while irq parent deals with the interrupts.
> > > >
> > > Your approach seems too complex just to circumvent a simple match node.
> > > I think we are stretching too much to support something that is not a
> > > priority.
> > > 
> > 
> > What "something" are you referring to here?
> > 
> MPM :)
> 

There are still new platforms coming out with MPM, so there's even a
business case to care about it.

> BTW, I am discussing with the internal folks here on if we need to mask
> TLMM when the wakeup-parent is MPM. If we don't have to, we should be
> able to follow the same model as we done in this patch and don't even
> have to check the compatible or use the approach suggested by Stephen.
> 

Looking forward to the result of that discussion.

Regards,
Bjorn

Re: [PATCH 4.9 00/92] 4.9.142-stable review

[Naresh Kamboju]

On Thu, 29 Nov 2018 at 19:51, Greg Kroah-Hartman
 wrote:
>
> This is the start of the stable review cycle for the 4.9.142 release.
> There are 92 patches in this series, all will be posted as a response
> to this one.  If anyone has any issues with these being applied, please
> let me know.
>
> Responses should be made by Sat Dec  1 14:00:37 UTC 2018.
> Anything received after that time might be too late.
>
> The whole patch series can be found in one patch at:
> 
> https://www.kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.9.142-rc1.gz
> or in the git tree and branch at:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git 
> linux-4.9.y
> and the diffstat can be found below.
>
> thanks,
>
> greg k-h

Results from Linaro’s test farm.
No regressions on arm64, arm, x86_64, and i386.

Summary


kernel: 4.9.142-rc1
git repo: 
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git
git branch: linux-4.9.y
git commit: f46aebe62697538df220b8708e266369ffd901c5
git describe: v4.9.141-93-gf46aebe62697
Test details: 
https://qa-reports.linaro.org/lkft/linux-stable-rc-4.9-oe/build/v4.9.141-93-gf46aebe62697

No regressions (compared to build v4.9.141)

No fixes (compared to build v4.9.141)

Ran 21336 total tests in the following environments and test suites.

Environments
--
- dragonboard-410c - arm64
- hi6220-hikey - arm64
- i386
- juno-r2 - arm64
- qemu_arm
- qemu_arm64
- qemu_i386
- qemu_x86_64
- x15 - arm
- x86_64

Test Suites
---
* boot
* install-android-platform-tools-r2600
* kselftest
* libhugetlbfs
* ltp-cap_bounds-tests
* ltp-containers-tests
* ltp-cve-tests
* ltp-fcntl-locktests-tests
* ltp-filecaps-tests
* ltp-fs-tests
* ltp-fs_bind-tests
* ltp-fs_perms_simple-tests
* ltp-fsx-tests
* ltp-hugetlb-tests
* ltp-io-tests
* ltp-ipc-tests
* ltp-math-tests
* ltp-nptl-tests
* ltp-sched-tests
* ltp-securebits-tests
* ltp-syscalls-tests
* ltp-timers-tests
* ltp-pty-tests
* ltp-open-posix-tests
* kselftest-vsyscall-mode-native
* kselftest-vsyscall-mode-none

-- 
Linaro LKFT
https://lkft.linaro.org

Re: [PATCH 4.9 00/92] 4.9.142-stable review

[Naresh Kamboju]

On Thu, 29 Nov 2018 at 19:51, Greg Kroah-Hartman
 wrote:
>
> This is the start of the stable review cycle for the 4.9.142 release.
> There are 92 patches in this series, all will be posted as a response
> to this one.  If anyone has any issues with these being applied, please
> let me know.
>
> Responses should be made by Sat Dec  1 14:00:37 UTC 2018.
> Anything received after that time might be too late.
>
> The whole patch series can be found in one patch at:
> 
> https://www.kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.9.142-rc1.gz
> or in the git tree and branch at:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git 
> linux-4.9.y
> and the diffstat can be found below.
>
> thanks,
>
> greg k-h

Results from Linaro’s test farm.
No regressions on arm64, arm, x86_64, and i386.

Summary


kernel: 4.9.142-rc1
git repo: 
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git
git branch: linux-4.9.y
git commit: f46aebe62697538df220b8708e266369ffd901c5
git describe: v4.9.141-93-gf46aebe62697
Test details: 
https://qa-reports.linaro.org/lkft/linux-stable-rc-4.9-oe/build/v4.9.141-93-gf46aebe62697

No regressions (compared to build v4.9.141)

No fixes (compared to build v4.9.141)

Ran 21336 total tests in the following environments and test suites.

Environments
--
- dragonboard-410c - arm64
- hi6220-hikey - arm64
- i386
- juno-r2 - arm64
- qemu_arm
- qemu_arm64
- qemu_i386
- qemu_x86_64
- x15 - arm
- x86_64

Test Suites
---
* boot
* install-android-platform-tools-r2600
* kselftest
* libhugetlbfs
* ltp-cap_bounds-tests
* ltp-containers-tests
* ltp-cve-tests
* ltp-fcntl-locktests-tests
* ltp-filecaps-tests
* ltp-fs-tests
* ltp-fs_bind-tests
* ltp-fs_perms_simple-tests
* ltp-fsx-tests
* ltp-hugetlb-tests
* ltp-io-tests
* ltp-ipc-tests
* ltp-math-tests
* ltp-nptl-tests
* ltp-sched-tests
* ltp-securebits-tests
* ltp-syscalls-tests
* ltp-timers-tests
* ltp-pty-tests
* ltp-open-posix-tests
* kselftest-vsyscall-mode-native
* kselftest-vsyscall-mode-none

-- 
Linaro LKFT
https://lkft.linaro.org

Re: [PATCH 2/3] clk: qcom: gcc-msm8998: Disable halt check of UFS clocks

[Stephen Boyd]

Quoting Bjorn Andersson (2018-11-29 22:52:58)
> Drop the halt check of the UFS symbol clocks, in accordance with other
> platforms. This makes clk_disable_unused() happy and makes it possible
> to turn the clocks on again without an error.
> 
> Signed-off-by: Bjorn Andersson 

Someone was supposed to figure out why we needed to SKIP here instead of
doing things in the proper order. Is anyone attempting to figure that
out?

Re: [PATCH 2/3] clk: qcom: gcc-msm8998: Disable halt check of UFS clocks

[Stephen Boyd]

Quoting Bjorn Andersson (2018-11-29 22:52:58)
> Drop the halt check of the UFS symbol clocks, in accordance with other
> platforms. This makes clk_disable_unused() happy and makes it possible
> to turn the clocks on again without an error.
> 
> Signed-off-by: Bjorn Andersson 

Someone was supposed to figure out why we needed to SKIP here instead of
doing things in the proper order. Is anyone attempting to figure that
out?

Re: [PATCH 1/3] clk: qcom: gcc-msm8998: Mark hmss_dvm and lpass_at critical

[Stephen Boyd]

Quoting Bjorn Andersson (2018-11-29 22:52:57)
> Keep the two clocks enabled, so that the platform passes
> clk_disable_unused().
> 
> Signed-off-by: Bjorn Andersson 
> ---
>  drivers/clk/qcom/gcc-msm8998.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/clk/qcom/gcc-msm8998.c b/drivers/clk/qcom/gcc-msm8998.c
> index 9f0ae403d5f5..d89f8e7c2a59 100644
> --- a/drivers/clk/qcom/gcc-msm8998.c
> +++ b/drivers/clk/qcom/gcc-msm8998.c
> @@ -1972,6 +1972,7 @@ static struct clk_branch gcc_hmss_dvm_bus_clk = {
> .enable_mask = BIT(0),
> .hw.init = &(struct clk_init_data){
> .name = "gcc_hmss_dvm_bus_clk",
> +   .flags = CLK_IS_CRITICAL,

Please add a comment about why they're critical. This is a temporary
solution?

> .ops = _branch2_ops,
> },
> },

Re: [PATCH 1/3] clk: qcom: gcc-msm8998: Mark hmss_dvm and lpass_at critical

[Stephen Boyd]

Quoting Bjorn Andersson (2018-11-29 22:52:57)
> Keep the two clocks enabled, so that the platform passes
> clk_disable_unused().
> 
> Signed-off-by: Bjorn Andersson 
> ---
>  drivers/clk/qcom/gcc-msm8998.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/clk/qcom/gcc-msm8998.c b/drivers/clk/qcom/gcc-msm8998.c
> index 9f0ae403d5f5..d89f8e7c2a59 100644
> --- a/drivers/clk/qcom/gcc-msm8998.c
> +++ b/drivers/clk/qcom/gcc-msm8998.c
> @@ -1972,6 +1972,7 @@ static struct clk_branch gcc_hmss_dvm_bus_clk = {
> .enable_mask = BIT(0),
> .hw.init = &(struct clk_init_data){
> .name = "gcc_hmss_dvm_bus_clk",
> +   .flags = CLK_IS_CRITICAL,

Please add a comment about why they're critical. This is a temporary
solution?

> .ops = _branch2_ops,
> },
> },

Re: [PATCH 1/4] of/device: Add a way to probe drivers by match data

[Stephen Boyd]

Quoting Rob Herring (2018-11-29 17:01:54)
> On Thu, Nov 29, 2018 at 6:28 PM Stephen Boyd  wrote:
> >
> > Quoting Stephen Boyd (2018-11-07 10:37:31)
> > > appropriate structure with to_platform_device() or to_i2c_client()?
> > >
> > > So the example would become
> > >
> > >   struct of_driver_probe_func {
> > > int (*probe)(struct device *dev);
> > >   };
> > >
> > >   struct of_driver_probe_func mtk_probes[] = {
> > > mtk_probe1,
> > > mtk_probe2,
> > > mtk_probe3,
> > >   };
> > >
> > >   struct platform_driver mtk_driver = {
> > > .driver = {
> > > .name = "mtk-foo";
> > > .of_match_table = mtk_match_table,
> > > .of_probes = _probes;
> > > },
> > >   };
> > >
> > > And the probe functions might need to container_of() the device pointer
> > > to get the struct they know they need. The probe function could also be
> > > added to of_device_id and then we would have to look and see if that
> > > pointer is populated when the device is matched in generic device code.
> > >
> >
> > I guess I'll go down the path of extending the of_device_id structure?
> 
> Unfortunately, I don't think you can change of_device_id as it's part
> of the kernel ABI.

Ok. Then I'll go down the path of making it a parallel array?

Re: [PATCH 1/4] of/device: Add a way to probe drivers by match data

[Stephen Boyd]

Quoting Rob Herring (2018-11-29 17:01:54)
> On Thu, Nov 29, 2018 at 6:28 PM Stephen Boyd  wrote:
> >
> > Quoting Stephen Boyd (2018-11-07 10:37:31)
> > > appropriate structure with to_platform_device() or to_i2c_client()?
> > >
> > > So the example would become
> > >
> > >   struct of_driver_probe_func {
> > > int (*probe)(struct device *dev);
> > >   };
> > >
> > >   struct of_driver_probe_func mtk_probes[] = {
> > > mtk_probe1,
> > > mtk_probe2,
> > > mtk_probe3,
> > >   };
> > >
> > >   struct platform_driver mtk_driver = {
> > > .driver = {
> > > .name = "mtk-foo";
> > > .of_match_table = mtk_match_table,
> > > .of_probes = _probes;
> > > },
> > >   };
> > >
> > > And the probe functions might need to container_of() the device pointer
> > > to get the struct they know they need. The probe function could also be
> > > added to of_device_id and then we would have to look and see if that
> > > pointer is populated when the device is matched in generic device code.
> > >
> >
> > I guess I'll go down the path of extending the of_device_id structure?
> 
> Unfortunately, I don't think you can change of_device_id as it's part
> of the kernel ABI.

Ok. Then I'll go down the path of making it a parallel array?

[PATCH] sdhci: fix the fake timeout bug

[Du, Alek]

>From b893df3a1a937bd5fe22d39ceae93454a2e5e0e4 Mon Sep 17 00:00:00 2001
From: Alek Du 
Date: Fri, 30 Nov 2018 14:02:28 +0800
Subject: [PATCH] sdhci: fix the fake timeout bug

We observed some fake timeouts on some devices, the log is like this:

[ 7586.290201] mmc1: Timeout waiting for hardware cmd interrupt.
[ 7586.290420] mmc1: sdhci:  SDHCI REGISTER DUMP ===
...
[ 7586.291774] mmc1: sdhci: Wake-up:   0x | Clock:0x0203

>From the clock control register dump, we are pretty sure the clock was
stabilized.

In other cases, we also observed:

[ 7596.530171] mmc1: Timeout waiting for hardware cmd interrupt.

and

[ 1956.534634] mmc1: Reset 0x2 never completed.

But we are pretty sure the mmc controller is working perfectly under low
system load.

After checking the sdhci code, we found the timeout check actually has a
little window that the CPU can be scheduled out and when it comes back,
the original time set or check is not valid.

Signed-off-by: Alek Du 
---
 drivers/mmc/host/sdhci.c | 19 ---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 99bdae53fa2e..f88c49fc574e 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -218,12 +218,17 @@ void sdhci_reset(struct sdhci_host *host, u8 mask)
/* hw clears the bit when it's done */
while (sdhci_readb(host, SDHCI_SOFTWARE_RESET) & mask) {
if (ktime_after(ktime_get(), timeout)) {
+   /* check it again, since there is a window between
+  bit check and time check */
+   if (!(sdhci_readb(host, SDHCI_SOFTWARE_RESET) & mask))
+   break;
pr_err("%s: Reset 0x%x never completed.\n",
mmc_hostname(host->mmc), (int)mask);
sdhci_dumpregs(host);
return;
+   } else {
+   udelay(10);
}
-   udelay(10);
}
 }
 EXPORT_SYMBOL_GPL(sdhci_reset);
@@ -1395,9 +1400,10 @@ void sdhci_send_command(struct sdhci_host *host, struct 
mmc_command *cmd)
timeout += DIV_ROUND_UP(cmd->busy_timeout, 1000) * HZ + HZ;
else
timeout += 10 * HZ;
-   sdhci_mod_timer(host, cmd->mrq, timeout);
 
sdhci_writew(host, SDHCI_MAKE_CMD(cmd->opcode, flags), SDHCI_COMMAND);
+   /* setup timer after command to avoid fake timeout */
+   sdhci_mod_timer(host, cmd->mrq, timeout);
 }
 EXPORT_SYMBOL_GPL(sdhci_send_command);
 
@@ -1611,12 +1617,19 @@ void sdhci_enable_clk(struct sdhci_host *host, u16 clk)
while (!((clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL))
& SDHCI_CLOCK_INT_STABLE)) {
if (ktime_after(ktime_get(), timeout)) {
+   /* check it again since there is a window between
+  status check and time check */
+   if ((clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL))
+   & SDHCI_CLOCK_INT_STABLE)
+   break;
pr_err("%s: Internal clock never stabilised.\n",
   mmc_hostname(host->mmc));
sdhci_dumpregs(host);
return;
}
-   udelay(10);
+   else {
+   udelay(10);
+   }
}
 
clk |= SDHCI_CLOCK_CARD_EN;
-- 
2.17.1

[PATCH] sdhci: fix the fake timeout bug

[Du, Alek]

>From b893df3a1a937bd5fe22d39ceae93454a2e5e0e4 Mon Sep 17 00:00:00 2001
From: Alek Du 
Date: Fri, 30 Nov 2018 14:02:28 +0800
Subject: [PATCH] sdhci: fix the fake timeout bug

We observed some fake timeouts on some devices, the log is like this:

[ 7586.290201] mmc1: Timeout waiting for hardware cmd interrupt.
[ 7586.290420] mmc1: sdhci:  SDHCI REGISTER DUMP ===
...
[ 7586.291774] mmc1: sdhci: Wake-up:   0x | Clock:0x0203

>From the clock control register dump, we are pretty sure the clock was
stabilized.

In other cases, we also observed:

[ 7596.530171] mmc1: Timeout waiting for hardware cmd interrupt.

and

[ 1956.534634] mmc1: Reset 0x2 never completed.

But we are pretty sure the mmc controller is working perfectly under low
system load.

After checking the sdhci code, we found the timeout check actually has a
little window that the CPU can be scheduled out and when it comes back,
the original time set or check is not valid.

Signed-off-by: Alek Du 
---
 drivers/mmc/host/sdhci.c | 19 ---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 99bdae53fa2e..f88c49fc574e 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -218,12 +218,17 @@ void sdhci_reset(struct sdhci_host *host, u8 mask)
/* hw clears the bit when it's done */
while (sdhci_readb(host, SDHCI_SOFTWARE_RESET) & mask) {
if (ktime_after(ktime_get(), timeout)) {
+   /* check it again, since there is a window between
+  bit check and time check */
+   if (!(sdhci_readb(host, SDHCI_SOFTWARE_RESET) & mask))
+   break;
pr_err("%s: Reset 0x%x never completed.\n",
mmc_hostname(host->mmc), (int)mask);
sdhci_dumpregs(host);
return;
+   } else {
+   udelay(10);
}
-   udelay(10);
}
 }
 EXPORT_SYMBOL_GPL(sdhci_reset);
@@ -1395,9 +1400,10 @@ void sdhci_send_command(struct sdhci_host *host, struct 
mmc_command *cmd)
timeout += DIV_ROUND_UP(cmd->busy_timeout, 1000) * HZ + HZ;
else
timeout += 10 * HZ;
-   sdhci_mod_timer(host, cmd->mrq, timeout);
 
sdhci_writew(host, SDHCI_MAKE_CMD(cmd->opcode, flags), SDHCI_COMMAND);
+   /* setup timer after command to avoid fake timeout */
+   sdhci_mod_timer(host, cmd->mrq, timeout);
 }
 EXPORT_SYMBOL_GPL(sdhci_send_command);
 
@@ -1611,12 +1617,19 @@ void sdhci_enable_clk(struct sdhci_host *host, u16 clk)
while (!((clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL))
& SDHCI_CLOCK_INT_STABLE)) {
if (ktime_after(ktime_get(), timeout)) {
+   /* check it again since there is a window between
+  status check and time check */
+   if ((clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL))
+   & SDHCI_CLOCK_INT_STABLE)
+   break;
pr_err("%s: Internal clock never stabilised.\n",
   mmc_hostname(host->mmc));
sdhci_dumpregs(host);
return;
}
-   udelay(10);
+   else {
+   udelay(10);
+   }
}
 
clk |= SDHCI_CLOCK_CARD_EN;
-- 
2.17.1

[PATCH] arm64: dts: qcom: msm8998: Fix compatible of scm node

[Bjorn Andersson]

The scm binding and driver was updated to rely on the fallback to the
default qcom,scm for any modern SoC and as such both are required. Add
the default compatible to make the scm instance probe.

Fixes: d850156a226a ("arm64: dts: qcom: msm8998: Add firmware node")
Signed-off-by: Bjorn Andersson 
---
 arch/arm64/boot/dts/qcom/msm8998.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi 
b/arch/arm64/boot/dts/qcom/msm8998.dtsi
index 78227cce16db..a15949d32bad 100644
--- a/arch/arm64/boot/dts/qcom/msm8998.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi
@@ -239,7 +239,7 @@
 
firmware {
scm {
-   compatible = "qcom,scm-msm8998";
+   compatible = "qcom,scm-msm8998", "qcom,scm";
};
};
 
-- 
2.18.0

[PATCH] arm64: dts: qcom: msm8998: Fix compatible of scm node

[Bjorn Andersson]

The scm binding and driver was updated to rely on the fallback to the
default qcom,scm for any modern SoC and as such both are required. Add
the default compatible to make the scm instance probe.

Fixes: d850156a226a ("arm64: dts: qcom: msm8998: Add firmware node")
Signed-off-by: Bjorn Andersson 
---
 arch/arm64/boot/dts/qcom/msm8998.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi 
b/arch/arm64/boot/dts/qcom/msm8998.dtsi
index 78227cce16db..a15949d32bad 100644
--- a/arch/arm64/boot/dts/qcom/msm8998.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi
@@ -239,7 +239,7 @@
 
firmware {
scm {
-   compatible = "qcom,scm-msm8998";
+   compatible = "qcom,scm-msm8998", "qcom,scm";
};
};
 
-- 
2.18.0

[PATCH] arm64: defconfig: Enable GCC and PINCTRL for MSM8998

[Bjorn Andersson]

Enable the GCC and PINCTRL for MSM8998 to make upstream boot to console.

Signed-off-by: Bjorn Andersson 
---
 arch/arm64/configs/defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 1f1f4eab89df..206f50d121ba 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -348,6 +348,7 @@ CONFIG_PINCTRL_IPQ8074=y
 CONFIG_PINCTRL_MSM8916=y
 CONFIG_PINCTRL_MSM8994=y
 CONFIG_PINCTRL_MSM8996=y
+CONFIG_PINCTRL_MSM8998=y
 CONFIG_PINCTRL_QDF2XXX=y
 CONFIG_PINCTRL_QCOM_SPMI_PMIC=y
 CONFIG_PINCTRL_MT7622=y
@@ -605,6 +606,7 @@ CONFIG_IPQ_GCC_8074=y
 CONFIG_MSM_GCC_8916=y
 CONFIG_MSM_GCC_8994=y
 CONFIG_MSM_MMCC_8996=y
+CONFIG_MSM_GCC_8998=y
 CONFIG_HWSPINLOCK=y
 CONFIG_HWSPINLOCK_QCOM=y
 CONFIG_ARM_MHU=y
-- 
2.18.0

[PATCH] arm64: defconfig: Enable GCC and PINCTRL for MSM8998

[Bjorn Andersson]

Enable the GCC and PINCTRL for MSM8998 to make upstream boot to console.

Signed-off-by: Bjorn Andersson 
---
 arch/arm64/configs/defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 1f1f4eab89df..206f50d121ba 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -348,6 +348,7 @@ CONFIG_PINCTRL_IPQ8074=y
 CONFIG_PINCTRL_MSM8916=y
 CONFIG_PINCTRL_MSM8994=y
 CONFIG_PINCTRL_MSM8996=y
+CONFIG_PINCTRL_MSM8998=y
 CONFIG_PINCTRL_QDF2XXX=y
 CONFIG_PINCTRL_QCOM_SPMI_PMIC=y
 CONFIG_PINCTRL_MT7622=y
@@ -605,6 +606,7 @@ CONFIG_IPQ_GCC_8074=y
 CONFIG_MSM_GCC_8916=y
 CONFIG_MSM_GCC_8994=y
 CONFIG_MSM_MMCC_8996=y
+CONFIG_MSM_GCC_8998=y
 CONFIG_HWSPINLOCK=y
 CONFIG_HWSPINLOCK_QCOM=y
 CONFIG_ARM_MHU=y
-- 
2.18.0

Re: [PATCH 2/2] dt-bindings: arm: mediatek: document clk bindings for MT7629

[Stephen Boyd]

Quoting Ryder Lee (2018-11-05 00:43:56)
> This patch adds the binding documentation for apmixedsys, infracfg,
> pciesys, pericfg, topckgen, ethsys, sgmiisys and ssusbsys for MT7629.
> 
> Signed-off-by: Ryder Lee 
> ---

Applied to clk-next

Re: [PATCH 2/2] dt-bindings: arm: mediatek: document clk bindings for MT7629

[Stephen Boyd]

Quoting Ryder Lee (2018-11-05 00:43:56)
> This patch adds the binding documentation for apmixedsys, infracfg,
> pciesys, pericfg, topckgen, ethsys, sgmiisys and ssusbsys for MT7629.
> 
> Signed-off-by: Ryder Lee 
> ---

Applied to clk-next

Re: [PATCH 1/2] clk: mediatek: add clock support for MT7629 SoC

[Stephen Boyd]

Quoting Ryder Lee (2018-11-05 00:43:55)
> Add all supported clocks exported from every susbystem found on MT7629 SoC.
> 
> Signed-off-by: Wenzhen Yu 
> Signed-off-by: Ryder Lee 
> ---

Applied to clk-next

Re: arch/x86/include/asm/cmpxchg.h:245:2: error: 'asm' operand has impossible constraints

[Juergen Gross]

On 30/11/2018 07:44, kbuild test robot wrote:
> Hi Juergen,
> 
> FYI, the error/warning still remains.

This problem is one of gcc. It is in no way the (direct) result of my
patch. Please see my gcc bug report:

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=908154


Juergen

> 
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
> master
> head:   94f371cb73944b410a269d570d6946c042f2ddd0
> commit: 6da63eb241a05b0e676d68975e793c0521387141 x86/paravirt: Move the 
> pv_irq_ops under the PARAVIRT_XXL umbrella
> date:   3 months ago
> config: i386-randconfig-sb0-11301144 (attached as .config)
> compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
> reproduce:
> git checkout 6da63eb241a05b0e676d68975e793c0521387141
> # save the attached .config to linux build tree
> make ARCH=i386 
> 
> All errors (new ones prefixed by >>):
> 
>In file included from arch/x86/include/asm/atomic.h:8:0,
> from arch/x86/include/asm/msr.h:67,
> from arch/x86/include/asm/processor.h:21,
> from arch/x86/include/asm/cpufeature.h:5,
> from arch/x86/include/asm/thread_info.h:53,
> from include/linux/thread_info.h:38,
> from arch/x86/include/asm/preempt.h:7,
> from include/linux/preempt.h:81,
> from include/linux/spinlock.h:51,
> from include/linux/mmzone.h:8,
> from include/linux/gfp.h:6,
> from include/linux/mm.h:10,
> from mm/slub.c:13:
>mm/slub.c: In function '__slab_free':
>>> arch/x86/include/asm/cmpxchg.h:245:2: error: 'asm' operand has impossible 
>>> constraints
>  asm volatile(pfx "cmpxchg%c4b %2; sete %0"   \
>  ^
>arch/x86/include/asm/cmpxchg.h:254:2: note: in expansion of macro 
> '__cmpxchg_double'
>  __cmpxchg_double(LOCK_PREFIX, p1, p2, o1, o2, n1, n2)
>  ^
>include/asm-generic/atomic-instrumented.h:457:2: note: in expansion of 
> macro 'arch_cmpxchg_double'
>  arch_cmpxchg_double(__ai_p1, (p2), (o1), (o2), (n1), (n2)); \
>  ^
>mm/slub.c:403:7: note: in expansion of macro 'cmpxchg_double'
>   if (cmpxchg_double(>freelist, >counters,
>   ^
> 
> vim +/asm +245 arch/x86/include/asm/cmpxchg.h
> 
> 3d94ae0c Jeremy Fitzhardinge 2011-09-28  235  
> cdcd6298 Jan Beulich 2012-01-02  236  #define __cmpxchg_double(pfx, 
> p1, p2, o1, o2, n1, n2)   \
> cdcd6298 Jan Beulich 2012-01-02  237  ({  
> \
> cdcd6298 Jan Beulich 2012-01-02  238  bool __ret; 
> \
> cdcd6298 Jan Beulich 2012-01-02  239  __typeof__(*(p1)) 
> __old1 = (o1), __new1 = (n1); \
> cdcd6298 Jan Beulich 2012-01-02  240  __typeof__(*(p2)) 
> __old2 = (o2), __new2 = (n2); \
> cdcd6298 Jan Beulich 2012-01-02  241  
> BUILD_BUG_ON(sizeof(*(p1)) != sizeof(long));\
> cdcd6298 Jan Beulich 2012-01-02  242  
> BUILD_BUG_ON(sizeof(*(p2)) != sizeof(long));\
> cdcd6298 Jan Beulich 2012-01-02  243  VM_BUG_ON((unsigned 
> long)(p1) % (2 * sizeof(long)));\
> cdcd6298 Jan Beulich 2012-01-02  244  VM_BUG_ON((unsigned 
> long)((p1) + 1) != (unsigned long)(p2));\
> cdcd6298 Jan Beulich 2012-01-02 @245  asm volatile(pfx 
> "cmpxchg%c4b %2; sete %0"  \
> cdcd6298 Jan Beulich 2012-01-02  246   : "=a" 
> (__ret), "+d" (__old2), \
> cdcd6298 Jan Beulich 2012-01-02  247 "+m" 
> (*(p1)), "+m" (*(p2))   \
> cdcd6298 Jan Beulich 2012-01-02  248   : "i" (2 * 
> sizeof(long)), "a" (__old1),\
> cdcd6298 Jan Beulich 2012-01-02  249 "b" 
> (__new1), "c" (__new2)); \
> cdcd6298 Jan Beulich 2012-01-02  250  __ret;  
> \
> cdcd6298 Jan Beulich 2012-01-02  251  })
> cdcd6298 Jan Beulich 2012-01-02  252  
> 
> :: The code at line 245 was first introduced by commit
> :: cdcd629869fabcd38ebd24a03b0a05ec1cbcafb0 x86: Fix and improve 
> cmpxchg_double{,_local}()
> 
> :: TO: Jan Beulich 
> :: CC: Ingo Molnar 
> 
> ---
> 0-DAY kernel test infrastructureOpen Source Technology Center
> https://lists.01.org/pipermail/kbuild-all   Intel Corporation
> 

Re: [PATCH v2] signal: add procfd_signal() syscall

[Christian Brauner]

On Thu, Nov 29, 2018 at 11:13:57PM -0600, Eric W. Biederman wrote:
> Arnd Bergmann  writes:
> 
> > On Thu, Nov 29, 2018 at 9:14 PM Andy Lutomirski  wrote:
> >> > On Nov 29, 2018, at 11:55 AM, Christian Brauner  
> >> > wrote:
> >> >> On Thu, Nov 29, 2018 at 11:22:58AM -0800, Andy Lutomirski wrote:
> >> >>> On Thu, Nov 29, 2018 at 11:17 AM Christian Brauner 
> >> >>>  wrote:
> >>  On November 30, 2018 5:54:18 AM GMT+13:00, Andy Lutomirski 
> >>   wrote:
> >> >>
> >> >> The #1 fix would add a copy_siginfo_from_user64() or similar.
> >> >
> >> > Thanks very much! That all helped a bunch already! I'll try to go the
> >> > copy_siginfo_from_user64() way first and see if I can make this work. If
> >> > we do this I would however only want to use it for the new syscall first
> >> > and not change all other signal syscalls over to it too. I'd rather keep
> >> > this patchset focussed and small and do such conversions caused by the
> >> > new approach later. Does that sound reasonable?
> >>
> >> Absolutely. I don’t think we can change old syscalls — the ABI is set in 
> >> stone.
> >> But for new syscalls, I think the always-64-bit behavior makes sense.
> >
> > It looks like we already have a 'struct signalfd_siginfo' that is defined 
> > in a
> > sane architecture-independent way, so I'd suggest we use that.
> 
> Unfortunately it isn't maintained very well.  What you can
> express with signalfd_siginfo is a subset what you can express with
> siginfo.  Many of the linux extensions to siginfo for exception
> information add pointers and have integers right after those pointers.
> Not all of those linux specific extentions for exceptions are handled
> by signalfd_siginfo (it needs new fields).
> 
> As originally defined siginfo had the sigval_t union at the end so it
> was perfectly fine on both 32bit and 64bit as it only had a single
> pointer in the structure and the other fields were 32bits in size.
> 
> Although I do feel the pain as x86_64 has to deal with 3 versions
> of siginfo.  A 64bit one.  A 32bit one for ia32.  A 32bit one for x32
> with a 64bit si_clock_t.
> 
> > We may then also want to make sure that any system call that takes a
> > siginfo has a replacement that takes a signalfd_siginfo, and that this
> > replacement can be used to implement the old version purely in
> > user space.
> 
> If you are not implementing CRIU and reinserting exceptions to yourself.
> At most user space wants the ability to implement sigqueue:
> 
> AKA:
> sigqueue(pid_t pid, int sig, const union sigval value);
> 
> Well sigqueue with it's own si_codes so the following would cover all
> the use cases I know of:
> int sendnewsig(pid_t pid, int sig, int si_code, const union sigval value);
> 
> The si_code could even be set to SI_USER to request that the normal
> trusted SI_USER values be filled in.  si_code values of < 0 if not
> recognized could reasonably safely be treated as the _rt member of
> the siginfo union.  Or perhaps better we error out in such a case.
> 
> If we want to be flexible and not have N kinds of system calls that
> is the direction I would go.  That is simple, and you don't need any of
> the rest.
> 
> 
> Alternatively we abandon the mistake of sigqueueinfo and not allow
> a signal number in the arguments that differs from the si_signo in the
> siginfo and allow passing the entire thing unchanged from sender to
> receiver.  That is maximum flexibility.
> 
> signalfd_siginfo just sucks in practice.  It is larger that siginfo 104
> bytes versus 48.  We must deliver it to userspace as a siginfo so it
> must be translated.  Because of the translation signalfd_siginfo adds
> no flexiblity in practice, because it can not just be passed through.
> Finallay signalfd_siginfo does not have encodings for all of the
> siginfo union members, so it fails to be fully general.
> 
> Personally if I was to define signalfd_siginfo today I would make it:
> struct siginfo_subset {
>   __u32 sis_signo;
>   __s32 sis_errno;
>   __s32 sis_code;
> __u32 sis_pad;
>   __u32 sis_pid;
>   __u32 sis_uid;
>   __u64 sis_data (A pointer or integer data field);
> };
> 
> That is just 32bytes, and is all that is needed for everything
> except for synchronous exceptions.  Oh and that happens to be a proper
> subset of a any sane siginfo layout, on both 32bit and 64bit.
> 
> This is one of those rare times where POSIX is sane and what linux
> has implemented is not.

Thanks for the in-depth explanation. So your point is that we are better
off if we stick with siginfo_t instead of struct signalfd_siginfo in
procfd_signal()?

Re: [PATCH 1/2] clk: mediatek: add clock support for MT7629 SoC

[Stephen Boyd]

Quoting Ryder Lee (2018-11-05 00:43:55)
> Add all supported clocks exported from every susbystem found on MT7629 SoC.
> 
> Signed-off-by: Wenzhen Yu 
> Signed-off-by: Ryder Lee 
> ---

Applied to clk-next

Re: arch/x86/include/asm/cmpxchg.h:245:2: error: 'asm' operand has impossible constraints

[Juergen Gross]

On 30/11/2018 07:44, kbuild test robot wrote:
> Hi Juergen,
> 
> FYI, the error/warning still remains.

This problem is one of gcc. It is in no way the (direct) result of my
patch. Please see my gcc bug report:

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=908154


Juergen

> 
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
> master
> head:   94f371cb73944b410a269d570d6946c042f2ddd0
> commit: 6da63eb241a05b0e676d68975e793c0521387141 x86/paravirt: Move the 
> pv_irq_ops under the PARAVIRT_XXL umbrella
> date:   3 months ago
> config: i386-randconfig-sb0-11301144 (attached as .config)
> compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
> reproduce:
> git checkout 6da63eb241a05b0e676d68975e793c0521387141
> # save the attached .config to linux build tree
> make ARCH=i386 
> 
> All errors (new ones prefixed by >>):
> 
>In file included from arch/x86/include/asm/atomic.h:8:0,
> from arch/x86/include/asm/msr.h:67,
> from arch/x86/include/asm/processor.h:21,
> from arch/x86/include/asm/cpufeature.h:5,
> from arch/x86/include/asm/thread_info.h:53,
> from include/linux/thread_info.h:38,
> from arch/x86/include/asm/preempt.h:7,
> from include/linux/preempt.h:81,
> from include/linux/spinlock.h:51,
> from include/linux/mmzone.h:8,
> from include/linux/gfp.h:6,
> from include/linux/mm.h:10,
> from mm/slub.c:13:
>mm/slub.c: In function '__slab_free':
>>> arch/x86/include/asm/cmpxchg.h:245:2: error: 'asm' operand has impossible 
>>> constraints
>  asm volatile(pfx "cmpxchg%c4b %2; sete %0"   \
>  ^
>arch/x86/include/asm/cmpxchg.h:254:2: note: in expansion of macro 
> '__cmpxchg_double'
>  __cmpxchg_double(LOCK_PREFIX, p1, p2, o1, o2, n1, n2)
>  ^
>include/asm-generic/atomic-instrumented.h:457:2: note: in expansion of 
> macro 'arch_cmpxchg_double'
>  arch_cmpxchg_double(__ai_p1, (p2), (o1), (o2), (n1), (n2)); \
>  ^
>mm/slub.c:403:7: note: in expansion of macro 'cmpxchg_double'
>   if (cmpxchg_double(>freelist, >counters,
>   ^
> 
> vim +/asm +245 arch/x86/include/asm/cmpxchg.h
> 
> 3d94ae0c Jeremy Fitzhardinge 2011-09-28  235  
> cdcd6298 Jan Beulich 2012-01-02  236  #define __cmpxchg_double(pfx, 
> p1, p2, o1, o2, n1, n2)   \
> cdcd6298 Jan Beulich 2012-01-02  237  ({  
> \
> cdcd6298 Jan Beulich 2012-01-02  238  bool __ret; 
> \
> cdcd6298 Jan Beulich 2012-01-02  239  __typeof__(*(p1)) 
> __old1 = (o1), __new1 = (n1); \
> cdcd6298 Jan Beulich 2012-01-02  240  __typeof__(*(p2)) 
> __old2 = (o2), __new2 = (n2); \
> cdcd6298 Jan Beulich 2012-01-02  241  
> BUILD_BUG_ON(sizeof(*(p1)) != sizeof(long));\
> cdcd6298 Jan Beulich 2012-01-02  242  
> BUILD_BUG_ON(sizeof(*(p2)) != sizeof(long));\
> cdcd6298 Jan Beulich 2012-01-02  243  VM_BUG_ON((unsigned 
> long)(p1) % (2 * sizeof(long)));\
> cdcd6298 Jan Beulich 2012-01-02  244  VM_BUG_ON((unsigned 
> long)((p1) + 1) != (unsigned long)(p2));\
> cdcd6298 Jan Beulich 2012-01-02 @245  asm volatile(pfx 
> "cmpxchg%c4b %2; sete %0"  \
> cdcd6298 Jan Beulich 2012-01-02  246   : "=a" 
> (__ret), "+d" (__old2), \
> cdcd6298 Jan Beulich 2012-01-02  247 "+m" 
> (*(p1)), "+m" (*(p2))   \
> cdcd6298 Jan Beulich 2012-01-02  248   : "i" (2 * 
> sizeof(long)), "a" (__old1),\
> cdcd6298 Jan Beulich 2012-01-02  249 "b" 
> (__new1), "c" (__new2)); \
> cdcd6298 Jan Beulich 2012-01-02  250  __ret;  
> \
> cdcd6298 Jan Beulich 2012-01-02  251  })
> cdcd6298 Jan Beulich 2012-01-02  252  
> 
> :: The code at line 245 was first introduced by commit
> :: cdcd629869fabcd38ebd24a03b0a05ec1cbcafb0 x86: Fix and improve 
> cmpxchg_double{,_local}()
> 
> :: TO: Jan Beulich 
> :: CC: Ingo Molnar 
> 
> ---
> 0-DAY kernel test infrastructureOpen Source Technology Center
> https://lists.01.org/pipermail/kbuild-all   Intel Corporation
> 

Re: [PATCH v2] signal: add procfd_signal() syscall

[Christian Brauner]

On Thu, Nov 29, 2018 at 11:13:57PM -0600, Eric W. Biederman wrote:
> Arnd Bergmann  writes:
> 
> > On Thu, Nov 29, 2018 at 9:14 PM Andy Lutomirski  wrote:
> >> > On Nov 29, 2018, at 11:55 AM, Christian Brauner  
> >> > wrote:
> >> >> On Thu, Nov 29, 2018 at 11:22:58AM -0800, Andy Lutomirski wrote:
> >> >>> On Thu, Nov 29, 2018 at 11:17 AM Christian Brauner 
> >> >>>  wrote:
> >>  On November 30, 2018 5:54:18 AM GMT+13:00, Andy Lutomirski 
> >>   wrote:
> >> >>
> >> >> The #1 fix would add a copy_siginfo_from_user64() or similar.
> >> >
> >> > Thanks very much! That all helped a bunch already! I'll try to go the
> >> > copy_siginfo_from_user64() way first and see if I can make this work. If
> >> > we do this I would however only want to use it for the new syscall first
> >> > and not change all other signal syscalls over to it too. I'd rather keep
> >> > this patchset focussed and small and do such conversions caused by the
> >> > new approach later. Does that sound reasonable?
> >>
> >> Absolutely. I don’t think we can change old syscalls — the ABI is set in 
> >> stone.
> >> But for new syscalls, I think the always-64-bit behavior makes sense.
> >
> > It looks like we already have a 'struct signalfd_siginfo' that is defined 
> > in a
> > sane architecture-independent way, so I'd suggest we use that.
> 
> Unfortunately it isn't maintained very well.  What you can
> express with signalfd_siginfo is a subset what you can express with
> siginfo.  Many of the linux extensions to siginfo for exception
> information add pointers and have integers right after those pointers.
> Not all of those linux specific extentions for exceptions are handled
> by signalfd_siginfo (it needs new fields).
> 
> As originally defined siginfo had the sigval_t union at the end so it
> was perfectly fine on both 32bit and 64bit as it only had a single
> pointer in the structure and the other fields were 32bits in size.
> 
> Although I do feel the pain as x86_64 has to deal with 3 versions
> of siginfo.  A 64bit one.  A 32bit one for ia32.  A 32bit one for x32
> with a 64bit si_clock_t.
> 
> > We may then also want to make sure that any system call that takes a
> > siginfo has a replacement that takes a signalfd_siginfo, and that this
> > replacement can be used to implement the old version purely in
> > user space.
> 
> If you are not implementing CRIU and reinserting exceptions to yourself.
> At most user space wants the ability to implement sigqueue:
> 
> AKA:
> sigqueue(pid_t pid, int sig, const union sigval value);
> 
> Well sigqueue with it's own si_codes so the following would cover all
> the use cases I know of:
> int sendnewsig(pid_t pid, int sig, int si_code, const union sigval value);
> 
> The si_code could even be set to SI_USER to request that the normal
> trusted SI_USER values be filled in.  si_code values of < 0 if not
> recognized could reasonably safely be treated as the _rt member of
> the siginfo union.  Or perhaps better we error out in such a case.
> 
> If we want to be flexible and not have N kinds of system calls that
> is the direction I would go.  That is simple, and you don't need any of
> the rest.
> 
> 
> Alternatively we abandon the mistake of sigqueueinfo and not allow
> a signal number in the arguments that differs from the si_signo in the
> siginfo and allow passing the entire thing unchanged from sender to
> receiver.  That is maximum flexibility.
> 
> signalfd_siginfo just sucks in practice.  It is larger that siginfo 104
> bytes versus 48.  We must deliver it to userspace as a siginfo so it
> must be translated.  Because of the translation signalfd_siginfo adds
> no flexiblity in practice, because it can not just be passed through.
> Finallay signalfd_siginfo does not have encodings for all of the
> siginfo union members, so it fails to be fully general.
> 
> Personally if I was to define signalfd_siginfo today I would make it:
> struct siginfo_subset {
>   __u32 sis_signo;
>   __s32 sis_errno;
>   __s32 sis_code;
> __u32 sis_pad;
>   __u32 sis_pid;
>   __u32 sis_uid;
>   __u64 sis_data (A pointer or integer data field);
> };
> 
> That is just 32bytes, and is all that is needed for everything
> except for synchronous exceptions.  Oh and that happens to be a proper
> subset of a any sane siginfo layout, on both 32bit and 64bit.
> 
> This is one of those rare times where POSIX is sane and what linux
> has implemented is not.

Thanks for the in-depth explanation. So your point is that we are better
off if we stick with siginfo_t instead of struct signalfd_siginfo in
procfd_signal()?

[PATCH 2/3] clk: qcom: gcc-msm8998: Disable halt check of UFS clocks

[Bjorn Andersson]

Drop the halt check of the UFS symbol clocks, in accordance with other
platforms. This makes clk_disable_unused() happy and makes it possible
to turn the clocks on again without an error.

Signed-off-by: Bjorn Andersson 
---
 drivers/clk/qcom/gcc-msm8998.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/qcom/gcc-msm8998.c b/drivers/clk/qcom/gcc-msm8998.c
index d89f8e7c2a59..3d232d266d72 100644
--- a/drivers/clk/qcom/gcc-msm8998.c
+++ b/drivers/clk/qcom/gcc-msm8998.c
@@ -2403,7 +2403,7 @@ static struct clk_branch gcc_ufs_phy_aux_clk = {
 
 static struct clk_branch gcc_ufs_rx_symbol_0_clk = {
.halt_reg = 0x75014,
-   .halt_check = BRANCH_HALT,
+   .halt_check = BRANCH_HALT_SKIP,
.clkr = {
.enable_reg = 0x75014,
.enable_mask = BIT(0),
@@ -2416,7 +2416,7 @@ static struct clk_branch gcc_ufs_rx_symbol_0_clk = {
 
 static struct clk_branch gcc_ufs_rx_symbol_1_clk = {
.halt_reg = 0x7605c,
-   .halt_check = BRANCH_HALT,
+   .halt_check = BRANCH_HALT_SKIP,
.clkr = {
.enable_reg = 0x7605c,
.enable_mask = BIT(0),
@@ -2429,7 +2429,7 @@ static struct clk_branch gcc_ufs_rx_symbol_1_clk = {
 
 static struct clk_branch gcc_ufs_tx_symbol_0_clk = {
.halt_reg = 0x75010,
-   .halt_check = BRANCH_HALT,
+   .halt_check = BRANCH_HALT_SKIP,
.clkr = {
.enable_reg = 0x75010,
.enable_mask = BIT(0),
-- 
2.18.0

[PATCH 2/3] clk: qcom: gcc-msm8998: Disable halt check of UFS clocks

[Bjorn Andersson]

Drop the halt check of the UFS symbol clocks, in accordance with other
platforms. This makes clk_disable_unused() happy and makes it possible
to turn the clocks on again without an error.

Signed-off-by: Bjorn Andersson 
---
 drivers/clk/qcom/gcc-msm8998.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/qcom/gcc-msm8998.c b/drivers/clk/qcom/gcc-msm8998.c
index d89f8e7c2a59..3d232d266d72 100644
--- a/drivers/clk/qcom/gcc-msm8998.c
+++ b/drivers/clk/qcom/gcc-msm8998.c
@@ -2403,7 +2403,7 @@ static struct clk_branch gcc_ufs_phy_aux_clk = {
 
 static struct clk_branch gcc_ufs_rx_symbol_0_clk = {
.halt_reg = 0x75014,
-   .halt_check = BRANCH_HALT,
+   .halt_check = BRANCH_HALT_SKIP,
.clkr = {
.enable_reg = 0x75014,
.enable_mask = BIT(0),
@@ -2416,7 +2416,7 @@ static struct clk_branch gcc_ufs_rx_symbol_0_clk = {
 
 static struct clk_branch gcc_ufs_rx_symbol_1_clk = {
.halt_reg = 0x7605c,
-   .halt_check = BRANCH_HALT,
+   .halt_check = BRANCH_HALT_SKIP,
.clkr = {
.enable_reg = 0x7605c,
.enable_mask = BIT(0),
@@ -2429,7 +2429,7 @@ static struct clk_branch gcc_ufs_rx_symbol_1_clk = {
 
 static struct clk_branch gcc_ufs_tx_symbol_0_clk = {
.halt_reg = 0x75010,
-   .halt_check = BRANCH_HALT,
+   .halt_check = BRANCH_HALT_SKIP,
.clkr = {
.enable_reg = 0x75010,
.enable_mask = BIT(0),
-- 
2.18.0

[PATCH 3/3] clk: qcom: gcc-msm8998: Add clkref clocks

[Bjorn Andersson]

Add clkref clocks for usb3, hdmi, ufs, pcie, and usb2. They are all
sourced off CXO_IN, so parent them off "xo" until a proper link to the
rpmcc can be described in DT.

Signed-off-by: Bjorn Andersson 
---
 drivers/clk/qcom/gcc-msm8998.c   | 75 
 include/dt-bindings/clock/qcom,gcc-msm8998.h |  5 ++
 2 files changed, 80 insertions(+)

diff --git a/drivers/clk/qcom/gcc-msm8998.c b/drivers/clk/qcom/gcc-msm8998.c
index 3d232d266d72..3cf16a4f931c 100644
--- a/drivers/clk/qcom/gcc-msm8998.c
+++ b/drivers/clk/qcom/gcc-msm8998.c
@@ -2543,6 +2543,76 @@ static struct clk_branch gcc_usb_phy_cfg_ahb2phy_clk = {
},
 };
 
+static struct clk_branch gcc_hdmi_clkref_clk = {
+   .halt_reg = 0x88000,
+   .clkr = {
+   .enable_reg = 0x88000,
+   .enable_mask = BIT(0),
+   .hw.init = &(struct clk_init_data){
+   .name = "gcc_hdmi_clkref_clk",
+   .parent_names = (const char *[]){ "xo" },
+   .num_parents = 1,
+   .ops = _branch2_ops,
+   },
+   },
+};
+
+static struct clk_branch gcc_ufs_clkref_clk = {
+   .halt_reg = 0x88004,
+   .clkr = {
+   .enable_reg = 0x88004,
+   .enable_mask = BIT(0),
+   .hw.init = &(struct clk_init_data){
+   .name = "gcc_ufs_clkref_clk",
+   .parent_names = (const char *[]){ "xo" },
+   .num_parents = 1,
+   .ops = _branch2_ops,
+   },
+   },
+};
+
+static struct clk_branch gcc_usb3_clkref_clk = {
+   .halt_reg = 0x88008,
+   .clkr = {
+   .enable_reg = 0x88008,
+   .enable_mask = BIT(0),
+   .hw.init = &(struct clk_init_data){
+   .name = "gcc_usb3_clkref_clk",
+   .parent_names = (const char *[]){ "xo" },
+   .num_parents = 1,
+   .ops = _branch2_ops,
+   },
+   },
+};
+
+static struct clk_branch gcc_pcie_clkref_clk = {
+   .halt_reg = 0x8800c,
+   .clkr = {
+   .enable_reg = 0x8800c,
+   .enable_mask = BIT(0),
+   .hw.init = &(struct clk_init_data){
+   .name = "gcc_pcie_clkref_clk",
+   .parent_names = (const char *[]){ "xo" },
+   .num_parents = 1,
+   .ops = _branch2_ops,
+   },
+   },
+};
+
+static struct clk_branch gcc_rx1_usb2_clkref_clk = {
+   .halt_reg = 0x88014,
+   .clkr = {
+   .enable_reg = 0x88014,
+   .enable_mask = BIT(0),
+   .hw.init = &(struct clk_init_data){
+   .name = "gcc_rx1_usb2_clkref_clk",
+   .parent_names = (const char *[]){ "xo" },
+   .num_parents = 1,
+   .ops = _branch2_ops,
+   },
+   },
+};
+
 static struct gdsc pcie_0_gdsc = {
.gdscr = 0x6b004,
.gds_hw_ctrl = 0x0,
@@ -2735,6 +2805,11 @@ static struct clk_regmap *gcc_msm8998_clocks[] = {
[USB30_MASTER_CLK_SRC] = _master_clk_src.clkr,
[USB30_MOCK_UTMI_CLK_SRC] = _mock_utmi_clk_src.clkr,
[USB3_PHY_AUX_CLK_SRC] = _phy_aux_clk_src.clkr,
+   [GCC_HDMI_CLKREF_CLK] = _hdmi_clkref_clk.clkr,
+   [GCC_UFS_CLKREF_CLK] = _ufs_clkref_clk.clkr,
+   [GCC_USB3_CLKREF_CLK] = _usb3_clkref_clk.clkr,
+   [GCC_PCIE_CLKREF_CLK] = _pcie_clkref_clk.clkr,
+   [GCC_RX1_USB2_CLKREF_CLK] = _rx1_usb2_clkref_clk.clkr,
 };
 
 static struct gdsc *gcc_msm8998_gdscs[] = {
diff --git a/include/dt-bindings/clock/qcom,gcc-msm8998.h 
b/include/dt-bindings/clock/qcom,gcc-msm8998.h
index 58a242e656b1..b3448800980a 100644
--- a/include/dt-bindings/clock/qcom,gcc-msm8998.h
+++ b/include/dt-bindings/clock/qcom,gcc-msm8998.h
@@ -180,6 +180,11 @@
 #define USB30_MASTER_CLK_SRC   163
 #define USB30_MOCK_UTMI_CLK_SRC164
 #define USB3_PHY_AUX_CLK_SRC   165
+#define GCC_USB3_CLKREF_CLK166
+#define GCC_HDMI_CLKREF_CLK167
+#define GCC_UFS_CLKREF_CLK 168
+#define GCC_PCIE_CLKREF_CLK169
+#define GCC_RX1_USB2_CLKREF_CLK170
 
 #define PCIE_0_GDSC0
 #define UFS_GDSC   1
-- 
2.18.0

[PATCH 1/3] clk: qcom: gcc-msm8998: Mark hmss_dvm and lpass_at critical

[Bjorn Andersson]

Keep the two clocks enabled, so that the platform passes
clk_disable_unused().

Signed-off-by: Bjorn Andersson 
---
 drivers/clk/qcom/gcc-msm8998.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/clk/qcom/gcc-msm8998.c b/drivers/clk/qcom/gcc-msm8998.c
index 9f0ae403d5f5..d89f8e7c2a59 100644
--- a/drivers/clk/qcom/gcc-msm8998.c
+++ b/drivers/clk/qcom/gcc-msm8998.c
@@ -1972,6 +1972,7 @@ static struct clk_branch gcc_hmss_dvm_bus_clk = {
.enable_mask = BIT(0),
.hw.init = &(struct clk_init_data){
.name = "gcc_hmss_dvm_bus_clk",
+   .flags = CLK_IS_CRITICAL,
.ops = _branch2_ops,
},
},
@@ -2015,6 +2016,7 @@ static struct clk_branch gcc_lpass_at_clk = {
.enable_mask = BIT(0),
.hw.init = &(struct clk_init_data){
.name = "gcc_lpass_at_clk",
+   .flags = CLK_IS_CRITICAL,
.ops = _branch2_ops,
},
},
-- 
2.18.0

[PATCH 0/3] clk: qcom: gcc-msm8998: Fixes and clkref clocks

[Bjorn Andersson]

Mark critical clocks critical, don't halt-check UFS clocks and add clkref
branches.

Bjorn Andersson (3):
  clk: qcom: gcc-msm8998: Mark hmss_dvm and lpass_at critical
  clk: qcom: gcc-msm8998: Disable halt check of UFS clocks
  clk: qcom: gcc-msm8998: Add clkref clocks

 drivers/clk/qcom/gcc-msm8998.c   | 83 +++-
 include/dt-bindings/clock/qcom,gcc-msm8998.h |  5 ++
 2 files changed, 85 insertions(+), 3 deletions(-)

-- 
2.18.0

[PATCH 3/3] clk: qcom: gcc-msm8998: Add clkref clocks

[Bjorn Andersson]

Add clkref clocks for usb3, hdmi, ufs, pcie, and usb2. They are all
sourced off CXO_IN, so parent them off "xo" until a proper link to the
rpmcc can be described in DT.

Signed-off-by: Bjorn Andersson 
---
 drivers/clk/qcom/gcc-msm8998.c   | 75 
 include/dt-bindings/clock/qcom,gcc-msm8998.h |  5 ++
 2 files changed, 80 insertions(+)

diff --git a/drivers/clk/qcom/gcc-msm8998.c b/drivers/clk/qcom/gcc-msm8998.c
index 3d232d266d72..3cf16a4f931c 100644
--- a/drivers/clk/qcom/gcc-msm8998.c
+++ b/drivers/clk/qcom/gcc-msm8998.c
@@ -2543,6 +2543,76 @@ static struct clk_branch gcc_usb_phy_cfg_ahb2phy_clk = {
},
 };
 
+static struct clk_branch gcc_hdmi_clkref_clk = {
+   .halt_reg = 0x88000,
+   .clkr = {
+   .enable_reg = 0x88000,
+   .enable_mask = BIT(0),
+   .hw.init = &(struct clk_init_data){
+   .name = "gcc_hdmi_clkref_clk",
+   .parent_names = (const char *[]){ "xo" },
+   .num_parents = 1,
+   .ops = _branch2_ops,
+   },
+   },
+};
+
+static struct clk_branch gcc_ufs_clkref_clk = {
+   .halt_reg = 0x88004,
+   .clkr = {
+   .enable_reg = 0x88004,
+   .enable_mask = BIT(0),
+   .hw.init = &(struct clk_init_data){
+   .name = "gcc_ufs_clkref_clk",
+   .parent_names = (const char *[]){ "xo" },
+   .num_parents = 1,
+   .ops = _branch2_ops,
+   },
+   },
+};
+
+static struct clk_branch gcc_usb3_clkref_clk = {
+   .halt_reg = 0x88008,
+   .clkr = {
+   .enable_reg = 0x88008,
+   .enable_mask = BIT(0),
+   .hw.init = &(struct clk_init_data){
+   .name = "gcc_usb3_clkref_clk",
+   .parent_names = (const char *[]){ "xo" },
+   .num_parents = 1,
+   .ops = _branch2_ops,
+   },
+   },
+};
+
+static struct clk_branch gcc_pcie_clkref_clk = {
+   .halt_reg = 0x8800c,
+   .clkr = {
+   .enable_reg = 0x8800c,
+   .enable_mask = BIT(0),
+   .hw.init = &(struct clk_init_data){
+   .name = "gcc_pcie_clkref_clk",
+   .parent_names = (const char *[]){ "xo" },
+   .num_parents = 1,
+   .ops = _branch2_ops,
+   },
+   },
+};
+
+static struct clk_branch gcc_rx1_usb2_clkref_clk = {
+   .halt_reg = 0x88014,
+   .clkr = {
+   .enable_reg = 0x88014,
+   .enable_mask = BIT(0),
+   .hw.init = &(struct clk_init_data){
+   .name = "gcc_rx1_usb2_clkref_clk",
+   .parent_names = (const char *[]){ "xo" },
+   .num_parents = 1,
+   .ops = _branch2_ops,
+   },
+   },
+};
+
 static struct gdsc pcie_0_gdsc = {
.gdscr = 0x6b004,
.gds_hw_ctrl = 0x0,
@@ -2735,6 +2805,11 @@ static struct clk_regmap *gcc_msm8998_clocks[] = {
[USB30_MASTER_CLK_SRC] = _master_clk_src.clkr,
[USB30_MOCK_UTMI_CLK_SRC] = _mock_utmi_clk_src.clkr,
[USB3_PHY_AUX_CLK_SRC] = _phy_aux_clk_src.clkr,
+   [GCC_HDMI_CLKREF_CLK] = _hdmi_clkref_clk.clkr,
+   [GCC_UFS_CLKREF_CLK] = _ufs_clkref_clk.clkr,
+   [GCC_USB3_CLKREF_CLK] = _usb3_clkref_clk.clkr,
+   [GCC_PCIE_CLKREF_CLK] = _pcie_clkref_clk.clkr,
+   [GCC_RX1_USB2_CLKREF_CLK] = _rx1_usb2_clkref_clk.clkr,
 };
 
 static struct gdsc *gcc_msm8998_gdscs[] = {
diff --git a/include/dt-bindings/clock/qcom,gcc-msm8998.h 
b/include/dt-bindings/clock/qcom,gcc-msm8998.h
index 58a242e656b1..b3448800980a 100644
--- a/include/dt-bindings/clock/qcom,gcc-msm8998.h
+++ b/include/dt-bindings/clock/qcom,gcc-msm8998.h
@@ -180,6 +180,11 @@
 #define USB30_MASTER_CLK_SRC   163
 #define USB30_MOCK_UTMI_CLK_SRC164
 #define USB3_PHY_AUX_CLK_SRC   165
+#define GCC_USB3_CLKREF_CLK166
+#define GCC_HDMI_CLKREF_CLK167
+#define GCC_UFS_CLKREF_CLK 168
+#define GCC_PCIE_CLKREF_CLK169
+#define GCC_RX1_USB2_CLKREF_CLK170
 
 #define PCIE_0_GDSC0
 #define UFS_GDSC   1
-- 
2.18.0

[PATCH 1/3] clk: qcom: gcc-msm8998: Mark hmss_dvm and lpass_at critical

[Bjorn Andersson]

Keep the two clocks enabled, so that the platform passes
clk_disable_unused().

Signed-off-by: Bjorn Andersson 
---
 drivers/clk/qcom/gcc-msm8998.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/clk/qcom/gcc-msm8998.c b/drivers/clk/qcom/gcc-msm8998.c
index 9f0ae403d5f5..d89f8e7c2a59 100644
--- a/drivers/clk/qcom/gcc-msm8998.c
+++ b/drivers/clk/qcom/gcc-msm8998.c
@@ -1972,6 +1972,7 @@ static struct clk_branch gcc_hmss_dvm_bus_clk = {
.enable_mask = BIT(0),
.hw.init = &(struct clk_init_data){
.name = "gcc_hmss_dvm_bus_clk",
+   .flags = CLK_IS_CRITICAL,
.ops = _branch2_ops,
},
},
@@ -2015,6 +2016,7 @@ static struct clk_branch gcc_lpass_at_clk = {
.enable_mask = BIT(0),
.hw.init = &(struct clk_init_data){
.name = "gcc_lpass_at_clk",
+   .flags = CLK_IS_CRITICAL,
.ops = _branch2_ops,
},
},
-- 
2.18.0

[PATCH 0/3] clk: qcom: gcc-msm8998: Fixes and clkref clocks

[Bjorn Andersson]

Mark critical clocks critical, don't halt-check UFS clocks and add clkref
branches.

Bjorn Andersson (3):
  clk: qcom: gcc-msm8998: Mark hmss_dvm and lpass_at critical
  clk: qcom: gcc-msm8998: Disable halt check of UFS clocks
  clk: qcom: gcc-msm8998: Add clkref clocks

 drivers/clk/qcom/gcc-msm8998.c   | 83 +++-
 include/dt-bindings/clock/qcom,gcc-msm8998.h |  5 ++
 2 files changed, 85 insertions(+), 3 deletions(-)

-- 
2.18.0

Re: [PATCH v2 2/3] clk: mediatek: Add flags to mtk_gate

[Stephen Boyd]

Quoting matthias@kernel.org (2018-11-16 10:09:00)
> From: Jasper Mattsson 
> 
> This is required to mark gates as CLK_IS_CRITICAL.
> 
> Fixes: 96596aa06628 ("clk: mediatek: add clk support for MT6797")
> Signed-off-by: Jasper Mattsson 
> Signed-off-by: Matthias Brugger 
> ---

These other two look ok to me. I'll apply them to the clk tree and wait
to merge to clk-next in case someone from Mediatek wants to review these
changes.

Re: [PATCH v2 2/3] clk: mediatek: Add flags to mtk_gate

[Stephen Boyd]

Quoting matthias@kernel.org (2018-11-16 10:09:00)
> From: Jasper Mattsson 
> 
> This is required to mark gates as CLK_IS_CRITICAL.
> 
> Fixes: 96596aa06628 ("clk: mediatek: add clk support for MT6797")
> Signed-off-by: Jasper Mattsson 
> Signed-off-by: Matthias Brugger 
> ---

These other two look ok to me. I'll apply them to the clk tree and wait
to merge to clk-next in case someone from Mediatek wants to review these
changes.

Re: [PATCH v2 3/3] clk: mediatek: Mark bus and DRAM related clocks as critical

[Stephen Boyd]

Quoting matthias@kernel.org (2018-11-16 10:09:01)
> From: Jasper Mattsson 
> 
> This marks MUXes axi_sel and ddrphycfg_sel as well as gates
> infra_dramc_f26m and infra_dramc_b_f26m as with CLK_IS_CRITICAL.
> 
> Fixes: 96596aa06628 ("clk: mediatek: add clk support for MT6797")
> Signed-off-by: Jasper Mattsson 
> Signed-off-by: Matthias Brugger 
> ---

Can you add comments in the commit text and in the code about why the
CLK_IS_CRITICAL flag is added to these clks? It makes it easier to
figure out why the flag is there months from now when we all forget

RE: [RFC PATCH v2 02/15] usb:cdns3: Device side header file.

[PETER CHEN]

 
 
> +
> +/*
> + * USBSS-DEV register interface.
> + * This corresponds to the USBSS Device Controller Interface  */
> +/**
> + * struct xhci_cap_regs - xHCI Host Controller Capability Registers.

struct cdns3_usb_regs - device controller registers

> +struct cdns3_device;
> +
> +struct cdns3_endpoint {
> + struct usb_ep   endpoint;
> + struct list_headrequest_list;
> + struct list_headep_match_pending_list;
> +
> + struct cdns3_trb*trb_pool;
> + dma_addr_t  trb_pool_dma;
> +
> + struct cdns3_device *cdns3_dev;
> + charname[20];
> +
> +#define EP_ENABLED   BIT(0)
> +#define EP_STALL BIT(1)
> +#define EP_WEDGE BIT(2)
> +#define EP_TRANSFER_STARTED  BIT(3)
> +#define EP_UPDATE_EP_TRBADDR BIT(4)
> +#define EP_PENDING_REQUEST   BIT(5)
> +#define EP_USED  BIT(5)
> + u32 flags;
> +
> + void*aligned_buff;
> + dma_addr_t  aligned_dma_addr;
> + u8  dir;
> + u8  num;
> + u8  type;
> +
> + int free_trbs;
> + u8  pcs;
> + u8  ccs;
> + int enqueue;
> + int dequeue;
> +};
> +

Would you please add kernel doc for above structure?

> +struct cdns3_request {
> + struct usb_request request;
> + struct cdns3_endpoint *priv_ep;
> + struct cdns3_trb *trb;
> + int start_trb;
> + int end_trb;
> + int on_ring:1;
> +};
> +
> +#define to_cdns3_request(r) (container_of(r, struct cdns3_request,
> +request))
> +
> +struct cdns3_device {
> + struct device   dev;
> + struct cdns3_usb_regs   __iomem *regs;
> +
> + struct usb_gadget   gadget;
> + struct usb_gadget_driver*gadget_driver;
> +
> + struct usb_ctrlrequest  *setup;
> + dma_addr_t  setup_dma;
> + dma_addr_t  trb_ep0_dma;
> + struct cdns3_trb*trb_ep0;
> + void*zlp_buf;
> +
> + struct cdns3_endpoint   *eps[USB_SS_ENDPOINTS_MAX_COUNT];
> + int ep_nums;
> + /*
> +  * field used for improving performance. It holds the last
> +  * selected endpoint number
> +  */
> + u32 selected_ep;
> + struct usb_request  *ep0_request;
> + int ep0_data_dir;
> + int hw_configured_flag;
> + int wake_up_flag;
> + u16 isoch_delay;
> + /* generic spin-lock for drivers */
> + spinlock_t  lock;
> +
> + unsignedis_connected:1;
> + unsignedin_standby_mode:1;
> + unsignedstatus_completion_no_call:1;
> + unsignedu1_allowed:1;
> + unsignedu2_allowed:1;
> +
> + u32 usb_ien;
> + u32 ep_ien;
> + int setup_pending;
> + struct device   *sysdev;
> + /* The device mode is enabled */
> + int start_gadget;
> + struct list_headep_match_list;
> + /* KB */
> + int onchip_mem_allocated_size;
> + /* Memory is allocated for OUT */
> + int out_mem_is_allocated:1;
> + struct work_struct  pending_status_wq;
> + struct usb_request  *pending_status_request;
> +};
> +

kernel-doc please

Peter

Re: [PATCH v2 3/3] clk: mediatek: Mark bus and DRAM related clocks as critical

[Stephen Boyd]

Quoting matthias@kernel.org (2018-11-16 10:09:01)
> From: Jasper Mattsson 
> 
> This marks MUXes axi_sel and ddrphycfg_sel as well as gates
> infra_dramc_f26m and infra_dramc_b_f26m as with CLK_IS_CRITICAL.
> 
> Fixes: 96596aa06628 ("clk: mediatek: add clk support for MT6797")
> Signed-off-by: Jasper Mattsson 
> Signed-off-by: Matthias Brugger 
> ---

Can you add comments in the commit text and in the code about why the
CLK_IS_CRITICAL flag is added to these clks? It makes it easier to
figure out why the flag is there months from now when we all forget

RE: [RFC PATCH v2 02/15] usb:cdns3: Device side header file.

[PETER CHEN]

 
 
> +
> +/*
> + * USBSS-DEV register interface.
> + * This corresponds to the USBSS Device Controller Interface  */
> +/**
> + * struct xhci_cap_regs - xHCI Host Controller Capability Registers.

struct cdns3_usb_regs - device controller registers

> +struct cdns3_device;
> +
> +struct cdns3_endpoint {
> + struct usb_ep   endpoint;
> + struct list_headrequest_list;
> + struct list_headep_match_pending_list;
> +
> + struct cdns3_trb*trb_pool;
> + dma_addr_t  trb_pool_dma;
> +
> + struct cdns3_device *cdns3_dev;
> + charname[20];
> +
> +#define EP_ENABLED   BIT(0)
> +#define EP_STALL BIT(1)
> +#define EP_WEDGE BIT(2)
> +#define EP_TRANSFER_STARTED  BIT(3)
> +#define EP_UPDATE_EP_TRBADDR BIT(4)
> +#define EP_PENDING_REQUEST   BIT(5)
> +#define EP_USED  BIT(5)
> + u32 flags;
> +
> + void*aligned_buff;
> + dma_addr_t  aligned_dma_addr;
> + u8  dir;
> + u8  num;
> + u8  type;
> +
> + int free_trbs;
> + u8  pcs;
> + u8  ccs;
> + int enqueue;
> + int dequeue;
> +};
> +

Would you please add kernel doc for above structure?

> +struct cdns3_request {
> + struct usb_request request;
> + struct cdns3_endpoint *priv_ep;
> + struct cdns3_trb *trb;
> + int start_trb;
> + int end_trb;
> + int on_ring:1;
> +};
> +
> +#define to_cdns3_request(r) (container_of(r, struct cdns3_request,
> +request))
> +
> +struct cdns3_device {
> + struct device   dev;
> + struct cdns3_usb_regs   __iomem *regs;
> +
> + struct usb_gadget   gadget;
> + struct usb_gadget_driver*gadget_driver;
> +
> + struct usb_ctrlrequest  *setup;
> + dma_addr_t  setup_dma;
> + dma_addr_t  trb_ep0_dma;
> + struct cdns3_trb*trb_ep0;
> + void*zlp_buf;
> +
> + struct cdns3_endpoint   *eps[USB_SS_ENDPOINTS_MAX_COUNT];
> + int ep_nums;
> + /*
> +  * field used for improving performance. It holds the last
> +  * selected endpoint number
> +  */
> + u32 selected_ep;
> + struct usb_request  *ep0_request;
> + int ep0_data_dir;
> + int hw_configured_flag;
> + int wake_up_flag;
> + u16 isoch_delay;
> + /* generic spin-lock for drivers */
> + spinlock_t  lock;
> +
> + unsignedis_connected:1;
> + unsignedin_standby_mode:1;
> + unsignedstatus_completion_no_call:1;
> + unsignedu1_allowed:1;
> + unsignedu2_allowed:1;
> +
> + u32 usb_ien;
> + u32 ep_ien;
> + int setup_pending;
> + struct device   *sysdev;
> + /* The device mode is enabled */
> + int start_gadget;
> + struct list_headep_match_list;
> + /* KB */
> + int onchip_mem_allocated_size;
> + /* Memory is allocated for OUT */
> + int out_mem_is_allocated:1;
> + struct work_struct  pending_status_wq;
> + struct usb_request  *pending_status_request;
> +};
> +

kernel-doc please

Peter

[PATCH] HID: input: support Microsoft wireless radio control hotkey

[Chris Chiu]

The ASUS laptops start to support the airplane mode radio management
to replace the original machanism of airplane mode toggle hotkey.
On the ASUS P5440FF, it presents as a HID device connecting via
I2C, name i2c-AMPD0001. When pressing hotkey, the Embedded Controller
send hid report up via I2C and switch the airplane mode indicator
LED based on the status.

However, it's not working because it fails to be identified as a
hidinput device. It fails in hidinput_connect() due to the macro
IS_INPUT_APPLICATION doesn't identify HID_GD_WIRELESS_RADIO_CTLS
as a legit application code.

It's easy to add the HID I2C vendor and product id to the quirk
list and apply HID_QUIRK_HIDINPUT_FORCE to make it work. But it
can be more generic to support such kind of application on PC.

Signed-off-by: Chris Chiu 
---
 include/linux/hid.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/hid.h b/include/linux/hid.h
index d44a78362942..f4805f605fed 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -836,7 +836,7 @@ static inline bool hid_is_using_ll_driver(struct hid_device 
*hdev,
 
 /* Applications from HID Usage Tables 4/8/99 Version 1.1 */
 /* We ignore a few input applications that are not widely used */
-#define IS_INPUT_APPLICATION(a) (((a >= 0x0001) && (a <= 0x00010008)) || 
(a == 0x00010080) || (a == 0x000c0001) || ((a >= 0x000d0002) && (a <= 
0x000d0006)))
+#define IS_INPUT_APPLICATION(a) (((a >= 0x0001) && (a <= 0x00010008)) || 
(a == 0x00010080) || || (a == 0x0001000c) || (a == 0x000c0001) || ((a >= 
0x000d0002) && (a <= 0x000d0006)))
 
 /* HID core API */
 
-- 
2.11.0

[PATCH] HID: input: support Microsoft wireless radio control hotkey

[Chris Chiu]

The ASUS laptops start to support the airplane mode radio management
to replace the original machanism of airplane mode toggle hotkey.
On the ASUS P5440FF, it presents as a HID device connecting via
I2C, name i2c-AMPD0001. When pressing hotkey, the Embedded Controller
send hid report up via I2C and switch the airplane mode indicator
LED based on the status.

However, it's not working because it fails to be identified as a
hidinput device. It fails in hidinput_connect() due to the macro
IS_INPUT_APPLICATION doesn't identify HID_GD_WIRELESS_RADIO_CTLS
as a legit application code.

It's easy to add the HID I2C vendor and product id to the quirk
list and apply HID_QUIRK_HIDINPUT_FORCE to make it work. But it
can be more generic to support such kind of application on PC.

Signed-off-by: Chris Chiu 
---
 include/linux/hid.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/hid.h b/include/linux/hid.h
index d44a78362942..f4805f605fed 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -836,7 +836,7 @@ static inline bool hid_is_using_ll_driver(struct hid_device 
*hdev,
 
 /* Applications from HID Usage Tables 4/8/99 Version 1.1 */
 /* We ignore a few input applications that are not widely used */
-#define IS_INPUT_APPLICATION(a) (((a >= 0x0001) && (a <= 0x00010008)) || 
(a == 0x00010080) || (a == 0x000c0001) || ((a >= 0x000d0002) && (a <= 
0x000d0006)))
+#define IS_INPUT_APPLICATION(a) (((a >= 0x0001) && (a <= 0x00010008)) || 
(a == 0x00010080) || || (a == 0x0001000c) || (a == 0x000c0001) || ((a >= 
0x000d0002) && (a <= 0x000d0006)))
 
 /* HID core API */
 
-- 
2.11.0

Re: [PATCH v2 0/3] Mark clocks as critical for MT6797

[Stephen Boyd]

Quoting matthias@kernel.org (2018-11-16 10:08:58)
> From: Matthias Brugger 
> 
> Jasper send this series some month ago. As there was no reaction from
> his side, I'll do a friendly take-over.
> I tested the patches on my Helios X20 boards and they fix the issue.
> I didn't add a Tested-by tag as I added my Signed-off-by.
> 
> Changes since v1:
> - add a fixes tag.
> 
> ---
> 
> Currently, DRAM-related clocks and the axi_sel MUX are not marked with
> CLK_IS_CRITICAL for MT6797. This causes memory corruption when the
> system is booted without clk_ignore_unused.
> 
> This patchset
> 
> 1. Makes it possible to mark outputs of MUXes as critical by introducing
>a new macro, MUX_FLAGS,
> 2. Makes it possible to mark gates as critical by adding flags to
>mtk_gate, and
> 3. Marks axi_sel, ddrphycfg_sel, infra_dramc_f26m and infra_dramc_b_f26m
>as critical.
> 
> The addition of flags to mtk_gate also exists in the patch series "Add
> basic and clock support for Mediatek MT8183 SoC" [1].  The type of
> flags is unsigned int in that series, but the real type is unsigned
> long, so my patch differs from that patch.

Will anyone from Mediatek review this? Why aren't the people who signed
off on drivers/clk/mediatek/clk-mt6797.c included on this patch series?
They no longer work there?

Re: [PATCH v2 0/3] Mark clocks as critical for MT6797

[Stephen Boyd]

Quoting matthias@kernel.org (2018-11-16 10:08:58)
> From: Matthias Brugger 
> 
> Jasper send this series some month ago. As there was no reaction from
> his side, I'll do a friendly take-over.
> I tested the patches on my Helios X20 boards and they fix the issue.
> I didn't add a Tested-by tag as I added my Signed-off-by.
> 
> Changes since v1:
> - add a fixes tag.
> 
> ---
> 
> Currently, DRAM-related clocks and the axi_sel MUX are not marked with
> CLK_IS_CRITICAL for MT6797. This causes memory corruption when the
> system is booted without clk_ignore_unused.
> 
> This patchset
> 
> 1. Makes it possible to mark outputs of MUXes as critical by introducing
>a new macro, MUX_FLAGS,
> 2. Makes it possible to mark gates as critical by adding flags to
>mtk_gate, and
> 3. Marks axi_sel, ddrphycfg_sel, infra_dramc_f26m and infra_dramc_b_f26m
>as critical.
> 
> The addition of flags to mtk_gate also exists in the patch series "Add
> basic and clock support for Mediatek MT8183 SoC" [1].  The type of
> flags is unsigned int in that series, but the real type is unsigned
> long, so my patch differs from that patch.

Will anyone from Mediatek review this? Why aren't the people who signed
off on drivers/clk/mediatek/clk-mt6797.c included on this patch series?
They no longer work there?

arch/x86/include/asm/cmpxchg.h:245:2: error: 'asm' operand has impossible constraints

[kbuild test robot]

Hi Juergen,

FYI, the error/warning still remains.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
master
head:   94f371cb73944b410a269d570d6946c042f2ddd0
commit: 6da63eb241a05b0e676d68975e793c0521387141 x86/paravirt: Move the 
pv_irq_ops under the PARAVIRT_XXL umbrella
date:   3 months ago
config: i386-randconfig-sb0-11301144 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
git checkout 6da63eb241a05b0e676d68975e793c0521387141
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

   In file included from arch/x86/include/asm/atomic.h:8:0,
from arch/x86/include/asm/msr.h:67,
from arch/x86/include/asm/processor.h:21,
from arch/x86/include/asm/cpufeature.h:5,
from arch/x86/include/asm/thread_info.h:53,
from include/linux/thread_info.h:38,
from arch/x86/include/asm/preempt.h:7,
from include/linux/preempt.h:81,
from include/linux/spinlock.h:51,
from include/linux/mmzone.h:8,
from include/linux/gfp.h:6,
from include/linux/mm.h:10,
from mm/slub.c:13:
   mm/slub.c: In function '__slab_free':
>> arch/x86/include/asm/cmpxchg.h:245:2: error: 'asm' operand has impossible 
>> constraints
 asm volatile(pfx "cmpxchg%c4b %2; sete %0"   \
 ^
   arch/x86/include/asm/cmpxchg.h:254:2: note: in expansion of macro 
'__cmpxchg_double'
 __cmpxchg_double(LOCK_PREFIX, p1, p2, o1, o2, n1, n2)
 ^
   include/asm-generic/atomic-instrumented.h:457:2: note: in expansion of macro 
'arch_cmpxchg_double'
 arch_cmpxchg_double(__ai_p1, (p2), (o1), (o2), (n1), (n2)); \
 ^
   mm/slub.c:403:7: note: in expansion of macro 'cmpxchg_double'
  if (cmpxchg_double(>freelist, >counters,
  ^

vim +/asm +245 arch/x86/include/asm/cmpxchg.h

3d94ae0c Jeremy Fitzhardinge 2011-09-28  235  
cdcd6298 Jan Beulich 2012-01-02  236  #define __cmpxchg_double(pfx, p1, 
p2, o1, o2, n1, n2) \
cdcd6298 Jan Beulich 2012-01-02  237  ({
\
cdcd6298 Jan Beulich 2012-01-02  238bool __ret; 
\
cdcd6298 Jan Beulich 2012-01-02  239__typeof__(*(p1)) __old1 = 
(o1), __new1 = (n1); \
cdcd6298 Jan Beulich 2012-01-02  240__typeof__(*(p2)) __old2 = 
(o2), __new2 = (n2); \
cdcd6298 Jan Beulich 2012-01-02  241BUILD_BUG_ON(sizeof(*(p1)) != 
sizeof(long));\
cdcd6298 Jan Beulich 2012-01-02  242BUILD_BUG_ON(sizeof(*(p2)) != 
sizeof(long));\
cdcd6298 Jan Beulich 2012-01-02  243VM_BUG_ON((unsigned long)(p1) % 
(2 * sizeof(long)));\
cdcd6298 Jan Beulich 2012-01-02  244VM_BUG_ON((unsigned long)((p1) 
+ 1) != (unsigned long)(p2));\
cdcd6298 Jan Beulich 2012-01-02 @245asm volatile(pfx "cmpxchg%c4b 
%2; sete %0"  \
cdcd6298 Jan Beulich 2012-01-02  246 : "=a" (__ret), 
"+d" (__old2), \
cdcd6298 Jan Beulich 2012-01-02  247   "+m" (*(p1)), 
"+m" (*(p2))   \
cdcd6298 Jan Beulich 2012-01-02  248 : "i" (2 * 
sizeof(long)), "a" (__old1),\
cdcd6298 Jan Beulich 2012-01-02  249   "b" (__new1), 
"c" (__new2)); \
cdcd6298 Jan Beulich 2012-01-02  250__ret;  
\
cdcd6298 Jan Beulich 2012-01-02  251  })
cdcd6298 Jan Beulich 2012-01-02  252  

:: The code at line 245 was first introduced by commit
:: cdcd629869fabcd38ebd24a03b0a05ec1cbcafb0 x86: Fix and improve 
cmpxchg_double{,_local}()

:: TO: Jan Beulich 
:: CC: Ingo Molnar 

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

arch/x86/include/asm/cmpxchg.h:245:2: error: 'asm' operand has impossible constraints

[kbuild test robot]

Hi Juergen,

FYI, the error/warning still remains.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
master
head:   94f371cb73944b410a269d570d6946c042f2ddd0
commit: 6da63eb241a05b0e676d68975e793c0521387141 x86/paravirt: Move the 
pv_irq_ops under the PARAVIRT_XXL umbrella
date:   3 months ago
config: i386-randconfig-sb0-11301144 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
git checkout 6da63eb241a05b0e676d68975e793c0521387141
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

   In file included from arch/x86/include/asm/atomic.h:8:0,
from arch/x86/include/asm/msr.h:67,
from arch/x86/include/asm/processor.h:21,
from arch/x86/include/asm/cpufeature.h:5,
from arch/x86/include/asm/thread_info.h:53,
from include/linux/thread_info.h:38,
from arch/x86/include/asm/preempt.h:7,
from include/linux/preempt.h:81,
from include/linux/spinlock.h:51,
from include/linux/mmzone.h:8,
from include/linux/gfp.h:6,
from include/linux/mm.h:10,
from mm/slub.c:13:
   mm/slub.c: In function '__slab_free':
>> arch/x86/include/asm/cmpxchg.h:245:2: error: 'asm' operand has impossible 
>> constraints
 asm volatile(pfx "cmpxchg%c4b %2; sete %0"   \
 ^
   arch/x86/include/asm/cmpxchg.h:254:2: note: in expansion of macro 
'__cmpxchg_double'
 __cmpxchg_double(LOCK_PREFIX, p1, p2, o1, o2, n1, n2)
 ^
   include/asm-generic/atomic-instrumented.h:457:2: note: in expansion of macro 
'arch_cmpxchg_double'
 arch_cmpxchg_double(__ai_p1, (p2), (o1), (o2), (n1), (n2)); \
 ^
   mm/slub.c:403:7: note: in expansion of macro 'cmpxchg_double'
  if (cmpxchg_double(>freelist, >counters,
  ^

vim +/asm +245 arch/x86/include/asm/cmpxchg.h

3d94ae0c Jeremy Fitzhardinge 2011-09-28  235  
cdcd6298 Jan Beulich 2012-01-02  236  #define __cmpxchg_double(pfx, p1, 
p2, o1, o2, n1, n2) \
cdcd6298 Jan Beulich 2012-01-02  237  ({
\
cdcd6298 Jan Beulich 2012-01-02  238bool __ret; 
\
cdcd6298 Jan Beulich 2012-01-02  239__typeof__(*(p1)) __old1 = 
(o1), __new1 = (n1); \
cdcd6298 Jan Beulich 2012-01-02  240__typeof__(*(p2)) __old2 = 
(o2), __new2 = (n2); \
cdcd6298 Jan Beulich 2012-01-02  241BUILD_BUG_ON(sizeof(*(p1)) != 
sizeof(long));\
cdcd6298 Jan Beulich 2012-01-02  242BUILD_BUG_ON(sizeof(*(p2)) != 
sizeof(long));\
cdcd6298 Jan Beulich 2012-01-02  243VM_BUG_ON((unsigned long)(p1) % 
(2 * sizeof(long)));\
cdcd6298 Jan Beulich 2012-01-02  244VM_BUG_ON((unsigned long)((p1) 
+ 1) != (unsigned long)(p2));\
cdcd6298 Jan Beulich 2012-01-02 @245asm volatile(pfx "cmpxchg%c4b 
%2; sete %0"  \
cdcd6298 Jan Beulich 2012-01-02  246 : "=a" (__ret), 
"+d" (__old2), \
cdcd6298 Jan Beulich 2012-01-02  247   "+m" (*(p1)), 
"+m" (*(p2))   \
cdcd6298 Jan Beulich 2012-01-02  248 : "i" (2 * 
sizeof(long)), "a" (__old1),\
cdcd6298 Jan Beulich 2012-01-02  249   "b" (__new1), 
"c" (__new2)); \
cdcd6298 Jan Beulich 2012-01-02  250__ret;  
\
cdcd6298 Jan Beulich 2012-01-02  251  })
cdcd6298 Jan Beulich 2012-01-02  252  

:: The code at line 245 was first introduced by commit
:: cdcd629869fabcd38ebd24a03b0a05ec1cbcafb0 x86: Fix and improve 
cmpxchg_double{,_local}()

:: TO: Jan Beulich 
:: CC: Ingo Molnar 

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH v12 0/5] x86/boot/KASLR: Parse ACPI table and limit KASLR to choosing immovable memory

[Chao Fan]

On Fri, Nov 30, 2018 at 09:15:13AM +0800, Chao Fan wrote:
>On Thu, Nov 29, 2018 at 12:32:46PM -0500, Masayoshi Mizuma wrote:
>>Hi Chao,
>>
>>Thank you for your continued working.
>
>Thanks for your test.
>
>>
>>Could you please build your patches before sending?
>
>Sorry for the mistake, I build it with the whole patches.
>I found there are some problems with the method to splite patch.
>I will rework on it and build every commit.
>
>Thanks,
>Chao Fan
>
>>Your patches depend on the following kconfig,
>>so please build them under the config combination.
>>
>>RANDOMIZE_BASE
>>MEMORY_HOTREMOVE
>>EARLY_PARSE_RSDP
>>KEXEC
>>EFI

Hi Masa,

After retest, I make clean and make again.
I met the problem is the useless #endif issue you mentioned.
But I found both:

#ifndef BOOT_STRING
EXPORT_SYMBOL(kstrtoull);

and

EXPORT_SYMBOL(kstrtoull);
#ifndef BOOT_STRING

work for me. So I don't know the key problem, cause the config you
mentioned are y.

Thanks,
Chao Fan

>>
>>Thanks,
>>Masa
>>
>>On Thu, Nov 29, 2018 at 04:16:26PM +0800, Chao Fan wrote:
>>> ***Background:
>>> People reported that KASLR may randomly choose some positions
>>> which are located in movable memory regions. This will break memory
>>> hotplug feature and make the movable memory chosen by KASLR can't be
>>> removed.
>>> 
>>> ***Solutions:
>>> Get the information of memory hot-remove, then KASLR will know the
>>> right regions. Information about memory hot-remove is in ACPI
>>> tables, which will be parsed after start_kernel(), so that KASLR
>>> can't get the information.
>>> 
>>> Somebody suggest to add a kernel parameter to specify the
>>> immovable memory so that limit KASLR in these regions. Then I make
>>> a patchset. After several versions, Ingo gave a suggestion:
>>> https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1634024.html
>>> Follow Ingo's suggestion, imitate the ACPI code to parse the ACPI
>>> tables, so that the kaslr can get necessary memory information in
>>> ACPI tables.
>>> I think ACPI code is an independent part, so imitate the codes
>>> and functions to 'compressed/' directory, so that kaslr won't
>>> influence the initialization of ACPI.
>>> 
>>> PATCH 1/5 Add get_acpi_rsdp() to parse RSDP in cmdline from KEXEC
>>> PATCH 2/5 Add efi_get_rsdp_addr() to find RSDP from EFI table when
>>>   booting from EFI.
>>> PATCH 3/5 Add bios_get_rsdp_addr() to search RSDP in memory when EFI
>>>   table not found.
>>> PATCH 4/5 Compute SRAT table from RSDP and walk SRAT table to store
>>>   the immovable memory regions.
>>> PATCH 5/5 Calculate the intersection between memory regions from e820/efi
>>>   memory table and immovable memory regions. Limit KASLR to
>>>   choosing these regions for randomization.
>>> 
>>> v1->v2:
>>>  -  Simplify some code.
>>> Follow Baoquan He's suggestion:
>>>  - Reuse the head file of acpi code.
>>> 
>>> v2->v3:
>>>  - Test in more conditions, so remove the 'RFC' tag.
>>>  - Change some comments.
>>> 
>>> v3->v4:
>>> Follow Thomas Gleixner's suggetsion:
>>>  - Put the whole efi related function into #define CONFIG_EFI and return
>>>false in the other stub.
>>> 
>>> v4->v5:
>>> Follow Dou Liyang's suggestion:
>>>  - Add more comments about some functions based on kernel code.
>>>  - Change some typo in comments.
>>>  - Clean useless variable.
>>>  - Add check for the boundary of array.
>>>  - Add check for 'movable_node' parameter
>>> 
>>> v5->v6:
>>> Follow Baoquan He's suggestion:
>>>  - Change some log.
>>>  - Add the check for acpi_rsdp
>>>  - Change some code logical to make code clear
>>> 
>>> v6->v7:
>>> Follow Rafael's suggestion:
>>>  - Add more comments and patch log.
>>> Follow test robot's suggestion:
>>>  - Add "static" tag for function
>>> 
>>> v7-v8:
>>> Follow Kees Cook's suggestion:
>>>  - Use mem_overlaps() to check memory region.
>>>  - Use #ifdef in the definition of function.
>>> 
>>> v8-v9:
>>> Follow Boris' suggestion:
>>>  - Change code style.
>>>  - Splite PATCH 1/3 to more path.
>>>  - Introduce some new function
>>>  - Use existing function to rework some code
>>> Follow Masayoshi's suggetion:
>>>  - Make code more readable
>>> 
>>> v9->v10:
>>> Follow Baoquan's suggestion:
>>>  - Change some log
>>>  - Merge last two patch together.
>>> 
>>> v10->v11:
>>> Follow Boris' suggestion:
>>>  - Link kstrtoull() instead of copying it.
>>>  - Drop the useless wrapped function.
>>> 
>>> v11->v12:
>>> Follow Boris' suggestion:
>>>  - Change patch log and code comments.
>>>  - Add 'CONFIG_EARLY_PARSE_RSDP' to make code easy to read
>>>  - Put strtoull() to misc.c
>>> Follow Masa's suggestion:
>>>  - Remove the detection for 'movable_node'
>>>  - Change the code logical about cmdline_find_option()
>>> 
>>> Any comments will be welcome.
>>> 
>>> 
>>> Chao Fan (5):
>>>   x86/boot: Add get_acpi_rsdp() to parse RSDP in cmdline from KEXEC
>>>   x86/boot: Add efi_get_rsdp_addr() to find RSDP from EFI table
>>>   x86/boot: Add bios_get_rsdp_addr() to 

Re: [PATCH v2] arm64: dts: sdm845: add video nodes

[Alexandre Courbot]

On Wed, Nov 28, 2018 at 10:12 PM Malathi Gottam  wrote:
>
> This adds video nodes to sdm845 based on the examples
> in the bindings.
>
> Signed-off-by: Malathi Gottam 
> ---
>  arch/arm64/boot/dts/qcom/sdm845.dtsi | 35 +++
>  1 file changed, 35 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi 
> b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> index 0c9a2aa..4c9d6a4 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -84,6 +84,11 @@
> reg = <0 0x8620 0 0x2d0>;
> no-map;
> };
> +
> +   venus_region: memory@9580 {
> +   reg = <0x0 0x9580 0x0 0x50>;

This patch depends on the firmware loader being fixed to not expect a
6MB area size. I think you can do it as follows:

1) Record the reserved area size in the venus_core structure during
venus_load_fw(),
2) Use that area size in place of VENUS_FW_MEM_SIZE everywhere else in the code.

That way we don't put any artificial limitation on the expected
firmware size, or on the reserved area in the DT matching a hardcoded
size.

Once you have this, the driver should work no matter what the size of
the reserved area is, provided the firmware first into it.

> +   no-map;
> +   };
> };
>
> cpus {
> @@ -1103,5 +1108,35 @@
> status = "disabled";
> };
> };
> +
> +   video-codec@aa0 {
> +   compatible = "qcom,sdm845-venus";
> +   reg = <0x0aa0 0xff000>;
> +   interrupts = ;
> +   power-domains = < VENUS_GDSC>;
> +   clocks = < VIDEO_CC_VENUS_CTL_CORE_CLK>,
> +< VIDEO_CC_VENUS_AHB_CLK>,
> +< VIDEO_CC_VENUS_CTL_AXI_CLK>;
> +   clock-names = "core", "iface", "bus";
> +   iommus = <_smmu 0x10a0 0x8>,
> +<_smmu 0x10b0 0x0>;
> +   memory-region = <_region>;
> +
> +   video-core0 {
> +   compatible = "venus-decoder";
> +   clocks = < VIDEO_CC_VCODEC0_CORE_CLK>,
> +< VIDEO_CC_VCODEC0_AXI_CLK>;
> +   clock-names = "core", "bus";
> +   power-domains = < VCODEC0_GDSC>;
> +   };
> +
> +   video-core1 {
> +   compatible = "venus-encoder";
> +   clocks = < VIDEO_CC_VCODEC1_CORE_CLK>,
> +< VIDEO_CC_VCODEC1_AXI_CLK>;
> +   clock-names = "core", "bus";
> +   power-domains = < VCODEC1_GDSC>;
> +   };

We should probably have status = "disabled" here and enable this node
on a per-board basis?

Also, shouldn't we define the firmware subnode here too?

Cheers,
Alex.
> --
> 1.9.1
>

Re: [PATCH v12 0/5] x86/boot/KASLR: Parse ACPI table and limit KASLR to choosing immovable memory

[Chao Fan]

On Fri, Nov 30, 2018 at 09:15:13AM +0800, Chao Fan wrote:
>On Thu, Nov 29, 2018 at 12:32:46PM -0500, Masayoshi Mizuma wrote:
>>Hi Chao,
>>
>>Thank you for your continued working.
>
>Thanks for your test.
>
>>
>>Could you please build your patches before sending?
>
>Sorry for the mistake, I build it with the whole patches.
>I found there are some problems with the method to splite patch.
>I will rework on it and build every commit.
>
>Thanks,
>Chao Fan
>
>>Your patches depend on the following kconfig,
>>so please build them under the config combination.
>>
>>RANDOMIZE_BASE
>>MEMORY_HOTREMOVE
>>EARLY_PARSE_RSDP
>>KEXEC
>>EFI

Hi Masa,

After retest, I make clean and make again.
I met the problem is the useless #endif issue you mentioned.
But I found both:

#ifndef BOOT_STRING
EXPORT_SYMBOL(kstrtoull);

and

EXPORT_SYMBOL(kstrtoull);
#ifndef BOOT_STRING

work for me. So I don't know the key problem, cause the config you
mentioned are y.

Thanks,
Chao Fan

>>
>>Thanks,
>>Masa
>>
>>On Thu, Nov 29, 2018 at 04:16:26PM +0800, Chao Fan wrote:
>>> ***Background:
>>> People reported that KASLR may randomly choose some positions
>>> which are located in movable memory regions. This will break memory
>>> hotplug feature and make the movable memory chosen by KASLR can't be
>>> removed.
>>> 
>>> ***Solutions:
>>> Get the information of memory hot-remove, then KASLR will know the
>>> right regions. Information about memory hot-remove is in ACPI
>>> tables, which will be parsed after start_kernel(), so that KASLR
>>> can't get the information.
>>> 
>>> Somebody suggest to add a kernel parameter to specify the
>>> immovable memory so that limit KASLR in these regions. Then I make
>>> a patchset. After several versions, Ingo gave a suggestion:
>>> https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1634024.html
>>> Follow Ingo's suggestion, imitate the ACPI code to parse the ACPI
>>> tables, so that the kaslr can get necessary memory information in
>>> ACPI tables.
>>> I think ACPI code is an independent part, so imitate the codes
>>> and functions to 'compressed/' directory, so that kaslr won't
>>> influence the initialization of ACPI.
>>> 
>>> PATCH 1/5 Add get_acpi_rsdp() to parse RSDP in cmdline from KEXEC
>>> PATCH 2/5 Add efi_get_rsdp_addr() to find RSDP from EFI table when
>>>   booting from EFI.
>>> PATCH 3/5 Add bios_get_rsdp_addr() to search RSDP in memory when EFI
>>>   table not found.
>>> PATCH 4/5 Compute SRAT table from RSDP and walk SRAT table to store
>>>   the immovable memory regions.
>>> PATCH 5/5 Calculate the intersection between memory regions from e820/efi
>>>   memory table and immovable memory regions. Limit KASLR to
>>>   choosing these regions for randomization.
>>> 
>>> v1->v2:
>>>  -  Simplify some code.
>>> Follow Baoquan He's suggestion:
>>>  - Reuse the head file of acpi code.
>>> 
>>> v2->v3:
>>>  - Test in more conditions, so remove the 'RFC' tag.
>>>  - Change some comments.
>>> 
>>> v3->v4:
>>> Follow Thomas Gleixner's suggetsion:
>>>  - Put the whole efi related function into #define CONFIG_EFI and return
>>>false in the other stub.
>>> 
>>> v4->v5:
>>> Follow Dou Liyang's suggestion:
>>>  - Add more comments about some functions based on kernel code.
>>>  - Change some typo in comments.
>>>  - Clean useless variable.
>>>  - Add check for the boundary of array.
>>>  - Add check for 'movable_node' parameter
>>> 
>>> v5->v6:
>>> Follow Baoquan He's suggestion:
>>>  - Change some log.
>>>  - Add the check for acpi_rsdp
>>>  - Change some code logical to make code clear
>>> 
>>> v6->v7:
>>> Follow Rafael's suggestion:
>>>  - Add more comments and patch log.
>>> Follow test robot's suggestion:
>>>  - Add "static" tag for function
>>> 
>>> v7-v8:
>>> Follow Kees Cook's suggestion:
>>>  - Use mem_overlaps() to check memory region.
>>>  - Use #ifdef in the definition of function.
>>> 
>>> v8-v9:
>>> Follow Boris' suggestion:
>>>  - Change code style.
>>>  - Splite PATCH 1/3 to more path.
>>>  - Introduce some new function
>>>  - Use existing function to rework some code
>>> Follow Masayoshi's suggetion:
>>>  - Make code more readable
>>> 
>>> v9->v10:
>>> Follow Baoquan's suggestion:
>>>  - Change some log
>>>  - Merge last two patch together.
>>> 
>>> v10->v11:
>>> Follow Boris' suggestion:
>>>  - Link kstrtoull() instead of copying it.
>>>  - Drop the useless wrapped function.
>>> 
>>> v11->v12:
>>> Follow Boris' suggestion:
>>>  - Change patch log and code comments.
>>>  - Add 'CONFIG_EARLY_PARSE_RSDP' to make code easy to read
>>>  - Put strtoull() to misc.c
>>> Follow Masa's suggestion:
>>>  - Remove the detection for 'movable_node'
>>>  - Change the code logical about cmdline_find_option()
>>> 
>>> Any comments will be welcome.
>>> 
>>> 
>>> Chao Fan (5):
>>>   x86/boot: Add get_acpi_rsdp() to parse RSDP in cmdline from KEXEC
>>>   x86/boot: Add efi_get_rsdp_addr() to find RSDP from EFI table
>>>   x86/boot: Add bios_get_rsdp_addr() to 

Re: [PATCH v2] arm64: dts: sdm845: add video nodes

[Alexandre Courbot]

On Wed, Nov 28, 2018 at 10:12 PM Malathi Gottam  wrote:
>
> This adds video nodes to sdm845 based on the examples
> in the bindings.
>
> Signed-off-by: Malathi Gottam 
> ---
>  arch/arm64/boot/dts/qcom/sdm845.dtsi | 35 +++
>  1 file changed, 35 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi 
> b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> index 0c9a2aa..4c9d6a4 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -84,6 +84,11 @@
> reg = <0 0x8620 0 0x2d0>;
> no-map;
> };
> +
> +   venus_region: memory@9580 {
> +   reg = <0x0 0x9580 0x0 0x50>;

This patch depends on the firmware loader being fixed to not expect a
6MB area size. I think you can do it as follows:

1) Record the reserved area size in the venus_core structure during
venus_load_fw(),
2) Use that area size in place of VENUS_FW_MEM_SIZE everywhere else in the code.

That way we don't put any artificial limitation on the expected
firmware size, or on the reserved area in the DT matching a hardcoded
size.

Once you have this, the driver should work no matter what the size of
the reserved area is, provided the firmware first into it.

> +   no-map;
> +   };
> };
>
> cpus {
> @@ -1103,5 +1108,35 @@
> status = "disabled";
> };
> };
> +
> +   video-codec@aa0 {
> +   compatible = "qcom,sdm845-venus";
> +   reg = <0x0aa0 0xff000>;
> +   interrupts = ;
> +   power-domains = < VENUS_GDSC>;
> +   clocks = < VIDEO_CC_VENUS_CTL_CORE_CLK>,
> +< VIDEO_CC_VENUS_AHB_CLK>,
> +< VIDEO_CC_VENUS_CTL_AXI_CLK>;
> +   clock-names = "core", "iface", "bus";
> +   iommus = <_smmu 0x10a0 0x8>,
> +<_smmu 0x10b0 0x0>;
> +   memory-region = <_region>;
> +
> +   video-core0 {
> +   compatible = "venus-decoder";
> +   clocks = < VIDEO_CC_VCODEC0_CORE_CLK>,
> +< VIDEO_CC_VCODEC0_AXI_CLK>;
> +   clock-names = "core", "bus";
> +   power-domains = < VCODEC0_GDSC>;
> +   };
> +
> +   video-core1 {
> +   compatible = "venus-encoder";
> +   clocks = < VIDEO_CC_VCODEC1_CORE_CLK>,
> +< VIDEO_CC_VCODEC1_AXI_CLK>;
> +   clock-names = "core", "bus";
> +   power-domains = < VCODEC1_GDSC>;
> +   };

We should probably have status = "disabled" here and enable this node
on a per-board basis?

Also, shouldn't we define the firmware subnode here too?

Cheers,
Alex.
> --
> 1.9.1
>