Re: [PATCH v6] add support for Freescale's MMA8653FC 10 bit accelerometer

[Greg KH]

On Fri, Mar 27, 2015 at 08:38:55AM +0100, Martin Kepplinger wrote:
> From: Martin Kepplinger <martin.kepplinger@xxxxxxxxxxxxxxxxxxxxx>
> 
> The MMA8653FC is a low-power, three-axis, capacitive micromachined
> accelerometer with 10 bits of resolution with flexible user-programmable
> options.
> 
> Embedded interrupt functions enable overall power savings, by relieving the
> host processor from continuously polling data, for example using the poll()
> system call.
> 
> The device can be configured to generate wake-up interrupt signals from any
> combination of the configurable embedded functions, enabling the MMA8653FC
> to monitor events while remaining in a low-power mode during periods of
> inactivity.
> 
> This driver provides devicetree properties to program the device's behaviour
> and a simple, tested and documented sysfs interface. The data sheet and more
> information is available on Freescale's website.
> 
> Signed-off-by: Martin Kepplinger <martin.kepplinger@xxxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Christoph Muellner <christoph.muellner@xxxxxxxxxxxxxxxxxxxxx>
> ---
> 
> patch revision history
> ......................
> v6 fix staging integration, base it on next-20150326 and change recipients
> v5 clean up (suggested by Varka Bhadram) and move the driver to staging
> v4 changes DT propery names, adds a missing interrupt source and removes
>    the DT option to set interrupt line active high due to unsuccesful testing
> v3 moves the driver from drivers/input/misc to drivers/misc
> v2 corrects licensing and commit messages and adds appropriate recipients
> 
>  drivers/staging/Kconfig               |   2 +
>  drivers/staging/Makefile              |   1 +
>  drivers/staging/mma8653fc/Kconfig     |  10 +
>  drivers/staging/mma8653fc/Makefile    |   1 +
>  drivers/staging/mma8653fc/TODO        | 146 ++++++
>  drivers/staging/mma8653fc/mma8653fc.c | 864 ++++++++++++++++++++++++++++++++++
>  6 files changed, 1024 insertions(+)
>  create mode 100644 drivers/staging/mma8653fc/Kconfig
>  create mode 100644 drivers/staging/mma8653fc/Makefile
>  create mode 100644 drivers/staging/mma8653fc/TODO
>  create mode 100644 drivers/staging/mma8653fc/mma8653fc.c
> 
> diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig
> index bfacf69..834d949 100644
> --- a/drivers/staging/Kconfig
> +++ b/drivers/staging/Kconfig
> @@ -112,4 +112,6 @@ source "drivers/staging/i2o/Kconfig"
>  
>  source "drivers/staging/fsl-mc/Kconfig"
>  
> +source "drivers/staging/mma8653fc/Kconfig"
> +
>  endif # STAGING
> diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile
> index 2bbd1bf..cfea86a 100644
> --- a/drivers/staging/Makefile
> +++ b/drivers/staging/Makefile
> @@ -48,3 +48,4 @@ obj-$(CONFIG_COMMON_CLK_XLNX_CLKWZRD)	+= clocking-wizard/
>  obj-$(CONFIG_FB_TFT)		+= fbtft/
>  obj-$(CONFIG_I2O)		+= i2o/
>  obj-$(CONFIG_FSL_MC_BUS)	+= fsl-mc/
> +obj-$(CONFIG_MMA8653FC)		+= mma8653fc/
> diff --git a/drivers/staging/mma8653fc/Kconfig b/drivers/staging/mma8653fc/Kconfig
> new file mode 100644
> index 0000000..988451b
> --- /dev/null
> +++ b/drivers/staging/mma8653fc/Kconfig
> @@ -0,0 +1,10 @@
> +config MMA8653FC
> +        tristate "MMA8653FC - Freescale's 3-Axis, 10-bit Digital Accelerometer"
> +        depends on I2C
> +        default n

n is always the default, no need to have this line.

> +        help
> +          Say Y here if you want to support Freescale's MMA8653FC Accelerometer
> +          through I2C interface.
> +
> +          To compile this driver as a module, choose M here: the
> +          module will be called mma8653fc.
> diff --git a/drivers/staging/mma8653fc/Makefile b/drivers/staging/mma8653fc/Makefile
> new file mode 100644
> index 0000000..9a245a3
> --- /dev/null
> +++ b/drivers/staging/mma8653fc/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_MMA8653FC)         += mma8653fc.o
> diff --git a/drivers/staging/mma8653fc/TODO b/drivers/staging/mma8653fc/TODO
> new file mode 100644
> index 0000000..0a31225
> --- /dev/null
> +++ b/drivers/staging/mma8653fc/TODO
> @@ -0,0 +1,146 @@
> +- move to IIO device API. The current DT/sysfs interface is documented below

Then why not put this under drivers/staging/iio/ ?

What is preventing you from moving to this API today?  How long is it
going to take?  Why merge the code now and require lots of tiny patches
in order to get it to the IIO api and not just take a week and do it now
before merging this?

Also, I need an "owner" of this driver in order to be able to accept it,
look at the other TODO files in drivers/staging/*/ to see the format, or
even better yet, please provide a MAINTAINERS file entry for this.

I really think you should just take the tiny amount of time and do this
correctly and not put it in staging.  Otherwise, this is going to take a
lot more work on your part to get it cleaned up correctly.

thanks,

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