Received-SPF: pass (google.com: domain of linux-kernel+bounces-187346-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Precedence: bulk
MIME-Version: 1.0
References: <20240521075717.50330-4-angelogioacchino.delregno@collabora.com> <eca4d113-ba59-45aa-9224-22235fb09ddc@bosc.ac.cn>
In-Reply-To: <eca4d113-ba59-45aa-9224-22235fb09ddc@bosc.ac.cn>
From: Chen-Yu Tsai <wenst@chromium.org>
Date: Thu, 23 May 2024 18:41:02 +0800
Message-ID: <CAGXv+5Gzau7qFrsOrKsm7yXNX7AKadgNM5S3SaTNNu57=5-8EQ@mail.gmail.com>
Subject: Re: [v5,3/3] drm/mediatek: Implement OF graphs support for display paths
To: Sui Jingfeng <suijingfeng@bosc.ac.cn>
Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>, chunkuang.hu@kernel.org, 
	robh@kernel.org, krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org, 
	p.zabel@pengutronix.de, airlied@gmail.com, daniel@ffwll.ch, 
	maarten.lankhorst@linux.intel.com, mripard@kernel.org, tzimmermann@suse.de, 
	matthias.bgg@gmail.com, shawn.sung@mediatek.com, yu-chang.lee@mediatek.com, 
	ck.hu@mediatek.com, jitao.shi@mediatek.com, devicetree@vger.kernel.org, 
	linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, 
	linux-mediatek@lists.infradead.org, linux-arm-kernel@lists.infradead.org, 
	kernel@collabora.com, Alexandre Mergnat <amergnat@baylibre.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Wed, May 22, 2024 at 8:32=E2=80=AFPM Sui Jingfeng <suijingfeng@bosc.ac.c=
n> 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] =3D { false };
> > +     int ret;
> > +
> > +     for_each_endpoint_of_node(node, ep_node) {
> > +             ret =3D 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 >=3D 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@hua=
wei.com/

ChenYu

> > +                     ret =3D dev_err_probe(dev, -EINVAL,
> > +                                         "Invalid endpoint%u number\n"=
, of_ep.port);
> > +                     break;
> > +             }
> > +
> > +             output_present[of_ep.id] =3D true;
> > +     }
> > +
> > +     if (ret)
> > +             return ret;
> > +
> > +     if (output_present[CRTC_MAIN]) {
> > +             ret =3D mtk_drm_of_ddp_path_build_one(dev, CRTC_MAIN,
> > +                                                 &data->main_path, &da=
ta->main_len);
> > +             if (ret)
> > +                     return ret;
> > +     }
> > +
> > +     if (output_present[CRTC_EXT]) {
> > +             ret =3D mtk_drm_of_ddp_path_build_one(dev, CRTC_EXT,
> > +                                                 &data->ext_path, &dat=
a->ext_len);
> > +             if (ret)
> > +                     return ret;
> > +     }
> > +
> > +     if (output_present[CRTC_THIRD]) {
> > +             ret =3D mtk_drm_of_ddp_path_build_one(dev, CRTC_THIRD,
> > +                                                 &data->third_path, &d=
ata->third_len);
> > +             if (ret)
> > +                     return ret;
> > +     }
> > +
> > +     return 0;
> > +}
> > +
>
> --
> Best regards
> Sui Jingfeng
>