[PATCH v3 1/5] mbox: add polarfire soc system controller mailbox

Jonathan Neuschäfer j.neuschaefer at gmx.net
Thu Jan 21 08:38:42 EST 2021


On Thu, Jan 21, 2021 at 12:46:42PM +0000, Conor.Dooley at microchip.com wrote:
> On 02/01/2021 13:01, Jonathan Neuschäfer wrote
> 
> >Hello,
> >
> >I've added review comments below. Some of them might be more detailed
> >than necessary, and reflect my opinion rather than something that must
> >be fixed. Anyway, I hope my comments make sense.
> >
> the more detailed feedback the better in my book, if i dont mention it youll probably see in changed in the next day or two

Great.


> >> +static irqreturn_t mpfs_mbox_inbox_isr(int irq, void *data)
> >> +{
> >> +    struct mbox_chan *chan = (struct mbox_chan *)data;
> >
> >This cast and the one at the end of mpfs_mbox_rx_data are somewhat
> >uncessary, because C allows implicit conversion of void pointers to and
> >from other pointer types.
> >
> 
> true, i had put them in thinking it made it more clear, but on reflection it doesnt.

The main problem that I personally have with explicit pointer casts is
that they are accepted by the compiler even when they would be a bad
idea (casting from one non-void pointer type to another (incompatible)
non-void pointer type), so they can hide a class of bugs.



Thanks,
Jonathan Neuschäfer
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-riscv/attachments/20210121/139de30d/attachment.sig>


More information about the linux-riscv mailing list