| Message ID | 1539758250-7380-2-git-send-email-talho@nvidia.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | BPMP-FW version query ("tag") update | expand |
On 17/10/2018 07:37, Timo Alho wrote: > Add a helper function to check that firmware is supporting a given MRQ > command. > > Signed-off-by: Timo Alho <talho@nvidia.com> > --- > drivers/firmware/tegra/bpmp.c | 28 ++++++++++++++++++++++++++++ > include/soc/tegra/bpmp.h | 7 +++++++ > 2 files changed, 35 insertions(+) > > diff --git a/drivers/firmware/tegra/bpmp.c b/drivers/firmware/tegra/bpmp.c > index 41448ba..5bb46a5 100644 > --- a/drivers/firmware/tegra/bpmp.c > +++ b/drivers/firmware/tegra/bpmp.c > @@ -470,6 +470,34 @@ void tegra_bpmp_free_mrq(struct tegra_bpmp *bpmp, unsigned int mrq, void *data) > } > EXPORT_SYMBOL_GPL(tegra_bpmp_free_mrq); > > +bool tegra_bpmp_mrq_is_supported(struct tegra_bpmp *bpmp, unsigned int mrq) > +{ > + struct mrq_query_abi_request req = { .mrq = cpu_to_le32(mrq) }; > + struct mrq_query_abi_response resp; > + struct tegra_bpmp_message msg = { > + .mrq = MRQ_QUERY_ABI, > + .tx = { > + .data = &req, > + .size = sizeof(req), > + }, > + .rx = { > + .data = &resp, > + .size = sizeof(resp), > + }, > + }; > + int ret; > + > + ret = tegra_bpmp_transfer(bpmp, &msg); > + if (ret != 0 || msg.rx.ret != 0) { > + /* something went wrong; assume not supported */ > + dev_warn(bpmp->dev, "tegra_bpmp_transfer failed\n"); > + return false; > + } > + > + return resp.status != 0 ? false : true; Nit-pick ... seems that the '!=' is not necessary if you switch the false and true around like the original version of this. > +} > +EXPORT_SYMBOL_GPL(tegra_bpmp_mrq_is_supported); > + > static void tegra_bpmp_mrq_handle_ping(unsigned int mrq, > struct tegra_bpmp_channel *channel, > void *data) > diff --git a/include/soc/tegra/bpmp.h b/include/soc/tegra/bpmp.h > index e69e4c4..b02f926 100644 > --- a/include/soc/tegra/bpmp.h > +++ b/include/soc/tegra/bpmp.h > @@ -129,6 +129,7 @@ int tegra_bpmp_request_mrq(struct tegra_bpmp *bpmp, unsigned int mrq, > tegra_bpmp_mrq_handler_t handler, void *data); > void tegra_bpmp_free_mrq(struct tegra_bpmp *bpmp, unsigned int mrq, > void *data); > +bool tegra_bpmp_mrq_is_supported(struct tegra_bpmp *bpmp, unsigned int mrq); > #else > static inline struct tegra_bpmp *tegra_bpmp_get(struct device *dev) > { > @@ -164,6 +165,12 @@ static inline void tegra_bpmp_free_mrq(struct tegra_bpmp *bpmp, > unsigned int mrq, void *data) > { > } > + > +static inline bool tegra_bpmp_mrq_is_supported(struct tegra_bpmp *bpmp, > + unsigned int mrq) > +{ > + return false; > +} > #endif > > #if IS_ENABLED(CONFIG_CLK_TEGRA_BPMP) > Otherwise ... Acked-by: Jon Hunter <jonathanh@nvidia.com> Cheers, Jon
On 17/10/2018 07:37, Timo Alho wrote: > Add a helper function to check that firmware is supporting a given MRQ > command. > > Signed-off-by: Timo Alho <talho@nvidia.com> > --- > drivers/firmware/tegra/bpmp.c | 28 ++++++++++++++++++++++++++++ > include/soc/tegra/bpmp.h | 7 +++++++ > 2 files changed, 35 insertions(+) > > diff --git a/drivers/firmware/tegra/bpmp.c b/drivers/firmware/tegra/bpmp.c > index 41448ba..5bb46a5 100644 > --- a/drivers/firmware/tegra/bpmp.c > +++ b/drivers/firmware/tegra/bpmp.c > @@ -470,6 +470,34 @@ void tegra_bpmp_free_mrq(struct tegra_bpmp *bpmp, unsigned int mrq, void *data) > } > EXPORT_SYMBOL_GPL(tegra_bpmp_free_mrq); > > +bool tegra_bpmp_mrq_is_supported(struct tegra_bpmp *bpmp, unsigned int mrq) > +{ > + struct mrq_query_abi_request req = { .mrq = cpu_to_le32(mrq) }; > + struct mrq_query_abi_response resp; > + struct tegra_bpmp_message msg = { > + .mrq = MRQ_QUERY_ABI, > + .tx = { > + .data = &req, > + .size = sizeof(req), > + }, > + .rx = { > + .data = &resp, > + .size = sizeof(resp), > + }, > + }; > + int ret; > + > + ret = tegra_bpmp_transfer(bpmp, &msg); > + if (ret != 0 || msg.rx.ret != 0) { > + /* something went wrong; assume not supported */ > + dev_warn(bpmp->dev, "tegra_bpmp_transfer failed\n"); Should we make this a dev_dbg? I am wondering if we want to avoid this print in the case of someone using and older version of the firmware where this is not a critical error? In other words, should the caller of this function be responsible for printing any error messages? Cheers Jon
On 17.10.2018 13.09, Jonathan Hunter wrote: > > On 17/10/2018 07:37, Timo Alho wrote: >> + >> + ret = tegra_bpmp_transfer(bpmp, &msg); >> + if (ret != 0 || msg.rx.ret != 0) { >> + /* something went wrong; assume not supported */ >> + dev_warn(bpmp->dev, "tegra_bpmp_transfer failed\n"); > > Should we make this a dev_dbg? I am wondering if we want to avoid this > print in the case of someone using and older version of the firmware > where this is not a critical error? In other words, should the caller of > this function be responsible for printing any error messages? This warning gets printed only when there is an error in communication between CPU and BPMP-FW. For cases where communication was successful, but MRQ is not supported, it would be responsibility of the caller to print something if that would be useful or needed. -Timo
On Wed, Oct 17, 2018 at 09:37:27AM +0300, Timo Alho wrote: > Add a helper function to check that firmware is supporting a given MRQ > command. > > Signed-off-by: Timo Alho <talho@nvidia.com> > --- > drivers/firmware/tegra/bpmp.c | 28 ++++++++++++++++++++++++++++ > include/soc/tegra/bpmp.h | 7 +++++++ > 2 files changed, 35 insertions(+) > > diff --git a/drivers/firmware/tegra/bpmp.c b/drivers/firmware/tegra/bpmp.c > index 41448ba..5bb46a5 100644 > --- a/drivers/firmware/tegra/bpmp.c > +++ b/drivers/firmware/tegra/bpmp.c > @@ -470,6 +470,34 @@ void tegra_bpmp_free_mrq(struct tegra_bpmp *bpmp, unsigned int mrq, void *data) > } > EXPORT_SYMBOL_GPL(tegra_bpmp_free_mrq); > Acked-by: Sivaram Nair <sivaramn@nvidia.com>
On 17/10/2018 17:47, Timo Alho wrote: > > > On 17.10.2018 13.09, Jonathan Hunter wrote: >> >> On 17/10/2018 07:37, Timo Alho wrote: >>> + >>> + ret = tegra_bpmp_transfer(bpmp, &msg); >>> + if (ret != 0 || msg.rx.ret != 0) { >>> + /* something went wrong; assume not supported */ >>> + dev_warn(bpmp->dev, "tegra_bpmp_transfer failed\n"); >> >> Should we make this a dev_dbg? I am wondering if we want to avoid this >> print in the case of someone using and older version of the firmware >> where this is not a critical error? In other words, should the caller of >> this function be responsible for printing any error messages? > > This warning gets printed only when there is an error in communication > between CPU and BPMP-FW. > > For cases where communication was successful, but MRQ is not supported, > it would be responsibility of the caller to print something if that > would be useful or needed. OK great, then that's fine. Cheers Jon
diff --git a/drivers/firmware/tegra/bpmp.c b/drivers/firmware/tegra/bpmp.c index 41448ba..5bb46a5 100644 --- a/drivers/firmware/tegra/bpmp.c +++ b/drivers/firmware/tegra/bpmp.c @@ -470,6 +470,34 @@ void tegra_bpmp_free_mrq(struct tegra_bpmp *bpmp, unsigned int mrq, void *data) } EXPORT_SYMBOL_GPL(tegra_bpmp_free_mrq); +bool tegra_bpmp_mrq_is_supported(struct tegra_bpmp *bpmp, unsigned int mrq) +{ + struct mrq_query_abi_request req = { .mrq = cpu_to_le32(mrq) }; + struct mrq_query_abi_response resp; + struct tegra_bpmp_message msg = { + .mrq = MRQ_QUERY_ABI, + .tx = { + .data = &req, + .size = sizeof(req), + }, + .rx = { + .data = &resp, + .size = sizeof(resp), + }, + }; + int ret; + + ret = tegra_bpmp_transfer(bpmp, &msg); + if (ret != 0 || msg.rx.ret != 0) { + /* something went wrong; assume not supported */ + dev_warn(bpmp->dev, "tegra_bpmp_transfer failed\n"); + return false; + } + + return resp.status != 0 ? false : true; +} +EXPORT_SYMBOL_GPL(tegra_bpmp_mrq_is_supported); + static void tegra_bpmp_mrq_handle_ping(unsigned int mrq, struct tegra_bpmp_channel *channel, void *data) diff --git a/include/soc/tegra/bpmp.h b/include/soc/tegra/bpmp.h index e69e4c4..b02f926 100644 --- a/include/soc/tegra/bpmp.h +++ b/include/soc/tegra/bpmp.h @@ -129,6 +129,7 @@ int tegra_bpmp_request_mrq(struct tegra_bpmp *bpmp, unsigned int mrq, tegra_bpmp_mrq_handler_t handler, void *data); void tegra_bpmp_free_mrq(struct tegra_bpmp *bpmp, unsigned int mrq, void *data); +bool tegra_bpmp_mrq_is_supported(struct tegra_bpmp *bpmp, unsigned int mrq); #else static inline struct tegra_bpmp *tegra_bpmp_get(struct device *dev) { @@ -164,6 +165,12 @@ static inline void tegra_bpmp_free_mrq(struct tegra_bpmp *bpmp, unsigned int mrq, void *data) { } + +static inline bool tegra_bpmp_mrq_is_supported(struct tegra_bpmp *bpmp, + unsigned int mrq) +{ + return false; +} #endif #if IS_ENABLED(CONFIG_CLK_TEGRA_BPMP)
Add a helper function to check that firmware is supporting a given MRQ command. Signed-off-by: Timo Alho <talho@nvidia.com> --- drivers/firmware/tegra/bpmp.c | 28 ++++++++++++++++++++++++++++ include/soc/tegra/bpmp.h | 7 +++++++ 2 files changed, 35 insertions(+)