Re: [PATCH 03/10] staging: rtl8188eu: Remove _ReadLEDSetting() function.

2014-04-22 Thread Dan Carpenter
On Sun, Apr 20, 2014 at 02:17:11PM +0530, navin patidar wrote:
 _ReadLEDSetting() doesn't read led settings this function actually
 initialize  member variables of struct led_priv, we should do that
 inside rtl8188eu_InitSwLeds().
 

This seems like a bugfix or is it just a cleanup?  It's not clear to me
from the patch description.  Have you tested this patch?

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 03/10] staging: rtl8188eu: Remove _ReadLEDSetting() function.

2014-04-22 Thread navin patidar
Hi dan

it's just a cleanup patch and i also tested all patches of this patch
series on the hardware (TP-LINK  TL-WN723N).
my mistake i should have added Tested-by tag for all patches.

Tested-by: navin patidar navin.pati...@gmail.com

regards,
--navin-patidar

On Tue, Apr 22, 2014 at 3:41 PM, Dan Carpenter dan.carpen...@oracle.com wrote:
 On Sun, Apr 20, 2014 at 02:17:11PM +0530, navin patidar wrote:
 _ReadLEDSetting() doesn't read led settings this function actually
 initialize  member variables of struct led_priv, we should do that
 inside rtl8188eu_InitSwLeds().


 This seems like a bugfix or is it just a cleanup?  It's not clear to me
 from the patch description.  Have you tested this patch?

 regards,
 dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 03/10] staging: rtl8188eu: Remove _ReadLEDSetting() function.

2014-04-22 Thread Dan Carpenter
On Tue, Apr 22, 2014 at 07:31:50PM +0530, navin patidar wrote:
 Hi dan
 
 it's just a cleanup patch and i also tested all patches of this patch
 series on the hardware (TP-LINK  TL-WN723N).
 my mistake i should have added Tested-by tag for all patches.
 
 Tested-by: navin patidar navin.pati...@gmail.com

No no.  It's not necesary to add a tested by to everything.  It was just
something I was curious about.  The patches looked ok.  If you hadn't
tested them I would review them a second time more carefully.

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 03/10] staging: rtl8188eu: Remove _ReadLEDSetting() function.

2014-04-22 Thread Larry Finger

On 04/22/2014 09:14 AM, Dan Carpenter wrote:

On Tue, Apr 22, 2014 at 07:31:50PM +0530, navin patidar wrote:

Hi dan

it's just a cleanup patch and i also tested all patches of this patch
series on the hardware (TP-LINK  TL-WN723N).
my mistake i should have added Tested-by tag for all patches.

Tested-by: navin patidar navin.pati...@gmail.com


No no.  It's not necesary to add a tested by to everything.  It was just
something I was curious about.  The patches looked ok.  If you hadn't
tested them I would review them a second time more carefully.


To me, the opposite case is true. If you have not tested, then you need to 
explicitly state compile tested only in the commit message. Without saying 
that, your signed-off-by is implying that you have tested, as well as declaring 
that you own the copyright to the material.


Larry

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 03/10] staging: rtl8188eu: Remove _ReadLEDSetting() function.

2014-04-22 Thread Dan Carpenter
I sometimes mark my patches as untested and there are a couple other
people who do as well.  For the staging tree probably 80% of the patches
are not tested.  I kind of know who regular developers are and who tests
their patches.

But one thing I really want to stress is that if I ask something about a
patch, it doesn't mean I'm annoyed.  It just means I was curious.

regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 03/10] staging: rtl8188eu: Remove _ReadLEDSetting() function.

2014-04-22 Thread navin patidar
On Tue, Apr 22, 2014 at 7:58 PM, Larry Finger larry.fin...@lwfinger.net wrote:

 To me, the opposite case is true. If you have not tested, then you need to
 explicitly state compile tested only in the commit message. Without saying
 that, your signed-off-by is implying that you have tested, as well as
 declaring that you own the copyright to the material.

I didn't know that  signed-off-by implies both tested and authored.
thanks for clearing that up.

regards,
--navin-patidar
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 03/10] staging: rtl8188eu: Remove _ReadLEDSetting() function.

2014-04-20 Thread navin patidar
_ReadLEDSetting() doesn't read led settings this function actually
initialize  member variables of struct led_priv, we should do that
inside rtl8188eu_InitSwLeds().

Signed-off-by: navin patidar navin.pati...@gmail.com
---
 drivers/staging/rtl8188eu/hal/rtl8188eu_led.c |4 
 drivers/staging/rtl8188eu/hal/usb_halinit.c   |   12 
 2 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/rtl8188eu/hal/rtl8188eu_led.c 
b/drivers/staging/rtl8188eu/hal/rtl8188eu_led.c
index 08dfd94..01eeb70 100644
--- a/drivers/staging/rtl8188eu/hal/rtl8188eu_led.c
+++ b/drivers/staging/rtl8188eu/hal/rtl8188eu_led.c
@@ -92,8 +92,12 @@ exit:
 void rtl8188eu_InitSwLeds(struct adapter *padapter)
 {
struct led_priv *pledpriv = (padapter-ledpriv);
+   struct hal_data_8188e   *haldata = GET_HAL_DATA(padapter);
 
+   pledpriv-bRegUseLed = true;
+   pledpriv-LedStrategy = SW_LED_MODE1;
pledpriv-LedControlHandler = LedControl8188eu;
+   haldata-bLedOpenDrain = true;
 
InitLed871x(padapter, (pledpriv-SwLed0), LED_PIN_LED0);
 
diff --git a/drivers/staging/rtl8188eu/hal/usb_halinit.c 
b/drivers/staging/rtl8188eu/hal/usb_halinit.c
index c92067f..0dfd62b 100644
--- a/drivers/staging/rtl8188eu/hal/usb_halinit.c
+++ b/drivers/staging/rtl8188eu/hal/usb_halinit.c
@@ -1129,16 +1129,6 @@ static unsigned int rtl8188eu_inirp_deinit(struct 
adapter *Adapter)
 /* EEPROM/EFUSE Content Parsing */
 /*  */
 /*  */
-static void _ReadLEDSetting(struct adapter *Adapter, u8 *PROMContent, bool 
AutoloadFail)
-{
-   struct led_priv *pledpriv = (Adapter-ledpriv);
-   struct hal_data_8188e   *haldata = GET_HAL_DATA(Adapter);
-
-   pledpriv-bRegUseLed = true;
-   pledpriv-LedStrategy = SW_LED_MODE1;
-   haldata-bLedOpenDrain = true;/*  Support Open-drain arrangement for 
controlling the LED. */
-}
-
 static void Hal_EfuseParsePIDVID_8188EU(struct adapter *adapt, u8 *hwinfo, 
bool AutoLoadFail)
 {
struct hal_data_8188e   *haldata = GET_HAL_DATA(adapt);
@@ -1215,8 +1205,6 @@ readAdapterInfo_8188EU(
/*  */
Hal_InitChannelPlan(adapt);
Hal_CustomizeByCustomerID_8188EU(adapt);
-
-   _ReadLEDSetting(adapt, eeprom-efuse_eeprom_data, 
eeprom-bautoload_fail_flag);
 }
 
 static void _ReadPROMContent(
-- 
1.7.10.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel