Problem with usage of pwdn-gpios in sensor drivers

Hello,

In the DT for sensors, one can declare a pwdn-gpios GPIO. I thought that pwdn stands for PoWerDowN, i.e. when I set the GPIO to 1, the sensor is powered down, shut down, not powered anymore. But when I look at the *_power_on() methods in the sensor drivers in kernel/nvidia, I see that they terminate after a call to gpio_set_value(pw->pwdn_gpio, 1);, which sets the GPIO to 1 and thus should power down the sensor.

Is that gpio misnamed or does pwdn not mean ‘power down’ or are the pwdn pins on all supported boards actually in ACTIVE_LOW logic ? If pwdn is actually in ACTIVE_LOW logic, maybe we could simply name it ‘pwup’, for PoWerUP, and admit that it is a ACTIVE_HIGH signal.

Thanks

hello phdm,

please don’t be confused with the pin name, you shall check the value for the actual high/low.
thanks

Hello JerryChang,

Do you confirm that the pin name is wrong ?

hello phdm,

no, I mean it’s actually a GPIO pin,
for example,
$L4T_Sources/r32.5/Linux_for_Tegra/source/public/hardware/nvidia/platform/t18x/common/kernel-dts/t18x-common-platforms/tegra186-quill-camera-imx274-dual.dtsi

#define CAM0_PWDN	TEGRA_MAIN_GPIO(R, 0)
#define CAM1_PWDN	TEGRA_MAIN_GPIO(L, 6)

That pin is called PWDN, but we must set it to 1 to power up the sensor.

The logic would be either

  • to call it PWUP
  • to tell in the DT it is GPIO_ACTIVE_LOW, and use gpiod_set_value(gpio, 0)
    in the driver to power the sensor up and gpiod_set_value(gpio, 1) to
    power the sensor down.

I struggled with that because on the custom boards that I use, the power
gpio is a GPIO_ACTIVE_LOW, which means if I set the pin to 0, the sensor
is powered, but if I set the power gpio to 1, the sensor is powered down.

Unfortunately the drivers in kernel/nvidia do not look at the GPIO_ACTIVE_LOW
part of the description in the DT. They should use devm_gpiod_get
instead of of_get_named_gpio to parse the DT, and gpiod_set_value
instead of gpio_set_value (No ‘d’) to set the value.

hello phdm,

it’s actually a GPIO pin, which depends-on your device tree definition,
so, please have your own DT property customization. thanks

Hello JerryChang,

hello phdm,

it’s actually a GPIO pin, which depends-on your device tree definition,
so, please have your own DT property customization. thanks

That’s what I do, of course, but I had a hard time discovering that the
‘GPIO_ACTIVE_LOW’ clause in the DT is not taken into account by the drivers
in kernel/nvidia, because they use the deprecated ‘gpio_set_value’ function
which sets directly the physical value of the pin, instead of the
‘gpiod_set_value’ function that sets the physical value of the pin by
combining the logical value set by the user and the GPIO_ACTIVE_LOW or
GPIO_ACTIVE_HIGH flag of the DT.

Kind regards

hello phdm,

may I know which JetPack release you’re used?

please also share the code snippet about the issue,
it would be helpful if we could reproduce this locally.
thanks

hello phdm,

may I know which JetPack release you’re used?

JetPack-4.3

please also share the code snippet about the issue,
it would be helpful if we could reproduce this locally.
thanks

Replace the GPIO_ACTIVE_HIGH in pwdn-gpios entry of your sensor,
by GPIO_ACTIVE_LOW, and you will notice that that does not have
any effect.

To take into account the GPIO_ACTIVE_LOW flag, one must replace
pwdn_gpio = of_get_named_gpio(node, “pwdn-gpios”, 0);
by
pwdn_gpio = devm_gpiod_get(dev, “pwdn”, GPIOD_OUT_HIGH);

and
gpio_set_value(pw->pwdn_gpio, 1);
by
gpiod_set_value(pw->pwdn_gpio, 1);

and also change the type of pwdn_gpio from
unsigned int pwdn_gpio;
to
struct gpio_desc *pwdn_gpio;

The same applies for the other gpio’s like reset-gpio

And finally, because the gpio is called pwdn-gpios, one must call
gpio_set_value(pw->pwdn_gpio, 1);
to power down the sensor,
and
gpio_set_value(pw->pwdn_gpio, 0);
to power up the sensor.

Kind regards

hello phdm,

for your reference, here’s my steps to check GPIO pin.
this pin is used for ov5693 sensor driver.
i.e. reset-gpios = <&tegra_main_gpio TEGRA194_MAIN_GPIO(H, 4) GPIO_ACTIVE_HIGH>;

you may examine the pin state before and after camera streaming,
for example,
before streaming.

# cat /sys/kernel/debug/gpio | grep 348
 gpio-348 (                    |cam_reset_gpio      ) out lo

after streaming.

# cat /sys/kernel/debug/gpio | grep 348
 gpio-348 (                    |cam_reset_gpio      ) out hi

I have question regard to your code snippet.
why you’re having different kernel API to fetch the GPIO pin, are you having “pwdn” define in the sensor device tree?

No, devm_gpiod_get automatically adds “-gpios” to the string given in parameter.
So it looks here at “pwdn-gpios”.

You are correct. Most of the sensor drivers and examples use the deprecated gpio_set_value function and ignore your GPIO flags from your device tree. Unless the driver uses the new gpiod APIs it will not gracefully and automatically use your ACTIVE LOW/HIGH flag from your Device Tree. There are some kernel drivers that do use the older gpio APIs and do read the flags and swap the logic depending on the flags, but not many. If you need to utilize the active high or active low logic from the same compiled kernel driver (depending on your device tree), you will have to update the driver to use gpiod APIs. However, this is not too difficult of a task even for someone that is new to kernel driver development.

And yes there are many things that are often misnamed as you translate from the electrical world to the software world, not to mention through the evolution of a design. Active low signals are often represented with a bar over top of them on schematics. Software folks don’t always get what this means and have no bar over top convention to name symbols, variables, etc. Some try conventions like _SIGNAL or nSIGNAL, but these are not widely used and not everyone even understands this convention so it does not get used regularly.