From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933924AbcAKQdd (ORCPT ); Mon, 11 Jan 2016 11:33:33 -0500 Received: from eusmtp01.atmel.com ([212.144.249.243]:52521 "EHLO eusmtp01.atmel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932457AbcAKQdb (ORCPT ); Mon, 11 Jan 2016 11:33:31 -0500 Subject: Re: [PATCH] spi: atmel: improve internal vs gpio chip-select choice To: Nicolas Ferre , Mans Rullgard , Mark Brown , , , linux-arm-kernel , "Yang, Wenyou" References: <1452211907-16074-1-git-send-email-mans@mansr.com> <5693C9A3.9040403@atmel.com> From: Cyrille Pitchen Message-ID: <5693D957.30103@atmel.com> Date: Mon, 11 Jan 2016 17:33:27 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: <5693C9A3.9040403@atmel.com> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 8bit X-Originating-IP: [10.161.30.18] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Mans, Le 11/01/2016 16:26, Nicolas Ferre a écrit : > Le 08/01/2016 01:11, Mans Rullgard a écrit : [...] >> @@ -1569,13 +1576,6 @@ static int atmel_spi_probe(struct platform_device *pdev) >> >> atmel_get_caps(as); >> >> - as->use_cs_gpios = true; >> - if (atmel_spi_is_v2(as) && > > I don't see this atmel_spi_is_v2() test in the resulting code anymore: > did you make sure that the v1 of the peripheral is protected? > >> - !of_get_property(pdev->dev.of_node, "cs-gpios", NULL)) { >> - as->use_cs_gpios = false; >> - master->num_chipselect = 4; > > This line is pretty important: you mustn't remove it blindly! > > Actually, few lines above master->num_chipselect is initialized with: master->dev.of_node = pdev->dev.of_node; [...] master->num_chipselect = master->dev.of_node ? 0 : 4; So 0 in the case of DT support, 4 otherwise. Now looking at the source code of spi_register_master(), this function: 1 - calls of_spi_register_master(), which initializes master->num_chipselect: nb = of_gpio_name_count(np, "cs-gpios"); master->num_chipselect = max_t(int, nb, master->num_chipselect); if (nb == 0 || nb == -ENOENT) return 0; 2 - check master->num_chipselect with: if (master->num_chipselect == 0) return -EINVAL; It means that in the case of DT support, master->num_chipselect was initialized according to the value of the "cs-gpios" DT property. Hence this property used to be mandatory but now we want it to be optional for hardware version 2 and later. This is why we added a check "hw version >= 2 and cs-gpios missing". In such a case the master->num_chipselect was initialized to 4 so spi_register_master() does not fail with -EINVAL. I have applied the following fix after your patch and now the driver works again on sama5d2: --- a/drivers/spi/spi-atmel.c +++ b/drivers/spi/spi-atmel.c @@ -1576,6 +1576,11 @@ static int atmel_spi_probe(struct platform_device *pdev) atmel_get_caps(as); + if (atmel_spi_is_v2(as) && + master->dev.of_node && + !of_get_property(master->dev.of_node, "cs-gpios", NULL)) + master->num_chipselect = 4; + as->use_dma = false; as->use_pdc = false; if (as->caps.has_dma_support) { >> - } >> - >> as->use_dma = false; >> as->use_pdc = false; >> if (as->caps.has_dma_support) { >> > > So, sorry but it's a NACK for me. > > Bye, > Best regards, Cyrille