[PATCH v2] spi: mvebu_a3700_spi: add support for cs-gpios

[Hilliard, George]

On Sep 30, 2020, at 12:20 AM, Stefan Roese <sr at 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.

> 
> Other than that, please feel free to add my:
> 
> Reviewed-by: Stefan Roese <sr at denx.de>
> 
> to the next version.

Your review is much appreciated!

> 
> 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: sr at denx.de

George