mbox series

[v2,00/13] Make atmel serial driver aware of GCLK

Message ID 20220906135511.144725-1-sergiu.moga@microchip.com
Headers show
Series Make atmel serial driver aware of GCLK | expand

Message

Sergiu Moga Sept. 6, 2022, 1:54 p.m. UTC
This series of patches introduces the GCLK as a potential clock source for
the baudrate generator of UART on sama5d2 SoCs. Unlike the serial mode of
the USART offered by FLEXCOM, the UART does not provide a fractional part
that can be added to the clock divisor to obtain a more accurate result,
which greatly decreases the flexibility available for producing a higher
variety of baudrates. Now, with the last patch of the series, the driver
will check for a GCLK in the DT. If provided, whenever `atmel_set_termios`
is called, unless there is a fractional part, the driver will compare the
error rate between the desired baudrate and the actual baudrate obtained
through each of the available clock sources and will choose the clock source
with the lowest error rate. While at it, convert the DT binding
for UART/USART to json-schema, update the FLEXCOM binding to reference the
new UART/USART binding (while differentiating between the SPI of USART and the
SPI of FLEXCOM) and do some small DT related fixups.

The DT bindings related patches of this patch series depend on this patch
series converting atmel-flexcom bindings to json-schema:
https://lore.kernel.org/all/20220708115619.254073-1-kavyasree.kotagiri@microchip.com/

v1 -> v2:
- [PATCH 3] dt-bindings: mfd: atmel,sama5d2-flexcom: Add SPI child node ref
    binding:
	- use full schema paths

- [PATCH 5] dt-bindings: serial: atmel,at91-usart: convert to json-schema
	- only do what the commit says, split the addition of other compatibles
	(PATCH 6) and properties (PATCH 13) in other patches
	- remove unnecessary "|"'s
	- mention header in `atmel,usart-mode`'s description
	- place `if:` under `allOf:`
	- respect order of spi0's DT properties: compatible, then reg then the
	reset of properties

- two new baudrate clock source related patches:
  [PATCH 9] tty: serial: atmel: Add definition for GCLK as baudrate source clock
			+
  [PATCH 10] tty: serial: atmel: Define BRSRCCK bitmask of UART IP's Mode
    Register:
	- v1's bitfield definition of GCLK was wrong, so add two more patches:
		- one for the definition of GCLK of USART IP's
		- one for the definition of BRSRCCK bitmask and its bitfields
		for UART IP's

- a new cleanup related patch that introduces a new struct atmel_uart_port field:
  [PATCH 11] tty: serial: atmel: Only divide Clock Divisor if the IP is USART:
  	- this ensures a division by 8 which is unnecessary and unappliable to
	UART IP's is only done for USART IP's

- four new patches regarding DT fixes and a SPI binding update that I came
upon:
  [PATCH 1] spi: dt-bindings: atmel,at91rm9200-spi: Add DMA related properties
  [PATCH 2] ARM: dts: at91: sama7g5: Swap rx and tx for spi11
  [PATCH 4] ARM: dts: at91: sam9x60ek: Add DBGU compatibles to uart1
  [PATCH 6] dt-bindings: serial: atmel,at91-usart: Highlight SAM9X60 incremental

- [PATCH 12] tty: serial: atmel: Make the driver aware of the existence of GCLK
	- take into account the different placement of the baudrate clock source
	into the IP's Mode Register (USART vs UART)
	- don't check for atmel_port->gclk != NULL
	- use clk_round_rate instead of clk_set_rate + clk_get_rate
	- remove clk_disable_unprepare from the end of the probe method

Sergiu Moga (13):
  spi: dt-bindings: atmel,at91rm9200-spi: Add DMA related properties
  ARM: dts: at91: sama7g5: Swap rx and tx for spi11
  dt-bindings: mfd: atmel,sama5d2-flexcom: Add SPI child node ref
    binding
  ARM: dts: at91: sam9x60ek: Add DBGU compatibles to uart1
  dt-bindings: serial: atmel,at91-usart: convert to json-schema
  dt-bindings: serial: atmel,at91-usart: Add SAM9260 compatibles to
    SAM9x60
  dt-bindings: mfd: atmel,sama5d2-flexcom: Add USART child node ref
    binding
  tty: serial: atmel: Define GCLK as USART baudrate source clock
  tty: serial: atmel: Define BRSRCCK bitmask of UART IP's Mode Register
  tty: serial: atmel: Only divide Clock Divisor if the IP is USART
  clk: at91: sama5d2: Add Generic Clocks for UART/USART
  tty: serial: atmel: Make the driver aware of the existence of GCLK
  dt-bindings: serial: atmel,at91-usart: Add gclk as a possible USART
    clock

 .../bindings/mfd/atmel,sama5d2-flexcom.yaml   |  19 +-
 .../devicetree/bindings/mfd/atmel-usart.txt   |  98 ---------
 .../bindings/serial/atmel,at91-usart.yaml     | 191 ++++++++++++++++++
 .../bindings/spi/atmel,at91rm9200-spi.yaml    |  10 +
 arch/arm/boot/dts/at91-sam9x60ek.dts          |   2 +-
 arch/arm/boot/dts/sama7g5.dtsi                |   6 +-
 drivers/clk/at91/sama5d2.c                    |  10 +
 drivers/tty/serial/atmel_serial.c             |  65 +++++-
 drivers/tty/serial/atmel_serial.h             |   4 +
 9 files changed, 295 insertions(+), 110 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/mfd/atmel-usart.txt
 create mode 100644 Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml

Comments

Mark Brown Sept. 6, 2022, 3:12 p.m. UTC | #1
On Tue, Sep 06, 2022 at 04:55:00PM +0300, Sergiu Moga wrote:
> The DT nodes of the SPI IP's may contain DMA related properties so
> make sure that the binding is able to properly validate those as
> well by making it aware of these optional properties.

Acked-by: Mark Brown <broonie@kernel.org>

though it looks like perhaps this could just go separately?
Rob Herring Sept. 6, 2022, 9:41 p.m. UTC | #2
On Tue, 06 Sep 2022 16:55:00 +0300, Sergiu Moga wrote:
> The DT nodes of the SPI IP's may contain DMA related properties so
> make sure that the binding is able to properly validate those as
> well by making it aware of these optional properties.
> 
> Signed-off-by: Sergiu Moga <sergiu.moga@microchip.com>
> ---
> 
> 
> v1 -> v2:
> - Nothing, this patch was not here before
> 
> 
>  .../devicetree/bindings/spi/atmel,at91rm9200-spi.yaml  | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 

Running 'make dtbs_check' with the schema in this patch gives the
following warnings. Consider if they are expected or the schema is
incorrect. These may not be new warnings.

Note that it is not yet a requirement to have 0 warnings for dtbs_check.
This will change in the future.

Full log is available here: https://patchwork.ozlabs.org/patch/


spi@400: dma-names:0: 'tx' was expected
	arch/arm/boot/dts/at91-sama7g5ek.dtb

spi@400: dma-names:1: 'rx' was expected
	arch/arm/boot/dts/at91-sama7g5ek.dtb

spi@400: Unevaluated properties are not allowed ('dma-names' was unexpected)
	arch/arm/boot/dts/at91-sama7g5ek.dtb
Ilpo Järvinen Sept. 7, 2022, 9:21 a.m. UTC | #3
On Tue, 6 Sep 2022, Sergiu Moga wrote:

> Define the bit that represents the choice of having GCLK as a baudrate
> source clock inside the USCLKS bitmask of the Mode Register of
> USART IP's.
> 
> Signed-off-by: Sergiu Moga <sergiu.moga@microchip.com>
> ---
> 
> 
> v1 -> v2:
> - Nothing, this patch was not here before
> 
> 
> 
>  drivers/tty/serial/atmel_serial.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/tty/serial/atmel_serial.h b/drivers/tty/serial/atmel_serial.h
> index 0d8a0f9cc5c3..70d0611e56fd 100644
> --- a/drivers/tty/serial/atmel_serial.h
> +++ b/drivers/tty/serial/atmel_serial.h
> @@ -49,6 +49,7 @@
>  #define	ATMEL_US_USCLKS		GENMASK(5, 4)	/* Clock Selection */
>  #define		ATMEL_US_USCLKS_MCK		(0 <<  4)
>  #define		ATMEL_US_USCLKS_MCK_DIV8	(1 <<  4)
> +#define		ATMEL_US_USCLKS_GCLK		(2 <<  4)

This would be FIELD_PREP(ATMEL_US_USCLKS, 2) from linux/bitfield.h.

They should all be converted to use FIELD_PREP(), IMHO (in a separate 
patch).

>  #define		ATMEL_US_USCLKS_SCK		(3 <<  4)
>  #define	ATMEL_US_CHRL		GENMASK(7, 6)	/* Character Length */
>  #define		ATMEL_US_CHRL_5			(0 <<  6)
>
Ilpo Järvinen Sept. 7, 2022, 9:24 a.m. UTC | #4
On Tue, 6 Sep 2022, Sergiu Moga wrote:

> Add definitions for the Baud Rate Source Clock bitmask of the
> Mode Register of UART IP's and its bitfields.
> 
> Signed-off-by: Sergiu Moga <sergiu.moga@microchip.com>
> ---
> 
> 
> 
> v1 -> v2:
> - Nothing, this patch was not here before
> 
> 
> 
>  drivers/tty/serial/atmel_serial.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/tty/serial/atmel_serial.h b/drivers/tty/serial/atmel_serial.h
> index 70d0611e56fd..ed64035ba6c3 100644
> --- a/drivers/tty/serial/atmel_serial.h
> +++ b/drivers/tty/serial/atmel_serial.h
> @@ -68,6 +68,9 @@
>  #define		ATMEL_US_NBSTOP_1		(0 << 12)
>  #define		ATMEL_US_NBSTOP_1_5		(1 << 12)
>  #define		ATMEL_US_NBSTOP_2		(2 << 12)
> +#define	ATMEL_UA_BRSRCCK	GENMASK(13, 12)	/* Clock Selection for UART */
> +#define		ATMEL_UA_BRSRCCK_PERIPH_CLK	(0 << 12)
> +#define		ATMEL_UA_BRSRCCK_GCLK		(1 << 12)

FIELD_PREP(ATMEL_UA_BRSRCCK, ...)

>  #define	ATMEL_US_CHMODE		GENMASK(15, 14)	/* Channel Mode */
>  #define		ATMEL_US_CHMODE_NORMAL		(0 << 14)
>  #define		ATMEL_US_CHMODE_ECHO		(1 << 14)
>
Ilpo Järvinen Sept. 7, 2022, 9:36 a.m. UTC | #5
On Tue, 6 Sep 2022, Sergiu Moga wrote:

> Previously, the atmel serial driver did not take into account the
> possibility of using the more customizable generic clock as its
> baudrate generator. Unless there is a Fractional Part available to
> increase accuracy, there is a high chance that we may be able to
> generate a baudrate closer to the desired one by using the GCLK as the
> clock source. Now, depending on the error rate between
> the desired baudrate and the actual baudrate, the serial driver will
> fallback on the generic clock. The generic clock must be provided
> in the DT node of the serial that may need a more flexible clock source.
> 
> Signed-off-by: Sergiu Moga <sergiu.moga@microchip.com>
> ---
> 
> 
> 
> v1 -> v2:
> - take into account the different placement of the baudrate clock source
> into the IP's Mode Register (USART vs UART)
> - don't check for atmel_port->gclk != NULL
> - use clk_round_rate instead of clk_set_rate + clk_get_rate
> - remove clk_disable_unprepare from the end of the probe method
> 
> 
> 
>  drivers/tty/serial/atmel_serial.c | 52 ++++++++++++++++++++++++++++++-
>  1 file changed, 51 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index 6aa01ca5489c..b2b6fd6ea2a5 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -15,6 +15,7 @@
>  #include <linux/init.h>
>  #include <linux/serial.h>
>  #include <linux/clk.h>
> +#include <linux/clk-provider.h>
>  #include <linux/console.h>
>  #include <linux/sysrq.h>
>  #include <linux/tty_flip.h>
> @@ -77,6 +78,8 @@ static void atmel_stop_rx(struct uart_port *port);
>  #endif
>  
>  #define ATMEL_ISR_PASS_LIMIT	256
> +#define ERROR_RATE(desired_value, actual_value) \
> +	((int)(100 - ((desired_value) * 100) / (actual_value)))

Make a function instead.

Is percent accurate enough or would you perhaps want something slightly 
more accurate?

Given you've abs() at the caller side, the error rate could be 
underestimated, is underestimating OK?
Sergiu Moga Sept. 7, 2022, 11:11 a.m. UTC | #6
On 07.09.2022 12:36, Ilpo Järvinen wrote:
> On Tue, 6 Sep 2022, Sergiu Moga wrote:
> 
>> Previously, the atmel serial driver did not take into account the
>> possibility of using the more customizable generic clock as its
>> baudrate generator. Unless there is a Fractional Part available to
>> increase accuracy, there is a high chance that we may be able to
>> generate a baudrate closer to the desired one by using the GCLK as the
>> clock source. Now, depending on the error rate between
>> the desired baudrate and the actual baudrate, the serial driver will
>> fallback on the generic clock. The generic clock must be provided
>> in the DT node of the serial that may need a more flexible clock source.
>>
>> Signed-off-by: Sergiu Moga <sergiu.moga@microchip.com>
>> ---
>>
>>
>>
>> v1 -> v2:
>> - take into account the different placement of the baudrate clock source
>> into the IP's Mode Register (USART vs UART)
>> - don't check for atmel_port->gclk != NULL
>> - use clk_round_rate instead of clk_set_rate + clk_get_rate
>> - remove clk_disable_unprepare from the end of the probe method
>>
>>
>>
>>   drivers/tty/serial/atmel_serial.c | 52 ++++++++++++++++++++++++++++++-
>>   1 file changed, 51 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
>> index 6aa01ca5489c..b2b6fd6ea2a5 100644
>> --- a/drivers/tty/serial/atmel_serial.c
>> +++ b/drivers/tty/serial/atmel_serial.c
>> @@ -15,6 +15,7 @@
>>   #include <linux/init.h>
>>   #include <linux/serial.h>
>>   #include <linux/clk.h>
>> +#include <linux/clk-provider.h>
>>   #include <linux/console.h>
>>   #include <linux/sysrq.h>
>>   #include <linux/tty_flip.h>
>> @@ -77,6 +78,8 @@ static void atmel_stop_rx(struct uart_port *port);
>>   #endif
>>
>>   #define ATMEL_ISR_PASS_LIMIT 256
>> +#define ERROR_RATE(desired_value, actual_value) \
>> +     ((int)(100 - ((desired_value) * 100) / (actual_value)))
> 
> Make a function instead.
> 


Alright, I see the point of making it an inline function instead.


> Is percent accurate enough or would you perhaps want something slightly
> more accurate?
> 


It is accurate enough for the all the baudrates I have tested. It 
usually taps into the GCLK whenever high baudrates such as 921600 are 
used. For 115200 for example, the error rate was slightly better in the 
case of the peripheral clock and it acted accordingly, choosing the 
latter as its baudrate source clock. I do not think that a higher 
accuracy than this would be needed though. Say that using percent 
accuracy yields that the error rates are equal, but the gclk would have 
been better in this case by, say, a few 10 ^ -4, but the code logic does 
not see it so it proceeds using the peripheral clock. In that case, the 
error rate of the peripheral clock would still be low enough relative to 
the desired baudrate for the communication to function properly.

The higher the baudrate, the lower the error rate must be in order for 
things to go smoothly. For example, for a baudrate of 57600 I noticed 
that even an error rate as big as 6% is still enough for the 
communication to work properly, while in the case of 921600 anything 
bigger than 2% and things do not go smoothly anymore. So I guess that it 
would be safe to say that, unless you go for baudrates as high as tens 
of millions, things should work well with just percent accuracy. A 
higher accuracy always definetely helps, but I believe it is not needed 
in this case.


> Given you've abs() at the caller side, the error rate could be
> underestimated, is underestimating OK?
> 


Yes, this should be fine. While (both empirically and after looking 
stuff up) I noticed that in the case of negative error rates, their 
absolute value needs to be smaller than the one of positive error rates, 
it must be so by a very small margin that is negligible when estimating 
through percent accuracy.


> --
>   i.
> 
>>   struct atmel_dma_buffer {
>>        unsigned char   *buf;
>> @@ -110,6 +113,7 @@ struct atmel_uart_char {
>>   struct atmel_uart_port {
>>        struct uart_port        uart;           /* uart */
>>        struct clk              *clk;           /* uart clock */
>> +     struct clk              *gclk;          /* uart generic clock */
>>        int                     may_wakeup;     /* cached value of device_may_wakeup for times we need to disable it */
>>        u32                     backup_imr;     /* IMR saved during suspend */
>>        int                     break_active;   /* break being received */
>> @@ -2117,6 +2121,8 @@ static void atmel_serial_pm(struct uart_port *port, unsigned int state,
>>                 * This is called on uart_close() or a suspend event.
>>                 */
>>                clk_disable_unprepare(atmel_port->clk);
>> +             if (__clk_is_enabled(atmel_port->gclk))
>> +                     clk_disable_unprepare(atmel_port->gclk);
>>                break;
>>        default:
>>                dev_err(port->dev, "atmel_serial: unknown pm %d\n", state);
>> @@ -2131,7 +2137,8 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
>>   {
>>        struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
>>        unsigned long flags;
>> -     unsigned int old_mode, mode, imr, quot, baud, div, cd, fp = 0;
>> +     unsigned int old_mode, mode, imr, quot, div, cd, fp = 0;
>> +     unsigned int baud, actual_baud, gclk_rate;
>>
>>        /* save the current mode register */
>>        mode = old_mode = atmel_uart_readl(port, ATMEL_US_MR);
>> @@ -2297,6 +2304,43 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
>>                cd &= 65535;
>>        }
>>
>> +     /*
>> +      * If there is no Fractional Part, there is a high chance that
>> +      * we may be able to generate a baudrate closer to the desired one
>> +      * if we use the GCLK as the clock source driving the baudrate
>> +      * generator.
>> +      */
>> +     if (!atmel_port->has_frac_baudrate) {
>> +             if (__clk_is_enabled(atmel_port->gclk))
>> +                     clk_disable_unprepare(atmel_port->gclk);
>> +             gclk_rate = clk_round_rate(atmel_port->gclk, 16 * baud);
>> +             actual_baud = clk_get_rate(atmel_port->clk) / (16 * cd);
>> +             if (gclk_rate && abs(ERROR_RATE(baud, actual_baud)) >
>> +                 abs(ERROR_RATE(baud, gclk_rate / 16))) {
>> +                     clk_set_rate(atmel_port->gclk, 16 * baud);
>> +                     if (!clk_prepare_enable(atmel_port->gclk)) {
>> +                             if (atmel_port->is_usart) {
>> +                                     mode &= ~ATMEL_US_USCLKS;
>> +                                     mode |= ATMEL_US_USCLKS_GCLK;
>> +                             } else {
>> +                                     mode &= ~ATMEL_UA_BRSRCCK;
>> +                                     mode |= ATMEL_UA_BRSRCCK_GCLK;
>> +                             }
>> +
>> +                             /*
>> +                              * Set the Clock Divisor for GCLK to 1.
>> +                              * Since we were able to generate the smallest
>> +                              * multiple of the desired baudrate times 16,
>> +                              * then we surely can generate a bigger multiple
>> +                              * with the exact error rate for an equally increased
>> +                              * CD. Thus no need to take into account
>> +                              * a higher value for CD.
>> +                              */
>> +                             cd = 1;
>> +                     }
>> +             }
>> +     }
>> +
>>        quot = cd | fp << ATMEL_US_FP_OFFSET;
>>
>>        if (!(port->iso7816.flags & SER_ISO7816_ENABLED))
>> @@ -2892,6 +2936,12 @@ static int atmel_serial_probe(struct platform_device *pdev)
>>        if (ret)
>>                goto err;
>>
>> +     atmel_port->gclk = devm_clk_get_optional(&pdev->dev, "gclk");
>> +     if (IS_ERR(atmel_port->gclk)) {
>> +             ret = PTR_ERR(atmel_port->gclk);
>> +             goto err;
>> +     }
>> +
>>        ret = atmel_init_port(atmel_port, pdev);
>>        if (ret)
>>                goto err_clk_disable_unprepare;
>>
Krzysztof Kozlowski Sept. 8, 2022, 12:23 p.m. UTC | #7
On 06/09/2022 15:55, Sergiu Moga wrote:
> The DT nodes of the SPI IP's may contain DMA related properties so
> make sure that the binding is able to properly validate those as
> well by making it aware of these optional properties.
> 
> Signed-off-by: Sergiu Moga <sergiu.moga@microchip.com>


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof
Krzysztof Kozlowski Sept. 8, 2022, 12:35 p.m. UTC | #8
On 06/09/2022 15:55, Sergiu Moga wrote:
> The Devicetree nodes for FLEXCOM's USART can also have an alternative
> clock source for the baudrate generator (other than the peripheral
> clock), namely the Generick Clock. Thus make the binding aware of
> this potential clock that someone may place in the clock related
> properties of the USART node.

Last sentence is confusing - what is the potential? Just skip it.

> 
> Signed-off-by: Sergiu Moga <sergiu.moga@microchip.com>
> ---
> 
> 
> 
> v1 -> v2:
> - Nothing, this patch was not here before

You have confusing order of patches. Bindings mixed with DTS mixed with
drivers. Keep things ordered.
1. DTS changes needed for aligning to schema.
2. all bindings
3. rest

Best regards,
Krzysztof
Sergiu Moga Sept. 8, 2022, 3:33 p.m. UTC | #9
On 08.09.2022 15:35, Krzysztof Kozlowski wrote:
> On 06/09/2022 15:55, Sergiu Moga wrote:
>> The Devicetree nodes for FLEXCOM's USART can also have an alternative
>> clock source for the baudrate generator (other than the peripheral
>> clock), namely the Generick Clock. Thus make the binding aware of
>> this potential clock that someone may place in the clock related
>> properties of the USART node.
> 
> Last sentence is confusing - what is the potential? Just skip it.
> 


I am sorry, I meant to say "possible". No idea how I ended up writing 
"potential". Guess I will just skip any adjectives entirely.


>>
>> Signed-off-by: Sergiu Moga <sergiu.moga@microchip.com>
>> ---
>>
>>
>>
>> v1 -> v2:
>> - Nothing, this patch was not here before
> 
> You have confusing order of patches. Bindings mixed with DTS mixed with
> drivers. Keep things ordered.
> 1. DTS changes needed for aligning to schema.
> 2. all bindings
> 3. rest
> 


Alright, it makes sense, will do so. I thought it would have looked 
better if I were to add the gclk in the schema after adding it in the 
drivers.


Other than that I hope I got the example[1] you have previously given me 
right.


[1] 
https://elixir.bootlin.com/linux/v6.0-rc4/source/Documentation/devicetree/bindings/example-schema.yaml#L91


> Best regards,
> Krzysztof


Thanks,
	Sergiu
Claudiu Beznea Sept. 12, 2022, 8:06 a.m. UTC | #10
On 06.09.2022 16:55, Sergiu Moga wrote:
> Previously, the atmel serial driver did not take into account the
> possibility of using the more customizable generic clock as its
> baudrate generator. Unless there is a Fractional Part available to
> increase accuracy, there is a high chance that we may be able to
> generate a baudrate closer to the desired one by using the GCLK as the
> clock source. Now, depending on the error rate between
> the desired baudrate and the actual baudrate, the serial driver will
> fallback on the generic clock. The generic clock must be provided
> in the DT node of the serial that may need a more flexible clock source.
> 
> Signed-off-by: Sergiu Moga <sergiu.moga@microchip.com>
> ---
> 
> 
> 
> v1 -> v2:
> - take into account the different placement of the baudrate clock source
> into the IP's Mode Register (USART vs UART)
> - don't check for atmel_port->gclk != NULL
> - use clk_round_rate instead of clk_set_rate + clk_get_rate
> - remove clk_disable_unprepare from the end of the probe method
> 
> 
> 
>  drivers/tty/serial/atmel_serial.c | 52 ++++++++++++++++++++++++++++++-
>  1 file changed, 51 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index 6aa01ca5489c..b2b6fd6ea2a5 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -15,6 +15,7 @@
>  #include <linux/init.h>
>  #include <linux/serial.h>
>  #include <linux/clk.h>
> +#include <linux/clk-provider.h>
>  #include <linux/console.h>
>  #include <linux/sysrq.h>
>  #include <linux/tty_flip.h>
> @@ -77,6 +78,8 @@ static void atmel_stop_rx(struct uart_port *port);
>  #endif
>  
>  #define ATMEL_ISR_PASS_LIMIT	256
> +#define ERROR_RATE(desired_value, actual_value) \
> +	((int)(100 - ((desired_value) * 100) / (actual_value)))
>  
>  struct atmel_dma_buffer {
>  	unsigned char	*buf;
> @@ -110,6 +113,7 @@ struct atmel_uart_char {
>  struct atmel_uart_port {
>  	struct uart_port	uart;		/* uart */
>  	struct clk		*clk;		/* uart clock */
> +	struct clk		*gclk;		/* uart generic clock */
>  	int			may_wakeup;	/* cached value of device_may_wakeup for times we need to disable it */
>  	u32			backup_imr;	/* IMR saved during suspend */
>  	int			break_active;	/* break being received */
> @@ -2117,6 +2121,8 @@ static void atmel_serial_pm(struct uart_port *port, unsigned int state,
>  		 * This is called on uart_close() or a suspend event.
>  		 */
>  		clk_disable_unprepare(atmel_port->clk);
> +		if (__clk_is_enabled(atmel_port->gclk))
> +			clk_disable_unprepare(atmel_port->gclk);
>  		break;
>  	default:
>  		dev_err(port->dev, "atmel_serial: unknown pm %d\n", state);
> @@ -2131,7 +2137,8 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
>  {
>  	struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
>  	unsigned long flags;
> -	unsigned int old_mode, mode, imr, quot, baud, div, cd, fp = 0;
> +	unsigned int old_mode, mode, imr, quot, div, cd, fp = 0;
> +	unsigned int baud, actual_baud, gclk_rate;
>  
>  	/* save the current mode register */
>  	mode = old_mode = atmel_uart_readl(port, ATMEL_US_MR);
> @@ -2297,6 +2304,43 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
>  		cd &= 65535;
>  	}
>  
> +	/*
> +	 * If there is no Fractional Part, there is a high chance that
> +	 * we may be able to generate a baudrate closer to the desired one
> +	 * if we use the GCLK as the clock source driving the baudrate
> +	 * generator.
> +	 */
> +	if (!atmel_port->has_frac_baudrate) {
> +		if (__clk_is_enabled(atmel_port->gclk))
> +			clk_disable_unprepare(atmel_port->gclk);
> +		gclk_rate = clk_round_rate(atmel_port->gclk, 16 * baud);
> +		actual_baud = clk_get_rate(atmel_port->clk) / (16 * cd);
> +		if (gclk_rate && abs(ERROR_RATE(baud, actual_baud)) >
> +		    abs(ERROR_RATE(baud, gclk_rate / 16))) {
> +			clk_set_rate(atmel_port->gclk, 16 * baud);

This is a personal taste: I would do like this:
			ret = clk_prepare_enable();
			if (ret)
				goto clk_prepare_failure;

			// and here the rest of the code...

> +			if (!clk_prepare_enable(atmel_port->gclk)) {
> +				if (atmel_port->is_usart) {
> +					mode &= ~ATMEL_US_USCLKS;
> +					mode |= ATMEL_US_USCLKS_GCLK;
> +				} else {
> +					mode &= ~ATMEL_UA_BRSRCCK;
> +					mode |= ATMEL_UA_BRSRCCK_GCLK;
> +				}
> +
> +				/*
> +				 * Set the Clock Divisor for GCLK to 1.
> +				 * Since we were able to generate the smallest
> +				 * multiple of the desired baudrate times 16,
> +				 * then we surely can generate a bigger multiple
> +				 * with the exact error rate for an equally increased
> +				 * CD. Thus no need to take into account
> +				 * a higher value for CD.
> +				 */
> +				cd = 1;
> +			}
> +		}
> +	}
> +

clk_prepare_failure: // or other label name

>  	quot = cd | fp << ATMEL_US_FP_OFFSET;
>  
>  	if (!(port->iso7816.flags & SER_ISO7816_ENABLED))
> @@ -2892,6 +2936,12 @@ static int atmel_serial_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto err;
>  
> +	atmel_port->gclk = devm_clk_get_optional(&pdev->dev, "gclk");
> +	if (IS_ERR(atmel_port->gclk)) {
> +		ret = PTR_ERR(atmel_port->gclk);
> +		goto err;

This should be:
		goto err_clk_disable_unprepare;

to disable the peripheral clock previously enabled.

> +	}
> +
>  	ret = atmel_init_port(atmel_port, pdev);
>  	if (ret)
>  		goto err_clk_disable_unprepare;