Re: [PATCH v2 3/4] gpio: aspeed: Create llops to handle hardware access

Re: [PATCH v2 3/4] gpio: aspeed: Create llops to handle hardware access

[kernel test robot]

BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <20240830034047.2251482-4-billy_tsai@aspeedtech.com>
References: <20240830034047.2251482-4-billy_tsai@aspeedtech.com>
TO: Billy Tsai <billy_tsai@aspeedtech.com>
TO: linus.walleij@linaro.org
TO: brgl@bgdev.pl
TO: robh@kernel.org
TO: krzk+dt@kernel.org
TO: conor+dt@kernel.org
TO: joel@jms.id.au
TO: andrew@codeconstruct.com.au
TO: linux-gpio@vger.kernel.org
TO: devicetree@vger.kernel.org
TO: linux-arm-kernel@lists.infradead.org
TO: linux-aspeed@lists.ozlabs.org
TO: linux-kernel@vger.kernel.org
TO: BMC-SW@aspeedtech.com

Hi Billy,

kernel test robot noticed the following build warnings:

[auto build test WARNING on brgl/gpio/for-next]
[also build test WARNING on linus/master v6.11-rc5 next-20240830]
[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#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Billy-Tsai/dt-bindings-gpio-aspeed-ast2400-gpio-Support-ast2700/20240830-114325
base:   https://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git gpio/for-next
patch link:    https://lore.kernel.org/r/20240830034047.2251482-4-billy_tsai%40aspeedtech.com
patch subject: [PATCH v2 3/4] gpio: aspeed: Create llops to handle hardware access
:::::: branch date: 2 days ago
:::::: commit date: 2 days ago
config: parisc-randconfig-r071-20240831 (https://download.01.org/0day-ci/archive/20240831/202408312313.HTx2vwvy-lkp@intel.com/config)
compiler: hppa-linux-gcc (GCC) 14.1.0

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Closes: https://lore.kernel.org/r/202408312313.HTx2vwvy-lkp@intel.com/

smatch warnings:
drivers/gpio/gpio-aspeed.c:399 aspeed_gpio_set() error: uninitialized symbol 'copro'.
drivers/gpio/gpio-aspeed.c:418 aspeed_gpio_dir_in() error: uninitialized symbol 'copro'.
drivers/gpio/gpio-aspeed.c:443 aspeed_gpio_dir_out() error: uninitialized symbol 'copro'.
drivers/gpio/gpio-aspeed.c:507 aspeed_gpio_irq_ack() error: uninitialized symbol 'copro'.
drivers/gpio/gpio-aspeed.c:533 aspeed_gpio_irq_set_mask() error: uninitialized symbol 'copro'.
drivers/gpio/gpio-aspeed.c:596 aspeed_gpio_set_type() error: uninitialized symbol 'copro'.
drivers/gpio/gpio-aspeed.c:664 aspeed_gpio_reset_tolerance() error: uninitialized symbol 'copro'.

vim +/copro +399 drivers/gpio/gpio-aspeed.c

361b79119a4b7f Joel Stanley           2016-08-30  385  
361b79119a4b7f Joel Stanley           2016-08-30  386  static void aspeed_gpio_set(struct gpio_chip *gc, unsigned int offset,
361b79119a4b7f Joel Stanley           2016-08-30  387  			    int val)
361b79119a4b7f Joel Stanley           2016-08-30  388  {
361b79119a4b7f Joel Stanley           2016-08-30  389  	struct aspeed_gpio *gpio = gpiochip_get_data(gc);
361b79119a4b7f Joel Stanley           2016-08-30  390  	unsigned long flags;
a7ca13826e478f Benjamin Herrenschmidt 2018-06-29  391  	bool copro;
361b79119a4b7f Joel Stanley           2016-08-30  392  
61a7904b6ace99 Iwona Winiarska        2021-12-04  393  	raw_spin_lock_irqsave(&gpio->lock, flags);
0e6ca482ec6e28 Billy Tsai             2024-08-30  394  	if (gpio->llops->copro_request)
0e6ca482ec6e28 Billy Tsai             2024-08-30  395  		copro = gpio->llops->copro_request(gpio, offset);
361b79119a4b7f Joel Stanley           2016-08-30  396  
361b79119a4b7f Joel Stanley           2016-08-30  397  	__aspeed_gpio_set(gc, offset, val);
361b79119a4b7f Joel Stanley           2016-08-30  398  
0e6ca482ec6e28 Billy Tsai             2024-08-30 @399  	if (copro && gpio->llops->copro_release)
0e6ca482ec6e28 Billy Tsai             2024-08-30  400  		gpio->llops->copro_release(gpio, offset);
61a7904b6ace99 Iwona Winiarska        2021-12-04  401  	raw_spin_unlock_irqrestore(&gpio->lock, flags);
361b79119a4b7f Joel Stanley           2016-08-30  402  }
361b79119a4b7f Joel Stanley           2016-08-30  403  
361b79119a4b7f Joel Stanley           2016-08-30  404  static int aspeed_gpio_dir_in(struct gpio_chip *gc, unsigned int offset)
361b79119a4b7f Joel Stanley           2016-08-30  405  {
361b79119a4b7f Joel Stanley           2016-08-30  406  	struct aspeed_gpio *gpio = gpiochip_get_data(gc);
361b79119a4b7f Joel Stanley           2016-08-30  407  	unsigned long flags;
a7ca13826e478f Benjamin Herrenschmidt 2018-06-29  408  	bool copro;
361b79119a4b7f Joel Stanley           2016-08-30  409  
1736f75d35e474 Andrew Jeffery         2017-01-24  410  	if (!have_input(gpio, offset))
1736f75d35e474 Andrew Jeffery         2017-01-24  411  		return -ENOTSUPP;
1736f75d35e474 Andrew Jeffery         2017-01-24  412  
61a7904b6ace99 Iwona Winiarska        2021-12-04  413  	raw_spin_lock_irqsave(&gpio->lock, flags);
361b79119a4b7f Joel Stanley           2016-08-30  414  
0e6ca482ec6e28 Billy Tsai             2024-08-30  415  	if (gpio->llops->copro_request)
0e6ca482ec6e28 Billy Tsai             2024-08-30  416  		copro = gpio->llops->copro_request(gpio, offset);
0e6ca482ec6e28 Billy Tsai             2024-08-30  417  	gpio->llops->reg_bits_set(gpio, offset, reg_dir, 0);
0e6ca482ec6e28 Billy Tsai             2024-08-30 @418  	if (copro && gpio->llops->copro_release)
0e6ca482ec6e28 Billy Tsai             2024-08-30  419  		gpio->llops->copro_release(gpio, offset);
361b79119a4b7f Joel Stanley           2016-08-30  420  
61a7904b6ace99 Iwona Winiarska        2021-12-04  421  	raw_spin_unlock_irqrestore(&gpio->lock, flags);
361b79119a4b7f Joel Stanley           2016-08-30  422  
361b79119a4b7f Joel Stanley           2016-08-30  423  	return 0;
361b79119a4b7f Joel Stanley           2016-08-30  424  }
361b79119a4b7f Joel Stanley           2016-08-30  425  
361b79119a4b7f Joel Stanley           2016-08-30  426  static int aspeed_gpio_dir_out(struct gpio_chip *gc,
361b79119a4b7f Joel Stanley           2016-08-30  427  			       unsigned int offset, int val)
361b79119a4b7f Joel Stanley           2016-08-30  428  {
361b79119a4b7f Joel Stanley           2016-08-30  429  	struct aspeed_gpio *gpio = gpiochip_get_data(gc);
361b79119a4b7f Joel Stanley           2016-08-30  430  	unsigned long flags;
a7ca13826e478f Benjamin Herrenschmidt 2018-06-29  431  	bool copro;
361b79119a4b7f Joel Stanley           2016-08-30  432  
1736f75d35e474 Andrew Jeffery         2017-01-24  433  	if (!have_output(gpio, offset))
1736f75d35e474 Andrew Jeffery         2017-01-24  434  		return -ENOTSUPP;
1736f75d35e474 Andrew Jeffery         2017-01-24  435  
61a7904b6ace99 Iwona Winiarska        2021-12-04  436  	raw_spin_lock_irqsave(&gpio->lock, flags);
361b79119a4b7f Joel Stanley           2016-08-30  437  
0e6ca482ec6e28 Billy Tsai             2024-08-30  438  	if (gpio->llops->copro_request)
0e6ca482ec6e28 Billy Tsai             2024-08-30  439  		copro = gpio->llops->copro_request(gpio, offset);
af7949284910a1 Benjamin Herrenschmidt 2018-05-17  440  	__aspeed_gpio_set(gc, offset, val);
0e6ca482ec6e28 Billy Tsai             2024-08-30  441  	gpio->llops->reg_bits_set(gpio, offset, reg_dir, 1);
361b79119a4b7f Joel Stanley           2016-08-30  442  
0e6ca482ec6e28 Billy Tsai             2024-08-30 @443  	if (copro && gpio->llops->copro_release)
0e6ca482ec6e28 Billy Tsai             2024-08-30  444  		gpio->llops->copro_release(gpio, offset);
61a7904b6ace99 Iwona Winiarska        2021-12-04  445  	raw_spin_unlock_irqrestore(&gpio->lock, flags);
361b79119a4b7f Joel Stanley           2016-08-30  446  
361b79119a4b7f Joel Stanley           2016-08-30  447  	return 0;
361b79119a4b7f Joel Stanley           2016-08-30  448  }
361b79119a4b7f Joel Stanley           2016-08-30  449  
361b79119a4b7f Joel Stanley           2016-08-30  450  static int aspeed_gpio_get_direction(struct gpio_chip *gc, unsigned int offset)
361b79119a4b7f Joel Stanley           2016-08-30  451  {
361b79119a4b7f Joel Stanley           2016-08-30  452  	struct aspeed_gpio *gpio = gpiochip_get_data(gc);
361b79119a4b7f Joel Stanley           2016-08-30  453  	unsigned long flags;
361b79119a4b7f Joel Stanley           2016-08-30  454  	u32 val;
361b79119a4b7f Joel Stanley           2016-08-30  455  
1736f75d35e474 Andrew Jeffery         2017-01-24  456  	if (!have_input(gpio, offset))
e42615ec233b30 Matti Vaittinen        2019-11-06  457  		return GPIO_LINE_DIRECTION_OUT;
1736f75d35e474 Andrew Jeffery         2017-01-24  458  
1736f75d35e474 Andrew Jeffery         2017-01-24  459  	if (!have_output(gpio, offset))
e42615ec233b30 Matti Vaittinen        2019-11-06  460  		return GPIO_LINE_DIRECTION_IN;
1736f75d35e474 Andrew Jeffery         2017-01-24  461  
61a7904b6ace99 Iwona Winiarska        2021-12-04  462  	raw_spin_lock_irqsave(&gpio->lock, flags);
361b79119a4b7f Joel Stanley           2016-08-30  463  
0e6ca482ec6e28 Billy Tsai             2024-08-30  464  	val = gpio->llops->reg_bits_read(gpio, offset, reg_dir);
361b79119a4b7f Joel Stanley           2016-08-30  465  
61a7904b6ace99 Iwona Winiarska        2021-12-04  466  	raw_spin_unlock_irqrestore(&gpio->lock, flags);
361b79119a4b7f Joel Stanley           2016-08-30  467  
e42615ec233b30 Matti Vaittinen        2019-11-06  468  	return val ? GPIO_LINE_DIRECTION_OUT : GPIO_LINE_DIRECTION_IN;
361b79119a4b7f Joel Stanley           2016-08-30  469  }
361b79119a4b7f Joel Stanley           2016-08-30  470  
361b79119a4b7f Joel Stanley           2016-08-30  471  static inline int irqd_to_aspeed_gpio_data(struct irq_data *d,
361b79119a4b7f Joel Stanley           2016-08-30  472  					   struct aspeed_gpio **gpio,
0e6ca482ec6e28 Billy Tsai             2024-08-30  473  					   int *offset)
361b79119a4b7f Joel Stanley           2016-08-30  474  {
1736f75d35e474 Andrew Jeffery         2017-01-24  475  	struct aspeed_gpio *internal;
361b79119a4b7f Joel Stanley           2016-08-30  476  
a7ca13826e478f Benjamin Herrenschmidt 2018-06-29  477  	*offset = irqd_to_hwirq(d);
361b79119a4b7f Joel Stanley           2016-08-30  478  
1736f75d35e474 Andrew Jeffery         2017-01-24  479  	internal = irq_data_get_irq_chip_data(d);
1736f75d35e474 Andrew Jeffery         2017-01-24  480  
1736f75d35e474 Andrew Jeffery         2017-01-24  481  	/* This might be a bit of a questionable place to check */
a7ca13826e478f Benjamin Herrenschmidt 2018-06-29  482  	if (!have_irq(internal, *offset))
1736f75d35e474 Andrew Jeffery         2017-01-24  483  		return -ENOTSUPP;
1736f75d35e474 Andrew Jeffery         2017-01-24  484  
1736f75d35e474 Andrew Jeffery         2017-01-24  485  	*gpio = internal;
361b79119a4b7f Joel Stanley           2016-08-30  486  
361b79119a4b7f Joel Stanley           2016-08-30  487  	return 0;
361b79119a4b7f Joel Stanley           2016-08-30  488  }
361b79119a4b7f Joel Stanley           2016-08-30  489  
361b79119a4b7f Joel Stanley           2016-08-30  490  static void aspeed_gpio_irq_ack(struct irq_data *d)
361b79119a4b7f Joel Stanley           2016-08-30  491  {
361b79119a4b7f Joel Stanley           2016-08-30  492  	struct aspeed_gpio *gpio;
361b79119a4b7f Joel Stanley           2016-08-30  493  	unsigned long flags;
a7ca13826e478f Benjamin Herrenschmidt 2018-06-29  494  	int rc, offset;
a7ca13826e478f Benjamin Herrenschmidt 2018-06-29  495  	bool copro;
361b79119a4b7f Joel Stanley           2016-08-30  496  
0e6ca482ec6e28 Billy Tsai             2024-08-30  497  	rc = irqd_to_aspeed_gpio_data(d, &gpio, &offset);
361b79119a4b7f Joel Stanley           2016-08-30  498  	if (rc)
361b79119a4b7f Joel Stanley           2016-08-30  499  		return;
361b79119a4b7f Joel Stanley           2016-08-30  500  
61a7904b6ace99 Iwona Winiarska        2021-12-04  501  	raw_spin_lock_irqsave(&gpio->lock, flags);
0e6ca482ec6e28 Billy Tsai             2024-08-30  502  	if (gpio->llops->copro_request)
0e6ca482ec6e28 Billy Tsai             2024-08-30  503  		copro = gpio->llops->copro_request(gpio, offset);
a7ca13826e478f Benjamin Herrenschmidt 2018-06-29  504  
0e6ca482ec6e28 Billy Tsai             2024-08-30  505  	gpio->llops->reg_bits_set(gpio, offset, reg_irq_status, 1);
a7ca13826e478f Benjamin Herrenschmidt 2018-06-29  506  
0e6ca482ec6e28 Billy Tsai             2024-08-30 @507  	if (copro && gpio->llops->copro_release)
0e6ca482ec6e28 Billy Tsai             2024-08-30  508  		gpio->llops->copro_release(gpio, offset);
61a7904b6ace99 Iwona Winiarska        2021-12-04  509  	raw_spin_unlock_irqrestore(&gpio->lock, flags);
361b79119a4b7f Joel Stanley           2016-08-30  510  }
361b79119a4b7f Joel Stanley           2016-08-30  511  
361b79119a4b7f Joel Stanley           2016-08-30  512  static void aspeed_gpio_irq_set_mask(struct irq_data *d, bool set)
361b79119a4b7f Joel Stanley           2016-08-30  513  {
361b79119a4b7f Joel Stanley           2016-08-30  514  	struct aspeed_gpio *gpio;
361b79119a4b7f Joel Stanley           2016-08-30  515  	unsigned long flags;
a7ca13826e478f Benjamin Herrenschmidt 2018-06-29  516  	int rc, offset;
a7ca13826e478f Benjamin Herrenschmidt 2018-06-29  517  	bool copro;
361b79119a4b7f Joel Stanley           2016-08-30  518  
0e6ca482ec6e28 Billy Tsai             2024-08-30  519  	rc = irqd_to_aspeed_gpio_data(d, &gpio, &offset);
361b79119a4b7f Joel Stanley           2016-08-30  520  	if (rc)
361b79119a4b7f Joel Stanley           2016-08-30  521  		return;
361b79119a4b7f Joel Stanley           2016-08-30  522  
061df08f063a97 Linus Walleij          2023-03-09  523  	/* Unmasking the IRQ */
061df08f063a97 Linus Walleij          2023-03-09  524  	if (set)
061df08f063a97 Linus Walleij          2023-03-09  525  		gpiochip_enable_irq(&gpio->chip, irqd_to_hwirq(d));
061df08f063a97 Linus Walleij          2023-03-09  526  
61a7904b6ace99 Iwona Winiarska        2021-12-04  527  	raw_spin_lock_irqsave(&gpio->lock, flags);
0e6ca482ec6e28 Billy Tsai             2024-08-30  528  	if (gpio->llops->copro_request)
0e6ca482ec6e28 Billy Tsai             2024-08-30  529  		copro = gpio->llops->copro_request(gpio, offset);
361b79119a4b7f Joel Stanley           2016-08-30  530  
0e6ca482ec6e28 Billy Tsai             2024-08-30  531  	gpio->llops->reg_bits_set(gpio, offset, reg_irq_enable, set);
361b79119a4b7f Joel Stanley           2016-08-30  532  
0e6ca482ec6e28 Billy Tsai             2024-08-30 @533  	if (copro && gpio->llops->copro_release)
0e6ca482ec6e28 Billy Tsai             2024-08-30  534  		gpio->llops->copro_release(gpio, offset);
61a7904b6ace99 Iwona Winiarska        2021-12-04  535  	raw_spin_unlock_irqrestore(&gpio->lock, flags);
061df08f063a97 Linus Walleij          2023-03-09  536  
061df08f063a97 Linus Walleij          2023-03-09  537  	/* Masking the IRQ */
061df08f063a97 Linus Walleij          2023-03-09  538  	if (!set)
061df08f063a97 Linus Walleij          2023-03-09  539  		gpiochip_disable_irq(&gpio->chip, irqd_to_hwirq(d));
361b79119a4b7f Joel Stanley           2016-08-30  540  }
361b79119a4b7f Joel Stanley           2016-08-30  541  
361b79119a4b7f Joel Stanley           2016-08-30  542  static void aspeed_gpio_irq_mask(struct irq_data *d)
361b79119a4b7f Joel Stanley           2016-08-30  543  {
361b79119a4b7f Joel Stanley           2016-08-30  544  	aspeed_gpio_irq_set_mask(d, false);
361b79119a4b7f Joel Stanley           2016-08-30  545  }
361b79119a4b7f Joel Stanley           2016-08-30  546  
361b79119a4b7f Joel Stanley           2016-08-30  547  static void aspeed_gpio_irq_unmask(struct irq_data *d)
361b79119a4b7f Joel Stanley           2016-08-30  548  {
361b79119a4b7f Joel Stanley           2016-08-30  549  	aspeed_gpio_irq_set_mask(d, true);
361b79119a4b7f Joel Stanley           2016-08-30  550  }
361b79119a4b7f Joel Stanley           2016-08-30  551  
361b79119a4b7f Joel Stanley           2016-08-30  552  static int aspeed_gpio_set_type(struct irq_data *d, unsigned int type)
361b79119a4b7f Joel Stanley           2016-08-30  553  {
361b79119a4b7f Joel Stanley           2016-08-30  554  	u32 type0 = 0;
361b79119a4b7f Joel Stanley           2016-08-30  555  	u32 type1 = 0;
361b79119a4b7f Joel Stanley           2016-08-30  556  	u32 type2 = 0;
361b79119a4b7f Joel Stanley           2016-08-30  557  	irq_flow_handler_t handler;
361b79119a4b7f Joel Stanley           2016-08-30  558  	struct aspeed_gpio *gpio;
361b79119a4b7f Joel Stanley           2016-08-30  559  	unsigned long flags;
a7ca13826e478f Benjamin Herrenschmidt 2018-06-29  560  	int rc, offset;
a7ca13826e478f Benjamin Herrenschmidt 2018-06-29  561  	bool copro;
361b79119a4b7f Joel Stanley           2016-08-30  562  
0e6ca482ec6e28 Billy Tsai             2024-08-30  563  	rc = irqd_to_aspeed_gpio_data(d, &gpio, &offset);
361b79119a4b7f Joel Stanley           2016-08-30  564  	if (rc)
361b79119a4b7f Joel Stanley           2016-08-30  565  		return -EINVAL;
361b79119a4b7f Joel Stanley           2016-08-30  566  
361b79119a4b7f Joel Stanley           2016-08-30  567  	switch (type & IRQ_TYPE_SENSE_MASK) {
361b79119a4b7f Joel Stanley           2016-08-30  568  	case IRQ_TYPE_EDGE_BOTH:
0e6ca482ec6e28 Billy Tsai             2024-08-30  569  		type2 = 1;
df561f6688fef7 Gustavo A. R. Silva    2020-08-23  570  		fallthrough;
361b79119a4b7f Joel Stanley           2016-08-30  571  	case IRQ_TYPE_EDGE_RISING:
0e6ca482ec6e28 Billy Tsai             2024-08-30  572  		type0 = 1;
df561f6688fef7 Gustavo A. R. Silva    2020-08-23  573  		fallthrough;
361b79119a4b7f Joel Stanley           2016-08-30  574  	case IRQ_TYPE_EDGE_FALLING:
361b79119a4b7f Joel Stanley           2016-08-30  575  		handler = handle_edge_irq;
361b79119a4b7f Joel Stanley           2016-08-30  576  		break;
361b79119a4b7f Joel Stanley           2016-08-30  577  	case IRQ_TYPE_LEVEL_HIGH:
0e6ca482ec6e28 Billy Tsai             2024-08-30  578  		type0 = 1;
df561f6688fef7 Gustavo A. R. Silva    2020-08-23  579  		fallthrough;
361b79119a4b7f Joel Stanley           2016-08-30  580  	case IRQ_TYPE_LEVEL_LOW:
0e6ca482ec6e28 Billy Tsai             2024-08-30  581  		type1 = 1;
361b79119a4b7f Joel Stanley           2016-08-30  582  		handler = handle_level_irq;
361b79119a4b7f Joel Stanley           2016-08-30  583  		break;
361b79119a4b7f Joel Stanley           2016-08-30  584  	default:
361b79119a4b7f Joel Stanley           2016-08-30  585  		return -EINVAL;
361b79119a4b7f Joel Stanley           2016-08-30  586  	}
361b79119a4b7f Joel Stanley           2016-08-30  587  
61a7904b6ace99 Iwona Winiarska        2021-12-04  588  	raw_spin_lock_irqsave(&gpio->lock, flags);
0e6ca482ec6e28 Billy Tsai             2024-08-30  589  	if (gpio->llops->copro_request)
0e6ca482ec6e28 Billy Tsai             2024-08-30  590  		copro = gpio->llops->copro_request(gpio, offset);
361b79119a4b7f Joel Stanley           2016-08-30  591  
0e6ca482ec6e28 Billy Tsai             2024-08-30  592  	gpio->llops->reg_bits_set(gpio, offset, reg_irq_type0, type0);
0e6ca482ec6e28 Billy Tsai             2024-08-30  593  	gpio->llops->reg_bits_set(gpio, offset, reg_irq_type1, type1);
0e6ca482ec6e28 Billy Tsai             2024-08-30  594  	gpio->llops->reg_bits_set(gpio, offset, reg_irq_type2, type2);
361b79119a4b7f Joel Stanley           2016-08-30  595  
0e6ca482ec6e28 Billy Tsai             2024-08-30 @596  	if (copro && gpio->llops->copro_release)
0e6ca482ec6e28 Billy Tsai             2024-08-30  597  		gpio->llops->copro_release(gpio, offset);
61a7904b6ace99 Iwona Winiarska        2021-12-04  598  	raw_spin_unlock_irqrestore(&gpio->lock, flags);
361b79119a4b7f Joel Stanley           2016-08-30  599  
361b79119a4b7f Joel Stanley           2016-08-30  600  	irq_set_handler_locked(d, handler);
361b79119a4b7f Joel Stanley           2016-08-30  601  
361b79119a4b7f Joel Stanley           2016-08-30  602  	return 0;
361b79119a4b7f Joel Stanley           2016-08-30  603  }
361b79119a4b7f Joel Stanley           2016-08-30  604  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH v2 0/4] Add Aspeed G7 gpio support

[Billy Tsai]

The Aspeed 7th generation SoC features two GPIO controllers: one with 12
GPIO pins and another with 216 GPIO pins. The main difference from the
previous generation is that the control logic has been updated to support
per-pin control, allowing each pin to have its own 32-bit register for
configuring value, direction, interrupt type, and more.
This patch serial also add low-level operations (llops) to abstract the
register access for GPIO registers and the coprocessor request/release in
gpio-aspeed.c making it easier to extend the driver to support different
hardware register layouts.

Changes since v1:
- Merge the gpio-aspeed-g7.c into the gpio-aspeed.c.
- Create the llops in gpio-aspeed.c for flexibility.

Billy Tsai (4):
  dt-bindings: gpio: aspeed,ast2400-gpio: Support ast2700
  gpio: aspeed: Remove the name for bank array
  gpio: aspeed: Create llops to handle hardware access
  gpio: aspeed: Support G7 Aspeed gpio controller

 .../bindings/gpio/aspeed,ast2400-gpio.yaml    |  46 +-
 drivers/gpio/gpio-aspeed.c                    | 442 ++++++++++--------
 2 files changed, 303 insertions(+), 185 deletions(-)

-- 
2.25.1

[PATCH v2 3/4] gpio: aspeed: Create llops to handle hardware access

[Billy Tsai]

Add low-level operations (llops) to abstract the register access for GPIO
registers and the coprocessor request/release. With this abstraction
layer, the driver can separate the hardware and software logic, making it
easier to extend the driver to support different hardware register
layouts.

Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
---
 drivers/gpio/gpio-aspeed.c | 309 +++++++++++++++++--------------------
 1 file changed, 138 insertions(+), 171 deletions(-)

diff --git a/drivers/gpio/gpio-aspeed.c b/drivers/gpio/gpio-aspeed.c
index 24f50a0ea4ab..74c4e80958bf 100644
--- a/drivers/gpio/gpio-aspeed.c
+++ b/drivers/gpio/gpio-aspeed.c
@@ -39,6 +39,7 @@ struct aspeed_bank_props {
 struct aspeed_gpio_config {
 	unsigned int nr_gpios;
 	const struct aspeed_bank_props *props;
+	unsigned int version;
 };
 
 /*
@@ -62,10 +63,13 @@ struct aspeed_gpio {
 
 	u8 *offset_timer;
 	unsigned int timer_users[4];
+	const int *debounce_timers_array;
+	int debounce_timers_num;
 	struct clk *clk;
 
 	u32 *dcache;
 	u8 *cf_copro_bankmap;
+	const struct aspeed_gpio_llops *llops;
 };
 
 struct aspeed_gpio_bank {
@@ -178,6 +182,15 @@ enum aspeed_gpio_reg {
 	reg_cmdsrc1,
 };
 
+struct aspeed_gpio_llops {
+	bool (*copro_request)(struct aspeed_gpio *gpio, unsigned int offset);
+	void (*copro_release)(struct aspeed_gpio *gpio, unsigned int offset);
+	void (*reg_bits_set)(struct aspeed_gpio *gpio, unsigned int offset,
+			     const enum aspeed_gpio_reg reg, u32 val);
+	u32 (*reg_bits_read)(struct aspeed_gpio *gpio, unsigned int offset,
+			     const enum aspeed_gpio_reg reg);
+};
+
 #define GPIO_VAL_VALUE	0x00
 #define GPIO_VAL_DIR	0x04
 
@@ -237,10 +250,6 @@ static inline void __iomem *bank_reg(struct aspeed_gpio *gpio,
 #define GPIO_OFFSET(x)	((x) & 0x1f)
 #define GPIO_BIT(x)	BIT(GPIO_OFFSET(x))
 
-#define _GPIO_SET_DEBOUNCE(t, o, i) ((!!((t) & BIT(i))) << GPIO_OFFSET(o))
-#define GPIO_SET_DEBOUNCE1(t, o) _GPIO_SET_DEBOUNCE(t, o, 1)
-#define GPIO_SET_DEBOUNCE2(t, o) _GPIO_SET_DEBOUNCE(t, o, 0)
-
 static const struct aspeed_gpio_bank *to_bank(unsigned int offset)
 {
 	unsigned int bank = GPIO_BANK(offset);
@@ -296,35 +305,17 @@ static inline bool have_output(struct aspeed_gpio *gpio, unsigned int offset)
 }
 
 static void aspeed_gpio_change_cmd_source(struct aspeed_gpio *gpio,
-					  const struct aspeed_gpio_bank *bank,
-					  int bindex, int cmdsrc)
+					  unsigned int offset,
+					  int cmdsrc)
 {
-	void __iomem *c0 = bank_reg(gpio, bank, reg_cmdsrc0);
-	void __iomem *c1 = bank_reg(gpio, bank, reg_cmdsrc1);
-	u32 bit, reg;
-
 	/*
-	 * Each register controls 4 banks, so take the bottom 2
-	 * bits of the bank index, and use them to select the
-	 * right control bit (0, 8, 16 or 24).
+	 * The command source register is only valid in bits 0, 8, 16, and 24, so we use
+	 * (offset & ~(0x7)) to ensure that reg_bits_set always targets a valid bit.
 	 */
-	bit = BIT((bindex & 3) << 3);
-
 	/* Source 1 first to avoid illegal 11 combination */
-	reg = ioread32(c1);
-	if (cmdsrc & 2)
-		reg |= bit;
-	else
-		reg &= ~bit;
-	iowrite32(reg, c1);
-
+	gpio->llops->reg_bits_set(gpio, offset & ~(0x7), reg_cmdsrc1, !!(cmdsrc & BIT(1)));
 	/* Then Source 0 */
-	reg = ioread32(c0);
-	if (cmdsrc & 1)
-		reg |= bit;
-	else
-		reg &= ~bit;
-	iowrite32(reg, c0);
+	gpio->llops->reg_bits_set(gpio, offset & ~(0x7), reg_cmdsrc0, !!(cmdsrc & BIT(0)));
 }
 
 static bool aspeed_gpio_copro_request(struct aspeed_gpio *gpio,
@@ -343,7 +334,7 @@ static bool aspeed_gpio_copro_request(struct aspeed_gpio *gpio,
 	copro_ops->request_access(copro_data);
 
 	/* Change command source back to ARM */
-	aspeed_gpio_change_cmd_source(gpio, bank, offset >> 3, GPIO_CMDSRC_ARM);
+	aspeed_gpio_change_cmd_source(gpio, offset, GPIO_CMDSRC_ARM);
 
 	/* Update cache */
 	gpio->dcache[GPIO_BANK(offset)] = ioread32(bank_reg(gpio, bank, reg_rdata));
@@ -354,8 +345,6 @@ static bool aspeed_gpio_copro_request(struct aspeed_gpio *gpio,
 static void aspeed_gpio_copro_release(struct aspeed_gpio *gpio,
 				      unsigned int offset)
 {
-	const struct aspeed_gpio_bank *bank = to_bank(offset);
-
 	if (!copro_ops || !gpio->cf_copro_bankmap)
 		return;
 	if (!gpio->cf_copro_bankmap[offset >> 3])
@@ -364,7 +353,7 @@ static void aspeed_gpio_copro_release(struct aspeed_gpio *gpio,
 		return;
 
 	/* Change command source back to ColdFire */
-	aspeed_gpio_change_cmd_source(gpio, bank, offset >> 3,
+	aspeed_gpio_change_cmd_source(gpio, offset,
 				      GPIO_CMDSRC_COLDFIRE);
 
 	/* Restart the coprocessor */
@@ -374,29 +363,24 @@ static void aspeed_gpio_copro_release(struct aspeed_gpio *gpio,
 static int aspeed_gpio_get(struct gpio_chip *gc, unsigned int offset)
 {
 	struct aspeed_gpio *gpio = gpiochip_get_data(gc);
-	const struct aspeed_gpio_bank *bank = to_bank(offset);
 
-	return !!(ioread32(bank_reg(gpio, bank, reg_val)) & GPIO_BIT(offset));
+	return gpio->llops->reg_bits_read(gpio, offset, reg_val);
 }
 
 static void __aspeed_gpio_set(struct gpio_chip *gc, unsigned int offset,
 			      int val)
 {
 	struct aspeed_gpio *gpio = gpiochip_get_data(gc);
-	const struct aspeed_gpio_bank *bank = to_bank(offset);
-	void __iomem *addr;
 	u32 reg;
 
-	addr = bank_reg(gpio, bank, reg_val);
 	reg = gpio->dcache[GPIO_BANK(offset)];
-
 	if (val)
 		reg |= GPIO_BIT(offset);
 	else
 		reg &= ~GPIO_BIT(offset);
 	gpio->dcache[GPIO_BANK(offset)] = reg;
 
-	iowrite32(reg, addr);
+	gpio->llops->reg_bits_set(gpio, offset, reg_val, val);
 }
 
 static void aspeed_gpio_set(struct gpio_chip *gc, unsigned int offset,
@@ -407,36 +391,32 @@ static void aspeed_gpio_set(struct gpio_chip *gc, unsigned int offset,
 	bool copro;
 
 	raw_spin_lock_irqsave(&gpio->lock, flags);
-	copro = aspeed_gpio_copro_request(gpio, offset);
+	if (gpio->llops->copro_request)
+		copro = gpio->llops->copro_request(gpio, offset);
 
 	__aspeed_gpio_set(gc, offset, val);
 
-	if (copro)
-		aspeed_gpio_copro_release(gpio, offset);
+	if (copro && gpio->llops->copro_release)
+		gpio->llops->copro_release(gpio, offset);
 	raw_spin_unlock_irqrestore(&gpio->lock, flags);
 }
 
 static int aspeed_gpio_dir_in(struct gpio_chip *gc, unsigned int offset)
 {
 	struct aspeed_gpio *gpio = gpiochip_get_data(gc);
-	const struct aspeed_gpio_bank *bank = to_bank(offset);
-	void __iomem *addr = bank_reg(gpio, bank, reg_dir);
 	unsigned long flags;
 	bool copro;
-	u32 reg;
 
 	if (!have_input(gpio, offset))
 		return -ENOTSUPP;
 
 	raw_spin_lock_irqsave(&gpio->lock, flags);
 
-	reg = ioread32(addr);
-	reg &= ~GPIO_BIT(offset);
-
-	copro = aspeed_gpio_copro_request(gpio, offset);
-	iowrite32(reg, addr);
-	if (copro)
-		aspeed_gpio_copro_release(gpio, offset);
+	if (gpio->llops->copro_request)
+		copro = gpio->llops->copro_request(gpio, offset);
+	gpio->llops->reg_bits_set(gpio, offset, reg_dir, 0);
+	if (copro && gpio->llops->copro_release)
+		gpio->llops->copro_release(gpio, offset);
 
 	raw_spin_unlock_irqrestore(&gpio->lock, flags);
 
@@ -447,26 +427,21 @@ static int aspeed_gpio_dir_out(struct gpio_chip *gc,
 			       unsigned int offset, int val)
 {
 	struct aspeed_gpio *gpio = gpiochip_get_data(gc);
-	const struct aspeed_gpio_bank *bank = to_bank(offset);
-	void __iomem *addr = bank_reg(gpio, bank, reg_dir);
 	unsigned long flags;
 	bool copro;
-	u32 reg;
 
 	if (!have_output(gpio, offset))
 		return -ENOTSUPP;
 
 	raw_spin_lock_irqsave(&gpio->lock, flags);
 
-	reg = ioread32(addr);
-	reg |= GPIO_BIT(offset);
-
-	copro = aspeed_gpio_copro_request(gpio, offset);
+	if (gpio->llops->copro_request)
+		copro = gpio->llops->copro_request(gpio, offset);
 	__aspeed_gpio_set(gc, offset, val);
-	iowrite32(reg, addr);
+	gpio->llops->reg_bits_set(gpio, offset, reg_dir, 1);
 
-	if (copro)
-		aspeed_gpio_copro_release(gpio, offset);
+	if (copro && gpio->llops->copro_release)
+		gpio->llops->copro_release(gpio, offset);
 	raw_spin_unlock_irqrestore(&gpio->lock, flags);
 
 	return 0;
@@ -475,7 +450,6 @@ static int aspeed_gpio_dir_out(struct gpio_chip *gc,
 static int aspeed_gpio_get_direction(struct gpio_chip *gc, unsigned int offset)
 {
 	struct aspeed_gpio *gpio = gpiochip_get_data(gc);
-	const struct aspeed_gpio_bank *bank = to_bank(offset);
 	unsigned long flags;
 	u32 val;
 
@@ -487,7 +461,7 @@ static int aspeed_gpio_get_direction(struct gpio_chip *gc, unsigned int offset)
 
 	raw_spin_lock_irqsave(&gpio->lock, flags);
 
-	val = ioread32(bank_reg(gpio, bank, reg_dir)) & GPIO_BIT(offset);
+	val = gpio->llops->reg_bits_read(gpio, offset, reg_dir);
 
 	raw_spin_unlock_irqrestore(&gpio->lock, flags);
 
@@ -496,8 +470,7 @@ static int aspeed_gpio_get_direction(struct gpio_chip *gc, unsigned int offset)
 
 static inline int irqd_to_aspeed_gpio_data(struct irq_data *d,
 					   struct aspeed_gpio **gpio,
-					   const struct aspeed_gpio_bank **bank,
-					   u32 *bit, int *offset)
+					   int *offset)
 {
 	struct aspeed_gpio *internal;
 
@@ -510,70 +483,55 @@ static inline int irqd_to_aspeed_gpio_data(struct irq_data *d,
 		return -ENOTSUPP;
 
 	*gpio = internal;
-	*bank = to_bank(*offset);
-	*bit = GPIO_BIT(*offset);
 
 	return 0;
 }
 
 static void aspeed_gpio_irq_ack(struct irq_data *d)
 {
-	const struct aspeed_gpio_bank *bank;
 	struct aspeed_gpio *gpio;
 	unsigned long flags;
-	void __iomem *status_addr;
 	int rc, offset;
 	bool copro;
-	u32 bit;
 
-	rc = irqd_to_aspeed_gpio_data(d, &gpio, &bank, &bit, &offset);
+	rc = irqd_to_aspeed_gpio_data(d, &gpio, &offset);
 	if (rc)
 		return;
 
-	status_addr = bank_reg(gpio, bank, reg_irq_status);
-
 	raw_spin_lock_irqsave(&gpio->lock, flags);
-	copro = aspeed_gpio_copro_request(gpio, offset);
+	if (gpio->llops->copro_request)
+		copro = gpio->llops->copro_request(gpio, offset);
 
-	iowrite32(bit, status_addr);
+	gpio->llops->reg_bits_set(gpio, offset, reg_irq_status, 1);
 
-	if (copro)
-		aspeed_gpio_copro_release(gpio, offset);
+	if (copro && gpio->llops->copro_release)
+		gpio->llops->copro_release(gpio, offset);
 	raw_spin_unlock_irqrestore(&gpio->lock, flags);
 }
 
 static void aspeed_gpio_irq_set_mask(struct irq_data *d, bool set)
 {
-	const struct aspeed_gpio_bank *bank;
 	struct aspeed_gpio *gpio;
 	unsigned long flags;
-	u32 reg, bit;
-	void __iomem *addr;
 	int rc, offset;
 	bool copro;
 
-	rc = irqd_to_aspeed_gpio_data(d, &gpio, &bank, &bit, &offset);
+	rc = irqd_to_aspeed_gpio_data(d, &gpio, &offset);
 	if (rc)
 		return;
 
-	addr = bank_reg(gpio, bank, reg_irq_enable);
-
 	/* Unmasking the IRQ */
 	if (set)
 		gpiochip_enable_irq(&gpio->chip, irqd_to_hwirq(d));
 
 	raw_spin_lock_irqsave(&gpio->lock, flags);
-	copro = aspeed_gpio_copro_request(gpio, offset);
+	if (gpio->llops->copro_request)
+		copro = gpio->llops->copro_request(gpio, offset);
 
-	reg = ioread32(addr);
-	if (set)
-		reg |= bit;
-	else
-		reg &= ~bit;
-	iowrite32(reg, addr);
+	gpio->llops->reg_bits_set(gpio, offset, reg_irq_enable, set);
 
-	if (copro)
-		aspeed_gpio_copro_release(gpio, offset);
+	if (copro && gpio->llops->copro_release)
+		gpio->llops->copro_release(gpio, offset);
 	raw_spin_unlock_irqrestore(&gpio->lock, flags);
 
 	/* Masking the IRQ */
@@ -596,34 +554,31 @@ static int aspeed_gpio_set_type(struct irq_data *d, unsigned int type)
 	u32 type0 = 0;
 	u32 type1 = 0;
 	u32 type2 = 0;
-	u32 bit, reg;
-	const struct aspeed_gpio_bank *bank;
 	irq_flow_handler_t handler;
 	struct aspeed_gpio *gpio;
 	unsigned long flags;
-	void __iomem *addr;
 	int rc, offset;
 	bool copro;
 
-	rc = irqd_to_aspeed_gpio_data(d, &gpio, &bank, &bit, &offset);
+	rc = irqd_to_aspeed_gpio_data(d, &gpio, &offset);
 	if (rc)
 		return -EINVAL;
 
 	switch (type & IRQ_TYPE_SENSE_MASK) {
 	case IRQ_TYPE_EDGE_BOTH:
-		type2 |= bit;
+		type2 = 1;
 		fallthrough;
 	case IRQ_TYPE_EDGE_RISING:
-		type0 |= bit;
+		type0 = 1;
 		fallthrough;
 	case IRQ_TYPE_EDGE_FALLING:
 		handler = handle_edge_irq;
 		break;
 	case IRQ_TYPE_LEVEL_HIGH:
-		type0 |= bit;
+		type0 = 1;
 		fallthrough;
 	case IRQ_TYPE_LEVEL_LOW:
-		type1 |= bit;
+		type1 = 1;
 		handler = handle_level_irq;
 		break;
 	default:
@@ -631,25 +586,15 @@ static int aspeed_gpio_set_type(struct irq_data *d, unsigned int type)
 	}
 
 	raw_spin_lock_irqsave(&gpio->lock, flags);
-	copro = aspeed_gpio_copro_request(gpio, offset);
-
-	addr = bank_reg(gpio, bank, reg_irq_type0);
-	reg = ioread32(addr);
-	reg = (reg & ~bit) | type0;
-	iowrite32(reg, addr);
+	if (gpio->llops->copro_request)
+		copro = gpio->llops->copro_request(gpio, offset);
 
-	addr = bank_reg(gpio, bank, reg_irq_type1);
-	reg = ioread32(addr);
-	reg = (reg & ~bit) | type1;
-	iowrite32(reg, addr);
+	gpio->llops->reg_bits_set(gpio, offset, reg_irq_type0, type0);
+	gpio->llops->reg_bits_set(gpio, offset, reg_irq_type1, type1);
+	gpio->llops->reg_bits_set(gpio, offset, reg_irq_type2, type2);
 
-	addr = bank_reg(gpio, bank, reg_irq_type2);
-	reg = ioread32(addr);
-	reg = (reg & ~bit) | type2;
-	iowrite32(reg, addr);
-
-	if (copro)
-		aspeed_gpio_copro_release(gpio, offset);
+	if (copro && gpio->llops->copro_release)
+		gpio->llops->copro_release(gpio, offset);
 	raw_spin_unlock_irqrestore(&gpio->lock, flags);
 
 	irq_set_handler_locked(d, handler);
@@ -661,7 +606,6 @@ static void aspeed_gpio_irq_handler(struct irq_desc *desc)
 {
 	struct gpio_chip *gc = irq_desc_get_handler_data(desc);
 	struct irq_chip *ic = irq_desc_get_chip(desc);
-	struct aspeed_gpio *data = gpiochip_get_data(gc);
 	unsigned int i, p, banks;
 	unsigned long reg;
 	struct aspeed_gpio *gpio = gpiochip_get_data(gc);
@@ -670,9 +614,7 @@ static void aspeed_gpio_irq_handler(struct irq_desc *desc)
 
 	banks = DIV_ROUND_UP(gpio->chip.ngpio, 32);
 	for (i = 0; i < banks; i++) {
-		const struct aspeed_gpio_bank *bank = &aspeed_gpio_banks[i];
-
-		reg = ioread32(bank_reg(data, bank, reg_irq_status));
+		reg = gpio->llops->reg_bits_read(gpio, i, reg_irq_status);
 
 		for_each_set_bit(p, &reg, 32)
 			generic_handle_domain_irq(gc->irq.domain, i * 32 + p);
@@ -711,26 +653,16 @@ static int aspeed_gpio_reset_tolerance(struct gpio_chip *chip,
 {
 	struct aspeed_gpio *gpio = gpiochip_get_data(chip);
 	unsigned long flags;
-	void __iomem *treg;
 	bool copro;
-	u32 val;
-
-	treg = bank_reg(gpio, to_bank(offset), reg_tolerance);
 
 	raw_spin_lock_irqsave(&gpio->lock, flags);
-	copro = aspeed_gpio_copro_request(gpio, offset);
+	if (gpio->llops->copro_request)
+		copro = gpio->llops->copro_request(gpio, offset);
 
-	val = readl(treg);
+	gpio->llops->reg_bits_set(gpio, offset, reg_tolerance, enable);
 
-	if (enable)
-		val |= GPIO_BIT(offset);
-	else
-		val &= ~GPIO_BIT(offset);
-
-	writel(val, treg);
-
-	if (copro)
-		aspeed_gpio_copro_release(gpio, offset);
+	if (copro && gpio->llops->copro_release)
+		gpio->llops->copro_release(gpio, offset);
 	raw_spin_unlock_irqrestore(&gpio->lock, flags);
 
 	return 0;
@@ -821,21 +753,11 @@ static inline bool timer_allocation_registered(struct aspeed_gpio *gpio,
 static void configure_timer(struct aspeed_gpio *gpio, unsigned int offset,
 		unsigned int timer)
 {
-	const struct aspeed_gpio_bank *bank = to_bank(offset);
-	const u32 mask = GPIO_BIT(offset);
-	void __iomem *addr;
-	u32 val;
-
 	/* Note: Debounce timer isn't under control of the command
 	 * source registers, so no need to sync with the coprocessor
 	 */
-	addr = bank_reg(gpio, bank, reg_debounce_sel1);
-	val = ioread32(addr);
-	iowrite32((val & ~mask) | GPIO_SET_DEBOUNCE1(timer, offset), addr);
-
-	addr = bank_reg(gpio, bank, reg_debounce_sel2);
-	val = ioread32(addr);
-	iowrite32((val & ~mask) | GPIO_SET_DEBOUNCE2(timer, offset), addr);
+	gpio->llops->reg_bits_set(gpio, offset, reg_debounce_sel1, !!(timer & BIT(1)));
+	gpio->llops->reg_bits_set(gpio, offset, reg_debounce_sel2, !!(timer & BIT(0)));
 }
 
 static int enable_debounce(struct gpio_chip *chip, unsigned int offset,
@@ -866,15 +788,15 @@ static int enable_debounce(struct gpio_chip *chip, unsigned int offset,
 	}
 
 	/* Try to find a timer already configured for the debounce period */
-	for (i = 1; i < ARRAY_SIZE(debounce_timers); i++) {
+	for (i = 1; i < gpio->debounce_timers_num; i++) {
 		u32 cycles;
 
-		cycles = ioread32(gpio->base + debounce_timers[i]);
+		cycles = ioread32(gpio->base + gpio->debounce_timers_array[i]);
 		if (requested_cycles == cycles)
 			break;
 	}
 
-	if (i == ARRAY_SIZE(debounce_timers)) {
+	if (i == gpio->debounce_timers_num) {
 		int j;
 
 		/*
@@ -905,7 +827,7 @@ static int enable_debounce(struct gpio_chip *chip, unsigned int offset,
 
 		i = j;
 
-		iowrite32(requested_cycles, gpio->base + debounce_timers[i]);
+		iowrite32(requested_cycles, gpio->base + gpio->debounce_timers_array[i]);
 	}
 
 	if (WARN(i == 0, "Cannot register index of disabled timer\n")) {
@@ -1027,7 +949,7 @@ int aspeed_gpio_copro_grab_gpio(struct gpio_desc *desc,
 
 	/* Switch command source */
 	if (gpio->cf_copro_bankmap[bindex] == 1)
-		aspeed_gpio_change_cmd_source(gpio, bank, bindex,
+		aspeed_gpio_change_cmd_source(gpio, offset,
 					      GPIO_CMDSRC_COLDFIRE);
 
 	if (vreg_offset)
@@ -1051,7 +973,6 @@ int aspeed_gpio_copro_release_gpio(struct gpio_desc *desc)
 	struct gpio_chip *chip = gpiod_to_chip(desc);
 	struct aspeed_gpio *gpio = gpiochip_get_data(chip);
 	int rc = 0, bindex, offset = gpio_chip_hwgpio(desc);
-	const struct aspeed_gpio_bank *bank = to_bank(offset);
 	unsigned long flags;
 
 	if (!gpio->cf_copro_bankmap)
@@ -1072,7 +993,7 @@ int aspeed_gpio_copro_release_gpio(struct gpio_desc *desc)
 
 	/* Switch command source */
 	if (gpio->cf_copro_bankmap[bindex] == 0)
-		aspeed_gpio_change_cmd_source(gpio, bank, bindex,
+		aspeed_gpio_change_cmd_source(gpio, offset,
 					      GPIO_CMDSRC_ARM);
  bail:
 	raw_spin_unlock_irqrestore(&gpio->lock, flags);
@@ -1082,12 +1003,10 @@ EXPORT_SYMBOL_GPL(aspeed_gpio_copro_release_gpio);
 
 static void aspeed_gpio_irq_print_chip(struct irq_data *d, struct seq_file *p)
 {
-	const struct aspeed_gpio_bank *bank;
 	struct aspeed_gpio *gpio;
-	u32 bit;
 	int rc, offset;
 
-	rc = irqd_to_aspeed_gpio_data(d, &gpio, &bank, &bit, &offset);
+	rc = irqd_to_aspeed_gpio_data(d, &gpio, &offset);
 	if (rc)
 		return;
 
@@ -1120,7 +1039,7 @@ static const struct aspeed_bank_props ast2400_bank_props[] = {
 
 static const struct aspeed_gpio_config ast2400_config =
 	/* 220 for simplicity, really 216 with two 4-GPIO holes, four at end */
-	{ .nr_gpios = 220, .props = ast2400_bank_props, };
+	{ .nr_gpios = 220, .props = ast2400_bank_props, .version = 4};
 
 static const struct aspeed_bank_props ast2500_bank_props[] = {
 	/*     input	  output   */
@@ -1132,7 +1051,7 @@ static const struct aspeed_bank_props ast2500_bank_props[] = {
 
 static const struct aspeed_gpio_config ast2500_config =
 	/* 232 for simplicity, actual number is 228 (4-GPIO hole in GPIOAB) */
-	{ .nr_gpios = 232, .props = ast2500_bank_props, };
+	{ .nr_gpios = 232, .props = ast2500_bank_props, .version = 4};
 
 static const struct aspeed_bank_props ast2600_bank_props[] = {
 	/*     input	  output   */
@@ -1148,7 +1067,7 @@ static const struct aspeed_gpio_config ast2600_config =
 	 * We expect ngpio being set in the device tree and this is a fallback
 	 * option.
 	 */
-	{ .nr_gpios = 208, .props = ast2600_bank_props, };
+	{ .nr_gpios = 208, .props = ast2600_bank_props, .version = 4};
 
 static const struct of_device_id aspeed_gpio_of_table[] = {
 	{ .compatible = "aspeed,ast2400-gpio", .data = &ast2400_config, },
@@ -1158,6 +1077,40 @@ static const struct of_device_id aspeed_gpio_of_table[] = {
 };
 MODULE_DEVICE_TABLE(of, aspeed_gpio_of_table);
 
+static void aspeed_g4_reg_bits_set(struct aspeed_gpio *gpio, unsigned int offset,
+				   const enum aspeed_gpio_reg reg, u32 val)
+{
+	const struct aspeed_gpio_bank *bank = to_bank(offset);
+	void __iomem *addr = bank_reg(gpio, bank, reg);
+	u32 temp;
+
+	temp = ioread32(addr);
+	if (val)
+		temp |= GPIO_BIT(offset);
+	else
+		temp &= ~GPIO_BIT(offset);
+
+	iowrite32(temp, addr);
+}
+
+static u32 aspeed_g4_reg_bits_read(struct aspeed_gpio *gpio, unsigned int offset,
+				   const enum aspeed_gpio_reg reg)
+{
+	const struct aspeed_gpio_bank *bank = to_bank(offset);
+	void __iomem *addr = bank_reg(gpio, bank, reg);
+
+	if (reg == reg_rdata)
+		return ioread32(addr);
+	return !!(ioread32(addr) & GPIO_BIT(offset));
+}
+
+struct aspeed_gpio_llops aspeed_g4_llops = {
+	.copro_request = aspeed_gpio_copro_request,
+	.copro_release = aspeed_gpio_copro_release,
+	.reg_bits_set = aspeed_g4_reg_bits_set,
+	.reg_bits_read = aspeed_g4_reg_bits_read,
+};
+
 static int __init aspeed_gpio_probe(struct platform_device *pdev)
 {
 	const struct of_device_id *gpio_id;
@@ -1191,6 +1144,18 @@ static int __init aspeed_gpio_probe(struct platform_device *pdev)
 
 	gpio->config = gpio_id->data;
 
+	if (gpio->config->version == 4) {
+		if (!gpio->llops)
+			gpio->llops = &aspeed_g4_llops;
+
+		if (!gpio->debounce_timers_array) {
+			gpio->debounce_timers_array = debounce_timers;
+			gpio->debounce_timers_num = ARRAY_SIZE(debounce_timers);
+		}
+	} else {
+		return -EOPNOTSUPP;
+	}
+
 	gpio->chip.parent = &pdev->dev;
 	err = of_property_read_u32(pdev->dev.of_node, "ngpios", &ngpio);
 	gpio->chip.ngpio = (u16) ngpio;
@@ -1218,15 +1183,17 @@ static int __init aspeed_gpio_probe(struct platform_device *pdev)
 	 * Populate it with initial values read from the HW and switch
 	 * all command sources to the ARM by default
 	 */
-	for (i = 0; i < banks; i++) {
-		const struct aspeed_gpio_bank *bank = &aspeed_gpio_banks[i];
-		void __iomem *addr = bank_reg(gpio, bank, reg_rdata);
-		gpio->dcache[i] = ioread32(addr);
-		aspeed_gpio_change_cmd_source(gpio, bank, 0, GPIO_CMDSRC_ARM);
-		aspeed_gpio_change_cmd_source(gpio, bank, 1, GPIO_CMDSRC_ARM);
-		aspeed_gpio_change_cmd_source(gpio, bank, 2, GPIO_CMDSRC_ARM);
-		aspeed_gpio_change_cmd_source(gpio, bank, 3, GPIO_CMDSRC_ARM);
-	}
+	if (gpio->config->version == 4)
+		for (i = 0; i < banks; i++) {
+			const struct aspeed_gpio_bank *bank = &aspeed_gpio_banks[i];
+			void __iomem *addr = bank_reg(gpio, bank, reg_rdata);
+
+			gpio->dcache[i] = ioread32(addr);
+			aspeed_gpio_change_cmd_source(gpio, i * 8 + 0, GPIO_CMDSRC_ARM);
+			aspeed_gpio_change_cmd_source(gpio, i * 8 + 8, GPIO_CMDSRC_ARM);
+			aspeed_gpio_change_cmd_source(gpio, i * 8 + 16, GPIO_CMDSRC_ARM);
+			aspeed_gpio_change_cmd_source(gpio, i * 8 + 24, GPIO_CMDSRC_ARM);
+		}
 
 	/* Set up an irqchip */
 	irq = platform_get_irq(pdev, 0);
-- 
2.25.1

[PATCH v2 3/4] gpio: aspeed: Create llops to handle hardware access

[Billy Tsai]

Add low-level operations (llops) to abstract the register access for GPIO
registers and the coprocessor request/release. With this abstraction
layer, the driver can separate the hardware and software logic, making it
easier to extend the driver to support different hardware register
layouts.

Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
---
 drivers/gpio/gpio-aspeed.c | 309 +++++++++++++++++--------------------
 1 file changed, 138 insertions(+), 171 deletions(-)

diff --git a/drivers/gpio/gpio-aspeed.c b/drivers/gpio/gpio-aspeed.c
index 24f50a0ea4ab..74c4e80958bf 100644
--- a/drivers/gpio/gpio-aspeed.c
+++ b/drivers/gpio/gpio-aspeed.c
@@ -39,6 +39,7 @@ struct aspeed_bank_props {
 struct aspeed_gpio_config {
 	unsigned int nr_gpios;
 	const struct aspeed_bank_props *props;
+	unsigned int version;
 };
 
 /*
@@ -62,10 +63,13 @@ struct aspeed_gpio {
 
 	u8 *offset_timer;
 	unsigned int timer_users[4];
+	const int *debounce_timers_array;
+	int debounce_timers_num;
 	struct clk *clk;
 
 	u32 *dcache;
 	u8 *cf_copro_bankmap;
+	const struct aspeed_gpio_llops *llops;
 };
 
 struct aspeed_gpio_bank {
@@ -178,6 +182,15 @@ enum aspeed_gpio_reg {
 	reg_cmdsrc1,
 };
 
+struct aspeed_gpio_llops {
+	bool (*copro_request)(struct aspeed_gpio *gpio, unsigned int offset);
+	void (*copro_release)(struct aspeed_gpio *gpio, unsigned int offset);
+	void (*reg_bits_set)(struct aspeed_gpio *gpio, unsigned int offset,
+			     const enum aspeed_gpio_reg reg, u32 val);
+	u32 (*reg_bits_read)(struct aspeed_gpio *gpio, unsigned int offset,
+			     const enum aspeed_gpio_reg reg);
+};
+
 #define GPIO_VAL_VALUE	0x00
 #define GPIO_VAL_DIR	0x04
 
@@ -237,10 +250,6 @@ static inline void __iomem *bank_reg(struct aspeed_gpio *gpio,
 #define GPIO_OFFSET(x)	((x) & 0x1f)
 #define GPIO_BIT(x)	BIT(GPIO_OFFSET(x))
 
-#define _GPIO_SET_DEBOUNCE(t, o, i) ((!!((t) & BIT(i))) << GPIO_OFFSET(o))
-#define GPIO_SET_DEBOUNCE1(t, o) _GPIO_SET_DEBOUNCE(t, o, 1)
-#define GPIO_SET_DEBOUNCE2(t, o) _GPIO_SET_DEBOUNCE(t, o, 0)
-
 static const struct aspeed_gpio_bank *to_bank(unsigned int offset)
 {
 	unsigned int bank = GPIO_BANK(offset);
@@ -296,35 +305,17 @@ static inline bool have_output(struct aspeed_gpio *gpio, unsigned int offset)
 }
 
 static void aspeed_gpio_change_cmd_source(struct aspeed_gpio *gpio,
-					  const struct aspeed_gpio_bank *bank,
-					  int bindex, int cmdsrc)
+					  unsigned int offset,
+					  int cmdsrc)
 {
-	void __iomem *c0 = bank_reg(gpio, bank, reg_cmdsrc0);
-	void __iomem *c1 = bank_reg(gpio, bank, reg_cmdsrc1);
-	u32 bit, reg;
-
 	/*
-	 * Each register controls 4 banks, so take the bottom 2
-	 * bits of the bank index, and use them to select the
-	 * right control bit (0, 8, 16 or 24).
+	 * The command source register is only valid in bits 0, 8, 16, and 24, so we use
+	 * (offset & ~(0x7)) to ensure that reg_bits_set always targets a valid bit.
 	 */
-	bit = BIT((bindex & 3) << 3);
-
 	/* Source 1 first to avoid illegal 11 combination */
-	reg = ioread32(c1);
-	if (cmdsrc & 2)
-		reg |= bit;
-	else
-		reg &= ~bit;
-	iowrite32(reg, c1);
-
+	gpio->llops->reg_bits_set(gpio, offset & ~(0x7), reg_cmdsrc1, !!(cmdsrc & BIT(1)));
 	/* Then Source 0 */
-	reg = ioread32(c0);
-	if (cmdsrc & 1)
-		reg |= bit;
-	else
-		reg &= ~bit;
-	iowrite32(reg, c0);
+	gpio->llops->reg_bits_set(gpio, offset & ~(0x7), reg_cmdsrc0, !!(cmdsrc & BIT(0)));
 }
 
 static bool aspeed_gpio_copro_request(struct aspeed_gpio *gpio,
@@ -343,7 +334,7 @@ static bool aspeed_gpio_copro_request(struct aspeed_gpio *gpio,
 	copro_ops->request_access(copro_data);
 
 	/* Change command source back to ARM */
-	aspeed_gpio_change_cmd_source(gpio, bank, offset >> 3, GPIO_CMDSRC_ARM);
+	aspeed_gpio_change_cmd_source(gpio, offset, GPIO_CMDSRC_ARM);
 
 	/* Update cache */
 	gpio->dcache[GPIO_BANK(offset)] = ioread32(bank_reg(gpio, bank, reg_rdata));
@@ -354,8 +345,6 @@ static bool aspeed_gpio_copro_request(struct aspeed_gpio *gpio,
 static void aspeed_gpio_copro_release(struct aspeed_gpio *gpio,
 				      unsigned int offset)
 {
-	const struct aspeed_gpio_bank *bank = to_bank(offset);
-
 	if (!copro_ops || !gpio->cf_copro_bankmap)
 		return;
 	if (!gpio->cf_copro_bankmap[offset >> 3])
@@ -364,7 +353,7 @@ static void aspeed_gpio_copro_release(struct aspeed_gpio *gpio,
 		return;
 
 	/* Change command source back to ColdFire */
-	aspeed_gpio_change_cmd_source(gpio, bank, offset >> 3,
+	aspeed_gpio_change_cmd_source(gpio, offset,
 				      GPIO_CMDSRC_COLDFIRE);
 
 	/* Restart the coprocessor */
@@ -374,29 +363,24 @@ static void aspeed_gpio_copro_release(struct aspeed_gpio *gpio,
 static int aspeed_gpio_get(struct gpio_chip *gc, unsigned int offset)
 {
 	struct aspeed_gpio *gpio = gpiochip_get_data(gc);
-	const struct aspeed_gpio_bank *bank = to_bank(offset);
 
-	return !!(ioread32(bank_reg(gpio, bank, reg_val)) & GPIO_BIT(offset));
+	return gpio->llops->reg_bits_read(gpio, offset, reg_val);
 }
 
 static void __aspeed_gpio_set(struct gpio_chip *gc, unsigned int offset,
 			      int val)
 {
 	struct aspeed_gpio *gpio = gpiochip_get_data(gc);
-	const struct aspeed_gpio_bank *bank = to_bank(offset);
-	void __iomem *addr;
 	u32 reg;
 
-	addr = bank_reg(gpio, bank, reg_val);
 	reg = gpio->dcache[GPIO_BANK(offset)];
-
 	if (val)
 		reg |= GPIO_BIT(offset);
 	else
 		reg &= ~GPIO_BIT(offset);
 	gpio->dcache[GPIO_BANK(offset)] = reg;
 
-	iowrite32(reg, addr);
+	gpio->llops->reg_bits_set(gpio, offset, reg_val, val);
 }
 
 static void aspeed_gpio_set(struct gpio_chip *gc, unsigned int offset,
@@ -407,36 +391,32 @@ static void aspeed_gpio_set(struct gpio_chip *gc, unsigned int offset,
 	bool copro;
 
 	raw_spin_lock_irqsave(&gpio->lock, flags);
-	copro = aspeed_gpio_copro_request(gpio, offset);
+	if (gpio->llops->copro_request)
+		copro = gpio->llops->copro_request(gpio, offset);
 
 	__aspeed_gpio_set(gc, offset, val);
 
-	if (copro)
-		aspeed_gpio_copro_release(gpio, offset);
+	if (copro && gpio->llops->copro_release)
+		gpio->llops->copro_release(gpio, offset);
 	raw_spin_unlock_irqrestore(&gpio->lock, flags);
 }
 
 static int aspeed_gpio_dir_in(struct gpio_chip *gc, unsigned int offset)
 {
 	struct aspeed_gpio *gpio = gpiochip_get_data(gc);
-	const struct aspeed_gpio_bank *bank = to_bank(offset);
-	void __iomem *addr = bank_reg(gpio, bank, reg_dir);
 	unsigned long flags;
 	bool copro;
-	u32 reg;
 
 	if (!have_input(gpio, offset))
 		return -ENOTSUPP;
 
 	raw_spin_lock_irqsave(&gpio->lock, flags);
 
-	reg = ioread32(addr);
-	reg &= ~GPIO_BIT(offset);
-
-	copro = aspeed_gpio_copro_request(gpio, offset);
-	iowrite32(reg, addr);
-	if (copro)
-		aspeed_gpio_copro_release(gpio, offset);
+	if (gpio->llops->copro_request)
+		copro = gpio->llops->copro_request(gpio, offset);
+	gpio->llops->reg_bits_set(gpio, offset, reg_dir, 0);
+	if (copro && gpio->llops->copro_release)
+		gpio->llops->copro_release(gpio, offset);
 
 	raw_spin_unlock_irqrestore(&gpio->lock, flags);
 
@@ -447,26 +427,21 @@ static int aspeed_gpio_dir_out(struct gpio_chip *gc,
 			       unsigned int offset, int val)
 {
 	struct aspeed_gpio *gpio = gpiochip_get_data(gc);
-	const struct aspeed_gpio_bank *bank = to_bank(offset);
-	void __iomem *addr = bank_reg(gpio, bank, reg_dir);
 	unsigned long flags;
 	bool copro;
-	u32 reg;
 
 	if (!have_output(gpio, offset))
 		return -ENOTSUPP;
 
 	raw_spin_lock_irqsave(&gpio->lock, flags);
 
-	reg = ioread32(addr);
-	reg |= GPIO_BIT(offset);
-
-	copro = aspeed_gpio_copro_request(gpio, offset);
+	if (gpio->llops->copro_request)
+		copro = gpio->llops->copro_request(gpio, offset);
 	__aspeed_gpio_set(gc, offset, val);
-	iowrite32(reg, addr);
+	gpio->llops->reg_bits_set(gpio, offset, reg_dir, 1);
 
-	if (copro)
-		aspeed_gpio_copro_release(gpio, offset);
+	if (copro && gpio->llops->copro_release)
+		gpio->llops->copro_release(gpio, offset);
 	raw_spin_unlock_irqrestore(&gpio->lock, flags);
 
 	return 0;
@@ -475,7 +450,6 @@ static int aspeed_gpio_dir_out(struct gpio_chip *gc,
 static int aspeed_gpio_get_direction(struct gpio_chip *gc, unsigned int offset)
 {
 	struct aspeed_gpio *gpio = gpiochip_get_data(gc);
-	const struct aspeed_gpio_bank *bank = to_bank(offset);
 	unsigned long flags;
 	u32 val;
 
@@ -487,7 +461,7 @@ static int aspeed_gpio_get_direction(struct gpio_chip *gc, unsigned int offset)
 
 	raw_spin_lock_irqsave(&gpio->lock, flags);
 
-	val = ioread32(bank_reg(gpio, bank, reg_dir)) & GPIO_BIT(offset);
+	val = gpio->llops->reg_bits_read(gpio, offset, reg_dir);
 
 	raw_spin_unlock_irqrestore(&gpio->lock, flags);
 
@@ -496,8 +470,7 @@ static int aspeed_gpio_get_direction(struct gpio_chip *gc, unsigned int offset)
 
 static inline int irqd_to_aspeed_gpio_data(struct irq_data *d,
 					   struct aspeed_gpio **gpio,
-					   const struct aspeed_gpio_bank **bank,
-					   u32 *bit, int *offset)
+					   int *offset)
 {
 	struct aspeed_gpio *internal;
 
@@ -510,70 +483,55 @@ static inline int irqd_to_aspeed_gpio_data(struct irq_data *d,
 		return -ENOTSUPP;
 
 	*gpio = internal;
-	*bank = to_bank(*offset);
-	*bit = GPIO_BIT(*offset);
 
 	return 0;
 }
 
 static void aspeed_gpio_irq_ack(struct irq_data *d)
 {
-	const struct aspeed_gpio_bank *bank;
 	struct aspeed_gpio *gpio;
 	unsigned long flags;
-	void __iomem *status_addr;
 	int rc, offset;
 	bool copro;
-	u32 bit;
 
-	rc = irqd_to_aspeed_gpio_data(d, &gpio, &bank, &bit, &offset);
+	rc = irqd_to_aspeed_gpio_data(d, &gpio, &offset);
 	if (rc)
 		return;
 
-	status_addr = bank_reg(gpio, bank, reg_irq_status);
-
 	raw_spin_lock_irqsave(&gpio->lock, flags);
-	copro = aspeed_gpio_copro_request(gpio, offset);
+	if (gpio->llops->copro_request)
+		copro = gpio->llops->copro_request(gpio, offset);
 
-	iowrite32(bit, status_addr);
+	gpio->llops->reg_bits_set(gpio, offset, reg_irq_status, 1);
 
-	if (copro)
-		aspeed_gpio_copro_release(gpio, offset);
+	if (copro && gpio->llops->copro_release)
+		gpio->llops->copro_release(gpio, offset);
 	raw_spin_unlock_irqrestore(&gpio->lock, flags);
 }
 
 static void aspeed_gpio_irq_set_mask(struct irq_data *d, bool set)
 {
-	const struct aspeed_gpio_bank *bank;
 	struct aspeed_gpio *gpio;
 	unsigned long flags;
-	u32 reg, bit;
-	void __iomem *addr;
 	int rc, offset;
 	bool copro;
 
-	rc = irqd_to_aspeed_gpio_data(d, &gpio, &bank, &bit, &offset);
+	rc = irqd_to_aspeed_gpio_data(d, &gpio, &offset);
 	if (rc)
 		return;
 
-	addr = bank_reg(gpio, bank, reg_irq_enable);
-
 	/* Unmasking the IRQ */
 	if (set)
 		gpiochip_enable_irq(&gpio->chip, irqd_to_hwirq(d));
 
 	raw_spin_lock_irqsave(&gpio->lock, flags);
-	copro = aspeed_gpio_copro_request(gpio, offset);
+	if (gpio->llops->copro_request)
+		copro = gpio->llops->copro_request(gpio, offset);
 
-	reg = ioread32(addr);
-	if (set)
-		reg |= bit;
-	else
-		reg &= ~bit;
-	iowrite32(reg, addr);
+	gpio->llops->reg_bits_set(gpio, offset, reg_irq_enable, set);
 
-	if (copro)
-		aspeed_gpio_copro_release(gpio, offset);
+	if (copro && gpio->llops->copro_release)
+		gpio->llops->copro_release(gpio, offset);
 	raw_spin_unlock_irqrestore(&gpio->lock, flags);
 
 	/* Masking the IRQ */
@@ -596,34 +554,31 @@ static int aspeed_gpio_set_type(struct irq_data *d, unsigned int type)
 	u32 type0 = 0;
 	u32 type1 = 0;
 	u32 type2 = 0;
-	u32 bit, reg;
-	const struct aspeed_gpio_bank *bank;
 	irq_flow_handler_t handler;
 	struct aspeed_gpio *gpio;
 	unsigned long flags;
-	void __iomem *addr;
 	int rc, offset;
 	bool copro;
 
-	rc = irqd_to_aspeed_gpio_data(d, &gpio, &bank, &bit, &offset);
+	rc = irqd_to_aspeed_gpio_data(d, &gpio, &offset);
 	if (rc)
 		return -EINVAL;
 
 	switch (type & IRQ_TYPE_SENSE_MASK) {
 	case IRQ_TYPE_EDGE_BOTH:
-		type2 |= bit;
+		type2 = 1;
 		fallthrough;
 	case IRQ_TYPE_EDGE_RISING:
-		type0 |= bit;
+		type0 = 1;
 		fallthrough;
 	case IRQ_TYPE_EDGE_FALLING:
 		handler = handle_edge_irq;
 		break;
 	case IRQ_TYPE_LEVEL_HIGH:
-		type0 |= bit;
+		type0 = 1;
 		fallthrough;
 	case IRQ_TYPE_LEVEL_LOW:
-		type1 |= bit;
+		type1 = 1;
 		handler = handle_level_irq;
 		break;
 	default:
@@ -631,25 +586,15 @@ static int aspeed_gpio_set_type(struct irq_data *d, unsigned int type)
 	}
 
 	raw_spin_lock_irqsave(&gpio->lock, flags);
-	copro = aspeed_gpio_copro_request(gpio, offset);
-
-	addr = bank_reg(gpio, bank, reg_irq_type0);
-	reg = ioread32(addr);
-	reg = (reg & ~bit) | type0;
-	iowrite32(reg, addr);
+	if (gpio->llops->copro_request)
+		copro = gpio->llops->copro_request(gpio, offset);
 
-	addr = bank_reg(gpio, bank, reg_irq_type1);
-	reg = ioread32(addr);
-	reg = (reg & ~bit) | type1;
-	iowrite32(reg, addr);
+	gpio->llops->reg_bits_set(gpio, offset, reg_irq_type0, type0);
+	gpio->llops->reg_bits_set(gpio, offset, reg_irq_type1, type1);
+	gpio->llops->reg_bits_set(gpio, offset, reg_irq_type2, type2);
 
-	addr = bank_reg(gpio, bank, reg_irq_type2);
-	reg = ioread32(addr);
-	reg = (reg & ~bit) | type2;
-	iowrite32(reg, addr);
-
-	if (copro)
-		aspeed_gpio_copro_release(gpio, offset);
+	if (copro && gpio->llops->copro_release)
+		gpio->llops->copro_release(gpio, offset);
 	raw_spin_unlock_irqrestore(&gpio->lock, flags);
 
 	irq_set_handler_locked(d, handler);
@@ -661,7 +606,6 @@ static void aspeed_gpio_irq_handler(struct irq_desc *desc)
 {
 	struct gpio_chip *gc = irq_desc_get_handler_data(desc);
 	struct irq_chip *ic = irq_desc_get_chip(desc);
-	struct aspeed_gpio *data = gpiochip_get_data(gc);
 	unsigned int i, p, banks;
 	unsigned long reg;
 	struct aspeed_gpio *gpio = gpiochip_get_data(gc);
@@ -670,9 +614,7 @@ static void aspeed_gpio_irq_handler(struct irq_desc *desc)
 
 	banks = DIV_ROUND_UP(gpio->chip.ngpio, 32);
 	for (i = 0; i < banks; i++) {
-		const struct aspeed_gpio_bank *bank = &aspeed_gpio_banks[i];
-
-		reg = ioread32(bank_reg(data, bank, reg_irq_status));
+		reg = gpio->llops->reg_bits_read(gpio, i, reg_irq_status);
 
 		for_each_set_bit(p, &reg, 32)
 			generic_handle_domain_irq(gc->irq.domain, i * 32 + p);
@@ -711,26 +653,16 @@ static int aspeed_gpio_reset_tolerance(struct gpio_chip *chip,
 {
 	struct aspeed_gpio *gpio = gpiochip_get_data(chip);
 	unsigned long flags;
-	void __iomem *treg;
 	bool copro;
-	u32 val;
-
-	treg = bank_reg(gpio, to_bank(offset), reg_tolerance);
 
 	raw_spin_lock_irqsave(&gpio->lock, flags);
-	copro = aspeed_gpio_copro_request(gpio, offset);
+	if (gpio->llops->copro_request)
+		copro = gpio->llops->copro_request(gpio, offset);
 
-	val = readl(treg);
+	gpio->llops->reg_bits_set(gpio, offset, reg_tolerance, enable);
 
-	if (enable)
-		val |= GPIO_BIT(offset);
-	else
-		val &= ~GPIO_BIT(offset);
-
-	writel(val, treg);
-
-	if (copro)
-		aspeed_gpio_copro_release(gpio, offset);
+	if (copro && gpio->llops->copro_release)
+		gpio->llops->copro_release(gpio, offset);
 	raw_spin_unlock_irqrestore(&gpio->lock, flags);
 
 	return 0;
@@ -821,21 +753,11 @@ static inline bool timer_allocation_registered(struct aspeed_gpio *gpio,
 static void configure_timer(struct aspeed_gpio *gpio, unsigned int offset,
 		unsigned int timer)
 {
-	const struct aspeed_gpio_bank *bank = to_bank(offset);
-	const u32 mask = GPIO_BIT(offset);
-	void __iomem *addr;
-	u32 val;
-
 	/* Note: Debounce timer isn't under control of the command
 	 * source registers, so no need to sync with the coprocessor
 	 */
-	addr = bank_reg(gpio, bank, reg_debounce_sel1);
-	val = ioread32(addr);
-	iowrite32((val & ~mask) | GPIO_SET_DEBOUNCE1(timer, offset), addr);
-
-	addr = bank_reg(gpio, bank, reg_debounce_sel2);
-	val = ioread32(addr);
-	iowrite32((val & ~mask) | GPIO_SET_DEBOUNCE2(timer, offset), addr);
+	gpio->llops->reg_bits_set(gpio, offset, reg_debounce_sel1, !!(timer & BIT(1)));
+	gpio->llops->reg_bits_set(gpio, offset, reg_debounce_sel2, !!(timer & BIT(0)));
 }
 
 static int enable_debounce(struct gpio_chip *chip, unsigned int offset,
@@ -866,15 +788,15 @@ static int enable_debounce(struct gpio_chip *chip, unsigned int offset,
 	}
 
 	/* Try to find a timer already configured for the debounce period */
-	for (i = 1; i < ARRAY_SIZE(debounce_timers); i++) {
+	for (i = 1; i < gpio->debounce_timers_num; i++) {
 		u32 cycles;
 
-		cycles = ioread32(gpio->base + debounce_timers[i]);
+		cycles = ioread32(gpio->base + gpio->debounce_timers_array[i]);
 		if (requested_cycles == cycles)
 			break;
 	}
 
-	if (i == ARRAY_SIZE(debounce_timers)) {
+	if (i == gpio->debounce_timers_num) {
 		int j;
 
 		/*
@@ -905,7 +827,7 @@ static int enable_debounce(struct gpio_chip *chip, unsigned int offset,
 
 		i = j;
 
-		iowrite32(requested_cycles, gpio->base + debounce_timers[i]);
+		iowrite32(requested_cycles, gpio->base + gpio->debounce_timers_array[i]);
 	}
 
 	if (WARN(i == 0, "Cannot register index of disabled timer\n")) {
@@ -1027,7 +949,7 @@ int aspeed_gpio_copro_grab_gpio(struct gpio_desc *desc,
 
 	/* Switch command source */
 	if (gpio->cf_copro_bankmap[bindex] == 1)
-		aspeed_gpio_change_cmd_source(gpio, bank, bindex,
+		aspeed_gpio_change_cmd_source(gpio, offset,
 					      GPIO_CMDSRC_COLDFIRE);
 
 	if (vreg_offset)
@@ -1051,7 +973,6 @@ int aspeed_gpio_copro_release_gpio(struct gpio_desc *desc)
 	struct gpio_chip *chip = gpiod_to_chip(desc);
 	struct aspeed_gpio *gpio = gpiochip_get_data(chip);
 	int rc = 0, bindex, offset = gpio_chip_hwgpio(desc);
-	const struct aspeed_gpio_bank *bank = to_bank(offset);
 	unsigned long flags;
 
 	if (!gpio->cf_copro_bankmap)
@@ -1072,7 +993,7 @@ int aspeed_gpio_copro_release_gpio(struct gpio_desc *desc)
 
 	/* Switch command source */
 	if (gpio->cf_copro_bankmap[bindex] == 0)
-		aspeed_gpio_change_cmd_source(gpio, bank, bindex,
+		aspeed_gpio_change_cmd_source(gpio, offset,
 					      GPIO_CMDSRC_ARM);
  bail:
 	raw_spin_unlock_irqrestore(&gpio->lock, flags);
@@ -1082,12 +1003,10 @@ EXPORT_SYMBOL_GPL(aspeed_gpio_copro_release_gpio);
 
 static void aspeed_gpio_irq_print_chip(struct irq_data *d, struct seq_file *p)
 {
-	const struct aspeed_gpio_bank *bank;
 	struct aspeed_gpio *gpio;
-	u32 bit;
 	int rc, offset;
 
-	rc = irqd_to_aspeed_gpio_data(d, &gpio, &bank, &bit, &offset);
+	rc = irqd_to_aspeed_gpio_data(d, &gpio, &offset);
 	if (rc)
 		return;
 
@@ -1120,7 +1039,7 @@ static const struct aspeed_bank_props ast2400_bank_props[] = {
 
 static const struct aspeed_gpio_config ast2400_config =
 	/* 220 for simplicity, really 216 with two 4-GPIO holes, four at end */
-	{ .nr_gpios = 220, .props = ast2400_bank_props, };
+	{ .nr_gpios = 220, .props = ast2400_bank_props, .version = 4};
 
 static const struct aspeed_bank_props ast2500_bank_props[] = {
 	/*     input	  output   */
@@ -1132,7 +1051,7 @@ static const struct aspeed_bank_props ast2500_bank_props[] = {
 
 static const struct aspeed_gpio_config ast2500_config =
 	/* 232 for simplicity, actual number is 228 (4-GPIO hole in GPIOAB) */
-	{ .nr_gpios = 232, .props = ast2500_bank_props, };
+	{ .nr_gpios = 232, .props = ast2500_bank_props, .version = 4};
 
 static const struct aspeed_bank_props ast2600_bank_props[] = {
 	/*     input	  output   */
@@ -1148,7 +1067,7 @@ static const struct aspeed_gpio_config ast2600_config =
 	 * We expect ngpio being set in the device tree and this is a fallback
 	 * option.
 	 */
-	{ .nr_gpios = 208, .props = ast2600_bank_props, };
+	{ .nr_gpios = 208, .props = ast2600_bank_props, .version = 4};
 
 static const struct of_device_id aspeed_gpio_of_table[] = {
 	{ .compatible = "aspeed,ast2400-gpio", .data = &ast2400_config, },
@@ -1158,6 +1077,40 @@ static const struct of_device_id aspeed_gpio_of_table[] = {
 };
 MODULE_DEVICE_TABLE(of, aspeed_gpio_of_table);
 
+static void aspeed_g4_reg_bits_set(struct aspeed_gpio *gpio, unsigned int offset,
+				   const enum aspeed_gpio_reg reg, u32 val)
+{
+	const struct aspeed_gpio_bank *bank = to_bank(offset);
+	void __iomem *addr = bank_reg(gpio, bank, reg);
+	u32 temp;
+
+	temp = ioread32(addr);
+	if (val)
+		temp |= GPIO_BIT(offset);
+	else
+		temp &= ~GPIO_BIT(offset);
+
+	iowrite32(temp, addr);
+}
+
+static u32 aspeed_g4_reg_bits_read(struct aspeed_gpio *gpio, unsigned int offset,
+				   const enum aspeed_gpio_reg reg)
+{
+	const struct aspeed_gpio_bank *bank = to_bank(offset);
+	void __iomem *addr = bank_reg(gpio, bank, reg);
+
+	if (reg == reg_rdata)
+		return ioread32(addr);
+	return !!(ioread32(addr) & GPIO_BIT(offset));
+}
+
+struct aspeed_gpio_llops aspeed_g4_llops = {
+	.copro_request = aspeed_gpio_copro_request,
+	.copro_release = aspeed_gpio_copro_release,
+	.reg_bits_set = aspeed_g4_reg_bits_set,
+	.reg_bits_read = aspeed_g4_reg_bits_read,
+};
+
 static int __init aspeed_gpio_probe(struct platform_device *pdev)
 {
 	const struct of_device_id *gpio_id;
@@ -1191,6 +1144,18 @@ static int __init aspeed_gpio_probe(struct platform_device *pdev)
 
 	gpio->config = gpio_id->data;
 
+	if (gpio->config->version == 4) {
+		if (!gpio->llops)
+			gpio->llops = &aspeed_g4_llops;
+
+		if (!gpio->debounce_timers_array) {
+			gpio->debounce_timers_array = debounce_timers;
+			gpio->debounce_timers_num = ARRAY_SIZE(debounce_timers);
+		}
+	} else {
+		return -EOPNOTSUPP;
+	}
+
 	gpio->chip.parent = &pdev->dev;
 	err = of_property_read_u32(pdev->dev.of_node, "ngpios", &ngpio);
 	gpio->chip.ngpio = (u16) ngpio;
@@ -1218,15 +1183,17 @@ static int __init aspeed_gpio_probe(struct platform_device *pdev)
 	 * Populate it with initial values read from the HW and switch
 	 * all command sources to the ARM by default
 	 */
-	for (i = 0; i < banks; i++) {
-		const struct aspeed_gpio_bank *bank = &aspeed_gpio_banks[i];
-		void __iomem *addr = bank_reg(gpio, bank, reg_rdata);
-		gpio->dcache[i] = ioread32(addr);
-		aspeed_gpio_change_cmd_source(gpio, bank, 0, GPIO_CMDSRC_ARM);
-		aspeed_gpio_change_cmd_source(gpio, bank, 1, GPIO_CMDSRC_ARM);
-		aspeed_gpio_change_cmd_source(gpio, bank, 2, GPIO_CMDSRC_ARM);
-		aspeed_gpio_change_cmd_source(gpio, bank, 3, GPIO_CMDSRC_ARM);
-	}
+	if (gpio->config->version == 4)
+		for (i = 0; i < banks; i++) {
+			const struct aspeed_gpio_bank *bank = &aspeed_gpio_banks[i];
+			void __iomem *addr = bank_reg(gpio, bank, reg_rdata);
+
+			gpio->dcache[i] = ioread32(addr);
+			aspeed_gpio_change_cmd_source(gpio, i * 8 + 0, GPIO_CMDSRC_ARM);
+			aspeed_gpio_change_cmd_source(gpio, i * 8 + 8, GPIO_CMDSRC_ARM);
+			aspeed_gpio_change_cmd_source(gpio, i * 8 + 16, GPIO_CMDSRC_ARM);
+			aspeed_gpio_change_cmd_source(gpio, i * 8 + 24, GPIO_CMDSRC_ARM);
+		}
 
 	/* Set up an irqchip */
 	irq = platform_get_irq(pdev, 0);
-- 
2.25.1

[PATCH v2 3/4] gpio: aspeed: Create llops to handle hardware access

[kernel test robot]

Hi Billy,

kernel test robot noticed the following build warnings:

[auto build test WARNING on brgl/gpio/for-next]
[also build test WARNING on linus/master v6.11-rc5 next-20240830]
[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#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Billy-Tsai/dt-bindings-gpio-aspeed-ast2400-gpio-Support-ast2700/20240830-114325
base:   https://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git gpio/for-next
patch link:    https://lore.kernel.org/r/20240830034047.2251482-4-billy_tsai%40aspeedtech.com
patch subject: [PATCH v2 3/4] gpio: aspeed: Create llops to handle hardware access
config: i386-buildonly-randconfig-004-20240830 (https://download.01.org/0day-ci/archive/20240830/202408302344.bCpCF6bu-lkp at intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240830/202408302344.bCpCF6bu-lkp at intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408302344.bCpCF6bu-lkp at intel.com/

All warnings (new ones prefixed by >>):

>> drivers/gpio/gpio-aspeed.c:394:6: warning: variable 'copro' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
     394 |         if (gpio->llops->copro_request)
         |             ^~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpio/gpio-aspeed.c:399:6: note: uninitialized use occurs here
     399 |         if (copro && gpio->llops->copro_release)
         |             ^~~~~
   drivers/gpio/gpio-aspeed.c:394:2: note: remove the 'if' if its condition is always true
     394 |         if (gpio->llops->copro_request)
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     395 |                 copro = gpio->llops->copro_request(gpio, offset);
   drivers/gpio/gpio-aspeed.c:391:12: note: initialize the variable 'copro' to silence this warning
     391 |         bool copro;
         |                   ^
         |                    = 0
   drivers/gpio/gpio-aspeed.c:415:6: warning: variable 'copro' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
     415 |         if (gpio->llops->copro_request)
         |             ^~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpio/gpio-aspeed.c:418:6: note: uninitialized use occurs here
     418 |         if (copro && gpio->llops->copro_release)
         |             ^~~~~
   drivers/gpio/gpio-aspeed.c:415:2: note: remove the 'if' if its condition is always true
     415 |         if (gpio->llops->copro_request)
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     416 |                 copro = gpio->llops->copro_request(gpio, offset);
   drivers/gpio/gpio-aspeed.c:408:12: note: initialize the variable 'copro' to silence this warning
     408 |         bool copro;
         |                   ^
         |                    = 0
   drivers/gpio/gpio-aspeed.c:438:6: warning: variable 'copro' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
     438 |         if (gpio->llops->copro_request)
         |             ^~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpio/gpio-aspeed.c:443:6: note: uninitialized use occurs here
     443 |         if (copro && gpio->llops->copro_release)
         |             ^~~~~
   drivers/gpio/gpio-aspeed.c:438:2: note: remove the 'if' if its condition is always true
     438 |         if (gpio->llops->copro_request)
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     439 |                 copro = gpio->llops->copro_request(gpio, offset);
   drivers/gpio/gpio-aspeed.c:431:12: note: initialize the variable 'copro' to silence this warning
     431 |         bool copro;
         |                   ^
         |                    = 0
   drivers/gpio/gpio-aspeed.c:502:6: warning: variable 'copro' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
     502 |         if (gpio->llops->copro_request)
         |             ^~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpio/gpio-aspeed.c:507:6: note: uninitialized use occurs here
     507 |         if (copro && gpio->llops->copro_release)
         |             ^~~~~
   drivers/gpio/gpio-aspeed.c:502:2: note: remove the 'if' if its condition is always true
     502 |         if (gpio->llops->copro_request)
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     503 |                 copro = gpio->llops->copro_request(gpio, offset);
   drivers/gpio/gpio-aspeed.c:495:12: note: initialize the variable 'copro' to silence this warning
     495 |         bool copro;
         |                   ^
         |                    = 0
   drivers/gpio/gpio-aspeed.c:528:6: warning: variable 'copro' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
     528 |         if (gpio->llops->copro_request)
         |             ^~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpio/gpio-aspeed.c:533:6: note: uninitialized use occurs here
     533 |         if (copro && gpio->llops->copro_release)
         |             ^~~~~
   drivers/gpio/gpio-aspeed.c:528:2: note: remove the 'if' if its condition is always true
     528 |         if (gpio->llops->copro_request)
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     529 |                 copro = gpio->llops->copro_request(gpio, offset);
   drivers/gpio/gpio-aspeed.c:517:12: note: initialize the variable 'copro' to silence this warning
     517 |         bool copro;
         |                   ^
         |                    = 0
   drivers/gpio/gpio-aspeed.c:589:6: warning: variable 'copro' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
     589 |         if (gpio->llops->copro_request)
         |             ^~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpio/gpio-aspeed.c:596:6: note: uninitialized use occurs here
     596 |         if (copro && gpio->llops->copro_release)
         |             ^~~~~
   drivers/gpio/gpio-aspeed.c:589:2: note: remove the 'if' if its condition is always true
     589 |         if (gpio->llops->copro_request)
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     590 |                 copro = gpio->llops->copro_request(gpio, offset);
   drivers/gpio/gpio-aspeed.c:561:12: note: initialize the variable 'copro' to silence this warning
     561 |         bool copro;
         |                   ^
         |                    = 0
   drivers/gpio/gpio-aspeed.c:659:6: warning: variable 'copro' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
     659 |         if (gpio->llops->copro_request)
         |             ^~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpio/gpio-aspeed.c:664:6: note: uninitialized use occurs here
     664 |         if (copro && gpio->llops->copro_release)
         |             ^~~~~
   drivers/gpio/gpio-aspeed.c:659:2: note: remove the 'if' if its condition is always true
     659 |         if (gpio->llops->copro_request)
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     660 |                 copro = gpio->llops->copro_request(gpio, offset);
   drivers/gpio/gpio-aspeed.c:656:12: note: initialize the variable 'copro' to silence this warning
     656 |         bool copro;
         |                   ^
         |                    = 0
   7 warnings generated.


vim +394 drivers/gpio/gpio-aspeed.c

   385	
   386	static void aspeed_gpio_set(struct gpio_chip *gc, unsigned int offset,
   387				    int val)
   388	{
   389		struct aspeed_gpio *gpio = gpiochip_get_data(gc);
   390		unsigned long flags;
   391		bool copro;
   392	
   393		raw_spin_lock_irqsave(&gpio->lock, flags);
 > 394		if (gpio->llops->copro_request)
   395			copro = gpio->llops->copro_request(gpio, offset);
   396	
   397		__aspeed_gpio_set(gc, offset, val);
   398	
   399		if (copro && gpio->llops->copro_release)
   400			gpio->llops->copro_release(gpio, offset);
   401		raw_spin_unlock_irqrestore(&gpio->lock, flags);
   402	}
   403	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 3/4] gpio: aspeed: Create llops to handle hardware access

[kernel test robot]

Hi Billy,

kernel test robot noticed the following build warnings:

[auto build test WARNING on brgl/gpio/for-next]
[also build test WARNING on linus/master v6.11-rc5 next-20240830]
[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#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Billy-Tsai/dt-bindings-gpio-aspeed-ast2400-gpio-Support-ast2700/20240830-114325
base:   https://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git gpio/for-next
patch link:    https://lore.kernel.org/r/20240830034047.2251482-4-billy_tsai%40aspeedtech.com
patch subject: [PATCH v2 3/4] gpio: aspeed: Create llops to handle hardware access
config: i386-buildonly-randconfig-004-20240830 (https://download.01.org/0day-ci/archive/20240830/202408302344.bCpCF6bu-lkp@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240830/202408302344.bCpCF6bu-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408302344.bCpCF6bu-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/gpio/gpio-aspeed.c:394:6: warning: variable 'copro' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
     394 |         if (gpio->llops->copro_request)
         |             ^~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpio/gpio-aspeed.c:399:6: note: uninitialized use occurs here
     399 |         if (copro && gpio->llops->copro_release)
         |             ^~~~~
   drivers/gpio/gpio-aspeed.c:394:2: note: remove the 'if' if its condition is always true
     394 |         if (gpio->llops->copro_request)
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     395 |                 copro = gpio->llops->copro_request(gpio, offset);
   drivers/gpio/gpio-aspeed.c:391:12: note: initialize the variable 'copro' to silence this warning
     391 |         bool copro;
         |                   ^
         |                    = 0
   drivers/gpio/gpio-aspeed.c:415:6: warning: variable 'copro' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
     415 |         if (gpio->llops->copro_request)
         |             ^~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpio/gpio-aspeed.c:418:6: note: uninitialized use occurs here
     418 |         if (copro && gpio->llops->copro_release)
         |             ^~~~~
   drivers/gpio/gpio-aspeed.c:415:2: note: remove the 'if' if its condition is always true
     415 |         if (gpio->llops->copro_request)
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     416 |                 copro = gpio->llops->copro_request(gpio, offset);
   drivers/gpio/gpio-aspeed.c:408:12: note: initialize the variable 'copro' to silence this warning
     408 |         bool copro;
         |                   ^
         |                    = 0
   drivers/gpio/gpio-aspeed.c:438:6: warning: variable 'copro' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
     438 |         if (gpio->llops->copro_request)
         |             ^~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpio/gpio-aspeed.c:443:6: note: uninitialized use occurs here
     443 |         if (copro && gpio->llops->copro_release)
         |             ^~~~~
   drivers/gpio/gpio-aspeed.c:438:2: note: remove the 'if' if its condition is always true
     438 |         if (gpio->llops->copro_request)
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     439 |                 copro = gpio->llops->copro_request(gpio, offset);
   drivers/gpio/gpio-aspeed.c:431:12: note: initialize the variable 'copro' to silence this warning
     431 |         bool copro;
         |                   ^
         |                    = 0
   drivers/gpio/gpio-aspeed.c:502:6: warning: variable 'copro' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
     502 |         if (gpio->llops->copro_request)
         |             ^~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpio/gpio-aspeed.c:507:6: note: uninitialized use occurs here
     507 |         if (copro && gpio->llops->copro_release)
         |             ^~~~~
   drivers/gpio/gpio-aspeed.c:502:2: note: remove the 'if' if its condition is always true
     502 |         if (gpio->llops->copro_request)
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     503 |                 copro = gpio->llops->copro_request(gpio, offset);
   drivers/gpio/gpio-aspeed.c:495:12: note: initialize the variable 'copro' to silence this warning
     495 |         bool copro;
         |                   ^
         |                    = 0
   drivers/gpio/gpio-aspeed.c:528:6: warning: variable 'copro' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
     528 |         if (gpio->llops->copro_request)
         |             ^~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpio/gpio-aspeed.c:533:6: note: uninitialized use occurs here
     533 |         if (copro && gpio->llops->copro_release)
         |             ^~~~~
   drivers/gpio/gpio-aspeed.c:528:2: note: remove the 'if' if its condition is always true
     528 |         if (gpio->llops->copro_request)
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     529 |                 copro = gpio->llops->copro_request(gpio, offset);
   drivers/gpio/gpio-aspeed.c:517:12: note: initialize the variable 'copro' to silence this warning
     517 |         bool copro;
         |                   ^
         |                    = 0
   drivers/gpio/gpio-aspeed.c:589:6: warning: variable 'copro' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
     589 |         if (gpio->llops->copro_request)
         |             ^~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpio/gpio-aspeed.c:596:6: note: uninitialized use occurs here
     596 |         if (copro && gpio->llops->copro_release)
         |             ^~~~~
   drivers/gpio/gpio-aspeed.c:589:2: note: remove the 'if' if its condition is always true
     589 |         if (gpio->llops->copro_request)
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     590 |                 copro = gpio->llops->copro_request(gpio, offset);
   drivers/gpio/gpio-aspeed.c:561:12: note: initialize the variable 'copro' to silence this warning
     561 |         bool copro;
         |                   ^
         |                    = 0
   drivers/gpio/gpio-aspeed.c:659:6: warning: variable 'copro' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
     659 |         if (gpio->llops->copro_request)
         |             ^~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpio/gpio-aspeed.c:664:6: note: uninitialized use occurs here
     664 |         if (copro && gpio->llops->copro_release)
         |             ^~~~~
   drivers/gpio/gpio-aspeed.c:659:2: note: remove the 'if' if its condition is always true
     659 |         if (gpio->llops->copro_request)
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     660 |                 copro = gpio->llops->copro_request(gpio, offset);
   drivers/gpio/gpio-aspeed.c:656:12: note: initialize the variable 'copro' to silence this warning
     656 |         bool copro;
         |                   ^
         |                    = 0
   7 warnings generated.


vim +394 drivers/gpio/gpio-aspeed.c

   385	
   386	static void aspeed_gpio_set(struct gpio_chip *gc, unsigned int offset,
   387				    int val)
   388	{
   389		struct aspeed_gpio *gpio = gpiochip_get_data(gc);
   390		unsigned long flags;
   391		bool copro;
   392	
   393		raw_spin_lock_irqsave(&gpio->lock, flags);
 > 394		if (gpio->llops->copro_request)
   395			copro = gpio->llops->copro_request(gpio, offset);
   396	
   397		__aspeed_gpio_set(gc, offset, val);
   398	
   399		if (copro && gpio->llops->copro_release)
   400			gpio->llops->copro_release(gpio, offset);
   401		raw_spin_unlock_irqrestore(&gpio->lock, flags);
   402	}
   403	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH v2 3/4] gpio: aspeed: Create llops to handle hardware access

[kernel test robot]

Hi Billy,

kernel test robot noticed the following build warnings:

[auto build test WARNING on brgl/gpio/for-next]
[also build test WARNING on linus/master v6.11-rc5 next-20240830]
[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#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Billy-Tsai/dt-bindings-gpio-aspeed-ast2400-gpio-Support-ast2700/20240830-114325
base:   https://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git gpio/for-next
patch link:    https://lore.kernel.org/r/20240830034047.2251482-4-billy_tsai%40aspeedtech.com
patch subject: [PATCH v2 3/4] gpio: aspeed: Create llops to handle hardware access
config: openrisc-randconfig-r131-20240830 (https://download.01.org/0day-ci/archive/20240831/202408310953.Z0c7NIuz-lkp at intel.com/config)
compiler: or1k-linux-gcc (GCC) 14.1.0
reproduce: (https://download.01.org/0day-ci/archive/20240831/202408310953.Z0c7NIuz-lkp at intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408310953.Z0c7NIuz-lkp at intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/gpio/gpio-aspeed.c:1107:26: sparse: sparse: symbol 'aspeed_g4_llops' was not declared. Should it be static?

vim +/aspeed_g4_llops +1107 drivers/gpio/gpio-aspeed.c

  1106	
> 1107	struct aspeed_gpio_llops aspeed_g4_llops = {
  1108		.copro_request = aspeed_gpio_copro_request,
  1109		.copro_release = aspeed_gpio_copro_release,
  1110		.reg_bits_set = aspeed_g4_reg_bits_set,
  1111		.reg_bits_read = aspeed_g4_reg_bits_read,
  1112	};
  1113	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 3/4] gpio: aspeed: Create llops to handle hardware access

[kernel test robot]

Hi Billy,

kernel test robot noticed the following build warnings:

[auto build test WARNING on brgl/gpio/for-next]
[also build test WARNING on linus/master v6.11-rc5 next-20240830]
[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#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Billy-Tsai/dt-bindings-gpio-aspeed-ast2400-gpio-Support-ast2700/20240830-114325
base:   https://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git gpio/for-next
patch link:    https://lore.kernel.org/r/20240830034047.2251482-4-billy_tsai%40aspeedtech.com
patch subject: [PATCH v2 3/4] gpio: aspeed: Create llops to handle hardware access
config: openrisc-randconfig-r131-20240830 (https://download.01.org/0day-ci/archive/20240831/202408310953.Z0c7NIuz-lkp@intel.com/config)
compiler: or1k-linux-gcc (GCC) 14.1.0
reproduce: (https://download.01.org/0day-ci/archive/20240831/202408310953.Z0c7NIuz-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408310953.Z0c7NIuz-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/gpio/gpio-aspeed.c:1107:26: sparse: sparse: symbol 'aspeed_g4_llops' was not declared. Should it be static?

vim +/aspeed_g4_llops +1107 drivers/gpio/gpio-aspeed.c

  1106	
> 1107	struct aspeed_gpio_llops aspeed_g4_llops = {
  1108		.copro_request = aspeed_gpio_copro_request,
  1109		.copro_release = aspeed_gpio_copro_release,
  1110		.reg_bits_set = aspeed_g4_reg_bits_set,
  1111		.reg_bits_read = aspeed_g4_reg_bits_read,
  1112	};
  1113	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH v2 3/4] gpio: aspeed: Create llops to handle hardware access

[Dan Carpenter]

Hi Billy,

kernel test robot noticed the following build warnings:

https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Billy-Tsai/dt-bindings-gpio-aspeed-ast2400-gpio-Support-ast2700/20240830-114325
base:   https://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git gpio/for-next
patch link:    https://lore.kernel.org/r/20240830034047.2251482-4-billy_tsai%40aspeedtech.com
patch subject: [PATCH v2 3/4] gpio: aspeed: Create llops to handle hardware access
config: parisc-randconfig-r071-20240831 (https://download.01.org/0day-ci/archive/20240831/202408312313.HTx2vwvy-lkp at intel.com/config)
compiler: hppa-linux-gcc (GCC) 14.1.0

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202408312313.HTx2vwvy-lkp at intel.com/

smatch warnings:
drivers/gpio/gpio-aspeed.c:399 aspeed_gpio_set() error: uninitialized symbol 'copro'.
drivers/gpio/gpio-aspeed.c:418 aspeed_gpio_dir_in() error: uninitialized symbol 'copro'.
drivers/gpio/gpio-aspeed.c:443 aspeed_gpio_dir_out() error: uninitialized symbol 'copro'.
drivers/gpio/gpio-aspeed.c:507 aspeed_gpio_irq_ack() error: uninitialized symbol 'copro'.
drivers/gpio/gpio-aspeed.c:533 aspeed_gpio_irq_set_mask() error: uninitialized symbol 'copro'.
drivers/gpio/gpio-aspeed.c:596 aspeed_gpio_set_type() error: uninitialized symbol 'copro'.
drivers/gpio/gpio-aspeed.c:664 aspeed_gpio_reset_tolerance() error: uninitialized symbol 'copro'.

vim +/copro +399 drivers/gpio/gpio-aspeed.c

361b79119a4b7f Joel Stanley           2016-08-30  386  static void aspeed_gpio_set(struct gpio_chip *gc, unsigned int offset,
361b79119a4b7f Joel Stanley           2016-08-30  387  			    int val)
361b79119a4b7f Joel Stanley           2016-08-30  388  {
361b79119a4b7f Joel Stanley           2016-08-30  389  	struct aspeed_gpio *gpio = gpiochip_get_data(gc);
361b79119a4b7f Joel Stanley           2016-08-30  390  	unsigned long flags;
a7ca13826e478f Benjamin Herrenschmidt 2018-06-29  391  	bool copro;
361b79119a4b7f Joel Stanley           2016-08-30  392  
61a7904b6ace99 Iwona Winiarska        2021-12-04  393  	raw_spin_lock_irqsave(&gpio->lock, flags);
0e6ca482ec6e28 Billy Tsai             2024-08-30  394  	if (gpio->llops->copro_request)
0e6ca482ec6e28 Billy Tsai             2024-08-30  395  		copro = gpio->llops->copro_request(gpio, offset);

Uninitialized on else  path

361b79119a4b7f Joel Stanley           2016-08-30  396  
361b79119a4b7f Joel Stanley           2016-08-30  397  	__aspeed_gpio_set(gc, offset, val);
361b79119a4b7f Joel Stanley           2016-08-30  398  
0e6ca482ec6e28 Billy Tsai             2024-08-30 @399  	if (copro && gpio->llops->copro_release)
                                                            ^^^^^

0e6ca482ec6e28 Billy Tsai             2024-08-30  400  		gpio->llops->copro_release(gpio, offset);
61a7904b6ace99 Iwona Winiarska        2021-12-04  401  	raw_spin_unlock_irqrestore(&gpio->lock, flags);
361b79119a4b7f Joel Stanley           2016-08-30  402  }
361b79119a4b7f Joel Stanley           2016-08-30  403  
361b79119a4b7f Joel Stanley           2016-08-30  404  static int aspeed_gpio_dir_in(struct gpio_chip *gc, unsigned int offset)
361b79119a4b7f Joel Stanley           2016-08-30  405  {
361b79119a4b7f Joel Stanley           2016-08-30  406  	struct aspeed_gpio *gpio = gpiochip_get_data(gc);
361b79119a4b7f Joel Stanley           2016-08-30  407  	unsigned long flags;
a7ca13826e478f Benjamin Herrenschmidt 2018-06-29  408  	bool copro;
361b79119a4b7f Joel Stanley           2016-08-30  409  
1736f75d35e474 Andrew Jeffery         2017-01-24  410  	if (!have_input(gpio, offset))
1736f75d35e474 Andrew Jeffery         2017-01-24  411  		return -ENOTSUPP;
1736f75d35e474 Andrew Jeffery         2017-01-24  412  
61a7904b6ace99 Iwona Winiarska        2021-12-04  413  	raw_spin_lock_irqsave(&gpio->lock, flags);
361b79119a4b7f Joel Stanley           2016-08-30  414  
0e6ca482ec6e28 Billy Tsai             2024-08-30  415  	if (gpio->llops->copro_request)
0e6ca482ec6e28 Billy Tsai             2024-08-30  416  		copro = gpio->llops->copro_request(gpio, offset);
0e6ca482ec6e28 Billy Tsai             2024-08-30  417  	gpio->llops->reg_bits_set(gpio, offset, reg_dir, 0);
0e6ca482ec6e28 Billy Tsai             2024-08-30 @418  	if (copro && gpio->llops->copro_release)
0e6ca482ec6e28 Billy Tsai             2024-08-30  419  		gpio->llops->copro_release(gpio, offset);
361b79119a4b7f Joel Stanley           2016-08-30  420  
61a7904b6ace99 Iwona Winiarska        2021-12-04  421  	raw_spin_unlock_irqrestore(&gpio->lock, flags);
361b79119a4b7f Joel Stanley           2016-08-30  422  
361b79119a4b7f Joel Stanley           2016-08-30  423  	return 0;
361b79119a4b7f Joel Stanley           2016-08-30  424  }
361b79119a4b7f Joel Stanley           2016-08-30  425  
361b79119a4b7f Joel Stanley           2016-08-30  426  static int aspeed_gpio_dir_out(struct gpio_chip *gc,
361b79119a4b7f Joel Stanley           2016-08-30  427  			       unsigned int offset, int val)
361b79119a4b7f Joel Stanley           2016-08-30  428  {
361b79119a4b7f Joel Stanley           2016-08-30  429  	struct aspeed_gpio *gpio = gpiochip_get_data(gc);
361b79119a4b7f Joel Stanley           2016-08-30  430  	unsigned long flags;
a7ca13826e478f Benjamin Herrenschmidt 2018-06-29  431  	bool copro;
361b79119a4b7f Joel Stanley           2016-08-30  432  
1736f75d35e474 Andrew Jeffery         2017-01-24  433  	if (!have_output(gpio, offset))
1736f75d35e474 Andrew Jeffery         2017-01-24  434  		return -ENOTSUPP;
1736f75d35e474 Andrew Jeffery         2017-01-24  435  
61a7904b6ace99 Iwona Winiarska        2021-12-04  436  	raw_spin_lock_irqsave(&gpio->lock, flags);
361b79119a4b7f Joel Stanley           2016-08-30  437  
0e6ca482ec6e28 Billy Tsai             2024-08-30  438  	if (gpio->llops->copro_request)
0e6ca482ec6e28 Billy Tsai             2024-08-30  439  		copro = gpio->llops->copro_request(gpio, offset);
af7949284910a1 Benjamin Herrenschmidt 2018-05-17  440  	__aspeed_gpio_set(gc, offset, val);
0e6ca482ec6e28 Billy Tsai             2024-08-30  441  	gpio->llops->reg_bits_set(gpio, offset, reg_dir, 1);
361b79119a4b7f Joel Stanley           2016-08-30  442  
0e6ca482ec6e28 Billy Tsai             2024-08-30 @443  	if (copro && gpio->llops->copro_release)
0e6ca482ec6e28 Billy Tsai             2024-08-30  444  		gpio->llops->copro_release(gpio, offset);
61a7904b6ace99 Iwona Winiarska        2021-12-04  445  	raw_spin_unlock_irqrestore(&gpio->lock, flags);
361b79119a4b7f Joel Stanley           2016-08-30  446  
361b79119a4b7f Joel Stanley           2016-08-30  447  	return 0;
361b79119a4b7f Joel Stanley           2016-08-30  448  }
361b79119a4b7f Joel Stanley           2016-08-30  449  
361b79119a4b7f Joel Stanley           2016-08-30  450  static int aspeed_gpio_get_direction(struct gpio_chip *gc, unsigned int offset)
361b79119a4b7f Joel Stanley           2016-08-30  451  {
361b79119a4b7f Joel Stanley           2016-08-30  452  	struct aspeed_gpio *gpio = gpiochip_get_data(gc);
361b79119a4b7f Joel Stanley           2016-08-30  453  	unsigned long flags;
361b79119a4b7f Joel Stanley           2016-08-30  454  	u32 val;
361b79119a4b7f Joel Stanley           2016-08-30  455  
1736f75d35e474 Andrew Jeffery         2017-01-24  456  	if (!have_input(gpio, offset))
e42615ec233b30 Matti Vaittinen        2019-11-06  457  		return GPIO_LINE_DIRECTION_OUT;
1736f75d35e474 Andrew Jeffery         2017-01-24  458  
1736f75d35e474 Andrew Jeffery         2017-01-24  459  	if (!have_output(gpio, offset))
e42615ec233b30 Matti Vaittinen        2019-11-06  460  		return GPIO_LINE_DIRECTION_IN;
1736f75d35e474 Andrew Jeffery         2017-01-24  461  
61a7904b6ace99 Iwona Winiarska        2021-12-04  462  	raw_spin_lock_irqsave(&gpio->lock, flags);
361b79119a4b7f Joel Stanley           2016-08-30  463  
0e6ca482ec6e28 Billy Tsai             2024-08-30  464  	val = gpio->llops->reg_bits_read(gpio, offset, reg_dir);
361b79119a4b7f Joel Stanley           2016-08-30  465  
61a7904b6ace99 Iwona Winiarska        2021-12-04  466  	raw_spin_unlock_irqrestore(&gpio->lock, flags);
361b79119a4b7f Joel Stanley           2016-08-30  467  
e42615ec233b30 Matti Vaittinen        2019-11-06  468  	return val ? GPIO_LINE_DIRECTION_OUT : GPIO_LINE_DIRECTION_IN;
361b79119a4b7f Joel Stanley           2016-08-30  469  }
361b79119a4b7f Joel Stanley           2016-08-30  470  
361b79119a4b7f Joel Stanley           2016-08-30  471  static inline int irqd_to_aspeed_gpio_data(struct irq_data *d,
361b79119a4b7f Joel Stanley           2016-08-30  472  					   struct aspeed_gpio **gpio,
0e6ca482ec6e28 Billy Tsai             2024-08-30  473  					   int *offset)
361b79119a4b7f Joel Stanley           2016-08-30  474  {
1736f75d35e474 Andrew Jeffery         2017-01-24  475  	struct aspeed_gpio *internal;
361b79119a4b7f Joel Stanley           2016-08-30  476  
a7ca13826e478f Benjamin Herrenschmidt 2018-06-29  477  	*offset = irqd_to_hwirq(d);
361b79119a4b7f Joel Stanley           2016-08-30  478  
1736f75d35e474 Andrew Jeffery         2017-01-24  479  	internal = irq_data_get_irq_chip_data(d);
1736f75d35e474 Andrew Jeffery         2017-01-24  480  
1736f75d35e474 Andrew Jeffery         2017-01-24  481  	/* This might be a bit of a questionable place to check */
a7ca13826e478f Benjamin Herrenschmidt 2018-06-29  482  	if (!have_irq(internal, *offset))
1736f75d35e474 Andrew Jeffery         2017-01-24  483  		return -ENOTSUPP;
1736f75d35e474 Andrew Jeffery         2017-01-24  484  
1736f75d35e474 Andrew Jeffery         2017-01-24  485  	*gpio = internal;
361b79119a4b7f Joel Stanley           2016-08-30  486  
361b79119a4b7f Joel Stanley           2016-08-30  487  	return 0;
361b79119a4b7f Joel Stanley           2016-08-30  488  }
361b79119a4b7f Joel Stanley           2016-08-30  489  
361b79119a4b7f Joel Stanley           2016-08-30  490  static void aspeed_gpio_irq_ack(struct irq_data *d)
361b79119a4b7f Joel Stanley           2016-08-30  491  {
361b79119a4b7f Joel Stanley           2016-08-30  492  	struct aspeed_gpio *gpio;
361b79119a4b7f Joel Stanley           2016-08-30  493  	unsigned long flags;
a7ca13826e478f Benjamin Herrenschmidt 2018-06-29  494  	int rc, offset;
a7ca13826e478f Benjamin Herrenschmidt 2018-06-29  495  	bool copro;
361b79119a4b7f Joel Stanley           2016-08-30  496  
0e6ca482ec6e28 Billy Tsai             2024-08-30  497  	rc = irqd_to_aspeed_gpio_data(d, &gpio, &offset);
361b79119a4b7f Joel Stanley           2016-08-30  498  	if (rc)
361b79119a4b7f Joel Stanley           2016-08-30  499  		return;
361b79119a4b7f Joel Stanley           2016-08-30  500  
61a7904b6ace99 Iwona Winiarska        2021-12-04  501  	raw_spin_lock_irqsave(&gpio->lock, flags);
0e6ca482ec6e28 Billy Tsai             2024-08-30  502  	if (gpio->llops->copro_request)
0e6ca482ec6e28 Billy Tsai             2024-08-30  503  		copro = gpio->llops->copro_request(gpio, offset);
a7ca13826e478f Benjamin Herrenschmidt 2018-06-29  504  
0e6ca482ec6e28 Billy Tsai             2024-08-30  505  	gpio->llops->reg_bits_set(gpio, offset, reg_irq_status, 1);
a7ca13826e478f Benjamin Herrenschmidt 2018-06-29  506  
0e6ca482ec6e28 Billy Tsai             2024-08-30 @507  	if (copro && gpio->llops->copro_release)
0e6ca482ec6e28 Billy Tsai             2024-08-30  508  		gpio->llops->copro_release(gpio, offset);
61a7904b6ace99 Iwona Winiarska        2021-12-04  509  	raw_spin_unlock_irqrestore(&gpio->lock, flags);
361b79119a4b7f Joel Stanley           2016-08-30  510  }
361b79119a4b7f Joel Stanley           2016-08-30  511  
361b79119a4b7f Joel Stanley           2016-08-30  512  static void aspeed_gpio_irq_set_mask(struct irq_data *d, bool set)
361b79119a4b7f Joel Stanley           2016-08-30  513  {
361b79119a4b7f Joel Stanley           2016-08-30  514  	struct aspeed_gpio *gpio;
361b79119a4b7f Joel Stanley           2016-08-30  515  	unsigned long flags;
a7ca13826e478f Benjamin Herrenschmidt 2018-06-29  516  	int rc, offset;
a7ca13826e478f Benjamin Herrenschmidt 2018-06-29  517  	bool copro;
361b79119a4b7f Joel Stanley           2016-08-30  518  
0e6ca482ec6e28 Billy Tsai             2024-08-30  519  	rc = irqd_to_aspeed_gpio_data(d, &gpio, &offset);
361b79119a4b7f Joel Stanley           2016-08-30  520  	if (rc)
361b79119a4b7f Joel Stanley           2016-08-30  521  		return;
361b79119a4b7f Joel Stanley           2016-08-30  522  
061df08f063a97 Linus Walleij          2023-03-09  523  	/* Unmasking the IRQ */
061df08f063a97 Linus Walleij          2023-03-09  524  	if (set)
061df08f063a97 Linus Walleij          2023-03-09  525  		gpiochip_enable_irq(&gpio->chip, irqd_to_hwirq(d));
061df08f063a97 Linus Walleij          2023-03-09  526  
61a7904b6ace99 Iwona Winiarska        2021-12-04  527  	raw_spin_lock_irqsave(&gpio->lock, flags);
0e6ca482ec6e28 Billy Tsai             2024-08-30  528  	if (gpio->llops->copro_request)
0e6ca482ec6e28 Billy Tsai             2024-08-30  529  		copro = gpio->llops->copro_request(gpio, offset);
361b79119a4b7f Joel Stanley           2016-08-30  530  
0e6ca482ec6e28 Billy Tsai             2024-08-30  531  	gpio->llops->reg_bits_set(gpio, offset, reg_irq_enable, set);
361b79119a4b7f Joel Stanley           2016-08-30  532  
0e6ca482ec6e28 Billy Tsai             2024-08-30 @533  	if (copro && gpio->llops->copro_release)
0e6ca482ec6e28 Billy Tsai             2024-08-30  534  		gpio->llops->copro_release(gpio, offset);
61a7904b6ace99 Iwona Winiarska        2021-12-04  535  	raw_spin_unlock_irqrestore(&gpio->lock, flags);
061df08f063a97 Linus Walleij          2023-03-09  536  
061df08f063a97 Linus Walleij          2023-03-09  537  	/* Masking the IRQ */
061df08f063a97 Linus Walleij          2023-03-09  538  	if (!set)
061df08f063a97 Linus Walleij          2023-03-09  539  		gpiochip_disable_irq(&gpio->chip, irqd_to_hwirq(d));
361b79119a4b7f Joel Stanley           2016-08-30  540  }
361b79119a4b7f Joel Stanley           2016-08-30  541  
361b79119a4b7f Joel Stanley           2016-08-30  542  static void aspeed_gpio_irq_mask(struct irq_data *d)
361b79119a4b7f Joel Stanley           2016-08-30  543  {
361b79119a4b7f Joel Stanley           2016-08-30  544  	aspeed_gpio_irq_set_mask(d, false);
361b79119a4b7f Joel Stanley           2016-08-30  545  }
361b79119a4b7f Joel Stanley           2016-08-30  546  
361b79119a4b7f Joel Stanley           2016-08-30  547  static void aspeed_gpio_irq_unmask(struct irq_data *d)
361b79119a4b7f Joel Stanley           2016-08-30  548  {
361b79119a4b7f Joel Stanley           2016-08-30  549  	aspeed_gpio_irq_set_mask(d, true);
361b79119a4b7f Joel Stanley           2016-08-30  550  }
361b79119a4b7f Joel Stanley           2016-08-30  551  
361b79119a4b7f Joel Stanley           2016-08-30  552  static int aspeed_gpio_set_type(struct irq_data *d, unsigned int type)
361b79119a4b7f Joel Stanley           2016-08-30  553  {
361b79119a4b7f Joel Stanley           2016-08-30  554  	u32 type0 = 0;
361b79119a4b7f Joel Stanley           2016-08-30  555  	u32 type1 = 0;
361b79119a4b7f Joel Stanley           2016-08-30  556  	u32 type2 = 0;
361b79119a4b7f Joel Stanley           2016-08-30  557  	irq_flow_handler_t handler;
361b79119a4b7f Joel Stanley           2016-08-30  558  	struct aspeed_gpio *gpio;
361b79119a4b7f Joel Stanley           2016-08-30  559  	unsigned long flags;
a7ca13826e478f Benjamin Herrenschmidt 2018-06-29  560  	int rc, offset;
a7ca13826e478f Benjamin Herrenschmidt 2018-06-29  561  	bool copro;
361b79119a4b7f Joel Stanley           2016-08-30  562  
0e6ca482ec6e28 Billy Tsai             2024-08-30  563  	rc = irqd_to_aspeed_gpio_data(d, &gpio, &offset);
361b79119a4b7f Joel Stanley           2016-08-30  564  	if (rc)
361b79119a4b7f Joel Stanley           2016-08-30  565  		return -EINVAL;
361b79119a4b7f Joel Stanley           2016-08-30  566  
361b79119a4b7f Joel Stanley           2016-08-30  567  	switch (type & IRQ_TYPE_SENSE_MASK) {
361b79119a4b7f Joel Stanley           2016-08-30  568  	case IRQ_TYPE_EDGE_BOTH:
0e6ca482ec6e28 Billy Tsai             2024-08-30  569  		type2 = 1;
df561f6688fef7 Gustavo A. R. Silva    2020-08-23  570  		fallthrough;
361b79119a4b7f Joel Stanley           2016-08-30  571  	case IRQ_TYPE_EDGE_RISING:
0e6ca482ec6e28 Billy Tsai             2024-08-30  572  		type0 = 1;
df561f6688fef7 Gustavo A. R. Silva    2020-08-23  573  		fallthrough;
361b79119a4b7f Joel Stanley           2016-08-30  574  	case IRQ_TYPE_EDGE_FALLING:
361b79119a4b7f Joel Stanley           2016-08-30  575  		handler = handle_edge_irq;
361b79119a4b7f Joel Stanley           2016-08-30  576  		break;
361b79119a4b7f Joel Stanley           2016-08-30  577  	case IRQ_TYPE_LEVEL_HIGH:
0e6ca482ec6e28 Billy Tsai             2024-08-30  578  		type0 = 1;
df561f6688fef7 Gustavo A. R. Silva    2020-08-23  579  		fallthrough;
361b79119a4b7f Joel Stanley           2016-08-30  580  	case IRQ_TYPE_LEVEL_LOW:
0e6ca482ec6e28 Billy Tsai             2024-08-30  581  		type1 = 1;
361b79119a4b7f Joel Stanley           2016-08-30  582  		handler = handle_level_irq;
361b79119a4b7f Joel Stanley           2016-08-30  583  		break;
361b79119a4b7f Joel Stanley           2016-08-30  584  	default:
361b79119a4b7f Joel Stanley           2016-08-30  585  		return -EINVAL;
361b79119a4b7f Joel Stanley           2016-08-30  586  	}
361b79119a4b7f Joel Stanley           2016-08-30  587  
61a7904b6ace99 Iwona Winiarska        2021-12-04  588  	raw_spin_lock_irqsave(&gpio->lock, flags);
0e6ca482ec6e28 Billy Tsai             2024-08-30  589  	if (gpio->llops->copro_request)
0e6ca482ec6e28 Billy Tsai             2024-08-30  590  		copro = gpio->llops->copro_request(gpio, offset);
361b79119a4b7f Joel Stanley           2016-08-30  591  
0e6ca482ec6e28 Billy Tsai             2024-08-30  592  	gpio->llops->reg_bits_set(gpio, offset, reg_irq_type0, type0);
0e6ca482ec6e28 Billy Tsai             2024-08-30  593  	gpio->llops->reg_bits_set(gpio, offset, reg_irq_type1, type1);
0e6ca482ec6e28 Billy Tsai             2024-08-30  594  	gpio->llops->reg_bits_set(gpio, offset, reg_irq_type2, type2);
361b79119a4b7f Joel Stanley           2016-08-30  595  
0e6ca482ec6e28 Billy Tsai             2024-08-30 @596  	if (copro && gpio->llops->copro_release)
0e6ca482ec6e28 Billy Tsai             2024-08-30  597  		gpio->llops->copro_release(gpio, offset);
61a7904b6ace99 Iwona Winiarska        2021-12-04  598  	raw_spin_unlock_irqrestore(&gpio->lock, flags);
361b79119a4b7f Joel Stanley           2016-08-30  599  
361b79119a4b7f Joel Stanley           2016-08-30  600  	irq_set_handler_locked(d, handler);
361b79119a4b7f Joel Stanley           2016-08-30  601  
361b79119a4b7f Joel Stanley           2016-08-30  602  	return 0;
361b79119a4b7f Joel Stanley           2016-08-30  603  }

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 3/4] gpio: aspeed: Create llops to handle hardware access

[Dan Carpenter]

Hi Billy,

kernel test robot noticed the following build warnings:

https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Billy-Tsai/dt-bindings-gpio-aspeed-ast2400-gpio-Support-ast2700/20240830-114325
base:   https://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git gpio/for-next
patch link:    https://lore.kernel.org/r/20240830034047.2251482-4-billy_tsai%40aspeedtech.com
patch subject: [PATCH v2 3/4] gpio: aspeed: Create llops to handle hardware access
config: parisc-randconfig-r071-20240831 (https://download.01.org/0day-ci/archive/20240831/202408312313.HTx2vwvy-lkp@intel.com/config)
compiler: hppa-linux-gcc (GCC) 14.1.0

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202408312313.HTx2vwvy-lkp@intel.com/

smatch warnings:
drivers/gpio/gpio-aspeed.c:399 aspeed_gpio_set() error: uninitialized symbol 'copro'.
drivers/gpio/gpio-aspeed.c:418 aspeed_gpio_dir_in() error: uninitialized symbol 'copro'.
drivers/gpio/gpio-aspeed.c:443 aspeed_gpio_dir_out() error: uninitialized symbol 'copro'.
drivers/gpio/gpio-aspeed.c:507 aspeed_gpio_irq_ack() error: uninitialized symbol 'copro'.
drivers/gpio/gpio-aspeed.c:533 aspeed_gpio_irq_set_mask() error: uninitialized symbol 'copro'.
drivers/gpio/gpio-aspeed.c:596 aspeed_gpio_set_type() error: uninitialized symbol 'copro'.
drivers/gpio/gpio-aspeed.c:664 aspeed_gpio_reset_tolerance() error: uninitialized symbol 'copro'.

vim +/copro +399 drivers/gpio/gpio-aspeed.c

361b79119a4b7f Joel Stanley           2016-08-30  386  static void aspeed_gpio_set(struct gpio_chip *gc, unsigned int offset,
361b79119a4b7f Joel Stanley           2016-08-30  387  			    int val)
361b79119a4b7f Joel Stanley           2016-08-30  388  {
361b79119a4b7f Joel Stanley           2016-08-30  389  	struct aspeed_gpio *gpio = gpiochip_get_data(gc);
361b79119a4b7f Joel Stanley           2016-08-30  390  	unsigned long flags;
a7ca13826e478f Benjamin Herrenschmidt 2018-06-29  391  	bool copro;
361b79119a4b7f Joel Stanley           2016-08-30  392  
61a7904b6ace99 Iwona Winiarska        2021-12-04  393  	raw_spin_lock_irqsave(&gpio->lock, flags);
0e6ca482ec6e28 Billy Tsai             2024-08-30  394  	if (gpio->llops->copro_request)
0e6ca482ec6e28 Billy Tsai             2024-08-30  395  		copro = gpio->llops->copro_request(gpio, offset);

Uninitialized on else  path

361b79119a4b7f Joel Stanley           2016-08-30  396  
361b79119a4b7f Joel Stanley           2016-08-30  397  	__aspeed_gpio_set(gc, offset, val);
361b79119a4b7f Joel Stanley           2016-08-30  398  
0e6ca482ec6e28 Billy Tsai             2024-08-30 @399  	if (copro && gpio->llops->copro_release)
                                                            ^^^^^

0e6ca482ec6e28 Billy Tsai             2024-08-30  400  		gpio->llops->copro_release(gpio, offset);
61a7904b6ace99 Iwona Winiarska        2021-12-04  401  	raw_spin_unlock_irqrestore(&gpio->lock, flags);
361b79119a4b7f Joel Stanley           2016-08-30  402  }
361b79119a4b7f Joel Stanley           2016-08-30  403  
361b79119a4b7f Joel Stanley           2016-08-30  404  static int aspeed_gpio_dir_in(struct gpio_chip *gc, unsigned int offset)
361b79119a4b7f Joel Stanley           2016-08-30  405  {
361b79119a4b7f Joel Stanley           2016-08-30  406  	struct aspeed_gpio *gpio = gpiochip_get_data(gc);
361b79119a4b7f Joel Stanley           2016-08-30  407  	unsigned long flags;
a7ca13826e478f Benjamin Herrenschmidt 2018-06-29  408  	bool copro;
361b79119a4b7f Joel Stanley           2016-08-30  409  
1736f75d35e474 Andrew Jeffery         2017-01-24  410  	if (!have_input(gpio, offset))
1736f75d35e474 Andrew Jeffery         2017-01-24  411  		return -ENOTSUPP;
1736f75d35e474 Andrew Jeffery         2017-01-24  412  
61a7904b6ace99 Iwona Winiarska        2021-12-04  413  	raw_spin_lock_irqsave(&gpio->lock, flags);
361b79119a4b7f Joel Stanley           2016-08-30  414  
0e6ca482ec6e28 Billy Tsai             2024-08-30  415  	if (gpio->llops->copro_request)
0e6ca482ec6e28 Billy Tsai             2024-08-30  416  		copro = gpio->llops->copro_request(gpio, offset);
0e6ca482ec6e28 Billy Tsai             2024-08-30  417  	gpio->llops->reg_bits_set(gpio, offset, reg_dir, 0);
0e6ca482ec6e28 Billy Tsai             2024-08-30 @418  	if (copro && gpio->llops->copro_release)
0e6ca482ec6e28 Billy Tsai             2024-08-30  419  		gpio->llops->copro_release(gpio, offset);
361b79119a4b7f Joel Stanley           2016-08-30  420  
61a7904b6ace99 Iwona Winiarska        2021-12-04  421  	raw_spin_unlock_irqrestore(&gpio->lock, flags);
361b79119a4b7f Joel Stanley           2016-08-30  422  
361b79119a4b7f Joel Stanley           2016-08-30  423  	return 0;
361b79119a4b7f Joel Stanley           2016-08-30  424  }
361b79119a4b7f Joel Stanley           2016-08-30  425  
361b79119a4b7f Joel Stanley           2016-08-30  426  static int aspeed_gpio_dir_out(struct gpio_chip *gc,
361b79119a4b7f Joel Stanley           2016-08-30  427  			       unsigned int offset, int val)
361b79119a4b7f Joel Stanley           2016-08-30  428  {
361b79119a4b7f Joel Stanley           2016-08-30  429  	struct aspeed_gpio *gpio = gpiochip_get_data(gc);
361b79119a4b7f Joel Stanley           2016-08-30  430  	unsigned long flags;
a7ca13826e478f Benjamin Herrenschmidt 2018-06-29  431  	bool copro;
361b79119a4b7f Joel Stanley           2016-08-30  432  
1736f75d35e474 Andrew Jeffery         2017-01-24  433  	if (!have_output(gpio, offset))
1736f75d35e474 Andrew Jeffery         2017-01-24  434  		return -ENOTSUPP;
1736f75d35e474 Andrew Jeffery         2017-01-24  435  
61a7904b6ace99 Iwona Winiarska        2021-12-04  436  	raw_spin_lock_irqsave(&gpio->lock, flags);
361b79119a4b7f Joel Stanley           2016-08-30  437  
0e6ca482ec6e28 Billy Tsai             2024-08-30  438  	if (gpio->llops->copro_request)
0e6ca482ec6e28 Billy Tsai             2024-08-30  439  		copro = gpio->llops->copro_request(gpio, offset);
af7949284910a1 Benjamin Herrenschmidt 2018-05-17  440  	__aspeed_gpio_set(gc, offset, val);
0e6ca482ec6e28 Billy Tsai             2024-08-30  441  	gpio->llops->reg_bits_set(gpio, offset, reg_dir, 1);
361b79119a4b7f Joel Stanley           2016-08-30  442  
0e6ca482ec6e28 Billy Tsai             2024-08-30 @443  	if (copro && gpio->llops->copro_release)
0e6ca482ec6e28 Billy Tsai             2024-08-30  444  		gpio->llops->copro_release(gpio, offset);
61a7904b6ace99 Iwona Winiarska        2021-12-04  445  	raw_spin_unlock_irqrestore(&gpio->lock, flags);
361b79119a4b7f Joel Stanley           2016-08-30  446  
361b79119a4b7f Joel Stanley           2016-08-30  447  	return 0;
361b79119a4b7f Joel Stanley           2016-08-30  448  }
361b79119a4b7f Joel Stanley           2016-08-30  449  
361b79119a4b7f Joel Stanley           2016-08-30  450  static int aspeed_gpio_get_direction(struct gpio_chip *gc, unsigned int offset)
361b79119a4b7f Joel Stanley           2016-08-30  451  {
361b79119a4b7f Joel Stanley           2016-08-30  452  	struct aspeed_gpio *gpio = gpiochip_get_data(gc);
361b79119a4b7f Joel Stanley           2016-08-30  453  	unsigned long flags;
361b79119a4b7f Joel Stanley           2016-08-30  454  	u32 val;
361b79119a4b7f Joel Stanley           2016-08-30  455  
1736f75d35e474 Andrew Jeffery         2017-01-24  456  	if (!have_input(gpio, offset))
e42615ec233b30 Matti Vaittinen        2019-11-06  457  		return GPIO_LINE_DIRECTION_OUT;
1736f75d35e474 Andrew Jeffery         2017-01-24  458  
1736f75d35e474 Andrew Jeffery         2017-01-24  459  	if (!have_output(gpio, offset))
e42615ec233b30 Matti Vaittinen        2019-11-06  460  		return GPIO_LINE_DIRECTION_IN;
1736f75d35e474 Andrew Jeffery         2017-01-24  461  
61a7904b6ace99 Iwona Winiarska        2021-12-04  462  	raw_spin_lock_irqsave(&gpio->lock, flags);
361b79119a4b7f Joel Stanley           2016-08-30  463  
0e6ca482ec6e28 Billy Tsai             2024-08-30  464  	val = gpio->llops->reg_bits_read(gpio, offset, reg_dir);
361b79119a4b7f Joel Stanley           2016-08-30  465  
61a7904b6ace99 Iwona Winiarska        2021-12-04  466  	raw_spin_unlock_irqrestore(&gpio->lock, flags);
361b79119a4b7f Joel Stanley           2016-08-30  467  
e42615ec233b30 Matti Vaittinen        2019-11-06  468  	return val ? GPIO_LINE_DIRECTION_OUT : GPIO_LINE_DIRECTION_IN;
361b79119a4b7f Joel Stanley           2016-08-30  469  }
361b79119a4b7f Joel Stanley           2016-08-30  470  
361b79119a4b7f Joel Stanley           2016-08-30  471  static inline int irqd_to_aspeed_gpio_data(struct irq_data *d,
361b79119a4b7f Joel Stanley           2016-08-30  472  					   struct aspeed_gpio **gpio,
0e6ca482ec6e28 Billy Tsai             2024-08-30  473  					   int *offset)
361b79119a4b7f Joel Stanley           2016-08-30  474  {
1736f75d35e474 Andrew Jeffery         2017-01-24  475  	struct aspeed_gpio *internal;
361b79119a4b7f Joel Stanley           2016-08-30  476  
a7ca13826e478f Benjamin Herrenschmidt 2018-06-29  477  	*offset = irqd_to_hwirq(d);
361b79119a4b7f Joel Stanley           2016-08-30  478  
1736f75d35e474 Andrew Jeffery         2017-01-24  479  	internal = irq_data_get_irq_chip_data(d);
1736f75d35e474 Andrew Jeffery         2017-01-24  480  
1736f75d35e474 Andrew Jeffery         2017-01-24  481  	/* This might be a bit of a questionable place to check */
a7ca13826e478f Benjamin Herrenschmidt 2018-06-29  482  	if (!have_irq(internal, *offset))
1736f75d35e474 Andrew Jeffery         2017-01-24  483  		return -ENOTSUPP;
1736f75d35e474 Andrew Jeffery         2017-01-24  484  
1736f75d35e474 Andrew Jeffery         2017-01-24  485  	*gpio = internal;
361b79119a4b7f Joel Stanley           2016-08-30  486  
361b79119a4b7f Joel Stanley           2016-08-30  487  	return 0;
361b79119a4b7f Joel Stanley           2016-08-30  488  }
361b79119a4b7f Joel Stanley           2016-08-30  489  
361b79119a4b7f Joel Stanley           2016-08-30  490  static void aspeed_gpio_irq_ack(struct irq_data *d)
361b79119a4b7f Joel Stanley           2016-08-30  491  {
361b79119a4b7f Joel Stanley           2016-08-30  492  	struct aspeed_gpio *gpio;
361b79119a4b7f Joel Stanley           2016-08-30  493  	unsigned long flags;
a7ca13826e478f Benjamin Herrenschmidt 2018-06-29  494  	int rc, offset;
a7ca13826e478f Benjamin Herrenschmidt 2018-06-29  495  	bool copro;
361b79119a4b7f Joel Stanley           2016-08-30  496  
0e6ca482ec6e28 Billy Tsai             2024-08-30  497  	rc = irqd_to_aspeed_gpio_data(d, &gpio, &offset);
361b79119a4b7f Joel Stanley           2016-08-30  498  	if (rc)
361b79119a4b7f Joel Stanley           2016-08-30  499  		return;
361b79119a4b7f Joel Stanley           2016-08-30  500  
61a7904b6ace99 Iwona Winiarska        2021-12-04  501  	raw_spin_lock_irqsave(&gpio->lock, flags);
0e6ca482ec6e28 Billy Tsai             2024-08-30  502  	if (gpio->llops->copro_request)
0e6ca482ec6e28 Billy Tsai             2024-08-30  503  		copro = gpio->llops->copro_request(gpio, offset);
a7ca13826e478f Benjamin Herrenschmidt 2018-06-29  504  
0e6ca482ec6e28 Billy Tsai             2024-08-30  505  	gpio->llops->reg_bits_set(gpio, offset, reg_irq_status, 1);
a7ca13826e478f Benjamin Herrenschmidt 2018-06-29  506  
0e6ca482ec6e28 Billy Tsai             2024-08-30 @507  	if (copro && gpio->llops->copro_release)
0e6ca482ec6e28 Billy Tsai             2024-08-30  508  		gpio->llops->copro_release(gpio, offset);
61a7904b6ace99 Iwona Winiarska        2021-12-04  509  	raw_spin_unlock_irqrestore(&gpio->lock, flags);
361b79119a4b7f Joel Stanley           2016-08-30  510  }
361b79119a4b7f Joel Stanley           2016-08-30  511  
361b79119a4b7f Joel Stanley           2016-08-30  512  static void aspeed_gpio_irq_set_mask(struct irq_data *d, bool set)
361b79119a4b7f Joel Stanley           2016-08-30  513  {
361b79119a4b7f Joel Stanley           2016-08-30  514  	struct aspeed_gpio *gpio;
361b79119a4b7f Joel Stanley           2016-08-30  515  	unsigned long flags;
a7ca13826e478f Benjamin Herrenschmidt 2018-06-29  516  	int rc, offset;
a7ca13826e478f Benjamin Herrenschmidt 2018-06-29  517  	bool copro;
361b79119a4b7f Joel Stanley           2016-08-30  518  
0e6ca482ec6e28 Billy Tsai             2024-08-30  519  	rc = irqd_to_aspeed_gpio_data(d, &gpio, &offset);
361b79119a4b7f Joel Stanley           2016-08-30  520  	if (rc)
361b79119a4b7f Joel Stanley           2016-08-30  521  		return;
361b79119a4b7f Joel Stanley           2016-08-30  522  
061df08f063a97 Linus Walleij          2023-03-09  523  	/* Unmasking the IRQ */
061df08f063a97 Linus Walleij          2023-03-09  524  	if (set)
061df08f063a97 Linus Walleij          2023-03-09  525  		gpiochip_enable_irq(&gpio->chip, irqd_to_hwirq(d));
061df08f063a97 Linus Walleij          2023-03-09  526  
61a7904b6ace99 Iwona Winiarska        2021-12-04  527  	raw_spin_lock_irqsave(&gpio->lock, flags);
0e6ca482ec6e28 Billy Tsai             2024-08-30  528  	if (gpio->llops->copro_request)
0e6ca482ec6e28 Billy Tsai             2024-08-30  529  		copro = gpio->llops->copro_request(gpio, offset);
361b79119a4b7f Joel Stanley           2016-08-30  530  
0e6ca482ec6e28 Billy Tsai             2024-08-30  531  	gpio->llops->reg_bits_set(gpio, offset, reg_irq_enable, set);
361b79119a4b7f Joel Stanley           2016-08-30  532  
0e6ca482ec6e28 Billy Tsai             2024-08-30 @533  	if (copro && gpio->llops->copro_release)
0e6ca482ec6e28 Billy Tsai             2024-08-30  534  		gpio->llops->copro_release(gpio, offset);
61a7904b6ace99 Iwona Winiarska        2021-12-04  535  	raw_spin_unlock_irqrestore(&gpio->lock, flags);
061df08f063a97 Linus Walleij          2023-03-09  536  
061df08f063a97 Linus Walleij          2023-03-09  537  	/* Masking the IRQ */
061df08f063a97 Linus Walleij          2023-03-09  538  	if (!set)
061df08f063a97 Linus Walleij          2023-03-09  539  		gpiochip_disable_irq(&gpio->chip, irqd_to_hwirq(d));
361b79119a4b7f Joel Stanley           2016-08-30  540  }
361b79119a4b7f Joel Stanley           2016-08-30  541  
361b79119a4b7f Joel Stanley           2016-08-30  542  static void aspeed_gpio_irq_mask(struct irq_data *d)
361b79119a4b7f Joel Stanley           2016-08-30  543  {
361b79119a4b7f Joel Stanley           2016-08-30  544  	aspeed_gpio_irq_set_mask(d, false);
361b79119a4b7f Joel Stanley           2016-08-30  545  }
361b79119a4b7f Joel Stanley           2016-08-30  546  
361b79119a4b7f Joel Stanley           2016-08-30  547  static void aspeed_gpio_irq_unmask(struct irq_data *d)
361b79119a4b7f Joel Stanley           2016-08-30  548  {
361b79119a4b7f Joel Stanley           2016-08-30  549  	aspeed_gpio_irq_set_mask(d, true);
361b79119a4b7f Joel Stanley           2016-08-30  550  }
361b79119a4b7f Joel Stanley           2016-08-30  551  
361b79119a4b7f Joel Stanley           2016-08-30  552  static int aspeed_gpio_set_type(struct irq_data *d, unsigned int type)
361b79119a4b7f Joel Stanley           2016-08-30  553  {
361b79119a4b7f Joel Stanley           2016-08-30  554  	u32 type0 = 0;
361b79119a4b7f Joel Stanley           2016-08-30  555  	u32 type1 = 0;
361b79119a4b7f Joel Stanley           2016-08-30  556  	u32 type2 = 0;
361b79119a4b7f Joel Stanley           2016-08-30  557  	irq_flow_handler_t handler;
361b79119a4b7f Joel Stanley           2016-08-30  558  	struct aspeed_gpio *gpio;
361b79119a4b7f Joel Stanley           2016-08-30  559  	unsigned long flags;
a7ca13826e478f Benjamin Herrenschmidt 2018-06-29  560  	int rc, offset;
a7ca13826e478f Benjamin Herrenschmidt 2018-06-29  561  	bool copro;
361b79119a4b7f Joel Stanley           2016-08-30  562  
0e6ca482ec6e28 Billy Tsai             2024-08-30  563  	rc = irqd_to_aspeed_gpio_data(d, &gpio, &offset);
361b79119a4b7f Joel Stanley           2016-08-30  564  	if (rc)
361b79119a4b7f Joel Stanley           2016-08-30  565  		return -EINVAL;
361b79119a4b7f Joel Stanley           2016-08-30  566  
361b79119a4b7f Joel Stanley           2016-08-30  567  	switch (type & IRQ_TYPE_SENSE_MASK) {
361b79119a4b7f Joel Stanley           2016-08-30  568  	case IRQ_TYPE_EDGE_BOTH:
0e6ca482ec6e28 Billy Tsai             2024-08-30  569  		type2 = 1;
df561f6688fef7 Gustavo A. R. Silva    2020-08-23  570  		fallthrough;
361b79119a4b7f Joel Stanley           2016-08-30  571  	case IRQ_TYPE_EDGE_RISING:
0e6ca482ec6e28 Billy Tsai             2024-08-30  572  		type0 = 1;
df561f6688fef7 Gustavo A. R. Silva    2020-08-23  573  		fallthrough;
361b79119a4b7f Joel Stanley           2016-08-30  574  	case IRQ_TYPE_EDGE_FALLING:
361b79119a4b7f Joel Stanley           2016-08-30  575  		handler = handle_edge_irq;
361b79119a4b7f Joel Stanley           2016-08-30  576  		break;
361b79119a4b7f Joel Stanley           2016-08-30  577  	case IRQ_TYPE_LEVEL_HIGH:
0e6ca482ec6e28 Billy Tsai             2024-08-30  578  		type0 = 1;
df561f6688fef7 Gustavo A. R. Silva    2020-08-23  579  		fallthrough;
361b79119a4b7f Joel Stanley           2016-08-30  580  	case IRQ_TYPE_LEVEL_LOW:
0e6ca482ec6e28 Billy Tsai             2024-08-30  581  		type1 = 1;
361b79119a4b7f Joel Stanley           2016-08-30  582  		handler = handle_level_irq;
361b79119a4b7f Joel Stanley           2016-08-30  583  		break;
361b79119a4b7f Joel Stanley           2016-08-30  584  	default:
361b79119a4b7f Joel Stanley           2016-08-30  585  		return -EINVAL;
361b79119a4b7f Joel Stanley           2016-08-30  586  	}
361b79119a4b7f Joel Stanley           2016-08-30  587  
61a7904b6ace99 Iwona Winiarska        2021-12-04  588  	raw_spin_lock_irqsave(&gpio->lock, flags);
0e6ca482ec6e28 Billy Tsai             2024-08-30  589  	if (gpio->llops->copro_request)
0e6ca482ec6e28 Billy Tsai             2024-08-30  590  		copro = gpio->llops->copro_request(gpio, offset);
361b79119a4b7f Joel Stanley           2016-08-30  591  
0e6ca482ec6e28 Billy Tsai             2024-08-30  592  	gpio->llops->reg_bits_set(gpio, offset, reg_irq_type0, type0);
0e6ca482ec6e28 Billy Tsai             2024-08-30  593  	gpio->llops->reg_bits_set(gpio, offset, reg_irq_type1, type1);
0e6ca482ec6e28 Billy Tsai             2024-08-30  594  	gpio->llops->reg_bits_set(gpio, offset, reg_irq_type2, type2);
361b79119a4b7f Joel Stanley           2016-08-30  595  
0e6ca482ec6e28 Billy Tsai             2024-08-30 @596  	if (copro && gpio->llops->copro_release)
0e6ca482ec6e28 Billy Tsai             2024-08-30  597  		gpio->llops->copro_release(gpio, offset);
61a7904b6ace99 Iwona Winiarska        2021-12-04  598  	raw_spin_unlock_irqrestore(&gpio->lock, flags);
361b79119a4b7f Joel Stanley           2016-08-30  599  
361b79119a4b7f Joel Stanley           2016-08-30  600  	irq_set_handler_locked(d, handler);
361b79119a4b7f Joel Stanley           2016-08-30  601  
361b79119a4b7f Joel Stanley           2016-08-30  602  	return 0;
361b79119a4b7f Joel Stanley           2016-08-30  603  }

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH v2 3/4] gpio: aspeed: Create llops to handle hardware access

[Andrew Jeffery]

On Fri, 2024-08-30 at 11:40 +0800, Billy Tsai wrote:
> Add low-level operations (llops) to abstract the register access for GPIO
> registers and the coprocessor request/release. With this abstraction
> layer, the driver can separate the hardware and software logic, making it
> easier to extend the driver to support different hardware register
> layouts.
> 
> Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
> ---
>  drivers/gpio/gpio-aspeed.c | 309 +++++++++++++++++--------------------
>  1 file changed, 138 insertions(+), 171 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-aspeed.c b/drivers/gpio/gpio-aspeed.c
> index 24f50a0ea4ab..74c4e80958bf 100644
> --- a/drivers/gpio/gpio-aspeed.c
> +++ b/drivers/gpio/gpio-aspeed.c
> @@ -39,6 +39,7 @@ struct aspeed_bank_props {
>  struct aspeed_gpio_config {
>  	unsigned int nr_gpios;
>  	const struct aspeed_bank_props *props;
> +	unsigned int version;

I'm interested to see whether this is necessary in practice.

>  };
>  
>  /*
> @@ -62,10 +63,13 @@ struct aspeed_gpio {
>  
>  	u8 *offset_timer;
>  	unsigned int timer_users[4];
> +	const int *debounce_timers_array;
> +	int debounce_timers_num;
>  	struct clk *clk;
>  
>  	u32 *dcache;
>  	u8 *cf_copro_bankmap;
> +	const struct aspeed_gpio_llops *llops;
>  };
>  
>  struct aspeed_gpio_bank {
> @@ -178,6 +182,15 @@ enum aspeed_gpio_reg {
>  	reg_cmdsrc1,
>  };
>  
> +struct aspeed_gpio_llops {
> +	bool (*copro_request)(struct aspeed_gpio *gpio, unsigned int offset);
> +	void (*copro_release)(struct aspeed_gpio *gpio, unsigned int offset);
> +	void (*reg_bits_set)(struct aspeed_gpio *gpio, unsigned int offset,
> +			     const enum aspeed_gpio_reg reg, u32 val);
> +	u32 (*reg_bits_read)(struct aspeed_gpio *gpio, unsigned int offset,
> +			     const enum aspeed_gpio_reg reg);
> +};
> +
>  #define GPIO_VAL_VALUE	0x00
>  #define GPIO_VAL_DIR	0x04
>  
> @@ -237,10 +250,6 @@ static inline void __iomem *bank_reg(struct aspeed_gpio *gpio,
>  #define GPIO_OFFSET(x)	((x) & 0x1f)
>  #define GPIO_BIT(x)	BIT(GPIO_OFFSET(x))
>  
> -#define _GPIO_SET_DEBOUNCE(t, o, i) ((!!((t) & BIT(i))) << GPIO_OFFSET(o))
> -#define GPIO_SET_DEBOUNCE1(t, o) _GPIO_SET_DEBOUNCE(t, o, 1)
> -#define GPIO_SET_DEBOUNCE2(t, o) _GPIO_SET_DEBOUNCE(t, o, 0)
> -
>  static const struct aspeed_gpio_bank *to_bank(unsigned int offset)
>  {
>  	unsigned int bank = GPIO_BANK(offset);
> @@ -296,35 +305,17 @@ static inline bool have_output(struct aspeed_gpio *gpio, unsigned int offset)
>  }
>  
>  static void aspeed_gpio_change_cmd_source(struct aspeed_gpio *gpio,
> -					  const struct aspeed_gpio_bank *bank,
> -					  int bindex, int cmdsrc)
> +					  unsigned int offset,
> +					  int cmdsrc)
>  {
> -	void __iomem *c0 = bank_reg(gpio, bank, reg_cmdsrc0);
> -	void __iomem *c1 = bank_reg(gpio, bank, reg_cmdsrc1);
> -	u32 bit, reg;
> -
>  	/*
> -	 * Each register controls 4 banks, so take the bottom 2
> -	 * bits of the bank index, and use them to select the
> -	 * right control bit (0, 8, 16 or 24).
> +	 * The command source register is only valid in bits 0, 8, 16, and 24, so we use
> +	 * (offset & ~(0x7)) to ensure that reg_bits_set always targets a valid bit.
>  	 */
> -	bit = BIT((bindex & 3) << 3);
> -
>  	/* Source 1 first to avoid illegal 11 combination */
> -	reg = ioread32(c1);
> -	if (cmdsrc & 2)
> -		reg |= bit;
> -	else
> -		reg &= ~bit;
> -	iowrite32(reg, c1);
> -
> +	gpio->llops->reg_bits_set(gpio, offset & ~(0x7), reg_cmdsrc1, !!(cmdsrc & BIT(1)));
>  	/* Then Source 0 */
> -	reg = ioread32(c0);
> -	if (cmdsrc & 1)
> -		reg |= bit;
> -	else
> -		reg &= ~bit;
> -	iowrite32(reg, c0);
> +	gpio->llops->reg_bits_set(gpio, offset & ~(0x7), reg_cmdsrc0, !!(cmdsrc & BIT(0)));
>  }
>  
>  static bool aspeed_gpio_copro_request(struct aspeed_gpio *gpio,
> @@ -343,7 +334,7 @@ static bool aspeed_gpio_copro_request(struct aspeed_gpio *gpio,
>  	copro_ops->request_access(copro_data);
>  
>  	/* Change command source back to ARM */
> -	aspeed_gpio_change_cmd_source(gpio, bank, offset >> 3, GPIO_CMDSRC_ARM);
> +	aspeed_gpio_change_cmd_source(gpio, offset, GPIO_CMDSRC_ARM);
>  
>  	/* Update cache */
>  	gpio->dcache[GPIO_BANK(offset)] = ioread32(bank_reg(gpio, bank, reg_rdata));
> @@ -354,8 +345,6 @@ static bool aspeed_gpio_copro_request(struct aspeed_gpio *gpio,
>  static void aspeed_gpio_copro_release(struct aspeed_gpio *gpio,
>  				      unsigned int offset)
>  {
> -	const struct aspeed_gpio_bank *bank = to_bank(offset);
> -
>  	if (!copro_ops || !gpio->cf_copro_bankmap)
>  		return;
>  	if (!gpio->cf_copro_bankmap[offset >> 3])
> @@ -364,7 +353,7 @@ static void aspeed_gpio_copro_release(struct aspeed_gpio *gpio,
>  		return;
>  
>  	/* Change command source back to ColdFire */
> -	aspeed_gpio_change_cmd_source(gpio, bank, offset >> 3,
> +	aspeed_gpio_change_cmd_source(gpio, offset,
>  				      GPIO_CMDSRC_COLDFIRE);
>  
>  	/* Restart the coprocessor */
> @@ -374,29 +363,24 @@ static void aspeed_gpio_copro_release(struct aspeed_gpio *gpio,
>  static int aspeed_gpio_get(struct gpio_chip *gc, unsigned int offset)
>  {
>  	struct aspeed_gpio *gpio = gpiochip_get_data(gc);
> -	const struct aspeed_gpio_bank *bank = to_bank(offset);
>  
> -	return !!(ioread32(bank_reg(gpio, bank, reg_val)) & GPIO_BIT(offset));
> +	return gpio->llops->reg_bits_read(gpio, offset, reg_val);
>  }
>  
>  static void __aspeed_gpio_set(struct gpio_chip *gc, unsigned int offset,
>  			      int val)
>  {
>  	struct aspeed_gpio *gpio = gpiochip_get_data(gc);
> -	const struct aspeed_gpio_bank *bank = to_bank(offset);
> -	void __iomem *addr;
>  	u32 reg;
>  
> -	addr = bank_reg(gpio, bank, reg_val);
>  	reg = gpio->dcache[GPIO_BANK(offset)];
> -
>  	if (val)
>  		reg |= GPIO_BIT(offset);
>  	else
>  		reg &= ~GPIO_BIT(offset);
>  	gpio->dcache[GPIO_BANK(offset)] = reg;
>  
> -	iowrite32(reg, addr);
> +	gpio->llops->reg_bits_set(gpio, offset, reg_val, val);
>  }
>  
>  static void aspeed_gpio_set(struct gpio_chip *gc, unsigned int offset,
> @@ -407,36 +391,32 @@ static void aspeed_gpio_set(struct gpio_chip *gc, unsigned int offset,
>  	bool copro;
>  
>  	raw_spin_lock_irqsave(&gpio->lock, flags);
> -	copro = aspeed_gpio_copro_request(gpio, offset);
> +	if (gpio->llops->copro_request)
> +		copro = gpio->llops->copro_request(gpio, offset);

A bit minor perhaps, but I'd add a static function to do the test and
call:

static bool aspeed_gpio_copro_request(struct aspeed_gpio *gpio,
unsigned int offset)
{
    if (gpio->llops->copro_request)
        return gpio->llops->copro_request(gpio, offset);

    return false;
}

static void aspeed_gpio_copro_release(struct aspeed_gpio *gpio,
unsigned int offset)
{
    if (gpio->llops->copro_release)
        gpio->llops->copro_release(gpio, offset);
}

A bit less noise at the call-sites that way.

That's only a suggestion for the optional copro function pointers
though. For the reg_bits_read and reg_bits_set callbacks we should
ensure they're not NULL in the driver probe() implementation, or error
out if they are.

>  
>  	__aspeed_gpio_set(gc, offset, val);
>  
> -	if (copro)
> -		aspeed_gpio_copro_release(gpio, offset);
> +	if (copro && gpio->llops->copro_release)
> +		gpio->llops->copro_release(gpio, offset);
>  	raw_spin_unlock_irqrestore(&gpio->lock, flags);
>  }
>  
>  static int aspeed_gpio_dir_in(struct gpio_chip *gc, unsigned int offset)
>  {
>  	struct aspeed_gpio *gpio = gpiochip_get_data(gc);
> -	const struct aspeed_gpio_bank *bank = to_bank(offset);
> -	void __iomem *addr = bank_reg(gpio, bank, reg_dir);
>  	unsigned long flags;
>  	bool copro;
> -	u32 reg;
>  
>  	if (!have_input(gpio, offset))
>  		return -ENOTSUPP;
>  
>  	raw_spin_lock_irqsave(&gpio->lock, flags);
>  
> -	reg = ioread32(addr);
> -	reg &= ~GPIO_BIT(offset);
> -
> -	copro = aspeed_gpio_copro_request(gpio, offset);
> -	iowrite32(reg, addr);
> -	if (copro)
> -		aspeed_gpio_copro_release(gpio, offset);
> +	if (gpio->llops->copro_request)
> +		copro = gpio->llops->copro_request(gpio, offset);
> +	gpio->llops->reg_bits_set(gpio, offset, reg_dir, 0);
> +	if (copro && gpio->llops->copro_release)
> +		gpio->llops->copro_release(gpio, offset);
>  
>  	raw_spin_unlock_irqrestore(&gpio->lock, flags);
>  
> @@ -447,26 +427,21 @@ static int aspeed_gpio_dir_out(struct gpio_chip *gc,
>  			       unsigned int offset, int val)
>  {
>  	struct aspeed_gpio *gpio = gpiochip_get_data(gc);
> -	const struct aspeed_gpio_bank *bank = to_bank(offset);
> -	void __iomem *addr = bank_reg(gpio, bank, reg_dir);
>  	unsigned long flags;
>  	bool copro;
> -	u32 reg;
>  
>  	if (!have_output(gpio, offset))
>  		return -ENOTSUPP;
>  
>  	raw_spin_lock_irqsave(&gpio->lock, flags);
>  
> -	reg = ioread32(addr);
> -	reg |= GPIO_BIT(offset);
> -
> -	copro = aspeed_gpio_copro_request(gpio, offset);
> +	if (gpio->llops->copro_request)
> +		copro = gpio->llops->copro_request(gpio, offset);
>  	__aspeed_gpio_set(gc, offset, val);
> -	iowrite32(reg, addr);
> +	gpio->llops->reg_bits_set(gpio, offset, reg_dir, 1);
>  
> -	if (copro)
> -		aspeed_gpio_copro_release(gpio, offset);
> +	if (copro && gpio->llops->copro_release)
> +		gpio->llops->copro_release(gpio, offset);
>  	raw_spin_unlock_irqrestore(&gpio->lock, flags);
>  
>  	return 0;
> @@ -475,7 +450,6 @@ static int aspeed_gpio_dir_out(struct gpio_chip *gc,
>  static int aspeed_gpio_get_direction(struct gpio_chip *gc, unsigned int offset)
>  {
>  	struct aspeed_gpio *gpio = gpiochip_get_data(gc);
> -	const struct aspeed_gpio_bank *bank = to_bank(offset);
>  	unsigned long flags;
>  	u32 val;
>  
> @@ -487,7 +461,7 @@ static int aspeed_gpio_get_direction(struct gpio_chip *gc, unsigned int offset)
>  
>  	raw_spin_lock_irqsave(&gpio->lock, flags);
>  
> -	val = ioread32(bank_reg(gpio, bank, reg_dir)) & GPIO_BIT(offset);
> +	val = gpio->llops->reg_bits_read(gpio, offset, reg_dir);
>  
>  	raw_spin_unlock_irqrestore(&gpio->lock, flags);
>  
> @@ -496,8 +470,7 @@ static int aspeed_gpio_get_direction(struct gpio_chip *gc, unsigned int offset)
>  
>  static inline int irqd_to_aspeed_gpio_data(struct irq_data *d,
>  					   struct aspeed_gpio **gpio,
> -					   const struct aspeed_gpio_bank **bank,
> -					   u32 *bit, int *offset)
> +					   int *offset)
>  {
>  	struct aspeed_gpio *internal;
>  
> @@ -510,70 +483,55 @@ static inline int irqd_to_aspeed_gpio_data(struct irq_data *d,
>  		return -ENOTSUPP;
>  
>  	*gpio = internal;
> -	*bank = to_bank(*offset);
> -	*bit = GPIO_BIT(*offset);
>  
>  	return 0;
>  }
>  
>  static void aspeed_gpio_irq_ack(struct irq_data *d)
>  {
> -	const struct aspeed_gpio_bank *bank;
>  	struct aspeed_gpio *gpio;
>  	unsigned long flags;
> -	void __iomem *status_addr;
>  	int rc, offset;
>  	bool copro;
> -	u32 bit;
>  
> -	rc = irqd_to_aspeed_gpio_data(d, &gpio, &bank, &bit, &offset);
> +	rc = irqd_to_aspeed_gpio_data(d, &gpio, &offset);
>  	if (rc)
>  		return;
>  
> -	status_addr = bank_reg(gpio, bank, reg_irq_status);
> -
>  	raw_spin_lock_irqsave(&gpio->lock, flags);
> -	copro = aspeed_gpio_copro_request(gpio, offset);
> +	if (gpio->llops->copro_request)
> +		copro = gpio->llops->copro_request(gpio, offset);
>  
> -	iowrite32(bit, status_addr);
> +	gpio->llops->reg_bits_set(gpio, offset, reg_irq_status, 1);
>  
> -	if (copro)
> -		aspeed_gpio_copro_release(gpio, offset);
> +	if (copro && gpio->llops->copro_release)
> +		gpio->llops->copro_release(gpio, offset);
>  	raw_spin_unlock_irqrestore(&gpio->lock, flags);
>  }
>  
>  static void aspeed_gpio_irq_set_mask(struct irq_data *d, bool set)
>  {
> -	const struct aspeed_gpio_bank *bank;
>  	struct aspeed_gpio *gpio;
>  	unsigned long flags;
> -	u32 reg, bit;
> -	void __iomem *addr;
>  	int rc, offset;
>  	bool copro;
>  
> -	rc = irqd_to_aspeed_gpio_data(d, &gpio, &bank, &bit, &offset);
> +	rc = irqd_to_aspeed_gpio_data(d, &gpio, &offset);
>  	if (rc)
>  		return;
>  
> -	addr = bank_reg(gpio, bank, reg_irq_enable);
> -
>  	/* Unmasking the IRQ */
>  	if (set)
>  		gpiochip_enable_irq(&gpio->chip, irqd_to_hwirq(d));
>  
>  	raw_spin_lock_irqsave(&gpio->lock, flags);
> -	copro = aspeed_gpio_copro_request(gpio, offset);
> +	if (gpio->llops->copro_request)
> +		copro = gpio->llops->copro_request(gpio, offset);
>  
> -	reg = ioread32(addr);
> -	if (set)
> -		reg |= bit;
> -	else
> -		reg &= ~bit;
> -	iowrite32(reg, addr);
> +	gpio->llops->reg_bits_set(gpio, offset, reg_irq_enable, set);
>  
> -	if (copro)
> -		aspeed_gpio_copro_release(gpio, offset);
> +	if (copro && gpio->llops->copro_release)
> +		gpio->llops->copro_release(gpio, offset);
>  	raw_spin_unlock_irqrestore(&gpio->lock, flags);
>  
>  	/* Masking the IRQ */
> @@ -596,34 +554,31 @@ static int aspeed_gpio_set_type(struct irq_data *d, unsigned int type)
>  	u32 type0 = 0;
>  	u32 type1 = 0;
>  	u32 type2 = 0;
> -	u32 bit, reg;
> -	const struct aspeed_gpio_bank *bank;
>  	irq_flow_handler_t handler;
>  	struct aspeed_gpio *gpio;
>  	unsigned long flags;
> -	void __iomem *addr;
>  	int rc, offset;
>  	bool copro;
>  
> -	rc = irqd_to_aspeed_gpio_data(d, &gpio, &bank, &bit, &offset);
> +	rc = irqd_to_aspeed_gpio_data(d, &gpio, &offset);
>  	if (rc)
>  		return -EINVAL;
>  
>  	switch (type & IRQ_TYPE_SENSE_MASK) {
>  	case IRQ_TYPE_EDGE_BOTH:
> -		type2 |= bit;
> +		type2 = 1;
>  		fallthrough;
>  	case IRQ_TYPE_EDGE_RISING:
> -		type0 |= bit;
> +		type0 = 1;
>  		fallthrough;
>  	case IRQ_TYPE_EDGE_FALLING:
>  		handler = handle_edge_irq;
>  		break;
>  	case IRQ_TYPE_LEVEL_HIGH:
> -		type0 |= bit;
> +		type0 = 1;
>  		fallthrough;
>  	case IRQ_TYPE_LEVEL_LOW:
> -		type1 |= bit;
> +		type1 = 1;
>  		handler = handle_level_irq;
>  		break;
>  	default:
> @@ -631,25 +586,15 @@ static int aspeed_gpio_set_type(struct irq_data *d, unsigned int type)
>  	}
>  
>  	raw_spin_lock_irqsave(&gpio->lock, flags);
> -	copro = aspeed_gpio_copro_request(gpio, offset);
> -
> -	addr = bank_reg(gpio, bank, reg_irq_type0);
> -	reg = ioread32(addr);
> -	reg = (reg & ~bit) | type0;
> -	iowrite32(reg, addr);
> +	if (gpio->llops->copro_request)
> +		copro = gpio->llops->copro_request(gpio, offset);
>  
> -	addr = bank_reg(gpio, bank, reg_irq_type1);
> -	reg = ioread32(addr);
> -	reg = (reg & ~bit) | type1;
> -	iowrite32(reg, addr);
> +	gpio->llops->reg_bits_set(gpio, offset, reg_irq_type0, type0);
> +	gpio->llops->reg_bits_set(gpio, offset, reg_irq_type1, type1);
> +	gpio->llops->reg_bits_set(gpio, offset, reg_irq_type2, type2);
>  
> -	addr = bank_reg(gpio, bank, reg_irq_type2);
> -	reg = ioread32(addr);
> -	reg = (reg & ~bit) | type2;
> -	iowrite32(reg, addr);
> -
> -	if (copro)
> -		aspeed_gpio_copro_release(gpio, offset);
> +	if (copro && gpio->llops->copro_release)
> +		gpio->llops->copro_release(gpio, offset);
>  	raw_spin_unlock_irqrestore(&gpio->lock, flags);
>  
>  	irq_set_handler_locked(d, handler);
> @@ -661,7 +606,6 @@ static void aspeed_gpio_irq_handler(struct irq_desc *desc)
>  {
>  	struct gpio_chip *gc = irq_desc_get_handler_data(desc);
>  	struct irq_chip *ic = irq_desc_get_chip(desc);
> -	struct aspeed_gpio *data = gpiochip_get_data(gc);
>  	unsigned int i, p, banks;
>  	unsigned long reg;
>  	struct aspeed_gpio *gpio = gpiochip_get_data(gc);
> @@ -670,9 +614,7 @@ static void aspeed_gpio_irq_handler(struct irq_desc *desc)
>  
>  	banks = DIV_ROUND_UP(gpio->chip.ngpio, 32);
>  	for (i = 0; i < banks; i++) {
> -		const struct aspeed_gpio_bank *bank = &aspeed_gpio_banks[i];
> -
> -		reg = ioread32(bank_reg(data, bank, reg_irq_status));
> +		reg = gpio->llops->reg_bits_read(gpio, i, reg_irq_status);
>  
>  		for_each_set_bit(p, &reg, 32)
>  			generic_handle_domain_irq(gc->irq.domain, i * 32 + p);
> @@ -711,26 +653,16 @@ static int aspeed_gpio_reset_tolerance(struct gpio_chip *chip,
>  {
>  	struct aspeed_gpio *gpio = gpiochip_get_data(chip);
>  	unsigned long flags;
> -	void __iomem *treg;
>  	bool copro;
> -	u32 val;
> -
> -	treg = bank_reg(gpio, to_bank(offset), reg_tolerance);
>  
>  	raw_spin_lock_irqsave(&gpio->lock, flags);
> -	copro = aspeed_gpio_copro_request(gpio, offset);
> +	if (gpio->llops->copro_request)
> +		copro = gpio->llops->copro_request(gpio, offset);
>  
> -	val = readl(treg);
> +	gpio->llops->reg_bits_set(gpio, offset, reg_tolerance, enable);
>  
> -	if (enable)
> -		val |= GPIO_BIT(offset);
> -	else
> -		val &= ~GPIO_BIT(offset);
> -
> -	writel(val, treg);
> -
> -	if (copro)
> -		aspeed_gpio_copro_release(gpio, offset);
> +	if (copro && gpio->llops->copro_release)
> +		gpio->llops->copro_release(gpio, offset);
>  	raw_spin_unlock_irqrestore(&gpio->lock, flags);
>  
>  	return 0;
> @@ -821,21 +753,11 @@ static inline bool timer_allocation_registered(struct aspeed_gpio *gpio,
>  static void configure_timer(struct aspeed_gpio *gpio, unsigned int offset,
>  		unsigned int timer)
>  {
> -	const struct aspeed_gpio_bank *bank = to_bank(offset);
> -	const u32 mask = GPIO_BIT(offset);
> -	void __iomem *addr;
> -	u32 val;
> -
>  	/* Note: Debounce timer isn't under control of the command
>  	 * source registers, so no need to sync with the coprocessor
>  	 */
> -	addr = bank_reg(gpio, bank, reg_debounce_sel1);
> -	val = ioread32(addr);
> -	iowrite32((val & ~mask) | GPIO_SET_DEBOUNCE1(timer, offset), addr);
> -
> -	addr = bank_reg(gpio, bank, reg_debounce_sel2);
> -	val = ioread32(addr);
> -	iowrite32((val & ~mask) | GPIO_SET_DEBOUNCE2(timer, offset), addr);
> +	gpio->llops->reg_bits_set(gpio, offset, reg_debounce_sel1, !!(timer & BIT(1)));
> +	gpio->llops->reg_bits_set(gpio, offset, reg_debounce_sel2, !!(timer & BIT(0)));
>  }
>  
>  static int enable_debounce(struct gpio_chip *chip, unsigned int offset,
> @@ -866,15 +788,15 @@ static int enable_debounce(struct gpio_chip *chip, unsigned int offset,
>  	}
>  
>  	/* Try to find a timer already configured for the debounce period */
> -	for (i = 1; i < ARRAY_SIZE(debounce_timers); i++) {
> +	for (i = 1; i < gpio->debounce_timers_num; i++) {
>  		u32 cycles;
>  
> -		cycles = ioread32(gpio->base + debounce_timers[i]);
> +		cycles = ioread32(gpio->base + gpio->debounce_timers_array[i]);
>  		if (requested_cycles == cycles)
>  			break;
>  	}
>  
> -	if (i == ARRAY_SIZE(debounce_timers)) {
> +	if (i == gpio->debounce_timers_num) {
>  		int j;
>  
>  		/*
> @@ -905,7 +827,7 @@ static int enable_debounce(struct gpio_chip *chip, unsigned int offset,
>  
>  		i = j;
>  
> -		iowrite32(requested_cycles, gpio->base + debounce_timers[i]);
> +		iowrite32(requested_cycles, gpio->base + gpio->debounce_timers_array[i]);
>  	}
>  
>  	if (WARN(i == 0, "Cannot register index of disabled timer\n")) {
> @@ -1027,7 +949,7 @@ int aspeed_gpio_copro_grab_gpio(struct gpio_desc *desc,
>  
>  	/* Switch command source */
>  	if (gpio->cf_copro_bankmap[bindex] == 1)
> -		aspeed_gpio_change_cmd_source(gpio, bank, bindex,
> +		aspeed_gpio_change_cmd_source(gpio, offset,
>  					      GPIO_CMDSRC_COLDFIRE);
>  
>  	if (vreg_offset)
> @@ -1051,7 +973,6 @@ int aspeed_gpio_copro_release_gpio(struct gpio_desc *desc)
>  	struct gpio_chip *chip = gpiod_to_chip(desc);
>  	struct aspeed_gpio *gpio = gpiochip_get_data(chip);
>  	int rc = 0, bindex, offset = gpio_chip_hwgpio(desc);
> -	const struct aspeed_gpio_bank *bank = to_bank(offset);
>  	unsigned long flags;
>  
>  	if (!gpio->cf_copro_bankmap)
> @@ -1072,7 +993,7 @@ int aspeed_gpio_copro_release_gpio(struct gpio_desc *desc)
>  
>  	/* Switch command source */
>  	if (gpio->cf_copro_bankmap[bindex] == 0)
> -		aspeed_gpio_change_cmd_source(gpio, bank, bindex,
> +		aspeed_gpio_change_cmd_source(gpio, offset,
>  					      GPIO_CMDSRC_ARM);
>   bail:
>  	raw_spin_unlock_irqrestore(&gpio->lock, flags);
> @@ -1082,12 +1003,10 @@ EXPORT_SYMBOL_GPL(aspeed_gpio_copro_release_gpio);
>  
>  static void aspeed_gpio_irq_print_chip(struct irq_data *d, struct seq_file *p)
>  {
> -	const struct aspeed_gpio_bank *bank;
>  	struct aspeed_gpio *gpio;
> -	u32 bit;
>  	int rc, offset;
>  
> -	rc = irqd_to_aspeed_gpio_data(d, &gpio, &bank, &bit, &offset);
> +	rc = irqd_to_aspeed_gpio_data(d, &gpio, &offset);
>  	if (rc)
>  		return;
>  
> @@ -1120,7 +1039,7 @@ static const struct aspeed_bank_props ast2400_bank_props[] = {
>  
>  static const struct aspeed_gpio_config ast2400_config =
>  	/* 220 for simplicity, really 216 with two 4-GPIO holes, four at end */
> -	{ .nr_gpios = 220, .props = ast2400_bank_props, };
> +	{ .nr_gpios = 220, .props = ast2400_bank_props, .version = 4};
>  
>  static const struct aspeed_bank_props ast2500_bank_props[] = {
>  	/*     input	  output   */
> @@ -1132,7 +1051,7 @@ static const struct aspeed_bank_props ast2500_bank_props[] = {
>  
>  static const struct aspeed_gpio_config ast2500_config =
>  	/* 232 for simplicity, actual number is 228 (4-GPIO hole in GPIOAB) */
> -	{ .nr_gpios = 232, .props = ast2500_bank_props, };
> +	{ .nr_gpios = 232, .props = ast2500_bank_props, .version = 4};
>  
>  static const struct aspeed_bank_props ast2600_bank_props[] = {
>  	/*     input	  output   */
> @@ -1148,7 +1067,7 @@ static const struct aspeed_gpio_config ast2600_config =
>  	 * We expect ngpio being set in the device tree and this is a fallback
>  	 * option.
>  	 */
> -	{ .nr_gpios = 208, .props = ast2600_bank_props, };
> +	{ .nr_gpios = 208, .props = ast2600_bank_props, .version = 4};
>  
>  static const struct of_device_id aspeed_gpio_of_table[] = {
>  	{ .compatible = "aspeed,ast2400-gpio", .data = &ast2400_config, },
> @@ -1158,6 +1077,40 @@ static const struct of_device_id aspeed_gpio_of_table[] = {
>  };
>  MODULE_DEVICE_TABLE(of, aspeed_gpio_of_table);
>  
> +static void aspeed_g4_reg_bits_set(struct aspeed_gpio *gpio, unsigned int offset,
> +				   const enum aspeed_gpio_reg reg, u32 val)
> +{
> +	const struct aspeed_gpio_bank *bank = to_bank(offset);
> +	void __iomem *addr = bank_reg(gpio, bank, reg);
> +	u32 temp;
> +
> +	temp = ioread32(addr);
> +	if (val)
> +		temp |= GPIO_BIT(offset);
> +	else
> +		temp &= ~GPIO_BIT(offset);
> +
> +	iowrite32(temp, addr);
> +}
> +
> +static u32 aspeed_g4_reg_bits_read(struct aspeed_gpio *gpio, unsigned int offset,
> +				   const enum aspeed_gpio_reg reg)
> +{
> +	const struct aspeed_gpio_bank *bank = to_bank(offset);
> +	void __iomem *addr = bank_reg(gpio, bank, reg);
> +
> +	if (reg == reg_rdata)
> +		return ioread32(addr);
> +	return !!(ioread32(addr) & GPIO_BIT(offset));
> +}
> +
> +struct aspeed_gpio_llops aspeed_g4_llops = {
> +	.copro_request = aspeed_gpio_copro_request,
> +	.copro_release = aspeed_gpio_copro_release,
> +	.reg_bits_set = aspeed_g4_reg_bits_set,
> +	.reg_bits_read = aspeed_g4_reg_bits_read,
> +};
> +
>  static int __init aspeed_gpio_probe(struct platform_device *pdev)
>  {
>  	const struct of_device_id *gpio_id;
> @@ -1191,6 +1144,18 @@ static int __init aspeed_gpio_probe(struct platform_device *pdev)
>  
>  	gpio->config = gpio_id->data;
>  
> +	if (gpio->config->version == 4) {
> +		if (!gpio->llops)
> +			gpio->llops = &aspeed_g4_llops;
> +
> +		if (!gpio->debounce_timers_array) {
> +			gpio->debounce_timers_array = debounce_timers;
> +			gpio->debounce_timers_num = ARRAY_SIZE(debounce_timers);
> +		}

Why not embed the llops and debounce array pointer/size straight into
the config struct that we provide via  .data in the OF match table?

I think that would just mean reordering some of the function and struct
definitions in the source?

That way we can get rid of the version member.

Also, let's make sure here that the reg_bits_set and reg_bits_read
callbacks are not NULL (and error out of aspeed_gpio_probe() if they
are).

On the whole though, I feel this change turns out to be a decent
cleanup. It pushes the bank/offset bit-hackery down and away from the
call-sites.

Andrew

> +	} else {
> +		return -EOPNOTSUPP;
> +	}
> +
>  	gpio->chip.parent = &pdev->dev;
>  	err = of_property_read_u32(pdev->dev.of_node, "ngpios", &ngpio);
>  	gpio->chip.ngpio = (u16) ngpio;
> @@ -1218,15 +1183,17 @@ static int __init aspeed_gpio_probe(struct platform_device *pdev)
>  	 * Populate it with initial values read from the HW and switch
>  	 * all command sources to the ARM by default
>  	 */
> -	for (i = 0; i < banks; i++) {
> -		const struct aspeed_gpio_bank *bank = &aspeed_gpio_banks[i];
> -		void __iomem *addr = bank_reg(gpio, bank, reg_rdata);
> -		gpio->dcache[i] = ioread32(addr);
> -		aspeed_gpio_change_cmd_source(gpio, bank, 0, GPIO_CMDSRC_ARM);
> -		aspeed_gpio_change_cmd_source(gpio, bank, 1, GPIO_CMDSRC_ARM);
> -		aspeed_gpio_change_cmd_source(gpio, bank, 2, GPIO_CMDSRC_ARM);
> -		aspeed_gpio_change_cmd_source(gpio, bank, 3, GPIO_CMDSRC_ARM);
> -	}
> +	if (gpio->config->version == 4)
> +		for (i = 0; i < banks; i++) {
> +			const struct aspeed_gpio_bank *bank = &aspeed_gpio_banks[i];
> +			void __iomem *addr = bank_reg(gpio, bank, reg_rdata);
> +
> +			gpio->dcache[i] = ioread32(addr);
> +			aspeed_gpio_change_cmd_source(gpio, i * 8 + 0, GPIO_CMDSRC_ARM);
> +			aspeed_gpio_change_cmd_source(gpio, i * 8 + 8, GPIO_CMDSRC_ARM);
> +			aspeed_gpio_change_cmd_source(gpio, i * 8 + 16, GPIO_CMDSRC_ARM);
> +			aspeed_gpio_change_cmd_source(gpio, i * 8 + 24, GPIO_CMDSRC_ARM);
> +		}
>  
>  	/* Set up an irqchip */
>  	irq = platform_get_irq(pdev, 0);

Re: [PATCH v2 3/4] gpio: aspeed: Create llops to handle hardware access

[Andrew Jeffery]

On Fri, 2024-08-30 at 11:40 +0800, Billy Tsai wrote:
> Add low-level operations (llops) to abstract the register access for GPIO
> registers and the coprocessor request/release. With this abstraction
> layer, the driver can separate the hardware and software logic, making it
> easier to extend the driver to support different hardware register
> layouts.
> 
> Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
> ---
>  drivers/gpio/gpio-aspeed.c | 309 +++++++++++++++++--------------------
>  1 file changed, 138 insertions(+), 171 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-aspeed.c b/drivers/gpio/gpio-aspeed.c
> index 24f50a0ea4ab..74c4e80958bf 100644
> --- a/drivers/gpio/gpio-aspeed.c
> +++ b/drivers/gpio/gpio-aspeed.c
> @@ -39,6 +39,7 @@ struct aspeed_bank_props {
>  struct aspeed_gpio_config {
>  	unsigned int nr_gpios;
>  	const struct aspeed_bank_props *props;
> +	unsigned int version;

I'm interested to see whether this is necessary in practice.

>  };
>  
>  /*
> @@ -62,10 +63,13 @@ struct aspeed_gpio {
>  
>  	u8 *offset_timer;
>  	unsigned int timer_users[4];
> +	const int *debounce_timers_array;
> +	int debounce_timers_num;
>  	struct clk *clk;
>  
>  	u32 *dcache;
>  	u8 *cf_copro_bankmap;
> +	const struct aspeed_gpio_llops *llops;
>  };
>  
>  struct aspeed_gpio_bank {
> @@ -178,6 +182,15 @@ enum aspeed_gpio_reg {
>  	reg_cmdsrc1,
>  };
>  
> +struct aspeed_gpio_llops {
> +	bool (*copro_request)(struct aspeed_gpio *gpio, unsigned int offset);
> +	void (*copro_release)(struct aspeed_gpio *gpio, unsigned int offset);
> +	void (*reg_bits_set)(struct aspeed_gpio *gpio, unsigned int offset,
> +			     const enum aspeed_gpio_reg reg, u32 val);
> +	u32 (*reg_bits_read)(struct aspeed_gpio *gpio, unsigned int offset,
> +			     const enum aspeed_gpio_reg reg);
> +};
> +
>  #define GPIO_VAL_VALUE	0x00
>  #define GPIO_VAL_DIR	0x04
>  
> @@ -237,10 +250,6 @@ static inline void __iomem *bank_reg(struct aspeed_gpio *gpio,
>  #define GPIO_OFFSET(x)	((x) & 0x1f)
>  #define GPIO_BIT(x)	BIT(GPIO_OFFSET(x))
>  
> -#define _GPIO_SET_DEBOUNCE(t, o, i) ((!!((t) & BIT(i))) << GPIO_OFFSET(o))
> -#define GPIO_SET_DEBOUNCE1(t, o) _GPIO_SET_DEBOUNCE(t, o, 1)
> -#define GPIO_SET_DEBOUNCE2(t, o) _GPIO_SET_DEBOUNCE(t, o, 0)
> -
>  static const struct aspeed_gpio_bank *to_bank(unsigned int offset)
>  {
>  	unsigned int bank = GPIO_BANK(offset);
> @@ -296,35 +305,17 @@ static inline bool have_output(struct aspeed_gpio *gpio, unsigned int offset)
>  }
>  
>  static void aspeed_gpio_change_cmd_source(struct aspeed_gpio *gpio,
> -					  const struct aspeed_gpio_bank *bank,
> -					  int bindex, int cmdsrc)
> +					  unsigned int offset,
> +					  int cmdsrc)
>  {
> -	void __iomem *c0 = bank_reg(gpio, bank, reg_cmdsrc0);
> -	void __iomem *c1 = bank_reg(gpio, bank, reg_cmdsrc1);
> -	u32 bit, reg;
> -
>  	/*
> -	 * Each register controls 4 banks, so take the bottom 2
> -	 * bits of the bank index, and use them to select the
> -	 * right control bit (0, 8, 16 or 24).
> +	 * The command source register is only valid in bits 0, 8, 16, and 24, so we use
> +	 * (offset & ~(0x7)) to ensure that reg_bits_set always targets a valid bit.
>  	 */
> -	bit = BIT((bindex & 3) << 3);
> -
>  	/* Source 1 first to avoid illegal 11 combination */
> -	reg = ioread32(c1);
> -	if (cmdsrc & 2)
> -		reg |= bit;
> -	else
> -		reg &= ~bit;
> -	iowrite32(reg, c1);
> -
> +	gpio->llops->reg_bits_set(gpio, offset & ~(0x7), reg_cmdsrc1, !!(cmdsrc & BIT(1)));
>  	/* Then Source 0 */
> -	reg = ioread32(c0);
> -	if (cmdsrc & 1)
> -		reg |= bit;
> -	else
> -		reg &= ~bit;
> -	iowrite32(reg, c0);
> +	gpio->llops->reg_bits_set(gpio, offset & ~(0x7), reg_cmdsrc0, !!(cmdsrc & BIT(0)));
>  }
>  
>  static bool aspeed_gpio_copro_request(struct aspeed_gpio *gpio,
> @@ -343,7 +334,7 @@ static bool aspeed_gpio_copro_request(struct aspeed_gpio *gpio,
>  	copro_ops->request_access(copro_data);
>  
>  	/* Change command source back to ARM */
> -	aspeed_gpio_change_cmd_source(gpio, bank, offset >> 3, GPIO_CMDSRC_ARM);
> +	aspeed_gpio_change_cmd_source(gpio, offset, GPIO_CMDSRC_ARM);
>  
>  	/* Update cache */
>  	gpio->dcache[GPIO_BANK(offset)] = ioread32(bank_reg(gpio, bank, reg_rdata));
> @@ -354,8 +345,6 @@ static bool aspeed_gpio_copro_request(struct aspeed_gpio *gpio,
>  static void aspeed_gpio_copro_release(struct aspeed_gpio *gpio,
>  				      unsigned int offset)
>  {
> -	const struct aspeed_gpio_bank *bank = to_bank(offset);
> -
>  	if (!copro_ops || !gpio->cf_copro_bankmap)
>  		return;
>  	if (!gpio->cf_copro_bankmap[offset >> 3])
> @@ -364,7 +353,7 @@ static void aspeed_gpio_copro_release(struct aspeed_gpio *gpio,
>  		return;
>  
>  	/* Change command source back to ColdFire */
> -	aspeed_gpio_change_cmd_source(gpio, bank, offset >> 3,
> +	aspeed_gpio_change_cmd_source(gpio, offset,
>  				      GPIO_CMDSRC_COLDFIRE);
>  
>  	/* Restart the coprocessor */
> @@ -374,29 +363,24 @@ static void aspeed_gpio_copro_release(struct aspeed_gpio *gpio,
>  static int aspeed_gpio_get(struct gpio_chip *gc, unsigned int offset)
>  {
>  	struct aspeed_gpio *gpio = gpiochip_get_data(gc);
> -	const struct aspeed_gpio_bank *bank = to_bank(offset);
>  
> -	return !!(ioread32(bank_reg(gpio, bank, reg_val)) & GPIO_BIT(offset));
> +	return gpio->llops->reg_bits_read(gpio, offset, reg_val);
>  }
>  
>  static void __aspeed_gpio_set(struct gpio_chip *gc, unsigned int offset,
>  			      int val)
>  {
>  	struct aspeed_gpio *gpio = gpiochip_get_data(gc);
> -	const struct aspeed_gpio_bank *bank = to_bank(offset);
> -	void __iomem *addr;
>  	u32 reg;
>  
> -	addr = bank_reg(gpio, bank, reg_val);
>  	reg = gpio->dcache[GPIO_BANK(offset)];
> -
>  	if (val)
>  		reg |= GPIO_BIT(offset);
>  	else
>  		reg &= ~GPIO_BIT(offset);
>  	gpio->dcache[GPIO_BANK(offset)] = reg;
>  
> -	iowrite32(reg, addr);
> +	gpio->llops->reg_bits_set(gpio, offset, reg_val, val);
>  }
>  
>  static void aspeed_gpio_set(struct gpio_chip *gc, unsigned int offset,
> @@ -407,36 +391,32 @@ static void aspeed_gpio_set(struct gpio_chip *gc, unsigned int offset,
>  	bool copro;
>  
>  	raw_spin_lock_irqsave(&gpio->lock, flags);
> -	copro = aspeed_gpio_copro_request(gpio, offset);
> +	if (gpio->llops->copro_request)
> +		copro = gpio->llops->copro_request(gpio, offset);

A bit minor perhaps, but I'd add a static function to do the test and
call:

static bool aspeed_gpio_copro_request(struct aspeed_gpio *gpio,
unsigned int offset)
{
    if (gpio->llops->copro_request)
        return gpio->llops->copro_request(gpio, offset);

    return false;
}

static void aspeed_gpio_copro_release(struct aspeed_gpio *gpio,
unsigned int offset)
{
    if (gpio->llops->copro_release)
        gpio->llops->copro_release(gpio, offset);
}

A bit less noise at the call-sites that way.

That's only a suggestion for the optional copro function pointers
though. For the reg_bits_read and reg_bits_set callbacks we should
ensure they're not NULL in the driver probe() implementation, or error
out if they are.

>  
>  	__aspeed_gpio_set(gc, offset, val);
>  
> -	if (copro)
> -		aspeed_gpio_copro_release(gpio, offset);
> +	if (copro && gpio->llops->copro_release)
> +		gpio->llops->copro_release(gpio, offset);
>  	raw_spin_unlock_irqrestore(&gpio->lock, flags);
>  }
>  
>  static int aspeed_gpio_dir_in(struct gpio_chip *gc, unsigned int offset)
>  {
>  	struct aspeed_gpio *gpio = gpiochip_get_data(gc);
> -	const struct aspeed_gpio_bank *bank = to_bank(offset);
> -	void __iomem *addr = bank_reg(gpio, bank, reg_dir);
>  	unsigned long flags;
>  	bool copro;
> -	u32 reg;
>  
>  	if (!have_input(gpio, offset))
>  		return -ENOTSUPP;
>  
>  	raw_spin_lock_irqsave(&gpio->lock, flags);
>  
> -	reg = ioread32(addr);
> -	reg &= ~GPIO_BIT(offset);
> -
> -	copro = aspeed_gpio_copro_request(gpio, offset);
> -	iowrite32(reg, addr);
> -	if (copro)
> -		aspeed_gpio_copro_release(gpio, offset);
> +	if (gpio->llops->copro_request)
> +		copro = gpio->llops->copro_request(gpio, offset);
> +	gpio->llops->reg_bits_set(gpio, offset, reg_dir, 0);
> +	if (copro && gpio->llops->copro_release)
> +		gpio->llops->copro_release(gpio, offset);
>  
>  	raw_spin_unlock_irqrestore(&gpio->lock, flags);
>  
> @@ -447,26 +427,21 @@ static int aspeed_gpio_dir_out(struct gpio_chip *gc,
>  			       unsigned int offset, int val)
>  {
>  	struct aspeed_gpio *gpio = gpiochip_get_data(gc);
> -	const struct aspeed_gpio_bank *bank = to_bank(offset);
> -	void __iomem *addr = bank_reg(gpio, bank, reg_dir);
>  	unsigned long flags;
>  	bool copro;
> -	u32 reg;
>  
>  	if (!have_output(gpio, offset))
>  		return -ENOTSUPP;
>  
>  	raw_spin_lock_irqsave(&gpio->lock, flags);
>  
> -	reg = ioread32(addr);
> -	reg |= GPIO_BIT(offset);
> -
> -	copro = aspeed_gpio_copro_request(gpio, offset);
> +	if (gpio->llops->copro_request)
> +		copro = gpio->llops->copro_request(gpio, offset);
>  	__aspeed_gpio_set(gc, offset, val);
> -	iowrite32(reg, addr);
> +	gpio->llops->reg_bits_set(gpio, offset, reg_dir, 1);
>  
> -	if (copro)
> -		aspeed_gpio_copro_release(gpio, offset);
> +	if (copro && gpio->llops->copro_release)
> +		gpio->llops->copro_release(gpio, offset);
>  	raw_spin_unlock_irqrestore(&gpio->lock, flags);
>  
>  	return 0;
> @@ -475,7 +450,6 @@ static int aspeed_gpio_dir_out(struct gpio_chip *gc,
>  static int aspeed_gpio_get_direction(struct gpio_chip *gc, unsigned int offset)
>  {
>  	struct aspeed_gpio *gpio = gpiochip_get_data(gc);
> -	const struct aspeed_gpio_bank *bank = to_bank(offset);
>  	unsigned long flags;
>  	u32 val;
>  
> @@ -487,7 +461,7 @@ static int aspeed_gpio_get_direction(struct gpio_chip *gc, unsigned int offset)
>  
>  	raw_spin_lock_irqsave(&gpio->lock, flags);
>  
> -	val = ioread32(bank_reg(gpio, bank, reg_dir)) & GPIO_BIT(offset);
> +	val = gpio->llops->reg_bits_read(gpio, offset, reg_dir);
>  
>  	raw_spin_unlock_irqrestore(&gpio->lock, flags);
>  
> @@ -496,8 +470,7 @@ static int aspeed_gpio_get_direction(struct gpio_chip *gc, unsigned int offset)
>  
>  static inline int irqd_to_aspeed_gpio_data(struct irq_data *d,
>  					   struct aspeed_gpio **gpio,
> -					   const struct aspeed_gpio_bank **bank,
> -					   u32 *bit, int *offset)
> +					   int *offset)
>  {
>  	struct aspeed_gpio *internal;
>  
> @@ -510,70 +483,55 @@ static inline int irqd_to_aspeed_gpio_data(struct irq_data *d,
>  		return -ENOTSUPP;
>  
>  	*gpio = internal;
> -	*bank = to_bank(*offset);
> -	*bit = GPIO_BIT(*offset);
>  
>  	return 0;
>  }
>  
>  static void aspeed_gpio_irq_ack(struct irq_data *d)
>  {
> -	const struct aspeed_gpio_bank *bank;
>  	struct aspeed_gpio *gpio;
>  	unsigned long flags;
> -	void __iomem *status_addr;
>  	int rc, offset;
>  	bool copro;
> -	u32 bit;
>  
> -	rc = irqd_to_aspeed_gpio_data(d, &gpio, &bank, &bit, &offset);
> +	rc = irqd_to_aspeed_gpio_data(d, &gpio, &offset);
>  	if (rc)
>  		return;
>  
> -	status_addr = bank_reg(gpio, bank, reg_irq_status);
> -
>  	raw_spin_lock_irqsave(&gpio->lock, flags);
> -	copro = aspeed_gpio_copro_request(gpio, offset);
> +	if (gpio->llops->copro_request)
> +		copro = gpio->llops->copro_request(gpio, offset);
>  
> -	iowrite32(bit, status_addr);
> +	gpio->llops->reg_bits_set(gpio, offset, reg_irq_status, 1);
>  
> -	if (copro)
> -		aspeed_gpio_copro_release(gpio, offset);
> +	if (copro && gpio->llops->copro_release)
> +		gpio->llops->copro_release(gpio, offset);
>  	raw_spin_unlock_irqrestore(&gpio->lock, flags);
>  }
>  
>  static void aspeed_gpio_irq_set_mask(struct irq_data *d, bool set)
>  {
> -	const struct aspeed_gpio_bank *bank;
>  	struct aspeed_gpio *gpio;
>  	unsigned long flags;
> -	u32 reg, bit;
> -	void __iomem *addr;
>  	int rc, offset;
>  	bool copro;
>  
> -	rc = irqd_to_aspeed_gpio_data(d, &gpio, &bank, &bit, &offset);
> +	rc = irqd_to_aspeed_gpio_data(d, &gpio, &offset);
>  	if (rc)
>  		return;
>  
> -	addr = bank_reg(gpio, bank, reg_irq_enable);
> -
>  	/* Unmasking the IRQ */
>  	if (set)
>  		gpiochip_enable_irq(&gpio->chip, irqd_to_hwirq(d));
>  
>  	raw_spin_lock_irqsave(&gpio->lock, flags);
> -	copro = aspeed_gpio_copro_request(gpio, offset);
> +	if (gpio->llops->copro_request)
> +		copro = gpio->llops->copro_request(gpio, offset);
>  
> -	reg = ioread32(addr);
> -	if (set)
> -		reg |= bit;
> -	else
> -		reg &= ~bit;
> -	iowrite32(reg, addr);
> +	gpio->llops->reg_bits_set(gpio, offset, reg_irq_enable, set);
>  
> -	if (copro)
> -		aspeed_gpio_copro_release(gpio, offset);
> +	if (copro && gpio->llops->copro_release)
> +		gpio->llops->copro_release(gpio, offset);
>  	raw_spin_unlock_irqrestore(&gpio->lock, flags);
>  
>  	/* Masking the IRQ */
> @@ -596,34 +554,31 @@ static int aspeed_gpio_set_type(struct irq_data *d, unsigned int type)
>  	u32 type0 = 0;
>  	u32 type1 = 0;
>  	u32 type2 = 0;
> -	u32 bit, reg;
> -	const struct aspeed_gpio_bank *bank;
>  	irq_flow_handler_t handler;
>  	struct aspeed_gpio *gpio;
>  	unsigned long flags;
> -	void __iomem *addr;
>  	int rc, offset;
>  	bool copro;
>  
> -	rc = irqd_to_aspeed_gpio_data(d, &gpio, &bank, &bit, &offset);
> +	rc = irqd_to_aspeed_gpio_data(d, &gpio, &offset);
>  	if (rc)
>  		return -EINVAL;
>  
>  	switch (type & IRQ_TYPE_SENSE_MASK) {
>  	case IRQ_TYPE_EDGE_BOTH:
> -		type2 |= bit;
> +		type2 = 1;
>  		fallthrough;
>  	case IRQ_TYPE_EDGE_RISING:
> -		type0 |= bit;
> +		type0 = 1;
>  		fallthrough;
>  	case IRQ_TYPE_EDGE_FALLING:
>  		handler = handle_edge_irq;
>  		break;
>  	case IRQ_TYPE_LEVEL_HIGH:
> -		type0 |= bit;
> +		type0 = 1;
>  		fallthrough;
>  	case IRQ_TYPE_LEVEL_LOW:
> -		type1 |= bit;
> +		type1 = 1;
>  		handler = handle_level_irq;
>  		break;
>  	default:
> @@ -631,25 +586,15 @@ static int aspeed_gpio_set_type(struct irq_data *d, unsigned int type)
>  	}
>  
>  	raw_spin_lock_irqsave(&gpio->lock, flags);
> -	copro = aspeed_gpio_copro_request(gpio, offset);
> -
> -	addr = bank_reg(gpio, bank, reg_irq_type0);
> -	reg = ioread32(addr);
> -	reg = (reg & ~bit) | type0;
> -	iowrite32(reg, addr);
> +	if (gpio->llops->copro_request)
> +		copro = gpio->llops->copro_request(gpio, offset);
>  
> -	addr = bank_reg(gpio, bank, reg_irq_type1);
> -	reg = ioread32(addr);
> -	reg = (reg & ~bit) | type1;
> -	iowrite32(reg, addr);
> +	gpio->llops->reg_bits_set(gpio, offset, reg_irq_type0, type0);
> +	gpio->llops->reg_bits_set(gpio, offset, reg_irq_type1, type1);
> +	gpio->llops->reg_bits_set(gpio, offset, reg_irq_type2, type2);
>  
> -	addr = bank_reg(gpio, bank, reg_irq_type2);
> -	reg = ioread32(addr);
> -	reg = (reg & ~bit) | type2;
> -	iowrite32(reg, addr);
> -
> -	if (copro)
> -		aspeed_gpio_copro_release(gpio, offset);
> +	if (copro && gpio->llops->copro_release)
> +		gpio->llops->copro_release(gpio, offset);
>  	raw_spin_unlock_irqrestore(&gpio->lock, flags);
>  
>  	irq_set_handler_locked(d, handler);
> @@ -661,7 +606,6 @@ static void aspeed_gpio_irq_handler(struct irq_desc *desc)
>  {
>  	struct gpio_chip *gc = irq_desc_get_handler_data(desc);
>  	struct irq_chip *ic = irq_desc_get_chip(desc);
> -	struct aspeed_gpio *data = gpiochip_get_data(gc);
>  	unsigned int i, p, banks;
>  	unsigned long reg;
>  	struct aspeed_gpio *gpio = gpiochip_get_data(gc);
> @@ -670,9 +614,7 @@ static void aspeed_gpio_irq_handler(struct irq_desc *desc)
>  
>  	banks = DIV_ROUND_UP(gpio->chip.ngpio, 32);
>  	for (i = 0; i < banks; i++) {
> -		const struct aspeed_gpio_bank *bank = &aspeed_gpio_banks[i];
> -
> -		reg = ioread32(bank_reg(data, bank, reg_irq_status));
> +		reg = gpio->llops->reg_bits_read(gpio, i, reg_irq_status);
>  
>  		for_each_set_bit(p, &reg, 32)
>  			generic_handle_domain_irq(gc->irq.domain, i * 32 + p);
> @@ -711,26 +653,16 @@ static int aspeed_gpio_reset_tolerance(struct gpio_chip *chip,
>  {
>  	struct aspeed_gpio *gpio = gpiochip_get_data(chip);
>  	unsigned long flags;
> -	void __iomem *treg;
>  	bool copro;
> -	u32 val;
> -
> -	treg = bank_reg(gpio, to_bank(offset), reg_tolerance);
>  
>  	raw_spin_lock_irqsave(&gpio->lock, flags);
> -	copro = aspeed_gpio_copro_request(gpio, offset);
> +	if (gpio->llops->copro_request)
> +		copro = gpio->llops->copro_request(gpio, offset);
>  
> -	val = readl(treg);
> +	gpio->llops->reg_bits_set(gpio, offset, reg_tolerance, enable);
>  
> -	if (enable)
> -		val |= GPIO_BIT(offset);
> -	else
> -		val &= ~GPIO_BIT(offset);
> -
> -	writel(val, treg);
> -
> -	if (copro)
> -		aspeed_gpio_copro_release(gpio, offset);
> +	if (copro && gpio->llops->copro_release)
> +		gpio->llops->copro_release(gpio, offset);
>  	raw_spin_unlock_irqrestore(&gpio->lock, flags);
>  
>  	return 0;
> @@ -821,21 +753,11 @@ static inline bool timer_allocation_registered(struct aspeed_gpio *gpio,
>  static void configure_timer(struct aspeed_gpio *gpio, unsigned int offset,
>  		unsigned int timer)
>  {
> -	const struct aspeed_gpio_bank *bank = to_bank(offset);
> -	const u32 mask = GPIO_BIT(offset);
> -	void __iomem *addr;
> -	u32 val;
> -
>  	/* Note: Debounce timer isn't under control of the command
>  	 * source registers, so no need to sync with the coprocessor
>  	 */
> -	addr = bank_reg(gpio, bank, reg_debounce_sel1);
> -	val = ioread32(addr);
> -	iowrite32((val & ~mask) | GPIO_SET_DEBOUNCE1(timer, offset), addr);
> -
> -	addr = bank_reg(gpio, bank, reg_debounce_sel2);
> -	val = ioread32(addr);
> -	iowrite32((val & ~mask) | GPIO_SET_DEBOUNCE2(timer, offset), addr);
> +	gpio->llops->reg_bits_set(gpio, offset, reg_debounce_sel1, !!(timer & BIT(1)));
> +	gpio->llops->reg_bits_set(gpio, offset, reg_debounce_sel2, !!(timer & BIT(0)));
>  }
>  
>  static int enable_debounce(struct gpio_chip *chip, unsigned int offset,
> @@ -866,15 +788,15 @@ static int enable_debounce(struct gpio_chip *chip, unsigned int offset,
>  	}
>  
>  	/* Try to find a timer already configured for the debounce period */
> -	for (i = 1; i < ARRAY_SIZE(debounce_timers); i++) {
> +	for (i = 1; i < gpio->debounce_timers_num; i++) {
>  		u32 cycles;
>  
> -		cycles = ioread32(gpio->base + debounce_timers[i]);
> +		cycles = ioread32(gpio->base + gpio->debounce_timers_array[i]);
>  		if (requested_cycles == cycles)
>  			break;
>  	}
>  
> -	if (i == ARRAY_SIZE(debounce_timers)) {
> +	if (i == gpio->debounce_timers_num) {
>  		int j;
>  
>  		/*
> @@ -905,7 +827,7 @@ static int enable_debounce(struct gpio_chip *chip, unsigned int offset,
>  
>  		i = j;
>  
> -		iowrite32(requested_cycles, gpio->base + debounce_timers[i]);
> +		iowrite32(requested_cycles, gpio->base + gpio->debounce_timers_array[i]);
>  	}
>  
>  	if (WARN(i == 0, "Cannot register index of disabled timer\n")) {
> @@ -1027,7 +949,7 @@ int aspeed_gpio_copro_grab_gpio(struct gpio_desc *desc,
>  
>  	/* Switch command source */
>  	if (gpio->cf_copro_bankmap[bindex] == 1)
> -		aspeed_gpio_change_cmd_source(gpio, bank, bindex,
> +		aspeed_gpio_change_cmd_source(gpio, offset,
>  					      GPIO_CMDSRC_COLDFIRE);
>  
>  	if (vreg_offset)
> @@ -1051,7 +973,6 @@ int aspeed_gpio_copro_release_gpio(struct gpio_desc *desc)
>  	struct gpio_chip *chip = gpiod_to_chip(desc);
>  	struct aspeed_gpio *gpio = gpiochip_get_data(chip);
>  	int rc = 0, bindex, offset = gpio_chip_hwgpio(desc);
> -	const struct aspeed_gpio_bank *bank = to_bank(offset);
>  	unsigned long flags;
>  
>  	if (!gpio->cf_copro_bankmap)
> @@ -1072,7 +993,7 @@ int aspeed_gpio_copro_release_gpio(struct gpio_desc *desc)
>  
>  	/* Switch command source */
>  	if (gpio->cf_copro_bankmap[bindex] == 0)
> -		aspeed_gpio_change_cmd_source(gpio, bank, bindex,
> +		aspeed_gpio_change_cmd_source(gpio, offset,
>  					      GPIO_CMDSRC_ARM);
>   bail:
>  	raw_spin_unlock_irqrestore(&gpio->lock, flags);
> @@ -1082,12 +1003,10 @@ EXPORT_SYMBOL_GPL(aspeed_gpio_copro_release_gpio);
>  
>  static void aspeed_gpio_irq_print_chip(struct irq_data *d, struct seq_file *p)
>  {
> -	const struct aspeed_gpio_bank *bank;
>  	struct aspeed_gpio *gpio;
> -	u32 bit;
>  	int rc, offset;
>  
> -	rc = irqd_to_aspeed_gpio_data(d, &gpio, &bank, &bit, &offset);
> +	rc = irqd_to_aspeed_gpio_data(d, &gpio, &offset);
>  	if (rc)
>  		return;
>  
> @@ -1120,7 +1039,7 @@ static const struct aspeed_bank_props ast2400_bank_props[] = {
>  
>  static const struct aspeed_gpio_config ast2400_config =
>  	/* 220 for simplicity, really 216 with two 4-GPIO holes, four at end */
> -	{ .nr_gpios = 220, .props = ast2400_bank_props, };
> +	{ .nr_gpios = 220, .props = ast2400_bank_props, .version = 4};
>  
>  static const struct aspeed_bank_props ast2500_bank_props[] = {
>  	/*     input	  output   */
> @@ -1132,7 +1051,7 @@ static const struct aspeed_bank_props ast2500_bank_props[] = {
>  
>  static const struct aspeed_gpio_config ast2500_config =
>  	/* 232 for simplicity, actual number is 228 (4-GPIO hole in GPIOAB) */
> -	{ .nr_gpios = 232, .props = ast2500_bank_props, };
> +	{ .nr_gpios = 232, .props = ast2500_bank_props, .version = 4};
>  
>  static const struct aspeed_bank_props ast2600_bank_props[] = {
>  	/*     input	  output   */
> @@ -1148,7 +1067,7 @@ static const struct aspeed_gpio_config ast2600_config =
>  	 * We expect ngpio being set in the device tree and this is a fallback
>  	 * option.
>  	 */
> -	{ .nr_gpios = 208, .props = ast2600_bank_props, };
> +	{ .nr_gpios = 208, .props = ast2600_bank_props, .version = 4};
>  
>  static const struct of_device_id aspeed_gpio_of_table[] = {
>  	{ .compatible = "aspeed,ast2400-gpio", .data = &ast2400_config, },
> @@ -1158,6 +1077,40 @@ static const struct of_device_id aspeed_gpio_of_table[] = {
>  };
>  MODULE_DEVICE_TABLE(of, aspeed_gpio_of_table);
>  
> +static void aspeed_g4_reg_bits_set(struct aspeed_gpio *gpio, unsigned int offset,
> +				   const enum aspeed_gpio_reg reg, u32 val)
> +{
> +	const struct aspeed_gpio_bank *bank = to_bank(offset);
> +	void __iomem *addr = bank_reg(gpio, bank, reg);
> +	u32 temp;
> +
> +	temp = ioread32(addr);
> +	if (val)
> +		temp |= GPIO_BIT(offset);
> +	else
> +		temp &= ~GPIO_BIT(offset);
> +
> +	iowrite32(temp, addr);
> +}
> +
> +static u32 aspeed_g4_reg_bits_read(struct aspeed_gpio *gpio, unsigned int offset,
> +				   const enum aspeed_gpio_reg reg)
> +{
> +	const struct aspeed_gpio_bank *bank = to_bank(offset);
> +	void __iomem *addr = bank_reg(gpio, bank, reg);
> +
> +	if (reg == reg_rdata)
> +		return ioread32(addr);
> +	return !!(ioread32(addr) & GPIO_BIT(offset));
> +}
> +
> +struct aspeed_gpio_llops aspeed_g4_llops = {
> +	.copro_request = aspeed_gpio_copro_request,
> +	.copro_release = aspeed_gpio_copro_release,
> +	.reg_bits_set = aspeed_g4_reg_bits_set,
> +	.reg_bits_read = aspeed_g4_reg_bits_read,
> +};
> +
>  static int __init aspeed_gpio_probe(struct platform_device *pdev)
>  {
>  	const struct of_device_id *gpio_id;
> @@ -1191,6 +1144,18 @@ static int __init aspeed_gpio_probe(struct platform_device *pdev)
>  
>  	gpio->config = gpio_id->data;
>  
> +	if (gpio->config->version == 4) {
> +		if (!gpio->llops)
> +			gpio->llops = &aspeed_g4_llops;
> +
> +		if (!gpio->debounce_timers_array) {
> +			gpio->debounce_timers_array = debounce_timers;
> +			gpio->debounce_timers_num = ARRAY_SIZE(debounce_timers);
> +		}

Why not embed the llops and debounce array pointer/size straight into
the config struct that we provide via  .data in the OF match table?

I think that would just mean reordering some of the function and struct
definitions in the source?

That way we can get rid of the version member.

Also, let's make sure here that the reg_bits_set and reg_bits_read
callbacks are not NULL (and error out of aspeed_gpio_probe() if they
are).

On the whole though, I feel this change turns out to be a decent
cleanup. It pushes the bank/offset bit-hackery down and away from the
call-sites.

Andrew

> +	} else {
> +		return -EOPNOTSUPP;
> +	}
> +
>  	gpio->chip.parent = &pdev->dev;
>  	err = of_property_read_u32(pdev->dev.of_node, "ngpios", &ngpio);
>  	gpio->chip.ngpio = (u16) ngpio;
> @@ -1218,15 +1183,17 @@ static int __init aspeed_gpio_probe(struct platform_device *pdev)
>  	 * Populate it with initial values read from the HW and switch
>  	 * all command sources to the ARM by default
>  	 */
> -	for (i = 0; i < banks; i++) {
> -		const struct aspeed_gpio_bank *bank = &aspeed_gpio_banks[i];
> -		void __iomem *addr = bank_reg(gpio, bank, reg_rdata);
> -		gpio->dcache[i] = ioread32(addr);
> -		aspeed_gpio_change_cmd_source(gpio, bank, 0, GPIO_CMDSRC_ARM);
> -		aspeed_gpio_change_cmd_source(gpio, bank, 1, GPIO_CMDSRC_ARM);
> -		aspeed_gpio_change_cmd_source(gpio, bank, 2, GPIO_CMDSRC_ARM);
> -		aspeed_gpio_change_cmd_source(gpio, bank, 3, GPIO_CMDSRC_ARM);
> -	}
> +	if (gpio->config->version == 4)
> +		for (i = 0; i < banks; i++) {
> +			const struct aspeed_gpio_bank *bank = &aspeed_gpio_banks[i];
> +			void __iomem *addr = bank_reg(gpio, bank, reg_rdata);
> +
> +			gpio->dcache[i] = ioread32(addr);
> +			aspeed_gpio_change_cmd_source(gpio, i * 8 + 0, GPIO_CMDSRC_ARM);
> +			aspeed_gpio_change_cmd_source(gpio, i * 8 + 8, GPIO_CMDSRC_ARM);
> +			aspeed_gpio_change_cmd_source(gpio, i * 8 + 16, GPIO_CMDSRC_ARM);
> +			aspeed_gpio_change_cmd_source(gpio, i * 8 + 24, GPIO_CMDSRC_ARM);
> +		}
>  
>  	/* Set up an irqchip */
>  	irq = platform_get_irq(pdev, 0);