All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
	Michael Ellerman <mpe@ellerman.id.au>,
	Nicholas Piggin <npiggin@gmail.com>,
	Naveen N Rao <naveen@kernel.org>,
	Madhavan Srinivasan <maddy@linux.ibm.com>
Subject: Re: [PATCH v1 1/1] powerpc/8xx: Drop legacy-of-mm-gpiochip.h header
Date: Mon, 18 Nov 2024 14:32:51 +0200	[thread overview]
Message-ID: <Zzsz81-quzIZ_13L@smile.fi.intel.com> (raw)
In-Reply-To: <124ab5ab-3bfd-4ad7-a75d-981da9c03423@csgroup.eu>

On Mon, Nov 18, 2024 at 12:02:44PM +0100, Christophe Leroy wrote:
> Le 18/11/2024 à 10:20, Andy Shevchenko a écrit :
> > On Sat, Nov 16, 2024 at 11:44:35AM +0100, Christophe Leroy wrote:
> > > Le 15/11/2024 à 14:38, Andy Shevchenko a écrit :
> > > > Remove legacy-of-mm-gpiochip.h header file, replace of_* functions
> > > > and structs with appropriate alternatives.
> > > 
> > > Looks like you don't really have an alternative to
> > > of_mm_gpiochip_add_data(), you are replacing one single line by 11 new ones,
> > > and that is done twice (once for cpm1_gpiochip_add16(), once for
> > > cpm1_gpiochip_add32()).

Note, the overall statistics is +1 LoC only!

> > True, but that's the issue that we have of_specific API. If someone propose
> > the common API for the agnostic approach,  it would be nice, but I am not
> > the one. And TBH I do not see the advantage of it right now as almost every
> > GPIO driver is using its own labeling schema (*). Note, that this patch also
> > fixes a memory leak as a side effect.
> 
> Can you explain that in the commit message instead of saying you use
> appropriate alternatives that do not exist ?
> 
> Don't hesitate to mention the memory leak it in the commit message as well.
> 
> > *) the legacy API is mostly used by PPC code, do you want that of_mm_* thingy
> > to be moved to PPC specific code instead of killing it? Would be done this
> > way as well.
> 
> No no, your change is ok for me, just need an accurate commit message.

Fine, just sent a v2, which is now compile-tested as suggested below.

...

> > > Does not build:
> > 
> > Crap, I most likely built something else and not these files...
> > I have carefully build-test this for v2.
> 
> Just use mpc885_ads_defconfig

Thanks, that is helpful!

-- 
With Best Regards,
Andy Shevchenko




      reply	other threads:[~2024-11-18 12:33 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-11-15 13:38 [PATCH v1 1/1] powerpc/8xx: Drop legacy-of-mm-gpiochip.h header Andy Shevchenko
2024-11-16 10:44 ` Christophe Leroy
2024-11-18  9:20   ` Andy Shevchenko
2024-11-18 11:02     ` Christophe Leroy
2024-11-18 12:32       ` Andy Shevchenko [this message]

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=Zzsz81-quzIZ_13L@smile.fi.intel.com \
    --to=andriy.shevchenko@linux.intel.com \
    --cc=christophe.leroy@csgroup.eu \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=maddy@linux.ibm.com \
    --cc=mpe@ellerman.id.au \
    --cc=naveen@kernel.org \
    --cc=npiggin@gmail.com \
    /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.