Re: [PATCH] [RFC] hwmon: nct6775: Register fan PWMs as PWM chip

[Richard Weinberger]

Hello Uwe,

----- Ursprüngliche Mail -----
> Von: "Uwe Kleine-König" <ukleinek@xxxxxxxxxx>
>> +	struct nct6775_data *data = pwmchip_get_drvdata(chip);
>> +	const u8 *wfhw = _wfhw;
>> +
>> +	if (get_pwm_period(data, pwm->hwpwm, &wf->period_length_ns))
>> +		return 1;
> 
> That looks wrong. In principle nct6775_pwm_round_waveform_fromhw()
> doesn't depend on hardware state. It's supposed to just convert the
> settings stored in _wfhw to wf. If you know that some things are
> constant during the lifetime of the PWM and you read those from
> hardware, return a proper error code, not 1.

I see. Since the frequency is never changed by the driver we could
read it while probing and use here the cached value.

> Rounding down wf->period_length_ns is fine, so this must be:
> 
>	if (wf->period_length_ns < cur_period)
>		return 1;

But then the period is no longer fixed and something larger than supported
can get configured. Smaller values get caught, though.

e.g.
root@fedora:/sys/class/pwm/pwmchip0/pwm2# cat period 
43243
root@fedora:/sys/class/pwm/pwmchip0/pwm2# echo 43200 > period 
bash: echo: write error: Invalid argument
root@fedora:/sys/class/pwm/pwmchip0/pwm2# echo 50000 > period 
root@fedora:/sys/class/pwm/pwmchip0/pwm2# echo $?
0

Thanks,
//richard