* [PATCH 1/5] pinctrl: Add core function pinmux_gpio_get_direction
2018-06-27 11:48 [PATCH 0/5] pinctrl_gpio_get_direction & ingenic fixes Paul Cercueil
@ 2018-06-27 11:49 ` Paul Cercueil
2018-06-27 11:49 ` [PATCH 2/5] pinctrl: Add API function pinctrl_gpio_get_direction Paul Cercueil
` (4 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Paul Cercueil @ 2018-06-27 11:49 UTC (permalink / raw)
To: Linus Walleij; +Cc: linux-gpio, linux-kernel, Paul Cercueil
Right now there is a core function to set the direction (input or output)
of a GPIO, pinmux_gpio_set_direction(). This function is provided to the
consumers with two wrappers, pinctrl_gpio_direction_input() /
pinctrl_gpio_direction_output(). Typically, these functions are used
within GPIO drivers, where the .direction_input/.direction_output
callbacks of the "gpio_chip" simply call into these pinctrl functions.
However, until now there was no corresponding pinmux_get_direction()
core function (nor pinctrl_gpio_get_direction), which means that it was
not possible to call into the pinctrl subsystem for the .get_direction
callback of the "gpio_chip".
Here, we introduce the pinmux_gpio_get_direction() core function, as
well as the .gpio_get_direction callback in the pinmux_ops structure.
Now it's up to the pinctrl drivers to implement that callback.
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
drivers/pinctrl/pinmux.c | 17 +++++++++++++++++
drivers/pinctrl/pinmux.h | 3 +++
include/linux/pinctrl/pinmux.h | 5 +++++
3 files changed, 25 insertions(+)
diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
index b8e9bda8ec98..e502a4f5be6f 100644
--- a/drivers/pinctrl/pinmux.c
+++ b/drivers/pinctrl/pinmux.c
@@ -291,6 +291,23 @@ int pinmux_gpio_direction(struct pinctrl_dev *pctldev,
return ret;
}
+int pinmux_gpio_get_direction(struct pinctrl_dev *pctldev,
+ struct pinctrl_gpio_range *range,
+ unsigned int pin)
+{
+ const struct pinmux_ops *ops;
+ int ret;
+
+ ops = pctldev->desc->pmxops;
+
+ if (ops->gpio_get_direction)
+ ret = ops->gpio_get_direction(pctldev, range, pin);
+ else
+ ret = -ENXIO;
+
+ return ret;
+}
+
static int pinmux_func_name_to_selector(struct pinctrl_dev *pctldev,
const char *function)
{
diff --git a/drivers/pinctrl/pinmux.h b/drivers/pinctrl/pinmux.h
index a331fcdbedd9..8cdab9e71870 100644
--- a/drivers/pinctrl/pinmux.h
+++ b/drivers/pinctrl/pinmux.h
@@ -24,6 +24,9 @@ void pinmux_free_gpio(struct pinctrl_dev *pctldev, unsigned pin,
int pinmux_gpio_direction(struct pinctrl_dev *pctldev,
struct pinctrl_gpio_range *range,
unsigned pin, bool input);
+int pinmux_gpio_get_direction(struct pinctrl_dev *pctldev,
+ struct pinctrl_gpio_range *range,
+ unsigned pin);
int pinmux_map_to_setting(const struct pinctrl_map *map,
struct pinctrl_setting *setting);
diff --git a/include/linux/pinctrl/pinmux.h b/include/linux/pinctrl/pinmux.h
index ace60d775b20..b3e73deffb49 100644
--- a/include/linux/pinctrl/pinmux.h
+++ b/include/linux/pinctrl/pinmux.h
@@ -56,6 +56,8 @@ struct pinctrl_dev;
* depending on whether the GPIO is configured as input or output,
* a direction selector function may be implemented as a backing
* to the GPIO controllers that need pin muxing.
+ * @gpio_get_direction: Return the direction (input or output) of the given
+ * GPIO.
* @strict: do not allow simultaneous use of the same pin for GPIO and another
* function. Check both gpio_owner and mux_owner strictly before approving
* the pin request.
@@ -82,6 +84,9 @@ struct pinmux_ops {
struct pinctrl_gpio_range *range,
unsigned offset,
bool input);
+ int (*gpio_get_direction) (struct pinctrl_dev *pctrldev,
+ struct pinctrl_gpio_range *range,
+ unsigned int offset);
bool strict;
};
--
2.11.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/5] pinctrl: Add API function pinctrl_gpio_get_direction
2018-06-27 11:48 [PATCH 0/5] pinctrl_gpio_get_direction & ingenic fixes Paul Cercueil
2018-06-27 11:49 ` [PATCH 1/5] pinctrl: Add core function pinmux_gpio_get_direction Paul Cercueil
@ 2018-06-27 11:49 ` Paul Cercueil
2018-06-27 14:07 ` kbuild test robot
2018-06-27 15:24 ` kbuild test robot
2018-06-27 11:49 ` [PATCH 3/5] pinctrl: ingenic: Fix inverted direction for < JZ4770 Paul Cercueil
` (3 subsequent siblings)
5 siblings, 2 replies; 15+ messages in thread
From: Paul Cercueil @ 2018-06-27 11:49 UTC (permalink / raw)
To: Linus Walleij; +Cc: linux-gpio, linux-kernel, Paul Cercueil
This function can be used to retrieve the direction (input/output) of a
given GPIO.
It should *ONLY* be used from gpiolib-based GPIO drivers, as part of their
gpio_get_direction() semantics, platforms and individual drivers shall
*NOT* touch pin control GPIO calls.
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
drivers/pinctrl/core.c | 31 +++++++++++++++++++++++++++++++
include/linux/pinctrl/consumer.h | 1 +
2 files changed, 32 insertions(+)
diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index e5a303002021..7fee8347fb51 100644
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -849,6 +849,37 @@ int pinctrl_gpio_direction_output(unsigned gpio)
EXPORT_SYMBOL_GPL(pinctrl_gpio_direction_output);
/**
+ * pinctrl_gpio_get_direction() - Get the direction (input/output) of a GPIO pin
+ * @gpio: the GPIO pin number from the GPIO subsystem number space
+ *
+ * This function should *ONLY* be used from gpiolib-based GPIO drivers,
+ * as part of their gpio_get_direction() semantics, platforms and individual
+ * drivers shall *NOT* touch pin control GPIO calls.
+ */
+int pinctrl_gpio_get_direction(unsigned int gpio)
+{
+ struct pinctrl_dev *pctldev;
+ struct pinctrl_gpio_range *range;
+ int ret;
+ int pin;
+
+ ret = pinctrl_get_device_gpio_range(gpio, &pctldev, &range);
+ if (ret)
+ return ret;
+
+ mutex_lock(&pctldev->mutex);
+
+ /* Convert to the pin controllers number space */
+ pin = gpio_to_pin(range, gpio);
+ ret = pinmux_gpio_get_direction(pctldev, range, pin);
+
+ mutex_unlock(&pctldev->mutex);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(pinctrl_gpio_get_direction);
+
+/**
* pinctrl_gpio_set_config() - Apply config to given GPIO pin
* @gpio: the GPIO pin number from the GPIO subsystem number space
* @config: the configuration to apply to the GPIO
diff --git a/include/linux/pinctrl/consumer.h b/include/linux/pinctrl/consumer.h
index 0412cc9833e9..5f945c329f1c 100644
--- a/include/linux/pinctrl/consumer.h
+++ b/include/linux/pinctrl/consumer.h
@@ -29,6 +29,7 @@ extern int pinctrl_gpio_request(unsigned gpio);
extern void pinctrl_gpio_free(unsigned gpio);
extern int pinctrl_gpio_direction_input(unsigned gpio);
extern int pinctrl_gpio_direction_output(unsigned gpio);
+extern int pinctrl_gpio_get_direction(unsigned gpio);
extern int pinctrl_gpio_set_config(unsigned gpio, unsigned long config);
extern struct pinctrl * __must_check pinctrl_get(struct device *dev);
--
2.11.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 2/5] pinctrl: Add API function pinctrl_gpio_get_direction
2018-06-27 11:49 ` [PATCH 2/5] pinctrl: Add API function pinctrl_gpio_get_direction Paul Cercueil
@ 2018-06-27 14:07 ` kbuild test robot
2018-06-27 15:24 ` kbuild test robot
1 sibling, 0 replies; 15+ messages in thread
From: kbuild test robot @ 2018-06-27 14:07 UTC (permalink / raw)
To: Paul Cercueil
Cc: kbuild-all, Linus Walleij, linux-gpio, linux-kernel, Paul Cercueil
[-- Attachment #1: Type: text/plain, Size: 2405 bytes --]
Hi Paul,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on pinctrl/devel]
[also build test ERROR on v4.18-rc2 next-20180627]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Paul-Cercueil/pinctrl-Add-core-function-pinmux_gpio_get_direction/20180627-201609
base: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git devel
config: x86_64-randconfig-x002-201825 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
drivers//pinctrl/core.c: In function 'pinctrl_gpio_get_direction':
>> drivers//pinctrl/core.c:873:8: error: implicit declaration of function 'pinmux_gpio_get_direction'; did you mean 'pinmux_gpio_direction'? [-Werror=implicit-function-declaration]
ret = pinmux_gpio_get_direction(pctldev, range, pin);
^~~~~~~~~~~~~~~~~~~~~~~~~
pinmux_gpio_direction
cc1: some warnings being treated as errors
vim +873 drivers//pinctrl/core.c
849
850 /**
851 * pinctrl_gpio_get_direction() - Get the direction (input/output) of a GPIO pin
852 * @gpio: the GPIO pin number from the GPIO subsystem number space
853 *
854 * This function should *ONLY* be used from gpiolib-based GPIO drivers,
855 * as part of their gpio_get_direction() semantics, platforms and individual
856 * drivers shall *NOT* touch pin control GPIO calls.
857 */
858 int pinctrl_gpio_get_direction(unsigned int gpio)
859 {
860 struct pinctrl_dev *pctldev;
861 struct pinctrl_gpio_range *range;
862 int ret;
863 int pin;
864
865 ret = pinctrl_get_device_gpio_range(gpio, &pctldev, &range);
866 if (ret)
867 return ret;
868
869 mutex_lock(&pctldev->mutex);
870
871 /* Convert to the pin controllers number space */
872 pin = gpio_to_pin(range, gpio);
> 873 ret = pinmux_gpio_get_direction(pctldev, range, pin);
874
875 mutex_unlock(&pctldev->mutex);
876
877 return ret;
878 }
879 EXPORT_SYMBOL_GPL(pinctrl_gpio_get_direction);
880
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 33709 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/5] pinctrl: Add API function pinctrl_gpio_get_direction
2018-06-27 11:49 ` [PATCH 2/5] pinctrl: Add API function pinctrl_gpio_get_direction Paul Cercueil
2018-06-27 14:07 ` kbuild test robot
@ 2018-06-27 15:24 ` kbuild test robot
1 sibling, 0 replies; 15+ messages in thread
From: kbuild test robot @ 2018-06-27 15:24 UTC (permalink / raw)
To: Paul Cercueil
Cc: kbuild-all, Linus Walleij, linux-gpio, linux-kernel, Paul Cercueil
[-- Attachment #1: Type: text/plain, Size: 9112 bytes --]
Hi Paul,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on pinctrl/devel]
[also build test ERROR on v4.18-rc2 next-20180627]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Paul-Cercueil/pinctrl-Add-core-function-pinmux_gpio_get_direction/20180627-201609
base: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git devel
config: x86_64-randconfig-s0-06272107 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
drivers//pinctrl/core.c: In function 'pinctrl_gpio_get_direction':
>> drivers//pinctrl/core.c:873:8: error: implicit declaration of function 'pinmux_gpio_get_direction' [-Werror=implicit-function-declaration]
ret = pinmux_gpio_get_direction(pctldev, range, pin);
^~~~~~~~~~~~~~~~~~~~~~~~~
Cyclomatic Complexity 5 include/linux/compiler.h:__write_once_size
Cyclomatic Complexity 1 include/linux/kasan-checks.h:kasan_check_write
Cyclomatic Complexity 1 arch/x86/include/asm/bitops.h:fls64
Cyclomatic Complexity 1 include/linux/log2.h:__ilog2_u64
Cyclomatic Complexity 1 include/linux/list.h:INIT_LIST_HEAD
Cyclomatic Complexity 1 include/linux/list.h:__list_del
Cyclomatic Complexity 1 include/asm-generic/getorder.h:__get_order
Cyclomatic Complexity 1 arch/x86/include/asm/atomic.h:arch_atomic_set
Cyclomatic Complexity 1 include/asm-generic/atomic-instrumented.h:atomic_set
Cyclomatic Complexity 3 include/linux/string.h:kmemdup
Cyclomatic Complexity 1 include/linux/err.h:ERR_PTR
Cyclomatic Complexity 1 include/linux/err.h:PTR_ERR
Cyclomatic Complexity 1 include/linux/spinlock.h:spinlock_check
Cyclomatic Complexity 1 include/linux/refcount.h:refcount_set
Cyclomatic Complexity 1 include/linux/kref.h:kref_init
Cyclomatic Complexity 1 include/linux/kobject.h:kobject_name
Cyclomatic Complexity 2 include/linux/device.h:dev_name
Cyclomatic Complexity 28 include/linux/slab.h:kmalloc_index
Cyclomatic Complexity 68 include/linux/slab.h:kmalloc_large
Cyclomatic Complexity 5 include/linux/slab.h:kmalloc
Cyclomatic Complexity 1 include/linux/slab.h:kzalloc
Cyclomatic Complexity 1 drivers//pinctrl/devicetree.h:pinctrl_dt_to_map
Cyclomatic Complexity 1 drivers//pinctrl/devicetree.h:pinctrl_dt_free_maps
Cyclomatic Complexity 1 drivers//pinctrl/pinmux.h:pinmux_check_ops
Cyclomatic Complexity 1 drivers//pinctrl/pinmux.h:pinmux_validate_map
Cyclomatic Complexity 1 drivers//pinctrl/pinmux.h:pinmux_request_gpio
Cyclomatic Complexity 1 drivers//pinctrl/pinmux.h:pinmux_free_gpio
Cyclomatic Complexity 1 drivers//pinctrl/pinmux.h:pinmux_gpio_direction
Cyclomatic Complexity 1 drivers//pinctrl/pinmux.h:pinmux_map_to_setting
Cyclomatic Complexity 1 drivers//pinctrl/pinmux.h:pinmux_free_setting
Cyclomatic Complexity 1 drivers//pinctrl/pinmux.h:pinmux_enable_setting
Cyclomatic Complexity 1 drivers//pinctrl/pinmux.h:pinmux_disable_setting
Cyclomatic Complexity 1 drivers//pinctrl/pinmux.h:pinmux_show_map
Cyclomatic Complexity 1 drivers//pinctrl/pinmux.h:pinmux_show_setting
Cyclomatic Complexity 1 drivers//pinctrl/pinmux.h:pinmux_init_device_debugfs
Cyclomatic Complexity 1 drivers//pinctrl/pinmux.h:pinmux_generic_free_functions
Cyclomatic Complexity 2 drivers//pinctrl/core.c:gpio_to_pin
Cyclomatic Complexity 1 drivers//pinctrl/core.c:pinctrl_generic_free_groups
Cyclomatic Complexity 1 drivers//pinctrl/core.c:devm_pinctrl_match
Cyclomatic Complexity 2 drivers//pinctrl/core.c:map_type
Cyclomatic Complexity 4 drivers//pinctrl/core.c:pinctrl_check_ops
Cyclomatic Complexity 1 drivers//pinctrl/core.h:pin_desc_get
Cyclomatic Complexity 2 include/linux/list.h:__list_add
Cyclomatic Complexity 1 include/linux/list.h:list_add_tail
Cyclomatic Complexity 2 drivers//pinctrl/core.c:create_state
Cyclomatic Complexity 2 include/linux/list.h:__list_del_entry
Cyclomatic Complexity 1 include/linux/list.h:list_del
Cyclomatic Complexity 6 drivers//pinctrl/core.c:pinctrl_match_gpio_range
Cyclomatic Complexity 5 drivers//pinctrl/core.c:pinctrl_get_device_gpio_range
Cyclomatic Complexity 2 drivers//pinctrl/core.c:pinctrl_gpio_direction
Cyclomatic Complexity 1 include/linux/err.h:IS_ERR
Cyclomatic Complexity 3 include/linux/err.h:IS_ERR_OR_NULL
Cyclomatic Complexity 5 drivers//pinctrl/core.c:devm_pinctrl_dev_match
Cyclomatic Complexity 1 include/asm-generic/gpio.h:gpio_to_chip
Cyclomatic Complexity 11 drivers//pinctrl/core.c:pinctrl_ready_for_gpio_range
Cyclomatic Complexity 5 drivers//pinctrl/core.c:find_pinctrl
Cyclomatic Complexity 1 include/linux/kref.h:kref_get
Cyclomatic Complexity 5 drivers//pinctrl/core.c:find_state
Cyclomatic Complexity 4 drivers//pinctrl/core.c:pinctrl_free_setting
Cyclomatic Complexity 10 drivers//pinctrl/core.c:pinctrl_free
Cyclomatic Complexity 2 drivers//pinctrl/core.c:pinctrl_release
Cyclomatic Complexity 2 include/linux/kref.h:kref_put
Cyclomatic Complexity 3 drivers//pinctrl/core.c:pinctrl_init_debugfs
Cyclomatic Complexity 1 drivers//pinctrl/core.c:pinctrl_init
Cyclomatic Complexity 8 drivers//pinctrl/core.c:pinctrl_init_device_debugfs
Cyclomatic Complexity 1 drivers//pinctrl/core.c:pinctrl_gpioranges_open
Cyclomatic Complexity 1 drivers//pinctrl/core.c:pinctrl_groups_open
Cyclomatic Complexity 1 drivers//pinctrl/core.c:pinctrl_pins_open
Cyclomatic Complexity 1 drivers//pinctrl/core.c:pinctrl_open
Cyclomatic Complexity 1 drivers//pinctrl/core.c:pinctrl_maps_open
Cyclomatic Complexity 1 drivers//pinctrl/core.c:pinctrl_devices_open
Cyclomatic Complexity 6 drivers//pinctrl/core.c:pinctrl_gpioranges_show
Cyclomatic Complexity 4 drivers//pinctrl/core.c:pinctrl_pins_show
Cyclomatic Complexity 1 include/linux/radix-tree.h:radix_tree_insert
Cyclomatic Complexity 5 drivers//pinctrl/core.c:pinctrl_register_one_pin
Cyclomatic Complexity 3 drivers//pinctrl/core.c:pinctrl_register_pins
Cyclomatic Complexity 4 drivers//pinctrl/core.c:pinctrl_free_pindescs
Cyclomatic Complexity 10 drivers//pinctrl/core.c:pinctrl_init_controller
Cyclomatic Complexity 1 drivers//pinctrl/core.c:pinctrl_remove_device_debugfs
Cyclomatic Complexity 8 drivers//pinctrl/core.c:pinctrl_maps_show
Cyclomatic Complexity 6 drivers//pinctrl/core.c:pinctrl_devices_show
Cyclomatic Complexity 1 drivers//pinctrl/core.c:pinctrl_provide_dummies
Cyclomatic Complexity 1 drivers//pinctrl/core.c:pinctrl_dev_get_name
Cyclomatic Complexity 13 drivers//pinctrl/core.c:pinctrl_show
Cyclomatic Complexity 1 drivers//pinctrl/core.c:pinctrl_dev_get_devname
Cyclomatic Complexity 1 drivers//pinctrl/core.c:pinctrl_dev_get_drvdata
Cyclomatic Complexity 6 drivers//pinctrl/core.c:get_pinctrl_dev_from_devname
Cyclomatic Complexity 11 drivers//pinctrl/core.c:add_setting
Cyclomatic Complexity 12 drivers//pinctrl/core.c:create_pinctrl
Cyclomatic Complexity 5 drivers//pinctrl/core.c:get_pinctrl_dev_from_of_node
Cyclomatic Complexity 4 drivers//pinctrl/core.c:pin_get_from_name
Cyclomatic Complexity 2 drivers//pinctrl/core.c:pin_get_name
Cyclomatic Complexity 7 drivers//pinctrl/core.c:pinctrl_groups_show
Cyclomatic Complexity 2 drivers//pinctrl/core.c:pin_is_valid
Cyclomatic Complexity 1 drivers//pinctrl/core.c:pinctrl_add_gpio_range
Cyclomatic Complexity 2 drivers//pinctrl/core.c:pinctrl_add_gpio_ranges
Cyclomatic Complexity 2 drivers//pinctrl/core.c:pinctrl_find_and_add_gpio_range
Cyclomatic Complexity 9 drivers//pinctrl/core.c:pinctrl_find_gpio_range_from_pin_nolock
vim +/pinmux_gpio_get_direction +873 drivers//pinctrl/core.c
849
850 /**
851 * pinctrl_gpio_get_direction() - Get the direction (input/output) of a GPIO pin
852 * @gpio: the GPIO pin number from the GPIO subsystem number space
853 *
854 * This function should *ONLY* be used from gpiolib-based GPIO drivers,
855 * as part of their gpio_get_direction() semantics, platforms and individual
856 * drivers shall *NOT* touch pin control GPIO calls.
857 */
858 int pinctrl_gpio_get_direction(unsigned int gpio)
859 {
860 struct pinctrl_dev *pctldev;
861 struct pinctrl_gpio_range *range;
862 int ret;
863 int pin;
864
865 ret = pinctrl_get_device_gpio_range(gpio, &pctldev, &range);
866 if (ret)
867 return ret;
868
869 mutex_lock(&pctldev->mutex);
870
871 /* Convert to the pin controllers number space */
872 pin = gpio_to_pin(range, gpio);
> 873 ret = pinmux_gpio_get_direction(pctldev, range, pin);
874
875 mutex_unlock(&pctldev->mutex);
876
877 return ret;
878 }
879 EXPORT_SYMBOL_GPL(pinctrl_gpio_get_direction);
880
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 26141 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 3/5] pinctrl: ingenic: Fix inverted direction for < JZ4770
2018-06-27 11:48 [PATCH 0/5] pinctrl_gpio_get_direction & ingenic fixes Paul Cercueil
2018-06-27 11:49 ` [PATCH 1/5] pinctrl: Add core function pinmux_gpio_get_direction Paul Cercueil
2018-06-27 11:49 ` [PATCH 2/5] pinctrl: Add API function pinctrl_gpio_get_direction Paul Cercueil
@ 2018-06-27 11:49 ` Paul Cercueil
2018-07-09 11:58 ` Linus Walleij
2018-06-27 11:49 ` [PATCH 4/5] pinctrl: ingenic: Implement pinmux callback .gpio_get_direction Paul Cercueil
` (2 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Paul Cercueil @ 2018-06-27 11:49 UTC (permalink / raw)
To: Linus Walleij; +Cc: linux-gpio, linux-kernel, Paul Cercueil
The .gpio_set_direction() callback was setting inverted direction
for SoCs older than the JZ4770, this restores the correct behaviour.
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
drivers/pinctrl/pinctrl-ingenic.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pinctrl/pinctrl-ingenic.c b/drivers/pinctrl/pinctrl-ingenic.c
index a1d7156d0a43..6a1b6058b991 100644
--- a/drivers/pinctrl/pinctrl-ingenic.c
+++ b/drivers/pinctrl/pinctrl-ingenic.c
@@ -536,7 +536,7 @@ static int ingenic_pinmux_gpio_set_direction(struct pinctrl_dev *pctldev,
ingenic_config_pin(jzpc, pin, JZ4770_GPIO_PAT1, input);
} else {
ingenic_config_pin(jzpc, pin, JZ4740_GPIO_SELECT, false);
- ingenic_config_pin(jzpc, pin, JZ4740_GPIO_DIR, input);
+ ingenic_config_pin(jzpc, pin, JZ4740_GPIO_DIR, !input);
ingenic_config_pin(jzpc, pin, JZ4740_GPIO_FUNC, false);
}
--
2.11.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 3/5] pinctrl: ingenic: Fix inverted direction for < JZ4770
2018-06-27 11:49 ` [PATCH 3/5] pinctrl: ingenic: Fix inverted direction for < JZ4770 Paul Cercueil
@ 2018-07-09 11:58 ` Linus Walleij
0 siblings, 0 replies; 15+ messages in thread
From: Linus Walleij @ 2018-07-09 11:58 UTC (permalink / raw)
To: Paul Cercueil; +Cc: open list:GPIO SUBSYSTEM, linux-kernel
On Wed, Jun 27, 2018 at 1:49 PM Paul Cercueil <paul@crapouillou.net> wrote:
> The .gpio_set_direction() callback was setting inverted direction
> for SoCs older than the JZ4770, this restores the correct behaviour.
>
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
As you pointed out that this was an important fix I picked it
out and applied for fixes (v4.18) before looking at the rest of
the patches.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 4/5] pinctrl: ingenic: Implement pinmux callback .gpio_get_direction
2018-06-27 11:48 [PATCH 0/5] pinctrl_gpio_get_direction & ingenic fixes Paul Cercueil
` (2 preceding siblings ...)
2018-06-27 11:49 ` [PATCH 3/5] pinctrl: ingenic: Fix inverted direction for < JZ4770 Paul Cercueil
@ 2018-06-27 11:49 ` Paul Cercueil
2018-06-27 11:49 ` [PATCH 5/5] GPIO: ingenic: Implement .get_direction Paul Cercueil
2018-06-27 17:18 ` [PATCH 0/5] pinctrl_gpio_get_direction & ingenic fixes Andy Shevchenko
5 siblings, 0 replies; 15+ messages in thread
From: Paul Cercueil @ 2018-06-27 11:49 UTC (permalink / raw)
To: Linus Walleij; +Cc: linux-gpio, linux-kernel, Paul Cercueil
This will allow the GPIO driver to use the previously introduced
pinctrl_gpio_get_direction function to implement its .get_direction
callback.
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
drivers/pinctrl/pinctrl-ingenic.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/pinctrl/pinctrl-ingenic.c b/drivers/pinctrl/pinctrl-ingenic.c
index 6a1b6058b991..2dab52cf119b 100644
--- a/drivers/pinctrl/pinctrl-ingenic.c
+++ b/drivers/pinctrl/pinctrl-ingenic.c
@@ -543,12 +543,25 @@ static int ingenic_pinmux_gpio_set_direction(struct pinctrl_dev *pctldev,
return 0;
}
+static int ingenic_pinmux_gpio_get_direction(struct pinctrl_dev *pctldev,
+ struct pinctrl_gpio_range *range,
+ unsigned int pin)
+{
+ struct ingenic_pinctrl *jzpc = pinctrl_dev_get_drvdata(pctldev);
+
+ if (jzpc->version >= ID_JZ4770)
+ return ingenic_get_pin_config(jzpc, pin, JZ4770_GPIO_PAT1);
+ else
+ return !ingenic_get_pin_config(jzpc, pin, JZ4740_GPIO_DIR);
+}
+
static const struct pinmux_ops ingenic_pmxops = {
.get_functions_count = pinmux_generic_get_function_count,
.get_function_name = pinmux_generic_get_function_name,
.get_function_groups = pinmux_generic_get_function_groups,
.set_mux = ingenic_pinmux_set_mux,
.gpio_set_direction = ingenic_pinmux_gpio_set_direction,
+ .gpio_get_direction = ingenic_pinmux_gpio_get_direction,
};
static int ingenic_pinconf_get(struct pinctrl_dev *pctldev,
--
2.11.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 5/5] GPIO: ingenic: Implement .get_direction
2018-06-27 11:48 [PATCH 0/5] pinctrl_gpio_get_direction & ingenic fixes Paul Cercueil
` (3 preceding siblings ...)
2018-06-27 11:49 ` [PATCH 4/5] pinctrl: ingenic: Implement pinmux callback .gpio_get_direction Paul Cercueil
@ 2018-06-27 11:49 ` Paul Cercueil
2018-06-27 17:18 ` [PATCH 0/5] pinctrl_gpio_get_direction & ingenic fixes Andy Shevchenko
5 siblings, 0 replies; 15+ messages in thread
From: Paul Cercueil @ 2018-06-27 11:49 UTC (permalink / raw)
To: Linus Walleij; +Cc: linux-gpio, linux-kernel, Paul Cercueil
Use the newly introduced pinctrl_gpio_get_direction to implement the
.get_direction callback.
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
drivers/gpio/gpio-ingenic.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/gpio/gpio-ingenic.c b/drivers/gpio/gpio-ingenic.c
index e738e384a5ca..79e8b67fe83d 100644
--- a/drivers/gpio/gpio-ingenic.c
+++ b/drivers/gpio/gpio-ingenic.c
@@ -274,6 +274,12 @@ static int ingenic_gpio_direction_output(struct gpio_chip *gc,
return pinctrl_gpio_direction_output(gc->base + offset);
}
+static int ingenic_gpio_get_direction(struct gpio_chip *gc,
+ unsigned int offset)
+{
+ return pinctrl_gpio_get_direction(gc->base + offset);
+}
+
static const struct of_device_id ingenic_gpio_of_match[] = {
{ .compatible = "ingenic,jz4740-gpio", .data = (void *)ID_JZ4740 },
{ .compatible = "ingenic,jz4770-gpio", .data = (void *)ID_JZ4770 },
@@ -325,6 +331,7 @@ static int ingenic_gpio_probe(struct platform_device *pdev)
jzgc->gc.set = ingenic_gpio_set;
jzgc->gc.get = ingenic_gpio_get;
+ jzgc->gc.get_direction = ingenic_gpio_get_direction;
jzgc->gc.direction_input = ingenic_gpio_direction_input;
jzgc->gc.direction_output = ingenic_gpio_direction_output;
--
2.11.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 0/5] pinctrl_gpio_get_direction & ingenic fixes
2018-06-27 11:48 [PATCH 0/5] pinctrl_gpio_get_direction & ingenic fixes Paul Cercueil
` (4 preceding siblings ...)
2018-06-27 11:49 ` [PATCH 5/5] GPIO: ingenic: Implement .get_direction Paul Cercueil
@ 2018-06-27 17:18 ` Andy Shevchenko
2018-06-28 19:11 ` Paul Cercueil
2018-07-09 12:09 ` Linus Walleij
5 siblings, 2 replies; 15+ messages in thread
From: Andy Shevchenko @ 2018-06-27 17:18 UTC (permalink / raw)
To: Paul Cercueil
Cc: Linus Walleij, open list:GPIO SUBSYSTEM, Linux Kernel Mailing List
On Wed, Jun 27, 2018 at 2:48 PM, Paul Cercueil <paul@crapouillou.net> wrote:
> Hi Linus,
>
> Here's a set of (rather RFC) patches, to implement
> pinctrl_gpio_get_direction(). I did that, because my gpio-ingenic driver
> calls pinctrl_gpio_set_direction() within its gpio_chip's .set_direction
> callback, but there was no corresponding function to implement the
> .get_direction callback. If that's not the right way to do it, please
> advise.
>
> If not merging the whole series, patch [3/5] is a real fix that should
> go through.
>
> Note that it doesn't make checkpatch.pl happy, I wasn't sure whether I
> should try to comply to checkpatch.pl or match the coding style in the
> pinctrl subsystem, I chose the latter.
I dunno what Linus would going to say about this, but I would like to
see a schematics for this piece of IP.
Even if GPIO and pin muxing has only one set of buffers to indicate
input or output (same registers in use) it's a GPIO driver business to
get direction from GPIO part of IP.
Looking into the existing code I would rather say that
pinctrl-ingenic.c should incorporate gpio-ingenic.c as they are
(partially) sharing same registers.
To ->get_direction() implementation it's pretty straight forward, just
read necessary registers in the gpio-ingenic.c directly. No need to
have pin control or pin muxing to be involved.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/5] pinctrl_gpio_get_direction & ingenic fixes
2018-06-27 17:18 ` [PATCH 0/5] pinctrl_gpio_get_direction & ingenic fixes Andy Shevchenko
@ 2018-06-28 19:11 ` Paul Cercueil
2018-06-29 15:29 ` Andy Shevchenko
2018-07-09 12:18 ` Linus Walleij
2018-07-09 12:09 ` Linus Walleij
1 sibling, 2 replies; 15+ messages in thread
From: Paul Cercueil @ 2018-06-28 19:11 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Linus Walleij, open list:GPIO SUBSYSTEM, Linux Kernel Mailing List
Hi Andy,
Le mer. 27 juin 2018 à 19:18, Andy Shevchenko
<andy.shevchenko@gmail.com> a écrit :
> On Wed, Jun 27, 2018 at 2:48 PM, Paul Cercueil <paul@crapouillou.net>
> wrote:
>> Hi Linus,
>>
>> Here's a set of (rather RFC) patches, to implement
>> pinctrl_gpio_get_direction(). I did that, because my gpio-ingenic
>> driver
>> calls pinctrl_gpio_set_direction() within its gpio_chip's
>> .set_direction
>> callback, but there was no corresponding function to implement the
>> .get_direction callback. If that's not the right way to do it,
>> please
>> advise.
>>
>> If not merging the whole series, patch [3/5] is a real fix that
>> should
>> go through.
>>
>> Note that it doesn't make checkpatch.pl happy, I wasn't sure
>> whether I
>> should try to comply to checkpatch.pl or match the coding style in
>> the
>> pinctrl subsystem, I chose the latter.
>
> I dunno what Linus would going to say about this, but I would like to
> see a schematics for this piece of IP.
ftp://ftp.ingenic.com/SOC/JZ4780/JZ4780_pm.pdf
> Even if GPIO and pin muxing has only one set of buffers to indicate
> input or output (same registers in use) it's a GPIO driver business to
> get direction from GPIO part of IP.
If I follow that logic it's also a GPIO driver business to set the
direction
of a GPIO, right? Truth is the pinctrl subsystem takes care of that. So
why
have "set direction" and no "get direction"?
> Looking into the existing code I would rather say that
> pinctrl-ingenic.c should incorporate gpio-ingenic.c as they are
> (partially) sharing same registers.
> To ->get_direction() implementation it's pretty straight forward, just
> read necessary registers in the gpio-ingenic.c directly. No need to
> have pin control or pin muxing to be involved.
Sure, it'd be pretty straightforward to do it from the GPIO driver, but
I'd
still like to hear Linus' point of view about this.
As for merging pinctrl-ingenic.c and gpio-ingenic.c... I wouldn't
disagree more,
even if they share registers, they belong to different subsystems.
Besides,
your platform might need the pinctrl driver but not the GPIO one, or
you might
want to provide the GPIO driver as a loadable module, etc.
Regards,
-Paul
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/5] pinctrl_gpio_get_direction & ingenic fixes
2018-06-28 19:11 ` Paul Cercueil
@ 2018-06-29 15:29 ` Andy Shevchenko
2018-07-09 12:18 ` Linus Walleij
1 sibling, 0 replies; 15+ messages in thread
From: Andy Shevchenko @ 2018-06-29 15:29 UTC (permalink / raw)
To: Paul Cercueil
Cc: Linus Walleij, open list:GPIO SUBSYSTEM, Linux Kernel Mailing List
On Thu, Jun 28, 2018 at 10:11 PM, Paul Cercueil <paul@crapouillou.net> wrote:
> Le mer. 27 juin 2018 à 19:18, Andy Shevchenko <andy.shevchenko@gmail.com> a
> écrit :
>> On Wed, Jun 27, 2018 at 2:48 PM, Paul Cercueil <paul@crapouillou.net>
>> wrote:
>> I dunno what Linus would going to say about this, but I would like to
>> see a schematics for this piece of IP.
> ftp://ftp.ingenic.com/SOC/JZ4780/JZ4780_pm.pdf
I'm not sure you referred to a proper document "Mobile Application
Processor. Programming Manual"
I didn't find quickly what I'm looking for. Care to point me out to
the page number?
Thanks!
>> Even if GPIO and pin muxing has only one set of buffers to indicate
>> input or output (same registers in use) it's a GPIO driver business to
>> get direction from GPIO part of IP.
>
>
> If I follow that logic it's also a GPIO driver business to set the direction
> of a GPIO, right? Truth is the pinctrl subsystem takes care of that. So why
> have "set direction" and no "get direction"?
Because it's a pin muxing business depending on the function chosen.
It solely depends on a schematic. For the example, everyone knows that
MOSI is out always, otherwise SPI function would not work.
.get_direction() has useful meaning only for GPIO function. (This is
my understanding from electrical configuration of buffers).
>> Looking into the existing code I would rather say that
>> pinctrl-ingenic.c should incorporate gpio-ingenic.c as they are
>> (partially) sharing same registers.
>> To ->get_direction() implementation it's pretty straight forward, just
>> read necessary registers in the gpio-ingenic.c directly. No need to
>> have pin control or pin muxing to be involved.
> Sure, it'd be pretty straightforward to do it from the GPIO driver, but I'd
> still like to hear Linus' point of view about this.
Sure.
> As for merging pinctrl-ingenic.c and gpio-ingenic.c... I wouldn't disagree
> more,
> even if they share registers, they belong to different subsystems. Besides,
> your platform might need the pinctrl driver but not the GPIO one, or you
> might
> want to provide the GPIO driver as a loadable module, etc.
It was just my observation, I'm not a maintainer of that code nor
author, so, up to you how to proceed.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/5] pinctrl_gpio_get_direction & ingenic fixes
2018-06-28 19:11 ` Paul Cercueil
2018-06-29 15:29 ` Andy Shevchenko
@ 2018-07-09 12:18 ` Linus Walleij
1 sibling, 0 replies; 15+ messages in thread
From: Linus Walleij @ 2018-07-09 12:18 UTC (permalink / raw)
To: Paul Cercueil; +Cc: Andy Shevchenko, open list:GPIO SUBSYSTEM, linux-kernel
On Thu, Jun 28, 2018 at 9:11 PM Paul Cercueil <paul@crapouillou.net> wrote:
> Sure, it'd be pretty straightforward to do it from the GPIO driver, but
> I'd still like to hear Linus' point of view about this.
I'm not sure about it, I guess it would be my second choice.
> As for merging pinctrl-ingenic.c and gpio-ingenic.c... I wouldn't
> disagree more,
> even if they share registers, they belong to different subsystems.
We actually have many pin control drivers also exposing
a GPIO chip. (Or several.) The way I see it is that we use
two kernel APIs for the same hardware, but one hardware
piece should still just have one driver, albeit exposing two
interfaces.
We actually also have serial ports and wireless network
cards exposing random GPIO chips because they happen
to have some arbitrary GPIO lines up for grabs.
So it is not just for the pin control and GPIO usecase, it is
for any usecase where the same hardware has something
plus a GPIO API. Pin control with GPIO as front-end is
special by being jitted together with the ranges though, so
they are closer related.
A good reason to keep it together in one driver is e.g. that they
have common clocking, so otherwise two drivers are grabbing
the same silicon clock and enabling/disabling it, runtime PM
would be a mess if both drivers start to request prepare
enable disable clocks at the same time etc. (I don't know
if this applied to Ingenic.)
> Besides,
> your platform might need the pinctrl driver but not the GPIO one, or
> you might
> want to provide the GPIO driver as a loadable module, etc.
I understand that concern. I think it's as nice with one
loadable module for pin control and GPIO at the same
time though, especially if the split is anyways kind
artificial between the two APIs.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/5] pinctrl_gpio_get_direction & ingenic fixes
2018-06-27 17:18 ` [PATCH 0/5] pinctrl_gpio_get_direction & ingenic fixes Andy Shevchenko
2018-06-28 19:11 ` Paul Cercueil
@ 2018-07-09 12:09 ` Linus Walleij
2018-07-11 23:22 ` Paul Cercueil
1 sibling, 1 reply; 15+ messages in thread
From: Linus Walleij @ 2018-07-09 12:09 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: Paul Cercueil, open list:GPIO SUBSYSTEM, linux-kernel
Hi folks,
On Wed, Jun 27, 2018 at 7:18 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> Even if GPIO and pin muxing has only one set of buffers to indicate
> input or output (same registers in use) it's a GPIO driver business to
> get direction from GPIO part of IP.
>
> Looking into the existing code I would rather say that
> pinctrl-ingenic.c should incorporate gpio-ingenic.c as they are
> (partially) sharing same registers.
Usually we only split the functionality into two drivers if the two features
pin control and GPIO are explicitly in different hardware blocks,
and typically not sharing the same memory range.
If these registers are intermingled and the hardware actually
just one piece of silicon, I would suggest to try to merge the
two drivers into a combined pin control and GPIO driver
inside drivers/pinctrl/pinctrl-ingenic.c.
We have a few drivers like that already, good textbook
examples of how to do this include
drivers/pinctrl/pinctrl-sx150x.c where the two blocks are
handled in one driver using both APIs.
Paul could you have a look at if we can simply merge these
two into one big driver? It is much more natural to write
into the same set of registers when we do that.
If you still prefer to proceed with the GPIO/pinctrl as separate
drivers we need to look into this patch set, which I am
a bit ambivalent about, because it makes sense but at the
same time I want to keep GPIO and pin control business
separate because separation of concerns is just nice.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/5] pinctrl_gpio_get_direction & ingenic fixes
2018-07-09 12:09 ` Linus Walleij
@ 2018-07-11 23:22 ` Paul Cercueil
0 siblings, 0 replies; 15+ messages in thread
From: Paul Cercueil @ 2018-07-11 23:22 UTC (permalink / raw)
To: Linus Walleij; +Cc: Andy Shevchenko, open list:GPIO SUBSYSTEM, linux-kernel
Hi Linus,
Le lun. 9 juil. 2018 à 14:09, Linus Walleij <linus.walleij@linaro.org>
a écrit :
> Hi folks,
>
> On Wed, Jun 27, 2018 at 7:18 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
>
>> Even if GPIO and pin muxing has only one set of buffers to indicate
>> input or output (same registers in use) it's a GPIO driver business
>> to
>> get direction from GPIO part of IP.
>>
>> Looking into the existing code I would rather say that
>> pinctrl-ingenic.c should incorporate gpio-ingenic.c as they are
>> (partially) sharing same registers.
>
> Usually we only split the functionality into two drivers if the two
> features
> pin control and GPIO are explicitly in different hardware blocks,
> and typically not sharing the same memory range.
>
> If these registers are intermingled and the hardware actually
> just one piece of silicon, I would suggest to try to merge the
> two drivers into a combined pin control and GPIO driver
> inside drivers/pinctrl/pinctrl-ingenic.c.
>
> We have a few drivers like that already, good textbook
> examples of how to do this include
> drivers/pinctrl/pinctrl-sx150x.c where the two blocks are
> handled in one driver using both APIs.
>
> Paul could you have a look at if we can simply merge these
> two into one big driver? It is much more natural to write
> into the same set of registers when we do that.
Well I wish you had told me that when I submitted the ingenic
pinctrl/gpio
patchset :)
I won't have much time before 4.19-rc1, but I can have a look after
that.
> If you still prefer to proceed with the GPIO/pinctrl as separate
> drivers we need to look into this patch set, which I am
> a bit ambivalent about, because it makes sense but at the
> same time I want to keep GPIO and pin control business
> separate because separation of concerns is just nice.
Well I can still implement the get_direction() function in the GPIO
driver by reading the registers instead of calling into pinctrl.
I just thought it felt illogic as set_direction() does that.
> Yours,
> Linus Walleij
Thanks,
-Paul
^ permalink raw reply [flat|nested] 15+ messages in thread