Date:	Wed, 23 Apr 2014 17:52:15 +0800
From:	Ley Foon Tan <lftan@...era.com>
To:	Arnd Bergmann <arnd@...db.de>
Cc:	Linux-Arch <linux-arch@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
	cltang@...esourcery.com, Matthew Gerlach <mgerlach@...era.com>,
	Vince Bridgers <vbridger@...era.com>
Subject: Re: [PATCH 19/28] nios2: Device tree support

On Wed, Apr 23, 2014 at 3:35 PM, Arnd Bergmann <arnd@...db.de> wrote:

>> > for Altera.
>> Yes, the vendor prefix should be changed to lower case. FYI, this dts
>> file is generated by our dts generator tool called sopc2dts.
>> Our tool already aware of this requirement, but haven't support this yet.
>> I can edit this file manually to change 'ALTR' to 'altr', but I will
>> update nios2 code to match for both "ALTR" and "altr" for backward
>> compatibility. "ALTR" will be deprecated later. Are you okay with
>> this?
>
> You seem to have a large number of "ALTR" strings at the moment.
> I don't think it would be good to allow matching both variants
> everywhere, that would be a lot of extra code.
>
> If it's only about the root compatible property, I have no objections,
> but I don't know if it will help you when all the other strings don't
> match.
ALTR is used on compatible and dts parameters, but these all are in
arch/nios2. Drivers already can handle both ALTR and altr.
I can write a helper function in arch/nios2 to match both ALTR and
altr. I believe it shouldn't take a lot of extra code.
What do you think?

>> >> +                     ranges = < 0x00400000 0x08400000 0x00000020
>> >> +                             0x00004D40 0x08004D40 0x00000008
>> >> +                             0x00004D50 0x08004D50 0x00000008
>> >> +                             0x00004000 0x08004000 0x00000400
>> >> +                             0x00004400 0x08004400 0x00000040
>> >> +                             0x00004800 0x08004800 0x00000040
>> >> +                             0x00002000 0x08002000 0x00002000
>> >> +                             0x00004C80 0x08004C80 0x00000020
>> >> +                             0x00004CC0 0x08004CC0 0x00000010
>> >> +                             0x00004CE0 0x08004CE0 0x00000010
>> >> +                             0x00004D00 0x08004D00 0x00000010 >;
>> >
>
>> > - The ranges should reflect what the bus actually translates,
>> >   which is typically not individual bytes but rather whole
>> >   address ranges.
>> The ranges here reflect the address range translate by each device and
>> user can assign any base address they desired in our FPGA. The address
>> ranges might not in continuous regions as well. So, we will keep this
>> translation way.
>
> If you can support any address, why not just have an empty 'ranges'
> property?
We prefer not to add in any invalid address ranges here.


>>
>> I will add the binding doc for timer, but sysid driver is not in
>> mainline kernel yet. I will remove sysid entry from this file.
>
> Ok. BTW, could the sysid be used for the soc device strings, or
> is that something else?
Initially, we have thought to use sysid here. But, sysid can be used
by our ARM SoC device. For some reasons, we decided to make sysid as
separate module and not tight to CPU.

>
>> >> +                             reg = < 0x00004000 0x00000400
>> >> +                                     0x00004400 0x00000040
>> >> +                                     0x00004800 0x00000040
>> >> +                                     0x00002000 0x00002000 >;
>> >> +                                reg-names = "control_port", "rx_csr", "tx_csr", "s1";
>> >
>> > - wrong order, missing "tx_desc" and "rx_desc" entries
>> FYI, TSE driver supports both SGDMA and MSGDMA soft IP and "tx_desc"
>> and "rx_desc" entries are for MSGDMA (see below). But this 3c120
>> hardware design is using TSE + SGDMA. So, the expect entries are
>> "control_port", "rx_csr", "tx_csr", "s1".
>> I can sort the entries order by their addresses.
>
> You can't reorder the registers, as they can be accessed by
> index in some OS, the strings are just annotations really.
Okay, will keep the order.
Currently, the reg names and ordering are based on hardware
information and dts generator tool generates this entry based on this
hardware information. These shouldn't change unless hardware is
changed.

>
>> Excerpt from Documentation/devicetree/bindings/net/altera_tse.txt
>> - reg-names: Should contain the reg names
>>   "control_port": MAC configuration space region
>>   "tx_csr":       xDMA Tx dispatcher control and status space region
>>   "tx_desc":      MSGDMA Tx dispatcher descriptor space region
>>   "rx_csr" :      xDMA Rx dispatcher control and status space region
>>   "rx_desc":      MSGDMA Rx dispatcher descriptor space region
>>   "rx_resp":      MSGDMA Rx dispatcher response space region
>>   "s1":           SGDMA descriptor memory
>
> The important part is
>
> - reg: Address and length of the register set for the device. It contains
>   the information of registers in the same order as described by reg-names.
>
> You can either provide dummy entries for tx_desc, rx_desc and rx_resp,
> or change the binding to reflect the current usage, and provide two
> different lists depending on the compatible string.
We can update the binding doc to have separate lists depending on the
compatible string.
Added our TSE driver maintainer in CC list.


Regards
Ley Foon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists