diff mbox

[U-Boot,v5,5/9] arm: serial: Add ability to use pre-initialized UARTs

Message ID 1441639105-25181-6-git-send-email-s.temerkhanov@gmail.com
State Deferred
Delegated to: Tom Rini
Headers show

Commit Message

Sergey Temerkhanov Sept. 7, 2015, 3:18 p.m. UTC 
On some systems, UART initialization is performed before running U-Boot.
This commit allows to skip UART re-initializaion on those systems

Signed-off-by: Sergey Temerkhanov <s.temerkhanov@gmail.com>
Signed-off-by: Radha Mohan Chintakuntla <rchintakuntla@cavium.com>

---

Changes in v5:
- Added OF control support

Changes in v4:
- Fixed build warnings
- Moved to DM_SERIAL

Changes in v3:
- Added __used keyword

Changes in v2: None

 drivers/serial/serial_pl01x.c           | 13 +++++++------
 include/dm/platform_data/serial_pl01x.h |  6 ++++++
 2 files changed, 13 insertions(+), 6 deletions(-)

Comments

Simon Glass Sept. 8, 2015, 3:56 a.m. UTC | #1 
Hi Sergey,

On 7 September 2015 at 09:18, Sergey Temerkhanov
<s.temerkhanov@gmail.com> wrote:
> On some systems, UART initialization is performed before running U-Boot.
> This commit allows to skip UART re-initializaion on those systems
>
> Signed-off-by: Sergey Temerkhanov <s.temerkhanov@gmail.com>
> Signed-off-by: Radha Mohan Chintakuntla <rchintakuntla@cavium.com>
>
> ---
>
> Changes in v5:
> - Added OF control support
>
> Changes in v4:
> - Fixed build warnings
> - Moved to DM_SERIAL
>
> Changes in v3:
> - Added __used keyword
>
> Changes in v2: None
>
>  drivers/serial/serial_pl01x.c           | 13 +++++++------
>  include/dm/platform_data/serial_pl01x.h |  6 ++++++
>  2 files changed, 13 insertions(+), 6 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

But please see one issue below. Also you might consider implementing
the debug UART for this driver so you get early debugging,

>
> diff --git a/drivers/serial/serial_pl01x.c b/drivers/serial/serial_pl01x.c
> index ecf3bc0..83151e0 100644
> --- a/drivers/serial/serial_pl01x.c
> +++ b/drivers/serial/serial_pl01x.c
> @@ -125,7 +125,7 @@ static int pl011_set_line_control(struct pl01x_regs *regs)
>  }
>
>  static int pl01x_generic_setbrg(struct pl01x_regs *regs, enum pl01x_type type,
> -                               int clock, int baudrate)
> +                                      int clock, int baudrate)
>  {
>         switch (type) {
>         case TYPE_PL010: {
> @@ -295,7 +295,6 @@ __weak struct serial_device *default_serial_console(void)
>  #endif /* nCONFIG_DM_SERIAL */
>
>  #ifdef CONFIG_DM_SERIAL
> -
>  struct pl01x_priv {
>         struct pl01x_regs *regs;
>         enum pl01x_type type;
> @@ -306,9 +305,9 @@ static int pl01x_serial_setbrg(struct udevice *dev, int baudrate)
>         struct pl01x_serial_platdata *plat = dev_get_platdata(dev);
>         struct pl01x_priv *priv = dev_get_priv(dev);
>
> -       pl01x_generic_setbrg(priv->regs, priv->type, plat->clock, baudrate);
> -
> -       return 0;
> +       return (plat->flags & PL0X_PREINITIALIZED) ? 0 :
> +               pl01x_generic_setbrg(priv->regs, priv->type,
> +                                    plat->clock, baudrate);
>  }
>
>  static int pl01x_serial_probe(struct udevice *dev)
> @@ -318,7 +317,8 @@ static int pl01x_serial_probe(struct udevice *dev)
>
>         priv->regs = (struct pl01x_regs *)plat->base;
>         priv->type = plat->type;
> -       return pl01x_generic_serial_init(priv->regs, priv->type);
> +       return (plat->flags & PL0X_PREINITIALIZED) ? 0 :
> +               pl01x_generic_serial_init(priv->regs, priv->type);
>  }
>
>  static int pl01x_serial_getc(struct udevice *dev)
> @@ -372,6 +372,7 @@ static int pl01x_serial_ofdata_to_platdata(struct udevice *dev)
>         plat->base = addr;
>         plat->clock = fdtdec_get_int(gd->fdt_blob, dev->of_offset, "clock", 1);
>         plat->type = dev_get_driver_data(dev);
> +       plat->flags = fdtdec_get_int(gd->fdt_blob, dev->of_offset, "flags", 0);

Can I suggest a boolean option here, like u-boot,skip-init? You can
use fdtdec_get_bool() to read it. Also please add it to
doc/device-tree-bindings/serial/pl01x.txt.

>         return 0;
>  }
>  #endif
> diff --git a/include/dm/platform_data/serial_pl01x.h b/include/dm/platform_data/serial_pl01x.h
> index 5e068f3..73e1be0 100644
> --- a/include/dm/platform_data/serial_pl01x.h
> +++ b/include/dm/platform_data/serial_pl01x.h
> @@ -11,17 +11,23 @@ enum pl01x_type {
>         TYPE_PL011,
>  };
>
> +enum pl01x_flags {
> +       PL0X_PREINITIALIZED = 1 << 0, /* Skip port initialization */
> +};
> +
>  /*
>   *Information about a serial port
>   *
>   * @base: Register base address
>   * @type: Port type
>   * @clock: Input clock rate, used for calculating the baud rate divisor
> + * @flags: Port flags
>   */
>  struct pl01x_serial_platdata {
>         unsigned long base;
>         enum pl01x_type type;
>         unsigned int clock;
> +       unsigned long flags;
>  };
>
>  #endif
> --
> 2.2.0
>

Regards,
Simon
Sergey Temerkhanov Sept. 8, 2015, 11:44 a.m. UTC | #2 
On Tue, Sep 8, 2015 at 6:56 AM, Simon Glass <sjg@chromium.org> wrote:
> Hi Sergey,
>
> On 7 September 2015 at 09:18, Sergey Temerkhanov
> <s.temerkhanov@gmail.com> wrote:
>> On some systems, UART initialization is performed before running U-Boot.
>> This commit allows to skip UART re-initializaion on those systems
>>
>> Signed-off-by: Sergey Temerkhanov <s.temerkhanov@gmail.com>
>> Signed-off-by: Radha Mohan Chintakuntla <rchintakuntla@cavium.com>
>>
>> ---
>>
>> Changes in v5:
>> - Added OF control support
>>
>> Changes in v4:
>> - Fixed build warnings
>> - Moved to DM_SERIAL
>>
>> Changes in v3:
>> - Added __used keyword
>>
>> Changes in v2: None
>>
>>  drivers/serial/serial_pl01x.c           | 13 +++++++------
>>  include/dm/platform_data/serial_pl01x.h |  6 ++++++
>>  2 files changed, 13 insertions(+), 6 deletions(-)
>
> Reviewed-by: Simon Glass <sjg@chromium.org>
>
> But please see one issue below. Also you might consider implementing
> the debug UART for this driver so you get early debugging,

I'll look into this and create a separate patch once it's ready.
>
>>
>> diff --git a/drivers/serial/serial_pl01x.c b/drivers/serial/serial_pl01x.c
>> index ecf3bc0..83151e0 100644
>> --- a/drivers/serial/serial_pl01x.c
>> +++ b/drivers/serial/serial_pl01x.c
>> @@ -125,7 +125,7 @@ static int pl011_set_line_control(struct pl01x_regs *regs)
>>  }
>>
>>  static int pl01x_generic_setbrg(struct pl01x_regs *regs, enum pl01x_type type,
>> -                               int clock, int baudrate)
>> +                                      int clock, int baudrate)
>>  {
>>         switch (type) {
>>         case TYPE_PL010: {
>> @@ -295,7 +295,6 @@ __weak struct serial_device *default_serial_console(void)
>>  #endif /* nCONFIG_DM_SERIAL */
>>
>>  #ifdef CONFIG_DM_SERIAL
>> -
>>  struct pl01x_priv {
>>         struct pl01x_regs *regs;
>>         enum pl01x_type type;
>> @@ -306,9 +305,9 @@ static int pl01x_serial_setbrg(struct udevice *dev, int baudrate)
>>         struct pl01x_serial_platdata *plat = dev_get_platdata(dev);
>>         struct pl01x_priv *priv = dev_get_priv(dev);
>>
>> -       pl01x_generic_setbrg(priv->regs, priv->type, plat->clock, baudrate);
>> -
>> -       return 0;
>> +       return (plat->flags & PL0X_PREINITIALIZED) ? 0 :
>> +               pl01x_generic_setbrg(priv->regs, priv->type,
>> +                                    plat->clock, baudrate);
>>  }
>>
>>  static int pl01x_serial_probe(struct udevice *dev)
>> @@ -318,7 +317,8 @@ static int pl01x_serial_probe(struct udevice *dev)
>>
>>         priv->regs = (struct pl01x_regs *)plat->base;
>>         priv->type = plat->type;
>> -       return pl01x_generic_serial_init(priv->regs, priv->type);
>> +       return (plat->flags & PL0X_PREINITIALIZED) ? 0 :
>> +               pl01x_generic_serial_init(priv->regs, priv->type);
>>  }
>>
>>  static int pl01x_serial_getc(struct udevice *dev)
>> @@ -372,6 +372,7 @@ static int pl01x_serial_ofdata_to_platdata(struct udevice *dev)
>>         plat->base = addr;
>>         plat->clock = fdtdec_get_int(gd->fdt_blob, dev->of_offset, "clock", 1);
>>         plat->type = dev_get_driver_data(dev);
>> +       plat->flags = fdtdec_get_int(gd->fdt_blob, dev->of_offset, "flags", 0);
>
> Can I suggest a boolean option here, like u-boot,skip-init? You can
> use fdtdec_get_bool() to read it.

Agreed. This improves code readability and maintainability.

> Also please add it to
> doc/device-tree-bindings/serial/pl01x.txt.

OK.

>
>>         return 0;
>>  }
>>  #endif
>> diff --git a/include/dm/platform_data/serial_pl01x.h b/include/dm/platform_data/serial_pl01x.h
>> index 5e068f3..73e1be0 100644
>> --- a/include/dm/platform_data/serial_pl01x.h
>> +++ b/include/dm/platform_data/serial_pl01x.h
>> @@ -11,17 +11,23 @@ enum pl01x_type {
>>         TYPE_PL011,
>>  };
>>
>> +enum pl01x_flags {
>> +       PL0X_PREINITIALIZED = 1 << 0, /* Skip port initialization */
>> +};
>> +
>>  /*
>>   *Information about a serial port
>>   *
>>   * @base: Register base address
>>   * @type: Port type
>>   * @clock: Input clock rate, used for calculating the baud rate divisor
>> + * @flags: Port flags
>>   */
>>  struct pl01x_serial_platdata {
>>         unsigned long base;
>>         enum pl01x_type type;
>>         unsigned int clock;
>> +       unsigned long flags;
>>  };
>>
>>  #endif
>> --
>> 2.2.0
>>
>
> Regards,
> Simon
diff mbox

Patch

diff --git a/drivers/serial/serial_pl01x.c b/drivers/serial/serial_pl01x.c
index ecf3bc0..83151e0 100644
--- a/drivers/serial/serial_pl01x.c
+++ b/drivers/serial/serial_pl01x.c
@@ -125,7 +125,7 @@  static int pl011_set_line_control(struct pl01x_regs *regs)
 }
 
 static int pl01x_generic_setbrg(struct pl01x_regs *regs, enum pl01x_type type,
-				int clock, int baudrate)
+				       int clock, int baudrate)
 {
 	switch (type) {
 	case TYPE_PL010: {
@@ -295,7 +295,6 @@  __weak struct serial_device *default_serial_console(void)
 #endif /* nCONFIG_DM_SERIAL */
 
 #ifdef CONFIG_DM_SERIAL
-
 struct pl01x_priv {
 	struct pl01x_regs *regs;
 	enum pl01x_type type;
@@ -306,9 +305,9 @@  static int pl01x_serial_setbrg(struct udevice *dev, int baudrate)
 	struct pl01x_serial_platdata *plat = dev_get_platdata(dev);
 	struct pl01x_priv *priv = dev_get_priv(dev);
 
-	pl01x_generic_setbrg(priv->regs, priv->type, plat->clock, baudrate);
-
-	return 0;
+	return (plat->flags & PL0X_PREINITIALIZED) ? 0 :
+		pl01x_generic_setbrg(priv->regs, priv->type,
+				     plat->clock, baudrate);
 }
 
 static int pl01x_serial_probe(struct udevice *dev)
@@ -318,7 +317,8 @@  static int pl01x_serial_probe(struct udevice *dev)
 
 	priv->regs = (struct pl01x_regs *)plat->base;
 	priv->type = plat->type;
-	return pl01x_generic_serial_init(priv->regs, priv->type);
+	return (plat->flags & PL0X_PREINITIALIZED) ? 0 :
+		pl01x_generic_serial_init(priv->regs, priv->type);
 }
 
 static int pl01x_serial_getc(struct udevice *dev)
@@ -372,6 +372,7 @@  static int pl01x_serial_ofdata_to_platdata(struct udevice *dev)
 	plat->base = addr;
 	plat->clock = fdtdec_get_int(gd->fdt_blob, dev->of_offset, "clock", 1);
 	plat->type = dev_get_driver_data(dev);
+	plat->flags = fdtdec_get_int(gd->fdt_blob, dev->of_offset, "flags", 0);
 	return 0;
 }
 #endif
diff --git a/include/dm/platform_data/serial_pl01x.h b/include/dm/platform_data/serial_pl01x.h
index 5e068f3..73e1be0 100644
--- a/include/dm/platform_data/serial_pl01x.h
+++ b/include/dm/platform_data/serial_pl01x.h
@@ -11,17 +11,23 @@  enum pl01x_type {
 	TYPE_PL011,
 };
 
+enum pl01x_flags {
+	PL0X_PREINITIALIZED = 1 << 0, /* Skip port initialization */
+};
+
 /*
  *Information about a serial port
  *
  * @base: Register base address
  * @type: Port type
  * @clock: Input clock rate, used for calculating the baud rate divisor
+ * @flags: Port flags
  */
 struct pl01x_serial_platdata {
 	unsigned long base;
 	enum pl01x_type type;
 	unsigned int clock;
+	unsigned long flags;
 };
 
 #endif