From: Bin Liu <b-liu@ti.com>
To: Alexey Spirkov <AlexeiS@astrosoft.ru>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
"andrew@ncrmnt.org" <andrew@ncrmnt.org>
Subject: Fix big-endian application issue for MUSB gadget
Date: Thu, 2 Aug 2018 11:44:00 -0500 [thread overview]
Message-ID: <20180802164400.GC31129@uda0271908> (raw)
Hi,
On Thu, Jul 26, 2018 at 12:52:53PM +0000, Alexey Spirkov wrote:
> Existing code is not applicable to big-endian machines
> ctrlrequest fields received in USB endian - i.e. in little-endian
> and should be converted to cpu endianness before usage.
>
> Signed-off-by: Alexey Spirkov <alexeis@astrosoft.ru>
> ---
> drivers/usb/musb/musb_gadget_ep0.c | 33 ++++++++++++++++++++-------------
> 1 file changed, 20 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/usb/musb/musb_gadget_ep0.c b/drivers/usb/musb/musb_gadget_ep0.c
> index 91a5027..5d5c933 100644
> --- a/drivers/usb/musb/musb_gadget_ep0.c
> +++ b/drivers/usb/musb/musb_gadget_ep0.c
> @@ -82,7 +82,7 @@ static int service_tx_status_request(
> u16 tmp;
> void __iomem *regs;
>
> - epnum = (u8) ctrlrequest->wIndex;
> + epnum = (u8) le16_to_cpu(ctrlrequest->wIndex);
> if (!epnum) {
> result[0] = 0;
> break;
> @@ -217,14 +217,15 @@ __acquires(musb->lock)
> case USB_REQ_SET_ADDRESS:
> /* change it after the status stage */
> musb->set_address = true;
> - musb->address = (u8) (ctrlrequest->wValue & 0x7f);
> + musb->address = (u8) (le16_to_cpu(ctrlrequest->wValue) &
> + 0x7f);
> handled = 1;
> break;
>
> case USB_REQ_CLEAR_FEATURE:
> switch (recip) {
> case USB_RECIP_DEVICE:
> - if (ctrlrequest->wValue
> + if (le16_to_cpu(ctrlrequest->wValue)
> != USB_DEVICE_REMOTE_WAKEUP)
> break;
> musb->may_wakeup = 0;
> @@ -234,7 +235,7 @@ __acquires(musb->lock)
> break;
> case USB_RECIP_ENDPOINT:{
> const u8 epnum =
> - ctrlrequest->wIndex & 0x0f;
> + le16_to_cpu(ctrlrequest->wIndex) & 0x0f;
> struct musb_ep *musb_ep;
> struct musb_hw_ep *ep;
> struct musb_request *request;
> @@ -243,12 +244,14 @@ __acquires(musb->lock)
> u16 csr;
>
> if (epnum == 0 || epnum >= MUSB_C_NUM_EPS ||
> - ctrlrequest->wValue != USB_ENDPOINT_HALT)
> + le16_to_cpu(ctrlrequest->wValue)
> + != USB_ENDPOINT_HALT)
> break;
>
> ep = musb->endpoints + epnum;
> regs = ep->regs;
> - is_in = ctrlrequest->wIndex & USB_DIR_IN;
> + is_in = le16_to_cpu(ctrlrequest->wIndex) &
> + USB_DIR_IN;
> if (is_in)
> musb_ep = &ep->ep_in;
> else
> @@ -300,17 +303,19 @@ __acquires(musb->lock)
> switch (recip) {
> case USB_RECIP_DEVICE:
> handled = 1;
> - switch (ctrlrequest->wValue) {
> + switch (le16_to_cpu(ctrlrequest->wValue)) {
> case USB_DEVICE_REMOTE_WAKEUP:
> musb->may_wakeup = 1;
> break;
> case USB_DEVICE_TEST_MODE:
> if (musb->g.speed != USB_SPEED_HIGH)
> goto stall;
> - if (ctrlrequest->wIndex & 0xff)
> + if (le16_to_cpu(ctrlrequest->wIndex) &
> + 0xff)
> goto stall;
>
> - switch (ctrlrequest->wIndex >> 8) {
> + switch (le16_to_cpu(ctrlrequest->wIndex)
> + >> 8) {
> case 1:
> pr_debug("TEST_J\n");
> /* TEST_J */
> @@ -399,7 +404,7 @@ __acquires(musb->lock)
>
> case USB_RECIP_ENDPOINT:{
> const u8 epnum =
> - ctrlrequest->wIndex & 0x0f;
> + le16_to_cpu(ctrlrequest->wIndex) & 0x0f;
> struct musb_ep *musb_ep;
> struct musb_hw_ep *ep;
> void __iomem *regs;
> @@ -407,12 +412,14 @@ __acquires(musb->lock)
> u16 csr;
>
> if (epnum == 0 || epnum >= MUSB_C_NUM_EPS ||
> - ctrlrequest->wValue != USB_ENDPOINT_HALT)
> + le16_to_cpu(ctrlrequest->wValue)
> + != USB_ENDPOINT_HALT)
> break;
>
> ep = musb->endpoints + epnum;
> regs = ep->regs;
> - is_in = ctrlrequest->wIndex & USB_DIR_IN;
> + is_in = le16_to_cpu(ctrlrequest->wIndex) &
> + USB_DIR_IN;
> if (is_in)
> musb_ep = &ep->ep_in;
> else
Since this function uses ctrlrequest->wIndex and ctrlrequest->wValue
many times, can you please create local vars for them, then we don't
have to call le16_to_cpu() on them in every instance?
> @@ -608,7 +615,7 @@ musb_read_setup(struct musb *musb, struct usb_ctrlrequest *req)
> */
> musb->set_address = false;
> musb->ackpend = MUSB_CSR0_P_SVDRXPKTRDY;
> - if (req->wLength == 0) {
> + if (le16_to_cpu(req->wLength) == 0) {
> if (req->bRequestType & USB_DIR_IN)
> musb->ackpend |= MUSB_CSR0_TXPKTRDY;
> musb->ep0_state = MUSB_EP0_STAGE_ACKWAIT;
Regards,
-Bin.
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
next reply other threads:[~2018-08-02 16:44 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-08-02 16:44 Bin Liu [this message]
-- strict thread matches above, loose matches on Subject: below --
2018-08-02 16:51 Fix big-endian application issue for MUSB gadget Bin Liu
2018-07-26 12:52 Alexey Spirkov
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=20180802164400.GC31129@uda0271908 \
--to=b-liu@ti.com \
--cc=AlexeiS@astrosoft.ru \
--cc=andrew@ncrmnt.org \
--cc=gregkh@linuxfoundation.org \
--cc=linux-usb@vger.kernel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.