From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752072AbdARKty (ORCPT ); Wed, 18 Jan 2017 05:49:54 -0500 Received: from us01smtprelay-2.synopsys.com ([198.182.60.111]:43197 "EHLO smtprelay.synopsys.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751947AbdARKtv (ORCPT ); Wed, 18 Jan 2017 05:49:51 -0500 Subject: Re: [RFC/RFT PATCH 2/4] drm/bridge: dw-hdmi: Add support for custom PHY handling To: Neil Armstrong , , References: <1484656294-6140-1-git-send-email-narmstrong@baylibre.com> <1484656294-6140-3-git-send-email-narmstrong@baylibre.com> CC: , , , From: Jose Abreu Message-ID: <88d1f45a-bfd2-2e6b-6c23-2d0686faa2ce@synopsys.com> Date: Wed, 18 Jan 2017 10:40:33 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <1484656294-6140-3-git-send-email-narmstrong@baylibre.com> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.107.19.90] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Neil, On 17-01-2017 12:31, Neil Armstrong wrote: > @@ -1434,9 +1434,18 @@ static int dw_hdmi_setup(struct dw_hdmi *hdmi, struct drm_display_mode *mode) > hdmi_av_composer(hdmi, mode); > > /* HDMI Initializateion Step B.2 */ > - ret = dw_hdmi_phy_init(hdmi); > - if (ret) > - return ret; > + if (hdmi->plat_data->hdmi_phy_init) { > + ret = hdmi->plat_data->hdmi_phy_init(hdmi, hdmi->plat_data, > @@ -1551,7 +1560,11 @@ static void dw_hdmi_poweron(struct dw_hdmi *hdmi) > > static void dw_hdmi_poweroff(struct dw_hdmi *hdmi) > { > - dw_hdmi_phy_disable(hdmi); > + if (hdmi->phy_enabled && hdmi->plat_data->hdmi_phy_disable) { > + hdmi->plat_data->hdmi_phy_disable(hdmi, hdmi->plat_data); > + hdmi->phy_enabled = false; > + } else > + dw_hdmi_phy_disable(hdmi); > hdmi->bridge_is_on = false; > } > > @@ -1921,7 +1962,9 @@ static int dw_hdmi_detect_phy(struct dw_hdmi *hdmi) > return -ENODEV; > } > > - if (!hdmi->phy->configure && !hdmi->plat_data->configure_phy) { > + if (!hdmi->phy->configure && > + !hdmi->plat_data->configure_phy && > + !hdmi->plat_data->hdmi_phy_init) { > dev_err(hdmi->dev, "%s requires platform support\n", > hdmi->phy->name); > return -ENODEV; > @@ -2119,9 +2164,10 @@ static int dw_hdmi_detect_phy(struct dw_hdmi *hdmi) > + if (!hdmi->plat_data || !hdmi->plat_data->hdmi_read_hpd) > + /* Unmute interrupts */ > + hdmi_writeb(hdmi, ~(HDMI_IH_PHY_STAT0_HPD | HDMI_IH_PHY_STAT0_RX_SENSE), > + HDMI_IH_MUTE_PHY_STAT0); > > [sniped some parts of the patch] I personally don't like this kind of 'if plat_data ... else ...' Can't we just abstract all of the phy configuration (for a Synopsys Phy) to a new file and then pass it always in pdata as default? Then overwrite it with custom one if needed. Much like what you did with the regmap. Or even leave it in dw-hdmi.c but have a generic pdata structure with generic phy init/disable functions. Best regards, Jose Miguel Abreu