* [PATCH v2 2/5] Revert "dmaengine: imx-sdma: fix pagefault when channel is disabled during interrupt"
From: Robin Gong @ 2018-06-08 13:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1528465490-19684-1-git-send-email-yibin.gong@nxp.com>
This reverts commit 2746e2c389f9d50043d21e2204270403efb9d62f.
Don't need this patch anymore,since we can easily check 'sdmac->desc' to avoid
handling dma interrupt after channel disabled if virt-dma used.
Signed-off-by: Robin Gong <yibin.gong@nxp.com>
---
drivers/dma/imx-sdma.c | 21 ---------------------
1 file changed, 21 deletions(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 8d0c1fd..d93b58f 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -354,7 +354,6 @@ struct sdma_channel {
unsigned int chn_count;
unsigned int chn_real_count;
struct imx_dma_data data;
- bool enabled;
};
#define IMX_DMA_SG_LOOP BIT(0)
@@ -615,14 +614,7 @@ static int sdma_config_ownership(struct sdma_channel *sdmac,
static void sdma_enable_channel(struct sdma_engine *sdma, int channel)
{
- unsigned long flags;
- struct sdma_channel *sdmac = &sdma->channel[channel];
-
writel(BIT(channel), sdma->regs + SDMA_H_START);
-
- spin_lock_irqsave(&sdmac->lock, flags);
- sdmac->enabled = true;
- spin_unlock_irqrestore(&sdmac->lock, flags);
}
/*
@@ -740,14 +732,6 @@ static void sdma_update_channel_loop(struct sdma_channel *sdmac)
struct sdma_buffer_descriptor *bd;
int error = 0;
enum dma_status old_status = sdmac->status;
- unsigned long flags;
-
- spin_lock_irqsave(&sdmac->lock, flags);
- if (!sdmac->enabled) {
- spin_unlock_irqrestore(&sdmac->lock, flags);
- return;
- }
- spin_unlock_irqrestore(&sdmac->lock, flags);
/*
* loop mode. Iterate over descriptors, re-setup them and
@@ -1008,15 +992,10 @@ static int sdma_disable_channel(struct dma_chan *chan)
struct sdma_channel *sdmac = to_sdma_chan(chan);
struct sdma_engine *sdma = sdmac->sdma;
int channel = sdmac->channel;
- unsigned long flags;
writel_relaxed(BIT(channel), sdma->regs + SDMA_H_STATSTOP);
sdmac->status = DMA_ERROR;
- spin_lock_irqsave(&sdmac->lock, flags);
- sdmac->enabled = false;
- spin_unlock_irqrestore(&sdmac->lock, flags);
-
return 0;
}
--
2.7.4
^ permalink raw reply related
* [PATCH v2 3/5] dmaengine: imx-sdma: remove usless lock
From: Robin Gong @ 2018-06-08 13:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1528465490-19684-1-git-send-email-yibin.gong@nxp.com>
No need anymore for 'lock' now since virtual dma will provide
the common lock instead.
Signed-off-by: Robin Gong <yibin.gong@nxp.com>
---
drivers/dma/imx-sdma.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index d93b58f..6faec89 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -349,7 +349,6 @@ struct sdma_channel {
unsigned long event_mask[2];
unsigned long watermark_level;
u32 shp_addr, per_addr;
- spinlock_t lock;
enum dma_status status;
unsigned int chn_count;
unsigned int chn_real_count;
@@ -1908,7 +1907,6 @@ static int sdma_probe(struct platform_device *pdev)
struct sdma_channel *sdmac = &sdma->channel[i];
sdmac->sdma = sdma;
- spin_lock_init(&sdmac->lock);
sdmac->channel = i;
sdmac->vc.desc_free = sdma_desc_free;
--
2.7.4
^ permalink raw reply related
* [PATCH v2 4/5] dmaengine: imx-sdma: remove the maximum limation for bd numbers
From: Robin Gong @ 2018-06-08 13:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1528465490-19684-1-git-send-email-yibin.gong@nxp.com>
No this limitation now after virtual dma used since bd is allocated
dynamically instead of static.
Signed-off-by: Robin Gong <yibin.gong@nxp.com>
---
drivers/dma/imx-sdma.c | 14 --------------
1 file changed, 14 deletions(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 6faec89..afaee72 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -292,7 +292,6 @@ struct sdma_context_data {
u32 scratch7;
} __attribute__ ((packed));
-#define NUM_BD (int)(PAGE_SIZE / sizeof(struct sdma_buffer_descriptor))
struct sdma_engine;
@@ -1297,13 +1296,6 @@ static struct dma_async_tx_descriptor *sdma_prep_slave_sg(
if (ret)
goto err_bd_out;
- if (sg_len > NUM_BD) {
- dev_err(sdma->dev, "SDMA channel %d: maximum number of sg exceeded: %d > %d\n",
- channel, sg_len, NUM_BD);
- ret = -EINVAL;
- goto err_bd_out;
- }
-
desc->chn_count = 0;
for_each_sg(sgl, sg, sg_len, i) {
struct sdma_buffer_descriptor *bd = &desc->bd[i];
@@ -1413,12 +1405,6 @@ static struct dma_async_tx_descriptor *sdma_prep_dma_cyclic(
if (ret)
goto err_bd_out;
- if (num_periods > NUM_BD) {
- dev_err(sdma->dev, "SDMA channel %d: maximum number of sg exceeded: %d > %d\n",
- channel, num_periods, NUM_BD);
- goto err_bd_out;
- }
-
if (period_len > 0xffff) {
dev_err(sdma->dev, "SDMA channel %d: maximum period size exceeded: %zu > %d\n",
channel, period_len, 0xffff);
--
2.7.4
^ permalink raw reply related
* [PATCH v2 5/5] dmaengine: imx-sdma: add sdma_transfer_init to decrease code overlap
From: Robin Gong @ 2018-06-08 13:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1528465490-19684-1-git-send-email-yibin.gong@nxp.com>
There are lot of codes overlap between prep_sg and prep_cyclic function.
Add sdma_transfer_init() function to elimated the code overlap.
Signed-off-by: Robin Gong <yibin.gong@nxp.com>
---
drivers/dma/imx-sdma.c | 83 ++++++++++++++++++++++----------------------------
1 file changed, 37 insertions(+), 46 deletions(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index afaee72..25de89e 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -1255,6 +1255,40 @@ static void sdma_free_chan_resources(struct dma_chan *chan)
clk_disable(sdma->clk_ahb);
}
+static struct sdma_desc *sdma_transfer_init(struct sdma_channel *sdmac,
+ enum dma_transfer_direction direction, u32 bds)
+{
+ struct sdma_desc *desc;
+
+ desc = kzalloc((sizeof(*desc)), GFP_KERNEL);
+ if (!desc)
+ goto err_out;
+
+ sdmac->status = DMA_IN_PROGRESS;
+ sdmac->direction = direction;
+ sdmac->flags = 0;
+
+ desc->chn_count = 0;
+ desc->chn_real_count = 0;
+ desc->buf_tail = 0;
+ desc->buf_ptail = 0;
+ desc->sdmac = sdmac;
+ desc->num_bd = bds;
+
+ if (sdma_alloc_bd(desc))
+ goto err_desc_out;
+
+ if (sdma_load_context(sdmac))
+ goto err_desc_out;
+
+ return desc;
+
+err_desc_out:
+ kfree(desc);
+err_out:
+ return NULL;
+}
+
static struct dma_async_tx_descriptor *sdma_prep_slave_sg(
struct dma_chan *chan, struct scatterlist *sgl,
unsigned int sg_len, enum dma_transfer_direction direction,
@@ -1267,36 +1301,13 @@ static struct dma_async_tx_descriptor *sdma_prep_slave_sg(
struct scatterlist *sg;
struct sdma_desc *desc;
- if (sdmac->status == DMA_IN_PROGRESS)
- return NULL;
- sdmac->status = DMA_IN_PROGRESS;
-
- sdmac->flags = 0;
-
- desc = kzalloc((sizeof(*desc)), GFP_KERNEL);
+ desc = sdma_transfer_init(sdmac, direction, sg_len);
if (!desc)
goto err_out;
- desc->buf_tail = 0;
- desc->buf_ptail = 0;
- desc->sdmac = sdmac;
- desc->num_bd = sg_len;
- desc->chn_real_count = 0;
-
- if (sdma_alloc_bd(desc)) {
- kfree(desc);
- goto err_out;
- }
-
dev_dbg(sdma->dev, "setting up %d entries for channel %d.\n",
sg_len, channel);
- sdmac->direction = direction;
- ret = sdma_load_context(sdmac);
- if (ret)
- goto err_bd_out;
-
- desc->chn_count = 0;
for_each_sg(sgl, sg, sg_len, i) {
struct sdma_buffer_descriptor *bd = &desc->bd[i];
int param;
@@ -1372,38 +1383,18 @@ static struct dma_async_tx_descriptor *sdma_prep_dma_cyclic(
struct sdma_engine *sdma = sdmac->sdma;
int num_periods = buf_len / period_len;
int channel = sdmac->channel;
- int ret, i = 0, buf = 0;
+ int i = 0, buf = 0;
struct sdma_desc *desc;
dev_dbg(sdma->dev, "%s channel: %d\n", __func__, channel);
- if (sdmac->status == DMA_IN_PROGRESS)
- return NULL;
- sdmac->status = DMA_IN_PROGRESS;
-
- desc = kzalloc((sizeof(*desc)), GFP_KERNEL);
+ desc = sdma_transfer_init(sdmac, direction, num_periods);
if (!desc)
goto err_out;
-
- desc->buf_tail = 0;
- desc->buf_ptail = 0;
- desc->sdmac = sdmac;
- desc->num_bd = num_periods;
- desc->chn_real_count = 0;
desc->period_len = period_len;
sdmac->flags |= IMX_DMA_SG_LOOP;
- sdmac->direction = direction;
-
- if (sdma_alloc_bd(desc)) {
- kfree(desc);
- goto err_out;
- }
-
- ret = sdma_load_context(sdmac);
- if (ret)
- goto err_bd_out;
if (period_len > 0xffff) {
dev_err(sdma->dev, "SDMA channel %d: maximum period size exceeded: %zu > %d\n",
--
2.7.4
^ permalink raw reply related
* [PATCH v2 2/5] arm64: dts: renesas: r8a77980: add VSPD support
From: Simon Horman @ 2018-06-08 13:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5f151ec5-bec7-ed06-cb94-4f0cfe8e43dc@cogentembedded.com>
[CC Laurent, Geert]
On Thu, Jun 07, 2018 at 11:20:47PM +0300, Sergei Shtylyov wrote:
> Describe VSPD0 in the R8A77980 device tree; it will be used by DU in
> the next patch...
>
> Based on the original (and large) patch by Vladimir Barinov.
>
> Signed-off-by: Vladimir Barinov <vladimir.barinov@cogentembedded.com>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>
> ---
> arch/arm64/boot/dts/renesas/r8a77980.dtsi | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> Index: renesas/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> ===================================================================
> --- renesas.orig/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> +++ renesas/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> @@ -653,6 +653,16 @@
> resets = <&cpg 408>;
> };
>
> + vspd0: vsp at fea20000 {
> + compatible = "renesas,vsp2";
> + reg = <0 0xfea20000 0 0x4000>;
As per "[PATCH] arm64: dts: renesas: Fix VSPD registers range"
I think the width of the range should be 0x5000.
> + interrupts = <GIC_SPI 169 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&cpg CPG_MOD 623>;
> + power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
> + resets = <&cpg 623>;
> + renesas,fcp = <&fcpvd0>;
> + };
> +
> fcpvd0: fcp at fea27000 {
> compatible = "renesas,fcpv";
> reg = <0 0xfea27000 0 0x200>;
>
^ permalink raw reply
* [PATCH v2 12/16] dt-bindings/interrupt-controller: update Marvell ICU bindings
From: Miquel Raynal @ 2018-06-08 14:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180605202902.GA8875@rob-hp-laptop>
Hi Rob,
Thanks for reviewing.
On Tue, 5 Jun 2018 14:29:02 -0600, Rob Herring <robh@kernel.org> wrote:
> On Tue, May 22, 2018 at 11:40:38AM +0200, Miquel Raynal wrote:
> > Change the documentation to reflect the new bindings used for Marvell
> > ICU. This involves describing each interrupt group as a subnode of the
> > ICU node. Each of them having their own compatible.
>
> Need to explain why you need to do this and why breaking backwards
> compatibility is okay.
As explained by Thomas, backward compatibility is not broken as old
bindings are still documented and supported.
I will update the commit message to reflect that point.
>
> >
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> > .../bindings/interrupt-controller/marvell,icu.txt | 81 ++++++++++++++++++----
> > 1 file changed, 69 insertions(+), 12 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/interrupt-controller/marvell,icu.txt b/Documentation/devicetree/bindings/interrupt-controller/marvell,icu.txt
> > index 649b7ec9d9b1..6f7e4355b3d8 100644
> > --- a/Documentation/devicetree/bindings/interrupt-controller/marvell,icu.txt
> > +++ b/Documentation/devicetree/bindings/interrupt-controller/marvell,icu.txt
> > @@ -5,6 +5,8 @@ The Marvell ICU (Interrupt Consolidation Unit) controller is
> > responsible for collecting all wired-interrupt sources in the CP and
> > communicating them to the GIC in the AP, the unit translates interrupt
> > requests on input wires to MSG memory mapped transactions to the GIC.
> > +These messages will access a different GIC memory area depending on
> > +their type (NSR, SR, SEI, REI, etc).
> >
> > Required properties:
> >
> > @@ -12,20 +14,19 @@ Required properties:
> >
> > - reg: Should contain ICU registers location and length.
> >
> > +Subnodes: Each group of interrupt is declared as a subnode of the ICU,
> > +with their own compatible.
> > +
> > +Required properties for the icu_nsr/icu_sei subnodes:
> > +
> > +- compatible: Should be "marvell,cp110-icu-nsr" or "marvell,cp110-icu-sei".
> > +
> > - #interrupt-cells: Specifies the number of cells needed to encode an
> > - interrupt source. The value shall be 3.
> > + interrupt source. The value shall be 2.
> >
> > - The 1st cell is the group type of the ICU interrupt. Possible group
> > - types are:
> > + The 1st cell is the index of the interrupt in the ICU unit.
> >
> > - ICU_GRP_NSR (0x0) : Shared peripheral interrupt, non-secure
> > - ICU_GRP_SR (0x1) : Shared peripheral interrupt, secure
> > - ICU_GRP_SEI (0x4) : System error interrupt
> > - ICU_GRP_REI (0x5) : RAM error interrupt
>
> What happens to SR and REI interrupts?
They were unused since the beginning.
These values are still detailed below (in the legacy section).
[...]
>
> > -
> > - The 2nd cell is the index of the interrupt in the ICU unit.
> > -
> > - The 3rd cell is the type of the interrupt. See arm,gic.txt for
> > + The 2nd cell is the type of the interrupt. See arm,gic.txt for
> > details.
> >
> > - interrupt-controller: Identifies the node as an interrupt
> > @@ -35,17 +36,73 @@ Required properties:
> > that allows to trigger interrupts using MSG memory mapped
> > transactions.
> >
> > +Note: each 'interrupts' property referring to any 'icu_xxx' node shall
> > + have a different number within [0:206].
> > +
> > Example:
> >
> > icu: interrupt-controller at 1e0000 {
> > compatible = "marvell,cp110-icu";
> > reg = <0x1e0000 0x440>;
> > +
> > + CP110_LABEL(icu_nsr): icu-nsr {
>
> 'interrupt-controller' is the proper node name. Is there no register
> range associated sub nodes?
I will update the name.
A few are used only for NSR, a few only for SEI and others are used for
both. But ok, I will add register ranges.
>
> > + compatible = "marvell,cp110-icu-nsr";
> > + #interrupt-cells = <2>;
> > + interrupt-controller;
> > + msi-parent = <&gicp>;
> > + };
> > +
> > + CP110_LABEL(icu_sei): icu-sei {
> > + compatible = "marvell,cp110-icu-sei";
> > + #interrupt-cells = <2>;
> > + interrupt-controller;
> > + msi-parent = <&sei>;
> > + };
>
> Mixture of tabs and spaces.
Oops.
>
> > +};
> > +
Thanks,
Miqu?l
^ permalink raw reply
* [PATCH v2 2/5] arm64: dts: renesas: r8a77980: add VSPD support
From: Laurent Pinchart @ 2018-06-08 14:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180608135456.cathgcuyp24csbwx@verge.net.au>
Hello Simon,
On Friday, 8 June 2018 16:54:56 EEST Simon Horman wrote:
> [CC Laurent, Geert]
>
> On Thu, Jun 07, 2018 at 11:20:47PM +0300, Sergei Shtylyov wrote:
> > Describe VSPD0 in the R8A77980 device tree; it will be used by DU in
> > the next patch...
> >
> > Based on the original (and large) patch by Vladimir Barinov.
> >
> > Signed-off-by: Vladimir Barinov <vladimir.barinov@cogentembedded.com>
> > Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> >
> > ---
> >
> > arch/arm64/boot/dts/renesas/r8a77980.dtsi | 10 ++++++++++
> > 1 file changed, 10 insertions(+)
> >
> > Index: renesas/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> > ===================================================================
> > --- renesas.orig/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> > +++ renesas/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> > @@ -653,6 +653,16 @@
> > resets = <&cpg 408>;
> > };
> >
> > + vspd0: vsp at fea20000 {
> > + compatible = "renesas,vsp2";
> > + reg = <0 0xfea20000 0 0x4000>;
>
> As per "[PATCH] arm64: dts: renesas: Fix VSPD registers range"
> I think the width of the range should be 0x5000.
I agree with that.
> > + interrupts = <GIC_SPI 169 IRQ_TYPE_LEVEL_HIGH>;
> > + clocks = <&cpg CPG_MOD 623>;
> > + power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
> > + resets = <&cpg 623>;
> > + renesas,fcp = <&fcpvd0>;
> > + };
> > +
> > fcpvd0: fcp at fea27000 {
> > compatible = "renesas,fcpv";
> > reg = <0 0xfea27000 0 0x200>;
--
Regards,
Laurent Pinchart
^ permalink raw reply
* [PATCH v2 3/5] arm64: dts: renesas: r8a77980: add DU support
From: Laurent Pinchart @ 2018-06-08 14:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <842200af-095f-7096-a542-1d2865543264@cogentembedded.com>
Hi Sergei,
Thank you for the patch.
On Thursday, 7 June 2018 23:21:38 EEST Sergei Shtylyov wrote:
> Define the generic R8A77980 part of the DU device node.
>
> Based on the original (and large) patch by Vladimir Barinov.
>
> Signed-off-by: Vladimir Barinov <vladimir.barinov@cogentembedded.com>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> arch/arm64/boot/dts/renesas/r8a77980.dtsi | 30 ++++++++++++++++++++++++++
> 1 file changed, 30 insertions(+)
>
> Index: renesas/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> ===================================================================
> --- renesas.orig/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> +++ renesas/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> @@ -671,6 +671,36 @@
> resets = <&cpg 603>;
> };
>
> + du: display at feb00000 {
> + compatible = "renesas,du-r8a77980",
> + "renesas,du-r8a77970";
> + reg = <0 0xfeb00000 0 0x80000>;
> + interrupts = <GIC_SPI 256 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&cpg CPG_MOD 724>;
> + clock-names = "du.0";
> + power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
> + resets = <&cpg 724>;
> + vsps = <&vspd0>;
> + status = "disabled";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port at 0 {
> + reg = <0>;
> + du_out_rgb: endpoint {
> + };
> + };
> +
> + port at 1 {
> + reg = <1>;
> + du_out_lvds0: endpoint {
> + };
> + };
> + };
> + };
> +
> prr: chipid at fff00044 {
> compatible = "renesas,prr";
> reg = <0 0xfff00044 0 4>;
--
Regards,
Laurent Pinchart
^ permalink raw reply
* [PATCH v2 1/5] arm64: dts: renesas: r8a77980: add FCPVD support
From: Laurent Pinchart @ 2018-06-08 14:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <73f61c34-d0ec-75cd-b6fd-a41cfb39078f@cogentembedded.com>
Hi Sergei,
Thank you for the patch.
On Thursday, 7 June 2018 23:19:31 EEST Sergei Shtylyov wrote:
> Describe FCPVD0 in the R8A77980 device tree; it will be used by VSPD0 in
> the next patch...
>
> Based on the original (and large) patch by Vladimir Barinov.
>
> Signed-off-by: Vladimir Barinov <vladimir.barinov@cogentembedded.com>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> arch/arm64/boot/dts/renesas/r8a77980.dtsi | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> Index: renesas/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> ===================================================================
> --- renesas.orig/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> +++ renesas/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> @@ -653,6 +653,14 @@
> resets = <&cpg 408>;
> };
>
> + fcpvd0: fcp at fea27000 {
> + compatible = "renesas,fcpv";
> + reg = <0 0xfea27000 0 0x200>;
> + clocks = <&cpg CPG_MOD 603>;
> + power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
> + resets = <&cpg 603>;
> + };
> +
> prr: chipid at fff00044 {
> compatible = "renesas,prr";
> reg = <0 0xfff00044 0 4>;
--
Regards,
Laurent Pinchart
^ permalink raw reply
* [PATCH 1/2] reset: imx7: Fix always writing bits as 0
From: Lucas Stach @ 2018-06-08 14:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <c441872d7dd355ae09e945447441389e93e3b888.1527621510.git.leonard.crestez@nxp.com>
Am Dienstag, den 29.05.2018, 22:39 +0300 schrieb Leonard Crestez:
> Right now the only user of reset-imx7 is pci-imx6 and the
> reset_control_assert and deassert calls on pciephy_reset don't toggle
> the PCIEPHY_BTN and PCIEPHY_G_RST bits as expected. Fix this by writing
> 1 or 0 respectively.
>
> The reference manual is not very clear regarding SRC_PCIEPHY_RCR but for
> other registers like MIPIPHY and HSICPHY the bits are explicitly
> documented as "1 means assert, 0 means deassert".
>
> The values are still reversed for IMX7_RESET_PCIE_CTRL_APPS_EN.
>
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
Reviewed-by: Lucas Stach <l.stach@pengutronix.de>
> ---
> ?drivers/reset/reset-imx7.c | 2 +-
> ?1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/reset/reset-imx7.c b/drivers/reset/reset-imx7.c
> index 4db177bc89bc..fdeac1946429 100644
> --- a/drivers/reset/reset-imx7.c
> +++ b/drivers/reset/reset-imx7.c
> @@ -78,11 +78,11 @@ static struct imx7_src *to_imx7_src(struct reset_controller_dev *rcdev)
> ?static int imx7_reset_set(struct reset_controller_dev *rcdev,
> > ? ??unsigned long id, bool assert)
> ?{
> > ? struct imx7_src *imx7src = to_imx7_src(rcdev);
> > ? const struct imx7_src_signal *signal = &imx7_src_signals[id];
> > - unsigned int value = 0;
> > + unsigned int value = assert ? signal->bit : 0;
> ?
> > ? switch (id) {
> > ? case IMX7_RESET_PCIEPHY:
> > ? /*
> > ? ?* wait for more than 10us to release phy g_rst and
^ permalink raw reply
* [PATCH v2 4/5] arm64: dts: renesas: r8a77980: add LVDS support
From: Laurent Pinchart @ 2018-06-08 14:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <3e46c70c-c6be-d019-95a9-bf09c888c9c0@cogentembedded.com>
Hi Sergei,
Thank you for the patch.
On Thursday, 7 June 2018 23:23:06 EEST Sergei Shtylyov wrote:
> Define the generic R8A77980 part of the LVDS device node.
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> arch/arm64/boot/dts/renesas/r8a77980.dtsi | 29 ++++++++++++++++++++++++++
> 1 file changed, 29 insertions(+)
>
> Index: renesas/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> ===================================================================
> --- renesas.orig/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> +++ renesas/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> @@ -696,6 +696,35 @@
> port at 1 {
> reg = <1>;
> du_out_lvds0: endpoint {
> + remote-endpoint = <&lvds0_in>;
> + };
> + };
> + };
> + };
> +
> + lvds0: lvds-encoder at feb90000 {
> + compatible = "renesas,r8a77980-lvds";
> + reg = <0 0xfeb90000 0 0x14>;
> + clocks = <&cpg CPG_MOD 727>;
> + power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
> + resets = <&cpg 727>;
> + status = "disabled";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port at 0 {
> + reg = <0>;
> + lvds0_in: endpoint {
> + remote-endpoint =
> + <&du_out_lvds0>;
> + };
> + };
> +
> + port at 1 {
> + reg = <1>;
> + lvds0_out: endpoint {
> };
> };
> };
--
Regards,
Laurent Pinchart
^ permalink raw reply
* [PATCH v2 0/5] Add R8A77980/Condor/V3HSK LVDS/HDMI support
From: Laurent Pinchart @ 2018-06-08 14:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <390fabf0-4caf-1600-3780-1893207d94f7@cogentembedded.com>
Hi Sergei,
On Thursday, 7 June 2018 23:17:03 EEST Sergei Shtylyov wrote:
> Hello!
>
> Here's the set of 5 patches against Simon Horman's 'renesas.git' repo's
> 'renesas-devel-20180604-v4.17' tag. We're adding the R8A77980 FCPVD/VSPD/
> DU/LVDS device nodes and then describing the LVDS decoder and HDMI encoder
> connected to the LVDS output. These patches depend on the Thine THC63LVD1024
> driver and the R8A77980 LVDS support patch in order to work, and R8A77980
> GPIO DT patches in order to apply/compile...
>
> [1/5] arm64: dts: renesas: r8a77980: add FCPVD support
> [2/5] arm64: dts: renesas: r8a77980: add VSPD support
> [3/5] arm64: dts: renesas: r8a77980: add DU support
> [4/5] arm64: dts: renesas: r8a77980: add LVDS support
Based on the recent request of the ARM SoC maintainers to avoid a plethora of
small patches, I think you can squash 1/5 to 4/5 all together.
> [5/5] arm64: dts: renesas: condor/v3hsk: add DU/LVDS/HDMI support
--
Regards,
Laurent Pinchart
^ permalink raw reply
* [PATCH 2/2] PCI: imx: Initial imx7d pm support
From: Lucas Stach @ 2018-06-08 14:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <a6176c8bcfbca08ce4e93e304bdcd48c4600f8e2.1527621510.git.leonard.crestez@nxp.com>
Am Dienstag, den 29.05.2018, 22:39 +0300 schrieb Leonard Crestez:
> On imx7d the phy is turned off in suspend and must be reset on resume.
> Right now lspci -v fails after a suspend/resume cycle, fix this by
> adding minimal suspend/resume code from the nxp vendor tree.
>
> This is currently only enabled for imx7 but the same sequence can be
> applied to other imx pcie variants.
>
> Tested on imx7d-sabresd with an Intel 5622ANHMW wireless pcie adapter.
>
> > The original author is mostly Richard Zhu <hongxing.zhu@nxp.com>, this
> patch adjusts the code to the upstream imx7d implemention using reset
> controls and power domains.
>
> > Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> ---
> ?drivers/pci/dwc/pci-imx6.c | 94 ++++++++++++++++++++++++++++++++++++--
> ?1 file changed, 89 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/pci/dwc/pci-imx6.c b/drivers/pci/dwc/pci-imx6.c
> index 4818ef875f8a..ff6077eeb387 100644
> --- a/drivers/pci/dwc/pci-imx6.c
> +++ b/drivers/pci/dwc/pci-imx6.c
> @@ -540,10 +540,27 @@ static int imx6_pcie_wait_for_speed_change(struct imx6_pcie *imx6_pcie)
> ?
> > ? dev_err(dev, "Speed change timeout\n");
> > ? return -EINVAL;
> ?}
> ?
> +static void imx6_pcie_ltssm_enable(struct device *dev)
> +{
> > + struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
> +
> > + switch (imx6_pcie->variant) {
> > + case IMX6Q:
> > + case IMX6SX:
> > + case IMX6QP:
> > + regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> > + ???IMX6Q_GPR12_PCIE_CTL_2,
> > + ???IMX6Q_GPR12_PCIE_CTL_2);
> > + break;
> > + case IMX7D:
> > + reset_control_deassert(imx6_pcie->apps_reset);
> > + }
> +}
> +
> ?static int imx6_pcie_establish_link(struct imx6_pcie *imx6_pcie)
> ?{
> > ? struct dw_pcie *pci = imx6_pcie->pci;
> > ? struct device *dev = pci->dev;
> > ? u32 tmp;
> @@ -558,15 +575,11 @@ static int imx6_pcie_establish_link(struct imx6_pcie *imx6_pcie)
> > ? tmp &= ~PCIE_RC_LCR_MAX_LINK_SPEEDS_MASK;
> > ? tmp |= PCIE_RC_LCR_MAX_LINK_SPEEDS_GEN1;
> > ? dw_pcie_writel_dbi(pci, PCIE_RC_LCR, tmp);
> ?
> > ? /* Start LTSSM. */
> > - if (imx6_pcie->variant == IMX7D)
> > - reset_control_deassert(imx6_pcie->apps_reset);
> > - else
> > - regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> > - ???IMX6Q_GPR12_PCIE_CTL_2, 1 << 10);
> > + imx6_pcie_ltssm_enable(dev);
> ?
> > ? ret = imx6_pcie_wait_for_link(imx6_pcie);
> > ? if (ret)
> > ? goto err_reset_phy;
> ?
> @@ -681,10 +694,80 @@ static int imx6_add_pcie_port(struct imx6_pcie *imx6_pcie,
> ?
> ?static const struct dw_pcie_ops dw_pcie_ops = {
> > ? .link_up = imx6_pcie_link_up,
> ?};
> ?
> +#ifdef CONFIG_PM_SLEEP
> +static int imx6_pcie_suspend_noirq(struct device *dev)
> +{
> > + struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
> +
> + if (imx6_pcie->variant == IMX7D) {
Instead of this large indented block, please just have an early return
at the start of this function, like:
if (imx6_pcie->variant != IMX7D)
return 0;
Same for the resume function.
> + /* Disable clks */
> > + clk_disable_unprepare(imx6_pcie->pcie);
> > + clk_disable_unprepare(imx6_pcie->pcie_phy);
> > + clk_disable_unprepare(imx6_pcie->pcie_bus);
> > + /* turn off external osc input */
> > + regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> > + ???IMX7D_GPR12_PCIE_PHY_REFCLK_SEL,
> > + ???IMX7D_GPR12_PCIE_PHY_REFCLK_SEL);
> > + }
> +
> > + return 0;
> +}
> +
> +static void imx6_pcie_ltssm_disable(struct device *dev)
> +{
> > + struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
> +
> > + switch (imx6_pcie->variant) {
> > + case IMX6Q:
> > + case IMX6SX:
> > + case IMX6QP:
> > + regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> > + ???IMX6Q_GPR12_PCIE_CTL_2, 0);
> > + break;
> > + case IMX7D:
> > + reset_control_assert(imx6_pcie->apps_reset);
> > + break;
> > + }
> +}
> +
> +static int imx6_pcie_resume_noirq(struct device *dev)
> +{
> > + int ret = 0;
> > + struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
> > + struct pcie_port *pp = &imx6_pcie->pci->pp;
> +
> > + if (imx6_pcie->variant == IMX7D) {
> > + imx6_pcie_ltssm_disable(dev);
The LTSSM disable seems misplaced here. I would have expected it to be
in the suspend function.
> + imx6_pcie_assert_core_reset(imx6_pcie);
> > + imx6_pcie_init_phy(imx6_pcie);
> > + imx6_pcie_deassert_core_reset(imx6_pcie);
> +
> > + /*
> > + ?* controller maybe turn off, re-configure again
> > + ?*/
> > + dw_pcie_setup_rc(pp);
> +
> > + imx6_pcie_ltssm_enable(dev);
> +
> > + ret = imx6_pcie_wait_for_link(imx6_pcie);
> > + if (ret < 0)
> + pr_info("pcie link is down after resume.\n");
If the PHY has been powered down and LTSSM stopped I guess we need to
do the full imx6_pcie_establish_link() dance again here to reliably re-
establish the link. The above seems unsafe.
> + }
> +
> > + return 0;
> +}
> +
> +static const struct dev_pm_ops imx6_pcie_pm_ops = {
> > + SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(imx6_pcie_suspend_noirq,
> > + ??????imx6_pcie_resume_noirq)
> +};
> +#endif
This structure must be outside of the ifdef, or you'll break the build
on !CONFIG_PM_SLEEP.
> ?static int imx6_pcie_probe(struct platform_device *pdev)
> ?{
> > ? struct device *dev = &pdev->dev;
> > ? struct dw_pcie *pci;
> > ? struct imx6_pcie *imx6_pcie;
> @@ -847,10 +930,11 @@ static const struct of_device_id imx6_pcie_of_match[] = {
> ?static struct platform_driver imx6_pcie_driver = {
> > ? .driver = {
> > > ? .name = "imx6q-pcie",
> > ? .of_match_table = imx6_pcie_of_match,
> > ? .suppress_bind_attrs = true,
> > + .pm = &imx6_pcie_pm_ops,
> > ? },
> > ? .probe????= imx6_pcie_probe,
> > ? .shutdown = imx6_pcie_shutdown,
> ?};
> ?
Regards,
Lucas
^ permalink raw reply
* [PATCH v2 13/16] dt-bindings/interrupt-controller: add documentation for Marvell SEI controller
From: Miquel Raynal @ 2018-06-08 14:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180605205121.GA19249@rob-hp-laptop>
Hi Rob, Marc,
On Tue, 5 Jun 2018 14:51:21 -0600, Rob Herring <robh@kernel.org> wrote:
> On Tue, May 22, 2018 at 11:40:39AM +0200, Miquel Raynal wrote:
> > Describe the System Error Interrupt (SEI) controller. It aggregates two
> > types of interrupts, wired and MSIs from respectively the AP and the
> > CPs, into a single SPI interrupt.
> >
> > Suggested-by: Haim Boot <hayim@marvell.com>
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> > .../bindings/interrupt-controller/marvell,sei.txt | 50 ++++++++++++++++++++++
> > 1 file changed, 50 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/interrupt-controller/marvell,sei.txt
> >
> > diff --git a/Documentation/devicetree/bindings/interrupt-controller/marvell,sei.txt b/Documentation/devicetree/bindings/interrupt-controller/marvell,sei.txt
> > new file mode 100644
> > index 000000000000..689981036c30
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/interrupt-controller/marvell,sei.txt
> > @@ -0,0 +1,50 @@
> > +Marvell SEI (System Error Interrupt) Controller
> > +-----------------------------------------------
> > +
> > +Marvell SEI (System Error Interrupt) controller is an interrupt
> > +aggregator. It receives interrupts from several sources and aggregates
> > +them to a single interrupt line (an SPI) on the parent interrupt
> > +controller.
> > +
> > +This interrupt controller can handle up to 64 SEIs, a set comes from the
> > +AP and is wired while a second set comes from the CPs by the mean of
> > +MSIs. Each 'domain' is represented as a subnode.
> > +
> > +Required properties:
> > +
> > +- compatible: should be "marvell,armada-8k-sei".
> > +- reg: SEI registers location and length.
> > +- interrupts: identifies the parent IRQ that will be triggered.
> > +
> > +Child node 'sei-wired-controller' required properties:
> > +
> > +- marvell,sei-ranges: ranges of wired interrupts.
> > +- #interrupt-cells: number of cells to define an SEI wired interrupt
> > + coming from the AP, should be 1. The cell is the IRQ
> > + number.
> > +- interrupt-controller: identifies the node as an interrupt controller.
> > +
> > +Child node 'sei-msi-controller' required properties:
> > +
> > +- marvell,sei-ranges: ranges of non-wired interrupts triggered by way of
> > + MSIs.
> > +- msi-controller: identifies the node as an MSI controller.
> > +
> > +Example:
> > +
> > + sei: sei at 3f0200 {
> > + compatible = "marvell,armada-8k-sei";
> > + reg = <0x3f0200 0x40>;
> > + interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>;
> > +
> > + sei_wired_controller: sei-wired-controller at 0 {
> > + marvell,sei-ranges = <0 21>;
> > + #interrupt-cells = <1>;
> > + interrupt-controller;
> > + };
> > +
> > + sei_msi_controller: sei-msi-controller at 21 {
> > + marvell,sei-ranges = <21 43>;
> > + msi-controller;
> > + };
>
> I still think this should just be all one node. There's several examples
> in the tree of nodes which are both interrupt-controller and
> msi-controller. Marvell MPIC is one example.
I checked Marvell MPIC example (armada 370 XP), it does not use
hierarchy domains, so I totally understand your point but I'm not sure
how I could get inspired by this driver (I'm looking for others).
Here I'm stuck. I know from a pure DT point of view the following is
not a valid argument. But from Linux, there is no easy way to handle
this situation without two different device nodes due to the internals
of the irqchip subsystem. There is simply no easy solution and having
only one node would require consequent changes in the core.
Maybe Marc will have an idea, but I think we already gave up on this
topic :/
Regards,
Miqu?l
^ permalink raw reply
* [PATCH v2 5/5] arm64: perf: Add support for chaining event counters
From: Suzuki K Poulose @ 2018-06-08 14:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180606180119.4ofhges6codarbmk@lakrids.cambridge.arm.com>
Hi Mark,
On 06/06/2018 07:01 PM, Mark Rutland wrote:
> On Tue, May 29, 2018 at 11:55:56AM +0100, Suzuki K Poulose wrote:
>> Add support for 64bit event by using chained event counters
>> and 64bit cycle counters.
>>
>> Arm v8 PMUv3 allows chaining a pair of adjacent PMU counters
>> (with the lower counter number being always "even"). The low
>> counter is programmed to count the event of interest and the
>> high counter(odd numbered) is programmed with a special event
>> code (0x1e - Chain).
>
> I found this a little difficult to read. How about:
>
> PMUv3 allows chaining a pair of adjacent 32-bit counters, effectively
> forming a 64-bit counter. The low/even counter is programmed to count
> the event of interest, and the high/odd counter is programmed to count
> the CHAIN event, taken when the low/even counter overflows.
>
Sure, that looks better.
>> Thus we need special allocation schemes
>> to make the full use of available counters. So, we allocate the
>> counters from either ends. i.e, chained counters are allocated
>> from the lower end in pairs of two and the normal counters are
>> allocated from the higher number. Also makes necessary changes to
>> handle the chained events as a single event with 2 counters.
>
> Why do we need to allocate in this way?
>
> I can see this might make allocating a pair of counters more likely in
> some cases, but there are still others where it wouldn't be possible
> (and we'd rely on the rotation logic to repack the counters for us).
It makes the efficient use of the counters in all cases and allows
counting maximum number of events with any given set, keeping the
precedence on the order of their "inserts".
e.g, if the number of counters happened to be "odd" (not sure if it is
even possible).
>
> [...]
>
>> +static inline u32 armv8pmu_read_evcntr(int idx)
>> +{
>> + return (armv8pmu_select_counter(idx) == idx) ?
>> + read_sysreg(pmxevcntr_el0) : 0;
>> +}
>
> Given armv8pmu_select_counter() always returns the idx passed to it, I
> don't think we need to check anything here. We can clean that up to be
> void, and remove the existing checks.
>
> [...]
OK.
>
>> +static inline u64 armv8pmu_read_chain_counter(int idx)
>> +{
>> + u64 prev_hi, hi, lo;
>> +
>> + hi = armv8pmu_read_evcntr(idx);
>> + do {
>> + prev_hi = hi;
>> + isb();
>> + lo = armv8pmu_read_evcntr(idx - 1);
>> + isb();
>> + hi = armv8pmu_read_evcntr(idx);
>> + } while (prev_hi != hi);
>> +
>> + return (hi << 32) | lo;
>> +}
>
>> +static inline void armv8pmu_write_chain_counter(int idx, u64 value)
>> +{
>> + armv8pmu_write_evcntr(idx, value >> 32);
>> + isb();
>> + armv8pmu_write_evcntr(idx - 1, value);
>> +}
>
> Can we use upper_32_bits() and lower_32_bits() here?
>
> As a more general thing, I think we need to clean up the way we
> read/write counters, because I don't think that it's right that we poke
> them while they're running -- that means you get some arbitrary skew on
> counter groups.
>
> It looks like the only case we do that is the IRQ handler, so we should
> be able to stop/start the PMU there.
Since we don't stop the "counting" of events usually when an IRQ is
triggered, the skew will be finally balanced when the events are stopped
in a the group. So, I think, stopping the PMU may not be really a good
thing after all. Just my thought.
>
> With that, we can get rid of the ISB here, and likewise in
> read_chain_counter, which wouldn't need to be a loop.
>
>> +
>> +static inline void armv8pmu_write_hw_counter(struct perf_event *event,
>> + u64 value)
>> +{
>> + int idx = event->hw.idx;
>> +
>> + if (armv8pmu_event_is_chained(event))
>> + armv8pmu_write_chain_counter(idx, value);
>> + else
>> + armv8pmu_write_evcntr(idx, value);
>> +}
>> +
>> static inline void armv8pmu_write_counter(struct perf_event *event, u64 value)
>> {
>> struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
>> @@ -541,14 +612,14 @@ static inline void armv8pmu_write_counter(struct perf_event *event, u64 value)
>> smp_processor_id(), idx);
>> else if (idx == ARMV8_IDX_CYCLE_COUNTER) {
>> /*
>> - * Set the upper 32bits as this is a 64bit counter but we only
>> - * count using the lower 32bits and we want an interrupt when
>> - * it overflows.
>> + * Set the upper 32bits if we are counting this in
>> + * 32bit mode, as this is a 64bit counter.
>> */
>
> It would be good to keep the explaination as to why.
>
Sure
>> - value |= 0xffffffff00000000ULL;
>> + if (!armv8pmu_event_is_64bit(event))
>> + value |= 0xffffffff00000000ULL;
>> write_sysreg(value, pmccntr_el0);
>> - } else if (armv8pmu_select_counter(idx) == idx)
>> - write_sysreg(value, pmxevcntr_el0);
>> + } else
>> + armv8pmu_write_hw_counter(event, value);
>> }
>
>> +static inline void armv8pmu_write_event_type(struct perf_event *event)
>> +{
>> + struct hw_perf_event *hwc = &event->hw;
>> + int idx = hwc->idx;
>> +
>> + /*
>> + * For chained events, write the the low counter event type
>> + * followed by the high counter. The high counter is programmed
>> + * with CHAIN event code with filters set to count at all ELs.
>> + */
>> + if (armv8pmu_event_is_chained(event)) {
>> + u32 chain_evt = ARMV8_PMUV3_PERFCTR_CHAIN |
>> + ARMV8_PMU_INCLUDE_EL2;
>> +
>> + armv8pmu_write_evtype(idx - 1, hwc->config_base);
>> + isb();
>> + armv8pmu_write_evtype(idx, chain_evt);
>
> The ISB isn't necessary here, AFAICT. We only do this while the PMU is
> disabled; no?
You're right. I was just following the ARM ARM.
>
>> + } else
>> + armv8pmu_write_evtype(idx, hwc->config_base);
>> +}
>
> [...]
>
>> @@ -679,6 +679,12 @@ static void cpu_pm_pmu_setup(struct arm_pmu *armpmu, unsigned long cmd)
>> continue;
>>
>> event = hw_events->events[idx];
>> + /*
>> + * If there is no event at this idx (e.g, an idx used
>> + * by a chained event in Arm v8 PMUv3), skip it.
>> + */
>> + if (!event)
>> + continue;
>
> We may as well lose the used_mask test if we're looking at the event
> regardless.
Ok
Suzuki
^ permalink raw reply
* [PATCH v2 5/5] arm64: perf: Add support for chaining event counters
From: Mark Rutland @ 2018-06-08 15:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4a5b5e7f-fc0b-84e3-fc65-b9f860029207@arm.com>
On Fri, Jun 08, 2018 at 03:46:57PM +0100, Suzuki K Poulose wrote:
> On 06/06/2018 07:01 PM, Mark Rutland wrote:
> > > Thus we need special allocation schemes
> > > to make the full use of available counters. So, we allocate the
> > > counters from either ends. i.e, chained counters are allocated
> > > from the lower end in pairs of two and the normal counters are
> > > allocated from the higher number. Also makes necessary changes to
> > > handle the chained events as a single event with 2 counters.
> >
> > Why do we need to allocate in this way?
> >
> > I can see this might make allocating a pair of counters more likely in
> > some cases, but there are still others where it wouldn't be possible
> > (and we'd rely on the rotation logic to repack the counters for us).
>
> It makes the efficient use of the counters in all cases and allows
> counting maximum number of events with any given set, keeping the precedence
> on the order of their "inserts".
> e.g, if the number of counters happened to be "odd" (not sure if it is even
> possible).
Unfortunately, that doesn't always work.
Say you have a system with 8 counters, and you open 8 (32-bit) events.
Then you close the events in counters 0, 2, 4, and 6. The live events
aren't moved, and stay where they are, in counters 1, 3, 5, and 7.
Now, say you open a 64-bit event. When we try to add it, we try to
allocate an index for two consecutive counters, and find that we can't,
despite 4 counters being free.
We return -EAGAIN to the core perf code, whereupon it removes any other
events in that group (none in this case).
Then we wait for the rotation logic to pull all of the events out and
schedule them back in, re-packing them, which should (eventually) work
regardless of how we allocate counters.
... we might need to re-pack events to solve that. :/
[...]
> > > +static inline void armv8pmu_write_chain_counter(int idx, u64 value)
> > > +{
> > > + armv8pmu_write_evcntr(idx, value >> 32);
> > > + isb();
> > > + armv8pmu_write_evcntr(idx - 1, value);
> > > +}
> >
> > Can we use upper_32_bits() and lower_32_bits() here?
> >
> > As a more general thing, I think we need to clean up the way we
> > read/write counters, because I don't think that it's right that we poke
> > them while they're running -- that means you get some arbitrary skew on
> > counter groups.
> >
> > It looks like the only case we do that is the IRQ handler, so we should
> > be able to stop/start the PMU there.
>
> Since we don't stop the "counting" of events usually when an IRQ is
> triggered, the skew will be finally balanced when the events are stopped
> in a the group. So, I think, stopping the PMU may not be really a good
> thing after all. Just my thought.
That's not quite right -- if one event in a group overflows, we'll
reprogram it *while* other events are counting, losing some events in
the process.
Stopping the PMU for the duration of the IRQ handler ensures that events
in a group are always in-sync with one another.
Thanks,
Mark.
^ permalink raw reply
* arm: mach-mvebu: dts: enable-method is always overwritten
From: Yves Lefloch @ 2018-06-08 15:43 UTC (permalink / raw)
To: linux-arm-kernel
Hello everybody,
I'm facing an issue that I believe to be a conflict between device-tree and
machine_desc.
My platform is arm/mach-mvebu. I have a DT based on "armada-xp-db-dxbc2.dts" (I
just included it and added a few okays), my CPU is a Marvell Bobcat2 switching
chip. My kernel is a vanilla 4.16.
Everything works fine except that my second core won't boot: `CPU1: failed to
come online'.
I tracked down the problem to arch/arm/mach-mvebu/platsmp.c: in this file is
defined a machine_desc that hardcodes the SMP ops to `marvell,armada-xp-smp'
whereas my device tree (by including armada-xp-98dx3236.dtsi) attempts to set
the ops to `marvell,98dx3236-smp' through enable-method. In setup_arch() the
machine_desc's ops overwrites the enable-method's ops, causing the wrong
smp_boot_secondary() call to be issued.
Now there is a note from 2014 saying that this machine_desc's `smp' field is
hardcoded like that because of "old Device Trees that were not specifying the
cpus enable-method property". As far as I can tell, this is still the case, for
instance "armada-370-db.dts" doesn't have any enable-method property.
I have worked around this by commenting out `armada_xp_smp_ops.smp' but
obviously I would prefer to keep a vanilla kernel.
So I propose to:
- Add `enable-method = "marvell,armada-xp-smp"' to armada-370-xp.dtsi, because
it seems that all Armada 370/XP include it;
- Remove the `smp' field of `armada_xp_smp_ops'.
If you agree with the diagnosis and the proposed fix I will write a patch.
Regards,
Yves Lefloch.
^ permalink raw reply
* [PATCH v4 1/7] interconnect: Add generic on-chip interconnect API
From: Alexandre Bailon @ 2018-06-08 15:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180309210958.16672-2-georgi.djakov@linaro.org>
On 03/09/2018 10:09 PM, Georgi Djakov wrote:
> This patch introduce a new API to get requirements and configure the
> interconnect buses across the entire chipset to fit with the current
> demand.
>
> The API is using a consumer/provider-based model, where the providers are
> the interconnect buses and the consumers could be various drivers.
> The consumers request interconnect resources (path) between endpoints and
> set the desired constraints on this data flow path. The providers receive
> requests from consumers and aggregate these requests for all master-slave
> pairs on that path. Then the providers configure each participating in the
> topology node according to the requested data flow path, physical links and
> constraints. The topology could be complicated and multi-tiered and is SoC
> specific.
>
> Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
> ---
> Documentation/interconnect/interconnect.rst | 96 ++++++
> drivers/Kconfig | 2 +
> drivers/Makefile | 1 +
> drivers/interconnect/Kconfig | 10 +
> drivers/interconnect/Makefile | 1 +
> drivers/interconnect/core.c | 489 ++++++++++++++++++++++++++++
> include/linux/interconnect-provider.h | 109 +++++++
> include/linux/interconnect.h | 40 +++
> 8 files changed, 748 insertions(+)
> create mode 100644 Documentation/interconnect/interconnect.rst
> create mode 100644 drivers/interconnect/Kconfig
> create mode 100644 drivers/interconnect/Makefile
> create mode 100644 drivers/interconnect/core.c
> create mode 100644 include/linux/interconnect-provider.h
> create mode 100644 include/linux/interconnect.h
>
> diff --git a/Documentation/interconnect/interconnect.rst b/Documentation/interconnect/interconnect.rst
> new file mode 100644
> index 000000000000..23eba68e8424
> --- /dev/null
> +++ b/Documentation/interconnect/interconnect.rst
> @@ -0,0 +1,96 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +=====================================
> +GENERIC SYSTEM INTERCONNECT SUBSYSTEM
> +=====================================
> +
> +Introduction
> +------------
> +
> +This framework is designed to provide a standard kernel interface to control
> +the settings of the interconnects on a SoC. These settings can be throughput,
> +latency and priority between multiple interconnected devices or functional
> +blocks. This can be controlled dynamically in order to save power or provide
> +maximum performance.
> +
> +The interconnect bus is a hardware with configurable parameters, which can be
> +set on a data path according to the requests received from various drivers.
> +An example of interconnect buses are the interconnects between various
> +components or functional blocks in chipsets. There can be multiple interconnects
> +on a SoC that can be multi-tiered.
> +
> +Below is a simplified diagram of a real-world SoC interconnect bus topology.
> +
> +::
> +
> + +----------------+ +----------------+
> + | HW Accelerator |--->| M NoC |<---------------+
> + +----------------+ +----------------+ |
> + | | +------------+
> + +-----+ +-------------+ V +------+ | |
> + | DDR | | +--------+ | PCIe | | |
> + +-----+ | | Slaves | +------+ | |
> + ^ ^ | +--------+ | | C NoC |
> + | | V V | |
> + +------------------+ +------------------------+ | | +-----+
> + | |-->| |-->| |-->| CPU |
> + | |-->| |<--| | +-----+
> + | Mem NoC | | S NoC | +------------+
> + | |<--| |---------+ |
> + | |<--| |<------+ | | +--------+
> + +------------------+ +------------------------+ | | +-->| Slaves |
> + ^ ^ ^ ^ ^ | | +--------+
> + | | | | | | V
> + +------+ | +-----+ +-----+ +---------+ +----------------+ +--------+
> + | CPUs | | | GPU | | DSP | | Masters |-->| P NoC |-->| Slaves |
> + +------+ | +-----+ +-----+ +---------+ +----------------+ +--------+
> + |
> + +-------+
> + | Modem |
> + +-------+
> +
> +Terminology
> +-----------
> +
> +Interconnect provider is the software definition of the interconnect hardware.
> +The interconnect providers on the above diagram are M NoC, S NoC, C NoC and Mem
> +NoC.
> +
> +Interconnect node is the software definition of the interconnect hardware
> +port. Each interconnect provider consists of multiple interconnect nodes,
> +which are connected to other SoC components including other interconnect
> +providers. The point on the diagram where the CPUs connects to the memory is
> +called an interconnect node, which belongs to the Mem NoC interconnect provider.
> +
> +Interconnect endpoints are the first or the last element of the path. Every
> +endpoint is a node, but not every node is an endpoint.
> +
> +Interconnect path is everything between two endpoints including all the nodes
> +that have to be traversed to reach from a source to destination node. It may
> +include multiple master-slave pairs across several interconnect providers.
> +
> +Interconnect consumers are the entities which make use of the data paths exposed
> +by the providers. The consumers send requests to providers requesting various
> +throughput, latency and priority. Usually the consumers are device drivers, that
> +send request based on their needs. An example for a consumer is a video decoder
> +that supports various formats and image sizes.
> +
> +Interconnect providers
> +----------------------
> +
> +Interconnect provider is an entity that implements methods to initialize and
> +configure a interconnect bus hardware. The interconnect provider drivers should
> +be registered with the interconnect provider core.
> +
> +The interconnect framework provider API functions are documented in
> +.. kernel-doc:: include/linux/interconnect-provider.h
> +
> +Interconnect consumers
> +----------------------
> +
> +Interconnect consumers are the clients which use the interconnect APIs to
> +get paths between endpoints and set their bandwidth/latency/QoS requirements
> +for these interconnect paths.
> +
> +The interconnect framework consumer API functions are documented in
> +.. kernel-doc:: include/linux/interconnect.h
> diff --git a/drivers/Kconfig b/drivers/Kconfig
> index 879dc0604cba..96a1db022cee 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -219,4 +219,6 @@ source "drivers/siox/Kconfig"
>
> source "drivers/slimbus/Kconfig"
>
> +source "drivers/interconnect/Kconfig"
> +
> endmenu
> diff --git a/drivers/Makefile b/drivers/Makefile
> index 24cd47014657..0cca95740d9b 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -185,3 +185,4 @@ obj-$(CONFIG_TEE) += tee/
> obj-$(CONFIG_MULTIPLEXER) += mux/
> obj-$(CONFIG_UNISYS_VISORBUS) += visorbus/
> obj-$(CONFIG_SIOX) += siox/
> +obj-$(CONFIG_INTERCONNECT) += interconnect/
> diff --git a/drivers/interconnect/Kconfig b/drivers/interconnect/Kconfig
> new file mode 100644
> index 000000000000..a261c7d41deb
> --- /dev/null
> +++ b/drivers/interconnect/Kconfig
> @@ -0,0 +1,10 @@
> +menuconfig INTERCONNECT
> + tristate "On-Chip Interconnect management support"
> + help
> + Support for management of the on-chip interconnects.
> +
> + This framework is designed to provide a generic interface for
> + managing the interconnects in a SoC.
> +
> + If unsure, say no.
> +
> diff --git a/drivers/interconnect/Makefile b/drivers/interconnect/Makefile
> new file mode 100644
> index 000000000000..5edf0ae80818
> --- /dev/null
> +++ b/drivers/interconnect/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_INTERCONNECT) += core.o
> diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c
> new file mode 100644
> index 000000000000..6306e258b9b9
> --- /dev/null
> +++ b/drivers/interconnect/core.c
> @@ -0,0 +1,489 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Interconnect framework core driver
> + *
> + * Copyright (c) 2018, Linaro Ltd.
> + * Author: Georgi Djakov <georgi.djakov@linaro.org>
> + */
> +
> +#include <linux/device.h>
> +#include <linux/idr.h>
> +#include <linux/init.h>
> +#include <linux/interconnect.h>
> +#include <linux/interconnect-provider.h>
> +#include <linux/list.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/slab.h>
> +
> +static DEFINE_IDR(icc_idr);
> +static LIST_HEAD(icc_provider_list);
> +static DEFINE_MUTEX(icc_provider_list_mutex);
> +static DEFINE_MUTEX(icc_path_mutex);
> +
> +/**
> + * struct icc_req - constraints that are attached to each node
> + *
> + * @req_node: entry in list of requests for the particular @node
> + * @node: the interconnect node to which this constraint applies
> + * @avg_bw: an integer describing the average bandwidth in kbps
> + * @peak_bw: an integer describing the peak bandwidth in kbps
> + */
> +struct icc_req {
> + struct hlist_node req_node;
> + struct icc_node *node;
> + u32 avg_bw;
> + u32 peak_bw;
> +};
> +
> +/**
> + * struct icc_path - interconnect path structure
> + * @num_nodes: number of hops (nodes)
> + * @reqs: array of the requests applicable to this path of nodes
> + */
> +struct icc_path {
> + size_t num_nodes;
> + struct icc_req reqs[0];
> +};
> +
> +static struct icc_node *node_find(const int id)
> +{
> + struct icc_node *node;
> +
> + node = idr_find(&icc_idr, id);
> +
> + return node;
> +}
> +
> +static struct icc_path *path_allocate(struct icc_node *node, ssize_t num_nodes)
> +{
> + struct icc_path *path;
> + size_t i;
> +
> + path = kzalloc(sizeof(*path) + num_nodes * sizeof(*path->reqs),
> + GFP_KERNEL);
> + if (!path)
> + return ERR_PTR(-ENOMEM);
> +
> + path->num_nodes = num_nodes;
> +
> + for (i = 0; i < num_nodes; i++) {
> + hlist_add_head(&path->reqs[i].req_node, &node->req_list);
> +
> + path->reqs[i].node = node;
> + /* reference to previous node was saved during path traversal */
> + node = node->reverse;
> + }
> +
> + return path;
> +}
> +
> +static struct icc_path *path_find(struct icc_node *src, struct icc_node *dst)
> +{
> + struct icc_node *node = NULL;
> + struct list_head traverse_list;
> + struct list_head edge_list;
> + struct list_head tmp_list;
> + size_t i, number = 0;
> + bool found = false;
> +
> + INIT_LIST_HEAD(&traverse_list);
> + INIT_LIST_HEAD(&edge_list);
> + INIT_LIST_HEAD(&tmp_list);
> +
> + list_add_tail(&src->search_list, &traverse_list);
> +
> + do {
> + list_for_each_entry(node, &traverse_list, search_list) {
> + if (node == dst) {
> + found = true;
> + list_add(&node->search_list, &tmp_list);
> + break;
> + }
> + for (i = 0; i < node->num_links; i++) {
> + struct icc_node *tmp = node->links[i];
> +
> + if (!tmp)
> + return ERR_PTR(-ENOENT);
> +
> + if (tmp->is_traversed)
> + continue;
> +
> + tmp->is_traversed = true;
> + tmp->reverse = node;
> + list_add_tail(&tmp->search_list, &edge_list);
> + }
> + }
> + if (found)
> + break;
> +
> + list_splice_init(&traverse_list, &tmp_list);
> + list_splice_init(&edge_list, &traverse_list);
> +
> + /* count the number of nodes */
> + number++;
> +
> + } while (!list_empty(&traverse_list));
> +
> + /* reset the traversed state */
> + list_for_each_entry(node, &tmp_list, search_list)
> + node->is_traversed = false;
> +
> + if (found)
> + return path_allocate(dst, number);
> +
> + return ERR_PTR(-EPROBE_DEFER);
> +}
> +
> +static int path_init(struct icc_path *path)
> +{
> + struct icc_node *node;
> + size_t i;
> +
> + for (i = 0; i < path->num_nodes; i++) {
> + node = path->reqs[i].node;
> +
> + mutex_lock(&node->provider->lock);
> + node->provider->users++;
> + mutex_unlock(&node->provider->lock);
> + }
> +
> + return 0;
> +}
> +
> +static void node_aggregate(struct icc_node *node)
> +{
> + struct icc_req *r;
> + u32 agg_avg = 0;
> + u32 agg_peak = 0;
> +
> + hlist_for_each_entry(r, &node->req_list, req_node) {
> + /* sum(averages) and max(peaks) */
> + agg_avg += r->avg_bw;
> + agg_peak = max(agg_peak, r->peak_bw);
> + }
> +
> + node->avg_bw = agg_avg;
> + node->peak_bw = agg_peak;
> +}
> +
> +static void provider_aggregate(struct icc_provider *provider, u32 *avg_bw,
> + u32 *peak_bw)
> +{
> + struct icc_node *n;
> + u32 agg_avg = 0;
> + u32 agg_peak = 0;
> +
> + /* aggregate for the interconnect provider */
> + list_for_each_entry(n, &provider->nodes, node_list) {
> + /* sum the average and max the peak */
> + agg_avg += n->avg_bw;
> + agg_peak = max(agg_peak, n->peak_bw);
> + }
> +
> + *avg_bw = agg_avg;
> + *peak_bw = agg_peak;
> +}
> +
> +static int constraints_apply(struct icc_path *path)
> +{
> + struct icc_node *next, *prev = NULL;
> + int i;
> +
> + for (i = 0; i < path->num_nodes; i++, prev = next) {
> + struct icc_provider *provider;
> + u32 avg_bw = 0;
> + u32 peak_bw = 0;
> + int ret;
> +
> + next = path->reqs[i].node;
> + /*
> + * Both endpoints should be valid master-slave pairs of the
> + * same interconnect provider that will be configured.
> + */
> + if (!next || !prev)
> + continue;
> +
> + if (next->provider != prev->provider)
> + continue;
> +
> + provider = next->provider;
> + mutex_lock(&provider->lock);
> +
> + /* aggregate requests for the provider */
> + provider_aggregate(provider, &avg_bw, &peak_bw);
> +
> + if (provider->set) {
> + /* set the constraints */
> + ret = provider->set(prev, next, avg_bw, peak_bw);
> + }
> +
> + mutex_unlock(&provider->lock);
> +
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +/**
> + * icc_set() - set constraints on an interconnect path between two endpoints
> + * @path: reference to the path returned by icc_get()
> + * @avg_bw: average bandwidth in kbps
> + * @peak_bw: peak bandwidth in kbps
> + *
> + * This function is used by an interconnect consumer to express its own needs
> + * in term of bandwidth and QoS for a previously requested path between two
> + * endpoints. The requests are aggregated and each node is updated accordingly.
> + *
> + * Returns 0 on success, or an approproate error code otherwise.
> + */
> +int icc_set(struct icc_path *path, u32 avg_bw, u32 peak_bw)
> +{
> + struct icc_node *node;
> + size_t i;
> + int ret;
> +
> + if (!path)
> + return 0;
> +
> + for (i = 0; i < path->num_nodes; i++) {
> + node = path->reqs[i].node;
> +
> + mutex_lock(&icc_path_mutex);
> +
> + /* update the consumer request for this path */
> + path->reqs[i].avg_bw = avg_bw;
> + path->reqs[i].peak_bw = peak_bw;
> +
> + /* aggregate requests for this node */
> + node_aggregate(node);
> +
> + mutex_unlock(&icc_path_mutex);
> + }
> +
> + ret = constraints_apply(path);
> + if (ret)
> + pr_err("interconnect: error applying constraints (%d)", ret);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(icc_set);
> +
> +/**
> + * icc_get() - return a handle for path between two endpoints
> + * @src_id: source device port id
> + * @dst_id: destination device port id
> + *
> + * This function will search for a path between two endpoints and return an
> + * icc_path handle on success. Use icc_put() to release
> + * constraints when the they are not needed anymore.
> + *
> + * Return: icc_path pointer on success, or ERR_PTR() on error
> + */
> +struct icc_path *icc_get(const int src_id, const int dst_id)
> +{
> + struct icc_node *src, *dst;
> + struct icc_path *path = ERR_PTR(-EPROBE_DEFER);
> +
> + src = node_find(src_id);
> + if (!src)
> + goto out;
> +
> + dst = node_find(dst_id);
> + if (!dst)
> + goto out;
> +
> + mutex_lock(&icc_path_mutex);
> + path = path_find(src, dst);
> + mutex_unlock(&icc_path_mutex);
> + if (IS_ERR(path))
> + goto out;
> +
> + path_init(path);
> +
> +out:
> + return path;
> +}
> +EXPORT_SYMBOL_GPL(icc_get);
> +
> +/**
> + * icc_put() - release the reference to the icc_path
> + * @path: interconnect path
> + *
> + * Use this function to release the constraints on a path when the path is
> + * no longer needed. The constraints will be re-aggregated.
> + */
> +void icc_put(struct icc_path *path)
> +{
> + struct icc_node *node;
> + size_t i;
> + int ret;
> +
> + if (!path || WARN_ON_ONCE(IS_ERR(path)))
> + return;
> +
> + ret = icc_set(path, 0, 0);
> + if (ret)
> + pr_err("%s: error (%d)\n", __func__, ret);
> +
> + for (i = 0; i < path->num_nodes; i++) {
> + node = path->reqs[i].node;
> + hlist_del(&path->reqs[i].req_node);
> +
> + mutex_lock(&node->provider->lock);
> + node->provider->users--;
> + mutex_unlock(&node->provider->lock);
> + }
> +
> + kfree(path);
> +}
> +EXPORT_SYMBOL_GPL(icc_put);
> +
> +/**
> + * icc_node_create() - create a node
> + * @id: node id
> + *
> + * Return: icc_node pointer on success, or ERR_PTR() on error
> + */
> +struct icc_node *icc_node_create(int id)
> +{
> + struct icc_node *node;
> +
> + /* check if node already exists */
> + node = node_find(id);
> + if (node)
> + return node;
> +
> + node = kzalloc(sizeof(*node), GFP_KERNEL);
> + if (!node)
> + return ERR_PTR(-ENOMEM);
> +
> + id = idr_alloc(&icc_idr, node, id, id + 1, GFP_KERNEL);
> + if (WARN(id < 0, "couldn't get idr"))
> + return ERR_PTR(id);
> +
> + node->id = id;
> +
> + return node;
> +}
> +EXPORT_SYMBOL_GPL(icc_node_create);
> +
> +/**
> + * icc_link_create() - create a link between two nodes
> + * @src_id: source node id
I guess src_id has become node and is not an id anymore,
so it should be updated.
> + * @dst_id: destination node id
> + *
> + * Return: 0 on success, or an error code otherwise
> + */
> +int icc_link_create(struct icc_node *node, const int dst_id)
> +{
> + struct icc_node *dst;
> + struct icc_node **new;
> + int ret = 0;
> +
> + if (IS_ERR_OR_NULL(node))
> + return PTR_ERR(node);
> +
> + mutex_lock(&node->provider->lock);
> +
> + dst = node_find(dst_id);
> + if (!dst)
> + dst = icc_node_create(dst_id);
> +
> + new = krealloc(node->links,
> + (node->num_links + 1) * sizeof(*node->links),
> + GFP_KERNEL);
> + if (!new) {
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> + node->links = new;
> + node->links[node->num_links++] = dst;
> +
> +out:
> + mutex_unlock(&node->provider->lock);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(icc_link_create);
> +
> +/**
> + * icc_add_node() - add an interconnect node to interconnect provider
> + * @node: pointer to the interconnect node
> + * @provider: pointer to the interconnect provider
> + *
> + * Return: 0 on success, or an error code otherwise
> + */
> +int icc_node_add(struct icc_node *node, struct icc_provider *provider)
> +{
> + if (WARN_ON(!node))
> + return -EINVAL;
> +
> + if (WARN_ON(!provider))
> + return -EINVAL;
> +
> + node->provider = provider;
> +
> + mutex_lock(&provider->lock);
> + list_add_tail(&node->node_list, &provider->nodes);
> + mutex_unlock(&provider->lock);
> +
> + return 0;
> +}
> +
> +/**
> + * icc_add_provider() - add a new interconnect provider
> + * @icc_provider: the interconnect provider that will be added into topology
> + *
> + * Return: 0 on success, or an error code otherwise
> + */
> +int icc_add_provider(struct icc_provider *provider)
> +{
> + if (WARN_ON(!provider))
> + return -EINVAL;
> +
> + if (WARN_ON(!provider->set))
> + return -EINVAL;
> +
> + mutex_init(&provider->lock);
> + INIT_LIST_HEAD(&provider->nodes);
> +
> + mutex_lock(&icc_provider_list_mutex);
> + list_add(&provider->provider_list, &icc_provider_list);
> + mutex_unlock(&icc_provider_list_mutex);
> +
> + dev_dbg(provider->dev, "interconnect provider added to topology\n");
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(icc_add_provider);
> +
> +/**
> + * icc_del_provider() - delete previously added interconnect provider
> + * @icc_provider: the interconnect provider that will be removed from topology
> + *
> + * Return: 0 on success, or an error code otherwise
> + */
> +int icc_del_provider(struct icc_provider *provider)
> +{
> + mutex_lock(&provider->lock);
> + if (provider->users) {
> + pr_warn("interconnect provider still has %d users\n",
> + provider->users);
> + }
> + mutex_unlock(&provider->lock);
> +
> + mutex_lock(&icc_provider_list_mutex);
> + list_del(&provider->provider_list);
> + mutex_unlock(&icc_provider_list_mutex);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(icc_del_provider);
> +
> +MODULE_AUTHOR("Georgi Djakov <georgi.djakov@linaro.org");
> +MODULE_DESCRIPTION("Interconnect Driver Core");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/interconnect-provider.h b/include/linux/interconnect-provider.h
> new file mode 100644
> index 000000000000..779b5b5b1306
> --- /dev/null
> +++ b/include/linux/interconnect-provider.h
> @@ -0,0 +1,109 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2018, Linaro Ltd.
> + * Author: Georgi Djakov <georgi.djakov@linaro.org>
> + */
> +
> +#ifndef _LINUX_INTERCONNECT_PROVIDER_H
> +#define _LINUX_INTERCONNECT_PROVIDER_H
> +
> +#include <linux/interconnect.h>
> +
> +struct icc_node;
> +
> +/**
> + * struct icc_provider - interconnect provider (controller) entity that might
> + * provide multiple interconnect controls
> + *
> + * @provider_list: list of the registered interconnect providers
> + * @nodes: internal list of the interconnect provider nodes
> + * @set: pointer to device specific set operation function
> + * @dev: the device this interconnect provider belongs to
> + * @lock: lock to provide consistency during aggregation/update of constraints
> + * @users: count of active users
> + * @data: pointer to private data
> + */
> +struct icc_provider {
> + struct list_head provider_list;
> + struct list_head nodes;
> + int (*set)(struct icc_node *src, struct icc_node *dst,
> + u32 avg_bw, u32 peak_bw);
> + struct device *dev;
> + struct mutex lock;
> + int users;
> + void *data;
> +};
> +
> +/**
> + * struct icc_node - entity that is part of the interconnect topology
> + *
> + * @id: platform specific node id
> + * @name: node name used in debugfs
> + * @links: a list of targets where we can go next when traversing
> + * @num_links: number of links to other interconnect nodes
> + * @provider: points to the interconnect provider of this node
> + * @node_list: list of interconnect nodes associated with @provider
> + * @search_list: list used when walking the nodes graph
> + * @reverse: pointer to previous node when walking the nodes graph
> + * @is_traversed: flag that is used when walking the nodes graph
> + * @req_list: a list of QoS constraint requests associated with this node
> + * @avg_bw: aggregated value of average bandwidth
> + * @peak_bw: aggregated value of peak bandwidth
> + * @data: pointer to private data
> + */
> +struct icc_node {
> + int id;
> + const char *name;
> + struct icc_node **links;
> + size_t num_links;
> +
> + struct icc_provider *provider;
> + struct list_head node_list;
> + struct list_head orphan_list;
> + struct list_head search_list;
> + struct icc_node *reverse;
> + bool is_traversed;
> + struct hlist_head req_list;
> + u32 avg_bw;
> + u32 peak_bw;
> + void *data;
> +};
> +
> +#if IS_ENABLED(CONFIG_INTERCONNECT)
> +
> +struct icc_node *icc_node_create(int id);
> +int icc_node_add(struct icc_node *node, struct icc_provider *provider);
> +int icc_link_create(struct icc_node *node, const int dst_id);
> +int icc_add_provider(struct icc_provider *provider);
> +int icc_del_provider(struct icc_provider *provider);
> +
> +#else
> +
> +static inline struct icc_node *icc_node_create(int id)
> +{
> + return ERR_PTR(-ENOTSUPP);
> +}
> +
> +int icc_node_add(struct icc_node *node, struct icc_provider *provider)
> +{
> + return -ENOTSUPP;
> +}
> +
> +static inline int icc_link_create(struct icc_node *node, const int dst_id)
> +{
> + return -ENOTSUPP;
> +}
> +
> +static inline int icc_add_provider(struct icc_provider *provider)
> +{
> + return -ENOTSUPP;
> +}
> +
> +static inline int icc_del_provider(struct icc_provider *provider)
> +{
> + return -ENOTSUPP;
> +}
> +
> +#endif /* CONFIG_INTERCONNECT */
> +
> +#endif /* _LINUX_INTERCONNECT_PROVIDER_H */
> diff --git a/include/linux/interconnect.h b/include/linux/interconnect.h
> new file mode 100644
> index 000000000000..5a7cf72b76a5
> --- /dev/null
> +++ b/include/linux/interconnect.h
> @@ -0,0 +1,40 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2018, Linaro Ltd.
> + * Author: Georgi Djakov <georgi.djakov@linaro.org>
> + */
> +
> +#ifndef _LINUX_INTERCONNECT_H
> +#define _LINUX_INTERCONNECT_H
> +
> +#include <linux/types.h>
> +#include <linux/mutex.h>
> +
> +struct icc_path;
> +struct device;
> +
> +#if IS_ENABLED(CONFIG_INTERCONNECT)
> +
> +struct icc_path *icc_get(const int src_id, const int dst_id);
> +void icc_put(struct icc_path *path);
> +int icc_set(struct icc_path *path, u32 avg_bw, u32 peak_bw);
> +
> +#else
> +
> +static inline struct icc_path *icc_get(const int src_id, const int dst_id)
> +{
> + return NULL;
> +}
> +
> +static inline void icc_put(struct icc_path *path)
> +{
> +}
> +
> +static inline int icc_set(struct icc_path *path, u32 avg_bw, u32 peak_bw)
> +{
> + return 0;
> +}
> +
> +#endif /* CONFIG_INTERCONNECT */
> +
> +#endif /* _LINUX_INTERCONNECT_H */
>
^ permalink raw reply
* arm: mach-mvebu: dts: enable-method is always overwritten
From: Andrew Lunn @ 2018-06-08 15:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <a2d4701e-28b3-07ee-6b2f-588b2e9a75c4@online.net>
On Fri, Jun 08, 2018 at 05:43:11PM +0200, Yves Lefloch wrote:
> Hello everybody,
Adding Chris Packham. He did some work in this area.
Andrew
>
> I'm facing an issue that I believe to be a conflict between device-tree and
> machine_desc.
>
> My platform is arm/mach-mvebu. I have a DT based on "armada-xp-db-dxbc2.dts"
> (I just included it and added a few okays), my CPU is a Marvell Bobcat2
> switching chip. My kernel is a vanilla 4.16.
>
> Everything works fine except that my second core won't boot: `CPU1: failed
> to come online'.
> I tracked down the problem to arch/arm/mach-mvebu/platsmp.c: in this file is
> defined a machine_desc that hardcodes the SMP ops to `marvell,armada-xp-smp'
> whereas my device tree (by including armada-xp-98dx3236.dtsi) attempts to
> set the ops to `marvell,98dx3236-smp' through enable-method. In setup_arch()
> the machine_desc's ops overwrites the enable-method's ops, causing the wrong
> smp_boot_secondary() call to be issued.
>
> Now there is a note from 2014 saying that this machine_desc's `smp' field is
> hardcoded like that because of "old Device Trees that were not specifying
> the cpus enable-method property". As far as I can tell, this is still the
> case, for instance "armada-370-db.dts" doesn't have any enable-method
> property.
>
> I have worked around this by commenting out `armada_xp_smp_ops.smp' but
> obviously I would prefer to keep a vanilla kernel.
>
> So I propose to:
> - Add `enable-method = "marvell,armada-xp-smp"' to armada-370-xp.dtsi,
> because it seems that all Armada 370/XP include it;
> - Remove the `smp' field of `armada_xp_smp_ops'.
>
> If you agree with the diagnosis and the proposed fix I will write a patch.
>
> Regards,
> Yves Lefloch.
^ permalink raw reply
* [PATCH v2 5/5] arm64: perf: Add support for chaining event counters
From: Suzuki K Poulose @ 2018-06-08 16:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180608152419.kamu7ptmgsoah5m3@lakrids.cambridge.arm.com>
On 08/06/18 16:24, Mark Rutland wrote:
> On Fri, Jun 08, 2018 at 03:46:57PM +0100, Suzuki K Poulose wrote:
>> On 06/06/2018 07:01 PM, Mark Rutland wrote:
>>>> Thus we need special allocation schemes
>>>> to make the full use of available counters. So, we allocate the
>>>> counters from either ends. i.e, chained counters are allocated
>>>> from the lower end in pairs of two and the normal counters are
>>>> allocated from the higher number. Also makes necessary changes to
>>>> handle the chained events as a single event with 2 counters.
>>>
>>> Why do we need to allocate in this way?
>>>
>>> I can see this might make allocating a pair of counters more likely in
>>> some cases, but there are still others where it wouldn't be possible
>>> (and we'd rely on the rotation logic to repack the counters for us).
>>
>> It makes the efficient use of the counters in all cases and allows
>> counting maximum number of events with any given set, keeping the precedence
>> on the order of their "inserts".
>> e.g, if the number of counters happened to be "odd" (not sure if it is even
>> possible).
>
> Unfortunately, that doesn't always work.
>
> Say you have a system with 8 counters, and you open 8 (32-bit) events.
I was talking about the following (imaginary) case :
We have 7 counters, and you have 5 32bit counters and 1 64bit counter.
Without the above scheme, you would place first 5 events on the first
5 counters and then you can't place the 64bit counter, as you are now
left with a low/odd counter and a high/even counter.
>
> Then you close the events in counters 0, 2, 4, and 6. The live events
> aren't moved, and stay where they are, in counters 1, 3, 5, and 7.
>
> Now, say you open a 64-bit event. When we try to add it, we try to
> allocate an index for two consecutive counters, and find that we can't,
> despite 4 counters being free.
>
> We return -EAGAIN to the core perf code, whereupon it removes any other
> events in that group (none in this case).
>
> Then we wait for the rotation logic to pull all of the events out and
> schedule them back in, re-packing them, which should (eventually) work
> regardless of how we allocate counters.
>
> ... we might need to re-pack events to solve that. :/
I agree, removing and putting them back in is not going to work unless
we re-pack or delay allocating the counters until we start the PMU.
>
> [...]
>
>>>> +static inline void armv8pmu_write_chain_counter(int idx, u64 value)
>>>> +{
>>>> + armv8pmu_write_evcntr(idx, value >> 32);
>>>> + isb();
>>>> + armv8pmu_write_evcntr(idx - 1, value);
>>>> +}
>>>
>>> Can we use upper_32_bits() and lower_32_bits() here?
>>>
>>> As a more general thing, I think we need to clean up the way we
>>> read/write counters, because I don't think that it's right that we poke
>>> them while they're running -- that means you get some arbitrary skew on
>>> counter groups.
>>>
>>> It looks like the only case we do that is the IRQ handler, so we should
>>> be able to stop/start the PMU there.
>>
>> Since we don't stop the "counting" of events usually when an IRQ is
>> triggered, the skew will be finally balanced when the events are stopped
>> in a the group. So, I think, stopping the PMU may not be really a good
>> thing after all. Just my thought.
>
> That's not quite right -- if one event in a group overflows, we'll
> reprogram it *while* other events are counting, losing some events in
> the process.
Oh yes, you're right. I can fix it.
>
> Stopping the PMU for the duration of the IRQ handler ensures that events
> in a group are always in-sync with one another.
Suzuki
^ permalink raw reply
* [GIT PULL] arm64 patches for 4.18
From: Catalin Marinas @ 2018-06-08 16:40 UTC (permalink / raw)
To: linux-arm-kernel
Hi Linus,
Please pull the arm64 updates for 4.18 below. Apart from the core arm64
and perf changes, the Spectre v4 mitigation touches the arm KVM code and
the ACPI PPTT support touches drivers/ (acpi and cacheinfo). I should
have the maintainers' acks in place.
Thanks.
The following changes since commit 75bc37fefc4471e718ba8e651aa74673d4e0a9eb:
Linux 4.17-rc4 (2018-05-06 16:57:38 -1000)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux tags/arm64-upstream
for you to fetch changes up to 0fe42512b2f03f9e5a20b9f55ef1013a68b4cd48:
arm64: Fix syscall restarting around signal suppressed by tracer (2018-06-08 13:21:39 +0100)
----------------------------------------------------------------
arm64 updates for 4.18:
- Spectre v4 mitigation (Speculative Store Bypass Disable) support for
arm64 using SMC firmware call to set a hardware chicken bit
- ACPI PPTT (Processor Properties Topology Table) parsing support and
enable the feature for arm64
- Report signal frame size to user via auxv (AT_MINSIGSTKSZ). The
primary motivation is Scalable Vector Extensions which requires more
space on the signal frame than the currently defined MINSIGSTKSZ
- ARM perf patches: allow building arm-cci as module, demote dev_warn()
to dev_dbg() in arm-ccn event_init(), miscellaneous cleanups
- cmpwait() WFE optimisation to avoid some spurious wakeups
- L1_CACHE_BYTES reverted back to 64 (for performance reasons that have
to do with some network allocations) while keeping ARCH_DMA_MINALIGN
to 128. cache_line_size() returns the actual hardware Cache Writeback
Granule
- Turn LSE atomics on by default in Kconfig
- Kernel fault reporting tidying
- Some #include and miscellaneous cleanups
----------------------------------------------------------------
Arnd Bergmann (3):
drivers/bus: arm-cci: fix build warnings
ARM: mcpm, perf/arm-cci: export mcpm_is_available
arm64: cpu_errata: include required headers
Catalin Marinas (4):
Revert "arm64: Increase the max granular size"
arm64: Increase ARCH_DMA_MINALIGN to 128
Merge branch 'for-next/perf' of git://git.kernel.org/.../will/linux
arm64: KVM: Move VCPU_WORKAROUND_2_FLAG macros to the top of the file
Dave Martin (4):
arm64/sve: Write ZCR_EL1 on context switch only if changed
arm64/sve: Thin out initialisation sanity-checks for sve_max_vl
arm64: signal: Report signal frame size to userspace via auxv
arm64: Fix syscall restarting around signal suppressed by tracer
Jeremy Linton (13):
drivers: base: cacheinfo: move cache_setup_of_node()
drivers: base: cacheinfo: setup DT cache properties early
cacheinfo: rename of_node to fw_token
arm64/acpi: Create arch specific cpu to acpi id helper
ACPI/PPTT: Add Processor Properties Topology Table parsing
ACPI: Enable PPTT support on ARM64
drivers: base cacheinfo: Add support for ACPI based firmware tables
arm64: Add support for ACPI based firmware tables
arm64: topology: rename cluster_id
arm64: topology: enable ACPI/PPTT based CPU topology
ACPI: Add PPTT to injectable table list
arm64: topology: divorce MC scheduling domain from core_siblings
arm64: topology: Avoid checking numa mask for scheduler MC selection
John Garry (1):
drivers/perf: Remove ARM_SPE_PMU explicit PERF_EVENTS dependency
Marc Zyngier (14):
arm/arm64: smccc: Add SMCCC-specific return codes
arm64: Call ARCH_WORKAROUND_2 on transitions between EL0 and EL1
arm64: Add per-cpu infrastructure to call ARCH_WORKAROUND_2
arm64: Add ARCH_WORKAROUND_2 probing
arm64: Add 'ssbd' command-line option
arm64: ssbd: Add global mitigation state accessor
arm64: ssbd: Skip apply_ssbd if not using dynamic mitigation
arm64: ssbd: Restore mitigation status on CPU resume
arm64: ssbd: Introduce thread flag to control userspace mitigation
arm64: ssbd: Add prctl interface for per-thread mitigation
arm64: KVM: Add HYP per-cpu accessors
arm64: KVM: Add ARCH_WORKAROUND_2 support for guests
arm64: KVM: Handle guest's ARCH_WORKAROUND_2 requests
arm64: KVM: Add ARCH_WORKAROUND_2 discovery through ARCH_FEATURES_FUNC_ID
Mark Rutland (4):
arm_pmu: simplify arm_pmu::handle_irq
drivers/perf: arm-ccn: don't log to dmesg in event_init
arm64: make is_permission_fault() name clearer
arm64: Unify kernel fault reporting
Masahiro Yamada (1):
arm64: remove no-op macro VMLINUX_SYMBOL()
Robin Murphy (5):
arm64: Select ARCH_HAS_FAST_MULTIPLIER
perf/arm-cci: Remove unnecessary period adjustment
perf/arm-cc*: Fix MODULE_LICENSE() tags
perf/arm-cci: Remove pointless PMU disabling
perf/arm-cci: Allow building as a module
Sudeep Holla (1):
ACPI / PPTT: fix build when CONFIG_ACPI_PPTT is not enabled
Vincenzo Frascino (1):
arm64: Remove duplicate include
Will Deacon (2):
arm64: cmpwait: Clear event register before arming exclusive monitor
arm64: Kconfig: Enable LSE atomics by default
Wolfram Sang (1):
perf: simplify getting .drvdata
Documentation/admin-guide/kernel-parameters.txt | 17 +
arch/arm/common/mcpm_entry.c | 2 +
arch/arm/include/asm/kvm_host.h | 12 +
arch/arm/include/asm/kvm_mmu.h | 5 +
arch/arm/kernel/perf_event_v6.c | 4 +-
arch/arm/kernel/perf_event_v7.c | 3 +-
arch/arm/kernel/perf_event_xscale.c | 6 +-
arch/arm64/Kconfig | 15 +-
arch/arm64/include/asm/acpi.h | 4 +
arch/arm64/include/asm/cache.h | 6 +-
arch/arm64/include/asm/cmpxchg.h | 4 +-
arch/arm64/include/asm/cpucaps.h | 3 +-
arch/arm64/include/asm/cpufeature.h | 22 +
arch/arm64/include/asm/elf.h | 13 +
arch/arm64/include/asm/fpsimdmacros.h | 12 +-
arch/arm64/include/asm/kvm_asm.h | 30 +-
arch/arm64/include/asm/kvm_host.h | 26 +
arch/arm64/include/asm/kvm_mmu.h | 25 +-
arch/arm64/include/asm/processor.h | 5 +
arch/arm64/include/asm/thread_info.h | 1 +
arch/arm64/include/asm/topology.h | 6 +-
arch/arm64/include/uapi/asm/auxvec.h | 3 +-
arch/arm64/kernel/Makefile | 1 +
arch/arm64/kernel/armv8_deprecated.c | 3 +-
arch/arm64/kernel/asm-offsets.c | 1 +
arch/arm64/kernel/cacheinfo.c | 15 +-
arch/arm64/kernel/cpu_errata.c | 182 +++++++
arch/arm64/kernel/cpufeature.c | 10 +-
arch/arm64/kernel/entry-fpsimd.S | 2 +-
arch/arm64/kernel/entry.S | 30 ++
arch/arm64/kernel/fpsimd.c | 18 +-
arch/arm64/kernel/hibernate.c | 11 +
arch/arm64/kernel/perf_event.c | 3 +-
arch/arm64/kernel/ptrace.c | 5 -
arch/arm64/kernel/signal.c | 57 ++-
arch/arm64/kernel/ssbd.c | 110 ++++
arch/arm64/kernel/suspend.c | 8 +
arch/arm64/kernel/topology.c | 104 +++-
arch/arm64/kernel/vmlinux.lds.S | 20 +-
arch/arm64/kvm/hyp/hyp-entry.S | 38 +-
arch/arm64/kvm/hyp/switch.c | 42 ++
arch/arm64/kvm/reset.c | 4 +
arch/arm64/mm/dma-mapping.c | 5 +
arch/arm64/mm/fault.c | 46 +-
arch/riscv/kernel/cacheinfo.c | 1 -
drivers/acpi/Kconfig | 3 +
drivers/acpi/Makefile | 1 +
drivers/acpi/pptt.c | 655 ++++++++++++++++++++++++
drivers/acpi/tables.c | 2 +-
drivers/base/cacheinfo.c | 157 +++---
drivers/perf/Kconfig | 36 +-
drivers/perf/arm-cci.c | 47 +-
drivers/perf/arm-ccn.c | 22 +-
drivers/perf/arm_pmu.c | 2 +-
drivers/perf/arm_spe_pmu.c | 6 +-
include/linux/acpi.h | 19 +
include/linux/arm-smccc.h | 10 +
include/linux/cacheinfo.h | 25 +-
include/linux/perf/arm_pmu.h | 2 +-
virt/kvm/arm/arm.c | 4 +
virt/kvm/arm/psci.c | 18 +-
61 files changed, 1689 insertions(+), 260 deletions(-)
create mode 100644 arch/arm64/kernel/ssbd.c
create mode 100644 drivers/acpi/pptt.c
--
Catalin
^ permalink raw reply
* [PATCH 2/2] imx_v6_v7_defconfig: Enable imx6qdl-sabreauto sensors
From: Fabio Estevam @ 2018-06-08 17:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <70b90919f98a6941425b39065dc8d37c95f0fd9f.1528390418.git.leonard.crestez@nxp.com>
On Thu, Jun 7, 2018 at 2:00 PM, Leonard Crestez <leonard.crestez@nxp.com> wrote:
> CONFIG_SENSORS_ISL29018 supports isil,il29023 light sensor
> CONFIG_MMA8452 supports fsl,mma8451 accelerometer
>
> CONFIG_MAG3110 for fsl,mag3110 is already enabled
>
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
Just a minor nit in the Subject. The "ARM" prefix is missing.
Maybe Shawn can adjust it when applying the patch.
Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
^ permalink raw reply
* [PATCH 04/24] 32-bit userspace ABI: introduce ARCH_32BIT_OFF_T config option
From: Catalin Marinas @ 2018-06-08 17:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180516081910.10067-5-ynorov@caviumnetworks.com>
On Wed, May 16, 2018 at 11:18:49AM +0300, Yury Norov wrote:
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 76c0b54443b1..ee079244dc3c 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -264,6 +264,21 @@ config ARCH_THREAD_STACK_ALLOCATOR
> config ARCH_WANTS_DYNAMIC_TASK_STRUCT
> bool
>
> +config ARCH_32BIT_OFF_T
> + bool
> + depends on !64BIT
> + help
> + All new 32-bit architectures should have 64-bit off_t type on
> + userspace side which corresponds to the loff_t kernel type. This
> + is the requirement for modern ABIs. Some existing architectures
> + already have 32-bit off_t. This option is enabled for all such
> + architectures explicitly. Namely: arc, arm, blackfin, cris, frv,
> + h8300, hexagon, m32r, m68k, metag, microblaze, mips32, mn10300,
> + nios2, openrisc, parisc32, powerpc32, score, sh, sparc, tile32,
> + unicore32, x86_32 and xtensa. This is the complete list. Any
> + new 32-bit architecture should declare 64-bit off_t type on user
> + side and so should not enable this option.
Do you know if this is the case for riscv and nds32, merged in the
meantime? If not, I suggest you drop this patch altogether and just
define force_o_largefile() for arm64/ilp32 as we don't seem to stick to
"all new 32-bit architectures should have 64-bit off_t".
--
Catalin
^ permalink raw reply
* [PATCH v4 1/3] arm64: mm: Support Common Not Private translations
From: James Morse @ 2018-06-08 17:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526638022-4137-2-git-send-email-vladimir.murzin@arm.com>
Hi Vladimir,
On 18/05/18 11:07, Vladimir Murzin wrote:
> Common Not Private (CNP) is a feature of ARMv8.2 extension which
> allows translation table entries to be shared between different PEs in
> the same inner shareable domain, so the hardware can use this fact to
> optimise the caching of such entries in the TLB.
>
> CNP occupies one bit in TTBRx_ELy and VTTBR_EL2, which advertises to
> the hardware that the translation table entries pointed to by this
> TTBR are the same as every PE in the same inner shareable domain for
> which the equivalent TTBR also has CNP bit set. In case CNP bit is set
> but TTBR does not point at the same translation table entries for a
> given ASID and VMID, then the system is mis-configured, so the results
> of translations are UNPREDICTABLE.
>
> For EL1 we postpone setting CNP till all cpus are up and rely on
> cpufeature framework to 1) patch the code which is sensitive to CNP
> and 2) update TTBR1_EL1 with CNP bit set. TTBR1_EL1 can be
> reprogrammed as result of hibernation or cpuidle (via __enable_mmu).
> cpuidle's path has been changed to restore CnP and for hibernation the
> code has been changed to save raw TTBR1_EL1 and blindly restore it on
> resume.
>
> For EL0 there are a few cases we need to care of changes in
> TTBR0_EL1:
> - a switch to idmap
> - software emulated PAN
(Nit: I don't think this has anything to do with EL0, its just the low half of
the VA space in TTBR0...)
> we rule out latter via Kconfig options and for the former we make
> sure that CNP is set for non-zero ASIDs only.
Reviewed-by: James Morse <james.morse@arm.com>
> diff --git a/arch/arm64/include/asm/mmu_context.h b/arch/arm64/include/asm/mmu_context.h
> index 39ec0b8..4f9e3ea 100644
> --- a/arch/arm64/include/asm/mmu_context.h
> +++ b/arch/arm64/include/asm/mmu_context.h
> @@ -149,6 +149,18 @@ static inline void cpu_replace_ttbr1(pgd_t *pgdp)
>
> phys_addr_t pgd_phys = virt_to_phys(pgdp);
>
> + if (system_supports_cnp() && !WARN_ON(pgdp != lm_alias(swapper_pg_dir))) {
> + /*
> + * cpu_replace_ttbr1() is used when there's a boot CPU
> + * up (i.e. cpufeature framework is not up yet) and
> + * latter only when we enable CNP via cpufeature's
> + * enable() callback.
> + * Also we rely on the cpu_hwcap bit being set before
(Nit: stray whitespace)
> + * calling the enable() function.
> + */
> + pgd_phys |= TTBR_CNP_BIT;
> + }
> +
> replace_phys = (void *)__pa_symbol(idmap_cpu_replace_ttbr1);
>
> cpu_install_idmap();
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index 9d1b06d..199e9dd 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -858,6 +860,16 @@ static bool has_cache_dic(const struct arm64_cpu_capabilities *entry,
> return read_sanitised_ftr_reg(SYS_CTR_EL0) & BIT(CTR_DIC_SHIFT);
> }
>
> +static bool __maybe_unused
> +has_useable_cnp(const struct arm64_cpu_capabilities *entry, int scope)
> +{
> +#ifdef CONFIG_CRASH_DUMP
> + if (elfcorehdr_size)
> + return false;
> +#endif
It might be worth a comment why kdump is relevant, (here or in the commit
message). Kdump isn't guaranteed to power-off all secondary CPUs, CNP may share
TLB entries with a CPU stuck in the crashed kernel.
I don't think we can't trust it even if we bring all CPUs into the new kernel as
the DT may not reflect all the CPUs the crashed-kernel had. (kexec doesn't have
this problem as it always powers-off secondaries before kexec-ing)
> + return has_cpuid_feature(entry, scope);
> +}
> +
> #ifdef CONFIG_UNMAP_KERNEL_AT_EL0
> static int __kpti_forced; /* 0: not forced, >0: forced on, <0: forced off */
>
> @@ -1642,6 +1667,11 @@ cpufeature_pan_not_uao(const struct arm64_cpu_capabilities *entry, int __unused)
> return (cpus_have_const_cap(ARM64_HAS_PAN) && !cpus_have_const_cap(ARM64_HAS_UAO));
> }
>
> +static void __maybe_unused cpu_enable_cnp (struct arm64_cpu_capabilities const *cap)
Nit: stray space between cpu_enable_cnp and the parameters.
> +{
> + cpu_replace_ttbr1(lm_alias(swapper_pg_dir));
> +}
> +
> /*
> * We emulate only the following system register space.
> * Op0 = 0x3, CRn = 0x0, Op1 = 0x0, CRm = [0, 4 - 7]
> diff --git a/arch/arm64/kernel/hibernate.c b/arch/arm64/kernel/hibernate.c
> index 1ec5f28..ea27121 100644
> --- a/arch/arm64/kernel/hibernate.c
> +++ b/arch/arm64/kernel/hibernate.c
> @@ -125,7 +125,7 @@ int arch_hibernation_header_save(void *addr, unsigned int max_size)
> return -EOVERFLOW;
>
> arch_hdr_invariants(&hdr->invariants);
> - hdr->ttbr1_el1 = __pa_symbol(swapper_pg_dir);
> + hdr->ttbr1_el1 = read_sysreg(ttbr1_el1);
> hdr->reenter_kernel = _cpu_resume;
>
> /* We can't use __hyp_get_vectors() because kvm may still be loaded */
We also run__cpu_suspend_exit() from hibernate's restore, which will restore CNP
so this isn't strictly necessary.
There is some future-cleanup we can do here, if hibernate set the idmap when
calling __cpu_suspend_exit(), that function could do the replace_ttbr1() without
an extra install/uninstall of the idmap.
Thanks,
James
^ permalink raw reply
* [PATCH v4 2/3] arm64: KVM: Enable Common Not Private translations
From: James Morse @ 2018-06-08 17:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526638022-4137-3-git-send-email-vladimir.murzin@arm.com>
Hi Vladimir,
On 18/05/18 11:07, Vladimir Murzin wrote:
> We rely on cpufeature framework to detect and enable CNP so for KVM we
> need to patch hyp to set CNP bit just before TTBR0_EL2 gets written.
>
> For the guest we encode CNP bit while building vttbr, so we don't need
> to bother with that in a world switch.
With the bare-constant fix suggested by Catalin,
Reviewed-by: James Morse <james.morse@arm.com>
Thanks,
James
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox