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

[Marek Vasut]

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 ?
Does it misbehave in U-Boot ?

>>> +	 */
>>> +	if (IS_ENABLED(CONFIG_RZG2L)) {
>>> +		unsigned short status = sci_in(port, SCxSR);
>>> +		sci_out(port, SCxSR, status & ~SCIF_ERRORS);
>>> +	} else {
>>> +		sci_in(port, SCxSR);
>>> +		sci_out(port, SCxSR, SCxSR_ERROR_CLEAR(port));
>>> +	}
>>>    	sci_in(port, SCLSR);
>>>    	sci_out(port, SCLSR, 0x00);
>>>    }
>>> @@ -193,6 +203,23 @@ static int sh_serial_probe(struct udevice *dev)
>>>    	priv->type	= plat->type;
>>>    	priv->clk_mode	= plat->clk_mode;
>>>    
>>> +	if (IS_ENABLED(CONFIG_RZG2L)) {
>>> +		struct reset_ctl rst;
>>> +		int ret;
>>> +
>>> +		ret = reset_get_by_index(dev, 0, &rst);
>>> +		if (ret < 0) {
>>> +			dev_err(dev, "failed to get reset line\n");
>>> +			return ret;
>>> +		}
>>> +
>>> +		ret = reset_deassert(&rst);
>>> +		if (ret < 0) {
>>> +			dev_err(dev, "failed to de-assert reset line\n");
>>> +			return ret;
>>> +		}
>>> +	}
>>
>> devm_reset_control_get_optional() or something should do here too , right ?
>>
>> Note that R-Car does have SCIF reset too, so this can be generic code.
> 
> For R-Car systems the current behaviour is working and well tested, we
> concluded that we shouldn't change it so this reset de-assert was made
> RZ/G2L specific.

I can test on R-Car just fine, no worries.

SH R2Dplus is even tested in CI nightly.

> For RZ/G2L, de-asserting the reset is not optional.

Let's avoid special-cases like that.