From mboxrd@z Thu Jan 1 00:00:00 1970 From: Simon Glass Date: Sun, 14 May 2017 21:02:41 -0600 Subject: [U-Boot] [PATCH 5/5 v2] spi: ich: Configure SPI BIOS parameters for Linux upon U-Boot exit In-Reply-To: References: <20170424074804.15143-1-sr@denx.de> <20170424074804.15143-5-sr@denx.de> <510e2572-5a15-ca4b-796a-08aef2424bc0@denx.de> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Jagan, On 9 May 2017 at 18:47, Bin Meng wrote: > On Tue, May 9, 2017 at 7:20 PM, Stefan Roese wrote: >> (Added Simon to Cc) > > Really added Simon :-) > >> >> >> On 09.05.2017 13:14, Jagan Teki wrote: >>> >>> On Mon, Apr 24, 2017 at 1:18 PM, Stefan Roese wrote: >>>> >>>> This patch adds a remove function to the Intel ICH SPI driver, that will >>>> be called upon U-Boot exit, directly before the OS (Linux) is started. >>>> This function takes care of configuring the BIOS registers in the SPI >>>> controller (similar to what a "standard" BIOS or coreboot does), so that >>>> the Linux MTD device driver is able to correctly read/write to the SPI >>>> NOR chip. Without this, the chip is not detected at all. >>>> >>>> Signed-off-by: Stefan Roese >>>> Reviewed-by: Simon Glass >>>> Cc: Bin Meng >>>> Cc: Jagan Teki >>>> --- >>>> v2: >>>> - Added Simons RB line >>>> >>>> drivers/spi/ich.c | 18 ++++++++++++++++++ >>>> drivers/spi/ich.h | 54 >>>> +++++++++++++++++++++++++++++++++++++++++++++++------- >>>> 2 files changed, 65 insertions(+), 7 deletions(-) >>>> >>>> diff --git a/drivers/spi/ich.c b/drivers/spi/ich.c >>>> index 893fe33b66..bf2e99b5cc 100644 >>>> --- a/drivers/spi/ich.c >>>> +++ b/drivers/spi/ich.c >>>> @@ -617,6 +617,22 @@ static int ich_spi_probe(struct udevice *dev) >>>> return 0; >>>> } >>>> >>>> +static int ich_spi_remove(struct udevice *bus) >>>> +{ >>>> + struct ich_spi_priv *ctlr = dev_get_priv(bus); >>>> + >>>> + /* >>>> + * Configure SPI controller so that the Linux MTD driver can >>>> fully >>>> + * access the SPI NOR chip >>>> + */ >>>> + ich_writew(ctlr, SPI_OPPREFIX, ctlr->preop); >>>> + ich_writew(ctlr, SPI_OPTYPE, ctlr->optype); >>>> + ich_writel(ctlr, SPI_OPMENU_LOWER, ctlr->opmenu); >>>> + ich_writel(ctlr, SPI_OPMENU_UPPER, ctlr->opmenu + sizeof(u32)); >>>> + >>>> + return 0; >>>> +} >>>> + >>>> static int ich_spi_set_speed(struct udevice *bus, uint speed) >>>> { >>>> struct ich_spi_priv *priv = dev_get_priv(bus); >>>> @@ -700,4 +716,6 @@ U_BOOT_DRIVER(ich_spi) = { >>>> .priv_auto_alloc_size = sizeof(struct ich_spi_priv), >>>> .child_pre_probe = ich_spi_child_pre_probe, >>>> .probe = ich_spi_probe, >>>> + .remove = ich_spi_remove, >>>> + .flags = DM_FLAG_OS_PREPARE, >>>> }; >>>> diff --git a/drivers/spi/ich.h b/drivers/spi/ich.h >>>> index bd0a820809..dcb8a9048f 100644 >>>> --- a/drivers/spi/ich.h >>>> +++ b/drivers/spi/ich.h >>>> @@ -102,13 +102,6 @@ enum { >>>> }; >>>> >>>> enum { >>>> - SPI_OPCODE_TYPE_READ_NO_ADDRESS = 0, >>>> - SPI_OPCODE_TYPE_WRITE_NO_ADDRESS = 1, >>>> - SPI_OPCODE_TYPE_READ_WITH_ADDRESS = 2, >>>> - SPI_OPCODE_TYPE_WRITE_WITH_ADDRESS = 3 >>>> -}; >>>> - >>>> -enum { >>>> ICH_MAX_CMD_LEN = 5, >>>> }; >>>> >>>> @@ -124,8 +117,55 @@ struct spi_trans { >>>> uint32_t offset; >>>> }; >>>> >>>> +#define SPI_OPCODE_WRSR 0x01 >>>> +#define SPI_OPCODE_PAGE_PROGRAM 0x02 >>>> +#define SPI_OPCODE_READ 0x03 >>>> +#define SPI_OPCODE_WRDIS 0x04 >>>> +#define SPI_OPCODE_RDSR 0x05 >>>> #define SPI_OPCODE_WREN 0x06 >>>> #define SPI_OPCODE_FAST_READ 0x0b >>>> +#define SPI_OPCODE_ERASE_SECT 0x20 >>>> +#define SPI_OPCODE_READ_ID 0x9f >>>> +#define SPI_OPCODE_ERASE_BLOCK 0xd8 >>> >>> >>> Wonder why the flash part should be part of SPI, can't we use existing >>> spi_flash through command interface if there is specific stuff like >>> this? This driver is odd in that it tries to decode low-level SPI requests coming in via spi_xfer() and issue high-level (SPI flash) commands to the controller. The controller does not actually support generic SPI operation. >> > > These flash commands need to be programmed to SPI controller register, > by Intel's design, to make Linux MTD driver happy. Possibly we may do > like this, like get such value dynamically from spi_flash to program > this to SPI controller? I once asked a question about this: if some > other flash part that does not have the exact same command set as what > was programmed to the SPI controller, will Linux MTD driver still > work? > >> >> This patch only changes some defines here and passes some allowed >> opcodes via some configuration registers to the Linux driver. >> >> I didn't look closely into this U-Boot driver and how it interacts >> with the SPI NOR. Simon is most likely the best person to answer >> on your questions regarding the usage of spi_flash. Simon could >> you please answer Jagan's questions? > > Regards, > Bin Regards, Simon