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

[Christopher Spinrath]

Hi Marek,

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.

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...

Cheers,
Christopher

> -
> -	for (i = 0; i < CM_FX6_SATA_INIT_RETRIES; i++) {
> -		err = setup_sata();
> -		if (err) {
> -			printf("SATA setup failed: %d\n", err);
> -			return err;
> -		}
> -
> -		udelay(100);
> -
> -		err = dwc_ahsata_probe(dev);
> -		if (!err)
> -			break;
> -
> -		/* There is no device on the SATA port */
> -		if (sata_dm_port_status(0, 0) == 0)
> -			break;
> -
> -		/* There's a device, but link not established. Retry */
> -		device_remove(dev, DM_REMOVE_NORMAL);
> -	}
> -
> -	return 0;
> -}
> -
> -static int sata_imx_remove(struct udevice *dev)
> -{
> -	cm_fx6_sata_power(0);
> -	mdelay(250);
> -
> -	return 0;
> -}
> -
> -struct ahci_ops sata_imx_ops = {
> -	.port_status = dwc_ahsata_port_status,
> -	.reset	= dwc_ahsata_bus_reset,
> -	.scan	= dwc_ahsata_scan,
> -};
> -
> -static const struct udevice_id sata_imx_ids[] = {
> -	{ .compatible = "fsl,imx6q-ahci" },
> -	{ }
> -};
> -
> -U_BOOT_DRIVER(sata_imx) = {
> -	.name		= "dwc_ahci",
> -	.id		= UCLASS_AHCI,
> -	.of_match	= sata_imx_ids,
> -	.ops		= &sata_imx_ops,
> -	.probe		= sata_imx_probe,
> -	.remove		= sata_imx_remove,  /* reset bus to stop it */
> -};
> -#endif /* AHCI */
> diff --git a/configs/cm_fx6_defconfig b/configs/cm_fx6_defconfig
> index ce3f9de3f9..e928cbc948 100644
> --- a/configs/cm_fx6_defconfig
> +++ b/configs/cm_fx6_defconfig
> @@ -52,7 +52,6 @@ CONFIG_DEFAULT_DEVICE_TREE="imx6q-cm-fx6"
>  CONFIG_ENV_IS_IN_SPI_FLASH=y
>  CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG=y
>  CONFIG_DWC_AHSATA=y
> -# CONFIG_DWC_AHSATA_AHCI is not set
>  CONFIG_DM_KEYBOARD=y
>  CONFIG_DM_MMC=y
>  CONFIG_FSL_ESDHC=y
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190514/b77de23a/attachment.sig>