public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Philipp Zabel <p.zabel@pengutronix.de>
To: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Hans de Goede <hdegoede@redhat.com>,
	Lee Jones <lee.jones@linaro.org>
Subject: Re: [PATCH] reset: generate reset_control_get variants with macro expansion
Date: Fri, 22 Jul 2016 14:58:04 +0200	[thread overview]
Message-ID: <1469192284.27223.32.camel@pengutronix.de> (raw)
In-Reply-To: <CAK7LNATuDMhcH0z77=Wmt1uC437KLbmvLOwtFUYW-y-XpQ-KYg@mail.gmail.com>

Hi Masahiro,

Am Donnerstag, den 21.07.2016, 18:53 +0900 schrieb Masahiro Yamada:
[...]
> For ctags, scripts/tags.sh exists for that purpose, doesn't it?
> 
> For example,
> commit e0e5070b20e01f0321f97db4e4e174f3f6b49e50
> converted func defines and adjusted tags.sh at the same time.

True. So ctags is covered, alhough this somewhat moves the maintenance
effort from nearly duplicated function definitions to keeping
scripts/tags.sh up to date with all the different macro function
generators instead.

> > And secondly, we would now have detailed kerneldoc comments for two
> > functions that are never called directly, but the public facing API
> > itself is completely without documentation. I wouldn't mind removing
> > duplicated documentation paragraphs though, for example by referencing
> > reset_control_get_* from the of_reset_control_get_* kerneldoc comments.
> 
> Linux is not always kind enough to document every public API.
> 
> As you see, we generally do not document
> static inline wrappers.

Why not? As long as there is a meaningful difference between them, it
would make sense to describe them individually.

> In our case, they are all simply derived from two base functions.
> Documenting the two is enough.

I disagree. As a user of an API and its documentation, I would like to
have descriptions of functions that are actually used in the code, not
descriptions of the internal implementation behind them. This also
allows to have shorter paragraphs for each function, since they only
have to describe the behavior of that particular variant.

> I need to expand drivers/usb/host/ehci-platform.c
> to support multiple reset lines, so I really want
> devm_reset_control_get_optional_shared_by_index() supported.
> 
> (So, probably
> devm_reset_control_get_exclusive_by_index
> devm_reset_control_get_shared_by_index
> devm_reset_control_get_optional_exclusive_by_index
> as well)
>
> If you are unhappy with this patch,
> I will send an alternative one,
> which adds 4 more functions in a stupid and straightforward way.

Yes, I would prefer straightforward addition of the needed inline
functions.

regards
Philipp

      parent reply	other threads:[~2016-07-22 12:58 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-21  5:05 [PATCH] reset: generate reset_control_get variants with macro expansion Masahiro Yamada
2016-07-21  8:41 ` Philipp Zabel
2016-07-21  9:53   ` Masahiro Yamada
2016-07-21 12:06     ` Lee Jones
2016-07-22 12:58     ` Philipp Zabel [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=1469192284.27223.32.camel@pengutronix.de \
    --to=p.zabel@pengutronix.de \
    --cc=hdegoede@redhat.com \
    --cc=lee.jones@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=yamada.masahiro@socionext.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox