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

Stefan Roese sr at denx.de
Wed Sep 30 17:35:19 CEST 2020


On 30.09.20 16:25, Hilliard, George wrote:
> 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.

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


More information about the U-Boot mailing list