On Tue, Oct 11, 2022 at 09:46:08AM -0700, Guenter Roeck wrote: > On 10/11/22 09:12, Naresh Solanki wrote: > > Hi Guenter, > > > > fan-common is intended for fan properties. i.e., those derived from > > fan datasheets. > > For min-rpm, some fans have minimum non zero rpm like 1900rpm below which > > the fan cannot run. > > > > I would argue the properties are for fan controllers, not for fans. > Fans don't have or depend on specific pwm frequencies. Fan controllers > do. Presumably fan controllers can produce a range of frequencies. If they need a specific frequency, then why are they programmable? Something outside the fan controller must have the constraint. > Fans don't have a configurable pwm polarity. Fan controllers do, > to match the hardware on a board. We don't model an inverter, so it's got to go somewhere. > Fans don't have or rely on > a target rpm. That is a system property, configured into the > fan controller. And so on. Yes, but it is per fan. per fan properties/settings should go in the fan node IMO. > If this is for fans, we'll need another set of properties for > fan controllers. The driver for max6639, being a fan controller, > would need to implement those properties. > > Also, as implemented in the MAX6639, max-rpm is the fan's > rpm range, not the actual rpm. Your implementation is just > confusing, including the example in the bindings. Valid values > should be what the chip accepts, not some random value. A fan would have some design maximum RPM depending on its mechanical design and lifetime requirements. A controller would have some maximum in terms of electrical pulse frequency or register bit sizes (depending how RPM or pulse counts are exposed to s/w. That should all be implied from the controller part and not in DT. It's the mechanical limits of the fan we should be defining here. > Really, I don't understand where you are going with this. Certainly it needs more thought for different cases. Rob