| Message ID | 20220215053844.7119-3-Frank.Li@nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | NTB function for PCIe RC to EP connection | expand |
[+cc Jon, Dave, Allen, linux-ntb, since you need at least an ack from the NTB folks; beginning of thread: https://lore.kernel.org/r/20220215053844.7119-1-Frank.Li@nxp.com] In subject, s/Added/Add/. But I don't think it's quite right yet, because this doesn't actually add any functions. On Mon, Feb 14, 2022 at 11:38:42PM -0600, Frank Li wrote: > Supported below memory map method > > bar 0: config and spad data > bar 2: door bell > bar 4: memory map windows s/bar/BAR/ s/spad/?/ (I don't know what this is) Presumably these BAR numbers apply to some specific hardware? This probably should specify *which* hardware. Please make the commit log say what this patch does. > Signed-off-by: Frank Li <Frank.Li@nxp.com> > --- > drivers/ntb/hw/epf/ntb_hw_epf.c | 48 ++++++++++++++++++++++++--------- > 1 file changed, 35 insertions(+), 13 deletions(-) > > diff --git a/drivers/ntb/hw/epf/ntb_hw_epf.c b/drivers/ntb/hw/epf/ntb_hw_epf.c > index b019755e4e21b..3ece49cb18ffa 100644 > --- a/drivers/ntb/hw/epf/ntb_hw_epf.c > +++ b/drivers/ntb/hw/epf/ntb_hw_epf.c > @@ -45,7 +45,6 @@ > > #define NTB_EPF_MIN_DB_COUNT 3 > #define NTB_EPF_MAX_DB_COUNT 31 > -#define NTB_EPF_MW_OFFSET 2 > > #define NTB_EPF_COMMAND_TIMEOUT 1000 /* 1 Sec */ > > @@ -67,6 +66,7 @@ struct ntb_epf_dev { > enum pci_barno ctrl_reg_bar; > enum pci_barno peer_spad_reg_bar; > enum pci_barno db_reg_bar; > + enum pci_barno mw_bar; > > unsigned int mw_count; > unsigned int spad_count; > @@ -92,6 +92,8 @@ struct ntb_epf_data { > enum pci_barno peer_spad_reg_bar; > /* BAR that contains Doorbell region and Memory window '1' */ > enum pci_barno db_reg_bar; > + /* BAR that contains memory windows*/ > + enum pci_barno mw_bar; > }; > > static int ntb_epf_send_command(struct ntb_epf_dev *ndev, u32 command, > @@ -411,7 +413,7 @@ static int ntb_epf_mw_set_trans(struct ntb_dev *ntb, int pidx, int idx, > return -EINVAL; > } > > - bar = idx + NTB_EPF_MW_OFFSET; > + bar = idx + ndev->mw_bar; > > mw_size = pci_resource_len(ntb->pdev, bar); > > @@ -453,7 +455,7 @@ static int ntb_epf_peer_mw_get_addr(struct ntb_dev *ntb, int idx, > if (idx == 0) > offset = readl(ndev->ctrl_reg + NTB_EPF_MW1_OFFSET); > > - bar = idx + NTB_EPF_MW_OFFSET; > + bar = idx + ndev->mw_bar; > > if (base) > *base = pci_resource_start(ndev->ntb.pdev, bar) + offset; > @@ -565,6 +567,7 @@ static int ntb_epf_init_pci(struct ntb_epf_dev *ndev, > struct pci_dev *pdev) > { > struct device *dev = ndev->dev; > + size_t spad_sz, spad_off; > int ret; > > pci_set_drvdata(pdev, ndev); > @@ -599,10 +602,16 @@ static int ntb_epf_init_pci(struct ntb_epf_dev *ndev, > goto err_dma_mask; > } > > - ndev->peer_spad_reg = pci_iomap(pdev, ndev->peer_spad_reg_bar, 0); > - if (!ndev->peer_spad_reg) { > - ret = -EIO; > - goto err_dma_mask; > + if (ndev->peer_spad_reg_bar) { > + ndev->peer_spad_reg = pci_iomap(pdev, ndev->peer_spad_reg_bar, 0); > + if (!ndev->peer_spad_reg) { > + ret = -EIO; > + goto err_dma_mask; > + } > + } else { > + spad_sz = 4 * readl(ndev->ctrl_reg + NTB_EPF_SPAD_COUNT); > + spad_off = readl(ndev->ctrl_reg + NTB_EPF_SPAD_OFFSET); > + ndev->peer_spad_reg = ndev->ctrl_reg + spad_off + spad_sz; > } > > ndev->db_reg = pci_iomap(pdev, ndev->db_reg_bar, 0); > @@ -657,6 +666,7 @@ static int ntb_epf_pci_probe(struct pci_dev *pdev, > enum pci_barno peer_spad_reg_bar = BAR_1; > enum pci_barno ctrl_reg_bar = BAR_0; > enum pci_barno db_reg_bar = BAR_2; > + enum pci_barno mw_bar = BAR_2; > struct device *dev = &pdev->dev; > struct ntb_epf_data *data; > struct ntb_epf_dev *ndev; > @@ -671,17 +681,16 @@ static int ntb_epf_pci_probe(struct pci_dev *pdev, > > data = (struct ntb_epf_data *)id->driver_data; > if (data) { > - if (data->peer_spad_reg_bar) > - peer_spad_reg_bar = data->peer_spad_reg_bar; > - if (data->ctrl_reg_bar) > - ctrl_reg_bar = data->ctrl_reg_bar; > - if (data->db_reg_bar) > - db_reg_bar = data->db_reg_bar; > + peer_spad_reg_bar = data->peer_spad_reg_bar; > + ctrl_reg_bar = data->ctrl_reg_bar; > + db_reg_bar = data->db_reg_bar; > + mw_bar = data->mw_bar; > } > > ndev->peer_spad_reg_bar = peer_spad_reg_bar; > ndev->ctrl_reg_bar = ctrl_reg_bar; > ndev->db_reg_bar = db_reg_bar; > + ndev->mw_bar = mw_bar; > ndev->dev = dev; > > ntb_epf_init_struct(ndev, pdev); > @@ -729,6 +738,14 @@ static const struct ntb_epf_data j721e_data = { > .ctrl_reg_bar = BAR_0, > .peer_spad_reg_bar = BAR_1, > .db_reg_bar = BAR_2, > + .mw_bar = BAR_2, > +}; > + > +static const struct ntb_epf_data mx8_data = { > + .ctrl_reg_bar = BAR_0, > + .peer_spad_reg_bar = BAR_0, > + .db_reg_bar = BAR_2, > + .mw_bar = BAR_4, > }; > > static const struct pci_device_id ntb_epf_pci_tbl[] = { > @@ -737,6 +754,11 @@ static const struct pci_device_id ntb_epf_pci_tbl[] = { > .class = PCI_CLASS_MEMORY_RAM << 8, .class_mask = 0xffff00, > .driver_data = (kernel_ulong_t)&j721e_data, > }, > + { > + PCI_DEVICE(PCI_VENDOR_ID_FREESCALE, 0x0809), > + .class = PCI_CLASS_MEMORY_RAM << 8, .class_mask = 0xffff00, > + .driver_data = (kernel_ulong_t)&mx8_data, > + }, > { }, > }; > > -- > 2.24.0.rc1 >
On Thu, Feb 17, 2022 at 3:59 PM Bjorn Helgaas <helgaas@kernel.org> wrote: > > [+cc Jon, Dave, Allen, linux-ntb, since you need at least an ack from > the NTB folks; beginning of thread: > https://lore.kernel.org/r/20220215053844.7119-1-Frank.Li@nxp.com] > > In subject, s/Added/Add/. > > But I don't think it's quite right yet, because this doesn't actually add > any functions. How about "Allow more flexibility in the memory bar map method"? > > On Mon, Feb 14, 2022 at 11:38:42PM -0600, Frank Li wrote: > > Supported below memory map method > > > > bar 0: config and spad data > > bar 2: door bell > > bar 4: memory map windows > > s/bar/BAR/ > s/spad/?/ (I don't know what this is) SCRATCHPAD REGION https://www.kernel.org/doc/html/latest/driver-api/ntb.html > > Presumably these BAR numbers apply to some specific hardware? This > probably should specify *which* hardware. Yes, BAR number on the EP side is configurable, which is controlled in the 3/4 patch. The Original BAR memory method can't be used in the new RC->EP NTB usage mode. Most Other code can be reused, so I enhanced the bar number config part. > > Please make the commit log say what this patch does. Does it help if attach ASCII diagram in patch 3/4 or cover letter one > > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > > --- > > drivers/ntb/hw/epf/ntb_hw_epf.c | 48 ++++++++++++++++++++++++--------- > > 1 file changed, 35 insertions(+), 13 deletions(-) > > > > diff --git a/drivers/ntb/hw/epf/ntb_hw_epf.c b/drivers/ntb/hw/epf/ntb_hw_epf.c > > index b019755e4e21b..3ece49cb18ffa 100644 > > --- a/drivers/ntb/hw/epf/ntb_hw_epf.c > > +++ b/drivers/ntb/hw/epf/ntb_hw_epf.c > > @@ -45,7 +45,6 @@ > > > > #define NTB_EPF_MIN_DB_COUNT 3 > > #define NTB_EPF_MAX_DB_COUNT 31 > > -#define NTB_EPF_MW_OFFSET 2 > > > > #define NTB_EPF_COMMAND_TIMEOUT 1000 /* 1 Sec */ > > > > @@ -67,6 +66,7 @@ struct ntb_epf_dev { > > enum pci_barno ctrl_reg_bar; > > enum pci_barno peer_spad_reg_bar; > > enum pci_barno db_reg_bar; > > + enum pci_barno mw_bar; > > > > unsigned int mw_count; > > unsigned int spad_count; > > @@ -92,6 +92,8 @@ struct ntb_epf_data { > > enum pci_barno peer_spad_reg_bar; > > /* BAR that contains Doorbell region and Memory window '1' */ > > enum pci_barno db_reg_bar; > > + /* BAR that contains memory windows*/ > > + enum pci_barno mw_bar; > > }; > > > > static int ntb_epf_send_command(struct ntb_epf_dev *ndev, u32 command, > > @@ -411,7 +413,7 @@ static int ntb_epf_mw_set_trans(struct ntb_dev *ntb, int pidx, int idx, > > return -EINVAL; > > } > > > > - bar = idx + NTB_EPF_MW_OFFSET; > > + bar = idx + ndev->mw_bar; > > > > mw_size = pci_resource_len(ntb->pdev, bar); > > > > @@ -453,7 +455,7 @@ static int ntb_epf_peer_mw_get_addr(struct ntb_dev *ntb, int idx, > > if (idx == 0) > > offset = readl(ndev->ctrl_reg + NTB_EPF_MW1_OFFSET); > > > > - bar = idx + NTB_EPF_MW_OFFSET; > > + bar = idx + ndev->mw_bar; > > > > if (base) > > *base = pci_resource_start(ndev->ntb.pdev, bar) + offset; > > @@ -565,6 +567,7 @@ static int ntb_epf_init_pci(struct ntb_epf_dev *ndev, > > struct pci_dev *pdev) > > { > > struct device *dev = ndev->dev; > > + size_t spad_sz, spad_off; > > int ret; > > > > pci_set_drvdata(pdev, ndev); > > @@ -599,10 +602,16 @@ static int ntb_epf_init_pci(struct ntb_epf_dev *ndev, > > goto err_dma_mask; > > } > > > > - ndev->peer_spad_reg = pci_iomap(pdev, ndev->peer_spad_reg_bar, 0); > > - if (!ndev->peer_spad_reg) { > > - ret = -EIO; > > - goto err_dma_mask; > > + if (ndev->peer_spad_reg_bar) { > > + ndev->peer_spad_reg = pci_iomap(pdev, ndev->peer_spad_reg_bar, 0); > > + if (!ndev->peer_spad_reg) { > > + ret = -EIO; > > + goto err_dma_mask; > > + } > > + } else { > > + spad_sz = 4 * readl(ndev->ctrl_reg + NTB_EPF_SPAD_COUNT); > > + spad_off = readl(ndev->ctrl_reg + NTB_EPF_SPAD_OFFSET); > > + ndev->peer_spad_reg = ndev->ctrl_reg + spad_off + spad_sz; > > } > > > > ndev->db_reg = pci_iomap(pdev, ndev->db_reg_bar, 0); > > @@ -657,6 +666,7 @@ static int ntb_epf_pci_probe(struct pci_dev *pdev, > > enum pci_barno peer_spad_reg_bar = BAR_1; > > enum pci_barno ctrl_reg_bar = BAR_0; > > enum pci_barno db_reg_bar = BAR_2; > > + enum pci_barno mw_bar = BAR_2; > > struct device *dev = &pdev->dev; > > struct ntb_epf_data *data; > > struct ntb_epf_dev *ndev; > > @@ -671,17 +681,16 @@ static int ntb_epf_pci_probe(struct pci_dev *pdev, > > > > data = (struct ntb_epf_data *)id->driver_data; > > if (data) { > > - if (data->peer_spad_reg_bar) > > - peer_spad_reg_bar = data->peer_spad_reg_bar; > > - if (data->ctrl_reg_bar) > > - ctrl_reg_bar = data->ctrl_reg_bar; > > - if (data->db_reg_bar) > > - db_reg_bar = data->db_reg_bar; > > + peer_spad_reg_bar = data->peer_spad_reg_bar; > > + ctrl_reg_bar = data->ctrl_reg_bar; > > + db_reg_bar = data->db_reg_bar; > > + mw_bar = data->mw_bar; > > } > > > > ndev->peer_spad_reg_bar = peer_spad_reg_bar; > > ndev->ctrl_reg_bar = ctrl_reg_bar; > > ndev->db_reg_bar = db_reg_bar; > > + ndev->mw_bar = mw_bar; > > ndev->dev = dev; > > > > ntb_epf_init_struct(ndev, pdev); > > @@ -729,6 +738,14 @@ static const struct ntb_epf_data j721e_data = { > > .ctrl_reg_bar = BAR_0, > > .peer_spad_reg_bar = BAR_1, > > .db_reg_bar = BAR_2, > > + .mw_bar = BAR_2, > > +}; > > + > > +static const struct ntb_epf_data mx8_data = { > > + .ctrl_reg_bar = BAR_0, > > + .peer_spad_reg_bar = BAR_0, > > + .db_reg_bar = BAR_2, > > + .mw_bar = BAR_4, > > }; > > > > static const struct pci_device_id ntb_epf_pci_tbl[] = { > > @@ -737,6 +754,11 @@ static const struct pci_device_id ntb_epf_pci_tbl[] = { > > .class = PCI_CLASS_MEMORY_RAM << 8, .class_mask = 0xffff00, > > .driver_data = (kernel_ulong_t)&j721e_data, > > }, > > + { > > + PCI_DEVICE(PCI_VENDOR_ID_FREESCALE, 0x0809), > > + .class = PCI_CLASS_MEMORY_RAM << 8, .class_mask = 0xffff00, > > + .driver_data = (kernel_ulong_t)&mx8_data, > > + }, > > { }, > > }; > > > > -- > > 2.24.0.rc1 > >
On Thu, Feb 17, 2022 at 04:24:56PM -0600, Zhi Li wrote: > On Thu, Feb 17, 2022 at 3:59 PM Bjorn Helgaas <helgaas@kernel.org> wrote: > > > > [+cc Jon, Dave, Allen, linux-ntb, since you need at least an ack from > > the NTB folks; beginning of thread: > > https://lore.kernel.org/r/20220215053844.7119-1-Frank.Li@nxp.com] > > > > In subject, s/Added/Add/. > > > > But I don't think it's quite right yet, because this doesn't actually add > > any functions. > > How about "Allow more flexibility in the memory bar map method"? s/bar/BAR/ Are you saying that the BAR numbers were fixed before, and you're adding the ability to change the BAR numbers? It would be useful to know what sort of flexibility you're adding. > > On Mon, Feb 14, 2022 at 11:38:42PM -0600, Frank Li wrote: > > > Supported below memory map method > > > > > > bar 0: config and spad data > > > bar 2: door bell > > > bar 4: memory map windows > > > > s/bar/BAR/ > > s/spad/?/ (I don't know what this is) > > SCRATCHPAD REGION > https://www.kernel.org/doc/html/latest/driver-api/ntb.html Just spelling out "scrachpad" is probably enough. And s/door bell/doorbell/ to match your usage elsewhere. > > Please make the commit log say what this patch does. > > Does it help if attach ASCII diagram in patch 3/4 or cover letter one The diagram is nice, but doesn't need to be replicated everywhere. A description of what the patch does would be better. Bjorn
diff --git a/drivers/ntb/hw/epf/ntb_hw_epf.c b/drivers/ntb/hw/epf/ntb_hw_epf.c index b019755e4e21b..3ece49cb18ffa 100644 --- a/drivers/ntb/hw/epf/ntb_hw_epf.c +++ b/drivers/ntb/hw/epf/ntb_hw_epf.c @@ -45,7 +45,6 @@ #define NTB_EPF_MIN_DB_COUNT 3 #define NTB_EPF_MAX_DB_COUNT 31 -#define NTB_EPF_MW_OFFSET 2 #define NTB_EPF_COMMAND_TIMEOUT 1000 /* 1 Sec */ @@ -67,6 +66,7 @@ struct ntb_epf_dev { enum pci_barno ctrl_reg_bar; enum pci_barno peer_spad_reg_bar; enum pci_barno db_reg_bar; + enum pci_barno mw_bar; unsigned int mw_count; unsigned int spad_count; @@ -92,6 +92,8 @@ struct ntb_epf_data { enum pci_barno peer_spad_reg_bar; /* BAR that contains Doorbell region and Memory window '1' */ enum pci_barno db_reg_bar; + /* BAR that contains memory windows*/ + enum pci_barno mw_bar; }; static int ntb_epf_send_command(struct ntb_epf_dev *ndev, u32 command, @@ -411,7 +413,7 @@ static int ntb_epf_mw_set_trans(struct ntb_dev *ntb, int pidx, int idx, return -EINVAL; } - bar = idx + NTB_EPF_MW_OFFSET; + bar = idx + ndev->mw_bar; mw_size = pci_resource_len(ntb->pdev, bar); @@ -453,7 +455,7 @@ static int ntb_epf_peer_mw_get_addr(struct ntb_dev *ntb, int idx, if (idx == 0) offset = readl(ndev->ctrl_reg + NTB_EPF_MW1_OFFSET); - bar = idx + NTB_EPF_MW_OFFSET; + bar = idx + ndev->mw_bar; if (base) *base = pci_resource_start(ndev->ntb.pdev, bar) + offset; @@ -565,6 +567,7 @@ static int ntb_epf_init_pci(struct ntb_epf_dev *ndev, struct pci_dev *pdev) { struct device *dev = ndev->dev; + size_t spad_sz, spad_off; int ret; pci_set_drvdata(pdev, ndev); @@ -599,10 +602,16 @@ static int ntb_epf_init_pci(struct ntb_epf_dev *ndev, goto err_dma_mask; } - ndev->peer_spad_reg = pci_iomap(pdev, ndev->peer_spad_reg_bar, 0); - if (!ndev->peer_spad_reg) { - ret = -EIO; - goto err_dma_mask; + if (ndev->peer_spad_reg_bar) { + ndev->peer_spad_reg = pci_iomap(pdev, ndev->peer_spad_reg_bar, 0); + if (!ndev->peer_spad_reg) { + ret = -EIO; + goto err_dma_mask; + } + } else { + spad_sz = 4 * readl(ndev->ctrl_reg + NTB_EPF_SPAD_COUNT); + spad_off = readl(ndev->ctrl_reg + NTB_EPF_SPAD_OFFSET); + ndev->peer_spad_reg = ndev->ctrl_reg + spad_off + spad_sz; } ndev->db_reg = pci_iomap(pdev, ndev->db_reg_bar, 0); @@ -657,6 +666,7 @@ static int ntb_epf_pci_probe(struct pci_dev *pdev, enum pci_barno peer_spad_reg_bar = BAR_1; enum pci_barno ctrl_reg_bar = BAR_0; enum pci_barno db_reg_bar = BAR_2; + enum pci_barno mw_bar = BAR_2; struct device *dev = &pdev->dev; struct ntb_epf_data *data; struct ntb_epf_dev *ndev; @@ -671,17 +681,16 @@ static int ntb_epf_pci_probe(struct pci_dev *pdev, data = (struct ntb_epf_data *)id->driver_data; if (data) { - if (data->peer_spad_reg_bar) - peer_spad_reg_bar = data->peer_spad_reg_bar; - if (data->ctrl_reg_bar) - ctrl_reg_bar = data->ctrl_reg_bar; - if (data->db_reg_bar) - db_reg_bar = data->db_reg_bar; + peer_spad_reg_bar = data->peer_spad_reg_bar; + ctrl_reg_bar = data->ctrl_reg_bar; + db_reg_bar = data->db_reg_bar; + mw_bar = data->mw_bar; } ndev->peer_spad_reg_bar = peer_spad_reg_bar; ndev->ctrl_reg_bar = ctrl_reg_bar; ndev->db_reg_bar = db_reg_bar; + ndev->mw_bar = mw_bar; ndev->dev = dev; ntb_epf_init_struct(ndev, pdev); @@ -729,6 +738,14 @@ static const struct ntb_epf_data j721e_data = { .ctrl_reg_bar = BAR_0, .peer_spad_reg_bar = BAR_1, .db_reg_bar = BAR_2, + .mw_bar = BAR_2, +}; + +static const struct ntb_epf_data mx8_data = { + .ctrl_reg_bar = BAR_0, + .peer_spad_reg_bar = BAR_0, + .db_reg_bar = BAR_2, + .mw_bar = BAR_4, }; static const struct pci_device_id ntb_epf_pci_tbl[] = { @@ -737,6 +754,11 @@ static const struct pci_device_id ntb_epf_pci_tbl[] = { .class = PCI_CLASS_MEMORY_RAM << 8, .class_mask = 0xffff00, .driver_data = (kernel_ulong_t)&j721e_data, }, + { + PCI_DEVICE(PCI_VENDOR_ID_FREESCALE, 0x0809), + .class = PCI_CLASS_MEMORY_RAM << 8, .class_mask = 0xffff00, + .driver_data = (kernel_ulong_t)&mx8_data, + }, { }, };
Supported below memory map method bar 0: config and spad data bar 2: door bell bar 4: memory map windows Signed-off-by: Frank Li <Frank.Li@nxp.com> --- drivers/ntb/hw/epf/ntb_hw_epf.c | 48 ++++++++++++++++++++++++--------- 1 file changed, 35 insertions(+), 13 deletions(-)