From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757446AbbA0AEi (ORCPT ); Mon, 26 Jan 2015 19:04:38 -0500 Received: from mezzanine.sirena.org.uk ([106.187.55.193]:35282 "EHLO mezzanine.sirena.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756822AbbA0AEg (ORCPT ); Mon, 26 Jan 2015 19:04:36 -0500 Date: Tue, 27 Jan 2015 00:04:28 +0000 From: Mark Brown To: Ricardo Ribalda Delgado Cc: Michal Simek , =?iso-8859-1?Q?S=F6ren?= Brinkmann , linux-spi@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Message-ID: <20150127000428.GK21293@sirena.org.uk> References: <1422029330-10971-1-git-send-email-ricardo.ribalda@gmail.com> <1422029330-10971-9-git-send-email-ricardo.ribalda@gmail.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="3w8x3zxtEJX8SUiO" Content-Disposition: inline In-Reply-To: <1422029330-10971-9-git-send-email-ricardo.ribalda@gmail.com> X-Cookie: My LESLIE GORE record is BROKEN ... 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: [PATCH 08/18] spi/xilinx: Support cores with no interrupt 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 --3w8x3zxtEJX8SUiO Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Jan 23, 2015 at 05:08:40PM +0100, Ricardo Ribalda Delgado wrote: > The core can run in polling mode. In fact, the performance of the core > is similar (or even better), due to the fact most of the spi > transactions are just a couple of bytes and there is one irq per > transactions. > When an mtd device is connected via spi, reading 8MB of data produces > more than 80K interrupts (with irq disabling, context swith....) Right, for short transfers polling tends to win every time over interrupts - if you look at other controller drivers you'll see a lot of them use this technique. The best practice here is generally to use a copy break and do very short transfers in polling mode and go back to interrupts for larger ones. This break is typically done at the point where we go over a FIFO in transfer size since normally if the device is designed to be run with DMA the FIFO will be quite small. Obviously this isn't quite the same case since not only is there no DMA support (yet?) but the FIFO could be any size but similar things apply especially if someone configured the device with a large FIFO. > - xspi_init_hw(xspi); > - > xspi->irq =3D platform_get_irq(pdev, 0); > - if (xspi->irq < 0) { > - ret =3D xspi->irq; > - goto put_master; > + if (xspi->irq >=3D 0) { > + /* Register for SPI Interrupt */ > + ret =3D devm_request_irq(&pdev->dev, xspi->irq, xilinx_spi_irq, 0, > + dev_name(&pdev->dev), xspi); > + if (ret) > + goto put_master; > } > =20 > - /* Register for SPI Interrupt */ > - ret =3D devm_request_irq(&pdev->dev, xspi->irq, xilinx_spi_irq, 0, > - dev_name(&pdev->dev), xspi); > - if (ret) > - goto put_master; > + /* SPI controller initializations */ > + xspi_init_hw(xspi); This appears to move the interrupt request before the hardware reset. Are you sure the interrupt handler won't misbehave if it fires before we finished initializing? One thing the hardware reset should do is get the device in a known good state. --3w8x3zxtEJX8SUiO Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAEBCAAGBQJUxtYLAAoJECTWi3JdVIfQOwUH/1aYs5vxXqB1lWaVjS5o4GNq pl9g+ATdXLxZScdsv1aquXZH/mjGQVKvVjQ47eUkB1eDPUdqhwR9pmEK3VDK24vc D3bhYTsmkL1VtYWLBFRsY2EMWfQitXLE+B9/Cgpls5WLLalKeWL/5eGMSdat7Ysn 2JrU7OeVIZihJnua731RwDWuCu0VYHxDwpybdYp20xqru+T4KZJVGpDCVZRiRpuj nuNTdWuJgzaMviPIDnKW8ena2sarnBDpYLF3EQZsGHWQvBneJXVC1Wv4wJzsXapv /D5vAJBuApN6mqBeeG5qEPeePwIbm6HM2DtKQXW5CIFbyTLMZsE2FWoijoObE2M= =ekUg -----END PGP SIGNATURE----- --3w8x3zxtEJX8SUiO-- From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mark Brown Subject: Re: [PATCH 08/18] spi/xilinx: Support cores with no interrupt Date: Tue, 27 Jan 2015 00:04:28 +0000 Message-ID: <20150127000428.GK21293@sirena.org.uk> References: <1422029330-10971-1-git-send-email-ricardo.ribalda@gmail.com> <1422029330-10971-9-git-send-email-ricardo.ribalda@gmail.com> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="3w8x3zxtEJX8SUiO" Cc: Michal Simek , =?iso-8859-1?Q?S=F6ren?= Brinkmann , linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Ricardo Ribalda Delgado Return-path: Content-Disposition: inline In-Reply-To: <1422029330-10971-9-git-send-email-ricardo.ribalda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> Sender: linux-spi-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-ID: --3w8x3zxtEJX8SUiO Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Jan 23, 2015 at 05:08:40PM +0100, Ricardo Ribalda Delgado wrote: > The core can run in polling mode. In fact, the performance of the core > is similar (or even better), due to the fact most of the spi > transactions are just a couple of bytes and there is one irq per > transactions. > When an mtd device is connected via spi, reading 8MB of data produces > more than 80K interrupts (with irq disabling, context swith....) Right, for short transfers polling tends to win every time over interrupts - if you look at other controller drivers you'll see a lot of them use this technique. The best practice here is generally to use a copy break and do very short transfers in polling mode and go back to interrupts for larger ones. This break is typically done at the point where we go over a FIFO in transfer size since normally if the device is designed to be run with DMA the FIFO will be quite small. Obviously this isn't quite the same case since not only is there no DMA support (yet?) but the FIFO could be any size but similar things apply especially if someone configured the device with a large FIFO. > - xspi_init_hw(xspi); > - > xspi->irq =3D platform_get_irq(pdev, 0); > - if (xspi->irq < 0) { > - ret =3D xspi->irq; > - goto put_master; > + if (xspi->irq >=3D 0) { > + /* Register for SPI Interrupt */ > + ret =3D devm_request_irq(&pdev->dev, xspi->irq, xilinx_spi_irq, 0, > + dev_name(&pdev->dev), xspi); > + if (ret) > + goto put_master; > } > =20 > - /* Register for SPI Interrupt */ > - ret =3D devm_request_irq(&pdev->dev, xspi->irq, xilinx_spi_irq, 0, > - dev_name(&pdev->dev), xspi); > - if (ret) > - goto put_master; > + /* SPI controller initializations */ > + xspi_init_hw(xspi); This appears to move the interrupt request before the hardware reset. Are you sure the interrupt handler won't misbehave if it fires before we finished initializing? One thing the hardware reset should do is get the device in a known good state. --3w8x3zxtEJX8SUiO Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAEBCAAGBQJUxtYLAAoJECTWi3JdVIfQOwUH/1aYs5vxXqB1lWaVjS5o4GNq pl9g+ATdXLxZScdsv1aquXZH/mjGQVKvVjQ47eUkB1eDPUdqhwR9pmEK3VDK24vc D3bhYTsmkL1VtYWLBFRsY2EMWfQitXLE+B9/Cgpls5WLLalKeWL/5eGMSdat7Ysn 2JrU7OeVIZihJnua731RwDWuCu0VYHxDwpybdYp20xqru+T4KZJVGpDCVZRiRpuj nuNTdWuJgzaMviPIDnKW8ena2sarnBDpYLF3EQZsGHWQvBneJXVC1Wv4wJzsXapv /D5vAJBuApN6mqBeeG5qEPeePwIbm6HM2DtKQXW5CIFbyTLMZsE2FWoijoObE2M= =ekUg -----END PGP SIGNATURE----- --3w8x3zxtEJX8SUiO-- -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 From: broonie@kernel.org (Mark Brown) Date: Tue, 27 Jan 2015 00:04:28 +0000 Subject: [PATCH 08/18] spi/xilinx: Support cores with no interrupt In-Reply-To: <1422029330-10971-9-git-send-email-ricardo.ribalda@gmail.com> References: <1422029330-10971-1-git-send-email-ricardo.ribalda@gmail.com> <1422029330-10971-9-git-send-email-ricardo.ribalda@gmail.com> Message-ID: <20150127000428.GK21293@sirena.org.uk> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Fri, Jan 23, 2015 at 05:08:40PM +0100, Ricardo Ribalda Delgado wrote: > The core can run in polling mode. In fact, the performance of the core > is similar (or even better), due to the fact most of the spi > transactions are just a couple of bytes and there is one irq per > transactions. > When an mtd device is connected via spi, reading 8MB of data produces > more than 80K interrupts (with irq disabling, context swith....) Right, for short transfers polling tends to win every time over interrupts - if you look at other controller drivers you'll see a lot of them use this technique. The best practice here is generally to use a copy break and do very short transfers in polling mode and go back to interrupts for larger ones. This break is typically done at the point where we go over a FIFO in transfer size since normally if the device is designed to be run with DMA the FIFO will be quite small. Obviously this isn't quite the same case since not only is there no DMA support (yet?) but the FIFO could be any size but similar things apply especially if someone configured the device with a large FIFO. > - xspi_init_hw(xspi); > - > xspi->irq = platform_get_irq(pdev, 0); > - if (xspi->irq < 0) { > - ret = xspi->irq; > - goto put_master; > + if (xspi->irq >= 0) { > + /* Register for SPI Interrupt */ > + ret = devm_request_irq(&pdev->dev, xspi->irq, xilinx_spi_irq, 0, > + dev_name(&pdev->dev), xspi); > + if (ret) > + goto put_master; > } > > - /* Register for SPI Interrupt */ > - ret = devm_request_irq(&pdev->dev, xspi->irq, xilinx_spi_irq, 0, > - dev_name(&pdev->dev), xspi); > - if (ret) > - goto put_master; > + /* SPI controller initializations */ > + xspi_init_hw(xspi); This appears to move the interrupt request before the hardware reset. Are you sure the interrupt handler won't misbehave if it fires before we finished initializing? One thing the hardware reset should do is get the device in a known good state. -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 473 bytes Desc: Digital signature URL: