On 03/20/2015 04:50 PM, 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> ---
(...)
+static int mma8653fc_i2c_probe(struct i2c_client *client, + const struct i2c_device_id *id) +{ + struct mma8653fc *mma; + const struct mma8653fc_pdata *pdata; + int err; + u8 byte; + + err = i2c_check_functionality(client->adapter, + I2C_FUNC_SMBUS_BYTE_DATA); + if (!err) { + dev_err(&client->dev, "SMBUS Byte Data not Supported\n"); + return -EIO; + } + + mma = kzalloc(sizeof(*mma), GFP_KERNEL);
Why not devm_* API..?
+ if (!mma) { + err = -ENOMEM; + goto err_out; + } + + pdata = dev_get_platdata(&client->dev); + if (!pdata) { + pdata = mma8653fc_probe_dt(&client->dev); + if (IS_ERR(pdata)) { + err = PTR_ERR(pdata); + pr_err("pdata from DT failed\n");
Use dev_err() instead of pr_err()..
+ goto err_free_mem; + } + } + mma->pdata = *pdata; + pdata = &mma->pdata; + mma->client = client; + mma->read = &mma8653fc_read; + mma->write = &mma8653fc_write; + + mutex_init(&mma->mutex); + + i2c_set_clientdata(client, mma); + + err = sysfs_create_group(&client->dev.kobj, &mma8653fc_attr_group); + if (err) + goto err_free_mem; + + mma->irq = irq_of_parse_and_map(client->dev.of_node, 0); + if (!mma->irq) { + dev_err(&client->dev, "Unable to parse or map IRQ\n"); + goto no_irq; + } + + err = irq_set_irq_type(mma->irq, IRQ_TYPE_EDGE_FALLING); + if (err) { + dev_err(&client->dev, "set_irq_type failed\n"); + goto err_free_mem; + } + + err = request_threaded_irq(mma->irq, NULL, mma8653fc_irq, IRQF_ONESHOT, + dev_name(&mma->client->dev), mma);
devm_* API..?
+ if (err) { + dev_err(&client->dev, "irq %d busy?\n", mma->irq); + goto err_free_mem; + } + + if (mma->write(mma, MMA8653FC_CTRL_REG2, SOFT_RESET)) + goto err_free_irq; + + __mma8653fc_disable(mma); + mma->standby = true; + + /* enable desired interrupts */ + mma->orientation = '\0'; + mma->bafro = '\0'; + byte = 0; + if (pdata->int_src_data_ready) { + byte |= INT_EN_DRDY; + dev_dbg(&client->dev, "DATA READY interrupt source enabled\n"); + } + if (pdata->int_src_ff_mt_x || pdata->int_src_ff_mt_y || + pdata->int_src_ff_mt_z) { + byte |= INT_EN_FF_MT; + dev_dbg(&client->dev, "FF MT interrupt source enabled\n"); + } + if (pdata->int_src_lndprt) { + if (mma->write(mma, MMA8653FC_PL_CFG, PL_EN)) + goto err_free_irq; + byte |= INT_EN_LNDPRT; + dev_dbg(&client->dev, "LNDPRT interrupt source enabled\n"); + } + if (pdata->int_src_aslp) { + byte |= INT_EN_ASLP; + dev_dbg(&client->dev, "ASLP interrupt source enabled\n"); + } + if (mma->write(mma, MMA8653FC_CTRL_REG4, byte)) + goto err_free_irq; + + /* force everything to line 1 */ + if (pdata->int1) { + if (mma->write(mma, MMA8653FC_CTRL_REG5, + (INT_CFG_ASLP | INT_CFG_LNDPRT | + INT_CFG_FF_MT | INT_CFG_DRDY))) + goto err_free_irq; + dev_dbg(&client->dev, "using interrupt line 1\n"); + } +no_irq: + /* range mode */ + byte = mma->read(mma, MMA8653FC_XYZ_DATA_CFG); + byte &= ~RANGE_MASK; + switch (pdata->range) { + case DYN_RANGE_2G: + byte |= RANGE2G; + dev_dbg(&client->dev, "use 2g range\n"); + break; + case DYN_RANGE_4G: + byte |= RANGE4G; + dev_dbg(&client->dev, "use 4g range\n"); + break; + case DYN_RANGE_8G: + byte |= RANGE8G; + dev_dbg(&client->dev, "use 8g range\n"); + break; + default: + dev_err(&client->dev, "wrong range mode value\n"); + goto err_free_irq; + } + if (mma->write(mma, MMA8653FC_XYZ_DATA_CFG, byte)) + goto err_free_irq; + + /* data calibration offsets */ + if (pdata->x_axis_offset) { + if (mma->write(mma, MMA8653FC_OFF_X, pdata->x_axis_offset)) + goto err_free_irq; + } + if (pdata->y_axis_offset) { + if (mma->write(mma, MMA8653FC_OFF_Y, pdata->y_axis_offset)) + goto err_free_irq; + } + if (pdata->z_axis_offset) { + if (mma->write(mma, MMA8653FC_OFF_Z, pdata->z_axis_offset)) + goto err_free_irq; + } + + /* if autosleep, wake on both landscape and motion changes */ + if (pdata->auto_wake_sleep) { + byte = 0; + byte |= WAKE_LNDPRT; + byte |= WAKE_FF_MT; + if (mma->write(mma, MMA8653FC_CTRL_REG3, byte)) + goto err_free_irq; + if (mma->write(mma, MMA8653FC_CTRL_REG2, SLPE)) + goto err_free_irq; + dev_dbg(&client->dev, "auto sleep enabled\n"); + } + + /* data rates */ + byte = 0; + byte = mma->read(mma, MMA8653FC_CTRL_REG1); + if (byte < 0) + goto err_free_irq; + byte &= ~ODR_MASK; + byte |= ODR_DEFAULT; + byte &= ~ASLP_RATE_MASK; + byte |= ASLP_RATE_DEFAULT; + if (mma->write(mma, MMA8653FC_CTRL_REG1, byte)) + goto err_free_irq; + + /* freefall / motion config */ + byte = 0; + if (pdata->motion_mode) { + byte |= FF_MT_CFG_OAE; + dev_dbg(&client->dev, "detect motion instead of freefall\n"); + } + byte |= FF_MT_CFG_ELE; + if (pdata->int_src_ff_mt_x) + byte |= FF_MT_CFG_XEFE; + if (pdata->int_src_ff_mt_y) + byte |= FF_MT_CFG_YEFE; + if (pdata->int_src_ff_mt_z) + byte |= FF_MT_CFG_ZEFE; + if (mma->write(mma, MMA8653FC_FF_MT_CFG, byte)) + goto err_free_irq; + + if (pdata->freefall_motion_thr) { + if (mma->write(mma, MMA8653FC_FF_MT_THS, + pdata->freefall_motion_thr)) + goto err_free_irq; + /* calculate back to mg */ + dev_dbg(&client->dev, "threshold set to %dmg\n", + (63 * pdata->freefall_motion_thr) - 1); + } + + byte = mma->read(mma, MMA8653FC_WHO_AM_I); + if (byte != MMA8653FC_DEVICE_ID) { + dev_err(&client->dev, "wrong device for driver\n"); + goto err_free_irq; + } + dev_info(&client->dev, + "accelerometer driver loaded. device id %x\n", byte); + + return 0; + + err_free_irq: + if (mma->irq) + free_irq(mma->irq, mma); + err_free_mem: + kfree(mma);
If we use devm_* API's above steps are not required
+ err_out: + return err; +} + +static int mma8653fc_remove(struct i2c_client *client) +{ + struct mma8653fc *mma = i2c_get_clientdata(client); + + free_irq(mma->irq, mma); + dev_dbg(&client->dev, "unregistered accelerometer\n"); + kfree(mma);
same as above.. -- Varka Bhadram -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html