Re: [PATCH v7 2/2] clk: Add ccf driver for Renesas 8T49N241

From: Geert Uytterhoeven
Date: Thu Oct 21 2021 - 03:38:27 EST

Next message: Tao Zhang: "[PATCH 00/10] Add support for TPDM and TPDA"
Previous message: Manivannan Sadhasivam: "Re: [PATCH v1] PCI: qcom: Fix incorrect register offset in pcie init"
In reply to: Alex Helms: "[PATCH v7 2/2] clk: Add ccf driver for Renesas 8T49N241"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Hi Alex,

On Wed, Oct 20, 2021 at 8:10 PM Alex Helms
<alexander.helms.jy@xxxxxxxxxxx> wrote:
> This is a common clock framework driver that supports the 8T49N241 chip.
> No other chips in the family are currently supported. The driver
> supports setting the rate for all four outputs on the chip and
> automatically calculating/setting the appropriate VCO value.
>
> The driver can read a full register map from the device tree
> and will use that register map to initialize the attached device
> (via I2C) when the system boots. Any configuration not supported by
> the common clock framework must be done via the full register map,
> including optimized settings.
>
> All outputs are currently assumed to be LVDS unless overridden in
> the full register map in the DT.
>
> Signed-off-by: Alex Helms <alexander.helms.jy@xxxxxxxxxxx>
> Acked-by: Michal Simek <michal.simek@xxxxxxxxxx>

Thanks for your patch!

> --- /dev/null
> +++ b/drivers/clk/8t49n24x-core.c

> +/**
> + * __renesas_bits_to_shift - num bits to shift given specified mask
> + * @mask: 32-bit word input to count zero bits on right
> + *
> + * Given a bit mask indicating where a value will be stored in
> + * a register, return the number of bits you need to shift the value
> + * before ORing it into the register value.
> + *
> + * Return: number of bits to shift
> + */
> +int __renesas_bits_to_shift(unsigned int mask)
> +{
> + if (mask) {
> + // ffs considers the first bit position 1, we need an index
> + return ffs(mask) - 1;

ffs(x) - 1 == __ffs(x), isn't it?

> + } else {
> + return 0;
> + }
> +}
> +
> +int __renesas_i2c_write_bulk(struct i2c_client *client, struct regmap *map,
> + unsigned int reg, u8 val[], size_t val_count)
> +{
> + u8 block[WRITE_BLOCK_SIZE];
> + unsigned int block_offset = reg;
> + unsigned int i, err, currentOffset = 0;

int err, as regmap_bulk_write() returns negative error codes.

> +
> + dev_dbg(&client->dev,
> + "I2C->0x%04x : [hex] . First byte: %02x, Second byte: %02x",
> + reg, reg >> 8, reg & 0xFF);
> +
> + print_hex_dump_debug("i2c_write_bulk: ", DUMP_PREFIX_NONE,
> + 16, 1, val, val_count, false);
> +
> + for (i = 0; i < val_count; i++) {
> + block[currentOffset++] = val[i];
> +
> + if (i > 0 && (i + 1) % WRITE_BLOCK_SIZE == 0) {
> + err = regmap_bulk_write(map, block_offset, block, WRITE_BLOCK_SIZE);
> + if (err)
> + break;
> + block_offset += WRITE_BLOCK_SIZE;
> + currentOffset = 0;
> + }
> + }
> +
> + if (err == 0 && currentOffset > 0)
> + err = regmap_bulk_write(map, block_offset, block, currentOffset);
> +
> + return err;
> +}
> +
> +static int __i2c_write(struct i2c_client *client, struct regmap *map,
> + unsigned int reg, unsigned int val)
> +{
> + dev_dbg(&client->dev, "I2C->0x%x : [hex] %x", reg, val);
> + return regmap_write(map, reg, val);
> +}
> +
> +static int __i2c_write_with_mask(struct i2c_client *client, struct regmap *map,
> + unsigned int reg, u8 val, u8 original, u8 mask)
> +{
> + return __i2c_write(client, map, reg,
> + ((val << __renesas_bits_to_shift(mask)) & mask) | (original & ~mask));
> +}
> +
> +void r8t49n24x_get_offsets(u8 output_num, struct clk_register_offsets *offsets)
> +{
> + offsets->oe_offset = 0;
> + offsets->oe_mask = 0;
> + offsets->dis_mask = 0;
> + offsets->n_17_16_offset = 0;
> + offsets->n_17_16_mask = 0;
> + offsets->n_15_8_offset = 0;
> + offsets->n_7_0_offset = 0;
> + offsets->nfrac_27_24_offset = 0;
> + offsets->nfrac_27_24_mask = 0;
> + offsets->nfrac_23_16_offset = 0;
> + offsets->nfrac_15_8_offset = 0;
> + offsets->nfrac_7_0_offset = 0;
> +
> + switch (output_num) {
> + case 0:
> + offsets->oe_offset = R8T49N24X_REG_OUTEN;
> + offsets->oe_mask = R8T49N24X_REG_OUTEN0_MASK;
> + offsets->dis_mask = R8T49N24X_REG_Q0_DIS_MASK;
> + break;
> + case 1:
> + offsets->oe_offset = R8T49N24X_REG_OUTEN;
> + offsets->oe_mask = R8T49N24X_REG_OUTEN1_MASK;
> + offsets->dis_mask = R8T49N24X_REG_Q1_DIS_MASK;
> + offsets->n_17_16_offset = R8T49N24X_REG_N_Q1_17_16;
> + offsets->n_17_16_mask = R8T49N24X_REG_N_Q1_17_16_MASK;
> + offsets->n_15_8_offset = R8T49N24X_REG_N_Q1_15_8;
> + offsets->n_7_0_offset = R8T49N24X_REG_N_Q1_7_0;
> + offsets->nfrac_27_24_offset = R8T49N24X_REG_NFRAC_Q1_27_24;
> + offsets->nfrac_27_24_mask = R8T49N24X_REG_NFRAC_Q1_27_24_MASK;
> + offsets->nfrac_23_16_offset = R8T49N24X_REG_NFRAC_Q1_23_16;
> + offsets->nfrac_15_8_offset = R8T49N24X_REG_NFRAC_Q1_15_8;
> + offsets->nfrac_7_0_offset = R8T49N24X_REG_NFRAC_Q1_7_0;
> + break;
> + case 2:
> + offsets->oe_offset = R8T49N24X_REG_OUTEN;
> + offsets->oe_mask = R8T49N24X_REG_OUTEN2_MASK;
> + offsets->dis_mask = R8T49N24X_REG_Q2_DIS_MASK;
> + offsets->n_17_16_offset = R8T49N24X_REG_N_Q2_17_16;
> + offsets->n_17_16_mask = R8T49N24X_REG_N_Q2_17_16_MASK;
> + offsets->n_15_8_offset = R8T49N24X_REG_N_Q2_15_8;
> + offsets->n_7_0_offset = R8T49N24X_REG_N_Q2_7_0;
> + offsets->nfrac_27_24_offset = R8T49N24X_REG_NFRAC_Q2_27_24;
> + offsets->nfrac_27_24_mask = R8T49N24X_REG_NFRAC_Q2_27_24_MASK;
> + offsets->nfrac_23_16_offset = R8T49N24X_REG_NFRAC_Q2_23_16;
> + offsets->nfrac_15_8_offset = R8T49N24X_REG_NFRAC_Q2_15_8;
> + offsets->nfrac_7_0_offset = R8T49N24X_REG_NFRAC_Q2_7_0;
> + break;
> + case 3:
> + offsets->oe_offset = R8T49N24X_REG_OUTEN;
> + offsets->oe_mask = R8T49N24X_REG_OUTEN3_MASK;
> + offsets->dis_mask = R8T49N24X_REG_Q3_DIS_MASK;
> + offsets->n_17_16_offset = R8T49N24X_REG_N_Q3_17_16;
> + offsets->n_17_16_mask = R8T49N24X_REG_N_Q3_17_16_MASK;
> + offsets->n_15_8_offset = R8T49N24X_REG_N_Q3_15_8;
> + offsets->n_7_0_offset = R8T49N24X_REG_N_Q3_7_0;
> + offsets->nfrac_27_24_offset = R8T49N24X_REG_NFRAC_Q3_27_24;
> + offsets->nfrac_27_24_mask = R8T49N24X_REG_NFRAC_Q3_27_24_MASK;
> + offsets->nfrac_23_16_offset = R8T49N24X_REG_NFRAC_Q3_23_16;
> + offsets->nfrac_15_8_offset = R8T49N24X_REG_NFRAC_Q3_15_8;
> + offsets->nfrac_7_0_offset = R8T49N24X_REG_NFRAC_Q3_7_0;
> + break;
> + }
> +}
> +
> +/**
> + * r8t49n24x_calc_div_q0 - Calculate dividers and VCO freq to generate
> + * the specified Q0 frequency.
> + * @chip: Device data structure. contains all requested frequencies
> + * for all outputs.
> + *
> + * The actual output divider is ns1 * ns2 * 2. fOutput = fVCO / (ns1 * ns2 * 2)
> + *
> + * The options for ns1 (when the source is the VCO) are 4,5,6. ns2 is a
> + * 16-bit value.
> + *
> + * chip->divs: structure for specifying ns1/ns2 values. If 0 after this
> + * function, Q0 is not requested
> + */
> +static void r8t49n24x_calc_div_q0(struct clk_r8t49n24x_chip *chip)
> +{
> + unsigned int i;
> + unsigned int min_div = 0, max_div = 0, best_vco = 0;
> + unsigned int min_ns2 = 0, max_ns2 = 0;
> + bool is_lower_vco = false;
> +
> + chip->divs.ns1_q0 = 0;
> + chip->divs.ns2_q0 = 0;
> +
> + if (chip->clk[0].requested == 0)
> + return;
> +
> + min_div = (R8T49N24X_VCO_MIN / (chip->clk[0].requested * 2)) * 2;

To avoid overflow, you may want to write this as:

min_div = (R8T49N24X_VCO_MIN / 2 / chip->clk[0].requested * 2) * 2;

> + max_div = (R8T49N24X_VCO_MAX / (chip->clk[0].requested * 2)) * 2;

Likewise.

> +/**
> + * r8t49n24x_calc_divs - Calculate dividers to generate the specified frequency.
> + * @chip: Device data structure. contains all requested frequencies
> + * for all outputs.
> + *
> + * Calculate the clock dividers (dsmint, dsmfrac for vco; ns1/ns2 for q0,
> + * n/nfrac for q1-3) for a given target frequency.
> + *
> + * Return: 0 on success, negative errno otherwise.
> + */
> +static int r8t49n24x_calc_divs(struct clk_r8t49n24x_chip *chip)
> +{
> + unsigned int i;
> + unsigned int vco = 0;
> + unsigned int pfd = 0;
> + u64 rem = 0;
> +
> + r8t49n24x_calc_div_q0(chip);
> +
> + dev_dbg(&chip->i2c_client->dev,
> + "after r8t49n24x_calc_div_q0. ns1: %u [/%u], ns2: %u",
> + chip->divs.ns1_q0, q0_ns1_options[chip->divs.ns1_q0],
> + chip->divs.ns2_q0);
> +
> + chip->divs.dsmint = 0;
> + chip->divs.dsmfrac = 0;
> +
> + if (chip->clk[0].requested > 0) {
> + /* Q0 is in use and is governing the actual VCO freq */
> + vco = q0_ns1_options[chip->divs.ns1_q0] * chip->divs.ns2_q0
> + * 2 * chip->clk[0].requested;
> + } else {
> + unsigned int freq = 0;
> + unsigned int min_div = 0, max_div = 0;
> + unsigned int i = 0;
> + bool is_lower_vco = false;
> +
> + /*
> + * Q0 is not in use. Use the first requested (fractional)
> + * output frequency as the one controlling the VCO.
> + */
> + for (i = 1; i < NUM_OUTPUTS; i++) {
> + if (chip->clk[i].requested != 0) {
> + freq = chip->clk[i].requested;
> + break;
> + }
> + }
> +
> + if (!freq) {
> + dev_err(&chip->i2c_client->dev, "NO FREQUENCIES SPECIFIED");
> + return -EINVAL;
> + }
> +
> + /*
> + * First, determine the min/max div for the output frequency.
> + */
> + min_div = R8T49N24X_MIN_INT_DIVIDER;
> + max_div = (R8T49N24X_VCO_MAX / (freq * 2)) * 2;

(R8T49N24X_VCO_MAX / 2 / freq) * 2

> +
> + dev_dbg(&chip->i2c_client->dev,
> + "calc_divs for fractional output. freq: %u, min_div: %u, max_div: %u",
> + freq, min_div, max_div);
> +
> + i = min_div;
> +
> + while (i <= max_div) {
> + unsigned int current_vco = freq * i;
> +
> + dev_dbg(&chip->i2c_client->dev,
> + "calc_divs for fractional output. walk: %u, freq: %u, vco: %u",
> + i, freq, vco);
> +
> + if (current_vco >= R8T49N24X_VCO_MIN &&
> + vco <= R8T49N24X_VCO_MAX) {
> + if (current_vco <= R8T49N24X_VCO_OPT) {
> + if (current_vco > vco || !is_lower_vco) {
> + is_lower_vco = true;
> + vco = current_vco;
> + }
> + } else if (!is_lower_vco && current_vco > vco) {
> + vco = current_vco;
> + }
> + }
> + /* Divider must be even. */
> + i += 2;
> + }
> + }
> +
> + if (!vco) {
> + dev_err(&chip->i2c_client->dev, "no integer divider in range found. NOT SUPPORTED.");
> + return -EINVAL;
> + }
> +
> + /* Setup dividers for outputs with fractional dividers. */
> + for (i = 1; i < NUM_OUTPUTS; i++) {
> + if (chip->clk[i].requested) {
> + /*
> + * The value written to the chip is half
> + * the calculated divider.
> + */
> + chip->divs.nint[i - 1] = div64_u64_rem((u64)vco,
> + chip->clk[i].requested * 2,
> + &rem);

vco is 32-bit, so no need for an expensive 64-by-64 remainder
calculation.

> + chip->divs.nfrac[i - 1] = div64_u64(rem << 28,
> + chip->clk[i].requested * 2);

chip->clk[i].requested is unsigned long, so div64_ul() will do.

> +
> + dev_dbg(&chip->i2c_client->dev,
> + "div to get Q%i freq %lu from vco %u: int part: %u, rem: %llu, frac part: %u",
> + i, chip->clk[i].requested,
> + vco, chip->divs.nint[i - 1], rem,
> + chip->divs.nfrac[i - 1]);
> + }
> + }
> +
> + /* Calculate freq for pfd */
> + pfd = chip->input_clk_freq * (chip->doubler_disabled ? 1 : 2);
> +
> + /*
> + * Calculate dsmint & dsmfrac:
> + * -----------------------------
> + * dsm = float(vco)/float(pfd)
> + * dsmfrac = dsm-floor(dsm) * 2^21
> + * rem = vco % pfd
> + * therefore:
> + * dsmfrac = (rem * 2^21)/pfd
> + */
> + chip->divs.dsmint = div64_u64_rem(vco, pfd, &rem);

vco and pfd are both 32-bit.

> + chip->divs.dsmfrac = div64_u64(rem << 21, pfd);

div64_ul().

> +
> + dev_dbg(&chip->i2c_client->dev,
> + "vco: %u, pfd: %u, dsmint: %u, dsmfrac: %u, rem: %llu",
> + vco, pfd, chip->divs.dsmint,
> + chip->divs.dsmfrac, rem);
> +
> + return 0;
> +}

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

Next message: Tao Zhang: "[PATCH 00/10] Add support for TPDM and TPDA"
Previous message: Manivannan Sadhasivam: "Re: [PATCH v1] PCI: qcom: Fix incorrect register offset in pcie init"
In reply to: Alex Helms: "[PATCH v7 2/2] clk: Add ccf driver for Renesas 8T49N241"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]