Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932070AbbKBF2n (ORCPT ); Mon, 2 Nov 2015 00:28:43 -0500 Received: from smtp.codeaurora.org ([198.145.29.96]:52859 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751372AbbKBF2k (ORCPT ); Mon, 2 Nov 2015 00:28:40 -0500 Subject: Re: [RFC v2 3/5] drm/dsi: Check for used channels To: Andrzej Hajda , 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-4-git-send-email-architt@codeaurora.org> <563367F1.2060202@samsung.com> 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: Archit Taneja Message-ID: <5636F481.8080000@codeaurora.org> Date: Mon, 2 Nov 2015 10:58:33 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-Version: 1.0 In-Reply-To: <563367F1.2060202@samsung.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3403 Lines: 116 On 10/30/2015 06:22 PM, Andrzej Hajda wrote: > 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) > This is better. I'll change it to this. > > >> + 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 :) I'll probably end up sending another iteration (or more). I'll change the name. Thanks, Archit > > 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); >> > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/