Re: [PATCH v2 9/9] media: i2c: tw9910: Remove soc_camera dependencies

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Hi Fabio,

On Wed, Jan 03, 2018 at 03:27:53PM -0200, Fabio Estevam wrote:
> Hi Jacopo,
>
> On Wed, Jan 3, 2018 at 3:13 PM, jacopo mondi <jacopo@xxxxxxxxxx> wrote:
>
> > That would be true if I would have declared the GPIO to be ACTIVE_LOW.
> > See patch [5/9] in this series and search for "rstb". The GPIO (which
> > is shared between two devices) is said to be active high...
>
> Just looked at your patch 5/9 and it seems it only works because you
> made two inversions :-)
>
> 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 :)

>
> So this is an active low reset.
>

Indeed

> So you should have converted it to:
>
> GPIO_LOOKUP("sh7722_pfc", GPIO_PTT3, "rstb", GPIO_ACTIVE_LOW),
>
> and then in this patch you should do as I said earlier:
>
> gpiod_set_value(priv->rstb_gpio, 1);
> usleep_range(500, 1000);
> gpiod_set_value(priv->rstb_gpio, 0);

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.

But maybe that's just me, and if that's common practice, I'll happly
change this!

Thanks
   j

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux