diff mbox series

[v2,10/13] tty: serial: atmel: Only divide Clock Divisor if the IP is USART

Message ID 20220906135511.144725-11-sergiu.moga@microchip.com (mailing list archive)
State Superseded
Headers show
Series Make atmel serial driver aware of GCLK | expand

Commit Message

Sergiu Moga Sept. 6, 2022, 1:55 p.m. UTC
Make sure that the driver only divides the clock divisor if the
IP handled at that point is USART, since UART IP's do not support
implicit peripheral clock division. Instead, in the case of UART,
go with the highest possible clock divisor.

Signed-off-by: Sergiu Moga <sergiu.moga@microchip.com>
---


v1 -> v2:
- Nothing, this patch was not here before and is mainly meant as both cleanup
and as a way to introduce a new field into struct atmel_uart_port that will be
used by the last patch to diferentiate between USART and UART regarding the
location of the Baudrate Clock Source bitmask.




 drivers/tty/serial/atmel_serial.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

Comments

Ilpo Järvinen Sept. 7, 2022, 9:23 a.m. UTC | #1
On Tue, 6 Sep 2022, Sergiu Moga wrote:

> Make sure that the driver only divides the clock divisor if the
> IP handled at that point is USART, since UART IP's do not support
> implicit peripheral clock division. Instead, in the case of UART,
> go with the highest possible clock divisor.
> 
> Signed-off-by: Sergiu Moga <sergiu.moga@microchip.com>
> ---
> 
> 
> v1 -> v2:
> - Nothing, this patch was not here before and is mainly meant as both cleanup
> and as a way to introduce a new field into struct atmel_uart_port that will be
> used by the last patch to diferentiate between USART and UART regarding the
> location of the Baudrate Clock Source bitmask.
> 
> 
> 
> 
>  drivers/tty/serial/atmel_serial.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index 7450d3853031..6aa01ca5489c 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -150,6 +150,7 @@ struct atmel_uart_port {
>  	u32			rts_low;
>  	bool			ms_irq_enabled;
>  	u32			rtor;	/* address of receiver timeout register if it exists */
> +	bool			is_usart;
>  	bool			has_frac_baudrate;
>  	bool			has_hw_timer;
>  	struct timer_list	uart_timer;
> @@ -1825,6 +1826,7 @@ static void atmel_get_ip_name(struct uart_port *port)
>  	 */
>  	atmel_port->has_frac_baudrate = false;
>  	atmel_port->has_hw_timer = false;
> +	atmel_port->is_usart = false;
>  
>  	if (name == new_uart) {
>  		dev_dbg(port->dev, "Uart with hw timer");
> @@ -1834,6 +1836,7 @@ static void atmel_get_ip_name(struct uart_port *port)
>  		dev_dbg(port->dev, "Usart\n");
>  		atmel_port->has_frac_baudrate = true;
>  		atmel_port->has_hw_timer = true;
> +		atmel_port->is_usart = true;
>  		atmel_port->rtor = ATMEL_US_RTOR;
>  		version = atmel_uart_readl(port, ATMEL_US_VERSION);
>  		switch (version) {
> @@ -1863,6 +1866,7 @@ static void atmel_get_ip_name(struct uart_port *port)
>  			dev_dbg(port->dev, "This version is usart\n");
>  			atmel_port->has_frac_baudrate = true;
>  			atmel_port->has_hw_timer = true;
> +			atmel_port->is_usart = true;
>  			atmel_port->rtor = ATMEL_US_RTOR;
>  			break;
>  		case 0x203:
> @@ -2282,10 +2286,17 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
>  		cd = uart_get_divisor(port, baud);
>  	}
>  
> -	if (cd > 65535) {	/* BRGR is 16-bit, so switch to slower clock */
> +	/*
> +	 * BRGR is 16-bit, so switch to slower clock.
> +	 * Otherwise, keep the highest possible value for the clock divisor.
> +	 */
> +	if (atmel_port->is_usart && cd > 65535) {

Should this be cd > ATMEL_US_CD ?

>  		cd /= 8;
>  		mode |= ATMEL_US_USCLKS_MCK_DIV8;
> +	} else {
> +		cd &= 65535;

ATMEL_US_CD?

>  	}
> +
>  	quot = cd | fp << ATMEL_US_FP_OFFSET;
>  
>  	if (!(port->iso7816.flags & SER_ISO7816_ENABLED))
>
Sergiu Moga Sept. 7, 2022, 9:34 a.m. UTC | #2
On 07.09.2022 12:23, Ilpo Järvinen wrote:
> On Tue, 6 Sep 2022, Sergiu Moga wrote:
> 
>> Make sure that the driver only divides the clock divisor if the
>> IP handled at that point is USART, since UART IP's do not support
>> implicit peripheral clock division. Instead, in the case of UART,
>> go with the highest possible clock divisor.
>>
>> Signed-off-by: Sergiu Moga <sergiu.moga@microchip.com>
>> ---
>>
>>
>> v1 -> v2:
>> - Nothing, this patch was not here before and is mainly meant as both cleanup
>> and as a way to introduce a new field into struct atmel_uart_port that will be
>> used by the last patch to diferentiate between USART and UART regarding the
>> location of the Baudrate Clock Source bitmask.
>>
>>
>>
>>
>>   drivers/tty/serial/atmel_serial.c | 13 ++++++++++++-
>>   1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
>> index 7450d3853031..6aa01ca5489c 100644
>> --- a/drivers/tty/serial/atmel_serial.c
>> +++ b/drivers/tty/serial/atmel_serial.c
>> @@ -150,6 +150,7 @@ struct atmel_uart_port {
>>        u32                     rts_low;
>>        bool                    ms_irq_enabled;
>>        u32                     rtor;   /* address of receiver timeout register if it exists */
>> +     bool                    is_usart;
>>        bool                    has_frac_baudrate;
>>        bool                    has_hw_timer;
>>        struct timer_list       uart_timer;
>> @@ -1825,6 +1826,7 @@ static void atmel_get_ip_name(struct uart_port *port)
>>         */
>>        atmel_port->has_frac_baudrate = false;
>>        atmel_port->has_hw_timer = false;
>> +     atmel_port->is_usart = false;
>>
>>        if (name == new_uart) {
>>                dev_dbg(port->dev, "Uart with hw timer");
>> @@ -1834,6 +1836,7 @@ static void atmel_get_ip_name(struct uart_port *port)
>>                dev_dbg(port->dev, "Usart\n");
>>                atmel_port->has_frac_baudrate = true;
>>                atmel_port->has_hw_timer = true;
>> +             atmel_port->is_usart = true;
>>                atmel_port->rtor = ATMEL_US_RTOR;
>>                version = atmel_uart_readl(port, ATMEL_US_VERSION);
>>                switch (version) {
>> @@ -1863,6 +1866,7 @@ static void atmel_get_ip_name(struct uart_port *port)
>>                        dev_dbg(port->dev, "This version is usart\n");
>>                        atmel_port->has_frac_baudrate = true;
>>                        atmel_port->has_hw_timer = true;
>> +                     atmel_port->is_usart = true;
>>                        atmel_port->rtor = ATMEL_US_RTOR;
>>                        break;
>>                case 0x203:
>> @@ -2282,10 +2286,17 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
>>                cd = uart_get_divisor(port, baud);
>>        }
>>
>> -     if (cd > 65535) {       /* BRGR is 16-bit, so switch to slower clock */
>> +     /*
>> +      * BRGR is 16-bit, so switch to slower clock.
>> +      * Otherwise, keep the highest possible value for the clock divisor.
>> +      */
>> +     if (atmel_port->is_usart && cd > 65535) {
> 
> Should this be cd > ATMEL_US_CD ?
> 
>>                cd /= 8;
>>                mode |= ATMEL_US_USCLKS_MCK_DIV8;
>> +     } else {
>> +             cd &= 65535;
> 
> ATMEL_US_CD?


Yes, it would seem that would be right. It did not cross my mind to 
check whether there already was something pre-defined or not since the 
code also previously used 65535.

> 
>>        }
>> +
>>        quot = cd | fp << ATMEL_US_FP_OFFSET;
>>
>>        if (!(port->iso7816.flags & SER_ISO7816_ENABLED))
>>
> 
> --
>   i.
> 


Thanks,
	Sergiu
diff mbox series

Patch

diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 7450d3853031..6aa01ca5489c 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -150,6 +150,7 @@  struct atmel_uart_port {
 	u32			rts_low;
 	bool			ms_irq_enabled;
 	u32			rtor;	/* address of receiver timeout register if it exists */
+	bool			is_usart;
 	bool			has_frac_baudrate;
 	bool			has_hw_timer;
 	struct timer_list	uart_timer;
@@ -1825,6 +1826,7 @@  static void atmel_get_ip_name(struct uart_port *port)
 	 */
 	atmel_port->has_frac_baudrate = false;
 	atmel_port->has_hw_timer = false;
+	atmel_port->is_usart = false;
 
 	if (name == new_uart) {
 		dev_dbg(port->dev, "Uart with hw timer");
@@ -1834,6 +1836,7 @@  static void atmel_get_ip_name(struct uart_port *port)
 		dev_dbg(port->dev, "Usart\n");
 		atmel_port->has_frac_baudrate = true;
 		atmel_port->has_hw_timer = true;
+		atmel_port->is_usart = true;
 		atmel_port->rtor = ATMEL_US_RTOR;
 		version = atmel_uart_readl(port, ATMEL_US_VERSION);
 		switch (version) {
@@ -1863,6 +1866,7 @@  static void atmel_get_ip_name(struct uart_port *port)
 			dev_dbg(port->dev, "This version is usart\n");
 			atmel_port->has_frac_baudrate = true;
 			atmel_port->has_hw_timer = true;
+			atmel_port->is_usart = true;
 			atmel_port->rtor = ATMEL_US_RTOR;
 			break;
 		case 0x203:
@@ -2282,10 +2286,17 @@  static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
 		cd = uart_get_divisor(port, baud);
 	}
 
-	if (cd > 65535) {	/* BRGR is 16-bit, so switch to slower clock */
+	/*
+	 * BRGR is 16-bit, so switch to slower clock.
+	 * Otherwise, keep the highest possible value for the clock divisor.
+	 */
+	if (atmel_port->is_usart && cd > 65535) {
 		cd /= 8;
 		mode |= ATMEL_US_USCLKS_MCK_DIV8;
+	} else {
+		cd &= 65535;
 	}
+
 	quot = cd | fp << ATMEL_US_FP_OFFSET;
 
 	if (!(port->iso7816.flags & SER_ISO7816_ENABLED))