With Arnd's script [1] help, I found some bugfixes in Spreadtrum 4.14 product kernel, but missing in v4.14.141:
25a09ce79639 ppp: mppe: Revert "ppp: mppe: Add softdep to arc4" 47d3d7fdb10a ip6: fix skb leak in ip6frag_expire_frag_queue() 5b9cea15a3de serial: sprd: Modify the baud rate calculation formula 513e1073d52e locking/lockdep: Add debug_locks check in __lock_downgrade() 957063c92473 pinctrl: sprd: Use define directive for sprd_pinconf_params values 87a2b65fc855 power: supply: sysfs: ratelimit property read error message
[1] https://lore.kernel.org/lkml/20190322154425.3852517-19-arnd@arndb.de/T/
Changes from v1: - Drop 2 unnecessary patches (patch 1 and patch 4) from v1 patch set. - Add upstream commit id in change log for each stable patch.
David Lechner (1): power: supply: sysfs: ratelimit property read error message
Eric Biggers (1): ppp: mppe: Revert "ppp: mppe: Add softdep to arc4"
Eric Dumazet (1): ip6: fix skb leak in ip6frag_expire_frag_queue()
Lanqing Liu (1): serial: sprd: Modify the baud rate calculation formula
Nathan Chancellor (1): pinctrl: sprd: Use define directive for sprd_pinconf_params values
Waiman Long (1): locking/lockdep: Add debug_locks check in __lock_downgrade()
drivers/net/ppp/ppp_mppe.c | 1 - drivers/pinctrl/sprd/pinctrl-sprd.c | 6 ++---- drivers/power/supply/power_supply_sysfs.c | 3 ++- drivers/tty/serial/sprd_serial.c | 2 +- include/net/ipv6_frag.h | 1 - kernel/locking/lockdep.c | 3 +++ 6 files changed, 8 insertions(+), 8 deletions(-)
From: Eric Dumazet edumazet@google.com
[Upstream commit 47d3d7fdb10a21c223036b58bd70ffdc24a472c4]
Since ip6frag_expire_frag_queue() now pulls the head skb from frag queue, we should no longer use skb_get(), since this leads to an skb leak.
Stefan Bader initially reported a problem in 4.4.stable [1] caused by the skb_get(), so this patch should also fix this issue.
296583.091021] kernel BUG at /build/linux-6VmqmP/linux-4.4.0/net/core/skbuff.c:1207! [296583.091734] Call Trace: [296583.091749] [<ffffffff81740e50>] __pskb_pull_tail+0x50/0x350 [296583.091764] [<ffffffff8183939a>] _decode_session6+0x26a/0x400 [296583.091779] [<ffffffff817ec719>] __xfrm_decode_session+0x39/0x50 [296583.091795] [<ffffffff818239d0>] icmpv6_route_lookup+0xf0/0x1c0 [296583.091809] [<ffffffff81824421>] icmp6_send+0x5e1/0x940 [296583.091823] [<ffffffff81753238>] ? __netif_receive_skb+0x18/0x60 [296583.091838] [<ffffffff817532b2>] ? netif_receive_skb_internal+0x32/0xa0 [296583.091858] [<ffffffffc0199f74>] ? ixgbe_clean_rx_irq+0x594/0xac0 [ixgbe] [296583.091876] [<ffffffffc04eb260>] ? nf_ct_net_exit+0x50/0x50 [nf_defrag_ipv6] [296583.091893] [<ffffffff8183d431>] icmpv6_send+0x21/0x30 [296583.091906] [<ffffffff8182b500>] ip6_expire_frag_queue+0xe0/0x120 [296583.091921] [<ffffffffc04eb27f>] nf_ct_frag6_expire+0x1f/0x30 [nf_defrag_ipv6] [296583.091938] [<ffffffff810f3b57>] call_timer_fn+0x37/0x140 [296583.091951] [<ffffffffc04eb260>] ? nf_ct_net_exit+0x50/0x50 [nf_defrag_ipv6] [296583.091968] [<ffffffff810f5464>] run_timer_softirq+0x234/0x330 [296583.091982] [<ffffffff8108a339>] __do_softirq+0x109/0x2b0
Fixes: d4289fcc9b16 ("net: IP6 defrag: use rbtrees for IPv6 defrag") Signed-off-by: Eric Dumazet edumazet@google.com Reported-by: Stefan Bader stefan.bader@canonical.com Cc: Peter Oskolkov posk@google.com Cc: Florian Westphal fw@strlen.de Signed-off-by: David S. Miller davem@davemloft.net Signed-off-by: Baolin Wang baolin.wang@linaro.org --- include/net/ipv6_frag.h | 1 - 1 file changed, 1 deletion(-)
diff --git a/include/net/ipv6_frag.h b/include/net/ipv6_frag.h index 28aa9b3..1f77fb4 100644 --- a/include/net/ipv6_frag.h +++ b/include/net/ipv6_frag.h @@ -94,7 +94,6 @@ static inline u32 ip6frag_obj_hashfn(const void *data, u32 len, u32 seed) goto out;
head->dev = dev; - skb_get(head); spin_unlock(&fq->q.lock);
icmpv6_send(head, ICMPV6_TIME_EXCEED, ICMPV6_EXC_FRAGTIME, 0);
From: Waiman Long longman@redhat.com
[Upstream commit 513e1073d52e55b8024b4f238a48de7587c64ccf]
Tetsuo Handa had reported he saw an incorrect "downgrading a read lock" warning right after a previous lockdep warning. It is likely that the previous warning turned off lock debugging causing the lockdep to have inconsistency states leading to the lock downgrade warning.
Fix that by add a check for debug_locks at the beginning of __lock_downgrade().
Reported-by: Tetsuo Handa penguin-kernel@i-love.sakura.ne.jp Reported-by: syzbot+53383ae265fb161ef488@syzkaller.appspotmail.com Signed-off-by: Waiman Long longman@redhat.com Signed-off-by: Peter Zijlstra (Intel) peterz@infradead.org Cc: Andrew Morton akpm@linux-foundation.org Cc: Linus Torvalds torvalds@linux-foundation.org Cc: Paul E. McKenney paulmck@linux.vnet.ibm.com Cc: Peter Zijlstra peterz@infradead.org Cc: Thomas Gleixner tglx@linutronix.de Cc: Will Deacon will.deacon@arm.com Link: https://lkml.kernel.org/r/1547093005-26085-1-git-send-email-longman@redhat.c... Signed-off-by: Ingo Molnar mingo@kernel.org Signed-off-by: Baolin Wang baolin.wang@linaro.org --- kernel/locking/lockdep.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c index 565005a..5c370c6 100644 --- a/kernel/locking/lockdep.c +++ b/kernel/locking/lockdep.c @@ -3650,6 +3650,9 @@ static int reacquire_held_locks(struct task_struct *curr, unsigned int depth, unsigned int depth; int i;
+ if (unlikely(!debug_locks)) + return 0; + depth = curr->lockdep_depth; /* * This function is about (re)setting the class of a held lock,
On Thu, Sep 05, 2019 at 11:07:14AM +0800, Baolin Wang wrote:
From: Waiman Long longman@redhat.com
[Upstream commit 513e1073d52e55b8024b4f238a48de7587c64ccf]
Tetsuo Handa had reported he saw an incorrect "downgrading a read lock" warning right after a previous lockdep warning. It is likely that the previous warning turned off lock debugging causing the lockdep to have inconsistency states leading to the lock downgrade warning.
Fix that by add a check for debug_locks at the beginning of __lock_downgrade().
Reported-by: Tetsuo Handa penguin-kernel@i-love.sakura.ne.jp Reported-by: syzbot+53383ae265fb161ef488@syzkaller.appspotmail.com Signed-off-by: Waiman Long longman@redhat.com Signed-off-by: Peter Zijlstra (Intel) peterz@infradead.org Cc: Andrew Morton akpm@linux-foundation.org Cc: Linus Torvalds torvalds@linux-foundation.org Cc: Paul E. McKenney paulmck@linux.vnet.ibm.com Cc: Peter Zijlstra peterz@infradead.org Cc: Thomas Gleixner tglx@linutronix.de Cc: Will Deacon will.deacon@arm.com Link: https://lkml.kernel.org/r/1547093005-26085-1-git-send-email-longman@redhat.c... Signed-off-by: Ingo Molnar mingo@kernel.org Signed-off-by: Baolin Wang baolin.wang@linaro.org
kernel/locking/lockdep.c | 3 +++ 1 file changed, 3 insertions(+)
Why isn't this relevant for 4.19.y? I can't add a patch to 4.14.y and then have someone upgrade to 4.19.y and not have the same fix in there, that would be a regression.
So can you redo this series also with a 4.19.y set at the same so we don't get out of sync? I've queued up your first patch already as that was in 4.19.y (and also needed in 4.9.y).
thanks,
greg k-h
On Tue, 10 Sep 2019 at 22:32, Greg KH greg@kroah.com wrote:
On Thu, Sep 05, 2019 at 11:07:14AM +0800, Baolin Wang wrote:
From: Waiman Long longman@redhat.com
[Upstream commit 513e1073d52e55b8024b4f238a48de7587c64ccf]
Tetsuo Handa had reported he saw an incorrect "downgrading a read lock" warning right after a previous lockdep warning. It is likely that the previous warning turned off lock debugging causing the lockdep to have inconsistency states leading to the lock downgrade warning.
Fix that by add a check for debug_locks at the beginning of __lock_downgrade().
Reported-by: Tetsuo Handa penguin-kernel@i-love.sakura.ne.jp Reported-by: syzbot+53383ae265fb161ef488@syzkaller.appspotmail.com Signed-off-by: Waiman Long longman@redhat.com Signed-off-by: Peter Zijlstra (Intel) peterz@infradead.org Cc: Andrew Morton akpm@linux-foundation.org Cc: Linus Torvalds torvalds@linux-foundation.org Cc: Paul E. McKenney paulmck@linux.vnet.ibm.com Cc: Peter Zijlstra peterz@infradead.org Cc: Thomas Gleixner tglx@linutronix.de Cc: Will Deacon will.deacon@arm.com Link: https://lkml.kernel.org/r/1547093005-26085-1-git-send-email-longman@redhat.c... Signed-off-by: Ingo Molnar mingo@kernel.org Signed-off-by: Baolin Wang baolin.wang@linaro.org
kernel/locking/lockdep.c | 3 +++ 1 file changed, 3 insertions(+)
Why isn't this relevant for 4.19.y? I can't add a patch to 4.14.y and then have someone upgrade to 4.19.y and not have the same fix in there, that would be a regression.
So can you redo this series also with a 4.19.y set at the same so we don't get out of sync? I've queued up your first patch already as that was in 4.19.y (and also needed in 4.9.y).
I understood, will do. Thanks.
From: Nathan Chancellor natechancellor@gmail.com
[Upstream commit 957063c924736d4341e5d588757b9f31e8f6fa24]
Clang warns when one enumerated type is implicitly converted to another:
drivers/pinctrl/sprd/pinctrl-sprd.c:845:19: warning: implicit conversion from enumeration type 'enum sprd_pinconf_params' to different enumeration type 'enum pin_config_param' [-Wenum-conversion] {"sprd,control", SPRD_PIN_CONFIG_CONTROL, 0}, ~ ^~~~~~~~~~~~~~~~~~~~~~~ drivers/pinctrl/sprd/pinctrl-sprd.c:846:22: warning: implicit conversion from enumeration type 'enum sprd_pinconf_params' to different enumeration type 'enum pin_config_param' [-Wenum-conversion] {"sprd,sleep-mode", SPRD_PIN_CONFIG_SLEEP_MODE, 0}, ~ ^~~~~~~~~~~~~~~~~~~~~~~~~~
It is expected that pinctrl drivers can extend pin_config_param because of the gap between PIN_CONFIG_END and PIN_CONFIG_MAX so this conversion isn't an issue. Most drivers that take advantage of this define the PIN_CONFIG variables as constants, rather than enumerated values. Do the same thing here so that Clang no longer warns.
Link: https://github.com/ClangBuiltLinux/linux/issues/138 Signed-off-by: Nathan Chancellor natechancellor@gmail.com Reviewed-by: Baolin Wang baolin.wang@linaro.org Signed-off-by: Linus Walleij linus.walleij@linaro.org Signed-off-by: Baolin Wang baolin.wang@linaro.org --- drivers/pinctrl/sprd/pinctrl-sprd.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/pinctrl/sprd/pinctrl-sprd.c b/drivers/pinctrl/sprd/pinctrl-sprd.c index 6352991..83958bd 100644 --- a/drivers/pinctrl/sprd/pinctrl-sprd.c +++ b/drivers/pinctrl/sprd/pinctrl-sprd.c @@ -159,10 +159,8 @@ struct sprd_pinctrl { struct sprd_pinctrl_soc_info *info; };
-enum sprd_pinconf_params { - SPRD_PIN_CONFIG_CONTROL = PIN_CONFIG_END + 1, - SPRD_PIN_CONFIG_SLEEP_MODE = PIN_CONFIG_END + 2, -}; +#define SPRD_PIN_CONFIG_CONTROL (PIN_CONFIG_END + 1) +#define SPRD_PIN_CONFIG_SLEEP_MODE (PIN_CONFIG_END + 2)
static int sprd_pinctrl_get_id_by_name(struct sprd_pinctrl *sprd_pctl, const char *name)
From: David Lechner david@lechnology.com
[Upstream commit 87a2b65fc855e6be50f791c2ebbb492541896827]
This adds rate limiting to the message that is printed when reading a power supply property via sysfs returns an error. This will prevent userspace applications from unintentionally dDOSing the system by continuously reading a property that returns an error.
Signed-off-by: David Lechner david@lechnology.com Signed-off-by: Sebastian Reichel sebastian.reichel@collabora.com Signed-off-by: Baolin Wang baolin.wang@linaro.org --- drivers/power/supply/power_supply_sysfs.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c index eb5dc74..2ccaf4f 100644 --- a/drivers/power/supply/power_supply_sysfs.c +++ b/drivers/power/supply/power_supply_sysfs.c @@ -91,7 +91,8 @@ static ssize_t power_supply_show_property(struct device *dev, dev_dbg(dev, "driver has no data for `%s' property\n", attr->attr.name); else if (ret != -ENODEV && ret != -EAGAIN) - dev_err(dev, "driver failed to report `%s' property: %zd\n", + dev_err_ratelimited(dev, + "driver failed to report `%s' property: %zd\n", attr->attr.name, ret); return ret; }
From: Eric Biggers ebiggers@google.com
[Upstream commit 25a09ce79639a8775244808c17282c491cff89cf]
Commit 0e5a610b5ca5 ("ppp: mppe: switch to RC4 library interface"), which was merged through the crypto tree for v5.3, changed ppp_mppe.c to use the new arc4_crypt() library function rather than access RC4 through the dynamic crypto_skcipher API.
Meanwhile commit aad1dcc4f011 ("ppp: mppe: Add softdep to arc4") was merged through the net tree and added a module soft-dependency on "arc4".
The latter commit no longer makes sense because the code now uses the "libarc4" module rather than "arc4", and also due to the direct use of arc4_crypt(), no module soft-dependency is required.
So revert the latter commit.
Cc: Takashi Iwai tiwai@suse.de Cc: Ard Biesheuvel ard.biesheuvel@linaro.org Signed-off-by: Eric Biggers ebiggers@google.com Signed-off-by: David S. Miller davem@davemloft.net Signed-off-by: Baolin Wang baolin.wang@linaro.org --- drivers/net/ppp/ppp_mppe.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/drivers/net/ppp/ppp_mppe.c b/drivers/net/ppp/ppp_mppe.c index d9eda7c..6c7fd98 100644 --- a/drivers/net/ppp/ppp_mppe.c +++ b/drivers/net/ppp/ppp_mppe.c @@ -63,7 +63,6 @@ MODULE_DESCRIPTION("Point-to-Point Protocol Microsoft Point-to-Point Encryption support"); MODULE_LICENSE("Dual BSD/GPL"); MODULE_ALIAS("ppp-compress-" __stringify(CI_MPPE)); -MODULE_SOFTDEP("pre: arc4"); MODULE_VERSION("1.0.2");
static unsigned int
On Thu, Sep 05, 2019 at 11:10:45AM +0800, Baolin Wang wrote:
From: Eric Biggers ebiggers@google.com
[Upstream commit 25a09ce79639a8775244808c17282c491cff89cf]
Commit 0e5a610b5ca5 ("ppp: mppe: switch to RC4 library interface"), which was merged through the crypto tree for v5.3, changed ppp_mppe.c to use the new arc4_crypt() library function rather than access RC4 through the dynamic crypto_skcipher API.
Meanwhile commit aad1dcc4f011 ("ppp: mppe: Add softdep to arc4") was merged through the net tree and added a module soft-dependency on "arc4".
The latter commit no longer makes sense because the code now uses the "libarc4" module rather than "arc4", and also due to the direct use of arc4_crypt(), no module soft-dependency is required.
So revert the latter commit.
Cc: Takashi Iwai tiwai@suse.de Cc: Ard Biesheuvel ard.biesheuvel@linaro.org Signed-off-by: Eric Biggers ebiggers@google.com Signed-off-by: David S. Miller davem@davemloft.net Signed-off-by: Baolin Wang baolin.wang@linaro.org
drivers/net/ppp/ppp_mppe.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/drivers/net/ppp/ppp_mppe.c b/drivers/net/ppp/ppp_mppe.c index d9eda7c..6c7fd98 100644 --- a/drivers/net/ppp/ppp_mppe.c +++ b/drivers/net/ppp/ppp_mppe.c @@ -63,7 +63,6 @@ MODULE_DESCRIPTION("Point-to-Point Protocol Microsoft Point-to-Point Encryption support"); MODULE_LICENSE("Dual BSD/GPL"); MODULE_ALIAS("ppp-compress-" __stringify(CI_MPPE)); -MODULE_SOFTDEP("pre: arc4");
Why is this being backported? This revert was only needed because of a different patch that was merged in v5.3, as I explained in the commit message.
- Eric
On Fri, 6 Sep 2019 at 00:16, Eric Biggers ebiggers@google.com wrote:
On Thu, Sep 05, 2019 at 11:10:45AM +0800, Baolin Wang wrote:
From: Eric Biggers ebiggers@google.com
[Upstream commit 25a09ce79639a8775244808c17282c491cff89cf]
Commit 0e5a610b5ca5 ("ppp: mppe: switch to RC4 library interface"), which was merged through the crypto tree for v5.3, changed ppp_mppe.c to use the new arc4_crypt() library function rather than access RC4 through the dynamic crypto_skcipher API.
Meanwhile commit aad1dcc4f011 ("ppp: mppe: Add softdep to arc4") was merged through the net tree and added a module soft-dependency on "arc4".
The latter commit no longer makes sense because the code now uses the "libarc4" module rather than "arc4", and also due to the direct use of arc4_crypt(), no module soft-dependency is required.
So revert the latter commit.
Cc: Takashi Iwai tiwai@suse.de Cc: Ard Biesheuvel ard.biesheuvel@linaro.org Signed-off-by: Eric Biggers ebiggers@google.com Signed-off-by: David S. Miller davem@davemloft.net Signed-off-by: Baolin Wang baolin.wang@linaro.org
drivers/net/ppp/ppp_mppe.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/drivers/net/ppp/ppp_mppe.c b/drivers/net/ppp/ppp_mppe.c index d9eda7c..6c7fd98 100644 --- a/drivers/net/ppp/ppp_mppe.c +++ b/drivers/net/ppp/ppp_mppe.c @@ -63,7 +63,6 @@ MODULE_DESCRIPTION("Point-to-Point Protocol Microsoft Point-to-Point Encryption support"); MODULE_LICENSE("Dual BSD/GPL"); MODULE_ALIAS("ppp-compress-" __stringify(CI_MPPE)); -MODULE_SOFTDEP("pre: arc4");
Why is this being backported? This revert was only needed because of a different patch that was merged in v5.3, as I explained in the commit message.
Sorry I missed this. I should remove this patch from our product kernel too.
From: Lanqing Liu lanqing.liu@unisoc.com
[Upstream commit 5b9cea15a3de5d65000d49f626b71b00d42a0577]
When the source clock is not divisible by the expected baud rate and the remainder is not less than half of the expected baud rate, the old formular will round up the frequency division coefficient. This will make the actual baud rate less than the expected value and can not meet the external transmission requirements.
Thus this patch modifies the baud rate calculation formula to support the serial controller output the maximum baud rate.
Signed-off-by: Lanqing Liu lanqing.liu@unisoc.com Signed-off-by: Baolin Wang baolin.wang@linaro.org Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org Signed-off-by: Baolin Wang baolin.wang@linaro.org --- drivers/tty/serial/sprd_serial.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/sprd_serial.c b/drivers/tty/serial/sprd_serial.c index e902494..72e96ab8 100644 --- a/drivers/tty/serial/sprd_serial.c +++ b/drivers/tty/serial/sprd_serial.c @@ -380,7 +380,7 @@ static void sprd_set_termios(struct uart_port *port, /* ask the core to calculate the divisor for us */ baud = uart_get_baud_rate(port, termios, old, 0, SPRD_BAUD_IO_LIMIT);
- quot = (unsigned int)((port->uartclk + baud / 2) / baud); + quot = port->uartclk / baud;
/* set data length */ switch (termios->c_cflag & CSIZE) {
On Thu, Sep 05, 2019 at 11:11:26AM +0800, Baolin Wang wrote:
From: Lanqing Liu lanqing.liu@unisoc.com
[Upstream commit 5b9cea15a3de5d65000d49f626b71b00d42a0577]
When the source clock is not divisible by the expected baud rate and the remainder is not less than half of the expected baud rate, the old formular will round up the frequency division coefficient. This will make the actual baud rate less than the expected value and can not meet the external transmission requirements.
Thus this patch modifies the baud rate calculation formula to support the serial controller output the maximum baud rate.
Signed-off-by: Lanqing Liu lanqing.liu@unisoc.com Signed-off-by: Baolin Wang baolin.wang@linaro.org Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org Signed-off-by: Baolin Wang baolin.wang@linaro.org
drivers/tty/serial/sprd_serial.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/sprd_serial.c b/drivers/tty/serial/sprd_serial.c index e902494..72e96ab8 100644 --- a/drivers/tty/serial/sprd_serial.c +++ b/drivers/tty/serial/sprd_serial.c @@ -380,7 +380,7 @@ static void sprd_set_termios(struct uart_port *port, /* ask the core to calculate the divisor for us */ baud = uart_get_baud_rate(port, termios, old, 0, SPRD_BAUD_IO_LIMIT);
- quot = (unsigned int)((port->uartclk + baud / 2) / baud);
- quot = port->uartclk / baud;
Are you sure the original patch is even correct?
By replacing the divisor rounding with truncation you are introducing larger errors for some baud rates, something which could possibly even break working systems.
Perhaps the original patch should even be reverted, but in any case backporting this to stable looks questionable.
/* set data length */ switch (termios->c_cflag & CSIZE) {
Johan
Hi Johan,
On Thu, 5 Sep 2019 at 17:01, Johan Hovold johan@kernel.org wrote:
On Thu, Sep 05, 2019 at 11:11:26AM +0800, Baolin Wang wrote:
From: Lanqing Liu lanqing.liu@unisoc.com
[Upstream commit 5b9cea15a3de5d65000d49f626b71b00d42a0577]
When the source clock is not divisible by the expected baud rate and the remainder is not less than half of the expected baud rate, the old formular will round up the frequency division coefficient. This will make the actual baud rate less than the expected value and can not meet the external transmission requirements.
Thus this patch modifies the baud rate calculation formula to support the serial controller output the maximum baud rate.
Signed-off-by: Lanqing Liu lanqing.liu@unisoc.com Signed-off-by: Baolin Wang baolin.wang@linaro.org Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org Signed-off-by: Baolin Wang baolin.wang@linaro.org
drivers/tty/serial/sprd_serial.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/sprd_serial.c b/drivers/tty/serial/sprd_serial.c index e902494..72e96ab8 100644 --- a/drivers/tty/serial/sprd_serial.c +++ b/drivers/tty/serial/sprd_serial.c @@ -380,7 +380,7 @@ static void sprd_set_termios(struct uart_port *port, /* ask the core to calculate the divisor for us */ baud = uart_get_baud_rate(port, termios, old, 0, SPRD_BAUD_IO_LIMIT);
quot = (unsigned int)((port->uartclk + baud / 2) / baud);
quot = port->uartclk / baud;Are you sure the original patch is even correct?
By replacing the divisor rounding with truncation you are introducing larger errors for some baud rates, something which could possibly even break working systems.
Our UART clock source is 26M, and there is no difference for lower than 3M baud rate between dividing closest or dividing down. But we have one special use case is our BT/GPS want to set 3.25M baud rate, but we have to select 3M baud rate in baud_table since no 3.25M setting. So in this case if we use the old formula, we will only get about 2.8M baud rate, which can not meet our requirement. If we change the dividing down method, we can get 3.25M baud rate.
I have to say this is a workaroud for our special case, and can solve our problem. If you have any good suggestion, we can change to a better solution. Thanks.
Perhaps the original patch should even be reverted, but in any case backporting this to stable looks questionable.
/* set data length */ switch (termios->c_cflag & CSIZE) {Johan
On Thu, Sep 05, 2019 at 05:58:05PM +0800, Baolin Wang wrote:
Hi Johan,
On Thu, 5 Sep 2019 at 17:01, Johan Hovold johan@kernel.org wrote:
On Thu, Sep 05, 2019 at 11:11:26AM +0800, Baolin Wang wrote:
From: Lanqing Liu lanqing.liu@unisoc.com
[Upstream commit 5b9cea15a3de5d65000d49f626b71b00d42a0577]
When the source clock is not divisible by the expected baud rate and the remainder is not less than half of the expected baud rate, the old formular will round up the frequency division coefficient. This will make the actual baud rate less than the expected value and can not meet the external transmission requirements.
Thus this patch modifies the baud rate calculation formula to support the serial controller output the maximum baud rate.
Signed-off-by: Lanqing Liu lanqing.liu@unisoc.com Signed-off-by: Baolin Wang baolin.wang@linaro.org Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org Signed-off-by: Baolin Wang baolin.wang@linaro.org
drivers/tty/serial/sprd_serial.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/sprd_serial.c b/drivers/tty/serial/sprd_serial.c index e902494..72e96ab8 100644 --- a/drivers/tty/serial/sprd_serial.c +++ b/drivers/tty/serial/sprd_serial.c @@ -380,7 +380,7 @@ static void sprd_set_termios(struct uart_port *port, /* ask the core to calculate the divisor for us */ baud = uart_get_baud_rate(port, termios, old, 0, SPRD_BAUD_IO_LIMIT);
quot = (unsigned int)((port->uartclk + baud / 2) / baud);
quot = port->uartclk / baud;Are you sure the original patch is even correct?
By replacing the divisor rounding with truncation you are introducing larger errors for some baud rates, something which could possibly even break working systems.
Our UART clock source is 26M, and there is no difference for lower than 3M baud rate between dividing closest or dividing down.
But there is; you have introduced larger errors for at least a few standard rates by changing to truncation.
But we have one special use case is our BT/GPS want to set 3.25M baud rate, but we have to select 3M baud rate in baud_table since no 3.25M setting. So in this case if we use the old formula, we will only get about 2.8M baud rate, which can not meet our requirement. If we change the dividing down method, we can get 3.25M baud rate.
I have to say this is a workaroud for our special case, and can solve our problem. If you have any good suggestion, we can change to a better solution. Thanks.
Yeah, I don't think purposefully introducing larger errors, and risk breaking other people's setups, to work around that use case is warranted.
We have an interface for setting arbitrary baudrates (TCSETS2) which you can you use (even if glibc support is still not in place). You'd just need to lift the seemingly arbitrary limitation to 3 Mbaud in the driver first.
Perhaps the original patch should even be reverted, but in any case backporting this to stable looks questionable.
So I think reverting this may be the right thing to do.
Johan
linux-stable-mirror@lists.linaro.org
-
Baolin Wang -
Eric Biggers -
Greg KH -
Johan Hovold