Re: [PATCH v4 14/34] drm/exynos: hdmi: remove the i2c drivers and use devtree

[Tomasz Figa <t.figa@samsung.com>]

On 10.02.2014 08:30, Inki Dae wrote:
> 2014-02-08 11:52 GMT+09:00 Tomasz Figa <tomasz.figa@gmail.com>:
>> Hi,
>>
>>
>> On 30.01.2014 22:19, Sean Paul wrote:
>>>
>>> From: Daniel Kurtz <djkurtz@chromium.org>
>>>
>>> The i2c client was previously being passed into the hdmi driver via a
>>> dedicated i2c driver, and then a global variable. This patch removes all
>>> of that and just uses the device tree to get the i2c_client. This patch
>>> also properly references the client so we don't lose it before we're
>>> done with it.
>>>
>>> Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
>>> [seanpaul changed to phandle lookup instead of using of node name]
>>> Signed-off-by: Sean Paul <seanpaul@chromium.org>
>>> ---
>>>
>>> Changes in v2:
>>>    - Change include to linux/i2c.h instead of linux/of_i2c.h
>>> Changes in v3: None
>>> Changes in v4:
>>>    - Changed to find phy via phandle instead of by name
>>>
>>>    .../devicetree/bindings/video/exynos_hdmi.txt      |  5 ++
>>>    drivers/gpu/drm/exynos/Makefile                    |  1 -
>>>    drivers/gpu/drm/exynos/exynos_ddc.c                | 63
>>> ---------------------
>>>    drivers/gpu/drm/exynos/exynos_hdmi.c               | 59
>>> +++++++++-----------
>>>    drivers/gpu/drm/exynos/exynos_hdmi.h               | 23 --------
>>>    drivers/gpu/drm/exynos/exynos_hdmiphy.c            | 65
>>> ----------------------
>>>    6 files changed, 32 insertions(+), 184 deletions(-)
>>>    delete mode 100644 drivers/gpu/drm/exynos/exynos_ddc.c
>>>    delete mode 100644 drivers/gpu/drm/exynos/exynos_hdmi.h
>>>    delete mode 100644 drivers/gpu/drm/exynos/exynos_hdmiphy.c
>>>
>>> diff --git a/Documentation/devicetree/bindings/video/exynos_hdmi.txt
>>> b/Documentation/devicetree/bindings/video/exynos_hdmi.txt
>>> index 50decf8..f9187a2 100644
>>> --- a/Documentation/devicetree/bindings/video/exynos_hdmi.txt
>>> +++ b/Documentation/devicetree/bindings/video/exynos_hdmi.txt
>>> @@ -25,6 +25,9 @@ Required properties:
>>>                  sclk_pixel.
>>>    - clock-names: aliases as per driver requirements for above clock IDs:
>>>          "hdmi", "sclk_hdmi", "sclk_pixel", "sclk_hdmiphy" and "mout_hdmi".
>>> +- ddc: phandle to the hdmi ddc node
>>> +- phy: phandle to the hdmi phy node
>>> +
>>
>>
>> Adding new required properties to an already defined binding is breaking
>> backwards compatibility, which is supposed to be preserved, since DT is an
>> ABI.
>
> Right, that's a problem. If we use this patch, and  we add additional
> codes for these property to hdmi driver, then the existing dtb will be
> broken by dt binding fail. However, as long as I know, there is
> another case that phy property is used like this way; MIPI-CSI device
> node. And also it seems reasonable to add ddc property in similar
> reason if we could handle that the existing dt node is deplicated.
>
> So what we could select I think are,
> 1. to deplicate the existing dt node, and using this patch.
> 2. or, to separate phy and ddc parts as each module so that each
> module can bind device tree. However, this way would make hdmi part of
> exynos drm to be complicated because we should resolve probe ordering
> issue.
>
> It seems that we need to discuss and make a consensus about this issue.
>
> Tomasz and Sean, please, share your opinion. I tend to proper 1.

I tend to prefer option 1 too. I don't have anything against this patch 
itself.

All I'm just worried about is applying different standards to patches 
changing DT bindings depending on subsystem (and authors?).

Either we declare that we are okay with making Exynos-specific bindings 
unstable or we keep stability for all of them. I wouldn't be upset if we 
went with the former, as the whole stability making it impossible to get 
rid of some fugly broken bindings is getting more and more annoying.

Best regards,
Tomasz