Re: [PATCH v2 1/2] i2c: add DMA support for freescale i2c driver

2014-03-09 Thread Shawn Guo
On Thu, Mar 06, 2014 at 12:57:42PM +0100, Marek Vasut wrote:
> On Thursday, March 06, 2014 at 06:02:03 AM, Yao Yuan wrote:
> > On Thu, March 06, 2014 at 12:44:14 PM, Marek Vasut wrote:
> > > On Thursday, March 06, 2014 at 05:36:14 AM, Yao Yuan wrote:
> > > > On Thu, March 06, 2014 at 11:23:50 AM, Marek Vasut wrote:
> > > > > On Wednesday, March 05, 2014 at 07:52:31 AM, Yuan Yao wrote:
> > > > > > Add dma support for i2c. This function depend on DMA driver.
> > > > > > You can turn on it by write both the dmas and dma-name properties
> > > > > > in dts node.
> > > > > > 
> > > > > > Signed-off-by: Yuan Yao 
> > > > > > ---
> > > > > 
> > > > > [...]
> > > > > 
> > > > > > @@ -601,6 +826,7 @@ static int i2c_imx_probe(struct
> > > > > > platform_device
> > > > > 
> > > > > *pdev)
> > > > > 
> > > > > > void __iomem *base;
> > > > > > int irq, ret;
> > > > > > u32 bitrate;
> > > > > > 
> > > > > > +   u32 phy_addr;
> > > > > > 
> > > > > > dev_dbg(&pdev->dev, "<%s>\n", __func__);
> > > > > > 
> > > > > > @@ -611,6 +837,7 @@ static int i2c_imx_probe(struct
> > > > > > platform_device
> > > > > 
> > > > > *pdev)
> > > > > 
> > > > > > }
> > > > > > 
> > > > > > res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > > > > > 
> > > > > > +   phy_addr = res->start;
> > > > > 
> > > > > Uh ... Shawn, I really think I am lost here. Don't you need to map
> > > > > this memory before you can use it for DMA ? The DMA mapping function
> > > > > should give you the physical address and is the right way to go
> > > > > about this instead of pulling the address from here, no ?
> > > > > 
> > > > > I might be wrong here, I am rather uncertain, so please help me out.
> > > > > Thanks!
> > > > 
> > > > Hi, Marek, Thanks for your suggestion.
> > > > Here you can review the code in include/linux/ioport.h The
> > > > resource->start describes the entity on the CPU bus as a starting
> > > > physical address. So I thinks it can used for dma directly.
> > > 
> > > This doesn't feel right for some reason. If this is a register area, you
> > > should
> > > ioremap() it. If it's a memory area you do DMA to/from, you need to make
> > > sure you correctly flush/invalidate caches and properly handle the
> > > effects the write buffer might have. But I have a feeling you actually do
> > > DMA to/from register space here ?
> > 
> > Yes, It's a register area. But I don't know why I should ioremap() it? It's
> > a bus address and DMA can use it directly. Is there some problem for my
> > understanding ?
> 
> I am not too sure here, thus I am poking someone who can clearly answer this.

There is already a devm_ioremap_resource() call in the existing code for
CPU to access registers in virtual address.  And my understanding on
Yuan's patch is that he needs the physical address of I2C DATA register
for DMA from/to the controller.

Shawn

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH V2] i2c: mxs: Use devm_ioremap_resource()

2014-03-09 Thread Jingoo Han
Use devm_ioremap_resource() in order to make the code simpler,
and remove redundant return value check of platform_get_resource()
because the value is checked by devm_ioremap_resource().

Signed-off-by: Jingoo Han 
Acked-by: Shawn Guo 
Acked-by: Marek Vasut 
---
Changes since v1:
- Replace 'return -ENOENT' with 'return irq' when returning error
  values from platform_get_irq(), per Wolfram Sang

 drivers/i2c/busses/i2c-mxs.c |   18 ++
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c
index 0cde4e6..7170fc8 100644
--- a/drivers/i2c/busses/i2c-mxs.c
+++ b/drivers/i2c/busses/i2c-mxs.c
@@ -806,7 +806,6 @@ static int mxs_i2c_probe(struct platform_device *pdev)
struct mxs_i2c_dev *i2c;
struct i2c_adapter *adap;
struct resource *res;
-   resource_size_t res_size;
int err, irq;
 
i2c = devm_kzalloc(dev, sizeof(struct mxs_i2c_dev), GFP_KERNEL);
@@ -819,18 +818,13 @@ static int mxs_i2c_probe(struct platform_device *pdev)
}
 
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-   irq = platform_get_irq(pdev, 0);
-
-   if (!res || irq < 0)
-   return -ENOENT;
+   i2c->regs = devm_ioremap_resource(&pdev->dev, res);
+   if (IS_ERR(i2c->regs))
+   return PTR_ERR(i2c->regs);
 
-   res_size = resource_size(res);
-   if (!devm_request_mem_region(dev, res->start, res_size, res->name))
-   return -EBUSY;
-
-   i2c->regs = devm_ioremap_nocache(dev, res->start, res_size);
-   if (!i2c->regs)
-   return -EBUSY;
+   irq = platform_get_irq(pdev, 0);
+   if (irq < 0)
+   return irq;
 
err = devm_request_irq(dev, irq, mxs_i2c_isr, 0, dev_name(dev), i2c);
if (err)
-- 
1.7.10.4


--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: i2c-tools-4: timeline?

2014-03-09 Thread Jean Delvare
Hi Thomas,

On Tue, 4 Mar 2014 16:42:11 +0100, Thomas De Schampheleire wrote:
> Hi Jean, all,
> 
> Wiki page 'Plan for i2c-tools 4' [1] describes four steps to fix the
> 'i2c-dev.h' problem:
> 
> Step 1: Cleaning up i2c-dev.h
> Step 2: Creating a library for i2c_smbus_* functions
> Step 3: Extending the library
> Step 4: Documentation
> 
> According to this page, some of these steps have already been
> implemented in several commits. These changes date from April 2012.

Indeed, steps 1 and 2 are done. I started step 3 but I didn't go far
enough to commit it.

> Very recently (February 2014), i2c-tools-3.1.1 has been released, but
> these changes are not part of that release.

No, release 3.1.1 was only a bug fix release on top of 3.1.0. I decided
to do that because 3.1.0 was getting old and a few important fixes had
accumulated over time. This is essentially unrelated to the i2c-tools 4
plan.

> My question is: is there any update regarding i2c-tools 4? Do you have
> an approximate timeframe in which we could expect this release?

No time frame I'm afraid. This is on my to-do list, and it has been for
a long time now. However it's rather far away from the top, so I really
have no idea when I will be able to work on it again.

I was wondering if anyone was actually paying attention to my work on
i2c-tools 4. I'm glad to finally see some interest in it. Apparently
you are waiting for it to be done and released? Are you writing some
tool or application that would benefit from libi2c?

In case you are interested, the patches I currently have for step 3 of
the plan are available here:
  http://jdelvare.nerim.net/devel/i2c/libi2c/

Basically the idea is to move the code that was common to i2cdump,
i2cset, i2cget and i2cdetect to libi2c, under the assumption that it
would probably be useful for other tools and applications and would
thus make libi2c more valuable. However this would make these functions
before part of the library's API, so some care must be taken to only
include what's really needed, that the naming and parameters are right,
etc. That's what I did not have enough time for until now. If you want
to help with that, that would be great.

Thanks,
-- 
Jean Delvare
SUSE L3 Support
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] i2c: mxs: Use devm_ioremap_resource()

2014-03-09 Thread Wolfram Sang

> - if (!res || irq < 0)
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0)
>   return -ENOENT;

Please return irq here.



signature.asc
Description: Digital signature


Re: [PATCH 1/2] i2c: bcm2835: Use devm_ioremap_resource()

2014-03-09 Thread Wolfram Sang
On Tue, Feb 11, 2014 at 10:02:36PM +0900, Jingoo Han wrote:
> Use devm_ioremap_resource() in order to make the code simpler,
> and remove redundant return value check of platform_get_resource()
> because the value is checked by devm_ioremap_resource().
> 
> Signed-off-by: Jingoo Han 

Applied to for-next, thanks!



signature.asc
Description: Digital signature


Re: [PATCH] i2c: i801: enable Intel BayTrail SMBUS

2014-03-09 Thread Wolfram Sang
On Sat, Mar 01, 2014 at 12:03:56AM +0800, Chew Chiau Ee wrote:
> From: Chew, Kean ho 
> 
> Add Device ID of Intel BayTrail SMBus Controller.
> 
> Signed-off-by: Chew, Kean ho 
> Signed-off-by: Chew, Chiau Ee 

Applied to for-next, thanks!



signature.asc
Description: Digital signature


Re: [PATCH] i2c-davinci: Implement a bus recovery that actually works

2014-03-09 Thread Wolfram Sang
On Fri, Feb 28, 2014 at 11:32:05AM +0100, mike.looijm...@topic.nl wrote:
> From: Mike Looijmans 
> 
> Having a board where the I2C bus locks up occasionally made it clear
> that the bus recovery in the i2c-davinci driver will only work on
> some boards, because on regular boards, this will only toggle GPIO
> lines that aren't muxed to the actual pins.
> 
> The I2C controller has the built-in capability to bit-bang its lines.
> Use this to implement a generic recovery routine that puts the
> controller in GPIO mode and pulse the clk lines until both SDA and
> SCL return to a high state.
> 
> Because the controller must be held in reset while doing so, the
> recovery routine must re-init the controller. Since this was already
> being done after each call to i2c_recover_bus, move that call into
> the recovery routine as well.
> 
> Tested on a custom board with OMAP-L138, and after this change, the
> board can recover from chips keeping SDA low.
> 
> Note: This is an adapted port from 2.6.37 code, and was only tested
> with that kernel.

What about using struct i2c_bus_recovery_info, so the actual recovery
logic is taken from the core?

Thanks,

   Wolfram



signature.asc
Description: Digital signature


Re: [PATCH] i2c-davinci: Handle signals gracefully

2014-03-09 Thread Wolfram Sang
On Thu, Jan 09, 2014 at 12:11:25PM +0100, Mike Looijmans wrote:
> When a signal is caught while the i2c-davinci bus driver is transferring,
> the drive just "abandons" the transfer and leaves the controller to fend
> for itself. The next I2C transaction will find the controller in an
> undefined state and often results in a stream of "initiating i2c bus recovery"
> messages until the controller arrives in a defined state. This behaviour
> also sends out "half" or possibly even mixed messages to I2C client
> devices which may put them in an undesired state as well.
> 
> This patch fixes this issue by always attempting to finish the current
> transaction, and then check on a pending signal. It either reports
> success if all data has been transferred, or it returns failure when
> the transaction was aborted. This keeps the controller in a defined
> state, and is also much friendlier towards client devices, because
> it will only send complete messages.

Even more, you should complete the whole transfer. There are devices
where things can really go wrong if you send a half-complete command and
then start with the next one. So, not checking signals at all is the way
to go for I2C drivers. There is some cruft left, so I am happy about
patches fixing that, with testing on real HW. Like yours here.



signature.asc
Description: Digital signature


Re: [PATCH 2/2] i2c: designware-pci: set ideal HCNT, LCNT and SDA hold time value

2014-03-09 Thread Wolfram Sang
On Fri, Mar 07, 2014 at 10:12:51PM +0800, Chew Chiau Ee wrote:
> From: Chew, Chiau Ee 
> 
> On Intel BayTrail, there was case whereby the resulting fast mode
> bus speed becomes slower (~20% slower compared to expected speed)
> if using the HCNT/LCNT calculated in the core layer. Thus, this
> patch is added to allow pci glue layer to pass in optimal
> HCNT/LCNT/SDA hold time values to core layer since the core
> layer supports cofigurable HCNT/LCNT/SDA hold time values now.
> 
> Signed-off-by: Chew, Chiau Ee 

Can you make use of those instead?

u32 sda_falling_time;
u32 scl_falling_time;

This is more consistent with using sda_hold_time and lets them have a
common (and more readable) unit.



signature.asc
Description: Digital signature


Re: [PATCH 1/2] i2c: designware-pci: add 10-bit addressing mode functionality for BYT I2C

2014-03-09 Thread Wolfram Sang
On Fri, Mar 07, 2014 at 10:12:50PM +0800, Chew Chiau Ee wrote:
> From: Chew, Chiau Ee 
> 
> All the I2C controllers on Intel BayTrail LPSS subsystem able
> to support 10-bit addressing mode functionality.
> 
> Signed-off-by: Chew, Chiau Ee 
> Signed-off-by: Ong, Boon Leong 
> ---
>  drivers/i2c/busses/i2c-designware-pcidrv.c |   17 +++--
>  1 files changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c 
> b/drivers/i2c/busses/i2c-designware-pcidrv.c
> index f1dabee..87f2fc4 100644
> --- a/drivers/i2c/busses/i2c-designware-pcidrv.c
> +++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
> @@ -64,12 +64,19 @@ struct dw_pci_controller {
>   u32 tx_fifo_depth;
>   u32 rx_fifo_depth;
>   u32 clk_khz;
> + u32 functionality;
>  };
>  
>  #define INTEL_MID_STD_CFG  (DW_IC_CON_MASTER |   \
>   DW_IC_CON_SLAVE_DISABLE |   \
>   DW_IC_CON_RESTART_EN)
>  
> +#define DW_DEFAULT_FUNCTIONALITY (I2C_FUNC_I2C | \
> + I2C_FUNC_SMBUS_BYTE |   \
> + I2C_FUNC_SMBUS_BYTE_DATA |  \
> + I2C_FUNC_SMBUS_WORD_DATA |  \
> + I2C_FUNC_SMBUS_I2C_BLOCK)

Can't we have I2C_FUNC_SMBUS_EMUL here? (Need checking with
I2C_SMBUS_QUICK)



signature.asc
Description: Digital signature


Re: [PATCH v2] i2c: designware-pci: Add Baytrail PCI IDs

2014-03-09 Thread Wolfram Sang
On Wed, Feb 19, 2014 at 04:10:29PM +0200, Mika Westerberg wrote:
> Intel Baytrail I2C controllers can be enumerated from PCI as well as from
> ACPI. In order to support this add the Baytrail PCI IDs to the driver.
> 
> Signed-off-by: Mika Westerberg 

Applied to for-next, thanks!



signature.asc
Description: Digital signature


Re: [PATCH] i2c: designware-pci: Cleanup driver power management

2014-03-09 Thread Wolfram Sang
On Tue, Feb 04, 2014 at 02:37:07PM +0200, Mika Westerberg wrote:
> The PCI part of the DesignWare I2C driver does a lot of things that are not
> required anymore. For example drivers aren't supposed to handle PCI state
> transitions themselves. This is all provided by the PCI bus core already.
> 
> In addition to that there is no point scheduling RPM suspend on driver's
> idle hook but instead we can use RPM autosuspend for this (which is enabled
> in the driver already).
> 
> As a bonus, this patch also fixes following compile warning which is
> emitted when the driver was compiled without CONFIG_PM_RUNTIME set:
> 
> drivers/i2c/busses/i2c-designware-pcidrv.c:245:12: warning: 
> ‘i2c_dw_pci_runtime_idle’ defined but not used [-Wunused-function]
> 
> Reported-by: xinhui.pan 
> Reported-by: Jingoo Han 
> Signed-off-by: Mika Westerberg 

Applied to for-next, thanks!



signature.asc
Description: Digital signature


Re: [PATCH v2 1/2] i2c designware make SCL and SDA falling time configurable

2014-03-09 Thread Wolfram Sang
On Mon, Jan 20, 2014 at 05:43:43PM +0100, Romain Baeriswyl wrote:
> This patch allows to set independantly SCL and SDA falling times.
> The tLOW period is computed by taking into account the SCL falling time.
> The tHIGH period is computed by taking into account the SDA falling time.
> 
> For instance in case the margin on tLOW is considered too small, it can
> be increased by increasing the SCL falling time which is by default set
> at 300ns.
> 
> The same applies for tHIGH period with the help of SDA falling time.
> 
> Signed-off-by: Romain Baeriswyl 
> Reviewed-by: Christian Ruppert 

Applied to for-next, thanks!



signature.asc
Description: Digital signature


Re: [PATCH v2 2/2] i2c designware add support of I2C standard mode

2014-03-09 Thread Wolfram Sang

> + /* fast mode by default */

Please add "because of legacy reasons" since the usual default is
100kHz.

> + clk_freq = 40;

> + /* Only standard mode at 100kHz and fast mode at 400kHz
> +  * are supported.
> +  */
> + if (clk_freq != 10 && clk_freq != 40)
> + return -EINVAL;

You didn't add the error message I requested last time.



signature.asc
Description: Digital signature