Re: [U-Boot] [PATCH 03/13 v3] serial: atmel_usart: Use fixed clock value in SPL version with DM_SERIAL

2019-04-02 Thread Eugen.Hristev


On 02.04.2019 13:40, Stefan Roese wrote:
> External E-Mail
> 
> 
> Hi Eugen,
> 
> On 02.04.19 12:12, eugen.hris...@microchip.com wrote:
>>
>>
>> On 02.04.2019 11:57, Stefan Roese wrote:
>>
>>> This patch adds an alterative SPL version of atmel_serial_enable_clk().
>>> This enables the usage of this driver without full clock support (in
>>> drivers and DT nodes). This saves some space in the SPL image.
>>>
>>> Please note that this fixed clock support is only added to the SPL code
>>> in the DM_SERIAL part of this file. All boards not using SPL & DM_SERIAL
>>> should not be affected.
>>>
>>> This patch also introduces CONFIG_SPL_UART_CLOCK for the fixed UART
>>> input clock. It defaults to 132096000 for ARCH_AT91 but can be set to
>>> a different value if needed.
>>>
>>> Signed-off-by: Stefan Roese 
>>> Cc: Heiko Schocher 
>>> Cc: Andreas Bießmann 
>>> Cc: Eugen Hristev 
>>> Reviewed-by: Heiko Schocher 
>>> Tested on the taurus board:
>>> Tested-by: Heiko Schocher 
>>> ---
>>> v3:
>>> - Depend fixed clock atmel_serial_enable_clk() function also on
>>>     !CONFIG_SPL_CLK so that board with full clocj support in SPL
>>>     can still use the normal function here
>>> - Introcude CONFIG_SPL_UART_CLOCK and use this Kconfig option instead
>>>     of the hardcoded value
>>>
>>> v2:
>>> - Reword patch subject and commit text to make it more clear, that
>>>     this change only affects ports with SPL and DM_SERIAL enabled
>>>    drivers/serial/Kconfig   |  9 +
>>>    drivers/serial/atmel_usart.c | 12 
>>>    2 files changed, 21 insertions(+)
>>>
>>> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
>>> index 887cd687c0..d6bf8309d3 100644
>>> --- a/drivers/serial/Kconfig
>>> +++ b/drivers/serial/Kconfig
>>> @@ -508,6 +508,15 @@ config ATMEL_USART
>>>  configured in the device tree, and input clock frequency can
>>>  be got from the clk node.
>>> +config SPL_UART_CLOCK
>>> +    int "SPL fixed UART input clock"
>>> +    depends on SPL
>>> +    default 132096000 if ARCH_AT91
>>
>> Hi Stefan,
>>
>> This doesn't look good. This has to be unset for the platforms that do
>> not specifically set it.
>>
>> For example, when I build the sama5d2_xplained_mmc_defconfig, I look in
>> .config and :
>>
>> CONFIG_SPL_UART_CLOCK=132096000
>>
>> This may be unused, but it's confusing
> 
> Okay, I agree.
> 
>> Can you make this default unset and then specifically set it in the
>> boards that need it ? ( the specific defconfig )
>>
>> And if you have a mutual exclusion with CONFIG_SPL_CLK can we make it
>> somehow ? (in a menu or choice..., either old behavior or fixed uart 
>> clock)
> 
> How about this version?
> 
> config SPL_UART_CLOCK
>  int "SPL fixed UART input clock"
>  depends on SPL && !SPL_CLK
>  help
>    Provide a fixed clock value as input to the UART controller. This
>    might be needed on platforms which can't enable CONFIG_SPL_CLK
>    because of SPL image size restirctions.

Looks much better. Typo on restrictirions

Can also add some default then, to not break build in case someone 
disables SPL_CLK but does not set SPL_UART_CLOCK ?

Thanks !

> 
> Thanks,
> Stefan
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH 03/13 v3] serial: atmel_usart: Use fixed clock value in SPL version with DM_SERIAL

2019-04-02 Thread Stefan Roese

Hi Eugen,

On 02.04.19 12:12, eugen.hris...@microchip.com wrote:



On 02.04.2019 11:57, Stefan Roese wrote:


This patch adds an alterative SPL version of atmel_serial_enable_clk().
This enables the usage of this driver without full clock support (in
drivers and DT nodes). This saves some space in the SPL image.

Please note that this fixed clock support is only added to the SPL code
in the DM_SERIAL part of this file. All boards not using SPL & DM_SERIAL
should not be affected.

This patch also introduces CONFIG_SPL_UART_CLOCK for the fixed UART
input clock. It defaults to 132096000 for ARCH_AT91 but can be set to
a different value if needed.

Signed-off-by: Stefan Roese 
Cc: Heiko Schocher 
Cc: Andreas Bießmann 
Cc: Eugen Hristev 
Reviewed-by: Heiko Schocher 
Tested on the taurus board:
Tested-by: Heiko Schocher 
---
v3:
- Depend fixed clock atmel_serial_enable_clk() function also on
!CONFIG_SPL_CLK so that board with full clocj support in SPL
can still use the normal function here
- Introcude CONFIG_SPL_UART_CLOCK and use this Kconfig option instead
of the hardcoded value

v2:
- Reword patch subject and commit text to make it more clear, that
this change only affects ports with SPL and DM_SERIAL enabled

   drivers/serial/Kconfig   |  9 +

   drivers/serial/atmel_usart.c | 12 
   2 files changed, 21 insertions(+)

diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
index 887cd687c0..d6bf8309d3 100644
--- a/drivers/serial/Kconfig
+++ b/drivers/serial/Kconfig
@@ -508,6 +508,15 @@ config ATMEL_USART
  configured in the device tree, and input clock frequency can
  be got from the clk node.
   
+config SPL_UART_CLOCK

+   int "SPL fixed UART input clock"
+   depends on SPL
+   default 132096000 if ARCH_AT91


Hi Stefan,

This doesn't look good. This has to be unset for the platforms that do
not specifically set it.

For example, when I build the sama5d2_xplained_mmc_defconfig, I look in
.config and :

CONFIG_SPL_UART_CLOCK=132096000

This may be unused, but it's confusing


Okay, I agree.
 

Can you make this default unset and then specifically set it in the
boards that need it ? ( the specific defconfig )

And if you have a mutual exclusion with CONFIG_SPL_CLK can we make it
somehow ? (in a menu or choice..., either old behavior or fixed uart clock)


How about this version?

config SPL_UART_CLOCK
int "SPL fixed UART input clock"
depends on SPL && !SPL_CLK
help
  Provide a fixed clock value as input to the UART controller. This
  might be needed on platforms which can't enable CONFIG_SPL_CLK
  because of SPL image size restirctions.

Thanks,
Stefan
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH 03/13 v3] serial: atmel_usart: Use fixed clock value in SPL version with DM_SERIAL

2019-04-02 Thread Eugen.Hristev


On 02.04.2019 11:57, Stefan Roese wrote:

> This patch adds an alterative SPL version of atmel_serial_enable_clk().
> This enables the usage of this driver without full clock support (in
> drivers and DT nodes). This saves some space in the SPL image.
> 
> Please note that this fixed clock support is only added to the SPL code
> in the DM_SERIAL part of this file. All boards not using SPL & DM_SERIAL
> should not be affected.
> 
> This patch also introduces CONFIG_SPL_UART_CLOCK for the fixed UART
> input clock. It defaults to 132096000 for ARCH_AT91 but can be set to
> a different value if needed.
> 
> Signed-off-by: Stefan Roese 
> Cc: Heiko Schocher 
> Cc: Andreas Bießmann 
> Cc: Eugen Hristev 
> Reviewed-by: Heiko Schocher 
> Tested on the taurus board:
> Tested-by: Heiko Schocher 
> ---
> v3:
> - Depend fixed clock atmel_serial_enable_clk() function also on
>!CONFIG_SPL_CLK so that board with full clocj support in SPL
>can still use the normal function here
> - Introcude CONFIG_SPL_UART_CLOCK and use this Kconfig option instead
>of the hardcoded value
> 
> v2:
> - Reword patch subject and commit text to make it more clear, that
>this change only affects ports with SPL and DM_SERIAL enabled
>
>   drivers/serial/Kconfig   |  9 +
>   drivers/serial/atmel_usart.c | 12 
>   2 files changed, 21 insertions(+)
> 
> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
> index 887cd687c0..d6bf8309d3 100644
> --- a/drivers/serial/Kconfig
> +++ b/drivers/serial/Kconfig
> @@ -508,6 +508,15 @@ config ATMEL_USART
> configured in the device tree, and input clock frequency can
> be got from the clk node.
>   
> +config SPL_UART_CLOCK
> + int "SPL fixed UART input clock"
> + depends on SPL
> + default 132096000 if ARCH_AT91

Hi Stefan,

This doesn't look good. This has to be unset for the platforms that do 
not specifically set it.

For example, when I build the sama5d2_xplained_mmc_defconfig, I look in 
.config and :

CONFIG_SPL_UART_CLOCK=132096000

This may be unused, but it's confusing

Can you make this default unset and then specifically set it in the 
boards that need it ? ( the specific defconfig )

And if you have a mutual exclusion with CONFIG_SPL_CLK can we make it 
somehow ? (in a menu or choice..., either old behavior or fixed uart clock)

Does this make sense?

Thanks

> + help
> +   Provide a fixed clock value as input to the UART controller. This
> +   might be needed on platforms which can't enable CONFIG_SPL_CLK
> +   because of SPL image size restirctions.
> +
>   config BCM283X_MU_SERIAL
>   bool "Support for BCM283x Mini-UART"
>   depends on DM_SERIAL && ARCH_BCM283X
> diff --git a/drivers/serial/atmel_usart.c b/drivers/serial/atmel_usart.c
> index aa8cdff840..c450a4e08a 100644
> --- a/drivers/serial/atmel_usart.c
> +++ b/drivers/serial/atmel_usart.c
> @@ -218,6 +218,17 @@ static const struct dm_serial_ops atmel_serial_ops = {
>   .setbrg = atmel_serial_setbrg,
>   };
>   
> +#if defined(CONFIG_SPL_BUILD) && !defined(CONFIG_SPL_CLK)
> +static int atmel_serial_enable_clk(struct udevice *dev)
> +{
> + struct atmel_serial_priv *priv = dev_get_priv(dev);
> +
> + /* Use fixed clock value in SPL */
> + priv->usart_clk_rate = CONFIG_SPL_UART_CLOCK;
> +
> + return 0;
> +}
> +#else
>   static int atmel_serial_enable_clk(struct udevice *dev)
>   {
>   struct atmel_serial_priv *priv = dev_get_priv(dev);
> @@ -245,6 +256,7 @@ static int atmel_serial_enable_clk(struct udevice *dev)
>   
>   return 0;
>   }
> +#endif
>   
>   static int atmel_serial_probe(struct udevice *dev)
>   {
> 
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot