[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAGXv+5Gzau7qFrsOrKsm7yXNX7AKadgNM5S3SaTNNu57=5-8EQ@mail.gmail.com>
Date: Thu, 23 May 2024 18:41:02 +0800
From: Chen-Yu Tsai <wenst@...omium.org>
To: Sui Jingfeng <suijingfeng@...c.ac.cn>
Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>, chunkuang.hu@...nel.org,
robh@...nel.org, krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org,
p.zabel@...gutronix.de, airlied@...il.com, daniel@...ll.ch,
maarten.lankhorst@...ux.intel.com, mripard@...nel.org, tzimmermann@...e.de,
matthias.bgg@...il.com, shawn.sung@...iatek.com, yu-chang.lee@...iatek.com,
ck.hu@...iatek.com, jitao.shi@...iatek.com, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org,
linux-mediatek@...ts.infradead.org, linux-arm-kernel@...ts.infradead.org,
kernel@...labora.com, Alexandre Mergnat <amergnat@...libre.com>
Subject: Re: [v5,3/3] drm/mediatek: Implement OF graphs support for display paths
On Wed, May 22, 2024 at 8:32 PM Sui Jingfeng <suijingfeng@...c.ac.cn> wrote:
>
> Hi,
>
>
> On 5/21/24 15:57, AngeloGioacchino Del Regno wrote:
> > +static int mtk_drm_of_ddp_path_build(struct device *dev, struct device_node *node,
> > + struct mtk_mmsys_driver_data *data)
> > +{
> > + struct device_node *ep_node;
> > + struct of_endpoint of_ep;
> > + bool output_present[MAX_CRTC] = { false };
> > + int ret;
> > +
> > + for_each_endpoint_of_node(node, ep_node) {
> > + ret = of_graph_parse_endpoint(ep_node, &of_ep);
> > + of_node_put(ep_node);
>
> There is going to *double* decline the reference counter, as the
> __of_get_next_child() will decrease the reference counter for us
> on the next iteration.
>
>
> > + if (ret) {
> > + dev_err_probe(dev, ret, "Cannot parse endpoint\n");
> > + break;
> > + }
>
> Move the 'of_node_put(ep_node)' into brace '{}' here, if we really cares
> about the reference count.
>
> > +
> > + if (of_ep.id >= MAX_CRTC) {
>
> ditto ?
Maybe we should just add a scoped version of for_each_endpoint_of_node().
See https://lore.kernel.org/all/20240223124432.26443-1-Jonathan.Cameron@huawei.com/
ChenYu
> > + ret = dev_err_probe(dev, -EINVAL,
> > + "Invalid endpoint%u number\n", of_ep.port);
> > + break;
> > + }
> > +
> > + output_present[of_ep.id] = true;
> > + }
> > +
> > + if (ret)
> > + return ret;
> > +
> > + if (output_present[CRTC_MAIN]) {
> > + ret = mtk_drm_of_ddp_path_build_one(dev, CRTC_MAIN,
> > + &data->main_path, &data->main_len);
> > + if (ret)
> > + return ret;
> > + }
> > +
> > + if (output_present[CRTC_EXT]) {
> > + ret = mtk_drm_of_ddp_path_build_one(dev, CRTC_EXT,
> > + &data->ext_path, &data->ext_len);
> > + if (ret)
> > + return ret;
> > + }
> > +
> > + if (output_present[CRTC_THIRD]) {
> > + ret = mtk_drm_of_ddp_path_build_one(dev, CRTC_THIRD,
> > + &data->third_path, &data->third_len);
> > + if (ret)
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
> > +
>
> --
> Best regards
> Sui Jingfeng
>
Powered by blists - more mailing lists