From mboxrd@z Thu Jan 1 00:00:00 1970
From: Philipp Zabel
Subject: Re: [PATCH v8] reset: Add driver for gpio-controlled reset pins
Date: Wed, 17 Jul 2013 09:17:04 +0200
Message-ID: <1374045424.4350.5.camel@pizza.hi.pengutronix.de>
References: <1369904940-716-1-git-send-email-p.zabel@pengutronix.de>
<20130716015038.GD28375@S2101-09.ap.freescale.net>
<20130716065130.GB30067@S2101-09.ap.freescale.net>
<51E56AF6.30506@wwwdotorg.org>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path:
In-Reply-To: <51E56AF6.30506-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
List-Unsubscribe: ,
List-Archive:
List-Post:
List-Help:
List-Subscribe: ,
Errors-To: devicetree-discuss-bounces+gldd-devicetree-discuss=m.gmane.org-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
Sender: "devicetree-discuss"
To: Stephen Warren
Cc: Marek Vasut , Fabio Estevam , Mike Turquette , Len Brown , Sascha Hauer , "Rafael J. Wysocki" , Pavel Machek , devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
List-Id: devicetree@vger.kernel.org
Am Dienstag, den 16.07.2013, 09:47 -0600 schrieb Stephen Warren:
> On 07/16/2013 12:51 AM, Shawn Guo wrote:
> > On Tue, Jul 16, 2013 at 09:50:42AM +0800, Shawn Guo wrote:
> >> Hi Philipp,
> >>
> >> On Thu, May 30, 2013 at 11:09:00AM +0200, Philipp Zabel wrote:
> >>> This driver implements a reset controller device that toggle a gpio
> >>> connected to a reset pin of a peripheral IC. The delay between assertion
> >>> and de-assertion of the reset signal can be configured via device tree.
> >>>
> >>> Signed-off-by: Philipp Zabel
> >>> Reviewed-by: Stephen Warren
> >>
> >> I see this patch is very useful, as GPIOs are widely used to reset
> >> components/devices on board. But I do not find the patch in v3.11-rc1.
> >> What's your plan about it?
> >>
> >> Also, I'm wondering if we should register the driver a little bit
> >> early. Please see the following patch. If it makes sense to you,
> >> I can send the patch to you, or you can just quash it into yours.
> >
> > And here is another change request.
>
> > diff --git a/drivers/reset/gpio-reset.c b/drivers/reset/gpio-reset.c
>
> > - gpio_set_value(drvdata->gpio, value);
> > + if (gpio_cansleep(drvdata->gpio))
> > + gpio_set_value_cansleep(drvdata->gpio, value);
> > + else
> > + gpio_set_value(drvdata->gpio, value);
>
> That's not right. Calling gpio_set_value() v.s.
> gpio_set_value_cansleep() should be based on the properties of the
> calling context, not the GPIO being controlled. In other words, if it's
> permissible to call gpio_set_value_cansleep() at this point in the code,
> simply always call that, and remove the conditional logic.
In which case I'd say let's just call the _cansleep variant here
unconditionally and declare that reset_control_assert/deassert() may
sleep, just as reset_control_reset() has to anyway.
regards
Philipp
From mboxrd@z Thu Jan 1 00:00:00 1970
From: p.zabel@pengutronix.de (Philipp Zabel)
Date: Wed, 17 Jul 2013 09:17:04 +0200
Subject: [PATCH v8] reset: Add driver for gpio-controlled reset pins
In-Reply-To: <51E56AF6.30506@wwwdotorg.org>
References: <1369904940-716-1-git-send-email-p.zabel@pengutronix.de>
<20130716015038.GD28375@S2101-09.ap.freescale.net>
<20130716065130.GB30067@S2101-09.ap.freescale.net>
<51E56AF6.30506@wwwdotorg.org>
Message-ID: <1374045424.4350.5.camel@pizza.hi.pengutronix.de>
To: linux-arm-kernel@lists.infradead.org
List-Id: linux-arm-kernel.lists.infradead.org
Am Dienstag, den 16.07.2013, 09:47 -0600 schrieb Stephen Warren:
> On 07/16/2013 12:51 AM, Shawn Guo wrote:
> > On Tue, Jul 16, 2013 at 09:50:42AM +0800, Shawn Guo wrote:
> >> Hi Philipp,
> >>
> >> On Thu, May 30, 2013 at 11:09:00AM +0200, Philipp Zabel wrote:
> >>> This driver implements a reset controller device that toggle a gpio
> >>> connected to a reset pin of a peripheral IC. The delay between assertion
> >>> and de-assertion of the reset signal can be configured via device tree.
> >>>
> >>> Signed-off-by: Philipp Zabel
> >>> Reviewed-by: Stephen Warren
> >>
> >> I see this patch is very useful, as GPIOs are widely used to reset
> >> components/devices on board. But I do not find the patch in v3.11-rc1.
> >> What's your plan about it?
> >>
> >> Also, I'm wondering if we should register the driver a little bit
> >> early. Please see the following patch. If it makes sense to you,
> >> I can send the patch to you, or you can just quash it into yours.
> >
> > And here is another change request.
>
> > diff --git a/drivers/reset/gpio-reset.c b/drivers/reset/gpio-reset.c
>
> > - gpio_set_value(drvdata->gpio, value);
> > + if (gpio_cansleep(drvdata->gpio))
> > + gpio_set_value_cansleep(drvdata->gpio, value);
> > + else
> > + gpio_set_value(drvdata->gpio, value);
>
> That's not right. Calling gpio_set_value() v.s.
> gpio_set_value_cansleep() should be based on the properties of the
> calling context, not the GPIO being controlled. In other words, if it's
> permissible to call gpio_set_value_cansleep() at this point in the code,
> simply always call that, and remove the conditional logic.
In which case I'd say let's just call the _cansleep variant here
unconditionally and declare that reset_control_assert/deassert() may
sleep, just as reset_control_reset() has to anyway.
regards
Philipp