The config file related to the memory windows start the numbering of
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@baylibre.com>
---
 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,
 };
 
 /*
@@ -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

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.

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.

> 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@baylibre.com>
> ---
>  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?

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
>

On Mon 19 May 2025 at 17:21, Frank Li <Frank.li@nxp.com> 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@baylibre.com>
>> ---
>>  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.


>
> 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

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@nxp.com> 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@baylibre.com>
> >> ---
> >>  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.

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

On Wed 21 May 2025 at 12:28, Frank Li <Frank.li@nxp.com> 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@nxp.com> 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@baylibre.com>
>> >> ---
>> >>  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