From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754139AbbE1PEf (ORCPT ); Thu, 28 May 2015 11:04:35 -0400 Received: from mezzanine.sirena.org.uk ([106.187.55.193]:60757 "EHLO mezzanine.sirena.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753584AbbE1PEU (ORCPT ); Thu, 28 May 2015 11:04:20 -0400 Date: Thu, 28 May 2015 16:03:56 +0100 From: Mark Brown To: Harini Katakam Cc: Ranjit Waghmode , Rob Herring , Pawel Moll , Mark Rutland , "ijc+devicetree@hellion.org.uk" , Kumar Gala , Michal Simek , =?iso-8859-1?Q?S=F6ren?= Brinkmann , "devicetree@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , linux-spi , Punnaiah Choudary Kalluri , ran27jit@gmail.com Message-ID: <20150528150356.GA21577@sirena.org.uk> References: <1432106871-27232-1-git-send-email-ranjit.waghmode@xilinx.com> <1432106871-27232-2-git-send-email-ranjit.waghmode@xilinx.com> <20150522115854.GG21391@sirena.org.uk> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="bU64ygGtHgdLJp+F" Content-Disposition: inline In-Reply-To: X-Cookie: Positively no smoking. User-Agent: Mutt/1.5.23 (2014-03-12) X-SA-Exim-Connect-IP: 94.175.94.161 X-SA-Exim-Mail-From: broonie@sirena.org.uk Subject: Re: [RFC PATCH 2/2] spi: Add support for Zynq Ultrascale+ MPSoC GQSPI controller X-SA-Exim-Version: 4.2.1 (built Mon, 26 Dec 2011 16:24:06 +0000) X-SA-Exim-Scanned: Yes (on mezzanine.sirena.org.uk) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --bU64ygGtHgdLJp+F Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Fri, May 22, 2015 at 08:43:54PM +0530, Harini Katakam wrote: > On Fri, May 22, 2015 at 5:28 PM, Mark Brown wrote: > > On Wed, May 20, 2015 at 12:57:51PM +0530, Ranjit Waghmode wrote: > > Why is there a default case here? That's going to men we try to handle > > any random chip select that gets passed in as pointing to this lower > > device which doesn't seem right. The fact that this is trying to handle > > mirroring of the chip select to two devices is also raising alarm bells > > here... > This SPI controller has two CS lines and two data bus. > Two devices can be connected to these and either the upper or the > lower or both (Explained below) can be selected. > When two flash devices are used, one of the HW configurations in > which they can be connected is called "parallel" mode where they I know what wiring chip selects in parallel is but that's not the question - the question is about the handling of the default case. > >> +static void zynqmp_qspi_chipselect(struct spi_device *qspi, bool is_high) > >> +{ > >> + if (is_high) { > >> + /* Manually start the generic FIFO command */ > >> + zynqmp_gqspi_write(xqspi, GQSPI_CONFIG_OFST, > >> + zynqmp_gqspi_read(xqspi, GQSPI_CONFIG_OFST) | > >> + GQSPI_CFG_START_GEN_FIFO_MASK); > > No, this is broken - setting the chip select should set the chip select, > > it shouldn't have any impact on transfers. Transfers should be started > > in the transfer operations. > This is the only way to assert the CS. It doesn't start transferring any data. OK, then you can't implement a separate set_cs() operation and shouldn't be trying to do so. This will break in multiple ways when the framework tries to use the operations separately. You probably need to implement a single flat transfer() operation. --bU64ygGtHgdLJp+F Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAEBCAAGBQJVZy5bAAoJECTWi3JdVIfQHssH/iyjeBwoPxxRDn9j42CeGb2w XywOInE1XlKDRMgzztEBRvHbpPOl3m9+Yvdh3QIoESGcDjMHvkYBqXO9uhpEtfNL rOBOL+tXlY384F7Tx4pdhstNIwm7yXqx2RM0Pg7ObA0mg1ecfbJ2rG2xd90N7cn3 QVcsmhDSGSeFLA9wLLsVG7PZ4MaaqkOAcHr+ixjrSwFuicPCTspFgovdkxxb0saE WG6l0cVd1D7nancABt+0PmVtIv0+iG4fa/HuTQZmbK5flbpzJreCuB3gu2sYwZq5 ++ddRQ9DGjL2GOyrRnEkb5YrP/B/X3AxntLI4o7le1NIUHsWnbTh9FIOqiachwE= =jNn9 -----END PGP SIGNATURE----- --bU64ygGtHgdLJp+F--