Hi Simon,
On 01/10/14 18:22, Simon Glass wrote:
Hi Nikita,
On 1 October 2014 05:58, Nikita Kiryanov nikita@compulab.co.il 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 sjg@chromium.org
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