Re: [PATCH 1/4] firmware: tegra: propagate error code to caller

From: Jon Hunter
Date: Thu Sep 21 2017 - 07:20:12 EST



On 07/09/17 10:31, Timo Alho wrote:
> Response messages from Tegra BPMP firmware contain an error return
> code as the first word of payload. The error code is used to indicate
> incorrectly formatted request message or use of non-existing resource
> (clk, reset, powergate) identifier. Current implementation of
> tegra_bpmp_transfer() ignores this code and does not pass it to
> caller. Fix this by adding an extra struct member to
> tegra_bpmp_message and populate that with return code.
>
> Signed-off-by: Timo Alho <talho@xxxxxxxxxx>
> ---
> drivers/firmware/tegra/bpmp.c | 22 ++++++++++++++++------
> include/soc/tegra/bpmp.h | 1 +
> 2 files changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/firmware/tegra/bpmp.c b/drivers/firmware/tegra/bpmp.c
> index 73ca55b..33683b5 100644
> --- a/drivers/firmware/tegra/bpmp.c
> +++ b/drivers/firmware/tegra/bpmp.c
> @@ -194,16 +194,24 @@ static int tegra_bpmp_wait_master_free(struct tegra_bpmp_channel *channel)
> }
>
> static ssize_t __tegra_bpmp_channel_read(struct tegra_bpmp_channel *channel,
> - void *data, size_t size)
> + void *data, size_t size, int *ret)
> {
> + int err;
> +
> if (data && size > 0)
> memcpy(data, channel->ib->data, size);
>
> - return tegra_ivc_read_advance(channel->ivc);
> + err = tegra_ivc_read_advance(channel->ivc);
> + if (err < 0)
> + return err;
> +
> + *ret = channel->ib->code;
> +
> + return 0;
> }
>
> static ssize_t tegra_bpmp_channel_read(struct tegra_bpmp_channel *channel,
> - void *data, size_t size)
> + void *data, size_t size, int *ret)
> {
> struct tegra_bpmp *bpmp = channel->bpmp;
> unsigned long flags;
> @@ -217,7 +225,7 @@ static ssize_t tegra_bpmp_channel_read(struct tegra_bpmp_channel *channel,
> }
>
> spin_lock_irqsave(&bpmp->lock, flags);
> - err = __tegra_bpmp_channel_read(channel, data, size);
> + err = __tegra_bpmp_channel_read(channel, data, size, ret);
> clear_bit(index, bpmp->threaded.allocated);
> spin_unlock_irqrestore(&bpmp->lock, flags);
>
> @@ -337,7 +345,8 @@ int tegra_bpmp_transfer_atomic(struct tegra_bpmp *bpmp,
> if (err < 0)
> return err;
>
> - return __tegra_bpmp_channel_read(channel, msg->rx.data, msg->rx.size);
> + return __tegra_bpmp_channel_read(channel, msg->rx.data, msg->rx.size,
> + &msg->rx.ret);
> }
> EXPORT_SYMBOL_GPL(tegra_bpmp_transfer_atomic);
>
> @@ -371,7 +380,8 @@ int tegra_bpmp_transfer(struct tegra_bpmp *bpmp,
> if (err == 0)
> return -ETIMEDOUT;
>
> - return tegra_bpmp_channel_read(channel, msg->rx.data, msg->rx.size);
> + return tegra_bpmp_channel_read(channel, msg->rx.data, msg->rx.size,
> + &msg->rx.ret);
> }
> EXPORT_SYMBOL_GPL(tegra_bpmp_transfer);
>
> diff --git a/include/soc/tegra/bpmp.h b/include/soc/tegra/bpmp.h
> index 9ba6522..57519f4 100644
> --- a/include/soc/tegra/bpmp.h
> +++ b/include/soc/tegra/bpmp.h
> @@ -110,6 +110,7 @@ struct tegra_bpmp_message {
> struct {
> void *data;
> size_t size;
> + int ret;
> } rx;
> };

Looks good to me ...

Acked-by: Jon Hunter <jonathanh@xxxxxxxxxx>

Cheers
Jon


--
nvpublic