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

[Peng Ma]

Hi Simon,

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

Best Regards,
Peng
>-----Original Message-----
>From: Simon Glass <sjg at chromium.org>
>Sent: 2019年12月28日 10:27
>To: Peng Ma <peng.ma at nxp.com>
>Cc: Priyanka Jain <priyanka.jain at nxp.com>; Marcel Ziswiler
><marcel.ziswiler at toradex.com>; Andy Tang <andy.tang at nxp.com>;
>u-boot at 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 at 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 at 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