[PATCH] imx-drm: imx-hdmi: move memory and resource allocation into probe function

2014-05-15 Thread Thierry Reding
On Wed, May 14, 2014 at 11:24:14PM +0200, Philipp Zabel wrote:
> Move memory allocation and resource acquisition from the bind function into
> the probe function. This calls the devres managed functions once instead of
> possibly multiple times in the bind function and avoids leaking memory (as
> long as the hdmi platform device stays bound).
> 
> While at it, request the irq only after the interrupt handler mutes are
> set up, to avoid spurious interrupts.
> 
> Signed-off-by: Philipp Zabel 
> ---
>  drivers/staging/imx-drm/imx-hdmi.c | 163 
> +++--
>  1 file changed, 83 insertions(+), 80 deletions(-)

Very nice. I like it. One comment below, though it's unrelated to what
you're trying to achieve here. It's primarily a note to myself.

> diff --git a/drivers/staging/imx-drm/imx-hdmi.c 
> b/drivers/staging/imx-drm/imx-hdmi.c
[...]
>  static int imx_hdmi_platform_probe(struct platform_device *pdev)
>  {
[...]
> + ddc_node = of_parse_phandle(np, "ddc-i2c-bus", 0);
> + if (ddc_node) {
> + hdmi->ddc = of_find_i2c_adapter_by_node(ddc_node);
> + if (!hdmi->ddc)
> + dev_dbg(hdmi->dev, "failed to read ddc node\n");
> +
> + of_node_put(ddc_node);
> + } else {
> + dev_dbg(hdmi->dev, "no ddc property found\n");
> + }

This seems to be emerging as a common pattern. Perhaps we should add a
common helper for this.

Thierry
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: 



[PATCH] imx-drm: imx-hdmi: move memory and resource allocation into probe function

2014-05-15 Thread Philipp Zabel
Move memory allocation and resource acquisition from the bind function into
the probe function. This calls the devres managed functions once instead of
possibly multiple times in the bind function and avoids leaking memory (as
long as the hdmi platform device stays bound).

While at it, request the irq only after the interrupt handler mutes are
set up, to avoid spurious interrupts.

Signed-off-by: Philipp Zabel 
---
 drivers/staging/imx-drm/imx-hdmi.c | 163 +++--
 1 file changed, 83 insertions(+), 80 deletions(-)

diff --git a/drivers/staging/imx-drm/imx-hdmi.c 
b/drivers/staging/imx-drm/imx-hdmi.c
index d47dedd..cb3950b 100644
--- a/drivers/staging/imx-drm/imx-hdmi.c
+++ b/drivers/staging/imx-drm/imx-hdmi.c
@@ -1591,67 +1591,9 @@ MODULE_DEVICE_TABLE(of, imx_hdmi_dt_ids);

 static int imx_hdmi_bind(struct device *dev, struct device *master, void *data)
 {
-   struct platform_device *pdev = to_platform_device(dev);
-   const struct of_device_id *of_id =
-   of_match_device(imx_hdmi_dt_ids, dev);
struct drm_device *drm = data;
-   struct device_node *np = dev->of_node;
-   struct device_node *ddc_node;
-   struct imx_hdmi *hdmi;
-   struct resource *iores;
-   int ret, irq;
-
-   hdmi = devm_kzalloc(dev, sizeof(*hdmi), GFP_KERNEL);
-   if (!hdmi)
-   return -ENOMEM;
-
-   hdmi->dev = dev;
-   hdmi->connector_status = connector_status_disconnected;
-   hdmi->sample_rate = 48000;
-   hdmi->ratio = 100;
-
-   if (of_id) {
-   const struct platform_device_id *device_id = of_id->data;
-   hdmi->dev_type = device_id->driver_data;
-   }
-
-   ddc_node = of_parse_phandle(np, "ddc-i2c-bus", 0);
-   if (ddc_node) {
-   hdmi->ddc = of_find_i2c_adapter_by_node(ddc_node);
-   if (!hdmi->ddc)
-   dev_dbg(hdmi->dev, "failed to read ddc node\n");
-
-   of_node_put(ddc_node);
-   } else {
-   dev_dbg(hdmi->dev, "no ddc property found\n");
-   }
-
-   irq = platform_get_irq(pdev, 0);
-   if (irq < 0)
-   return -EINVAL;
-
-   ret = devm_request_threaded_irq(dev, irq, imx_hdmi_hardirq,
-   imx_hdmi_irq, IRQF_SHARED,
-   dev_name(dev), hdmi);
-   if (ret)
-   return ret;
-
-   iores = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-   hdmi->regs = devm_ioremap_resource(dev, iores);
-   if (IS_ERR(hdmi->regs))
-   return PTR_ERR(hdmi->regs);
-
-   hdmi->regmap = syscon_regmap_lookup_by_phandle(np, "gpr");
-   if (IS_ERR(hdmi->regmap))
-   return PTR_ERR(hdmi->regmap);
-
-   hdmi->isfr_clk = devm_clk_get(hdmi->dev, "isfr");
-   if (IS_ERR(hdmi->isfr_clk)) {
-   ret = PTR_ERR(hdmi->isfr_clk);
-   dev_err(hdmi->dev,
-   "Unable to get HDMI isfr clk: %d\n", ret);
-   return ret;
-   }
+   struct imx_hdmi *hdmi = dev_get_drvdata(dev);
+   int ret;

ret = clk_prepare_enable(hdmi->isfr_clk);
if (ret) {
@@ -1660,14 +1602,6 @@ static int imx_hdmi_bind(struct device *dev, struct 
device *master, void *data)
return ret;
}

-   hdmi->iahb_clk = devm_clk_get(hdmi->dev, "iahb");
-   if (IS_ERR(hdmi->iahb_clk)) {
-   ret = PTR_ERR(hdmi->iahb_clk);
-   dev_err(hdmi->dev,
-   "Unable to get HDMI iahb clk: %d\n", ret);
-   goto err_isfr;
-   }
-
ret = clk_prepare_enable(hdmi->iahb_clk);
if (ret) {
dev_err(hdmi->dev,
@@ -1675,16 +1609,6 @@ static int imx_hdmi_bind(struct device *dev, struct 
device *master, void *data)
goto err_isfr;
}

-   /* Product and revision IDs */
-   dev_info(dev,
-   "Detected HDMI controller 0x%x:0x%x:0x%x:0x%x\n",
-   hdmi_readb(hdmi, HDMI_DESIGN_ID),
-   hdmi_readb(hdmi, HDMI_REVISION_ID),
-   hdmi_readb(hdmi, HDMI_PRODUCT_ID0),
-   hdmi_readb(hdmi, HDMI_PRODUCT_ID1));
-
-   initialize_hdmi_ih_mutes(hdmi);
-
/*
 * To prevent overflows in HDMI_IH_FC_STAT2, set the clk regenerator
 * N and cts values before enabling phy
@@ -1711,8 +1635,6 @@ static int imx_hdmi_bind(struct device *dev, struct 
device *master, void *data)
/* Unmute interrupts */
hdmi_writeb(hdmi, ~HDMI_IH_PHY_STAT0_HPD, HDMI_IH_MUTE_PHY_STAT0);

-   dev_set_drvdata(dev, hdmi);
-
return 0;

 err_iahb:
@@ -1746,6 +1668,87 @@ static const struct component_ops hdmi_ops = {

 static int imx_hdmi_platform_probe(struct platform_device *pdev)
 {
+   struct device *dev = >dev;
+   const struct of_device_id *of_id =
+   of_match_device(imx_hdmi_dt_ids, dev);
+   struct 

[PATCH] imx-drm: imx-hdmi: move memory and resource allocation into probe function

2014-05-14 Thread Fabio Estevam
Philipp,

On Wed, May 14, 2014 at 6:24 PM, Philipp Zabel  
wrote:
> Move memory allocation and resource acquisition from the bind function into
> the probe function. This calls the devres managed functions once instead of
> possibly multiple times in the bind function and avoids leaking memory (as
> long as the hdmi platform device stays bound).
>
> While at it, request the irq only after the interrupt handler mutes are
> set up, to avoid spurious interrupts.
>
> Signed-off-by: Philipp Zabel 

This one does not apply against the latest version from linux-next.