* [RFC] Configure i.MX6 RGMII pad group control registers from device tree
From: Michal Vokáč @ 2018-06-11 12:36 UTC (permalink / raw)
To: linux-arm-kernel
Ahoj,
To configure individual pad's characteristics on i.MX6 SoC a
fsl,pins = <PIN_FUNC_ID CONFIG> property can be used. Is there any convenient
way to configure the pad group control registers?
The issue is that some bits (DDR_SEL and ODT) in the individual RGMII pad
control registers are read-only. To tweak those parameters (signal voltage and
termination resistors) one need to write to the pad group control registers for
the whole RGMII pad group. Namely IOMUXC_SW_PAD_CTL_GRP_DDR_TYPE_RGMII and
IOMUXC_SW_PAD_CTL_GRP_RGMII_TERM. The group registers in general are not
accessible from the list in arch/arm/boot/dts/imx6dl-pinfunc.h.
I could not find any other way to change the group registers than hacking-in
some lines into the imx6q_init_machine(void) function in
arch/arm/mach-imx/mach-imx6q.c source. As I work towards upstreaming my board
this should be done from my device tree or solved in some universal way.
Any hints will be much appreciated.
Michal
^ permalink raw reply
* [PATCH v1] ARM: imx: add imx7d-m4
From: Stefan Agner @ 2018-06-11 12:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <a4657c84-a807-d194-950c-38f5f8e4a53c@pengutronix.de>
On 11.06.2018 13:53, Oleksij Rempel wrote:
> On 11.06.2018 13:48, Stefan Agner wrote:
>> On 11.06.2018 10:35, Lucas Stach wrote:
>>> Hi Shawn,
>>>
>>> Am Montag, den 11.06.2018, 16:20 +0800 schrieb Shawn Guo:
>>>> On Mon, Jun 11, 2018 at 10:02:53AM +0200, Oleksij Rempel wrote:
>>>>> Hi all,
>>>>>
>>>>> this patch was send 05.04.2018. Any comments?
>>>>>
>>>>> @Shawn, can you please take it?
>>>>
>>>> Honestly I'm not sure how useful it will be.??If we can have some i.MX
>>>> developers ACK on it, I will be more comfortable to take it.
>>>
>>> This is all highly experimental and in PoC stage, but we see some value
>>> in running a second Linux system on the M4 coprocessor. There are lots
>>> of things that still need to be figured out, but we are working on this
>>> from time to time when there are some hours to spare.
>>>
>>> This patch seems like a good step in the right direction and IMHO the
>>> amount of code and changes is small enough to carry it upstream without
>>> impacting anything else. I would be happy if this could be pulled in.
>>
>> I agree with Lucas here, this is rather minimal and not invasive.
>>
>>
>> Out of interest, on what memory region do you run Linux? Do you use
>> caches? In some experiments a while ago I noticed that only 2MiB/(or
>> 4MiB) of DDR memory can use caches, which is somewhat tight to run Linux
>> on.
>>
>> https://blog.printk.io/2017/05/i-mx-7-cortex-m4-memory-locations-and-performance/
>
> here is DT part for master system on Cortex A7 to run Linux on Cortex M4:
>
> memory {
> device_type = "memory";
> reg = <0x80000000 0x40000000>;
> };
>
> reserved-memory {
> #address-cells = <1>;
> #size-cells = <1>;
> ranges;
>
> m4_reserved_sysmem1: rproc at 88000000 {
> reg = <0x88000000 0x4000000>;
> no-map;
> };
So I guess that is where Linux on the M4 goes? Afaik this is in the
uncacheable area, so it is rather slow?
--
Stefan
>
> /* not really needed node. used as example */
> m4_reserved_sysmem2: rproc at 88080000 {
> reg = <0x8c000000 0x80000>;
> no-map;
> };
> };
>
> mailbox_test {
> compatible = "mailbox-test";
> reg = <0x00900000 0x00020000>, <0x00920000
> 0x00020000>;
> mboxes = <&mu0a 0>, <&mu0a 0>;
> mbox-names = "tx", "rx";
> };
> };
>
> /* node reserved for rproc */
> &uart1 {
> assigned-clock-rates = <240000000>;
> status = "disabled";
> };
>
> &gpt2 {
> assigned-clock-rates = <24000000>;
> status = "disabled";
> };
>
> &mu0a {
> status = "okay";
> #mbox-cells = <1>;
> };
>
> &imx_rproc {
> status = "okay";
> memory-region = <&m4_reserved_sysmem1>,
> <&m4_reserved_sysmem2>;
>
> remote-nodes = <&gpt2>, <&uart1>;
> };
>
>
>
>> --
>> Stefan
>>
>>
>>>>>
>>>>> On 05.04.2018 13:51, Oleksij Rempel wrote:
>>>>>> Provide basic support for Cortex-M4 located on NXP iMX7D.
>>>>>> This code was tested in combination with imx-rproc driver
>>>>>> which will upload with specially formatted ELF image containing
>>>>>> kernel, device and CPIO rootfs.
>>>>>>
>>>>>>>>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
>>>>>> ---
>>>>>> ?arch/arm/boot/dts/Makefile?????????|??2 +-
>>>>>> ?arch/arm/mach-imx/Kconfig??????????| 33 +++++++++++++++++++++------------
>>>>>> ?arch/arm/mach-imx/Makefile?????????|??3 ++-
>>>>>> ?arch/arm/mach-imx/mach-imx7d-cm4.c | 21 +++++++++++++++++++++
>>>>>> ?4 files changed, 45 insertions(+), 14 deletions(-)
>>>>>> ?create mode 100644 arch/arm/mach-imx/mach-imx7d-cm4.c
>>>>>>
>>>>>> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
>>>>>> index 852452515bea..d49bb9a58aee 100644
>>>>>> --- a/arch/arm/boot/dts/Makefile
>>>>>> +++ b/arch/arm/boot/dts/Makefile
>>>>>> @@ -527,7 +527,7 @@ dtb-$(CONFIG_SOC_IMX6UL) += \
>>>>>>>>> ? imx6ul-tx6ul-0011.dtb \
>>>>>>>>> ? imx6ul-tx6ul-mainboard.dtb \
>>>>>>>>> ? imx6ull-14x14-evk.dtb
>>>>>> -dtb-$(CONFIG_SOC_IMX7D) += \
>>>>>> +dtb-$(CONFIG_SOC_IMX7D_CA7) += \
>>>>>>>>> ? imx7d-cl-som-imx7.dtb \
>>>>>>>>> ? imx7d-colibri-emmc-eval-v3.dtb \
>>>>>>>>> ? imx7d-colibri-eval-v3.dtb \
>>>>>> diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
>>>>>> index 782699e67600..101c8599d952 100644
>>>>>> --- a/arch/arm/mach-imx/Kconfig
>>>>>> +++ b/arch/arm/mach-imx/Kconfig
>>>>>> @@ -528,18 +528,6 @@ config SOC_IMX6UL
>>>>>>>>> ? help
>>>>>>>>> ? ??This enables support for Freescale i.MX6 UltraLite processor.
>>>>>> ?
>>>>>> -config SOC_IMX7D
>>>>>>>>> - bool "i.MX7 Dual support"
>>>>>>>>> - select PINCTRL_IMX7D
>>>>>>>>> - select ARM_GIC
>>>>>>>>> - select HAVE_ARM_ARCH_TIMER
>>>>>>>>> - select HAVE_IMX_ANATOP
>>>>>>>>> - select HAVE_IMX_MMDC
>>>>>>>>> - select HAVE_IMX_SRC
>>>>>>>>> - select IMX_GPCV2
>>>>>>>>> - help
>>>>>>>>> - This enables support for Freescale i.MX7 Dual processor.
>>>>>> -
>>>>>> ?config SOC_LS1021A
>>>>>>>>> ? bool "Freescale LS1021A support"
>>>>>>>>> ? select ARM_GIC
>>>>>> @@ -554,6 +542,27 @@ comment "Cortex-A/Cortex-M asymmetric multiprocessing platforms"
>>>>>> ?
>>>>>> ?if ARCH_MULTI_V7 || ARM_SINGLE_ARMV7M
>>>>>> ?
>>>>>> +config SOC_IMX7D_CA7
>>>>>>>>> + bool
>>>>>>>>> + select ARM_GIC
>>>>>>>>> + select HAVE_ARM_ARCH_TIMER
>>>>>>>>> + select HAVE_IMX_ANATOP
>>>>>>>>> + select HAVE_IMX_MMDC
>>>>>>>>> + select HAVE_IMX_SRC
>>>>>>>>> + select IMX_GPCV2
>>>>>> +
>>>>>> +config SOC_IMX7D_CM4
>>>>>>>>> + bool
>>>>>>>>> + select ARMV7M_SYSTICK
>>>>>> +
>>>>>> +config SOC_IMX7D
>>>>>>>>> + bool "i.MX7 Dual support"
>>>>>>>>> + select PINCTRL_IMX7D
>>>>>>>>> + select SOC_IMX7D_CA7 if ARCH_MULTI_V7
>>>>>>>>> + select SOC_IMX7D_CM4 if ARM_SINGLE_ARMV7M
>>>>>>>>> + help
>>>>>>>>> + This enables support for Freescale i.MX7 Dual processor.
>>>>>> +
>>>>>> ?config SOC_VF610
>>>>>>>>> ? bool "Vybrid Family VF610 support"
>>>>>>>>> ? select ARM_GIC if ARCH_MULTI_V7
>>>>>> diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
>>>>>> index 8ff71058207d..68640f100ef3 100644
>>>>>> --- a/arch/arm/mach-imx/Makefile
>>>>>> +++ b/arch/arm/mach-imx/Makefile
>>>>>> @@ -80,7 +80,8 @@ obj-$(CONFIG_SOC_IMX6Q) += mach-imx6q.o
>>>>>> ?obj-$(CONFIG_SOC_IMX6SL) += mach-imx6sl.o
>>>>>> ?obj-$(CONFIG_SOC_IMX6SX) += mach-imx6sx.o
>>>>>> ?obj-$(CONFIG_SOC_IMX6UL) += mach-imx6ul.o
>>>>>> -obj-$(CONFIG_SOC_IMX7D) += mach-imx7d.o
>>>>>> +obj-$(CONFIG_SOC_IMX7D_CA7) += mach-imx7d.o
>>>>>> +obj-$(CONFIG_SOC_IMX7D_CM4) += mach-imx7d-cm4.o
>>>>>> ?
>>>>>> ?ifeq ($(CONFIG_SUSPEND),y)
>>>>>> ?AFLAGS_suspend-imx6.o :=-Wa,-march=armv7-a
>>>>>> diff --git a/arch/arm/mach-imx/mach-imx7d-cm4.c b/arch/arm/mach-imx/mach-imx7d-cm4.c
>>>>>> new file mode 100644
>>>>>> index 000000000000..c36dea79aeb8
>>>>>> --- /dev/null
>>>>>> +++ b/arch/arm/mach-imx/mach-imx7d-cm4.c
>>>>>> @@ -0,0 +1,21 @@
>>>>>> +/*
>>>>>> + * Copyright 2017 Pengutronix
>>>>>> + *
>>>>>> + * 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/kernel.h>
>>>>>> +#include <asm/v7m.h>
>>>>>> +#include <asm/mach/arch.h>
>>>>>> +
>>>>>> +static const char * const imx7d_cm4_dt_compat[] __initconst = {
>>>>>>>>> + "fsl,imx7d-cm4",
>>>>>>>>> + NULL,
>>>>>> +};
>>>>>> +
>>>>>> +DT_MACHINE_START(IMX7D, "Freescale i.MX7 Dual Cortex-M4 (Device Tree)")
>>>>>>>>> + .dt_compat = imx7d_cm4_dt_compat,
>>>>>>>>> + .restart = armv7m_restart,
>>>>>> +MACHINE_END
>>>>>>
>>>>
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> linux-arm-kernel mailing list
>>>> linux-arm-kernel at lists.infradead.org
>>>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>>
>>> _______________________________________________
>>> linux-arm-kernel mailing list
>>> linux-arm-kernel at lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
>>
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v6 4/5] clocksource: add driver for i.MX EPIT timer
From: Clément Péron @ 2018-06-11 12:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5df13d8f6422d9099c1692dd2091d9ca@agner.ch>
Hi Stefan,
> > +
> > +#define EPITCR 0x00
> > +#define EPITSR 0x04
> > +#define EPITLR 0x08
> > +#define EPITCMPR 0x0c
> > +#define EPITCNR 0x10
> > +
> > +#define EPITCR_EN BIT(0)
> > +#define EPITCR_ENMOD BIT(1)
> > +#define EPITCR_OCIEN BIT(2)
> > +#define EPITCR_RLD BIT(3)
> > +#define EPITCR_PRESC(x) (((x) & 0xfff) << 4)
> > +#define EPITCR_SWR BIT(16)
> > +#define EPITCR_IOVW BIT(17)
> > +#define EPITCR_DBGEN BIT(18)
> > +#define EPITCR_WAITEN BIT(19)
> > +#define EPITCR_RES BIT(20)
> > +#define EPITCR_STOPEN BIT(21)
> > +#define EPITCR_OM_DISCON (0 << 22)
> > +#define EPITCR_OM_TOGGLE (1 << 22)
> > +#define EPITCR_OM_CLEAR (2 << 22)
> > +#define EPITCR_OM_SET (3 << 22)
> > +#define EPITCR_CLKSRC_OFF (0 << 24)
> > +#define EPITCR_CLKSRC_PERIPHERAL (1 << 24)
> > +#define EPITCR_CLKSRC_REF_HIGH (2 << 24)
> > +#define EPITCR_CLKSRC_REF_LOW (3 << 24)
> > +
> > +#define EPITSR_OCIF BIT(0)
> > +
> > +struct epit_timer {
> > + void __iomem *base;
> > + int irq;
> > + struct clk *clk;
> > + struct clock_event_device ced;
> > + struct irqaction act;
> > +};
> > +
> > +static void __iomem *sched_clock_reg;
> > +
> > +static inline struct epit_timer *to_epit_timer(struct clock_event_device *ced)
> > +{
> > + return container_of(ced, struct epit_timer, ced);
> > +}
> > +
> > +static inline void epit_irq_disable(struct epit_timer *epittm)
> > +{
> > + u32 val;
> > +
> > + val = readl_relaxed(epittm->base + EPITCR);
> > + writel_relaxed(val & ~EPITCR_OCIEN, epittm->base + EPITCR);
> > +}
> > +
> > +static inline void epit_irq_enable(struct epit_timer *epittm)
> > +{
> > + u32 val;
> > +
> > + val = readl_relaxed(epittm->base + EPITCR);
> > + writel_relaxed(val | EPITCR_OCIEN, epittm->base + EPITCR);
> > +}
> > +
> > +static void epit_irq_acknowledge(struct epit_timer *epittm)
> > +{
> > + writel_relaxed(EPITSR_OCIF, epittm->base + EPITSR);
> > +}
> > +
> > +static u64 notrace epit_read_sched_clock(void)
> > +{
> > + return ~readl_relaxed(sched_clock_reg);
> > +}
> > +
> > +static int epit_set_next_event(unsigned long cycles,
> > + struct clock_event_device *ced)
> > +{
> > + struct epit_timer *epittm = to_epit_timer(ced);
> > + unsigned long tcmp;
> > +
> > + tcmp = readl_relaxed(epittm->base + EPITCNR) - cycles;
> > + writel_relaxed(tcmp, epittm->base + EPITCMPR);
> > +
> > + return 0;
> > +}
> > +
> > +/* Left event sources disabled, no more interrupts appear */
> > +static int epit_shutdown(struct clock_event_device *ced)
> > +{
> > + struct epit_timer *epittm = to_epit_timer(ced);
> > + unsigned long flags;
> > +
> > + /*
> > + * The timer interrupt generation is disabled at least
> > + * for enough time to call epit_set_next_event()
> > + */
> > + local_irq_save(flags);
> > +
> > + /* Disable interrupt in EPIT module */
> > + epit_irq_disable(epittm);
> > +
> > + /* Clear pending interrupt */
> > + epit_irq_acknowledge(epittm);
> > +
> > + local_irq_restore(flags);
> > +
> > + return 0;
> > +}
> > +
> > +static int epit_set_oneshot(struct clock_event_device *ced)
> > +{
> > + struct epit_timer *epittm = to_epit_timer(ced);
> > + unsigned long flags;
> > +
> > + /*
> > + * The timer interrupt generation is disabled at least
> > + * for enough time to call epit_set_next_event()
> > + */
> > + local_irq_save(flags);
> > +
> > + /* Disable interrupt in EPIT module */
> > + epit_irq_disable(epittm);
> > +
> > + /* Clear pending interrupt, only while switching mode */
> > + if (!clockevent_state_oneshot(ced))
> > + epit_irq_acknowledge(epittm);
> > +
> > + /*
> > + * Do not put overhead of interrupt enable/disable into
> > + * epit_set_next_event(), the core has about 4 minutes
> > + * to call epit_set_next_event() or shutdown clock after
> > + * mode switching
> > + */
> > + epit_irq_enable(epittm);
> > + local_irq_restore(flags);
> > +
> > + return 0;
> > +}
> > +
> > +static irqreturn_t epit_timer_interrupt(int irq, void *dev_id)
> > +{
> > + struct clock_event_device *ced = dev_id;
> > + struct epit_timer *epittm = to_epit_timer(ced);
> > +
> > + epit_irq_acknowledge(epittm);
> > +
> > + ced->event_handler(ced);
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static int __init epit_clocksource_init(struct epit_timer *epittm)
> > +{
> > + unsigned int c = clk_get_rate(epittm->clk);
> > +
> > + sched_clock_reg = epittm->base + EPITCNR;
> > + sched_clock_register(epit_read_sched_clock, 32, c);
> > +
> > + return clocksource_mmio_init(epittm->base + EPITCNR, "epit", c, 200, 32,
> > + clocksource_mmio_readl_down);
> > +}
> > +
> > +static int __init epit_clockevent_init(struct epit_timer *epittm)
> > +{
> > + struct clock_event_device *ced = &epittm->ced;
> > + struct irqaction *act = &epittm->act;
> > +
> > + ced->name = "epit";
> > + ced->features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_DYNIRQ;
> > + ced->set_state_shutdown = epit_shutdown;
> > + ced->tick_resume = epit_shutdown;
> > + ced->set_state_oneshot = epit_set_oneshot;
> > + ced->set_next_event = epit_set_next_event;
> > + ced->rating = 200;
> > + ced->cpumask = cpumask_of(0);
> > + ced->irq = epittm->irq;
> > + clockevents_config_and_register(ced, clk_get_rate(epittm->clk),
> > + 0xff, 0xfffffffe);
> > +
> > + act->name = "i.MX EPIT Timer Tick",
> > + act->flags = IRQF_TIMER | IRQF_IRQPOLL;
> > + act->handler = epit_timer_interrupt;
> > + act->dev_id = ced;
> > +
> > + /* Make irqs happen */
> > + return setup_irq(epittm->irq, act);
> > +}
> > +
> > +static int __init epit_timer_init(struct device_node *np)
> > +{
> > + struct epit_timer *epittm;
> > + int ret;
> > +
> > + epittm = kzalloc(sizeof(*epittm), GFP_KERNEL);
> > + if (!epittm)
> > + return -ENOMEM;
> > +
> > + epittm->base = of_iomap(np, 0);
> > + if (!epittm->base) {
> > + ret = -ENXIO;
> > + goto out_kfree;
> > + }
> > +
> > + epittm->irq = irq_of_parse_and_map(np, 0);
> > + if (!epittm->irq) {
> > + ret = -EINVAL;
> > + goto out_iounmap;
> > + }
> > +
> > + /* Get EPIT clock */
> > + epittm->clk = of_clk_get(np, 0);
> > + if (IS_ERR(epittm->clk)) {
> > + pr_err("i.MX EPIT: unable to get clk\n");
> > + ret = PTR_ERR(epittm->clk);
> > + goto out_iounmap;
> > + }
>
> There is something off with indent here.
Thanks for pointing out that, I will fix it.
>
> There is a helper library in drivers/clocksource/timer-of.c which might
> be useful for this driver.
Indeed, but will require a bit of rewrite.
Clement
>
> --
> Stefan
>
> > +
> > + ret = clk_prepare_enable(epittm->clk);
> > + if (ret) {
> > + pr_err("i.MX EPIT: unable to prepare+enable clk\n");
> > + goto out_iounmap;
> > + }
> > +
> > + /* Initialise to a known state (all timers off, and timing reset) */
> > + writel_relaxed(0x0, epittm->base + EPITCR);
> > + writel_relaxed(0xffffffff, epittm->base + EPITLR);
> > + writel_relaxed(EPITCR_EN | EPITCR_CLKSRC_REF_HIGH | EPITCR_WAITEN,
> > + epittm->base + EPITCR);
> > +
> > + ret = epit_clocksource_init(epittm);
> > + if (ret) {
> > + pr_err("i.MX EPIT: failed to init clocksource\n");
> > + goto out_clk_disable;
> > + }
> > +
> > + ret = epit_clockevent_init(epittm);
> > + if (ret) {
> > + pr_err("i.MX EPIT: failed to init clockevent\n");
> > + goto out_clk_disable;
> > + }
> > +
> > + return 0;
> > +
> > +out_clk_disable:
> > + clk_disable_unprepare(epittm->clk);
> > +out_iounmap:
> > + iounmap(epittm->base);
> > +out_kfree:
> > + kfree(epittm);
> > +
> > + return ret;
> > +}
> > +TIMER_OF_DECLARE(epit_timer, "fsl,imx31-epit", epit_timer_init);
^ permalink raw reply
* [PATCH 1/1] ARM: dts: s5pv210: Add missing interrupt-controller property to gph2
From: Krzysztof Kozlowski @ 2018-06-11 12:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1528640149-2195-1-git-send-email-pawel.mikolaj.chmiel@gmail.com>
On Sun, Jun 10, 2018 at 4:15 PM, Pawe? Chmiel
<pawel.mikolaj.chmiel@gmail.com> wrote:
> This commit adds missing interrupt-controller property to gph2 block,
Just "Add missing". See:
https://elixir.bootlin.com/linux/latest/source/Documentation/process/submitting-patches.rst#L151
> to silence following warnings during build
> /soc/pinctrl at e0200000/gph2: Missing interrupt-controller or interrupt-map property
>
> Observed on not yet mainlined, an S5PV210 based
> Samsung Galaxy S (i9000) phone.
The warning is not reproduceable (as you mentioned board is not
present in mainline) thus please skip it. Instead, either describe
existing reason for this change (e.g. because bindings require it for
node of every bank of pins supporting GPIO interrupts) or include this
in series mainlining new board (where the reason will be - it will be
used by new board etc).
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH v6 4/5] clocksource: add driver for i.MX EPIT timer
From: Stefan Agner @ 2018-06-11 12:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAJiuCcdHKpfCkLhV=rZcLHx2k9BAh7bnDQMWDkquXJOYiQoTeA@mail.gmail.com>
On 11.06.2018 14:42, Cl?ment P?ron wrote:
> Hi Stefan,
>
>
>> > +
>> > +#define EPITCR 0x00
>> > +#define EPITSR 0x04
>> > +#define EPITLR 0x08
>> > +#define EPITCMPR 0x0c
>> > +#define EPITCNR 0x10
>> > +
>> > +#define EPITCR_EN BIT(0)
>> > +#define EPITCR_ENMOD BIT(1)
>> > +#define EPITCR_OCIEN BIT(2)
>> > +#define EPITCR_RLD BIT(3)
>> > +#define EPITCR_PRESC(x) (((x) & 0xfff) << 4)
>> > +#define EPITCR_SWR BIT(16)
>> > +#define EPITCR_IOVW BIT(17)
>> > +#define EPITCR_DBGEN BIT(18)
>> > +#define EPITCR_WAITEN BIT(19)
>> > +#define EPITCR_RES BIT(20)
>> > +#define EPITCR_STOPEN BIT(21)
>> > +#define EPITCR_OM_DISCON (0 << 22)
>> > +#define EPITCR_OM_TOGGLE (1 << 22)
>> > +#define EPITCR_OM_CLEAR (2 << 22)
>> > +#define EPITCR_OM_SET (3 << 22)
>> > +#define EPITCR_CLKSRC_OFF (0 << 24)
>> > +#define EPITCR_CLKSRC_PERIPHERAL (1 << 24)
>> > +#define EPITCR_CLKSRC_REF_HIGH (2 << 24)
>> > +#define EPITCR_CLKSRC_REF_LOW (3 << 24)
>> > +
>> > +#define EPITSR_OCIF BIT(0)
>> > +
>> > +struct epit_timer {
>> > + void __iomem *base;
>> > + int irq;
>> > + struct clk *clk;
>> > + struct clock_event_device ced;
>> > + struct irqaction act;
>> > +};
>> > +
>> > +static void __iomem *sched_clock_reg;
>> > +
>> > +static inline struct epit_timer *to_epit_timer(struct clock_event_device *ced)
>> > +{
>> > + return container_of(ced, struct epit_timer, ced);
>> > +}
>> > +
>> > +static inline void epit_irq_disable(struct epit_timer *epittm)
>> > +{
>> > + u32 val;
>> > +
>> > + val = readl_relaxed(epittm->base + EPITCR);
>> > + writel_relaxed(val & ~EPITCR_OCIEN, epittm->base + EPITCR);
>> > +}
>> > +
>> > +static inline void epit_irq_enable(struct epit_timer *epittm)
>> > +{
>> > + u32 val;
>> > +
>> > + val = readl_relaxed(epittm->base + EPITCR);
>> > + writel_relaxed(val | EPITCR_OCIEN, epittm->base + EPITCR);
>> > +}
>> > +
>> > +static void epit_irq_acknowledge(struct epit_timer *epittm)
>> > +{
>> > + writel_relaxed(EPITSR_OCIF, epittm->base + EPITSR);
>> > +}
>> > +
>> > +static u64 notrace epit_read_sched_clock(void)
>> > +{
>> > + return ~readl_relaxed(sched_clock_reg);
>> > +}
>> > +
>> > +static int epit_set_next_event(unsigned long cycles,
>> > + struct clock_event_device *ced)
>> > +{
>> > + struct epit_timer *epittm = to_epit_timer(ced);
>> > + unsigned long tcmp;
>> > +
>> > + tcmp = readl_relaxed(epittm->base + EPITCNR) - cycles;
>> > + writel_relaxed(tcmp, epittm->base + EPITCMPR);
>> > +
>> > + return 0;
>> > +}
>> > +
>> > +/* Left event sources disabled, no more interrupts appear */
>> > +static int epit_shutdown(struct clock_event_device *ced)
>> > +{
>> > + struct epit_timer *epittm = to_epit_timer(ced);
>> > + unsigned long flags;
>> > +
>> > + /*
>> > + * The timer interrupt generation is disabled at least
>> > + * for enough time to call epit_set_next_event()
>> > + */
>> > + local_irq_save(flags);
>> > +
>> > + /* Disable interrupt in EPIT module */
>> > + epit_irq_disable(epittm);
>> > +
>> > + /* Clear pending interrupt */
>> > + epit_irq_acknowledge(epittm);
>> > +
>> > + local_irq_restore(flags);
>> > +
>> > + return 0;
>> > +}
>> > +
>> > +static int epit_set_oneshot(struct clock_event_device *ced)
>> > +{
>> > + struct epit_timer *epittm = to_epit_timer(ced);
>> > + unsigned long flags;
>> > +
>> > + /*
>> > + * The timer interrupt generation is disabled at least
>> > + * for enough time to call epit_set_next_event()
>> > + */
>> > + local_irq_save(flags);
>> > +
>> > + /* Disable interrupt in EPIT module */
>> > + epit_irq_disable(epittm);
>> > +
>> > + /* Clear pending interrupt, only while switching mode */
>> > + if (!clockevent_state_oneshot(ced))
>> > + epit_irq_acknowledge(epittm);
>> > +
>> > + /*
>> > + * Do not put overhead of interrupt enable/disable into
>> > + * epit_set_next_event(), the core has about 4 minutes
>> > + * to call epit_set_next_event() or shutdown clock after
>> > + * mode switching
>> > + */
>> > + epit_irq_enable(epittm);
>> > + local_irq_restore(flags);
>> > +
>> > + return 0;
>> > +}
>> > +
>> > +static irqreturn_t epit_timer_interrupt(int irq, void *dev_id)
>> > +{
>> > + struct clock_event_device *ced = dev_id;
>> > + struct epit_timer *epittm = to_epit_timer(ced);
>> > +
>> > + epit_irq_acknowledge(epittm);
>> > +
>> > + ced->event_handler(ced);
>> > +
>> > + return IRQ_HANDLED;
>> > +}
>> > +
>> > +static int __init epit_clocksource_init(struct epit_timer *epittm)
>> > +{
>> > + unsigned int c = clk_get_rate(epittm->clk);
>> > +
>> > + sched_clock_reg = epittm->base + EPITCNR;
>> > + sched_clock_register(epit_read_sched_clock, 32, c);
>> > +
>> > + return clocksource_mmio_init(epittm->base + EPITCNR, "epit", c, 200, 32,
>> > + clocksource_mmio_readl_down);
>> > +}
>> > +
>> > +static int __init epit_clockevent_init(struct epit_timer *epittm)
>> > +{
>> > + struct clock_event_device *ced = &epittm->ced;
>> > + struct irqaction *act = &epittm->act;
>> > +
>> > + ced->name = "epit";
>> > + ced->features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_DYNIRQ;
>> > + ced->set_state_shutdown = epit_shutdown;
>> > + ced->tick_resume = epit_shutdown;
>> > + ced->set_state_oneshot = epit_set_oneshot;
>> > + ced->set_next_event = epit_set_next_event;
>> > + ced->rating = 200;
>> > + ced->cpumask = cpumask_of(0);
>> > + ced->irq = epittm->irq;
>> > + clockevents_config_and_register(ced, clk_get_rate(epittm->clk),
>> > + 0xff, 0xfffffffe);
>> > +
>> > + act->name = "i.MX EPIT Timer Tick",
>> > + act->flags = IRQF_TIMER | IRQF_IRQPOLL;
>> > + act->handler = epit_timer_interrupt;
>> > + act->dev_id = ced;
>> > +
>> > + /* Make irqs happen */
>> > + return setup_irq(epittm->irq, act);
>> > +}
>> > +
>> > +static int __init epit_timer_init(struct device_node *np)
>> > +{
>> > + struct epit_timer *epittm;
>> > + int ret;
>> > +
>> > + epittm = kzalloc(sizeof(*epittm), GFP_KERNEL);
>> > + if (!epittm)
>> > + return -ENOMEM;
>> > +
>> > + epittm->base = of_iomap(np, 0);
>> > + if (!epittm->base) {
>> > + ret = -ENXIO;
>> > + goto out_kfree;
>> > + }
>> > +
>> > + epittm->irq = irq_of_parse_and_map(np, 0);
>> > + if (!epittm->irq) {
>> > + ret = -EINVAL;
>> > + goto out_iounmap;
>> > + }
>> > +
>> > + /* Get EPIT clock */
>> > + epittm->clk = of_clk_get(np, 0);
>> > + if (IS_ERR(epittm->clk)) {
>> > + pr_err("i.MX EPIT: unable to get clk\n");
>> > + ret = PTR_ERR(epittm->clk);
>> > + goto out_iounmap;
>> > + }
>>
>> There is something off with indent here.
> Thanks for pointing out that, I will fix it.
>
>>
>> There is a helper library in drivers/clocksource/timer-of.c which might
>> be useful for this driver.
> Indeed, but will require a bit of rewrite.
That is fine for me ;-) Afaict, if should be simplify code a quite a
bit, and doable with reasonable amount of work.
It is typically the expectation for new drivers to make use of such
libraries. In the end it is up to the clocksource maintainer(s) to
decide whether they make it as an requirement for this driver.
--
Stefan
>
> Clement
>
>>
>> --
>> Stefan
>>
>> > +
>> > + ret = clk_prepare_enable(epittm->clk);
>> > + if (ret) {
>> > + pr_err("i.MX EPIT: unable to prepare+enable clk\n");
>> > + goto out_iounmap;
>> > + }
>> > +
>> > + /* Initialise to a known state (all timers off, and timing reset) */
>> > + writel_relaxed(0x0, epittm->base + EPITCR);
>> > + writel_relaxed(0xffffffff, epittm->base + EPITLR);
>> > + writel_relaxed(EPITCR_EN | EPITCR_CLKSRC_REF_HIGH | EPITCR_WAITEN,
>> > + epittm->base + EPITCR);
>> > +
>> > + ret = epit_clocksource_init(epittm);
>> > + if (ret) {
>> > + pr_err("i.MX EPIT: failed to init clocksource\n");
>> > + goto out_clk_disable;
>> > + }
>> > +
>> > + ret = epit_clockevent_init(epittm);
>> > + if (ret) {
>> > + pr_err("i.MX EPIT: failed to init clockevent\n");
>> > + goto out_clk_disable;
>> > + }
>> > +
>> > + return 0;
>> > +
>> > +out_clk_disable:
>> > + clk_disable_unprepare(epittm->clk);
>> > +out_iounmap:
>> > + iounmap(epittm->base);
>> > +out_kfree:
>> > + kfree(epittm);
>> > +
>> > + return ret;
>> > +}
>> > +TIMER_OF_DECLARE(epit_timer, "fsl,imx31-epit", epit_timer_init);
^ permalink raw reply
* [PATCH V3] ARM: shmobile: Rework the PMIC IRQ line quirk
From: Geert Uytterhoeven @ 2018-06-11 13:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <fa4f843b-ae9d-fac2-77e2-b3ac1116f916@gmail.com>
Hi Marek,
On Mon, Jun 11, 2018 at 2:15 PM Marek Vasut <marek.vasut@gmail.com> wrote:
> On 06/11/2018 11:56 AM, Geert Uytterhoeven wrote:
> > On Mon, Jun 4, 2018 at 7:59 PM Marek Vasut <marek.vasut@gmail.com> wrote:
> >> Rather than hard-coding the quirk topology, which stopped scaling,
> >> parse the information from DT. The code looks for all compatible
> >> PMICs -- da9036 and da9210 -- and checks if their IRQ line is tied
> >
> > da9063
> >
> >> to the same pin. If so, the code sends a matching sequence to the
> >> PMIC to deassert the IRQ.
> >> @@ -122,7 +143,13 @@ static struct notifier_block regulator_quirk_nb = {
> >>
> >> static int __init rcar_gen2_regulator_quirk(void)
> >> {
> >> - u32 mon;
> >> + struct device_node *np;
> >> + const struct of_device_id *id;
> >> + struct regulator_quirk *quirk;
> >> + struct regulator_quirk *pos;
> >> + struct of_phandle_args *argsa, *argsb;
> >> + u32 mon, addr;
> >> + int ret;
> >
> > Some people prefer "Reverse Christmas Tree Ordering", i.e. longest line first.
> >
> >>
> >> if (!of_machine_is_compatible("renesas,koelsch") &&
> >> !of_machine_is_compatible("renesas,lager") &&
> >> @@ -130,6 +157,45 @@ static int __init rcar_gen2_regulator_quirk(void)
> >> !of_machine_is_compatible("renesas,gose"))
> >> return -ENODEV;
> >
> > I think the board checks above can be removed. That will auto-enable the
> > fix on e.g. Porter (once its regulators have ended up in DTS, of course).
>
> Removing the check would also enable it on boards where we don't want
> this enabled, so I'd prefer to keep the check to avoid strange surprises.
Like, Porter? ;-)
> >> + ret = of_property_read_u32(np, "reg", &addr);
> >> + if (ret)
> >> + return ret;
> >
> > I think it's safer to skip this entry and continue, after calling
> > kfree(quirk), of course.
> >
> >> +
> >> + quirk->id = id;
> >> + quirk->i2c_msg.addr = addr;
> >> + quirk->shared = false;
> >> +
> >> + ret = of_irq_parse_one(np, 0, &quirk->irq_args);
> >> + if (ret)
> >> + return ret;
> >
> > kfree(quirk) and continue...
>
> I wonder if it shouldn't rather free the entire list and abort ?
"Be strict when sending, be liberal when receiving."
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* [PATCH] media: stm32-dcmi: add power saving support
From: kbuild test robot @ 2018-06-11 13:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1528709597-7734-1-git-send-email-hugues.fruchet@st.com>
Hi Hugues,
I love your patch! Yet something to improve:
[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on v4.17 next-20180608]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Hugues-Fruchet/media-stm32-dcmi-add-power-saving-support/20180611-174016
base: git://linuxtv.org/media_tree.git master
config: sparc64-allyesconfig (attached as .config)
compiler: sparc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=sparc64
All errors (new ones prefixed by >>):
drivers/media//platform/stm32/stm32-dcmi.c: In function 'dcmi_suspend':
drivers/media//platform/stm32/stm32-dcmi.c:1886:2: error: implicit declaration of function 'pinctrl_pm_select_sleep_state' [-Werror=implicit-function-declaration]
pinctrl_pm_select_sleep_state(dev);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/media//platform/stm32/stm32-dcmi.c: In function 'dcmi_resume':
>> drivers/media//platform/stm32/stm32-dcmi.c:1894:2: error: implicit declaration of function 'pinctrl_pm_select_default_state'; did you mean 'irq_set_default_host'? [-Werror=implicit-function-declaration]
pinctrl_pm_select_default_state(dev);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
irq_set_default_host
cc1: some warnings being treated as errors
vim +1894 drivers/media//platform/stm32/stm32-dcmi.c
1879
1880 static __maybe_unused int dcmi_suspend(struct device *dev)
1881 {
1882 /* disable clock */
1883 pm_runtime_force_suspend(dev);
1884
1885 /* change pinctrl state */
> 1886 pinctrl_pm_select_sleep_state(dev);
1887
1888 return 0;
1889 }
1890
1891 static __maybe_unused int dcmi_resume(struct device *dev)
1892 {
1893 /* restore pinctl default state */
> 1894 pinctrl_pm_select_default_state(dev);
1895
1896 /* clock enable */
1897 pm_runtime_force_resume(dev);
1898
1899 return 0;
1900 }
1901
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 53703 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180611/742b4bf3/attachment-0001.gz>
^ permalink raw reply
* [PATCH v3] irqchip/gic-v3-its: fix ITS queue timeout
From: Marc Zyngier @ 2018-06-11 13:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1528719909-16572-1-git-send-email-yangyingliang@huawei.com>
On 11/06/18 13:25, Yang Yingliang wrote:
> On a NUMA system, if an ITS is local to an offline
> node, the ITS driver may pick an offline CPU to bind
> the LPI. In this case, we need to pick an online CPU.
> But on some systems, binding LPI to non-local node
> CPU will cause deadlock. In this case, we don't
> bind the LPI to any online CPU and return an error code.
>
> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
> ---
> drivers/irqchip/irq-gic-v3-its.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index 5416f2b..137c433 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -2309,7 +2309,13 @@ static int its_irq_domain_activate(struct irq_domain *domain,
> cpu_mask = cpumask_of_node(its_dev->its->numa_node);
>
> /* Bind the LPI to the first possible CPU */
> - cpu = cpumask_first(cpu_mask);
> + cpu = cpumask_first_and(cpu_mask, cpu_online_mask);
> + if (cpu >= nr_cpu_ids) {
> + if (its_dev->its->flags & ITS_FLAGS_WORKAROUND_CAVIUM_23144)
> + return -EINVAL;
> +
> + cpu = cpumask_first(cpu_online_mask);
> + }
> its_dev->event_map.col_map[event] = cpu;
> irq_data_update_effective_affinity(d, cpumask_of(cpu));
>
>
Queued as a fix for 4.18 with a slightly updated change-log and a cc to
stable.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* [PATCH V3] ARM: shmobile: Rework the PMIC IRQ line quirk
From: Marek Vasut @ 2018-06-11 13:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAMuHMdXw146fwm0tHS7CBAofDAgRgDD2tNqgEJwzx0Jm1xKCyw@mail.gmail.com>
On 06/11/2018 03:03 PM, Geert Uytterhoeven wrote:
> Hi Marek,
Hi,
> On Mon, Jun 11, 2018 at 2:15 PM Marek Vasut <marek.vasut@gmail.com> wrote:
>> On 06/11/2018 11:56 AM, Geert Uytterhoeven wrote:
>>> On Mon, Jun 4, 2018 at 7:59 PM Marek Vasut <marek.vasut@gmail.com> wrote:
>>>> Rather than hard-coding the quirk topology, which stopped scaling,
>>>> parse the information from DT. The code looks for all compatible
>>>> PMICs -- da9036 and da9210 -- and checks if their IRQ line is tied
>>>
>>> da9063
>>>
>>>> to the same pin. If so, the code sends a matching sequence to the
>>>> PMIC to deassert the IRQ.
>
>>>> @@ -122,7 +143,13 @@ static struct notifier_block regulator_quirk_nb = {
>>>>
>>>> static int __init rcar_gen2_regulator_quirk(void)
>>>> {
>>>> - u32 mon;
>>>> + struct device_node *np;
>>>> + const struct of_device_id *id;
>>>> + struct regulator_quirk *quirk;
>>>> + struct regulator_quirk *pos;
>>>> + struct of_phandle_args *argsa, *argsb;
>>>> + u32 mon, addr;
>>>> + int ret;
>>>
>>> Some people prefer "Reverse Christmas Tree Ordering", i.e. longest line first.
>>>
>>>>
>>>> if (!of_machine_is_compatible("renesas,koelsch") &&
>>>> !of_machine_is_compatible("renesas,lager") &&
>>>> @@ -130,6 +157,45 @@ static int __init rcar_gen2_regulator_quirk(void)
>>>> !of_machine_is_compatible("renesas,gose"))
>>>> return -ENODEV;
>>>
>>> I think the board checks above can be removed. That will auto-enable the
>>> fix on e.g. Porter (once its regulators have ended up in DTS, of course).
>>
>> Removing the check would also enable it on boards where we don't want
>> this enabled, so I'd prefer to keep the check to avoid strange surprises.
>
> Like, Porter? ;-)
I'm adding Porter in a separate patch.
>>>> + ret = of_property_read_u32(np, "reg", &addr);
>>>> + if (ret)
>>>> + return ret;
>>>
>>> I think it's safer to skip this entry and continue, after calling
>>> kfree(quirk), of course.
>>>
>>>> +
>>>> + quirk->id = id;
>>>> + quirk->i2c_msg.addr = addr;
>>>> + quirk->shared = false;
>>>> +
>>>> + ret = of_irq_parse_one(np, 0, &quirk->irq_args);
>>>> + if (ret)
>>>> + return ret;
>>>
>>> kfree(quirk) and continue...
>>
>> I wonder if it shouldn't rather free the entire list and abort ?
>
> "Be strict when sending, be liberal when receiving."
Meaning ? I think "the language barrier is protecting me" (TM)
--
Best regards,
Marek Vasut
^ permalink raw reply
* [PATCH RESEND v4 2/2] arm/arm64: KVM: Add KVM_GET/SET_VCPU_EVENTS
From: James Morse @ 2018-06-11 13:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1528487320-2873-3-git-send-email-gengdongjiu@huawei.com>
Hi Dongjiu Geng,
Please only put 'RESEND' in the subject if the patch content is identical.
This patch is not the same as v4.
On 08/06/18 20:48, Dongjiu Geng wrote:
> For the migrating VMs, user space may need to know the exception
> state. For example, in the machine A, KVM make an SError pending,
> when migrate to B, KVM also needs to pend an SError.
>
> This new IOCTL exports user-invisible states related to SError.
> Together with appropriate user space changes, user space can get/set
> the SError exception state to do migrate/snapshot/suspend.
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index fdac969..8896737 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -835,11 +835,13 @@ struct kvm_clock_data {
>
> Capability: KVM_CAP_VCPU_EVENTS
> Extended by: KVM_CAP_INTR_SHADOW
> -Architectures: x86
> +Architectures: x86, arm, arm64
> Type: vm ioctl
Isn't this actually a per-vcpu ioctl? Can we fix the documentation?
> Parameters: struct kvm_vcpu_event (out)
> Returns: 0 on success, -1 on error
>
> +X86:
> +
> Gets currently pending exceptions, interrupts, and NMIs as well as related
> states of the vcpu.
>
> @@ -881,15 +883,32 @@ Only two fields are defined in the flags field:
> - KVM_VCPUEVENT_VALID_SMM may be set in the flags field to signal that
> smi contains a valid state.
>
> +ARM, ARM64:
> +
> +Gets currently pending SError exceptions as well as related states of the vcpu.
> +
> +struct kvm_vcpu_events {
> + struct {
> + __u8 serror_pending;
> + __u8 serror_has_esr;
> + /* Align it to 8 bytes */
> + __u8 pad[6];
> + __u64 serror_esr;
> + } exception;
> + __u32 reserved[12];
> +};
> +
> 4.32 KVM_SET_VCPU_EVENTS
>
> -Capability: KVM_CAP_VCPU_EVENTS
> +Capebility: KVM_CAP_VCPU_EVENTS
(please fix this)
> Extended by: KVM_CAP_INTR_SHADOW
> -Architectures: x86
> +Architectures: x86, arm, arm64
> Type: vm ioctl
(this is also a vcpu ioctl)
> Parameters: struct kvm_vcpu_event (in)
> Returns: 0 on success, -1 on error
>
> +X86:
> +
> Set pending exceptions, interrupts, and NMIs as well as related states of the
> vcpu.
>
> @@ -910,6 +929,12 @@ shall be written into the VCPU.
>
> KVM_VCPUEVENT_VALID_SMM can only be set if KVM_CAP_X86_SMM is available.
>
> +ARM, ARM64:
> +
> +Set pending SError exceptions as well as related states of the vcpu.
There are some deliberate choices here I think we should document:
| This API can't be used to clear a pending SError.
If there already was an SError pending, this API just overwrites it with the new
one. The architecture has some rules about merging multiple SError. (details in
2.5.3 Multiple SError interrupts of [0])
I don't think KVM needs to enforce these, as they are implementation-defined if
one of the ESR is implementation-defined... the part that matters is reporting
the 'most severe' RAS ESR if there are multiple pending. As only user-space ever
sets these, let's make it user-spaces problem to do.
I think we should recommend user-space always reads the pending values and
applies its merging-multiple-SError logic. (I assume your Qemu patches do this).
Something like:
| User-space should first use KVM_GET_VCPU_EVENTS in case KVM has made an SError
| pending as part of its device emulation. When both values are architected RAS
| SError ESR values, the new ESR should reflect the combined effect of both
| errors.
> diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h
> index caae484..c3e6975 100644
> --- a/arch/arm/include/uapi/asm/kvm.h
> +++ b/arch/arm/include/uapi/asm/kvm.h
> @@ -124,6 +124,18 @@ struct kvm_sync_regs {
> struct kvm_arch_memory_slot {
> };
>
> +/* for KVM_GET/SET_VCPU_EVENTS */
> +struct kvm_vcpu_events {
> + struct {
> + __u8 serror_pending;
> + __u8 serror_has_esr;
> + /* Align it to 8 bytes */
> + __u8 pad[6];
> + __u64 serror_esr;
> + } exception;
> + __u32 reserved[12];
> +};
> +
You haven't defined __KVM_HAVE_VCPU_EVENTS for 32bit, so presumably this struct
will never be used. Why is it here?
(I agree if we ever provide it on 32bit, the struct layout should be the same.
Is this only here to force that to happen?)
[...]
> +int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
> + struct kvm_vcpu_events *events)
> +{
> + bool serror_pending = events->exception.serror_pending;
> + bool has_esr = events->exception.serror_has_esr;
> +
> + if (serror_pending && has_esr) {
> + if (!cpus_have_const_cap(ARM64_HAS_RAS_EXTN))
> + return -EINVAL;
> +
> + kvm_set_sei_esr(vcpu, events->exception.serror_esr);
kvm_set_sei_esr() will silently discard the top 40 bits of serror_esr, (which is
correct, we shouldn't copy them into hardware without know what they do).
Could we please force user-space to zero these bits, we can advertise extra CAPs
if new features turn up in that space, instead of user-space passing <something>
and relying on the kernel to remove it.
(Background: VSESR is a 64bit register that holds the value to go in a 32bit
register. I suspect the top-half could get re-used for control values or
something we don't want to give user-space)
> diff --git a/arch/arm64/kvm/inject_fault.c b/arch/arm64/kvm/inject_fault.c
> index d8e7165..a55e91d 100644
> --- a/arch/arm64/kvm/inject_fault.c
> +++ b/arch/arm64/kvm/inject_fault.c
> @@ -164,9 +164,9 @@ void kvm_inject_undefined(struct kvm_vcpu *vcpu)
> inject_undef64(vcpu);
> }
>
> -static void pend_guest_serror(struct kvm_vcpu *vcpu, u64 esr)
> +void kvm_set_sei_esr(struct kvm_vcpu *vcpu, u64 esr)
> {
> - vcpu_set_vsesr(vcpu, esr);
> + vcpu_set_vsesr(vcpu, esr & ESR_ELx_ISS_MASK);
> *vcpu_hcr(vcpu) |= HCR_VSE;
> }
>
> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> index a4c1b76..79ecba9 100644
> --- a/virt/kvm/arm/arm.c
> +++ b/virt/kvm/arm/arm.c
> @@ -1107,6 +1107,25 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
> + case KVM_SET_VCPU_EVENTS: {
> + struct kvm_vcpu_events events;
> +
> + if (copy_from_user(&events, argp, sizeof(events)))
> + return -EFAULT;
> +
> + return kvm_arm_vcpu_set_events(vcpu, &events);
> + }
Please check the padding[] and reserved[] are zero, otherwise we can't re-use these.
Thanks,
James
[0]
https://static.docs.arm.com/ddi0587/a/RAS%20Extension-release%20candidate_march_29.pdf
^ permalink raw reply
* [PATCH RESEND v4 2/2] arm/arm64: KVM: Add KVM_GET/SET_VCPU_EVENTS
From: James Morse @ 2018-06-11 13:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <86zi04875t.wl-marc.zyngier@arm.com>
Hi Dongjiu Geng,
On 09/06/18 13:40, Marc Zyngier wrote:
> On Fri, 08 Jun 2018 20:48:40 +0100, Dongjiu Geng wrote:
>> For the migrating VMs, user space may need to know the exception
>> state. For example, in the machine A, KVM make an SError pending,
>> when migrate to B, KVM also needs to pend an SError.
>>
>> This new IOCTL exports user-invisible states related to SError.
>> Together with appropriate user space changes, user space can get/set
>> the SError exception state to do migrate/snapshot/suspend.
>> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
>> index 04b3256..df4faee 100644
>> --- a/arch/arm64/include/uapi/asm/kvm.h
>> +++ b/arch/arm64/include/uapi/asm/kvm.h
>> @@ -153,6 +154,18 @@ struct kvm_sync_regs {
>> struct kvm_arch_memory_slot {
>> };
>>
>> +/* for KVM_GET/SET_VCPU_EVENTS */
>> +struct kvm_vcpu_events {
>> + struct {
>> + __u8 serror_pending;
>> + __u8 serror_has_esr;
>> + /* Align it to 8 bytes */
>> + __u8 pad[6];
>> + __u64 serror_esr;
>> + } exception;
>> + __u32 reserved[12];
>> +};
>> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
>> index 56a0260..4426915 100644
>> --- a/arch/arm64/kvm/guest.c
>> +++ b/arch/arm64/kvm/guest.c
>> +int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
>> + struct kvm_vcpu_events *events)
>> +{
>> + bool serror_pending = events->exception.serror_pending;
>> + bool has_esr = events->exception.serror_has_esr;
>> +
>> + if (serror_pending && has_esr) {
>> + if (!cpus_have_const_cap(ARM64_HAS_RAS_EXTN))
>> + return -EINVAL;
>> +
>> + kvm_set_sei_esr(vcpu, events->exception.serror_esr);
>> + } else if (serror_pending) {
>> + kvm_inject_vabt(vcpu);
>> + }
>> +
>> + return 0;
>
> There was an earlier request to check that all the padding is set to
> zero. I still think this makes sense.
I agree, not just the exception.padding[], but reserved[] too.
Thanks,
James
^ permalink raw reply
* [PATCH 1/2] arm64: avoid alloc memory on offline node
From: Bjorn Helgaas @ 2018-06-11 13:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <16c4db2f-bc70-d0f2-fb38-341d9117ff66@huawei.com>
On Mon, Jun 11, 2018 at 08:32:10PM +0800, Xie XiuQi wrote:
> Hi Michal,
>
> On 2018/6/11 16:52, Michal Hocko wrote:
> > On Mon 11-06-18 11:23:18, Xie XiuQi wrote:
> >> Hi Michal,
> >>
> >> On 2018/6/7 20:21, Michal Hocko wrote:
> >>> On Thu 07-06-18 19:55:53, Hanjun Guo wrote:
> >>>> On 2018/6/7 18:55, Michal Hocko wrote:
> >>> [...]
> >>>>> I am not sure I have the full context but pci_acpi_scan_root calls
> >>>>> kzalloc_node(sizeof(*info), GFP_KERNEL, node)
> >>>>> and that should fall back to whatever node that is online. Offline node
> >>>>> shouldn't keep any pages behind. So there must be something else going
> >>>>> on here and the patch is not the right way to handle it. What does
> >>>>> faddr2line __alloc_pages_nodemask+0xf0 tells on this kernel?
> >>>>
> >>>> The whole context is:
> >>>>
> >>>> The system is booted with a NUMA node has no memory attaching to it
> >>>> (memory-less NUMA node), also with NR_CPUS less than CPUs presented
> >>>> in MADT, so CPUs on this memory-less node are not brought up, and
> >>>> this NUMA node will not be online (but SRAT presents this NUMA node);
> >>>>
> >>>> Devices attaching to this NUMA node such as PCI host bridge still
> >>>> return the valid NUMA node via _PXM, but actually that valid NUMA node
> >>>> is not online which lead to this issue.
> >>>
> >>> But we should have other numa nodes on the zonelists so the allocator
> >>> should fall back to other node. If the zonelist is not intiailized
> >>> properly, though, then this can indeed show up as a problem. Knowing
> >>> which exact place has blown up would help get a better picture...
> >>>
> >>
> >> I specific a non-exist node to allocate memory using kzalloc_node,
> >> and got this following error message.
> >>
> >> And I found out there is just a VM_WARN, but it does not prevent the memory
> >> allocation continue.
> >>
> >> This nid would be use to access NODE_DADA(nid), so if nid is invalid,
> >> it would cause oops here.
> >>
> >> 459 /*
> >> 460 * Allocate pages, preferring the node given as nid. The node must be valid and
> >> 461 * online. For more general interface, see alloc_pages_node().
> >> 462 */
> >> 463 static inline struct page *
> >> 464 __alloc_pages_node(int nid, gfp_t gfp_mask, unsigned int order)
> >> 465 {
> >> 466 VM_BUG_ON(nid < 0 || nid >= MAX_NUMNODES);
> >> 467 VM_WARN_ON(!node_online(nid));
> >> 468
> >> 469 return __alloc_pages(gfp_mask, order, nid);
> >> 470 }
> >> 471
> >>
> >> (I wrote a ko, to allocate memory on a non-exist node using kzalloc_node().)
> >
> > OK, so this is an artificialy broken code, right. You shouldn't get a
> > non-existent node via standard APIs AFAICS. The original report was
> > about an existing node which is offline AFAIU. That would be a different
> > case. If I am missing something and there are legitimate users that try
> > to allocate from non-existing nodes then we should handle that in
> > node_zonelist.
>
> I think hanjun's comments may help to understood this question:
> - NUMA node will be built if CPUs and (or) memory are valid on this NUMA
> node;
>
> - But if we boot the system with memory-less node and also with
> CONFIG_NR_CPUS less than CPUs in SRAT, for example, 64 CPUs total with 4
> NUMA nodes, 16 CPUs on each NUMA node, if we boot with
> CONFIG_NR_CPUS=48, then we will not built numa node for node 3, but with
> devices on that numa node, alloc memory will be panic because NUMA node
> 3 is not a valid node.
>
> I triggered this BUG on arm64 platform, and I found a similar bug has
> been fixed on x86 platform. So I sent a similar patch for this bug.
>
> Or, could we consider to fix it in the mm subsystem?
The patch below (b755de8dfdfe) seems like totally the wrong direction.
I don't think we want every caller of kzalloc_node() to have check for
node_online().
Why would memory on an off-line node even be in the allocation pool?
I wouldn't expect that memory to be put in the pool until the node
comes online and the memory is accessible, so this sounds like some
kind of setup issue.
But I'm definitely not an mm person.
> From b755de8dfdfef97effaa91379ffafcb81f4d62a1 Mon Sep 17 00:00:00 2001
> From: Yinghai Lu <Yinghai.Lu@Sun.COM>
> Date: Wed, 20 Feb 2008 12:41:52 -0800
> Subject: [PATCH] x86: make dev_to_node return online node
>
> a numa system (with multi HT chains) may return node without ram. Aka it
> is not online. Try to get an online node, otherwise return -1.
>
> Signed-off-by: Yinghai Lu <yinghai.lu@sun.com>
> Signed-off-by: Ingo Molnar <mingo@elte.hu>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
> arch/x86/pci/acpi.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
> index d95de2f..ea8685f 100644
> --- a/arch/x86/pci/acpi.c
> +++ b/arch/x86/pci/acpi.c
> @@ -172,6 +172,9 @@ struct pci_bus * __devinit pci_acpi_scan_root(struct acpi_device *device, int do
> set_mp_bus_to_node(busnum, node);
> else
> node = get_mp_bus_to_node(busnum);
> +
> + if (node != -1 && !node_online(node))
> + node = -1;
> #endif
>
> /* Allocate per-root-bus (not per bus) arch-specific data.
^ permalink raw reply
* [PATCH V3] ARM: shmobile: Rework the PMIC IRQ line quirk
From: Geert Uytterhoeven @ 2018-06-11 13:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <967b800e-c935-aa35-da5a-ca3672fd18c2@gmail.com>
Hi Marek,
On Mon, Jun 11, 2018 at 3:39 PM Marek Vasut <marek.vasut@gmail.com> wrote:
> On 06/11/2018 03:03 PM, Geert Uytterhoeven wrote:
> > On Mon, Jun 11, 2018 at 2:15 PM Marek Vasut <marek.vasut@gmail.com> wrote:
> >> On 06/11/2018 11:56 AM, Geert Uytterhoeven wrote:
> >>> On Mon, Jun 4, 2018 at 7:59 PM Marek Vasut <marek.vasut@gmail.com> wrote:
> >>>> Rather than hard-coding the quirk topology, which stopped scaling,
> >>>> parse the information from DT. The code looks for all compatible
> >>>> PMICs -- da9036 and da9210 -- and checks if their IRQ line is tied
> >>>> to the same pin. If so, the code sends a matching sequence to the
> >>>> PMIC to deassert the IRQ.
> >>>> + ret = of_property_read_u32(np, "reg", &addr);
> >>>> + if (ret)
> >>>> + return ret;
> >>>
> >>> I think it's safer to skip this entry and continue, after calling
> >>> kfree(quirk), of course.
> >>>
> >>>> +
> >>>> + quirk->id = id;
> >>>> + quirk->i2c_msg.addr = addr;
> >>>> + quirk->shared = false;
> >>>> +
> >>>> + ret = of_irq_parse_one(np, 0, &quirk->irq_args);
> >>>> + if (ret)
> >>>> + return ret;
> >>>
> >>> kfree(quirk) and continue...
> >>
> >> I wonder if it shouldn't rather free the entire list and abort ?
> >
> > "Be strict when sending, be liberal when receiving."
>
> Meaning ? I think "the language barrier is protecting me" (TM)
Do the best you can, given the buggy DT you received.
I.e. don't fail completely, just ignore the bad device node, and continue.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* [PATCH v2 5/5] arm64: perf: Add support for chaining event counters
From: Suzuki K Poulose @ 2018-06-11 13:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4a5b5e7f-fc0b-84e3-fc65-b9f860029207@arm.com>
On 08/06/18 15:46, Suzuki K Poulose wrote:
> Hi Mark,
>
> On 06/06/2018 07:01 PM, Mark Rutland wrote:
>> On Tue, May 29, 2018 at 11:55:56AM +0100, Suzuki K Poulose wrote:
>
>>> -??????? value |= 0xffffffff00000000ULL;
>>> +??????? if (!armv8pmu_event_is_64bit(event))
>>> +??????????? value |= 0xffffffff00000000ULL;
>>> ????????? write_sysreg(value, pmccntr_el0);
>>> -??? } else if (armv8pmu_select_counter(idx) == idx)
>>> -??????? write_sysreg(value, pmxevcntr_el0);
>>> +??? } else
>>> +??????? armv8pmu_write_hw_counter(event, value);
>>> ? }
>>
>>> +static inline void armv8pmu_write_event_type(struct perf_event *event)
>>> +{
>>> +??? struct hw_perf_event *hwc = &event->hw;
>>> +??? int idx = hwc->idx;
>>> +
>>> +??? /*
>>> +???? * For chained events, write the the low counter event type
>>> +???? * followed by the high counter. The high counter is programmed
>>> +???? * with CHAIN event code with filters set to count at all ELs.
>>> +???? */
>>> +??? if (armv8pmu_event_is_chained(event)) {
>>> +??????? u32 chain_evt = ARMV8_PMUV3_PERFCTR_CHAIN |
>>> +??????????????? ARMV8_PMU_INCLUDE_EL2;
>>> +
>>> +??????? armv8pmu_write_evtype(idx - 1, hwc->config_base);
>>> +??????? isb();
>>> +??????? armv8pmu_write_evtype(idx, chain_evt);
>>
>> The ISB isn't necessary here, AFAICT. We only do this while the PMU is
>> disabled; no?
>
> You're right. I was just following the ARM ARM.
Taking another look, it is not clear about the semantics of "pmu->enable()"
and pmu->disable() callbacks. I don't see any reference to them in the perf core
driver anymore. The perf core uses add() / del () instead, with the PMU
turned off. Do you have any idea about the enable()/disable() callbacks ?
Am I missing something ?
Suzuki
^ permalink raw reply
* [PATCH V3] ARM: shmobile: Rework the PMIC IRQ line quirk
From: Marek Vasut @ 2018-06-11 14:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAMuHMdVJ+ad-kr0iYuoaH3chJ49_g6yjEb8PHmTpwjFdYcG4=Q@mail.gmail.com>
On 06/11/2018 03:49 PM, Geert Uytterhoeven wrote:
> Hi Marek,
Hi,
> On Mon, Jun 11, 2018 at 3:39 PM Marek Vasut <marek.vasut@gmail.com> wrote:
>> On 06/11/2018 03:03 PM, Geert Uytterhoeven wrote:
>>> On Mon, Jun 11, 2018 at 2:15 PM Marek Vasut <marek.vasut@gmail.com> wrote:
>>>> On 06/11/2018 11:56 AM, Geert Uytterhoeven wrote:
>>>>> On Mon, Jun 4, 2018 at 7:59 PM Marek Vasut <marek.vasut@gmail.com> wrote:
>>>>>> Rather than hard-coding the quirk topology, which stopped scaling,
>>>>>> parse the information from DT. The code looks for all compatible
>>>>>> PMICs -- da9036 and da9210 -- and checks if their IRQ line is tied
>>>>>> to the same pin. If so, the code sends a matching sequence to the
>>>>>> PMIC to deassert the IRQ.
>
>>>>>> + ret = of_property_read_u32(np, "reg", &addr);
>>>>>> + if (ret)
>>>>>> + return ret;
>>>>>
>>>>> I think it's safer to skip this entry and continue, after calling
>>>>> kfree(quirk), of course.
>>>>>
>>>>>> +
>>>>>> + quirk->id = id;
>>>>>> + quirk->i2c_msg.addr = addr;
>>>>>> + quirk->shared = false;
>>>>>> +
>>>>>> + ret = of_irq_parse_one(np, 0, &quirk->irq_args);
>>>>>> + if (ret)
>>>>>> + return ret;
>>>>>
>>>>> kfree(quirk) and continue...
>>>>
>>>> I wonder if it shouldn't rather free the entire list and abort ?
>>>
>>> "Be strict when sending, be liberal when receiving."
>>
>> Meaning ? I think "the language barrier is protecting me" (TM)
>
> Do the best you can, given the buggy DT you received.
> I.e. don't fail completely, just ignore the bad device node, and continue.
But if you ignore node, you might as well ignore one which is shared and
then the system crashes due to IRQ storm anyway. So hum, what can we do ?
--
Best regards,
Marek Vasut
^ permalink raw reply
* [PATCH V3] ARM: shmobile: Rework the PMIC IRQ line quirk
From: Geert Uytterhoeven @ 2018-06-11 14:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <47e87deb-a7a8-4780-53b5-4e4ed6e1bac3@gmail.com>
Hi Marek,
On Mon, Jun 11, 2018 at 4:04 PM Marek Vasut <marek.vasut@gmail.com> wrote:
> On 06/11/2018 03:49 PM, Geert Uytterhoeven wrote:
> > On Mon, Jun 11, 2018 at 3:39 PM Marek Vasut <marek.vasut@gmail.com> wrote:
> >> On 06/11/2018 03:03 PM, Geert Uytterhoeven wrote:
> >>> On Mon, Jun 11, 2018 at 2:15 PM Marek Vasut <marek.vasut@gmail.com> wrote:
> >>>> On 06/11/2018 11:56 AM, Geert Uytterhoeven wrote:
> >>>>> On Mon, Jun 4, 2018 at 7:59 PM Marek Vasut <marek.vasut@gmail.com> wrote:
> >>>>>> Rather than hard-coding the quirk topology, which stopped scaling,
> >>>>>> parse the information from DT. The code looks for all compatible
> >>>>>> PMICs -- da9036 and da9210 -- and checks if their IRQ line is tied
> >>>>>> to the same pin. If so, the code sends a matching sequence to the
> >>>>>> PMIC to deassert the IRQ.
> >
> >>>>>> + ret = of_property_read_u32(np, "reg", &addr);
> >>>>>> + if (ret)
> >>>>>> + return ret;
> >>>>>
> >>>>> I think it's safer to skip this entry and continue, after calling
> >>>>> kfree(quirk), of course.
> >>>>>
> >>>>>> +
> >>>>>> + quirk->id = id;
> >>>>>> + quirk->i2c_msg.addr = addr;
> >>>>>> + quirk->shared = false;
> >>>>>> +
> >>>>>> + ret = of_irq_parse_one(np, 0, &quirk->irq_args);
> >>>>>> + if (ret)
> >>>>>> + return ret;
> >>>>>
> >>>>> kfree(quirk) and continue...
> >>>>
> >>>> I wonder if it shouldn't rather free the entire list and abort ?
> >>>
> >>> "Be strict when sending, be liberal when receiving."
> >>
> >> Meaning ? I think "the language barrier is protecting me" (TM)
> >
> > Do the best you can, given the buggy DT you received.
> > I.e. don't fail completely, just ignore the bad device node, and continue.
>
> But if you ignore node, you might as well ignore one which is shared and
> then the system crashes due to IRQ storm anyway. So hum, what can we do ?
Correct. If it's a critical node, it will crash regardless.
If it's a non-critical node, you have the choice between aborting and crashing,
or ignoring and keeping the system alive. Your call.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* [PATCH] clk: imx6: fix video_27m parent for IMX6QDL_CLK_CKO1_SEL
From: Philipp Puschmann @ 2018-06-11 14:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAOMZO5C8CA02V+oiag5tGpKBWN+UP28KwQ98jQ9PpQ2_aCq_bA@mail.gmail.com>
> Hi Philipp,
>
> On Fri, Jun 8, 2018 at 7:19 AM, Philipp Puschmann <pp@emlix.com> wrote:
>> q/dl datasheets list the 5th selection value for ck01_sel as
>> video_27M_clk_root.
>>
>> By replacing the dummy value we then can set IMX6QDL_CLK_VIDEO_27M
>> as parent for IMX6QDL_CLK_CKO1_SEL.
>>
>> Signed-off-by: Philipp Puschmann <pp@emlix.com>
>
> You could still have added my Reviewed-by tag that I sent previously :-)
>
> Here it goes:
>
> Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
>
> Also in the future, when posting a new version of a patch, please mark
> it as such:
>
> [PATCH v2] clk: imx6: fix video_27m parent for IMX6QDL_CLK_CKO1_SEL
>
> and write below the --- line what are the changes in this new version.
>
> Thanks
>
Hi Fabio,
thank you for your hints, next time i will do it that way
^ permalink raw reply
* [PATCH V3] ARM: shmobile: Rework the PMIC IRQ line quirk
From: Marek Vasut @ 2018-06-11 14:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAMuHMdXJEC8HbuB5+YHMLjZy9GfCYyXmGTk0TXkoAxLhxTOJyA@mail.gmail.com>
On 06/11/2018 04:10 PM, Geert Uytterhoeven wrote:
> Hi Marek,
Hi,
> On Mon, Jun 11, 2018 at 4:04 PM Marek Vasut <marek.vasut@gmail.com> wrote:
>> On 06/11/2018 03:49 PM, Geert Uytterhoeven wrote:
>>> On Mon, Jun 11, 2018 at 3:39 PM Marek Vasut <marek.vasut@gmail.com> wrote:
>>>> On 06/11/2018 03:03 PM, Geert Uytterhoeven wrote:
>>>>> On Mon, Jun 11, 2018 at 2:15 PM Marek Vasut <marek.vasut@gmail.com> wrote:
>>>>>> On 06/11/2018 11:56 AM, Geert Uytterhoeven wrote:
>>>>>>> On Mon, Jun 4, 2018 at 7:59 PM Marek Vasut <marek.vasut@gmail.com> wrote:
>>>>>>>> Rather than hard-coding the quirk topology, which stopped scaling,
>>>>>>>> parse the information from DT. The code looks for all compatible
>>>>>>>> PMICs -- da9036 and da9210 -- and checks if their IRQ line is tied
>>>>>>>> to the same pin. If so, the code sends a matching sequence to the
>>>>>>>> PMIC to deassert the IRQ.
>>>
>>>>>>>> + ret = of_property_read_u32(np, "reg", &addr);
>>>>>>>> + if (ret)
>>>>>>>> + return ret;
>>>>>>>
>>>>>>> I think it's safer to skip this entry and continue, after calling
>>>>>>> kfree(quirk), of course.
>>>>>>>
>>>>>>>> +
>>>>>>>> + quirk->id = id;
>>>>>>>> + quirk->i2c_msg.addr = addr;
>>>>>>>> + quirk->shared = false;
>>>>>>>> +
>>>>>>>> + ret = of_irq_parse_one(np, 0, &quirk->irq_args);
>>>>>>>> + if (ret)
>>>>>>>> + return ret;
>>>>>>>
>>>>>>> kfree(quirk) and continue...
>>>>>>
>>>>>> I wonder if it shouldn't rather free the entire list and abort ?
>>>>>
>>>>> "Be strict when sending, be liberal when receiving."
>>>>
>>>> Meaning ? I think "the language barrier is protecting me" (TM)
>>>
>>> Do the best you can, given the buggy DT you received.
>>> I.e. don't fail completely, just ignore the bad device node, and continue.
>>
>> But if you ignore node, you might as well ignore one which is shared and
>> then the system crashes due to IRQ storm anyway. So hum, what can we do ?
>
> Correct. If it's a critical node, it will crash regardless.
> If it's a non-critical node, you have the choice between aborting and crashing,
> or ignoring and keeping the system alive. Your call.
But wait, since we control which machines this code runs on , can't we
assure they have valid DTs ? This situation with invalid DT starts to
look a bit hypothetical to me.
--
Best regards,
Marek Vasut
^ permalink raw reply
* [PATCH v2 5/5] arm64: perf: Add support for chaining event counters
From: Mark Rutland @ 2018-06-11 14:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <847dfe5c-665c-6398-f87f-3ca56e73f5aa@arm.com>
On Mon, Jun 11, 2018 at 02:54:16PM +0100, Suzuki K Poulose wrote:
> On 08/06/18 15:46, Suzuki K Poulose wrote:
> > On 06/06/2018 07:01 PM, Mark Rutland wrote:
> > > On Tue, May 29, 2018 at 11:55:56AM +0100, Suzuki K Poulose wrote:
>
> > > > -??????? value |= 0xffffffff00000000ULL;
> > > > +??????? if (!armv8pmu_event_is_64bit(event))
> > > > +??????????? value |= 0xffffffff00000000ULL;
> > > > ????????? write_sysreg(value, pmccntr_el0);
> > > > -??? } else if (armv8pmu_select_counter(idx) == idx)
> > > > -??????? write_sysreg(value, pmxevcntr_el0);
> > > > +??? } else
> > > > +??????? armv8pmu_write_hw_counter(event, value);
> > > > ? }
> > >
> > > > +static inline void armv8pmu_write_event_type(struct perf_event *event)
> > > > +{
> > > > +??? struct hw_perf_event *hwc = &event->hw;
> > > > +??? int idx = hwc->idx;
> > > > +
> > > > +??? /*
> > > > +???? * For chained events, write the the low counter event type
> > > > +???? * followed by the high counter. The high counter is programmed
> > > > +???? * with CHAIN event code with filters set to count at all ELs.
> > > > +???? */
> > > > +??? if (armv8pmu_event_is_chained(event)) {
> > > > +??????? u32 chain_evt = ARMV8_PMUV3_PERFCTR_CHAIN |
> > > > +??????????????? ARMV8_PMU_INCLUDE_EL2;
> > > > +
> > > > +??????? armv8pmu_write_evtype(idx - 1, hwc->config_base);
> > > > +??????? isb();
> > > > +??????? armv8pmu_write_evtype(idx, chain_evt);
> > >
> > > The ISB isn't necessary here, AFAICT. We only do this while the PMU is
> > > disabled; no?
> >
> > You're right. I was just following the ARM ARM.
>
> Taking another look, it is not clear about the semantics of "pmu->enable()"
> and pmu->disable() callbacks.
I was talking about pmu::{pmu_disable,pmu_enable}(), so I'm not sure I
follow how arm_pmu::{enable,disable}() are relevant here.
The arm_pmu::{enable,disable}() callbacks enable or disable individual
counters. For example, leaving unused counters disabled may save power,
even if the PMU as a whole is enabled.
> I don't see any reference to them in the perf core
> driver anymore. The perf core uses add() / del () instead, with the PMU
> turned off. Do you have any idea about the enable()/disable() callbacks ?
I'm not sure I understand what you're asking here.
Thanks,
Mark.
^ permalink raw reply
* [PATCH V3] ARM: shmobile: Rework the PMIC IRQ line quirk
From: Geert Uytterhoeven @ 2018-06-11 14:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <6864ed2c-39ac-615d-f38e-b28c3647e451@gmail.com>
Hi Marek,
On Mon, Jun 11, 2018 at 4:19 PM Marek Vasut <marek.vasut@gmail.com> wrote:
> On 06/11/2018 04:10 PM, Geert Uytterhoeven wrote:
> > On Mon, Jun 11, 2018 at 4:04 PM Marek Vasut <marek.vasut@gmail.com> wrote:
> >> On 06/11/2018 03:49 PM, Geert Uytterhoeven wrote:
> >>> On Mon, Jun 11, 2018 at 3:39 PM Marek Vasut <marek.vasut@gmail.com> wrote:
> >>>> On 06/11/2018 03:03 PM, Geert Uytterhoeven wrote:
> >>>>> On Mon, Jun 11, 2018 at 2:15 PM Marek Vasut <marek.vasut@gmail.com> wrote:
> >>>>>> On 06/11/2018 11:56 AM, Geert Uytterhoeven wrote:
> >>>>>>> On Mon, Jun 4, 2018 at 7:59 PM Marek Vasut <marek.vasut@gmail.com> wrote:
> >>>>>>>> Rather than hard-coding the quirk topology, which stopped scaling,
> >>>>>>>> parse the information from DT. The code looks for all compatible
> >>>>>>>> PMICs -- da9036 and da9210 -- and checks if their IRQ line is tied
> >>>>>>>> to the same pin. If so, the code sends a matching sequence to the
> >>>>>>>> PMIC to deassert the IRQ.
> >>>
> >>>>>>>> + ret = of_property_read_u32(np, "reg", &addr);
> >>>>>>>> + if (ret)
> >>>>>>>> + return ret;
> >>>>>>>
> >>>>>>> I think it's safer to skip this entry and continue, after calling
> >>>>>>> kfree(quirk), of course.
> >>>>>>>
> >>>>>>>> +
> >>>>>>>> + quirk->id = id;
> >>>>>>>> + quirk->i2c_msg.addr = addr;
> >>>>>>>> + quirk->shared = false;
> >>>>>>>> +
> >>>>>>>> + ret = of_irq_parse_one(np, 0, &quirk->irq_args);
> >>>>>>>> + if (ret)
> >>>>>>>> + return ret;
> >>>>>>>
> >>>>>>> kfree(quirk) and continue...
> >>>>>>
> >>>>>> I wonder if it shouldn't rather free the entire list and abort ?
> >>>>>
> >>>>> "Be strict when sending, be liberal when receiving."
> >>>>
> >>>> Meaning ? I think "the language barrier is protecting me" (TM)
> >>>
> >>> Do the best you can, given the buggy DT you received.
> >>> I.e. don't fail completely, just ignore the bad device node, and continue.
> >>
> >> But if you ignore node, you might as well ignore one which is shared and
> >> then the system crashes due to IRQ storm anyway. So hum, what can we do ?
> >
> > Correct. If it's a critical node, it will crash regardless.
> > If it's a non-critical node, you have the choice between aborting and crashing,
> > or ignoring and keeping the system alive. Your call.
>
> But wait, since we control which machines this code runs on , can't we
> assure they have valid DTs ? This situation with invalid DT starts to
> look a bit hypothetical to me.
That assumes you keep the list of machines to check, and don't want to fix the
issue automatically when detected (on any R-Car Gen2 or RZ/G1 platform, so
you still need to check for r8a779[0-4] and r8a774[23457]).
Anyway, as we care about booting old DTBs on new kernels (for a while), we
have a few more release cycles to bikeshed ;-)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* [PATCH v6 0/5] Reintroduce i.MX EPIT Timer
From: Vladimir Zapolskiy @ 2018-06-11 14:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180607140544.22268-1-peron.clem@gmail.com>
Hi Cl?ment,
On 06/07/2018 05:05 PM, Cl?ment P?ron wrote:
> From: Cl?ment Peron <clement.peron@devialet.com>
>
> As suggested in the commit message we have added the device tree support,
> proper bindings and we moved the driver into the correct folder.
>
> Moreover we made some changes like use of relaxed IO accesor,
> implement sched_clock, delay_timer and reduce the clockevents min_delta.
>
I reviewed and tested the driver on i.MX31, as expected it works fine,
and I'll give my tags per a commit, please add them to v7 changes.
--
Best wishes,
Vladimir
^ permalink raw reply
* [PATCH v6 2/5] ARM: imx: remove inexistant EPIT timer init
From: Vladimir Zapolskiy @ 2018-06-11 14:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180607140544.22268-3-peron.clem@gmail.com>
On 06/07/2018 05:05 PM, Cl?ment P?ron wrote:
> From: Cl?ment Peron <clement.peron@devialet.com>
>
> i.MX EPIT timer has been removed but not the init function declaration.
>
> Signed-off-by: Cl?ment Peron <clement.peron@devialet.com>
> Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
Reviewed-by: Vladimir Zapolskiy <vz@mleia.com>
Tested-by: Vladimir Zapolskiy <vz@mleia.com>
--
With best wishes,
Vladimir
^ permalink raw reply
* [PATCH v6 3/5] dt-bindings: timer: add i.MX EPIT timer binding
From: Vladimir Zapolskiy @ 2018-06-11 14:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180607140544.22268-4-peron.clem@gmail.com>
On 06/07/2018 05:05 PM, Cl?ment P?ron wrote:
> From: Cl?ment Peron <clement.peron@devialet.com>
>
> Add devicetree binding document for NXP's i.MX SoC specific
> EPIT timer driver.
>
> Signed-off-by: Cl?ment Peron <clement.peron@devialet.com>
Reviewed-by: Vladimir Zapolskiy <vz@mleia.com>
--
With best wishes,
Vladimir
^ permalink raw reply
* [PATCH v6 4/5] clocksource: add driver for i.MX EPIT timer
From: Vladimir Zapolskiy @ 2018-06-11 14:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180607140544.22268-5-peron.clem@gmail.com>
On 06/07/2018 05:05 PM, Cl?ment P?ron wrote:
> From: Colin Didier <colin.didier@devialet.com>
>
> Add driver for NXP's EPIT timer used in i.MX SoC.
>
> Signed-off-by: Colin Didier <colin.didier@devialet.com>
> Signed-off-by: Cl?ment Peron <clement.peron@devialet.com>
Reviewed-by: Vladimir Zapolskiy <vz@mleia.com>
Tested-by: Vladimir Zapolskiy <vz@mleia.com>
I tested the driver on i.MX31 only, and I didn't find any problems.
Please fix the indentation issue found by Stefan in v7.
Regarding utilization of timer-of.c it can be postponed IMHO,
but it's up to clocksource maintainers and you to decide, and if
you do such an update for v7, then please don't add my tags,
I'll review and test it again.
--
With best wishes,
Vladimir
^ permalink raw reply
* [PATCH v6 5/5] ARM: dts: imx: add missing compatible and clock properties for EPIT
From: Vladimir Zapolskiy @ 2018-06-11 14:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180607140544.22268-6-peron.clem@gmail.com>
On 06/07/2018 05:05 PM, Cl?ment P?ron wrote:
> From: Colin Didier <colin.didier@devialet.com>
>
> Add missing compatible and clock properties for EPIT node.
>
> Signed-off-by: Colin Didier <colin.didier@devialet.com>
> Signed-off-by: Cl?ment Peron <clement.peron@devialet.com>
Reviewed-by: Vladimir Zapolskiy <vz@mleia.com>
--
With best wishes,
Vladimir
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox