On 26/09/17 18:31, Dmitry Osipenko wrote: > On 26.09.2017 19:08, Stephen Warren wrote: >> On 09/26/2017 08:08 AM, Jon Hunter wrote: >>> Hi Dmitry, >>> >>> On 25/09/17 23:35, Dmitry Osipenko wrote: >>>> DMA config is incorrect, because of it DMA transfer is never issued and >>>> tegra20_fuse_read() always returns 0x0. >>>> >>>> Signed-off-by: Dmitry Osipenko <digetx@xxxxxxxxx> >>>> --- >>>> drivers/soc/tegra/fuse/fuse-tegra.c | 1 + >>>> drivers/soc/tegra/fuse/fuse-tegra20.c | 3 ++- >>>> 2 files changed, 3 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/soc/tegra/fuse/fuse-tegra.c >>>> b/drivers/soc/tegra/fuse/fuse-tegra.c >>>> index b7c552e3133c..73a3a2c74021 100644 >>>> --- a/drivers/soc/tegra/fuse/fuse-tegra.c >>>> +++ b/drivers/soc/tegra/fuse/fuse-tegra.c >>>> @@ -132,6 +132,7 @@ static int tegra_fuse_probe(struct platform_device *pdev) >>>> /* take over the memory region from the early initialization */ >>>> res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >>>> + fuse->phys = res->start; >>>> fuse->base = devm_ioremap_resource(&pdev->dev, res); >>>> if (IS_ERR(fuse->base)) >>>> return PTR_ERR(fuse->base); >>>> diff --git a/drivers/soc/tegra/fuse/fuse-tegra20.c >>>> b/drivers/soc/tegra/fuse/fuse-tegra20.c >>>> index 294413a969a0..a33f48c06771 100644 >>>> --- a/drivers/soc/tegra/fuse/fuse-tegra20.c >>>> +++ b/drivers/soc/tegra/fuse/fuse-tegra20.c >>>> @@ -59,7 +59,7 @@ static u32 tegra20_fuse_read(struct tegra_fuse *fuse, >>>> unsigned int offset) >>>> mutex_lock(&fuse->apbdma.lock); >>>> - fuse->apbdma.config.src_addr = fuse->apbdma.phys + FUSE_BEGIN + offset; >>>> + fuse->apbdma.config.src_addr = fuse->phys + FUSE_BEGIN + offset; >>>> err = dmaengine_slave_config(fuse->apbdma.chan, &fuse->apbdma.config); >>>> if (err) >>>> @@ -119,6 +119,7 @@ static int tegra20_fuse_probe(struct tegra_fuse *fuse) >>>> fuse->apbdma.config.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES; >>>> fuse->apbdma.config.src_maxburst = 1; >>>> fuse->apbdma.config.dst_maxburst = 1; >>>> + fuse->apbdma.config.direction = DMA_DEV_TO_MEM; >>>> init_completion(&fuse->apbdma.wait); >>>> mutex_init(&fuse->apbdma.lock); >>> >>> Thanks for the fix. >>> >>> When booting the mainline on Tegra20 Trimslice, I only see the >>> tegra20_fuse_read_early() called and not the tegra20_fuse_read(). That's >>> not to say it is not needed, but I am wondering if we really need this >>> complex tegra20_fuse_read() using DMA and whether we should just have >>> the normal fuse->read() call tegra20_fuse_read_early() as well to >>> simplify matters? >>> >>> Maybe Thierry or Stephen know the history here? >> >> There's some HW bug related to reading the fuse registers from the CPU. The fix >> was implemented long ago by Olof; see the git commit description pasted below. >> IIRC, the code directly reads the fuse registers before the point where DMA is >> available (the argument being we can't do anything else, and this period of time >> is short so the risk hopefully low), but once DMA is available, it is used to >> avoid the HW bug. The bug was apparently fixed in Tegra30; see the other commit >> description pasted below. >> >>> commit e2f91578b35347341482f6af9e4fcf3174531efd >>> Author: Olof Johansson <olof@xxxxxxxxx> >>> Date: Wed Oct 12 23:52:29 2011 -0700 >>> >>> ARM: tegra: use APB DMA for accessing APB devices >>> Tegra2 hangs if APB registers are accessed from the cpu during an >>> apb dma operation. The workaround is to use apb dma to read/write the >>> registers instead. >>> There is a dependency loop between fuses, clocks, and APBDMA. If dma >>> is enabled, fuse reads must go through APBDMA to avoid corruption due >>> to a hw bug. APBDMA requires a clock to be enabled. Clocks must read >>> a fuse to determine allowable cpu frequencies. >>> Separate out the fuse DMA initialization, and allow the fuse read >>> and write functions to be called without using DMA before the DMA >>> initialization has been completed. Access to the fuses before APBDMA >>> is initialized won't hit the hardware bug because nothing else can be >>> using DMA. >>> Original fuse registar access code from Varun Wadekar >>> <vwadekar@xxxxxxxxxx>, improved by Colin Cross <ccross@xxxxxxxxxxx> >>> and later moved to separate driver by Jon Mayo <jmayo@xxxxxxxxxx>. >>> Major refactoring/cleanup by Olof Johansson <olof@xxxxxxxxx>. >>> Changes since v1: >>> * fix 'return false' on error condition >>> * dequeue dma ops in case of timeout >>> From: Jon Mayo <jmayo@xxxxxxxxxx>. >>> Signed-off-by: Jon Mayo <jmayo@xxxxxxxxxx>. >>> Signed-off-by: Olof Johansson <olof@xxxxxxxxx> >>> Acked-by: Stephen Warren <swarren@xxxxxxxxxx> >> >>> commit b861c275ea5cfeab32241c3c92a203579d5699ff >>> Author: Laxman Dewangan <ldewangan@xxxxxxxxxx> >>> Date: Wed Jun 20 18:06:34 2012 +0530 >>> >>> ARM: tegra: apbio access using dma for tegra20 only >>> The Tegra20 HW issue with accessing APBIO registers (such >>> as fuse registers) directly from the CPU concurrently with >>> APB DMA accesses has been fixed in Tegra30 and later chips. >>> Access these registers directly from the CPU on Tegra30 >>> and later, and apply the workaround only for Tegra20. >>> Signed-off-by: Laxman Dewangan <ldewangan@xxxxxxxxxx> >>> Tested-by: Chaitanya Bandi <bandik@xxxxxxxxxx> >>> Signed-off-by: Stephen Warren <swarren@xxxxxxxxxx> > > Thank you very much for the clarification. Ditto, being lazy I had only looked back as far as when it move to drivers/soc/tegra, should have looked further! Great, so looks like a good fix. Cheers Jon -- nvpublic -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
