subject:"Re\: \[PATCH\] rsi\: code clean\-up"

Re: [PATCH] rsi: code clean-up

2016-09-08 Thread Kalle Valo

Arend Van Spriel  writes:

> On 8-9-2016 7:36, Prameela Rani Garnepudi wrote:
>> From: Prameela Rani Garnepudi 
>
> Seems like you missed the essential training on main-line driver
> development :-p
>
> Typically you should split this up in conceptually separate patches so
> it is easier for people to review. A large patch changing things all
> over the place solving different things is taking a lot more time to review.
>
> So go work on branch, make a commit for each issue you want to address,
> and when ready do (using 6 commits as example):
>
> $ git format-patch -s -M --cover-letter -6
>
> The '--cover-letter' results in -...patch file in which you can
> summarize the changes in bullet list so Kalle can reuse that moving the
> patches upstream to networking subsystem.
>
> When done use git to send the patches:
>
> $ git send-email --to='Kalle Valo '
> --cc=linux-wireless@vger.kernel.org *.patch

And write a commit log for each patch. In minimum the commit log should
answer to the question "Why?".

A good way to learn how the this all works is to see how others do it.
For example, you can see all ath9k commits like this:

gitk drivers/net/wireless/ath/ath9k/

or iwlwifi:

gitk drivers/net/wireless/intel/iwlwifi/

-- 
Kalle Valo

Re: [PATCH] rsi: code clean-up

2016-09-08 Thread Arend Van Spriel

On 8-9-2016 7:36, Prameela Rani Garnepudi wrote:
> From: Prameela Rani Garnepudi 

Seems like you missed the essential training on main-line driver
development :-p

Typically you should split this up in conceptually separate patches so
it is easier for people to review. A large patch changing things all
over the place solving different things is taking a lot more time to review.

So go work on branch, make a commit for each issue you want to address,
and when ready do (using 6 commits as example):

$ git format-patch -s -M --cover-letter -6

The '--cover-letter' results in -...patch file in which you can
summarize the changes in bullet list so Kalle can reuse that moving the
patches upstream to networking subsystem.

When done use git to send the patches:

$ git send-email --to='Kalle Valo '
--cc=linux-wireless@vger.kernel.org *.patch

Regards,
> Signed-off-by: Prameela Rani Garnepudi 
> ---
>  drivers/net/wireless/rsi/rsi_91x_core.c |  18 ++--
>  drivers/net/wireless/rsi/rsi_91x_debugfs.c  |  13 +--
>  drivers/net/wireless/rsi/rsi_91x_mac80211.c |  81 +--
>  drivers/net/wireless/rsi/rsi_91x_main.c |  16 ++-
>  drivers/net/wireless/rsi/rsi_91x_mgmt.c | 155 
> +++-
>  drivers/net/wireless/rsi/rsi_91x_sdio.c |  14 +--
>  drivers/net/wireless/rsi/rsi_91x_sdio_ops.c |  11 +-
>  drivers/net/wireless/rsi/rsi_91x_usb.c  |  34 --
>  drivers/net/wireless/rsi/rsi_common.h   |   2 +-
>  drivers/net/wireless/rsi/rsi_debugfs.h  |   3 +-
>  drivers/net/wireless/rsi/rsi_main.h |  36 ---
>  drivers/net/wireless/rsi/rsi_mgmt.h |  54 +-
>  drivers/net/wireless/rsi/rsi_sdio.h |  18 ++--
>  13 files changed, 228 insertions(+), 227 deletions(-)
> 
> diff --git a/drivers/net/wireless/rsi/rsi_91x_core.c 
> b/drivers/net/wireless/rsi/rsi_91x_core.c
> index f3d3995..c6db8a9 100644
> --- a/drivers/net/wireless/rsi/rsi_91x_core.c
> +++ b/drivers/net/wireless/rsi/rsi_91x_core.c
> @@ -134,7 +134,7 @@ static u8 rsi_core_determine_hal_queue(struct rsi_common 
> *common)
>   bool recontend_queue = false;
>   u32 q_len = 0;
>   u8 q_num = INVALID_QUEUE;
> - u8 ii = 0;
> + u8 ii;
>  
>   if (skb_queue_len(>tx_queue[MGMT_SOFT_Q])) {
>   if (!common->mgmt_q_block)
> @@ -142,8 +142,10 @@ static u8 rsi_core_determine_hal_queue(struct rsi_common 
> *common)
>   return q_num;
>   }
>  
> - if (common->hw_data_qs_blocked)
> + if (common->hw_data_qs_blocked) {
> + rsi_dbg(INFO_ZONE, "%s: data queue blocked\n", __func__);
>   return q_num;
> + }
>  
>   if (common->pkt_cnt != 0) {
>   --common->pkt_cnt;
> @@ -210,6 +212,7 @@ static void rsi_core_queue_pkt(struct rsi_common *common,
>  struct sk_buff *skb)
>  {
>   u8 q_num = skb->priority;
> +
>   if (q_num >= NUM_SOFT_QUEUES) {
>   rsi_dbg(ERR_ZONE, "%s: Invalid Queue Number: q_num = %d\n",
>   __func__, q_num);
> @@ -285,7 +288,7 @@ void rsi_core_qos_processor(struct rsi_common *common)
>   }
>  
>   skb = rsi_core_dequeue_pkt(common, q_num);
> - if (skb == NULL) {
> + if (!skb) {
>   rsi_dbg(ERR_ZONE, "skb null\n");
>   mutex_unlock(>tx_rxlock);
>   break;
> @@ -331,15 +334,16 @@ void rsi_core_xmit(struct rsi_common *common, struct 
> sk_buff *skb)
>   __func__);
>   goto xmit_fail;
>   }
> - info = IEEE80211_SKB_CB(skb);
> - tx_params = (struct skb_info *)info->driver_data;
> - tmp_hdr = (struct ieee80211_hdr *)>data[0];
>  
>   if (common->fsm_state != FSM_MAC_INIT_DONE) {
>   rsi_dbg(ERR_ZONE, "%s: FSM state not open\n", __func__);
>   goto xmit_fail;
>   }
>  
> + info = IEEE80211_SKB_CB(skb);
> + tx_params = (struct skb_info *)info->driver_data;
> + tmp_hdr = (struct ieee80211_hdr *)>data[0];
> +
>   if ((ieee80211_is_mgmt(tmp_hdr->frame_control)) ||
>   (ieee80211_is_ctl(tmp_hdr->frame_control)) ||
>   (ieee80211_is_qos_nullfunc(tmp_hdr->frame_control))) {
> @@ -360,7 +364,7 @@ void rsi_core_xmit(struct rsi_common *common, struct 
> sk_buff *skb)
>  
>   if ((q_num != MGMT_SOFT_Q) &&
>   ((skb_queue_len(>tx_queue[q_num]) + 1) >=
> -  DATA_QUEUE_WATER_MARK)) {
> +  DATA_QUEUE_WATER_MARK)) {
>   rsi_dbg(ERR_ZONE, "%s: sw queue full\n", __func__);
>   if (!ieee80211_queue_stopped(adapter->hw, WME_AC(q_num)))
>   ieee80211_stop_queue(adapter->hw, WME_AC(q_num));
> diff --git a/drivers/net/wireless/rsi/rsi_91x_debugfs.c 
> b/drivers/net/wireless/rsi/rsi_91x_debugfs.c
> index 828a042..5ec7bce 100644
> ---

Re: [PATCH] rsi: code clean-up

2016-09-07 Thread Kalle Valo

Julian Calaby  writes:

> Hi,
>
> On Wed, Sep 7, 2016 at 8:26 PM, Prameela Rani Garnepudi
>  wrote:
>>  From 0c3da95dfac07df5dffb5e00bcd8a38f2f8491c9 Mon Sep 17 00:00:00 2001
>> From: Prameela Rani Garnepudi 
>> Date: Wed, 7 Sep 2016 15:28:43 +0530
>> Subject: [PATCH] rsi: code clean-up
>
> Describe what the changes are achieving.
>
> You're also still sending this as a HTML email, there are instructions
> on how to configure your email client to send plain text emails here:
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/email-clients.txt
>
> Because you're sending multi-part HTML emails, your emails are being
> rejected by the mailing list.

I recommend to try to use git-send-email to submit patches, it makes
life so much more easier:

https://www.kernel.org/pub/software/scm/git/docs/git-send-email.html

-- 
Kalle Valo

Re: [PATCH] rsi: code clean-up

2016-09-07 Thread Julian Calaby

Hi,

On Wed, Sep 7, 2016 at 8:26 PM, Prameela Rani Garnepudi
 wrote:
>  From 0c3da95dfac07df5dffb5e00bcd8a38f2f8491c9 Mon Sep 17 00:00:00 2001
> From: Prameela Rani Garnepudi 
> Date: Wed, 7 Sep 2016 15:28:43 +0530
> Subject: [PATCH] rsi: code clean-up

Describe what the changes are achieving.

You're also still sending this as a HTML email, there are instructions
on how to configure your email client to send plain text emails here:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/email-clients.txt

Because you're sending multi-part HTML emails, your emails are being
rejected by the mailing list.

> Signed-off-by: Prameela Rani Garnepudi
> 
> ---
>   drivers/net/wireless/rsi/rsi_91x_core.c |  18 ++--
>   drivers/net/wireless/rsi/rsi_91x_debugfs.c  |  13 +--
>   drivers/net/wireless/rsi/rsi_91x_mac80211.c |  81 +--
>   drivers/net/wireless/rsi/rsi_91x_main.c |  16 ++-
>   drivers/net/wireless/rsi/rsi_91x_mgmt.c | 155
> +++-
>   drivers/net/wireless/rsi/rsi_91x_sdio.c |  14 +--
>   drivers/net/wireless/rsi/rsi_91x_sdio_ops.c |  11 +-
>   drivers/net/wireless/rsi/rsi_91x_usb.c  |  34 --
>   drivers/net/wireless/rsi/rsi_common.h   |   2 +-
>   drivers/net/wireless/rsi/rsi_debugfs.h  |   3 +-
>   drivers/net/wireless/rsi/rsi_main.h |  36 ---
>   drivers/net/wireless/rsi/rsi_mgmt.h |  54 +-
>   drivers/net/wireless/rsi/rsi_sdio.h |  18 ++--
>   13 files changed, 228 insertions(+), 227 deletions(-)

You're doing a lot of stuff here, you really need to split this up
into several patches, e.g.
1. Fix the spelling mistakes
2. Add the new debug messages
etc. etc. etc.

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

Re: [PATCH] rsi: code clean-up

Re: [PATCH] rsi: code clean-up

Re: [PATCH] rsi: code clean-up

Re: [PATCH] rsi: code clean-up

4 matches

Site Navigation

Mail list logo

Footer information