Hi,
On 06/20/2014 10:02 AM, Olof Johansson wrote:
> On Fri, Jun 20, 2014 at 12:27 AM, Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
>> Hi,
>>
>> On 06/19/2014 07:18 PM, Olof Johansson wrote:
>>> Hi,
>>>
>>>
>>>
>>> On Thu, Jun 19, 2014 at 6:04 AM, Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote:
>>>> The pwrseq subsystem handles complex power sequences, typically useful
>>>> for subsystems that makes use of discoverable buses, like for example
>>>> MMC and I2C.
>>>>
>>>> The pwrseq subsystem is dependant on CONFIG_OF to be able to parse a DT
>>>> childnode to find out what power sequence method to bind for a device.
>>>>
>>>> From the DT childnode, the pwrseq DT parser tries to locate a
>>>> "power-method" property, which string is matched towards the list of
>>>> supported pwrseq methods.
>>>>
>>>> Each pwrseq method implements it's own power sequence and interfaces
>>>> the pwrseq core through a few callback functions.
>>>>
>>>> To instantiate a pwrseq method, clients shall use the devm_pwrseq_get()
>>>> API. If needed, clients can explicity drop the references to a pwrseq
>>>> method using devm_pwrseq_put() API.
>>>>
>>>> Besides instantiation, the pwrseq API provides clients opportunity to
>>>> select a certain power state. In this intial version, PWRSEQ_POWER_ON
>>>> and PWRSEQ_POWER_OFF are supported. Those are also mandatory for each
>>>> pwrseq method to support.
>>>>
>>>> Signed-off-by: Ulf Hansson <ulf.hansson@xxxxxxxxxx>
>>>> ---
>>>> .../devicetree/bindings/pwrseq/pwrseq.txt | 48 ++++++
>>>> drivers/Makefile | 2 +-
>>>> drivers/pwrseq/Makefile | 2 +
>>>> drivers/pwrseq/core.c | 175 ++++++++++++++++++++
>>>> drivers/pwrseq/core.h | 37 +++++
>>>> drivers/pwrseq/method.c | 38 +++++
>>>> include/linux/pwrseq.h | 50 ++++++
>>>> 7 files changed, 351 insertions(+), 1 deletion(-)
>>>> create mode 100644 Documentation/devicetree/bindings/pwrseq/pwrseq.txt
>>>> create mode 100644 drivers/pwrseq/Makefile
>>>> create mode 100644 drivers/pwrseq/core.c
>>>> create mode 100644 drivers/pwrseq/core.h
>>>> create mode 100644 drivers/pwrseq/method.c
>>>> create mode 100644 include/linux/pwrseq.h
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/pwrseq/pwrseq.txt b/Documentation/devicetree/bindings/pwrseq/pwrseq.txt
>>>> new file mode 100644
>>>> index 0000000..80848ae
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/pwrseq/pwrseq.txt
>>>> @@ -0,0 +1,48 @@
>>>> +Power sequence DT bindings
>>>> +
>>>> +Each power sequence method has a corresponding "power-method" property string.
>>>> +This property shall be set in a subnode for a device. That subnode should also
>>>> +describe resourses which are specific to that power method.
>>>> +
>>>> +Do note, power sequences as such isn't encoded through DT. Instead those are
>>>> +implemented by each power method.
>>>> +
>>>> +Required subnode properties:
>>>> +- power-method: should contain the string for the power method to bind.
>>>> +
>>>> + Supported power methods: None.
>>>> +
>>>> +Example:
>>>> +
>>>> +Note, the "clock" power method in this example isn't actually supported, but
>>>> +used to visualize how a childnode could be described.
>>>> +
>>>> +// WLAN SDIO channel
>>>> +sdi1_per2@80118000 {
>>>> + compatible = "arm,pl18x", "arm,primecell";
>>>> + reg = <0x80118000 0x1000>;
>>>> + interrupts = <0 50 IRQ_TYPE_LEVEL_HIGH>;
>>>> +
>>>> + dmas = <&dma 32 0 0x2>, /* Logical - DevToMem */
>>>> + <&dma 32 0 0x0>; /* Logical - MemToDev */
>>>> + dma-names = "rx", "tx";
>>>> +
>>>> + clocks = <&prcc_kclk 2 4>, <&prcc_pclk 2 6>;
>>>> + clock-names = "sdi", "apb_pclk";
>>>> +
>>>> + arm,primecell-periphid = <0x10480180>;
>>>> + max-frequency = <100000000>;
>>>> + bus-width = <4>;
>>>> + non-removable;
>>>> + pinctrl-names = "default", "sleep";
>>>> + pinctrl-0 = <&sdi1_default_mode>;
>>>> + pinctrl-1 = <&sdi1_sleep_mode>;
>>>> +
>>>> + status = "okay";
>>>> +
>>>> + pwrseq: pwrseq1 {
>>>> + power-method = "clock";
>>>> + clocks = <&someclk 1 2>, <&someclk 3 4>;
>>>> + clock-names = "pwrseq1", "pwrseq2";
>>>> + };
>>>
>>> I am strongly against the subnode approach as a general framework. We
>>> don't have a subnode for interrupts, nor for clocks or pinctrl. So why
>>> should we have it for the power sequencing?
>>>
>>> Sure, that fits the linux driver model better, but that's irrelevant
>>> w.r.t. describing the hardware.
>>
>> Actually this is about describing the hardware, when you have e.g. an
>> mmc device which needs pwrseq, there will be 2 sets of certain
>> resources, ie clocks for the host controller and clocks going directly
>> to the mmc device. I think putting those both in the same subnode is
>> a BAD idea, so we really do need a subnode to group the pwrseq resources
>> together.
>
> I disagree.
>
> The clock is the input to the module, and it is what needs to be
> enabled for the module to work. It's not the input to some
> power-sequence component on the module, or next to the module on the
> bus.
Right, it is an input to the sdio-module, not to the mmc-host, so its an
input to a different piece of hardware (at different ends of the mmc bus),
but since the mmc-bus normally is fully discoverable we've no node for the
other end of the bus.
So from the mmc-host pov, which is the one which needs to bind the pwrseq
driver, as that needs to be done before it can probe its bus, this is
a different piece of hardware, hence a subnode to the host makes perfect
sense. This is in no way part of the host, so certainly it does not belong
inside the hosts subnode.
> It probably makes sense to not use the standard names for the new
> resources.
I disagree, being able to use standard names is very useful, and actually
is a must if we don't want to have to have special versions of devm_get_clk,
and all other devm_get_xxx esp. for pwrseq stuff.
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
