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

[Timo Alho]

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.