This is needed for properly registering gpio regmap as a child of a regmap
pin controller.
Signed-off-by: Álvaro Fernández Rojas <[email protected]>
Reviewed-by: Michael Walle <[email protected]>
---
v2: split this patch from the bcm63xx-pinctrl series
drivers/gpio/gpio-regmap.c | 1 +
include/linux/gpio/regmap.h | 3 +++
2 files changed, 4 insertions(+)
diff --git a/drivers/gpio/gpio-regmap.c b/drivers/gpio/gpio-regmap.c
index fed1e269c42a..8898ab3e1d59 100644
--- a/drivers/gpio/gpio-regmap.c
+++ b/drivers/gpio/gpio-regmap.c
@@ -249,6 +249,7 @@ struct gpio_regmap *gpio_regmap_register(const struct gpio_regmap_config *config
chip = &gpio->gpio_chip;
chip->parent = config->parent;
+ chip->of_node = config->of_node ?: dev_of_node(config->parent);
chip->base = -1;
chip->ngpio = config->ngpio;
chip->names = config->names;
diff --git a/include/linux/gpio/regmap.h b/include/linux/gpio/regmap.h
index ad76f3d0a6ba..566d76d0dbae 100644
--- a/include/linux/gpio/regmap.h
+++ b/include/linux/gpio/regmap.h
@@ -4,6 +4,7 @@
#define _LINUX_GPIO_REGMAP_H
struct device;
+struct device_node;
struct gpio_regmap;
struct irq_domain;
struct regmap;
@@ -15,6 +16,7 @@ struct regmap;
* struct gpio_regmap_config - Description of a generic regmap gpio_chip.
* @parent: The parent device
* @regmap: The regmap used to access the registers
+ * @of_node: (Optional) The device node
* given, the name of the device is used
* @label: (Optional) Descriptive name for GPIO controller.
* If not given, the name of the device is used.
@@ -57,6 +59,7 @@ struct regmap;
struct gpio_regmap_config {
struct device *parent;
struct regmap *regmap;
+ struct device_node *of_node;
const char *label;
int ngpio;
--
2.20.1
On Thu, Mar 4, 2021 at 5:18 PM Álvaro Fernández Rojas <[email protected]> wrote:
>
> This is needed for properly registering gpio regmap as a child of a regmap
> pin controller.
> + chip->of_node = config->of_node ?: dev_of_node(config->parent);
After a closer look I have no clue why you need this patch at all.
The second part, i.e. assigning parent's fwnode, is done already in
the GPIO library core.
The first part, keeping fwnode in the regmap configuration puzzles me. Why?
--
With Best Regards,
Andy Shevchenko
Hi Andy,
> El 4 mar 2021, a las 16:22, Andy Shevchenko <[email protected]> escribió:
>
> On Thu, Mar 4, 2021 at 5:18 PM Álvaro Fernández Rojas <[email protected]> wrote:
>>
>> This is needed for properly registering gpio regmap as a child of a regmap
>> pin controller.
>
>> + chip->of_node = config->of_node ?: dev_of_node(config->parent);
>
> After a closer look I have no clue why you need this patch at all.
> The second part, i.e. assigning parent's fwnode, is done already in
> the GPIO library core.
> The first part, keeping fwnode in the regmap configuration puzzles me. Why?
I’ve flagged this as superseded since Linus asked me to send it with bcm63xx patches and I’ve already answered this same question there.
>
> --
> With Best Regards,
> Andy Shevchenko
Best regards,
Álvaro.
Am 2021-03-04 16:22, schrieb Andy Shevchenko:
> On Thu, Mar 4, 2021 at 5:18 PM Álvaro Fernández Rojas
> <[email protected]> wrote:
>>
>> This is needed for properly registering gpio regmap as a child of a
>> regmap
>> pin controller.
>
>> + chip->of_node = config->of_node ?:
>> dev_of_node(config->parent);
>
> After a closer look I have no clue why you need this patch at all.
> The second part, i.e. assigning parent's fwnode, is done already in
> the GPIO library core.
> The first part, keeping fwnode in the regmap configuration puzzles me.
> Why?
You're right if chip->of_node is not set it will eventually be set to
node of the parent device.
In case of the BCM driver the parent device is the pinctrl device and
the node is a children (if that is correct I cannot say). So, in this
case you cannot use the node of the parent, because that would be the
pinctrl one.
You could just use
chip->of_node = dev_of_node(config->parent)
But then it is not obvious that it is optional. Hence my suggestion
to explicitly set to the parent of_node if none is supplied.
-michael
Am 2021-03-04 08:15, schrieb Álvaro Fernández Rojas:
> This is needed for properly registering gpio regmap as a child of a
> regmap
> pin controller.
>
> Signed-off-by: Álvaro Fernández Rojas <[email protected]>
> Reviewed-by: Michael Walle <[email protected]>
> ---
> v2: split this patch from the bcm63xx-pinctrl series
>
> drivers/gpio/gpio-regmap.c | 1 +
> include/linux/gpio/regmap.h | 3 +++
> 2 files changed, 4 insertions(+)
>
> diff --git a/drivers/gpio/gpio-regmap.c b/drivers/gpio/gpio-regmap.c
> index fed1e269c42a..8898ab3e1d59 100644
> --- a/drivers/gpio/gpio-regmap.c
> +++ b/drivers/gpio/gpio-regmap.c
> @@ -249,6 +249,7 @@ struct gpio_regmap *gpio_regmap_register(const
> struct gpio_regmap_config *config
>
> chip = &gpio->gpio_chip;
> chip->parent = config->parent;
> + chip->of_node = config->of_node ?: dev_of_node(config->parent);
> chip->base = -1;
> chip->ngpio = config->ngpio;
> chip->names = config->names;
> diff --git a/include/linux/gpio/regmap.h b/include/linux/gpio/regmap.h
> index ad76f3d0a6ba..566d76d0dbae 100644
> --- a/include/linux/gpio/regmap.h
> +++ b/include/linux/gpio/regmap.h
> @@ -4,6 +4,7 @@
> #define _LINUX_GPIO_REGMAP_H
>
> struct device;
> +struct device_node;
> struct gpio_regmap;
> struct irq_domain;
> struct regmap;
> @@ -15,6 +16,7 @@ struct regmap;
> * struct gpio_regmap_config - Description of a generic regmap
> gpio_chip.
> * @parent: The parent device
> * @regmap: The regmap used to access the registers
> + * @of_node: (Optional) The device node
> * given, the name of the device is used
Something is messed up here ;) This line should be together with
the one containing @regmap. While at it please add the
"If not given, the of_node of the parent device is used."
-michael
> * @label: (Optional) Descriptive name for GPIO controller.
> * If not given, the name of the device is used.
> @@ -57,6 +59,7 @@ struct regmap;
> struct gpio_regmap_config {
> struct device *parent;
> struct regmap *regmap;
> + struct device_node *of_node;
>
> const char *label;
> int ngpio;
Hi Michael,
> El 4 mar 2021, a las 9:18, Michael Walle <[email protected]> escribió:
>
> Am 2021-03-04 08:15, schrieb Álvaro Fernández Rojas:
>> This is needed for properly registering gpio regmap as a child of a regmap
>> pin controller.
>> Signed-off-by: Álvaro Fernández Rojas <[email protected]>
>> Reviewed-by: Michael Walle <[email protected]>
>> ---
>> v2: split this patch from the bcm63xx-pinctrl series
>> drivers/gpio/gpio-regmap.c | 1 +
>> include/linux/gpio/regmap.h | 3 +++
>> 2 files changed, 4 insertions(+)
>> diff --git a/drivers/gpio/gpio-regmap.c b/drivers/gpio/gpio-regmap.c
>> index fed1e269c42a..8898ab3e1d59 100644
>> --- a/drivers/gpio/gpio-regmap.c
>> +++ b/drivers/gpio/gpio-regmap.c
>> @@ -249,6 +249,7 @@ struct gpio_regmap *gpio_regmap_register(const
>> struct gpio_regmap_config *config
>> chip = &gpio->gpio_chip;
>> chip->parent = config->parent;
>> + chip->of_node = config->of_node ?: dev_of_node(config->parent);
>> chip->base = -1;
>> chip->ngpio = config->ngpio;
>> chip->names = config->names;
>> diff --git a/include/linux/gpio/regmap.h b/include/linux/gpio/regmap.h
>> index ad76f3d0a6ba..566d76d0dbae 100644
>> --- a/include/linux/gpio/regmap.h
>> +++ b/include/linux/gpio/regmap.h
>> @@ -4,6 +4,7 @@
>> #define _LINUX_GPIO_REGMAP_H
>> struct device;
>> +struct device_node;
>> struct gpio_regmap;
>> struct irq_domain;
>> struct regmap;
>> @@ -15,6 +16,7 @@ struct regmap;
>> * struct gpio_regmap_config - Description of a generic regmap gpio_chip.
>> * @parent: The parent device
>> * @regmap: The regmap used to access the registers
>> + * @of_node: (Optional) The device node
>> * given, the name of the device is used
>
> Something is messed up here ;) This line should be together with
> the one containing @regmap. While at it please add the
> "If not given, the of_node of the parent device is used."
I think I was still sleeping when I did this…
Excuse me for that :(
>
> -michael
>
>> * @label: (Optional) Descriptive name for GPIO controller.
>> * If not given, the name of the device is used.
>> @@ -57,6 +59,7 @@ struct regmap;
>> struct gpio_regmap_config {
>> struct device *parent;
>> struct regmap *regmap;
>> + struct device_node *of_node;
>> const char *label;
>> int ngpio;
Best regards,
Álvaro.
Hi "?lvaro,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on gpio/for-next]
[also build test ERROR on v5.12-rc3 next-20210317]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/lvaro-Fern-ndez-Rojas/gpio-regmap-set-gpio_chip-of_node/20210304-152016
base: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git for-next
config: x86_64-randconfig-a013-20210317 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 8ef111222a3dd12a9175f69c3bff598c46e8bdf7)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
# https://github.com/0day-ci/linux/commit/832a71aa1e741f0ef3b28952bf53627a820a7d68
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review lvaro-Fern-ndez-Rojas/gpio-regmap-set-gpio_chip-of_node/20210304-152016
git checkout 832a71aa1e741f0ef3b28952bf53627a820a7d68
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
>> drivers/gpio/gpio-regmap.c:252:8: error: no member named 'of_node' in 'struct gpio_chip'
chip->of_node = config->of_node ?: dev_of_node(config->parent);
~~~~ ^
1 error generated.
vim +252 drivers/gpio/gpio-regmap.c
192
193 /**
194 * gpio_regmap_register() - Register a generic regmap GPIO controller
195 * @config: configuration for gpio_regmap
196 *
197 * Return: A pointer to the registered gpio_regmap or ERR_PTR error value.
198 */
199 struct gpio_regmap *gpio_regmap_register(const struct gpio_regmap_config *config)
200 {
201 struct gpio_regmap *gpio;
202 struct gpio_chip *chip;
203 int ret;
204
205 if (!config->parent)
206 return ERR_PTR(-EINVAL);
207
208 if (!config->ngpio)
209 return ERR_PTR(-EINVAL);
210
211 /* we need at least one */
212 if (!config->reg_dat_base && !config->reg_set_base)
213 return ERR_PTR(-EINVAL);
214
215 /* if we have a direction register we need both input and output */
216 if ((config->reg_dir_out_base || config->reg_dir_in_base) &&
217 (!config->reg_dat_base || !config->reg_set_base))
218 return ERR_PTR(-EINVAL);
219
220 /* we don't support having both registers simultaneously for now */
221 if (config->reg_dir_out_base && config->reg_dir_in_base)
222 return ERR_PTR(-EINVAL);
223
224 gpio = kzalloc(sizeof(*gpio), GFP_KERNEL);
225 if (!gpio)
226 return ERR_PTR(-ENOMEM);
227
228 gpio->parent = config->parent;
229 gpio->regmap = config->regmap;
230 gpio->ngpio_per_reg = config->ngpio_per_reg;
231 gpio->reg_stride = config->reg_stride;
232 gpio->reg_mask_xlate = config->reg_mask_xlate;
233 gpio->reg_dat_base = config->reg_dat_base;
234 gpio->reg_set_base = config->reg_set_base;
235 gpio->reg_clr_base = config->reg_clr_base;
236 gpio->reg_dir_in_base = config->reg_dir_in_base;
237 gpio->reg_dir_out_base = config->reg_dir_out_base;
238
239 /* if not set, assume there is only one register */
240 if (!gpio->ngpio_per_reg)
241 gpio->ngpio_per_reg = config->ngpio;
242
243 /* if not set, assume they are consecutive */
244 if (!gpio->reg_stride)
245 gpio->reg_stride = 1;
246
247 if (!gpio->reg_mask_xlate)
248 gpio->reg_mask_xlate = gpio_regmap_simple_xlate;
249
250 chip = &gpio->gpio_chip;
251 chip->parent = config->parent;
> 252 chip->of_node = config->of_node ?: dev_of_node(config->parent);
253 chip->base = -1;
254 chip->ngpio = config->ngpio;
255 chip->names = config->names;
256 chip->label = config->label ?: dev_name(config->parent);
257
258 /*
259 * If our regmap is fast_io we should probably set can_sleep to false.
260 * Right now, the regmap doesn't save this property, nor is there any
261 * access function for it.
262 * The only regmap type which uses fast_io is regmap-mmio. For now,
263 * assume a safe default of true here.
264 */
265 chip->can_sleep = true;
266
267 chip->get = gpio_regmap_get;
268 if (gpio->reg_set_base && gpio->reg_clr_base)
269 chip->set = gpio_regmap_set_with_clear;
270 else if (gpio->reg_set_base)
271 chip->set = gpio_regmap_set;
272
273 if (gpio->reg_dir_in_base || gpio->reg_dir_out_base) {
274 chip->get_direction = gpio_regmap_get_direction;
275 chip->direction_input = gpio_regmap_direction_input;
276 chip->direction_output = gpio_regmap_direction_output;
277 }
278
279 ret = gpiochip_add_data(chip, gpio);
280 if (ret < 0)
281 goto err_free_gpio;
282
283 if (config->irq_domain) {
284 ret = gpiochip_irqchip_add_domain(chip, config->irq_domain);
285 if (ret)
286 goto err_remove_gpiochip;
287 }
288
289 return gpio;
290
291 err_remove_gpiochip:
292 gpiochip_remove(chip);
293 err_free_gpio:
294 kfree(gpio);
295 return ERR_PTR(ret);
296 }
297 EXPORT_SYMBOL_GPL(gpio_regmap_register);
298
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]