[Freedreno] [DPU PATCH 3/3] drm/msm/dp: add support for DP PLL driver

[Jordan Crouse]

On Thu, Nov 01, 2018 at 05:03:15PM -0400, Sean Paul wrote:
> On Wed, Oct 10, 2018 at 10:15:59AM -0700, Chandan Uddaraju wrote:
> > Add the needed DP PLL specific files to support
> > display port interface on msm targets.
> > 
> > The DP driver calls the DP PLL driver registration.
> > The DP driver sets the link and pixel clock sources.
> > 
> > Signed-off-by: Chandan Uddaraju <chandanu at codeaurora.org>
> > ---
> >  drivers/gpu/drm/msm/Kconfig                   |  16 +
> >  drivers/gpu/drm/msm/Makefile                  |   6 +
> >  drivers/gpu/drm/msm/dp/dp_ctrl.c              |   1 +
> >  drivers/gpu/drm/msm/dp/dp_display.c           |  50 +++
> >  drivers/gpu/drm/msm/dp/dp_display.h           |   3 +
> >  drivers/gpu/drm/msm/dp/dp_parser.h            |   3 +
> >  drivers/gpu/drm/msm/dp/dp_power.c             |  77 +++-
> >  drivers/gpu/drm/msm/dp/dp_power.h             |   2 +
> >  drivers/gpu/drm/msm/dp/pll/dp_pll.c           | 153 ++++++++
> >  drivers/gpu/drm/msm/dp/pll/dp_pll.h           |  64 ++++
> >  drivers/gpu/drm/msm/dp/pll/dp_pll_10nm.c      | 401 +++++++++++++++++++
> >  drivers/gpu/drm/msm/dp/pll/dp_pll_10nm.h      |  94 +++++
> >  drivers/gpu/drm/msm/dp/pll/dp_pll_10nm_util.c | 531 ++++++++++++++++++++++++++
> >  13 files changed, 1389 insertions(+), 12 deletions(-)
> >  create mode 100644 drivers/gpu/drm/msm/dp/pll/dp_pll.c
> >  create mode 100644 drivers/gpu/drm/msm/dp/pll/dp_pll.h
> >  create mode 100644 drivers/gpu/drm/msm/dp/pll/dp_pll_10nm.c
> >  create mode 100644 drivers/gpu/drm/msm/dp/pll/dp_pll_10nm.h
> >  create mode 100644 drivers/gpu/drm/msm/dp/pll/dp_pll_10nm_util.c

<snip>

> > +struct msm_dp_pll *msm_dp_pll_10nm_init(struct platform_device *pdev, int id)
> > +{
> > +	struct dp_pll_10nm *dp_10nm_pll;
> > +	struct msm_dp_pll *pll;
> > +	int ret;
> > +
> > +	if (!pdev)
> > +		return ERR_PTR(-ENODEV);
> > +
> > +	dp_10nm_pll = devm_kzalloc(&pdev->dev, sizeof(*dp_10nm_pll), GFP_KERNEL);
> > +	if (!dp_10nm_pll)
> > +		return ERR_PTR(-ENOMEM);
> > +
> > +	DBG("DP PLL%d", id);
> 
> Please remove (or convert to DRM_DEV_DEBUG)
> 
> > +
> > +	dp_10nm_pll->pdev = pdev;
> > +	dp_10nm_pll->id = id;
> > +	dp_pdb = dp_10nm_pll;
> > +
> > +	dp_10nm_pll->pll_base = msm_ioremap(pdev, "pll_base", "DP_PLL");
> > +	if (IS_ERR_OR_NULL(dp_10nm_pll->pll_base)) {
> > +		dev_err(&pdev->dev, "failed to map CMN PLL base\n");
> 
> Print the error if pll_base is ERR_PTR, same for below.

FWIW the return value from msm_ioremap is always ERR_PTR never NULL. And
msm_ioremap prints error messages on all failures so all you need is:

if (IS_ERR(ptr))
    return ERR_CAST(ptr);

> > +		return ERR_PTR(-ENOMEM);
> 
> You should preserve the error if pll_base is an ERR_PTR, same for below.
> 
> > +	}
> > +
> > +	dp_10nm_pll->phy_base = msm_ioremap(pdev, "phy_base", "DP_PHY");
> > +	if (IS_ERR_OR_NULL(dp_10nm_pll->phy_base)) {
> > +		dev_err(&pdev->dev, "failed to map CMN PHY base\n");
> > +		return ERR_PTR(-ENOMEM);
> > +	}
> > +
> > +	dp_10nm_pll->ln_tx0_base = msm_ioremap(pdev, "ln_tx0_base", "DP_LN_TX0");
> > +	if (IS_ERR_OR_NULL(dp_10nm_pll->ln_tx0_base)) {
> > +		dev_err(&pdev->dev, "failed to map CMN LN_TX0 base\n");
> > +		return ERR_PTR(-ENOMEM);
> > +	}
> > +
> > +	dp_10nm_pll->ln_tx1_base = msm_ioremap(pdev, "ln_tx1_base", "DP_LN_TX1");
> > +	if (IS_ERR_OR_NULL(dp_10nm_pll->ln_tx1_base)) {
> > +		dev_err(&pdev->dev, "failed to map CMN LN_TX1 base\n");
> > +		return ERR_PTR(-ENOMEM);
> > +	}

<snip>

Jordan

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project