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