2022-11-02 15:03:01

by Frank Li

[permalink] [raw]
Subject: [PATCH v16 0/7] pci-epf-vntb clean up

continue patch series https://www.spinics.net/lists/linux-pci/msg130372.html
https://www.spinics.net/lists/linux-pci/msg130924.html

Lorenzo suggest create new series. Version number continue from old one
Change from v15 to v16
consistent subject
Add () after pci_epc_mem_free_addr

Change from v14-v15
Fixed according to Manivannan Sadhasivam's comments, except endian
problem. Endian problem is out of this patch series scope.
kernel test robot report sparse build warning problem already fixed
at patch 6 and 7.

Change from v13-v14
split spare warning fix to two patch
remove local variable reorder

Frank Li (7):
PCI: endpoint: pci-epf-vntb: Clean up kernel_doc warning
PCI: endpoint: pci-epf-vntb: Fix indentation of the struct
epf_ntb_ctrl
PCI: endpoint: pci-epf-vntb: fix call pci_epc_mem_free_addr at err
path
PCI: endpoint: pci-epf-vntb: remove unused field epf_db_phy
PCI: endpoint: pci-epf-vntb: replace hardcode 4 with sizeof(u32)
PCI: endpoint: pci-epf-vntb: fix sparse build warning at epf_db
PCI: endpoint: pci-epf-vntb: fix sparse build warning at ntb->reg

drivers/pci/endpoint/functions/pci-epf-vntb.c | 149 ++++++++++--------
1 file changed, 85 insertions(+), 64 deletions(-)

--
2.34.1



2022-11-02 15:03:16

by Frank Li

[permalink] [raw]
Subject: [PATCH v16 2/7] PCI: endpoint: pci-epf-vntb: fix indentation of the struct epf_ntb_ctrl

From: Frank Li <[email protected]>

Align the indentation of struct epf_ntb_ctrl with other structs in the driver

Signed-off-by: Frank Li <[email protected]>
---
drivers/pci/endpoint/functions/pci-epf-vntb.c | 28 +++++++++----------
1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
index c0115bcb3b5e..1863006cc36c 100644
--- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
+++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
@@ -99,20 +99,20 @@ enum epf_ntb_bar {
* NTB Driver NTB Driver
*/
struct epf_ntb_ctrl {
- u32 command;
- u32 argument;
- u16 command_status;
- u16 link_status;
- u32 topology;
- u64 addr;
- u64 size;
- u32 num_mws;
- u32 reserved;
- u32 spad_offset;
- u32 spad_count;
- u32 db_entry_size;
- u32 db_data[MAX_DB_COUNT];
- u32 db_offset[MAX_DB_COUNT];
+ u32 command;
+ u32 argument;
+ u16 command_status;
+ u16 link_status;
+ u32 topology;
+ u64 addr;
+ u64 size;
+ u32 num_mws;
+ u32 reserved;
+ u32 spad_offset;
+ u32 spad_count;
+ u32 db_entry_size;
+ u32 db_data[MAX_DB_COUNT];
+ u32 db_offset[MAX_DB_COUNT];
} __packed;

struct epf_ntb {
--
2.34.1


2022-11-02 15:03:53

by Frank Li

[permalink] [raw]
Subject: [PATCH v16 3/7] PCI: endpoint: pci-epf-vntb: fix call pci_epc_mem_free_addr() at err path

From: Frank Li <[email protected]>

Replace pci_epc_mem_free_addr() with pci_epf_free_space() at error handle
path to match pci_epf_alloc_space().

Fixes: e35f56bb0330 ("PCI: endpoint: Support NTB transfer between RC and EP")

Signed-off-by: Frank Li <[email protected]>
---
drivers/pci/endpoint/functions/pci-epf-vntb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
index 1863006cc36c..191924a83454 100644
--- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
+++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
@@ -571,7 +571,7 @@ static int epf_ntb_db_bar_init(struct epf_ntb *ntb)
return ret;

err_alloc_peer_mem:
- pci_epc_mem_free_addr(ntb->epf->epc, epf_bar->phys_addr, mw_addr, epf_bar->size);
+ pci_epf_free_space(ntb->epf, mw_addr, barno, 0);
return -1;
}

--
2.34.1


2022-11-02 15:04:39

by Frank Li

[permalink] [raw]
Subject: [PATCH v16 7/7] PCI: endpoint: pci-epf-vntb: fix sparse build warning at ntb->reg

From: Frank Li <[email protected]>

pci-epf-vntb.c:1128:33: sparse: expected void [noderef] __iomem *base
pci-epf-vntb.c:1128:33: sparse: got struct epf_ntb_ctrl *reg

Add __iomem type convert in vntb_epf_peer_spad_read() and
vntb_epf_peer_spad_write().

Reported-by: kernel test robot <[email protected]>
Signed-off-by: Frank Li <[email protected]>
Acked-by: Manivannan Sadhasivam <[email protected]>
---
drivers/pci/endpoint/functions/pci-epf-vntb.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
index f896846ed970..04698e7995a5 100644
--- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
+++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
@@ -1121,7 +1121,7 @@ static u32 vntb_epf_spad_read(struct ntb_dev *ndev, int idx)
struct epf_ntb *ntb = ntb_ndev(ndev);
int off = ntb->reg->spad_offset, ct = ntb->reg->spad_count * sizeof(u32);
u32 val;
- void __iomem *base = ntb->reg;
+ void __iomem *base = (void __iomem *)ntb->reg;

val = readl(base + off + ct + idx * sizeof(u32));
return val;
@@ -1132,7 +1132,7 @@ static int vntb_epf_spad_write(struct ntb_dev *ndev, int idx, u32 val)
struct epf_ntb *ntb = ntb_ndev(ndev);
struct epf_ntb_ctrl *ctrl = ntb->reg;
int off = ctrl->spad_offset, ct = ctrl->spad_count * sizeof(u32);
- void __iomem *base = ntb->reg;
+ void __iomem *base = (void __iomem *)ntb->reg;

writel(val, base + off + ct + idx * sizeof(u32));
return 0;
@@ -1143,7 +1143,7 @@ static u32 vntb_epf_peer_spad_read(struct ntb_dev *ndev, int pidx, int idx)
struct epf_ntb *ntb = ntb_ndev(ndev);
struct epf_ntb_ctrl *ctrl = ntb->reg;
int off = ctrl->spad_offset;
- void __iomem *base = ntb->reg;
+ void __iomem *base = (void __iomem *)ntb->reg;
u32 val;

val = readl(base + off + idx * sizeof(u32));
@@ -1155,7 +1155,7 @@ static int vntb_epf_peer_spad_write(struct ntb_dev *ndev, int pidx, int idx, u32
struct epf_ntb *ntb = ntb_ndev(ndev);
struct epf_ntb_ctrl *ctrl = ntb->reg;
int off = ctrl->spad_offset;
- void __iomem *base = ntb->reg;
+ void __iomem *base = (void __iomem *)ntb->reg;

writel(val, base + off + idx * sizeof(u32));
return 0;
--
2.34.1


2022-11-02 23:20:35

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v16 0/7] pci-epf-vntb clean up

On Wed, Nov 02, 2022 at 10:10:07AM -0400, Frank Li wrote:
> continue patch series https://www.spinics.net/lists/linux-pci/msg130372.html
> https://www.spinics.net/lists/linux-pci/msg130924.html
>
> Lorenzo suggest create new series. Version number continue from old one
> Change from v15 to v16
> consistent subject
> Add () after pci_epc_mem_free_addr

Don't forget these comments on your v15:
https://lore.kernel.org/r/20221101172006.GA1264778@bhelgaas

2022-11-03 01:12:58

by Frank Li

[permalink] [raw]
Subject: RE: [EXT] Re: [PATCH v16 0/7] pci-epf-vntb clean up



> -----Original Message-----
> From: Bjorn Helgaas <[email protected]>
> Sent: Wednesday, November 2, 2022 6:03 PM
> To: Frank Li <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: [EXT] Re: [PATCH v16 0/7] pci-epf-vntb clean up
>
> Caution: EXT Email
>
> On Wed, Nov 02, 2022 at 10:10:07AM -0400, Frank Li wrote:
> > continue patch series
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww
> .spinics.net%2Flists%2Flinux-
> pci%2Fmsg130372.html&amp;data=05%7C01%7CFrank.Li%40nxp.com%7C01d
> f759917bd40de2a6a08dabd2668df%7C686ea1d3bc2b4c6fa92cd99c5c301635%
> 7C0%7C0%7C638030269909169306%7CUnknown%7CTWFpbGZsb3d8eyJWIjoi
> MC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C300
> 0%7C%7C%7C&amp;sdata=ejz4GV8zkQCqKY6fVhpS4Xh2yKmL1661pRdqpnOJ
> xp4%3D&amp;reserved=0
> >
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww
> .spinics.net%2Flists%2Flinux-
> pci%2Fmsg130924.html&amp;data=05%7C01%7CFrank.Li%40nxp.com%7C01d
> f759917bd40de2a6a08dabd2668df%7C686ea1d3bc2b4c6fa92cd99c5c301635%
> 7C0%7C0%7C638030269909169306%7CUnknown%7CTWFpbGZsb3d8eyJWIjoi
> MC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C300
> 0%7C%7C%7C&amp;sdata=QhWY8%2F0FvNK1hvMM9JL34j5M9I9gu%2BgBt1
> JQEih0HrE%3D&amp;reserved=0
> >
> > Lorenzo suggest create new series. Version number continue from old one
> > Change from v15 to v16
> > consistent subject
> > Add () after pci_epc_mem_free_addr
>
> Don't forget these comments on your v15:
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.k
> ernel.org%2Fr%2F20221101172006.GA1264778%40bhelgaas&amp;data=05%7
> C01%7CFrank.Li%40nxp.com%7C01df759917bd40de2a6a08dabd2668df%7C68
> 6ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638030269909169306%7CUn
> known%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6
> Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=Gmi8t7jCrFXo
> 2LIxd7epeC7aOAlgaw7Xe51zLJq5gzQ%3D&amp;reserved=0

[Frank Li] I fixed all. Just cover letter reused V15 one and forget update
Patch lists. The real patches subject should be updated.


2022-11-23 17:21:29

by Lorenzo Pieralisi

[permalink] [raw]
Subject: Re: [PATCH v16 0/7] pci-epf-vntb clean up

On Wed, 2 Nov 2022 10:10:07 -0400, Frank Li wrote:
> continue patch series https://www.spinics.net/lists/linux-pci/msg130372.html
> https://www.spinics.net/lists/linux-pci/msg130924.html
>
> Lorenzo suggest create new series. Version number continue from old one
> Change from v15 to v16
> consistent subject
> Add () after pci_epc_mem_free_addr
>
> [...]

Applied to pci/endpoint, thanks!

[1/7] PCI: endpoint: pci-epf-vntb: clean up kernel_doc warning
https://git.kernel.org/lpieralisi/pci/c/929880484045
[2/7] PCI: endpoint: pci-epf-vntb: fix indentation of the struct epf_ntb_ctrl
https://git.kernel.org/lpieralisi/pci/c/1d118fed348f
[3/7] PCI: endpoint: pci-epf-vntb: fix call pci_epc_mem_free_addr() at err path
https://git.kernel.org/lpieralisi/pci/c/0c031262d2dd
[4/7] PCI: endpoint: pci-epf-vntb: remove unused field epf_db_phy
https://git.kernel.org/lpieralisi/pci/c/03d426ae5426
[5/7] PCI: endpoint: pci-epf-vntb: replace hardcode 4 with sizeof(u32)
https://git.kernel.org/lpieralisi/pci/c/2b35c886556a
[6/7] PCI: endpoint: pci-epf-vntb: fix sparse build warning at epf_db
https://git.kernel.org/lpieralisi/pci/c/01dcec6d57ce
[7/7] PCI: endpoint: pci-epf-vntb: fix sparse build warning at ntb->reg
https://git.kernel.org/lpieralisi/pci/c/5f697b25009c

Thanks,
Lorenzo

2022-12-14 00:44:56

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v16 7/7] PCI: endpoint: pci-epf-vntb: fix sparse build warning at ntb->reg

On Wed, Nov 02, 2022 at 10:10:14AM -0400, Frank Li wrote:
> From: Frank Li <[email protected]>
>
> pci-epf-vntb.c:1128:33: sparse: expected void [noderef] __iomem *base
> pci-epf-vntb.c:1128:33: sparse: got struct epf_ntb_ctrl *reg
>
> Add __iomem type convert in vntb_epf_peer_spad_read() and
> vntb_epf_peer_spad_write().

I don't understand all the bits and pieces here, but I'm a little
dubious about adding all these "(void __iomem *)"casts. There are
very few of them in drivers/pci/, and I doubt this driver is so unique
that it needs them.

> @@ -1121,7 +1121,7 @@ static u32 vntb_epf_spad_read(struct ntb_dev *ndev, int idx)
> struct epf_ntb *ntb = ntb_ndev(ndev);
> int off = ntb->reg->spad_offset, ct = ntb->reg->spad_count * sizeof(u32);
> u32 val;
> - void __iomem *base = ntb->reg;
> + void __iomem *base = (void __iomem *)ntb->reg;
>
> val = readl(base + off + ct + idx * sizeof(u32));
> return val;
> @@ -1132,7 +1132,7 @@ static int vntb_epf_spad_write(struct ntb_dev *ndev, int idx, u32 val)
> struct epf_ntb *ntb = ntb_ndev(ndev);
> struct epf_ntb_ctrl *ctrl = ntb->reg;
> int off = ctrl->spad_offset, ct = ctrl->spad_count * sizeof(u32);
> - void __iomem *base = ntb->reg;
> + void __iomem *base = (void __iomem *)ntb->reg;
>
> writel(val, base + off + ct + idx * sizeof(u32));

These things look gratuitously different to begin with:

int off = ntb->reg->spad_offset, ct = ntb->reg->spad_count * sizeof(u32);
int off = ctrl->spad_offset, ct = ctrl->spad_count * sizeof(u32);

They're doing the same thing, and they should do it the same way.

Since db_data[] and db_offset[] are never referenced except to be
initialized to zero, I'm guessing the point of vntb_epf_spad_read()
and vntb_epf_spad_write() is to read/write things in those arrays?

You access other things in ntb->reg directly by dereferencing a
pointer, e.g.,

ntb->reg->link_status |= LINK_STATUS_UP;
addr = ntb->reg->addr;
ctrl->command_status = COMMAND_STATUS_OK;

Why don't you just compute the appropriate *index* and access the
array directly instead of using readl() and writel()?

Bjorn

2022-12-14 01:03:34

by Frank Li

[permalink] [raw]
Subject: RE: [EXT] Re: [PATCH v16 7/7] PCI: endpoint: pci-epf-vntb: fix sparse build warning at ntb->reg

>
> On Wed, Nov 02, 2022 at 10:10:14AM -0400, Frank Li wrote:
> > From: Frank Li <[email protected]>
> >
> > pci-epf-vntb.c:1128:33: sparse: expected void [noderef] __iomem
> *base
> > pci-epf-vntb.c:1128:33: sparse: got struct epf_ntb_ctrl *reg
> >
> > Add __iomem type convert in vntb_epf_peer_spad_read() and
> > vntb_epf_peer_spad_write().
>
> I don't understand all the bits and pieces here, but I'm a little
> dubious about adding all these "(void __iomem *)"casts. There are
> very few of them in drivers/pci/, and I doubt this driver is so unique
> that it needs them.

sparse compiler report warning without cast. I write it at commit message.

Best regards
Frank Li

>
> > @@ -1121,7 +1121,7 @@ static u32 vntb_epf_spad_read(struct ntb_dev
> *ndev, int idx)
> > struct epf_ntb *ntb = ntb_ndev(ndev);
> > int off = ntb->reg->spad_offset, ct = ntb->reg->spad_count *
> sizeof(u32);
> > u32 val;
> > - void __iomem *base = ntb->reg;
> > + void __iomem *base = (void __iomem *)ntb->reg;
> >
> > val = readl(base + off + ct + idx * sizeof(u32));
> > return val;
> > @@ -1132,7 +1132,7 @@ static int vntb_epf_spad_write(struct ntb_dev
> *ndev, int idx, u32 val)
> > struct epf_ntb *ntb = ntb_ndev(ndev);
> > struct epf_ntb_ctrl *ctrl = ntb->reg;
> > int off = ctrl->spad_offset, ct = ctrl->spad_count * sizeof(u32);
> > - void __iomem *base = ntb->reg;
> > + void __iomem *base = (void __iomem *)ntb->reg;
> >
> > writel(val, base + off + ct + idx * sizeof(u32));
>
> These things look gratuitously different to begin with:
>
> int off = ntb->reg->spad_offset, ct = ntb->reg->spad_count * sizeof(u32);
> int off = ctrl->spad_offset, ct = ctrl->spad_count * sizeof(u32);
>
> They're doing the same thing, and they should do it the same way.
>
> Since db_data[] and db_offset[] are never referenced except to be
> initialized to zero, I'm guessing the point of vntb_epf_spad_read()
> and vntb_epf_spad_write() is to read/write things in those arrays?
>
> You access other things in ntb->reg directly by dereferencing a
> pointer, e.g.,
>
> ntb->reg->link_status |= LINK_STATUS_UP;
> addr = ntb->reg->addr;
> ctrl->command_status = COMMAND_STATUS_OK;
>
> Why don't you just compute the appropriate *index* and access the
> array directly instead of using readl() and writel()?
>
> Bjorn

2022-12-14 11:46:45

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [EXT] Re: [PATCH v16 7/7] PCI: endpoint: pci-epf-vntb: fix sparse build warning at ntb->reg

On Wed, Dec 14, 2022 at 12:49:15AM +0000, Frank Li wrote:
> >
> > On Wed, Nov 02, 2022 at 10:10:14AM -0400, Frank Li wrote:
> > > From: Frank Li <[email protected]>
> > >
> > > pci-epf-vntb.c:1128:33: sparse: expected void [noderef] __iomem
> > *base
> > > pci-epf-vntb.c:1128:33: sparse: got struct epf_ntb_ctrl *reg
> > >
> > > Add __iomem type convert in vntb_epf_peer_spad_read() and
> > > vntb_epf_peer_spad_write().
> >
> > I don't understand all the bits and pieces here, but I'm a little
> > dubious about adding all these "(void __iomem *)"casts. There are
> > very few of them in drivers/pci/, and I doubt this driver is so unique
> > that it needs them.
>
> sparse compiler report warning without cast. I write it at commit message.

As a matter of fact, I did read your commit message. My point is that
I don't think littering the code with casts is the best solution. I
wrote more details below; please read the entire email.

> > > @@ -1121,7 +1121,7 @@ static u32 vntb_epf_spad_read(struct ntb_dev
> > *ndev, int idx)
> > > struct epf_ntb *ntb = ntb_ndev(ndev);
> > > int off = ntb->reg->spad_offset, ct = ntb->reg->spad_count *
> > sizeof(u32);
> > > u32 val;
> > > - void __iomem *base = ntb->reg;
> > > + void __iomem *base = (void __iomem *)ntb->reg;
> > >
> > > val = readl(base + off + ct + idx * sizeof(u32));
> > > return val;
> > > @@ -1132,7 +1132,7 @@ static int vntb_epf_spad_write(struct ntb_dev
> > *ndev, int idx, u32 val)
> > > struct epf_ntb *ntb = ntb_ndev(ndev);
> > > struct epf_ntb_ctrl *ctrl = ntb->reg;
> > > int off = ctrl->spad_offset, ct = ctrl->spad_count * sizeof(u32);
> > > - void __iomem *base = ntb->reg;
> > > + void __iomem *base = (void __iomem *)ntb->reg;
> > >
> > > writel(val, base + off + ct + idx * sizeof(u32));
> >
> > These things look gratuitously different to begin with:
> >
> > int off = ntb->reg->spad_offset, ct = ntb->reg->spad_count * sizeof(u32);
> > int off = ctrl->spad_offset, ct = ctrl->spad_count * sizeof(u32);
> >
> > They're doing the same thing, and they should do it the same way.
> >
> > Since db_data[] and db_offset[] are never referenced except to be
> > initialized to zero, I'm guessing the point of vntb_epf_spad_read()
> > and vntb_epf_spad_write() is to read/write things in those arrays?
> >
> > You access other things in ntb->reg directly by dereferencing a
> > pointer, e.g.,
> >
> > ntb->reg->link_status |= LINK_STATUS_UP;
> > addr = ntb->reg->addr;
> > ctrl->command_status = COMMAND_STATUS_OK;
> >
> > Why don't you just compute the appropriate *index* and access the
> > array directly instead of using readl() and writel()?
> >
> > Bjorn

2022-12-14 15:48:08

by Frank Li

[permalink] [raw]
Subject: RE: [EXT] Re: [PATCH v16 7/7] PCI: endpoint: pci-epf-vntb: fix sparse build warning at ntb->reg



> -----Original Message-----
> From: Bjorn Helgaas <[email protected]>
> Sent: Tuesday, December 13, 2022 6:27 PM
> To: Frank Li <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: [EXT] Re: [PATCH v16 7/7] PCI: endpoint: pci-epf-vntb: fix sparse
> build warning at ntb->reg
>
> Caution: EXT Email
>
> On Wed, Nov 02, 2022 at 10:10:14AM -0400, Frank Li wrote:
> > From: Frank Li <[email protected]>
> >
> > pci-epf-vntb.c:1128:33: sparse: expected void [noderef] __iomem
> *base
> > pci-epf-vntb.c:1128:33: sparse: got struct epf_ntb_ctrl *reg
> >
> > Add __iomem type convert in vntb_epf_peer_spad_read() and
> > vntb_epf_peer_spad_write().
>
> I don't understand all the bits and pieces here, but I'm a little
> dubious about adding all these "(void __iomem *)"casts. There are
> very few of them in drivers/pci/, and I doubt this driver is so unique
> that it needs them.
>
> > @@ -1121,7 +1121,7 @@ static u32 vntb_epf_spad_read(struct ntb_dev
> *ndev, int idx)
> > struct epf_ntb *ntb = ntb_ndev(ndev);
> > int off = ntb->reg->spad_offset, ct = ntb->reg->spad_count *
> sizeof(u32);
> > u32 val;
> > - void __iomem *base = ntb->reg;
> > + void __iomem *base = (void __iomem *)ntb->reg;
> >
> > val = readl(base + off + ct + idx * sizeof(u32));
> > return val;
> > @@ -1132,7 +1132,7 @@ static int vntb_epf_spad_write(struct ntb_dev
> *ndev, int idx, u32 val)
> > struct epf_ntb *ntb = ntb_ndev(ndev);
> > struct epf_ntb_ctrl *ctrl = ntb->reg;
> > int off = ctrl->spad_offset, ct = ctrl->spad_count * sizeof(u32);
> > - void __iomem *base = ntb->reg;
> > + void __iomem *base = (void __iomem *)ntb->reg;
> >
> > writel(val, base + off + ct + idx * sizeof(u32));
>
> These things look gratuitously different to begin with:
>
> int off = ntb->reg->spad_offset, ct = ntb->reg->spad_count * sizeof(u32);
> int off = ctrl->spad_offset, ct = ctrl->spad_count * sizeof(u32);
>
> They're doing the same thing, and they should do it the same way.
>
> Since db_data[] and db_offset[] are never referenced except to be
> initialized to zero,

Db_data and db_offset map to PCI host bar0, so PCI host will read it.
It is generally used as MSI physical address and data, which PCI host
do doorbell by write these. I have followed patch, which under review.
Irq MSI platform msi change lots, I need more time to study such change.

Default use software polling. Even though it is zero, pci host driver still use it
to calculate doorbell register offset.

> I'm guessing the point of vntb_epf_spad_read()
> and vntb_epf_spad_write() is to read/write things in those arrays?

No, it is separated region.

>
> You access other things in ntb->reg directly by dereferencing a
> pointer, e.g.,
>
> ntb->reg->link_status |= LINK_STATUS_UP;
> addr = ntb->reg->addr;
> ctrl->command_status = COMMAND_STATUS_OK;
>
> Why don't you just compute the appropriate *index* and access the
> array directly instead of using readl() and writel()?

Good question.
NTB transfer layer treat it as register, so it need keep write\read order.
1. write data to buffer,
2. update header point.

1 and 2 must be keep order. NTB transfer layer have not added memory
barrier. Need use writel to guarantee order.

About ntb->reg, actually I think it should use readl also, but I port these code
from pci_epf_ntb.c and pci_epf_test.c.

PCI host side use writel, so write is ordered.

But I think reg->* 's order can't be guaranteed at ARM platform. Reg->addr may
get order value when check reg->command.

At least a rmb() need after reg->command. This code are almost run once at
Begging, so no problem happen.

>
> Bjorn