Re: [PATCH] i2c: ismt: fix wrong device address when unmap the data buffer

[Andy Shevchenko]

On Mon, Jun 12, 2017 at 12:28 PM, Peter Rosin <peda@xxxxxxxxxx> wrote:
> On 2017-06-12 11:11, Andy Shevchenko wrote:
>> On Mon, Jun 12, 2017 at 6:42 AM, Song liwei <liwei.song@xxxxxxxxxxxxx> wrote:
>>> From: Liwei Song <liwei.song@xxxxxxxxxxxxx>

>>> After finished I2C block read/write, when unmap the data buffer,
>>> a wrong device address was pass to dma_unmap_single(),

>>> the right
>>> device address should be "dev" not "&adap->dev", the relation is
>>> *(&adap->dev) == dev.
>>
>> This is confusing. You are telling that there are two copies of struct
>> device here?
>
> Yes, there are two copies.

No, there is not. See below.

> There's the local dev variable that is like
> this:
>         struct device *dev = &priv->pci_dev->dev;
>
> And then there's the adapter device in adap->dev (inlined in adap, so
> the explanation that the relations is "*(&adap->dev) == dev" is doubly
> wrong).

I got the idea, but your explanation is not a penny better.

There are two struct devices, one is a real PCI device, which
represents actual device what *does* DMA.
This struct should be used according to DMA API.

Another struct device which is wrongly used is an artificial one that
represents I2C adapter in terms of Linux kernel.

The relation ship (if designed correctly) *should be* adap->dev.parent
== &pci_dev->dev.

> The bug is that the first argument to dma_unmap_single is not the
> same as the first argument to dma_map_single that appears a few lines
> up.

I understand that.

> I cannot tell if that argument should be "dev" or "&adap->dev"
> though, but the two calls must refer to the same struct device *.

See above. It's a simple choice.

-- 
With Best Regards,
Andy Shevchenko