On 17.10.2016 15:28, Sumit Saxena wrote:
>> -----Original Message-----
>> From: Tomas Henzl [mailto:the...@redhat.com]
>> Sent: Monday, October 17, 2016 6:44 PM
>> To: Sumit Saxena; linux-scsi@vger.kernel.org
>> Cc: martin.peter...@oracle.com; j...@linux.vnet.ibm.com;
>> kashyap.de...@broadcom.com
>> Subject: Re: [PATCH 4/7] megaraid_sas: Send SYNCHRONIZE_CACHE command to
>> firmware
>>
>> On 17.10.2016 12:24, Sumit Saxena wrote:
>>> megaraid_sas driver returns SYNCHRONIZE_CACHE command back to SCSI
>>> layer without sending it to firmware as firmware takes care of flushing
> cache.
>>> This patch will change the driver behavior wrt SYNCHRONIZE_CACHE
> handling.
>>> If underlying firmware has support to handle the SYNCHORNIZE_CACHE,
>>> driver will send it for firmware otherwise complete it back to SCSI
>>> layer with SUCCESS immediately.
>>> If  Firmware  handle SYNCHORNIZE_CACHE for both VD and JBOD
>>> "canHandleSyncCache" bit in scratch pad register(offset 0x00B4) will
>>> be set.
>>>
>>> This behavior can be controlled via module parameter and user can
>>> fallback to old behavior of returning SYNCHRONIZE_CACHE by driver only
>>> without sending it to firmware.
>>>
>>> Signed-off-by: Sumit Saxena <sumit.sax...@broadcom.com>
>>> Signed-off-by: Kashyap Desai <kashyap.de...@broadcom.com>
>>> ---
>>>  drivers/scsi/megaraid/megaraid_sas.h        |  3 +++
>>>  drivers/scsi/megaraid/megaraid_sas_base.c   | 14 ++++++--------
>>>  drivers/scsi/megaraid/megaraid_sas_fusion.c |  3 +++
>>> drivers/scsi/megaraid/megaraid_sas_fusion.h |  2 ++
>>>  4 files changed, 14 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/scsi/megaraid/megaraid_sas.h
>>> b/drivers/scsi/megaraid/megaraid_sas.h
>>> index ca86c88..43fd14f 100644
>>> --- a/drivers/scsi/megaraid/megaraid_sas.h
>>> +++ b/drivers/scsi/megaraid/megaraid_sas.h
>>> @@ -1429,6 +1429,8 @@ enum FW_BOOT_CONTEXT {
>>>  #define MR_MAX_REPLY_QUEUES_EXT_OFFSET_SHIFT    14
>>>  #define MR_MAX_MSIX_REG_ARRAY                   16
>>>  #define MR_RDPQ_MODE_OFFSET                        0X00800000
>>> +#define MR_CAN_HANDLE_SYNC_CACHE_OFFSET            0X01000000
>>> +
>>>  /*
>>>  * register set for both 1068 and 1078 controllers
>>>  * structure extended for 1078 registers @@ -2140,6 +2142,7 @@ struct
>>> megasas_instance {
>>>     u8 is_imr;
>>>     u8 is_rdpq;
>>>     bool dev_handle;
>>> +   bool fw_sync_cache_support;
>>>  };
>>>  struct MR_LD_VF_MAP {
>>>     u32 size;
>>> diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c
>>> b/drivers/scsi/megaraid/megaraid_sas_base.c
>>> index 092387f..a4a8e2f 100644
>>> --- a/drivers/scsi/megaraid/megaraid_sas_base.c
>>> +++ b/drivers/scsi/megaraid/megaraid_sas_base.c
>>> @@ -104,6 +104,10 @@ unsigned int scmd_timeout =
>>> MEGASAS_DEFAULT_CMD_TIMEOUT;  module_param(scmd_timeout, int,
>>> S_IRUGO);  MODULE_PARM_DESC(scmd_timeout, "scsi command timeout
>>> (10-90s), default 90s. See megasas_reset_timer.");
>>>
>>> +bool block_sync_cache;
>>> +module_param(block_sync_cache, bool, S_IRUGO);
>>> +MODULE_PARM_DESC(block_sync_cache, "Block SYNC CACHE by driver
>>> +Default: 0(Send it to firmware)");
>>> +
>>>  MODULE_LICENSE("GPL");
>>>  MODULE_VERSION(MEGASAS_VERSION);
>>>  MODULE_AUTHOR("megaraidlinux....@avagotech.com");
>>> @@ -1700,16 +1704,10 @@ megasas_queue_command(struct Scsi_Host
>> *shost, struct scsi_cmnd *scmd)
>>>             goto out_done;
>>>     }
>>>
>>> -   switch (scmd->cmnd[0]) {
>>> -   case SYNCHRONIZE_CACHE:
>>> -           /*
>>> -            * FW takes care of flush cache on its own
>>> -            * No need to send it down
>>> -            */
>>> +   if ((scmd->cmnd[0] == SYNCHRONIZE_CACHE) &&
>>> +           (!instance->fw_sync_cache_support)) {
>> Maybe I'm wrong, but isn't the logic inverted?
>> when fw_sync_cache_support is true  it means that there is a flash cache
> or a
>> battery and we don't have to send the command down ?
>>
> If "instance->fw_sync_cache_support" is set to true, it means driver can
> send SYNCHRONIZE_CACHE to firmware(firmware has infrastructure to support
> SYNCHRONIZE_CACHE).
> If "instance->fw_sync_cache_support" is set to false, it means FW does not
> support to handle SYNCHRONIZE_CACHE and FW will flush cache during
> shutdown.

Thanks, in that case it is correct.

>>> +   if (!block_sync_cache)
>>> +           instance->fw_sync_cache_support = (scratch_pad_2 &
>>> +                   MR_CAN_HANDLE_SYNC_CACHE_OFFSET) ? 1 : 0;

Please add a warning or log the state of the synchronise cache command
on the controller.


>>>     IOCInitMessage =
>>>       dma_alloc_coherent(&instance->pdev->dev,
>>> diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.h
>>> b/drivers/scsi/megaraid/megaraid_sas_fusion.h
>>> index e3bee04..2857154 100644
>>> --- a/drivers/scsi/megaraid/megaraid_sas_fusion.h
>>> +++ b/drivers/scsi/megaraid/megaraid_sas_fusion.h
>>> @@ -72,6 +72,8 @@
>>>  #define MPI2_SUP_REPLY_POST_HOST_INDEX_OFFSET   (0x0000030C)
>>>  #define MPI2_REPLY_POST_HOST_INDEX_OFFSET  (0x0000006C)
>>>
>>> +extern bool block_sync_cache;
>>> +
>>>  /*
>>>   * Raid context flags
>>>   */
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to