[PATCH] usb: uhci: Work around bogus clang shift overflow warning from DMA_BIT_MASK(64)

[PATCH] usb: uhci: Work around bogus clang shift overflow warning from DMA_BIT_MASK(64)

[Nathan Chancellor]

After commit 18a9ec886d32 ("usb: uhci: Add Aspeed AST2700 support"),
clang incorrectly warns:

  In file included from drivers/usb/host/uhci-hcd.c:855:
  drivers/usb/host/uhci-platform.c:69:32: error: shift count >= width of type [-Werror,-Wshift-count-overflow]
     69 | static const u64 dma_mask_64 = DMA_BIT_MASK(64);
        |                                ^~~~~~~~~~~~~~~~
  include/linux/dma-mapping.h:93:54: note: expanded from macro 'DMA_BIT_MASK'
     93 | #define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
        |                                                      ^ ~~~

clang has a long outstanding and complicated problem [1] with generating
a proper control flow graph at global scope, resulting in it being
unable to understand that this shift can never happen due to the
'n == 64' check.

Restructure the code to do the DMA_BIT_MASK() assignments within
uhci_hcd_platform_probe() (i.e., function scope) to avoid this global
scope issue.

Closes: https://github.com/ClangBuiltLinux/linux/issues/2136
Link: https://github.com/ClangBuiltLinux/linux/issues/92 [1]
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
 drivers/usb/host/uhci-platform.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/host/uhci-platform.c b/drivers/usb/host/uhci-platform.c
index 37607f985cc0..f532d3a0acbf 100644
--- a/drivers/usb/host/uhci-platform.c
+++ b/drivers/usb/host/uhci-platform.c
@@ -65,16 +65,16 @@ static const struct hc_driver uhci_platform_hc_driver = {
 	.hub_control =		uhci_hub_control,
 };
 
-static const u64 dma_mask_32 = DMA_BIT_MASK(32);
-static const u64 dma_mask_64 = DMA_BIT_MASK(64);
+static const bool use_dma_mask_64 = true;
 
 static int uhci_hcd_platform_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
-	const u64 *dma_mask_ptr;
+	u64 dma_mask = DMA_BIT_MASK(32);
 	struct usb_hcd *hcd;
 	struct uhci_hcd	*uhci;
 	struct resource *res;
+	const bool *of_data;
 	int ret;
 
 	if (usb_disabled())
@@ -85,11 +85,11 @@ static int uhci_hcd_platform_probe(struct platform_device *pdev)
 	 * Since shared usb code relies on it, set it here for now.
 	 * Once we have dma capability bindings this can go away.
 	 */
-	dma_mask_ptr = (u64 *)of_device_get_match_data(&pdev->dev);
-	if (!dma_mask_ptr)
-		dma_mask_ptr = &dma_mask_32;
+	of_data = of_device_get_match_data(&pdev->dev);
+	if (of_data && *of_data == use_dma_mask_64)
+		dma_mask = DMA_BIT_MASK(64);
 
-	ret = dma_coerce_mask_and_coherent(&pdev->dev, *dma_mask_ptr);
+	ret = dma_coerce_mask_and_coherent(&pdev->dev, dma_mask);
 	if (ret)
 		return ret;
 
@@ -200,7 +200,7 @@ static void uhci_hcd_platform_shutdown(struct platform_device *op)
 static const struct of_device_id platform_uhci_ids[] = {
 	{ .compatible = "generic-uhci", },
 	{ .compatible = "platform-uhci", },
-	{ .compatible = "aspeed,ast2700-uhci", .data = &dma_mask_64},
+	{ .compatible = "aspeed,ast2700-uhci", .data = &use_dma_mask_64 },
 	{}
 };
 MODULE_DEVICE_TABLE(of, platform_uhci_ids);

---
base-commit: 877c80dfbf788e57a3338627899033b7007037ee
change-id: 20251014-usb-uhci-avoid-bogus-clang-shift-warning-a80166a24472

Best regards,
--  
Nathan Chancellor <nathan@kernel.org>

Re: [PATCH] usb: uhci: Work around bogus clang shift overflow warning from DMA_BIT_MASK(64)

[Alan Stern]

On Tue, Oct 14, 2025 at 04:38:19PM -0700, Nathan Chancellor wrote:
> After commit 18a9ec886d32 ("usb: uhci: Add Aspeed AST2700 support"),
> clang incorrectly warns:
> 
>   In file included from drivers/usb/host/uhci-hcd.c:855:
>   drivers/usb/host/uhci-platform.c:69:32: error: shift count >= width of type [-Werror,-Wshift-count-overflow]
>      69 | static const u64 dma_mask_64 = DMA_BIT_MASK(64);
>         |                                ^~~~~~~~~~~~~~~~
>   include/linux/dma-mapping.h:93:54: note: expanded from macro 'DMA_BIT_MASK'
>      93 | #define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
>         |                                                      ^ ~~~
> 
> clang has a long outstanding and complicated problem [1] with generating
> a proper control flow graph at global scope, resulting in it being
> unable to understand that this shift can never happen due to the
> 'n == 64' check.
> 
> Restructure the code to do the DMA_BIT_MASK() assignments within
> uhci_hcd_platform_probe() (i.e., function scope) to avoid this global
> scope issue.
> 
> Closes: https://github.com/ClangBuiltLinux/linux/issues/2136
> Link: https://github.com/ClangBuiltLinux/linux/issues/92 [1]
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> ---

Do you think you could instead copy the approach used in:

https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/commit/?id=274f2232a94f6ca626d60288044e13d9a58c7612

IMO it is cleaner, and it also moves the DMA_BIT_MASK() computations 
into a function scope.

Alan Stern

>  drivers/usb/host/uhci-platform.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/usb/host/uhci-platform.c b/drivers/usb/host/uhci-platform.c
> index 37607f985cc0..f532d3a0acbf 100644
> --- a/drivers/usb/host/uhci-platform.c
> +++ b/drivers/usb/host/uhci-platform.c
> @@ -65,16 +65,16 @@ static const struct hc_driver uhci_platform_hc_driver = {
>  	.hub_control =		uhci_hub_control,
>  };
>  
> -static const u64 dma_mask_32 = DMA_BIT_MASK(32);
> -static const u64 dma_mask_64 = DMA_BIT_MASK(64);
> +static const bool use_dma_mask_64 = true;
>  
>  static int uhci_hcd_platform_probe(struct platform_device *pdev)
>  {
>  	struct device_node *np = pdev->dev.of_node;
> -	const u64 *dma_mask_ptr;
> +	u64 dma_mask = DMA_BIT_MASK(32);
>  	struct usb_hcd *hcd;
>  	struct uhci_hcd	*uhci;
>  	struct resource *res;
> +	const bool *of_data;
>  	int ret;
>  
>  	if (usb_disabled())
> @@ -85,11 +85,11 @@ static int uhci_hcd_platform_probe(struct platform_device *pdev)
>  	 * Since shared usb code relies on it, set it here for now.
>  	 * Once we have dma capability bindings this can go away.
>  	 */
> -	dma_mask_ptr = (u64 *)of_device_get_match_data(&pdev->dev);
> -	if (!dma_mask_ptr)
> -		dma_mask_ptr = &dma_mask_32;
> +	of_data = of_device_get_match_data(&pdev->dev);
> +	if (of_data && *of_data == use_dma_mask_64)
> +		dma_mask = DMA_BIT_MASK(64);
>  
> -	ret = dma_coerce_mask_and_coherent(&pdev->dev, *dma_mask_ptr);
> +	ret = dma_coerce_mask_and_coherent(&pdev->dev, dma_mask);
>  	if (ret)
>  		return ret;
>  
> @@ -200,7 +200,7 @@ static void uhci_hcd_platform_shutdown(struct platform_device *op)
>  static const struct of_device_id platform_uhci_ids[] = {
>  	{ .compatible = "generic-uhci", },
>  	{ .compatible = "platform-uhci", },
> -	{ .compatible = "aspeed,ast2700-uhci", .data = &dma_mask_64},
> +	{ .compatible = "aspeed,ast2700-uhci", .data = &use_dma_mask_64 },
>  	{}
>  };
>  MODULE_DEVICE_TABLE(of, platform_uhci_ids);
> 
> ---
> base-commit: 877c80dfbf788e57a3338627899033b7007037ee
> change-id: 20251014-usb-uhci-avoid-bogus-clang-shift-warning-a80166a24472
> 
> Best regards,
> --  
> Nathan Chancellor <nathan@kernel.org>
> 

Re: [PATCH] usb: uhci: Work around bogus clang shift overflow warning from DMA_BIT_MASK(64)

[Nathan Chancellor]

On Tue, Oct 14, 2025 at 11:07:27PM -0400, Alan Stern wrote:
> On Tue, Oct 14, 2025 at 04:38:19PM -0700, Nathan Chancellor wrote:
> > After commit 18a9ec886d32 ("usb: uhci: Add Aspeed AST2700 support"),
> > clang incorrectly warns:
> > 
> >   In file included from drivers/usb/host/uhci-hcd.c:855:
> >   drivers/usb/host/uhci-platform.c:69:32: error: shift count >= width of type [-Werror,-Wshift-count-overflow]
> >      69 | static const u64 dma_mask_64 = DMA_BIT_MASK(64);
> >         |                                ^~~~~~~~~~~~~~~~
> >   include/linux/dma-mapping.h:93:54: note: expanded from macro 'DMA_BIT_MASK'
> >      93 | #define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
> >         |                                                      ^ ~~~
> > 
> > clang has a long outstanding and complicated problem [1] with generating
> > a proper control flow graph at global scope, resulting in it being
> > unable to understand that this shift can never happen due to the
> > 'n == 64' check.
> > 
> > Restructure the code to do the DMA_BIT_MASK() assignments within
> > uhci_hcd_platform_probe() (i.e., function scope) to avoid this global
> > scope issue.
> > 
> > Closes: https://github.com/ClangBuiltLinux/linux/issues/2136
> > Link: https://github.com/ClangBuiltLinux/linux/issues/92 [1]
> > Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> > ---
> 
> Do you think you could instead copy the approach used in:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/commit/?id=274f2232a94f6ca626d60288044e13d9a58c7612
> 
> IMO it is cleaner, and it also moves the DMA_BIT_MASK() computations 
> into a function scope.

Sure, would something like this be what you had in mind?

diff --git a/drivers/usb/host/uhci-platform.c b/drivers/usb/host/uhci-platform.c
index 37607f985cc0..5e02f2ceafb6 100644
--- a/drivers/usb/host/uhci-platform.c
+++ b/drivers/usb/host/uhci-platform.c
@@ -65,13 +65,10 @@ static const struct hc_driver uhci_platform_hc_driver = {
 	.hub_control =		uhci_hub_control,
 };
 
-static const u64 dma_mask_32 = DMA_BIT_MASK(32);
-static const u64 dma_mask_64 = DMA_BIT_MASK(64);
-
 static int uhci_hcd_platform_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
-	const u64 *dma_mask_ptr;
+	bool dma_mask_64 = false;
 	struct usb_hcd *hcd;
 	struct uhci_hcd	*uhci;
 	struct resource *res;
@@ -85,11 +82,11 @@ static int uhci_hcd_platform_probe(struct platform_device *pdev)
 	 * Since shared usb code relies on it, set it here for now.
 	 * Once we have dma capability bindings this can go away.
 	 */
-	dma_mask_ptr = (u64 *)of_device_get_match_data(&pdev->dev);
-	if (!dma_mask_ptr)
-		dma_mask_ptr = &dma_mask_32;
+	if (of_device_get_match_data(&pdev->dev))
+		dma_mask_64 = true;
 
-	ret = dma_coerce_mask_and_coherent(&pdev->dev, *dma_mask_ptr);
+	ret = dma_coerce_mask_and_coherent(&pdev->dev,
+		dma_mask_64 ? DMA_BIT_MASK(64) : DMA_BIT_MASK(32));
 	if (ret)
 		return ret;
 
@@ -200,7 +197,7 @@ static void uhci_hcd_platform_shutdown(struct platform_device *op)
 static const struct of_device_id platform_uhci_ids[] = {
 	{ .compatible = "generic-uhci", },
 	{ .compatible = "platform-uhci", },
-	{ .compatible = "aspeed,ast2700-uhci", .data = &dma_mask_64},
+	{ .compatible = "aspeed,ast2700-uhci", .data = (void *)1 },
 	{}
 };
 MODULE_DEVICE_TABLE(of, platform_uhci_ids);

The

  const struct of_device_id *match;

  match = of_match_device(dev->dev.driver->of_match_table, &dev->dev);
  if (match && match->data)

part of the change you linked to is equivalent to

  if (of_device_get_match_data(&dev->dev))

if someone wanted to do a further clean up.

Cheers,
Nathan

Re: [PATCH] usb: uhci: Work around bogus clang shift overflow warning from DMA_BIT_MASK(64)

[Alan Stern]

On Tue, Oct 14, 2025 at 08:19:52PM -0700, Nathan Chancellor wrote:
> On Tue, Oct 14, 2025 at 11:07:27PM -0400, Alan Stern wrote:
> > On Tue, Oct 14, 2025 at 04:38:19PM -0700, Nathan Chancellor wrote:
> > > After commit 18a9ec886d32 ("usb: uhci: Add Aspeed AST2700 support"),
> > > clang incorrectly warns:
> > > 
> > >   In file included from drivers/usb/host/uhci-hcd.c:855:
> > >   drivers/usb/host/uhci-platform.c:69:32: error: shift count >= width of type [-Werror,-Wshift-count-overflow]
> > >      69 | static const u64 dma_mask_64 = DMA_BIT_MASK(64);
> > >         |                                ^~~~~~~~~~~~~~~~
> > >   include/linux/dma-mapping.h:93:54: note: expanded from macro 'DMA_BIT_MASK'
> > >      93 | #define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
> > >         |                                                      ^ ~~~
> > > 
> > > clang has a long outstanding and complicated problem [1] with generating
> > > a proper control flow graph at global scope, resulting in it being
> > > unable to understand that this shift can never happen due to the
> > > 'n == 64' check.
> > > 
> > > Restructure the code to do the DMA_BIT_MASK() assignments within
> > > uhci_hcd_platform_probe() (i.e., function scope) to avoid this global
> > > scope issue.
> > > 
> > > Closes: https://github.com/ClangBuiltLinux/linux/issues/2136
> > > Link: https://github.com/ClangBuiltLinux/linux/issues/92 [1]
> > > Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> > > ---
> > 
> > Do you think you could instead copy the approach used in:
> > 
> > https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/commit/?id=274f2232a94f6ca626d60288044e13d9a58c7612
> > 
> > IMO it is cleaner, and it also moves the DMA_BIT_MASK() computations 
> > into a function scope.
> 
> Sure, would something like this be what you had in mind?
> 
> diff --git a/drivers/usb/host/uhci-platform.c b/drivers/usb/host/uhci-platform.c
> index 37607f985cc0..5e02f2ceafb6 100644
> --- a/drivers/usb/host/uhci-platform.c
> +++ b/drivers/usb/host/uhci-platform.c
> @@ -65,13 +65,10 @@ static const struct hc_driver uhci_platform_hc_driver = {
>  	.hub_control =		uhci_hub_control,
>  };
>  
> -static const u64 dma_mask_32 = DMA_BIT_MASK(32);
> -static const u64 dma_mask_64 = DMA_BIT_MASK(64);
> -
>  static int uhci_hcd_platform_probe(struct platform_device *pdev)
>  {
>  	struct device_node *np = pdev->dev.of_node;
> -	const u64 *dma_mask_ptr;
> +	bool dma_mask_64 = false;
>  	struct usb_hcd *hcd;
>  	struct uhci_hcd	*uhci;
>  	struct resource *res;
> @@ -85,11 +82,11 @@ static int uhci_hcd_platform_probe(struct platform_device *pdev)
>  	 * Since shared usb code relies on it, set it here for now.
>  	 * Once we have dma capability bindings this can go away.
>  	 */
> -	dma_mask_ptr = (u64 *)of_device_get_match_data(&pdev->dev);
> -	if (!dma_mask_ptr)
> -		dma_mask_ptr = &dma_mask_32;
> +	if (of_device_get_match_data(&pdev->dev))
> +		dma_mask_64 = true;
>  
> -	ret = dma_coerce_mask_and_coherent(&pdev->dev, *dma_mask_ptr);
> +	ret = dma_coerce_mask_and_coherent(&pdev->dev,
> +		dma_mask_64 ? DMA_BIT_MASK(64) : DMA_BIT_MASK(32));
>  	if (ret)
>  		return ret;
>  
> @@ -200,7 +197,7 @@ static void uhci_hcd_platform_shutdown(struct platform_device *op)
>  static const struct of_device_id platform_uhci_ids[] = {
>  	{ .compatible = "generic-uhci", },
>  	{ .compatible = "platform-uhci", },
> -	{ .compatible = "aspeed,ast2700-uhci", .data = &dma_mask_64},
> +	{ .compatible = "aspeed,ast2700-uhci", .data = (void *)1 },
>  	{}
>  };
>  MODULE_DEVICE_TABLE(of, platform_uhci_ids);
> 
> The
> 
>   const struct of_device_id *match;
> 
>   match = of_match_device(dev->dev.driver->of_match_table, &dev->dev);
>   if (match && match->data)
> 
> part of the change you linked to is equivalent to
> 
>   if (of_device_get_match_data(&dev->dev))
> 
> if someone wanted to do a further clean up.

That's a small enough matter, we don't need to worry about it.

At any rate, yes, this is what I had in mind, thanks.  When you submit 
it, you may add:

Reviewed-by: Alan Stern <stern@rowland.harvard.edu>

Alan Stern