From: Tony Prisk <linux@prisktech.co.nz>
To: Mark Rutland <mark.rutland@arm.com>, Roel Kluin <roel.kluin@gmail.com>
Cc: "grant.likely@linaro.org" <grant.likely@linaro.org>,
"rob.herring@calxeda.com" <rob.herring@calxeda.com>,
"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
linux-serial@vger.kernel.org
Subject: Re: [PATCH] In absence of braces, port and NULL are missed
Date: Tue, 15 Oct 2013 06:37:06 +1300 [thread overview]
Message-ID: <525C2BC2.1030207@prisktech.co.nz> (raw)
In-Reply-To: <20131014125449.GB31708@e106331-lin.cambridge.arm.com>
On 15/10/13 01:54, Mark Rutland wrote:
> [Adding Tony Prisk and linux-serial]
>
> Hi Roel,
>
> While this looks like a good fix, the commit message is a bit confusing.
> How about something like the message between the scissor lines below:
>
> ---->8----
> [PATCH] serial: vt8500: add missing braces
>
> Due to missing braces on an if statement, port will always be assigned
> -1 regardless of any alias entries in the dt, which was clearly not the
> intended behaviour.
>
> This patch adds the missing braces, fixing the issue.
> ---->8----
>
> Also, while this is device tree related, you should Cc the driver
> maintainer when submitting driver changes.
>
> Cheers,
> Mark.
>
> On Sun, Oct 13, 2013 at 11:29:36PM +0100, Roel Kluin wrote:
>> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
>> ---
>> drivers/tty/serial/vt8500_serial.c | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/tty/serial/vt8500_serial.c b/drivers/tty/serial/vt8500_serial.c
>> index 93b697a..4cf71c0 100644
>> --- a/drivers/tty/serial/vt8500_serial.c
>> +++ b/drivers/tty/serial/vt8500_serial.c
>> @@ -561,12 +561,13 @@ static int vt8500_serial_probe(struct platform_device *pdev)
>> if (!mmres || !irqres)
>> return -ENODEV;
>>
>> - if (np)
>> + if (np) {
>> port = of_alias_get_id(np, "serial");
>> if (port >= VT8500_MAX_PORTS)
>> port = -1;
>> - else
>> + } else {
>> port = -1;
>> + }
>>
>> if (port < 0) {
>> /* calculate the port id */
>> --
>> 1.8.1.2
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe devicetree" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
Nice find and thanks for the fix.
Acked-by: Tony Prisk <linux@prisktech.co.nz>
Regards
Tony P
next prev parent reply other threads:[~2013-10-14 17:36 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-10-13 22:29 [PATCH] In absence of braces, port and NULL are missed Roel Kluin
2013-10-14 12:54 ` Mark Rutland
2013-10-14 17:37 ` Tony Prisk [this message]
2013-10-14 21:21 ` Roel Kluin
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=525C2BC2.1030207@prisktech.co.nz \
--to=linux@prisktech.co.nz \
--cc=devicetree@vger.kernel.org \
--cc=grant.likely@linaro.org \
--cc=linux-serial@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=rob.herring@calxeda.com \
--cc=roel.kluin@gmail.com \
/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.