Message ID | 1535532253-9315-1-git-send-email-ley.foon.tan@intel.com |
---|---|
State | Superseded |
Delegated to: | Marek Vasut |
Headers | show |
Series | [U-Boot,v4] gpio: dwapb_gpio: Add reset ctrl to driver | expand |
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; > + } [...]
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
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
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
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.
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), };
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(-)