two proposed additions to the config string

[ron minnich]

Maybe I can divine this stuff but I'm not sure how :-)

I think we need a hz entry for the rtc and each core.

The rtc should be pretty obvious; it would be nice to just know the clock rate of the rtc, rather than try to divine it with timing loops. Actually, I don't even know how one could figure it out absent some fixed reference source, and I don't know what that is. Just counting on 10 mhz. to always be the rate seems dicy.

Similarly, I think each core might have a hz attribute too. I can imagine a machine in which some cores run at lower rates for power reasons (maybe this is a terrible idea?). Certainly, today in x86 systems, it's common to up and down clock cores quite dynamically. It would be even nicer just to have a register I can read that tells me the clock rate. I can dream, right?

So rtc would be

rtc {

addr 0xwhatever;

hz 0xwhatever;
};

harts would be

core {0{0{hz 0xwhatever; ...};};};

hz in general might be useful for many things; the uart comes to mind. For 16550 uart emulations it's good to know what the source clock is.

If this is a bad idea for some reason I'm sure someone will let me know :-)

ron

[Michael Clark]

On 18 Jan 2017, at 6:35 AM, ron minnich <rmin...@gmail.com> wrote:

Maybe I can divine this stuff but I'm not sure how :-)

I think we need a hz entry for the rtc and each core.

The rtc should be pretty obvious; it would be nice to just know the clock rate of the rtc, rather than try to divine it with timing loops. Actually, I don't even know how one could figure it out absent some fixed reference source, and I don't know what that is. Just counting on 10 mhz. to always be the rate seems dicy.

Similarly, I think each core might have a hz attribute too. I can imagine a machine in which some cores run at lower rates for power reasons (maybe this is a terrible idea?). Certainly, today in x86 systems, it's common to up and down clock cores quite dynamically. It would be even nicer just to have a register I can read that tells me the clock rate. I can dream, right?

So rtc would be

rtc {

addr 0xwhatever;

hz 0xwhatever;
};

This is a good idea.

I noticed that sbi_timebase currently returns a hard-coded value. BBL could easily be modified to return a setting from the config string. Also the BBL config string code can already handle base 10 integers as well as hex.

I noticed that clock-frequency is the canonical term used by open firmware. It is the term used in of_serial.c in linux-kernel.

rtc {
    addr 0xwhatever;
    clock-frequency 10000000;
};

harts would be

core {0{0{hz 0xwhatever; ...};};};

hz in general might be useful for many things; the uart comes to mind. For 16550 uart emulations it’s good to know what the source clock is.

The question for the UART would be which clock? It may be the system clock in some systems (FPGA) or it could be separate 1MHz reference clock. The UART section probably could have its own clock frequency.

Also should the default UART settings be in the config string e.g. “115200,8N1” so that the firmware can program the UART, or is this reasonable to hard code as a “documented” start up value for early console. 

I personally cheated and set the UART frequency to its maximum for SBI console code talking to a virtual UART:

static void uart_init()
{
  /* enable UART frequency programming */
  uart_base[REG_LCR] = LCR_DLAB;

  /* set highest frequency */
  uart_base[REG_DLL] = 1;
  uart_base[REG_DLM] = 0;

  /* 8-bit data, 1-bit odd parity */
  uart_base[REG_LCR] = LCR_8BIT | LCR_PODD;

  /* interrupt */
  uart_base[REG_IER] = IER_ERBDA;
}

The loader might need the model too, as some models have 32-bit register layout instead of 8-bit register layout (Xilinx AXI UART16550 comes to mind) and various other quirks (although looking at of_serial.c this might be handled with a “reg-shift” property).

e.g. an MMIO UART with a 1MHz clock that is set to 115200,8N1 at POST/BIST.

uart {

    model “meta8250”;

    addr 0x40003000;

    clock-frequency 1000000;

    settings “115200,8N1”;

}

If this is a bad idea for some reason I’m sure someone will let me know :-)

Sounds practical.

ron

--
You received this message because you are subscribed to the Google Groups "RISC-V ISA Dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to isa-dev+u...@groups.riscv.org.
To post to this group, send email to isa...@groups.riscv.org.
Visit this group at https://groups.google.com/a/groups.riscv.org/group/isa-dev/.
To view this discussion on the web visit https://groups.google.com/a/groups.riscv.org/d/msgid/isa-dev/CAP6exYJX3EpFxjW-tYEEmnOPK74-bBrfauC%2BjOkdVyBXmA4Qdg%40mail.gmail.com.

[ron minnich]

On Tue, Jan 17, 2017 at 2:36 PM Michael Clark <michae...@mac.com> wrote:

I noticed that clock-frequency is the canonical term used by open firmware. It is the term used in of_serial.c in linux-kernel.

rtc {
    addr 0xwhatever;
    clock-frequency 10000000;
};

riscv is a system that we hope has a long life. I'm not sure u-boot or linux usage should drive the choice of names, and I don't see that clock-frequency has more information than hz but it's up to the community :-)

 

The question for the UART would be which clock? It may be the system clock in some systems (FPGA) or it could be separate 1MHz reference clock. The UART section probably could have its own clock frequency.

That's what I'm thinking. 

I think we should also assume not all cores run at the same frequency.

 

Also should the default UART settings be in the config string e.g. “115200,8N1” so that the firmware can program the UART, or is this reasonable to hard code as a “documented” start up value for early console. 

Were we to put that into the config string, I would prefer the fields be broken out. The fewer parsers in firmware the better, and adding a whole parser for 115200,8N1 (and all possible variations) seems like too much.

uart { hz = whatever;

default {baud = whatever; bits = 8; etc. etc.}

};

here again we see the nice aspect of the config string. To add a set of default values is no big deal. Were we to think in terms of OFW we might end up with default-baud, default-bits, etc. because x-y is frequently used in OFW to denote a hierarchy. config strings have a hierarchy and we ought to use it. Any device can have a tag of default with device-dependent entries.

The loader might need the model too, as some models have 32-bit register layout instead of 8-bit register layout (Xilinx AXI UART16550 comes to mind) and various other quirks (although looking at of_serial.c this might be handled with a “reg-shift” property).

Yes, this is always fun. We had to put special coreboot code in for 8-bit vs. 32-bit aligned registers on 16550. 

    settings “115200,8N1”;

Again, I'm not a fan of adding another parser when it's just as easy to specify these values with several config entries. But I think putting in the settings is a good idea.

Thanks

ron

[Andrew Waterman]

On Tue, Jan 17, 2017 at 9:35 AM, ron minnich <rmin...@gmail.com> wrote:
> Maybe I can divine this stuff but I'm not sure how :-)
>
> I think we need a hz entry for the rtc and each core.
>
> The rtc should be pretty obvious; it would be nice to just know the clock
> rate of the rtc, rather than try to divine it with timing loops. Actually, I
> don't even know how one could figure it out absent some fixed reference
> source, and I don't know what that is. Just counting on 10 mhz. to always be
> the rate seems dicy.

Agreed. The RTC frequency may not be in the config string that's
baked into ROM, because the RTC might be configurable, but the boot
sequence should be able to determine/set the frequency and insert it
into the config string for the next software stage.

>
> Similarly, I think each core might have a hz attribute too. I can imagine a
> machine in which some cores run at lower rates for power reasons (maybe this
> is a terrible idea?). Certainly, today in x86 systems, it's common to up and
> down clock cores quite dynamically. It would be even nicer just to have a
> register I can read that tells me the clock rate. I can dream, right?

In general, cores may not even be capable of running at a fixed clock
frequency. If the hz key is present, it should probably connote
either a fixed frequency, or the default frequency if DVFS is
supported.

>
> So rtc would be
> rtc {
> addr 0xwhatever;
> hz 0xwhatever;
> };
>
> harts would be
> core {0{0{hz 0xwhatever; ...};};};
>
> hz in general might be useful for many things; the uart comes to mind. For
> 16550 uart emulations it's good to know what the source clock is.
>
> If this is a bad idea for some reason I'm sure someone will let me know :-)
>
> ron
>

[Karsten Merker]

On Wed, Jan 18, 2017 at 11:35:57AM +1300, Michael Clark wrote:

> On 18 Jan 2017, at 6:35 AM, ron minnich <rmin...@gmail.com> wrote:
>
> Maybe I can divine this stuff but I'm not sure how :-)
>
> I think we need a hz entry for the rtc and each core.
>
> The rtc should be pretty obvious; it would be nice to just
> know the clock rate of the rtc, rather than try to divine
> it with timing loops. Actually, I don't even know how one
> could figure it out absent some fixed reference source, and
> I don't know what that is. Just counting on 10 mhz. to
> always be the rate seems dicy.

Hello,

I assume "rtc" here means the machine timer register (mtime)?
I ask because in software engineering an "rtc" usually means
a (battery-backed) hardware real-time-clock that provides
persistent time-of-day and date registers.

> harts would be
> core {0{0{hz 0xwhatever; ...};};};
>
> hz in general might be useful for many things; the uart comes to mind. For
> 16550 uart emulations it’s good to know what the source clock is.
>
>
> The question for the UART would be which clock? It may be the system clock in
> some systems (FPGA) or it could be separate 1MHz reference clock.
> The UART section probably could have its own clock frequency.
>
> Also should the default UART settings be in the config string e.g. “115200,8N1”
> so that the firmware can program the UART, or is this reasonable to hard code
> as a “documented” start up value for early console.

[...]

> The loader might need the model too, as some models have 32-bit register layout
> instead of 8-bit register layout (Xilinx AXI UART16550 comes to mind) and
> various other quirks (although looking at of_serial.c this might be handled
> with a “reg-shift” property).
>
> e.g. an MMIO UART with a 1MHz clock that is set to 115200,8N1 at POST/BIST.
>
> uart {
> model “meta8250”;
> addr 0x40003000;
> clock-frequency 1000000;
> settings “115200,8N1”;
> }

According to chapter 8 of the privileged spec v1.9.1,
config-string uses the device-tree data model (which includes
the existing bindings, i.e. structures and property names).
Extensions to the existing bindings can be introduced if
necessary, but where bindings already exist, they must be used.
Therefore please don't invent incompatible bindings for stuff
that is already there like the binding definition for
8250/16550-style UARTs; please cf.

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/serial/8250.txt

What you want to define as "model" is already defined as the
"compatible" property.

Your "addr" is already defined as the "reg" property (which lists
the MMIO register base adddress(es) and -range(s)). Real-world
SoCs often have multiple logically-connected register blocks
interspaced with registers belonging to other IP blocks in the
same SoC, so it is important that it is possible to specifiy
multiple base addresses and ranges for one IP block.

The clock supply to the UART is handled by either the
"clock-frequency" property for a fixed-frequency clock or by a
phandle reference to a separate clock definition in case of a
variable clock supply.

In real-world SoCs, the UART clocks are usually not
fixed-frequency clocks, but instead the UART clock is derived
from a (choice of) PLL clock(s) with multiple programmable
pre-dividers, which themselves are driven by one or multiple
(sometimes programmable) oscillators. As multiple devices can
depend on the same parent clocks and can need runtime adjustments
of their clock supply, it can be necessary to do coordinated
modifications of the parent clock settings of multiple devices at
runtime. In case of an UART it can for example be necessary to
modify the UART input clock frequency for achieving specific
baudrates. To be able to do properly coordinated clock tree
changes, the device-tree (and thereby config string) must provide
a proper representation of the clock hardware to the operating
system. Therefore complex clock setups are represented by a
separate clock definition that gets referenced by the devices
that use the corresponding clock instead of just supplying a
fixed "clock-frequency" property. A fixed clock-frequency property
can only be used if the UART is really driven by a fixed-frequency
clock.

In real-world SoCs these interdependencies in the clock tree
commonly exist for many of the on-chip peripherals (MMC
controllers, SPI controllers, PWM controllers, audio controllers,
infraread controllers, etc).

To get back from a visit to the complexities of the clock
subsystem to the 16550 UART bindings:

For UART models that only accept 32bit register access we have
the "reg-io-width" property.

The interrupt line that the UART is connected to is specified by
the "interrupts" property.

So a UART definition could look like (in the simple case of a
16550a-compatible UART implementation driven by a single
fixed-frequency clock and triggering interrupt 25 on a
directly-connected master interrupt-controller):

uart@12340000 {
compatible = "ns16550a";
reg = <0x12340000> <0x100>;
clock-frequency = "1000000";
interrupts = <25>;
}

Regarding the "settings" property from your example above: please
don't intermix hardware description and system configuration
with each other. A node describing a piece of hardware or an IP
block like the UART shall contain the description of how the
hardware works and how is interconnected, but not how is gets
configured in a particular system. For passing configuration
information, device-tree has the "chosen" node, in which
information like the amount of available system memory or the
device to use as the system console can be provided. The
configuration entries in the "chosen" node can contain a
reference to the hardware node they configure. A real-world
example from an ARM-SoC device-tree:

chosen {
stdout-path = "serial0:115200n8";
};

where serial0 is a phandle reference to the corresponding UART
node.

Regards,
Karsten
--
Gem. Par. 28 Abs. 4 Bundesdatenschutzgesetz widerspreche ich der Nutzung
sowie der Weitergabe meiner personenbezogenen Daten für Zwecke der
Werbung sowie der Markt- oder Meinungsforschung.

[Karsten Merker]

On Tue, Jan 17, 2017 at 04:53:25PM -0800, Andrew Waterman wrote:
> On Tue, Jan 17, 2017 at 9:35 AM, ron minnich <rmin...@gmail.com> wrote:

> > Similarly, I think each core might have a hz attribute too. I can imagine a
> > machine in which some cores run at lower rates for power reasons (maybe this
> > is a terrible idea?). Certainly, today in x86 systems, it's common to up and
> > down clock cores quite dynamically. It would be even nicer just to have a
> > register I can read that tells me the clock rate. I can dream, right?
>
> In general, cores may not even be capable of running at a fixed clock
> frequency. If the hz key is present, it should probably connote
> either a fixed frequency, or the default frequency if DVFS is
> supported.

We already have an established binding for DVFS information:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/opp/opp.txt

[ron minnich]

On Wed, Jan 18, 2017 at 9:06 AM Karsten Merker <mer...@debian.org> wrote:

I assume "rtc" here means the machine timer register (mtime)?
I ask because in software engineering an "rtc" usually means
a (battery-backed) hardware real-time-clock that provides
persistent time-of-day and date registers.

I've never heard the terms "software engineering" and "rtc" used in the same sentence in quite this way :-)

but yes, in  the 1.9.x spec, rtc means mtime. Not my definition, I just work here :-)

We had a discussion here yesterday about 'clock-frequency' as a name vs. 'hz', including a person whose first language is not english. hz was preferred: it's an SI unit, it's short, and it packs the same info as clock-frequency into fewer bits.

 

>     harts would be
>     core {0{0{hz 0xwhatever; ...};};};
>
>     hz in general might be useful for many things; the uart comes to mind. For
>     16550 uart emulations it’s good to know what the source clock is.
>
>
> The question for the UART would be which clock? It may be the system clock in
> some systems (FPGA) or it could be separate 1MHz reference clock.
> The UART section probably could have its own clock frequency.

Generally, to keep life simple for payloads, firmware tends to fix a lot of these settings, particularly for uarts -- that's what we've done on many coreboot ARM systems anyway. So "config string presented by SOC" may be expanded upon/changed by coreboot for the payload.  

According to chapter 8 of the privileged spec v1.9.1,
config-string uses the device-tree data model (which includes
the existing bindings, i.e. structures and property names).

well, not as I read it: " Config string is a new format that is backwards-compatible with device tree string (as far as DTS specs exist) but can include additional configuration information in other memory regions."

config string is a superset of DTS.  

Extensions to the existing bindings can be introduced if
necessary, but where bindings already exist, they must be used.

well, I reread the (very short) chapter and I don't read it that way at all. And that part of the spec in particular is still in flux.

 

Further, while I understand the impetus to lock us down to "what Linux says in 2016", I don't agree with it. DTS is a 1980s construct and I would hope I'm still not seeing it in 2050 (I won't anyway but maybe you will).

ron

[Karsten Merker]

On Wed, Jan 18, 2017 at 05:27:05PM +0000, ron minnich wrote:
> On Wed, Jan 18, 2017 at 9:06 AM Karsten Merker <mer...@debian.org> wrote:
>
> I assume "rtc" here means the machine timer register (mtime)?
> I ask because in software engineering an "rtc" usually means
> a (battery-backed) hardware real-time-clock that provides
> persistent time-of-day and date registers.
>
> I've never heard the terms "software engineering" and "rtc" used in the same
> sentence in quite this way :-)

Ok, maybe I am a software guy who looks too much at low-level
stuff like device drivers and bootloaders ;-).

> but yes, in  the 1.9.x spec, rtc means mtime. Not my definition, I just work
> here :-)

Thanks :-).

> According to chapter 8 of the privileged spec v1.9.1,
> config-string uses the device-tree data model (which includes
> the existing bindings, i.e. structures and property names).
>
> well, not as I read it: " Config string is a new format that is
> backwards-compatible with device tree string (as far as DTS specs exist) but
> can include additional configuration information in other memory regions."
>
> config string is a superset of DTS.  
>
> Extensions to the existing bindings can be introduced if
> necessary, but where bindings already exist, they must be used.
>
> well, I reread the (very short) chapter and I don't read it that way at all.

To me the meaning of the phrase "Config string is a new format

that is backwards-compatible with device tree string (as far as

DTS specs exist)" is clearly what I have written: use device-tree
bindings as far as we have them already and extend them if the
existing ones are not enough to describe our system. By way of
these possible extensions config-string is a superset of
device-tree, but "superset" means "extend where necessary",
not "ignore everything we have and reimplement all of it in an
incompatible way".

I have taken part in the discussions that have led to the current
wording in the privileged spec. We have really gone through all
the pros and cons of the various proposals and the result of the
discussion clearly was: config-string shall use the device-tree
data model (including the bindings). Not necessarily the
representation as a binary device-tree blob (dtb) (because some
people didn't like the binary format and saw problems with
relocation and overlays) or device-tree source (dts) (because dts
allows quite a lot of freedom regarding the formats that can be
used to provide values and people feared that that would make a
parser in limited ROM space too complex), but definitely the data
model including the bindings. The decision was that
config-string should be a device-tree representation in form of a
"DTS light", i.e. use the same bindings, but be stricter with
the syntax and allow only a subset of the numerical formats,
limit the charset to ASCII, limit the valid whitespace
characters, etc. to keep the parser small and easy to validate).

> Further, while I understand the impetus to lock us down to
> "what Linux says in 2016", I don't agree with it. DTS is a
> 1980s construct and I would hope I'm still not seeing it in
> 2050 (I won't anyway but maybe you will).

This has nothing to do with "lock us down to what Linux says in
2016". Device-Tree is used as a hardware description language by
Linux, by the BSDs, by u-boot, and even the UEFI spec includes
device-tree as hardware description language instead of ACPI.
Heck, even the ACPI DSD nowadays supports supplying hardware
information in device-tree format.

[ron minnich]

I was not aware of the amount of work you had put into this already; given that fact I'll yield the point :-)

ron

[Michael Clark]

On 19 Jan 2017, at 6:06 AM, Karsten Merker <mer...@debian.org> wrote:

On Wed, Jan 18, 2017 at 11:35:57AM +1300, Michael Clark wrote:

   On 18 Jan 2017, at 6:35 AM, ron minnich <rmin...@gmail.com> wrote:

   Maybe I can divine this stuff but I'm not sure how :-)

   I think we need a hz entry for the rtc and each core.

OK. hz is nice and simple. 

I changed the sim I am working on to emit hz in the rtc section of the config and for bbl to read the hz from the config string that it later returns in calls to the SBI timebase API. This solves a hard-coded value issue with bbl.

https://github.com/michaeljclark/bbl-lite/commit/4492d28b36316d16f869696e325e56f9eeef143e

Ron, In fact you helped me find a hang bug in the sim I am working on. I noticed that linux was hanging in nanosleep (actually in __getnstimeofday64). It seems that if the hz value for the RTC is wrong nanosleep can go into a loop, so in looking into this you helped my fix a bug. 

We should think about where these sorts of patches go. lowrisc have a version of bbl which has UART console support instead of HTIF console. I have a similar bbl patch for a virtual UART console, and another patch for MMIO HTIF similar to that used by RISCVEMU…

okay. it would be interesting to plumb through a UART to the in-tree driver using the existing linux config mechanisms.

Your "addr" is already defined as the "reg" property (which lists
the MMIO register base adddress(es) and -range(s)).  Real-world
SoCs often have multiple logically-connected register blocks
interspaced with registers belonging to other IP blocks in the
same SoC, so it is important that it is possible to specifiy
multiple base addresses and ranges for one IP block.

addr is currently used by RISC-V config strings in the wild however I guess this can easily be changed to reg.

The clock supply to the UART is handled by either the
"clock-frequency" property for a fixed-frequency clock or by a
phandle reference to a separate clock definition in case of a
variable clock supply.

In real-world SoCs, the UART clocks are usually not
fixed-frequency clocks, but instead the UART clock is derived
from a (choice of) PLL clock(s) with multiple programmable
pre-dividers, which themselves are driven by one or multiple
(sometimes programmable) oscillators.  As multiple devices can
depend on the same parent clocks and can need runtime adjustments
of their clock supply, it can be necessary to do coordinated
modifications of the parent clock settings of multiple devices at
runtime.  In case of an UART it can for example be necessary to
modify the UART input clock frequency for achieving specific
baudrates.  To be able to do properly coordinated clock tree
changes, the device-tree (and thereby config string) must provide
a proper representation of the clock hardware to the operating
system.  Therefore complex clock setups are represented by a
separate clock definition that gets referenced by the devices
that use the corresponding clock instead of just supplying a
fixed "clock-frequency" property. A fixed clock-frequency property
can only be used if the UART is really driven by a fixed-frequency
clock.

Understand.

In real-world SoCs these interdependencies in the clock tree
commonly exist for many of the on-chip peripherals (MMC
controllers, SPI controllers, PWM controllers, audio controllers,
infraread controllers, etc).

To get back from a visit to the complexities of the clock
subsystem to the 16550 UART bindings:

For UART models that only accept 32bit register access we have
the "reg-io-width" property.

The interrupt line that the UART is connected to is specified by
the "interrupts" property.

So a UART definition could look like (in the simple case of a
16550a-compatible UART implementation driven by a single
fixed-frequency clock and triggering interrupt 25 on a
directly-connected master interrupt-controller):

uart@12340000 {
compatible = "ns16550a";
reg = <0x12340000> <0x100>;
clock-frequency = "1000000";
interrupts = <25>;
}

This makes sense.

The question is how much of the format of the current config string will survive in a future platform specification. Numerical literals in the config string are currently not enclosed by angle brackets. I also see you have a quoted numeric literal.

Regarding the "settings" property from your example above: please
don't intermix hardware description and system configuration
with each other.  A node describing a piece of hardware or an IP
block like the UART shall contain the description of how the
hardware works and how is interconnected, but not how is gets
configured in a particular system.  For passing configuration
information, device-tree has the "chosen" node, in which
information like the amount of available system memory or the
device to use as the system console can be provided.  The
configuration entries in the "chosen" node can contain a
reference to the hardware node they configure. A real-world
example from an ARM-SoC device-tree:

chosen {
       stdout-path = "serial0:115200n8";
};

This makes sense.

The settings used to describe a configuration of hardware may be in Flash/NVRAM whereas the hardware description may be in EEPROM/ROM.

However there is still the point about breaking up domain specific encodings of properties to avoid a proliferation of domain specific parsers. e.g.

chosen {

stdout-path = {

device “serial0”;

baud 115200;

data-bits 8;

parity “none”;

stop-bits 1;

};

};

[ron minnich]

I've continued to ruminate on this. My current feeling is that I don't want to put a DTS parser into harvey. I've got a working config string parser. Other future kernel developers might also share this opinion. DTS just looks clumsy and writing parsers for all its various sub-syntaxes makes no sense to me. DTS today is the result of 30 years or so of backwards-compatible evolution, and there are times when it pays to make a clean break.

I don't see "serial0:115200n8" as something I want a modern kernel to deal with. And as for the other DTS examples shown, just looking at them gives me a headache.

Given that the provision of an additional config string is allowed by the 1.9.1 spec, I think coreboot will provide both the DTS string and what, to me, is a more sane looking string conforming to the basic string config syntax.