lkml.org 
[lkml]   [2016]   [Jul]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH 2/3] staging/rtl8192e: use s8 instead of char
    Date
    On Tuesday, July 19, 2016 12:05:00 PM CEST Jes Sorensen wrote:
    > Arnd Bergmann <arnd@arndb.de> writes:
    > > On Tuesday, July 19, 2016 11:46:04 AM CEST Jes Sorensen wrote:
    > >> > diff --git a/drivers/staging/rtl8192e/rtl819x_TSProc.c b/drivers/staging/rtl8192e/rtl819x_TSProc.c
    > >> > index 2c8a526773ed..e0a2fe5e6148 100644
    > >> > --- a/drivers/staging/rtl8192e/rtl819x_TSProc.c
    > >> > +++ b/drivers/staging/rtl8192e/rtl819x_TSProc.c
    > >> > @@ -323,7 +323,7 @@ bool GetTs(struct rtllib_device *ieee, struct ts_common_info **ppTS,
    > >> > if (ieee->current_network.qos_data.supported == 0) {
    > >> > UP = 0;
    > >> > } else {
    > >> > - if (!IsACValid(TID)) {
    > >> > + if (!IsACValid((s8)TID)) {
    > >> > netdev_warn(ieee->dev, "%s(): TID(%d) is not valid\n",
    > >> > __func__, TID);
    > >> > return false;
    > >>
    > >> TID is a 4-bit field, it should never go negative. The cast to s8 seems
    > >> wrong to me, if anything it should be using u8. I do realize the macro
    > >> IsACValid checks against negative too, but that just looks silly to me.
    > >
    > > Ok, I'll remove the extra comparison then to avoid the warning:
    > >
    > > staging/rtl8192e/rtl819x_TSProc.c:326:14: error: comparison is always
    > > true due to limited range of data type [-Werror=type-limits]
    > >
    > > I guess it should be a separate patch. I had just stumbled over the
    > > same thing before resending the patch but decided not to change it
    > > to keep the patch simple.
    >
    > I think that would be better, albeit not a big issue.

    Ok, and since Kalle applied the first patch to his tree, I'm now sending
    a series of three patches that are all for Greg, which also avoids some
    possible confusion.

    > I'd like to get rid of all the drivers/staging/rtl* drivers eventually

    That would be great, yes.

    Can you clarify what the long-term plan is? I see that
    drivers/net/wireless/realtek/rtlwifi/ has most of the PCIe parts
    and one USB device (rtl8192cu/rtl8188cus) while
    drivers/net/wireless/rtl8xxx has all the USB parts including
    that one.

    Does that mean we want the staging drivers for PCIe devices
    to get merged into rtlwifi, and the remaining USB drivers to get
    replaced by r8xxxu?

    As one data point that I can provide (but you are probably
    aware of), I could never get my rtl8188cus stick to work with
    rtlwifi, but I found the older r8712u device to work fine with
    the staging/rtl8712 driver.

    Arnd

    \
     
     \ /
      Last update: 2016-07-20 11:01    [W:2.822 / U:0.160 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site