Hello Trevor,
thanks for your patience waiting for me to come around looking at your
patch. I only found some trivial stuff to fix this time around.
On Wed, Apr 24, 2024 at 08:58:48AM -0400, Trevor Gamblin wrote:
> diff --git a/drivers/pwm/pwm-axi-pwmgen.c b/drivers/pwm/pwm-axi-pwmgen.c
> new file mode 100644
> index 000000000000..e0bf90cc2ba3
> --- /dev/null
> +++ b/drivers/pwm/pwm-axi-pwmgen.c
> @@ -0,0 +1,248 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Analog Devices AXI PWM generator
> + *
> + * Copyright 2024 Analog Devices Inc.
> + * Copyright 2024 Baylibre SAS
If there is public documentation available for the device, extra points
for a link to it here.
> [...]
> +static int axi_pwmgen_setup(struct regmap *regmap, struct device *dev)
> +{
> + int ret;
> + u32 val;
> +
> + ret = regmap_read(regmap, AXI_PWMGEN_REG_CORE_MAGIC, &val);
> + if (ret)
> + return ret;
> +
> + if (val != AXI_PWMGEN_REG_CORE_MAGIC_VAL)
> + return dev_err_probe(dev, -ENODEV,
> + "failed to read expected value from register: got %08x, expected %08x\n",
> + val,
> + AXI_PWMGEN_REG_CORE_MAGIC_VAL);
Join these two lines, please.
> + ret = regmap_read(regmap, AXI_PWMGEN_REG_CORE_VERSION, &val);
> + if (ret)
> + return ret;
> +
> + if (ADI_AXI_PCORE_VER_MAJOR(val) != 2) {
> + return dev_err_probe(dev, -ENODEV, "Unsupported peripheral version %u.%u.%u\n",
> + ADI_AXI_PCORE_VER_MAJOR(val),
> + ADI_AXI_PCORE_VER_MINOR(val),
> + ADI_AXI_PCORE_VER_PATCH(val));
> + }
> +
> + /* Enable the core */
> + ret = regmap_update_bits(regmap, AXI_PWMGEN_REG_CONFIG, AXI_PWMGEN_REG_CONFIG_RESET, 0);
> + if (ret)
> + return ret;
> +
> + ret = regmap_read(regmap, AXI_PWMGEN_REG_NPWM, &val);
> + if (ret)
> + return ret;
> +
> + /* Return the number of PWMs */
> + return val;
If the AXI_PWMGEN_REG_NPWM register contains a value >= 0x80000000, the
return value is considered a negative error code. Not sure how necessary
it is to check for this case. I'll leave that to you.
> +}
> [...]
> +static int axi_pwmgen_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct regmap *regmap;
> + struct pwm_chip *chip;
> + struct axi_pwmgen_ddata *ddata;
> + struct clk *clk;
> + void __iomem *io_base;
> + int ret;
> +
> + io_base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(io_base))
> + return PTR_ERR(io_base);
> +
> + regmap = devm_regmap_init_mmio(dev, io_base, &axi_pwmgen_regmap_config);
> + if (IS_ERR(regmap))
> + return dev_err_probe(dev, PTR_ERR(regmap),
> + "failed to init register map\n");
> +
> + ret = axi_pwmgen_setup(regmap, dev);
> + if (ret < 0)
> + return ret;
> +
> + chip = devm_pwmchip_alloc(dev, ret, sizeof(*ddata));
> + if (IS_ERR(chip))
> + return PTR_ERR(chip);
> + ddata = pwmchip_get_drvdata(chip);
> + ddata->regmap = regmap;
> +
> + clk = devm_clk_get_enabled(dev, NULL);
> + if (IS_ERR(clk))
> + return dev_err_probe(dev, PTR_ERR(clk), "failed to get clock\n");
> +
> + ret = devm_clk_rate_exclusive_get(dev, clk);
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to get exclusive rate\n");
> +
> + ret = devm_add_action_or_reset(dev, axi_pwmgen_clk_rate_exclusive_put, clk);
> + if (ret)
> + return ret;
this should be dropped as devm_clk_rate_exclusive_get() already takes
care for calling clk_rate_exclusive_put().
> + ddata->clk_rate_hz = clk_get_rate(clk);
> + if (!ddata->clk_rate_hz || ddata->clk_rate_hz > NSEC_PER_SEC)
> + return dev_err_probe(dev, -EINVAL,
> + "Invalid clock rate: %lu\n", ddata->clk_rate_hz);
> +
> + chip->ops = &axi_pwmgen_pwm_ops;
> + chip->atomic = true;
> +
> + ret = devm_pwmchip_add(dev, chip);
> + if (ret)
> + return dev_err_probe(dev, ret, "could not add PWM chip\n");
> +
> + return 0;
> +}
Best regards
Uwe
Attachment:
signature.asc
Description: PGP signature
