diff mbox series

[v3,3/5] bus: hisi_lpc: Clean some types

Message ID 1572888139-47298-4-git-send-email-john.garry@huawei.com
State Accepted
Commit 663accf1872b22c5eff05235d4750f3a253f5158
Headers show
Series hisi_lpc: Improve build test coverage | expand

Commit Message

John Garry Nov. 4, 2019, 5:22 p.m. UTC 
Sparse complains of these:
drivers/bus/hisi_lpc.c:82:38: warning: incorrect type in argument 1 (different address spaces)
drivers/bus/hisi_lpc.c:82:38:    expected void const volatile [noderef] <asn:2>*addr
drivers/bus/hisi_lpc.c:82:38:    got unsigned char *
drivers/bus/hisi_lpc.c:131:35: warning: incorrect type in argument 1 (different address spaces)
drivers/bus/hisi_lpc.c:131:35:    expected unsigned char *mbase
drivers/bus/hisi_lpc.c:131:35:    got void [noderef] <asn:2>*membase
drivers/bus/hisi_lpc.c:186:35: warning: incorrect type in argument 1 (different address spaces)
drivers/bus/hisi_lpc.c:186:35:    expected unsigned char *mbase
drivers/bus/hisi_lpc.c:186:35:    got void [noderef] <asn:2>*membase
drivers/bus/hisi_lpc.c:228:16: warning: cast to restricted __le32
drivers/bus/hisi_lpc.c:251:13: warning: incorrect type in assignment (different base types)
drivers/bus/hisi_lpc.c:251:13:    expected unsigned int [unsigned] [usertype] val
drivers/bus/hisi_lpc.c:251:13:    got restricted __le32 [usertype] <noident>

Clean them up.

Signed-off-by: John Garry <john.garry@huawei.com>

---
 drivers/bus/hisi_lpc.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

-- 
2.17.1

Comments

Joe Perches Nov. 4, 2019, 6:39 p.m. UTC | #1 
On Tue, 2019-11-05 at 01:22 +0800, John Garry wrote:
> Sparse complains of these:

> drivers/bus/hisi_lpc.c:82:38: warning: incorrect type in argument 1 (different address spaces)

> drivers/bus/hisi_lpc.c:82:38:    expected void const volatile [noderef] <asn:2>*addr

> drivers/bus/hisi_lpc.c:82:38:    got unsigned char *

> drivers/bus/hisi_lpc.c:131:35: warning: incorrect type in argument 1 (different address spaces)

> drivers/bus/hisi_lpc.c:131:35:    expected unsigned char *mbase

> drivers/bus/hisi_lpc.c:131:35:    got void [noderef] <asn:2>*membase

> drivers/bus/hisi_lpc.c:186:35: warning: incorrect type in argument 1 (different address spaces)

> drivers/bus/hisi_lpc.c:186:35:    expected unsigned char *mbase

> drivers/bus/hisi_lpc.c:186:35:    got void [noderef] <asn:2>*membase

> drivers/bus/hisi_lpc.c:228:16: warning: cast to restricted __le32

> drivers/bus/hisi_lpc.c:251:13: warning: incorrect type in assignment (different base types)

> drivers/bus/hisi_lpc.c:251:13:    expected unsigned int [unsigned] [usertype] val

> drivers/bus/hisi_lpc.c:251:13:    got restricted __le32 [usertype] <noident>

> 

> Clean them up.


OK, it might also be good to change the _in and _out functions
to use void * and not unsigned char * for buf

---
 drivers/bus/hisi_lpc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/bus/hisi_lpc.c b/drivers/bus/hisi_lpc.c
index 20c9571..ec2bfb 100644
--- a/drivers/bus/hisi_lpc.c
+++ b/drivers/bus/hisi_lpc.c
@@ -100,7 +100,7 @@ static int wait_lpc_idle(unsigned char *mbase, unsigned int waitcnt)
  */
 static int hisi_lpc_target_in(struct hisi_lpc_dev *lpcdev,
 			      struct lpc_cycle_para *para, unsigned long addr,
-			      unsigned char *buf, unsigned long opcnt)
+			      void *buf, unsigned long opcnt)
 {
 	unsigned int cmd_word;
 	unsigned int waitcnt;
@@ -153,7 +153,7 @@ static int hisi_lpc_target_in(struct hisi_lpc_dev *lpcdev,
  */
 static int hisi_lpc_target_out(struct hisi_lpc_dev *lpcdev,
 			       struct lpc_cycle_para *para, unsigned long addr,
-			       const unsigned char *buf, unsigned long opcnt)
+			       const void *buf, unsigned long opcnt)
 {
 	unsigned int waitcnt;
 	unsigned long flags;
John Garry Nov. 5, 2019, 9:50 a.m. UTC | #2 
On 04/11/2019 18:39, Joe Perches wrote:
> On Tue, 2019-11-05 at 01:22 +0800, John Garry wrote:

>> Sparse complains of these:

>> drivers/bus/hisi_lpc.c:82:38: warning: incorrect type in argument 1 (different address spaces)

>> drivers/bus/hisi_lpc.c:82:38:    expected void const volatile [noderef] <asn:2>*addr

>> drivers/bus/hisi_lpc.c:82:38:    got unsigned char *

>> drivers/bus/hisi_lpc.c:131:35: warning: incorrect type in argument 1 (different address spaces)

>> drivers/bus/hisi_lpc.c:131:35:    expected unsigned char *mbase

>> drivers/bus/hisi_lpc.c:131:35:    got void [noderef] <asn:2>*membase

>> drivers/bus/hisi_lpc.c:186:35: warning: incorrect type in argument 1 (different address spaces)

>> drivers/bus/hisi_lpc.c:186:35:    expected unsigned char *mbase

>> drivers/bus/hisi_lpc.c:186:35:    got void [noderef] <asn:2>*membase

>> drivers/bus/hisi_lpc.c:228:16: warning: cast to restricted __le32

>> drivers/bus/hisi_lpc.c:251:13: warning: incorrect type in assignment (different base types)

>> drivers/bus/hisi_lpc.c:251:13:    expected unsigned int [unsigned] [usertype] val

>> drivers/bus/hisi_lpc.c:251:13:    got restricted __le32 [usertype] <noident>

>>

>> Clean them up.

> 

> OK, it might also be good to change the _in and _out functions

> to use void * and not unsigned char * for buf


Hi Joe,

In fact, using unsigned char * is the right thing to do, and really the 
upper layer prob should not be passing void *. I'll look at this as a 
follow up.

Cheers,
John

> 

> ---

>   drivers/bus/hisi_lpc.c | 4 ++--

>   1 file changed, 2 insertions(+), 2 deletions(-)

> 

> diff --git a/drivers/bus/hisi_lpc.c b/drivers/bus/hisi_lpc.c

> index 20c9571..ec2bfb 100644

> --- a/drivers/bus/hisi_lpc.c

> +++ b/drivers/bus/hisi_lpc.c

> @@ -100,7 +100,7 @@ static int wait_lpc_idle(unsigned char *mbase, unsigned int waitcnt)

>    */

>   static int hisi_lpc_target_in(struct hisi_lpc_dev *lpcdev,

>   			      struct lpc_cycle_para *para, unsigned long addr,

> -			      unsigned char *buf, unsigned long opcnt)

> +			      void *buf, unsigned long opcnt)

>   {

>   	unsigned int cmd_word;

>   	unsigned int waitcnt;

> @@ -153,7 +153,7 @@ static int hisi_lpc_target_in(struct hisi_lpc_dev *lpcdev,

>    */

>   static int hisi_lpc_target_out(struct hisi_lpc_dev *lpcdev,

>   			       struct lpc_cycle_para *para, unsigned long addr,

> -			       const unsigned char *buf, unsigned long opcnt)

> +			       const void *buf, unsigned long opcnt)

>   {

>   	unsigned int waitcnt;

>   	unsigned long flags;

> 

> 

> .

>
Joe Perches Nov. 5, 2019, 3:48 p.m. UTC | #3 
On Tue, 2019-11-05 at 09:50 +0000, John Garry wrote:
> On 04/11/2019 18:39, Joe Perches wrote:

> > On Tue, 2019-11-05 at 01:22 +0800, John Garry wrote:

> > > Sparse complains of these:

> > > drivers/bus/hisi_lpc.c:82:38: warning: incorrect type in argument 1 (different address spaces)

> > > drivers/bus/hisi_lpc.c:82:38:    expected void const volatile [noderef] <asn:2>*addr

> > > drivers/bus/hisi_lpc.c:82:38:    got unsigned char *

> > > drivers/bus/hisi_lpc.c:131:35: warning: incorrect type in argument 1 (different address spaces)

> > > drivers/bus/hisi_lpc.c:131:35:    expected unsigned char *mbase

> > > drivers/bus/hisi_lpc.c:131:35:    got void [noderef] <asn:2>*membase

> > > drivers/bus/hisi_lpc.c:186:35: warning: incorrect type in argument 1 (different address spaces)

> > > drivers/bus/hisi_lpc.c:186:35:    expected unsigned char *mbase

> > > drivers/bus/hisi_lpc.c:186:35:    got void [noderef] <asn:2>*membase

> > > drivers/bus/hisi_lpc.c:228:16: warning: cast to restricted __le32

> > > drivers/bus/hisi_lpc.c:251:13: warning: incorrect type in assignment (different base types)

> > > drivers/bus/hisi_lpc.c:251:13:    expected unsigned int [unsigned] [usertype] val

> > > drivers/bus/hisi_lpc.c:251:13:    got restricted __le32 [usertype] <noident>

> > > 

> > > Clean them up.

> > 

> > OK, it might also be good to change the _in and _out functions

> > to use void * and not unsigned char * for buf

> 

> Hi Joe,


Hi John.

> In fact, using unsigned char * is the right thing to do, and really the 

> upper layer prob should not be passing void *. I'll look at this as a 

> follow up.


Maybe so, but it looks like the generic readsb and writesb routines
take void * so it might be a lot of other files to modify.

g'luck and cheers, Joe
diff mbox series

Patch

diff --git a/drivers/bus/hisi_lpc.c b/drivers/bus/hisi_lpc.c
index 20c957185af2..8101df901830 100644
--- a/drivers/bus/hisi_lpc.c
+++ b/drivers/bus/hisi_lpc.c
@@ -74,7 +74,7 @@  struct hisi_lpc_dev {
 /* About 10us. This is specific for single IO operations, such as inb */
 #define LPC_PEROP_WAITCNT	100
 
-static int wait_lpc_idle(unsigned char *mbase, unsigned int waitcnt)
+static int wait_lpc_idle(void __iomem *mbase, unsigned int waitcnt)
 {
 	u32 status;
 
@@ -209,7 +209,7 @@  static u32 hisi_lpc_comm_in(void *hostdata, unsigned long pio, size_t dwidth)
 	struct hisi_lpc_dev *lpcdev = hostdata;
 	struct lpc_cycle_para iopara;
 	unsigned long addr;
-	u32 rd_data = 0;
+	__le32 rd_data = 0;
 	int ret;
 
 	if (!lpcdev || !dwidth || dwidth > LPC_MAX_DWIDTH)
@@ -244,13 +244,12 @@  static void hisi_lpc_comm_out(void *hostdata, unsigned long pio,
 	struct lpc_cycle_para iopara;
 	const unsigned char *buf;
 	unsigned long addr;
+	__le32 _val = cpu_to_le32(val);
 
 	if (!lpcdev || !dwidth || dwidth > LPC_MAX_DWIDTH)
 		return;
 
-	val = cpu_to_le32(val);
-
-	buf = (const unsigned char *)&val;
+	buf = (const unsigned char *)&_val;
 	addr = hisi_lpc_pio_to_addr(lpcdev, pio);
 
 	iopara.opflags = FG_INCRADDR_LPC;