Hi Sean,
On Thu, Jul 1, 2021 at 5:52 PM Sean Anderson <sean.anderson@xxxxxxxx> wrote:
> On 6/30/21 5:12 AM, Geert Uytterhoeven wrote:
> > On Wed, Jun 30, 2021 at 9:57 AM Luca Ceresoli <luca@xxxxxxxxxxxxxxxx> wrote:
> >> On 29/06/21 23:41, Sean Anderson wrote:
> >> > On 6/29/21 5:23 PM, Luca Ceresoli wrote:
> >> >> On 29/06/21 17:47, Sean Anderson wrote:
> >> >>> These properties allow configuring the SD/OE pin as described in the
> >> >>> datasheet.
> >> >>
> >> >> *Many* thanks for addressing this issue so quickly!
> >> >>
> >> >>> Signed-off-by: Sean Anderson <sean.anderson@xxxxxxxx>
> >
> >> >>> a/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml
> >> >>> b/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml
> >> >>> index 28675b0b80f1..51f0f78cc3f4 100644
> >> >>> --- a/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml
> >> >>> +++ b/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml
> >> >>> @@ -30,6 +30,22 @@ description: |
> >> >>> 3 -- OUT3
> >> >>> 4 -- OUT4
> >> >>>
> >> >>> + The idt,(en|dis)able-shutdown and idt,output-enable-active-(high|low)
> >> >>> + properties control the SH (en_global_shutdown) and SP bits of the
> >> >>> + Primary Source and Shutdown Register, respectively. Their behavior is
> >> >>> + summarized by the following table:
> >> >>> +
> >> >>> + SH SP Output when the SD/OE pin is Low/High
> >> >>> + == == =====================================
> >> >>> + 0 0 Active/Inactive
> >> >>> + 0 1 Inactive/Active
> >> >>> + 1 0 Active/Shutdown
> >> >>> + 1 1 Inactive/Shutdown
> >> >>> +
> >> >>> + If no properties related to these bits are specified, then they will
> >> >>> + be left in their default state. This may be useful if the SH and SP
> >> >>> + bits are set to a default value using the OTP memory.
> >> >>
> >> >> This paragraph looks more an implementation description than a hardware
> >> >> description.
> >> >
> >> > It of course *is* an implementation description. As Geert found out, it
> >> > is important to keep the defaults if none of these properties are
> >> > specified.
> >>
> >> DT should describe hardware, not implementation. The difference is
> >> subtle at times, but it is important. Other OSes, bootloaders,
> >> firmwares, whatever can have a totally different implementation but use
> >> the same DT.
> >
> > In general, it's best for a driver not to rely on any preprogramming
> > done by e.g. the bootloader before.
>
> This is part of the reason I wanted to add these properties in the first
> place. I'm working on a board where one version has had the OTP
> programmed, and one version has not. But of course, if we set these bits
> in software then I do not have to worry about whether the OTP has set up
> something sane.
>
> >
> > The concept of "One-Time Programming (OTP) bits" adds yet another
> > dimension to the already complicated boot chain of dependencies.
> > But due to the one-time feature I consider that more stable than
> > other firmware, which can be upgraded, causing changed behavior,
> > unlike OTP bits.
> >
> >> Perhaps these properties might be made mandatory later, after upgrading
> >> all DTs (at least those in mainline Linux). and a grace period.
> >
> > Yes, they should be marked as required.
>
> I don't think I can do that without going through all existing users and
> defining these properties for them. Otherwise, dt_bindings_check will
> complain. I believe (but please correct me if I'm wrong) that patches
> are not to introduce new warnings.
>
> However, setting these propreties is not possible for me to do; I would
> need someone familiar with their board to determine how the SD/OE pin is
> used, and what the correct setting is.
Sure, we can only make them required once all in-tree DTS files have been
fixed.
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
