From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1031126AbbJ3MwK (ORCPT ); Fri, 30 Oct 2015 08:52:10 -0400 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 X-AuditID: cbfec7f5-f794b6d000001495-de-563367f66655 Subject: Re: [RFC v2 3/5] drm/dsi: Check for used channels 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-4-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: <563367F1.2060202@samsung.com> Date: Fri, 30 Oct 2015 13:52:01 +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-4-git-send-email-architt@codeaurora.org> Content-type: text/plain; charset=windows-1252 Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFvrJLMWRmVeSWpSXmKPExsVy+t/xK7rf0o3DDHatZ7HoPXeSyaKp4y2r xf9tE5ktrnx9z2ax/PJ6RosHc28yWUzcf5bd4vKuOWwW//fsYLe4/ZvPgcvjcl8vk8febwtY PDat6mTz2P7tAavHvJOBHve7jzN59Da/Y/Po/2vg8XmTXABnFJdNSmpOZllqkb5dAldG55Nn bAUzRSoOP/nC3MDYLdDFyMkhIWAi8ef5H3YIW0ziwr31bF2MXBxCAksZJZbNvssM4TxnlFh7 p5sJpEpYwEri3PnzYLaIgLfEnNV9jCC2kMABRokdN/JBGphB7P7pq8ASbAKaEn8332QDsXkF tCR2t88DmsrBwSKgKrF3ZSBIWFQgQmLihAZWiBJBiR+T77GAlHAKuEvc3CAJYjIL6Encv6gF UsEsIC+xec1b5gmMArOQNMxCqJqFpGoBI/MqRtHU0uSC4qT0XCO94sTc4tK8dL3k/NxNjJAI +bqDcekxq0OMAhyMSjy8PxKMwoRYE8uKK3MPMUpwMCuJ8PY4GYcJ8aYkVlalFuXHF5XmpBYf YpTmYFES5525632IkEB6YklqdmpqQWoRTJaJg1OqgdF5gqSrmNV3qdObtkbdjjYNdeTtKdl4 6dStD9/TeNbfe9QjJBxYkPMsISEtoaSwTKZ06s3K+tcHatkTOPvXFTlv4pcOVszfvnpb73Hv E0oNrQVfz/Lmf0zfmzzz4G7hX96lXgxm1xdVfI0XeVUdk6NyItlvrltQSuin76EzZn05vkDg 5oxzNUosxRmJhlrMRcWJAF8EwJ6MAgAA 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: > 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); >