[PATCH v3 1/1] mtd: cfi_cmdset_0001: Factor out do_write_buffer_locked() to reduce stack frame
David Laight
david.laight.linux at gmail.com
Thu Apr 9 00:26:11 PDT 2026
On Wed, 8 Apr 2026 23:11:48 +0200
Andy Shevchenko <andriy.shevchenko at linux.intel.com> wrote:
> Compiler is not happy about used stack frame:
>
> drivers/mtd/chips/cfi_cmdset_0001.c: In function 'do_write_buffer':
> drivers/mtd/chips/cfi_cmdset_0001.c:1887:1: error: the frame size of 1296 bytes is larger than 1280 bytes [-Werror=frame-larger-than=]
>
> Fix this by factoring out do_write_buffer_locked().
Does this just split the large stack frame between two nested functions?
I'd also expect the compiler to inline do_write_buffer_locked() so it
makes little difference.
OTOH I can't immediately see where the large stack frame comes from.
David
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko at linux.intel.com>
> ---
> v3: addressed set but unused variables when MTD_XIP=y (LKP)
> v2: kept DIS/ENABLE_VPP paired
>
> drivers/mtd/chips/cfi_cmdset_0001.c | 88 +++++++++++++++++------------
> 1 file changed, 51 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/mtd/chips/cfi_cmdset_0001.c b/drivers/mtd/chips/cfi_cmdset_0001.c
> index 5a4d2e16a9d1..7733e076ad40 100644
> --- a/drivers/mtd/chips/cfi_cmdset_0001.c
> +++ b/drivers/mtd/chips/cfi_cmdset_0001.c
> @@ -1154,7 +1154,8 @@ static void __xipram xip_enable(struct map_info *map, struct flchip *chip,
>
> static int __xipram xip_wait_for_operation(
> struct map_info *map, struct flchip *chip,
> - unsigned long adr, unsigned int chip_op_time_max)
> + unsigned long adr, unsigned long inval_adr, int inval_len,
> + unsigned int chip_op_time, unsigned int chip_op_time_max)
> {
> struct cfi_private *cfi = map->fldrv_priv;
> struct cfi_pri_intelext *cfip = cfi->cmdset_priv;
> @@ -1276,8 +1277,7 @@ static int __xipram xip_wait_for_operation(
> #define XIP_INVAL_CACHED_RANGE(map, from, size) \
> INVALIDATE_CACHED_RANGE(map, from, size)
>
> -#define INVAL_CACHE_AND_WAIT(map, chip, cmd_adr, inval_adr, inval_len, usec, usec_max) \
> - xip_wait_for_operation(map, chip, cmd_adr, usec_max)
> +#define INVAL_CACHE_AND_WAIT xip_wait_for_operation
Isn't that separate and unrelated?
The compiler might optimise away the parameters you are adding back.
David
>
> #else
>
> @@ -1720,42 +1720,24 @@ static int cfi_intelext_write_words (struct mtd_info *mtd, loff_t to , size_t le
> }
>
>
> -static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip,
> - unsigned long adr, const struct kvec **pvec,
> - unsigned long *pvec_seek, int len)
> +static int __xipram do_write_buffer_locked(struct map_info *map, struct flchip *chip,
> + unsigned long cmd_adr, unsigned long adr,
> + const struct kvec **pvec,
> + unsigned long *pvec_seek, int len)
> {
> struct cfi_private *cfi = map->fldrv_priv;
> map_word status, write_cmd, datum;
> - unsigned long cmd_adr;
> - int ret, wbufsize, word_gap, words;
> + int ret, word_gap, words;
> const struct kvec *vec;
> unsigned long vec_seek;
> unsigned long initial_adr;
> int initial_len = len;
>
> - wbufsize = cfi_interleave(cfi) << cfi->cfiq->MaxBufWriteSize;
> - adr += chip->start;
> initial_adr = adr;
> - cmd_adr = adr & ~(wbufsize-1);
> -
> - /* Sharp LH28F640BF chips need the first address for the
> - * Page Buffer Program command. See Table 5 of
> - * LH28F320BF, LH28F640BF, LH28F128BF Series (Appendix FUM00701) */
> - if (is_LH28F640BF(cfi))
> - cmd_adr = adr;
>
> /* Let's determine this according to the interleave only once */
> write_cmd = (cfi->cfiq->P_ID != P_ID_INTEL_PERFORMANCE) ? CMD(0xe8) : CMD(0xe9);
>
> - mutex_lock(&chip->mutex);
> - ret = get_chip(map, chip, cmd_adr, FL_WRITING);
> - if (ret) {
> - mutex_unlock(&chip->mutex);
> - return ret;
> - }
> -
> - XIP_INVAL_CACHED_RANGE(map, initial_adr, initial_len);
> - ENABLE_VPP(map);
> xip_disable(map, chip, cmd_adr);
>
> /* §4.8 of the 28FxxxJ3A datasheet says "Any time SR.4 and/or SR.5 is set
> @@ -1789,7 +1771,7 @@ static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip,
> xip_enable(map, chip, cmd_adr);
> printk(KERN_ERR "%s: Chip not ready for buffer write. Xstatus = %lx, status = %lx\n",
> map->name, Xstatus.x[0], status.x[0]);
> - goto out;
> + return ret;
> }
>
> /* Figure out the number of words to write */
> @@ -1853,7 +1835,7 @@ static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip,
> chip->state = FL_STATUS;
> xip_enable(map, chip, cmd_adr);
> printk(KERN_ERR "%s: buffer write error (status timeout)\n", map->name);
> - goto out;
> + return ret;
> }
>
> /* check for errors */
> @@ -1866,21 +1848,53 @@ static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip,
> map_write(map, CMD(0x70), cmd_adr);
> xip_enable(map, chip, cmd_adr);
>
> - if (chipstatus & 0x02) {
> - ret = -EROFS;
> - } else if (chipstatus & 0x08) {
> + if (chipstatus & 0x02)
> + return -EROFS;
> +
> + if (chipstatus & 0x08) {
> printk(KERN_ERR "%s: buffer write error (bad VPP)\n", map->name);
> - ret = -EIO;
> - } else {
> - printk(KERN_ERR "%s: buffer write error (status 0x%lx)\n", map->name, chipstatus);
> - ret = -EINVAL;
> + return -EIO;
> }
>
> - goto out;
> + printk(KERN_ERR "%s: buffer write error (status 0x%lx)\n", map->name, chipstatus);
> + return -EINVAL;
> }
>
> xip_enable(map, chip, cmd_adr);
> - out: DISABLE_VPP(map);
> + return 0;
> +}
> +
> +static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip,
> + unsigned long adr, const struct kvec **pvec,
> + unsigned long *pvec_seek, int len)
> +{
> + struct cfi_private *cfi = map->fldrv_priv;
> + unsigned long cmd_adr;
> + int ret, wbufsize;
> +
> + wbufsize = cfi_interleave(cfi) << cfi->cfiq->MaxBufWriteSize;
> + adr += chip->start;
> + cmd_adr = adr & ~(wbufsize - 1);
> +
> + /* Sharp LH28F640BF chips need the first address for the
> + * Page Buffer Program command. See Table 5 of
> + * LH28F320BF, LH28F640BF, LH28F128BF Series (Appendix FUM00701) */
> + if (is_LH28F640BF(cfi))
> + cmd_adr = adr;
> +
> + mutex_lock(&chip->mutex);
> + ret = get_chip(map, chip, cmd_adr, FL_WRITING);
> + if (ret) {
> + mutex_unlock(&chip->mutex);
> + return ret;
> + }
> +
> + XIP_INVAL_CACHED_RANGE(map, adr, len);
> + ENABLE_VPP(map);
> +
> + ret = do_write_buffer_locked(map, chip, cmd_adr, adr, pvec, pvec_seek, len);
> +
> + DISABLE_VPP(map);
> put_chip(map, chip, cmd_adr);
> mutex_unlock(&chip->mutex);
> return ret;
More information about the linux-mtd
mailing list