Re: [PATCH 1/2] dt-bindings: irqchip: Introduce TISCI Interrupt router bindings

2018-10-06 Thread Marc Zyngier
On Sat, 06 Oct 2018 08:28:11 +0100,
Lokesh Vutla  wrote:
> 
> Add the DT binding documentation for Interrupt router driver.
> 
> Signed-off-by: Lokesh Vutla 
> ---
>  .../interrupt-controller/ti,sci-intr.txt  | 83 +++
>  MAINTAINERS   |  1 +
>  2 files changed, 84 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
> 
> diff --git 
> a/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt 
> b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
> new file mode 100644
> index ..681ca53cc5fb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
> @@ -0,0 +1,83 @@
> +Texas Instruments K3 Interrupt Router
> +=
> +
> +The Interrupt Router (INTR) module provides a mechanism to mux M
> +interrupt inputs to N interrupt outputs, where all M inputs are selectable
> +to be driven per N output. There is one register per output (MUXCNTL_N) that
> +controls the selection.
> +
> +
> + Interrupt Router
> + +--+
> + |  Inputs Outputs  |
> ++---+| +--+ |
> +| GPIO  |--->| | irq0 | |   Host IRQ
> ++---+| +--+ |  controller
> + |.+-+  |  +---+
> ++---+|.|  0  |  |->|  GIC  |
> +| INTA  |--->|.+-+  |  +---+
> ++---+|.  .  |
> + | +--+  .  |
> + | | irqM |+-+  |
> + | +--+|  N  |  |
> + | +-+  |
> + +--+
> +
> +Configuration of these MUXCNTL_N registers is done by a system controller
> +(like the Device Memory and Security Controller on K3 AM654 SoC). System
> +controller will keep track of the used and unused registers within the 
> Router.
> +Driver should request the system controller to get the range of GIC IRQs
> +assigned to the requesting hosts. It is the drivers responsibility to keep
> +track of GIC IRQs.

I would drop the GIC here, and replace it by "parent interrupt
controller", as nothing here is GIC specific.

> +
> +Communication between the host processor running an OS and the system
> +controller happens through a protocol called TI System Control Interface
> +(TISCI protocol). For more details refer:
> +Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
> +
> +TISCI Interrupt Router Node:
> +
> +- compatible:Must be "ti,sci-intr".
> +- interrupt-controller:  Identifies the node as an interrupt controller
> +- #interrupt-cells:  Specifies the number of cells needed to encode an
> + interrupt source. The value should be 3.
> + First cell should contain the TISCI device ID of source
> + Second cell should contain the interrupt source offset
> + within the device
> + Third cell specifies the trigger type as defined
> + in interrupts.txt in this directory.

Are all trigger types supported?

> +- interrupt-parent:  phandle of irq parent for TISCI intr. The parent must
> + use the same interrupt-cells format as GIC.

Why that constraint? From what I can see, the two are fairly
independent, and the constraint looks more of a Linux driver issue
than a DT constraint.

> +- ti,sci:Phandle to TI-SCI compatible System controller node.
> +- ti,sci-dst-id: TISCI device ID of the destination IRQ controller.
> +- ti,sci-rm-range-girq:  Tuple corresponding to unique TISCI resource 
> type that
> + defines the dst host irq ranges assigned to this
> + interrupt router from this host context.
> + Tuple should be of format .
> +
> +Example:
> +
> +The following example demonstrates both interrupt router node and the 
> consumer
> +node(main gpio) on the AM654 SoC:
> +
> +main_intr: interrupt-controller@1 {
> + compatible = "ti,sci-intr";
> + interrupt-controller;
> + interrupt-parent = <>;
> + #interrupt-cells = <3>;
> + ti,sci = <>;
> + ti,sci-dst-id = <56>;
> + ti,sci-rm-range-girq = <0xb 0x1>;
> +};
> +
> +main_gpio0:  main_gpio0@60 {
> + ...
> + interrupt-parent = <_intr>;
> + interrupts = <57 256 IRQ_TYPE_EDGE_RISING>,
> + <57 257 IRQ_TYPE_EDGE_RISING>,
> + <57 258 IRQ_TYPE_EDGE_RISING>,
> + <57 259 IRQ_TYPE_EDGE_RISING>,
> 

Re: [PATCH 1/2] dt-bindings: irqchip: Introduce TISCI Interrupt router bindings

2018-10-06 Thread Marc Zyngier
On Sat, 06 Oct 2018 08:28:11 +0100,
Lokesh Vutla  wrote:
> 
> Add the DT binding documentation for Interrupt router driver.
> 
> Signed-off-by: Lokesh Vutla 
> ---
>  .../interrupt-controller/ti,sci-intr.txt  | 83 +++
>  MAINTAINERS   |  1 +
>  2 files changed, 84 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
> 
> diff --git 
> a/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt 
> b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
> new file mode 100644
> index ..681ca53cc5fb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
> @@ -0,0 +1,83 @@
> +Texas Instruments K3 Interrupt Router
> +=
> +
> +The Interrupt Router (INTR) module provides a mechanism to mux M
> +interrupt inputs to N interrupt outputs, where all M inputs are selectable
> +to be driven per N output. There is one register per output (MUXCNTL_N) that
> +controls the selection.
> +
> +
> + Interrupt Router
> + +--+
> + |  Inputs Outputs  |
> ++---+| +--+ |
> +| GPIO  |--->| | irq0 | |   Host IRQ
> ++---+| +--+ |  controller
> + |.+-+  |  +---+
> ++---+|.|  0  |  |->|  GIC  |
> +| INTA  |--->|.+-+  |  +---+
> ++---+|.  .  |
> + | +--+  .  |
> + | | irqM |+-+  |
> + | +--+|  N  |  |
> + | +-+  |
> + +--+
> +
> +Configuration of these MUXCNTL_N registers is done by a system controller
> +(like the Device Memory and Security Controller on K3 AM654 SoC). System
> +controller will keep track of the used and unused registers within the 
> Router.
> +Driver should request the system controller to get the range of GIC IRQs
> +assigned to the requesting hosts. It is the drivers responsibility to keep
> +track of GIC IRQs.

I would drop the GIC here, and replace it by "parent interrupt
controller", as nothing here is GIC specific.

> +
> +Communication between the host processor running an OS and the system
> +controller happens through a protocol called TI System Control Interface
> +(TISCI protocol). For more details refer:
> +Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
> +
> +TISCI Interrupt Router Node:
> +
> +- compatible:Must be "ti,sci-intr".
> +- interrupt-controller:  Identifies the node as an interrupt controller
> +- #interrupt-cells:  Specifies the number of cells needed to encode an
> + interrupt source. The value should be 3.
> + First cell should contain the TISCI device ID of source
> + Second cell should contain the interrupt source offset
> + within the device
> + Third cell specifies the trigger type as defined
> + in interrupts.txt in this directory.

Are all trigger types supported?

> +- interrupt-parent:  phandle of irq parent for TISCI intr. The parent must
> + use the same interrupt-cells format as GIC.

Why that constraint? From what I can see, the two are fairly
independent, and the constraint looks more of a Linux driver issue
than a DT constraint.

> +- ti,sci:Phandle to TI-SCI compatible System controller node.
> +- ti,sci-dst-id: TISCI device ID of the destination IRQ controller.
> +- ti,sci-rm-range-girq:  Tuple corresponding to unique TISCI resource 
> type that
> + defines the dst host irq ranges assigned to this
> + interrupt router from this host context.
> + Tuple should be of format .
> +
> +Example:
> +
> +The following example demonstrates both interrupt router node and the 
> consumer
> +node(main gpio) on the AM654 SoC:
> +
> +main_intr: interrupt-controller@1 {
> + compatible = "ti,sci-intr";
> + interrupt-controller;
> + interrupt-parent = <>;
> + #interrupt-cells = <3>;
> + ti,sci = <>;
> + ti,sci-dst-id = <56>;
> + ti,sci-rm-range-girq = <0xb 0x1>;
> +};
> +
> +main_gpio0:  main_gpio0@60 {
> + ...
> + interrupt-parent = <_intr>;
> + interrupts = <57 256 IRQ_TYPE_EDGE_RISING>,
> + <57 257 IRQ_TYPE_EDGE_RISING>,
> + <57 258 IRQ_TYPE_EDGE_RISING>,
> + <57 259 IRQ_TYPE_EDGE_RISING>,
> 

Re: [PATCH 2/2] irqchip: ti-sci-intr: Add support for Interrupt Router driver

2018-10-06 Thread Marc Zyngier
Hi Lokesh,

On Sat, 06 Oct 2018 08:28:12 +0100,
Lokesh Vutla  wrote:
> 
> Texas Instruments' K3 generation SoCs has an IP Interrupt Router
> that does allows for multiplexing of input interrupts to host
> interrupt controller. Interrupt Router inputs are either from a
> peripheral or from an Interrupt Aggregator which is another
> interrupt controller.
> 
> Configuration of the interrupt router registers can only be done by
> a system co-processor and the driver needs to send a message to this
> co processor over TISCI protocol.

I assume that this co-processor only deals with the routing itself,
and doesn't need to be talked to during interrupt processing, right?

> 
> Add support for Interrupt Router driver over TISCI protocol.
> 
> Signed-off-by: Lokesh Vutla 
> ---
>  MAINTAINERS   |   1 +
>  drivers/irqchip/Kconfig   |  11 +
>  drivers/irqchip/Makefile  |   1 +
>  drivers/irqchip/irq-ti-sci-intr.c | 325 ++
>  4 files changed, 338 insertions(+)
>  create mode 100644 drivers/irqchip/irq-ti-sci-intr.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a23778b68d74..cf3c834f8cee 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -14626,6 +14626,7 @@ F:
> Documentation/devicetree/bindings/clock/ti,sci-clk.txt
>  F:   drivers/clk/keystone/sci-clk.c
>  F:   drivers/reset/reset-ti-sci.c
>  F:   Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
> +F:   drivers/irqchip/irq-ti-sci-intr.c
>  
>  THANKO'S RAREMONO AM/FM/SW RADIO RECEIVER USB DRIVER
>  M:   Hans Verkuil 
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index 96451b581452..9a965fe22043 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -374,6 +374,17 @@ config QCOM_PDC
> Power Domain Controller driver to manage and configure wakeup
> IRQs for Qualcomm Technologies Inc (QTI) mobile chips.
>  
> +config TI_SCI_INTR_IRQCHIP
> + tristate "TISCI based Interrupt Router irqchip driver"
> + depends on TI_SCI_PROTOCOL && ARCH_K3
> + select IRQ_DOMAIN
> + select IRQ_DOMAIN_HIERARCHY
> + help
> +   This enables the irqchip driver support for K3 Interrupt router
> +   over TI System Control Interface available on some new TI's SoCs.
> +   If you wish to use interrupt router irq resources managed by the
> +   TI System Controller, say Y here. Otherwise, say N.

I don't really see the point of making this user-selectable. If you're
compiling support for a given platform, this platform configuration
fragment should itself select the necessary dependencies for the
system to work as expected. Here, you are leaving the choice to the
user, with a 50% chance of getting a system that doesn't boot...

> +
>  endmenu
>  
>  config SIFIVE_PLIC
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index b822199445ff..44bf65606d60 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -89,3 +89,4 @@ obj-$(CONFIG_GOLDFISH_PIC)  += irq-goldfish-pic.o
>  obj-$(CONFIG_NDS32)  += irq-ativic32.o
>  obj-$(CONFIG_QCOM_PDC)   += qcom-pdc.o
>  obj-$(CONFIG_SIFIVE_PLIC)+= irq-sifive-plic.o
> +obj-$(CONFIG_TI_SCI_INTR_IRQCHIP)+= irq-ti-sci-intr.o
> diff --git a/drivers/irqchip/irq-ti-sci-intr.c 
> b/drivers/irqchip/irq-ti-sci-intr.c
> new file mode 100644
> index ..f04fe6da1b09
> --- /dev/null
> +++ b/drivers/irqchip/irq-ti-sci-intr.c
> @@ -0,0 +1,325 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Texas Instruments' K3 Interrupt Router irqchip driver
> + *
> + * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/
> + *   Lokesh Vutla 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define TI_SCI_DEV_ID_MASK   0x
> +#define TI_SCI_DEV_ID_SHIFT  16
> +#define TI_SCI_IRQ_ID_MASK   0x
> +#define TI_SCI_IRQ_ID_SHIFT  0
> +#define TI_SCI_IS_EVENT_IRQ  BIT(31)
> +
> +#define HWIRQ_TO_DEVID(HWIRQ)(((HWIRQ) >> (TI_SCI_DEV_ID_SHIFT)) & \
> +  (TI_SCI_DEV_ID_MASK))
> +#define HWIRQ_TO_IRQID(HWIRQ)((HWIRQ) & (TI_SCI_IRQ_ID_MASK))

nit: s/(HWIRQ)/(hwirq)/g

> +
> +/**
> + * struct ti_sci_intr_irq_domain - Structure representing a TISCI based
> + *  Interrupt Router IRQ domain.
> + * @sci: Pointer to TISCI handle
> + * @dst_irq: TISCI resource pointer representing destination irq controller.
> + * @dst_id:  TISCI device ID of the destination irq controller.
> + */
> +struct ti_sci_intr_irq_domain {
> + const struct ti_sci_handle *sci;
> + struct ti_sci_resource *dst_irq;
> + u16 dst_id;
> +};
> +
> +/**
> + * struct ti_sci_intr_irq_desc - Description of an Interrupt Router IRQ
> + * @src_id:  TISCI device ID of the IRQ source
> + * @src_index:   IRQ source index within 

Re: [PATCH 2/2] irqchip: ti-sci-intr: Add support for Interrupt Router driver

2018-10-06 Thread Marc Zyngier
Hi Lokesh,

On Sat, 06 Oct 2018 08:28:12 +0100,
Lokesh Vutla  wrote:
> 
> Texas Instruments' K3 generation SoCs has an IP Interrupt Router
> that does allows for multiplexing of input interrupts to host
> interrupt controller. Interrupt Router inputs are either from a
> peripheral or from an Interrupt Aggregator which is another
> interrupt controller.
> 
> Configuration of the interrupt router registers can only be done by
> a system co-processor and the driver needs to send a message to this
> co processor over TISCI protocol.

I assume that this co-processor only deals with the routing itself,
and doesn't need to be talked to during interrupt processing, right?

> 
> Add support for Interrupt Router driver over TISCI protocol.
> 
> Signed-off-by: Lokesh Vutla 
> ---
>  MAINTAINERS   |   1 +
>  drivers/irqchip/Kconfig   |  11 +
>  drivers/irqchip/Makefile  |   1 +
>  drivers/irqchip/irq-ti-sci-intr.c | 325 ++
>  4 files changed, 338 insertions(+)
>  create mode 100644 drivers/irqchip/irq-ti-sci-intr.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a23778b68d74..cf3c834f8cee 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -14626,6 +14626,7 @@ F:
> Documentation/devicetree/bindings/clock/ti,sci-clk.txt
>  F:   drivers/clk/keystone/sci-clk.c
>  F:   drivers/reset/reset-ti-sci.c
>  F:   Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
> +F:   drivers/irqchip/irq-ti-sci-intr.c
>  
>  THANKO'S RAREMONO AM/FM/SW RADIO RECEIVER USB DRIVER
>  M:   Hans Verkuil 
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index 96451b581452..9a965fe22043 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -374,6 +374,17 @@ config QCOM_PDC
> Power Domain Controller driver to manage and configure wakeup
> IRQs for Qualcomm Technologies Inc (QTI) mobile chips.
>  
> +config TI_SCI_INTR_IRQCHIP
> + tristate "TISCI based Interrupt Router irqchip driver"
> + depends on TI_SCI_PROTOCOL && ARCH_K3
> + select IRQ_DOMAIN
> + select IRQ_DOMAIN_HIERARCHY
> + help
> +   This enables the irqchip driver support for K3 Interrupt router
> +   over TI System Control Interface available on some new TI's SoCs.
> +   If you wish to use interrupt router irq resources managed by the
> +   TI System Controller, say Y here. Otherwise, say N.

I don't really see the point of making this user-selectable. If you're
compiling support for a given platform, this platform configuration
fragment should itself select the necessary dependencies for the
system to work as expected. Here, you are leaving the choice to the
user, with a 50% chance of getting a system that doesn't boot...

> +
>  endmenu
>  
>  config SIFIVE_PLIC
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index b822199445ff..44bf65606d60 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -89,3 +89,4 @@ obj-$(CONFIG_GOLDFISH_PIC)  += irq-goldfish-pic.o
>  obj-$(CONFIG_NDS32)  += irq-ativic32.o
>  obj-$(CONFIG_QCOM_PDC)   += qcom-pdc.o
>  obj-$(CONFIG_SIFIVE_PLIC)+= irq-sifive-plic.o
> +obj-$(CONFIG_TI_SCI_INTR_IRQCHIP)+= irq-ti-sci-intr.o
> diff --git a/drivers/irqchip/irq-ti-sci-intr.c 
> b/drivers/irqchip/irq-ti-sci-intr.c
> new file mode 100644
> index ..f04fe6da1b09
> --- /dev/null
> +++ b/drivers/irqchip/irq-ti-sci-intr.c
> @@ -0,0 +1,325 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Texas Instruments' K3 Interrupt Router irqchip driver
> + *
> + * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/
> + *   Lokesh Vutla 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define TI_SCI_DEV_ID_MASK   0x
> +#define TI_SCI_DEV_ID_SHIFT  16
> +#define TI_SCI_IRQ_ID_MASK   0x
> +#define TI_SCI_IRQ_ID_SHIFT  0
> +#define TI_SCI_IS_EVENT_IRQ  BIT(31)
> +
> +#define HWIRQ_TO_DEVID(HWIRQ)(((HWIRQ) >> (TI_SCI_DEV_ID_SHIFT)) & \
> +  (TI_SCI_DEV_ID_MASK))
> +#define HWIRQ_TO_IRQID(HWIRQ)((HWIRQ) & (TI_SCI_IRQ_ID_MASK))

nit: s/(HWIRQ)/(hwirq)/g

> +
> +/**
> + * struct ti_sci_intr_irq_domain - Structure representing a TISCI based
> + *  Interrupt Router IRQ domain.
> + * @sci: Pointer to TISCI handle
> + * @dst_irq: TISCI resource pointer representing destination irq controller.
> + * @dst_id:  TISCI device ID of the destination irq controller.
> + */
> +struct ti_sci_intr_irq_domain {
> + const struct ti_sci_handle *sci;
> + struct ti_sci_resource *dst_irq;
> + u16 dst_id;
> +};
> +
> +/**
> + * struct ti_sci_intr_irq_desc - Description of an Interrupt Router IRQ
> + * @src_id:  TISCI device ID of the IRQ source
> + * @src_index:   IRQ source index within 

[PATCH] jffs2: free jffs2_sb_info through jffs2_kill_sb()

2018-10-06 Thread Hou Tao
When an invalid mount option is passed to jffs2, jffs2_parse_options()
will fail and jffs2_sb_info will be freed, but then jffs2_sb_info will
be used (use-after-free) and freeed (double-free) in jffs2_kill_sb().

Fix it by removing the buggy invocation of kfree() when getting invalid
mount options.

Cc: sta...@kernel.org
Signed-off-by: Hou Tao 
---
 fs/jffs2/super.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/fs/jffs2/super.c b/fs/jffs2/super.c
index 87bdf0f4cba1..902a7dd10e5c 100644
--- a/fs/jffs2/super.c
+++ b/fs/jffs2/super.c
@@ -285,10 +285,8 @@ static int jffs2_fill_super(struct super_block *sb, void 
*data, int silent)
sb->s_fs_info = c;
 
ret = jffs2_parse_options(c, data);
-   if (ret) {
-   kfree(c);
+   if (ret)
return -EINVAL;
-   }
 
/* Initialize JFFS2 superblock locks, the further initialization will
 * be done later */
-- 
2.16.2.dirty



[PATCH] jffs2: free jffs2_sb_info through jffs2_kill_sb()

2018-10-06 Thread Hou Tao
When an invalid mount option is passed to jffs2, jffs2_parse_options()
will fail and jffs2_sb_info will be freed, but then jffs2_sb_info will
be used (use-after-free) and freeed (double-free) in jffs2_kill_sb().

Fix it by removing the buggy invocation of kfree() when getting invalid
mount options.

Cc: sta...@kernel.org
Signed-off-by: Hou Tao 
---
 fs/jffs2/super.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/fs/jffs2/super.c b/fs/jffs2/super.c
index 87bdf0f4cba1..902a7dd10e5c 100644
--- a/fs/jffs2/super.c
+++ b/fs/jffs2/super.c
@@ -285,10 +285,8 @@ static int jffs2_fill_super(struct super_block *sb, void 
*data, int silent)
sb->s_fs_info = c;
 
ret = jffs2_parse_options(c, data);
-   if (ret) {
-   kfree(c);
+   if (ret)
return -EINVAL;
-   }
 
/* Initialize JFFS2 superblock locks, the further initialization will
 * be done later */
-- 
2.16.2.dirty



setup_local_APIC issue with Xenon E5-1650V3

2018-10-06 Thread Toralf Förster
I'm trying to get a new system and running with kernel 4.18.12, but run
into an APIC error as seen in [1].

It is a new system, never tried older kernels till now.

The kernel command line "noapic" doesn't help, now I do wonder what else
I can do.

The same kernel config was fine for 2 years with an i7-3930, I just had
to change the network card


Thx or any help

[1]  https://zwiebeltoralf.de/pub/Screenshot_20181006_101121.png

-- 
Toralf
PGP C4EACDDE 0076E94E


Re: [PATCH v4.19-rc7] treewide: Replace more open-coded allocation size multiplications

2018-10-06 Thread Fengguang Wu

On Fri, Oct 05, 2018 at 08:14:34PM -0700, Joel Fernandes wrote:

On Fri, Oct 05, 2018 at 05:22:35PM -0700, Greg KH wrote:

On Fri, Oct 05, 2018 at 05:04:16PM -0700, Kees Cook wrote:
> On Fri, Oct 5, 2018 at 4:51 PM, Greg KH  wrote:
> > On Fri, Oct 05, 2018 at 04:35:59PM -0700, Kees Cook wrote:
> >> As done treewide earlier, this catches several more open-coded
> >> allocation size calculations that were added to the kernel during the
> >> merge window. This performs the following mechanical transformations
> >> using Coccinelle:
> >>
> >>   kvmalloc(a * b, ...) -> kvmalloc_array(a, b, ...)
> >>   kvzalloc(a * b, ...) -> kvcalloc(a, b, ...)
> >>   devm_kzalloc(..., a * b, ...) -> devm_kcalloc(..., a, b, ...)
> >>
> >> Signed-off-by: Kees Cook 
> >
> > Has this had any testing in linux-next?
>
> No; they're mechanical transformations (though I did build test them).
> If you want I could add this to linux-next for a week?

That would be good, thanks.

> > And when was "earlier"?
>
> v4.18, when all of these were originally eliminated:
>
> 026f05079b00 treewide: Use array_size() in f2fs_kzalloc()
> c86065938aab treewide: Use array_size() in f2fs_kmalloc()
> 76e43e37a407 treewide: Use array_size() in sock_kmalloc()
> 84ca176bf54a treewide: Use array_size() in kvzalloc_node()
> fd7becedb1f0 treewide: Use array_size() in vzalloc_node()
> fad953ce0b22 treewide: Use array_size() in vzalloc()
> 42bc47b35320 treewide: Use array_size() in vmalloc()
> a86854d0c599 treewide: devm_kzalloc() -> devm_kcalloc()
> 3c4211ba8ad8 treewide: devm_kmalloc() -> devm_kmalloc_array()
> 778e1cdd81bb treewide: kvzalloc() -> kvcalloc()
> 344476e16acb treewide: kvmalloc() -> kvmalloc_array()
> 590b5b7d8671 treewide: kzalloc_node() -> kcalloc_node()
> 6396bb221514 treewide: kzalloc() -> kcalloc()
> 6da2ec56059c treewide: kmalloc() -> kmalloc_array()
>
> The new patch is catching new open-coded multiplications introduced in v4.19.

As this is getting smaller, why not just break it up and do it through
all of the different subsystems instead of one large patch?

And do we have a way to add a rule to 0-day to catch these so that they
get a warning when they are added again?


They could just be added to scripts/coccinelle and 0-day will report them?

For example, 0-day ran scripts/coccinelle/api/platform_no_drv_owner.cocci on
a recently submitted patch and reported it here:
https://lore.kernel.org/lkml/201808301856.vmnjerss%25fengguang...@intel.com/

But I'm not sure if 0-day runs make coccicheck on specific semantic patches,
or runs all of them (CC'd Fengguang).


0-day runs all coccinelle scripts. However only auto report out
warnings that are known to have low false positives.

So if you add new coccinelle scripts that emit accurate enough
warnings, it'd be good to inform the LKP team to add the new
warnings to our auto-report-out white list.

Thanks,
Fengguang


setup_local_APIC issue with Xenon E5-1650V3

2018-10-06 Thread Toralf Förster
I'm trying to get a new system and running with kernel 4.18.12, but run
into an APIC error as seen in [1].

It is a new system, never tried older kernels till now.

The kernel command line "noapic" doesn't help, now I do wonder what else
I can do.

The same kernel config was fine for 2 years with an i7-3930, I just had
to change the network card


Thx or any help

[1]  https://zwiebeltoralf.de/pub/Screenshot_20181006_101121.png

-- 
Toralf
PGP C4EACDDE 0076E94E


Re: [PATCH v4.19-rc7] treewide: Replace more open-coded allocation size multiplications

2018-10-06 Thread Fengguang Wu

On Fri, Oct 05, 2018 at 08:14:34PM -0700, Joel Fernandes wrote:

On Fri, Oct 05, 2018 at 05:22:35PM -0700, Greg KH wrote:

On Fri, Oct 05, 2018 at 05:04:16PM -0700, Kees Cook wrote:
> On Fri, Oct 5, 2018 at 4:51 PM, Greg KH  wrote:
> > On Fri, Oct 05, 2018 at 04:35:59PM -0700, Kees Cook wrote:
> >> As done treewide earlier, this catches several more open-coded
> >> allocation size calculations that were added to the kernel during the
> >> merge window. This performs the following mechanical transformations
> >> using Coccinelle:
> >>
> >>   kvmalloc(a * b, ...) -> kvmalloc_array(a, b, ...)
> >>   kvzalloc(a * b, ...) -> kvcalloc(a, b, ...)
> >>   devm_kzalloc(..., a * b, ...) -> devm_kcalloc(..., a, b, ...)
> >>
> >> Signed-off-by: Kees Cook 
> >
> > Has this had any testing in linux-next?
>
> No; they're mechanical transformations (though I did build test them).
> If you want I could add this to linux-next for a week?

That would be good, thanks.

> > And when was "earlier"?
>
> v4.18, when all of these were originally eliminated:
>
> 026f05079b00 treewide: Use array_size() in f2fs_kzalloc()
> c86065938aab treewide: Use array_size() in f2fs_kmalloc()
> 76e43e37a407 treewide: Use array_size() in sock_kmalloc()
> 84ca176bf54a treewide: Use array_size() in kvzalloc_node()
> fd7becedb1f0 treewide: Use array_size() in vzalloc_node()
> fad953ce0b22 treewide: Use array_size() in vzalloc()
> 42bc47b35320 treewide: Use array_size() in vmalloc()
> a86854d0c599 treewide: devm_kzalloc() -> devm_kcalloc()
> 3c4211ba8ad8 treewide: devm_kmalloc() -> devm_kmalloc_array()
> 778e1cdd81bb treewide: kvzalloc() -> kvcalloc()
> 344476e16acb treewide: kvmalloc() -> kvmalloc_array()
> 590b5b7d8671 treewide: kzalloc_node() -> kcalloc_node()
> 6396bb221514 treewide: kzalloc() -> kcalloc()
> 6da2ec56059c treewide: kmalloc() -> kmalloc_array()
>
> The new patch is catching new open-coded multiplications introduced in v4.19.

As this is getting smaller, why not just break it up and do it through
all of the different subsystems instead of one large patch?

And do we have a way to add a rule to 0-day to catch these so that they
get a warning when they are added again?


They could just be added to scripts/coccinelle and 0-day will report them?

For example, 0-day ran scripts/coccinelle/api/platform_no_drv_owner.cocci on
a recently submitted patch and reported it here:
https://lore.kernel.org/lkml/201808301856.vmnjerss%25fengguang...@intel.com/

But I'm not sure if 0-day runs make coccicheck on specific semantic patches,
or runs all of them (CC'd Fengguang).


0-day runs all coccinelle scripts. However only auto report out
warnings that are known to have low false positives.

So if you add new coccinelle scripts that emit accurate enough
warnings, it'd be good to inform the LKP team to add the new
warnings to our auto-report-out white list.

Thanks,
Fengguang


Re: [PATCH 1/2] Add FAT_IOCTL_GET_VOLUME_LABEL in fat_generic_ioctl()

2018-10-06 Thread Pali Rohár
On Saturday 06 October 2018 16:33:10 chen.chenchacha wrote:
> On Thu, 2018-10-04 at 19:33 +0200, Pali Rohár wrote:
> > On Friday 05 October 2018 01:21:00 chenchacha wrote:
> > > Signed-off-by: chenchacha 
> > > ---
> > >  fs/fat/file.c | 22 ++
> > >  include/uapi/linux/msdos_fs.h |  1 +
> > >  2 files changed, 23 insertions(+)
> > > 
> > > diff --git a/fs/fat/file.c b/fs/fat/file.c
> > > index 4724cc9ad650..56db0b5a8df1 100644
> > > --- a/fs/fat/file.c
> > > +++ b/fs/fat/file.c
> > > @@ -121,10 +121,30 @@ static int fat_ioctl_get_volume_id(struct
> > > inode *inode, u32 __user *user_attr)
> > >   return put_user(sbi->vol_id, user_attr);
> > >  }
> > >  
> > > +static int fat_ioctl_get_volume_label(struct inode *inode, u8
> > > __user *label)
> > > +{
> > > + struct super_block *sb = inode->i_sb;
> > > + struct inode *root_inode = d_inode(sb->s_root);
> > > + struct buffer_head *bh = NULL;
> > > + struct msdos_dir_entry *de;
> > > + int err;
> > > +
> > > + inode_lock_shared(root_inode);
> > > + err = fat_get_root_entry(root_inode, , );
> > > + if (err == 0) {
> > > + if (copy_to_user(label, de->name, MSDOS_NAME))
> > 
> > You need to convert entry name from 8.3 format to label in correct
> > encoding specified by codepage mount option. Plus needs to handle
> > leading 0x03 byte.
> > 
> I think it might be better to put the decoder/encoder on the
> application layter.

I do not think so. On all other places in msdos.ko and vfat.ko driver
which communicate with userspace is that decoder/encoder active and user
does not see raw bytes.

It is really bad to mix encodings and API of different calls.

If in mount option I specified that I want to use XYZ encoding, why
driver does not going to respect it?

> Similarly, the handle leading 0x03 byte is in the
> write function. And make read operation pure, just to carry the volume
> label in root directory to the user.
> 
> What do you think, Pali?
> 

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: PGP signature


Re: [PATCH 1/2] Add FAT_IOCTL_GET_VOLUME_LABEL in fat_generic_ioctl()

2018-10-06 Thread Pali Rohár
On Saturday 06 October 2018 16:33:10 chen.chenchacha wrote:
> On Thu, 2018-10-04 at 19:33 +0200, Pali Rohár wrote:
> > On Friday 05 October 2018 01:21:00 chenchacha wrote:
> > > Signed-off-by: chenchacha 
> > > ---
> > >  fs/fat/file.c | 22 ++
> > >  include/uapi/linux/msdos_fs.h |  1 +
> > >  2 files changed, 23 insertions(+)
> > > 
> > > diff --git a/fs/fat/file.c b/fs/fat/file.c
> > > index 4724cc9ad650..56db0b5a8df1 100644
> > > --- a/fs/fat/file.c
> > > +++ b/fs/fat/file.c
> > > @@ -121,10 +121,30 @@ static int fat_ioctl_get_volume_id(struct
> > > inode *inode, u32 __user *user_attr)
> > >   return put_user(sbi->vol_id, user_attr);
> > >  }
> > >  
> > > +static int fat_ioctl_get_volume_label(struct inode *inode, u8
> > > __user *label)
> > > +{
> > > + struct super_block *sb = inode->i_sb;
> > > + struct inode *root_inode = d_inode(sb->s_root);
> > > + struct buffer_head *bh = NULL;
> > > + struct msdos_dir_entry *de;
> > > + int err;
> > > +
> > > + inode_lock_shared(root_inode);
> > > + err = fat_get_root_entry(root_inode, , );
> > > + if (err == 0) {
> > > + if (copy_to_user(label, de->name, MSDOS_NAME))
> > 
> > You need to convert entry name from 8.3 format to label in correct
> > encoding specified by codepage mount option. Plus needs to handle
> > leading 0x03 byte.
> > 
> I think it might be better to put the decoder/encoder on the
> application layter.

I do not think so. On all other places in msdos.ko and vfat.ko driver
which communicate with userspace is that decoder/encoder active and user
does not see raw bytes.

It is really bad to mix encodings and API of different calls.

If in mount option I specified that I want to use XYZ encoding, why
driver does not going to respect it?

> Similarly, the handle leading 0x03 byte is in the
> write function. And make read operation pure, just to carry the volume
> label in root directory to the user.
> 
> What do you think, Pali?
> 

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: PGP signature


Re: [PATCH] hugetlb: fix ARM 3level page tables

2018-10-06 Thread Naresh Kamboju
Arnd,

On Fri, 5 Oct 2018 at 09:17, Arnd Bergmann  wrote:
>
> The check for __HAVE_ARCH_HUGE_PTEP_GET comes before the definition,
> leading to an extraneous definition of huge_ptep_get:
>
> In file included from arch/arm/include/asm/hugetlb.h:28,
>  from include/linux/hugetlb.h:456,
>  from arch/arm/kvm/../../../virt/kvm/arm/mmu.c:22:
> arch/arm/include/asm/hugetlb-3level.h:33:21: error: redefinition of 
> 'huge_ptep_get'
>  static inline pte_t huge_ptep_get(pte_t *ptep)
>
> Change the header file inclusions to be in the correct order for
> this to work.
>
> Fixes: bb1d18ffc7ae ("hugetlb: introduce generic version of huge_ptep_get")
> Signed-off-by: Arnd Bergmann 

I have noticed this build error on linux next tree for arm32 build.
Thanks for the fix.

- Naresh


Re: [PATCH] hugetlb: fix ARM 3level page tables

2018-10-06 Thread Naresh Kamboju
Arnd,

On Fri, 5 Oct 2018 at 09:17, Arnd Bergmann  wrote:
>
> The check for __HAVE_ARCH_HUGE_PTEP_GET comes before the definition,
> leading to an extraneous definition of huge_ptep_get:
>
> In file included from arch/arm/include/asm/hugetlb.h:28,
>  from include/linux/hugetlb.h:456,
>  from arch/arm/kvm/../../../virt/kvm/arm/mmu.c:22:
> arch/arm/include/asm/hugetlb-3level.h:33:21: error: redefinition of 
> 'huge_ptep_get'
>  static inline pte_t huge_ptep_get(pte_t *ptep)
>
> Change the header file inclusions to be in the correct order for
> this to work.
>
> Fixes: bb1d18ffc7ae ("hugetlb: introduce generic version of huge_ptep_get")
> Signed-off-by: Arnd Bergmann 

I have noticed this build error on linux next tree for arm32 build.
Thanks for the fix.

- Naresh


[PATCH 2/2] irqchip: ti-sci-intr: Add support for Interrupt Router driver

2018-10-06 Thread Lokesh Vutla
Texas Instruments' K3 generation SoCs has an IP Interrupt Router
that does allows for multiplexing of input interrupts to host
interrupt controller. Interrupt Router inputs are either from a
peripheral or from an Interrupt Aggregator which is another
interrupt controller.

Configuration of the interrupt router registers can only be done by
a system co-processor and the driver needs to send a message to this
co processor over TISCI protocol.

Add support for Interrupt Router driver over TISCI protocol.

Signed-off-by: Lokesh Vutla 
---
 MAINTAINERS   |   1 +
 drivers/irqchip/Kconfig   |  11 +
 drivers/irqchip/Makefile  |   1 +
 drivers/irqchip/irq-ti-sci-intr.c | 325 ++
 4 files changed, 338 insertions(+)
 create mode 100644 drivers/irqchip/irq-ti-sci-intr.c

diff --git a/MAINTAINERS b/MAINTAINERS
index a23778b68d74..cf3c834f8cee 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14626,6 +14626,7 @@ F:  
Documentation/devicetree/bindings/clock/ti,sci-clk.txt
 F: drivers/clk/keystone/sci-clk.c
 F: drivers/reset/reset-ti-sci.c
 F: Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
+F: drivers/irqchip/irq-ti-sci-intr.c
 
 THANKO'S RAREMONO AM/FM/SW RADIO RECEIVER USB DRIVER
 M: Hans Verkuil 
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 96451b581452..9a965fe22043 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -374,6 +374,17 @@ config QCOM_PDC
  Power Domain Controller driver to manage and configure wakeup
  IRQs for Qualcomm Technologies Inc (QTI) mobile chips.
 
+config TI_SCI_INTR_IRQCHIP
+   tristate "TISCI based Interrupt Router irqchip driver"
+   depends on TI_SCI_PROTOCOL && ARCH_K3
+   select IRQ_DOMAIN
+   select IRQ_DOMAIN_HIERARCHY
+   help
+ This enables the irqchip driver support for K3 Interrupt router
+ over TI System Control Interface available on some new TI's SoCs.
+ If you wish to use interrupt router irq resources managed by the
+ TI System Controller, say Y here. Otherwise, say N.
+
 endmenu
 
 config SIFIVE_PLIC
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index b822199445ff..44bf65606d60 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -89,3 +89,4 @@ obj-$(CONFIG_GOLDFISH_PIC)+= irq-goldfish-pic.o
 obj-$(CONFIG_NDS32)+= irq-ativic32.o
 obj-$(CONFIG_QCOM_PDC) += qcom-pdc.o
 obj-$(CONFIG_SIFIVE_PLIC)  += irq-sifive-plic.o
+obj-$(CONFIG_TI_SCI_INTR_IRQCHIP)  += irq-ti-sci-intr.o
diff --git a/drivers/irqchip/irq-ti-sci-intr.c 
b/drivers/irqchip/irq-ti-sci-intr.c
new file mode 100644
index ..f04fe6da1b09
--- /dev/null
+++ b/drivers/irqchip/irq-ti-sci-intr.c
@@ -0,0 +1,325 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Texas Instruments' K3 Interrupt Router irqchip driver
+ *
+ * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/
+ * Lokesh Vutla 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define TI_SCI_DEV_ID_MASK 0x
+#define TI_SCI_DEV_ID_SHIFT16
+#define TI_SCI_IRQ_ID_MASK 0x
+#define TI_SCI_IRQ_ID_SHIFT0
+#define TI_SCI_IS_EVENT_IRQBIT(31)
+
+#define HWIRQ_TO_DEVID(HWIRQ)  (((HWIRQ) >> (TI_SCI_DEV_ID_SHIFT)) & \
+(TI_SCI_DEV_ID_MASK))
+#define HWIRQ_TO_IRQID(HWIRQ)  ((HWIRQ) & (TI_SCI_IRQ_ID_MASK))
+
+/**
+ * struct ti_sci_intr_irq_domain - Structure representing a TISCI based
+ *Interrupt Router IRQ domain.
+ * @sci:   Pointer to TISCI handle
+ * @dst_irq:   TISCI resource pointer representing destination irq controller.
+ * @dst_id:TISCI device ID of the destination irq controller.
+ */
+struct ti_sci_intr_irq_domain {
+   const struct ti_sci_handle *sci;
+   struct ti_sci_resource *dst_irq;
+   u16 dst_id;
+};
+
+/**
+ * struct ti_sci_intr_irq_desc - Description of an Interrupt Router IRQ
+ * @src_id:TISCI device ID of the IRQ source
+ * @src_index: IRQ source index within the device.
+ * @dst_irq:   Destination host IRQ.
+ */
+struct ti_sci_intr_irq_desc {
+   u16 src_id;
+   u16 src_index;
+   u16 dst_irq;
+};
+
+static struct irq_chip ti_sci_intr_irq_chip = {
+   .name   = "INTR",
+   .irq_eoi= irq_chip_eoi_parent,
+   .irq_mask   = irq_chip_mask_parent,
+   .irq_unmask = irq_chip_unmask_parent,
+   .irq_retrigger  = irq_chip_retrigger_hierarchy,
+   .irq_set_type   = irq_chip_set_type_parent,
+   .irq_set_affinity   = irq_chip_set_affinity_parent,
+};
+
+/**
+ * ti_sci_intr_irq_domain_translate() - Retrieve hwirq and type from
+ * IRQ firmware specific handler.
+ 

[PATCH 0/2] irqchip: ti-sci-intr: Add support for K3 Interrupt Router

2018-10-06 Thread Lokesh Vutla
This series adds irqchip driver for Texas Instruments' K3 based
Interrupt Router.

This series depends on TISCI IRQ management support posted here[1]

[1] 
http://lists.infradead.org/pipermail/linux-arm-kernel/2018-October/605784.html

Lokesh Vutla (2):
  dt-bindings: irqchip: Introduce TISCI Interrupt router bindings
  irqchip: ti-sci-intr: Add support for Interrupt Router driver

 .../interrupt-controller/ti,sci-intr.txt  |  83 +
 MAINTAINERS   |   2 +
 drivers/irqchip/Kconfig   |  11 +
 drivers/irqchip/Makefile  |   1 +
 drivers/irqchip/irq-ti-sci-intr.c | 325 ++
 5 files changed, 422 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
 create mode 100644 drivers/irqchip/irq-ti-sci-intr.c

-- 
2.19.0



[PATCH 1/2] dt-bindings: irqchip: Introduce TISCI Interrupt router bindings

2018-10-06 Thread Lokesh Vutla
Add the DT binding documentation for Interrupt router driver.

Signed-off-by: Lokesh Vutla 
---
 .../interrupt-controller/ti,sci-intr.txt  | 83 +++
 MAINTAINERS   |  1 +
 2 files changed, 84 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt

diff --git 
a/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt 
b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
new file mode 100644
index ..681ca53cc5fb
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
@@ -0,0 +1,83 @@
+Texas Instruments K3 Interrupt Router
+=
+
+The Interrupt Router (INTR) module provides a mechanism to mux M
+interrupt inputs to N interrupt outputs, where all M inputs are selectable
+to be driven per N output. There is one register per output (MUXCNTL_N) that
+controls the selection.
+
+
+ Interrupt Router
+ +--+
+ |  Inputs Outputs  |
++---+| +--+ |
+| GPIO  |--->| | irq0 | |   Host IRQ
++---+| +--+ |  controller
+ |.+-+  |  +---+
++---+|.|  0  |  |->|  GIC  |
+| INTA  |--->|.+-+  |  +---+
++---+|.  .  |
+ | +--+  .  |
+ | | irqM |+-+  |
+ | +--+|  N  |  |
+ | +-+  |
+ +--+
+
+Configuration of these MUXCNTL_N registers is done by a system controller
+(like the Device Memory and Security Controller on K3 AM654 SoC). System
+controller will keep track of the used and unused registers within the Router.
+Driver should request the system controller to get the range of GIC IRQs
+assigned to the requesting hosts. It is the drivers responsibility to keep
+track of GIC IRQs.
+
+Communication between the host processor running an OS and the system
+controller happens through a protocol called TI System Control Interface
+(TISCI protocol). For more details refer:
+Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
+
+TISCI Interrupt Router Node:
+
+- compatible:  Must be "ti,sci-intr".
+- interrupt-controller:Identifies the node as an interrupt controller
+- #interrupt-cells:Specifies the number of cells needed to encode an
+   interrupt source. The value should be 3.
+   First cell should contain the TISCI device ID of source
+   Second cell should contain the interrupt source offset
+   within the device
+   Third cell specifies the trigger type as defined
+   in interrupts.txt in this directory.
+- interrupt-parent:phandle of irq parent for TISCI intr. The parent must
+   use the same interrupt-cells format as GIC.
+- ti,sci:  Phandle to TI-SCI compatible System controller node.
+- ti,sci-dst-id:   TISCI device ID of the destination IRQ controller.
+- ti,sci-rm-range-girq:Tuple corresponding to unique TISCI resource 
type that
+   defines the dst host irq ranges assigned to this
+   interrupt router from this host context.
+   Tuple should be of format .
+
+Example:
+
+The following example demonstrates both interrupt router node and the consumer
+node(main gpio) on the AM654 SoC:
+
+main_intr: interrupt-controller@1 {
+   compatible = "ti,sci-intr";
+   interrupt-controller;
+   interrupt-parent = <>;
+   #interrupt-cells = <3>;
+   ti,sci = <>;
+   ti,sci-dst-id = <56>;
+   ti,sci-rm-range-girq = <0xb 0x1>;
+};
+
+main_gpio0:  main_gpio0@60 {
+   ...
+   interrupt-parent = <_intr>;
+   interrupts = <57 256 IRQ_TYPE_EDGE_RISING>,
+   <57 257 IRQ_TYPE_EDGE_RISING>,
+   <57 258 IRQ_TYPE_EDGE_RISING>,
+   <57 259 IRQ_TYPE_EDGE_RISING>,
+   <57 260 IRQ_TYPE_EDGE_RISING>,
+   <57 261 IRQ_TYPE_EDGE_RISING>;
+   ...
+};
diff --git a/MAINTAINERS b/MAINTAINERS
index 29c08106bd22..a23778b68d74 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14625,6 +14625,7 @@ F:  
Documentation/devicetree/bindings/reset/ti,sci-reset.txt
 F: Documentation/devicetree/bindings/clock/ti,sci-clk.txt
 F: drivers/clk/keystone/sci-clk.c
 F: drivers/reset/reset-ti-sci.c
+F: 

[PATCH 1/2] dt-bindings: irqchip: Introduce TISCI Interrupt router bindings

2018-10-06 Thread Lokesh Vutla
Add the DT binding documentation for Interrupt router driver.

Signed-off-by: Lokesh Vutla 
---
 .../interrupt-controller/ti,sci-intr.txt  | 83 +++
 MAINTAINERS   |  1 +
 2 files changed, 84 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt

diff --git 
a/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt 
b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
new file mode 100644
index ..681ca53cc5fb
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
@@ -0,0 +1,83 @@
+Texas Instruments K3 Interrupt Router
+=
+
+The Interrupt Router (INTR) module provides a mechanism to mux M
+interrupt inputs to N interrupt outputs, where all M inputs are selectable
+to be driven per N output. There is one register per output (MUXCNTL_N) that
+controls the selection.
+
+
+ Interrupt Router
+ +--+
+ |  Inputs Outputs  |
++---+| +--+ |
+| GPIO  |--->| | irq0 | |   Host IRQ
++---+| +--+ |  controller
+ |.+-+  |  +---+
++---+|.|  0  |  |->|  GIC  |
+| INTA  |--->|.+-+  |  +---+
++---+|.  .  |
+ | +--+  .  |
+ | | irqM |+-+  |
+ | +--+|  N  |  |
+ | +-+  |
+ +--+
+
+Configuration of these MUXCNTL_N registers is done by a system controller
+(like the Device Memory and Security Controller on K3 AM654 SoC). System
+controller will keep track of the used and unused registers within the Router.
+Driver should request the system controller to get the range of GIC IRQs
+assigned to the requesting hosts. It is the drivers responsibility to keep
+track of GIC IRQs.
+
+Communication between the host processor running an OS and the system
+controller happens through a protocol called TI System Control Interface
+(TISCI protocol). For more details refer:
+Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
+
+TISCI Interrupt Router Node:
+
+- compatible:  Must be "ti,sci-intr".
+- interrupt-controller:Identifies the node as an interrupt controller
+- #interrupt-cells:Specifies the number of cells needed to encode an
+   interrupt source. The value should be 3.
+   First cell should contain the TISCI device ID of source
+   Second cell should contain the interrupt source offset
+   within the device
+   Third cell specifies the trigger type as defined
+   in interrupts.txt in this directory.
+- interrupt-parent:phandle of irq parent for TISCI intr. The parent must
+   use the same interrupt-cells format as GIC.
+- ti,sci:  Phandle to TI-SCI compatible System controller node.
+- ti,sci-dst-id:   TISCI device ID of the destination IRQ controller.
+- ti,sci-rm-range-girq:Tuple corresponding to unique TISCI resource 
type that
+   defines the dst host irq ranges assigned to this
+   interrupt router from this host context.
+   Tuple should be of format .
+
+Example:
+
+The following example demonstrates both interrupt router node and the consumer
+node(main gpio) on the AM654 SoC:
+
+main_intr: interrupt-controller@1 {
+   compatible = "ti,sci-intr";
+   interrupt-controller;
+   interrupt-parent = <>;
+   #interrupt-cells = <3>;
+   ti,sci = <>;
+   ti,sci-dst-id = <56>;
+   ti,sci-rm-range-girq = <0xb 0x1>;
+};
+
+main_gpio0:  main_gpio0@60 {
+   ...
+   interrupt-parent = <_intr>;
+   interrupts = <57 256 IRQ_TYPE_EDGE_RISING>,
+   <57 257 IRQ_TYPE_EDGE_RISING>,
+   <57 258 IRQ_TYPE_EDGE_RISING>,
+   <57 259 IRQ_TYPE_EDGE_RISING>,
+   <57 260 IRQ_TYPE_EDGE_RISING>,
+   <57 261 IRQ_TYPE_EDGE_RISING>;
+   ...
+};
diff --git a/MAINTAINERS b/MAINTAINERS
index 29c08106bd22..a23778b68d74 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14625,6 +14625,7 @@ F:  
Documentation/devicetree/bindings/reset/ti,sci-reset.txt
 F: Documentation/devicetree/bindings/clock/ti,sci-clk.txt
 F: drivers/clk/keystone/sci-clk.c
 F: drivers/reset/reset-ti-sci.c
+F: 

[PATCH 2/2] irqchip: ti-sci-intr: Add support for Interrupt Router driver

2018-10-06 Thread Lokesh Vutla
Texas Instruments' K3 generation SoCs has an IP Interrupt Router
that does allows for multiplexing of input interrupts to host
interrupt controller. Interrupt Router inputs are either from a
peripheral or from an Interrupt Aggregator which is another
interrupt controller.

Configuration of the interrupt router registers can only be done by
a system co-processor and the driver needs to send a message to this
co processor over TISCI protocol.

Add support for Interrupt Router driver over TISCI protocol.

Signed-off-by: Lokesh Vutla 
---
 MAINTAINERS   |   1 +
 drivers/irqchip/Kconfig   |  11 +
 drivers/irqchip/Makefile  |   1 +
 drivers/irqchip/irq-ti-sci-intr.c | 325 ++
 4 files changed, 338 insertions(+)
 create mode 100644 drivers/irqchip/irq-ti-sci-intr.c

diff --git a/MAINTAINERS b/MAINTAINERS
index a23778b68d74..cf3c834f8cee 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14626,6 +14626,7 @@ F:  
Documentation/devicetree/bindings/clock/ti,sci-clk.txt
 F: drivers/clk/keystone/sci-clk.c
 F: drivers/reset/reset-ti-sci.c
 F: Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
+F: drivers/irqchip/irq-ti-sci-intr.c
 
 THANKO'S RAREMONO AM/FM/SW RADIO RECEIVER USB DRIVER
 M: Hans Verkuil 
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 96451b581452..9a965fe22043 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -374,6 +374,17 @@ config QCOM_PDC
  Power Domain Controller driver to manage and configure wakeup
  IRQs for Qualcomm Technologies Inc (QTI) mobile chips.
 
+config TI_SCI_INTR_IRQCHIP
+   tristate "TISCI based Interrupt Router irqchip driver"
+   depends on TI_SCI_PROTOCOL && ARCH_K3
+   select IRQ_DOMAIN
+   select IRQ_DOMAIN_HIERARCHY
+   help
+ This enables the irqchip driver support for K3 Interrupt router
+ over TI System Control Interface available on some new TI's SoCs.
+ If you wish to use interrupt router irq resources managed by the
+ TI System Controller, say Y here. Otherwise, say N.
+
 endmenu
 
 config SIFIVE_PLIC
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index b822199445ff..44bf65606d60 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -89,3 +89,4 @@ obj-$(CONFIG_GOLDFISH_PIC)+= irq-goldfish-pic.o
 obj-$(CONFIG_NDS32)+= irq-ativic32.o
 obj-$(CONFIG_QCOM_PDC) += qcom-pdc.o
 obj-$(CONFIG_SIFIVE_PLIC)  += irq-sifive-plic.o
+obj-$(CONFIG_TI_SCI_INTR_IRQCHIP)  += irq-ti-sci-intr.o
diff --git a/drivers/irqchip/irq-ti-sci-intr.c 
b/drivers/irqchip/irq-ti-sci-intr.c
new file mode 100644
index ..f04fe6da1b09
--- /dev/null
+++ b/drivers/irqchip/irq-ti-sci-intr.c
@@ -0,0 +1,325 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Texas Instruments' K3 Interrupt Router irqchip driver
+ *
+ * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/
+ * Lokesh Vutla 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define TI_SCI_DEV_ID_MASK 0x
+#define TI_SCI_DEV_ID_SHIFT16
+#define TI_SCI_IRQ_ID_MASK 0x
+#define TI_SCI_IRQ_ID_SHIFT0
+#define TI_SCI_IS_EVENT_IRQBIT(31)
+
+#define HWIRQ_TO_DEVID(HWIRQ)  (((HWIRQ) >> (TI_SCI_DEV_ID_SHIFT)) & \
+(TI_SCI_DEV_ID_MASK))
+#define HWIRQ_TO_IRQID(HWIRQ)  ((HWIRQ) & (TI_SCI_IRQ_ID_MASK))
+
+/**
+ * struct ti_sci_intr_irq_domain - Structure representing a TISCI based
+ *Interrupt Router IRQ domain.
+ * @sci:   Pointer to TISCI handle
+ * @dst_irq:   TISCI resource pointer representing destination irq controller.
+ * @dst_id:TISCI device ID of the destination irq controller.
+ */
+struct ti_sci_intr_irq_domain {
+   const struct ti_sci_handle *sci;
+   struct ti_sci_resource *dst_irq;
+   u16 dst_id;
+};
+
+/**
+ * struct ti_sci_intr_irq_desc - Description of an Interrupt Router IRQ
+ * @src_id:TISCI device ID of the IRQ source
+ * @src_index: IRQ source index within the device.
+ * @dst_irq:   Destination host IRQ.
+ */
+struct ti_sci_intr_irq_desc {
+   u16 src_id;
+   u16 src_index;
+   u16 dst_irq;
+};
+
+static struct irq_chip ti_sci_intr_irq_chip = {
+   .name   = "INTR",
+   .irq_eoi= irq_chip_eoi_parent,
+   .irq_mask   = irq_chip_mask_parent,
+   .irq_unmask = irq_chip_unmask_parent,
+   .irq_retrigger  = irq_chip_retrigger_hierarchy,
+   .irq_set_type   = irq_chip_set_type_parent,
+   .irq_set_affinity   = irq_chip_set_affinity_parent,
+};
+
+/**
+ * ti_sci_intr_irq_domain_translate() - Retrieve hwirq and type from
+ * IRQ firmware specific handler.
+ 

[PATCH 0/2] irqchip: ti-sci-intr: Add support for K3 Interrupt Router

2018-10-06 Thread Lokesh Vutla
This series adds irqchip driver for Texas Instruments' K3 based
Interrupt Router.

This series depends on TISCI IRQ management support posted here[1]

[1] 
http://lists.infradead.org/pipermail/linux-arm-kernel/2018-October/605784.html

Lokesh Vutla (2):
  dt-bindings: irqchip: Introduce TISCI Interrupt router bindings
  irqchip: ti-sci-intr: Add support for Interrupt Router driver

 .../interrupt-controller/ti,sci-intr.txt  |  83 +
 MAINTAINERS   |   2 +
 drivers/irqchip/Kconfig   |  11 +
 drivers/irqchip/Makefile  |   1 +
 drivers/irqchip/irq-ti-sci-intr.c | 325 ++
 5 files changed, 422 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
 create mode 100644 drivers/irqchip/irq-ti-sci-intr.c

-- 
2.19.0



[PATCH] STAGING/EMXX_UDC: Fixed all meaningful sparse errors.

2018-10-06 Thread Carmeli Tamir
Fixed all meaningful sparse errors: 
1. Added static to udc_controller 
2. Added mising __iomem modifier to handle p_regs 
3. Added missing le16_to_cpu

Signed-off-by: Tamir Carmeli 
---

 drivers/staging/emxx_udc/emxx_udc.c | 69 +++--
 drivers/staging/emxx_udc/emxx_udc.h |  2 +-
 3 files changed, 38 insertions(+), 35 deletions(-)

diff --git a/drivers/staging/emxx_udc/emxx_udc.c 
b/drivers/staging/emxx_udc/emxx_udc.c
index 3e51476..83eb430 100644
--- a/drivers/staging/emxx_udc/emxx_udc.c
+++ b/drivers/staging/emxx_udc/emxx_udc.c
@@ -56,25 +56,25 @@ static void _nbu2ss_fifo_flush(struct nbu2ss_udc *, struct 
nbu2ss_ep *);
 
 /*===*/
 /* Global */
-struct nbu2ss_udc udc_controller;
+static struct nbu2ss_udc udc_controller;
 
 /*-*/
 /* Read */
-static inline u32 _nbu2ss_readl(void *address)
+static inline u32 _nbu2ss_readl(void __iomem *address)
 {
return __raw_readl(address);
 }
 
 /*-*/
 /* Write */
-static inline void _nbu2ss_writel(void *address, u32 udata)
+static inline void _nbu2ss_writel(void __iomem *address, u32 udata)
 {
__raw_writel(udata, address);
 }
 
 /*-*/
 /* Set Bit */
-static inline void _nbu2ss_bitset(void *address, u32 udata)
+static inline void _nbu2ss_bitset(void __iomem *address, u32 udata)
 {
u32 reg_dt = __raw_readl(address) | (udata);
 
@@ -83,7 +83,7 @@ static inline void _nbu2ss_bitset(void *address, u32 udata)
 
 /*-*/
 /* Clear Bit */
-static inline void _nbu2ss_bitclr(void *address, u32 udata)
+static inline void _nbu2ss_bitclr(void __iomem *address, u32 udata)
 {
u32 reg_dt = __raw_readl(address) & ~(udata);
 
@@ -135,6 +135,7 @@ static void _nbu2ss_ep0_complete(struct usb_ep *_ep, struct 
usb_request *_req)
 {
u8  recipient;
u16 selector;
+   u16 wIndex;
u32 test_mode;
struct usb_ctrlrequest  *p_ctrl;
struct nbu2ss_udc *udc;
@@ -149,10 +150,11 @@ static void _nbu2ss_ep0_complete(struct usb_ep *_ep, 
struct usb_request *_req)
/*-*/
/* SET_FEATURE */
recipient = (u8)(p_ctrl->bRequestType & USB_RECIP_MASK);
-   selector  = p_ctrl->wValue;
+   selector  = le16_to_cpu(p_ctrl->wValue);
if ((recipient == USB_RECIP_DEVICE) &&
(selector == USB_DEVICE_TEST_MODE)) {
-   test_mode = (u32)(p_ctrl->wIndex >> 8);
+   wIndex = le16_to_cpu(p_ctrl->wIndex);
+   test_mode = (u32)(wIndex >> 8);
_nbu2ss_set_test_mode(udc, test_mode);
}
}
@@ -184,7 +186,7 @@ static u32 _nbu2ss_get_begin_ram_address(struct nbu2ss_udc 
*udc)
u32 num, buf_type;
u32 data, last_ram_adr, use_ram_size;
 
-   struct ep_regs *p_ep_regs;
+   struct ep_regs __iomem *p_ep_regs;
 
last_ram_adr = (D_RAM_SIZE_CTRL / sizeof(u32)) * 2;
use_ram_size = 0;
@@ -377,7 +379,7 @@ static void _nbu2ss_ep_dma_exit(struct nbu2ss_udc *udc, 
struct nbu2ss_ep *ep)
 {
u32 num;
u32 data;
-   struct fc_regs  *preg = udc->p_regs;
+   struct fc_regs __iomem  *preg = udc->p_regs;
 
if (udc->vbus_active == 0)
return; /* VBUS OFF */
@@ -408,7 +410,7 @@ static void _nbu2ss_ep_dma_exit(struct nbu2ss_udc *udc, 
struct nbu2ss_ep *ep)
 /* Abort DMA */
 static void _nbu2ss_ep_dma_abort(struct nbu2ss_udc *udc, struct nbu2ss_ep *ep)
 {
-   struct fc_regs  *preg = udc->p_regs;
+   struct fc_regs __iomem *preg = udc->p_regs;
 
_nbu2ss_bitclr(>EP_DCR[ep->epnum - 1].EP_DCR1, DCR1_EPN_REQEN);
mdelay(DMA_DISABLE_TIME);   /* DCR1_EPN_REQEN Clear */
@@ -426,7 +428,7 @@ static void _nbu2ss_ep_in_end(
 {
u32 data;
u32 num;
-   struct fc_regs  *preg = udc->p_regs;
+   struct fc_regs __iomem *preg = udc->p_regs;
 
if (length >= sizeof(u32))
return;
@@ -817,7 +819,7 @@ static int _nbu2ss_out_dma(
u32 burst = 1;
u32 data;
int result = -EINVAL;
-   struct fc_regs  *preg = udc->p_regs;
+   struct fc_regs __iomem  *preg = udc->p_regs;
 
if (req->dma_flag)
return 1;   /* DMA is forwarded */
@@ -880,7 +882,7 @@ static int _nbu2ss_epn_out_pio(
union 

[PATCH] STAGING/EMXX_UDC: Fixed all meaningful sparse errors.

2018-10-06 Thread Carmeli Tamir
Fixed all meaningful sparse errors: 
1. Added static to udc_controller 
2. Added mising __iomem modifier to handle p_regs 
3. Added missing le16_to_cpu

Signed-off-by: Tamir Carmeli 
---

 drivers/staging/emxx_udc/emxx_udc.c | 69 +++--
 drivers/staging/emxx_udc/emxx_udc.h |  2 +-
 3 files changed, 38 insertions(+), 35 deletions(-)

diff --git a/drivers/staging/emxx_udc/emxx_udc.c 
b/drivers/staging/emxx_udc/emxx_udc.c
index 3e51476..83eb430 100644
--- a/drivers/staging/emxx_udc/emxx_udc.c
+++ b/drivers/staging/emxx_udc/emxx_udc.c
@@ -56,25 +56,25 @@ static void _nbu2ss_fifo_flush(struct nbu2ss_udc *, struct 
nbu2ss_ep *);
 
 /*===*/
 /* Global */
-struct nbu2ss_udc udc_controller;
+static struct nbu2ss_udc udc_controller;
 
 /*-*/
 /* Read */
-static inline u32 _nbu2ss_readl(void *address)
+static inline u32 _nbu2ss_readl(void __iomem *address)
 {
return __raw_readl(address);
 }
 
 /*-*/
 /* Write */
-static inline void _nbu2ss_writel(void *address, u32 udata)
+static inline void _nbu2ss_writel(void __iomem *address, u32 udata)
 {
__raw_writel(udata, address);
 }
 
 /*-*/
 /* Set Bit */
-static inline void _nbu2ss_bitset(void *address, u32 udata)
+static inline void _nbu2ss_bitset(void __iomem *address, u32 udata)
 {
u32 reg_dt = __raw_readl(address) | (udata);
 
@@ -83,7 +83,7 @@ static inline void _nbu2ss_bitset(void *address, u32 udata)
 
 /*-*/
 /* Clear Bit */
-static inline void _nbu2ss_bitclr(void *address, u32 udata)
+static inline void _nbu2ss_bitclr(void __iomem *address, u32 udata)
 {
u32 reg_dt = __raw_readl(address) & ~(udata);
 
@@ -135,6 +135,7 @@ static void _nbu2ss_ep0_complete(struct usb_ep *_ep, struct 
usb_request *_req)
 {
u8  recipient;
u16 selector;
+   u16 wIndex;
u32 test_mode;
struct usb_ctrlrequest  *p_ctrl;
struct nbu2ss_udc *udc;
@@ -149,10 +150,11 @@ static void _nbu2ss_ep0_complete(struct usb_ep *_ep, 
struct usb_request *_req)
/*-*/
/* SET_FEATURE */
recipient = (u8)(p_ctrl->bRequestType & USB_RECIP_MASK);
-   selector  = p_ctrl->wValue;
+   selector  = le16_to_cpu(p_ctrl->wValue);
if ((recipient == USB_RECIP_DEVICE) &&
(selector == USB_DEVICE_TEST_MODE)) {
-   test_mode = (u32)(p_ctrl->wIndex >> 8);
+   wIndex = le16_to_cpu(p_ctrl->wIndex);
+   test_mode = (u32)(wIndex >> 8);
_nbu2ss_set_test_mode(udc, test_mode);
}
}
@@ -184,7 +186,7 @@ static u32 _nbu2ss_get_begin_ram_address(struct nbu2ss_udc 
*udc)
u32 num, buf_type;
u32 data, last_ram_adr, use_ram_size;
 
-   struct ep_regs *p_ep_regs;
+   struct ep_regs __iomem *p_ep_regs;
 
last_ram_adr = (D_RAM_SIZE_CTRL / sizeof(u32)) * 2;
use_ram_size = 0;
@@ -377,7 +379,7 @@ static void _nbu2ss_ep_dma_exit(struct nbu2ss_udc *udc, 
struct nbu2ss_ep *ep)
 {
u32 num;
u32 data;
-   struct fc_regs  *preg = udc->p_regs;
+   struct fc_regs __iomem  *preg = udc->p_regs;
 
if (udc->vbus_active == 0)
return; /* VBUS OFF */
@@ -408,7 +410,7 @@ static void _nbu2ss_ep_dma_exit(struct nbu2ss_udc *udc, 
struct nbu2ss_ep *ep)
 /* Abort DMA */
 static void _nbu2ss_ep_dma_abort(struct nbu2ss_udc *udc, struct nbu2ss_ep *ep)
 {
-   struct fc_regs  *preg = udc->p_regs;
+   struct fc_regs __iomem *preg = udc->p_regs;
 
_nbu2ss_bitclr(>EP_DCR[ep->epnum - 1].EP_DCR1, DCR1_EPN_REQEN);
mdelay(DMA_DISABLE_TIME);   /* DCR1_EPN_REQEN Clear */
@@ -426,7 +428,7 @@ static void _nbu2ss_ep_in_end(
 {
u32 data;
u32 num;
-   struct fc_regs  *preg = udc->p_regs;
+   struct fc_regs __iomem *preg = udc->p_regs;
 
if (length >= sizeof(u32))
return;
@@ -817,7 +819,7 @@ static int _nbu2ss_out_dma(
u32 burst = 1;
u32 data;
int result = -EINVAL;
-   struct fc_regs  *preg = udc->p_regs;
+   struct fc_regs __iomem  *preg = udc->p_regs;
 
if (req->dma_flag)
return 1;   /* DMA is forwarded */
@@ -880,7 +882,7 @@ static int _nbu2ss_epn_out_pio(
union 

RE: [RFC PATCH 2/2] dmaengine: xilinx_dma: Add Xilinx AXI MCDMA Engine driver support

2018-10-06 Thread Radhey Shyam Pandey
Thanks for the review.

> On 31-07-18, 23:16, Radhey Shyam Pandey wrote:
> >  struct xilinx_dma_config {
> > @@ -402,6 +470,7 @@ struct xilinx_dma_config {
> > int (*clk_init)(struct platform_device *pdev, struct clk **axi_clk,
> > struct clk **tx_clk, struct clk **txs_clk,
> > struct clk **rx_clk, struct clk **rxs_clk);
> > +   irqreturn_t (*irq_handler)(int irq, void *data);
> 
> this sounds like a preparatory change?

Yes, I will split it in a separate patch.
> 
> > +   } else if (chan->xdev->dma_config->dmatype ==
> XDMA_TYPE_AXIMCDMA) {
> > +   /* Allocate the buffer descriptors. */
> > +   chan->seg_mv = dma_zalloc_coherent(chan->dev,
> > + sizeof(*chan->seg_mv) *
> > + XILINX_DMA_NUM_DESCS,
> > + >seg_p,
> GFP_KERNEL);
> > +   if (!chan->seg_mv) {
> > +   dev_err(chan->dev,
> > +   "unable to allocate channel %d descriptors\n",
> > +   chan->id);
> > +   return -ENOMEM;
> > +   }
> > +   for (i = 0; i < XILINX_DMA_NUM_DESCS; i++) {
> > +   chan->seg_mv[i].hw.next_desc =
> > +   lower_32_bits(chan->seg_p + sizeof(*chan->seg_mv) *
> > +   ((i + 1) % XILINX_DMA_NUM_DESCS));
> > +   chan->seg_mv[i].hw.next_desc_msb =
> > +   upper_32_bits(chan->seg_p + sizeof(*chan->seg_mv) *
> > +   ((i + 1) % XILINX_DMA_NUM_DESCS));
> > +   chan->seg_mv[i].phys = chan->seg_p +
> > +   sizeof(*chan->seg_v) * i;
> > +   list_add_tail(>seg_mv[i].node,
> > + >free_seg_list);
> > +   }
> 
> only change with this and previous one seems to be use of seg_mv instead
> of seg_v right? if so, can you try to modularise this..
Agree. I will modularise it.

> 
> >  /**
> > + * xilinx_mcdma_start_transfer - Starts MCDMA transfer
> > + * @chan: Driver specific channel struct pointer
> > + */
> > +static void xilinx_mcdma_start_transfer(struct xilinx_dma_chan *chan)
> > +{
> > +   struct xilinx_dma_tx_descriptor *head_desc, *tail_desc;
> > +   struct xilinx_axidma_tx_segment *tail_segment;
> > +   u32 reg;
> > +
> > +   if (chan->err)
> > +   return;
> > +
> > +   if (!chan->idle)
> > +   return;
> > +
> > +   if (list_empty(>pending_list))
> > +   return;
> 
> okay i was thinking that we need lock here, but then this is called with
> lock held, worth mentioning in the comment though..
Ok. Will add.

> 
> > +static irqreturn_t xilinx_mcdma_irq_handler(int irq, void *data)
> > +{
> > +   struct xilinx_dma_chan *chan = data;
> > +   u32 status, ser_offset, chan_sermask, chan_offset = 0, chan_id;
> > +
> > +   if (chan->direction == DMA_DEV_TO_MEM)
> > +   ser_offset = XILINX_MCDMA_RXINT_SER_OFFSET;
> > +   else
> > +   ser_offset = XILINX_MCDMA_TXINT_SER_OFFSET;
> > +
> > +   /* Read the channel id raising the interrupt*/
> > +   chan_sermask = dma_ctrl_read(chan, ser_offset);
> > +   chan_id = ffs(chan_sermask);
> > +
> > +   if (!chan_id)
> > +   return IRQ_NONE;
> > +
> > +   if (chan->direction == DMA_DEV_TO_MEM)
> > +   chan_offset = XILINX_DMA_MAX_CHANS_PER_DEVICE / 2;
> > +
> > +   chan_offset = chan_offset + (chan_id - 1);
> > +   chan = chan->xdev->chan[chan_offset];
> > +   /* Read the status and ack the interrupts. */
> > +   status = dma_ctrl_read(chan,
> XILINX_MCDMA_CHAN_SR_OFFSET(chan->tdest));
> > +   if (!(status & XILINX_MCDMA_IRQ_ALL_MASK))
> > +   return IRQ_NONE;
> > +
> > +   dma_ctrl_write(chan, XILINX_MCDMA_CHAN_SR_OFFSET(chan-
> >tdest),
> > +  status & XILINX_MCDMA_IRQ_ALL_MASK);
> > +
> > +   if (status & XILINX_MCDMA_IRQ_ERR_MASK) {
> > +   dev_err(chan->dev, "Channel %p has errors %x cdr %x tdr
> %x\n",
> > +   chan, dma_ctrl_read(chan,
> > +   XILINX_MCDMA_CH_ERR_OFFSET),
> dma_ctrl_read(chan,
> > +   XILINX_MCDMA_CHAN_CDESC_OFFSET(chan->tdest)),
> > +   dma_ctrl_read(chan,
> > + XILINX_MCDMA_CHAN_TDESC_OFFSET
> > + (chan->tdest)));
> 
> this looks very hard to read, please start each dma_ctrl_read() from a
> new line to make it better

Ok . will change.
> 
> > +   chan->err = true;
> > +   }
> > +
> > +   if (status & XILINX_MCDMA_IRQ_DELAY_MASK) {
> > +   /*
> > +* Device takes too long to do the transfer when user requires
> > +* responsiveness.
> > +*/
> > +   dev_dbg(chan->dev, "Inter-packet latency too long\n");
> 
> so we just log it..?
For now yes. It will be used later when MCDMA is used by the network client.

> 
> > +   }
> > +
> 

RE: [RFC PATCH 2/2] dmaengine: xilinx_dma: Add Xilinx AXI MCDMA Engine driver support

2018-10-06 Thread Radhey Shyam Pandey
Thanks for the review.

> On 31-07-18, 23:16, Radhey Shyam Pandey wrote:
> >  struct xilinx_dma_config {
> > @@ -402,6 +470,7 @@ struct xilinx_dma_config {
> > int (*clk_init)(struct platform_device *pdev, struct clk **axi_clk,
> > struct clk **tx_clk, struct clk **txs_clk,
> > struct clk **rx_clk, struct clk **rxs_clk);
> > +   irqreturn_t (*irq_handler)(int irq, void *data);
> 
> this sounds like a preparatory change?

Yes, I will split it in a separate patch.
> 
> > +   } else if (chan->xdev->dma_config->dmatype ==
> XDMA_TYPE_AXIMCDMA) {
> > +   /* Allocate the buffer descriptors. */
> > +   chan->seg_mv = dma_zalloc_coherent(chan->dev,
> > + sizeof(*chan->seg_mv) *
> > + XILINX_DMA_NUM_DESCS,
> > + >seg_p,
> GFP_KERNEL);
> > +   if (!chan->seg_mv) {
> > +   dev_err(chan->dev,
> > +   "unable to allocate channel %d descriptors\n",
> > +   chan->id);
> > +   return -ENOMEM;
> > +   }
> > +   for (i = 0; i < XILINX_DMA_NUM_DESCS; i++) {
> > +   chan->seg_mv[i].hw.next_desc =
> > +   lower_32_bits(chan->seg_p + sizeof(*chan->seg_mv) *
> > +   ((i + 1) % XILINX_DMA_NUM_DESCS));
> > +   chan->seg_mv[i].hw.next_desc_msb =
> > +   upper_32_bits(chan->seg_p + sizeof(*chan->seg_mv) *
> > +   ((i + 1) % XILINX_DMA_NUM_DESCS));
> > +   chan->seg_mv[i].phys = chan->seg_p +
> > +   sizeof(*chan->seg_v) * i;
> > +   list_add_tail(>seg_mv[i].node,
> > + >free_seg_list);
> > +   }
> 
> only change with this and previous one seems to be use of seg_mv instead
> of seg_v right? if so, can you try to modularise this..
Agree. I will modularise it.

> 
> >  /**
> > + * xilinx_mcdma_start_transfer - Starts MCDMA transfer
> > + * @chan: Driver specific channel struct pointer
> > + */
> > +static void xilinx_mcdma_start_transfer(struct xilinx_dma_chan *chan)
> > +{
> > +   struct xilinx_dma_tx_descriptor *head_desc, *tail_desc;
> > +   struct xilinx_axidma_tx_segment *tail_segment;
> > +   u32 reg;
> > +
> > +   if (chan->err)
> > +   return;
> > +
> > +   if (!chan->idle)
> > +   return;
> > +
> > +   if (list_empty(>pending_list))
> > +   return;
> 
> okay i was thinking that we need lock here, but then this is called with
> lock held, worth mentioning in the comment though..
Ok. Will add.

> 
> > +static irqreturn_t xilinx_mcdma_irq_handler(int irq, void *data)
> > +{
> > +   struct xilinx_dma_chan *chan = data;
> > +   u32 status, ser_offset, chan_sermask, chan_offset = 0, chan_id;
> > +
> > +   if (chan->direction == DMA_DEV_TO_MEM)
> > +   ser_offset = XILINX_MCDMA_RXINT_SER_OFFSET;
> > +   else
> > +   ser_offset = XILINX_MCDMA_TXINT_SER_OFFSET;
> > +
> > +   /* Read the channel id raising the interrupt*/
> > +   chan_sermask = dma_ctrl_read(chan, ser_offset);
> > +   chan_id = ffs(chan_sermask);
> > +
> > +   if (!chan_id)
> > +   return IRQ_NONE;
> > +
> > +   if (chan->direction == DMA_DEV_TO_MEM)
> > +   chan_offset = XILINX_DMA_MAX_CHANS_PER_DEVICE / 2;
> > +
> > +   chan_offset = chan_offset + (chan_id - 1);
> > +   chan = chan->xdev->chan[chan_offset];
> > +   /* Read the status and ack the interrupts. */
> > +   status = dma_ctrl_read(chan,
> XILINX_MCDMA_CHAN_SR_OFFSET(chan->tdest));
> > +   if (!(status & XILINX_MCDMA_IRQ_ALL_MASK))
> > +   return IRQ_NONE;
> > +
> > +   dma_ctrl_write(chan, XILINX_MCDMA_CHAN_SR_OFFSET(chan-
> >tdest),
> > +  status & XILINX_MCDMA_IRQ_ALL_MASK);
> > +
> > +   if (status & XILINX_MCDMA_IRQ_ERR_MASK) {
> > +   dev_err(chan->dev, "Channel %p has errors %x cdr %x tdr
> %x\n",
> > +   chan, dma_ctrl_read(chan,
> > +   XILINX_MCDMA_CH_ERR_OFFSET),
> dma_ctrl_read(chan,
> > +   XILINX_MCDMA_CHAN_CDESC_OFFSET(chan->tdest)),
> > +   dma_ctrl_read(chan,
> > + XILINX_MCDMA_CHAN_TDESC_OFFSET
> > + (chan->tdest)));
> 
> this looks very hard to read, please start each dma_ctrl_read() from a
> new line to make it better

Ok . will change.
> 
> > +   chan->err = true;
> > +   }
> > +
> > +   if (status & XILINX_MCDMA_IRQ_DELAY_MASK) {
> > +   /*
> > +* Device takes too long to do the transfer when user requires
> > +* responsiveness.
> > +*/
> > +   dev_dbg(chan->dev, "Inter-packet latency too long\n");
> 
> so we just log it..?
For now yes. It will be used later when MCDMA is used by the network client.

> 
> > +   }
> > +
> 

Re: [PATCH] kvm/x86 : avoid shifting signed 32-bit value by 31 bits

2018-10-06 Thread Wei Yang
On Sat, Oct 06, 2018 at 11:31:04AM +0800, peng.h...@zte.com.cn wrote:
>>On Thu, Oct 04, 2018 at 01:47:18PM -0400, Peng Hao wrote:
>>>
>>>From: Peng Hao 
>>>
>>>  modify AVIC_LOGICAL_ID_ENTRY_VALID_MASK to unsigned
>>>
>>>Signed-off-by: Peng Hao 
>>>---
>>> arch/x86/kvm/svm.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>>diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
>>>index d96092b..bf1ded4 100644
>>>--- a/arch/x86/kvm/svm.c
>>>+++ b/arch/x86/kvm/svm.c
>>>@@ -262,7 +262,7 @@ struct amd_svm_iommu_ir {
>>> };
>>>
>>> #define AVIC_LOGICAL_ID_ENTRY_GUEST_PHYSICAL_ID_MASK(0xFF)
>>>-#define AVIC_LOGICAL_ID_ENTRY_VALID_MASK(1 << 31)
>>>+#define AVIC_LOGICAL_ID_ENTRY_VALID_MASK(1UL << 31)
>
>>It is reasonable to change to unsigned, while not necessary to unsigned
>>long?
>AVIC_LOGICAL_ID_ENTRY_VALID_MASK is used in function avic_ldr_write.
>here I think it doesn't matter if you use unsigned or unsigned long. Do you 
>have any suggestions?

In current case, AVIC_LOGICAL_ID_ENTRY_VALID_MASK is used to calculate
the value of new_entry with type of u32. So the definition here is not
harmful.

Also, I did a quick grep and found similar definition (1 << 31) is popular
in the whole kernel tree.

The reason to make this change is not that strong to me. Would you
minding sharing more reason behind this change?

>>--
>>Wei Yang
>>Help you, Help me


-- 
Wei Yang
Help you, Help me


Re: [PATCH] kvm/x86 : avoid shifting signed 32-bit value by 31 bits

2018-10-06 Thread Wei Yang
On Sat, Oct 06, 2018 at 11:31:04AM +0800, peng.h...@zte.com.cn wrote:
>>On Thu, Oct 04, 2018 at 01:47:18PM -0400, Peng Hao wrote:
>>>
>>>From: Peng Hao 
>>>
>>>  modify AVIC_LOGICAL_ID_ENTRY_VALID_MASK to unsigned
>>>
>>>Signed-off-by: Peng Hao 
>>>---
>>> arch/x86/kvm/svm.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>>diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
>>>index d96092b..bf1ded4 100644
>>>--- a/arch/x86/kvm/svm.c
>>>+++ b/arch/x86/kvm/svm.c
>>>@@ -262,7 +262,7 @@ struct amd_svm_iommu_ir {
>>> };
>>>
>>> #define AVIC_LOGICAL_ID_ENTRY_GUEST_PHYSICAL_ID_MASK(0xFF)
>>>-#define AVIC_LOGICAL_ID_ENTRY_VALID_MASK(1 << 31)
>>>+#define AVIC_LOGICAL_ID_ENTRY_VALID_MASK(1UL << 31)
>
>>It is reasonable to change to unsigned, while not necessary to unsigned
>>long?
>AVIC_LOGICAL_ID_ENTRY_VALID_MASK is used in function avic_ldr_write.
>here I think it doesn't matter if you use unsigned or unsigned long. Do you 
>have any suggestions?

In current case, AVIC_LOGICAL_ID_ENTRY_VALID_MASK is used to calculate
the value of new_entry with type of u32. So the definition here is not
harmful.

Also, I did a quick grep and found similar definition (1 << 31) is popular
in the whole kernel tree.

The reason to make this change is not that strong to me. Would you
minding sharing more reason behind this change?

>>--
>>Wei Yang
>>Help you, Help me


-- 
Wei Yang
Help you, Help me


[PATCH v2 1/4] platform/x86: intel_pmc_core: Show Latency Tolerance info

2018-10-06 Thread Rajneesh Bhardwaj
This adds support to show the Latency Tolerance Reporting for the IPs on
the PCH as reported by the PMC. The format shown here is raw LTR data
payload that can further be decoded as per the PCI specification.

This also fixes some minor alignment issues in the header file by
removing spaces and converting to tabs at some places.

Signed-off-by: Rajneesh Bhardwaj 
---
Changes in v2:
 * Removed IP # from map and displaying IP # while printing.
 * Other style fixes as per Andy's suggestion.

 drivers/platform/x86/intel_pmc_core.c | 73 +++
 drivers/platform/x86/intel_pmc_core.h | 56 +---
 2 files changed, 122 insertions(+), 7 deletions(-)

diff --git a/drivers/platform/x86/intel_pmc_core.c 
b/drivers/platform/x86/intel_pmc_core.c
index 2d272a3e0176..217a822a8da1 100644
--- a/drivers/platform/x86/intel_pmc_core.c
+++ b/drivers/platform/x86/intel_pmc_core.c
@@ -110,10 +110,37 @@ static const struct pmc_bit_map spt_pfear_map[] = {
{},
 };
 
+static const struct pmc_bit_map spt_ltr_show_map[] = {
+   {"LTR_SOUTHPORT_A", SPT_PMC_LTR_SPA},
+   {"LTR_SOUTHPORT_B", SPT_PMC_LTR_SPB},
+   {"LTR_SATA",SPT_PMC_LTR_SATA},
+   {"LTR_GIGABIT_ETHERNET",SPT_PMC_LTR_GBE},
+   {"LTR_XHCI",SPT_PMC_LTR_XHCI},
+   /* IP 5 is reserved */
+   {"LTR_ME",  SPT_PMC_LTR_ME},
+   /* EVA is Enterprise Value Add, doesn't really exist on PCH */
+   {"LTR_EVA", SPT_PMC_LTR_EVA},
+   {"LTR_SOUTHPORT_C", SPT_PMC_LTR_SPC},
+   {"LTR_HD_AUDIO",SPT_PMC_LTR_AZ},
+   /* IP 10 is reserved */
+   {"LTR_LPSS",SPT_PMC_LTR_LPSS},
+   {"LTR_SOUTHPORT_D", SPT_PMC_LTR_SPD},
+   {"LTR_SOUTHPORT_E", SPT_PMC_LTR_SPE},
+   {"LTR_CAMERA",  SPT_PMC_LTR_CAM},
+   {"LTR_ESPI",SPT_PMC_LTR_ESPI},
+   {"LTR_SCC", SPT_PMC_LTR_SCC},
+   {"LTR_ISH", SPT_PMC_LTR_ISH},
+   /* Below two cannot be used for LTR_IGNORE */
+   {"LTR_CURRENT_PLATFORM",SPT_PMC_LTR_CUR_PLT},
+   {"LTR_AGGREGATED_SYSTEM",   SPT_PMC_LTR_CUR_ASLT},
+   {}
+};
+
 static const struct pmc_reg_map spt_reg_map = {
.pfear_sts = spt_pfear_map,
.mphy_sts = spt_mphy_map,
.pll_sts = spt_pll_map,
+   .ltr_show_sts = spt_ltr_show_map,
.slp_s0_offset = SPT_PMC_SLP_S0_RES_COUNTER_OFFSET,
.ltr_ignore_offset = SPT_PMC_LTR_IGNORE_OFFSET,
.regmap_length = SPT_PMC_MMIO_REG_LEN,
@@ -252,10 +279,39 @@ static const struct pmc_bit_map *cnp_slps0_dbg_maps[] = {
NULL,
 };
 
+static const struct pmc_bit_map cnp_ltr_show_map[] = {
+   {"LTR_SOUTHPORT_A", CNP_PMC_LTR_SPA},
+   {"LTR_SOUTHPORT_B", CNP_PMC_LTR_SPB},
+   {"LTR_SATA",CNP_PMC_LTR_SATA},
+   {"LTR_GIGABIT_ETHERNET",CNP_PMC_LTR_GBE},
+   {"LTR_XHCI",CNP_PMC_LTR_XHCI},
+   /* IP 5 is reserved */
+   {"LTR_ME",  CNP_PMC_LTR_ME},
+   /* EVA is Enterprise Value Add, doesn't really exist on PCH */
+   {"LTR_EVA", CNP_PMC_LTR_EVA},
+   {"LTR_SOUTHPORT_C", CNP_PMC_LTR_SPC},
+   {"LTR_HD_AUDIO",CNP_PMC_LTR_AZ},
+   {"LTR_CNV", CNP_PMC_LTR_CNV},
+   {"LTR_LPSS",CNP_PMC_LTR_LPSS},
+   {"LTR_SOUTHPORT_D", CNP_PMC_LTR_SPD},
+   {"LTR_SOUTHPORT_E", CNP_PMC_LTR_SPE},
+   {"LTR_CAMERA",  CNP_PMC_LTR_CAM},
+   {"LTR_ESPI",CNP_PMC_LTR_ESPI},
+   {"LTR_SCC", CNP_PMC_LTR_SCC},
+   {"LTR_ISH", CNP_PMC_LTR_ISH},
+   {"LTR_UFSX2",   CNP_PMC_LTR_UFSX2},
+   {"LTR_EMMC",CNP_PMC_LTR_EMMC},
+   /* Below two cannot be used for LTR_IGNORE */
+   {"LTR_CURRENT_PLATFORM",CNP_PMC_LTR_CUR_PLT},
+   {"LTR_AGGREGATED_SYSTEM",   CNP_PMC_LTR_CUR_ASLT},
+   {}
+};
+
 static const struct pmc_reg_map cnp_reg_map = {
.pfear_sts = cnp_pfear_map,
.slp_s0_offset = CNP_PMC_SLP_S0_RES_COUNTER_OFFSET,
.slps0_dbg_maps = cnp_slps0_dbg_maps,
+   .ltr_show_sts = cnp_ltr_show_map,
.slps0_dbg_offset = CNP_PMC_SLPS0_DBG_OFFSET,
.ltr_ignore_offset = CNP_PMC_LTR_IGNORE_OFFSET,
.regmap_length = CNP_PMC_MMIO_REG_LEN,
@@ -592,6 +648,21 @@ static int pmc_core_slps0_dbg_show(struct seq_file *s, 
void *unused)
 }
 DEFINE_SHOW_ATTRIBUTE(pmc_core_slps0_dbg);
 
+static int pmc_core_ltr_show(struct seq_file *s, void *unused)
+{
+   struct pmc_dev *pmcdev = s->private;
+   const struct pmc_bit_map *map = pmcdev->map->ltr_show_sts;
+   int index;
+
+   for (index = 0; map[index].name 

Re: [PATCH v4] remoteproc: qcom: Introduce Non-PAS ADSP PIL driver

2018-10-06 Thread Bjorn Andersson
On Mon 24 Sep 04:07 PDT 2018, Rohit kumar wrote:

> This adds Non PAS ADSP PIL driver for Qualcomm
> Technologies Inc SoCs.
> Added initial support for SDM845 with ADSP bootup and
> shutdown operation handled from Application Processor
> SubSystem(APSS).
> 
> Signed-off-by: Rohit kumar 

Sorry for missing this on the last few patches, I thought we said we
where going to name the driver qcom_q6v5_adsp. Rather than spending more
time on this I applied the patch with this change, as it does look good.

Please let me know if you have any concerns with this.

Regards,
Bjorn

> ---
> Changes since v3:
>   Addressed comments posted by Sibi
> 
> This patch is dependent on the rpmh powerdomain driver 
> https://lkml.org/lkml/2018/6/27/7
> and renaming of Hexagon v5 PAS driver 
> https://patchwork.kernel.org/patch/10601119/ .
> 
>  drivers/remoteproc/Kconfig |  14 ++
>  drivers/remoteproc/Makefile|   1 +
>  drivers/remoteproc/qcom_adsp_pil.c | 502 
> +
>  3 files changed, 517 insertions(+)
>  create mode 100644 drivers/remoteproc/qcom_adsp_pil.c
> 
> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> index 8894935..f554669 100644
> --- a/drivers/remoteproc/Kconfig
> +++ b/drivers/remoteproc/Kconfig
> @@ -140,6 +140,20 @@ config QCOM_Q6V5_WCSS
> Say y here to support the Qualcomm Peripheral Image Loader for the
> Hexagon V5 based WCSS remote processors.
>  
> +config QCOM_ADSP_PIL
> + tristate "Qualcomm Technology Inc ADSP Peripheral Image Loader"
> + depends on OF && ARCH_QCOM
> + depends on QCOM_SMEM
> + depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n
> + depends on QCOM_SYSMON || QCOM_SYSMON=n
> + select MFD_SYSCON
> + select QCOM_MDT_LOADER
> + select QCOM_Q6V5_COMMON
> + select QCOM_RPROC_COMMON
> + help
> +   Say y here to support the Peripheral Image Loader
> +   for the Qualcomm Technology Inc. ADSP remote processors.
> +
>  config QCOM_SYSMON
>   tristate "Qualcomm sysmon driver"
>   depends on RPMSG
> diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
> index 050f41a..0e1b89c 100644
> --- a/drivers/remoteproc/Makefile
> +++ b/drivers/remoteproc/Makefile
> @@ -19,6 +19,7 @@ obj-$(CONFIG_QCOM_Q6V5_COMMON)  += qcom_q6v5.o
>  obj-$(CONFIG_QCOM_Q6V5_MSS)  += qcom_q6v5_mss.o
>  obj-$(CONFIG_QCOM_Q6V5_PAS)  += qcom_q6v5_pas.o
>  obj-$(CONFIG_QCOM_Q6V5_WCSS) += qcom_q6v5_wcss.o
> +obj-$(CONFIG_QCOM_ADSP_PIL)  += qcom_adsp_pil.o
>  obj-$(CONFIG_QCOM_SYSMON)+= qcom_sysmon.o
>  obj-$(CONFIG_QCOM_WCNSS_PIL) += qcom_wcnss_pil.o
>  qcom_wcnss_pil-y += qcom_wcnss.o
> diff --git a/drivers/remoteproc/qcom_adsp_pil.c 
> b/drivers/remoteproc/qcom_adsp_pil.c
> new file mode 100644
> index 000..f2f5e56
> --- /dev/null
> +++ b/drivers/remoteproc/qcom_adsp_pil.c
> @@ -0,0 +1,502 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Qualcomm Technology Inc. ADSP Peripheral Image Loader for SDM845.
> + * Copyright (c) 2018, The Linux Foundation. All rights reserved.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "qcom_common.h"
> +#include "qcom_q6v5.h"
> +#include "remoteproc_internal.h"
> +
> +/* time out value */
> +#define ACK_TIMEOUT  1000
> +#define BOOT_FSM_TIMEOUT 1
> +/* mask values */
> +#define EVB_MASK GENMASK(27, 4)
> +/*QDSP6SS register offsets*/
> +#define RST_EVB_REG  0x10
> +#define CORE_START_REG   0x400
> +#define BOOT_CMD_REG 0x404
> +#define BOOT_STATUS_REG  0x408
> +#define RET_CFG_REG  0x1C
> +/*TCSR register offsets*/
> +#define LPASS_MASTER_IDLE_REG0x8
> +#define LPASS_HALTACK_REG0x4
> +#define LPASS_PWR_ON_REG 0x10
> +#define LPASS_HALTREQ_REG0x0
> +
> +/* 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",
> + "qdsp6ss_xo", "qdsp6ss_sleep", "qdsp6ss_core",
> +};
> +
> +struct adsp_pil_data {
> + int crash_reason_smem;
> + const char *firmware_name;
> +
> + const char *ssr_name;
> + const char *sysmon_name;
> + int ssctl_id;
> +};
> +
> +struct qcom_adsp {
> + struct device *dev;
> + struct rproc *rproc;
> +
> + struct qcom_q6v5 q6v5;
> +
> + struct clk *xo;
> +
> + int num_clks;
> + struct clk_bulk_data *clks;
> +
> + void __iomem *qdsp6ss_base;
> +
> + struct reset_control *pdc_sync_reset;
> + struct reset_control *cc_lpass_restart;
> +
> + 

[PATCH v2 1/4] platform/x86: intel_pmc_core: Show Latency Tolerance info

2018-10-06 Thread Rajneesh Bhardwaj
This adds support to show the Latency Tolerance Reporting for the IPs on
the PCH as reported by the PMC. The format shown here is raw LTR data
payload that can further be decoded as per the PCI specification.

This also fixes some minor alignment issues in the header file by
removing spaces and converting to tabs at some places.

Signed-off-by: Rajneesh Bhardwaj 
---
Changes in v2:
 * Removed IP # from map and displaying IP # while printing.
 * Other style fixes as per Andy's suggestion.

 drivers/platform/x86/intel_pmc_core.c | 73 +++
 drivers/platform/x86/intel_pmc_core.h | 56 +---
 2 files changed, 122 insertions(+), 7 deletions(-)

diff --git a/drivers/platform/x86/intel_pmc_core.c 
b/drivers/platform/x86/intel_pmc_core.c
index 2d272a3e0176..217a822a8da1 100644
--- a/drivers/platform/x86/intel_pmc_core.c
+++ b/drivers/platform/x86/intel_pmc_core.c
@@ -110,10 +110,37 @@ static const struct pmc_bit_map spt_pfear_map[] = {
{},
 };
 
+static const struct pmc_bit_map spt_ltr_show_map[] = {
+   {"LTR_SOUTHPORT_A", SPT_PMC_LTR_SPA},
+   {"LTR_SOUTHPORT_B", SPT_PMC_LTR_SPB},
+   {"LTR_SATA",SPT_PMC_LTR_SATA},
+   {"LTR_GIGABIT_ETHERNET",SPT_PMC_LTR_GBE},
+   {"LTR_XHCI",SPT_PMC_LTR_XHCI},
+   /* IP 5 is reserved */
+   {"LTR_ME",  SPT_PMC_LTR_ME},
+   /* EVA is Enterprise Value Add, doesn't really exist on PCH */
+   {"LTR_EVA", SPT_PMC_LTR_EVA},
+   {"LTR_SOUTHPORT_C", SPT_PMC_LTR_SPC},
+   {"LTR_HD_AUDIO",SPT_PMC_LTR_AZ},
+   /* IP 10 is reserved */
+   {"LTR_LPSS",SPT_PMC_LTR_LPSS},
+   {"LTR_SOUTHPORT_D", SPT_PMC_LTR_SPD},
+   {"LTR_SOUTHPORT_E", SPT_PMC_LTR_SPE},
+   {"LTR_CAMERA",  SPT_PMC_LTR_CAM},
+   {"LTR_ESPI",SPT_PMC_LTR_ESPI},
+   {"LTR_SCC", SPT_PMC_LTR_SCC},
+   {"LTR_ISH", SPT_PMC_LTR_ISH},
+   /* Below two cannot be used for LTR_IGNORE */
+   {"LTR_CURRENT_PLATFORM",SPT_PMC_LTR_CUR_PLT},
+   {"LTR_AGGREGATED_SYSTEM",   SPT_PMC_LTR_CUR_ASLT},
+   {}
+};
+
 static const struct pmc_reg_map spt_reg_map = {
.pfear_sts = spt_pfear_map,
.mphy_sts = spt_mphy_map,
.pll_sts = spt_pll_map,
+   .ltr_show_sts = spt_ltr_show_map,
.slp_s0_offset = SPT_PMC_SLP_S0_RES_COUNTER_OFFSET,
.ltr_ignore_offset = SPT_PMC_LTR_IGNORE_OFFSET,
.regmap_length = SPT_PMC_MMIO_REG_LEN,
@@ -252,10 +279,39 @@ static const struct pmc_bit_map *cnp_slps0_dbg_maps[] = {
NULL,
 };
 
+static const struct pmc_bit_map cnp_ltr_show_map[] = {
+   {"LTR_SOUTHPORT_A", CNP_PMC_LTR_SPA},
+   {"LTR_SOUTHPORT_B", CNP_PMC_LTR_SPB},
+   {"LTR_SATA",CNP_PMC_LTR_SATA},
+   {"LTR_GIGABIT_ETHERNET",CNP_PMC_LTR_GBE},
+   {"LTR_XHCI",CNP_PMC_LTR_XHCI},
+   /* IP 5 is reserved */
+   {"LTR_ME",  CNP_PMC_LTR_ME},
+   /* EVA is Enterprise Value Add, doesn't really exist on PCH */
+   {"LTR_EVA", CNP_PMC_LTR_EVA},
+   {"LTR_SOUTHPORT_C", CNP_PMC_LTR_SPC},
+   {"LTR_HD_AUDIO",CNP_PMC_LTR_AZ},
+   {"LTR_CNV", CNP_PMC_LTR_CNV},
+   {"LTR_LPSS",CNP_PMC_LTR_LPSS},
+   {"LTR_SOUTHPORT_D", CNP_PMC_LTR_SPD},
+   {"LTR_SOUTHPORT_E", CNP_PMC_LTR_SPE},
+   {"LTR_CAMERA",  CNP_PMC_LTR_CAM},
+   {"LTR_ESPI",CNP_PMC_LTR_ESPI},
+   {"LTR_SCC", CNP_PMC_LTR_SCC},
+   {"LTR_ISH", CNP_PMC_LTR_ISH},
+   {"LTR_UFSX2",   CNP_PMC_LTR_UFSX2},
+   {"LTR_EMMC",CNP_PMC_LTR_EMMC},
+   /* Below two cannot be used for LTR_IGNORE */
+   {"LTR_CURRENT_PLATFORM",CNP_PMC_LTR_CUR_PLT},
+   {"LTR_AGGREGATED_SYSTEM",   CNP_PMC_LTR_CUR_ASLT},
+   {}
+};
+
 static const struct pmc_reg_map cnp_reg_map = {
.pfear_sts = cnp_pfear_map,
.slp_s0_offset = CNP_PMC_SLP_S0_RES_COUNTER_OFFSET,
.slps0_dbg_maps = cnp_slps0_dbg_maps,
+   .ltr_show_sts = cnp_ltr_show_map,
.slps0_dbg_offset = CNP_PMC_SLPS0_DBG_OFFSET,
.ltr_ignore_offset = CNP_PMC_LTR_IGNORE_OFFSET,
.regmap_length = CNP_PMC_MMIO_REG_LEN,
@@ -592,6 +648,21 @@ static int pmc_core_slps0_dbg_show(struct seq_file *s, 
void *unused)
 }
 DEFINE_SHOW_ATTRIBUTE(pmc_core_slps0_dbg);
 
+static int pmc_core_ltr_show(struct seq_file *s, void *unused)
+{
+   struct pmc_dev *pmcdev = s->private;
+   const struct pmc_bit_map *map = pmcdev->map->ltr_show_sts;
+   int index;
+
+   for (index = 0; map[index].name 

Re: [PATCH v4] remoteproc: qcom: Introduce Non-PAS ADSP PIL driver

2018-10-06 Thread Bjorn Andersson
On Mon 24 Sep 04:07 PDT 2018, Rohit kumar wrote:

> This adds Non PAS ADSP PIL driver for Qualcomm
> Technologies Inc SoCs.
> Added initial support for SDM845 with ADSP bootup and
> shutdown operation handled from Application Processor
> SubSystem(APSS).
> 
> Signed-off-by: Rohit kumar 

Sorry for missing this on the last few patches, I thought we said we
where going to name the driver qcom_q6v5_adsp. Rather than spending more
time on this I applied the patch with this change, as it does look good.

Please let me know if you have any concerns with this.

Regards,
Bjorn

> ---
> Changes since v3:
>   Addressed comments posted by Sibi
> 
> This patch is dependent on the rpmh powerdomain driver 
> https://lkml.org/lkml/2018/6/27/7
> and renaming of Hexagon v5 PAS driver 
> https://patchwork.kernel.org/patch/10601119/ .
> 
>  drivers/remoteproc/Kconfig |  14 ++
>  drivers/remoteproc/Makefile|   1 +
>  drivers/remoteproc/qcom_adsp_pil.c | 502 
> +
>  3 files changed, 517 insertions(+)
>  create mode 100644 drivers/remoteproc/qcom_adsp_pil.c
> 
> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> index 8894935..f554669 100644
> --- a/drivers/remoteproc/Kconfig
> +++ b/drivers/remoteproc/Kconfig
> @@ -140,6 +140,20 @@ config QCOM_Q6V5_WCSS
> Say y here to support the Qualcomm Peripheral Image Loader for the
> Hexagon V5 based WCSS remote processors.
>  
> +config QCOM_ADSP_PIL
> + tristate "Qualcomm Technology Inc ADSP Peripheral Image Loader"
> + depends on OF && ARCH_QCOM
> + depends on QCOM_SMEM
> + depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n
> + depends on QCOM_SYSMON || QCOM_SYSMON=n
> + select MFD_SYSCON
> + select QCOM_MDT_LOADER
> + select QCOM_Q6V5_COMMON
> + select QCOM_RPROC_COMMON
> + help
> +   Say y here to support the Peripheral Image Loader
> +   for the Qualcomm Technology Inc. ADSP remote processors.
> +
>  config QCOM_SYSMON
>   tristate "Qualcomm sysmon driver"
>   depends on RPMSG
> diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
> index 050f41a..0e1b89c 100644
> --- a/drivers/remoteproc/Makefile
> +++ b/drivers/remoteproc/Makefile
> @@ -19,6 +19,7 @@ obj-$(CONFIG_QCOM_Q6V5_COMMON)  += qcom_q6v5.o
>  obj-$(CONFIG_QCOM_Q6V5_MSS)  += qcom_q6v5_mss.o
>  obj-$(CONFIG_QCOM_Q6V5_PAS)  += qcom_q6v5_pas.o
>  obj-$(CONFIG_QCOM_Q6V5_WCSS) += qcom_q6v5_wcss.o
> +obj-$(CONFIG_QCOM_ADSP_PIL)  += qcom_adsp_pil.o
>  obj-$(CONFIG_QCOM_SYSMON)+= qcom_sysmon.o
>  obj-$(CONFIG_QCOM_WCNSS_PIL) += qcom_wcnss_pil.o
>  qcom_wcnss_pil-y += qcom_wcnss.o
> diff --git a/drivers/remoteproc/qcom_adsp_pil.c 
> b/drivers/remoteproc/qcom_adsp_pil.c
> new file mode 100644
> index 000..f2f5e56
> --- /dev/null
> +++ b/drivers/remoteproc/qcom_adsp_pil.c
> @@ -0,0 +1,502 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Qualcomm Technology Inc. ADSP Peripheral Image Loader for SDM845.
> + * Copyright (c) 2018, The Linux Foundation. All rights reserved.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "qcom_common.h"
> +#include "qcom_q6v5.h"
> +#include "remoteproc_internal.h"
> +
> +/* time out value */
> +#define ACK_TIMEOUT  1000
> +#define BOOT_FSM_TIMEOUT 1
> +/* mask values */
> +#define EVB_MASK GENMASK(27, 4)
> +/*QDSP6SS register offsets*/
> +#define RST_EVB_REG  0x10
> +#define CORE_START_REG   0x400
> +#define BOOT_CMD_REG 0x404
> +#define BOOT_STATUS_REG  0x408
> +#define RET_CFG_REG  0x1C
> +/*TCSR register offsets*/
> +#define LPASS_MASTER_IDLE_REG0x8
> +#define LPASS_HALTACK_REG0x4
> +#define LPASS_PWR_ON_REG 0x10
> +#define LPASS_HALTREQ_REG0x0
> +
> +/* 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",
> + "qdsp6ss_xo", "qdsp6ss_sleep", "qdsp6ss_core",
> +};
> +
> +struct adsp_pil_data {
> + int crash_reason_smem;
> + const char *firmware_name;
> +
> + const char *ssr_name;
> + const char *sysmon_name;
> + int ssctl_id;
> +};
> +
> +struct qcom_adsp {
> + struct device *dev;
> + struct rproc *rproc;
> +
> + struct qcom_q6v5 q6v5;
> +
> + struct clk *xo;
> +
> + int num_clks;
> + struct clk_bulk_data *clks;
> +
> + void __iomem *qdsp6ss_base;
> +
> + struct reset_control *pdc_sync_reset;
> + struct reset_control *cc_lpass_restart;
> +
> + 

[PATCH v2 4/4] platform/x86: intel_telemetry: report debugfs failure

2018-10-06 Thread Rajneesh Bhardwaj
On some Goldmont based systems such as ASRock J3455M the BIOS may not
enable the IPC1 device that provides access to the PMC and PUNIT. In
such scenarios, the IOSS and PSS resources from the platform device can
not be obtained and result in a invalid telemetry_plt_config which is an
internal data structure that holds platform config and is maintained by
the telemetry platform driver.

This is also applicable to the platforms where the BIOS supports IPC1
device under debug configurations but IPC1 is disabled by user or the
policy.

This change allows user to know the reason for not seeing entries under
/sys/kernel/debug/telemetry/* when there is no apparent failure at boot.

Cc: Matt Turner 
Cc: Len Brown 
Cc: Souvik Kumar Chakravarty 
Cc: Kuppuswamy Sathyanarayanan 

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=198779
Acked-by: Matt Turner 
Signed-off-by: Rajneesh Bhardwaj 
---
Changes in v2:
 * Removed print and out label both as suggested by Andy.
 * changed to pr_info.
 * Other minor style fixes.


 drivers/platform/x86/intel_telemetry_debugfs.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/intel_telemetry_debugfs.c 
b/drivers/platform/x86/intel_telemetry_debugfs.c
index ffd0474b0531..1423fa8710fd 100644
--- a/drivers/platform/x86/intel_telemetry_debugfs.c
+++ b/drivers/platform/x86/intel_telemetry_debugfs.c
@@ -951,12 +951,16 @@ static int __init telemetry_debugfs_init(void)
debugfs_conf = (struct telemetry_debugfs_conf *)id->driver_data;
 
err = telemetry_pltconfig_valid();
-   if (err < 0)
+   if (err < 0) {
+   pr_info("Invalid pltconfig, ensure IPC1 device is enabled in 
BIOS\n");
return -ENODEV;
+   }
 
err = telemetry_debugfs_check_evts();
-   if (err < 0)
+   if (err < 0) {
+   pr_info("telemetry_debugfs_check_evts failed\n");
return -EINVAL;
+   }
 
register_pm_notifier(_notifier);
 
-- 
2.17.1



[PATCH v2 3/4] platform/x86: intel_pmc_core: Decode Snoop / Non Snoop LTR

2018-10-06 Thread Rajneesh Bhardwaj
The LTR values follow PCIE LTR encoding format and can be decoded as per
https://pcisig.com/sites/default/files/specification_documents/ECN_LatencyTolnReporting_14Aug08.pdf

This adds support to translate the raw LTR values as read from the PMC
to meaningful values in nanosecond units of time.

Signed-off-by: Rajneesh Bhardwaj 
---
Changes in v2:
 * Get rid of union and bitfields to decode LTR and use FIELD_GET macro
 * Change get_ltr_scale to convert_ltr_scale.
 * Other style fixes and misc. improvements suggested by Andy for v1.

 drivers/platform/x86/intel_pmc_core.c | 64 +--
 drivers/platform/x86/intel_pmc_core.h |  5 +++
 2 files changed, 66 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/intel_pmc_core.c 
b/drivers/platform/x86/intel_pmc_core.c
index c616cfedf2be..fbcab53456f3 100644
--- a/drivers/platform/x86/intel_pmc_core.c
+++ b/drivers/platform/x86/intel_pmc_core.c
@@ -21,6 +21,7 @@
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -650,16 +651,73 @@ static int pmc_core_slps0_dbg_show(struct seq_file *s, 
void *unused)
 }
 DEFINE_SHOW_ATTRIBUTE(pmc_core_slps0_dbg);
 
+static u32 convert_ltr_scale(u32 val)
+{
+   u32 scale = 0;
+   /*
+* As per PCIE specification supporting document
+* ECN_LatencyTolnReporting_14Aug08.pdf the Latency
+* Tolerance Reporting data payload is encoded in a
+* 3 bit scale and 10 bit value fields. Values are
+* multiplied by the indicated scale to yield an absolute time
+* value, expressible in a range from 1 nanosecond to
+* 2^25*(2^10-1) = 34,326,183,936 nanoseconds.
+*
+* scale encoding is as follows:
+*
+* --
+* |scale factor|   Multiplier (ns) |
+* --
+* |0   |   1   |
+* |1   |   32  |
+* |2   |   1024|
+* |3   |   32768   |
+* |4   |   1048576 |
+* |5   |   33554432|
+* |6   |   Invalid |
+* |7   |   Invalid |
+* --
+*/
+   if (val > 5)
+   pr_warn("Invalid LTR scale factor.\n");
+   else
+   scale = 1U << (5 * (val));
+
+   return scale;
+}
+
 static int pmc_core_ltr_show(struct seq_file *s, void *unused)
 {
struct pmc_dev *pmcdev = s->private;
const struct pmc_bit_map *map = pmcdev->map->ltr_show_sts;
+   u64 decoded_snoop_ltr, decoded_non_snoop_ltr;
+   u32 ltr_raw_data, scale, val;
+   u16 snoop_ltr, nonsnoop_ltr;
int index;
 
for (index = 0; map[index].name ; index++) {
-   seq_printf(s, "IP %-2d :%-32s\tRAW LTR: 0x%x\n", index,
-  map[index].name,
-  pmc_core_reg_read(pmcdev, map[index].bit_mask));
+   decoded_snoop_ltr = decoded_non_snoop_ltr = 0;
+   ltr_raw_data = pmc_core_reg_read(pmcdev,
+map[index].bit_mask);
+   snoop_ltr = ltr_raw_data & ~MTPMC_MASK;
+   nonsnoop_ltr = (ltr_raw_data >> 0x10) & ~MTPMC_MASK;
+
+   if (FIELD_GET(LTR_REQ_NONSNOOP, ltr_raw_data)) {
+   scale = FIELD_GET(LTR_DECODED_SCALE, nonsnoop_ltr);
+   val = FIELD_GET(LTR_DECODED_VAL, nonsnoop_ltr);
+   decoded_non_snoop_ltr = val * convert_ltr_scale(scale);
+   }
+
+   if (FIELD_GET(LTR_REQ_SNOOP, ltr_raw_data)) {
+   scale = FIELD_GET(LTR_DECODED_SCALE, snoop_ltr);
+   val = FIELD_GET(LTR_DECODED_VAL, snoop_ltr);
+   decoded_snoop_ltr = val * convert_ltr_scale(scale);
+   }
+
+   seq_printf(s, "IP %-2d :%-24s\tRaw LTR: 0x%-16x\t Non-Snoop LTR 
(ns): %-16llu\t Snoop LTR (ns): %-16llu\n",
+  index, map[index].name, ltr_raw_data,
+  decoded_non_snoop_ltr,
+  decoded_snoop_ltr);
}
return 0;
 }
diff --git a/drivers/platform/x86/intel_pmc_core.h 
b/drivers/platform/x86/intel_pmc_core.h
index 7f8181057ec8..cc49cd4c86e9 100644
--- a/drivers/platform/x86/intel_pmc_core.h
+++ b/drivers/platform/x86/intel_pmc_core.h
@@ -177,6 +177,11 @@ enum ppfear_regs {
 #define CNP_PMC_LTR_EMMC   0x1BF4
 #define CNP_PMC_LTR_UFSX2  0x1BF8
 
+#define LTR_REQ_NONSNOOP   BIT(31)
+#define LTR_REQ_SNOOP  BIT(15)
+#define LTR_DECODED_VALGENMASK(9, 

[PATCH v2 4/4] platform/x86: intel_telemetry: report debugfs failure

2018-10-06 Thread Rajneesh Bhardwaj
On some Goldmont based systems such as ASRock J3455M the BIOS may not
enable the IPC1 device that provides access to the PMC and PUNIT. In
such scenarios, the IOSS and PSS resources from the platform device can
not be obtained and result in a invalid telemetry_plt_config which is an
internal data structure that holds platform config and is maintained by
the telemetry platform driver.

This is also applicable to the platforms where the BIOS supports IPC1
device under debug configurations but IPC1 is disabled by user or the
policy.

This change allows user to know the reason for not seeing entries under
/sys/kernel/debug/telemetry/* when there is no apparent failure at boot.

Cc: Matt Turner 
Cc: Len Brown 
Cc: Souvik Kumar Chakravarty 
Cc: Kuppuswamy Sathyanarayanan 

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=198779
Acked-by: Matt Turner 
Signed-off-by: Rajneesh Bhardwaj 
---
Changes in v2:
 * Removed print and out label both as suggested by Andy.
 * changed to pr_info.
 * Other minor style fixes.


 drivers/platform/x86/intel_telemetry_debugfs.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/intel_telemetry_debugfs.c 
b/drivers/platform/x86/intel_telemetry_debugfs.c
index ffd0474b0531..1423fa8710fd 100644
--- a/drivers/platform/x86/intel_telemetry_debugfs.c
+++ b/drivers/platform/x86/intel_telemetry_debugfs.c
@@ -951,12 +951,16 @@ static int __init telemetry_debugfs_init(void)
debugfs_conf = (struct telemetry_debugfs_conf *)id->driver_data;
 
err = telemetry_pltconfig_valid();
-   if (err < 0)
+   if (err < 0) {
+   pr_info("Invalid pltconfig, ensure IPC1 device is enabled in 
BIOS\n");
return -ENODEV;
+   }
 
err = telemetry_debugfs_check_evts();
-   if (err < 0)
+   if (err < 0) {
+   pr_info("telemetry_debugfs_check_evts failed\n");
return -EINVAL;
+   }
 
register_pm_notifier(_notifier);
 
-- 
2.17.1



[PATCH v2 3/4] platform/x86: intel_pmc_core: Decode Snoop / Non Snoop LTR

2018-10-06 Thread Rajneesh Bhardwaj
The LTR values follow PCIE LTR encoding format and can be decoded as per
https://pcisig.com/sites/default/files/specification_documents/ECN_LatencyTolnReporting_14Aug08.pdf

This adds support to translate the raw LTR values as read from the PMC
to meaningful values in nanosecond units of time.

Signed-off-by: Rajneesh Bhardwaj 
---
Changes in v2:
 * Get rid of union and bitfields to decode LTR and use FIELD_GET macro
 * Change get_ltr_scale to convert_ltr_scale.
 * Other style fixes and misc. improvements suggested by Andy for v1.

 drivers/platform/x86/intel_pmc_core.c | 64 +--
 drivers/platform/x86/intel_pmc_core.h |  5 +++
 2 files changed, 66 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/intel_pmc_core.c 
b/drivers/platform/x86/intel_pmc_core.c
index c616cfedf2be..fbcab53456f3 100644
--- a/drivers/platform/x86/intel_pmc_core.c
+++ b/drivers/platform/x86/intel_pmc_core.c
@@ -21,6 +21,7 @@
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -650,16 +651,73 @@ static int pmc_core_slps0_dbg_show(struct seq_file *s, 
void *unused)
 }
 DEFINE_SHOW_ATTRIBUTE(pmc_core_slps0_dbg);
 
+static u32 convert_ltr_scale(u32 val)
+{
+   u32 scale = 0;
+   /*
+* As per PCIE specification supporting document
+* ECN_LatencyTolnReporting_14Aug08.pdf the Latency
+* Tolerance Reporting data payload is encoded in a
+* 3 bit scale and 10 bit value fields. Values are
+* multiplied by the indicated scale to yield an absolute time
+* value, expressible in a range from 1 nanosecond to
+* 2^25*(2^10-1) = 34,326,183,936 nanoseconds.
+*
+* scale encoding is as follows:
+*
+* --
+* |scale factor|   Multiplier (ns) |
+* --
+* |0   |   1   |
+* |1   |   32  |
+* |2   |   1024|
+* |3   |   32768   |
+* |4   |   1048576 |
+* |5   |   33554432|
+* |6   |   Invalid |
+* |7   |   Invalid |
+* --
+*/
+   if (val > 5)
+   pr_warn("Invalid LTR scale factor.\n");
+   else
+   scale = 1U << (5 * (val));
+
+   return scale;
+}
+
 static int pmc_core_ltr_show(struct seq_file *s, void *unused)
 {
struct pmc_dev *pmcdev = s->private;
const struct pmc_bit_map *map = pmcdev->map->ltr_show_sts;
+   u64 decoded_snoop_ltr, decoded_non_snoop_ltr;
+   u32 ltr_raw_data, scale, val;
+   u16 snoop_ltr, nonsnoop_ltr;
int index;
 
for (index = 0; map[index].name ; index++) {
-   seq_printf(s, "IP %-2d :%-32s\tRAW LTR: 0x%x\n", index,
-  map[index].name,
-  pmc_core_reg_read(pmcdev, map[index].bit_mask));
+   decoded_snoop_ltr = decoded_non_snoop_ltr = 0;
+   ltr_raw_data = pmc_core_reg_read(pmcdev,
+map[index].bit_mask);
+   snoop_ltr = ltr_raw_data & ~MTPMC_MASK;
+   nonsnoop_ltr = (ltr_raw_data >> 0x10) & ~MTPMC_MASK;
+
+   if (FIELD_GET(LTR_REQ_NONSNOOP, ltr_raw_data)) {
+   scale = FIELD_GET(LTR_DECODED_SCALE, nonsnoop_ltr);
+   val = FIELD_GET(LTR_DECODED_VAL, nonsnoop_ltr);
+   decoded_non_snoop_ltr = val * convert_ltr_scale(scale);
+   }
+
+   if (FIELD_GET(LTR_REQ_SNOOP, ltr_raw_data)) {
+   scale = FIELD_GET(LTR_DECODED_SCALE, snoop_ltr);
+   val = FIELD_GET(LTR_DECODED_VAL, snoop_ltr);
+   decoded_snoop_ltr = val * convert_ltr_scale(scale);
+   }
+
+   seq_printf(s, "IP %-2d :%-24s\tRaw LTR: 0x%-16x\t Non-Snoop LTR 
(ns): %-16llu\t Snoop LTR (ns): %-16llu\n",
+  index, map[index].name, ltr_raw_data,
+  decoded_non_snoop_ltr,
+  decoded_snoop_ltr);
}
return 0;
 }
diff --git a/drivers/platform/x86/intel_pmc_core.h 
b/drivers/platform/x86/intel_pmc_core.h
index 7f8181057ec8..cc49cd4c86e9 100644
--- a/drivers/platform/x86/intel_pmc_core.h
+++ b/drivers/platform/x86/intel_pmc_core.h
@@ -177,6 +177,11 @@ enum ppfear_regs {
 #define CNP_PMC_LTR_EMMC   0x1BF4
 #define CNP_PMC_LTR_UFSX2  0x1BF8
 
+#define LTR_REQ_NONSNOOP   BIT(31)
+#define LTR_REQ_SNOOP  BIT(15)
+#define LTR_DECODED_VALGENMASK(9, 

[PATCH v2 2/4] platform/x86: intel_pmc_core: Fix LTR IGNORE Max offset

2018-10-06 Thread Rajneesh Bhardwaj
Cannonlake PCH allows us to ignore LTR from more IPs than Sunrisepoint
PCH so make the LTR ignore platform specific.

Signed-off-by: Rajneesh Bhardwaj 
---
 drivers/platform/x86/intel_pmc_core.c | 4 +++-
 drivers/platform/x86/intel_pmc_core.h | 4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/intel_pmc_core.c 
b/drivers/platform/x86/intel_pmc_core.c
index 217a822a8da1..c616cfedf2be 100644
--- a/drivers/platform/x86/intel_pmc_core.c
+++ b/drivers/platform/x86/intel_pmc_core.c
@@ -148,6 +148,7 @@ static const struct pmc_reg_map spt_reg_map = {
.ppfear_buckets = SPT_PPFEAR_NUM_ENTRIES,
.pm_cfg_offset = SPT_PMC_PM_CFG_OFFSET,
.pm_read_disable_bit = SPT_PMC_READ_DISABLE_BIT,
+   .ltr_ignore_max = SPT_NUM_IP_IGN_ALLOWED,
 };
 
 /* Cannonlake: PGD PFET Enable Ack Status Register(s) bitmap */
@@ -319,6 +320,7 @@ static const struct pmc_reg_map cnp_reg_map = {
.ppfear_buckets = CNP_PPFEAR_NUM_ENTRIES,
.pm_cfg_offset = CNP_PMC_PM_CFG_OFFSET,
.pm_read_disable_bit = CNP_PMC_READ_DISABLE_BIT,
+   .ltr_ignore_max = CNP_NUM_IP_IGN_ALLOWED,
 };
 
 static inline u8 pmc_core_reg_read_byte(struct pmc_dev *pmcdev, int offset)
@@ -565,7 +567,7 @@ static ssize_t pmc_core_ltr_ignore_write(struct file *file, 
const char __user
goto out_unlock;
}
 
-   if (val > NUM_IP_IGN_ALLOWED) {
+   if (val > map->ltr_ignore_max) {
err = -EINVAL;
goto out_unlock;
}
diff --git a/drivers/platform/x86/intel_pmc_core.h 
b/drivers/platform/x86/intel_pmc_core.h
index 7a00436e337d..7f8181057ec8 100644
--- a/drivers/platform/x86/intel_pmc_core.h
+++ b/drivers/platform/x86/intel_pmc_core.h
@@ -44,7 +44,7 @@
 #define SPT_PMC_READ_DISABLE_BIT   0x16
 #define SPT_PMC_MSG_FULL_STS_BIT   0x18
 #define NUM_RETRIES100
-#define NUM_IP_IGN_ALLOWED 17
+#define SPT_NUM_IP_IGN_ALLOWED 17
 
 #define SPT_PMC_LTR_CUR_PLT0x350
 #define SPT_PMC_LTR_CUR_ASLT   0x354
@@ -154,6 +154,7 @@ enum ppfear_regs {
 #define CNP_PPFEAR_NUM_ENTRIES 8
 #define CNP_PMC_READ_DISABLE_BIT   22
 #define CNP_PMC_LATCH_SLPS0_EVENTS BIT(31)
+#define CNP_NUM_IP_IGN_ALLOWED 19
 #define CNP_PMC_LTR_CUR_PLT0x1B50
 #define CNP_PMC_LTR_CUR_ASLT   0x1B54
 #define CNP_PMC_LTR_SPA0x1B60
@@ -216,6 +217,7 @@ struct pmc_reg_map {
const u32 pm_cfg_offset;
const int pm_read_disable_bit;
const u32 slps0_dbg_offset;
+   const u32 ltr_ignore_max;
 };
 
 /**
-- 
2.17.1



[PATCH v2 2/4] platform/x86: intel_pmc_core: Fix LTR IGNORE Max offset

2018-10-06 Thread Rajneesh Bhardwaj
Cannonlake PCH allows us to ignore LTR from more IPs than Sunrisepoint
PCH so make the LTR ignore platform specific.

Signed-off-by: Rajneesh Bhardwaj 
---
 drivers/platform/x86/intel_pmc_core.c | 4 +++-
 drivers/platform/x86/intel_pmc_core.h | 4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/intel_pmc_core.c 
b/drivers/platform/x86/intel_pmc_core.c
index 217a822a8da1..c616cfedf2be 100644
--- a/drivers/platform/x86/intel_pmc_core.c
+++ b/drivers/platform/x86/intel_pmc_core.c
@@ -148,6 +148,7 @@ static const struct pmc_reg_map spt_reg_map = {
.ppfear_buckets = SPT_PPFEAR_NUM_ENTRIES,
.pm_cfg_offset = SPT_PMC_PM_CFG_OFFSET,
.pm_read_disable_bit = SPT_PMC_READ_DISABLE_BIT,
+   .ltr_ignore_max = SPT_NUM_IP_IGN_ALLOWED,
 };
 
 /* Cannonlake: PGD PFET Enable Ack Status Register(s) bitmap */
@@ -319,6 +320,7 @@ static const struct pmc_reg_map cnp_reg_map = {
.ppfear_buckets = CNP_PPFEAR_NUM_ENTRIES,
.pm_cfg_offset = CNP_PMC_PM_CFG_OFFSET,
.pm_read_disable_bit = CNP_PMC_READ_DISABLE_BIT,
+   .ltr_ignore_max = CNP_NUM_IP_IGN_ALLOWED,
 };
 
 static inline u8 pmc_core_reg_read_byte(struct pmc_dev *pmcdev, int offset)
@@ -565,7 +567,7 @@ static ssize_t pmc_core_ltr_ignore_write(struct file *file, 
const char __user
goto out_unlock;
}
 
-   if (val > NUM_IP_IGN_ALLOWED) {
+   if (val > map->ltr_ignore_max) {
err = -EINVAL;
goto out_unlock;
}
diff --git a/drivers/platform/x86/intel_pmc_core.h 
b/drivers/platform/x86/intel_pmc_core.h
index 7a00436e337d..7f8181057ec8 100644
--- a/drivers/platform/x86/intel_pmc_core.h
+++ b/drivers/platform/x86/intel_pmc_core.h
@@ -44,7 +44,7 @@
 #define SPT_PMC_READ_DISABLE_BIT   0x16
 #define SPT_PMC_MSG_FULL_STS_BIT   0x18
 #define NUM_RETRIES100
-#define NUM_IP_IGN_ALLOWED 17
+#define SPT_NUM_IP_IGN_ALLOWED 17
 
 #define SPT_PMC_LTR_CUR_PLT0x350
 #define SPT_PMC_LTR_CUR_ASLT   0x354
@@ -154,6 +154,7 @@ enum ppfear_regs {
 #define CNP_PPFEAR_NUM_ENTRIES 8
 #define CNP_PMC_READ_DISABLE_BIT   22
 #define CNP_PMC_LATCH_SLPS0_EVENTS BIT(31)
+#define CNP_NUM_IP_IGN_ALLOWED 19
 #define CNP_PMC_LTR_CUR_PLT0x1B50
 #define CNP_PMC_LTR_CUR_ASLT   0x1B54
 #define CNP_PMC_LTR_SPA0x1B60
@@ -216,6 +217,7 @@ struct pmc_reg_map {
const u32 pm_cfg_offset;
const int pm_read_disable_bit;
const u32 slps0_dbg_offset;
+   const u32 ltr_ignore_max;
 };
 
 /**
-- 
2.17.1



Re: [PATCH v4] dt-binding: remoteproc: Add QTI ADSP PIL bindings

2018-10-06 Thread Bjorn Andersson
On Mon 10 Sep 20:54 PDT 2018, Rohit kumar wrote:
> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,adsp-pil.txt 
> b/Documentation/devicetree/bindings/remoteproc/qcom,adsp-pil.txt
[..]
> += EXAMPLE
> +The following example describes the resources needed to boot control the
> +ADSP, as it is found on SDM845 boards.
> + adsp-pil {

Updated this to remoteproc@1730 and applied the patch to rproc-next.

Regards,
Bjorn

> + compatible = "qcom,sdm845-adsp-pil";
> +
> + reg = <0x1730 0x40c>;
> +
> + interrupts-extended = < 0 162 IRQ_TYPE_EDGE_RISING>,
> + <_smp2p_in 0 IRQ_TYPE_EDGE_RISING>,
> + <_smp2p_in 1 IRQ_TYPE_EDGE_RISING>,
> + <_smp2p_in 2 IRQ_TYPE_EDGE_RISING>,
> + <_smp2p_in 3 IRQ_TYPE_EDGE_RISING>;
> + interrupt-names = "wdog", "fatal", "ready",
> + "handover", "stop-ack";
> +
> + 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",
> + "lpass_ahbs_aon_cbcr",
> + "lpass_ahbm_aon_cbcr", "qdsp6ss_xo",
> + "qdsp6ss_sleep", "qdsp6ss_core";
> +
> + power-domains = < SDM845_CX>;
> +
> + resets = <_reset PDC_AUDIO_SYNC_RESET>,
> +  <_reset AOSS_CC_LPASS_RESTART>;
> + reset-names = "pdc_sync", "cc_lpass";
> +
> + qcom,halt-regs = <_mutex_regs 0x22000>;
> +
> + memory-region = <_adsp_mem>;
> +
> + qcom,smem-states = <_smp2p_out 0>;
> + qcom,smem-state-names = "stop";
> + };


Re: [PATCH v4] dt-binding: remoteproc: Add QTI ADSP PIL bindings

2018-10-06 Thread Bjorn Andersson
On Mon 10 Sep 20:54 PDT 2018, Rohit kumar wrote:
> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,adsp-pil.txt 
> b/Documentation/devicetree/bindings/remoteproc/qcom,adsp-pil.txt
[..]
> += EXAMPLE
> +The following example describes the resources needed to boot control the
> +ADSP, as it is found on SDM845 boards.
> + adsp-pil {

Updated this to remoteproc@1730 and applied the patch to rproc-next.

Regards,
Bjorn

> + compatible = "qcom,sdm845-adsp-pil";
> +
> + reg = <0x1730 0x40c>;
> +
> + interrupts-extended = < 0 162 IRQ_TYPE_EDGE_RISING>,
> + <_smp2p_in 0 IRQ_TYPE_EDGE_RISING>,
> + <_smp2p_in 1 IRQ_TYPE_EDGE_RISING>,
> + <_smp2p_in 2 IRQ_TYPE_EDGE_RISING>,
> + <_smp2p_in 3 IRQ_TYPE_EDGE_RISING>;
> + interrupt-names = "wdog", "fatal", "ready",
> + "handover", "stop-ack";
> +
> + 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",
> + "lpass_ahbs_aon_cbcr",
> + "lpass_ahbm_aon_cbcr", "qdsp6ss_xo",
> + "qdsp6ss_sleep", "qdsp6ss_core";
> +
> + power-domains = < SDM845_CX>;
> +
> + resets = <_reset PDC_AUDIO_SYNC_RESET>,
> +  <_reset AOSS_CC_LPASS_RESTART>;
> + reset-names = "pdc_sync", "cc_lpass";
> +
> + qcom,halt-regs = <_mutex_regs 0x22000>;
> +
> + memory-region = <_adsp_mem>;
> +
> + qcom,smem-states = <_smp2p_out 0>;
> + qcom,smem-state-names = "stop";
> + };


Re: [PATCH] remoteproc: qcom: q6v5: Propagate EPROBE_DEFER

2018-10-06 Thread Bjorn Andersson
On Mon 24 Sep 23:50 PDT 2018, Sibi Sankar wrote:

> On 2018-09-20 07:21, Bjorn Andersson wrote:
> > In the case that the interrupts fail to result because of the
> > interrupt-controller not yet being registered the
> > platform_get_irq_byname() call will fail with -EPROBE_DEFER, but passing
> > this into devm_request_threaded_irq() will result in -EINVAL being
> > returned, the driver is therefor not reprobed later.
> > 
> 
> The patch looks fine.
> Reviewed-by: Sibi Sankar 
> 

Thanks for the review Sibi, applied the patch.

Regards,
Bjorn

> > Fixes: 3b415c8fb263 ("remoteproc: q6v5: Extract common resource
> > handling")
> > Cc: sta...@vger.kernel.org
> > Signed-off-by: Bjorn Andersson 
> > ---
> >  drivers/remoteproc/qcom_q6v5.c | 12 
> >  1 file changed, 12 insertions(+)
> > 
> > diff --git a/drivers/remoteproc/qcom_q6v5.c
> > b/drivers/remoteproc/qcom_q6v5.c
> > index 61a760ee4aac..e9ab90c19304 100644
> > --- a/drivers/remoteproc/qcom_q6v5.c
> > +++ b/drivers/remoteproc/qcom_q6v5.c
> > @@ -198,6 +198,9 @@ int qcom_q6v5_init(struct qcom_q6v5 *q6v5, struct
> > platform_device *pdev,
> > }
> > 
> > q6v5->fatal_irq = platform_get_irq_byname(pdev, "fatal");
> > +   if (q6v5->fatal_irq == -EPROBE_DEFER)
> > +   return -EPROBE_DEFER;
> > +
> > ret = devm_request_threaded_irq(>dev, q6v5->fatal_irq,
> > NULL, q6v5_fatal_interrupt,
> > IRQF_TRIGGER_RISING | IRQF_ONESHOT,
> > @@ -208,6 +211,9 @@ int qcom_q6v5_init(struct qcom_q6v5 *q6v5, struct
> > platform_device *pdev,
> > }
> > 
> > q6v5->ready_irq = platform_get_irq_byname(pdev, "ready");
> > +   if (q6v5->ready_irq == -EPROBE_DEFER)
> > +   return -EPROBE_DEFER;
> > +
> > ret = devm_request_threaded_irq(>dev, q6v5->ready_irq,
> > NULL, q6v5_ready_interrupt,
> > IRQF_TRIGGER_RISING | IRQF_ONESHOT,
> > @@ -218,6 +224,9 @@ int qcom_q6v5_init(struct qcom_q6v5 *q6v5, struct
> > platform_device *pdev,
> > }
> > 
> > q6v5->handover_irq = platform_get_irq_byname(pdev, "handover");
> > +   if (q6v5->handover_irq == -EPROBE_DEFER)
> > +   return -EPROBE_DEFER;
> > +
> > ret = devm_request_threaded_irq(>dev, q6v5->handover_irq,
> > NULL, q6v5_handover_interrupt,
> > IRQF_TRIGGER_RISING | IRQF_ONESHOT,
> > @@ -229,6 +238,9 @@ int qcom_q6v5_init(struct qcom_q6v5 *q6v5, struct
> > platform_device *pdev,
> > disable_irq(q6v5->handover_irq);
> > 
> > q6v5->stop_irq = platform_get_irq_byname(pdev, "stop-ack");
> > +   if (q6v5->stop_irq == -EPROBE_DEFER)
> > +   return -EPROBE_DEFER;
> > +
> > ret = devm_request_threaded_irq(>dev, q6v5->stop_irq,
> > NULL, q6v5_stop_interrupt,
> > IRQF_TRIGGER_RISING | IRQF_ONESHOT,
> 
> -- 
> -- Sibi Sankar --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project.


Re: [PATCH] remoteproc: qcom: q6v5: Propagate EPROBE_DEFER

2018-10-06 Thread Bjorn Andersson
On Mon 24 Sep 23:50 PDT 2018, Sibi Sankar wrote:

> On 2018-09-20 07:21, Bjorn Andersson wrote:
> > In the case that the interrupts fail to result because of the
> > interrupt-controller not yet being registered the
> > platform_get_irq_byname() call will fail with -EPROBE_DEFER, but passing
> > this into devm_request_threaded_irq() will result in -EINVAL being
> > returned, the driver is therefor not reprobed later.
> > 
> 
> The patch looks fine.
> Reviewed-by: Sibi Sankar 
> 

Thanks for the review Sibi, applied the patch.

Regards,
Bjorn

> > Fixes: 3b415c8fb263 ("remoteproc: q6v5: Extract common resource
> > handling")
> > Cc: sta...@vger.kernel.org
> > Signed-off-by: Bjorn Andersson 
> > ---
> >  drivers/remoteproc/qcom_q6v5.c | 12 
> >  1 file changed, 12 insertions(+)
> > 
> > diff --git a/drivers/remoteproc/qcom_q6v5.c
> > b/drivers/remoteproc/qcom_q6v5.c
> > index 61a760ee4aac..e9ab90c19304 100644
> > --- a/drivers/remoteproc/qcom_q6v5.c
> > +++ b/drivers/remoteproc/qcom_q6v5.c
> > @@ -198,6 +198,9 @@ int qcom_q6v5_init(struct qcom_q6v5 *q6v5, struct
> > platform_device *pdev,
> > }
> > 
> > q6v5->fatal_irq = platform_get_irq_byname(pdev, "fatal");
> > +   if (q6v5->fatal_irq == -EPROBE_DEFER)
> > +   return -EPROBE_DEFER;
> > +
> > ret = devm_request_threaded_irq(>dev, q6v5->fatal_irq,
> > NULL, q6v5_fatal_interrupt,
> > IRQF_TRIGGER_RISING | IRQF_ONESHOT,
> > @@ -208,6 +211,9 @@ int qcom_q6v5_init(struct qcom_q6v5 *q6v5, struct
> > platform_device *pdev,
> > }
> > 
> > q6v5->ready_irq = platform_get_irq_byname(pdev, "ready");
> > +   if (q6v5->ready_irq == -EPROBE_DEFER)
> > +   return -EPROBE_DEFER;
> > +
> > ret = devm_request_threaded_irq(>dev, q6v5->ready_irq,
> > NULL, q6v5_ready_interrupt,
> > IRQF_TRIGGER_RISING | IRQF_ONESHOT,
> > @@ -218,6 +224,9 @@ int qcom_q6v5_init(struct qcom_q6v5 *q6v5, struct
> > platform_device *pdev,
> > }
> > 
> > q6v5->handover_irq = platform_get_irq_byname(pdev, "handover");
> > +   if (q6v5->handover_irq == -EPROBE_DEFER)
> > +   return -EPROBE_DEFER;
> > +
> > ret = devm_request_threaded_irq(>dev, q6v5->handover_irq,
> > NULL, q6v5_handover_interrupt,
> > IRQF_TRIGGER_RISING | IRQF_ONESHOT,
> > @@ -229,6 +238,9 @@ int qcom_q6v5_init(struct qcom_q6v5 *q6v5, struct
> > platform_device *pdev,
> > disable_irq(q6v5->handover_irq);
> > 
> > q6v5->stop_irq = platform_get_irq_byname(pdev, "stop-ack");
> > +   if (q6v5->stop_irq == -EPROBE_DEFER)
> > +   return -EPROBE_DEFER;
> > +
> > ret = devm_request_threaded_irq(>dev, q6v5->stop_irq,
> > NULL, q6v5_stop_interrupt,
> > IRQF_TRIGGER_RISING | IRQF_ONESHOT,
> 
> -- 
> -- Sibi Sankar --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project.


Re: [PATCH v2] remoteproc: qcom: pas: Add QCS404 remoteprocs

2018-10-06 Thread Bjorn Andersson
On Thu 27 Sep 23:27 PDT 2018, Sibi Sankar wrote:

> On 2018-09-28 00:33, Bjorn Andersson wrote:
> > Add compatibles for the three PAS based remote processors found in
> > QCS404.
> > 
> > Signed-off-by: Bjorn Andersson 
> > ---
> > 
> 
> Reviewed-by: Sibi Sankar 
> 

Thanks for the review Sibi, applied the patch.

Regards,
Bjorn

> > Changes since v1:
> > - Fixed incorrect sysmon_name, as pointed out by Sibi.
> > 
> >  .../devicetree/bindings/remoteproc/qcom,adsp.txt |  3 +++
> >  drivers/remoteproc/qcom_adsp_pil.c   | 12 
> >  2 files changed, 15 insertions(+)
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/remoteproc/qcom,adsp.txt
> > b/Documentation/devicetree/bindings/remoteproc/qcom,adsp.txt
> > index b7d058228185..9c0cff3a5ed8 100644
> > --- a/Documentation/devicetree/bindings/remoteproc/qcom,adsp.txt
> > +++ b/Documentation/devicetree/bindings/remoteproc/qcom,adsp.txt
> > @@ -10,6 +10,9 @@ on the Qualcomm ADSP Hexagon core.
> > "qcom,msm8974-adsp-pil"
> > "qcom,msm8996-adsp-pil"
> > "qcom,msm8996-slpi-pil"
> > +   "qcom,qcs404-adsp-pas"
> > +   "qcom,qcs404-cdsp-pas"
> > +   "qcom,qcs404-wcss-pas"
> > "qcom,sdm845-adsp-pas"
> > "qcom,sdm845-cdsp-pas"
> > 
> > diff --git a/drivers/remoteproc/qcom_adsp_pil.c
> > b/drivers/remoteproc/qcom_adsp_pil.c
> > index da2254ea1135..d5e58235e83a 100644
> > --- a/drivers/remoteproc/qcom_adsp_pil.c
> > +++ b/drivers/remoteproc/qcom_adsp_pil.c
> > @@ -362,10 +362,22 @@ static const struct adsp_data slpi_resource_init =
> > {
> > .ssctl_id = 0x16,
> >  };
> > 
> > +static const struct adsp_data wcss_resource_init = {
> > +   .crash_reason_smem = 421,
> > +   .firmware_name = "wcnss.mdt",
> > +   .pas_id = 6,
> > +   .ssr_name = "mpss",
> > +   .sysmon_name = "wcnss",
> > +   .ssctl_id = 0x12,
> > +};
> > +
> >  static const struct of_device_id adsp_of_match[] = {
> > { .compatible = "qcom,msm8974-adsp-pil", .data = _resource_init},
> > { .compatible = "qcom,msm8996-adsp-pil", .data = _resource_init},
> > { .compatible = "qcom,msm8996-slpi-pil", .data = _resource_init},
> > +   { .compatible = "qcom,qcs404-adsp-pas", .data = _resource_init },
> > +   { .compatible = "qcom,qcs404-cdsp-pas", .data = _resource_init },
> > +   { .compatible = "qcom,qcs404-wcss-pas", .data = _resource_init },
> > { .compatible = "qcom,sdm845-adsp-pas", .data = _resource_init},
> > { .compatible = "qcom,sdm845-cdsp-pas", .data = _resource_init},
> > { },
> 
> -- 
> -- Sibi Sankar --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project.


Re: [PATCH v2] remoteproc: qcom: pas: Add QCS404 remoteprocs

2018-10-06 Thread Bjorn Andersson
On Thu 27 Sep 23:27 PDT 2018, Sibi Sankar wrote:

> On 2018-09-28 00:33, Bjorn Andersson wrote:
> > Add compatibles for the three PAS based remote processors found in
> > QCS404.
> > 
> > Signed-off-by: Bjorn Andersson 
> > ---
> > 
> 
> Reviewed-by: Sibi Sankar 
> 

Thanks for the review Sibi, applied the patch.

Regards,
Bjorn

> > Changes since v1:
> > - Fixed incorrect sysmon_name, as pointed out by Sibi.
> > 
> >  .../devicetree/bindings/remoteproc/qcom,adsp.txt |  3 +++
> >  drivers/remoteproc/qcom_adsp_pil.c   | 12 
> >  2 files changed, 15 insertions(+)
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/remoteproc/qcom,adsp.txt
> > b/Documentation/devicetree/bindings/remoteproc/qcom,adsp.txt
> > index b7d058228185..9c0cff3a5ed8 100644
> > --- a/Documentation/devicetree/bindings/remoteproc/qcom,adsp.txt
> > +++ b/Documentation/devicetree/bindings/remoteproc/qcom,adsp.txt
> > @@ -10,6 +10,9 @@ on the Qualcomm ADSP Hexagon core.
> > "qcom,msm8974-adsp-pil"
> > "qcom,msm8996-adsp-pil"
> > "qcom,msm8996-slpi-pil"
> > +   "qcom,qcs404-adsp-pas"
> > +   "qcom,qcs404-cdsp-pas"
> > +   "qcom,qcs404-wcss-pas"
> > "qcom,sdm845-adsp-pas"
> > "qcom,sdm845-cdsp-pas"
> > 
> > diff --git a/drivers/remoteproc/qcom_adsp_pil.c
> > b/drivers/remoteproc/qcom_adsp_pil.c
> > index da2254ea1135..d5e58235e83a 100644
> > --- a/drivers/remoteproc/qcom_adsp_pil.c
> > +++ b/drivers/remoteproc/qcom_adsp_pil.c
> > @@ -362,10 +362,22 @@ static const struct adsp_data slpi_resource_init =
> > {
> > .ssctl_id = 0x16,
> >  };
> > 
> > +static const struct adsp_data wcss_resource_init = {
> > +   .crash_reason_smem = 421,
> > +   .firmware_name = "wcnss.mdt",
> > +   .pas_id = 6,
> > +   .ssr_name = "mpss",
> > +   .sysmon_name = "wcnss",
> > +   .ssctl_id = 0x12,
> > +};
> > +
> >  static const struct of_device_id adsp_of_match[] = {
> > { .compatible = "qcom,msm8974-adsp-pil", .data = _resource_init},
> > { .compatible = "qcom,msm8996-adsp-pil", .data = _resource_init},
> > { .compatible = "qcom,msm8996-slpi-pil", .data = _resource_init},
> > +   { .compatible = "qcom,qcs404-adsp-pas", .data = _resource_init },
> > +   { .compatible = "qcom,qcs404-cdsp-pas", .data = _resource_init },
> > +   { .compatible = "qcom,qcs404-wcss-pas", .data = _resource_init },
> > { .compatible = "qcom,sdm845-adsp-pas", .data = _resource_init},
> > { .compatible = "qcom,sdm845-cdsp-pas", .data = _resource_init},
> > { },
> 
> -- 
> -- Sibi Sankar --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project.


Re: [PATCH] remoteproc: qcom: q6v5: Fix a race condition on fatal crash

2018-10-06 Thread Bjorn Andersson
On Mon 01 Oct 07:25 PDT 2018, Sibi Sankar wrote:

> Currently with GLINK_SSR enabled each fatal crash results in servicing
> a crash from wdog as well. This is due to a race that occurs in setting
> the running flag in the shutdown path. Fix this by moving the running
> flag to the end of fatal interrupt handler.
> 
> Crash Logs:
> qcom-q6v5-pil 408.remoteproc: fatal error without message
> remoteproc remoteproc0: crash detected in 408.remoteproc: type fatal
>   error
> remoteproc remoteproc0: handling crash #1 in 408.remoteproc
> remoteproc remoteproc0: recovering 408.remoteproc
> qcom-q6v5-pil 408.remoteproc: watchdog without message
> remoteproc remoteproc0: crash detected in 408.remoteproc: type watchdog
> remoteproc:glink-edge: intent request timed out
> qcom_glink_ssr remoteproc:glink-edge.glink_ssr.-1.-1: failed to send
>   cleanup message
> qcom_glink_ssr remoteproc:glink-edge.glink_ssr.-1.-1: timeout waiting
>   for cleanup done message
> qcom-q6v5-pil 408.remoteproc: timed out on wait
> qcom-q6v5-pil 408.remoteproc: port failed halt
> remoteproc remoteproc0: stopped remote processor 408.remoteproc
> qcom-q6v5-pil 408.remoteproc: MBA booted, loading mpss
> remoteproc remoteproc0: remote processor 408.remoteproc is now up
> remoteproc remoteproc0: handling crash #2 in 408.remoteproc
> remoteproc remoteproc0: recovering 408.remoteproc
> qcom-q6v5-pil 408.remoteproc: port failed halt
> remoteproc remoteproc0: stopped remote processor 408.remoteproc
> qcom-q6v5-pil 408.remoteproc: MBA booted, loading mpss
> remoteproc remoteproc0: remote processor 408.remoteproc is now up
> 
> [bjorn: move running flag to the end of fatal interrupt handler]

Turned this line into a Suggested-by

> Signed-off-by: Sibi Sankar 

Applied.

Thanks,
Bjorn

> ---
>  drivers/remoteproc/qcom_q6v5.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/remoteproc/qcom_q6v5.c b/drivers/remoteproc/qcom_q6v5.c
> index e9ab90c19304..edeb2e43209e 100644
> --- a/drivers/remoteproc/qcom_q6v5.c
> +++ b/drivers/remoteproc/qcom_q6v5.c
> @@ -84,6 +84,7 @@ static irqreturn_t q6v5_fatal_interrupt(int irq, void *data)
>   else
>   dev_err(q6v5->dev, "fatal error without message\n");
>  
> + q6v5->running = false;
>   rproc_report_crash(q6v5->rproc, RPROC_FATAL_ERROR);
>  
>   return IRQ_HANDLED;
> @@ -150,8 +151,6 @@ int qcom_q6v5_request_stop(struct qcom_q6v5 *q6v5)
>  {
>   int ret;
>  
> - q6v5->running = false;
> -
>   qcom_smem_state_update_bits(q6v5->state,
>   BIT(q6v5->stop_bit), BIT(q6v5->stop_bit));
>  
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 


Re: [PATCH] remoteproc: qcom: q6v5: Fix a race condition on fatal crash

2018-10-06 Thread Bjorn Andersson
On Mon 01 Oct 07:25 PDT 2018, Sibi Sankar wrote:

> Currently with GLINK_SSR enabled each fatal crash results in servicing
> a crash from wdog as well. This is due to a race that occurs in setting
> the running flag in the shutdown path. Fix this by moving the running
> flag to the end of fatal interrupt handler.
> 
> Crash Logs:
> qcom-q6v5-pil 408.remoteproc: fatal error without message
> remoteproc remoteproc0: crash detected in 408.remoteproc: type fatal
>   error
> remoteproc remoteproc0: handling crash #1 in 408.remoteproc
> remoteproc remoteproc0: recovering 408.remoteproc
> qcom-q6v5-pil 408.remoteproc: watchdog without message
> remoteproc remoteproc0: crash detected in 408.remoteproc: type watchdog
> remoteproc:glink-edge: intent request timed out
> qcom_glink_ssr remoteproc:glink-edge.glink_ssr.-1.-1: failed to send
>   cleanup message
> qcom_glink_ssr remoteproc:glink-edge.glink_ssr.-1.-1: timeout waiting
>   for cleanup done message
> qcom-q6v5-pil 408.remoteproc: timed out on wait
> qcom-q6v5-pil 408.remoteproc: port failed halt
> remoteproc remoteproc0: stopped remote processor 408.remoteproc
> qcom-q6v5-pil 408.remoteproc: MBA booted, loading mpss
> remoteproc remoteproc0: remote processor 408.remoteproc is now up
> remoteproc remoteproc0: handling crash #2 in 408.remoteproc
> remoteproc remoteproc0: recovering 408.remoteproc
> qcom-q6v5-pil 408.remoteproc: port failed halt
> remoteproc remoteproc0: stopped remote processor 408.remoteproc
> qcom-q6v5-pil 408.remoteproc: MBA booted, loading mpss
> remoteproc remoteproc0: remote processor 408.remoteproc is now up
> 
> [bjorn: move running flag to the end of fatal interrupt handler]

Turned this line into a Suggested-by

> Signed-off-by: Sibi Sankar 

Applied.

Thanks,
Bjorn

> ---
>  drivers/remoteproc/qcom_q6v5.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/remoteproc/qcom_q6v5.c b/drivers/remoteproc/qcom_q6v5.c
> index e9ab90c19304..edeb2e43209e 100644
> --- a/drivers/remoteproc/qcom_q6v5.c
> +++ b/drivers/remoteproc/qcom_q6v5.c
> @@ -84,6 +84,7 @@ static irqreturn_t q6v5_fatal_interrupt(int irq, void *data)
>   else
>   dev_err(q6v5->dev, "fatal error without message\n");
>  
> + q6v5->running = false;
>   rproc_report_crash(q6v5->rproc, RPROC_FATAL_ERROR);
>  
>   return IRQ_HANDLED;
> @@ -150,8 +151,6 @@ int qcom_q6v5_request_stop(struct qcom_q6v5 *q6v5)
>  {
>   int ret;
>  
> - q6v5->running = false;
> -
>   qcom_smem_state_update_bits(q6v5->state,
>   BIT(q6v5->stop_bit), BIT(q6v5->stop_bit));
>  
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 


Re: [PATCH] samples/rpmsg: Introduce a module parameter for message count

2018-10-06 Thread Bjorn Andersson
On Tue 11 Sep 10:46 PDT 2018, Suman Anna wrote:

> The current rpmsg_client_sample uses a fixed number of messages to
> be sent to each instance. This is currently set at 100. Introduce
> an optional module parameter 'count' so that the number of messages
> to be exchanged can be made flexible.
> 

Rather than sending N messages as fast as possible to any sample channel
that comes up, how about making the sample create a debugfs entry that
we can write messages to from user space?

That would make it possible to improve the handling of multiple
remoteprocs and would allow for a variation in message lengths etc.

Regards,
Bjorn

> Signed-off-by: Suman Anna 
> ---
>  samples/rpmsg/rpmsg_client_sample.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/samples/rpmsg/rpmsg_client_sample.c 
> b/samples/rpmsg/rpmsg_client_sample.c
> index f161dfd3e70a..9b6b27ea504f 100644
> --- a/samples/rpmsg/rpmsg_client_sample.c
> +++ b/samples/rpmsg/rpmsg_client_sample.c
> @@ -22,7 +22,9 @@
>  #include 
>  
>  #define MSG  "hello world!"
> -#define MSG_LIMIT100
> +
> +static int count = 100;
> +module_param(count, int, 0644);
>  
>  struct instance_data {
>   int rx_count;
> @@ -41,7 +43,7 @@ static int rpmsg_sample_cb(struct rpmsg_device *rpdev, void 
> *data, int len,
>  data, len,  true);
>  
>   /* samples should not live forever */
> - if (idata->rx_count >= MSG_LIMIT) {
> + if (idata->rx_count >= count) {
>   dev_info(>dev, "goodbye!\n");
>   return 0;
>   }
> -- 
> 2.18.0
> 


Re: [PATCH] samples/rpmsg: Introduce a module parameter for message count

2018-10-06 Thread Bjorn Andersson
On Tue 11 Sep 10:46 PDT 2018, Suman Anna wrote:

> The current rpmsg_client_sample uses a fixed number of messages to
> be sent to each instance. This is currently set at 100. Introduce
> an optional module parameter 'count' so that the number of messages
> to be exchanged can be made flexible.
> 

Rather than sending N messages as fast as possible to any sample channel
that comes up, how about making the sample create a debugfs entry that
we can write messages to from user space?

That would make it possible to improve the handling of multiple
remoteprocs and would allow for a variation in message lengths etc.

Regards,
Bjorn

> Signed-off-by: Suman Anna 
> ---
>  samples/rpmsg/rpmsg_client_sample.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/samples/rpmsg/rpmsg_client_sample.c 
> b/samples/rpmsg/rpmsg_client_sample.c
> index f161dfd3e70a..9b6b27ea504f 100644
> --- a/samples/rpmsg/rpmsg_client_sample.c
> +++ b/samples/rpmsg/rpmsg_client_sample.c
> @@ -22,7 +22,9 @@
>  #include 
>  
>  #define MSG  "hello world!"
> -#define MSG_LIMIT100
> +
> +static int count = 100;
> +module_param(count, int, 0644);
>  
>  struct instance_data {
>   int rx_count;
> @@ -41,7 +43,7 @@ static int rpmsg_sample_cb(struct rpmsg_device *rpdev, void 
> *data, int len,
>  data, len,  true);
>  
>   /* samples should not live forever */
> - if (idata->rx_count >= MSG_LIMIT) {
> + if (idata->rx_count >= count) {
>   dev_info(>dev, "goodbye!\n");
>   return 0;
>   }
> -- 
> 2.18.0
> 


Re: [PATCH 3/5] remoteproc: Add missing kernel-doc comment for auto-boot

2018-10-06 Thread Bjorn Andersson
On Fri 14 Sep 17:37 PDT 2018, Suman Anna wrote:

> The commit ddf711872c9d ("remoteproc: Introduce auto-boot flag")
> introduced the auto-boot flag but missed adding the corresponding
> kernel-doc comment. Add the same.
> 
> Signed-off-by: Suman Anna 

Applied.

Thanks,
Bjorn

> ---
>  include/linux/remoteproc.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
> index e3c5d856b6da..75f9ca05b865 100644
> --- a/include/linux/remoteproc.h
> +++ b/include/linux/remoteproc.h
> @@ -439,6 +439,7 @@ struct rproc_dump_segment {
>   * @cached_table: copy of the resource table
>   * @table_sz: size of @cached_table
>   * @has_iommu: flag to indicate if remote processor is behind an MMU
> + * @auto_boot: flag to indicate if remote processor should be auto-started
>   * @dump_segments: list of segments in the firmware
>   */
>  struct rproc {
> -- 
> 2.18.0
> 


Re: [PATCH 3/5] remoteproc: Add missing kernel-doc comment for auto-boot

2018-10-06 Thread Bjorn Andersson
On Fri 14 Sep 17:37 PDT 2018, Suman Anna wrote:

> The commit ddf711872c9d ("remoteproc: Introduce auto-boot flag")
> introduced the auto-boot flag but missed adding the corresponding
> kernel-doc comment. Add the same.
> 
> Signed-off-by: Suman Anna 

Applied.

Thanks,
Bjorn

> ---
>  include/linux/remoteproc.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
> index e3c5d856b6da..75f9ca05b865 100644
> --- a/include/linux/remoteproc.h
> +++ b/include/linux/remoteproc.h
> @@ -439,6 +439,7 @@ struct rproc_dump_segment {
>   * @cached_table: copy of the resource table
>   * @table_sz: size of @cached_table
>   * @has_iommu: flag to indicate if remote processor is behind an MMU
> + * @auto_boot: flag to indicate if remote processor should be auto-started
>   * @dump_segments: list of segments in the firmware
>   */
>  struct rproc {
> -- 
> 2.18.0
> 


Re: [PATCH 2/5] remoteproc: Check for NULL firmwares in sysfs interface

2018-10-06 Thread Bjorn Andersson
On Fri 14 Sep 17:37 PDT 2018, Suman Anna wrote:

> The remoteproc framework provides a sysfs file 'firmware'
> for modifying the firmware image name from userspace. Add
> an additional check to ensure NULL firmwares are errored
> out right away, rather than getting a delayed error while
> requesting a firmware during the start of a remoteproc
> later on.
> 
> Signed-off-by: Suman Anna 

Applied.

Regards,
Bjorn

> ---
>  drivers/remoteproc/remoteproc_sysfs.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/remoteproc/remoteproc_sysfs.c 
> b/drivers/remoteproc/remoteproc_sysfs.c
> index 2142b3ea726e..ce93f4d710f3 100644
> --- a/drivers/remoteproc/remoteproc_sysfs.c
> +++ b/drivers/remoteproc/remoteproc_sysfs.c
> @@ -49,6 +49,11 @@ static ssize_t firmware_store(struct device *dev,
>   }
>  
>   len = strcspn(buf, "\n");
> + if (!len) {
> + dev_err(dev, "can't provide a NULL firmware\n");
> + err = -EINVAL;
> + goto out;
> + }
>  
>   p = kstrndup(buf, len, GFP_KERNEL);
>   if (!p) {
> -- 
> 2.18.0
> 


Re: [PATCH 2/5] remoteproc: Check for NULL firmwares in sysfs interface

2018-10-06 Thread Bjorn Andersson
On Fri 14 Sep 17:37 PDT 2018, Suman Anna wrote:

> The remoteproc framework provides a sysfs file 'firmware'
> for modifying the firmware image name from userspace. Add
> an additional check to ensure NULL firmwares are errored
> out right away, rather than getting a delayed error while
> requesting a firmware during the start of a remoteproc
> later on.
> 
> Signed-off-by: Suman Anna 

Applied.

Regards,
Bjorn

> ---
>  drivers/remoteproc/remoteproc_sysfs.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/remoteproc/remoteproc_sysfs.c 
> b/drivers/remoteproc/remoteproc_sysfs.c
> index 2142b3ea726e..ce93f4d710f3 100644
> --- a/drivers/remoteproc/remoteproc_sysfs.c
> +++ b/drivers/remoteproc/remoteproc_sysfs.c
> @@ -49,6 +49,11 @@ static ssize_t firmware_store(struct device *dev,
>   }
>  
>   len = strcspn(buf, "\n");
> + if (!len) {
> + dev_err(dev, "can't provide a NULL firmware\n");
> + err = -EINVAL;
> + goto out;
> + }
>  
>   p = kstrndup(buf, len, GFP_KERNEL);
>   if (!p) {
> -- 
> 2.18.0
> 


Re: [PATCH 1/5] remoteproc: Fix unbalanced boot with sysfs for no auto-boot rprocs

2018-10-06 Thread Bjorn Andersson
On Fri 14 Sep 17:37 PDT 2018, Suman Anna wrote:

> The remoteproc core performs automatic boot and shutdown of a remote
> processor during rproc_add() and rproc_del() for remote processors
> supporting 'auto-boot'. The remoteproc devices not using 'auto-boot'
> require either a remoteproc client driver or a userspace client to
> use the sysfs 'state' variable to perform the boot and shutdown. The
> in-kernel client drivers hold the corresponding remoteproc driver
> module's reference count when they acquire a rproc handle through
> the rproc_get_by_phandle() API, but there is no such support for
> userspace applications performing the boot through sysfs interface.
> 
> The shutdown of a remoteproc upon removing a remoteproc platform
> driver is automatic only with 'auto-boot' and this can cause a
> remoteproc with no auto-boot to stay powered on and never freed
> up if booted using the sysfs interface without a matching stop,
> and when the remoteproc driver module is removed or unbound from
> the device. This will result in a memory leak as well as the
> corresponding remoteproc ida being never deallocated. Fix this
> by holding a module reference count for the remoteproc's driver
> during a sysfs 'start' and releasing it during the sysfs 'stop'
> operation.
> 

This prevents you from rmmod'ing the remoteproc driver, but it does not
prevent you from issuing an unbind of the driver - resulting in the same
issue.

I would prefer if we made sure that rproc_del() always cleaned up any
resources (and stopped the remoteproc processor), but I'm uncertain of
how to deal with remote processors that are supposed to survive Linux
shutting down.

But I'm also uncertain how we can make the remoteproc core ensure that
no dynamic resources are leaked in such scenario.

Regards,
Bjorn

> Signed-off-by: Suman Anna 
> ---
>  drivers/remoteproc/remoteproc_sysfs.c | 16 +++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/remoteproc/remoteproc_sysfs.c 
> b/drivers/remoteproc/remoteproc_sysfs.c
> index 47be411400e5..2142b3ea726e 100644
> --- a/drivers/remoteproc/remoteproc_sysfs.c
> +++ b/drivers/remoteproc/remoteproc_sysfs.c
> @@ -11,6 +11,7 @@
>   * GNU General Public License for more details.
>   */
>  
> +#include 
>  #include 
>  
>  #include "remoteproc_internal.h"
> @@ -100,14 +101,27 @@ static ssize_t state_store(struct device *dev,
>   if (rproc->state == RPROC_RUNNING)
>   return -EBUSY;
>  
> + /*
> +  * prevent underlying implementation from being removed
> +  * when remoteproc does not support auto-boot
> +  */
> + if (!rproc->auto_boot &&
> + !try_module_get(dev->parent->driver->owner))
> + return -EINVAL;
> +
>   ret = rproc_boot(rproc);
> - if (ret)
> + if (ret) {
>   dev_err(>dev, "Boot failed: %d\n", ret);
> + if (!rproc->auto_boot)
> + module_put(dev->parent->driver->owner);
> + }
>   } else if (sysfs_streq(buf, "stop")) {
>   if (rproc->state != RPROC_RUNNING)
>   return -EINVAL;
>  
>   rproc_shutdown(rproc);
> + if (!rproc->auto_boot)
> + module_put(dev->parent->driver->owner);
>   } else {
>   dev_err(>dev, "Unrecognised option: %s\n", buf);
>   ret = -EINVAL;
> -- 
> 2.18.0
> 


Re: [PATCH 1/5] remoteproc: Fix unbalanced boot with sysfs for no auto-boot rprocs

2018-10-06 Thread Bjorn Andersson
On Fri 14 Sep 17:37 PDT 2018, Suman Anna wrote:

> The remoteproc core performs automatic boot and shutdown of a remote
> processor during rproc_add() and rproc_del() for remote processors
> supporting 'auto-boot'. The remoteproc devices not using 'auto-boot'
> require either a remoteproc client driver or a userspace client to
> use the sysfs 'state' variable to perform the boot and shutdown. The
> in-kernel client drivers hold the corresponding remoteproc driver
> module's reference count when they acquire a rproc handle through
> the rproc_get_by_phandle() API, but there is no such support for
> userspace applications performing the boot through sysfs interface.
> 
> The shutdown of a remoteproc upon removing a remoteproc platform
> driver is automatic only with 'auto-boot' and this can cause a
> remoteproc with no auto-boot to stay powered on and never freed
> up if booted using the sysfs interface without a matching stop,
> and when the remoteproc driver module is removed or unbound from
> the device. This will result in a memory leak as well as the
> corresponding remoteproc ida being never deallocated. Fix this
> by holding a module reference count for the remoteproc's driver
> during a sysfs 'start' and releasing it during the sysfs 'stop'
> operation.
> 

This prevents you from rmmod'ing the remoteproc driver, but it does not
prevent you from issuing an unbind of the driver - resulting in the same
issue.

I would prefer if we made sure that rproc_del() always cleaned up any
resources (and stopped the remoteproc processor), but I'm uncertain of
how to deal with remote processors that are supposed to survive Linux
shutting down.

But I'm also uncertain how we can make the remoteproc core ensure that
no dynamic resources are leaked in such scenario.

Regards,
Bjorn

> Signed-off-by: Suman Anna 
> ---
>  drivers/remoteproc/remoteproc_sysfs.c | 16 +++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/remoteproc/remoteproc_sysfs.c 
> b/drivers/remoteproc/remoteproc_sysfs.c
> index 47be411400e5..2142b3ea726e 100644
> --- a/drivers/remoteproc/remoteproc_sysfs.c
> +++ b/drivers/remoteproc/remoteproc_sysfs.c
> @@ -11,6 +11,7 @@
>   * GNU General Public License for more details.
>   */
>  
> +#include 
>  #include 
>  
>  #include "remoteproc_internal.h"
> @@ -100,14 +101,27 @@ static ssize_t state_store(struct device *dev,
>   if (rproc->state == RPROC_RUNNING)
>   return -EBUSY;
>  
> + /*
> +  * prevent underlying implementation from being removed
> +  * when remoteproc does not support auto-boot
> +  */
> + if (!rproc->auto_boot &&
> + !try_module_get(dev->parent->driver->owner))
> + return -EINVAL;
> +
>   ret = rproc_boot(rproc);
> - if (ret)
> + if (ret) {
>   dev_err(>dev, "Boot failed: %d\n", ret);
> + if (!rproc->auto_boot)
> + module_put(dev->parent->driver->owner);
> + }
>   } else if (sysfs_streq(buf, "stop")) {
>   if (rproc->state != RPROC_RUNNING)
>   return -EINVAL;
>  
>   rproc_shutdown(rproc);
> + if (!rproc->auto_boot)
> + module_put(dev->parent->driver->owner);
>   } else {
>   dev_err(>dev, "Unrecognised option: %s\n", buf);
>   ret = -EINVAL;
> -- 
> 2.18.0
> 


Re: [RFC v3 1/1] ns: add binfmt_misc to the user namespace

2018-10-06 Thread Andrei Vagin
On Thu, Oct 04, 2018 at 12:50:22AM +0200, Laurent Vivier wrote:
> This patch allows to have a different binfmt_misc configuration
> for each new user namespace. By default, the binfmt_misc configuration
> is the one of the host, but if the binfmt_misc filesystem is mounted
> in the new namespace a new empty binfmt instance is created and used
> in this namespace.
> 
> For instance, using "unshare" we can start a chroot of an another
> architecture and configure the binfmt_misc interpreter without being root
> to run the binaries in this chroot.
> 
> Signed-off-by: Laurent Vivier 
> ---
>  fs/binfmt_misc.c   | 85 +++---
>  include/linux/user_namespace.h | 15 ++
>  kernel/user.c  | 14 ++
>  kernel/user_namespace.c|  9 
>  4 files changed, 95 insertions(+), 28 deletions(-)
> 
> diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
> index aa4a7a23ff99..78780bc87506 100644
> --- a/fs/binfmt_misc.c
> +++ b/fs/binfmt_misc.c
> @@ -38,9 +38,6 @@ enum {
>   VERBOSE_STATUS = 1 /* make it zero to save 400 bytes kernel memory */
>  };
>  
> -static LIST_HEAD(entries);
> -static int enabled = 1;
> -
>  enum {Enabled, Magic};
>  #define MISC_FMT_PRESERVE_ARGV0 (1 << 31)
>  #define MISC_FMT_OPEN_BINARY (1 << 30)
> @@ -60,10 +57,7 @@ typedef struct {
>   struct file *interp_file;
>  } Node;
>  
> -static DEFINE_RWLOCK(entries_lock);
>  static struct file_system_type bm_fs_type;
> -static struct vfsmount *bm_mnt;
> -static int entry_count;
>  
>  /*
>   * Max length of the register string.  Determined by:
> @@ -85,13 +79,13 @@ static int entry_count;
>   * if we do, return the node, else NULL
>   * locking is done in load_misc_binary
>   */
> -static Node *check_file(struct linux_binprm *bprm)
> +static Node *check_file(struct user_namespace *ns, struct linux_binprm *bprm)
>  {
>   char *p = strrchr(bprm->interp, '.');
>   struct list_head *l;
>  
>   /* Walk all the registered handlers. */
> - list_for_each(l, ) {
> + list_for_each(l, >binfmt_ns->entries) {
>   Node *e = list_entry(l, Node, list);
>   char *s;
>   int j;
> @@ -133,17 +127,18 @@ static int load_misc_binary(struct linux_binprm *bprm)
>   struct file *interp_file = NULL;
>   int retval;
>   int fd_binary = -1;
> + struct user_namespace *ns = current_user_ns();
>  
>   retval = -ENOEXEC;
> - if (!enabled)
> + if (!ns->binfmt_ns->enabled)
>   return retval;
>  
>   /* to keep locking time low, we copy the interpreter string */
> - read_lock(_lock);
> - fmt = check_file(bprm);
> + read_lock(>binfmt_ns->entries_lock);

It looks like ns->binfmt_ns isn't protected by any lock and
ns->binfmt_ns can be changed between read_lock() and read_unlock().

This can be fixed if ns->binfmt_ns will be dereferenced only once in
this function:

struct binfmt_namespace *binfmt_ns = ns->binfmt_ns;

> + fmt = check_file(ns ,bprm);
>   if (fmt)
>   dget(fmt->dentry);
> - read_unlock(_lock);
> + read_unlock(>binfmt_ns->entries_lock);
>   if (!fmt)
>   return retval;
>  
> @@ -609,19 +604,19 @@ static void bm_evict_inode(struct inode *inode)
>   kfree(e);
>  }
>  
> -static void kill_node(Node *e)
> +static void kill_node(struct user_namespace *ns, Node *e)
>  {
>   struct dentry *dentry;
>  
> - write_lock(_lock);
> + write_lock(>binfmt_ns->entries_lock);
>   list_del_init(>list);
> - write_unlock(_lock);
> + write_unlock(>binfmt_ns->entries_lock);
>  
>   dentry = e->dentry;
>   drop_nlink(d_inode(dentry));
>   d_drop(dentry);
>   dput(dentry);
> - simple_release_fs(_mnt, _count);
> + simple_release_fs(>binfmt_ns->bm_mnt, >binfmt_ns->entry_count);
>  }
>  
>  /* / */
> @@ -651,6 +646,7 @@ static ssize_t bm_entry_write(struct file *file, const 
> char __user *buffer,
>   struct dentry *root;
>   Node *e = file_inode(file)->i_private;
>   int res = parse_command(buffer, count);
> + struct user_namespace *ns = file->f_path.dentry->d_sb->s_user_ns;
>  
>   switch (res) {
>   case 1:
> @@ -667,7 +663,7 @@ static ssize_t bm_entry_write(struct file *file, const 
> char __user *buffer,
>   inode_lock(d_inode(root));
>  
>   if (!list_empty(>list))
> - kill_node(e);
> + kill_node(ns, e);
>  
>   inode_unlock(d_inode(root));
>   break;
> @@ -693,6 +689,7 @@ static ssize_t bm_register_write(struct file *file, const 
> char __user *buffer,
>   struct inode *inode;
>   struct super_block *sb = file_inode(file)->i_sb;
>   struct dentry *root = sb->s_root, *dentry;
> + struct user_namespace *ns = file->f_path.dentry->d_sb->s_user_ns;
>   int err = 0;
>  
>   e = create_entry(buffer, count);
> @@ -716,7 +713,8 @@ static ssize_t bm_register_write(struct file *file, const 
> 

Re: [RFC v3 1/1] ns: add binfmt_misc to the user namespace

2018-10-06 Thread Andrei Vagin
On Thu, Oct 04, 2018 at 12:50:22AM +0200, Laurent Vivier wrote:
> This patch allows to have a different binfmt_misc configuration
> for each new user namespace. By default, the binfmt_misc configuration
> is the one of the host, but if the binfmt_misc filesystem is mounted
> in the new namespace a new empty binfmt instance is created and used
> in this namespace.
> 
> For instance, using "unshare" we can start a chroot of an another
> architecture and configure the binfmt_misc interpreter without being root
> to run the binaries in this chroot.
> 
> Signed-off-by: Laurent Vivier 
> ---
>  fs/binfmt_misc.c   | 85 +++---
>  include/linux/user_namespace.h | 15 ++
>  kernel/user.c  | 14 ++
>  kernel/user_namespace.c|  9 
>  4 files changed, 95 insertions(+), 28 deletions(-)
> 
> diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
> index aa4a7a23ff99..78780bc87506 100644
> --- a/fs/binfmt_misc.c
> +++ b/fs/binfmt_misc.c
> @@ -38,9 +38,6 @@ enum {
>   VERBOSE_STATUS = 1 /* make it zero to save 400 bytes kernel memory */
>  };
>  
> -static LIST_HEAD(entries);
> -static int enabled = 1;
> -
>  enum {Enabled, Magic};
>  #define MISC_FMT_PRESERVE_ARGV0 (1 << 31)
>  #define MISC_FMT_OPEN_BINARY (1 << 30)
> @@ -60,10 +57,7 @@ typedef struct {
>   struct file *interp_file;
>  } Node;
>  
> -static DEFINE_RWLOCK(entries_lock);
>  static struct file_system_type bm_fs_type;
> -static struct vfsmount *bm_mnt;
> -static int entry_count;
>  
>  /*
>   * Max length of the register string.  Determined by:
> @@ -85,13 +79,13 @@ static int entry_count;
>   * if we do, return the node, else NULL
>   * locking is done in load_misc_binary
>   */
> -static Node *check_file(struct linux_binprm *bprm)
> +static Node *check_file(struct user_namespace *ns, struct linux_binprm *bprm)
>  {
>   char *p = strrchr(bprm->interp, '.');
>   struct list_head *l;
>  
>   /* Walk all the registered handlers. */
> - list_for_each(l, ) {
> + list_for_each(l, >binfmt_ns->entries) {
>   Node *e = list_entry(l, Node, list);
>   char *s;
>   int j;
> @@ -133,17 +127,18 @@ static int load_misc_binary(struct linux_binprm *bprm)
>   struct file *interp_file = NULL;
>   int retval;
>   int fd_binary = -1;
> + struct user_namespace *ns = current_user_ns();
>  
>   retval = -ENOEXEC;
> - if (!enabled)
> + if (!ns->binfmt_ns->enabled)
>   return retval;
>  
>   /* to keep locking time low, we copy the interpreter string */
> - read_lock(_lock);
> - fmt = check_file(bprm);
> + read_lock(>binfmt_ns->entries_lock);

It looks like ns->binfmt_ns isn't protected by any lock and
ns->binfmt_ns can be changed between read_lock() and read_unlock().

This can be fixed if ns->binfmt_ns will be dereferenced only once in
this function:

struct binfmt_namespace *binfmt_ns = ns->binfmt_ns;

> + fmt = check_file(ns ,bprm);
>   if (fmt)
>   dget(fmt->dentry);
> - read_unlock(_lock);
> + read_unlock(>binfmt_ns->entries_lock);
>   if (!fmt)
>   return retval;
>  
> @@ -609,19 +604,19 @@ static void bm_evict_inode(struct inode *inode)
>   kfree(e);
>  }
>  
> -static void kill_node(Node *e)
> +static void kill_node(struct user_namespace *ns, Node *e)
>  {
>   struct dentry *dentry;
>  
> - write_lock(_lock);
> + write_lock(>binfmt_ns->entries_lock);
>   list_del_init(>list);
> - write_unlock(_lock);
> + write_unlock(>binfmt_ns->entries_lock);
>  
>   dentry = e->dentry;
>   drop_nlink(d_inode(dentry));
>   d_drop(dentry);
>   dput(dentry);
> - simple_release_fs(_mnt, _count);
> + simple_release_fs(>binfmt_ns->bm_mnt, >binfmt_ns->entry_count);
>  }
>  
>  /* / */
> @@ -651,6 +646,7 @@ static ssize_t bm_entry_write(struct file *file, const 
> char __user *buffer,
>   struct dentry *root;
>   Node *e = file_inode(file)->i_private;
>   int res = parse_command(buffer, count);
> + struct user_namespace *ns = file->f_path.dentry->d_sb->s_user_ns;
>  
>   switch (res) {
>   case 1:
> @@ -667,7 +663,7 @@ static ssize_t bm_entry_write(struct file *file, const 
> char __user *buffer,
>   inode_lock(d_inode(root));
>  
>   if (!list_empty(>list))
> - kill_node(e);
> + kill_node(ns, e);
>  
>   inode_unlock(d_inode(root));
>   break;
> @@ -693,6 +689,7 @@ static ssize_t bm_register_write(struct file *file, const 
> char __user *buffer,
>   struct inode *inode;
>   struct super_block *sb = file_inode(file)->i_sb;
>   struct dentry *root = sb->s_root, *dentry;
> + struct user_namespace *ns = file->f_path.dentry->d_sb->s_user_ns;
>   int err = 0;
>  
>   e = create_entry(buffer, count);
> @@ -716,7 +713,8 @@ static ssize_t bm_register_write(struct file *file, const 
> 

A different PD controller firmware problem?

2018-10-06 Thread Theodore Y. Ts'o
On Tue, Sep 11, 2018 at 01:02:00PM +, mario.limoncie...@dell.com wrote:
> > I tried 9370 and it detects the adapter correctly. IIRC I did the same
> > for 5530 and it worked as well.
> 
> Thanks for confirming that.  Hopefully the same change can be ported to PD 
> controller
> firmware then on other models, I'll inquire.

Hey Mario,

Sorry for the thread hijack (I've changed the subject line to make it
clear it's a separate issue), but just this evening I just had a
very interesting problem with my Dell XPS 9370, and it appears to
be related to the PD controller.

Sortly after 12:30am US/Eastern, I got a low power warning on my
system, and the battery power had dropped below 10%.  Apparently the
laptop was not accepting any charge any more.  I tried doing a suspend
to ram, and then unsuspended it, and it still wasn't accepting any
charge, even though the adapter indicated it was plugged in and
supplying power.  I then did a power cycle, and still the laptop
didn't indicate it was charging with a USB C 45W power supply plugged
in.

I inserted a Satechi USB C voltage monitor in-line, and found that
while it was powered on, the laptop has pulling 0 mA at 5V.  If the
laptop was suspended, it would pull 3A at 5V.  Rebooting and power
cycling didn't change this syndrome.

What *did* fix it was powering down, and disconnecting the power
adapter for 30 seconds or so.  Then when I plugged it back in, the
laptop started accepting 20V at 2A.  I assume what happened is that
the PD controller had crashed, and it required a powerdown *and*
unplugging the power to force the EC to reset.

I have noticed other problems where a USB C to HDMI adapter doesn't
quite work right (the laptop refuses to talk to the display), and the
*only* way to fix things is to powerdown Linux and then remove the
power plug.  So this is not the first time that this particular
technique is needed to make my Dell XPS 9370 (with NVMe SSD, currently
running XPS 13 9370 System Firmware version 0.1.5.1) happy again.

What's the best place to report this sort of problem?  And is there
anything more I can do to debug these sorts of apparent PD Controller
/ EC bugs?

- Ted


A different PD controller firmware problem?

2018-10-06 Thread Theodore Y. Ts'o
On Tue, Sep 11, 2018 at 01:02:00PM +, mario.limoncie...@dell.com wrote:
> > I tried 9370 and it detects the adapter correctly. IIRC I did the same
> > for 5530 and it worked as well.
> 
> Thanks for confirming that.  Hopefully the same change can be ported to PD 
> controller
> firmware then on other models, I'll inquire.

Hey Mario,

Sorry for the thread hijack (I've changed the subject line to make it
clear it's a separate issue), but just this evening I just had a
very interesting problem with my Dell XPS 9370, and it appears to
be related to the PD controller.

Sortly after 12:30am US/Eastern, I got a low power warning on my
system, and the battery power had dropped below 10%.  Apparently the
laptop was not accepting any charge any more.  I tried doing a suspend
to ram, and then unsuspended it, and it still wasn't accepting any
charge, even though the adapter indicated it was plugged in and
supplying power.  I then did a power cycle, and still the laptop
didn't indicate it was charging with a USB C 45W power supply plugged
in.

I inserted a Satechi USB C voltage monitor in-line, and found that
while it was powered on, the laptop has pulling 0 mA at 5V.  If the
laptop was suspended, it would pull 3A at 5V.  Rebooting and power
cycling didn't change this syndrome.

What *did* fix it was powering down, and disconnecting the power
adapter for 30 seconds or so.  Then when I plugged it back in, the
laptop started accepting 20V at 2A.  I assume what happened is that
the PD controller had crashed, and it required a powerdown *and*
unplugging the power to force the EC to reset.

I have noticed other problems where a USB C to HDMI adapter doesn't
quite work right (the laptop refuses to talk to the display), and the
*only* way to fix things is to powerdown Linux and then remove the
power plug.  So this is not the first time that this particular
technique is needed to make my Dell XPS 9370 (with NVMe SSD, currently
running XPS 13 9370 System Firmware version 0.1.5.1) happy again.

What's the best place to report this sort of problem?  And is there
anything more I can do to debug these sorts of apparent PD Controller
/ EC bugs?

- Ted


<    1   2   3