linux-kernel - Re: [RFC PATCH] SPI/ACPI: DesignWare: Add ACPI support for Designware SPI driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 5 Feb 2016 11:09:00 +0000
From:	Mark Brown <broonie@...nel.org>
To:	qiujiang <qiujiang@...wei.com>
Cc:	linux-spi@...r.kernel.org, linux-acpi@...r.kernel.org,
	linux-kernel@...r.kernel.org, linuxarm@...wei.com,
	haifeng.wei@...wei.com, charles.chenxin@...wei.com,
	Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
	Jarkko Nikula <jarkko.nikula@...ux.intel.com>
Subject: Re: [RFC PATCH] SPI/ACPI: DesignWare: Add ACPI support for
 Designware SPI driver

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. :(

Download attachment "signature.asc" of type "application/pgp-signature" (474 bytes)