LKML Archive mirror
 help / color / mirror / Atom feed
From: Mark Brown <broonie@kernel.org>
To: qiujiang <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: Fri, 5 Feb 2016 11:09:00 +0000	[thread overview]
Message-ID: <20160205110900.GA12311@sirena.org.uk> (raw)
In-Reply-To: <1454656280-130658-1-git-send-email-qiujiang@huawei.com>

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

On Fri, Feb 05, 2016 at 03:11:20PM +0800, qiujiang wrote:

> This patch added ACPI support for DesignWare SPI mmio driver. It
> was based the corresponding DT driver and compatible for this two
> way. This patch has been tested on Hisilicon D02 board. It relies
> on the GPIO patchset.

Intel are heavy users of this driver on their systems which also use
ACPI.  Have you discussed this binding with them?  I've copied Andy and
Jarkko who've worked on the driver recently.

Please use subject lines matching the style for the subsystem.  This
makes it easier for people to identify relevant patches.

> +	char propname[32];

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

> +	if (ACPI_COMPANION(&pdev->dev)) {
> +		for (i = 0; i < dws->num_cs; i++) {
> +			snprintf(propname, sizeof(propname), "cs%d", i);
> +			gpiod = devm_gpiod_get(&pdev->dev,
> +				propname, GPIOD_ASIS);
> +			if (IS_ERR(gpiod)) {
> +				dev_err(&pdev->dev, "Get gpio desc failed!\n");
> +				return PTR_ERR(gpiod);
> +			}
> +		}
> +	}

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?

> +static const struct acpi_device_id dw_spi_mmio_acpi_match[] = {
> +		{"HISI0171", 0},
> +		{ }
> +};
> +MODULE_DEVICE_TABLE(acpi, dw_spi_mmio_acpi_match);

I really do wish ACPI had some more sensible system for allocating
device IDs so the tables were a little more legible. :(

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

  reply	other threads:[~2016-02-05 11:09 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 [this message]
2016-02-14  9:31   ` Jiang Qiu
2016-02-15 17:44     ` Mark Brown
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=20160205110900.GA12311@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).