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 <[email protected]>
---
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.
v2 -> v3:
- Use ATMEL_US_CD instead of 65535
- Previously [PATCH 10]
drivers/tty/serial/atmel_serial.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index ab4a9dfae07d..ad7483230090 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:
@@ -2283,10 +2287,21 @@ static void atmel_set_termios(struct uart_port *port,
cd = uart_get_divisor(port, baud);
}
- if (cd > 65535) { /* BRGR is 16-bit, so switch to slower clock */
+ /*
+ * If the current value of the Clock Divisor surpasses the 16 bit
+ * ATMEL_US_CD mask and the IP is USART, switch to the Peripheral
+ * Clock implicitly divided by 8.
+ * If the IP is UART however, keep the highest possible value for
+ * the CD and avoid needless division of CD, since UART IP's do not
+ * support implicit division of the Peripheral Clock.
+ */
+ if (atmel_port->is_usart && cd > ATMEL_US_CD) {
cd /= 8;
mode |= ATMEL_US_USCLKS_MCK_DIV8;
+ } else {
+ cd &= ATMEL_US_CD;
}
+
quot = cd | fp << ATMEL_US_FP_OFFSET;
if (!(port->iso7816.flags & SER_ISO7816_ENABLED))
--
2.34.1
On Tue, 13 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 <[email protected]>
> ---
>
>
> 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.
>
>
>
> v2 -> v3:
> - Use ATMEL_US_CD instead of 65535
> - Previously [PATCH 10]
> @@ -2283,10 +2287,21 @@ static void atmel_set_termios(struct uart_port *port,
> cd = uart_get_divisor(port, baud);
> }
>
> - if (cd > 65535) { /* BRGR is 16-bit, so switch to slower clock */
> + /*
> + * If the current value of the Clock Divisor surpasses the 16 bit
> + * ATMEL_US_CD mask and the IP is USART, switch to the Peripheral
> + * Clock implicitly divided by 8.
> + * If the IP is UART however, keep the highest possible value for
> + * the CD and avoid needless division of CD, since UART IP's do not
> + * support implicit division of the Peripheral Clock.
> + */
> + if (atmel_port->is_usart && cd > ATMEL_US_CD) {
> cd /= 8;
> mode |= ATMEL_US_USCLKS_MCK_DIV8;
> + } else {
> + cd &= ATMEL_US_CD;
Now that I read the above comment with more thought, would:
cd = min(cd, ATMEL_US_CD);
be more appropriate here if "the highest possible value" is sought for?
--
i.
On 14.09.2022 14:01, Ilpo Järvinen wrote:
> On Tue, 13 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 <[email protected]>
>> ---
>>
>>
>> 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.
>>
>>
>>
>> v2 -> v3:
>> - Use ATMEL_US_CD instead of 65535
>> - Previously [PATCH 10]
>
>
>> @@ -2283,10 +2287,21 @@ static void atmel_set_termios(struct uart_port *port,
>> cd = uart_get_divisor(port, baud);
>> }
>>
>> - if (cd > 65535) { /* BRGR is 16-bit, so switch to slower clock */
>> + /*
>> + * If the current value of the Clock Divisor surpasses the 16 bit
>> + * ATMEL_US_CD mask and the IP is USART, switch to the Peripheral
>> + * Clock implicitly divided by 8.
>> + * If the IP is UART however, keep the highest possible value for
>> + * the CD and avoid needless division of CD, since UART IP's do not
>> + * support implicit division of the Peripheral Clock.
>> + */
>> + if (atmel_port->is_usart && cd > ATMEL_US_CD) {
>> cd /= 8;
>> mode |= ATMEL_US_USCLKS_MCK_DIV8;
>> + } else {
>> + cd &= ATMEL_US_CD;
>
> Now that I read the above comment with more thought, would:
> cd = min(cd, ATMEL_US_CD);
> be more appropriate here if "the highest possible value" is sought for?
>
>
> --
> i.
>
Yes, you are right, that would actually be the correct operation. I
somehow only thought of frequencies lower than ATMEL_US_CD when writing
that operation, although the commentary does say my intention. Thanks :).
Regards,
Sergiu