Re: [PATCH V3 11/16] scsi: ufs: refactor configuring power mode

2014-09-15 Thread Dolev Raviv

> 2014-09-10 20:54 GMT+09:00 Dolev Raviv :
>> +static int ufshcd_config_pwr_mode(struct ufs_hba *hba,
>> +   struct ufs_pa_layer_attr *desired_pwr_mode)
>> +{
>> +   struct ufs_pa_layer_attr final_params = { 0 };
>> +   int ret;
>> +
>> +   if (hba->vops->pwr_change_notify)
>
> If hba->vops is null as no vendor specific callbacks, this causes
> a null pointer dereference.  So null check for hba->vops is also needed
> just like other callbacks.

Sure, will fix both.

>
>> +   hba->vops->pwr_change_notify(hba,
>> +PRE_CHANGE, desired_pwr_mode, &final_params);
>> +   else
>> +   memcpy(&final_params, desired_pwr_mode,
>> sizeof(final_params));
>> +
>> /*
>>  * Configure attributes for power mode change with below.
>>  * - PA_RXGEAR, PA_ACTIVERXDATALANES, PA_RXTERMINATION,
>>  * - PA_TXGEAR, PA_ACTIVETXDATALANES, PA_TXTERMINATION,
>>  * - PA_HSSERIES
>>  */
>> -   ufshcd_dme_set(hba, UIC_ARG_MIB(PA_RXGEAR), gear[RX]);
>> -   ufshcd_dme_set(hba, UIC_ARG_MIB(PA_ACTIVERXDATALANES),
>> lanes[RX]);
>> -   if (pwr[RX] == FASTAUTO_MODE)
>> +   ufshcd_dme_set(hba, UIC_ARG_MIB(PA_RXGEAR),
>> final_params.gear_rx);
>> +   ufshcd_dme_set(hba, UIC_ARG_MIB(PA_ACTIVERXDATALANES),
>> +   final_params.lane_rx);
>> +   if (final_params.pwr_rx == FASTAUTO_MODE ||
>> +   final_params.pwr_rx == FAST_MODE)
>> ufshcd_dme_set(hba, UIC_ARG_MIB(PA_RXTERMINATION),
>> TRUE);
>> +   else
>> +   ufshcd_dme_set(hba, UIC_ARG_MIB(PA_RXTERMINATION),
>> FALSE);
>>
>> -   ufshcd_dme_set(hba, UIC_ARG_MIB(PA_TXGEAR), gear[TX]);
>> -   ufshcd_dme_set(hba, UIC_ARG_MIB(PA_ACTIVETXDATALANES),
>> lanes[TX]);
>> -   if (pwr[TX] == FASTAUTO_MODE)
>> +   ufshcd_dme_set(hba, UIC_ARG_MIB(PA_TXGEAR),
>> final_params.gear_tx);
>> +   ufshcd_dme_set(hba, UIC_ARG_MIB(PA_ACTIVETXDATALANES),
>> +   final_params.lane_tx);
>> +   if (final_params.pwr_tx == FASTAUTO_MODE ||
>> +   final_params.pwr_tx == FAST_MODE)
>> ufshcd_dme_set(hba, UIC_ARG_MIB(PA_TXTERMINATION),
>> TRUE);
>> -
>> -   if (pwr[RX] == FASTAUTO_MODE || pwr[TX] == FASTAUTO_MODE)
>> -   ufshcd_dme_set(hba, UIC_ARG_MIB(PA_HSSERIES),
>> PA_HS_MODE_B);
>> -
>> -   ret = ufshcd_uic_change_pwr_mode(hba, pwr[RX] << 4 | pwr[TX]);
>> -   if (ret)
>> +   else
>> +   ufshcd_dme_set(hba, UIC_ARG_MIB(PA_TXTERMINATION),
>> FALSE);
>> +
>> +   if ((final_params.pwr_rx == FASTAUTO_MODE ||
>> +   final_params.pwr_tx == FASTAUTO_MODE ||
>> +   final_params.pwr_rx == FAST_MODE ||
>> +   final_params.pwr_tx == FAST_MODE) &&
>> +   final_params.hs_rate == PA_HS_MODE_B)
>> +   ufshcd_dme_set(hba, UIC_ARG_MIB(PA_HSSERIES),
>> +   final_params.hs_rate);
>> +
>> +   ret = ufshcd_uic_change_pwr_mode(hba, final_params.pwr_rx << 4
>> +   | final_params.pwr_tx);
>> +   if (ret) {
>> dev_err(hba->dev,
>> "pwr_mode: power mode change failed %d\n", ret);
>> +   } else {
>> +   if (hba->vops->pwr_change_notify)
>
> Ditto.
>
>> +   hba->vops->pwr_change_notify(hba,
>> +   POST_CHANGE, NULL, &final_params);
>> +
>> +   memcpy(&hba->pwr_info, &final_params,
>> sizeof(final_params));
>> +   }
> --
> 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
>


-- 
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation


--
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


Re: [PATCH V3 11/16] scsi: ufs: refactor configuring power mode

2014-09-11 Thread Akinobu Mita
2014-09-10 20:54 GMT+09:00 Dolev Raviv :
> +static int ufshcd_config_pwr_mode(struct ufs_hba *hba,
> +   struct ufs_pa_layer_attr *desired_pwr_mode)
> +{
> +   struct ufs_pa_layer_attr final_params = { 0 };
> +   int ret;
> +
> +   if (hba->vops->pwr_change_notify)

If hba->vops is null as no vendor specific callbacks, this causes
a null pointer dereference.  So null check for hba->vops is also needed
just like other callbacks.

> +   hba->vops->pwr_change_notify(hba,
> +PRE_CHANGE, desired_pwr_mode, &final_params);
> +   else
> +   memcpy(&final_params, desired_pwr_mode, sizeof(final_params));
> +
> /*
>  * Configure attributes for power mode change with below.
>  * - PA_RXGEAR, PA_ACTIVERXDATALANES, PA_RXTERMINATION,
>  * - PA_TXGEAR, PA_ACTIVETXDATALANES, PA_TXTERMINATION,
>  * - PA_HSSERIES
>  */
> -   ufshcd_dme_set(hba, UIC_ARG_MIB(PA_RXGEAR), gear[RX]);
> -   ufshcd_dme_set(hba, UIC_ARG_MIB(PA_ACTIVERXDATALANES), lanes[RX]);
> -   if (pwr[RX] == FASTAUTO_MODE)
> +   ufshcd_dme_set(hba, UIC_ARG_MIB(PA_RXGEAR), final_params.gear_rx);
> +   ufshcd_dme_set(hba, UIC_ARG_MIB(PA_ACTIVERXDATALANES),
> +   final_params.lane_rx);
> +   if (final_params.pwr_rx == FASTAUTO_MODE ||
> +   final_params.pwr_rx == FAST_MODE)
> ufshcd_dme_set(hba, UIC_ARG_MIB(PA_RXTERMINATION), TRUE);
> +   else
> +   ufshcd_dme_set(hba, UIC_ARG_MIB(PA_RXTERMINATION), FALSE);
>
> -   ufshcd_dme_set(hba, UIC_ARG_MIB(PA_TXGEAR), gear[TX]);
> -   ufshcd_dme_set(hba, UIC_ARG_MIB(PA_ACTIVETXDATALANES), lanes[TX]);
> -   if (pwr[TX] == FASTAUTO_MODE)
> +   ufshcd_dme_set(hba, UIC_ARG_MIB(PA_TXGEAR), final_params.gear_tx);
> +   ufshcd_dme_set(hba, UIC_ARG_MIB(PA_ACTIVETXDATALANES),
> +   final_params.lane_tx);
> +   if (final_params.pwr_tx == FASTAUTO_MODE ||
> +   final_params.pwr_tx == FAST_MODE)
> ufshcd_dme_set(hba, UIC_ARG_MIB(PA_TXTERMINATION), TRUE);
> -
> -   if (pwr[RX] == FASTAUTO_MODE || pwr[TX] == FASTAUTO_MODE)
> -   ufshcd_dme_set(hba, UIC_ARG_MIB(PA_HSSERIES), PA_HS_MODE_B);
> -
> -   ret = ufshcd_uic_change_pwr_mode(hba, pwr[RX] << 4 | pwr[TX]);
> -   if (ret)
> +   else
> +   ufshcd_dme_set(hba, UIC_ARG_MIB(PA_TXTERMINATION), FALSE);
> +
> +   if ((final_params.pwr_rx == FASTAUTO_MODE ||
> +   final_params.pwr_tx == FASTAUTO_MODE ||
> +   final_params.pwr_rx == FAST_MODE ||
> +   final_params.pwr_tx == FAST_MODE) &&
> +   final_params.hs_rate == PA_HS_MODE_B)
> +   ufshcd_dme_set(hba, UIC_ARG_MIB(PA_HSSERIES),
> +   final_params.hs_rate);
> +
> +   ret = ufshcd_uic_change_pwr_mode(hba, final_params.pwr_rx << 4
> +   | final_params.pwr_tx);
> +   if (ret) {
> dev_err(hba->dev,
> "pwr_mode: power mode change failed %d\n", ret);
> +   } else {
> +   if (hba->vops->pwr_change_notify)

Ditto.

> +   hba->vops->pwr_change_notify(hba,
> +   POST_CHANGE, NULL, &final_params);
> +
> +   memcpy(&hba->pwr_info, &final_params, sizeof(final_params));
> +   }
--
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