Subject: Re: [PATCH V3 06/25] smartpqi: add support for BMIC sense feature cmd and feature bits
In general: This patch contains a lot of whitespace, indentation, and minor comment formatting changes which should rather go into a separate patch IMHO. This one is big enough without them.
Don: Moved formatting changes into patch smartpqi-align-code-with-oob-driver
Further remarks below.
> [...]
>
> @@ -2552,7 +2686,7 @@ static int
> pqi_raid_bypass_submit_scsi_cmd(struct pqi_ctrl_info *ctrl_info,
> u32 next_bypass_group;
> struct pqi_encryption_info *encryption_info_ptr;
> struct pqi_encryption_info encryption_info;
> - struct pqi_scsi_dev_raid_map_data rmd = {0};
> + struct pqi_scsi_dev_raid_map_data rmd = { 0 };
>
>
> if (get_unaligned_le16(&raid_map->flags) &
> - RAID_MAP_ENCRYPTION_ENABLED) {
> + RAID_MAP_ENCRYPTION_ENABLED) {
> + if (rmd.data_length > device->max_transfer_encrypted)
> + return PQI_RAID_BYPASS_INELIGIBLE;
> pqi_set_encryption_info(&encryption_info, raid_map,
> rmd.first_block);
> encryption_info_ptr = &encryption_info; @@ -2623,10
> +2759,6 @@ static int pqi_raid_bypass_submit_scsi_cmd(struct
> pqi_ctrl_info *ctrl_info,
>
This hunk is fine, but AFAICS it doesn't belong here logically, it should rather be part of patch 04 and 05.
Don: The patch adds max_transfer_encrypted field as part of new feature support. We would like to leave the update in this patch.
> @@ static int pqi_ctrl_init_resume(struct pqi_ctrl_info *ctrl_info)
>
> pqi_start_heartbeat_timer(ctrl_info);
>
> + if (ctrl_info->enable_r5_writes || ctrl_info-
> >enable_r6_writes) {
> + rc = pqi_get_advanced_raid_bypass_config(ctrl_info);
> + if (rc) {
> + dev_err(&ctrl_info->pci_dev->dev,
> + "error obtaining advanced RAID bypass
> configuration\n");
> + return rc;
Do you need to error out here ? Can't you simply unset the enable_rX_writes feature?
This function should never fail, so a failure indicates a serious problem. But we're considering some changes in that area that we may push up at a later date.
Regards
Martin
