diff mbox series

[v6,7/9] hw/misc/zynq_slcr: add clock generation for uarts

Message ID 20190904125531.27545-8-damien.hedde@greensocs.com (mailing list archive)
State New, archived
Headers show
Series Clock framework API | expand

Commit Message

Damien Hedde Sept. 4, 2019, 12:55 p.m. UTC 
Switch the slcr to multi-phase reset and add some clocks:
+ the main input clock (ps_clk)
+ the reference clock outputs for each uart (uart0 & 1)

The clock frequencies are computed using the internal pll & uart configuration
registers and the ps_clk frequency.

Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
 hw/misc/zynq_slcr.c | 145 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 142 insertions(+), 3 deletions(-)

Comments

Peter Maydell Dec. 2, 2019, 3:20 p.m. UTC | #1 
On Wed, 4 Sep 2019 at 13:56, Damien Hedde <damien.hedde@greensocs.com> wrote:
>
> Switch the slcr to multi-phase reset and add some clocks:
> + the main input clock (ps_clk)
> + the reference clock outputs for each uart (uart0 & 1)
>
> The clock frequencies are computed using the internal pll & uart configuration
> registers and the ps_clk frequency.
>
> Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>

Review of this and the following two patches by some Xilinx
person would be nice. I've just looked them over for general
issues, and haven't checked against the hardware specs.

> ---


> +/*
> + * return the output frequency of a clock given:
> + * + the frequencies in an array corresponding to mux's indexes
> + * + the register xxx_CLK_CTRL value
> + * + enable bit index in ctrl register
> + *
> + * This function make the assumption that ctrl_reg value is organized as follow:

"makes"; "that the"; "follows"

> + * + bits[13:8] clock divisor
> + * + bits[5:4]  clock mux selector (index in array)
> + * + bits[index] clock enable
> + */
> +static uint64_t zynq_slcr_compute_clock(const uint64_t mux[],
> +                                        uint32_t ctrl_reg,
> +                                        unsigned index)
> +{
> +    uint32_t srcsel = extract32(ctrl_reg, 4, 2); /* bits [5:4] */
> +    uint32_t divisor = extract32(ctrl_reg, 8, 6); /* bits [13:8] */
> +
> +    /* first, check if clock is enabled */
> +    if (((ctrl_reg >> index) & 1u) == 0) {
> +        return 0;
> +    }
> +
> +    /*
> +     * according to the Zynq technical ref. manual UG585 v1.12.2 in
> +     * "Clocks" chapter, section 25.10.1 page 705" the range of the divisor
> +     * is [1;63].

Is this the range notation the spec doc uses?

> +     * So divide the source while avoiding division-by-zero.
> +     */
> +    return mux[srcsel] / (divisor ? divisor : 1u);
> +}
> +

> +static const ClockPortInitArray zynq_slcr_clocks = {
> +    QDEV_CLOCK_IN(ZynqSLCRState, ps_clk, zynq_slcr_ps_clk_callback),
> +    QDEV_CLOCK_OUT(ZynqSLCRState, uart0_ref_clk),
> +    QDEV_CLOCK_OUT(ZynqSLCRState, uart1_ref_clk),
> +    QDEV_CLOCK_END
> +};
> +
>  static void zynq_slcr_init(Object *obj)
>  {
>      ZynqSLCRState *s = ZYNQ_SLCR(obj);
> @@ -425,6 +559,8 @@ static void zynq_slcr_init(Object *obj)
>      memory_region_init_io(&s->iomem, obj, &slcr_ops, s, "slcr",
>                            ZYNQ_SLCR_MMIO_SIZE);
>      sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->iomem);
> +
> +    qdev_init_clocks(DEVICE(obj), zynq_slcr_clocks);
>  }
>
>  static const VMStateDescription vmstate_zynq_slcr = {
> @@ -440,9 +576,12 @@ static const VMStateDescription vmstate_zynq_slcr = {
>  static void zynq_slcr_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
> +    ResettableClass *rc = RESETTABLE_CLASS(klass);
>
>      dc->vmsd = &vmstate_zynq_slcr;
> -    dc->reset = zynq_slcr_reset;
> +    rc->phases.init = zynq_slcr_reset_init;
> +    rc->phases.hold = zynq_slcr_reset_hold;
> +    rc->phases.exit = zynq_slcr_reset_exit;
>  }

We're adding an input clock, so doesn't the migration
state struct need to be updated to migrate it ?

thanks
-- PMM
Damien Hedde Dec. 4, 2019, 12:51 p.m. UTC | #2 
On 12/2/19 4:20 PM, Peter Maydell wrote:
> On Wed, 4 Sep 2019 at 13:56, Damien Hedde <damien.hedde@greensocs.com> wrote:
>>
>> Switch the slcr to multi-phase reset and add some clocks:
>> + the main input clock (ps_clk)
>> + the reference clock outputs for each uart (uart0 & 1)
>>
>> The clock frequencies are computed using the internal pll & uart configuration
>> registers and the ps_clk frequency.
>>
>> Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
> 
> Review of this and the following two patches by some Xilinx
> person would be nice. I've just looked them over for general
> issues, and haven't checked against the hardware specs.
> 
>> ---
> 
> 
>> +/*
>> + * return the output frequency of a clock given:
>> + * + the frequencies in an array corresponding to mux's indexes
>> + * + the register xxx_CLK_CTRL value
>> + * + enable bit index in ctrl register
>> + *
>> + * This function make the assumption that ctrl_reg value is organized as follow:
> 
> "makes"; "that the"; "follows"
> 
>> + * + bits[13:8] clock divisor
>> + * + bits[5:4]  clock mux selector (index in array)
>> + * + bits[index] clock enable
>> + */
>> +static uint64_t zynq_slcr_compute_clock(const uint64_t mux[],
>> +                                        uint32_t ctrl_reg,
>> +                                        unsigned index)
>> +{
>> +    uint32_t srcsel = extract32(ctrl_reg, 4, 2); /* bits [5:4] */
>> +    uint32_t divisor = extract32(ctrl_reg, 8, 6); /* bits [13:8] */
>> +
>> +    /* first, check if clock is enabled */
>> +    if (((ctrl_reg >> index) & 1u) == 0) {
>> +        return 0;
>> +    }
>> +
>> +    /*
>> +     * according to the Zynq technical ref. manual UG585 v1.12.2 in
>> +     * "Clocks" chapter, section 25.10.1 page 705" the range of the divisor
>> +     * is [1;63].
> 
> Is this the range notation the spec doc uses?

The exact terms is:
"The 6-bit divider provides a divide range of 1 to 63"
At the time, I checked also the kernel sources, and this is the behavior
implemented in the driver as well (1 based timer and allowing 0 special
value for bypass). The bypass is undocumented as far as I can tell.

> 
>> +     * So divide the source while avoiding division-by-zero.
>> +     */
>> +    return mux[srcsel] / (divisor ? divisor : 1u);
>> +}
>> +
> 
>> +static const ClockPortInitArray zynq_slcr_clocks = {
>> +    QDEV_CLOCK_IN(ZynqSLCRState, ps_clk, zynq_slcr_ps_clk_callback),
>> +    QDEV_CLOCK_OUT(ZynqSLCRState, uart0_ref_clk),
>> +    QDEV_CLOCK_OUT(ZynqSLCRState, uart1_ref_clk),
>> +    QDEV_CLOCK_END
>> +};
>> +
>>  static void zynq_slcr_init(Object *obj)
>>  {
>>      ZynqSLCRState *s = ZYNQ_SLCR(obj);
>> @@ -425,6 +559,8 @@ static void zynq_slcr_init(Object *obj)
>>      memory_region_init_io(&s->iomem, obj, &slcr_ops, s, "slcr",
>>                            ZYNQ_SLCR_MMIO_SIZE);
>>      sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->iomem);
>> +
>> +    qdev_init_clocks(DEVICE(obj), zynq_slcr_clocks);
>>  }
>>
>>  static const VMStateDescription vmstate_zynq_slcr = {
>> @@ -440,9 +576,12 @@ static const VMStateDescription vmstate_zynq_slcr = {
>>  static void zynq_slcr_class_init(ObjectClass *klass, void *data)
>>  {
>>      DeviceClass *dc = DEVICE_CLASS(klass);
>> +    ResettableClass *rc = RESETTABLE_CLASS(klass);
>>
>>      dc->vmsd = &vmstate_zynq_slcr;
>> -    dc->reset = zynq_slcr_reset;
>> +    rc->phases.init = zynq_slcr_reset_init;
>> +    rc->phases.hold = zynq_slcr_reset_hold;
>> +    rc->phases.exit = zynq_slcr_reset_exit;
>>  }
> 
> We're adding an input clock, so doesn't the migration
> state struct need to be updated to migrate it ?
Yes, we can. Although this input clock is really not expected to change.

> 
> thanks
> -- PMM
>
diff mbox series

Patch

diff --git a/hw/misc/zynq_slcr.c b/hw/misc/zynq_slcr.c
index b9a38272d9..fa2ac608eb 100644
--- a/hw/misc/zynq_slcr.c
+++ b/hw/misc/zynq_slcr.c
@@ -22,6 +22,7 @@ 
 #include "qemu/log.h"
 #include "qemu/module.h"
 #include "hw/registerfields.h"
+#include "hw/qdev-clock.h"
 
 #ifndef ZYNQ_SLCR_ERR_DEBUG
 #define ZYNQ_SLCR_ERR_DEBUG 0
@@ -45,6 +46,12 @@  REG32(LOCKSTA, 0x00c)
 REG32(ARM_PLL_CTRL, 0x100)
 REG32(DDR_PLL_CTRL, 0x104)
 REG32(IO_PLL_CTRL, 0x108)
+/* fields for [ARM|DDR|IO]_PLL_CTRL registers */
+    FIELD(xxx_PLL_CTRL, PLL_RESET, 0, 1)
+    FIELD(xxx_PLL_CTRL, PLL_PWRDWN, 1, 1)
+    FIELD(xxx_PLL_CTRL, PLL_BYPASS_QUAL, 3, 1)
+    FIELD(xxx_PLL_CTRL, PLL_BYPASS_FORCE, 4, 1)
+    FIELD(xxx_PLL_CTRL, PLL_FPDIV, 12, 7)
 REG32(PLL_STATUS, 0x10c)
 REG32(ARM_PLL_CFG, 0x110)
 REG32(DDR_PLL_CFG, 0x114)
@@ -64,6 +71,10 @@  REG32(SMC_CLK_CTRL, 0x148)
 REG32(LQSPI_CLK_CTRL, 0x14c)
 REG32(SDIO_CLK_CTRL, 0x150)
 REG32(UART_CLK_CTRL, 0x154)
+    FIELD(UART_CLK_CTRL, CLKACT0, 0, 1)
+    FIELD(UART_CLK_CTRL, CLKACT1, 1, 1)
+    FIELD(UART_CLK_CTRL, SRCSEL,  4, 2)
+    FIELD(UART_CLK_CTRL, DIVISOR, 8, 6)
 REG32(SPI_CLK_CTRL, 0x158)
 REG32(CAN_CLK_CTRL, 0x15c)
 REG32(CAN_MIOCLK_CTRL, 0x160)
@@ -179,11 +190,106 @@  typedef struct ZynqSLCRState {
     MemoryRegion iomem;
 
     uint32_t regs[ZYNQ_SLCR_NUM_REGS];
+
+    ClockIn *ps_clk;
+    ClockOut *uart0_ref_clk;
+    ClockOut *uart1_ref_clk;
 } ZynqSLCRState;
 
-static void zynq_slcr_reset(DeviceState *d)
+/*
+ * return the output frequency of ARM/DDR/IO pll
+ * using input frequency and PLL_CTRL register
+ */
+static uint64_t zynq_slcr_compute_pll(uint64_t input, uint32_t ctrl_reg)
+{
+    uint32_t mult = ((ctrl_reg & R_xxx_PLL_CTRL_PLL_FPDIV_MASK) >>
+            R_xxx_PLL_CTRL_PLL_FPDIV_SHIFT);
+
+    /* first, check if pll is bypassed */
+    if (ctrl_reg & R_xxx_PLL_CTRL_PLL_BYPASS_FORCE_MASK) {
+        return input;
+    }
+
+    /* is pll disabled ? */
+    if (ctrl_reg & (R_xxx_PLL_CTRL_PLL_RESET_MASK |
+                    R_xxx_PLL_CTRL_PLL_PWRDWN_MASK)) {
+        return 0;
+    }
+
+    return input * mult;
+}
+
+/*
+ * return the output frequency of a clock given:
+ * + the frequencies in an array corresponding to mux's indexes
+ * + the register xxx_CLK_CTRL value
+ * + enable bit index in ctrl register
+ *
+ * This function make the assumption that ctrl_reg value is organized as follow:
+ * + bits[13:8] clock divisor
+ * + bits[5:4]  clock mux selector (index in array)
+ * + bits[index] clock enable
+ */
+static uint64_t zynq_slcr_compute_clock(const uint64_t mux[],
+                                        uint32_t ctrl_reg,
+                                        unsigned index)
+{
+    uint32_t srcsel = extract32(ctrl_reg, 4, 2); /* bits [5:4] */
+    uint32_t divisor = extract32(ctrl_reg, 8, 6); /* bits [13:8] */
+
+    /* first, check if clock is enabled */
+    if (((ctrl_reg >> index) & 1u) == 0) {
+        return 0;
+    }
+
+    /*
+     * according to the Zynq technical ref. manual UG585 v1.12.2 in
+     * "Clocks" chapter, section 25.10.1 page 705" the range of the divisor
+     * is [1;63].
+     * So divide the source while avoiding division-by-zero.
+     */
+    return mux[srcsel] / (divisor ? divisor : 1u);
+}
+
+/*
+ * macro helper around zynq_slcr_compute_clock to avoid repeating
+ * the register name.
+ */
+#define ZYNQ_COMPUTE_CLOCK(_state, _plls, _reg, _enable_field) \
+    zynq_slcr_compute_clock((_plls), (_state)->regs[R_ ## _reg], \
+            R_ ## _reg ## _ ## _enable_field ## _SHIFT)
+
+static void zynq_slcr_compute_clocks(ZynqSLCRState *s)
+{
+    uint64_t ps_clk = clock_get_frequency(s->ps_clk);
+
+    /* consider all output clocks are disabled while in reset */
+    if (device_is_resetting(DEVICE(s))) {
+        ps_clk = 0;
+    }
+
+    uint64_t io_pll = zynq_slcr_compute_pll(ps_clk, s->regs[R_IO_PLL_CTRL]);
+    uint64_t arm_pll = zynq_slcr_compute_pll(ps_clk, s->regs[R_ARM_PLL_CTRL]);
+    uint64_t ddr_pll = zynq_slcr_compute_pll(ps_clk, s->regs[R_DDR_PLL_CTRL]);
+
+    uint64_t uart_mux[4] = {io_pll, io_pll, arm_pll, ddr_pll};
+
+    /* compute uartX reference clocks */
+    clock_set_frequency(s->uart0_ref_clk,
+            ZYNQ_COMPUTE_CLOCK(s, uart_mux, UART_CLK_CTRL, CLKACT0));
+    clock_set_frequency(s->uart1_ref_clk,
+            ZYNQ_COMPUTE_CLOCK(s, uart_mux, UART_CLK_CTRL, CLKACT1));
+}
+
+static void zynq_slcr_ps_clk_callback(void *opaque)
+{
+    ZynqSLCRState *s = (ZynqSLCRState *) opaque;
+    zynq_slcr_compute_clocks(s);
+}
+
+static void zynq_slcr_reset_init(Object *obj, ResetType type)
 {
-    ZynqSLCRState *s = ZYNQ_SLCR(d);
+    ZynqSLCRState *s = ZYNQ_SLCR(obj);
     int i;
 
     DB_PRINT("RESET\n");
@@ -277,6 +383,21 @@  static void zynq_slcr_reset(DeviceState *d)
     s->regs[R_DDRIOB + 12] = 0x00000021;
 }
 
+static void zynq_slcr_reset_hold(Object *obj)
+{
+    ZynqSLCRState *s = ZYNQ_SLCR(obj);
+
+    /* will disable all output clocks */
+    zynq_slcr_compute_clocks(s);
+}
+
+static void zynq_slcr_reset_exit(Object *obj)
+{
+    ZynqSLCRState *s = ZYNQ_SLCR(obj);
+
+    /* will compute output clocks according to ps_clk and registers */
+    zynq_slcr_compute_clocks(s);
+}
 
 static bool zynq_slcr_check_offset(hwaddr offset, bool rnw)
 {
@@ -409,6 +530,12 @@  static void zynq_slcr_write(void *opaque, hwaddr offset,
             qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
         }
         break;
+    case R_IO_PLL_CTRL:
+    case R_ARM_PLL_CTRL:
+    case R_DDR_PLL_CTRL:
+    case R_UART_CLK_CTRL:
+        zynq_slcr_compute_clocks(s);
+        break;
     }
 }
 
@@ -418,6 +545,13 @@  static const MemoryRegionOps slcr_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
+static const ClockPortInitArray zynq_slcr_clocks = {
+    QDEV_CLOCK_IN(ZynqSLCRState, ps_clk, zynq_slcr_ps_clk_callback),
+    QDEV_CLOCK_OUT(ZynqSLCRState, uart0_ref_clk),
+    QDEV_CLOCK_OUT(ZynqSLCRState, uart1_ref_clk),
+    QDEV_CLOCK_END
+};
+
 static void zynq_slcr_init(Object *obj)
 {
     ZynqSLCRState *s = ZYNQ_SLCR(obj);
@@ -425,6 +559,8 @@  static void zynq_slcr_init(Object *obj)
     memory_region_init_io(&s->iomem, obj, &slcr_ops, s, "slcr",
                           ZYNQ_SLCR_MMIO_SIZE);
     sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->iomem);
+
+    qdev_init_clocks(DEVICE(obj), zynq_slcr_clocks);
 }
 
 static const VMStateDescription vmstate_zynq_slcr = {
@@ -440,9 +576,12 @@  static const VMStateDescription vmstate_zynq_slcr = {
 static void zynq_slcr_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
+    ResettableClass *rc = RESETTABLE_CLASS(klass);
 
     dc->vmsd = &vmstate_zynq_slcr;
-    dc->reset = zynq_slcr_reset;
+    rc->phases.init = zynq_slcr_reset_init;
+    rc->phases.hold = zynq_slcr_reset_hold;
+    rc->phases.exit = zynq_slcr_reset_exit;
 }
 
 static const TypeInfo zynq_slcr_info = {