From: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> To: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>, Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>, Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>, "linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, "devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, "linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>, Baolin Wang <baolin.wang-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> Subject: Re: [PATCH v4 1/3] pinctrl: Add sleep related configuration Date: Wed, 12 Jul 2017 14:30:25 +0200 [thread overview] Message-ID: <CACRpkdapE2+7jR3J0Qw5Hnvg-19-sR5uCYnvMoHiAbowm8-Uug@mail.gmail.com> (raw) In-Reply-To: <20170627120626.GA1067-lxIno14LUO0EEoCn2XhGlw@public.gmane.org> On Tue, Jun 27, 2017 at 2:06 PM, Baolin Wang <baolin.wang-lxIno14LUO0EEoCn2XhGlw@public.gmane.org> wrote: > If we introduce "sleep-input-enable" config, we can set the pin's config > as below: > > vio_sd0_ms_3: regctrl3 { > pins = "SC9860_RFCTL30", "SC9860_RFCTL31", "SC9860_RFCTL32"; > function = "func1"; > sprd,sleep-mode = <0x3>; > sleep-input-enable; > }; This looks like a "default" mode. Is that correct? I.e. do you set up this on probe then do not touch it? It seems some of the problems come from the insistance to use a single node for all configuration. Compare to this nomadik: i2c0 { i2c0_default_mux: i2c0_mux { i2c0_default_mux { function = "i2c0"; groups = "i2c0_a_1"; }; }; i2c0_default_mode: i2c0_default { i2c0_default_cfg { pins = "GPIO62_D3", "GPIO63_D2"; input-enable; }; }; }; It is easy to imagine: i2c0 { i2c0_default_mux: i2c0_mux { i2c0_default_mux { function = "i2c0"; groups = "i2c0_a_1"; }; }; i2c0_default_mode: i2c0_default { i2c0_default_cfg { pins = "GPIO62_D3", "GPIO63_D2"; input-enable; }; }; i2c0_default_mode_sleep: i2c0_default_sleep { i2c0_default_cfg { pins = "GPIO62_D3", "GPIO63_D2"; sleep-hardware-state; input-disable; }; }; }; Notice the new bool property "sleep-hardware-state" that just indicate that this should be programmed into the registers for the sleep state. > But If we create one extra "sleep-xxx" state for sleep-related configs, > it will be like: > > grp1: regctrl3 { > pins = "SC9860_RFCTL30", "SC9860_RFCTL31"; > function = "func1"; > sprd,sleep-mode = <0x3>; > }; > > sleep-input: input_grp { > pins = "SC9860_RFCTL30", "SC9860_RFCTL31", "SC9860_RFCTL32"; > input-enable; > }; > > pinctrl-names = "sleep-input"; > pinctrl-0 = <&sleep-input>; > > "sleep-input" state will be selected when initializing pinctrl driver, The state you should use for initial configuration should be called just "init". > "grp1" > will be selected by user to set other pin configuration. Like "default"? > Then we need config "SC9860_RFCTL30" pin in 2 different places, which is > more inconvenient for users. I'm not so sure about that. Having a lot more sleep,* config options may be even more inconvenient for users, and especially for the community of developers as a whole. Several config nodes on the other hand, we have had in the pin control subsystem since day 1. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html
WARNING: multiple messages have this Message-ID (diff)
From: Linus Walleij <linus.walleij@linaro.org> To: Rob Herring <robh@kernel.org>, Linus Walleij <linus.walleij@linaro.org>, Mark Rutland <mark.rutland@arm.com>, "linux-gpio@vger.kernel.org" <linux-gpio@vger.kernel.org>, "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, Mark Brown <broonie@kernel.org>, Baolin Wang <baolin.wang@linaro.org> Subject: Re: [PATCH v4 1/3] pinctrl: Add sleep related configuration Date: Wed, 12 Jul 2017 14:30:25 +0200 [thread overview] Message-ID: <CACRpkdapE2+7jR3J0Qw5Hnvg-19-sR5uCYnvMoHiAbowm8-Uug@mail.gmail.com> (raw) In-Reply-To: <20170627120626.GA1067@spreadtrum.com> On Tue, Jun 27, 2017 at 2:06 PM, Baolin Wang <baolin.wang@spreadtrum.com> wrote: > If we introduce "sleep-input-enable" config, we can set the pin's config > as below: > > vio_sd0_ms_3: regctrl3 { > pins = "SC9860_RFCTL30", "SC9860_RFCTL31", "SC9860_RFCTL32"; > function = "func1"; > sprd,sleep-mode = <0x3>; > sleep-input-enable; > }; This looks like a "default" mode. Is that correct? I.e. do you set up this on probe then do not touch it? It seems some of the problems come from the insistance to use a single node for all configuration. Compare to this nomadik: i2c0 { i2c0_default_mux: i2c0_mux { i2c0_default_mux { function = "i2c0"; groups = "i2c0_a_1"; }; }; i2c0_default_mode: i2c0_default { i2c0_default_cfg { pins = "GPIO62_D3", "GPIO63_D2"; input-enable; }; }; }; It is easy to imagine: i2c0 { i2c0_default_mux: i2c0_mux { i2c0_default_mux { function = "i2c0"; groups = "i2c0_a_1"; }; }; i2c0_default_mode: i2c0_default { i2c0_default_cfg { pins = "GPIO62_D3", "GPIO63_D2"; input-enable; }; }; i2c0_default_mode_sleep: i2c0_default_sleep { i2c0_default_cfg { pins = "GPIO62_D3", "GPIO63_D2"; sleep-hardware-state; input-disable; }; }; }; Notice the new bool property "sleep-hardware-state" that just indicate that this should be programmed into the registers for the sleep state. > But If we create one extra "sleep-xxx" state for sleep-related configs, > it will be like: > > grp1: regctrl3 { > pins = "SC9860_RFCTL30", "SC9860_RFCTL31"; > function = "func1"; > sprd,sleep-mode = <0x3>; > }; > > sleep-input: input_grp { > pins = "SC9860_RFCTL30", "SC9860_RFCTL31", "SC9860_RFCTL32"; > input-enable; > }; > > pinctrl-names = "sleep-input"; > pinctrl-0 = <&sleep-input>; > > "sleep-input" state will be selected when initializing pinctrl driver, The state you should use for initial configuration should be called just "init". > "grp1" > will be selected by user to set other pin configuration. Like "default"? > Then we need config "SC9860_RFCTL30" pin in 2 different places, which is > more inconvenient for users. I'm not so sure about that. Having a lot more sleep,* config options may be even more inconvenient for users, and especially for the community of developers as a whole. Several config nodes on the other hand, we have had in the pin control subsystem since day 1. Yours, Linus Walleij
next prev parent reply other threads:[~2017-07-12 12:30 UTC|newest] Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-06-21 11:55 [PATCH v4 1/3] pinctrl: Add sleep related configuration Baolin Wang 2017-06-21 11:55 ` Baolin Wang [not found] ` <62a67526907157d41235403bd05af309fd4db16b.1498045588.git.baolin.wang-lxIno14LUO0EEoCn2XhGlw@public.gmane.org> 2017-06-21 11:55 ` [PATCH v4 2/3] dt-bindings: pinctrl: Add Spreadtrum SC9860 pin controller Baolin Wang 2017-06-21 11:55 ` Baolin Wang 2017-06-21 11:55 ` [PATCH v4 3/3] pinctrl: sprd: Add Spreadtrum pin control driver Baolin Wang 2017-06-21 11:55 ` Baolin Wang [not found] ` <fac8c9ae08fb42bab47d5349720ee097b25222cb.1498045588.git.baolin.wang-lxIno14LUO0EEoCn2XhGlw@public.gmane.org> 2017-06-27 12:33 ` kbuild test robot 2017-06-27 12:33 ` kbuild test robot 2017-06-26 16:13 ` [PATCH v4 1/3] pinctrl: Add sleep related configuration Rob Herring 2017-06-27 8:21 ` Baolin Wang 2017-06-27 8:21 ` Baolin Wang 2017-06-27 12:06 ` Baolin Wang 2017-06-27 12:06 ` Baolin Wang [not found] ` <20170627120626.GA1067-lxIno14LUO0EEoCn2XhGlw@public.gmane.org> 2017-07-07 8:13 ` Baolin Wang 2017-07-07 8:13 ` Baolin Wang 2017-07-12 12:30 ` Linus Walleij [this message] 2017-07-12 12:30 ` Linus Walleij 2017-07-13 6:35 ` Baolin Wang
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CACRpkdapE2+7jR3J0Qw5Hnvg-19-sR5uCYnvMoHiAbowm8-Uug@mail.gmail.com \ --to=linus.walleij-qsej5fyqhm4dnm+yrofe0a@public.gmane.org \ --cc=baolin.wang-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \ --cc=broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \ --cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \ --cc=linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \ --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \ --cc=mark.rutland-5wv7dgnIgG8@public.gmane.org \ --cc=robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.