From: Grygorii Strashko <grygorii.strashko@ti•com>
To: Arnd Bergmann <arnd@arndb•de>
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, 4 Feb 2016 19:32:46 +0200 [thread overview]
Message-ID: <56B38B3E.1010601@ti.com> (raw)
In-Reply-To: <3707703.8KjeGIMijU@wuerfel>
On 02/04/2016 03:07 PM, Arnd Bergmann wrote:
> 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.
>
I don't see any build warnings (in netcp) with my patch + below diff and If I manually enable
ARCH_DMA_ADDR_T_64BIT.
--- a/drivers/net/ethernet/ti/netcp_core.c
+++ b/drivers/net/ethernet/ti/netcp_core.c
@@ -1058,6 +1058,7 @@ netcp_tx_map_skb(struct sk_buff *skb, struct netcp_intf *netcp)
u32 page_offset = frag->page_offset;
u32 buf_len = skb_frag_size(frag);
dma_addr_t desc_dma;
+ u32 desc_dma_32;
u32 pkt_info;
dma_addr = dma_map_page(dev, page, page_offset, buf_len,
@@ -1079,7 +1080,8 @@ netcp_tx_map_skb(struct sk_buff *skb, struct netcp_intf *netcp)
(netcp->tx_compl_qid & KNAV_DMA_DESC_RETQ_MASK) <<
KNAV_DMA_DESC_RETQ_SHIFT;
set_pkt_info(dma_addr, buf_len, 0, ndesc);
- set_words(&desc_dma, 1, &pdesc->next_desc);
+ desc_dma_32 = (u32)desc_dma;
+ set_words(&desc_dma_32, 1, &pdesc->next_desc);
pkt_len += buf_len;
if (pdesc != desc)
knav_pool_desc_map(netcp->tx_pool, pdesc,
--
regards,
-grygorii
next prev parent reply other threads:[~2016-02-04 17:32 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
2016-02-04 17:32 ` Grygorii Strashko [this message]
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=56B38B3E.1010601@ti.com \
--to=grygorii.strashko@ti$(echo .)com \
--cc=arnd@arndb$(echo .)de \
--cc=davem@davemloft$(echo .)net \
--cc=fcooper@ti$(echo .)com \
--cc=m-karicheri2@ti$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=ssantosh@kernel$(echo .)org \
--cc=w-kwok2@ti$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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