Re: [PATCH 09/15] dt-bindings: phy: qcom,qmp-pcie: mark current bindings as legacy

Johan Hovold
2022-10-18T12:44:56Z

On Tue, Oct 18, 2022 at 02:37:35PM +0300, Dmitry Baryshkov wrote:
> On Tue, 18 Oct 2022 at 13:21, Johan Hovold <johan@kernel.org> wrote:
> >
> > On Tue, Oct 18, 2022 at 12:52:03PM +0300, Dmitry Baryshkov wrote:
> > > Hi,
> > >
> > > On Mon, 17 Oct 2022 at 17:54, Johan Hovold <johan+linaro@kernel.org> wrote:
> > > >
> > > > The current QMP PCIe PHY bindings are based on the original MSM8996
> > > > binding which provided multiple PHYs per IP block and these in turn were
> > > > described by child nodes.
> > > >
> > > > Later QMP PCIe PHY blocks only provide a single PHY and the remnant
> > > > child node does not really reflect the hardware.
> > > >
> > > > The original MSM8996 binding also ended up describing the individual
> > > > register blocks as belonging to either the wrapper node or the PHY child
> > > > nodes.
> > > >
> > > > This is an unnecessary level of detail which has lead to problems when
> > > > later IP blocks using different register layouts have been forced to fit
> > > > the original mould rather than updating the binding. The bindings are
> > > > arguable also incomplete as they only the describe register blocks used
> > > > by the current Linux drivers (e.g. does not include the per lane PCS
> > > > registers).
> > >
> > > I'd like to point out that it's not only a problem peculiar to the
> > > PCIe PHYs. Other QMP PHY families also follow the same approach of
> > > separating the serdes into the common part and rx/tx/PCS/whatever into
> > > the PHY subnodes.
> >
> > Yeah, I already mentioned that in the cover letter.
> >
> > > For the USB+DP combo PHYs we have to have subnodes, however it would
> > > also be logical to move serdes register to the subnode devices,
> > > leaving only DP_COM in the base node.
> >
> > No, not at all. First, we may not even need the subnodes (the individual
> > PHYs can be indexed), but even if we do keep them for the combo case,
> > the register block should go in the wrapper node as the registers can be
> > and are shared (e.g. for sc8280xp for which the current binding is
> > completely broken).
>
> Hmm, which register blocks are shared on the sc8280xp combo PHY? Could
> you please lightly describe it, if possible?

At least serdes and tx. Note that the combo PHY on sc8280xp also
supports USB4, so there is likely some differences compared to the older
platforms.

And while I haven't looked in detail at the older platforms for the
combo PHYs yet, describing the separate register subregions there
appears to be just as misguided for those (e.g. as the binding currently
only describes what Linux is using) and is mostly an artefact of how the
original Linux driver was implemented.

Johan