Re: [PATCH v2 1/2] DT: pinctrl: Add binding documentation for Spreadtrum pin controller

[Baolin Wang]

On 26 June 2017 at 06:19, Linus Walleij <linus.walleij@xxxxxxxxxx> wrote:
> On Wed, Jun 21, 2017 at 10:10 AM, Baolin Wang <baolin.wang@xxxxxxxxxx> wrote:
>> On 20 June 2017 at 17:31, Linus Walleij <linus.walleij@xxxxxxxxxx> wrote:
>>> On Tue, Jun 13, 2017 at 5:15 AM, Baolin Wang <baolin.wang@xxxxxxxxxxxxxx> wrote:
>>>
>>>> I forgot one most important reason why we can not use the "sleep" state. As I explained
>>>> above, the sleep related configuration will bind with the pin's sleep mode. If we set the
>>>> pin's sleep mode as AP_SLEEP, then we can select "sleep" state when AP system goes into
>>>> deep sleep mode by issuing "pinctrl_force_sleep()" in pinctrl suspend function.
>>>>
>>>> But if we set the pin's sleep mode as PUBCP_SLEEP and pubcp system doesn't run linux kernel
>>>> (it run another thread OS), then we can not select "sleep" state since the AP system does
>>>> not go into deep sleep mode (AP system run linux kernel OS).
>>>
>>> Allright yes it makes sense, and also there are systems that just go into
>>> "hardware sleep" and just put the pin into some pre-programmed mode.
>>>
>>> I'm a bit back-and-forth. I didn't mean that some code would actually
>>> switch the state to "sleep" when we go to sleep, I meant that when
>>> the system configures "default" mode it should also look up and
>>> program the "sleep" mode, but this approach with a special property
>>> is just another way of achieveing the same thing.
>>>
>>> But then we should add a whole slew of sleep states.
>>>
>>> I was thinking whether we could avoid having a special DT property
>>> by parsing ahead to states we do not currently use and programming
>>> that into the sleep mode registers.
>>
>> Yes, for most scenarios, it can work with the "sleep" state to set
>> sleep-related config. But for our Spreadtrum platform scenario (I do
>> not know if there are other platforms need this feature), we can not
>> select the "sleep" state when pubcp system goes into deep sleep mode
>> but ap system does not go into deep sleep mode. So I think we still
>> need these "sleep-bias-pull-up", "sleep-bias-pull-down",
>> "sleep-input-enable" and "sleep-output-enable" properties.
>
> I don't really mean you should select the "sleep" state.
>
> I meant, as part of setting the "default" state or even the "init"
> state, we would inspect the "sleep" state, use those settings, and
> program them into the registers at this early point.

I understood your points. But we can not program all into the
registers at one early point, sometimes these sleep-related configs
need depend on some conditions in users' drivers, like on condition 1:
driver need to set one pin as input-enable when specified system goes
into deep sleep, on condition 2: driver need set this pin as
output-enable when specified system goes into deep sleep. So I still
think it is better if we introduce some standard sleep related
configs.

>
> Then never touch the registers again, and never really go to the
> sleep state by software, just by hardware.
>
> Yours,
> Linus Walleij



-- 
Baolin.wang
Best Regards