Re: [U-Boot] [PATCH 14/23] gpio: Provide dummy get/request & is_valid functions

[Simon Glass]

Hi Paul,

On 30 September 2016 at 12:12, Paul Burton  wrote:
> On Monday, 26 September 2016 18:35:25 BST Simon Glass wrote:
>
>> Hi Paul,
>
>>
>
>> On 26 September 2016 at 12:29, Paul Burton  wrote:
>
>> > Allow for drivers to make use of driver model GPIOs when they're enabled
>
>> > & available without needing to #ifdef on CONFIG_DM_GPIO by providing
>
>> > dummy functions covering GPIO requests. Each will simply return -ENODEV
>
>> > or -EINVAL, depending upon which the real implementation returns when a
>
>> > GPIO isn't found. Only the driver model versions of the GPIO request
>
>> > functions are covered & dm_gpio_request is excluded since it's
>
>> > documented as only being of use for debugging, so drivers shouldn't be
>
>> > calling it anyway.
>
>> >
>
>> > Also provide a dummy dm_gpio_is_valid, with the idea that all other GPIO
>
>> > functions called would be within an if (dm_gpio_is_valid(...)) statement
>
>> > and have been optimised out in cases where that returns a compile-time
>
>> > constant false.
>
>> >
>
>> > This parallels the clock API, keeping the #ifdefs & checks in a single
>
>> > location allowing drivers or other code to use GPIOs without needing to
>
>> > perform such checks themselves.
>
>> >
>
>> > Signed-off-by: Paul Burton 
>
>> >
>
>> > ---
>
>> >
>
>> > include/asm-generic/gpio.h | 38 ++
>
>> > 1 file changed, 38 insertions(+)
>
>>
>
>> Ick - why not just define DM_GPIO? I don't think we want to provide
>
>> special support for legacy things. It is just confusing.
>
>>
>
>> - Simon
>
>
>
> Hi Simon,
>
>
>
> For the MIPS Boston system I already enable DM_GPIO, I went with this
> approach so that it wouldn't necessarily need to be enabled for the Crown
> Bay x86 system which is the other user of the pch_gbe driver.
>
>
>
> Would having pch_gbe depend on CONFIG_DM_GPIO be ok with you Bin? I see
> crownbay_defconfig enables CONFIG_CMD_GPIO but couldn't find any GPIO-using
> code at a glance. So hopefully it would just work without needing to change
> any existing crownbay code?

It seems OK to me, but in any case, DM_GPIO should be enabled for x86.

Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 14/23] gpio: Provide dummy get/request & is_valid functions

[Paul Burton]

On Monday, 26 September 2016 18:35:25 BST Simon Glass wrote:
> Hi Paul,
> 
> On 26 September 2016 at 12:29, Paul Burton  wrote:
> > Allow for drivers to make use of driver model GPIOs when they're enabled
> > & available without needing to #ifdef on CONFIG_DM_GPIO by providing
> > dummy functions covering GPIO requests. Each will simply return -ENODEV
> > or -EINVAL, depending upon which the real implementation returns when a
> > GPIO isn't found. Only the driver model versions of the GPIO request
> > functions are covered & dm_gpio_request is excluded since it's
> > documented as only being of use for debugging, so drivers shouldn't be
> > calling it anyway.
> > 
> > Also provide a dummy dm_gpio_is_valid, with the idea that all other GPIO
> > functions called would be within an if (dm_gpio_is_valid(...)) statement
> > and have been optimised out in cases where that returns a compile-time
> > constant false.
> > 
> > This parallels the clock API, keeping the #ifdefs & checks in a single
> > location allowing drivers or other code to use GPIOs without needing to
> > perform such checks themselves.
> > 
> > Signed-off-by: Paul Burton 
> > 
> > ---
> > 
> >  include/asm-generic/gpio.h | 38 ++
> >  1 file changed, 38 insertions(+)
> 
> Ick - why not just define DM_GPIO? I don't think we want to provide
> special support for legacy things. It is just confusing.
> 
> - Simon

Hi Simon,

For the MIPS Boston system I already enable DM_GPIO, I went with this approach 
so that it 
wouldn't necessarily need to be enabled for the Crown Bay x86 system which is 
the other 
user of the pch_gbe driver.

Would having pch_gbe depend on CONFIG_DM_GPIO be ok with you Bin? I see 
crownbay_defconfig enables CONFIG_CMD_GPIO but couldn't find any GPIO-using 
code at 
a glance. So hopefully it would just work without needing to change any 
existing crownbay 
code?

Thanks,
Paul


signature.asc
Description: This is a digitally signed message part.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 14/23] gpio: Provide dummy get/request & is_valid functions

[Simon Glass]

Hi Paul,

On 26 September 2016 at 12:29, Paul Burton  wrote:
> Allow for drivers to make use of driver model GPIOs when they're enabled
> & available without needing to #ifdef on CONFIG_DM_GPIO by providing
> dummy functions covering GPIO requests. Each will simply return -ENODEV
> or -EINVAL, depending upon which the real implementation returns when a
> GPIO isn't found. Only the driver model versions of the GPIO request
> functions are covered & dm_gpio_request is excluded since it's
> documented as only being of use for debugging, so drivers shouldn't be
> calling it anyway.
>
> Also provide a dummy dm_gpio_is_valid, with the idea that all other GPIO
> functions called would be within an if (dm_gpio_is_valid(...)) statement
> and have been optimised out in cases where that returns a compile-time
> constant false.
>
> This parallels the clock API, keeping the #ifdefs & checks in a single
> location allowing drivers or other code to use GPIOs without needing to
> perform such checks themselves.
>
> Signed-off-by: Paul Burton 
>
> ---
>
>  include/asm-generic/gpio.h | 38 ++
>  1 file changed, 38 insertions(+)

Ick - why not just define DM_GPIO? I don't think we want to provide
special support for legacy things. It is just confusing.

- Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 14/23] gpio: Provide dummy get/request & is_valid functions

[Paul Burton]

Allow for drivers to make use of driver model GPIOs when they're enabled
& available without needing to #ifdef on CONFIG_DM_GPIO by providing
dummy functions covering GPIO requests. Each will simply return -ENODEV
or -EINVAL, depending upon which the real implementation returns when a
GPIO isn't found. Only the driver model versions of the GPIO request
functions are covered & dm_gpio_request is excluded since it's
documented as only being of use for debugging, so drivers shouldn't be
calling it anyway.

Also provide a dummy dm_gpio_is_valid, with the idea that all other GPIO
functions called would be within an if (dm_gpio_is_valid(...)) statement
and have been optimised out in cases where that returns a compile-time
constant false.

This parallels the clock API, keeping the #ifdefs & checks in a single
location allowing drivers or other code to use GPIOs without needing to
perform such checks themselves.

Signed-off-by: Paul Burton 

---

 include/asm-generic/gpio.h | 38 ++
 1 file changed, 38 insertions(+)

diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
index 4aa0004..f6593a7 100644
--- a/include/asm-generic/gpio.h
+++ b/include/asm-generic/gpio.h
@@ -135,6 +135,9 @@ struct gpio_desc {
  */
 static inline bool dm_gpio_is_valid(const struct gpio_desc *desc)
 {
+   if (!IS_ENABLED(CONFIG_DM_GPIO))
+   return false;
+
return desc->dev != NULL;
 }
 
@@ -343,7 +346,14 @@ const char *gpio_get_bank_info(struct udevice *dev, int 
*offset_count);
  * @desc:  Returns description, on success
  * @return 0 if OK, -ve on error
  */
+#ifdef CONFIG_DM_GPIO
 int dm_gpio_lookup_name(const char *name, struct gpio_desc *desc);
+#else
+static inline int dm_gpio_lookup_name(const char *name, struct gpio_desc *desc)
+{
+   return -EINVAL;
+}
+#endif
 
 /**
  * gpio_lookup_name - Look up a GPIO name and return its details
@@ -356,8 +366,17 @@ int dm_gpio_lookup_name(const char *name, struct gpio_desc 
*desc);
  * @offsetp: Returns the offset number within this device
  * @gpiop: Returns the absolute GPIO number, numbered from 0
  */
+#ifdef CONFIG_DM_GPIO
 int gpio_lookup_name(const char *name, struct udevice **devp,
 unsigned int *offsetp, unsigned int *gpiop);
+#else
+static inline int
+gpio_lookup_name(const char *name, struct udevice **devp,
+unsigned int *offsetp, unsigned int *gpiop)
+{
+   return -EINVAL;
+}
+#endif
 
 /**
  * gpio_get_values_as_int() - Turn the values of a list of GPIOs into an int
@@ -428,8 +447,17 @@ int gpio_claim_vector(const int *gpio_num_array, const 
char *fmt);
  * something wrong with the list, or other -ve for another error (e.g.
  * -EBUSY if a GPIO was already requested)
  */
+#ifdef CONFIG_DM_GPIO
 int gpio_request_by_name(struct udevice *dev, const char *list_name,
 int index, struct gpio_desc *desc, int flags);
+#else
+static inline int
+gpio_request_by_name(struct udevice *dev, const char *list_name,
+int index, struct gpio_desc *desc, int flags)
+{
+   return -ENOENT;
+}
+#endif
 
 /**
  * gpio_request_list_by_name() - Request a list of GPIOs
@@ -452,9 +480,19 @@ int gpio_request_by_name(struct udevice *dev, const char 
*list_name,
  * @flags: Indicates the GPIO input/output settings (GPIOD_...)
  * @return number of GPIOs requested, or -ve on error
  */
+#ifdef CONFIG_DM_GPIO
 int gpio_request_list_by_name(struct udevice *dev, const char *list_name,
  struct gpio_desc *desc_list, int max_count,
  int flags);
+#else
+static inline int
+gpio_request_list_by_name(struct udevice *dev, const char *list_name,
+ struct gpio_desc *desc_list, int max_count,
+ int flags)
+{
+   return -ENOENT;
+}
+#endif
 
 /**
  * dm_gpio_request() - manually request a GPIO
-- 
2.10.0

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot