From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nikita Kiryanov Date: Thu, 02 Oct 2014 13:28:47 +0300 Subject: [U-Boot] [PATCH v3 08/11] dm: imx: Use gpio_request() to request GPIOs In-Reply-To: References: <1410966166-20767-1-git-send-email-sjg@chromium.org> <1410966166-20767-9-git-send-email-sjg@chromium.org> <542BEC4A.1030603@compulab.co.il> Message-ID: <542D28DF.4040705@compulab.co.il> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Simon, On 01/10/14 18:22, Simon Glass wrote: > Hi Nikita, > > On 1 October 2014 05:58, Nikita Kiryanov wrote: >> Hi Simon, >> >> On 17/09/14 18:02, Simon Glass wrote: >>> >>> GPIOs should be requested before use. Without this, driver model will not >>> permit the GPIO to be used. >>> >>> Signed-off-by: Simon Glass >> >> >> This patch introduces a bunch of errors (once the driver model stuff is >> turned on), all related to the gpios never being freed, but requested >> anew when reinitializing subsystems. >> >> The errors are: >> >> CM-FX6 # sata init >> Warning: iSSD setup failed! >> AHCI 0001.0300 32 slots 1 ports 3 Gbps 0x1 impl SATA mode >> flags: ncq stag pm led clo only pmp pio slum part >> SATA Device Info: >> S/N: 123900127157 >> Product model number: SanDisk SSD i100 8GB >> Firmware version: 11.56.00 >> Capacity: 15649200 sectors > > Is it correct to init something twice? Of course. A re-init is a common use case, not just for sata, and anyway the reason for a failed re-init shouldn't be because sata code is preventing itself from utilizing its own GPIOs. > It has already been done when > U-Boot boots I think. If it is, then are you thinking of changing > cm_fx6_setup_issd() to cope with that? Yes. > >> >> CM-FX6 # usb start >> (Re)start USB... >> USB0: USB OTG pwr gpio request failed: -16 >> USB OTG pwr gpio request failed: -16 >> USB EHCI 1.00 >> scanning bus 0 for devices... 2 USB Device(s) found >> USB1: USB hub rst gpio request failed: -16 >> USB hub rst gpio request failed: -16 >> USB EHCI 1.00 >> scanning bus 1 for devices... 6 USB Device(s) found >> scanning usb for storage devices... max USB Storage Device reached: 5 >> stopping >> 5 Storage Device(s) found >> >> CM-FX6 # sf probe >> mxc_spi: cannot setup gpio -16 >> SF: Failed to set up slave >> Failed to initialize SPI flash at 0:0 > > I took at look at how this works for SPI. The approach of calling a > board function to find out the GPIO should go away with driver model - > we can use device tree, or platform data if that is not yet available. > > Also if you change the board code to 'stash' the GPIO and not request > it a second time, you will need to do that in every board. It seems > better to me to make this change in the driver, at least for SPI. There are benefits to stashing (or a better word would be 'pre-claiming') it in board code. If a gpio is necessary for SPI to work, it is not really meant to be used by other users, even if it's not immediately requested on boot. Pre-claiming it in board code enforces this. > board_spi_cs_gpio() was added very recently in commit 155fa9af9. Yes, that is one of my patches :) > If you change it so that setup_cs_gpio() remembers the GPIO after calling > board_spi_cs_gpio() then we can avoid passing the problem on to > boards. I would like to revisit this debate after the SPI driver is converted to driver model. For now I favour the pre-claiming in board code approach. > >> >> CM-FX6 # saveenv >> Saving Environment to SPI Flash... >> mxc_spi: cannot setup gpio -16 >> SF: Failed to set up slave >> *** Warning - spi_flash_probe() failed, using default environment > > Same issue. > >> >> I am going to submit a modified version of the cm_fx6 patches to address >> these problems. > > I think those are already merged - I based my patches on your cm_fx6 > patches and there were already in mainline. I was actually referring to a modified version of your patches that touches cm_fx6 code. That is, take your changes as follows: imx: Add error checking to setup_i2c() (sans the issd stuff) dm: imx: Use gpio_request() to request GPIOs (just the i2c-mxv7.c stuff) and add a new patch that refactors cm_fx6 init stuff. > >> >> -- >> Regards, >> Nikita Kiryanov > > Regards, > Simon > -- Regards, Nikita Kiryanov