Re: [PATCH v8 2/2] pwm: Add Loongson PWM controller support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello Binbin,

On Tue, Dec 10, 2024 at 08:37:06PM +0800, Binbin Zhou wrote:
> +static int pwm_loongson_probe(struct platform_device *pdev)
> +{
> +	int ret;
> +	struct pwm_chip *chip;
> +	struct pwm_loongson_ddata *ddata;
> +	struct device *dev = &pdev->dev;
> +
> +	chip = devm_pwmchip_alloc(dev, 1, sizeof(*ddata));
> +	if (IS_ERR(chip))
> +		return PTR_ERR(chip);
> +	ddata = to_pwm_loongson_ddata(chip);
> +
> +	ddata->base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(ddata->base))
> +		return PTR_ERR(ddata->base);
> +
> +	ddata->clk = devm_clk_get_optional_enabled(dev, NULL);
> +	if (IS_ERR(ddata->clk))
> +		return dev_err_probe(dev, PTR_ERR(ddata->clk),
> +				     "failed to get pwm clock\n");
> +	if (ddata->clk) {
> +		ret = devm_clk_rate_exclusive_get(dev, ddata->clk);
> +		if (ret)
> +			return ret;

Error message please. Also please make all errors start with a capital
letter.

> +		ddata->clk_rate = clk_get_rate(ddata->clk);
> +	} else {
> +		ddata->clk_rate = LOONGSON_PWM_FREQ_DEFAULT;
> +	}
> +
> +	/* Explicitly initialize the CTRL register */
> +	pwm_loongson_writel(ddata, 0, LOONGSON_PWM_REG_CTRL);

This disables all outputs, right? Ideally the driver takes over running
channels. Consider the bootloader initialized a display with a splash
screen. Disabling the PWM might disable the backlight of the display
which hurts the visual experience.

> +	chip->ops = &pwm_loongson_ops;
> +	chip->atomic = true;
> +	dev_set_drvdata(dev, chip);
> +
> +	ret = devm_pwmchip_add(dev, chip);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "failed to add PWM chip\n");
> +
> +	return 0;
> +}
> +
> +static int pwm_loongson_suspend(struct device *dev)
> +{
> +	struct pwm_chip *chip = dev_get_drvdata(dev);
> +	struct pwm_loongson_ddata *ddata = to_pwm_loongson_ddata(chip);
> +
> +	ddata->lss.ctrl = pwm_loongson_readl(ddata, LOONGSON_PWM_REG_CTRL);
> +	ddata->lss.duty = pwm_loongson_readl(ddata, LOONGSON_PWM_REG_DUTY);
> +	ddata->lss.period = pwm_loongson_readl(ddata, LOONGSON_PWM_REG_PERIOD);
> +
> +	clk_disable_unprepare(ddata->clk);
> +
> +	return 0;

Is this needed assuming that before suspend the consumer stopped the
PWM?

Best regards
Uwe

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux