Re: [PATCH v4 6/6] serial: 8250_em: Add serial8250_em_{reg_update(),out_helper()}

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Fri, Feb 17, 2023 at 11:42:55AM +0000, Biju Das wrote:
> As per RZ/V2M hardware manual(Rev.1.30 Jun, 2022), UART IP has a
> restriction as mentioned below.
> 
> 40.6.1 Point for Caution when Changing the Register Settings:
> 
> When changing the settings of the following registers, a PRESETn master
> reset or FIFO reset + SW reset (FCR[2],FCR[1], HCR0[7]) must be input to
> re-initialize them.
> 
> Target Registers: FCR, LCR, MCR, DLL, DLM, HCR0.
> 
> This patch adds serial8250_em_reg_update() and serial8250_em_serial_
> out_helper to handle it.
> 
> DLL/DLM register can be updated only by setting LCR[7]. So the
> updation of LCR[7] will perform reset for DLL/DLM register changes.
> 
> EMMA mobile has the same register set as RZ/V2M and this patch is tested on
> EMEV2 board. So, there is no harm in applying the same restriction here as
> well as the HW manual for EMMA mobile is not updated for a long time.

...

> +	serial8250_em_serial_out_helper(p, UART_FCR_EM, fcr | UART_FCR_CLEAR_RCVR |
> +				 UART_FCR_CLEAR_XMIT);


I would put it like

	serial8250_em_serial_out_helper(p, UART_FCR_EM, fcr |
							UART_FCR_CLEAR_RCVR |
							UART_FCR_CLEAR_XMIT);

...

> +	switch (off) {
> +	case UART_FCR_EM:
> +		fcr = value;
> +		break;
> +	case UART_LCR:
> +		lcr = value;
> +		break;
> +	case UART_MCR:
> +		mcr = value;

Missing break; statement.

> +	}

...

> +	switch (offset) {
> +	case UART_TX:
> +	case UART_SCR:
> +	case UART_IER:
> +	case UART_DLL_EM:
> +	case UART_DLM_EM:
> +		serial8250_em_serial_out_helper(p, offset, value);
> +		break;
> +	case UART_FCR:
> +		serial8250_em_reg_update(p, UART_FCR_EM, value);
> +		break;
> +	case UART_LCR:
> +	case UART_MCR:
> +		serial8250_em_reg_update(p, offset, value);

Missing break; statement.

> +	}

-- 
With Best Regards,
Andy Shevchenko