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 -- 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
