linux-kernel - Re: [PATCH V3 3/7] drm: bridge: samsung-dsim: Fetch pll-clock-frequency automatically

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHCN7x+Me-wbUNNyN9fJwg3KETE+0S2MfPOsAb=-CSuSUvZvPg@mail.gmail.com>
Date:   Thu, 4 May 2023 07:57:01 -0500
From:   Adam Ford <aford173@...il.com>
To:     Alexander Stein <alexander.stein@...tq-group.com>
Cc:     dri-devel@...ts.freedesktop.org, marex@...x.de,
        Neil Armstrong <neil.armstrong@...aro.org>,
        Robert Foss <rfoss@...nel.org>,
        Jonas Karlman <jonas@...boo.se>, aford@...conembedded.com,
        Jernej Skrabec <jernej.skrabec@...il.com>,
        linux-kernel@...r.kernel.org,
        Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
        Andrzej Hajda <andrzej.hajda@...el.com>,
        Chen-Yu Tsai <wenst@...omium.org>,
        Marek Szyprowski <m.szyprowski@...sung.com>,
        Jagan Teki <jagan@...rulasolutions.com>
Subject: Re: [PATCH V3 3/7] drm: bridge: samsung-dsim: Fetch
 pll-clock-frequency automatically

On Thu, May 4, 2023 at 7:40 AM Alexander Stein
<alexander.stein@...tq-group.com> wrote:
>
> Hi Adam,
>
> Am Donnerstag, 4. Mai 2023, 14:00:08 CEST schrieb Adam Ford:
> > On Thu, May 4, 2023 at 4:21 AM Alexander Stein
> >
> > <alexander.stein@...tq-group.com> wrote:
> > > Am Dienstag, 2. Mai 2023, 03:07:55 CEST schrieb Adam Ford:
> > > > Make the pll-clock-frequency optional.  If it's present, use it
> > > > to maintain backwards compatibility with existing hardware.  If it
> > > > is absent, read clock rate of "sclk_mipi" to determine the rate.
> > > >
> > > > Signed-off-by: Adam Ford <aford173@...il.com>
> > > > Tested-by: Chen-Yu Tsai <wenst@...omium.org>
> > > > ---
> > > >
> > > >  drivers/gpu/drm/bridge/samsung-dsim.c | 12 ++++++++++--
> > > >  1 file changed, 10 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
> > > > b/drivers/gpu/drm/bridge/samsung-dsim.c index bf4b33d2de76..2dc02a9e37c0
> > > > 100644
> > > > --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> > > > +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> > > > @@ -1726,12 +1726,20 @@ static int samsung_dsim_parse_dt(struct
> > > > samsung_dsim *dsi) {
> > > >
> > > >       struct device *dev = dsi->dev;
> > > >       struct device_node *node = dev->of_node;
> > > >
> > > > +     struct clk *pll_clk;
> > > >
> > > >       int ret;
> > > >
> > > >       ret = samsung_dsim_of_read_u32(node,
> > > >       "samsung,pll-clock-frequency",
> > > >
> > > >                                      &dsi->pll_clk_rate);
> > > >
> > > > -     if (ret < 0)
> > > > -             return ret;
> > > > +
> > > > +     /* If it doesn't exist, read it from the clock instead of failing
> > > > */
> > > > +     if (ret < 0) {
> > > > +             pll_clk = devm_clk_get(dev, "sclk_mipi");
> > > > +             if (!IS_ERR(pll_clk))
> > > > +                     dsi->pll_clk_rate = clk_get_rate(pll_clk);
> > > > +             else
> > > > +                     return PTR_ERR(pll_clk);
> > > > +     }
> > >
> > > Now that 'samsung,pll-clock-frequency' is optional the error in
> > > samsung_dsim_of_read_u32() should be changed. Otherwise you will get
> > >
> > > > /soc@...us@...00000/dsi@...10000: failed to get 'samsung,pll-clock-
> > >
> > > frequency' property
> >
> > I'll change the message from err to info with a message that reads "no
> > samsung,pll-clock-frequency, using pixel clock"
> >
> > Does that work?
>
> Having just a info is totally fine with me. Thanks.
> Although your suggested message somehow implies (to me) using pixel clock is
> just a fallback. I'm a bit concerned some might think "samsung,pll-clock-
> frequency" should be provided in DT. But this might just be me.

Oops, I got the PLL and burst burst clock confused.  I think both
burst-clock and pll clock messages should get updates.

The pll clock should say something like "samsung,pll-clock-frequency
not defined, using sclk_mipi"

The imx8m n/m/p have the sclk_mipi defined in the device tree, and
this patch allows them to not have
to manually set the pll clock since it can be read.  This allows to
people to change the frequency of the PLL in
in one place and let the driver read it instead of having to set the
value in two places for the same clock.

For the burst clock, I'd like to propose
"samsung,burst-clock-frequency not defined, using pixel clock"

Does that work for you?

adam

> frequency

>
> Best regards,
> Alexander
>
> > adam
> >
> > > Best regards,
> > > Alexander
> > >
> > > >       ret = samsung_dsim_of_read_u32(node, "samsung,burst-clock-
> > >
> > > frequency",
> > >
> > > >                                      &dsi->burst_clk_rate);
> > >
> > > --
> > > TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
> > > Amtsgericht München, HRB 105018
> > > Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
> > > http://www.tq-group.com/
>
>
> --
> TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
> Amtsgericht München, HRB 105018
> Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
> http://www.tq-group.com/
>
>