Re: [PATCH 4/4] mmc: pwrseq-simple: add disable-post-power-on option

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On Wed, Dec 14, 2016 at 5:46 PM, Matt Ranostay <matt@ranostay.consulting> wrote:
> On Tue, Dec 13, 2016 at 12:01 AM, Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote:
>> On 9 December 2016 at 19:09, Rob Herring <robh@xxxxxxxxxx> wrote:
>>> On Fri, Dec 02, 2016 at 01:06:47AM -0800, Matt Ranostay wrote:
>>>> On Fri, Dec 2, 2016 at 12:28 AM, Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote:
>>>> > On 2 December 2016 at 08:22, Matt Ranostay <matt@ranostay.consulting> wrote:
>>>> >>
>>>> >>
>>>> >>> On Dec 1, 2016, at 23:13, Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote:
>>>> >>>
>>>> >>>> On 2 December 2016 at 07:17, Matt Ranostay <matt@ranostay.consulting> wrote:
>>>> >>>> Add optional device-post-power-on parameters to disable post_power_on
>>>> >>>> function from being called since this breaks some device.
>>>> >>>>
>>>> >>>> Cc: Tony Lindgren <tony@xxxxxxxxxxx>
>>>> >>>> Cc: Ulf Hansson <ulf.hansson@xxxxxxxxxx>
>>>> >>>> Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
>>>> >>>> ---
>>>> >>>> Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt | 2 ++
>>>> >>>> drivers/mmc/core/pwrseq_simple.c                            | 7 +++++++
>>>> >>>> 2 files changed, 9 insertions(+)
>>>> >>>>
>>>> >>>> diff --git a/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt b/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
>>>> >>>> index bea306d772d1..09fa153f743e 100644
>>>> >>>> --- a/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
>>>> >>>> +++ b/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
>>>> >>>> @@ -24,6 +24,8 @@ Optional properties:
>>>> >>>>        de-asserting the reset-gpios (if any)
>>>> >>>> - invert-off-state: Invert the power down state for the reset-gpios (if any)
>>>> >>>>        and pwrdn-gpios (if any)
>>>> >>>> +- disable-post-power-on : Avoid post_power_on function from being called since
>>>> >>>> +       this breaks some devices
>>>> >>>
>>>> >>> This is a bit weird. I would like to avoid this, if possible.
>>>> >>>
>>>> >>> Perhaps if you simply describe the sequence in detail for your device
>>>> >>> we can figure out the best option together.
>>>> >>
>>>> >> Yeah I know it is a bit weird but we need to keep that reset pin high for at least some time after pre power on.   So any case it would be another property
>>>> >
>>>> > This went offlist, please resend.
>>>> >
>>>> > Moreover, please try to describe the sequence you need in detail
>>>> > according to your HW spec.
>>>> >
>>>>
>>>> Yeah oops....
>>>>
>>>> So basically we need to drive the reset and powerdown lines high with
>>>> a 300 milliseconds delay between both...
>>>> can't have the reset line low with post power on (pretty sure but
>>>> would need a delay anyway), and we need both reset + powerdown line
>>>> set low on powerdown.
>>>>
>>>> So the power down sequence would need to be reversed for this
>>>> application in pwrseq-simple.
>>>
>>> This sounds like you need a device specific sequence to me. Otherwise,
>>> write a language to describe any power control waveforms rather than
>>> trying to bolt on more and more properties. (Don't really go write a
>>> language.)
>>
>> Actually this isn't so device specific. The cw1200 wifi chip which is
>> being used with ux500 SoCs has a very similar sequence. Allow me to
>> check the details and get back.
>
> Ok. It would be good to have something common than a bunch of one-off
> solutions for sure.
>
>>
>> Anyway, my point is that I think we can figure out a common sequence
>> to support these kind required sequences. That is to me preferred over
>> adding a new (two to support cw1200) device specific pwrseq driver.
>>

So briefly looked at the cw1200 driver (I'm guessing the datasheet is
NDA since I couldn't find it) and it seems like it would be a good fit
to have a common pwrseq driver.

So minor things I noticed that the cw1200 device has different from
our SD8787 chipset.

* SD8787 has a "powerdown" line, and CW1200 has a "powerup" line.. I
know this is a simple logic inversion.
* CW1200 has a clock control line, that seems to be asserted between
the powerup/reset steps.

Would there need to be some per driver functions that could be
registered for edge cases like this?

>> Kind regards
>> Uffe
--
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



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux