On 2016-10-06 18:16, Kiwoong Kim wrote:
Hi, Subhash.
Thanks Kim for the response.
On 2016-10-06 03:28, Kiwoong Kim wrote:
> Hi, Subhash.
>
> Some UFS host controllers may need to call the vendor specific
> callback before and after controlling by clock control framework,
> regardless of whether available clocks are turned on or off.
Are you suggesting to call ufshcd_vops_setup_clocks() 2 times, one
before
the on/off by ufshcd core driver and one after the on/off? If yes,
then we
also have add 3rd argument clarifying if this is PRE_CHANGE or
POST_CHANGE.
>
> Is there any special reason to limit to invoke the callback
> only when the clocks are turned on or not?
>
> Besides, the callback is acknowledged from core driver
> because 2nd argument is whether the clocks are turned on or not.
>
> If you have any other idea, please let me know.
This is my suggestion:
1. Add 3rd argument to setup_clocks ops to let the vendor callback
function know if this is called PRE_CHANGE or POST_CHANGE.
2. If #1 is in place, call setup_clocks 2 times, one with PRE_CHANGE
argument before making any clock changes in core driver, 2nd with
POST_CHANGE argument after making the clock changes to core driver.
Yes, that's exactly the same what I mean.
Thanks, Will send out new version adding that.
Thanks
Let me know if this would work or not.
Thanks,
Subhash
>
> Thanks
> Regards
>
>> Vendor specific setup_clocks callback may require the clocks managed
>> by ufshcd driver to be ON. So if the vendor specific setup_clocks
>> callback
>> is called while the required clocks are turned off, it could result
>> into
>> unclocked register access.
>>
>> To prevent possible unclock register access, this change makes sure
>> that
>> required clocks remain enabled before calling into vendor specific
>> setup_clocks callback.
>>
>> Signed-off-by: Subhash Jadavani <subha...@codeaurora.org>
>> ---
>> Changes from v2:
>> * Don't call ufshcd_vops_setup_clocks() again for clock off
>> ---
>> drivers/scsi/ufs/ufshcd.c | 22 +++++++++++++++++++++-
>> 1 file changed, 21 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
>> index 05c7456..c1a77d3 100644
>> --- a/drivers/scsi/ufs/ufshcd.c
>> +++ b/drivers/scsi/ufs/ufshcd.c
>> @@ -5389,6 +5389,17 @@ static int __ufshcd_setup_clocks(struct ufs_hba
>> *hba, bool on,
>> if (!head || list_empty(head))
>> goto out;
>>
>> + /*
>> + * vendor specific setup_clocks ops may depend on clocks managed by
>> + * this standard driver hence call the vendor specific setup_clocks
>> + * before disabling the clocks managed here.
>> + */
>> + if (!on) {
>> + ret = ufshcd_vops_setup_clocks(hba, on);
>> + if (ret)
>> + return ret;
>> + }
>> +
>> list_for_each_entry(clki, head, list) {
>> if (!IS_ERR_OR_NULL(clki->clk)) {
>> if (skip_ref_clk && !strcmp(clki->name, "ref_clk"))
>> @@ -5410,7 +5421,16 @@ static int __ufshcd_setup_clocks(struct ufs_hba
>> *hba, bool on,
>> }
>> }
>>
>> - ret = ufshcd_vops_setup_clocks(hba, on);
>> + /*
>> + * vendor specific setup_clocks ops may depend on clocks managed by
>> + * this standard driver hence call the vendor specific setup_clocks
>> + * after enabling the clocks managed here.
>> + */
>> + if (on) {
>> + ret = ufshcd_vops_setup_clocks(hba, on);
>> + if (ret)
>> + return ret;
>> + }
>> out:
>> if (ret) {
>> list_for_each_entry(clki, head, list) {
>> --
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
>> Forum,
>> a Linux Foundation Collaborative Project
>>
>> --
>> 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
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
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
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
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
