[PATCH 2/2] lm87: Add support for configuration through platform_data

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

 



Jean Delvare wrote:
> Hi Ben,
> 
> On Tue, 19 Aug 2008 19:30:36 +0100, Ben Hutchings wrote:
[...]
> > In case (1) platform_data is not set and in case (2) it is.  I don't think
> > there is a third case where we would want to set the channel but not reset
> > the chip.
> 
> Hmm, OK, this makes some sense. I'm still not sure what is the point of
> resetting the chip though. If the firmware had initialized it, you
> loose the settings. If it didn't, the reset is a no-op.

It seems like a good idea to make sure we're starting from a known state.

> And resetting the chip at removal time makes even less sense to me.
> While I would understand that you stop the monitoring (if and only if
> the chip wasn't already running at load time), resetting it but letting
> it run seems pointless.

Resetting it does stop it.  But what I'll do in the next iteration is to
restore the initial configuration register on removal.

[...]
> Not that I care much... After all, you'll be the only user of that
> code, at least for now, and it can evolve later if others have
> different needs. But I am still worried that you are defining a
> non-intuitive interface and do not document it. If you were to add a
> paragraph to Documentation/hwmon/lm87 about how platform code can
> influence the initialization, I'd probably buy it.
[...]

I'll synch it with the header comment in lm87.c.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.




[Index of Archives]     [Linux Kernel]     [Linux Hardware Monitoring]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux