On 20. 09. 19 10:26, Simon Goldschmidt wrote: > > > Michal Simek <michal.si...@xilinx.com <mailto:michal.si...@xilinx.com>> > schrieb am Fr., 20. Sep. 2019, 10:17: > > On 19. 09. 19 16:35, Simon Goldschmidt wrote: > > > > > > Michal Simek <michal.si...@xilinx.com > <mailto:michal.si...@xilinx.com> <mailto:michal.si...@xilinx.com > <mailto:michal.si...@xilinx.com>>> > > schrieb am Do., 19. Sep. 2019, 16:22: > > > > On 17. 09. 19 14:53, Simon Goldschmidt wrote: > > > On Tue, Sep 17, 2019 at 2:48 PM Michal Simek > > <michal.si...@xilinx.com <mailto:michal.si...@xilinx.com> > <mailto:michal.si...@xilinx.com <mailto:michal.si...@xilinx.com>>> > wrote: > > >> > > >> On 17. 09. 19 14:43, Simon Goldschmidt wrote: > > >>> On Tue, Sep 17, 2019 at 1:22 PM Michal Simek > > <michal.si...@xilinx.com <mailto:michal.si...@xilinx.com> > <mailto:michal.si...@xilinx.com <mailto:michal.si...@xilinx.com>>> > wrote: > > >>>> > > >>>> From: Ashok Reddy Soma <ashok.reddy.s...@xilinx.com > <mailto:ashok.reddy.s...@xilinx.com> > > <mailto:ashok.reddy.s...@xilinx.com > <mailto:ashok.reddy.s...@xilinx.com>>> > > >>>> > > >>>> When two instances of AXI QSPI with flash are added and > tested > > >>>> simultaneously the spi driver operations are relocated twice. > > >>>> As a result code is accessing addresses outside of RAM when > > >>>> relocated second time which is causing a crash. > > >>>> > > >>>> Tested on Microblaze. > > >>>> > > >>>> Similar change was done in past by: > > >>>> commit f238b3f0fbc9 ("watchdog: dm: Support manual relocation > > for watchdogs") > > >>>> commit 2588f2ddfd60 ("dm: sf: Add support for all targets > which > > requires MANUAL_RELOC") > > >>>> commit 1b4c2aa25bdf ("gpio: dm: Support manual relocation for > > gpio") > > >>>> > > >>>> Signed-off-by: Ashok Reddy Soma > <ashok.reddy.s...@xilinx.com <mailto:ashok.reddy.s...@xilinx.com> > > <mailto:ashok.reddy.s...@xilinx.com > <mailto:ashok.reddy.s...@xilinx.com>>> > > >>>> Signed-off-by: Michal Simek <michal.si...@xilinx.com > <mailto:michal.si...@xilinx.com> > > <mailto:michal.si...@xilinx.com <mailto:michal.si...@xilinx.com>>> > > >>>> --- > > >>>> > > >>>> drivers/spi/spi-uclass.c | 34 > +++++++++++++++++++--------------- > > >>>> 1 file changed, 19 insertions(+), 15 deletions(-) > > >>>> > > >>>> diff --git a/drivers/spi/spi-uclass.c > b/drivers/spi/spi-uclass.c > > >>>> index 88cb2a126227..6d63006d186c 100644 > > >>>> --- a/drivers/spi/spi-uclass.c > > >>>> +++ b/drivers/spi/spi-uclass.c > > >>>> @@ -129,21 +129,25 @@ static int spi_post_probe(struct > udevice > > *bus) > > >>>> #endif > > >>>> #if defined(CONFIG_NEEDS_MANUAL_RELOC) > > >>>> struct dm_spi_ops *ops = spi_get_ops(bus); > > >>>> - > > >>>> - if (ops->claim_bus) > > >>>> - ops->claim_bus += gd->reloc_off; > > >>>> - if (ops->release_bus) > > >>>> - ops->release_bus += gd->reloc_off; > > >>>> - if (ops->set_wordlen) > > >>>> - ops->set_wordlen += gd->reloc_off; > > >>>> - if (ops->xfer) > > >>>> - ops->xfer += gd->reloc_off; > > >>>> - if (ops->set_speed) > > >>>> - ops->set_speed += gd->reloc_off; > > >>>> - if (ops->set_mode) > > >>>> - ops->set_mode += gd->reloc_off; > > >>>> - if (ops->cs_info) > > >>>> - ops->cs_info += gd->reloc_off; > > >>>> + static int reloc_done; > > >>> > > >>> So with CONFIG_NEEDS_MANUAL_RELOC, we don't support multiple > > >>> (differen) spi (or watchdog, sf, gpio) drivers? > > >> > > >> If you look at commits above you will see that several > subsystems > > >> support multiple instances already. > > >> We can check others as well. > > > > > > I have looked at the commits above, that's why I ask. To me, > this code > > > relocates the ops of the device called first. If a second device > > has different > > > ops, it won't get relocated, or am I wrong? > > > > > > Given your other similar commits that were already accepted, I > > don't want to > > > hold this one, but I still would like to understand. > > > > I think you are right. It should be called once per driver and > after > > relocation. That means that this should be the part variable > should be > > the part of every ops and there should be also handling in > connection to > > bind/unbind method. > > On microblaze which is one architecture common systems only > one driver > > is in place that's why none has found any issue with it. > > > > Let me play with this a little bit to see if we can setup > system with > > also bind/unbind to see how this is exactly working. > > > > > > Right, that's what I came to when thinking about it again. That > way, it > > could even be the framework that relocates all ops... > > Framework is doing it already but we should do it without static > variable. > > > Right, it's done in the UCLASSes. What I meant was it could already be > done in the core DM framework.
DM framework could call it and we can get it out of post_probe functions but still actual relocation should be likely done in uclasses because every device type has different ops. Thanks, Michal _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot