On 28/05/2024 14:57, Konrad Dybcio wrote:
> On 5/28/24 14:45, Marc Gonzalez wrote:
>
>> On 28/05/2024 14:31, Konrad Dybcio wrote:
>>
>>> [...]
>>>
>>>> + hdmi_cec_default: hdmi-cec-default-state {
>>>> + pins = "gpio31";
>>>> + function = "hdmi_cec";
>>>> + drive-strength = <2>;
>>>> + bias-pull-up;
>>>> + };
>>>> +
>>>> + hdmi_cec_sleep: hdmi-cec-sleep-state {
>>>> + pins = "gpio31";
>>>> + function = "hdmi_cec";
>>>> + drive-strength = <2>;
>>>> + bias-pull-up;
>>>> + };
>>>
>>> It's super strange that both states are identical. Usually, the pull-up
>>> is disabled and the GPIO is unmuxed (i.e. function = "gpio"). If that's
>>> not the case for whatever reason, you can drop the sleep variants and
>>> simply reference the leftover one from both pinctrl-0 and pinctrl-1.
>>
>> Patch is a direct translation of the vendor code:
>> https://git.codelinaro.org/clo/la/kernel/msm-4.4/-/blob/caf_migration/kernel.lnx.4.4.r38-rel/arch/arm/boot/dts/qcom/msm8998-pinctrl.dtsi#L2324-2400
>>
>> I suppose it wouldn't be the first time that vendor code
>> is doing something odd.
>>
>> Though, I'm a bit confused why you would single out hdmi-cec
>> when hdmi_ddc is the same?
>
> As in, me in the above message or the vendor devicetree?
>
> If the former, it's just an example
>
> If the latter, the muxing function differs so they must have their
> own separate nodes
I meant:
You (rightly) point out that
hdmi_cec_default & hdmi_cec_sleep nodes are identical
in my patch.
I stated that, in fact,
hdmi_ddc_default & hdmi_ddc_sleep nodes are ALSO identical
in my patch.
And the reason they are identical in my patch is because
they are identical in the vendor code downstream:
mdss_hdmi_cec_active & mdss_hdmi_cec_suspend
mdss_hdmi_ddc_active & mdss_hdmi_ddc_suspend
If I understand correctly, you are saying I should delete
hdmi_cec_sleep and hdmi_ddc_sleep nodes, and refer
to the default nodes in the hdmi pinctrl-1 prop?
Regards
