[U-Boot] [PATCH v4 03/11] pci: xilinx: Add a driver for Xilinx AXI to PCIe bridge

[Simon Glass]

Hi Paul,

On 4 August 2016 at 04:24, Paul Burton <paul.burton at imgtec.com> wrote:
>
>
> On 04/08/16 10:54, Michal Simek wrote:
>>
>> On 1.8.2016 12:06, Paul Burton wrote:
>>>
>>> This patch adds a driver for the Xilinx AXI bridge for PCI express, an
>>> IP block which can be used on some generations of Xilinx FPGAs. This is
>>> mostly a case of implementing PCIe ECAM specification, but with some
>>> quirks about what devices are valid to access.
>>>
>>> Signed-off-by: Paul Burton <paul.burton at imgtec.com>
>>> Reviewed-by: Simon Glass <sjg at chromium.org>
>>>
>>> ---
>>>
>>> Changes in v4: None
>>> Changes in v3: None
>>> Changes in v2:
>>> - Clean up error returns & documentation
>>>
>>>  drivers/pci/Kconfig       |   7 ++
>>>  drivers/pci/Makefile      |   1 +
>>>  drivers/pci/pcie_xilinx.c | 219
>>> ++++++++++++++++++++++++++++++++++++++++++++++
>>>  3 files changed, 227 insertions(+)
>>>  create mode 100644 drivers/pci/pcie_xilinx.c

[...]

>>> +
>>> +/**
>>> + * pcie_xilinx_ofdata_to_platdata() - Translate from DT to device state
>>> + * @dev: A pointer to the device being operated on
>>> + *
>>> + * Translate relevant data from the device tree pertaining to device
>>> @dev into
>>> + * state that the driver will later make use of. This state is stored in
>>> the
>>> + * device's private data structure.
>>> + *
>>> + * Return: 0 on success, else -EINVAL
>>> + */
>>> +static int pcie_xilinx_ofdata_to_platdata(struct udevice *dev)
>>> +{
>>> +       struct xilinx_pcie *pcie = dev_get_priv(dev);
>>> +       struct fdt_resource reg_res;
>>> +       DECLARE_GLOBAL_DATA_PTR;
>>> +       int err;
>>> +
>>> +       err = fdt_get_resource(gd->fdt_blob, dev->of_offset, "reg",
>>> +                              0, &reg_res);
>>> +       if (err < 0) {
>>> +               error("\"reg\" resource not found\n");
>>> +               return err;
>>> +       }
>>> +
>>> +       pcie->cfg_base = ioremap_nocache(reg_res.start,
>>> +                                        fdt_resource_size(&reg_res));
>>> +
>>
>>
>> Getting error here.
>>   CC      cmd/mem.o
>>   CC      drivers/net/phy/atheros.o
>>   CC      drivers/net/xilinx_emaclite.o
>> drivers/pci/pcie_xilinx.c: In function 'pcie_xilinx_ofdata_to_platdata':
>> drivers/pci/pcie_xilinx.c:196:17: warning: assignment makes pointer from
>> integer without a cast
>>   pcie->cfg_base = ioremap_nocache(reg_res.start,
>>                  ^
>
>
> Thanks, I'll switch to map_physmem.
>
>>   CC      drivers/net/phy/broadcom.o
>>   CC      cmd/mfsl.o
>>
>>
>> Simon: Is this the right place for doing ioremap?
>> IRC There should be probe function where you call this and this function
>> will just fill platdata structure.
>
>
> ofdata_to_platdata is called right before probe, so I'm not sure there's any
> point in passing the physical address via the private data struct just to
> map it very shortly after & ignore it forevermore.

It is more correct though. If you don't want to do that, I suggest
moving it to probe().

Regards,
Simon