[PATCH] staging: vt6656: Use ether_addr_copy() in vnt_fill_ieee80211_rts.
Both struct ieee80211_rts and struct ieee80211_hdr defined in linux/ieee80211.h are declared as __aligned(2) so it is safe to use ether_addr_copy() instead of memcpy(). Signed-off-by: Krzysztof Adamski --- drivers/staging/vt6656/rxtx.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/staging/vt6656/rxtx.c b/drivers/staging/vt6656/rxtx.c index ea5140a..280c923 100644 --- a/drivers/staging/vt6656/rxtx.c +++ b/drivers/staging/vt6656/rxtx.c @@ -36,6 +36,7 @@ * */ +#include #include "device.h" #include "rxtx.h" #include "card.h" @@ -392,8 +393,8 @@ static int vnt_fill_ieee80211_rts(struct vnt_usb_send_context *tx_context, rts->frame_control = cpu_to_le16(IEEE80211_FTYPE_CTL | IEEE80211_STYPE_RTS); - memcpy(rts->ra, hdr->addr1, ETH_ALEN); - memcpy(rts->ta, hdr->addr2, ETH_ALEN); + ether_addr_copy(rts->ra, hdr->addr1); + ether_addr_copy(rts->ta, hdr->addr2); return 0; } -- 1.9.3 ___ 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] staging: vt6656: Use ether_addr_copy() in vnt_fill_ieee80211_rts.
On Tue, 2014-12-16 at 09:30 +0100, Krzysztof Adamski wrote: > Both struct ieee80211_rts and struct ieee80211_hdr defined in > linux/ieee80211.h are declared as __aligned(2) so it is safe to use > ether_addr_copy() instead of memcpy(). Just fyi: That the structure is declared __aligned(2) is not the important bit. What's necessary is that the members in the struct are __aligned(2). In this case, all of these members are. > diff --git a/drivers/staging/vt6656/rxtx.c b/drivers/staging/vt6656/rxtx.c [] > @@ -392,8 +393,8 @@ static int vnt_fill_ieee80211_rts(struct > vnt_usb_send_context *tx_context, > rts->frame_control = > cpu_to_le16(IEEE80211_FTYPE_CTL | IEEE80211_STYPE_RTS); > > - memcpy(rts->ra, hdr->addr1, ETH_ALEN); > - memcpy(rts->ta, hdr->addr2, ETH_ALEN); > + ether_addr_copy(rts->ra, hdr->addr1); > + ether_addr_copy(rts->ta, hdr->addr2); ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: vt6656: Use ether_addr_copy() in vnt_fill_ieee80211_rts.
On Tue, Dec 16, 2014 at 12:42:06AM -0800, Joe Perches wrote: On Tue, 2014-12-16 at 09:30 +0100, Krzysztof Adamski wrote: Both struct ieee80211_rts and struct ieee80211_hdr defined in linux/ieee80211.h are declared as __aligned(2) so it is safe to use ether_addr_copy() instead of memcpy(). Just fyi: That the structure is declared __aligned(2) is not the important bit. What's necessary is that the members in the struct are __aligned(2). In this case, all of these members are. Hi, Thank you for your feedback. Yes, you are right. It was a bit of a shortcut in my reasoning. Since struct is aligned and it's packed and both members preceding the array we want to copy are 16 bit large, the array address is guaranteed to be properly aligned. Best regards, Krzysztof Adamski ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: lustre: lustre: osc: lproc_osc.c: Fix for possible null pointer dereference
On Tue, Dec 16, 2014 at 12:07:19AM +0100, Rickard Strandqvist wrote: > Hi Dan > > Quite right! Had to try it. > > Do nothing then? > But you must agree that it is still ugly and confusing code. > Yes. I agree that it's confusing. I also suspect that "obd" is never NULL but I haven't actually looked and these things are sometimes complicated to verify. I fine with merging the patch as a cleanup. Smatch has code to not warn about these but it's not 100% correct so it still warns sometimes when it shouldn't. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: lustre: lustre: obdclass: lprocfs_status.c: Fix for possible null pointer dereference
On Mon, Dec 15, 2014 at 11:23:25PM +0100, Rickard Strandqvist wrote:
> Hi Joe
>
> No, it does not look like end can be NULL then.
> Then remove the end != NULL instead?
> ...
> if (end != NULL && *end == '.') {
>
Yes. Please do.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [GIT PULL] Staging driver patches for 3.19-rc1
Am 15.12.2014 um 19:56 schrieb Greg KH: > On Mon, Dec 15, 2014 at 10:41:03AM -0800, Greg KH wrote: >> On Mon, Dec 15, 2014 at 10:39:15AM -0800, Christoph Hellwig wrote: >>> On Mon, Dec 15, 2014 at 07:23:35PM +0100, Richard Weinberger wrote: I don't understand this kind of logic. a) Binder is considered a piece of shite. b) Google is working on a (hopefully sane) replacement. Why moving it out of staging then? What is the benefit? >>> >>> There is none, and Greg didn't even bother addressing the various >>> comments when this first came up. >> >> I thought I did, it was a long thread at the time, and I was on the road >> for 3 weeks, sorry if I missed something. >> >>> So a clear NAK from me on this one. >> >> You don't have to maintain it, I do, so why does it concern you? > > Ok, that was a bit snotty on my part, I apologize. > > But really, this is self-contained, doesn't touch any core > infrastructure, and is really just like any other driver for hardware > that people don't use. It shouldn't affect anything elsewhere in the > kernel, so objecting to it seems odd to me. Doesn't it use internal stuff from fs/file.c? Anyway, Linus pulled it. I'm just a bit astonished that binder finally sneaked into the core kernel. Hopefully no smart ass will ever decide to make some userspace component hard depend on it... Thanks, //richard ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] drivers: staging: lustre: Use mult if units not specified
On Mon, Dec 15, 2014 at 11:41:49PM -0600, Chris Rorvick wrote:
> Units can be passed to lprocfs_write_frac_u64_helper() via a suffix
> (e.g., "...K", "...M", etc.) tacked onto the value. A comment states
> that "specified units override the multiplier," though the multiplier is
> overridden regardless. Update the conditional logic so that it only
> applies when units are specified.
>
That introduces a bug. We need to take the initial '-' into
consideration. Just remove the condition. Also remove the "mult"
parameter since that is always 1.
bool negative = false;
...
if (*pbuf == '-') {
negative = true;
pbuf++;
}
...
mult = negative ? -units : units;
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: rtl8723au: core: fixing "foo * bar" should be "foo *bar"
Fixed a coding style error, "foo * bar" should be "foo *bar"
Signed-off-by: Asaf Vertz
---
drivers/staging/rtl8723au/core/rtw_efuse.c | 12 ++--
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/rtl8723au/core/rtw_efuse.c
b/drivers/staging/rtl8723au/core/rtw_efuse.c
index 81960e7..0e64d12 100644
--- a/drivers/staging/rtl8723au/core/rtw_efuse.c
+++ b/drivers/staging/rtl8723au/core/rtw_efuse.c
@@ -328,12 +328,12 @@ EFUSE_Read1Byte23a(struct rtw_adapter *Adapter, u16
Address)
void
EFUSE_Write1Byte(
- struct rtw_adapter *Adapter,
+ struct rtw_adapter *Adapter,
u16 Address,
u8 Value);
void
EFUSE_Write1Byte(
- struct rtw_adapter *Adapter,
+ struct rtw_adapter *Adapter,
u16 Address,
u8 Value)
{
@@ -636,7 +636,7 @@ Efuse_ReadAllMap(struct rtw_adapter *pAdapter, u8
efuseType, u8 *Efuse)
*---*/
static void
efuse_ShadowRead1Byte(
- struct rtw_adapter *pAdapter,
+ struct rtw_adapter *pAdapter,
u16 Offset,
u8 *Value)
{
@@ -648,7 +648,7 @@ efuse_ShadowRead1Byte(
/* Read Two Bytes */
static void
efuse_ShadowRead2Byte(
- struct rtw_adapter *pAdapter,
+ struct rtw_adapter *pAdapter,
u16 Offset,
u16 *Value)
{
@@ -661,7 +661,7 @@ efuse_ShadowRead2Byte(
/* Read Four Bytes */
static void
efuse_ShadowRead4Byte(
- struct rtw_adapter *pAdapter,
+ struct rtw_adapter *pAdapter,
u16 Offset,
u32 *Value)
{
@@ -723,7 +723,7 @@ void EFUSE_ShadowMapUpdate23a(struct rtw_adapter *pAdapter,
u8 efuseType)
*---*/
void
EFUSE_ShadowRead23a(
- struct rtw_adapter *pAdapter,
+ struct rtw_adapter *pAdapter,
u8 Type,
u16 Offset,
u32 *Value)
--
1.7.0.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] drivers: staging: lustre: Use mult if units not specified
On 2014/12/16, 2:41 AM, "Dan Carpenter" wrote:
>On Mon, Dec 15, 2014 at 11:41:49PM -0600, Chris Rorvick wrote:
>> Units can be passed to lprocfs_write_frac_u64_helper() via a suffix
>> (e.g., "...K", "...M", etc.) tacked onto the value. A comment states
>> that "specified units override the multiplier," though the multiplier is
>> overridden regardless. Update the conditional logic so that it only
>> applies when units are specified.
>>
>
>That introduces a bug. We need to take the initial '-' into
>consideration. Just remove the condition. Also remove the "mult"
>parameter since that is always 1.
>
> bool negative = false;
>
> ...
>
> if (*pbuf == '-') {
> negative = true;
> pbuf++;
> }
>
> ...
>
> mult = negative ? -units : units;
Sorry, that isn't right. Chris' patch is actually doing the right thing
to check for units > 1. The proposed change above discards "mult"
entirely, which breaks the users of this function that are not in this
file (e.g. osc_cached_mb_seq_write() or ll_max_cached_mb_seq_write())
that have tunables in units of MB by default, but can also use parameters
with units like "4.5G" for convenience.
Cheers, Andreas
--
Andreas Dilger
Lustre Software Architect
Intel High Performance Data Division
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] drivers: staging: lustre: Use mult if units not specified
On Tue, Dec 16, 2014 at 11:14:35AM +, Dilger, Andreas wrote: > > Sorry, that isn't right. Chris' patch is actually doing the right thing > to check for units > 1. It's not right because it discards the negative. > The proposed change above discards "mult" > entirely, which breaks the users of this function that are not in this > file (e.g. osc_cached_mb_seq_write() or ll_max_cached_mb_seq_write()) > that have tunables in units of MB by default, but can also use parameters > with units like "4.5G" for convenience. I think you are confusing lprocfs_write_frac_helper() and lprocfs_write_frac_u64_helper(). There is only one caller for this function. ! grep lprocfs_write_frac_u64_helper drivers/staging/lustre/ -R | grep -v smatch | grep -v '\.i:' drivers/staging/lustre/lustre/obdclass/lprocfs_status.c:return lprocfs_write_frac_u64_helper(buffer, count, val, 1); drivers/staging/lustre/lustre/obdclass/lprocfs_status.c:int lprocfs_write_frac_u64_helper(const char *buffer, unsigned long count, drivers/staging/lustre/lustre/obdclass/lprocfs_status.c:EXPORT_SYMBOL(lprocfs_write_frac_u64_helper); drivers/staging/lustre/lustre/include/lprocfs_status.h:extern int lprocfs_write_frac_u64_helper(const char *buffer, regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8723au: core: fixing "foo * bar" should be "foo *bar"
Asaf Vertz writes:
> Fixed a coding style error, "foo * bar" should be "foo *bar"
>
> Signed-off-by: Asaf Vertz
> ---
> drivers/staging/rtl8723au/core/rtw_efuse.c | 12 ++--
> 1 files changed, 6 insertions(+), 6 deletions(-)
If you want to fix the 'error' here, include it with a cleanup that also
fixes the ugly multiline arguments to the functions.
Jes
> diff --git a/drivers/staging/rtl8723au/core/rtw_efuse.c
> b/drivers/staging/rtl8723au/core/rtw_efuse.c
> index 81960e7..0e64d12 100644
> --- a/drivers/staging/rtl8723au/core/rtw_efuse.c
> +++ b/drivers/staging/rtl8723au/core/rtw_efuse.c
> @@ -328,12 +328,12 @@ EFUSE_Read1Byte23a(struct rtw_adapter *Adapter, u16
> Address)
>
> void
> EFUSE_Write1Byte(
> - struct rtw_adapter *Adapter,
> + struct rtw_adapter *Adapter,
> u16 Address,
> u8 Value);
> void
> EFUSE_Write1Byte(
> - struct rtw_adapter *Adapter,
> + struct rtw_adapter *Adapter,
> u16 Address,
> u8 Value)
> {
> @@ -636,7 +636,7 @@ Efuse_ReadAllMap(struct rtw_adapter *pAdapter, u8
> efuseType, u8 *Efuse)
>
> *---*/
> static void
> efuse_ShadowRead1Byte(
> - struct rtw_adapter *pAdapter,
> + struct rtw_adapter *pAdapter,
> u16 Offset,
> u8 *Value)
> {
> @@ -648,7 +648,7 @@ efuse_ShadowRead1Byte(
> /* Read Two Bytes */
> static void
> efuse_ShadowRead2Byte(
> - struct rtw_adapter *pAdapter,
> + struct rtw_adapter *pAdapter,
> u16 Offset,
> u16 *Value)
> {
> @@ -661,7 +661,7 @@ efuse_ShadowRead2Byte(
> /* Read Four Bytes */
> static void
> efuse_ShadowRead4Byte(
> - struct rtw_adapter *pAdapter,
> + struct rtw_adapter *pAdapter,
> u16 Offset,
> u32 *Value)
> {
> @@ -723,7 +723,7 @@ void EFUSE_ShadowMapUpdate23a(struct rtw_adapter
> *pAdapter, u8 efuseType)
>
> *---*/
> void
> EFUSE_ShadowRead23a(
> - struct rtw_adapter *pAdapter,
> + struct rtw_adapter *pAdapter,
> u8 Type,
> u16 Offset,
> u32 *Value)
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] drivers: staging: lustre: Use mult if units not specified
On Tue, Dec 16, 2014 at 5:35 AM, Dan Carpenter wrote: > On Tue, Dec 16, 2014 at 11:14:35AM +, Dilger, Andreas wrote: > > > > Sorry, that isn't right. Chris' patch is actually doing the right thing > > to check for units > 1. > > It's not right because it discards the negative. I don't think this patch introduces a bug. If anything, it was already there. It looked to me like the value passed in to `mult' was assumed to be positive and was simply being used as a flag to indicate whether `buffer' started with a '-' when units were passed. For example, say the value passed in is "-2K" and the `mult' is 1. The check for '-' will negate `mult' making it -1. Then the units conditional will override mult with `-units' (i.e., -1024.) Now say we pass "-2" with `mult' equal to 1024. The result is same, but the path is a bit different. `mult' will again be negated due to `buffer' beginning with '-', but then it will be left alone at the units check. In both of the above cases the negative sign is properly accounted for. > > The proposed change above discards "mult" > > entirely, which breaks the users of this function that are not in this > > file (e.g. osc_cached_mb_seq_write() or ll_max_cached_mb_seq_write()) > > that have tunables in units of MB by default, but can also use parameters > > with units like "4.5G" for convenience. > > I think you are confusing lprocfs_write_frac_helper() and > lprocfs_write_frac_u64_helper(). There is only one caller for this > function. By this logic, lprocfs_write_frac_u64_helper() should just be removed and it's code should be folded into lprocfs_write_u64_helper(), no? Regards, Chris ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] drivers: staging: lustre: Use mult if units not specified
On Tue, Dec 16, 2014 at 06:53:19AM -0600, Chris Rorvick wrote: > On Tue, Dec 16, 2014 at 5:35 AM, Dan Carpenter > wrote: > > On Tue, Dec 16, 2014 at 11:14:35AM +, Dilger, Andreas wrote: > > > > > > Sorry, that isn't right. Chris' patch is actually doing the right thing > > > to check for units > 1. > > > > It's not right because it discards the negative. > > I don't think this patch introduces a bug. If anything, it was already > there. The original code may be totally buggy. Who knows? Why are we passing negative numbers here anyway instead of just returning -EINVAL? But the new code is also buggy and not consistent with itself. In the original code if the user data is "-1k" or "-1024" that was treated the same. In the new code, "-1k" means negative 1024 because the user supplies units but "-1024" means positive 1024 because there are no units given. > > > The proposed change above discards "mult" > > > entirely, which breaks the users of this function that are not in this > > > file (e.g. osc_cached_mb_seq_write() or ll_max_cached_mb_seq_write()) > > > that have tunables in units of MB by default, but can also use parameters > > > with units like "4.5G" for convenience. > > > > I think you are confusing lprocfs_write_frac_helper() and > > lprocfs_write_frac_u64_helper(). There is only one caller for this > > function. > > By this logic, lprocfs_write_frac_u64_helper() should just be removed > and it's code should be folded into lprocfs_write_u64_helper(), no? > There are vast swathes of lustre code which need to be deleted but I haven't looked at this one. Probably. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] staging: rtl8723au: core: fixing "foo * bar" should be "foo *bar"
Fixed a coding style error, "foo * bar" should be "foo *bar"
Signed-off-by: Asaf Vertz
---
Changes in v2:
- fix ugly multiline arguments to the functions
drivers/staging/rtl8723au/core/rtw_efuse.c | 32 ++-
1 files changed, 7 insertions(+), 25 deletions(-)
diff --git a/drivers/staging/rtl8723au/core/rtw_efuse.c
b/drivers/staging/rtl8723au/core/rtw_efuse.c
index 81960e7..a6deddc 100644
--- a/drivers/staging/rtl8723au/core/rtw_efuse.c
+++ b/drivers/staging/rtl8723au/core/rtw_efuse.c
@@ -327,15 +327,9 @@ EFUSE_Read1Byte23a(struct rtw_adapter *Adapter, u16
Address)
*---*/
void
-EFUSE_Write1Byte(
- struct rtw_adapter *Adapter,
- u16 Address,
- u8 Value);
+EFUSE_Write1Byte(struct rtw_adapter *Adapter, u16 Address, u8 Value);
void
-EFUSE_Write1Byte(
- struct rtw_adapter *Adapter,
- u16 Address,
- u8 Value)
+EFUSE_Write1Byte(struct rtw_adapter *Adapter, u16 Address, u8 Value)
{
u8 Bytetemp = {0x00};
u8 temp = {0x00};
@@ -635,10 +629,7 @@ Efuse_ReadAllMap(struct rtw_adapter *pAdapter, u8
efuseType, u8 *Efuse)
*
*---*/
static void
-efuse_ShadowRead1Byte(
- struct rtw_adapter *pAdapter,
- u16 Offset,
- u8 *Value)
+efuse_ShadowRead1Byte(struct rtw_adapter *pAdapter, u16 Offset, u8 *Value)
{
struct eeprom_priv *pEEPROM = GET_EEPROM_EFUSE_PRIV(pAdapter);
@@ -647,10 +638,7 @@ efuse_ShadowRead1Byte(
/* Read Two Bytes */
static void
-efuse_ShadowRead2Byte(
- struct rtw_adapter *pAdapter,
- u16 Offset,
- u16 *Value)
+efuse_ShadowRead2Byte(struct rtw_adapter *pAdapter, u16 Offset, u16 *Value)
{
struct eeprom_priv *pEEPROM = GET_EEPROM_EFUSE_PRIV(pAdapter);
@@ -660,10 +648,7 @@ efuse_ShadowRead2Byte(
/* Read Four Bytes */
static void
-efuse_ShadowRead4Byte(
- struct rtw_adapter *pAdapter,
- u16 Offset,
- u32 *Value)
+efuse_ShadowRead4Byte(struct rtw_adapter *pAdapter, u16 Offset, u32 *Value)
{
struct eeprom_priv *pEEPROM = GET_EEPROM_EFUSE_PRIV(pAdapter);
@@ -722,11 +707,8 @@ void EFUSE_ShadowMapUpdate23a(struct rtw_adapter
*pAdapter, u8 efuseType)
*
*---*/
void
-EFUSE_ShadowRead23a(
- struct rtw_adapter *pAdapter,
- u8 Type,
- u16 Offset,
- u32 *Value)
+EFUSE_ShadowRead23a(struct rtw_adapter *pAdapter,
+ u8 Type, u16 Offset, u32 *Value)
{
if (Type == 1)
efuse_ShadowRead1Byte(pAdapter, Offset, (u8 *)Value);
--
1.7.0.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: rtl8723au: core: fixing "foo * bar" should be "foo *bar"
Asaf Vertz writes:
> Fixed a coding style error, "foo * bar" should be "foo *bar"
>
> Signed-off-by: Asaf Vertz
> ---
> Changes in v2:
> - fix ugly multiline arguments to the functions
>
> drivers/staging/rtl8723au/core/rtw_efuse.c | 32 ++-
> 1 files changed, 7 insertions(+), 25 deletions(-)
Signed-off-by: Jes Sorensen
> diff --git a/drivers/staging/rtl8723au/core/rtw_efuse.c
> b/drivers/staging/rtl8723au/core/rtw_efuse.c
> index 81960e7..a6deddc 100644
> --- a/drivers/staging/rtl8723au/core/rtw_efuse.c
> +++ b/drivers/staging/rtl8723au/core/rtw_efuse.c
> @@ -327,15 +327,9 @@ EFUSE_Read1Byte23a(struct rtw_adapter *Adapter, u16
> Address)
>
> *---*/
>
> void
> -EFUSE_Write1Byte(
> - struct rtw_adapter *Adapter,
> - u16 Address,
> - u8 Value);
> +EFUSE_Write1Byte(struct rtw_adapter *Adapter, u16 Address, u8 Value);
> void
> -EFUSE_Write1Byte(
> - struct rtw_adapter *Adapter,
> - u16 Address,
> - u8 Value)
> +EFUSE_Write1Byte(struct rtw_adapter *Adapter, u16 Address, u8 Value)
> {
> u8 Bytetemp = {0x00};
> u8 temp = {0x00};
> @@ -635,10 +629,7 @@ Efuse_ReadAllMap(struct rtw_adapter *pAdapter, u8
> efuseType, u8 *Efuse)
> *
>
> *---*/
> static void
> -efuse_ShadowRead1Byte(
> - struct rtw_adapter *pAdapter,
> - u16 Offset,
> - u8 *Value)
> +efuse_ShadowRead1Byte(struct rtw_adapter *pAdapter, u16 Offset, u8 *Value)
> {
> struct eeprom_priv *pEEPROM = GET_EEPROM_EFUSE_PRIV(pAdapter);
>
> @@ -647,10 +638,7 @@ efuse_ShadowRead1Byte(
>
> /* Read Two Bytes */
> static void
> -efuse_ShadowRead2Byte(
> - struct rtw_adapter *pAdapter,
> - u16 Offset,
> - u16 *Value)
> +efuse_ShadowRead2Byte(struct rtw_adapter *pAdapter, u16 Offset, u16 *Value)
> {
> struct eeprom_priv *pEEPROM = GET_EEPROM_EFUSE_PRIV(pAdapter);
>
> @@ -660,10 +648,7 @@ efuse_ShadowRead2Byte(
>
> /* Read Four Bytes */
> static void
> -efuse_ShadowRead4Byte(
> - struct rtw_adapter *pAdapter,
> - u16 Offset,
> - u32 *Value)
> +efuse_ShadowRead4Byte(struct rtw_adapter *pAdapter, u16 Offset, u32 *Value)
> {
> struct eeprom_priv *pEEPROM = GET_EEPROM_EFUSE_PRIV(pAdapter);
>
> @@ -722,11 +707,8 @@ void EFUSE_ShadowMapUpdate23a(struct rtw_adapter
> *pAdapter, u8 efuseType)
> *
>
> *---*/
> void
> -EFUSE_ShadowRead23a(
> - struct rtw_adapter *pAdapter,
> - u8 Type,
> - u16 Offset,
> - u32 *Value)
> +EFUSE_ShadowRead23a(struct rtw_adapter *pAdapter,
> + u8 Type, u16 Offset, u32 *Value)
> {
> if (Type == 1)
> efuse_ShadowRead1Byte(pAdapter, Offset, (u8 *)Value);
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] drivers: staging: lustre: Use mult if units not specified
Oh wait... You're right. This doesn't change the code how the code works. My bad. Still, it's better to just remove the condition instead of making the condition even more complicated and confusing. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Staging: rtl8192u : fix space before , coding style issue in r8190_rtl8256.c
This is a patch to r8190_rtl8256.c file that fixes space before , warning found
by checkpatch.pl tool
Signed-off-by: Mohammad Jamal
---
drivers/staging/rtl8192u/r8190_rtl8256.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/rtl8192u/r8190_rtl8256.c
b/drivers/staging/rtl8192u/r8190_rtl8256.c
index 45514aa..1868352 100644
--- a/drivers/staging/rtl8192u/r8190_rtl8256.c
+++ b/drivers/staging/rtl8192u/r8190_rtl8256.c
@@ -23,7 +23,7 @@
* Return: NONE
* Note: 8226 support both 20M and 40 MHz
*---*/
-void PHY_SetRF8256Bandwidth(struct net_device *dev , HT_CHANNEL_WIDTH
Bandwidth)
+void PHY_SetRF8256Bandwidth(struct net_device *dev, HT_CHANNEL_WIDTH Bandwidth)
{
u8 eRFPath;
struct r8192_priv *priv = ieee80211_priv(dev);
--
1.7.9.5
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/4] Drivers: scsi: storvsc: Fix miscellaneous issues
The first two patches in this series are a resend; these were submitted some months ago and as far as I know, there were no outstanding issues. Win8 and win8 r2 hosts do support SPC-3 features but claim SPC-2 compliance. This issue is fixed here as well. K. Y. Srinivasan (4): Drivers: scsi: storvsc: In responce to a scan event, scan the host Drivers: scsi: storvsc: Force discovery of LUNs that may have been removed. Drivers: scsi: storvsc: Fix a bug in storvsc limits Drivers: scsi: storvsc: Force SPC-3 compliance on win8 and win8 r2 hosts drivers/scsi/storvsc_drv.c | 71 +++ 1 files changed, 57 insertions(+), 14 deletions(-) -- 1.7.4.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/4] Drivers: scsi: storvsc: Fix a bug in storvsc limits
Commit 4cd83ecdac20d30725b4f96e5d7814a1e290bc7e changed the limits to reflect the values on the host. It turns out that WS2008R2 cannot correctly handle these new limits. Fix this bug by setting the limits based on the host. Signed-off-by: K. Y. Srinivasan --- drivers/scsi/storvsc_drv.c | 15 --- 1 files changed, 12 insertions(+), 3 deletions(-) diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c index a7163c6..fdc5164 100644 --- a/drivers/scsi/storvsc_drv.c +++ b/drivers/scsi/storvsc_drv.c @@ -1782,6 +1782,9 @@ static int storvsc_probe(struct hv_device *device, bool dev_is_ide = ((dev_id->driver_data == IDE_GUID) ? true : false); int target = 0; struct storvsc_device *stor_device; + int max_luns_per_target; + int max_targets; + int max_channels; /* * Based on the windows host we are running on, @@ -1795,12 +1798,18 @@ static int storvsc_probe(struct hv_device *device, vmscsi_size_delta = sizeof(struct vmscsi_win8_extension); vmstor_current_major = VMSTOR_WIN7_MAJOR; vmstor_current_minor = VMSTOR_WIN7_MINOR; + max_luns_per_target = STORVSC_IDE_MAX_LUNS_PER_TARGET; + max_targets = STORVSC_IDE_MAX_TARGETS; + max_channels = STORVSC_IDE_MAX_CHANNELS; break; default: sense_buffer_size = POST_WIN7_STORVSC_SENSE_BUFFER_SIZE; vmscsi_size_delta = 0; vmstor_current_major = VMSTOR_WIN8_MAJOR; vmstor_current_minor = VMSTOR_WIN8_MINOR; + max_luns_per_target = STORVSC_MAX_LUNS_PER_TARGET; + max_targets = STORVSC_MAX_TARGETS; + max_channels = STORVSC_MAX_CHANNELS; break; } @@ -1848,9 +1857,9 @@ static int storvsc_probe(struct hv_device *device, break; case SCSI_GUID: - host->max_lun = STORVSC_MAX_LUNS_PER_TARGET; - host->max_id = STORVSC_MAX_TARGETS; - host->max_channel = STORVSC_MAX_CHANNELS - 1; + host->max_lun = max_luns_per_target; + host->max_id = max_targets; + host->max_channel = max_channels - 1; break; default: -- 1.7.4.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/4] Drivers: scsi: storvsc: In responce to a scan event, scan the host
The virtual HBA that storvsc implements can support multiple channels and
targets. So, scan the host when the host notifies that a scan is needed.
Signed-off-by: K. Y. Srinivasan
---
drivers/scsi/storvsc_drv.c | 19 +++
1 files changed, 7 insertions(+), 12 deletions(-)
diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index e3ba251..0a96fef 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -426,21 +426,16 @@ done:
kfree(wrk);
}
-static void storvsc_bus_scan(struct work_struct *work)
+static void storvsc_host_scan(struct work_struct *work)
{
struct storvsc_scan_work *wrk;
- int id, order_id;
+ struct Scsi_Host *host;
wrk = container_of(work, struct storvsc_scan_work, work);
- for (id = 0; id < wrk->host->max_id; ++id) {
- if (wrk->host->reverse_ordering)
- order_id = wrk->host->max_id - id - 1;
- else
- order_id = id;
-
- scsi_scan_target(&wrk->host->shost_gendev, 0,
- order_id, SCAN_WILD_CARD, 1);
- }
+ host = wrk->host;
+
+ scsi_scan_host(host);
+
kfree(wrk);
}
@@ -1198,7 +1193,7 @@ static void storvsc_on_receive(struct hv_device *device,
if (!work)
return;
- INIT_WORK(&work->work, storvsc_bus_scan);
+ INIT_WORK(&work->work, storvsc_host_scan);
work->host = stor_device->host;
schedule_work(&work->work);
break;
--
1.7.4.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/4] Drivers: scsi: storvsc: Force SPC-3 compliance on win8 and win8 r2 hosts
On win8 and win8 r2 hosts force SPC-3 compliance for MSFT virtual disks.
Ubuntu has been carrying a similar patch outside the tree for a while now.
Starting with win10, the host will support SPC-3 compliance. Based on all
the testing that has been done on win8 and win8 r2 hosts, we are comfortable
claiming SPC-3 compliance on these hosts as well. This will enable TRIM
support on these hosts.
Suggested by: James Bottomley
Signed-off-by: K. Y. Srinivasan
---
drivers/scsi/storvsc_drv.c | 13 +
1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index fdc5164..7487e07 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -1468,6 +1468,19 @@ static int storvsc_device_configure(struct scsi_device
*sdevice)
*/
sdevice->sdev_bflags |= msft_blist_flags;
+ /*
+* If the host is WIN8 or WIN8 R2, claim conformance to SPC-3
+* if the device is a MSFT virtual device.
+*/
+ if (!strncmp(sdevice->vendor, "Msft", 4)) {
+ switch (vmbus_proto_version) {
+ case VERSION_WIN8:
+ case VERSION_WIN8_1:
+ sdevice->scsi_level = SCSI_SPC_3;
+ break;
+ }
+ }
+
return 0;
}
--
1.7.4.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/4] Drivers: scsi: storvsc: Force discovery of LUNs that may have been removed.
The host asks the guest to scan when a LUN is removed or added.
The only way a guest can identify the removed LUN is when an I/O is
attempted on a removed LUN - the SRB status code indicates that the LUN
is invalid. We currently handle this SRB status and remove the device.
Rather than waiting for an I/O to remove the device, force the discovery of
LUNs that may have been removed prior to discovering LUNs that may have
been added.
Signed-off-by: K. Y. Srinivasan
---
drivers/scsi/storvsc_drv.c | 26 ++
1 files changed, 26 insertions(+), 0 deletions(-)
diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 0a96fef..a7163c6 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -430,10 +430,36 @@ static void storvsc_host_scan(struct work_struct *work)
{
struct storvsc_scan_work *wrk;
struct Scsi_Host *host;
+ struct scsi_device *sdev;
+ unsigned long flags;
wrk = container_of(work, struct storvsc_scan_work, work);
host = wrk->host;
+ /*
+* Before scanning the host, first check to see if any of the
+* currrently known devices have been hot removed. We issue a
+* "unit ready" command against all currently known devices.
+* This I/O will result in an error for devices that have been
+* removed. As part of handling the I/O error, we remove the device.
+*
+* When a LUN is added or removed, the host sends us a signal to
+* scan the host. Thus we are forced to discover the LUNs that
+* may have been removed this way.
+*/
+ mutex_lock(&host->scan_mutex);
+ spin_lock_irqsave(host->host_lock, flags);
+ list_for_each_entry(sdev, &host->__devices, siblings) {
+ spin_unlock_irqrestore(host->host_lock, flags);
+ scsi_test_unit_ready(sdev, 1, 1, NULL);
+ spin_lock_irqsave(host->host_lock, flags);
+ continue;
+ }
+ spin_unlock_irqrestore(host->host_lock, flags);
+ mutex_unlock(&host->scan_mutex);
+ /*
+* Now scan the host to discover LUNs that may have been added.
+*/
scsi_scan_host(host);
kfree(wrk);
--
1.7.4.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv2 0/8] from now on, use 'isil' as prefix for Intersil
As suggested by Jason on a v0, I made a specific series for that work,
which intend to remove all remaining 'isl' prefix for Intersil to only
have 'isil'. More details below.
When Intersil ISL12057 driver was introduced by commit 70e123373c05
("rtc: Add support for Intersil ISL12057 I2C RTC chip"), the vendor
prefix 'isl' was used instead of the expected 'isil' (Intersil
NASDAQ symbol) and documented in vendor-prefixes.txt.
Recently, a patch from Philip Zabel (7a6540ca856a, "ARM: mvebu:
Change vendor prefix for Intersil Corporation to isil") fixed that
prefix in ReadyNAS devices .dts files (AFAICT, the only users of
the driver).
Then, commits 7c75c1d5e72b ("dt-bindings: Document deprecated device
vendor name to fix related warning") and b2ea3f82e798 (dt-bindings:
Document correct and deprecated vendor-prefix with device isl29028)
decided to go the other way and deprecate isil in vendor-prefixes.txt
and in isl29028.c staging driver.
While trying and merge a fix I wrote for ISL12057 drivers to finish
Philip's work, it conflicted with the two recently introduced commits,
and revealed the issue: at the moment, there are various compatible
strings in drivers and .dts files for Intersil products which use
either isl or isil:
$ grep -R "isil," .
./Documentation/devicetree/bindings/i2c/trivial-devices.txt:isil,isl29028
(deprecated, use isl)
./drivers/staging/iio/light/isl29028.c: { .compatible = "isil,isl29028", },/*
deprecated, don't use */
./drivers/staging/iio/light/isl29018.c: { .compatible = "isil,isl29018", },
./drivers/staging/iio/light/isl29018.c: { .compatible = "isil,isl29023", },
./drivers/staging/iio/light/isl29018.c: { .compatible = "isil,isl29035", },
./arch/powerpc/boot/dts/p1022rdk.dts:compatible = "isil,zl6100";
./arch/powerpc/boot/dts/p1022rdk.dts:compatible = "isil,zl6100";
./arch/powerpc/boot/dts/p1022rdk.dts:compatible = "isil,zl6100";
./arch/powerpc/boot/dts/p1022rdk.dts:compatible = "isil,zl6100";
./arch/arm/boot/dts/exynos5800-peach-pi.dts: compatible = "isil,isl29018";
./arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi: compatible = "isil,isl1208";
./arch/arm/boot/dts/tegra20-ventana.dts: compatible = "isil,isl29018";
./arch/arm/boot/dts/tegra20-seaboard.dts:compatible = "isil,isl29018";
./arch/arm/boot/dts/armada-xp-netgear-rn2120.dts: compatible = "isil,isl12057";
./arch/arm/boot/dts/armada-370-netgear-rn104.dts: compatible = "isil,isl12057";
./arch/arm/boot/dts/exynos5420-peach-pit.dts: compatible = "isil,isl29018";
./arch/arm/boot/dts/armada-370-netgear-rn102.dts: compatible = "isil,isl12057";
$ grep -R "isl," .
./Documentation/devicetree/bindings/regulator/isl9305.txt:- compatible:
"isl,isl9305" or "isl,isl9305h"
./Documentation/devicetree/bindings/regulator/isl9305.txt:
compatible = "isl,isl9305";
./Documentation/devicetree/bindings/i2c/trivial-devices.txt:isl,isl12057
Intersil ISL12057 I2C RTC Chip
./Documentation/devicetree/bindings/i2c/trivial-devices.txt:isl,isl29028
Intersil ISL29028 Ambient Light
./drivers/regulator/isl9305.c: { .compatible = "isl,isl9305" },
./drivers/regulator/isl9305.c: { .compatible = "isl,isl9305h" },
./drivers/staging/iio/light/isl29028.c: { .compatible = "isl,isl29028", },
./drivers/rtc/rtc-isl12057.c: { .compatible = "isl,isl12057" },
./drivers/rtc/rtc-isl12022.c: { .compatible = "isl,isl12022" },
./arch/arm/boot/dts/tegra30-cardhu.dtsi: compatible = "isl,isl29028";
./arch/arm/boot/dts/zynq-parallella.dts: compatible = "isl,isl9305";
AFAICT, it seems it makes sense to *definitively* settle for isil as the
vendor prefix for Intersil, as Philip did in 7a6540ca856a: it's the NASDAQ
symbol and this choice requires less changes than opting for isl.
So, this series changes compatible strings in .dts files to use isil where
isl was found before, and modify drivers w/ compatible strings using isl
to add one using isil. In those cases, a comment is made that the old
compatible string is kept for backward compatibility (w/ out-fo-tree users
of those drivers). Additionally, it leaves only isil as prefix in
vendor-prefixes.txt. Those changes should prevent any new inclusion of
isl compatible strings for Intersil devices due to copy-and-paste.
Changes since v1:
- split previous patch fixing trivial-devices.txt and
vendor-prefixes.txt in two different patches, as suggested by Uwe
- fixed a space/tab issue spotted by Uwe
Arnaud Ebalard (8):
dt-bindings: use isil prefix for Intersil in vendor-prefixes.txt
dt-bindings: use isil prefix for Intersil in I2C trivial-devices.txt
rtc: isl12022: deprecate use of isl in compatible string for isil
rtc: isl12057: deprecate use of isl in compatible string for isil
staging: iio: isl29028: deprecate use of isl in compatible string for isil
regulator: isl9305: deprecate use of isl in compatible string for isil
arm: dts: zynq: update isl9305 compatible string to use isil ve
[PATCHv2 5/8] staging: iio: isl29028: deprecate use of isl in compatible string for isil
"isil" and "isl" prefixes are used at various locations inside the kernel
to reference Intersil corporation. This patch is part of a series fixing
those locations were "isl" is used in compatible strings to use the now
expected "isil" prefix instead (NASDAQ symbol for Intersil and most used
version). The old compatible string is kept for backward compatibility.
Signed-off-by: Arnaud Ebalard
---
drivers/staging/iio/light/isl29028.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/iio/light/isl29028.c
b/drivers/staging/iio/light/isl29028.c
index e969107ddb47..6440e3b293ca 100644
--- a/drivers/staging/iio/light/isl29028.c
+++ b/drivers/staging/iio/light/isl29028.c
@@ -537,8 +537,8 @@ static const struct i2c_device_id isl29028_id[] = {
MODULE_DEVICE_TABLE(i2c, isl29028_id);
static const struct of_device_id isl29028_of_match[] = {
- { .compatible = "isl,isl29028", },
- { .compatible = "isil,isl29028", },/* deprecated, don't use */
+ { .compatible = "isl,isl29028", }, /* for backward compat., don't use */
+ { .compatible = "isil,isl29028", },
{ },
};
MODULE_DEVICE_TABLE(of, isl29028_of_match);
--
2.1.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH v2] tools: hv: kvp_daemon: make IPv6-only-injection work
> -Original Message-
> From: devel [mailto:driverdev-devel-boun...@linuxdriverproject.org] On
> Behalf Of Dexuan Cui
> Sent: Wednesday, December 10, 2014 19:33 PM
> To: gre...@linuxfoundation.org; linux-ker...@vger.kernel.org; driverdev-
> de...@linuxdriverproject.org; vkuzn...@redhat.com; o...@aepfle.de;
> a...@canonical.com; jasow...@redhat.com; KY Srinivasan
> Cc: Haiyang Zhang
> Subject: [PATCH v2] tools: hv: kvp_daemon: make IPv6-only-injection work
>
> In the case the host only injects an IPv6 address, the dhcp_enabled flag is
> true (it's only for IPv4 according to Hyper-V host team), but we still need to
> proceed to parse the IPv6 information.
>
> Cc: Vitaly Kuznetsov
> Cc: K. Y. Srinivasan
> Signed-off-by: Dexuan Cui
> ---
>
> v2: removed the distro-specific logic as Vitaly suggested.
>
> tools/hv/hv_kvp_daemon.c | 12 ++--
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
> index 6a6432a..4b3ee35 100644
> --- a/tools/hv/hv_kvp_daemon.c
> +++ b/tools/hv/hv_kvp_daemon.c
> @@ -1308,16 +1308,17 @@ static int kvp_set_ip_info(char *if_name, struct
> hv_kvp_ipaddr_value *new_val)
> if (error)
> goto setval_error;
>
> + /*
> + * The dhcp_enabled flag is only for IPv4. In the case the host only
> + * injects an IPv6 address, the flag is true, but we still need to
> + * proceed to parse and pass the IPv6 information to the
> + * disto-specific script hv_set_ifconfig.
> + */
> if (new_val->dhcp_enabled) {
> error = kvp_write_file(file, "BOOTPROTO", "", "dhcp");
> if (error)
> goto setval_error;
>
> - /*
> - * We are done!.
> - */
> - goto setval_done;
> -
> } else {
> error = kvp_write_file(file, "BOOTPROTO", "", "none");
> if (error)
> @@ -1345,7 +1346,6 @@ static int kvp_set_ip_info(char *if_name, struct
> hv_kvp_ipaddr_value *new_val)
> if (error)
> goto setval_error;
>
> -setval_done:
> fclose(file);
>
> /*
> --
> 1.9.1
Hi Vitaly,
Can you please ACK the v2 patch?
Or, please let me know if you have new comments.
Thanks,
-- Dexuan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 0/2] lprocfs_write_frac_u64_helper cleanup
Added a second patch to address Dan Carpenter's concern with the complexity of passing the sign through `mult'. Compile tested only. Chris Rorvick (2): drivers: staging: lustre: Use mult if units not specified drivers: staging: lustre: Track sign separately drivers/staging/lustre/lustre/obdclass/lprocfs_status.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) -- 2.1.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 2/2] drivers: staging: lustre: Track sign separately
The `mult' parameter is negated if the user data begins with a '-' so
that the final value has the appropriate sign. But `mult' is only used
if the user data does not include a "units" suffix. In this case,
`mult' is overridden with the numeric scale conveyed by the units suffix,
but retains the sign of the original value.
Having `mult' serving double-duty works but is confusing. Use a new
local variable to store the sign of the user data instead. This also
fixes a pitfall of passing 0 to `mult', expecting it to be ignored when
a units suffix is specified, but having the effect of taking the
absolute value of the user-provided data.
Signed-off-by: Chris Rorvick
---
drivers/staging/lustre/lustre/obdclass/lprocfs_status.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
index 92ed0a0..b6c3352 100644
--- a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
+++ b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
@@ -1864,6 +1864,7 @@ int lprocfs_write_frac_u64_helper(const char *buffer,
unsigned long count,
char kernbuf[22], *end, *pbuf;
__u64 whole, frac = 0, units;
unsigned frac_d = 1;
+ int sign = 1;
if (count > (sizeof(kernbuf) - 1))
return -EINVAL;
@@ -1874,7 +1875,7 @@ int lprocfs_write_frac_u64_helper(const char *buffer,
unsigned long count,
kernbuf[count] = '\0';
pbuf = kernbuf;
if (*pbuf == '-') {
- mult = -mult;
+ sign = -1;
pbuf++;
}
@@ -1911,11 +1912,11 @@ int lprocfs_write_frac_u64_helper(const char *buffer,
unsigned long count,
}
/* Specified units override the multiplier */
if (units > 1)
- mult = mult < 0 ? -units : units;
+ mult = units;
frac *= mult;
do_div(frac, frac_d);
- *val = whole * mult + frac;
+ *val = sign * (whole * mult + frac);
return 0;
}
EXPORT_SYMBOL(lprocfs_write_frac_u64_helper);
--
2.1.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 1/2] drivers: staging: lustre: Use mult if units not specified
Units can be passed to lprocfs_write_frac_u64_helper() via a suffix (e.g., "...K", "...M", etc.) tacked onto the value. A comment states that "specified units override the multiplier," though the multiplier is overridden regardless. Update the conditional logic so that it only applies when units are specified. Signed-off-by: Chris Rorvick --- drivers/staging/lustre/lustre/obdclass/lprocfs_status.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c index 61e04af..92ed0a0 100644 --- a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c +++ b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c @@ -1910,7 +1910,7 @@ int lprocfs_write_frac_u64_helper(const char *buffer, unsigned long count, units <<= 10; } /* Specified units override the multiplier */ - if (units) + if (units > 1) mult = mult < 0 ? -units : units; frac *= mult; -- 2.1.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 2/2] drivers: staging: lustre: Track sign separately
On 2014/12/16, 9:24 PM, "Chris Rorvick" wrote:
>The `mult' parameter is negated if the user data begins with a '-' so
>that the final value has the appropriate sign. But `mult' is only used
>if the user data does not include a "units" suffix. In this case,
>`mult' is overridden with the numeric scale conveyed by the units suffix,
>but retains the sign of the original value.
>
>Having `mult' serving double-duty works but is confusing. Use a new
>local variable to store the sign of the user data instead. This also
>fixes a pitfall of passing 0 to `mult', expecting it to be ignored when
>a units suffix is specified, but having the effect of taking the
>absolute value of the user-provided data.
>
>Signed-off-by: Chris Rorvick
Both patches look good to me.
Reviewed-by: Andreas Dilger
>---
> drivers/staging/lustre/lustre/obdclass/lprocfs_status.c | 7 ---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
>diff --git a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
>b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
>index 92ed0a0..b6c3352 100644
>--- a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
>+++ b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
>@@ -1864,6 +1864,7 @@ int lprocfs_write_frac_u64_helper(const char
>*buffer, unsigned long count,
> char kernbuf[22], *end, *pbuf;
> __u64 whole, frac = 0, units;
> unsigned frac_d = 1;
>+ int sign = 1;
>
> if (count > (sizeof(kernbuf) - 1))
> return -EINVAL;
>@@ -1874,7 +1875,7 @@ int lprocfs_write_frac_u64_helper(const char
>*buffer, unsigned long count,
> kernbuf[count] = '\0';
> pbuf = kernbuf;
> if (*pbuf == '-') {
>- mult = -mult;
>+ sign = -1;
> pbuf++;
> }
>
>@@ -1911,11 +1912,11 @@ int lprocfs_write_frac_u64_helper(const char
>*buffer, unsigned long count,
> }
> /* Specified units override the multiplier */
> if (units > 1)
>- mult = mult < 0 ? -units : units;
>+ mult = units;
>
> frac *= mult;
> do_div(frac, frac_d);
>- *val = whole * mult + frac;
>+ *val = sign * (whole * mult + frac);
> return 0;
> }
> EXPORT_SYMBOL(lprocfs_write_frac_u64_helper);
>--
>2.1.0
>
>
Cheers, Andreas
--
Andreas Dilger
Lustre Software Architect
Intel High Performance Data Division
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 0/2] lprocfs_write_frac_u64_helper cleanup
On Tue, Dec 16, 2014 at 10:24:00PM -0600, Chris Rorvick wrote: > Added a second patch to address Dan Carpenter's concern with the > complexity of passing the sign through `mult'. Compile tested only. Thanks. That does look cleaner. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
