From patchwork Mon Mar 13 04:08:59 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Greg Ungerer X-Patchwork-Id: 9619723 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 944C860414 for ; Mon, 13 Mar 2017 04:19:15 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 7935D28415 for ; Mon, 13 Mar 2017 04:19:15 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 6C6CB28445; Mon, 13 Mar 2017 04:19:15 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id AB01B28415 for ; Mon, 13 Mar 2017 04:19:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750738AbdCMETO (ORCPT ); Mon, 13 Mar 2017 00:19:14 -0400 Received: from icp-osb-irony-out8.external.iinet.net.au ([203.59.1.225]:37593 "EHLO icp-osb-irony-out8.external.iinet.net.au" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750714AbdCMETN (ORCPT ); Mon, 13 Mar 2017 00:19:13 -0400 X-Greylist: delayed 590 seconds by postgrey-1.27 at vger.kernel.org; Mon, 13 Mar 2017 00:19:12 EDT X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: A2APBQCGGsZY/zXSMGddHAEBBAEBCgEBg1GBa4MSmwMGmGwahgiCQ1cBAgEBAQEBAmsFI4VDUigngQKJcwyyADomAophhgmMVAyDDQWcQAGSO4pnhjoCk0NXgQQjFggkCEGEBwEBCIJYLjWJUwEBAQ X-IPAS-Result: A2APBQCGGsZY/zXSMGddHAEBBAEBCgEBg1GBa4MSmwMGmGwahgiCQ1cBAgEBAQEBAmsFI4VDUigngQKJcwyyADomAophhgmMVAyDDQWcQAGSO4pnhjoCk0NXgQQjFggkCEGEBwEBCIJYLjWJUwEBAQ X-IronPort-AV: E=Sophos;i="5.36,156,1486396800"; d="scan'208";a="193763997" Received: from unknown (HELO goober.accelecon.com) ([103.48.210.53]) by icp-osb-irony-out8.iinet.net.au with ESMTP; 13 Mar 2017 12:09:02 +0800 From: Greg Ungerer To: linux-spi@vger.kernel.org Cc: Greg Ungerer Subject: [RFC] spi: imx: fix use of native chip-selects with devicetree Date: Mon, 13 Mar 2017 14:08:59 +1000 Message-Id: <1489378139-11707-1-git-send-email-gerg@linux-m68k.org> X-Mailer: git-send-email 1.9.1 Sender: linux-spi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-spi@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP I have an iMX253 based system with 2 SPI buses, one with a flash hanging of it, and one with a SLIC. On modern kernels (I am using 4.9 - but I think current mainline 4.11-rc2 is the same) the SPI bus with the SLIC device can't be configured to work with a devicetree setup. The problem is that the SLIC device hardware arrangement needs to use the iMX SPI native chipselect to provide the neccessary hardware cycles. Maybe I am missing something but I can't see how that can be made to work with the current devicetree setup required on this platform. It would appear that using a cs_gpio field of "<0>" is meant to indicate use of a native chipselect - though I couldn't find that documented anywhere. In any case I couldn't see any other way to do it either. But with a cs_gpio field set to "<0>" the config code doesn't correctly configure the iMX SPI registers to use a native chipselect. In fact it actively looks wrong in the way it mangles the cs_gpio to calculate a native chipselect when using a devicetree (it looks like it would be correct for the platform setup case though). Below is the patch I came up with to fix it for my hardware. This results in the correct native chipselect being set for my SLIC hardware and it all works nicely. Is this on the right track or have I missed something comepletely? -------- The commonly used mechanism of specifying the native chipselect on an SPI device in devicetree (that is "cs-gpios = <0> ...") does not result in the native chipselect being configured for use. So external SPI devices that require use of the native chipselect will not work. You can successfully specify native chipselects if using a platform setup by specifying the cs-gpio as negative offset by 32. And that looks to be working correctly. The logic in the spi-imx.c driver during probe uses core spi function of_spi_register_master() in spi.c to parse the "cs-gpios" devicetree tag. For valid GPIO values that will be recorded for use, all other entries in cs_gpios list will be set to -ENOENT. So entries like "<0>" will be set to -ENOENT in the cs_gpios list. When the SPI device registers are setup the code will use the GPIO listed in the cs_gpios list for the desired chipselect. If the cs_gpio is less then 0 then it is intended to be for a native chipselect, and its cs_gpio value is added to 32 to get the chipselect number to use. Problem is that with devicetree this can only ever be -ENOENT (which is -2), and that alone results in an invalid chipselect number. But also doesn't allow selection of the native chipselect at all. To fix I modified the setup logic so that if cs_gpio is less than 0, and it is -ENOENT, then we use the chipselect number associated with this spi device. This will allow platform setups to continue to be able to specify the chipselect number they want, and on devicetree systems the absence of a valid GPIO will use the native chipselect. Acked-by: Greg Ungerer --- drivers/spi/spi-imx.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c index 9a7c62f..c6e13f7 100644 --- a/drivers/spi/spi-imx.c +++ b/drivers/spi/spi-imx.c @@ -516,10 +516,12 @@ static int mx31_config(struct spi_device *spi, struct spi_imx_config *config) reg |= MX31_CSPICTRL_POL; if (spi->mode & SPI_CS_HIGH) reg |= MX31_CSPICTRL_SSPOL; - if (spi->cs_gpio < 0) - reg |= (spi->cs_gpio + 32) << - (is_imx35_cspi(spi_imx) ? MX35_CSPICTRL_CS_SHIFT : - MX31_CSPICTRL_CS_SHIFT); + if (spi->cs_gpio < 0) { + int cs = (spi->cs_gpio == -ENOENT) ? spi->chip_select : + spi->cs_gpio + 32; + reg |= cs << (is_imx35_cspi(spi_imx) ? MX35_CSPICTRL_CS_SHIFT : + MX31_CSPICTRL_CS_SHIFT); + } if (spi_imx->usedma) reg |= MX31_CSPICTRL_SMC; @@ -608,8 +610,11 @@ static int mx21_config(struct spi_device *spi, struct spi_imx_config *config) reg |= MX21_CSPICTRL_POL; if (spi->mode & SPI_CS_HIGH) reg |= MX21_CSPICTRL_SSPOL; - if (spi->cs_gpio < 0) - reg |= (spi->cs_gpio + 32) << MX21_CSPICTRL_CS_SHIFT; + if (spi->cs_gpio < 0) { + int cs = (spi->cs_gpio == -ENOENT) ? spi->chip_select : + spi->cs_gpio + 32; + reg |= cs << MX21_CSPICTRL_CS_SHIFT; + } writel(reg, spi_imx->base + MXC_CSPICTRL);