Hi Luca,
On Tue, Jan 24, 2023 at 9:12 AM Luca Ceresoli <luca.ceresoli@xxxxxxxxxxx> wrote:
> On Thu, 19 Jan 2023 14:27:43 -0500
> Sean Anderson <sean.anderson@xxxxxxxx> wrote:
> > On 1/11/23 10:55, Geert Uytterhoeven wrote:
> > > "make dtbs_check":
> > >
> > > arch/arm64/boot/dts/renesas/r8a77951-salvator-xs.dtb: clock-generator@6a: 'idt,shutdown' is a required property
> > > From schema: Documentation/devicetree/bindings/clock/idt,versaclock5.yaml
> > > arch/arm64/boot/dts/renesas/r8a77951-salvator-xs.dtb: clock-generator@6a: 'idt,output-enable-active' is a required property
> > > From schema: Documentation/devicetree/bindings/clock/idt,versaclock5.yaml
> > >
> > > Versaclock 5 clock generators can have their configuration stored in
> > > One-Time Programmable (OTP) memory. Hence there is no need to specify
> > > DT properties for manual configuration if the OTP has been programmed
> > > before. Likewise, the Linux driver does not touch the SD/OE bits if the
> > > corresponding properties are not specified, cfr. commit d83e561d43bc71e5
> > > ("clk: vc5: Add properties for configuring SD/OE behavior").
> > >
> > > Reflect this in the bindings by making the "idt,shutdown" and
> > > "idt,output-enable-active" properties not required, just like the
> > > various "idt,*" properties in the per-output child nodes.
> >
> > IMO we should set this stuff explicitly.
>
> I took a moment to think better about this and I think I get your point
> Sean in preferring that the hardware is described in detail.
>
> However I'm still leaning towards approving Geert's proposal.
>
> I'm based on the principle that DT is there to describe the aspects of
> the hardware that the software needs _and_ it is unable to discover by
> itself.
>
> Based on that, does the software need to know SD/OR configuration? If
> they are already written in the OTP then it doesn't. Also if the chip
> default is the use that is implemented on the board, it also doesn't
> (like lots of optional properties, especially when in most cases a
> given chip is used in the default configuration but not always).
These clock generators are typically programmed through OTP because
(at least some of) the configuration is needed to boot the board.
> To some extent, writing settings in an OTP is similar to producing a
> different chip where these values are hard-coded and not configured.
Indeed. Except that here they can still be overwritten by software
later.
The latter is an inherent danger, and is probably the reason why Sean
wants to make it explicit: if the configuration is ever changed by
software, and the system is restarted without resetting the clock
generator (at least 5P49V6901A does not have a reset line), or using
kexec/kdump, the newly booted kernel does not know the intended
settings, and won't restore the correct configuration.
> I'm wondering whether Geert has a practical example of a situation
> where it is better to have these properties optional.
My issue was that these properties were introduced long after the
initial bindings, hence pre-existing DTS does not have them.
Yes, we can add them, but then we have to read out the OTP-programmed
settings first. If that's the way to go, I can look into that, though...
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
