Re: [PATCH v3 2/2] dt-bindings: rtc: m41t80: Mark the clock: subnode as deprecated

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Fri, Dec 16, 2022 at 03:36:17PM +0100, Marek Vasut wrote:
> On 12/16/22 15:24, Sebastian Reichel wrote:
> > On Thu, Dec 15, 2022 at 08:39:47PM +0100, Marek Vasut wrote:
> > > On 12/15/22 19:06, Sebastian Reichel wrote:
> > > > On Sun, Dec 11, 2022 at 09:51:24PM +0100, Marek Vasut wrote:
> > > > > The clock {} subnode seems like it is describing an always-on clock
> > > > > generated by the PMIC. This should rather be modeled by consumer of
> > > > > the clock taking phandle to the RTC node itself, since it already
> > > > > does have clock-cells and all. Since there are no users of the clock
> > > > > subnode in tree anyway, mark it as deprecated to avoid proliferation
> > > > > of this approach.
> > > > > 
> > > > > Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>
> > > > > Signed-off-by: Marek Vasut <marex@xxxxxxx>
> > > > > ---
> > > > > Cc: Alessandro Zummo <a.zummo@xxxxxxxxxxxx>
> > > > > Cc: Alexandre Belloni <alexandre.belloni@xxxxxxxxxxx>
> > > > > Cc: Rob Herring <robh+dt@xxxxxxxxxx>
> > > > > Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@xxxxxxxxxx>
> > > > > Cc: linux-rtc@xxxxxxxxxxxxxxx
> > > > > To: devicetree@xxxxxxxxxxxxxxx
> > > > > ---
> > > > > V2: - Add AB from Krzysztof
> > > > > V3: - No change
> > > > > ---
> > > > 
> > > > I just noticed this by accident. Basically everything in the patch
> > > > description is wrong:
> > > > 
> > > > 1. There is a in-tree user: arch/arm/boot/dts/imx6dl-qmx6.dtsi
> > > 
> > > Sorry, I missed this one.
> > > 
> > > > 2. The PMIC has nothing to do with this
> > > 
> > > In [3] the commit message claims the PMIC supplies 32kHz clock to i.MX6 CKIL,
> > > which per IMX6DQRM rev.6 Table 18-3 row SNVS indirectly supplies SNVS RTC.
> > > This reminded me of commit:
> > 
> > The word PMIC is not mentioned once in [3].
> 
> s@PMIC@m41t62 RTC@, sorry.
> 
> > PMIC is not involved.
> > The QMX6 32khz chain is like this:
> > 
> > 32kHz crystal -> m41t62 crystal input
> > m41t62 clock output -> i.MX6 CKIL
> > 
> > > 9509593f327ac ("arm64: dts: imx8mm: Model PMIC to SNVS RTC clock path on
> > > Data Modul i.MX8M Mini eDM SBC")
> > > 
> > > which solves exactly the same problem, system hangs when 32 kHz clock are
> > > stopped, except this time on i.MX8MM, clock are generated by PMIC on I2C
> > > (notice how the PMIC is referenced directly) and the clock are supplied to
> > > the SVNS RTC XTal terminals.
> > > 
> > > I wonder if this could be reused on the QMX6 board too?
> > 
> > IIRC On i.MX6 referencing the I2C connected RTC results in boot
> > hanging forever when trying to get the ckil clock in
> > imx6q_clocks_init. At least it used to be the case when I was
> > working on this - I no longer have access to the boards. Of course
> > properly referencing the RTC clock was the first route I tried.
> 
> Hmmmmm, what shall we do, un-deprecate the clock sub-node?

Depends on the exact meaning of "deprecate: true". I think we all
agree, that it's better to avoid the sub-node and only use it when
it's really required. But having a deprecation warning for an
in-tree user without a clear path forward also seems to be annoying.
I think it makes sense for the DT binding maintainers (Rob/Krzysztof)
to comment on this.

-- Sebastian

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux