Hello Simon,
Am 12.07.2019 um 23:11 schrieb Simon Glass:
Hi Heiko,
On Wed, 26 Jun 2019 at 22:12, Heiko Schocher <h...@denx.de> wrote:
Hello Simon,
Am 26.06.2019 um 17:07 schrieb Simon Glass:
Hi Heiko,
On Mon, 24 Jun 2019 at 00:16, Heiko Schocher <h...@denx.de> wrote:
Hello Simon,
Am 22.06.2019 um 21:09 schrieb Simon Glass:
Hi Heiko,
On Wed, 12 Jun 2019 at 05:12, Heiko Schocher <h...@denx.de> wrote:
add gpio-hog support. GPIO hogging is a mechanism
providing automatic GPIO request and configuration
as part of the gpio-controller's driver probe function.
for more infos see:
doc/device-tree-bindings/gpio/gpio.txt
Signed-off-by: Heiko Schocher <h...@denx.de>
---
clean travis build, see result:
https://travis-ci.org/hsdenx/u-boot-test/builds/544040419
Changes in v4:
- rework as suggested by Patrick
based on patch:
http://patchwork.ozlabs.org/patch/1098913/
use new UCLASS_NOP
hog detection + DT parsing in gpio_post_bind() .... info saved in platdata
gpio configuration for hog (request and set_dir) => probe function
[..]
+config DM_GPIO_HOG
Can we drop the DM_ prefix? At some point DM_GPIO will go away and we
don't want to rename these sorts of options.
Hmm... we also have the DM_GPIO symbol, as the gpio hog only works with
DM_GPIO enabled, I thought DM_GPIO_HOG is the better choice, but of course
I can change this.
Yes I understand that, but in general we hope one day to rename
DM_GPIO to GPIO, once everything uses DM. That's why I am trying to
keep the DM_ suffix only for things that need it.
Yep, already dropped, see temp version (travisbuild clean):
https://github.com/hsdenx/u-boot-test/commit/509bad758b8abc94dbbebc2a387b9e97c30b6543
+ bool "Enable GPIO hog support"
+ depends on DM_GPIO
+ default n
+ help
+ Enable gpio hog support
+ The GPIO chip may contain GPIO hog definitions. GPIO hogging
+ is a mechanism providing automatic GPIO request and config-
+ uration as part of the gpio-controller's driver probe function.
+
config ALTERA_PIO
bool "Altera PIO driver"
depends on DM_GPIO
diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c
index da5e9ba6e5..308d0863ad 100644
--- a/drivers/gpio/gpio-uclass.c
+++ b/drivers/gpio/gpio-uclass.c
@@ -5,6 +5,9 @@
#include <common.h>
#include <dm.h>
+#include <dm/device-internal.h>
+#include <dm/lists.h>
+#include <dm/uclass-internal.h>
#include <dt-bindings/gpio/gpio.h>
#include <errno.h>
#include <fdtdec.h>
@@ -141,6 +144,118 @@ static int gpio_find_and_xlate(struct gpio_desc *desc,
return gpio_xlate_offs_flags(desc->dev, desc, args);
}
+#if defined(CONFIG_DM_GPIO_HOG)
+
+struct gpio_hog_priv {
+ struct gpio_desc gpiod;
As above I don't think you need this.
Hmm.. when probing I need to request the gpio with gpio_dev_request_index()
for which I need to pass a pointer to struct gpio_desc ...
The caller can declare this in a local variable.
The caller is always the device probe function, as gpio_hog_probe_all()
loops over all gpio hog devices and probes them with device_probe()
which cannot return a gpio_desc pointer, nor can such a pointer passed to.
Why isn't it valid to store in each gpio hog device the gpio desc it
belongs too in device private data?
I don't see any point. Your caller needs to store the pointer anyway,
or if not, could just as easily have a local variable with the info.
Also, passing a pointer to the struct is how we consistently do it so far.
What is the benefit of storing it in the callee rather than the caller?
And later I want to have the possibility to access the gpio for example
with dm_gpio_set_value() and need therefore the gpio descriptor, so I must
store it somewhere. Or do I overlook something obvious?
In that case the caller should store it, perhaps in a local variable,
or perhaps in a private data structure if needed for a long time.
[..]
Of course I can introduce a list for each gpio hog definition filled
when calling gpio_hog_probe_all() ... but than I have a second list
which uses memory, may has to keep in sync the list if device are
removed ... I want to avoid such an approach ... as we have already a
list of gpio hog devices and the priv storage pointer for device private
data.
I understand what you are saying about devices being removed, but
actually removable is fine, it's only unbinding that is dangerous. And
we already have that problem. We would need to add refcounts to fix
it, and given that bootloaders don't normally need to unbind, I have
not worried about it too much.
The approach I suggest would not use any more memory I think.
What do you think?
Well I still think it is better to have the caller own the struct.
+
+ return 0;
+}
+
+int gpio_hog_probe_all(void)
+{
+ struct udevice *dev;
+ int ret;
+
+ for (uclass_first_device(UCLASS_NOP, &dev);
Perhaps you should create a new uclass for this?
Hmm.. yes, I had an approach with a new uclass, but I thought, the new [1]
UCLASS_NOP is valid for this here to?
[1] http://patchwork.ozlabs.org/patch/1098913/
I see your point and I can see arguments for each side. But the fact
that you have to check the driver suggests to me that a new uclass is
better.
[..]
Ok with this approach the gpio_hog additions to gpio-uclass can be
moved to a gpio-hog-uclass.c file. Also the problem where to store
the gpio descriptor moves from device private data to uclass private
data?
I don't think you have explained why you actually need to store the
GPIO descriptor. Where do you use it later? Is it just the
gpio_hog_probe_all() function?
You can use it later! See example this patch adds in
doc/device-tree-bindings/gpio/gpio.txt:
+ struct gpio_desc *desc;
+ int ret;
+
+ ret = gpio_hog_lookup_name("boot_rescue", &desc);
+ if (ret)
+ return;
+ if (dm_gpio_get_value(desc) == 1)
+ printf("\nBooting into Rescue System\n");
+ else if (dm_gpio_get_value(desc) == 0)
+ printf("\nBoot normal\n");
So in this example, gpio "board_rescue" is defined through DTS and code
which uses this gpio is generic ... makes it nice to handle board
differences (as I have on the aristainetos board, which I rework
for DM support).
bye,
Heiko
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: h...@denx.de
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
