* Re: [RFC PATCH v1 00/25] printk: new implementation
From: Sergey Senozhatsky @ 2019-03-13 8:46 UTC (permalink / raw)
To: John Ogness
Cc: Sergey Senozhatsky, Petr Mladek, linux-kernel, Peter Zijlstra,
Steven Rostedt, Daniel Wang, Andrew Morton, Linus Torvalds,
Greg Kroah-Hartman, Alan Cox, Jiri Slaby, Peter Feiner,
linux-serial, Sergey Senozhatsky, Sebastian Siewior
In-Reply-To: <87d0mv9off.fsf@linutronix.de>
On (03/13/19 09:19), John Ogness wrote:
> >> Yes. I will post a series that only implements the ringbuffer using
> >> your simplified API. That will be enough to remove printk_safe and
> >> actually does most of the work of updating devkmsg, kmsg_dump, and
> >> syslog.
> >
> > This may _not_ be enough to remove printk_safe. One of the reasons
> > printk_safe "condom" came into existence was console_sem (which
> > is a bit too important to ignore it):
> >
> > printk()
> > console_trylock()
> > console_unlock()
> > up()
> > raw_spin_lock_irqsave(&sem->lock, flags)
> > __up()
> > wake_up_process()
> > WARN/etc
> > printk()
> > console_trylock()
> > down_trylock()
> > raw_spin_lock_irqsave(&sem->lock, flags) << deadlock
> >
> > Back then we were looking at
> >
> > printk->console_sem->lock->printk->console_sem->lock
> >
> > deadlock report from LG, if I'm not mistaken.
>
> The main drawback of printk_safe is the safe buffers, which, aside from
> bogus timestamping, may never make it back to the printk log buffer.
>
> With the new ring buffer the safe buffers are not needed, even in the
> recursive situation. As you are pointing out, the notification/wake
> component of printk_safe will still be needed. I will leave that (small)
> part in printk_safe.c.
Yeah, all I'm saying is that as it stands new printk() is missing a huge
and necessary part - console semaphore. And things can get very different
once you add that missing part. It brings a lot of stuff back to printk.
logbuf and logbuf_lock are not really huge printk problems. scheduler,
timekeeping locks, etc. are much bigger ones. Those dependencies don't
come from logbuf/logbuf_lock.
-ss
^ permalink raw reply
* Re: Serial console is causing system lock-up
From: John Ogness @ 2019-03-13 8:43 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Petr Mladek, Nigel Croxon, Theodore Y. Ts'o,
Greg Kroah-Hartman, Steven Rostedt, Sergey Senozhatsky, dm-devel,
Mikulas Patocka, linux-serial
In-Reply-To: <20190313023836.GC783@jagdpanzerIV>
On 2019-03-13, Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> wrote:
>>> The current printk implementation is handling all console printing
>>> as best effort. Trying hard enough to dramatically affect the
>>> system, but not trying hard enough to guarantee success.
>>
>> I agree that direct output is more reliable. It might be very useful
>> for debugging some types of problems. The question is
>> if it is worth the cost (code complexity, serializing CPUs
>> == slowing down the entire system).
>
> Agreed.
>
> I'm very skeptical about "serializing CPUs" part. It looks like one
> "print or die trying" is replaced with another "print or die trying".
> What happened to log_store() + flush_on_panic()?
We are literally discussing in a thread where the current printk
implementation failed to get messages out (lots of dropped messages)
_and_ printk console printing was responsible for _killing_ the machine.
What would my proposal do in _this_ situation:
1. If no emergency console was available or the messages were not
classified as emergency, messages would have been dropped during console
printing and the system would have run unaffected. The number of dropped
messages might not even be more if the scheduler could run the printk
kthread effectively.
OR
2. If an emergency console was available and the messages were
classified as emergency, _no_ messages would have dropped, the system
would have become very slow on the CPUs generating the messages, and
then eventually it would have recovered.
I don't understand how you can think "print or die trying" is replaced
with another "print or die trying". But it is probably not constructive
to debate this right now. Petr has laid out a good course that will
allow us to advance in smaller, more conservative, steps.
By the way, Sergey, I appreciate your skepticism.
John Ogness
^ permalink raw reply
* Re: [RFC PATCH v1 00/25] printk: new implementation
From: Sebastian Siewior @ 2019-03-13 8:40 UTC (permalink / raw)
To: John Ogness
Cc: Sergey Senozhatsky, Petr Mladek, linux-kernel, Peter Zijlstra,
Steven Rostedt, Daniel Wang, Andrew Morton, Linus Torvalds,
Greg Kroah-Hartman, Alan Cox, Jiri Slaby, Peter Feiner,
linux-serial, Sergey Senozhatsky
In-Reply-To: <87d0mv9off.fsf@linutronix.de>
On 2019-03-13 09:19:32 [+0100], John Ogness wrote:
> recursive situation. As you are pointing out, the notification/wake
> component of printk_safe will still be needed. I will leave that (small)
> part in printk_safe.c.
Does this mean we keep irq_work part or we bury it and solve it by other
means?
> John Ogness
Sebastian
^ permalink raw reply
* Re: [PATCH v8 2/2] arm64: dts: Add Mediatek SoC MT8183 and evaluation board dts and Makefile
From: Erin Lo @ 2019-03-13 8:26 UTC (permalink / raw)
To: Matthias Brugger
Cc: Rob Herring, Mark Rutland, Thomas Gleixner, Jason Cooper,
Marc Zyngier, Greg Kroah-Hartman, Stephen Boyd, devicetree,
srv_heupstream, linux-kernel, linux-serial, linux-mediatek,
linux-arm-kernel, yingjoe.chen, mars.cheng, eddie.huang,
linux-clk, zhiyong.tao, Mengqi.Zhang, Ben Ho, Seiya Wang,
Weiyi Lu
In-Reply-To: <d81e9ae0-f571-a890-75e7-be8242ad3879@gmail.com>
On Tue, 2019-03-12 at 13:22 +0100, Matthias Brugger wrote:
>
> On 11/03/2019 09:54, Erin Lo wrote:
> > From: Ben Ho <Ben.Ho@mediatek.com>
> >
> > Add basic chip support for Mediatek 8183, include
> > uart node with correct uart clocks, pwrap device
> >
> > Add clock controller nodes, include topckgen, infracfg,
> > apmixedsys and subsystem.
> >
> > Signed-off-by: Ben Ho <Ben.Ho@mediatek.com>
> > Signed-off-by: Erin Lo <erin.lo@mediatek.com>
> > Signed-off-by: Seiya Wang <seiya.wang@mediatek.com>
> > Signed-off-by: Weiyi Lu <weiyi.lu@mediatek.com>
> > Signed-off-by: Zhiyong Tao <zhiyong.tao@mediatek.com>
> > Signed-off-by: Hsin-Hsiung Wang <hsin-hsiung.wang@mediatek.com>
> > Signed-off-by: Eddie Huang <eddie.huang@mediatek.com>
> > ---
> > arch/arm64/boot/dts/mediatek/Makefile | 1 +
> > arch/arm64/boot/dts/mediatek/mt8183-evb.dts | 31 +++
> > arch/arm64/boot/dts/mediatek/mt8183.dtsi | 335 ++++++++++++++++++++++++++++
> > 3 files changed, 367 insertions(+)
> > create mode 100644 arch/arm64/boot/dts/mediatek/mt8183-evb.dts
> > create mode 100644 arch/arm64/boot/dts/mediatek/mt8183.dtsi
> >
> > diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile
> > index e8f952f..458bbc4 100644
> > --- a/arch/arm64/boot/dts/mediatek/Makefile
> > +++ b/arch/arm64/boot/dts/mediatek/Makefile
> > @@ -7,3 +7,4 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-x20-dev.dtb
> > dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-rfb1.dtb
> > dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-bananapi-bpi-r64.dtb
> > dtb-$(CONFIG_ARCH_MEDIATEK) += mt8173-evb.dtb
> > +dtb-$(CONFIG_ARCH_MEDIATEK) += mt8183-evb.dtb
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8183-evb.dts b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
> > new file mode 100644
> > index 0000000..9b52559
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
> > @@ -0,0 +1,31 @@
> > +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> > +/*
> > + * Copyright (c) 2018 MediaTek Inc.
> > + * Author: Ben Ho <ben.ho@mediatek.com>
> > + * Erin Lo <erin.lo@mediatek.com>
> > + */
> > +
> > +/dts-v1/;
> > +#include "mt8183.dtsi"
> > +
> > +/ {
> > + model = "MediaTek MT8183 evaluation board";
> > + compatible = "mediatek,mt8183-evb", "mediatek,mt8183";
> > +
> > + aliases {
> > + serial0 = &uart0;
> > + };
> > +
> > + memory@40000000 {
> > + device_type = "memory";
> > + reg = <0 0x40000000 0 0x80000000>;
> > + };
> > +
> > + chosen {
> > + stdout-path = "serial0:921600n8";
> > + };
> > +};
> > +
> > +&uart0 {
> > + status = "okay";
> > +};
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > new file mode 100644
> > index 0000000..64f8bd6
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > @@ -0,0 +1,335 @@
> > +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> > +/*
> > + * Copyright (c) 2018 MediaTek Inc.
> > + * Author: Ben Ho <ben.ho@mediatek.com>
> > + * Erin Lo <erin.lo@mediatek.com>
> > + */
> > +
> > +#include <dt-bindings/clock/mt8183-clk.h>
> > +#include <dt-bindings/interrupt-controller/arm-gic.h>
> > +#include <dt-bindings/interrupt-controller/irq.h>
> > +
> > +/ {
> > + compatible = "mediatek,mt8183";
> > + interrupt-parent = <&sysirq>;
> > + #address-cells = <2>;
> > + #size-cells = <2>;
> > +
> > + cpus {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + cpu-map {
> > + cluster0 {
> > + core0 {
> > + cpu = <&cpu0>;
> > + };
> > + core1 {
> > + cpu = <&cpu1>;
> > + };
> > + core2 {
> > + cpu = <&cpu2>;
> > + };
> > + core3 {
> > + cpu = <&cpu3>;
> > + };
> > + };
> > +
> > + cluster1 {
> > + core0 {
> > + cpu = <&cpu4>;
> > + };
> > + core1 {
> > + cpu = <&cpu5>;
> > + };
> > + core2 {
> > + cpu = <&cpu6>;
> > + };
> > + core3 {
> > + cpu = <&cpu7>;
> > + };
> > + };
> > + };
> > +
> > + cpu0: cpu@0 {
> > + device_type = "cpu";
> > + compatible = "arm,cortex-a53";
> > + reg = <0x000>;
> > + enable-method = "psci";
> > + };
> > +
> > + cpu1: cpu@1 {
> > + device_type = "cpu";
> > + compatible = "arm,cortex-a53";
> > + reg = <0x001>;
> > + enable-method = "psci";
> > + };
> > +
> > + cpu2: cpu@2 {
> > + device_type = "cpu";
> > + compatible = "arm,cortex-a53";
> > + reg = <0x002>;
> > + enable-method = "psci";
> > + };
> > +
> > + cpu3: cpu@3 {
> > + device_type = "cpu";
> > + compatible = "arm,cortex-a53";
> > + reg = <0x003>;
> > + enable-method = "psci";
> > + };
> > +
> > + cpu4: cpu@100 {
> > + device_type = "cpu";
> > + compatible = "arm,cortex-a73";
> > + reg = <0x100>;
> > + enable-method = "psci";
> > + };
> > +
> > + cpu5: cpu@101 {
> > + device_type = "cpu";
> > + compatible = "arm,cortex-a73";
> > + reg = <0x101>;
> > + enable-method = "psci";
> > + };
> > +
> > + cpu6: cpu@102 {
> > + device_type = "cpu";
> > + compatible = "arm,cortex-a73";
> > + reg = <0x102>;
> > + enable-method = "psci";
> > + };
> > +
> > + cpu7: cpu@103 {
> > + device_type = "cpu";
> > + compatible = "arm,cortex-a73";
> > + reg = <0x103>;
> > + enable-method = "psci";
> > + };
> > + };
> > +
> > + pmu-a53 {
> > + compatible = "arm,cortex-a53-pmu";
> > + interrupt-parent = <&gic>;
> > + interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_LOW &ppi_cluster0>;
> > + };
> > +
> > + pmu-a73 {
> > + compatible = "arm,cortex-a73-pmu";
> > + interrupt-parent = <&gic>;
> > + interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_LOW &ppi_cluster1>;
> > + };
> > +
> > + psci {
> > + compatible = "arm,psci-1.0";
> > + method = "smc";
> > + };
> > +
> > + clk26m: oscillator {
> > + compatible = "fixed-clock";
> > + #clock-cells = <0>;
> > + clock-frequency = <26000000>;
> > + clock-output-names = "clk26m";
> > + };
> > +
> > + timer {
> > + compatible = "arm,armv8-timer";
> > + interrupt-parent = <&gic>;
> > + interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_LOW 0>,
> > + <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW 0>,
> > + <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW 0>,
> > + <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW 0>;
> > + };
> > +
> > + soc {
> > + #address-cells = <2>;
> > + #size-cells = <2>;
> > + compatible = "simple-bus";
> > + ranges;
> > +
> > + gic: interrupt-controller@c000000 {
> > + compatible = "arm,gic-v3";
> > + #interrupt-cells = <4>;
> > + interrupt-parent = <&gic>;
> > + interrupt-controller;
> > + reg = <0 0x0c000000 0 0x40000>, /* GICD */
> > + <0 0x0c100000 0 0x200000>, /* GICR */
> > + <0 0x0c400000 0 0x2000>, /* GICC */
> > + <0 0x0c410000 0 0x1000>, /* GICH */
> > + <0 0x0c420000 0 0x2000>; /* GICV */
> > +
> > + interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH 0>;
> > + ppi-partitions {
> > + ppi_cluster0: interrupt-partition-0 {
> > + affinity = <&cpu0 &cpu1 &cpu2 &cpu3>;
> > + };
> > + ppi_cluster1: interrupt-partition-1 {
> > + affinity = <&cpu4 &cpu5 &cpu6 &cpu7>;
> > + };
> > + };
> > + };
> > +
> > + mcucfg: syscon@c530000 {
> > + compatible = "mediatek,mt8183-mcucfg", "syscon";
>
> Binding is not documented. I found some other bindings not documented as well.
> Please update the binding documentation to add this compatible, otherwise I
> won't be able to take this patch.
>
> Regards,
> Matthias
>
The binding is
http://lists.infradead.org/pipermail/linux-mediatek/2018-December/016249.html
which is included in below series as cover letter mentioned.
http://lists.infradead.org/pipermail/linux-mediatek/2018-December/016243.html
Do you have any idea about how we arrange it in next version?
> > + reg = <0 0x0c530000 0 0x1000>;
> > + #clock-cells = <1>;
> > + };
> > +
> > + sysirq: interrupt-controller@c530a80 {
> > + compatible = "mediatek,mt8183-sysirq",
> > + "mediatek,mt6577-sysirq";
> > + interrupt-controller;
> > + #interrupt-cells = <3>;
> > + interrupt-parent = <&gic>;
> > + reg = <0 0x0c530a80 0 0x50>;
> > + };
> > +
> > + topckgen: syscon@10000000 {
> > + compatible = "mediatek,mt8183-topckgen", "syscon";
> > + reg = <0 0x10000000 0 0x1000>;
> > + #clock-cells = <1>;
> > + };
> > +
> > + infracfg: syscon@10001000 {
> > + compatible = "mediatek,mt8183-infracfg", "syscon";
> > + reg = <0 0x10001000 0 0x1000>;
> > + #clock-cells = <1>;
> > + };
> > +
> > + pio: pinctrl@10005000 {
> > + compatible = "mediatek,mt8183-pinctrl";
> > + reg = <0 0x10005000 0 0x1000>,
> > + <0 0x11f20000 0 0x1000>,
> > + <0 0x11e80000 0 0x1000>,
> > + <0 0x11e70000 0 0x1000>,
> > + <0 0x11e90000 0 0x1000>,
> > + <0 0x11d30000 0 0x1000>,
> > + <0 0x11d20000 0 0x1000>,
> > + <0 0x11c50000 0 0x1000>,
> > + <0 0x11f30000 0 0x1000>,
> > + <0 0x1000b000 0 0x1000>;
> > + reg-names = "iocfg0", "iocfg1", "iocfg2",
> > + "iocfg3", "iocfg4", "iocfg5",
> > + "iocfg6", "iocfg7", "iocfg8",
> > + "eint";
> > + gpio-controller;
> > + #gpio-cells = <2>;
> > + gpio-ranges = <&pio 0 0 192>;
> > + interrupt-controller;
> > + interrupts = <GIC_SPI 177 IRQ_TYPE_LEVEL_HIGH>;
> > + #interrupt-cells = <3>;
> > + };
I will remove it in next version since the binding is not ready.
Best Regards,
Erin.
> > +
> > + apmixedsys: syscon@1000c000 {
> > + compatible = "mediatek,mt8183-apmixedsys", "syscon";
> > + reg = <0 0x1000c000 0 0x1000>;
> > + #clock-cells = <1>;
> > + };
> > +
> > + pwrap: pwrap@1000d000 {
> > + compatible = "mediatek,mt8183-pwrap";
> > + reg = <0 0x1000d000 0 0x1000>;
> > + reg-names = "pwrap";
> > + interrupts = <GIC_SPI 209 IRQ_TYPE_LEVEL_HIGH>;
> > + clocks = <&topckgen CLK_TOP_MUX_PMICSPI>,
> > + <&infracfg CLK_INFRA_PMIC_AP>;
> > + clock-names = "spi", "wrap";
> > + };
> > +
> > + uart0: serial@11002000 {
> > + compatible = "mediatek,mt8183-uart",
> > + "mediatek,mt6577-uart";
> > + reg = <0 0x11002000 0 0x1000>;
> > + interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_LOW>;
> > + clocks = <&clk26m>, <&infracfg CLK_INFRA_UART0>;
> > + clock-names = "baud", "bus";
> > + status = "disabled";
> > + };
> > +
> > + uart1: serial@11003000 {
> > + compatible = "mediatek,mt8183-uart",
> > + "mediatek,mt6577-uart";
> > + reg = <0 0x11003000 0 0x1000>;
> > + interrupts = <GIC_SPI 92 IRQ_TYPE_LEVEL_LOW>;
> > + clocks = <&clk26m>, <&infracfg CLK_INFRA_UART1>;
> > + clock-names = "baud", "bus";
> > + status = "disabled";
> > + };
> > +
> > + uart2: serial@11004000 {
> > + compatible = "mediatek,mt8183-uart",
> > + "mediatek,mt6577-uart";
> > + reg = <0 0x11004000 0 0x1000>;
> > + interrupts = <GIC_SPI 93 IRQ_TYPE_LEVEL_LOW>;
> > + clocks = <&clk26m>, <&infracfg CLK_INFRA_UART2>;
> > + clock-names = "baud", "bus";
> > + status = "disabled";
> > + };
> > +
> > + audiosys: syscon@11220000 {
> > + compatible = "mediatek,mt8183-audiosys", "syscon";
> > + reg = <0 0x11220000 0 0x1000>;
> > + #clock-cells = <1>;
> > + };
> > +
> > + mfgcfg: syscon@13000000 {
> > + compatible = "mediatek,mt8183-mfgcfg", "syscon";
> > + reg = <0 0x13000000 0 0x1000>;
> > + #clock-cells = <1>;
> > + };
> > +
> > + mmsys: syscon@14000000 {
> > + compatible = "mediatek,mt8183-mmsys", "syscon";
> > + reg = <0 0x14000000 0 0x1000>;
> > + #clock-cells = <1>;
> > + };
> > +
> > + imgsys: syscon@15020000 {
> > + compatible = "mediatek,mt8183-imgsys", "syscon";
> > + reg = <0 0x15020000 0 0x1000>;
> > + #clock-cells = <1>;
> > + };
> > +
> > + vdecsys: syscon@16000000 {
> > + compatible = "mediatek,mt8183-vdecsys", "syscon";
> > + reg = <0 0x16000000 0 0x1000>;
> > + #clock-cells = <1>;
> > + };
> > +
> > + vencsys: syscon@17000000 {
> > + compatible = "mediatek,mt8183-vencsys", "syscon";
> > + reg = <0 0x17000000 0 0x1000>;
> > + #clock-cells = <1>;
> > + };
> > +
> > + ipu_conn: syscon@19000000 {
> > + compatible = "mediatek,mt8183-ipu_conn", "syscon";
> > + reg = <0 0x19000000 0 0x1000>;
> > + #clock-cells = <1>;
> > + };
> > +
> > + ipu_adl: syscon@19010000 {
> > + compatible = "mediatek,mt8183-ipu_adl", "syscon";
> > + reg = <0 0x19010000 0 0x1000>;
> > + #clock-cells = <1>;
> > + };
> > +
> > + ipu_core0: syscon@19180000 {
> > + compatible = "mediatek,mt8183-ipu_core0", "syscon";
> > + reg = <0 0x19180000 0 0x1000>;
> > + #clock-cells = <1>;
> > + };
> > +
> > + ipu_core1: syscon@19280000 {
> > + compatible = "mediatek,mt8183-ipu_core1", "syscon";
> > + reg = <0 0x19280000 0 0x1000>;
> > + #clock-cells = <1>;
> > + };
> > +
> > + camsys: syscon@1a000000 {
> > + compatible = "mediatek,mt8183-camsys", "syscon";
> > + reg = <0 0x1a000000 0 0x1000>;
> > + #clock-cells = <1>;
> > + };
> > + };
> > +};
> >
^ permalink raw reply
* Re: [RFC PATCH v1 00/25] printk: new implementation
From: John Ogness @ 2019-03-13 8:19 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Petr Mladek, linux-kernel, Peter Zijlstra, Steven Rostedt,
Daniel Wang, Andrew Morton, Linus Torvalds, Greg Kroah-Hartman,
Alan Cox, Jiri Slaby, Peter Feiner, linux-serial,
Sergey Senozhatsky, Sebastian Siewior
In-Reply-To: <20190313021534.GB783@jagdpanzerIV>
On 2019-03-13, Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> wrote:
>>> I suggest the following way forward (separate patchsets):
>>>
>>> 1. Replace log buffer (least controversial thing)
>>
>> Yes. I will post a series that only implements the ringbuffer using
>> your simplified API. That will be enough to remove printk_safe and
>> actually does most of the work of updating devkmsg, kmsg_dump, and
>> syslog.
>
> This may _not_ be enough to remove printk_safe. One of the reasons
> printk_safe "condom" came into existence was console_sem (which
> is a bit too important to ignore it):
>
> printk()
> console_trylock()
> console_unlock()
> up()
> raw_spin_lock_irqsave(&sem->lock, flags)
> __up()
> wake_up_process()
> WARN/etc
> printk()
> console_trylock()
> down_trylock()
> raw_spin_lock_irqsave(&sem->lock, flags) << deadlock
>
> Back then we were looking at
>
> printk->console_sem->lock->printk->console_sem->lock
>
> deadlock report from LG, if I'm not mistaken.
The main drawback of printk_safe is the safe buffers, which, aside from
bogus timestamping, may never make it back to the printk log buffer.
With the new ring buffer the safe buffers are not needed, even in the
recursive situation. As you are pointing out, the notification/wake
component of printk_safe will still be needed. I will leave that (small)
part in printk_safe.c.
John Ogness
^ permalink raw reply
* Re: [PATCH 1/8] drivers: tty: serial: 8250_bcm2835aux: use devm_platform_ioremap_resource()
From: Enrico Weigelt, metux IT consult @ 2019-03-13 6:59 UTC (permalink / raw)
To: Greg KH, Enrico Weigelt, metux IT consult
Cc: jslaby, linux-serial, linux-kernel
In-Reply-To: <20190312163356.GB13549@kroah.com>
On 12.03.19 17:33, Greg KH wrote:
> On Tue, Mar 12, 2019 at 03:57:33PM +0100, Enrico Weigelt, metux IT consult wrote:
>> ---
>> drivers/tty/serial/8250/8250_bcm2835aux.c | 12 ++++--------
>> 1 file changed, 4 insertions(+), 8 deletions(-)
>
> No changelog or signed-off-by, sorry, please fix up the series and
> resend.
Maybe the threading got messed up somehow (at least my tbird doesn't
show it correctly), so you've probably didn't see my intro letter: there
I wrote that this is yet WIP, not meant for actual submission, instead
just to ask you folks whether my approach in general would be work.
Sorry for the confusion.
Of course, when the stuff becomes ready for submission, all these
things will be cleaned up.
OTOH, if you think some of the patches indeed were ready, but yet need
some cleanups (like mentioned missing description), just let me know.
--mtx
--
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287
^ permalink raw reply
* Re: Serial console is causing system lock-up
From: Sergey Senozhatsky @ 2019-03-13 2:38 UTC (permalink / raw)
To: Petr Mladek
Cc: Nigel Croxon, Theodore Y. Ts'o, Sergey Senozhatsky,
John Ogness, Greg Kroah-Hartman, Steven Rostedt,
Sergey Senozhatsky, dm-devel, Mikulas Patocka, linux-serial
In-Reply-To: <20190312120824.4eaa4eyjcxvuzm23@pathway.suse.cz>
On (03/12/19 13:08), Petr Mladek wrote:
> > 2. You seem unwilling to acknowledge the difference between emergency
> > and informational messages. A message is either critical or it is
> > not. If it is, it should be handled as such, regardless of
> > interference, regardless if it means turning an SMP machine into a UP
> > machine. If it is not critical, it should be sent along a
> > non-interfering path so the the system is _not_ affected.
>
> This means that any critical message is always more important than any
> workload. It opens doors for iteresting DOS attacks.
Agreed.
So what I thought about "mutual exclusive error reporting with
quadratic latencies" was that if any workload involves error
reporting then it might be a good idea to _not_ parallelize such
workload anymore.
> > The current printk implementation is handling all console printing as
> > best effort. Trying hard enough to dramatically affect the system, but
> > not trying hard enough to guarantee success.
>
> I agree that direct output is more reliable. It might be very useful
> for debugging some types of problems. The question is
> if it is worth the cost (code complexity, serializing CPUs
> == slowing down the entire system).
Agreed.
I'm very skeptical about "serializing CPUs" part. It looks like one
"print or die trying" is replaced with another "print or die trying".
What happened to log_store() + flush_on_panic()?
-ss
^ permalink raw reply
* Re: [RFC PATCH v1 00/25] printk: new implementation
From: Sergey Senozhatsky @ 2019-03-13 2:15 UTC (permalink / raw)
To: John Ogness
Cc: Petr Mladek, Sergey Senozhatsky, linux-kernel, Peter Zijlstra,
Steven Rostedt, Daniel Wang, Andrew Morton, Linus Torvalds,
Greg Kroah-Hartman, Alan Cox, Jiri Slaby, Peter Feiner,
linux-serial, Sergey Senozhatsky, Sebastian Siewior
In-Reply-To: <874l8815uc.fsf@linutronix.de>
On (03/12/19 16:15), John Ogness wrote:
> > I suggest the following way forward (separate patchsets):
> >
> > 1. Replace log buffer (least controversial thing)
>
> Yes. I will post a series that only implements the ringbuffer using your
> simplified API. That will be enough to remove printk_safe and actually
> does most of the work of updating devkmsg, kmsg_dump, and syslog.
This may _not_ be enough to remove printk_safe. One of the reasons
printk_safe "condom" came into existence was console_sem (which
is a bit too important to ignore it):
printk()
console_trylock()
console_unlock()
up()
raw_spin_lock_irqsave(&sem->lock, flags)
__up()
wake_up_process()
WARN/etc
printk()
console_trylock()
down_trylock()
raw_spin_lock_irqsave(&sem->lock, flags) << deadlock
Back then we were looking at
printk->console_sem->lock->printk->console_sem->lock
deadlock report from LG, if I'm not mistaken.
-ss
^ permalink raw reply
* Re: [RFC PATCH v1 00/25] printk: new implementation
From: Sergey Senozhatsky @ 2019-03-13 2:00 UTC (permalink / raw)
To: Petr Mladek
Cc: Sergey Senozhatsky, John Ogness, linux-kernel, Peter Zijlstra,
Steven Rostedt, Daniel Wang, Andrew Morton, Linus Torvalds,
Greg Kroah-Hartman, Alan Cox, Jiri Slaby, Peter Feiner,
linux-serial, Sergey Senozhatsky
In-Reply-To: <20190312123857.juatd6fwtfmqajze@pathway.suse.cz>
On (03/12/19 13:38), Petr Mladek wrote:
> > Hmm. OK. So one of the things with printk is that it's fully sequential.
> > We call console drivers one by one. Slow consoles can affect what appears
> > on the fast consoles; fast console have no impact on slow ones.
> >
> > call_console_drivers()
> > for_each_console(c)
> > c->write(c, text, text_len);
> >
> > So a list of (slow_serial serial netcon) console drivers is a camel train;
> > fast netcon is not fast anymore, and slow consoles sometimes are the reason
> > we have dropped messages. And if we drop messages we drop them for all
> > consoles, including fast netcon. Turning that sequential pipline into a
> > bunch of per-console kthreads/irq and letting fast consoles to be fast is
> > not a completely bad thing. Let's think more about this, I'd like to read
> > more opinions.
>
> Per-console kthread sounds interesting but there is the problem with
> reliability. I mean that kthread need not get scheduled.
Correct, it has to get scheduled. From that point of view IRQ offloading
looks better - either to irq_work (like John suggested) or to serial
drivers' irq handler (poll uart xmit + logbuf).
kthread offloading is not super reliable. That's why I played tricks
with CPU affinity - scheduler sometimes schedule printk_kthread on the
same CPU which spins in console_unlock() loop printing the messages, so
printk_kthread offloading never happens. It was first discovered by Jan
Kara (back in the days of async-printk patch set). I think at some point
Jan's async-printk patch set had two printk kthreads.
We also had some concerns regarding offloading on UP systems.
> Some of these problems might get solved by the per-console loglevel
> patchset.
Yes, some.
> Sigh, any feature might be useful in some situation. But we always
> have to consider the cost and the gain. I wonder how common is
> to actively use two consoles at the same time and what would
> be the motivation.
Facebook fleet for example. The motivation is - to have a fancy fast
console that does things which simple serial consoles cannot do and
a slow serial console, which is sometimes more reliable, as last resort.
Fancy stuff usually means dependencies - net, mm, etc. So when fancy
console stop working, slow serial console still does.
-ss
^ permalink raw reply
* Re: [PATCH 3/4] printk: Add consoles to a virtual "console" bus
From: Calvin Owens @ 2019-03-12 21:52 UTC (permalink / raw)
To: Petr Mladek
Cc: Sergey Senozhatsky, Steven Rostedt, Greg Kroah-Hartman,
Jonathan Corbet, linux-kernel@vger.kernel.org,
linux-serial@vger.kernel.org
In-Reply-To: <20190311133341.vqcagdo4e4vy6dfu@pathway.suse.cz>
On Monday 03/11 at 14:33 +0100, Petr Mladek wrote:
> On Fri 2019-03-01 16:48:19, Calvin Owens wrote:
> > This patch embeds a device struct in the console struct, and registers
> > them on a "console" bus so we can expose attributes in sysfs.
> >
> > Currently, most drivers declare static console structs, and that is
> > incompatible with the dev refcount model. So we end up needing to patch
> > all of the console drivers to:
> >
> > 1. Dynamically allocate the console struct using a new helper
> > 2. Handle the allocation in (1) possibly failing
> > 3. Dispose of (1) with put_device()
> >
> > Early console structures must still be static, since they're required
> > before we're able to allocate memory. The least ugly way I can come up
> > with to handle this is an "is_static" flag in the structure which makes
> > the gets and puts NOPs, and is checked in ->release() to catch mistakes.
> >
> > diff --git a/drivers/char/lp.c b/drivers/char/lp.c
> > index 5c8d780637bd..e09cb192a469 100644
> > --- a/drivers/char/lp.c
> > +++ b/drivers/char/lp.c
> > @@ -857,12 +857,12 @@ static void lp_console_write(struct console *co, const char *s,
> > parport_release(dev);
> > }
> >
> > -static struct console lpcons = {
> > - .name = "lp",
> > +static const struct console_operations lp_cons_ops = {
> > .write = lp_console_write,
> > - .flags = CON_PRINTBUFFER,
> > };
> >
> > +static struct console *lpcons;
>
> I have got the following compilation error (see below):
>
> CC drivers/char/lp.o
> drivers/char/lp.c: In function ‘lp_register’:
> drivers/char/lp.c:925:2: error: ‘lpcons’ undeclared (first use in this function)
> lpcons = allocate_console_dfl(&lp_cons_ops, "lp", NULL);
> ^
> drivers/char/lp.c:925:2: note: each undeclared identifier is reported only once for each function it appears in
> In file included from drivers/char/lp.c:125:0:
> drivers/char/lp.c:925:33: error: ‘lp_cons_ops’ undeclared (first use in this function)
D'oh, will fix.
>
> > #endif /* console on line printer */
> >
> > /* --- initialisation code ------------------------------------- */
> > @@ -921,6 +921,11 @@ static int lp_register(int nr, struct parport *port)
> > &ppdev_cb, nr);
> > if (lp_table[nr].dev == NULL)
> > return 1;
> > +
> > + lpcons = allocate_console_dfl(&lp_cons_ops, "lp", NULL);
> > + if (!lpcons)
> > + return -ENOMEM;
>
> This should be done inside #ifdef CONFIG_LP_CONSOLE
> to avoid the above compilation error.
>
> > +
> > lp_table[nr].flags |= LP_EXIST;
> >
> > if (reset)
>
> [...]
> > diff --git a/include/linux/console.h b/include/linux/console.h
> > index 3c27a4a29b8c..382591683033 100644
> > --- a/include/linux/console.h
> > +++ b/include/linux/console.h
> > @@ -142,20 +143,28 @@ static inline int con_debug_leave(void)
> > #define CON_BRL (32) /* Used for a braille device */
> > #define CON_EXTENDED (64) /* Use the extended output format a la /dev/kmsg */
> >
> > -struct console {
> > - char name[16];
> > +struct console;
> > +
> > +struct console_operations {
> > void (*write)(struct console *, const char *, unsigned);
> > int (*read)(struct console *, char *, unsigned);
> > struct tty_driver *(*device)(struct console *, int *);
> > void (*unblank)(void);
> > int (*setup)(struct console *, char *);
> > int (*match)(struct console *, char *name, int idx, char *options);
> > +};
> > +
> > +struct console {
> > + char name[16];
> > short flags;
> > short index;
> > int cflag;
> > void *data;
> > struct console *next;
> > int level;
> > + const struct console_operations *ops;
> > + struct device dev;
> > + int is_static;
> > };
> >
> > /*
> > @@ -167,6 +176,29 @@ struct console {
> > extern int console_set_on_cmdline;
> > extern struct console *early_console;
> >
> > +extern struct console *allocate_console(const struct console_operations *ops,
> > + const char *name, short flags,
> > + short index, void *data);
> > +
> > +#define allocate_console_dfl(ops, name, data) \
> > + allocate_console(ops, name, CON_PRINTBUFFER, -1, data)
> > +
> > +/*
> > + * Helpers for get/put that do the right thing for static early consoles.
> > + */
> > +
> > +#define get_console(con) \
> > +do { \
> > + if (!con->is_static) \
> > + get_device(&(con)->dev); \
> > +} while (0)
> > +
> > +#define put_console(con) \
> > +do { \
> > + if (con && !con->is_static) \
> > + put_device(&((struct console *)con)->dev); \
> > +} while (0)
> > +
> > extern int add_preferred_console(char *name, int idx, char *options);
> > extern void register_console(struct console *);
> > extern int unregister_console(struct console *);
> > diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> > index 5fe2b037e833..29b43c4df3d6 100644
> > --- a/include/linux/serial_core.h
> > +++ b/include/linux/serial_core.h
> > @@ -426,6 +426,25 @@ int uart_add_one_port(struct uart_driver *reg, struct uart_port *port);
> > int uart_remove_one_port(struct uart_driver *reg, struct uart_port *port);
> > int uart_match_port(struct uart_port *port1, struct uart_port *port2);
> >
> > +/*
> > + * This ugliness removes the need for #ifdef boilerplate in UART drivers which
> > + * allow their console functionality to be disabled via Kconfig.
> > + */
> > +#define uart_allocate_console(drv, ops, name, flags, idx, kcfg) \
> > +({ \
> > + int __retval = 0; \
> > + if (IS_ENABLED(CONFIG_##kcfg)) { \
>
> I wonder if it is worth it. I do not see much existing #ifdef's
> removed by this patch. I would expect that the code is never
> called when the driver is disabled in Kconfig.
>
> IMHO, such a trick hidden in a macro could cause more confusion
> than good.
Yeah... I wasn't really sure about this either, I don't disagree that it's
marginally evil.
>
> > + (drv)->cons = allocate_console(ops, name, flags, idx, drv); \
> > + __retval = (drv)->cons ? 0 : -ENOMEM; \
> > + } \
> > + __retval; \
> > +})
> > +
> > +#define uart_allocate_console_dfl(drv, ops, name, kcfg) \
> > + uart_allocate_console(drv, ops, name, CON_PRINTBUFFER, -1, kcfg)
> > +
> > +#define uart_put_console(drv) put_console((drv)->cons)
> > +
> > /*
> > * Power Management
> > */
> > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> > index 2e0eb89f046c..67e1e993ab80 100644
> > --- a/kernel/printk/printk.c
> > +++ b/kernel/printk/printk.c
> > @@ -108,6 +108,8 @@ enum devkmsg_log_masks {
> >
> > static unsigned int __read_mostly devkmsg_log = DEVKMSG_LOG_MASK_DEFAULT;
> >
> > +static int printk_late_done;
> > +
> > static int __control_devkmsg(char *str)
> > {
> > if (!str)
> > @@ -1731,7 +1733,7 @@ static void call_console_drivers(const char *ext_text, size_t ext_len,
> > continue;
> > if (!(con->flags & CON_ENABLED))
> > continue;
> > - if (!con->write)
> > + if (!con->ops->write)
> > continue;
> > if (!cpu_online(smp_processor_id()) &&
> > !(con->flags & CON_ANYTIME))
> > @@ -1739,9 +1741,9 @@ static void call_console_drivers(const char *ext_text, size_t ext_len,
> > if (suppress_message_printing(level, con))
> > continue;
> > if (con->flags & CON_EXTENDED)
> > - con->write(con, ext_text, ext_len);
> > + con->ops->write(con, ext_text, ext_len);
> > else
> > - con->write(con, text, len);
> > + con->ops->write(con, text, len);
> > }
> > }
> >
> > @@ -2052,7 +2054,7 @@ asmlinkage __visible void early_printk(const char *fmt, ...)
> > n = vscnprintf(buf, sizeof(buf), fmt, ap);
> > va_end(ap);
> >
> > - early_console->write(early_console, buf, n);
> > + early_console->ops->write(early_console, buf, n);
> > }
> > #endif
> >
> > @@ -2481,8 +2483,8 @@ void console_unblank(void)
> > console_locked = 1;
> > console_may_schedule = 0;
> > for_each_console(c)
> > - if ((c->flags & CON_ENABLED) && c->unblank)
> > - c->unblank();
> > + if ((c->flags & CON_ENABLED) && c->ops->unblank)
> > + c->ops->unblank();
> > console_unlock();
> > }
> >
> > @@ -2515,9 +2517,9 @@ struct tty_driver *console_device(int *index)
> >
> > console_lock();
> > for_each_console(c) {
> > - if (!c->device)
> > + if (!c->ops->device)
> > continue;
> > - driver = c->device(c, index);
> > + driver = c->ops->device(c, index);
> > if (driver)
> > break;
> > }
> > @@ -2558,6 +2560,68 @@ static int __init keep_bootcon_setup(char *str)
> >
> > early_param("keep_bootcon", keep_bootcon_setup);
> >
> > +static struct bus_type console_subsys = {
> > + .name = "console",
> > +};
> > +
> > +static void console_release(struct device *dev)
> > +{
> > + struct console *con = container_of(dev, struct console, dev);
> > +
> > + if (WARN(con->is_static, "Freeing static early console!\n"))
> > + return;
> > +
> > + if (WARN(con->flags & CON_ENABLED, "Freeing running console!\n"))
> > + return;
> > +
> > + pr_info("Freeing console %s\n", con->name);
> > + kfree(con);
> > +}
> > +
> > +static void console_init_device(struct console *con)
> > +{
> > + device_initialize(&con->dev);
> > + dev_set_name(&con->dev, "%s", con->name);
> > + con->dev.release = console_release;
> > +}
> > +
> > +static void console_register_device(struct console *new)
> > +{
> > + /*
> > + * We might be called very early from register_console(): in that case,
> > + * printk_late_init() will take care of this later.
> > + */
> > + if (!printk_late_done)
> > + return;
> > +
> > + if (new->is_static)
> > + console_init_device(new);
> > +
> > + new->dev.bus = &console_subsys;
> > + WARN_ON(device_add(&new->dev));
> > +}
> > +
> > +struct console *allocate_console(const struct console_operations *ops,
> > + const char *name, short flags, short index,
> > + void *data)
> > +{
> > + struct console *new;
> > +
> > + new = kzalloc(sizeof(*new), GFP_KERNEL);
>
> I have just realized that page_alloc_init() is called before
> parse_early_param(). Therefore we should be able to use
> memblock_alloc() for early consoles and reduce problems
> with static structures.
Ah nice, I'll look into that. This would all be a lot nicer without
the special cases.
>
> > + if (!new)
> > + return NULL;
> > +
> > + new->ops = ops;
> > + strscpy(new->name, name, sizeof(new->name));
> > + new->flags = flags;
> > + new->index = index;
> > + new->data = data;
> > +
> > + console_init_device(new);
>
> This is a side effect that I would not expect from a function called
> alloc*(). I would rename:
>
> s/allocate_console/create_console/ or
> s/allocate_console/init_console/
Makes sense.
>
> > + return new;
> > +}
> > +EXPORT_SYMBOL_GPL(allocate_console);
> > +
> > /*
> > * The console driver calls this routine during kernel initialization
> > * to register the console printing procedure with printk() and to
> > @@ -2622,10 +2686,10 @@ void register_console(struct console *newcon)
> > if (!has_preferred) {
> > if (newcon->index < 0)
> > newcon->index = 0;
> > - if (newcon->setup == NULL ||
> > - newcon->setup(newcon, NULL) == 0) {
> > + if (newcon->ops->setup == NULL ||
> > + newcon->ops->setup(newcon, NULL) == 0) {
> > newcon->flags |= CON_ENABLED;
> > - if (newcon->device) {
> > + if (newcon->ops->device) {
>
> There might be confusion why we need two devices (con->dev
> and con->ops->device).
>
> I would rename con->ops->device to con->ops->tty_dev.
Makes sense.
>
> > newcon->flags |= CON_CONSDEV;
> > has_preferred = true;
> > }
> > @@ -2639,8 +2703,8 @@ void register_console(struct console *newcon)
> > for (i = 0, c = console_cmdline;
> > i < MAX_CMDLINECONSOLES && c->name[0];
> > i++, c++) {
> > - if (!newcon->match ||
> > - newcon->match(newcon, c->name, c->index, c->options) != 0) {
> > + if (!newcon->ops->match ||
> > + newcon->ops->match(newcon, c->name, c->index, c->options) != 0) {
> > /* default matching */
> > BUILD_BUG_ON(sizeof(c->name) != sizeof(newcon->name));
> > if (strcmp(c->name, newcon->name) != 0)
> > @@ -2660,8 +2724,8 @@ void register_console(struct console *newcon)
> > if (_braille_register_console(newcon, c))
> > return;
> >
> > - if (newcon->setup &&
> > - newcon->setup(newcon, c->options) != 0)
> > + if (newcon->ops->setup &&
> > + newcon->ops->setup(newcon, c->options) != 0)
> > break;
> > }
> >
> > @@ -2706,6 +2770,8 @@ void register_console(struct console *newcon)
> > console_drivers->next = newcon;
> > }
> >
> > + get_console(newcon);
> > +
> > if (newcon->flags & CON_EXTENDED)
> > nr_ext_console_drivers++;
> >
> > @@ -2730,6 +2796,7 @@ void register_console(struct console *newcon)
> > exclusive_console_stop_seq = console_seq;
> > logbuf_unlock_irqrestore(flags);
> > }
> > + console_register_device(newcon);
>
> This calls console_init_device() for the statically defined
> early consoles. I guess that it would invalidate the above
> get_console().
The get_console() macro checks for ->is_static and is a NOP if it's
set, which is definitely confusing.
> Also it is not symmetric with unregister_console(). We add it
> to the console_subsys here and did not remove it when
> the console is unregistered.
We do a put() in the unregister path, somebody could hold a reference
through sysfs so we can't just remove it. Or am I misunderstanding?
> IMHO, this might be done already in allocate_console().
> And eventually in printk_late_init() for consoles that
> are allocated earlier.
>
> I am not familiar with the device-related sysfs API.
> But I see that device_initialize() and device_add()
> can be done by device_register(). The separate
> calls are needed only when a special reference
> handling is needed. Are we special?
We're special: the console initcalls run before the device core is
initialized, initialize() lets us use the refcount.
That said, I'm hopeful your suggestion about using memblock_alloc()
will make some of this confusion unnecessary, it's definitely...
confusing.
> Anyway, please, try to describe the expected workflow
> in the commit description:
>
> + where the structure should get allocated
> + when it is added to the sysfs hierarchy
> + what happens when the console gets registered
> and unregistered
> + when it is removed from the sysfs hierarchy
> + when the structure is released/freed
>
> + What is needed to get the console registered
> and unregistered repeatedly on a running system
>
> Finally, we might want to show state of the CON_ENABLED flag
> in sysfs to distinguish the currently used consoles.
Makes sense.
>
> > console_unlock();
> > console_sysfs_notify();
>
> Thanks a lot for working on this. I know that it is not easy.
> But it is really appreciated.
Thanks :)
--Calvin
> Best Regards,
> Petr
^ permalink raw reply
* Re: [PATCH 1/4] printk: Introduce per-console loglevel setting
From: Calvin Owens @ 2019-03-12 21:00 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Petr Mladek, Sergey Senozhatsky, Steven Rostedt,
Greg Kroah-Hartman, Jonathan Corbet, linux-kernel@vger.kernel.org,
linux-serial@vger.kernel.org
In-Reply-To: <20190308031032.GA30056@jagdpanzerIV>
On Friday 03/08 at 12:10 +0900, Sergey Senozhatsky wrote:
> On (03/01/19 16:48), Calvin Owens wrote:
> [..]
> > msg = log_from_idx(console_idx);
> > - if (suppress_message_printing(msg->level)) {
> > - /*
> > - * Skip record we have buffered and already printed
> > - * directly to the console when we received it, and
> > - * record that has level above the console loglevel.
> > - */
> > - console_idx = log_next(console_idx);
> > - console_seq++;
> > - goto skip;
> > - }
> >
> > /* Output to all consoles once old messages replayed. */
> > if (unlikely(exclusive_console &&
> > @@ -2405,7 +2402,7 @@ void console_unlock(void)
> > console_lock_spinning_enable();
> >
> > stop_critical_timings(); /* don't trace print latency */
> > - call_console_drivers(ext_text, ext_len, text, len);
> > + call_console_drivers(ext_text, ext_len, text, len, msg->level);
> > start_critical_timings();
>
> So it seems that now we always format the text and ext message (if
> needed) and only then check if there is at least one console we can
> print that message on.
>
> Can we iterate the consoles first and check if msg is worth
> the effort (per console suppress_message_printing()) and only
> if it is do all the formatting and call console drivers?
Makes sense, will do.
Thanks,
Calvin
> -ss
^ permalink raw reply
* Re: [PATCH 3/4] printk: Add consoles to a virtual "console" bus
From: Calvin Owens @ 2019-03-12 20:52 UTC (permalink / raw)
To: Petr Mladek
Cc: Sergey Senozhatsky, Steven Rostedt, Greg Kroah-Hartman,
Jonathan Corbet, linux-kernel@vger.kernel.org,
linux-serial@vger.kernel.org
In-Reply-To: <20190308155352.g43mt44adau6cgh6@pathway.suse.cz>
On Friday 03/08 at 16:53 +0100, Petr Mladek wrote:
> On Fri 2019-03-01 16:48:19, Calvin Owens wrote:
> > This patch embeds a device struct in the console struct, and registers
> > them on a "console" bus so we can expose attributes in sysfs.
> >
> > Early console structures must still be static, since they're required
> > before we're able to allocate memory. The least ugly way I can come up
> > with to handle this is an "is_static" flag in the structure which makes
> > the gets and puts NOPs, and is checked in ->release() to catch mistakes.
>
> I wonder if it might get detected by is_kernel_inittext().
I don't think inittext() in particular would work, since these actually need
to exist forever if you pass "earlyprintk=[...],keep" so they aren't __init.
But I bet you're right that we could catch the static case without needing
the explicit flag, something like is_module_address() (but it would also need
to work for the built-in case). I'll see if I can get this to work.
Thanks,
Calvin
> Best Regards,
> Petr
^ permalink raw reply
* Re: [PATCH 3/4] printk: Add consoles to a virtual "console" bus
From: Calvin Owens @ 2019-03-12 20:44 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Petr Mladek, John Ogness, Sergey Senozhatsky, Steven Rostedt,
Jonathan Corbet, linux-kernel@vger.kernel.org,
linux-serial@vger.kernel.org
In-Reply-To: <20190308163411.GA12898@kroah.com>
On Friday 03/08 at 17:34 +0100, Greg Kroah-Hartman wrote:
> On Fri, Mar 08, 2019 at 04:58:14PM +0100, Petr Mladek wrote:
> > On Fri 2019-03-08 03:56:19, John Ogness wrote:
> > > On 2019-03-02, Calvin Owens <calvinowens@fb.com> wrote:
> > > > This patch embeds a device struct in the console struct, and registers
> > > > them on a "console" bus so we can expose attributes in sysfs.
> > >
> > > I expect that "class" would be more appropriate than "bus". These
> > > devices really are grouped together based on their function and not the
> > > medium by which they are accessed.
> >
> > Good point. "class" looks better to me as well.
> >
> > Greg, any opinion, where to put the entries for struct console ?
>
> Hang them off of the device that the console belongs to?
>
> Classes and busses are almost identical except:
> - busses is the binding of a driver to a device (usb, pci, etc.)
> - classes are usually userspace interactions to a device (input,
> tty, etc.)
>
> So this sounds like a class to me.
Sounds good, will make it a class.
> If you want me to review this, I'll be glad to so do once 5.1-rc1 is
> out...
Yeah, I realized after sending this the timing was pretty terrible, I'll
wait for 5.1-rc1 before rebasing/resending.
Thanks,
Calvin
> thanks,
>
> greg k-h
^ permalink raw reply
* Re: [PATCH v2 3/4] dt-bindings: serial: sprd: Add dma properties to support DMA mode
From: Rob Herring @ 2019-03-12 18:22 UTC (permalink / raw)
Cc: gregkh, jslaby, robh+dt, mark.rutland, orsonzhai, zhang.lyra,
baolin.wang, broonie, lanqing.liu, linux-serial, linux-kernel,
devicetree
In-Reply-To: <5ef1bbcf1d78829f28377a599d81d0adc7d61ef1.1551689518.git.baolin.wang@linaro.org>
On Mon, 4 Mar 2019 16:58:23 +0800, Baolin Wang wrote:
> From: Lanqing Liu <lanqing.liu@unisoc.com>
>
> This patch adds dmas and dma-names properties for the UART DMA mode.
>
> Signed-off-by: Lanqing Liu <lanqing.liu@unisoc.com>
> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
> ---
> .../devicetree/bindings/serial/sprd-uart.txt | 6 ++++++
> 1 file changed, 6 insertions(+)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH 1/8] drivers: tty: serial: 8250_bcm2835aux: use devm_platform_ioremap_resource()
From: Greg KH @ 2019-03-12 16:33 UTC (permalink / raw)
To: Enrico Weigelt, metux IT consult; +Cc: jslaby, linux-serial, linux-kernel
In-Reply-To: <1552402660-31730-2-git-send-email-info@metux.net>
On Tue, Mar 12, 2019 at 03:57:33PM +0100, Enrico Weigelt, metux IT consult wrote:
> ---
> drivers/tty/serial/8250/8250_bcm2835aux.c | 12 ++++--------
> 1 file changed, 4 insertions(+), 8 deletions(-)
No changelog or signed-off-by, sorry, please fix up the series and
resend.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v8 1/2] dt-bindings: serial: Add compatible for Mediatek MT8183
From: Greg Kroah-Hartman @ 2019-03-12 16:33 UTC (permalink / raw)
To: Matthias Brugger
Cc: Erin Lo, Rob Herring, Mark Rutland, Thomas Gleixner, Jason Cooper,
Marc Zyngier, Stephen Boyd, devicetree, srv_heupstream,
linux-kernel, linux-serial, linux-mediatek, linux-arm-kernel,
yingjoe.chen, mars.cheng, eddie.huang, linux-clk, zhiyong.tao,
Mengqi.Zhang
In-Reply-To: <d824b15d-49b4-ef6c-779a-0c21c8c05d62@gmail.com>
On Tue, Mar 12, 2019 at 03:41:57PM +0100, Matthias Brugger wrote:
>
>
> On 11/03/2019 09:54, Erin Lo wrote:
> > This adds dt-binding documentation of uart for Mediatek MT8183 SoC
> > Platform.
> >
> > Signed-off-by: Erin Lo <erin.lo@mediatek.com>
> > Acked-by: Rob Herring <robh@kernel.org>
> > ---
> > Documentation/devicetree/bindings/serial/mtk-uart.txt | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/Documentation/devicetree/bindings/serial/mtk-uart.txt b/Documentation/devicetree/bindings/serial/mtk-uart.txt
> > index 742cb47..bcfb131 100644
> > --- a/Documentation/devicetree/bindings/serial/mtk-uart.txt
> > +++ b/Documentation/devicetree/bindings/serial/mtk-uart.txt
> > @@ -16,6 +16,7 @@ Required properties:
> > * "mediatek,mt8127-uart" for MT8127 compatible UARTS
> > * "mediatek,mt8135-uart" for MT8135 compatible UARTS
> > * "mediatek,mt8173-uart" for MT8173 compatible UARTS
> > + * "mediatek,mt8183-uart", "mediatek,mt6577-uart" for MT8183 compatible UARTS
> > * "mediatek,mt6577-uart" for MT6577 and all of the above
> >
> > - reg: The base address of the UART register bank.
> >
>
> Acked-by: Matthias Brugger <matthias.bgg@gmail.com>
>
> Greg will you take this through your branch or do you prefer me to queue it?
I can take it, let's wait for 5.1-rc1 to come out first please.
thanks,
greg k-h
^ permalink raw reply
* Re: Serial console is causing system lock-up
From: John Ogness @ 2019-03-12 15:19 UTC (permalink / raw)
To: Petr Mladek
Cc: Nigel Croxon, Theodore Y. Ts'o, Sergey Senozhatsky,
Greg Kroah-Hartman, Steven Rostedt, Sergey Senozhatsky, dm-devel,
Mikulas Patocka, linux-serial
In-Reply-To: <20190312120824.4eaa4eyjcxvuzm23@pathway.suse.cz>
On 2019-03-12, Petr Mladek <pmladek@suse.com> wrote:
> On Tue 2019-03-12 09:17:49, John Ogness wrote:
>> The current printk implementation is handling all console printing as
>> best effort. Trying hard enough to dramatically affect the system,
>> but not trying hard enough to guarantee success.
>
> I agree that direct output is more reliable. It might be very useful
> for debugging some types of problems. The question is if it is worth
> the cost (code complexity, serializing CPUs == slowing down the
> entire system).
>
> But it is is possible that a reasonable offloading (in the direction
> of last Sergey's approach) might be a better deal.
>
>
> I suggest the following way forward (separate patchsets):
>
> 1. Replace log buffer (least controversial thing)
> 2. Reliable offload to kthread (would be useful anyway)
> 3. Atomic consoles (a lot of tricky code, might not be
> worth the effort)
>
> Could we agree on this?
Since this is about the new RFC printk design, I've responded in that
thread [0].
John Ogness
[0] https://lkml.kernel.org/r/874l8815uc.fsf@linutronix.de
^ permalink raw reply
* Re: [RFC PATCH v1 00/25] printk: new implementation
From: John Ogness @ 2019-03-12 15:15 UTC (permalink / raw)
To: Petr Mladek
Cc: Sergey Senozhatsky, linux-kernel, Peter Zijlstra, Steven Rostedt,
Daniel Wang, Andrew Morton, Linus Torvalds, Greg Kroah-Hartman,
Alan Cox, Jiri Slaby, Peter Feiner, linux-serial,
Sergey Senozhatsky, Sebastian Siewior
In-Reply-To: <20190312123857.juatd6fwtfmqajze@pathway.suse.cz>
On 2019-03-12, Petr Mladek <pmladek@suse.com> wrote:
> Per-console kthread sounds interesting but there is the problem with
> reliability. I mean that kthread need not get scheduled.
>
> Some of these problems might get solved by the per-console loglevel
> patchset.
>
> Sigh, any feature might be useful in some situation. But we always
> have to consider the cost and the gain. I wonder how common is
> to actively use two consoles at the same time and what would
> be the motivation.
The following is from the linux-serial mailing list:
"Re: Serial console is causing system lock-up" [0]
I'm responding to it here because it really belongs in this thread.
On 2019-03-12, Petr Mladek <pmladek@suse.com> wrote:
> On Tue 2019-03-12 09:17:49, John Ogness wrote:
>> The current printk implementation is handling all console printing as
>> best effort. Trying hard enough to dramatically affect the system, but
>> not trying hard enough to guarantee success.
>
> I agree that direct output is more reliable. It might be very useful
> for debugging some types of problems. The question is if it is worth
> the cost (code complexity, serializing CPUs == slowing down the
> entire system).
>
> But it is is possible that a reasonable offloading (in the direction
> of last Sergey's approach) might be a better deal.
>
>
> I suggest the following way forward (separate patchsets):
>
> 1. Replace log buffer (least controversial thing)
Yes. I will post a series that only implements the ringbuffer using your
simplified API. That will be enough to remove printk_safe and actually
does most of the work of updating devkmsg, kmsg_dump, and syslog.
> 2. Reliable offload to kthread (would be useful anyway)
Yes. I would like to implement per-console kthreads for this series. I
think the advantages are obvious. For PREEMPT_RT the offloading will
need to be always active. (PREEMPT_RT cannot call the console->write()
from atomic contexts.) But I think this would be acceptable at first. It
would certainly be better than what PREEMPT_RT is doing now.
> 3. Atomic consoles (a lot of tricky code, might not be
> worth the effort)
I think this will be necessary. PREEMPT_RT cannot support reliable
emergency console messages without it. And for kernel developers this is
also very helpful. People like PeterZ are using their own patches
because the mainline kernel is not providing this functionality.
The decision about _when_ to use it is still in the air. But I guess
we'll worry about that when we get that far. There's enough to do until
then.
> Could we agree on this?
I think this is a sane path forward. Thank you for providing some
direction here.
John Ogness
[0] https://marc.info/?l=linux-serial&m=155239250721543
^ permalink raw reply
* [PATCH 8/8] drivers: tty: serial: xilinx_uartps: use helpers
From: Enrico Weigelt, metux IT consult @ 2019-03-12 14:57 UTC (permalink / raw)
To: gregkh, jslaby, linux-serial, linux-kernel
In-Reply-To: <1552402660-31730-1-git-send-email-info@metux.net>
---
drivers/tty/serial/xilinx_uartps.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 74089f5..92aff38 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -953,15 +953,14 @@ static int cdns_uart_verify_port(struct uart_port *port,
*/
static int cdns_uart_request_port(struct uart_port *port)
{
- if (!request_mem_region(port->mapbase, CDNS_UART_REGISTER_SPACE,
- CDNS_UART_NAME)) {
+ if (!uart_memres_request(port, CDNS_UART_NAME)) {
return -ENOMEM;
}
- port->membase = ioremap(port->mapbase, CDNS_UART_REGISTER_SPACE);
+ port->membase = uart_memres_ioremap(port);
if (!port->membase) {
dev_err(port->dev, "Unable to map registers\n");
- release_mem_region(port->mapbase, CDNS_UART_REGISTER_SPACE);
+ uart_memres_release(port);
return -ENOMEM;
}
return 0;
@@ -976,7 +975,7 @@ static int cdns_uart_request_port(struct uart_port *port)
*/
static void cdns_uart_release_port(struct uart_port *port)
{
- release_mem_region(port->mapbase, CDNS_UART_REGISTER_SPACE);
+ uart_memres_release(port);
iounmap(port->membase);
port->membase = NULL;
}
@@ -1626,7 +1625,7 @@ static int cdns_uart_probe(struct platform_device *pdev)
* This function also registers this device with the tty layer
* and triggers invocation of the config_port() entry point.
*/
- port->mapbase = res->start;
+ uart_memres_set(*res);
port->irq = irq;
port->dev = &pdev->dev;
port->uartclk = clk_get_rate(cdns_uart_data->uartclk);
@@ -1707,7 +1706,7 @@ static int cdns_uart_remove(struct platform_device *pdev)
&cdns_uart_data->clk_rate_change_nb);
#endif
rc = uart_remove_one_port(cdns_uart_data->cdns_uart_driver, port);
- port->mapbase = 0;
+ uart_memres_clear(port);
mutex_lock(&bitmap_lock);
if (cdns_uart_data->id < MAX_UART_INSTANCES)
clear_bit(cdns_uart_data->id, bitmap);
--
1.9.1
^ permalink raw reply related
* [PATCH 7/8] drivers: tty: serial: use memres
From: Enrico Weigelt, metux IT consult @ 2019-03-12 14:57 UTC (permalink / raw)
To: gregkh, jslaby, linux-serial, linux-kernel
In-Reply-To: <1552402660-31730-1-git-send-email-info@metux.net>
---
drivers/tty/serial/zs.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/tty/serial/zs.c b/drivers/tty/serial/zs.c
index b03d3e4..2fd4821 100644
--- a/drivers/tty/serial/zs.c
+++ b/drivers/tty/serial/zs.c
@@ -986,14 +986,13 @@ static void zs_release_port(struct uart_port *uport)
{
iounmap(uport->membase);
uport->membase = 0;
- release_mem_region(uport->mapbase, ZS_CHAN_IO_SIZE);
+ uart_memres_release(uport);
}
static int zs_map_port(struct uart_port *uport)
{
if (!uport->membase)
- uport->membase = ioremap_nocache(uport->mapbase,
- ZS_CHAN_IO_SIZE);
+ uport->membase = uart_memres_ioremap_nocache(uport);
if (!uport->membase) {
printk(KERN_ERR "zs: Cannot map MMIO\n");
return -ENOMEM;
@@ -1005,13 +1004,13 @@ static int zs_request_port(struct uart_port *uport)
{
int ret;
- if (!request_mem_region(uport->mapbase, ZS_CHAN_IO_SIZE, "scc")) {
+ if (!uart_memres_request(uport, "scc")) {
printk(KERN_ERR "zs: Unable to reserve MMIO resource\n");
return -EBUSY;
}
ret = zs_map_port(uport);
if (ret) {
- release_mem_region(uport->mapbase, ZS_CHAN_IO_SIZE);
+ uart_release_memres(uport);
return ret;
}
return 0;
@@ -1113,9 +1112,12 @@ static int __init zs_probe_sccs(void)
uport->flags = UPF_BOOT_AUTOCONF;
uport->ops = &zs_ops;
uport->line = chip * ZS_NUM_CHAN + side;
- uport->mapbase = dec_kn_slot_base +
- zs_parms.scc[chip] +
- (side ^ ZS_CHAN_B) * ZS_CHAN_IO_SIZE;
+ uart_memres_set(
+ DEFINE_RES_MEM(
+ dec_kn_slot_base +
+ zs_parms.scc[chip] +
+ (side ^ ZS_CHAN_B) * ZS_CHAN_IO_SIZE,
+ ZS_CHAN_IO_SIZE));
for (i = 0; i < ZS_NUM_REGS; i++)
zport->regs[i] = zs_init_regs[i];
--
1.9.1
^ permalink raw reply related
* [PATCH 6/8] drivers: tty: serial: vt8500: use memres
From: Enrico Weigelt, metux IT consult @ 2019-03-12 14:57 UTC (permalink / raw)
To: gregkh, jslaby, linux-serial, linux-kernel
In-Reply-To: <1552402660-31730-1-git-send-email-info@metux.net>
---
drivers/tty/serial/vt8500_serial.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/vt8500_serial.c b/drivers/tty/serial/vt8500_serial.c
index 3d58e9b..331a9dd 100644
--- a/drivers/tty/serial/vt8500_serial.c
+++ b/drivers/tty/serial/vt8500_serial.c
@@ -696,13 +696,13 @@ static int vt8500_serial_probe(struct platform_device *pdev)
);
vt8500_port->uart.type = PORT_VT8500;
vt8500_port->uart.iotype = UPIO_MEM;
- vt8500_port->uart.mapbase = mmres->start;
vt8500_port->uart.irq = irqres->start;
vt8500_port->uart.fifosize = 16;
vt8500_port->uart.ops = &vt8500_uart_pops;
vt8500_port->uart.line = port;
vt8500_port->uart.dev = &pdev->dev;
vt8500_port->uart.flags = UPF_IOREMAP | UPF_BOOT_AUTOCONF;
+ uart_memres_set(&vt8500_port->uart, *mmres);
/* Serial core uses the magic "16" everywhere - adjust for it */
vt8500_port->uart.uartclk = 16 * clk_get_rate(vt8500_port->clk) /
--
1.9.1
^ permalink raw reply related
* [PATCH 5/8] drivers: tty: serial: introduce struct resource
From: Enrico Weigelt, metux IT consult @ 2019-03-12 14:57 UTC (permalink / raw)
To: gregkh, jslaby, linux-serial, linux-kernel
In-Reply-To: <1552402660-31730-1-git-send-email-info@metux.net>
The standard data structure for holding io resources in the
kernel is struct resource. Serial drivers yet don't really
use it (except when retrieving from oftree).
This patch introduces a new field in struct uart_port for that,
plus several helpers. Yet it's up to the individual drivers for
using it - serial_core doesn't care yet. Therefore, the old fields
mapbase and mapsize need to be filled properly by the drivers.
---
include/linux/serial_core.h | 49 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 49 insertions(+)
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 5fe2b03..9b07a40 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -24,6 +24,7 @@
#include <linux/compiler.h>
#include <linux/console.h>
#include <linux/interrupt.h>
+#include <linux/ioport.h>
#include <linux/circ_buf.h>
#include <linux/spinlock.h>
#include <linux/sched.h>
@@ -256,6 +257,7 @@ struct uart_port {
unsigned int minor;
resource_size_t mapbase; /* for ioremap */
resource_size_t mapsize;
+ struct resource memres;
struct device *dev; /* parent device */
unsigned char hub6; /* this should be in the 8250 driver */
unsigned char suspended;
@@ -427,6 +429,53 @@ void uart_console_write(struct uart_port *port, const char *s,
int uart_match_port(struct uart_port *port1, struct uart_port *port2);
/*
+ * Port resource management
+ */
+static inline void uart_memres_set(struct uart_port *port,
+ struct resource res)
+{
+ port->memres = res;
+ port->mapbase = res.start;
+}
+
+static inline void uart_memres_clear(struct uart_port *port)
+{
+ port->memres = DEFINE_RES_MEM(0, 0);
+}
+
+static inline int uart_memres_valid(struct uart_port *port)
+{
+ return (port->memres.start != 0);
+}
+
+static inline struct resource *uart_memres_request(struct uart_port *port,
+ const char *name)
+{
+ return request_mem_region(
+ port->memres.start,
+ resource_size(&port->memres),
+ name);
+}
+
+static inline void uart_memres_release(struct uart_port *port)
+{
+ return release_mem_region(port->memres.start,
+ resource_size(&port->memres));
+}
+
+static inline void __iomem *uart_memres_ioremap_nocache(struct uart_port *port)
+{
+ return ioremap_nocache(port->memres.start,
+ resource_size(&port->memres.start));
+}
+
+static inline void __iomem *uart_memres_ioremap(struct uart_port *port)
+{
+ return ioremap(port->memres.start,
+ resource_size(&port->memres.start));
+}
+
+/*
* Power Management
*/
int uart_suspend_port(struct uart_driver *reg, struct uart_port *port);
--
1.9.1
^ permalink raw reply related
* [PATCH 4/8] drivers: tty: serial: use devm_ioremap_resource()
From: Enrico Weigelt, metux IT consult @ 2019-03-12 14:57 UTC (permalink / raw)
To: gregkh, jslaby, linux-serial, linux-kernel
In-Reply-To: <1552402660-31730-1-git-send-email-info@metux.net>
instead of fetching out start and len from a struct resource for
passing it to devm_ioremap(), directly use devm_ioremap_resource()
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
drivers/tty/serial/8250/8250_lpc18xx.c | 3 +--
drivers/tty/serial/8250/8250_mtk.c | 3 +--
drivers/tty/serial/8250/8250_uniphier.c | 2 +-
drivers/tty/serial/amba-pl010.c | 3 +--
drivers/tty/serial/samsung.c | 2 +-
drivers/tty/serial/sirfsoc_uart.c | 3 +--
6 files changed, 6 insertions(+), 10 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_lpc18xx.c b/drivers/tty/serial/8250/8250_lpc18xx.c
index eddf119..f36902b 100644
--- a/drivers/tty/serial/8250/8250_lpc18xx.c
+++ b/drivers/tty/serial/8250/8250_lpc18xx.c
@@ -119,8 +119,7 @@ static int lpc18xx_serial_probe(struct platform_device *pdev)
memset(&uart, 0, sizeof(uart));
- uart.port.membase = devm_ioremap(&pdev->dev, res->start,
- resource_size(res));
+ uart.port.membase = devm_ioremap_resource(&pdev->dev, res);
if (!uart.port.membase)
return -ENOMEM;
diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
index c1fdbc0..bf6eb8d 100644
--- a/drivers/tty/serial/8250/8250_mtk.c
+++ b/drivers/tty/serial/8250/8250_mtk.c
@@ -383,8 +383,7 @@ static int mtk8250_probe(struct platform_device *pdev)
return -EINVAL;
}
- uart.port.membase = devm_ioremap(&pdev->dev, regs->start,
- resource_size(regs));
+ uart.port.membase = devm_ioremap_resource(&pdev->dev, regs);
if (!uart.port.membase)
return -ENOMEM;
diff --git a/drivers/tty/serial/8250/8250_uniphier.c b/drivers/tty/serial/8250/8250_uniphier.c
index 164ba13..9c1244e 100644
--- a/drivers/tty/serial/8250/8250_uniphier.c
+++ b/drivers/tty/serial/8250/8250_uniphier.c
@@ -171,7 +171,7 @@ static int uniphier_uart_probe(struct platform_device *pdev)
return -EINVAL;
}
- membase = devm_ioremap(dev, regs->start, resource_size(regs));
+ membase = devm_ioremap_resource(dev, regs);
if (!membase)
return -ENOMEM;
diff --git a/drivers/tty/serial/amba-pl010.c b/drivers/tty/serial/amba-pl010.c
index 2c37d11..109b8df 100644
--- a/drivers/tty/serial/amba-pl010.c
+++ b/drivers/tty/serial/amba-pl010.c
@@ -713,8 +713,7 @@ static int pl010_probe(struct amba_device *dev, const struct amba_id *id)
if (!uap)
return -ENOMEM;
- base = devm_ioremap(&dev->dev, dev->res.start,
- resource_size(&dev->res));
+ base = devm_ioremap_resource(&dev->dev, &dev->res);
if (!base)
return -ENOMEM;
diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
index 83fd516..a49cf15 100644
--- a/drivers/tty/serial/samsung.c
+++ b/drivers/tty/serial/samsung.c
@@ -1775,7 +1775,7 @@ static int s3c24xx_serial_init_port(struct s3c24xx_uart_port *ourport,
dbg("resource %pR)\n", res);
- port->membase = devm_ioremap(port->dev, res->start, resource_size(res));
+ port->membase = devm_ioremap_resource(port->dev, res);
if (!port->membase) {
dev_err(port->dev, "failed to remap controller address\n");
return -EBUSY;
diff --git a/drivers/tty/serial/sirfsoc_uart.c b/drivers/tty/serial/sirfsoc_uart.c
index 38622f2..db2e08d 100644
--- a/drivers/tty/serial/sirfsoc_uart.c
+++ b/drivers/tty/serial/sirfsoc_uart.c
@@ -1359,8 +1359,7 @@ static int sirfsoc_uart_probe(struct platform_device *pdev)
goto err;
}
port->mapbase = res->start;
- port->membase = devm_ioremap(&pdev->dev,
- res->start, resource_size(res));
+ port->membase = devm_ioremap_resource(&pdev->dev, res);
if (!port->membase) {
dev_err(&pdev->dev, "Cannot remap resource.\n");
ret = -ENOMEM;
--
1.9.1
^ permalink raw reply related
* [PATCH 3/8] drivers: tty: serial: 8250_ingenic: use devm_ioremap_resource()
From: Enrico Weigelt, metux IT consult @ 2019-03-12 14:57 UTC (permalink / raw)
To: gregkh, jslaby, linux-serial, linux-kernel
In-Reply-To: <1552402660-31730-1-git-send-email-info@metux.net>
Use helper devm_ioremap_resource() to make the code a little bit
shorter and easier to read.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
drivers/tty/serial/8250/8250_ingenic.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_ingenic.c b/drivers/tty/serial/8250/8250_ingenic.c
index 424c07c..90ae3e2 100644
--- a/drivers/tty/serial/8250/8250_ingenic.c
+++ b/drivers/tty/serial/8250/8250_ingenic.c
@@ -249,8 +249,7 @@ static int ingenic_uart_probe(struct platform_device *pdev)
if (line >= 0)
uart.port.line = line;
- uart.port.membase = devm_ioremap(&pdev->dev, regs->start,
- resource_size(regs));
+ uart.port.membase = devm_ioremap_resource(&pdev->dev, regs);
if (!uart.port.membase)
return -ENOMEM;
--
1.9.1
^ permalink raw reply related
* [PATCH 2/8] drivers: tty: serial: 8250_dw: use devm_ioremap_resource()
From: Enrico Weigelt, metux IT consult @ 2019-03-12 14:57 UTC (permalink / raw)
To: gregkh, jslaby, linux-serial, linux-kernel
In-Reply-To: <1552402660-31730-1-git-send-email-info@metux.net>
Use helper devm_ioremap_resource() to make the code a little bit
shorter and easier to read.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
drivers/tty/serial/8250/8250_dw.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index d31b975..f0a294d 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -526,7 +526,7 @@ static int dw8250_probe(struct platform_device *pdev)
p->set_ldisc = dw8250_set_ldisc;
p->set_termios = dw8250_set_termios;
- p->membase = devm_ioremap(dev, regs->start, resource_size(regs));
+ p->membase = devm_ioremap_resource(dev, regs);
if (!p->membase)
return -ENOMEM;
--
1.9.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox