From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thierry Reding Subject: Re: [PATCH v4 3/6] drm/dsi: Try to match non-DT dsi devices Date: Thu, 21 Jan 2016 17:05:15 +0100 Message-ID: <20160121160514.GC647@ulmo.nvidia.com> References: <1448884892-7731-1-git-send-email-architt@codeaurora.org> <1449751300-2841-1-git-send-email-architt@codeaurora.org> <1449751300-2841-4-git-send-email-architt@codeaurora.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1061182927==" Return-path: Received: from hqemgate14.nvidia.com (hqemgate14.nvidia.com [216.228.121.143]) by gabe.freedesktop.org (Postfix) with ESMTPS id 95F4F6E25B for ; Thu, 21 Jan 2016 08:05:26 -0800 (PST) In-Reply-To: <1449751300-2841-4-git-send-email-architt@codeaurora.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Archit Taneja Cc: linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org, a.hajda@samsung.com, dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org --===============1061182927== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="c3bfwLpm8qysLVxt" Content-Disposition: inline --c3bfwLpm8qysLVxt Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Dec 10, 2015 at 06:11:37PM +0530, Archit Taneja wrote: > Add a device name field in mipi_dsi_device. This name is different from > the actual dev name (which is of the format "hostname.reg"). When the > device is created via DT, this name is set to the modalias string. Why? What's the use of setting this to the modalias string? > In the non-DT case, the driver creating the DSI device provides the > name by populating a filed in mipi_dsi_device_info. >=20 > Matching for DT case would be as it was before. For the non-DT case, > we compare the device and driver names. Other buses (like i2c/spi) "I2C" and "SPI", please. > perform a non-DT match by comparing the device name and entries in the > driver's id_table. Such a mechanism isn't used for the dsi bus. "DSI", please. >=20 > Reviewed-by: Andrzej Hajda > Signed-off-by: Archit Taneja > --- > drivers/gpu/drm/drm_mipi_dsi.c | 25 ++++++++++++++++++++++++- > include/drm/drm_mipi_dsi.h | 6 ++++++ > 2 files changed, 30 insertions(+), 1 deletion(-) >=20 > diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_ds= i.c > index 9434585..5a46802 100644 > --- a/drivers/gpu/drm/drm_mipi_dsi.c > +++ b/drivers/gpu/drm/drm_mipi_dsi.c > @@ -45,9 +45,26 @@ > * subset of the MIPI DCS command set. > */ > =20 > +static const struct device_type mipi_dsi_device_type; > + > static int mipi_dsi_device_match(struct device *dev, struct device_drive= r *drv) > { > - return of_driver_match_device(dev, drv); > + struct mipi_dsi_device *dsi; > + > + if (dev->type =3D=3D &mipi_dsi_device_type) > + dsi =3D to_mipi_dsi_device(dev); > + else > + return 0; I think this check is redundant. I'm not aware of any case where the bus ->match() callback is called on a device that isn't on said bus. > + /* attempt OF style match */ > + if (of_driver_match_device(dev, drv)) > + return 1; > + > + /* compare dsi device and driver names */ "DSI", please. > + if (!strcmp(dsi->name, drv->name)) > + return 1; > + > + return 0; > } > =20 > static const struct dev_pm_ops mipi_dsi_device_pm_ops =3D { > @@ -125,6 +142,7 @@ struct mipi_dsi_device *mipi_dsi_device_new(struct mi= pi_dsi_host *host, > dsi->dev.type =3D &mipi_dsi_device_type; > dsi->dev.of_node =3D info->node; > dsi->channel =3D info->reg; > + strlcpy(dsi->name, info->type, sizeof(dsi->name)); Don't you need to check info->type !=3D NULL before doing this? > =20 > dev_set_name(&dsi->dev, "%s.%d", dev_name(host->dev), info->reg); > =20 > @@ -148,6 +166,11 @@ of_mipi_dsi_device_add(struct mipi_dsi_host *host, s= truct device_node *node) > int ret; > u32 reg; > =20 > + if (of_modalias_node(node, info.type, sizeof(info.type)) < 0) { > + dev_err(dev, "modalias failure on %s\n", node->full_name); > + return ERR_PTR(-EINVAL); > + } > + > ret =3D of_property_read_u32(node, "reg", ®); > if (ret) { > dev_err(dev, "device node %s has no valid reg property: %d\n", > diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h > index 90f4f3c..cb084af 100644 > --- a/include/drm/drm_mipi_dsi.h > +++ b/include/drm/drm_mipi_dsi.h > @@ -139,8 +139,11 @@ enum mipi_dsi_pixel_format { > MIPI_DSI_FMT_RGB565, > }; > =20 > +#define DSI_DEV_NAME_SIZE 20 > + > /** > * struct mipi_dsi_device_info - template for creating a mipi_dsi_device > + * @type: dsi peripheral chip type > * @reg: DSI virtual channel assigned to peripheral > * @node: pointer to OF device node > * > @@ -148,6 +151,7 @@ enum mipi_dsi_pixel_format { > * DSI device > */ > struct mipi_dsi_device_info { > + char type[DSI_DEV_NAME_SIZE]; Why limit ourselves to 20 characters? And why even so complicated? Isn't the type always static when someone specifies this? Couldn't we simply use a const char *name here instead? Thierry --c3bfwLpm8qysLVxt Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAABCAAGBQJWoQG3AAoJEN0jrNd/PrOhml8P/Ay6gbYUv8NJvS2e309XOm65 H9IBMCmR2nCVLywoCTsfnAqcVLFkA01NadnJCbLzhjQZ1LYmvLWkC0UVFru0uQ/3 bwLZAdUdsbt2CuNEzZztoac4DJj3XC4u8mTkoDBPiKUebDmLkt74EBRabEaCYfT6 M3dD8sD6NuQZSQBkvbuKLlIlC78eG1Tv0WtUbqhOKah++sSPgzLInYP/MBjEOCxM wQuquKL5yX+Rq+UgJ26DO5gly4kQjR95aPW4w9UYn5aa/xQpoq/4W3T4UI3BxIWo dwUiondm6dYGMWLJ/oUzYCYOdNijsOyQucxu6njUSFpCrPt+OUnNNo0hqDbXgHEI H+gSZddXRqrWOBOPOEfFJW5or52I8CcoWwHBpYrPIoUJrg70Tnl/sXoyPPEE8am5 8rxJUorQYIsQUGVR7l1If21W3lGMPEUZI/+6nmDHbguUI5dL6BvaDIT6biaUas/G WXjAsrTLJ30Qk5p4zpo3o41FC4X4HYfaiS61CvhBuFR7XYrgUE1Ts85MRPrF0Wdj 0KZKPT8Um+cEBwTN2bnOdU+C+TP6nuyqxjoQ8W5LIb7yaCrKMI7rHp03K2n4bQuA tQ28hWwRU9X0VA79B7+KuXWckfwbQxuHC/NTsTzIKzEF7o4iBdoWm2t+c4nTq9G3 vbvepXsfWr5FzRMQiHUR =ih/q -----END PGP SIGNATURE----- --c3bfwLpm8qysLVxt-- --===============1061182927== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHA6Ly9saXN0 cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK --===============1061182927==--