Re: [PATCH 2/4] clk: tegra: check BPMP response return code

[Jon Hunter]

On 02/10/17 09:43, Timo Alho wrote:
> 
> 
> On 29.09.2017 17:53, Jonathan Hunter wrote:
>>
>> On 29/09/17 14:46, Timo Alho wrote:
>>> Hi Jon,
>>>
>>> On 21.09.2017 14:21, Jonathan Hunter wrote:
>>>>
>>>>
>>>> On 07/09/17 10:31, Timo Alho wrote:
>>>>> Check return code in BPMP response message(s). The typical error case
>>>>> is when clock operation is attempted with invalid clock identifier.
>>>>>
>>>>> Also remove error print from call to clk_get_info() as the
>>>>> implementation loops through range of all possible identifier, but the
>>>>> operation is expected error out when the clock id is unused.
>>>>>
>>>>> Signed-off-by: Timo Alho <talho@xxxxxxxxxx>
>>>>> ---
>>>>> ÂÂ drivers/clk/tegra/clk-bpmp.c | 15 ++++++++++-----
>>>>> ÂÂ 1 file changed, 10 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/drivers/clk/tegra/clk-bpmp.c
>>>>> b/drivers/clk/tegra/clk-bpmp.c
>>>>> index 638ace6..a896692 100644
>>>>> --- a/drivers/clk/tegra/clk-bpmp.c
>>>>> +++ b/drivers/clk/tegra/clk-bpmp.c
>>>>> @@ -55,6 +55,7 @@ struct tegra_bpmp_clk_message {
>>>>> ÂÂÂÂÂÂ struct {
>>>>> ÂÂÂÂÂÂÂÂÂÂ void *data;
>>>>> ÂÂÂÂÂÂÂÂÂÂ size_t size;
>>>>> +ÂÂÂÂÂÂÂ int ret;
>>>>> ÂÂÂÂÂÂ } rx;
>>>>> ÂÂ };
>>>>> ÂÂ @@ -64,6 +65,7 @@ static int tegra_bpmp_clk_transfer(struct
>>>>> tegra_bpmp *bpmp,
>>>>> ÂÂÂÂÂÂ struct mrq_clk_request request;
>>>>> ÂÂÂÂÂÂ struct tegra_bpmp_message msg;
>>>>> ÂÂÂÂÂÂ void *req = &request;
>>>>> +ÂÂÂ int err;
>>>>> ÂÂ ÂÂÂÂÂ memset(&request, 0, sizeof(request));
>>>>> ÂÂÂÂÂÂ request.cmd_and_id = (clk->cmd << 24) | clk->id;
>>>>> @@ -84,7 +86,13 @@ static int tegra_bpmp_clk_transfer(struct
>>>>> tegra_bpmp *bpmp,
>>>>> ÂÂÂÂÂÂ msg.rx.data = clk->rx.data;
>>>>> ÂÂÂÂÂÂ msg.rx.size = clk->rx.size;
>>>>> ÂÂ -ÂÂÂ return tegra_bpmp_transfer(bpmp, &msg);
>>>>> +ÂÂÂ err = tegra_bpmp_transfer(bpmp, &msg);
>>>>> +ÂÂÂ if (err < 0)
>>>>> +ÂÂÂÂÂÂÂ return err;
>>>>> +ÂÂÂ else if (msg.rx.ret < 0)
>>>>> +ÂÂÂÂÂÂÂ return -EINVAL;
>>>>
>>>> I assume that the error codes returned do not correlated to the Linux
>>>> error codes here. Is that correct? If not we could just return the
>>>> actual error code. Otherwise would it be useful to print a message with
>>>> the bpmp error code for debug?
>>>
>>> The error codes are not 1:1 match with Linux. Unfortunately, printing
>>> message for debug is not either viable as during clock probing we are
>>> expecting many of the calls to return -BPMP_EINVAL to indicate that
>>> particular clock ID is unused.
>>
>> OK. Could it return other errors other than BPMP_EINVAL? I am just
>> wondering if we need to differentiate between unused and an actual
>> error? Maybe that is not possible here?
> 
> Other error codes are possible (though they are not explicitly
> documented in abi header). It's not easy to differentiate the error code
> at this level: -BPMP_EINVAL is "expected" condition with
> CMD_CLK_GET_ALL_INFO, whereas -BPMP_EINVAL is true error on other commands.

OK, thanks for clarifying.

Acked-by: Jon Hunter <jonathanh@xxxxxxxxxx>

Cheers
Jon

-- 
nvpublic