* [PATCH 1/3] crypto: brcm: DT documentation for Broadcom SPU driver
From: Rob (William) Rice @ 2016-12-07 15:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161206140607.GB24177@leverpostej>
Mark,
Thanks for your comments. Replies below.
Rob
On 12/6/2016 9:06 AM, Mark Rutland wrote:
> On Wed, Nov 30, 2016 at 03:07:31PM -0500, Rob Rice wrote:
>> Device tree documentation for Broadcom Secure Processing Unit
>> (SPU) crypto driver.
>>
>> Signed-off-by: Steve Lin <steven.lin1@broadcom.com>
>> Signed-off-by: Rob Rice <rob.rice@broadcom.com>
>> ---
>> .../devicetree/bindings/crypto/brcm,spu-crypto.txt | 25 ++++++++++++++++++++++
>> 1 file changed, 25 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/crypto/brcm,spu-crypto.txt
>>
>> diff --git a/Documentation/devicetree/bindings/crypto/brcm,spu-crypto.txt b/Documentation/devicetree/bindings/crypto/brcm,spu-crypto.txt
>> new file mode 100644
>> index 0000000..e5fe942
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/crypto/brcm,spu-crypto.txt
>> @@ -0,0 +1,25 @@
>> +The Broadcom Secure Processing Unit (SPU) driver supports symmetric
>> +cryptographic offload for Broadcom SoCs with SPU hardware. A SoC may have
>> +multiple SPU hardware blocks.
> Bindings shound describe *hardware*, not *drivers*. Please drop mention
> of the driver, and just decribe the hardware.
Makes sense. I'll change it.
>
>> +Required properties:
>> +- compatible : Should be "brcm,spum-crypto" for devices with SPU-M hardware
>> + (e.g., Northstar2) or "brcm,spum-nsp-crypto" for the Northstar Plus variant
>> + of the SPU-M hardware.
>> +
>> +- reg: Should contain SPU registers location and length.
>> +- mboxes: A list of mailbox channels to be used by the kernel driver. Mailbox
>> +channels correspond to DMA rings on the device.
>> +
>> +Example:
>> + spu-crypto at 612d0000 {
>> + compatible = "brcm,spum-crypto";
>> + reg = <0 0x612d0000 0 0x900>, /* SPU 0 control regs */
>> + <0 0x612f0000 0 0x900>, /* SPU 1 control regs */
>> + <0 0x61310000 0 0x900>, /* SPU 2 control regs */
>> + <0 0x61330000 0 0x900>; /* SPU 3 control regs */
> The above didn't mention there were several register sets, and the
> comment beside each makes them sound like they're separate SPU
> instances, so I don't think it makes sense to group them as one node.
>
> What's going on here?
That's right. For the SoC I used as the example, there are four SPU
hardware blocks. The driver round robins crypto requests to the hardware
blocks to handle requests in parallel and increase throughput. I want
one instance of the SPU driver that registers algos once with the crypto
API and manages all the mailbox channels. Maybe I can achieve that with
separate device tree entries for each SPU block, I'm not sure. I'll look
into it.
>
>> + mboxes = <&pdc0 0>,
>> + <&pdc1 0>,
>> + <&pdc2 0>,
>> + <&pdc3 0>;
> Does each mbox correspond to one of the SPUs above? Or is there a shared
> pool?
Yes, each of these mailbox channels corresponds to a different SPU. PDC
is a DMA ring manager for DMAs to the SPUs.
>
> Thanks,
> Mark.
^ permalink raw reply
* [PATCH] ARM: dts: armada-{370, xp}: drop "marvell, orion-spi" from SPI controllers
From: Uwe Kleine-König @ 2016-12-07 15:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <87eg1jhi4l.fsf@free-electrons.com>
Hello Gregory,
On Wed, Dec 07, 2016 at 04:30:02PM +0100, Gregory CLEMENT wrote:
> On mer., d?c. 07 2016, Uwe Kleine-K?nig <uwe@kleine-koenig.org> wrote:
>
> > From: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> >
> > The SPI controllers on Armada 370 and XP differ from the original Orion
> > SPI controllers (at least) in the configuration of the baud rate. So
> > it's wrong to claim compatibility which results in bogus baud rates.
>
> Until two years ago with the commits
> df59fa7f4bca9658b75f0f5fee225b3a057475c5 and
> 4dacccfac69494ba70248b134352f299171c41b7
> we used "marvell,orion-spi" compatible on Armada XP and Armada 370
> without any problem.
>
> The new compatible "marvell,armada-xp-spi" and "marvell,armada-xp-spi"
> allows to have more choice for the baudrate for a given clock but it is
> not true that Armada 370 and Armada XP are not compatible with
> "marvell,orion-spi".
The issue I was faced with that made me create this patch is that in
barebox no special case for 370/XP was active. The result was that a
device that could be operated at 60 MHz only got a clock of 11 MHz and
the driver assumed it configured 41.666 MHz. I didn't check if the same
can happen in the opposite direction (or if there are other important
incompatibilities) but still I'd say this is a bug with my patch being
the obvious fix.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* [PATCH] drm: hdlcd: Work properly in big-endian mode
From: Robin Murphy @ 2016-12-07 15:31 UTC (permalink / raw)
To: linux-arm-kernel
Under a big-endian kernel, colours on the framebuffer all come out a
delightful shade of wrong, since we fail to take the reversed byte
order into account. Fortunately, the HDLCD has a control bit to make it
automatically byteswap big-endian data; let's use it as appropriate.
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/gpu/drm/arm/hdlcd_crtc.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
index 28341b32067f..eceb7bed5dd0 100644
--- a/drivers/gpu/drm/arm/hdlcd_crtc.c
+++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
@@ -63,6 +63,7 @@ static int hdlcd_set_pxl_fmt(struct drm_crtc *crtc)
uint32_t pixel_format;
struct simplefb_format *format = NULL;
int i;
+ u32 reg;
pixel_format = crtc->primary->state->fb->pixel_format;
@@ -76,7 +77,11 @@ static int hdlcd_set_pxl_fmt(struct drm_crtc *crtc)
/* HDLCD uses 'bytes per pixel', zero means 1 byte */
btpp = (format->bits_per_pixel + 7) / 8;
- hdlcd_write(hdlcd, HDLCD_REG_PIXEL_FORMAT, (btpp - 1) << 3);
+ reg = (btpp - 1) << 3;
+#ifdef __BIG_ENDIAN
+ reg |= HDLCD_PIXEL_FMT_BIG_ENDIAN;
+#endif
+ hdlcd_write(hdlcd, HDLCD_REG_PIXEL_FORMAT, reg);
/*
* The format of the HDLCD_REG_<color>_SELECT register is:
--
2.10.2.dirty
^ permalink raw reply related
* [PATCH] ARM: dts: armada-{370, xp}: drop "marvell, orion-spi" from SPI controllers
From: Gregory CLEMENT @ 2016-12-07 15:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161207152109.17545-1-uwe@kleine-koenig.org>
Hi Uwe,
On mer., d?c. 07 2016, Uwe Kleine-K?nig <uwe@kleine-koenig.org> wrote:
> From: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
>
> The SPI controllers on Armada 370 and XP differ from the original Orion
> SPI controllers (at least) in the configuration of the baud rate. So
> it's wrong to claim compatibility which results in bogus baud rates.
Until two years ago with the commits
df59fa7f4bca9658b75f0f5fee225b3a057475c5 and
4dacccfac69494ba70248b134352f299171c41b7
we used "marvell,orion-spi" compatible on Armada XP and Armada 370
without any problem.
The new compatible "marvell,armada-xp-spi" and "marvell,armada-xp-spi"
allows to have more choice for the baudrate for a given clock but it is
not true that Armada 370 and Armada XP are not compatible with
"marvell,orion-spi".
Gregory
>
> Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> ---
> arch/arm/boot/dts/armada-370.dtsi | 4 ++--
> arch/arm/boot/dts/armada-xp.dtsi | 4 ++--
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/boot/dts/armada-370.dtsi b/arch/arm/boot/dts/armada-370.dtsi
> index b4258105e91f..b9377c11b379 100644
> --- a/arch/arm/boot/dts/armada-370.dtsi
> +++ b/arch/arm/boot/dts/armada-370.dtsi
> @@ -435,13 +435,13 @@
> * board level if a different configuration is used.
> */
> &spi0 {
> - compatible = "marvell,armada-370-spi", "marvell,orion-spi";
> + compatible = "marvell,armada-370-spi";
> pinctrl-0 = <&spi0_pins1>;
> pinctrl-names = "default";
> };
>
> &spi1 {
> - compatible = "marvell,armada-370-spi", "marvell,orion-spi";
> + compatible = "marvell,armada-370-spi";
> pinctrl-0 = <&spi1_pins>;
> pinctrl-names = "default";
> };
> diff --git a/arch/arm/boot/dts/armada-xp.dtsi b/arch/arm/boot/dts/armada-xp.dtsi
> index 4a5f99e65b51..3a416834d80b 100644
> --- a/arch/arm/boot/dts/armada-xp.dtsi
> +++ b/arch/arm/boot/dts/armada-xp.dtsi
> @@ -367,13 +367,13 @@
> };
>
> &spi0 {
> - compatible = "marvell,armada-xp-spi", "marvell,orion-spi";
> + compatible = "marvell,armada-xp-spi";
> pinctrl-0 = <&spi0_pins>;
> pinctrl-names = "default";
> };
>
> &spi1 {
> - compatible = "marvell,armada-xp-spi", "marvell,orion-spi";
> + compatible = "marvell,armada-xp-spi";
> pinctrl-0 = <&spi1_pins>;
> pinctrl-names = "default";
> };
> --
> 2.10.2
>
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply
* [PATCH v4 4/4] ARM: da850: fix da850_set_pll0rate()
From: Bartosz Golaszewski @ 2016-12-07 15:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481124138-27337-1-git-send-email-bgolaszewski@baylibre.com>
This function is confusing - its second argument is an index to the
freq table, not the requested clock rate in Hz, but it's used as the
set_rate callback for the pll0 clock. It leads to an oops when the
caller doesn't know the internals and passes the rate in Hz as
argument instead of the cpufreq index since this argument isn't bounds
checked either.
Fix it by iterating over the array of supported frequencies and
selecting a one that matches or returning -EINVAL for unsupported
rates.
Also: update the davinci cpufreq driver. It's the only user of this
clock and currently it passes the cpufreq table index to
clk_set_rate(), which is confusing. Make it pass the requested clock
rate in Hz.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
---
arch/arm/mach-davinci/da850.c | 22 ++++++++++++++++++----
drivers/cpufreq/davinci-cpufreq.c | 2 +-
2 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
index 5f8ffaa..b410598 100644
--- a/arch/arm/mach-davinci/da850.c
+++ b/arch/arm/mach-davinci/da850.c
@@ -1197,14 +1197,28 @@ static int da850_set_armrate(struct clk *clk, unsigned long index)
return clk_set_rate(pllclk, index);
}
-static int da850_set_pll0rate(struct clk *clk, unsigned long index)
+static int da850_set_pll0rate(struct clk *clk, unsigned long rate)
{
- unsigned int prediv, mult, postdiv;
- struct da850_opp *opp;
struct pll_data *pll = clk->pll_data;
+ struct cpufreq_frequency_table *freq;
+ unsigned int prediv, mult, postdiv;
+ struct da850_opp *opp = NULL;
int ret;
- opp = (struct da850_opp *) cpufreq_info.freq_table[index].driver_data;
+ rate /= 1000;
+
+ for (freq = da850_freq_table;
+ freq->frequency != CPUFREQ_TABLE_END; freq++) {
+ /* rate is in Hz, freq->frequency is in KHz */
+ if (freq->frequency == rate) {
+ opp = (struct da850_opp *)freq->driver_data;
+ break;
+ }
+ }
+
+ if (!opp)
+ return -EINVAL;
+
prediv = opp->prediv;
mult = opp->mult;
postdiv = opp->postdiv;
diff --git a/drivers/cpufreq/davinci-cpufreq.c b/drivers/cpufreq/davinci-cpufreq.c
index b95a872..d54a27c 100644
--- a/drivers/cpufreq/davinci-cpufreq.c
+++ b/drivers/cpufreq/davinci-cpufreq.c
@@ -55,7 +55,7 @@ static int davinci_target(struct cpufreq_policy *policy, unsigned int idx)
return ret;
}
- ret = clk_set_rate(armclk, idx);
+ ret = clk_set_rate(armclk, new_freq * 1000);
if (ret)
return ret;
--
2.9.3
^ permalink raw reply related
* [PATCH v4 3/4] ARM: da850: coding style fix
From: Bartosz Golaszewski @ 2016-12-07 15:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481124138-27337-1-git-send-email-bgolaszewski@baylibre.com>
Fix alignment of the clock lookup table entries.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
arch/arm/mach-davinci/da850.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
index 6b1fbac..5f8ffaa 100644
--- a/arch/arm/mach-davinci/da850.c
+++ b/arch/arm/mach-davinci/da850.c
@@ -566,7 +566,7 @@ static struct clk_lookup da850_clks[] = {
CLK("spi_davinci.0", NULL, &spi0_clk),
CLK("spi_davinci.1", NULL, &spi1_clk),
CLK("vpif", NULL, &vpif_clk),
- CLK("ahci_da850", NULL, &sata_clk),
+ CLK("ahci_da850", NULL, &sata_clk),
CLK("davinci-rproc.0", NULL, &dsp_clk),
CLK(NULL, NULL, &ehrpwm_clk),
CLK("ehrpwm.0", "fck", &ehrpwm0_clk),
--
2.9.3
^ permalink raw reply related
* [PATCH v4 2/4] ARM: da850: don't add the emac clock to the clock lookup table twice
From: Bartosz Golaszewski @ 2016-12-07 15:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481124138-27337-1-git-send-email-bgolaszewski@baylibre.com>
Similarly to the aemif clock - this screws up the linked list of clock
children. Create a separate clock for mdio inheriting the rate from
emac_clk.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
arch/arm/mach-davinci/da850.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
index e9d019c..6b1fbac 100644
--- a/arch/arm/mach-davinci/da850.c
+++ b/arch/arm/mach-davinci/da850.c
@@ -319,6 +319,11 @@ static struct clk emac_clk = {
.gpsc = 1,
};
+static struct clk mdio_clk = {
+ .name = "mdio",
+ .parent = &emac_clk,
+};
+
static struct clk mcasp_clk = {
.name = "mcasp",
.parent = &async3_clk,
@@ -539,7 +544,7 @@ static struct clk_lookup da850_clks[] = {
CLK(NULL, "arm", &arm_clk),
CLK(NULL, "rmii", &rmii_clk),
CLK("davinci_emac.1", NULL, &emac_clk),
- CLK("davinci_mdio.0", "fck", &emac_clk),
+ CLK("davinci_mdio.0", "fck", &mdio_clk),
CLK("davinci-mcasp.0", NULL, &mcasp_clk),
CLK("davinci-mcbsp.0", NULL, &mcbsp0_clk),
CLK("davinci-mcbsp.1", NULL, &mcbsp1_clk),
--
2.9.3
^ permalink raw reply related
* [PATCH v4 1/4] ARM: da850: fix infinite loop in clk_set_rate()
From: Bartosz Golaszewski @ 2016-12-07 15:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481124138-27337-1-git-send-email-bgolaszewski@baylibre.com>
The aemif clock is added twice to the lookup table in da850.c. This
breaks the children list of pll0_sysclk3 as we're using the same list
links in struct clk. When calling clk_set_rate(), we get stuck in
propagate_rate().
Create a separate clock for nand, inheriting the rate of the aemif
clock and retrieve it in the davinci_nand module.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
arch/arm/mach-davinci/da850.c | 20 +++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
index e770c97..e9d019c 100644
--- a/arch/arm/mach-davinci/da850.c
+++ b/arch/arm/mach-davinci/da850.c
@@ -367,6 +367,16 @@ static struct clk aemif_clk = {
.flags = ALWAYS_ENABLED,
};
+/*
+ * In order to avoid adding the aemif_clk to the clock lookup table twice (and
+ * screwing up the linked list in the process) create a separate clock for
+ * nand inheriting the rate from aemif_clk.
+ */
+static struct clk aemif_nand_clk = {
+ .name = "nand",
+ .parent = &aemif_clk,
+};
+
static struct clk usb11_clk = {
.name = "usb11",
.parent = &pll0_sysclk4,
@@ -537,7 +547,15 @@ static struct clk_lookup da850_clks[] = {
CLK("da830-mmc.0", NULL, &mmcsd0_clk),
CLK("da830-mmc.1", NULL, &mmcsd1_clk),
CLK("ti-aemif", NULL, &aemif_clk),
- CLK(NULL, "aemif", &aemif_clk),
+ /*
+ * The only user of this clock is davinci_nand and it get's it through
+ * con_id. The nand node itself is created from within the aemif
+ * driver to guarantee that it's probed after the aemif timing
+ * parameters are configured. of_dev_auxdata is not accessible from
+ * the aemif driver and can't be passed to of_platform_populate(). For
+ * that reason we're leaving the dev_id here as NULL.
+ */
+ CLK(NULL, "aemif", &aemif_nand_clk),
CLK("ohci-da8xx", "usb11", &usb11_clk),
CLK("musb-da8xx", "usb20", &usb20_clk),
CLK("spi_davinci.0", NULL, &spi0_clk),
--
2.9.3
^ permalink raw reply related
* [PATCH v4 0/4] ARM: da850: fix pll0 rate setting
From: Bartosz Golaszewski @ 2016-12-07 15:22 UTC (permalink / raw)
To: linux-arm-kernel
While trying to set the pll0 rate from the kernel I noticed there are
two issues with da850 clocks. The first patch fixes an infinite loop
in propagate_rate(). The third fixes an oops in da850_set_pll0rate().
The second patch is just a coding style fix, while we're at it.
v1 -> v2:
- change the approach in 1/3: create a new clock for nand inheriting
the rate from the aemif clock (verified that nand still works on
da850-lcdk)
- patch 3/3: also update the davinci_cpufreq driver - the only
(indirect) user of da850_set_pll0rate()
- s/requested_rate/rate in 3/3
v2 -> v3:
- 1/3: keep the "aemif" connector id for the nand clock
- 3/3: instead of multiplying freq->frequency, divide rate by 1000
- retested both davinci_nand and clk_set_rate() for pll0
v3 -> v4:
- 1/4: add a comment explaining why we're leaving a NULL dev_id
- 2/4: emac_clk is added twice to the clock lookup table too - fix it
and verify that davinci_mmdio gets a functional clock
- 4/4: s/opp == NULL/!opp/
Bartosz Golaszewski (4):
ARM: da850: fix infinite loop in clk_set_rate()
ARM: da850: don't add the emac clock to the clock lookup table twice
ARM: da850: coding style fix
ARM: da850: fix da850_set_pll0rate()
arch/arm/mach-davinci/da850.c | 51 +++++++++++++++++++++++++++++++++------
drivers/cpufreq/davinci-cpufreq.c | 2 +-
2 files changed, 45 insertions(+), 8 deletions(-)
--
2.9.3
^ permalink raw reply
* [PATCH] ARM: dts: armada-{370, xp}: drop "marvell, orion-spi" from SPI controllers
From: Uwe Kleine-König @ 2016-12-07 15:21 UTC (permalink / raw)
To: linux-arm-kernel
From: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
The SPI controllers on Armada 370 and XP differ from the original Orion
SPI controllers (at least) in the configuration of the baud rate. So
it's wrong to claim compatibility which results in bogus baud rates.
Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
---
arch/arm/boot/dts/armada-370.dtsi | 4 ++--
arch/arm/boot/dts/armada-xp.dtsi | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm/boot/dts/armada-370.dtsi b/arch/arm/boot/dts/armada-370.dtsi
index b4258105e91f..b9377c11b379 100644
--- a/arch/arm/boot/dts/armada-370.dtsi
+++ b/arch/arm/boot/dts/armada-370.dtsi
@@ -435,13 +435,13 @@
* board level if a different configuration is used.
*/
&spi0 {
- compatible = "marvell,armada-370-spi", "marvell,orion-spi";
+ compatible = "marvell,armada-370-spi";
pinctrl-0 = <&spi0_pins1>;
pinctrl-names = "default";
};
&spi1 {
- compatible = "marvell,armada-370-spi", "marvell,orion-spi";
+ compatible = "marvell,armada-370-spi";
pinctrl-0 = <&spi1_pins>;
pinctrl-names = "default";
};
diff --git a/arch/arm/boot/dts/armada-xp.dtsi b/arch/arm/boot/dts/armada-xp.dtsi
index 4a5f99e65b51..3a416834d80b 100644
--- a/arch/arm/boot/dts/armada-xp.dtsi
+++ b/arch/arm/boot/dts/armada-xp.dtsi
@@ -367,13 +367,13 @@
};
&spi0 {
- compatible = "marvell,armada-xp-spi", "marvell,orion-spi";
+ compatible = "marvell,armada-xp-spi";
pinctrl-0 = <&spi0_pins>;
pinctrl-names = "default";
};
&spi1 {
- compatible = "marvell,armada-xp-spi", "marvell,orion-spi";
+ compatible = "marvell,armada-xp-spi";
pinctrl-0 = <&spi1_pins>;
pinctrl-names = "default";
};
--
2.10.2
^ permalink raw reply related
* [PATCH 0/7] arm: Add livepatch support
From: Petr Mladek @ 2016-12-07 15:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481043967-15602-1-git-send-email-abelvesa@linux.com>
On Tue 2016-12-06 17:06:00, Abel Vesa wrote:
> This is just an idea I've been trying out for a while now.
>
> Just in case somebody wants to play with it, this applies to linux-arm/for-next.
>
> Also please note that this was only tested in qemu, but I will do some testing
> on some real hardware in the following days.
>
> FWICT, on this arch the compiler always generates a function prologue somewhere
> between these lines:
>
> e1a0c00d mov ip, sp
> e92ddff0 push {r4-r9, sl, fp, ip, lr, pc}
> e24cb004 sub fp, ip, #4
> e24dd064 sub sp, sp, #100 ; 0x64 <--- local variables
> e52de004 push {lr} ; (str lr, [sp, #-4]!)
> ebf9c2c9 bl 80110364 <__gnu_mcount_nc>
> ....
>
> Every function that follows this pattern (the number of registers pushed and the
> sp subtraction for the local variables being the only acceptable exception) can
> be patched with this mechanism. IIRC, only the inline functions and notrace
> functions do not follow this pattern.
Please, where do you check that the given function follows this
pattern? I do not have experience with arm at all. But compiler
is able to do crazy optimizations these days.
I think that this was already mentioned somewhere. But please, put
this detailed explanation also to related patch/code so that it
can later be found in the git commits. It will also help to
better understand/review the particular patches.
Best Regards,
Petr
^ permalink raw reply
* [PATCH v5 4/5] [media] dt-bindings: add TI VPIF documentation
From: Laurent Pinchart @ 2016-12-07 15:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161207050826.23174-5-khilman@baylibre.com>
Hi Kevin,
Thank you for the patch.
On Tuesday 06 Dec 2016 21:08:25 Kevin Hilman wrote:
> Acked-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Kevin Hilman <khilman@baylibre.com>
> ---
> .../devicetree/bindings/media/ti,da850-vpif.txt | 67 +++++++++++++++++++
> 1 file changed, 67 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/media/ti,da850-vpif.txt
>
> diff --git a/Documentation/devicetree/bindings/media/ti,da850-vpif.txt
> b/Documentation/devicetree/bindings/media/ti,da850-vpif.txt new file mode
> 100644
> index 000000000000..fa06dfdb6898
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/ti,da850-vpif.txt
> @@ -0,0 +1,67 @@
> +Texas Instruments VPIF
> +----------------------
> +
> +The TI Video Port InterFace (VPIF) is the primary component for video
> +capture and display on the DA850/AM18x family of TI DaVinci/Sitara
> +SoCs.
> +
> +TI Document reference: SPRUH82C, Chapter 35
> +http://www.ti.com/lit/pdf/spruh82
> +
> +Required properties:
> +- compatible: must be "ti,da850-vpif"
> +- reg: physical base address and length of the registers set for the
> device;
> +- interrupts: should contain IRQ line for the VPIF
> +
> +Video Capture:
> +
> +VPIF has a 16-bit parallel bus input, supporting 2 8-bit channels or a
> +single 16-bit channel. It should contain at least one port child node
> +with child 'endpoint' node. Please refer to the bindings defined in
> +Documentation/devicetree/bindings/media/video-interfaces.txt.
You might want to clarify how endpoints are use in the two cases. Apart from
that,
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> +Example using 2 8-bit input channels, one of which is connected to an
> +I2C-connected TVP5147 decoder:
> +
> + vpif: vpif at 217000 {
> + compatible = "ti,da850-vpif";
> + reg = <0x217000 0x1000>;
> + interrupts = <92>;
> +
> + port {
> + vpif_ch0: endpoint at 0 {
> + reg = <0>;
> + bus-width = <8>;
> + remote-endpoint = <&composite>;
> + };
> +
> + vpif_ch1: endpoint at 1 {
> + reg = <1>;
> + bus-width = <8>;
> + data-shift = <8>;
> + };
> + };
> + };
> +
> +[ ... ]
> +
> +&i2c0 {
> +
> + tvp5147 at 5d {
> + compatible = "ti,tvp5147";
> + reg = <0x5d>;
> + status = "okay";
> +
> + port {
> + composite: endpoint {
> + hsync-active = <1>;
> + vsync-active = <1>;
> + pclk-sample = <0>;
> +
> + /* VPIF channel 0 (lower 8-bits) */
> + remote-endpoint = <&vpif_ch0>;
> + bus-width = <8>;
> + };
> + };
> + };
> +};
--
Regards,
Laurent Pinchart
^ permalink raw reply
* [RFC v3 05/10] iommu: Do not map reserved regions
From: Auger Eric @ 2016-12-07 15:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <fff44f65-d9a6-1e35-97cd-997b92d2fce3@arm.com>
Hi Robin
On 06/12/2016 18:36, Robin Murphy wrote:
> On 15/11/16 13:09, Eric Auger wrote:
>> As we introduced IOMMU_RESV_NOMAP and IOMMU_RESV_MSI regions,
>> let's prevent those new regions from being mapped.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> ---
>> drivers/iommu/iommu.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>> index 6ee529f..a4530ad 100644
>> --- a/drivers/iommu/iommu.c
>> +++ b/drivers/iommu/iommu.c
>> @@ -343,6 +343,9 @@ static int iommu_group_create_direct_mappings(struct iommu_group *group,
>> start = ALIGN(entry->start, pg_size);
>> end = ALIGN(entry->start + entry->length, pg_size);
>>
>> + if (entry->prot & IOMMU_RESV_MASK)
>
> This seems to be the only place that this mask is used, and frankly I
> think it's less clear than simply "(IOMMU_RESV_NOMAP | IOMMU_RESV_MSI)"
> would be, at which point we may as well drop the mask and special value
> trickery altogether. Plus, per my previous comment, if it were to be "if
> (entry->type != <direct mapped type>)" instead, that's about as obvious
> as it can get.
OK I will add this new type entry in the reserved window struct.
thanks
Eric
>
> Robin.
>
>> + continue;
>> +
>> for (addr = start; addr < end; addr += pg_size) {
>> phys_addr_t phys_addr;
>>
>>
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply
* [RFC v3 06/10] iommu: iommu_get_group_resv_regions
From: Auger Eric @ 2016-12-07 15:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <8eb9662b-d677-546c-feb8-32fe5bd624f2@arm.com>
Hi Robin,
On 06/12/2016 19:13, Robin Murphy wrote:
> On 15/11/16 13:09, Eric Auger wrote:
>> Introduce iommu_get_group_resv_regions whose role consists in
>> enumerating all devices from the group and collecting their
>> reserved regions. It checks duplicates.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>
>> ---
>>
>> - we do not move list elements from device to group list since
>> the iommu_put_resv_regions() could not be called.
>> - at the moment I did not introduce any iommu_put_group_resv_regions
>> since it simply consists in voiding/freeing the list
>> ---
>> drivers/iommu/iommu.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++
>> include/linux/iommu.h | 8 ++++++++
>> 2 files changed, 61 insertions(+)
>>
>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>> index a4530ad..e0fbcc5 100644
>> --- a/drivers/iommu/iommu.c
>> +++ b/drivers/iommu/iommu.c
>> @@ -133,6 +133,59 @@ static ssize_t iommu_group_show_name(struct iommu_group *group, char *buf)
>> return sprintf(buf, "%s\n", group->name);
>> }
>>
>> +static bool iommu_resv_region_present(struct iommu_resv_region *region,
>> + struct list_head *head)
>> +{
>> + struct iommu_resv_region *entry;
>> +
>> + list_for_each_entry(entry, head, list) {
>> + if ((region->start == entry->start) &&
>> + (region->length == entry->length) &&
>> + (region->prot == entry->prot))
>> + return true;
>> + }
>> + return false;
>> +}
>> +
>> +static int
>> +iommu_insert_device_resv_regions(struct list_head *dev_resv_regions,
>> + struct list_head *group_resv_regions)
>> +{
>> + struct iommu_resv_region *entry, *region;
>> +
>> + list_for_each_entry(entry, dev_resv_regions, list) {
>> + if (iommu_resv_region_present(entry, group_resv_regions))
>> + continue;
>
> In the case of overlapping regions which _aren't_ an exact match, would
> it be better to expand the existing one rather than leave the caller to
> sort it out? It seems a bit inconsistent to handle only the one case here.
Well this is mostly here to avoid inserting several times the same PCIe
host bridge windows (retrieved from several PCIe EP attached to the same
bridge). I don't know if it is worth making things over-complicated. Do
you have another situation in mind?
>
>> + region = iommu_alloc_resv_region(entry->start, entry->length,
>> + entry->prot);
>> + if (!region)
>> + return -ENOMEM;
>> +
>> + list_add_tail(®ion->list, group_resv_regions);
>> + }
>> + return 0;
>> +}
>> +
>> +int iommu_get_group_resv_regions(struct iommu_group *group,
>> + struct list_head *head)
>> +{
>> + struct iommu_device *device;
>> + int ret = 0;
>> +
>> + list_for_each_entry(device, &group->devices, list) {
>
> Should we not be taking the group mutex around this?
Yes you're right.
Thanks
Eric
>
> Robin.
>
>> + struct list_head dev_resv_regions;
>> +
>> + INIT_LIST_HEAD(&dev_resv_regions);
>> + iommu_get_resv_regions(device->dev, &dev_resv_regions);
>> + ret = iommu_insert_device_resv_regions(&dev_resv_regions, head);
>> + iommu_put_resv_regions(device->dev, &dev_resv_regions);
>> + if (ret)
>> + break;
>> + }
>> + return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(iommu_get_group_resv_regions);
>> +
>> static IOMMU_GROUP_ATTR(name, S_IRUGO, iommu_group_show_name, NULL);
>>
>> static void iommu_group_release(struct kobject *kobj)
>> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
>> index 0aea877..0f7ae2c 100644
>> --- a/include/linux/iommu.h
>> +++ b/include/linux/iommu.h
>> @@ -243,6 +243,8 @@ extern void iommu_set_fault_handler(struct iommu_domain *domain,
>> extern int iommu_request_dm_for_dev(struct device *dev);
>> extern struct iommu_resv_region *
>> iommu_alloc_resv_region(phys_addr_t start, size_t length, unsigned int prot);
>> +extern int iommu_get_group_resv_regions(struct iommu_group *group,
>> + struct list_head *head);
>>
>> extern int iommu_attach_group(struct iommu_domain *domain,
>> struct iommu_group *group);
>> @@ -462,6 +464,12 @@ static inline void iommu_put_resv_regions(struct device *dev,
>> return NULL;
>> }
>>
>> +static inline int iommu_get_group_resv_regions(struct iommu_group *group,
>> + struct list_head *head)
>> +{
>> + return -ENODEV;
>> +}
>> +
>> static inline int iommu_request_dm_for_dev(struct device *dev)
>> {
>> return -ENODEV;
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* [PATCH v3 4/6] mfd: dt: Add bindings for the Aspeed LPC Host Controller (LHC)
From: Linus Walleij @ 2016-12-07 15:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161206025321.1792-5-andrew@aj.id.au>
On Tue, Dec 6, 2016 at 3:53 AM, Andrew Jeffery <andrew@aj.id.au> wrote:
> The LPC bus pinmux configuration on fifth generation Aspeed SoCs depends
> on bits in both the System Control Unit and the LPC Host Controller.
>
> The Aspeed LPC Host Controller is described as a child node of the
> LPC host-range syscon device for arbitration of access by the host
> controller and pinmux drivers.
>
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
^ permalink raw reply
* [RESEND PATCH v2 4/7] drm/vc4: Add support for the VEC (Video Encoder) IP
From: Boris Brezillon @ 2016-12-07 15:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <c1b904db-be3d-1e83-37bc-1f750ecfe224@gmail.com>
On Mon, 5 Dec 2016 17:50:13 -0800
Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 12/02/2016 05:48 AM, Boris Brezillon wrote:
> > The VEC IP is a TV DAC, providing support for PAL and NTSC standards.
> >
> > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> > ---
>
> > diff --git a/drivers/gpu/drm/vc4/vc4_vec.c b/drivers/gpu/drm/vc4/vc4_vec.c
> > new file mode 100644
> > index 000000000000..2d4256fcc6f2
> > --- /dev/null
> > +++ b/drivers/gpu/drm/vc4/vc4_vec.c
> > @@ -0,0 +1,657 @@
> > +/*
> > + * Copyright (C) 2016 Broadcom Limited
>
> The standard copyright template post acquisition is just Broadcom, not
> Broadcom Limited, nor Broadcom corporation. Can you audit your entire
> submission and fix this up accordingly?
This is the only patch adding a copyright header.
Eric, can you fix that when applying the series or should I resend a
new version?
^ permalink raw reply
* [PATCH v3 3/6] mfd: dt: Add Aspeed Low Pin Count Controller bindings
From: Linus Walleij @ 2016-12-07 15:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161206025321.1792-4-andrew@aj.id.au>
On Tue, Dec 6, 2016 at 3:53 AM, Andrew Jeffery <andrew@aj.id.au> wrote:
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH v3 1/6] mfd: dt: Fix "indicates" typo in mfd bindings document
From: Linus Walleij @ 2016-12-07 15:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161206025321.1792-2-andrew@aj.id.au>
On Tue, Dec 6, 2016 at 3:53 AM, Andrew Jeffery <andrew@aj.id.au> wrote:
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
My speling mistak.
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH] dmaengine: pl330: do not generate unaligned access
From: Vladimir Murzin @ 2016-12-07 15:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <99e9e221-0b1a-2a76-c774-3f4be1fb3f3d@arm.com>
On 07/12/16 15:04, Robin Murphy wrote:
> On 07/12/16 13:17, Vladimir Murzin wrote:
>> When PL330 is used with !MMU the following fault is seen:
>>
>> Unhandled fault: alignment exception (0x801) at 0x8f26a002
>> Internal error: : 801 [#1] ARM
>> Modules linked in:
>> CPU: 0 PID: 640 Comm: dma0chan0-copy0 Not tainted 4.8.0-6a82063-clean+ #1600
>> Hardware name: ARM-Versatile Express
>> task: 8f1baa80 task.stack: 8e6fe000
>> PC is at _setup_req+0x4c/0x350
>> LR is at 0x8f2cbc00
>> pc : [<801ea538>] lr : [<8f2cbc00>] psr: 60000093
>> sp : 8e6ffdc0 ip : 00000000 fp : 00000000
>> r10: 00000000 r9 : 8f2cba10 r8 : 8f2cbc00
>> r7 : 80000013 r6 : 8f21a050 r5 : 8f21a000 r4 : 8f2ac800
>> r3 : 8e6ffe18 r2 : 00944251 r1 : ffffffbc r0 : 8f26a000
>> Flags: nZCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment none
>> Control: 00c5387c
>> Process dma0chan0-copy0 (pid: 640, stack limit = 0x8e6fe210)
>> Stack: (0x8e6ffdc0 to 0x8e700000)
>> fdc0: 00000001 60000093 00000000 8f2cba10 8f26a000 00000004 8f0ae000 8f2cbc00
>> fde0: 8f0ae000 8f2ac800 8f21a000 8f21a050 80000013 8f2cbc00 8f2cba10 00000000
>> fe00: 60000093 801ebca0 8e6ffe18 000013ff 40000093 00000000 00944251 8f2ac800
>> fe20: a0000013 8f2b1320 00001986 00000000 00000001 000013ff 8f1e4f00 8f2cba10
>> fe40: 8e6fff6c 801e9044 00000003 00000000 fef98c80 002faf07 8e6ffe7c 00000000
>> fe60: 00000002 00000000 00001986 8f1f158d 8f1e4f00 80568de4 00000002 00000000
>> fe80: 00001986 8f1f53ff 40000001 80580500 8f1f158d 8001e00c 00000000 cfdfdfdf
>> fea0: fdae2a25 00000001 00000004 8e6fe000 00000008 00000010 00000000 00000005
>> fec0: 8f2b1330 8f2b1334 8e6ffe80 8e6ffe8c 00001986 00000000 8f21a014 00000001
>> fee0: 8e6ffe60 8e6ffe78 00000002 00000000 000013ff 00000001 80568de4 8f1e8018
>> ff00: 0000158d 8055ec30 00000001 803f6b00 00001986 8f2cba10 fdae2a25 00000001
>> ff20: 8f1baca8 8e6fff24 8e6fff24 00000000 8e6fff24 ac6f3037 00000000 00000000
>> ff40: 00000000 8e6fe000 8f1e4f40 00000000 8f1e4f40 8f1e4f00 801e84ec 00000000
>> ff60: 00000000 00000000 00000000 80031714 dfdfdfcf 00000000 dfdfdfcf 8f1e4f00
>> ff80: 00000000 8e6fff84 8e6fff84 00000000 8e6fff90 8e6fff90 8e6fffac 8f1e4f40
>> ffa0: 80031640 00000000 00000000 8000f548 00000000 00000000 00000000 00000000
>> ffc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
>> ffe0: 00000000 00000000 00000000 00000000 00000013 00000000 dfdfdfcf cfdfdfdf
>> [<801ea538>] (_setup_req) from [<801ebca0>] (pl330_tasklet+0x41c/0x490)
>> [<801ebca0>] (pl330_tasklet) from [<801e9044>] (dmatest_func+0xb58/0x149c)
>> [<801e9044>] (dmatest_func) from [<80031714>] (kthread+0xd4/0xec)
>> [<80031714>] (kthread) from [<8000f548>] (ret_from_fork+0x14/0x2c)
>> Code: e3a03001 e3e01043 e5c03001 e59d3048 (e5802002)
>>
>> This happens because _emit_{ADDH,MOV,GO) accessing to unaligned data
>> while writing to buffer. Fix it with writing to buffer byte by byte.
>>
>> Cc: Robin Murphy <robin.murphy@arm.com>
>> Cc: Vinod Koul <vinod.koul@intel.com>
>> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
>
> Reviewed-by: Robin Murphy <robin.murphy@arm.com>
>
> I don't have a NOMMU setup, but I checked arm64 (both little- and
> big-endian) sees no regressions:
>
> Tested-by: Robin Murphy <robin.murphy@arm.com>
Thanks, Robin!
Vladimir
>
> Cheers,
> Robin.
>
>> ---
>> drivers/dma/pl330.c | 15 ++++++++++-----
>> 1 file changed, 10 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
>> index 030fe05..e4c1ba7 100644
>> --- a/drivers/dma/pl330.c
>> +++ b/drivers/dma/pl330.c
>> @@ -570,7 +570,8 @@ static inline u32 _emit_ADDH(unsigned dry_run, u8 buf[],
>>
>> buf[0] = CMD_DMAADDH;
>> buf[0] |= (da << 1);
>> - *((__le16 *)&buf[1]) = cpu_to_le16(val);
>> + buf[1] = val;
>> + buf[2] = val >> 8;
>>
>> PL330_DBGCMD_DUMP(SZ_DMAADDH, "\tDMAADDH %s %u\n",
>> da == 1 ? "DA" : "SA", val);
>> @@ -724,7 +725,10 @@ static inline u32 _emit_MOV(unsigned dry_run, u8 buf[],
>>
>> buf[0] = CMD_DMAMOV;
>> buf[1] = dst;
>> - *((__le32 *)&buf[2]) = cpu_to_le32(val);
>> + buf[2] = val;
>> + buf[3] = val >> 8;
>> + buf[4] = val >> 16;
>> + buf[5] = val >> 24;
>>
>> PL330_DBGCMD_DUMP(SZ_DMAMOV, "\tDMAMOV %s 0x%x\n",
>> dst == SAR ? "SAR" : (dst == DAR ? "DAR" : "CCR"), val);
>> @@ -899,10 +903,11 @@ static inline u32 _emit_GO(unsigned dry_run, u8 buf[],
>>
>> buf[0] = CMD_DMAGO;
>> buf[0] |= (ns << 1);
>> -
>> buf[1] = chan & 0x7;
>> -
>> - *((__le32 *)&buf[2]) = cpu_to_le32(addr);
>> + buf[2] = addr;
>> + buf[3] = addr >> 8;
>> + buf[4] = addr >> 16;
>> + buf[5] = addr >> 24;
>>
>> return SZ_DMAGO;
>> }
>>
>
>
^ permalink raw reply
* [PATCH 6/7] arm: Add livepatch to build if CONFIG_LIVEPATCH
From: Petr Mladek @ 2016-12-07 15:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481043967-15602-7-git-send-email-abelvesa@linux.com>
On Tue 2016-12-06 17:06:06, Abel Vesa wrote:
> Necessary livepatch file added to makefile.
>
> Signed-off-by: Abel Vesa <abelvesa@linux.com>
> ---
> arch/arm/kernel/Makefile | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
> index ad325a8..9e70220 100644
> --- a/arch/arm/kernel/Makefile
> +++ b/arch/arm/kernel/Makefile
> @@ -48,6 +48,7 @@ obj-$(CONFIG_HAVE_ARM_TWD) += smp_twd.o
> obj-$(CONFIG_ARM_ARCH_TIMER) += arch_timer.o
> obj-$(CONFIG_FUNCTION_TRACER) += entry-ftrace.o
> obj-$(CONFIG_DYNAMIC_FTRACE) += ftrace.o insn.o
> +obj-$(CONFIG_LIVEPATCH) += livepatch.o
It is strange that you add a source file in one patch and make it
build in a much later patch.
I suggest to restructure the entire patchset a bit. Please, first
add support for FTRACE_WITH_REGS. It makes sense on its own.
Then add the livepatch support on top of it.
Otherwise, it is not necessary to send v2 immediately for such
non-trivial code. There might be more people that would want
to look at it and it might take days until they find a time.
It is always better to collect some feedback, think about it
over night(s). Every question often opens many other questions
and it usually takes some time until all settles down into
a good picture again.
Best Regards,
Petr
^ permalink raw reply
* [PATCH] dmaengine: pl330: do not generate unaligned access
From: Robin Murphy @ 2016-12-07 15:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481116660-27837-1-git-send-email-vladimir.murzin@arm.com>
On 07/12/16 13:17, Vladimir Murzin wrote:
> When PL330 is used with !MMU the following fault is seen:
>
> Unhandled fault: alignment exception (0x801) at 0x8f26a002
> Internal error: : 801 [#1] ARM
> Modules linked in:
> CPU: 0 PID: 640 Comm: dma0chan0-copy0 Not tainted 4.8.0-6a82063-clean+ #1600
> Hardware name: ARM-Versatile Express
> task: 8f1baa80 task.stack: 8e6fe000
> PC is at _setup_req+0x4c/0x350
> LR is at 0x8f2cbc00
> pc : [<801ea538>] lr : [<8f2cbc00>] psr: 60000093
> sp : 8e6ffdc0 ip : 00000000 fp : 00000000
> r10: 00000000 r9 : 8f2cba10 r8 : 8f2cbc00
> r7 : 80000013 r6 : 8f21a050 r5 : 8f21a000 r4 : 8f2ac800
> r3 : 8e6ffe18 r2 : 00944251 r1 : ffffffbc r0 : 8f26a000
> Flags: nZCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment none
> Control: 00c5387c
> Process dma0chan0-copy0 (pid: 640, stack limit = 0x8e6fe210)
> Stack: (0x8e6ffdc0 to 0x8e700000)
> fdc0: 00000001 60000093 00000000 8f2cba10 8f26a000 00000004 8f0ae000 8f2cbc00
> fde0: 8f0ae000 8f2ac800 8f21a000 8f21a050 80000013 8f2cbc00 8f2cba10 00000000
> fe00: 60000093 801ebca0 8e6ffe18 000013ff 40000093 00000000 00944251 8f2ac800
> fe20: a0000013 8f2b1320 00001986 00000000 00000001 000013ff 8f1e4f00 8f2cba10
> fe40: 8e6fff6c 801e9044 00000003 00000000 fef98c80 002faf07 8e6ffe7c 00000000
> fe60: 00000002 00000000 00001986 8f1f158d 8f1e4f00 80568de4 00000002 00000000
> fe80: 00001986 8f1f53ff 40000001 80580500 8f1f158d 8001e00c 00000000 cfdfdfdf
> fea0: fdae2a25 00000001 00000004 8e6fe000 00000008 00000010 00000000 00000005
> fec0: 8f2b1330 8f2b1334 8e6ffe80 8e6ffe8c 00001986 00000000 8f21a014 00000001
> fee0: 8e6ffe60 8e6ffe78 00000002 00000000 000013ff 00000001 80568de4 8f1e8018
> ff00: 0000158d 8055ec30 00000001 803f6b00 00001986 8f2cba10 fdae2a25 00000001
> ff20: 8f1baca8 8e6fff24 8e6fff24 00000000 8e6fff24 ac6f3037 00000000 00000000
> ff40: 00000000 8e6fe000 8f1e4f40 00000000 8f1e4f40 8f1e4f00 801e84ec 00000000
> ff60: 00000000 00000000 00000000 80031714 dfdfdfcf 00000000 dfdfdfcf 8f1e4f00
> ff80: 00000000 8e6fff84 8e6fff84 00000000 8e6fff90 8e6fff90 8e6fffac 8f1e4f40
> ffa0: 80031640 00000000 00000000 8000f548 00000000 00000000 00000000 00000000
> ffc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> ffe0: 00000000 00000000 00000000 00000000 00000013 00000000 dfdfdfcf cfdfdfdf
> [<801ea538>] (_setup_req) from [<801ebca0>] (pl330_tasklet+0x41c/0x490)
> [<801ebca0>] (pl330_tasklet) from [<801e9044>] (dmatest_func+0xb58/0x149c)
> [<801e9044>] (dmatest_func) from [<80031714>] (kthread+0xd4/0xec)
> [<80031714>] (kthread) from [<8000f548>] (ret_from_fork+0x14/0x2c)
> Code: e3a03001 e3e01043 e5c03001 e59d3048 (e5802002)
>
> This happens because _emit_{ADDH,MOV,GO) accessing to unaligned data
> while writing to buffer. Fix it with writing to buffer byte by byte.
>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: Vinod Koul <vinod.koul@intel.com>
> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
Reviewed-by: Robin Murphy <robin.murphy@arm.com>
I don't have a NOMMU setup, but I checked arm64 (both little- and
big-endian) sees no regressions:
Tested-by: Robin Murphy <robin.murphy@arm.com>
Cheers,
Robin.
> ---
> drivers/dma/pl330.c | 15 ++++++++++-----
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
> index 030fe05..e4c1ba7 100644
> --- a/drivers/dma/pl330.c
> +++ b/drivers/dma/pl330.c
> @@ -570,7 +570,8 @@ static inline u32 _emit_ADDH(unsigned dry_run, u8 buf[],
>
> buf[0] = CMD_DMAADDH;
> buf[0] |= (da << 1);
> - *((__le16 *)&buf[1]) = cpu_to_le16(val);
> + buf[1] = val;
> + buf[2] = val >> 8;
>
> PL330_DBGCMD_DUMP(SZ_DMAADDH, "\tDMAADDH %s %u\n",
> da == 1 ? "DA" : "SA", val);
> @@ -724,7 +725,10 @@ static inline u32 _emit_MOV(unsigned dry_run, u8 buf[],
>
> buf[0] = CMD_DMAMOV;
> buf[1] = dst;
> - *((__le32 *)&buf[2]) = cpu_to_le32(val);
> + buf[2] = val;
> + buf[3] = val >> 8;
> + buf[4] = val >> 16;
> + buf[5] = val >> 24;
>
> PL330_DBGCMD_DUMP(SZ_DMAMOV, "\tDMAMOV %s 0x%x\n",
> dst == SAR ? "SAR" : (dst == DAR ? "DAR" : "CCR"), val);
> @@ -899,10 +903,11 @@ static inline u32 _emit_GO(unsigned dry_run, u8 buf[],
>
> buf[0] = CMD_DMAGO;
> buf[0] |= (ns << 1);
> -
> buf[1] = chan & 0x7;
> -
> - *((__le32 *)&buf[2]) = cpu_to_le32(addr);
> + buf[2] = addr;
> + buf[3] = addr >> 8;
> + buf[4] = addr >> 16;
> + buf[5] = addr >> 24;
>
> return SZ_DMAGO;
> }
>
^ permalink raw reply
* [RFC v3 09/10] iommu/arm-smmu: Implement reserved region get/put callbacks
From: Auger Eric @ 2016-12-07 15:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <02c0ce15-fa67-e65f-3a4e-f543cc9fd3b8@arm.com>
Hi Robin,
On 06/12/2016 19:55, Robin Murphy wrote:
> On 15/11/16 13:09, Eric Auger wrote:
>> The get() populates the list with the PCI host bridge windows
>> and the MSI IOVA range.
>>
>> At the moment an arbitray MSI IOVA window is set at 0x8000000
>> of size 1MB. This will allow to report those info in iommu-group
>> sysfs?
First thank you for reviewing the series. This is definitively helpful!
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>
>> ---
>>
>> RFC v2 -> v3:
>> - use existing get/put_resv_regions
>>
>> RFC v1 -> v2:
>> - use defines for MSI IOVA base and length
>> ---
>> drivers/iommu/arm-smmu.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 52 insertions(+)
>>
>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
>> index 8f72814..81f1a83 100644
>> --- a/drivers/iommu/arm-smmu.c
>> +++ b/drivers/iommu/arm-smmu.c
>> @@ -278,6 +278,9 @@ enum arm_smmu_s2cr_privcfg {
>>
>> #define FSYNR0_WNR (1 << 4)
>>
>> +#define MSI_IOVA_BASE 0x8000000
>> +#define MSI_IOVA_LENGTH 0x100000
>> +
>> static int force_stage;
>> module_param(force_stage, int, S_IRUGO);
>> MODULE_PARM_DESC(force_stage,
>> @@ -1545,6 +1548,53 @@ static int arm_smmu_of_xlate(struct device *dev, struct of_phandle_args *args)
>> return iommu_fwspec_add_ids(dev, &fwid, 1);
>> }
>>
>> +static void arm_smmu_get_resv_regions(struct device *dev,
>> + struct list_head *head)
>> +{
>> + struct iommu_resv_region *region;
>> + struct pci_host_bridge *bridge;
>> + struct resource_entry *window;
>> +
>> + /* MSI region */
>> + region = iommu_alloc_resv_region(MSI_IOVA_BASE, MSI_IOVA_LENGTH,
>> + IOMMU_RESV_MSI);
>> + if (!region)
>> + return;
>> +
>> + list_add_tail(®ion->list, head);
>> +
>> + if (!dev_is_pci(dev))
>> + return;
>> +
>> + bridge = pci_find_host_bridge(to_pci_dev(dev)->bus);
>> +
>> + resource_list_for_each_entry(window, &bridge->windows) {
>> + phys_addr_t start;
>> + size_t length;
>> +
>> + if (resource_type(window->res) != IORESOURCE_MEM &&
>> + resource_type(window->res) != IORESOURCE_IO)
>
> As Joerg commented elsewhere, considering anything other than memory
> resources isn't right (I appreciate you've merely copied my own mistake
> here). We need some other way to handle root complexes where the CPU
> MMIO views of PCI windows appear in PCI memory space - using the I/O
> address of I/O resources only works by chance on Juno, and it still
> doesn't account for config space. I suggest we just leave that out for
> the time being to make life easier (does it even apply to anything other
> than Juno?) and figure it out later.
OK so I understand I should remove IORESOURCE_IO check.
>
>> + continue;
>> +
>> + start = window->res->start - window->offset;
>> + length = window->res->end - window->res->start + 1;
>> + region = iommu_alloc_resv_region(start, length,
>> + IOMMU_RESV_NOMAP);
>> + if (!region)
>> + return;
>> + list_add_tail(®ion->list, head);
>> + }
>> +}
>
> Either way, there's nothing SMMU-specific about PCI windows. The fact
> that we'd have to copy-paste all of this into the SMMUv3 driver
> unchanged suggests it should go somewhere common (although I would be
> inclined to leave the insertion of the fake MSI region to driver-private
> wrappers). As I said before, the current iova_reserve_pci_windows()
> simply wants splitting into appropriate public callbacks for
> get_resv_regions and apply_resv_regions.
Do you mean somewhere common in the arm-smmu subsystem (new file) or in
another subsystem (pci?)
More generally the current implementation does not handle the case where
any of those PCIe host bridge window collide with the MSI window. To me
this is a flaw.
1) Either we take into account the PCIe windows and prevent any
collision when allocating the MSI window.
2) or we do not care about PCIe host bridge windows at kernel level.
If 1) we are back to the original issue of where do we put the MSI
window. Obviously at a place which might not be QEMU friendly anymore.
What allocation policy shall we use?
Second option - sorry I may look stubborn - which I definitively prefer
and which was also advocated by Alex, we handle PCI host bridge windows
at user level. MSI window is reported through the iommu group sysfs.
PCIe host bridge windows can be enumerated through /proc/iomem. Both x86
iommu and arm smmu would report an MSI reserved window. ARM MSI window
would become a de facto reserved window for guests.
Thoughts?
Eric
>
> Robin.
>
>> +static void arm_smmu_put_resv_regions(struct device *dev,
>> + struct list_head *head)
>> +{
>> + struct iommu_resv_region *entry, *next;
>> +
>> + list_for_each_entry_safe(entry, next, head, list)
>> + kfree(entry);
>> +}
>> +
>> static struct iommu_ops arm_smmu_ops = {
>> .capable = arm_smmu_capable,
>> .domain_alloc = arm_smmu_domain_alloc,
>> @@ -1560,6 +1610,8 @@ static int arm_smmu_of_xlate(struct device *dev, struct of_phandle_args *args)
>> .domain_get_attr = arm_smmu_domain_get_attr,
>> .domain_set_attr = arm_smmu_domain_set_attr,
>> .of_xlate = arm_smmu_of_xlate,
>> + .get_resv_regions = arm_smmu_get_resv_regions,
>> + .put_resv_regions = arm_smmu_put_resv_regions,
>> .pgsize_bitmap = -1UL, /* Restricted during device attach */
>> };
>>
>>
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply
* [PATCH v3 0/4] mm: fix the "counter.sh" failure for libhugetlbfs
From: Michal Hocko @ 2016-12-07 15:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161206100358.GA4619@sha-win-210.asiapac.arm.com>
On Tue 06-12-16 18:03:59, Huang Shijie wrote:
> On Mon, Dec 05, 2016 at 05:31:01PM +0800, Michal Hocko wrote:
> > On Mon 05-12-16 17:17:07, Huang Shijie wrote:
> > [...]
> > > The failure is caused by:
> > > 1) kernel fails to allocate a gigantic page for the surplus case.
> > > And the gather_surplus_pages() will return NULL in the end.
> > >
> > > 2) The condition checks for some functions are wrong:
> > > return_unused_surplus_pages()
> > > nr_overcommit_hugepages_store()
> > > hugetlb_overcommit_handler()
> >
> > OK, so how is this any different from gigantic (1G) hugetlb pages on
> I think there is no different from gigantic (1G) hugetlb pages on
> x86_64. Do anyone ever tested the 1G hugetlb pages in x86_64 with the "counter.sh"
> before?
I suspect nobody has because the gigantic page support is still somehow
coarse and from a quick look into the code we only support pre-allocated
giga pages. In other words surplus pages and their accounting is not
supported at all.
I haven't yet checked your patchset but I can tell you one thing.
Surplus and subpool pages code is tricky as hell. And it is not just a
matter of teaching the huge page allocation code to do the right thing.
There are subtle details all over the place. E.g. we currently
do not free giga pages AFAICS. In fact I believe that the giga pages are
kind of implanted to the existing code without any higher level
consistency. This should change long term. But I am worried it is much
more work.
Now I might be wrong because I might misremember things which might have
been changed recently but please make sure you describe the current
state and changes of giga pages when touching this area much better if
you want to pursue this route...
Thanks!
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* [RFC PATCH v3 2/2] drm/panel: Add support for Chunghwa CLAA070WP03XG panel
From: Daniel Vetter @ 2016-12-07 14:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <2C9FE0C5-F521-48CD-B274-66128312CA75@soulik.info>
On Wed, Dec 07, 2016 at 08:57:23AM +0800, Ayaka wrote:
>
>
> ??? iPad ??
>
> > Thierry Reding <thierry.reding@gmail.com> ? 2016?12?6? ??11:46 ???
> >
> >> On Tue, Sep 20, 2016 at 03:02:51AM +0800, Randy Li wrote:
> >> The Chunghwa CLAA070WP03XG is a 7" 1280x800 panel, which can be
> >> supported by the simple panel driver.
> >>
> >> Signed-off-by: Randy Li <ayaka@soulik.info>
> >> ---
> >> .../display/panel/chunghwa,claa070wp03xg.txt | 7 ++++++
> >> drivers/gpu/drm/panel/panel-simple.c | 27 ++++++++++++++++++++++
> >> 2 files changed, 34 insertions(+)
> >> create mode 100644 Documentation/devicetree/bindings/display/panel/chunghwa,claa070wp03xg.txt
> >
> > Applied, thanks.
> Wait, it is RFC, not pass the test.
Well 2 months of silence, it's reasonable to assume that this works for
you ... I guess you need to supply a fixup patch asap ;-)
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply
* [PATCH v5 3/5] [media] davinci: vpif_capture: fix start/stop streaming locking
From: Laurent Pinchart @ 2016-12-07 14:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161207050826.23174-4-khilman@baylibre.com>
Hi Kevin,
Thank you for the patch.
On Tuesday 06 Dec 2016 21:08:24 Kevin Hilman wrote:
> Video capture subdevs may be over I2C and may sleep during xfer, so we
> cannot do IRQ-disabled locking when calling the subdev.
>
> The IRQ-disabled locking is meant to protect the DMA queue list
> throughout the rest of the driver, so update the locking in
> [start|stop]_streaming to protect just this list.
>
> Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Kevin Hilman <khilman@baylibre.com>
I would also add a comment to document the irqlock field as protecting the
dma_queue list only. Something like
- /* Used in video-buf */
+ /* Protects the dma_queue field */
spinlock_t irqlock;
With that,
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/media/platform/davinci/vpif_capture.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/platform/davinci/vpif_capture.c
> b/drivers/media/platform/davinci/vpif_capture.c index
> c24049acd40a..f72a719efb3d 100644
> --- a/drivers/media/platform/davinci/vpif_capture.c
> +++ b/drivers/media/platform/davinci/vpif_capture.c
> @@ -179,8 +179,6 @@ static int vpif_start_streaming(struct vb2_queue *vq,
> unsigned int count) unsigned long addr, flags;
> int ret;
>
> - spin_lock_irqsave(&common->irqlock, flags);
> -
> /* Initialize field_id */
> ch->field_id = 0;
>
> @@ -211,6 +209,7 @@ static int vpif_start_streaming(struct vb2_queue *vq,
> unsigned int count) vpif_config_addr(ch, ret);
>
> /* Get the next frame from the buffer queue */
> + spin_lock_irqsave(&common->irqlock, flags);
> common->cur_frm = common->next_frm = list_entry(common-
>dma_queue.next,
> struct vpif_cap_buffer, list);
> /* Remove buffer from the buffer queue */
> @@ -244,6 +243,7 @@ static int vpif_start_streaming(struct vb2_queue *vq,
> unsigned int count) return 0;
>
> err:
> + spin_lock_irqsave(&common->irqlock, flags);
> list_for_each_entry_safe(buf, tmp, &common->dma_queue, list) {
> list_del(&buf->list);
> vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_QUEUED);
> @@ -287,7 +287,6 @@ static void vpif_stop_streaming(struct vb2_queue *vq)
> vpif_dbg(1, debug, "stream off failed in subdev\n");
>
> /* release all active buffers */
> - spin_lock_irqsave(&common->irqlock, flags);
> if (common->cur_frm == common->next_frm) {
> vb2_buffer_done(&common->cur_frm->vb.vb2_buf,
> VB2_BUF_STATE_ERROR);
> @@ -300,6 +299,7 @@ static void vpif_stop_streaming(struct vb2_queue *vq)
> VB2_BUF_STATE_ERROR);
> }
>
> + spin_lock_irqsave(&common->irqlock, flags);
> while (!list_empty(&common->dma_queue)) {
> common->next_frm = list_entry(common->dma_queue.next,
> struct vpif_cap_buffer, list);
--
Regards,
Laurent Pinchart
^ 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