All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v4] gpio: dwapb_gpio: Add reset ctrl to driver
@ 2018-08-29  8:44 Ley Foon Tan
  2018-08-29 11:35 ` Marek Vasut
  0 siblings, 1 reply; 6+ messages in thread
From: Ley Foon Tan @ 2018-08-29  8:44 UTC (permalink / raw)
  To: u-boot

Add code to reset all reset signals as in gpio DT node. A reset property
is an optional feature, so only print out a warning and do not fail if a
reset property is not present.

If a reset property is discovered, then use it to deassert, thus
bringing the IP out of reset.

Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>

---
v4:
- Add struct gpio_dwapb_priv

v3:
- Add .remove function.
- Add error handling when return non-zero from reset_get_bulk().

v2:
- Move reset to probe() function.
---
 drivers/gpio/dwapb_gpio.c |   51 +++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/dwapb_gpio.c b/drivers/gpio/dwapb_gpio.c
index 7cf2d47..0b88384 100644
--- a/drivers/gpio/dwapb_gpio.c
+++ b/drivers/gpio/dwapb_gpio.c
@@ -15,6 +15,7 @@
 #include <dm/lists.h>
 #include <dm/root.h>
 #include <errno.h>
+#include <reset.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -29,6 +30,10 @@ DECLARE_GLOBAL_DATA_PTR;
 #define GPIO_PORTA_EOI		0x4c
 #define GPIO_EXT_PORT(p)	(0x50 + (p) * 4)
 
+struct gpio_dwapb_priv {
+	struct reset_ctl_bulk	resets;
+};
+
 struct gpio_dwapb_platdata {
 	const char	*name;
 	int		bank;
@@ -99,13 +104,42 @@ static const struct dm_gpio_ops gpio_dwapb_ops = {
 	.get_function		= dwapb_gpio_get_function,
 };
 
+static int gpio_dwapb_reset(struct udevice *dev)
+{
+	int ret;
+	struct gpio_dwapb_priv *priv = dev_get_priv(dev);
+
+	ret = reset_get_bulk(dev, &priv->resets);
+	if (ret) {
+		dev_warn(dev, "Can't get reset: %d\n", ret);
+		/* Return 0 if error due to !CONFIG_DM_RESET and reset
+		 * DT property is not present.
+		 */
+		if (ret == -ENOENT || ret == -ENOTSUPP)
+			return 0;
+		else
+			return ret;
+	}
+
+	ret = reset_deassert_bulk(&priv->resets);
+	if (ret) {
+		reset_release_bulk(&priv->resets);
+		dev_err(dev, "Failed to reset: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
 static int gpio_dwapb_probe(struct udevice *dev)
 {
 	struct gpio_dev_priv *priv = dev_get_uclass_priv(dev);
 	struct gpio_dwapb_platdata *plat = dev->platdata;
 
-	if (!plat)
-		return 0;
+	if (!plat) {
+		/* Reset on parent device only */
+		return gpio_dwapb_reset(dev);
+	}
 
 	priv->gpio_count = plat->pins;
 	priv->bank_name = plat->name;
@@ -166,6 +200,17 @@ err:
 	return ret;
 }
 
+static int gpio_dwapb_remove(struct udevice *dev)
+{
+	struct gpio_dwapb_platdata *plat = dev_get_platdata(dev);
+	struct gpio_dwapb_priv *priv = dev_get_priv(dev);
+
+	if (!plat && priv)
+		return reset_release_bulk(&priv->resets);
+
+	return 0;
+}
+
 static const struct udevice_id gpio_dwapb_ids[] = {
 	{ .compatible = "snps,dw-apb-gpio" },
 	{ }
@@ -178,4 +223,6 @@ U_BOOT_DRIVER(gpio_dwapb) = {
 	.ops		= &gpio_dwapb_ops,
 	.bind		= gpio_dwapb_bind,
 	.probe		= gpio_dwapb_probe,
+	.remove		= gpio_dwapb_remove,
+	.priv_auto_alloc_size   = sizeof(struct gpio_dwapb_priv),
 };
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH v4] gpio: dwapb_gpio: Add reset ctrl to driver
  2018-08-29  8:44 [U-Boot] [PATCH v4] gpio: dwapb_gpio: Add reset ctrl to driver Ley Foon Tan
@ 2018-08-29 11:35 ` Marek Vasut
  2018-08-30  0:45   ` Ley Foon Tan
  0 siblings, 1 reply; 6+ messages in thread
From: Marek Vasut @ 2018-08-29 11:35 UTC (permalink / raw)
  To: u-boot

On 08/29/2018 10:44 AM, Ley Foon Tan wrote:
> Add code to reset all reset signals as in gpio DT node. A reset property
> is an optional feature, so only print out a warning and do not fail if a
> reset property is not present.
> 
> If a reset property is discovered, then use it to deassert, thus
> bringing the IP out of reset.
> 
> Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>

This is much better.

> +static int gpio_dwapb_reset(struct udevice *dev)
> +{
> +	int ret;
> +	struct gpio_dwapb_priv *priv = dev_get_priv(dev);
> +
> +	ret = reset_get_bulk(dev, &priv->resets);
> +	if (ret) {
> +		dev_warn(dev, "Can't get reset: %d\n", ret);

Won't this barf on machines which either don't have DM_RESET enabled or
don't have it described in DT ?

> +		/* Return 0 if error due to !CONFIG_DM_RESET and reset
> +		 * DT property is not present.
> +		 */
> +		if (ret == -ENOENT || ret == -ENOTSUPP)
> +			return 0;
> +		else
> +			return ret;
> +	}
[...]

-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH v4] gpio: dwapb_gpio: Add reset ctrl to driver
  2018-08-29 11:35 ` Marek Vasut
@ 2018-08-30  0:45   ` Ley Foon Tan
  2018-08-30  9:07     ` Marek Vasut
  0 siblings, 1 reply; 6+ messages in thread
From: Ley Foon Tan @ 2018-08-30  0:45 UTC (permalink / raw)
  To: u-boot

On Wed, Aug 29, 2018 at 7:57 PM Marek Vasut <marex@denx.de> wrote:
>
> On 08/29/2018 10:44 AM, Ley Foon Tan wrote:
> > Add code to reset all reset signals as in gpio DT node. A reset property
> > is an optional feature, so only print out a warning and do not fail if a
> > reset property is not present.
> >
> > If a reset property is discovered, then use it to deassert, thus
> > bringing the IP out of reset.
> >
> > Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
>
> This is much better.
>
> > +static int gpio_dwapb_reset(struct udevice *dev)
> > +{
> > +     int ret;
> > +     struct gpio_dwapb_priv *priv = dev_get_priv(dev);
> > +
> > +     ret = reset_get_bulk(dev, &priv->resets);
> > +     if (ret) {
> > +             dev_warn(dev, "Can't get reset: %d\n", ret);
>
> Won't this barf on machines which either don't have DM_RESET enabled or
> don't have it described in DT ?
By default, dev_warn() is not show up. Or we can move this dev_warn
when "return ret" error below.
>
> > +             /* Return 0 if error due to !CONFIG_DM_RESET and reset
> > +              * DT property is not present.
> > +              */
> > +             if (ret == -ENOENT || ret == -ENOTSUPP)
> > +                     return 0;
> > +             else
> > +                     return ret;
> > +     }
> [...]
>
> --
> Best regards,
> Marek Vasut

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH v4] gpio: dwapb_gpio: Add reset ctrl to driver
  2018-08-30  0:45   ` Ley Foon Tan
@ 2018-08-30  9:07     ` Marek Vasut
  2018-09-04  1:34       ` Ley Foon Tan
  0 siblings, 1 reply; 6+ messages in thread
From: Marek Vasut @ 2018-08-30  9:07 UTC (permalink / raw)
  To: u-boot

On 08/30/2018 02:45 AM, Ley Foon Tan wrote:
> On Wed, Aug 29, 2018 at 7:57 PM Marek Vasut <marex@denx.de> wrote:
>>
>> On 08/29/2018 10:44 AM, Ley Foon Tan wrote:
>>> Add code to reset all reset signals as in gpio DT node. A reset property
>>> is an optional feature, so only print out a warning and do not fail if a
>>> reset property is not present.
>>>
>>> If a reset property is discovered, then use it to deassert, thus
>>> bringing the IP out of reset.
>>>
>>> Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
>>
>> This is much better.
>>
>>> +static int gpio_dwapb_reset(struct udevice *dev)
>>> +{
>>> +     int ret;
>>> +     struct gpio_dwapb_priv *priv = dev_get_priv(dev);
>>> +
>>> +     ret = reset_get_bulk(dev, &priv->resets);
>>> +     if (ret) {
>>> +             dev_warn(dev, "Can't get reset: %d\n", ret);
>>
>> Won't this barf on machines which either don't have DM_RESET enabled or
>> don't have it described in DT ?
> By default, dev_warn() is not show up. Or we can move this dev_warn
> when "return ret" error below.

I think that'd be better. If there is no reset support, no point in
warning. Or what do you think ?

btw you could then also flatten the indent with some
ret = reset....
if (ret == -ENOENT || ....)
 return 0;

dev_warn();
return ret;

>>> +             /* Return 0 if error due to !CONFIG_DM_RESET and reset
>>> +              * DT property is not present.
>>> +              */
>>> +             if (ret == -ENOENT || ret == -ENOTSUPP)
>>> +                     return 0;
>>> +             else
>>> +                     return ret;
>>> +     }
>> [...]
>>
>> --
>> Best regards,
>> Marek Vasut


-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH v4] gpio: dwapb_gpio: Add reset ctrl to driver
  2018-08-30  9:07     ` Marek Vasut
@ 2018-09-04  1:34       ` Ley Foon Tan
  2018-09-04  4:34         ` Marek Vasut
  0 siblings, 1 reply; 6+ messages in thread
From: Ley Foon Tan @ 2018-09-04  1:34 UTC (permalink / raw)
  To: u-boot

On Thu, Aug 30, 2018 at 8:11 PM Marek Vasut <marex@denx.de> wrote:
>
> On 08/30/2018 02:45 AM, Ley Foon Tan wrote:
> > On Wed, Aug 29, 2018 at 7:57 PM Marek Vasut <marex@denx.de> wrote:
> >>
> >> On 08/29/2018 10:44 AM, Ley Foon Tan wrote:
> >>> Add code to reset all reset signals as in gpio DT node. A reset property
> >>> is an optional feature, so only print out a warning and do not fail if a
> >>> reset property is not present.
> >>>
> >>> If a reset property is discovered, then use it to deassert, thus
> >>> bringing the IP out of reset.
> >>>
> >>> Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
> >>
> >> This is much better.
> >>
> >>> +static int gpio_dwapb_reset(struct udevice *dev)
> >>> +{
> >>> +     int ret;
> >>> +     struct gpio_dwapb_priv *priv = dev_get_priv(dev);
> >>> +
> >>> +     ret = reset_get_bulk(dev, &priv->resets);
> >>> +     if (ret) {
> >>> +             dev_warn(dev, "Can't get reset: %d\n", ret);
> >>
> >> Won't this barf on machines which either don't have DM_RESET enabled or
> >> don't have it described in DT ?
> > By default, dev_warn() is not show up. Or we can move this dev_warn
> > when "return ret" error below.
>
> I think that'd be better. If there is no reset support, no point in
> warning. Or what do you think ?
>
> btw you could then also flatten the indent with some
> ret = reset....
> if (ret == -ENOENT || ....)
>  return 0;
>
> dev_warn();
> return ret;

Okay, will change this.

Regards
Ley Foon

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH v4] gpio: dwapb_gpio: Add reset ctrl to driver
  2018-09-04  1:34       ` Ley Foon Tan
@ 2018-09-04  4:34         ` Marek Vasut
  0 siblings, 0 replies; 6+ messages in thread
From: Marek Vasut @ 2018-09-04  4:34 UTC (permalink / raw)
  To: u-boot

On 09/04/2018 03:34 AM, Ley Foon Tan wrote:
> On Thu, Aug 30, 2018 at 8:11 PM Marek Vasut <marex@denx.de> wrote:
>>
>> On 08/30/2018 02:45 AM, Ley Foon Tan wrote:
>>> On Wed, Aug 29, 2018 at 7:57 PM Marek Vasut <marex@denx.de> wrote:
>>>>
>>>> On 08/29/2018 10:44 AM, Ley Foon Tan wrote:
>>>>> Add code to reset all reset signals as in gpio DT node. A reset property
>>>>> is an optional feature, so only print out a warning and do not fail if a
>>>>> reset property is not present.
>>>>>
>>>>> If a reset property is discovered, then use it to deassert, thus
>>>>> bringing the IP out of reset.
>>>>>
>>>>> Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
>>>>
>>>> This is much better.
>>>>
>>>>> +static int gpio_dwapb_reset(struct udevice *dev)
>>>>> +{
>>>>> +     int ret;
>>>>> +     struct gpio_dwapb_priv *priv = dev_get_priv(dev);
>>>>> +
>>>>> +     ret = reset_get_bulk(dev, &priv->resets);
>>>>> +     if (ret) {
>>>>> +             dev_warn(dev, "Can't get reset: %d\n", ret);
>>>>
>>>> Won't this barf on machines which either don't have DM_RESET enabled or
>>>> don't have it described in DT ?
>>> By default, dev_warn() is not show up. Or we can move this dev_warn
>>> when "return ret" error below.
>>
>> I think that'd be better. If there is no reset support, no point in
>> warning. Or what do you think ?
>>
>> btw you could then also flatten the indent with some
>> ret = reset....
>> if (ret == -ENOENT || ....)
>>  return 0;
>>
>> dev_warn();
>> return ret;
> 
> Okay, will change this.

OK, I'll queue it into -next then, thanks.

-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-09-04  4:34 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-29  8:44 [U-Boot] [PATCH v4] gpio: dwapb_gpio: Add reset ctrl to driver Ley Foon Tan
2018-08-29 11:35 ` Marek Vasut
2018-08-30  0:45   ` Ley Foon Tan
2018-08-30  9:07     ` Marek Vasut
2018-09-04  1:34       ` Ley Foon Tan
2018-09-04  4:34         ` Marek Vasut

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.