All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
To: Liam Girdwood <lgirdwood@gmail.com>, Mark Brown <broonie@kernel.org>
Cc: "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>
Subject: Question about
Date: Tue, 18 Nov 2014 17:00:09 +0200	[thread overview]
Message-ID: <546B5EF9.3080102@mentor.com> (raw)

Hello,

I need to set a GPIO (active high) output high on boot, which enables a
power rail supplying some external devices.

I have a question regarding "regulator-boot-on" and "enable-active-high"
fixed regulator device tree properties (actually AFAIU it applies to
gpio regulator as well, by the way, which one is proper to use in my
situation?)

Here is what we have from the code:

  [...]
  constraints->boot_on = of_property_read_bool(np, "regulator-boot-on");
  [...]
  if (init_data->constraints.boot_on)
          config->enabled_at_boot = true;
  [...]
  config->enable_high = of_property_read_bool(np, "enable-active-high");
  [...]
  cfg.ena_gpio_invert = !config->enable_high;
  if (config->enabled_at_boot) {
          if (config->enable_high)
                  cfg.ena_gpio_flags |= GPIOF_OUT_INIT_HIGH;
          else
                  cfg.ena_gpio_flags |= GPIOF_OUT_INIT_LOW;
  } else {
          if (config->enable_high)
                  cfg.ena_gpio_flags |= GPIOF_OUT_INIT_LOW;
          else
                 cfg.ena_gpio_flags |= GPIOF_OUT_INIT_HIGH;
  }
  [...]
  ret = gpio_request_one(config->ena_gpio,
                         GPIOF_DIR_OUT | config->ena_gpio_flags,
                         rdev_get_name(rdev));
  [...]
  /* Enable GPIO at initial use */
  if (pin->enable_count == 0)
          gpiod_set_value_cansleep(pin->gpiod,
                                  !pin->ena_gpio_invert);
  [...]


If we simplify the matter by assuming GPIOF_OUT_INIT_LOW is inverted
GPIOF_OUT_INIT_HIGH and vice versa, then it is easy to compute by
running over the variants that GPIO output value is set HIGH (regardless
of GPIO active low status) if and only if "regulator-boot-on" is
provided and "enable-active-high" has no effect at all.

This fact confuses me, because from the general regulator and fixed
regulator device tree bindings documentation I get:

  [...]
  - regulator-boot-on: bootloader/firmware enabled regulator
  [...]
  - enable-active-high: Polarity of GPIO is Active high
  If this property is missing, the default assumed is Active low.
  [...]

According to the documentation I'd assume that "regulator-boot-on" does
not touch gpio output value setting (so, if it is controlled by
bootloader or firmware, then it might be out of Linux kernel control).
Also my impression of "enable-active-high" property is that is should
have some effect on the GPIO output value (but it is not, see above),
and actually I don't quite understand why this property exists - there
is a high chance that "enable-active-high" and the real GPIO polarity do
not coincide, it should be more reliable to get GPIO flags of a
particular GPIO right in the regulator driver/framework.

Let's consider two possible configurations:

| regulator-boot-on | enable-active-high | GPIO polarity | GPIO output |
+-------------------+--------------------+---------------+-------------+
|        no         |         yes        |  active high  |    low      |
|        no         |          no        |  active low   |   high      |

I'd rather think that both resulting GPIO outputs are incorrect or
better to say do not correspond to my perception of "regulator-boot-on"
and "enable-active-high" DTS properties described in the documentation,
however above "enable-active-high" and actual GPIO polarity are the same
(when they are not, it is another open topic for discussion).

Do I miss something or have a mistake? Is there a problem in the
implemented logic?

Should documentation be updated to reflect "regulator-boot-on" role that
a regulator is re-enabled by the kernel?

Should "enable-active-high" be replaced by getting GPIO flags directly?

Thank you in advance.

--
With best wishes,
Vladimir

WARNING: multiple messages have this Message-ID (diff)
From: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
To: Liam Girdwood <lgirdwood@gmail.com>, Mark Brown <broonie@kernel.org>
Cc: "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	linux-kernel@vger.kernel.org
Subject: Question about
Date: Tue, 18 Nov 2014 17:00:09 +0200	[thread overview]
Message-ID: <546B5EF9.3080102@mentor.com> (raw)

Hello,

I need to set a GPIO (active high) output high on boot, which enables a
power rail supplying some external devices.

I have a question regarding "regulator-boot-on" and "enable-active-high"
fixed regulator device tree properties (actually AFAIU it applies to
gpio regulator as well, by the way, which one is proper to use in my
situation?)

Here is what we have from the code:

  [...]
  constraints->boot_on = of_property_read_bool(np, "regulator-boot-on");
  [...]
  if (init_data->constraints.boot_on)
          config->enabled_at_boot = true;
  [...]
  config->enable_high = of_property_read_bool(np, "enable-active-high");
  [...]
  cfg.ena_gpio_invert = !config->enable_high;
  if (config->enabled_at_boot) {
          if (config->enable_high)
                  cfg.ena_gpio_flags |= GPIOF_OUT_INIT_HIGH;
          else
                  cfg.ena_gpio_flags |= GPIOF_OUT_INIT_LOW;
  } else {
          if (config->enable_high)
                  cfg.ena_gpio_flags |= GPIOF_OUT_INIT_LOW;
          else
                 cfg.ena_gpio_flags |= GPIOF_OUT_INIT_HIGH;
  }
  [...]
  ret = gpio_request_one(config->ena_gpio,
                         GPIOF_DIR_OUT | config->ena_gpio_flags,
                         rdev_get_name(rdev));
  [...]
  /* Enable GPIO at initial use */
  if (pin->enable_count == 0)
          gpiod_set_value_cansleep(pin->gpiod,
                                  !pin->ena_gpio_invert);
  [...]


If we simplify the matter by assuming GPIOF_OUT_INIT_LOW is inverted
GPIOF_OUT_INIT_HIGH and vice versa, then it is easy to compute by
running over the variants that GPIO output value is set HIGH (regardless
of GPIO active low status) if and only if "regulator-boot-on" is
provided and "enable-active-high" has no effect at all.

This fact confuses me, because from the general regulator and fixed
regulator device tree bindings documentation I get:

  [...]
  - regulator-boot-on: bootloader/firmware enabled regulator
  [...]
  - enable-active-high: Polarity of GPIO is Active high
  If this property is missing, the default assumed is Active low.
  [...]

According to the documentation I'd assume that "regulator-boot-on" does
not touch gpio output value setting (so, if it is controlled by
bootloader or firmware, then it might be out of Linux kernel control).
Also my impression of "enable-active-high" property is that is should
have some effect on the GPIO output value (but it is not, see above),
and actually I don't quite understand why this property exists - there
is a high chance that "enable-active-high" and the real GPIO polarity do
not coincide, it should be more reliable to get GPIO flags of a
particular GPIO right in the regulator driver/framework.

Let's consider two possible configurations:

| regulator-boot-on | enable-active-high | GPIO polarity | GPIO output |
+-------------------+--------------------+---------------+-------------+
|        no         |         yes        |  active high  |    low      |
|        no         |          no        |  active low   |   high      |

I'd rather think that both resulting GPIO outputs are incorrect or
better to say do not correspond to my perception of "regulator-boot-on"
and "enable-active-high" DTS properties described in the documentation,
however above "enable-active-high" and actual GPIO polarity are the same
(when they are not, it is another open topic for discussion).

Do I miss something or have a mistake? Is there a problem in the
implemented logic?

Should documentation be updated to reflect "regulator-boot-on" role that
a regulator is re-enabled by the kernel?

Should "enable-active-high" be replaced by getting GPIO flags directly?

Thank you in advance.

--
With best wishes,
Vladimir

             reply	other threads:[~2014-11-18 15:00 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-18 15:00 Vladimir Zapolskiy [this message]
2014-11-18 15:00 ` Question about Vladimir Zapolskiy
2014-11-19 14:38 ` Question about fixed regulator DT properties Vladimir Zapolskiy
2014-11-19 14:38   ` Vladimir Zapolskiy
2014-11-25 12:17   ` Mark Brown
2014-11-25 12:17     ` Mark Brown
2014-11-26 17:27     ` Vladimir Zapolskiy
2014-11-26 17:27       ` Vladimir Zapolskiy
2014-11-26 17:53       ` Mark Brown
2014-11-26 19:13         ` Vladimir Zapolskiy
2014-11-26 19:13           ` Vladimir Zapolskiy
2014-11-26 19:20           ` Mark Brown
2014-11-26 19:20             ` Mark Brown
2014-11-26 19:57             ` Vladimir Zapolskiy
2014-11-26 19:57               ` Vladimir Zapolskiy
2014-11-26 20:36               ` Mark Brown
  -- strict thread matches above, loose matches on Subject: below --
2013-03-24 20:29 question about Kevin Wilson
2013-03-25  2:57 ` Moritz Fischer
2013-03-25 17:22   ` Rami Rosen
2010-05-28 11:46 Dan Carpenter
2010-05-28 13:42 ` Haojian Zhuang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=546B5EF9.3080102@mentor.com \
    --to=vladimir_zapolskiy@mentor.com \
    --cc=broonie@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.