Re: [PATCH] staging: rtl8712: assign initial value to a static variable in r8712_efuse_pg_packet_write()
On 2019/2/28 16:04, Greg KH wrote:
> On Wed, Feb 27, 2019 at 11:33:22AM +0800, Mao Wenan wrote:
>> repeat_times is a static variable, but each time when it enters
>> r8712_efuse_pg_packet_write(), it is set to zero,
>> this value is not consistent with last calling, so next behavior
>> is not our expect.
>>
>> Signed-off-by: Mao Wenan
>> ---
>> drivers/staging/rtl8712/rtl8712_efuse.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/staging/rtl8712/rtl8712_efuse.c
>> b/drivers/staging/rtl8712/rtl8712_efuse.c
>> index 8bc45ffd3029..3e8421255170 100644
>> --- a/drivers/staging/rtl8712/rtl8712_efuse.c
>> +++ b/drivers/staging/rtl8712/rtl8712_efuse.c
>> @@ -358,7 +358,7 @@ u8 r8712_efuse_pg_packet_write(struct _adapter
>> *padapter, const u8 offset,
>> u8 pg_header = 0;
>> u16 efuse_addr = 0, curr_size = 0;
>> u8 efuse_data, target_word_cnts = 0;
>> -static int repeat_times;
>> +static int repeat_times = 0;
>> int sub_repeat;
>> u8 bResult = true;
>>
>> @@ -368,7 +368,6 @@ u8 r8712_efuse_pg_packet_write(struct _adapter
>> *padapter, const u8 offset,
>> return false;
>> pg_header = MAKE_EFUSE_HEADER(offset, word_en);
>> target_word_cnts = calculate_word_cnts(word_en);
>> -repeat_times = 0;
>> efuse_addr = 0;
>> while (efuse_addr < efuse_available_max_size) {
>> curr_size = r8712_efuse_get_current_size(padapter);
>
> This patch does not apply to my tree at all.
>
> And are you sure this is correct?
sorry, my base tree is wrong, this fix is invalid.
>
> greg k-h
>
> .
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: davinci: drop pointless static qualifier in vpfe_resizer_init()
On 2019/3/11 22:07, Dan Carpenter wrote: > On Mon, Mar 11, 2019 at 10:14:05PM +0800, Mao Wenan wrote: >> There is no need to have the 'T *v' variable static >> since new value always be assigned before use it. >> >> Signed-off-by: Mao Wenan >> --- >> drivers/staging/media/davinci_vpfe/dm365_resizer.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/staging/media/davinci_vpfe/dm365_resizer.c >> b/drivers/staging/media/davinci_vpfe/dm365_resizer.c >> index 6098f43ac51b..a2a672d4615d 100644 >> --- a/drivers/staging/media/davinci_vpfe/dm365_resizer.c >> +++ b/drivers/staging/media/davinci_vpfe/dm365_resizer.c >> @@ -1881,7 +1881,7 @@ int vpfe_resizer_init(struct vpfe_resizer_device >> *vpfe_rsz, >> struct v4l2_subdev *sd = &vpfe_rsz->crop_resizer.subdev; >> struct media_pad *pads = &vpfe_rsz->crop_resizer.pads[0]; >> struct media_entity *me = &sd->entity; >> -static resource_size_t res_len; >> +resource_size_t res_len; > ^ > Could you remove the extra space character also, please. sure, I will do this, sorry, I don't pay attention to the old code style. > >> struct resource *res; >> int ret; > > regards, > dan carpenter > > > > . > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH net] staging: rtl8188eu: use is_zero_ether_addr() instead of memcmp()
ping...
On 2019/3/9 11:26, Mao Wenan wrote:
> Using is_zero_ether_addr() instead of directly use
> memcmp() to determine if the ethernet address is all
> zeros.
>
> Signed-off-by: Mao Wenan
> ---
> drivers/staging/rtl8188eu/core/rtw_mlme.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme.c
> b/drivers/staging/rtl8188eu/core/rtw_mlme.c
> index 714f7a70ed64..beae367df93b 100644
> --- a/drivers/staging/rtl8188eu/core/rtw_mlme.c
> +++ b/drivers/staging/rtl8188eu/core/rtw_mlme.c
> @@ -180,9 +180,8 @@ struct wlan_network *rtw_find_network(struct __queue
> *scanned_queue, u8 *addr)
> {
> struct list_head *phead, *plist;
> struct wlan_network *pnetwork = NULL;
> - u8 zero_addr[ETH_ALEN] = {0, 0, 0, 0, 0, 0};
>
> - if (!memcmp(zero_addr, addr, ETH_ALEN)) {
> + if (is_zero_ether_addr(addr)) {
> pnetwork = NULL;
> goto exit;
> }
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH net] staging: rtl8188eu: use is_zero_ether_addr() instead of memcmp()
On 2019/3/12 14:35, Joe Perches wrote:
> On Tue, 2019-03-12 at 14:29 +0800, maowenan wrote:
>> ping...
>>
>> On 2019/3/9 11:26, Mao Wenan wrote:
>>> Using is_zero_ether_addr() instead of directly use
>>> memcmp() to determine if the ethernet address is all
>>> zeros.
> []
>>> diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme.c
>>> b/drivers/staging/rtl8188eu/core/rtw_mlme.c
> []
>>> @@ -180,9 +180,8 @@ struct wlan_network *rtw_find_network(struct __queue
>>> *scanned_queue, u8 *addr)
>>> {
>>> struct list_head *phead, *plist;
>>> struct wlan_network *pnetwork = NULL;
>>> - u8 zero_addr[ETH_ALEN] = {0, 0, 0, 0, 0, 0};
>>>
>>> - if (!memcmp(zero_addr, addr, ETH_ALEN)) {
>>> + if (is_zero_ether_addr(addr)) {
>
> How did you verify that addr is __aligned(2)?
/**
* is_zero_ether_addr - Determine if give Ethernet address is all zeros.
* @addr: Pointer to a six-byte array containing the Ethernet address
*
* Return true if the address is all zeroes.
*/
I think they are completely equivalent functions, no need to check addr is
__aligned(2),
because addr may be defined as unsigned char MacAddress[ETH_ALEN]; the length
is 6.
>
>
>
> .
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: davinci: drop pointless static qualifier in vpfe_resizer_init()
Ping... Thank you. On 2019/3/11 22:35, Dan Carpenter wrote: > Thanks! > > Reviewed-by: Dan Carpenter > > regards, > dan carpenter > > > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH net] staging: rtl8188eu: use is_zero_ether_addr() instead of memcmp()
On 2019/3/17 19:26, Greg KH wrote:
> On Sat, Mar 09, 2019 at 11:26:00AM +0800, Mao Wenan wrote:
>> Using is_zero_ether_addr() instead of directly use
>> memcmp() to determine if the ethernet address is all
>> zeros.
>>
>> Signed-off-by: Mao Wenan
>> ---
>> drivers/staging/rtl8188eu/core/rtw_mlme.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme.c
>> b/drivers/staging/rtl8188eu/core/rtw_mlme.c
>> index 714f7a70ed64..beae367df93b 100644
>> --- a/drivers/staging/rtl8188eu/core/rtw_mlme.c
>> +++ b/drivers/staging/rtl8188eu/core/rtw_mlme.c
>> @@ -180,9 +180,8 @@ struct wlan_network *rtw_find_network(struct __queue
>> *scanned_queue, u8 *addr)
>> {
>> struct list_head *phead, *plist;
>> struct wlan_network *pnetwork = NULL;
>> -u8 zero_addr[ETH_ALEN] = {0, 0, 0, 0, 0, 0};
>>
>> -if (!memcmp(zero_addr, addr, ETH_ALEN)) {
>> +if (is_zero_ether_addr(addr)) {
>
> As Joe said, you have to prove that this is properly aligned before you
> can call this function. Please do so.
The previous function to call rtw_find_network(), has been aligned for
parameter 'addr',
because it has been defined an arry[6];
such as unsigned char MacAddress[ETH_ALEN] in struct wlan_bssid_ex;
179 struct wlan_bssid_ex {
180 u32 Length;
181 unsigned char MacAddress[ETH_ALEN];
182 u8 Reserved[2];/* 0]: IS beacon frame */
183 struct ndis_802_11_ssid ssid;
184 u32 Privacy;
185 NDIS_802_11_RSSI Rssi;/* in dBM,raw data ,get from PHY) */
186 enum NDIS_802_11_NETWORK_TYPE NetworkTypeInUse;
187 struct ndis_802_11_config Configuration;
188 enum ndis_802_11_network_infra InfrastructureMode;
189 unsigned char SupportedRates[NDIS_802_11_LENGTH_RATES_EX];
190 struct wlan_phy_infoPhyInfo;
191 u32 ie_length;
192 u8 ies[MAX_IE_SZ]; /* timestamp, beacon interval, and
193 * capability information)
194 */
195 } __packed;
rtw_survey_event_callback->rtw_find_network(&pmlmepriv->scanned_queue,
pnetwork->MacAddress);
struct wlan_bssid_ex *pnetwork;
>
> thanks,
>
> greg k-h
>
> .
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH net] staging: Remove set but not used variable ‘status’
On 2019/5/25 13:01, Greg KH wrote: > On Sat, May 25, 2019 at 12:26:42PM +0800, Mao Wenan wrote: >> Fixes gcc '-Wunused-but-set-variable' warning: >> >> drivers/staging/kpc2000/kpc_spi/spi_driver.c: In function >> ‘kp_spi_transfer_one_message’: >> drivers/staging/kpc2000/kpc_spi/spi_driver.c:282:9: warning: variable >> ‘status’ set but not used [-Wunused-but-set-variable] >> int status = 0; >> ^~ >> The variable 'status' is not used any more, remve it. >> >> Signed-off-by: Mao Wenan >> --- >> drivers/staging/kpc2000/kpc_spi/spi_driver.c | 3 --- >> 1 file changed, 3 deletions(-) > > What is [PATCH net] in the subject for? This is not a networking driver > :( Sorry, this is my mistake. > > > . > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH net] staging: Remove set but not used variable ‘status’
On 2019/5/25 20:59, Sven Van Asbroeck wrote:
> On Sat, May 25, 2019 at 12:20 AM Mao Wenan wrote:
>>
>> The variable 'status' is not used any more, remve it.
>
>> /* do the transfers for this message */
>> list_for_each_entry(transfer, &m->transfers, transfer_list) {
>> if (transfer->tx_buf == NULL && transfer->rx_buf == NULL &&
>> transfer->len) {
>> -status = -EINVAL;
>> break;
>> }
>
> This looks like an error condition that's not reported to the spi core.
>
> Instead of removing the status variable (which also removes the error value!),
> maybe this should be reported to the spi core instead ?
>
> Other spi drivers appear to do the following on the error path:
> m->status = status;
> return status;
I have reviewed the code again, and it is good idea to store m->status in error
path, like below?
@@ -374,7 +374,7 @@ kp_spi_transfer_one_message(struct spi_master *master,
struct spi_message *m)
list_for_each_entry(transfer, &m->transfers, transfer_list) {
if (transfer->tx_buf == NULL && transfer->rx_buf == NULL &&
transfer->len) {
status = -EINVAL;
-break;
+goto error;
}
/* transfer */
@@ -412,7 +412,7 @@ kp_spi_transfer_one_message(struct spi_master *master,
struct spi_message *m)
if (count != transfer->len) {
status = -EIO;
-break;
+goto error;
}
}
...
/* done work */
spi_finalize_current_message(master);
return 0;
+
+ error:
+m->status = status;
+return status;
}
>
>>
>> @@ -370,7 +368,6 @@ kp_spi_transfer_one_message(struct spi_master *master,
>> struct spi_message *m)
>>
>> if (count != transfer->len) {
>> -status = -EIO;
>> break;
>
> Same issue here.
>
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH -next v2] staging: kpc2000: Remove set but not used variable ‘status’
please ignore v2 version.
I will send v3 later according to Sven Van Asbroeck 's comments.
On 2019/5/25 16:13, Mao Wenan wrote:
> Fixes gcc '-Wunused-but-set-variable' warning:
>
> drivers/staging/kpc2000/kpc_spi/spi_driver.c: In function
> ‘kp_spi_transfer_one_message’:
> drivers/staging/kpc2000/kpc_spi/spi_driver.c:282:9: warning: variable
> ‘status’ set but not used [-Wunused-but-set-variable]
> int status = 0;
> ^~
> The variable 'status' is not used any more, remve it.
>
> Signed-off-by: Mao Wenan
> ---
> v2: change the subject of the patch.
> ---
> drivers/staging/kpc2000/kpc_spi/spi_driver.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/drivers/staging/kpc2000/kpc_spi/spi_driver.c
> b/drivers/staging/kpc2000/kpc_spi/spi_driver.c
> index 86df16547a92..16f9518f8d63 100644
> --- a/drivers/staging/kpc2000/kpc_spi/spi_driver.c
> +++ b/drivers/staging/kpc2000/kpc_spi/spi_driver.c
> @@ -279,7 +279,6 @@ kp_spi_transfer_one_message(struct spi_master *master,
> struct spi_message *m)
> struct kp_spi *kpspi;
> struct spi_transfer *transfer;
> union kp_spi_config sc;
> -int status = 0;
>
> spidev = m->spi;
> kpspi = spi_master_get_devdata(master);
> @@ -332,7 +331,6 @@ kp_spi_transfer_one_message(struct spi_master *master,
> struct spi_message *m)
> /* do the transfers for this message */
> list_for_each_entry(transfer, &m->transfers, transfer_list) {
> if (transfer->tx_buf == NULL && transfer->rx_buf == NULL &&
> transfer->len) {
> -status = -EINVAL;
> break;
> }
>
> @@ -370,7 +368,6 @@ kp_spi_transfer_one_message(struct spi_master *master,
> struct spi_message *m)
> m->actual_length += count;
>
> if (count != transfer->len) {
> -status = -EIO;
> break;
> }
> }
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
