All of lore.kernel.org
 help / color / mirror / Atom feed
* feedback on mainlining wilc1000 staging driver
@ 2018-08-15 20:22 Ajay Singh
  2018-08-16  6:43 ` Greg KH
  2018-08-16 10:47 ` Kalle Valo
  0 siblings, 2 replies; 15+ messages in thread
From: Ajay Singh @ 2018-08-15 20:22 UTC (permalink / raw)
  To: devel, gregkh, linux-wireless
  Cc: ganesh.krishna, aditya.shankar, nicolas.ferre, claudiu.beznea,
	adham.abozaeid

Hi Greg,

We all are working on submitting and reviewing patches for wilc1000 in
staging driver for quite some time. 

We would like to have feedback on the next steps to bring wilc1000
driver closer to move into the wireless subsystem tree. 

In summary, the following major things from TODO have been addressed in
staging:
-remove the defined feature as kernel versions
-remove OS wrapper functions
-remove custom debug and tracing functions
-rework comments and function headers(also coding style)
-remove build warnings
-replace all semaphores with mutexes or completions
-make spi and sdio components coexist in one build
-turn compile-time platform configuration (BEAGLE_BOARD, PANDA_BOARD,
PLAT_WMS8304, PLAT_RKXXXX, CUSTOMER_PLATFORM, ...) into run-time
options that are read from DT
-replace SIOCDEVPRIVATE commands with generic API functions
-use wext-core handling instead of private SIOCSIWPRIV implementation
-Move handling for each individual members of 'union message_body' out
into a separate 'struct work_struct' and completely remove the
multiplexer that is currently part of host_if_work(), allowing movement
of the implementation of each message handler into the callsite of the
function that currently queues the 'host_if_msg'.
-convert all uses of the old GPIO API from <linux/gpio.h> to the GPIO
descriptor API in <linux/gpio/consumer.h> and look up GPIO lines from
device tree, ACPI or board files, board files should use
<linux/gpio/machine.h>

I have few patches to submit based on last week comments. I
will send these patches as soon as the merge window is closed.

1. Delete wilc_debugfs.c file, as not used.
2. Remove use of remaining global variables.
3. Remove udelay realted checkpatch warning.
4. Use void return for function whose return value is not used.


Currently, there are 2 known items pending in our TODO. 
-support soft-ap and p2p mode
-support resume/suspend function
 
For the above 2 remaining TODO items, we need to add extra code but as
suggested in [1], we have not taken up this activity yet.
 
We would like to know your opinion whether we should start working on
these features in staging-next tree or take it up after having moved
the code to wireless sub-system.

Do you think it’s better to initiate review in parallel with
wireless subsystem maintainer to identify any pending TODO items. 

Please guide us on how to proceed further.

[1] . https://patchwork.kernel.org/patch/9809145
 

Regards,
Ajay

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: feedback on mainlining wilc1000 staging driver
  2018-08-16  6:43 ` Greg KH
@ 2018-08-15 20:53   ` Ajay Singh
  0 siblings, 0 replies; 15+ messages in thread
From: Ajay Singh @ 2018-08-15 20:53 UTC (permalink / raw)
  To: Greg KH
  Cc: devel, linux-wireless, ganesh.krishna, aditya.shankar,
	nicolas.ferre, claudiu.beznea, adham.abozaeid

Hi Greg,

On Thu, 16 Aug 2018 08:43:54 +0200
Greg KH <gregkh@linuxfoundation.org> wrote:

> On Thu, Aug 16, 2018 at 01:52:43AM +0530, Ajay Singh wrote:
> > Hi Greg,
> > 
> > We all are working on submitting and reviewing patches for wilc1000
> > in staging driver for quite some time. 
> > 
> > We would like to have feedback on the next steps to bring wilc1000
> > driver closer to move into the wireless subsystem tree.   
> 
> <snip>
> 
> It is the middle of the merge window, combined with all of the 1ltf
> crap, and other stable stuff.  Can you wait until after the merge
> window for this please?  There's nothing we can do now anyway.
> Submit the rest of your pending patches, get those merged and then we
> can review the code to see what you have left to do.
> 

Thanks for your time.
I can wait till the end of merge window to discuss this. 
My intention is not to send changes during the current merge window
but to understand what is pending for our driver.
Let's revisit this later as you have suggested.

Regards,
Ajay

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: feedback on mainlining wilc1000 staging driver
  2018-08-15 20:22 feedback on mainlining wilc1000 staging driver Ajay Singh
@ 2018-08-16  6:43 ` Greg KH
  2018-08-15 20:53   ` Ajay Singh
  2018-08-16 10:47 ` Kalle Valo
  1 sibling, 1 reply; 15+ messages in thread
From: Greg KH @ 2018-08-16  6:43 UTC (permalink / raw)
  To: Ajay Singh
  Cc: devel, linux-wireless, ganesh.krishna, aditya.shankar,
	nicolas.ferre, claudiu.beznea, adham.abozaeid

On Thu, Aug 16, 2018 at 01:52:43AM +0530, Ajay Singh wrote:
> Hi Greg,
> 
> We all are working on submitting and reviewing patches for wilc1000 in
> staging driver for quite some time. 
> 
> We would like to have feedback on the next steps to bring wilc1000
> driver closer to move into the wireless subsystem tree. 

<snip>

It is the middle of the merge window, combined with all of the 1ltf
crap, and other stable stuff.  Can you wait until after the merge window
for this please?  There's nothing we can do now anyway.  Submit the rest
of your pending patches, get those merged and then we can review the
code to see what you have left to do.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: feedback on mainlining wilc1000 staging driver
  2018-08-15 20:22 feedback on mainlining wilc1000 staging driver Ajay Singh
  2018-08-16  6:43 ` Greg KH
@ 2018-08-16 10:47 ` Kalle Valo
  2018-08-16 10:53   ` Kalle Valo
  1 sibling, 1 reply; 15+ messages in thread
From: Kalle Valo @ 2018-08-16 10:47 UTC (permalink / raw)
  To: Ajay Singh
  Cc: devel, gregkh, linux-wireless, ganesh.krishna, aditya.shankar,
	nicolas.ferre, claudiu.beznea, adham.abozaeid

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

> Hi Greg,
>
> We all are working on submitting and reviewing patches for wilc1000 in
> staging driver for quite some time. 
>
> We would like to have feedback on the next steps to bring wilc1000
> driver closer to move into the wireless subsystem tree. 
>
> In summary, the following major things from TODO have been addressed in
> staging:
> -remove the defined feature as kernel versions
> -remove OS wrapper functions
> -remove custom debug and tracing functions
> -rework comments and function headers(also coding style)
> -remove build warnings
> -replace all semaphores with mutexes or completions
> -make spi and sdio components coexist in one build
> -turn compile-time platform configuration (BEAGLE_BOARD, PANDA_BOARD,
> PLAT_WMS8304, PLAT_RKXXXX, CUSTOMER_PLATFORM, ...) into run-time
> options that are read from DT
> -replace SIOCDEVPRIVATE commands with generic API functions
> -use wext-core handling instead of private SIOCSIWPRIV implementation

>From wireless point of view: if I see wext mentioned anywhere in the
driver I stop right there. cfg80211 is a hard requirement for us
nowadays.

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: feedback on mainlining wilc1000 staging driver
  2018-08-16 10:47 ` Kalle Valo
@ 2018-08-16 10:53   ` Kalle Valo
  2018-08-17  4:32     ` Ajay Singh
  0 siblings, 1 reply; 15+ messages in thread
From: Kalle Valo @ 2018-08-16 10:53 UTC (permalink / raw)
  To: Ajay Singh
  Cc: devel, gregkh, linux-wireless, ganesh.krishna, aditya.shankar,
	nicolas.ferre, claudiu.beznea, adham.abozaeid

Kalle Valo <kvalo@codeaurora.org> writes:

> Ajay Singh <ajay.kathat@microchip.com> writes:
>
>> Hi Greg,
>>
>> We all are working on submitting and reviewing patches for wilc1000 in
>> staging driver for quite some time. 
>>
>> We would like to have feedback on the next steps to bring wilc1000
>> driver closer to move into the wireless subsystem tree. 
>>
>> In summary, the following major things from TODO have been addressed in
>> staging:
>> -remove the defined feature as kernel versions
>> -remove OS wrapper functions
>> -remove custom debug and tracing functions
>> -rework comments and function headers(also coding style)
>> -remove build warnings
>> -replace all semaphores with mutexes or completions
>> -make spi and sdio components coexist in one build
>> -turn compile-time platform configuration (BEAGLE_BOARD, PANDA_BOARD,
>> PLAT_WMS8304, PLAT_RKXXXX, CUSTOMER_PLATFORM, ...) into run-time
>> options that are read from DT
>> -replace SIOCDEVPRIVATE commands with generic API functions
>> -use wext-core handling instead of private SIOCSIWPRIV implementation
>
> From wireless point of view: if I see wext mentioned anywhere in the
> driver I stop right there. cfg80211 is a hard requirement for us
> nowadays.

Clarification: Depending on the hardware design either cfg80211 or
mac80211 is a hard requirement. I haven't checked wilc1000 at all so I
don't know what design it has but if it's a "softmac" design then it has
to use mac80211, we do not want to support multiple 802.11 UMAC stacks.

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: feedback on mainlining wilc1000 staging driver
  2018-08-16 10:53   ` Kalle Valo
@ 2018-08-17  4:32     ` Ajay Singh
  2018-08-17  7:49       ` Kalle Valo
  0 siblings, 1 reply; 15+ messages in thread
From: Ajay Singh @ 2018-08-17  4:32 UTC (permalink / raw)
  To: Kalle Valo
  Cc: devel, gregkh, linux-wireless, nicolas.ferre, ganesh.krishna,
	adham.abozaeid, aditya.shankar

On Thu, 16 Aug 2018 13:53:50 +0300
Kalle Valo <kvalo@codeaurora.org> wrote:

> Kalle Valo <kvalo@codeaurora.org> writes:
> 
> > Ajay Singh <ajay.kathat@microchip.com> writes:
> >  
> >> Hi Greg,
> >>
> >> We all are working on submitting and reviewing patches for
> >> wilc1000 in staging driver for quite some time. 
> >>
> >> We would like to have feedback on the next steps to bring wilc1000
> >> driver closer to move into the wireless subsystem tree. 
> >>
> >> In summary, the following major things from TODO have been
> >> addressed in staging:
> >> -remove the defined feature as kernel versions
> >> -remove OS wrapper functions
> >> -remove custom debug and tracing functions
> >> -rework comments and function headers(also coding style)
> >> -remove build warnings
> >> -replace all semaphores with mutexes or completions
> >> -make spi and sdio components coexist in one build
> >> -turn compile-time platform configuration (BEAGLE_BOARD,
> >> PANDA_BOARD, PLAT_WMS8304, PLAT_RKXXXX, CUSTOMER_PLATFORM, ...)
> >> into run-time options that are read from DT
> >> -replace SIOCDEVPRIVATE commands with generic API functions
> >> -use wext-core handling instead of private SIOCSIWPRIV
> >> implementation  
> >
> > From wireless point of view: if I see wext mentioned anywhere in the
> > driver I stop right there. cfg80211 is a hard requirement for us
> > nowadays.  
> 
> Clarification: Depending on the hardware design either cfg80211 or
> mac80211 is a hard requirement. I haven't checked wilc1000 at all so I
> don't know what design it has but if it's a "softmac" design then it
> has to use mac80211, we do not want to support multiple 802.11 UMAC
> stacks.
> 

The TODO item to make use of wext-core is obsolete. Previously wilc1000
driver also had few wext ioctl use but now it’s completely removed and
cfg80211 operation callbacks are used.

wilc1000 driver make use of cfg80211 and isn’t a "softmac".

We need help to review and identify if there are any pending items
for wilc1000 driver, so we can address those issues and make it ready
to move to the wireless subsystem.

Regards,
Ajay

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: feedback on mainlining wilc1000 staging driver
  2018-08-17  4:32     ` Ajay Singh
@ 2018-08-17  7:49       ` Kalle Valo
  2018-08-17  8:21         ` Arend van Spriel
  0 siblings, 1 reply; 15+ messages in thread
From: Kalle Valo @ 2018-08-17  7:49 UTC (permalink / raw)
  To: Ajay Singh
  Cc: devel, gregkh, linux-wireless, nicolas.ferre, ganesh.krishna,
	adham.abozaeid, aditya.shankar

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

> On Thu, 16 Aug 2018 13:53:50 +0300
> Kalle Valo <kvalo@codeaurora.org> wrote:
>
>> Kalle Valo <kvalo@codeaurora.org> writes:
>>=20
>> > Ajay Singh <ajay.kathat@microchip.com> writes:
>> >=20=20
>> >> Hi Greg,
>> >>
>> >> We all are working on submitting and reviewing patches for
>> >> wilc1000 in staging driver for quite some time.=20
>> >>
>> >> We would like to have feedback on the next steps to bring wilc1000
>> >> driver closer to move into the wireless subsystem tree.=20
>> >>
>> >> In summary, the following major things from TODO have been
>> >> addressed in staging:
>> >> -remove the defined feature as kernel versions
>> >> -remove OS wrapper functions
>> >> -remove custom debug and tracing functions
>> >> -rework comments and function headers(also coding style)
>> >> -remove build warnings
>> >> -replace all semaphores with mutexes or completions
>> >> -make spi and sdio components coexist in one build
>> >> -turn compile-time platform configuration (BEAGLE_BOARD,
>> >> PANDA_BOARD, PLAT_WMS8304, PLAT_RKXXXX, CUSTOMER_PLATFORM, ...)
>> >> into run-time options that are read from DT
>> >> -replace SIOCDEVPRIVATE commands with generic API functions
>> >> -use wext-core handling instead of private SIOCSIWPRIV
>> >> implementation=20=20
>> >
>> > From wireless point of view: if I see wext mentioned anywhere in the
>> > driver I stop right there. cfg80211 is a hard requirement for us
>> > nowadays.=20=20
>>=20
>> Clarification: Depending on the hardware design either cfg80211 or
>> mac80211 is a hard requirement. I haven't checked wilc1000 at all so I
>> don't know what design it has but if it's a "softmac" design then it
>> has to use mac80211, we do not want to support multiple 802.11 UMAC
>> stacks.
>>=20
>
> The TODO item to make use of wext-core is obsolete. Previously wilc1000
> driver also had few wext ioctl use but now it=E2=80=99s completely remove=
d and
> cfg80211 operation callbacks are used.
>
> wilc1000 driver make use of cfg80211 and isn=E2=80=99t a "softmac".

Good.

> We need help to review and identify if there are any pending items
> for wilc1000 driver, so we can address those issues and make it ready
> to move to the wireless subsystem.

I think the best way to get that forward is to submit a patch (or
patchset) to linux-wireless, that's the easiest for reviewers.

--=20
Kalle Valo

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: feedback on mainlining wilc1000 staging driver
  2018-08-17  7:49       ` Kalle Valo
@ 2018-08-17  8:21         ` Arend van Spriel
  2018-08-17  8:36           ` Kalle Valo
  0 siblings, 1 reply; 15+ messages in thread
From: Arend van Spriel @ 2018-08-17  8:21 UTC (permalink / raw)
  To: Kalle Valo, Ajay Singh
  Cc: devel, gregkh, linux-wireless, nicolas.ferre, ganesh.krishna,
	adham.abozaeid, aditya.shankar

On 8/17/2018 9:49 AM, Kalle Valo wrote:
> Ajay Singh <ajay.kathat@microchip.com> writes:
>
>> On Thu, 16 Aug 2018 13:53:50 +0300
>> Kalle Valo <kvalo@codeaurora.org> wrote:
>>
>>> Kalle Valo <kvalo@codeaurora.org> writes:
>>>
>>>> Ajay Singh <ajay.kathat@microchip.com> writes:
>>>>
>>>>> Hi Greg,
>>>>>
>>>>> We all are working on submitting and reviewing patches for
>>>>> wilc1000 in staging driver for quite some time.
>>>>>
>>>>> We would like to have feedback on the next steps to bring wilc1000
>>>>> driver closer to move into the wireless subsystem tree.
>>>>>
>>>>> In summary, the following major things from TODO have been
>>>>> addressed in staging:
>>>>> -remove the defined feature as kernel versions
>>>>> -remove OS wrapper functions
>>>>> -remove custom debug and tracing functions
>>>>> -rework comments and function headers(also coding style)
>>>>> -remove build warnings
>>>>> -replace all semaphores with mutexes or completions
>>>>> -make spi and sdio components coexist in one build
>>>>> -turn compile-time platform configuration (BEAGLE_BOARD,
>>>>> PANDA_BOARD, PLAT_WMS8304, PLAT_RKXXXX, CUSTOMER_PLATFORM, ...)
>>>>> into run-time options that are read from DT
>>>>> -replace SIOCDEVPRIVATE commands with generic API functions
>>>>> -use wext-core handling instead of private SIOCSIWPRIV
>>>>> implementation
>>>>
>>>>  From wireless point of view: if I see wext mentioned anywhere in the
>>>> driver I stop right there. cfg80211 is a hard requirement for us
>>>> nowadays.
>>>
>>> Clarification: Depending on the hardware design either cfg80211 or
>>> mac80211 is a hard requirement. I haven't checked wilc1000 at all so I
>>> don't know what design it has but if it's a "softmac" design then it
>>> has to use mac80211, we do not want to support multiple 802.11 UMAC
>>> stacks.
>>>
>>
>> The TODO item to make use of wext-core is obsolete. Previously wilc1000
>> driver also had few wext ioctl use but now it’s completely removed and
>> cfg80211 operation callbacks are used.
>>
>> wilc1000 driver make use of cfg80211 and isn’t a "softmac".
>
> Good.
>
>> We need help to review and identify if there are any pending items
>> for wilc1000 driver, so we can address those issues and make it ready
>> to move to the wireless subsystem.
>
> I think the best way to get that forward is to submit a patch (or
> patchset) to linux-wireless, that's the easiest for reviewers.

For brcm80211 drivers we used a single patch introducing it under the 
wireless drivers folder. Because it was quite a sizable patch we parked 
it on the wireless wiki page. Had a few iterations doing it like that.

Regards,
Arend

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: feedback on mainlining wilc1000 staging driver
  2018-08-17  8:21         ` Arend van Spriel
@ 2018-08-17  8:36           ` Kalle Valo
  2018-08-17  9:09             ` Ajay Singh
  0 siblings, 1 reply; 15+ messages in thread
From: Kalle Valo @ 2018-08-17  8:36 UTC (permalink / raw)
  To: Arend van Spriel
  Cc: Ajay Singh, devel, gregkh, linux-wireless, nicolas.ferre,
	ganesh.krishna, adham.abozaeid, aditya.shankar

Arend van Spriel <arend.vanspriel@broadcom.com> writes:

> On 8/17/2018 9:49 AM, Kalle Valo wrote:
>> Ajay Singh <ajay.kathat@microchip.com> writes:
>>
>>> On Thu, 16 Aug 2018 13:53:50 +0300
>>> Kalle Valo <kvalo@codeaurora.org> wrote:
>>>
>>>> Kalle Valo <kvalo@codeaurora.org> writes:
>>>>
>>>>>  From wireless point of view: if I see wext mentioned anywhere in the
>>>>> driver I stop right there. cfg80211 is a hard requirement for us
>>>>> nowadays.
>>>>
>>>> Clarification: Depending on the hardware design either cfg80211 or
>>>> mac80211 is a hard requirement. I haven't checked wilc1000 at all so I
>>>> don't know what design it has but if it's a "softmac" design then it
>>>> has to use mac80211, we do not want to support multiple 802.11 UMAC
>>>> stacks.
>>>>
>>>
>>> The TODO item to make use of wext-core is obsolete. Previously wilc1000
>>> driver also had few wext ioctl use but now it=E2=80=99s completely remo=
ved and
>>> cfg80211 operation callbacks are used.
>>>
>>> wilc1000 driver make use of cfg80211 and isn=E2=80=99t a "softmac".
>>
>> Good.
>>
>>> We need help to review and identify if there are any pending items
>>> for wilc1000 driver, so we can address those issues and make it ready
>>> to move to the wireless subsystem.
>>
>> I think the best way to get that forward is to submit a patch (or
>> patchset) to linux-wireless, that's the easiest for reviewers.
>
> For brcm80211 drivers we used a single patch introducing it under the
> wireless drivers folder. Because it was quite a sizable patch we
> parked it on the wireless wiki page. Had a few iterations doing it
> like that.

Another option is to split it so that there's one patch per file, should
be even pretty easy to automate that. It's just so much easier to
comment on a patch submitted by email compared to the reviewer manually
copying code and then commenting it, yuck.

--=20
Kalle Valo

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: feedback on mainlining wilc1000 staging driver
  2018-08-17  8:36           ` Kalle Valo
@ 2018-08-17  9:09             ` Ajay Singh
  2018-08-23 11:07               ` Kalle Valo
  0 siblings, 1 reply; 15+ messages in thread
From: Ajay Singh @ 2018-08-17  9:09 UTC (permalink / raw)
  To: Kalle Valo
  Cc: Arend van Spriel, devel, gregkh, linux-wireless, nicolas.ferre,
	ganesh.krishna, adham.abozaeid, aditya.shankar

On Fri, 17 Aug 2018 11:36:02 +0300
Kalle Valo <kvalo@codeaurora.org> wrote:

> Arend van Spriel <arend.vanspriel@broadcom.com> writes:
> 
> > On 8/17/2018 9:49 AM, Kalle Valo wrote:  
> >> Ajay Singh <ajay.kathat@microchip.com> writes:
> >>  
> >>> On Thu, 16 Aug 2018 13:53:50 +0300
> >>> Kalle Valo <kvalo@codeaurora.org> wrote:
> >>>  
> >>>> Kalle Valo <kvalo@codeaurora.org> writes:
> >>>>  
> >>>>>  From wireless point of view: if I see wext mentioned anywhere
> >>>>> in the driver I stop right there. cfg80211 is a hard
> >>>>> requirement for us nowadays.  
> >>>>
> >>>> Clarification: Depending on the hardware design either cfg80211
> >>>> or mac80211 is a hard requirement. I haven't checked wilc1000 at
> >>>> all so I don't know what design it has but if it's a "softmac"
> >>>> design then it has to use mac80211, we do not want to support
> >>>> multiple 802.11 UMAC stacks.
> >>>>  
> >>>
> >>> The TODO item to make use of wext-core is obsolete. Previously
> >>> wilc1000 driver also had few wext ioctl use but now it’s
> >>> completely removed and cfg80211 operation callbacks are used.
> >>>
> >>> wilc1000 driver make use of cfg80211 and isn’t a "softmac".  
> >>
> >> Good.
> >>  
> >>> We need help to review and identify if there are any pending items
> >>> for wilc1000 driver, so we can address those issues and make it
> >>> ready to move to the wireless subsystem.  
> >>
> >> I think the best way to get that forward is to submit a patch (or
> >> patchset) to linux-wireless, that's the easiest for reviewers.  
> >
> > For brcm80211 drivers we used a single patch introducing it under
> > the wireless drivers folder. Because it was quite a sizable patch we
> > parked it on the wireless wiki page. Had a few iterations doing it
> > like that.  
> 
> Another option is to split it so that there's one patch per file,
> should be even pretty easy to automate that. It's just so much easier
> to comment on a patch submitted by email compared to the reviewer
> manually copying code and then commenting it, yuck.
> 

Sure. I will prepare a patch per file send for review as its easy to
review.

As Greg suggested, I will wait for the merge window to close and
after completing pending patches to staging, I will start the review.

For my understanding, the patches for review will be based on
wireless-testing branch. And the fixes will be submitted to staging tree
in parallel. right?

Regards,
Ajay

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: feedback on mainlining wilc1000 staging driver
  2018-08-17  9:09             ` Ajay Singh
@ 2018-08-23 11:07               ` Kalle Valo
  2018-09-26 10:27                 ` Ajay Singh
  0 siblings, 1 reply; 15+ messages in thread
From: Kalle Valo @ 2018-08-23 11:07 UTC (permalink / raw)
  To: Ajay Singh
  Cc: Arend van Spriel, devel, gregkh, linux-wireless, nicolas.ferre,
	ganesh.krishna, adham.abozaeid, aditya.shankar

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

>> >>> We need help to review and identify if there are any pending items
>> >>> for wilc1000 driver, so we can address those issues and make it
>> >>> ready to move to the wireless subsystem.  
>> >>
>> >> I think the best way to get that forward is to submit a patch (or
>> >> patchset) to linux-wireless, that's the easiest for reviewers.  
>> >
>> > For brcm80211 drivers we used a single patch introducing it under
>> > the wireless drivers folder. Because it was quite a sizable patch we
>> > parked it on the wireless wiki page. Had a few iterations doing it
>> > like that.  
>> 
>> Another option is to split it so that there's one patch per file,
>> should be even pretty easy to automate that. It's just so much easier
>> to comment on a patch submitted by email compared to the reviewer
>> manually copying code and then commenting it, yuck.
>> 
>
> Sure. I will prepare a patch per file send for review as its easy to
> review.
>
> As Greg suggested, I will wait for the merge window to close and
> after completing pending patches to staging, I will start the review.
>
> For my understanding, the patches for review will be based on
> wireless-testing branch.

In this case I think wireless-drivers-next is the safest choise,
wireless-testing also has other trees which might cause conflicts etc.

> And the fixes will be submitted to staging tree in parallel. right?

I don't think the fixes matter in the initial review. So yeah, Greg can
apply fixes as he sees fit for now. At some point we need to do a cut
off period and freeze the driver for the last review round before it
will be (hopefully) moved to drivers/net/wireless, but let's coordinate
with Greg once we are at that stage.

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: feedback on mainlining wilc1000 staging driver
  2018-08-23 11:07               ` Kalle Valo
@ 2018-09-26 10:27                 ` Ajay Singh
  2018-10-04 12:27                   ` Kalle Valo
  0 siblings, 1 reply; 15+ messages in thread
From: Ajay Singh @ 2018-09-26 10:27 UTC (permalink / raw)
  To: Kalle Valo
  Cc: Arend van Spriel, devel, gregkh, linux-wireless,
	Nicolas Ferre - M43238, Ganesh Krishna - C00112,
	Adham Abozaeid - C43058, Aditya Shankar - I16078

Hi Kalle,

On Thu, 23 Aug 2018 16:37:45 +0530
Kalle Valo <kvalo@codeaurora.org> wrote:

> Ajay Singh <ajay.kathat@microchip.com> writes:
> 
> >> >>> We need help to review and identify if there are any pending
> >> >>> items for wilc1000 driver, so we can address those issues and
> >> >>> make it ready to move to the wireless subsystem.    
> >> >>
> >> >> I think the best way to get that forward is to submit a patch
> >> >> (or patchset) to linux-wireless, that's the easiest for
> >> >> reviewers.    
> >> >
> >> > For brcm80211 drivers we used a single patch introducing it under
> >> > the wireless drivers folder. Because it was quite a sizable
> >> > patch we parked it on the wireless wiki page. Had a few
> >> > iterations doing it like that.    
> >> 
> >> Another option is to split it so that there's one patch per file,
> >> should be even pretty easy to automate that. It's just so much
> >> easier to comment on a patch submitted by email compared to the
> >> reviewer manually copying code and then commenting it, yuck.
> >>   
> >
> > Sure. I will prepare a patch per file send for review as its easy to
> > review.
> >
> > As Greg suggested, I will wait for the merge window to close and
> > after completing pending patches to staging, I will start the
> > review.
> >
> > For my understanding, the patches for review will be based on
> > wireless-testing branch.  
> 
> In this case I think wireless-drivers-next is the safest choise,
> wireless-testing also has other trees which might cause conflicts etc.

I have submitted a patch series for wilc1000 driver, single file
per patch and its based on wireless-drivers-next. 
I hope its done correctly, please provide inputs so we can
address and make this driver ready for mainline.

Regards,
Ajay


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: feedback on mainlining wilc1000 staging driver
  2018-09-26 10:27                 ` Ajay Singh
@ 2018-10-04 12:27                   ` Kalle Valo
  2018-10-05  2:33                     ` Ajay Singh
  0 siblings, 1 reply; 15+ messages in thread
From: Kalle Valo @ 2018-10-04 12:27 UTC (permalink / raw)
  To: Ajay Singh
  Cc: Arend van Spriel, devel, gregkh, linux-wireless,
	Nicolas Ferre - M43238, Ganesh Krishna - C00112,
	Adham Abozaeid - C43058, Aditya Shankar - I16078

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

> Hi Kalle,
>
> On Thu, 23 Aug 2018 16:37:45 +0530
> Kalle Valo <kvalo@codeaurora.org> wrote:
>
>> Ajay Singh <ajay.kathat@microchip.com> writes:
>> 
>> >> >>> We need help to review and identify if there are any pending
>> >> >>> items for wilc1000 driver, so we can address those issues and
>> >> >>> make it ready to move to the wireless subsystem.    
>> >> >>
>> >> >> I think the best way to get that forward is to submit a patch
>> >> >> (or patchset) to linux-wireless, that's the easiest for
>> >> >> reviewers.    
>> >> >
>> >> > For brcm80211 drivers we used a single patch introducing it under
>> >> > the wireless drivers folder. Because it was quite a sizable
>> >> > patch we parked it on the wireless wiki page. Had a few
>> >> > iterations doing it like that.    
>> >> 
>> >> Another option is to split it so that there's one patch per file,
>> >> should be even pretty easy to automate that. It's just so much
>> >> easier to comment on a patch submitted by email compared to the
>> >> reviewer manually copying code and then commenting it, yuck.
>> >>   
>> >
>> > Sure. I will prepare a patch per file send for review as its easy to
>> > review.
>> >
>> > As Greg suggested, I will wait for the merge window to close and
>> > after completing pending patches to staging, I will start the
>> > review.
>> >
>> > For my understanding, the patches for review will be based on
>> > wireless-testing branch.  
>> 
>> In this case I think wireless-drivers-next is the safest choise,
>> wireless-testing also has other trees which might cause conflicts etc.
>
> I have submitted a patch series for wilc1000 driver, single file
> per patch and its based on wireless-drivers-next. 
> I hope its done correctly, please provide inputs so we can
> address and make this driver ready for mainline.

Thanks, I see it in patchwork:

https://patchwork.kernel.org/project/linux-wireless/list/?series=23251&state=*&order=date

Do note that we have also another new driver (rtwlan/rtw88) under review
so review from the wireless folks might take some time.

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: feedback on mainlining wilc1000 staging driver
  2018-10-04 12:27                   ` Kalle Valo
@ 2018-10-05  2:33                     ` Ajay Singh
  2018-10-05  5:16                       ` Kalle Valo
  0 siblings, 1 reply; 15+ messages in thread
From: Ajay Singh @ 2018-10-05  2:33 UTC (permalink / raw)
  To: Kalle Valo
  Cc: Arend van Spriel, devel, gregkh, linux-wireless,
	Nicolas Ferre - M43238, Ganesh Krishna - C00112,
	Adham Abozaeid - C43058, Aditya Shankar - I16078

Hi Kalle,

On Thu, 4 Oct 2018 15:27:57 +0300
Kalle Valo <kvalo@codeaurora.org> wrote:

> Ajay Singh <ajay.kathat@microchip.com> writes:
> 
> > Hi Kalle,
> >
> > On Thu, 23 Aug 2018 16:37:45 +0530
> > Kalle Valo <kvalo@codeaurora.org> wrote:
> >
> >> Ajay Singh <ajay.kathat@microchip.com> writes:
> >> 
> >> >> >>> We need help to review and identify if there are any pending
> >> >> >>> items for wilc1000 driver, so we can address those issues
> >> >> >>> and make it ready to move to the wireless subsystem.    
> >> >> >>
> >> >> >> I think the best way to get that forward is to submit a patch
> >> >> >> (or patchset) to linux-wireless, that's the easiest for
> >> >> >> reviewers.    
> >> >> >
> >> >> > For brcm80211 drivers we used a single patch introducing it
> >> >> > under the wireless drivers folder. Because it was quite a
> >> >> > sizable patch we parked it on the wireless wiki page. Had a
> >> >> > few iterations doing it like that.    
> >> >> 
> >> >> Another option is to split it so that there's one patch per
> >> >> file, should be even pretty easy to automate that. It's just so
> >> >> much easier to comment on a patch submitted by email compared
> >> >> to the reviewer manually copying code and then commenting it,
> >> >> yuck. 
> >> >
> >> > Sure. I will prepare a patch per file send for review as its
> >> > easy to review.
> >> >
> >> > As Greg suggested, I will wait for the merge window to close and
> >> > after completing pending patches to staging, I will start the
> >> > review.
> >> >
> >> > For my understanding, the patches for review will be based on
> >> > wireless-testing branch.  
> >> 
> >> In this case I think wireless-drivers-next is the safest choise,
> >> wireless-testing also has other trees which might cause conflicts
> >> etc.
> >
> > I have submitted a patch series for wilc1000 driver, single file
> > per patch and its based on wireless-drivers-next. 
> > I hope its done correctly, please provide inputs so we can
> > address and make this driver ready for mainline.
> 
> Thanks, I see it in patchwork:
> 
> https://patchwork.kernel.org/project/linux-wireless/list/?series=23251&state=*&order=date
> 
> Do note that we have also another new driver (rtwlan/rtw88) under
> review so review from the wireless folks might take some time.
> 

Thanks for the update.
Btw I could see our driver was added to the pending branch. Just
curious, could you please share details about how the pending branch is
used ?

Regards,
Ajay

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: feedback on mainlining wilc1000 staging driver
  2018-10-05  2:33                     ` Ajay Singh
@ 2018-10-05  5:16                       ` Kalle Valo
  0 siblings, 0 replies; 15+ messages in thread
From: Kalle Valo @ 2018-10-05  5:16 UTC (permalink / raw)
  To: Ajay Singh
  Cc: Arend van Spriel, devel, gregkh, linux-wireless,
	Nicolas Ferre - M43238, Ganesh Krishna - C00112,
	Adham Abozaeid - C43058, Aditya Shankar - I16078

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

>> > I have submitted a patch series for wilc1000 driver, single file
>> > per patch and its based on wireless-drivers-next. 
>> > I hope its done correctly, please provide inputs so we can
>> > address and make this driver ready for mainline.
>> 
>> Thanks, I see it in patchwork:
>> 
>> https://patchwork.kernel.org/project/linux-wireless/list/?series=23251&state=*&order=date
>> 
>> Do note that we have also another new driver (rtwlan/rtw88) under
>> review so review from the wireless folks might take some time.
>> 
>
> Thanks for the update.
> Btw I could see our driver was added to the pending branch. Just
> curious, could you please share details about how the pending branch is
> used ?

I use pending branch mainly for testing bigger patches with kbuild bot,
though I think the bot is offline this week. (Or was it next week? Can't
remember)

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2018-10-05  5:16 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-15 20:22 feedback on mainlining wilc1000 staging driver Ajay Singh
2018-08-16  6:43 ` Greg KH
2018-08-15 20:53   ` Ajay Singh
2018-08-16 10:47 ` Kalle Valo
2018-08-16 10:53   ` Kalle Valo
2018-08-17  4:32     ` Ajay Singh
2018-08-17  7:49       ` Kalle Valo
2018-08-17  8:21         ` Arend van Spriel
2018-08-17  8:36           ` Kalle Valo
2018-08-17  9:09             ` Ajay Singh
2018-08-23 11:07               ` Kalle Valo
2018-09-26 10:27                 ` Ajay Singh
2018-10-04 12:27                   ` Kalle Valo
2018-10-05  2:33                     ` Ajay Singh
2018-10-05  5:16                       ` Kalle Valo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.