[PATCH 10/16] serial: sh: Add RZ/G2L SCIF support

[Marek Vasut]

On 10/4/23 18:38, Paul Barker wrote:
> On Wed, Oct 04, 2023 at 05:17:55PM +0200, Marek Vasut wrote:
>> On 10/4/23 15:43, Paul Barker wrote:
>>> On Wed, Oct 04, 2023 at 02:26:49PM +0200, Marek Vasut wrote:
>>>> On 10/4/23 10:48, Paul Barker wrote:
>>>>> On 03/10/2023 14:23, Marek Vasut wrote:
>>>>>> On 9/20/23 14:42, Paul Barker wrote:
>>>>>>> Extend the existing driver to support the SCIF serial ports on the
>>>>>>> Renesas RZ/G2L (R9A07G044) SoC. This also requires us to ensure that the
>>>>>>> relevant reset signal is de-asserted before we try to talk to the SCIF
>>>>>>> module.
>>>>>>>
>>>>>>> Signed-off-by: Paul Barker <paul.barker.ct at bp.renesas.com>
>>>>>>> Reviewed-by: Biju Das <biju.das.jz at bp.renesas.com>
>>>>>>> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj at bp.renesas.com>
>>>>>>> ---
>>>>>>>      arch/arm/mach-rmobile/Kconfig |  1 +
>>>>>>>      drivers/serial/serial_sh.c    | 32 ++++++++++++++++++++++++++++++--
>>>>>>>      drivers/serial/serial_sh.h    | 19 ++++++++++++++++++-
>>>>>>>      3 files changed, 49 insertions(+), 3 deletions(-)
>>>>>>>
>>>>>>> diff --git a/arch/arm/mach-rmobile/Kconfig b/arch/arm/mach-rmobile/Kconfig
>>>>>>> index 973e84fcf7ba..0ab22356aee5 100644
>>>>>>> --- a/arch/arm/mach-rmobile/Kconfig
>>>>>>> +++ b/arch/arm/mach-rmobile/Kconfig
>>>>>>> @@ -77,6 +77,7 @@ config RZG2L
>>>>>>>      	imply RENESAS_SDHI
>>>>>>>      	imply CLK_RZG2L
>>>>>>>      	imply PINCTRL_RZG2L
>>>>>>> +	imply SCIF_CONSOLE
>>>>>>>      	help
>>>>>>>      	  Enable support for the Renesas RZ/G2L family of SoCs, including the
>>>>>>>      	  the RZ/G2L itself (based on the R9A07G044 SoC).
>>>>>>> diff --git a/drivers/serial/serial_sh.c b/drivers/serial/serial_sh.c
>>>>>>> index 5e543dbf3d58..a2e9a57137a6 100644
>>>>>>> --- a/drivers/serial/serial_sh.c
>>>>>>> +++ b/drivers/serial/serial_sh.c
>>>>>>> @@ -17,6 +17,8 @@
>>>>>>>      #include <linux/compiler.h>
>>>>>>>      #include <dm/platform_data/serial_sh.h>
>>>>>>>      #include <linux/delay.h>
>>>>>>> +#include <dm/device_compat.h>
>>>>>>> +#include <reset.h>
>>>>>>>      #include "serial_sh.h"
>>>>>>>      DECLARE_GLOBAL_DATA_PTR;
>>>>>>> @@ -79,8 +81,16 @@ sh_serial_setbrg_generic(struct uart_port *port, int clk, int baudrate)
>>>>>>>      static void handle_error(struct uart_port *port)
>>>>>>>      {
>>>>>>> -	sci_in(port, SCxSR);
>>>>>>> -	sci_out(port, SCxSR, SCxSR_ERROR_CLEAR(port));
>>>>>>> +	/* The RZ/G2L datasheet says that error conditions are cleared by
>>>>>>> +	 * resetting the error bits in the FSR register to zero.
>>>>>>
>>>>>> Can you be more specific here ?
>>>>>>
>>>>>> It doesn't seem Linux sh-sci.c driver does anything special for G2L, so
>>>>>> is this special case really needed ?
>>>>>
>>>>> On page 1268 of the datasheet (R01UH0914EJ0130 Rev.1.30):
>>>>>
>>>>> "DR is cleared to 0 when DR = 1 is read and then 0 is written to the DR
>>>>> flag."
>>>>>
>>>>> On page 1270:
>>>>>
>>>>> "[Clearing condition]
>>>>> ● When 0 is written to ER after it has been read as 1"
>>>>>
>>>>> So zeros must be written to clear these errors, not ones.
>>>>>
>>>>> We have an open task to investigate the issue in the Linux driver and
>>>>> fix it.
>>>>
>>>> Is the G2L UART broken in Linux ?
>>>
>>> Likely yes, but we need to reproduce the issue.
>>>
>>>> Does it misbehave in U-Boot ?
>>>
>>> Yes, before I changed this, if a framing error occurred it could never
>>> be cleared and the serial port stopped sending/receiving data.
>>>
>>> The framing error was triggered by accident by unnecessarily re-doing
>>> pinmuxing for the serial port after TrustedFirmware had already set it
>>> up correctly. Since SCIF0 is wired to an FTDI USB/serial adaptor chip on
>>> the SMARC carrier board, it seems very difficult to trigger a framing
>>> error in any other way, the FTDI chip is too well behaved.
>>
>> Is it possible to trigger this on RZ/G2<non-L> with SCIF on those platforms
>> as well ?
> 
> It's not been seen or reported to my knowledge, but it's on our list to
> look into further.

Can you give it a quick try ? I'd really like to avoid G2L specific 
behavior here if it can be avoided.

Is there a testcase ?