public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
From: Arnd Bergmann <arnd@arndb.de>
To: Grygorii Strashko <grygorii.strashko@ti.com>
Cc: "Franklin S Cooper Jr." <fcooper@ti.com>,
	m-karicheri2@ti.com, netdev@vger.kernel.org, w-kwok2@ti.com,
	davem@davemloft.net, Santosh Shilimkar <ssantosh@kernel.org>
Subject: Re: Keystone 2 boards boot failure
Date: Thu, 04 Feb 2016 14:07:28 +0100	[thread overview]
Message-ID: <3707703.8KjeGIMijU@wuerfel> (raw)
In-Reply-To: <56B341EA.4020507@ti.com>

On Thursday 04 February 2016 14:19:54 Grygorii Strashko wrote:
> On 02/03/2016 10:40 PM, Arnd Bergmann wrote:
> > On Wednesday 03 February 2016 18:31:00 Grygorii Strashko wrote:
> >> On 02/03/2016 06:20 PM, Arnd Bergmann wrote:
> >>> On Wednesday 03 February 2016 16:21:05 Grygorii Strashko wrote:
> >>>> On 02/03/2016 04:11 PM, Franklin S Cooper Jr. wrote:
> >>>>> On 02/02/2016 07:19 PM, Franklin S Cooper Jr. wrote:
> >> config:
> >>
> >> CONFIG_ARCH_PHYS_ADDR_T_64BIT=y
> >> CONFIG_PHYS_ADDR_T_64BIT=y
> >>
> >> and
> >>
> >> #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT <--- should not be defined for KS2
> >> typedef u64 dma_addr_t;
> >> #else
> >> typedef u32 dma_addr_t;
> >> #endif
> >>
> >> Above is valid configuration for Keystone 2 with LPAE=y
> > 
> > Ok, but what do you mean with "should not be defined"? It clearly is
> > defined in any multiplatform configuration that enables another platform
> > needing 64-bit dma_addr_t.
> > 
> 
> Then we probably have bigger problem :) KS2 will not work as is with
> such configuration and not only KS2 - LPAE is enabled for TI DRA7 also.
> 
> Problem here is that dma_addr_t is used to fill DMA controllers data or can be 
> written directly to register, so all drivers need to be revised which was initially
> created for 32-bit HW and with assumption that dma_addr_t is 32-bits.

Almost everything should work fine. What all other drivers tend to do is
something like

	writel(dma_addr, reg_base + REG_OFFSET);

which works for 64-bit dma_addr_t as long as the upper 32 bits are
always zero, and that should be the case on keystone.

The part that does not work and that originally triggered the
warning I was fixing is

void f(u32 *data)
{
	writel(*data, reg_base + reg_offset);
}

...

	f((u32 *)&dma_addr);

as this driver does. I have not seen the same warning for any
other driver in the kernel, so it is clearly something that
rarely happens, and it still works by chance on little-endian
kernels, just not on big-endian.

> Also, I'm not sure that it will be possible to support both LE/BE in such case.
> 
> Actually, I've tried current multi_v7_defconfig and can see:
> # CONFIG_ARCH_PHYS_ADDR_T_64BIT is not set
> # CONFIG_PHYS_ADDR_T_64BIT is not set
> # CONFIG_ARM_LPAE is not set
> What is your "multiplatform configuration" ??

kernelci is testing multi_v7_defconfig with LPAE and KVM enabled on top.
This breaks all pre-Cortex-A15 platforms (A5, A8 and A9 don't have LPAE)
but should work on any A7/A15/A17 machine.
 
> So I propose to fix this regression first (as we both did - revert changes in 
> get/set_pad_info()) and have KS2 working again with current version of
> defconfig files (keystone_defconfig & multi_v7_defconfig) while this
> discussion is continuing. 

Could you please at least test the last patch I sent? It really shouldn't
be too hard to find the line that was wrong in my patch.

	Arnd

  reply	other threads:[~2016-02-04 13:07 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-02 16:50 Keystone 2 boards boot failure Franklin S Cooper Jr.
2016-02-02 20:41 ` Arnd Bergmann
2016-02-02 21:01   ` Franklin S Cooper Jr.
2016-02-02 21:26     ` Arnd Bergmann
2016-02-02 22:59       ` Franklin Cooper
2016-02-02 23:26         ` Arnd Bergmann
2016-02-03  1:19           ` Franklin S Cooper Jr.
2016-02-03 14:11             ` Franklin S Cooper Jr.
2016-02-03 14:21               ` Grygorii Strashko
2016-02-03 15:37                 ` Franklin S Cooper Jr.
2016-02-03 16:20                 ` Arnd Bergmann
2016-02-03 16:31                   ` Grygorii Strashko
2016-02-03 16:45                     ` Murali Karicheri
2016-02-03 20:40                     ` Arnd Bergmann
2016-02-04 12:19                       ` Grygorii Strashko
2016-02-04 13:07                         ` Arnd Bergmann [this message]
2016-02-04 17:32                           ` Grygorii Strashko
2016-02-03 16:41                   ` Murali Karicheri
2016-02-03 20:41                     ` Arnd Bergmann
2016-02-04 16:25                   ` Grygorii Strashko
2016-02-05 16:18                     ` Arnd Bergmann
2016-02-05 17:11                       ` Grygorii Strashko
2016-02-08 13:59                         ` Arnd Bergmann
2016-02-03 16:35             ` Arnd Bergmann
2016-02-03 17:08               ` santosh shilimkar
2016-02-03 18:47                 ` Murali Karicheri
2016-02-03 20:13                   ` santosh shilimkar
2016-02-05 18:55                     ` Murali Karicheri

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3707703.8KjeGIMijU@wuerfel \
    --to=arnd@arndb.de \
    --cc=davem@davemloft.net \
    --cc=fcooper@ti.com \
    --cc=grygorii.strashko@ti.com \
    --cc=m-karicheri2@ti.com \
    --cc=netdev@vger.kernel.org \
    --cc=ssantosh@kernel.org \
    --cc=w-kwok2@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox