Re: [PATCH v4 0/9] serial: sc16is7xx: fix GPIO regression and rs485 improvements

[Lech Perczak <lech.perczak@xxxxxxxxxxxxxxx>]

W dniu 30.05.2023 o 15:08, Hugo Villeneuve pisze:
> On Tue, 30 May 2023 11:30:07 +0200
> Lech Perczak <lech.perczak@xxxxxxxxxxxxxxx> wrote:
>
> > W dniu 29.05.2023 o 16:07, Hugo Villeneuve pisze:
> > > From: Hugo Villeneuve <hvilleneuve@xxxxxxxxxxxx>
> > >
> > > 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 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 2 introduces a delay after a reset operation to respect datasheet
> > > timing recommandations.
> > >
> > > Patch 3 fixes an issue with init of first port during probing.
> > >
> > > Patch 4 fixes a bug with the output value when first setting the GPIO direction.
> > >
> > > Patch 5 is a refactor of GPIO registration code.
> > >
> > > Patches 6 and 7 fix a GPIO regression by (re)allowing to choose GPIO function
> > > for GPIO pins shared with modem status lines.
> > >
> > > Patch 8 allows to read common rs485 device-tree flags and properties.
> > >
> > > Patch 9 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> <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> <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> <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> <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> <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> <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> <https://lkml.org/lkml/2023/5/25/7 <https://lkml.org/lkml/2023/5/25/7>>
> > >
> > > 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"
> > >
> > > Hugo Villeneuve (9):
> > > 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
> > > serial: sc16is7xx: refactor GPIO controller registration
> > > dt-bindings: sc16is7xx: Add property to change GPIO function
> > > serial: sc16is7xx: fix regression with GPIO configuration
> > > serial: sc16is7xx: add call to get rs485 DT flags and properties
> > > serial: sc16is7xx: improve comments about variants
> > >
> > > .../bindings/serial/nxp,sc16is7xx.txt | 46 ++++++
> > > drivers/tty/serial/sc16is7xx.c | 150 +++++++++++++-----
> > > 2 files changed, 156 insertions(+), 40 deletions(-)
> > >
> > >
> > > base-commit: 8b817fded42d8fe3a0eb47b1149d907851a3c942
> >
> > It would be a lot of sending, to do that for every patch separately, so for whole series:
> > Reviewed-by: Lech Perczak <lech.perczak@xxxxxxxxxxxxxxx>
> >
> > And where applicable - for code patches:
> > Tested-by: Lech Perczak <lech.perczak@xxxxxxxxxxxxxxx>
> >
> > I tested whole series at the same time.
> > I did my tests on an i.MX6 board with SC16IS760 over SPI, which differs a tiny bit from SC16IS752,
> > and everything works as it should.
> > Thank you for fixing this!
>
> Hi Lech,
> thank for your feedback.
>
> You mentioned before that without the patch "mark IOCONTROL register as volatile", things were not working properly for you. Could you retest by removing this patch and see if things are still working?
>
> Thank you, Hugo.

Hello Hugo,

Just checked - this patch is required, reverting it causes things to fail, so this patch should be marked as a pre-requisite for the actual fix and included in backports.
Perhaps using direct write to this register made it work, but it was likely by accident.

-- 
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@xxxxxxxxxxxxxxx
Website: http://www.camlingroup.com