LKML Archive mirror
 help / color / mirror / Atom feed
From: Mark Brown <broonie@kernel.org>
To: Jiang Qiu <qiujiang@huawei.com>
Cc: linux-spi@vger.kernel.org, linux-acpi@vger.kernel.org,
	linux-kernel@vger.kernel.org, linuxarm@huawei.com,
	haifeng.wei@huawei.com, charles.chenxin@huawei.com,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Jarkko Nikula <jarkko.nikula@linux.intel.com>
Subject: Re: [RFC PATCH] SPI/ACPI: DesignWare: Add ACPI support for Designware SPI driver
Date: Mon, 15 Feb 2016 17:44:38 +0000	[thread overview]
Message-ID: <20160215174438.GR18988@sirena.org.uk> (raw)
In-Reply-To: <56C04962.7080206@huawei.com>

[-- Attachment #1: Type: text/plain, Size: 2108 bytes --]

On Sun, Feb 14, 2016 at 05:31:14PM +0800, Jiang Qiu wrote:
> 在 2016/2/5 19:09, Mark Brown 写道:
> >On Fri, Feb 05, 2016 at 03:11:20PM +0800, qiujiang wrote:

Please fix your mail client to word wrap within paragraphs at something
substantially less than 80 columns.  Doing this makes your messages much
easier to read and reply to.  Your mail has lines wrapped alternately at
normal length and half length which is difficult to read, I've reflowed
this time.

> >>+	char propname[32];

> >That's a magic number, where did it come from and why is it a magic
> >nummber?

> I'm sorry for here without any comments. This number define is come from
> gpiolib.c. It means the max size of gpio property name. The reference code
> located in line 1815 of gpiolib.c.

That really isn't helping here.  The problem is that you've got a random
magic number thrown in here with no documentation.

> >I'm not seeing anywhere where we store the GPIO in this loop.  It is
> >therefore unclear to me how the chip select is going to work?

> In DT binding, of_get_named_gpio and devm_gpio_request were used to
> parse gpio pins defined in DTs and then request these pins. Similarly,
> for ACPI, devm_gpiod_get can do that two operation in a single
> function. It is a unified interface to ACPI and DT binding.

> If the gpiod is valid, the corresponding gpio pins has been requested.
> We do not need to save this gpiod any more.

No, that makes no sense at all.  If we're requesting a GPIO to use as
chip select we can't just then ignore the GPIO, we need to control it at
some point and...

> which gpio pin was used is defined in spi_device, named cs_gpio, the
> configuration to the gpio pins will be done in the setup callback
> routine of each device. What the spi master should do is just request
> these pins to the gpio subsystem.

...this would be a complete failure of abstraction - you appear to be
suggesting that the client devices should also separately request the
GPIOs and set them up.  That's at best going to be redundant and is
likely to introduce bugs.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

  reply	other threads:[~2016-02-15 17:44 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-05  7:11 [RFC PATCH] SPI/ACPI: DesignWare: Add ACPI support for Designware SPI driver qiujiang
2016-02-05 11:09 ` Mark Brown
2016-02-14  9:31   ` Jiang Qiu
2016-02-15 17:44     ` Mark Brown [this message]
2016-02-15  8:27   ` Hanjun Guo
2016-02-15 17:45     ` Mark Brown
2016-02-15 18:09       ` One Thousand Gnomes
2016-02-05 15:55 ` Andy Shevchenko
2016-02-05 16:11   ` Mark Brown
2016-02-14  9:47   ` Jiang Qiu

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=20160215174438.GR18988@sirena.org.uk \
    --to=broonie@kernel.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=charles.chenxin@huawei.com \
    --cc=haifeng.wei@huawei.com \
    --cc=jarkko.nikula@linux.intel.com \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-spi@vger.kernel.org \
    --cc=linuxarm@huawei.com \
    --cc=qiujiang@huawei.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 read-only IMAP folder(s) and NNTP newsgroup(s).