All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tom Rini <trini@konsulko.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 2/4] rapidio: add support for RapidIO boot
Date: Mon, 4 Jan 2016 10:51:00 -0500	[thread overview]
Message-ID: <20160104155100.GI4093@bill-the-cat> (raw)
In-Reply-To: <1447157610-3680-1-git-send-email-jacquiot.aurelien@gmail.com>

On Tue, Nov 10, 2015 at 01:13:29PM +0100, jacquiot.aurelien at gmail.com wrote:

> From: Aurelien Jacquiot <a-jacquiot@ti.com>
> 
> RapidIO is a high-performance packet-switched interconnect technology
> (www.rapidio.org) that can be used for messaging or memory transfers
> like Ethernet or PCIe.
[snip]
> +config CMD_RIO
> +        bool "rapidio"
> +        help
> +          RapidIO support.

Please expand this to include what is and is not supported in RapidIO
right now with your patches.

[snip]
> +	/* hostid */
> +	if (argc >= 2) {
> +		riohdid = simple_strtol(argv[2], NULL, 10);
> +	} else {
> +		s = getenv("riohdid");
> +		if (s != NULL)
> +			riohdid = simple_strtol(s, NULL, 10);
> +		goto call_init;
> +	}

Here and elsewhere, command inputs are in hex not decimal.  'sleep' is
the one and only exception (and causes headaches) to this rule.

> +	/* dev-id size */
> +	if (argc >= 3) {
> +		riosize = simple_strtol(argv[3], NULL, 10);
> +	} else {
> +		s = getenv("riosize");
> +		if (s != NULL)
> +			riosize = simple_strtol(s, NULL, 10);
> +		goto call_init;
> +	}
> +
> +	/* port bitmask */
> +	if (argc >= 4) {
> +		rioports = simple_strtoul(argv[4], NULL, 16);
> +	} else {
> +		s = getenv("rioports");
> +		if (s != NULL)
> +			rioports = simple_strtoul(s, NULL, 16);
> +		goto call_init;
> +	}
> +
> +	/* port path mode */
> +	if (argc >= 5) {
> +		riopmode = simple_strtol(argv[5], NULL, 10);
> +	} else {
> +		s = getenv("riopathmode");
> +		if (s != NULL)
> +			riopmode = simple_strtol(s, NULL, 10);
> +		goto call_init;
> +	}
> +
> +	/* baudrate */
> +	if (argc >= 6) {
> +		riobaudrate = simple_strtol(argv[6], NULL, 10);
> +	} else {
> +		s = getenv("riobaudrate");
> +		if (s != NULL)
> +			riobaudrate = simple_strtol(s, NULL, 10);
> +	}

At first glance this seems broken for the case where one of
riopathmode/riobaudrate/etc are neither passed nor in the environment.
Or will rio_init handle this case correctly and pass back failure?
Also, is there a usecase for having these be set in the environment
rather than passed?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160104/ce932fb7/attachment.sig>

      reply	other threads:[~2016-01-04 15:51 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-10 12:13 [U-Boot] [PATCH 2/4] rapidio: add support for RapidIO boot jacquiot.aurelien at gmail.com
2016-01-04 15:51 ` Tom Rini [this message]

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=20160104155100.GI4093@bill-the-cat \
    --to=trini@konsulko.com \
    --cc=u-boot@lists.denx.de \
    /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.