From: Thierry Reding Subject: Re: [PATCH] imx-drm: imx-hdmi: move memory and resource allocation into probe function Date: Wed, 14 May 2014 23:55:51 +0200 Message-ID: <20140514215551.GB6215@mithrandir> References: <1400102654-31692-1-git-send-email-philipp.zabel@gmail.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============2097352405==" Return-path: Received: from mail-ee0-f52.google.com (mail-ee0-f52.google.com [74.125.83.52]) by gabe.freedesktop.org (Postfix) with ESMTP id BC1096E112 for ; Wed, 14 May 2014 14:55:54 -0700 (PDT) Received: by mail-ee0-f52.google.com with SMTP id e53so107653eek.39 for ; Wed, 14 May 2014 14:55:53 -0700 (PDT) In-Reply-To: <1400102654-31692-1-git-send-email-philipp.zabel@gmail.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Philipp Zabel Cc: devel@driverdev.osuosl.org, Russell King , dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org --===============2097352405== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="NDin8bjvE/0mNLFQ" Content-Disposition: inline --NDin8bjvE/0mNLFQ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, May 14, 2014 at 11:24:14PM +0200, Philipp Zabel wrote: > Move memory allocation and resource acquisition from the bind function in= to > 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). >=20 > While at it, request the irq only after the interrupt handler mutes are > set up, to avoid spurious interrupts. >=20 > 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 =3D of_parse_phandle(np, "ddc-i2c-bus", 0); > + if (ddc_node) { > + hdmi->ddc =3D 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 --NDin8bjvE/0mNLFQ Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQIcBAEBAgAGBQJTc+ZnAAoJEN0jrNd/PrOhOaIP/1ka9jSDmqIJfG1ThEL3tUNB 3ljmxXgHmtXbHpUQzViknmHXQk6cPC2XpPeoUnDj19khOTFiW2VThieEeIUjxAqR PP2yFkdtB5buqe1m4aGcEgu/WhA1dJGrjJl5P6/xB4VAdX56b3LBRyVDS/gPdUaa B8Yvc65Nmkt5Wk7crKbWHGnfIyjgPMHWU0yQp1urKJbT9e5KjzBAMGRvyB+6HL6Y NhZH5RRW54eyp/uA0rcGC16o9QyvisRiyUzUHcV5t2hp/iIipMnaEk1Mkn48G3/7 882HKt8W5VrKMS18PTAgr90t9vy6GIwX3a8+Duyese7oKlAtvJgIApC1AJIXfn3d pkzxKWx+q5iLREIqwmJzYqdGPOAqjXMUQc2w5D7WG8vitbb1pXn4cW6/kR0REJah ASYhnxmzc5/5i9lNGAUKF1bhktscsZoyKLZbRTvzJ7mN77LHQQLDcfF8HFXYMT05 Kpvw9UMYSC9jM9U/4jbZxM0V9/RT/REqTJ1S+yaVIpOzlwWGk9fHC2IQ/BqaKAKt yLtHWTnGMqpZWqASGzlV4W5rjJPCmBfzd/ZBnz5eu4e+FRqV3I1Os0jMznO3xjbl xHtVB3G6JSowbCb/Jq6BZ+yTc6tR88G14o7QJCCUHlmCPlDU7HFbvshGLrPle5es cyg1xeonPP2+JVTVVmFw =J1Ia -----END PGP SIGNATURE----- --NDin8bjvE/0mNLFQ-- --===============2097352405== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel --===============2097352405==--