[PATCH v4 7/9] drm: bridge: dw-hdmi: Add support for custom PHY configuration

[Jose Abreu]

Hi Laurent,


On 02-03-2017 13:41, Laurent Pinchart wrote:
>
>> Hmm, this is kind of confusing. Why do you need a phy_ops and
>> then a separate configure_phy function? Can't we just use phy_ops
>> always? If its external phy then it would need to implement all
>> phy_ops functions.
> The phy_ops are indeed meant to support vendor PHYs. The .configure_phy() 
> operation is meant to support Synopsys PHYs that don't comply with the HDMI TX 
> MHL and 3D PHYs I2C register layout and thus need a different configure 
> function, but can share the other operations with the HDMI TX MHL and 3D PHYs. 
> Ideally I'd like to move that code to the dw-hdmi core, but at the moment I 
> don't have enough information to decide whether the register layout 
> corresponds to the standard Synopsys HDMI TX 2.0 PHY or if it has been 
> modified by the vendor. Once we'll have a second SoC using the same PHY we 
> should have more information to consolidate the code. Of course, if you can 
> share the HDMI TX 2.0 PHY datasheet, I won't mind reworking the code now ;-)
>

Well, if I want to keep my job I can't share the datasheet, and I
do want to keep my job :)

As far as I know this shouldn't change much. I already used (it
was like a year ago) the dw-hdmi driver in a HDMI TX 2.0 PHY. But
I am not following your logic here, sorry: You are mentioning a
custom phy, right? If its custom then it should implement its own
phy_ops. And I don't think that phy logic should go into core,
this should all be extracted because I really believe it will
make the code easier to read. Imagine this organization:

    Folders/Files:
        drm/bridge/dw-hdmi/dw-hdmi-core.c
        drm/bridge/dw-hdmi/dw-hdmi-phy-synopsys.c
        drm/bridge/dw-hdmi/dw-hdmi-phy-*renesas*.c
        drm/bridge/dw-hdmi/dw-hdmi-phy-something.c
    Structures:
        dw_hdmi_pdata
        dw_hdmi_phy_pdata
        dw_hdmi_phy_ops

As the phy is interacted using controller we would need to pass
some callbacks to the phy, but ultimately the phy would be a
platform driver which could have its own compatible string that
would be associated with controller (not sure exactly about this
because I only used this in non-dt).

This is just an idea though. I followed this logic in the RX side
and its very easy to add a new phy now, its a matter of creating
a new file, implement ops and associate it with controller. Of
course some phys will be very similar, for that we can use minor
tweaks via id detection.

What do you think?

Best regards,
Jose Miguel Abreu