Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
Date:   Thu, 17 Nov 2022 17:18:55 +0100
From:   Michael Walle <michael@walle.cc>
To:     William Breathitt Gray <william.gray@linaro.org>
Cc:     linus.walleij@linaro.org, brgl@bgdev.pl,
        andriy.shevchenko@linux.intel.com, linux-gpio@vger.kernel.org,
        linux-kernel@vger.kernel.org, broonie@kernel.org
Subject: Re: [PATCH v2 4/4] gpio: i8255: Migrate to regmap API
In-Reply-To: <61327a67cc308af413471a69a4810b2785e53e8e.1668129763.git.william.gray@linaro.org>
References: <cover.1668129763.git.william.gray@linaro.org>
 <61327a67cc308af413471a69a4810b2785e53e8e.1668129763.git.william.gray@linaro.org>
User-Agent: Roundcube Webmail/1.4.13
Message-ID: <5123090e11da67e57fb00984445ece2f@walle.cc>
Content-Type: text/plain; charset=US-ASCII;
 format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi,

Am 2022-11-11 02:55, schrieb William Breathitt Gray:

> +    config.map = devm_regmap_init_mmio(dev, regs, 
> &gpiomm_regmap_config);
> +    if (IS_ERR(config.map))
> +        return PTR_ERR(config.map);

I've just skimmed over your patch and noticed you're using an mmio
regmap. Please note that for now, gpio-regmap unconditionally sets
.can_sleep to true in the gpiochip [1]. So the users would need to
use the _cansleep() variants. See a proposal below.

> +int devm_i8255_regmap_register(struct device *const dev,
> +			       const struct i8255_regmap_config *const config)
> +{
> +	struct gpio_regmap_config gpio_config = {0};
> +	unsigned long i;
> +	int err;
> +
> +	if (!config->parent)
> +		return -EINVAL;
> +
> +	if (!config->map)
> +		return -EINVAL;
> +
> +	if (!config->num_ppi)
> +		return -EINVAL;
> +
> +	for (i = 0; i < config->num_ppi; i++) {
> +		err = i8255_ppi_init(config->map, i * 4);
> +		if (err)
> +			return err;
> +	}
> +
> +	gpio_config.parent = config->parent;
> +	gpio_config.regmap = config->map;

I'd propose to add a new config flag to indicate that accesses to
the device will be fast:

gpio_config.regmap_has_fast_io = true;

which will then set gpio->can_sleep = false.

-michael

> +	gpio_config.ngpio = I8255_NGPIO * config->num_ppi;
> +	gpio_config.names = config->names;
> +	gpio_config.reg_dat_base = GPIO_REGMAP_ADDR(I8255_REG_DAT_BASE);
> +	gpio_config.reg_set_base = GPIO_REGMAP_ADDR(I8255_REG_DAT_BASE);
> +	gpio_config.reg_dir_in_base = 
> GPIO_REGMAP_ADDR(I8255_REG_DIR_IN_BASE);
> +	gpio_config.ngpio_per_reg = I8255_NGPIO_PER_REG;
> +	gpio_config.irq_domain = config->domain;
> +	gpio_config.reg_mask_xlate = i8255_reg_mask_xlate;
> +
> +	return PTR_ERR_OR_ZERO(devm_gpio_regmap_register(dev, &gpio_config));
> +}
> +EXPORT_SYMBOL_NS_GPL(devm_i8255_regmap_register, I8255);

[1] 
https://elixir.bootlin.com/linux/v6.1-rc5/source/drivers/gpio/gpio-regmap.c#L260