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?
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.
What do you think?
+
+ 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?
Is it worth?
+/**
+ * gpio_hog_probe_all() - probe all gpio devices with
+ * gpio-hog subnodes.
+ *
+ * @return: Returns return value from device_probe()
What if one fails and the others succeed?
Currently I return with error code from device_probe(). So the
rest of the gpio-hogs are not probed.
Hmm.. may it is better to emmit a debug message and continue?
Sounds reasonable. But it seems to me that if there is an error there
should be some visible report, so this could perhaps return the error
code? Then the caller could print a messag?
Ok, so I add a printf message. I like more to add it in
gpio_hog_probe_all(), because more than one gpio_hog can fail.
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
