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 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html