On Thu, Feb 01, 2024 at 02:03:25PM +0100, Christina Quast wrote: > Hi Michael! > > The FIXMEs should have been removed before submitting, sorry for that. They were just a reminder to myself to check the kfifo spinlocks. > > The code looks good to me! What's the next steps? Should I include your uart related changes to the patch and resend it to the mailng list? > Hi Christina, Thanks for pushing the UART support forward. It's much appreciated. Unfortunately, your latest patchset (V4) had compilation issues, broke the ft260 I2C driver, and lost the data on the RX line at higher than 4800 baud rates. To shorten the cycle, it will be better that I submit the code from my repository. Otherwise, I will need to review the new patch set and retest everything again. Also, I want to cover more corner cases, and then I will submit the patchset with your and Daniel's credentials. Jiri, there is a pending patchset related to the GPIO support - https://www.spinics.net/lists/linux-input/msg83937.html The ft260 chip has multifunctional pins, and their configuration affects the UART operation. The GPIO patchset adds this part. I am considering rebasing it above the UART code discussed in this email thread. I wonder whether resubmitting the new patch set that includes UART and multifunctional pins (GPIO) support will be a better approach. What do you think? Thanks, Michael > > > -------- Original Message -------- > From: Michael Zaidman <michael.zaidman@xxxxxxxxx> > Sent: Wed Jan 31 16:48:55 GMT+01:00 2024 > To: Daniel Beer <daniel.beer@xxxxxxxxxxxxxxxxx>, Christina Quast <contact@xxxxxxxxxxxxxxxxxx> > Cc: linux-serial@xxxxxxxxxxxxxxx, ilpo.jarvinen@xxxxxxxxxxxxxxx, johan@xxxxxxxxxx, gregkh@xxxxxxxxxxxxxxxxxxx, David Lamparter <equinox@xxxxxxxxxx>, Jiri Kosina <jikos@xxxxxxxxxx>, Michael Zaidman <michael.zaidman@xxxxxxxxx> > Subject: Re: [PATCH v4 RESEND] hid-ft260: Add serial driver > > On Wed, Jan 31, 2024 at 07:28:25PM +1300, Daniel Beer wrote: > > On Mon, Jan 29, 2024 at 12:07:48AM +0200, Michael Zaidman wrote: > > > I pushed changes into https://github.com/MichaelZaidman/hid-ft260/tree/uart > > > branch and would appreciate your review and feedback before preparing the > > > cumulative patch set. > > > > > > I addressed the first FIXME. Please correct me if I did not get right what > > > you meant. > > > > > > I am still doubting regarding the second FIXME. I see only three tty drivers > > > are using the kfifo_avail and kfifo_len to retrieve the write room and number > > > of chars in the buffer, and neither use locking. What was your concern? > > > > I don't see anything obviously wrong, but I'm probably not the best > > person to be reviewing this -- Christina has spent the most time on it > > recently, and I think the FIXME comments are hers. > > > Thanks, Daniel, for your feedback. > I see, it's my mistake. I should have reached out to both of you. > > I added multiple fine-grained commits to simplify the review. > Christina, can you clarify the FIXME issue and review the changes? > > Thanks, > Michael >
