[U-Boot] [PATCH v1] pci: add DM based mpc85xx driver
Heiko Schocher
hs at denx.de
Fri Aug 30 04:16:47 UTC 2019
Hello Z.q. Hou,
Am 29.08.2019 um 10:11 schrieb Z.q. Hou:
> Hi Heiko,
>
>> -----Original Message-----
>> From: Heiko Schocher <hs at denx.de>
>> Sent: 2019年7月16日 11:36
>> To: u-boot at lists.denx.de
>> Cc: Heiko Schocher <hs at denx.de>; Alexander Graf <agraf at suse.de>;
>> Andrew F. Davis <afd at ti.com>; Xiaowei Bao <xiaowei.bao at nxp.com>; Bin
>> Meng <bmeng.cn at gmail.com>; Eugen Hristev
>> <eugen.hristev at microchip.com>; Heinrich Schuchardt
>> <xypron.glpk at gmx.de>; Horatiu Vultur <horatiu.vultur at microchip.com>;
>> Z.q. Hou <zhiqiang.hou at nxp.com>; Krzysztof Kozlowski <krzk at kernel.org>;
>> Liviu Dudau <Liviu.Dudau at foss.arm.com>; Marek Vasut
>> <marek.vasut at gmail.com>; Mario Six <mario.six at gdsys.cc>; Michal Simek
>> <michal.simek at xilinx.com>; Neil Armstrong <narmstrong at baylibre.com>;
>> Prabhakar Kushwaha <prabhakar.kushwaha at nxp.com>; Ryder Lee
>> <ryder.lee at mediatek.com>; Simon Glass <sjg at chromium.org>; Stefan
>> Roese <sr at denx.de>
>> Subject: [PATCH v1] pci: add DM based mpc85xx driver
>>
>> add DM based PCI Configuration space access support for MPC85xx PCI
>> Bridge
>
> Seems you're converting the arch/powerpc/cpu/mpc85xx/pci.c to driver model,
Yes, should have added this to commit message .. fixed.
> but not every mpc85xx platform uses this driver, platforms mpc8536, 8544,
> 8548 and 8568 use the drivers/pci/fsl_pci_init.c, please take them into account.
Oh, wasn;t aware of this! ... I have no idea why there are 2 drivers and
where are the differences between them, nor I can test all the other
plattforms, so this seems difficult to me.
Do you know, why there are 2 different drivers?
I made this new driver, because I work on DM support for the socrates
board, which is mpc8544 based ... !?
May I rename this driver from pci_mpc85xx.c to pci_mpc85xx_dm.c and it
can be a base for DM support and others, who can test their changes,
can add the other variants?
> Note: The platforms using arch/powerpc/cpu/mpc85xx/pci.c set up PCI LAWs in
> file arch/powerpc/cpu/mpc8xxx/law.c, while other platforms set up PCI LAWs in
> PCI driver. So I think you can move the PCI LAWs set-up into this DM PCI driver
> And remove the PCI entries from law.c.
Yes good idea, add this in v2 for this driver.
>> Signed-off-by: Heiko Schocher <hs at denx.de>
>>
>> ---
>> Travis build, see:
>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftravis
>> -ci.org%2Fhsdenx%2Fu-boot-test%2Fbuilds%2F558855544&data=02%7
>> C01%7CZhiqiang.Hou%40nxp.com%7C231ac8975b2242ea5bfb08d7099eccc
>> 8%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63698844995746
>> 0734&sdata=lQcXtvh5a0G9r%2BFlXnnGQ5VgDYvcR%2FEGWpShJ324su
>> U%3D&reserved=0
>>
>> MAINTAINERS | 5 ++
>> drivers/pci/Kconfig | 7 ++
>> drivers/pci/Makefile | 1 +
>> drivers/pci/pci_mpc85xx.c | 132
>> ++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 145 insertions(+)
>> create mode 100644 drivers/pci/pci_mpc85xx.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index e91684191f..01faa67a6d 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -639,6 +639,11 @@ S: Maintained
>> F: drivers/pci_endpoint/
>> F: include/pci_ep.h
>>
>> +PCI MPC85xx
>> +M: Heiko Schocher <hs at denx.de>
>> +S: Maintained
>> +F: drivers/pci/pci_mpc85xx.c
>> +
>> POWER
>> M: Jaehoon Chung <jh80.chung at samsung.com>
>> S: Maintained
>> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig index
>> 3fe38f7315..88db0067b4 100644
>> --- a/drivers/pci/Kconfig
>> +++ b/drivers/pci/Kconfig
>> @@ -68,6 +68,13 @@ config PCIE_FSL
>> PowerPC MPC85xx, MPC86xx, B series, P series and T series SoCs.
>> This driver does not support SRIO_PCIE_BOOT feature.
>>
>> +config PCI_MPC85XX
>> + bool "MPC85XX PowerPC PCI support"
>> + depends on DM_PCI
>> + help
>> + Say Y here if you want to enable PCI controller support on FSL
>> + PowerPC MPC85xx SoC.
>> +
>> config PCI_RCAR_GEN2
>> bool "Renesas RCar Gen2 PCIe driver"
>> depends on DM_PCI
>> diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile index
>> b5ebd50c85..929f119fb3 100644
>> --- a/drivers/pci/Makefile
>> +++ b/drivers/pci/Makefile
>> @@ -19,6 +19,7 @@ obj-$(CONFIG_PCIE_ECAM_GENERIC) +=
>> pcie_ecam_generic.o
>> obj-$(CONFIG_FSL_PCI_INIT) += fsl_pci_init.o
>> obj-$(CONFIG_PCI_INDIRECT_BRIDGE) += pci_indirect.o
>> obj-$(CONFIG_PCI_GT64120) += pci_gt64120.o
>> +obj-$(CONFIG_PCI_MPC85XX) += pci_mpc85xx.o
>> obj-$(CONFIG_PCI_MSC01) += pci_msc01.o
>> obj-$(CONFIG_PCIE_IMX) += pcie_imx.o
>> obj-$(CONFIG_FTPCI100) += pci_ftpci100.o diff --git
>> a/drivers/pci/pci_mpc85xx.c b/drivers/pci/pci_mpc85xx.c new file mode
>> 100644 index 0000000000..c0d35f7f2c
>> --- /dev/null
>> +++ b/drivers/pci/pci_mpc85xx.c
>> @@ -0,0 +1,132 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * (C) Copyright 2019
>> + * Heiko Schocher, DENX Software Engineering, hs at denx.de.
>> + *
>> + */
>> +#include <common.h>
>> +#include <asm/cpm_85xx.h>
>> +#include <pci.h>
>> +#include <dm.h>
>> +
>> +struct mpc85xx_pci_priv {
>> + void __iomem *cfg_addr;
>> + void __iomem *cfg_data;
>> +};
>> +
>> +static int mpc85xx_pci_dm_read_config(struct udevice *dev, pci_dev_t bdf,
>> + uint offset, ulong *value,
>> + enum pci_size_t size)
>> +{
>> + struct mpc85xx_pci_priv *priv = dev_get_priv(dev);
>> + u32 addr;
>> +
>> + addr = bdf | (offset & 0xfc) | ((offset & 0xf00) << 16) | 0x80000000;
>> + out_be32(priv->cfg_addr, addr);
>> + sync();
>> + *value = pci_conv_32_to_size(in_le32(priv->cfg_data), offset, size);
>> +
>> + return 0;
>> +}
>> +
>> +static int mpc85xx_pci_dm_write_config(struct udevice *dev, pci_dev_t
>> bdf,
>> + uint offset, ulong value,
>> + enum pci_size_t size)
>> +{
>> + struct mpc85xx_pci_priv *priv = dev_get_priv(dev);
>> + u32 addr;
>> +
>> + addr = bdf | (offset & 0xfc) | ((offset & 0xf00) << 16) | 0x80000000;
>> + out_be32(priv->cfg_addr, addr);
>> + sync();
>> + out_le32(priv->cfg_data, pci_conv_size_to_32(0, value, offset, size));
>> +
>> + return 0;
>> +}
>> +
>> +static int mpc85xx_pci_dm_probe(struct udevice *dev) {
>> + struct mpc85xx_pci_priv *priv = dev_get_priv(dev);
>> + struct pci_region *io;
>> + struct pci_region *mem;
>> + struct pci_region *pre;
>> + int ret;
>> + ccsr_pcix_t *pcix;
>> +
>> + ret = pci_get_regions(dev, &io, &mem, &pre);
>> + if (ret != 2) {
>> + printf("%s: wrong count of regions %d only 2 allowed\n",
>> + __func__, ret);
>> + return -EINVAL;
>> + }
>> +
>> + pcix = priv->cfg_addr;
>> + /* BAR 1: memory */
>> + out_be32(&pcix->potar1, (mem->bus_start >> 12) & 0x000fffff);
>> + out_be32(&pcix->potear1, 0);
>> + out_be32(&pcix->powbar1, (mem->phys_start >> 12) & 0x000fffff);
>> + out_be32(&pcix->powbear1, 0);
>> + out_be32(&pcix->powar1, (POWAR_EN | POWAR_MEM_READ |
>> + POWAR_MEM_WRITE | (__ilog2(mem->size) - 1)));
>> +
>> + /* BAR 1: IO */
>> + out_be32(&pcix->potar2, (io->bus_start >> 12) & 0x000fffff);
>> + out_be32(&pcix->potear2, 0);
>> + out_be32(&pcix->powbar2, (io->phys_start >> 12) & 0x000fffff);
>> + out_be32(&pcix->powbear2, 0);
>> + out_be32(&pcix->powar2, (POWAR_EN | POWAR_IO_READ |
>> + POWAR_IO_WRITE | (__ilog2(io->size) - 1)));
>> +
>> + out_be32(&pcix->pitar1, 0);
>> + out_be32(&pcix->piwbar1, 0);
>> + out_be32(&pcix->piwar1, (PIWAR_EN | PIWAR_PF | PIWAR_LOCAL |
>> + PIWAR_READ_SNOOP | PIWAR_WRITE_SNOOP |
>> PIWAR_MEM_2G));
>> +
>> + out_be32(&pcix->powar3, 0);
>> + out_be32(&pcix->powar4, 0);
>> + out_be32(&pcix->piwar2, 0);
>> + out_be32(&pcix->piwar3, 0);
>> +
>
> What about the HW issue below?
> #if defined(CONFIG_TARGET_MPC8555CDS) || defined(CONFIG_TARGET_MPC8541CDS)
> /*
> * This is a SW workaround for an apparent HW problem
> * in the PCI controller on the MPC85555/41 CDS boards.
> * The first config cycle must be to a valid, known
> * device on the PCI bus in order to trick the PCI
> * controller state machine into a known valid state.
> * Without this, the first config cycle has the chance
> * of hanging the controller permanently, just leaving
> * it in a semi-working state, or leaving it working.
> *
> * Pick on the Tundra, Device 17, to get it right.
> */
> {
> u8 header_type;
>
> pci_hose_read_config_byte(hose,
> PCI_BDF(0,BRIDGE_ID,0),
> PCI_HEADER_TYPE,
> &header_type);
> }
> #endif
I have no such hw ... if the board maintainers switch to DM support,
they can take a look at it ... I thought this is better than
adding code I could not test ... I add a note in commit message.
Thanks for your review!
bye,
Heiko
>
> Thanks,
> Zhiqiang
>
>> + return 0;
>> +}
>> +
>> +static int mpc85xx_pci_dm_remove(struct udevice *dev) {
>> + return 0;
>> +}
>> +
>> +static int mpc85xx_pci_ofdata_to_platdata(struct udevice *dev) {
>> + struct mpc85xx_pci_priv *priv = dev_get_priv(dev);
>> + fdt_addr_t addr;
>> +
>> + addr = devfdt_get_addr_index(dev, 0);
>> + if (addr == FDT_ADDR_T_NONE)
>> + return -EINVAL;
>> + priv->cfg_addr = (void __iomem *)addr;
>> + addr += 4;
>> + priv->cfg_data = (void __iomem *)addr;
>> +
>> + return 0;
>> +}
>> +
>> +static const struct dm_pci_ops mpc85xx_pci_ops = {
>> + .read_config = mpc85xx_pci_dm_read_config,
>> + .write_config = mpc85xx_pci_dm_write_config,
>> +};
>> +
>> +static const struct udevice_id mpc85xx_pci_ids[] = {
>> + { .compatible = "fsl,mpc8540-pci" },
>> + { }
>> +};
>> +
>> +U_BOOT_DRIVER(mpc85xx_pci) = {
>> + .name = "mpc85xx_pci",
>> + .id = UCLASS_PCI,
>> + .of_match = mpc85xx_pci_ids,
>> + .ops = &mpc85xx_pci_ops,
>> + .probe = mpc85xx_pci_dm_probe,
>> + .remove = mpc85xx_pci_dm_remove,
>> + .ofdata_to_platdata = mpc85xx_pci_ofdata_to_platdata,
>> + .priv_auto_alloc_size = sizeof(struct mpc85xx_pci_priv),
>> +};
>> --
>> 2.21.0
>
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: hs at denx.de
More information about the U-Boot
mailing list