From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dmitry Osipenko Subject: Re: [PATCH v1 1/3] soc/tegra: fuse: Fix reading registers using DMA on Tegra20 Date: Tue, 26 Sep 2017 20:31:21 +0300 Message-ID: <9921a66d-90f0-5a69-2da3-dc2c355eb86a@gmail.com> References: <27dadd0335aac71c9d4d613c33a6a1d0a285afa4.1506378772.git.digetx@gmail.com> <87d33561-d959-7444-552d-7226adf29eb4@nvidia.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Return-path: In-Reply-To: Content-Language: en-US Sender: linux-tegra-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Stephen Warren , Jon Hunter , Thierry Reding Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Stephen Warren List-Id: linux-tegra@vger.kernel.org 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 >>> --- >>>   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 >> 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 >>     , improved by Colin Cross >>     and later moved to separate driver by Jon Mayo . >>         Major refactoring/cleanup by Olof Johansson . >>         Changes since v1: >>         * fix 'return false' on error condition >>     * dequeue dma ops in case of timeout >>         From: Jon Mayo . >>     Signed-off-by: Jon Mayo . >>     Signed-off-by: Olof Johansson >>     Acked-by: Stephen Warren > >> commit b861c275ea5cfeab32241c3c92a203579d5699ff >> Author: Laxman Dewangan >> 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 >>     Tested-by: Chaitanya Bandi >>     Signed-off-by: Stephen Warren Thank you very much for the clarification. -- Dmitry