From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-x229.google.com (mail-wr0-x229.google.com [IPv6:2a00:1450:400c:c0c::229]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id CD1C2208F7AD7 for ; Tue, 12 Sep 2017 01:36:01 -0700 (PDT) Received: by mail-wr0-x229.google.com with SMTP id m18so19061942wrm.2 for ; Tue, 12 Sep 2017 01:38:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=ebCK4nHRyGwOHGCre+7/eU1M+YPPX3+0xlsmAtyJNNY=; b=kJ1mYiRbXB1oLoFZkD/E8diV+AwZvxIsAuAArIPEUGAhtzAXziqZfViDBD7I2RU3jF 5o+QWD05qOdDFKp+ZZ0EE0qkDaWLgb+Fu8m/aZQDzMpBYFzJImTz1+00py1dw5NYWiFt 0PMkxJZ9pzNPv72dNQRu6mvylYW3nxlsnRWzw= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=ebCK4nHRyGwOHGCre+7/eU1M+YPPX3+0xlsmAtyJNNY=; b=OryKcSfIce7kxkhyKfMJPqfAleCyi0YUUERl953bFal/2B4L7glMhUKub/3tDbwjhW GbPYCTC4w/0BT0+I89mP71o7TR0ihxa4geD1vAOqgkItwC9kRcf1I4cxCmcEfKLlMfPJ 3PaSsjNap7N0Un8wwTc1bIU+3ReLKkV9nI6awHIl5h4FDL6eDl1iXs3Nwt6HqpvoGqaH ju1q2nBwCrNjZNJ8uMA4VMjeeBrl/Qwsba5m+clfxazKg6CkCTT0xlQqKYNYc0BjnXXg Nba37+E1/OyPpnJL18xfYqaZdUK3Dv0Xs6irtH0RM+JVwOLUFeNtyWULD/Du7tpVsEcI eGAw== X-Gm-Message-State: AHPjjUhzlpHyG9yleS/50nHNH7Z9HdUe79wNmBiq3N0cfTdg2YwKTfVn 1OeeXVwef7bzA5oY X-Google-Smtp-Source: ADKCNb4dnTcxDqgAoMyD9oorMeRIigLM+IuU8zcZcqW/nORejPIqoOK5M5FNUpSKLQP30RAJvPR4kw== X-Received: by 10.223.139.146 with SMTP id o18mr10984575wra.236.1505205536855; Tue, 12 Sep 2017 01:38:56 -0700 (PDT) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id m9sm7598418wrf.51.2017.09.12.01.38.55 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 12 Sep 2017 01:38:55 -0700 (PDT) Date: Tue, 12 Sep 2017 09:38:54 +0100 From: Leif Lindholm To: methavanitpong.pipat@socionext.com Cc: edk2-devel@lists.01.org, masahisa.kojima@linaro.org, masami.hiramatsu@linaro.org, ard.biesheuvel@linaro.org Message-ID: <20170912083854.bnu4aevg6wnlqkrf@bivouac.eciton.net> References: <20170908182315.9591-1-ard.biesheuvel@linaro.org> <20170908182315.9591-14-ard.biesheuvel@linaro.org> <20170911191301.wcvgqyylckmbiswl@bivouac.eciton.net> MIME-Version: 1.0 In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [PATCH edk2-platforms 13/14] Silicon/Socionext: add driver for SPI NOR flash X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 12 Sep 2017 08:36:02 -0000 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit On Tue, Sep 12, 2017 at 01:41:34AM +0000, methavanitpong.pipat@socionext.com wrote: > > -----Original Message----- > > From: Leif Lindholm [mailto:leif.lindholm@linaro.org] > > Sent: Tuesday, September 12, 2017 4:13 AM > > To: Ard Biesheuvel > > Cc: edk2-devel@lists.01.org; Methavanitpong, Pipat/メタワニットポン ピパット > > ; masahisa.kojima@linaro.org; > > masami.hiramatsu@linaro.org > > Subject: Re: [PATCH edk2-platforms 13/14] Silicon/Socionext: add driver > > for SPI NOR flash > > > > On Fri, Sep 08, 2017 at 07:23:14PM +0100, Ard Biesheuvel wrote: > > > This imports the driver sources provided by Socionext for the FIP006 > > > SPI NOR flash device found on Synquacer SoCs. It has been slightly > > > tweaked to bring it up to date with the changes made on the EDK2 side > > > since it was forked. > > > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > > Signed-off-by: Ard Biesheuvel > [Methavanitpong, Pipat/メタワニットポン ピパット] > Hi Leif, > > 1. Hex magic > > There are 2 hex magic in this driver > > 1. FIP006 command sequencer decode command > > FIP006 in command sequencer mode detects accesses to its memory region. > An access to this region is translated into commands according to > FIP006_REG_CS_RD and FIP006_REG_CS_WR for read and write accordingly. > > A single access can be translated up to 8 1-byte commands, as the > registers have 8 slots each. Each command is transmitted consecutively > starting from their base addresses. > > Each slot in the registers are decoded as > > 1. Immediate value - CSDC(imm, ..., ..., DEC=0) > 2. Addr[7:0] - CSDC(0x00, ..., ..., DEC=1) > 3. Addr[15:8] - CSDC(0x01, ..., ..., DEC=1) > 4. Addr[23:16] - CSDC(0x02, ..., ..., DEC=1) > 5. Addr[31:24] - CSDC(0x03, ..., ..., DEC=1) > 6. HiZ for 1 byte - CSDC(0x04, ..., ..., DEC=1) > 7. Upper 4-bit immediate value + 4-bit HiZ - CSDC((imm << 4) | 0x05, ..., ..., DEC=1) > 8. Command termination - CSDC(0x07, ..., ..., DEC=1) > > 2. Micron N25Q flash command > > In order to send a command to a flash device, FIP006's FIP006_REG_CS_RD > and FIP006_REG_CS_WR must be set properly. NorFlashSetHostCommand() > sets these registers according to an instance's command definition table. > > For example; NorFlashSetHostCommand (Instance, 0x13) corresponds to a > read access with 4-byte addressing read serial nor flash command (0x13). > The result FIP006_REG_CS_RD is the following: > > FIP006_REG_CS_RD[0] = CSDC(0x13, 0, 1-bit lane, DEC=0) > FIP006_REG_CS_RD[1] = CSDC(0x03, 0, 1-bit lane, DEC=1) > FIP006_REG_CS_RD[2] = CSDC(0x02, 0, 1-bit lane, DEC=1) > FIP006_REG_CS_RD[3] = CSDC(0x01, 0, 1-bit lane, DEC=1) > FIP006_REG_CS_RD[4] = CSDC(0x00, 0, 1-bit lane, DEC=1) > FIP006_REG_CS_RD[5] = CSDC(0x07, 0, 1-bit lane, DEC=1) > FIP006_REG_CS_RD[6] = CSDC(0x07, 0, 1-bit lane, DEC=1) > FIP006_REG_CS_RD[7] = CSDC(0x07, 0, 1-bit lane, DEC=1) Thank you for the explanation. However, the code needs to be be readable by someone who does not have the instruction set for each given component fresh in their mind. That means that direct numeral expressions should be avoided wherever they are not mathematically obvious (and where you consider them obvious, consider whether everyone else would agree without already knowing what the code is intended to do). They should be replaced by #defines or enums as appropriate. > 2. Bit field declaration > > About the bitfield struct you mentioned in "Fip006Reg.h", > What should be a good way to declare it? The problem with bitfields is that they are not very well defined in the C language, and hence do not tend to be very portable. Tedious as it may seem, #defines with shifts and masks are usually the way to go. Regards, Leif