Re: [PATCH v6 1/4] gpiolib: Pass bitmaps, not integer arrays, to get/set array
From: Peter Rosin <hidden>
Date: 2018-09-01 00:23:47
Also in:
linux-doc, linux-gpio, linux-i2c, linux-iio, linux-mmc, linux-serial, lkml
On 2018-09-01 00:56, Janusz Krzysztofik wrote:
Most users of get/set array functions iterate consecutive bits of data, usually a single integer, while processing array of results obtained from, or building an array of values to be passed to those functions. Save time wasted on those iterations by changing the functions' API to accept bitmaps. All current users are updated as well. More benefits from the change are expected as soon as planned support for accepting/passing those bitmaps directly from/to respective GPIO chip callbacks if applicable is implemented.
quoted hunk ↗ jump to hunk
diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c index 401308e3d036..e28ddc20000d 100644 --- a/drivers/i2c/muxes/i2c-mux-gpio.c +++ b/drivers/i2c/muxes/i2c-mux-gpio.c@@ -22,18 +22,16 @@ struct gpiomux { struct i2c_mux_gpio_platform_data data; unsigned gpio_base; struct gpio_desc **gpios; - int *values; }; static void i2c_mux_gpio_set(const struct gpiomux *mux, unsigned val) { - int i; - - for (i = 0; i < mux->data.n_gpios; i++) - mux->values[i] = (val >> i) & 1; + DECLARE_BITMAP(value_bitmap, mux->data.n_gpios);
Picking a random driver for this comment, it applies to many of them. I think this creates a VLA? Can't you, for the bit-count, just go with BITS_PER_TYPE(unsigned)? Or whatever is appropriate for the driver in question. Also, I find that where you use DECLARE_BITMAP, the _bitmap suffix is just noise and I would very much like to zap it. Cheers, Peter
quoted hunk ↗ jump to hunk
+ + *value_bitmap = val; gpiod_set_array_value_cansleep(mux->data.n_gpios, - mux->gpios, mux->values); + mux->gpios, value_bitmap); } static int i2c_mux_gpio_select(struct i2c_mux_core *muxc, u32 chan)@@ -182,15 +180,13 @@ static int i2c_mux_gpio_probe(struct platform_device *pdev) return -EPROBE_DEFER; muxc = i2c_mux_alloc(parent, &pdev->dev, mux->data.n_values, - mux->data.n_gpios * sizeof(*mux->gpios) + - mux->data.n_gpios * sizeof(*mux->values), 0, + mux->data.n_gpios * sizeof(*mux->gpios), 0, i2c_mux_gpio_select, NULL); if (!muxc) { ret = -ENOMEM; goto alloc_failed; } mux->gpios = muxc->priv; - mux->values = (int *)(mux->gpios + mux->data.n_gpios); muxc->priv = mux; platform_set_drvdata(pdev, muxc);