From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stefan Roese <sr@denx.de>
Date: Mon, 21 Mar 2016 13:04:22 +0100
Subject: [U-Boot] [PATCH 6/6] i2c: designware_i2c: Add support for
 PCI(e) based I2C cores (x86)
In-Reply-To: <56EFB8D2.5030306@denx.de>
References: <1458287661-21745-1-git-send-email-sr@denx.de>
 <1458287661-21745-6-git-send-email-sr@denx.de>
 <CAEUhbmWW6jne1N7OdGLt1xDUkLY0Ky6Rn4VNnyd9TDOO+Z=+JA@mail.gmail.com>
 <56EFB8D2.5030306@denx.de>
Message-ID: <56EFE346.4030008@denx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Bin,

On 21.03.2016 10:03, Stefan Roese wrote:

<snip>

>>>    static int designware_i2c_probe_chip(struct udevice *bus, uint chip_addr,
>>> @@ -476,14 +519,45 @@ static int designware_i2c_probe(struct udevice *bus)
>>>    {
>>>           struct dw_i2c *priv = dev_get_priv(bus);
>>>
>>> +#ifdef CONFIG_X86
>>> +       /* Save base address from PCI BAR */
>>> +       priv->regs = (struct i2c_regs *)
>>> +               dm_pci_map_bar(bus, PCI_BASE_ADDRESS_0, PCI_REGION_MEM);
>>> +       /* Use BayTrail specific timing values */
>>> +       priv->scl_sda_cfg = &byt_config;
>>> +#else
>>
>> How about:
>>
>>       if (device_is_on_pci_bus(dev)) {
>>       do the PCI I2C stuff here;
>>       }
> 
> I've tried this but it generated compilation errors on socfpga, as the
> dm_pci_xxx functions are not available there. So it definitely needs
> some #ifdef here. I could go with your suggestion and use
> #if CONFIG_DM_PCI as well.
>   
>> See driver/net/designware.c for example.
>>
>>>           /* Save base address from device-tree */
>>>           priv->regs = (struct i2c_regs *)dev_get_addr(bus);
>>> +#endif

Enabling this code for x86 via if (device_is_on_pci_bus(dev)) results
in this ugly compilation warning:

drivers/i2c/designware_i2c.c: In function ?designware_i2c_probe?:
drivers/i2c/designware_i2c.c:530:16: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
   priv->regs = (struct i2c_regs *)dev_get_addr(bus);
                ^

This is because x86 defines fdt_addr_t / phys_addr_t as 64bit. So
I'm wondering, how dev_get_addr() should get used on x86. Has it
been used anywhere here at all? Should we perhaps go back to
a 32bit phy_addr representation again? So that dev_get_addr()
matches the (void *) size again?

The other option would to just leave the code as in v1 so that
dev_get_addr() is not referenced on x86 here.

Thanks,
Stefan