linux-kernel - Re: [PATCH v2 2/3] pinctrl: Add msm8x74 configuration

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:	Mon, 9 Dec 2013 09:18:33 +0100
From:	Linus Walleij <linus.walleij@...aro.org>
To:	Stephen Boyd <sboyd@...eaurora.org>
Cc:	Bjorn Andersson <bjorn.andersson@...ymobile.com>,
	Rob Herring <rob.herring@...xeda.com>,
	Pawel Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>,
	Stephen Warren <swarren@...dotorg.org>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Rob Landley <rob@...dley.net>,
	Grant Likely <grant.likely@...aro.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-arm-msm@...r.kernel.org" <linux-arm-msm@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v2 2/3] pinctrl: Add msm8x74 configuration

On Fri, Dec 6, 2013 at 11:22 PM, Stephen Boyd <sboyd@...eaurora.org> wrote:
> On 12/05/13 18:10, Bjorn Andersson wrote:

As the driver is merged I expect fixes to come in as additional patches.

>> Add initial definition of parameters for pinctrl-msm for the msm8x74
>> platform.
>
> Hmm. We've tried to remove 'x' from our code because it isn't really
> accurate and leads to more confusion.

So does this pin controller have a real name in the data sheet?

I usually prefer to name the drivers after the name of the IP
block rather than the SoC if possible.

Or should it just be named pinctrl-msm.c?

>> +#define PINGROUP(id, f1, f2, f3, f4, f5, f6, f7)     \
>> +     {                                               \
>> +             .name = "gpio" #id,                     \
>> +             .pins = gpio##id##_pins,                \
>> +             .npins = ARRAY_SIZE(gpio##id##_pins),   \
>> +             .funcs = {                              \
>> +                     MSM_MUX_NA, /* gpio mode */     \
>> +                     MSM_MUX_##f1,                   \
>> +                     MSM_MUX_##f2,                   \
>> +                     MSM_MUX_##f3,                   \
>> +                     MSM_MUX_##f4,                   \
>> +                     MSM_MUX_##f5,                   \
>> +                     MSM_MUX_##f6,                   \
>> +                     MSM_MUX_##f7                    \
>> +             },                                      \
>> +             .ctl_reg = 0x1000 + 0x10 * id ,         \
>
> Weird trailing space here.

Please send patches.

> Also, do we ever plan to have anything more than the gpio pins and the
> sdc pins? It seems like we spend a lot of space describing exactly the
> same thing in these structs for each of the 146 gpio pins when we could
> just know that range 0 to 146 is gpio pins and have different code for
> that part vs the 6 or something sd pins.

Some platforms use the .gpio_request_enable()/.gpio_disable_free()
instead of one group per pin for this very reason.

>> +static struct of_device_id msm8x74_pinctrl_of_match[] = {
>
> const?

Please send patches.

>> +static int __init msm8x74_pinctrl_init(void)
>> +{
>> +     return platform_driver_register(&msm8x74_pinctrl_driver);
>> +}
>> +arch_initcall(msm8x74_pinctrl_init);
>> +
>> +static void __exit msm8x74_pinctrl_exit(void)
>> +{
>> +     platform_driver_unregister(&msm8x74_pinctrl_driver);
>> +}
>> +module_exit(msm8x74_pinctrl_exit);
>
> Why not module_platform_driver()? I thought pinctrl supported deferred
> probing?

It does. Tested patches accepted.

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/