RE: [EXT] Re: [PATCH 1/3] ata: sata_sil: Continue probing other sata port when failed current port.

Hi Simon,

I am very sorry to reply late, Please see comments inline.

Best Regards, Peng

-----Original Message----- From: Simon Glass sjg@chromium.org Sent: 2019年12月28日 10:27 To: Peng Ma peng.ma@nxp.com Cc: Priyanka Jain priyanka.jain@nxp.com; Marcel Ziswiler marcel.ziswiler@toradex.com; Andy Tang andy.tang@nxp.com; u-boot@lists.denx.de Subject: [EXT] Re: [PATCH 1/3] ata: sata_sil: Continue probing other sata port when failed current port.

Caution: EXT Email

Hi Peng,

On Wed, 4 Dec 2019 at 03:36, Peng Ma peng.ma@nxp.com wrote:

...

In the initialization of sata driver, we want to initialize all port probes, Therefore, any detection failure between of them should continue initialization by skipping the current port instead of exit.

Signed-off-by: Peng Ma peng.ma@nxp.com

drivers/ata/sata_sil.c | 60 +++++++++++++++++++++++++++++++++++++++--- 1 file changed, 56 insertions(+), 4 deletions(-)

diff --git a/drivers/ata/sata_sil.c b/drivers/ata/sata_sil.c index d06d7a079d..bbba98f9a6 100644 --- a/drivers/ata/sata_sil.c +++ b/drivers/ata/sata_sil.c @@ -19,6 +19,7 @@ #if CONFIG_IS_ENABLED(BLK) #include <dm.h> #include <blk.h> +#include <dm/device-internal.h> #endif

#include "sata_sil.h" @@ -762,15 +763,33 @@ U_BOOT_DRIVER(sata_sil_driver) = { .platdata_auto_alloc_size = sizeof(struct sil_sata_priv), };

+static int (struct udevice *dev) +{

•   int ret;
  

•   ret = device_remove(dev, DM_REMOVE_NORMAL);
  

•   if (ret)
  

•           return ret;
  

•   ret = device_unbind(dev);
  

Why are you unbinding the devices? I don't think this is needed.

[Peng Ma] Before sil_init_sata function failed the function blk_create_devicef have already be called, In the blk_create_devicef function it would be bound the silicon sata to block device, So we should unbind this device here once the function sil_init_sata return failed.

...

•   if (ret)
  

•           return ret;
  

•   return 0;
  

+}

static int sil_pci_probe(struct udevice *dev) { struct udevice *blk;

•   int failed_number;
    char sata_name[10];
    pci_dev_t devno;
    u16 word;
    int ret;
    int i;
  
  

•   failed_number = 0;
  

•   /* Get PCI device number */
    devno = dm_pci_get_bdf(dev);
    if (devno == -1)
  

@@ -823,12 +842,44 @@ static int sil_pci_probe(struct udevice *dev) }

            ret = sil_init_sata(blk, i);

•           if (ret)
  

•                   return -ENODEV;
  

•           if (ret) {
  

•                   ret = sil_unbind_device(blk);
  

•                   if (ret)
  

•                           return ret;
  

•                   failed_number++;
  

•                   continue;
  

•           }
  
            ret = scan_sata(blk, i);
  

•           if (ret)
  

•                   return -ENODEV;
  

•           if (ret) {
  

•                   ret = sil_unbind_device(blk);
  

•                   if (ret)
  

•                           return ret;
  

•                   failed_number++;
  

•                   continue;
  

•           }
  

•   }
  

•   if (failed_number == sata_info.maxport)
  

•           return -ENODEV;
  

•   else
  

•           return 0;
  

+}

+static int sil_pci_remove(struct udevice *dev) {

•   int i;
  

•   struct sil_sata *sata;
  

•   struct sil_sata_priv *priv;
  

•   priv = dev_get_priv(dev);
  

•   for (i = sata_info.portbase; i < sata_info.maxport; i++) {
  

•           sata = priv->sil_sata_desc[i];
  

•           if (sata)
  

•                   free(sata);
    }
  
    return 0;
  

@@ -856,6 +907,7 @@ U_BOOT_DRIVER(sil_ahci_pci) = { .of_match = sil_pci_ids, .ops = &sata_sil_ops, .probe = sil_pci_probe,

•   .remove = sil_pci_remove,
    .priv_auto_alloc_size = sizeof(struct sil_sata_priv),  };
  
  

-- 2.17.1

Regards, Simon