[PATCH] tools: hv: ignore ENOBUFS in the KVP daemon

2014-11-19 Thread Dexuan Cui
Under high memory pressure and very high KVP R/W test pressure, the netlink
recvfrom() may transiently return ENOBUFS to the daemon -- we found this
during a 2-week stress test.

We'd better not terminate the daemon on this failure, because a typical KVP
user can re-try the R/W and hopefully it will succeed next time.

Cc: K. Y. Srinivasan 
Signed-off-by: Dexuan Cui 
---
 tools/hv/hv_kvp_daemon.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
index 22b0764..9f4b303 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -1559,8 +1559,15 @@ int main(int argc, char *argv[])
addr_p, &addr_l);
 
if (len < 0) {
+   int saved_errno = errno;
syslog(LOG_ERR, "recvfrom failed; pid:%u error:%d %s",
addr.nl_pid, errno, strerror(errno));
+
+   if (saved_errno == ENOBUFS) {
+   syslog(LOG_ERR, "error = ENOBUFS: ignored");
+   continue;
+   }
+
close(fd);
return -1;
}
-- 
1.9.1

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


Re: [PATCH] tools: hv: ignore ENOBUFS in the KVP daemon

2014-11-19 Thread Vitaly Kuznetsov
Dexuan Cui  writes:

> Under high memory pressure and very high KVP R/W test pressure, the netlink
> recvfrom() may transiently return ENOBUFS to the daemon -- we found this
> during a 2-week stress test.
>
> We'd better not terminate the daemon on this failure, because a typical KVP
> user can re-try the R/W and hopefully it will succeed next time.
>
> Cc: K. Y. Srinivasan 
> Signed-off-by: Dexuan Cui 
> ---
>  tools/hv/hv_kvp_daemon.c | 7 +++
>  1 file changed, 7 insertions(+)
>
> diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
> index 22b0764..9f4b303 100644
> --- a/tools/hv/hv_kvp_daemon.c
> +++ b/tools/hv/hv_kvp_daemon.c
> @@ -1559,8 +1559,15 @@ int main(int argc, char *argv[])
>   addr_p, &addr_l);
>
>   if (len < 0) {
> + int saved_errno = errno;
>   syslog(LOG_ERR, "recvfrom failed; pid:%u error:%d %s",
>   addr.nl_pid, errno, strerror(errno));
> +
> + if (saved_errno == ENOBUFS) {

is it possible to meet EAGAIN (or EWOULDBLOCK) here as well? I'd suggest
we ignore these as well in such case. Ignoring ENOMEM here is doubtful,
I think. But possible.

> + syslog(LOG_ERR, "error = ENOBUFS: ignored");
> + continue;
> + }
> +
>   close(fd);
>   return -1;
>   }

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


RE: [PATCH] tools: hv: ignore ENOBUFS in the KVP daemon

2014-11-19 Thread Dexuan Cui
> -Original Message-
> From: Vitaly Kuznetsov
> Sent: Wednesday, November 19, 2014 18:50 PM
> To: Dexuan Cui
> Cc: gre...@linuxfoundation.org; linux-ker...@vger.kernel.org; driverdev-
> de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com;
> jasow...@redhat.com; Haiyang Zhang
> Subject: Re: [PATCH] tools: hv: ignore ENOBUFS in the KVP daemon
> 
> Dexuan Cui  writes:
> 
> > Under high memory pressure and very high KVP R/W test pressure, the
> netlink
> > recvfrom() may transiently return ENOBUFS to the daemon -- we found
> this
> > during a 2-week stress test.
> >
> > We'd better not terminate the daemon on this failure, because a typical
> KVP
> > user can re-try the R/W and hopefully it will succeed next time.
> >
> > diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
> > index 22b0764..9f4b303 100644
> > --- a/tools/hv/hv_kvp_daemon.c
> > +++ b/tools/hv/hv_kvp_daemon.c
> > @@ -1559,8 +1559,15 @@ int main(int argc, char *argv[])
> > addr_p, &addr_l);
> >
> > if (len < 0) {
> > +   int saved_errno = errno;
> > syslog(LOG_ERR, "recvfrom failed; pid:%u
> error:%d %s",
> > addr.nl_pid, errno, strerror(errno));
> > +
> > +   if (saved_errno == ENOBUFS) {
> 
> is it possible to meet EAGAIN (or EWOULDBLOCK) here as well? I'd suggest
> we ignore these as well in such case. Ignoring ENOMEM here is doubtful,
> I think. But possible.
> 
>   Vitaly

I don't think EAGAIN is possible  because "man recvfrom" says
   "If  no messages are available at the socket, the receive calls wait for a
 message to arrive, unless the socket is nonblocking (see fcntl(2)), in 
which
 case the value -1 is returned and  the  external variable  errno is set to
EAGAIN or EWOULDBLOCK".

The same man page mention ENOMEM for recvmsg(), but not recvfrom().

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


Re: [PATCH] tools: hv: ignore ENOBUFS in the KVP daemon

2014-11-19 Thread Vitaly Kuznetsov
Dexuan Cui  writes:

>> -Original Message-
>> From: Vitaly Kuznetsov
>> Sent: Wednesday, November 19, 2014 18:50 PM
>> To: Dexuan Cui
>> Cc: gre...@linuxfoundation.org; linux-ker...@vger.kernel.org; driverdev-
>> de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com;
>> jasow...@redhat.com; Haiyang Zhang
>> Subject: Re: [PATCH] tools: hv: ignore ENOBUFS in the KVP daemon
>> 
>> Dexuan Cui  writes:
>> 
>> > Under high memory pressure and very high KVP R/W test pressure, the netlink
>> > recvfrom() may transiently return ENOBUFS to the daemon -- we found this
>> > during a 2-week stress test.
>> >
>> > We'd better not terminate the daemon on this failure, because a typical KVP
>> > user can re-try the R/W and hopefully it will succeed next time.
>> >
>> > diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
>> > index 22b0764..9f4b303 100644
>> > --- a/tools/hv/hv_kvp_daemon.c
>> > +++ b/tools/hv/hv_kvp_daemon.c
>> > @@ -1559,8 +1559,15 @@ int main(int argc, char *argv[])
>> >addr_p, &addr_l);
>> >
>> >if (len < 0) {
>> > +  int saved_errno = errno;
>> >syslog(LOG_ERR, "recvfrom failed; pid:%u error:%d %s",
>> >addr.nl_pid, errno, strerror(errno));
>> > +
>> > +  if (saved_errno == ENOBUFS) {
>> 
>> is it possible to meet EAGAIN (or EWOULDBLOCK) here as well? I'd suggest
>> we ignore these as well in such case. Ignoring ENOMEM here is doubtful,
>> I think. But possible.
>> 
>>   Vitaly
>
> I don't think EAGAIN is possible  because "man recvfrom" says
>"If  no messages are available at the socket, the receive calls wait for a
>  message to arrive, unless the socket is nonblocking (see fcntl(2)), in 
> which
>  case the value -1 is returned and  the  external variable  errno is set 
> to
> EAGAIN or EWOULDBLOCK".
>
> The same man page mention ENOMEM for recvmsg(), but not recvfrom().

Ah, sorry, I though your patch patches the other place: call to
netlink_send() which does sendmsg() (and my EAGAIN/EWOULDBLOCK/ENOMEM
comment was about it). It could also make sense to patch them both as I
think it is possible to hit these as well.

>
> -- Dexuan

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


RE: [PATCH] tools: hv: ignore ENOBUFS in the KVP daemon

2014-11-19 Thread Dexuan Cui
> -Original Message-
> From: Vitaly Kuznetsov [mailto:vkuzn...@redhat.com]
> Sent: Wednesday, November 19, 2014 20:41 PM
> To: Dexuan Cui
> Cc: gre...@linuxfoundation.org; linux-ker...@vger.kernel.org; driverdev-
> de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com;
> jasow...@redhat.com; Haiyang Zhang
> Subject: Re: [PATCH] tools: hv: ignore ENOBUFS in the KVP daemon
> 
> Dexuan Cui  writes:
> 
> >> -Original Message-
> >> From: Vitaly Kuznetsov
> >> Sent: Wednesday, November 19, 2014 18:50 PM
> >> To: Dexuan Cui
> >> Cc: gre...@linuxfoundation.org; linux-ker...@vger.kernel.org;
> driverdev-
> >> de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com;
> >> jasow...@redhat.com; Haiyang Zhang
> >> Subject: Re: [PATCH] tools: hv: ignore ENOBUFS in the KVP daemon
> >>
> >> Dexuan Cui  writes:
> >>
> >> > Under high memory pressure and very high KVP R/W test pressure,
> the netlink
> >> > recvfrom() may transiently return ENOBUFS to the daemon -- we found
> this
> >> > during a 2-week stress test.
> >> >
> >> > We'd better not terminate the daemon on this failure, because a
> typical KVP
> >> > user can re-try the R/W and hopefully it will succeed next time.
> >> >
> >> > diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
> >> > index 22b0764..9f4b303 100644
> >> > --- a/tools/hv/hv_kvp_daemon.c
> >> > +++ b/tools/hv/hv_kvp_daemon.c
> >> > @@ -1559,8 +1559,15 @@ int main(int argc, char *argv[])
> >> >  addr_p, &addr_l);
> >> >
> >> >  if (len < 0) {
> >> > +int saved_errno = errno;
> >> >  syslog(LOG_ERR, "recvfrom failed; pid:%u
> error:%d %s",
> >> >  addr.nl_pid, errno, 
> >> > strerror(errno));
> >> > +
> >> > +if (saved_errno == ENOBUFS) {
> >>
> >> is it possible to meet EAGAIN (or EWOULDBLOCK) here as well? I'd
> suggest
> >> we ignore these as well in such case. Ignoring ENOMEM here is doubtful,
> >> I think. But possible.
> >>
> >>   Vitaly
> >
> > I don't think EAGAIN is possible  because "man recvfrom" says
> >"If  no messages are available at the socket, the receive calls wait for 
> > a
> >  message to arrive, unless the socket is nonblocking (see fcntl(2)), in
> which
> >  case the value -1 is returned and  the  external variable  errno is 
> > set to
> > EAGAIN or EWOULDBLOCK".
> >
> > The same man page mention ENOMEM for recvmsg(), but not recvfrom().
> 
> Ah, sorry, I though your patch patches the other place: call to
> netlink_send() which does sendmsg() (and my
> EAGAIN/EWOULDBLOCK/ENOMEM
> comment was about it). It could also make sense to patch them both as I
> think it is possible to hit these as well.
> 
> > -- Dexuan
> --
>   Vitaly

OK, I can add this new check:
(I'll send out the v2 tomorrow in case  people have new comments)

--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -1770,8 +1770,15 @@ kvp_done:

len = netlink_send(fd, incoming_cn_msg);
if (len < 0) {
+   int saved_errno = errno;
syslog(LOG_ERR, "net_link send failed; error: %d %s", 
errno,
strerror(errno));
+
+   if (saved_errno == ENOMEM || saved_errno == EAGAIN) {
+   syslog(LOG_ERR, "send error: ignored");
+   continue;
+   }
+
exit(EXIT_FAILURE);
}
}

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


Re: [PATCH] tools: hv: ignore ENOBUFS in the KVP daemon

2014-11-19 Thread Vitaly Kuznetsov
Dexuan Cui  writes:

>> -Original Message-
>> From: Vitaly Kuznetsov [mailto:vkuzn...@redhat.com]
>> Sent: Wednesday, November 19, 2014 20:41 PM
>> To: Dexuan Cui
>> Cc: gre...@linuxfoundation.org; linux-ker...@vger.kernel.org; driverdev-
>> de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com;
>> jasow...@redhat.com; Haiyang Zhang
>> Subject: Re: [PATCH] tools: hv: ignore ENOBUFS in the KVP daemon
>> 
>> Dexuan Cui  writes:
>> 
>> >> -Original Message-
>> >> From: Vitaly Kuznetsov
>> >> Sent: Wednesday, November 19, 2014 18:50 PM
>> >> To: Dexuan Cui
>> >> Cc: gre...@linuxfoundation.org; linux-ker...@vger.kernel.org;
>> driverdev-
>> >> de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com;
>> >> jasow...@redhat.com; Haiyang Zhang
>> >> Subject: Re: [PATCH] tools: hv: ignore ENOBUFS in the KVP daemon
>> >>
>> >> Dexuan Cui  writes:
>> >>
>> >> > Under high memory pressure and very high KVP R/W test pressure,
>> the netlink
>> >> > recvfrom() may transiently return ENOBUFS to the daemon -- we found
>> this
>> >> > during a 2-week stress test.
>> >> >
>> >> > We'd better not terminate the daemon on this failure, because a
>> typical KVP
>> >> > user can re-try the R/W and hopefully it will succeed next time.
>> >> >
>> >> > diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
>> >> > index 22b0764..9f4b303 100644
>> >> > --- a/tools/hv/hv_kvp_daemon.c
>> >> > +++ b/tools/hv/hv_kvp_daemon.c
>> >> > @@ -1559,8 +1559,15 @@ int main(int argc, char *argv[])
>> >> > addr_p, &addr_l);
>> >> >
>> >> > if (len < 0) {
>> >> > +   int saved_errno = errno;
>> >> > syslog(LOG_ERR, "recvfrom failed; pid:%u
>> error:%d %s",
>> >> > addr.nl_pid, errno, 
>> >> > strerror(errno));
>> >> > +
>> >> > +   if (saved_errno == ENOBUFS) {
>> >>
>> >> is it possible to meet EAGAIN (or EWOULDBLOCK) here as well? I'd
>> suggest
>> >> we ignore these as well in such case. Ignoring ENOMEM here is doubtful,
>> >> I think. But possible.
>> >>
>> >>   Vitaly
>> >
>> > I don't think EAGAIN is possible  because "man recvfrom" says
>> >"If  no messages are available at the socket, the receive calls wait 
>> > for a
>> >  message to arrive, unless the socket is nonblocking (see fcntl(2)), in
>> which
>> >  case the value -1 is returned and  the  external variable  errno is 
>> > set to
>> > EAGAIN or EWOULDBLOCK".
>> >
>> > The same man page mention ENOMEM for recvmsg(), but not recvfrom().
>> 
>> Ah, sorry, I though your patch patches the other place: call to
>> netlink_send() which does sendmsg() (and my
>> EAGAIN/EWOULDBLOCK/ENOMEM
>> comment was about it). It could also make sense to patch them both as I
>> think it is possible to hit these as well.
>> 
>> > -- Dexuan
>> --
>>   Vitaly
>
> OK, I can add this new check:
> (I'll send out the v2 tomorrow in case  people have new comments)
>

Thanks!

> --- a/tools/hv/hv_kvp_daemon.c
> +++ b/tools/hv/hv_kvp_daemon.c
> @@ -1770,8 +1770,15 @@ kvp_done:
>
> len = netlink_send(fd, incoming_cn_msg);
> if (len < 0) {
> +   int saved_errno = errno;
> syslog(LOG_ERR, "net_link send failed; error: %d %s", 
> errno,
> strerror(errno));
> +
> +   if (saved_errno == ENOMEM || saved_errno ==  EAGAIN) {

Sorry for being pushy, but it seems ENOBUFS is also possible here (at
least man sendmsg mentions it).

> +   syslog(LOG_ERR, "send error: ignored");
> +   continue;
> +   }
> +
> exit(EXIT_FAILURE);
> }
> }
>
> Thanks,
> -- Dexuan

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


RE: [PATCH] tools: hv: ignore ENOBUFS in the KVP daemon

2014-11-19 Thread Dexuan Cui
> -Original Message-
> From: Vitaly Kuznetsov 
> >> --
> >>   Vitaly
> >
> > OK, I can add this new check:
> > (I'll send out the v2 tomorrow in case  people have new comments)
> >
> 
> Thanks!
> 
> > --- a/tools/hv/hv_kvp_daemon.c
> > +++ b/tools/hv/hv_kvp_daemon.c
> > @@ -1770,8 +1770,15 @@ kvp_done:
> >
> > len = netlink_send(fd, incoming_cn_msg);
> > if (len < 0) {
> > +   int saved_errno = errno;
> > syslog(LOG_ERR, "net_link send failed; error: %d 
> > %s", errno,
> > strerror(errno));
> > +
> > +   if (saved_errno == ENOMEM || saved_errno ==  
> > EAGAIN) {
> 
> Sorry for being pushy, but it seems ENOBUFS is also possible here (at
> least man sendmsg mentions it).
OK, I'll add this too. :-)

BTW, I realized sendmsg() can't return EAGAIN here as that's for non-blocking
socket.

Here I simply ignore the error, hoping the other end will re-try.

> 
> > +   syslog(LOG_ERR, "send error: ignored");
> > +   continue;
> > +   }
> > +
> > exit(EXIT_FAILURE);
> > }
> > }
> >
> > Thanks,
> > -- Dexuan
> 
>   Vitaly

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


Re: [PATCH] tools: hv: ignore ENOBUFS in the KVP daemon

2014-11-19 Thread Vitaly Kuznetsov
Dexuan Cui  writes:

>> -Original Message-
>> From: Vitaly Kuznetsov 
>> >> --
>> >>   Vitaly
>> >
>> > OK, I can add this new check:
>> > (I'll send out the v2 tomorrow in case  people have new comments)
>> >
>> 
>> Thanks!
>> 
>> > --- a/tools/hv/hv_kvp_daemon.c
>> > +++ b/tools/hv/hv_kvp_daemon.c
>> > @@ -1770,8 +1770,15 @@ kvp_done:
>> >
>> > len = netlink_send(fd, incoming_cn_msg);
>> > if (len < 0) {
>> > +   int saved_errno = errno;
>> > syslog(LOG_ERR, "net_link send failed; error: %d 
>> > %s", errno,
>> > strerror(errno));
>> > +
>> > +   if (saved_errno == ENOMEM || saved_errno ==  
>> > EAGAIN) {
>> 
>> Sorry for being pushy, but it seems ENOBUFS is also possible here (at
>> least man sendmsg mentions it).
> OK, I'll add this too. :-)
>
> BTW, I realized sendmsg() can't return EAGAIN here as that's for non-blocking
> socket.
>
> Here I simply ignore the error, hoping the other end will re-try.
>

I agree, it's sufficient to ignore ENOBUFS on recieve path and both
ENOMEM/ENOBUFS on send.

Thanks!

>> 
>> > +   syslog(LOG_ERR, "send error: ignored");
>> > +   continue;
>> > +   }
>> > +
>> > exit(EXIT_FAILURE);
>> > }
>> > }
>> >
>> > Thanks,
>> > -- Dexuan
>> 
>>   Vitaly
>
> -- Dexuan

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


Re: [PATCH 03/10] staging: rtl8723au: rtw_put_snap23a(): Use put_unaligned to set protocol

2014-11-19 Thread Jes Sorensen
Dan Carpenter  writes:
> On Mon, Nov 10, 2014 at 06:11:39PM -0500, jes.soren...@redhat.com wrote:
>> diff --git a/drivers/staging/rtl8723au/core/rtw_xmit.c
>> b/drivers/staging/rtl8723au/core/rtw_xmit.c
>> index 18a9f34..f8b1243 100644
>> --- a/drivers/staging/rtl8723au/core/rtw_xmit.c
>> +++ b/drivers/staging/rtl8723au/core/rtw_xmit.c
>> @@ -1247,7 +1247,7 @@ s32 rtw_put_snap23a(u8 *data, u16 h_proto)
>>  ether_addr_copy(data, rfc1042_header);
>
> Doesn't the ether_add_copy() also require that data is aligned?
>
> regards,
> dan carpenter

Hi Dan,

Actually I think the other one was a false positive, and wasn't
necessary either. We're putting the data onto an xmit frame allocated
within the driver, so alignment ought to be safe.

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


Re: [PATCH] staging: rtl8723au: fix sparse warning

2014-11-19 Thread Jes Sorensen
Aleh Suprunovich  writes:
> drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c:747:1: warning: symbol
> 'rtl8723a_EfusePgPacketRead' was not declared. Should it be static?
>
> Function 'rtl8723a_EfusePgPacketRead' seems to be unused in current
> staging code.
>
> Before, it was available as 'static s32 Hal_EfusePgPacketRead',
> but code that was using it removed, in the same commit as rename and
> signature change to 'bool rtl8723a_EfusePgPacketRead' has taken place.
>
> Signed-off-by: Aleh Suprunovich 
> ---
>  drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c | 78 
> ---
>  1 file changed, 78 deletions(-)

Looks good to me

Signed-off-by: Jes Sorensen 

>
> diff --git a/drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c 
> b/drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c
> index 9a75eb6..3e61a45 100644
> --- a/drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c
> +++ b/drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c
> @@ -743,84 +743,6 @@ u16 rtl8723a_EfuseGetCurrentSize_BT(struct rtw_adapter 
> *padapter)
>   return retU2;
>  }
>  
> -bool
> -rtl8723a_EfusePgPacketRead(struct rtw_adapter *padapter, u8 offset, u8 *data)
> -{
> - u8 efuse_data, word_cnts = 0;
> - u16 efuse_addr = 0;
> - u8 hoffset = 0, hworden = 0;
> - u8 i;
> - u8 max_section = 0;
> - s32 ret;
> -
> - if (data == NULL)
> - return false;
> -
> - EFUSE_GetEfuseDefinition23a(padapter, EFUSE_WIFI, 
> TYPE_EFUSE_MAX_SECTION,
> -  &max_section);
> - if (offset > max_section) {
> - DBG_8723A("%s: Packet offset(%d) is illegal(>%d)!\n",
> -   __func__, offset, max_section);
> - return false;
> - }
> -
> - memset(data, 0xFF, PGPKT_DATA_SIZE);
> - ret = true;
> -
> - /*  */
> - /*   Efuse has been pre-programmed dummy 5Bytes at the
> - end of Efuse by CP. */
> - /*  Skip dummy parts to prevent unexpected data read from Efuse. */
> - /*  By pass right now. 2009.02.19. */
> - /*  */
> - while (AVAILABLE_EFUSE_ADDR(efuse_addr)) {
> - if (efuse_OneByteRead23a(padapter, efuse_addr++, &efuse_data) ==
> - _FAIL) {
> - ret = false;
> - break;
> - }
> -
> - if (efuse_data == 0xFF)
> - break;
> -
> - if (EXT_HEADER(efuse_data)) {
> - hoffset = GET_HDR_OFFSET_2_0(efuse_data);
> - efuse_OneByteRead23a(padapter, efuse_addr++, 
> &efuse_data);
> - if (ALL_WORDS_DISABLED(efuse_data)) {
> - DBG_8723A("%s: Error!! All words disabled!\n",
> -   __func__);
> - continue;
> - }
> -
> - hoffset |= ((efuse_data & 0xF0) >> 1);
> - hworden = efuse_data & 0x0F;
> - } else {
> - hoffset = (efuse_data >> 4) & 0x0F;
> - hworden = efuse_data & 0x0F;
> - }
> -
> - if (hoffset == offset) {
> - for (i = 0; i < EFUSE_MAX_WORD_UNIT; i++) {
> - /* Check word enable condition in the section */
> - if (!(hworden & (0x01 << i))) {
> - ReadEFuseByte23a(padapter, efuse_addr++,
> -   &efuse_data);
> - data[i * 2] = efuse_data;
> -
> - ReadEFuseByte23a(padapter, efuse_addr++,
> -   &efuse_data);
> - data[(i * 2) + 1] = efuse_data;
> - }
> - }
> - } else {
> - word_cnts = Efuse_CalculateWordCnts23a(hworden);
> - efuse_addr += word_cnts * 2;
> - }
> - }
> -
> - return ret;
> -}
> -
>  void rtl8723a_read_chip_version(struct rtw_adapter *padapter)
>  {
>   u32 value32;
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] staging:rtl8723au: fix sparse warning: incorrect type in assignment

2014-11-19 Thread Kinka Huang
Signed-off-by: Kinka Huang 
---
 drivers/staging/rtl8723au/core/rtw_mlme.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8723au/core/rtw_mlme.c 
b/drivers/staging/rtl8723au/core/rtw_mlme.c
index 85d1eca..6c1a7fb 100644
--- a/drivers/staging/rtl8723au/core/rtw_mlme.c
+++ b/drivers/staging/rtl8723au/core/rtw_mlme.c
@@ -2158,9 +2158,9 @@ bool rtw_restructure_ht_ie23a(struct rtw_adapter 
*padapter, u8 *in_ie,
 
memset(&ht_capie, 0, sizeof(struct ieee80211_ht_cap));
 
-   ht_capie.cap_info = IEEE80211_HT_CAP_SUP_WIDTH_20_40 |
+   ht_capie.cap_info = 
cpu_to_le16(IEEE80211_HT_CAP_SUP_WIDTH_20_40 |
IEEE80211_HT_CAP_SGI_20 | IEEE80211_HT_CAP_SGI_40 |
-   IEEE80211_HT_CAP_TX_STBC | IEEE80211_HT_CAP_DSSSCCK40;
+   IEEE80211_HT_CAP_TX_STBC | IEEE80211_HT_CAP_DSSSCCK40);
 
GetHalDefVar8192CUsb(padapter, HAL_DEF_RX_PACKET_OFFSET,
 &rx_packet_offset);
-- 
1.8.3.2

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


Re: [PATCH] staging:rtl8723au: fix sparse warning: incorrect type in assignment

2014-11-19 Thread Jes Sorensen
Kinka Huang  writes:
> Signed-off-by: Kinka Huang 
> ---
>  drivers/staging/rtl8723au/core/rtw_mlme.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Eeeek, nice catch!

Acked-by: Jes Sorensen 

>
> diff --git a/drivers/staging/rtl8723au/core/rtw_mlme.c 
> b/drivers/staging/rtl8723au/core/rtw_mlme.c
> index 85d1eca..6c1a7fb 100644
> --- a/drivers/staging/rtl8723au/core/rtw_mlme.c
> +++ b/drivers/staging/rtl8723au/core/rtw_mlme.c
> @@ -2158,9 +2158,9 @@ bool rtw_restructure_ht_ie23a(struct rtw_adapter 
> *padapter, u8 *in_ie,
>  
>   memset(&ht_capie, 0, sizeof(struct ieee80211_ht_cap));
>  
> - ht_capie.cap_info = IEEE80211_HT_CAP_SUP_WIDTH_20_40 |
> + ht_capie.cap_info = 
> cpu_to_le16(IEEE80211_HT_CAP_SUP_WIDTH_20_40 |
>   IEEE80211_HT_CAP_SGI_20 | IEEE80211_HT_CAP_SGI_40 |
> - IEEE80211_HT_CAP_TX_STBC | IEEE80211_HT_CAP_DSSSCCK40;
> + IEEE80211_HT_CAP_TX_STBC | IEEE80211_HT_CAP_DSSSCCK40);
>  
>   GetHalDefVar8192CUsb(padapter, HAL_DEF_RX_PACKET_OFFSET,
>&rx_packet_offset);
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


ARE YOU THERE?

2014-11-19 Thread Blaise Kamaté
Hello my Dear,

I will greatly appreciate my correspondence meets you in good health condition.

My name is Mr. Blaise  Kamaté. I am seeking for your co-operation for 
investment partnership in your Country. I shall provide the FUND for the 
investment. When you acknowledged the receipt of this correspondence, 
thereafter I will give you the Full Details of my investment proposal.

I await your response in earliest.

My regards,
Mr. Blaise  Kamaté
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v2 0/9] staging: panel: Refactor panel initialization

2014-11-19 Thread Mariusz Gorski
This set of patches focuses on making current initialization
process easier to understand - especially it tries to emphasize
what are the priorities of the params coming from different
sources (Kconfig values, device profiles and module param values
set on loading). I paid attention not to change to behaviour of
the code itself (at least for now), so all hacky places are kept.

v2: Don't introduce new macros for param value check

Mariusz Gorski (9):
  staging: panel: Set default parport module param value
  staging: panel: Call init function directly
  staging: panel: Remove magic numbers
  staging: panel: Use defined value or checking module params state
  staging: panel: Start making module params read-only
  staging: panel: Make two more module params read-only
  staging: panel: Refactor LCD init code
  staging: panel: Remove more magic number comparison
  staging: panel: Move LCD-related state into struct lcd

 drivers/staging/panel/panel.c | 669 ++
 1 file changed, 354 insertions(+), 315 deletions(-)

-- 
2.1.3

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


[PATCH v2 1/9] staging: panel: Set default parport module param value

2014-11-19 Thread Mariusz Gorski
Set default parport module param value to DEFAULT_PARPORT so that
a if-block can be avoided.

Signed-off-by: Mariusz Gorski 
Acked-by: Willy Tarreau 
---
 drivers/staging/panel/panel.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c
index c6eeddf..3dd318a 100644
--- a/drivers/staging/panel/panel.c
+++ b/drivers/staging/panel/panel.c
@@ -429,7 +429,7 @@ static struct timer_list scan_timer;
 
 MODULE_DESCRIPTION("Generic parallel port LCD/Keypad driver");
 
-static int parport = -1;
+static int parport = DEFAULT_PARPORT;
 module_param(parport, int, );
 MODULE_PARM_DESC(parport, "Parallel port index (0=lpt1, 1=lpt2, ...)");
 
@@ -2231,9 +2231,6 @@ static int panel_init(void)
if (lcd_type < 0)
lcd_type = lcd_enabled;
 
-   if (parport < 0)
-   parport = DEFAULT_PARPORT;
-
/* take care of an eventual profile */
switch (profile) {
case PANEL_PROFILE_CUSTOM:
-- 
2.1.3

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


[PATCH v2 4/9] staging: panel: Use defined value or checking module params state

2014-11-19 Thread Mariusz Gorski
Avoid magic number and use a comparison with a defined value instead
that checks whether module param has been set by the user to some
value at loading time.

Signed-off-by: Mariusz Gorski 
---
v2: Don't introduce new macros for param value check

 drivers/staging/panel/panel.c | 86 +--
 1 file changed, 43 insertions(+), 43 deletions(-)

diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c
index 1b4a211..5288990 100644
--- a/drivers/staging/panel/panel.c
+++ b/drivers/staging/panel/panel.c
@@ -1411,29 +1411,29 @@ static void lcd_init(void)
switch (lcd_type) {
case LCD_TYPE_OLD:
/* parallel mode, 8 bits */
-   if (lcd_proto < 0)
+   if (lcd_proto == NOT_SET)
lcd_proto = LCD_PROTO_PARALLEL;
-   if (lcd_charset < 0)
+   if (lcd_charset == NOT_SET)
lcd_charset = LCD_CHARSET_NORMAL;
if (lcd_e_pin == PIN_NOT_SET)
lcd_e_pin = PIN_STROBE;
if (lcd_rs_pin == PIN_NOT_SET)
lcd_rs_pin = PIN_AUTOLF;
 
-   if (lcd_width < 0)
+   if (lcd_width == NOT_SET)
lcd_width = 40;
-   if (lcd_bwidth < 0)
+   if (lcd_bwidth == NOT_SET)
lcd_bwidth = 40;
-   if (lcd_hwidth < 0)
+   if (lcd_hwidth == NOT_SET)
lcd_hwidth = 64;
-   if (lcd_height < 0)
+   if (lcd_height == NOT_SET)
lcd_height = 2;
break;
case LCD_TYPE_KS0074:
/* serial mode, ks0074 */
-   if (lcd_proto < 0)
+   if (lcd_proto == NOT_SET)
lcd_proto = LCD_PROTO_SERIAL;
-   if (lcd_charset < 0)
+   if (lcd_charset == NOT_SET)
lcd_charset = LCD_CHARSET_KS0074;
if (lcd_bl_pin == PIN_NOT_SET)
lcd_bl_pin = PIN_AUTOLF;
@@ -1442,20 +1442,20 @@ static void lcd_init(void)
if (lcd_da_pin == PIN_NOT_SET)
lcd_da_pin = PIN_D0;
 
-   if (lcd_width < 0)
+   if (lcd_width == NOT_SET)
lcd_width = 16;
-   if (lcd_bwidth < 0)
+   if (lcd_bwidth == NOT_SET)
lcd_bwidth = 40;
-   if (lcd_hwidth < 0)
+   if (lcd_hwidth == NOT_SET)
lcd_hwidth = 16;
-   if (lcd_height < 0)
+   if (lcd_height == NOT_SET)
lcd_height = 2;
break;
case LCD_TYPE_NEXCOM:
/* parallel mode, 8 bits, generic */
-   if (lcd_proto < 0)
+   if (lcd_proto == NOT_SET)
lcd_proto = LCD_PROTO_PARALLEL;
-   if (lcd_charset < 0)
+   if (lcd_charset == NOT_SET)
lcd_charset = LCD_CHARSET_NORMAL;
if (lcd_e_pin == PIN_NOT_SET)
lcd_e_pin = PIN_AUTOLF;
@@ -1464,42 +1464,42 @@ static void lcd_init(void)
if (lcd_rw_pin == PIN_NOT_SET)
lcd_rw_pin = PIN_INITP;
 
-   if (lcd_width < 0)
+   if (lcd_width == NOT_SET)
lcd_width = 16;
-   if (lcd_bwidth < 0)
+   if (lcd_bwidth == NOT_SET)
lcd_bwidth = 40;
-   if (lcd_hwidth < 0)
+   if (lcd_hwidth == NOT_SET)
lcd_hwidth = 64;
-   if (lcd_height < 0)
+   if (lcd_height == NOT_SET)
lcd_height = 2;
break;
case LCD_TYPE_CUSTOM:
/* customer-defined */
-   if (lcd_proto < 0)
+   if (lcd_proto == NOT_SET)
lcd_proto = DEFAULT_LCD_PROTO;
-   if (lcd_charset < 0)
+   if (lcd_charset == NOT_SET)
lcd_charset = DEFAULT_LCD_CHARSET;
/* default geometry will be set later */
break;
case LCD_TYPE_HANTRONIX:
/* parallel mode, 8 bits, hantronix-like */
default:
-   if (lcd_proto < 0)
+   if (lcd_proto == NOT_SET)
lcd_proto = LCD_PROTO_PARALLEL;
-   if (lcd_charset < 0)
+   if (lcd_charset == NOT_SET)
lcd_charset = LCD_CHARSET_NORMAL;
if (lcd_e_pin == PIN_NOT_SET)
lcd_e_pin = PIN_STROBE;
if (lcd_rs_pin == PIN_NOT_SET)
lcd_rs_pin = PIN_SELECP;
 
-   if (lcd_width < 0)
+   if (lcd_width == NOT_SET)
lcd_width = 16;
-   if (lcd_bwidth < 0)
+  

[PATCH v2 5/9] staging: panel: Start making module params read-only

2014-11-19 Thread Mariusz Gorski
Start decoupling module params from the actual device state,
both for lcd and keypad, by keeping the params read-only
and moving the device state to related structs.

Signed-off-by: Mariusz Gorski 
Acked-by: Willy Tarreau 
---
 drivers/staging/panel/panel.c | 35 +--
 1 file changed, 21 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c
index 5288990..7e5bb53 100644
--- a/drivers/staging/panel/panel.c
+++ b/drivers/staging/panel/panel.c
@@ -212,6 +212,10 @@ static pmask_t phys_prev;
 static char inputs_stable;
 
 /* these variables are specific to the keypad */
+static struct {
+   bool enabled;
+} keypad;
+
 static char keypad_buffer[KEYPAD_BUFFER];
 static int keypad_buflen;
 static int keypad_start;
@@ -219,6 +223,9 @@ static char keypressed;
 static wait_queue_head_t keypad_read_wait;
 
 /* lcd-specific variables */
+static struct {
+   bool enabled;
+} lcd;
 
 /* contains the LCD config state */
 static unsigned long int lcd_flags;
@@ -1393,7 +1400,7 @@ static void panel_lcd_print(const char *s)
const char *tmp = s;
int count = strlen(s);
 
-   if (lcd_enabled && lcd_initialized) {
+   if (lcd.enabled && lcd_initialized) {
for (; count-- > 0; tmp++) {
if (!in_interrupt() && (((count + 1) & 0x1f) == 0))
/* let's be a little nice with other processes
@@ -1923,7 +1930,7 @@ static void panel_process_inputs(void)
 
 static void panel_scan_timer(void)
 {
-   if (keypad_enabled && keypad_initialized) {
+   if (keypad.enabled && keypad_initialized) {
if (spin_trylock_irq(&pprt_lock)) {
phys_scan_contacts();
 
@@ -1935,7 +1942,7 @@ static void panel_scan_timer(void)
panel_process_inputs();
}
 
-   if (lcd_enabled && lcd_initialized) {
+   if (lcd.enabled && lcd_initialized) {
if (keypressed) {
if (light_tempo == 0 && ((lcd_flags & LCD_FLAG_L) == 0))
lcd_backlight(1);
@@ -2114,7 +2121,7 @@ static void keypad_init(void)
 static int panel_notify_sys(struct notifier_block *this, unsigned long code,
void *unused)
 {
-   if (lcd_enabled && lcd_initialized) {
+   if (lcd.enabled && lcd_initialized) {
switch (code) {
case SYS_DOWN:
panel_lcd_print
@@ -2170,13 +2177,13 @@ static void panel_attach(struct parport *port)
/* must init LCD first, just in case an IRQ from the keypad is
 * generated at keypad init
 */
-   if (lcd_enabled) {
+   if (lcd.enabled) {
lcd_init();
if (misc_register(&lcd_dev))
goto err_unreg_device;
}
 
-   if (keypad_enabled) {
+   if (keypad.enabled) {
keypad_init();
if (misc_register(&keypad_dev))
goto err_lcd_unreg;
@@ -2184,7 +2191,7 @@ static void panel_attach(struct parport *port)
return;
 
 err_lcd_unreg:
-   if (lcd_enabled)
+   if (lcd.enabled)
misc_deregister(&lcd_dev);
 err_unreg_device:
parport_unregister_device(pprt);
@@ -2202,12 +2209,12 @@ static void panel_detach(struct parport *port)
return;
}
 
-   if (keypad_enabled && keypad_initialized) {
+   if (keypad.enabled && keypad_initialized) {
misc_deregister(&keypad_dev);
keypad_initialized = 0;
}
 
-   if (lcd_enabled && lcd_initialized) {
+   if (lcd.enabled && lcd_initialized) {
misc_deregister(&lcd_dev);
lcd_initialized = 0;
}
@@ -2283,8 +2290,8 @@ static int __init panel_init_module(void)
break;
}
 
-   lcd_enabled = (lcd_type > 0);
-   keypad_enabled = (keypad_type > 0);
+   lcd.enabled = (lcd_type > 0);
+   keypad.enabled = (keypad_type > 0);
 
switch (keypad_type) {
case KEYPAD_TYPE_OLD:
@@ -2309,7 +2316,7 @@ static int __init panel_init_module(void)
return -EIO;
}
 
-   if (!lcd_enabled && !keypad_enabled) {
+   if (!lcd.enabled && !keypad.enabled) {
/* no device enabled, let's release the parport */
if (pprt) {
parport_release(pprt);
@@ -2344,12 +2351,12 @@ static void __exit panel_cleanup_module(void)
del_timer_sync(&scan_timer);
 
if (pprt != NULL) {
-   if (keypad_enabled) {
+   if (keypad.enabled) {
misc_deregister(&keypad_dev);
keypad_initialized = 0;
}
 
-   if (lcd_enabled) {
+   if (lcd.enabled) {
panel_lcd_print("\x0cLCD driver " PANEL_VERSION
  

[PATCH v2 7/9] staging: panel: Refactor LCD init code

2014-11-19 Thread Mariusz Gorski
Rework lcd_init method to make it a little bit more clear about
the precedence of the params, move LCD geometry and pins layout
to the LCD struct and thus make the LCD-related module params
effectively read-only.

Signed-off-by: Mariusz Gorski 
---
 drivers/staging/panel/panel.c | 304 ++
 1 file changed, 163 insertions(+), 141 deletions(-)

diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c
index 5868a28..0bdb447 100644
--- a/drivers/staging/panel/panel.c
+++ b/drivers/staging/panel/panel.c
@@ -225,6 +225,21 @@ static wait_queue_head_t keypad_read_wait;
 /* lcd-specific variables */
 static struct {
bool enabled;
+   int height;
+   int width;
+   int bwidth;
+   int hwidth;
+   int charset;
+   int proto;
+   /* TODO: use union here? */
+   struct {
+   int e;
+   int rs;
+   int rw;
+   int cl;
+   int da;
+   int bl;
+   } pins;
 } lcd;
 
 /* Needed only for init */
@@ -766,7 +781,7 @@ static void lcd_send_serial(int byte)
 /* turn the backlight on or off */
 static void lcd_backlight(int on)
 {
-   if (lcd_bl_pin == PIN_NONE)
+   if (lcd.pins.bl == PIN_NONE)
return;
 
/* The backlight is activated by setting the AUTOFEED line to +5V  */
@@ -865,23 +880,23 @@ static void lcd_write_data_tilcd(int data)
 static void lcd_gotoxy(void)
 {
lcd_write_cmd(0x80  /* set DDRAM address */
- | (lcd_addr_y ? lcd_hwidth : 0)
+ | (lcd_addr_y ? lcd.hwidth : 0)
  /* we force the cursor to stay at the end of the
 line if it wants to go farther */
- | ((lcd_addr_x < lcd_bwidth) ? lcd_addr_x &
-(lcd_hwidth - 1) : lcd_bwidth - 1));
+ | ((lcd_addr_x < lcd.bwidth) ? lcd_addr_x &
+(lcd.hwidth - 1) : lcd.bwidth - 1));
 }
 
 static void lcd_print(char c)
 {
-   if (lcd_addr_x < lcd_bwidth) {
+   if (lcd_addr_x < lcd.bwidth) {
if (lcd_char_conv != NULL)
c = lcd_char_conv[(unsigned char)c];
lcd_write_data(c);
lcd_addr_x++;
}
/* prevents the cursor from wrapping onto the next line */
-   if (lcd_addr_x == lcd_bwidth)
+   if (lcd_addr_x == lcd.bwidth)
lcd_gotoxy();
 }
 
@@ -895,7 +910,7 @@ static void lcd_clear_fast_s(void)
lcd_gotoxy();
 
spin_lock_irq(&pprt_lock);
-   for (pos = 0; pos < lcd_height * lcd_hwidth; pos++) {
+   for (pos = 0; pos < lcd.height * lcd.hwidth; pos++) {
lcd_send_serial(0x5F);  /* R/W=W, RS=1 */
lcd_send_serial(' ' & 0x0F);
lcd_send_serial((' ' >> 4) & 0x0F);
@@ -918,7 +933,7 @@ static void lcd_clear_fast_p8(void)
lcd_gotoxy();
 
spin_lock_irq(&pprt_lock);
-   for (pos = 0; pos < lcd_height * lcd_hwidth; pos++) {
+   for (pos = 0; pos < lcd.height * lcd.hwidth; pos++) {
/* present the data to the data port */
w_dtr(pprt, ' ');
 
@@ -956,7 +971,7 @@ static void lcd_clear_fast_tilcd(void)
lcd_gotoxy();
 
spin_lock_irq(&pprt_lock);
-   for (pos = 0; pos < lcd_height * lcd_hwidth; pos++) {
+   for (pos = 0; pos < lcd.height * lcd.hwidth; pos++) {
/* present the data to the data port */
w_dtr(pprt, ' ');
udelay(60);
@@ -981,7 +996,7 @@ static void lcd_clear_display(void)
 
 static void lcd_init_display(void)
 {
-   lcd_flags = ((lcd_height > 1) ? LCD_FLAG_N : 0)
+   lcd_flags = ((lcd.height > 1) ? LCD_FLAG_N : 0)
| LCD_FLAG_D | LCD_FLAG_C | LCD_FLAG_B;
 
long_sleep(20); /* wait 20 ms after power-up for the paranoid */
@@ -1095,17 +1110,17 @@ static inline int handle_lcd_special_code(void)
case 'l':   /* Shift Cursor Left */
if (lcd_addr_x > 0) {
/* back one char if not at end of line */
-   if (lcd_addr_x < lcd_bwidth)
+   if (lcd_addr_x < lcd.bwidth)
lcd_write_cmd(0x10);
lcd_addr_x--;
}
processed = 1;
break;
case 'r':   /* shift cursor right */
-   if (lcd_addr_x < lcd_width) {
+   if (lcd_addr_x < lcd.width) {
/* allow the cursor to pass the end of the line */
if (lcd_addr_x <
-   (lcd_bwidth - 1))
+   (lcd.bwidth - 1))
lcd_write_cmd(0x14);
lcd_addr_x++;
}
@@ -1124,7 +1139,7 @@ static inline int handle_lcd_special_code(void)
case 'k': { /* kill end of line */
  

[PATCH v2 6/9] staging: panel: Make two more module params read-only

2014-11-19 Thread Mariusz Gorski
Make keypad_type and lcd_type module params read-only.
This step also starts making it more clear what is
the precedence of device params coming from different
sources (device profile, runtime module param values etc).

Signed-off-by: Mariusz Gorski 
---
 drivers/staging/panel/panel.c | 71 ++-
 1 file changed, 37 insertions(+), 34 deletions(-)

diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c
index 7e5bb53..5868a28 100644
--- a/drivers/staging/panel/panel.c
+++ b/drivers/staging/panel/panel.c
@@ -227,6 +227,9 @@ static struct {
bool enabled;
 } lcd;
 
+/* Needed only for init */
+static int selected_lcd_type = NOT_SET;
+
 /* contains the LCD config state */
 static unsigned long int lcd_flags;
 /* contains the LCD X offset */
@@ -1415,7 +1418,7 @@ static void panel_lcd_print(const char *s)
 /* initialize the LCD driver */
 static void lcd_init(void)
 {
-   switch (lcd_type) {
+   switch (selected_lcd_type) {
case LCD_TYPE_OLD:
/* parallel mode, 8 bits */
if (lcd_proto == NOT_SET)
@@ -2233,28 +2236,21 @@ static struct parport_driver panel_driver = {
 /* init function */
 static int __init panel_init_module(void)
 {
-   /* for backwards compatibility */
-   if (keypad_type == NOT_SET)
-   keypad_type = keypad_enabled;
-
-   if (lcd_type == NOT_SET)
-   lcd_type = lcd_enabled;
+   int selected_keypad_type = NOT_SET;
 
/* take care of an eventual profile */
switch (profile) {
case PANEL_PROFILE_CUSTOM:
/* custom profile */
-   if (keypad_type == NOT_SET)
-   keypad_type = DEFAULT_KEYPAD_TYPE;
-   if (lcd_type == NOT_SET)
-   lcd_type = DEFAULT_LCD_TYPE;
+   selected_keypad_type = DEFAULT_KEYPAD_TYPE;
+   selected_lcd_type = DEFAULT_LCD_TYPE;
break;
case PANEL_PROFILE_OLD:
/* 8 bits, 2*16, old keypad */
-   if (keypad_type == NOT_SET)
-   keypad_type = KEYPAD_TYPE_OLD;
-   if (lcd_type == NOT_SET)
-   lcd_type = LCD_TYPE_OLD;
+   selected_keypad_type = KEYPAD_TYPE_OLD;
+   selected_lcd_type = LCD_TYPE_OLD;
+
+   /* TODO: This two are a little hacky, sort it out later */
if (lcd_width == NOT_SET)
lcd_width = 16;
if (lcd_hwidth == NOT_SET)
@@ -2262,38 +2258,45 @@ static int __init panel_init_module(void)
break;
case PANEL_PROFILE_NEW:
/* serial, 2*16, new keypad */
-   if (keypad_type == NOT_SET)
-   keypad_type = KEYPAD_TYPE_NEW;
-   if (lcd_type == NOT_SET)
-   lcd_type = LCD_TYPE_KS0074;
+   selected_keypad_type = KEYPAD_TYPE_NEW;
+   selected_lcd_type = LCD_TYPE_KS0074;
break;
case PANEL_PROFILE_HANTRONIX:
/* 8 bits, 2*16 hantronix-like, no keypad */
-   if (keypad_type == NOT_SET)
-   keypad_type = KEYPAD_TYPE_NONE;
-   if (lcd_type == NOT_SET)
-   lcd_type = LCD_TYPE_HANTRONIX;
+   selected_keypad_type = KEYPAD_TYPE_NONE;
+   selected_lcd_type = LCD_TYPE_HANTRONIX;
break;
case PANEL_PROFILE_NEXCOM:
/* generic 8 bits, 2*16, nexcom keypad, eg. Nexcom. */
-   if (keypad_type == NOT_SET)
-   keypad_type = KEYPAD_TYPE_NEXCOM;
-   if (lcd_type == NOT_SET)
-   lcd_type = LCD_TYPE_NEXCOM;
+   selected_keypad_type = KEYPAD_TYPE_NEXCOM;
+   selected_lcd_type = LCD_TYPE_NEXCOM;
break;
case PANEL_PROFILE_LARGE:
/* 8 bits, 2*40, old keypad */
-   if (keypad_type == NOT_SET)
-   keypad_type = KEYPAD_TYPE_OLD;
-   if (lcd_type == NOT_SET)
-   lcd_type = LCD_TYPE_OLD;
+   selected_keypad_type = KEYPAD_TYPE_OLD;
+   selected_lcd_type = LCD_TYPE_OLD;
break;
}
 
-   lcd.enabled = (lcd_type > 0);
-   keypad.enabled = (keypad_type > 0);
+   /*
+* Overwrite selection with module param values (both keypad and lcd),
+* where the deprecated params have lower prio.
+*/
+   if (keypad_enabled > -1)
+   selected_keypad_type = keypad_enabled;
+   if (keypad_type > -1)
+   selected_keypad_type = keypad_type;
+
+   keypad.enabled = (selected_keypad_type > 0);
+
+   if (lcd_enabled > -1)
+   selected_lcd_type = lcd_enabled;
+   if (lcd_type > -1)
+   selected_lcd_type = lcd_type;
+
+   lcd.enabled = (selected_lcd_type > 0);
 
-   

[PATCH v2 3/9] staging: panel: Remove magic numbers

2014-11-19 Thread Mariusz Gorski
Get rid of magic numbers indicating that the value of a module param
is not set. Use a defined value instead.

Signed-off-by: Mariusz Gorski 
Acked-by: Willy Tarreau 
---
 drivers/staging/panel/panel.c | 22 --
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c
index 0d3f09e..1b4a211 100644
--- a/drivers/staging/panel/panel.c
+++ b/drivers/staging/panel/panel.c
@@ -133,6 +133,8 @@
 #define LCD_ESCAPE_LEN 24  /* max chars for LCD escape command */
 #define LCD_ESCAPE_CHAR27  /* use char 27 for escape command */
 
+#define NOT_SET-1
+
 /* macros to simplify use of the parallel port */
 #define r_ctr(x)(parport_read_control((x)->port))
 #define r_dtr(x)(parport_read_data((x)->port))
@@ -439,37 +441,37 @@ MODULE_PARM_DESC(profile,
 "1=16x2 old kp; 2=serial 16x2, new kp; 3=16x2 hantronix; "
 "4=16x2 nexcom; default=40x2, old kp");
 
-static int keypad_type = -1;
+static int keypad_type = NOT_SET;
 module_param(keypad_type, int, );
 MODULE_PARM_DESC(keypad_type,
 "Keypad type: 0=none, 1=old 6 keys, 2=new 6+1 keys, 3=nexcom 4 
keys");
 
-static int lcd_type = -1;
+static int lcd_type = NOT_SET;
 module_param(lcd_type, int, );
 MODULE_PARM_DESC(lcd_type,
 "LCD type: 0=none, 1=old //, 2=serial ks0074, 3=hantronix //, 
4=nexcom //, 5=compiled-in");
 
-static int lcd_height = -1;
+static int lcd_height = NOT_SET;
 module_param(lcd_height, int, );
 MODULE_PARM_DESC(lcd_height, "Number of lines on the LCD");
 
-static int lcd_width = -1;
+static int lcd_width = NOT_SET;
 module_param(lcd_width, int, );
 MODULE_PARM_DESC(lcd_width, "Number of columns on the LCD");
 
-static int lcd_bwidth = -1;/* internal buffer width (usually 40) */
+static int lcd_bwidth = NOT_SET;   /* internal buffer width (usually 40) */
 module_param(lcd_bwidth, int, );
 MODULE_PARM_DESC(lcd_bwidth, "Internal LCD line width (40)");
 
-static int lcd_hwidth = -1;/* hardware buffer width (usually 64) */
+static int lcd_hwidth = NOT_SET;   /* hardware buffer width (usually 64) */
 module_param(lcd_hwidth, int, );
 MODULE_PARM_DESC(lcd_hwidth, "LCD line hardware address (64)");
 
-static int lcd_charset = -1;
+static int lcd_charset = NOT_SET;
 module_param(lcd_charset, int, );
 MODULE_PARM_DESC(lcd_charset, "LCD character set: 0=standard, 1=KS0074");
 
-static int lcd_proto = -1;
+static int lcd_proto = NOT_SET;
 module_param(lcd_proto, int, );
 MODULE_PARM_DESC(lcd_proto,
 "LCD communication: 0=parallel (//), 1=serial, 2=TI LCD 
Interface");
@@ -515,11 +517,11 @@ MODULE_PARM_DESC(lcd_bl_pin,
 
 /* Deprecated module parameters - consider not using them anymore */
 
-static int lcd_enabled = -1;
+static int lcd_enabled = NOT_SET;
 module_param(lcd_enabled, int, );
 MODULE_PARM_DESC(lcd_enabled, "Deprecated option, use lcd_type instead");
 
-static int keypad_enabled = -1;
+static int keypad_enabled = NOT_SET;
 module_param(keypad_enabled, int, );
 MODULE_PARM_DESC(keypad_enabled, "Deprecated option, use keypad_type instead");
 
-- 
2.1.3

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


[PATCH v2 9/9] staging: panel: Move LCD-related state into struct lcd

2014-11-19 Thread Mariusz Gorski
Move more or less all LCD-related state into struct lcd
in order to get better cohesion; use bool instead of int
where it makes sense.

Signed-off-by: Mariusz Gorski 
Acked-by: Willy Tarreau 
---
 drivers/staging/panel/panel.c | 255 ++
 1 file changed, 134 insertions(+), 121 deletions(-)

diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c
index 19f6767..98325b7 100644
--- a/drivers/staging/panel/panel.c
+++ b/drivers/staging/panel/panel.c
@@ -225,12 +225,20 @@ static wait_queue_head_t keypad_read_wait;
 /* lcd-specific variables */
 static struct {
bool enabled;
+   bool initialized;
+   bool must_clear;
+
+   /* TODO: use bool here? */
+   char left_shift;
+
int height;
int width;
int bwidth;
int hwidth;
int charset;
int proto;
+   int light_tempo;
+
/* TODO: use union here? */
struct {
int e;
@@ -240,22 +248,26 @@ static struct {
int da;
int bl;
} pins;
+
+   /* contains the LCD config state */
+   unsigned long int flags;
+
+   /* Contains the LCD X and Y offset */
+   struct {
+   unsigned long int x;
+   unsigned long int y;
+   } addr;
+
+   /* Current escape sequence and it's length or -1 if outside */
+   struct {
+   char buf[LCD_ESCAPE_LEN + 1];
+   int len;
+   } esc_seq;
 } lcd;
 
 /* Needed only for init */
 static int selected_lcd_type = NOT_SET;
 
-/* contains the LCD config state */
-static unsigned long int lcd_flags;
-/* contains the LCD X offset */
-static unsigned long int lcd_addr_x;
-/* contains the LCD Y offset */
-static unsigned long int lcd_addr_y;
-/* current escape sequence, 0 terminated */
-static char lcd_escape[LCD_ESCAPE_LEN + 1];
-/* not in escape state. >=0 = escape cmd len */
-static int lcd_escape_len = -1;
-
 /*
  * Bit masks to convert LCD signals to parallel port outputs.
  * _d_ are values for data port, _c_ are for control port.
@@ -438,13 +450,8 @@ static atomic_t keypad_available = ATOMIC_INIT(1);
 
 static struct pardevice *pprt;
 
-static int lcd_initialized;
 static int keypad_initialized;
 
-static int light_tempo;
-
-static char lcd_must_clear;
-static char lcd_left_shift;
 static char init_in_progress;
 
 static void (*lcd_write_cmd)(int);
@@ -880,23 +887,23 @@ static void lcd_write_data_tilcd(int data)
 static void lcd_gotoxy(void)
 {
lcd_write_cmd(0x80  /* set DDRAM address */
- | (lcd_addr_y ? lcd.hwidth : 0)
+ | (lcd.addr.y ? lcd.hwidth : 0)
  /* we force the cursor to stay at the end of the
 line if it wants to go farther */
- | ((lcd_addr_x < lcd.bwidth) ? lcd_addr_x &
+ | ((lcd.addr.x < lcd.bwidth) ? lcd.addr.x &
 (lcd.hwidth - 1) : lcd.bwidth - 1));
 }
 
 static void lcd_print(char c)
 {
-   if (lcd_addr_x < lcd.bwidth) {
+   if (lcd.addr.x < lcd.bwidth) {
if (lcd_char_conv != NULL)
c = lcd_char_conv[(unsigned char)c];
lcd_write_data(c);
-   lcd_addr_x++;
+   lcd.addr.x++;
}
/* prevents the cursor from wrapping onto the next line */
-   if (lcd_addr_x == lcd.bwidth)
+   if (lcd.addr.x == lcd.bwidth)
lcd_gotoxy();
 }
 
@@ -905,8 +912,8 @@ static void lcd_clear_fast_s(void)
 {
int pos;
 
-   lcd_addr_x = 0;
-   lcd_addr_y = 0;
+   lcd.addr.x = 0;
+   lcd.addr.y = 0;
lcd_gotoxy();
 
spin_lock_irq(&pprt_lock);
@@ -918,8 +925,8 @@ static void lcd_clear_fast_s(void)
}
spin_unlock_irq(&pprt_lock);
 
-   lcd_addr_x = 0;
-   lcd_addr_y = 0;
+   lcd.addr.x = 0;
+   lcd.addr.y = 0;
lcd_gotoxy();
 }
 
@@ -928,8 +935,8 @@ static void lcd_clear_fast_p8(void)
 {
int pos;
 
-   lcd_addr_x = 0;
-   lcd_addr_y = 0;
+   lcd.addr.x = 0;
+   lcd.addr.y = 0;
lcd_gotoxy();
 
spin_lock_irq(&pprt_lock);
@@ -956,8 +963,8 @@ static void lcd_clear_fast_p8(void)
}
spin_unlock_irq(&pprt_lock);
 
-   lcd_addr_x = 0;
-   lcd_addr_y = 0;
+   lcd.addr.x = 0;
+   lcd.addr.y = 0;
lcd_gotoxy();
 }
 
@@ -966,8 +973,8 @@ static void lcd_clear_fast_tilcd(void)
 {
int pos;
 
-   lcd_addr_x = 0;
-   lcd_addr_y = 0;
+   lcd.addr.x = 0;
+   lcd.addr.y = 0;
lcd_gotoxy();
 
spin_lock_irq(&pprt_lock);
@@ -979,8 +986,8 @@ static void lcd_clear_fast_tilcd(void)
 
spin_unlock_irq(&pprt_lock);
 
-   lcd_addr_x = 0;
-   lcd_addr_y = 0;
+   lcd.addr.x = 0;
+   lcd.addr.y = 0;
lcd_gotoxy();
 }
 
@@ -988,15 +995,15 @@ static void lcd_clear_fast_tilcd(void)
 static void lcd_clear_display(void)
 {
lcd_

[PATCH v2 2/9] staging: panel: Call init function directly

2014-11-19 Thread Mariusz Gorski
Remove useless function and let the kernel call the actual
init function directly.

Signed-off-by: Mariusz Gorski 
Acked-by: Willy Tarreau 
---
 drivers/staging/panel/panel.c | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c
index 3dd318a..0d3f09e 100644
--- a/drivers/staging/panel/panel.c
+++ b/drivers/staging/panel/panel.c
@@ -,7 +,7 @@ static struct parport_driver panel_driver = {
 };
 
 /* init function */
-static int panel_init(void)
+static int __init panel_init_module(void)
 {
/* for backwards compatibility */
if (keypad_type < 0)
@@ -2334,11 +2334,6 @@ static int panel_init(void)
return 0;
 }
 
-static int __init panel_init_module(void)
-{
-   return panel_init();
-}
-
 static void __exit panel_cleanup_module(void)
 {
unregister_reboot_notifier(&panel_notifier);
-- 
2.1.3

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


[PATCH v2 8/9] staging: panel: Remove more magic number comparison

2014-11-19 Thread Mariusz Gorski
Use a defined value instead of magic number comparison
for checking whether a module param value has been set.

Signed-off-by: Mariusz Gorski 
---
v2: Don't introduce new macros for param value check

 drivers/staging/panel/panel.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c
index 0bdb447..19f6767 100644
--- a/drivers/staging/panel/panel.c
+++ b/drivers/staging/panel/panel.c
@@ -1494,17 +1494,17 @@ static void lcd_init(void)
}
 
/* Overwrite with module params set on loading */
-   if (lcd_height > -1)
+   if (lcd_height != NOT_SET)
lcd.height = lcd_height;
-   if (lcd_width > -1)
+   if (lcd_width != NOT_SET)
lcd.width = lcd_width;
-   if (lcd_bwidth > -1)
+   if (lcd_bwidth != NOT_SET)
lcd.bwidth = lcd_bwidth;
-   if (lcd_hwidth > -1)
+   if (lcd_hwidth != NOT_SET)
lcd.hwidth = lcd_hwidth;
-   if (lcd_charset > -1)
+   if (lcd_charset != NOT_SET)
lcd.charset = lcd_charset;
-   if (lcd_proto > -1)
+   if (lcd_proto != NOT_SET)
lcd.proto = lcd_proto;
if (lcd_e_pin != PIN_NOT_SET)
lcd.pins.e = lcd_e_pin;
@@ -2304,16 +2304,16 @@ static int __init panel_init_module(void)
 * Overwrite selection with module param values (both keypad and lcd),
 * where the deprecated params have lower prio.
 */
-   if (keypad_enabled > -1)
+   if (keypad_enabled != NOT_SET)
selected_keypad_type = keypad_enabled;
-   if (keypad_type > -1)
+   if (keypad_type != NOT_SET)
selected_keypad_type = keypad_type;
 
keypad.enabled = (selected_keypad_type > 0);
 
-   if (lcd_enabled > -1)
+   if (lcd_enabled != NOT_SET)
selected_lcd_type = lcd_enabled;
-   if (lcd_type > -1)
+   if (lcd_type != NOT_SET)
selected_lcd_type = lcd_type;
 
lcd.enabled = (selected_lcd_type > 0);
-- 
2.1.3

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


Re: [PATCH v2 4/9] staging: panel: Use defined value or checking module params state

2014-11-19 Thread Willy Tarreau
On Wed, Nov 19, 2014 at 09:38:46PM +0100, Mariusz Gorski wrote:
> Avoid magic number and use a comparison with a defined value instead
> that checks whether module param has been set by the user to some
> value at loading time.
> 
> Signed-off-by: Mariusz Gorski 

Acked-by: Willy Tarreau 

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


Re: [PATCH v2 6/9] staging: panel: Make two more module params read-only

2014-11-19 Thread Willy Tarreau
On Wed, Nov 19, 2014 at 09:38:48PM +0100, Mariusz Gorski wrote:
> Make keypad_type and lcd_type module params read-only.
> This step also starts making it more clear what is
> the precedence of device params coming from different
> sources (device profile, runtime module param values etc).
> 
> Signed-off-by: Mariusz Gorski 

Acked-by: Willy Tarreau 

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


Re: [PATCH v2 8/9] staging: panel: Remove more magic number comparison

2014-11-19 Thread Willy Tarreau
On Wed, Nov 19, 2014 at 09:38:50PM +0100, Mariusz Gorski wrote:
> Use a defined value instead of magic number comparison
> for checking whether a module param value has been set.
> 
> Signed-off-by: Mariusz Gorski 

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


Re: [PATCH v2 7/9] staging: panel: Refactor LCD init code

2014-11-19 Thread Willy Tarreau
On Wed, Nov 19, 2014 at 09:38:49PM +0100, Mariusz Gorski wrote:
> Rework lcd_init method to make it a little bit more clear about
> the precedence of the params, move LCD geometry and pins layout
> to the LCD struct and thus make the LCD-related module params
> effectively read-only.
> 
> Signed-off-by: Mariusz Gorski 

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


Re: [PATCH v2 0/9] staging: panel: Refactor panel initialization

2014-11-19 Thread Willy Tarreau
Hi Mariusz,

On Wed, Nov 19, 2014 at 09:38:42PM +0100, Mariusz Gorski wrote:
> This set of patches focuses on making current initialization
> process easier to understand - especially it tries to emphasize
> what are the priorities of the params coming from different
> sources (Kconfig values, device profiles and module param values
> set on loading). I paid attention not to change to behaviour of
> the code itself (at least for now), so all hacky places are kept.
> 
> v2: Don't introduce new macros for param value check

(...)

Great work, the code is already significantly cleaner now, thanks
for doing this work! I've acked all the new patches.

Best regards,
Willy

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


[PATCH RESEND] staging: skein: fixed sparse warnings related to static declarations

2014-11-19 Thread Niklas Svensson
drivers/staging/skein/skein_generic.c:30:5: warning: symbol 'skein256_update' 
was not declared. Should it be static?
drivers/staging/skein/skein_generic.c:65:5: warning: symbol 'skein512_update' 
was not declared. Should it be static?
drivers/staging/skein/skein_generic.c:100:5: warning: symbol 'skein1024_update' 
was not declared. Should it be static?

Signed-off-by: Niklas Svensson 
Acked-by: Jason Cooper 
---
 drivers/staging/skein/skein_generic.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/skein/skein_generic.c 
b/drivers/staging/skein/skein_generic.c
index 60d16b6..85bd7d0 100644
--- a/drivers/staging/skein/skein_generic.c
+++ b/drivers/staging/skein/skein_generic.c
@@ -27,7 +27,7 @@ static int skein256_init(struct shash_desc *desc)
SKEIN256_DIGEST_BIT_SIZE);
 }
 
-int skein256_update(struct shash_desc *desc, const u8 *data,
+static int skein256_update(struct shash_desc *desc, const u8 *data,
unsigned int len)
 {
return skein_256_update((struct skein_256_ctx *)shash_desc_ctx(desc),
@@ -62,7 +62,7 @@ static int skein512_init(struct shash_desc *desc)
SKEIN512_DIGEST_BIT_SIZE);
 }
 
-int skein512_update(struct shash_desc *desc, const u8 *data,
+static int skein512_update(struct shash_desc *desc, const u8 *data,
unsigned int len)
 {
return skein_512_update((struct skein_512_ctx *)shash_desc_ctx(desc),
@@ -97,7 +97,7 @@ static int skein1024_init(struct shash_desc *desc)
SKEIN1024_DIGEST_BIT_SIZE);
 }
 
-int skein1024_update(struct shash_desc *desc, const u8 *data,
+static int skein1024_update(struct shash_desc *desc, const u8 *data,
unsigned int len)
 {
return skein_1024_update((struct skein_1024_ctx *)shash_desc_ctx(desc),
-- 
2.1.0

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


RE: [PATCH] hv: hv_fcopy: drop the obsolete message on transfer failure

2014-11-19 Thread KY Srinivasan


> -Original Message-
> From: devel [mailto:driverdev-devel-boun...@linuxdriverproject.org] On
> Behalf Of Dexuan Cui
> Sent: Tuesday, November 11, 2014 9:03 PM
> To: gre...@linuxfoundation.org; linux-ker...@vger.kernel.org; driverdev-
> de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com;
> jasow...@redhat.com
> Cc: Haiyang Zhang
> Subject: [PATCH] hv: hv_fcopy: drop the obsolete message on transfer
> failure
> 
> In the case the user-space daemon crashes, hangs or is killed, we need to
> down the semaphore, otherwise, after the daemon starts next time, the
> obsolete data in fcopy_transaction.message or fcopy_transaction.fcopy_msg
> will be used immediately.
> 
> Cc: K. Y. Srinivasan 
> Signed-off-by: Dexuan Cui 
> ---
>  drivers/hv/hv_fcopy.c | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/hv/hv_fcopy.c b/drivers/hv/hv_fcopy.c index
> 23b2ce2..177122a 100644
> --- a/drivers/hv/hv_fcopy.c
> +++ b/drivers/hv/hv_fcopy.c
> @@ -86,6 +86,15 @@ static void fcopy_work_func(struct work_struct
> *dummy)
>* process the pending transaction.
>*/
>   fcopy_respond_to_host(HV_E_FAIL);
> +
> + /* In the case the user-space daemon crashes, hangs or is killed, we
> +  * need to down the semaphore, otherwise, after the daemon starts
> next
> +  * time, the obsolete data in fcopy_transaction.message or
> +  * fcopy_transaction.fcopy_msg will be used immediately.
> +  */
> + if (down_trylock(&fcopy_transaction.read_sema))
> + pr_debug("FCP: failed to acquire the semaphore\n");
> +
>  }

When the daemon is killed, we currently reset the state in the release 
function. Why can't we cleanup the semaphore state (initialize) here as well.

K. Y
> 
>  static int fcopy_handle_handshake(u32 version)
> --
> 1.9.1
> 
> ___
> devel mailing list
> de...@linuxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


RE: [PATCH v2 2/2] Tools: hv: vssdaemon: skip all filesystems mounted readonly

2014-11-19 Thread KY Srinivasan


> -Original Message-
> From: Vitaly Kuznetsov [mailto:vkuzn...@redhat.com]
> Sent: Monday, November 10, 2014 8:37 AM
> To: KY Srinivasan; Haiyang Zhang; Greg Kroah-Hartman
> Cc: de...@linuxdriverproject.org; linux-ker...@vger.kernel.org; Dexuan Cui
> Subject: [PATCH v2 2/2] Tools: hv: vssdaemon: skip all filesystems mounted
> readonly
> 
> Instead of making a list of exceptions for readonly filesystems in addition to
> iso9660 we already have it is better to skip freeze operation for all 
> readonly-
> mounted filesystems.
> 
> Signed-off-by: Vitaly Kuznetsov 
Signed-off-by: K. Y. Srinivasan 

> ---
>  tools/hv/hv_vss_daemon.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/hv/hv_vss_daemon.c b/tools/hv/hv_vss_daemon.c index
> ee44f0d..5e63f70 100644
> --- a/tools/hv/hv_vss_daemon.c
> +++ b/tools/hv/hv_vss_daemon.c
> @@ -102,7 +102,7 @@ static int vss_operate(int operation)
>   while ((ent = getmntent(mounts))) {
>   if (strncmp(ent->mnt_fsname, match, strlen(match)))
>   continue;
> - if (strcmp(ent->mnt_type, "iso9660") == 0)
> + if (hasmntopt(ent, MNTOPT_RO) != NULL)
>   continue;
>   if (strcmp(ent->mnt_type, "vfat") == 0)
>   continue;
> --
> 1.9.3

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


RE: [PATCH v2 1/2] Tools: hv: vssdaemon: report freeze errors

2014-11-19 Thread KY Srinivasan


> -Original Message-
> From: Vitaly Kuznetsov [mailto:vkuzn...@redhat.com]
> Sent: Monday, November 10, 2014 8:37 AM
> To: KY Srinivasan; Haiyang Zhang; Greg Kroah-Hartman
> Cc: de...@linuxdriverproject.org; linux-ker...@vger.kernel.org; Dexuan Cui
> Subject: [PATCH v2 1/2] Tools: hv: vssdaemon: report freeze errors
> 
> When ioctl(fd, FIFREEZE, 0) results in an error we cannot report it to syslog
> instantly since that can cause write to a frozen disk.
> However, the name of the filesystem which caused the error and errno are
> valuable and we would like to get a nice human-readable message in the log.
> Save errno before calling vss_operate(VSS_OP_THAW) and report the error
> right after.
> 
> Unfortunately, FITHAW errors cannot be reported the same way as we need
> to finish thawing all filesystems before calling syslog().
> 
> We should also avoid calling endmntent() for the second time in case we
> encountered an error during freezing of '/' as it usually results in SEGSEGV.
> 
> Signed-off-by: Vitaly Kuznetsov 
Signed-off-by: K. Y. Srinivasan 

> ---
>  tools/hv/hv_vss_daemon.c | 16 
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/hv/hv_vss_daemon.c b/tools/hv/hv_vss_daemon.c index
> b720d8f..ee44f0d 100644
> --- a/tools/hv/hv_vss_daemon.c
> +++ b/tools/hv/hv_vss_daemon.c
> @@ -82,7 +82,7 @@ static int vss_operate(int operation)
>   FILE *mounts;
>   struct mntent *ent;
>   unsigned int cmd;
> - int error = 0, root_seen = 0;
> + int error = 0, root_seen = 0, save_errno = 0;
> 
>   switch (operation) {
>   case VSS_OP_FREEZE:
> @@ -114,7 +114,6 @@ static int vss_operate(int operation)
>   if (error && operation == VSS_OP_FREEZE)
>   goto err;
>   }
> - endmntent(mounts);
> 
>   if (root_seen) {
>   error |= vss_do_freeze("/", cmd);
> @@ -122,10 +121,19 @@ static int vss_operate(int operation)
>   goto err;
>   }
> 
> - return error;
> + goto out;
>  err:
> - endmntent(mounts);
> + save_errno = errno;
>   vss_operate(VSS_OP_THAW);
> + /* Call syslog after we thaw all filesystems */
> + if (ent)
> + syslog(LOG_ERR, "FREEZE of %s failed; error:%d %s",
> +ent->mnt_dir, save_errno, strerror(save_errno));
> + else
> + syslog(LOG_ERR, "FREEZE of / failed; error:%d %s",
> save_errno,
> +strerror(save_errno));
> +out:
> + endmntent(mounts);
>   return error;
>  }
> 
> --
> 1.9.3

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


[PATCH v2] tools: hv: ignore ENOBUFS and ENOMEM in the KVP daemon

2014-11-19 Thread Dexuan Cui
Under high memory pressure and very high KVP R/W test pressure, the netlink
recvfrom() may transiently return ENOBUFS to the daemon -- we found this
during a 2-week stress test.

We'd better not terminate the daemon on the failure, because a typical KVP
user will re-try the R/W and hopefully it will succeed next time.

We can also ignore the errors on sending.

Cc: Vitaly Kuznetsov 
Cc: K. Y. Srinivasan 
Signed-off-by: Dexuan Cui 
---

v2: I also ignore the errors on sending, as Vitaly suggested.

 tools/hv/hv_kvp_daemon.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
index 22b0764..6a6432a 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -1559,8 +1559,15 @@ int main(int argc, char *argv[])
addr_p, &addr_l);
 
if (len < 0) {
+   int saved_errno = errno;
syslog(LOG_ERR, "recvfrom failed; pid:%u error:%d %s",
addr.nl_pid, errno, strerror(errno));
+
+   if (saved_errno == ENOBUFS) {
+   syslog(LOG_ERR, "receive error: ignored");
+   continue;
+   }
+
close(fd);
return -1;
}
@@ -1763,8 +1770,15 @@ kvp_done:
 
len = netlink_send(fd, incoming_cn_msg);
if (len < 0) {
+   int saved_errno = errno;
syslog(LOG_ERR, "net_link send failed; error: %d %s", 
errno,
strerror(errno));
+
+   if (saved_errno == ENOMEM || saved_errno == ENOBUFS) {
+   syslog(LOG_ERR, "send error: ignored");
+   continue;
+   }
+
exit(EXIT_FAILURE);
}
}
-- 
1.9.1

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


Re: [PATCH] staging: fwserial: remove multiple blank lines

2014-11-19 Thread Le Tan
Hi Greg,


2014-11-11 15:40 GMT+08:00 Le Tan :
> This patch fixes the multiple blank lines issue complained by checkpatch.pl
> by removing useless blank lines.
>
> Signed-off-by: Le Tan 
> ---
>  drivers/staging/fwserial/fwserial.c | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/drivers/staging/fwserial/fwserial.c 
> b/drivers/staging/fwserial/fwserial.c
> index af0c387..73deae3 100644
> --- a/drivers/staging/fwserial/fwserial.c
> +++ b/drivers/staging/fwserial/fwserial.c
> @@ -278,7 +278,6 @@ static void fwtty_send_txn_async(struct fwtty_peer *peer,
> len, fwtty_common_callback, txn);
>  }
>
> -
>  static void __fwtty_restart_tx(struct fwtty_port *port)
>  {
> int len, avail;
> @@ -508,7 +507,6 @@ static void fwtty_do_hangup(struct work_struct *work)
> tty_kref_put(tty);
>  }
>
> -
>  static void fwtty_emit_breaks(struct work_struct *work)
>  {
> struct fwtty_port *port = to_port(to_delayed_work(work), emit_breaks);
> @@ -1622,7 +1620,6 @@ static inline int mgmt_pkt_expected_len(__be16 code)
> case FWSC_VIRT_CABLE_PLUG_RSP:  /* | FWSC_RSP_OK */
> return sizeof(pkt.hdr) + sizeof(pkt.plug_rsp);
>
> -
> case FWSC_VIRT_CABLE_UNPLUG:
> case FWSC_VIRT_CABLE_UNPLUG_RSP:
> case FWSC_VIRT_CABLE_PLUG_RSP | FWSC_RSP_NACK:
> --
> 1.8.3.2
>

Is this patch OK to fix coding style issues? I sent this patch days
ago but got no reply. So I resend this to confirm it.

Thanks very much!

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


RE: [PATCH] hv: hv_fcopy: drop the obsolete message on transfer failure

2014-11-19 Thread Dexuan Cui
> -Original Message-
> From: KY Srinivasan
> Sent: Thursday, November 20, 2014 6:59 AM
> > diff --git a/drivers/hv/hv_fcopy.c b/drivers/hv/hv_fcopy.c index
> > 23b2ce2..177122a 100644
> > --- a/drivers/hv/hv_fcopy.c
> > +++ b/drivers/hv/hv_fcopy.c
> > @@ -86,6 +86,15 @@ static void fcopy_work_func(struct work_struct
> > *dummy)
> >   * process the pending transaction.
> >   */
> >  fcopy_respond_to_host(HV_E_FAIL);
> > +
> > +/* In the case the user-space daemon crashes, hangs or is killed, we
> > + * need to down the semaphore, otherwise, after the daemon starts
> > next
> > + * time, the obsolete data in fcopy_transaction.message or
> > + * fcopy_transaction.fcopy_msg will be used immediately.
> > + */
> > +if (down_trylock(&fcopy_transaction.read_sema))
> > +pr_debug("FCP: failed to acquire the semaphore\n");
> > +
> >  }
> 
> When the daemon is killed, we currently reset the state in the release
> function. Why can't we cleanup the semaphore state (initialize) here as well.
> 
> K. Y

Hi KY,
1) The down_trylock() here is necessary: the daemon can fail to respond in 5
seconds due to many reasons, e.g., the VM's CPU and I/O are too busy. In this
case, the daemon may become running later(NOTE: in this example, the daemon
is not killed), but from the host user's point of view, the PowerShell
copy-vmfile command has failed, so here we have to 'down' the semaphore
anyway, otherwise, the daemon can get obsolete data.

2) If we add a line
sema_init(&fcopy_transaction.read_sema, 0);
in fcopy_release(), it seems OK at a glance, but we have to handle the race
condition: the above down_trylock() and the sema_init() can, in theory, run
simultaneously on different virtual CPUs.  It's tricky to address this.

3) So I think we can reuse the same semaphore without an actually unnecessary
re-initialization. :-)

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