| Message ID | 20231111173604.93103-1-mchitale@ventanamicro.com |
|---|---|
| State | Accepted |
| Commit | 891b4814800bd3ad5d16c5d45e0cd05709f64721 |
| Delegated to: | Michal Simek |
| Headers | show |
| Series | [v2] drivers: pcie_xilinx: Fix "reg" not found error | expand |
On 11/11/23 18:36, Mayuresh Chitale wrote: > Fix the driver to use the dev_read_addr_size API to fetch the reg > property from the DT. > > Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com> > --- > Changes in v2: > ==== > - Remove global_data.h from include > - Use devm_ioremap instead of map_phsymem > > drivers/pci/pcie_xilinx.c | 26 +++++++++----------------- > 1 file changed, 9 insertions(+), 17 deletions(-) > > diff --git a/drivers/pci/pcie_xilinx.c b/drivers/pci/pcie_xilinx.c > index 53fd121e90..c1f5bbbb1b 100644 > --- a/drivers/pci/pcie_xilinx.c > +++ b/drivers/pci/pcie_xilinx.c > @@ -8,11 +8,9 @@ > #include <common.h> > #include <dm.h> > #include <pci.h> > -#include <asm/global_data.h> > #include <linux/bitops.h> > #include <linux/printk.h> > - > -#include <asm/io.h> > +#include <linux/io.h> > > /** > * struct xilinx_pcie - Xilinx PCIe controller state > @@ -140,20 +138,14 @@ static int pcie_xilinx_write_config(struct udevice *bus, pci_dev_t bdf, > static int pcie_xilinx_of_to_plat(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(dev), "reg", > - 0, ®_res); > - if (err < 0) { > - pr_err("\"reg\" resource not found\n"); > - return err; > - } > - > - pcie->cfg_base = map_physmem(reg_res.start, > - fdt_resource_size(®_res), > - MAP_NOCACHE); > + fdt_addr_t addr; > + fdt_size_t size; > + > + addr = dev_read_addr_size(dev, &size); > + if (addr == FDT_ADDR_T_NONE) > + return -EINVAL; > + > + pcie->cfg_base = devm_ioremap(dev, addr, size); this can also fail. Just put there. if (IS_ERR(pcie->cfg_base)) return PTR_ERR(pcie->cfg_base); M
one more thing here. Subject of your next patch is [PATCH v2] pci: xilinx: Enable MMIO region and here you are using drivers: pcie_xilinx: Please use one if you target the same code. I prefer "pci: xilinx:" style. Thanks, Michal
On Mon, 2023-11-13 at 09:10 +0100, Michal Simek wrote: > > On 11/11/23 18:36, Mayuresh Chitale wrote: > > Fix the driver to use the dev_read_addr_size API to fetch the reg > > property from the DT. > > > > Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com> > > --- > > Changes in v2: > > ==== > > - Remove global_data.h from include > > - Use devm_ioremap instead of map_phsymem > > > > drivers/pci/pcie_xilinx.c | 26 +++++++++----------------- > > 1 file changed, 9 insertions(+), 17 deletions(-) > > > > diff --git a/drivers/pci/pcie_xilinx.c b/drivers/pci/pcie_xilinx.c > > index 53fd121e90..c1f5bbbb1b 100644 > > --- a/drivers/pci/pcie_xilinx.c > > +++ b/drivers/pci/pcie_xilinx.c > > @@ -8,11 +8,9 @@ > > #include <common.h> > > #include <dm.h> > > #include <pci.h> > > -#include <asm/global_data.h> > > #include <linux/bitops.h> > > #include <linux/printk.h> > > - > > -#include <asm/io.h> > > +#include <linux/io.h> > > > > /** > > * struct xilinx_pcie - Xilinx PCIe controller state > > @@ -140,20 +138,14 @@ static int pcie_xilinx_write_config(struct > > udevice *bus, pci_dev_t bdf, > > static int pcie_xilinx_of_to_plat(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(dev), "reg", > > - 0, ®_res); > > - if (err < 0) { > > - pr_err("\"reg\" resource not found\n"); > > - return err; > > - } > > - > > - pcie->cfg_base = map_physmem(reg_res.start, > > - fdt_resource_size(®_res), > > - MAP_NOCACHE); > > + fdt_addr_t addr; > > + fdt_size_t size; > > + > > + addr = dev_read_addr_size(dev, &size); > > + if (addr == FDT_ADDR_T_NONE) > > + return -EINVAL; > > + > > + pcie->cfg_base = devm_ioremap(dev, addr, size); > > this can also fail. > > Just put there. > if (IS_ERR(pcie->cfg_base)) > return PTR_ERR(pcie->cfg_base); Ok. > > M
On Mon, 2023-11-13 at 10:12 +0100, Michal Simek wrote: > one more thing here. > > Subject of your next patch is > [PATCH v2] pci: xilinx: Enable MMIO region > and here you are using drivers: pcie_xilinx: > > Please use one if you target the same code. > I prefer "pci: xilinx:" style. Ok. > > Thanks, > Michal > >
diff --git a/drivers/pci/pcie_xilinx.c b/drivers/pci/pcie_xilinx.c index 53fd121e90..c1f5bbbb1b 100644 --- a/drivers/pci/pcie_xilinx.c +++ b/drivers/pci/pcie_xilinx.c @@ -8,11 +8,9 @@ #include <common.h> #include <dm.h> #include <pci.h> -#include <asm/global_data.h> #include <linux/bitops.h> #include <linux/printk.h> - -#include <asm/io.h> +#include <linux/io.h> /** * struct xilinx_pcie - Xilinx PCIe controller state @@ -140,20 +138,14 @@ static int pcie_xilinx_write_config(struct udevice *bus, pci_dev_t bdf, static int pcie_xilinx_of_to_plat(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(dev), "reg", - 0, ®_res); - if (err < 0) { - pr_err("\"reg\" resource not found\n"); - return err; - } - - pcie->cfg_base = map_physmem(reg_res.start, - fdt_resource_size(®_res), - MAP_NOCACHE); + fdt_addr_t addr; + fdt_size_t size; + + addr = dev_read_addr_size(dev, &size); + if (addr == FDT_ADDR_T_NONE) + return -EINVAL; + + pcie->cfg_base = devm_ioremap(dev, addr, size); return 0; }
Fix the driver to use the dev_read_addr_size API to fetch the reg property from the DT. Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com> --- Changes in v2: ==== - Remove global_data.h from include - Use devm_ioremap instead of map_phsymem drivers/pci/pcie_xilinx.c | 26 +++++++++----------------- 1 file changed, 9 insertions(+), 17 deletions(-)