Subject: Re: [PATCH 2/4] clk: tegra: check BPMP response return code
To: Timo Alho <talho@nvidia.com>,
        "thierry.reding@gmail.com" <thierry.reding@gmail.com>
CC: "linux-tegra@vger.kernel.org" <linux-tegra@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
References: <cover.1504776489.git.talho@nvidia.com>
 <6a69e270519fd1c7a12a335053bf59671abc3c4b.1504776489.git.talho@nvidia.com>
 <388e07bc-b7b4-cc17-026a-bd08d77942ca@nvidia.com>
 <fc9729a7-53fb-f609-be6a-6454cc69548f@nvidia.com>
From: Jon Hunter <jonathanh@nvidia.com>
Message-ID: <5dfbbc6d-0dc0-7d92-0bff-8b05ddd272b3@nvidia.com>
Date: Fri, 29 Sep 2017 15:53:31 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.3.0
MIME-Version: 1.0
In-Reply-To: <fc9729a7-53fb-f609-be6a-6454cc69548f@nvidia.com>
Content-Type: text/plain; charset="utf-8"
Content-Language: en-US
Content-Transfer-Encoding: 8BIT
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2544
Lines: 69


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@nvidia.com>
>>> ---
>>>   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?

Cheers
Jon

-- 
nvpublic