* [PATCH 1/2] firmware: arm_scpi: drop unnecessary type cast to scpi_shared_mem
@ 2017-10-05 11:11 Sudeep Holla
2017-10-05 11:11 ` [PATCH 2/2] firmware: arm_scpi: remove all single element structures Sudeep Holla
2017-10-05 19:06 ` [PATCH 1/2] firmware: arm_scpi: drop unnecessary type cast to scpi_shared_mem Heiner Kallweit
0 siblings, 2 replies; 8+ messages in thread
From: Sudeep Holla @ 2017-10-05 11:11 UTC (permalink / raw)
To: linux-arm-kernel
This patch drops the only present type cast of the SCPI payload pointer
to scpi_shared_mem inorder to align with other occurrences, IOW for
consistency.
Cc: Heiner Kallweit <hkallweit1@gmail.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
drivers/firmware/arm_scpi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/firmware/arm_scpi.c b/drivers/firmware/arm_scpi.c
index c923d1ebfa3e..a888970f1347 100644
--- a/drivers/firmware/arm_scpi.c
+++ b/drivers/firmware/arm_scpi.c
@@ -453,7 +453,7 @@ static void scpi_tx_prepare(struct mbox_client *c, void *msg)
unsigned long flags;
struct scpi_xfer *t = msg;
struct scpi_chan *ch = container_of(c, struct scpi_chan, cl);
- struct scpi_shared_mem *mem = (struct scpi_shared_mem *)ch->tx_payload;
+ struct scpi_shared_mem *mem = ch->tx_payload;
if (t->tx_buf) {
if (scpi_info->is_legacy)
--
2.7.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] firmware: arm_scpi: remove all single element structures
2017-10-05 11:11 [PATCH 1/2] firmware: arm_scpi: drop unnecessary type cast to scpi_shared_mem Sudeep Holla
@ 2017-10-05 11:11 ` Sudeep Holla
2017-10-05 19:07 ` Heiner Kallweit
2017-10-05 19:06 ` [PATCH 1/2] firmware: arm_scpi: drop unnecessary type cast to scpi_shared_mem Heiner Kallweit
1 sibling, 1 reply; 8+ messages in thread
From: Sudeep Holla @ 2017-10-05 11:11 UTC (permalink / raw)
To: linux-arm-kernel
Both clk_get_value and sensor_value structures contains a single element
and hence needs no packing making the whole structure defination
unnecessary.
This patch gets rid of both those structures
Cc: Heiner Kallweit <hkallweit1@gmail.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
drivers/firmware/arm_scpi.c | 22 +++++++---------------
1 file changed, 7 insertions(+), 15 deletions(-)
diff --git a/drivers/firmware/arm_scpi.c b/drivers/firmware/arm_scpi.c
index a888970f1347..f0c37a4ecddf 100644
--- a/drivers/firmware/arm_scpi.c
+++ b/drivers/firmware/arm_scpi.c
@@ -304,10 +304,6 @@ struct clk_get_info {
u8 name[20];
} __packed;
-struct clk_get_value {
- __le32 rate;
-} __packed;
-
struct clk_set_value {
__le16 id;
__le16 reserved;
@@ -346,10 +342,6 @@ struct _scpi_sensor_info {
char name[20];
};
-struct sensor_value {
- __le64 val;
-};
-
struct dev_pstate_set {
__le16 dev_id;
u8 pstate;
@@ -577,13 +569,13 @@ scpi_clk_get_range(u16 clk_id, unsigned long *min, unsigned long *max)
static unsigned long scpi_clk_get_val(u16 clk_id)
{
int ret;
- struct clk_get_value clk;
+ __le32 rate;
__le16 le_clk_id = cpu_to_le16(clk_id);
ret = scpi_send_message(CMD_GET_CLOCK_VALUE, &le_clk_id,
- sizeof(le_clk_id), &clk, sizeof(clk));
+ sizeof(le_clk_id), &rate, sizeof(rate));
- return ret ? ret : le32_to_cpu(clk.rate);
+ return ret ? ret : le32_to_cpu(rate);
}
static int scpi_clk_set_val(u16 clk_id, unsigned long rate)
@@ -775,19 +767,19 @@ static int scpi_sensor_get_info(u16 sensor_id, struct scpi_sensor_info *info)
static int scpi_sensor_get_value(u16 sensor, u64 *val)
{
__le16 id = cpu_to_le16(sensor);
- struct sensor_value buf;
+ __le64 value;
int ret;
ret = scpi_send_message(CMD_SENSOR_VALUE, &id, sizeof(id),
- &buf, sizeof(buf));
+ &value, sizeof(value));
if (ret)
return ret;
if (scpi_info->is_legacy)
/* only 32-bits supported, upper 32 bits can be junk */
- *val = le32_to_cpup((__le32 *)&buf.val);
+ *val = le32_to_cpup((__le32 *)&value);
else
- *val = le64_to_cpu(buf.val);
+ *val = le64_to_cpu(value);
return 0;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 1/2] firmware: arm_scpi: drop unnecessary type cast to scpi_shared_mem
2017-10-05 11:11 [PATCH 1/2] firmware: arm_scpi: drop unnecessary type cast to scpi_shared_mem Sudeep Holla
2017-10-05 11:11 ` [PATCH 2/2] firmware: arm_scpi: remove all single element structures Sudeep Holla
@ 2017-10-05 19:06 ` Heiner Kallweit
2017-10-06 8:51 ` Sudeep Holla
1 sibling, 1 reply; 8+ messages in thread
From: Heiner Kallweit @ 2017-10-05 19:06 UTC (permalink / raw)
To: linux-arm-kernel
Am 05.10.2017 um 13:11 schrieb Sudeep Holla:
> This patch drops the only present type cast of the SCPI payload pointer
> to scpi_shared_mem inorder to align with other occurrences, IOW for
> consistency.
>
> Cc: Heiner Kallweit <hkallweit1@gmail.com>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
> drivers/firmware/arm_scpi.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/firmware/arm_scpi.c b/drivers/firmware/arm_scpi.c
> index c923d1ebfa3e..a888970f1347 100644
> --- a/drivers/firmware/arm_scpi.c
> +++ b/drivers/firmware/arm_scpi.c
> @@ -453,7 +453,7 @@ static void scpi_tx_prepare(struct mbox_client *c, void *msg)
> unsigned long flags;
> struct scpi_xfer *t = msg;
> struct scpi_chan *ch = container_of(c, struct scpi_chan, cl);
> - struct scpi_shared_mem *mem = (struct scpi_shared_mem *)ch->tx_payload;
> + struct scpi_shared_mem *mem = ch->tx_payload;
>
This should work fine, however it might not be 100% clean with regard to __iomem
annotation. ch->tx_payload is of type "void __iomem *" and I would assume that
sparse complains about the new statement.
The old one casts away the __iomem, what isn't much better IMO.
By the way, I think this also applies to other places in this driver
where ch->rx/tx_payload are used.
> if (t->tx_buf) {
> if (scpi_info->is_legacy)
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] firmware: arm_scpi: remove all single element structures
2017-10-05 11:11 ` [PATCH 2/2] firmware: arm_scpi: remove all single element structures Sudeep Holla
@ 2017-10-05 19:07 ` Heiner Kallweit
2017-10-06 8:53 ` Sudeep Holla
0 siblings, 1 reply; 8+ messages in thread
From: Heiner Kallweit @ 2017-10-05 19:07 UTC (permalink / raw)
To: linux-arm-kernel
Am 05.10.2017 um 13:11 schrieb Sudeep Holla:
> Both clk_get_value and sensor_value structures contains a single element
> and hence needs no packing making the whole structure defination
> unnecessary.
>
> This patch gets rid of both those structures
>
> Cc: Heiner Kallweit <hkallweit1@gmail.com>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
> drivers/firmware/arm_scpi.c | 22 +++++++---------------
> 1 file changed, 7 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/firmware/arm_scpi.c b/drivers/firmware/arm_scpi.c
> index a888970f1347..f0c37a4ecddf 100644
> --- a/drivers/firmware/arm_scpi.c
> +++ b/drivers/firmware/arm_scpi.c
> @@ -304,10 +304,6 @@ struct clk_get_info {
> u8 name[20];
> } __packed;
>
> -struct clk_get_value {
> - __le32 rate;
> -} __packed;
> -
> struct clk_set_value {
> __le16 id;
> __le16 reserved;
> @@ -346,10 +342,6 @@ struct _scpi_sensor_info {
> char name[20];
> };
>
> -struct sensor_value {
> - __le64 val;
> -};
> -
> struct dev_pstate_set {
> __le16 dev_id;
> u8 pstate;
> @@ -577,13 +569,13 @@ scpi_clk_get_range(u16 clk_id, unsigned long *min, unsigned long *max)
> static unsigned long scpi_clk_get_val(u16 clk_id)
> {
> int ret;
> - struct clk_get_value clk;
> + __le32 rate;
> __le16 le_clk_id = cpu_to_le16(clk_id);
>
> ret = scpi_send_message(CMD_GET_CLOCK_VALUE, &le_clk_id,
> - sizeof(le_clk_id), &clk, sizeof(clk));
> + sizeof(le_clk_id), &rate, sizeof(rate));
>
> - return ret ? ret : le32_to_cpu(clk.rate);
> + return ret ? ret : le32_to_cpu(rate);
> }
>
> static int scpi_clk_set_val(u16 clk_id, unsigned long rate)
> @@ -775,19 +767,19 @@ static int scpi_sensor_get_info(u16 sensor_id, struct scpi_sensor_info *info)
> static int scpi_sensor_get_value(u16 sensor, u64 *val)
> {
> __le16 id = cpu_to_le16(sensor);
> - struct sensor_value buf;
> + __le64 value;
> int ret;
>
> ret = scpi_send_message(CMD_SENSOR_VALUE, &id, sizeof(id),
> - &buf, sizeof(buf));
> + &value, sizeof(value));
> if (ret)
> return ret;
>
> if (scpi_info->is_legacy)
> /* only 32-bits supported, upper 32 bits can be junk */
> - *val = le32_to_cpup((__le32 *)&buf.val);
> + *val = le32_to_cpup((__le32 *)&value);
> else
> - *val = le64_to_cpu(buf.val);
> + *val = le64_to_cpu(value);
>
> return 0;
> }
>
Looks good to me.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] firmware: arm_scpi: drop unnecessary type cast to scpi_shared_mem
2017-10-05 19:06 ` [PATCH 1/2] firmware: arm_scpi: drop unnecessary type cast to scpi_shared_mem Heiner Kallweit
@ 2017-10-06 8:51 ` Sudeep Holla
2017-10-06 18:47 ` Heiner Kallweit
0 siblings, 1 reply; 8+ messages in thread
From: Sudeep Holla @ 2017-10-06 8:51 UTC (permalink / raw)
To: linux-arm-kernel
On 05/10/17 20:06, Heiner Kallweit wrote:
> Am 05.10.2017 um 13:11 schrieb Sudeep Holla:
>> This patch drops the only present type cast of the SCPI payload pointer
>> to scpi_shared_mem inorder to align with other occurrences, IOW for
>> consistency.
>>
>> Cc: Heiner Kallweit <hkallweit1@gmail.com>
>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>> ---
>> drivers/firmware/arm_scpi.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/firmware/arm_scpi.c b/drivers/firmware/arm_scpi.c
>> index c923d1ebfa3e..a888970f1347 100644
>> --- a/drivers/firmware/arm_scpi.c
>> +++ b/drivers/firmware/arm_scpi.c
>> @@ -453,7 +453,7 @@ static void scpi_tx_prepare(struct mbox_client *c, void *msg)
>> unsigned long flags;
>> struct scpi_xfer *t = msg;
>> struct scpi_chan *ch = container_of(c, struct scpi_chan, cl);
>> - struct scpi_shared_mem *mem = (struct scpi_shared_mem *)ch->tx_payload;
>> + struct scpi_shared_mem *mem = ch->tx_payload;
>>
> This should work fine, however it might not be 100% clean with regard to __iomem
> annotation. ch->tx_payload is of type "void __iomem *" and I would assume that
> sparse complains about the new statement.
Yes sparse complains for both(with or without typecast)
> The old one casts away the __iomem, what isn't much better IMO.
>
Agreed.
> By the way, I think this also applies to other places in this driver
> where ch->rx/tx_payload are used.
>
Yes, but do you have any alternatives in your mind ? I am happy to hear
that.
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] firmware: arm_scpi: remove all single element structures
2017-10-05 19:07 ` Heiner Kallweit
@ 2017-10-06 8:53 ` Sudeep Holla
2017-10-06 18:51 ` Heiner Kallweit
0 siblings, 1 reply; 8+ messages in thread
From: Sudeep Holla @ 2017-10-06 8:53 UTC (permalink / raw)
To: linux-arm-kernel
On 05/10/17 20:07, Heiner Kallweit wrote:
> Am 05.10.2017 um 13:11 schrieb Sudeep Holla:
>> Both clk_get_value and sensor_value structures contains a single element
>> and hence needs no packing making the whole structure defination
>> unnecessary.
>>
>> This patch gets rid of both those structures
>>
>> Cc: Heiner Kallweit <hkallweit1@gmail.com>
>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
[..]
>>
> Looks good to me.
Thanks. Tested-by or Reviewed-by would help ;)
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] firmware: arm_scpi: drop unnecessary type cast to scpi_shared_mem
2017-10-06 8:51 ` Sudeep Holla
@ 2017-10-06 18:47 ` Heiner Kallweit
0 siblings, 0 replies; 8+ messages in thread
From: Heiner Kallweit @ 2017-10-06 18:47 UTC (permalink / raw)
To: linux-arm-kernel
Am 06.10.2017 um 10:51 schrieb Sudeep Holla:
>
>
> On 05/10/17 20:06, Heiner Kallweit wrote:
>> Am 05.10.2017 um 13:11 schrieb Sudeep Holla:
>>> This patch drops the only present type cast of the SCPI payload pointer
>>> to scpi_shared_mem inorder to align with other occurrences, IOW for
>>> consistency.
>>>
>>> Cc: Heiner Kallweit <hkallweit1@gmail.com>
>>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>>> ---
>>> drivers/firmware/arm_scpi.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/firmware/arm_scpi.c b/drivers/firmware/arm_scpi.c
>>> index c923d1ebfa3e..a888970f1347 100644
>>> --- a/drivers/firmware/arm_scpi.c
>>> +++ b/drivers/firmware/arm_scpi.c
>>> @@ -453,7 +453,7 @@ static void scpi_tx_prepare(struct mbox_client *c, void *msg)
>>> unsigned long flags;
>>> struct scpi_xfer *t = msg;
>>> struct scpi_chan *ch = container_of(c, struct scpi_chan, cl);
>>> - struct scpi_shared_mem *mem = (struct scpi_shared_mem *)ch->tx_payload;
>>> + struct scpi_shared_mem *mem = ch->tx_payload;
>>>
>> This should work fine, however it might not be 100% clean with regard to __iomem
>> annotation. ch->tx_payload is of type "void __iomem *" and I would assume that
>> sparse complains about the new statement.
>
> Yes sparse complains for both(with or without typecast)
>
>> The old one casts away the __iomem, what isn't much better IMO.
>>
>
> Agreed.
>
>> By the way, I think this also applies to other places in this driver
>> where ch->rx/tx_payload are used.
>>
>
> Yes, but do you have any alternatives in your mind ? I am happy to hear
> that.
>
I'll send a patch addressing this issue.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] firmware: arm_scpi: remove all single element structures
2017-10-06 8:53 ` Sudeep Holla
@ 2017-10-06 18:51 ` Heiner Kallweit
0 siblings, 0 replies; 8+ messages in thread
From: Heiner Kallweit @ 2017-10-06 18:51 UTC (permalink / raw)
To: linux-arm-kernel
Am 06.10.2017 um 10:53 schrieb Sudeep Holla:
>
>
> On 05/10/17 20:07, Heiner Kallweit wrote:
>> Am 05.10.2017 um 13:11 schrieb Sudeep Holla:
>>> Both clk_get_value and sensor_value structures contains a single element
>>> and hence needs no packing making the whole structure defination
>>> unnecessary.
>>>
>>> This patch gets rid of both those structures
>>>
>>> Cc: Heiner Kallweit <hkallweit1@gmail.com>
>>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>
> [..]
>
>>>
>> Looks good to me.
>
> Thanks. Tested-by or Reviewed-by would help ;)
>
>
Reviewed-by: Heiner Kallweit <hkallweit1@gmail.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-10-06 18:51 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-05 11:11 [PATCH 1/2] firmware: arm_scpi: drop unnecessary type cast to scpi_shared_mem Sudeep Holla
2017-10-05 11:11 ` [PATCH 2/2] firmware: arm_scpi: remove all single element structures Sudeep Holla
2017-10-05 19:07 ` Heiner Kallweit
2017-10-06 8:53 ` Sudeep Holla
2017-10-06 18:51 ` Heiner Kallweit
2017-10-05 19:06 ` [PATCH 1/2] firmware: arm_scpi: drop unnecessary type cast to scpi_shared_mem Heiner Kallweit
2017-10-06 8:51 ` Sudeep Holla
2017-10-06 18:47 ` Heiner Kallweit
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.