Hello Simon,
Am 26.06.2019 um 17:07 schrieb Simon Glass:
Hi Heiko,
On Mon, 24 Jun 2019 at 00:16, Heiko Schocher hs@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 hs@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 hs@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/509bad758b8abc94dbbebc2a387b9e9...
bool "Enable GPIO hog support"depends on DM_GPIOdefault nhelpEnable gpio hog supportThe GPIO chip may contain GPIO hog definitions. GPIO hoggingis 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?
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