All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andy Shevchenko <andriy.shevchenko@intel.com>
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Thomas Fourier <fourier.thomas@gmail.com>,
	Andy Shevchenko <andy@kernel.org>,
	Linus Walleij <linus.walleij@stericsson.com>,
	Russell King <rmk+kernel@arm.linux.org.uk>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] ARM: 6214/2: fix release_mem_region() size
Date: Sat, 27 Dec 2025 18:01:05 +0200	[thread overview]
Message-ID: <aVACwVDQP0MJvHVP@smile.fi.intel.com> (raw)
In-Reply-To: <CAMuHMdVquNEoxRQkcBEH0nC+CDuib6o0H6m3CBk3ZN2267LpQw@mail.gmail.com>

On Wed, Dec 17, 2025 at 08:28:27AM +0100, Geert Uytterhoeven wrote:
> Hi Thomas,
> 
> Thanks for your patch!
> 
> The patch prefix should be "auxdisplay: arm-charlcd:", not an rmk
> patch tracker ID.
> 
> On Tue, 16 Dec 2025 at 18:49, Thomas Fourier <fourier.thomas@gmail.com> wrote:
> > It seems like, after the request_mem_region(), the corresponding
> > release_mem_region() must take the same size. This was done
> > in charlcd_remove() but not in the error path in charlcd_probe().
> 
> Unfortunately charlcd_remove() was removed in one of these silly
> "make <foo> explicitly non-modular" patches...
> 
> > Fixes: ce8962455e90 ("ARM: 6214/2: driver for the character LCD found in ARM refdesigns")
> > Signed-off-by: Thomas Fourier <fourier.thomas@gmail.com>
> 
> > --- a/drivers/auxdisplay/arm-charlcd.c
> > +++ b/drivers/auxdisplay/arm-charlcd.c
> > @@ -323,7 +323,7 @@ static int __init charlcd_probe(struct platform_device *pdev)
> >  out_no_irq:
> >         iounmap(lcd->virtbase);
> >  out_no_memregion:
> 
> Looks like the first "goto out_no_memregion" is incorrect, and should be
> "goto out_no_resource".

Should it be in this change or in a separate change? I'm not sure I got what
should be done about this.

> > -       release_mem_region(lcd->phybase, SZ_4K);
> > +       release_mem_region(lcd->phybase, lcd->physize);
> >  out_no_resource:
> >         kfree(lcd);
> >         return ret;
> 
> The actual change LGTM to me, so
> Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>

Reading between the lines, should I alter the commit message to mention that
there is no more charlcd_remove()? Something like:

    It seems like, after the request_mem_region(), the corresponding
    release_mem_region() must take the same size. This was done
    in recently dropped charlcd_remove() but not in the error path
    in charlcd_probe().

(and Subject line)?

-- 
With Best Regards,
Andy Shevchenko



  reply	other threads:[~2025-12-27 16:01 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-16 17:47 [PATCH] ARM: 6214/2: fix release_mem_region() size Thomas Fourier
2025-12-17  7:28 ` Geert Uytterhoeven
2025-12-27 16:01   ` Andy Shevchenko [this message]
2025-12-27 18:39     ` Geert Uytterhoeven
2025-12-29 13:08 ` Andy Shevchenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aVACwVDQP0MJvHVP@smile.fi.intel.com \
    --to=andriy.shevchenko@intel.com \
    --cc=andy@kernel.org \
    --cc=fourier.thomas@gmail.com \
    --cc=geert@linux-m68k.org \
    --cc=linus.walleij@stericsson.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rmk+kernel@arm.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.