[Qemu-devel] [PATCH v6 9/9] hw/arm/xilinx_zynq: connect uart clocks to slcr

Damien Hedde posted 9 patches 6 years, 5 months ago
There is a newer version of this series
[Qemu-devel] [PATCH v6 9/9] hw/arm/xilinx_zynq: connect uart clocks to slcr
Posted by Damien Hedde 6 years, 5 months ago
Add the connection between the slcr's output clocks and the uarts inputs.

Also add the main board clock 'ps_clk', which is hard-coded to 33.33MHz
(the default frequency). This clock is used to feed the slcr's input
clock.

Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
 hw/arm/xilinx_zynq.c | 64 ++++++++++++++++++++++++++++++++++++++------
 1 file changed, 56 insertions(+), 8 deletions(-)

diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index 89da34808b..4d5d214ea9 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -33,6 +33,15 @@
 #include "hw/char/cadence_uart.h"
 #include "hw/net/cadence_gem.h"
 #include "hw/cpu/a9mpcore.h"
+#include "hw/qdev-clock.h"
+#include "sysemu/reset.h"
+
+#define TYPE_ZYNQ_MACHINE MACHINE_TYPE_NAME("xilinx-zynq-a9")
+#define ZYNQ_MACHINE(obj) \
+    OBJECT_CHECK(ZynqMachineState, (obj), TYPE_ZYNQ_MACHINE)
+
+/* board base frequency: 33.333333 MHz */
+#define PS_CLK_FREQUENCY (100 * 1000 * 1000 / 3)
 
 #define NUM_SPI_FLASHES 4
 #define NUM_QSPI_FLASHES 2
@@ -73,6 +82,11 @@ static const int dma_irqs[8] = {
     0xe3401000 + ARMV7_IMM16(extract32((val), 16, 16)), /* movt r1 ... */ \
     0xe5801000 + (addr)
 
+typedef struct ZynqMachineState {
+    MachineState parent;
+    ClockOut *ps_clk;
+} ZynqMachineState;
+
 static void zynq_write_board_setup(ARMCPU *cpu,
                                    const struct arm_boot_info *info)
 {
@@ -157,6 +171,7 @@ static inline void zynq_init_spi_flashes(uint32_t base_addr, qemu_irq irq,
 
 static void zynq_init(MachineState *machine)
 {
+    ZynqMachineState *zynq_machine = ZYNQ_MACHINE(machine);
     ram_addr_t ram_size = machine->ram_size;
     const char *kernel_filename = machine->kernel_filename;
     const char *kernel_cmdline = machine->kernel_cmdline;
@@ -165,7 +180,7 @@ static void zynq_init(MachineState *machine)
     MemoryRegion *address_space_mem = get_system_memory();
     MemoryRegion *ext_ram = g_new(MemoryRegion, 1);
     MemoryRegion *ocm_ram = g_new(MemoryRegion, 1);
-    DeviceState *dev;
+    DeviceState *dev, *slcr;
     SysBusDevice *busdev;
     qemu_irq pic[64];
     int n;
@@ -210,9 +225,17 @@ static void zynq_init(MachineState *machine)
                           1, 0x0066, 0x0022, 0x0000, 0x0000, 0x0555, 0x2aa,
                           0);
 
-    dev = qdev_create(NULL, "xilinx,zynq_slcr");
-    qdev_init_nofail(dev);
-    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xF8000000);
+    /* Create slcr, keep a pointer to connect clocks */
+    slcr = qdev_create(NULL, "xilinx,zynq_slcr");
+    qdev_init_nofail(slcr);
+    sysbus_mmio_map(SYS_BUS_DEVICE(slcr), 0, 0xF8000000);
+
+    /* Create the main clock source, and feed slcr with it */
+    zynq_machine->ps_clk = CLOCK_OUT(object_new(TYPE_CLOCK_OUT));
+    object_property_add_child(OBJECT(zynq_machine), "ps_clk",
+                              OBJECT(zynq_machine->ps_clk), &error_abort);
+    object_unref(OBJECT(zynq_machine->ps_clk));
+    clock_connect(qdev_get_clock_in(slcr, "ps_clk"), zynq_machine->ps_clk);
 
     dev = qdev_create(NULL, TYPE_A9MPCORE_PRIV);
     qdev_prop_set_uint32(dev, "num-cpu", 1);
@@ -233,8 +256,12 @@ static void zynq_init(MachineState *machine)
     sysbus_create_simple("xlnx,ps7-usb", 0xE0002000, pic[53-IRQ_OFFSET]);
     sysbus_create_simple("xlnx,ps7-usb", 0xE0003000, pic[76-IRQ_OFFSET]);
 
-    cadence_uart_create(0xE0000000, pic[59 - IRQ_OFFSET], serial_hd(0));
-    cadence_uart_create(0xE0001000, pic[82 - IRQ_OFFSET], serial_hd(1));
+    dev = cadence_uart_create(0xE0000000, pic[59 - IRQ_OFFSET], serial_hd(0));
+    qdev_connect_clock_out(slcr, "uart0_ref_clk",
+                           qdev_get_clock_in(dev, "refclk"), &error_abort);
+    dev = cadence_uart_create(0xE0001000, pic[82 - IRQ_OFFSET], serial_hd(1));
+    qdev_connect_clock_out(slcr, "uart1_ref_clk",
+                           qdev_get_clock_in(dev, "refclk"), &error_abort);
 
     sysbus_create_varargs("cadence_ttc", 0xF8001000,
             pic[42-IRQ_OFFSET], pic[43-IRQ_OFFSET], pic[44-IRQ_OFFSET], NULL);
@@ -315,14 +342,35 @@ static void zynq_init(MachineState *machine)
     arm_load_kernel(ARM_CPU(first_cpu), &zynq_binfo);
 }
 
-static void zynq_machine_init(MachineClass *mc)
+static void zynq_reset(MachineState *machine)
+{
+    ZynqMachineState *zynq_machine = ZYNQ_MACHINE(machine);
+    qemu_devices_reset();
+    clock_set_frequency(zynq_machine->ps_clk, PS_CLK_FREQUENCY);
+}
+
+static void zynq_machine_class_init(ObjectClass *oc, void *data)
 {
+    MachineClass *mc = MACHINE_CLASS(oc);
     mc->desc = "Xilinx Zynq Platform Baseboard for Cortex-A9";
     mc->init = zynq_init;
+    mc->reset = zynq_reset;
     mc->max_cpus = 1;
     mc->no_sdcard = 1;
     mc->ignore_memory_transaction_failures = true;
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a9");
 }
 
-DEFINE_MACHINE("xilinx-zynq-a9", zynq_machine_init)
+static const TypeInfo zynq_machine_type = {
+    .name = TYPE_ZYNQ_MACHINE,
+    .parent = TYPE_MACHINE,
+    .class_init = zynq_machine_class_init,
+    .instance_size = sizeof(ZynqMachineState),
+};
+
+static void zynq_machine_register_types(void)
+{
+    type_register_static(&zynq_machine_type);
+}
+
+type_init(zynq_machine_register_types)
-- 
2.22.0


Re: [PATCH v6 9/9] hw/arm/xilinx_zynq: connect uart clocks to slcr
Posted by Peter Maydell 6 years, 2 months ago
On Wed, 4 Sep 2019 at 13:56, Damien Hedde <damien.hedde@greensocs.com> wrote:
>
> Add the connection between the slcr's output clocks and the uarts inputs.
>
> Also add the main board clock 'ps_clk', which is hard-coded to 33.33MHz
> (the default frequency). This clock is used to feed the slcr's input
> clock.
>
> Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>

Nothing obviously wrong in the body of the patch, but as with
7 and 8, review from a Xilinx person would be helpful.

/* board base frequency: 33.333333 MHz */
#define PS_CLK_FREQUENCY (100 * 1000 * 1000 / 3)

This is interesting, because it's not an integer... I'll come back
to this topic in a reply to the cover letter in a moment.

thanks
-- PMM

Re: [PATCH v6 9/9] hw/arm/xilinx_zynq: connect uart clocks to slcr
Posted by Damien Hedde 6 years, 2 months ago

On 12/2/19 4:34 PM, Peter Maydell wrote:
> On Wed, 4 Sep 2019 at 13:56, Damien Hedde <damien.hedde@greensocs.com> wrote:
>>
>> Add the connection between the slcr's output clocks and the uarts inputs.
>>
>> Also add the main board clock 'ps_clk', which is hard-coded to 33.33MHz
>> (the default frequency). This clock is used to feed the slcr's input
>> clock.
>>
>> Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
> 
> Nothing obviously wrong in the body of the patch, but as with
> 7 and 8, review from a Xilinx person would be helpful.
> 
> /* board base frequency: 33.333333 MHz */
> #define PS_CLK_FREQUENCY (100 * 1000 * 1000 / 3)
> 
> This is interesting, because it's not an integer... I'll come back
> to this topic in a reply to the cover letter in a moment.

For this precise case, what I wanted is the resulting integer which I
got from the device trees in linux (btw I should probably add this point
in  comment). Just thought it was more readable this way than "33333333".

--
Damien

Re: [PATCH v6 9/9] hw/arm/xilinx_zynq: connect uart clocks to slcr
Posted by Philippe Mathieu-Daudé 6 years, 2 months ago
On 12/3/19 3:59 PM, Damien Hedde wrote:
> On 12/2/19 4:34 PM, Peter Maydell wrote:
>> On Wed, 4 Sep 2019 at 13:56, Damien Hedde <damien.hedde@greensocs.com> wrote:
>>>
>>> Add the connection between the slcr's output clocks and the uarts inputs.
>>>
>>> Also add the main board clock 'ps_clk', which is hard-coded to 33.33MHz
>>> (the default frequency). This clock is used to feed the slcr's input
>>> clock.
>>>
>>> Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
>>
>> Nothing obviously wrong in the body of the patch, but as with
>> 7 and 8, review from a Xilinx person would be helpful.
>>
>> /* board base frequency: 33.333333 MHz */
>> #define PS_CLK_FREQUENCY (100 * 1000 * 1000 / 3)
>>
>> This is interesting, because it's not an integer... I'll come back
>> to this topic in a reply to the cover letter in a moment.
> 
> For this precise case, what I wanted is the resulting integer which I
> got from the device trees in linux (btw I should probably add this point
> in  comment). Just thought it was more readable this way than "33333333".

FWIW I'm auditing if it is possible to use the float type for 
frequencies (before to ask on the list if this makes sense), because in 
hw/core/ptimer we use timers with periods, and loose some precision 
using 1/freq again.

Also we have MiB/KiB in "qemu/units.h" and I'd like to introduce MHz/KHz.