* [PATCH] Staging: rtl8192u: Reposition of return statements
@ 2015-02-19 22:38 Ksenija Stanojevic
2015-02-20 11:37 ` [Outreachy kernel] " Jes Sorensen
0 siblings, 1 reply; 4+ messages in thread
From: Ksenija Stanojevic @ 2015-02-19 22:38 UTC (permalink / raw)
To: outreachy-kernel; +Cc: Ksenija Stanojevic
This patch fixes the checkpatch.pl warning:
WARNING: "else is not generally useful after a break or return"
by placing return reg after if-else branch.
Signed-off-by: Ksenija Stanojevic <ksenija.stanojevic@gmail.com>
---
drivers/staging/rtl8192u/r819xU_phy.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/staging/rtl8192u/r819xU_phy.c b/drivers/staging/rtl8192u/r819xU_phy.c
index d006602..f343735 100644
--- a/drivers/staging/rtl8192u/r819xU_phy.c
+++ b/drivers/staging/rtl8192u/r819xU_phy.c
@@ -355,13 +355,12 @@ u32 rtl8192_phy_QueryRFReg(struct net_device *dev, RF90_RADIO_PATH_E eRFPath,
bitshift = rtl8192_CalculateBitShift(bitmask);
reg = (reg & bitmask) >> bitshift;
udelay(200);
- return reg;
} else {
reg = rtl8192_phy_RFSerialRead(dev, eRFPath, reg_addr);
bitshift = rtl8192_CalculateBitShift(bitmask);
reg = (reg & bitmask) >> bitshift;
- return reg;
}
+ return reg;
}
/******************************************************************************
--
1.9.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Outreachy kernel] [PATCH] Staging: rtl8192u: Reposition of return statements
2015-02-19 22:38 [PATCH] Staging: rtl8192u: Reposition of return statements Ksenija Stanojevic
@ 2015-02-20 11:37 ` Jes Sorensen
2015-02-20 11:48 ` Ksenija Stanojević
0 siblings, 1 reply; 4+ messages in thread
From: Jes Sorensen @ 2015-02-20 11:37 UTC (permalink / raw)
To: Ksenija Stanojevic, outreachy-kernel
On 02/19/15 17:38, Ksenija Stanojevic wrote:
> This patch fixes the checkpatch.pl warning:
> WARNING: "else is not generally useful after a break or return"
> by placing return reg after if-else branch.
>
> Signed-off-by: Ksenija Stanojevic <ksenija.stanojevic@gmail.com>
> ---
> drivers/staging/rtl8192u/r819xU_phy.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/staging/rtl8192u/r819xU_phy.c b/drivers/staging/rtl8192u/r819xU_phy.c
> index d006602..f343735 100644
> --- a/drivers/staging/rtl8192u/r819xU_phy.c
> +++ b/drivers/staging/rtl8192u/r819xU_phy.c
> @@ -355,13 +355,12 @@ u32 rtl8192_phy_QueryRFReg(struct net_device *dev, RF90_RADIO_PATH_E eRFPath,
> bitshift = rtl8192_CalculateBitShift(bitmask);
> reg = (reg & bitmask) >> bitshift;
> udelay(200);
> - return reg;
> } else {
> reg = rtl8192_phy_RFSerialRead(dev, eRFPath, reg_addr);
> bitshift = rtl8192_CalculateBitShift(bitmask);
> reg = (reg & bitmask) >> bitshift;
> - return reg;
> }
> + return reg;
> }
>
> /******************************************************************************
>
In this case the automated commit message is actually a little unclear,
at least to me. Explaining that you are simplifying the code by not
having two identical paths would be better.
You could take this patch further as most of the code is identical
except for the udelay() in the first case.
Cheers,
Jes
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Outreachy kernel] [PATCH] Staging: rtl8192u: Reposition of return statements
2015-02-20 11:37 ` [Outreachy kernel] " Jes Sorensen
@ 2015-02-20 11:48 ` Ksenija Stanojević
2015-02-20 11:50 ` Jes Sorensen
0 siblings, 1 reply; 4+ messages in thread
From: Ksenija Stanojević @ 2015-02-20 11:48 UTC (permalink / raw)
To: Jes Sorensen; +Cc: outreachy-kernel
[-- Attachment #1: Type: text/plain, Size: 1865 bytes --]
Yes, I agree.
Also these two functions are different:
reg = phy_FwRFSerialRead(dev, eRFPath, reg_addr);
reg = rtl8192_phy_RFSerialRead(dev, eRFPath, reg_addr);
On Fri, Feb 20, 2015 at 12:37 PM, Jes Sorensen <jes.sorensen@gmail.com>
wrote:
> On 02/19/15 17:38, Ksenija Stanojevic wrote:
> > This patch fixes the checkpatch.pl warning:
> > WARNING: "else is not generally useful after a break or return"
> > by placing return reg after if-else branch.
> >
> > Signed-off-by: Ksenija Stanojevic <ksenija.stanojevic@gmail.com>
> > ---
> > drivers/staging/rtl8192u/r819xU_phy.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/staging/rtl8192u/r819xU_phy.c
> b/drivers/staging/rtl8192u/r819xU_phy.c
> > index d006602..f343735 100644
> > --- a/drivers/staging/rtl8192u/r819xU_phy.c
> > +++ b/drivers/staging/rtl8192u/r819xU_phy.c
> > @@ -355,13 +355,12 @@ u32 rtl8192_phy_QueryRFReg(struct net_device *dev,
> RF90_RADIO_PATH_E eRFPath,
> > bitshift = rtl8192_CalculateBitShift(bitmask);
> > reg = (reg & bitmask) >> bitshift;
> > udelay(200);
> > - return reg;
> > } else {
> > reg = rtl8192_phy_RFSerialRead(dev, eRFPath, reg_addr);
> > bitshift = rtl8192_CalculateBitShift(bitmask);
> > reg = (reg & bitmask) >> bitshift;
> > - return reg;
> > }
> > + return reg;
> > }
> >
> >
> /******************************************************************************
> >
>
> In this case the automated commit message is actually a little unclear,
> at least to me. Explaining that you are simplifying the code by not
> having two identical paths would be better.
>
> You could take this patch further as most of the code is identical
> except for the udelay() in the first case.
>
> Cheers,
> Jes
>
>
[-- Attachment #2: Type: text/html, Size: 3761 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Outreachy kernel] [PATCH] Staging: rtl8192u: Reposition of return statements
2015-02-20 11:48 ` Ksenija Stanojević
@ 2015-02-20 11:50 ` Jes Sorensen
0 siblings, 0 replies; 4+ messages in thread
From: Jes Sorensen @ 2015-02-20 11:50 UTC (permalink / raw)
To: Ksenija Stanojević; +Cc: outreachy-kernel
On 02/20/15 06:48, Ksenija Stanojević wrote:
> Yes, I agree.
>
> Also these two functions are different:
>
> reg = phy_FwRFSerialRead(dev, eRFPath, reg_addr);
>
> reg = rtl8192_phy_RFSerialRead(dev, eRFPath, reg_addr);
That is correct, but the two lines:
bitshift = ....
ret = (reg & bitmask) >> bitshift;
are identical.
Cheers,
Jes
>
>
> On Fri, Feb 20, 2015 at 12:37 PM, Jes Sorensen <jes.sorensen@gmail.com>
> wrote:
>
>> On 02/19/15 17:38, Ksenija Stanojevic wrote:
>>> This patch fixes the checkpatch.pl warning:
>>> WARNING: "else is not generally useful after a break or return"
>>> by placing return reg after if-else branch.
>>>
>>> Signed-off-by: Ksenija Stanojevic <ksenija.stanojevic@gmail.com>
>>> ---
>>> drivers/staging/rtl8192u/r819xU_phy.c | 3 +--
>>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/staging/rtl8192u/r819xU_phy.c
>> b/drivers/staging/rtl8192u/r819xU_phy.c
>>> index d006602..f343735 100644
>>> --- a/drivers/staging/rtl8192u/r819xU_phy.c
>>> +++ b/drivers/staging/rtl8192u/r819xU_phy.c
>>> @@ -355,13 +355,12 @@ u32 rtl8192_phy_QueryRFReg(struct net_device *dev,
>> RF90_RADIO_PATH_E eRFPath,
>>> bitshift = rtl8192_CalculateBitShift(bitmask);
>>> reg = (reg & bitmask) >> bitshift;
>>> udelay(200);
>>> - return reg;
>>> } else {
>>> reg = rtl8192_phy_RFSerialRead(dev, eRFPath, reg_addr);
>>> bitshift = rtl8192_CalculateBitShift(bitmask);
>>> reg = (reg & bitmask) >> bitshift;
>>> - return reg;
>>> }
>>> + return reg;
>>> }
>>>
>>>
>> /******************************************************************************
>>>
>>
>> In this case the automated commit message is actually a little unclear,
>> at least to me. Explaining that you are simplifying the code by not
>> having two identical paths would be better.
>>
>> You could take this patch further as most of the code is identical
>> except for the udelay() in the first case.
>>
>> Cheers,
>> Jes
>>
>>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-02-20 11:50 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-02-19 22:38 [PATCH] Staging: rtl8192u: Reposition of return statements Ksenija Stanojevic
2015-02-20 11:37 ` [Outreachy kernel] " Jes Sorensen
2015-02-20 11:48 ` Ksenija Stanojević
2015-02-20 11:50 ` Jes Sorensen
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.