| Message ID | 1507201911-27976-1-git-send-email-sudeep.holla@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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) >
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.
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.
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)
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(-)