Re: [PATCH 14/25] serial: sh-sci: Add BRG register definitions

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Geert,

Thank you for the patch.

On Thursday 19 November 2015 19:38:53 Geert Uytterhoeven wrote:
> Add register definitions for the Baud Rate Generator for External Clock
> (BRG), as found in some SCIF and in HSCIF, including a new regtype for
> the "SH-4(A)"-derived SCIF variant with BRG.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  drivers/tty/serial/sh-sci.c | 46 ++++++++++++++++++++++++++++++++++++++++++
>  drivers/tty/serial/sh-sci.h |  5 +++++
>  include/linux/serial_sci.h  |  1 +
>  3 files changed, 52 insertions(+)
> 
> diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> index d82735dd62ae38d8..76738c9918885764 100644
> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -161,6 +161,8 @@ static const struct plat_sci_reg
> sci_regmap[SCIx_NR_REGTYPES][SCIx_NR_REGS] = { [HSSRR]		= 
sci_reg_invalid,
>  		[SCPCR]		= sci_reg_invalid,
>  		[SCPDR]		= sci_reg_invalid,
> +		[SCDL]		= sci_reg_invalid,
> +		[SCCKS]		= sci_reg_invalid,
>  	},
> 
>  	/*
> @@ -183,6 +185,8 @@ static const struct plat_sci_reg
> sci_regmap[SCIx_NR_REGTYPES][SCIx_NR_REGS] = { [HSSRR]		= 
sci_reg_invalid,
>  		[SCPCR]		= sci_reg_invalid,
>  		[SCPDR]		= sci_reg_invalid,
> +		[SCDL]		= sci_reg_invalid,
> +		[SCCKS]		= sci_reg_invalid,
>  	},
> 
>  	/*
> @@ -204,6 +208,8 @@ static const struct plat_sci_reg
> sci_regmap[SCIx_NR_REGTYPES][SCIx_NR_REGS] = { [HSSRR]		= 
sci_reg_invalid,
>  		[SCPCR]		= { 0x30, 16 },
>  		[SCPDR]		= { 0x34, 16 },
> +		[SCDL]		= sci_reg_invalid,
> +		[SCCKS]		= sci_reg_invalid,
>  	},
> 
>  	/*
> @@ -225,6 +231,8 @@ static const struct plat_sci_reg
> sci_regmap[SCIx_NR_REGTYPES][SCIx_NR_REGS] = { [HSSRR]		= 
sci_reg_invalid,
>  		[SCPCR]		= { 0x30, 16 },
>  		[SCPDR]		= { 0x34, 16 },
> +		[SCDL]		= sci_reg_invalid,
> +		[SCCKS]		= sci_reg_invalid,
>  	},
> 
>  	/*
> @@ -247,6 +255,8 @@ static const struct plat_sci_reg
> sci_regmap[SCIx_NR_REGTYPES][SCIx_NR_REGS] = { [HSSRR]		= 
sci_reg_invalid,
>  		[SCPCR]		= sci_reg_invalid,
>  		[SCPDR]		= sci_reg_invalid,
> +		[SCDL]		= sci_reg_invalid,
> +		[SCCKS]		= sci_reg_invalid,
>  	},
> 
>  	/*
> @@ -268,6 +278,8 @@ static const struct plat_sci_reg
> sci_regmap[SCIx_NR_REGTYPES][SCIx_NR_REGS] = { [HSSRR]		= 
sci_reg_invalid,
>  		[SCPCR]		= sci_reg_invalid,
>  		[SCPDR]		= sci_reg_invalid,
> +		[SCDL]		= sci_reg_invalid,
> +		[SCCKS]		= sci_reg_invalid,
>  	},
> 
>  	/*
> @@ -289,6 +301,32 @@ static const struct plat_sci_reg
> sci_regmap[SCIx_NR_REGTYPES][SCIx_NR_REGS] = { [HSSRR]		= 
sci_reg_invalid,
>  		[SCPCR]		= sci_reg_invalid,
>  		[SCPDR]		= sci_reg_invalid,
> +		[SCDL]		= sci_reg_invalid,
> +		[SCCKS]		= sci_reg_invalid,
> +	},
> +
> +	/*
> +	 * Common SCIF definitions for ports with a Baud Rate Generator for
> +	 * External Clock (BRG).
> +	 */
> +	[SCIx_SH4_SCIF_BRG_REGTYPE] = {
> +		[SCSMR]		= { 0x00, 16 },
> +		[SCBRR]		= { 0x04,  8 },
> +		[SCSCR]		= { 0x08, 16 },
> +		[SCxTDR]	= { 0x0c,  8 },
> +		[SCxSR]		= { 0x10, 16 },
> +		[SCxRDR]	= { 0x14,  8 },
> +		[SCFCR]		= { 0x18, 16 },
> +		[SCFDR]		= { 0x1c, 16 },
> +		[SCTFDR]	= sci_reg_invalid,
> +		[SCRFDR]	= sci_reg_invalid,
> +		[SCSPTR]	= { 0x20, 16 },
> +		[SCLSR]		= { 0x24, 16 },
> +		[HSSRR]		= sci_reg_invalid,
> +		[SCPCR]		= sci_reg_invalid,
> +		[SCPDR]		= sci_reg_invalid,
> +		[SCDL]		= { 0x30, 16 },
> +		[SCCKS]		= { 0x34, 16 },
>  	},
> 
>  	/*
> @@ -310,6 +348,8 @@ static const struct plat_sci_reg
> sci_regmap[SCIx_NR_REGTYPES][SCIx_NR_REGS] = { [HSSRR]		= { 0x40, 16 },
>  		[SCPCR]		= sci_reg_invalid,
>  		[SCPDR]		= sci_reg_invalid,
> +		[SCDL]		= { 0x30, 16 },
> +		[SCCKS]		= { 0x34, 16 },
>  	},
> 
>  	/*
> @@ -332,6 +372,8 @@ static const struct plat_sci_reg
> sci_regmap[SCIx_NR_REGTYPES][SCIx_NR_REGS] = { [HSSRR]		= 
sci_reg_invalid,
>  		[SCPCR]		= sci_reg_invalid,
>  		[SCPDR]		= sci_reg_invalid,
> +		[SCDL]		= sci_reg_invalid,
> +		[SCCKS]		= sci_reg_invalid,
>  	},
> 
>  	/*
> @@ -354,6 +396,8 @@ static const struct plat_sci_reg
> sci_regmap[SCIx_NR_REGTYPES][SCIx_NR_REGS] = { [HSSRR]		= 
sci_reg_invalid,
>  		[SCPCR]		= sci_reg_invalid,
>  		[SCPDR]		= sci_reg_invalid,
> +		[SCDL]		= sci_reg_invalid,
> +		[SCCKS]		= sci_reg_invalid,
>  	},
> 
>  	/*
> @@ -376,6 +420,8 @@ static const struct plat_sci_reg
> sci_regmap[SCIx_NR_REGTYPES][SCIx_NR_REGS] = { [HSSRR]		= 
sci_reg_invalid,
>  		[SCPCR]		= sci_reg_invalid,
>  		[SCPDR]		= sci_reg_invalid,
> +		[SCDL]		= sci_reg_invalid,
> +		[SCCKS]		= sci_reg_invalid,
>  	},
>  };
> 
> diff --git a/drivers/tty/serial/sh-sci.h b/drivers/tty/serial/sh-sci.h
> index bf69bbdcc1f9aa39..54994f0bfafdbe12 100644
> --- a/drivers/tty/serial/sh-sci.h
> +++ b/drivers/tty/serial/sh-sci.h
> @@ -27,6 +27,8 @@ enum {
>  	HSSRR,				/* Sampling Rate Register */
>  	SCPCR,				/* Serial Port Control Register */
>  	SCPDR,				/* Serial Port Data Register */
> +	SCDL,				/* BRG Frequency Division Register */
> +	SCCKS,				/* BRG Clock Select Register */
> 
>  	SCIx_NR_REGS,
>  };
> @@ -109,6 +111,9 @@ enum {
>  #define SCPDR_RTSD	BIT(4)	/* Serial Port RTS Output Pin Data */
>  #define SCPDR_CTSD	BIT(3)	/* Serial Port CTS Input Pin Data */
> 
> +/* BRG Clock Select Register (Some SCIF and HSCIF) */
> +#define SCCKS_CKS	BIT(15)	/* Select SCK (1) or SC_CLK (0) */

This is slightly confusing. SC_CLK is defined as the external clock selectable 
between the bus clock and SCIF_CLK. The CKS bit selects the output of the baud 
rate generator for external clock as being the divided SC_CLK or the undivided 
(H)SCK. The comment, and I know it comes from the datasheet, seems to imply 
that the bit selects the input of the BRG-EC. Speaking of which it would be 
good to have a comment somewhere in the code to explain that we have two 
chained BRGs.

> +#define SCCKS_XIN	BIT(14)	/* SC_CLK uses bus clock (1) or SCIF_CLK (0) 
*/
> 
>  #define SCxSR_TEND(port)	(((port)->type = PORT_SCI) ? SCI_TEND   :
> SCIF_TEND) #define SCxSR_RDxF(port)	(((port)->type = PORT_SCI) ? 
SCI_RDRF 
>  : SCIF_RDF) diff --git a/include/linux/serial_sci.h
> b/include/linux/serial_sci.h index 7c536ac5be05d3aa..9f2bfd0557429ac3
> 100644
> --- a/include/linux/serial_sci.h
> +++ b/include/linux/serial_sci.h
> @@ -32,6 +32,7 @@ enum {
>  	SCIx_SH2_SCIF_FIFODATA_REGTYPE,
>  	SCIx_SH3_SCIF_REGTYPE,
>  	SCIx_SH4_SCIF_REGTYPE,
> +	SCIx_SH4_SCIF_BRG_REGTYPE,
>  	SCIx_SH4_SCIF_NO_SCSPTR_REGTYPE,
>  	SCIx_SH4_SCIF_FIFODATA_REGTYPE,
>  	SCIx_SH7705_SCIF_REGTYPE,

-- 
Regards,

Laurent Pinchart