From: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> To: Mark Brown <broonie@kernel.org> Cc: "Michal Simek" <michal.simek@xilinx.com>, "Sören Brinkmann" <soren.brinkmann@xilinx.com>, linux-spi@vger.kernel.org, "moderated list:ARM/S5P EXYNOS AR..." <linux-arm-kernel@lists.infradead.org>, LKML <linux-kernel@vger.kernel.org> Subject: Re: [PATCH 08/18] spi/xilinx: Support cores with no interrupt Date: Tue, 27 Jan 2015 20:05:57 +0100 [thread overview] Message-ID: <CAPybu_1RW0F-ak-B_4H-riTUwF2yCrpKdT2RWviTdCMR_kQmww@mail.gmail.com> (raw) In-Reply-To: <20150127000428.GK21293@sirena.org.uk> Hello Mark > 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. Seems very reasonable. Only problem I can see is if the spi clk is veeery slow, then a irq approach, even for transfer size bellow buffer_size will be better. > > 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. The hardware does not support dma (at least without making use of another core called cdma....) We can always argue that the system engineer can select polling by not specifying an irq on the device tree. >> + 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. The hardware will be reseted and in good state by the function find_buffer_size, so this should be good. Also the initialization is slightly different for polling and irq mode. So far I have fixed what you have sent and implemented a new patch for the "heuristic" irq mode. I will try to run in on hw tomorrow and send a new patchset with the patches that have not been merged yet. Maybe you want to consider also this patch from Michal Simek to your topic/xilinx branch https://patchwork.kernel.org/patch/5648351/ Thanks! -- Ricardo Ribalda
WARNING: multiple messages have this Message-ID (diff)
From: ricardo.ribalda@gmail.com (Ricardo Ribalda Delgado) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH 08/18] spi/xilinx: Support cores with no interrupt Date: Tue, 27 Jan 2015 20:05:57 +0100 [thread overview] Message-ID: <CAPybu_1RW0F-ak-B_4H-riTUwF2yCrpKdT2RWviTdCMR_kQmww@mail.gmail.com> (raw) In-Reply-To: <20150127000428.GK21293@sirena.org.uk> Hello Mark > 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. Seems very reasonable. Only problem I can see is if the spi clk is veeery slow, then a irq approach, even for transfer size bellow buffer_size will be better. > > 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. The hardware does not support dma (at least without making use of another core called cdma....) We can always argue that the system engineer can select polling by not specifying an irq on the device tree. >> + 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. The hardware will be reseted and in good state by the function find_buffer_size, so this should be good. Also the initialization is slightly different for polling and irq mode. So far I have fixed what you have sent and implemented a new patch for the "heuristic" irq mode. I will try to run in on hw tomorrow and send a new patchset with the patches that have not been merged yet. Maybe you want to consider also this patch from Michal Simek to your topic/xilinx branch https://patchwork.kernel.org/patch/5648351/ Thanks! -- Ricardo Ribalda
next prev parent reply other threads:[~2015-01-27 19:06 UTC|newest] Thread overview: 89+ messages / expand[flat|nested] mbox.gz Atom feed top 2015-01-23 16:08 [PATCH 00/18] spi/xilinx: Speed-up Ricardo Ribalda Delgado 2015-01-23 16:08 ` Ricardo Ribalda Delgado 2015-01-23 16:08 ` [PATCH 01/18] spi/xilinx: Support for spi mode LSB_FIRST Ricardo Ribalda Delgado 2015-01-23 16:08 ` Ricardo Ribalda Delgado 2015-01-26 19:23 ` Mark Brown 2015-01-26 19:23 ` Mark Brown 2015-01-26 19:23 ` Mark Brown 2015-01-23 16:08 ` [PATCH 02/18] spi/xilinx: Support for spi mode LOOP Ricardo Ribalda Delgado 2015-01-23 16:08 ` Ricardo Ribalda Delgado 2015-01-26 19:23 ` Mark Brown 2015-01-26 19:23 ` Mark Brown 2015-01-26 19:23 ` Mark Brown 2015-01-23 16:08 ` [PATCH 03/18] spi/xilinx: Simplify data read from the Rx FIFO Ricardo Ribalda Delgado 2015-01-23 16:08 ` Ricardo Ribalda Delgado 2015-01-23 16:08 ` Ricardo Ribalda Delgado 2015-01-27 17:25 ` Mark Brown 2015-01-27 17:25 ` Mark Brown 2015-01-27 17:25 ` Mark Brown 2015-01-23 16:08 ` [PATCH 04/18] spi-xilinx: Simplify spi_fill_tx_fifo Ricardo Ribalda Delgado 2015-01-23 16:08 ` Ricardo Ribalda Delgado 2015-01-26 23:54 ` Mark Brown 2015-01-26 23:54 ` Mark Brown 2015-01-23 16:08 ` [PATCH 05/18] spi/xilinx: Leave the IRQ always enabled Ricardo Ribalda Delgado 2015-01-23 16:08 ` Ricardo Ribalda Delgado 2015-01-23 16:08 ` [PATCH 06/18] spi/xilinx: Code cleanup Ricardo Ribalda Delgado 2015-01-23 16:08 ` Ricardo Ribalda Delgado 2015-01-27 0:05 ` Mark Brown 2015-01-27 0:05 ` Mark Brown 2015-01-27 0:05 ` Mark Brown 2015-01-23 16:08 ` [PATCH 07/18] spi/xilinx: Use cached value of register Ricardo Ribalda Delgado 2015-01-23 16:08 ` Ricardo Ribalda Delgado 2015-01-23 16:08 ` [PATCH 08/18] spi/xilinx: Support cores with no interrupt Ricardo Ribalda Delgado 2015-01-23 16:08 ` Ricardo Ribalda Delgado 2015-01-23 16:08 ` Ricardo Ribalda Delgado 2015-01-27 0:04 ` Mark Brown 2015-01-27 0:04 ` Mark Brown 2015-01-27 0:04 ` Mark Brown 2015-01-27 19:05 ` Ricardo Ribalda Delgado [this message] 2015-01-27 19:05 ` Ricardo Ribalda Delgado 2015-01-27 19:49 ` Mark Brown 2015-01-27 19:49 ` Mark Brown 2015-01-27 19:49 ` Mark Brown 2015-01-27 19:56 ` Ricardo Ribalda Delgado 2015-01-27 19:56 ` Ricardo Ribalda Delgado 2015-01-23 16:08 ` [PATCH 09/18] spi/xilinx: Do not inhibit transmission in polling mode Ricardo Ribalda Delgado 2015-01-23 16:08 ` Ricardo Ribalda Delgado 2015-01-23 16:08 ` [PATCH 10/18] spi/xilinx: Support for spi mode CS_HIGH Ricardo Ribalda Delgado 2015-01-23 16:08 ` Ricardo Ribalda Delgado 2015-01-23 16:08 ` [PATCH 11/18] spi/xilinx: Remove rx_fn and tx_fn pointer Ricardo Ribalda Delgado 2015-01-23 16:08 ` Ricardo Ribalda Delgado 2015-01-23 16:08 ` Ricardo Ribalda Delgado 2015-01-27 0:09 ` Mark Brown 2015-01-27 0:09 ` Mark Brown 2015-01-27 0:09 ` Mark Brown 2015-01-27 10:00 ` Ricardo Ribalda Delgado 2015-01-27 10:00 ` Ricardo Ribalda Delgado 2015-01-27 10:00 ` Ricardo Ribalda Delgado 2015-01-27 15:42 ` Joe Perches 2015-01-27 15:42 ` Joe Perches 2015-01-27 15:42 ` Joe Perches 2015-01-27 19:07 ` Ricardo Ribalda Delgado 2015-01-27 19:07 ` Ricardo Ribalda Delgado 2015-01-27 19:07 ` Ricardo Ribalda Delgado 2015-01-23 16:08 ` [PATCH 12/18] spi/xilinx: Make spi_tx and spi_rx simmetric Ricardo Ribalda Delgado 2015-01-23 16:08 ` Ricardo Ribalda Delgado 2015-01-23 16:08 ` [PATCH 13/18] spi/xilinx: Convert remainding_bytes in remaining words Ricardo Ribalda Delgado 2015-01-23 16:08 ` Ricardo Ribalda Delgado 2015-01-23 16:08 ` [PATCH 14/18] spi/xilinx: Convert bits_per_word in bytes_per_word Ricardo Ribalda Delgado 2015-01-23 16:08 ` Ricardo Ribalda Delgado 2015-01-23 16:08 ` Ricardo Ribalda Delgado 2015-01-23 16:08 ` [PATCH 15/18] spi/xilinx: Remove iowrite/ioread wrappers Ricardo Ribalda Delgado 2015-01-23 16:08 ` Ricardo Ribalda Delgado 2015-01-23 16:08 ` [PATCH 16/18] spi/xilinx: Reset core before buffer_size detection Ricardo Ribalda Delgado 2015-01-23 16:08 ` Ricardo Ribalda Delgado 2015-01-27 0:11 ` Mark Brown 2015-01-27 0:11 ` Mark Brown 2015-01-27 0:11 ` Mark Brown 2015-01-23 16:08 ` [PATCH 17/18] spi/xilinx: Remove remaining_words driver data variable Ricardo Ribalda Delgado 2015-01-23 16:08 ` Ricardo Ribalda Delgado 2015-01-23 16:08 ` [PATCH 18/18] spi/xilinx: Check number of slaves range Ricardo Ribalda Delgado 2015-01-23 16:08 ` Ricardo Ribalda Delgado 2015-01-27 0:14 ` [PATCH 00/18] spi/xilinx: Speed-up Mark Brown 2015-01-27 0:14 ` Mark Brown 2015-01-27 0:14 ` Mark Brown 2015-01-27 10:17 ` Ricardo Ribalda Delgado 2015-01-27 10:17 ` Ricardo Ribalda Delgado 2015-01-27 11:59 ` Mark Brown 2015-01-27 11:59 ` Mark Brown 2015-01-27 11:59 ` Mark Brown
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=CAPybu_1RW0F-ak-B_4H-riTUwF2yCrpKdT2RWviTdCMR_kQmww@mail.gmail.com \ --to=ricardo.ribalda@gmail.com \ --cc=broonie@kernel.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-spi@vger.kernel.org \ --cc=michal.simek@xilinx.com \ --cc=soren.brinkmann@xilinx.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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.