On 2016-02-16 22:02, Jakub Kiciński wrote:
On Tue, 16 Feb 2016 08:37:11 +0100, Sean Nyekjær wrote:
On 2016-02-15 23:08, Jakub Kicinski wrote:
On Mon, 15 Feb 2016 08:58:53 +0100, Sean Nyekjaer wrote:
Working with RT patches reveals this driver have some spin_lock issues.
spin_lock moved from sc16is7xx_reg_proc to protect to call to
uart_handle_cts_change as it's the only call that isn't already
protected in serial-core.
Sorry but this does not look fine. You are removing all async items
from the driver. This works for you because in RT code can sleep with
spin locks taken but for upstream it's not acceptable. Did you test
this patch with upstream kernel and lockdep enabled?
I have a 4.1.y with cherry-pick of the commits from tty-testing
regarding sc16is7xx. (exept the new gpio api)
On our custom board we have 3 sc16is750 on a single SPI channel. I'm
using an FTDI chip(connected to a test PC) TX wired to the 3x RX pins on
the sc16is750.
The problem is easy reproducible just by creating data from the PC at
115200 baud, causes the kernel oops almost immediately.
I have not seen any issues with a vanilla 4.1.y, only when i'm applying
the RT patch and loading the system.
Thanks for providing the details, I think there is a misunderstanding
between us. Allow me to explain again what I meant ;)
I do acknowledge that there may be a problem with the driver on RT
kernels and I do trust you that you tested your patch on RT kernel
with latest version of the driver cherry-picked from mainline and it
works there.
However, IIUC you are proposing your patch to be included in mainline.
What I'm trying to say is that in mainline we need the async works
which you remove in your patch because we cannot perform SPI/I2C
blocking I/O under a spinlock... If you compile mainline with your
patch you will see a slew of "sleeping in atomic context" warnings.
So basically the questions are do you want this patch in mainline and
have you tested it on non-RT kernels? (Please don't be discouraged -
there definitely must be a bug somewhere if you're seeing crashes, but
I'm afraid the proposed solutions is not good enough...)
Thanks
Yeah the first priority must be for the driver to work on the mainline
kernel :-)
I just wanted to flag a problem with the driver with the RT patches
applied. I thought
RT patch (maybe) revaled that the driver have underlying problem.
So maybe this patch should be included in the RT patch?
No i have not tested this with a non-RT kernel, maybe i should try :-)
/Sean
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
