Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4] ARM64: dts: meson-gx: Add reserved memory zone and usable memory range
From: Andreas Färber @ 2017-01-16 15:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <7fcb8d94-840a-de2c-f43b-9123ccc65514@baylibre.com>

Hi Neil,

Am 16.01.2017 um 11:39 schrieb Neil Armstrong:
> On 01/15/2017 03:43 PM, Andreas F?rber wrote:
>> Am 13.01.2017 um 21:03 schrieb Kevin Hilman:
>>> Neil Armstrong <narmstrong@baylibre.com> writes:
>>>
>>>> The Amlogic Meson GXBB/GXL/GXM secure monitor uses part of the memory space,
>>>> this patch adds this reserved zone and redefines the usable memory range.
>>>>
>>>> The memory node is also moved from the dtsi files into the proper dts files
>>>> to handle variants memory sizes.
>>>>
>>>> This patch also fixes the memory sizes for the following platforms :
>>>> - gxl-s905x-p212 : 1GiB instead of 2GiB, a proper 2GiB dts should be pushed
>>>> - gxm-s912-q201 : 1GiB instead of 2GiB, a proper 2GiB dts should be pushed
>>>> - gxl-s905d-p231 : 1GiB instead of 2GiB, a proper 2GiB dts should be pushed
>>>> - gxl-nexbox-a95x : 1GiB instead of 2GiB, a proper 2GiB dts should be pushed
>>>>
>>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>>>
>>> Queued for v4.10-rc.
>>
>> What is the motivation for this change? I have a local U-Boot patch to
>> detect the amount of memory available as done downstream, but U-Boot
>> only updates the reg property that you seem to be abandoning here...
>>
>> So for devices that come in multiple RAM configurations - like R-Box Pro
>> - this would require separate .dts files now! This looks very wrong to
>> me, especially since I am not aware of other platforms doing the same.
>> Instead, there's memory reservations for top and bottom done in U-Boot
>> for reg, plus reserved-memory nodes for anything in the middle.
>>
>> Another thing to consider is that uEFI boot (bootefi) handles memory
>> reservation differently yet again, on the bootloader level. I have had
>> that working fine on Odroid-C2 and Vega S95.
>>
>> So if there's no bug this is fixing (none mentioned in commit message) I
>> strongly object to this patch.
>>
>> Regards,
>> Andreas
>>
> 
> Hi Andreas,
[snip]

Let's not copy&paste replies, see my response there.

> Handling multiple RAM configuration is another story, and the Arm-Soc and DT maintainers should give us
> their advices.

My point is, this should be thought through _before_ merging the patch,
not after.

It is the bootloader's task to deliver the correct memory _size_, with
kernel .dts having the minimum. If there's 1G and 2G models then the
linux.git .dts will have 1G, so that it can run on both, should the
bootloader fail to update it.
The consequence of your change would be that U-Boot needs to set
different $fdtfile values based on memory size, which is a plain stupid
idea for the reasons I already gave. And it has been fought by DT
maintainers in previous cases, such as FPGA configurations or
daughter-boards. Amlogic's vendor U-Boot does have the "fdt" command
available, for any user to adequately tweak a loaded .dtb for use with
mainline Linux (e.g., add linux,usable-memory there) - it can be
automated via environment variables or for lack of "source" command
maybe via "autoscr".

The reason that there are three vega-s95 .dts files never was the
differing memory reg size (which gets overridden), but rather connector
and Wifi chipset features as well as them simply having different names
and therefore different compatible strings.

Ideally I expect to be able to use one .dts for both R-Box Pro models as
well as for both Khadas Vim models - they are not marketed with
differing names, so the differences should hopefully be minor,
especially when we're using brcm,bcm4329-fmac for any chipset anyway.

> Actually there is a severe bug fixed here that cause a huge crash if such memory is not reserved while
> running stock u-boot version on various shipped products and Amlogic's own development boards.
> 
> The bug is easily triggered by running :
> # stress --vm 4 --vm-bytes 128M --timeout 10s &

First, that should've gone into the commit message please.

But this is what I get for that command line:

flag provided but not defined: -vm
Usage of stress:
  -failure regexp
    	fail only if output matches regexp
  -ignore regexp
    	ignore failure if output matches regexp
  -kill
    	kill timed out processes if true, otherwise just print pid (to
attach with gdb) (default true)
  -p N
    	run N processes in parallel (default 8)
  -timeout duration
    	timeout each process after duration (default 10m0s)

The only "stress" I found is in golang-org-x-tools package.

> [   46.937975] Bad mode in Error handler detected on CPU1, code 0xbf000000 -- SError
> ...
> [   47.058536] Internal error: Attempting to execute userspace memory: 8600000f [#3] PREEMPT SMP
> ...
> 
> Note this is a fix targeted for 4.10 to make the system stable and various users reported some severe
> crash now the system has more drivers and read-world use-cases are running on Amlogic SoCs.

I have been running "large" KVM guests on a Vega S95 Telos, with vendor
U-Boot as well as mainline U-Boot, and did not run into such a problem.

What I did run into yesterday during a large system update was multiple:

INFO: task grub2-probe:22018 blocked for more than 120 seconds.
      Not tainted 4.10.0-rc3-next-20170113+ #58
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
grub2-probe     D    0 22018  22017 0x00000000
Call trace:
[<ffffff8008085628>] __switch_to+0x90/0xa8
[<ffffff8008746238>] __schedule+0x188/0x570
[<ffffff8008746658>] schedule+0x38/0xa0
[<ffffff80081fb234>] wb_wait_for_completion+0x4c/0x80
[<ffffff80081fb2f0>] __writeback_inodes_sb_nr+0x88/0xa0
[<ffffff80081fb34c>] writeback_inodes_sb+0x2c/0x38
[<ffffff80081ff494>] sync_filesystem+0x3c/0xa8
[<ffffff8008208688>] fsync_bdev+0x20/0x70
[<ffffff80083508e8>] blkdev_ioctl+0x8b0/0x9d8
[<ffffff800820801c>] block_ioctl+0x34/0x40
[<ffffff80081e2d04>] do_vfs_ioctl+0xa4/0x748
[<ffffff80081e3434>] SyS_ioctl+0x8c/0xa0
[<ffffff8008082f30>] el0_svc_naked+0x24/0x28

I'm assuming that's an unrelated linux-next regression.

I have also been running vendor U-Boot on the R-Box Pro, without problems.

On the Odroid-C2 however the bootloader is provided on SD by the user,
so there is no excuse really for the user to use a broken bootloader.
Even if not using the mainline version for lack of MMC drivers, the
Hardkernel branch can easily be patched if necessary.

> Please feel free to push whatever changes that makes this memory reservation more coherent for 4.11,
> and respect the behavior of already shipped u-boot version and mainline U-Boot, UEFI, whatever...

Whatever the issue is, this patch is clearly wrong by design. Please
revert it ASAP!

For starters, have you tried simply adding a reserved-memory node for
0..0x01000000? v1 did not have that and instead messed with reg.

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 N?rnberg, Germany
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)

^ permalink raw reply

* [RFC PATCH 08/10] arm64/sve: ptrace: Wire up vector length control and reporting
From: Yao Qi @ 2017-01-16 15:11 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170116133231.GO3699@e103592.cambridge.arm.com>

On 17-01-16 13:32:31, Dave Martin wrote:
> On Mon, Jan 16, 2017 at 12:20:38PM +0000, Yao Qi wrote:
> > On 17-01-12 11:26:07, Dave Martin wrote:
> > > This patch adds support for manipulating a task's vector length at
> > > runtime via ptrace.
> > > 
> > 
> > I hope kernel doesn't provide such interface to ptracer to change vector
> > length.
> 
> It does, with this patch, beacuse...
> 
> > The vector length is sort of a read-only property of thread/process/
> > program to debugger, unless we really have a clear requirement to modify
> > vector length in debugging.  I may miss something because I haven't debug
> > SVE code yet.
> 
> ...the vector length is no longer read-only for the task, thanks to
> the new prctls().

What I meant "read-only" is that debugger can't change it, while the program
itself can change it via prctl().

> 
> This does add complexity, but I figured that any programmer's model
> state that the thread can modify for itself should be modifiable by the
> debugger, if for no other reason than the user may want to experiment to
> see what happens.  Without a ptrace interface, it would be necessary
> to inject a prctl() call into the target, which is possible but awkward.

We only need such interface if it is useful, see more below.

Suppose it is useful to change vector length through ptrace, we should align
ptrace interface to prctl() as much as possible.  Looks that both prctl
change and ptrace change can go through sve_set_vector_length, easy to keep
two consistent.

> 
> gdb must already re-detect the vector length on stop, since the target
> could have called the prctl() in the meantime.

Yes, gdb assumes the vector length may be changed, so it re-detects on
every stop, but I don't see the need for gdb to change the vector length.

> 
> Access via ptrace also allows things like trapping on exec, fork or
> clone and changing the vector length for the new process or thread
> before it starts to run.  I'm guessing here, but such a scenario seems
> legitimate (?)
> 

Yes, these cases are valid, but the usefulness is still questionable to
me.  I just doubt that SVE developers do need to change vector length
when they are debugging code.  Note that it is not my strong objection
to this patch, if kernel people believe this is useful, I am fine with
it.

-- 
Yao (??)

^ permalink raw reply

* [PATCH] rtc: sun6i: Switch to the external oscillator
From: Maxime Ripard @ 2017-01-16 15:21 UTC (permalink / raw)
  To: linux-arm-kernel

The RTC is clocked from either an internal, imprecise, oscillator or an
external one, which is usually much more accurate.

The difference perceived between the time elapsed and the time reported by
the RTC is in a 10% scale, which prevents the RTC from being useful at all.

Fortunately, the external oscillator is reported to be mandatory in the
Allwinner datasheet, so we can just switch to it.

Cc: stable at vger.kernel.org
Fixes: 9765d2d94309 ("rtc: sun6i: Add sun6i RTC driver")
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/rtc/rtc-sun6i.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/rtc/rtc-sun6i.c b/drivers/rtc/rtc-sun6i.c
index c169a2cd4727..dee524cfa13b 100644
--- a/drivers/rtc/rtc-sun6i.c
+++ b/drivers/rtc/rtc-sun6i.c
@@ -37,9 +37,11 @@
 
 /* Control register */
 #define SUN6I_LOSC_CTRL				0x0000
+#define SUN6I_LOSC_CTRL_KEY			(0x16aa << 16)
 #define SUN6I_LOSC_CTRL_ALM_DHMS_ACC		BIT(9)
 #define SUN6I_LOSC_CTRL_RTC_HMS_ACC		BIT(8)
 #define SUN6I_LOSC_CTRL_RTC_YMD_ACC		BIT(7)
+#define SUN6I_LOSC_CTRL_EXT_OSC			BIT(0)
 #define SUN6I_LOSC_CTRL_ACC_MASK		GENMASK(9, 7)
 
 /* RTC */
@@ -404,6 +406,10 @@ static int sun6i_rtc_probe(struct platform_device *pdev)
 	/* disable alarm wakeup */
 	writel(0, chip->base + SUN6I_ALARM_CONFIG);
 
+	/* switch to the external, more precise, oscillator */
+	writel(SUN6I_LOSC_CTRL_KEY | SUN6I_LOSC_CTRL_EXT_OSC,
+	       chip->base + SUN6I_LOSC_CTRL);
+
 	chip->rtc = rtc_device_register("rtc-sun6i", &pdev->dev,
 					&sun6i_rtc_ops, THIS_MODULE);
 	if (IS_ERR(chip->rtc)) {
-- 
2.11.0

^ permalink raw reply related

* [PATCH v7 15/15] irqchip: mbigen: Add ACPI support
From: Lorenzo Pieralisi @ 2017-01-16 15:24 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <587CD754.1050808@huawei.com>

On Mon, Jan 16, 2017 at 10:23:16PM +0800, Hanjun Guo wrote:
> Hi Lorenzo,
> 
> On 2017/1/16 19:38, Lorenzo Pieralisi wrote:
> > On Sat, Jan 14, 2017 at 10:56:54AM +0800, Hanjun Guo wrote:
> >> Hi Lorenzo,
> >>
> >> On 2017/1/13 18:21, Lorenzo Pieralisi wrote:
> >>> On Wed, Jan 11, 2017 at 11:06:39PM +0800, Hanjun Guo wrote:
> >>>> With the preparation of platform msi support and interrupt producer
> >>>> in DSDT, we can add mbigen ACPI support now.
> >>>>
> >>>> We are using _PRS methd to indicate number of irq pins instead
> >>>> of num_pins in DT to avoid _DSD usage in this case.
> >>>>
> >>>> For mbi-gen,
> >>>>     Device(MBI0) {
> >>>>           Name(_HID, "HISI0152")
> >>>>           Name(_UID, Zero)
> >>>>           Name(_CRS, ResourceTemplate() {
> >>>>                   Memory32Fixed(ReadWrite, 0xa0080000, 0x10000)
> >>>>           })
> >>>>
> >>>>           Name (_PRS, ResourceTemplate() {
> >>>> 		  Interrupt(ResourceProducer,...) {12,14,....}
> >>> I still do not understand why you are using _PRS for this, I think
> >>> the MBIgen configuration is static and if it is so the Interrupt
> >>> resource should be part of the _CRS unless there is something I am
> >>> missing here.
> >> Sorry for not clear in the commit message. MBIgen is an interrupt producer
> >> which produces irq resource to devices connecting to it, and MBIgen itself
> >> don't consume wired interrupts.
> > That's why you mark it as ResourceProducer, but that's not a reason to
> > put it in the _PRS instead of _CRS.
> 
> If using _CRS for the interrupt resource, the irq number represented
> will be mapped (i.e acpi_register_gsi()), then will conflict with the
> irq number of devices consuming it (mbigen is producing the
> interrupts), but I agree with you that let's ask Rafael's point of
> view.

Aha ! So here is why you are using _PRS because the kernel turns _CRS
Interrupt resources (even producers) into GSIs which is probably a
kernel bug, is that the reason ?

We don't abuse firmware bindings to make the kernel work, that's _never_
a good idea.

If the interrupt resource is a Resource Producer core ACPI should not
register the IRQ because that's not a GSI, probably this should be part of
Agustin changes too ?

> > IIUC _PRS is there to provide a way to define the possible resource
> > settings of a _configurable_ device (ie programmable) so that the actual
> > resource value you would programme with a call to its _SRS is sane (ie
> > the OS has a way, through the _PRS, to detect what possible resource
> > settings are available for the device).
> >
> > I think Rafael has more insights into how the _PRS is used on x86
> > systems so I would ask his point of view here before merrily merging
> > this code.
> 
> OK, Rafael is traveling now, hope he will have time to take a look.
> 
> How about updating this patch set then sending a new version for review
> with this patch unchanged? if Rafael have comments on this one, I will
> send a single updated one for this patch (if no other changes).

I think this patch (and the FW that goes with it) is wrong, but the rest
of the series, in particular the IORT bits, are ok with me.

Lorenzo

^ permalink raw reply

* [PATCH v1 3/3] watchdog: zx2967: add watchdog controller driver for ZTE's zx2967 family
From: Jun Nie @ 2017-01-16 15:25 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1484540395-3335-3-git-send-email-baoyou.xie@linaro.org>

On 2017?01?16? 12:19, Baoyou Xie wrote:
> This patch adds watchdog controller driver for ZTE's zx2967 family.
>
> Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>
> ---
>  drivers/watchdog/Kconfig      |  10 ++
>  drivers/watchdog/Makefile     |   1 +
>  drivers/watchdog/zx2967_wdt.c | 405 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 416 insertions(+)
>  create mode 100644 drivers/watchdog/zx2967_wdt.c
>
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 3eb58cb..79027da 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -714,6 +714,16 @@ config ASPEED_WATCHDOG
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called aspeed_wdt.
>
> +config ZX2967_WATCHDOG
> +	tristate "ZTE zx2967 SoCs watchdog support"
> +	depends on ARCH_ZX
> +	select WATCHDOG_CORE
> +	help
> +	  Say Y here to include support for the watchdog timer
> +	  in ZTE zx2967 SoCs.
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called zx2967_wdt.
> +
>  # AVR32 Architecture
>
>  config AT32AP700X_WDT
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index caa9f4a..ea08925 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -82,6 +82,7 @@ obj-$(CONFIG_BCM7038_WDT) += bcm7038_wdt.o
>  obj-$(CONFIG_ATLAS7_WATCHDOG) += atlas7_wdt.o
>  obj-$(CONFIG_RENESAS_WDT) += renesas_wdt.o
>  obj-$(CONFIG_ASPEED_WATCHDOG) += aspeed_wdt.o
> +obj-$(CONFIG_ZX2967_WATCHDOG) += zx2967_wdt.o
>
>  # AVR32 Architecture
>  obj-$(CONFIG_AT32AP700X_WDT) += at32ap700x_wdt.o
> diff --git a/drivers/watchdog/zx2967_wdt.c b/drivers/watchdog/zx2967_wdt.c
> new file mode 100644
> index 0000000..8791dd2
> --- /dev/null
> +++ b/drivers/watchdog/zx2967_wdt.c
> @@ -0,0 +1,405 @@
> +/*
> + * watchdog driver for ZTE's zx2967 family
> + *
> + * Copyright (C) 2017 ZTE Ltd.
> + *
> + * Author: Baoyou Xie <baoyou.xie@linaro.org>
> + *
> + * License terms: GNU General Public License (GPL) version 2
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/platform_device.h>
> +#include <linux/reboot.h>
> +#include <linux/reset.h>
> +#include <linux/watchdog.h>
> +
> +#define ZX2967_WDT_CFG_REG			0x4
> +#define ZX2967_WDT_LOAD_REG			0x8
> +#define ZX2967_WDT_REFRESH_REG			0x18
> +#define ZX2967_WDT_START_REG			0x1c
> +
> +#define ZX2967_WDT_REFRESH_MASK			0x3f
> +
> +#define ZX2967_WDT_CFG_DIV(n)			((((n)&0xff) - 1) << 8)
> +#define ZX2967_WDT_START_EN			0x1
> +
> +#define ZX2967_WDT_WRITEKEY			0x12340000
> +
> +#define ZX2967_WDT_DIV_DEFAULT			16
> +#define ZX2967_WDT_DEFAULT_TIMEOUT		32
> +#define ZX2967_WDT_MIN_TIMEOUT			1
> +#define ZX2967_WDT_MAX_TIMEOUT			500
> +#define ZX2967_WDT_MAX_COUNT			0xffff
> +
> +#define ZX2967_WDT_FLAG_REBOOT_MON		(1 << 0)
> +
> +#define ZX2967_RESET_MASK_REG			0xb0
> +
> +#define zx2967_wdt_write_reg(v, r) \
> +			writel((v) | ZX2967_WDT_WRITEKEY, r)
> +#define zx2967_wdt_read_reg(r)			readl(r)

For writel and readl, *_relaxed is recommended.

> +
> +static bool nowayout = WATCHDOG_NOWAYOUT;
> +static unsigned int timeout = ZX2967_WDT_DEFAULT_TIMEOUT;
> +
> +struct zx2967_wdt {
> +	struct device		*dev;
> +	struct clk		*clock;
> +	void __iomem		*reg_base;
> +	unsigned int		conf;
> +	unsigned int		load;
> +	unsigned int		flags;
> +	spinlock_t		lock;
> +	struct watchdog_device	wdt_device;
> +	struct notifier_block	restart_handler;
> +	struct notifier_block	reboot_handler;
> +};
> +
> +static void zx2967_wdt_refresh(struct zx2967_wdt *wdt)
> +{
> +	unsigned int val;
> +
> +	spin_lock(&wdt->lock);
> +
> +	val = zx2967_wdt_read_reg(wdt->reg_base + ZX2967_WDT_REFRESH_REG);
> +	val ^= ZX2967_WDT_REFRESH_MASK;
> +	zx2967_wdt_write_reg(val, wdt->reg_base + ZX2967_WDT_REFRESH_REG);
> +
> +	spin_unlock(&wdt->lock);
> +}
> +
> +static void __zx2967_wdt_stop(struct zx2967_wdt *wdt)
> +{
> +	unsigned int val;
> +
> +	spin_lock(&wdt->lock);
> +
> +	val = zx2967_wdt_read_reg(wdt->reg_base + ZX2967_WDT_START_REG);
> +	val &= ~(ZX2967_WDT_START_EN);
> +	zx2967_wdt_write_reg(val, wdt->reg_base + ZX2967_WDT_START_REG);
> +
> +	spin_unlock(&wdt->lock);
> +}
> +
> +static void __zx2967_wdt_start(struct zx2967_wdt *wdt)
> +{
> +	unsigned int val;
> +
> +	spin_lock(&wdt->lock);
> +
> +	val = zx2967_wdt_read_reg(wdt->reg_base + ZX2967_WDT_START_REG);
> +	val |= ZX2967_WDT_START_EN;
> +	zx2967_wdt_write_reg(val, wdt->reg_base + ZX2967_WDT_START_REG);
> +
> +	spin_unlock(&wdt->lock);
> +}
> +
> +static unsigned int
> +__zx2967_wdt_set_timeout(struct zx2967_wdt *wdt, unsigned int timeout)
> +{
> +	unsigned int freq = clk_get_rate(wdt->clock);
> +	unsigned int divisor = ZX2967_WDT_DIV_DEFAULT, count;
> +
> +	count = timeout * freq;
> +	if (count > divisor * ZX2967_WDT_MAX_COUNT)
> +		divisor = DIV_ROUND_UP(count, ZX2967_WDT_MAX_COUNT);
> +	count = DIV_ROUND_UP(count, divisor);
> +	zx2967_wdt_write_reg(ZX2967_WDT_CFG_DIV(divisor),
> +			     wdt->reg_base + ZX2967_WDT_CFG_REG);
> +	zx2967_wdt_write_reg(count, wdt->reg_base + ZX2967_WDT_LOAD_REG);

No spin lock here. Do we really need the spin lock?

> +	zx2967_wdt_refresh(wdt);
> +
> +	wdt->load = count;
> +	dev_info(wdt->dev, "count=%d, timeout=%d, divisor=%d\n",
> +		 count, timeout, divisor);
> +
> +	return (count * divisor) / freq;
> +}
> +
> +static int zx2967_wdt_set_timeout(struct watchdog_device *wdd,
> +				  unsigned int timeout)
> +{
> +	struct zx2967_wdt *wdt = watchdog_get_drvdata(wdd);
> +
> +	if (watchdog_timeout_invalid(&wdt->wdt_device, timeout)) {
> +		dev_err(wdt->dev, "timeout %d is invalid\n", timeout);
> +
> +		return -EINVAL;
> +	}
> +
> +	wdd->timeout = __zx2967_wdt_set_timeout(wdt, timeout);
> +
> +	return 0;
> +}
> +
> +static int zx2967_wdt_start(struct watchdog_device *wdd)
> +{
> +	struct zx2967_wdt *wdt = watchdog_get_drvdata(wdd);
> +
> +	__zx2967_wdt_stop(wdt);
> +	zx2967_wdt_set_timeout(wdd, wdd->timeout);
> +	__zx2967_wdt_start(wdt);
> +
> +	return 0;
> +}
> +
> +static int zx2967_wdt_stop(struct watchdog_device *wdd)
> +{
> +	struct zx2967_wdt *wdt = watchdog_get_drvdata(wdd);
> +
> +	__zx2967_wdt_stop(wdt);
> +
> +	return 0;
> +}
> +
> +static int zx2967_wdt_keepalive(struct watchdog_device *wdd)
> +{
> +	struct zx2967_wdt *wdt = watchdog_get_drvdata(wdd);
> +
> +	zx2967_wdt_refresh(wdt);
> +
> +	return 0;
> +}
> +
> +#define ZX2967_WDT_OPTIONS \
> +	(WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE)
> +static const struct watchdog_info zx2967_wdt_ident = {
> +	.options          =     ZX2967_WDT_OPTIONS,
> +	.firmware_version =	0,
> +	.identity         =	"zx2967 watchdog",
> +};
> +
> +static struct watchdog_ops zx2967_wdt_ops = {
> +	.owner = THIS_MODULE,
> +	.start = zx2967_wdt_start,
> +	.stop = zx2967_wdt_stop,
> +	.ping = zx2967_wdt_keepalive,
> +	.set_timeout = zx2967_wdt_set_timeout,
> +};
> +
> +static void zx2967_wdt_fix_sysdown(struct zx2967_wdt *wdt)
> +{
> +	__zx2967_wdt_stop(wdt);
> +	__zx2967_wdt_set_timeout(wdt, 15);
> +	__zx2967_wdt_start(wdt);
> +}
> +
> +static int zx2967_wdt_notify_sys(struct notifier_block *this,
> +			     unsigned long code, void *unused)
> +{
> +	struct zx2967_wdt *wdt = container_of(this, struct zx2967_wdt,
> +					      reboot_handler);
> +
> +	wdt->flags |= ZX2967_WDT_FLAG_REBOOT_MON;
> +	switch (code) {
> +	case SYS_HALT:
> +	case SYS_POWER_OFF:
> +	case SYS_RESTART:
> +		zx2967_wdt_fix_sysdown(wdt);
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	return 0;
> +}
> +
> +static int zx2967_wdt_restart(struct notifier_block *this,
> +			      unsigned long mode, void *cmd)
> +{
> +	struct zx2967_wdt *wdt;
> +
> +	wdt = container_of(this, struct zx2967_wdt, restart_handler);
> +
> +	zx2967_wdt_stop(&wdt->wdt_device);
> +
> +	zx2967_wdt_write_reg(0x80, wdt->reg_base + ZX2967_WDT_LOAD_REG);
> +	zx2967_wdt_refresh(wdt);
> +	zx2967_wdt_write_reg(ZX2967_WDT_START_EN,
> +			     wdt->reg_base + ZX2967_WDT_START_REG);
> +
> +	zx2967_wdt_start(&wdt->wdt_device);
> +	/* wait for reset*/
> +	mdelay(500);
> +
> +	return NOTIFY_DONE;
> +}
> +
> +static void zx2967_reset_mask_config(struct device *dev)
> +{
> +	struct device_node *np = NULL;
> +	void __iomem *reg;
> +	unsigned int  val, mask, config, size;
> +	const unsigned int *prop;
> +
> +	prop = of_get_property(dev->of_node, "reset-mask-config", &size);
> +	if (size < (sizeof(*prop) * 2)) {
> +		dev_err(dev, "bad data for reset-mask-config");
> +		return;
> +	}
> +	config = be32_to_cpup(prop++);
> +	mask = be32_to_cpup(prop);
> +	np = of_find_compatible_node(NULL, NULL, "zte,aon-sysctrl");
> +	if (!np) {
> +		dev_err(dev, "Cannot found pcu device node\n");
> +		return;
> +	}
> +	reg = of_iomap(np, 0) + ZX2967_RESET_MASK_REG;
> +	of_node_put(np);
> +
> +	val = readl(reg);
> +	val &= ~mask;
> +	val |= config;
> +	writel(val, reg);
> +}
> +
> +static int zx2967_wdt_probe(struct platform_device *pdev)
> +{
> +	struct device *dev;
> +	struct zx2967_wdt *wdt;
> +	struct resource *base;
> +	int err, ret = 0;
> +	unsigned int rate, val;
> +
> +	struct reset_control *rstc;
> +
> +	dev = &pdev->dev;
> +
> +	wdt = devm_kzalloc(dev, sizeof(*wdt), GFP_KERNEL);
> +	if (!wdt) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +
> +	platform_set_drvdata(pdev, wdt);
> +
> +	wdt->dev = dev;
> +	spin_lock_init(&wdt->lock);
> +
> +	wdt->wdt_device.info = &zx2967_wdt_ident;
> +	wdt->wdt_device.ops = &zx2967_wdt_ops;
> +	wdt->wdt_device.timeout = ZX2967_WDT_DEFAULT_TIMEOUT;
> +	wdt->wdt_device.max_timeout = ZX2967_WDT_MAX_TIMEOUT;
> +	wdt->wdt_device.min_timeout = ZX2967_WDT_MIN_TIMEOUT;
> +	wdt->wdt_device.parent = &pdev->dev;
> +
> +	base = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	wdt->reg_base = devm_ioremap_resource(dev, base);
> +
> +	if (of_find_property(dev->of_node, "reset-mask-config", NULL))
> +		zx2967_reset_mask_config(dev);
> +
> +	wdt->reboot_handler.notifier_call = zx2967_wdt_notify_sys;
> +	ret = register_reboot_notifier(&wdt->reboot_handler);
> +	wdt->clock = devm_clk_get(dev, "wdtclk");
> +	if (IS_ERR(wdt->clock)) {
> +		dev_err(dev, "failed to find watchdog clock source\n");
> +		ret = PTR_ERR(wdt->clock);
> +		goto out;
> +	}
> +	ret = clk_prepare_enable(wdt->clock);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to enable clock\n");
> +		goto out;
> +	}
> +
> +	rate = clk_get_rate(wdt->clock);
> +	if (rate == 24000000)
> +		ret = clk_set_rate(wdt->clock, 32768);
> +	rate = clk_get_rate(wdt->clock);

Do you want to check rate again? Or you want to just check set_rate 
return value?

> +
> +	rstc = devm_reset_control_get(dev, "wdtrst");
> +	if (!rstc) {
> +		dev_info(dev, "rstc get failed");

dev_err() may be more feasible.

> +	} else {
> +		reset_control_assert(rstc);
> +		mdelay(10);
> +		reset_control_deassert(rstc);
> +	}
> +
> +	watchdog_set_drvdata(&wdt->wdt_device, wdt);
> +
> +	watchdog_init_timeout(&wdt->wdt_device, timeout, &pdev->dev);
> +	watchdog_set_nowayout(&wdt->wdt_device, nowayout);
> +
> +	zx2967_wdt_stop(&wdt->wdt_device);
> +
> +	err = watchdog_register_device(&wdt->wdt_device);
> +	if (unlikely(err)) {
> +		ret = err;
> +		goto fail_register;
> +	}
> +
> +	wdt->restart_handler.notifier_call = zx2967_wdt_restart;
> +	wdt->restart_handler.priority = 128;
> +	ret = register_restart_handler(&wdt->restart_handler);
> +	if (ret) {
> +		pr_err("cannot register restart handler, %d\n", ret);
> +		goto fail_restart;
> +	}
> +
> +	val = zx2967_wdt_read_reg(wdt->reg_base + ZX2967_WDT_START_REG);
> +	dev_info(&pdev->dev, "watchdog enabled (timeout=%d sec, nowayout=%d)",
> +		 wdt->wdt_device.timeout, nowayout);
> +
> +	return 0;
> +
> +fail_restart:
> +	watchdog_unregister_device(&wdt->wdt_device);
> +fail_register:
> +	clk_disable_unprepare(wdt->clock);
> +out:
> +	return ret;
> +}
> +
> +static int zx2967_wdt_remove(struct platform_device *pdev)
> +{
> +	struct zx2967_wdt *wdt = platform_get_drvdata(pdev);
> +
> +	unregister_restart_handler(&wdt->restart_handler);
> +	watchdog_unregister_device(&wdt->wdt_device);
> +	clk_disable_unprepare(wdt->clock);
> +
> +	return 0;
> +}
> +
> +static void zx2967_wdt_shutdown(struct platform_device *pdev)
> +{
> +	struct zx2967_wdt *wdt = platform_get_drvdata(pdev);
> +
> +	if (!(wdt->flags & ZX2967_WDT_FLAG_REBOOT_MON))
> +		zx2967_wdt_stop(&wdt->wdt_device);
> +}
> +
> +static const struct of_device_id zx2967_wdt_match[] = {
> +	{ .compatible = "zte,zx296718-wdt", },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, zx2967_wdt_match);
> +
> +static const struct platform_device_id zx2967_wdt_ids[] = {
> +	{ .name = "zx2967-wdt", },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(platform, zx2967_wdt_ids);
> +
> +static struct platform_driver zx2967_wdt_driver = {
> +	.probe		= zx2967_wdt_probe,
> +	.remove		= zx2967_wdt_remove,
> +	.shutdown	= zx2967_wdt_shutdown,
> +	.id_table	= zx2967_wdt_ids,
> +	.driver		= {
> +		.name	= "zx2967-wdt",
> +		.of_match_table	= of_match_ptr(zx2967_wdt_match),
> +	},
> +};
> +module_platform_driver(zx2967_wdt_driver);
> +
> +MODULE_AUTHOR("Baoyou Xie <baoyou.xie@linaro.org>");
> +MODULE_DESCRIPTION("ZTE zx2967 Watchdog Device Driver");
> +MODULE_LICENSE("GPL");
>

^ permalink raw reply

* [PATCH 5/8] efi: Get the secure boot status [ver #6]
From: David Howells @ 2017-01-16 15:39 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170116144954.GB27351@codeblueprint.co.uk>

Matt Fleming <matt@codeblueprint.co.uk> wrote:

> On Wed, 11 Jan, at 03:27:23PM, David Howells wrote:
> > Matt Fleming <matt@codeblueprint.co.uk> wrote:
> > 
> > > > +	movb	$0, BP_secure_boot(%rsi)
> > > >  #ifdef CONFIG_EFI_STUB
> > > >  	/*
> > > >  	 * The entry point for the PE/COFF executable is efi_pe_entry, so
> > > 
> > > Is clearing ::secure_boot really necessary? Any code path that goes
> > > via efi_main() will set it correctly and all other code paths should
> > > get it cleared in sanitize_boot_params(), no?
> > 
> > No.
> > 
> > The boot_params->secure_boot parameter exists whether or not efi_main() is
> > traversed (ie. if EFI isn't enabled or CONFIG_EFI_STUB=n) and, if not cleared,
> > is of uncertain value.
> >
> > Further, sanitize_boot_params() has to be modified by this patch so as not to
> > clobber the secure_boot flag.
> 
> Any new parameters that boot loaders do not know about should be
> cleared to zero by default in the boot loader because boot_params
> itself should be zero'd when allocated.

Do you mean the boot loader or the boot wrapper?  If the loader, that is
outside my control - and given the purpose of the value, I'm not sure I
want to rely on that.

> There are two cases to consider:
> 
>  1) boot_params is not zero'd
>  2) boot_params is zero'd
> 
> 1) This is a broken boot loader implementation that violates the x86
> boot specification and I would never expect ->secure_boot to have a
> valid value.

If there's a boot specification that must be complied with, why does
sanitize_boot_params() even exist?  Why does the comment on it say:

 * Deal with bootloaders which fail to initialize unknown fields in
 * boot_params to zero.  The list fields in this list are taken from
 * analysis of kexec-tools; if other broken bootloaders initialize a
 * different set of fields we will need to figure out how to disambiguate.

> It should not be special-cased in sanitize_boot_params(), it should be
> zero'd.

Sigh.  sanitize_boot_params() is part of the problem.  The startup sequence
goes something like this:

 (0) We enter the boot wrapper.

 (1) We clear the secure-boot status value [my patch adds this].

 (2) The boot wrapper *may* invoke efi_main() - which will determine the
     secure-boot status.

 (3) The boot wrapper calls extract_kernel() to decompress the kernel.

 (4) extract_kernel() calls sanitize_boot_params() which would otherwise clear
     the secure-boot flag.

 (5) The boot wrapper jumps into the main kernel image, which now does not see
     the secure boot status value we calculated.

So, no, sanitize_boot_params() must *not* zero the value unless we change the
call point for s_b_p().

> 2) In this case ->secure_boot should be zero unless modified inside of
> efi_main().

I have no idea whether this is guaranteed or not.

> Did you hit the scenario where ->secure_boot has a garbage value while
> developing these patches? I wouldn't expect to see it in practice.

I haven't actually checked what the value was before I cleared it.  But, I've
found that security people get seriously paranoid about assuming things to be
implicitly so;-).

David

^ permalink raw reply

* [PATCH] rtc: sun6i: Switch to the external oscillator
From: Alexandre Belloni @ 2017-01-16 15:39 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170116152148.27439-1-maxime.ripard@free-electrons.com>

On 16/01/2017 at 16:21:48 +0100, Maxime Ripard wrote :
> The RTC is clocked from either an internal, imprecise, oscillator or an
> external one, which is usually much more accurate.
> 
> The difference perceived between the time elapsed and the time reported by
> the RTC is in a 10% scale, which prevents the RTC from being useful at all.
> 
> Fortunately, the external oscillator is reported to be mandatory in the
> Allwinner datasheet, so we can just switch to it.
> 

Still, I'm wondering whether the external clock should be taken.

We've had issues with at91 and tegra where this external clock was
suddenly able to be stopped, breaking the RTC because the CCF was not
aware the RTC was using it.

See:
http://patchwork.ozlabs.org/patch/502459/
http://patchwork.ozlabs.org/patch/714517/

Your reply can be that you don't care now and this has a low probability
and you'll handle the case when it happens and that will be fine.

> Cc: stable at vger.kernel.org
> Fixes: 9765d2d94309 ("rtc: sun6i: Add sun6i RTC driver")
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  drivers/rtc/rtc-sun6i.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/rtc/rtc-sun6i.c b/drivers/rtc/rtc-sun6i.c
> index c169a2cd4727..dee524cfa13b 100644
> --- a/drivers/rtc/rtc-sun6i.c
> +++ b/drivers/rtc/rtc-sun6i.c
> @@ -37,9 +37,11 @@
>  
>  /* Control register */
>  #define SUN6I_LOSC_CTRL				0x0000
> +#define SUN6I_LOSC_CTRL_KEY			(0x16aa << 16)
>  #define SUN6I_LOSC_CTRL_ALM_DHMS_ACC		BIT(9)
>  #define SUN6I_LOSC_CTRL_RTC_HMS_ACC		BIT(8)
>  #define SUN6I_LOSC_CTRL_RTC_YMD_ACC		BIT(7)
> +#define SUN6I_LOSC_CTRL_EXT_OSC			BIT(0)
>  #define SUN6I_LOSC_CTRL_ACC_MASK		GENMASK(9, 7)
>  
>  /* RTC */
> @@ -404,6 +406,10 @@ static int sun6i_rtc_probe(struct platform_device *pdev)
>  	/* disable alarm wakeup */
>  	writel(0, chip->base + SUN6I_ALARM_CONFIG);
>  
> +	/* switch to the external, more precise, oscillator */
> +	writel(SUN6I_LOSC_CTRL_KEY | SUN6I_LOSC_CTRL_EXT_OSC,
> +	       chip->base + SUN6I_LOSC_CTRL);
> +
>  	chip->rtc = rtc_device_register("rtc-sun6i", &pdev->dev,
>  					&sun6i_rtc_ops, THIS_MODULE);
>  	if (IS_ERR(chip->rtc)) {
> -- 
> 2.11.0
> 

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply

* [PATCH 8/8] efi: Add EFI_SECURE_BOOT bit [ver #6]
From: David Howells @ 2017-01-16 15:40 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170116134041.GA27351@codeblueprint.co.uk>

Matt Fleming <matt@codeblueprint.co.uk> wrote:

> > > Before we add more efi.flags bits I'd like this series to include the
> > > patch that makes use of EFI_SECURE_BOOT. Alternatively, you move this
> > > last patch to a new series.
> > 
> > Are you willing to take the kernel lock-down patches also?
> 
> I'm happy to take them through the EFI tree provided that they've been
> Reviewed/Ack-ed by the security folks.

Thinking further about it, it might be best to push the EFI_SECURE_BOOT flag
into a later series and exclude that patch from this one.

David

^ permalink raw reply

* [v2 2/3] ARM: dts: STM32 Add USB FS host mode support
From: Bruno Herrera @ 2017-01-16 15:41 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <21fbf365-7d15-04be-f6a5-451aedac05b5@st.com>

On Mon, Jan 16, 2017 at 9:47 AM, Alexandre Torgue
<alexandre.torgue@st.com> wrote:
>
>
> On 01/16/2017 11:26 AM, Bruno Herrera wrote:
>>
>> Hi Alex,
>>
>> On Mon, Jan 16, 2017 at 6:57 AM, Alexandre Torgue
>> <alexandre.torgue@st.com> wrote:
>>>
>>> Hi Bruno,
>>>
>>> On 01/16/2017 03:09 AM, Bruno Herrera wrote:
>>>>
>>>>
>>>> This patch adds the USB pins and nodes for USB HS/FS cores working at FS
>>>> speed,
>>>> using embedded PHY.
>>>>
>>>> Signed-off-by: Bruno Herrera <bruherrera@gmail.com>
>>>
>>>
>>>
>>> Sorry, but what is patch 1 & pacth 3 status ?
>>
>>
>> My bad, I'll add the status of the patch series version 3.
>>>
>>>
>>> For this one, can split it in 3 patches (one patch for SOC and one for
>>> each
>>> board) please.
>>>
>>
>> No problem.
>>>
>>>
>>>
>>>> ---
>>>>  arch/arm/boot/dts/stm32f429-disco.dts | 30
>>>> ++++++++++++++++++++++++++++++
>>>>  arch/arm/boot/dts/stm32f429.dtsi      | 35
>>>> ++++++++++++++++++++++++++++++++++-
>>>>  arch/arm/boot/dts/stm32f469-disco.dts | 30
>>>> ++++++++++++++++++++++++++++++
>>>>  3 files changed, 94 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/arm/boot/dts/stm32f429-disco.dts
>>>> b/arch/arm/boot/dts/stm32f429-disco.dts
>>>> index 7d0415e..374c5ed 100644
>>>> --- a/arch/arm/boot/dts/stm32f429-disco.dts
>>>> +++ b/arch/arm/boot/dts/stm32f429-disco.dts
>>>> @@ -88,6 +88,16 @@
>>>>                         gpios = <&gpioa 0 0>;
>>>>                 };
>>>>         };
>>>> +
>>>> +       /* This turns on vbus for otg for host mode (dwc2) */
>>>> +       vcc5v_otg: vcc5v-otg-regulator {
>>>> +               compatible = "regulator-fixed";
>>>> +               gpio = <&gpioc 4 0>;
>>>> +               pinctrl-names = "default";
>>>> +               pinctrl-0 = <&usbotg_pwren_h>;
>>>> +               regulator-name = "vcc5_host1";
>>>> +               regulator-always-on;
>>>> +       };
>>>>  };
>>>>
>>>>  &clk_hse {
>>>> @@ -99,3 +109,23 @@
>>>>         pinctrl-names = "default";
>>>>         status = "okay";
>>>>  };
>>>> +
>>>> +&usbotg_hs {
>>>> +       compatible = "st,stm32-fsotg", "snps,dwc2";
>>>> +       dr_mode = "host";
>>>> +       pinctrl-0 = <&usbotg_fs_pins_b>;
>>>> +       pinctrl-names = "default";
>>>> +       status = "okay";
>>>> +};
>>>> +
>>>> +&pinctrl {
>>>> +       usb-host {
>>>> +               usbotg_pwren_h: usbotg-pwren-h {
>>>> +                       pins {
>>>> +                               pinmux = <STM32F429_PC4_FUNC_GPIO>;
>>>> +                               bias-disable;
>>>> +                               drive-push-pull;
>>>> +                       };
>>>> +               };
>>>> +       };
>>>> +};
>>>
>>>
>>>
>>> Pinctrl muxing has to be defined/declared in stm32f429.dtsi
>>>
>> This is board specific logic and it vary from board to board, should
>> it be defined here?
>
>
> Pinmuxing definition is a SOC part (as it is a possibility offered by SOC).
> Pinmuxing choice is board specific.
>
> Regarding your code, it should not boot. Ex for disco:
>
>  +               gpio = <&gpiob 2 0>;
>>>> +               pinctrl-names = "default";
>>>> +               pinctrl-0 = <&usbotg_pwren_h>;
>
> +
>
>   usb-host {
>>>> +               usbotg_pwren_h: usbotg-pwren-h {
>>>> +                       pins {
>>>> +                               pinmux = <STM32F429_PB2_FUNC_GPIO>;
>
> Indeed, you are declaring two time the pin PB2 (one time through pinctrl and
> one other time through gpiolib). in strict mode you can't request 2 times
> the same Pin.
> I assume that your driver want controls this GPIO (request/set direction /
> set, get value ...). in this case you only need to declare this part:
>
> gpio = <&gpiob 2 0>;
>
> The GPIO lib will deal with pinctrl framework for you.
> And in this case, yes gpio declaration is board specific so this part will
> be in board file.
>
> Let me know, if I'm not enough clear.

Thats very clear! Thanks for bringing.
I'll retest without the pinctrl.

br,
Bruno

>
> Regards
> Alex
>
>
>
>
>
>>>
>>>
>>>> diff --git a/arch/arm/boot/dts/stm32f429.dtsi
>>>> b/arch/arm/boot/dts/stm32f429.dtsi
>>>> index e4dae0e..bc07aa8 100644
>>>> --- a/arch/arm/boot/dts/stm32f429.dtsi
>>>> +++ b/arch/arm/boot/dts/stm32f429.dtsi
>>>> @@ -206,7 +206,7 @@
>>>>                         reg = <0x40007000 0x400>;
>>>>                 };
>>>>
>>>> -               pin-controller {
>>>> +               pinctrl: pin-controller {
>>>>                         #address-cells = <1>;
>>>>                         #size-cells = <1>;
>>>>                         compatible = "st,stm32f429-pinctrl";
>>>> @@ -316,6 +316,30 @@
>>>>                                 };
>>>>                         };
>>>>
>>>> +                       usbotg_fs_pins_a: usbotg_fs at 0 {
>>>> +                               pins {
>>>> +                                       pinmux =
>>>> <STM32F429_PA10_FUNC_OTG_FS_ID>,
>>>> +
>>>> <STM32F429_PA11_FUNC_OTG_FS_DM>,
>>>> +
>>>> <STM32F429_PA12_FUNC_OTG_FS_DP>;
>>>> +                                       bias-disable;
>>>> +                                       drive-push-pull;
>>>> +                                       slew-rate = <2>;
>>>> +                               };
>>>> +                       };
>>>> +
>>>> +                       usbotg_fs_pins_b: usbotg_fs at 1 {
>>>> +                               pins {
>>>> +                                       pinmux =
>>>> <STM32F429_PB12_FUNC_OTG_HS_ID>,
>>>> +
>>>> <STM32F429_PB14_FUNC_OTG_HS_DM>,
>>>> +
>>>> <STM32F429_PB15_FUNC_OTG_HS_DP>;
>>>> +                                       bias-disable;
>>>> +                                       drive-push-pull;
>>>> +                                       slew-rate = <2>;
>>>> +                               };
>>>> +                       };
>>>> +
>>>> +
>>>> +
>>>>                         usbotg_hs_pins_a: usbotg_hs at 0 {
>>>>                                 pins {
>>>>                                         pinmux =
>>>> <STM32F429_PH4_FUNC_OTG_HS_ULPI_NXT>,
>>>> @@ -420,6 +444,15 @@
>>>>                         status = "disabled";
>>>>                 };
>>>>
>>>> +               usbotg_fs: usb at 50000000 {
>>>> +                       compatible = "st,stm32f4xx-fsotg", "snps,dwc2";
>>>> +                       reg = <0x50000000 0x40000>;
>>>> +                       interrupts = <67>;
>>>> +                       clocks = <&rcc 0 39>;
>>>> +                       clock-names = "otg";
>>>> +                       status = "disabled";
>>>> +               };
>>>> +
>>>>                 rng: rng at 50060800 {
>>>>                         compatible = "st,stm32-rng";
>>>>                         reg = <0x50060800 0x400>;
>>>> diff --git a/arch/arm/boot/dts/stm32f469-disco.dts
>>>> b/arch/arm/boot/dts/stm32f469-disco.dts
>>>> index 8877c00..8ae6763 100644
>>>> --- a/arch/arm/boot/dts/stm32f469-disco.dts
>>>> +++ b/arch/arm/boot/dts/stm32f469-disco.dts
>>>> @@ -68,6 +68,17 @@
>>>>         soc {
>>>>                 dma-ranges = <0xc0000000 0x0 0x10000000>;
>>>>         };
>>>> +
>>>> +       /* This turns on vbus for otg for host mode (dwc2) */
>>>> +       vcc5v_otg: vcc5v-otg-regulator {
>>>> +               compatible = "regulator-fixed";
>>>> +               enable-active-high;
>>>> +               gpio = <&gpiob 2 0>;
>>>> +               pinctrl-names = "default";
>>>> +               pinctrl-0 = <&usbotg_pwren_h>;
>>>> +               regulator-name = "vcc5_host1";
>>>> +               regulator-always-on;
>>>> +       };
>>>>  };
>>>>
>>>>  &rcc {
>>>> @@ -81,3 +92,22 @@
>>>>  &usart3 {
>>>>         status = "okay";
>>>>  };
>>>> +
>>>> +&usbotg_fs {
>>>> +       dr_mode = "host";
>>>> +       pinctrl-0 = <&usbotg_fs_pins_a>;
>>>> +       pinctrl-names = "default";
>>>> +       status = "okay";
>>>> +};
>>>> +
>>>> +&pinctrl {
>>>> +       usb-host {
>>>> +               usbotg_pwren_h: usbotg-pwren-h {
>>>> +                       pins {
>>>> +                               pinmux = <STM32F429_PB2_FUNC_GPIO>;
>>>> +                               bias-disable;
>>>> +                               drive-push-pull;
>>>> +                       };
>>>> +               };
>>>> +       };
>>>> +};
>>>
>>>
>>> Same. Note that if you have 2 configuration for one feature (like it is
>>> here
>>> for "usbotg_pwren_h"), you could index it. Not that I'm adding a
>>> dedidacted
>>> pinctroller for stm32f469.
>>>
>> Sorry, but I dont know what you mean by index here.
>> The usbotg_pwren_h (VBUS ENABLE) is attached in different port/pins
>> for each board.
>>
>> Br.,
>>
>>
>>> Br
>>> Alex
>>>>
>>>>
>>>>
>>>
>>>
>

^ permalink raw reply

* [RFC PATCH 08/10] arm64/sve: ptrace: Wire up vector length control and reporting
From: Pedro Alves @ 2017-01-16 15:47 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170116151156.GI28060@E107787-LIN>

On 01/16/2017 03:11 PM, Yao Qi wrote:
> 
>> > 
>> > gdb must already re-detect the vector length on stop, since the target
>> > could have called the prctl() in the meantime.
> Yes, gdb assumes the vector length may be changed, so it re-detects on
> every stop, but I don't see the need for gdb to change the vector length.
> 

Do we need to consider inferior function calls here?

Say the program is stopped in code that assumes "vector length N", and
the user does "print some_function_that_assumes_some_other_vector_length ()".

Is that a use case we need to cover?

If so, to make it work correctly, the debugger needs to be able to change the
vector length to the length assumed by that called function, and then
restore it back after the call completes (or is aborted).

I have no idea whether the debugger will be able to figure
out a function's assumed vector length from debug info or some such.

Thanks,
Pedro Alves

^ permalink raw reply

* [GIT PULL] STM32 DT changes for v4.11 #1
From: Alexandre Torgue @ 2017-01-16 15:49 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Olof, Arnd and Kevin,

Please consider this first round of STM32 DT updates for v4.11:

The following changes since commit 7ce7d89f48834cefece7804d38fc5d85382edf77:

   Linux 4.10-rc1 (2016-12-25 16:13:08 -0800)

are available in the git repository at:

   git://git.kernel.org/pub/scm/linux/kernel/git/atorgue/stm32.git 
tags/stm32-dt-for-v4.11-1

for you to fetch changes up to d99c66406a846748e26989759caa7e966c1804dd:

   ARM: dts: stm32: enable RTC on stm32429i-eval (2017-01-16 14:38:14 +0100)

----------------------------------------------------------------
STM32 DT updates for v4.11, round 1.

Highlights:
----------

  - ADD RTC support on STM32F429 MCU
  - Enable RTC on STM32F469and STM32F429 boards
  - ADD ADC support on STM32F429 MCU
  - Enable ADC on STM32F429 Eval board
  - Add I2S external clock
  - Fix memory size for STM32F429 Disco

Note:
-----
First patch "clk: stm32f4: Update DT bindings documentation")
has already been merged in clock tree.

----------------------------------------------------------------
Amelie Delaunay (5):
       ARM: dts: stm32: set HSE_RTC clock frequency to 1 MHz on stm32f429
       ARM: dts: stm32: Add RTC support for STM32F429 MCU
       ARM: dts: stm32: enable RTC on stm32f429-disco
       ARM: dts: stm32: enable RTC on stm32f469-disco
       ARM: dts: stm32: enable RTC on stm32429i-eval

Bruno Herrera (1):
       ARM: dts: stm32: Fix memory size from 8MB to 16MB on 
stm32f469-disco board

Bruno Meirelles Herrera (1):
       ARM: dts: stm32: Add missing USART3 pin config to stm32f469-disco 
board

Fabrice GASNIER (2):
       ARM: dts: stm32: Add ADC support to stm32f429
       ARM: dts: stm32: enable ADC on stm32f429i-eval board

Gabriel Fernandez (3):
       clk: stm32f4: Update DT bindings documentation
       ARM: dts: stm32: Add external I2S clock on stm32f429 MCU
       ARM: dts: stm32: Include auxiliary stm32fx clock definition

^ permalink raw reply

* [PATCH 16/37] PCI: endpoint: Introduce configfs entry for configuring EP functions
From: Christoph Hellwig @ 2017-01-16 15:51 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <587C61B3.3070107@ti.com>

On Mon, Jan 16, 2017 at 11:31:23AM +0530, Kishon Vijay Abraham I wrote:
> Actually not all devices have hardcoded headers. E.g the platform I'm using
> doesn't have hardcoded headers and it can be configured based on the function
> the user would like to use. If the devices are hardcoded, then using configfs
> can be skipped altogether. In such cases, APIs like pci_epf_create() can
> directly be used by the drivers instead of going via configfs.

That's exactly what I meant - the IDs need to be set by the driver for
the implemented PCI device, and it's not up to the gadget core to configur
e them, it's up to the implementation of the PCIe device which PCI it
exposes.

^ permalink raw reply

* [PATCH v1 5/7] ARM: dts: stm32f429: Add ltdc support
From: Alexandre Torgue @ 2017-01-16 15:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1484573344-11609-6-git-send-email-yannick.fertre@st.com>

Hi,

On 01/16/2017 02:29 PM, Yannick Fertre wrote:
> Add LTDC (Lcd-tft Display Controller) support.
>
> Signed-off-by: Yannick Fertre <yannick.fertre@st.com>

Can you change commit header by: "ARM: dts: stm32: Add ltdc support on 
stm32f429 MCU" please

> ---
>  arch/arm/boot/dts/stm32f429.dtsi | 25 ++++++++++++++++++++++++-
>  1 file changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi
> index 336ee4f..fc43415 100644
> --- a/arch/arm/boot/dts/stm32f429.dtsi
> +++ b/arch/arm/boot/dts/stm32f429.dtsi
> @@ -185,7 +185,7 @@
>  			interrupts = <1>, <2>, <3>, <6>, <7>, <8>, <9>, <10>, <23>, <40>, <41>, <42>, <62>, <76>;
>  		};
>
> -		pin-controller {
> +		pinctrl: pin-controller {
>  			#address-cells = <1>;
>  			#size-cells = <1>;
>  			compatible = "st,stm32f429-pinctrl";
> @@ -404,6 +404,29 @@
>  			interrupts = <80>;
>  			clocks = <&rcc 0 38>;
>  		};
> +
> +		st-display-subsystem {
> +			compatible = "st,display-subsystem";
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			ranges;
> +			dma-ranges;
> +
> +			ltdc_host: stm32-ltdc at 40016800 {
> +				compatible = "st,ltdc";
> +				reg = <0x40016800 0x200>;
> +				interrupts = <88>, <89>;
> +				resets = <&rcc 314>;
> +				clocks = <&rcc 1 8>;
> +				clock-names = "clk-lcd";
> +				status = "disabled";
> +
> +				port {
> +					ltdc_out_rgb: endpoint {
> +					};
> +				};
> +			};
> +		};
>  	};
>  };
>
>

^ permalink raw reply

* [PATCH v1 6/7] ARM: dts: stm32429i-eval: Enable ltdc & simple panel on Eval board
From: Alexandre Torgue @ 2017-01-16 16:02 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1484573344-11609-7-git-send-email-yannick.fertre@st.com>

Hi

On 01/16/2017 02:29 PM, Yannick Fertre wrote:
> Enable ltdc & enable am-480272h3tmqw-t01h panel.
>
> Signed-off-by: Yannick Fertre <yannick.fertre@st.com>
> ---

Can you please change commit header with:

"ARM: dts: stm32: Enable ltdc & simple panel on stm32f429-Eval board"

>  arch/arm/boot/dts/stm32429i-eval.dts | 58 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 58 insertions(+)
>
> diff --git a/arch/arm/boot/dts/stm32429i-eval.dts b/arch/arm/boot/dts/stm32429i-eval.dts
> index 2de6487..f987ca5 100644
> --- a/arch/arm/boot/dts/stm32429i-eval.dts
> +++ b/arch/arm/boot/dts/stm32429i-eval.dts
> @@ -88,6 +88,52 @@
>  		clocks = <&rcc 0 30>;
>  		clock-names = "main_clk";
>  	};
> +
> +	panel_rgb: panel-rgb {
> +		compatible = "ampire,am-480272h3tmqw-t01h";
> +		status = "okay";
> +		port {
> +			panel_in_rgb: endpoint {
> +				remote-endpoint = <&ltdc_out_rgb>;
> +			};
> +		};
> +	};
> +};
> +
> +&pinctrl {
> +	pinctrl_ltdc: ltdc at 0 {
> +		pins {

Pinmuxing definition is SOC specific. Please move it in stm32f429.dtsi 
file. Note that a development is ongoing to define pinmuxing in a 
dedicated pinctrl-stm32f4.dtsi file.


> +			pinmux = <STM32F429_PI12_FUNC_LCD_HSYNC>,
> +				 <STM32F429_PI13_FUNC_LCD_VSYNC>,
> +				 <STM32F429_PI14_FUNC_LCD_CLK>,
> +				 <STM32F429_PI15_FUNC_LCD_R0>,
> +				 <STM32F429_PJ0_FUNC_LCD_R1>,
> +				 <STM32F429_PJ1_FUNC_LCD_R2>,
> +				 <STM32F429_PJ2_FUNC_LCD_R3>,
> +				 <STM32F429_PJ3_FUNC_LCD_R4>,
> +				 <STM32F429_PJ4_FUNC_LCD_R5>,
> +				 <STM32F429_PJ5_FUNC_LCD_R6>,
> +				 <STM32F429_PJ6_FUNC_LCD_R7>,
> +				 <STM32F429_PJ7_FUNC_LCD_G0>,
> +				 <STM32F429_PJ8_FUNC_LCD_G1>,
> +				 <STM32F429_PJ9_FUNC_LCD_G2>,
> +				 <STM32F429_PJ10_FUNC_LCD_G3>,
> +				 <STM32F429_PJ11_FUNC_LCD_G4>,
> +				 <STM32F429_PJ12_FUNC_LCD_B0>,
> +				 <STM32F429_PJ13_FUNC_LCD_B1>,
> +				 <STM32F429_PJ14_FUNC_LCD_B2>,
> +				 <STM32F429_PJ15_FUNC_LCD_B3>,
> +				 <STM32F429_PK0_FUNC_LCD_G5>,
> +				 <STM32F429_PK1_FUNC_LCD_G6>,
> +				 <STM32F429_PK2_FUNC_LCD_G7>,
> +				 <STM32F429_PK3_FUNC_LCD_B4>,
> +				 <STM32F429_PK4_FUNC_LCD_B5>,
> +				 <STM32F429_PK5_FUNC_LCD_B6>,
> +				 <STM32F429_PK6_FUNC_LCD_B7>,
> +				 <STM32F429_PK7_FUNC_LCD_DE>;
> +			slew-rate = <2>;
> +		};
> +	};
>  };
>
>  &clk_hse {
> @@ -123,3 +169,15 @@
>  	pinctrl-names = "default";
>  	status = "okay";
>  };
> +
> +&ltdc_host{
> +	status = "okay";
> +	pinctrl-0 = <&pinctrl_ltdc>;
> +	pinctrl-names = "default";
> +
> +	port {
> +		ltdc_out_rgb: endpoint {
> +			remote-endpoint = <&panel_in_rgb>;
> +		};
> +	};
> +};
>

^ permalink raw reply

* [PATCH v1 0/7] DRM: add LTDC support for STM32F4
From: Neil Armstrong @ 2017-01-16 16:02 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1484573344-11609-1-git-send-email-yannick.fertre@st.com>

On 01/16/2017 02:28 PM, Yannick Fertre wrote:
> The purpose of this set of patches is to add a new driver for stm32f429.
> This driver was developed and tested on evaluation board stm32429i.
> 
> Stm32f4 is a MCU platform which don't have MMU so the last patches developed
> by Benjamin Gaignard regarding "DRM: allow to use mmuless devices"
> are necessary.
> 
> The board stm429i embeds a Ampire AM-480272H3TMQW-T01H screen.
> A new simple panel am-480272h3tmqw-t01h have been added to support it.
> 
> Yannick Fertre (7):
>   dt-bindings: display: add STM32 LTDC driver
>   drm/st: Add STM32 LTDC driver
>   dt-bindings: Add Ampire AM-480272H3TMQW-T01H panel
>   drm/panel: simple: Add support for Ampire AM-480272H3TMQW-T01H
>   ARM: dts: stm32f429: Add ltdc support
>   ARM: dts: stm32429i-eval: Enable ltdc & simple panel on Eval board
>   ARM: configs: Add STM32 LTDC support in STM32 defconfig
> 
>  .../display/panel/ampire,am-480272h3tmqw-t01h.txt  |    7 +
>  .../devicetree/bindings/display/st,ltdc.txt        |   57 +
>  arch/arm/boot/dts/stm32429i-eval.dts               |   58 +
>  arch/arm/boot/dts/stm32f429.dtsi                   |   25 +-
>  arch/arm/configs/stm32_defconfig                   |    5 +
>  drivers/gpu/drm/Kconfig                            |    2 +
>  drivers/gpu/drm/Makefile                           |    1 +
>  drivers/gpu/drm/panel/panel-simple.c               |   29 +
>  drivers/gpu/drm/st/Kconfig                         |   14 +
>  drivers/gpu/drm/st/Makefile                        |    7 +
>  drivers/gpu/drm/st/drv.c                           |  279 ++++
>  drivers/gpu/drm/st/drv.h                           |   25 +
>  drivers/gpu/drm/st/ltdc.c                          | 1438 ++++++++++++++++++++
>  drivers/gpu/drm/st/ltdc.h                          |   20 +
>  14 files changed, 1966 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/ampire,am-480272h3tmqw-t01h.txt
>  create mode 100644 Documentation/devicetree/bindings/display/st,ltdc.txt
>  create mode 100644 drivers/gpu/drm/st/Kconfig
>  create mode 100644 drivers/gpu/drm/st/Makefile
>  create mode 100644 drivers/gpu/drm/st/drv.c
>  create mode 100644 drivers/gpu/drm/st/drv.h
>  create mode 100644 drivers/gpu/drm/st/ltdc.c
>  create mode 100644 drivers/gpu/drm/st/ltdc.h
> 

Hi Yannick,

Shouldn't be more logical to use stm32 for the driver instead of st ?
It would eventually collude with the other STMicroelectronics SoCs and
will be aligned with other drivers like stm32-rtc, stm32-i2c, ...

Neil

^ permalink raw reply

* [PATCH v1 7/7] ARM: configs: Add STM32 LTDC support in STM32 defconfig
From: Alexandre Torgue @ 2017-01-16 16:03 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1484573344-11609-8-git-send-email-yannick.fertre@st.com>

Hi

On 01/16/2017 02:29 PM, Yannick Fertre wrote:
> Signed-off-by: Yannick Fertre <yannick.fertre@st.com>
> ---

commit header "ARM: configs: stm32: ADD LDTC support"

>  arch/arm/configs/stm32_defconfig | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/arch/arm/configs/stm32_defconfig b/arch/arm/configs/stm32_defconfig
> index 29068f5..e3974d9 100644
> --- a/arch/arm/configs/stm32_defconfig
> +++ b/arch/arm/configs/stm32_defconfig
> @@ -71,3 +71,8 @@ CONFIG_MAGIC_SYSRQ=y
>  # CONFIG_FTRACE is not set
>  CONFIG_CRC_ITU_T=y
>  CONFIG_CRC7=y
> +CONFIG_DRM=y
> +CONFIG_DRM_ST=y
> +CONFIG_DRM_PANEL=y
> +CONFIG_DRM_PANEL_SIMPLE=y
> +CONFIG_BACKLIGHT_LCD_SUPPORT=y
>

^ permalink raw reply

* [PATCH v1 2/2] arm: dts: mt2701: add nor flash node
From: Marek Vasut @ 2017-01-16 16:09 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170116094032.6f471f11@bbrezillon>

On 01/16/2017 09:40 AM, Boris Brezillon wrote:
> On Sun, 15 Jan 2017 01:23:48 +0100
> Marek Vasut <marek.vasut@gmail.com> wrote:
> 
>> On 01/14/2017 09:29 AM, Boris Brezillon wrote:
>>> On Fri, 13 Jan 2017 18:33:40 +0100
>>> Marek Vasut <marek.vasut@gmail.com> wrote:
>>>   
>>>> On 01/13/2017 05:56 PM, Boris Brezillon wrote:  
>>>>> On Fri, 13 Jan 2017 17:44:12 +0100
>>>>> Marek Vasut <marek.vasut@gmail.com> wrote:
>>>>>     
>>>>>> On 01/13/2017 05:28 PM, Boris Brezillon wrote:    
>>>>>>> On Fri, 13 Jan 2017 17:13:55 +0100
>>>>>>> Marek Vasut <marek.vasut@gmail.com> wrote:
>>>>>>>       
>>>>>>>> On 01/13/2017 04:12 PM, Matthias Brugger wrote:      
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 13/01/17 15:17, Boris Brezillon wrote:        
>>>>>>>>>> On Fri, 13 Jan 2017 15:13:29 +0800
>>>>>>>>>> Guochun Mao <guochun.mao@mediatek.com> wrote:
>>>>>>>>>>        
>>>>>>>>>>> Add Mediatek nor flash node.
>>>>>>>>>>>
>>>>>>>>>>> Signed-off-by: Guochun Mao <guochun.mao@mediatek.com>
>>>>>>>>>>> ---
>>>>>>>>>>>  arch/arm/boot/dts/mt2701-evb.dts |   25 +++++++++++++++++++++++++
>>>>>>>>>>>  arch/arm/boot/dts/mt2701.dtsi    |   12 ++++++++++++
>>>>>>>>>>>  2 files changed, 37 insertions(+)
>>>>>>>>>>>
>>>>>>>>>>> diff --git a/arch/arm/boot/dts/mt2701-evb.dts
>>>>>>>>>>> b/arch/arm/boot/dts/mt2701-evb.dts
>>>>>>>>>>> index 082ca88..85e5ae8 100644
>>>>>>>>>>> --- a/arch/arm/boot/dts/mt2701-evb.dts
>>>>>>>>>>> +++ b/arch/arm/boot/dts/mt2701-evb.dts
>>>>>>>>>>> @@ -24,6 +24,31 @@
>>>>>>>>>>>      };
>>>>>>>>>>>  };
>>>>>>>>>>>
>>>>>>>>>>> +&nor_flash {
>>>>>>>>>>> +    pinctrl-names = "default";
>>>>>>>>>>> +    pinctrl-0 = <&nor_pins_default>;
>>>>>>>>>>> +    status = "okay";
>>>>>>>>>>> +    flash at 0 {
>>>>>>>>>>> +        compatible = "jedec,spi-nor";
>>>>>>>>>>> +        reg = <0>;
>>>>>>>>>>> +    };
>>>>>>>>>>> +};
>>>>>>>>>>> +
>>>>>>>>>>> +&pio {
>>>>>>>>>>> +    nor_pins_default: nor {
>>>>>>>>>>> +        pins1 {
>>>>>>>>>>> +            pinmux = <MT2701_PIN_240_EXT_XCS__FUNC_EXT_XCS>,
>>>>>>>>>>> +                 <MT2701_PIN_241_EXT_SCK__FUNC_EXT_SCK>,
>>>>>>>>>>> +                 <MT2701_PIN_239_EXT_SDIO0__FUNC_EXT_SDIO0>,
>>>>>>>>>>> +                 <MT2701_PIN_238_EXT_SDIO1__FUNC_EXT_SDIO1>,
>>>>>>>>>>> +                 <MT2701_PIN_237_EXT_SDIO2__FUNC_EXT_SDIO2>,
>>>>>>>>>>> +                 <MT2701_PIN_236_EXT_SDIO3__FUNC_EXT_SDIO3>;
>>>>>>>>>>> +            drive-strength = <MTK_DRIVE_4mA>;
>>>>>>>>>>> +            bias-pull-up;
>>>>>>>>>>> +        };
>>>>>>>>>>> +    };
>>>>>>>>>>> +};
>>>>>>>>>>> +
>>>>>>>>>>>  &uart0 {
>>>>>>>>>>>      status = "okay";
>>>>>>>>>>>  };
>>>>>>>>>>> diff --git a/arch/arm/boot/dts/mt2701.dtsi
>>>>>>>>>>> b/arch/arm/boot/dts/mt2701.dtsi
>>>>>>>>>>> index bdf8954..1eefce4 100644
>>>>>>>>>>> --- a/arch/arm/boot/dts/mt2701.dtsi
>>>>>>>>>>> +++ b/arch/arm/boot/dts/mt2701.dtsi
>>>>>>>>>>> @@ -227,6 +227,18 @@
>>>>>>>>>>>          status = "disabled";
>>>>>>>>>>>      };
>>>>>>>>>>>
>>>>>>>>>>> +    nor_flash: spi at 11014000 {
>>>>>>>>>>> +        compatible = "mediatek,mt2701-nor",
>>>>>>>>>>> +                 "mediatek,mt8173-nor";        
>>>>>>>>>>
>>>>>>>>>> Why define both here? Is "mediatek,mt8173-nor" really providing a
>>>>>>>>>> subset of the features supported by "mediatek,mt2701-nor"?
>>>>>>>>>>        
>>>>>>>>>
>>>>>>>>> I think even if the ip block is the same, we should provide both
>>>>>>>>> bindings, just in case in the future we find out that mt2701 has some
>>>>>>>>> hidden bug, feature or bug-feature. This way even if we update the
>>>>>>>>> driver, we stay compatible with older device tree blobs in the wild.
>>>>>>>>>
>>>>>>>>> We can drop the mt2701-nor in the bindings definition if you want.       
>>>>>>>
>>>>>>> Oh, sorry, I misunderstood. What I meant is that if you want to
>>>>>>> list/support all possible compatibles, maybe you should just put one
>>>>>>> compatible in your DT and patch your driver (+ binding doc) to define
>>>>>>> all of them.      
>>>>>>
>>>>>> Uh, what ? I lost you here :-)  
>>>
>>> I mean adding a new entry in the mtk_nor_of_ids table (in
>>> mtk-quadspi.c) so that the mediatek,mt2701-nor compatible string can be
>>> matched directly, and you won't need to define 2 compatible strings in
>>> your device tree.  
>>
>> But then you grow the table in the driver, is that what we want if we
>> can avoid that ?
> 
> The space you save by not growing the mtk_nor_of_ids table is lost in
> your dtbs, so I'm not sure the size argument is relevant here. Also,
> note that distros are shipping a lot of dtbs, and you're likely to have
> several boards based on the mt2701 SoC, so, for this specific use case,
> it's better to make the in-driver of-id table grow than specifying 2
> compatibles in the DT. But as I said, I'm not sure we should rely on
> this argument to decide which approach to choose (we're talking about a
> few bytes here).
> 
>>
>>>>>>    
>>>>>>>> This exactly. We should have a DT compat in the form:
>>>>>>>> compatible = "vendor,<soc>-block", "vendor,<oldest-compat-soc>-block";
>>>>>>>> Then if we find a problem in the future, we can match on the
>>>>>>>> "vendor,<soc>-block" and still support the old DTs.      
>>>>>>>
>>>>>>> Not sure it's only in term of whose IP appeared first. My understanding
>>>>>>> is that it's a way to provide inheritance. For example:
>>>>>>>
>>>>>>> 	"<soc-vendor>,<ip-version>", "<ip-vendor>,<ip-version>";
>>>>>>>
>>>>>>> or
>>>>>>>
>>>>>>> 	"<soc-vendor>,<full-featured-ip-version>","<soc-vendor>,<basic-feature-ip-version>";
>>>>>>>
>>>>>>> BTW, which one is the oldest between mt8173 and mt2701? :-)      
>>>>>>
>>>>>> And that's another thing and I agree with you, but I don't think that's
>>>>>> what we're discussing in this thread. But (!), OT, I think we should
>>>>>> codify the rules in Documentation/ . This discussion came up multiple
>>>>>> times recently.
>>>>>>
>>>>>> And my question still stands, what do we put into the DT here, IMO
>>>>>> compatible = "mediatek,mt2701-nor", "mediatek,mt8173-nor";    
>>>>>
>>>>> I'd say
>>>>>
>>>>> 	compatible = "mediatek,mt8173-nor";
>>>>>
>>>>> because both compatible are referring to very specific IP version. It's
>>>>> not the same as    
>>>>
>>>> But then you don't have the ability to handle a block in this particular
>>>> SoC in case there's a bug found in it in the future,
>>>> so IMO it should be:
>>>>
>>>> compatible = "mediatek,mt2701-nor", "mediatek,mt8173-nor";  
>>>
>>> Sorry again, I meant
>>>
>>> 	compatible = "mediatek,mt2701-nor";
>>>   
>>>>  
>>>>> 	compatible = "mediatek,mt8173-nor", "mediatek,mt81xx-nor";    
>>>>
>>>> This doesn't look right, since here we add two new compatibles ...  
>>>
>>> That was just an example to describe how compatible inheritance works
>>> (at least that's my understanding of it), it does not apply to this
>>> particular use case.  
>>
>> Well this is OK I guess, but then you can also use "mediatek,mt8173-nor"
>> as the oldest supported compatible and be done with it, no ? It looks a
>> bit crappy though, I admit that ...
>>
> 
> Let's stop bikeshedding and wait for DT maintainers feedback
> before taking a decision ;-).

+1 :)

> Rob, Mark, any opinion?
> 


-- 
Best regards,
Marek Vasut

^ permalink raw reply

* USB: OHCI: high softirq load
From: Antoine Aubert @ 2017-01-16 16:11 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170116144717.GC19736@localhost>

Thanks Johan, you save my day.

Indeed, soft irq coming from ftdi_sio updates, with very high rate.
(dyndbg='file drivers/usb/* +p')

reducing latency timer = reducing sofirq.

Problem solved.

Antoine Aubert
a.aubert at overkiz.com

Le 16/01/2017 ? 15:47, Johan Hovold a ?crit :
> [ +CC: linux-usb ]
>
> On Mon, Jan 16, 2017 at 12:14:03PM +0100, Boris Brezillon wrote:
>> On Mon, 16 Jan 2017 11:54:23 +0100
>> Antoine Aubert <a.aubert@overkiz.com> wrote:
>>
>>> Also, I made a big misunderstanding
>>>
>>> With EHCI + OHCI = high level of softirq (USB2.0)
>> Well, the number of irqs and softirqs are likely to be related (you
>> usually trigger a softirq after you received an hardirq).
>>
>>> OHCI only = normal level
>> What about EHCI only? And what happens if you only plug 1 device?
>> Please share the content of /proc/interrupts (and everything you think
>> is relevant) for each of these cases.
>>
>>> Le 16/01/2017 ? 11:31, Antoine Aubert a ?crit :
>>>> Thx for your answer Boris
>>>>
>>>> Le 16/01/2017 ? 10:02, Boris Brezillon a ?crit :  
>>>>> Hi Antoine,
>>>>>
>>>>> On Mon, 16 Jan 2017 08:45:58 +0100
>>>>> Antoine Aubert <a.aubert@overkiz.com> wrote:
>>>>>  
>>>>>> Hi,
>>>>>>
>>>>>> Im working on a AT91SAM9G25cu board
>>>>>> (arch/arm/boot/dts/at91-kizboxmini.dts). We use linux-4.1.31, and when
>>>>>> OHCI is enabled, I got some wired effects.  
>>>>> Can you test on a more recent kernel (4.9 or 4.10-rc4)?  
>>>> I'll give a try, just need little time ;)  
>>>>>> eg with 3 FTDI pluged, interrupts: more than 3.5k/s, cpu softirq > 24%,
>>>>>> loadavg > 0.5  
>>>>> Can you check which interrupt is triggered (cat /proc/interrupts),  
>>>> cat /proc/interrupts
>>>>            CPU0      
>>>>  16:       2286  atmel-aic   1 Level     pmc, at91_tick, at91_rtc, ttyS0
>>>>  17:          0       PMC  17 Level     main_rc_osc
>>>>  18:          0       PMC   0 Level     main_osc
>>>>  19:          0       PMC  16 Level     mainck
>>>>  20:          0       PMC   1 Level     clk-plla
>>>>  21:          0       PMC   6 Level     clk-utmi
>>>>  22:          0       PMC   3 Level     clk-master
>>>>  23:     945527  atmel-aic  17 Level     tc_clkevt
>>>>  24:      21815  atmel-aic  20 Level     at_hdmac
>>>>  25:          0  atmel-aic  21 Level     at_hdmac
>>>>  30:     120299  atmel-aic  24 Level     eth0
>>>>  31:   22783651  atmel-aic  22 Level     ehci_hcd:usb1, ohci_hcd:usb2
>>>>  99:          0      GPIO  16 Edge      PB_RST
>>>> 100:          0      GPIO  17 Edge      PB_PROG
>>>> Err:          0
> Note that the ftdi driver uses a low-latency setting by default which
> implies that the device sends a status update every millisecond. Hence,
> the 1k interrupts per second (per device) while the port is open is
> expected.
>
> You can disable the low-latency behaviour using setserial:
>
> 	setserial /dev/ttyUSB0 ^low_latency
>
> and see the number of interrupts drop to 1/16th. This can then be
> reduced further by changing the latency_timer from its
> (non-low-latency) default of 16 ms, for example:
>
> 	echo 64 >/sys/bus/usb-serial/devices/ttyUSB0/latency_timer
>
> Johan

^ permalink raw reply

* [PATCH] rtc: sun6i: Switch to the external oscillator
From: Maxime Ripard @ 2017-01-16 16:13 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170116153959.7qhahevikxzai664@piout.net>

Hi,

On Mon, Jan 16, 2017 at 04:39:59PM +0100, Alexandre Belloni wrote:
> On 16/01/2017 at 16:21:48 +0100, Maxime Ripard wrote :
> > The RTC is clocked from either an internal, imprecise, oscillator or an
> > external one, which is usually much more accurate.
> > 
> > The difference perceived between the time elapsed and the time reported by
> > the RTC is in a 10% scale, which prevents the RTC from being useful at all.
> > 
> > Fortunately, the external oscillator is reported to be mandatory in the
> > Allwinner datasheet, so we can just switch to it.
> > 
> 
> Still, I'm wondering whether the external clock should be taken.
> 
> We've had issues with at91 and tegra where this external clock was
> suddenly able to be stopped, breaking the RTC because the CCF was not
> aware the RTC was using it.

That's a very good point...

> See:
> http://patchwork.ozlabs.org/patch/502459/
> http://patchwork.ozlabs.org/patch/714517/
> 
> Your reply can be that you don't care now and this has a low probability
> and you'll handle the case when it happens and that will be fine.

This is a bit more complicated for us.

The internal oscillator is running at 667kHz, with a 30% accuracy. The
external oscillator is supposed to run at 32768Hz, with a maximum
tolerance of 50ppm.

The RTC has an internal mux, between the internal and external
oscillators. If the internal is picked, a (variable) divider of 20 is
applied by default.

The output of that mux is also one of the parent of many of our clocks
in our main clock unit (for example the CPU one), so we need to have
that parenthood relationship expressed. I guess we could rework the
driver to first register the clock through the early clock probing
stuff, and then have the rest of the RTC to probe.

However, we also need to do so while remaining backward compatible
from a DT point of view.

I guess we could:
  - Add the two oscillators to the DTSI, with their proper accuracy
  - Put them both as parent clocks of the RTC node
  - Split the clock part and the RTC part in the driver, and have the
    clock part, if there is a clocks property in the node (which
    covers the backward case), register the mux, and pick the clock
    with the best accuracy. We don't change anything at the RTC level.
  - Change the parent clock of the CCU for the RTC.

That would work for you?
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170116/75818342/attachment.sig>

^ permalink raw reply

* [RFC PATCH] arm64: defconfig: enable SMMUv3 config
From: Wei Xu @ 2017-01-16 16:15 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <3026029.1QgHzGBYeh@wuerfel>

Hi Arnd,

On 2017/1/13 16:58, Arnd Bergmann wrote:
> On Friday, January 13, 2017 3:35:54 PM CET Wei Xu wrote:
>> On 2017/1/12 9:47, Catalin Marinas wrote:
>>> On Thu, Jan 12, 2017 at 08:36:12AM +0800, Zhou Wang wrote:
>>>> On 2017/1/9 19:50, Zhou Wang wrote:
>>>>> Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
>>>>> ---
>>>>>  arch/arm64/configs/defconfig | 1 +
>>>>>  1 file changed, 1 insertion(+)
>>>>>
>>>>> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
>>>>> index 869dded..3520c50 100644
>>>>> --- a/arch/arm64/configs/defconfig
>>>>> +++ b/arch/arm64/configs/defconfig
>>>>> @@ -440,6 +440,7 @@ CONFIG_PLATFORM_MHU=y
>>>>>  CONFIG_BCM2835_MBOX=y
>>>>>  CONFIG_HI6220_MBOX=y
>>>>>  CONFIG_ARM_SMMU=y
>>>>> +CONFIG_ARM_SMMU_V3=y
>>>>>  CONFIG_RASPBERRYPI_POWER=y
>>>>>  CONFIG_QCOM_SMEM=y
>>>>>  CONFIG_QCOM_SMD=y
>>>>
>>>> I just happened to find there is no SMMUv3 config in arm64 defconfig.
>>>>
>>>> Maybe we should add it in defconfig or I miss something.
>>>
>>
>> Hi Arnd,
>>
>>> It looks fine to me but it's usually the arm-soc guys picking the
>>> defconfig patches.
>>>
>>
>> It is fine to me.
>> Is it OK for me to pick up this or will you directly pick up this?
>> Thanks!
> 
> Either way works for me. I usually recommend platform maintainers
> picking up patches and then sending them on to arm at kernel.org,
> as that gets our attention quicker. A patch that gets sent with
> me personally on Cc might get lost in the email.

In this case I will pick up it.
Thanks!

Best Regards,
Wei

> 
> 	Arnd
> 
> 
> .
> 

^ permalink raw reply

* [GIT PULL] arm64: dts: juno: updates for v4.11
From: Sudeep Holla @ 2017-01-16 16:22 UTC (permalink / raw)
  To: linux-arm-kernel

Hi ARM SoC Team,

Please pull !

Regards,
Sudeep

The following changes since commit 7ce7d89f48834cefece7804d38fc5d85382edf77:

  Linux 4.10-rc1 (2016-12-25 16:13:08 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/sudeep.holla/linux.git
tags/juno-updates-4.11

for you to fetch changes up to ea2dcf9ac3254c964cddd67a032cb3b691fa47ce:

  arm64: dts: juno: add missing CoreSight STM component (2017-01-12
17:26:49 +0000)

----------------------------------------------------------------
ARMv8 Vexpress/Juno DT updates for v4.11

1. Addition of Coresight support on Juno R1 and R2 variants

2. Addition of STM(System Trace Macrocell) support on all Juno variants

----------------------------------------------------------------
Mike Leach (2):
      arm64: dts: juno: fix CoreSight support for Juno r1/r2 variants
      arm64: dts: juno: add missing CoreSight STM component

Sudeep Holla (1):
      arm64: dts: juno: refactor CoreSight support on Juno r0

 arch/arm64/boot/dts/arm/juno-base.dtsi    | 31 +++++++---
 arch/arm64/boot/dts/arm/juno-cs-r1r2.dtsi | 98
+++++++++++++++++++++++++++++++
 arch/arm64/boot/dts/arm/juno-r1.dts       | 13 ++++
 arch/arm64/boot/dts/arm/juno-r2.dts       | 13 ++++
 arch/arm64/boot/dts/arm/juno.dts          | 24 ++++++++
 5 files changed, 171 insertions(+), 8 deletions(-)
 create mode 100644 arch/arm64/boot/dts/arm/juno-cs-r1r2.dtsi

^ permalink raw reply

* [PATCH] Replacement for Arm initrd memblock reserve and free inconsistency.
From: Russell King - ARM Linux @ 2017-01-16 16:22 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <592F4D3D062D79449F140027567F70E8FE081234@exchmbx03>

On Thu, Nov 17, 2016 at 10:41:19AM +0000, william.helsby at stfc.ac.uk wrote:
> My understanding of your proposed patch is that it rounds the area to be reserved for the initrd image to page
> boundaries. It then checks that this rounding does not grow the initrd to overlap other areas.
> 
> If it does not overlap, all is well. The kernel will not kmalloc anything within the partial pages 
> at the beginning and end of the initrd image. When the initrd image is freed, complete pages are 
> released and there is no wasted memory.
> However, if it does overlap other regions, it disables the initrd image and the kernel panics with no root file system.

As best I can do when replying to this... (due to the stupidly long lines)...

Yes, that's what will happen.  The kernel memory allocator deals with
things on a per-page basis, and that's not going to change.  If someone
wants to place objects on a non-page aligned basis into memory, then
they're intentionally making life hard for the kernel.

You talk about kmalloc() above, but kmalloc() is not really relevant
here - kmalloc() should be thought as a sub-page memory allocator (even
though it's capable of larger allocations.)  It sources its memory from
page_alloc(), which not surprisingly deals with page-sized objects, and
those are either "in-use" or "free".  There's no finer granularity there.

So, if you have a page partially in-use, as far as the kernel's normal
allocators are concerned, the whole page is in use.

Boot time is slightly different until the normal allocators are
initialised - we have memblock, which doesn't really know about stuff
like pages.  It only knows about ranges of addresses.  However, during
initialisation, when memblock hands over to the normal allocators, any
page that is partially reserved is treated as wholely reserved because
- as I say above - the normal allocators can't deal with that level of
information.

Rounding the initrd up/down when reserving it in memblock is the right
thing to do to ensure that when memory is requested from memblock,
memblock doesn't allocate the overlapping pages - because we want to
free that memory later.

So, I don't think your idea of trying to cover all the cases of part-used
pages is really worth it - and the boot loader has to understand that the
kernel operates on a page-basis, and expects objects to be aligned to the
size of a page.

-- 
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

* [RFC PATCH 08/10] arm64/sve: ptrace: Wire up vector length control and reporting
From: Dave Martin @ 2017-01-16 16:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <a7261d6b-0074-7345-d380-ef0dd335dd56@redhat.com>

On Mon, Jan 16, 2017 at 03:47:55PM +0000, Pedro Alves wrote:
> On 01/16/2017 03:11 PM, Yao Qi wrote:
> > 
> >> > 
> >> > gdb must already re-detect the vector length on stop, since the target
> >> > could have called the prctl() in the meantime.
> > Yes, gdb assumes the vector length may be changed, so it re-detects on
> > every stop, but I don't see the need for gdb to change the vector length.
> > 
> 
> Do we need to consider inferior function calls here?
> 
> Say the program is stopped in code that assumes "vector length N", and
> the user does "print some_function_that_assumes_some_other_vector_length ()".
> 
> Is that a use case we need to cover?
> 
> If so, to make it work correctly, the debugger needs to be able to change the
> vector length to the length assumed by that called function, and then
> restore it back after the call completes (or is aborted).
> 
> I have no idea whether the debugger will be able to figure
> out a function's assumed vector length from debug info or some such.

I think the proposed ptrace interface can support this -- i.e., it
should provide everything needed to save off the VL and register state,
switch VL, do something else, then restore the VL and state (if not,
that's a bug).

My current position is that determining what vector length is
required by what function or binary is a 100% userspace problem, though.

ELF/DWARF could have annotations about this, though it wouldn't
necessarily be per-function -- you might require a whole image to be
built for the same vector length (if any).

Cheers
---Dave

^ permalink raw reply

* [linux-sunxi] [PATCH 1/2] drivers: pinctrl: add driver for Allwinner H5 SoC
From: Maxime Ripard @ 2017-01-16 16:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <38fe3491-457a-f0c5-54fb-9defdcd45045@arm.com>

On Mon, Jan 09, 2017 at 12:16:00AM +0000, Andr? Przywara wrote:
> On 05/01/17 22:42, Maxime Ripard wrote:
> > On Fri, Dec 30, 2016 at 01:55:44PM +0100, Linus Walleij wrote:
> >> On Mon, Dec 26, 2016 at 3:33 PM, Andr? Przywara <andre.przywara@arm.com> wrote:
> >>
> >>> So while this patch technically looks correct, I was wondering if we
> >>> should really explore the possibility of making the whole of sunxi
> >>> pinctrl DT controlled.
> >>> I brought this up a while ago, but people weren't overly enthusiastic
> >>> about it, though their argument weren't really convincing to me[1].
> >>>
> >>> So:
> >>> As this "driver" here is basically a table linking GPIO bit settings
> >>> (the actual mux value) to names and every pin we care about needs to be
> >>> enumerated in the DT anyway, why not just add something like:
> >>> allwinner,pinmux = <4>;
> >>> to each pin(group) in the DT and get rid of this "driver" file here
> >>> entirely?
> >>
> >> I'm open to that if you can use pinctrl-single which is in the kernel
> >> for this purpose only, and is used with both OMAPs and HiSilicon.
> > 
> > I'm not open to that, and I'm getting tired of discussing it over and
> > over again. Andre, if you want to be convinced again, please read the
> > last discussion we had on this topic.
> 
> As I said: It didn't convince me back then. And frankly we didn't really
> discuss it back then, I just refrained from entering a discussion
> against _two_ maintainers at this time, since my capacity on this kind
> of email threads is really very limited - especially for something that
> is a hobby to me.

This is also (mostly) a hobby to me, which is exactly why I prefer to
work on something actually useful, rather than just discussing this
over and over again. Just like I don't want (myself, or anyone,
really, since we're all in the same boat) to have to maintain two
separate pinctrl drivers.

We're having a documented, simple, pinctrl binding, using the generic
bindings now (that almost everyone else is using now, or is very close
to), and we can leverage as much documentation and code from that. Why
would we want to create and maintain a new driver with a new binding,
that will need to be documented again, learned by everyone, and will
lead only to confusion across the people who just want to have their
board supported?

And imagine the kind of mess we would be in if everyone getting a bit
involved in the support of one platform at one point wanted to trash
one big part of its infrastructure and start all over again because he
likes it better this way.

I'm sorry, but this is also our jobs as maintainers to prevent all
these kind of issues, and to maintain consistency. Switching to one
binding to another breaks that consistency on many level, both in
pinctrl and in the platform support itself. So sorry, but that's not
going to happen.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170116/e4bf707c/attachment.sig>

^ permalink raw reply

* [linux-sunxi] [PATCH] clk: sunxi-ng: fix PLL_CPUX adjusting on H3
From: Maxime Ripard @ 2017-01-16 16:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <557330d0-2dcd-2211-8538-51c4d6b68834@megous.com>

Hi Ondrej,

Sorry for the late reply,

On Mon, Jan 09, 2017 at 03:50:42PM +0100, Ond?ej Jirman wrote:
> Dne 9.1.2017 v 10:59 Maxime Ripard napsal(a):
> > On Sat, Jan 07, 2017 at 04:49:18PM +0100, Ond?ej Jirman wrote:
> >> Maxime,
> >>
> >> Dne 25.11.2016 v 01:28 megous at megous.com napsal(a):
> >>> From: Ondrej Jirman <megous@megous.com>
> >>>
> >>> When adjusting PLL_CPUX on H3, the PLL is temporarily driven
> >>> too high, and the system becomes unstable (oopses or hangs).
> >>>
> >>> Add a notifier to avoid this situation by temporarily switching
> >>> to a known stable 24 MHz oscillator.
> >>
> >> I have done more thorough testing on H3 and this approach with switching
> >> to 24MHz oscillator does not work. Motivation being that my Orange Pi
> >> One still gets lockups even with this patch under certain circumstances.
> >>
> >> So I have created a small test program for CPUS (additional OpenRISC CPU
> >> on the SoC) which randomly changes PLL_CPUX settings while main CPU is
> >> running a loop that sends messages to CPUS via msgbox.
> >>
> >> Assumption being that while CPUS is successfully receiving messages via
> >> msgbox, the main CPU didn't lock up, yet.
> >>
> >> With this I am able to quickly and thoroughly test various PLL_CPUX
> >> change and factor selection algorithms.
> >>
> >> Results are that bypassing CPUX clock by switching to 24 MHz oscillator
> >> does not work at all. Main CPU locks up in about 1 second into the test.
> >> Don't ask me why.
> > 
> > You mean that you are changing the frequency behind Linux' back? That
> > won't work. There's more to cpufreq than just changing the frequency,
> > but also adusting the number of loops per jiffy for the new frequency
> > for example. I don't really expect that setup to work even on a
> > perfectly stable system. CPUFreq *has* to be involved, otherwise, that
> > alone might introduce bugs, and you cannot draw any conclusions
> > anymore.
> 
> No, this has nothing to do with linux. I'm not running linux for this
> test. I'm running a small program on CPUS (Open RISC CPU) on the SoC
> loaded using FEL from USB.
> 
> The main cpu is just pushing messages into msgbox in a loop, so that
> CPUS can determine that the main CPU is still running ok and give
> feedback to me over UART. Not even DRAM is involved. The programs are
> running from SRAM.
> 
> This is the most direct test of PLL change stability that can be done on
> this SoC regardless of the OS. Not even CPU voltage switching is
> involved. I just set the maximum voltage and fiddle with CPU_PLL
> frequencies randomly, while waiting for the main CPU to lock up.

Ok.

> It does lock up quickly with mainline ccu_nkmp_find_best algorithm
> for finding factors.
> 
> Even with linux kernel, it breaks. It's just more difficult to hit the
> right conditions. I got oops only right after boot when running cpuburn
> to trigger thermal_zone issued OPP change, if I first run some cpupower
> commands. That's why I wrote this program to stress test various CPU_PLL
> change/factor selection algorithms independently of everything else, to
> get more predictable and quicker testing results.

Understood. Do you have the code available somewhere?

> >> What works is selecting NKMP factors so that M is always 1 and P is
> >> anything other than /1 only for frequencies under 288MHz. As mandated by
> >> the H3 datasheet. Mainline ccu_nkmp_find_best doesn't respect these
> >> conditions. With that I can change CPUX frequencies randomly 20x a
> >> second so far indefinitely without the main CPU ever locking up.
> >>
> >> Please drop or revert this patch. It is not a correct approach to the
> >> problem. I'd suggest dropping the entire clock notifier mechanism, too,
> >> unless it can be proven to work reliably.
> > 
> > It has been proven to work reliably on a number of other SoCs.
> 
> Unless it was stress tested like this with randomy changed settings, I
> doubt you can call it reliable. It may just be very hard to hit the
> issue on linux with particular OPP/thermal zone configuration. That's
> because the issue is dependent on before and after NKMP values. People
> may have just been lucky so far.

Yes, or maybe we just have OPPs that just don't trigger a low enough P
factor.

There's no rush anyway, the H3 cpufreq support is not enabled at the
moment, so that code basically does nothing for the moment.

What's your current plan to fix that? I guess the easiest (and most
likely to be reusable) would be to allow for clock tables, instead of
using the generic approach. We might have some other clocks (like
audio or video) that would need such a precise tuning in the future
too.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170116/94186f7d/attachment.sig>

^ 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