Re: [PATCH 2/4] staging: sep: No else is necessary after a break (reported by checkpatch)

2014-07-20 Thread Corentin LABBE 
Le 19/07/2014 20:00, Joe Perches a écrit :
 (Adding Mark Allyn and Jayant Mangalampalli)
 
 Is this still project still active?

I do not know

 
 On Sat, 2014-07-19 at 19:34 +0200, LABBE Corentin wrote:
 Signed-off-by: LABBE Corentin clabbe.montj...@gmail.com
 ---
  drivers/staging/sep/sep_main.c | 5 ++---
  1 file changed, 2 insertions(+), 3 deletions(-)

 diff --git a/drivers/staging/sep/sep_main.c b/drivers/staging/sep/sep_main.c
 index 177e4b9..1580d95f 100644
 --- a/drivers/staging/sep/sep_main.c
 +++ b/drivers/staging/sep/sep_main.c
 @@ -2881,12 +2881,11 @@ static int sep_free_dma_tables_and_dcb(struct 
 sep_device *sep, bool isapplet,
  if (is_kva) {
  error = -ENODEV;
  break;
 -} else {
 -error_temp = copy_to_user(
 +}
 +error_temp = copy_to_user(
  (void __user *)tail_pt,
  dcb_table_ptr-tail_data,
  dcb_table_ptr-tail_data_size);
 -}
  if (error_temp) {
  /* Release the DMA resource */
  error = -EFAULT;
 
 It'd be probably be better to rewrite the code to unindent
 a level by using continue.  Something like below:
 
 btw:
 
 the is_kva test looks very odd and should probably be
 moved outside the loop.
 
 pt_hold should probably be void * not unsigned long
 as it loses high order bits on x86-32.
 
 definition:
   aligned_u64 out_vr_tail_pt;
 use:
 + pt_hold = (unsigned long)dcb_table_ptr-
 + out_vr_tail_pt;
 

As I said in the introduction email, I have done thoses patch for the Eudyptula 
challenge,
since I have not the hardware needed by this driver I cannot modify beyond 
simple style changes without testing

Regards


 ---
  drivers/staging/sep/sep_main.c | 37 +
  1 file changed, 17 insertions(+), 20 deletions(-)
 
 diff --git a/drivers/staging/sep/sep_main.c b/drivers/staging/sep/sep_main.c
 index 75ca15e..24b4a54 100644
 --- a/drivers/staging/sep/sep_main.c
 +++ b/drivers/staging/sep/sep_main.c
 @@ -2871,26 +2871,23 @@ static int sep_free_dma_tables_and_dcb(struct 
 sep_device *sep, bool isapplet,
* Go over each DCB and see if
* tail pointer must be updated
*/
 - for (i = 0; i  (*dma_ctx)-nr_dcb_creat;
 -  i++, dcb_table_ptr++) {
 - if (dcb_table_ptr-out_vr_tail_pt) {
 - pt_hold = (unsigned long)dcb_table_ptr-
 - out_vr_tail_pt;
 - tail_pt = (void *)pt_hold;
 - if (is_kva) {
 - error = -ENODEV;
 - break;
 - } else {
 - error_temp = copy_to_user(
 - (void __user *)tail_pt,
 - dcb_table_ptr-tail_data,
 - dcb_table_ptr-tail_data_size);
 - }
 - if (error_temp) {
 - /* Release the DMA resource */
 - error = -EFAULT;
 - break;
 - }
 + for (i = 0; i  (*dma_ctx)-nr_dcb_creat; i++, dcb_table_ptr++) 
 {
 + if (!dcb_table_ptr-out_vr_tail_pt)
 + continue;
 + pt_hold = (unsigned long)dcb_table_ptr-
 + out_vr_tail_pt;
 + tail_pt = (void *)pt_hold;
 + if (is_kva) {
 + error = -ENODEV;
 + break;
 + }
 + error_temp = copy_to_user((void __user *)tail_pt,
 +   dcb_table_ptr-tail_data,
 +   
 dcb_table_ptr-tail_data_size);
 + if (error_temp) {
 + /* Release the DMA resource */
 + error = -EFAULT;
 + break;
   }
   }
   }
 
 

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

[PATCH] staging: vt6655: Fixed checkpatch errors

2014-07-20 Thread Petar Dimitrijevic 
This patch fixes the errors report by the checkpatch script.
Most of them are use of C99 comments.

The checkpatch doesn't report any errors after the clean up.
However, please not that there are still ton of warnings left.

Signed-off-by: Petar Dimitrijevic petar.dimitrije...@vorteksed.com.mk
---
 drivers/staging/vt6655/datarate.c | 35 +++
 1 file changed, 19 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/vt6655/datarate.c 
b/drivers/staging/vt6655/datarate.c
index f8420d6..83ad74c 100644
--- a/drivers/staging/vt6655/datarate.c
+++ b/drivers/staging/vt6655/datarate.c
@@ -47,11 +47,12 @@
 
 /*-  Static Classes  */
 
-extern unsigned short TxRate_iwconfig; //2008-5-8 add by chester
+extern unsigned short TxRate_iwconfig; /* 2008-5-8 add by chester */
 /*-  Static Variables  --*/
 static int msglevel = MSG_LEVEL_INFO;
-static const unsigned char acbyIERate[MAX_RATE] =
-{0x02, 0x04, 0x0B, 0x16, 0x0C, 0x12, 0x18, 0x24, 0x30, 0x48, 0x60, 0x6C};
+static const unsigned char acbyIERate[MAX_RATE] = {
+0x02, 0x04, 0x0B, 0x16, 0x0C, 0x12, 0x18, 0x24, 0x30, 0x48, 0x60, 0x6C
+};
 
 #define AUTORATE_TXOK_CNT   0x0400
 #define AUTORATE_TXFAIL_CNT 0x0064
@@ -70,7 +71,7 @@ s_vResetCounter(
 {
unsigned char ii;
 
-   // clear statistic counter for auto_rate
+   /* clear statistic counter for auto_rate */
for (ii = 0; ii = MAX_RATE; ii++) {
psNodeDBTable-uTxOk[ii] = 0;
psNodeDBTable-uTxFail[ii] = 0;
@@ -102,8 +103,8 @@ DATARATEbyGetRateIdx(
 {
unsigned char ii;
 
-   //Erase basicRate flag.
-   byRate = byRate  0x7F;//0111 
+   /* Erase basicRate flag. */
+   byRate = byRate  0x7F;/* 0111  */
 
for (ii = 0; ii  MAX_RATE; ii++) {
if (acbyIERate[ii] == byRate)
@@ -151,13 +152,14 @@ wGetRateIdx(
 {
unsigned short ii;
 
-   //Erase basicRate flag.
-   byRate = byRate  0x7F;//0111 
+   /* Erase basicRate flag. */
+   byRate = byRate  0x7F;/* 0111  */
 
for (ii = 0; ii  MAX_RATE; ii++) {
if (acbyIERate[ii] == byRate)
return ii;
}
+
return 0;
 }
 
@@ -218,7 +220,7 @@ RATEvParseMaxRate(
for (ii = 0; ii  uRateLen; ii++) {
byRate = (unsigned char)(pItemRates-abyRates[ii]);
if (WLAN_MGMT_IS_BASICRATE(byRate)  bUpdateBasicRate)  {
-   // Add to basic rate set, update 
pDevice-byTopCCKBasicRate and pDevice-byTopOFDMBasicRate
+   /* Add to basic rate set, update 
pDevice-byTopCCKBasicRate and pDevice-byTopOFDMBasicRate */
CARDbAddBasicRate((void *)pDevice, wGetRateIdx(byRate));
DBG_PRT(MSG_LEVEL_DEBUG, KERN_INFO ParseMaxRate 
AddBasicRate: %d\n, wGetRateIdx(byRate));
}
@@ -238,9 +240,9 @@ RATEvParseMaxRate(
 
for (ii = 0; ii  uExtRateLen; ii++) {
byRate = (unsigned char)(pItemExtRates-abyRates[ii]);
-   // select highest basic rate
+   /* select highest basic rate */
if 
(WLAN_MGMT_IS_BASICRATE(pItemExtRates-abyRates[ii])) {
-   // Add to basic rate set, update 
pDevice-byTopCCKBasicRate and pDevice-byTopOFDMBasicRate
+   /* Add to basic rate set, update 
pDevice-byTopCCKBasicRate and pDevice-byTopOFDMBasicRate */
CARDbAddBasicRate((void *)pDevice, 
wGetRateIdx(byRate));
DBG_PRT(MSG_LEVEL_DEBUG, KERN_INFO 
ParseMaxRate AddBasicRate: %d\n, wGetRateIdx(byRate));
}
@@ -302,10 +304,9 @@ RATEvTxRateFallBack(
unsigned short wIdxUpRate = 0;
unsigned long dwTxDiff = 0;
 
-   if (pDevice-pMgmt-eScanState != WMAC_NO_SCANNING) {
-   // Don't do Fallback when scanning Channel
+   if (pDevice-pMgmt-eScanState != WMAC_NO_SCANNING)
+   /* Don't do Fallback when scanning Channel */
return;
-   }
 
psNodeDBTable-uTimeCount++;
 
@@ -357,11 +358,13 @@ RATEvTxRateFallBack(
(psNodeDBTable-uTxFail[MAX_RATE] * 4)) {
psNodeDBTable-wTxDataRate = wIdxUpRate;
}
-   } else { // adhoc, if uTxOk =0  uTxFail = 0
+   } else {
+   /* adhoc, if uTxOk =0  uTxFail = 0 */
if (psNodeDBTable-uTxFail[MAX_RATE] == 0)
psNodeDBTable-wTxDataRate = wIdxUpRate;
}
-//2008-5-8 add by chester
+
+   /* 2008-5-8 add by chester */
TxRate_iwconfig = psNodeDBTable-wTxDataRate;
s_vResetCounter(psNodeDBTable);
 
-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org

Re: [PATCH] drivers: Let several drivers depends on HAS_IOMEM for 'devm_ioremap_resource'

2014-07-20 Thread Chen Gang 
On 07/19/2014 02:02 AM, Chen Gang wrote:
 2014-07-18 18:51 GMT+08:00 Richard Weinberger rich...@nod.at:
 Am 18.07.2014 12:44, schrieb Chen Gang:
 On 07/18/2014 03:35 PM, Richard Weinberger wrote:
 Am 18.07.2014 02:36, schrieb Chen Gang:

 On 07/18/2014 02:09 AM, Richard Weinberger wrote:
 Am 17.07.2014 12:48, schrieb Arnd Bergmann:
 AFAICT, NO_IOMEM only has a real purpose on UML these days. Could we 
 take
 a shortcut here and make COMPILE_TEST depend on !UML? Getting random 
 stuff
 to build on UML seems pointless to me and we special-case it in a 
 number of
 places already.

 If UML is the only arch without io memory the dependency on !UML seems
 reasonable to me. :-)


 For me, if only uml left, I suggest to implement dummy functions within
 uml instead of let CONFIG_UML appear in generic include directory. And
 then remove all HAS_IOMEM and NO_IOMEM from kernel.

 Erm, this is something completely different.
 I thought we're focusing on COMPILE_TEST?


 COMPILE_TEST is none-architecture specific, but UML is. So in generic
 include folder, if we're focusing on choosing whether COMPILE_TEST or
 UML, for me, I will choose COMPILE_TEST.

 If we're not only focusing on COMPILE_TEST, for me, if something only
 depend on one architecture, I'd like to put them under arch/*/ folder.

 Especially, after that, we can remove all HAS_IOMEM and NO_IOMEM, nobody
 has to think of them again. :-)

 And then we end up with a solution that on UML a lot of completely useless
 drivers are build which fail in various interesting manners because you'll
 add stubs for all kinds of io memory related functions to arch/um/?
 We had this kind of discussion already. You'll need more than ioremap...

 I like Arnd's idea *much* more to make COMPILE_TEST depend on !UML.

 
 That will let UML itself against COMPILE_TEST (but all the other
 architectures not).
 
 And if let COMPILE_TEST depend on !UML, can we still remove all
 HAS_IOMEM and NO_IOMEM from kernel? (I guess so).
 
 If we can remove them, we can send related patch firstly -- that will
 let current discussion be in UML architecture wide instead of kernel
 wide.
 

Next, I shall:

 - Remove HAS_IOMEM and NO_IOMEM from kernel, firstly.

 - Try to make dummy IOMEM functions for score architecture.

 - Continue discussing with UML for it.


By the way: how about HAS_DMA? can we treat it as HAS_IOMEM (remove
it from kernel)? (for me, I guess we can not).


At present, I shall finish sending patch for removing IOMEM today, and
continue to welcome any ideas, suggestions or completions for IOMEM or
DMA.


Thanks.
-- 
Chen Gang

Open share and attitude like air water and life which God blessed
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 2/2] Staging: Remove redundant comments in wcmd.h file

2014-07-20 Thread Igor Bezukh 
Removed redundant comments in the header file wcmd.h

Signed-off-by: Igor Bezukh igb...@gmail.com
---
 drivers/staging/vt6655/wcmd.h |   11 ---
 1 file changed, 11 deletions(-)

diff --git a/drivers/staging/vt6655/wcmd.h b/drivers/staging/vt6655/wcmd.h
index 2844476..696916c 100644
--- a/drivers/staging/vt6655/wcmd.h
+++ b/drivers/staging/vt6655/wcmd.h
@@ -33,12 +33,9 @@
 #include 80211hdr.h
 #include 80211mgr.h
 
-/*-  Export Definitions -*/
-
 #define AUTHENTICATE_TIMEOUT   1000 //ms
 #define ASSOCIATE_TIMEOUT  1000 //ms
 
-// Command code
 typedef enum tagCMD_CODE {
WLAN_CMD_BSSID_SCAN,
WLAN_CMD_SSID,
@@ -76,7 +73,6 @@ typedef struct tagCMD_ITEM {
bool bForceSCAN;
 } CMD_ITEM, *PCMD_ITEM;
 
-// Command state
 typedef enum tagCMD_STATE {
WLAN_CMD_SCAN_START,
WLAN_CMD_SCAN_END,
@@ -92,13 +88,6 @@ typedef enum tagCMD_STATE {
WLAN_CMD_IDLE
 } CMD_STATE, *PCMD_STATE;
 
-/*-  Export Classes  */
-
-/*-  Export Variables  --*/
-
-/*-  Export Types  --*/
-
-/*-  Export Functions  --*/
 void
 vResetCommandTimer(
void *hDeviceContext
-- 
1.7.9.5

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

[PATCHv2] Staging: vt6655: Remove redundant comments in wcmd.h

2014-07-20 Thread Igor Bezukh 
Removed redundant comments in the header file wcmd.h

Signed-off-by: Igor Bezukh igb...@gmail.com
---
Changes v2: I've missed another redundant comment //ms
 so I've deleted it and added vt6655 to the patch subject

 drivers/staging/vt6655/wcmd.h |   15 ++-
 1 file changed, 2 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/vt6655/wcmd.h b/drivers/staging/vt6655/wcmd.h
index 2844476..126b61c 100644
--- a/drivers/staging/vt6655/wcmd.h
+++ b/drivers/staging/vt6655/wcmd.h
@@ -33,12 +33,9 @@
 #include 80211hdr.h
 #include 80211mgr.h
 
-/*-  Export Definitions -*/
+#define AUTHENTICATE_TIMEOUT   1000
+#define ASSOCIATE_TIMEOUT  1000
 
-#define AUTHENTICATE_TIMEOUT   1000 //ms
-#define ASSOCIATE_TIMEOUT  1000 //ms
-
-// Command code
 typedef enum tagCMD_CODE {
WLAN_CMD_BSSID_SCAN,
WLAN_CMD_SSID,
@@ -76,7 +73,6 @@ typedef struct tagCMD_ITEM {
bool bForceSCAN;
 } CMD_ITEM, *PCMD_ITEM;
 
-// Command state
 typedef enum tagCMD_STATE {
WLAN_CMD_SCAN_START,
WLAN_CMD_SCAN_END,
@@ -92,13 +88,6 @@ typedef enum tagCMD_STATE {
WLAN_CMD_IDLE
 } CMD_STATE, *PCMD_STATE;
 
-/*-  Export Classes  */
-
-/*-  Export Variables  --*/
-
-/*-  Export Types  --*/
-
-/*-  Export Functions  --*/
 void
 vResetCommandTimer(
void *hDeviceContext
-- 
1.7.9.5

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

Re: [PATCH] drivers: Let several drivers depends on HAS_IOMEM for 'devm_ioremap_resource'

2014-07-20 Thread Chen Gang 
On 07/20/2014 04:38 PM, Chen Gang wrote:
 On 07/19/2014 02:02 AM, Chen Gang wrote:
 2014-07-18 18:51 GMT+08:00 Richard Weinberger rich...@nod.at:
 Am 18.07.2014 12:44, schrieb Chen Gang:
 On 07/18/2014 03:35 PM, Richard Weinberger wrote:
 Am 18.07.2014 02:36, schrieb Chen Gang:

 On 07/18/2014 02:09 AM, Richard Weinberger wrote:
 Am 17.07.2014 12:48, schrieb Arnd Bergmann:
 AFAICT, NO_IOMEM only has a real purpose on UML these days. Could we 
 take
 a shortcut here and make COMPILE_TEST depend on !UML? Getting random 
 stuff
 to build on UML seems pointless to me and we special-case it in a 
 number of
 places already.

 If UML is the only arch without io memory the dependency on !UML seems
 reasonable to me. :-)


 For me, if only uml left, I suggest to implement dummy functions within
 uml instead of let CONFIG_UML appear in generic include directory. And
 then remove all HAS_IOMEM and NO_IOMEM from kernel.

 Erm, this is something completely different.
 I thought we're focusing on COMPILE_TEST?


 COMPILE_TEST is none-architecture specific, but UML is. So in generic
 include folder, if we're focusing on choosing whether COMPILE_TEST or
 UML, for me, I will choose COMPILE_TEST.

 If we're not only focusing on COMPILE_TEST, for me, if something only
 depend on one architecture, I'd like to put them under arch/*/ folder.

 Especially, after that, we can remove all HAS_IOMEM and NO_IOMEM, nobody
 has to think of them again. :-)

 And then we end up with a solution that on UML a lot of completely useless
 drivers are build which fail in various interesting manners because you'll
 add stubs for all kinds of io memory related functions to arch/um/?
 We had this kind of discussion already. You'll need more than ioremap...

 I like Arnd's idea *much* more to make COMPILE_TEST depend on !UML.


 That will let UML itself against COMPILE_TEST (but all the other
 architectures not).

 And if let COMPILE_TEST depend on !UML, can we still remove all
 HAS_IOMEM and NO_IOMEM from kernel? (I guess so).

 If we can remove them, we can send related patch firstly -- that will
 let current discussion be in UML architecture wide instead of kernel
 wide.

 
 Next, I shall:
 
  - Remove HAS_IOMEM and NO_IOMEM from kernel, firstly.
 
  - Try to make dummy IOMEM functions for score architecture.
 
  - Continue discussing with UML for it.

 
Oh, sorry, I forgot, after remove IOMEM from kernel, also s390 and tile
need implement dummy IOMEM if !PCI.

If possible, I shall try to implement the dummy IOMEM in 'asm-generic',
and let uml, score, s390 and tile use them when they need.

 
 By the way: how about HAS_DMA? can we treat it as HAS_IOMEM (remove
 it from kernel)? (for me, I guess we can not).
 
 
 At present, I shall finish sending patch for removing IOMEM today, and
 continue to welcome any ideas, suggestions or completions for IOMEM or
 DMA.
 
 

Thanks.
-- 
Chen Gang

Open share and attitude like air water and life which God blessed
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] drivers: Let several drivers depends on HAS_IOMEM for 'devm_ioremap_resource'

2014-07-20 Thread Richard Weinberger 


Am 20.07.2014 10:38, schrieb Chen Gang:
 On 07/19/2014 02:02 AM, Chen Gang wrote:
 2014-07-18 18:51 GMT+08:00 Richard Weinberger rich...@nod.at:
 Am 18.07.2014 12:44, schrieb Chen Gang:
 On 07/18/2014 03:35 PM, Richard Weinberger wrote:
 Am 18.07.2014 02:36, schrieb Chen Gang:

 On 07/18/2014 02:09 AM, Richard Weinberger wrote:
 Am 17.07.2014 12:48, schrieb Arnd Bergmann:
 AFAICT, NO_IOMEM only has a real purpose on UML these days. Could we 
 take
 a shortcut here and make COMPILE_TEST depend on !UML? Getting random 
 stuff
 to build on UML seems pointless to me and we special-case it in a 
 number of
 places already.

 If UML is the only arch without io memory the dependency on !UML seems
 reasonable to me. :-)


 For me, if only uml left, I suggest to implement dummy functions within
 uml instead of let CONFIG_UML appear in generic include directory. And
 then remove all HAS_IOMEM and NO_IOMEM from kernel.

 Erm, this is something completely different.
 I thought we're focusing on COMPILE_TEST?


 COMPILE_TEST is none-architecture specific, but UML is. So in generic
 include folder, if we're focusing on choosing whether COMPILE_TEST or
 UML, for me, I will choose COMPILE_TEST.

 If we're not only focusing on COMPILE_TEST, for me, if something only
 depend on one architecture, I'd like to put them under arch/*/ folder.

 Especially, after that, we can remove all HAS_IOMEM and NO_IOMEM, nobody
 has to think of them again. :-)

 And then we end up with a solution that on UML a lot of completely useless
 drivers are build which fail in various interesting manners because you'll
 add stubs for all kinds of io memory related functions to arch/um/?
 We had this kind of discussion already. You'll need more than ioremap...

 I like Arnd's idea *much* more to make COMPILE_TEST depend on !UML.


 That will let UML itself against COMPILE_TEST (but all the other
 architectures not).

 And if let COMPILE_TEST depend on !UML, can we still remove all
 HAS_IOMEM and NO_IOMEM from kernel? (I guess so).

 If we can remove them, we can send related patch firstly -- that will
 let current discussion be in UML architecture wide instead of kernel
 wide.

 
 Next, I shall:
 
  - Remove HAS_IOMEM and NO_IOMEM from kernel, firstly.

This needs to be last, otherwise lot's of stuff will break.

  - Try to make dummy IOMEM functions for score architecture.
 
  - Continue discussing with UML for it.

If you find sane dummy functions for both UML and score I'm fine with it.

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

Re: [PATCH] drivers: Let several drivers depends on HAS_IOMEM for 'devm_ioremap_resource'

2014-07-20 Thread Chen Gang 
On 07/20/2014 05:45 PM, Richard Weinberger wrote:
 
 
 Am 20.07.2014 10:38, schrieb Chen Gang:
 On 07/19/2014 02:02 AM, Chen Gang wrote:
 2014-07-18 18:51 GMT+08:00 Richard Weinberger rich...@nod.at:
 Am 18.07.2014 12:44, schrieb Chen Gang:
 On 07/18/2014 03:35 PM, Richard Weinberger wrote:
 Am 18.07.2014 02:36, schrieb Chen Gang:

 On 07/18/2014 02:09 AM, Richard Weinberger wrote:
 Am 17.07.2014 12:48, schrieb Arnd Bergmann:
 AFAICT, NO_IOMEM only has a real purpose on UML these days. Could we 
 take
 a shortcut here and make COMPILE_TEST depend on !UML? Getting random 
 stuff
 to build on UML seems pointless to me and we special-case it in a 
 number of
 places already.

 If UML is the only arch without io memory the dependency on !UML seems
 reasonable to me. :-)


 For me, if only uml left, I suggest to implement dummy functions within
 uml instead of let CONFIG_UML appear in generic include directory. And
 then remove all HAS_IOMEM and NO_IOMEM from kernel.

 Erm, this is something completely different.
 I thought we're focusing on COMPILE_TEST?


 COMPILE_TEST is none-architecture specific, but UML is. So in generic
 include folder, if we're focusing on choosing whether COMPILE_TEST or
 UML, for me, I will choose COMPILE_TEST.

 If we're not only focusing on COMPILE_TEST, for me, if something only
 depend on one architecture, I'd like to put them under arch/*/ folder.

 Especially, after that, we can remove all HAS_IOMEM and NO_IOMEM, nobody
 has to think of them again. :-)

 And then we end up with a solution that on UML a lot of completely useless
 drivers are build which fail in various interesting manners because you'll
 add stubs for all kinds of io memory related functions to arch/um/?
 We had this kind of discussion already. You'll need more than ioremap...

 I like Arnd's idea *much* more to make COMPILE_TEST depend on !UML.


 That will let UML itself against COMPILE_TEST (but all the other
 architectures not).

 And if let COMPILE_TEST depend on !UML, can we still remove all
 HAS_IOMEM and NO_IOMEM from kernel? (I guess so).

 If we can remove them, we can send related patch firstly -- that will
 let current discussion be in UML architecture wide instead of kernel
 wide.


 Next, I shall:

  - Remove HAS_IOMEM and NO_IOMEM from kernel, firstly.
 
 This needs to be last, otherwise lot's of stuff will break.
 

OK, thanks, that sounds reasonable to me.

  - Try to make dummy IOMEM functions for score architecture.

  - Continue discussing with UML for it.
 
 If you find sane dummy functions for both UML and score I'm fine with it.
 

For me, what you said is necessary, tile and s390 also need dummy IOMEM
when !PCI.

So for me, I shall implement them in include/asm-generic, and let uml,
score, tile and s390 use dummy IOMEM when they need.

Welcome any other members ideas, suggestions and completions.


Thanks.
-- 
Chen Gang

Open share and attitude like air water and life which God blessed
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/3] staging: lustre/lustre/lov: Remove unneeded 'if' statement in lov_request.c/lov_check_set()

2014-07-20 Thread Riccardo Lucchese 
On Sat, Jul 19, 2014 at 12:59:22PM -0700, Joe Perches wrote:
 On Sat, 2014-07-19 at 21:34 +0200, Riccardo Lucchese wrote:
  It is silly to go through an if statement to set a single boolean
  value in function of a single boolean expression. In the function
  lov_check_set, assign the return value directly.
 []
  diff --git a/drivers/staging/lustre/lustre/lov/lov_request.c 
  b/drivers/staging/lustre/lustre/lov/lov_request.c
 []
  @@ -140,14 +140,13 @@ void lov_set_add_req(struct lov_request *req, struct 
  lov_request_set *set)
   
   static int lov_check_set(struct lov_obd *lov, int idx)
   {
  -   int rc = 0;
  +   int rc;
  mutex_lock(lov-lov_lock);
   
  -   if (lov-lov_tgts[idx] == NULL ||
  -   lov-lov_tgts[idx]-ltd_active ||
  -   (lov-lov_tgts[idx]-ltd_exp != NULL 
  -class_exp2cliimp(lov-lov_tgts[idx]-ltd_exp)-imp_connect_tried))
  -   rc = 1;
  +   rc = lov-lov_tgts[idx] == NULL ||
  +   lov-lov_tgts[idx]-ltd_active ||
  +   (lov-lov_tgts[idx]-ltd_exp != NULL 
  +
  class_exp2cliimp(lov-lov_tgts[idx]-ltd_exp)-imp_connect_tried);
   
  mutex_unlock(lov-lov_lock);
  return rc;
 
 Maybe consider using a temporary for lov-lov_tgtx[idx] like:
[...]

Indeed, it is nicer this way.

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

Re: [PATCH 1/3] staging: lustre/lustre/lov: Remove unneeded 'if' statement in lov_request.c/lov_check_set()

2014-07-20 Thread Dan Carpenter 
On Sun, Jul 20, 2014 at 01:08:36PM +0200, Riccardo Lucchese wrote:
 Dan,
 
 On Sun, Jul 20, 2014 at 07:52:53AM +0300, Dan Carpenter wrote:
  On Sat, Jul 19, 2014 at 09:34:56PM +0200, Riccardo Lucchese wrote:
   It is silly to go through an if statement to set a single boolean
   value in function of a single boolean expression. In the function
   lov_check_set, assign the return value directly.
   
   Signed-off-by: Riccardo Lucchese riccardo.lucch...@gmail.com
   ---
drivers/staging/lustre/lustre/lov/lov_request.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
   
   diff --git a/drivers/staging/lustre/lustre/lov/lov_request.c 
   b/drivers/staging/lustre/lustre/lov/lov_request.c
   index ce830e4..90fc66a 100644
   --- a/drivers/staging/lustre/lustre/lov/lov_request.c
   +++ b/drivers/staging/lustre/lustre/lov/lov_request.c
   @@ -140,14 +140,13 @@ void lov_set_add_req(struct lov_request *req, 
   struct lov_request_set *set)

static int lov_check_set(struct lov_obd *lov, int idx)
{
   - int rc = 0;
   + int rc;
 mutex_lock(lov-lov_lock);

   - if (lov-lov_tgts[idx] == NULL ||
   - lov-lov_tgts[idx]-ltd_active ||
   - (lov-lov_tgts[idx]-ltd_exp != NULL 
   -  class_exp2cliimp(lov-lov_tgts[idx]-ltd_exp)-imp_connect_tried))
   - rc = 1;
   + rc = lov-lov_tgts[idx] == NULL ||
   + lov-lov_tgts[idx]-ltd_active ||
   + (lov-lov_tgts[idx]-ltd_exp != NULL 
   +  
   class_exp2cliimp(lov-lov_tgts[idx]-ltd_exp)-imp_connect_tried);
  
  I don't see how this makes the code more readable at all.
 
 Thank you for the comment. Would you consider something like the
 following diff instead ? Otherwise, I will resend the series for
 review without this change.
 
 riccardo
 
 ---
 
 diff --git a/drivers/staging/lustre/lustre/lov/lov_request.c 
 b/drivers/staging/lustre/lustre/lov/lov_request.c
 index ce830e4..ae670bb 100644
 --- a/drivers/staging/lustre/lustre/lov/lov_request.c
 +++ b/drivers/staging/lustre/lustre/lov/lov_request.c
 @@ -140,14 +140,14 @@ void lov_set_add_req(struct lov_request *req, struct 
 lov_request_set *set)
  
  static int lov_check_set(struct lov_obd *lov, int idx)
  {
 - int rc = 0;
 + int rc;
 + struct lov_tgt_desc *desc;
   mutex_lock(lov-lov_lock);
  
 - if (lov-lov_tgts[idx] == NULL ||
 - lov-lov_tgts[idx]-ltd_active ||
 - (lov-lov_tgts[idx]-ltd_exp != NULL 
 -  class_exp2cliimp(lov-lov_tgts[idx]-ltd_exp)-imp_connect_tried))
 - rc = 1;
 + desc = lov-lov_tgts[idx];
 + rc = !desc || desc-ltd_active ||
 + (desc-ltd_exp 
 +  class_exp2cliimp(desc-ltd_exp)-imp_connect_tried);

Sure, I suppose.  Using desc is a clean up.  Otherwise the original
code was not silly.  It was fine.

I'm curious why you think if statements are less readable than other
statements.  That seems like nonsense.

regards,
dan carpenter


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

Re: [PATCH 1/3] staging: lustre/lustre/lov: Remove unneeded 'if' statement in lov_request.c/lov_check_set()

2014-07-20 Thread Riccardo Lucchese 
On Sun, Jul 20, 2014 at 02:37:47PM +0300, Dan Carpenter wrote:
 On Sun, Jul 20, 2014 at 01:08:36PM +0200, Riccardo Lucchese wrote:
  Dan,
  
  On Sun, Jul 20, 2014 at 07:52:53AM +0300, Dan Carpenter wrote:
   On Sat, Jul 19, 2014 at 09:34:56PM +0200, Riccardo Lucchese wrote:
[...]
   I don't see how this makes the code more readable at all.
  
  Thank you for the comment. Would you consider something like the
  following diff instead ? Otherwise, I will resend the series for
  review without this change.
  
  riccardo
  
  ---
  
  diff --git a/drivers/staging/lustre/lustre/lov/lov_request.c 
  b/drivers/staging/lustre/lustre/lov/lov_request.c
  index ce830e4..ae670bb 100644
  --- a/drivers/staging/lustre/lustre/lov/lov_request.c
  +++ b/drivers/staging/lustre/lustre/lov/lov_request.c
  @@ -140,14 +140,14 @@ void lov_set_add_req(struct lov_request *req, struct 
  lov_request_set *set)
   
   static int lov_check_set(struct lov_obd *lov, int idx)
   {
  -   int rc = 0;
  +   int rc;
  +   struct lov_tgt_desc *desc;
  mutex_lock(lov-lov_lock);
   
  -   if (lov-lov_tgts[idx] == NULL ||
  -   lov-lov_tgts[idx]-ltd_active ||
  -   (lov-lov_tgts[idx]-ltd_exp != NULL 
  -class_exp2cliimp(lov-lov_tgts[idx]-ltd_exp)-imp_connect_tried))
  -   rc = 1;
  +   desc = lov-lov_tgts[idx];
  +   rc = !desc || desc-ltd_active ||
  +   (desc-ltd_exp 
  +class_exp2cliimp(desc-ltd_exp)-imp_connect_tried);
 
 Sure, I suppose.  Using desc is a clean up.  Otherwise the original
 code was not silly.  It was fine.

The adjective silly was inappropriate and misleading, sorry about
that.

 I'm curious why you think if statements are less readable than other
 statements.  That seems like nonsense.

Not in general but, in this case, I find the patched code clearer.

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

[PATCH 3/5] Staging: bcm: nvm.c: Removed indentation level by using continue statement

2014-07-20 Thread Matthias Beyer 
Signed-off-by: Matthias Beyer m...@beyermatthias.de
---
 drivers/staging/bcm/nvm.c | 25 +
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/bcm/nvm.c b/drivers/staging/bcm/nvm.c
index 4aa195c..edbd0f9 100644
--- a/drivers/staging/bcm/nvm.c
+++ b/drivers/staging/bcm/nvm.c
@@ -1049,22 +1049,23 @@ static int bulk_read_complete_sector(struct 
bcm_mini_adapter *ad,
 offset + i,
 MAX_RW_SIZE);
 
-   if (bulk_read_stat == STATUS_SUCCESS) {
-   if (ad-ulFlashWriteSize == 1) {
-   for (j = 0; j  16; j++) {
-   if (read_bk[j] != tmpbuff[i+j]) {
-   if (STATUS_SUCCESS != 
(*ad-fpFlashWriteWithStatusCheck)(ad, partoff + i + j, tmpbuff[i+j])) {
-   return STATUS_FAILURE;
-   }
-   }
-   }
-   } else {
-   if (memcmp(read_bk, tmpbuff[i], MAX_RW_SIZE)) {
-   if (STATUS_SUCCESS != 
(*ad-fpFlashWriteWithStatusCheck)(ad, partoff + i, tmpbuff[i])) {
+   if (bulk_read_stat != STATUS_SUCCESS)
+   continue;
+
+   if (ad-ulFlashWriteSize == 1) {
+   for (j = 0; j  16; j++) {
+   if (read_bk[j] != tmpbuff[i+j]) {
+   if (STATUS_SUCCESS != 
(*ad-fpFlashWriteWithStatusCheck)(ad, partoff + i + j, tmpbuff[i+j])) {
return STATUS_FAILURE;
}
}
}
+   } else {
+   if (memcmp(read_bk, tmpbuff[i], MAX_RW_SIZE)) {
+   if (STATUS_SUCCESS != 
(*ad-fpFlashWriteWithStatusCheck)(ad, partoff + i, tmpbuff[i])) {
+   return STATUS_FAILURE;
+   }
+   }
}
}
 
-- 
2.0.2

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

[PATCH 2/5] Staging: bcm: nvm.c: Outsourced chunk of code into function

2014-07-20 Thread Matthias Beyer 
This patch outsources a chunk of code into an own function. It also
refactors the variable names which are used within this function.

The function name may be not appropriate.

Signed-off-by: Matthias Beyer m...@beyermatthias.de
---
 drivers/staging/bcm/nvm.c | 70 ---
 1 file changed, 48 insertions(+), 22 deletions(-)

diff --git a/drivers/staging/bcm/nvm.c b/drivers/staging/bcm/nvm.c
index 76c86eb..4aa195c 100644
--- a/drivers/staging/bcm/nvm.c
+++ b/drivers/staging/bcm/nvm.c
@@ -1033,6 +1033,44 @@ static ULONG BcmFlashUnProtectBlock(struct 
bcm_mini_adapter *Adapter, unsigned i
return ulStatus;
 }
 
+static int bulk_read_complete_sector(struct bcm_mini_adapter *ad,
+UCHAR read_bk[],
+PCHAR tmpbuff,
+unsigned int offset,
+unsigned int partoff,
+unsigned int i)
+{
+   unsigned int j;
+   int bulk_read_stat;
+
+   for (i = 0; i  ad-uiSectorSize; i += MAX_RW_SIZE) {
+   bulk_read_stat = BeceemFlashBulkRead(ad,
+(PUINT)read_bk,
+offset + i,
+MAX_RW_SIZE);
+
+   if (bulk_read_stat == STATUS_SUCCESS) {
+   if (ad-ulFlashWriteSize == 1) {
+   for (j = 0; j  16; j++) {
+   if (read_bk[j] != tmpbuff[i+j]) {
+   if (STATUS_SUCCESS != 
(*ad-fpFlashWriteWithStatusCheck)(ad, partoff + i + j, tmpbuff[i+j])) {
+   return STATUS_FAILURE;
+   }
+   }
+   }
+   } else {
+   if (memcmp(read_bk, tmpbuff[i], MAX_RW_SIZE)) {
+   if (STATUS_SUCCESS != 
(*ad-fpFlashWriteWithStatusCheck)(ad, partoff + i, tmpbuff[i])) {
+   return STATUS_FAILURE;
+   }
+   }
+   }
+   }
+   }
+
+   return STATUS_SUCCESS;
+}
+
 /*
  * Procedure:  BeceemFlashBulkWrite
  *
@@ -1169,29 +1207,17 @@ static int BeceemFlashBulkWrite(struct bcm_mini_adapter 
*Adapter,
/* do_gettimeofday(tw);
 * BCM_DEBUG_PRINT(Adapter,DBG_TYPE_PRINTK, 0, 0, Total time 
taken in Write  to Flash :%ld ms\n, (tw.tv_sec *1000 + tw.tv_usec/1000) - 
(te.tv_sec *1000 + te.tv_usec/1000));
 */
-   for (uiIndex = 0; uiIndex  Adapter-uiSectorSize; uiIndex += 
MAX_RW_SIZE) {
-   if (STATUS_SUCCESS == BeceemFlashBulkRead(Adapter, 
(PUINT)ucReadBk, uiOffsetFromSectStart + uiIndex, MAX_RW_SIZE)) {
-   if (Adapter-ulFlashWriteSize == 1) {
-   unsigned int uiReadIndex = 0;
-
-   for (uiReadIndex = 0; uiReadIndex  16; 
uiReadIndex++) {
-   if (ucReadBk[uiReadIndex] != 
pTempBuff[uiIndex + uiReadIndex]) {
-   if (STATUS_SUCCESS != 
(*Adapter-fpFlashWriteWithStatusCheck)(Adapter, uiPartOffset + uiIndex + 
uiReadIndex, pTempBuff[uiIndex+uiReadIndex])) {
-   Status = 
STATUS_FAILURE;
-   goto 
BeceemFlashBulkWrite_EXIT;
-   }
-   }
-   }
-   } else {
-   if (memcmp(ucReadBk, 
pTempBuff[uiIndex], MAX_RW_SIZE)) {
-   if (STATUS_SUCCESS != 
(*Adapter-fpFlashWriteWithStatusCheck)(Adapter, uiPartOffset + uiIndex, 
pTempBuff[uiIndex])) {
-   Status = STATUS_FAILURE;
-   goto 
BeceemFlashBulkWrite_EXIT;
-   }
-   }
-   }
-   }
+
+   if (STATUS_FAILURE == bulk_read_complete_sector(Adapter,
+   ucReadBk,
+   pTempBuff,
+   
uiOffsetFromSectStart,
+   uiPartOffset,
+

[PATCH 1/5] Staging: bcm: nvm.c: Shortened lines

2014-07-20 Thread Matthias Beyer 
Signed-off-by: Matthias Beyer m...@beyermatthias.de
---
 drivers/staging/bcm/nvm.c | 64 +--
 1 file changed, 40 insertions(+), 24 deletions(-)

diff --git a/drivers/staging/bcm/nvm.c b/drivers/staging/bcm/nvm.c
index 8ce4d41..76c86eb 100644
--- a/drivers/staging/bcm/nvm.c
+++ b/drivers/staging/bcm/nvm.c
@@ -2,35 +2,52 @@
 
 #define DWORD unsigned int
 
-static int BcmDoChipSelect(struct bcm_mini_adapter *Adapter, unsigned int 
offset);
+static int BcmDoChipSelect(struct bcm_mini_adapter *Adapter,
+  unsigned int offset);
 static int BcmGetActiveDSD(struct bcm_mini_adapter *Adapter);
 static int BcmGetActiveISO(struct bcm_mini_adapter *Adapter);
 static unsigned int BcmGetEEPROMSize(struct bcm_mini_adapter *Adapter);
 static int BcmGetFlashCSInfo(struct bcm_mini_adapter *Adapter);
-static unsigned int BcmGetFlashSectorSize(struct bcm_mini_adapter *Adapter, 
unsigned int FlashSectorSizeSig, unsigned int FlashSectorSize);
+static unsigned int BcmGetFlashSectorSize(struct bcm_mini_adapter *Adapter,
+ unsigned int FlashSectorSizeSig,
+ unsigned int FlashSectorSize);
 
 static VOID BcmValidateNvmType(struct bcm_mini_adapter *Adapter);
 static int BcmGetNvmSize(struct bcm_mini_adapter *Adapter);
 static unsigned int BcmGetFlashSize(struct bcm_mini_adapter *Adapter);
 static enum bcm_nvm_type BcmGetNvmType(struct bcm_mini_adapter *Adapter);
 
-static int BcmGetSectionValEndOffset(struct bcm_mini_adapter *Adapter, enum 
bcm_flash2x_section_val eFlash2xSectionVal);
-
-static B_UINT8 IsOffsetWritable(struct bcm_mini_adapter *Adapter, unsigned int 
uiOffset);
-static int IsSectionWritable(struct bcm_mini_adapter *Adapter, enum 
bcm_flash2x_section_val Section);
-static int IsSectionExistInVendorInfo(struct bcm_mini_adapter *Adapter, enum 
bcm_flash2x_section_val section);
-
-static int ReadDSDPriority(struct bcm_mini_adapter *Adapter, enum 
bcm_flash2x_section_val dsd);
-static int ReadDSDSignature(struct bcm_mini_adapter *Adapter, enum 
bcm_flash2x_section_val dsd);
-static int ReadISOPriority(struct bcm_mini_adapter *Adapter, enum 
bcm_flash2x_section_val iso);
-static int ReadISOSignature(struct bcm_mini_adapter *Adapter, enum 
bcm_flash2x_section_val iso);
-
-static int CorruptDSDSig(struct bcm_mini_adapter *Adapter, enum 
bcm_flash2x_section_val eFlash2xSectionVal);
-static int CorruptISOSig(struct bcm_mini_adapter *Adapter, enum 
bcm_flash2x_section_val eFlash2xSectionVal);
-static int SaveHeaderIfPresent(struct bcm_mini_adapter *Adapter, PUCHAR pBuff, 
unsigned int uiSectAlignAddr);
-static int WriteToFlashWithoutSectorErase(struct bcm_mini_adapter *Adapter, 
PUINT pBuff,
-   enum bcm_flash2x_section_val 
eFlash2xSectionVal,
-   unsigned int uiOffset, unsigned int 
uiNumBytes);
+static int BcmGetSectionValEndOffset(struct bcm_mini_adapter *Adapter,
+enum bcm_flash2x_section_val 
eFlash2xSectionVal);
+
+static B_UINT8 IsOffsetWritable(struct bcm_mini_adapter *Adapter,
+   unsigned int uiOffset);
+static int IsSectionWritable(struct bcm_mini_adapter *Adapter,
+enum bcm_flash2x_section_val Section);
+static int IsSectionExistInVendorInfo(struct bcm_mini_adapter *Adapter,
+ enum bcm_flash2x_section_val section);
+
+static int ReadDSDPriority(struct bcm_mini_adapter *Adapter,
+  enum bcm_flash2x_section_val dsd);
+static int ReadDSDSignature(struct bcm_mini_adapter *Adapter,
+   enum bcm_flash2x_section_val dsd);
+static int ReadISOPriority(struct bcm_mini_adapter *Adapter,
+  enum bcm_flash2x_section_val iso);
+static int ReadISOSignature(struct bcm_mini_adapter *Adapter,
+   enum bcm_flash2x_section_val iso);
+
+static int CorruptDSDSig(struct bcm_mini_adapter *Adapter,
+enum bcm_flash2x_section_val eFlash2xSectionVal);
+static int CorruptISOSig(struct bcm_mini_adapter *Adapter,
+enum bcm_flash2x_section_val eFlash2xSectionVal);
+static int SaveHeaderIfPresent(struct bcm_mini_adapter *Adapter,
+  PUCHAR pBuff,
+  unsigned int uiSectAlignAddr);
+static int WriteToFlashWithoutSectorErase(struct bcm_mini_adapter *Adapter,
+ PUINT pBuff,
+ enum bcm_flash2x_section_val 
eFlash2xSectionVal,
+ unsigned int uiOffset,
+ unsigned int uiNumBytes);
 static enum bcm_flash2x_section_val getHighestPriDSD(struct bcm_mini_adapter 
*Adapter);
 static enum bcm_flash2x_section_val getHighestPriISO(struct bcm_mini_adapter 
*Adapter);
 
@@

[PATCH v2 1/3] staging: lustre: lov: Cleanup lov_check_set() in lov_request.c

2014-07-20 Thread Riccardo Lucchese 
Make the code clearer by introducing a local variable and removing the
unnecessary 'if' statement.

Signed-off-by: Riccardo Lucchese riccardo.lucch...@gmail.com
Acked-by: Julia Lawall julia.law...@lip6.fr
---
Changes in v2:
 - Improved the commit message:
- changed the subject line to follow the same convention used by
  previous commits to the same file.
- rewrote the message body using a more formal language.
 - Introduced a local variable to make the code clearer.
 - Added Acked-by - Julia.

 drivers/staging/lustre/lustre/lov/lov_request.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/lustre/lustre/lov/lov_request.c 
b/drivers/staging/lustre/lustre/lov/lov_request.c
index ce830e4..ae670bb 100644
--- a/drivers/staging/lustre/lustre/lov/lov_request.c
+++ b/drivers/staging/lustre/lustre/lov/lov_request.c
@@ -140,14 +140,14 @@ void lov_set_add_req(struct lov_request *req, struct 
lov_request_set *set)
 
 static int lov_check_set(struct lov_obd *lov, int idx)
 {
-   int rc = 0;
+   int rc;
+   struct lov_tgt_desc *desc;
mutex_lock(lov-lov_lock);
 
-   if (lov-lov_tgts[idx] == NULL ||
-   lov-lov_tgts[idx]-ltd_active ||
-   (lov-lov_tgts[idx]-ltd_exp != NULL 
-class_exp2cliimp(lov-lov_tgts[idx]-ltd_exp)-imp_connect_tried))
-   rc = 1;
+   desc = lov-lov_tgts[idx];
+   rc = !desc || desc-ltd_active ||
+   (desc-ltd_exp 
+class_exp2cliimp(desc-ltd_exp)-imp_connect_tried);
 
mutex_unlock(lov-lov_lock);
return rc;
-- 
1.9.1

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

Re: [PATCH v2 1/3] staging: lustre: lov: Cleanup lov_check_set() in lov_request.c

2014-07-20 Thread Riccardo Lucchese 
On Sun, Jul 20, 2014 at 08:30:23AM -0700, Joe Perches wrote:
 On Sun, 2014-07-20 at 15:22 +0200, Riccardo Lucchese wrote:
  Make the code clearer by introducing a local variable and removing the
  unnecessary 'if' statement.
  
  Signed-off-by: Riccardo Lucchese riccardo.lucch...@gmail.com
  Acked-by: Julia Lawall julia.law...@lip6.fr
  ---
  Changes in v2:
   - Improved the commit message:
  - changed the subject line to follow the same convention used by
previous commits to the same file.
  - rewrote the message body using a more formal language.
   - Introduced a local variable to make the code clearer.
   - Added Acked-by - Julia.
  
   drivers/staging/lustre/lustre/lov/lov_request.c | 12 ++--
   1 file changed, 6 insertions(+), 6 deletions(-)
  
  diff --git a/drivers/staging/lustre/lustre/lov/lov_request.c 
  b/drivers/staging/lustre/lustre/lov/lov_request.c
  index ce830e4..ae670bb 100644
  --- a/drivers/staging/lustre/lustre/lov/lov_request.c
  +++ b/drivers/staging/lustre/lustre/lov/lov_request.c
  @@ -140,14 +140,14 @@ void lov_set_add_req(struct lov_request *req, struct 
  lov_request_set *set)
   
   static int lov_check_set(struct lov_obd *lov, int idx)
   {
  -   int rc = 0;
  +   int rc;
  +   struct lov_tgt_desc *desc;
  mutex_lock(lov-lov_lock);
   
  -   if (lov-lov_tgts[idx] == NULL ||
  -   lov-lov_tgts[idx]-ltd_active ||
  -   (lov-lov_tgts[idx]-ltd_exp != NULL 
  -class_exp2cliimp(lov-lov_tgts[idx]-ltd_exp)-imp_connect_tried))
  -   rc = 1;
  +   desc = lov-lov_tgts[idx];
  +   rc = !desc || desc-ltd_active ||
  +   (desc-ltd_exp 
  +class_exp2cliimp(desc-ltd_exp)-imp_connect_tried);
   
  mutex_unlock(lov-lov_lock);
  return rc;
 
 desc is not a good temporary name here.
 
 For consistency with the rest of the code
 please use tgt
 
 $ git grep = lov-lov_tgts.*];
 drivers/staging/lustre/lustre/lov/lov_obd.c:tgt = 
 lov-lov_tgts[i];
 drivers/staging/lustre/lustre/lov/lov_obd.c:tgt = 
 lov-lov_tgts[i];
 drivers/staging/lustre/lustre/lov/lov_obd.c:tgt = 
 lov-lov_tgts[index];
 drivers/staging/lustre/lustre/lov/lov_obd.c:tgt = 
 lov-lov_tgts[index];
 drivers/staging/lustre/lustre/lov/lov_obd.c:tgt = 
 lov-lov_tgts[qctl-qc_idx];
 drivers/staging/lustre/lustre/lov/lov_obd.c:tgt = 
 lov-lov_tgts[i];
 drivers/staging/lustre/lustre/lov/lov_obd.c:tgt = 
 lov-lov_tgts[info-idx];
 drivers/staging/lustre/lustre/lov/lov_obd.c:tgt = 
 lov-lov_tgts[ost_idx];
 drivers/staging/lustre/lustre/lov/lov_obd.c:tgt = 
 lov-lov_tgts[((struct obd_id_info *)val)-idx];
 drivers/staging/lustre/lustre/lov/lov_obd.c:tgt = 
 lov-lov_tgts[i];
 drivers/staging/lustre/lustre/lov/lov_obd.c:tgt = 
 lov-lov_tgts[i];
 drivers/staging/lustre/lustre/lov/lov_request.c:tgt = 
 lov-lov_tgts[ost_idx];
 drivers/staging/lustre/lustre/lov/lov_request.c:tgt = 
 lov-lov_tgts[lovreq-rq_idx];
 

Thank you for noticing this. I will send a v3.

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

Re: [PATCH v4 (staging-next) 1/5] staging:iio:hmc5843: Added regmap support

2014-07-20 Thread Jonathan Cameron 

On 16/07/14 14:07, Josef Gajdusek wrote:

This patch changes hmc5843.c to use regmap. This provides transparent caching
to the code as well as abstraction necessary to add support for SPI-based
hmc5983.

Signed-off-by: Josef Gajdusek a...@atx.name

Applied to the togreg branch of iio.git.  Will be initially pushed out as
testing for the autobuilders to mess with it.

Thanks,


---
  drivers/staging/iio/magnetometer/Kconfig   |   1 +
  drivers/staging/iio/magnetometer/hmc5843.c | 140 +++--
  2 files changed, 96 insertions(+), 45 deletions(-)

diff --git a/drivers/staging/iio/magnetometer/Kconfig 
b/drivers/staging/iio/magnetometer/Kconfig
index 34634da..ad88d61 100644
--- a/drivers/staging/iio/magnetometer/Kconfig
+++ b/drivers/staging/iio/magnetometer/Kconfig
@@ -8,6 +8,7 @@ config SENSORS_HMC5843
depends on I2C
select IIO_BUFFER
select IIO_TRIGGERED_BUFFER
+   select REGMAP_I2C
help
  Say Y here to add support for the Honeywell HMC5843, HMC5883 and
  HMC5883L 3-Axis Magnetometer (digital compass).
diff --git a/drivers/staging/iio/magnetometer/hmc5843.c 
b/drivers/staging/iio/magnetometer/hmc5843.c
index f595fdc..6f06f98 100644
--- a/drivers/staging/iio/magnetometer/hmc5843.c
+++ b/drivers/staging/iio/magnetometer/hmc5843.c
@@ -21,6 +21,7 @@

  #include linux/module.h
  #include linux/i2c.h
+#include linux/regmap.h
  #include linux/iio/iio.h
  #include linux/iio/sysfs.h
  #include linux/iio/trigger_consumer.h
@@ -34,6 +35,7 @@
  #define HMC5843_DATA_OUT_MSB_REGS 0x03
  #define HMC5843_STATUS_REG0x09
  #define HMC5843_ID_REG0x0a
+#define HMC5843_ID_END 0x0c

  enum hmc5843_ids {
HMC5843_ID,
@@ -49,6 +51,7 @@ enum hmc5843_ids {
  #define HMC5843_RANGE_GAIN_OFFSET 0x05
  #define HMC5843_RANGE_GAIN_DEFAULT0x01
  #define HMC5843_RANGE_GAINS   8
+#define HMC5843_RANGE_GAIN_MASK0xe0

  /* Device status */
  #define HMC5843_DATA_READY0x01
@@ -68,6 +71,7 @@ enum hmc5843_ids {
  #define HMC5843_RATE_OFFSET   0x02
  #define HMC5843_RATE_DEFAULT  0x04
  #define HMC5843_RATES 7
+#define HMC5843_RATE_MASK  0x1c

  /* Device measurement configuration */
  #define HMC5843_MEAS_CONF_NORMAL  0x00
@@ -121,10 +125,7 @@ struct hmc5843_chip_info {
  struct hmc5843_data {
struct i2c_client *client;
struct mutex lock;
-   u8 rate;
-   u8 meas_conf;
-   u8 operating_mode;
-   u8 range;
+   struct regmap *regmap;
const struct hmc5843_chip_info *variant;
__be16 buffer[8]; /* 3x 16-bit channels + padding + 64-bit timestamp */
  };
@@ -135,10 +136,8 @@ static s32 hmc5843_set_mode(struct hmc5843_data *data, u8 
operating_mode)
int ret;

mutex_lock(data-lock);
-   ret = i2c_smbus_write_byte_data(data-client, HMC5843_MODE_REG,
-   operating_mode  HMC5843_MODE_MASK);
-   if (ret = 0)
-   data-operating_mode = operating_mode;
+   ret = regmap_update_bits(data-regmap, HMC5843_MODE_REG,
+   HMC5843_MODE_MASK, operating_mode);
mutex_unlock(data-lock);

return ret;
@@ -146,15 +145,15 @@ static s32 hmc5843_set_mode(struct hmc5843_data *data, u8 
operating_mode)

  static int hmc5843_wait_measurement(struct hmc5843_data *data)
  {
-   s32 result;
int tries = 150;
+   int val;
+   int ret;

while (tries--  0) {
-   result = i2c_smbus_read_byte_data(data-client,
-   HMC5843_STATUS_REG);
-   if (result  0)
-   return result;
-   if (result  HMC5843_DATA_READY)
+   ret = regmap_read(data-regmap, HMC5843_STATUS_REG, val);
+   if (ret  0)
+   return ret;
+   if (val  HMC5843_DATA_READY)
break;
msleep(20);
}
@@ -171,20 +170,20 @@ static int hmc5843_wait_measurement(struct hmc5843_data 
*data)
  static int hmc5843_read_measurement(struct hmc5843_data *data,
int idx, int *val)
  {
-   s32 result;
__be16 values[3];
+   int ret;

mutex_lock(data-lock);
-   result = hmc5843_wait_measurement(data);
-   if (result  0) {
+   ret = hmc5843_wait_measurement(data);
+   if (ret  0) {
mutex_unlock(data-lock);
-   return result;
+   return ret;
}
-   result = i2c_smbus_read_i2c_block_data(data-client,
-   HMC5843_DATA_OUT_MSB_REGS, sizeof(values), (u8 *) values);
+   ret = regmap_bulk_read(data-regmap, HMC5843_DATA_OUT_MSB_REGS,
+   values, sizeof(values));
mutex_unlock(data-lock);
-   if (result  0)
-

Re: [PATCH v4 (staging-next) 2/5] staging:iio:hmc5843: Split hmc5843.c to multiple files

2014-07-20 Thread Jonathan Cameron 

On 20/07/14 17:32, Jonathan Cameron wrote:

On 16/07/14 14:07, Josef Gajdusek wrote:

This patch splits hmc5843.c to multiple files - the interface-agnostic
hmc5843_core.c, i2c specific hmc5843_i2c.c and header file hmc5843.h. This is
another step to add support of SPI-enabled hmc5983.

Signed-off-by: Josef Gajdusek a...@atx.name

Unfortunately taking the tables non static just causes sparse to complain:

drivers/staging/iio/magnetometer/hmc5843.h:62:28: warning: symbol 
'hmc5843_readable_table' was not declared. Should it be static?
drivers/staging/iio/magnetometer/hmc5843.h:71:28: warning: symbol 
'hmc5843_writable_table' was not declared. Should it be static?
drivers/staging/iio/magnetometer/hmc5843.h:80:28: warning: symbol 
'hmc5843_volatile_table' was not declared. Should it be static?
   CC [M]  drivers/staging/iio/magnetometer/hmc5843_core.o
   CHECK   drivers/staging/iio/magnetometer/hmc5843_i2c.c
drivers/staging/iio/magnetometer/hmc5843.h:62:28: warning: symbol 
'hmc5843_readable_table' was not declared. Should it be static?
drivers/staging/iio/magnetometer/hmc5843.h:71:28: warning: symbol 
'hmc5843_writable_table' was not declared. Should it be static?
drivers/staging/iio/magnetometer/hmc5843.h:80:28: warning: symbol 
'hmc5843_volatile_table' was not declared. Should it be static?
   CC [M]  drivers/staging/iio/magnetometer/hmc5843_i2c.o

I think you'll probably just have to duplicate them (which is irritating, but 
there we are).

Jonathan

p.s. For now I've backed out patch 1 as well so that we can keep this as a
clean series rather than splitting it through time!

---
  drivers/staging/iio/magnetometer/Kconfig   |  14 +-
  drivers/staging/iio/magnetometer/Makefile  |   3 +-
  drivers/staging/iio/magnetometer/hmc5843.h |  85 
  .../iio/magnetometer/{hmc5843.c = hmc5843_core.c} | 154 +
  drivers/staging/iio/magnetometer/hmc5843_i2c.c |  75 ++
  5 files changed, 203 insertions(+), 128 deletions(-)
  create mode 100644 drivers/staging/iio/magnetometer/hmc5843.h
  rename drivers/staging/iio/magnetometer/{hmc5843.c = hmc5843_core.c} (79%)
  create mode 100644 drivers/staging/iio/magnetometer/hmc5843_i2c.c

diff --git a/drivers/staging/iio/magnetometer/Kconfig 
b/drivers/staging/iio/magnetometer/Kconfig
index ad88d61..0a27f98 100644
--- a/drivers/staging/iio/magnetometer/Kconfig
+++ b/drivers/staging/iio/magnetometer/Kconfig
@@ -4,16 +4,22 @@
  menu Magnetometer sensors

  config SENSORS_HMC5843
-tristate Honeywell HMC5843/5883/5883L 3-Axis Magnetometer
-depends on I2C
+tristate
  select IIO_BUFFER
  select IIO_TRIGGERED_BUFFER
+
+config SENSORS_HMC5843_I2C
+tristate Honeywell HMC5843/5883/5883L 3-Axis Magnetometer (I2C)
+depends on I2C
+select SENSORS_HMC5843
  select REGMAP_I2C
  help
Say Y here to add support for the Honeywell HMC5843, HMC5883 and
HMC5883L 3-Axis Magnetometer (digital compass).

-  To compile this driver as a module, choose M here: the module
-  will be called hmc5843.
+  This driver can also be compiled as a set of modules.
+  If so, these modules will be created:
+  - hmc5843_core (core functions)
+  - hmc5843_i2c (support for HMC5843, HMC5883 and HMC5883L)

  endmenu
diff --git a/drivers/staging/iio/magnetometer/Makefile 
b/drivers/staging/iio/magnetometer/Makefile
index f9bfb2e..65baf1c 100644
--- a/drivers/staging/iio/magnetometer/Makefile
+++ b/drivers/staging/iio/magnetometer/Makefile
@@ -2,4 +2,5 @@
  # Makefile for industrial I/O Magnetometer sensors
  #

-obj-$(CONFIG_SENSORS_HMC5843)+= hmc5843.o
+obj-$(CONFIG_SENSORS_HMC5843)+= hmc5843_core.o
+obj-$(CONFIG_SENSORS_HMC5843_I2C)+= hmc5843_i2c.o
diff --git a/drivers/staging/iio/magnetometer/hmc5843.h 
b/drivers/staging/iio/magnetometer/hmc5843.h
new file mode 100644
index 000..0d9b02e
--- /dev/null
+++ b/drivers/staging/iio/magnetometer/hmc5843.h
@@ -0,0 +1,85 @@
+/*
+ * Header file for hmc5843 driver
+ *
+ * Split from hmc5843.c
+ * Copyright (C) Josef Gajdusek a...@atx.name
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * */
+
+
+#ifndef HMC5843_CORE_H
+#define HMC5843_CORE_H
+
+#include linux/regmap.h
+#include linux/iio/iio.h
+
+#define HMC5843_CONFIG_REG_A0x00
+#define HMC5843_CONFIG_REG_B0x01
+#define HMC5843_MODE_REG0x02
+#define HMC5843_DATA_OUT_MSB_REGS0x03
+#define HMC5843_STATUS_REG0x09
+#define HMC5843_ID_REG0x0a
+#define HMC5843_ID_END0x0c
+
+enum hmc5843_ids {
+HMC5843_ID,
+HMC5883_ID,
+HMC5883L_ID,
+};
+
+struct hmc5843_data {
+struct device *dev;
+struct mutex lock;
+struct regmap *regmap;
+const struct hmc5843_chip_info *variant;
+__be16 buffer[8]; /* 3x 16-bit channels +

[GIT PULL] Staging driver fixes for 3.16-rc6

2014-07-20 Thread Greg KH 
The following changes since commit 1795cd9b3a91d4b5473c97f491d63892442212ab:

  Linux 3.16-rc5 (2014-07-13 14:04:33 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git/ 
tags/staging-3.16-rc6

for you to fetch changes up to 9359003385a2faffa502d201771d45624037a4cd:

  Merge tag 'iio-fixes-for-3.16d' of 
git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio into staging-linus 
(2014-07-13 15:47:26 -0700)


Staging fixes for 3.16-rc6

Here are 2 IIO driver fixes for 3.16-rc6 that resolve some reported issues.

Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org


Greg Kroah-Hartman (1):
  Merge tag 'iio-fixes-for-3.16d' of git://git.kernel.org/.../jic23/iio 
into staging-linus

Martin Fuzzey (1):
  iio: mma8452: Use correct acceleration units.

Srinivas Pandruvada (1):
  iio:core: Handle error when mask type is not separate

 drivers/iio/accel/mma8452.c  | 8 +++-
 drivers/iio/industrialio-event.c | 3 +++
 2 files changed, 10 insertions(+), 1 deletion(-)
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel