lkml.org 
[lkml]   [2015]   [Aug]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v1 3/3] arm64: dts: add Hi6220 mailbox node
    Hi Mark,

    On Thu, Aug 27, 2015 at 05:31:09PM +0100, Mark Rutland wrote:
    > On Wed, Aug 26, 2015 at 07:59:50AM +0100, Leo Yan wrote:
    > > Hi Haojian,
    > >
    > > On Wed, Aug 26, 2015 at 09:25:41AM +0800, Haojian Zhuang wrote:
    > > > On Wed, 2015-08-26 at 00:00 +0800, Leo Yan wrote:
    > > > > On Tue, Aug 25, 2015 at 09:43:14PM +0800, Haojian Zhuang wrote:
    > > > > > On Tue, 2015-08-25 at 11:42 +0100, Mark Rutland wrote:
    > > > > > > > > Are you then going to hack GRUB, release a special HiKey version of
    > > > > > > > > GRUB, not support any other versions, and still can your firmware
    > > > > > > > > UEFI?
    > > > > > > >
    > > > > > > > I don't need to hack GRUB at all.
    > > > > > >
    > > > > > > Then it is working for you by pure chance alone.
    > > > > > >
    > > > > > > Please listen to the advice you are being given here; we're trying to
    > > > > > > ensure that your platform functions (and continues to function) as best
    > > > > > > it can.
    > > > > >
    > > > > > Since we discussed a lot on this, let's make a conclusion on it.
    > > > > >
    > > > > > 1. UEFI could append the reserved buffer in it's memory mapping.
    > > > > > 2. These reserved buffer must be declared in DT, since we also need to
    > > > > > support non-UEFI (uboot) at the same time.
    > > > > > 3. Mailbox node should reference reserved buffer by phandle in DT. Then
    > > > > > map the buffer as non-cacheable in driver.
    > > > > > 4. These reserved buffer must use "no-map" property since it should be
    > > > > > non-cacheable in driver.
    > > > >
    > > > > For more specific discussion for DTS, i list two options at here;
    > > > >
    > > > > - Option 1: just simply reserve memory regions through memory node,
    > > > > and mailbox node will directly use the buffer through reg ranges;
    > > > >
    > > > > - Option 2: use reserved-memory and mailbox node will refer phandle
    > > > > of reserved-memory;
    > > > >
    > > > > These two options both can work well with UEFI and Uboot, but option 1
    > > > > is more simple and straightforward; so i personally prefer it. But
    > > > > look forwarding your guys' suggestion.
    > > > >
    > > > > Option 1:
    > > > >
    > > > > memory@0 {
    > > > > device_type = "memory";
    > > > > reg = <0x00000000 0x00000000 0x00000000 0x05e00000>,
    > > > > <0x00000000 0x05f00000 0x00000000 0x00eff000>,
    > > > > <0x00000000 0x06e00000 0x00000000 0x0060f000>,
    > > > > <0x00000000 0x07410000 0x00000000 0x38bf0000>;
    > > > > };
    > > > >
    > > > > [...]
    > > > >
    > > > > mailbox: mailbox@f7510000 {
    > > > > #mbox-cells = <1>;
    > > > > compatible = "hisilicon,hi6220-mbox";
    > > > > reg = <0x0 0xf7510000 0x0 0x1000>, /* IPC_S */
    > > > > <0x0 0x06dff800 0x0 0x0800>; /* Mailbox buffer */
    > > > > interrupts = <GIC_SPI 94 IRQ_TYPE_LEVEL_HIGH>;
    > > > > };
    > > > >
    > > > > Option 2:
    > > > >
    > > > > memory@0 {
    > > > > device_type = "memory";
    > > > > reg = <0x0 0x0 0x0 0x40000000>;
    > > > > };
    > > > >
    > > > > reserved-memory {
    > > > > #address-cells = <2>;
    > > > > #size-cells = <2>;
    > > > > ranges;
    > > > >
    > > > > mcu_reserved: mcu_reserved@06dff000 {
    > > > > no-map;
    > > > > reg = <0x0 0x06dff000 0x0 0x00001000>, /* MCU mailbox buffer */
    > > > > <0x0 0x05e00000 0x0 0x00100000>, /* MCU firmware buffer */
    > > > > <0x0 0x0740f000 0x0 0x00001000>; /* MCU firmware section */
    > > > > };
    > > > > };
    > > > >
    > > > > [...]
    > > > >
    > > > > mailbox: mailbox@f7510000 {
    > > > > #mbox-cells = <1>;
    > > > > compatible = "hisilicon,hi6220-mbox";
    > > > > reg = <0x0 0xf7510000 0x0 0x1000>; /* IPC_S */
    > > > > memory-region = <&mcu_reserved>; /* Mailbox buffer */
    > > > > interrupts = <GIC_SPI 94 IRQ_TYPE_LEVEL_HIGH>;
    > > > > };
    > > >
    > > > I prefer the second one. From my view, memory node should only describe
    > > > the hardware information of memory.
    > >
    > > Yes, option 2 will be more simple for memory node.
    > >
    > > But option 2 also will introduce complexity for mailbox node, due mailbox
    > > driver need use property "reg" and "memory-region" to sepeately depict
    > > the regions for mailbox's ipc and slots. If later mailbox is designed to
    > > use SRAM for both ipc and slots, then it will no matter with DDR anymore,
    > > in this case option 1 will easily switch to support it.
    > >
    > > Another question is a general question: for Linux kernel, which is the
    > > best method to reserve memory regions? According to previous discussion,
    > > we can use /memory/ node or /reseved-memory/ node to reserve memory
    > > regions.
    >
    > If the memory is truly reserved for a purpose and cannot be used for
    > anything else, I don't think it should be in the memory node at all, and
    > should be carved out. That aligns with what you'd do in UEFI (either not
    > listing the region in the memory map, or listing it with attributes such
    > that it may not be mapped and/or used).
    >
    > I don't see much of a reason for /memreserve/, as it can cause issues
    > (by allowing the OS to map the region with cacheable attributes), and is
    > not as rigorously specified for ARM as it is for Power in ePAPR.
    >
    > I understand that reserved-memory is for carving out (potentially
    > reusable) memory pools for devices or other special uses (perhaps a
    > panic log). Usually such memory may also be used by the kernel for its
    > own purposes if not presently required by the device.
    >
    > Having an entry in reserved-memory does not necessitate the region also
    > appears in memory nodes, and if a region cannot be used by an OS (or
    > other software) for other purposes, I would not expect it to be describe
    > in any memory node. That will prevent other software (e.g. bootloaders)
    > from erroneously using the memory.
    >
    > If you have a region described with no-map, I would expect that this
    > doesn't exist in any memory node or the UEFI memory map, and is only
    > under reserved-memory so it may be referred to by phandle in a
    > consistent manner.
    >
    > > when review Juno's dts, i also see there have reserved 16MB DDR for secure
    > > world. If so, looks like /reserved-memory/ node is unnecessary. if have some
    > > specific scenarios will we use reserved-memory node to help reserve memory
    > > regions?
    >
    > I'd expect shared DMA pools to appear in reserved-memory. The OS can
    > choose to use these or ignore them if it chooses (or is otherwise forced
    > to, e.g. were it loaded over one).

    Thanks a lot for detailed explain; it's quite clear now.

    Thanks,
    Leo Yan


    \
     
     \ /
      Last update: 2015-08-28 09:01    [W:2.376 / U:0.284 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site