All of lore.kernel.org
 help / color / mirror / Atom feed
From: Niklas Cassel <cassel@kernel.org>
To: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Cc: "Jingoo Han" <jingoohan1@gmail.com>,
	"Lorenzo Pieralisi" <lpieralisi@kernel.org>,
	"Krzysztof Wilczyński" <kw@linux.com>,
	"Rob Herring" <robh@kernel.org>,
	"Bjorn Helgaas" <bhelgaas@google.com>,
	"Damien Le Moal" <dlemoal@kernel.org>,
	linux-pci@vger.kernel.org
Subject: Re: [PATCH] PCI: dwc: Reject a negative nr_irqs value in dw_pcie_edma_irq_verify()
Date: Thu, 2 Jan 2025 11:57:32 +0100	[thread overview]
Message-ID: <Z3ZxHLfN7rcpH414@ryzen> (raw)
In-Reply-To: <20241231184913.s24umoi2yi4wowod@thinkpad>

On Wed, Jan 01, 2025 at 12:19:13AM +0530, Manivannan Sadhasivam wrote:
> On Tue, Dec 31, 2024 at 07:13:33PM +0100, Niklas Cassel wrote:
> > On Tue, Dec 31, 2024 at 09:21:58PM +0530, Manivannan Sadhasivam wrote:
> > > On Fri, Dec 20, 2024 at 08:23:29AM +0100, Niklas Cassel wrote:
> > > > Platforms that do not have (one or more) dedicated IRQs for the eDMA
> > > > need to set nr_irqs to a non-zero value in their DWC glue driver.
> > > > 
> > > > Platforms that do have (one or more) dedicated IRQs do not need to
> > > > initialize nr_irqs. DWC common code will automatically set nr_irqs.
> > > > 
> > > > Since a glue driver can initialize nr_irqs, dw_pcie_edma_irq_verify()
> > > > should verify that nr_irqs, if non-zero, is a valid value. Thus, add a
> > > > check in dw_pcie_edma_irq_verify() to reject a negative nr_irqs value.
> > > > 
> > > 
> > > Why can't we make dw_edma_chip::nr_irqs unsigned?
> > 
> > dw_edma is defined in drivers/dma/dw-edma/dw-edma-core.h
> > in struct dw_edma.
> > 
> > struct dw_pcie (defined in drivers/pci/controller/dwc/pcie-designware.h)
> > simply has a struct dw_edma as a struct member.
> > 
> > If you bounce on nr_irqs in:
> > drivers/dma/dw-edma/dw-edma-core.c
> > and in
> > drivers/dma/dw-edma/dw-edma-pcie.c
> > you can see that this driver uses signed int for this everywhere.
> > 
> > I didn't feel like refactoring a whole DMA driver.
> > 
> 
> There is no need to refactor. Both 'dma' and 'dwc' drivers do not assume that
> 'nr_irqs' is signed. So simply changing the type to 'unsigned int' is enough.
> I don't see a valid reason to keep it signed and check for negative value.

If you bounce on nr_irqs in
drivers/dma/dw-edma/dw-edma-core.c
and in
drivers/dma/dw-edma/dw-edma-pcie.c

you will see that there are a lot of local variables
that are nr_irqs which are signed, in addition to the
struct member.

I don't want to change the whole driver simply to fix
a warning when building the DWC PCIe driver with W=1.

I have a different solution and will send a V2 soon.


Kind regards,
Niklas

      reply	other threads:[~2025-01-02 10:57 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-20  7:23 [PATCH] PCI: dwc: Reject a negative nr_irqs value in dw_pcie_edma_irq_verify() Niklas Cassel
2024-12-31 15:51 ` Manivannan Sadhasivam
2024-12-31 18:13   ` Niklas Cassel
2024-12-31 18:49     ` Manivannan Sadhasivam
2025-01-02 10:57       ` Niklas Cassel [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=Z3ZxHLfN7rcpH414@ryzen \
    --to=cassel@kernel.org \
    --cc=bhelgaas@google.com \
    --cc=dlemoal@kernel.org \
    --cc=jingoohan1@gmail.com \
    --cc=kw@linux.com \
    --cc=linux-pci@vger.kernel.org \
    --cc=lpieralisi@kernel.org \
    --cc=manivannan.sadhasivam@linaro.org \
    --cc=robh@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.