mbox series

[v8,00/10] serial: sc16is7xx: fix GPIO regression and rs485 improvements

Message ID 20230607140525.833982-1-hugo@hugovil.com
Headers show
Series serial: sc16is7xx: fix GPIO regression and rs485 improvements | expand

Message

Hugo Villeneuve June 7, 2023, 2:05 p.m. UTC 
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.

Patch 1 fixes an issue with init of first port during probing.

Patch 2 fixes an issue when debugging IOcontrol register, but it is also
necessary for patch "fix regression with GPIO configuration" to work.

Patch 3 fixes an incorrect label in sc16is7xx_probe() cleanup code.

Patch 4 is a refactor of GPIO registration code in preparation for patch 5.

Patches 5 and 6 fix a GPIO regression by (re)allowing to choose GPIO function
for GPIO pins shared with modem status lines.

Patch 7 fixes a bug with the output value when first setting the GPIO direction.

Patch 8 allows to read common rs485 device-tree flags and properties.

Patch 9 introduces a delay after a reset operation to respect datasheet
timing recommandations.

Patch 10 improves comments about chip variants.

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
      [v3] https://lkml.org/lkml/2023/5/25/7
      [v4] https://lkml.org/lkml/2023/5/29/656
      [v5] https://lkml.org/lkml/2023/6/1/1046
      [v6] https://lkml.org/lkml/2023/6/1/1328
      [v7] https://lkml.org/lkml/2023/6/2/861

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

Changes for V4:
- Increase reset post delay to relax scheduler.
- Put comments patches at the end.
- Remove Fixes tag for patch "mark IOCONTROL register as volatile".
- Improve commit messages after reviews.
- Fix coding style issues after reviews.
- Change GPIO registration to always register the maximum number of GPIOs
  supported by the chip, but maks-out GPIOs declared as modem control lines.
- Add patch to refactor GPIO registration.
- Remove patch "serial: sc16is7xx: fix syntax error in comments".
- Remove patch "add dump registers function"

Changes for V5:
- Change patch order to facilitate stable backport(s).
- Change duplicate device addresses in DT binding examples.
- Use GENMASK for bit masks.
- Replace of_property_for_each_u32() with device_property_read_u32_array
- Add "Cc: stable..." tags

Changes for V6:
- Fix compilation bug introduced by patch 3

Changes for V7:
- Minor changes and coding style fixes after review for
  patch 5 "fix regression with GPIO configuration".

Changes for V8:
- Move mctrl_mask to "struct sc16is7xx_port" to avoid compiler warning when
  CONFIG_GPIOLIB is undefined.
- Add "struct device" member to "struct sc16is7xx_port", in order to avoid
  passing a raw "struct device" to called functions from sc16is7xx_probe().
- Add new patch "serial: sc16is7xx: remove obsolete out_thread label"

Hugo Villeneuve (10):
  serial: sc16is7xx: fix broken port 0 uart init
  serial: sc16is7xx: mark IOCONTROL register as volatile
  serial: sc16is7xx: remove obsolete out_thread label
  serial: sc16is7xx: refactor GPIO controller registration
  dt-bindings: sc16is7xx: Add property to change GPIO function
  serial: sc16is7xx: fix regression with GPIO configuration
  serial: sc16is7xx: fix bug when first setting GPIO direction
  serial: sc16is7xx: add call to get rs485 DT flags and properties
  serial: sc16is7xx: add post reset delay
  serial: sc16is7xx: improve comments about variants

 .../bindings/serial/nxp,sc16is7xx.txt         |  46 +++++
 drivers/tty/serial/sc16is7xx.c                | 177 +++++++++++++-----
 2 files changed, 181 insertions(+), 42 deletions(-)


base-commit: a4d7d701121981e3c3fe69ade376fe9f26324161

Comments

Lech Perczak June 12, 2023, 12:10 p.m. UTC | #1 
W dniu 7.06.2023 o 16:05, Hugo Villeneuve pisze:
> 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.
>
> Patch 1 fixes an issue with init of first port during probing.
>
> Patch 2 fixes an issue when debugging IOcontrol register, but it is also
> necessary for patch "fix regression with GPIO configuration" to work.
>
> Patch 3 fixes an incorrect label in sc16is7xx_probe() cleanup code.
>
> Patch 4 is a refactor of GPIO registration code in preparation for patch 5.
>
> Patches 5 and 6 fix a GPIO regression by (re)allowing to choose GPIO function
> for GPIO pins shared with modem status lines.
>
> Patch 7 fixes a bug with the output value when first setting the GPIO direction.
>
> Patch 8 allows to read common rs485 device-tree flags and properties.
>
> Patch 9 introduces a delay after a reset operation to respect datasheet
> timing recommandations.
>
> Patch 10 improves comments about chip variants.
>
> 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 <https://lkml.org/lkml/2023/5/17/967>
> [v1] https://lkml.org/lkml/2023/5/17/777 <https://lkml.org/lkml/2023/5/17/777>
> [v1] https://lkml.org/lkml/2023/5/17/780 <https://lkml.org/lkml/2023/5/17/780>
> [v1] https://lkml.org/lkml/2023/5/17/785 <https://lkml.org/lkml/2023/5/17/785>
> [v1] https://lkml.org/lkml/2023/5/17/1311 <https://lkml.org/lkml/2023/5/17/1311>
> [v2] https://lkml.org/lkml/2023/5/18/516 <https://lkml.org/lkml/2023/5/18/516>
> [v3] https://lkml.org/lkml/2023/5/25/7 <https://lkml.org/lkml/2023/5/25/7>
> [v4] https://lkml.org/lkml/2023/5/29/656 <https://lkml.org/lkml/2023/5/29/656>
> [v5] https://lkml.org/lkml/2023/6/1/1046 <https://lkml.org/lkml/2023/6/1/1046>
> [v6] https://lkml.org/lkml/2023/6/1/1328 <https://lkml.org/lkml/2023/6/1/1328>
> [v7] https://lkml.org/lkml/2023/6/2/861 <https://lkml.org/lkml/2023/6/2/861>
>
> 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
>
> Changes for V4:
> - Increase reset post delay to relax scheduler.
> - Put comments patches at the end.
> - Remove Fixes tag for patch "mark IOCONTROL register as volatile".
> - Improve commit messages after reviews.
> - Fix coding style issues after reviews.
> - Change GPIO registration to always register the maximum number of GPIOs
> supported by the chip, but maks-out GPIOs declared as modem control lines.
> - Add patch to refactor GPIO registration.
> - Remove patch "serial: sc16is7xx: fix syntax error in comments".
> - Remove patch "add dump registers function"
>
> Changes for V5:
> - Change patch order to facilitate stable backport(s).
> - Change duplicate device addresses in DT binding examples.
> - Use GENMASK for bit masks.
> - Replace of_property_for_each_u32() with device_property_read_u32_array
> - Add "Cc: stable..." tags
>
> Changes for V6:
> - Fix compilation bug introduced by patch 3
>
> Changes for V7:
> - Minor changes and coding style fixes after review for
> patch 5 "fix regression with GPIO configuration".
>
> Changes for V8:
> - Move mctrl_mask to "struct sc16is7xx_port" to avoid compiler warning when
> CONFIG_GPIOLIB is undefined.
> - Add "struct device" member to "struct sc16is7xx_port", in order to avoid
> passing a raw "struct device" to called functions from sc16is7xx_probe().
> - Add new patch "serial: sc16is7xx: remove obsolete out_thread label"
>
> Hugo Villeneuve (10):
> serial: sc16is7xx: fix broken port 0 uart init
> serial: sc16is7xx: mark IOCONTROL register as volatile
> serial: sc16is7xx: remove obsolete out_thread label
> serial: sc16is7xx: refactor GPIO controller registration
> dt-bindings: sc16is7xx: Add property to change GPIO function
> serial: sc16is7xx: fix regression with GPIO configuration
> serial: sc16is7xx: fix bug when first setting GPIO direction
> serial: sc16is7xx: add call to get rs485 DT flags and properties
> serial: sc16is7xx: add post reset delay
> serial: sc16is7xx: improve comments about variants
>
> .../bindings/serial/nxp,sc16is7xx.txt | 46 +++++
> drivers/tty/serial/sc16is7xx.c | 177 +++++++++++++-----
> 2 files changed, 181 insertions(+), 42 deletions(-)
>
>
> base-commit: a4d7d701121981e3c3fe69ade376fe9f26324161
> -- 
> 2.30.2
>
Hello Hugo,

I had a chance to retest and re-review everything, including setting state at direction change.
So if the pending changes to v9 are only the conversion of dt-bindings to YAML and fixups to them, then:
Reviewed-by: Lech Perczak <lech.perczak@camlingroup.com>
Tested-by: Lech Perczak <lech.perczak@camlingroup.com>
Hugo Villeneuve June 12, 2023, 1:40 p.m. UTC | #2 
On Mon, 12 Jun 2023 14:10:20 +0200
Lech Perczak <lech.perczak@camlingroup.com> wrote:

> W dniu 7.06.2023 o 16:05, Hugo Villeneuve pisze:
> > 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.
> >
> > Patch 1 fixes an issue with init of first port during probing.
> >
> > Patch 2 fixes an issue when debugging IOcontrol register, but it is also
> > necessary for patch "fix regression with GPIO configuration" to work.
> >
> > Patch 3 fixes an incorrect label in sc16is7xx_probe() cleanup code.
> >
> > Patch 4 is a refactor of GPIO registration code in preparation for patch 5.
> >
> > Patches 5 and 6 fix a GPIO regression by (re)allowing to choose GPIO function
> > for GPIO pins shared with modem status lines.
> >
> > Patch 7 fixes a bug with the output value when first setting the GPIO direction.
> >
> > Patch 8 allows to read common rs485 device-tree flags and properties.
> >
> > Patch 9 introduces a delay after a reset operation to respect datasheet
> > timing recommandations.
> >
> > Patch 10 improves comments about chip variants.
> >
> > 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 <https://lkml.org/lkml/2023/5/17/967>
> > [v1] https://lkml.org/lkml/2023/5/17/777 <https://lkml.org/lkml/2023/5/17/777>
> > [v1] https://lkml.org/lkml/2023/5/17/780 <https://lkml.org/lkml/2023/5/17/780>
> > [v1] https://lkml.org/lkml/2023/5/17/785 <https://lkml.org/lkml/2023/5/17/785>
> > [v1] https://lkml.org/lkml/2023/5/17/1311 <https://lkml.org/lkml/2023/5/17/1311>
> > [v2] https://lkml.org/lkml/2023/5/18/516 <https://lkml.org/lkml/2023/5/18/516>
> > [v3] https://lkml.org/lkml/2023/5/25/7 <https://lkml.org/lkml/2023/5/25/7>
> > [v4] https://lkml.org/lkml/2023/5/29/656 <https://lkml.org/lkml/2023/5/29/656>
> > [v5] https://lkml.org/lkml/2023/6/1/1046 <https://lkml.org/lkml/2023/6/1/1046>
> > [v6] https://lkml.org/lkml/2023/6/1/1328 <https://lkml.org/lkml/2023/6/1/1328>
> > [v7] https://lkml.org/lkml/2023/6/2/861 <https://lkml.org/lkml/2023/6/2/861>
> >
> > 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
> >
> > Changes for V4:
> > - Increase reset post delay to relax scheduler.
> > - Put comments patches at the end.
> > - Remove Fixes tag for patch "mark IOCONTROL register as volatile".
> > - Improve commit messages after reviews.
> > - Fix coding style issues after reviews.
> > - Change GPIO registration to always register the maximum number of GPIOs
> > supported by the chip, but maks-out GPIOs declared as modem control lines.
> > - Add patch to refactor GPIO registration.
> > - Remove patch "serial: sc16is7xx: fix syntax error in comments".
> > - Remove patch "add dump registers function"
> >
> > Changes for V5:
> > - Change patch order to facilitate stable backport(s).
> > - Change duplicate device addresses in DT binding examples.
> > - Use GENMASK for bit masks.
> > - Replace of_property_for_each_u32() with device_property_read_u32_array
> > - Add "Cc: stable..." tags
> >
> > Changes for V6:
> > - Fix compilation bug introduced by patch 3
> >
> > Changes for V7:
> > - Minor changes and coding style fixes after review for
> > patch 5 "fix regression with GPIO configuration".
> >
> > Changes for V8:
> > - Move mctrl_mask to "struct sc16is7xx_port" to avoid compiler warning when
> > CONFIG_GPIOLIB is undefined.
> > - Add "struct device" member to "struct sc16is7xx_port", in order to avoid
> > passing a raw "struct device" to called functions from sc16is7xx_probe().
> > - Add new patch "serial: sc16is7xx: remove obsolete out_thread label"
> >
> > Hugo Villeneuve (10):
> > serial: sc16is7xx: fix broken port 0 uart init
> > serial: sc16is7xx: mark IOCONTROL register as volatile
> > serial: sc16is7xx: remove obsolete out_thread label
> > serial: sc16is7xx: refactor GPIO controller registration
> > dt-bindings: sc16is7xx: Add property to change GPIO function
> > serial: sc16is7xx: fix regression with GPIO configuration
> > serial: sc16is7xx: fix bug when first setting GPIO direction
> > serial: sc16is7xx: add call to get rs485 DT flags and properties
> > serial: sc16is7xx: add post reset delay
> > serial: sc16is7xx: improve comments about variants
> >
> > .../bindings/serial/nxp,sc16is7xx.txt | 46 +++++
> > drivers/tty/serial/sc16is7xx.c | 177 +++++++++++++-----
> > 2 files changed, 181 insertions(+), 42 deletions(-)
> >
> >
> > base-commit: a4d7d701121981e3c3fe69ade376fe9f26324161
> > -- 
> > 2.30.2
> >
> Hello Hugo,
> 
> I had a chance to retest and re-review everything, including setting state at direction change.
> So if the pending changes to v9 are only the conversion of dt-bindings to YAML and fixups to them, then:
> Reviewed-by: Lech Perczak <lech.perczak@camlingroup.com>
> Tested-by: Lech Perczak <lech.perczak@camlingroup.com>

Hi Lech,
thank you for testing this again.

By the way, conversion of dt-bindings to YAML and fixups to them will
be done in a new serie, after we finish/close this one.

If all goes well, I do not intend to publish a v9, depending on feedback
from Andy and Greg.

Hugo.


> Pozdrawiam/With kind regards,
> Lech Perczak
> 
> Sr. Software Engineer
> Camlin Technologies Poland Limited Sp. z o.o.
> Strzegomska 54,
> 53-611 Wroclaw
> Tel:     (+48) 71 75 000 16
> Email:   lech.perczak@camlingroup.com
> Website: http://www.camlingroup.com
> 
>