From mboxrd@z Thu Jan 1 00:00:00 1970 From: Fabio Estevam Date: Wed, 03 Jan 2018 18:14:35 +0000 Subject: Re: [PATCH v2 9/9] media: i2c: tw9910: Remove soc_camera dependencies Message-Id: List-Id: References: <1514469681-15602-1-git-send-email-jacopo+renesas@jmondi.org> <1514469681-15602-10-git-send-email-jacopo+renesas@jmondi.org> <20180103171347.GF9493@w540> <20180103173726.GH9493@w540> In-Reply-To: <20180103173726.GH9493@w540> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: jacopo mondi Cc: Jacopo Mondi , Laurent Pinchart , Magnus Damm , geert@glider.be, Mauro Carvalho Chehab , Hans Verkuil , Rob Herring , Mark Rutland , linux-renesas-soc@vger.kernel.org, linux-media , linux-sh@vger.kernel.org, "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" , linux-kernel On Wed, Jan 3, 2018 at 3:37 PM, jacopo mondi wrote: >> Initially the rest GPIO was doing: >> >> - gpio_set_value(GPIO_PTT3, 0); >> - mdelay(10); >> - gpio_set_value(GPIO_PTT3, 1); >> - mdelay(10); /* wait to let chip come out of reset */ > > And that's what my driver code does :) No, on 5/9 you converted the original code to: GPIO_LOOKUP("sh7722_pfc", GPIO_PTT3, "rstb", GPIO_ACTIVE_HIGH) It should be GPIO_ACTIVE_LOW instead. > My point is that if I read the manual and I see an active low gpio (0 > is reset state) then the driver code uses it as and active high one (1 > is the reset state), that would be weird to me. Then on this patch you should do: gpiod_set_value(priv->rstb_gpio, 1); ---> This tells the GPIO to go to its active state (In this case active = logic level 0) usleep_range(500, 1000); gpiod_set_value(priv->rstb_gpio, 0); ---> This tells the GPIO to go to its inactive state (In this case inactive = logic level 1) You can also look at Documentation/gpio/consumer.txt where the usage of the gpiod_xxx API is described. It seems you are confusing it with the legacy gpio_set_value() API (Documentation/gpio/gpio-legacy.txt) Hope this helps.