[PATCH 1/2] dt-bindings: pl330: document the optional resets property

[PATCH 1/2] dt-bindings: pl330: document the optional resets property

[Dinh Nguyen]

Add the optional resets property the pl330 dma node.

Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
---
 Documentation/devicetree/bindings/dma/arm-pl330.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/dma/arm-pl330.txt b/Documentation/devicetree/bindings/dma/arm-pl330.txt
index db7e2260f9c5..2c7fd1941abb 100644
--- a/Documentation/devicetree/bindings/dma/arm-pl330.txt
+++ b/Documentation/devicetree/bindings/dma/arm-pl330.txt
@@ -16,6 +16,9 @@ Optional properties:
   - dma-channels: contains the total number of DMA channels supported by the DMAC
   - dma-requests: contains the total number of DMA requests supported by the DMAC
   - arm,pl330-broken-no-flushp: quirk for avoiding to execute DMAFLUSHP
+  - resets: contains an entry for each entry in reset-names.
+	    See ../reset/reset.txt for details.
+  - reset-names: must contain at least "dma", and optional is "dma-ocp".
 
 Example:
 
-- 
2.20.0

[PATCH 2/2] dmagengine: pl330: add code to get reset property

[Dinh Nguyen]

The DMA controller on some SoCs can be held in reset, and thus requires
the reset signal(s) to deasserted. Most SoCs will have just one reset
signal, but there are others, i.e. Arria10/Stratix10 will have an
additional reset signal, referred to as the OCP.

Add code to get the reset property from the device tree for deassert and
assert.

Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
---
 drivers/dma/pl330.c | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index 6e6837214210..6018c43e785d 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -29,6 +29,7 @@
 #include <linux/err.h>
 #include <linux/pm_runtime.h>
 #include <linux/bug.h>
+#include <linux/reset.h>
 
 #include "dmaengine.h"
 #define PL330_MAX_CHAN		8
@@ -500,6 +501,9 @@ struct pl330_dmac {
 	unsigned int num_peripherals;
 	struct dma_pl330_chan *peripherals; /* keep at end */
 	int quirks;
+
+	struct reset_control	*rstc;
+	struct reset_control	*rstc_ocp;
 };
 
 static struct pl330_of_quirks {
@@ -3028,6 +3032,30 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id)
 
 	amba_set_drvdata(adev, pl330);
 
+	pl330->rstc = devm_reset_control_get_optional(&adev->dev, "dma");
+	if (IS_ERR(pl330->rstc)) {
+		dev_err(&adev->dev, "No reset controller specified.\n");
+		return PTR_ERR(pl330->rstc);
+	} else {
+		ret = reset_control_deassert(pl330->rstc);
+		if (ret) {
+			dev_err(&adev->dev, "Couldn't deassert the device from reset!\n");
+			return ret;
+		}
+	}
+
+	pl330->rstc_ocp = devm_reset_control_get_optional(&adev->dev, "dma-ocp");
+	if (IS_ERR(pl330->rstc_ocp)) {
+		dev_err(&adev->dev, "No reset controller specified.\n");
+		return PTR_ERR(pl330->rstc_ocp);
+	} else {
+		ret = reset_control_deassert(pl330->rstc_ocp);
+		if (ret) {
+			dev_err(&adev->dev, "Couldn't deassert the device from OCP reset!\n");
+			return ret;
+		}
+	}
+
 	for (i = 0; i < AMBA_NR_IRQS; i++) {
 		irq = adev->irq[i];
 		if (irq) {
@@ -3168,6 +3196,11 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id)
 probe_err2:
 	pl330_del(pl330);
 
+	if (pl330->rstc_ocp)
+		reset_control_assert(pl330->rstc_ocp);
+
+	if (pl330->rstc)
+		reset_control_assert(pl330->rstc);
 	return ret;
 }
 
@@ -3206,6 +3239,11 @@ static int pl330_remove(struct amba_device *adev)
 
 	pl330_del(pl330);
 
+	if (pl330->rstc_ocp)
+		reset_control_assert(pl330->rstc_ocp);
+
+	if (pl330->rstc)
+		reset_control_assert(pl330->rstc);
 	return 0;
 }
 
-- 
2.20.0

Re: [PATCH 2/2] dmagengine: pl330: add code to get reset property

[Vinod Koul]

On 23-05-19, 19:28, Dinh Nguyen wrote:
> The DMA controller on some SoCs can be held in reset, and thus requires
> the reset signal(s) to deasserted. Most SoCs will have just one reset
> signal, but there are others, i.e. Arria10/Stratix10 will have an
> additional reset signal, referred to as the OCP.
> 
> Add code to get the reset property from the device tree for deassert and
> assert.
> 
> Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
> ---
>  drivers/dma/pl330.c | 38 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
> 
> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
> index 6e6837214210..6018c43e785d 100644
> --- a/drivers/dma/pl330.c
> +++ b/drivers/dma/pl330.c
> @@ -29,6 +29,7 @@
>  #include <linux/err.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/bug.h>
> +#include <linux/reset.h>
>  
>  #include "dmaengine.h"
>  #define PL330_MAX_CHAN		8
> @@ -500,6 +501,9 @@ struct pl330_dmac {
>  	unsigned int num_peripherals;
>  	struct dma_pl330_chan *peripherals; /* keep at end */
>  	int quirks;
> +
> +	struct reset_control	*rstc;
> +	struct reset_control	*rstc_ocp;
>  };
>  
>  static struct pl330_of_quirks {
> @@ -3028,6 +3032,30 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id)
>  
>  	amba_set_drvdata(adev, pl330);
>  
> +	pl330->rstc = devm_reset_control_get_optional(&adev->dev, "dma");
> +	if (IS_ERR(pl330->rstc)) {
> +		dev_err(&adev->dev, "No reset controller specified.\n");

Wasnt this optional??

> +		return PTR_ERR(pl330->rstc);
> +	} else {
> +		ret = reset_control_deassert(pl330->rstc);
> +		if (ret) {
> +			dev_err(&adev->dev, "Couldn't deassert the device from reset!\n");
> +			return ret;
> +		}
> +	}
> +
> +	pl330->rstc_ocp = devm_reset_control_get_optional(&adev->dev, "dma-ocp");
> +	if (IS_ERR(pl330->rstc_ocp)) {
> +		dev_err(&adev->dev, "No reset controller specified.\n");
> +		return PTR_ERR(pl330->rstc_ocp);
> +	} else {
> +		ret = reset_control_deassert(pl330->rstc_ocp);
> +		if (ret) {
> +			dev_err(&adev->dev, "Couldn't deassert the device from OCP reset!\n");
> +			return ret;
> +		}
> +	}
> +
>  	for (i = 0; i < AMBA_NR_IRQS; i++) {
>  		irq = adev->irq[i];
>  		if (irq) {
> @@ -3168,6 +3196,11 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id)
>  probe_err2:
>  	pl330_del(pl330);
>  
> +	if (pl330->rstc_ocp)
> +		reset_control_assert(pl330->rstc_ocp);
> +
> +	if (pl330->rstc)
> +		reset_control_assert(pl330->rstc);
>  	return ret;
>  }
>  
> @@ -3206,6 +3239,11 @@ static int pl330_remove(struct amba_device *adev)
>  
>  	pl330_del(pl330);
>  
> +	if (pl330->rstc_ocp)
> +		reset_control_assert(pl330->rstc_ocp);
> +
> +	if (pl330->rstc)
> +		reset_control_assert(pl330->rstc);
>  	return 0;
>  }
>  
> -- 
> 2.20.0

-- 
~Vinod

Re: [PATCH 2/2] dmagengine: pl330: add code to get reset property

[Dinh Nguyen]

Hi Vinod,

On 6/4/19 7:14 AM, Vinod Koul wrote:
> On 23-05-19, 19:28, Dinh Nguyen wrote:
>> The DMA controller on some SoCs can be held in reset, and thus requires
>> the reset signal(s) to deasserted. Most SoCs will have just one reset
>> signal, but there are others, i.e. Arria10/Stratix10 will have an
>> additional reset signal, referred to as the OCP.
>>
>> Add code to get the reset property from the device tree for deassert and
>> assert.
>>
>> Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
>> ---
>>  drivers/dma/pl330.c | 38 ++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 38 insertions(+)
>>
>> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
>> index 6e6837214210..6018c43e785d 100644
>> --- a/drivers/dma/pl330.c
>> +++ b/drivers/dma/pl330.c
>> @@ -29,6 +29,7 @@
>>  #include <linux/err.h>
>>  #include <linux/pm_runtime.h>
>>  #include <linux/bug.h>
>> +#include <linux/reset.h>
>>  
>>  #include "dmaengine.h"
>>  #define PL330_MAX_CHAN		8
>> @@ -500,6 +501,9 @@ struct pl330_dmac {
>>  	unsigned int num_peripherals;
>>  	struct dma_pl330_chan *peripherals; /* keep at end */
>>  	int quirks;
>> +
>> +	struct reset_control	*rstc;
>> +	struct reset_control	*rstc_ocp;
>>  };
>>  
>>  static struct pl330_of_quirks {
>> @@ -3028,6 +3032,30 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id)
>>  
>>  	amba_set_drvdata(adev, pl330);
>>  
>> +	pl330->rstc = devm_reset_control_get_optional(&adev->dev, "dma");
>> +	if (IS_ERR(pl330->rstc)) {
>> +		dev_err(&adev->dev, "No reset controller specified.\n");
> 
> Wasnt this optional??

Yes, this is optional. The call devm_reset_control_get_optional() will
just return NULL if the reset property is not there, but an error
pointer if something really went wrong. Thus, I'm using IS_ERR() for the
error checking.

Dinh

Re: [PATCH 2/2] dmagengine: pl330: add code to get reset property

[Geert Uytterhoeven]

Hi Dinh,

On Tue, Jun 4, 2019 at 4:21 PM Dinh Nguyen <dinguyen@kernel.org> wrote:
> On 6/4/19 7:14 AM, Vinod Koul wrote:
> > On 23-05-19, 19:28, Dinh Nguyen wrote:
> >> The DMA controller on some SoCs can be held in reset, and thus requires
> >> the reset signal(s) to deasserted. Most SoCs will have just one reset
> >> signal, but there are others, i.e. Arria10/Stratix10 will have an
> >> additional reset signal, referred to as the OCP.
> >>
> >> Add code to get the reset property from the device tree for deassert and
> >> assert.
> >>
> >> Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
> >> ---
> >>  drivers/dma/pl330.c | 38 ++++++++++++++++++++++++++++++++++++++
> >>  1 file changed, 38 insertions(+)
> >>
> >> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
> >> index 6e6837214210..6018c43e785d 100644
> >> --- a/drivers/dma/pl330.c
> >> +++ b/drivers/dma/pl330.c
> >> @@ -29,6 +29,7 @@
> >>  #include <linux/err.h>
> >>  #include <linux/pm_runtime.h>
> >>  #include <linux/bug.h>
> >> +#include <linux/reset.h>
> >>
> >>  #include "dmaengine.h"
> >>  #define PL330_MAX_CHAN              8
> >> @@ -500,6 +501,9 @@ struct pl330_dmac {
> >>      unsigned int num_peripherals;
> >>      struct dma_pl330_chan *peripherals; /* keep at end */
> >>      int quirks;
> >> +
> >> +    struct reset_control    *rstc;
> >> +    struct reset_control    *rstc_ocp;
> >>  };
> >>
> >>  static struct pl330_of_quirks {
> >> @@ -3028,6 +3032,30 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id)
> >>
> >>      amba_set_drvdata(adev, pl330);
> >>
> >> +    pl330->rstc = devm_reset_control_get_optional(&adev->dev, "dma");
> >> +    if (IS_ERR(pl330->rstc)) {
> >> +            dev_err(&adev->dev, "No reset controller specified.\n");
> >
> > Wasnt this optional??
>
> Yes, this is optional. The call devm_reset_control_get_optional() will
> just return NULL if the reset property is not there, but an error
> pointer if something really went wrong. Thus, I'm using IS_ERR() for the
> error checking.

So the error message is incorrect, as this is a real error condition?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 2/2] dmagengine: pl330: add code to get reset property

[Dinh Nguyen]

Hi Geert,

On 6/4/19 11:31 AM, Geert Uytterhoeven wrote:
> Hi Dinh,
> 
> On Tue, Jun 4, 2019 at 4:21 PM Dinh Nguyen <dinguyen@kernel.org> wrote:
>> On 6/4/19 7:14 AM, Vinod Koul wrote:
>>> On 23-05-19, 19:28, Dinh Nguyen wrote:
>>>> The DMA controller on some SoCs can be held in reset, and thus requires
>>>> the reset signal(s) to deasserted. Most SoCs will have just one reset
>>>> signal, but there are others, i.e. Arria10/Stratix10 will have an
>>>> additional reset signal, referred to as the OCP.
>>>>
>>>> Add code to get the reset property from the device tree for deassert and
>>>> assert.
>>>>
>>>> Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
>>>> ---
>>>>  drivers/dma/pl330.c | 38 ++++++++++++++++++++++++++++++++++++++
>>>>  1 file changed, 38 insertions(+)
>>>>
>>>> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
>>>> index 6e6837214210..6018c43e785d 100644
>>>> --- a/drivers/dma/pl330.c
>>>> +++ b/drivers/dma/pl330.c
>>>> @@ -29,6 +29,7 @@
>>>>  #include <linux/err.h>
>>>>  #include <linux/pm_runtime.h>
>>>>  #include <linux/bug.h>
>>>> +#include <linux/reset.h>
>>>>
>>>>  #include "dmaengine.h"
>>>>  #define PL330_MAX_CHAN              8
>>>> @@ -500,6 +501,9 @@ struct pl330_dmac {
>>>>      unsigned int num_peripherals;
>>>>      struct dma_pl330_chan *peripherals; /* keep at end */
>>>>      int quirks;
>>>> +
>>>> +    struct reset_control    *rstc;
>>>> +    struct reset_control    *rstc_ocp;
>>>>  };
>>>>
>>>>  static struct pl330_of_quirks {
>>>> @@ -3028,6 +3032,30 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id)
>>>>
>>>>      amba_set_drvdata(adev, pl330);
>>>>
>>>> +    pl330->rstc = devm_reset_control_get_optional(&adev->dev, "dma");
>>>> +    if (IS_ERR(pl330->rstc)) {
>>>> +            dev_err(&adev->dev, "No reset controller specified.\n");
>>>
>>> Wasnt this optional??
>>
>> Yes, this is optional. The call devm_reset_control_get_optional() will
>> just return NULL if the reset property is not there, but an error
>> pointer if something really went wrong. Thus, I'm using IS_ERR() for the
>> error checking.
> 
> So the error message is incorrect, as this is a real error condition?
> 

Yes, you're right! Will correct in V2.

Dinh

Re: [PATCH 2/2] dmagengine: pl330: add code to get reset property

[Dinh Nguyen]

Hi Geert,

On 6/5/19 9:41 AM, Dinh Nguyen wrote:
> Hi Geert,
> 
> On 6/4/19 11:31 AM, Geert Uytterhoeven wrote:
>> Hi Dinh,
>>
>> On Tue, Jun 4, 2019 at 4:21 PM Dinh Nguyen <dinguyen@kernel.org> wrote:
>>> On 6/4/19 7:14 AM, Vinod Koul wrote:
>>>> On 23-05-19, 19:28, Dinh Nguyen wrote:
>>>>> The DMA controller on some SoCs can be held in reset, and thus requires
>>>>> the reset signal(s) to deasserted. Most SoCs will have just one reset
>>>>> signal, but there are others, i.e. Arria10/Stratix10 will have an
>>>>> additional reset signal, referred to as the OCP.
>>>>>
>>>>> Add code to get the reset property from the device tree for deassert and
>>>>> assert.
>>>>>
>>>>> Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
>>>>> ---
>>>>>  drivers/dma/pl330.c | 38 ++++++++++++++++++++++++++++++++++++++
>>>>>  1 file changed, 38 insertions(+)
>>>>>
>>>>> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
>>>>> index 6e6837214210..6018c43e785d 100644
>>>>> --- a/drivers/dma/pl330.c
>>>>> +++ b/drivers/dma/pl330.c
>>>>> @@ -29,6 +29,7 @@
>>>>>  #include <linux/err.h>
>>>>>  #include <linux/pm_runtime.h>
>>>>>  #include <linux/bug.h>
>>>>> +#include <linux/reset.h>
>>>>>
>>>>>  #include "dmaengine.h"
>>>>>  #define PL330_MAX_CHAN              8
>>>>> @@ -500,6 +501,9 @@ struct pl330_dmac {
>>>>>      unsigned int num_peripherals;
>>>>>      struct dma_pl330_chan *peripherals; /* keep at end */
>>>>>      int quirks;
>>>>> +
>>>>> +    struct reset_control    *rstc;
>>>>> +    struct reset_control    *rstc_ocp;
>>>>>  };
>>>>>
>>>>>  static struct pl330_of_quirks {
>>>>> @@ -3028,6 +3032,30 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id)
>>>>>
>>>>>      amba_set_drvdata(adev, pl330);
>>>>>
>>>>> +    pl330->rstc = devm_reset_control_get_optional(&adev->dev, "dma");
>>>>> +    if (IS_ERR(pl330->rstc)) {
>>>>> +            dev_err(&adev->dev, "No reset controller specified.\n");
>>>>
>>>> Wasnt this optional??
>>>
>>> Yes, this is optional. The call devm_reset_control_get_optional() will
>>> just return NULL if the reset property is not there, but an error
>>> pointer if something really went wrong. Thus, I'm using IS_ERR() for the
>>> error checking.
>>
>> So the error message is incorrect, as this is a real error condition?
>>
> 
> Yes, you're right! Will correct in V2.

Looking at this again, I think the error message is correct. The
optional call will return NULL if the resets property is not specified,
and will return an error pointer if the reset propert is specified, but
the pointer to the reset controller is not found.

So I think the error message is correct.

Dinh

Re: [PATCH 2/2] dmagengine: pl330: add code to get reset property

[Geert Uytterhoeven]

Hi Dinh,

On Wed, Jun 5, 2019 at 5:31 PM Dinh Nguyen <dinguyen@kernel.org> wrote:
> On 6/5/19 9:41 AM, Dinh Nguyen wrote:
> > On 6/4/19 11:31 AM, Geert Uytterhoeven wrote:
> >> On Tue, Jun 4, 2019 at 4:21 PM Dinh Nguyen <dinguyen@kernel.org> wrote:
> >>> On 6/4/19 7:14 AM, Vinod Koul wrote:
> >>>> On 23-05-19, 19:28, Dinh Nguyen wrote:
> >>>>> The DMA controller on some SoCs can be held in reset, and thus requires
> >>>>> the reset signal(s) to deasserted. Most SoCs will have just one reset
> >>>>> signal, but there are others, i.e. Arria10/Stratix10 will have an
> >>>>> additional reset signal, referred to as the OCP.
> >>>>>
> >>>>> Add code to get the reset property from the device tree for deassert and
> >>>>> assert.
> >>>>>
> >>>>> Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>

> >>>>> --- a/drivers/dma/pl330.c
> >>>>> +++ b/drivers/dma/pl330.c

> >>>>> @@ -3028,6 +3032,30 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id)
> >>>>>
> >>>>>      amba_set_drvdata(adev, pl330);
> >>>>>
> >>>>> +    pl330->rstc = devm_reset_control_get_optional(&adev->dev, "dma");
> >>>>> +    if (IS_ERR(pl330->rstc)) {
> >>>>> +            dev_err(&adev->dev, "No reset controller specified.\n");

"No reset controller specified.\n"

> >>>>
> >>>> Wasnt this optional??
> >>>
> >>> Yes, this is optional. The call devm_reset_control_get_optional() will
> >>> just return NULL if the reset property is not there, but an error
> >>> pointer if something really went wrong. Thus, I'm using IS_ERR() for the
> >>> error checking.
> >>
> >> So the error message is incorrect, as this is a real error condition?
> >
> > Yes, you're right! Will correct in V2.
>
> Looking at this again, I think the error message is correct. The
> optional call will return NULL if the resets property is not specified,
> and will return an error pointer if the reset propert is specified, but
> the pointer to the reset controller is not found.
>
> So I think the error message is correct.

Please reread the error message, and what you wrote above.

Error message: "No reset controller specified".
Rationale: NULL (i.e. no error) if "the resets property is not specified".

If an error pointer is returned, this may be due to probe deferral (to be
propagated, but further ignored), or due to a real failure.

So IMHO the code should read:

        if (IS_ERR(pl330->rstc)) {
                if (PTR_ERR(pl330->rstc) != -EPROBE_DEFER)
                        dev_err(&adev->dev, "Failed to get reset.\n");
                return PTR_ERR(pl330->rstc);
        } else { ... }

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 2/2] dmagengine: pl330: add code to get reset property

[Dinh Nguyen]

Hi Geert,

On 6/7/19 2:37 AM, Geert Uytterhoeven wrote:
> Hi Dinh,
> 
> On Wed, Jun 5, 2019 at 5:31 PM Dinh Nguyen <dinguyen@kernel.org> wrote:
>> On 6/5/19 9:41 AM, Dinh Nguyen wrote:
>>> On 6/4/19 11:31 AM, Geert Uytterhoeven wrote:
>>>> On Tue, Jun 4, 2019 at 4:21 PM Dinh Nguyen <dinguyen@kernel.org> wrote:
>>>>> On 6/4/19 7:14 AM, Vinod Koul wrote:
>>>>>> On 23-05-19, 19:28, Dinh Nguyen wrote:
>>>>>>> The DMA controller on some SoCs can be held in reset, and thus requires
>>>>>>> the reset signal(s) to deasserted. Most SoCs will have just one reset
>>>>>>> signal, but there are others, i.e. Arria10/Stratix10 will have an
>>>>>>> additional reset signal, referred to as the OCP.
>>>>>>>
>>>>>>> Add code to get the reset property from the device tree for deassert and
>>>>>>> assert.
>>>>>>>
>>>>>>> Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
> 
>>>>>>> --- a/drivers/dma/pl330.c
>>>>>>> +++ b/drivers/dma/pl330.c
> 
>>>>>>> @@ -3028,6 +3032,30 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id)
>>>>>>>
>>>>>>>      amba_set_drvdata(adev, pl330);
>>>>>>>
>>>>>>> +    pl330->rstc = devm_reset_control_get_optional(&adev->dev, "dma");
>>>>>>> +    if (IS_ERR(pl330->rstc)) {
>>>>>>> +            dev_err(&adev->dev, "No reset controller specified.\n");
> 
> "No reset controller specified.\n"
> 
>>>>>>
>>>>>> Wasnt this optional??
>>>>>
>>>>> Yes, this is optional. The call devm_reset_control_get_optional() will
>>>>> just return NULL if the reset property is not there, but an error
>>>>> pointer if something really went wrong. Thus, I'm using IS_ERR() for the
>>>>> error checking.
>>>>
>>>> So the error message is incorrect, as this is a real error condition?
>>>
>>> Yes, you're right! Will correct in V2.
>>
>> Looking at this again, I think the error message is correct. The
>> optional call will return NULL if the resets property is not specified,
>> and will return an error pointer if the reset propert is specified, but
>> the pointer to the reset controller is not found.
>>
>> So I think the error message is correct.
> 
> Please reread the error message, and what you wrote above.
> 
> Error message: "No reset controller specified".
> Rationale: NULL (i.e. no error) if "the resets property is not specified".
> 
> If an error pointer is returned, this may be due to probe deferral (to be
> propagated, but further ignored), or due to a real failure.
> 
> So IMHO the code should read:
> 
>         if (IS_ERR(pl330->rstc)) {
>                 if (PTR_ERR(pl330->rstc) != -EPROBE_DEFER)
>                         dev_err(&adev->dev, "Failed to get reset.\n");
>                 return PTR_ERR(pl330->rstc);
>         } else { ... }
> 

You're right! Will update in v2.

Thanks!

Dinh