Prev: Allow CONFIG_MIGRATION to be set without CONFIG_NUMA or memory hot-remove
Next: tracing: updates
From: Andrew Morton on 16 Feb 2010 15:30
On Sun, 14 Feb 2010 23:08:16 +0900
NISHIMOTO Hiroki <tlayboy(a)gmail.com> wrote:

> Add new driver for Avago ADJD-S371-QR999 digital color sensor.
>
> This driver has two functions.
> 1. raw_color
> Gets color values of object.
> 2. trim_color
> Gets color values of object except the influence of natural light.
>
> Reset, sleep, external clock functions are not mandatory, so not implemented.
>

There are quite a number of driver commands here which weren't
documented. Please fully describe the proposed userspace interface
within the changelog. Or, better, within a documentation file.
Because the userspace interface is the most important part. It's the
only thing we can never change.

It appears that the entire userspace interface for this driver is
implemented via sysfs? This is odd - we used to have this thing called
"/dev" for communicating with device drivers. What do people think
about this?

The proposed sysfs interface appears to emit multiple values from a
single sysfs file. This is considered to be against the sysfs design
rules.

Note that all the above discusses the usersapce interface! Once we get
that sorted out then we can start looking at the implementation.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Greg KH on 16 Feb 2010 15:40
On Sun, Feb 14, 2010 at 11:08:16PM +0900, NISHIMOTO Hiroki wrote:
> + * The main function of the ADJD-S371-QR999 is to detect color of object.
> + * If you need other functions led, reset, sleep, external clock,
> + * implement it youself by using gpio.
> + *
> + * ++ USAGE EXAMPLE ++
> + * + optimize the color value
> + * # echo 3 > /sys/devices/platform/s371_qr999/capacitor/xxx_cap
> + * # echo 1500 > /sys/devices/platform/s371_qr999/integration/xxx_int
> + *
> + * + get color values
> + * # cat /sys/devices/platform/s371_qr999/raw_color
> + *
> + * + get color values except the influence of natural light
> + * # cat /sys/devices/platform/s371_qr999/trim_get
> + * # cat /sys/devices/platform/s371_qr999/trim_color

As you are adding new sysfs files, you have to add the proper entries in
the Documentation/ABI/ directory as well.

Also, sysfs is ONE VALUE PER FILE, please do not do what you are doing
here with multiple values per a single file, that is not acceptable.

> + * Data Sheet
> + * http://www.avagotech.com/docs/AV02-0314EN
> + * Application Note:
> + * http://www.avagotech.com/docs/AV02-0359EN

I would recommend working with the hwmon developers to develop a
standard API for these types of devices, and use it instead of creating
your own.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 | 
Pages: 1
Prev: Allow CONFIG_MIGRATION to be set without CONFIG_NUMA or memory hot-remove
Next: tracing: updates