[PATCH v3] drivers/tty: amba: defer probing DMA availability until hw_init

[PATCH v3] drivers/tty: amba: defer probing DMA availability until hw_init

[Jorge Ramirez-Ortiz]

Fix a race condition that happens when device_initcall(pl011_dma_initicall)
is executed before all the devices have been probed - this issue was observed on
an hisi_6220 SoC (HiKey board from Linaro).

The deferred driver probing framework relies on late_initcall to trigger
deferred probes so it is just possible that, even with a valid DMA driver ready
to be loaded, we fail to synchronize with it.

The proposed implementation delays probing of the DMA until the uart device is
opened. As hw_init is invoked on port startup and port resume - but the DMA
probe is only required once - we avoid calling multiple times using a new field
in uart_amba_port to track this scenario.

Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
---
 drivers/tty/serial/amba-pl011.c | 66 ++++++++++-------------------------------
 1 file changed, 16 insertions(+), 50 deletions(-)

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 8d94c19..3501ccc 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -29,6 +29,7 @@
  * and hooked into this driver.
  */
 
+#define pr_fmt(fmt) "amba-pl011: "fmt
 
 #if defined(CONFIG_SERIAL_AMBA_PL011_CONSOLE) && defined(CONFIG_MAGIC_SYSRQ)
 #define SUPPORT_SYSRQ
@@ -164,6 +165,7 @@ struct uart_amba_port {
 	bool			using_rx_dma;
 	struct pl011_dmarx_data dmarx;
 	struct pl011_dmatx_data	dmatx;
+	bool			dma_probed;
 #endif
 };
 
@@ -261,7 +263,7 @@ static void pl011_sgbuf_free(struct dma_chan *chan, struct pl011_sgbuf *sg,
 	}
 }
 
-static void pl011_dma_probe_initcall(struct device *dev, struct uart_amba_port *uap)
+static void pl011_dma_probe(struct device *dev, struct uart_amba_port *uap)
 {
 	/* DMA is the sole user of the platform data right now */
 	struct amba_pl011_data *plat = dev_get_platdata(uap->port.dev);
@@ -275,9 +277,16 @@ static void pl011_dma_probe_initcall(struct device *dev, struct uart_amba_port *
 	struct dma_chan *chan;
 	dma_cap_mask_t mask;
 
-	chan = dma_request_slave_channel(dev, "tx");
+	uap->dma_probed = true;
+
+	chan = dma_request_slave_channel_reason(dev, "tx");
+	if (IS_ERR(chan)) {
+		if (PTR_ERR(chan) == -EPROBE_DEFER) {
+			dev_warn(uap->port.dev, "DMA driver not ready\n");
+			return;
+		}
+		dev_info(uap->port.dev, "no OF or ACPI DMA data\n");
 
-	if (!chan) {
 		/* We need platform data */
 		if (!plat || !plat->dma_filter) {
 			dev_info(uap->port.dev, "no DMA platform data\n");
@@ -383,54 +392,9 @@ static void pl011_dma_probe_initcall(struct device *dev, struct uart_amba_port *
 		dev_info(uap->port.dev, "DMA channel RX %s\n",
 			 dma_chan_name(uap->dmarx.chan));
 	}
-}
-
-#ifndef MODULE
-/*
- * Stack up the UARTs and let the above initcall be done at device
- * initcall time, because the serial driver is called as an arch
- * initcall, and at this time the DMA subsystem is not yet registered.
- * At this point the driver will switch over to using DMA where desired.
- */
-struct dma_uap {
-	struct list_head node;
-	struct uart_amba_port *uap;
-	struct device *dev;
-};
-
-static LIST_HEAD(pl011_dma_uarts);
-
-static int __init pl011_dma_initcall(void)
-{
-	struct list_head *node, *tmp;
-
-	list_for_each_safe(node, tmp, &pl011_dma_uarts) {
-		struct dma_uap *dmau = list_entry(node, struct dma_uap, node);
-		pl011_dma_probe_initcall(dmau->dev, dmau->uap);
-		list_del(node);
-		kfree(dmau);
-	}
-	return 0;
-}
-
-device_initcall(pl011_dma_initcall);
 
-static void pl011_dma_probe(struct device *dev, struct uart_amba_port *uap)
-{
-	struct dma_uap *dmau = kzalloc(sizeof(struct dma_uap), GFP_KERNEL);
-	if (dmau) {
-		dmau->uap = uap;
-		dmau->dev = dev;
-		list_add_tail(&dmau->node, &pl011_dma_uarts);
-	}
+	return;
 }
-#else
-static void pl011_dma_probe(struct device *dev, struct uart_amba_port *uap)
-{
-	pl011_dma_probe_initcall(dev, uap);
-}
-#endif
-
 static void pl011_dma_remove(struct uart_amba_port *uap)
 {
 	/* TODO: remove the initcall if it has not yet executed */
@@ -1548,6 +1512,9 @@ static int pl011_hwinit(struct uart_port *port)
 	uap->im = readw(uap->port.membase + UART011_IMSC);
 	writew(UART011_RTIM | UART011_RXIM, uap->port.membase + UART011_IMSC);
 
+	if (!uap->dma_probed)
+		pl011_dma_probe(uap->port.dev, uap);
+
 	if (dev_get_platdata(uap->port.dev)) {
 		struct amba_pl011_data *plat;
 
@@ -2218,7 +2185,6 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
 	uap->port.ops = &amba_pl011_pops;
 	uap->port.flags = UPF_BOOT_AUTOCONF;
 	uap->port.line = i;
-	pl011_dma_probe(&dev->dev, uap);
 
 	/* Ensure interrupts from this UART are masked and cleared */
 	writew(0, uap->port.membase + UART011_IMSC);
-- 
1.9.1

[PATCH v3] drivers/tty: amba: defer probing DMA availability until hw_init

[Russell King - ARM Linux]

On Wed, Feb 25, 2015 at 07:31:01PM -0500, Jorge Ramirez-Ortiz wrote:
> @@ -275,9 +277,16 @@ static void pl011_dma_probe_initcall(struct device *dev, struct uart_amba_port *
>  	struct dma_chan *chan;
>  	dma_cap_mask_t mask;
>  
> -	chan = dma_request_slave_channel(dev, "tx");
> +	uap->dma_probed = true;
> +
> +	chan = dma_request_slave_channel_reason(dev, "tx");
> +	if (IS_ERR(chan)) {
> +		if (PTR_ERR(chan) == -EPROBE_DEFER) {
> +			dev_warn(uap->port.dev, "DMA driver not ready\n");
> +			return;
> +		}
> +		dev_info(uap->port.dev, "no OF or ACPI DMA data\n");

So if we get an -EPROBE_DEFER, we never try again, because uap->dma_probed
is left true?

Should we even emit a message here?  What if userspace opens and closes
the device multiple times before the DMA driver is ready?

Please get rid of that "no OF or ACPI DMA data" message.  There are
situations where we have a DMA engine, but there is quite legally no
DMA available on the port.  Such a message would be an annoyance.
DMA is optional.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

[PATCH v3] drivers/tty: amba: defer probing DMA availability until hw_init

[Jorge Ramirez-Ortiz]

On 02/25/2015 07:56 PM, Russell King - ARM Linux wrote:
> On Wed, Feb 25, 2015 at 07:31:01PM -0500, Jorge Ramirez-Ortiz wrote:
>> @@ -275,9 +277,16 @@ static void pl011_dma_probe_initcall(struct device *dev, struct uart_amba_port *
>>  	struct dma_chan *chan;
>>  	dma_cap_mask_t mask;
>>  
>> -	chan = dma_request_slave_channel(dev, "tx");
>> +	uap->dma_probed = true;
>> +
>> +	chan = dma_request_slave_channel_reason(dev, "tx");
>> +	if (IS_ERR(chan)) {
>> +		if (PTR_ERR(chan) == -EPROBE_DEFER) {
>> +			dev_warn(uap->port.dev, "DMA driver not ready\n");
>> +			return;
>> +		}
>> +		dev_info(uap->port.dev, "no OF or ACPI DMA data\n");
> So if we get an -EPROBE_DEFER, we never try again, because uap->dma_probed
> is left true?

Yes that is the idea: the DMA is expected to be ready at that time (when user
requires access to the uart for the first time)

We wont see any degradation with respect to the previous version of the driver
(the previous version was a single attempt to probe the DMA on a device_initcall
- before even deferred probing had a chance to begin).

hw_init also gets called during resume and allowing that probe would have been
unnecessary.
As well as removing the comment you mentioned I realized that the following is
needed.

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 3501ccc..486d510 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -285,7 +285,6 @@ static void pl011_dma_probe(struct device *dev, struct
uart_amba_port *uap)
                        dev_warn(uap->port.dev, "DMA driver not ready\n");
                        return;
                }
-               dev_info(uap->port.dev, "no OF or ACPI DMA data\n");
 
                /* We need platform data */
                if (!plat || !plat->dma_filter) {
@@ -2208,7 +2207,6 @@ static int pl011_probe(struct amba_device *dev, const
struct amba_id *id)
        if (ret) {
                amba_ports[i] = NULL;
                uart_unregister_driver(&amba_reg);
-               pl011_dma_remove(uap);
        }
 
        return ret;


>
> Should we even emit a message here?  What if userspace opens and closes
> the device multiple times before the DMA driver is ready?

If the DMA is not ready when the user wants to use the UART for the first time
then we give up on attempting to associate a DMA to that uart.
I dont think the tty is available to userland before deferred probes have been
handled anyway?

And open/closing the tty will not cause further attempts to probe and therefore
the message will not be displayed.


>
> Please get rid of that "no OF or ACPI DMA data" message.  There are
> situations where we have a DMA engine, but there is quite legally no
> DMA available on the port.  Such a message would be an annoyance.
> DMA is optional.

yes since the DMA is optional I left it as an info message (not an error)

I put it there in case there was a problem in the device tree strings that
define the DMA to use.
But I guess it can become annoying even if it is only shown once.
OK I'll take it out.



>

[PATCH v3] drivers/tty: amba: defer probing DMA availability until hw_init

[Rob Herring]

On Wed, Feb 25, 2015 at 6:31 PM, Jorge Ramirez-Ortiz
<jorge.ramirez-ortiz@linaro.org> wrote:
> Fix a race condition that happens when device_initcall(pl011_dma_initicall)
> is executed before all the devices have been probed - this issue was observed on
> an hisi_6220 SoC (HiKey board from Linaro).
>
> The deferred driver probing framework relies on late_initcall to trigger
> deferred probes so it is just possible that, even with a valid DMA driver ready
> to be loaded, we fail to synchronize with it.
>
> The proposed implementation delays probing of the DMA until the uart device is
> opened. As hw_init is invoked on port startup and port resume - but the DMA
> probe is only required once - we avoid calling multiple times using a new field
> in uart_amba_port to track this scenario.
>
> Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
> ---
>  drivers/tty/serial/amba-pl011.c | 66 ++++++++++-------------------------------
>  1 file changed, 16 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
> index 8d94c19..3501ccc 100644
> --- a/drivers/tty/serial/amba-pl011.c
> +++ b/drivers/tty/serial/amba-pl011.c
> @@ -29,6 +29,7 @@
>   * and hooked into this driver.
>   */
>
> +#define pr_fmt(fmt) "amba-pl011: "fmt

You can drop this I think.

>  #if defined(CONFIG_SERIAL_AMBA_PL011_CONSOLE) && defined(CONFIG_MAGIC_SYSRQ)
>  #define SUPPORT_SYSRQ
> @@ -164,6 +165,7 @@ struct uart_amba_port {
>         bool                    using_rx_dma;
>         struct pl011_dmarx_data dmarx;
>         struct pl011_dmatx_data dmatx;
> +       bool                    dma_probed;
>  #endif
>  };
>
> @@ -261,7 +263,7 @@ static void pl011_sgbuf_free(struct dma_chan *chan, struct pl011_sgbuf *sg,
>         }
>  }
>
> -static void pl011_dma_probe_initcall(struct device *dev, struct uart_amba_port *uap)
> +static void pl011_dma_probe(struct device *dev, struct uart_amba_port *uap)
>  {
>         /* DMA is the sole user of the platform data right now */
>         struct amba_pl011_data *plat = dev_get_platdata(uap->port.dev);
> @@ -275,9 +277,16 @@ static void pl011_dma_probe_initcall(struct device *dev, struct uart_amba_port *
>         struct dma_chan *chan;
>         dma_cap_mask_t mask;
>
> -       chan = dma_request_slave_channel(dev, "tx");
> +       uap->dma_probed = true;

I would not set this to true in the case of EPROBE_DEFER. That way if
a DMA driver is loaded as a module some time later, DMA can be used.
We're not really using deferred probe here, but using it as an
indication that the UART is DMA capable.

> +
> +       chan = dma_request_slave_channel_reason(dev, "tx");
> +       if (IS_ERR(chan)) {
> +               if (PTR_ERR(chan) == -EPROBE_DEFER) {
> +                       dev_warn(uap->port.dev, "DMA driver not ready\n");
> +                       return;
> +               }
> +               dev_info(uap->port.dev, "no OF or ACPI DMA data\n");
>
> -       if (!chan) {
>                 /* We need platform data */
>                 if (!plat || !plat->dma_filter) {
>                         dev_info(uap->port.dev, "no DMA platform data\n");
> @@ -383,54 +392,9 @@ static void pl011_dma_probe_initcall(struct device *dev, struct uart_amba_port *
>                 dev_info(uap->port.dev, "DMA channel RX %s\n",
>                          dma_chan_name(uap->dmarx.chan));
>         }
> -}
> -
> -#ifndef MODULE
> -/*
> - * Stack up the UARTs and let the above initcall be done at device
> - * initcall time, because the serial driver is called as an arch
> - * initcall, and at this time the DMA subsystem is not yet registered.
> - * At this point the driver will switch over to using DMA where desired.
> - */
> -struct dma_uap {
> -       struct list_head node;
> -       struct uart_amba_port *uap;
> -       struct device *dev;
> -};
> -
> -static LIST_HEAD(pl011_dma_uarts);
> -
> -static int __init pl011_dma_initcall(void)
> -{
> -       struct list_head *node, *tmp;
> -
> -       list_for_each_safe(node, tmp, &pl011_dma_uarts) {
> -               struct dma_uap *dmau = list_entry(node, struct dma_uap, node);
> -               pl011_dma_probe_initcall(dmau->dev, dmau->uap);
> -               list_del(node);
> -               kfree(dmau);
> -       }
> -       return 0;
> -}
> -
> -device_initcall(pl011_dma_initcall);
>
> -static void pl011_dma_probe(struct device *dev, struct uart_amba_port *uap)
> -{
> -       struct dma_uap *dmau = kzalloc(sizeof(struct dma_uap), GFP_KERNEL);
> -       if (dmau) {
> -               dmau->uap = uap;
> -               dmau->dev = dev;
> -               list_add_tail(&dmau->node, &pl011_dma_uarts);
> -       }
> +       return;

You don't need an explicit return.

>  }
> -#else
> -static void pl011_dma_probe(struct device *dev, struct uart_amba_port *uap)
> -{
> -       pl011_dma_probe_initcall(dev, uap);
> -}
> -#endif
> -
>  static void pl011_dma_remove(struct uart_amba_port *uap)
>  {
>         /* TODO: remove the initcall if it has not yet executed */
> @@ -1548,6 +1512,9 @@ static int pl011_hwinit(struct uart_port *port)
>         uap->im = readw(uap->port.membase + UART011_IMSC);
>         writew(UART011_RTIM | UART011_RXIM, uap->port.membase + UART011_IMSC);
>
> +       if (!uap->dma_probed)
> +               pl011_dma_probe(uap->port.dev, uap);
> +
>         if (dev_get_platdata(uap->port.dev)) {
>                 struct amba_pl011_data *plat;
>
> @@ -2218,7 +2185,6 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
>         uap->port.ops = &amba_pl011_pops;
>         uap->port.flags = UPF_BOOT_AUTOCONF;
>         uap->port.line = i;
> -       pl011_dma_probe(&dev->dev, uap);
>
>         /* Ensure interrupts from this UART are masked and cleared */
>         writew(0, uap->port.membase + UART011_IMSC);
> --
> 1.9.1
>

[PATCH v3] drivers/tty: amba: defer probing DMA availability until hw_init

[Jorge Ramirez-Ortiz]

On 02/26/2015 09:12 AM, Rob Herring wrote:
> On Wed, Feb 25, 2015 at 6:31 PM, Jorge Ramirez-Ortiz
> <jorge.ramirez-ortiz@linaro.org> wrote:
>> Fix a race condition that happens when device_initcall(pl011_dma_initicall)
>> is executed before all the devices have been probed - this issue was observed on
>> an hisi_6220 SoC (HiKey board from Linaro).
>>
>> The deferred driver probing framework relies on late_initcall to trigger
>> deferred probes so it is just possible that, even with a valid DMA driver ready
>> to be loaded, we fail to synchronize with it.
>>
>> The proposed implementation delays probing of the DMA until the uart device is
>> opened. As hw_init is invoked on port startup and port resume - but the DMA
>> probe is only required once - we avoid calling multiple times using a new field
>> in uart_amba_port to track this scenario.
>>
>> Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
>> ---
>>  drivers/tty/serial/amba-pl011.c | 66 ++++++++++-------------------------------
>>  1 file changed, 16 insertions(+), 50 deletions(-)
>>
>> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
>> index 8d94c19..3501ccc 100644
>> --- a/drivers/tty/serial/amba-pl011.c
>> +++ b/drivers/tty/serial/amba-pl011.c
>> @@ -29,6 +29,7 @@
>>   * and hooked into this driver.
>>   */
>>
>> +#define pr_fmt(fmt) "amba-pl011: "fmt
> You can drop this I think.

why not keep it? it makes the trace more readable (if debug is enabled only)

>
>>  #if defined(CONFIG_SERIAL_AMBA_PL011_CONSOLE) && defined(CONFIG_MAGIC_SYSRQ)
>>  #define SUPPORT_SYSRQ
>> @@ -164,6 +165,7 @@ struct uart_amba_port {
>>         bool                    using_rx_dma;
>>         struct pl011_dmarx_data dmarx;
>>         struct pl011_dmatx_data dmatx;
>> +       bool                    dma_probed;
>>  #endif
>>  };
>>
>> @@ -261,7 +263,7 @@ static void pl011_sgbuf_free(struct dma_chan *chan, struct pl011_sgbuf *sg,
>>         }
>>  }
>>
>> -static void pl011_dma_probe_initcall(struct device *dev, struct uart_amba_port *uap)
>> +static void pl011_dma_probe(struct device *dev, struct uart_amba_port *uap)
>>  {
>>         /* DMA is the sole user of the platform data right now */
>>         struct amba_pl011_data *plat = dev_get_platdata(uap->port.dev);
>> @@ -275,9 +277,16 @@ static void pl011_dma_probe_initcall(struct device *dev, struct uart_amba_port *
>>         struct dma_chan *chan;
>>         dma_cap_mask_t mask;
>>
>> -       chan = dma_request_slave_channel(dev, "tx");
>> +       uap->dma_probed = true;
> I would not set this to true in the case of EPROBE_DEFER. That way if
> a DMA driver is loaded as a module some time later, DMA can be used.
> We're not really using deferred probe here, but using it as an
> indication that the UART is DMA capable.

ok. yes there is no need for the consistency I wast talking about on my previous
reply to RMK.
that was rubbish.

>
>> +
>> +       chan = dma_request_slave_channel_reason(dev, "tx");
>> +       if (IS_ERR(chan)) {
>> +               if (PTR_ERR(chan) == -EPROBE_DEFER) {
>> +                       dev_warn(uap->port.dev, "DMA driver not ready\n");

I'll change the warn for an info since there will be future opportunities to
load it.
I see value on the user knowing he might have a chance to use the DMA capability.


>> +                       return;
>> +               }
>> +               dev_info(uap->port.dev, "no OF or ACPI DMA data\n");
>>
>> -       if (!chan) {
>>                 /* We need platform data */
>>                 if (!plat || !plat->dma_filter) {
>>                         dev_info(uap->port.dev, "no DMA platform data\n");
>> @@ -383,54 +392,9 @@ static void pl011_dma_probe_initcall(struct device *dev, struct uart_amba_port *
>>                 dev_info(uap->port.dev, "DMA channel RX %s\n",
>>                          dma_chan_name(uap->dmarx.chan));
>>         }
>> -}
>> -
>> -#ifndef MODULE
>> -/*
>> - * Stack up the UARTs and let the above initcall be done at device
>> - * initcall time, because the serial driver is called as an arch
>> - * initcall, and at this time the DMA subsystem is not yet registered.
>> - * At this point the driver will switch over to using DMA where desired.
>> - */
>> -struct dma_uap {
>> -       struct list_head node;
>> -       struct uart_amba_port *uap;
>> -       struct device *dev;
>> -};
>> -
>> -static LIST_HEAD(pl011_dma_uarts);
>> -
>> -static int __init pl011_dma_initcall(void)
>> -{
>> -       struct list_head *node, *tmp;
>> -
>> -       list_for_each_safe(node, tmp, &pl011_dma_uarts) {
>> -               struct dma_uap *dmau = list_entry(node, struct dma_uap, node);
>> -               pl011_dma_probe_initcall(dmau->dev, dmau->uap);
>> -               list_del(node);
>> -               kfree(dmau);
>> -       }
>> -       return 0;
>> -}
>> -
>> -device_initcall(pl011_dma_initcall);
>>
>> -static void pl011_dma_probe(struct device *dev, struct uart_amba_port *uap)
>> -{
>> -       struct dma_uap *dmau = kzalloc(sizeof(struct dma_uap), GFP_KERNEL);
>> -       if (dmau) {
>> -               dmau->uap = uap;
>> -               dmau->dev = dev;
>> -               list_add_tail(&dmau->node, &pl011_dma_uarts);
>> -       }
>> +       return;
> You don't need an explicit return.

yes, not sure how that came in.

>
>>  }
>> -#else
>> -static void pl011_dma_probe(struct device *dev, struct uart_amba_port *uap)
>> -{
>> -       pl011_dma_probe_initcall(dev, uap);
>> -}
>> -#endif
>> -interviewing
>>  static void pl011_dma_remove(struct uart_amba_port *uap)
>>  {
>>         /* TODO: remove the initcall if it has not yet executed */
>> @@ -1548,6 +1512,9 @@ static int pl011_hwinit(struct uart_port *port)
>>         uap->im = readw(uap->port.membase + UART011_IMSC);
>>         writew(UART011_RTIM | UART011_RXIM, uap->port.membase + UART011_IMSC);
>>
>> +       if (!uap->dma_probed)
>> +               pl011_dma_probe(uap->port.dev, uap);
>> +
>>         if (dev_get_platdata(uap->port.dev)) {
>>                 struct amba_pl011_data *plat;
>>
>> @@ -2218,7 +2185,6 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
>>         uap->port.ops = &amba_pl011_pops;
>>         uap->port.flags = UPF_BOOT_AUTOCONF;
>>         uap->port.line = i;
>> -       pl011_dma_probe(&dev->dev, uap);
>>
>>         /* Ensure interrupts from this UART are masked and cleared */
>>         writew(0, uap->port.membase + UART011_IMSC);
>> --
>> 1.9.1
>>

[PATCH v3] drivers/tty: amba: defer probing DMA availability until hw_init

[Rob Herring]

On Thu, Feb 26, 2015 at 9:15 AM, Jorge Ramirez-Ortiz
<jorge.ramirez-ortiz@linaro.org> wrote:
> On 02/26/2015 09:12 AM, Rob Herring wrote:
>> On Wed, Feb 25, 2015 at 6:31 PM, Jorge Ramirez-Ortiz
>> <jorge.ramirez-ortiz@linaro.org> wrote:
>>> Fix a race condition that happens when device_initcall(pl011_dma_initicall)
>>> is executed before all the devices have been probed - this issue was observed on
>>> an hisi_6220 SoC (HiKey board from Linaro).
>>>
>>> The deferred driver probing framework relies on late_initcall to trigger
>>> deferred probes so it is just possible that, even with a valid DMA driver ready
>>> to be loaded, we fail to synchronize with it.
>>>
>>> The proposed implementation delays probing of the DMA until the uart device is
>>> opened. As hw_init is invoked on port startup and port resume - but the DMA
>>> probe is only required once - we avoid calling multiple times using a new field
>>> in uart_amba_port to track this scenario.
>>>
>>> Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
>>> ---
>>>  drivers/tty/serial/amba-pl011.c | 66 ++++++++++-------------------------------
>>>  1 file changed, 16 insertions(+), 50 deletions(-)
>>>
>>> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
>>> index 8d94c19..3501ccc 100644
>>> --- a/drivers/tty/serial/amba-pl011.c
>>> +++ b/drivers/tty/serial/amba-pl011.c
>>> @@ -29,6 +29,7 @@
>>>   * and hooked into this driver.
>>>   */
>>>
>>> +#define pr_fmt(fmt) "amba-pl011: "fmt
>> You can drop this I think.
>
> why not keep it? it makes the trace more readable (if debug is enabled only)

There's only 1 call and it should be converted to dev_ variant
instead. It is also unrelated to this patch.

Rob