From: Boris Ostrovsky <boris.ostrovsky@oracle.com>
To: Connor Davis <connojdavis@gmail.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Juergen Gross <jgross@suse.com>,
Stefano Stabellini <sstabellini@kernel.org>
Cc: Douglas Anderson <dianders@chromium.org>,
"Eric W. Biederman" <ebiederm@xmission.com>,
Chunfeng Yun <chunfeng.yun@mediatek.com>,
Petr Mladek <pmladek@suse.com>,
Sumit Garg <sumit.garg@linaro.org>,
Lee Jones <lee.jones@linaro.org>,
linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
xen-devel@lists.xenproject.org
Subject: Re: [PATCH v2 3/4] usb: dbgp: Fix return values for reset prep and startup
Date: Fri, 14 May 2021 11:33:45 -0400 [thread overview]
Message-ID: <f5c8d1a5-84fa-19fc-14da-6bec1705cb5e@oracle.com> (raw)
In-Reply-To: <0010a6165f3560f16123a142d297276e7d6c2087.1620952511.git.connojdavis@gmail.com>
On 5/13/21 8:56 PM, Connor Davis wrote:
> Callers of dbgp_reset_prep treat a 0 return value as "stop using
> the debug port", which means they don't make any subsequent calls to
> dbgp_reset_prep or dbgp_external_startup.
>
> To ensure the callers' interpretation is correct, first return -EPERM
> from xen_dbgp_op if !xen_initial_domain(). This ensures that
> both xen_dbgp_reset_prep and xen_dbgp_external_startup return 0
> iff the PHYSDEVOP_DBGP_RESET_{PREPARE,DONE} hypercalls succeed. Also
> update xen_dbgp_reset_prep and xen_dbgp_external_startup to return
> -EPERM when !CONFIG_XEN_DOM0 for consistency.
>
> Next, return nonzero from dbgp_reset_prep if xen_dbgp_reset_prep returns
> 0. The nonzero value ensures that callers of dbgp_reset_prep will
> subsequently call dbgp_external_startup when it is safe to do so.
>
> Also invert the return values from dbgp_external_startup for
> consistency with dbgp_reset_prep (this inversion has no functional
> change since no callers actually check the value).
>
> Signed-off-by: Connor Davis <connojdavis@gmail.com>
For Xen bits:
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
For the rest it seems to me that error code passing could be improved: if it's 0 or 1 it should be bool. Or pass actual error code, with zero for no-error case, such as ...
> ---
> drivers/usb/early/ehci-dbgp.c | 9 ++++++---
> drivers/xen/dbgp.c | 2 +-
> include/linux/usb/ehci-dbgp.h | 14 +++++++++-----
> 3 files changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/usb/early/ehci-dbgp.c b/drivers/usb/early/ehci-dbgp.c
> index 45b42d8f6453..ff993d330c01 100644
> --- a/drivers/usb/early/ehci-dbgp.c
> +++ b/drivers/usb/early/ehci-dbgp.c
> @@ -970,8 +970,8 @@ int dbgp_reset_prep(struct usb_hcd *hcd)
> int ret = xen_dbgp_reset_prep(hcd);
> u32 ctrl;
>
> - if (ret)
> - return ret;
> + if (!ret)
> + return 1;
... here or ...
>
> dbgp_not_safe = 1;
> if (!ehci_debug)
> @@ -995,7 +995,10 @@ EXPORT_SYMBOL_GPL(dbgp_reset_prep);
>
> int dbgp_external_startup(struct usb_hcd *hcd)
> {
> - return xen_dbgp_external_startup(hcd) ?: _dbgp_external_startup();
> + if (!xen_dbgp_external_startup(hcd))
> + return 1;
... here.
-boris
next prev parent reply other threads:[~2021-05-14 15:34 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-14 0:56 [PATCH v2 0/4] Support xen-driven USB3 debug capability Connor Davis
2021-05-14 0:56 ` [PATCH v2 1/4] usb: early: Avoid using DbC if already enabled Connor Davis
2021-05-14 0:56 ` [PATCH v2 2/4] xen: Export dbgp functions when CONFIG_XEN_DOM0 is enabled Connor Davis
2021-05-14 14:06 ` Connor Davis
2021-05-14 0:56 ` [PATCH v2 3/4] usb: dbgp: Fix return values for reset prep and startup Connor Davis
2021-05-14 15:33 ` Boris Ostrovsky [this message]
2021-05-17 9:32 ` [PATCH v2 1/4] usb: early: Avoid using DbC if already enabled Jan Beulich
2021-05-17 13:48 ` Connor Davis
2021-05-17 14:13 ` Jan Beulich
2021-05-17 14:24 ` Connor Davis
2021-05-18 21:50 ` Mathias Nyman
2021-05-14 6:46 ` [PATCH v2 0/4] Support xen-driven USB3 debug capability Greg Kroah-Hartman
2021-05-14 14:07 ` Connor Davis
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=f5c8d1a5-84fa-19fc-14da-6bec1705cb5e@oracle.com \
--to=boris.ostrovsky@oracle.com \
--cc=chunfeng.yun@mediatek.com \
--cc=connojdavis@gmail.com \
--cc=dianders@chromium.org \
--cc=ebiederm@xmission.com \
--cc=gregkh@linuxfoundation.org \
--cc=jgross@suse.com \
--cc=lee.jones@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=pmladek@suse.com \
--cc=sstabellini@kernel.org \
--cc=sumit.garg@linaro.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).