* [PATCH v1 0/2] pinctrl: generic: improve apply_setting error verbosity @ 2018-05-01 7:40 Matheus Castello 2018-05-01 7:40 ` [PATCH v1 1/2] pinctrl: generic: add API to solve generic sub-node property name Matheus Castello 2018-05-01 7:40 ` [PATCH v1 2/2] pinctrl: generic: improve apply_setting error verbosity Matheus Castello 0 siblings, 2 replies; 11+ messages in thread From: Matheus Castello @ 2018-05-01 7:40 UTC (permalink / raw) To: linus.walleij; +Cc: linux-kernel, Matheus Castello This series is for improve verbosity in debug for error cases using pinconf generic API. Legacy pinctrl-vendor drivers prints error messages with more verbosity when parse invalid or not supported sub-nodes properties. In this series we try to show messages with more information about the pinctrl-vendor driver and sub-nodes that returned error when applied. Let me know your opinion about this. Matheus Castello (2): pinctrl: generic: add API to solve generic sub-node property name pinctrl: generic: improve apply_setting error verbosity drivers/pinctrl/pinconf-generic.c | 61 +++++++++++++++++---------------- drivers/pinctrl/pinconf.c | 15 +++++++- include/linux/pinctrl/pinconf-generic.h | 32 +++++++++++++++++ 3 files changed, 77 insertions(+), 31 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v1 1/2] pinctrl: generic: add API to solve generic sub-node property name 2018-05-01 7:40 [PATCH v1 0/2] pinctrl: generic: improve apply_setting error verbosity Matheus Castello @ 2018-05-01 7:40 ` Matheus Castello 2018-05-01 7:40 ` [PATCH v1 2/2] pinctrl: generic: improve apply_setting error verbosity Matheus Castello 1 sibling, 0 replies; 11+ messages in thread From: Matheus Castello @ 2018-05-01 7:40 UTC (permalink / raw) To: linus.walleij; +Cc: linux-kernel, Matheus Castello Set dt_params with the definitions of the generic sub-node properties global in pinconf-generic.h and add a function to pinconf-generic API to decode a packed param returning the string name from sub-node property. This can be useful in debug from pinctrl-vendor driver or even for debug in pinconf driver. Signed-off-by: Matheus Castello <matheus@castello.eng.br> --- drivers/pinctrl/pinconf-generic.c | 61 +++++++++++++++++---------------- include/linux/pinctrl/pinconf-generic.h | 32 +++++++++++++++++ 2 files changed, 63 insertions(+), 30 deletions(-) diff --git a/drivers/pinctrl/pinconf-generic.c b/drivers/pinctrl/pinconf-generic.c index b4f7f8a..78585bc 100644 --- a/drivers/pinctrl/pinconf-generic.c +++ b/drivers/pinctrl/pinconf-generic.c @@ -156,34 +156,6 @@ EXPORT_SYMBOL_GPL(pinconf_generic_dump_config); #endif #ifdef CONFIG_OF -static const struct pinconf_generic_params dt_params[] = { - { "bias-bus-hold", PIN_CONFIG_BIAS_BUS_HOLD, 0 }, - { "bias-disable", PIN_CONFIG_BIAS_DISABLE, 0 }, - { "bias-high-impedance", PIN_CONFIG_BIAS_HIGH_IMPEDANCE, 0 }, - { "bias-pull-up", PIN_CONFIG_BIAS_PULL_UP, 1 }, - { "bias-pull-pin-default", PIN_CONFIG_BIAS_PULL_PIN_DEFAULT, 1 }, - { "bias-pull-down", PIN_CONFIG_BIAS_PULL_DOWN, 1 }, - { "drive-open-drain", PIN_CONFIG_DRIVE_OPEN_DRAIN, 0 }, - { "drive-open-source", PIN_CONFIG_DRIVE_OPEN_SOURCE, 0 }, - { "drive-push-pull", PIN_CONFIG_DRIVE_PUSH_PULL, 0 }, - { "drive-strength", PIN_CONFIG_DRIVE_STRENGTH, 0 }, - { "input-debounce", PIN_CONFIG_INPUT_DEBOUNCE, 0 }, - { "input-disable", PIN_CONFIG_INPUT_ENABLE, 0 }, - { "input-enable", PIN_CONFIG_INPUT_ENABLE, 1 }, - { "input-schmitt", PIN_CONFIG_INPUT_SCHMITT, 0 }, - { "input-schmitt-disable", PIN_CONFIG_INPUT_SCHMITT_ENABLE, 0 }, - { "input-schmitt-enable", PIN_CONFIG_INPUT_SCHMITT_ENABLE, 1 }, - { "low-power-disable", PIN_CONFIG_LOW_POWER_MODE, 0 }, - { "low-power-enable", PIN_CONFIG_LOW_POWER_MODE, 1 }, - { "output-disable", PIN_CONFIG_OUTPUT_ENABLE, 0 }, - { "output-enable", PIN_CONFIG_OUTPUT_ENABLE, 1 }, - { "output-high", PIN_CONFIG_OUTPUT, 1, }, - { "output-low", PIN_CONFIG_OUTPUT, 0, }, - { "power-source", PIN_CONFIG_POWER_SOURCE, 0 }, - { "sleep-hardware-state", PIN_CONFIG_SLEEP_HARDWARE_STATE, 0 }, - { "slew-rate", PIN_CONFIG_SLEW_RATE, 0 }, - { "skew-delay", PIN_CONFIG_SKEW_DELAY, 0 }, -}; /** * parse_dt_cfg() - Parse DT pinconf parameters @@ -247,14 +219,14 @@ int pinconf_generic_parse_dt_config(struct device_node *np, return -EINVAL; /* allocate a temporary array big enough to hold one of each option */ - max_cfg = ARRAY_SIZE(dt_params); + max_cfg = ARRAY_SIZE(pinconf_dt_params); if (pctldev) max_cfg += pctldev->desc->num_custom_params; cfg = kcalloc(max_cfg, sizeof(*cfg), GFP_KERNEL); if (!cfg) return -ENOMEM; - parse_dt_cfg(np, dt_params, ARRAY_SIZE(dt_params), cfg, &ncfg); + parse_dt_cfg(np, pinconf_dt_params, ARRAY_SIZE(pinconf_dt_params), cfg, &ncfg); if (pctldev && pctldev->desc->num_custom_params && pctldev->desc->custom_params) parse_dt_cfg(np, pctldev->desc->custom_params, @@ -409,4 +381,33 @@ void pinconf_generic_dt_free_map(struct pinctrl_dev *pctldev, } EXPORT_SYMBOL_GPL(pinconf_generic_dt_free_map); +const char *pinconf_generic_get_param_property_name(struct pinctrl_dev *pctldev, + unsigned int num_configs, unsigned long *param) +{ + struct pinctrl_desc *desc = pctldev->desc; + unsigned int num_params = ARRAY_SIZE(pinconf_dt_params), + num_custom_params = desc->num_custom_params, + i, j; + + for (i = 0; i < num_configs; i++) { + enum pin_config_param pin_param = + pinconf_to_config_param(param[i]); + + /* first search on default properties */ + for (j = 0; j < num_params; j++) { + if (pin_param == pinconf_dt_params[j].param) + return pinconf_dt_params[j].property; + } + + /* search on custom properties */ + for (j = 0; j < num_custom_params; j++) { + if (pin_param == desc->custom_params[j].param) + return desc->custom_params[j].property; + } + } + + return "property undefined"; +} +EXPORT_SYMBOL_GPL(pinconf_generic_get_param_property_name); + #endif diff --git a/include/linux/pinctrl/pinconf-generic.h b/include/linux/pinctrl/pinconf-generic.h index 6c06806..c51f4b3 100644 --- a/include/linux/pinctrl/pinconf-generic.h +++ b/include/linux/pinctrl/pinconf-generic.h @@ -184,6 +184,35 @@ struct pinconf_generic_params { u32 default_value; }; +static const struct pinconf_generic_params pinconf_dt_params[] = { + { "bias-bus-hold", PIN_CONFIG_BIAS_BUS_HOLD, 0 }, + { "bias-disable", PIN_CONFIG_BIAS_DISABLE, 0 }, + { "bias-high-impedance", PIN_CONFIG_BIAS_HIGH_IMPEDANCE, 0 }, + { "bias-pull-up", PIN_CONFIG_BIAS_PULL_UP, 1 }, + { "bias-pull-pin-default", PIN_CONFIG_BIAS_PULL_PIN_DEFAULT, 1 }, + { "bias-pull-down", PIN_CONFIG_BIAS_PULL_DOWN, 1 }, + { "drive-open-drain", PIN_CONFIG_DRIVE_OPEN_DRAIN, 0 }, + { "drive-open-source", PIN_CONFIG_DRIVE_OPEN_SOURCE, 0 }, + { "drive-push-pull", PIN_CONFIG_DRIVE_PUSH_PULL, 0 }, + { "drive-strength", PIN_CONFIG_DRIVE_STRENGTH, 0 }, + { "input-debounce", PIN_CONFIG_INPUT_DEBOUNCE, 0 }, + { "input-disable", PIN_CONFIG_INPUT_ENABLE, 0 }, + { "input-enable", PIN_CONFIG_INPUT_ENABLE, 1 }, + { "input-schmitt", PIN_CONFIG_INPUT_SCHMITT, 0 }, + { "input-schmitt-disable", PIN_CONFIG_INPUT_SCHMITT_ENABLE, 0 }, + { "input-schmitt-enable", PIN_CONFIG_INPUT_SCHMITT_ENABLE, 1 }, + { "low-power-disable", PIN_CONFIG_LOW_POWER_MODE, 0 }, + { "low-power-enable", PIN_CONFIG_LOW_POWER_MODE, 1 }, + { "output-disable", PIN_CONFIG_OUTPUT_ENABLE, 0 }, + { "output-enable", PIN_CONFIG_OUTPUT_ENABLE, 1 }, + { "output-high", PIN_CONFIG_OUTPUT, 1, }, + { "output-low", PIN_CONFIG_OUTPUT, 0, }, + { "power-source", PIN_CONFIG_POWER_SOURCE, 0 }, + { "sleep-hardware-state", PIN_CONFIG_SLEEP_HARDWARE_STATE, 0 }, + { "slew-rate", PIN_CONFIG_SLEW_RATE, 0 }, + { "skew-delay", PIN_CONFIG_SKEW_DELAY, 0 }, +}; + int pinconf_generic_dt_subnode_to_map(struct pinctrl_dev *pctldev, struct device_node *np, struct pinctrl_map **map, unsigned *reserved_maps, unsigned *num_maps, @@ -194,6 +223,9 @@ int pinconf_generic_dt_node_to_map(struct pinctrl_dev *pctldev, void pinconf_generic_dt_free_map(struct pinctrl_dev *pctldev, struct pinctrl_map *map, unsigned num_maps); +const char *pinconf_generic_get_param_property_name(struct pinctrl_dev *pctldev, + unsigned int num_configs, unsigned long *param); + static inline int pinconf_generic_dt_node_to_map_group( struct pinctrl_dev *pctldev, struct device_node *np_config, struct pinctrl_map **map, unsigned *num_maps) -- 2.7.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v1 2/2] pinctrl: generic: improve apply_setting error verbosity 2018-05-01 7:40 [PATCH v1 0/2] pinctrl: generic: improve apply_setting error verbosity Matheus Castello 2018-05-01 7:40 ` [PATCH v1 1/2] pinctrl: generic: add API to solve generic sub-node property name Matheus Castello @ 2018-05-01 7:40 ` Matheus Castello 2018-05-01 8:34 ` kbuild test robot 2018-05-01 10:05 ` kbuild test robot 1 sibling, 2 replies; 11+ messages in thread From: Matheus Castello @ 2018-05-01 7:40 UTC (permalink / raw) To: linus.walleij; +Cc: linux-kernel, Matheus Castello For generic pinconf: print the dev_error with the pinctrl vendor driver name, error code, the sub-node property name used and the pin that was tried to set. Improves the undestading of the error if use a generic sub-node property that generic-pinconf can do parse but the vendor pinctrl driver does not support. Signed-off-by: Matheus Castello <matheus@castello.eng.br> --- drivers/pinctrl/pinconf.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/drivers/pinctrl/pinconf.c b/drivers/pinctrl/pinconf.c index d3fe143..fd5962a 100644 --- a/drivers/pinctrl/pinconf.c +++ b/drivers/pinctrl/pinconf.c @@ -21,6 +21,7 @@ #include <linux/pinctrl/machine.h> #include <linux/pinctrl/pinctrl.h> #include <linux/pinctrl/pinconf.h> +#include <linux/pinctrl/pinconf-generic.h> #include "core.h" #include "pinconf.h" @@ -169,9 +170,21 @@ int pinconf_apply_setting(const struct pinctrl_setting *setting) setting->data.configs.configs, setting->data.configs.num_configs); if (ret < 0) { +#ifdef CONFIG_GENERIC_PINCONF dev_err(pctldev->dev, - "pin_config_set op failed for pin %d\n", + "%s error %d seting %s for pin %d\n", + pctldev->desc->name, ret, + pinconf_generic_get_param_property_name( + pctldev, setting->data.configs.num_configs, + setting->data.configs.configs), setting->data.configs.group_or_pin); +#endif + + dev_err(pctldev->dev, + "pin_config_set op failed for %s pin %d\n", + pctldev->desc->name, + setting->data.configs.group_or_pin); + return ret; } break; -- 2.7.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v1 2/2] pinctrl: generic: improve apply_setting error verbosity 2018-05-01 7:40 ` [PATCH v1 2/2] pinctrl: generic: improve apply_setting error verbosity Matheus Castello @ 2018-05-01 8:34 ` kbuild test robot 2018-05-01 10:05 ` kbuild test robot 1 sibling, 0 replies; 11+ messages in thread From: kbuild test robot @ 2018-05-01 8:34 UTC (permalink / raw) To: Matheus Castello Cc: kbuild-all, linus.walleij, linux-kernel, Matheus Castello [-- Attachment #1: Type: text/plain, Size: 3993 bytes --] Hi Matheus, Thank you for the patch! Yet something to improve: [auto build test ERROR on pinctrl/devel] [also build test ERROR on v4.17-rc3 next-20180501] [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/Matheus-Castello/pinctrl-generic-improve-apply_setting-error-verbosity/20180501-160704 base: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git devel config: x86_64-randconfig-x002-201817 (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 error/warnings (new ones prefixed by >>): drivers/pinctrl/pinconf.c: In function 'pinconf_apply_setting': >> drivers/pinctrl/pinconf.c:177:5: error: implicit declaration of function 'pinconf_generic_get_param_property_name'; did you mean 'pinconf_generic_dump_config'? [-Werror=implicit-function-declaration] pinconf_generic_get_param_property_name( ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ pinconf_generic_dump_config >> drivers/pinctrl/pinconf.c:175:26: warning: format '%s' expects argument of type 'char *', but argument 5 has type 'int' [-Wformat=] "%s error %d seting %s for pin %d\n", ~^ %d drivers/pinctrl/pinconf.c:177:5: pinconf_generic_get_param_property_name( ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ pctldev, setting->data.configs.num_configs, ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ setting->data.configs.configs), ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ cc1: some warnings being treated as errors vim +177 drivers/pinctrl/pinconf.c 150 151 int pinconf_apply_setting(const struct pinctrl_setting *setting) 152 { 153 struct pinctrl_dev *pctldev = setting->pctldev; 154 const struct pinconf_ops *ops = pctldev->desc->confops; 155 int ret; 156 157 if (!ops) { 158 dev_err(pctldev->dev, "missing confops\n"); 159 return -EINVAL; 160 } 161 162 switch (setting->type) { 163 case PIN_MAP_TYPE_CONFIGS_PIN: 164 if (!ops->pin_config_set) { 165 dev_err(pctldev->dev, "missing pin_config_set op\n"); 166 return -EINVAL; 167 } 168 ret = ops->pin_config_set(pctldev, 169 setting->data.configs.group_or_pin, 170 setting->data.configs.configs, 171 setting->data.configs.num_configs); 172 if (ret < 0) { 173 #ifdef CONFIG_GENERIC_PINCONF 174 dev_err(pctldev->dev, > 175 "%s error %d seting %s for pin %d\n", 176 pctldev->desc->name, ret, > 177 pinconf_generic_get_param_property_name( 178 pctldev, setting->data.configs.num_configs, 179 setting->data.configs.configs), 180 setting->data.configs.group_or_pin); 181 #endif 182 183 dev_err(pctldev->dev, 184 "pin_config_set op failed for %s pin %d\n", 185 pctldev->desc->name, 186 setting->data.configs.group_or_pin); 187 188 return ret; 189 } 190 break; 191 case PIN_MAP_TYPE_CONFIGS_GROUP: 192 if (!ops->pin_config_group_set) { 193 dev_err(pctldev->dev, 194 "missing pin_config_group_set op\n"); 195 return -EINVAL; 196 } 197 ret = ops->pin_config_group_set(pctldev, 198 setting->data.configs.group_or_pin, 199 setting->data.configs.configs, 200 setting->data.configs.num_configs); 201 if (ret < 0) { 202 dev_err(pctldev->dev, 203 "pin_config_group_set op failed for group %d\n", 204 setting->data.configs.group_or_pin); 205 return ret; 206 } 207 break; 208 default: 209 return -EINVAL; 210 } 211 212 return 0; 213 } 214 --- 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: 30311 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 2/2] pinctrl: generic: improve apply_setting error verbosity 2018-05-01 7:40 ` [PATCH v1 2/2] pinctrl: generic: improve apply_setting error verbosity Matheus Castello 2018-05-01 8:34 ` kbuild test robot @ 2018-05-01 10:05 ` kbuild test robot 2018-05-01 19:09 ` [PATCH v2 0/2] " Matheus Castello 1 sibling, 1 reply; 11+ messages in thread From: kbuild test robot @ 2018-05-01 10:05 UTC (permalink / raw) To: Matheus Castello Cc: kbuild-all, linus.walleij, linux-kernel, Matheus Castello [-- Attachment #1: Type: text/plain, Size: 3606 bytes --] Hi Matheus, Thank you for the patch! Yet something to improve: [auto build test ERROR on pinctrl/devel] [also build test ERROR on v4.17-rc3 next-20180501] [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/Matheus-Castello/pinctrl-generic-improve-apply_setting-error-verbosity/20180501-160704 base: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git devel config: x86_64-randconfig-s2-05011559 (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/pinconf.c: In function 'pinconf_apply_setting': >> drivers//pinctrl/pinconf.c:177:5: error: implicit declaration of function 'pinconf_generic_get_param_property_name' [-Werror=implicit-function-declaration] pinconf_generic_get_param_property_name( ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers//pinctrl/pinconf.c:175:26: warning: format '%s' expects argument of type 'char *', but argument 5 has type 'int' [-Wformat=] "%s error %d seting %s for pin %d\n", ^ cc1: some warnings being treated as errors vim +/pinconf_generic_get_param_property_name +177 drivers//pinctrl/pinconf.c 150 151 int pinconf_apply_setting(const struct pinctrl_setting *setting) 152 { 153 struct pinctrl_dev *pctldev = setting->pctldev; 154 const struct pinconf_ops *ops = pctldev->desc->confops; 155 int ret; 156 157 if (!ops) { 158 dev_err(pctldev->dev, "missing confops\n"); 159 return -EINVAL; 160 } 161 162 switch (setting->type) { 163 case PIN_MAP_TYPE_CONFIGS_PIN: 164 if (!ops->pin_config_set) { 165 dev_err(pctldev->dev, "missing pin_config_set op\n"); 166 return -EINVAL; 167 } 168 ret = ops->pin_config_set(pctldev, 169 setting->data.configs.group_or_pin, 170 setting->data.configs.configs, 171 setting->data.configs.num_configs); 172 if (ret < 0) { 173 #ifdef CONFIG_GENERIC_PINCONF 174 dev_err(pctldev->dev, 175 "%s error %d seting %s for pin %d\n", 176 pctldev->desc->name, ret, > 177 pinconf_generic_get_param_property_name( 178 pctldev, setting->data.configs.num_configs, 179 setting->data.configs.configs), 180 setting->data.configs.group_or_pin); 181 #endif 182 183 dev_err(pctldev->dev, 184 "pin_config_set op failed for %s pin %d\n", 185 pctldev->desc->name, 186 setting->data.configs.group_or_pin); 187 188 return ret; 189 } 190 break; 191 case PIN_MAP_TYPE_CONFIGS_GROUP: 192 if (!ops->pin_config_group_set) { 193 dev_err(pctldev->dev, 194 "missing pin_config_group_set op\n"); 195 return -EINVAL; 196 } 197 ret = ops->pin_config_group_set(pctldev, 198 setting->data.configs.group_or_pin, 199 setting->data.configs.configs, 200 setting->data.configs.num_configs); 201 if (ret < 0) { 202 dev_err(pctldev->dev, 203 "pin_config_group_set op failed for group %d\n", 204 setting->data.configs.group_or_pin); 205 return ret; 206 } 207 break; 208 default: 209 return -EINVAL; 210 } 211 212 return 0; 213 } 214 --- 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: 32961 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 0/2] pinctrl: generic: improve apply_setting error verbosity 2018-05-01 10:05 ` kbuild test robot @ 2018-05-01 19:09 ` Matheus Castello 2018-05-01 19:10 ` [PATCH v2 1/2] pinctrl: generic: add API to solve generic sub-node property name Matheus Castello 2018-05-01 19:10 ` [PATCH v2 2/2] pinctrl: generic: improve apply_setting error verbosity Matheus Castello 0 siblings, 2 replies; 11+ messages in thread From: Matheus Castello @ 2018-05-01 19:09 UTC (permalink / raw) To: linus.walleij; +Cc: kbuild-all, linux-kernel, Matheus Castello This series is for improve verbosity in debug for error cases using pinconf generic API. Legacy pinctrl-vendor drivers prints error messages with more verbosity when parse invalid or not supported sub-nodes properties. In this series we try to show messages with more information about the pinctrl-vendor driver and sub-nodes that returned error when applied. Let me know your opinion about this. Tested Kbuild for arch/x86_64 arch/arm Changes since v1: (Suggested by Linus Walleij) - Fix errors in Kbuild - Use CONFIG_OF instead CONFIG_GENERIC_PINCONF for use it only in flatned device tree enable configurations Matheus Castello (2): pinctrl: generic: add API to solve generic sub-node property name pinctrl: generic: improve apply_setting error verbosity drivers/pinctrl/pinconf-generic.c | 61 +++++++++++++++++---------------- drivers/pinctrl/pinconf.c | 15 +++++++- include/linux/pinctrl/pinconf-generic.h | 32 +++++++++++++++++ 3 files changed, 77 insertions(+), 31 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 1/2] pinctrl: generic: add API to solve generic sub-node property name 2018-05-01 19:09 ` [PATCH v2 0/2] " Matheus Castello @ 2018-05-01 19:10 ` Matheus Castello 2018-05-02 12:56 ` Linus Walleij 2018-05-01 19:10 ` [PATCH v2 2/2] pinctrl: generic: improve apply_setting error verbosity Matheus Castello 1 sibling, 1 reply; 11+ messages in thread From: Matheus Castello @ 2018-05-01 19:10 UTC (permalink / raw) To: linus.walleij; +Cc: kbuild-all, linux-kernel, Matheus Castello Set dt_params with the definitions of the generic sub-node properties global in pinconf-generic.h and add a function to pinconf-generic API to decode a packed param returning the string name from sub-node property. This can be useful in debug from pinctrl-vendor driver or even for debug in pinconf driver. Signed-off-by: Matheus Castello <matheus@castello.eng.br> --- drivers/pinctrl/pinconf-generic.c | 61 +++++++++++++++++---------------- include/linux/pinctrl/pinconf-generic.h | 32 +++++++++++++++++ 2 files changed, 63 insertions(+), 30 deletions(-) diff --git a/drivers/pinctrl/pinconf-generic.c b/drivers/pinctrl/pinconf-generic.c index b4f7f8a..78585bc 100644 --- a/drivers/pinctrl/pinconf-generic.c +++ b/drivers/pinctrl/pinconf-generic.c @@ -156,34 +156,6 @@ EXPORT_SYMBOL_GPL(pinconf_generic_dump_config); #endif #ifdef CONFIG_OF -static const struct pinconf_generic_params dt_params[] = { - { "bias-bus-hold", PIN_CONFIG_BIAS_BUS_HOLD, 0 }, - { "bias-disable", PIN_CONFIG_BIAS_DISABLE, 0 }, - { "bias-high-impedance", PIN_CONFIG_BIAS_HIGH_IMPEDANCE, 0 }, - { "bias-pull-up", PIN_CONFIG_BIAS_PULL_UP, 1 }, - { "bias-pull-pin-default", PIN_CONFIG_BIAS_PULL_PIN_DEFAULT, 1 }, - { "bias-pull-down", PIN_CONFIG_BIAS_PULL_DOWN, 1 }, - { "drive-open-drain", PIN_CONFIG_DRIVE_OPEN_DRAIN, 0 }, - { "drive-open-source", PIN_CONFIG_DRIVE_OPEN_SOURCE, 0 }, - { "drive-push-pull", PIN_CONFIG_DRIVE_PUSH_PULL, 0 }, - { "drive-strength", PIN_CONFIG_DRIVE_STRENGTH, 0 }, - { "input-debounce", PIN_CONFIG_INPUT_DEBOUNCE, 0 }, - { "input-disable", PIN_CONFIG_INPUT_ENABLE, 0 }, - { "input-enable", PIN_CONFIG_INPUT_ENABLE, 1 }, - { "input-schmitt", PIN_CONFIG_INPUT_SCHMITT, 0 }, - { "input-schmitt-disable", PIN_CONFIG_INPUT_SCHMITT_ENABLE, 0 }, - { "input-schmitt-enable", PIN_CONFIG_INPUT_SCHMITT_ENABLE, 1 }, - { "low-power-disable", PIN_CONFIG_LOW_POWER_MODE, 0 }, - { "low-power-enable", PIN_CONFIG_LOW_POWER_MODE, 1 }, - { "output-disable", PIN_CONFIG_OUTPUT_ENABLE, 0 }, - { "output-enable", PIN_CONFIG_OUTPUT_ENABLE, 1 }, - { "output-high", PIN_CONFIG_OUTPUT, 1, }, - { "output-low", PIN_CONFIG_OUTPUT, 0, }, - { "power-source", PIN_CONFIG_POWER_SOURCE, 0 }, - { "sleep-hardware-state", PIN_CONFIG_SLEEP_HARDWARE_STATE, 0 }, - { "slew-rate", PIN_CONFIG_SLEW_RATE, 0 }, - { "skew-delay", PIN_CONFIG_SKEW_DELAY, 0 }, -}; /** * parse_dt_cfg() - Parse DT pinconf parameters @@ -247,14 +219,14 @@ int pinconf_generic_parse_dt_config(struct device_node *np, return -EINVAL; /* allocate a temporary array big enough to hold one of each option */ - max_cfg = ARRAY_SIZE(dt_params); + max_cfg = ARRAY_SIZE(pinconf_dt_params); if (pctldev) max_cfg += pctldev->desc->num_custom_params; cfg = kcalloc(max_cfg, sizeof(*cfg), GFP_KERNEL); if (!cfg) return -ENOMEM; - parse_dt_cfg(np, dt_params, ARRAY_SIZE(dt_params), cfg, &ncfg); + parse_dt_cfg(np, pinconf_dt_params, ARRAY_SIZE(pinconf_dt_params), cfg, &ncfg); if (pctldev && pctldev->desc->num_custom_params && pctldev->desc->custom_params) parse_dt_cfg(np, pctldev->desc->custom_params, @@ -409,4 +381,33 @@ void pinconf_generic_dt_free_map(struct pinctrl_dev *pctldev, } EXPORT_SYMBOL_GPL(pinconf_generic_dt_free_map); +const char *pinconf_generic_get_param_property_name(struct pinctrl_dev *pctldev, + unsigned int num_configs, unsigned long *param) +{ + struct pinctrl_desc *desc = pctldev->desc; + unsigned int num_params = ARRAY_SIZE(pinconf_dt_params), + num_custom_params = desc->num_custom_params, + i, j; + + for (i = 0; i < num_configs; i++) { + enum pin_config_param pin_param = + pinconf_to_config_param(param[i]); + + /* first search on default properties */ + for (j = 0; j < num_params; j++) { + if (pin_param == pinconf_dt_params[j].param) + return pinconf_dt_params[j].property; + } + + /* search on custom properties */ + for (j = 0; j < num_custom_params; j++) { + if (pin_param == desc->custom_params[j].param) + return desc->custom_params[j].property; + } + } + + return "property undefined"; +} +EXPORT_SYMBOL_GPL(pinconf_generic_get_param_property_name); + #endif diff --git a/include/linux/pinctrl/pinconf-generic.h b/include/linux/pinctrl/pinconf-generic.h index 6c06806..c51f4b3 100644 --- a/include/linux/pinctrl/pinconf-generic.h +++ b/include/linux/pinctrl/pinconf-generic.h @@ -184,6 +184,35 @@ struct pinconf_generic_params { u32 default_value; }; +static const struct pinconf_generic_params pinconf_dt_params[] = { + { "bias-bus-hold", PIN_CONFIG_BIAS_BUS_HOLD, 0 }, + { "bias-disable", PIN_CONFIG_BIAS_DISABLE, 0 }, + { "bias-high-impedance", PIN_CONFIG_BIAS_HIGH_IMPEDANCE, 0 }, + { "bias-pull-up", PIN_CONFIG_BIAS_PULL_UP, 1 }, + { "bias-pull-pin-default", PIN_CONFIG_BIAS_PULL_PIN_DEFAULT, 1 }, + { "bias-pull-down", PIN_CONFIG_BIAS_PULL_DOWN, 1 }, + { "drive-open-drain", PIN_CONFIG_DRIVE_OPEN_DRAIN, 0 }, + { "drive-open-source", PIN_CONFIG_DRIVE_OPEN_SOURCE, 0 }, + { "drive-push-pull", PIN_CONFIG_DRIVE_PUSH_PULL, 0 }, + { "drive-strength", PIN_CONFIG_DRIVE_STRENGTH, 0 }, + { "input-debounce", PIN_CONFIG_INPUT_DEBOUNCE, 0 }, + { "input-disable", PIN_CONFIG_INPUT_ENABLE, 0 }, + { "input-enable", PIN_CONFIG_INPUT_ENABLE, 1 }, + { "input-schmitt", PIN_CONFIG_INPUT_SCHMITT, 0 }, + { "input-schmitt-disable", PIN_CONFIG_INPUT_SCHMITT_ENABLE, 0 }, + { "input-schmitt-enable", PIN_CONFIG_INPUT_SCHMITT_ENABLE, 1 }, + { "low-power-disable", PIN_CONFIG_LOW_POWER_MODE, 0 }, + { "low-power-enable", PIN_CONFIG_LOW_POWER_MODE, 1 }, + { "output-disable", PIN_CONFIG_OUTPUT_ENABLE, 0 }, + { "output-enable", PIN_CONFIG_OUTPUT_ENABLE, 1 }, + { "output-high", PIN_CONFIG_OUTPUT, 1, }, + { "output-low", PIN_CONFIG_OUTPUT, 0, }, + { "power-source", PIN_CONFIG_POWER_SOURCE, 0 }, + { "sleep-hardware-state", PIN_CONFIG_SLEEP_HARDWARE_STATE, 0 }, + { "slew-rate", PIN_CONFIG_SLEW_RATE, 0 }, + { "skew-delay", PIN_CONFIG_SKEW_DELAY, 0 }, +}; + int pinconf_generic_dt_subnode_to_map(struct pinctrl_dev *pctldev, struct device_node *np, struct pinctrl_map **map, unsigned *reserved_maps, unsigned *num_maps, @@ -194,6 +223,9 @@ int pinconf_generic_dt_node_to_map(struct pinctrl_dev *pctldev, void pinconf_generic_dt_free_map(struct pinctrl_dev *pctldev, struct pinctrl_map *map, unsigned num_maps); +const char *pinconf_generic_get_param_property_name(struct pinctrl_dev *pctldev, + unsigned int num_configs, unsigned long *param); + static inline int pinconf_generic_dt_node_to_map_group( struct pinctrl_dev *pctldev, struct device_node *np_config, struct pinctrl_map **map, unsigned *num_maps) -- 2.7.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/2] pinctrl: generic: add API to solve generic sub-node property name 2018-05-01 19:10 ` [PATCH v2 1/2] pinctrl: generic: add API to solve generic sub-node property name Matheus Castello @ 2018-05-02 12:56 ` Linus Walleij 0 siblings, 0 replies; 11+ messages in thread From: Linus Walleij @ 2018-05-02 12:56 UTC (permalink / raw) To: Matheus Castello; +Cc: kbuild-all, linux-kernel On Tue, May 1, 2018 at 9:10 PM, Matheus Castello <matheus@castello.eng.br> wrote: > Set dt_params with the definitions of the generic sub-node properties > global in pinconf-generic.h and add a function to pinconf-generic API > to decode a packed param returning the string name from sub-node > property. > > This can be useful in debug from pinctrl-vendor driver or even for > debug in pinconf driver. > > Signed-off-by: Matheus Castello <matheus@castello.eng.br> (...) > +const char *pinconf_generic_get_param_property_name(struct pinctrl_dev *pctldev, > + unsigned int num_configs, unsigned long *param) > +{ > + struct pinctrl_desc *desc = pctldev->desc; > + unsigned int num_params = ARRAY_SIZE(pinconf_dt_params), > + num_custom_params = desc->num_custom_params, > + i, j; > + > + for (i = 0; i < num_configs; i++) { > + enum pin_config_param pin_param = > + pinconf_to_config_param(param[i]); > + > + /* first search on default properties */ > + for (j = 0; j < num_params; j++) { > + if (pin_param == pinconf_dt_params[j].param) > + return pinconf_dt_params[j].property; > + } If this is only for OF, we should rename the function to something including *of* as well. I would like it if you manage to break the dependency to OF though. > +++ b/include/linux/pinctrl/pinconf-generic.h > > +static const struct pinconf_generic_params pinconf_dt_params[] = { > + { "bias-bus-hold", PIN_CONFIG_BIAS_BUS_HOLD, 0 }, > + { "bias-disable", PIN_CONFIG_BIAS_DISABLE, 0 }, > + { "bias-high-impedance", PIN_CONFIG_BIAS_HIGH_IMPEDANCE, 0 }, (...) Is it really advisible to put big static constant arrays into header files like this? This will end up with a copy in each file where it is included and it is not a small thing that gets replicated all over the place either. What about just add it as extern and exporting the symbol if you need it elsewhere? Yours, Linus Walleij ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 2/2] pinctrl: generic: improve apply_setting error verbosity 2018-05-01 19:09 ` [PATCH v2 0/2] " Matheus Castello 2018-05-01 19:10 ` [PATCH v2 1/2] pinctrl: generic: add API to solve generic sub-node property name Matheus Castello @ 2018-05-01 19:10 ` Matheus Castello 2018-05-02 12:51 ` Linus Walleij 1 sibling, 1 reply; 11+ messages in thread From: Matheus Castello @ 2018-05-01 19:10 UTC (permalink / raw) To: linus.walleij; +Cc: kbuild-all, linux-kernel, Matheus Castello For generic pinconf: print the dev_error with the pinctrl vendor driver name, error code, the sub-node property name used and the pin that was tried to set. Improves the undestading of the error if use a generic sub-node property that generic-pinconf can do parse but the vendor pinctrl driver does not support. Signed-off-by: Matheus Castello <matheus@castello.eng.br> --- drivers/pinctrl/pinconf.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/drivers/pinctrl/pinconf.c b/drivers/pinctrl/pinconf.c index d3fe143..ced2b67 100644 --- a/drivers/pinctrl/pinconf.c +++ b/drivers/pinctrl/pinconf.c @@ -21,6 +21,7 @@ #include <linux/pinctrl/machine.h> #include <linux/pinctrl/pinctrl.h> #include <linux/pinctrl/pinconf.h> +#include <linux/pinctrl/pinconf-generic.h> #include "core.h" #include "pinconf.h" @@ -169,9 +170,21 @@ int pinconf_apply_setting(const struct pinctrl_setting *setting) setting->data.configs.configs, setting->data.configs.num_configs); if (ret < 0) { +#ifdef CONFIG_OF dev_err(pctldev->dev, - "pin_config_set op failed for pin %d\n", + "%s error %d setting %s for pin %d\n", + pctldev->desc->name, ret, + pinconf_generic_get_param_property_name( + pctldev, setting->data.configs.num_configs, + setting->data.configs.configs), setting->data.configs.group_or_pin); +#endif + + dev_err(pctldev->dev, + "pin_config_set op failed for %s pin %d\n", + pctldev->desc->name, + setting->data.configs.group_or_pin); + return ret; } break; -- 2.7.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] pinctrl: generic: improve apply_setting error verbosity 2018-05-01 19:10 ` [PATCH v2 2/2] pinctrl: generic: improve apply_setting error verbosity Matheus Castello @ 2018-05-02 12:51 ` Linus Walleij 2018-05-08 5:27 ` Matheus Castello 0 siblings, 1 reply; 11+ messages in thread From: Linus Walleij @ 2018-05-02 12:51 UTC (permalink / raw) To: Matheus Castello; +Cc: kbuild-all, linux-kernel On Tue, May 1, 2018 at 9:10 PM, Matheus Castello <matheus@castello.eng.br> wrote: > For generic pinconf: print the dev_error with the pinctrl vendor > driver name, error code, the sub-node property name used and the > pin that was tried to set. > > Improves the undestading of the error if use a generic sub-node > property that generic-pinconf can do parse but the vendor pinctrl > driver does not support. > > Signed-off-by: Matheus Castello <matheus@castello.eng.br> > +#include <linux/pinctrl/pinconf-generic.h> > #include "core.h" > #include "pinconf.h" > > @@ -169,9 +170,21 @@ int pinconf_apply_setting(const struct pinctrl_setting *setting) > setting->data.configs.configs, > setting->data.configs.num_configs); > if (ret < 0) { > +#ifdef CONFIG_OF This doesn't seem right. If this is restricted for OF only the root cause to why it is like that needs to be found and the code refactored to fit anyone, there is also ACPI support in the works I think, surely they should be able to get verbose messages. > dev_err(pctldev->dev, > - "pin_config_set op failed for pin %d\n", > + "%s error %d setting %s for pin %d\n", > + pctldev->desc->name, ret, > + pinconf_generic_get_param_property_name( > + pctldev, setting->data.configs.num_configs, > + setting->data.configs.configs), > setting->data.configs.group_or_pin); This doesn't seem right. First argument is a %d, yet this is pctldev->desc->name? Something is fishy with the argument list. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] pinctrl: generic: improve apply_setting error verbosity 2018-05-02 12:51 ` Linus Walleij @ 2018-05-08 5:27 ` Matheus Castello 0 siblings, 0 replies; 11+ messages in thread From: Matheus Castello @ 2018-05-08 5:27 UTC (permalink / raw) To: Linus Walleij; +Cc: linux-kernel Hi Linus, thanks for the tips, I will study this to break dependency from OF. Best Regards, Matheus Castello On 05/02/2018 08:51 AM, Linus Walleij wrote: > On Tue, May 1, 2018 at 9:10 PM, Matheus Castello > <matheus@castello.eng.br> wrote: > >> For generic pinconf: print the dev_error with the pinctrl vendor >> driver name, error code, the sub-node property name used and the >> pin that was tried to set. >> >> Improves the undestading of the error if use a generic sub-node >> property that generic-pinconf can do parse but the vendor pinctrl >> driver does not support. >> >> Signed-off-by: Matheus Castello <matheus@castello.eng.br> > >> +#include <linux/pinctrl/pinconf-generic.h> >> #include "core.h" >> #include "pinconf.h" >> >> @@ -169,9 +170,21 @@ int pinconf_apply_setting(const struct pinctrl_setting *setting) >> setting->data.configs.configs, >> setting->data.configs.num_configs); >> if (ret < 0) { >> +#ifdef CONFIG_OF > > This doesn't seem right. > > If this is restricted for OF only the root cause to why it is > like that needs to be found and the code refactored to fit anyone, > there is also ACPI support in the works I think, surely they should > be able to get verbose messages. > >> dev_err(pctldev->dev, >> - "pin_config_set op failed for pin %d\n", >> + "%s error %d setting %s for pin %d\n", >> + pctldev->desc->name, ret, >> + pinconf_generic_get_param_property_name( >> + pctldev, setting->data.configs.num_configs, >> + setting->data.configs.configs), >> setting->data.configs.group_or_pin); > > This doesn't seem right. First argument is a %d, yet this > is pctldev->desc->name? > > Something is fishy with the argument list. > > Yours, > Linus Walleij > ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-05-08 5:27 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-05-01 7:40 [PATCH v1 0/2] pinctrl: generic: improve apply_setting error verbosity Matheus Castello 2018-05-01 7:40 ` [PATCH v1 1/2] pinctrl: generic: add API to solve generic sub-node property name Matheus Castello 2018-05-01 7:40 ` [PATCH v1 2/2] pinctrl: generic: improve apply_setting error verbosity Matheus Castello 2018-05-01 8:34 ` kbuild test robot 2018-05-01 10:05 ` kbuild test robot 2018-05-01 19:09 ` [PATCH v2 0/2] " Matheus Castello 2018-05-01 19:10 ` [PATCH v2 1/2] pinctrl: generic: add API to solve generic sub-node property name Matheus Castello 2018-05-02 12:56 ` Linus Walleij 2018-05-01 19:10 ` [PATCH v2 2/2] pinctrl: generic: improve apply_setting error verbosity Matheus Castello 2018-05-02 12:51 ` Linus Walleij 2018-05-08 5:27 ` Matheus Castello
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.