-----Original Message-----
From: Manivannan Sadhasivam <redacted>
Sent: Thursday, November 24, 2022 12:51 PM
To: Frank Li <frank.li@nxp.com>
Cc: lpieralisi@kernel.org; Aisheng Dong <aisheng.dong@nxp.com>;
bhelgaas@google.com; devicetree@vger.kernel.org; festevam@gmail.com;
imx@lists.linux.dev; jdmason@kudzu.us; kernel@pengutronix.de;
krzysztof.kozlowski+dt@linaro.org; kw@linux.com; linux-arm-
kernel@lists.infradead.org; dl-linux-imx [off-list ref]; linux-
kernel@vger.kernel.org; linux-pci@vger.kernel.org;
lorenzo.pieralisi@arm.com; lznuaa@gmail.com; maz@kernel.org;
ntb@lists.linux.dev; Peng Fan [off-list ref]; robh+dt@kernel.org;
s.hauer@pengutronix.de; shawnguo@kernel.org; tglx@linutronix.de;
kishon@kernel.org
Subject: Re: [EXT] Re: [PATCH v13 1/2] PCI: endpoint: pci-epf-vntb: change
doorbell register offset calc mathod

Caution: EXT Email

On Thu, Nov 24, 2022 at 05:49:32PM +0000, Frank Li wrote:

quoted

quoted

-----Original Message-----
From: Manivannan Sadhasivam <redacted>
Sent: Thursday, November 24, 2022 3:19 AM
To: Frank Li <frank.li@nxp.com>
Cc: lpieralisi@kernel.org; Aisheng Dong <aisheng.dong@nxp.com>;
bhelgaas@google.com; devicetree@vger.kernel.org;

festevam@gmail.com;

quoted

quoted

imx@lists.linux.dev; jdmason@kudzu.us; kernel@pengutronix.de;
kishon@ti.com; krzysztof.kozlowski+dt@linaro.org; kw@linux.com; linux-
arm-kernel@lists.infradead.org; dl-linux-imx [off-list ref];

linux-

quoted

quoted

kernel@vger.kernel.org; linux-pci@vger.kernel.org;
lorenzo.pieralisi@arm.com; lznuaa@gmail.com; maz@kernel.org;
ntb@lists.linux.dev; Peng Fan [off-list ref]; robh+dt@kernel.org;
s.hauer@pengutronix.de; shawnguo@kernel.org; tglx@linutronix.de
Subject: [EXT] Re: [PATCH v13 1/2] PCI: endpoint: pci-epf-vntb: change
doorbell register offset calc mathod

Caution: EXT Email

On Thu, Nov 24, 2022 at 12:50:35AM -0500, Frank Li wrote:

quoted

In drivers/ntb/hw/epf/ntb_hw_epf.c
ntb_epf_peer_db_set()
{
   ...
   db_offset = readl(ndev->ctrl_reg +

NTB_EPF_DB_OFFSET(interrupt_num));

quoted

quoted

quoted

   writel(db_data, ndev->db_reg + (db_entry_size * interrupt_num) +
               db_offset);
   ...
}

The door register offset's formular is
      offset = db_entry_size * interrupt_num +

db_offset[interrupt_number]

quoted

quoted

You did not mention the DB BAR here. Without that, this calculation

doesn't

quoted

quoted

make sense.

Doorbell register offset should means Base on DB BAR.
How about "The formula of  door register offset refer to DB BAR"?

"Doobell register offset in DB BAR is calculated using:"

quoted

quoted

quoted

Previous db_entry_size is 4, all db_offset is 0.

s/Previous/Previously

quoted

      irq | offset
       --------------
         0     0
         1     4
         2     8
        ...

Change to db_entry_size is 0 and db_offset is 0, 4, 8, ...
So we can get the same map value between irq and offset. This will be
convenience for hardware doorbell register memory map.

In your irq-imx-mu-msi.c driver, the msi_address is calculated as:

u64 addr = msi_data->msiir_addr + 4 * data->hwirq;

So the MSI addresses itself are of 4 bytes width. So the offsets will be
separated by 8 bytes like, 0, 8, 16,... and this won't match the MSI

addresses

quoted

quoted

as they are 4 bytes apart.

Addr is absolute physical IO address, which increased by 4. But it doesn't

matter.

quoted

It should be okay if range is between 2^32.

quoted

So you want to change the offset to 0, 4, 8,... by zeroing db_entry_size,
right?

I want to directly using db_offset[irq] value as offset. It will be simple.

I am not sure why ntb_hw_epf.c use below formular.
 "Db_offset[irq] + irq * db_entry_size"

Db_entry_size = 0 will be simple,  all offset will be controlled by db_offset[]

You can save db_offset[] as 0, 4, 8... or 0, 8, 16 as needs.

quoted

quoted

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/pci/endpoint/functions/pci-epf-vntb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c

b/drivers/pci/endpoint/functions/pci-epf-vntb.c

quoted

index 04698e7995a5..0d744975f815 100644

--- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
+++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c

@@ -461,11 +461,11 @@ static int

epf_ntb_config_spad_bar_alloc(struct

quoted

quoted

epf_ntb *ntb)

quoted

      ctrl->num_mws = ntb->num_mws;
      ntb->spad_size = spad_size;

-     ctrl->db_entry_size = sizeof(u32);
+     ctrl->db_entry_size = 0;

      for (i = 0; i < ntb->db_count; i++) {
              ntb->reg->db_data[i] = 1 + i;
-             ntb->reg->db_offset[i] = 0;
+             ntb->reg->db_offset[i] = sizeof(u32) * i;

If my above understanding is correct, then you could just reassign
"db_entry_size" in epf_ntb_epc_msi_init().

Yes, that's one method.
I want to use one method to calc db offset for both software polling
and MSI.  So overall logic should be simple.

I think it is better to leave db_entry_size for polling as it is and modify it
for MSI alone.

Okay, that means needn't this patch at all.

Thanks,
Mani

quoted

Frank Li

quoted

Thanks,
Mani

quoted

      }

      return 0;
--
2.34.1

--
மணிவண்ணன் சதாசிவம்

--
மணிவண்ணன் சதாசிவம்

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel