From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrzej Hajda Subject: Re: [RFC v2 3/5] drm/dsi: Check for used channels Date: Fri, 30 Oct 2015 13:52:01 +0100 Message-ID: <563367F1.2060202@samsung.com> References: <1435641851-27295-1-git-send-email-architt@codeaurora.org> <1444123482-25579-1-git-send-email-architt@codeaurora.org> <1444123482-25579-4-git-send-email-architt@codeaurora.org> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Return-path: Received: from mailout4.w1.samsung.com ([210.118.77.14]:13165 "EHLO mailout4.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S965731AbbJ3MwI (ORCPT ); Fri, 30 Oct 2015 08:52:08 -0400 In-reply-to: <1444123482-25579-4-git-send-email-architt@codeaurora.org> Sender: linux-arm-msm-owner@vger.kernel.org List-Id: linux-arm-msm@vger.kernel.org To: Archit Taneja , dri-devel@lists.freedesktop.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 On 10/06/2015 11:24 AM, Archit Taneja wrote: > We don't check whether a previously registered mipi_dsi_device under the > same host shares the same virtual channel. > > Before registering, check if any of the registered devices doesn't > already have the same virtual channel. > > This wasn't crucial when all the devices under a host were populated via > DT. Now that we also support creating devices manually, we could end up > in a situation where a driver tries to create a device with a virtual > channel already taken by a device populated in DT. > > Signed-off-by: Archit Taneja Beside few non-blcking nitpicks. Reviewed-by: Andrzej Hajda > --- > drivers/gpu/drm/drm_mipi_dsi.c | 26 ++++++++++++++++++++++++-- > 1 file changed, 24 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c > index 46ee515..db6130a 100644 > --- a/drivers/gpu/drm/drm_mipi_dsi.c > +++ b/drivers/gpu/drm/drm_mipi_dsi.c > @@ -123,6 +123,22 @@ static const struct device_type mipi_dsi_device_type = { > .release = mipi_dsi_dev_release, > }; > > +static int __dsi_check_chan_busy(struct device *dev, void *data) > +{ > + struct mipi_dsi_device *dsi = to_mipi_dsi_device(dev); > + u32 reg = *(u32 *) data; > + > + if (dsi && dsi->channel == reg) Maybe simpler would be: u32 *reg = data; if (dsi && dsi->channel == *reg) > + return -EBUSY; > + > + return 0; > +} > + > +static int mipi_dsi_check_chan_busy(struct mipi_dsi_host *host, u32 reg) > +{ > + return device_for_each_child(host->dev, ®, __dsi_check_chan_busy); > +} > + > struct mipi_dsi_device *mipi_dsi_device_new(struct mipi_dsi_host *host, > struct mipi_dsi_device_info *info) > { > @@ -150,14 +166,20 @@ struct mipi_dsi_device *mipi_dsi_device_new(struct mipi_dsi_host *host, > > dev_set_name(&dsi->dev, "%s.%d", dev_name(host->dev), info->reg); > > + r = mipi_dsi_check_chan_busy(host, info->reg); I know this r variable was introduced in the 1st patch, but if you will send next iteration consider replacing it with ret, looks more readable and follows file convention :) Regards Andrzej > + if (r) > + goto err; > + > r = device_register(&dsi->dev); > if (r) { > dev_err(dev, "failed to register device: %d\n", r); > - kfree(dsi); > - return ERR_PTR(r); > + goto err; > } > > return dsi; > +err: > + kfree(dsi); > + return ERR_PTR(r); > } > EXPORT_SYMBOL(mipi_dsi_device_new); >