From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id EE8C3C433EF for ; Wed, 29 Sep 2021 07:35:56 +0000 (UTC) Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id EF718613CE for ; Wed, 29 Sep 2021 07:35:55 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org EF718613CE Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 51FEB82DC1; Wed, 29 Sep 2021 09:35:53 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="nS+Buv8K"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 7A97182F00; Wed, 29 Sep 2021 09:35:51 +0200 (CEST) Received: from mail-pf1-x42e.google.com (mail-pf1-x42e.google.com [IPv6:2607:f8b0:4864:20::42e]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 7B3B38281E for ; Wed, 29 Sep 2021 09:35:47 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=tkuw584924@gmail.com Received: by mail-pf1-x42e.google.com with SMTP id m26so1226848pff.3 for ; Wed, 29 Sep 2021 00:35:47 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=gzIQ7JaFqo/0Gyo7lOM+iuEIXhznhMPcL41eo8gIrRI=; b=nS+Buv8K1fMIqG3av8za2AbwnLu9CTMniUq4TgP6nQpl2/mFRXG9J86e/7wajUhJnE tjkIKM0pGYXbJeXfAD530HUUnL2wmbitsC5fh7Y14ES8IUn+6RMa2oZSY6ruR6+o0t48 8BMjg3xNFVExm3POxSwuSKbQzfze8DLxW8jXCpoClJwalNKyyZwBgZkRxQdwnuTOh98e kUkCYqxbbQ6c26iNTKeForQ+IKLHpur4boXEsoRhmO4Gy9ngBuysy4XCQ3Od8/dMfdV1 XXU0dMtNzgpi8unP2LEQz/dsqEN2nP0VMoHiFx0HTBydW97wcAk2pOXMxWIKUQawpvwC 6l2g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=gzIQ7JaFqo/0Gyo7lOM+iuEIXhznhMPcL41eo8gIrRI=; b=c85K2moTYZC2dvS0kCPo+3D2zICZ/vUYwypP1BblX7bRaEGIwmsYhXUC0Fus4Z5TwG ldJmJ5hp0D75qdPq7X+U8dg0S17zRoxfg/fhjRK9b5ysTSaAG0JwL3X0zyW4D/vlXkTL 21P0bl9EknkdE0WndomFO524eRD7N/melUFQ2IahPNPdA1WB2KToxLzx2/Zns+61oqUi APhnP5aYOqSf5S21purtlPVJXtgB1j4Y5ROpE9TmdtGW15X9wtpGWGATHPDvnqn3lYtr k0JuKGcQc3mewCDCP3UZhSJRHRe6B0/AKsUdTRSHt9NkRUAd/KPmB+jeDXrXGSZWtkWw OkHw== X-Gm-Message-State: AOAM530xwSOq2flPUAC4PmWGUuvlmWUJoPB6F+gRPLeEukQnq84PkMdm g/k0jA6XuMIiquVi8rPk4Os= X-Google-Smtp-Source: ABdhPJwCs88MdxLtm1Okanb4jDpJstyX3aWkk+mUy1ypvEB7y/XQ6Ed/+5NvBlEID8g9t2aeEhmSNA== X-Received: by 2002:a05:6a00:2284:b0:43d:fc72:e565 with SMTP id f4-20020a056a00228400b0043dfc72e565mr9916067pfe.84.1632900945482; Wed, 29 Sep 2021 00:35:45 -0700 (PDT) Received: from [192.168.1.3] (fp76ee264d.knge102.ap.nuro.jp. [118.238.38.77]) by smtp.gmail.com with ESMTPSA id 4sm927931pjb.21.2021.09.29.00.35.43 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 29 Sep 2021 00:35:45 -0700 (PDT) Subject: Re: [PATCH] mtd: spi-nor-core: Add fixups for s25fs512s To: Pratyush Yadav Cc: u-boot@lists.denx.de, jagan@amarulasolutions.com, vigneshr@ti.com, Bacem.Daassi@infineon.com, Takahiro Kuwano References: <20210915034954.12462-1-Takahiro.Kuwano@infineon.com> <20210917170148.rx43uecm4xu4w4iw@ti.com> From: Takahiro Kuwano Message-ID: Date: Wed, 29 Sep 2021 16:35:42 +0900 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.14.0 MIME-Version: 1.0 In-Reply-To: <20210917170148.rx43uecm4xu4w4iw@ti.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean On 9/18/2021 2:01 AM, Pratyush Yadav wrote: > On 15/09/21 12:49PM, tkuw584924@gmail.com wrote: >> From: Takahiro Kuwano >> >> The current S25FS512S support has following issues that need to be fixed. >> >> - Non-uniform sectors by factory default. The setting needs to be >> checked and assign erase hook as needed. >> - Page size is wrongly advertised in SFDP. >> - READ_1_1_2 (3Bh/3Ch), READ_1_1_4 (6Bh/6Ch), and PP_1_1_4 (32h/34h) >> are not supported. >> - Bank Address Register (BAR) is not supported. >> >> In addtion, volatile version of Quad Enable is used for safety. >> >> For future use, the fixups is assigned for S25FS-S family. >> >> The datasheet can be found in the following link. >> https://www.cypress.com/file/216376/download >> >> Tested on Xilinx Zynq-7000 FPGA board. >> >> Signed-off-by: Takahiro Kuwano >> --- >> drivers/mtd/spi/spi-nor-core.c | 101 +++++++++++++++++++++++++++++++++ >> 1 file changed, 101 insertions(+) >> >> diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c >> index d5d905fa5a..5e847ebf6a 100644 >> --- a/drivers/mtd/spi/spi-nor-core.c >> +++ b/drivers/mtd/spi/spi-nor-core.c >> @@ -3097,6 +3097,102 @@ static int spi_nor_setup(struct spi_nor *nor, const struct flash_info *info, >> } >> >> #ifdef CONFIG_SPI_FLASH_SPANSION >> +static int s25fs_s_quad_enable(struct spi_nor *nor) >> +{ >> + return spansion_quad_enable_volatile(nor, 0, 8); >> +} >> + >> +static int s25fs_s_erase_non_uniform(struct spi_nor *nor, loff_t addr) >> +{ >> + /* Support 8 x 4KB sectors at bottom */ >> + return spansion_erase_non_uniform(nor, addr, SPINOR_OP_BE_4K_4B, 0, >> + SZ_32K); >> +} >> + >> +static int s25fs_s_setup(struct spi_nor *nor, const struct flash_info *info, >> + const struct spi_nor_flash_parameter *params) >> +{ >> + int ret; >> + u8 cfr3v; >> + >> +#ifdef CONFIG_SPI_FLASH_BAR > > Avoid using #ifdef. Prefer if (CONFIG_IS_ENABLED(SPI_FLASH_BAR) instead. > OK, I will do it. >> + return -ENOTSUPP; /* Bank Address Register is not supported */ > > Same question as the other patch I just reviewed. What would happen if > we don't do this? > If we don't do this, the spi-nor tries to use BAR with 3-byte addressing and the ops to the >16MB memory space will be performed in the first 16MB memory space. >> +#endif >> + /* >> + * Read CR3V to check if uniform sector is selected. If not, assign an >> + * erase hook that supports non-uniform erase. >> + */ >> + ret = spansion_read_any_reg(nor, SPINOR_REG_ADDR_CFR3V, 8, &cfr3v); > > Don't hard-code the 8. Use a #define instead. > I will do it. >> + if (ret) >> + return ret; >> + if (!(cfr3v & CFR3V_UNHYSA)) >> + nor->erase = s25fs_s_erase_non_uniform; > > Ok, but you still don't check for top/bottom configuration. This is the > same as the S25Hx flashes, right? > Yes, right. >> + >> + return spi_nor_default_setup(nor, info, params); >> +} >> + >> +static void s25fs_s_default_init(struct spi_nor *nor) >> +{ >> + nor->setup = s25fs_s_setup; >> +} >> + >> +static int s25fs_s_post_bfpt_fixup(struct spi_nor *nor, >> + const struct sfdp_parameter_header *header, >> + const struct sfdp_bfpt *bfpt, >> + struct spi_nor_flash_parameter *params) >> +{ >> + int ret; >> + u8 cfr3v; >> + >> + /* The erase size is set to 4K from BFPT, but it's wrong. Fix it. */ >> + nor->erase_opcode = SPINOR_OP_SE; >> + nor->mtd.erasesize = nor->info->sector_size; >> + >> + if (params->size > SZ_16M) { >> + ret = nor->write_reg(nor, SPINOR_OP_EN4B, NULL, 0); > > set_4byte() should call this as well, right? Why call it here? > The s25fs-s has SNOR_MFR_SPANSION (0x01). The set_4byte() issues SPINOR_OP_BRWR for SNOR_MFR_SPANSION but the s25fs-s does not support it. The s25fs-s support SPINOR_OP_EN4B instead. >> + if (ret) >> + return ret; >> + nor->addr_width = 4; >> + } else { >> + nor->addr_width = 3; >> + } >> + >> + /* >> + * The page_size is set to 512B from BFPT, but it actually depends on >> + * the configuration register. Look up the CFR3V and determine the >> + * page_size. >> + */ >> + ret = spansion_read_any_reg(nor, SPINOR_REG_ADDR_CFR3V, 8, &cfr3v); >> + if (ret) >> + return ret; >> + >> + if (cfr3v & CFR3V_PGMBUF) >> + params->page_size = 512; >> + else >> + params->page_size = 256; >> + >> + return 0; >> +} >> + >> +static void s25fs_s_post_sfdp_fixup(struct spi_nor *nor, >> + struct spi_nor_flash_parameter *params) >> +{ >> + /* READ_1_1_2 is not supported */ >> + params->hwcaps.mask &= ~SNOR_HWCAPS_READ_1_1_2; >> + /* READ_1_1_4 is not supported */ >> + params->hwcaps.mask &= ~SNOR_HWCAPS_READ_1_1_4; >> + /* PP_1_1_4 is not supported */ >> + params->hwcaps.mask &= ~SNOR_HWCAPS_PP_1_1_4; >> + /* Use volatile register to enable quad */ >> + params->quad_enable = s25fs_s_quad_enable; >> +} >> + >> +static struct spi_nor_fixups s25fs_s_fixups = { >> + .default_init = s25fs_s_default_init, >> + .post_bfpt = s25fs_s_post_bfpt_fixup, >> + .post_sfdp = s25fs_s_post_sfdp_fixup, >> +}; >> + >> static int s25hx_t_mdp_ready(struct spi_nor *nor) >> { >> u32 addr; >> @@ -3644,6 +3740,11 @@ void spi_nor_set_fixups(struct spi_nor *nor) >> break; >> } >> } >> + >> + /* For FS-S (family ID = 0x81) */ >> + if (JEDEC_MFR(nor->info) == SNOR_MFR_SPANSION && >> + nor->info->id[5] == 0x81) >> + nor->fixups = &s25fs_s_fixups; > > Ok. > >> #endif >> >> #ifdef CONFIG_SPI_FLASH_S28HS512T >> -- >> 2.25.1 >> >