On Wed 21 May 2025 at 12:28, Frank Li <Frank.li@xxxxxxx> wrote:
> On Tue, May 20, 2025 at 09:47:14AM +0200, Jerome Brunet wrote:
>> On Mon 19 May 2025 at 17:21, Frank Li <Frank.li@xxxxxxx> wrote:
>>
>> > On Mon, May 05, 2025 at 07:41:48PM +0200, Jerome Brunet wrote:
>> >
>> > PCI tree require keep consistent at subject
>> > git log --oneline drivers/pci/endpoint/functions/pci-epf-vntb.c
>> >
>> > require first char is UP case.
>>
>> Noted
>>
>> >
>> > Align memory window naming with configfs names.
>> >
>> >> The config file related to the memory windows start the numbering of
>> >
>> > memory windows (MW)
>> > then you can use MW later.
>>
>> Sure
>>
>> >
>> >> the MW from 1. The other NTB function does the same, yet the enumeration
>> >> defining the BARs of the vNTB function starts numbering the MW from 0.
>> >>
>> >> Both numbering are fine I suppose but mixing the two is a bit confusing.
>> >> The configfs file being the interface with userspace, lets keep that stable
>> >> and consistently start the numbering of the MW from 1.
>> >>
>> >> Signed-off-by: Jerome Brunet <jbrunet@xxxxxxxxxxxx>
>> >> ---
>> >> drivers/pci/endpoint/functions/pci-epf-vntb.c | 11 ++++++-----
>> >> 1 file changed, 6 insertions(+), 5 deletions(-)
>> >>
>> >> diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
>> >> index 35fa0a21fc91100a5539bff775e7ebc25e1fb9c1..f9f4a8bb65f364962dbf1e9011ab0e4479c61034 100644
>> >> --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
>> >> +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
>> >> @@ -70,9 +70,10 @@ static struct workqueue_struct *kpcintb_workqueue;
>> >> enum epf_ntb_bar {
>> >> BAR_CONFIG,
>> >> BAR_DB,
>> >> - BAR_MW0,
>> >> BAR_MW1,
>> >> BAR_MW2,
>> >> + BAR_MW3,
>> >> + BAR_MW4,
>> >
>> > where use BAR_MW3 and BAR_MW4?
>>
>> This is aligned with the file available in configfs and what is possible
>> in theory with the function, same as the NTB function and NTB host driver.
>>
>> Stopping at MW1 because it is only one used in the driver would be weird
>> and the number later introduced would be wrong.
>
> Yes, but BAR_MW3 and BAR_MW4 should be added only when both was used in code
> actaully.
If you want to be strict about that then BAR_MW4 should go to the next change,
it is needed in the enum.
BAR_MW3 shall stay here because the purpose of this change is to shift the MW
naming by one. MW2 which was present becomes MW3.
>
> Frank
>>
>>
>> >
>> > Frank
>> >> };
>> >>
>> >> /*
>> >> @@ -576,7 +577,7 @@ static int epf_ntb_mw_bar_init(struct epf_ntb *ntb)
>> >>
>> >> for (i = 0; i < ntb->num_mws; i++) {
>> >> size = ntb->mws_size[i];
>> >> - barno = ntb->epf_ntb_bar[BAR_MW0 + i];
>> >> + barno = ntb->epf_ntb_bar[BAR_MW1 + i];
>> >>
>> >> ntb->epf->bar[barno].barno = barno;
>> >> ntb->epf->bar[barno].size = size;
>> >> @@ -629,7 +630,7 @@ static void epf_ntb_mw_bar_clear(struct epf_ntb *ntb, int num_mws)
>> >> int i;
>> >>
>> >> for (i = 0; i < num_mws; i++) {
>> >> - barno = ntb->epf_ntb_bar[BAR_MW0 + i];
>> >> + barno = ntb->epf_ntb_bar[BAR_MW1 + i];
>> >> pci_epc_clear_bar(ntb->epf->epc,
>> >> ntb->epf->func_no,
>> >> ntb->epf->vfunc_no,
>> >> @@ -676,7 +677,7 @@ static int epf_ntb_init_epc_bar(struct epf_ntb *ntb)
>> >> epc_features = pci_epc_get_features(ntb->epf->epc, ntb->epf->func_no, ntb->epf->vfunc_no);
>> >>
>> >> /* These are required BARs which are mandatory for NTB functionality */
>> >> - for (bar = BAR_CONFIG; bar <= BAR_MW0; bar++, barno++) {
>> >> + for (bar = BAR_CONFIG; bar <= BAR_MW1; bar++, barno++) {
>> >> barno = pci_epc_get_next_free_bar(epc_features, barno);
>> >> if (barno < 0) {
>> >> dev_err(dev, "Fail to get NTB function BAR\n");
>> >> @@ -1048,7 +1049,7 @@ static int vntb_epf_mw_set_trans(struct ntb_dev *ndev, int pidx, int idx,
>> >> struct device *dev;
>> >>
>> >> dev = &ntb->ntb.dev;
>> >> - barno = ntb->epf_ntb_bar[BAR_MW0 + idx];
>> >> + barno = ntb->epf_ntb_bar[BAR_MW1 + idx];
>> >> epf_bar = &ntb->epf->bar[barno];
>> >> epf_bar->phys_addr = addr;
>> >> epf_bar->barno = barno;
>> >>
>> >> --
>> >> 2.47.2
>> >>
>>
>> --
>> Jerome
--
Jerome
