gpios search behaviour for gpio from _DSD

gpios search behaviour for gpio from _DSD

[Olliver Schinagl]

Hey list, Mika,

With commit 0d9a693cc86 the following snippet of code was added:

+       /* Try first from _DSD */
+       for (i = 0; i < ARRAY_SIZE(suffixes); i++) {
+               if (con_id && strcmp(con_id, "gpios")) {
+                       snprintf(propname, sizeof(propname), "%s-%s",
+                                con_id, suffixes[i]);

and I was wondering why the gpios suffix is singled out. Are we not 
allowed to check for all the strings in the suffixes array? Is gpios 
special or is gpio simply not allowed. If that strcmp check would be 
removed, would bad things happen?

Also, just to educate myself, isn't relying on left to right parsing 
complier specifc? E.g. if con_id is null, we end up passing NULL to 
strcmp and atleast for libc can cause segfaults iirc.

Thanks,
Olliver

Re: gpios search behaviour for gpio from _DSD

[Mika Westerberg]

On Thu, Sep 24, 2015 at 09:25:06AM +0200, Olliver Schinagl wrote:
> Hey list, Mika,
> 
> With commit 0d9a693cc86 the following snippet of code was added:
> 
> +       /* Try first from _DSD */
> +       for (i = 0; i < ARRAY_SIZE(suffixes); i++) {
> +               if (con_id && strcmp(con_id, "gpios")) {
> +                       snprintf(propname, sizeof(propname), "%s-%s",
> +                                con_id, suffixes[i]);
> 
> and I was wondering why the gpios suffix is singled out. Are we not allowed
> to check for all the strings in the suffixes array? Is gpios special or is
> gpio simply not allowed. If that strcmp check would be removed, would bad
> things happen?

We default to "gpios". So if you pass "reset" we actually look for
proprerty "reset-gpios". This is the recommend syntax AFAIK.

> Also, just to educate myself, isn't relying on left to right parsing
> complier specifc? E.g. if con_id is null, we end up passing NULL to strcmp
> and atleast for libc can cause segfaults iirc.

'&&' is so called short circuit operator so if we already know that
con_id is NULL we never evaluate the remaining conditions.

Re: gpios search behaviour for gpio from _DSD

[Olliver Schinagl]

Hey Mika,

Thanks for your reply.

On 24-09-15 10:43, Mika Westerberg wrote:
> On Thu, Sep 24, 2015 at 09:25:06AM +0200, Olliver Schinagl wrote:
>> Hey list, Mika,
>>
>> With commit 0d9a693cc86 the following snippet of code was added:
>>
>> +       /* Try first from _DSD */
>> +       for (i = 0; i < ARRAY_SIZE(suffixes); i++) {
>> +               if (con_id && strcmp(con_id, "gpios")) {
>> +                       snprintf(propname, sizeof(propname), "%s-%s",
>> +                                con_id, suffixes[i]);
>>
>> and I was wondering why the gpios suffix is singled out. Are we not allowed
>> to check for all the strings in the suffixes array? Is gpios special or is
>> gpio simply not allowed. If that strcmp check would be removed, would bad
>> things happen?
> We default to "gpios". So if you pass "reset" we actually look for
> proprerty "reset-gpios". This is the recommend syntax AFAIK.
What i ment is, without strcmp, the loop would test for:

reset-gpio and then reset-gpios (and then gpio and gpios)

So why the strcmp at all, Why only reset-gpios? Will the _DSD or acpi 
break and if we probe for reset-gpio (first, instead of reset-gpios)? 
I'm asking because i'm trying some work on gpio lib and this section is 
getting reworked too. While doing that I want to remove the strcmp. 
Hence, the question if things break on the ACPI side (I'm mostly failiar 
with OF.
>
>> Also, just to educate myself, isn't relying on left to right parsing
>> complier specifc? E.g. if con_id is null, we end up passing NULL to strcmp
>> and atleast for libc can cause segfaults iirc.
> '&&' is so called short circuit operator so if we already know that
> con_id is NULL we never evaluate the remaining conditions.
I just looked it up and of course you are right and I learned something new:

(4). Unlike the bitwise binary & operator, the && operator guarantees 
left-to-right evaluation; there is a sequence point after the evaluation 
of the first operand. If the first operand compares equal to 0, the 
second operand is not evaluated.

So && is guaranteed left to right.

Olliver

Re: gpios search behaviour for gpio from _DSD

[Mika Westerberg]

On Sat, Sep 26, 2015 at 09:30:59PM +0200, Olliver Schinagl wrote:
> What i ment is, without strcmp, the loop would test for:
> 
> reset-gpio and then reset-gpios (and then gpio and gpios)
> 
> So why the strcmp at all, Why only reset-gpios? Will the _DSD or acpi break
> and if we probe for reset-gpio (first, instead of reset-gpios)? I'm asking
> because i'm trying some work on gpio lib and this section is getting
> reworked too. While doing that I want to remove the strcmp. Hence, the
> question if things break on the ACPI side (I'm mostly failiar with OF.

I think it is there because we really want to force "-gpios" format for
named gpios (not "-gpio"). DT does not do it because they have lots of
existing "-gpio" users whereas in ACPI there is no such burden.