On 30.09.20 16:25, Hilliard, George wrote:
On Sep 30, 2020, at 12:20 AM, Stefan Roese <s...@denx.de> wrote:
Hi George,
thanks for the new version. One nitpicking comment below...
On 29.09.20 20:34, George Hilliard wrote:
+ /* Set up CS GPIOs in device tree, if any */
+ if (CONFIG_IS_ENABLED(DM_GPIO) && gpio_get_list_count(bus, "cs-gpios") >
0) {
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(plat->cs_gpios); i++) {
+ ret = gpio_request_by_name(bus, "cs-gpios", i,
&plat->cs_gpios[i], 0);
+ if (ret < 0 || !dm_gpio_is_valid(&plat->cs_gpios[i]))
+ // Use the native CS function for this line
+ continue;
Why do you introduce here C++ style comments? Even though they are
not banned any more (AFAIK), I see no reason to add multiple
comment styles in such a small patch. Additionally this is a multi-
line statement after the "if" and it needs parenthesis.
Both good points. The C++ style is by force of habit alone, no real reason for
it here.
I suppose "multi-line" means multiple lines of text, not multiple C statements.
Yes. AFAIU, it's just related to the number of lines following an
"if" (etc) statement. Which makes perfect sense IMHO. ;)
Other than that, please feel free to add my:
Reviewed-by: Stefan Roese <s...@denx.de>
to the next version.
Your review is much appreciated!
Thanks.
I do have another comment, for the next patch(es) you might send:
Please add a patch history to your patches when sending updated
versions. Please see here:
http://www.denx.de/wiki/view/U-Boot/Patches#Sending_updated_patch_versions
No need to resend this v3 patch though.
Thanks,
Stefan
Thanks,
Stefan
Viele Grüße,
Stefan
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de
George
Viele Grüße,
Stefan
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de
