Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] crypto: sun4i-ss: support the Security System PRNG
From: Corentin Labbe @ 2016-12-07 12:51 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161207120900.GC20680@gondor.apana.org.au>

On Wed, Dec 07, 2016 at 08:09:00PM +0800, Herbert Xu wrote:
> On Mon, Dec 05, 2016 at 01:57:38PM +0100, Corentin Labbe wrote:
> >
> > So how to expose PRNG to user space ? or more generally how to "use" a PRNG ?
> 
> We do have the algif_rng interface.
> 

So I must expose it as a crypto_rng ?

Could you explain why PRNG must not be used as hw_random ?

Regards
Corentin Labbe

^ permalink raw reply

* [PATCH v3 1/5] spi: Add basic support for Armada 3700 SPI Controller
From: Romain Perier @ 2016-12-07 12:42 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161205120540.mvihgafelszitzhq@sirena.org.uk>

Hello,

Le 05/12/2016 ? 13:05, Mark Brown a ?crit :
> On Thu, Dec 01, 2016 at 11:27:15AM +0100, Romain Perier wrote:
>
>> +config SPI_ARMADA_3700
>> +	tristate "Marvell Armada 3700 SPI Controller"
>> +	depends on ARCH_MVEBU && OF
>
> Why does this not have a COMPILE_TEST dependency?

Because that's a mistake, I will fix it.

>
>> +	/* Reset SPI unit */
>> +	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
>> +	val |= A3700_SPI_SRST;
>> +	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
>> +
>> +	for (i = 0; i < A3700_SPI_TIMEOUT; i++)
>> +		udelay(1);
>
> Why not just do a single udelay()?

Mhhhh... good point.


>
>> +static irqreturn_t a3700_spi_interrupt(int irq, void *dev_id)
>> +{
>> +	struct spi_master *master = dev_id;
>> +	struct a3700_spi *a3700_spi;
>> +	u32 cause;
>> +
>> +	a3700_spi = spi_master_get_devdata(master);
>> +
>> +	/* Get interrupt causes */
>> +	cause = spireg_read(a3700_spi, A3700_SPI_INT_STAT_REG);
>> +
>> +	/* mask and acknowledge the SPI interrupts */
>> +	spireg_write(a3700_spi, A3700_SPI_INT_MASK_REG, 0);
>> +	spireg_write(a3700_spi, A3700_SPI_INT_STAT_REG, cause);
>> +
>> +	/* Wake up the transfer */
>> +	if (a3700_spi->wait_mask & cause)
>> +		complete(&a3700_spi->done);
>> +
>> +	return IRQ_HANDLED;
>> +}
>
> This reports that we handled an interrupt even if we did not in fact
> handle an interrupt.  It's also a bit dodgy that it doesn't check what
> the interrupt was but that's less serious.

I don't understand, what do you expect ? That I return something != 
IRQ_HANDLED when the cause of the interrupt is not present in wait_mask ?

>
>> +	master->bus_num = (pdev->id != -1) ? pdev->id : 0;
>
> At most this should be just setting the bus number to pdev->id like
> other drivers do.

ack

>
>> +	ret = clk_prepare_enable(spi->clk);
>> +	if (ret) {
>> +		dev_err(dev, "could not prepare clk: %d\n", ret);
>> +		goto error;
>> +	}
>
> I'd expect the hardware prepare/unprepare to be managing the enable and
> disable of the clock in order to save a little power.

Ok, if that's better for power management, why not then.

>
>> +	dev_info(dev, "Marvell Armada 3700 SPI Controller at 0x%08lx, irq %d\n",
>> +		 (unsigned long)res->start, spi->irq);
>
> This is just adding noise to the boot, remove it.  It's not telling us
> anything we read from the hardware or anything.

ack


Thanks,
Romain

^ permalink raw reply

* [PATCH v3 1/3] soc: zte: pm_domains: Prepare for supporting ARMv8 2967 family
From: Shawn Guo @ 2016-12-07 12:41 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1481091204-6559-1-git-send-email-baoyou.xie@linaro.org>

On Wed, Dec 07, 2016 at 02:13:22PM +0800, Baoyou Xie wrote:
> The ARMv8 2967 family (296718, 296716 etc) uses different value
> for controlling the power domain on/off registers, Choose the
> value depending on the compatible.
> 
> Multiple domains are prepared for the family, this patch prepares
> the common functions.
> 
> Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>
> ---
>  drivers/soc/Kconfig          |   1 +
>  drivers/soc/Makefile         |   1 +
>  drivers/soc/zte/Kconfig      |  13 ++++
>  drivers/soc/zte/Makefile     |   4 ++
>  drivers/soc/zte/pm_domains.c | 150 +++++++++++++++++++++++++++++++++++++++++++
>  drivers/soc/zte/pm_domains.h |  48 ++++++++++++++
>  6 files changed, 217 insertions(+)
>  create mode 100644 drivers/soc/zte/Kconfig
>  create mode 100644 drivers/soc/zte/Makefile
>  create mode 100644 drivers/soc/zte/pm_domains.c
>  create mode 100644 drivers/soc/zte/pm_domains.h
> 
> diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig
> index f31bceb..f09023f 100644
> --- a/drivers/soc/Kconfig
> +++ b/drivers/soc/Kconfig
> @@ -11,5 +11,6 @@ source "drivers/soc/tegra/Kconfig"
>  source "drivers/soc/ti/Kconfig"
>  source "drivers/soc/ux500/Kconfig"
>  source "drivers/soc/versatile/Kconfig"
> +source "drivers/soc/zte/Kconfig"
>  
>  endmenu
> diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
> index 50c23d0..05eae52 100644
> --- a/drivers/soc/Makefile
> +++ b/drivers/soc/Makefile
> @@ -16,3 +16,4 @@ obj-$(CONFIG_ARCH_TEGRA)	+= tegra/
>  obj-$(CONFIG_SOC_TI)		+= ti/
>  obj-$(CONFIG_ARCH_U8500)	+= ux500/
>  obj-$(CONFIG_PLAT_VERSATILE)	+= versatile/
> +obj-$(CONFIG_ARCH_ZX)		+= zte/
> diff --git a/drivers/soc/zte/Kconfig b/drivers/soc/zte/Kconfig
> new file mode 100644
> index 0000000..4953c3fa
> --- /dev/null
> +++ b/drivers/soc/zte/Kconfig
> @@ -0,0 +1,13 @@
> +#
> +# zx SoC drivers
> +#
> +menuconfig SOC_ZX

Can we use SOC_ZTE to reflect the folder name 'zte'?  If we take
drivers/soc/samsung/Kconfig as example, SAMSUNG is like ZTE, while
EXYNOS is like ZX, I guess.

> +        bool "zx SoC driver support"
> +
> +if SOC_ZX
> +
> +config ZX_PM_DOMAINS
> +        bool "zx PM domains"
> +        depends on PM_GENERIC_DOMAINS
> +
> +endif
> diff --git a/drivers/soc/zte/Makefile b/drivers/soc/zte/Makefile
> new file mode 100644
> index 0000000..97ac8ea
> --- /dev/null
> +++ b/drivers/soc/zte/Makefile
> @@ -0,0 +1,4 @@
> +#
> +# zx SOC drivers
> +#
> +obj-$(CONFIG_ZX_PM_DOMAINS) += pm_domains.o
> diff --git a/drivers/soc/zte/pm_domains.c b/drivers/soc/zte/pm_domains.c
> new file mode 100644
> index 0000000..e4d1235
> --- /dev/null
> +++ b/drivers/soc/zte/pm_domains.c
> @@ -0,0 +1,150 @@
> +/*
> + * Copyright (C) 2015 ZTE Ltd.

Do we at least need year 2016?

> + *
> + * Author: Baoyou Xie <baoyou.xie@linaro.org>
> + * License terms: GNU General Public License (GPL) version 2

I see drivers/soc/zte/pm_domains.h use a different licence format.  Can
we make them unified?

> + */
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include "pm_domains.h"
> +
> +#define PCU_DM_CLKEN(zpd)	((zpd)->reg_offset[REG_CLKEN])
> +#define PCU_DM_ISOEN(zpd)	((zpd)->reg_offset[REG_ISOEN])
> +#define PCU_DM_RSTEN(zpd)	((zpd)->reg_offset[REG_RSTEN])
> +#define PCU_DM_PWREN(zpd)	((zpd)->reg_offset[REG_PWREN])
> +#define PCU_DM_PWRDN(zpd)	((zpd)->reg_offset[REG_PWRDN])
> +#define PCU_DM_ACK_SYNC(zpd)	((zpd)->reg_offset[REG_ACK_SYNC])
> +
> +static void __iomem *pcubase;
> +
> +int zx_normal_power_on(struct generic_pm_domain *domain)

What does 'normal' in the function name mean?

> +{
> +	struct zx_pm_domain *zpd = (struct zx_pm_domain *)domain;
> +	unsigned long loop = 1000;
> +	u32 val;
> +
> +	if (zpd->polarity == PWREN) {
> +		val = readl_relaxed(pcubase + PCU_DM_PWREN(zpd));
> +		val |= BIT(zpd->bit);
> +		writel_relaxed(val, pcubase + PCU_DM_PWREN(zpd));
> +	} else {
> +		val = readl_relaxed(pcubase + PCU_DM_PWRDN(zpd));
> +		val &= ~BIT(zpd->bit);
> +		writel_relaxed(val, pcubase + PCU_DM_PWRDN(zpd));
> +	}
> +
> +	do {
> +		udelay(1);
> +		val = readl_relaxed(pcubase + PCU_DM_ACK_SYNC(zpd))
> +				   & BIT(zpd->bit);
> +	} while (--loop && !val);
> +
> +	if (!loop) {
> +		pr_err("Error: %s %s fail\n", __func__, domain->name);
> +		return -EIO;
> +	}
> +
> +	val = readl_relaxed(pcubase + PCU_DM_RSTEN(zpd));
> +	val &= ~BIT(zpd->bit);
> +	writel_relaxed(val | BIT(zpd->bit), pcubase + PCU_DM_RSTEN(zpd));

For a single bit setting, it can be as simple as the following, right?

	val = readl_relaxed(pcubase + PCU_DM_RSTEN(zpd));
	val |= BIT(zpd->bit);
	writel_relaxed(val, pcubase + PCU_DM_RSTEN(zpd));


> +	udelay(5);
> +
> +	val = readl_relaxed(pcubase + PCU_DM_ISOEN(zpd));
> +	val &= ~BIT(zpd->bit);
> +	writel_relaxed(val, pcubase + PCU_DM_ISOEN(zpd));
> +	udelay(5);
> +
> +	val = readl_relaxed(pcubase + PCU_DM_CLKEN(zpd));
> +	val &= ~BIT(zpd->bit);
> +	writel_relaxed(val | BIT(zpd->bit), pcubase + PCU_DM_CLKEN(zpd));

Ditto

> +	udelay(5);
> +
> +	pr_info("normal poweron %s\n", domain->name);

Do we really want to see a message on every single power domain on/off
function call?  A pr_debug is better?

> +
> +	return 0;
> +}
> +
> +int zx_normal_power_off(struct generic_pm_domain *domain)
> +{
> +	struct zx_pm_domain *zpd = (struct zx_pm_domain *)domain;
> +	unsigned long loop = 1000;
> +	u32 val;
> +
> +	val = readl_relaxed(pcubase + PCU_DM_CLKEN(zpd));
> +	val &= ~BIT(zpd->bit);
> +	writel_relaxed(val, pcubase + PCU_DM_CLKEN(zpd));
> +	udelay(5);
> +
> +	val = readl_relaxed(pcubase + PCU_DM_ISOEN(zpd));
> +	val &= ~BIT(zpd->bit);
> +	writel_relaxed(val | BIT(zpd->bit), pcubase + PCU_DM_ISOEN(zpd));

Same here.

> +	udelay(5);
> +
> +	val = readl_relaxed(pcubase + PCU_DM_RSTEN(zpd));
> +	val &= ~BIT(zpd->bit);
> +	writel_relaxed(val, pcubase + PCU_DM_RSTEN(zpd));
> +	udelay(5);
> +
> +	if (zpd->polarity == PWREN) {
> +		val = readl_relaxed(pcubase + PCU_DM_PWREN(zpd));
> +		val &= ~BIT(zpd->bit);
> +		writel_relaxed(val, pcubase + PCU_DM_PWREN(zpd));
> +	} else {
> +		val = readl_relaxed(pcubase + PCU_DM_PWRDN(zpd));
> +		val |= BIT(zpd->bit);
> +		writel_relaxed(val, pcubase + PCU_DM_PWRDN(zpd));
> +	}
> +
> +	do {
> +		udelay(1);
> +		val = readl_relaxed(pcubase + PCU_DM_ACK_SYNC(zpd))
> +				   & BIT(zpd->bit);
> +	} while (--loop && val);
> +
> +	if (!loop) {
> +		pr_err("Error: %s %s fail\n", __func__, domain->name);
> +		return -EIO;
> +	}
> +
> +	pr_info("normal poweroff %s\n", domain->name);

pr_debug

> +
> +	return 0;
> +}
> +
> +int
> +zx_pd_probe(struct platform_device *pdev,

I do not see the need to break above two lines.

> +	   struct generic_pm_domain **zx_pm_domains,
> +	   int domain_num)

As long as the line doesn't exceed column 80, we do not need to break
the line into two.

> +{
> +	struct genpd_onecell_data *genpd_data;
> +	struct resource *res;
> +	int i;
> +
> +	genpd_data = devm_kzalloc(&pdev->dev, sizeof(*genpd_data), GFP_KERNEL);
> +	if (!genpd_data)
> +		return -ENOMEM;
> +
> +	genpd_data->domains = zx_pm_domains;
> +	genpd_data->num_domains = domain_num;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res) {
> +		dev_err(&pdev->dev, "no memory resource defined\n");
> +		return -ENODEV;
> +	}

The error check on 'res' is not really necessary, as
devm_ioremap_resource() will do that for you.

> +
> +	pcubase = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(pcubase)) {
> +		dev_err(&pdev->dev, "ioremap fail.\n");
> +		return -EIO;

PTR_ERR(pcubase) should be returned as error code here.

> +	}
> +
> +	for (i = 0; i < domain_num; ++i)
> +		pm_genpd_init(zx_pm_domains[i], NULL, false);
> +
> +	of_genpd_add_provider_onecell(pdev->dev.of_node, genpd_data);
> +	dev_info(&pdev->dev, "powerdomain init ok\n");
> +	return 0;
> +}
> diff --git a/drivers/soc/zte/pm_domains.h b/drivers/soc/zte/pm_domains.h
> new file mode 100644
> index 0000000..d3a52fd
> --- /dev/null
> +++ b/drivers/soc/zte/pm_domains.h
> @@ -0,0 +1,48 @@
> +/*
> + * Copyright (c) 2015 ZTE Co., Ltd.
> + *           http://www.zte.com.cn
> + *
> + * Header for ZTE's Power Domain Driver support
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef __ZTE_PM_DOMAIN_H
> +#define __ZTE_PM_DOMAIN_H
> +
> +#include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
> +
> +enum {
> +	REG_CLKEN,
> +	REG_ISOEN,
> +	REG_RSTEN,
> +	REG_PWREN,
> +	REG_PWRDN,
> +	REG_ACK_SYNC,
> +
> +	/* The size of the array - must be last */
> +	REG_ARRAY_SIZE,
> +};
> +
> +enum zx_power_polarity {
> +	PWREN,
> +	PWRDN,
> +};
> +
> +struct zx_pm_domain {
> +	struct		generic_pm_domain dm;

You can chose to have more tabs between 'generic_pm_domain' and 'dm'
for indention, but there should always be one space between 'struct' and
'generic_pm_domain'.

> +	const u16	bit;
> +	const enum zx_power_polarity	polarity;
> +	const u16	*reg_offset;
> +};
> +
> +extern int zx_normal_power_on(struct generic_pm_domain *domain);
> +extern int zx_normal_power_off(struct generic_pm_domain *domain);
> +extern int
> +zx_pd_probe(struct platform_device *pdev,
> +	   struct generic_pm_domain **zx_pm_domains,
> +	   int domain_num);

We do not need to break it into so many lines.

> +#endif /* __ZTE_PM_DOMAIN_H */
> -- 
> 2.7.4
> 

^ permalink raw reply

* [Question] New mmap64 syscall?
From: Yury Norov @ 2016-12-07 12:39 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <0F280FED-870A-42B5-ABC4-1976ACA32462@theobroma-systems.com>

Hi Philipp,

On Wed, Dec 07, 2016 at 12:07:24PM +0100, Dr.Philipp Tomsich wrote:
> [Resend, as my mail-client had insisted on using the wrong MIME type?]
> 
> > On 07 Dec 2016, at 11:34, Yury Norov <ynorov@caviumnetworks.com> wrote:
> > 
> >> If there is a use case for larger than 16TB offsets, we should add
> >> the call on all architectures, probably using your approach 3. I don't
> >> think that we should treat it as anything special for arm64 though.
> > 
> > From this point of view, 16+TB offset is a matter of 16+TB storage,
> > and it's more than real. The other consideration to add it is that
> > we have 64-bit support for offsets in syscalls like sys_llseek().
> > So mmap64() will simply extend this support.
> 
> I believe the question is rather if the 16TB offset is a real use-case for ILP32.

This is not for ilp32, but for all 32-bit architectures - both native
and compat. And because the scope is so generic, I think it's the
strong reason for us to support true 64-bit offset in mmap().

> This seems to bring the discussion full-circle, as this would indicate that 64bit is the 
> preferred bit-width for all sizes, offsets, etc. throughout all filesystem-related calls 
> (i.e. stat, seek, etc.).

AARCH64/ILP32 (and all new arches) exposes ino_t, off_t, blkcnt_t,
fsblkcnt_t, fsfilcnt_t and rlim_t as 64-bit types. (Size_t should
be 32-bit of course, because it's the same lengths as pointer.)

It allows to make syscalls that pass it support 64-bit values, refer
Documentation/arm64/ilp32.txt for details. Stat and seek are both
supporting 64-bit types. From this point of view, mmap() is the (only?)
exception in current ILP32 ABI.

> But if that is the case, then we should have gone with 64bit arguments in a single
> register for our ILP32 definition on AArch64.
 
There are 2 unrelated matters - the size of types, and the size of
register. Most of 32-bit architectures has hardware limitation on
register size (consider aarch32). And it doesn't mean that they are
forced to stuck with 32-bit off_t etc. This is still opened question
how to pass 64-bit parameters in aarch64/ilp32 because there we have
the choice (the reason why it's RFC). If you have new ideas - welcome
to that discussion. This topic also covers architectures that has to
pass 64-bit parameters in a pair.

> In other words: Why not keep ILP32 simple an ask users that need a 16TB+ offset
> to use LP64? It seems much more consistent with the other choices takes so far.

If user can switch to lp64, he doesn't need ilp32 at all, right? :)
Also, I don't understand how true 64-bit offset in mmap64() would
complicate this port.

Yury

^ permalink raw reply

* [PATCH] arm/xen: Use alloc_percpu rather than __alloc_percpu
From: Julien Grall @ 2016-12-07 12:24 UTC (permalink / raw)
  To: linux-arm-kernel

The function xen_guest_init is using __alloc_percpu with an alignment
which are not power of two.

However, the percpu allocator never supported alignments which are not power
of two and has always behaved incorectly in thise case.

Commit 3ca45a4 "percpu: ensure requested alignment is power of two"
introduced a check which trigger a warning [1] when booting linux-next
on Xen. But in fine this bug was always present.

This can be fixed by replacing the call to __alloc_percpu with
alloc_percpu. The latter will use an alignment which are a power of two.

[1]

[    0.023921] illegal size (48) or align (48) for percpu allocation
[    0.024167] ------------[ cut here ]------------
[    0.024344] WARNING: CPU: 0 PID: 1 at linux/mm/percpu.c:892 pcpu_alloc+0x88/0x6c0
[    0.024584] Modules linked in:
[    0.024708]
[    0.024804] CPU: 0 PID: 1 Comm: swapper/0 Not tainted
4.9.0-rc7-next-20161128 #473
[    0.025012] Hardware name: Foundation-v8A (DT)
[    0.025162] task: ffff80003d870000 task.stack: ffff80003d844000
[    0.025351] PC is at pcpu_alloc+0x88/0x6c0
[    0.025490] LR is at pcpu_alloc+0x88/0x6c0
[    0.025624] pc : [<ffff00000818e678>] lr : [<ffff00000818e678>]
pstate: 60000045
[    0.025830] sp : ffff80003d847cd0
[    0.025946] x29: ffff80003d847cd0 x28: 0000000000000000
[    0.026147] x27: 0000000000000000 x26: 0000000000000000
[    0.026348] x25: 0000000000000000 x24: 0000000000000000
[    0.026549] x23: 0000000000000000 x22: 00000000024000c0
[    0.026752] x21: ffff000008e97000 x20: 0000000000000000
[    0.026953] x19: 0000000000000030 x18: 0000000000000010
[    0.027155] x17: 0000000000000a3f x16: 00000000deadbeef
[    0.027357] x15: 0000000000000006 x14: ffff000088f79c3f
[    0.027573] x13: ffff000008f79c4d x12: 0000000000000041
[    0.027782] x11: 0000000000000006 x10: 0000000000000042
[    0.027995] x9 : ffff80003d847a40 x8 : 6f697461636f6c6c
[    0.028208] x7 : 6120757063726570 x6 : ffff000008f79c84
[    0.028419] x5 : 0000000000000005 x4 : 0000000000000000
[    0.028628] x3 : 0000000000000000 x2 : 000000000000017f
[    0.028840] x1 : ffff80003d870000 x0 : 0000000000000035
[    0.029056]
[    0.029152] ---[ end trace 0000000000000000 ]---
[    0.029297] Call trace:
[    0.029403] Exception stack(0xffff80003d847b00 to
                               0xffff80003d847c30)
[    0.029621] 7b00: 0000000000000030 0001000000000000
ffff80003d847cd0 ffff00000818e678
[    0.029901] 7b20: 0000000000000002 0000000000000004
ffff000008f7c060 0000000000000035
[    0.030153] 7b40: ffff000008f79000 ffff000008c4cd88
ffff80003d847bf0 ffff000008101778
[    0.030402] 7b60: 0000000000000030 0000000000000000
ffff000008e97000 00000000024000c0
[    0.030647] 7b80: 0000000000000000 0000000000000000
0000000000000000 0000000000000000
[    0.030895] 7ba0: 0000000000000035 ffff80003d870000
000000000000017f 0000000000000000
[    0.031144] 7bc0: 0000000000000000 0000000000000005
ffff000008f79c84 6120757063726570
[    0.031394] 7be0: 6f697461636f6c6c ffff80003d847a40
0000000000000042 0000000000000006
[    0.031643] 7c00: 0000000000000041 ffff000008f79c4d
ffff000088f79c3f 0000000000000006
[    0.031877] 7c20: 00000000deadbeef 0000000000000a3f
[    0.032051] [<ffff00000818e678>] pcpu_alloc+0x88/0x6c0
[    0.032229] [<ffff00000818ece8>] __alloc_percpu+0x18/0x20
[    0.032409] [<ffff000008d9606c>] xen_guest_init+0x174/0x2f4
[    0.032591] [<ffff0000080830f8>] do_one_initcall+0x38/0x130
[    0.032783] [<ffff000008d90c34>] kernel_init_freeable+0xe0/0x248
[    0.032995] [<ffff00000899a890>] kernel_init+0x10/0x100
[    0.033172] [<ffff000008082ec0>] ret_from_fork+0x10/0x50

Reported-by: Wei Chen <wei.chen@arm.com>
Link: https://lkml.org/lkml/2016/11/28/669
Signed-off-by: Julien Grall <julien.grall@arm.com>

Cc: stable at vger.kernel.org

---

I have requested backport to stable because the percpu allocator may
misbehaves when the alignment is not a power of two.
---
 arch/arm/xen/enlighten.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index f193414..4986dc0 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -372,8 +372,7 @@ static int __init xen_guest_init(void)
 	 * for secondary CPUs as they are brought up.
 	 * For uniformity we use VCPUOP_register_vcpu_info even on cpu0.
 	 */
-	xen_vcpu_info = __alloc_percpu(sizeof(struct vcpu_info),
-			                       sizeof(struct vcpu_info));
+	xen_vcpu_info = alloc_percpu(struct vcpu_info);
 	if (xen_vcpu_info == NULL)
 		return -ENOMEM;
 
-- 
1.9.1

^ permalink raw reply related

* [PATCH 4/7] arm: Add ftrace with regs support
From: Abel Vesa @ 2016-12-07 12:10 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <cf63d647-b5ce-a41b-31ee-6d14da60f541@arm.com>

On Wed, Dec 07, 2016 at 11:58:24AM +0000, Robin Murphy wrote:
> Hi Abel,
> 
> On 06/12/16 17:06, Abel Vesa wrote:
> > This adds __ftrace_regs_caller which, unlike __ftrace_caller,
> > adds register saving/restoring and livepatch handling if
> > the pc register gets modified by klp_ftrace_handler.
> > 
> > Signed-off-by: Abel Vesa <abelvesa@linux.com>
> > ---
> >  arch/arm/kernel/entry-ftrace.S | 49 ++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 49 insertions(+)
> > 
> > diff --git a/arch/arm/kernel/entry-ftrace.S b/arch/arm/kernel/entry-ftrace.S
> > index c73c403..b6ada5c 100644
> > --- a/arch/arm/kernel/entry-ftrace.S
> > +++ b/arch/arm/kernel/entry-ftrace.S
> > @@ -92,6 +92,46 @@
> >  2:	mcount_exit
> >  .endm
> >  
> > +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
> > +
> > +.macro __ftrace_regs_caller suffix
> > +
> > +	stmdb	sp!, {r0-r15}
> 
> As the kbuild robot reported, you can't do this. For ARM, it's unknown
> what the value stored for r13 will be, and for a Thumb-2 kernel the
> whole instruction is flat out unpredictable (i.e. it might just undef or
> anything).
> 
> > +	mov	r3, sp
> > +
> > +	ldr	r10, [sp, #60]
> > +
> > +	mcount_get_lr   r1                      @ lr of instrumented func
> > +	mcount_adjust_addr	r0, lr		@ instrumented function
> > +
> > +	.globl ftrace_regs_call\suffix
> > +ftrace_regs_call\suffix:
> > +	bl	ftrace_stub
> > +
> > +#ifdef CONFIG_FUNCTION_GRAPH_TRACER
> > +	.globl ftrace_regs_graph_call\suffix
> > +ftrace_regs_graph_call\suffix:
> > +	mov	r0, r0
> > +#endif
> > +#ifdef CONFIG_LIVEPATCH
> > +	ldr	r0, [sp, #60]
> > +	cmp	r0, r10
> > +	beq	ftrace_regs_caller_end
> > +	ldmia	sp!, {r0-r12}
> > +	add	sp, sp, #8
> > +	ldmia	sp!, {r11}
> > +	sub	sp, r12, #16
> > +	str	r11, [sp, #12]
> > +	ldmia	sp!, {r11, r12, lr, pc}
> > +#endif
> > +ftrace_regs_caller_end\suffix:
> > +	ldmia	sp!, {r0-r14}
> 
> Again, the value of SP at this point is now unknown (regardless of
> whether what the value on memory was correct or not). Not good.
> 
> Either use non-writeback addressing modes, or avoid saving/restoring SP
> at all (AFAICS it isn't necessary, since if the SP was changed in
> between, you then wouldn't know where to restore the saved registers
> from anyway).
> 
> Robin.
>
Yep, as I said in the cover letter, I'll try to fix that in the next
iteration of this patch. You're right, sp doesn't need to be pushed or
popped. I'll send a another patch series which will include a fix for 
that tomorrow, latest.

Thanks. 
> > +	add	sp, sp, #4
> > +	mov	pc, lr
> > +.endm
> > +
> > +#endif
> > +
> >  .macro __ftrace_caller suffix
> >  	mcount_enter
> >  
> > @@ -212,6 +252,15 @@ UNWIND(.fnstart)
> >  	__ftrace_caller
> >  UNWIND(.fnend)
> >  ENDPROC(ftrace_caller)
> > +
> > +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
> > +ENTRY(ftrace_regs_caller)
> > +UNWIND(.fnstart)
> > +	__ftrace_regs_caller
> > +UNWIND(.fnend)
> > +ENDPROC(ftrace_regs_caller)
> > +#endif
> > +
> >  #endif
> >  
> >  #ifdef CONFIG_FUNCTION_GRAPH_TRACER
> > 
> 

^ permalink raw reply

* [PATCH v3 0/6] crypto: ARM/arm64 CRC-T10DIF/CRC32/CRC32C roundup
From: Herbert Xu @ 2016-12-07 12:09 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1480963348-24203-1-git-send-email-ard.biesheuvel@linaro.org>

On Mon, Dec 05, 2016 at 06:42:22PM +0000, Ard Biesheuvel wrote:
> This v3 combines the CRC-T10DIF and CRC32 implementations for both ARM and
> arm64 that I sent out a couple of weeks ago, and adds support to the latter
> for CRC32C.

All applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* [PATCH 05/16] drivers/fsi: Add fake master driver
From: Mark Rutland @ 2016-12-07 12:09 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1481069677-53660-6-git-send-email-christopher.lee.bostic@gmail.com>

On Tue, Dec 06, 2016 at 06:14:26PM -0600, Chris Bostic wrote:
> From: Jeremy Kerr <jk@ozlabs.org>
> 
> For debugging, add a fake master driver, that only supports reads,
> returning a fixed set of data.

> +config FSI_MASTER_FAKE
> +	tristate "Fake FSI master"
> +	depends on FSI
> +	---help---
> +	This option enables a fake FSI master driver for debugging.
> +endif

> +static const struct of_device_id fsi_master_fake_match[] = {
> +	{ .compatible = "ibm,fsi-master-fake" },
> +	{ },
> +};

NAK.

DT should be treated as an ABI, and should describe the HW explicitly.
This makes no sense. This is also missing a binding document.

Have your module take a module parameter allowing you to bind it to
arbitrary devices, or do something like what PCI does where you can
bind/unbind arbitrary drivers to devices using sysfs.

> +
> +static struct platform_driver fsi_master_fake_driver = {
> +	.driver = {
> +		.name		= "fsi-master-fake",
> +		.of_match_table	= fsi_master_fake_match,
> +	},
> +	.probe	= fsi_master_fake_probe,
> +};
> +
> +static int __init fsi_master_fake_init(void)
> +{
> +	struct device_node *np;
> +
> +	platform_driver_register(&fsi_master_fake_driver);
> +
> +	for_each_compatible_node(np, NULL, "ibm,fsi-master-fake")
> +		of_platform_device_create(np, NULL, NULL);

As a general note, please use for_each_matching_node in situations like
this. That way you can reuse your existing of_device_id table, and not
reproduce the string.

That said, this is not necessary. The platform driver has an
of_match_table, so presumes the parent bus registers children, and hence
they should already have platform devices.

Thanks,
Mark.

^ permalink raw reply

* [PATCH v2] crypto: sun4i-ss: support the Security System PRNG
From: Herbert Xu @ 2016-12-07 12:09 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161205125738.GA13525@Red>

On Mon, Dec 05, 2016 at 01:57:38PM +0100, Corentin Labbe wrote:
>
> So how to expose PRNG to user space ? or more generally how to "use" a PRNG ?

We do have the algif_rng interface.

> I found hisi-rng, crypto4xx_ and exynos-rng which seems to be PRNG used as hwrng.

Thanks for checking.  Patches to remove these are welcome.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* [PATCH 15/16] drivers/fsi: Add documentation for GPIO bindings
From: Mark Rutland @ 2016-12-07 12:02 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1481069677-53660-16-git-send-email-christopher.lee.bostic@gmail.com>

On Tue, Dec 06, 2016 at 06:14:36PM -0600, Chris Bostic wrote:
> From: Chris Bostic <cbostic@us.ibm.com>
> 
> Add fsi master gpio device tree binding documentation

Please see Documentation/devicetree/bindings/submitting-patches.txt.

Specifically:

* Please put binding documents earlier in the series than code
  implementing the binding.

* Please document _all_ compatible strings used in the
  series.

Please also write the binding documents in terms of the hardware, rather
then the driver (e.g. introduce what the hardware is in the document,
don't mention the driver). The bindings are there to describe the former
to the latter, and the latter may change arbitrarily.

> Signed-off-by: Chris Bostic <cbostic@us.ibm.com>
> ---
>  .../devicetree/bindings/fsi/fsi-master-gpio.txt     | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/fsi/fsi-master-gpio.txt
> 
> diff --git a/Documentation/devicetree/bindings/fsi/fsi-master-gpio.txt b/Documentation/devicetree/bindings/fsi/fsi-master-gpio.txt

AFAICT, this is the first use of this directory. We should have a
general FSI binding document in there, covering what FSI is, the
"ibm,fsi-master" binding, etc.

> new file mode 100644
> index 0000000..ff3a62e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/fsi/fsi-master-gpio.txt
> @@ -0,0 +1,21 @@
> +Device-tree bindings for gpio-based FSI master driver

There's very little information here, so I'm not sure what to make of
this. Can you please elaborate on the above to make it clear what this
means?

IIUC, this is an FSI controller/master that we only communicate with via
GPIOs, right?

Or is this a *virtual* master? i.e. the GPIOs themselves form the master
and are directly connected to slaves?

> +-----------------------------------------------------
> +
> +Required properties:
> +	- compatible = "ibm,fsi-master-gpio";
> +	- clk-gpios;
> +	- data-gpios;

Please give a description of what each of these are used for, how many
are required, and what order elements must come in.

> +Optional properties:
> +	- enable-gpios;
> +	- trans-gpios;
> +	- mux-gpios;

Likewise.

> +
> +fsi-master {
> +	compatible = "ibm,fsi-master", "ibm,fsi-master-gpio";

This is backwards. The most specific string must come first.

> +	clk-gpios = <&gpio 0 &gpio 6>;
> +	data-gpios = <&gpio 1 &gpio 7>;
> +	enable-gpios = <&gpio 2 &gpio 8>;	/* Enable FSI data in/out */
> +	trans-gpios = <&gpio 3 &gpio 9>;	/* Volts translator direction */
> +	mux-gpios = <&gpio 4> &gpio 10>;	/* Multiplexer for FSI pins */

If this were described above, we don't need the comment here.

I note that in the patch, the mux-gpios property has an unmatched '>'
and won't compile.

As a general nit, please bracket elements of a list individually, e.g.

	trans-gpios = <&gpio 3>, <&gpio 9>;
	mux-gpios = <&gpio 4>, <&gpio 10>;

Thanks,
Mark.

^ permalink raw reply

* [PATCH v4 6/7] IIO: add STM32 timer trigger driver
From: Daniel Thompson @ 2016-12-07 12:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CA+M3ks4VbV6z6t7aORKGmduZfFF+Ls7Sq8umWRFyo3sn8WZt7g@mail.gmail.com>

On 07/12/16 11:00, Benjamin Gaignard wrote:
> 2016-12-07 11:50 GMT+01:00 Lee Jones <lee.jones@linaro.org>:
>> On Tue, 06 Dec 2016, Benjamin Gaignard wrote:
>>
>>> [snip]
>>>>> +
>>>>> +static const char * const triggers0[] = {
>>>>> +     TIM1_TRGO, TIM1_CH1, TIM1_CH2, TIM1_CH3, TIM1_CH4, NULL,
>>>>> +};
>>>>> +
>>>>> +static const char * const triggers1[] = {
>>>>> +     TIM2_TRGO, TIM2_CH1, TIM2_CH2, TIM2_CH3, TIM2_CH4, NULL,
>>>>> +};
>>>>> +
>>>>> +static const char * const triggers2[] = {
>>>>> +     TIM3_TRGO, TIM3_CH1, TIM3_CH2, TIM3_CH3, TIM3_CH4, NULL,
>>>>> +};
>>>>> +
>>>>> +static const char * const triggers3[] = {
>>>>> +     TIM4_TRGO, TIM4_CH1, TIM4_CH2, TIM4_CH3, TIM4_CH4, NULL,
>>>>> +};
>>>>> +
>>>>> +static const char * const triggers4[] = {
>>>>> +     TIM5_TRGO, TIM5_CH1, TIM5_CH2, TIM5_CH3, TIM5_CH4, NULL,
>>>>> +};
>>>>> +
>>>>> +static const char * const triggers5[] = {
>>>>> +     TIM6_TRGO, NULL,
>>>>> +};
>>>>> +
>>>>> +static const char * const triggers6[] = {
>>>>> +     TIM7_TRGO, NULL,
>>>>> +};
>>>>> +
>>>>> +static const char * const triggers7[] = {
>>>>> +     TIM8_TRGO, TIM8_CH1, TIM8_CH2, TIM8_CH3, TIM8_CH4, NULL,
>>>>> +};
>>>>> +
>>>>> +static const char * const triggers8[] = {
>>>>> +     TIM9_TRGO, TIM9_CH1, TIM9_CH2, NULL,
>>>>> +};
>>>>> +
>>>>> +static const char * const triggers9[] = {
>>>>> +     TIM12_TRGO, TIM12_CH1, TIM12_CH2, NULL,
>>>>> +};
>>>>> +
>>>>> +static const void *triggers_table[] = {
>>>>> +     triggers0,
>>>>> +     triggers1,
>>>>> +     triggers2,
>>>>> +     triggers3,
>>>>> +     triggers4,
>>>>> +     triggers5,
>>>>> +     triggers6,
>>>>> +     triggers7,
>>>>> +     triggers8,
>>>>> +     triggers9,
>>>>> +};
>>>>
>>>> What about:
>>>>
>>>> static const char * const triggers[][] = {
>>>>         { TIM1_TRGO, TIM1_CH1, TIM1_CH2, TIM1_CH3, TIM1_CH4, NULL },
>>>>         { TIM2_TRGO, TIM2_CH1, TIM2_CH2, TIM2_CH3, TIM2_CH4, NULL },
>>>>         { TIM3_TRGO, TIM3_CH1, TIM3_CH2, TIM3_CH3, TIM3_CH4, NULL },
>>>>         { TIM4_TRGO, TIM4_CH1, TIM4_CH2, TIM4_CH3, TIM4_CH4, NULL },
>>>>         { TIM5_TRGO, TIM5_CH1, TIM5_CH2, TIM5_CH3, TIM5_CH4, NULL },
>>>>         { TIM6_TRGO, NULL },
>>>>         { TIM7_TRGO, NULL },
>>>>         { TIM8_TRGO, TIM8_CH1, TIM8_CH2, TIM8_CH3, TIM8_CH4, NULL },
>>>>         { TIM9_TRGO, TIM9_CH1, TIM9_CH2, NULL },
>>>>         { TIM12_TRGO, TIM12_CH1, TIM12_CH2, NULL }
>>>> };
>>>
>>> I can't because the second dimension of the array isn't fix.
>>> I could have between 2 and 6 elements per row... to create a dual dimension
>>> array I would have to add NULL entries like that:
>>>
>>> #define MAX_TRIGGERS 6
>>>
>>> static const void *triggers_table[][MAX_TRIGGERS] = {
>>> { TIM1_TRGO, TIM1_CH1, TIM1_CH2, TIM1_CH3, TIM1_CH4, NULL,},
>>> { TIM2_TRGO, TIM2_CH1, TIM2_CH2, TIM2_CH3, TIM2_CH4, NULL,},
>>> { TIM3_TRGO, TIM3_CH1, TIM3_CH2, TIM3_CH3, TIM3_CH4, NULL,},
>>> { TIM4_TRGO, TIM4_CH1, TIM4_CH2, TIM4_CH3, TIM4_CH4, NULL,},
>>> { TIM5_TRGO, TIM5_CH1, TIM5_CH2, TIM5_CH3, TIM5_CH4, NULL,},
>>> { TIM6_TRGO, NULL,     NULL,     NULL,     NULL,     NULL,},
>>> { TIM7_TRGO, NULL,     NULL,     NULL,     NULL,     NULL,},
>>> { TIM8_TRGO, TIM8_CH1, TIM8_CH2, TIM8_CH3, TIM8_CH4, NULL,},
>>> { TIM9_TRGO, TIM9_CH1, TIM9_CH2, NULL,     NULL,     NULL,},
>>> { TIM12_TRGO, TIM12_CH1, TIM12_CH2, NULL,  NULL,     NULL,},
>>> };
>>
>> It was just an idea, not a tested implementation.
>>
>> I don't understand why you have to pad with NULLs, but either way, it
>> looks much better than before and saves lots of lines of code.
>
> I have tested it this morning and it works fine so I will include it in v5.
> I use NULL as limit when iterate in the table and for table padding too.

If the initializer is shorter than the array then the array will be 
implicitly zero/NULL padded. I don't think there is any need to type out 
all the NULLs (not even at -Wall).


Daniel.

^ permalink raw reply

* [PATCH 4/7] arm: Add ftrace with regs support
From: Robin Murphy @ 2016-12-07 11:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1481043967-15602-5-git-send-email-abelvesa@linux.com>

Hi Abel,

On 06/12/16 17:06, Abel Vesa wrote:
> This adds __ftrace_regs_caller which, unlike __ftrace_caller,
> adds register saving/restoring and livepatch handling if
> the pc register gets modified by klp_ftrace_handler.
> 
> Signed-off-by: Abel Vesa <abelvesa@linux.com>
> ---
>  arch/arm/kernel/entry-ftrace.S | 49 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 49 insertions(+)
> 
> diff --git a/arch/arm/kernel/entry-ftrace.S b/arch/arm/kernel/entry-ftrace.S
> index c73c403..b6ada5c 100644
> --- a/arch/arm/kernel/entry-ftrace.S
> +++ b/arch/arm/kernel/entry-ftrace.S
> @@ -92,6 +92,46 @@
>  2:	mcount_exit
>  .endm
>  
> +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
> +
> +.macro __ftrace_regs_caller suffix
> +
> +	stmdb	sp!, {r0-r15}

As the kbuild robot reported, you can't do this. For ARM, it's unknown
what the value stored for r13 will be, and for a Thumb-2 kernel the
whole instruction is flat out unpredictable (i.e. it might just undef or
anything).

> +	mov	r3, sp
> +
> +	ldr	r10, [sp, #60]
> +
> +	mcount_get_lr   r1                      @ lr of instrumented func
> +	mcount_adjust_addr	r0, lr		@ instrumented function
> +
> +	.globl ftrace_regs_call\suffix
> +ftrace_regs_call\suffix:
> +	bl	ftrace_stub
> +
> +#ifdef CONFIG_FUNCTION_GRAPH_TRACER
> +	.globl ftrace_regs_graph_call\suffix
> +ftrace_regs_graph_call\suffix:
> +	mov	r0, r0
> +#endif
> +#ifdef CONFIG_LIVEPATCH
> +	ldr	r0, [sp, #60]
> +	cmp	r0, r10
> +	beq	ftrace_regs_caller_end
> +	ldmia	sp!, {r0-r12}
> +	add	sp, sp, #8
> +	ldmia	sp!, {r11}
> +	sub	sp, r12, #16
> +	str	r11, [sp, #12]
> +	ldmia	sp!, {r11, r12, lr, pc}
> +#endif
> +ftrace_regs_caller_end\suffix:
> +	ldmia	sp!, {r0-r14}

Again, the value of SP at this point is now unknown (regardless of
whether what the value on memory was correct or not). Not good.

Either use non-writeback addressing modes, or avoid saving/restoring SP
at all (AFAICS it isn't necessary, since if the SP was changed in
between, you then wouldn't know where to restore the saved registers
from anyway).

Robin.

> +	add	sp, sp, #4
> +	mov	pc, lr
> +.endm
> +
> +#endif
> +
>  .macro __ftrace_caller suffix
>  	mcount_enter
>  
> @@ -212,6 +252,15 @@ UNWIND(.fnstart)
>  	__ftrace_caller
>  UNWIND(.fnend)
>  ENDPROC(ftrace_caller)
> +
> +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
> +ENTRY(ftrace_regs_caller)
> +UNWIND(.fnstart)
> +	__ftrace_regs_caller
> +UNWIND(.fnend)
> +ENDPROC(ftrace_regs_caller)
> +#endif
> +
>  #endif
>  
>  #ifdef CONFIG_FUNCTION_GRAPH_TRACER
> 

^ permalink raw reply

* [PATCH] dt: bindings: zx: Add header for PM domains specifiers
From: Shawn Guo @ 2016-12-07 11:48 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1480983711-29955-1-git-send-email-baoyou.xie@linaro.org>

On Tue, Dec 06, 2016 at 08:21:51AM +0800, Baoyou Xie wrote:
> This patch adds header with values used for ZTE 2967
> SoC's power domain driver.
> 
> Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>
> ---
>  include/dt-bindings/arm/zte_pm_domains.h | 23 +++++++++++++++++++++++

Considering that we are adding power domain drivers into
drivers/soc/zte, it might be better to put the header into folder
include/dt-bindings/soc/.

>  1 file changed, 23 insertions(+)
>  create mode 100644 include/dt-bindings/arm/zte_pm_domains.h
> 
> diff --git a/include/dt-bindings/arm/zte_pm_domains.h b/include/dt-bindings/arm/zte_pm_domains.h
> new file mode 100644
> index 0000000..1485e8d
> --- /dev/null
> +++ b/include/dt-bindings/arm/zte_pm_domains.h
> @@ -0,0 +1,23 @@
> +/*
> + * Copyright (C) 2015 Linaro Ltd.
> + *
> + * Author: Baoyou Xie <baoyou.xie@linaro.org>
> + * License terms: GNU General Public License (GPL) version 2
> + */
> +#ifndef _DT_BINDINGS_ARM_ZTE_PM_DOMAINS_H
> +#define _DT_BINDINGS_ARM_ZTE_PM_DOMAINS_H
> +
> +#define DM_ZX296718_SAPPU	0
> +#define DM_ZX296718_VDE		1  /*g1v6*/
> +#define DM_ZX296718_VCE		2  /*h1v6*/
> +#define DM_ZX296718_HDE		3  /*g2v2*/

The single line comment should be /* blabla */.  Note there is space
after and before *.

Shawn

> +#define DM_ZX296718_VIU		4
> +#define DM_ZX296718_USB20	5
> +#define DM_ZX296718_USB21	6
> +#define DM_ZX296718_USB30	7
> +#define DM_ZX296718_HSIC	8
> +#define DM_ZX296718_GMAC	9
> +#define DM_ZX296718_TS		10
> +#define DM_ZX296718_VOU		11
> +
> +#endif /* _DT_BINDINGS_ARM_ZTE_PM_DOMAINS_H */
> -- 
> 2.7.4
> 

^ permalink raw reply

* [RFC PATCH 00/23] arm: defconfigs: use kconfig fragments
From: Bartlomiej Zolnierkiewicz @ 2016-12-07 11:41 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAOesGMi6gqNLUZw_bQZE2foBts0c70z57bHcMVg1hVZ_E4easg@mail.gmail.com>


Hi,

On Tuesday, December 06, 2016 11:03:34 AM Olof Johansson wrote:
> On Tue, Dec 6, 2016 at 4:38 AM, Bartlomiej Zolnierkiewicz
> <b.zolnierkie@samsung.com> wrote:
> > Hi,
> >
> > This RFC patchset starts convertion of ARM defconfigs to use kconfig
> > fragments and dynamically generate defconfigs.  The goals of this
> > work are to:
> 
> You don't provide any motivation as to why this is better. As far as I

Benefits are:

- no code duplication (this initial patchset alone removes ~1700 lines
  from defconfigs without any change in functionality)

- prevention of "multi" defconfigs (i.e. multi_v7_defconfig) going out
  of sync with "SoC-family" ones (i.e. exynos_defconfig) - there will
  be just one place to update when changing things

- possibility to add support for more optimized defconfigs (i.e. board
  specific ones) in the future without duplicating the code

- making it easier to define arch specific parts of defconfigs in
  the future if we decide on doing it (i.e. we may want to enable
  things like CONFIG_SYSVIPC for all defconfigs)

> am concerned it'll just be a mess.
> 
> So:
> 
> Nack. So much nack. I really don't want to see a proliferation of
> config fragments like this.
> 
> I had a feeling it was a bad idea to pick up that one line config
> fragment before, since it opened the door for this kind of mess. :(

Like I said in the cover-letter I'm not satisfied with the current
patches and they have much room for improvement.

However I see that you don't like the idea itself... :(

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

^ permalink raw reply

* [PATCH 0/7] arm: Add livepatch support
From: Abel Vesa @ 2016-12-07 11:39 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <584767FF.6030807@huawei.com>

On Wed, Dec 07, 2016 at 09:38:07AM +0800, zhouchengming wrote:
> On 2016/12/7 1:06, 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.
> >
> >Considering that the function is livepatchable, when the time comes to call
> >ftrace_call, the ftrace_regs_caller is called instead.
> >
> >Because this arch didn't have a ftrace with regs implementation, the
> >ftrace_regs_caller was added.
> >
> >This new function adds the regs saving/restoring part, plus the part necessary
> >for the livepatch mechanism to work. After the regs are saved and the r3 is set
> >to contain the sp's value, we're keeping the old pc into r10 in order to be
> >checked later against the new pc.
> >
> >Next, the r1 and r0 are set for the ftrace_func, then, the ftrace_stub is called
> >and the klp_ftrace_handler overwrites the old pc with the new one.
> >
> >Here comes the tricky part. We're checking if the pc is still the old one, if it
> >is we jump the whole livepatching and go ahead with restoring the saved regs.
> >
> >If the pc is modified, it means we're livepatching current function and we need
> >to pop all regs from r1 through r12, jump over the next two regs saved on stack
> >(we're not interested in those since we're trying to get the same regs context
> >as it was at the point the function-to-be-patched was called) and put the new pc
> >into r11.
> >
> >Since r12 contains the sp from when the function just got branched to, we need
> >to set the sp back to that.
> >
> >Then we need to put the new pc on stack so that when we're popping r11 through
> >pc, we will actually jump to the first instruction from the new function.
> >
> >We don't need to worry about the returning phase since the epilogue of the new
> >function will take care of that and from there on everything goes back to
> >normal.
> >
> >The whole advantage of this over adding compiler support is that we're not
> >introducing nops at the beginning of the function. As a matter of fact, we're
> >not changing anything between an image with livepatch and an image without it
> >(except the ftrace_regs_call addition and the livepatch necessary code).
> >
> >As for the implementation of the ftrace_regs_caller, I still think there might
> >be some unsafe stack handling since I'm getting some build warnings. Those are
> >due to pushing/popping of a list of regs in which the sp resides. I'll try to
> >get around those in a next iteration (if necessary), but first I would like to
> >hear some opinions about this work and if it's worth going forward.
> >
> 
> Hi, so your idea is that when the pc is modified, we undo the work of the prologue
> of the old function, and then jump to the first instruction of the new function.
> But I doubt if we can really undo the work of the prologue correctly ? I don't know
> about arm, but gcc on arm64 may do some tricky things in prologue. So is there any
> chance we may restore a wrong context for the new function ?
> 
> Thanks.
>
I forgot to mention that this is actually taking advantage of how this arch deals 
with function calling. This mechanism might not be appliable to any other arch 
AFAIK. On arm 32bit, as long as the mcount prologue looks like in the shown 
example, the function is livepatchable. I will come back with a new version of
this patch today (latest tomorrow) with comments as Russel and Steven mentioned.
I hope that will clarify why this is working on arm 32bit.
> >Everything else should be pretty straightforward, so I'll skip explaining that.
> >
> >Abel Vesa (7):
> >   arm: Add livepatch arch specific code
> >   arm: ftrace: Add call modify mechanism
> >   arm: module: Add apply_relocate_add
> >   arm: Add ftrace with regs support
> >   arm: ftrace: Add ARCH_SUPPORTS_FTRACE_OPS for ftrace with regs
> >   arm: Add livepatch to build if CONFIG_LIVEPATCH
> >   arm: Add livepatch necessary arch selects into Kconfig
> >
> >  MAINTAINERS                      |  3 +++
> >  arch/arm/Kconfig                 |  4 ++++
> >  arch/arm/include/asm/ftrace.h    |  4 ++++
> >  arch/arm/include/asm/livepatch.h | 46 +++++++++++++++++++++++++++++++++++++
> >  arch/arm/kernel/Makefile         |  1 +
> >  arch/arm/kernel/entry-ftrace.S   | 49 ++++++++++++++++++++++++++++++++++++++++
> >  arch/arm/kernel/ftrace.c         | 21 +++++++++++++++++
> >  arch/arm/kernel/livepatch.c      | 43 +++++++++++++++++++++++++++++++++++
> >  arch/arm/kernel/module.c         |  9 ++++++++
> >  9 files changed, 180 insertions(+)
> >  create mode 100644 arch/arm/include/asm/livepatch.h
> >  create mode 100644 arch/arm/kernel/livepatch.c
> >
> 
> 

^ permalink raw reply

* [RFC PATCH 0/2] arm64: memory-hotplug: Add Memory Hotplug support
From: Xishi Qiu @ 2016-12-07 11:21 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <0f24c35b-10d5-d857-356d-01a21be48c2c@broadcom.com>

On 2016/12/7 16:43, Scott Branden wrote:

> Hi Xishi,
> 
> I followed you suggestions and found pfn_valid is always true.  Answers to your questions inline.
> 
> I could keep debugging this but hope Marcin sends out some code - I'm quite willing to test and help clean up the patchset.
> 
> On 16-12-01 07:11 PM, Xishi Qiu wrote:
>> On 2016/12/2 10:38, Scott Branden wrote:
>>
>>> Hi Xishi,
>>>
>>> Thanks for the reply - please see comments below.
>>>
>>> On 16-12-01 05:49 PM, Xishi Qiu wrote:
>>>> On 2016/12/2 8:19, Scott Branden wrote:
>>>>
>>>>> This patchset is sent for comment to add memory hotplug support for ARM64
>>>>> based platforms.  It follows hotplug code added for other architectures
>>>>> in the linux kernel.
>>>>>
>>>>> I tried testing the memory hotplug feature following documentation from
>>>>> Documentation/memory-hotplug.txt.  I don't think it is working as expected
>>>>> - see below:
>>>>>
>>>>> To add memory to the system I did the following:
>>>>> echo 0x400000000 > /sys/devices/system/memory/probe
>>>>>
>>>>> The memory is displayed as system ram:
>>>>> cat /proc/iomem:
>>>>> 74000000-77ffffff : System RAM
>>>>>   74080000-748dffff : Kernel code
>>>>>   74950000-749d2fff : Kernel data
>>>>> 400000000-43fffffff : System RAM
>>>>>
>>>>> But does not seem to be added to the kernel memory.
>>>>> /proc/meminfo did not change.
>>>>>
>>>>> What else needs to be done so the memory is added to the kernel memory
>>>>> pool for normal allocation?
>>>>>
>>>>
>>>> Hi Scott,
>>>>
>>>> Do you mean it still don't support hod-add after apply this patchset?
>>>
>>> After applying the patch it appears to partially support hot-add. Please let me know if you think it is working as expected?
>>>
>>> The memory probe functions in that the memory is registered with the system and shows up in /proc/iomem.  But, the memory is not available in /proc/meminfo.  Do you think something else needs to be adjusted for ARM64 to hotadd the memory
>>>
>>> I just found another clue:
>>> under /sys/devices/system/memory I only see one memory entry (before or after I try to hotadd additional memory).
>>>
>>> /sys/devices/system/memory # ls
>>> auto_online_blocks  memory0            uevent
>>> block_size_bytes    probe
>>>
>>> In arch/arm64/include/asm/sparsemem.h if I change SECTION_SIZE_BITS from 30 to 28 and recompile I get the following:
>>> /sys/devices/system/memory # ls
>>> auto_online_blocks  memory7            uevent
>>> block_size_bytes    probe
>>>
>>>
>>> In arch/arm64/include/asm/sparsemem.h if I change SECTION_SIZE_BITS from 30 to 27 and recompile I get the following:
>>> /sys/devices/system/memory # ls
>>> auto_online_blocks  memory14            uevent
>>> block_size_bytes    probe
>>>
>>> If looks to me like something is not working properly in the ARM64 implementation.  I should expect to see multiple memoryX entries under /sys/devices/system/memory?
>>>
>>
>> Hi Scott,
>>
>> 1. Do you enable the following configs?
>> CONFIG_SPARSEMEM
>> MEMORY_HOTPLUG
>> CONFIG_MEMORY_HOTPLUG_DEFAULT_ONLINE
> Yes, these configs are enabled
>>
>> 2. I find you missed create mapping in arch_add_memory(), and x86 has it.
> Could you please explain this further?  The patch I submitted hass arch_add_memory identical to the ia64 implementation.

Hi Scott,

I think we should create page table first for the new hotadd memory.
e.g. create_mapping_late(start, __phys_to_virt(start), size, PAGE_KERNEL);

I don't know why ia64 don't have this step.
CC Tony

>>
>> 3. We will add memblock first, so pfn_valid() maybe always return true(in the
>> following function), and this will lead __add_section() failed. Please check
>> it.
> You are correct - pfn_valid always returns true.  The function is in arch/arm64/mm/init.c and different than the one you indicated below:
> 

> #ifdef CONFIG_HAVE_ARCH_PFN_VALID
> int pfn_valid(unsigned long pfn)
> {
>     return memblock_is_map_memory(pfn << PAGE_SHIFT);
> }
> EXPORT_SYMBOL(pfn_valid);
> #endif
> 
>>
>> int pfn_valid(unsigned long pfn)
>> {
>>     return (pfn & PFN_MASK) == pfn && memblock_is_memory(pfn << PAGE_SHIFT);
>> }
>>
>> add_memory
>>   add_memory_resource
>>     memblock_add_node
>>       arch_add_memory
>>         __add_pages
>>           __add_section
>>             pfn_valid
>>
>> Thanks,
>> Xishi Qiu
>>
>>>
>>>
>>>>
>>>> Thanks,
>>>> Xishi Qiu
>>>>
>>>>> Scott Branden (2):
>>>>>   arm64: memory-hotplug: Add MEMORY_HOTPLUG, MEMORY_HOTREMOVE,
>>>>>     MEMORY_PROBE
>>>>>   arm64: defconfig: enable MEMORY_HOTPLUG config options
>>>>>
>>>>>  arch/arm64/Kconfig           | 10 ++++++++++
>>>>>  arch/arm64/configs/defconfig |  3 +++
>>>>>  arch/arm64/mm/init.c         | 42 ++++++++++++++++++++++++++++++++++++++++++
>>>>>  3 files changed, 55 insertions(+)
>>>>>
>>>>
>>>>
>>>>
>>>
>>> .
>>>
>>
>>
>>
> 
> .
> 

^ permalink raw reply

* [PATCH 4/5] arm64: dts: exynos5433: Add bus dt node using VDD_INT for Exynos5433
From: Chanwoo Choi @ 2016-12-07 11:17 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161206192104.GB12683@kozik-lap>

On 2016? 12? 07? 04:21, Krzysztof Kozlowski wrote:
> On Fri, Dec 02, 2016 at 04:18:06PM +0900, Chanwoo Choi wrote:
>> This patch adds the bus nodes using VDD_INT for Exynos5433 SoC.
>> Exynos5433 has the following AMBA AXI buses to translate data
>> between DRAM and sub-blocks.
>>
>> Following list specify the detailed correlation between sub-block and clock:
>> - CLK_ACLK_G2D_{400|266}  : Bus clock for G2D
>> - CLK_ACLK_MSCL_400       : Bus clock for MSCL (Mobile Scaler)
>> - CLK_ACLK_GSCL_333       : Bus clock for GSCL (General Scaler)
>> - CLK_SCLK_JPEG_MSCL      : Bus clock for JPEG
>> - CLK_ACLK_MFC_400        : Bus clock for MFC (Multi Format Codec)
>> - CLK_ACLK_HEVC_400       : Bus clock for HEVC (High Effective Video Codec)
>> - CLK_ACLK_BUS0_400       : NoC(Network On Chip)'s bus clock for PERIC/PERIS/FSYS/MSCL
>> - CLK_ACLK_BUS1_400       : NoC's bus clock for MFC/HEVC/G3D
>> - CLK_ACLK_BUS2_400       : NoC's bus clock for GSCL/DISP/G2D/CAM0/CAM1/ISP
>>
>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
>> ---
>>  arch/arm64/boot/dts/exynos/exynos5433-bus.dtsi | 208 +++++++++++++++++++++++++
>>  arch/arm64/boot/dts/exynos/exynos5433.dtsi     |   1 +
>>  2 files changed, 209 insertions(+)
>>  create mode 100644 arch/arm64/boot/dts/exynos/exynos5433-bus.dtsi
>>
>> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-bus.dtsi b/arch/arm64/boot/dts/exynos/exynos5433-bus.dtsi
>> new file mode 100644
>> index 000000000000..b1e1d9c622e1
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/exynos/exynos5433-bus.dtsi
>> @@ -0,0 +1,208 @@
>> +/*
>> + * Samsung's Exynos5433 SoC Memory interface and AMBA bus device tree source
>> + *
>> + * Copyright (c) 2016 Samsung Electronics Co., Ltd.
>> + * Chanwoo Choi <cw00.choi@samsung.com>
>> + *
>> + * Samsung's Exynos5433 SoC Memory interface and AMBA buses are listed
>> + * as device tree nodes are listed in this file.
> 
> This duplicates the introduction line and does not make sense.

I'll remove it.

> 
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +/ {
> 
> Shouldn't these be under soc node? It looks like property of SoC itself.

OK. Move to them under SoC.
- "/ {" -> "&soc {"

> 
>> +	/* INT (Internal) block using VDD_INT */
>> +	bus_g2d_400: bus_g2d_400 {
> 
> In node name, the dash '-' is preferred. The name should describe
> general class of device so probably this should be just "bus"... but I
> don't see a way how to do it reasonable anyway.

I'll change them as following with 'busX'.

The each dt node has the unique number('X')
because each dt node does not have the base address
and then need to identify oneself.

	bus_g2d_400: bus0 {
	bus_g2d_266: bus1 {
	bus_gscl: bus2 {
	bus_hevc: bus3 {
	bus_jpeg: bus4 {
	bus_mfc: bus5 {
	bus_mscl: bus6 {
	bus_noc0: bus7 {
	bus_noc1: bus8 {
	bus_noc2: bus9 {

> 
>> +		compatible = "samsung,exynos-bus";
>> +		clocks = <&cmu_top CLK_ACLK_G2D_400>;
>> +		clock-names = "bus";
>> +		operating-points-v2 = <&bus_g2d_400_opp_table>;
>> +		status ="disable";
> 
> Hm?

I'll fix it. disable -> disabled

> 
> 
>> +	};
>> +
>> +	bus_mscl: bus_mscl {
>> +		compatible = "samsung,exynos-bus";
>> +		clocks = <&cmu_top CLK_ACLK_MSCL_400>;
>> +		clock-names = "bus";
>> +		operating-points-v2 = <&bus_g2d_400_opp_table>;
>> +		status ="disable";
>> +	};
>> +
>> +	bus_jpeg: bus_jpeg {
>> +		compatible = "samsung,exynos-bus";
>> +		clocks = <&cmu_top CLK_SCLK_JPEG_MSCL>;
>> +		clock-names = "bus";
>> +		operating-points-v2 = <&bus_g2d_400_opp_table>;
>> +		status ="disable";
>> +	};
>> +
>> +	bus_mfc: bus_mfc {
>> +		compatible = "samsung,exynos-bus";
>> +		clocks = <&cmu_top CLK_ACLK_MFC_400>;
>> +
>> +		clock-names = "bus";
>> +		operating-points-v2 = <&bus_g2d_400_opp_table>;
>> +		status ="disable";
>> +	};
>> +
>> +	bus_g2d_266: bus_g2d_266 {
>> +		compatible = "samsung,exynos-bus";
>> +		clocks = <&cmu_top CLK_ACLK_G2D_266>;
>> +		clock-names = "bus";
>> +		operating-points-v2 = <&bus_g2d_266_opp_table>;
>> +		status ="disable";
>> +	};
>> +
>> +	bus_gscl: bus_gscl {
>> +		compatible = "samsung,exynos-bus";
>> +		clocks = <&cmu_top CLK_ACLK_GSCL_333>;
>> +		clock-names = "bus";
>> +		operating-points-v2 = <&bus_gscl_opp_table>;
>> +		status ="disable";
>> +	};
>> +
>> +	bus_hevc: bus_hevc {
>> +		compatible = "samsung,exynos-bus";
>> +		clocks = <&cmu_top CLK_ACLK_HEVC_400>;
>> +		clock-names = "bus";
>> +		operating-points-v2 = <&bus_hevc_opp_table>;
>> +		status ="disable";
>> +	};
>> +
>> +	bus_bus0: bus_bus0 {
> 
> bus, bus, bus, bus, jackpot! Let's try to find better name and label for
> these. :)

I'll change the name with 'noc' prefix because this bus is used
for NoC (Network On Chip)'s bus clock as commit msg.
- old : bus_bus0
- new : bus_noc0


Best Regards,
Chanwoo Choi

^ permalink raw reply

* [PATCH v11 7/7] arm: pmu: Add PMU definitions for cores not initially online
From: Mark Rutland @ 2016-12-07 11:08 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <457894f4-67f8-9d32-ee8b-5a34e9a21488@arm.com>

On Tue, Dec 06, 2016 at 11:56:56AM -0600, Jeremy Linton wrote:
> Hi,

> Which might not be the right choice because
> even without these ACPI specific bits, simply running a few cpus
> online/offline while simultaneously doing something like `perf stat
> -e cache-misses ls &` in a loop causes deadlocks/crashes.
> 
> That problem doesn't appear to be specific to the ACPI/PMU so I've
> stayed away from it in this patch set, although potentially a larger
> fix might cover this as well.

Urrgh; I can reproduce lockups on Seattle with v4.9-rc8.

I'll look into that.

If you see any more issues in this area, please report them in a new
thread.

Thanks,
Mark.

^ permalink raw reply

* [Question] New mmap64 syscall?
From: Dr. Philipp Tomsich @ 2016-12-07 11:07 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161207103451.GA869@yury-N73SV>

[Resend, as my mail-client had insisted on using the wrong MIME type?]

> On 07 Dec 2016, at 11:34, Yury Norov <ynorov@caviumnetworks.com> wrote:
> 
>> If there is a use case for larger than 16TB offsets, we should add
>> the call on all architectures, probably using your approach 3. I don't
>> think that we should treat it as anything special for arm64 though.
> 
> From this point of view, 16+TB offset is a matter of 16+TB storage,
> and it's more than real. The other consideration to add it is that
> we have 64-bit support for offsets in syscalls like sys_llseek().
> So mmap64() will simply extend this support.

I believe the question is rather if the 16TB offset is a real use-case for ILP32.  This
seems to bring the discussion full-circle, as this would indicate that 64bit is the 
preferred bit-width for all sizes, offsets, etc. throughout all filesystem-related calls 
(i.e. stat, seek, etc.).

But if that is the case, then we should have gone with 64bit arguments in a single
register for our ILP32 definition on AArch64.

In other words: Why not keep ILP32 simple an ask users that need a 16TB+ offset
to use LP64? It seems much more consistent with the other choices takes so far.

?Phil.

^ permalink raw reply

* [PATCH] arm/arm64: KVM: Check for properly initialized timer on init
From: Marc Zyngier @ 2016-12-07 11:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161206195652.GF4816@cbox>

On 06/12/16 19:56, Christoffer Dall wrote:
> On Tue, Dec 06, 2016 at 11:25:42AM +0000, Marc Zyngier wrote:
>> On 05/12/16 09:32, Christoffer Dall wrote:
>>> When the arch timer code fails to initialize (for example because the
>>> memory mapped timer doesn't work, which is currently seen with the AEM
>>> model), then KVM just continues happily with a final result that KVM
>>> eventually does a NULL pointer dereference of the uninitialized cycle
>>> counter.
>>>
>>> Check directly for this in the init path and give the user a reasonable
>>> error in this case.
>>>
>>> Cc: Shih-Wei Li <shihwei@cs.columbia.edu>
>>> Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
>>> ---
>>>  virt/kvm/arm/arch_timer.c | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
>>> index 27a1f63..5c12f53 100644
>>> --- a/virt/kvm/arm/arch_timer.c
>>> +++ b/virt/kvm/arm/arch_timer.c
>>> @@ -425,6 +425,11 @@ int kvm_timer_hyp_init(void)
>>>  	info = arch_timer_get_kvm_info();
>>>  	timecounter = &info->timecounter;
>>>  
>>> +	if (!timecounter->cc) {
>>> +		kvm_err("arch_timer: uninitialized timecounter\n");
>>
>> For consistency, I'll change the error message to say "kvm_arch_timer",
>> just like the below case.
>>
> 
> No objections, only problem is that the patch you queued uses
> kcm_arch_timer ;)

Yeah, that's the new and upgraded version: Kernel Cryogenic Machine, it
freezes time ;-).

I'll fix that shortly, thanks for the heads up!

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

^ permalink raw reply

* [PATCH v2] PCI: designware: add host_init error handling
From: Joao Pinto @ 2016-12-07 11:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1481106769-1404-1-git-send-email-srinivas.kandagatla@linaro.org>


Hi Srinivas!

Thanks for the update!

Acked-By: Joao Pinto <jpinto@synopsys.com>

?s 10:32 AM de 12/7/2016, Srinivas Kandagatla escreveu:
> This patch add support to return value from host_init() callback from drivers,
> so that the designware libary can handle or pass it to proper place. Issue with
> void return type is that errors or error handling within host_init() callback
> are never know to designware code, which could go ahead and access registers
> even in error cases.
> 
> Typical case in qcom controller driver is to turn off clks in case of errors,
> if designware code continues to read/write register when clocks are turned off
> the board would reboot/lockup.
> 
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
> Currently designware code does not have a way return errors generated
> as part of host_init() callback in controller drivers. This is an issue
> with controller drivers like qcom which turns off the clocks in error
> handling path. As the dw core is un aware of this would continue to
> access registers which faults resulting in board reboots/hangs.
> 
> There are two ways to solve this issue,
> one is remove error handling in the qcom controller host_init() function
> other is to handle error and pass back to dw core code which would then
> pass back to controller driver as part of dw_pcie_host_init() return value.
> 
> Second option seems more sensible and correct way to fix the issue,
> this patch does the same.
> 
> As part of this change to host_init() return type I had to patch other
> ihost controller drivers which use dw core. Most of the changes to other drivers
> are to return proper error codes to upper layer.
> Only compile tested drivers.
> 
> Changes since RFC:
> 	- Add error handling to other drivers as suggested by Joao Pinto
> 
>  drivers/pci/host/pci-dra7xx.c           | 10 ++++++++--
>  drivers/pci/host/pci-exynos.c           | 10 ++++++++--
>  drivers/pci/host/pci-imx6.c             | 10 ++++++++--
>  drivers/pci/host/pci-keystone.c         | 10 ++++++++--
>  drivers/pci/host/pci-layerscape.c       | 22 +++++++++++++---------
>  drivers/pci/host/pcie-armada8k.c        |  4 +++-
>  drivers/pci/host/pcie-designware-plat.c | 10 ++++++++--
>  drivers/pci/host/pcie-designware.c      |  4 +++-
>  drivers/pci/host/pcie-designware.h      |  2 +-
>  drivers/pci/host/pcie-qcom.c            |  5 +++--
>  drivers/pci/host/pcie-spear13xx.c       | 10 ++++++++--
>  11 files changed, 71 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/pci/host/pci-dra7xx.c b/drivers/pci/host/pci-dra7xx.c
> index 9595fad..811f0f9 100644
> --- a/drivers/pci/host/pci-dra7xx.c
> +++ b/drivers/pci/host/pci-dra7xx.c
> @@ -127,9 +127,10 @@ static void dra7xx_pcie_enable_interrupts(struct dra7xx_pcie *dra7xx)
>  				   LEG_EP_INTERRUPTS);
>  }
>  
> -static void dra7xx_pcie_host_init(struct pcie_port *pp)
> +static int dra7xx_pcie_host_init(struct pcie_port *pp)
>  {
>  	struct dra7xx_pcie *dra7xx = to_dra7xx_pcie(pp);
> +	int ret;
>  
>  	pp->io_base &= DRA7XX_CPU_TO_BUS_ADDR;
>  	pp->mem_base &= DRA7XX_CPU_TO_BUS_ADDR;
> @@ -138,10 +139,15 @@ static void dra7xx_pcie_host_init(struct pcie_port *pp)
>  
>  	dw_pcie_setup_rc(pp);
>  
> -	dra7xx_pcie_establish_link(dra7xx);
> +	ret = dra7xx_pcie_establish_link(dra7xx);
> +	if (ret < 0)
> +		return ret;
> +
>  	if (IS_ENABLED(CONFIG_PCI_MSI))
>  		dw_pcie_msi_init(pp);
>  	dra7xx_pcie_enable_interrupts(dra7xx);
> +
> +	return 0;
>  }
>  
>  static struct pcie_host_ops dra7xx_pcie_host_ops = {
> diff --git a/drivers/pci/host/pci-exynos.c b/drivers/pci/host/pci-exynos.c
> index f1c544b..c116fd9 100644
> --- a/drivers/pci/host/pci-exynos.c
> +++ b/drivers/pci/host/pci-exynos.c
> @@ -458,12 +458,18 @@ static int exynos_pcie_link_up(struct pcie_port *pp)
>  	return 0;
>  }
>  
> -static void exynos_pcie_host_init(struct pcie_port *pp)
> +static int exynos_pcie_host_init(struct pcie_port *pp)
>  {
>  	struct exynos_pcie *exynos_pcie = to_exynos_pcie(pp);
> +	int ret;
> +
> +	ret = exynos_pcie_establish_link(exynos_pcie);
> +	if (ret < 0)
> +		return ret;
>  
> -	exynos_pcie_establish_link(exynos_pcie);
>  	exynos_pcie_enable_interrupts(exynos_pcie);
> +
> +	return 0;
>  }
>  
>  static struct pcie_host_ops exynos_pcie_host_ops = {
> diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
> index c8cefb0..1251e92 100644
> --- a/drivers/pci/host/pci-imx6.c
> +++ b/drivers/pci/host/pci-imx6.c
> @@ -550,18 +550,24 @@ static int imx6_pcie_establish_link(struct imx6_pcie *imx6_pcie)
>  	return ret;
>  }
>  
> -static void imx6_pcie_host_init(struct pcie_port *pp)
> +static int imx6_pcie_host_init(struct pcie_port *pp)
>  {
>  	struct imx6_pcie *imx6_pcie = to_imx6_pcie(pp);
> +	int ret;
>  
>  	imx6_pcie_assert_core_reset(imx6_pcie);
>  	imx6_pcie_init_phy(imx6_pcie);
>  	imx6_pcie_deassert_core_reset(imx6_pcie);
>  	dw_pcie_setup_rc(pp);
> -	imx6_pcie_establish_link(imx6_pcie);
> +	ret = imx6_pcie_establish_link(imx6_pcie);
> +
> +	if (ret < 0)
> +		return ret;
>  
>  	if (IS_ENABLED(CONFIG_PCI_MSI))
>  		dw_pcie_msi_init(pp);
> +
> +	return 0;
>  }
>  
>  static int imx6_pcie_link_up(struct pcie_port *pp)
> diff --git a/drivers/pci/host/pci-keystone.c b/drivers/pci/host/pci-keystone.c
> index 043c19a..4067a75 100644
> --- a/drivers/pci/host/pci-keystone.c
> +++ b/drivers/pci/host/pci-keystone.c
> @@ -260,12 +260,16 @@ static int keystone_pcie_fault(unsigned long addr, unsigned int fsr,
>  	return 0;
>  }
>  
> -static void __init ks_pcie_host_init(struct pcie_port *pp)
> +static int __init ks_pcie_host_init(struct pcie_port *pp)
>  {
>  	struct keystone_pcie *ks_pcie = to_keystone_pcie(pp);
>  	u32 val;
> +	int ret;
> +
> +	ret = ks_pcie_establish_link(ks_pcie);
> +	if (ret < 0)
> +		return ret;
>  
> -	ks_pcie_establish_link(ks_pcie);
>  	ks_dw_pcie_setup_rc_app_regs(ks_pcie);
>  	ks_pcie_setup_interrupts(ks_pcie);
>  	writew(PCI_IO_RANGE_TYPE_32 | (PCI_IO_RANGE_TYPE_32 << 8),
> @@ -287,6 +291,8 @@ static void __init ks_pcie_host_init(struct pcie_port *pp)
>  	 */
>  	hook_fault_code(17, keystone_pcie_fault, SIGBUS, 0,
>  			"Asynchronous external abort");
> +
> +	return 0;
>  }
>  
>  static struct pcie_host_ops keystone_pcie_host_ops = {
> diff --git a/drivers/pci/host/pci-layerscape.c b/drivers/pci/host/pci-layerscape.c
> index 6537079..60c8b84 100644
> --- a/drivers/pci/host/pci-layerscape.c
> +++ b/drivers/pci/host/pci-layerscape.c
> @@ -103,30 +103,32 @@ static int ls1021_pcie_link_up(struct pcie_port *pp)
>  	return 1;
>  }
>  
> -static void ls1021_pcie_host_init(struct pcie_port *pp)
> +static int ls1021_pcie_host_init(struct pcie_port *pp)
>  {
>  	struct device *dev = pp->dev;
>  	struct ls_pcie *pcie = to_ls_pcie(pp);
>  	u32 index[2];
> +	int ret;
>  
>  	pcie->scfg = syscon_regmap_lookup_by_phandle(dev->of_node,
>  						     "fsl,pcie-scfg");
>  	if (IS_ERR(pcie->scfg)) {
>  		dev_err(dev, "No syscfg phandle specified\n");
> -		pcie->scfg = NULL;
> -		return;
> +		return PTR_ERR(pcie->scfg);
>  	}
>  
> -	if (of_property_read_u32_array(dev->of_node,
> -				       "fsl,pcie-scfg", index, 2)) {
> -		pcie->scfg = NULL;
> -		return;
> -	}
> +	ret = of_property_read_u32_array(dev->of_node,
> +				       "fsl,pcie-scfg", index, 2);
> +	if (ret < 0)
> +		return ret;
> +
>  	pcie->index = index[1];
>  
>  	dw_pcie_setup_rc(pp);
>  
>  	ls_pcie_drop_msg_tlp(pcie);
> +
> +	return 0;
>  }
>  
>  static int ls_pcie_link_up(struct pcie_port *pp)
> @@ -144,7 +146,7 @@ static int ls_pcie_link_up(struct pcie_port *pp)
>  	return 1;
>  }
>  
> -static void ls_pcie_host_init(struct pcie_port *pp)
> +static int ls_pcie_host_init(struct pcie_port *pp)
>  {
>  	struct ls_pcie *pcie = to_ls_pcie(pp);
>  
> @@ -153,6 +155,8 @@ static void ls_pcie_host_init(struct pcie_port *pp)
>  	ls_pcie_clear_multifunction(pcie);
>  	ls_pcie_drop_msg_tlp(pcie);
>  	iowrite32(0, pcie->pp.dbi_base + PCIE_DBI_RO_WR_EN);
> +
> +	return 0;
>  }
>  
>  static int ls_pcie_msi_host_init(struct pcie_port *pp,
> diff --git a/drivers/pci/host/pcie-armada8k.c b/drivers/pci/host/pcie-armada8k.c
> index 0ac0f18..29bdd8b 100644
> --- a/drivers/pci/host/pcie-armada8k.c
> +++ b/drivers/pci/host/pcie-armada8k.c
> @@ -134,12 +134,14 @@ static void armada8k_pcie_establish_link(struct armada8k_pcie *pcie)
>  		dev_err(pp->dev, "Link not up after reconfiguration\n");
>  }
>  
> -static void armada8k_pcie_host_init(struct pcie_port *pp)
> +static int armada8k_pcie_host_init(struct pcie_port *pp)
>  {
>  	struct armada8k_pcie *pcie = to_armada8k_pcie(pp);
>  
>  	dw_pcie_setup_rc(pp);
>  	armada8k_pcie_establish_link(pcie);
> +
> +	return 0;
>  }
>  
>  static irqreturn_t armada8k_pcie_irq_handler(int irq, void *arg)
> diff --git a/drivers/pci/host/pcie-designware-plat.c b/drivers/pci/host/pcie-designware-plat.c
> index 1a02038..e01adbb 100644
> --- a/drivers/pci/host/pcie-designware-plat.c
> +++ b/drivers/pci/host/pcie-designware-plat.c
> @@ -35,13 +35,19 @@ static irqreturn_t dw_plat_pcie_msi_irq_handler(int irq, void *arg)
>  	return dw_handle_msi_irq(pp);
>  }
>  
> -static void dw_plat_pcie_host_init(struct pcie_port *pp)
> +static int dw_plat_pcie_host_init(struct pcie_port *pp)
>  {
> +	int ret;
> +
>  	dw_pcie_setup_rc(pp);
> -	dw_pcie_wait_for_link(pp);
> +	ret = dw_pcie_wait_for_link(pp);
> +	if (ret)
> +		return ret;
>  
>  	if (IS_ENABLED(CONFIG_PCI_MSI))
>  		dw_pcie_msi_init(pp);
> +
> +	return 0;
>  }
>  
>  static struct pcie_host_ops dw_plat_pcie_host_ops = {
> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
> index bed1999..4a81b72 100644
> --- a/drivers/pci/host/pcie-designware.c
> +++ b/drivers/pci/host/pcie-designware.c
> @@ -638,7 +638,9 @@ int dw_pcie_host_init(struct pcie_port *pp)
>  	}
>  
>  	if (pp->ops->host_init)
> -		pp->ops->host_init(pp);
> +		ret = pp->ops->host_init(pp);
> +			if (ret < 0)
> +				goto error;
>  
>  	pp->root_bus_nr = pp->busn->start;
>  	if (IS_ENABLED(CONFIG_PCI_MSI)) {
> diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
> index a567ea2..eacf18f 100644
> --- a/drivers/pci/host/pcie-designware.h
> +++ b/drivers/pci/host/pcie-designware.h
> @@ -63,7 +63,7 @@ struct pcie_host_ops {
>  	int (*wr_other_conf)(struct pcie_port *pp, struct pci_bus *bus,
>  			unsigned int devfn, int where, int size, u32 val);
>  	int (*link_up)(struct pcie_port *pp);
> -	void (*host_init)(struct pcie_port *pp);
> +	int (*host_init)(struct pcie_port *pp);
>  	void (*msi_set_irq)(struct pcie_port *pp, int irq);
>  	void (*msi_clear_irq)(struct pcie_port *pp, int irq);
>  	phys_addr_t (*get_msi_addr)(struct pcie_port *pp);
> diff --git a/drivers/pci/host/pcie-qcom.c b/drivers/pci/host/pcie-qcom.c
> index 3593640..7d5fb38 100644
> --- a/drivers/pci/host/pcie-qcom.c
> +++ b/drivers/pci/host/pcie-qcom.c
> @@ -429,7 +429,7 @@ static int qcom_pcie_link_up(struct pcie_port *pp)
>  	return !!(val & PCI_EXP_LNKSTA_DLLLA);
>  }
>  
> -static void qcom_pcie_host_init(struct pcie_port *pp)
> +static int qcom_pcie_host_init(struct pcie_port *pp)
>  {
>  	struct qcom_pcie *pcie = to_qcom_pcie(pp);
>  	int ret;
> @@ -455,12 +455,13 @@ static void qcom_pcie_host_init(struct pcie_port *pp)
>  	if (ret)
>  		goto err;
>  
> -	return;
> +	return ret;
>  err:
>  	qcom_ep_reset_assert(pcie);
>  	phy_power_off(pcie->phy);
>  err_deinit:
>  	pcie->ops->deinit(pcie);
> +	return ret;
>  }
>  
>  static int qcom_pcie_rd_own_conf(struct pcie_port *pp, int where, int size,
> diff --git a/drivers/pci/host/pcie-spear13xx.c b/drivers/pci/host/pcie-spear13xx.c
> index 3cf197b..2408f80 100644
> --- a/drivers/pci/host/pcie-spear13xx.c
> +++ b/drivers/pci/host/pcie-spear13xx.c
> @@ -174,12 +174,18 @@ static int spear13xx_pcie_link_up(struct pcie_port *pp)
>  	return 0;
>  }
>  
> -static void spear13xx_pcie_host_init(struct pcie_port *pp)
> +static int spear13xx_pcie_host_init(struct pcie_port *pp)
>  {
>  	struct spear13xx_pcie *spear13xx_pcie = to_spear13xx_pcie(pp);
> +	int ret;
> +
> +	ret = spear13xx_pcie_establish_link(spear13xx_pcie);
> +	if (ret < 0)
> +		return ret;
>  
> -	spear13xx_pcie_establish_link(spear13xx_pcie);
>  	spear13xx_pcie_enable_interrupts(spear13xx_pcie);
> +
> +	return 0;
>  }
>  
>  static struct pcie_host_ops spear13xx_pcie_host_ops = {
> 

^ permalink raw reply

* [PATCH v4 6/7] IIO: add STM32 timer trigger driver
From: Benjamin Gaignard @ 2016-12-07 11:00 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161207105002.GA3625@dell.home>

2016-12-07 11:50 GMT+01:00 Lee Jones <lee.jones@linaro.org>:
> On Tue, 06 Dec 2016, Benjamin Gaignard wrote:
>
>> [snip]
>> >> +
>> >> +static const char * const triggers0[] = {
>> >> +     TIM1_TRGO, TIM1_CH1, TIM1_CH2, TIM1_CH3, TIM1_CH4, NULL,
>> >> +};
>> >> +
>> >> +static const char * const triggers1[] = {
>> >> +     TIM2_TRGO, TIM2_CH1, TIM2_CH2, TIM2_CH3, TIM2_CH4, NULL,
>> >> +};
>> >> +
>> >> +static const char * const triggers2[] = {
>> >> +     TIM3_TRGO, TIM3_CH1, TIM3_CH2, TIM3_CH3, TIM3_CH4, NULL,
>> >> +};
>> >> +
>> >> +static const char * const triggers3[] = {
>> >> +     TIM4_TRGO, TIM4_CH1, TIM4_CH2, TIM4_CH3, TIM4_CH4, NULL,
>> >> +};
>> >> +
>> >> +static const char * const triggers4[] = {
>> >> +     TIM5_TRGO, TIM5_CH1, TIM5_CH2, TIM5_CH3, TIM5_CH4, NULL,
>> >> +};
>> >> +
>> >> +static const char * const triggers5[] = {
>> >> +     TIM6_TRGO, NULL,
>> >> +};
>> >> +
>> >> +static const char * const triggers6[] = {
>> >> +     TIM7_TRGO, NULL,
>> >> +};
>> >> +
>> >> +static const char * const triggers7[] = {
>> >> +     TIM8_TRGO, TIM8_CH1, TIM8_CH2, TIM8_CH3, TIM8_CH4, NULL,
>> >> +};
>> >> +
>> >> +static const char * const triggers8[] = {
>> >> +     TIM9_TRGO, TIM9_CH1, TIM9_CH2, NULL,
>> >> +};
>> >> +
>> >> +static const char * const triggers9[] = {
>> >> +     TIM12_TRGO, TIM12_CH1, TIM12_CH2, NULL,
>> >> +};
>> >> +
>> >> +static const void *triggers_table[] = {
>> >> +     triggers0,
>> >> +     triggers1,
>> >> +     triggers2,
>> >> +     triggers3,
>> >> +     triggers4,
>> >> +     triggers5,
>> >> +     triggers6,
>> >> +     triggers7,
>> >> +     triggers8,
>> >> +     triggers9,
>> >> +};
>> >
>> > What about:
>> >
>> > static const char * const triggers[][] = {
>> >         { TIM1_TRGO, TIM1_CH1, TIM1_CH2, TIM1_CH3, TIM1_CH4, NULL },
>> >         { TIM2_TRGO, TIM2_CH1, TIM2_CH2, TIM2_CH3, TIM2_CH4, NULL },
>> >         { TIM3_TRGO, TIM3_CH1, TIM3_CH2, TIM3_CH3, TIM3_CH4, NULL },
>> >         { TIM4_TRGO, TIM4_CH1, TIM4_CH2, TIM4_CH3, TIM4_CH4, NULL },
>> >         { TIM5_TRGO, TIM5_CH1, TIM5_CH2, TIM5_CH3, TIM5_CH4, NULL },
>> >         { TIM6_TRGO, NULL },
>> >         { TIM7_TRGO, NULL },
>> >         { TIM8_TRGO, TIM8_CH1, TIM8_CH2, TIM8_CH3, TIM8_CH4, NULL },
>> >         { TIM9_TRGO, TIM9_CH1, TIM9_CH2, NULL },
>> >         { TIM12_TRGO, TIM12_CH1, TIM12_CH2, NULL }
>> > };
>>
>> I can't because the second dimension of the array isn't fix.
>> I could have between 2 and 6 elements per row... to create a dual dimension
>> array I would have to add NULL entries like that:
>>
>> #define MAX_TRIGGERS 6
>>
>> static const void *triggers_table[][MAX_TRIGGERS] = {
>> { TIM1_TRGO, TIM1_CH1, TIM1_CH2, TIM1_CH3, TIM1_CH4, NULL,},
>> { TIM2_TRGO, TIM2_CH1, TIM2_CH2, TIM2_CH3, TIM2_CH4, NULL,},
>> { TIM3_TRGO, TIM3_CH1, TIM3_CH2, TIM3_CH3, TIM3_CH4, NULL,},
>> { TIM4_TRGO, TIM4_CH1, TIM4_CH2, TIM4_CH3, TIM4_CH4, NULL,},
>> { TIM5_TRGO, TIM5_CH1, TIM5_CH2, TIM5_CH3, TIM5_CH4, NULL,},
>> { TIM6_TRGO, NULL,     NULL,     NULL,     NULL,     NULL,},
>> { TIM7_TRGO, NULL,     NULL,     NULL,     NULL,     NULL,},
>> { TIM8_TRGO, TIM8_CH1, TIM8_CH2, TIM8_CH3, TIM8_CH4, NULL,},
>> { TIM9_TRGO, TIM9_CH1, TIM9_CH2, NULL,     NULL,     NULL,},
>> { TIM12_TRGO, TIM12_CH1, TIM12_CH2, NULL,  NULL,     NULL,},
>> };
>
> It was just an idea, not a tested implementation.
>
> I don't understand why you have to pad with NULLs, but either way, it
> looks much better than before and saves lots of lines of code.

I have tested it this morning and it works fine so I will include it in v5.
I use NULL as limit when iterate in the table and for table padding too.

>
>> >> +static const char * const valids0[] = {
>> >> +     TIM5_TRGO, TIM2_TRGO, TIM4_TRGO, TIM3_TRGO, NULL,
>> >> +};
>> >> +
>> >> +static const char * const valids1[] = {
>> >> +     TIM1_TRGO, TIM8_TRGO, TIM3_TRGO, TIM4_TRGO, NULL,
>> >> +};
>> >> +
>> >> +static const char * const valids2[] = {
>> >> +     TIM1_TRGO, TIM8_TRGO, TIM5_TRGO, TIM4_TRGO, NULL,
>> >> +};
>> >> +
>> >> +static const char * const valids3[] = {
>> >> +     TIM1_TRGO, TIM2_TRGO, TIM3_TRGO, TIM8_TRGO, NULL,
>> >> +};
>> >> +
>> >> +static const char *const valids4[] = {
>> >> +     TIM2_TRGO, TIM3_TRGO, TIM4_TRGO, TIM8_TRGO, NULL,
>> >> +};
>> >> +
>> >> +static const char * const valids7[] = {
>> >> +     TIM1_TRGO, TIM2_TRGO, TIM4_TRGO, TIM5_TRGO, NULL,
>> >> +};
>> >> +
>> >> +static const char * const valids8[] = {
>> >> +     TIM2_TRGO, TIM3_TRGO, NULL,
>> >> +};
>> >> +
>> >> +static const char * const valids9[] = {
>> >> +     TIM4_TRGO, TIM5_TRGO, NULL,
>> >> +};
>> >> +
>> >> +static const void *valids_table[] = {
>> >> +     valids0,
>> >> +     valids1,
>> >> +     valids2,
>> >> +     valids3,
>> >> +     valids4,
>> >> +     NULL,
>> >> +     NULL,
>> >> +     valids7,
>> >> +     valids8,
>> >> +     valids9,
>> >> +};
>> >
>> > Same here.
>> >
>
> --
> Lee Jones
> Linaro STMicroelectronics Landing Team Lead
> Linaro.org ? Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* [PATCH 4/7] arm: Add ftrace with regs support
From: Russell King - ARM Linux @ 2016-12-07 10:57 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1481043967-15602-5-git-send-email-abelvesa@linux.com>

On Tue, Dec 06, 2016 at 05:06:04PM +0000, Abel Vesa wrote:
> This adds __ftrace_regs_caller which, unlike __ftrace_caller,
> adds register saving/restoring and livepatch handling if
> the pc register gets modified by klp_ftrace_handler.
> 
> Signed-off-by: Abel Vesa <abelvesa@linux.com>
> ---
>  arch/arm/kernel/entry-ftrace.S | 49 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 49 insertions(+)
> 
> diff --git a/arch/arm/kernel/entry-ftrace.S b/arch/arm/kernel/entry-ftrace.S
> index c73c403..b6ada5c 100644
> --- a/arch/arm/kernel/entry-ftrace.S
> +++ b/arch/arm/kernel/entry-ftrace.S
> @@ -92,6 +92,46 @@
>  2:	mcount_exit
>  .endm
>  
> +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
> +
> +.macro __ftrace_regs_caller suffix
> +
> +	stmdb	sp!, {r0-r15}
> +	mov	r3, sp
> +
> +	ldr	r10, [sp, #60]
> +
> +	mcount_get_lr   r1                      @ lr of instrumented func
> +	mcount_adjust_addr	r0, lr		@ instrumented function
> +
> +	.globl ftrace_regs_call\suffix
> +ftrace_regs_call\suffix:
> +	bl	ftrace_stub
> +
> +#ifdef CONFIG_FUNCTION_GRAPH_TRACER
> +	.globl ftrace_regs_graph_call\suffix
> +ftrace_regs_graph_call\suffix:
> +	mov	r0, r0
> +#endif
> +#ifdef CONFIG_LIVEPATCH
> +	ldr	r0, [sp, #60]
> +	cmp	r0, r10
> +	beq	ftrace_regs_caller_end
> +	ldmia	sp!, {r0-r12}
> +	add	sp, sp, #8
> +	ldmia	sp!, {r11}
> +	sub	sp, r12, #16
> +	str	r11, [sp, #12]
> +	ldmia	sp!, {r11, r12, lr, pc}

Some comments about the above stack manipulation (in the code) would be
useful - remember, the contents of your cover letter is lost when the
patches are applied.

However, I'm not convinced yet that this doesn't unbalance the stack,
unless the livepatching code pushes some extra registers onto it,
particularly with the following unstacking code after the livepatch
code.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply

* [PATCH v4 6/7] IIO: add STM32 timer trigger driver
From: Lee Jones @ 2016-12-07 10:50 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CA+M3ks6fxCucmaY0OgpMb_opA=YLrowijJ0YQ+imeprhWg6N_w@mail.gmail.com>

On Tue, 06 Dec 2016, Benjamin Gaignard wrote:

> [snip]
> >> +
> >> +static const char * const triggers0[] = {
> >> +     TIM1_TRGO, TIM1_CH1, TIM1_CH2, TIM1_CH3, TIM1_CH4, NULL,
> >> +};
> >> +
> >> +static const char * const triggers1[] = {
> >> +     TIM2_TRGO, TIM2_CH1, TIM2_CH2, TIM2_CH3, TIM2_CH4, NULL,
> >> +};
> >> +
> >> +static const char * const triggers2[] = {
> >> +     TIM3_TRGO, TIM3_CH1, TIM3_CH2, TIM3_CH3, TIM3_CH4, NULL,
> >> +};
> >> +
> >> +static const char * const triggers3[] = {
> >> +     TIM4_TRGO, TIM4_CH1, TIM4_CH2, TIM4_CH3, TIM4_CH4, NULL,
> >> +};
> >> +
> >> +static const char * const triggers4[] = {
> >> +     TIM5_TRGO, TIM5_CH1, TIM5_CH2, TIM5_CH3, TIM5_CH4, NULL,
> >> +};
> >> +
> >> +static const char * const triggers5[] = {
> >> +     TIM6_TRGO, NULL,
> >> +};
> >> +
> >> +static const char * const triggers6[] = {
> >> +     TIM7_TRGO, NULL,
> >> +};
> >> +
> >> +static const char * const triggers7[] = {
> >> +     TIM8_TRGO, TIM8_CH1, TIM8_CH2, TIM8_CH3, TIM8_CH4, NULL,
> >> +};
> >> +
> >> +static const char * const triggers8[] = {
> >> +     TIM9_TRGO, TIM9_CH1, TIM9_CH2, NULL,
> >> +};
> >> +
> >> +static const char * const triggers9[] = {
> >> +     TIM12_TRGO, TIM12_CH1, TIM12_CH2, NULL,
> >> +};
> >> +
> >> +static const void *triggers_table[] = {
> >> +     triggers0,
> >> +     triggers1,
> >> +     triggers2,
> >> +     triggers3,
> >> +     triggers4,
> >> +     triggers5,
> >> +     triggers6,
> >> +     triggers7,
> >> +     triggers8,
> >> +     triggers9,
> >> +};
> >
> > What about:
> >
> > static const char * const triggers[][] = {
> >         { TIM1_TRGO, TIM1_CH1, TIM1_CH2, TIM1_CH3, TIM1_CH4, NULL },
> >         { TIM2_TRGO, TIM2_CH1, TIM2_CH2, TIM2_CH3, TIM2_CH4, NULL },
> >         { TIM3_TRGO, TIM3_CH1, TIM3_CH2, TIM3_CH3, TIM3_CH4, NULL },
> >         { TIM4_TRGO, TIM4_CH1, TIM4_CH2, TIM4_CH3, TIM4_CH4, NULL },
> >         { TIM5_TRGO, TIM5_CH1, TIM5_CH2, TIM5_CH3, TIM5_CH4, NULL },
> >         { TIM6_TRGO, NULL },
> >         { TIM7_TRGO, NULL },
> >         { TIM8_TRGO, TIM8_CH1, TIM8_CH2, TIM8_CH3, TIM8_CH4, NULL },
> >         { TIM9_TRGO, TIM9_CH1, TIM9_CH2, NULL },
> >         { TIM12_TRGO, TIM12_CH1, TIM12_CH2, NULL }
> > };
> 
> I can't because the second dimension of the array isn't fix.
> I could have between 2 and 6 elements per row... to create a dual dimension
> array I would have to add NULL entries like that:
> 
> #define MAX_TRIGGERS 6
> 
> static const void *triggers_table[][MAX_TRIGGERS] = {
> { TIM1_TRGO, TIM1_CH1, TIM1_CH2, TIM1_CH3, TIM1_CH4, NULL,},
> { TIM2_TRGO, TIM2_CH1, TIM2_CH2, TIM2_CH3, TIM2_CH4, NULL,},
> { TIM3_TRGO, TIM3_CH1, TIM3_CH2, TIM3_CH3, TIM3_CH4, NULL,},
> { TIM4_TRGO, TIM4_CH1, TIM4_CH2, TIM4_CH3, TIM4_CH4, NULL,},
> { TIM5_TRGO, TIM5_CH1, TIM5_CH2, TIM5_CH3, TIM5_CH4, NULL,},
> { TIM6_TRGO, NULL,     NULL,     NULL,     NULL,     NULL,},
> { TIM7_TRGO, NULL,     NULL,     NULL,     NULL,     NULL,},
> { TIM8_TRGO, TIM8_CH1, TIM8_CH2, TIM8_CH3, TIM8_CH4, NULL,},
> { TIM9_TRGO, TIM9_CH1, TIM9_CH2, NULL,     NULL,     NULL,},
> { TIM12_TRGO, TIM12_CH1, TIM12_CH2, NULL,  NULL,     NULL,},
> };

It was just an idea, not a tested implementation.

I don't understand why you have to pad with NULLs, but either way, it
looks much better than before and saves lots of lines of code.

> >> +static const char * const valids0[] = {
> >> +     TIM5_TRGO, TIM2_TRGO, TIM4_TRGO, TIM3_TRGO, NULL,
> >> +};
> >> +
> >> +static const char * const valids1[] = {
> >> +     TIM1_TRGO, TIM8_TRGO, TIM3_TRGO, TIM4_TRGO, NULL,
> >> +};
> >> +
> >> +static const char * const valids2[] = {
> >> +     TIM1_TRGO, TIM8_TRGO, TIM5_TRGO, TIM4_TRGO, NULL,
> >> +};
> >> +
> >> +static const char * const valids3[] = {
> >> +     TIM1_TRGO, TIM2_TRGO, TIM3_TRGO, TIM8_TRGO, NULL,
> >> +};
> >> +
> >> +static const char *const valids4[] = {
> >> +     TIM2_TRGO, TIM3_TRGO, TIM4_TRGO, TIM8_TRGO, NULL,
> >> +};
> >> +
> >> +static const char * const valids7[] = {
> >> +     TIM1_TRGO, TIM2_TRGO, TIM4_TRGO, TIM5_TRGO, NULL,
> >> +};
> >> +
> >> +static const char * const valids8[] = {
> >> +     TIM2_TRGO, TIM3_TRGO, NULL,
> >> +};
> >> +
> >> +static const char * const valids9[] = {
> >> +     TIM4_TRGO, TIM5_TRGO, NULL,
> >> +};
> >> +
> >> +static const void *valids_table[] = {
> >> +     valids0,
> >> +     valids1,
> >> +     valids2,
> >> +     valids3,
> >> +     valids4,
> >> +     NULL,
> >> +     NULL,
> >> +     valids7,
> >> +     valids8,
> >> +     valids9,
> >> +};
> >
> > Same here.
> >

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* [PATCH v5 2/4] drm: bridge: add support for TI ths8135
From: Bartosz Golaszewski @ 2016-12-07 10:45 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1481107365-24839-3-git-send-email-bgolaszewski@baylibre.com>

2016-12-07 11:42 GMT+01:00 Bartosz Golaszewski <bgolaszewski@baylibre.com>:
> THS8135 is a configurable video DAC. Add DT bindings for this chip and
> use the dumb-vga-dac driver for now as no configuration is required to
> make it work.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
>  .../bindings/display/bridge/ti,ths8135.txt         | 52 ++++++++++++++++++++++
>  drivers/gpu/drm/bridge/dumb-vga-dac.c              |  1 +
>  2 files changed, 53 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,ths8135.txt
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/ti,ths8135.txt b/Documentation/devicetree/bindings/display/bridge/ti,ths8135.txt
> new file mode 100644
> index 0000000..23cd8ee
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/ti,ths8135.txt
> @@ -0,0 +1,52 @@
> +THS8135 Video DAC
> +-----------------
> +
> +This is the binding for Texas Instruments THS8135 Video DAC bridge.
> +
> +Required properties:
> +
> +- compatible: Must be "ti,ths8135"
> +
> +Required nodes:
> +
> +This device has two video ports. Their connections are modelled using the OF
> +graph bindings specified in Documentation/devicetree/bindings/graph.txt.
> +
> +- Video port 0 for RGB input
> +- Video port 1 for VGA output
> +
> +Example
> +-------
> +
> +vga-bridge {
> +       compatible = "ti,ths8135";
> +       #address-cells = <1>;
> +       #size-cells = <0>;
> +
> +       ports {
> +               #address-cells = <1>;
> +               #size-cells = <0>;
> +
> +               port at 0 {
> +                       #address-cells = <1>;
> +                       #size-cells = <0>;
> +                       reg = <0>;
> +
> +                       vga_bridge_in: endpoint at 0 {
> +                               reg = <0>;
> +                               remote-endpoint = <&lcdc_out_vga>;
> +                       };
> +               };
> +
> +               port at 1 {
> +                       #address-cells = <1>;
> +                       #size-cells = <0>;
> +                       reg = <1>;
> +
> +                       vga_bridge_out: endpoint at 0 {
> +                               reg = <0>;
> +                               remote-endpoint = <&vga_con_in>;
> +                       };
> +               };
> +       };
> +};
> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> index afec232..498fa75 100644
> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> @@ -204,6 +204,7 @@ static int dumb_vga_remove(struct platform_device *pdev)
>
>  static const struct of_device_id dumb_vga_match[] = {
>         { .compatible = "dumb-vga-dac" },
> +       { .compatible = "ti,ths8135" },
>         {},
>  };
>  MODULE_DEVICE_TABLE(of, dumb_vga_match);
> --
> 2.9.3
>

+ Maxime

Sorry, I forgot to include your e-mail.

Bartosz

^ permalink raw reply


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