From: "Jonathan Neuschäfer" <j.neuschaefer@gmx.net>
To: Conor.Dooley@microchip.com
Cc: j.neuschaefer@gmx.net, robh+dt@kernel.org, damien.lemoal@wdc.com,
jassisinghbrar@gmail.com, aou@eecs.berkeley.edu,
paul.walmsley@sifive.com, palmer@dabbelt.com,
devicetree@vger.kernel.org, linux-riscv@lists.infradead.org,
Lewis.Hanly@microchip.com, Cyril.Jean@microchip.com,
Daire.McNamara@microchip.com, atish.patra@wdc.com,
anup.patel@wdc.com, david.abdurachmanov@gmail.com
Subject: Re: [PATCH v3 1/5] mbox: add polarfire soc system controller mailbox
Date: Thu, 21 Jan 2021 14:38:42 +0100 [thread overview]
Message-ID: <YAmD4h9vssAQlXPh@latitude> (raw)
In-Reply-To: <3425400b-889f-5393-6c26-bf708f2734f1@microchip.com>
[-- 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 --]
WARNING: multiple messages have this Message-ID (diff)
From: "Jonathan Neuschäfer" <j.neuschaefer@gmx.net>
To: Conor.Dooley@microchip.com
Cc: devicetree@vger.kernel.org, damien.lemoal@wdc.com,
aou@eecs.berkeley.edu, Lewis.Hanly@microchip.com,
david.abdurachmanov@gmail.com, Daire.McNamara@microchip.com,
jassisinghbrar@gmail.com, j.neuschaefer@gmx.net,
atish.patra@wdc.com, anup.patel@wdc.com, robh+dt@kernel.org,
palmer@dabbelt.com, paul.walmsley@sifive.com,
Cyril.Jean@microchip.com, linux-riscv@lists.infradead.org
Subject: Re: [PATCH v3 1/5] mbox: add polarfire soc system controller mailbox
Date: Thu, 21 Jan 2021 14:38:42 +0100 [thread overview]
Message-ID: <YAmD4h9vssAQlXPh@latitude> (raw)
In-Reply-To: <3425400b-889f-5393-6c26-bf708f2734f1@microchip.com>
[-- Attachment #1.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 #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 161 bytes --]
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
next prev parent reply other threads:[~2021-01-21 13:58 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-23 16:32 [PATCH v3 0/5] Add support for the PolarFire SoC system controller conor.dooley
2020-12-23 16:32 ` conor.dooley
2020-12-23 16:32 ` [PATCH v3 1/5] mbox: add polarfire soc system controller mailbox conor.dooley
2020-12-23 16:32 ` conor.dooley
2021-01-02 13:01 ` Jonathan Neuschäfer
2021-01-02 13:01 ` Jonathan Neuschäfer
2021-01-21 12:46 ` Conor.Dooley
2021-01-21 12:46 ` Conor.Dooley
2021-01-21 13:38 ` Jonathan Neuschäfer [this message]
2021-01-21 13:38 ` Jonathan Neuschäfer
2020-12-23 16:33 ` [PATCH v3 2/5] dt-bindings: add bindings for polarfire soc mailbox conor.dooley
2020-12-23 16:33 ` conor.dooley
2020-12-31 18:34 ` Rob Herring
2020-12-31 18:34 ` Rob Herring
2021-01-02 11:30 ` Jonathan Neuschäfer
2021-01-02 11:30 ` Jonathan Neuschäfer
2020-12-23 16:33 ` [PATCH v3 4/5] dt-bindings: add bindings for polarfire soc system controller conor.dooley
2020-12-23 16:33 ` conor.dooley
2021-01-05 21:25 ` Jonathan Neuschäfer
2021-01-05 21:25 ` Jonathan Neuschäfer
2021-01-08 2:31 ` Rob Herring
2021-01-08 2:31 ` Rob Herring
2021-01-06 19:25 ` [PATCH v3 0/5] Add support for the PolarFire SoC " Atish Patra
2021-01-06 19:25 ` Atish Patra
2021-01-07 11:49 ` Cyril.Jean
2021-01-07 11:49 ` Cyril.Jean
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YAmD4h9vssAQlXPh@latitude \
--to=j.neuschaefer@gmx.net \
--cc=Conor.Dooley@microchip.com \
--cc=Cyril.Jean@microchip.com \
--cc=Daire.McNamara@microchip.com \
--cc=Lewis.Hanly@microchip.com \
--cc=anup.patel@wdc.com \
--cc=aou@eecs.berkeley.edu \
--cc=atish.patra@wdc.com \
--cc=damien.lemoal@wdc.com \
--cc=david.abdurachmanov@gmail.com \
--cc=devicetree@vger.kernel.org \
--cc=jassisinghbrar@gmail.com \
--cc=linux-riscv@lists.infradead.org \
--cc=palmer@dabbelt.com \
--cc=paul.walmsley@sifive.com \
--cc=robh+dt@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.