On 14.06.2023 12:22, Arseniy Krasnov wrote: > Hello Miquel and Jaime! > > On 14.06.2023 12:10, Miquel Raynal wrote: >> Hi liao, >> >> jaimeliao.tw@xxxxxxxxx wrote on Wed, 14 Jun 2023 17:06:16 +0800: >> >>> Hi Miquel >>> >>> >>>> >>>> Hello, >>>> >>>> AVKrasnov@xxxxxxxxxxxxxx wrote on Tue, 23 May 2023 13:16:34 +0300: >>>> >>>>> This adds support for OTP area access on MX30LFxG18AC chip series. >>>> >>>> Jaime, any feedback on this? Will you test it? >>>> >>>> How are we supposed to test the OTP is locked? I see this is still an >>>> open point. >>> After checking with internal, sub feature parameter are volatile register. >>> >>> It could be change after enter/exit OTP region or power cycle even OTP >>> >>> region have been locked. >>> >>> OTP operation mode still could be enter/exit and region is read only >>> after OTP in protect mode. >>> >>> #program command could execute but no use after setting OTP region in >>> protect mode. >>> >>> So that we can't check whether OTP region is locked via get feature. >>> >>> And we don't have region for checking status of OTP locked. >> >> Ah, too bad. But thanks a lot for the explanation. Arseniy, can you >> please change your comment to explain that the bit is volatile and thus >> there is no way to check if an otp region is locked? I would return >> EOPNOTSUPP in this case and verify that the core cleanly handles the >> situation. > > Ok, thanks for details. @Miquel, ok, I'll change comment from "don't know..." > to this explanation. About EOPNOTSUPP, IIUC I think it is not good way to > suppress '_get_fact_prot_info' and '_get_user_prot_info' callbacks with this > return code as it is the only way to know size of OTP region. > > Thanks, Arseniy Ahh sorry, there is '_lock_user_prot_reg', it will return EOPNOTSUPP. Now I see:) Thanks, Arseniy > >> >> Thanks, >> Miquèl >> >>> >>>> >>>>> >>>>> Signed-off-by: Arseniy Krasnov <AVKrasnov@xxxxxxxxxxxxxx> >>>>> --- >>>>> v1 -> v2: >>>>> * Add slab.h include due to kernel test robot error. >>>>> v2 -> v3: >>>>> * Use 'uint64_t' as input argument for 'do_div()' instead >>>>> of 'unsigned long' due to kernel test robot error. >>>>> v3 -> v4: >>>>> * Use 'dev_err()' instead of 'WARN()'. >>>>> * Call 'match_string()' before checking 'supports_set_get_features' >>>>> in 'macronix_nand_setup_otp(). >>>>> * Use 'u8' instead of 'uint8_t' as ./checkpatch.pl wants. >>>>> >>>>> drivers/mtd/nand/raw/nand_macronix.c | 216 +++++++++++++++++++++++++++ >>>>> 1 file changed, 216 insertions(+) >>>>> >>>>> diff --git a/drivers/mtd/nand/raw/nand_macronix.c b/drivers/mtd/nand/raw/nand_macronix.c >>>>> index 1472f925f386..be1ffa93bebb 100644 >>>>> --- a/drivers/mtd/nand/raw/nand_macronix.c >>>>> +++ b/drivers/mtd/nand/raw/nand_macronix.c >>>>> @@ -6,6 +6,7 @@ >>>>> * Author: Boris Brezillon <boris.brezillon@xxxxxxxxxxxxxxxxxx> >>>>> */ >>>>> >>>>> +#include <linux/slab.h> >>>>> #include "linux/delay.h" >>>>> #include "internals.h" >>>>> >>>>> @@ -31,6 +32,20 @@ >>>>> >>>>> #define MXIC_CMD_POWER_DOWN 0xB9 >>>>> >>>>> +#define ONFI_FEATURE_ADDR_30LFXG18AC_OTP 0x90 >>>>> +#define MACRONIX_30LFXG18AC_OTP_START_PAGE 0 >>>>> +#define MACRONIX_30LFXG18AC_OTP_PAGES 30 >>>>> +#define MACRONIX_30LFXG18AC_OTP_PAGE_SIZE 2112 >>>>> +#define MACRONIX_30LFXG18AC_OTP_START_BYTE \ >>>>> + (MACRONIX_30LFXG18AC_OTP_START_PAGE * \ >>>>> + MACRONIX_30LFXG18AC_OTP_PAGE_SIZE) >>>>> +#define MACRONIX_30LFXG18AC_OTP_SIZE_BYTES \ >>>>> + (MACRONIX_30LFXG18AC_OTP_PAGES * \ >>>>> + MACRONIX_30LFXG18AC_OTP_PAGE_SIZE) >>>>> + >>>>> +#define MACRONIX_30LFXG18AC_OTP_EN BIT(0) >>>>> +#define MACRONIX_30LFXG18AC_OTP_LOCKED BIT(1) >>>>> + >>>>> struct nand_onfi_vendor_macronix { >>>>> u8 reserved; >>>>> u8 reliability_func; >>>>> @@ -316,6 +331,206 @@ static void macronix_nand_deep_power_down_support(struct nand_chip *chip) >>>>> chip->ops.resume = mxic_nand_resume; >>>>> } >>>>> >>>>> +static int macronix_30lfxg18ac_get_otp_info(struct mtd_info *mtd, size_t len, >>>>> + size_t *retlen, >>>>> + struct otp_info *buf) >>>>> +{ >>>>> + if (len < sizeof(*buf)) >>>>> + return -EINVAL; >>>>> + >>>>> + /* Don't know how to check that OTP is locked. */ >>>>> + buf->locked = 0; >>>>> + buf->start = MACRONIX_30LFXG18AC_OTP_START_BYTE; >>>>> + buf->length = MACRONIX_30LFXG18AC_OTP_SIZE_BYTES; >>>>> + >>>>> + *retlen = sizeof(*buf); >>>>> + >>>>> + return 0; >>>>> +} >>>>> + >>>>> +static int macronix_30lfxg18ac_otp_enable(struct nand_chip *nand) >>>>> +{ >>>>> + u8 feature_buf[ONFI_SUBFEATURE_PARAM_LEN] = { 0 }; >>>>> + >>>>> + feature_buf[0] = MACRONIX_30LFXG18AC_OTP_EN; >>>>> + return nand_set_features(nand, ONFI_FEATURE_ADDR_30LFXG18AC_OTP, >>>>> + feature_buf); >>>>> +} >>>>> + >>>>> +static int macronix_30lfxg18ac_otp_disable(struct nand_chip *nand) >>>>> +{ >>>>> + u8 feature_buf[ONFI_SUBFEATURE_PARAM_LEN] = { 0 }; >>>>> + >>>>> + return nand_set_features(nand, ONFI_FEATURE_ADDR_30LFXG18AC_OTP, >>>>> + feature_buf); >>>>> +} >>>>> + >>>>> +static int __macronix_30lfxg18ac_rw_otp(struct mtd_info *mtd, >>>>> + loff_t offs_in_flash, >>>>> + size_t len, size_t *retlen, >>>>> + u_char *buf, bool write) >>>>> +{ >>>>> + struct nand_chip *nand; >>>>> + size_t bytes_handled; >>>>> + off_t offs_in_page; >>>>> + void *dma_buf; >>>>> + u64 page; >>>>> + int ret; >>>>> + >>>>> + /* 'nand_prog/read_page_op()' may use 'buf' as DMA buffer, >>>>> + * so allocate properly aligned memory for it. This is >>>>> + * needed because cross page accesses may lead to unaligned >>>>> + * buffer address for DMA. >>>>> + */ >>>>> + dma_buf = kmalloc(MACRONIX_30LFXG18AC_OTP_PAGE_SIZE, GFP_KERNEL); >>>>> + if (!dma_buf) >>>>> + return -ENOMEM; >>>>> + >>>>> + nand = mtd_to_nand(mtd); >>>>> + nand_select_target(nand, 0); >>>>> + >>>>> + ret = macronix_30lfxg18ac_otp_enable(nand); >>>>> + if (ret) >>>>> + goto out_otp; >>>>> + >>>>> + page = offs_in_flash; >>>>> + /* 'page' will be result of division. */ >>>>> + offs_in_page = do_div(page, MACRONIX_30LFXG18AC_OTP_PAGE_SIZE); >>>>> + bytes_handled = 0; >>>>> + >>>>> + while (bytes_handled < len && >>>>> + page < MACRONIX_30LFXG18AC_OTP_PAGES) { >>>>> + size_t bytes_to_handle; >>>>> + >>>>> + bytes_to_handle = min_t(size_t, len - bytes_handled, >>>>> + MACRONIX_30LFXG18AC_OTP_PAGE_SIZE - >>>>> + offs_in_page); >>>>> + >>>>> + if (write) { >>>>> + memcpy(dma_buf, &buf[bytes_handled], bytes_to_handle); >>>>> + ret = nand_prog_page_op(nand, page, offs_in_page, >>>>> + dma_buf, bytes_to_handle); >>>>> + } else { >>>>> + ret = nand_read_page_op(nand, page, offs_in_page, >>>>> + dma_buf, bytes_to_handle); >>>>> + if (!ret) >>>>> + memcpy(&buf[bytes_handled], dma_buf, >>>>> + bytes_to_handle); >>>>> + } >>>>> + if (ret) >>>>> + goto out_otp; >>>>> + >>>>> + bytes_handled += bytes_to_handle; >>>>> + offs_in_page = 0; >>>>> + page++; >>>>> + } >>>>> + >>>>> + *retlen = bytes_handled; >>>>> + >>>>> +out_otp: >>>>> + if (ret) >>>>> + dev_err(&mtd->dev, "failed to perform OTP IO: %i\n", ret); >>>>> + >>>>> + ret = macronix_30lfxg18ac_otp_disable(nand); >>>>> + if (ret) >>>>> + dev_err(&mtd->dev, "failed to leave OTP mode after %s\n", >>>>> + write ? "write" : "read"); >>>>> + >>>>> + nand_deselect_target(nand); >>>>> + kfree(dma_buf); >>>>> + >>>>> + return ret; >>>>> +} >>>>> + >>>>> +static int macronix_30lfxg18ac_write_otp(struct mtd_info *mtd, loff_t to, >>>>> + size_t len, size_t *rlen, >>>>> + const u_char *buf) >>>>> +{ >>>>> + return __macronix_30lfxg18ac_rw_otp(mtd, to, len, rlen, (u_char *)buf, >>>>> + true); >>>>> +} >>>>> + >>>>> +static int macronix_30lfxg18ac_read_otp(struct mtd_info *mtd, loff_t from, >>>>> + size_t len, size_t *rlen, >>>>> + u_char *buf) >>>>> +{ >>>>> + return __macronix_30lfxg18ac_rw_otp(mtd, from, len, rlen, buf, false); >>>>> +} >>>>> + >>>>> +static int macronix_30lfxg18ac_lock_otp(struct mtd_info *mtd, loff_t from, >>>>> + size_t len) >>>>> +{ >>>>> + u8 feature_buf[ONFI_SUBFEATURE_PARAM_LEN] = { 0 }; >>>>> + struct nand_chip *nand; >>>>> + int ret; >>>>> + >>>>> + if (from != MACRONIX_30LFXG18AC_OTP_START_BYTE || >>>>> + len != MACRONIX_30LFXG18AC_OTP_SIZE_BYTES) >>>>> + return -EINVAL; >>>>> + >>>>> + dev_dbg(&mtd->dev, "locking OTP\n"); >>>>> + >>>>> + nand = mtd_to_nand(mtd); >>>>> + nand_select_target(nand, 0); >>>>> + >>>>> + feature_buf[0] = MACRONIX_30LFXG18AC_OTP_EN | >>>>> + MACRONIX_30LFXG18AC_OTP_LOCKED; >>>>> + ret = nand_set_features(nand, ONFI_FEATURE_ADDR_30LFXG18AC_OTP, >>>>> + feature_buf); >>>>> + if (ret) { >>>>> + dev_err(&mtd->dev, >>>>> + "failed to lock OTP (set features): %i\n", ret); >>>>> + nand_deselect_target(nand); >>>>> + return ret; >>>>> + } >>>>> + >>>>> + /* Do dummy page prog with zero address. */ >>>>> + feature_buf[0] = 0; >>>>> + ret = nand_prog_page_op(nand, 0, 0, feature_buf, 1); >>>>> + if (ret) >>>>> + dev_err(&mtd->dev, >>>>> + "failed to lock OTP (page prog): %i\n", ret); >>>>> + >>>>> + ret = macronix_30lfxg18ac_otp_disable(nand); >>>>> + if (ret) >>>>> + dev_err(&mtd->dev, "failed to leave OTP mode after lock\n"); >>>>> + >>>>> + nand_deselect_target(nand); >>>>> + >>>>> + return ret; >>>>> +} >>>>> + >>>>> +static void macronix_nand_setup_otp(struct nand_chip *chip) >>>>> +{ >>>>> + static const char * const supported_otp_models[] = { >>>>> + "MX30LF1G18AC", >>>>> + "MX30LF2G18AC", >>>>> + "MX30LF4G18AC", >>>>> + }; >>>>> + struct mtd_info *mtd; >>>>> + >>>>> + if (match_string(supported_otp_models, >>>>> + ARRAY_SIZE(supported_otp_models), >>>>> + chip->parameters.model) < 0) >>>>> + return; >>>>> + >>>>> + if (!chip->parameters.supports_set_get_features) >>>>> + return; >>>>> + >>>>> + bitmap_set(chip->parameters.get_feature_list, >>>>> + ONFI_FEATURE_ADDR_30LFXG18AC_OTP, 1); >>>>> + bitmap_set(chip->parameters.set_feature_list, >>>>> + ONFI_FEATURE_ADDR_30LFXG18AC_OTP, 1); >>>>> + >>>>> + mtd = nand_to_mtd(chip); >>>>> + mtd->_get_fact_prot_info = macronix_30lfxg18ac_get_otp_info; >>>>> + mtd->_read_fact_prot_reg = macronix_30lfxg18ac_read_otp; >>>>> + mtd->_get_user_prot_info = macronix_30lfxg18ac_get_otp_info; >>>>> + mtd->_read_user_prot_reg = macronix_30lfxg18ac_read_otp; >>>>> + mtd->_write_user_prot_reg = macronix_30lfxg18ac_write_otp; >>>>> + mtd->_lock_user_prot_reg = macronix_30lfxg18ac_lock_otp; >>>>> +} >>>>> + >>>>> static int macronix_nand_init(struct nand_chip *chip) >>>>> { >>>>> if (nand_is_slc(chip)) >>>>> @@ -325,6 +540,7 @@ static int macronix_nand_init(struct nand_chip *chip) >>>>> macronix_nand_onfi_init(chip); >>>>> macronix_nand_block_protection_support(chip); >>>>> macronix_nand_deep_power_down_support(chip); >>>>> + macronix_nand_setup_otp(chip); >>>>> >>>>> return 0; >>>>> } >>>> >>>> >>>> Thanks, >>>> Miquèl >>> >>> Thanks >>> Jaime >> >>