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

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;
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-
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));
   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]

You did not mention the DB BAR here. Without that, this calculation doesn't
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

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
as they are 4 bytes apart.

Addr is absolute physical IO address, which increased by 4. But it doesn't matter.
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

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.

Thanks,
Mani

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