[U-Boot] [PATCH] ARM: imx: cm_fx6: Drop ad-hoc SATA binding

[Marek Vasut]

On 5/14/19 12:28 AM, Christopher Spinrath wrote:
> Hi Marek,

Hi,

> thanks for the patch!
> 
> Unfortunately, I am unable to test this patch right now, because both
> U-Boot v2019.04 and v2019.07-rc fail to boot on my (cm-fx6 based)
> Utilite Pro...
> I will debug this next weekend.

Good, please take over this patch as well. I don't even have the board.

> However, I already have one comment inline:
> 
> On 5/12/19 10:43 PM, Marek Vasut wrote:
>> Drop the ad-hoc AHCI binding code, this is superseded by
>> CONFIG_DWC_AHSATA_AHCI=y resp. drivers/ata/dwc_ahsata.c
>>
>> Signed-off-by: Marek Vasut <marex at denx.de>
>> Cc: Christopher Spinrath <christopher.spinrath at rwth-aachen.de>
>> Cc: Fabio Estevam <fabio.estevam at nxp.com>
>> Cc: Igor Grinberg <grinberg at compulab.co.il>
>> Cc: Nikita Kiryanov <nikita at compulab.co.il>
>> Cc: Stefano Babic <sbabic at denx.de>
>> ---
>>  board/compulab/cm_fx6/cm_fx6.c | 63 ----------------------------------
>>  configs/cm_fx6_defconfig       |  1 -
>>  2 files changed, 64 deletions(-)
>>
>> diff --git a/board/compulab/cm_fx6/cm_fx6.c b/board/compulab/cm_fx6/cm_fx6.c
>> index d42f57d4b7..b8f15cf3ab 100644
>> --- a/board/compulab/cm_fx6/cm_fx6.c
>> +++ b/board/compulab/cm_fx6/cm_fx6.c
>> @@ -724,66 +724,3 @@ U_BOOT_DEVICE(cm_fx6_serial) = {
>>  	.name	= "serial_mxc",
>>  	.platdata = &cm_fx6_mxc_serial_plat,
>>  };
>> -
>> -#if CONFIG_IS_ENABLED(AHCI)
>> -static int sata_imx_probe(struct udevice *dev)
>> -{
>> -	int i, err;
>> -
>> -	/* Make sure this gpio has logical 0 value */
>> -	gpio_direction_output(CM_FX6_SATA_PWLOSS_INT, 0);
>> -	udelay(100);
>> -	cm_fx6_sata_power(1);
> 
> The removal of this call to cm_fx6_sata_power (and the gpio
> configuration) worries me a bit.
> 
> *To my understanding* the SATA controller pins on the cm-fx6 module can
> be routed either
> 
> 1) directly to the daughterboard, which is then responsible for
> everything else (e.g. powering the drive); or
> 
> 2) to a drive soldered on the cm-fx6 module itself. In this case the
> power sequence implemented in cm_fx6_data_power is necessary to power up
> the drive.
> 
> While this patch is probably fine for modules using configuration 1), I
> think, it will cause a regression for modules using configuration 2).
> 
> Can we move the call to cm_fx6_sata_power to somewhere else? (I have no
> idea, if there is a proper callback/hook/...)
> 
> Optimally, we would also keep the call cm_fx6_sata_power(0) below, in
> case no drive is detected (to save some power), but well...

You probably want to describe a regulator in the boards' DT to handle
this special power sequencing requirement.

-- 
Best regards,
Marek Vasut