* [PATCH] drivers: remoteproc: constify rproc_ops structures
From: Bhumika Goyal @ 2016-12-17 11:29 UTC (permalink / raw)
To: linux-arm-kernel
Declare rproc_ops structures as const as they are only passed as an
argument to the function rproc_alloc. This argument is of type const, so
rproc_ops structures having this property can be declared const too.
Done using Coccinelle:
@r1 disable optional_qualifier @
identifier i;
position p;
@@
static struct rproc_ops i at p = {...};
@ok1@
identifier r1.i;
position p;
@@
rproc_alloc(...,&i at p,...)
@bad@
position p!={r1.p,ok1.p};
identifier r1.i;
@@
i at p
@depends on !bad disable optional_qualifier@
identifier r1.i;
@@
+const
struct rproc_ops i;
File sizes before:
text data bss dec hex filename
1258 416 0 1674 68a remoteproc/omap_remoteproc.o
2402 240 0 2642 a52 remoteproc/st_remoteproc.o
2064 272 0 2336 920 remoteproc/st_slim_rproc.o
2160 240 0 2400 960 remoteproc/wkup_m3_rproc.o
File sizes after:
text data bss dec hex filename
1297 368 0 1665 681 remoteproc/omap_remoteproc.o
2434 192 0 2626 a42 remoteproc/st_remoteproc.o
2112 240 0 2352 930 remoteproc/st_slim_rproc.o
2200 192 0 2392 958 remoteproc/wkup_m3_rproc.o
Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
---
drivers/remoteproc/omap_remoteproc.c | 2 +-
drivers/remoteproc/st_remoteproc.c | 2 +-
drivers/remoteproc/st_slim_rproc.c | 2 +-
drivers/remoteproc/wkup_m3_rproc.c | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/remoteproc/omap_remoteproc.c b/drivers/remoteproc/omap_remoteproc.c
index fa63bf2..a96ce90 100644
--- a/drivers/remoteproc/omap_remoteproc.c
+++ b/drivers/remoteproc/omap_remoteproc.c
@@ -177,7 +177,7 @@ static int omap_rproc_stop(struct rproc *rproc)
return 0;
}
-static struct rproc_ops omap_rproc_ops = {
+static const struct rproc_ops omap_rproc_ops = {
.start = omap_rproc_start,
.stop = omap_rproc_stop,
.kick = omap_rproc_kick,
diff --git a/drivers/remoteproc/st_remoteproc.c b/drivers/remoteproc/st_remoteproc.c
index da4e152..f21787b 100644
--- a/drivers/remoteproc/st_remoteproc.c
+++ b/drivers/remoteproc/st_remoteproc.c
@@ -107,7 +107,7 @@ static int st_rproc_stop(struct rproc *rproc)
return sw_err ?: pwr_err;
}
-static struct rproc_ops st_rproc_ops = {
+static const struct rproc_ops st_rproc_ops = {
.start = st_rproc_start,
.stop = st_rproc_stop,
};
diff --git a/drivers/remoteproc/st_slim_rproc.c b/drivers/remoteproc/st_slim_rproc.c
index 507716c..6cfd862 100644
--- a/drivers/remoteproc/st_slim_rproc.c
+++ b/drivers/remoteproc/st_slim_rproc.c
@@ -200,7 +200,7 @@ static void *slim_rproc_da_to_va(struct rproc *rproc, u64 da, int len)
return va;
}
-static struct rproc_ops slim_rproc_ops = {
+static const struct rproc_ops slim_rproc_ops = {
.start = slim_rproc_start,
.stop = slim_rproc_stop,
.da_to_va = slim_rproc_da_to_va,
diff --git a/drivers/remoteproc/wkup_m3_rproc.c b/drivers/remoteproc/wkup_m3_rproc.c
index 18175d0..1ada0e5 100644
--- a/drivers/remoteproc/wkup_m3_rproc.c
+++ b/drivers/remoteproc/wkup_m3_rproc.c
@@ -111,7 +111,7 @@ static void *wkup_m3_rproc_da_to_va(struct rproc *rproc, u64 da, int len)
return va;
}
-static struct rproc_ops wkup_m3_rproc_ops = {
+static const struct rproc_ops wkup_m3_rproc_ops = {
.start = wkup_m3_rproc_start,
.stop = wkup_m3_rproc_stop,
.da_to_va = wkup_m3_rproc_da_to_va,
--
1.9.1
^ permalink raw reply related
* [PATCH] ARM: dts: Add missing CPU frequencies for Exynos5422/5800
From: Anand Moon @ 2016-12-17 7:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1b6e8d3a-ec7a-db5d-dd0e-ef9d1480f80a@fivetechno.de>
Hi Markus,
On 16 December 2016 at 14:38, Markus Reichl <m.reichl@fivetechno.de> wrote:
> Am 16.12.2016 um 08:37 schrieb Krzysztof Kozlowski:
>> On Thu, Dec 15, 2016 at 04:52:58PM -0800, Doug Anderson wrote:
>>>> [ I added Arjun to Cc:, maybe he can help in explaining this issue
>>>> (unfortunately Inderpal's email is no longer working). ]
>>>>
>>>> Please also note that on Exynos5422/5800 SoCs the same ARM rail
>>>> voltage is used for 1.9 GHz & 2.0 GHz OPPs as for the 1.8 GHz one.
>>>> IOW if the problem exists it is already present in the mainline
>>>> kernel.
>>>
>>> Interesting. In the ChromeOS tree I see significantly higher voltages
>>> needed... Note that one might naively look at
>>> <https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.8/drivers/cpufreq/exynos5420-cpufreq.c#178>.
>>>
>>> 1362500, /* L0 2100 */
>>> 1312500, /* L1 2000 */
>>>
>>> ..but, amazingly enough those voltages aren't used at all. Surprise!
>>>
>>> I believe that the above numbers are actually not used and the ASV
>>> numbers are used instead. See
>>> <https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.8/arch/arm/mach-exynos/include/mach/asv-exynos542x.h#452>
>>>
>>> { 2100000,
>>> 1350000, 1350000, 1350000, 1350000, 1350000,
>>> 1337500, 1325000, 1312500, 1300000, 1287500,
>>> 1275000, 1262500, 1250000, 1237500 },
>>>
>>> I believe that interpretation there is: some bins of the CPU can run
>>> at 2.1 GHz just fine at 1.25 V but others need up to 1.35V.
>>
>> That is definitely the case. One could just look at vendors ASV table
>> (for 1.9 GHz):
>> { 1900000, 1300000, 1287500, 1262500, 1237500, 1225000, 1212500,
>> 1200000, 1187500, 1175000, 1162500, 1150000,
>> 1137500, 1125000, 1112500, 1112500},
>>
>> The theoretical difference is up to 1.875V! From my experiments I saw
>> BIN1 chips which should be the same... but some working on 1.2V, some on
>> 1.225V (@1.9 GHz). I didn't see any requiring higher voltages but that
>> does not mean that there aren't such...
>>
>>> ...so if you're running at 2.1 GHz at 1.25V then perhaps you're just
>>> running on a CPU from a nice bin?
>
> I've been running the proposed frequency/voltage combinations without any
> stability problems on my XU4, XU3 and even XU3-lite ( I did not delete the
> nodes on XU3-lite dts) with make -j8 kernel and ssvb-cpuburn.
> The chips are poorly cooled, especially the XU4 and quickly step down.
[snip]
As per my knowlegde Odroid XU3/4 can throttle at much high temperature.
https://github.com/hardkernel/linux/blob/odroidxu3-3.10.y/drivers/thermal/exynos_thermal.c#L1629
The device tree binding for thermal-zone is kept bit low alert
temperature values
in-order to avoid reaches critical temperature and board shutdown
when compiling the source code. We need t fix this thermal-zone
Their could be some race in thermal or the step wise governor for
exynos is not working correctly.
Better option is to print the cpufreq for cpu0 and cpu4 and respective temp
and plot a graph along timeline. It will give us clear idea on how much
time is spend on high frequency on stress testing.
#!/bin/bash
t=0
while true :
do
a=`cat /sys/devices/virtual/thermal/thermal_zone0/temp`
b=`cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_cur_freq`
c=`cat /sys/devices/system/cpu/cpu0/cpufreq/cpuinfo_cur_freq`
d=`cat /sys/devices/system/cpu/cpu4/cpufreq/scaling_cur_freq`
e=`cat /sys/devices/system/cpu/cpu4/cpufreq/cpuinfo_cur_freq`
(( t += 5 ))
echo $t,$a,$b,$d,$e
sleep 1
done
Best Regards
-Anand Moon
^ permalink raw reply
* [PATCH] i2c: mux: pca954x: fix i2c mux selection caching
From: Peter Rosin @ 2016-12-17 6:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161216232355.GO14217@n2100.armlinux.org.uk>
On 2016-12-17 00:23, Russell King - ARM Linux wrote:
> On Fri, Dec 16, 2016 at 10:20:35PM +0100, Peter Rosin wrote:
>> On 2016-12-16 21:06, Russell King wrote:
>>> smbus functions return -ve on error, 0 on success. However,
>>> __i2c_transfer() have a different return signature - -ve on error, or
>>> number of buffers transferred (which may be zero or greater.)
>>>
>>> The upshot of this is that the sense of the test is reversed when using
>>> the mux on a bus supporting the master_xfer method: we cache the value
>>> and never retry if we fail to transfer any buffers, but if we succeed,
>>> we clear the cached value.
>>
>> Ouch! Thanks for catching this.
>>
>>> Fix this.
>>
>> But lets fix the corner case of __i2c_transfer returning 0 instead of
>> the expected 1 as well (not sure if that's even possible, but lets close
>> the possibility just in case), so I'd prefer if you could fix
>> pca954x_reg_write() to return 0 iff __i2c_transfer(...) returns 1
>> instead, and -EREMOTEIO on other non-negative return values. Thanks!
>
> So you want something like this instead?
Yes, but I was originally thinking that the second hunk was no
longer needed...
Either way,
Acked-by: Peter Rosin <peda@axentia.se>
Cheers,
peda
> diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
> index 8bc3d36d2837..9c4ac26c014e 100644
> --- a/drivers/i2c/muxes/i2c-mux-pca954x.c
> +++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
> @@ -151,6 +151,9 @@ static int pca954x_reg_write(struct i2c_adapter *adap,
> buf[0] = val;
> msg.buf = buf;
> ret = __i2c_transfer(adap, &msg, 1);
> +
> + if (ret >= 0 && ret != 1)
> + ret = -EREMOTEIO;
> } else {
> union i2c_smbus_data data;
> ret = adap->algo->smbus_xfer(adap, client->addr,
> @@ -179,7 +182,7 @@ static int pca954x_select_chan(struct i2c_mux_core *muxc, u32 chan)
> /* Only select the channel if its different from the last channel */
> if (data->last_chan != regval) {
> ret = pca954x_reg_write(muxc->parent, client, regval);
> - data->last_chan = ret ? 0 : regval;
> + data->last_chan = ret < 0 ? 0 : regval;
> }
>
> return ret;
>
>
^ permalink raw reply
* [PATCH v8 2/8] ARM: EXYNOS: enable exynos_chipid for ARCH_EXYNOS
From: Pankaj Dubey @ 2016-12-17 4:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161216180401.GA6459@kozik-lap>
Hi Krzysztof,
On 16 December 2016 at 23:34, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> On Sat, Dec 10, 2016 at 06:38:37PM +0530, Pankaj Dubey wrote:
>> As now we have chipid driver to initialize SoC related information
>> lets include it in build by default.
> s/lets/let's/
>
> Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
>
Thanks for review. Will fix minor nitpick in v9 and include your
Reviewed-by tag.
Pankaj Dubey
> Best regards,
> Krzysztof
>
>>
>> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
>> ---
>> arch/arm/mach-exynos/Kconfig | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig
>> index 0bb63b8..29065c5 100644
>> --- a/arch/arm/mach-exynos/Kconfig
>> +++ b/arch/arm/mach-exynos/Kconfig
>> @@ -16,6 +16,7 @@ menuconfig ARCH_EXYNOS
>> select ARM_AMBA
>> select ARM_GIC
>> select COMMON_CLK_SAMSUNG
>> + select EXYNOS_CHIPID
>> select EXYNOS_THERMAL
>> select EXYNOS_PMU
>> select EXYNOS_SROM
>> --
>> 2.7.4
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" 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 v8 1/8] soc: samsung: add exynos chipid driver support
From: Pankaj Dubey @ 2016-12-17 4:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161216173703.GA3746@kozik-lap>
Hi Krzysztof,
On 16 December 2016 at 23:07, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> On Sat, Dec 10, 2016 at 06:38:36PM +0530, Pankaj Dubey wrote:
>> Exynos SoCs have Chipid, for identification of product IDs and SoC revisions.
>> This patch intends to provide initialization code for all these functionalities,
>> at the same time it provides some sysfs entries for accessing these information
>> to user-space.
>>
>> This driver uses existing binding for exynos-chipid.
>>
>> CC: Grant Likely <grant.likely@linaro.org>
>> CC: Rob Herring <robh+dt@kernel.org>
>> CC: Linus Walleij <linus.walleij@linaro.org>
>> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
>> [m.szyprowski: for suggestion and code snippet of product_id_to_soc_id]
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>> drivers/soc/samsung/Kconfig | 5 ++
>> drivers/soc/samsung/Makefile | 1 +
>> drivers/soc/samsung/exynos-chipid.c | 116 ++++++++++++++++++++++++++++++++++++
>> 3 files changed, 122 insertions(+)
>> create mode 100644 drivers/soc/samsung/exynos-chipid.c
>>
>> diff --git a/drivers/soc/samsung/Kconfig b/drivers/soc/samsung/Kconfig
>> index 2455339..f9ab858 100644
>> --- a/drivers/soc/samsung/Kconfig
>> +++ b/drivers/soc/samsung/Kconfig
>> @@ -14,4 +14,9 @@ config EXYNOS_PM_DOMAINS
>> bool "Exynos PM domains" if COMPILE_TEST
>> depends on PM_GENERIC_DOMAINS || COMPILE_TEST
>>
>> +config EXYNOS_CHIPID
>> + bool "Exynos Chipid controller driver" if COMPILE_TEST
>> + depends on (ARM && ARCH_EXYNOS) || ((ARM || ARM64) && COMPILE_TEST)
>
> 1. Why this can be compile tested only on ARM architectures?
Well I just used dependency same as EXYNOS_PMU, but I can see it will
be enabled for compile test on ARM64 isn't it?
> 2. Don't you need also SOC_BUS?
CHIPID needs SoC_BUS and for the same reason it is selecting SOC_BUS
in the next line.
If we mark it as a dependency (under depends on), even then we need to
select this either
under same EXYNOS_CHIPID config or ARCH_EXYNOS config.
>
>> + select SOC_BUS
>> +
>> endif
>> diff --git a/drivers/soc/samsung/Makefile b/drivers/soc/samsung/Makefile
>> index 3619f2e..2a8a85e 100644
>> --- a/drivers/soc/samsung/Makefile
>> +++ b/drivers/soc/samsung/Makefile
>> @@ -1,3 +1,4 @@
>> obj-$(CONFIG_EXYNOS_PMU) += exynos-pmu.o exynos3250-pmu.o exynos4-pmu.o \
>> exynos5250-pmu.o exynos5420-pmu.o
>> obj-$(CONFIG_EXYNOS_PM_DOMAINS) += pm_domains.o
>> +obj-$(CONFIG_EXYNOS_CHIPID) += exynos-chipid.o
>
> Please put it before EXYNOS_PMU, keeping this sorted alphabetical helps
> avoiding conflicts of continuous edits.
>
>> diff --git a/drivers/soc/samsung/exynos-chipid.c b/drivers/soc/samsung/exynos-chipid.c
>> new file mode 100644
>> index 0000000..cf0128b
>> --- /dev/null
>> +++ b/drivers/soc/samsung/exynos-chipid.c
>> @@ -0,0 +1,116 @@
>> +/*
>> + * Copyright (c) 2016 Samsung Electronics Co., Ltd.
>> + * http://www.samsung.com/
>> + *
>> + * EXYNOS - CHIP ID support
>> + * Author: Pankaj Dubey <pankaj.dubey@samsung.com>
>> + *
>> + * 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.
>> + */
>> +
>> +#include <linux/io.h>
>> +#include <linux/of.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/slab.h>
>> +#include <linux/sys_soc.h>
>> +
>> +#define EXYNOS_SUBREV_MASK (0xF << 4)
>> +#define EXYNOS_MAINREV_MASK (0xF << 0)
>> +#define EXYNOS_REV_MASK (EXYNOS_SUBREV_MASK | EXYNOS_MAINREV_MASK)
>> +
>> +static const struct exynos_soc_id {
>> + const char *name;
>> + unsigned int id;
>> + unsigned int mask;
>> +} soc_ids[] = {
>> + { "EXYNOS3250", 0xE3472000, 0xFFFFF000 },
>> + { "EXYNOS4210", 0x43200000, 0xFFFE0000 },
>> + { "EXYNOS4212", 0x43220000, 0xFFFE0000 },
>> + { "EXYNOS4412", 0xE4412000, 0xFFFE0000 },
>> + { "EXYNOS5250", 0x43520000, 0xFFFFF000 },
>> + { "EXYNOS5260", 0xE5260000, 0xFFFFF000 },
>> + { "EXYNOS5410", 0xE5410000, 0xFFFFF000 },
>> + { "EXYNOS5420", 0xE5420000, 0xFFFFF000 },
>> + { "EXYNOS5440", 0xE5440000, 0xFFFFF000 },
>> + { "EXYNOS5800", 0xE5422000, 0xFFFFF000 },
>> + { "EXYNOS7420", 0xE7420000, 0xFFFFF000 },
>> + { "EXYNOS5433", 0xE5433000, 0xFFFFF000 },
>> +};
>> +
>> +static const char * __init product_id_to_soc_id(unsigned int product_id)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < ARRAY_SIZE(soc_ids); i++)
>> + if ((product_id & soc_ids[i].mask) == soc_ids[i].id)
>> + return soc_ids[i].name;
>> + return "UNKNOWN";
>> +}
>> +
>> +static const struct of_device_id of_exynos_chipid_ids[] = {
>> + {
>> + .compatible = "samsung,exynos4210-chipid",
>> + },
>> + {},
>> +};
>> +
>> +/**
>> + * exynos_chipid_early_init: Early chipid initialization
>> + */
>
> This comment is meaningless, it duplicates the name of function.
Ok, will remove this in v9.
>
>> +int __init exynos_chipid_early_init(void)
>> +{
>> + struct soc_device_attribute *soc_dev_attr;
>> + struct soc_device *soc_dev;
>> + struct device_node *root;
>> + struct device_node *np;
>> + void __iomem *exynos_chipid_base;
>> + const struct of_device_id *match;
>> + u32 product_id;
>> + u32 revision;
>> +
>> + np = of_find_matching_node_and_match(NULL,
>> + of_exynos_chipid_ids, &match);
>
> You don't use the match here, so how about either
> of_find_matching_node() or of_find_compatible_node()? The latter looks
> better (less calls inside) and actually you want just check one
> compatible field?
>
Ok, will adopt this in v9.
>> + if (!np)
>> + return -ENODEV;
>> +
>> + exynos_chipid_base = of_iomap(np, 0);
>> +
>> + if (!exynos_chipid_base)
>> + return PTR_ERR(exynos_chipid_base);
>> +
>> + product_id = readl_relaxed(exynos_chipid_base);
>
> Duplicated space before '='.
Ok, will fix this.
>
>> + revision = product_id & EXYNOS_REV_MASK;
>> + iounmap(exynos_chipid_base);
>> +
>> + soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
>> + if (!soc_dev_attr)
>> + return -ENODEV;
>> +
>> + soc_dev_attr->family = "Samsung Exynos";
>> +
>> + root = of_find_node_by_path("/");
>> + of_property_read_string(root, "model", &soc_dev_attr->machine);
>> + of_node_put(root);
>> +
>> + soc_dev_attr->revision = kasprintf(GFP_KERNEL, "%x", revision);
>> + soc_dev_attr->soc_id = product_id_to_soc_id(product_id);
>> +
>> +
>> + pr_info("Exynos: CPU[%s] CPU_REV[0x%x] Detected\n",
>> + product_id_to_soc_id(product_id), revision);
>> +
>> + soc_dev = soc_device_register(soc_dev_attr);
>> + if (IS_ERR(soc_dev)) {
>> + kfree(soc_dev_attr->revision);
>> + kfree_const(soc_dev_attr->soc_id);
>
> It wasn't allocated with *_const, so no need to free it.
Yes, will fix this.
>
> Best regards,
> Krzysztof
Thanks for review.
Pankaj Dubey
^ permalink raw reply
* [PATCH v8 4/8] ARM: EXYNOS: refactor firmware specific routines
From: Pankaj Dubey @ 2016-12-17 3:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161216182531.GA6440@kozik-lap>
Hi Krzysztof,
On 16 December 2016 at 23:55, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> On Sat, Dec 10, 2016 at 06:38:39PM +0530, Pankaj Dubey wrote:
>> To remove dependency on soc_is_exynosMMMM macros and remove multiple
>> checks for such macros lets refactor code in firmware.c file.
>> SoC specific firmware_ops are separated and registered during
>> exynos_firmware_init based on matching machine compatible.
>>
>> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
>> ---
>> arch/arm/mach-exynos/firmware.c | 100 ++++++++++++++++++++++++++++++----------
>> 1 file changed, 75 insertions(+), 25 deletions(-)
>>
>> diff --git a/arch/arm/mach-exynos/firmware.c b/arch/arm/mach-exynos/firmware.c
>> index fd6da54..525fbd9 100644
>> --- a/arch/arm/mach-exynos/firmware.c
>> +++ b/arch/arm/mach-exynos/firmware.c
>> @@ -35,6 +35,25 @@ static void exynos_save_cp15(void)
>> : : "cc");
>> }
>>
>> +static int exynos3250_do_idle(unsigned long mode)
>> +{
>> + switch (mode) {
>> + case FW_DO_IDLE_AFTR:
>> + writel_relaxed(virt_to_phys(exynos_cpu_resume_ns),
>> + sysram_ns_base_addr + 0x24);
>> + writel_relaxed(EXYNOS_AFTR_MAGIC, sysram_ns_base_addr + 0x20);
>> + flush_cache_all();
>> + exynos_smc(SMC_CMD_SAVE, OP_TYPE_CORE,
>> + SMC_POWERSTATE_IDLE, 0);
>> + exynos_smc(SMC_CMD_SHUTDOWN, OP_TYPE_CLUSTER,
>> + SMC_POWERSTATE_IDLE, 0);
>> + break;
>> + case FW_DO_IDLE_SLEEP:
>> + exynos_smc(SMC_CMD_SLEEP, 0, 0, 0);
>> + }
>> + return 0;
>> +}
>> +
>> static int exynos_do_idle(unsigned long mode)
>> {
>> switch (mode) {
>> @@ -44,14 +63,7 @@ static int exynos_do_idle(unsigned long mode)
>> writel_relaxed(virt_to_phys(exynos_cpu_resume_ns),
>> sysram_ns_base_addr + 0x24);
>> writel_relaxed(EXYNOS_AFTR_MAGIC, sysram_ns_base_addr + 0x20);
>> - if (soc_is_exynos3250()) {
>> - flush_cache_all();
>> - exynos_smc(SMC_CMD_SAVE, OP_TYPE_CORE,
>> - SMC_POWERSTATE_IDLE, 0);
>> - exynos_smc(SMC_CMD_SHUTDOWN, OP_TYPE_CLUSTER,
>> - SMC_POWERSTATE_IDLE, 0);
>> - } else
>> - exynos_smc(SMC_CMD_CPU0AFTR, 0, 0, 0);
>> + exynos_smc(SMC_CMD_CPU0AFTR, 0, 0, 0);
>> break;
>> case FW_DO_IDLE_SLEEP:
>> exynos_smc(SMC_CMD_SLEEP, 0, 0, 0);
>> @@ -59,28 +71,25 @@ static int exynos_do_idle(unsigned long mode)
>> return 0;
>> }
>>
>> -static int exynos_cpu_boot(int cpu)
>> +static int exynos4412_cpu_boot(int cpu)
>> {
>> /*
>> - * Exynos3250 doesn't need to send smc command for secondary CPU boot
>> - * because Exynos3250 removes WFE in secure mode.
>> - */
>> - if (soc_is_exynos3250())
>> - return 0;
>> -
>> - /*
>> * The second parameter of SMC_CMD_CPU1BOOT command means CPU id.
>> * But, Exynos4212 has only one secondary CPU so second parameter
>> * isn't used for informing secure firmware about CPU id.
>> */
>> - if (soc_is_exynos4212())
>> - cpu = 0;
>> + cpu = 0;
>
> Why are you clearing the cpu for Exynos4412? Was it tested on
> Exynos4412?
>
No I have not tested on Exynos4412.
I can see I missed this, and we are suppose clear the cpu only for Exynos4212.
I will fix this in v9 and resubmit again. Thanks for noticing this and
pointing out.
>> + exynos_smc(SMC_CMD_CPU1BOOT, cpu, 0, 0);
>> + return 0;
>> +}
>>
>> +static int exynos_cpu_boot(int cpu)
>> +{
>> exynos_smc(SMC_CMD_CPU1BOOT, cpu, 0, 0);
>
> This will be executed on Exynos4212...
>
Yes, which is wrong. This should be for Exynos4412 and previous one
(exynos4412_cpu_boot) is applicable for Exynos4212. I will fix this in v9.
>> return 0;
>> }
>>
>> -static int exynos_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
>> +static int exynos4412_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
>> {
>> void __iomem *boot_reg;
>>
>> @@ -94,14 +103,24 @@ static int exynos_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
>> * additional offset for every CPU, with Exynos4412 being the only
>> * exception.
>> */
>> - if (soc_is_exynos4412())
>> - boot_reg += 4 * cpu;
>> + boot_reg += 4 * cpu;
>> + writel_relaxed(boot_addr, boot_reg);
>> + return 0;
>> +}
>> +
>> +static int exynos_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
>> +{
>> + void __iomem *boot_reg;
>>
>> + if (!sysram_ns_base_addr)
>> + return -ENODEV;
>> +
>> + boot_reg = sysram_ns_base_addr + 0x1c;
>> writel_relaxed(boot_addr, boot_reg);
>> return 0;
>> }
>>
>> -static int exynos_get_cpu_boot_addr(int cpu, unsigned long *boot_addr)
>> +static int exynos4412_get_cpu_boot_addr(int cpu, unsigned long *boot_addr)
>> {
>> void __iomem *boot_reg;
>>
>> @@ -109,10 +128,19 @@ static int exynos_get_cpu_boot_addr(int cpu, unsigned long *boot_addr)
>> return -ENODEV;
>>
>> boot_reg = sysram_ns_base_addr + 0x1c;
>> + boot_reg += 4 * cpu;
>> + *boot_addr = readl_relaxed(boot_reg);
>> + return 0;
>> +}
>> +
>> +static int exynos_get_cpu_boot_addr(int cpu, unsigned long *boot_addr)
>> +{
>> + void __iomem *boot_reg;
>>
>> - if (soc_is_exynos4412())
>> - boot_reg += 4 * cpu;
>> + if (!sysram_ns_base_addr)
>> + return -ENODEV;
>>
>> + boot_reg = sysram_ns_base_addr + 0x1c;
>> *boot_addr = readl_relaxed(boot_reg);
>> return 0;
>> }
>> @@ -148,6 +176,23 @@ static int exynos_resume(void)
>> return 0;
>> }
>>
>> +static const struct firmware_ops exynos3250_firmware_ops = {
>> + .do_idle = IS_ENABLED(CONFIG_EXYNOS_CPU_SUSPEND) ? exynos3250_do_idle : NULL,
>> + .set_cpu_boot_addr = exynos_set_cpu_boot_addr,
>> + .get_cpu_boot_addr = exynos_get_cpu_boot_addr,
>
> You know that lack of cpu_boot() is not equivalent to previous
> 'return 0' code? Now -ENOSYS will be returned... which is not a problem
> because return values for cpu_boot are ignored... just wondering whether
> this was planned.
Yes, I feel it should return -ENOSYS, if the particular ops is not
relevant or applicable
for some SoC, rather having blank implementation and returning 0 is
should return error
code.
>
>> + .suspend = IS_ENABLED(CONFIG_PM_SLEEP) ? exynos_suspend : NULL,
>> + .resume = IS_ENABLED(CONFIG_EXYNOS_CPU_SUSPEND) ? exynos_resume : NULL,
>> +};
>> +
>> +static const struct firmware_ops exynos4412_firmware_ops = {
>> + .do_idle = IS_ENABLED(CONFIG_EXYNOS_CPU_SUSPEND) ? exynos_do_idle : NULL,
>> + .set_cpu_boot_addr = exynos4412_set_cpu_boot_addr,
>> + .get_cpu_boot_addr = exynos4412_get_cpu_boot_addr,
>> + .cpu_boot = exynos4412_cpu_boot,
>> + .suspend = IS_ENABLED(CONFIG_PM_SLEEP) ? exynos_suspend : NULL,
>> + .resume = IS_ENABLED(CONFIG_EXYNOS_CPU_SUSPEND) ? exynos_resume : NULL,
>> +};
>> +
>> static const struct firmware_ops exynos_firmware_ops = {
>> .do_idle = IS_ENABLED(CONFIG_EXYNOS_CPU_SUSPEND) ? exynos_do_idle : NULL,
>> .set_cpu_boot_addr = exynos_set_cpu_boot_addr,
>> @@ -212,7 +257,12 @@ void __init exynos_firmware_init(void)
>>
>> pr_info("Running under secure firmware.\n");
>>
>> - register_firmware_ops(&exynos_firmware_ops);
>> + if (of_machine_is_compatible("samsung,exynos3250"))
>> + register_firmware_ops(&exynos3250_firmware_ops);
>> + else if (of_machine_is_compatible("samsung,exynos4412"))
>> + register_firmware_ops(&exynos4412_firmware_ops);
>> + else
>> + register_firmware_ops(&exynos_firmware_ops);
>
> I prefer one register_firmware_ops() call, so something like:
> const struct firmware_ops *ops;
> if (...)
> ops = &exynos3250_firmware_ops;
> else if ()
> ...
> register_firmware_ops(ops);
>
> It is a matter of taste but for me it is more common pattern, looks more
> readable and it reduces number of callers to register_firmware_ops() (so
> it is easier to find them).
>
This suggestion looks good to me as well. Will adopt this in v9.
Thanks for your review.
Pankaj Dubey
^ permalink raw reply
* [PATCH] Documentation: dt: Explicitly mark Samsung Exynos SoC bindings as unstable
From: Pankaj Dubey @ 2016-12-17 3:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481897676-13578-1-git-send-email-m.szyprowski@samsung.com>
Hi Marek,
On 16 December 2016 at 19:44, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> Samsung Exynos SoCs and boards related bindings evolved since the initial
> introduction, but initially the bindings were minimal and a bit incomplete
> (they never described all the hardware modules available in the SoCs).
> Since then some significant (not fully compatible) changes have been
> already committed a few times (like gpio replaced by pinctrl, display ddc,
> mfc reserved memory, some core clocks added to various hardware modules,
> added more required nodes).
>
> On the other side there are no boards which have device tree embedded in
> the bootloader. Device tree blob is always compiled from the kernel tree
> and updated together with the kernel image.
>
> Thus to avoid further adding a bunch of workarounds for old/missing
> bindings and allow to make cleanup of the existing code and device tree
> files, lets mark Samsung Exynos SoC platform bindings as unstable. This
> means that bindings can may change at any time and users should use the
> dtb file compiled from the same kernel source tree as the kernel image.
>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
I agree with you. This is very much required. It's not only about new
bindings, we are facing problems in adopting existing bindings as well
(e.g scu), to make exynos support completely DT based and simplify our
code base.
I expect and foresee requirements of many more such changes in very near future.
Reviewed-by: Pankaj Dubey <pankaj.dubey@samsung.com>
Thanks,
Pankaj Dubey
^ permalink raw reply
* [PATCH 2/2] remoteproc: Remove firmware_loading_complete
From: Sarangdhar Joshi @ 2016-12-17 2:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161216192839.GT3439@tuxbot>
On 12/16/2016 11:28 AM, Bjorn Andersson wrote:
> On Fri 16 Dec 00:26 PST 2016, loic pallardy wrote:
>
>>
>>
>> On 12/16/2016 01:03 AM, Sarangdhar Joshi wrote:
>>> rproc_del() waits on firmware_loading_complete in order to
>>> make sure rproc_add() completed successfully before calling
>>> rproc_shutdown(). However since rproc_add() will always be
>>> called before rproc_del(), we do not need to wait on
>>> firmware_loading_complete. Drop this completion variable
>>> altogether.
>>>
>> Hi,
>>
>> firmware_loading_complete is used to synchronize all operations on rproc
>> with parallel work launched by request_firmware_nowait.
>
> We had a deadlock scenario in this code, where a call to rproc_boot()
> would grab the rproc mutex and the request_firmware_nowait() callback
> would wait on this lock before it would signal the completion that the
> rproc_boot() was waiting for.
>
> As the request_firmware_nowait() doesn't do anything other than handle
> auto_boot and signal the completion - and there is an internal sleep
> mechanism for handling concurrent request_firmware calls - I posted a
> patch and dropped the rproc_boot() wait thing.
That's right. Should have added reference to commit
"e9b4f9efff5021 ("remoteproc: Drop wait in __rproc_boot()")"
>
>> rproc_add could be done and firmware loading still pending. In that case
>> rproc_del mustn't be called before end of the procedure.
>
> You're right.
>
> We might have an outstanding request_firmware_nowait() when we hit
> rproc_del() and we might free the underlaying rproc context.
>
> Holding a reference over the request_firmware_nowait() would solve this,
> but would cause issues if we get a rproc_add() from the same driver
> (e.g. after module unload/load) before the firmware timer has fired -
> and released the resources.
The asynchronous work request_firmware_work_func() is protected by
get_device()/put_device() on remoteproc device. So we are probably
covered for remoteproc device. However, I agree that parent device will
still be an issue.
>
> This issue could be remedied by moving the rproc_delete_debug_dir() to
> rproc_del() and aim for not having any objects exposed outside the
> remoteproc core once rproc_del() returns.
>
>>
>> If you decide to remove this synchronization you need either to modify rproc
>> boot sequence or to replace it by something else.
>>
>
> I agree.
I agree too. rproc_boot() calls for non auto_boot case anyway calls
request_firmware(). So calling __request_firmware asynchronously for non
auto_boot case seems redundant. I was planning to send a patch to call
rproc_add_virtio_devices() for auto_boot case only. I guess I'll need to
take care of only auto_boot case for the current issue then.
Regards,
Sarang
>
> Regards,
> Bjorn
>
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* [PATCHv4 00/15] clk: ti: add support for hwmod clocks
From: Stephen Boyd @ 2016-12-17 1:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4b016883-e7c9-94d8-d964-3c8dfee6ee13@ti.com>
On 12/13, Tero Kristo wrote:
> On 13/12/16 06:40, Michael Turquette wrote:
> >>>On 12/12, Michael Turquette wrote:
> >
> >Is the goal to describe this hardware topology in DT? Is that right
> >thing to do? I think it's cool to have this modeled *somehow* in Linux,
> >but I'm not sure DT is the right place to model the interconnect and
> >every device hanging off of it.
> >
> >I don't want to put words in Stephen's mouth, but I think the issue over
> >whether clockdomains are CCF clock providers or some genpd thing is
> >probably less important to him than the fact that the DT bindings are
> >super detailed to inner workings of the SoC.
>
> Ok, so your preference would be to reduce the data under DT, and the
> ideal approach would be a single prcm node. I think we still need to
> keep the prm / cm1 / cm2 as separate nodes, as they are pretty
> individual from hardware point of view, provide quite different
> features, and they reside in some cases in quite different address
> spaces also. Anyway, here's what I gather we should probably have in
> DT:
>
> - reset provider
> * example: resets = <&prm OMAP4_IVA2_RESET>;
> * only from 'prm' node
>
> - genpd provider (for the hwmods, clockdomains, powerdomains,
> voltage domains)
> * examples: power-domains = <&cm2 OMAP4_DSS_CORE_MOD>;
> power-domains = <&cm2 OMAP4_DSS_CLKDM>;
> power-domains = <&prm OMAP4_DSS_PWRDM>;
> power-domains = <&prm OMAP4_CORE_VOLTDM>;
> * from all 'prm', 'cm1' and 'cm2' nodes, though 'prm' would be the
> only one providing _CLKDM, _PWRDM, _VOLTDM genpds.
>
> - clock provider (for anything that requires clocks)
> * example: clocks = <&cm1 OMAP4_DPLL_MPU_CK>;
> * from all 'prm', 'cm1' and 'cm2' nodes
>
> This would eventually cause an ABI breakage for the clock handles,
> if we transfer the existing clocks to this format, and remove the
> existing clock handles from DT. Otherwise, I think we could just
> transition the existing hwmod data to this new format only, and add
> the clockdomain / powerdomain / voltagedomain support a bit later.
>
This sounds about right. Is the ABI break because we get rid of
clock nodes and just have a few big nodes? I thought we had
already broken DT ABI here but if we didn't then that isn't
great. Perhaps to make things keep working we can detect the old
style one node per clock design and register a bunch of providers
individually from the single driver probe? It would have to match
up the registers with a clk_hw pointer somewhere, but it should
be possible. Alternatively, we keep both designs around for some
time and have different compatibles and different driver entry
points.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* [PATCH 3/3] arm64: dts: rockchip: add clk-480m for ehci and ohci of rk3399
From: Heiko Stuebner @ 2016-12-17 1:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5853903D.8030605@rock-chips.com>
Am Freitag, 16. Dezember 2016, 14:57:01 CET schrieb Xing Zheng:
> Hi Heiko, Doug,
>
> On 2016?12?16? 02:18, Heiko Stuebner wrote:
> > Am Donnerstag, 15. Dezember 2016, 08:34:09 CET schrieb Doug Anderson:
> >> I still need to digest all of the things that were added to this
> >> thread overnight, but nothing I've seen so far indicates that you need
> >> the post-gated clock. AKA I still think you need to redo your patch
> >>
> >> to replace:
> >> clocks = <&cru HCLK_HOST0>, <&cru HCLK_HOST0_ARB>,
> >>
> >> <&cru SCLK_USBPHY0_480M_SRC>;
> >>
> >> with:
> >> clocks = <&cru HCLK_HOST0>, <&cru HCLK_HOST0_ARB>,
> >>
> >> <&u2phy0>;
> >>
> >> Can you please comment on that?
> >
> > Also, with the change, the ehci will keep the clock (and thus the phy)
> > always on. Does the phy-autosuspend even save anything now?
> >
> > In any case, could we make the clock-names entry sound nicer than
> > usbphy0_480m please? bindings/usb/atmel-usb.txt calls its UTMI clock
> > simply "usb_clk", but something like "utmi" should also work.
> > While at it you could also fix up the other clock names to something like
> > "host" and "arbiter" or so?.
> >
> >
> > Heiko
>
> The usbphy related clock tress like this:
>
>
> Actually, at drivers/phy/phy-rockchip-inno-usb2.c, we can only
> enable/disable the master gate via GRF is PHY_PLL, not UTMI_CLK.
>
> And the naming style of the "hclk_host0" keep the name "hclk_host0" on
> the clcok tree diagram:
>
>
> Therefore, could we rename the clock name like this:
> ----
> for usb_host0_ehci and usb_host0_ohci:
> clocks = <&cru HCLK_HOST0>, <&cru HCLK_HOST0_ARB>,
> <&cru SCLK_U2PHY0>;
> clock-names = "hclk_host0", "hclk_host0_arb",
> "sclk_u2phy0";
>
> for usb_host1_ehci and usb_host1_ohci:
> clocks = <&cru HCLK_HOST1>, <&cru HCLK_HOST1_ARB>,
> <&cru SCLK_U2PHY1>;
> clock-names = "hclk_host1", "hclk_host1_arb",
> "sclk_u2phy1";
> ----
>
> BTW, the "arb" is an abbreviation for arbiter.
clock-naming wise, the clock names in devicetree bindings should always
describe the clock in the context of the peripheral, not the hosts clock-tree.
So if the clock supplies the "arbiter" part, the clock-name should be called
"arbiter". Same for "utmi" and the host clock that could be named "usbhost" or
just "host" in the clock-names property.
^ permalink raw reply
* [PATCH] mtk-vcodec: use designated initializers
From: Kees Cook @ 2016-12-17 1:00 UTC (permalink / raw)
To: linux-arm-kernel
Prepare to mark sensitive kernel structures for randomization by making
sure they're using designated initializers. These were identified during
allyesconfig builds of x86, arm, and arm64, with most initializer fixes
extracted from grsecurity.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/media/platform/mtk-vcodec/venc/venc_h264_if.c | 8 ++++----
drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c | 8 ++++----
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/media/platform/mtk-vcodec/venc/venc_h264_if.c b/drivers/media/platform/mtk-vcodec/venc/venc_h264_if.c
index b76c80bdf30b..4eb3be37ba14 100644
--- a/drivers/media/platform/mtk-vcodec/venc/venc_h264_if.c
+++ b/drivers/media/platform/mtk-vcodec/venc/venc_h264_if.c
@@ -665,10 +665,10 @@ static int h264_enc_deinit(unsigned long handle)
}
static const struct venc_common_if venc_h264_if = {
- h264_enc_init,
- h264_enc_encode,
- h264_enc_set_param,
- h264_enc_deinit,
+ .init = h264_enc_init,
+ .encode = h264_enc_encode,
+ .set_param = h264_enc_set_param,
+ .deinit = h264_enc_deinit,
};
const struct venc_common_if *get_h264_enc_comm_if(void);
diff --git a/drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c b/drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c
index 544f57186243..a6fa145f2c54 100644
--- a/drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c
+++ b/drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c
@@ -470,10 +470,10 @@ static int vp8_enc_deinit(unsigned long handle)
}
static const struct venc_common_if venc_vp8_if = {
- vp8_enc_init,
- vp8_enc_encode,
- vp8_enc_set_param,
- vp8_enc_deinit,
+ .init = vp8_enc_init,
+ .encode = vp8_enc_encode,
+ .set_param = vp8_enc_set_param,
+ .deinit = vp8_enc_deinit,
};
const struct venc_common_if *get_vp8_enc_comm_if(void);
--
2.7.4
--
Kees Cook
Nexus Security
^ permalink raw reply related
* [PATCH v6 0/5] davinci: VPIF: add DT support
From: Kevin Hilman @ 2016-12-17 0:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <d4b0501a-f83a-c8b1-e460-1ba50f68cca7@xs4all.nl>
Hans Verkuil <hverkuil@xs4all.nl> writes:
> On 07/12/16 19:30, Kevin Hilman wrote:
>> Prepare the groundwork for adding DT support for davinci VPIF drivers.
>> This series does some fixups/cleanups and then adds the DT binding and
>> DT compatible string matching for DT probing.
>>
>> The controversial part from previous versions around async subdev
>> parsing, and specifically hard-coding the input/output routing of
>> subdevs, has been left out of this series. That part can be done as a
>> follow-on step after agreement has been reached on the path forward.
>> With this version, platforms can still use the VPIF capture/display
>> drivers, but must provide platform_data for the subdevs and subdev
>> routing.
>>
>> Tested video capture to memory on da850-lcdk board using composite
>> input.
>
> Other than the comment for the first patch this series looks good.
>
> So once that's addressed I'll queue it up for 4.11.
I've fixed that issue, and sent an update for just that patch in reply
to the original.
Thanks for the review,
Kevin
^ permalink raw reply
* [PATCH v6.1 1/5] [media] davinci: VPIF: fix module loading, init errors
From: Kevin Hilman @ 2016-12-17 0:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161207183025.20684-2-khilman@baylibre.com>
Fix problems with automatic module loading by adding MODULE_ALIAS. Also
fix various load-time errors cause by incorrect or not present
platform_data.
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Kevin Hilman <khilman@baylibre.com>
---
Minor tweaks since v6
- added ack from Sakari
- droped an extraneous change for NULL subdev_info
drivers/media/platform/davinci/vpif.c | 5 ++++-
drivers/media/platform/davinci/vpif_capture.c | 13 +++++++++++++
drivers/media/platform/davinci/vpif_display.c | 6 ++++++
3 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/davinci/vpif.c b/drivers/media/platform/davinci/vpif.c
index 0380cf2e5775..f50148dcba64 100644
--- a/drivers/media/platform/davinci/vpif.c
+++ b/drivers/media/platform/davinci/vpif.c
@@ -32,6 +32,9 @@
MODULE_DESCRIPTION("TI DaVinci Video Port Interface driver");
MODULE_LICENSE("GPL");
+#define VPIF_DRIVER_NAME "vpif"
+MODULE_ALIAS("platform:" VPIF_DRIVER_NAME);
+
#define VPIF_CH0_MAX_MODES 22
#define VPIF_CH1_MAX_MODES 2
#define VPIF_CH2_MAX_MODES 15
@@ -466,7 +469,7 @@ static const struct dev_pm_ops vpif_pm = {
static struct platform_driver vpif_driver = {
.driver = {
- .name = "vpif",
+ .name = VPIF_DRIVER_NAME,
.pm = vpif_pm_ops,
},
.remove = vpif_remove,
diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
index 5104cc0ee40e..892a26f3c5b4 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -45,6 +45,7 @@ module_param(debug, int, 0644);
MODULE_PARM_DESC(debug, "Debug level 0-1");
#define VPIF_DRIVER_NAME "vpif_capture"
+MODULE_ALIAS("platform:" VPIF_DRIVER_NAME);
/* global variables */
static struct vpif_device vpif_obj = { {NULL} };
@@ -647,6 +648,10 @@ static int vpif_input_to_subdev(
vpif_dbg(2, debug, "vpif_input_to_subdev\n");
+ if (!chan_cfg)
+ return -1;
+ if (input_index >= chan_cfg->input_count)
+ return -1;
subdev_name = chan_cfg->inputs[input_index].subdev_name;
if (subdev_name == NULL)
return -1;
@@ -685,6 +690,9 @@ static int vpif_set_input(
if (sd_index >= 0) {
sd = vpif_obj.sd[sd_index];
subdev_info = &vpif_cfg->subdev_info[sd_index];
+ } else {
+ /* no subdevice, no input to setup */
+ return 0;
}
/* first setup input path from sub device to vpif */
@@ -1435,6 +1443,11 @@ static __init int vpif_probe(struct platform_device *pdev)
int res_idx = 0;
int i, err;
+ if (!pdev->dev.platform_data) {
+ dev_warn(&pdev->dev, "Missing platform data. Giving up.\n");
+ return -EINVAL;
+ }
+
vpif_dev = &pdev->dev;
err = initialize_vpif();
diff --git a/drivers/media/platform/davinci/vpif_display.c b/drivers/media/platform/davinci/vpif_display.c
index 75b27233ec2f..7f632b757d32 100644
--- a/drivers/media/platform/davinci/vpif_display.c
+++ b/drivers/media/platform/davinci/vpif_display.c
@@ -42,6 +42,7 @@ module_param(debug, int, 0644);
MODULE_PARM_DESC(debug, "Debug level 0-1");
#define VPIF_DRIVER_NAME "vpif_display"
+MODULE_ALIAS("platform:" VPIF_DRIVER_NAME);
/* Is set to 1 in case of SDTV formats, 2 in case of HDTV formats. */
static int ycmux_mode;
@@ -1249,6 +1250,11 @@ static __init int vpif_probe(struct platform_device *pdev)
int res_idx = 0;
int i, err;
+ if (!pdev->dev.platform_data) {
+ dev_warn(&pdev->dev, "Missing platform data. Giving up.\n");
+ return -EINVAL;
+ }
+
vpif_dev = &pdev->dev;
err = initialize_vpif();
--
2.9.3
^ permalink raw reply related
* [PATCH v6 1/5] [media] davinci: VPIF: fix module loading, init errors
From: Kevin Hilman @ 2016-12-16 23:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <19dc6321-1433-e089-f753-e5f736e26073@xs4all.nl>
Hans Verkuil <hverkuil@xs4all.nl> writes:
> On 07/12/16 19:30, Kevin Hilman wrote:
>> Fix problems with automatic module loading by adding MODULE_ALIAS. Also
>> fix various load-time errors cause by incorrect or not present
>> platform_data.
>>
>> Signed-off-by: Kevin Hilman <khilman@baylibre.com>
>> ---
>> drivers/media/platform/davinci/vpif.c | 5 ++++-
>> drivers/media/platform/davinci/vpif_capture.c | 15 ++++++++++++++-
>> drivers/media/platform/davinci/vpif_display.c | 6 ++++++
>> 3 files changed, 24 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/media/platform/davinci/vpif.c b/drivers/media/platform/davinci/vpif.c
>> index 0380cf2e5775..f50148dcba64 100644
>> --- a/drivers/media/platform/davinci/vpif.c
>> +++ b/drivers/media/platform/davinci/vpif.c
>> @@ -32,6 +32,9 @@
>> MODULE_DESCRIPTION("TI DaVinci Video Port Interface driver");
>> MODULE_LICENSE("GPL");
>>
>> +#define VPIF_DRIVER_NAME "vpif"
>> +MODULE_ALIAS("platform:" VPIF_DRIVER_NAME);
>> +
>> #define VPIF_CH0_MAX_MODES 22
>> #define VPIF_CH1_MAX_MODES 2
>> #define VPIF_CH2_MAX_MODES 15
>> @@ -466,7 +469,7 @@ static const struct dev_pm_ops vpif_pm = {
>>
>> static struct platform_driver vpif_driver = {
>> .driver = {
>> - .name = "vpif",
>> + .name = VPIF_DRIVER_NAME,
>> .pm = vpif_pm_ops,
>> },
>> .remove = vpif_remove,
>> diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
>> index 5104cc0ee40e..20c4344ed118 100644
>> --- a/drivers/media/platform/davinci/vpif_capture.c
>> +++ b/drivers/media/platform/davinci/vpif_capture.c
>> @@ -45,6 +45,7 @@ module_param(debug, int, 0644);
>> MODULE_PARM_DESC(debug, "Debug level 0-1");
>>
>> #define VPIF_DRIVER_NAME "vpif_capture"
>> +MODULE_ALIAS("platform:" VPIF_DRIVER_NAME);
>>
>> /* global variables */
>> static struct vpif_device vpif_obj = { {NULL} };
>> @@ -647,6 +648,10 @@ static int vpif_input_to_subdev(
>>
>> vpif_dbg(2, debug, "vpif_input_to_subdev\n");
>>
>> + if (!chan_cfg)
>> + return -1;
>> + if (input_index >= chan_cfg->input_count)
>> + return -1;
>> subdev_name = chan_cfg->inputs[input_index].subdev_name;
>> if (subdev_name == NULL)
>> return -1;
>> @@ -654,7 +659,7 @@ static int vpif_input_to_subdev(
>> /* loop through the sub device list to get the sub device info */
>> for (i = 0; i < vpif_cfg->subdev_count; i++) {
>> subdev_info = &vpif_cfg->subdev_info[i];
>> - if (!strcmp(subdev_info->name, subdev_name))
>> + if (subdev_info && !strcmp(subdev_info->name, subdev_name))
>
> Why this change? subdev_info can never be NULL.
A debugging leftover I guess. Will remove and resend.
Thanks for the review,
Kevin
^ permalink raw reply
* [PATCH] i2c: mux: pca954x: fix i2c mux selection caching
From: Russell King - ARM Linux @ 2016-12-16 23:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <251c31e3-141c-8884-d56a-fa539714d1ff@axentia.se>
On Fri, Dec 16, 2016 at 10:20:35PM +0100, Peter Rosin wrote:
> On 2016-12-16 21:06, Russell King wrote:
> > smbus functions return -ve on error, 0 on success. However,
> > __i2c_transfer() have a different return signature - -ve on error, or
> > number of buffers transferred (which may be zero or greater.)
> >
> > The upshot of this is that the sense of the test is reversed when using
> > the mux on a bus supporting the master_xfer method: we cache the value
> > and never retry if we fail to transfer any buffers, but if we succeed,
> > we clear the cached value.
>
> Ouch! Thanks for catching this.
>
> > Fix this.
>
> But lets fix the corner case of __i2c_transfer returning 0 instead of
> the expected 1 as well (not sure if that's even possible, but lets close
> the possibility just in case), so I'd prefer if you could fix
> pca954x_reg_write() to return 0 iff __i2c_transfer(...) returns 1
> instead, and -EREMOTEIO on other non-negative return values. Thanks!
So you want something like this instead?
diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
index 8bc3d36d2837..9c4ac26c014e 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -151,6 +151,9 @@ static int pca954x_reg_write(struct i2c_adapter *adap,
buf[0] = val;
msg.buf = buf;
ret = __i2c_transfer(adap, &msg, 1);
+
+ if (ret >= 0 && ret != 1)
+ ret = -EREMOTEIO;
} else {
union i2c_smbus_data data;
ret = adap->algo->smbus_xfer(adap, client->addr,
@@ -179,7 +182,7 @@ static int pca954x_select_chan(struct i2c_mux_core *muxc, u32 chan)
/* Only select the channel if its different from the last channel */
if (data->last_chan != regval) {
ret = pca954x_reg_write(muxc->parent, client, regval);
- data->last_chan = ret ? 0 : regval;
+ data->last_chan = ret < 0 ? 0 : regval;
}
return ret;
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently@9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply related
* [PATCH v6 5/5] ARM: dts: imx6q-evi: support cyclone-ps-spi
From: Joshua Clayton @ 2016-12-16 23:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1481918884.git.stillcompiling@gmail.com>
Add support for Altera cyclone V FPGA connected to an spi port
to the evi devicetree file
Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
---
arch/arm/boot/dts/imx6q-evi.dts | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/arch/arm/boot/dts/imx6q-evi.dts b/arch/arm/boot/dts/imx6q-evi.dts
index 7c7c1a8..ec4d365 100644
--- a/arch/arm/boot/dts/imx6q-evi.dts
+++ b/arch/arm/boot/dts/imx6q-evi.dts
@@ -95,6 +95,15 @@
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_ecspi1 &pinctrl_ecspi1cs>;
status = "okay";
+
+ fpga_spi: cyclonespi at 0 {
+ compatible = "altr,cyclone-ps-spi-fpga-mgr";
+ spi-max-frequency = <20000000>;
+ reg = <0>;
+ pinctrl-0 = <&pinctrl_fpgaspi>;
+ config-gpios = <&gpio4 9 GPIO_ACTIVE_LOW>;
+ status-gpios = <&gpio4 11 GPIO_ACTIVE_LOW>;
+ };
};
&ecspi3 {
@@ -322,6 +331,13 @@
>;
};
+ pinctrl_fpgaspi: fpgaspigrp {
+ fsl,pins = <
+ MX6QDL_PAD_KEY_ROW1__GPIO4_IO09 0x1b0b0
+ MX6QDL_PAD_KEY_ROW2__GPIO4_IO11 0x1b0b0
+ >;
+ };
+
pinctrl_gpminand: gpminandgrp {
fsl,pins = <
MX6QDL_PAD_NANDF_CLE__NAND_CLE 0xb0b1
--
2.9.3
^ permalink raw reply related
* [PATCH v6 4/5] fpga manager: Add cyclone-ps-spi driver for Altera FPGAs
From: Joshua Clayton @ 2016-12-16 23:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1481918884.git.stillcompiling@gmail.com>
cyclone-ps-spi loads FPGA firmware over spi, using the "passive serial"
interface on Altera Cyclone FPGAS.
This is one of the simpler ways to set up an FPGA at runtime.
The signal interface is close to unidirectional spi with lsb first.
Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
---
drivers/fpga/Kconfig | 7 ++
drivers/fpga/Makefile | 1 +
drivers/fpga/cyclone-ps-spi.c | 186 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 194 insertions(+)
create mode 100644 drivers/fpga/cyclone-ps-spi.c
diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
index ce861a2..e6c032d 100644
--- a/drivers/fpga/Kconfig
+++ b/drivers/fpga/Kconfig
@@ -20,6 +20,13 @@ config FPGA_REGION
FPGA Regions allow loading FPGA images under control of
the Device Tree.
+config FPGA_MGR_CYCLONE_PS_SPI
+ tristate "Altera Cyclone FPGA Passive Serial over SPI"
+ depends on SPI
+ help
+ FPGA manager driver support for Altera Cyclone using the
+ passive serial interface over SPI
+
config FPGA_MGR_SOCFPGA
tristate "Altera SOCFPGA FPGA Manager"
depends on ARCH_SOCFPGA || COMPILE_TEST
diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
index 8df07bc..a112bef 100644
--- a/drivers/fpga/Makefile
+++ b/drivers/fpga/Makefile
@@ -6,6 +6,7 @@
obj-$(CONFIG_FPGA) += fpga-mgr.o
# FPGA Manager Drivers
+obj-$(CONFIG_FPGA_MGR_CYCLONE_PS_SPI) += cyclone-ps-spi.o
obj-$(CONFIG_FPGA_MGR_SOCFPGA) += socfpga.o
obj-$(CONFIG_FPGA_MGR_SOCFPGA_A10) += socfpga-a10.o
obj-$(CONFIG_FPGA_MGR_ZYNQ_FPGA) += zynq-fpga.o
diff --git a/drivers/fpga/cyclone-ps-spi.c b/drivers/fpga/cyclone-ps-spi.c
new file mode 100644
index 0000000..f9126f9
--- /dev/null
+++ b/drivers/fpga/cyclone-ps-spi.c
@@ -0,0 +1,186 @@
+/**
+ * Altera Cyclone Passive Serial SPI Driver
+ *
+ * Copyright (c) 2017 United Western Technologies, Corporation
+ *
+ * Joshua Clayton <stillcompiling@gmail.com>
+ *
+ * Manage Altera FPGA firmware that is loaded over spi using the passive
+ * serial configuration method.
+ * Firmware must be in binary "rbf" format.
+ * Works on Cyclone V. Should work on cyclone series.
+ * May work on other Altera FPGAs.
+ *
+ */
+
+#include <linux/bitrev.h>
+#include <linux/delay.h>
+#include <linux/fpga/fpga-mgr.h>
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+#include <linux/of_gpio.h>
+#include <linux/spi/spi.h>
+#include <linux/sizes.h>
+
+#define FPGA_RESET_TIME 50 /* time in usecs to trigger FPGA config */
+#define FPGA_MIN_DELAY 50 /* min usecs to wait for config status */
+#define FPGA_MAX_DELAY 1000 /* max usecs to wait for config status */
+
+struct cyclonespi_conf {
+ struct gpio_desc *config;
+ struct gpio_desc *status;
+ struct spi_device *spi;
+};
+
+static const struct of_device_id of_ef_match[] = {
+ { .compatible = "altr,cyclone-ps-spi-fpga-mgr", },
+ {}
+};
+MODULE_DEVICE_TABLE(of, of_ef_match);
+
+static enum fpga_mgr_states cyclonespi_state(struct fpga_manager *mgr)
+{
+ struct cyclonespi_conf *conf = (struct cyclonespi_conf *)mgr->priv;
+
+ if (gpiod_get_value(conf->status))
+ return FPGA_MGR_STATE_RESET;
+
+ return FPGA_MGR_STATE_UNKNOWN;
+}
+
+static int cyclonespi_write_init(struct fpga_manager *mgr,
+ struct fpga_image_info *info,
+ const char *buf, size_t count)
+{
+ struct cyclonespi_conf *conf = (struct cyclonespi_conf *)mgr->priv;
+ int i;
+
+ if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
+ dev_err(&mgr->dev, "Partial reconfiguration not supported.\n");
+ return -EINVAL;
+ }
+
+ gpiod_set_value(conf->config, 1);
+ usleep_range(FPGA_RESET_TIME, FPGA_RESET_TIME + 20);
+ if (!gpiod_get_value(conf->status)) {
+ dev_err(&mgr->dev, "Status pin should be low.\n");
+ return -EIO;
+ }
+
+ gpiod_set_value(conf->config, 0);
+ for (i = 0; i < (FPGA_MAX_DELAY / FPGA_MIN_DELAY); i++) {
+ usleep_range(FPGA_MIN_DELAY, FPGA_MIN_DELAY + 20);
+ if (!gpiod_get_value(conf->status))
+ return 0;
+ }
+
+ dev_err(&mgr->dev, "Status pin not ready.\n");
+ return -EIO;
+}
+
+static void rev_buf(void *buf, size_t len)
+{
+ u32 *fw32 = (u32 *)buf;
+ const u32 *fw_end = (u32 *)(buf + len);
+
+ /* set buffer to lsb first */
+ while (fw32 < fw_end) {
+ *fw32 = bitrev8x4(*fw32);
+ fw32++;
+ }
+}
+
+static int cyclonespi_write(struct fpga_manager *mgr, const char *buf,
+ size_t count)
+{
+ struct cyclonespi_conf *conf = (struct cyclonespi_conf *)mgr->priv;
+ const char *fw_data = buf;
+ const char *fw_data_end = fw_data + count;
+
+ while (fw_data < fw_data_end) {
+ int ret;
+ size_t stride = min(fw_data_end - fw_data, SZ_4K);
+
+ rev_buf((void *)fw_data, stride);
+ ret = spi_write(conf->spi, fw_data, stride);
+ if (ret) {
+ dev_err(&mgr->dev, "spi error in firmware write: %d\n",
+ ret);
+ return ret;
+ }
+ fw_data += stride;
+ }
+
+ return 0;
+}
+
+static int cyclonespi_write_complete(struct fpga_manager *mgr,
+ struct fpga_image_info *info)
+{
+ struct cyclonespi_conf *conf = (struct cyclonespi_conf *)mgr->priv;
+
+ if (gpiod_get_value(conf->status)) {
+ dev_err(&mgr->dev, "Error during configuration.\n");
+ return -EIO;
+ }
+
+ return 0;
+}
+
+static const struct fpga_manager_ops cyclonespi_ops = {
+ .state = cyclonespi_state,
+ .write_init = cyclonespi_write_init,
+ .write = cyclonespi_write,
+ .write_complete = cyclonespi_write_complete,
+};
+
+static int cyclonespi_probe(struct spi_device *spi)
+{
+ struct cyclonespi_conf *conf = devm_kzalloc(&spi->dev, sizeof(*conf),
+ GFP_KERNEL);
+
+ if (!conf)
+ return -ENOMEM;
+
+ conf->spi = spi;
+ conf->config = devm_gpiod_get(&spi->dev, "config", GPIOD_OUT_HIGH);
+ if (IS_ERR(conf->config)) {
+ dev_err(&spi->dev, "Failed to get config gpio: %ld\n",
+ PTR_ERR(conf->config));
+ return PTR_ERR(conf->config);
+ }
+
+ conf->status = devm_gpiod_get(&spi->dev, "status", GPIOD_IN);
+ if (IS_ERR(conf->status)) {
+ dev_err(&spi->dev, "Failed to get status gpio: %ld\n",
+ PTR_ERR(conf->status));
+ return PTR_ERR(conf->status);
+ }
+
+ return fpga_mgr_register(&spi->dev,
+ "Altera Cyclone PS SPI FPGA Manager",
+ &cyclonespi_ops, conf);
+}
+
+static int cyclonespi_remove(struct spi_device *spi)
+{
+ fpga_mgr_unregister(&spi->dev);
+
+ return 0;
+}
+
+static struct spi_driver cyclonespi_driver = {
+ .driver = {
+ .name = "cyclone-ps-spi",
+ .owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(of_ef_match),
+ },
+ .probe = cyclonespi_probe,
+ .remove = cyclonespi_remove,
+};
+
+module_spi_driver(cyclonespi_driver)
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Joshua Clayton <stillcompiling@gmail.com>");
+MODULE_DESCRIPTION("Module to load Altera FPGA firmware over spi");
--
2.9.3
^ permalink raw reply related
* [PATCH v6 3/5] doc: dt: add cyclone-ps-spi binding document
From: Joshua Clayton @ 2016-12-16 23:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1481918884.git.stillcompiling@gmail.com>
Describe a cyclone-ps-spi devicetree entry, required features
Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
Acked-by: Rob Herring <robh@kernel.org>
---
.../bindings/fpga/cyclone-ps-spi-fpga-mgr.txt | 25 ++++++++++++++++++++++
1 file changed, 25 insertions(+)
create mode 100644 Documentation/devicetree/bindings/fpga/cyclone-ps-spi-fpga-mgr.txt
diff --git a/Documentation/devicetree/bindings/fpga/cyclone-ps-spi-fpga-mgr.txt b/Documentation/devicetree/bindings/fpga/cyclone-ps-spi-fpga-mgr.txt
new file mode 100644
index 0000000..3f515c7
--- /dev/null
+++ b/Documentation/devicetree/bindings/fpga/cyclone-ps-spi-fpga-mgr.txt
@@ -0,0 +1,25 @@
+Altera Cyclone Passive Serial SPI FPGA Manager
+
+Altera Cyclone FPGAs support a method of loading the bitstream over what is
+referred to as "passive serial".
+The passive serial link is not technically spi, and might require extra
+circuits in order to play nicely with other spi slaves on the same bus.
+
+See https://www.altera.com/literature/hb/cyc/cyc_c51013.pdf
+
+Required properties:
+- compatible : should contain "altr,cyclone-ps-spi-fpga-mgr"
+- reg : spi slave id of the fpga
+- config-gpios : config pin (referred to as nCONFIG in the cyclone manual)
+- status-gpios : status pin (referred to as nSTATUS in the cyclone manual)
+
+both gpios pins are normally active low open drain.
+
+Example:
+ fpga_spi: evi-fpga-spi at 0 {
+ compatible = "altr,cyclone-ps-spi-fpga-mgr";
+ spi-max-frequency = <20000000>;
+ reg = <0>;
+ config-gpios = <&gpio4 9 GPIO_ACTIVE_LOW>;
+ status-gpios = <&gpio4 11 GPIO_ACTIVE_LOW>;
+ };
--
2.9.3
^ permalink raw reply related
* [PATCH v6 2/5] lib: implement __arch_bitrev8x4()
From: Joshua Clayton @ 2016-12-16 23:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1481918884.git.stillcompiling@gmail.com>
Implement faster bitrev8x4() for arm, arm64 and mips, all the platforms
with CONFIG_HAVE_ARCH_BITREVERSE.
ARM platforms just need a byteswap added to the existing __arch_bitrev32()
Amusingly, the mips implementation is exactly the opposite, requiring
removal of the byteswap from its __arch_bitrev32()
Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
---
arch/arm/include/asm/bitrev.h | 6 ++++++
arch/arm64/include/asm/bitrev.h | 6 ++++++
arch/mips/include/asm/bitrev.h | 6 ++++++
include/linux/bitrev.h | 1 +
4 files changed, 19 insertions(+)
diff --git a/arch/arm/include/asm/bitrev.h b/arch/arm/include/asm/bitrev.h
index ec291c3..9482f78 100644
--- a/arch/arm/include/asm/bitrev.h
+++ b/arch/arm/include/asm/bitrev.h
@@ -17,4 +17,10 @@ static __always_inline __attribute_const__ u8 __arch_bitrev8(u8 x)
return __arch_bitrev32((u32)x) >> 24;
}
+static __always_inline __attribute_const__ u32 __arch_bitrev8x4(u32 x)
+{
+ __asm__ ("rbit %0, %1; rev %0, %0" : "=r" (x) : "r" (x));
+ return x;
+}
+
#endif
diff --git a/arch/arm64/include/asm/bitrev.h b/arch/arm64/include/asm/bitrev.h
index a5a0c36..1801078 100644
--- a/arch/arm64/include/asm/bitrev.h
+++ b/arch/arm64/include/asm/bitrev.h
@@ -16,4 +16,10 @@ static __always_inline __attribute_const__ u8 __arch_bitrev8(u8 x)
return __arch_bitrev32((u32)x) >> 24;
}
+static __always_inline __attribute_const__ u32 __arch_bitrev8x4(u32 x)
+{
+ __asm__ ("rbit %0, %1; rev %0, %0" : "=r" (x) : "r" (x));
+ return x;
+}
+
#endif
diff --git a/arch/mips/include/asm/bitrev.h b/arch/mips/include/asm/bitrev.h
index bc739a4..9ac6439 100644
--- a/arch/mips/include/asm/bitrev.h
+++ b/arch/mips/include/asm/bitrev.h
@@ -27,4 +27,10 @@ static __always_inline __attribute_const__ u8 __arch_bitrev8(u8 x)
return ret;
}
+static __always_inline __attribute_const__ u32 __arch_bitrev8x4(u32 x)
+{
+ u32 ret;
+ asm("bitswap %0, %1" : "=r"(ret) : "r"(x));
+ return ret;
+}
#endif /* __MIPS_ASM_BITREV_H__ */
diff --git a/include/linux/bitrev.h b/include/linux/bitrev.h
index 868dcb6..b1cfa1a 100644
--- a/include/linux/bitrev.h
+++ b/include/linux/bitrev.h
@@ -9,6 +9,7 @@
#define __bitrev32 __arch_bitrev32
#define __bitrev16 __arch_bitrev16
#define __bitrev8 __arch_bitrev8
+#define __bitrev8x4 __arch_bitrev8x4
#else
extern u8 const byte_rev_table[256];
--
2.9.3
^ permalink raw reply related
* [PATCH v6 1/5] lib: add bitrev8x4()
From: Joshua Clayton @ 2016-12-16 23:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1481918884.git.stillcompiling@gmail.com>
Add a function to reverse bytes within a 32 bit word.
Operate on a u32 rather than individual bytes.
ARCH specific versions require substantially fewer instructions than
working a byte at a time.
Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
---
include/linux/bitrev.h | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/include/linux/bitrev.h b/include/linux/bitrev.h
index fb790b8..868dcb6 100644
--- a/include/linux/bitrev.h
+++ b/include/linux/bitrev.h
@@ -27,6 +27,14 @@ static inline u32 __bitrev32(u32 x)
return (__bitrev16(x & 0xffff) << 16) | __bitrev16(x >> 16);
}
+static inline u32 __bitrev8x4(u32 x)
+{
+ return(__bitrev8(x & 0xff) |
+ (__bitrev8((x >> 8) & 0xff) << 8) |
+ (__bitrev8((x >> 16) & 0xff) << 16) |
+ (__bitrev8((x >> 24) & 0xff) << 24));
+}
+
#endif /* CONFIG_HAVE_ARCH_BITREVERSE */
#define __constant_bitrev32(x) \
@@ -50,6 +58,15 @@ static inline u32 __bitrev32(u32 x)
__x; \
})
+#define __constant_bitrev8x4(x) \
+({ \
+ u32 __x = x; \
+ __x = ((__x & (u32)0xF0F0F0F0UL) >> 4) | ((__x & (u32)0x0F0F0F0FUL) << 4); \
+ __x = ((__x & (u32)0xCCCCCCCCUL) >> 2) | ((__x & (u32)0x33333333UL) << 2); \
+ __x = ((__x & (u32)0xAAAAAAAAUL) >> 1) | ((__x & (u32)0x55555555UL) << 1); \
+ __x; \
+})
+
#define __constant_bitrev8(x) \
({ \
u8 __x = x; \
@@ -75,6 +92,14 @@ static inline u32 __bitrev32(u32 x)
__bitrev16(__x); \
})
+#define bitrev8x4(x) \
+({ \
+ u32 __x = x; \
+ __builtin_constant_p(__x) ? \
+ __constant_bitrev8x4(__x) : \
+ __bitrev8x4(__x); \
+})
+
#define bitrev8(x) \
({ \
u8 __x = x; \
--
2.9.3
^ permalink raw reply related
* [PATCH v6 0/5] Altera Cyclone Passive Serial SPI FPGA Manager
From: Joshua Clayton @ 2016-12-16 23:17 UTC (permalink / raw)
To: linux-arm-kernel
This series adds an FPGA manager for Altera cyclone FPGAs
that can program them using an spi port and a couple of gpios, using
Alteras passive serial protocol.
Need ACKs from ARCH maintainers for ARCH specific implementations of
__arch_bitrev8x4(), and I've added more ARCHes, so will need more ACKS,
but the generic code will work without that patch, so if there is a holdup
the rest of the series can go in without patch 2
Changes from v5:
- Rebased on next-20161214xi
- Corrected for FPGA Mgr API change in write_init() and write_complete()
- Better describe the device cyclone-ps-spi runs on in the file header.
- Split the bitrev8x4 patch into generic and arch specific patches...
- Added AARCH64 and MIPS implementations of bitrev8x4()... they all have to
have an implementation for it to compile cleanly across platforms
- Added the changes to imx6q-evi.dts to the patch set.
Changes from v4:
- Added the needed return statement to __arch_bitrev8x4()
- Added Rob Herrings ACK for and fix a typo in the commit log of patch 2
Changes from v3:
- Fixed up the state() function to return the state of the status pin
reqested by Alan Tull
- Switched the pin to ACTIVE_LOW and coresponding logic level, and updated
the corresponding documentation. Thanks Rob Herring for pointing out my
mistake.
- Per Rob Herring, switched from "gpio" to "gpios" in dts
Changes from v2:
- Merged patch 3 and 4 as suggested in review by Moritz Fischer
- Changed FPGA_MIN_DELAY from 250 to 50 ms is the time advertized by
Altera. This now works, as we don't assume it is done
Changes from v1:
- Changed the name from cyclone-spi-fpga-mgr to cyclone-ps-spi-fpga-mgr
This name change was requested by Alan Tull, to be specific about which
programming method is being employed on the fpga.
- Changed the name of the reset-gpio to config-gpio to closer match the
way the pins are described in the Altera manual
- Moved MODULE_LICENCE, _AUTHOR, and _DESCRIPTION to the bottom
- Added a bitrev8x4() function to the bitrev headers and implemented ARM
const, runtime, and ARM specific faster versions (This may end up
needing to be a standalone patch)
- Moved the bitswapping into cyclonespi_write(), as requested.
This falls short of my desired generic lsb first spi support, but is a step
in that direction.
- Fixed whitespace problems introduced during refactoring
- Replaced magic number for initial delay with a descriptive macro
- Poll the fpga to see when it is ready rather than a fixed 1 ms sleep
Joshua Clayton (5):
lib: add bitrev8x4()
lib: implement __arch_bitrev8x4()
doc: dt: add cyclone-ps-spi binding document
fpga manager: Add cyclone-ps-spi driver for Altera FPGAs
ARM: dts: imx6q-evi: support cyclone-ps-spi
.../bindings/fpga/cyclone-ps-spi-fpga-mgr.txt | 25 +++
arch/arm/boot/dts/imx6q-evi.dts | 16 ++
arch/arm/include/asm/bitrev.h | 6 +
arch/arm64/include/asm/bitrev.h | 6 +
arch/mips/include/asm/bitrev.h | 6 +
drivers/fpga/Kconfig | 7 +
drivers/fpga/Makefile | 1 +
drivers/fpga/cyclone-ps-spi.c | 186 +++++++++++++++++++++
include/linux/bitrev.h | 26 +++
9 files changed, 279 insertions(+)
create mode 100644 Documentation/devicetree/bindings/fpga/cyclone-ps-spi-fpga-mgr.txt
create mode 100644 drivers/fpga/cyclone-ps-spi.c
--
2.9.3
^ permalink raw reply
* [PATCH] ARM: disallow ARM_THUMB for ARMv4 builds
From: Arnd Bergmann @ 2016-12-16 21:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKv+Gu93Gtkh7spA-HUHNKSmDjhK_nuwR9KaDu8=S2GDyhUJCA@mail.gmail.com>
On Friday, December 16, 2016 5:20:22 PM CET Ard Biesheuvel wrote:
>
> Can't we use the old
>
> tst lr, #1
> moveq pc, lr
> bx lr
>
> trick? (where bx lr needs to be emitted as a plain opcode to hide it
> from the assembler)
>
Yes, that should work around the specific problem in theory, but back
when Jonas tried it, it still didn't work. There may also be other
problems in that configuration.
Arnd
^ permalink raw reply
* [PATCH] i2c: mux: pca954x: fix i2c mux selection caching
From: Peter Rosin @ 2016-12-16 21:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <E1cHylf-00071o-KZ@rmk-PC.armlinux.org.uk>
On 2016-12-16 21:06, Russell King wrote:
> smbus functions return -ve on error, 0 on success. However,
> __i2c_transfer() have a different return signature - -ve on error, or
> number of buffers transferred (which may be zero or greater.)
>
> The upshot of this is that the sense of the test is reversed when using
> the mux on a bus supporting the master_xfer method: we cache the value
> and never retry if we fail to transfer any buffers, but if we succeed,
> we clear the cached value.
Ouch! Thanks for catching this.
> Fix this.
But lets fix the corner case of __i2c_transfer returning 0 instead of
the expected 1 as well (not sure if that's even possible, but lets close
the possibility just in case), so I'd prefer if you could fix
pca954x_reg_write() to return 0 iff __i2c_transfer(...) returns 1
instead, and -EREMOTEIO on other non-negative return values. Thanks!
Cheers,
peda
> Fixes: 463e8f845cbf ("i2c: mux: pca954x: retry updating the mux selection on failure")
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
> drivers/i2c/muxes/i2c-mux-pca954x.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
> index 8bc3d36d2837..b6d62ecbd5b6 100644
> --- a/drivers/i2c/muxes/i2c-mux-pca954x.c
> +++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
> @@ -179,7 +179,7 @@ static int pca954x_select_chan(struct i2c_mux_core *muxc, u32 chan)
> /* Only select the channel if its different from the last channel */
> if (data->last_chan != regval) {
> ret = pca954x_reg_write(muxc->parent, client, regval);
> - data->last_chan = ret ? 0 : regval;
> + data->last_chan = ret >= 0 ? regval : 0;
> }
>
> return ret;
>
^ permalink raw reply
* [PATCH v7 1/2] provide lock-less versions of clk_{enable|disable}
From: David Lechner @ 2016-12-16 20:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <35adff3d-c6a6-7b65-fb61-6da447dd6c3f@baylibre.com>
On 12/12/2016 04:27 AM, Alexandre Bailon wrote:
> On 12/12/2016 10:02 AM, Sekhar Nori wrote:
>> Hi Uwe,
>>
>> On Saturday 10 December 2016 12:24 AM, Uwe Kleine-K?nig wrote:
>>> Hello,
>>>
>>> On Fri, Dec 09, 2016 at 05:59:32PM +0100, Alexandre Bailon wrote:
>>>> Rename __clk_{enable|disble} in davinci_clk_{enable|disable}.
>>>> davinci_clk_{enable|disable} is a lock-less version of
>>>> clk_{enable|disable}.
>>>> This is useful to recursively enable clock without doing recursive call
>>>> to clk_enable(), which would cause a recursive locking issue.
>>>> The lock-less version could be used by example by the usb20 phy clock,
>>>> that need to enable the usb20 clock before to start.
>>>
>>> I wouldn't call that lock-less. The difference is that the newly exposed
>>> funcion requires the caller to already hold the lock. So maybe a better
>>
>> Right.
> Is it enough to update the commit message or should I also update the
> patch title?
> If so, could you suggest one because I don't know how to describe it
> shortly.
How about... "ARM: davinci: Make __clk_{enable,disable} functions public"
>
> Thanks,
> Alexandre
>
^ permalink raw reply
* [PATCH] i2c: mux: pca954x: fix i2c mux selection caching
From: Russell King @ 2016-12-16 20:06 UTC (permalink / raw)
To: linux-arm-kernel
smbus functions return -ve on error, 0 on success. However,
__i2c_transfer() have a different return signature - -ve on error, or
number of buffers transferred (which may be zero or greater.)
The upshot of this is that the sense of the test is reversed when using
the mux on a bus supporting the master_xfer method: we cache the value
and never retry if we fail to transfer any buffers, but if we succeed,
we clear the cached value.
Fix this.
Fixes: 463e8f845cbf ("i2c: mux: pca954x: retry updating the mux selection on failure")
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/i2c/muxes/i2c-mux-pca954x.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
index 8bc3d36d2837..b6d62ecbd5b6 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -179,7 +179,7 @@ static int pca954x_select_chan(struct i2c_mux_core *muxc, u32 chan)
/* Only select the channel if its different from the last channel */
if (data->last_chan != regval) {
ret = pca954x_reg_write(muxc->parent, client, regval);
- data->last_chan = ret ? 0 : regval;
+ data->last_chan = ret >= 0 ? regval : 0;
}
return ret;
--
2.7.4
^ permalink raw reply related
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