Hi Deepak,
Thanks for the patch, comment inline.
On Mon, Jun 24, 2019 at 10:22 AM Deepak Ukey <deepak.ukey@xxxxxxxxxxxxx> wrote:
>
> Disabling the SATA drive interface cause kernel panic. When the drive
> Interface is disabled, device should be deregistered after aborting
> all pending IO's.
Can you share the call trace for the panic?
>
> Signed-off-by: Deepak Ukey <deepak.ukey@xxxxxxxxxxxxx>
> Signed-off-by: Viswas G <Viswas.G@xxxxxxxxxxxxx>
> ---
> drivers/scsi/pm8001/pm8001_sas.c | 6 +++++-
> drivers/scsi/pm8001/pm80xx_hwi.c | 2 +-
> drivers/scsi/pm8001/pm80xx_hwi.h | 1 +
> 3 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c
> index 88eef3b..0d680f3 100644
> --- a/drivers/scsi/pm8001/pm8001_sas.c
> +++ b/drivers/scsi/pm8001/pm8001_sas.c
> @@ -889,6 +889,8 @@ static void pm8001_dev_gone_notify(struct domain_device *dev)
> pm8001_exec_internal_task_abort(pm8001_ha, pm8001_dev ,
> dev, 1, 0);
> spin_lock_irqsave(&pm8001_ha->lock, flags);
> + while (pm8001_dev->running_req)
> + msleep(20);
We are holding spin lock, and sleep, are you sure it's right?
> }
> PM8001_CHIP_DISP->dereg_dev_req(pm8001_ha, device_id);
> pm8001_free_dev(pm8001_dev);
> @@ -1256,8 +1258,10 @@ int pm8001_abort_task(struct sas_task *task)
> PM8001_MSG_DBG(pm8001_ha,
> pm8001_printk("Waiting for Port reset\n"));
> wait_for_completion(&completion_reset);
> - if (phy->port_reset_status)
> + if (phy->port_reset_status) {
> + pm8001_dev_gone_notify(dev);
> goto out;
> + }
>
> /*
> * 4. SATA Abort ALL
> diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c
> index 301de40..63e8af7 100644
> --- a/drivers/scsi/pm8001/pm80xx_hwi.c
> +++ b/drivers/scsi/pm8001/pm80xx_hwi.c
> @@ -604,7 +604,7 @@ static void update_main_config_table(struct pm8001_hba_info *pm8001_ha)
> pm8001_ha->main_cfg_tbl.pm80xx_tbl.port_recovery_timer &=
> 0x0000ffff;
> pm8001_ha->main_cfg_tbl.pm80xx_tbl.port_recovery_timer |=
> - 0x140000;
> + CHIP_8006_PORT_RECOVERY_TIMEOUT;
> }
> pm8001_mw32(address, MAIN_PORT_RECOVERY_TIMER,
> pm8001_ha->main_cfg_tbl.pm80xx_tbl.port_recovery_timer);
> diff --git a/drivers/scsi/pm8001/pm80xx_hwi.h b/drivers/scsi/pm8001/pm80xx_hwi.h
> index 84d7426..a22bb4d 100644
> --- a/drivers/scsi/pm8001/pm80xx_hwi.h
> +++ b/drivers/scsi/pm8001/pm80xx_hwi.h
> @@ -230,6 +230,7 @@
> #define SAS_MAX_AIP 0x200000
> #define IT_NEXUS_TIMEOUT 0x7D0
> #define PORT_RECOVERY_TIMEOUT ((IT_NEXUS_TIMEOUT/100) + 30)
> +#define CHIP_8006_PORT_RECOVERY_TIMEOUT 0x640000
This value is 4x bigger than the original, is it a intended, can you
mention is in commit message why?
>
> #ifdef __LITTLE_ENDIAN_BITFIELD
> struct sas_identify_frame_local {
> --
> 1.8.5.6
>
Regards,
Jack
