diff mbox series

serial: zynq: Add support for serial parameters

Message ID 1624335861-4025-1-git-send-email-hayashi.kunihiko@socionext.com
State Superseded
Headers show
Series serial: zynq: Add support for serial parameters | expand

Commit Message

Kunihiko Hayashi June 22, 2021, 4:24 a.m. UTC 
This adds serial parameters that include stop bit mode, parity mode,
and character length. Mark parity and space parity modes are not
supported.

Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>

---
 drivers/serial/serial_zynq.c | 64 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

-- 
2.7.4

Comments

Michal Simek June 22, 2021, 12:44 p.m. UTC | #1 
Hi,

On 6/22/21 6:24 AM, Kunihiko Hayashi wrote:
> This adds serial parameters that include stop bit mode, parity mode,

> and character length. Mark parity and space parity modes are not

> supported.

> 

> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>

> ---

>  drivers/serial/serial_zynq.c | 64 ++++++++++++++++++++++++++++++++++++++++++++

>  1 file changed, 64 insertions(+)

> 

> diff --git a/drivers/serial/serial_zynq.c b/drivers/serial/serial_zynq.c

> index 799d524..a644e78 100644

> --- a/drivers/serial/serial_zynq.c

> +++ b/drivers/serial/serial_zynq.c

> @@ -28,7 +28,17 @@

>  #define ZYNQ_UART_CR_TXRST	BIT(1) /* TX logic reset */

>  #define ZYNQ_UART_CR_RXRST	BIT(0) /* RX logic reset */

>  

> +#define ZYNQ_UART_MR_STOPMODE_2_BIT	0x00000080  /* 2 stop bits */

> +#define ZYNQ_UART_MR_STOPMODE_1_5_BIT	0x00000040  /* 1.5 stop bits */

> +#define ZYNQ_UART_MR_STOPMODE_1_BIT	0x00000000  /* 1 stop bit */

> +

>  #define ZYNQ_UART_MR_PARITY_NONE	0x00000020  /* No parity mode */

> +#define ZYNQ_UART_MR_PARITY_ODD		0x00000008  /* Odd parity mode */

> +#define ZYNQ_UART_MR_PARITY_EVEN	0x00000000  /* Even parity mode */

> +

> +#define ZYNQ_UART_MR_CHARLEN_6_BIT	0x00000006  /* 6 bits data */

> +#define ZYNQ_UART_MR_CHARLEN_7_BIT	0x00000004  /* 7 bits data */

> +#define ZYNQ_UART_MR_CHARLEN_8_BIT	0x00000000  /* 8 bits data */

>  

>  struct uart_zynq {

>  	u32 control; /* 0x0 - Control Register [8:0] */

> @@ -137,6 +147,59 @@ static int zynq_serial_setbrg(struct udevice *dev, int baudrate)

>  	return 0;

>  }

>  

> +static int zynq_serial_setconfig(struct udevice *dev, uint serial_config)

> +{

> +	struct zynq_uart_plat *plat = dev_get_plat(dev);

> +	struct uart_zynq *regs = plat->regs;

> +	u32 val = 0;

> +

> +	switch (SERIAL_GET_BITS(serial_config)) {

> +	case SERIAL_6_BITS:

> +		val |= ZYNQ_UART_MR_CHARLEN_6_BIT;

> +		break;

> +	case SERIAL_7_BITS:

> +		val |= ZYNQ_UART_MR_CHARLEN_7_BIT;

> +		break;

> +	case SERIAL_8_BITS:

> +		val |= ZYNQ_UART_MR_CHARLEN_8_BIT;

> +		break;

> +	default:

> +		return -ENOTSUPP; /* not supported in driver */

> +	}

> +

> +	switch (SERIAL_GET_STOP(serial_config)) {

> +	case SERIAL_ONE_STOP:

> +		val |= ZYNQ_UART_MR_STOPMODE_1_BIT;

> +		break;

> +	case SERIAL_ONE_HALF_STOP:

> +		val |= ZYNQ_UART_MR_STOPMODE_1_5_BIT;

> +		break;

> +	case SERIAL_TWO_STOP:

> +		val |= ZYNQ_UART_MR_STOPMODE_2_BIT;

> +		break;

> +	default:

> +		return -ENOTSUPP; /* not supported in driver */

> +	}

> +

> +	switch (SERIAL_GET_PARITY(serial_config)) {

> +	case SERIAL_PAR_NONE:

> +		val |= ZYNQ_UART_MR_PARITY_NONE;

> +		break;

> +	case SERIAL_PAR_ODD:

> +		val |= ZYNQ_UART_MR_PARITY_ODD;

> +		break;

> +	case SERIAL_PAR_EVEN:

> +		val |= ZYNQ_UART_MR_PARITY_EVEN;

> +		break;

> +	default:

> +		return -ENOTSUPP; /* not supported in driver */

> +	}

> +

> +	writel(val, &regs->mode);

> +

> +	return 0;

> +}

> +

>  static int zynq_serial_probe(struct udevice *dev)

>  {

>  	struct zynq_uart_plat *plat = dev_get_plat(dev);

> @@ -198,6 +261,7 @@ static const struct dm_serial_ops zynq_serial_ops = {

>  	.pending = zynq_serial_pending,

>  	.getc = zynq_serial_getc,

>  	.setbrg = zynq_serial_setbrg,

> +	.setconfig = zynq_serial_setconfig,

>  };

>  

>  static const struct udevice_id zynq_serial_ids[] = {

> 


I am just curious how you have tested it because only hook is in
test/dm/serial.c and I can't see no way how to change this setting via
u-boot command line.
That being said I see that this change adds 184 bytes which is quite a
lot especially for SPL on zynqmp. That's why would like to know how this
feature should be used. If make sense for example to limit it to only
full U-Boot.

Thanks,
Michal
Kunihiko Hayashi June 23, 2021, 10:52 a.m. UTC | #2 
Hi Michal,

On 2021/06/22 21:44, Michal Simek wrote:
> Hi,

> 

> On 6/22/21 6:24 AM, Kunihiko Hayashi wrote:

>> This adds serial parameters that include stop bit mode, parity mode,

>> and character length. Mark parity and space parity modes are not

>> supported.

>>

>> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>

>> ---

>>   drivers/serial/serial_zynq.c | 64 ++++++++++++++++++++++++++++++++++++++++++++

>>   1 file changed, 64 insertions(+)


[snip]

> I am just curious how you have tested it because only hook is in

> test/dm/serial.c and I can't see no way how to change this setting via

> u-boot command line.


I was misunderstanding.

Surely there is no way to execute .setconfig function, and
neither command line nor devicetree actually affects the serial mode.
The mode just inherits that of the previous firmware.

> That being said I see that this change adds 184 bytes which is quite a

> lot especially for SPL on zynqmp. That's why would like to know how this

> feature should be used. If make sense for example to limit it to only

> full U-Boot.


I see. I didn't think enough about the size limit of SPL.
Anyway, I withdraw this patch.

Thank you,

---
Best Regards
Kunihiko Hayashi
Michal Simek June 23, 2021, 11:15 a.m. UTC | #3 
Hi Kunihiko,

On 6/23/21 12:52 PM, Kunihiko Hayashi wrote:
> Hi Michal,

> 

> On 2021/06/22 21:44, Michal Simek wrote:

>> Hi,

>>

>> On 6/22/21 6:24 AM, Kunihiko Hayashi wrote:

>>> This adds serial parameters that include stop bit mode, parity mode,

>>> and character length. Mark parity and space parity modes are not

>>> supported.

>>>

>>> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>

>>> ---

>>>   drivers/serial/serial_zynq.c | 64

>>> ++++++++++++++++++++++++++++++++++++++++++++

>>>   1 file changed, 64 insertions(+)

> 

> [snip]

> 

>> I am just curious how you have tested it because only hook is in

>> test/dm/serial.c and I can't see no way how to change this setting via

>> u-boot command line.

> 

> I was misunderstanding.

> 

> Surely there is no way to execute .setconfig function, and

> neither command line nor devicetree actually affects the serial mode.

> The mode just inherits that of the previous firmware.

> 

>> That being said I see that this change adds 184 bytes which is quite a

>> lot especially for SPL on zynqmp. That's why would like to know how this

>> feature should be used. If make sense for example to limit it to only

>> full U-Boot.

> 

> I see. I didn't think enough about the size limit of SPL.

> Anyway, I withdraw this patch.


Up2you. Maybe someone will add any support for calling these functions.
For me it is fine to add it to full U-Boot but not to SPL for DM testing.

Thanks,
Michal
Kunihiko Hayashi June 24, 2021, 9:41 a.m. UTC | #4 
Hi Michal,

On 2021/06/23 20:15, Michal Simek wrote:
> Hi Kunihiko,

> 

> On 6/23/21 12:52 PM, Kunihiko Hayashi wrote:

>> Hi Michal,

>>

>> On 2021/06/22 21:44, Michal Simek wrote:

>>> Hi,

>>>

>>> On 6/22/21 6:24 AM, Kunihiko Hayashi wrote:

>>>> This adds serial parameters that include stop bit mode, parity mode,

>>>> and character length. Mark parity and space parity modes are not

>>>> supported.

>>>>

>>>> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>

>>>> ---

>>>>    drivers/serial/serial_zynq.c | 64

>>>> ++++++++++++++++++++++++++++++++++++++++++++

>>>>    1 file changed, 64 insertions(+)

>>

>> [snip]

>>

>>> I am just curious how you have tested it because only hook is in

>>> test/dm/serial.c and I can't see no way how to change this setting via

>>> u-boot command line.

>>

>> I was misunderstanding.

>>

>> Surely there is no way to execute .setconfig function, and

>> neither command line nor devicetree actually affects the serial mode.

>> The mode just inherits that of the previous firmware.

>>

>>> That being said I see that this change adds 184 bytes which is quite a

>>> lot especially for SPL on zynqmp. That's why would like to know how this

>>> feature should be used. If make sense for example to limit it to only

>>> full U-Boot.

>>

>> I see. I didn't think enough about the size limit of SPL.

>> Anyway, I withdraw this patch.

> 

> Up2you. Maybe someone will add any support for calling these functions.

> For me it is fine to add it to full U-Boot but not to SPL for DM testing.


Ok, I expect to such support.
I'll modify it to add the function when CONFIG_SPL_BUILD isn't defined
if there is no concern, and resend it.

Thank you,

---
Best Regards
Kunihiko Hayashi
diff mbox series

Patch

diff --git a/drivers/serial/serial_zynq.c b/drivers/serial/serial_zynq.c
index 799d524..a644e78 100644
--- a/drivers/serial/serial_zynq.c
+++ b/drivers/serial/serial_zynq.c
@@ -28,7 +28,17 @@ 
 #define ZYNQ_UART_CR_TXRST	BIT(1) /* TX logic reset */
 #define ZYNQ_UART_CR_RXRST	BIT(0) /* RX logic reset */
 
+#define ZYNQ_UART_MR_STOPMODE_2_BIT	0x00000080  /* 2 stop bits */
+#define ZYNQ_UART_MR_STOPMODE_1_5_BIT	0x00000040  /* 1.5 stop bits */
+#define ZYNQ_UART_MR_STOPMODE_1_BIT	0x00000000  /* 1 stop bit */
+
 #define ZYNQ_UART_MR_PARITY_NONE	0x00000020  /* No parity mode */
+#define ZYNQ_UART_MR_PARITY_ODD		0x00000008  /* Odd parity mode */
+#define ZYNQ_UART_MR_PARITY_EVEN	0x00000000  /* Even parity mode */
+
+#define ZYNQ_UART_MR_CHARLEN_6_BIT	0x00000006  /* 6 bits data */
+#define ZYNQ_UART_MR_CHARLEN_7_BIT	0x00000004  /* 7 bits data */
+#define ZYNQ_UART_MR_CHARLEN_8_BIT	0x00000000  /* 8 bits data */
 
 struct uart_zynq {
 	u32 control; /* 0x0 - Control Register [8:0] */
@@ -137,6 +147,59 @@  static int zynq_serial_setbrg(struct udevice *dev, int baudrate)
 	return 0;
 }
 
+static int zynq_serial_setconfig(struct udevice *dev, uint serial_config)
+{
+	struct zynq_uart_plat *plat = dev_get_plat(dev);
+	struct uart_zynq *regs = plat->regs;
+	u32 val = 0;
+
+	switch (SERIAL_GET_BITS(serial_config)) {
+	case SERIAL_6_BITS:
+		val |= ZYNQ_UART_MR_CHARLEN_6_BIT;
+		break;
+	case SERIAL_7_BITS:
+		val |= ZYNQ_UART_MR_CHARLEN_7_BIT;
+		break;
+	case SERIAL_8_BITS:
+		val |= ZYNQ_UART_MR_CHARLEN_8_BIT;
+		break;
+	default:
+		return -ENOTSUPP; /* not supported in driver */
+	}
+
+	switch (SERIAL_GET_STOP(serial_config)) {
+	case SERIAL_ONE_STOP:
+		val |= ZYNQ_UART_MR_STOPMODE_1_BIT;
+		break;
+	case SERIAL_ONE_HALF_STOP:
+		val |= ZYNQ_UART_MR_STOPMODE_1_5_BIT;
+		break;
+	case SERIAL_TWO_STOP:
+		val |= ZYNQ_UART_MR_STOPMODE_2_BIT;
+		break;
+	default:
+		return -ENOTSUPP; /* not supported in driver */
+	}
+
+	switch (SERIAL_GET_PARITY(serial_config)) {
+	case SERIAL_PAR_NONE:
+		val |= ZYNQ_UART_MR_PARITY_NONE;
+		break;
+	case SERIAL_PAR_ODD:
+		val |= ZYNQ_UART_MR_PARITY_ODD;
+		break;
+	case SERIAL_PAR_EVEN:
+		val |= ZYNQ_UART_MR_PARITY_EVEN;
+		break;
+	default:
+		return -ENOTSUPP; /* not supported in driver */
+	}
+
+	writel(val, &regs->mode);
+
+	return 0;
+}
+
 static int zynq_serial_probe(struct udevice *dev)
 {
 	struct zynq_uart_plat *plat = dev_get_plat(dev);
@@ -198,6 +261,7 @@  static const struct dm_serial_ops zynq_serial_ops = {
 	.pending = zynq_serial_pending,
 	.getc = zynq_serial_getc,
 	.setbrg = zynq_serial_setbrg,
+	.setconfig = zynq_serial_setconfig,
 };
 
 static const struct udevice_id zynq_serial_ids[] = {