Message-ID: <CABXAfNKhtdRSm1BJK+pk9t4wwnFBXzAP_CKSVhS0p7m_FZG_8Q@mail.gmail.com>
Date:	Mon, 9 Nov 2015 15:43:38 +0530
From:	punit vara <punitvara@...il.com>
To:	glen lee <glen.lee@...el.com>
Cc:	Dan Carpenter <dan.carpenter@...cle.com>,
	devel@...verdev.osuosl.org, chris.park@...el.com,
	Austin Shin <austin.shin@...el.com>,
	Greg KH <gregkh@...uxfoundation.org>,
	linux-wireless@...r.kernel.org, johnny.kim@...el.com,
	linux-kernel@...r.kernel.org, tony.cho@...el.com, leo.kim@...el.com
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 <glen.lee@...el.com> wrote:
>
>
> On 2015년 11월 09일 18:05, punit vara wrote:
>>
>> On Mon, Nov 9, 2015 at 2:25 PM, glen lee <glen.lee@...el.com> 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 :-)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists