| Message ID | 20160926182917.27531-15-paul.burton@imgtec.com |
|---|---|
| State | Deferred |
| Delegated to: | Daniel Schwierzeck |
| Headers | show |
Hi Paul, On 26 September 2016 at 12:29, Paul Burton <paul.burton@imgtec.com> 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 <paul.burton@imgtec.com> > > --- > > 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
On Monday, 26 September 2016 18:35:25 BST Simon Glass wrote: > Hi Paul, > > On 26 September 2016 at 12:29, Paul Burton <paul.burton@imgtec.com> 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 <paul.burton@imgtec.com> > > > > --- > > > > 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
Hi Paul, On 30 September 2016 at 12:12, Paul Burton <paul.burton@imgtec.com> wrote: > On Monday, 26 September 2016 18:35:25 BST Simon Glass wrote: > >> Hi Paul, > >> > >> On 26 September 2016 at 12:29, Paul Burton <paul.burton@imgtec.com> 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 <paul.burton@imgtec.com> > >> > > >> > --- > >> > > >> > 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
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
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 <paul.burton@imgtec.com> --- include/asm-generic/gpio.h | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+)