Re: [PATCH v6] add support for Freescale's MMA8653FC 10 bit accelerometer
From: Dan Carpenter
Date: Fri Mar 27 2015 - 04:04:25 EST
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
> + 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
> +
> +Documentation/ABI/testing/...
> +-----------------------------
> +What: /sys/bus/i2c/drivers/mma8653fc/*/standby
> +Date: March 2015
> +Contact: Martin Kepplinger <martin.kepplinger@xxxxxxxxxxxxxxxxxxxxx>
> +Description:
> + Write 0 to this in order to turn on the device, and 1 to turn
> + it off. Read to see if it is turned on or off.
> +
> +
> +What: /sys/bus/i2c/drivers/mma8653fc/*/currentmode
> +Date: March 2015
> +Contact: Martin Kepplinger <martin.kepplinger@xxxxxxxxxxxxxxxxxxxxx>
> +Description:
> + Reading this provides the current state of the device, read
> + directly from a register. This can be "standby", "wake" or
> + "sleep".
> +
> +
> +What: /sys/bus/i2c/drivers/mma8653fc/*/position
> +Date: March 2015
> +Contact: Martin Kepplinger <martin.kepplinger@xxxxxxxxxxxxxxxxxxxxx>
> +Description:
> + Read only. Without interrupts enabled gets current position
> + values by reading. Poll "position" with interrupt conditions
> + set, to get notified; see Documentation/.../fsl,mma8653fc.txt
> +
> + position file format:
> + "x y z [landscape/portrait status] [front/back status]"
> +
> + x y z values:
> + in mg
> + landscape/portrait status char:
> + r landscape right
> + d portrait down
> + u portrait up
> + l landscape left
> + front/back status char:
> + f front facing
> + b back facing
> +
> +
> +Documentation/devicetree/bindings/...
> +-------------------------------------
> +Required properties:
> +- compatible
> + "fsl,mma8653fc"
> +- reg
> + I2C address
> +
> +Optional properties:
> +
> +- interrupt-parent
> + a phandle for the interrupt controller (see
> + Documentation/devicetree/bindings/interrupt-controller/interrupts.txt)
> +- interrupts
> + interrupt line to which the chip is connected (active low)
> +- int1
> + set to use interrupt line 1, default is line 2
> + the interrupt sources can be routed to one of the two lines
> +- ir-freefall-motion-x
> + activate freefall/motion interrupts on x axis
> +- ir-freefall-motion-y
> + activate freefall/motion interrupts on y axis
> +- ir-freefall-motion-z
> + activate freefall/motion interrupts on z axis
> +- irq-threshold
> + 0 < value < 8000: threshold for motion interrupts in mg
> +- ir-landscape-portrait
> + activate landscape/portrait interrupts
> +- ir-auto-wake
> + activate wake/sleep change interrupts
> +- ir-data-ready:
> + activate data-ready interrupts
> + Interrupt events can be activated in any combination.
> +- dynamic-range
> + 2, 4, or 8: dynamic measurement range in g, default: 2
> + In ±2 g mode, sensitivity = 256 counts/g.
> + In ±4 g mode, sensitivity = 128 counts/g.
> + In ±8 g mode, sensitivity = 64 counts/g.
> +- auto-wake-sleep
> + auto sleep mode (lower frequency)
> +- motion-mode
> + use motion mode instead of freefall mode (trigger if >threshold).
> + per default an interrupt occurs if motion values fall below the
> + value set in "threshold" and therefore can detect free fall on the
> + vertical axis (depending on the position of the device).
> + Setting this values inverts the behaviour and an interrupt occurs
> + above the threshold value, so usually activate horizontal axis in
> + this case.
> +
> +- x-offset
> + 0 < value < 500: calibration offset in mg
> + The offset correction values are used to realign the Zero-g position
> + of the X, Y, and Z-axis after the device is mounted on a board.
> + this value has an offset of 250 itself:
> + 0 is -250mg, 250 is 0 mg, 500 is 250mg
> +- y-offset
> + see x-offset
> +- z-offset
> + see x-offset
> +
> +Example 1:
> +for a device laying on flat ground to recognize acceleration over 100mg.
> +x-axis is calibrated to +10mg. Adapt interrupt line to your device.
> +
> +mma8653fc@1d {
> + compatible = "fsl,mma8653fc";
> + interrupt-parent = <&gpio1>;
> + interrupts = <5 0>;
> + reg = <0x1d>;
> +
> + dynamic-range = <2>;
> + motion-mode;
> + ir-freefall-motion-x;
> + ir-freefall-motion-y;
> + irq-threshold = <100>;
> + x-offset = <160>;
Just one tab.
> +};
> +
> +Example 2:
> +for a device mounted on a wall with y being the vertical axis. This recognizes
> +y-acceleration below 800mg, so free fall or changing the orientation of the
> +device (y not being the vertical axis and having ~1000mg anymore).
> +
> +mma8653fc@1d {
> + compatible = "fsl,mma8653fc";
> + interrupt-parent = <&gpio1>;
> + interrupts = <5 0>;
> + reg = <0x1d>;
> +
> + dynamic-range = <2>;
> + ir-freefall-motion-y;
> + irq-threshold = <800>;
> +};
> +
> +Example 3:
> +minimal example. lets users read current acceleration values. No polling
> +is available.
> +
> +mma8653fc@1d {
> + compatible = "fsl,mma8653fc";
> + reg = <0x1d>;
> +};
> diff --git a/drivers/staging/mma8653fc/mma8653fc.c b/drivers/staging/mma8653fc/mma8653fc.c
> new file mode 100644
> index 0000000..4bd7f99
> --- /dev/null
> +++ b/drivers/staging/mma8653fc/mma8653fc.c
> @@ -0,0 +1,864 @@
> +/*
> + * mma8653fc.c - Support for Freescale MMA8653FC 3-axis 10-bit accelerometer
> + *
> + * Copyright (c) 2014 Theobroma Systems Design and Consulting GmbH
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/delay.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/slab.h>
> +#include <linux/module.h>
> +#include <linux/i2c.h>
> +#include <linux/types.h>
> +#include <linux/of_device.h>
> +#include <linux/of_irq.h>
> +
> +#define DRV_NAME "mma8653fc"
> +#define MMA8653FC_DEVICE_ID 0x5a
> +
> +#define MMA8653FC_STATUS 0x00
> +
> +#define ZYXOW_MASK 0x80
> +#define ZYXDR 0x08
> +
> +#define MMA8653FC_WHO_AM_I 0x0d
> +
> +#define MMA8653FC_SYSMOD 0x0b
> +#define STATE_STANDBY 0x00
> +#define STATE_WAKE 0x01
> +#define STATE_SLEEP 0x02
> +
> +#define MMA8450_STATUS_ZXYDR 0x08
> +
> +/*
> + * 10 bit output data registers
> + * MSB: 7:0 bits 9:2 of data word
> + * LSB: 7:6 bits 1:0 of data word
> + */
> +#define MMA8653FC_OUT_X_MSB 0x01
> +#define MMA8653FC_OUT_X_LSB 0x02
> +#define MMA8653FC_OUT_Y_MSB 0x03
> +#define MMA8653FC_OUT_Y_LSB 0x04
> +#define MMA8653FC_OUT_Z_MSB 0x05
> +#define MMA8653FC_OUT_Z_LSB 0x06
> +
> +/*
> + * Portrait/Landscape Status
> + */
> +#define MMA8653FC_PL_STATUS 0x10
> +
> +#define NEWLP 0x80
> +#define LAPO_HIGH 0x04
> +#define LAPO_LOW 0x02
> +#define BAFRO 0x01
> +
> +/*
> + * Portrait/Landscape Configuration
> + */
> +#define MMA8653FC_PL_CFG 0x11
> +
> +#define PL_EN (1 << 6)
> +
> +/*
> + * Data calibration registers
> + */
> +#define MMA8653FC_OFF_X 0x2f
> +#define MMA8653FC_OFF_Y 0x30
> +#define MMA8653FC_OFF_Z 0x31
> +
> +/* 0 to 500 for dts, but -250 to 250 in mg */
> +#define DEFAULT_OFF 250
> +
> +/*
> + * bits 1:0 dynamic range
> + * 00 +/- 2g
> + * 01 +/- 4g
> + * 10 +/- 8g
> + *
> + * HPF_Out bit 4 - data high pass or low pass filtered
> + */
> +#define MMA8653FC_XYZ_DATA_CFG 0x0e
> +
> +#define RANGE_MASK 0x03
> +#define RANGE2G 0x00
> +#define RANGE4G 0x01
> +#define RANGE8G 0x02
> +/* values for calculation */
> +#define SHIFT_2G 8
> +#define INCR_2G 128
> +#define SHIFT_4G 7
> +#define INCR_4G 64
> +#define SHIFT_8G 6
> +#define INCR_8G 32
> +#define DYN_RANGE_2G 2
> +#define DYN_RANGE_4G 4
> +#define DYN_RANGE_8G 8
> +
> +/*
> + * System Control Reg 1
> + */
> +#define MMA8653FC_CTRL_REG1 0x2a
> +
> +#define ACTIVE_BIT (1 << 0)
> +#define ODR_MASK 0x38
> +#define ODR_DEFAULT 0x20 /* 50 Hz */
> +#define ASLP_RATE_MASK 0xc0
> +#define ASLP_RATE_DEFAULT 0x80 /* 6.25 Hz */
> +
> +/*
> + * Sys Control Reg 2
> + *
> + * auto-sleep enable, software reset
> + */
> +#define MMA8653FC_CTRL_REG2 0x2b
> +
> +#define SLPE (1 << 2)
> +#define SELFTEST (1 << 7)
> +#define SOFT_RESET (1 << 6)
> +
> +/*
> + * Interrupt Source
> + */
> +#define MMA8653FC_INT_SOURCE 0x0c
> +
> +#define SRC_ASLP (1 << 7)
> +#define SRC_LNDPRT (1 << 4)
> +#define SRC_FF_MT (1 << 2)
> +#define SRC_DRDY (1 << 0)
> +
> +/*
> + * Interrupt Control Register
> + *
> + * default: active low
> + * default push-pull, not open-drain
> + */
> +#define MMA8653FC_CTRL_REG3 0x2c
> +
> +#define WAKE_LNDPRT (1 << 5)
> +#define WAKE_FF_MT (1 << 3)
> +#define IPOL (1 << 1)
> +#define PP_OD (1 << 0)
> +
> +/*
> + * Interrupt Enable Register
> + */
> +#define MMA8653FC_CTRL_REG4 0x2d
> +
> +#define INT_EN_ASLP (1 << 7)
> +#define INT_EN_LNDPRT (1 << 4)
> +#define INT_EN_FF_MT (1 << 2)
> +#define INT_EN_DRDY (1 << 0)
> +
> +/*
> + * Interrupt Configuration Register
> + * bit value 0 ... INT2 (default)
> + * bit value 1 ... INT1
> + */
> +#define MMA8653FC_CTRL_REG5 0x2e
> +
> +#define INT_CFG_ASLP (1 << 7)
> +#define INT_CFG_LNDPRT (1 << 4)
> +#define INT_CFG_FF_MT (1 << 2)
> +#define INT_CFG_DRDY (1 << 0)
> +
> +/*
> + * Freefall / Motion Configuration Register
> + *
> + * Event Latch enable/disable, motion or freefall mode
> + * and event flag enable per axis
> + */
> +#define MMA8653FC_FF_MT_CFG 0x15
> +
> +#define FF_MT_CFG_ELE (1 << 7)
> +#define FF_MT_CFG_OAE (1 << 6)
> +#define FF_MT_CFG_ZEFE (1 << 5)
> +#define FF_MT_CFG_YEFE (1 << 4)
> +#define FF_MT_CFG_XEFE (1 << 3)
> +
> +/*
> + * Freefall / Motion Source Register
> + */
> +#define MMA8653FC_FF_MT_SRC 0x16
> +
> +/*
> + * Freefall / Motion Threshold Register
> + *
> + * define motion threshold
> + * 0.063 g/LSB, 127 counts(0x7f) (7 bit from LSB)
> + * range: 0.063g - 8g
> + */
> +#define MMA8653FC_FF_MT_THS 0x17
> +
> +struct axis_triple {
> + s16 x;
> + s16 y;
> + s16 z;
> +};
> +
> +struct mma8653fc_pdata {
> + s8 x_axis_offset;
> + s8 y_axis_offset;
> + s8 z_axis_offset;
> + bool auto_wake_sleep;
> + u32 range;
> + bool int1;
> + bool motion_mode;
> + u8 freefall_motion_thr;
> + bool int_src_data_ready;
> + bool int_src_ff_mt_x;
> + bool int_src_ff_mt_y;
> + bool int_src_ff_mt_z;
> + bool int_src_lndprt;
> + bool int_src_aslp;
> +};
> +
> +struct mma8653fc {
> + struct i2c_client *client;
> + struct mutex mutex;
> + struct mma8653fc_pdata pdata;
> + struct axis_triple saved;
> + char orientation;
> + char bafro;
> + bool standby;
> + int irq;
> + unsigned int_mask;
> + u8 (*read)(struct mma8653fc *, unsigned char);
> + void (*write)(struct mma8653fc *, unsigned char, unsigned char);
> +};
> +
> +/* defaults */
> +static const struct mma8653fc_pdata mma8653fc_default_init = {
> + .range = 2,
> + .x_axis_offset = DEFAULT_OFF,
> + .y_axis_offset = DEFAULT_OFF,
> + .z_axis_offset = DEFAULT_OFF,
> + .auto_wake_sleep = false,
> + .int1 = false,
> + .motion_mode = false,
> + .freefall_motion_thr = 5,
> + .int_src_data_ready = false,
> + .int_src_ff_mt_x = false,
> + .int_src_ff_mt_y = false,
> + .int_src_ff_mt_z = false,
> + .int_src_lndprt = false,
> + .int_src_aslp = false,
> +};
> +
> +static void mma8653fc_get_triple(struct mma8653fc *mma)
> +{
> + u8 buf[6];
> + u8 status;
> +
> + buf[0] = 0;
We don't use this.
> +
> + status = mma->read(mma, MMA8653FC_STATUS);
> + if (status & ZYXOW_MASK)
> + dev_dbg(&mma->client->dev, "previous read not completed\n");
> +
> + buf[0] = mma->read(mma, MMA8653FC_OUT_X_MSB);
> + buf[1] = mma->read(mma, MMA8653FC_OUT_X_LSB);
> + buf[2] = mma->read(mma, MMA8653FC_OUT_Y_MSB);
> + buf[3] = mma->read(mma, MMA8653FC_OUT_Y_LSB);
> + buf[4] = mma->read(mma, MMA8653FC_OUT_Z_MSB);
> + buf[5] = mma->read(mma, MMA8653FC_OUT_Z_LSB);
> +
> + mutex_lock(&mma->mutex);
> + /* move from registers to s16 */
> + mma->saved.x = (buf[1] | (buf[0] << 8)) >> 6;
> + mma->saved.y = (buf[3] | (buf[2] << 8)) >> 6;
> + mma->saved.z = (buf[5] | (buf[4] << 8)) >> 6;
Why is buf[] and array instead of individual variables?
u8 x_msb, x_lsb, etc..
> + mma->saved.x = sign_extend32(mma->saved.x, 9);
> + mma->saved.y = sign_extend32(mma->saved.y, 9);
> + mma->saved.z = sign_extend32(mma->saved.z, 9);
> +
> + /* calc g, see data sheet and application note */
> + switch (mma->pdata.range) {
> + case DYN_RANGE_2G:
> + mma->saved.x = le16_to_cpu((1000 * mma->saved.x +
> + INCR_2G) >> SHIFT_2G);
It might be nicer to break these up like this:
mma->saved.x = le16_to_cpu((1000 * mma->saved.x + INCR_2G) >>
SHIFT_2G);
Little endian math always confuses me. mma->saved.x starts as little
endian and then we multiply it and add and shift and change it to cpu
endian? Does that work?
> + mma->saved.y = le16_to_cpu((1000 * mma->saved.y +
> + INCR_2G) >> SHIFT_2G);
> + mma->saved.z = le16_to_cpu((1000 * mma->saved.z +
> + INCR_2G) >> SHIFT_2G);
> + break;
> + case DYN_RANGE_4G:
> + mma->saved.x = le16_to_cpu((1000 * mma->saved.x +
> + INCR_4G) >> SHIFT_4G);
> + mma->saved.y = le16_to_cpu((1000 * mma->saved.y +
> + INCR_4G) >> SHIFT_4G);
> + mma->saved.z = le16_to_cpu((1000 * mma->saved.z +
> + INCR_4G) >> SHIFT_4G);
> + break;
> + case DYN_RANGE_8G:
> + mma->saved.x = le16_to_cpu((1000 * mma->saved.x +
> + INCR_8G) >> SHIFT_8G);
> + mma->saved.y = le16_to_cpu((1000 * mma->saved.y +
> + INCR_8G) >> SHIFT_8G);
> + mma->saved.z = le16_to_cpu((1000 * mma->saved.z +
> + INCR_8G) >> SHIFT_8G);
> + break;
> + default:
> + dev_err(&mma->client->dev, "internal data corrupt\n");
> + }
> + mutex_unlock(&mma->mutex);
> +}
> +
> +static void mma8653fc_get_orientation(struct mma8653fc *mma, u8 byte)
> +{
> + if ((byte & LAPO_HIGH) && !(LAPO_LOW))
> + mma->orientation = 'r'; /* landscape right */
> + if (!(byte & LAPO_HIGH) && (byte & LAPO_LOW))
> + mma->orientation = 'd'; /* portrait down */
> + if (!(byte & LAPO_HIGH) && !(byte & LAPO_LOW))
> + mma->orientation = 'u'; /* portrait up */
> + if ((byte & LAPO_HIGH) && (byte & LAPO_LOW))
> + mma->orientation = 'l'; /* landscape left */
> +
> + if (byte & BAFRO)
> + mma->bafro = 'b'; /* back facing */
> + else
> + mma->bafro = 'f'; /* front facing */
We could use defines here maybe:
#define LANDSCAPE_RIGHT 'r'
> +}
> +
> +static irqreturn_t mma8653fc_irq(int irq, void *handle)
> +{
> + struct mma8653fc *mma = handle;
> + u8 int_src;
> + u8 byte;
> +
> + int_src = mma->read(mma, MMA8653FC_INT_SOURCE);
> + if (int_src & SRC_DRDY)
> + /* data ready handle */
Where is the body of this if statement?
> + if (int_src & SRC_FF_MT) {
> + /* freefall/motion change handle */
> + dev_dbg(&mma->client->dev,
> + "freefall or motion change\n");
> + byte = mma->read(mma, MMA8653FC_FF_MT_SRC);
> + }
> + if (int_src & SRC_LNDPRT) {
> + /* landscape/portrait change handle */
> + dev_dbg(&mma->client->dev,
> + "landscape / portrait change\n");
> + byte = mma->read(mma, MMA8653FC_PL_STATUS);
> + mma8653fc_get_orientation(mma, byte);
> + }
> + if (int_src & SRC_ASLP)
> + /* autosleep change handle */
Same.
> + mma8653fc_get_triple(mma);
> +
> + sysfs_notify(&mma->client->dev.kobj, NULL, "position");
> +
> + return IRQ_HANDLED;
> +}
> +
> +static void __mma8653fc_enable(struct mma8653fc *mma)
> +{
> + u8 byte;
> +
> + byte = mma->read(mma, MMA8653FC_CTRL_REG1);
> + mma->write(mma, MMA8653FC_CTRL_REG1, byte | ACTIVE_BIT);
> +}
> +
> +static void __mma8653fc_disable(struct mma8653fc *mma)
> +{
> + u8 byte;
> +
> + byte = mma->read(mma, MMA8653FC_CTRL_REG1);
> + mma->write(mma, MMA8653FC_CTRL_REG1, byte & ~ACTIVE_BIT);
> +}
> +
> +static ssize_t mma8653fc_standby_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct mma8653fc *mma = i2c_get_clientdata(client);
> +
> + return scnprintf(buf, PAGE_SIZE, "%u\n", mma->standby);
> +}
> +
> +static ssize_t mma8653fc_standby_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct mma8653fc *mma = i2c_get_clientdata(client);
> + unsigned int val;
> + int error;
> +
> + error = kstrtouint(buf, 10, &val);
> + if (error)
> + return error;
> +
> + mutex_lock(&mma->mutex);
> +
> + if (val) {
> + if (!mma->standby)
> + __mma8653fc_disable(mma);
> + } else {
> + if (mma->standby)
> + __mma8653fc_enable(mma);
> + }
> +
> + mma->standby = !!val;
> +
> + mutex_unlock(&mma->mutex);
> +
> + return count;
> +}
> +
> +static DEVICE_ATTR(standby, 0664, mma8653fc_standby_show,
> + mma8653fc_standby_store);
> +
> +static ssize_t mma8653fc_currentmode_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct mma8653fc *mma = i2c_get_clientdata(client);
> + ssize_t count;
> + u8 byte;
> +
> + byte = mma->read(mma, MMA8653FC_SYSMOD);
> + if (byte < 0)
> + return byte;
> +
> + switch (byte) {
> + case STATE_STANDBY:
> + count = scnprintf(buf, PAGE_SIZE, "standby\n");
> + break;
> + case STATE_WAKE:
> + count = scnprintf(buf, PAGE_SIZE, "wake\n");
> + break;
> + case STATE_SLEEP:
> + count = scnprintf(buf, PAGE_SIZE, "sleep\n");
> + break;
> + default:
> + count = scnprintf(buf, PAGE_SIZE, "unknown\n");
> + }
> + return count;
> +}
> +static DEVICE_ATTR(currentmode, S_IRUGO, mma8653fc_currentmode_show, NULL);
> +
> +static ssize_t mma8653fc_position_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct mma8653fc *mma = i2c_get_clientdata(client);
> + ssize_t count;
> + u8 byte;
> +
> + if (!mma->irq) {
> + byte = mma->read(mma, MMA8653FC_PL_STATUS);
> + if (byte & NEWLP)
> + mma8653fc_get_orientation(mma, byte);
> +
> + byte = mma->read(mma, MMA8653FC_STATUS);
> + if (byte & ZYXDR)
> + mma8653fc_get_triple(mma);
> +
> + msleep(20);
> + dev_dbg(&client->dev, "data polled\n");
> + }
> + mutex_lock(&mma->mutex);
> + count = scnprintf(buf, PAGE_SIZE, "%d %d %d %c %c\n",
> + mma->saved.x, mma->saved.y, mma->saved.z,
> + mma->orientation, mma->bafro);
> + mutex_unlock(&mma->mutex);
> +
> + return count;
> +}
> +static DEVICE_ATTR(position, S_IRUGO, mma8653fc_position_show, NULL);
> +
> +static struct attribute *mma8653fc_attributes[] = {
> + &dev_attr_position.attr,
> + &dev_attr_standby.attr,
> + &dev_attr_currentmode.attr,
> + NULL,
> +};
> +
> +static const struct attribute_group mma8653fc_attr_group = {
> + .attrs = mma8653fc_attributes,
> +};
> +
> +static u8 mma8653fc_read(struct mma8653fc *mma, unsigned char reg)
> +{
> + u8 val;
> +
> + val = i2c_smbus_read_byte_data(mma->client, reg);
> + if (val < 0) {
u8 is never less than zero.
> + dev_err(&mma->client->dev,
> + "failed to read %x register\n", reg);
> + }
> + return val;
> +}
> +
> +static void mma8653fc_write(struct mma8653fc *mma, unsigned char reg,
> + unsigned char val)
> +{
> + int ret;
> +
> + ret = i2c_smbus_write_byte_data(mma->client, reg, val);
> + if (ret) {
> + dev_err(&mma->client->dev,
> + "failed to write %x register\n", reg);
> + }
> +}
> +
> +static const struct of_device_id mma8653fc_dt_ids[] = {
> + { .compatible = "fsl,mma8653fc", },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, mma8653fc_dt_ids);
> +
> +static const struct mma8653fc_pdata *mma8653fc_probe_dt(struct device *dev)
> +{
> + struct mma8653fc_pdata *pdata;
> + struct device_node *node = dev->of_node;
> + const struct of_device_id *match;
> + u32 testu32;
> + s32 tests32;
> +
> + if (!node) {
> + dev_err(dev, "no associated DT data\n");
> + return ERR_PTR(-EINVAL);
> + }
> +
> + match = of_match_device(mma8653fc_dt_ids, dev);
> + if (!match) {
> + dev_err(dev, "unknown device model\n");
> + return ERR_PTR(-EINVAL);
> + }
> +
> + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> + if (!pdata)
> + return ERR_PTR(-ENOMEM);
> +
> + *pdata = mma8653fc_default_init;
> +
> + /* overwrite from dts */
> + testu32 = pdata->x_axis_offset;
> + tests32 = 0;
Not used.
> + of_property_read_u32(node, "x-offset", &testu32);
> + tests32 = testu32 - DEFAULT_OFF;
> + if ((tests32) && (tests32 <= DEFAULT_OFF) &&
> + (tests32 >= -DEFAULT_OFF)) {
Emacs adds parenthesis all over the place because it is trying to
convert the world to LISP.
> + dev_info(dev, "use %dmg offset on X axis\n", tests32);
> + /* calc register value, resolution: 1.96mg */
> + pdata->x_axis_offset = (s8) (tests32 / 2);
> + }
> + testu32 = pdata->y_axis_offset;
> + tests32 = 0;
Not used.
The signedness bug and the missing if statement bodies could have been
caught by Smatch I think. It might be worth running. It gets run
automatically after merging also. http://smatch.sf.net
regards,
dan carpenter
--
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/