Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] irqchip/gic-v3-its: fix ITS queue timeout
From: Marc Zyngier @ 2018-06-06  9:13 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1528252824-15144-1-git-send-email-yangyingliang@huawei.com>

On Wed, 06 Jun 2018 03:40:24 +0100,
Yang Yingliang wrote:

[I'm travelling, so please do not expect any quick answer...]

> 
> When the kernel booted with maxcpus=x, 'x' is smaller
> than actual cpu numbers, the TAs of offline cpus won't

TA? Target Address? Target Affinity? Timing Advance? Terrible Acronym?

> be set to its->collection.
> 
> If LPI is bind to offline cpu, sync cmd will use zero TA,
> it leads to ITS queue timeout.  Fix this by choosing a
> online cpu, if there is no online cpu in cpu_mask.

So instead of fixing the emission of a sync command on a non-mapped
collection, you hack set_affinity? It doesn't feel like the right
thing to do.

It is also worth noticing that mapping an LPI to a collection that is
not mapped yet is perfectly legal.

> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
> ---
>  drivers/irqchip/irq-gic-v3-its.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index 5416f2b..d8b9539 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -2309,7 +2309,9 @@ 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)
> +		cpu = cpumask_first(cpu_online_mask);

Now you're completely ignoring cpu_mask which constraints the NUMA
affinity. On some systems, this ends up with a deadlock (Cavium TX1,
if I remember well).

Wouldn't it be better to just return that the affinity setting request
is impossible to satisfy? And more to the point, how comes we end-up
in such a case?

Thanks,

	M.

-- 
Jazz is not dead, it just smell funny.

^ permalink raw reply

* [PATCH] ARM: Build secure_cntvoff.S unconditionally to fix shmobile !SMP build
From: Geert Uytterhoeven @ 2018-06-06  9:22 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180606090953.GW17671@n2100.armlinux.org.uk>

Hi Russell,

On Wed, Jun 6, 2018 at 11:09 AM, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
> On Wed, Jun 06, 2018 at 10:52:34AM +0200, Geert Uytterhoeven wrote:
>> If CONFIG_SMP=n, building a kernel for R-Car Gen2 fails with:
>>
>>     arch/arm/mach-shmobile/setup-rcar-gen2.o: In function `rcar_gen2_timer_init':
>>     setup-rcar-gen2.c:(.init.text+0x30): undefined reference to `secure_cntvoff_init'
>>
>> Indeed, on R-Car Gen2 SoCs, secure_cntvoff_init() is not only needed for
>> secondary CPUs, but also for the boot CPU.  This is most visible on SoCs
>> with Cortex A7 cores (e.g. R-Car E2, cfr. commit 9ce3fa6816c2fb59 ("ARM:
>> shmobile: rcar-gen2: Add CA7 arch_timer initialization for r8a7794")),
>> but Cortex A15 is affected, too.
>>
>> Fix this by always providing secure_cntvoff_init().
>>
>> Reported-by: Arnd Bergmann <arnd@arndb.de>
>> Fixes: 7c607944bc657616 ("ARM: smp: Add initialization of CNTVOFF")

Interestingly, the commit description says:

   "It should be done by the bootloader but it is currently not the case,
    even for boot CPU because this SoC is booting in secure mode."
    ^^^^^^^^^^^^^^^^^

So it should be called for !SMP on sunxi, too?

>> Fixes: cad160ed0a94927e ("ARM: shmobile: Convert file to use cntvoff")
>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
> This doesn't look right to me, but I don't have secure_cntvoff in any
> tree here that I can look at to check.  What if secure_cntvoff contains
> instructions only available on ARMv7 CPUs, and not ARMv4?

Compiled != called.

> It makes no sense (to me) to always build this.

But you're right, making it depend on CPU_V7 is better.
Thanks!

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] ARM: Always build secure_cntvoff.S on ARM V7 to fix shmobile !SMP build
From: Geert Uytterhoeven @ 2018-06-06  9:25 UTC (permalink / raw)
  To: linux-arm-kernel

If CONFIG_SMP=n, building a kernel for R-Car Gen2 fails with:

    arch/arm/mach-shmobile/setup-rcar-gen2.o: In function `rcar_gen2_timer_init':
    setup-rcar-gen2.c:(.init.text+0x30): undefined reference to `secure_cntvoff_init'

Indeed, on R-Car Gen2 SoCs, secure_cntvoff_init() is not only needed for
secondary CPUs, but also for the boot CPU.  This is most visible on SoCs
with Cortex A7 cores (e.g. R-Car E2, cfr. commit 9ce3fa6816c2fb59 ("ARM:
shmobile: rcar-gen2: Add CA7 arch_timer initialization for r8a7794")),
but Cortex A15 is affected, too.

Fix this by always providing secure_cntvoff_init() when building for ARM
V7.

Reported-by: Arnd Bergmann <arnd@arndb.de>
Fixes: 7c607944bc657616 ("ARM: smp: Add initialization of CNTVOFF")
Fixes: cad160ed0a94927e ("ARM: shmobile: Convert file to use cntvoff")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
v2:
  - Add Reviewed-by,
  - Add a dependency on CPU_V7.
---
 arch/arm/common/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/common/Makefile b/arch/arm/common/Makefile
index 1e9f7af8f70ff6ba..3157be413297e5d2 100644
--- a/arch/arm/common/Makefile
+++ b/arch/arm/common/Makefile
@@ -10,7 +10,7 @@ obj-$(CONFIG_DMABOUNCE)		+= dmabounce.o
 obj-$(CONFIG_SHARP_LOCOMO)	+= locomo.o
 obj-$(CONFIG_SHARP_PARAM)	+= sharpsl_param.o
 obj-$(CONFIG_SHARP_SCOOP)	+= scoop.o
-obj-$(CONFIG_SMP)		+= secure_cntvoff.o
+obj-$(CONFIG_CPU_V7)		+= secure_cntvoff.o
 obj-$(CONFIG_PCI_HOST_ITE8152)  += it8152.o
 obj-$(CONFIG_MCPM)		+= mcpm_head.o mcpm_entry.o mcpm_platsmp.o vlock.o
 CFLAGS_REMOVE_mcpm_entry.o	= -pg
-- 
2.7.4

^ permalink raw reply related

* [PATCH] ARM: Build secure_cntvoff.S unconditionally to fix shmobile !SMP build
From: Russell King - ARM Linux @ 2018-06-06  9:27 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAMuHMdVq0Mwui6jrnX4sd2=L+p3Ta4qM2PxQ2HLQ0btajTV6hw@mail.gmail.com>

On Wed, Jun 06, 2018 at 11:22:41AM +0200, Geert Uytterhoeven wrote:
> Hi Russell,
> 
> On Wed, Jun 6, 2018 at 11:09 AM, Russell King - ARM Linux
> <linux@armlinux.org.uk> wrote:
> >> Fixes: cad160ed0a94927e ("ARM: shmobile: Convert file to use cntvoff")
> >> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >
> > This doesn't look right to me, but I don't have secure_cntvoff in any
> > tree here that I can look at to check.  What if secure_cntvoff contains
> > instructions only available on ARMv7 CPUs, and not ARMv4?
> 
> Compiled != called.

That makes little difference when the assembler validates that the
instructions are possible on the target architecture and errors out
if not.

That's why I qualified my reply by saying that I didn't have the
contents of secure_cntvoff to reference - there's no clue whether
it's a .c or .S file from just this patch, and there's no visibility
what it contains.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

^ permalink raw reply

* [PATCH] ARM: Build secure_cntvoff.S unconditionally to fix shmobile !SMP build
From: Geert Uytterhoeven @ 2018-06-06  9:28 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180606092725.GX17671@n2100.armlinux.org.uk>

Hi Russell,

On Wed, Jun 6, 2018 at 11:27 AM, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
> On Wed, Jun 06, 2018 at 11:22:41AM +0200, Geert Uytterhoeven wrote:
>> On Wed, Jun 6, 2018 at 11:09 AM, Russell King - ARM Linux
>> <linux@armlinux.org.uk> wrote:
>> >> Fixes: cad160ed0a94927e ("ARM: shmobile: Convert file to use cntvoff")
>> >> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> >
>> > This doesn't look right to me, but I don't have secure_cntvoff in any
>> > tree here that I can look at to check.  What if secure_cntvoff contains
>> > instructions only available on ARMv7 CPUs, and not ARMv4?
>>
>> Compiled != called.
>
> That makes little difference when the assembler validates that the
> instructions are possible on the target architecture and errors out
> if not.

The file does have ".arch armv7-a".

> That's why I qualified my reply by saying that I didn't have the
> contents of secure_cntvoff to reference - there's no clue whether
> it's a .c or .S file from just this patch, and there's no visibility
> what it contains.

https://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc.git/commit/?id=7c607944bc657616
https://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc.git/commit/?id=cad160ed0a94927e

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 IMX6SX_CLK_CKO1_SEL
From: Philipp Puschmann @ 2018-06-06  9:29 UTC (permalink / raw)
  To: linux-arm-kernel

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>
---
 drivers/clk/imx/clk-imx6q.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/imx/clk-imx6q.c b/drivers/clk/imx/clk-imx6q.c
index 8d518ad5dc13..7e6a8f079634 100644
--- a/drivers/clk/imx/clk-imx6q.c
+++ b/drivers/clk/imx/clk-imx6q.c
@@ -65,7 +65,7 @@ static const char *ipg_per_sels[] = { "ipg", "osc", };
 static const char *ecspi_sels[] = { "pll3_60m", "osc", };
 static const char *can_sels[] = { "pll3_60m", "osc", "pll3_80m", };
 static const char *cko1_sels[]	= { "pll3_usb_otg", "pll2_bus", "pll1_sys", "pll5_video_div",
-				    "dummy", "axi", "enfc", "ipu1_di0", "ipu1_di1", "ipu2_di0",
+				    "video_27m", "axi", "enfc", "ipu1_di0", "ipu1_di1", "ipu2_di0",
 				    "ipu2_di1", "ahb", "ipg", "ipg_per", "ckil", "pll4_audio_div", };
 static const char *cko2_sels[] = {
 	"mmdc_ch0_axi", "mmdc_ch1_axi", "usdhc4", "usdhc1",
-- 
2.17.0

^ permalink raw reply related

* [PATCH] ARM: dts: imx6: RDU2: correct touchscreen axis inversion
From: Lucas Stach @ 2018-06-06  9:30 UTC (permalink / raw)
  To: linux-arm-kernel

The RMI4 touchscreen driver applied inversion and axis swap in the
wrong order, violating the DT binding for those properties. This is
fixed now, so correct the RDU2 DT to apply the inversion to the
correct axis.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi b/arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi
index 911f7f0e3cea..605ec969a859 100644
--- a/arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi
@@ -558,14 +558,14 @@
 
 		rmi4-f11 at 11 {
 			reg = <0x11>;
-			touchscreen-inverted-y;
+			touchscreen-inverted-x;
 			touchscreen-swapped-x-y;
 			syna,sensor-type = <1>;
 		};
 
 		rmi4-f12 at 12 {
 			reg = <0x12>;
-			touchscreen-inverted-y;
+			touchscreen-inverted-x;
 			touchscreen-swapped-x-y;
 			syna,sensor-type = <1>;
 		};
-- 
2.17.1

^ permalink raw reply related

* [alsa-devel] [PATCH 0/3] ASoC: stm32: sai: add support of iec958 controls
From: Arnaud Pouliquen @ 2018-06-06  9:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <s5hpo15xf1q.wl-tiwai@suse.de>



On 06/05/2018 08:29 PM, Takashi Iwai wrote:
> On Tue, 05 Jun 2018 17:50:57 +0200,
> Arnaud Pouliquen wrote:
>> 
>> Hi Takashi,
>> 
>> On 04/17/2018 01:17 PM, Mark Brown wrote:
>> > On Tue, Apr 17, 2018 at 08:29:17AM +0000, Olivier MOYSAN wrote:
>> > 
>> >> I guess the blocking patch in this patchset is the patch "add IEC958 
>> >> channel status control helper". This patch has been reviewed several 
>> >> times, but did not get a ack so far.
>> >> If you think these helpers will not be merged, I will reintegrate the 
>> >> corresponding code in stm driver.
>> >> Please let me know, if I need to prepare a v2 without helpers, or if we 
>> >> can go further in the review of iec helpers patch ?
>> > 
>> > I don't mind either way but you're right here, I'm waiting for Takashi
>> > to review the first patch.? I'd probably be OK with it just integrated
>> > into the driver if we have to go that way though.
>> 
>> Gentlemen reminder for this patch set. We would appreciate to have your
>> feedback on iec helper.
>> From our point of view it could be useful to have a generic management
>> of the iec controls based on helpers instead of redefining them in DAIs.
>> Having the same behavior for these controls could be useful to ensure
>> coherence of the control management used by application (for instance
>> Gstreamer uses it to determine iec raw mode capability for iec61937 streams)
> 
> Oh sorry for the late reply, I've totally overlooked the thread.
> 
> And, another sorry: the patchset doesn't look convincing enough to
> me.
> 
> First off, the provided API definition appears somewhat
> unconventional, the mixture of the ops, the static data and the
> dynamic data.
Sorry i can't figure out your point. I suppose that you speak about the
snd_pcm_iec958_params.
what would be a more conventional API?

> 
> Moreover, this is only for your driver, ATM.? 
It is also compatible with the sound/sti driver, even if we does not
propose patch yet. We also plan to propose an implementation, for the
HDMI_codec that would need to export a control to allow none-audio mode.

>If it were an API that
> does clean up the already existing usages, I'd happily apply it. There
> are lots of drivers creating and controlling the IEC958 ctls even
> now.
> 
> Also, the patchset requires more fine-tuning, in anyways.? The changes
> in create_iec958_consumre() are basically irrelevant, should be split
> off as an individual fix.? it is linked to the control, as not possible in existing implementation
(rate and width are get from hwparam or runtime). But no problem we can
split it in a separate patch.

Also, the new function doesn't create the
> "XXX Mask" controls.? And the byte comparison should be replaced with
> memcmp(), etc, etc.
Yes mask are missing, can be added. For the rest could you comment
directly in code? i suppose that you want to replace the for loops by
memcmp, memcpy...
> 
> So, please proceed rather with the open codes for now.? If you can
> provide a patch that cleans up the *multiple* driver codes, again,
> I'll happily take it.? But it can be done anytime later, too.
Not simple to clean up the other drivers as this control is a PCM
control, that is mainly implemented as a mixer or card control.
This means that it should be registered on the pcm_new in CPU DAI or in
the DAI codec, to be able to bind it to the PCM device.
Inpact is not straigthforward as this could generate regression on driver.

For now We can add the clean up on the sti driver based on this helper,
and we are working on the HDMI_codec, we could also use this helper to
export the control....

So if you estimate that it is interesting to purchase on this helper we
can try to come back with a patch set that implements the helper for
the 3 drivers.

The other option, is that we drop the helpers, and implement the control
directly in our drivers.

Please just tell us if we should continue to propose the helpers or not.

Thanks,
Arnaud

> 
> 
> thanks,
> 
> Takashi

^ permalink raw reply

* [PATCH] ARM: dts: cygnus: Add HWRNG node
From: Clément Péron @ 2018-06-06  9:34 UTC (permalink / raw)
  To: linux-arm-kernel

From: Cl?ment Peron <clement.peron@devialet.com>

There is a HWRNG in Broadcom Cygnus SoC, so enable it.

Signed-off-by: Cl?ment Peron <clement.peron@devialet.com>
---
 arch/arm/boot/dts/bcm-cygnus.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/bcm-cygnus.dtsi b/arch/arm/boot/dts/bcm-cygnus.dtsi
index 1cee40ac4613..b7178e84d56d 100644
--- a/arch/arm/boot/dts/bcm-cygnus.dtsi
+++ b/arch/arm/boot/dts/bcm-cygnus.dtsi
@@ -452,6 +452,11 @@
 			status = "disabled";
 		};
 
+		hwrng: hwrng at 18032000 {
+			compatible = "brcm,iproc-rng200";
+			reg = <0x18032000 0x28>;
+		};
+
 		sdhci0: sdhci at 18041000 {
 			compatible = "brcm,sdhci-iproc-cygnus";
 			reg = <0x18041000 0x100>;
-- 
2.17.1

^ permalink raw reply related

* [PATCH v4 05/14] coresight: get/put module in coresight_build/release_path
From: Suzuki K Poulose @ 2018-06-06  9:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180606082422.GB19727@kroah.com>

On 06/06/2018 09:24 AM, Greg Kroah-Hartman wrote:
> On Tue, Jun 05, 2018 at 04:07:01PM -0500, Kim Phillips wrote:
>> Increment the refcnt for driver modules in current use by calling
>> module_get in coresight_build_path and module_put in release_path.
>>
>> This prevents driver modules from being unloaded when they are in use,
>> either in sysfs or perf mode.
> 
> Why does it matter?  Shouldn't you be allowed to remove any module at
> any point in time, much like a networking driver?
> 
> 
>>
>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>> Cc: Leo Yan <leo.yan@linaro.org>
>> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
>> Cc: Randy Dunlap <rdunlap@infradead.org>
>> Cc: Suzuki K Poulose <Suzuki.Poulose@arm.com>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Cc: Russell King <linux@armlinux.org.uk>
>> Signed-off-by: Kim Phillips <kim.phillips@arm.com>
>> ---
>>   drivers/hwtracing/coresight/coresight.c | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c
>> index 338f1719641c..1c941351f1d1 100644
>> --- a/drivers/hwtracing/coresight/coresight.c
>> +++ b/drivers/hwtracing/coresight/coresight.c
>> @@ -465,6 +465,12 @@ static int _coresight_build_path(struct coresight_device *csdev,
>>   
>>   	node->csdev = csdev;
>>   	list_add(&node->link, path);
>> +
>> +	if (!try_module_get(csdev->dev.parent->driver->owner)) {
> 
> What is to keep parent->driver from going away right here?  What keeps
> parent around?  This feels very fragile to me, I don't see any locking
> anywhere around this code path to try to keep things in place.

You're right. We do have coresight_mutex, which is held across the build 
path and the csdev is removed when a device is unregistered. However, I
see that we don't hold the mutex while removing the connections from
coresight_unregister(). Holding the mutex should protect us from the
csdev being removed, while we build the path.

And while we are at this, I also realised that we hold references to the
parent devices for each connection (via bus_find_device() from 
of_coresight_get_endpoint_device()), while parsing the platform data, 
which is never released.

Thanks
Suzuki

> 
> thanks,
> 
> greg k-h
> 

^ permalink raw reply

* [alsa-devel] [PATCH 0/3] ASoC: stm32: sai: add support of iec958 controls
From: Takashi Iwai @ 2018-06-06  9:47 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <c4b2d22f-320d-a415-6328-b33f9c03aafe@st.com>

On Wed, 06 Jun 2018 11:31:45 +0200,
Arnaud Pouliquen wrote:
> 
> 
> 
> On 06/05/2018 08:29 PM, Takashi Iwai wrote:
> > On Tue, 05 Jun 2018 17:50:57 +0200,
> > Arnaud Pouliquen wrote:
> >> 
> >> Hi Takashi,
> >> 
> >> On 04/17/2018 01:17 PM, Mark Brown wrote:
> >> > On Tue, Apr 17, 2018 at 08:29:17AM +0000, Olivier MOYSAN wrote:
> >> > 
> >> >> I guess the blocking patch in this patchset is the patch "add IEC958 
> >> >> channel status control helper". This patch has been reviewed several 
> >> >> times, but did not get a ack so far.
> >> >> If you think these helpers will not be merged, I will reintegrate the 
> >> >> corresponding code in stm driver.
> >> >> Please let me know, if I need to prepare a v2 without helpers, or if we 
> >> >> can go further in the review of iec helpers patch ?
> >> > 
> >> > I don't mind either way but you're right here, I'm waiting for Takashi
> >> > to review the first patch.? I'd probably be OK with it just integrated
> >> > into the driver if we have to go that way though.
> >> 
> >> Gentlemen reminder for this patch set. We would appreciate to have your
> >> feedback on iec helper.
> >> From our point of view it could be useful to have a generic management
> >> of the iec controls based on helpers instead of redefining them in DAIs.
> >> Having the same behavior for these controls could be useful to ensure
> >> coherence of the control management used by application (for instance
> >> Gstreamer uses it to determine iec raw mode capability for iec61937 streams)
> > 
> > Oh sorry for the late reply, I've totally overlooked the thread.
> > 
> > And, another sorry: the patchset doesn't look convincing enough to
> > me.
> > 
> > First off, the provided API definition appears somewhat
> > unconventional, the mixture of the ops, the static data and the
> > dynamic data.
> Sorry i can't figure out your point. I suppose that you speak about the
> snd_pcm_iec958_params.
> what would be a more conventional API?

Imagine you'd want to put a const to the data passed to the API for
hardening.  The current struct is a mixture of static and dynamic
data.


> > Moreover, this is only for your driver, ATM.? 
> It is also compatible with the sound/sti driver, even if we does not
> propose patch yet. We also plan to propose an implementation, for the
> HDMI_codec that would need to export a control to allow none-audio mode.
> 
> >If it were an API that
> > does clean up the already existing usages, I'd happily apply it. There
> > are lots of drivers creating and controlling the IEC958 ctls even
> > now.
> > 
> > Also, the patchset requires more fine-tuning, in anyways.? The changes
> > in create_iec958_consumre() are basically irrelevant, should be split
> > off as an individual fix.? it is linked to the control, as not possible in existing implementation
> (rate and width are get from hwparam or runtime). But no problem we can
> split it in a separate patch.
> 
> Also, the new function doesn't create the
> > "XXX Mask" controls.? And the byte comparison should be replaced with
> > memcmp(), etc, etc.
> Yes mask are missing, can be added. For the rest could you comment
> directly in code? i suppose that you want to replace the for loops by
> memcmp, memcpy...

Right.

> > So, please proceed rather with the open codes for now.? If you can
> > provide a patch that cleans up the *multiple* driver codes, again,
> > I'll happily take it.? But it can be done anytime later, too.
> Not simple to clean up the other drivers as this control is a PCM
> control, that is mainly implemented as a mixer or card control.
> This means that it should be registered on the pcm_new in CPU DAI or in
> the DAI codec, to be able to bind it to the PCM device.
> Inpact is not straigthforward as this could generate regression on driver.

Yes, and that's my point.  The application of API is relatively
limited -- although the API itself has nothing to do with ASoC at
all.

> For now We can add the clean up on the sti driver based on this helper,
> and we are working on the HDMI_codec, we could also use this helper to
> export the control....
> 
> So if you estimate that it is interesting to purchase on this helper we
> can try to come back with a patch set that implements the helper for
> the 3 drivers.

Right.  Basically there are two cases we add a new API:

1. It's absolutely new and nothing else can do it
2. API simplifies the whole tree, not only one you're trying to add.

And in this case, let's prove 2 at first, that the API *is* actually
useful in multiple situations we already have.  Then I'll happily ack
for that.  More drivers cleanup, better.  At best, think of more
range above ASoC, as you're proposing ALSA core API, not the ASoC
one.

> The other option, is that we drop the helpers, and implement the control
> directly in our drivers.

This is of course another, maybe easier option.

> Please just tell us if we should continue to propose the helpers or not.

I have no preference over two ways, but am only interested in the
resulting patches :)


thanks,

Takashi

^ permalink raw reply

* [PATCH v4 14/14] coresight: allow the coresight core driver to be built as a module
From: Suzuki K Poulose @ 2018-06-06  9:57 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180605210710.22227-15-kim.phillips@arm.com>

On 06/05/2018 10:07 PM, Kim Phillips wrote:
> Allow to build coresight as a module.  This enhances
> coresight developer efficiency by allowing the development to
> take place exclusively on the target, and without needing to
> reboot in between changes.
> 
> - Kconfig becomes a tristate, to allow =m
> - append -core to source file name to allow module to
>    be called coresight by the Makefile
> - modules can have only one init/exit, so we add the core bus
>    register/unregister function calls to the etm_perf init/exit
>    functions, since coresight.c does not have etm_pmu defined.

It feels a bit weird to move the coresight core specific steps to
etm_perf_init. Why not call etm_perf_init from coresight init routine
and keep everything core specific in the coresight-core.c ?


Suzuki

^ permalink raw reply

* [PATCH] arm64: topology: Avoid checking numa mask for scheduler MC selection
From: Sudeep Holla @ 2018-06-06 10:05 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180605190837.493505-1-jeremy.linton@arm.com>



On 05/06/18 20:08, Jeremy Linton wrote:
> The numa mask subset check has problems if !CONFIG_NUMA, over hotplug
> operations or during early boot. Lets disable the NUMA siblings checks
> for the time being, as NUMA in socket machines have LLC's that will
> assure that the scheduler topology isn't "borken".
> 

^ broken ? (not sure if usage of borken is intentional :))

> Futher, as a defensive mechanism during hotplug, lets assure that the

^ Further

> LLC siblings are also masked.
>

Also add the symptoms of the issue we say as Geert suggested me.
Something like:
" This often leads to system hang or crash during CPU hotplug and system
suspend operation. This is mostly observed on HMP systems where the
CPU compute capacities are different and ends up in different scheduler
domains. Since cpumask_of_node is returned instead core_sibling, the
scheduler is confused with incorrect cpumasks(e.g. one CPU in two
different sched domains at the same time) on CPU hotplug."

You can add Reported-by: Geert... ?

> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> ---
>  arch/arm64/kernel/topology.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
> index 7415c166281f..f845a8617812 100644
> --- a/arch/arm64/kernel/topology.c
> +++ b/arch/arm64/kernel/topology.c
> @@ -215,13 +215,8 @@ EXPORT_SYMBOL_GPL(cpu_topology);
>  
>  const struct cpumask *cpu_coregroup_mask(int cpu)
>  {
> -	const cpumask_t *core_mask = cpumask_of_node(cpu_to_node(cpu));
> +	const cpumask_t *core_mask = &cpu_topology[cpu].core_sibling;
>  
> -	/* Find the smaller of NUMA, core or LLC siblings */
> -	if (cpumask_subset(&cpu_topology[cpu].core_sibling, core_mask)) {
> -		/* not numa in package, lets use the package siblings */
> -		core_mask = &cpu_topology[cpu].core_sibling;
> -	}
>  	if (cpu_topology[cpu].llc_id != -1) {
>  		if (cpumask_subset(&cpu_topology[cpu].llc_siblings, core_mask))
>  			core_mask = &cpu_topology[cpu].llc_siblings;
> @@ -239,8 +234,10 @@ static void update_siblings_masks(unsigned int cpuid)
>  	for_each_possible_cpu(cpu) {
>  		cpu_topo = &cpu_topology[cpu];
>  
> -		if (cpuid_topo->llc_id == cpu_topo->llc_id)
> +		if (cpuid_topo->llc_id == cpu_topo->llc_id) {
>  			cpumask_set_cpu(cpu, &cpuid_topo->llc_siblings);
> +			cpumask_set_cpu(cpuid, &cpu_topo->llc_siblings);
> +		}
>  
>  		if (cpuid_topo->package_id != cpu_topo->package_id)
>  			continue;
> 

Looks good to me for now. I might need to tweek it a bit when I add the
support to update topology on hotplug. But that's for latter. For now,

Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>

-- 
Regards,
Sudeep

^ permalink raw reply

* [PATCH v3 0/6] Add MCAN Support for dra76x
From: Tony Lindgren @ 2018-06-06 10:09 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180606060826.14671-1-faiz_abbas@ti.com>

* Faiz Abbas <faiz_abbas@ti.com> [180606 06:09]:
> The following patches add dts and sysconfig support
> for MCAN on TI's dra76 SOCs
> 
> The patches depend on the following series:
> https://patchwork.kernel.org/patch/10221105/
> 
> Changes in v3:
>  1. Added reset functionality to the ti-sysc
>     driver. This enables me to drop the hwmod
>     data patch as everything is being done in dt.
>
>  2. Dropped ti,hwmods from the dts nodes

Cool good to hear that works :)

>  4. Removed the status="disabled" in dtsi
>     followed by status="okay" in dts.

Hmm okay is the default and is not needed. I did not notice
okay in this set based on a quick glance so maybe you already
dropped it?

Regards,

Tony

^ permalink raw reply

* [PATCH v3 0/6] Add MCAN Support for dra76x
From: Faiz Abbas @ 2018-06-06 10:13 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180606100937.GF5738@atomide.com>

Hi,

On Wednesday 06 June 2018 03:39 PM, Tony Lindgren wrote:
> * Faiz Abbas <faiz_abbas@ti.com> [180606 06:09]:
>> The following patches add dts and sysconfig support
>> for MCAN on TI's dra76 SOCs
>>
>> The patches depend on the following series:
>> https://patchwork.kernel.org/patch/10221105/
>>
>> Changes in v3:
>>  1. Added reset functionality to the ti-sysc
>>     driver. This enables me to drop the hwmod
>>     data patch as everything is being done in dt.
>>
>>  2. Dropped ti,hwmods from the dts nodes
> 
> Cool good to hear that works :)
> 
>>  4. Removed the status="disabled" in dtsi
>>     followed by status="okay" in dts.
> 
> Hmm okay is the default and is not needed. I did not notice
> okay in this set based on a quick glance so maybe you already
> dropped it?

I guess that wasn't clear. I meant to say I have dropped both
status=disabled and status=okay because okay is default.

Thanks,
Faiz

^ permalink raw reply

* [PATCH 01/20] coresight: Fix memory leak in coresight_register
From: Suzuki K Poulose @ 2018-06-06 10:16 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <b0100f00-c706-3a41-ca55-3c1329c53aa3@gmail.com>

On 06/06/2018 07:44 AM, Arvind Yadav wrote:
> Hi Suzuki,
> 
> 
> On Wednesday 06 June 2018 03:13 AM, Suzuki K Poulose wrote:
>> commit 6403587a930c ("coresight: use put_device() instead of kfree()")
>> introduced a memory leak where, if we fail to register the device
>> for coresight_device, we don't free the "coresight_device" object,
>> which was allocated via kzalloc(). Fix this by jumping to the
>> appropriate error path.
> put_device() will decrement the last reference and then
> free the memory by calling dev->release.? Internally
> put_device() -> kobject_put() -> kobject_cleanup() which is
> responsible to call 'dev -> release' and also free other kobject
> resources. If you will see the coresight_device_release. There
> we are releasing all allocated memory. Still if you call kfree() again
> then it'll be redundancy.

You're right. I think it would be good to have a comment explaining this
to prevent this fix popping up in the future :-). I will add it

Thanks
Suzuki

^ permalink raw reply

* [PATCH 4/4] soc: imx: add SC firmware IPC and APIs
From: A.s. Dong @ 2018-06-06 10:28 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <AM0PR04MB4211FCC9F3902F14EBD28313806E0@AM0PR04MB4211.eurprd04.prod.outlook.com>

Hi Sascha,

> -----Original Message-----
> From: A.s. Dong
> Sent: Monday, May 28, 2018 5:25 PM
> To: 'Sascha Hauer' <s.hauer@pengutronix.de>
> Cc: 'linux-arm-kernel at lists.infradead.org' <linux-arm-
> kernel at lists.infradead.org>; 'dongas86 at gmail.com' <dongas86@gmail.com>;
> dl-linux-imx <linux-imx@nxp.com>; 'kernel at pengutronix.de'
> <kernel@pengutronix.de>; Fabio Estevam <fabio.estevam@nxp.com>;
> 'shawnguo at kernel.org' <shawnguo@kernel.org>
> Subject: RE: [PATCH 4/4] soc: imx: add SC firmware IPC and APIs
> 
...

> > > > > We discussed this internally and came to the conclusion that the
> > > > > SCU is not a generic user of a MU. The MU is designed to work
> > > > > together with a piece of SRAM to form a mailbox system. Instead
> > > > > of working as designed the SCU morses the messages through the
> > > > > doorbell (what the MU really is). For anything that uses the MU
> > > > > in the way it is designed I would suggest using the mailbox
> > > > > interface from drivers/mailbox/ along with the binding from
> > > Documentation/devicetree/bindings/mailbox/mailbox.txt.
> > > > >
> > > > > In the way I suggest there is no need for any MU ID.
> > > > >
> > > >
> > > > So you're suggesting switch to use mailbox instead of private MU
> > > > library function calls?
> > > > Something like:
> > > >         scu {
> > > >                 compatible = "nxp,imx8qxp-scu", "simple-bus";
> > > >                 mboxes = <&mailbox 0>;
> > > >         }
> > > > Then IPC is implemented based on mailbox?
> > > >
> > > > As I replied Oleksij Rempel in another mail in this thread, we've
> > > > tried mailbox but got performance regression issue and need more
> > > > time to investigate whether it's really quite suitable for i.MX
> > > > SCU firmware as SCU handling message quite fast in micro seconds.
> > > > (Mailbox polling method has much more latency than current MU
> > > > sample polling we
> > > > used) and we want to avoid the SCU firmware API changes.
> > >
> > > I am not suggesting to do mailboxing (using shared memory) for the SCU.
> > > I am also not suggesting any API update for the SCU communication.
> > > I am just mentioning that doing mailboxing is the way the MU was
> > > originally designed for. Looking at the reference manual I see many
> > > MUs on
> > the i.MX8.
> > > I guess most of them are for communication between the different
> > > cores on the system. For these it's probably worth writing some
> > > generic MU
> > driver.
> > > The way the MU is used for communication with the SCU though is so
> > > special that it's not worth writing a generic driver, so just
> > > integrate the MU access functions in the SCU code.
> > >
> >
> > I understand it.
> >
> > One problem of the way you suggested may be that:
> > If we doing like below, we may lose flexibility to change the MU used
> > for SCU firmware communication.
> > 	scu at 5d1b0000 {
> > 		compatible = "fsl,imx8qxp-scu";
> > 		reg = <0x0 0x5d1b0000 0x0 0x10000>;
> > 	};
> >
> > And current design is that the system supports multiple MU channels
> > used by various users at the same time, e.g. SCU, Power Domain, Clock and
> others.
> > User can flexibly change it under their nodes: And each MU channel is
> > protected by their private lock and not affect each others.
> >
> > e.g.
> >         scu {
> >                 compatible = "nxp,imx8qxp-scu", "simple-bus";
> >                 fsl,mu = <&lsio_mu0>;
> >
> >                 clk: clk {
> >                         compatible = "fsl,imx8qxp-clk";
> >                         #clock-cells = <1>;
> >                 };
> >
> >                 iomuxc: iomuxc {
> >                         compatible = "fsl,imx8qxp-iomuxc";
> >                         fsl,mu = <&lsio_mu3>;
> >                 };
> >
> >                 imx8qx-pm {
> >                         #address-cells = <1>;
> >                         #size-cells = <0>;
> >                         fsl,mu = <&lsio_mu4>;
> > 	.............
> >         }
> >
> > The default code only uses MU0 which is used by SCU.
> >
> > The change may affect this design. Any ideas?
> > Do you think we can keep the current way?
> >
> 
> Any comments about this?
> 

Have you got a chance to help look at it?
We need identify the direction to go next.

Regards
Dong Aisheng

> Regards
> Dong Aisheng
> 
> > Regards
> > Dong Aisheng
> >
> > > Sascha
> > >
> > > --
> > > Pengutronix e.K.                           |                             |
> > > Industrial Linux Solutions                 |
> > >
> >
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fww
> > >
> >
> w.pengutronix.de%2F&data=02%7C01%7Caisheng.dong%40nxp.com%7C266
> > >
> 24a5c38e5488a80b708d5b0f3107b%7C686ea1d3bc2b4c6fa92cd99c5c301635%
> > >
> >
> 7C0%7C0%7C636609480354404338&sdata=XP%2BYdt9I7zURrQun2jRbempLhC
> > > XrYtMVMb3dEWrZuro%3D&reserved=0  |
> > > Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0
> |
> > > Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply

* [PATCH v5 6/6] tty/serial: atmel: changed the driver to work under at91-usart mfd
From: Richard Genoud @ 2018-06-06 11:02 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180604165943.31381-7-radu.pirea@microchip.com>

Typo in the subject:
changed->change


On 04/06/2018 18:59, Radu Pirea wrote:
> This patch modifies the place where resources and device tree properties
> are searched.
> 
> Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
> ---
>  drivers/tty/serial/Kconfig        |  1 +
>  drivers/tty/serial/atmel_serial.c | 41 ++++++++++++++++++-------------
>  2 files changed, 25 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index 3682fd3e960c..25e55332f8b1 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -119,6 +119,7 @@ config SERIAL_ATMEL
>  	depends on ARCH_AT91 || COMPILE_TEST
>  	select SERIAL_CORE
>  	select SERIAL_MCTRL_GPIO if GPIOLIB
> +	select MFD_AT91_USART
>  	help
>  	  This enables the driver for the on-chip UARTs of the Atmel
>  	  AT91 processors.
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index df46a9e88c34..5c74e03396ef 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -193,8 +193,8 @@ static struct console atmel_console;
>  
>  #if defined(CONFIG_OF)
>  static const struct of_device_id atmel_serial_dt_ids[] = {
> -	{ .compatible = "atmel,at91rm9200-usart" },
> -	{ .compatible = "atmel,at91sam9260-usart" },
> +	{ .compatible = "atmel,at91rm9200-usart-serial" },
> +	{ .compatible = "atmel,at91sam9260-usart-serial" },
Sorry, I didn't catch that before, but we can drop "atmel,at91sam9260-usart-serial" don't we ?
Only "atmel,at91rm9200-usart-serial" is used in the MFD driver.

>  	{ /* sentinel */ }
>  };
>  #endif
> @@ -915,6 +915,7 @@ static void atmel_tx_dma(struct uart_port *port)
>  static int atmel_prepare_tx_dma(struct uart_port *port)
>  {
>  	struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
> +	struct device *mfd_dev = port->dev->parent;
>  	dma_cap_mask_t		mask;
>  	struct dma_slave_config config;
>  	int ret, nent;
> @@ -922,7 +923,7 @@ static int atmel_prepare_tx_dma(struct uart_port *port)
>  	dma_cap_zero(mask);
>  	dma_cap_set(DMA_SLAVE, mask);
>  
> -	atmel_port->chan_tx = dma_request_slave_channel(port->dev, "tx");
> +	atmel_port->chan_tx = dma_request_slave_channel(mfd_dev, "tx");
>  	if (atmel_port->chan_tx == NULL)
>  		goto chan_err;
>  	dev_info(port->dev, "using %s for tx DMA transfers\n",
> @@ -1093,6 +1094,7 @@ static void atmel_rx_from_dma(struct uart_port *port)
>  static int atmel_prepare_rx_dma(struct uart_port *port)
>  {
>  	struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
> +	struct device *mfd_dev = port->dev->parent;
>  	struct dma_async_tx_descriptor *desc;
>  	dma_cap_mask_t		mask;
>  	struct dma_slave_config config;
> @@ -1104,7 +1106,7 @@ static int atmel_prepare_rx_dma(struct uart_port *port)
>  	dma_cap_zero(mask);
>  	dma_cap_set(DMA_CYCLIC, mask);
>  
> -	atmel_port->chan_rx = dma_request_slave_channel(port->dev, "rx");
> +	atmel_port->chan_rx = dma_request_slave_channel(mfd_dev, "rx");
>  	if (atmel_port->chan_rx == NULL)
>  		goto chan_err;
>  	dev_info(port->dev, "using %s for rx DMA transfers\n",
> @@ -1631,7 +1633,7 @@ static void atmel_tasklet_tx_func(unsigned long data)
>  static void atmel_init_property(struct atmel_uart_port *atmel_port,
>  				struct platform_device *pdev)
>  {
> -	struct device_node *np = pdev->dev.of_node;
> +	struct device_node *np = pdev->dev.parent->of_node;
I think this is not needed anymore (cf atmel_probe())

>  
>  	/* DMA/PDC usage specification */
>  	if (of_property_read_bool(np, "atmel,use-dma-rx")) {
> @@ -2222,8 +2224,8 @@ static const char *atmel_type(struct uart_port *port)
>   */
>  static void atmel_release_port(struct uart_port *port)
>  {
> -	struct platform_device *pdev = to_platform_device(port->dev);
> -	int size = pdev->resource[0].end - pdev->resource[0].start + 1;
> +	struct platform_device *mpdev = to_platform_device(port->dev->parent);
> +	int size = resource_size(mpdev->resource);
>  
>  	release_mem_region(port->mapbase, size);
>  
> @@ -2238,8 +2240,8 @@ static void atmel_release_port(struct uart_port *port)
>   */
>  static int atmel_request_port(struct uart_port *port)
>  {
> -	struct platform_device *pdev = to_platform_device(port->dev);
> -	int size = pdev->resource[0].end - pdev->resource[0].start + 1;
> +	struct platform_device *mpdev = to_platform_device(port->dev->parent);
> +	int size = resource_size(mpdev->resource);
>  
>  	if (!request_mem_region(port->mapbase, size, "atmel_serial"))
>  		return -EBUSY;
> @@ -2341,27 +2343,28 @@ static int atmel_init_port(struct atmel_uart_port *atmel_port,
>  {
>  	int ret;
>  	struct uart_port *port = &atmel_port->uart;
> +	struct platform_device *mpdev = to_platform_device(pdev->dev.parent);
>  
>  	atmel_init_property(atmel_port, pdev);
>  	atmel_set_ops(port);
>  
> -	uart_get_rs485_mode(&pdev->dev, &port->rs485);
> +	uart_get_rs485_mode(&mpdev->dev, &port->rs485);
>  
>  	port->iotype		= UPIO_MEM;
>  	port->flags		= UPF_BOOT_AUTOCONF | UPF_IOREMAP;
>  	port->ops		= &atmel_pops;
>  	port->fifosize		= 1;
>  	port->dev		= &pdev->dev;
> -	port->mapbase	= pdev->resource[0].start;
> -	port->irq	= pdev->resource[1].start;
> +	port->mapbase		= mpdev->resource[0].start;
> +	port->irq		= mpdev->resource[1].start;
>  	port->rs485_config	= atmel_config_rs485;
> -	port->membase	= NULL;
> +	port->membase		= NULL;
>  
>  	memset(&atmel_port->rx_ring, 0, sizeof(atmel_port->rx_ring));
>  
>  	/* for console, the clock could already be configured */
>  	if (!atmel_port->clk) {
> -		atmel_port->clk = clk_get(&pdev->dev, "usart");
> +		atmel_port->clk = clk_get(&mpdev->dev, "usart");
>  		if (IS_ERR(atmel_port->clk)) {
>  			ret = PTR_ERR(atmel_port->clk);
>  			atmel_port->clk = NULL;
> @@ -2652,11 +2655,13 @@ static int atmel_serial_resume(struct platform_device *pdev)
>  static void atmel_serial_probe_fifos(struct atmel_uart_port *atmel_port,
>  				     struct platform_device *pdev)
>  {
> +	struct device *dev = pdev->dev.parent;
> +
Ditto

>  	atmel_port->fifo_size = 0;
>  	atmel_port->rts_low = 0;
>  	atmel_port->rts_high = 0;
>  
> -	if (of_property_read_u32(pdev->dev.of_node,
> +	if (of_property_read_u32(dev->of_node,
Ditto
>  				 "atmel,fifo-size",
>  				 &atmel_port->fifo_size))
>  		return;
> @@ -2694,13 +2699,15 @@ static void atmel_serial_probe_fifos(struct atmel_uart_port *atmel_port,
>  static int atmel_serial_probe(struct platform_device *pdev)
>  {
>  	struct atmel_uart_port *atmel_port;
> -	struct device_node *np = pdev->dev.of_node;
> +	struct device_node *np = pdev->dev.parent->of_node;
>  	void *data;
>  	int ret = -ENODEV;
>  	bool rs485_enabled;
>  
>  	BUILD_BUG_ON(ATMEL_SERIAL_RINGSIZE & (ATMEL_SERIAL_RINGSIZE - 1));
>  
> +	pdev->dev.of_node = np;
> +
As atmel_serial device's of_node is now the parent node, the changes
in atmel_init_property() and atmel_serial_probe_fifos() are not needed anymore.
And maybe add a comment to explain that this is part of a MFD and all
the attributes are in the parent node, so we're using the MFD device node
instead of this device node.

>  	ret = of_alias_get_id(np, "serial");
>  	if (ret < 0)
>  		/* port id not found in platform data nor device-tree aliases:
> @@ -2845,7 +2852,7 @@ static struct platform_driver atmel_serial_driver = {
>  	.suspend	= atmel_serial_suspend,
>  	.resume		= atmel_serial_resume,
>  	.driver		= {
> -		.name			= "atmel_usart",
> +		.name			= "atmel_usart_serial",
>  		.of_match_table		= of_match_ptr(atmel_serial_dt_ids),
>  	},
>  };
> 

Thanks !

Richard.

^ permalink raw reply

* panic kexec broken on ARM64?
From: James Morse @ 2018-06-06 11:37 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <5e539510-01b7-ebc5-778e-b49a5261f9bc@i2se.com>

Hi Stefan,

On 06/06/18 08:02, Stefan Wahren wrote:
> Am 05.06.2018 um 19:46 schrieb James Morse:
>> On 05/06/18 09:01, Petr Tesarik wrote:
>>> I attached a hardware debugger and found
>>> out that all CPU cores were stopped except one which was stuck in the
>>> idle thread. It seems that irq_set_irqchip_state() may sleep, which is
>>> definitely not safe after a kernel panic.

>> I don't know much about irqchip stuff, but __irq_get_desc_lock() takes a
>> raw_spin_lock(), and calls gic_irq_get_irqchip_state() which is just poking
>> around in mmio registers, this should all be safe unless you re-entered the same
>> code.

>>> If I'm right, then this is broken in general, but I have only ever seen
>>> it on RPi 3 Model B+ (even RPi3 Model B works fine), so the issue may
>>> be more subtle.

>> Is there a hardware difference around the interrupt controller on these?

> No, but the RPi 3 B has a different USB network chip on board (smsc95xx, Fast
> ethernet) instead of lan78xx (Gigabit ethernet).

Bingo: its the lan78xx driver that is sleeping from the irqchip callbacks; The
smsc95xx driver doesn't have a struct irq_chip, which is why the RPi-3-B doesn't
do this.

It may be valid for kdump to only teardown the 'root irqdomain' (if that even
means anything). I assume these secondary irqchip's would have a
summary-interrupt that goes to another irqchip. But I can't see a way to tell
them apart..,

I think we need to wait until after the merge window for Marc's wisdom on this!


Thanks,

James

^ permalink raw reply

* panic kexec broken on ARM64?
From: Petr Tesarik @ 2018-06-06 11:41 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180606100024.50be6b75@ezekiel.suse.cz>

On Wed, 6 Jun 2018 10:00:24 +0200
Petr Tesarik <ptesarik@suse.cz> wrote:

> On Wed, 6 Jun 2018 09:02:04 +0200
> Stefan Wahren <stefan.wahren@i2se.com> wrote:
> 
> > Hi Petr,
> > 
> > Am 05.06.2018 um 19:46 schrieb James Morse:  
> > > Hi Petr,
> > >
> > > (CC: +Akashi, Marc)
> > >
> > > On 05/06/18 09:01, Petr Tesarik wrote:    
> > >> I have observed hangs after crash on a Raspberry Pi 3 Model B+ board
> > >> when a panic kernel is loaded.    
> > > kdump is a best-effort thing, it looks like this is a case where the
> > > crashed-kernel can't tear itself down.
> > >
> > > Do you have the rest of the stack trace? Was it handling an irq when it decided
> > > to panic?:
> > > https://lkml.org/lkml/2018/3/13/1134    
> > 
> > the Raspberry Pi 3 B+ support is very fresh (linux-next). Since i didn't 
> > see a version, i need to doublecheck.
> > 
> > You are actually using linux-next and not the downstream kernel?  
> 
> Very good point. I'll try again with linux-next.

It took me some time to set up everything correctly again...

Unfortunately, it makes no difference. I set a hardware breakpoint on
machine_crash_shutdown, followed by a breakpoint at __switch_to, and it
did trigger:

(gdb) lx-version 
Linux version 4.17.0-next-20180605-18-default (root at thunderx10) (gcc version 4.8.5 (SUSE Linux)) #1 SMP Wed Jun 6 10:26:46 CEST 2018
(gdb) bt
#0  __switch_to (prev=0xffff80002b428240, next=0xffff000008c32700 <init_task>) at arch/arm64/kernel/process.c:419
#1  0xffff0000088003d4 in context_switch (rf=<optimized out>, next=<optimized out>, prev=<optimized out>, rq=<optimized out>) at kernel/sched/core.c:2860
#2  __schedule (preempt=false) at kernel/sched/core.c:3502
#3  0xffff00000880092c in schedule () at kernel/sched/core.c:3546
#4  0xffff000008803e24 in schedule_timeout (timeout=<optimized out>) at kernel/time/timer.c:1801
#5  0xffff00000880144c in do_wait_for_common (state=<optimized out>, timeout=<optimized out>, action=<optimized out>, x=<optimized out>)
    at kernel/sched/completion.c:83
#6  __wait_for_common (state=<optimized out>, timeout=<optimized out>, action=<optimized out>, x=<optimized out>) at kernel/sched/completion.c:104
#7  wait_for_common (x=0xffff80002d0ef548, timeout=500, state=<optimized out>) at kernel/sched/completion.c:115
#8  0xffff000008801554 in wait_for_completion_timeout (x=0xffff80002d0ef548, timeout=<optimized out>) at kernel/sched/completion.c:155
#9  0xffff0000008f5ef8 in usb_start_wait_urb (urb=0xffff80002c593400, timeout=5000, actual_length=0xffff80002d0ef5dc) at drivers/usb/core/message.c:62
#10 0xffff0000008f602c in usb_internal_control_msg (timeout=<optimized out>, len=<optimized out>, data=<optimized out>, cmd=<optimized out>, pipe=<optimized out>, 
    usb_dev=<optimized out>) at drivers/usb/core/message.c:101
#11 usb_control_msg (dev=0xffff80002c684000, pipe=2147484800, request=161 '\241', requesttype=192 '\300', value=0, index=152, data=0xffff80002d421c80, size=4, 
    timeout=5000) at drivers/usb/core/message.c:152
#12 0xffff000000f29e10 in lan78xx_read_reg (index=152, data=0xffff80002d0ef66c, dev=<optimized out>, dev=<optimized out>) at drivers/net/usb/lan78xx.c:449
#13 0xffff000000f2c018 in lan78xx_irq_bus_sync_unlock (irqd=<optimized out>) at drivers/net/usb/lan78xx.c:1954
#14 0xffff0000081168e4 in chip_bus_sync_unlock (desc=<optimized out>) at kernel/irq/internals.h:147
#15 __irq_put_desc_unlock (desc=0xffff80002e7a9400, flags=<optimized out>, bus=true) at kernel/irq/irqdesc.c:837
#16 0xffff0000081176c0 in irq_put_desc_busunlock (flags=<optimized out>, desc=<optimized out>) at kernel/irq/internals.h:173
#17 irq_set_irqchip_state (irq=<optimized out>, which=IRQCHIP_STATE_ACTIVE, val=false) at kernel/irq/manage.c:2205
#18 0xffff00000809e0b0 in machine_kexec_mask_interrupts () at arch/arm64/kernel/machine_kexec.c:233
#19 machine_crash_shutdown (regs=<optimized out>) at arch/arm64/kernel/machine_kexec.c:259
#20 0xffff00000815b358 in __crash_kexec (regs=0xffff80002d0efb50) at kernel/kexec_core.c:943
#21 0xffff00000815b45c in crash_kexec (regs=0xffff80002d0efb50) at kernel/kexec_core.c:965
#22 0xffff00000808dc84 in die (str=<optimized out>, regs=0xffff80002d0efb50, err=<optimized out>) at arch/arm64/kernel/traps.c:210
#23 0xffff0000080a2114 in die_kernel_fault (msg=0xffff000008a09c88 "NULL pointer dereference", addr=0, esr=2516582468, regs=<optimized out>)
    at arch/arm64/mm/fault.c:269
#24 0xffff0000080a1d68 in __do_kernel_fault (addr=0, esr=2516582468, regs=0xffff80002d0efb50) at arch/arm64/mm/fault.c:297
#25 0xffff000008806e38 in do_page_fault (addr=0, esr=2516582468, regs=0xffff80002d0efb50) at arch/arm64/mm/fault.c:599
#26 0xffff0000088070dc in do_translation_fault (addr=0, esr=<optimized out>, regs=<optimized out>) at arch/arm64/mm/fault.c:608
#27 0xffff0000080812cc in do_mem_abort (addr=0, esr=2516582468, regs=0xffff80002d0efb50) at arch/arm64/mm/fault.c:744
#28 0xffff000008082ed0 in el1_sync () at arch/arm64/kernel/entry.S:583
Backtrace stopped: previous frame identical to this frame (corrupt stack?)
(gdb)

The system hanged in the idle thread after continuing here.

Petr T

^ permalink raw reply

* [PATCH 09/10] dpaa_eth: add support for hardware timestamping
From: Y.b. Lu @ 2018-06-06 11:48 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180605135748.mlarwiyzf2oe27ax@localhost>

Hi Richard,

> -----Original Message-----
> From: Richard Cochran [mailto:richardcochran at gmail.com]
> Sent: Tuesday, June 5, 2018 9:58 PM
> To: Y.b. Lu <yangbo.lu@nxp.com>
> Cc: netdev at vger.kernel.org; Madalin-cristian Bucur
> <madalin.bucur@nxp.com>; Rob Herring <robh+dt@kernel.org>; Shawn Guo
> <shawnguo@kernel.org>; David S . Miller <davem@davemloft.net>;
> devicetree at vger.kernel.org; linuxppc-dev at lists.ozlabs.org;
> linux-arm-kernel at lists.infradead.org; linux-kernel at vger.kernel.org
> Subject: Re: [PATCH 09/10] dpaa_eth: add support for hardware timestamping
> 
> On Tue, Jun 05, 2018 at 03:35:28AM +0000, Y.b. Lu wrote:
> > [Y.b. Lu] Actually these timestamping codes affected DPAA networking
> performance in our previous performance test.
> > That's why we used ifdef for it.
> 
> How much does time stamping hurt performance?
> 
> If the time stamping is compiled in but not enabled at run time, does it still
> affect performace?

[Y.b. Lu] I can't remember and find the old data since it had been a long time.
I just did the iperf test today between two 10G ports. I didn?t see any performance changes with timestamping code ?
So, let's me remove the ifdef in next version.
Thanks a lot.


> 
> Thanks,
> Richard

^ permalink raw reply

* [RFC PATCH v2 0/6] serial: uartps: Add run time support for more IPs than hardcoded 2
From: Michal Simek @ 2018-06-06 12:41 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

this series is trying to address discussion I had with Alan in past
https://patchwork.kernel.org/patch/9738445/ and also with Rob in v1
https://lkml.org/lkml/2018/4/26/551.

The first 5 patches are preparation patches to have the last patch as
small as possible to focus on changes there.

Cases without DT aliases are not solved in this series but one function
was shared in RFC v1.
The purpose of this series to get feedback on solution where every
driver instance allocate at run time own uart_driver.

For example this is how it works.
uart0 on higher alias
serial0 = &uart1;
serial100 = &uart0;

~# ls -la /dev/ttyPS*
crw-------    1 root     root      252,   0 Jun  6 12:19 /dev/ttyPS0
crw--w----    1 root     root      253, 100 Jan  1  1970 /dev/ttyPS100

Thanks,
Michal


Changes in v2:
- new patch - it can be sent separately too
- Remove nr field logic
- new patch - it can be sent separately too
- new patch - it can be sent separately too
- new patch - it can be sent separately too
- Register one uart_driver with unique minor at probe time

Michal Simek (6):
  serial: uartps: Do not initialize field to zero again
  serial: uartps: Move register to probe based on run time detection
  serial: uartps: Do not use static struct uart_driver out of probe()
  serial: uartps: Move alias reading higher in probe()
  serial: uartps: Fill struct uart_driver in probe()
  serial: uartps: Remove CDNS_UART_NR_PORTS macro

 drivers/tty/serial/xilinx_uartps.c | 126 ++++++++++++++++++++-----------------
 1 file changed, 68 insertions(+), 58 deletions(-)

-- 
1.9.1

^ permalink raw reply

* [RFC PATCH v2 1/6] serial: uartps: Do not initialize field to zero again
From: Michal Simek @ 2018-06-06 12:41 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <cover.1528288895.git.michal.simek@xilinx.com>

Writing zero and NULLs to already initialized fields is not needed.
Remove this additional writes.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

Changes in v2:
- new patch - it can be sent separately too

 drivers/tty/serial/xilinx_uartps.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 8a3e34234e98..5f116f3ecd4a 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -1510,15 +1510,12 @@ static int cdns_uart_probe(struct platform_device *pdev)
 
 	/* At this point, we've got an empty uart_port struct, initialize it */
 	spin_lock_init(&port->lock);
-	port->membase	= NULL;
-	port->irq	= 0;
 	port->type	= PORT_UNKNOWN;
 	port->iotype	= UPIO_MEM32;
 	port->flags	= UPF_BOOT_AUTOCONF;
 	port->ops	= &cdns_uart_ops;
 	port->fifosize	= CDNS_UART_FIFO_SIZE;
 	port->line	= id;
-	port->dev	= NULL;
 
 	/*
 	 * Register the port.
-- 
1.9.1

^ permalink raw reply related

* [RFC PATCH v2 2/6] serial: uartps: Move register to probe based on run time detection
From: Michal Simek @ 2018-06-06 12:41 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <cover.1528288895.git.michal.simek@xilinx.com>

Find out the highest serial alias and allocate that amount of
structures/minor numbers to be able to handle all of them.
Origin setting that there are two prealocated CDNS_UART_NR_PORTS is kept
there.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

Changes in v2:
- Remove nr field logic

Discussed here: https://patchwork.kernel.org/patch/9738445/

The same solution is done in pl011 driver.

---
 drivers/tty/serial/xilinx_uartps.c | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 5f116f3ecd4a..e24382f58dea 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -1438,6 +1438,14 @@ static int cdns_uart_probe(struct platform_device *pdev)
 	if (!port)
 		return -ENOMEM;
 
+	if (!cdns_uart_uart_driver.state) {
+		rc = uart_register_driver(&cdns_uart_uart_driver);
+		if (rc < 0) {
+			dev_err(&pdev->dev, "Failed to register driver\n");
+			return rc;
+		}
+	}
+
 	match = of_match_node(cdns_uart_of_match, pdev->dev.of_node);
 	if (match && match->data) {
 		const struct cdns_platform_data *data = match->data;
@@ -1617,28 +1625,14 @@ static int cdns_uart_remove(struct platform_device *pdev)
 
 static int __init cdns_uart_init(void)
 {
-	int retval = 0;
-
-	/* Register the cdns_uart driver with the serial core */
-	retval = uart_register_driver(&cdns_uart_uart_driver);
-	if (retval)
-		return retval;
-
 	/* Register the platform driver */
-	retval = platform_driver_register(&cdns_uart_platform_driver);
-	if (retval)
-		uart_unregister_driver(&cdns_uart_uart_driver);
-
-	return retval;
+	return platform_driver_register(&cdns_uart_platform_driver);
 }
 
 static void __exit cdns_uart_exit(void)
 {
 	/* Unregister the platform driver */
 	platform_driver_unregister(&cdns_uart_platform_driver);
-
-	/* Unregister the cdns_uart driver */
-	uart_unregister_driver(&cdns_uart_uart_driver);
 }
 
 arch_initcall(cdns_uart_init);
-- 
1.9.1

^ permalink raw reply related

* [RFC PATCH v2 3/6] serial: uartps: Do not use static struct uart_driver out of probe()
From: Michal Simek @ 2018-06-06 12:41 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <cover.1528288895.git.michal.simek@xilinx.com>

cdns_uart_suspend()/resume() and remove() are using static reference
to struct uart_driver. Assign this referece to private data structure
as preparation step for dynamic struct uart_driver allocation.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

Changes in v2:
- new patch - it can be sent separately too

 drivers/tty/serial/xilinx_uartps.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index e24382f58dea..fe96fd950d3a 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -179,6 +179,7 @@
  * @port:		Pointer to the UART port
  * @uartclk:		Reference clock
  * @pclk:		APB clock
+ * @cdns_uart_driver:	Pointer to UART driver
  * @baud:		Current baud rate
  * @clk_rate_change_nb:	Notifier block for clock changes
  * @quirks:		Flags for RXBS support.
@@ -187,6 +188,7 @@ struct cdns_uart {
 	struct uart_port	*port;
 	struct clk		*uartclk;
 	struct clk		*pclk;
+	struct uart_driver	*cdns_uart_driver;
 	unsigned int		baud;
 	struct notifier_block	clk_rate_change_nb;
 	u32			quirks;
@@ -1280,6 +1282,7 @@ static int __init cdns_uart_console_setup(struct console *co, char *options)
 static int cdns_uart_suspend(struct device *device)
 {
 	struct uart_port *port = dev_get_drvdata(device);
+	struct cdns_uart *cdns_uart = port->private_data;
 	struct tty_struct *tty;
 	struct device *tty_dev;
 	int may_wake = 0;
@@ -1296,7 +1299,7 @@ static int cdns_uart_suspend(struct device *device)
 	 * Call the API provided in serial_core.c file which handles
 	 * the suspend.
 	 */
-	uart_suspend_port(&cdns_uart_uart_driver, port);
+	uart_suspend_port(cdns_uart->cdns_uart_driver, port);
 	if (!(console_suspend_enabled && !may_wake)) {
 		unsigned long flags = 0;
 
@@ -1324,6 +1327,7 @@ static int cdns_uart_suspend(struct device *device)
 static int cdns_uart_resume(struct device *device)
 {
 	struct uart_port *port = dev_get_drvdata(device);
+	struct cdns_uart *cdns_uart = port->private_data;
 	unsigned long flags = 0;
 	u32 ctrl_reg;
 	struct tty_struct *tty;
@@ -1339,8 +1343,6 @@ static int cdns_uart_resume(struct device *device)
 	}
 
 	if (console_suspend_enabled && !may_wake) {
-		struct cdns_uart *cdns_uart = port->private_data;
-
 		clk_enable(cdns_uart->pclk);
 		clk_enable(cdns_uart->uartclk);
 
@@ -1374,7 +1376,7 @@ static int cdns_uart_resume(struct device *device)
 		spin_unlock_irqrestore(&port->lock, flags);
 	}
 
-	return uart_resume_port(&cdns_uart_uart_driver, port);
+	return uart_resume_port(cdns_uart->cdns_uart_driver, port);
 }
 #endif /* ! CONFIG_PM_SLEEP */
 static int __maybe_unused cdns_runtime_suspend(struct device *dev)
@@ -1446,6 +1448,8 @@ static int cdns_uart_probe(struct platform_device *pdev)
 		}
 	}
 
+	cdns_uart_data->cdns_uart_driver = &cdns_uart_uart_driver;
+
 	match = of_match_node(cdns_uart_of_match, pdev->dev.of_node);
 	if (match && match->data) {
 		const struct cdns_platform_data *data = match->data;
@@ -1603,7 +1607,7 @@ static int cdns_uart_remove(struct platform_device *pdev)
 	clk_notifier_unregister(cdns_uart_data->uartclk,
 			&cdns_uart_data->clk_rate_change_nb);
 #endif
-	rc = uart_remove_one_port(&cdns_uart_uart_driver, port);
+	rc = uart_remove_one_port(cdns_uart_data->cdns_uart_driver, port);
 	port->mapbase = 0;
 	clk_disable_unprepare(cdns_uart_data->uartclk);
 	clk_disable_unprepare(cdns_uart_data->pclk);
-- 
1.9.1

^ permalink raw reply related


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