linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Bjorn Helgaas <helgaas@kernel.org>
To: Justin Stitt <justinstitt@google.com>
Cc: "Lorenzo Pieralisi" <lpieralisi@kernel.org>,
	"Krzysztof Wilczyński" <kw@linux.com>,
	"Rob Herring" <robh@kernel.org>,
	"Bjorn Helgaas" <bhelgaas@google.com>,
	"Ray Jui" <rjui@broadcom.com>,
	"Scott Branden" <sbranden@broadcom.com>,
	"Broadcom internal kernel review list"
	<bcm-kernel-feedback-list@broadcom.com>,
	"Nathan Chancellor" <nathan@kernel.org>,
	"Nick Desaulniers" <ndesaulniers@google.com>,
	"Tom Rix" <trix@redhat.com>,
	linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, llvm@lists.linux.dev
Subject: Re: [PATCH] PCI: iproc: fix -Wvoid-pointer-to-enum-cast warning
Date: Mon, 14 Aug 2023 18:00:08 -0500	[thread overview]
Message-ID: <20230814230008.GA196797@bhelgaas> (raw)
In-Reply-To: <20230814-void-drivers-pci-controller-pcie-iproc-platform-v1-1-81a121607851@google.com>

On Mon, Aug 14, 2023 at 10:29:22PM +0000, Justin Stitt wrote:
> When building with clang 18 I see the following warning:
> |       drivers/pci/controller/pcie-iproc-platform.c:55:15: warning: cast to smaller
> |                integer type 'enum iproc_pcie_type' from 'const void *' [-Wvoid-pointer-to-enum-cast]
> |          55 |         pcie->type = (enum iproc_pcie_type) of_device_get_match_data(dev);
> 
> This is due to the fact that `of_device_get_match_data` returns a void*
> while `enum iproc_pcie_type` has the size of an int. This leads to
> truncation and possible data loss.
> 
> Link: https://github.com/ClangBuiltLinux/linux/issues/1910
> Reported-by: Nathan Chancellor <nathan@kernel.org>
> Signed-off-by: Justin Stitt <justinstitt@google.com>
> ---
> Note: There is likely no data loss occurring here since `enum
> iproc_pcie_type` has only a few fields enumerated from 0. Definitely not
> enough to cause data loss from pointer-width to int-width.
> ---
>  drivers/pci/controller/pcie-iproc-platform.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/pcie-iproc-platform.c b/drivers/pci/controller/pcie-iproc-platform.c
> index acdc583d2980..83cbc95f4384 100644
> --- a/drivers/pci/controller/pcie-iproc-platform.c
> +++ b/drivers/pci/controller/pcie-iproc-platform.c
> @@ -52,7 +52,7 @@ static int iproc_pltfm_pcie_probe(struct platform_device *pdev)
>  	pcie = pci_host_bridge_priv(bridge);
>  
>  	pcie->dev = dev;
> -	pcie->type = (enum iproc_pcie_type) of_device_get_match_data(dev);
> +	pcie->type = (uintptr_t) of_device_get_match_data(dev);

This seems a little ugly on both ends: we have to cast the enum to
(int *) in the of_device_id table:

  static const struct of_device_id iproc_pcie_of_match_table[] = {
    {
      .compatible = "brcm,iproc-pcie",
      .data = (int *)IPROC_PCIE_PAXB,
    },

and then we have to cast it back to the the enum type here, and we
can't even use the actual enum type:

  pcie->type = (uintptr_t) of_device_get_match_data(dev);

I think this would be nicer if we made a struct iproc_pcie_of_data
along the lines of ks_pcie_of_data and put the enum values in
instances of that struct.

It's definitely a little more code and space, but it might also help
us get rid of some of the "switch (pcie->type)" stuff scattered around
this driver.

Bjorn

      reply	other threads:[~2023-08-14 23:00 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-14 22:29 [PATCH] PCI: iproc: fix -Wvoid-pointer-to-enum-cast warning Justin Stitt
2023-08-14 23:00 ` Bjorn Helgaas [this message]

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=20230814230008.GA196797@bhelgaas \
    --to=helgaas@kernel.org \
    --cc=bcm-kernel-feedback-list@broadcom.com \
    --cc=bhelgaas@google.com \
    --cc=justinstitt@google.com \
    --cc=kw@linux.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=llvm@lists.linux.dev \
    --cc=lpieralisi@kernel.org \
    --cc=nathan@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=rjui@broadcom.com \
    --cc=robh@kernel.org \
    --cc=sbranden@broadcom.com \
    --cc=trix@redhat.com \
    /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).