[U-Boot] [PATCH v3 08/11] dm: imx: Use gpio_request() to request GPIOs

[Nikita Kiryanov]

Hi Simon,

On 01/10/14 18:22, Simon Glass wrote:
> Hi Nikita,
>
> On 1 October 2014 05:58, Nikita Kiryanov <nikita at 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 at 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
>

-- 
Regards,
Nikita Kiryanov