From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754434AbbJ3MmS (ORCPT ); Fri, 30 Oct 2015 08:42:18 -0400 Received: from mailout3.w1.samsung.com ([210.118.77.13]:22577 "EHLO mailout3.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754019AbbJ3MmP (ORCPT ); Fri, 30 Oct 2015 08:42:15 -0400 X-AuditID: cbfec7f5-f794b6d000001495-56-563365a5a7a5 Subject: Re: [RFC v2 2/5] drm/dsi: Try to match non-DT dsi devices To: Archit Taneja , dri-devel@lists.freedesktop.org References: <1435641851-27295-1-git-send-email-architt@codeaurora.org> <1444123482-25579-1-git-send-email-architt@codeaurora.org> <1444123482-25579-3-git-send-email-architt@codeaurora.org> Cc: linux-kernel@vger.kernel.org, airlied@linux.ie, daniel@ffwll.ch, treding@nvidia.com, l.stach@pengutronix.de, robh@kernel.org, linux-arm-msm@vger.kernel.org, jani.nikula@linux.intel.com From: Andrzej Hajda Message-id: <563365A0.5000507@samsung.com> Date: Fri, 30 Oct 2015 13:42:08 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-version: 1.0 In-reply-to: <1444123482-25579-3-git-send-email-architt@codeaurora.org> Content-type: text/plain; charset=windows-1252 Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFvrNLMWRmVeSWpSXmKPExsVy+t/xa7pLU43DDCbtZrfoPXeSyaKp4y2r xf9tE5ktrnx9z2ax/PJ6RosHc28yWUzcf5bd4vKuOWwW//fsYLe4/ZvPgcvjcl8vk8febwtY PDat6mTz2P7tAavHvJOBHve7jzN59Da/Y/Po/2vg8XmTXABnFJdNSmpOZllqkb5dAlfGpdbf rAWvlCsaT89namDcI9PFyMkhIWAicfzzXGYIW0ziwr31bF2MXBxCAksZJe68u8UC4TxnlGg9 doIRpEpYwEni+N6lYB0iAt4Sc1b3gcWFBA4wSjSdrQJpYAax+6evAkuwCWhK/N18kw3E5hXQ kjj55xtYM4uAqsTttSfYQWxRgQiJiRMaWCFqBCV+TL7HAmJzCrhLHGw/D1TDATRUT+L+RS2Q MLOAvMTmNW+ZJzAKzELSMQuhahaSqgWMzKsYRVNLkwuKk9JzjfSKE3OLS/PS9ZLzczcxQqLk 6w7GpcesDjEKcDAq8fD+SDAKE2JNLCuuzD3EKMHBrCTC2+NkHCbEm5JYWZValB9fVJqTWnyI UZqDRUmcd+au9yFCAumJJanZqakFqUUwWSYOTqkGxgXz7od/FXRQerSgNGRTdF7Rno+7XU/s O1RQXmXLeLfC2fZy+84jXvt2H/5ztsR9UvWzqKTZty5cOcE8QeVKxzp2kd9Cs3efsfp116B4 xuTIndIS5+9pmUaVTpq1s2ttRXDjqbrb1dYLD91Qm34rYOOfb30/uue+yUz8o3M30awp4VNS nB1XmqwSS3FGoqEWc1FxIgC5qrTWjgIAAA== Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 10/06/2015 11:24 AM, 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. > In the non-DT case, the driver creating the DSI device provides the > name by populating a filed in mipi_dsi_device_info. > > Matching for DT case would be as it was before. For the non-DT case, > we compare the device and driver names. Matching by comparing driver and > device names isn't the best thing to do. I'd appreciate some suggestions > here. > > Other buses (like i2c/spi) perform a non-DT match by comparing the > device name and entries in the driver's id_table. The id_table structs > for different buses are defined in "include/linux/mod_devicetable.h", I > didn't want to touch that for now. > > Signed-off-by: Archit Taneja Beside small comments below. Reviewed-by: Andrzej Hajda > --- > drivers/gpu/drm/drm_mipi_dsi.c | 29 ++++++++++++++++++++++++++++- > include/drm/drm_mipi_dsi.h | 8 ++++++++ > 2 files changed, 36 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c > index 245ecfe..46ee515 100644 > --- a/drivers/gpu/drm/drm_mipi_dsi.c > +++ b/drivers/gpu/drm/drm_mipi_dsi.c > @@ -45,9 +45,30 @@ > * subset of the MIPI DCS command set. > */ > > +static const struct device_type mipi_dsi_device_type; > + > static int mipi_dsi_device_match(struct device *dev, struct device_driver *drv) > { > - return of_driver_match_device(dev, drv); > + struct mipi_dsi_device *dsi; > + > + if (dev->type == &mipi_dsi_device_type) > + dsi = to_mipi_dsi_device(dev); > + else > + return 0; > + > + /* Attempt OF style match */ > + if (of_driver_match_device(dev, drv)) > + return 1; > + > + /* > + * Try to compare dsi device and driver names. If this matching approach > + * isn't strong, we'd probably want the dsi drivers to populate the > + * id_table field and use that instead > + */ > + if (!strcmp(dsi->name, drv->name)) > + return 1; > + > + return 0; > } > > static const struct dev_pm_ops mipi_dsi_device_pm_ops = { > @@ -125,6 +146,7 @@ struct mipi_dsi_device *mipi_dsi_device_new(struct mipi_dsi_host *host, > dsi->dev.type = &mipi_dsi_device_type; > dsi->dev.of_node = info->node; > dsi->channel = info->reg; > + strlcpy(dsi->name, info->name, sizeof(dsi->name)); > > dev_set_name(&dsi->dev, "%s.%d", dev_name(host->dev), info->reg); > > @@ -147,6 +169,11 @@ of_mipi_dsi_device_add(struct mipi_dsi_host *host, struct device_node *node) > int ret; > u32 reg; > > + if (of_modalias_node(node, info.name, sizeof(info.name)) < 0) { > + dev_err(dev, "modalias failure on %s\n", node->full_name); > + return ERR_PTR(-EINVAL); > + } > + > ret = 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..93dec7b 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, > }; > > +#define DSI_DEV_NAME_SIZE 20 > + > /** > * struct mipi_dsi_device_info - template for creating a mipi_dsi_device > + * @name: name of the dsi peripheral > * @reg: DSI virtual channel assigned to peripheral > * @node: pointer to OF device node > * > @@ -148,14 +151,17 @@ enum mipi_dsi_pixel_format { > * DSI device > */ > struct mipi_dsi_device_info { > + char name[DSI_DEV_NAME_SIZE]; I wonder if it shouldn't be called type as in case of i2c, but no strong feelings. > u32 reg; > struct device_node *node; > }; > > + Empty line. > /** > * struct mipi_dsi_device - DSI peripheral device > * @host: DSI host for this peripheral > * @dev: driver model device node for this peripheral > + * @name: name of the dsi peripheral > * @channel: virtual channel assigned to the peripheral > * @format: pixel format for video mode > * @lanes: number of active data lanes > @@ -165,6 +171,8 @@ struct mipi_dsi_device { > struct mipi_dsi_host *host; > struct device dev; > > + char name[DSI_DEV_NAME_SIZE]; > + This empty line can be also removed. Regards Andrzej > unsigned int channel; > unsigned int lanes; > enum mipi_dsi_pixel_format format;