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

["Jonathan Neuschäfer" <j.neuschaefer@gmx.net>]

[-- Attachment #1: Type: text/plain, Size: 1228 bytes --]

On Thu, Jan 21, 2021 at 12:46:42PM +0000, Conor.Dooley@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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]