Re: [PATCH 0/5] Regulator deadcode cleanups

["Dr. David Alan Gilbert" <linux@treblig.org>]

* Mark Brown (broonie@kernel.org) wrote:
> On Sun, Apr 27, 2025 at 02:58:03PM +0000, Dr. David Alan Gilbert wrote:
> > * Mark Brown (broonie@kernel.org) wrote:
> 
> > > Please do some analysis as to why the functions are there, don't just
> > > blindly delete things.
> 
> > I'd appreciate some more idea of what you're after;  each patch
> > shows where and when the function was added or last used.  Some have
> 
> Something that indicates that this is a patch written by a human rather
> than some automated noise, that considers things like API usability and
> coherence, or what people might do if the API is not available when they
> need it, rather than just mechanically churning something out.  None of
> your commit logs consider what the code you're deleting does at all.

I do manually write each patch, but I don't have that global feel of the
API; but I do use my judgement to avoid some things:
   * I tend not to remove one side of an obvious pair of functions
     (e.g. a set/clear or an alloc/free)

   * I avoid things that look like a function for every firmware interface
     where the functions are almost documenting the interface.

   * I only bother deleting one line functions if it's part of a set.

and some others.  It's not automatic, but I don't claim to understand
the whole interface.  I will try and follow a thread if I end up deleting
something which then makes it look like something else isn't needed.
I do have _some_ feel - so have spotted some bugs where a function
should have been called.

> > comments saying things like the devm_ version is being used (so it
> > seemed reasonable to me to delete the plain version if no one uses it).
> 
> Deleting the plain version of something where a devm version exists is
> an obvious example of making the API less coherent and hard to use,
> managed resources aren't universally appropriate and so you should
> generally be able to do the same thing with manual resource management.

OK, that's something I hadn't expected - I'd thought if one style was
used for many years, that the other was redundant.

It can be quite tricky to see why functions have ended up not being
used for years (or decades), some is making a nice API, but sometimes
it's people blindly copying another API or who had a set of patches
which used the function but those patches got abandoned.

It also varies a lot between maintainer - some really prefer not
to have unused functions at all.

Dave

-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\        dave @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/