| Message ID | 20230525040324.3773741-1-hugo@hugovil.com |
|---|---|
| Headers | show |
| Series | serial: sc16is7xx: fix GPIO regression and rs485 improvements | expand |
Thu, May 25, 2023 at 12:03:13AM -0400, Hugo Villeneuve kirjoitti: > From: Hugo Villeneuve <hvilleneuve@dimonoff.com> > > Hello, > this patch series mainly fixes a GPIO regression and improve RS485 flags and properties > detection from DT. > > It now also includes various small fixes and improvements that were previously > sent as separate patches, but that made testing everything difficult. > Patches 1 and 2 are simple comments fix/improvements. Usually we put fixes at the beginning of the series, but these patches are missing Fixed tag. Are they really fixes or can be simply moved to the end of the series? > Patch 3 fixes an issue when debugging IOcontrol register. After testing the GPIO > regression patches (patches 6 and 7, tests done by Lech Perczak), it appers that > this patch is also necessary for having the correct IOcontrol register values. > > Patch 4 introduces a delay after a reset operation to respect datasheet > timing recommandations. > > Patch 5 fixes an issue with init of first port during probing. This commit > brings some questions and I appreciate if people from the serial subsystem could > comment on my proposed solution. > > Patch 6 fixes a bug with the output value when first setting the GPIO direction. > > Patch 7, 8 and 9 fix a GPIO regression by (re)allowing to choose GPIO function for > GPIO pins shared with modem status lines. > > Patch 10 allows to read common rs485 device-tree flags and properties. > > Patch 11 add a custom dump function as relying on regmal debugfs is not really > practical for this driver. > > I have tested the changes on a custom board with two SC16IS752 DUART using a > Variscite IMX8MN NANO SOM. Other comments are per individual emails.
On Thu, 25 May 2023 13:27:52 +0300 andy.shevchenko@gmail.com wrote: > Thu, May 25, 2023 at 12:03:13AM -0400, Hugo Villeneuve kirjoitti: > > From: Hugo Villeneuve <hvilleneuve@dimonoff.com> > > > > Hello, > > this patch series mainly fixes a GPIO regression and improve RS485 flags and properties > > detection from DT. > > > > It now also includes various small fixes and improvements that were previously > > sent as separate patches, but that made testing everything difficult. > > > Patches 1 and 2 are simple comments fix/improvements. > > Usually we put fixes at the beginning of the series, but these patches are > missing Fixed tag. Are they really fixes or can be simply moved to the end of > the series? Hi, these are not code fixes, they are comments improvements. I was not aware that you need to put a Fixes tag for correcting syntax errors in comments, or adding comments to improve clarity. I often submit such comments patches but was never asked to put a Fixes tag before. Seems strange to me... Hugo. > > Patch 3 fixes an issue when debugging IOcontrol register. After testing the GPIO > > regression patches (patches 6 and 7, tests done by Lech Perczak), it appers that > > this patch is also necessary for having the correct IOcontrol register values. > > > > Patch 4 introduces a delay after a reset operation to respect datasheet > > timing recommandations. > > > > Patch 5 fixes an issue with init of first port during probing. This commit > > brings some questions and I appreciate if people from the serial subsystem could > > comment on my proposed solution. > > > > Patch 6 fixes a bug with the output value when first setting the GPIO direction. > > > > Patch 7, 8 and 9 fix a GPIO regression by (re)allowing to choose GPIO function for > > GPIO pins shared with modem status lines. > > > > Patch 10 allows to read common rs485 device-tree flags and properties. > > > > Patch 11 add a custom dump function as relying on regmal debugfs is not really > > practical for this driver. > > > > I have tested the changes on a custom board with two SC16IS752 DUART using a > > Variscite IMX8MN NANO SOM. > > Other comments are per individual emails. > > -- > With Best Regards, > Andy Shevchenko > > >
On Thu, May 25, 2023 at 4:26 PM Hugo Villeneuve <hugo@hugovil.com> wrote: > On Thu, 25 May 2023 13:27:52 +0300 > andy.shevchenko@gmail.com wrote: > > Thu, May 25, 2023 at 12:03:13AM -0400, Hugo Villeneuve kirjoitti: > > > From: Hugo Villeneuve <hvilleneuve@dimonoff.com> > > > > > > Hello, > > > this patch series mainly fixes a GPIO regression and improve RS485 flags and properties > > > detection from DT. > > > > > > It now also includes various small fixes and improvements that were previously > > > sent as separate patches, but that made testing everything difficult. > > > > > Patches 1 and 2 are simple comments fix/improvements. > > > > Usually we put fixes at the beginning of the series, but these patches are > > missing Fixed tag. Are they really fixes or can be simply moved to the end of > > the series? > > these are not code fixes, they are comments improvements. I was not aware that you need to put a Fixes tag for correcting syntax errors in comments, or adding comments to improve clarity. > > I often submit such comments patches but was never asked to put a Fixes tag before. Seems strange to me... In this case there are probably no conflicts, but the usual grouping of patches is following 1) fixes that may be backported; 2) cleanups / refactoring /etc; 3) new features. 4) additional light-weit cleanups, such as whitespace cleaning (it's a radical, we probably do not accept pure whitespace cleaning patches, but you got the idea). Seems patches 1 and 2 fall into category 4).
On Thu, 25 May 2023 16:37:17 +0300 Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > On Thu, May 25, 2023 at 4:26 PM Hugo Villeneuve <hugo@hugovil.com> wrote: > > On Thu, 25 May 2023 13:27:52 +0300 > > andy.shevchenko@gmail.com wrote: > > > Thu, May 25, 2023 at 12:03:13AM -0400, Hugo Villeneuve kirjoitti: > > > > From: Hugo Villeneuve <hvilleneuve@dimonoff.com> > > > > > > > > Hello, > > > > this patch series mainly fixes a GPIO regression and improve RS485 flags and properties > > > > detection from DT. > > > > > > > > It now also includes various small fixes and improvements that were previously > > > > sent as separate patches, but that made testing everything difficult. > > > > > > > Patches 1 and 2 are simple comments fix/improvements. > > > > > > Usually we put fixes at the beginning of the series, but these patches are > > > missing Fixed tag. Are they really fixes or can be simply moved to the end of > > > the series? > > > > these are not code fixes, they are comments improvements. I was not aware that you need to put a Fixes tag for correcting syntax errors in comments, or adding comments to improve clarity. > > > > I often submit such comments patches but was never asked to put a Fixes tag before. Seems strange to me... > > In this case there are probably no conflicts, but the usual grouping > of patches is following > 1) fixes that may be backported; > 2) cleanups / refactoring /etc; > 3) new features. > 4) additional light-weit cleanups, such as whitespace cleaning (it's a > radical, we probably do not accept pure whitespace cleaning patches, > but you got the idea). > > Seems patches 1 and 2 fall into category 4). Ok, I changed the order to put these two patches at the end. Hugo.
From: Hugo Villeneuve <hvilleneuve@dimonoff.com> Hello, this patch series mainly fixes a GPIO regression and improve RS485 flags and properties detection from DT. It now also includes various small fixes and improvements that were previously sent as separate patches, but that made testing everything difficult. Patches 1 and 2 are simple comments fix/improvements. Patch 3 fixes an issue when debugging IOcontrol register. After testing the GPIO regression patches (patches 6 and 7, tests done by Lech Perczak), it appers that this patch is also necessary for having the correct IOcontrol register values. Patch 4 introduces a delay after a reset operation to respect datasheet timing recommandations. Patch 5 fixes an issue with init of first port during probing. This commit brings some questions and I appreciate if people from the serial subsystem could comment on my proposed solution. Patch 6 fixes a bug with the output value when first setting the GPIO direction. Patch 7, 8 and 9 fix a GPIO regression by (re)allowing to choose GPIO function for GPIO pins shared with modem status lines. Patch 10 allows to read common rs485 device-tree flags and properties. Patch 11 add a custom dump function as relying on regmal debugfs is not really practical for this driver. I have tested the changes on a custom board with two SC16IS752 DUART using a Variscite IMX8MN NANO SOM. Thank you. Link: [v1] https://lkml.org/lkml/2023/5/17/967 [v1] https://lkml.org/lkml/2023/5/17/777 [v1] https://lkml.org/lkml/2023/5/17/780 [v1] https://lkml.org/lkml/2023/5/17/785 [v1] https://lkml.org/lkml/2023/5/17/1311 [v2] https://lkml.org/lkml/2023/5/18/516 Changes for V3: - Integrated all patches into single serie to facilitate debugging and tests. - Reduce number of exported GPIOs depending on new property nxp,modem-control-line-ports - Added additional example in DT bindings Hugo Villeneuve (11): serial: sc16is7xx: fix syntax error in comments serial: sc16is7xx: improve comments about variants serial: sc16is7xx: mark IOCONTROL register as volatile serial: sc16is7xx: add post reset delay serial: sc16is7xx: fix broken port 0 uart init serial: sc16is7xx: fix bug when first setting GPIO direction dt-bindings: sc16is7xx: Add property to change GPIO function serial: sc16is7xx: fix regression with GPIO configuration serial: sc16is7xx: add I/O register translation offset serial: sc16is7xx: add call to get rs485 DT flags and properties serial: sc16is7xx: add dump registers function .../bindings/serial/nxp,sc16is7xx.txt | 46 +++++ drivers/tty/serial/sc16is7xx.c | 180 +++++++++++++++--- 2 files changed, 199 insertions(+), 27 deletions(-) base-commit: 933174ae28ba72ab8de5b35cb7c98fc211235096