lkml.org 
[lkml]   [2012]   [Jan]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] x86/platform: (TS-5500) revised ADC driver
On Fri, Jan 20, 2012 at 06:43:22PM -0500, Vivien Didelot wrote:
> Update of the ADC driver according to the Guenter Roeck's review.
>
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> ---
> arch/x86/platform/ts5500/ts5500_adc.c | 228 +++++++++++++--------------------
> 1 files changed, 89 insertions(+), 139 deletions(-)
>
> diff --git a/arch/x86/platform/ts5500/ts5500_adc.c b/arch/x86/platform/ts5500/ts5500_adc.c
> index fc4560f..da6decf 100644
> --- a/arch/x86/platform/ts5500/ts5500_adc.c
> +++ b/arch/x86/platform/ts5500/ts5500_adc.c
> @@ -62,57 +62,62 @@
>
Hi Vivien,

This isn't really a revised driver ... it is a patch on top of the previous version.

Regarding the location, I'd really like to know from the powers-that-be if
arch/x86/platform/ts5500/
or
drivers/platform/x86
or
drivers/hwmon

would be the appropriate location for a driver like this. As mentioned before,
my strong preference is drivers/hwmon, but I would like to hear from others.

Also, I am not sure if the current approach is appropriate to start with.
Looking at the datasheet as well as into existing kernel code, it appears quite likely
that some kind of more or less generic MAX197 driver exists somewhere. The existence
of is_max197_installed() - without any calling code - is a strong indication that
this is the case, as well as the "static" platform data in your original patch.
It might be more appropriate to take this more or less generic driver, move it to
drivers/hwmon, and provide - for example through platform data - a means
to read from and write to the chip on a per-platform basis, ie with per-platform
access functions.

Thanks,
Guenter


\
 
 \ /
  Last update: 2012-01-21 05:51    [W:0.065 / U:7.096 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site