From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753576AbcAGQOZ (ORCPT ); Thu, 7 Jan 2016 11:14:25 -0500 Received: from unicorn.mansr.com ([81.2.72.234]:41802 "EHLO unicorn.mansr.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753347AbcAGQOX convert rfc822-to-8bit (ORCPT ); Thu, 7 Jan 2016 11:14:23 -0500 From: =?iso-8859-1?Q?M=E5ns_Rullg=E5rd?= To: Mark Brown Cc: Cyrille Pitchen , Nicolas Ferre , linux-spi@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org, Rob Herring , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala Subject: Re: [PATCH v3 1/3] spi: atmel: add support for the internal chip-select of the spi controller References: <20160107154024.GF6588@sirena.org.uk> Date: Thu, 07 Jan 2016 16:14:12 +0000 In-Reply-To: <20160107154024.GF6588@sirena.org.uk> (Mark Brown's message of "Thu, 7 Jan 2016 15:40:24 +0000") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Mark Brown writes: > On Tue, Jan 05, 2016 at 09:50:54PM +0000, Måns Rullgård wrote: >> Cyrille Pitchen writes: > >> > + as->use_cs_gpios = true; >> > + if (atmel_spi_is_v2(as) && >> > + !of_get_property(pdev->dev.of_node, "cs-gpios", NULL)) { >> > + as->use_cs_gpios = false; >> > + master->num_chipselect = 4; >> > + } > >> This part breaks the AVR32 boards and probably anything else that >> doesn't use devicetree but does use GPIOs for chip select. > > Shouldn't this just be setting defaults for the case where nothing is > provided? Traditionally the platform data included a GPIO number to use and that was that. At some point DT support was added wherein the platform data is overridden by cs_gpio from struct spi_device if this is valid (it is set to a negative value by default). Thus far all was well. Then came this patch. It assumes that everybody uses DT and treats a missing cs-gpios property as indication that the controller's own pins should be used. It also assumes that if any device uses GPIO for CS all of them do, even though the SPI core driver might provide a partial list (probably since many boards don't use all the available chip selects, but it doesn't prevent someone abusing this). To work correctly in call cases, this driver should use, for each peripheral, the following priority: - spi->cs_gpio (filled from DT or -ENOENT) - GPIO from platform data - controller chip select pin The trouble is that there is no way to reliably tell a valid GPIO number of zero in the platform data from an unset value. In practice, I believe existing old boards using this driver all use a non-zero GPIO (the AVR32 platform code requires this), so checking for a non-zero number is probably sufficient. I'll cook up a patch for this unless someone objects. -- Måns Rullgård