linux-gpio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Charles Keepax <ckeepax@opensource.cirrus.com>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: <broonie@kernel.org>, <brgl@bgdev.pl>,
	<linux-gpio@vger.kernel.org>, <linux-spi@vger.kernel.org>,
	<patches@opensource.cirrus.com>
Subject: Re: [PATCH 1/3] gpio: swnode: Add ability to specify native chip selects for SPI
Date: Mon, 8 Apr 2024 13:21:31 +0000	[thread overview]
Message-ID: <ZhPvW46kGeOnG++E@ediswmail9.ad.cirrus.com> (raw)
In-Reply-To: <CACRpkdZP_9y-Z=eZcbQe=ZF2ejutP6gD2ofTxXNvGTh7CUfwFg@mail.gmail.com>

On Thu, Apr 04, 2024 at 10:16:35AM +0200, Linus Walleij wrote:
> On Tue, Mar 26, 2024 at 3:11 PM Charles Keepax
> <ckeepax@opensource.cirrus.com> wrote:
> > +const struct software_node swnode_gpio_undefined = {
> > +       .name = "gpio-internal-undefined",
> > +};
> > +EXPORT_SYMBOL_GPL(swnode_gpio_undefined);
> 
> This needs a comment in the code telling exactly why this is here.
> It is also taking up space and code here on systems that have no use
> for it, so I wonder if it is possible to make this optional.
> 

Happy to add the comment, less sure about how to make it
optional. I could ifdef it based the SPI config, but whilst that
is the current user the mechanism feels like it is more generic
than that and could be used in other bindings as well.

> > +       if (!strcmp(gdev_node->name, "gpio-internal-undefined"))
> > +               return ERR_PTR(-ENOENT);
> 
> This needs a comment stating why this check is here, it's not
> obvious.

Happy to add a comment here as well.

Thanks,
Charles

  reply	other threads:[~2024-04-08 13:21 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-26 14:11 [PATCH 0/3] Add bridged amplifiers to cs42l43 Charles Keepax
2024-03-26 14:11 ` [PATCH 1/3] gpio: swnode: Add ability to specify native chip selects for SPI Charles Keepax
2024-04-04  8:16   ` Linus Walleij
2024-04-08 13:21     ` Charles Keepax [this message]
2024-04-09  7:12       ` Linus Walleij
2024-04-09  8:44         ` Charles Keepax
2024-12-11  0:15   ` Dmitry Torokhov
2024-12-12 10:45     ` Charles Keepax
2024-03-26 14:11 ` [PATCH 2/3] spi: Add a mechanism to use the fwnode name for the SPI device Charles Keepax
2024-03-26 14:11 ` [PATCH 3/3] spi: cs42l43: Add bridged cs35l56 amplifiers Charles Keepax
2024-03-27 18:34   ` kernel test robot
2024-03-28 13:15   ` kernel test robot

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=ZhPvW46kGeOnG++E@ediswmail9.ad.cirrus.com \
    --to=ckeepax@opensource.cirrus.com \
    --cc=brgl@bgdev.pl \
    --cc=broonie@kernel.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-spi@vger.kernel.org \
    --cc=patches@opensource.cirrus.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;
as well as URLs for NNTP newsgroup(s).