Re: [PATCH v6 1/6] i2c: designware: Cleaning and comment style fixes.

From: Andy Shevchenko
Date: Wed Feb 15 2017 - 08:12:38 EST

Next message: Luca Abeni: "Re: [PATCH 3/2] sched/deadline: Use deadline instead of period when calculating overflow"
Previous message: Arnaldo Carvalho de Melo: "Re: [PATCH 2/2] perf/x86/intel/pt: Fail event creation if VMX operation is on"
In reply to: Luis Oliveira: "[PATCH v6 1/6] i2c: designware: Cleaning and comment style fixes."
Next in thread: Luis Oliveira: "[PATCH v6 2/6] i2c: designware: refactoring of the i2c-designware"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Wed, 2017-02-15 at 12:22 +0000, Luis Oliveira wrote:
> The purpose of this commit is to fix some comments and styling in the
> existing code due to the need of reuse this code. What is being made
> here is:
>
> - Sorted the headers files
> - Corrected some comments style (capital letters, lowcase i2c)
> - Reverse tree in the variables declaration
> - Add/remove empty lines and tabs where needed
> - Fix of misspelled word "endianness" and "transferred"
> - Replaced the return variable "r" with the more standard "ret"
>
> The value of this, besides the rules of coding style, is because I
> will use this code after and it will make my future patch a lot bigger
> and
> complicated to review. The work here won't bring any additional work
> to
> backported fixes because is just style and reordering.
>

Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>

> Signed-off-by: Luis Oliveira <lolivei@xxxxxxxxxxxx>
> Acked-by: Jarkko Nikula <jarkko.nikula@xxxxxxxxxxxxxxx>
> ---
> V5->v6
> - Added the replacement of "r" for "ret" return variables, before it
> Â was done in the 7th patch
>
> Âdrivers/i2c/busses/i2c-designware-core.cÂÂÂÂ| 86 ++++++++++++++----
> -----------
> Âdrivers/i2c/busses/i2c-designware-core.hÂÂÂÂ|ÂÂ2 +-
> Âdrivers/i2c/busses/i2c-designware-platdrv.c | 42 +++++++-------
> Â3 files changed, 66 insertions(+), 64 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-designware-core.c
> b/drivers/i2c/busses/i2c-designware-core.c
> index a62c14c3b2b7..8cf2a8c54ba9 100644
> --- a/drivers/i2c/busses/i2c-designware-core.c
> +++ b/drivers/i2c/busses/i2c-designware-core.c
> @@ -21,17 +21,17 @@
> Â * ------------------------------------------------------------------
> ----------
> Â *
> Â */
> +#include <linux/delay.h>
> Â#include <linux/export.h>
> Â#include <linux/errno.h>
> Â#include <linux/err.h>
> Â#include <linux/i2c.h>
> Â#include <linux/interrupt.h>
> Â#include <linux/io.h>
> -#include <linux/pm_runtime.h>
> -#include <linux/delay.h>
> Â#include <linux/module.h>
> -#include "i2c-designware-core.h"
> +#include <linux/pm_runtime.h>
> Â
> +#include "i2c-designware-core.h"
> Â/*
> Â * Registers offset
> Â */
> @@ -98,7 +98,7 @@
> Â
> Â#define DW_IC_ERR_TX_ABRT 0x1
> Â
> -#define DW_IC_TAR_10BITADDR_MASTER BIT(12)
> +#define DW_IC_TAR_10BITADDR_MASTER BIT(12)
> Â
> Â#define DW_IC_COMP_PARAM_1_SPEED_MODE_HIGH (BIT(2) | BIT(3))
> Â#define DW_IC_COMP_PARAM_1_SPEED_MODE_MASK GENMASK(3, 2)
> @@ -113,10 +113,10 @@
> Â#define TIMEOUT 20 /* ms */
> Â
> Â/*
> - * hardware abort codes from the DW_IC_TX_ABRT_SOURCE register
> + * Hardware abort codes from the DW_IC_TX_ABRT_SOURCE register
> Â *
> - * only expected abort codes are listed here
> - * refer to the datasheet for the full list
> + * Only expected abort codes are listed here,
> + * refer to the datasheet for the full list.
> Â */
> Â#define ABRT_7B_ADDR_NOACK 0
> Â#define ABRT_10ADDR1_NOACK 1
> @@ -318,7 +318,7 @@ static void i2c_dw_release_lock(struct dw_i2c_dev
> *dev)
> Â}
> Â
> Â/**
> - * i2c_dw_init() - initialize the designware i2c master hardware
> + * i2c_dw_init() - Initialize the designware I2C master hardware
> Â * @dev: device private data
> Â *
> Â * This functions configures and enables the I2C master.
> @@ -338,14 +338,14 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
> Â
> Â reg = dw_readl(dev, DW_IC_COMP_TYPE);
> Â if (reg == ___constant_swab32(DW_IC_COMP_TYPE_VALUE)) {
> - /* Configure register endianess access */
> + /* Configure register endianness access */
> Â dev->accessor_flags |= ACCESS_SWAP;
> Â } else if (reg == (DW_IC_COMP_TYPE_VALUE & 0x0000ffff)) {
> Â /* Configure register access mode 16bit */
> Â dev->accessor_flags |= ACCESS_16BIT;
> Â } else if (reg != DW_IC_COMP_TYPE_VALUE) {
> - dev_err(dev->dev, "Unknown Synopsys component type: "
> - "0x%08x\n", reg);
> + dev_err(dev->dev,
> + "Unknown Synopsys component type: 0x%08x\n",
> reg);
> Â i2c_dw_release_lock(dev);
> Â return -ENODEV;
> Â }
> @@ -355,7 +355,7 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
> Â /* Disable the adapter */
> Â __i2c_dw_enable_and_wait(dev, false);
> Â
> - /* set standard and fast speed deviders for high/low periods
> */
> + /* Set standard and fast speed deviders for high/low periods
> */
> Â
> Â sda_falling_time = dev->sda_falling_time ?: 300; /* ns */
> Â scl_falling_time = dev->scl_falling_time ?: 300; /* ns */
> @@ -444,7 +444,7 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
> Â dw_writel(dev, dev->tx_fifo_depth / 2, DW_IC_TX_TL);
> Â dw_writel(dev, 0, DW_IC_RX_TL);
> Â
> - /* configure the i2c master */
> + /* Configure the I2C master */
> Â dw_writel(dev, dev->master_cfg , DW_IC_CON);
> Â
> Â i2c_dw_release_lock(dev);
> @@ -480,7 +480,7 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev
> *dev)
> Â /* Disable the adapter */
> Â __i2c_dw_enable_and_wait(dev, false);
> Â
> - /* if the slave address is ten bit address, enable 10BITADDR
> */
> + /* If the slave address is ten bit address, enable 10BITADDR
> */
> Â if (dev->dynamic_tar_update_enabled) {
> Â /*
> Â Â* If I2C_DYNAMIC_TAR_UPDATE is set, the 10-bit
> addressing
> @@ -505,7 +505,7 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev
> *dev)
> Â Â*/
> Â dw_writel(dev, msgs[dev->msg_write_idx].addr | ic_tar,
> DW_IC_TAR);
> Â
> - /* enforce disabled interrupts (due to HW issues) */
> + /* Enforce disabled interrupts (due to HW issues) */
> Â i2c_dw_disable_int(dev);
> Â
> Â /* Enable the adapter */
> @@ -539,9 +539,9 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
> Â u32 flags = msgs[dev->msg_write_idx].flags;
> Â
> Â /*
> - Â* if target address has changed, we need to
> - Â* reprogram the target address in the i2c
> - Â* adapter when we are done with this transfer
> + Â* If target address has changed, we need to
> + Â* reprogram the target address in the I2C
> + Â* adapter when we are done with this transfer.
> Â Â*/
> Â if (msgs[dev->msg_write_idx].addr != addr) {
> Â dev_err(dev->dev,
> @@ -601,7 +601,7 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
> Â
> Â if (msgs[dev->msg_write_idx].flags &
> I2C_M_RD) {
> Â
> - /* avoid rx buffer overrun */
> + /* Avoid rx buffer overrun */
> Â if (dev->rx_outstanding >= dev-
> >rx_fifo_depth)
> Â break;
> Â
> @@ -730,7 +730,7 @@ static int i2c_dw_handle_tx_abort(struct
> dw_i2c_dev *dev)
> Â}
> Â
> Â/*
> - * Prepare controller for a transaction and call i2c_dw_xfer_msg
> + * Prepare controller for a transaction and call i2c_dw_xfer_msg.
> Â */
> Âstatic int
> Âi2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
> @@ -761,10 +761,10 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct
> i2c_msg msgs[], int num)
> Â if (ret < 0)
> Â goto done;
> Â
> - /* start the transfers */
> + /* Start the transfers */
> Â i2c_dw_xfer_init(dev);
> Â
> - /* wait for tx to complete */
> + /* Wait for tx to complete */
> Â if (!wait_for_completion_timeout(&dev->cmd_complete, adap-
> >timeout)) {
> Â dev_err(dev->dev, "controller timed out\n");
> Â /* i2c_dw_init implicitly disables the adapter */
> @@ -788,7 +788,7 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct
> i2c_msg msgs[], int num)
> Â goto done;
> Â }
> Â
> - /* no error */
> + /* No error */
> Â if (likely(!dev->cmd_err && !dev->status)) {
> Â ret = num;
> Â goto done;
> @@ -823,8 +823,8 @@ static u32 i2c_dw_func(struct i2c_adapter *adap)
> Â}
> Â
> Âstatic const struct i2c_algorithm i2c_dw_algo = {
> - .master_xfer = i2c_dw_xfer,
> - .functionality = i2c_dw_func,
> + .master_xfer = i2c_dw_xfer,
> + .functionality = i2c_dw_func,
> Â};
> Â
> Âstatic u32 i2c_dw_read_clear_intrbits(struct dw_i2c_dev *dev)
> @@ -905,7 +905,7 @@ static irqreturn_t i2c_dw_isr(int this_irq, void
> *dev_id)
> Â
> Â /*
> Â Â* Anytime TX_ABRT is set, the contents of the tx/rx
> - Â* buffers are flushed.ÂÂMake sure to skip them.
> + Â* buffers are flushed. Make sure to skip them.
> Â Â*/
> Â dw_writel(dev, 0, DW_IC_INTR_MASK);
> Â goto tx_aborted;
> @@ -927,7 +927,7 @@ static irqreturn_t i2c_dw_isr(int this_irq, void
> *dev_id)
> Â if ((stat & (DW_IC_INTR_TX_ABRT | DW_IC_INTR_STOP_DET)) ||
> dev->msg_err)
> Â complete(&dev->cmd_complete);
> Â else if (unlikely(dev->accessor_flags & ACCESS_INTR_MASK)) {
> - /* workaround to trigger pending interrupt */
> + /* Workaround to trigger pending interrupt */
> Â stat = dw_readl(dev, DW_IC_INTR_MASK);
> Â i2c_dw_disable_int(dev);
> Â dw_writel(dev, stat, DW_IC_INTR_MASK);
> @@ -962,23 +962,23 @@ EXPORT_SYMBOL_GPL(i2c_dw_read_comp_param);
> Âint i2c_dw_probe(struct dw_i2c_dev *dev)
> Â{
> Â struct i2c_adapter *adap = &dev->adapter;
> - int r;
> + int ret;
> Â u32 reg;
> Â
> Â init_completion(&dev->cmd_complete);
> Â
> - r = i2c_dw_init(dev);
> - if (r)
> - return r;
> + ret = i2c_dw_init(dev);
> + if (ret)
> + return ret;
> Â
> - r = i2c_dw_acquire_lock(dev);
> - if (r)
> - return r;
> + ret = i2c_dw_acquire_lock(dev);
> + if (ret)
> + return ret;
> Â
> Â /*
> Â Â* Test if dynamic TAR update is enabled in this controller
> by writing
> Â Â* to IC_10BITADDR_MASTER field in IC_CON: when it is enabled
> this
> - Â* field is read-only so it should not succeed
> + Â* field is read-only so it should not succeed.
> Â Â*/
> Â reg = dw_readl(dev, DW_IC_CON);
> Â dw_writel(dev, reg ^ DW_IC_CON_10BITADDR_MASTER, DW_IC_CON);
> @@ -999,13 +999,13 @@ int i2c_dw_probe(struct dw_i2c_dev *dev)
> Â i2c_set_adapdata(adap, dev);
> Â
> Â i2c_dw_disable_int(dev);
> - r = devm_request_irq(dev->dev, dev->irq, i2c_dw_isr,
> + ret = devm_request_irq(dev->dev, dev->irq, i2c_dw_isr,
> Â ÂÂÂÂÂIRQF_SHARED | IRQF_COND_SUSPEND,
> Â ÂÂÂÂÂdev_name(dev->dev), dev);
> - if (r) {
> + if (ret) {
> Â dev_err(dev->dev, "failure requesting irq %i: %d\n",
> - dev->irq, r);
> - return r;
> + dev->irq, ret);
> + return ret;
> Â }
> Â
> Â /*
> @@ -1015,12 +1015,12 @@ int i2c_dw_probe(struct dw_i2c_dev *dev)
> Â Â* registered I2C slaves that do I2C transfers in their
> probe.
> Â Â*/
> Â pm_runtime_get_noresume(dev->dev);
> - r = i2c_add_numbered_adapter(adap);
> - if (r)
> - dev_err(dev->dev, "failure adding adapter: %d\n", r);
> + ret = i2c_add_numbered_adapter(adap);
> + if (ret)
> + dev_err(dev->dev, "failure adding adapter: %d\n",
> ret);
> Â pm_runtime_put_noidle(dev->dev);
> Â
> - return r;
> + return ret;
> Â}
> ÂEXPORT_SYMBOL_GPL(i2c_dw_probe);
> Â
> diff --git a/drivers/i2c/busses/i2c-designware-core.h
> b/drivers/i2c/busses/i2c-designware-core.h
> index 26250b425e2f..00bee4da9beb 100644
> --- a/drivers/i2c/busses/i2c-designware-core.h
> +++ b/drivers/i2c/busses/i2c-designware-core.h
> @@ -48,7 +48,7 @@
> Â * @cmd_complete: tx completion indicator
> Â * @clk: input reference clock
> Â * @cmd_err: run time hadware error code
> - * @msgs: points to an array of messages currently being transfered
> + * @msgs: points to an array of messages currently being transferred
> Â * @msgs_num: the number of elements in msgs
> Â * @msg_write_idx: the element index of the current tx message in the
> msgs
> Â * array
> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c
> b/drivers/i2c/busses/i2c-designware-platdrv.c
> index 6ce431323125..381f0c1058b4 100644
> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> @@ -21,26 +21,28 @@
> Â * ------------------------------------------------------------------
> ----------
> Â *
> Â */
> -#include <linux/kernel.h>
> -#include <linux/module.h>
> +#include <linux/acpi.h>
> +#include <linux/clk-provider.h>
> +#include <linux/clk.h>
> Â#include <linux/delay.h>
> Â#include <linux/dmi.h>
> -#include <linux/i2c.h>
> -#include <linux/clk.h>
> -#include <linux/clk-provider.h>
> -#include <linux/errno.h>
> -#include <linux/sched.h>
> Â#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/i2c.h>
> Â#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> Â#include <linux/of.h>
> +#include <linux/platform_data/i2c-designware.h>
> Â#include <linux/platform_device.h>
> Â#include <linux/pm.h>
> Â#include <linux/pm_runtime.h>
> Â#include <linux/property.h>
> -#include <linux/io.h>
> +#include <linux/reset.h>
> +#include <linux/sched.h>
> Â#include <linux/slab.h>
> -#include <linux/acpi.h>
> -#include <linux/platform_data/i2c-designware.h>
> +
> Â#include "i2c-designware-core.h"
> Â
> Âstatic u32 i2c_dw_get_clk_rate_khz(struct dw_i2c_dev *dev)
> @@ -176,11 +178,11 @@ static void dw_i2c_set_fifo_size(struct
> dw_i2c_dev *dev, int id)
> Âstatic int dw_i2c_plat_probe(struct platform_device *pdev)
> Â{
> Â struct dw_i2c_platform_data *pdata = dev_get_platdata(&pdev-
> >dev);
> - struct dw_i2c_dev *dev;
> Â struct i2c_adapter *adap;
> - struct resource *mem;
> - int irq, r;
> + struct dw_i2c_dev *dev;
> Â u32 acpi_speed, ht = 0;
> + struct resource *mem;
> + int irq, ret;
> Â
> Â irq = platform_get_irq(pdev, 0);
> Â if (irq < 0)
> @@ -238,9 +240,9 @@ static int dw_i2c_plat_probe(struct
> platform_device *pdev)
> Â return -EINVAL;
> Â }
> Â
> - r = i2c_dw_eval_lock_support(dev);
> - if (r)
> - return r;
> + ret = i2c_dw_eval_lock_support(dev);
> + if (ret)
> + return ret;
> Â
> Â dev->functionality = I2C_FUNC_10BIT_ADDR |
> DW_IC_DEFAULT_FUNCTIONALITY;
> Â
> @@ -285,11 +287,11 @@ static int dw_i2c_plat_probe(struct
> platform_device *pdev)
> Â pm_runtime_enable(&pdev->dev);
> Â }
> Â
> - r = i2c_dw_probe(dev);
> - if (r && !dev->pm_runtime_disabled)
> + ret = i2c_dw_probe(dev);
> + if (ret && !dev->pm_runtime_disabled)
> Â pm_runtime_disable(&pdev->dev);
> Â
> - return r;
> + return ret;
> Â}
> Â
> Âstatic int dw_i2c_plat_remove(struct platform_device *pdev)
> @@ -371,7 +373,7 @@ static const struct dev_pm_ops dw_i2c_dev_pm_ops =
> {
> Â#define DW_I2C_DEV_PMOPS NULL
> Â#endif
> Â
> -/* work with hotplug and coldplug */
> +/* Work with hotplug and coldplug */
> ÂMODULE_ALIAS("platform:i2c_designware");
> Â
> Âstatic struct platform_driver dw_i2c_driver = {

--
Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
Intel Finland Oy

Next message: Luca Abeni: "Re: [PATCH 3/2] sched/deadline: Use deadline instead of period when calculating overflow"
Previous message: Arnaldo Carvalho de Melo: "Re: [PATCH 2/2] perf/x86/intel/pt: Fail event creation if VMX operation is on"
In reply to: Luis Oliveira: "[PATCH v6 1/6] i2c: designware: Cleaning and comment style fixes."
Next in thread: Luis Oliveira: "[PATCH v6 2/6] i2c: designware: refactoring of the i2c-designware"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]