linux-kernel - Re: [lm-sensors] [RFC 5/5] hwmon: add support for Technologic Systems TS-5500 A-D converter

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 04 May 2011 10:03:24 +0100
From:	Jonathan Cameron <jic23@....ac.uk>
To:	Vivien Didelot <vivien.didelot@...oirfairelinux.com>
CC:	Guenter Roeck <guenter.roeck@...csson.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"lm-sensors@...sensors.org" <lm-sensors@...sensors.org>,
	"linux-serial@...r.kernel.org" <linux-serial@...r.kernel.org>,
	Jonas Fonseca <jonas.fonseca@...oirfairelinux.com>,
	"platform-driver-x86@...r.kernel.org" 
	<platform-driver-x86@...r.kernel.org>,
	linux-iio <linux-iio@...r.kernel.org>
Subject: Re: [lm-sensors] [RFC 5/5] hwmon: add support for Technologic Systems
 TS-5500 A-D converter

On 05/03/11 16:55, Vivien Didelot wrote:
> Hi Guenter,
> 
> Excerpts from Guenter Roeck's message of 2011-04-29 23:39:38 -0400:
>> Hi Vivien,
>>
>> The headline and file name are a bit misleading, since the driver is really
>> for MAX197 on a TS-5500 board.
>>
>> You should split the driver into two parts, a generic driver
>> for the MAX197 and a platform driver (residing somewhere in arch/
>> or possibly drivers/platform/) to instantiate it.
>>
>> There should be a platform data include file, probably in
>> include/linux/platform_data/.
>>
>> .ioaddr in platform data should not be necessary. The driver's probe
>> function should get the values using platform_get_resource().
>>
>> Having said that, from reading the code it looks like the chip is not really
>> used for hardware monitoring, but for generic ADC functionality. A quick look
>> into the TS-5500 user manual confirms this. So this should not be a hwmon
>> driver in the first place, but a generic ADC driver. Given the ADC conversion rate
>> of the MAX197, the hwmon ABI is not optimal anyway. You should move this driver
>> into the iio subsystem, probably to drivers/staging/iio/adc. Copying the iio
>> mailing list for input.
>>
>> Thanks,
>> Guenter
> 
> I've took a closer look to the manual and that's right, in fact the
> driver doesn't talk to the MAX197 directly. The board uses a CPLD to
> abstract the interface to the MAX197. So all the MAX197 logic is hidden
> by the CPLD.  Therefore, the driver files should probably not have
> function and structure names with a "max197_" prefix. I'll make the code
> a bit clearer. What do you think?
> 
> Ok for iio subsystem, several ADC drivers are in drivers/hwmon/ but
> drivers/staging/iio/adc seems to be the good place now.
Just as a heads up, beware there are a few abi changes (and a lot of core
ones) working their way through review /already in staging-next.
I only mention it because merges against that tree will go through staging-next
and as it's name suggests is sometimes a fast moving target!

Jonathan
> Regards,
> Vivien.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-serial" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/