Re: [PATCH v2] pwm: renesas-tpu: Add suspend/resume function

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

 



Dear Morimoto-san,

Thank for your feedback!

On 2019/05/23 11:02, Kuninori Morimoto wrote:
Hi

+static int tpu_pwm_suspend(struct device *dev)
+{
+	struct tpu_device *tpu = dev_get_drvdata(dev);
+	struct pwm_chip *chip = &tpu->chip;
+	struct pwm_device *pwm;
+	int i;
+
+	for (i = 0; i <= 3; i++) {
+		if ((pwm_get_chip_data(&chip->pwms[i])) != NULL) {
+			pwm = &chip->pwms[i];
+			if (!test_bit(PWMF_REQUESTED, &pwm->flags))
+				return 0;
+		}
+	}
why 3 ?
According to Hardware manual, 16-Bit Timer Pulse Unit (TPU)
supports four 16-bit timers for both R-car GEN2 and GEN3.
About code, how about this ?

		pwm = &chip->pwms[i];
		if (pwm_get_chip_data(pwm)) {
			...

Can we use chip->pwms at driver ? I'm not sure but pwm.h say

	struct pwm_chip {
		...
=>		/* only used internally by the PWM framework */
		struct list_head list;
		struct pwm_device *pwms;
	};
As described in the header: "@pwms: array of PWM devices allocated by the framework" Therefore, we can not use chip->pwms and must specify the device that will be used.
Otherwise the compilation will fail.
+
+	pm_runtime_put(dev);
+
+	return 0;
+}
Do we need to call pm_runtime_xxx here ?

"pm_runtime_put(dev);" function is called for runtime idle operations.

+static int tpu_pwm_resume(struct device *dev)
+{
+	struct tpu_device *tpu = dev_get_drvdata(dev);
+	struct pwm_chip *chip = &tpu->chip;
+	struct pwm_device *pwm;
+	int i;
+
+	pm_runtime_get_sync(dev);
+
+	for (i = 0; i <= 3; i++) {
+		if ((pwm_get_chip_data(&chip->pwms[i])) != NULL) {
+			pwm = &chip->pwms[i];
+			tpu_pwm_restart_timer(pwm);
+		}
+	}
ditto

Thank you,

Dong




[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux