linux-kernel - Re: [PATCH RFC 1/5] phy: qcom-qmp: add support for pipe clock muxing

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]

Date:   Fri, 22 Apr 2022 13:22:08 +0200
From:   Johan Hovold <johan@...nel.org>
To:     Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
Cc:     Johan Hovold <johan+linaro@...nel.org>,
        Andy Gross <agross@...nel.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
        Kishon Vijay Abraham I <kishon@...com>,
        Vinod Koul <vkoul@...nel.org>,
        Stephen Boyd <swboyd@...omium.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Stanimir Varbanov <svarbanov@...sol.com>,
        Krzysztof Wilczyński <kw@...ux.com>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Prasad Malisetty <quic_pmaliset@...cinc.com>,
        linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org,
        linux-phy@...ts.infradead.org
Subject: Re: [PATCH RFC 1/5] phy: qcom-qmp: add support for pipe clock muxing

[ Again, please trim unnecessary context from your replies. ]

On Fri, Apr 22, 2022 at 01:35:01PM +0300, Dmitry Baryshkov wrote:
> On Fri, 22 Apr 2022 at 13:20, Johan Hovold <johan@...nel.org> wrote:
> >
> > On Thu, Apr 21, 2022 at 02:08:27PM +0300, Dmitry Baryshkov wrote:
> > > On 21/04/2022 13:20, Johan Hovold wrote:

> > > > +   /* Get optional pipe clock mux and default reference source clock. */
> > > > +   qphy->pipemux_clk = of_clk_get_by_name(np, "mux");
> > > > +   if (IS_ERR(qphy->pipemux_clk)) {
> > > > +           ret = PTR_ERR(qphy->pipemux_clk);
> > > > +           if (ret == -EPROBE_DEFER)
> > > > +                   return ret;
> > > > +
> > > > +           qphy->pipemux_clk = NULL;

> > > > +   } else {
> > > > +           qphy->piperef_clk = of_clk_get_by_name(np, "ref");
> > > > +           if (IS_ERR(qphy->piperef_clk)) {
> > > > +                   ret = PTR_ERR(qphy->piperef_clk);
> > > > +                   return dev_err_probe(dev, ret,
> > > > +                                        "failed to get lane%d piperef_clk\n",
> > > > +                                        id);
> > > > +           }
> > > > +   }
> > > > +
> > >
> > > As a second thought.
> > > This needs to be more explicit. If the chipset requires the pipe clock
> > > remuxing, we must fail if the clocks were not provided. So depending on
> > > the qmp instance/property the driver should either use devm_clk_get()
> > > (instead of _optional) or skip this block completely.
> >
> > No, the kernel is not a DT validator (and we have the YAML bindings for
> > that now).
> 
> It is not about DT validation. It is about passing a correct DT.

Heh. That's the same thing.

> The file can come up from the kernel. It can come from the older
> kernel.  OR it can come from the vendor. Or it even might be being a
> part of firmware flashed into the device.  So we can not assume that
> the DT is correct just because the in-kernel DT passes YAML
> validation.

Again, no. The kernel does not need to implement DT validation and can
assume that the DT describes the hardware accurately. If the DT says
there's a mux, the driver can use it. If there's no mux in DT, the
driver can assume it isn't there.

The only thing that complicates things here is the sc7280 dts which has
been released in 5.16. We don't care about Qualcomm's kernels and dts.

> So, as I wrote, the whole patchset needs much more care about compatibility.
> 
> > > But this will not work with earlier DTS files.
> >
> > So this is not a problem (but if we really wanted to have the driver
> > validate the DT it can be done by updating the compatible strings).
> 
> We should not update compatible strings just because the driver
> changes. Compat strings describe the hardware, not the Linux point of
> view on it.

We can, it's a documented practise in case a binding needs to be
updated in an incompatible way:

	https://www.kernel.org/doc/html/latest/devicetree/bindings/ABI.html

But I don't think it'll be needed here.

Johan