Re: [PATCH v2] staging: rtl8723au: Fixes unnecessary return warning
On Sat, Jan 30, 2016 at 4:57 PM, Bhaktipriya Shridhar
wrote:
> This patch fixes checkpatch.pl warning in rtw_mlme_ext.c file.
> WARNING: void function return statements are not generally useful
>
> Signed-off-by: Bhaktipriya Shridhar
Looks sane to me.
Reviewed-by: Julian Calaby
> ---
> Changes in v2:
>- Removed the unnecessary blank lines.
> drivers/staging/rtl8723au/core/rtw_mlme_ext.c | 20
> 1 file changed, 20 deletions(-)
>
> diff --git a/drivers/staging/rtl8723au/core/rtw_mlme_ext.c
> b/drivers/staging/rtl8723au/core/rtw_mlme_ext.c
> index d28f29a..7cd0052 100644
> --- a/drivers/staging/rtl8723au/core/rtw_mlme_ext.c
> +++ b/drivers/staging/rtl8723au/core/rtw_mlme_ext.c
> @@ -2656,8 +2656,6 @@ static void issue_probersp(struct rtw_adapter
> *padapter, unsigned char *da)
> pattrib->last_txcmdsz = pattrib->pktlen;
>
> dump_mgntframe23a(padapter, pmgntframe);
> -
> - return;
> }
>
> static int _issue_probereq(struct rtw_adapter *padapter,
> @@ -2957,8 +2955,6 @@ static void issue_auth(struct rtw_adapter *padapter,
> struct sta_info *psta,
> rtw_wep_encrypt23a(padapter, pmgntframe);
> DBG_8723A("%s\n", __func__);
> dump_mgntframe23a(padapter, pmgntframe);
> -
> - return;
> }
>
> #ifdef CONFIG_8723AU_AP_MODE
> @@ -3338,8 +3334,6 @@ exit:
> }
> } else
> kfree(pmlmepriv->assoc_req);
> -
> - return;
> }
>
> /* when wait_ack is true, this function should be called at process context
> */
> @@ -4102,8 +4096,6 @@ static void rtw_site_survey(struct rtw_adapter
> *padapter)
> pmlmeext->chan_scan_time = SURVEY_TO;
> pmlmeext->sitesurvey_res.state = SCAN_DISABLE;
> }
> -
> - return;
> }
>
> /* collect bss info from Beacon and Probe request/response frames. */
> @@ -4759,8 +4751,6 @@ void report_survey_event23a(struct rtw_adapter
> *padapter,
> rtw_enqueue_cmd23a(pcmdpriv, pcmd_obj);
>
> pmlmeext->sitesurvey_res.bss_cnt++;
> -
> - return;
> }
>
> void report_surveydone_event23a(struct rtw_adapter *padapter)
> @@ -4802,8 +4792,6 @@ void report_surveydone_event23a(struct rtw_adapter
> *padapter)
> DBG_8723A("survey done event(%x)\n", psurveydone_evt->bss_cnt);
>
> rtw_enqueue_cmd23a(pcmdpriv, pcmd_obj);
> -
> - return;
> }
>
> void report_join_res23a(struct rtw_adapter *padapter, int res)
> @@ -4850,8 +4838,6 @@ void report_join_res23a(struct rtw_adapter *padapter,
> int res)
> rtw_joinbss_event_prehandle23a(padapter, (u8
> *)&pjoinbss_evt->network);
>
> rtw_enqueue_cmd23a(pcmdpriv, pcmd_obj);
> -
> - return;
> }
>
> void report_del_sta_event23a(struct rtw_adapter *padapter,
> @@ -4906,8 +4892,6 @@ void report_del_sta_event23a(struct rtw_adapter
> *padapter,
> DBG_8723A("report_del_sta_event23a: delete STA, mac_id =%d\n",
> mac_id);
>
> rtw_enqueue_cmd23a(pcmdpriv, pcmd_obj);
> -
> - return;
> }
>
> void report_add_sta_event23a(struct rtw_adapter *padapter,
> @@ -4951,8 +4935,6 @@ void report_add_sta_event23a(struct rtw_adapter
> *padapter,
> DBG_8723A("report_add_sta_event23a: add STA\n");
>
> rtw_enqueue_cmd23a(pcmdpriv, pcmd_obj);
> -
> - return;
> }
>
> /
> @@ -5394,8 +5376,6 @@ static void link_timer_hdl(unsigned long data)
> issue_assocreq(padapter);
> set_link_timer(pmlmeext, REASSOC_TO);
> }
> -
> - return;
> }
>
> static void addba_timer_hdl(unsigned long data)
> --
> 2.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Julian Calaby
Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8723au: Fixes unnecessary return warning
Hi Bhakti,
On Sat, Jan 30, 2016 at 5:53 PM, Bhakti Priya wrote:
> Hi,
>
> Thank you for your reply. I've just sent version 2 of the patch with
> the blank lines removed.
> I will be happy to extend checkpatch.pl. As suggested by you, I am
> trying to detect such blank lines in a line removal patch by checking
> if the line above the deleted line was a blank line and the line
> following the deleted line had a closing brace.
> Can you please guide me and let me know if I am headed in the right direction.
As I understand it, the algorithm needs to work like this:
1. For each patch hunk:
2. Filter out all lines that match /^-/
3. Remove the first character (" " or "+")
4. Normalise EOL characters: s/\r\n?/\n/
5. Over the entire hunk, find any case that matches
/({|\n)\s*\n\s*(\n|})/ where \s matches all space characters except
\n.
6. Report the middle line the preceding regular expression matches to the user.
I'm confident I can write it as a shell script, but I don't know
enough Perl to add that test to checkpatch.pl
Thanks,
--
Julian Calaby
Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8723au: Fixes unnecessary return warning
On Sat, 2016-01-30 at 23:02 +1100, Julian Calaby wrote:
> Hi Bhakti,
>
> On Sat, Jan 30, 2016 at 5:53 PM, Bhakti Priya wrote:
> > Hi,
> >
> > Thank you for your reply. I've just sent version 2 of the patch with
> > the blank lines removed.
> > I will be happy to extend checkpatch.pl. As suggested by you, I am
> > trying to detect such blank lines in a line removal patch by checking
> > if the line above the deleted line was a blank line and the line
> > following the deleted line had a closing brace.
> > Can you please guide me and let me know if I am headed in the right
> > direction.
>
> As I understand it, the algorithm needs to work like this:
> 1. For each patch hunk:
> 2. Filter out all lines that match /^-/
> 3. Remove the first character (" " or "+")
> 4. Normalise EOL characters: s/\r\n?/\n/
> 5. Over the entire hunk, find any case that matches
> /({|\n)\s*\n\s*(\n|})/ where \s matches all space characters except
> \n.
> 6. Report the middle line the preceding regular expression matches to the
> user.
>
> I'm confident I can write it as a shell script, but I don't know
> enough Perl to add that test to checkpatch.pl
That's basically what the $prevline variable in checkpatch does.
Likely it's enough to check that.
Perhaps Andy Whitcroft knows.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/2] staging:iio:adc:added space around '-'
On 24/01/16 17:14, Lars-Peter Clausen wrote: > On 01/24/2016 05:36 PM, Jonathan Cameron wrote: >> On 20/01/16 14:21, Dan Carpenter wrote: >>> On Fri, Jan 15, 2016 at 09:15:52PM +0100, Lars-Peter Clausen wrote: On 01/15/2016 08:42 PM, Bhumika Goyal wrote: > This patch adds apace around '-' operator.Found using checkpatch.pl > > Signed-off-by: Bhumika Goyal > --- > drivers/staging/iio/adc/ad7280a.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/staging/iio/adc/ad7280a.c > b/drivers/staging/iio/adc/ad7280a.c > index f45ebed..0c73bce 100644 > --- a/drivers/staging/iio/adc/ad7280a.c > +++ b/drivers/staging/iio/adc/ad7280a.c > @@ -744,14 +744,14 @@ out: > } > > static IIO_DEVICE_ATTR_NAMED(in_thresh_low_value, > - in_voltage-voltage_thresh_low_value, > + in_voltage - voltage_thresh_low_value, Hi, Thanks for patch. But when sending cleanup patches like this please make sure that you a) understand what the code does and how your change affects it and b) as a bare minimum of testing perform a compile test, if possible also do functional testing. The patch as it is, is neither semantically nor syntactically correct. As an exercise please make sure you understand why. >>> >>> Ugh! >>> >>> It took me a long time to figure out the bug in this patch... Why does >>> that filename have a mix of dashes and underscores? Too late to fix it >>> now... :/ >>> >> Very deliberately. The - is indicating it is a differential channel! >> Literally A minus B. >> >> It's an awfully compact representation for maths ;) >> This is obscured partly in this case as it's specifying an attribute >> shared by a set of differential channels so it's the generalization >> of >> in_voltage0-voltage1_thresh_low_value >> which does begin to slightly stretch the argument that it is nice and >> clear ;( > > One thing we should maybe take a look at is making it explicit that this is > a string so it does not get picked up by checkpatch. Make sense. Patches welcome :) > > -- > To unsubscribe from this list: send the line "unsubscribe linux-iio" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: iio: ad5933: avoid uninitialized variable in error case
On 25/01/16 15:50, Arnd Bergmann wrote:
> The ad5933_i2c_read function returns an error code to indicate
> whether it could read data or not. However ad5933_work() ignores
> this return code and just accesses the data unconditionally,
> which gets detected by gcc as a possible bug:
>
> drivers/staging/iio/impedance-analyzer/ad5933.c: In function 'ad5933_work':
> drivers/staging/iio/impedance-analyzer/ad5933.c:649:16: warning: 'status' may
> be used uninitialized in this function [-Wmaybe-uninitialized]
>
> This adds minimal error handling so we only evaluate the
> data if it was correctly read.
>
> Signed-off-by: Arnd Bergmann
Hi Arnd,
Thanks for the patch. The handling in here is a little fiddly
by the look of things. Lars can you take a look at this when
you have a minute?
At a very high level, it doesn't make sense to fix this instance and
not the one in the context of the patch below.
See below...
> ---
> drivers/staging/iio/impedance-analyzer/ad5933.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c
> b/drivers/staging/iio/impedance-analyzer/ad5933.c
> index 10c43dda0f5a..304bb464e478 100644
> --- a/drivers/staging/iio/impedance-analyzer/ad5933.c
> +++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
> @@ -647,6 +647,7 @@ static void ad5933_work(struct work_struct *work)
> __be16 buf[2];
> int val[2];
> unsigned char status;
> + int ret;
>
> mutex_lock(&indio_dev->mlock);
> if (st->state == AD5933_CTRL_INIT_START_FREQ) {
> @@ -658,9 +659,9 @@ static void ad5933_work(struct work_struct *work)
> return;
> }
>
> - ad5933_i2c_read(st->client, AD5933_REG_STATUS, 1, &status);
> + ret = ad5933_i2c_read(st->client, AD5933_REG_STATUS, 1, &status);
>
> - if (status & AD5933_STAT_DATA_VALID) {
> + if (!ret && (status & AD5933_STAT_DATA_VALID)) {
The else is non trivial here as it assumes we will get the data later. If we
get such a failure, we probably want to drop out completely rather than paper
over the gaps..
> int scan_count = bitmap_weight(indio_dev->active_scan_mask,
> indio_dev->masklength);
Same issue on the next line - this results in known garbage data being spooled
out.
> ad5933_i2c_read(st->client,
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] iio: ade7753: avoid uninitialized data
On 25/01/16 15:52, Arnd Bergmann wrote:
> The ade7753_spi_read_reg_16() will either successfully read a value
> from SPI, or return a failure code without delivering data. However,
> the ade7753_stop_device() and ade7753_reset() functions use the returned
> data without checking for an error condition first. Gcc detects this
> as a possible bug and warns about it:
>
> drivers/staging/iio/meter/ade7753.c: In function 'ade7753_remove':
> drivers/staging/iio/meter/ade7753.c:348:6: error: 'val' may be used
> uninitialized in this function [-Werror=maybe-uninitialized]
> val |= BIT(4); /* AD converters can be turned off */
> ^
> drivers/staging/iio/meter/ade7753.c:345:6: note: 'val' was declared here
> u16 val;
> ^
> drivers/staging/iio/meter/ade7753.c: In function 'ade7753_probe':
> drivers/staging/iio/meter/ade7753.c:222:6: error: 'val' may be used
> uninitialized in this function [-Werror=maybe-uninitialized]
>
> In both cases, we can avoids the warning by checking the return code
> before using the data.
>
> Signed-off-by: Arnd Bergmann
Thanks.
Applied to the togreg branch of iio.git - initially pushed out as testing for
the autobuilders to play with it.
Thanks,
Jonathan
> ---
> drivers/staging/iio/meter/ade7753.c | 12 ++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/iio/meter/ade7753.c
> b/drivers/staging/iio/meter/ade7753.c
> index f129039bece3..69287108f793 100644
> --- a/drivers/staging/iio/meter/ade7753.c
> +++ b/drivers/staging/iio/meter/ade7753.c
> @@ -217,8 +217,12 @@ error_ret:
> static int ade7753_reset(struct device *dev)
> {
> u16 val;
> + int ret;
> +
> + ret = ade7753_spi_read_reg_16(dev, ADE7753_MODE, &val);
> + if (ret)
> + return ret;
>
> - ade7753_spi_read_reg_16(dev, ADE7753_MODE, &val);
> val |= BIT(6); /* Software Chip Reset */
>
> return ade7753_spi_write_reg_16(dev, ADE7753_MODE, val);
> @@ -343,8 +347,12 @@ error_ret:
> static int ade7753_stop_device(struct device *dev)
> {
> u16 val;
> + int ret;
> +
> + ret = ade7753_spi_read_reg_16(dev, ADE7753_MODE, &val);
> + if (ret)
> + return ret;
>
> - ade7753_spi_read_reg_16(dev, ADE7753_MODE, &val);
> val |= BIT(4); /* AD converters can be turned off */
>
> return ade7753_spi_write_reg_16(dev, ADE7753_MODE, val);
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[patch] staging: rtl8712: memory corruption in wpa_set_encryption()
->KeyMaterial is declared as a 16 byte array, but we only ever allocate
either 5 or 13 bytes of it. The problem is that we memset() all 16
bytes to zero so we're memsetting past the end of the allocated memory.
I fixed this in slightly lazy way, by just allocating 16 bytes. This
works but there is a lot more cleanup you could do to this code if you
wanted. Which is why this code is in staging.
Signed-off-by: Dan Carpenter
diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
index edfc680..db2e31bc 100644
--- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
+++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
@@ -398,12 +398,9 @@ static int wpa_set_encryption(struct net_device *dev,
struct ieee_param *param,
wep_key_idx = 0;
if (wep_key_len > 0) {
wep_key_len = wep_key_len <= 5 ? 5 : 13;
- pwep = kmalloc((u32)(wep_key_len +
- FIELD_OFFSET(struct NDIS_802_11_WEP,
- KeyMaterial)), GFP_ATOMIC);
+ pwep = kzalloc(sizeof(*pwep), GFP_ATOMIC);
if (pwep == NULL)
return -ENOMEM;
- memset(pwep, 0, sizeof(struct NDIS_802_11_WEP));
pwep->KeyLength = wep_key_len;
pwep->Length = wep_key_len +
FIELD_OFFSET(struct NDIS_802_11_WEP,
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH resend] staging: rts5208: Remove unnecessary synchronize_irq() before free_irq()
Calling synchronize_irq() right before free_irq() is quite useless. On one
hand the IRQ can easily fire again before free_irq() is entered, on the
other hand free_irq() itself calls synchronize_irq() internally (in a race
condition free way), before any state associated with the IRQ is freed.
Patch was generated using the following semantic patch:
//
@@
expression irq;
@@
-synchronize_irq(irq);
free_irq(irq, ...);
//
Signed-off-by: Lars-Peter Clausen
---
drivers/staging/rts5208/rtsx.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/staging/rts5208/rtsx.c b/drivers/staging/rts5208/rtsx.c
index 1fe8e3e..5dfcdfb 100644
--- a/drivers/staging/rts5208/rtsx.c
+++ b/drivers/staging/rts5208/rtsx.c
@@ -320,7 +320,6 @@ static int rtsx_suspend(struct pci_dev *pci, pm_message_t
state)
rtsx_do_before_power_down(chip, PM_S3);
if (dev->irq >= 0) {
- synchronize_irq(dev->irq);
free_irq(dev->irq, (void *)dev);
dev->irq = -1;
}
@@ -398,7 +397,6 @@ static void rtsx_shutdown(struct pci_dev *pci)
rtsx_do_before_power_down(chip, PM_S1);
if (dev->irq >= 0) {
- synchronize_irq(dev->irq);
free_irq(dev->irq, (void *)dev);
dev->irq = -1;
}
--
2.1.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: iio: ad5933: avoid uninitialized variable in error case
On 01/30/2016 03:18 PM, Jonathan Cameron wrote:
> On 25/01/16 15:50, Arnd Bergmann wrote:
>> The ad5933_i2c_read function returns an error code to indicate
>> whether it could read data or not. However ad5933_work() ignores
>> this return code and just accesses the data unconditionally,
>> which gets detected by gcc as a possible bug:
>>
>> drivers/staging/iio/impedance-analyzer/ad5933.c: In function 'ad5933_work':
>> drivers/staging/iio/impedance-analyzer/ad5933.c:649:16: warning: 'status'
>> may be used uninitialized in this function [-Wmaybe-uninitialized]
>>
>> This adds minimal error handling so we only evaluate the
>> data if it was correctly read.
>>
>> Signed-off-by: Arnd Bergmann
> Hi Arnd,
>
> Thanks for the patch. The handling in here is a little fiddly
> by the look of things. Lars can you take a look at this when
> you have a minute?
>
> At a very high level, it doesn't make sense to fix this instance and
> not the one in the context of the patch below.
> See below...
>> ---
>> drivers/staging/iio/impedance-analyzer/ad5933.c | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c
>> b/drivers/staging/iio/impedance-analyzer/ad5933.c
>> index 10c43dda0f5a..304bb464e478 100644
>> --- a/drivers/staging/iio/impedance-analyzer/ad5933.c
>> +++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
>> @@ -647,6 +647,7 @@ static void ad5933_work(struct work_struct *work)
>> __be16 buf[2];
>> int val[2];
>> unsigned char status;
>> +int ret;
>>
>> mutex_lock(&indio_dev->mlock);
>> if (st->state == AD5933_CTRL_INIT_START_FREQ) {
>> @@ -658,9 +659,9 @@ static void ad5933_work(struct work_struct *work)
>> return;
>> }
>>
>> -ad5933_i2c_read(st->client, AD5933_REG_STATUS, 1, &status);
>> +ret = ad5933_i2c_read(st->client, AD5933_REG_STATUS, 1, &status);
>>
>> -if (status & AD5933_STAT_DATA_VALID) {
>> +if (!ret && (status & AD5933_STAT_DATA_VALID)) {
> The else is non trivial here as it assumes we will get the data later. If we
> get such a failure, we probably want to drop out completely rather than paper
> over the gaps..
I agree. Although we could argue that Arnd's approach allows to recover from
temporary failure. But then again we don't want to keep polling forever if
it's a permanent failure. I'd say the best thing for a quick fix is to just
error out and assume the error is permanent.
>> int scan_count = bitmap_weight(indio_dev->active_scan_mask,
>> indio_dev->masklength);
> Same issue on the next line - this results in known garbage data being spooled
> out.
Also agreed.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/2] staging:iio:adc:added space around '-'
We could make checkpatch.pl not complain if the line says checkpatch: on it. It would look like this. - in_voltage-voltage_thresh_low_value, + in_voltage-voltage_thresh_low_value, /* checkpatch: not math */ I suppose I could have made the explanation longer since the it won't complain about the 80 character limit... What do you guys think? regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/2] staging:iio:adc:added space around '-'
On 01/30/2016 04:12 PM, Dan Carpenter wrote: > We could make checkpatch.pl not complain if the line says checkpatch: on > it. It would look like this. > > - in_voltage-voltage_thresh_low_value, > + in_voltage-voltage_thresh_low_value, /* checkpatch: not math > */ > > I suppose I could have made the explanation longer since the it won't > complain about the 80 character limit... What do you guys think? We could add it as a temporary way to silence the checker. But it feels a bit ugly, there is really no reason why this shouldn't be a string, other than that the current device attr macros don't support that. - Lars ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 04/22] staging: iio: Fix dependencies for !HAS_IOMEM archs
On 26/01/16 10:59, Geert Uytterhoeven wrote:
> On Mon, Jan 25, 2016 at 11:24 PM, Richard Weinberger wrote:
>> Not every arch has io memory.
>> So, unbreak the build by fixing the dependencies.
>>
>> Signed-off-by: Richard Weinberger
>
> Acked-by: Geert Uytterhoeven
Applied to the same obscure branch of iio.git as patch 6.
Will unwind this cross merge window mess in my tree sometime
in the next week or so.
Thanks,
Jonathan
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
> ge...@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like
> that.
> -- Linus Torvalds
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 06/10] staging/android: turn fence_info into a __64 pointer
Hi Gustavo,
s/__64/__u64/ in the commit message.
On 29 January 2016 at 23:20, Gustavo Padovan wrote:
> From: Gustavo Padovan
>
> Making fence_info a pointer enables us to extend the struct in the future
> without breaking the ABI.
>
> Signed-off-by: Gustavo Padovan
> diff --git a/drivers/staging/android/uapi/sync.h
> b/drivers/staging/android/uapi/sync.h
> index ed281fc..9f07aa7 100644
> --- a/drivers/staging/android/uapi/sync.h
> +++ b/drivers/staging/android/uapi/sync.h
> @@ -54,7 +54,7 @@ struct sync_file_info {
> charname[32];
> __s32 status;
>
> - __u8fence_info[0];
> + __u64 *fence_info;
I believe you misinterpreted what was meant with "_u64 pointer". As
the storage use for of a pointer varies across 32/64 bit arch, we
explicitly use a variable type that is large enough (and consistent)
for both cases. Thus the above should be
+ __u64 fence_info;
Hope this clarifies things.
-Emil
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 09/10] staging/android: add flags member to sync ioctl structs
Hi Gustavo,
> @@ -54,6 +59,7 @@ struct sync_file_info {
> __u32 len;
As mentioned previously - can we rework this variable to indicate the
total length (or the number) of fence_info struct instances. It seems
to be the more common approach afaict. Might also want to move it just
above the fence_into?
-Emil
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Staging: clocking-wizard: Avoid CamelCase
This patch fixes the checkpatch.pl issue: CHECK: Avoid CamelCase: CHECK: Avoid CamelCase: Signed-off-by: SirnamSwetha --- drivers/staging/clocking-wizard/clk-xlnx-clock-wizard.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/staging/clocking-wizard/clk-xlnx-clock-wizard.c b/drivers/staging/clocking-wizard/clk-xlnx-clock-wizard.c index b8e2f61..72c79aa 100644 --- a/drivers/staging/clocking-wizard/clk-xlnx-clock-wizard.c +++ b/drivers/staging/clocking-wizard/clk-xlnx-clock-wizard.c @@ -32,8 +32,8 @@ #define WZRD_CLK_CFG_REG(n)(0x200 + 4 * (n)) -#define WZRD_CLkOUT0_FRAC_EN BIT(18) -#define WZRD_CLkFBOUT_FRAC_EN BIT(26) +#define WZRD_CLKOUT0_FRAC_EN BIT(18) +#define WZRD_CLKFBOUT_FRAC_EN BIT(26) #define WZRD_CLKFBOUT_MULT_SHIFT 8 #define WZRD_CLKFBOUT_MULT_MASK(0xff << WZRD_CLKFBOUT_MULT_SHIFT) @@ -195,9 +195,9 @@ static int clk_wzrd_probe(struct platform_device *pdev) /* we don't support fractional div/mul yet */ reg = readl(clk_wzrd->base + WZRD_CLK_CFG_REG(0)) & - WZRD_CLkFBOUT_FRAC_EN; + WZRD_CLKFBOUT_FRAC_EN; reg |= readl(clk_wzrd->base + WZRD_CLK_CFG_REG(2)) & -WZRD_CLkOUT0_FRAC_EN; +WZRD_CLKOUT0_FRAC_EN; if (reg) dev_warn(&pdev->dev, "fractional div/mul not supported\n"); -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Staging: clocking-wizard: CHECK:Please use a blank line
This patch fixes the checkpatch.pl issue:
CHECK: Please use a blank line after function/struct/union/enum declarations
Signed-off-by: SirnamSwetha
---
drivers/staging/clocking-wizard/clk-xlnx-clock-wizard.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/staging/clocking-wizard/clk-xlnx-clock-wizard.c
b/drivers/staging/clocking-wizard/clk-xlnx-clock-wizard.c
index 72c79aa..7b8be52 100644
--- a/drivers/staging/clocking-wizard/clk-xlnx-clock-wizard.c
+++ b/drivers/staging/clocking-wizard/clk-xlnx-clock-wizard.c
@@ -71,6 +71,7 @@ struct clk_wzrd {
int speed_grade;
bool suspended;
};
+
#define to_clk_wzrd(_nb) container_of(_nb, struct clk_wzrd, nb)
/* maximum frequencies for input/output clocks per speed grade */
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/2] staging: xgifb: XGI_main_26.c fixed spacing to match coding style
From: madiraju
Removed unnecessary spaces to match coding style.
Signed-off-by: Santosh Madiraju
---
drivers/staging/xgifb/XGI_main_26.c | 22 ++
1 file changed, 6 insertions(+), 16 deletions(-)
diff --git a/drivers/staging/xgifb/XGI_main_26.c
b/drivers/staging/xgifb/XGI_main_26.c
index 89f5b55..4978737 100644
--- a/drivers/staging/xgifb/XGI_main_26.c
+++ b/drivers/staging/xgifb/XGI_main_26.c
@@ -1989,7 +1989,6 @@ static int xgifb_probe(struct pci_dev *pdev,
&fb_info->var.vsync_len,
&fb_info->var.sync,
&fb_info->var.vmode)) {
-
if ((fb_info->var.vmode & FB_VMODE_MASK) ==
FB_VMODE_INTERLACED) {
fb_info->var.yres <<= 1;
@@ -1999,10 +1998,7 @@ static int xgifb_probe(struct pci_dev *pdev,
fb_info->var.pixclock >>= 1;
fb_info->var.yres >>= 1;
fb_info->var.yres_virtual >>= 1;
- }
-
- }
-
+ } }
fb_info->flags = FBINFO_FLAG_DEFAULT;
fb_info->screen_base = xgifb_info->video_vbase;
fb_info->fbops = &XGIfb_ops;
@@ -2064,33 +2060,27 @@ static struct pci_driver xgifb_driver = {
.remove = xgifb_remove
};
-
-
/*/
/* MODULE */
/*/
module_param(mode, charp, 0);
-MODULE_PARM_DESC(mode,
- "Selects the desired default display mode in the format XxYxDepth (eg.
1024x768x16).");
+MODULE_PARM_DESC(mode, "Selects the desired default display mode in the format
XxYxDepth (eg. 1024x768x16).");
module_param(forcecrt2type, charp, 0);
-MODULE_PARM_DESC(forcecrt2type,
- "Force the second display output type. Possible values are NONE, LCD,
TV, VGA, SVIDEO or COMPOSITE.");
+MODULE_PARM_DESC(forcecrt2type, "Force the second display output type.
Possible values are NONE, LCD, TV, VGA, SVIDEO or COMPOSITE.");
module_param(vesa, int, 0);
-MODULE_PARM_DESC(vesa,
- "Selects the desired default display mode by VESA mode number (eg.
0x117).");
+MODULE_PARM_DESC(vesa, "Selects the desired default display mode by VESA mode
number (eg. 0x117).");
module_param(filter, int, 0);
-MODULE_PARM_DESC(filter,
- "Selects TV flicker filter type (only for systems with a SiS301 video
bridge). Possible values 0-7. Default: [no filter]).");
+MODULE_PARM_DESC(filter, "Selects TV flicker filter type (only for systems
with a SiS301 video bridge). Possible values 0-7. Default: [no filter]).");
static int __init xgifb_init(void)
{
char *option = NULL;
- if (forcecrt2type != NULL)
+ if (!forcecrt2type)
XGIfb_search_crt2type(forcecrt2type);
if (fb_get_options("xgifb", &option))
return -ENODEV;
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/2] staging:iio:adc:added space around '-'
On Sat, 2016-01-30 at 18:12 +0300, Dan Carpenter wrote:
> We could make checkpatch.pl not complain if the line says checkpatch:
> on
> it. It would look like this.
>
> - in_voltage-voltage_thresh_low_value,
> + in_voltage-voltage_thresh_low_value, /* checkpatch:
> not math */
>
> I suppose I could have made the explanation longer since the it won't
> complain about the 80 character limit... What do yo/u guys think?
Maybe use a more generic thing like the checkpatch type
in_voltage-voltage_thresh_low_value, /* checkpatch-SPACING */
Even so, it might uglify checkpatch code a lot to
check something like this per-line or per-block.
And that likely would have to be per line in the
code as checkpatch couldn't see when a patch block
addition occurs outside the scope of a comment.
I suppose inside checkpatch the "sub report {"
function could be extended to look at the specific
$rawline being tested for any "checkpatch" comment
and if so, test if it's the specific $type.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch] staging: rtl8712: memory corruption in wpa_set_encryption()
On Saturday, January 30, 2016 05:41:10 PM Dan Carpenter wrote:
> ->KeyMaterial is declared as a 16 byte array, but we only ever allocate
> either 5 or 13 bytes of it. The problem is that we memset() all 16
> bytes to zero so we're memsetting past the end of the allocated memory.
>
> I fixed this in slightly lazy way, by just allocating 16 bytes. This
> works but there is a lot more cleanup you could do to this code if you
> wanted. Which is why this code is in staging.
Better in every way than a crazy variable alloc if you ask me.
>
> Signed-off-by: Dan Carpenter
>
> diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> index edfc680..db2e31bc 100644
> --- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> +++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> @@ -398,12 +398,9 @@ static int wpa_set_encryption(struct net_device *dev,
> struct ieee_param *param,
> wep_key_idx = 0;
> if (wep_key_len > 0) {
> wep_key_len = wep_key_len <= 5 ? 5 : 13;
> - pwep = kmalloc((u32)(wep_key_len +
> - FIELD_OFFSET(struct NDIS_802_11_WEP,
> - KeyMaterial)), GFP_ATOMIC);
> + pwep = kzalloc(sizeof(*pwep), GFP_ATOMIC);
> if (pwep == NULL)
> return -ENOMEM;
> - memset(pwep, 0, sizeof(struct NDIS_802_11_WEP));
Should there be a newline after the "if" statement?
> pwep->KeyLength = wep_key_len;
> pwep->Length = wep_key_len +
>FIELD_OFFSET(struct NDIS_802_11_WEP,
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
