From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Pinchart Subject: Re: [PATCH 3/5] media: ov2640: add primary dt support Date: Tue, 09 Dec 2014 10:07:47 +0200 Message-ID: <4259882.xUnFntZnaF@avalon> References: <1418038147-13221-1-git-send-email-josh.wu@atmel.com> <13013762.Jqm1jQRnFM@avalon> <54866809.7020402@atmel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7Bit Return-path: In-Reply-To: <54866809.7020402-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org> Sender: devicetree-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Josh Wu Cc: linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, m.chehab-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, g.liakhovetski-Mmb7MZpHnFY@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-Id: devicetree@vger.kernel.org Hi Josh, On Tuesday 09 December 2014 11:10:01 Josh Wu wrote: > On 12/9/2014 2:39 AM, Laurent Pinchart wrote: > > Hi Josh, > > > > Thank you for the patch. > > > > On Monday 08 December 2014 19:29:05 Josh Wu wrote: > >> Add device tree support for ov2640. > >> > >> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > >> Signed-off-by: Josh Wu > >> --- > >> > >> v1 -> v2: > >> 1. use gpiod APIs. > >> 2. change the gpio pin's name according to datasheet. > >> 3. reduce the delay for .reset() function. > >> > >> drivers/media/i2c/soc_camera/ov2640.c | 86 +++++++++++++++++++++++++--- > >> 1 file changed, 80 insertions(+), 6 deletions(-) > >> > >> diff --git a/drivers/media/i2c/soc_camera/ov2640.c > >> b/drivers/media/i2c/soc_camera/ov2640.c index 9ee910d..2a57979 100644 > >> --- a/drivers/media/i2c/soc_camera/ov2640.c > >> +++ b/drivers/media/i2c/soc_camera/ov2640.c [snip] > >> +static int ov2640_probe_dt(struct i2c_client *client, > >> + struct ov2640_priv *priv) > >> +{ > >> + priv->resetb_gpio = devm_gpiod_get_optional(&client->dev, "resetb", > >> + GPIOD_OUT_HIGH); > >> + if (!priv->resetb_gpio) > >> + dev_warn(&client->dev, "resetb gpio not found!\n"); > > > > No need to warn here, it's perfectly fine if the reset signal isn't > > connected to a GPIO. > > I want to print some information if no GPIO is assigned. So I'd like use > dev_dbg() here. > What do you feel? If you want to print a message in that case dev_dbg is the most appropriate log level. I would skip it completely, but that's up to you. > >> + else if (IS_ERR(priv->resetb_gpio)) > >> + return -EINVAL; > >> + > >> + priv->pwdn_gpio = devm_gpiod_get_optional(&client->dev, "pwdn", > >> + GPIOD_OUT_HIGH); > >> + if (!priv->pwdn_gpio) > >> + dev_warn(&client->dev, "pwdn gpio not found!\n"); > > > > Same here. > > ditto. > > >> + else if (IS_ERR(priv->pwdn_gpio)) > >> + return -EINVAL; > >> + > >> + /* Initialize the soc_camera_subdev_desc */ > >> + priv->ssdd_dt.power = ov2640_hw_power; > >> + priv->ssdd_dt.reset = ov2640_hw_reset; > >> + client->dev.platform_data = &priv->ssdd_dt; > >> + > >> + return 0; > >> +} > >> + > >> /* > >> * i2c_driver functions > >> */ -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html