Re: [PATCH 03/10] staging: rtl8188eu: Remove _ReadLEDSetting() function.
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.
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.
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.
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.
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.
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.
_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