Michal Simek michal.simek@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.simek@xilinx.com
wrote:
On 17. 09. 19 14:43, Simon Goldschmidt wrote:
On Tue, Sep 17, 2019 at 1:22 PM Michal Simek michal.simek@xilinx.com
wrote:
From: Ashok Reddy Soma ashok.reddy.soma@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.soma@xilinx.com Signed-off-by: Michal Simek michal.simek@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...
Regards, Simon
Thanks, Michal