Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1547989

Re: [PATCH v5 2/7] i2c: designware: refactoring of the i2c-designware

From Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Newsgroups linux.kernel
Subject Re: [PATCH v5 2/7] i2c: designware: refactoring of the i2c-designware
Date 2016-12-28 16:20 +0100
Message-ID <sTobf-1zo-3@gated-at.bofh.it> (permalink)
References <sTnIe-11F-7@gated-at.bofh.it> <sTnIe-11F-15@gated-at.bofh.it>
Organization Intel Finland Oy

Show all headers | View raw


On Wed, 2016-12-28 at 14:43 +0000, Luis Oliveira wrote:
> - Factor out all _master() part of code from i2c-designware-core
>   and i2c-designware-platdrv to separate functions.
> - Standardize all code related with MASTER mode.
> - I have to take off DW_IC_INTR_TX_EMPTY from DW_IC_INTR_DEFAULT_MASK
>   because it is master specific.
> 
> The purpose of this is to prepare the controller to have is I2C MASTER
> flow in a separate driver. To do this first all the
> functions/definitions related to the MASTER flow were identified.

Thanks for an update.
Some style related comments below (For the code related is up to you, my
tag still stands).

> 
> Signed-off-by: Luis Oliveira <lolivei@synopsys.com>
> ---
> Changes V4->V5: (ACK by Andy)

When you get an Ack, or other tag (Reviewed-by, Tested-by, etc), and you
send new version, include this tag to your commit message (it applies to
all affected patches in your series).

It would be also good to have some high level changelog in the cover
letter, from this series I don't see, for example, which base you did
use (i2c-next? linux-next? v4.9? v4.10-rc1?).

> +       dev_dbg(dev->dev,
> +               "%s: enabled=%#x stat=%#x\n", __func__, enabled,
stat);

I hope you can fit format string on the first line. __func__ is
redundant when you are using debug printing (Dynamic Debug would include
it if asked for).

> +static void i2c_dw_configure_master(struct platform_device *pdev)
> +{
> +	struct dw_i2c_dev *dev = platform_get_drvdata(pdev);

By the way, does it make sense to pass struct dw_i2c_dev * as a
parameter of the function?

> +
> +	dev->master_cfg = DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE
> |
> +			  DW_IC_CON_RESTART_EN;
> +
> +	dev_dbg(&pdev->dev, "I am registed as a I2C Master!\n");
> +
> +	switch (dev->clk_freq) {
> +	case 100000:
> +		dev->master_cfg |= DW_IC_CON_SPEED_STD;
> +		break;
> +	case 3400000:
> +		dev->master_cfg |= DW_IC_CON_SPEED_HIGH;
> +		break;
> +	default:
> +		dev->master_cfg |= DW_IC_CON_SPEED_FAST;
> +	}
> +}
> +
> 


-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH v5 0/7] i2c: designware: add I2C SLAVE support Luis Oliveira <Luis.Oliveira@synopsys.com> - 2016-12-28 15:50 +0100
  [PATCH v5 6/7] i2c: designware: enable SLAVE in platform module Luis Oliveira <Luis.Oliveira@synopsys.com> - 2016-12-28 15:50 +0100
    Re: [PATCH v5 6/7] i2c: designware: enable SLAVE in platform module Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2016-12-28 16:50 +0100
      Re: [PATCH v5 6/7] i2c: designware: enable SLAVE in platform module Luis Oliveira <Luis.Oliveira@synopsys.com> - 2016-12-28 17:00 +0100
        Re: [PATCH v5 6/7] i2c: designware: enable SLAVE in platform module Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2016-12-28 17:40 +0100
          Re: [PATCH v5 6/7] i2c: designware: enable SLAVE in platform module Luis Oliveira <Luis.Oliveira@synopsys.com> - 2016-12-28 17:50 +0100
            Re: [PATCH v5 6/7] i2c: designware: enable SLAVE in platform module Carlos Palminha <CARLOS.PALMINHA@synopsys.com> - 2016-12-28 18:00 +0100
            Re: [PATCH v5 6/7] i2c: designware: enable SLAVE in platform module Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2016-12-28 18:20 +0100
              Re: [PATCH v5 6/7] i2c: designware: enable SLAVE in platform module Luis Oliveira <Luis.Oliveira@synopsys.com> - 2016-12-28 19:20 +0100
  [PATCH v5 2/7] i2c: designware: refactoring of the i2c-designware Luis Oliveira <Luis.Oliveira@synopsys.com> - 2016-12-28 15:50 +0100
    Re: [PATCH v5 2/7] i2c: designware: refactoring of the  i2c-designware Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2016-12-28 16:20 +0100
      Re: [PATCH v5 2/7] i2c: designware: refactoring of the i2c-designware Luis Oliveira <Luis.Oliveira@synopsys.com> - 2016-12-28 16:40 +0100
  [PATCH v5 5/7] i2c: designware: add SLAVE mode functions Luis Oliveira <Luis.Oliveira@synopsys.com> - 2016-12-28 15:50 +0100
    Re: [PATCH v5 5/7] i2c: designware: add SLAVE mode functions Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2016-12-28 16:40 +0100
      Re: [PATCH v5 5/7] i2c: designware: add SLAVE mode functions Luis Oliveira <Luis.Oliveira@synopsys.com> - 2016-12-28 17:10 +0100
  [PATCH v5 4/7] i2c: designware: introducing I2C_SLAVE definitions Luis Oliveira <Luis.Oliveira@synopsys.com> - 2016-12-28 15:50 +0100
    Re: [PATCH v5 4/7] i2c: designware: introducing I2C_SLAVE  definitions Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2016-12-28 16:20 +0100
  [PATCH v5 1/7] i2c: designware: Cleaning and comment style fixes. Luis Oliveira <Luis.Oliveira@synopsys.com> - 2016-12-28 15:50 +0100
  [PATCH v5 3/7] i2c: designware: MASTER mode as separated driver Luis Oliveira <Luis.Oliveira@synopsys.com> - 2016-12-28 15:50 +0100

csiph-web