2015-11-08 21:32:13

by punit vara

[permalink] [raw]
Subject: [PATCH V2] Staging: wilc1000: Fix build break due to undeclared *wilc and implicit declaration of init_irq

This patch is to the linux_wlan.c file that fixes up break found during
make drivers/staging/wilc1000/linux_wlan.o

Patch add following things to file :
-init_irq declaration
-At preprocessor (!defined WILC_SDIO) to defination of init_irq
-At preprocessor (!defined WILC_SDIO) to defination isr_uh_routine
-removes unnecessary lines to declare *wilc

Patch fixes 702c0e50f and 2c1d05d10 tags.

Signed-off-by: Punit Vara <[email protected]>
---
-Fixes tag added suggested by Dan carpenter.
-Remove declaration of autovariable with same type and same name suggested by Joe Perches

drivers/staging/wilc1000/linux_wlan.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/wilc1000/linux_wlan.c b/drivers/staging/wilc1000/linux_wlan.c
index 086f1db..5bd14ed 100644
--- a/drivers/staging/wilc1000/linux_wlan.c
+++ b/drivers/staging/wilc1000/linux_wlan.c
@@ -208,7 +208,7 @@ static int dev_state_ev_handler(struct notifier_block *this, unsigned long event
return NOTIFY_DONE;
}

-#if (defined WILC_SPI) || (defined WILC_SDIO_IRQ_GPIO)
+#if (defined WILC_SPI) || (defined WILC_SDIO_IRQ_GPIO) || (!defined WILC_SDIO)
static irqreturn_t isr_uh_routine(int irq, void *user_data)
{
perInterface_wlan_t *nic;
@@ -246,7 +246,7 @@ irqreturn_t isr_bh_routine(int irq, void *userdata)
return IRQ_HANDLED;
}

-#if (defined WILC_SPI) || (defined WILC_SDIO_IRQ_GPIO)
+#if (defined WILC_SPI) || (defined WILC_SDIO_IRQ_GPIO) || (!defined WILC_SDIO)
static int init_irq(struct net_device *dev)
{
int ret = 0;
@@ -937,6 +937,10 @@ static void wlan_deinitialize_threads(struct net_device *dev)
}
}

+#if (!defined WILC_SDIO) || (defined WILC_SDIO_IRQ_GPIO)
+static int init_irq(struct net_device *dev);
+#endif
+
int wilc1000_wlan_init(struct net_device *dev, perInterface_wlan_t *p_nic)
{
wilc_wlan_inp_t nwi;
@@ -1578,9 +1582,7 @@ int wilc_netdev_init(struct wilc **wilc)

static int __init init_wilc_driver(void)
{
-#ifdef WILC_SPI
struct wilc *wilc;
-#endif

#if defined(WILC_DEBUGFS)
if (wilc_debugfs_init() < 0) {
--
2.6.2


2015-11-08 22:00:21

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH V2] Staging: wilc1000: Fix build break due to undeclared *wilc and implicit declaration of init_irq

Top posting beucase I am a bad person. Punit, you should have CC'd Glen
since this is his code.

The fix is still not right. It will cause an unused variable warning on
some configs.

To be honest, this code makes no sense. Why do we even have the wilc
variable in this function when we never use it? Just declare move it to
wilc_netdev_init(). Also there are too many ifdefs in this code.

regards,
dan carpenter

On Mon, Nov 09, 2015 at 03:01:50AM +0530, Punit Vara wrote:
> This patch is to the linux_wlan.c file that fixes up break found during
> make drivers/staging/wilc1000/linux_wlan.o
>
> Patch add following things to file :
> -init_irq declaration
> -At preprocessor (!defined WILC_SDIO) to defination of init_irq
> -At preprocessor (!defined WILC_SDIO) to defination isr_uh_routine
> -removes unnecessary lines to declare *wilc
>
> Patch fixes 702c0e50f and 2c1d05d10 tags.
>
> Signed-off-by: Punit Vara <[email protected]>
> ---
> -Fixes tag added suggested by Dan carpenter.
> -Remove declaration of autovariable with same type and same name suggested by Joe Perches
>
> drivers/staging/wilc1000/linux_wlan.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/wilc1000/linux_wlan.c b/drivers/staging/wilc1000/linux_wlan.c
> index 086f1db..5bd14ed 100644
> --- a/drivers/staging/wilc1000/linux_wlan.c
> +++ b/drivers/staging/wilc1000/linux_wlan.c
> @@ -208,7 +208,7 @@ static int dev_state_ev_handler(struct notifier_block *this, unsigned long event
> return NOTIFY_DONE;
> }
>
> -#if (defined WILC_SPI) || (defined WILC_SDIO_IRQ_GPIO)
> +#if (defined WILC_SPI) || (defined WILC_SDIO_IRQ_GPIO) || (!defined WILC_SDIO)
> static irqreturn_t isr_uh_routine(int irq, void *user_data)
> {
> perInterface_wlan_t *nic;
> @@ -246,7 +246,7 @@ irqreturn_t isr_bh_routine(int irq, void *userdata)
> return IRQ_HANDLED;
> }
>
> -#if (defined WILC_SPI) || (defined WILC_SDIO_IRQ_GPIO)
> +#if (defined WILC_SPI) || (defined WILC_SDIO_IRQ_GPIO) || (!defined WILC_SDIO)
> static int init_irq(struct net_device *dev)
> {
> int ret = 0;
> @@ -937,6 +937,10 @@ static void wlan_deinitialize_threads(struct net_device *dev)
> }
> }
>
> +#if (!defined WILC_SDIO) || (defined WILC_SDIO_IRQ_GPIO)
> +static int init_irq(struct net_device *dev);
> +#endif
> +
> int wilc1000_wlan_init(struct net_device *dev, perInterface_wlan_t *p_nic)
> {
> wilc_wlan_inp_t nwi;
> @@ -1578,9 +1582,7 @@ int wilc_netdev_init(struct wilc **wilc)
>
> static int __init init_wilc_driver(void)
> {
> -#ifdef WILC_SPI
> struct wilc *wilc;
> -#endif
>
> #if defined(WILC_DEBUGFS)
> if (wilc_debugfs_init() < 0) {
> --
> 2.6.2
>
> _______________________________________________
> devel mailing list
> [email protected]
> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

2015-11-09 02:00:25

by Glen Lee

[permalink] [raw]
Subject: Re: [PATCH V2] Staging: wilc1000: Fix build break due to undeclared *wilc and implicit declaration of init_irq

I just check the email, I will get back here after I check the patches related with this issue.

regards,
glen lee.


On 2015년 11월 09일 06:59, Dan Carpenter wrote:
> Top posting beucase I am a bad person. Punit, you should have CC'd Glen
> since this is his code.
>
> The fix is still not right. It will cause an unused variable warning on
> some configs.
>
> To be honest, this code makes no sense. Why do we even have the wilc
> variable in this function when we never use it? Just declare move it to
> wilc_netdev_init(). Also there are too many ifdefs in this code.
>
> regards,
> dan carpenter
>
> On Mon, Nov 09, 2015 at 03:01:50AM +0530, Punit Vara wrote:
>> This patch is to the linux_wlan.c file that fixes up break found during
>> make drivers/staging/wilc1000/linux_wlan.o
>>
>> Patch add following things to file :
>> -init_irq declaration
>> -At preprocessor (!defined WILC_SDIO) to defination of init_irq
>> -At preprocessor (!defined WILC_SDIO) to defination isr_uh_routine
>> -removes unnecessary lines to declare *wilc
>>
>> Patch fixes 702c0e50f and 2c1d05d10 tags.
>>
>> Signed-off-by: Punit Vara <[email protected]>
>> ---
>> -Fixes tag added suggested by Dan carpenter.
>> -Remove declaration of autovariable with same type and same name suggested by Joe Perches
>>
>> drivers/staging/wilc1000/linux_wlan.c | 10 ++++++----
>> 1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/staging/wilc1000/linux_wlan.c b/drivers/staging/wilc1000/linux_wlan.c
>> index 086f1db..5bd14ed 100644
>> --- a/drivers/staging/wilc1000/linux_wlan.c
>> +++ b/drivers/staging/wilc1000/linux_wlan.c
>> @@ -208,7 +208,7 @@ static int dev_state_ev_handler(struct notifier_block *this, unsigned long event
>> return NOTIFY_DONE;
>> }
>>
>> -#if (defined WILC_SPI) || (defined WILC_SDIO_IRQ_GPIO)
>> +#if (defined WILC_SPI) || (defined WILC_SDIO_IRQ_GPIO) || (!defined WILC_SDIO)
>> static irqreturn_t isr_uh_routine(int irq, void *user_data)
>> {
>> perInterface_wlan_t *nic;
>> @@ -246,7 +246,7 @@ irqreturn_t isr_bh_routine(int irq, void *userdata)
>> return IRQ_HANDLED;
>> }
>>
>> -#if (defined WILC_SPI) || (defined WILC_SDIO_IRQ_GPIO)
>> +#if (defined WILC_SPI) || (defined WILC_SDIO_IRQ_GPIO) || (!defined WILC_SDIO)
>> static int init_irq(struct net_device *dev)
>> {
>> int ret = 0;
>> @@ -937,6 +937,10 @@ static void wlan_deinitialize_threads(struct net_device *dev)
>> }
>> }
>>
>> +#if (!defined WILC_SDIO) || (defined WILC_SDIO_IRQ_GPIO)
>> +static int init_irq(struct net_device *dev);
>> +#endif
>> +
>> int wilc1000_wlan_init(struct net_device *dev, perInterface_wlan_t *p_nic)
>> {
>> wilc_wlan_inp_t nwi;
>> @@ -1578,9 +1582,7 @@ int wilc_netdev_init(struct wilc **wilc)
>>
>> static int __init init_wilc_driver(void)
>> {
>> -#ifdef WILC_SPI
>> struct wilc *wilc;
>> -#endif
>>
>> #if defined(WILC_DEBUGFS)
>> if (wilc_debugfs_init() < 0) {
>> --
>> 2.6.2
>>
>> _______________________________________________
>> devel mailing list
>> [email protected]
>> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

2015-11-09 07:59:42

by Glen Lee

[permalink] [raw]
Subject: Re: [PATCH V2] Staging: wilc1000: Fix build break due to undeclared *wilc and implicit declaration of init_irq

Hi Punit Vara,

I cannot find build errors on my build machines.

According the log which you have posted before says *wilc is undeclared in the function init_wilc_driver,
which means WILC_SPI is selected because one of SPI or SDIO should be chosen at the moment.
Hence, struct wilc *wilc should be compiled together.
It looks like wilc1000 is compiled without SPI or SDIO.

Of course, there are many cases that I don't know, so you could let me know the wilc1000 build configuration?

static int __init init_wilc_driver(void)
{
#ifdef WILC_SPI
struct wilc *wilc;
#endif

..

#ifdef WILC_SDIO
{
int ret;

ret = sdio_register_driver(&wilc_bus);
if (ret < 0)
PRINT_D(INIT_DBG, "init_wilc_driver: Failed register sdio driver\n");

return ret;
}
#else
PRINT_D(INIT_DBG, "Initializing netdev\n");
if (wilc_netdev_init(&wilc))
PRINT_ER("Couldn't initialize netdev\n");
return 0;
#endif

regards,
glen lee.


On 2015년 11월 09일 11:03, glen lee wrote:
> I just check the email, I will get back here after I check the patches
> related with this issue.
>
> regards,
> glen lee.
>
>
> On 2015년 11월 09일 06:59, Dan Carpenter wrote:
>> Top posting beucase I am a bad person. Punit, you should have CC'd Glen
>> since this is his code.
>>
>> The fix is still not right. It will cause an unused variable warning on
>> some configs.
>>
>> To be honest, this code makes no sense. Why do we even have the wilc
>> variable in this function when we never use it? Just declare move it to
>> wilc_netdev_init(). Also there are too many ifdefs in this code.
>>
>> regards,
>> dan carpenter
>>
>> On Mon, Nov 09, 2015 at 03:01:50AM +0530, Punit Vara wrote:
>>> This patch is to the linux_wlan.c file that fixes up break found during
>>> make drivers/staging/wilc1000/linux_wlan.o
>>>
>>> Patch add following things to file :
>>> -init_irq declaration
>>> -At preprocessor (!defined WILC_SDIO) to defination of init_irq
>>> -At preprocessor (!defined WILC_SDIO) to defination isr_uh_routine
>>> -removes unnecessary lines to declare *wilc
>>>
>>> Patch fixes 702c0e50f and 2c1d05d10 tags.
>>>
>>> Signed-off-by: Punit Vara <[email protected]>
>>> ---
>>> -Fixes tag added suggested by Dan carpenter.
>>> -Remove declaration of autovariable with same type and same name
>>> suggested by Joe Perches
>>>
>>> drivers/staging/wilc1000/linux_wlan.c | 10 ++++++----
>>> 1 file changed, 6 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/staging/wilc1000/linux_wlan.c
>>> b/drivers/staging/wilc1000/linux_wlan.c
>>> index 086f1db..5bd14ed 100644
>>> --- a/drivers/staging/wilc1000/linux_wlan.c
>>> +++ b/drivers/staging/wilc1000/linux_wlan.c
>>> @@ -208,7 +208,7 @@ static int dev_state_ev_handler(struct
>>> notifier_block *this, unsigned long event
>>> return NOTIFY_DONE;
>>> }
>>> -#if (defined WILC_SPI) || (defined WILC_SDIO_IRQ_GPIO)
>>> +#if (defined WILC_SPI) || (defined WILC_SDIO_IRQ_GPIO) || (!defined
>>> WILC_SDIO)
>>> static irqreturn_t isr_uh_routine(int irq, void *user_data)
>>> {
>>> perInterface_wlan_t *nic;
>>> @@ -246,7 +246,7 @@ irqreturn_t isr_bh_routine(int irq, void *userdata)
>>> return IRQ_HANDLED;
>>> }
>>> -#if (defined WILC_SPI) || (defined WILC_SDIO_IRQ_GPIO)
>>> +#if (defined WILC_SPI) || (defined WILC_SDIO_IRQ_GPIO) || (!defined
>>> WILC_SDIO)
>>> static int init_irq(struct net_device *dev)
>>> {
>>> int ret = 0;
>>> @@ -937,6 +937,10 @@ static void wlan_deinitialize_threads(struct
>>> net_device *dev)
>>> }
>>> }
>>> +#if (!defined WILC_SDIO) || (defined WILC_SDIO_IRQ_GPIO)
>>> +static int init_irq(struct net_device *dev);
>>> +#endif
>>> +
>>> int wilc1000_wlan_init(struct net_device *dev, perInterface_wlan_t
>>> *p_nic)
>>> {
>>> wilc_wlan_inp_t nwi;
>>> @@ -1578,9 +1582,7 @@ int wilc_netdev_init(struct wilc **wilc)
>>> static int __init init_wilc_driver(void)
>>> {
>>> -#ifdef WILC_SPI
>>> struct wilc *wilc;
>>> -#endif
>>> #if defined(WILC_DEBUGFS)
>>> if (wilc_debugfs_init() < 0) {
>>> --
>>> 2.6.2
>>>
>>> _______________________________________________
>>> devel mailing list
>>> [email protected]
>>> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
>>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe
> linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2015-11-09 08:19:09

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH V2] Staging: wilc1000: Fix build break due to undeclared *wilc and implicit declaration of init_irq

On Mon, Nov 09, 2015 at 05:02:48PM +0900, glen lee wrote:
> Hi Punit Vara,
>
> I cannot find build errors on my build machines.
>
> According the log which you have posted before says *wilc is undeclared in the function init_wilc_driver,
> which means WILC_SPI is selected because one of SPI or SDIO should be chosen at the moment.
> Hence, struct wilc *wilc should be compiled together.
> It looks like wilc1000 is compiled without SPI or SDIO.
>
> Of course, there are many cases that I don't know, so you could let me know the wilc1000 build configuration?
>
> static int __init init_wilc_driver(void)
> {
> #ifdef WILC_SPI

This should be #ifndef WILC_SDIO


> struct wilc *wilc;
> #endif

But the large question remains of why do we have this variable here any
way?

regards,
dan carpenter

2015-11-09 08:51:21

by punit vara

[permalink] [raw]
Subject: Re: [PATCH V2] Staging: wilc1000: Fix build break due to undeclared *wilc and implicit declaration of init_irq

On Mon, Nov 9, 2015 at 1:48 PM, Dan Carpenter <[email protected]> wrote:
> On Mon, Nov 09, 2015 at 05:02:48PM +0900, glen lee wrote:
>> Hi Punit Vara,
>>
>> I cannot find build errors on my build machines.
>>
>> According the log which you have posted before says *wilc is undeclared in the function init_wilc_driver,
>> which means WILC_SPI is selected because one of SPI or SDIO should be chosen at the moment.
>> Hence, struct wilc *wilc should be compiled together.
>> It looks like wilc1000 is compiled without SPI or SDIO.
>>
>> Of course, there are many cases that I don't know, so you could let me know the wilc1000 build configuration?
>>
>> static int __init init_wilc_driver(void)
>> {
>> #ifdef WILC_SPI
>
> This should be #ifndef WILC_SDIO
>
>
>> struct wilc *wilc;
>> #endif
>
> But the large question remains of why do we have this variable here any
> way?
>
> regards,
> dan carpenter
>
I do not know why it is there .that is why I did not touch it first
my proposed patch was like
#if (defined WILC_SPI) || (!defined WILC_SDIO)
struct wilc *wilc;
#endif

2015-11-09 08:53:06

by Glen Lee

[permalink] [raw]
Subject: Re: [PATCH V2] Staging: wilc1000: Fix build break due to undeclared *wilc and implicit declaration of init_irq


On 2015년 11월 09일 17:18, Dan Carpenter wrote:
> On Mon, Nov 09, 2015 at 05:02:48PM +0900, glen lee wrote:
>> Hi Punit Vara,
>>
>> I cannot find build errors on my build machines.
>>
>> According the log which you have posted before says *wilc is undeclared in the function init_wilc_driver,
>> which means WILC_SPI is selected because one of SPI or SDIO should be chosen at the moment.
>> Hence, struct wilc *wilc should be compiled together.
>> It looks like wilc1000 is compiled without SPI or SDIO.
>>
>> Of course, there are many cases that I don't know, so you could let me know the wilc1000 build configuration?
>>
>> static int __init init_wilc_driver(void)
>> {
>> #ifdef WILC_SPI
> This should be #ifndef WILC_SDIO

I will do this in the next patch series.

>
>> struct wilc *wilc;
>> #endif
> But the large question remains of why do we have this variable here any
> way?

As you pointed out, the variable is do-nothing for spi driver for now.
After reworking SPI driver, the wilc will be passed to SPI as spi drive data like we already did in SDIO.

We have done this to remove extern variable g_linux_wlan which is primary structure of wilc1000.
For now it is not used, but need it not to break the build.

static int linux_sdio_probe(struct sdio_func *func, const struct sdio_device_id *id)
{
struct wilc_sdio *wl_sdio;
struct wilc *wilc;

PRINT_D(INIT_DBG, "probe function\n");
wl_sdio = kzalloc(sizeof(struct wilc_sdio), GFP_KERNEL);
if (!wl_sdio)
return -ENOMEM;

PRINT_D(INIT_DBG, "Initializing netdev\n");
local_sdio_func = func;
if (wilc_netdev_init(&wilc)) {
PRINT_ER("Couldn't initialize netdev\n");
kfree(wl_sdio);
return -1;
}
wl_sdio->func = func;
wl_sdio->wilc = wilc;
sdio_set_drvdata(func, wl_sdio);

regards,
glen lee.

>
> regards,
> dan carpenter
>

2015-11-09 09:05:52

by punit vara

[permalink] [raw]
Subject: Re: [PATCH V2] Staging: wilc1000: Fix build break due to undeclared *wilc and implicit declaration of init_irq

On Mon, Nov 9, 2015 at 2:25 PM, glen lee <[email protected]> wrote:
>
> On 2015년 11월 09일 17:18, Dan Carpenter wrote:
>>
>> On Mon, Nov 09, 2015 at 05:02:48PM +0900, glen lee wrote:
>>>
>>> Hi Punit Vara,
>>>
>>> I cannot find build errors on my build machines.
>>>
>>> According the log which you have posted before says *wilc is undeclared
>>> in the function init_wilc_driver,
>>> which means WILC_SPI is selected because one of SPI or SDIO should be
>>> chosen at the moment.
>>> Hence, struct wilc *wilc should be compiled together.
>>> It looks like wilc1000 is compiled without SPI or SDIO.
>>>
>>> Of course, there are many cases that I don't know, so you could let me
>>> know the wilc1000 build configuration?
>>>
>>> static int __init init_wilc_driver(void)
>>> {
>>> #ifdef WILC_SPI
>>
>> This should be #ifndef WILC_SDIO
>
>
> I will do this in the next patch series.
>
>>
>>> struct wilc *wilc;
>>> #endif
>>
>> But the large question remains of why do we have this variable here any
>> way?
>
>
> As you pointed out, the variable is do-nothing for spi driver for now.
> After reworking SPI driver, the wilc will be passed to SPI as spi drive data
> like we already did in SDIO.
>
> We have done this to remove extern variable g_linux_wlan which is primary
> structure of wilc1000.
> For now it is not used, but need it not to break the build.
>
> static int linux_sdio_probe(struct sdio_func *func, const struct
> sdio_device_id *id)
> {
> struct wilc_sdio *wl_sdio;
> struct wilc *wilc;
>
> PRINT_D(INIT_DBG, "probe function\n");
> wl_sdio = kzalloc(sizeof(struct wilc_sdio), GFP_KERNEL);
> if (!wl_sdio)
> return -ENOMEM;
>
> PRINT_D(INIT_DBG, "Initializing netdev\n");
> local_sdio_func = func;
> if (wilc_netdev_init(&wilc)) {
> PRINT_ER("Couldn't initialize netdev\n");
> kfree(wl_sdio);
> return -1;
> }
> wl_sdio->func = func;
> wl_sdio->wilc = wilc;
> sdio_set_drvdata(func, wl_sdio);
>
> regards,
> glen lee.
>
>>
>> regards,
>> dan carpenter
>>
>
How about this patch @Dan and @glen ,For me it does not create any
build error. For #ifndef WILC_SDIO that pointer *wilc is not
compiling so that creates the error.

--- a/drivers/staging/wilc1000/linux_wlan.c
+++ b/drivers/staging/wilc1000/linux_wlan.c
@@ -224,7 +224,7 @@ static int dev_state_ev_handler(struct
notifier_block *this, unsigned long event

}

-#if (defined WILC_SPI) || (defined WILC_SDIO_IRQ_GPIO)
+#if (defined WILC_SPI) || (defined WILC_SDIO_IRQ_GPIO) || (!defined WIC_SDIO)
static irqreturn_t isr_uh_routine(int irq, void *user_data)
{
perInterface_wlan_t *nic;
@@ -264,7 +264,7 @@ irqreturn_t isr_bh_routine(int irq, void *userdata)
return IRQ_HANDLED;
}

-#if (defined WILC_SPI) || (defined WILC_SDIO_IRQ_GPIO)
+#if (defined WILC_SPI) || (defined WILC_SDIO_IRQ_GPIO) || (!defined WILC_SDIO)
static int init_irq(struct net_device *dev)
{
int ret = 0;
@@ -1083,6 +1083,10 @@ static void wlan_deinitialize_threads(struct
net_device *dev)
}
}

+#if (!defined WILC_SDIO) || (defined WILC_SDIO_IRQ_GPIO)
+static int init_irq(struct net_device *dev);
+#endif
+
int wilc1000_wlan_init(struct net_device *dev, perInterface_wlan_t *p_nic)
{
wilc_wlan_inp_t nwi;
@@ -1791,7 +1795,7 @@ int wilc_netdev_init(struct wilc **wilc)
/*The 1st function called after module inserted*/
static int __init init_wilc_driver(void)
{
-#ifdef WILC_SPI
+#if (defined WILC_SPI) || (!defined WILC_SDIO)
struct wilc *wilc;
#endif

2015-11-09 09:58:44

by Glen Lee

[permalink] [raw]
Subject: Re: [PATCH V2] Staging: wilc1000: Fix build break due to undeclared *wilc and implicit declaration of init_irq



On 2015년 11월 09일 18:05, punit vara wrote:
> On Mon, Nov 9, 2015 at 2:25 PM, glen lee <[email protected]> wrote:
>> On 2015년 11월 09일 17:18, Dan Carpenter wrote:
>>> On Mon, Nov 09, 2015 at 05:02:48PM +0900, glen lee wrote:
>>>> Hi Punit Vara,
>>>>
>>>> I cannot find build errors on my build machines.
>>>>
>>>> According the log which you have posted before says *wilc is undeclared
>>>> in the function init_wilc_driver,
>>>> which means WILC_SPI is selected because one of SPI or SDIO should be
>>>> chosen at the moment.
>>>> Hence, struct wilc *wilc should be compiled together.
>>>> It looks like wilc1000 is compiled without SPI or SDIO.
>>>>
>>>> Of course, there are many cases that I don't know, so you could let me
>>>> know the wilc1000 build configuration?
>>>>
>>>> static int __init init_wilc_driver(void)
>>>> {
>>>> #ifdef WILC_SPI
>>> This should be #ifndef WILC_SDIO
>>
>> I will do this in the next patch series.
>>
>>>> struct wilc *wilc;
>>>> #endif
>>> But the large question remains of why do we have this variable here any
>>> way?
>>
>> As you pointed out, the variable is do-nothing for spi driver for now.
>> After reworking SPI driver, the wilc will be passed to SPI as spi drive data
>> like we already did in SDIO.
>>
>> We have done this to remove extern variable g_linux_wlan which is primary
>> structure of wilc1000.
>> For now it is not used, but need it not to break the build.
>>
>> static int linux_sdio_probe(struct sdio_func *func, const struct
>> sdio_device_id *id)
>> {
>> struct wilc_sdio *wl_sdio;
>> struct wilc *wilc;
>>
>> PRINT_D(INIT_DBG, "probe function\n");
>> wl_sdio = kzalloc(sizeof(struct wilc_sdio), GFP_KERNEL);
>> if (!wl_sdio)
>> return -ENOMEM;
>>
>> PRINT_D(INIT_DBG, "Initializing netdev\n");
>> local_sdio_func = func;
>> if (wilc_netdev_init(&wilc)) {
>> PRINT_ER("Couldn't initialize netdev\n");
>> kfree(wl_sdio);
>> return -1;
>> }
>> wl_sdio->func = func;
>> wl_sdio->wilc = wilc;
>> sdio_set_drvdata(func, wl_sdio);
>>
>> regards,
>> glen lee.
>>
>>> regards,
>>> dan carpenter
>>>
> How about this patch @Dan and @glen ,For me it does not create any
> build error. For #ifndef WILC_SDIO that pointer *wilc is not
> compiling so that creates the error.

As Dan said, I also think there are too many ifdefs now. We will remove the defines after
reworking SPI/SDIO modules.

For now, I cannot test since build error does not happens in my side. :(

Just in case, will this works for you?
config WILC1000_DRIVER
bool "WILC1000 support (WiFi only)"
depends on CFG80211 && WEXT_CORE && INET
+ depends on MMC || SPI

I will look into more about this later.

regards,
glen lee.

>
> --- a/drivers/staging/wilc1000/linux_wlan.c
> +++ b/drivers/staging/wilc1000/linux_wlan.c
> @@ -224,7 +224,7 @@ static int dev_state_ev_handler(struct
> notifier_block *this, unsigned long event
>
> }
>
> -#if (defined WILC_SPI) || (defined WILC_SDIO_IRQ_GPIO)
> +#if (defined WILC_SPI) || (defined WILC_SDIO_IRQ_GPIO) || (!defined WIC_SDIO)
> static irqreturn_t isr_uh_routine(int irq, void *user_data)
> {
> perInterface_wlan_t *nic;
> @@ -264,7 +264,7 @@ irqreturn_t isr_bh_routine(int irq, void *userdata)
> return IRQ_HANDLED;
> }
>
> -#if (defined WILC_SPI) || (defined WILC_SDIO_IRQ_GPIO)
> +#if (defined WILC_SPI) || (defined WILC_SDIO_IRQ_GPIO) || (!defined WILC_SDIO)
> static int init_irq(struct net_device *dev)
> {
> int ret = 0;
> @@ -1083,6 +1083,10 @@ static void wlan_deinitialize_threads(struct
> net_device *dev)
> }
> }
>
> +#if (!defined WILC_SDIO) || (defined WILC_SDIO_IRQ_GPIO)
> +static int init_irq(struct net_device *dev);
> +#endif
> +
> int wilc1000_wlan_init(struct net_device *dev, perInterface_wlan_t *p_nic)
> {
> wilc_wlan_inp_t nwi;
> @@ -1791,7 +1795,7 @@ int wilc_netdev_init(struct wilc **wilc)
> /*The 1st function called after module inserted*/
> static int __init init_wilc_driver(void)
> {
> -#ifdef WILC_SPI
> +#if (defined WILC_SPI) || (!defined WILC_SDIO)
> struct wilc *wilc;
> #endif

2015-11-09 10:13:42

by punit vara

[permalink] [raw]
Subject: Re: [PATCH V2] Staging: wilc1000: Fix build break due to undeclared *wilc and implicit declaration of init_irq

On Mon, Nov 9, 2015 at 3:31 PM, glen lee <[email protected]> wrote:
>
>
> On 2015년 11월 09일 18:05, punit vara wrote:
>>
>> On Mon, Nov 9, 2015 at 2:25 PM, glen lee <[email protected]> wrote:
>>>
>>> On 2015년 11월 09일 17:18, Dan Carpenter wrote:
>>>>
>>>> On Mon, Nov 09, 2015 at 05:02:48PM +0900, glen lee wrote:
>>>>>
>>>>> Hi Punit Vara,
>>>>>
>>>>> I cannot find build errors on my build machines.
>>>>>
>>>>> According the log which you have posted before says *wilc is undeclared
>>>>> in the function init_wilc_driver,
>>>>> which means WILC_SPI is selected because one of SPI or SDIO should be
>>>>> chosen at the moment.
>>>>> Hence, struct wilc *wilc should be compiled together.
>>>>> It looks like wilc1000 is compiled without SPI or SDIO.
>>>>>
>>>>> Of course, there are many cases that I don't know, so you could let me
>>>>> know the wilc1000 build configuration?
>>>>>
>>>>> static int __init init_wilc_driver(void)
>>>>> {
>>>>> #ifdef WILC_SPI
>>>>
>>>> This should be #ifndef WILC_SDIO
>>>
>>>
>>> I will do this in the next patch series.
>>>
>>>>> struct wilc *wilc;
>>>>> #endif
>>>>
>>>> But the large question remains of why do we have this variable here any
>>>> way?
>>>
>>>
>>> As you pointed out, the variable is do-nothing for spi driver for now.
>>> After reworking SPI driver, the wilc will be passed to SPI as spi drive
>>> data
>>> like we already did in SDIO.
>>>
>>> We have done this to remove extern variable g_linux_wlan which is primary
>>> structure of wilc1000.
>>> For now it is not used, but need it not to break the build.
>>>
>>> static int linux_sdio_probe(struct sdio_func *func, const struct
>>> sdio_device_id *id)
>>> {
>>> struct wilc_sdio *wl_sdio;
>>> struct wilc *wilc;
>>>
>>> PRINT_D(INIT_DBG, "probe function\n");
>>> wl_sdio = kzalloc(sizeof(struct wilc_sdio), GFP_KERNEL);
>>> if (!wl_sdio)
>>> return -ENOMEM;
>>>
>>> PRINT_D(INIT_DBG, "Initializing netdev\n");
>>> local_sdio_func = func;
>>> if (wilc_netdev_init(&wilc)) {
>>> PRINT_ER("Couldn't initialize netdev\n");
>>> kfree(wl_sdio);
>>> return -1;
>>> }
>>> wl_sdio->func = func;
>>> wl_sdio->wilc = wilc;
>>> sdio_set_drvdata(func, wl_sdio);
>>>
>>> regards,
>>> glen lee.
>>>
>>>> regards,
>>>> dan carpenter
>>>>
>> How about this patch @Dan and @glen ,For me it does not create any
>> build error. For #ifndef WILC_SDIO that pointer *wilc is not
>> compiling so that creates the error.
>
>
> As Dan said, I also think there are too many ifdefs now. We will remove the
> defines after
> reworking SPI/SDIO modules.
>
> For now, I cannot test since build error does not happens in my side. :(
>
> Just in case, will this works for you?
> config WILC1000_DRIVER
> bool "WILC1000 support (WiFi only)"
> depends on CFG80211 && WEXT_CORE && INET
> + depends on MMC || SPI
>
> I will look into more about this later.
>
> regards,
> glen lee.
>
>
>>
>> --- a/drivers/staging/wilc1000/linux_wlan.c
>> +++ b/drivers/staging/wilc1000/linux_wlan.c
>> @@ -224,7 +224,7 @@ static int dev_state_ev_handler(struct
>> notifier_block *this, unsigned long event
>>
>> }
>>
>> -#if (defined WILC_SPI) || (defined WILC_SDIO_IRQ_GPIO)
>> +#if (defined WILC_SPI) || (defined WILC_SDIO_IRQ_GPIO) || (!defined
>> WIC_SDIO)
>> static irqreturn_t isr_uh_routine(int irq, void *user_data)
>> {
>> perInterface_wlan_t *nic;
>> @@ -264,7 +264,7 @@ irqreturn_t isr_bh_routine(int irq, void *userdata)
>> return IRQ_HANDLED;
>> }
>>
>> -#if (defined WILC_SPI) || (defined WILC_SDIO_IRQ_GPIO)
>> +#if (defined WILC_SPI) || (defined WILC_SDIO_IRQ_GPIO) || (!defined
>> WILC_SDIO)
>> static int init_irq(struct net_device *dev)
>> {
>> int ret = 0;
>> @@ -1083,6 +1083,10 @@ static void wlan_deinitialize_threads(struct
>> net_device *dev)
>> }
>> }
>>
>> +#if (!defined WILC_SDIO) || (defined WILC_SDIO_IRQ_GPIO)
>> +static int init_irq(struct net_device *dev);
>> +#endif
>> +
>> int wilc1000_wlan_init(struct net_device *dev, perInterface_wlan_t
>> *p_nic)
>> {
>> wilc_wlan_inp_t nwi;
>> @@ -1791,7 +1795,7 @@ int wilc_netdev_init(struct wilc **wilc)
>> /*The 1st function called after module inserted*/
>> static int __init init_wilc_driver(void)
>> {
>> -#ifdef WILC_SPI
>> +#if (defined WILC_SPI) || (!defined WILC_SDIO)
>> struct wilc *wilc;
>> #endif
>
>
If I don't do any changes as I did . and add line in Kconfig as you told me

make ./drivers/staging/wilc1000/ this working fine for me

but

make ./drivers/staging/wilc1000/linux_wlan.o is creates error. Yes
there are two many ifdef I also agreed with Dan.

You can fetch latest changes from staging . you can create that build
error . Untill then I will follow whatever Dan say :-)

2015-11-09 10:31:54

by Sudip Mukherjee

[permalink] [raw]
Subject: Re: [PATCH V2] Staging: wilc1000: Fix build break due to undeclared *wilc and implicit declaration of init_irq

On Mon, Nov 09, 2015 at 03:43:38PM +0530, punit vara wrote:
> On Mon, Nov 9, 2015 at 3:31 PM, glen lee <[email protected]> wrote:
> >
> >
> > On 2015년 11월 09일 18:05, punit vara wrote:
> >>
> >> On Mon, Nov 9, 2015 at 2:25 PM, glen lee <[email protected]> wrote:
> >>>
> >>> On 2015년 11월 09일 17:18, Dan Carpenter wrote:
> >>>>
> >>>> On Mon, Nov 09, 2015 at 05:02:48PM +0900, glen lee wrote:
> >>>>>
>
> make ./drivers/staging/wilc1000/ this working fine for me
>
> but
>
> make ./drivers/staging/wilc1000/linux_wlan.o is creates error.

I am also not getting build failure. Can you please post your .config file.

regards
sudip

2015-11-09 14:39:48

by punit vara

[permalink] [raw]
Subject: Re: [PATCH V2] Staging: wilc1000: Fix build break due to undeclared *wilc and implicit declaration of init_irq

On Mon, Nov 9, 2015 at 3:53 PM, Sudip Mukherjee
<[email protected]> wrote:
> On Mon, Nov 09, 2015 at 03:43:38PM +0530, punit vara wrote:
>> On Mon, Nov 9, 2015 at 3:31 PM, glen lee <[email protected]> wrote:
>> >
>> >
>> > On 2015년 11월 09일 18:05, punit vara wrote:
>> >>
>> >> On Mon, Nov 9, 2015 at 2:25 PM, glen lee <[email protected]> wrote:
>> >>>
>> >>> On 2015년 11월 09일 17:18, Dan Carpenter wrote:
>> >>>>
>> >>>> On Mon, Nov 09, 2015 at 05:02:48PM +0900, glen lee wrote:
>> >>>>>
>>
>> make ./drivers/staging/wilc1000/ this working fine for me
>>
>> but
>>
>> make ./drivers/staging/wilc1000/linux_wlan.o is creates error.
>
> I am also not getting build failure. Can you please post your .config file.
>
> regards
> sudip
Can you tell me Sudip where .config file is saved ? Sorry for asking
silly question :-(

2015-11-13 14:19:10

by Sudip Mukherjee

[permalink] [raw]
Subject: Re: [PATCH V2] Staging: wilc1000: Fix build break due to undeclared *wilc and implicit declaration of init_irq

On Mon, Nov 09, 2015 at 08:09:36PM +0530, punit vara wrote:
> On Mon, Nov 9, 2015 at 3:53 PM, Sudip Mukherjee
> <[email protected]> wrote:
> > On Mon, Nov 09, 2015 at 03:43:38PM +0530, punit vara wrote:
> >> On Mon, Nov 9, 2015 at 3:31 PM, glen lee <[email protected]> wrote:
> >> >
> >> >
> >> > On 2015년 11월 09일 18:05, punit vara wrote:
> >> >>
> >> >> On Mon, Nov 9, 2015 at 2:25 PM, glen lee <[email protected]> wrote:
> >> >>>
> >> >>> On 2015년 11월 09일 17:18, Dan Carpenter wrote:
> >> >>>>
> >> >>>> On Mon, Nov 09, 2015 at 05:02:48PM +0900, glen lee wrote:
> >> >>>>>
> >>
> >> make ./drivers/staging/wilc1000/ this working fine for me
> >>
> >> but
> >>
> >> make ./drivers/staging/wilc1000/linux_wlan.o is creates error.
> >
> > I am also not getting build failure. Can you please post your .config file.
> >
> > regards
> > sudip
> Can you tell me Sudip where .config file is saved ? Sorry for asking
> silly question :-(

at the root of your linux tree. the kernel will be compiled based on the
options in these file.

regards
sudip

2015-12-02 03:09:14

by punit vara

[permalink] [raw]
Subject: Re: [PATCH V2] Staging: wilc1000: Fix build break due to undeclared *wilc and implicit declaration of init_irq

On Mon, Nov 9, 2015 at 3:53 PM, Sudip Mukherjee
<[email protected]> wrote:
> On Mon, Nov 09, 2015 at 03:43:38PM +0530, punit vara wrote:
>> On Mon, Nov 9, 2015 at 3:31 PM, glen lee <[email protected]> wrote:
>> >
>> >
>> > On 2015년 11월 09일 18:05, punit vara wrote:
>> >>
>> >> On Mon, Nov 9, 2015 at 2:25 PM, glen lee <[email protected]> wrote:
>> >>>
>> >>> On 2015년 11월 09일 17:18, Dan Carpenter wrote:
>> >>>>
>> >>>> On Mon, Nov 09, 2015 at 05:02:48PM +0900, glen lee wrote:
>> >>>>>
>>
>> make ./drivers/staging/wilc1000/ this working fine for me
>>
>> but
>>
>> make ./drivers/staging/wilc1000/linux_wlan.o is creates error.
>
> I am also not getting build failure. Can you please post your .config file.
>
> regards
> sudip
I have attached .config file . and please tell me should I focus on
this patch or not . Because build is still broken.For temporary this
patch can be useful to fix the build .Later on as per requirement Glen
can change the code .


Attachments:
config.gz (50.14 kB)

2015-12-02 07:00:47

by Sudip Mukherjee

[permalink] [raw]
Subject: Re: [PATCH V2] Staging: wilc1000: Fix build break due to undeclared *wilc and implicit declaration of init_irq

On Wed, Dec 02, 2015 at 08:39:10AM +0530, punit vara wrote:
> On Mon, Nov 9, 2015 at 3:53 PM, Sudip Mukherjee
> <[email protected]> wrote:
> > On Mon, Nov 09, 2015 at 03:43:38PM +0530, punit vara wrote:
> >> On Mon, Nov 9, 2015 at 3:31 PM, glen lee <[email protected]> wrote:
> >> >
> >> >
> >> > On 2015년 11월 09일 18:05, punit vara wrote:
> >> >>
> >> >> On Mon, Nov 9, 2015 at 2:25 PM, glen lee <[email protected]> wrote:
> >> >>>
> >> >>> On 2015년 11월 09일 17:18, Dan Carpenter wrote:
> >> >>>>
> >> >>>> On Mon, Nov 09, 2015 at 05:02:48PM +0900, glen lee wrote:
> >> >>>>>
> >>
> >> make ./drivers/staging/wilc1000/ this working fine for me
> >>
> >> but
> >>
> >> make ./drivers/staging/wilc1000/linux_wlan.o is creates error.
> >
> > I am also not getting build failure. Can you please post your .config file.
> >
> > regards
> > sudip
> I have attached .config file . and please tell me should I focus on
> this patch or not . Because build is still broken.For temporary this
> patch can be useful to fix the build .Later on as per requirement Glen
> can change the code .

With your config file also I am not getting any build failure. Can you
please tell what are the exact compilation steps you are following?

Here are my steps after I have copied your config file as .config.
1) make oldconfig && make prepare
2) make bzImage && make modules

And I dont see any build failure.
I think you are not building bzImage and the modules before testing wilc
compilation. And that is why it is unable to find SPI or SDIO.

regards
sudip

2015-12-02 07:31:58

by punit vara

[permalink] [raw]
Subject: Re: [PATCH V2] Staging: wilc1000: Fix build break due to undeclared *wilc and implicit declaration of init_irq

On Wed, Dec 2, 2015 at 12:30 PM, Sudip Mukherjee
<[email protected]> wrote:
> On Wed, Dec 02, 2015 at 08:39:10AM +0530, punit vara wrote:
>> On Mon, Nov 9, 2015 at 3:53 PM, Sudip Mukherjee
>> <[email protected]> wrote:
>> > On Mon, Nov 09, 2015 at 03:43:38PM +0530, punit vara wrote:
>> >> On Mon, Nov 9, 2015 at 3:31 PM, glen lee <[email protected]> wrote:
>> >> >
>> >> >
>> >> > On 2015년 11월 09일 18:05, punit vara wrote:
>> >> >>
>> >> >> On Mon, Nov 9, 2015 at 2:25 PM, glen lee <[email protected]> wrote:
>> >> >>>
>> >> >>> On 2015년 11월 09일 17:18, Dan Carpenter wrote:
>> >> >>>>
>> >> >>>> On Mon, Nov 09, 2015 at 05:02:48PM +0900, glen lee wrote:
>> >> >>>>>
>> >>
>> >> make ./drivers/staging/wilc1000/ this working fine for me
>> >>
>> >> but
>> >>
>> >> make ./drivers/staging/wilc1000/linux_wlan.o is creates error.
>> >
>> > I am also not getting build failure. Can you please post your .config file.
>> >
>> > regards
>> > sudip
>> I have attached .config file . and please tell me should I focus on
>> this patch or not . Because build is still broken.For temporary this
>> patch can be useful to fix the build .Later on as per requirement Glen
>> can change the code .
>
> With your config file also I am not getting any build failure. Can you
> please tell what are the exact compilation steps you are following?
>
> Here are my steps after I have copied your config file as .config.
> 1) make oldconfig && make prepare
> 2) make bzImage && make modules
>
> And I dont see any build failure.
> I think you are not building bzImage and the modules before testing wilc
> compilation. And that is why it is unable to find SPI or SDIO.
>
> regards
> sudip
Steps I have followed:
1. I have updated staging tree and checkout to new branch.
2. make drivers/staging/wilc1000/linux_wlan.o

When I have performed second step it automatically ask me about
different modules options "yes/no" I have pressed ENTER in every
option .At last configuration are written to .config. and module start
building and I have found error at last. I will try to perform 2 step
you have suggested .