[PATCH 1/9] staging: wilc1000: revert fix related to vif index

[PATCH 1/9] staging: wilc1000: revert fix related to vif index

[Ajay.Kathat]

From: Ajay Singh <ajay.kathat@microchip.com>

This patch reverts 0e490657c721 ("staging: wilc1000: Fix problem with
wrong vif index") commit.

The issue was not completely fixed with the above commit.
After 'for' loop completion, '->idx' value is correct but '->vif_num'
still contains incorrect interface count.

Fixes: 0e490657c721 ("staging: wilc1000: Fix problem with wrong vif index")
Cc: <stable@vger.kernel.org> # v4.12
Signed-off-by: Ajay Singh <ajay.kathat@microchip.com>
---
 drivers/staging/wilc1000/linux_wlan.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/wilc1000/linux_wlan.c b/drivers/staging/wilc1000/linux_wlan.c
index f096f9e..648b658 100644
--- a/drivers/staging/wilc1000/linux_wlan.c
+++ b/drivers/staging/wilc1000/linux_wlan.c
@@ -1017,12 +1017,11 @@ int wilc_netdev_init(struct wilc **wilc, struct device *dev, int io_type,
 			strcpy(ndev->name, "p2p%d");
 			vif->ifc_id = 0;
 		}
+		vif->idx = wl->vif_num;
 		vif->wilc = *wilc;
 		vif->ndev = ndev;
 		wl->vif[i] = vif;
 		wl->vif_num = i;
-		vif->idx = wl->vif_num;
-
 		ndev->netdev_ops = &wilc_netdev_ops;
 
 		wdev = wilc_create_wiphy(ndev, dev);
-- 
2.7.4

Re: [PATCH 1/9] staging: wilc1000: revert fix related to vif index

[Dan Carpenter]

This patch was very confusing to review...

From a process perspective, you really should fold patches 1 and 2
together.  Otherwise we're re-introducing a bug.

regards,
dan carpenter

Re: [PATCH 1/9] staging: wilc1000: revert fix related to vif index

[Ajay.Kathat]

Hi Dan,

On 2/7/2019 1:42 PM, Dan Carpenter wrote:
> This patch was very confusing to review...

Apologies for the confusion.

> From a process perspective, you really should fold patches 1 and 2
> together.  Otherwise we're re-introducing a bug.

Patch 1 and Patch 2 both are required if commit 0e490657c721 ("staging:
wilc1000: Fix problem with wrong vif index") is applied else Patch 2 is
enough for complete fix. For the above reason, I have divided the fixes
into 2 patches.
Please suggest, if I should go ahead and submit this in a single patch.
Should I put 2 'Fixes:' tag in that merge commit or only 0e490657c721
commit is enough.

Regards,
Ajay

Re: [PATCH 1/9] staging: wilc1000: revert fix related to vif index

[Dan Carpenter]

On Thu, Feb 07, 2019 at 09:39:54AM +0000, Ajay.Kathat@microchip.com wrote:
> Hi Dan,
> 
> On 2/7/2019 1:42 PM, Dan Carpenter wrote:
> > This patch was very confusing to review...
> 
> Apologies for the confusion.
> 
> > From a process perspective, you really should fold patches 1 and 2
> > together.  Otherwise we're re-introducing a bug.
> 
> Patch 1 and Patch 2 both are required if commit 0e490657c721 ("staging:
> wilc1000: Fix problem with wrong vif index") is applied else Patch 2 is
> enough for complete fix. For the above reason, I have divided the fixes
> into 2 patches.
> Please suggest, if I should go ahead and submit this in a single patch.
> Should I put 2 'Fixes:' tag in that merge commit or only 0e490657c721
> commit is enough.
> 

I always just go with the oldest one.  Another option is to include
both.

regards,
dan carpenter

[PATCH 2/9] staging: wilc1000: fix to set the correct value for 'vif_num'

[Ajay.Kathat]

From: Ajay Singh <ajay.kathat@microchip.com>

Set correct interface count value in '->vif_num'.
'vif_num' was incorrectly set one less than total number of interfaces
because 'i' is used to set its value, which starts from 0.

Fixes: 735bb39ca3be ("staging: wilc1000: simplify vif[i]->ndev accesses")
Cc: <stable@vger.kernel.org> # v4.10
Signed-off-by: Ajay Singh <ajay.kathat@microchip.com>
---
 drivers/staging/wilc1000/linux_wlan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/wilc1000/linux_wlan.c b/drivers/staging/wilc1000/linux_wlan.c
index 648b658..8fa5f90 100644
--- a/drivers/staging/wilc1000/linux_wlan.c
+++ b/drivers/staging/wilc1000/linux_wlan.c
@@ -1021,7 +1021,7 @@ int wilc_netdev_init(struct wilc **wilc, struct device *dev, int io_type,
 		vif->wilc = *wilc;
 		vif->ndev = ndev;
 		wl->vif[i] = vif;
-		wl->vif_num = i;
+		wl->vif_num = i + 1;
 		ndev->netdev_ops = &wilc_netdev_ops;
 
 		wdev = wilc_create_wiphy(ndev, dev);
-- 
2.7.4

Re: [PATCH 2/9] staging: wilc1000: fix to set the correct value for 'vif_num'

[Dan Carpenter]

On Thu, Feb 07, 2019 at 07:22:25AM +0000, Ajay.Kathat@microchip.com wrote:
> From: Ajay Singh <ajay.kathat@microchip.com>
> 
> Set correct interface count value in '->vif_num'.
> 'vif_num' was incorrectly set one less than total number of interfaces
> because 'i' is used to set its value, which starts from 0.
> 
> Fixes: 735bb39ca3be ("staging: wilc1000: simplify vif[i]->ndev accesses")
> Cc: <stable@vger.kernel.org> # v4.10
> Signed-off-by: Ajay Singh <ajay.kathat@microchip.com>
> ---
>  drivers/staging/wilc1000/linux_wlan.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/wilc1000/linux_wlan.c b/drivers/staging/wilc1000/linux_wlan.c
> index 648b658..8fa5f90 100644
> --- a/drivers/staging/wilc1000/linux_wlan.c
> +++ b/drivers/staging/wilc1000/linux_wlan.c
> @@ -1021,7 +1021,7 @@ int wilc_netdev_init(struct wilc **wilc, struct device *dev, int io_type,

The line before the diff looks like this:

		vif->idx = wl->vif_num;

can we can change that to:

		vif->idx = i;

It's the same thing, but more clear.

>  		vif->wilc = *wilc;
>  		vif->ndev = ndev;
>  		wl->vif[i] = vif;
> -		wl->vif_num = i;
> +		wl->vif_num = i + 1;
>  		ndev->netdev_ops = &wilc_netdev_ops;

regards,
dan carpenter