Re: [PATCH 1/2 v3] hwmon: (aspeed-pwm-tacho) reduce fan_tach period

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

 



On Mon, Jun 26, 2017 at 11:09 PM, Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
> On 06/26/2017 06:08 AM, Joel Stanley wrote:
>>
>> On Sat, Jun 24, 2017 at 11:23 PM, Guenter Roeck <linux@xxxxxxxxxxxx>
>> wrote:
>>>
>>> On 06/23/2017 08:39 PM, Patrick Venture wrote:
>>
>>
>>>>
>>>> Reduce the fan_tach period such that the fan controller uses a shorter
>>>> period to measure the rpm.
>>>>
>>>
>>> This explains what you are doing, but not why. What is the problem you
>>> are
>>> trying to solve, and why doesn't it create other problems ? Presumably
>>> there
>>> was a reason for the larger period used earlier. If not, if it was just a
>>> conservative setting, here is the place to say it.
>>>
>>> I'd update the information myself, but I don't think the underlying
>>> specification
>>> is public, or at least I did not find it.
>>
>>
>> The datasheet for the Aspeed SoC is not public. Patrick and I can help
>> answer questions, and I've added Ryan from Aspeed who can answer
>> questions in more detail.
>>
>> As I understand it this setting is a trade off between giving the tach
>> unit enough time to measure complete fan rotations under all
>> conditions (ie, when the fans are going slow) and getting a timely
>> measurement. With the driver as-is we have a large delay between
>> readings, which makes writing a control loop impossible.
>>
>> I'm not convinced that this driver had enough testing before it was
>> merged. I agree that Patrick should provide reasoning for his changes,
>
>
> The driver or the latest set of patches ?
>
> Everyone is encouraged to review patches. If there are concerns with a
> driver,
> those should be raised during the review process. I am not really the
> fastest
> reacting maintainer nowadays, so I would think there should have been enough
> time for such feedback. In case concerns were raised and I missed it, my
> apologies.

The driver. I was too slow to voice my objections, so you were fine on
your part.

Cheers,

Joel
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux