Add GPIO_ACCESS_BY_PORT_MASK mode

[qianfan-Zhao]

We had a discuess on issue #10305, this PR is the implements.

[qianfan-Zhao]

@b0661 @carlescufi Could you please review this PR?

[codecov-io]

Codecov Report

Merging #10741 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master   #10741   +/-   ##
=======================================
  Coverage   53.26%   53.26%           
=======================================
  Files         215      215           
  Lines       25827    25827           
  Branches     5695     5695           
=======================================
  Hits        13758    13758           
  Misses       9753     9753           
  Partials     2316     2316

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c229126...6180703. Read the comment docs.

[b0661]

LGTM. Thank you for adding the masked functions.

[qianfan-Zhao]

On my custom board, there has 3 leds connect to GPIOA.17, GPIOA.18, GPIOA.22.
Set gpio LOW can turn on the led, and HIGH can turn off.

Tested this pr with below examples:

#define PINS				(BIT(17) | BIT(18) | BIT(22))

static bool port_write_masked_test(void)
{
	struct device *gpioa = device_get_binding(CONFIG_GPIO_SAM_PORTA_LABEL);
	const int values[3] = {
		BIT(17) | BIT(18),
		BIT(17) | BIT(22),
		BIT(18) | BIT(22)
	};

	gpio_port_configure_masked(gpioa, PINS, GPIO_DIR_OUT);

	while (1) {
		for (int i = 0; i < ARRAY_SIZE(values); i++) {
			gpio_port_write_masked(gpioa, PINS, ~values[i]); /* Turn on */
			k_sleep(1000);
			gpio_port_write_masked(gpioa, PINS, ~0); /* Turn off */
			k_sleep(1000);
		}
	}
}

[pabigot]

I don't like this API extension, but it was discussed before I came on board, so I don't feel that it's fair for me to reject it. I do have a couple thoughts:

The whole concept of configuring multiple pins to one configuration seems a mistaken generalization of the API to configure a single pin to a configuration. For multi-pin synchronous configuration I prefer to pass an array of configuration flags, and a range of GPIO ordinals that cover the pins to be configured. In work blocked by #10880 I have the following API:

/** @brief Configure multiple pins in one I2C transaction.
 *
 * @param dev Pointer to the device structure for the driver instance.
 *
 * @param p0 Pin number corresponding to configs[0].
 *
 * @param configs Sequence of pin configuration values as with
 * `gpio_pin_configure`.  A negative value indicates the corresponding
 * pin should not be reconfigured.
 *
 * @param count the number of value in the `configs` sequence.
 *
 * @retval zero on success, or a negative error code.  If error the
 * previous pin configuration is unchanged. */
int gpio_sx1509b_multiconfigure(struct device *dev,
                               unsigned int p0,
                               const int *configs,
                               size_t count);

This API is designed to support configuring all the pins for an application on startup. It could also be used before entering a deep-sleep mode to put all pins into the state required for lowest-power a device.

For synchronized setting and clearing of output signals I can't argue strongly against the masked value approach, but the API I have in the same work is the following, which adds the ability to invert a set of pins (assuming that the driver is able to determine their current value internally).

/** @brief Multi-pin set, clear, and toggle API.
 *
 * If a bit is set in both @p set and @p clear its current value is
 * toggled.  If no bits are set in @p set or @p clear the current IOX
 * output configuration is returned from cache.
 *
 * @param dev Pointer to the device structure for the driver instance.
 *
 * @param set bits identifying IOX outputs that should be high.
 *
 * @param clear bits identifying IOX outputs that should be low.
 *
 * @return a non-negative value indicates success and provides the
 * value of the SX1509B data register.  A negative value indicates a
 * failure to configure the outputs. */
int gpio_sx1509b_output_sct(struct device *dev,
                           u16_t set,
                           u16_t clear);

If people think either of these have merit, then they might be an option instead of the API implemented in this PR. In that case the masks would have to be updated to u32_t values (the SX1509B has 16 pins).

Finally, if the primary driver for this is to be able to synchronously change LED state I would prefer that be done through the LED API, specifically by changing #9511 to support a sequence of GPIO-controlled LEDs rather than a single LED for each driver instance, and extending the API to support set/clear/toggle on multiple LEDs.

[mnkp]

I believe this is not the right approach. As reported in #11917 current GPIO API is too slow. The major contributor to this state of matter is GPIO_ACCESS_BY_* operator. We need to do a major redesign of GPIO API to get rid of it.

I propose to start the work on the issue after 1.14 is released as now we need to focus on bug fixes.

[galak]

Closing as we deprecated the PORT apis and are working on an update GPIO interface. So please comment on the issue/PRs that @mknp referenced.