Re: [PATCH 1/2] dt-bindings: drm/bridge: ti-sn65dsi83: Add enable delay property

[Marek Vasut <marex@denx.de>]

On 12/9/22 14:38, Alexander Stein wrote:
> Am Freitag, 9. Dezember 2022, 13:43:02 CET schrieb Marek Vasut:
>> On 12/9/22 13:21, Alexander Stein wrote:
>>> Hi Marek,
>>>
>>> Am Freitag, 9. Dezember 2022, 13:02:10 CET schrieb Marek Vasut:
>>>> On 12/9/22 10:36, Alexander Stein wrote:
>>>>> Hello Krzysztof,
>>>>
>>>> Hi,
>>>>
>>>>> Am Freitag, 9. Dezember 2022, 10:07:45 CET schrieb Krzysztof Kozlowski:
>>>>>> On 09/12/2022 09:54, Alexander Stein wrote:
>>>>>>> Hello Krzysztof,
>>>>>>>
>>>>>>> thanks for the fast feedback.
>>>>>>>
>>>>>>> Am Freitag, 9. Dezember 2022, 09:39:49 CET schrieb Krzysztof
> Kozlowski:
>>>>>>>> On 09/12/2022 09:33, Alexander Stein wrote:
>>>>>>>>> It takes some time until the enable GPIO has settled when turning
>>>>>>>>> on.
>>>>>>>>> This delay is platform specific and may be caused by e.g. voltage
>>>>>>>>> shifts, capacitors etc.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
>>>>>>>>> ---
>>>>>>>>>
>>>>>>>>>     .../devicetree/bindings/display/bridge/ti,sn65dsi83.yaml      | 4
>>>>>>>>>     ++++
>>>>>>>>>     1 file changed, 4 insertions(+)
>>>>>>>>>
>>>>>>>>> diff --git
>>>>>>>>> a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
>>>>>>>>> b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
>>>>>>>>> index 48a97bb3e2e0d..3f50d497cf8ac 100644
>>>>>>>>> ---
>>>>>>>>> a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
>>>>>>>>> +++
>>>>>>>>> b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
>>>>>>>>>
>>>>>>>>> @@ -32,6 +32,10 @@ properties:
>>>>>>>>>         maxItems: 1
>>>>>>>>>         description: GPIO specifier for bridge_en pin (active high).
>>>>>>>>>
>>>>>>>>> +  ti,enable-delay-us:
>>>>>>>>> +    default: 10000
>>>>>>>>> +    description: Enable time delay for enable-gpios
>>>>>>>>
>>>>>>>> Aren't you now mixing two separate delays? One for entire block on (I
>>>>>>>> would assume mostly fixed delay) and one depending on regulators
>>>>>>>> (regulator-ramp-delay, regulator-enable-ramp-delay). Maybe you miss
>>>>>>>> the
>>>>>>>> second delays in your power supply? If so, the first one might be
>>>>>>>> fixed
>>>>>>>> and hard-coded in the driver?
>>>>>>>
>>>>>>> Apparently there are two different delays: reset time (t_reset) of
>>>>>>> 10ms
>>>>>>> as
>>>>>>> specified by datasheet. This is already ensured by a following delay
>>>>>>> after
>>>>>>> requesting enable_gpio as low and switching the GPIO to low in disable
>>>>>>> path.
>>>>>>>
>>>>>>> When enabling this GPIO it takes some time until it is valid on the
>>>>>>> chip,
>>>>>>> this is what this series is about. It's highly platform specific.
>>>>>>>
>>>>>>> Unfortunately this is completely unrelated to the vcc-supply
>>>>>>> regulator.
>>>>>>> This one has to be enabled before the enable GPIO can be enabled. So
>>>>>>> there is no regulator-ramp-delay.
>>>>>>
>>>>>> Your driver does one after another - regulator followed immediately by
>>>>>> gpio - so this as well can be a delay from regulator (maybe not ramp
>>>>>> but
>>>>>> enable delay).
>>>>
>>>> The chip has two separate input pins:
>>>>
>>>> VCC -- power supply that's regulator
>>>> EN -- reset line, that's GPIO
>>>>
>>>> Alexander is talking about EN line here.
>>>>
>>>>> But this will introduce a section which must not be interrupted or
>>>>> delayed.
>>>>> This is impossible as the enable gpio is attached to an i2c expander in
>>>>> my
>>>>> case.
>>>>>
>>>>> Given the following time chart:
>>>>>     vcc                  set             EN
>>>>>
>>>>> enable               GPIO             PAD
>>>>>
>>>>>      |                    |<-- t_raise -->|
>>>>>      |
>>>>>      | <-- t_vcc_gpio --> |               |
>>>>>      | <--        t_enable_delay      --> |
>>>>>
>>>>> t_raise is the time from changing the GPIO output at the expander until
>>>>> voltage on the EN (input) pad from the bridge has reached high voltage
>>>>> level. This is an electrical characteristic I can not change and have to
>>>>> take into account.
>>>>> t_vcc_gpio is the time from enabling supply voltage to enabling the
>>>>> bridge
>>>>> (removing from reset). Minimum t_vcc_gpio is something which can be
>>>>> addressed by the regulator and is no problem so far. But there is no
>>>>> upper bound to it.
>>>>
>>>> What exactly is your EN signal rise time (should be ns or so)? Can you
>>>> look at that with a scope , maybe even with relation to the VCC regulator
>>>> ?
>>>
>>> I checked EN rise time using a scope, it's ~110ms. I not an expert in
>>> hardware but on the mainboard there is some capacitor attached to this
>>> line, which increased the time, independent from the internal pull-up.
>>
>> This does seem like a hardware bug right there, can you double-check
>> this with the hardware engineer ?
> 
> Yep, checked with hardware engineer. An 470nF is attached, together with an
> open drain output and only the internal pull-up. So yes ~113ms rising time
> until 0.7 x VCC.

I don't suppose you can have that capacitor reduced or better yet, some 
external pull up added, can you ?