RE: [EXT] Re: [PATCH v13 6/6] PCI: endpoint: pci-epf-vntb: fix sparse build warning

[Frank Li <frank.li@nxp.com>]

> -----Original Message-----
> From: Lorenzo Pieralisi <lpieralisi@kernel.org>
> Sent: Thursday, October 27, 2022 5:43 AM
> To: Frank Li <frank.li@nxp.com>
> Cc: imx@lists.linux.dev; Jon Mason <jdmason@kudzu.us>; Dave Jiang
> <dave.jiang@intel.com>; Allen Hubbe <allenbh@gmail.com>; Kishon Vijay
> Abraham I <kishon@ti.com>; Krzysztof Wilczyński <kw@linux.com>;
> Manivannan Sadhasivam <mani@kernel.org>; Bjorn Helgaas
> <bhelgaas@google.com>; open list:NTB DRIVER CORE <ntb@lists.linux.dev>;
> open list:PCI ENDPOINT SUBSYSTEM <linux-pci@vger.kernel.org>; open list
> <linux-kernel@vger.kernel.org>
> Subject: [EXT] Re: [PATCH v13 6/6] PCI: endpoint: pci-epf-vntb: fix sparse
> build warning
> 
> Caution: EXT Email
> 
> On Fri, Oct 07, 2022 at 03:13:26PM -0400, Frank Li wrote:
> > From: Frank Li <frank.li@nxp.com>
> >
> > Using  epf_db[i] instead of readl() because epf_db is located in local
> > memory and allocated by dma_alloc_coherent().
> >
> > Sparse build warning when there are not __iomem at readl().
> > Added __iomem force type convert in vntb_epf_peer_spad_read\write()
> > and vntb_epf_spad_read\write(). This require strong order at read and
> > write.
> 
> This commit log is unreadable sorry and this patch fixes multiple things
> and even rearrange local variables in a way that is completely
> unrelated to the patch aim itself.

[Frank Li] Not everyone is English native speaker.  If you think it is bad, 
Please give me what you want to change to? 

Abut rearrange local variable, It is my fault when split patch. 

> 
> If you are fixing sparse warning reports the warnings and fix them
> one by one.

[Frank Li] Error in vntb_epf_peer_spad_write is the same as vntb_epf_peer_spad_read
All are the same type error. Did you still prefer fixed one by one? 

> 
> Could you please pay attention to these details ? I don't have time
> to do it for you, sorry.

[Frank Li] Do you have other comments about other patches? 

> 
> Thanks,
> Lorenzo
> 
> > Signed-off-by: Frank Li <frank.li@nxp.com>
> > ---
> >  drivers/pci/endpoint/functions/pci-epf-vntb.c | 27 +++++++++----------
> >  1 file changed, 13 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c
> b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> > index 54616281da9e..9f1ec6788e51 100644
> > --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
> > +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> > @@ -136,7 +136,7 @@ struct epf_ntb {
> >
> >       struct epf_ntb_ctrl *reg;
> >
> > -     void __iomem *epf_db;
> > +     u32 *epf_db;
> >
> >       phys_addr_t vpci_mw_phy[MAX_MW];
> >       void __iomem *vpci_mw_addr[MAX_MW];
> > @@ -257,12 +257,9 @@ static void epf_ntb_cmd_handler(struct
> work_struct *work)
> >       ntb = container_of(work, struct epf_ntb, cmd_handler.work);
> >
> >       for (i = 1; i < ntb->db_count; i++) {
> > -             if (readl(ntb->epf_db + i * sizeof(u32))) {
> > -                     if (readl(ntb->epf_db + i * sizeof(u32)))
> > -                             ntb->db |= 1 << (i - 1);
> > -
> > +             if (ntb->epf_db[i]) {
> >                       ntb_db_event(&ntb->ntb, i);
> > -                     writel(0, ntb->epf_db + i * sizeof(u32));
> > +                     ntb->epf_db[i] = 0;
> >               }
> >       }
> >
> > @@ -529,13 +526,15 @@ static int epf_ntb_configure_interrupt(struct
> epf_ntb *ntb)
> >  static int epf_ntb_db_bar_init(struct epf_ntb *ntb)
> >  {
> >       const struct pci_epc_features *epc_features;
> > -     u32 align;
> >       struct device *dev = &ntb->epf->dev;
> > -     int ret;
> >       struct pci_epf_bar *epf_bar;
> > -     void __iomem *mw_addr;
> >       enum pci_barno barno;
> > -     size_t size = sizeof(u32) * ntb->db_count;
> > +     void *mw_addr;
> > +     size_t size;
> > +     u32 align;
> > +     int ret;
> > +
> > +     size = sizeof(u32) * ntb->db_count;
> >
> >       epc_features = pci_epc_get_features(ntb->epf->epc,
> >                                           ntb->epf->func_no,
> > @@ -1123,7 +1122,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;
> > @@ -1134,7 +1133,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;
> > @@ -1145,7 +1144,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));
> > @@ -1157,7 +1156,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
> >