diff mbox series

[v2,2/2] PCI: starfive: Simplify event doorbell bitmap initialization in pcie-starfive

Message ID 20250316171250.5901-2-linux.amoon@gmail.com (mailing list archive)
State New
Headers show
Series [v2,1/2] PCI: plda: Remove unused IRQ handler and simplify IRQ request logic | expand

Commit Message

Anand Moon March 16, 2025, 5:12 p.m. UTC
The events_bitmap initialization in starfive_pcie_probe() previously
masked out the PLDA_AXI_DOORBELL and PLDA_PCIE_DOORBELL events.

These masking has been removed, allowing these events to be included
in the bitmap. With this change ensures that all interrupt events are
properly accounted for and may be necessary for handling doorbell
events in certain use cases.

PCIe Doorbell Events: These are typically used to notify a device about
an event or to trigger an action. For example, a host system can write
to a doorbell register on a PCIe device to signal that new data is
available or that an operation should start12.

AXI-PCIe Bridge: This bridge acts as a protocol converter between AXI
(Advanced eXtensible Interface) and PCIe (Peripheral Component Interconnect
Express) domains. It allows transactions to be converted and communicated
between these two different protocols3.

Fixes: 39b91eb40c6a ("PCI: starfive: Add JH7110 PCIe controller")
Cc: Minda Chen <minda.chen@starfivetech.com>
Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
v2: new patch
---
 drivers/pci/controller/plda/pcie-starfive.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

Comments

Minda Chen March 17, 2025, 2:23 a.m. UTC | #1
> 
> The events_bitmap initialization in starfive_pcie_probe() previously masked out
> the PLDA_AXI_DOORBELL and PLDA_PCIE_DOORBELL events.
> 
> These masking has been removed, allowing these events to be included in the
> bitmap. With this change ensures that all interrupt events are properly
> accounted for and may be necessary for handling doorbell events in certain use
> cases.
> 
> PCIe Doorbell Events: These are typically used to notify a device about an event
> or to trigger an action. For example, a host system can write to a doorbell
> register on a PCIe device to signal that new data is available or that an
> operation should start12.
> 
> AXI-PCIe Bridge: This bridge acts as a protocol converter between AXI
> (Advanced eXtensible Interface) and PCIe (Peripheral Component Interconnect
> Express) domains. It allows transactions to be converted and communicated
> between these two different protocols3.
> 
> Fixes: 39b91eb40c6a ("PCI: starfive: Add JH7110 PCIe controller")
> Cc: Minda Chen <minda.chen@starfivetech.com>
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
> v2: new patch
> ---
>  drivers/pci/controller/plda/pcie-starfive.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/controller/plda/pcie-starfive.c
> b/drivers/pci/controller/plda/pcie-starfive.c
> index e73c1b7bc8efc..d2c2a8e039e10 100644
> --- a/drivers/pci/controller/plda/pcie-starfive.c
> +++ b/drivers/pci/controller/plda/pcie-starfive.c
> @@ -410,9 +410,7 @@ static int starfive_pcie_probe(struct platform_device
> *pdev)
>  	plda->host_ops = &sf_host_ops;
>  	plda->num_events = PLDA_MAX_EVENT_NUM;
>  	/* mask doorbell event */
> -	plda->events_bitmap = GENMASK(PLDA_INT_EVENT_NUM - 1, 0)
> -			     & ~BIT(PLDA_AXI_DOORBELL)
> -			     & ~BIT(PLDA_PCIE_DOORBELL);
> +	plda->events_bitmap = GENMASK(PLDA_INT_EVENT_NUM - 1, 0);
>  	plda->events_bitmap <<= PLDA_NUM_DMA_EVENTS;
>  	ret = plda_pcie_host_init(&pcie->plda, &starfive_pcie_ops,
>  				  &stf_pcie_event);
> --
> 2.48.1

Hi Anand
   Mask the door bell interrupt is required. In some case, ( eg :NVMe read/write mass data) found error.
Anand Moon March 17, 2025, 3:26 a.m. UTC | #2
Hi Minda

On Mon, 17 Mar 2025 at 07:53, Minda Chen <minda.chen@starfivetech.com> wrote:
>
>
>
> >
> > The events_bitmap initialization in starfive_pcie_probe() previously masked out
> > the PLDA_AXI_DOORBELL and PLDA_PCIE_DOORBELL events.
> >
> > These masking has been removed, allowing these events to be included in the
> > bitmap. With this change ensures that all interrupt events are properly
> > accounted for and may be necessary for handling doorbell events in certain use
> > cases.
> >
> > PCIe Doorbell Events: These are typically used to notify a device about an event
> > or to trigger an action. For example, a host system can write to a doorbell
> > register on a PCIe device to signal that new data is available or that an
> > operation should start12.
> >
> > AXI-PCIe Bridge: This bridge acts as a protocol converter between AXI
> > (Advanced eXtensible Interface) and PCIe (Peripheral Component Interconnect
> > Express) domains. It allows transactions to be converted and communicated
> > between these two different protocols3.
> >
> > Fixes: 39b91eb40c6a ("PCI: starfive: Add JH7110 PCIe controller")
> > Cc: Minda Chen <minda.chen@starfivetech.com>
> > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > ---
> > v2: new patch
> > ---
> >  drivers/pci/controller/plda/pcie-starfive.c | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/drivers/pci/controller/plda/pcie-starfive.c
> > b/drivers/pci/controller/plda/pcie-starfive.c
> > index e73c1b7bc8efc..d2c2a8e039e10 100644
> > --- a/drivers/pci/controller/plda/pcie-starfive.c
> > +++ b/drivers/pci/controller/plda/pcie-starfive.c
> > @@ -410,9 +410,7 @@ static int starfive_pcie_probe(struct platform_device
> > *pdev)
> >       plda->host_ops = &sf_host_ops;
> >       plda->num_events = PLDA_MAX_EVENT_NUM;
> >       /* mask doorbell event */
> > -     plda->events_bitmap = GENMASK(PLDA_INT_EVENT_NUM - 1, 0)
> > -                          & ~BIT(PLDA_AXI_DOORBELL)
> > -                          & ~BIT(PLDA_PCIE_DOORBELL);
> > +     plda->events_bitmap = GENMASK(PLDA_INT_EVENT_NUM - 1, 0);
> >       plda->events_bitmap <<= PLDA_NUM_DMA_EVENTS;
> >       ret = plda_pcie_host_init(&pcie->plda, &starfive_pcie_ops,
> >                                 &stf_pcie_event);
> > --
> > 2.48.1
>
> Hi Anand
>    Mask the door bell interrupt is required. In some case, ( eg :NVMe read/write mass data) found error.

Thank you for your review comments.

I have tested using the Starfive Vision Five 2 board with a Samsung NVMe drive
and did not encounter any data read/write errors.
However, we can consider dropping this patch if there are issues with
other development boards.
I am also available to test with different NVMe modules if needed.

Thanks
-Anand
diff mbox series

Patch

diff --git a/drivers/pci/controller/plda/pcie-starfive.c b/drivers/pci/controller/plda/pcie-starfive.c
index e73c1b7bc8efc..d2c2a8e039e10 100644
--- a/drivers/pci/controller/plda/pcie-starfive.c
+++ b/drivers/pci/controller/plda/pcie-starfive.c
@@ -410,9 +410,7 @@  static int starfive_pcie_probe(struct platform_device *pdev)
 	plda->host_ops = &sf_host_ops;
 	plda->num_events = PLDA_MAX_EVENT_NUM;
 	/* mask doorbell event */
-	plda->events_bitmap = GENMASK(PLDA_INT_EVENT_NUM - 1, 0)
-			     & ~BIT(PLDA_AXI_DOORBELL)
-			     & ~BIT(PLDA_PCIE_DOORBELL);
+	plda->events_bitmap = GENMASK(PLDA_INT_EVENT_NUM - 1, 0);
 	plda->events_bitmap <<= PLDA_NUM_DMA_EVENTS;
 	ret = plda_pcie_host_init(&pcie->plda, &starfive_pcie_ops,
 				  &stf_pcie_event);