diff mbox series

soc: kunpeng_hccs: fix some sparse warnings about incorrect type

Message ID 20230815125233.65469-1-lihuisong@huawei.com (mailing list archive)
State Superseded
Headers show
Series soc: kunpeng_hccs: fix some sparse warnings about incorrect type | expand

Commit Message

lihuisong (C) Aug. 15, 2023, 12:52 p.m. UTC
This patch fixes some sparse warnings about incorrect type.
The address about PCC communication space should use '__iomem' tag.

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202308151142.dH5Muhva-lkp@intel.com/
Fixes: 886bdf9c883b ("soc: hisilicon: Support HCCS driver on Kunpeng SoC")
Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 drivers/soc/hisilicon/kunpeng_hccs.c | 13 +++++++------
 drivers/soc/hisilicon/kunpeng_hccs.h |  2 +-
 2 files changed, 8 insertions(+), 7 deletions(-)

Comments

Arnd Bergmann Aug. 16, 2023, 1:17 p.m. UTC | #1
On Tue, Aug 15, 2023, at 14:52, Huisong Li wrote:
> @@ -199,8 +200,8 @@ static int hccs_pcc_cmd_send(struct hccs_dev *hdev, u8 cmd,
>  			     struct hccs_desc *desc)
>  {
>  	struct hccs_mbox_client_info *cl_info = &hdev->cl_info;
> -	struct acpi_pcct_shared_memory *comm_base = cl_info->pcc_comm_addr;
> -	void *comm_space = (void *)(comm_base + 1);
> +	void __iomem *comm_space = (u8 *)cl_info->pcc_comm_addr +
> +					sizeof(struct acpi_pcct_shared_memory);
>  	struct hccs_fw_inner_head *fw_inner_head;
>  	struct acpi_pcct_shared_memory tmp = {0};
>  	u16 comm_space_size;

The cast still looks wrong, while both comm_space and ->pcc_comm_addr
are __iomem pointers, casting to a non-__iomem u8 pointer should
still produce that warning. I'd suggest casting to (u8 __iomem *)
or (void __iomem *), I assume you still need a cast here to get
the correct address.

      Arnd
lihuisong (C) Aug. 17, 2023, 1:37 a.m. UTC | #2
在 2023/8/16 21:17, Arnd Bergmann 写道:
> On Tue, Aug 15, 2023, at 14:52, Huisong Li wrote:
>> @@ -199,8 +200,8 @@ static int hccs_pcc_cmd_send(struct hccs_dev *hdev, u8 cmd,
>>   			     struct hccs_desc *desc)
>>   {
>>   	struct hccs_mbox_client_info *cl_info = &hdev->cl_info;
>> -	struct acpi_pcct_shared_memory *comm_base = cl_info->pcc_comm_addr;
>> -	void *comm_space = (void *)(comm_base + 1);
>> +	void __iomem *comm_space = (u8 *)cl_info->pcc_comm_addr +
>> +					sizeof(struct acpi_pcct_shared_memory);
>>   	struct hccs_fw_inner_head *fw_inner_head;
>>   	struct acpi_pcct_shared_memory tmp = {0};
>>   	u16 comm_space_size;
> The cast still looks wrong, while both comm_space and ->pcc_comm_addr
> are __iomem pointers, casting to a non-__iomem u8 pointer should
> still produce that warning. I'd suggest casting to (u8 __iomem *)
> or (void __iomem *), I assume you still need a cast here to get
> the correct address.
Thanks Arnd.

You are right. So I modify it by dropping (u8 *) here in patch v2.
Please take a look at this again.
If it is ok for you, can you apply this patch to soc/soc.git?

/Huisong
Arnd Bergmann Aug. 17, 2023, 11:54 a.m. UTC | #3
On Thu, Aug 17, 2023, at 03:37, lihuisong (C) wrote:
> 在 2023/8/16 21:17, Arnd Bergmann 写道:
>> On Tue, Aug 15, 2023, at 14:52, Huisong Li wrote:
>>> @@ -199,8 +200,8 @@ static int hccs_pcc_cmd_send(struct hccs_dev *hdev, u8 cmd,
>>>   			     struct hccs_desc *desc)
>>>   {
>>>   	struct hccs_mbox_client_info *cl_info = &hdev->cl_info;
>>> -	struct acpi_pcct_shared_memory *comm_base = cl_info->pcc_comm_addr;
>>> -	void *comm_space = (void *)(comm_base + 1);
>>> +	void __iomem *comm_space = (u8 *)cl_info->pcc_comm_addr +
>>> +					sizeof(struct acpi_pcct_shared_memory);
>>>   	struct hccs_fw_inner_head *fw_inner_head;
>>>   	struct acpi_pcct_shared_memory tmp = {0};
>>>   	u16 comm_space_size;
>> The cast still looks wrong, while both comm_space and ->pcc_comm_addr
>> are __iomem pointers, casting to a non-__iomem u8 pointer should
>> still produce that warning. I'd suggest casting to (u8 __iomem *)
>> or (void __iomem *), I assume you still need a cast here to get
>> the correct address.
> Thanks Arnd.
>
> You are right. So I modify it by dropping (u8 *) here in patch v2.
> Please take a look at this again.

Right, if pcc_comm_addr is already a u8 __iomem* or void*, that
works, I though the cast was needed since it points to a larger
object, which would throw off the calculation.

> If it is ok for you, can you apply this patch to soc/soc.git?

Yes, of course.

       Arnd
lihuisong (C) Aug. 18, 2023, 1:38 a.m. UTC | #4
在 2023/8/17 19:54, Arnd Bergmann 写道:
> On Thu, Aug 17, 2023, at 03:37, lihuisong (C) wrote:
>> 在 2023/8/16 21:17, Arnd Bergmann 写道:
>>> On Tue, Aug 15, 2023, at 14:52, Huisong Li wrote:
>>>> @@ -199,8 +200,8 @@ static int hccs_pcc_cmd_send(struct hccs_dev *hdev, u8 cmd,
>>>>    			     struct hccs_desc *desc)
>>>>    {
>>>>    	struct hccs_mbox_client_info *cl_info = &hdev->cl_info;
>>>> -	struct acpi_pcct_shared_memory *comm_base = cl_info->pcc_comm_addr;
>>>> -	void *comm_space = (void *)(comm_base + 1);
>>>> +	void __iomem *comm_space = (u8 *)cl_info->pcc_comm_addr +
>>>> +					sizeof(struct acpi_pcct_shared_memory);
>>>>    	struct hccs_fw_inner_head *fw_inner_head;
>>>>    	struct acpi_pcct_shared_memory tmp = {0};
>>>>    	u16 comm_space_size;
>>> The cast still looks wrong, while both comm_space and ->pcc_comm_addr
>>> are __iomem pointers, casting to a non-__iomem u8 pointer should
>>> still produce that warning. I'd suggest casting to (u8 __iomem *)
>>> or (void __iomem *), I assume you still need a cast here to get
>>> the correct address.
>> Thanks Arnd.
>>
>> You are right. So I modify it by dropping (u8 *) here in patch v2.
>> Please take a look at this again.
> Right, if pcc_comm_addr is already a u8 __iomem* or void*, that
> works, I though the cast was needed since it points to a larger
> object, which would throw off the calculation.
This pcc_comm_addr is already defined "void __iomem *" type in patch v2.
So that works well, right?

/Huisong
>
>> If it is ok for you, can you apply this patch to soc/soc.git?
> Yes, of course.
>
>         Arnd
> .
Arnd Bergmann Aug. 18, 2023, 1:57 a.m. UTC | #5
On Fri, Aug 18, 2023, at 03:38, lihuisong (C) wrote:
> 在 2023/8/17 19:54, Arnd Bergmann 写道:
>> On Thu, Aug 17, 2023, at 03:37, lihuisong (C) wrote:
>>> 在 2023/8/16 21:17, Arnd Bergmann 写道:
>>>> On Tue, Aug 15, 2023, at 14:52, Huisong Li wrote:
>>>>> @@ -199,8 +200,8 @@ static int hccs_pcc_cmd_send(struct hccs_dev *hdev, u8 cmd,
>>>>>    			     struct hccs_desc *desc)
>>>>>    {
>>>>>    	struct hccs_mbox_client_info *cl_info = &hdev->cl_info;
>>>>> -	struct acpi_pcct_shared_memory *comm_base = cl_info->pcc_comm_addr;
>>>>> -	void *comm_space = (void *)(comm_base + 1);
>>>>> +	void __iomem *comm_space = (u8 *)cl_info->pcc_comm_addr +
>>>>> +					sizeof(struct acpi_pcct_shared_memory);
>>>>>    	struct hccs_fw_inner_head *fw_inner_head;
>>>>>    	struct acpi_pcct_shared_memory tmp = {0};
>>>>>    	u16 comm_space_size;
>>>> The cast still looks wrong, while both comm_space and ->pcc_comm_addr
>>>> are __iomem pointers, casting to a non-__iomem u8 pointer should
>>>> still produce that warning. I'd suggest casting to (u8 __iomem *)
>>>> or (void __iomem *), I assume you still need a cast here to get
>>>> the correct address.
>>> Thanks Arnd.
>>>
>>> You are right. So I modify it by dropping (u8 *) here in patch v2.
>>> Please take a look at this again.
>> Right, if pcc_comm_addr is already a u8 __iomem* or void*, that
>> works, I though the cast was needed since it points to a larger
>> object, which would throw off the calculation.
> This pcc_comm_addr is already defined "void __iomem *" type in patch v2.
> So that works well, right?

Yes, that works.

     Arnd
diff mbox series

Patch

diff --git a/drivers/soc/hisilicon/kunpeng_hccs.c b/drivers/soc/hisilicon/kunpeng_hccs.c
index 0d6f6bacd3f6..4e94753ab0f7 100644
--- a/drivers/soc/hisilicon/kunpeng_hccs.c
+++ b/drivers/soc/hisilicon/kunpeng_hccs.c
@@ -156,8 +156,8 @@  static int hccs_register_pcc_channel(struct hccs_dev *hdev)
 	}
 
 	if (pcc_chan->shmem_base_addr) {
-		cl_info->pcc_comm_addr = (void __force *)ioremap(
-			pcc_chan->shmem_base_addr, pcc_chan->shmem_size);
+		cl_info->pcc_comm_addr = ioremap(pcc_chan->shmem_base_addr,
+						 pcc_chan->shmem_size);
 		if (!cl_info->pcc_comm_addr) {
 			dev_err(dev, "Failed to ioremap PCC communication region for channel-%d.\n",
 				hdev->chan_id);
@@ -177,7 +177,8 @@  static int hccs_register_pcc_channel(struct hccs_dev *hdev)
 static int hccs_check_chan_cmd_complete(struct hccs_dev *hdev)
 {
 	struct hccs_mbox_client_info *cl_info = &hdev->cl_info;
-	struct acpi_pcct_shared_memory *comm_base = cl_info->pcc_comm_addr;
+	struct acpi_pcct_shared_memory __iomem *comm_base =
+							cl_info->pcc_comm_addr;
 	u16 status;
 	int ret;
 
@@ -199,8 +200,8 @@  static int hccs_pcc_cmd_send(struct hccs_dev *hdev, u8 cmd,
 			     struct hccs_desc *desc)
 {
 	struct hccs_mbox_client_info *cl_info = &hdev->cl_info;
-	struct acpi_pcct_shared_memory *comm_base = cl_info->pcc_comm_addr;
-	void *comm_space = (void *)(comm_base + 1);
+	void __iomem *comm_space = (u8 *)cl_info->pcc_comm_addr +
+					sizeof(struct acpi_pcct_shared_memory);
 	struct hccs_fw_inner_head *fw_inner_head;
 	struct acpi_pcct_shared_memory tmp = {0};
 	u16 comm_space_size;
@@ -212,7 +213,7 @@  static int hccs_pcc_cmd_send(struct hccs_dev *hdev, u8 cmd,
 	tmp.command = cmd;
 	/* Clear cmd complete bit */
 	tmp.status = 0;
-	memcpy_toio(comm_base, (void *)&tmp,
+	memcpy_toio(cl_info->pcc_comm_addr, (void *)&tmp,
 			sizeof(struct acpi_pcct_shared_memory));
 
 	/* Copy the message to the PCC comm space */
diff --git a/drivers/soc/hisilicon/kunpeng_hccs.h b/drivers/soc/hisilicon/kunpeng_hccs.h
index 9d71fb78443f..6012d2776028 100644
--- a/drivers/soc/hisilicon/kunpeng_hccs.h
+++ b/drivers/soc/hisilicon/kunpeng_hccs.h
@@ -50,7 +50,7 @@  struct hccs_mbox_client_info {
 	struct mbox_chan *mbox_chan;
 	struct pcc_mbox_chan *pcc_chan;
 	u64 deadline_us;
-	void *pcc_comm_addr;
+	void __iomem *pcc_comm_addr;
 };
 
 struct hccs_dev {