| Message ID | 20180427125339.1308-2-mario.six@gdsys.cc |
|---|---|
| State | Changes Requested |
| Delegated to: | Simon Glass |
| Headers | show |
| Series | [U-Boot,v2,1/3] pci: Don't use pci_indirect when DM is active | expand |
Hi Mario, On 27 April 2018 at 06:53, Mario Six <mario.six@gdsys.cc> wrote: > Register the IMMR region as a PCI region when PCI is used on MPC83xx. > > Signed-off-by: Mario Six <mario.six@gdsys.cc> > --- > > v1 -> v2: > No changes > > --- > drivers/pci/pci-uclass.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c > index a2e829608a..37ca09d76b 100644 > --- a/drivers/pci/pci-uclass.c > +++ b/drivers/pci/pci-uclass.c > @@ -902,6 +902,11 @@ static int decode_regions(struct pci_controller *hose, ofnode parent_node, > base, size, PCI_REGION_MEM | PCI_REGION_SYS_MEMORY); > #endif > > +#if defined(MPC83xx) && defined(CONFIG_SYS_IMMR) > + pci_set_region(hose->regions + hose->region_count++, > + CONFIG_SYS_IMMR, CONFIG_SYS_IMMR, 0x100000, > + PCI_REGION_MEM | PCI_REGION_SYS_MEMORY); > +#endif Please don't add device-specific code in this file. This could go in the device tree, perhaps. If not, we perhaps need to do it in your PCI driver? > return 0; > } > > -- > 2.16.1 > Regards, Simon
Hi Simon, On Thu, May 3, 2018 at 9:02 PM, Simon Glass <sjg@chromium.org> wrote: > Hi Mario, > > On 27 April 2018 at 06:53, Mario Six <mario.six@gdsys.cc> wrote: >> Register the IMMR region as a PCI region when PCI is used on MPC83xx. >> >> Signed-off-by: Mario Six <mario.six@gdsys.cc> >> --- >> >> v1 -> v2: >> No changes >> >> --- >> drivers/pci/pci-uclass.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c >> index a2e829608a..37ca09d76b 100644 >> --- a/drivers/pci/pci-uclass.c >> +++ b/drivers/pci/pci-uclass.c >> @@ -902,6 +902,11 @@ static int decode_regions(struct pci_controller *hose, ofnode parent_node, >> base, size, PCI_REGION_MEM | PCI_REGION_SYS_MEMORY); >> #endif >> >> +#if defined(MPC83xx) && defined(CONFIG_SYS_IMMR) >> + pci_set_region(hose->regions + hose->region_count++, >> + CONFIG_SYS_IMMR, CONFIG_SYS_IMMR, 0x100000, >> + PCI_REGION_MEM | PCI_REGION_SYS_MEMORY); >> +#endif > > Please don't add device-specific code in this file. This could go in > the device tree, perhaps. If not, we perhaps need to do it in your PCI > driver? > OK, I'll try to put it into the DT (I have absolutely no expertise with PCI, so I'm still learning with this one). >> return 0; >> } >> >> -- >> 2.16.1 >> > > Regards, > Simon > Best regards, Mario
Hi Mario, On 4 May 2018 at 02:15, Mario Six <mario.six@gdsys.cc> wrote: > Hi Simon, > > On Thu, May 3, 2018 at 9:02 PM, Simon Glass <sjg@chromium.org> wrote: >> Hi Mario, >> >> On 27 April 2018 at 06:53, Mario Six <mario.six@gdsys.cc> wrote: >>> Register the IMMR region as a PCI region when PCI is used on MPC83xx. >>> >>> Signed-off-by: Mario Six <mario.six@gdsys.cc> >>> --- >>> >>> v1 -> v2: >>> No changes >>> >>> --- >>> drivers/pci/pci-uclass.c | 5 +++++ >>> 1 file changed, 5 insertions(+) >>> >>> diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c >>> index a2e829608a..37ca09d76b 100644 >>> --- a/drivers/pci/pci-uclass.c >>> +++ b/drivers/pci/pci-uclass.c >>> @@ -902,6 +902,11 @@ static int decode_regions(struct pci_controller *hose, ofnode parent_node, >>> base, size, PCI_REGION_MEM | PCI_REGION_SYS_MEMORY); >>> #endif >>> >>> +#if defined(MPC83xx) && defined(CONFIG_SYS_IMMR) >>> + pci_set_region(hose->regions + hose->region_count++, >>> + CONFIG_SYS_IMMR, CONFIG_SYS_IMMR, 0x100000, >>> + PCI_REGION_MEM | PCI_REGION_SYS_MEMORY); >>> +#endif >> >> Please don't add device-specific code in this file. This could go in >> the device tree, perhaps. If not, we perhaps need to do it in your PCI >> driver? >> > > OK, I'll try to put it into the DT (I have absolutely no expertise with PCI, so > I'm still learning with this one). Here's a possible idea: decode_regions() is called from the PCI uclass' pre_probe() method. This means that by the time your PCI controller enters the probe() method, all the regions should be set up. So you should be able to add the above code into the probe() method of your PCI controller driver. Perhaps can check a compatible string etc. to decide whether to add this region, if it is no described in the DT itself. Regards, Simon
Hi Simon, On Fri, May 4, 2018 at 11:37 PM, Simon Glass <sjg@chromium.org> wrote: > Hi Mario, > > On 4 May 2018 at 02:15, Mario Six <mario.six@gdsys.cc> wrote: >> Hi Simon, >> >> On Thu, May 3, 2018 at 9:02 PM, Simon Glass <sjg@chromium.org> wrote: >>> Hi Mario, >>> >>> On 27 April 2018 at 06:53, Mario Six <mario.six@gdsys.cc> wrote: >>>> Register the IMMR region as a PCI region when PCI is used on MPC83xx. >>>> >>>> Signed-off-by: Mario Six <mario.six@gdsys.cc> >>>> --- >>>> >>>> v1 -> v2: >>>> No changes >>>> >>>> --- >>>> drivers/pci/pci-uclass.c | 5 +++++ >>>> 1 file changed, 5 insertions(+) >>>> >>>> diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c >>>> index a2e829608a..37ca09d76b 100644 >>>> --- a/drivers/pci/pci-uclass.c >>>> +++ b/drivers/pci/pci-uclass.c >>>> @@ -902,6 +902,11 @@ static int decode_regions(struct pci_controller > *hose, ofnode parent_node, >>>> base, size, PCI_REGION_MEM | > PCI_REGION_SYS_MEMORY); >>>> #endif >>>> >>>> +#if defined(MPC83xx) && defined(CONFIG_SYS_IMMR) >>>> + pci_set_region(hose->regions + hose->region_count++, >>>> + CONFIG_SYS_IMMR, CONFIG_SYS_IMMR, 0x100000, >>>> + PCI_REGION_MEM | PCI_REGION_SYS_MEMORY); >>>> +#endif >>> >>> Please don't add device-specific code in this file. This could go in >>> the device tree, perhaps. If not, we perhaps need to do it in your PCI >>> driver? >>> >> >> OK, I'll try to put it into the DT (I have absolutely no expertise with > PCI, so >> I'm still learning with this one). > > Here's a possible idea: > > decode_regions() is called from the PCI uclass' pre_probe() method. This > means that by the time your PCI controller enters the probe() method, all > the regions should be set up. So you should be able to add the above code > into the probe() method of your PCI controller driver. Perhaps can check a > compatible string etc. to decide whether to add this region, if it is no > described in the DT itself. > OK, I'll do that for v3, thanks for the explanation. > Regards, > Simon > Best regards, Mario
diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index a2e829608a..37ca09d76b 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -902,6 +902,11 @@ static int decode_regions(struct pci_controller *hose, ofnode parent_node, base, size, PCI_REGION_MEM | PCI_REGION_SYS_MEMORY); #endif +#if defined(MPC83xx) && defined(CONFIG_SYS_IMMR) + pci_set_region(hose->regions + hose->region_count++, + CONFIG_SYS_IMMR, CONFIG_SYS_IMMR, 0x100000, + PCI_REGION_MEM | PCI_REGION_SYS_MEMORY); +#endif return 0; }
Register the IMMR region as a PCI region when PCI is used on MPC83xx. Signed-off-by: Mario Six <mario.six@gdsys.cc> --- v1 -> v2: No changes --- drivers/pci/pci-uclass.c | 5 +++++ 1 file changed, 5 insertions(+)