linux-kernel - Re: [PATCH 3/3] pinctrl: qcom: handle input-enable pinconf property

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 4 Feb 2015 11:03:36 +0100
From:	Linus Walleij <linus.walleij@...aro.org>
To:	Bjorn Andersson <bjorn.andersson@...ymobile.com>
Cc:	Stanimir Varbanov <svarbanov@...sol.com>,
	"linux-arm-msm@...r.kernel.org" <linux-arm-msm@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
	Stephen Boyd <sboyd@...eaurora.org>
Subject: Re: [PATCH 3/3] pinctrl: qcom: handle input-enable pinconf property

On Fri, Jan 30, 2015 at 5:20 PM, Bjorn Andersson
<bjorn.andersson@...ymobile.com> wrote:
> On Fri 30 Jan 02:27 PST 2015, Stanimir Varbanov wrote:
>
>> +     case PIN_CONFIG_INPUT_ENABLE:
>> +             /* Pin is output */
>> +             if (arg)
>> +                     return -EINVAL;
>> +             arg = 1;
>> +             break;
>
> My idea of this function is to query if we have the specific option
> enabled, so I don't like the fact that we're returning an error here, we
> should just return 0 with arg 0 (or something like that).
>
> However, that would not give the results we expect and your patch is
> "correct".
>
> Linus, conf_items in pinconf_generic_dump_one() seems to represent
> boolean properties of the pins. Returning 0 from pin_config_*_get()
> should in my view then be treated as it not being active.
>
> Is this in line with your view and should we modify
> pinconf_generic_dump_one() to continue for these values if the getter
> returns 0?
>
> If not, at least all the bias properties here should return -EINVAL as
> well. (which I think is wrong)

Well currently the semantics are:

- ENOTSUPP = this property is not even supported
- EINVAL       = this value exists but can not be determined

It has this form primarily to serve the non-boolean properties.
For example pull-up can return -EINVAL if pull-up is supported
but pull-down is currently active, so it cannot say what
resistance it is pulled up with, as it is "infinite" (NAN,
thus translated -EINVAL).

It just folds over to the boolean props doing things in the
same way to simplify things... -EINVAL just means
"false". If we should return 1/0 from boolean props we need
to handle them as a special case in the pinconf-generic.
code, by extending the struct pinconf_generic_params,
which is possible of course.

Further: as of now pinconf_generic_dump_one() doesn't print
anything for inactive pulls etc return -EINVAL, but maybe
it should? It was just handy on some system to only see
the stuff that was really active, not to get a list of stuff that
was not active as well.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/