Re: [PATCH can-next v7 5/6] can: gs_usb: fix hardware timestamp state for mixed channels

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] can: gs_usb: invalid context-analysis lock annotations in
  timestamp helpers
--

commit ac918b2b251bb4e9c64c17ed5a99e1135d686016
Author: Shuangpeng Bai <shuangpeng.kernel@gmail.com>

can: gs_usb: fix hardware timestamp state for mixed channels

This commit initializes the hardware timestamping infrastructure during probe
whenever any channel supports GS_CAN_FEATURE_HW_TIMESTAMP, and starts or stops
it when the first or last channel is opened or closed.

> diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
> index 980c4a748c946..b7780a23eff3b 100644
> --- a/drivers/net/can/usb/gs_usb.c
> +++ b/drivers/net/can/usb/gs_usb.c

[ ... ]

> @@ -405,6 +411,7 @@ static int gs_cmd_reset(struct gs_can *dev)
>  
>  static inline int gs_usb_get_timestamp(const struct gs_usb *parent,
>  				       u32 *timestamp_p)
> +	__must_not_hold(parent->tc_lock)
>  {
>  	__le32 timestamp;
>  	int rc;
> @@ -423,7 +430,16 @@ static inline int gs_usb_get_timestamp(const struct gs_usb *parent,
>  	return 0;
>  }
>  
> -static u64 gs_usb_timestamp_read(struct cyclecounter *cc) __must_hold(&dev->tc_lock)
> +static u64 gs_usb_timestamp_read_initial(struct cyclecounter *cc)
> +	__must_hold(&container_of(cc, struct gs_usb, cc)->tc_lock)
> +{
> +	const struct gs_usb *parent = container_of(cc, struct gs_usb, cc);
> +
> +	return parent->timestamp_initial;
> +}
> +
> +static u64 gs_usb_timestamp_read(struct cyclecounter *cc)
> +	__must_hold(&container_of(cc, struct gs_usb, cc)->tc_lock)

[Severity: Low]
Does building with CONFIG_WARN_CONTEXT_ANALYSIS_ALL=y fail on these lock
annotations?

When WARN_CONTEXT_ANALYSIS is enabled, __must_hold() expands to a Clang
requires_capability attribute on the function declarator at file scope, where
the statement expression inside container_of() is rejected as a compile error.
Calling lockdep_assert_held(&parent->tc_lock) inside the function body avoids
this, and gs_usb_get_timestamp() also needs a pointer
(__must_not_hold(&parent->tc_lock)) rather than passing parent->tc_lock by
value.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261114-gs_usb-fixes-v7-0-58b6341b1c08@pengutronix.de?part=5