From: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
To: Dan Carpenter <dan.carpenter@linaro.org>
Cc: linux-usb@vger.kernel.org, heikki.krogerus@linux.intel.com,
diogo.ivo@tecnico.ulisboa.pt
Subject: Re: [bug report] usb: typec: ucsi: Only enable supported notifications
Date: Wed, 8 May 2024 15:22:49 +0100 [thread overview]
Message-ID: <eo56goqwajv2p3wwfrma2wowv7t5fc6fhfapkb75ixs5rvl63a@4joxxdo7bq37> (raw)
In-Reply-To: <dd955f16-bbb9-4d51-98ea-31cad358ab69@moroto.mountain>
Hello Dan,
On Wed, May 08, 2024 at 10:53:05AM GMT, Dan Carpenter wrote:
> Hello Diogo Ivo,
>
> Commit 27ffe4ff0b33 ("usb: typec: ucsi: Only enable supported
> notifications") from Mar 27, 2024 (linux-next), leads to the
> following Smatch static checker warning:
>
> drivers/usb/typec/ucsi/ucsi.c:1671 ucsi_get_supported_notifications()
> warn: was expecting a 64 bit value instead of '((((1))) << (24))'
>
> drivers/usb/typec/ucsi/ucsi.c
> 1665 static u64 ucsi_get_supported_notifications(struct ucsi *ucsi)
> 1666 {
> 1667 u8 features = ucsi->cap.features;
> 1668 u64 ntfy = UCSI_ENABLE_NTFY_ALL;
> 1669
> 1670 if (!(features & UCSI_CAP_ALT_MODE_DETAILS))
> --> 1671 ntfy &= ~UCSI_ENABLE_NTFY_CAM_CHANGE;
>
> ntfy is a u64 but UCSI_ENABLE_NTFY_CAM_CHANGE is unsigned long. So on
> a 32 bit system this will clear the high 32 bits. So far as I can see
> ntfy should just be a u32. Either way, the types should match.
> BIT_ULL() is the way to do that if it really needs to be a u64.
In my view this variable really should be a u64 and the definitions of
the UCSI_ENABLE_NTFY_* need to be changed to BIT_ULL(). This is due to
UCSI versions 2.0 and up definining a new notification on bit 33, crossing
the u32 barrier. My suggestion for addressing this is sending two
patches, one for changing BIT() -> BIT_ULL() and adding the missing
define for the notification of bit 33 and a separate patch to handle
this new notification bit in ucsi_get_supported_notifications()/ucsi_init().
Thank you for the report and please let me know if this sounds
reasonable, or if it would be better to split the changes in another
way.
Best regards,
Diogo
next prev parent reply other threads:[~2024-05-08 14:23 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-08 7:53 [bug report] usb: typec: ucsi: Only enable supported notifications Dan Carpenter
2024-05-08 14:22 ` Diogo Ivo [this message]
2024-05-08 14:34 ` Dan Carpenter
2024-05-08 14:45 ` Diogo Ivo
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=eo56goqwajv2p3wwfrma2wowv7t5fc6fhfapkb75ixs5rvl63a@4joxxdo7bq37 \
--to=diogo.ivo@tecnico.ulisboa.pt \
--cc=dan.carpenter@linaro.org \
--cc=heikki.krogerus@linux.intel.com \
--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 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 read-only IMAP folder(s) and NNTP newsgroup(s).